diff --git a/__tests__/explore-declaration-only.test.ts b/__tests__/explore-declaration-only.test.ts index 004711f7d..ab9748839 100644 --- a/__tests__/explore-declaration-only.test.ts +++ b/__tests__/explore-declaration-only.test.ts @@ -96,15 +96,36 @@ describe('CG-28 — a declaration-only file does not outrank implementation on a if (testDir && fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true }); }); + /** + * Type-level for the purposes of this gate: a type declaration, or a member + * an interface declares. + * + * The second half is not a loosening. Since #1638 a `method_signature` / + * `property_signature` is indexed as a `method` / `property` node, so a file + * of nothing but interfaces no longer reads as nothing but `interface` kinds + * — but a bodiless signature is on the same side of the line as the interface + * that owns it, which is exactly how `getAmbientDeclarationPathsAmong` counts + * it. What this still catches, and is here to catch, is a `function` or a + * `class` creeping into the fixture: that would silently exempt the file and + * make every assertion below vacuous. + */ + const isTypeLevel = (n: { id: string; kind: string }, filePath: string): boolean => { + if (n.kind === 'interface' || n.kind === 'type_alias') return true; + if (n.kind !== 'method' && n.kind !== 'property') return false; + const interfaceIds = new Set( + cg.getNodesInFile(filePath).filter((x) => x.kind === 'interface').map((x) => x.id), + ); + return cg.getIncomingEdges(n.id) + .some((e) => e.kind === 'contains' && interfaceIds.has(e.source)); + }; + describe('fixture shape — if this rots, the gate below means nothing', () => { it('holds two declaration-only files that differ only in the banner', () => { for (const p of [HANDWRITTEN_DECL, GENERATED_DECL]) { const nodes = cg.getNodesInFile(p).filter((n) => n.kind !== 'file' && n.kind !== 'import'); expect(nodes.length, `${p} declares nothing`).toBeGreaterThan(10); - // Every symbol type-level, nothing with a body — the structural test the - // penalty keys on. A `function`/`class` creeping in would silently exempt - // the file and make every assertion below vacuous. - expect(nodes.every((n) => n.kind === 'interface' || n.kind === 'type_alias'), `${p} has a non-type symbol`).toBe(true); + // Nothing with a body — the structural test the penalty keys on. + expect(nodes.every((n) => isTypeLevel(n, p)), `${p} has a non-type symbol`).toBe(true); } // Only one of them announces itself, so the CG-25 penalty is the ONLY // difference between the two — that is what makes them comparable. @@ -119,7 +140,7 @@ describe('CG-28 — a declaration-only file does not outrank implementation on a // structure of any answer about that code. const nodes = cg.getNodesInFile(SHARED_TYPES).filter((n) => n.kind !== 'file' && n.kind !== 'import'); expect(nodes.length).toBeGreaterThan(0); - expect(nodes.every((n) => n.kind === 'interface' || n.kind === 'type_alias')).toBe(true); + expect(nodes.every((n) => isTypeLevel(n, SHARED_TYPES))).toBe(true); expect(cg.getFile(SHARED_TYPES)?.generated).toBeFalsy(); }); @@ -176,6 +197,23 @@ describe('CG-28 — a declaration-only file does not outrank implementation on a expect(isAmbient(SHARED_TYPES)).toBe(false); expect(isAmbient(HANDWRITTEN_DECL)).toBe(true); }); + + it('still flags a shim whose interfaces now contribute method/property nodes', () => { + // The silent-failure guard for #1638. Interface members are indexed, so a + // pure-interface `.d.ts` no longer holds only `interface` kinds — and the + // ambient rule is spelled as "EVERY declared symbol is type-level". Read + // literally that stops flagging the moment the extractor improves, and + // nothing else fails: the file just quietly ranks undamped again. + // + // Pinned from both ends on purpose. The `toBeGreaterThan(0)` half is what + // keeps the other half honest — assert only the flag and this test would + // still pass on an index where the members were never extracted at all, + // which is precisely the state it exists to detect a regression FROM. + const members = cg.getNodesInFile(HANDWRITTEN_DECL) + .filter((n) => n.kind === 'method' || n.kind === 'property'); + expect(members.length, 'interface members are not indexed — see #1638').toBeGreaterThan(0); + expect(cg.ambientDeclarationFilePredicate([HANDWRITTEN_DECL])(HANDWRITTEN_DECL)).toBe(true); + }); }); describe('the counter-case — a query that NAMES a declared type', () => { diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index ad0ba2374..ea24cc71d 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -566,6 +566,55 @@ interface Hprops { expect(refs.some((r) => r.referenceName === 'IOrderField')).toBe(true); }); + it('indexes interface members, not just the interface itself', () => { + // tree-sitter-typescript spells interface members `method_signature` / + // `property_signature`, distinct from the class-member types the extractor + // listed, so they were never captured (#1638). Java/C# are unaffected — + // their grammars reuse `method_declaration`, already in their methodTypes. + // The cost lands on `.d.ts` platform APIs: with no declaration node, call + // sites through the interface have nothing to attach an edge to. + const code = ` +export interface PlatformApi { + fetchPage(id: string): Promise; + version: string; +} +`; + const result = extractFromSource('api.d.ts', code); + + const iface = result.nodes.find((n) => n.kind === 'interface' && n.name === 'PlatformApi'); + const method = result.nodes.find((n) => n.kind === 'method' && n.name === 'fetchPage'); + const prop = result.nodes.find((n) => n.kind === 'property' && n.name === 'version'); + expect(iface).toBeDefined(); + expect(method).toBeDefined(); + expect(prop).toBeDefined(); + + // Attached to the interface, not merely present. A member the graph holds + // but hangs off the file is not a declaration a call edge can be resolved + // through, which is the whole point of extracting it. + const contained = result.edges + .filter((e) => e.kind === 'contains' && e.source === iface!.id) + .map((e) => e.target); + expect(contained).toContain(method!.id); + expect(contained).toContain(prop!.id); + }); + + it('does not mint a top-level function from a type literal method signature', () => { + // The failure mode the class-like guard on `method_signature` exists for + // (#1638). `extractMethod` treats a method node with no class-like parent + // as a free function — right for `method_definition`, wrong for a bodiless + // signature, whose only home outside an interface is a type literal. Those + // members are already extracted onto the alias (#359), so without the guard + // the file gains a phantom `function stop` beside the real `Handle::stop`. + const result = extractFromSource('t.ts', ` +export type Handle = { stop(): void; label: string }; +`); + + const alias = result.nodes.find((n) => n.kind === 'type_alias' && n.name === 'Handle'); + expect(alias).toBeDefined(); + expect(result.nodes.find((n) => n.kind === 'method' && n.name === 'stop')).toBeDefined(); + expect(result.nodes.filter((n) => n.kind === 'function' && n.name === 'stop')).toEqual([]); + }); + it('should extract type references from interface method signatures', () => { const code = ` import type { IPage } from '../PromoterList'; @@ -842,10 +891,20 @@ export type Names = ['alpha', 'beta']; `; const result = extractFromSource('noise.ts', code); + // Since #1638 the fixture's own interfaces legitimately declare `id` / `name` + // (`User::id`, `User::name`, `Service::name`), so membership in the name list + // no longer implies a leak. What #634 guards is the *source*: a node minted + // from a string literal in `Pick` or a tuple has no declaring + // interface, so exclude anything a `contains` edge ties to one. + const ifaceIds = new Set(result.nodes.filter((n) => n.kind === 'interface').map((n) => n.id)); + const declaredInInterface = new Set( + result.edges.filter((e) => e.kind === 'contains' && ifaceIds.has(e.source)).map((e) => e.target) + ); const leaked = result.nodes.filter( (n) => (n.kind === 'method' || n.kind === 'property') && - ['id', 'name', 'foo', 'bar', 'alpha', 'beta'].includes(n.name) + ['id', 'name', 'foo', 'bar', 'alpha', 'beta'].includes(n.name) && + !declaredInInterface.has(n.id) ); expect(leaked).toEqual([]); }); diff --git a/__tests__/object-literal-methods.test.ts b/__tests__/object-literal-methods.test.ts index 1722ad2d0..ade53d81a 100644 --- a/__tests__/object-literal-methods.test.ts +++ b/__tests__/object-literal-methods.test.ts @@ -53,7 +53,11 @@ describe('object-literal method extraction', () => { // Each action's body was walked: fetchUser references its sibling `reset`, // so an in-store calls edge will resolve once the pipeline runs. - const fetchUser = result.nodes.find((n) => n.name === 'fetchUser')!; + // By KIND as well as name: the fixture's `Store` interface declares a + // `fetchUser` too, and since #1638 that signature is a node of its own — + // one that appears FIRST in the file, so a name-only lookup finds the + // declaration and reads its return type where the action's body was meant. + const fetchUser = result.nodes.find((n) => n.kind === 'function' && n.name === 'fetchUser')!; const fetchUserRefs = result.unresolvedReferences.filter((r) => r.fromNodeId === fetchUser.id); expect(fetchUserRefs.map((r) => r.referenceName)).toContain('reset'); diff --git a/codegraph-kernel/src/tsjs/extractors.rs b/codegraph-kernel/src/tsjs/extractors.rs index f240c5e79..766769cef 100644 --- a/codegraph-kernel/src/tsjs/extractors.rs +++ b/codegraph-kernel/src/tsjs/extractors.rs @@ -270,8 +270,33 @@ impl<'t> Walker<'t> { let name = self.text(name_node).to_string(); // TS/JS field definitions carry an explicit `type` field; the generic - // scan is for other languages (#808). - let type_text = node.child_by_field_name("type").map(|t| { + // scan is for other languages (#808). A `property_signature` (an + // interface member, #1638) carries a `type` field and no value, so it + // reads the type field too: the generic scan's exclusion list covers + // `identifier` but not the `property_identifier` an interface member is + // named with, so it would stop on the name and make the signature repeat + // it (`counts counts`) instead of naming the type. Mirrors + // extractProperty's isTsJsField. + let is_ts_js_field = matches!( + node.kind(), + "public_field_definition" | "field_definition" | "property_signature" + ); + let type_node = if is_ts_js_field { + node.child_by_field_name("type") + } else { + (0..node.named_child_count()).filter_map(|i| node.named_child(i)).find(|c| { + !matches!( + c.kind(), + "modifier" + | "modifiers" + | "identifier" + | "accessor_list" + | "accessors" + | "equals_value_clause" + ) + }) + }; + let type_text = type_node.map(|t| { let raw = self.text(t); raw.strip_prefix(':').unwrap_or(raw).trim_start().to_string() }); diff --git a/codegraph-kernel/src/tsjs/mod.rs b/codegraph-kernel/src/tsjs/mod.rs index afe6361d5..a76c4d485 100644 --- a/codegraph-kernel/src/tsjs/mod.rs +++ b/codegraph-kernel/src/tsjs/mod.rs @@ -55,10 +55,32 @@ impl Variant { /// typescriptExtractor.methodTypes / javascriptExtractor.methodTypes. fn is_method_type(v: Variant, kind: &str) -> bool { kind == "method_definition" - || (v.is_ts() && kind == "public_field_definition") + || (v.is_ts() && matches!(kind, "public_field_definition" | "method_signature")) || (!v.is_ts() && kind == "field_definition") } +/// typescriptExtractor.propertyTypes. The interface counterpart of +/// `public_field_definition`: it carries no value, so it is always a property +/// and never goes through classify_ts_class_member (#1638). +fn is_property_type(v: Variant, kind: &str) -> bool { + v.is_ts() && kind == "property_signature" +} + +/// Method node types that spell a SIGNATURE — a declaration with no body (#1638). +/// +/// They are a method of whatever type declares them and nothing on their own, so +/// they must not take `extract_method`'s "no class-like parent, so treat it as a +/// free function" fallback. The other method types can: a `method_definition` +/// outside a class really is a function. This one appears outside a class only +/// inside a type literal (`type Handle = { stop(): void }`), whose members +/// `extract_ts_type_alias_members` already extracts and attaches to the alias +/// (#359) — take the fallback and the file gains a phantom top-level +/// `function stop` beside the real `Handle::stop`. Mirrors the TS extractor's +/// SIGNATURE_METHOD_NODE_TYPES (extraction/tree-sitter.ts). +fn is_signature_method_type(kind: &str) -> bool { + kind == "method_signature" +} + fn is_function_type(kind: &str) -> bool { matches!(kind, "function_declaration" | "arrow_function" | "function_expression") } @@ -622,7 +644,9 @@ impl<'t> Walker<'t> { } else if is_class_type(self.variant, kind) { self.extract_class(node); skip_children = true; - } else if is_method_type(self.variant, kind) { + } else if is_method_type(self.variant, kind) + && (!is_signature_method_type(kind) || self.inside_class_like()) + { if classify_ts_class_member(node) == Member::Property { let prop = self.extract_property(node); if let (Some((row, name)), Some(value)) = (prop, node.child_by_field_name("value")) { @@ -664,12 +688,22 @@ impl<'t> Walker<'t> { self.extract_call(node); } else if kind == "new_expression" { self.extract_instantiation(node); - } else if self.variant.is_ts() - && matches!(kind, "property_signature" | "method_signature") - && self.inside_class_like() - { - let parent = self.top_row(); - self.extract_type_annotations(node, parent); + } else if is_property_type(self.variant, kind) && self.inside_class_like() { + // NOTE: `property_signature` / `method_signature` used to be handled + // here together, hanging their type annotations off the ENCLOSING + // INTERFACE — the only anchor available while the members themselves + // went unextracted. Since #1638 `method_signature` is a method type + // and `property_signature` a property type, so the method branch + // above claims the first (under the same inside_class_like guard + // this branch had) and this one extracts the second as a real node. + // The `references` edges survive — extract_method and + // extract_property each call extract_type_annotations — but now hang + // off the member, the more precise anchor: `Api::fetch → PageId` + // says which member wants the type, where `Api → PageId` only said + // the file did. + self.extract_property(node); + self.scan_fn_ref_subtree(node, 0); + skip_children = true; } if !skip_children { diff --git a/src/db/queries.ts b/src/db/queries.ts index 4e2990c7c..2ec444428 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -55,6 +55,24 @@ function isLowValueFile(filePath: string, generated?: ReadonlySet): bool const SQLITE_PARAM_CHUNK_SIZE = 500; +/** + * A SQL predicate: is the node aliased `alias` a member an INTERFACE declares? + * + * `method_signature` / `property_signature` enter the graph as `method` / + * `property` nodes hung off their interface by a `contains` edge (#1638). They + * have no body and originate no behaviour, so for a structural judgement about + * a FILE they are the interface restated, not an extra thing the file declares. + * See {@link QueryBuilder.getAmbientDeclarationPathsAmong}, the one caller, for + * why treating them as opaque would break that rule in three places at once. + * + * Seeks `idx_edges_target_kind`, so it costs a key lookup per row rather than a + * join over the whole edge table. + */ +const IS_INTERFACE_MEMBER = (alias: string): string => `EXISTS ( + SELECT 1 FROM edges ce JOIN nodes owner ON owner.id = ce.source + WHERE ce.target = ${alias}.id AND ce.kind = 'contains' AND owner.kind = 'interface' +)`; + /** * How much of the exact-name bonus a `deprioritize`d path keeps (#982). Damped * rather than zeroed: a query that genuinely targets that tree must still rank @@ -2736,6 +2754,29 @@ export class QueryBuilder { * restricted to the candidate list: the file that imports it is usually * not itself a candidate. * + * ### Interface MEMBERS are transparent to all four conditions + * + * A `method_signature` / `property_signature` inside an interface enters the + * graph as a `method` / `property` node (#1638). Read literally that would + * break every condition here at once: condition 2 sees non-type kinds and + * stops flagging, and — worse, because it is silent — condition 4 starts + * seeing inbound `calls` edges the moment a call site through the shim's API + * finally has a signature to land on. An ambient `.d.ts` would quietly lose + * its damping precisely BECAUSE the platform API it declares is widely used. + * + * So an interface-owned member is treated the way `parameter` already is: it + * neither qualifies, disqualifies, nor counts as inbound dependency. That is + * not a new judgement call, it is what keeps the rule measuring what it was + * measured on — before #1638 these nodes did not exist, so excluding them + * reproduces the 0–4% flag rate the thresholds above were tuned against. It + * is also the semantically right answer: a signature with no body is on the + * same side of the line as the interface that owns it, and a call edge + * landing on one is still not a file that can answer a flow question. + * + * The interface ITSELF is untouched: the `references` edges an importing + * module aims at `UploadStorage` still disqualify the file under (4), which + * is what keeps a depended-on `types.ts` out of the flag. + * * Bounded-lookup like {@link getGeneratedPathsAmong}: callers hold a ranked * candidate list, so this is a partial-index probe over a handful of paths. */ @@ -2751,14 +2792,15 @@ export class QueryBuilder { // things the file declares, so they neither qualify nor disqualify. const rows = this.db .prepare(` - SELECT file_path, - SUM(CASE WHEN kind NOT IN ('file','import','export','parameter') + SELECT n.file_path AS file_path, + SUM(CASE WHEN n.kind NOT IN ('file','import','export','parameter') + AND NOT ${IS_INTERFACE_MEMBER('n')} THEN 1 ELSE 0 END) AS declared, - SUM(CASE WHEN kind IN ('interface','type_alias','enum','enum_member','namespace') + SUM(CASE WHEN n.kind IN ('interface','type_alias','enum','enum_member','namespace') THEN 1 ELSE 0 END) AS typeDeclared - FROM nodes - WHERE file_path IN (${placeholders}) - GROUP BY file_path + FROM nodes n + WHERE n.file_path IN (${placeholders}) + GROUP BY n.file_path `) .all(...chunk) as Array<{ file_path: string; declared: number; typeDeclared: number }>; let candidates = rows @@ -2775,17 +2817,22 @@ export class QueryBuilder { ); candidates = candidates.filter((p) => !hit.has(p)); }; - // (3) originates behaviour + // (3) originates behaviour — a signature has no body to originate from, + // so an edge attributed to one is not evidence about this file. disqualify(` SELECT DISTINCT n.file_path AS file_path FROM edges e JOIN nodes n ON n.id = e.source WHERE e.kind IN ('calls','instantiates') AND n.file_path IN ($IN$) + AND NOT ${IS_INTERFACE_MEMBER('n')} `); - // (4) something outside the file depends on it + // (4) something outside the file depends on it — but a call that lands on + // an interface's own signature is a use of the API, not a dependency on + // this file's structure. The edges aimed at the interface still count. disqualify(` SELECT DISTINCT t.file_path AS file_path FROM edges e JOIN nodes t ON t.id = e.target JOIN nodes s ON s.id = e.source WHERE t.file_path IN ($IN$) AND s.file_path <> t.file_path + AND NOT ${IS_INTERFACE_MEMBER('t')} `); for (const path of candidates) found.add(path); } diff --git a/src/extraction/languages/typescript.ts b/src/extraction/languages/typescript.ts index 72059114f..29371fd60 100644 --- a/src/extraction/languages/typescript.ts +++ b/src/extraction/languages/typescript.ts @@ -41,8 +41,15 @@ export function classifyTsClassMember(node: SyntaxNode): 'method' | 'property' { export const typescriptExtractor: LanguageExtractor = { functionTypes: ['function_declaration', 'arrow_function', 'function_expression'], classTypes: ['class_declaration', 'abstract_class_declaration'], - methodTypes: ['method_definition', 'public_field_definition'], + // `method_signature` is the interface/type-literal form of a method; without it + // an interface's members never enter the graph, so a `.d.ts` platform API has + // no declaration node for call sites to attach to (#1638). Java/C# don't need + // an equivalent — their grammars reuse `method_declaration`. + methodTypes: ['method_definition', 'public_field_definition', 'method_signature'], classifyMethodNode: classifyTsClassMember, + // The interface counterpart of `public_field_definition`. It carries no value, + // so it is always a property and never needs classifyMethodNode. + propertyTypes: ['property_signature'], interfaceTypes: ['interface_declaration'], structTypes: [], enumTypes: ['enum_declaration'], diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 7ef90c273..3328d56c6 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -52,6 +52,20 @@ const RTK_HOOK_NAME_RE = /^use[A-Z][A-Za-z0-9]*(?:Query|Mutation)$/; * initialized with one of these is a component, not a constant (#841). */ const REACT_COMPONENT_HOCS = new Set(['forwardRef', 'memo', 'React.forwardRef', 'React.memo']); +/** + * Method node types that spell a SIGNATURE — a declaration with no body (#1638). + * + * They are a method of whatever type declares them and nothing on their own, so + * they must not take `extractMethod`'s "no class-like parent, so treat it as a + * free function" fallback. The other `methodTypes` can: a `method_definition` + * outside a class really is a function. This one appears outside a class only + * inside a type literal (`type Handle = { stop(): void }`), whose members + * `extractTypeAlias` already extracts and attaches to the alias (#359) — take + * the fallback and the file gains a phantom top-level `function stop` beside + * the real `Handle::stop`. + */ +const SIGNATURE_METHOD_NODE_TYPES = new Set(['method_signature']); + /** Vue store collections whose object-literal members are the symbols an agent * looks for. Extracted as function nodes so `actions`/`mutations`/`getters` are * findable + readable (the foundation under any later dispatch-bridge synth). */ @@ -1037,8 +1051,13 @@ export class TreeSitterExtractor { this.extractClass(node); skipChildren = true; } - // Check for method declarations (only if not already handled by functionTypes) - else if (this.extractor.methodTypes.includes(nodeType)) { + // Check for method declarations (only if not already handled by functionTypes). + // A bodiless SIGNATURE only counts as one where a type declares it — see + // SIGNATURE_METHOD_NODE_TYPES for what falling through would otherwise mint. + else if ( + this.extractor.methodTypes.includes(nodeType) + && (!SIGNATURE_METHOD_NODE_TYPES.has(nodeType) || this.isInsideClassLikeNode()) + ) { // TS/JS class fields parse as a methodTypes node; only function-valued // fields are methods — a plain field (`public fonts: Fonts;`) is a // property (#808). classifyMethodNode is absent for other languages. @@ -1293,22 +1312,16 @@ export class TreeSitterExtractor { else if (nodeType === 'impl_item') { this.extractRustImplItem(node); } - // TypeScript interface members: property_signature (`foo: T`, `foo?: T`) - // and method_signature (`foo(arg: A): R`) both carry type annotations the - // interface walker would otherwise drop. Extract them as `references` - // edges from the interface so resolvers can wire callers/impact for - // types that only appear in interface members. - else if ( - (nodeType === 'property_signature' || nodeType === 'method_signature') && - this.isInsideClassLikeNode() && - this.TYPE_ANNOTATION_LANGUAGES.has(this.language) - ) { - const parentId = this.nodeStack[this.nodeStack.length - 1]; - if (parentId) { - this.extractTypeAnnotations(node, parentId); - } - // don't skipChildren — nested signatures still need traversal - } + // NOTE: `property_signature` / `method_signature` used to be handled here, + // hanging their type annotations off the ENCLOSING INTERFACE — the only + // anchor available while the members themselves went unextracted. Since + // #1638 they are in the TS extractor's `methodTypes` / `propertyTypes`, so + // the branches above claim them first (under the same `isInsideClassLikeNode` + // guard this branch had, so nothing it used to reach is now missed) and this + // one was dead. The `references` edges survive — `extractMethod` and + // `extractProperty` each call `extractTypeAnnotations` — but now hang off + // the member, which is the more precise anchor: `Api::fetch → PageId` says + // which member wants the type, where `Api → PageId` only said the file did. // Visit children (unless the extract method already visited them) if (!skipChildren) { @@ -2036,8 +2049,18 @@ export class TreeSitterExtractor { // and the initializer VALUE, which the generic finder below would // wrongly pick — so fields use the type field only (#808). Other // languages (C# property_declaration) keep the generic scan. + // + // A `property_signature` (an interface member, #1638) carries a `type` + // field and no value, so it reads the type field too. It cannot take the + // generic scan: that scan's exclusion list covers `identifier` but not the + // `property_identifier` an interface member is named with, so it stops on + // the name and `interface Stats { counts: Record }` yields + // `signature: "counts counts"` instead of the type. Named explicitly + // rather than folded into the field test so no other language's + // `property_declaration` moves off the generic scan. const isTsJsField = - node.type === 'public_field_definition' || node.type === 'field_definition'; + node.type === 'public_field_definition' || node.type === 'field_definition' + || node.type === 'property_signature'; const typeNode = isTsJsField ? getChildByField(node, 'type') : node.namedChildren.find( diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index b0585745e..06345f1ba 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -358,6 +358,14 @@ export const RELEVANCE_KIND_WEIGHT: Readonly> = { }; const DEFAULT_RELEVANCE_KIND_WEIGHT = 0.5; +/** + * The "member of a type" tier of the table above, named so the one kind that + * cannot be read off `node.kind` can be placed on it: an interface's + * `method_signature` (#1638). Same value as `property`/`field`, deliberately — + * it is the same tier, not a new one. + */ +const TYPE_MEMBER_RELEVANCE_WEIGHT = 0.5; + /** * Kinds whose evidentiary value depends on whether anything USES them. An * exported `const DEFAULTS` that half the codebase references is a real @@ -3305,6 +3313,35 @@ export class ToolHandler { // substantive definition (skip empty stubs + test files, same relevance the // trace endpoint picker uses) and inject it as an entry, so every symbol the // agent explicitly named is in the subgraph and its file is scored. + /** + * Is this a member an INTERFACE declares — a signature with no body (#1638)? + * + * It arrives as an ordinary `method` node, so without asking, every ranking + * stage reads a `.d.ts` full of `method_signature`s as a file full of + * callables. Two stages below ask, for the same reason: a signature is the + * declaration of behaviour, never behaviour, and the rank a file earns must + * not grow just because its interfaces spell their members out. + * + * Cached; reached only for `method` nodes on paths that already probe the + * graph per node, so it adds a key lookup, not a pass. + */ + const interfaceMemberCache = new Map(); + const isInterfaceOwnedMethod = (node: Node): boolean => { + if (node.kind !== 'method') return false; + const cached = interfaceMemberCache.get(node.id); + if (cached !== undefined) return cached; + let owned = false; + try { + owned = cg.getIncomingEdges(node.id).some( + (e) => e.kind === 'contains' && cg.getNode(e.source)?.kind === 'interface', + ); + } catch { + owned = false; // a probe failure must not manufacture a penalty + } + interfaceMemberCache.set(node.id, owned); + return owned; + }; + const namedSeedIds = new Set(); // The subset of named seeds that earns the named-FIRST sort tier. We still // SEED every ≤3-def name (so RWR / flow ranking is unchanged), but only the @@ -3475,7 +3512,21 @@ export class ToolHandler { // so a named symbol FTS already gathered never sorted to the top.) namedSeedIds.add(n.id); } - for (const n of tierPicks) tierSeedIds.add(n.id); + // An interface's `method_signature` seeds (so RWR and the flow ranking + // still see it, and a query that names it still reaches its file) but + // never earns the named-FIRST tier (#1638). That tier means "the agent + // asked for the symbol DEFINED here", and this seeding says as much — + // it resolves a token to its substantive definition and sorts bodies + // first. A declaration is the stub that sort demotes, not the answer. + // Without this the tier is reachable by prose: `body`, `stream` and + // `metadata` are member names in any platform `.d.ts`, and each one + // corroborates the next through `coNamedInFile`, so an ambient shim + // walks past the NL-stopword guard and lands above every implementation + // file — the exact inversion CG-28 exists to prevent, arriving on a key + // that sorts above the CG-28 penalty. + for (const n of tierPicks) { + if (!isInterfaceOwnedMethod(n)) tierSeedIds.add(n.id); + } } } @@ -3526,9 +3577,21 @@ export class ToolHandler { isolationCache.set(node.id, isolated); return isolated; }; + /** + * A `method_signature` reaches here as a `method`, which the kind table + * rates 1.0: "a callable — the unit an architecture question is about". It + * is not that. It is the row below on the same scale, "a member of a type", + * and rating it as a callable is how a 28-interface `.d.ts` doubled its + * score the moment its members became indexable (#1638). Only `method` + * needs correcting; `property` already sits in the member tier whoever + * declares it. + */ const relevanceWeight = (node: Node, probeIsolation: boolean): number => { - const weight = RELEVANCE_KIND_WEIGHT[node.kind] ?? DEFAULT_RELEVANCE_KIND_WEIGHT; - if (!probeIsolation || !WEAK_RELEVANCE_KINDS.has(node.kind)) return weight; + const signatureOnly = isInterfaceOwnedMethod(node); + const weight = signatureOnly + ? TYPE_MEMBER_RELEVANCE_WEIGHT + : RELEVANCE_KIND_WEIGHT[node.kind] ?? DEFAULT_RELEVANCE_KIND_WEIGHT; + if (!probeIsolation || !(signatureOnly || WEAK_RELEVANCE_KINDS.has(node.kind))) return weight; return isUsageIsolated(node) ? ISOLATED_WEAK_KIND_WEIGHT : weight; }; @@ -3756,8 +3819,33 @@ export class ToolHandler { // (org-user.storage.ts, call-connected to the matches) accrues mass; a lone // text match (LensSwitcher.swift, matched "switch" but calls nothing in the // flow) gets only its restart probability → ~0, and is dropped by the gate. + // + // A file the ambient-declaration penalty has already damped is a candidate, + // but not a place a walk STARTS. The restart vector is uniform over seeds, + // so every seed divides the restart mass the implementation files compete + // for — and since #1638 a platform `.d.ts` contributes one seed per member, + // whose names (`body`, `stream`, `metadata`) are exactly what a prose flow + // query matches. That is what halves an implementation file's graph mass + // while the shim's holds steady: dilution of the restart vector, not + // connectivity. `contains` is not a RANK_EDGE, so these members carry almost + // no walk mass of their own; seeding is the whole of their effect on rank. + // + // `isDampedDeclaration` and not a bare ambient test: it already exempts a + // file whose declared type the query NAMED, so a query genuinely about the + // declared type keeps its seeds and the shim still ranks first. Damped files + // stay in the candidate set, stay reachable, and keep their `score` + // contribution — this changes only where the walk starts. + const rwrSeedIds = new Set(); + for (const id of entryNodeIds) { + const seed = subgraph.nodes.get(id); + if (seed && isDampedDeclaration(seed.filePath)) continue; + rwrSeedIds.add(id); + } const nodeRwr = this.computeGraphRelevance( - [...subgraph.nodes.keys()], subgraph.edges, entryNodeIds, + // Fall back to the unfiltered seeds when EVERY seed is damped: the walk + // must not lose its restart vector and return all-uniform. + [...subgraph.nodes.keys()], subgraph.edges, + rwrSeedIds.size > 0 ? rwrSeedIds : entryNodeIds, ); // // Carries `rankPenalty` too, so generated/low-value files are demoted on the