Skip to content

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

Open
trangdoan982 wants to merge 12 commits into
eng-2108-add-ranked-search-to-queryengine-with-a-vault-iterationfrom
eng-2109-create-node-search-modal-with-ranked-results-and-preview
Open

ENG-2109 Create node search modal with ranked results and preview#1285
trangdoan982 wants to merge 12 commits into
eng-2108-add-ranked-search-to-queryengine-with-a-vault-iterationfrom
eng-2109-create-node-search-modal-with-ranked-results-and-preview

Conversation

@trangdoan982

@trangdoan982 trangdoan982 commented Aug 7, 2026

Copy link
Copy Markdown
Member

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


Open in Devin Review

@linear-code

linear-code Bot commented Aug 7, 2026

Copy link
Copy Markdown

ENG-2109

@vercel

vercel Bot commented Aug 7, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
discourse-graph Skipped Skipped Aug 10, 2026 3:19pm

Request Review

@supabase

supabase Bot commented Aug 7, 2026

Copy link
Copy Markdown

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-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment thread apps/obsidian/src/components/NodeSearchModal.tsx
Comment thread apps/obsidian/src/components/NodeSearchModal.tsx
@trangdoan982
trangdoan982 force-pushed the eng-2108-add-ranked-search-to-queryengine-with-a-vault-iteration branch from 44c784b to 07bb663 Compare August 8, 2026 17:32
@trangdoan982
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
@graphite-app

graphite-app Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

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

@trangdoan982
trangdoan982 force-pushed the eng-2108-add-ranked-search-to-queryengine-with-a-vault-iteration branch from 07bb663 to f89d51d Compare August 9, 2026 03:55
@trangdoan982
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
@trangdoan982
trangdoan982 force-pushed the eng-2108-add-ranked-search-to-queryengine-with-a-vault-iteration branch from f89d51d to 79938ba Compare August 9, 2026 17:22
@trangdoan982
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
@trangdoan982
trangdoan982 force-pushed the eng-2108-add-ranked-search-to-queryengine-with-a-vault-iteration branch from 79938ba to 9708e95 Compare August 9, 2026 17:28
trangdoan982 and others added 5 commits August 9, 2026 13:29
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>
@trangdoan982
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
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>
Comment thread apps/obsidian/src/components/NodeSearchModal.tsx Outdated
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>
Comment thread apps/obsidian/src/components/NodeSearchModal.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant