feat(resolver): phase 8.4 + 8.5 — barrel re-export resolution and CHA/RTA dynamic dispatch#1302
feat(resolver): phase 8.4 + 8.5 — barrel re-export resolution and CHA/RTA dynamic dispatch#1302carlos-alm wants to merge 7 commits into
Conversation
…r call edges The JS/WASM call-edge path (buildImportedNamesMap) was mapping imported symbol names to the barrel file itself rather than the actual definition file. For example, `Button` imported from `components/index.ts` (a barrel) was mapped to `components/index.ts` instead of `components/Button.ts`, so resolveCallTargets could not find the node and the call edge was dropped. The native engine already traced through barrels correctly in buildImportedNamesForNative. This PR mirrors that behavior in the WASM/JS fallback path and caches results to avoid redundant DFS walks across files. Changes: - PipelineContext: add barrelExportCache (build-scoped Map keyed by "barrelPath|symbolName") to avoid repeated traversal of the reexportMap - resolve-imports: export resolveBarrelExportCached, a cache-aware wrapper around resolveBarrelExport; reset cache at the start of each build run - build-edges/buildImportedNamesMap: add barrel tracing via traceBarrel helper — matches the logic already in buildImportedNamesForNative - build-edges: replace all resolveBarrelExport calls with the cached variant (emitTypeOnlySymbolEdges, buildBarrelEdges, buildImportedNamesForNative, makeContextLookup) for consistency and performance Also opens #1297 for the pre-existing WASM-only failure where barrel-through import edges are not emitted on full builds.
…k in resolveBarrelExportCached
The "emits barrel-through edges on full build" assertion in this test also guards #1297 (barrel-through import edges missing on WASM full builds). The fix was delivered by the phase 8.4 changes to buildBarrelEdges — switching to resolveBarrelExportCached which initialises ctx.barrelExportCache before the edge-building pass. Closes #1297
|
Claude finished @carlos-alm's task —— View job PR Review: feat(resolver): phase 8.4 — barrel file re-export chain resolution
Root Cause AnalysisProblem Being Solved: The JS/WASM call-edge path ( Why This Approach: The fix mirrors the pattern already used in Risk Assessment: Low. The change is purely additive within the existing abstraction. The Phase 2: Configuration Change DetectionNo lint rules, TypeScript flags, build configuration, or test thresholds were modified. Clean. Phase 3: Test IntegrityTests are strengthening coverage, not weakening it:
Phase 4: Code Quality
Clean implementation. One minor correctness note:
Matches
One observation worth flagging:
Backlog Compliance
Final RecommendationRating: ⭐⭐⭐⭐ (4/5) Action: APPROVE Reasoning: The fix is correct, minimal, well-tested, and mirrors the exact pattern already used in the native path. Root cause is properly addressed (not symptom-masked). Both the positive and negative test assertions are meaningful. The 3-commit structure is clean: main feature, minor correctness tweak, documentation. No configuration relaxation, no weakened assertions, no new dependencies. The one thing keeping this from 5/5 is the absence of a test covering the incremental-build path for this specific fixture (all existing phase-8.4 tests use |
Greptile SummaryThis PR delivers two pipeline improvements: Phase 8.4 fixes barrel re-export resolution in the WASM/JS call-edge path so imported symbols map to their leaf definition file rather than the barrel, and Phase 8.5 introduces Class Hierarchy Analysis (CHA) with Rapid Type Analysis (RTA) filtering to expand interface/abstract method dispatch to all instantiated concrete implementations.
Confidence Score: 5/5Safe to merge; changes are additive and well-tested across both engine paths. The barrel-resolution fix is a straightforward correctness change with targeted tests. The CHA/RTA implementation is well-structured, correctly handles multi-interface dispatch (extractor emits one ClassRelation per interface), guards against hierarchy cycles, and follows existing patterns for post-pass edge insertion. The two notes in runPostNativeCha (no debug log on empty instantiated set, no explicit transaction around batchInsertEdges) are operational polish issues that do not affect correctness under normal conditions. src/domain/graph/builder/stages/native-orchestrator.ts — the runPostNativeCha function; silent early-return and non-transactional inserts are worth hardening before this path sees heavy production traffic. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[buildEdges entry] --> B[propagateReturnTypesAcrossFiles]
B --> C[buildChaContext]
C --> D{Engine?}
D -->|WASM / JS| E[buildCallEdgesJS]
E --> F[buildFileCallEdges inline CHA dispatch]
F --> F1{receiver type}
F1 -->|this/self/super| G[resolveThisDispatch]
F1 -->|typed receiver| H[resolveChaTargets RTA-filtered]
D -->|Native FFI fallback| I[native engine builds edges]
I --> J[buildParamFlowPtsPostPass]
J --> K[buildChaPostPass]
D -->|Native orchestrator| L[tryNativeOrchestrator]
L --> M[runPostNativeCha DB query]
M --> N{chaTargetIds > 0?}
N -->|yes| O[classifyNodeRoles re-run]
G --> P[emit CHA calls edges]
H --> P
K --> P
M --> P
Reviews (4): Last reviewed commit: "fix(parity): re-classify roles after CHA..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const barrelCallEdge = callEdges.find( | ||
| (e) => | ||
| e.caller_name === 'render' && | ||
| e.callee_name === 'Button' && | ||
| e.callee_file === 'components/Button.ts', | ||
| ); |
There was a problem hiding this comment.
caller_file, so any render → Button edge from any file would satisfy it. With the current minimal fixture this is unambiguous, but adding a caller_file check future-proofs the test if new render functions appear in the fixture.
| const barrelCallEdge = callEdges.find( | |
| (e) => | |
| e.caller_name === 'render' && | |
| e.callee_name === 'Button' && | |
| e.callee_file === 'components/Button.ts', | |
| ); | |
| const barrelCallEdge = callEdges.find( | |
| (e) => | |
| e.caller_name === 'render' && | |
| e.caller_file === 'App.ts' && | |
| e.callee_name === 'Button' && | |
| e.callee_file === 'components/Button.ts', | |
| ); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed — added caller_file === 'App.ts' to the assertion so the match is fully specified and won't become ambiguous if new render functions are added to the fixture.
… + RTA) Introduces Class Hierarchy Analysis (CHA) and Rapid Type Analysis (RTA) to improve call-graph coverage for OOP codebases. CHA: when a call targets an interface or abstract method (e.g. worker.doWork() where worker: IWorker), emit additional call edges to all known concrete implementations reachable via the class/interface hierarchy. RTA filter: CHA targets are narrowed to types actually instantiated in the program via new X() — prevents dead/test-only implementations from inflating the dispatch fan-out. this/self/super dispatch: inside a method body, this.method() now resolves through the class's own method table and parent hierarchy via qualified-name lookup (e.g. ClassName.method), rather than relying solely on global name matching which may miss inherited or ambiguous methods. Implementation: - src/domain/graph/builder/cha.ts (new): ChaContext, buildChaContext, resolveThisDispatch, resolveChaTargets - ExtractorOutput.newExpressions: dedicated RTA instantiation list extracted from all new X() expressions in JS/TS files (not just assigned ones) - WASM path: CHA dispatch runs inline in buildFileCallEdges - Native path: DB-based CHA expansion post-pass in tryNativeOrchestrator (interface dispatch only; this/super dispatch is a known gap for native) - build-edges.ts: buildChaPostPass wired for native FFI path (fallback mode) Test: tests/integration/phase-8.5-cha-dispatch.test.ts covers CHA dispatch, RTA filter, this-dispatch (wasm only), and super-dispatch (wasm only) across the cha-dispatch fixture with IWorker/ConcreteWorker/MockWorker/GhostWorker.
carlos-alm
changed the title
…est assertion (#1302) - Add caller_file check to the barrel call resolution test assertion to future-proof it against fixture growth (greptile review) - Add comment in propagateReturnTypesAcrossFiles documenting the Phase 8.4 barrel-tracing side-effect on cross-file return-type propagation
|
Addressed both Greptile review points:
|
Codegraph Impact Analysis32 functions changed → 24 callers affected across 9 files
|
…for C# RTA (#1302) C# extractor never populated newExpressions, so WASM CHA had no RTA seed for C# instantiated types. Additionally, newExpressions was stripped during worker thread serialization (SerializedExtractorOutput lacked the field, and neither serializeExtractorOutput nor deserializeResult handled it). - csharp.ts: populate newExpressions in extractCSharpSymbols and push type names in handleCsObjectCreation - wasm-worker-protocol.ts: add newExpressions field to SerializedExtractorOutput - wasm-worker-entry.ts: include newExpressions in serializeExtractorOutput - wasm-worker-pool.ts: restore newExpressions in deserializeResult
…1302) The Rust orchestrator classifies roles as part of its internal pipeline, before the JS runPostNativeCha post-pass adds CHA call edges. Implementor methods (e.g. UserRepository.Save) had no incoming edges at classification time and were tagged dead-ffi, diverging from the WASM engine which classifies roles after all edges (including CHA) are inserted. runPostNativeCha now returns the Set of target node IDs for newly inserted edges. tryNativeOrchestrator uses this set to query the affected files and re-runs classifyNodeRoles (incremental) on those files only, bringing native and WASM role results into alignment.
|
Two additional fixes pushed after investigating a 1. C# The C# extractor never populated 2. Native engine roles stale after CHA post-pass
Fix: All 8 |
1 participant
Summary
Phase 8.4: Barrel file re-export chain resolution for call edges
buildImportedNamesMap) was mapping imported symbol names to the barrel file rather than the actual definition file. For example,Buttonimported fromcomponents/index.ts(a barrel) was mapped tocomponents/index.tsinstead ofcomponents/Button.ts, soresolveCallTargetscould not find the node and the call edge was dropped. The native engine already traced through barrels correctly — this PR mirrors that in the WASM/JS fallback path.importskind) were also not emitted on WASM full builds.buildBarrelEdgesnow uses the same cached barrel resolver, which initialisesctx.barrelExportCacheinresolveImportsbefore the edge-building pass.Phase 8.5: Enhanced dynamic dispatch resolution (CHA + RTA)
Introduces Class Hierarchy Analysis (CHA) and Rapid Type Analysis (RTA) to improve call-graph coverage for OOP codebases.
worker.doWork()whereworker: IWorker), emit additionalcallsedges to all known concrete implementations reachable via the class/interface hierarchy — not just the interface declaration.new X(). Types that are declared but never constructed (e.g. test stubs, dead implementations) are excluded, keeping dispatch fan-out accurate.this.method()now resolves through the class's own method table and parent hierarchy via qualified-name lookup (ClassName.method), rather than relying on global name matching which misses inherited methods and may produce false positives on name collisions.newExpressionsextraction: the JS extractor now captures allnew X()constructor names — including unassigned ones likedoSomething(new Foo())— for accurate RTA coverage.Implementation paths:
buildFileCallEdges(handles CHA dispatch, this/self/super, and RTA)tryNativeOrchestratorreadsimplements/extendsedges and constructor call evidence from SQLite to expand interface dispatch (known gap: this/super dispatch requires raw unresolved call sites not persisted by the Rust pipeline)buildChaPostPassinbuild-edges.tsfor when the Rust orchestrator is unavailableChanges
Phase 8.4:
PipelineContext: addbarrelExportCache(build-scopedMap) to avoid repeated DFS traversal ofreexportMapresolve-imports: exportresolveBarrelExportCached; initialise the cache at the start ofresolveImportsbuild-edges: applytraceBarrelinbuildImportedNamesMap; switchbuildBarrelEdgesandbuildImportedNamesForNativeto useresolveBarrelExportCachedPhase 8.5:
src/domain/graph/builder/cha.ts(new):ChaContext,buildChaContext,resolveThisDispatch,resolveChaTargetssrc/types.ts: addnewExpressions?: readonly string[]toExtractorOutputsrc/extractors/javascript.ts:extractNewExpressionsWalkwired into both extraction paths; fix pre-existinguseOptionalChainlint issuesrc/domain/graph/builder/stages/build-edges.ts: CHA context built before edge transaction; inline CHA dispatch inbuildFileCallEdges;buildChaPostPassfor native FFI fallbacksrc/domain/graph/builder/stages/native-orchestrator.ts:runPostNativeChaDB-based expansion post-passTest plan
phase-8.4-barrel-call-resolution.test.ts— call edges resolve through barrel re-exports (wasm + native)phase-8.5-cha-dispatch.test.ts— CHA interface dispatch to ConcreteWorker/MockWorker; RTA excludes GhostWorker; this-dispatch and super-dispatch (wasm only; native gap documented as skipped)issue-1174-chained-barrel-incremental.test.ts:110— barrel-through import edges on full build (guards fix(wasm): barrel-through import edges missing on WASM full builds #1297)Closes #1297