fix(cha): scope CHA implementor map by file, walk ancestors for inherited methods - #2399
Conversation
Sorry, something went wrong.
…ited methods
Two bugs in Class Hierarchy Analysis (CHA) dispatch, present identically
in both engines and across all four independent implementations of the
same implementors-map + BFS pattern (WASM inline, WASM/native DB-driven
post-pass, native orchestrator post-pass):
1. The implementors map (interface/base-class name -> concrete classes)
was keyed by bare simple name, scanning every file with no file/module
scoping. Two unrelated files each declaring their own same-named
interface had their implementor sets merged, producing a false call
edge into the wrong file's method.
2. CHA dispatch did a direct qualified lookup (`${concreteClass}.${method}`)
for every RTA-instantiated concrete class. When that class inherits the
dispatched method from an ancestor without overriding it, no node
exists under the concrete class's own qualified name, so the lookup
missed and the edge was never emitted.
Add a file-scoped implementors map (`${parentName}|${file}`, populated
only when that file also locally declares a same-named parent) that the
BFS root prefers over the bare map, and an ancestor-walk fallback that
follows the class hierarchy up to the declaring ancestor when a direct
qualified lookup misses. Mirrored across cha.ts, helpers.ts's
runChaPostPass, native-orchestrator.ts's runPostNativeCha, and the Rust
build_edges.rs.
The file-scoping fix required deriving "locally declared names" from each
file's full definitions list (filtered to class/interface/struct/etc.
kinds), not the classes-relations list — a bare interface with no
extends/implements clause of its own never appears in the latter.
Closes #2237
Internal resolver fix — no README/CLAUDE.md/ROADMAP-documented behavior
changes. docs check acknowledged.
Impact: 19 functions changed, 23 affected
Greptile SummaryThe PR scopes CHA implementor traversal by file and adds ancestor walking for inherited virtual methods across the TypeScript, database-driven, native-orchestrated, and Rust implementations.
Confidence Score: 4/5The PR is not yet safe to merge because an inherited dispatch can still resolve an unrelated file's same-qualified concrete method before walking the selected hierarchy. The new resolver preserves file identity during BFS but immediately abandons it when the selected concrete class lacks an override, allowing a project-wide name collision to produce a false call edge and suppress the correct inherited target. Files Needing Attention: src/domain/graph/builder/cha.ts, src/domain/graph/builder/helpers.ts, src/domain/graph/builder/stages/native-orchestrator.ts, crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Call["Typed virtual call"] --> Root["CHA receiver root + caller file"]
Root --> BFS["File-aware implementor BFS"]
BFS --> RTA{"Implementor instantiated?"}
RTA -->|No| BFS
RTA -->|Yes| Scoped["Same-file qualified method lookup"]
Scoped -->|Found| Edge["Emit CHA call edge"]
Scoped -->|Missing| Global["Project-wide qualified-name fallback"]
Global -->|Found| Wrong["May select unrelated same-named class method"]
Global -->|Missing| Parent["Walk scoped parent chain"]
Parent --> Scoped
Reviews (2): Last reviewed commit: "fix(cha): preserve file identity through..." | Re-trigger Greptile |
Sorry, something went wrong.
| const qualified = `${current}.${methodName}`; | ||
| const found = lookup.byName(qualified).filter((n) => n.kind === 'method'); | ||
| if (found.length > 0) return found; |
There was a problem hiding this comment.
Scoped implementor identity is lost
When unrelated files declare the same interface, implementing class, and method names, the scoped BFS selects a bare class name and lookup.byName returns that qualified method from every file, causing false call edges to unrelated implementations. Preserve the selected implementor's file identity through method resolution; the DB and Rust mirrors require the same correction.
Knowledge Base Used:
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed in 8b3d112. File identity is now propagated through the whole BFS (not just the root) and into the qualified-method lookup: each hop prefers a same-file lookup and falls back to the bare/global one only when the scoped lookup finds nothing — never a regression, only a precision gain when file identity is known. Mirrored across cha.ts, helpers.ts's runChaPostPass, native-orchestrator.ts's runPostNativeCha, and Rust's build_edges.rs.
Added a dedicated regression test (two files each declaring their own same-named implementor class with its own same-named method) in tests/unit/cha.test.ts, crates/codegraph-core/.../build_edges.rs, and tests/integration/issue-2237-cha-implementor-scoping.test.ts — verified it reproduces the leak without this fix and is clean with it.
Sorry, something went wrong.
| const qualified = `${current}.${methodName}`; | ||
| const found = lookup.byName(qualified).filter((n) => n.kind === 'method'); | ||
| if (found.length > 0) return found; | ||
| current = chaCtx.parents.get(current); |
There was a problem hiding this comment.
Ancestor walk crosses class hierarchies
When two files declare an identically named class with different parent chains, this first-write-wins bare-name lookup can follow the unrelated class's ancestors, causing an inherited dispatch to target the wrong method or omit the correct edge. The walk must retain the selected concrete class's file identity, with the equivalent change applied to the DB and Rust implementations.
Knowledge Base Used:
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed in 8b3d112 (same commit as the sibling finding above). The ancestor walk now also carries the known file forward: at each hop it prefers parentsByFile/cha_parents_by_file (scoped to the descendant's own file) before falling back to the bare, first-write-wins parents/cha_parents map — so a same-named class in an unrelated file with a different parent chain can no longer be followed by mistake.
Added a dedicated regression test (two files each declaring their own same-named class with a different parent, with the bare map deliberately poisoned toward the wrong one) in the same three test files as the sibling finding — verified it reproduces the cross-hierarchy walk without this fix and is clean with it.
Sorry, something went wrong.
Codegraph Impact Analysis18 functions changed → 23 callers affected across 7 files
|
Sorry, something went wrong.
Fixes cargo fmt (rustfmt) failure from the prior push, and addresses a Greptile review finding: once resolveChaTargets/resolve_cha_dispatch (and their DB-driven mirrors) scope the BFS root to a specific file, that file identity was dropped again before the qualified-method lookup and the ancestor-parent lookup — so a same-named concrete class (or a same-named class with a different parent) declared independently in an unrelated file could still leak into the resolved edge. Propagate the known file through every BFS level (not just the root) and into resolveMethodViaAncestors/resolve_method_via_ancestors: each step now prefers a same-file lookup (method node, or parents-by-file edge) and falls back to the prior bare/global lookup when nothing scoped is found — never a regression, only a precision gain when file identity happens to be known. Mirrored across cha.ts, helpers.ts, native-orchestrator.ts, and build_edges.rs. Internal resolver fix — no README/CLAUDE.md/ROADMAP-documented behavior changes. docs check acknowledged. Impact: 9 functions changed, 18 affected
Sorry, something went wrong.
| const qualified = `${current}.${methodName}`; | ||
| const scopedFound = currentFile ? lookup.byNameAndFile(qualified, currentFile) : []; | ||
| const found = (scopedFound.length > 0 ? scopedFound : lookup.byName(qualified)).filter( | ||
| (n) => n.kind === 'method', | ||
| ); |
There was a problem hiding this comment.
Global fallback bypasses ancestor walk
When a file-scoped concrete implementor inherits the dispatched method and an unrelated file declares the same concrete class name with its own same-named method, the empty scoped lookup falls back to the project-wide byName result before consulting the selected class's parent. CHA therefore emits a false edge to the unrelated method and omits the inherited target from the selected hierarchy.
Knowledge Base Used:
Sorry, something went wrong.
Summary
Two bugs in Class Hierarchy Analysis (CHA) dispatch (issue #2237), present identically in both engines and across all four independent implementations of the same implementors-map + BFS pattern found while researching the fix:
Issue 1 — implementor-map collision: the implementors map (interface/base-class name → concrete classes) was keyed by bare simple name, scanning every file with no file/module scoping. Two unrelated files each declaring their own same-named interface had their implementor sets merged, producing a false call edge into the wrong file's method.
Issue 2 — missed inherited methods: CHA dispatch did a direct qualified lookup (${concreteClass}.${method}) for every RTA-instantiated concrete class. When that class inherits the dispatched method from an ancestor without overriding it, no node exists under the concrete class's own qualified name, so the lookup missed and the edge was never emitted.
Fix
Test plan