You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Found during Greptile review of #2236 (native CHA typed-dispatch fallback)
Greptile flagged two correctness concerns against the new Rust fallback in resolve_call_targets_core (crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs). Verified both against the WASM source it mirrors (src/domain/graph/builder/cha.ts) — both are pre-existing characteristics already present identically in WASM, not new divergences introduced by #2236. Filing as a dual-engine follow-up rather than fixing unilaterally in one engine (which would create a new native/wasm divergence).
Issue 1: CHA implementor map is keyed by bare simple name, globally
buildChaContext (cha.ts) and its Rust mirror build_cha_implementors_map both build implementors: Map<string, string[]> keyed by the interface/base-class's bare name only, scanning every file in the build with no notion of import/module scoping:
// cha.ts buildChaContextfor(constsymbolsoffileSymbols.values()){for(constclsofsymbols.classes){recordImplements(cls,implementors);// implementors.get(cls.implements) — bare name keyrecordExtends(cls,implementors,parents);}}
If two unrelated modules each declare their own interface/class with the same simple name (e.g. two different Handler interfaces in different packages), resolveChaTargets/resolve_cha_dispatch's BFS will merge their implementor sets and can emit a call edge into the wrong module's method — corrupting call-graph, impact, and role results for that pattern.
This is the same general class of bug as #2062 (resolveThisDispatch cross-file same-named-class collision) — a recurring pattern of flat, name-only maps that don't verify a real heritage/import relationship before accepting a match.
Issue 2: CHA dispatch doesn't walk up for inherited (non-overriding) methods
resolveChaTargets (cha.ts) and resolve_cha_dispatch (Rust) both do a direct qualified lookup${concreteClass}.${methodName} for every RTA-instantiated concrete class reached via BFS:
When an instantiated concrete class inherits the dispatched method from an ancestor without overriding it, no node exists named ${cls}.${methodName} (the method node is registered under the declaring ancestor's name) — so the lookup misses and the reachable call edge is never emitted, in both engines identically.
Suggested fix (dual-engine, not attempted in #2236)
Issue 2: when the direct qualified lookup on a concrete class misses, walk up that class's own extends chain (already available via the parents map in cha.ts / would need adding to the Rust ChaContext mirror) to find the nearest ancestor that actually declares the method, mirroring how resolveThisDispatch already walks chaCtx.parents.
Both changes must land in cha.ts and its Rust mirror together to preserve engine parity — this is explicitly why #2236 (scoped narrowly to the proximity-gate fallback itself) did not attempt either fix.
Found during Greptile review of #2236 (native CHA typed-dispatch fallback)
Greptile flagged two correctness concerns against the new Rust fallback in
resolve_call_targets_core(crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs). Verified both against the WASM source it mirrors (src/domain/graph/builder/cha.ts) — both are pre-existing characteristics already present identically in WASM, not new divergences introduced by #2236. Filing as a dual-engine follow-up rather than fixing unilaterally in one engine (which would create a new native/wasm divergence).Issue 1: CHA implementor map is keyed by bare simple name, globally
buildChaContext(cha.ts) and its Rust mirrorbuild_cha_implementors_mapboth buildimplementors: Map<string, string[]>keyed by the interface/base-class's bare name only, scanning every file in the build with no notion of import/module scoping:If two unrelated modules each declare their own interface/class with the same simple name (e.g. two different
Handlerinterfaces in different packages),resolveChaTargets/resolve_cha_dispatch's BFS will merge their implementor sets and can emit a call edge into the wrong module's method — corrupting call-graph, impact, and role results for that pattern.This is the same general class of bug as #2062 (
resolveThisDispatchcross-file same-named-class collision) — a recurring pattern of flat, name-only maps that don't verify a real heritage/import relationship before accepting a match.Issue 2: CHA dispatch doesn't walk up for inherited (non-overriding) methods
resolveChaTargets(cha.ts) andresolve_cha_dispatch(Rust) both do a direct qualified lookup${concreteClass}.${methodName}for every RTA-instantiated concrete class reached via BFS:When an instantiated concrete class inherits the dispatched method from an ancestor without overriding it, no node exists named
${cls}.${methodName}(the method node is registered under the declaring ancestor's name) — so the lookup misses and the reachable call edge is never emitted, in both engines identically.Suggested fix (dual-engine, not attempted in #2236)
extends/implementsedge from a hierarchy rooted in a file the caller can see (or otherwise disambiguate same-named declarations across files) — likely shares a resolution strategy with resolveThisDispatch resolves this/super dispatch to the wrong file when a same-named base class exists elsewhere with no same-file match #2062's suggested fix.extendschain (already available via theparentsmap incha.ts/ would need adding to the RustChaContextmirror) to find the nearest ancestor that actually declares the method, mirroring howresolveThisDispatchalready walkschaCtx.parents.Both changes must land in
cha.tsand its Rust mirror together to preserve engine parity — this is explicitly why #2236 (scoped narrowly to the proximity-gate fallback itself) did not attempt either fix.