-
Notifications
You must be signed in to change notification settings - Fork 6
ENG-1734 Add Roam semantic search to Advanced Node Search #1186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e289dd9
9c80e1e
894901f
3b97b9b
aebb862
1bf4f21
dccf234
053d5da
2339198
3286231
7acc534
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,15 @@ import { | |
| getPulledDiscourseNodeUid, | ||
| queryDiscourseNodesByFormat, | ||
| } from "~/utils/discourseNodeSearch"; | ||
| import type { | ||
| ScoredSearchResult, | ||
| SearchResult, | ||
| } from "~/utils/discourseNodeSearchTypes"; | ||
|
|
||
| export type { | ||
| ScoredSearchResult, | ||
| SearchResult, | ||
| } from "~/utils/discourseNodeSearchTypes"; | ||
|
|
||
| export const DEBOUNCE_MS = 250; | ||
| export const MAX_RESULTS = 50; | ||
|
|
@@ -36,17 +45,6 @@ export const SORT_FIELD_LABELS: Record<SortField, string> = { | |
| 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,11 +54,6 @@ export type DockedSearchState = { | |
| dgSearchId?: string; | ||
| }; | ||
|
|
||
| export type ScoredSearchHit = { | ||
| result: SearchResult; | ||
| score: number; | ||
| }; | ||
|
|
||
| type MiniSearchDocument = SearchResult & { | ||
| id: string; | ||
| }; | ||
|
|
@@ -214,22 +207,26 @@ 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": | ||
| comparison = compareNumbers(aHit.score, bHit.score, sort.direction); | ||
| if (aEntry.source !== bEntry.source) { | ||
| comparison = aEntry.source === "semantic" ? -1 : 1; | ||
| break; | ||
| } | ||
| comparison = compareNumbers(aEntry.score, bEntry.score, sort.direction); | ||
|
trangdoan982 marked this conversation as resolved.
|
||
| break; | ||
| case "alphabetical": | ||
| comparison = compareStrings( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
@@ -259,10 +256,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, | ||
|
|
@@ -272,7 +269,7 @@ export const searchIndexedNodes = ({ | |
| allResults: SearchResult[]; | ||
| searchTerm: string; | ||
| typeFilter?: string[]; | ||
| }): ScoredSearchHit[] => { | ||
| }): ScoredSearchResult[] => { | ||
| const resultsByUid = new Map( | ||
| allResults.map((result) => [result.uid, result]), | ||
| ); | ||
|
|
@@ -291,7 +288,11 @@ 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: "miniSearch", | ||
| }; | ||
| }) | ||
| .filter((hit): hit is ScoredSearchHit => !!hit); | ||
| .filter((entry): entry is ScoredSearchResult => !!entry); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| export type DiscourseNodeSearchSource = "semantic" | "miniSearch"; | ||
|
|
||
| export type SearchResult = { | ||
| uid: string; | ||
| title: string; | ||
| type: string; | ||
| nodeTypeLabel: string; | ||
| excerpt: string; | ||
| createdAt: string; | ||
| lastModified: string; | ||
| authorName: string; | ||
| }; | ||
|
|
||
| export type ScoredSearchResult = { | ||
| result: SearchResult; | ||
| score: number; | ||
| source: DiscourseNodeSearchSource; | ||
| }; | ||
|
|
||
| export const combineSemanticAndMiniSearchResults = ({ | ||
| semantic, | ||
| miniSearch, | ||
| }: { | ||
| semantic: ScoredSearchResult[]; | ||
| miniSearch: ScoredSearchResult[]; | ||
| }): ScoredSearchResult[] => { | ||
| const seenUids = new Set(semantic.map((entry) => entry.result.uid)); | ||
| const combined = [...semantic]; | ||
|
|
||
| miniSearch.forEach((entry) => { | ||
| if (seenUids.has(entry.result.uid)) return; | ||
| seenUids.add(entry.result.uid); | ||
| combined.push(entry); | ||
| }); | ||
|
|
||
| return combined; | ||
| }; | ||
|
Comment on lines
+20
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== File outline ==\n'
ast-grep outline apps/roam/src/utils/discourseNodeSearchTypes.ts --view expanded || true
printf '\n== File contents ==\n'
cat -n apps/roam/src/utils/discourseNodeSearchTypes.ts
printf '\n== Search usages ==\n'
rg -n "combineSemanticAndMiniSearchResults|ScoredSearchResult|semantic.*result.uid|miniSearch" apps/roam/src -SRepository: DiscourseGraphs/discourse-graph Length of output: 8447 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== searchDiscourseNodes ==\n'
cat -n apps/roam/src/utils/searchDiscourseNodes.ts
printf '\n== Advanced search semantic path ==\n'
sed -n '250,320p' apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts
printf '\n== useAdvancedNodeSearchResults ==\n'
sed -n '1,140p' apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts
printf '\n== semantic provider result shaping ==\n'
sed -n '330,420p' apps/roam/src/utils/discourseNodeSearchProviders.tsRepository: DiscourseGraphs/discourse-graph Length of output: 9516 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== runRoamSemanticSearch definition ==\n'
rg -n "export .*runRoamSemanticSearch|const runRoamSemanticSearch|function runRoamSemanticSearch" apps/roam/src/utils/discourseNodeSearchProviders.ts apps/roam/src -S
printf '\n== relevant provider section ==\n'
sed -n '480,620p' apps/roam/src/utils/discourseNodeSearchProviders.tsRepository: DiscourseGraphs/discourse-graph Length of output: 3341 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '220,320p' apps/roam/src/utils/discourseNodeSearchProviders.ts
printf '\n== filterResultsToDiscourseNodeTitles ==\n'
rg -n "filterResultsToDiscourseNodeTitles" apps/roam/src/utils/discourseNodeSearchProviders.ts apps/roam/src -n -A 80 -B 20Repository: DiscourseGraphs/discourse-graph Length of output: 46659 🌐 Web query:
💡 Result: The Roam Alpha API provides a Citations:
Dedup semantic hits before combining The Roam semantic path still passes repeated 🤖 Prompt for AI Agents |
||
|
|
||
| export const toScoredSearchResultFromSemantic = ({ | ||
| uid, | ||
| title, | ||
| type, | ||
| nodeTypeLabel, | ||
| score, | ||
| resultsByUid, | ||
| }: { | ||
| uid: string; | ||
| title: string; | ||
| type?: string; | ||
| nodeTypeLabel?: string; | ||
| score: number; | ||
| resultsByUid: Map<string, SearchResult>; | ||
| }): ScoredSearchResult => { | ||
| const indexedResult = resultsByUid.get(uid); | ||
| if (indexedResult) { | ||
| return { result: indexedResult, score, source: "semantic" }; | ||
| } | ||
|
|
||
| return { | ||
| result: { | ||
| uid, | ||
| title, | ||
| type: type || "", | ||
| nodeTypeLabel: nodeTypeLabel || "", | ||
| excerpt: "", | ||
| createdAt: "", | ||
| lastModified: "", | ||
| authorName: "Unknown", | ||
| }, | ||
| score, | ||
| source: "semantic", | ||
| }; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import type { DiscourseNode } from "~/utils/getDiscourseNodes"; | ||
| import { | ||
| combineSemanticAndMiniSearchResults, | ||
| toScoredSearchResultFromSemantic, | ||
| type ScoredSearchResult, | ||
| type SearchResult, | ||
| } from "~/utils/discourseNodeSearchTypes"; | ||
| import { runRoamSemanticSearch } from "~/utils/discourseNodeSearchProviders"; | ||
|
|
||
| export const SEMANTIC_SEARCH_MIN_DISCOURSE_RESULTS = 5; | ||
|
|
||
| export type { | ||
| DiscourseNodeSearchSource, | ||
| ScoredSearchResult, | ||
| SearchResult, | ||
| } from "~/utils/discourseNodeSearchTypes"; | ||
|
|
||
| export const isRoamSemanticSearchEnabled = (): boolean => | ||
| window.roamAlphaAPI.data.semanticSearchEnabled(); | ||
|
|
||
| export const searchDiscourseNodes = async ({ | ||
| nodeTypes, | ||
| query, | ||
| resultsByUid, | ||
| runMiniSearch, | ||
| }: { | ||
| nodeTypes: DiscourseNode[]; | ||
| query: string; | ||
| resultsByUid: Map<string, SearchResult>; | ||
| runMiniSearch: () => ScoredSearchResult[]; | ||
| }): Promise<ScoredSearchResult[]> => { | ||
| const trimmedQuery = query.trim(); | ||
| if (!trimmedQuery) return []; | ||
|
|
||
| if (!isRoamSemanticSearchEnabled()) { | ||
| return runMiniSearch(); | ||
| } | ||
|
|
||
| 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; | ||
| } | ||
|
|
||
| return combineSemanticAndMiniSearchResults({ | ||
| semantic: semanticResults, | ||
| miniSearch: runMiniSearch(), | ||
| }); | ||
| } catch { | ||
| return runMiniSearch(); | ||
| } | ||
| }; | ||
|
Comment on lines
+21
to
+69
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Locate the relevant search utilities and call sites.
git ls-files 'apps/roam/src/**/*search*' 'apps/roam/src/**/*Search*' 'apps/roam/src/**/*mini*' | sed -n '1,200p'
printf '\n--- searchDiscourseNodes.ts ---\n'
cat -n apps/roam/src/utils/searchDiscourseNodes.ts | sed -n '1,220p'
printf '\n--- useAdvancedNodeSearchResults references ---\n'
rg -n "useAdvancedNodeSearchResults|searchDiscourseNodes|runMiniSearch|MiniSearch" apps/roam/src -A 3 -B 3Repository: DiscourseGraphs/discourse-graph Length of output: 29073 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n--- AdvancedNodeSearchDialog/utils.ts (search helper) ---\n'
cat -n apps/roam/src/components/AdvancedNodeSearchDialog/utils.ts | sed -n '240,340p'
printf '\n--- AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts (fallback path) ---\n'
cat -n apps/roam/src/components/AdvancedNodeSearchDialog/useAdvancedNodeSearchResults.ts | sed -n '70,110p'
printf '\n--- MiniSearch search-error handling in the repo ---\n'
rg -n "try \\{|catch \\{|miniSearch\\.search\\(|search\\(MiniSearch\\.wildcard|searchTerm" apps/roam/src/components/AdvancedNodeSearchDialog apps/roam/src/utils -A 3 -B 3Repository: DiscourseGraphs/discourse-graph Length of output: 50387 🌐 Web query:
💡 Result: MiniSearch does not explicitly throw a "malformed query" exception or a generic "invalid query syntax" error in its official documentation or API. Instead, errors encountered during the search process typically manifest as JavaScript runtime exceptions—such as TypeErrors—triggered when the Citations:
Guard the MiniSearch fallback too 🤖 Prompt for AI Agents |
||


Uh oh!
There was an error while loading. Please reload this page.