perf(rewrite): reverse link index bounds re-resolution to the blast radius; index clone deleted (NRN-506) - #268
Merged
Merged
Conversation
…he change's blast radius (NRN-506) The apply-time overlay re-resolved every link in the vault for every touched path — O(vault links) per maintenance action, which architecture invariant 13 names a defect regardless of output correctness. It now re-derives only the affected set. `links::reverse` introduces the reverse index. Resolution is a pure function of four lookup tables, so every entry a link can read is named by a `LookupKey` (lowercased path, lowercased stem) and a vault path owns a fixed pair of them. A link whose key set is disjoint from the changed paths' keys cannot change its answer. That framing is what makes the radius exact in both directions the naive "re-resolve the touched document" shortcut gets wrong: a create can resolve a link that dangled elsewhere in the vault, and a delete can dangle one that resolved. Keys are computed without short-circuiting the ladder, so the recorded set is a superset of what any one vault state makes a link read — the over-approximation costs a redundant re-resolve, never a missed one. `resolve_links` gains a scoped sibling, `resolve_links_within`, and runs in two passes: pass one computes outcomes while the documents are borrowed immutably, pass two writes them back. The lookup tables now point into the graph instead of copying it, which deletes the per-document heading-slug and block-id clones the old single-pass shape needed. Semantics are unchanged — candidate lookups still see every document, so a created file is visible vault-wide; only the set of links re-derived is bounded. `graph::incremental_equivalence` is the proof obligation: create, delete, rename/move, alias add and remove, a new stem collision against a resolved target, growth and shrink of an ambiguous candidate set, heading/block churn under anchors, and a non-Markdown asset landing under a Markdown link — each asserts the overlaid graph is indistinguishable from a cold full walk of the same on-disk state, plus a seeded property test over random single-document changes. The post-create scope guard now also tallies links re-resolved. On the ~1500-doc fixture a single create re-resolves 2 of 2703 links (the created document's own link plus the referrer the reverse index finds), flat against the ~200-doc fixture's 2 of 411.
…place, never clones the index (NRN-506) `post_create_validate` deep-copied the whole graph index — every document's head and body text — so it could overlay one new file onto it and look at the result. On a ~1500-doc vault that copy cost ~0.8ms per create and bought nothing: the verb owns its loaded index and has no use for the pre-write state once the apply has landed, so the overlay can mutate it directly. Measured on the ~1500-doc fixture (2703 links), the pass drops from ~2.1ms (0.82ms clone + ~1.26ms whole-composite re-resolve) to ~1.3ms, and re-resolves 2 links instead of 2703.
…ped index clone (NRN-506)
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
…ed path, not the caller's spelling (NRN-506) overlay_changed_paths seeded its rescope set from the caller's changed-path spelling before parsing. A non-normalized spelling (e.g. ./probe.md) parses to a normalized document.path (probe.md), and resolve_links_within scopes by document.path — so a freshly created document whose changed-path entry was not already normalized never had its own outgoing links re-resolved, publishing Unresolved with no unresolved_reason instead of resolving. Insert each document's emitted path into rescope at push time instead of relying on the caller's affected set already covering it, and correct the stale comment claiming every parsed doc is already in scope via affected. Adds a regression case pinning the exact scenario: a baseline hub.md/other.md pair plus an overlay create at the non-normalized path ./probe.md whose own links target both. The wider fix to normalize the whole affected set (which also touches the pre-existing stale-duplicate retain bug) is tracked separately as NRN-524.
…lder and self-anchor coverage (NRN-506) The existing incremental-equals-rebuild property compares the overlay against a cold rebuild, which conflates two axes: whether the overlay chose the right SCOPE to re-resolve, and whether it landed on the right DOCUMENT SET (file identity / duplicate handling, tracked separately as NRN-524). A changed-path spelling variant or a phantom entry can trip the second axis without the scoping logic being at fault, so that axis can't be swept with those shapes. Add assert_overlay_scoping_is_sound: overlay onto the baseline, clone the result, run a full resolve_links pass over the clone, and assert link-for-link equality. Both sides see the identical overlaid document set, so this isolates the scoping axis cleanly. A 300-seed property sweep drives it with the same create/delete/rename/rewrite change vectors as the existing property, plus a ./-prefixed spelling variant and a phantom (never-existed) changed-path entry on top. Also: - Two coverage-gap cases the reviewers flagged: an embed created at the vault root (dir/a.md's ![[shared]] with shared.md at the root, so the root rung fires distinctly from the base rung) and heading churn against a document's own self-anchor link ([[#Heading]]). - The property generator's per-link shape match grows a wikilink-embed arm (![[target]]) and a self-anchor arm ([[#Heading]] naming the document's own heading), alongside the existing wikilink/folder-wikilink/markdown/anchor arms. - The case-collision exclusion comment on the generator's distinct_path now names the pinned defect test it dodges (case_only_duplicate_filenames_silently_overwrite_in_path_lower in links::resolve) and cites NRN-524, so a reader knows it avoids a known defect, not just a macOS filesystem quirk.
…t CHANGELOG, dead alias/comment cleanup (NRN-506) - reverse.rs module doc described a warm per-document-update capability that does not exist yet (build/record/sources_for_paths only — no remove, no public per-document update). Rewrite states the present-tense facts: the index is rebuilt fresh per overlay_changed_paths call in O(total links); record's per-document shape is what makes a FUTURE warm index cheap (NRN-523), not something the index does today. - CHANGELOG's NRN-506 entry now says plainly that the measured per-create win is dominated by the deleted GraphIndex clone, not by the resolve bound: the overlay still walks every link once per call to build the reverse index, so the change is not yet asymptotically cheaper on that axis — the resolve-bound half (2 of 2703 links) is real but rides alongside that per-call O(total links) build until a warm graph (NRN-523) lands. - docs/architecture.md invariant 13 drops "and aliases" from the blast-radius sentence — aliases left the resolution ladder in NRN-455, and the reverse index correctly records no alias keys (pinned by test). - mutate/new.rs: delete the stale "Alias checks are skipped (alias_field: None)" sentence — the post-create overlay receives owner_index_options(config), which propagates the configured alias_field like every other pass. Also rebind the mutated index reference to a distinctly named post_create_index at the post-create-validate call site, so the pre-write -> post-create meaning change on the same graph is structural, not a comment-only convention a later edit could silently violate. - reverse.rs: size sources_by_key's initial capacity from a link-derived estimate (sum of per-document link counts) instead of documents.len() — keys run 1-3 per link, so the old estimate undersized the map on any vault where documents carry more than a handful of links each.
…he code enforces (NRN-506)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Run A lane 2 of the phase-4 delivery (charter NRN-a56; design NRN-a55 §10, architecture invariant 13).
norn-core/src/links/reverse.rs): resolution entries are named by lookup keys (normalized path / stem, lowercased); each link records the superset of keys it can ever read (full ladder, no short-circuit), so a change's blast radius — links from the touched docs plus links to their keys — is exact in both hard directions (a create resolving a link that dangled elsewhere; a delete dangling one that resolved). Over-approximation costs a redundant re-resolve, never a miss.overlay_changed_pathsre-derives only the affected set via the scoped resolver (resolve_links_within); whole-graph candidate semantics untouched. Measured: 2 of 2703 links re-resolved per create on a 1523-doc fixture (guard-asserted with a witnessed reverse hit, not a vacuous zero).GraphIndexclone is deleted —post_create_validateoverlays in place on the verb-owned index (0.8 ms/create gone; no Arc machinery needed at this call shape).Honest measurements (release, 1523 docs / 2703 links)
post_create_validate: ~2.09 ms → ~1.31 ms; links re-resolved 2703 → 2.load_graph_index(~21 ms, called twice per confirmed mutation), filed as NRN-523.Also filed from this lane: NRN-524 (pre-existing case-fold stale-document overlay bug, confirmed on clean HEAD), NRN-525 (
publish_incrementstages the whole links table — invariant-13 class).Verification
Quartet clean (check --locked / clippy -D warnings / fmt / Cargo.lock untouched).
cargo test --workspace: 2090 passed, 0 failed with the pinned 0.48.1 oracle on PATH — including all norn-parity suites (self_check, consistency, ledger/drift gates). No parity drift. The local-machine oracle-pin skew (0.48.2 installed vs 0.48.1 pinned) is the known NRN-348/NRN-490 environmental issue.