test(bench): add JS/TS super.method() class-inheritance fixtures

[carlos-alm]

Summary

• Adds inheritance.js to the JavaScript resolution benchmark with super.method() call patterns: instance methods (2-level chain), multi-level hierarchy, and static methods
• Adds super-dispatch.ts to the TypeScript resolution benchmark with a multi-level super.method() chain (PrefixLogger → TimestampLogger → Logger)
• Updates both expected-edges.json manifests with 5 new class-inheritance mode edges

Context

The resolveThisDispatch function in cha.ts (Phase 8.5) already handles super.method() dispatch correctly — it starts hierarchy traversal from the parent class when receiver === 'super'. This PR adds benchmark fixtures to formally verify and document that behavior in the resolution benchmark.

All 5 new edges resolve through the CHA path with technique cha, precision and recall thresholds unchanged for both JS and TS benchmarks.

Test plan

• npx vitest run tests/benchmarks/resolution/resolution-benchmark.test.ts -t "javascript|typescript" — both pass
• npx vitest run tests/integration/phase-8.5-cha-dispatch.test.ts — 12 pass, 4 skipped (existing native skips)
• npm test — 2856 pass, 32 skipped

Closes #1315

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review

---

⚠️ Scope / Title Mismatch — Read This First

The PR title and description say "add JS/TS super.method() class-inheritance fixtures", but the diff is substantially larger than that. The 32 changed files and +1076/-30 lines include:

• A new production module (src/domain/graph/builder/cha.ts, 175 lines)
• Major rewrites to two pipeline stages (build-edges.ts +151/-10, native-orchestrator.ts +240/-11)
• New C# extractor behavior (src/extractors/csharp.ts — adds newExpressions support)
• New integration tests (tests/integration/phase-8.5-cha-dispatch.test.ts, 210 lines)
• New fixture files (10 .ts fixture files in tests/fixtures/cha-dispatch/)

The benchmark fixtures (the stated purpose) are 5 files totalling ~70 lines. The rest implements Phase 8.5 CHA + RTA dispatch. The PR description fundamentally misrepresents the scope.

---

Phase 1: Root Cause Analysis

Problem Being Solved: Virtual method dispatch was either unresolved or resolved only to interface method declarations. For dispatch(worker: IWorker) calling worker.doWork(), the old graph produced either no edge or a single edge to IWorker.doWork. Phase 8.5 adds Class Hierarchy Analysis (CHA) with Rapid Type Analysis (RTA) filtering to expand such calls to all instantiated concrete implementations. The benchmark fixtures document the specific super.method() sub-case.

Why This Approach: The CHA/RTA approach is the correct solution — industry-standard for OO call graph construction. The cha.ts module is properly separated with clean interfaces (ChaContext, buildChaContext, resolveThisDispatch, resolveChaTargets). The two-path design (inline in buildFileCallEdges for WASM, post-pass in buildChaPostPass/runPostNativeCha for native) is an honest reflection of the native engine's constraints.

Risk Assessment: Medium-high. This is core call-graph computation — any bug here silently emits false-positive or false-negative edges for every OO codebase.

---

Phase 2: Configuration Change Detection

No lint rule changes, no TypeScript strictness flags disabled, no test timeouts increased. The embedding regression test (tests/search/embedding-regression.test.ts) was modified but only to add new expected entries — not to loosen thresholds. No configuration concerns.

---

Phase 3: Test Integrity

RED FLAG — Native this/super dispatch is skipped and documented as a "known gap":

tests/integration/phase-8.5-cha-dispatch.test.ts:127:

it.skipIf(engine === 'native')(
  'this-dispatch: emits ConcreteWorker.doWork → ConcreteWorker.prepare',
  ...

tests/integration/phase-8.5-cha-dispatch.test.ts:147:

it.skipIf(engine === 'native')('super-dispatch: emits Lion.speak → Animal.speak', ...)

tests/integration/phase-8.5-cha-dispatch.test.ts:169:

it.skipIf(engine === 'native')('CHA transitive: emits runJob → PrintJob.run (3-level hierarchy)', ...)

The comment at line 163-167 says:

"The pre-compiled native binary (v3.11.2) does not yet include the abstract_class_declaration fix, so transitive CHA tests are WASM-only until the native binary is updated."

And at lines 121-125:

"Note: this/super dispatch requires the raw unresolved call sites which are not persisted to the DB by the Rust pipeline. That case is handled by the WASM path… and is a known gap for the native orchestrator."

Per CLAUDE.md: "Never document bugs as expected behavior." These 4 skips document behavioral divergence between engines as acceptable. The this/super gap on the native path is a real correctness defect. The tests should either:

1. Assert the failure case explicitly (so a fix is detectable), or
2. Not be merged until the native path parity exists.

Skipping them with skipIf(engine === 'native') is the pattern the rules explicitly prohibit.

Otherwise, test quality is high: The RTA filter negative assertion (GhostWorker.doWork must NOT appear) is exactly the right way to test filtering behavior. The AbstractJob.run negative assertion at line 201 is also correct and passes both engines.

---

Phase 4: Code Quality

src/domain/graph/builder/cha.ts — Good

Clean, well-commented module. resolveThisDispatch correctly starts from the parent class when receiver === 'super' (cha.ts:112). The visited-set cycle guard (cha.ts:116-117) is correct. resolveChaTargets BFS is correct.

Minor: list.includes(cls.name) in buildChaContext (cha.ts:52) is O(n) inside a loop. For large codebases with many implementors of one interface this could be slow. A Set per parent name would be O(1). Not a blocker.

native-orchestrator.ts:runPostNativeCha — Potential Dedup Gap

At native-orchestrator.ts:482-498, the seeded seen set is built from edges where source_id IN (...) — scoped to the source_ids of callToMethods. This is correct for deduplication. However, the runPostNativeCha function runs as a separate DB pass after the Rust pipeline completes — it reads the DB, not allEdgeRows. There's a race window: if buildChaPostPass (the JS post-pass in build-edges.ts) runs on the same session, both passes could independently expand the same interface dispatch edges. This is handled by the routing logic (one path or the other runs, not both), but the comment at native-orchestrator.ts:387-403 acknowledges this explicitly. A potential issue is that runPostNativeCha runs on existing DB edges (after native orchestrator) while buildChaPostPass runs on allEdgeRows in-memory (during JS path). These are mutually exclusive paths — acceptable.

native-orchestrator.ts:runPostNativeCha — Parity Gap Still Present

The comment at native-orchestrator.ts:396-403 explicitly states:

"Note: this/super dispatch requires the raw unresolved call sites which are not persisted to the DB by the Rust pipeline. That case is handled by the WASM path… and is a known gap for the native orchestrator."

This is exactly the condition CLAUDE.md prohibits documenting as a known gap. The fix would require the native engine to persist raw call sites (including receiver) to the DB — or to run a WASM-based post-pass for this/super sites on native builds.

build-edges.ts:buildChaPostPass — Seeding Bug Risk

build-edges.ts:645-647: seeding seenByPair only from calls edges is explicitly justified to avoid false dedup at file-node level (the comment is clear). Correct.

src/extractors/csharp.ts:handleCsObjectCreation

The cast (ctx.newExpressions as string[]).push(typeName) (csharp.ts:260) bypasses the readonly string[] type. This is a common pattern in this codebase (same cast appears in javascript.ts) but it's worth noting as a type-safety smell. The newExpressions field on ExtractorOutput is typed as readonly string[] — the cast silently breaks immutability. Not a blocker but a code smell.

src/domain/parser.ts and wasm-worker-protocol.ts

The diff adds newExpressions to the WASM worker serialization protocol — correctly propagating the field through the worker pool boundary so the main thread's buildChaContext can see constructor calls from worker-parsed files.

---

Phase 5: Backlog Compliance

Axis	Assessment
Zero-dep	✓ — No new runtime dependencies. Builds on existing tree-sitter/SQLite/in-memory structures.
Foundation-aligned	✓ — Fully local. No cloud calls, no external services. Improves agent accuracy (fewer hallucinated/missing call edges).
Problem-fit	5 — CHA dispatch directly reduces hallucinated dependencies. Agents seeing dispatch → IWorker.doWork instead of the concrete implementations get incorrect blast-radius data. This fixes a root cause of graph inaccuracy for OO codebases.
Breaking	No — Purely additive edges. No API contract changes, no schema changes, no CLI output format changes. Edge confidence values shift (new CHA edges appear), but this is correct behavior, not a breaking change.
Tier	1

---

Critical Concerns

1. it.skipIf(engine === 'native') for behavioral gaps (lines 127, 147, 169 in phase-8.5-cha-dispatch.test.ts): This documents engine divergence as acceptable. Per CLAUDE.md this is a blocking concern. The native path producing no this/super edges while the WASM path does is a real accuracy gap. The fix path is clear (persist receiver info in the Rust pipeline, or add a WASM fallback post-pass for native builds), but until it's fixed the test should not skip — it should expect(...).toBeDefined() and be marked as a known-failing issue, not silently skipped.

2. PR description misrepresents scope: The title says "fixtures" but ships core Phase 8.5 production code. Minor issue (no correctness impact) but makes PR review harder and git history harder to read.

---

Final Assessment

## Root Cause Analysis
Problem Being Solved: Virtual method dispatch missing or under-resolved in OO codebases
Why This Approach: CHA + RTA is the correct industry-standard approach; well-architected
Risk Assessment: Medium-high — core call-graph logic; native path has documented parity gap

## Backlog Compliance
- Zero-dep: ✓
- Foundation-aligned: ✓
- Problem-fit: 5 — directly fixes hallucinated/missing call edges for OO codebases
- Breaking: No
- Tier: 1

## Critical Concerns
1. it.skipIf(engine === 'native') for this/super/transitive CHA — violates CLAUDE.md "Never
   document bugs as expected behavior". 4 tests skip rather than fail for a real accuracy gap.
2. PR title/description misrepresents scope — ships Phase 8.5 production code, not just fixtures.

## Final Recommendation
- Rating: ⭐⭐⭐☆☆ (3/5)
- Action: REQUEST CHANGES
- Reasoning: The CHA/RTA implementation itself is high quality and the right approach.
  The blocking issue is the 4 native-skipped tests — they must either be converted to
  tracked failures (linked issue + TODO comment) or the native parity gap must be
  closed before merging. The CLAUDE.md rule is explicit: engine divergence is a bug in
  the less-accurate engine, not a documented gap. Open an issue for the native
  this/super/transitive gap and reference it in the test (skipIf → todo, or convert
  to expect-failure pattern), then this PR is ready to merge.

[greptile-apps]

Greptile Summary

This PR adds super.method() class-inheritance and prototype-based method benchmark fixtures for the JS and TS resolution benchmarks, alongside the production-side extraction and resolution logic needed to make those fixtures pass.

• New fixtures: inheritance.js (2-level and static super chains), prototypes.js/prototypes2.js (pre-ES6 prototype OOP), and super-dispatch.ts (3-class Logger hierarchy) — each backed by new expected-edges.json entries.
• Production changes: extractPrototypeMethodsWalk in javascript.ts handles Foo.prototype.bar = fn and Foo.prototype = {…} patterns; call-resolver.ts gains an inline new Expr receiver inference pass and a prototype-alias typeMap fallback so resolved aliases reach lookup.byName.
• Housekeeping: CHA_DISPATCH_PENALTY is now exported from build-edges.ts and imported in native-orchestrator.ts (fixing the previous magic-number divergence); it.skipIf in the CHA integration test is replaced with explicit it.todo blocks referencing issue fix(native): persist raw call-site receiver info to DB for this/super dispatch on native path #1326; embedding-regression tests now use ctx.skip() instead of a silent return.

Confidence Score: 5/5

Safe to merge — all changes are additive benchmark fixtures plus well-scoped extraction and resolver enhancements backed by passing tests

The production changes (prototype extraction walk, inline new-expr inference, prototype-alias resolver fallback) follow established patterns in the codebase, use the shared setTypeMapEntry helper with appropriate confidence values, and are exercised directly by the new benchmark manifests. The two extraction paths (extractSymbolsQuery / extractSymbolsWalk) are mutually exclusive, so extractPrototypeMethodsWalk runs exactly once per file with no duplicate-definition risk. The CHA_DISPATCH_PENALTY export cleanly closes the previously flagged constant-divergence gap. No logic errors or data-correctness issues were identified.

No files require special attention

Important Files Changed

Filename	Overview
src/extractors/javascript.ts	Adds extractPrototypeMethodsWalk (and helpers) to capture Foo.prototype.bar=fn and Foo.prototype={…} patterns; called once in each of the two mutually-exclusive extraction paths (query vs walk), no duplicate-definition risk
src/domain/graph/builder/call-resolver.ts	Adds inline new-expression receiver inference (regex on raw receiver text) and prototype-alias typeMap fallback lookup; both changes are well-scoped and fall through cleanly to existing resolution paths
src/domain/graph/builder/stages/build-edges.ts	Exports CHA_DISPATCH_PENALTY constant to fix the previously identified divergence between WASM and native paths
src/domain/graph/builder/stages/native-orchestrator.ts	Imports CHA_DISPATCH_PENALTY from build-edges.ts and replaces two hardcoded 0.1 literals; single source of truth now maintained
tests/integration/phase-8.5-cha-dispatch.test.ts	Replaces it.skipIf(engine==='native') with if/else blocks using it.todo() for native gap tests, making intent explicit and referencing tracking issue #1326
tests/search/embedding-regression.test.ts	Replaces silent return-on-rate-limit with ctx.skip() (properly marks tests as skipped), and adds Windows EBUSY retry loop in afterAll cleanup using Atomics.wait for synchronous backoff

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["call-resolver.ts\nresolveByMethodOrGlobal()"] --> B{typeName resolved?}
    B -- "typeMap hit" --> D
    B -- "no: check inline\nnew-expr regex" --> C["regex: /new ClassName/\nextracts typeName"]
    C --> D{lookup.byName\ntypeName.method}
    D -- "found" --> RET["return resolved nodes"]
    D -- "not found" --> E["prototype-alias fallback\ntypeMap.get('ClassName.method')"]
    E -- "alias found" --> F["lookup.byName(aliasTarget)\nfilter by computeConfidence >= 0.5"]
    F --> RET
    E -- "no alias" --> G["Phase 8.3d composite pts\nlookup continues..."]

    subgraph "New extraction (javascript.ts)"
        H["extractPrototypeMethodsWalk()"] --> I{"Foo.prototype.bar = rhs"}
        I -- "fn/arrow" --> J["emit Definition\nFoo.bar as method"]
        I -- "identifier" --> K["setTypeMapEntry\ntypeMap['Foo.bar'] = identifier\nconfidence 0.9"]
        H --> L{"Foo.prototype = { ... }"}
        L -- "pair/method_def" --> M["emitPrototypeMethod per key"]
    end

Loading

_{Reviews (8): Last reviewed commit: "fix: resolve merge conflicts with main" | Re-trigger Greptile}

[greptile-apps]

The CHA_DISPATCH_PENALTY constant is not exported, so native-orchestrator.ts duplicates the magic number 0.1 inline. If the penalty is tuned in one place, the other silently diverges, producing different confidence values between the WASM and native paths for the same CHA edge.

Suggested change

	/** Phase 8.5: confidence penalty applied to CHA-dispatch edges. */
	const CHA_DISPATCH_PENALTY = 0.1;
	/** Phase 8.5: confidence penalty applied to CHA-dispatch edges. */
	export const CHA_DISPATCH_PENALTY = 0.1;

[carlos-alm]

Fixed — exported CHA_DISPATCH_PENALTY from build-edges.ts and replaced the hardcoded 0.1 in native-orchestrator.ts with an import of the named constant. Both paths now reference the same single source of truth. Also updated the comment above the query to reference the constant name rather than the literal value.

[github-actions]

Codegraph Impact Analysis

37 functions changed → 35 callers affected across 10 files

• resolveByMethodOrGlobal in src/domain/graph/builder/call-resolver.ts:62 (8 transitive callers)
• runPostNativeCha in src/domain/graph/builder/stages/native-orchestrator.ts:406 (4 transitive callers)
• extractSymbolsQuery in src/extractors/javascript.ts:315 (1 transitive callers)
• extractConstDeclarators in src/extractors/javascript.ts:472 (3 transitive callers)
• extractSymbolsWalk in src/extractors/javascript.ts:593 (1 transitive callers)
• extractNewExprTypeName in src/extractors/javascript.ts:1169 (16 transitive callers)
• findReturnNewExprType in src/extractors/javascript.ts:1280 (11 transitive callers)
• handleVarDeclaratorTypeMap in src/extractors/javascript.ts:1442 (12 transitive callers)
• handleParamTypeMap in src/extractors/javascript.ts:1536 (12 transitive callers)
• extractCallbackDefinition in src/extractors/javascript.ts:1927 (6 transitive callers)
• extractDynamicImportNames in src/extractors/javascript.ts:2037 (6 transitive callers)
• extractPrototypeMethodsWalk in src/extractors/javascript.ts:2104 (3 transitive callers)
• walk in src/extractors/javascript.ts:2109 (12 transitive callers)
• handlePrototypeAssignment in src/extractors/javascript.ts:2133 (12 transitive callers)
• emitPrototypeMethod in src/extractors/javascript.ts:2176 (12 transitive callers)
• extractPrototypeObjectLiteral in src/extractors/javascript.ts:2199 (11 transitive callers)
• Animal in tests/benchmarks/resolution/fixtures/javascript/inheritance.js:3 (0 transitive callers)
• Animal.speak in tests/benchmarks/resolution/fixtures/javascript/inheritance.js:4 (0 transitive callers)
• Dog in tests/benchmarks/resolution/fixtures/javascript/inheritance.js:9 (0 transitive callers)
• Dog.speak in tests/benchmarks/resolution/fixtures/javascript/inheritance.js:10 (0 transitive callers)

[carlos-alm]

Addressed Claude's review feedback:

• it.skipIf(engine === 'native') behavioral gaps: Opened issue fix(native): persist raw call-site receiver info to DB for this/super dispatch on native path #1326 to track the native this/super/transitive CHA gap. Converted all 4 it.skipIf(engine === 'native') calls to it.todo('... (native gap #1326)') so the gap is visible as pending work in test output (todo), not hidden as silent skips. The WASM path tests continue to run and pass normally via if/else branching.

• PR title misrepresents scope: Noted — this PR was created to add the benchmark fixtures but the branch already contained the Phase 8.5 CHA implementation commits. The title mismatch is a git history concern (the commits themselves are correctly scoped). No code change needed; flagging only.

• Silent pass on rate-limit in embedding-regression tests: Fixed — replaced if (rateLimited) return with ctx.skip() (Vitest test context API) throughout. Rate-limited runs now show as skipped in Vitest output instead of passing green.

[carlos-alm]

@greptileai

[carlos-alm]

@claude

[claude]

Claude finished @carlos-alm's task —— View job

---

Re-review

All three issues from the initial review have been addressed. Verified:

1. it.skipIf(engine === 'native') → it.todo (CLAUDE.md compliance)

The 4 native-skipped tests are now converted to it.todo('... (native gap #1326)') via if (engine === 'native') { it.todo(...) } else { it(...) } branching (tests/integration/phase-8.5-cha-dispatch.test.ts:130-167, 180-208). The gap is tracked in issue #1326. WASM path tests still run and pass in the else branch. This is the correct pattern — the gap is visible as pending work in test output, not silently hidden.

2. CHA_DISPATCH_PENALTY exported and imported correctly

build-edges.ts:81: export const CHA_DISPATCH_PENALTY = 0.1;
native-orchestrator.ts:55: import { CHA_DISPATCH_PENALTY } from './build-edges.js';
Both paths now reference the same constant at native-orchestrator.ts:542. No more divergence risk.

3. Silent pass on rate-limit → ctx.skip()

tests/search/embedding-regression.test.ts:89-150: All if (rateLimited) return occurrences replaced with if (rateLimited) ctx.skip(). Rate-limited runs now show as skipped in Vitest output instead of passing green.

---

Assessment

Rating: ⭐⭐⭐⭐☆ (4/5)
Action: APPROVE

All three blocking/advisory concerns from the initial review are resolved. The implementation quality was already high — the fixes address CLAUDE.md compliance (engine parity documentation), constant duplication risk, and test output integrity.

The remaining minor note from Greptile (O(n) list.includes in buildChaContext) is a non-blocker for current codebase sizes and can be addressed as a follow-up if profiling warrants it.

View job

[carlos-alm]

Fixed the intermittent Windows CI failure: the afterAll in embedding-regression.test.ts now calls flushDeferredClose() before fs.rmSync, and retries up to 3 times with a 100ms backoff on EBUSY. This matches the existing readFileSafe retry pattern in the codebase and correctly handles the Windows SQLite WAL file lock that can persist briefly after db.close().

[carlos-alm]

@greptileai

[carlos-alm]

@claude

[claude]

Claude finished @carlos-alm's task —— View job

---

Re-review (Windows EBUSY fix)

• Gather context on latest fix
• Review embedding-regression.test.ts afterAll changes
• Assess overall PR readiness

---

Windows EBUSY Fix Assessment

The fix at tests/search/embedding-regression.test.ts:84-101 is correct and matches the documented pattern:

1. flushDeferredClose() call (line 89): Correctly calls the existing flushDeferredClose() from src/db/connection.ts:186 before attempting rmSync. The function synchronously drains any DB handles queued by closeDbDeferred(). This directly addresses the root cause — WAL file locks held by deferred handles.

2. Retry loop with Atomics.wait backoff (lines 92-100): 3 attempts with 100ms synchronous sleep between them. The Atomics.wait on a fresh SharedArrayBuffer is a valid synchronous sleep in a Node.js worker context. The error re-throw on the final attempt is correct — it won't silently swallow persistent failures.

3. Pattern consistency: The commit message claims it "matches the existing readFileSafe retry pattern." That's accurate in spirit (same retry-on-EBUSY approach), though readFileSafe uses async setTimeout. The sync Atomics.wait is appropriate here since afterAll can be synchronous.

One minor observation: flushDeferredClose() closes handles queued via closeDbDeferred(). The buildGraph call in beforeAll uses the standard DB open path — whether those handles end up in the deferred queue depends on the teardown path in buildGraph. If buildGraph doesn't use closeDbDeferred, the flushDeferredClose() call is a no-op (harmless). The retry loop is the actual fix; flushDeferredClose is a belt-and-suspenders addition. Not a problem.

---

Overall Assessment

All four issues from the review history are now resolved:

1. ✅ it.skipIf(engine === 'native') → it.todo('... (native gap #1326)')
2. ✅ CHA_DISPATCH_PENALTY exported and shared between WASM and native paths
3. ✅ ctx.skip() replacing silent return for rate-limited tests
4. ✅ Windows EBUSY retry with backoff in afterAll

Rating: ⭐⭐⭐⭐⭐ (5/5)
Action: APPROVE

The Windows fix is correct, targeted, and consistent with existing codebase patterns. The PR is ready to merge.

[carlos-alm]

Resolved merge conflict with main: call-resolver.ts had a 3-way conflict between main's effectiveReceiver strip (from parameter-flow PR #1308) and this PR's let typeName mutation + inline-new-expression handling + prototype-alias fallback. Both changes are intentional and non-overlapping — merged both sides. All other auto-merged files were clean. CI re-triggered.

[carlos-alm]

@greptileai