fix(extraction): index TypeScript interface members (#1638) - #1686
fix(extraction): index TypeScript interface members (#1638)#1686maxmilian wants to merge 1 commit into
Conversation
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.
|
Read through this while assembling a local integration of the open fixes. The two-line extractor change and the
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. |
|
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 On splitting, I tried the shape you describe and it does not produce the clean first half either of us would want. The measurements:
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 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. |
|
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
So, on this sample:
Happy to re-run on other shapes if the maintainer names a corpus. |
Closes #1638.
tree-sitter-typescript spells interface members with their own node types —
method_signatureandproperty_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 reusemethod_declaration, which is already in theirmethodTypes.The cost lands on any codebase whose platform API is a
.d.tsinterface. 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 (isInsideClassLikeNodelists'interface'), so members attach with no traversal change. Precedent:extractTsTypeAliasMembersalready makestype X = { foo(): T }members first-class (#359) — interfaces were the inconsistent gap.Three consequences, each handled and tested:
extractMethod's "no class-like parent → free function" fallback. Outside an interface such a node appears only in a type literal, whose members TypeScripttypealias members not used in method-call resolution → false cross-modulecallsedges via path-proximity #359 already extracts — without a guard,type Handle = { stop(): void }gains a phantom top-levelfunction stop.property_signature/method_signaturebranch 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. Thereferencesedges survive and now anchor on the member, soApi::fetch → PageIdsays which member wants the type whereApi → PageIdonly said the file did.CG-28: a proposal, not an assertion
getAmbientDeclarationPathsAmongreads "every declared symbol is type-level", which a pure-interface.d.tsstops 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
parameteralready gets ("structural bookkeeping, neither qualifies nor disqualifies"), via one sharedIS_INTERFACE_MEMBER(alias)fragment seekingidx_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-ontypes.tsstill 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.rsre-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_WEIGHTratesmethodat 1.0 as "a callable", and its own doc puts "a member of a type" at ~0.5. Amethod_signatureis the latter. This restores the raw score almost exactly (53 → 52.5) and cuts the rendered envelope from 6044 to 1933 characters.body,streamandmetadataare member names in any platform.d.tsand were reaching that tier throughcoNamedInFile, 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 builtdist/, none touching extraction or queries). After: 3952 / 85 — failure-list diff is exactly the one entry above, with none disappearing.Added:
containsedge rather than merely existing;.d.tsis 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 acontainsedge from an interface; a node minted fromPick<User,'id'>or a tuple has no declaring interface, so the guard is intact.object-literal-methods.test.ts— its Zustand fixture declares bothinterface Store { fetchUser(): … }and an action of that name, sofind(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 anisTypeLevelhelper. Afunctionorclasscreeping into that fixture still fails, and the gate assertions themselves are untouched.