Skip to content

Commit 40b2d14

Browse files
authored
fix(cha): skip CHA expansion for super-dispatch edges
* fix(cha): skip CHA expansion for super-dispatch edges in runChaPostPass and runPostNativeCha When a method calls super.method(), the resolver creates an edge from the subclass method to the parent class method (e.g. PostMixin.m → A.m). runChaPostPass and runPostNativeCha then incorrectly CHA-expanded this edge to all sibling subclasses of A (e.g. B from other files that also extend A), producing false PostMixin.m → B.m edges across unrelated files. Fix: before BFS-expanding a caller→type.method edge, check whether the caller's class is itself a direct child of the target type. If so, the edge was produced by a super.method() call (going up the hierarchy) and must not be expanded downward to sibling subclasses. This applies to both runChaPostPass (WASM path) and runPostNativeCha (native orchestrator path), which share the same CHA expansion loop structure. Verified by node scripts/parity-compare.mjs --langs jelly-micro: the 6 PostMixin super4 false edges are eliminated; all 3048 tests pass. * fix(cha): replace heuristic super-dispatch guard with technique-based SQL filter The direct-child heuristic in runChaPostPass / runPostNativeCha had two flaws raised in review: 1. False positive: any B.method → A.method edge where B extends A was treated as a super-dispatch edge, including typed-parameter calls (param: A).foo() from within B.method, suppressing legitimate CHA expansion to sibling subclasses. 2. Deep-hierarchy miss: implementors.get(typeName) only returns direct children. For D extends C extends B, D is not in implementors.get("B"), so the guard failed to fire and the false expansion still occurred. Fix: tag super-dispatch edges with technique='super-dispatch' at insertion time (build-edges.ts for the WASM path, runPostNativeThisDispatch for the native path). The CHA expansion queries in runChaPostPass and runPostNativeCha then filter these edges out via SQL (AND e.technique != 'super-dispatch'), eliminating the heuristic entirely. Add Tiger.ts sibling fixture and a test asserting Lion.speak does not CHA-expand to Tiger.speak, covering both the fix and the regression. Impact: 6 functions changed, 6 affected
1 parent 33bcd93 commit 40b2d14

5 files changed

Lines changed: 55 additions & 12 deletions

File tree

src/domain/graph/builder/helpers.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,14 +464,15 @@ export function runChaPostPass(db: BetterSqlite3Database): number {
464464

465465
const callToMethods = db
466466
.prepare(
467-
`SELECT e.source_id, tgt.name AS method_name
467+
`SELECT e.source_id, src.name AS caller_name, tgt.name AS method_name
468468
FROM edges e
469469
JOIN nodes tgt ON e.target_id = tgt.id
470+
JOIN nodes src ON e.source_id = src.id
470471
WHERE e.kind = 'calls' AND tgt.kind = 'method'
471472
AND INSTR(tgt.name, '.') > 0
472-
AND (e.technique IS NULL OR e.technique != 'cha')`,
473+
AND (e.technique IS NULL OR e.technique != 'super-dispatch')`,
473474
)
474-
.all() as Array<{ source_id: number; method_name: string }>;
475+
.all() as Array<{ source_id: number; caller_name: string; method_name: string }>;
475476

476477
const seen = new Set<string>();
477478
// Scope deduplication to only the source_ids we are about to expand, avoiding

src/domain/graph/builder/stages/build-edges.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,10 @@ function buildChaPostPass(
748748
: computeConfidence(relPath, t.file, null) - CHA_DISPATCH_PENALTY;
749749
if (conf > 0) {
750750
seenByPair.add(edgeKey);
751-
allEdgeRows.push([caller.id, t.id, 'calls', conf, 0, 'cha']);
751+
// Tag super-dispatch edges distinctly so runChaPostPass can exclude them
752+
// from further CHA expansion (super calls are not virtual dispatch).
753+
const technique = call.receiver === 'super' ? 'super-dispatch' : 'cha';
754+
allEdgeRows.push([caller.id, t.id, 'calls', conf, 0, technique]);
752755
}
753756
}
754757
}

src/domain/graph/builder/stages/native-orchestrator.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -572,26 +572,37 @@ function runPostNativeCha(
572572
// incremental role reclassification; confidence uses CHA_TYPED_DISPATCH_CONFIDENCE matching runChaPostPass.
573573
// When scopeToChangedFiles is true, restrict to call sites in the changed files
574574
// (safe because no hierarchy or RTA evidence changed outside those files).
575-
let callToMethods: Array<{ source_id: number; method_name: string; caller_file: string | null }>;
575+
let callToMethods: Array<{
576+
source_id: number;
577+
caller_name: string;
578+
method_name: string;
579+
caller_file: string | null;
580+
}>;
576581
if (scopeToChangedFiles && changedFiles && changedFiles.length > 0) {
577582
const CHUNK_SIZE = 500;
578-
const rows: Array<{ source_id: number; method_name: string; caller_file: string | null }> = [];
583+
const rows: Array<{
584+
source_id: number;
585+
caller_name: string;
586+
method_name: string;
587+
caller_file: string | null;
588+
}> = [];
579589
for (let i = 0; i < changedFiles.length; i += CHUNK_SIZE) {
580590
const chunk = changedFiles.slice(i, i + CHUNK_SIZE);
581591
const ph = chunk.map(() => '?').join(',');
582592
const chunkRows = db
583593
.prepare(
584-
`SELECT e.source_id, tgt.name AS method_name, src.file AS caller_file
594+
`SELECT e.source_id, src.name AS caller_name, tgt.name AS method_name, src.file AS caller_file
585595
FROM edges e
586596
JOIN nodes tgt ON e.target_id = tgt.id
587597
JOIN nodes src ON e.source_id = src.id
588598
WHERE e.kind = 'calls' AND tgt.kind = 'method'
589599
AND INSTR(tgt.name, '.') > 0
590-
AND (e.technique IS NULL OR e.technique != 'cha')
600+
AND (e.technique IS NULL OR e.technique != 'super-dispatch')
591601
AND src.file IN (${ph})`,
592602
)
593603
.all(...chunk) as Array<{
594604
source_id: number;
605+
caller_name: string;
595606
method_name: string;
596607
caller_file: string | null;
597608
}>;
@@ -601,15 +612,20 @@ function runPostNativeCha(
601612
} else {
602613
callToMethods = db
603614
.prepare(`
604-
SELECT e.source_id, tgt.name AS method_name, src.file AS caller_file
615+
SELECT e.source_id, src.name AS caller_name, tgt.name AS method_name, src.file AS caller_file
605616
FROM edges e
606617
JOIN nodes tgt ON e.target_id = tgt.id
607618
JOIN nodes src ON e.source_id = src.id
608619
WHERE e.kind = 'calls' AND tgt.kind = 'method'
609620
AND INSTR(tgt.name, '.') > 0
610-
AND (e.technique IS NULL OR e.technique != 'cha')
621+
AND (e.technique IS NULL OR e.technique != 'super-dispatch')
611622
`)
612-
.all() as Array<{ source_id: number; method_name: string; caller_file: string | null }>;
623+
.all() as Array<{
624+
source_id: number;
625+
caller_name: string;
626+
method_name: string;
627+
caller_file: string | null;
628+
}>;
613629
}
614630

615631
// Seed seen-pairs only from the source_ids we'll be expanding — avoids loading every
@@ -950,7 +966,10 @@ async function runPostNativeThisDispatch(
950966
seen.add(key);
951967
const conf = computeConfidence(relPath, t.file, null) - CHA_DISPATCH_PENALTY;
952968
if (conf <= 0) continue;
953-
newEdges.push([callerRow.id, t.id, 'calls', conf, 0, 'cha']);
969+
// Tag super-dispatch edges distinctly so runPostNativeCha can exclude them
970+
// from further CHA expansion (super calls are not virtual dispatch).
971+
const technique = call.receiver === 'super' ? 'super-dispatch' : 'cha';
972+
newEdges.push([callerRow.id, t.id, 'calls', conf, 0, technique]);
954973
targetIds.add(t.id);
955974
affectedFiles.add(relPath);
956975
if (t.file) affectedFiles.add(t.file);
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { Animal } from './Animal.js';
2+
3+
export class Tiger extends Animal {
4+
speak(): string {
5+
return 'ROAR';
6+
}
7+
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,19 @@ describe.each(ENGINES)('Phase 8.5 CHA dispatch (%s)', (engine) => {
148148
).toBeDefined();
149149
});
150150

151+
it('super-dispatch: does NOT CHA-expand Lion.speak to sibling Tiger.speak', () => {
152+
// Tiger is also a subclass of Animal (sibling to Lion). A super.speak() call
153+
// in Lion.speak goes to Animal.speak — it must NOT be CHA-expanded to
154+
// Tiger.speak (a sibling), which Lion would never invoke.
155+
const edge = callEdges.find(
156+
(e) => e.caller_name === 'Lion.speak' && e.callee_name === 'Tiger.speak',
157+
);
158+
expect(
159+
edge,
160+
`Expected NO Lion.speak → Tiger.speak edge (super-dispatch must not expand to sibling subclasses).\nActual edges:\n${JSON.stringify(callEdges, null, 2)}`,
161+
).toBeUndefined();
162+
});
163+
151164
// ── transitive multi-level CHA (issue #1311) ───────────────────────────
152165
// Hierarchy: IJob → AbstractJob (non-instantiated) → PrintJob / ScanJob
153166
// resolveChaTargets must BFS through AbstractJob to reach the concrete types.

0 commit comments

Comments
 (0)