ENG-2248 Add a cached relations index and link resolution - #1433
ENG-2248 Add a cached relations index and link resolution#1433trangdoan982 wants to merge 4 commits into
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Reading relations from disk costs a full vault file read plus a JSON parse per call. That is fine for the Discourse Context panel, which asks once per file open, but not for anything that renders per link. RelationsIndex keeps a parsed snapshot grouped by endpoint id so a render path can ask synchronously, rebuilt from vault events so it covers our own writes and edits arriving over sync alike. resolveDiscourseLinkTarget answers, for one link, whether it points at a discourse node and how many relations that node has — from in-memory caches only, avoiding getNodeTypeIdForFile, which polls 500ms waiting on frontmatter. Link parsing moves to internalLinkParsing, which wikilinkDragHandler had its own byte-identical copy of. Nothing consumes this yet; the overlay that does follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two review findings: Renaming relations.json out of the root, or another file into it, left the snapshot stale; only modify/create/delete were watched. Markdown links with a #heading or #^block subpath never matched, since the pattern required .md immediately before the closing paren. Wikilinks with subpaths already worked, so the two forms behaved differently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review finding: an invalidation arriving mid-read starts a newer load, but the superseded load's finally cleared inFlight unconditionally, discarding the newer load's tracking. Later callers then saw no load in flight and started redundant ones. Clearing is now gated on the generation still matching, which is the same signal that decides whether the load's result is worth keeping. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
509543f to
bbab339
Compare
|
@coderabbitai full review |
|
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
First of three stacked PRs for ENG-1249, tracked by ENG-2248. Stack: this → ENG-2249 → ENG-2250.
https://www.loom.com/share/2f5badd2168546dba687e2844ebc2d46
First of three stacked PRs for ENG-1249. Stack: this → ENG-2249 → ENG-2250.
Reviewer brief
Result: No user-visible change. This adds the data layer the overlay needs; nothing consumes it yet.
Review focus: the concurrency rules in
RelationsIndex, and what the relation count deliberately excludes. Both are expanded below.Why an index exists at all — the cost argument
getRelationsForFilereads and parses the whole ofrelations.json, then scans every relation in it:That is fine for the Discourse Context panel, which asks once per file open. The overlay asks once per discourse-node link on screen, per viewport update. On a note with 40 such links that is 40 file reads and 40 parses to answer 40 questions about one document — on every keystroke.
RelationsIndexkeeps a parsed snapshot grouped by endpoint id, so a lookup is aMaphit and answers synchronously — which matters as much as speed, because a CodeMirrorViewPlugincannot await anything while building decorations.flowchart LR subgraph store["relations.json"] R1["r1: a to b"] R2["r2: c to a"] R3["r3: a to a"] end store --> IDX["endpoint index<br>a → r1, r2, r3<br>b → r1<br>c → r2"] IDX --> Q["get('a') → 3<br>O(1), no await"]Each relation is filed under both endpoints, so "relations touching X" is one lookup regardless of direction. A self-relation like
r3is filed once, not twice, so one endpoint never yields it twice.The concurrency rules — each one is load-bearing
inFlightcleared in afinallyensureLoadedhands out an already-settled promise forever — the snapshot stays stale and every later read requests a load that never runs.initialize()andinvalidate()only.0until the reload lands — on the very action that triggered it, since saving a relation writesrelations.json.ViewPlugin, which can only observe transactions, can detect that counts changed. Used by ENG-2249.What the count excludes, and why it must
"How many relations does this node have?" is ambiguous. A node in the test vault has 6 relations in
relations.json; the panel lists 4.Deleting a relation type does not delete its relations; they stay as orphans and the panel drops them while grouping by type. Imported relations awaiting acceptance (
tentative === false) are likewise listed separately.Verification
pnpm install --frozen-lockfile+pnpm ci:validate: 8/8 check-types, 5/5 test:unit.Scope check
$scope-checkagainst the ENG ticket and final diff.Done When: None. Groundwork for the ticket's overlay requirement.Local delegated full review
Review findings addressed here:
relations.jsonrenames now invalidate the snapshot, and markdown links with a#headingsubpath now match. One finding is open by design — see the thread ondiscourseLinkFrontmatter.ts.🤖 Generated with Claude Code