From e289dd9e11d1b34950cbf9a5cb315729b61bec30 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Thu, 2 Jul 2026 22:56:54 -0400 Subject: [PATCH 01/10] 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 d3a682864..475eb3688 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 c3040eadf..4b08f17be 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 000000000..a81488707 --- /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 a34d1be34..24f759541 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 000000000..eb42252b3 --- /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/10] 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 a81488707..000000000 --- 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/10] 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 463e98e21..6ae5dd4a1 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 000000000..2d2003088 --- /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 475eb3688..d8b4541c3 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 000000000..16b699518 --- /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/10] 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 9562c116f..a29a1ef84 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 9888d3c4a..7cd250061 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 6ae5dd4a1..463e98e21 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 2d2003088..000000000 --- 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 d8b4541c3..0b1a0eaed 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 16b699518..000000000 --- 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/10] 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 a29a1ef84..9562c116f 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 7cd250061..9888d3c4a 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/10] 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 0b1a0eaed..211d90b3b 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 4b08f17be..abaa88ba8 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 000000000..cc9601b39 --- /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 eb42252b3..e947104b1 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/10] 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 211d90b3b..0db9d3103 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 abaa88ba8..2405eac84 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 cc9601b39..9d830f726 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 e947104b1..d9ceacf46 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/10] 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 0db9d3103..dbf8e8df0 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/10] 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 dbf8e8df0..6ee537b5f 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 d9ceacf46..720bb0db1 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/10] 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 6ee537b5f..f3e5f746f 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 2405eac84..9c75a6016 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 720bb0db1..d9ceacf46 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(); } };