Skip to content

resolver: note razor marker removals; make retention counters mean what they log - #591

Merged
zzet merged 1 commit into
zzet:mainfrom
pbednarcik:fix/retention-review-notes
Aug 17, 2026
Merged

resolver: note razor marker removals; make retention counters mean what they log#591
zzet merged 1 commit into
zzet:mainfrom
pbednarcik:fix/retention-review-notes

Conversation

@pbednarcik

Copy link
Copy Markdown
Contributor

Follow-up to the #531 review — the four non-blocking notes. (The blocking one and the
test-comment fix I see you already landed in 9c9f8c9, along with the CI guards in
1d4c9d9 — thanks, that saved me the biggest half of this list.)

  1. Razor marker removals now dirty the retention. The real adjacency mutation in
    razor_using.go is the RemoveEdge consuming the @using markers, and it had no
    removal helper — the note above it was inert, exactly as the review said. Added
    noteImportEdgeRemovals (removal-side sibling of noteImportEdgeReindexes: known
    file → per-file dirty, provenance-less → wholesale generation), wired it to the
    marker consumption, and dropped the inert call. The new test follows the pattern
    your pin test established: it drives the production resolveRazorUsings pass with
    no note* call in the test body, and the projected adjacency keeps the marker
    target unresolved so the file can only be served by the adjacency retention —
    watched it fail without the fix (projection calls = 1, want 2). The write-site
    audit table now covers RemoveEdge sites too and lists razor under removals;
    csharp_partial_merge.go's removals are classified kind-bounded (implements/extends
    moves between type nodes — imports edges never originate at a type).

  2. Counters now mean what they log. reach_fallback is a zap.Bool (it was a
    flag logged as a count), and reach_retained is renamed reach_occupancy — it
    reports the retention map's size, which is what it always measured; per-page
    retention hits were already reach_adj_cached. One log key changes name, so
    anything parsing the prepare line needs the new key.

  3. Identity-preserving skip: kept, re-commented. The comment now names the one
    shape that actually produces it — the deferred-LSP terminal clears, where
    terminality is meta-only so OldTo equals the target — and says it runs after the
    pass's final prepare. I kept the skip rather than removing it: it encodes a true
    invariant (an identity-preserving rewrite cannot change stored adjacency), and if a
    future writer produces the shape mid-pass, skipping is the correct behavior where
    removal would silently evict the files the retention serves.

  4. Cap comment matches what the cap now bounds — both retentions, with the
    adjacency entries called out as the heavy ones (~5 MB real, tens of MB at the cap,
    per your measurement).

Testing: the new test failed before the fix and passes after; ./internal/resolver
and ./internal/indexer FAIL name-sets on Windows are identical to clean main
(zero new, zero repaired); gofmt clean.

@zzet
zzet merged commit 0258e46 into zzet:main Aug 17, 2026
10 checks passed
@pbednarcik
pbednarcik deleted the fix/retention-review-notes branch August 17, 2026 09:28
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.

2 participants