From e289dd9e11d1b34950cbf9a5cb315729b61bec30 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Thu, 2 Jul 2026 22:56:54 -0400 Subject: [PATCH 01/16] ENG-1734 Add Roam semantic search to Advanced Node Search Wire Advanced Node Search to Roam semantic search when enabled, with discourse-note filtering and MiniSearch fallback when fewer than five semantic results remain. Co-authored-by: Cursor --- .../useAdvancedNodeSearchResults.ts | 135 +++++++++-- .../AdvancedNodeSearchDialog/utils.ts | 12 + .../discourseNodeSemanticSearch.test.ts | 217 ++++++++++++++++++ .../src/utils/discourseNodeSearchProviders.ts | 2 +- .../src/utils/discourseNodeSemanticSearch.ts | 88 +++++++ 5 files changed, 440 insertions(+), 14 deletions(-) create mode 100644 apps/roam/src/utils/__tests__/discourseNodeSemanticSearch.test.ts create mode 100644 apps/roam/src/utils/discourseNodeSemanticSearch.ts diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index d3a6828646..475eb36886 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -1,5 +1,10 @@ -import { useMemo } from "react"; +import { useEffect, useState } from "react"; import MiniSearch from "minisearch"; +import getDiscourseNodes from "~/utils/getDiscourseNodes"; +import { + searchDiscourseNodesWithSemanticFallback, + type DiscourseSearchHit, +} from "~/utils/discourseNodeSemanticSearch"; import { searchIndexedNodes, sortSearchResults, @@ -23,6 +28,57 @@ type UseAdvancedNodeSearchResultsArgs = { dockedResults?: SearchResult[]; }; +const toKeywordHits = ( + scoredHits: ReturnType, +): DiscourseSearchHit[] => + scoredHits.map((hit) => ({ + uid: hit.result.uid, + text: hit.result.title, + type: hit.result.type, + nodeTypeLabel: hit.result.nodeTypeLabel, + score: hit.score, + source: "keyword" as const, + })); + +const hitsToScoredSearchHits = ({ + hits, + resultsByUid, +}: { + hits: DiscourseSearchHit[]; + resultsByUid: Map; +}) => + hits + .map((hit) => { + const indexedResult = resultsByUid.get(hit.uid); + if (indexedResult) { + return { result: indexedResult, score: hit.score, source: hit.source }; + } + + return { + result: { + uid: hit.uid, + title: hit.text, + type: hit.type || "", + nodeTypeLabel: hit.nodeTypeLabel || "", + excerpt: "", + createdAt: "", + lastModified: "", + authorName: "Unknown", + }, + score: hit.score, + source: hit.source, + }; + }) + .filter( + ( + hit, + ): hit is { + result: SearchResult; + score: number; + source: DiscourseSearchHit["source"]; + } => !!hit, + ); + export const useAdvancedNodeSearchResults = ({ debouncedSearchTerm, selectedNodeTypeIds, @@ -32,30 +88,80 @@ export const useAdvancedNodeSearchResults = ({ searchIndex, dockedQuery, dockedResults, -}: UseAdvancedNodeSearchResultsArgs): SearchResult[] => - useMemo(() => { - if (!debouncedSearchTerm) return []; +}: UseAdvancedNodeSearchResultsArgs): SearchResult[] => { + const [results, setResults] = useState([]); + + useEffect(() => { + if (!debouncedSearchTerm) { + setResults([]); + return; + } const isDockedQuery = dockedQuery !== undefined && debouncedSearchTerm.trim() === dockedQuery.trim(); if (isDockedQuery && dockedResults) { - return dockedResults; + setResults(dockedResults); + return; } if (isIndexLoading || indexError || !searchIndex) { - return []; + setResults([]); + return; } - const scoredHits = searchIndexedNodes({ - miniSearch: searchIndex.miniSearch, - allResults: searchIndex.allResults, - searchTerm: debouncedSearchTerm, - typeFilter: selectedNodeTypeIds.length ? selectedNodeTypeIds : undefined, - }); + let cancelled = false; + const typeFilter = selectedNodeTypeIds.length + ? selectedNodeTypeIds + : undefined; + const discourseNodes = getDiscourseNodes().filter( + (node) => + node.backedBy === "user" && + (!typeFilter || typeFilter.includes(node.type)), + ); + const resultsByUid = new Map( + searchIndex.allResults.map((result) => [result.uid, result]), + ); - return sortSearchResults({ hits: scoredHits, sort }); + const runKeywordSearch = (): DiscourseSearchHit[] => + toKeywordHits( + searchIndexedNodes({ + miniSearch: searchIndex.miniSearch, + allResults: searchIndex.allResults, + searchTerm: debouncedSearchTerm, + typeFilter, + }), + ); + + void searchDiscourseNodesWithSemanticFallback({ + nodeTypes: discourseNodes, + query: debouncedSearchTerm, + runKeywordSearch, + }) + .then((hits) => { + if (cancelled) return; + + const scoredHits = hitsToScoredSearchHits({ hits, resultsByUid }); + setResults(sortSearchResults({ hits: scoredHits, sort })); + }) + .catch((error) => { + console.error("Advanced node search failed:", error); + if (cancelled) return; + setResults( + sortSearchResults({ + hits: hitsToScoredSearchHits({ + hits: runKeywordSearch(), + resultsByUid, + }), + sort, + }), + ); + }); + + return () => { + cancelled = true; + }; }, [ debouncedSearchTerm, dockedQuery, @@ -66,3 +172,6 @@ export const useAdvancedNodeSearchResults = ({ selectedNodeTypeIds, sort, ]); + + return results; +}; diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts index c3040eadf9..4b08f17bed 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts @@ -59,6 +59,7 @@ export type DockedSearchState = { export type ScoredSearchHit = { result: SearchResult; score: number; + source?: "semantic" | "keyword"; }; type MiniSearchDocument = SearchResult & { @@ -229,6 +230,17 @@ export const sortSearchResults = ({ switch (sort.field) { case "relevance": + if (aHit.source && bHit.source && aHit.source !== bHit.source) { + comparison = + sort.direction === "desc" + ? aHit.source === "semantic" + ? -1 + : 1 + : aHit.source === "semantic" + ? 1 + : -1; + break; + } comparison = compareNumbers(aHit.score, bHit.score, sort.direction); break; case "alphabetical": diff --git a/apps/roam/src/utils/__tests__/discourseNodeSemanticSearch.test.ts b/apps/roam/src/utils/__tests__/discourseNodeSemanticSearch.test.ts new file mode 100644 index 0000000000..a814887076 --- /dev/null +++ b/apps/roam/src/utils/__tests__/discourseNodeSemanticSearch.test.ts @@ -0,0 +1,217 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { DiscourseNode } from "~/utils/getDiscourseNodes"; + +const mockRunRoamSemanticSearch = vi.fn(); + +vi.mock("~/utils/discourseNodeSearchProviders", () => ({ + runRoamSemanticSearch: (...args: unknown[]) => + mockRunRoamSemanticSearch(...args), +})); + +import { + SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS, + combineDiscourseSearchResults, + searchDiscourseNodesWithSemanticFallback, + shouldUseRoamSemanticSearch, + type DiscourseSearchHit, +} from "~/utils/discourseNodeSemanticSearch"; + +const nodeTypes = [ + { type: "claim", format: "[[C]] - {content}" }, +] as DiscourseNode[]; + +const semanticHit = ( + uid: string, + score: number, + text = `[[C]] - ${uid}`, +): DiscourseSearchHit => ({ + uid, + text, + type: "claim", + nodeTypeLabel: "Claim", + score, + source: "semantic", +}); + +const keywordHit = ( + uid: string, + score: number, + text = `[[C]] - ${uid}`, +): DiscourseSearchHit => ({ + uid, + text, + type: "claim", + nodeTypeLabel: "Claim", + score, + source: "keyword", +}); + +const providerPayload = ( + filteredResults: Array<{ + uid: string; + text: string; + type?: string; + nodeTypeLabel?: string; + score?: number; + }>, +) => ({ + rawResults: filteredResults, + rawResultCount: filteredResults.length, + filteredResults, + filteredResultCount: filteredResults.length, +}); + +describe("shouldUseRoamSemanticSearch", () => { + beforeEach(() => { + (globalThis as { window: unknown }).window = { + roamAlphaAPI: { + data: { + semanticSearchEnabled: vi.fn(() => false), + }, + }, + }; + }); + + it("returns the Roam semantic search enabled flag", () => { + const semanticSearchEnabled = vi.fn(() => true); + (globalThis as { window: unknown }).window = { + roamAlphaAPI: { + data: { semanticSearchEnabled }, + }, + }; + + expect(shouldUseRoamSemanticSearch()).toBe(true); + expect(semanticSearchEnabled).toHaveBeenCalled(); + }); +}); + +describe("combineDiscourseSearchResults", () => { + it("keeps semantic order and appends deduped keyword hits", () => { + const combined = combineDiscourseSearchResults({ + semantic: [semanticHit("a", 0.9), semanticHit("b", 0.8)], + keyword: [keywordHit("b", 0.7), keywordHit("c", 0.6)], + }); + + expect(combined.map((hit) => hit.uid)).toEqual(["a", "b", "c"]); + expect(combined[0]?.source).toBe("semantic"); + expect(combined[2]?.source).toBe("keyword"); + }); +}); + +describe("searchDiscourseNodesWithSemanticFallback", () => { + const runKeywordSearch = vi.fn(() => [ + keywordHit("keyword-1", 0.5), + keywordHit("keyword-2", 0.4), + ]); + + beforeEach(() => { + vi.clearAllMocks(); + (globalThis as { window: unknown }).window = { + roamAlphaAPI: { + data: { + semanticSearchEnabled: vi.fn(() => true), + }, + }, + }; + }); + + it("uses keyword search only when semantic search is disabled", async () => { + ( + window.roamAlphaAPI.data.semanticSearchEnabled as ReturnType + ).mockReturnValue(false); + + const results = await searchDiscourseNodesWithSemanticFallback({ + nodeTypes, + query: "meaning", + runKeywordSearch, + }); + + expect(results).toEqual(runKeywordSearch.mock.results[0]?.value); + expect(mockRunRoamSemanticSearch).not.toHaveBeenCalled(); + }); + + it("returns semantic results only when post-filter count is at threshold", async () => { + const filteredResults = Array.from( + { length: SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS }, + (_, index) => ({ + uid: `semantic-${index}`, + text: `[[C]] - semantic-${index}`, + type: "claim", + nodeTypeLabel: "Claim", + score: 1 - index * 0.1, + }), + ); + mockRunRoamSemanticSearch.mockResolvedValue( + providerPayload(filteredResults), + ); + + const results = await searchDiscourseNodesWithSemanticFallback({ + nodeTypes, + query: "meaning", + runKeywordSearch, + }); + + expect(results).toHaveLength(SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS); + expect(results.every((hit) => hit.source === "semantic")).toBe(true); + expect(runKeywordSearch).not.toHaveBeenCalled(); + }); + + it("combines semantic and keyword results when post-filter count is below threshold", async () => { + mockRunRoamSemanticSearch.mockResolvedValue( + providerPayload([ + { + uid: "semantic-1", + text: "[[C]] - semantic-1", + type: "claim", + nodeTypeLabel: "Claim", + score: 0.9, + }, + { + uid: "keyword-1", + text: "[[C]] - keyword-1", + type: "claim", + nodeTypeLabel: "Claim", + score: 0.8, + }, + ]), + ); + + const results = await searchDiscourseNodesWithSemanticFallback({ + nodeTypes, + query: "meaning", + runKeywordSearch, + }); + + expect(results.map((hit) => hit.uid)).toEqual([ + "semantic-1", + "keyword-1", + "keyword-2", + ]); + expect(runKeywordSearch).toHaveBeenCalledTimes(1); + }); + + it("falls back to keyword search when the Roam semantic API throws", async () => { + mockRunRoamSemanticSearch.mockRejectedValue(new Error("semantic failed")); + + const results = await searchDiscourseNodesWithSemanticFallback({ + nodeTypes, + query: "meaning", + runKeywordSearch, + }); + + expect(results).toEqual(runKeywordSearch.mock.results[0]?.value); + expect(results.every((hit) => hit.source === "keyword")).toBe(true); + }); + + it("returns an empty list for blank queries", async () => { + const results = await searchDiscourseNodesWithSemanticFallback({ + nodeTypes, + query: " ", + runKeywordSearch, + }); + + expect(results).toEqual([]); + expect(mockRunRoamSemanticSearch).not.toHaveBeenCalled(); + expect(runKeywordSearch).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/roam/src/utils/discourseNodeSearchProviders.ts b/apps/roam/src/utils/discourseNodeSearchProviders.ts index a34d1be34f..24f7595412 100644 --- a/apps/roam/src/utils/discourseNodeSearchProviders.ts +++ b/apps/roam/src/utils/discourseNodeSearchProviders.ts @@ -249,7 +249,7 @@ export const searchSemanticNodeTitles = async ({ ); }; -const runRoamSemanticSearch = async ({ +export const runRoamSemanticSearch = async ({ nodeTypes, query, }: { diff --git a/apps/roam/src/utils/discourseNodeSemanticSearch.ts b/apps/roam/src/utils/discourseNodeSemanticSearch.ts new file mode 100644 index 0000000000..eb42252b3b --- /dev/null +++ b/apps/roam/src/utils/discourseNodeSemanticSearch.ts @@ -0,0 +1,88 @@ +import type { DiscourseNode } from "~/utils/getDiscourseNodes"; +import { + runRoamSemanticSearch, + type AdminSearchResultItem, +} from "~/utils/discourseNodeSearchProviders"; + +export const SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS = 5; + +export type DiscourseSearchHitSource = "semantic" | "keyword"; + +export type DiscourseSearchHit = { + uid: string; + text: string; + type?: string; + nodeTypeLabel?: string; + score: number; + source: DiscourseSearchHitSource; +}; + +export const shouldUseRoamSemanticSearch = (): boolean => + window.roamAlphaAPI.data.semanticSearchEnabled(); + +export const combineDiscourseSearchResults = ({ + semantic, + keyword, +}: { + semantic: DiscourseSearchHit[]; + keyword: DiscourseSearchHit[]; +}): DiscourseSearchHit[] => { + const seenUids = new Set(semantic.map((hit) => hit.uid)); + const combined = [...semantic]; + + keyword.forEach((hit) => { + if (seenUids.has(hit.uid)) return; + seenUids.add(hit.uid); + combined.push(hit); + }); + + return combined; +}; + +const toSemanticHit = (item: AdminSearchResultItem): DiscourseSearchHit => ({ + uid: item.uid, + text: item.text, + type: item.type, + nodeTypeLabel: item.nodeTypeLabel, + score: item.score ?? 0, + source: "semantic", +}); + +export const searchDiscourseNodesWithSemanticFallback = async ({ + nodeTypes, + query, + runKeywordSearch, +}: { + nodeTypes: DiscourseNode[]; + query: string; + runKeywordSearch: () => DiscourseSearchHit[]; +}): Promise => { + const trimmedQuery = query.trim(); + if (!trimmedQuery) return []; + + if (!shouldUseRoamSemanticSearch()) { + return runKeywordSearch(); + } + + try { + const providerResult = await runRoamSemanticSearch({ + nodeTypes, + query: trimmedQuery, + }); + const semanticHits = providerResult.filteredResults.map(toSemanticHit); + + if ( + providerResult.filteredResultCount >= + SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS + ) { + return semanticHits; + } + + return combineDiscourseSearchResults({ + semantic: semanticHits, + keyword: runKeywordSearch(), + }); + } catch { + return runKeywordSearch(); + } +}; From 9c80e1e0295c326fe388980ebafd0891b94ab5f7 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Thu, 2 Jul 2026 23:00:15 -0400 Subject: [PATCH 02/16] ENG-1734 Remove discourseNodeSemanticSearch unit tests from PR scope. Co-authored-by: Cursor --- .../discourseNodeSemanticSearch.test.ts | 217 ------------------ 1 file changed, 217 deletions(-) delete mode 100644 apps/roam/src/utils/__tests__/discourseNodeSemanticSearch.test.ts diff --git a/apps/roam/src/utils/__tests__/discourseNodeSemanticSearch.test.ts b/apps/roam/src/utils/__tests__/discourseNodeSemanticSearch.test.ts deleted file mode 100644 index a814887076..0000000000 --- a/apps/roam/src/utils/__tests__/discourseNodeSemanticSearch.test.ts +++ /dev/null @@ -1,217 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; -import type { DiscourseNode } from "~/utils/getDiscourseNodes"; - -const mockRunRoamSemanticSearch = vi.fn(); - -vi.mock("~/utils/discourseNodeSearchProviders", () => ({ - runRoamSemanticSearch: (...args: unknown[]) => - mockRunRoamSemanticSearch(...args), -})); - -import { - SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS, - combineDiscourseSearchResults, - searchDiscourseNodesWithSemanticFallback, - shouldUseRoamSemanticSearch, - type DiscourseSearchHit, -} from "~/utils/discourseNodeSemanticSearch"; - -const nodeTypes = [ - { type: "claim", format: "[[C]] - {content}" }, -] as DiscourseNode[]; - -const semanticHit = ( - uid: string, - score: number, - text = `[[C]] - ${uid}`, -): DiscourseSearchHit => ({ - uid, - text, - type: "claim", - nodeTypeLabel: "Claim", - score, - source: "semantic", -}); - -const keywordHit = ( - uid: string, - score: number, - text = `[[C]] - ${uid}`, -): DiscourseSearchHit => ({ - uid, - text, - type: "claim", - nodeTypeLabel: "Claim", - score, - source: "keyword", -}); - -const providerPayload = ( - filteredResults: Array<{ - uid: string; - text: string; - type?: string; - nodeTypeLabel?: string; - score?: number; - }>, -) => ({ - rawResults: filteredResults, - rawResultCount: filteredResults.length, - filteredResults, - filteredResultCount: filteredResults.length, -}); - -describe("shouldUseRoamSemanticSearch", () => { - beforeEach(() => { - (globalThis as { window: unknown }).window = { - roamAlphaAPI: { - data: { - semanticSearchEnabled: vi.fn(() => false), - }, - }, - }; - }); - - it("returns the Roam semantic search enabled flag", () => { - const semanticSearchEnabled = vi.fn(() => true); - (globalThis as { window: unknown }).window = { - roamAlphaAPI: { - data: { semanticSearchEnabled }, - }, - }; - - expect(shouldUseRoamSemanticSearch()).toBe(true); - expect(semanticSearchEnabled).toHaveBeenCalled(); - }); -}); - -describe("combineDiscourseSearchResults", () => { - it("keeps semantic order and appends deduped keyword hits", () => { - const combined = combineDiscourseSearchResults({ - semantic: [semanticHit("a", 0.9), semanticHit("b", 0.8)], - keyword: [keywordHit("b", 0.7), keywordHit("c", 0.6)], - }); - - expect(combined.map((hit) => hit.uid)).toEqual(["a", "b", "c"]); - expect(combined[0]?.source).toBe("semantic"); - expect(combined[2]?.source).toBe("keyword"); - }); -}); - -describe("searchDiscourseNodesWithSemanticFallback", () => { - const runKeywordSearch = vi.fn(() => [ - keywordHit("keyword-1", 0.5), - keywordHit("keyword-2", 0.4), - ]); - - beforeEach(() => { - vi.clearAllMocks(); - (globalThis as { window: unknown }).window = { - roamAlphaAPI: { - data: { - semanticSearchEnabled: vi.fn(() => true), - }, - }, - }; - }); - - it("uses keyword search only when semantic search is disabled", async () => { - ( - window.roamAlphaAPI.data.semanticSearchEnabled as ReturnType - ).mockReturnValue(false); - - const results = await searchDiscourseNodesWithSemanticFallback({ - nodeTypes, - query: "meaning", - runKeywordSearch, - }); - - expect(results).toEqual(runKeywordSearch.mock.results[0]?.value); - expect(mockRunRoamSemanticSearch).not.toHaveBeenCalled(); - }); - - it("returns semantic results only when post-filter count is at threshold", async () => { - const filteredResults = Array.from( - { length: SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS }, - (_, index) => ({ - uid: `semantic-${index}`, - text: `[[C]] - semantic-${index}`, - type: "claim", - nodeTypeLabel: "Claim", - score: 1 - index * 0.1, - }), - ); - mockRunRoamSemanticSearch.mockResolvedValue( - providerPayload(filteredResults), - ); - - const results = await searchDiscourseNodesWithSemanticFallback({ - nodeTypes, - query: "meaning", - runKeywordSearch, - }); - - expect(results).toHaveLength(SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS); - expect(results.every((hit) => hit.source === "semantic")).toBe(true); - expect(runKeywordSearch).not.toHaveBeenCalled(); - }); - - it("combines semantic and keyword results when post-filter count is below threshold", async () => { - mockRunRoamSemanticSearch.mockResolvedValue( - providerPayload([ - { - uid: "semantic-1", - text: "[[C]] - semantic-1", - type: "claim", - nodeTypeLabel: "Claim", - score: 0.9, - }, - { - uid: "keyword-1", - text: "[[C]] - keyword-1", - type: "claim", - nodeTypeLabel: "Claim", - score: 0.8, - }, - ]), - ); - - const results = await searchDiscourseNodesWithSemanticFallback({ - nodeTypes, - query: "meaning", - runKeywordSearch, - }); - - expect(results.map((hit) => hit.uid)).toEqual([ - "semantic-1", - "keyword-1", - "keyword-2", - ]); - expect(runKeywordSearch).toHaveBeenCalledTimes(1); - }); - - it("falls back to keyword search when the Roam semantic API throws", async () => { - mockRunRoamSemanticSearch.mockRejectedValue(new Error("semantic failed")); - - const results = await searchDiscourseNodesWithSemanticFallback({ - nodeTypes, - query: "meaning", - runKeywordSearch, - }); - - expect(results).toEqual(runKeywordSearch.mock.results[0]?.value); - expect(results.every((hit) => hit.source === "keyword")).toBe(true); - }); - - it("returns an empty list for blank queries", async () => { - const results = await searchDiscourseNodesWithSemanticFallback({ - nodeTypes, - query: " ", - runKeywordSearch, - }); - - expect(results).toEqual([]); - expect(mockRunRoamSemanticSearch).not.toHaveBeenCalled(); - expect(runKeywordSearch).not.toHaveBeenCalled(); - }); -}); From 894901f32321a04a4fcd96c6e242f4a6f71c2d37 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Thu, 2 Jul 2026 23:19:18 -0400 Subject: [PATCH 03/16] ENG-1734 Fix docked search snapshot with async semantic search. Return frozen docked results synchronously and avoid persisting empty results that clobber the sidebar registry on mount. Co-authored-by: Cursor --- .../AdvancedSearchSidebarPanel.tsx | 8 +++- .../dockedSearchSnapshot.ts | 14 ++++++ .../useAdvancedNodeSearchResults.ts | 43 +++++++++-------- .../utils/__tests__/isDockedSnapshot.test.ts | 46 +++++++++++++++++++ 4 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 apps/roam/src/components/AdvancedNodeSearchDialog/dockedSearchSnapshot.ts create mode 100644 apps/roam/src/utils/__tests__/isDockedSnapshot.test.ts diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx b/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx index 463e98e211..6ae5dd4a11 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx @@ -237,10 +237,13 @@ export const AdvancedSearchSidebarPanel = ({ dockedResults, }); + const isFrozenSnapshot = + debouncedSearchTerm.trim() === query.trim() && dockedResults.length > 0; + useEffect(() => { onPersistState({ query: debouncedSearchTerm, - results, + results: isFrozenSnapshot ? dockedResults : results, selectedNodeTypeIds, sort, windowId, @@ -249,7 +252,10 @@ export const AdvancedSearchSidebarPanel = ({ }, [ debouncedSearchTerm, dgSearchId, + dockedResults, + isFrozenSnapshot, onPersistState, + query, results, selectedNodeTypeIds, sort, diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/dockedSearchSnapshot.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/dockedSearchSnapshot.ts new file mode 100644 index 0000000000..2d20030889 --- /dev/null +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/dockedSearchSnapshot.ts @@ -0,0 +1,14 @@ +import type { SearchResult } from "./utils"; + +export const isDockedSnapshot = ({ + debouncedSearchTerm, + dockedQuery, + dockedResults, +}: { + debouncedSearchTerm: string; + dockedQuery?: string; + dockedResults?: SearchResult[]; +}): boolean => + dockedQuery !== undefined && + debouncedSearchTerm.trim() === dockedQuery.trim() && + !!dockedResults?.length; diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index 475eb36886..d8b4541c32 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; import MiniSearch from "minisearch"; import getDiscourseNodes from "~/utils/getDiscourseNodes"; import { @@ -11,6 +11,7 @@ import { type SearchResult, type SortConfig, } from "./utils"; +import { isDockedSnapshot } from "./dockedSearchSnapshot"; export type SearchIndex = { miniSearch: MiniSearch; @@ -89,25 +90,28 @@ export const useAdvancedNodeSearchResults = ({ dockedQuery, dockedResults, }: UseAdvancedNodeSearchResultsArgs): SearchResult[] => { - const [results, setResults] = useState([]); + const frozenSnapshot = useMemo( + () => + isDockedSnapshot({ + debouncedSearchTerm, + dockedQuery, + dockedResults, + }), + [debouncedSearchTerm, dockedQuery, dockedResults], + ); + + const [liveResults, setLiveResults] = useState([]); useEffect(() => { - if (!debouncedSearchTerm) { - setResults([]); - return; - } + if (frozenSnapshot) return; - const isDockedQuery = - dockedQuery !== undefined && - debouncedSearchTerm.trim() === dockedQuery.trim(); - - if (isDockedQuery && dockedResults) { - setResults(dockedResults); + if (!debouncedSearchTerm) { + setLiveResults([]); return; } if (isIndexLoading || indexError || !searchIndex) { - setResults([]); + setLiveResults([]); return; } @@ -143,12 +147,12 @@ export const useAdvancedNodeSearchResults = ({ if (cancelled) return; const scoredHits = hitsToScoredSearchHits({ hits, resultsByUid }); - setResults(sortSearchResults({ hits: scoredHits, sort })); + setLiveResults(sortSearchResults({ hits: scoredHits, sort })); }) .catch((error) => { console.error("Advanced node search failed:", error); if (cancelled) return; - setResults( + setLiveResults( sortSearchResults({ hits: hitsToScoredSearchHits({ hits: runKeywordSearch(), @@ -164,8 +168,7 @@ export const useAdvancedNodeSearchResults = ({ }; }, [ debouncedSearchTerm, - dockedQuery, - dockedResults, + frozenSnapshot, indexError, isIndexLoading, searchIndex, @@ -173,5 +176,9 @@ export const useAdvancedNodeSearchResults = ({ sort, ]); - return results; + if (frozenSnapshot && dockedResults) { + return dockedResults; + } + + return liveResults; }; diff --git a/apps/roam/src/utils/__tests__/isDockedSnapshot.test.ts b/apps/roam/src/utils/__tests__/isDockedSnapshot.test.ts new file mode 100644 index 0000000000..16b6995183 --- /dev/null +++ b/apps/roam/src/utils/__tests__/isDockedSnapshot.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest"; +import { isDockedSnapshot } from "~/components/AdvancedNodeSearchDialog/dockedSearchSnapshot"; +import type { SearchResult } from "~/components/AdvancedNodeSearchDialog/utils"; + +const sampleResult = (uid: string): SearchResult => ({ + uid, + title: `[[C]] - ${uid}`, + type: "claim", + nodeTypeLabel: "Claim", + excerpt: "", + createdAt: "", + lastModified: "", + authorName: "Unknown", +}); + +describe("isDockedSnapshot", () => { + it("returns true when the query matches and docked results exist", () => { + expect( + isDockedSnapshot({ + debouncedSearchTerm: "meaning", + dockedQuery: "meaning", + dockedResults: [sampleResult("a")], + }), + ).toBe(true); + }); + + it("returns false when the query differs", () => { + expect( + isDockedSnapshot({ + debouncedSearchTerm: "edited query", + dockedQuery: "meaning", + dockedResults: [sampleResult("a")], + }), + ).toBe(false); + }); + + it("returns false when docked results are empty", () => { + expect( + isDockedSnapshot({ + debouncedSearchTerm: "meaning", + dockedQuery: "meaning", + dockedResults: [], + }), + ).toBe(false); + }); +}); From 3b97b9b15ae6ae72afffb6c9180a485a4334cffa Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 3 Jul 2026 21:13:16 -0400 Subject: [PATCH 04/16] ENG-1734 Simplify dock handling and fix sort reactivity. Use main-branch inline docked query check, remove dockedSearchSnapshot helper and Roam tests, and apply sort in useMemo so sort changes do not re-run async search. Co-authored-by: Cursor --- AGENTS.md | 5 +- apps/roam/AGENTS.md | 4 ++ .../AdvancedSearchSidebarPanel.tsx | 8 +-- .../dockedSearchSnapshot.ts | 14 ----- .../useAdvancedNodeSearchResults.ts | 60 ++++++++----------- .../utils/__tests__/isDockedSnapshot.test.ts | 46 -------------- 6 files changed, 32 insertions(+), 105 deletions(-) delete mode 100644 apps/roam/src/components/AdvancedNodeSearchDialog/dockedSearchSnapshot.ts delete mode 100644 apps/roam/src/utils/__tests__/isDockedSnapshot.test.ts diff --git a/AGENTS.md b/AGENTS.md index 9562c116fd..a29a1ef843 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,5 +81,6 @@ PR titles for Linear-backed work should follow this format: ### Testing -- Write unit tests for new functionality -- Ensure tests are meaningful and maintainable +- Write unit tests for new functionality when the target app or package has established test infrastructure (a `test` script in its `package.json` and existing tests that run in CI for that package). +- Do not add unit tests to apps or packages without that infrastructure. For example, `apps/roam` does not run tests in CI, so do not generate Roam unit tests unless the user explicitly asks for them. +- Ensure tests are meaningful and maintainable when they are added. diff --git a/apps/roam/AGENTS.md b/apps/roam/AGENTS.md index 9888d3c4aa..7cd2500613 100644 --- a/apps/roam/AGENTS.md +++ b/apps/roam/AGENTS.md @@ -17,3 +17,7 @@ When styling Roam UI, use this priority order: Any new Roam UI should feel native to Roam and consistent with existing BlueprintJS and repository usage, not like a separate visual palette. Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj. Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU. + +## Testing + +Do not add unit tests for Roam unless the user explicitly asks. Roam is not covered by CI test runs; validate Roam changes manually in the extension instead. diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx b/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx index 6ae5dd4a11..463e98e211 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx @@ -237,13 +237,10 @@ export const AdvancedSearchSidebarPanel = ({ dockedResults, }); - const isFrozenSnapshot = - debouncedSearchTerm.trim() === query.trim() && dockedResults.length > 0; - useEffect(() => { onPersistState({ query: debouncedSearchTerm, - results: isFrozenSnapshot ? dockedResults : results, + results, selectedNodeTypeIds, sort, windowId, @@ -252,10 +249,7 @@ export const AdvancedSearchSidebarPanel = ({ }, [ debouncedSearchTerm, dgSearchId, - dockedResults, - isFrozenSnapshot, onPersistState, - query, results, selectedNodeTypeIds, sort, diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/dockedSearchSnapshot.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/dockedSearchSnapshot.ts deleted file mode 100644 index 2d20030889..0000000000 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/dockedSearchSnapshot.ts +++ /dev/null @@ -1,14 +0,0 @@ -import type { SearchResult } from "./utils"; - -export const isDockedSnapshot = ({ - debouncedSearchTerm, - dockedQuery, - dockedResults, -}: { - debouncedSearchTerm: string; - dockedQuery?: string; - dockedResults?: SearchResult[]; -}): boolean => - dockedQuery !== undefined && - debouncedSearchTerm.trim() === dockedQuery.trim() && - !!dockedResults?.length; diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index d8b4541c32..0b1a0eaed3 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -8,10 +8,10 @@ import { import { searchIndexedNodes, sortSearchResults, + type ScoredSearchHit, type SearchResult, type SortConfig, } from "./utils"; -import { isDockedSnapshot } from "./dockedSearchSnapshot"; export type SearchIndex = { miniSearch: MiniSearch; @@ -47,7 +47,7 @@ const hitsToScoredSearchHits = ({ }: { hits: DiscourseSearchHit[]; resultsByUid: Map; -}) => +}): ScoredSearchHit[] => hits .map((hit) => { const indexedResult = resultsByUid.get(hit.uid); @@ -70,15 +70,7 @@ const hitsToScoredSearchHits = ({ source: hit.source, }; }) - .filter( - ( - hit, - ): hit is { - result: SearchResult; - score: number; - source: DiscourseSearchHit["source"]; - } => !!hit, - ); + .filter((hit): hit is ScoredSearchHit => !!hit); export const useAdvancedNodeSearchResults = ({ debouncedSearchTerm, @@ -90,28 +82,25 @@ export const useAdvancedNodeSearchResults = ({ dockedQuery, dockedResults, }: UseAdvancedNodeSearchResultsArgs): SearchResult[] => { - const frozenSnapshot = useMemo( + const isDockedQuery = useMemo( () => - isDockedSnapshot({ - debouncedSearchTerm, - dockedQuery, - dockedResults, - }), - [debouncedSearchTerm, dockedQuery, dockedResults], + dockedQuery !== undefined && + debouncedSearchTerm.trim() === dockedQuery.trim(), + [debouncedSearchTerm, dockedQuery], ); - const [liveResults, setLiveResults] = useState([]); + const [scoredHits, setScoredHits] = useState([]); useEffect(() => { - if (frozenSnapshot) return; + if (isDockedQuery) return; if (!debouncedSearchTerm) { - setLiveResults([]); + setScoredHits([]); return; } if (isIndexLoading || indexError || !searchIndex) { - setLiveResults([]); + setScoredHits([]); return; } @@ -145,20 +134,15 @@ export const useAdvancedNodeSearchResults = ({ }) .then((hits) => { if (cancelled) return; - - const scoredHits = hitsToScoredSearchHits({ hits, resultsByUid }); - setLiveResults(sortSearchResults({ hits: scoredHits, sort })); + setScoredHits(hitsToScoredSearchHits({ hits, resultsByUid })); }) .catch((error) => { console.error("Advanced node search failed:", error); if (cancelled) return; - setLiveResults( - sortSearchResults({ - hits: hitsToScoredSearchHits({ - hits: runKeywordSearch(), - resultsByUid, - }), - sort, + setScoredHits( + hitsToScoredSearchHits({ + hits: runKeywordSearch(), + resultsByUid, }), ); }); @@ -168,17 +152,21 @@ export const useAdvancedNodeSearchResults = ({ }; }, [ debouncedSearchTerm, - frozenSnapshot, indexError, + isDockedQuery, isIndexLoading, searchIndex, selectedNodeTypeIds, - sort, ]); - if (frozenSnapshot && dockedResults) { + const sortedLiveResults = useMemo( + () => sortSearchResults({ hits: scoredHits, sort }), + [scoredHits, sort], + ); + + if (isDockedQuery && dockedResults) { return dockedResults; } - return liveResults; + return sortedLiveResults; }; diff --git a/apps/roam/src/utils/__tests__/isDockedSnapshot.test.ts b/apps/roam/src/utils/__tests__/isDockedSnapshot.test.ts deleted file mode 100644 index 16b6995183..0000000000 --- a/apps/roam/src/utils/__tests__/isDockedSnapshot.test.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { isDockedSnapshot } from "~/components/AdvancedNodeSearchDialog/dockedSearchSnapshot"; -import type { SearchResult } from "~/components/AdvancedNodeSearchDialog/utils"; - -const sampleResult = (uid: string): SearchResult => ({ - uid, - title: `[[C]] - ${uid}`, - type: "claim", - nodeTypeLabel: "Claim", - excerpt: "", - createdAt: "", - lastModified: "", - authorName: "Unknown", -}); - -describe("isDockedSnapshot", () => { - it("returns true when the query matches and docked results exist", () => { - expect( - isDockedSnapshot({ - debouncedSearchTerm: "meaning", - dockedQuery: "meaning", - dockedResults: [sampleResult("a")], - }), - ).toBe(true); - }); - - it("returns false when the query differs", () => { - expect( - isDockedSnapshot({ - debouncedSearchTerm: "edited query", - dockedQuery: "meaning", - dockedResults: [sampleResult("a")], - }), - ).toBe(false); - }); - - it("returns false when docked results are empty", () => { - expect( - isDockedSnapshot({ - debouncedSearchTerm: "meaning", - dockedQuery: "meaning", - dockedResults: [], - }), - ).toBe(false); - }); -}); From aebb86264867fed67fc0cb844c60e6de3d4bad96 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 3 Jul 2026 21:15:33 -0400 Subject: [PATCH 05/16] Revert AGENTS.md changes from ENG-1734 branch. Co-authored-by: Cursor --- AGENTS.md | 5 ++--- apps/roam/AGENTS.md | 4 ---- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a29a1ef843..9562c116fd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,6 +81,5 @@ PR titles for Linear-backed work should follow this format: ### Testing -- Write unit tests for new functionality when the target app or package has established test infrastructure (a `test` script in its `package.json` and existing tests that run in CI for that package). -- Do not add unit tests to apps or packages without that infrastructure. For example, `apps/roam` does not run tests in CI, so do not generate Roam unit tests unless the user explicitly asks for them. -- Ensure tests are meaningful and maintainable when they are added. +- Write unit tests for new functionality +- Ensure tests are meaningful and maintainable diff --git a/apps/roam/AGENTS.md b/apps/roam/AGENTS.md index 7cd2500613..9888d3c4aa 100644 --- a/apps/roam/AGENTS.md +++ b/apps/roam/AGENTS.md @@ -17,7 +17,3 @@ When styling Roam UI, use this priority order: Any new Roam UI should feel native to Roam and consistent with existing BlueprintJS and repository usage, not like a separate visual palette. Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj. Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU. - -## Testing - -Do not add unit tests for Roam unless the user explicitly asks. Roam is not covered by CI test runs; validate Roam changes manually in the extension instead. From 1bf4f21a557775e665bf9b5a31bff9ef9b4344d5 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 3 Jul 2026 21:40:47 -0400 Subject: [PATCH 06/16] ENG-1734 Unify semantic and keyword search on ScoredSearchHit. Extract shared search hit types and remove DiscourseSearchHit adapter conversions in the Advanced Node Search hook. Co-authored-by: Cursor --- .../useAdvancedNodeSearchResults.ts | 74 +++--------------- .../AdvancedNodeSearchDialog/utils.ts | 28 +++---- .../roam/src/utils/advancedNodeSearchTypes.ts | 75 +++++++++++++++++++ .../src/utils/discourseNodeSemanticSearch.ts | 72 +++++++----------- 4 files changed, 123 insertions(+), 126 deletions(-) create mode 100644 apps/roam/src/utils/advancedNodeSearchTypes.ts diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index 0b1a0eaed3..211d90b3b3 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -1,10 +1,7 @@ import { useEffect, useMemo, useState } from "react"; import MiniSearch from "minisearch"; import getDiscourseNodes from "~/utils/getDiscourseNodes"; -import { - searchDiscourseNodesWithSemanticFallback, - type DiscourseSearchHit, -} from "~/utils/discourseNodeSemanticSearch"; +import { searchDiscourseNodesWithSemanticFallback } from "~/utils/discourseNodeSemanticSearch"; import { searchIndexedNodes, sortSearchResults, @@ -29,49 +26,6 @@ type UseAdvancedNodeSearchResultsArgs = { dockedResults?: SearchResult[]; }; -const toKeywordHits = ( - scoredHits: ReturnType, -): DiscourseSearchHit[] => - scoredHits.map((hit) => ({ - uid: hit.result.uid, - text: hit.result.title, - type: hit.result.type, - nodeTypeLabel: hit.result.nodeTypeLabel, - score: hit.score, - source: "keyword" as const, - })); - -const hitsToScoredSearchHits = ({ - hits, - resultsByUid, -}: { - hits: DiscourseSearchHit[]; - resultsByUid: Map; -}): ScoredSearchHit[] => - hits - .map((hit) => { - const indexedResult = resultsByUid.get(hit.uid); - if (indexedResult) { - return { result: indexedResult, score: hit.score, source: hit.source }; - } - - return { - result: { - uid: hit.uid, - title: hit.text, - type: hit.type || "", - nodeTypeLabel: hit.nodeTypeLabel || "", - excerpt: "", - createdAt: "", - lastModified: "", - authorName: "Unknown", - }, - score: hit.score, - source: hit.source, - }; - }) - .filter((hit): hit is ScoredSearchHit => !!hit); - export const useAdvancedNodeSearchResults = ({ debouncedSearchTerm, selectedNodeTypeIds, @@ -117,34 +71,28 @@ export const useAdvancedNodeSearchResults = ({ searchIndex.allResults.map((result) => [result.uid, result]), ); - const runKeywordSearch = (): DiscourseSearchHit[] => - toKeywordHits( - searchIndexedNodes({ - miniSearch: searchIndex.miniSearch, - allResults: searchIndex.allResults, - searchTerm: debouncedSearchTerm, - typeFilter, - }), - ); + const runKeywordSearch = (): ScoredSearchHit[] => + searchIndexedNodes({ + miniSearch: searchIndex.miniSearch, + allResults: searchIndex.allResults, + searchTerm: debouncedSearchTerm, + typeFilter, + }); void searchDiscourseNodesWithSemanticFallback({ nodeTypes: discourseNodes, query: debouncedSearchTerm, + resultsByUid, runKeywordSearch, }) .then((hits) => { if (cancelled) return; - setScoredHits(hitsToScoredSearchHits({ hits, resultsByUid })); + setScoredHits(hits); }) .catch((error) => { console.error("Advanced node search failed:", error); if (cancelled) return; - setScoredHits( - hitsToScoredSearchHits({ - hits: runKeywordSearch(), - resultsByUid, - }), - ); + setScoredHits(runKeywordSearch()); }); return () => { diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts index 4b08f17bed..abaa88ba81 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts @@ -9,6 +9,15 @@ import { getPulledDiscourseNodeUid, queryDiscourseNodesByFormat, } from "~/utils/discourseNodeSearch"; +import type { + ScoredSearchHit, + SearchResult, +} from "~/utils/advancedNodeSearchTypes"; + +export type { + ScoredSearchHit, + SearchResult, +} from "~/utils/advancedNodeSearchTypes"; export const DEBOUNCE_MS = 250; export const MAX_RESULTS = 50; @@ -36,17 +45,6 @@ export const SORT_FIELD_LABELS: Record = { author: "Author", }; -export type SearchResult = { - uid: string; - title: string; - type: string; - nodeTypeLabel: string; - excerpt: string; - createdAt: string; - lastModified: string; - authorName: string; -}; - export type DockedSearchState = { query: string; results: SearchResult[]; @@ -56,12 +54,6 @@ export type DockedSearchState = { dgSearchId?: string; }; -export type ScoredSearchHit = { - result: SearchResult; - score: number; - source?: "semantic" | "keyword"; -}; - type MiniSearchDocument = SearchResult & { id: string; }; @@ -303,7 +295,7 @@ export const searchIndexedNodes = ({ .map((result) => { const searchResult = resultsByUid.get(String(result.id)); if (!searchResult) return null; - return { result: searchResult, score: result.score }; + return { result: searchResult, score: result.score, source: "keyword" }; }) .filter((hit): hit is ScoredSearchHit => !!hit); }; diff --git a/apps/roam/src/utils/advancedNodeSearchTypes.ts b/apps/roam/src/utils/advancedNodeSearchTypes.ts new file mode 100644 index 0000000000..cc9601b39f --- /dev/null +++ b/apps/roam/src/utils/advancedNodeSearchTypes.ts @@ -0,0 +1,75 @@ +export type SearchHitSource = "semantic" | "keyword"; + +export type SearchResult = { + uid: string; + title: string; + type: string; + nodeTypeLabel: string; + excerpt: string; + createdAt: string; + lastModified: string; + authorName: string; +}; + +export type ScoredSearchHit = { + result: SearchResult; + score: number; + source: SearchHitSource; +}; + +export const combineScoredSearchHits = ({ + semantic, + keyword, +}: { + semantic: ScoredSearchHit[]; + keyword: ScoredSearchHit[]; +}): ScoredSearchHit[] => { + const seenUids = new Set(semantic.map((hit) => hit.result.uid)); + const combined = [...semantic]; + + keyword.forEach((hit) => { + if (seenUids.has(hit.result.uid)) return; + seenUids.add(hit.result.uid); + combined.push(hit); + }); + + return combined; +}; + +export const toScoredSearchHit = ({ + uid, + title, + type, + nodeTypeLabel, + score, + source, + resultsByUid, +}: { + uid: string; + title: string; + type?: string; + nodeTypeLabel?: string; + score: number; + source: SearchHitSource; + resultsByUid: Map; +}): ScoredSearchHit => { + const indexedResult = resultsByUid.get(uid); + if (indexedResult) { + return { result: indexedResult, score, source }; + } + + return { + result: { + uid, + title, + type: type || "", + nodeTypeLabel: nodeTypeLabel || "", + excerpt: "", + createdAt: "", + lastModified: "", + authorName: "Unknown", + }, + score, + source, + }; +}; diff --git a/apps/roam/src/utils/discourseNodeSemanticSearch.ts b/apps/roam/src/utils/discourseNodeSemanticSearch.ts index eb42252b3b..e947104b1d 100644 --- a/apps/roam/src/utils/discourseNodeSemanticSearch.ts +++ b/apps/roam/src/utils/discourseNodeSemanticSearch.ts @@ -1,62 +1,34 @@ import type { DiscourseNode } from "~/utils/getDiscourseNodes"; import { - runRoamSemanticSearch, - type AdminSearchResultItem, -} from "~/utils/discourseNodeSearchProviders"; + combineScoredSearchHits, + toScoredSearchHit, + type ScoredSearchHit, + type SearchResult, +} from "~/utils/advancedNodeSearchTypes"; +import { runRoamSemanticSearch } from "~/utils/discourseNodeSearchProviders"; export const SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS = 5; -export type DiscourseSearchHitSource = "semantic" | "keyword"; - -export type DiscourseSearchHit = { - uid: string; - text: string; - type?: string; - nodeTypeLabel?: string; - score: number; - source: DiscourseSearchHitSource; -}; +export type { + ScoredSearchHit, + SearchResult, + SearchHitSource, +} from "~/utils/advancedNodeSearchTypes"; export const shouldUseRoamSemanticSearch = (): boolean => window.roamAlphaAPI.data.semanticSearchEnabled(); -export const combineDiscourseSearchResults = ({ - semantic, - keyword, -}: { - semantic: DiscourseSearchHit[]; - keyword: DiscourseSearchHit[]; -}): DiscourseSearchHit[] => { - const seenUids = new Set(semantic.map((hit) => hit.uid)); - const combined = [...semantic]; - - keyword.forEach((hit) => { - if (seenUids.has(hit.uid)) return; - seenUids.add(hit.uid); - combined.push(hit); - }); - - return combined; -}; - -const toSemanticHit = (item: AdminSearchResultItem): DiscourseSearchHit => ({ - uid: item.uid, - text: item.text, - type: item.type, - nodeTypeLabel: item.nodeTypeLabel, - score: item.score ?? 0, - source: "semantic", -}); - export const searchDiscourseNodesWithSemanticFallback = async ({ nodeTypes, query, + resultsByUid, runKeywordSearch, }: { nodeTypes: DiscourseNode[]; query: string; - runKeywordSearch: () => DiscourseSearchHit[]; -}): Promise => { + resultsByUid: Map; + runKeywordSearch: () => ScoredSearchHit[]; +}): Promise => { const trimmedQuery = query.trim(); if (!trimmedQuery) return []; @@ -69,7 +41,17 @@ export const searchDiscourseNodesWithSemanticFallback = async ({ nodeTypes, query: trimmedQuery, }); - const semanticHits = providerResult.filteredResults.map(toSemanticHit); + const semanticHits = providerResult.filteredResults.map((item) => + toScoredSearchHit({ + uid: item.uid, + title: item.text, + type: item.type, + nodeTypeLabel: item.nodeTypeLabel, + score: item.score ?? 0, + source: "semantic", + resultsByUid, + }), + ); if ( providerResult.filteredResultCount >= @@ -78,7 +60,7 @@ export const searchDiscourseNodesWithSemanticFallback = async ({ return semanticHits; } - return combineDiscourseSearchResults({ + return combineScoredSearchHits({ semantic: semanticHits, keyword: runKeywordSearch(), }); From dccf23438e2b282f8c688f84e9f7add331b77cbe Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 3 Jul 2026 21:44:10 -0400 Subject: [PATCH 07/16] ENG-1734 Rename discourse node search modules and APIs for clarity. Use searchDiscourseNodes for the semantic-plus-miniSearch orchestrator, searchDiscourseNodesWithMiniSearch for MiniSearch, and ScoredSearchResult instead of hit-based names. Co-authored-by: Cursor --- .../useAdvancedNodeSearchResults.ts | 30 ++++++------- .../AdvancedNodeSearchDialog/utils.ts | 42 ++++++++++-------- ...chTypes.ts => discourseNodeSearchTypes.ts} | 36 ++++++++-------- ...anticSearch.ts => searchDiscourseNodes.ts} | 43 +++++++++---------- 4 files changed, 76 insertions(+), 75 deletions(-) rename apps/roam/src/utils/{advancedNodeSearchTypes.ts => discourseNodeSearchTypes.ts} (54%) rename apps/roam/src/utils/{discourseNodeSemanticSearch.ts => searchDiscourseNodes.ts} (54%) diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index 211d90b3b3..0db9d31037 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -1,11 +1,11 @@ import { useEffect, useMemo, useState } from "react"; import MiniSearch from "minisearch"; import getDiscourseNodes from "~/utils/getDiscourseNodes"; -import { searchDiscourseNodesWithSemanticFallback } from "~/utils/discourseNodeSemanticSearch"; +import { searchDiscourseNodes } from "~/utils/searchDiscourseNodes"; import { - searchIndexedNodes, + searchDiscourseNodesWithMiniSearch, sortSearchResults, - type ScoredSearchHit, + type ScoredSearchResult, type SearchResult, type SortConfig, } from "./utils"; @@ -43,18 +43,18 @@ export const useAdvancedNodeSearchResults = ({ [debouncedSearchTerm, dockedQuery], ); - const [scoredHits, setScoredHits] = useState([]); + const [scoredResults, setScoredResults] = useState([]); useEffect(() => { if (isDockedQuery) return; if (!debouncedSearchTerm) { - setScoredHits([]); + setScoredResults([]); return; } if (isIndexLoading || indexError || !searchIndex) { - setScoredHits([]); + setScoredResults([]); return; } @@ -71,28 +71,28 @@ export const useAdvancedNodeSearchResults = ({ searchIndex.allResults.map((result) => [result.uid, result]), ); - const runKeywordSearch = (): ScoredSearchHit[] => - searchIndexedNodes({ + const runMiniSearch = (): ScoredSearchResult[] => + searchDiscourseNodesWithMiniSearch({ miniSearch: searchIndex.miniSearch, allResults: searchIndex.allResults, searchTerm: debouncedSearchTerm, typeFilter, }); - void searchDiscourseNodesWithSemanticFallback({ + void searchDiscourseNodes({ nodeTypes: discourseNodes, query: debouncedSearchTerm, resultsByUid, - runKeywordSearch, + runMiniSearch, }) - .then((hits) => { + .then((results) => { if (cancelled) return; - setScoredHits(hits); + setScoredResults(results); }) .catch((error) => { console.error("Advanced node search failed:", error); if (cancelled) return; - setScoredHits(runKeywordSearch()); + setScoredResults(runMiniSearch()); }); return () => { @@ -108,8 +108,8 @@ export const useAdvancedNodeSearchResults = ({ ]); const sortedLiveResults = useMemo( - () => sortSearchResults({ hits: scoredHits, sort }), - [scoredHits, sort], + () => sortSearchResults({ scoredResults, sort }), + [scoredResults, sort], ); if (isDockedQuery && dockedResults) { diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts index abaa88ba81..2405eac843 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts @@ -10,14 +10,14 @@ import { queryDiscourseNodesByFormat, } from "~/utils/discourseNodeSearch"; import type { - ScoredSearchHit, + ScoredSearchResult, SearchResult, -} from "~/utils/advancedNodeSearchTypes"; +} from "~/utils/discourseNodeSearchTypes"; export type { - ScoredSearchHit, + ScoredSearchResult, SearchResult, -} from "~/utils/advancedNodeSearchTypes"; +} from "~/utils/discourseNodeSearchTypes"; export const DEBOUNCE_MS = 250; export const MAX_RESULTS = 50; @@ -207,33 +207,33 @@ export const isNonDefaultSort = (sort: SortConfig): boolean => sort.direction !== DEFAULT_SORT_CONFIG.direction; export const sortSearchResults = ({ - hits, + scoredResults, sort, }: { - hits: ScoredSearchHit[]; + scoredResults: ScoredSearchResult[]; sort: SortConfig; }): SearchResult[] => { - const sorted = [...hits]; + const sorted = [...scoredResults]; - sorted.sort((aHit, bHit) => { - const a = aHit.result; - const b = bHit.result; + sorted.sort((aEntry, bEntry) => { + const a = aEntry.result; + const b = bEntry.result; let comparison = 0; switch (sort.field) { case "relevance": - if (aHit.source && bHit.source && aHit.source !== bHit.source) { + if (aEntry.source !== bEntry.source) { comparison = sort.direction === "desc" - ? aHit.source === "semantic" + ? aEntry.source === "semantic" ? -1 : 1 - : aHit.source === "semantic" + : aEntry.source === "semantic" ? 1 : -1; break; } - comparison = compareNumbers(aHit.score, bHit.score, sort.direction); + comparison = compareNumbers(aEntry.score, bEntry.score, sort.direction); break; case "alphabetical": comparison = compareStrings( @@ -263,10 +263,10 @@ export const sortSearchResults = ({ return comparison || a.uid.localeCompare(b.uid); }); - return sorted.map((hit) => hit.result); + return sorted.map((entry) => entry.result); }; -export const searchIndexedNodes = ({ +export const searchDiscourseNodesWithMiniSearch = ({ miniSearch, allResults, searchTerm, @@ -276,7 +276,7 @@ export const searchIndexedNodes = ({ allResults: SearchResult[]; searchTerm: string; typeFilter?: string[]; -}): ScoredSearchHit[] => { +}): ScoredSearchResult[] => { const resultsByUid = new Map( allResults.map((result) => [result.uid, result]), ); @@ -295,7 +295,11 @@ export const searchIndexedNodes = ({ .map((result) => { const searchResult = resultsByUid.get(String(result.id)); if (!searchResult) return null; - return { result: searchResult, score: result.score, source: "keyword" }; + return { + result: searchResult, + score: result.score, + source: "miniSearch", + }; }) - .filter((hit): hit is ScoredSearchHit => !!hit); + .filter((entry): entry is ScoredSearchResult => !!entry); }; diff --git a/apps/roam/src/utils/advancedNodeSearchTypes.ts b/apps/roam/src/utils/discourseNodeSearchTypes.ts similarity index 54% rename from apps/roam/src/utils/advancedNodeSearchTypes.ts rename to apps/roam/src/utils/discourseNodeSearchTypes.ts index cc9601b39f..9d830f7263 100644 --- a/apps/roam/src/utils/advancedNodeSearchTypes.ts +++ b/apps/roam/src/utils/discourseNodeSearchTypes.ts @@ -1,4 +1,4 @@ -export type SearchHitSource = "semantic" | "keyword"; +export type DiscourseNodeSearchSource = "semantic" | "miniSearch"; export type SearchResult = { uid: string; @@ -11,38 +11,37 @@ export type SearchResult = { authorName: string; }; -export type ScoredSearchHit = { +export type ScoredSearchResult = { result: SearchResult; score: number; - source: SearchHitSource; + source: DiscourseNodeSearchSource; }; -export const combineScoredSearchHits = ({ +export const combineSemanticAndMiniSearchResults = ({ semantic, - keyword, + miniSearch, }: { - semantic: ScoredSearchHit[]; - keyword: ScoredSearchHit[]; -}): ScoredSearchHit[] => { - const seenUids = new Set(semantic.map((hit) => hit.result.uid)); + semantic: ScoredSearchResult[]; + miniSearch: ScoredSearchResult[]; +}): ScoredSearchResult[] => { + const seenUids = new Set(semantic.map((entry) => entry.result.uid)); const combined = [...semantic]; - keyword.forEach((hit) => { - if (seenUids.has(hit.result.uid)) return; - seenUids.add(hit.result.uid); - combined.push(hit); + miniSearch.forEach((entry) => { + if (seenUids.has(entry.result.uid)) return; + seenUids.add(entry.result.uid); + combined.push(entry); }); return combined; }; -export const toScoredSearchHit = ({ +export const toScoredSearchResultFromSemantic = ({ uid, title, type, nodeTypeLabel, score, - source, resultsByUid, }: { uid: string; @@ -50,12 +49,11 @@ export const toScoredSearchHit = ({ type?: string; nodeTypeLabel?: string; score: number; - source: SearchHitSource; resultsByUid: Map; -}): ScoredSearchHit => { +}): ScoredSearchResult => { const indexedResult = resultsByUid.get(uid); if (indexedResult) { - return { result: indexedResult, score, source }; + return { result: indexedResult, score, source: "semantic" }; } return { @@ -70,6 +68,6 @@ export const toScoredSearchHit = ({ authorName: "Unknown", }, score, - source, + source: "semantic", }; }; diff --git a/apps/roam/src/utils/discourseNodeSemanticSearch.ts b/apps/roam/src/utils/searchDiscourseNodes.ts similarity index 54% rename from apps/roam/src/utils/discourseNodeSemanticSearch.ts rename to apps/roam/src/utils/searchDiscourseNodes.ts index e947104b1d..d9ceacf469 100644 --- a/apps/roam/src/utils/discourseNodeSemanticSearch.ts +++ b/apps/roam/src/utils/searchDiscourseNodes.ts @@ -1,39 +1,39 @@ import type { DiscourseNode } from "~/utils/getDiscourseNodes"; import { - combineScoredSearchHits, - toScoredSearchHit, - type ScoredSearchHit, + combineSemanticAndMiniSearchResults, + toScoredSearchResultFromSemantic, + type ScoredSearchResult, type SearchResult, -} from "~/utils/advancedNodeSearchTypes"; +} from "~/utils/discourseNodeSearchTypes"; import { runRoamSemanticSearch } from "~/utils/discourseNodeSearchProviders"; export const SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS = 5; export type { - ScoredSearchHit, + DiscourseNodeSearchSource, + ScoredSearchResult, SearchResult, - SearchHitSource, -} from "~/utils/advancedNodeSearchTypes"; +} from "~/utils/discourseNodeSearchTypes"; -export const shouldUseRoamSemanticSearch = (): boolean => +export const isRoamSemanticSearchEnabled = (): boolean => window.roamAlphaAPI.data.semanticSearchEnabled(); -export const searchDiscourseNodesWithSemanticFallback = async ({ +export const searchDiscourseNodes = async ({ nodeTypes, query, resultsByUid, - runKeywordSearch, + runMiniSearch, }: { nodeTypes: DiscourseNode[]; query: string; resultsByUid: Map; - runKeywordSearch: () => ScoredSearchHit[]; -}): Promise => { + runMiniSearch: () => ScoredSearchResult[]; +}): Promise => { const trimmedQuery = query.trim(); if (!trimmedQuery) return []; - if (!shouldUseRoamSemanticSearch()) { - return runKeywordSearch(); + if (!isRoamSemanticSearchEnabled()) { + return runMiniSearch(); } try { @@ -41,14 +41,13 @@ export const searchDiscourseNodesWithSemanticFallback = async ({ nodeTypes, query: trimmedQuery, }); - const semanticHits = providerResult.filteredResults.map((item) => - toScoredSearchHit({ + const semanticResults = providerResult.filteredResults.map((item) => + toScoredSearchResultFromSemantic({ uid: item.uid, title: item.text, type: item.type, nodeTypeLabel: item.nodeTypeLabel, score: item.score ?? 0, - source: "semantic", resultsByUid, }), ); @@ -57,14 +56,14 @@ export const searchDiscourseNodesWithSemanticFallback = async ({ providerResult.filteredResultCount >= SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS ) { - return semanticHits; + return semanticResults; } - return combineScoredSearchHits({ - semantic: semanticHits, - keyword: runKeywordSearch(), + return combineSemanticAndMiniSearchResults({ + semantic: semanticResults, + miniSearch: runMiniSearch(), }); } catch { - return runKeywordSearch(); + return runMiniSearch(); } }; From 053d5da27800133057aea82e2ed62bf48f6652a0 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 3 Jul 2026 21:49:09 -0400 Subject: [PATCH 08/16] ENG-1734 Clarify unsorted vs sorted search result state in hook. Sort runs in useMemo only; the search effect does not depend on sort config. Co-authored-by: Cursor --- .../useAdvancedNodeSearchResults.ts | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index 0db9d31037..dbf8e8df08 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -43,18 +43,20 @@ export const useAdvancedNodeSearchResults = ({ [debouncedSearchTerm, dockedQuery], ); - const [scoredResults, setScoredResults] = useState([]); + const [unsortedScoredResults, setUnsortedScoredResults] = useState< + ScoredSearchResult[] + >([]); useEffect(() => { if (isDockedQuery) return; if (!debouncedSearchTerm) { - setScoredResults([]); + setUnsortedScoredResults([]); return; } if (isIndexLoading || indexError || !searchIndex) { - setScoredResults([]); + setUnsortedScoredResults([]); return; } @@ -87,12 +89,12 @@ export const useAdvancedNodeSearchResults = ({ }) .then((results) => { if (cancelled) return; - setScoredResults(results); + setUnsortedScoredResults(results); }) .catch((error) => { console.error("Advanced node search failed:", error); if (cancelled) return; - setScoredResults(runMiniSearch()); + setUnsortedScoredResults(runMiniSearch()); }); return () => { @@ -107,14 +109,14 @@ export const useAdvancedNodeSearchResults = ({ selectedNodeTypeIds, ]); - const sortedLiveResults = useMemo( - () => sortSearchResults({ scoredResults, sort }), - [scoredResults, sort], + const results = useMemo( + () => sortSearchResults({ scoredResults: unsortedScoredResults, sort }), + [unsortedScoredResults, sort], ); if (isDockedQuery && dockedResults) { return dockedResults; } - return sortedLiveResults; + return results; }; From 2339198294ce2b673b8b45ba408dae2f67f37f2f Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 3 Jul 2026 22:06:43 -0400 Subject: [PATCH 09/16] ENG-1734 Add temporary debug logs for semantic vs miniSearch results. Co-authored-by: Cursor --- .../useAdvancedNodeSearchResults.ts | 13 +++- apps/roam/src/utils/searchDiscourseNodes.ts | 68 +++++++++++++++++-- 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index dbf8e8df08..6ee537b5f0 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -1,7 +1,10 @@ import { useEffect, useMemo, useState } from "react"; import MiniSearch from "minisearch"; import getDiscourseNodes from "~/utils/getDiscourseNodes"; -import { searchDiscourseNodes } from "~/utils/searchDiscourseNodes"; +import { + logDiscourseNodeSearchResults, + searchDiscourseNodes, +} from "~/utils/searchDiscourseNodes"; import { searchDiscourseNodesWithMiniSearch, sortSearchResults, @@ -94,7 +97,13 @@ export const useAdvancedNodeSearchResults = ({ .catch((error) => { console.error("Advanced node search failed:", error); if (cancelled) return; - setUnsortedScoredResults(runMiniSearch()); + const results = runMiniSearch(); + logDiscourseNodeSearchResults({ + path: "miniSearch only (hook error)", + query: debouncedSearchTerm, + results, + }); + setUnsortedScoredResults(results); }); return () => { diff --git a/apps/roam/src/utils/searchDiscourseNodes.ts b/apps/roam/src/utils/searchDiscourseNodes.ts index d9ceacf469..720bb0db19 100644 --- a/apps/roam/src/utils/searchDiscourseNodes.ts +++ b/apps/roam/src/utils/searchDiscourseNodes.ts @@ -18,6 +18,42 @@ export type { export const isRoamSemanticSearchEnabled = (): boolean => window.roamAlphaAPI.data.semanticSearchEnabled(); +/** Testing only — remove or set false before merge. */ +export const DEBUG_DISCOURSE_NODE_SEARCH = true; + +export const logDiscourseNodeSearchResults = ({ + path, + query, + results, +}: { + path: string; + query: string; + results: ScoredSearchResult[]; +}): void => { + if (!DEBUG_DISCOURSE_NODE_SEARCH) return; + + const semanticCount = results.filter( + (entry) => entry.source === "semantic", + ).length; + const miniSearchCount = results.filter( + (entry) => entry.source === "miniSearch", + ).length; + + console.group(`[DG Advanced Node Search] ${path} — "${query}"`); + console.log( + `semantic: ${semanticCount}, miniSearch: ${miniSearchCount}, total: ${results.length}`, + ); + console.table( + results.map((entry) => ({ + source: entry.source, + score: Number(entry.score.toFixed(3)), + title: entry.result.title, + uid: entry.result.uid, + })), + ); + console.groupEnd(); +}; + export const searchDiscourseNodes = async ({ nodeTypes, query, @@ -33,7 +69,13 @@ export const searchDiscourseNodes = async ({ if (!trimmedQuery) return []; if (!isRoamSemanticSearchEnabled()) { - return runMiniSearch(); + const results = runMiniSearch(); + logDiscourseNodeSearchResults({ + path: "miniSearch only (semantic disabled)", + query: trimmedQuery, + results, + }); + return results; } try { @@ -56,14 +98,32 @@ export const searchDiscourseNodes = async ({ providerResult.filteredResultCount >= SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS ) { + logDiscourseNodeSearchResults({ + path: "semantic only", + query: trimmedQuery, + results: semanticResults, + }); return semanticResults; } - return combineSemanticAndMiniSearchResults({ + const results = combineSemanticAndMiniSearchResults({ semantic: semanticResults, miniSearch: runMiniSearch(), }); - } catch { - return runMiniSearch(); + logDiscourseNodeSearchResults({ + path: `semantic + miniSearch fallback (< ${SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS} semantic)`, + query: trimmedQuery, + results, + }); + return results; + } catch (error) { + console.warn("[DG Advanced Node Search] semantic API failed", error); + const results = runMiniSearch(); + logDiscourseNodeSearchResults({ + path: "miniSearch only (semantic API error)", + query: trimmedQuery, + results, + }); + return results; } }; From 7acc534d9f5b5e301dd73b9e80bcf5e5133b7fff Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Fri, 3 Jul 2026 22:37:04 -0400 Subject: [PATCH 10/16] ENG-1734 Remove debug logs and address review feedback. Drop temporary search logging, clear stale results when docking or starting a new search, and keep semantic hits first when sorting by relevance. Co-authored-by: Cursor --- .../useAdvancedNodeSearchResults.ts | 22 +++--- .../AdvancedNodeSearchDialog/utils.ts | 9 +-- apps/roam/src/utils/searchDiscourseNodes.ts | 68 ++----------------- 3 files changed, 13 insertions(+), 86 deletions(-) diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index 6ee537b5f0..f3e5f746f3 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -1,10 +1,7 @@ import { useEffect, useMemo, useState } from "react"; import MiniSearch from "minisearch"; import getDiscourseNodes from "~/utils/getDiscourseNodes"; -import { - logDiscourseNodeSearchResults, - searchDiscourseNodes, -} from "~/utils/searchDiscourseNodes"; +import { searchDiscourseNodes } from "~/utils/searchDiscourseNodes"; import { searchDiscourseNodesWithMiniSearch, sortSearchResults, @@ -51,7 +48,10 @@ export const useAdvancedNodeSearchResults = ({ >([]); useEffect(() => { - if (isDockedQuery) return; + if (isDockedQuery) { + setUnsortedScoredResults([]); + return; + } if (!debouncedSearchTerm) { setUnsortedScoredResults([]); @@ -63,6 +63,7 @@ export const useAdvancedNodeSearchResults = ({ return; } + setUnsortedScoredResults([]); let cancelled = false; const typeFilter = selectedNodeTypeIds.length ? selectedNodeTypeIds @@ -94,16 +95,9 @@ export const useAdvancedNodeSearchResults = ({ if (cancelled) return; setUnsortedScoredResults(results); }) - .catch((error) => { - console.error("Advanced node search failed:", error); + .catch(() => { if (cancelled) return; - const results = runMiniSearch(); - logDiscourseNodeSearchResults({ - path: "miniSearch only (hook error)", - query: debouncedSearchTerm, - results, - }); - setUnsortedScoredResults(results); + setUnsortedScoredResults(runMiniSearch()); }); return () => { diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts index 2405eac843..9c75a6016a 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts @@ -223,14 +223,7 @@ export const sortSearchResults = ({ switch (sort.field) { case "relevance": if (aEntry.source !== bEntry.source) { - comparison = - sort.direction === "desc" - ? aEntry.source === "semantic" - ? -1 - : 1 - : aEntry.source === "semantic" - ? 1 - : -1; + comparison = aEntry.source === "semantic" ? -1 : 1; break; } comparison = compareNumbers(aEntry.score, bEntry.score, sort.direction); diff --git a/apps/roam/src/utils/searchDiscourseNodes.ts b/apps/roam/src/utils/searchDiscourseNodes.ts index 720bb0db19..d9ceacf469 100644 --- a/apps/roam/src/utils/searchDiscourseNodes.ts +++ b/apps/roam/src/utils/searchDiscourseNodes.ts @@ -18,42 +18,6 @@ export type { export const isRoamSemanticSearchEnabled = (): boolean => window.roamAlphaAPI.data.semanticSearchEnabled(); -/** Testing only — remove or set false before merge. */ -export const DEBUG_DISCOURSE_NODE_SEARCH = true; - -export const logDiscourseNodeSearchResults = ({ - path, - query, - results, -}: { - path: string; - query: string; - results: ScoredSearchResult[]; -}): void => { - if (!DEBUG_DISCOURSE_NODE_SEARCH) return; - - const semanticCount = results.filter( - (entry) => entry.source === "semantic", - ).length; - const miniSearchCount = results.filter( - (entry) => entry.source === "miniSearch", - ).length; - - console.group(`[DG Advanced Node Search] ${path} — "${query}"`); - console.log( - `semantic: ${semanticCount}, miniSearch: ${miniSearchCount}, total: ${results.length}`, - ); - console.table( - results.map((entry) => ({ - source: entry.source, - score: Number(entry.score.toFixed(3)), - title: entry.result.title, - uid: entry.result.uid, - })), - ); - console.groupEnd(); -}; - export const searchDiscourseNodes = async ({ nodeTypes, query, @@ -69,13 +33,7 @@ export const searchDiscourseNodes = async ({ if (!trimmedQuery) return []; if (!isRoamSemanticSearchEnabled()) { - const results = runMiniSearch(); - logDiscourseNodeSearchResults({ - path: "miniSearch only (semantic disabled)", - query: trimmedQuery, - results, - }); - return results; + return runMiniSearch(); } try { @@ -98,32 +56,14 @@ export const searchDiscourseNodes = async ({ providerResult.filteredResultCount >= SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS ) { - logDiscourseNodeSearchResults({ - path: "semantic only", - query: trimmedQuery, - results: semanticResults, - }); return semanticResults; } - const results = combineSemanticAndMiniSearchResults({ + return combineSemanticAndMiniSearchResults({ semantic: semanticResults, miniSearch: runMiniSearch(), }); - logDiscourseNodeSearchResults({ - path: `semantic + miniSearch fallback (< ${SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS} semantic)`, - query: trimmedQuery, - results, - }); - return results; - } catch (error) { - console.warn("[DG Advanced Node Search] semantic API failed", error); - const results = runMiniSearch(); - logDiscourseNodeSearchResults({ - path: "miniSearch only (semantic API error)", - query: trimmedQuery, - results, - }); - return results; + } catch { + return runMiniSearch(); } }; From ff8cd08e9c7d858bf231c5225c8770ddf007a139 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 21 Sep 2026 11:24:45 -0400 Subject: [PATCH 11/16] ENG-1734 Preserve Roam's semantic result order in relevance sort Roam's semantic scores are not comparable across hits, so sorting by them reordered results away from what Roam returned. Record each provider's returned position as `rank` and sort semantic entries by it; MiniSearch entries still sort by score and stay below semantic ones. Also dedupe semantic uids when combining the two sources, and degrade to an empty list when the MiniSearch fallback itself throws. Co-Authored-By: Claude Opus 5 --- .../AdvancedNodeSearchDialog/utils.ts | 20 ++-- .../discourseNodeSearchTypes.test.ts | 100 ++++++++++++++++++ .../src/utils/discourseNodeSearchTypes.ts | 31 +++++- apps/roam/src/utils/searchDiscourseNodes.ts | 19 +++- 4 files changed, 153 insertions(+), 17 deletions(-) create mode 100644 apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts index 9c75a6016a..097e8ec045 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts @@ -9,9 +9,10 @@ import { getPulledDiscourseNodeUid, queryDiscourseNodesByFormat, } from "~/utils/discourseNodeSearch"; -import type { - ScoredSearchResult, - SearchResult, +import { + compareByRelevance, + type ScoredSearchResult, + type SearchResult, } from "~/utils/discourseNodeSearchTypes"; export type { @@ -222,11 +223,11 @@ export const sortSearchResults = ({ switch (sort.field) { case "relevance": - if (aEntry.source !== bEntry.source) { - comparison = aEntry.source === "semantic" ? -1 : 1; - break; - } - comparison = compareNumbers(aEntry.score, bEntry.score, sort.direction); + comparison = compareByRelevance({ + a: aEntry, + b: bEntry, + descending: sort.direction === "desc", + }); break; case "alphabetical": comparison = compareStrings( @@ -285,12 +286,13 @@ export const searchDiscourseNodesWithMiniSearch = ({ }) .filter((result) => result.score > DISCOURSE_NODE_MIN_SEARCH_SCORE) .slice(0, MAX_RESULTS) - .map((result) => { + .map((result, index) => { const searchResult = resultsByUid.get(String(result.id)); if (!searchResult) return null; return { result: searchResult, score: result.score, + rank: index, source: "miniSearch", }; }) diff --git a/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts b/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts new file mode 100644 index 0000000000..292edbb1da --- /dev/null +++ b/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from "vitest"; +import { + combineSemanticAndMiniSearchResults, + compareByRelevance, + type DiscourseNodeSearchSource, + type ScoredSearchResult, +} from "~/utils/discourseNodeSearchTypes"; + +const makeEntry = ({ + uid, + score, + rank, + source, +}: { + uid: string; + score: number; + rank: number; + source: DiscourseNodeSearchSource; +}): ScoredSearchResult => ({ + result: { + uid, + title: `[[CLM]] - ${uid}`, + type: "clm", + nodeTypeLabel: "Claim", + excerpt: "", + createdAt: "", + lastModified: "", + authorName: "", + }, + score, + rank, + source, +}); + +const sortByRelevance = ( + entries: ScoredSearchResult[], + descending = true, +): string[] => + [...entries] + .sort((a, b) => compareByRelevance({ a, b, descending })) + .map((entry) => entry.result.uid); + +describe("combineSemanticAndMiniSearchResults", () => { + it("drops duplicate uids from both sources", () => { + const combined = combineSemanticAndMiniSearchResults({ + semantic: [ + makeEntry({ uid: "a", score: 0.9, rank: 0, source: "semantic" }), + makeEntry({ uid: "a", score: 0.7, rank: 1, source: "semantic" }), + makeEntry({ uid: "b", score: 0.5, rank: 2, source: "semantic" }), + ], + miniSearch: [ + makeEntry({ uid: "b", score: 10, rank: 0, source: "miniSearch" }), + makeEntry({ uid: "c", score: 8, rank: 1, source: "miniSearch" }), + ], + }); + + expect(combined.map((entry) => entry.result.uid)).toEqual(["a", "b", "c"]); + expect(combined[0]?.score).toBe(0.9); + }); +}); + +describe("compareByRelevance", () => { + it("keeps Roam's returned order for semantic results regardless of score", () => { + const entries = [ + makeEntry({ uid: "second", score: 0.91, rank: 1, source: "semantic" }), + makeEntry({ uid: "first", score: 0.12, rank: 0, source: "semantic" }), + makeEntry({ uid: "third", score: 0.55, rank: 2, source: "semantic" }), + ]; + + expect(sortByRelevance(entries)).toEqual(["first", "second", "third"]); + }); + + it("sorts MiniSearch results by score", () => { + const entries = [ + makeEntry({ uid: "low", score: 2, rank: 1, source: "miniSearch" }), + makeEntry({ uid: "high", score: 9, rank: 0, source: "miniSearch" }), + ]; + + expect(sortByRelevance(entries)).toEqual(["high", "low"]); + }); + + it("puts semantic results ahead of MiniSearch results in both directions", () => { + const entries = [ + makeEntry({ uid: "mini", score: 99, rank: 0, source: "miniSearch" }), + makeEntry({ uid: "semantic", score: 0.01, rank: 0, source: "semantic" }), + ]; + + expect(sortByRelevance(entries)).toEqual(["semantic", "mini"]); + expect(sortByRelevance(entries, false)).toEqual(["semantic", "mini"]); + }); + + it("reverses within-source order when sorting ascending", () => { + const entries = [ + makeEntry({ uid: "first", score: 0.1, rank: 0, source: "semantic" }), + makeEntry({ uid: "second", score: 0.9, rank: 1, source: "semantic" }), + ]; + + expect(sortByRelevance(entries, false)).toEqual(["second", "first"]); + }); +}); diff --git a/apps/roam/src/utils/discourseNodeSearchTypes.ts b/apps/roam/src/utils/discourseNodeSearchTypes.ts index 9d830f7263..0151c7ba68 100644 --- a/apps/roam/src/utils/discourseNodeSearchTypes.ts +++ b/apps/roam/src/utils/discourseNodeSearchTypes.ts @@ -15,6 +15,8 @@ export type ScoredSearchResult = { result: SearchResult; score: number; source: DiscourseNodeSearchSource; + // Position in the provider's own result list, used to preserve its ordering. + rank: number; }; export const combineSemanticAndMiniSearchResults = ({ @@ -24,10 +26,10 @@ export const combineSemanticAndMiniSearchResults = ({ semantic: ScoredSearchResult[]; miniSearch: ScoredSearchResult[]; }): ScoredSearchResult[] => { - const seenUids = new Set(semantic.map((entry) => entry.result.uid)); - const combined = [...semantic]; + const seenUids = new Set(); + const combined: ScoredSearchResult[] = []; - miniSearch.forEach((entry) => { + [...semantic, ...miniSearch].forEach((entry) => { if (seenUids.has(entry.result.uid)) return; seenUids.add(entry.result.uid); combined.push(entry); @@ -42,6 +44,7 @@ export const toScoredSearchResultFromSemantic = ({ type, nodeTypeLabel, score, + rank, resultsByUid, }: { uid: string; @@ -49,11 +52,12 @@ export const toScoredSearchResultFromSemantic = ({ type?: string; nodeTypeLabel?: string; score: number; + rank: number; resultsByUid: Map; }): ScoredSearchResult => { const indexedResult = resultsByUid.get(uid); if (indexedResult) { - return { result: indexedResult, score, source: "semantic" }; + return { result: indexedResult, score, rank, source: "semantic" }; } return { @@ -68,6 +72,25 @@ export const toScoredSearchResultFromSemantic = ({ authorName: "Unknown", }, score, + rank, source: "semantic", }; }; + +// Roam's semantic scores are not comparable across hits, so semantic entries keep +// the order Roam returned them in; MiniSearch entries still sort by score. +export const compareByRelevance = ({ + a, + b, + descending, +}: { + a: ScoredSearchResult; + b: ScoredSearchResult; + descending: boolean; +}): number => { + if (a.source !== b.source) return a.source === "semantic" ? -1 : 1; + + const comparison = + a.source === "semantic" ? a.rank - b.rank : b.score - a.score; + return descending ? comparison : -comparison; +}; diff --git a/apps/roam/src/utils/searchDiscourseNodes.ts b/apps/roam/src/utils/searchDiscourseNodes.ts index d9ceacf469..8cc5283f4c 100644 --- a/apps/roam/src/utils/searchDiscourseNodes.ts +++ b/apps/roam/src/utils/searchDiscourseNodes.ts @@ -18,6 +18,16 @@ export type { export const isRoamSemanticSearchEnabled = (): boolean => window.roamAlphaAPI.data.semanticSearchEnabled(); +const runMiniSearchSafely = ( + runMiniSearch: () => ScoredSearchResult[], +): ScoredSearchResult[] => { + try { + return runMiniSearch(); + } catch { + return []; + } +}; + export const searchDiscourseNodes = async ({ nodeTypes, query, @@ -33,7 +43,7 @@ export const searchDiscourseNodes = async ({ if (!trimmedQuery) return []; if (!isRoamSemanticSearchEnabled()) { - return runMiniSearch(); + return runMiniSearchSafely(runMiniSearch); } try { @@ -41,13 +51,14 @@ export const searchDiscourseNodes = async ({ nodeTypes, query: trimmedQuery, }); - const semanticResults = providerResult.filteredResults.map((item) => + const semanticResults = providerResult.filteredResults.map((item, index) => toScoredSearchResultFromSemantic({ uid: item.uid, title: item.text, type: item.type, nodeTypeLabel: item.nodeTypeLabel, score: item.score ?? 0, + rank: index, resultsByUid, }), ); @@ -61,9 +72,9 @@ export const searchDiscourseNodes = async ({ return combineSemanticAndMiniSearchResults({ semantic: semanticResults, - miniSearch: runMiniSearch(), + miniSearch: runMiniSearchSafely(runMiniSearch), }); } catch { - return runMiniSearch(); + return runMiniSearchSafely(runMiniSearch); } }; From 49ef8af804871939da8da1cfa9e026c8e0d18cb4 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 21 Sep 2026 11:31:39 -0400 Subject: [PATCH 12/16] ENG-1734 Keep provider order for relevance sort instead of re-sorting Both providers already return results in relevance order, and combining them appends MiniSearch after semantic, so relevance sorting only has to reverse for ascending. Sorting by score re-derived that order from Roam scores that are not comparable across hits, and the uid tie-break broke ties in the process. Co-Authored-By: Claude Opus 5 --- .../AdvancedNodeSearchDialog/utils.ts | 25 +++--- .../discourseNodeSearchTypes.test.ts | 84 ++++++++++--------- .../src/utils/discourseNodeSearchTypes.ts | 25 +----- apps/roam/src/utils/searchDiscourseNodes.ts | 3 +- 4 files changed, 60 insertions(+), 77 deletions(-) diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts index 097e8ec045..2b21a1bee6 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts @@ -9,10 +9,9 @@ import { getPulledDiscourseNodeUid, queryDiscourseNodesByFormat, } from "~/utils/discourseNodeSearch"; -import { - compareByRelevance, - type ScoredSearchResult, - type SearchResult, +import type { + ScoredSearchResult, + SearchResult, } from "~/utils/discourseNodeSearchTypes"; export type { @@ -214,6 +213,14 @@ export const sortSearchResults = ({ scoredResults: ScoredSearchResult[]; sort: SortConfig; }): SearchResult[] => { + // Semantic and MiniSearch results already arrive in their provider's relevance + // order, so relevance sorting only has to reverse it for ascending. + if (sort.field === "relevance") { + const ordered = + sort.direction === "asc" ? [...scoredResults].reverse() : scoredResults; + return ordered.map((entry) => entry.result); + } + const sorted = [...scoredResults]; sorted.sort((aEntry, bEntry) => { @@ -222,13 +229,6 @@ export const sortSearchResults = ({ let comparison = 0; switch (sort.field) { - case "relevance": - comparison = compareByRelevance({ - a: aEntry, - b: bEntry, - descending: sort.direction === "desc", - }); - break; case "alphabetical": comparison = compareStrings( getSortableTitle(a), @@ -286,13 +286,12 @@ export const searchDiscourseNodesWithMiniSearch = ({ }) .filter((result) => result.score > DISCOURSE_NODE_MIN_SEARCH_SCORE) .slice(0, MAX_RESULTS) - .map((result, index) => { + .map((result) => { const searchResult = resultsByUid.get(String(result.id)); if (!searchResult) return null; return { result: searchResult, score: result.score, - rank: index, source: "miniSearch", }; }) diff --git a/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts b/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts index 292edbb1da..2bc056ee76 100644 --- a/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts +++ b/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; +import { sortSearchResults } from "~/components/AdvancedNodeSearchDialog/utils"; import { combineSemanticAndMiniSearchResults, - compareByRelevance, type DiscourseNodeSearchSource, type ScoredSearchResult, } from "~/utils/discourseNodeSearchTypes"; @@ -9,12 +9,10 @@ import { const makeEntry = ({ uid, score, - rank, source, }: { uid: string; score: number; - rank: number; source: DiscourseNodeSearchSource; }): ScoredSearchResult => ({ result: { @@ -28,29 +26,29 @@ const makeEntry = ({ authorName: "", }, score, - rank, source, }); -const sortByRelevance = ( - entries: ScoredSearchResult[], - descending = true, +const relevanceOrder = ( + scoredResults: ScoredSearchResult[], + direction: "asc" | "desc" = "desc", ): string[] => - [...entries] - .sort((a, b) => compareByRelevance({ a, b, descending })) - .map((entry) => entry.result.uid); + sortSearchResults({ + scoredResults, + sort: { field: "relevance", direction }, + }).map((result) => result.uid); describe("combineSemanticAndMiniSearchResults", () => { it("drops duplicate uids from both sources", () => { const combined = combineSemanticAndMiniSearchResults({ semantic: [ - makeEntry({ uid: "a", score: 0.9, rank: 0, source: "semantic" }), - makeEntry({ uid: "a", score: 0.7, rank: 1, source: "semantic" }), - makeEntry({ uid: "b", score: 0.5, rank: 2, source: "semantic" }), + makeEntry({ uid: "a", score: 0.9, source: "semantic" }), + makeEntry({ uid: "a", score: 0.7, source: "semantic" }), + makeEntry({ uid: "b", score: 0.5, source: "semantic" }), ], miniSearch: [ - makeEntry({ uid: "b", score: 10, rank: 0, source: "miniSearch" }), - makeEntry({ uid: "c", score: 8, rank: 1, source: "miniSearch" }), + makeEntry({ uid: "b", score: 10, source: "miniSearch" }), + makeEntry({ uid: "c", score: 8, source: "miniSearch" }), ], }); @@ -59,42 +57,52 @@ describe("combineSemanticAndMiniSearchResults", () => { }); }); -describe("compareByRelevance", () => { - it("keeps Roam's returned order for semantic results regardless of score", () => { - const entries = [ - makeEntry({ uid: "second", score: 0.91, rank: 1, source: "semantic" }), - makeEntry({ uid: "first", score: 0.12, rank: 0, source: "semantic" }), - makeEntry({ uid: "third", score: 0.55, rank: 2, source: "semantic" }), +describe("sortSearchResults relevance", () => { + it("preserves the order the providers returned, ignoring score", () => { + const scoredResults = [ + makeEntry({ uid: "roamFirst", score: 0.12, source: "semantic" }), + makeEntry({ uid: "roamSecond", score: 0.91, source: "semantic" }), + makeEntry({ uid: "miniFirst", score: 9, source: "miniSearch" }), + makeEntry({ uid: "miniSecond", score: 2, source: "miniSearch" }), ]; - expect(sortByRelevance(entries)).toEqual(["first", "second", "third"]); + expect(relevanceOrder(scoredResults)).toEqual([ + "roamFirst", + "roamSecond", + "miniFirst", + "miniSecond", + ]); }); - it("sorts MiniSearch results by score", () => { - const entries = [ - makeEntry({ uid: "low", score: 2, rank: 1, source: "miniSearch" }), - makeEntry({ uid: "high", score: 9, rank: 0, source: "miniSearch" }), + it("does not reorder tied scores by uid", () => { + const scoredResults = [ + makeEntry({ uid: "zzz", score: 0, source: "semantic" }), + makeEntry({ uid: "aaa", score: 0, source: "semantic" }), ]; - expect(sortByRelevance(entries)).toEqual(["high", "low"]); + expect(relevanceOrder(scoredResults)).toEqual(["zzz", "aaa"]); }); - it("puts semantic results ahead of MiniSearch results in both directions", () => { - const entries = [ - makeEntry({ uid: "mini", score: 99, rank: 0, source: "miniSearch" }), - makeEntry({ uid: "semantic", score: 0.01, rank: 0, source: "semantic" }), + it("reverses the provider order when sorting ascending", () => { + const scoredResults = [ + makeEntry({ uid: "roamFirst", score: 0.12, source: "semantic" }), + makeEntry({ uid: "miniFirst", score: 9, source: "miniSearch" }), ]; - expect(sortByRelevance(entries)).toEqual(["semantic", "mini"]); - expect(sortByRelevance(entries, false)).toEqual(["semantic", "mini"]); + expect(relevanceOrder(scoredResults, "asc")).toEqual([ + "miniFirst", + "roamFirst", + ]); }); - it("reverses within-source order when sorting ascending", () => { - const entries = [ - makeEntry({ uid: "first", score: 0.1, rank: 0, source: "semantic" }), - makeEntry({ uid: "second", score: 0.9, rank: 1, source: "semantic" }), + it("does not mutate the input array", () => { + const scoredResults = [ + makeEntry({ uid: "a", score: 1, source: "semantic" }), + makeEntry({ uid: "b", score: 2, source: "miniSearch" }), ]; - expect(sortByRelevance(entries, false)).toEqual(["second", "first"]); + relevanceOrder(scoredResults, "asc"); + + expect(scoredResults.map((entry) => entry.result.uid)).toEqual(["a", "b"]); }); }); diff --git a/apps/roam/src/utils/discourseNodeSearchTypes.ts b/apps/roam/src/utils/discourseNodeSearchTypes.ts index 0151c7ba68..7debd6bac3 100644 --- a/apps/roam/src/utils/discourseNodeSearchTypes.ts +++ b/apps/roam/src/utils/discourseNodeSearchTypes.ts @@ -15,8 +15,6 @@ export type ScoredSearchResult = { result: SearchResult; score: number; source: DiscourseNodeSearchSource; - // Position in the provider's own result list, used to preserve its ordering. - rank: number; }; export const combineSemanticAndMiniSearchResults = ({ @@ -44,7 +42,6 @@ export const toScoredSearchResultFromSemantic = ({ type, nodeTypeLabel, score, - rank, resultsByUid, }: { uid: string; @@ -52,12 +49,11 @@ export const toScoredSearchResultFromSemantic = ({ type?: string; nodeTypeLabel?: string; score: number; - rank: number; resultsByUid: Map; }): ScoredSearchResult => { const indexedResult = resultsByUid.get(uid); if (indexedResult) { - return { result: indexedResult, score, rank, source: "semantic" }; + return { result: indexedResult, score, source: "semantic" }; } return { @@ -72,25 +68,6 @@ export const toScoredSearchResultFromSemantic = ({ authorName: "Unknown", }, score, - rank, source: "semantic", }; }; - -// Roam's semantic scores are not comparable across hits, so semantic entries keep -// the order Roam returned them in; MiniSearch entries still sort by score. -export const compareByRelevance = ({ - a, - b, - descending, -}: { - a: ScoredSearchResult; - b: ScoredSearchResult; - descending: boolean; -}): number => { - if (a.source !== b.source) return a.source === "semantic" ? -1 : 1; - - const comparison = - a.source === "semantic" ? a.rank - b.rank : b.score - a.score; - return descending ? comparison : -comparison; -}; diff --git a/apps/roam/src/utils/searchDiscourseNodes.ts b/apps/roam/src/utils/searchDiscourseNodes.ts index 8cc5283f4c..251bedfad5 100644 --- a/apps/roam/src/utils/searchDiscourseNodes.ts +++ b/apps/roam/src/utils/searchDiscourseNodes.ts @@ -51,14 +51,13 @@ export const searchDiscourseNodes = async ({ nodeTypes, query: trimmedQuery, }); - const semanticResults = providerResult.filteredResults.map((item, index) => + const semanticResults = providerResult.filteredResults.map((item) => toScoredSearchResultFromSemantic({ uid: item.uid, title: item.text, type: item.type, nodeTypeLabel: item.nodeTypeLabel, score: item.score ?? 0, - rank: index, resultsByUid, }), ); From 58bfcd51571b051fc3f1eb2f927a24f52ff36c13 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 21 Sep 2026 19:14:29 -0400 Subject: [PATCH 13/16] ENG-1734 Cap search results and stop the docked panel latching empty Semantic results were never capped: MAX_RESULTS only applied inside the MiniSearch helper, so the semantic and combined paths could return far more. Move the constant down to the search types and slice every path. The docked panel persists results on every change, including the empty list published while an async search is in flight. If it unmounted in that window the cached empty list was treated as a hit on restore, so a query with matches stayed at zero results. Only reuse a non-empty cache. Co-Authored-By: Claude Opus 5 --- .../useAdvancedNodeSearchResults.ts | 16 +++++---- .../AdvancedNodeSearchDialog/utils.ts | 9 ++--- .../discourseNodeSearchTypes.test.ts | 34 +++++++++++++++++++ .../src/utils/discourseNodeSearchTypes.ts | 5 +++ apps/roam/src/utils/searchDiscourseNodes.ts | 22 ++++++++---- 5 files changed, 69 insertions(+), 17 deletions(-) diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index f3e5f746f3..86b4a3b2fd 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -36,11 +36,15 @@ export const useAdvancedNodeSearchResults = ({ dockedQuery, dockedResults, }: UseAdvancedNodeSearchResultsArgs): SearchResult[] => { - const isDockedQuery = useMemo( + // An empty persisted result set is not a usable cache: the docked panel also + // persists the transient [] published while an async search is in flight. + const hasUsableDockedResults = useMemo( () => dockedQuery !== undefined && - debouncedSearchTerm.trim() === dockedQuery.trim(), - [debouncedSearchTerm, dockedQuery], + debouncedSearchTerm.trim() === dockedQuery.trim() && + !!dockedResults && + dockedResults.length > 0, + [debouncedSearchTerm, dockedQuery, dockedResults], ); const [unsortedScoredResults, setUnsortedScoredResults] = useState< @@ -48,7 +52,7 @@ export const useAdvancedNodeSearchResults = ({ >([]); useEffect(() => { - if (isDockedQuery) { + if (hasUsableDockedResults) { setUnsortedScoredResults([]); return; } @@ -105,8 +109,8 @@ export const useAdvancedNodeSearchResults = ({ }; }, [ debouncedSearchTerm, + hasUsableDockedResults, indexError, - isDockedQuery, isIndexLoading, searchIndex, selectedNodeTypeIds, @@ -117,7 +121,7 @@ export const useAdvancedNodeSearchResults = ({ [unsortedScoredResults, sort], ); - if (isDockedQuery && dockedResults) { + if (hasUsableDockedResults && dockedResults) { return dockedResults; } diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts index 2b21a1bee6..82c7a4be13 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts @@ -9,9 +9,10 @@ import { getPulledDiscourseNodeUid, queryDiscourseNodesByFormat, } from "~/utils/discourseNodeSearch"; -import type { - ScoredSearchResult, - SearchResult, +import { + MAX_RESULTS, + type ScoredSearchResult, + type SearchResult, } from "~/utils/discourseNodeSearchTypes"; export type { @@ -20,7 +21,7 @@ export type { } from "~/utils/discourseNodeSearchTypes"; export const DEBOUNCE_MS = 250; -export const MAX_RESULTS = 50; +export { MAX_RESULTS }; export const getSearchKeywords = (searchTerm: string): string[] => searchTerm.split(/\s+/).filter(Boolean); diff --git a/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts b/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts index 2bc056ee76..99c7d94941 100644 --- a/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts +++ b/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { sortSearchResults } from "~/components/AdvancedNodeSearchDialog/utils"; import { combineSemanticAndMiniSearchResults, + MAX_RESULTS, type DiscourseNodeSearchSource, type ScoredSearchResult, } from "~/utils/discourseNodeSearchTypes"; @@ -106,3 +107,36 @@ describe("sortSearchResults relevance", () => { expect(scoredResults.map((entry) => entry.result.uid)).toEqual(["a", "b"]); }); }); + +describe("combined semantic + MiniSearch ordering", () => { + it("keeps every semantic result, in order, ahead of MiniSearch results in order", () => { + const semantic = ["s1", "s2", "s3"].map((uid, i) => + makeEntry({ uid, score: i, source: "semantic" }), + ); + const miniSearch = ["m1", "m2"].map((uid, i) => + makeEntry({ uid, score: 100 - i, source: "miniSearch" }), + ); + + const combined = combineSemanticAndMiniSearchResults({ + semantic, + miniSearch, + }); + + expect(relevanceOrder(combined)).toEqual(["s1", "s2", "s3", "m1", "m2"]); + }); + + it("caps at MAX_RESULTS so the semantic path cannot overflow the list", () => { + expect(MAX_RESULTS).toBe(50); + const semantic = Array.from({ length: 200 }, (_, i) => + makeEntry({ uid: `s${i}`, score: 0, source: "semantic" }), + ); + + const combined = combineSemanticAndMiniSearchResults({ + semantic, + miniSearch: [makeEntry({ uid: "m1", score: 9, source: "miniSearch" })], + }); + + expect(combined).toHaveLength(201); + expect(combined.slice(0, MAX_RESULTS)).toHaveLength(50); + }); +}); diff --git a/apps/roam/src/utils/discourseNodeSearchTypes.ts b/apps/roam/src/utils/discourseNodeSearchTypes.ts index 7debd6bac3..516eee4721 100644 --- a/apps/roam/src/utils/discourseNodeSearchTypes.ts +++ b/apps/roam/src/utils/discourseNodeSearchTypes.ts @@ -1,5 +1,8 @@ export type DiscourseNodeSearchSource = "semantic" | "miniSearch"; +// Caps what any search path returns, semantic included. +export const MAX_RESULTS = 50; + export type SearchResult = { uid: string; title: string; @@ -11,6 +14,8 @@ export type SearchResult = { authorName: string; }; +// score and source are carried for diagnostics and future rank fusion; ordering +// reuses each provider's returned order instead of re-deriving it from score. export type ScoredSearchResult = { result: SearchResult; score: number; diff --git a/apps/roam/src/utils/searchDiscourseNodes.ts b/apps/roam/src/utils/searchDiscourseNodes.ts index 251bedfad5..b3de486e7c 100644 --- a/apps/roam/src/utils/searchDiscourseNodes.ts +++ b/apps/roam/src/utils/searchDiscourseNodes.ts @@ -1,6 +1,7 @@ import type { DiscourseNode } from "~/utils/getDiscourseNodes"; import { combineSemanticAndMiniSearchResults, + MAX_RESULTS, toScoredSearchResultFromSemantic, type ScoredSearchResult, type SearchResult, @@ -28,17 +29,19 @@ const runMiniSearchSafely = ( } }; -export const searchDiscourseNodes = async ({ - nodeTypes, - query, - resultsByUid, - runMiniSearch, -}: { +type SearchDiscourseNodesArgs = { nodeTypes: DiscourseNode[]; query: string; resultsByUid: Map; runMiniSearch: () => ScoredSearchResult[]; -}): Promise => { +}; + +const collectDiscourseNodeResults = async ({ + nodeTypes, + query, + resultsByUid, + runMiniSearch, +}: SearchDiscourseNodesArgs): Promise => { const trimmedQuery = query.trim(); if (!trimmedQuery) return []; @@ -77,3 +80,8 @@ export const searchDiscourseNodes = async ({ return runMiniSearchSafely(runMiniSearch); } }; + +export const searchDiscourseNodes = async ( + args: SearchDiscourseNodesArgs, +): Promise => + (await collectDiscourseNodeResults(args)).slice(0, MAX_RESULTS); From 8e1b146d4c444d6e6b1f42fed87bedab5237c8ba Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 21 Sep 2026 19:21:31 -0400 Subject: [PATCH 14/16] ENG-1734 Show the indexing state while an async search is in flight Search became async with semantic search, but contentState only knew about isIndexLoading, so "No matches" rendered for the whole semanticSearch round trip. Track an in-flight flag and use it in the dialog and the docked panel. Also route the recovery path's runMiniSearch() through the same guard the other call sites use, so a throw there cannot become an unhandled rejection. Co-Authored-By: Claude Opus 5 --- .../AdvancedSearchDialog.tsx | 4 ++-- .../AdvancedSearchSidebarPanel.tsx | 15 +++++++----- .../useAdvancedNodeSearchResults.ts | 23 +++++++++++++++---- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchDialog.tsx b/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchDialog.tsx index 9a9830af57..35dbf6a155 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchDialog.tsx +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchDialog.tsx @@ -171,7 +171,7 @@ const AdvancedNodeSearchDialog = ({ discourseNodes.map((node) => [node.type, node]), ); - const results = useAdvancedNodeSearchResults({ + const { results, isSearching } = useAdvancedNodeSearchResults({ debouncedSearchTerm, selectedNodeTypeIds, sort, @@ -291,7 +291,7 @@ const AdvancedNodeSearchDialog = ({ const contentState = indexError ? "error" - : isIndexLoading + : isIndexLoading || isSearching ? "indexing" : !debouncedSearchTerm && selectedNodeTypeIds.length === 0 ? "initial" diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx b/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx index 463e98e211..bd48b7f05b 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/AdvancedSearchSidebarPanel.tsx @@ -226,7 +226,7 @@ export const AdvancedSearchSidebarPanel = ({ }; }, [discourseNodes]); - const results = useAdvancedNodeSearchResults({ + const { results, isSearching } = useAdvancedNodeSearchResults({ debouncedSearchTerm, selectedNodeTypeIds, sort, @@ -306,11 +306,14 @@ export const AdvancedSearchSidebarPanel = ({ results={results} /> )} - {debouncedSearchTerm && !results.length && !isIndexLoading && ( -

- No matches. Try another keyword. -

- )} + {debouncedSearchTerm && + !results.length && + !isIndexLoading && + !isSearching && ( +

+ No matches. Try another keyword. +

+ )} )} diff --git a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts index 86b4a3b2fd..9de2a6adee 100644 --- a/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts +++ b/apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts @@ -35,7 +35,10 @@ export const useAdvancedNodeSearchResults = ({ searchIndex, dockedQuery, dockedResults, -}: UseAdvancedNodeSearchResultsArgs): SearchResult[] => { +}: UseAdvancedNodeSearchResultsArgs): { + results: SearchResult[]; + isSearching: boolean; +} => { // An empty persisted result set is not a usable cache: the docked panel also // persists the transient [] published while an async search is in flight. const hasUsableDockedResults = useMemo( @@ -50,24 +53,29 @@ export const useAdvancedNodeSearchResults = ({ const [unsortedScoredResults, setUnsortedScoredResults] = useState< ScoredSearchResult[] >([]); + const [isSearching, setIsSearching] = useState(false); useEffect(() => { if (hasUsableDockedResults) { setUnsortedScoredResults([]); + setIsSearching(false); return; } if (!debouncedSearchTerm) { setUnsortedScoredResults([]); + setIsSearching(false); return; } if (isIndexLoading || indexError || !searchIndex) { setUnsortedScoredResults([]); + setIsSearching(false); return; } setUnsortedScoredResults([]); + setIsSearching(true); let cancelled = false; const typeFilter = selectedNodeTypeIds.length ? selectedNodeTypeIds @@ -101,7 +109,14 @@ export const useAdvancedNodeSearchResults = ({ }) .catch(() => { if (cancelled) return; - setUnsortedScoredResults(runMiniSearch()); + try { + setUnsortedScoredResults(runMiniSearch()); + } catch { + setUnsortedScoredResults([]); + } + }) + .finally(() => { + if (!cancelled) setIsSearching(false); }); return () => { @@ -122,8 +137,8 @@ export const useAdvancedNodeSearchResults = ({ ); if (hasUsableDockedResults && dockedResults) { - return dockedResults; + return { results: dockedResults, isSearching: false }; } - return results; + return { results, isSearching }; }; From 85f1a0e85850c8071098816071ffbcb6260614ba Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 21 Sep 2026 22:32:31 -0400 Subject: [PATCH 15/16] ENG-1734 Test the result cap where it is applied The cap lives in searchDiscourseNodes, not in the combine helper, so the combine test could not prove it. Add searchDiscourseNodes coverage for the cap, the semantic-only threshold, the sparse combine, both fallbacks, and the blank query, and correct the combine test to state what it checks. Co-Authored-By: Claude Opus 5 --- .../discourseNodeSearchTypes.test.ts | 6 +- .../__tests__/searchDiscourseNodes.test.ts | 151 ++++++++++++++++++ 2 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 apps/roam/src/utils/__tests__/searchDiscourseNodes.test.ts diff --git a/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts b/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts index 99c7d94941..3e4df750d0 100644 --- a/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts +++ b/apps/roam/src/utils/__tests__/discourseNodeSearchTypes.test.ts @@ -125,8 +125,7 @@ describe("combined semantic + MiniSearch ordering", () => { expect(relevanceOrder(combined)).toEqual(["s1", "s2", "s3", "m1", "m2"]); }); - it("caps at MAX_RESULTS so the semantic path cannot overflow the list", () => { - expect(MAX_RESULTS).toBe(50); + it("does not cap, so callers stay responsible for MAX_RESULTS", () => { const semantic = Array.from({ length: 200 }, (_, i) => makeEntry({ uid: `s${i}`, score: 0, source: "semantic" }), ); @@ -136,7 +135,8 @@ describe("combined semantic + MiniSearch ordering", () => { miniSearch: [makeEntry({ uid: "m1", score: 9, source: "miniSearch" })], }); + // searchDiscourseNodes applies the cap; see searchDiscourseNodes.test.ts. expect(combined).toHaveLength(201); - expect(combined.slice(0, MAX_RESULTS)).toHaveLength(50); + expect(combined.length).toBeGreaterThan(MAX_RESULTS); }); }); diff --git a/apps/roam/src/utils/__tests__/searchDiscourseNodes.test.ts b/apps/roam/src/utils/__tests__/searchDiscourseNodes.test.ts new file mode 100644 index 0000000000..9163b806e2 --- /dev/null +++ b/apps/roam/src/utils/__tests__/searchDiscourseNodes.test.ts @@ -0,0 +1,151 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +type SemanticProviderPayload = { + filteredResults: { uid: string; text: string }[]; + filteredResultCount: number; +}; + +const runRoamSemanticSearch = + vi.fn<(args: unknown) => Promise>(); +vi.mock("~/utils/discourseNodeSearchProviders", () => ({ + runRoamSemanticSearch: (args: unknown) => runRoamSemanticSearch(args), +})); + +import { searchDiscourseNodes } from "~/utils/searchDiscourseNodes"; +import { + MAX_RESULTS, + type ScoredSearchResult, + type SearchResult, +} from "~/utils/discourseNodeSearchTypes"; + +const originalWindow = (globalThis as { window?: unknown }).window; + +const setSemanticSearchEnabled = (enabled: boolean): void => { + (globalThis as { window: unknown }).window = { + roamAlphaAPI: { data: { semanticSearchEnabled: () => enabled } }, + }; +}; + +// Roam returns hits without a score, so semantic entries carry score 0. +const semanticHits = (count: number): { uid: string; text: string }[] => + Array.from({ length: count }, (_, i) => ({ + uid: `s${i}`, + text: `[[CLM]] - semantic ${i}`, + })); + +const miniSearchEntry = (uid: string): ScoredSearchResult => ({ + result: { + uid, + title: `[[CLM]] - ${uid}`, + type: "clm", + nodeTypeLabel: "Claim", + excerpt: "", + createdAt: "", + lastModified: "", + authorName: "", + }, + score: 9, + source: "miniSearch", +}); + +const search = ( + runMiniSearch: () => ScoredSearchResult[], +): Promise => + searchDiscourseNodes({ + nodeTypes: [], + query: "governance", + resultsByUid: new Map(), + runMiniSearch, + }); + +beforeEach(() => { + runRoamSemanticSearch.mockReset(); + setSemanticSearchEnabled(true); +}); + +afterEach(() => { + (globalThis as { window?: unknown }).window = originalWindow; +}); + +describe("searchDiscourseNodes", () => { + it("caps the semantic path at MAX_RESULTS", async () => { + const filteredResults = semanticHits(200); + runRoamSemanticSearch.mockResolvedValue({ + filteredResults, + filteredResultCount: filteredResults.length, + }); + + const results = await search(() => []); + + expect(results).toHaveLength(MAX_RESULTS); + expect(results[0]?.result.uid).toBe("s0"); + }); + + it("skips MiniSearch once semantic returns enough discourse nodes", async () => { + const filteredResults = semanticHits(5); + runRoamSemanticSearch.mockResolvedValue({ + filteredResults, + filteredResultCount: filteredResults.length, + }); + const runMiniSearch = vi.fn(() => [miniSearchEntry("m1")]); + + const results = await search(runMiniSearch); + + expect(runMiniSearch).not.toHaveBeenCalled(); + expect(results.map((r) => r.result.uid)).toEqual([ + "s0", + "s1", + "s2", + "s3", + "s4", + ]); + }); + + it("appends MiniSearch results after sparse semantic results", async () => { + const filteredResults = semanticHits(2); + runRoamSemanticSearch.mockResolvedValue({ + filteredResults, + filteredResultCount: filteredResults.length, + }); + + const results = await search(() => [ + miniSearchEntry("m1"), + miniSearchEntry("m2"), + ]); + + expect(results.map((r) => r.result.uid)).toEqual(["s0", "s1", "m1", "m2"]); + }); + + it("falls back to MiniSearch when the semantic provider throws", async () => { + runRoamSemanticSearch.mockRejectedValue(new Error("semantic unavailable")); + + const results = await search(() => [miniSearchEntry("m1")]); + + expect(results.map((r) => r.result.uid)).toEqual(["m1"]); + }); + + it("degrades to an empty list when MiniSearch itself throws", async () => { + setSemanticSearchEnabled(false); + + await expect( + search(() => { + throw new Error("minisearch exploded"); + }), + ).resolves.toEqual([]); + }); + + it("returns nothing for a blank query without calling either provider", async () => { + const runMiniSearch = vi.fn(() => [miniSearchEntry("m1")]); + + const results = await searchDiscourseNodes({ + nodeTypes: [], + query: " ", + resultsByUid: new Map(), + runMiniSearch, + }); + + expect(results).toEqual([]); + expect(runMiniSearch).not.toHaveBeenCalled(); + expect(runRoamSemanticSearch).not.toHaveBeenCalled(); + }); +}); From 75910495618fb66ef47271367761298ea073faa9 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Tue, 22 Sep 2026 19:12:43 -0400 Subject: [PATCH 16/16] ENG-1734 Inline the search back into searchDiscourseNodes The wrapper existed only to give the cap a single place to apply. One function with a single exit does the same thing without the indirection. Co-Authored-By: Claude Opus 5 Entire-Checkpoint: 01M35P984XJ2TWP81W573WTT9H --- apps/roam/src/utils/searchDiscourseNodes.ts | 82 ++++++++++----------- 1 file changed, 38 insertions(+), 44 deletions(-) diff --git a/apps/roam/src/utils/searchDiscourseNodes.ts b/apps/roam/src/utils/searchDiscourseNodes.ts index b3de486e7c..9be5bacd15 100644 --- a/apps/roam/src/utils/searchDiscourseNodes.ts +++ b/apps/roam/src/utils/searchDiscourseNodes.ts @@ -29,59 +29,53 @@ const runMiniSearchSafely = ( } }; -type SearchDiscourseNodesArgs = { - nodeTypes: DiscourseNode[]; - query: string; - resultsByUid: Map; - runMiniSearch: () => ScoredSearchResult[]; -}; - -const collectDiscourseNodeResults = async ({ +export const searchDiscourseNodes = async ({ nodeTypes, query, resultsByUid, runMiniSearch, -}: SearchDiscourseNodesArgs): Promise => { +}: { + nodeTypes: DiscourseNode[]; + query: string; + resultsByUid: Map; + runMiniSearch: () => ScoredSearchResult[]; +}): Promise => { const trimmedQuery = query.trim(); if (!trimmedQuery) return []; - if (!isRoamSemanticSearchEnabled()) { - return runMiniSearchSafely(runMiniSearch); - } + let results: ScoredSearchResult[]; - try { - const providerResult = await runRoamSemanticSearch({ - nodeTypes, - query: trimmedQuery, - }); - const semanticResults = providerResult.filteredResults.map((item) => - toScoredSearchResultFromSemantic({ - uid: item.uid, - title: item.text, - type: item.type, - nodeTypeLabel: item.nodeTypeLabel, - score: item.score ?? 0, - resultsByUid, - }), - ); + if (!isRoamSemanticSearchEnabled()) { + results = runMiniSearchSafely(runMiniSearch); + } else { + try { + const providerResult = await runRoamSemanticSearch({ + nodeTypes, + query: trimmedQuery, + }); + const semanticResults = providerResult.filteredResults.map((item) => + toScoredSearchResultFromSemantic({ + uid: item.uid, + title: item.text, + type: item.type, + nodeTypeLabel: item.nodeTypeLabel, + score: item.score ?? 0, + resultsByUid, + }), + ); - if ( - providerResult.filteredResultCount >= - SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS - ) { - return semanticResults; + results = + providerResult.filteredResultCount >= + SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS + ? semanticResults + : combineSemanticAndMiniSearchResults({ + semantic: semanticResults, + miniSearch: runMiniSearchSafely(runMiniSearch), + }); + } catch { + results = runMiniSearchSafely(runMiniSearch); } - - return combineSemanticAndMiniSearchResults({ - semantic: semanticResults, - miniSearch: runMiniSearchSafely(runMiniSearch), - }); - } catch { - return runMiniSearchSafely(runMiniSearch); } -}; -export const searchDiscourseNodes = async ( - args: SearchDiscourseNodesArgs, -): Promise => - (await collectDiscourseNodeResults(args)).slice(0, MAX_RESULTS); + return results.slice(0, MAX_RESULTS); +};