Skip to content

Commit 2a07fa5

Browse files
clay-goodclaude
andcommitted
feat(cha): layered base-class resolution recovers cross-file override edges
Measuring the prior ambiguity-skip on real repos showed it was far too conservative: it dropped a cross-file override edge whenever the base class name was reused ANYWHERE — ~37% of all base-references on Laravel (1034/2810), a large recall loss on exactly the big namespaced codebases CHA targets. Replaced the bare same-file-then-skip logic in buildClassNodes with layered, most-specific-evidence-first resolution: 1. a class of that name in the child's own file; 2. the file the child imports the name from (existing importMap, now threaded in — covers Java/TS/JS/Python/Go/Rust/Ruby); 3. a class of that name unique within the child's directory (same package); 4. a globally-unique class of that name; 5. otherwise skip (genuinely ambiguous across dirs, no import). Layers 1-4 each carry real evidence; only truly unresolvable bases are skipped, keeping the false-negatives-over-false-positives bias. Dogfooded: - DesignPatternsPHP 71 -> 87 override edges: recovered the real FactoryMethod/Logger, StaticFactory/Formatter, and Bridge/Formatter hierarchies the skip had dropped (each implementer resolves to its OWN directory's base); cross-namespace false edges stay gone; zero cross-top-directory edges. - Laravel (1640 files): 1758 override edges / 512 cross-package; an adversarial agent audit of all reused-name families (Builder/Grammar/Connection/Driver/Guard/Loader/... incl. all 6 globally-ambiguous base names) found ZERO false positives — import resolves cross-package Contracts/* bases, directory-locality resolves same-namespace siblings (Query vs Schema), never cross-wiring. ~100% precision with a large recall gain. Residual: a globally-duplicated base name neither same-dir-unique nor import-disambiguated is still skipped (rare). Full FQCN extraction for non-importMap languages (PHP/Kotlin/Swift/Scala/ C#) is the further future enhancement. Tests +1; full suite green (3763). Decision: 320bf215. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 03eb4f4 commit 2a07fa5

4 files changed

Lines changed: 81 additions & 7 deletions

File tree

openspec/changes/archive/add-type-hierarchy-resolved-dispatch/tasks.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,31 @@
9595
> (`cha.test.ts` Kotlin/PHP/Swift/Scala override + ambiguous-cross-file + Kotlin-qualified-supertype;
9696
> `graph.test.ts` bfsFromDB DB-path propagation). Full suite green (3762). Languages with CHA hierarchy
9797
> support: TS/JS, Python, Java, C++, C#, Ruby, Go, Kotlin, PHP, Swift, Scala.
98+
>
99+
> **Layered base resolution — recall recovery (follow-up, same PR #155).** Measuring the previous
100+
> ambiguity-skip on real repos showed it was far too conservative: it dropped a cross-file override
101+
> edge whenever the base class name was reused ANYWHERE — **~37% of all base-references on Laravel
102+
> (1,034 of 2,810)**. Replaced the bare same-file-then-skip logic in `buildClassNodes` with layered,
103+
> most-specific-evidence-first resolution: **(1)** same file → **(2)** the file the child imports the
104+
> name from (`importMap`, now threaded in — covers Java/TS/JS/Python/Go/Rust/Ruby) → **(3)** unique
105+
> within the child's directory (same package) → **(4)** globally unique → **(5)** skip (genuinely
106+
> ambiguous across directories with no import). Layers 1–4 each carry real evidence; only truly
107+
> unresolvable bases are skipped, preserving the false-negative-over-false-positive bias.
108+
> Dogfooded:
109+
> - **DesignPatternsPHP** 71 → 87 override edges: recovered the real `FactoryMethod/Logger`,
110+
> `StaticFactory/Formatter`, and `Bridge/Formatter` hierarchies the skip had dropped — each
111+
> implementer now resolves to its OWN directory's base — while the cross-namespace false edges stay
112+
> gone, and ZERO cross-top-directory edges remain.
113+
> - **Laravel** (1,640 files): 1,758 override edges, 512 cross-package. An adversarial agent audit of
114+
> the riskiest reused names (`Builder`/`Grammar`/`Connection`/`Driver`/`Guard`/`Loader`/… incl. all
115+
> 6 genuinely globally-ambiguous base names) found **zero false positives** — import resolves the
116+
> cross-package `Contracts/*` bases, directory-locality resolves same-namespace siblings (Query vs
117+
> Schema `Builder`/`Grammar`), never cross-wiring. Precision held at ~100% with a large recall gain.
118+
>
119+
> Residual: a globally-duplicated base name that is neither same-dir-unique NOR import-disambiguated is
120+
> still skipped (rare; e.g. a non-importMap language using cross-namespace inheritance without same-dir
121+
> co-location). Tests: +1 (`cha.test.ts` directory-locality recovery resolves to the correct twin).
122+
> Full suite green (3763).
98123
99124
## 1. Confirm the surface is purely additive (no type changes)
100125
- [x] Verify `EdgeConfidence` already includes `'synthesized'` (`call-graph.ts:34`), `CallEdge` already

openspec/specs/analyzer/spec.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5571,6 +5571,12 @@ The system SHALL skip cross-file parent resolution when the parent name is ambig
55715571

55725572
> Decision recorded: d5967a48
55735573
> Date: 2026-06-17
5574+
### Requirement: ResolveChaBaseClassesByLayeredEvidenceSamefileImportSamedirectoryGlobaluniqueInsteadOfBarenamethenskip
5575+
5576+
The system SHALL resolve CHA base-class references using layered evidence (same-file, import, same-directory, global-unique) before falling back to ambiguity-skip.
5577+
5578+
> Decision recorded: 320bf215
5579+
> Date: 2026-06-17
55745580
55755581
## Technical Notes
55765582

@@ -6068,3 +6074,13 @@ The polymorphic-dispatch (CHA) feature requires class-hierarchy edges for each s
60686074
When multiple classes share a bare name across different files (e.g. two unrelated `Logger` interfaces in different PHP namespaces), a global first-match would fabricate a false override edge and steal the real one from the correct twin; skipping the resolution entirely is safer for downstream dispatch accuracy.
60696075

60706076
**Consequences:** Some legitimate cross-file inheritance edges will be missed when name collisions exist; this is an acceptable precision-over-recall tradeoff that avoids polluting the call graph with phantom dispatch targets.
6077+
6078+
### Resolve CHA base classes by layered evidence (same-file → import → same-directory → global-unique) instead of bare-name-then-skip
6079+
6080+
**Status:** Approved
6081+
**Date:** 2026-06-17
6082+
**ID:** 320bf215
6083+
6084+
The prior ambiguity-skip in buildClassNodes dropped cross-file override edges whenever the base class name was reused anywhere in the codebase — measured at ~37% of all base-references on Laravel (1034/2810). Replaced with layered, most-specific-evidence-first resolution: (1) class in child's own file; (2) file the child imports the name from via importMap; (3) class unique within child's directory (same package); (4) globally-unique class; (5) skip. Each layer carries real evidence, so only genuinely-ambiguous cross-directory bases with no import are skipped, preserving the false-negatives-over-false-positives bias while recovering legitimate edges.
6085+
6086+
**Consequences:** Threads the existing importMap into buildClassNodes (new optional param). Recall recovered substantially with no precision loss: DesignPatternsPHP 71→87 override edges, Laravel 1758 override edges / 512 cross-package at ~100% precision. Residual: globally-duplicated base names without import or same-dir co-location are still skipped. True FQCN/namespace extraction for non-importMap languages (PHP/Kotlin/Swift/Scala/C#) remains a future enhancement.

src/core/analyzer/call-graph.ts

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2905,6 +2905,7 @@ async function extractClassRelationships(
29052905
function buildClassNodes(
29062906
allNodes: Map<string, FunctionNode>,
29072907
relationships: Map<string, { parentClasses: string[]; interfaces: string[] }>,
2908+
importMap?: ImportMap,
29082909
): { classes: ClassNode[]; inheritanceEdges: InheritanceEdge[] } {
29092910
// Group FunctionNodes by (filePath, className).
29102911
// Free functions use a synthetic "[basename]" module name keyed by filePath alone.
@@ -2964,19 +2965,33 @@ function buildClassNodes(
29642965
// genuine cross-file inheritance falls back to the global first match.
29652966
const byName = new Map<string, ClassNode>();
29662967
const nameCount = new Map<string, number>();
2968+
const byNameAndDir = new Map<string, ClassNode[]>(); // `${dir}\0${name}` → classes
2969+
const dirOf = (p: string): string => { const i = p.lastIndexOf('/'); return i >= 0 ? p.slice(0, i) : ''; };
29672970
for (const cls of classMap.values()) {
29682971
nameCount.set(cls.name, (nameCount.get(cls.name) ?? 0) + 1);
29692972
if (!byName.has(cls.name)) byName.set(cls.name, cls);
2973+
const dk = `${dirOf(cls.filePath)}\0${cls.name}`;
2974+
const arr = byNameAndDir.get(dk);
2975+
if (arr) arr.push(cls); else byNameAndDir.set(dk, [cls]);
29702976
}
2977+
// Resolve a base/interface NAME to a ClassNode, most-specific evidence first:
2978+
// 1. same file 2. the file the child imports the name from 3. unique within the
2979+
// child's directory (same package) 4. globally unique 5. otherwise SKIP.
2980+
// Earlier layers carry real evidence (declaration site, import, package); the global-
2981+
// unique fallback is safe (only one candidate). When the bare name is ambiguous across
2982+
// directories and no import disambiguates it (e.g. several namespaced `Builder` classes),
2983+
// skip rather than guess a first-match — false-negatives over false-positives.
29712984
const resolveParent = (parentName: string, childFile: string): ClassNode | undefined => {
29722985
const sameFile = classMap.get(`${childFile}::${parentName}`);
29732986
if (sameFile) return sameFile;
2974-
// The base is not declared in the child's file and its bare name is AMBIGUOUS
2975-
// (several classes share it across files — e.g. two unrelated `Logger` interfaces
2976-
// in different PHP namespaces). A global first-match would both fabricate a false
2977-
// override edge AND steal the real one from the correct twin, so skip rather than
2978-
// guess — false-negatives over false-positives.
2979-
if ((nameCount.get(parentName) ?? 0) > 1) return undefined;
2987+
const importedFrom = importMap?.get(childFile)?.get(parentName);
2988+
if (importedFrom) {
2989+
const viaImport = classMap.get(`${importedFrom}::${parentName}`);
2990+
if (viaImport) return viaImport;
2991+
}
2992+
const sameDir = byNameAndDir.get(`${dirOf(childFile)}\0${parentName}`);
2993+
if (sameDir && sameDir.length === 1) return sameDir[0];
2994+
if ((nameCount.get(parentName) ?? 0) > 1) return undefined; // ambiguous across dirs → skip
29802995
return byName.get(parentName);
29812996
};
29822997

@@ -4764,7 +4779,7 @@ export class CallGraphBuilder {
47644779

47654780
// Pass 7: Build class hierarchy (inheritance + grouping)
47664781
const relationships = await extractClassRelationships(files);
4767-
const { classes, inheritanceEdges } = buildClassNodes(allNodes, relationships);
4782+
const { classes, inheritanceEdges } = buildClassNodes(allNodes, relationships, importMap);
47684783
// Merge IaC module groupings (deduped by id) into the class set.
47694784
const classIds = new Set(classes.map(c => c.id));
47704785
for (const c of iacClasses) if (!classIds.has(c.id)) classes.push(c);

src/core/analyzer/cha.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,24 @@ describe('CHA — ambiguous cross-file base names', () => {
245245
// file AND is ambiguous (several classes share it across files), resolution must
246246
// skip — guessing a global first-match both fabricates a false override edge and
247247
// steals the real one. Bias: false-negatives over false-positives.
248+
it('recovers a cross-file override edge via same-directory resolution (correct twin)', async () => {
249+
// Two same-named `Logger` interfaces in different directories, each with an
250+
// implementer in a SEPARATE file within its own directory. Same-directory
251+
// resolution must wire each implementer to ITS directory's Logger — recovering
252+
// the real edge that the bare-name ambiguity-skip would have dropped, without
253+
// cross-wiring to the other directory's twin.
254+
const b = await new CallGraphBuilder().build([
255+
{ path: 'a/Logger.ts', content: `export class Logger { log() { return 1; } }`, language: 'TypeScript' },
256+
{ path: 'a/FileLogger.ts', content: `import { Logger } from './Logger'; export class FileLogger extends Logger { log() { return 2; } }`, language: 'TypeScript' },
257+
{ path: 'b/Logger.ts', content: `export class Logger { log() { return 9; } }`, language: 'TypeScript' },
258+
]);
259+
const aLog = 'a/Logger.ts::Logger.log';
260+
const bLog = 'b/Logger.ts::Logger.log';
261+
const fileLog = 'a/FileLogger.ts::FileLogger.log';
262+
expect(b.edges.some(e => e.synthesizedBy === 'override' && e.callerId === aLog && e.calleeId === fileLog)).toBe(true);
263+
expect(b.edges.some(e => e.synthesizedBy === 'override' && e.callerId === bLog && e.calleeId === fileLog)).toBe(false);
264+
});
265+
248266
it('does not synthesize an override edge to an ambiguous cross-file base', async () => {
249267
const b = await new CallGraphBuilder().build([
250268
{ path: 'a.ts', content: `export class Logger { log() { return 1; } }`, language: 'TypeScript' },

0 commit comments

Comments
 (0)