ENG-2109 Create node search modal with ranked results and preview by trangdoan982 · Pull Request #1285 · DiscourseGraphs/discourse-graph

trangdoan982 · 2026-08-07T23:12:06Z

https://www.loom.com/share/877db7d2de834548a81bca1750bbbecb

Stacked on #1284 (ENG-2108). Base is eng-2108-…, not main, so this diff shows only the modal. Review and merge #1284 first; this base will need repointing once it lands, since the repo squash-merges.

Scope check

Ran $scope-check against ENG-2109 and the final diff.
Scope beyond Done When: Match highlighting, pulled forward from F11 (ENG-2115).
Required now: RankedDiscourseNode already carries the whole SearchResult, and ENG-2108 scores file.basename specifically so those offsets stay valid for renderResults — rendering a plain title discards data the design deliberately preserved. It is also what makes the ranking legible enough to evaluate live, which is why this ticket was stacked rather than deferred.
Anyone affected or consulted: Yes — requested during implementation.
Decision: recorded on both ENG-2109 (moved into Solution and Done When) and ENG-2115 (struck through, leaving theme verification).

Nothing else exceeds the boundary. F11 keeps badge colour/prefix-stripping and the community-theme pass; the node type renders as plain text here.

What this does

The first UI for the Obsidian advanced node search (F2 + F3 + F5), on top of the ENG-2108 seam.

Modal + React root via createRoot, matching ModifyNodeModal
Ranked results through rankDiscourseNodesByTitle, input debounced 250ms to match SearchBar.tsx, capped at 50
Highlighted matches via renderResults(el, title, match)
Preview pane using MarkdownRenderer.render with a Component for lifecycle — the NodeTypeSettings.tsx pattern — showing created · modified · author
Keyboard navigation
Command open-node-search, hotkeys: []
States: loading / ready / empty / error

🤖 Generated with Claude Code

linear-code · 2026-08-07T23:12:09Z

ENG-2109

vercel · 2026-08-07T23:12:12Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
discourse-graph	Skipped		Aug 22, 2026 4:37am

supabase · 2026-08-07T23:12:21Z

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

devin-ai-integration

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

graphite-app · 2026-08-08T17:41:48Z

PR size/scope check

This PR is over our review-size guideline.

Recommended: ~200 lines changed
Acceptable limit: up to 400 lines when well-scoped/self-contained
Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

What single problem this PR solves
Why the files/changes are coupled

mdroidian · 2026-08-13T04:31:50Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da528fbc30

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

mdroidian · 2026-08-13T06:01:48Z

+};
+
+const LOCAL_AUTHOR_NAME = "You";
+const UNRESOLVED_AUTHOR_NAME = "Unknown";


Shouldn't we assume this is "Imported"?

I have 2 cases where:

if there's authorId -> means it was imported -> try to resolve to either name or "Unknown"

if there's no authorId -> means it's local -> "You"

Add the discourse node search surface: a Modal hosting a React root, a result list ranked by the QueryEngine functions from ENG-2108, and a Markdown preview of the active result. Register it as "Open node search" with no default hotkey, so users bind their own and we avoid colliding with core or community bindings. Highlight matched substrings with Obsidian's renderResults, passing the same string that was scored. Using the platform renderer rather than hand-rolled markup means highlights inherit theme styling, which is the code path that produced the equivalent Roam bug. Open with every node listed in title order rather than an empty prompt, so the modal doubles as a node browser. Model candidate loading as a discriminated union covering loading, ready, empty and error; the fetch is synchronous today, but semantic search will make it a network call and threading those states through later costs far more than carrying them now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

renderResults applies Obsidian's suggestion highlight, which is styled for the quick switcher rather than for search. Point it at --text-highlight-bg instead, the variable behind the yellow in Obsidian's own search view, so matches read the same way there, here, and in the Roam implementation. Target the span element rather than Obsidian's internal class name: renderResults wraps matched ranges in spans and leaves unmatched text as bare text nodes, so every span inside the title is a match, and the rule survives a class rename. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The preview pane read the newly selected note asynchronously while `content` still held the previous note's text, so the render effect fired once with the new file's path and the old file's body — the header showed one note while the pane rendered another. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Follows the Roam result row: the node type is a rounded badge of the first three letters, inline before the title, reusing the colors the editor already paints discourse tags with so a type reads the same in both places. Author names now resolve through `plugin.settings.userNames`, which `fetchUserNames` fills with one query for every person in the vault's spaces. The modal refreshes it at most once per open, and only when an imported node is actually missing a name, so nothing queries per result. Resolution also moved to the selected result, which is the only one whose author is displayed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

getNodeTagColors clamped any index past the twelfth node type to 0, so every type beyond the palette length shared one color. Cycling spreads them instead. This also changes existing tag colors for vaults with more than twelve types. Author resolution now distinguishes the two cases the scope doc separates: no authorId means the note is local ("You"), while an authorId that cannot be resolved from settings or Supabase stays "Unknown" rather than claiming local authorship. A non-numeric authorId counts as present-but-unresolvable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Move the arrow-key handler from the search input to the modal container, so navigation keeps working when focus moves elsewhere inside the modal, and so result actions have one place to live when they arrive. Mirrors the Roam dialog, which binds its handler at the same level. Activate rows on hover as well as click, again matching Roam. Suppress the mouseenter that fires when scrolling drags a row under a stationary cursor — that is the list moving, not the user choosing, and honouring it makes arrow keys jump back a row. Prevent the default on mousedown so clicking a result never pulls focus out of the input. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A "?" chip told the reader nothing except that something was wrong. Roam handles the same case by storing the type's label on each result at index time and falling back to that; we have no stored label, but node formats are `PREFIX - {content}`, so the title still carries the prefix the badge would have shown. A note whose type was deleted, or imported from a differently configured vault, now reads QUE or CLM instead of ?. Omit the chip entirely when the title has no prefix either. Abbreviating the note's own words would produce a confident-looking label that says nothing about its type, which is worse than no label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The palette-cycling fix is a real one — past the twelfth node type every type collapsed to a single colour — but it is a behaviour change to a util shared with the editor, and nothing in the search modal needs it: this vault has nine node types, so clamping and cycling agree. Reverted here so the search PR stays to the search surface; worth its own change. Also drop the badge comments that restated their code, keeping the one that explains what Roam does differently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The revert left a whitespace-only diff: the pre-commit formatter collapsed a double blank line the file already had. Committing without it so colorUtils drops out of this PR entirely rather than appearing as a one-line change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A narrowing query rebuilds `results` before the reset effect runs, so the old index could point past the new list for one render — blanking the preview and leaving no row highlighted. Clamping at render covers that frame; the effect still resets the state so arrow keys continue from the top. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* ENG-2113 Add platform-aware keyboard hint symbols Obsidian renders modifiers as glyphs on macOS and as words on Windows and Linux. Roam's search footer hardcoded the macOS glyphs at each call site and showed the wrong hint on Windows (ENG-2000); routing every hint through one map is what keeps that from repeating. `formatHintKeys` takes `isMacOS` so the non-mac branch can be exercised without that platform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2113 Add footer action bar with open in active pane and split Enter opens the active result in the current pane, Shift+Enter in a split, and both close the modal. Mod+Enter and Alt+Enter deliberately fall through, so the insert action (ENG-2114) can claim Mod+Enter as it does in Roam. The footer reuses Obsidian's own `prompt-instruction` markup, the classes `SuggestModal.setInstructions()` emits, so it matches the native quick switcher. This modal extends plain `Modal`, so that API is unavailable. Its actions are left-aligned rather than centred because they sit under a full-width result list. The Enter branch lives in the existing wrapper `onKeyDown`, which ENG-2109 moved off the input so result actions would have one place to live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2113 Open the active result in a new tab rather than the current one Replacing the page the user was already reading loses their place, which is the opposite of what a lookup surface should do. `getLeaf("tab")` adds a tab to the main panel instead, so the previous note stays open behind it. This reuses the existing `openFileInNewTab`, so the `openFileInActivePane` helper added earlier in this branch is no longer needed. The label now reads "open in new tab" to match. Diverges from the ticket's stated Solution, which specified `getLeaf(false)`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2113 Style footer keys as caps, matching Roam Obsidian's `prompt-instruction-command` is bold with no border, which made the lone `esc` hint read as emphasis rather than as a key. Roam's search footer draws every key as a bordered cap instead, so `esc` sits with the rest of the set. Keeps the `prompt-instructions` container for its native type and spacing, and takes the cap's border, radius, and background from Obsidian's CSS variables so it still follows the active theme. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2113 Drop the results list tooltip Obsidian renders `aria-label` as a hover tooltip, so labelling the listbox meant a tooltip covered the results as soon as the pointer entered the list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2113 Make close clickable and drop the duplicate badge tooltip The close hint was the only footer item that ignored a click, which read as broken next to two working actions. It now goes through the same `FooterAction` as the others and calls the modal's own close. The badge carried both `title` and `aria-label` with the same text, so hovering one stacked a native tooltip on top of Obsidian's. Keeping `aria-label`, since Obsidian's is the themed one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ENG-2113 Let focused footer buttons handle their own Enter A footer button reached by Tab had its bubbling Enter intercepted by the modal's keydown handler, whose preventDefault suppressed the button's native click. So Enter on close opened a new tab, and Enter on split opened a new tab too. Also moves the footer's layout onto Tailwind utilities, leaving only the four properties Obsidian defends with `button:not(.clickable-icon)` and `button:hover` — both (0,1,1), which outrank a single utility class — plus `font-size`, which has no inherit utility. Trims comments that explained history rather than the code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

FrontMatterCache is `{[key: string]: any}`, so asserting it to Record<string, unknown> never changed the type — typescript-eslint 8, which landed on main in #1293, correctly flags the assertion as unnecessary. A type annotation gets the same `unknown` values without the redundant cast. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

devin-ai-integration Bot reviewed Aug 7, 2026

View reviewed changes

Comment thread apps/obsidian/src/components/NodeSearchModal.tsx

vercel Bot temporarily deployed to Preview August 8, 2026 00:06 Inactive

graphite-app Bot reviewed Aug 8, 2026

View reviewed changes

Comment thread apps/obsidian/src/components/NodeSearchModal.tsx

vercel Bot temporarily deployed to Preview August 8, 2026 17:30 Inactive

trangdoan982 force-pushed the eng-2109-create-node-search-modal-with-ranked-results-and-preview branch from adea63a to 2d09490 Compare August 8, 2026 17:33

vercel Bot temporarily deployed to Preview August 8, 2026 17:33 Inactive

trangdoan982 mentioned this pull request Aug 8, 2026

ENG-2108 Add ranked search to QueryEngine with a vault-iteration fallback #1284

Merged

1 task

vercel Bot temporarily deployed to Preview August 8, 2026 17:41 Inactive

trangdoan982 force-pushed the eng-2109-create-node-search-modal-with-ranked-results-and-preview branch from 45ee16f to e5f9945 Compare August 9, 2026 03:56

vercel Bot temporarily deployed to Preview August 9, 2026 03:56 Inactive

vercel Bot temporarily deployed to Preview August 9, 2026 17:19 Inactive

trangdoan982 force-pushed the eng-2109-create-node-search-modal-with-ranked-results-and-preview branch from 6a5112f to fe08c76 Compare August 9, 2026 17:23

vercel Bot temporarily deployed to Preview August 9, 2026 17:23 Inactive

trangdoan982 force-pushed the eng-2109-create-node-search-modal-with-ranked-results-and-preview branch from fe08c76 to 33fb1fc Compare August 9, 2026 17:29

vercel Bot temporarily deployed to Preview August 9, 2026 17:29 Inactive

vercel Bot temporarily deployed to Preview August 9, 2026 17:38 Inactive

vercel Bot temporarily deployed to Preview August 9, 2026 17:44 Inactive

vercel Bot temporarily deployed to Preview August 9, 2026 17:49 Inactive

vercel Bot temporarily deployed to Preview August 9, 2026 17:53 Inactive

vercel Bot temporarily deployed to Preview August 9, 2026 17:54 Inactive

vercel Bot temporarily deployed to Preview August 9, 2026 18:01 Inactive

trangdoan982 requested a review from mdroidian August 9, 2026 18:02

chatgpt-codex-connector Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread apps/obsidian/src/components/NodeSearchModal.tsx Outdated

mdroidian approved these changes Aug 13, 2026

View reviewed changes

vercel Bot temporarily deployed to Preview August 18, 2026 01:59 Inactive

vercel Bot temporarily deployed to Preview August 18, 2026 02:40 Inactive

This was referenced Aug 19, 2026

ENG-2110 Add node type filter dropdown menu #1315

Merged

ENG-2112 Add sort button with sort dropdown #1320

Open

trangdoan982 force-pushed the eng-2109-create-node-search-modal-with-ranked-results-and-preview branch from 84818c7 to c97cabe Compare August 22, 2026 04:27

vercel Bot temporarily deployed to Preview August 22, 2026 04:27 Inactive

Base automatically changed from eng-2108-add-ranked-search-to-queryengine-with-a-vault-iteration to main August 22, 2026 04:34

trangdoan982 and others added 14 commits August 22, 2026 00:34

Tighten comments on the node type badge and author name cache

39da641

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ENG-2109 Drop the remaining explanatory comments

447dd27

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ENG-2109 Address review: pointer guard, modal sizing utilities

470df4f

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

trangdoan982 force-pushed the eng-2109-create-node-search-modal-with-ranked-results-and-preview branch from c97cabe to 470df4f Compare August 22, 2026 04:34

vercel Bot deployed to Preview August 22, 2026 04:34 View deployment

vercel Bot temporarily deployed to Preview August 22, 2026 04:37 Inactive

trangdoan982 merged commit 9cb1a2c into main Aug 22, 2026
9 checks passed

Navigation Menu

Uh oh!

ENG-2109 Create node search modal with ranked results and preview - #1285

ENG-2109 Create node search modal with ranked results and preview#1285
trangdoan982 merged 15 commits into
mainfrom
eng-2109-create-node-search-modal-with-ranked-results-and-preview

trangdoan982 commented Aug 7, 2026 •

edited

Loading

Uh oh!

linear-code Bot commented Aug 7, 2026

Uh oh!

vercel Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

supabase Bot commented Aug 7, 2026

Uh oh!

devin-ai-integration Bot left a comment

Uh oh!

Uh oh!

Uh oh!

graphite-app Bot commented Aug 8, 2026

Uh oh!

mdroidian commented Aug 13, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

mdroidian Aug 13, 2026

Uh oh!

trangdoan982 Aug 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

trangdoan982 commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Scope check

What this does

Uh oh!

linear-code Bot commented Aug 7, 2026

Uh oh!

vercel Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

supabase Bot commented Aug 7, 2026

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

graphite-app Bot commented Aug 8, 2026

PR size/scope check

Uh oh!

mdroidian commented Aug 13, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

mdroidian Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

trangdoan982 Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

trangdoan982 commented Aug 7, 2026 •

edited

Loading

vercel Bot commented Aug 7, 2026 •

edited

Loading