Skip to content

fix(extraction): index TypeScript interface members (#1638) - #1686

Open
maxmilian wants to merge 1 commit into
colbymchenry:mainfrom
maxmilian:fix/1638-ts-interface-members
Open

fix(extraction): index TypeScript interface members (#1638)#1686
maxmilian wants to merge 1 commit into
colbymchenry:mainfrom
maxmilian:fix/1638-ts-interface-members

Conversation

@maxmilian

@maxmilian maxmilian commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Closes #1638.

tree-sitter-typescript spells interface members with their own node types — method_signature and property_signature — distinct from the class-member types the TS extractor listed, so an interface's members never entered the graph. Java and C# were never affected: their grammars reuse method_declaration, which is already in their methodTypes.

The cost lands on any codebase whose platform API is a .d.ts interface. With no declaration node for a member, every call site through that API has nothing to attach an edge to.

The fix is two lines in typescript.ts. The walker already treats an interface as a class-like parent (isInsideClassLikeNode lists 'interface'), so members attach with no traversal change. Precedent: extractTsTypeAliasMembers already makes type X = { foo(): T } members first-class (#359) — interfaces were the inconsistent gap.

Three consequences, each handled and tested:

  1. A bodiless signature must not take extractMethod's "no class-like parent → free function" fallback. Outside an interface such a node appears only in a type literal, whose members TypeScript type alias members not used in method-call resolution → false cross-module calls edges via path-proximity #359 already extracts — without a guard, type Handle = { stop(): void } gains a phantom top-level function stop.
  2. The property_signature / method_signature branch that hung type annotations off the enclosing interface is now unreachable (the new branches carry the same guard and claim those types first) and is removed. The references edges survive and now anchor on the member, so Api::fetch → PageId says which member wants the type where Api → PageId only said the file did.
  3. CG-28 — see below.

CG-28: a proposal, not an assertion

getAmbientDeclarationPathsAmong reads "every declared symbol is type-level", which a pure-interface .d.ts stops satisfying the moment its members are indexed.

Condition 4 turned out to be the dangerous one, not condition 2. Condition 2 breaks loudly. Condition 4 — "nothing else in the index points at it" — breaks silently and backwards: once members exist, call sites through a platform API finally have a signature to land on, so an ambient shim loses its damping precisely because the API it declares is widely used.

This PR makes an interface-owned member transparent to conditions 2, 3 and 4 — the treatment parameter already gets ("structural bookkeeping, neither qualifies nor disqualifies"), via one shared IS_INTERFACE_MEMBER(alias) fragment seeking idx_edges_target_kind. The reasoning is that this reproduces the graph the rule was measured on: before this change those nodes did not exist. The interface itself is untouched, so a depended-on types.ts still fails condition 4 and stays out of the flag.

I cannot verify that from outside. The 0–4% flag rate is a corpus measurement, and whether folding members in keeps it there needs a run against the corpus the comment cites — Kotlin sealed classes, Rust mod.rs re-exports, django locale tables. Please treat this half as a proposal and re-measure.

One test still fails, and I would rather flag it than paper over it

__tests__/explore-declaration-only.test.ts > the gate — a prose flow query > does not let it outrank the implementation files.

Detection is correct: the shim is still flagged ambient and still damped at 0.5. Magnitude is not. The fixture's shim holds 143 nodes where it held 29, and the extra bodiless names flood FTS, shifting the RWR restart vector — implementation files' graph mass halves (0.307 → 0.137) while the shim's holds steady, and it takes rank 1.

I fixed what I could argue from the code's own stated intent, both in tools.ts:

  • RELEVANCE_KIND_WEIGHT rates method at 1.0 as "a callable", and its own doc puts "a member of a type" at ~0.5. A method_signature is the latter. This restores the raw score almost exactly (53 → 52.5) and cuts the rendered envelope from 6044 to 1933 characters.
  • The named-seed tier means "the agent asked for the symbol defined here". Prose words like body, stream and metadata are member names in any platform .d.ts and were reaching that tier through coNamedInFile, sorting above the CG-28 penalty. Interface signatures now seed (RWR unchanged) but never tier.

Closing the rest means either retuning AMBIENT_DECLARATION_RANK_PENALTY — documented as "softer than GENERATED on purpose" — or changing what FTS returns. Both are calibration decisions that belong to whoever holds the corpus, so I stopped there rather than adjusting the assertion. Happy to take direction.

Tests

Baseline on b9ca4b7: 3950 pass / 84 fail (all 84 are CLI/MCP/spawn suites needing a built dist/, none touching extraction or queries). After: 3952 / 85 — failure-list diff is exactly the one entry above, with none disappearing.

Added:

  • interface members enter the graph (the issue's repro), and attach to the interface by a contains edge rather than merely existing;
  • no phantom top-level function from a type-literal signature;
  • a guardrail that a pure-interface .d.ts is still CG-28-damped, pinned from both ends — it asserts members are indexed as well as the flag, so it cannot pass vacuously on an index where extraction silently reverted.

Adjusted, in each case re-keyed on source rather than name:

  • extraction.test.ts [TypeScript] String literal type arguments in generic tuple elements are not indexed as symbols #634 excluded nodes reached by a contains edge from an interface; a node minted from Pick<User,'id'> or a tuple has no declaring interface, so the guard is intact.
  • object-literal-methods.test.ts — its Zustand fixture declares both interface Store { fetchUser(): … } and an action of that name, so find(n => n.name === 'fetchUser') started hitting the signature.
  • explore-declaration-only.test.ts's two fixture-shape assertions describe extraction output and must track it; they now accept interface-owned members via an isTypeLevel helper. A function or class creeping into that fixture still fails, and the gate assertions themselves are untouched.

⚠️ Scope limit: as it stands this fix only reaches the wasm path

Raised by @bompus in review, verified against current main, and it is the first thing a reviewer should know.

The change here is entirely on the TypeScript side (src/extraction/tree-sitter.ts, src/extraction/languages/typescript.ts). But typescript and tsx are both in DEFAULT_ROUTED (src/extraction/kernel/index.ts:37), so when a kernel binary is present the Rust walker replaces extraction outright — and is_method_type (codegraph-kernel/src/tsjs/mod.rs:56) matches only method_definition and TS public_field_definition, with no property_signature/method_signature anywhere.

That is not an edge case. .github/workflows/release.yml:33 states that as of 1.5.0 the kernel is the release's headline, not an optional extra; the prebuild matrix is required and the binaries ship under release/kernel/. So the wasm path this PR fixes is the fallback, and on a released install the change is currently inert.

Nor do the existing guards catch it: the loader compares node/edge tables plus an ABI version (src/extraction/kernel/loader.ts:132), which a divergence in extraction logic passes silently. scripts/kernel-parity.mjs is the tool that would have caught it, and is the acceptance test the port should be judged on.

@bompus is opening the Rust mirror as a PR against this branch (fix/1638-ts-interface-members) so the two halves stay one reviewable unit. This PR should not be merged until that lands — merging the TS half alone would advertise a fix that a released install does not get.

Also folded in: signature: "counts counts" on interface properties

Found by @bompus while porting. isTsJsField (tree-sitter.ts:2039) gates extractProperty's narrowing on public_field_definition/field_definition, so a property_signature falls through to the generic named-child scan — whose exclusion list covers identifier but not property_identifier. The scan therefore stops on the name node and the type annotation is never read, so interface Stats { counts: Record<string, number> } yields signature: "counts counts".

This is a gap rather than a decision: #808 targeted field definitions carrying initializer values, and interface members could not reach that code path when it was written.

It is fixed here rather than in a follow-up because of blast radius. Before this PR no node existed for a property_signature on either path, so reading its type field changes the signature of nothing that ships today — the whole affected set is nodes this PR introduces. Landing them with a known-wrong field and correcting it later is the worse trade. The fix is gated on property_signature specifically, so no other language's property_declaration scan moves.

tree-sitter-typescript spells interface members with their own node types,
`method_signature` and `property_signature`, distinct from the class-member
types the TS extractor listed — so an interface's members never entered the
graph. Java and C# were never affected: their grammars reuse
`method_declaration` for interface methods, which was already in methodTypes.

The cost lands on any codebase whose platform API is a `.d.ts` interface.
With no declaration node for a member, every call site through that API has
nothing to attach an edge to, so the calls are invisible to callers/impact.

Adds `method_signature` to methodTypes and `property_signature` as the TS
`propertyTypes`. The walker already treats an interface as a class-like
parent, so members attach to their interface with no traversal change.

Three consequences handled here:

- A bodiless signature must not take extractMethod's "no class-like parent,
  so treat it as a free function" fallback. Outside an interface it appears
  only in a type literal, whose members extractTypeAlias already extracts
  (colbymchenry#359) — without the guard `type Handle = { stop(): void }` gains a phantom
  top-level `function stop` beside the real `Handle::stop`.

- The `property_signature`/`method_signature` branch that hung type
  annotations off the enclosing interface is now unreachable and removed. The
  `references` edges survive via extractMethod/extractProperty and now hang
  off the member, a more precise anchor.

- CG-28's ambient-declaration rule reads "every declared symbol is
  type-level", which a pure-interface `.d.ts` stops satisfying the moment its
  members are indexed. An interface-owned member is now transparent to all
  four conditions, so the rule keeps measuring what it was measured on.
@danusha2345

Copy link
Copy Markdown
Contributor

Read through this while assembling a local integration of the open fixes. The two-line extractor change and the SIGNATURE_METHOD_NODE_TYPES guard look right, and anchoring the references on the member is a real improvement. Two things kept me from taking it into the local build as-is:

  1. The still-failing explore-declaration-only case: the 29 → 143 node jump on the fixture shim is the same thing that will happen to every real .d.ts in a project, so the ranking shift is the production effect, not a fixture artefact. A member weight of 0.5 restores the score but not the rank, as you note.
  2. The CG-28 IS_INTERFACE_MEMBER transparency is reasoned well, but it changes a corpus-measured rule; it wants the re-measure you ask for before it ships.

Might be worth splitting: the extractor + parity tests as one PR (unambiguous win, easy to review), and the ranking / CG-28 half as a follow-up with the corpus numbers.

@maxmilian

Copy link
Copy Markdown
Contributor Author

Thanks for reading it that carefully, @danusha2345 — and I agree with your first point without reservation. The 29 → 143 jump is the production effect, not a fixture artefact: any real .d.ts gains a node per member, so the FTS dilution follows every project that has one. I would rather that sat in the PR body as a known cost than be discovered by someone downstream.

On splitting, I tried the shape you describe and it does not produce the clean first half either of us would want. The measurements:

  • Extractor + guard + test parity, without the CG-28 change: 7 new failures, six of them in explore-declaration-only. The moment interface members are indexed, a pure-interface .d.ts stops satisfying condition 2 ("every declared symbol is type-level"), so the ambient penalty stops applying. That is not a ranking nicety — it is the damping rule silently switching off, and it is worse than the ranking shift.
  • Extractor + guard + tests + IS_INTERFACE_MEMBER transparency (this PR): 1 new failure, the ranking one.

So the CG-28 half is not a follow-up that can wait — it is what keeps the first half from regressing the rule outright. And the failing test lands in whichever PR carries the extractor change, because the extra nodes are what shifts the RWR restart vector. A split moves the red from one PR to another rather than isolating it.

What is separable is the two tools.ts mitigations (the method_signature relevance weight, and signatures seeding without earning the named-FIRST tier). Those are ranking-only, and I would happily lift them out if @colbymchenry would rather review them apart from the extraction change.

The re-measure you and I both want is the same one: the 0–4% flag rate against the corpus the comment names. I cannot run that from outside, but if it would help your local integration, I can produce before/after node-count and flag-rate numbers on whatever repos you point me at.

@danusha2345

Copy link
Copy Markdown
Contributor

Took you up on the offer, but ran it here instead so the numbers come from repos you cannot see. Method: this PR merged onto current main (+ the open fixes I already carry), then for each repo a fresh index -f with the baseline build and with the PR build, and getAmbientDeclarationPathsAmong(<every indexed file>) as the flag-rate probe. The middle row is your "extractor without the CG-28 half" experiment — the PR's index queried by the baseline's SQL.

repo files nodes before → after interface members flagged before extractor-only (old rule) this PR
codegraph itself (TS, 6 .d.ts) 786 17,611 → 22,046 (+25%) 21 → 4,344 1 (0.13%) 0 1 (0.13%)
Android app: Kotlin + Go + TS (Wails) 274 5,022 → 5,022 43 → 43 0 0 0
Betaflight fork (C, 4k files) 4,055 88,135 → 88,135 0 6 (0.15%) 6 6
small TS/Dart app 51 958 → 958 0 0 0 0

So, on this sample:

  • The split really does not work — your "7 failures" is visible as a number: with the members indexed but the old rule, the only genuinely ambient shim in the TS repo (platform-shims.d.ts, the fixture) loses its flag. With the IS_INTERFACE_MEMBER transparency it keeps it, and nothing else gains one. Flag rate is unchanged on all four repos, which is the re-measure this half needed. Withdrawn: I'd take the PR as one piece.
  • Kotlin/Java/Go are untouched: their interface members were already nodes (43 here), and making them transparent flips no file. Same for C, which has no interface kind at all.
  • The cost is where you said: on an interface-heavy TS repo the graph grows a quarter (4.3k bodiless members). That is the FTS dilution behind the one remaining red test, and it will be felt in codegraph_explore on any repo whose platform API is a big .d.ts — worth stating in the PR body as the known trade-off, together with the two tools.ts mitigations that soften it.

Happy to re-run on other shapes if the maintainer names a corpus.

@bompus

bompus commented Sep 5, 2026

Copy link
Copy Markdown

Verified on a real TypeScript repo (Chrome MV3 extension, 582 files, TS/JS/Vue/markdown, Windows 11, tree-sitter wasm walker, kernel off). Branch: this PR merged onto current main (b9ca4b7) plus our fork's markdown/literal extras; control build indexed the same tree without the PR.

Merges clean onto main. Index of our repo before/after:

metric main + #1686
nodes / edges 12,323 / 39,337 13,021 / 40,288
interfaces with members 0 of 101 101 (698 member nodes)

PR test files on Node 22 (bundled runtime): 648 pass, 1 fail — explore-declaration-only CG-28, the declaration-only collision the issue text already names, so it looks like a known gap rather than a regression.

One side effect worth a look before merge: imports edges whose target is a property/method node (the #1537 shape) go from 19 to 61 on our tree, because the new interface member nodes are now candidates for the name-only import resolver. #1538 fixes that resolver, and with both applied the count is 0, so they land best together.

@bompus

bompus commented Sep 6, 2026

Copy link
Copy Markdown

Heads up on a gap this PR will hit in practice, and an offer of the missing half.

The fix lands on the wasm path only. The changed files are all TS-side (extraction/tree-sitter.ts, extraction/languages/typescript.ts, …) with nothing under codegraph-kernel/. But typescript/tsx are in DEFAULT_ROUTED, so on any install where a codegraph-kernel.node is present the Rust walker replaces extraction entirely and interface members stay unindexed — the PR's own extraction.test.ts cases pass on a wasm-only checkout and silently do nothing on a kernel build. The loader's guard is ABI version plus byte-equal NodeKind/EdgeKind tables, so a divergence in extraction logic like this one is not detected.

I ported the mirror while verifying a fork on the kernel path. Offering it here rather than as a competing PR — it is your change, and it wants to ride the same branch. The Rust side is four edits in codegraph-kernel/src/tsjs/:

  1. method_signature joins is_method_type (mirrors typescriptExtractor.methodTypes).
  2. New is_property_type for property_signature (mirrors propertyTypes). It carries no value, so it is always a property and never goes through classify_ts_class_member.
  3. New is_signature_method_type, guarding the method branch with && (!is_signature_method_type(kind) || self.inside_class_like()). This mirrors SIGNATURE_METHOD_NODE_TYPES: a method_signature must not take extract_method's "no class-like parent, so treat it as a free function" fallback, because outside a class it only appears in a type literal (type Handle = { stop(): void }) whose members extract_ts_type_alias_members already attaches to the alias (TypeScript type alias members not used in method-call resolution → false cross-module calls edges via path-proximity #359) — take the fallback and you get a phantom top-level function stop beside the real Handle::stop.
  4. The old branch that matched property_signature | method_signature together and hung their type annotations off the enclosing interface is replaced by the property branch. The references edges survive (both extract_method and extract_property call extract_type_annotations) but now anchor on the member — Api::fetch → PageId instead of Api → PageId.

Happy to open it as a PR against fix/1638-ts-interface-members, or paste the diff here, whichever you prefer.


One thing the port surfaced that is a question for the TS side. Routing interface members through extractProperty exposes them to #808's narrowing: the explicit type field is read only for public_field_definition / field_definition, and everything else falls back to the generic named-child scan. A property_signature is not a field definition, so it takes that scan — and the scan stops on the property_identifier, so the member's signature comes out as the name repeated. For

interface Stats {
  counts: Record<string, number>;
}

the extracted counts gets signature: "counts counts" rather than the type annotation.

That is what this PR ships today; I mirrored it in Rust deliberately, because kernel/wasm parity is the harder contract and my parity suite fails the moment the two disagree. But it looks unintended — reading type_annotation for signature members would give the real type, and #808's narrowing was aimed at other languages' field nodes, not at interface members, which did not reach this code path when it was written. If you agree it should be fixed, it has to change both sides in the same commit or the kernel and wasm paths diverge again; I can supply the Rust half of that too.

@maxmilian

Copy link
Copy Markdown
Contributor Author

Thanks — I verified both points against current main and you are right on both, including on the part that matters most for whether this PR does anything at all.

On the kernel gap: conceded, and it is worse than "an install where a kernel is present." DEFAULT_ROUTED (src/extraction/kernel/index.ts:37) carries typescript/tsx, and is_method_type (codegraph-kernel/src/tsjs/mod.rs:56) matches only method_definition and TS public_field_definition — no property_signature/method_signature anywhere on the Rust side. And .github/workflows/release.yml:33 states that as of 1.5.0 the kernel is the release's headline, with the prebuild matrix required and the binaries bundled under release/kernel/. So the wasm path this PR fixes is the fallback, not the norm: as it stands the change is inert for a released install. That belongs in the PR body, and I will put it there rather than leave it for a reviewer to discover.

Your read of the loader guard is also right — sameTable(info.nodeKinds, …) plus the ABI version (loader.ts:132) compares the tables, so a divergence in extraction logic passes it silently. scripts/kernel-parity.mjs is what would have caught this, and it is the acceptance test I would want the port to be judged on.

Please open it as a PR against fix/1638-ts-interface-members. A diff in a comment gets lost, and this way the port carries your authorship and the branch stays one reviewable unit for @colbymchenry. Your four edits are the same four I would have written; the is_signature_method_type guard in particular has to exist, because inside_class_like() (mod.rs:287) already treats interface as class-like, so without it a bare type Handle = { stop(): void } takes the free-function fallback on the kernel path exactly as it did on the wasm one. I will review it and run the parity sweep on my end.

On signature: "counts counts" — agreed, it is unintended, and I think it belongs in this PR rather than a follow-up. extractProperty's narrowing gates on node.type === 'public_field_definition' || 'field_definition' (tree-sitter.ts:2039); a property_signature misses it and takes the generic named-child scan, whose exclusion list covers identifier but not property_identifier — so the scan stops on the name and the type annotation is never read. #808 was aimed at field definitions with initializer values, and interface members did not reach that code path when it was written, so this is a gap rather than a decision.

The reason to fix it here rather than later: before this PR, no node existed for a property_signature on either path, so reading its type field changes the signature of nothing that ships today — the entire blast radius is nodes this PR introduces. Shipping them with the name doubled would mean landing a known-wrong field and then correcting it, which is a worse trade than one slightly larger diff. I will keep the fix gated on property_signature specifically so no other language's property_declaration scan moves.

So: yes to the Rust half of that too, in the same commit as the TS half. I will hold off touching the TS side of the signature fix until your PR is up, so the two land together instead of racing.

@bompus

bompus commented Sep 6, 2026

Copy link
Copy Markdown

The Rust port is up as requested: maxmilian#1, based on fix/1638-ts-interface-members. Two commits — the four codegraph-kernel/src/tsjs/ edits, then the signature: "counts counts" fix with its TS and Rust halves in one commit, as @maxmilian asked.

Putting the acceptance numbers here as well, since this is the PR @colbymchenry will be reviewing and the kernel gap is the thing that decides whether it does anything on a released install.

scripts/kernel-parity.mjs over src, __tests__ and ui — 626 files, wasm totals 16,308 nodes / 17,353 edges / 100,384 refs — with the kernel built from this branch versus with the port applied:

byte-parity files with diffs
this branch as it stands (948e455) 452 / 626 169
with maxmilian#1 619 / 626 2

The first row is the scope limit measured rather than argued: 2,386 property and 1,174 method nodes missing in the kernel, 3,560 contains edges, and ~3k references on each side anchoring differently. On a kernel install, that is what this PR currently does not deliver.

The two files still diverging are Dart fixtures (torture.dart, TortureCtors.dart) and they diverge identically in the control run — pre-existing, unrelated. No TS or TSX file diverges, before or after the signature fix.

One thing I could not run and would rather say than leave implied: the vitest suite. My host runs Bun and vitest's worker pool does not survive it on Windows, so the "648 pass, 1 fail" I reported earlier came from a Node 22 run I no longer have. The port itself is Rust-only, but the signature commit touches tree-sitter.ts, so that suite result wants confirming on your end.

Also still open from my earlier comment, unrelated to the port: imports edges targeting a property/method node go 19 → 61 on my tree with this PR, and to 0 with #1538 also applied. Worth deciding whether they should land together.

@bompus

bompus commented Sep 6, 2026

Copy link
Copy Markdown

Correcting one thing I said above, before it gets read as a general claim about this PR.

I reported that imports edges targeting a property/method node go 19 → 61 on my tree "because the new interface member nodes are now candidates for the name-only import resolver". The count is right, but the causal read is not reliably general. Self-indexing codegraph itself, three builds, fresh index -f each:

build nodes / edges importsproperty/method
this PR alone 17,736 / 65,698 489
+ the kernel mirror (maxmilian#1) 21,347 / 71,498 492
+ #1538 21,363 / 71,115 0

The baseline is already 489 before this PR's members exist at all — on this repo most of those edges are not TS interface members (Rust traits and the like reach the same interface/property kinds). This PR adds 3. So the near-tripling on my Chrome-extension tree reflects that repo's shape, not a general effect of indexing interface members.

#1538 still zeroes them, and it still merges cleanly onto fix/1638-ts-interface-members. But it is worth landing on its own account rather than as a mitigation this PR requires, and I would not want my earlier number weighed as an argument against merging this one.

Also, on the kernel gap: running the full suite on both arms turned up a sharper piece of evidence than the DEFAULT_ROUTED reading. This PR's own new test — extraction.test.ts > indexes interface members, not just the interface itself — fails on a kernel build without the Rust port, along with all 7 kernel-tsjs-parity cases. Details and the full before/after are on maxmilian#1.

@bompus

bompus commented Sep 6, 2026

Copy link
Copy Markdown

Dug into the one remaining red test. One finding changes where the fix has to go, so putting it here rather than only on the branch.

Rank is ordered by graphScore (RWR mass), not by score. From the CODEGRAPH_EXPLORE_DEBUG sidecar on this branch, flow query, CG-28 fixture:

rank score graphScore subgraph nodes file
1 26.25 0.184398 24 types/platform-shims.d.ts
2 27 0.137461 5 src/storage/metadata.ts
3 9.45 0.109091 8 types/worker-configuration.d.ts
4 18 0.058601 5 src/storage/stream.ts

The shim already loses on score — 26.25 against 27 — and still takes rank 1. Rank tracks graphScore descending, exactly, in every run I made.

Both tools.ts mitigations move score or the seed tier — the method_signature relevance weight, and keeping signatures out of the named-FIRST tier. Neither touches graphScore. That is consistent with your own result that a 0.5 member weight "restores the score but not the rank", and it means no further weight tuning reaches this failure.

Where the members actually hurt: the restart vector, not connectivity. contains is not in RANK_EDGES, so an interface member is nearly isolated in the walk graph and carries almost no walk mass itself. Its whole effect is occupying a seed — and the restart vector is uniform over seeds, so each member divides the restart mass the implementation files compete for. Since #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 mechanism behind the 0.307 → 0.137 halving you measured.

Dropping interface-owned signatures from the restart vector only — they stay candidates, stay reachable, keep their score contribution:

before after
src/storage/metadata.ts 0.137461 0.189009
types/platform-shims.d.ts 0.184398 0.191047
gap 0.046937 0.002038

The halving is undone and stream.ts moves above worker-configuration.d.ts. The inversion is not — the test stays red at a 0.002 margin, so this is a diagnosis, not a fix.

I also measured the "member of a type ~0.5" weighting applied to the per-file mass sum: it moves the shim only 0.191047 → 0.189274, because the members were never carrying the mass. Combined, the gap is 0.00027 and still inverted. The residual is no longer about members — it is the shim's 9 interface nodes against metadata.ts's 5, since file mass sums over nodes. Closing that needs a third lever; the ambient penalty at ~0.45 would do it arithmetically, but that is the corpus-measured number @danusha2345 flagged and I would not tune it to pass one fixture.

It is on maxmilian#2 as a draft against this branch, with the full numbers. Full suite: 94 failures before, 94 after, the one differing name being a watcher.test.ts fs.watch race that passes 3/3 in isolation. The other 11 assertions in explore-declaration-only.test.ts are unaffected, including the CG-25 control and the pure-type-module counter-case.

Entirely yours to take, rewrite, or close — it is your ranking core, and the corpus re-measure is the part neither of us can do from outside.

@bompus

bompus commented Sep 6, 2026

Copy link
Copy Markdown

Update on the CG-28 red test: it is fixed. explore-declaration-only.test.ts is 12 passed / 0 failed, and the full suite loses that failure and gains no other. maxmilian#2 is no longer a draft.

First, a correction to my last comment. I wrote that "rank is ordered by graphScore, not score". The second half holds; the first does not, and 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 first (pinned, named, central, entry) and orders by graphScore within a tier:

rank=1 graph=0.504025 central=true  entry=true   src/storage/metadata.ts
rank=2 graph=0.214871 central=true  entry=true   src/storage/stream.ts
rank=3 graph=0.009459 central=false entry=true   types/platform-shims.d.ts
rank=4 graph=0.000000 central=false entry=true   types/worker-configuration.d.ts
rank=5 graph=0.147744 central=false entry=false  src/lib/bucket.ts
rank=6 graph=0.072574 central=false entry=false  src/routes/upload.ts

The load-bearing part survives: score does not order the list. The shim lost on score — 26.25 against 27 — and still took rank 1. So the two tools.ts mitigations, which move score and the seed tier, cannot reach this failure; graphScore is what has to move.

What moves it. A declaration-only file is not a place a walk starts. contains is not a RANK_EDGE, so its members carry almost no walk mass — their whole effect is occupying seeds in a restart vector that is uniform over seeds, dividing the mass implementation files compete for. Excluding damped declaration files from the restart vector only:

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

The shim stays a candidate, stays reachable, keeps its score contribution and is still named in the response — no suppression.

The predicate is isDampedDeclaration, not isAmbientDeclaration, which is what makes both of the gate's claims hold at once: it already exempts a file whose declared type the query named, so on what does the UploadStorage interface declare the shim still ranks 1 at graph mass 1.0 and penalty 1.

This touches neither getAmbientDeclarationPathsAmong nor the 0.5 penalty, so the flag rate is unchanged by construction — the same files are flagged, only the restart vector differs. It does change ranking for any project holding a declaration-only file, so it still wants a look on the corpus before it ships.

@maxmilian it is yours to take or rewrite. With it, this branch has no failing test I can find.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants