Skip to content

fix(explore): a damped declaration file is a candidate, not a walk start (CG-28) - #2

Closed
bompus wants to merge 1 commit into
maxmilian:fix/1638-ts-interface-membersfrom
bompus:cg28/rwr-restart-vector
Closed

fix(explore): a damped declaration file is a candidate, not a walk start (CG-28)#2
bompus wants to merge 1 commit into
maxmilian:fix/1638-ts-interface-membersfrom
bompus:cg28/rwr-restart-vector

Conversation

@bompus

@bompus bompus commented Sep 6, 2026

Copy link
Copy Markdown

Closes the CG-28 ranking failure in one line. __tests__/explore-declaration-only.test.ts goes to 12 passed / 0 failed, and the full suite against this base goes 31 failed → 30 failed with the CG-28 gate as the only difference in the failing set.

The mechanism

On a prose flow query, a declaration-only file outranks the implementation it declares. The damage enters through the restart vector, not through connectivity.

contains is not in RANK_EDGES, so a declared member is nearly isolated in the walk graph and carries almost no mass of its own. Its whole effect is that it occupies a seed — and the restart vector is uniform over seeds, so every member divides the restart mass the implementation files are competing for. Since colbymchenry#1638 a platform .d.ts contributes one seed per member, with names (body, stream, metadata) that are exactly what a prose flow query matches. That is the 0.307 → 0.137 halving of an implementation file's mass while the shim's holds steady.

So: filter the damped files out of the seed set only. They stay candidates, stay reachable, keep their score contribution. This changes where the walk starts and nothing else.

The predicate is isDampedDeclaration rather than a bare ambient test, and that is what makes the counter-case work: it already exempts a file whose declared type the query named. On a query genuinely about the declared type the shim keeps its seeds and still ranks 1, at mass 1.0.

Result on the fixture

Flow query, ambient-decls-ts:

file before after rank
src/storage/metadata.ts 0.137461 0.504025 2 → 1
src/storage/stream.ts 0.058601 0.214871 4 → 2
types/platform-shims.d.ts 0.184398 0.009459 1 → 3, still named

Verified on a real repo

The fixture cannot say what the change costs on a codebase nobody built it for, so I ran it against vitejs/vite — 1,719 files, 13,793 nodes, 32,822 edges. The commit touches src/mcp/tools.ts only, so one index serves every arm and ranking is the sole variable; two runs of the same build give 0 changed rows.

  • Four prose flow queries: 0 changed rows against the unpatched base. No vite query reaches the damped-declaration path, because vite's .d.ts files are either named by the query (exempt) or already below implementation.
  • Type counter-case (the HMRPayload type and its update variants): packages/vite/types/hmrPayload.d.ts stays rank 1 at mass 0.185539, identical patched and unpatched.

The change is inert where it isn't needed, and that's the property I most wanted evidence for.

What I removed from this PR

This opened with two commits. The first one — dropping interface-owned method seeds — turned out to be both unnecessary and harmful, and the vite run is what showed it:

file base with that commit rank
packages/vite/src/node/server/hmr.ts 0.361433 0.258473 1 → 2
playground/vitestSetup.ts 0.181818 0.285714 2 → 1

A playground test-setup file displacing the HMR server implementation is a worse answer, and the cause is this same mechanism pointed the other way: dropping interface-owned method seeds re-normalizes the restart vector and moves mass off interface-heavy implementation files (hmr.ts declares HmrContext and friends). Meanwhile the CG-28 gate passes 12/12 without it. Deleting it costs nothing I can measure; keeping it costs one real query — so it's gone, and this PR is the single line the evidence actually supports.

Correction to my first version of this PR

I wrote that "rank is ordered by graphScore, not score". The second half is right, the first is not, and the data in my own table showed it: src/lib/bucket.ts at rank 5 carried more graph mass than platform-shims.d.ts at rank 3.

The comparator tiers firstpinned, named, central, entry — and orders by graphScore within a tier. Entry files fill the top ranks by descending mass, non-entry the rest by descending mass.

What survives, and is the load-bearing part: score does not order the list. The shim lost on score (26.25 against 27) and still took rank 1, so the RELEVANCE_KIND_WEIGHT and named-FIRST-tier mitigations — which move score and the seed tier — cannot reach this failure. graphScore is what has to move, and this is the change that moves it.

@bompus bompus changed the title CG-28 diagnosis: rank is ordered by graphScore, and the restart vector is where the members hurt fix(explore): close the CG-28 ranking failure — a declaration file is a candidate, not a walk start Sep 6, 2026
@bompus
bompus marked this pull request as ready for review September 6, 2026 05:51
@bompus

bompus commented Sep 6, 2026

Copy link
Copy Markdown
Author

Verified on a real repo — and it says to drop the first commit

The fixture proved the fix works. It could not say what the change costs on a codebase nobody built it for, so I ran it against vitejs/vite (shallow clone: 1,719 files, 13,793 nodes, 32,822 edges).

Method: the two commits touch src/mcp/tools.ts only, so one index serves every arm — ranking is the sole variable. Four prose flow queries plus one type query as a counter-case. Two runs of the same build produce 0 changed rows, so the numbers below are deterministic, not sampled.

arm contents
base 308d97c
perf + a9655a2 (interface-owned method seeds)
fix + 88f9540 (damped declaration seeds) — this PR
solo base + 88f9540 only

1. The CG-28 fix costs nothing here

fix vs perf is byte-identical across all five queries, and solo vs base is 0 changed rows. Both directions agree: no vite query reaches the damped-declaration path, because vite's .d.ts files are either named by the query (already exempt) or already below implementation. The fix is inert where it isn't needed — which is what I wanted to know and could not learn from a fixture built to trigger it.

2. The counter-case survives on a real repo

Query: the HMRPayload type and its update variants

packages/vite/types/hmrPayload.d.tsrank 1, graph mass 0.185539, identical in all four arms. A declaration file still wins when the query is about the declared type.

3. a9655a2 has a real cost, and it isn't paying for anything

The entire real-repo delta comes from the other commit, on one query — how does the dev server send an hmr payload to the browser:

file base with a9655a2 rank
src/node/server/hmr.ts 0.361433 0.258473 1 → 2
playground/vitestSetup.ts 0.181818 0.285714 2 → 1
src/node/server/send.ts 0.092569 0.141867 5 (unchanged)
src/types/ws.d.ts 0.090909 0.142857 3 (unchanged)

A playground test-setup file displacing the HMR server implementation is a worse answer. The mechanism is the same one this PR is about, pointed the other way: dropping interface-owned method seeds re-normalizes the restart vector, so mass moves away from interface-heavy implementation files — hmr.ts declares HmrContext and friends — and toward files whose seeds all survived. send.ts rising is the change working as intended; vitestSetup.ts taking rank 1 is it overshooting.

And it isn't load-bearing: solo passes explore-declaration-only.test.ts 12/12. Same-clone full-suite control, base vs solo: 32 failed → 31 failed, and the only set difference is the CG-28 gate flipping to pass, plus two known flakes (cli-install-init fails in base only; mcp-daemon's lockfile race fails in solo only and passes 3/3 in isolation).

Delete a9655a2 and nothing I can measure gets worse; keep it and one real query gets a worse top hit. I'd rather this PR be the one line that's actually justified, so I'm proposing to reduce it to that.

4. My own defect, for the record

The pushed commit carries a leftover // EXPERIMENT 3 marker on the new line. That's debug residue that should never have left my machine; it goes away with the cleanup above.

I'll hold the rewrite until there's a view on it — say the word and this becomes a single commit with the marker gone, or I'll leave the branch as-is and drop a9655a2 separately if you'd rather keep the history.

CG-28: on a prose flow query, a declaration-only file outranks the
implementation it declares. The damage enters through the RWR restart
vector, not through connectivity. `contains` is not a RANK_EDGE, so a
declared member is near-isolated and carries almost no walk mass of its
own — but the restart vector is uniform over seeds, so since colbymchenry#1638 a
platform `.d.ts` contributes one seed per member, and those member names
(`body`, `stream`, `metadata`) are exactly what a prose flow query
matches. Every such seed divides the restart mass the implementation
files are competing for. That is what halves an implementation file's
graph mass while the shim's holds steady.

Filter the damped files out of the seed set only. They stay candidates,
stay reachable, and keep their `score` contribution; this changes where
the walk starts and nothing else. The predicate is `isDampedDeclaration`
rather than a bare ambient test because it already exempts a file whose
declared type the query named — so the counter-case holds: on a query
about the declared type the shim still ranks first, at mass 1.0.

Fixture (ambient-decls-ts), flow query:
  storage/metadata.ts   0.137461 -> 0.504025   rank 2 -> 1
  storage/stream.ts     0.058601 -> 0.214871   rank 4 -> 2
  platform-shims.d.ts   0.184398 -> 0.009459   rank 1 -> 3, still named

__tests__/explore-declaration-only.test.ts: 12 passed, 0 failed.
Full suite against this base: 31 failed -> 30 failed, and the CG-28 gate
is the only difference in the failing set.

Verified on vitejs/vite (1,719 files, 13,793 nodes, 32,822 edges), one
index shared across arms so ranking is the only variable: zero changed
rows against the unpatched base on four prose flow queries, and the type
counter-case keeps types/hmrPayload.d.ts at rank 1 (mass 0.185539).
The change is inert where it is not needed.
@bompus
bompus force-pushed the cg28/rwr-restart-vector branch from 88f9540 to 53da6dc Compare September 6, 2026 06:14
@bompus bompus changed the title fix(explore): close the CG-28 ranking failure — a declaration file is a candidate, not a walk start fix(explore): a damped declaration file is a candidate, not a walk start (CG-28) Sep 6, 2026
@maxmilian

Copy link
Copy Markdown
Owner

Merged into fix/1638-ts-interface-members as 4f8d192 — cherry-picked, so your authorship is intact but GitHub can't detect it and auto-close; closing by hand. Upstream colbymchenry#1686 now carries it.

Verified on the merged branch myself rather than quoting your numbers: npx vitest run __tests__/explore-declaration-only.test.ts12 passed / 0 failed. The full suite is 4228 passed / 4 failed / 11 skipped, and the 4 are the pre-existing Dart parity assertions (confirmed by rebuilding the kernel against 948e455 and seeing the identical failures) — so the CG-28 gate is the only change in the failing set, as you said.

The part I most appreciate is the part you removed. Diagnosing that the damage enters through the restart vector rather than connectivity is what made this one line instead of a penalty retune, and then dropping the interface-owned-method-seed commit because the vite run showed it demoting hmr.ts for vitestSetup.ts — cutting your own work on evidence that only you would have seen — is the reason I merged this without hedging. Thank you.

@maxmilian maxmilian closed this Sep 7, 2026
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