Skip to content

Commit a62bab7

Browse files
committed
fix(resolver): resolveByGlobal picks the single best match, not every candidate
resolveByGlobal's exact-name lookup returned every same-named symbol in the codebase that cleared the 0.5 directory-proximity confidence threshold, and resolveCallTargets/emitDirectCallEdgesForCall then emitted a calls edge to each one instead of the single most likely target. A bare (no-receiver) or this/self/super call carries no type info to disambiguate, so fanning out turned one real call site into N-1 false edges (e.g. four object-literal close() methods in src/db/connection.ts each getting a calls edge from every caller that destructures a close() handle). Fix: reduce the exact-name lookup to the single unambiguous highest-confidence candidate. A genuine tie at the top confidence has no principled winner and now resolves to nothing rather than betting on all of them, falling through to the narrower same-class-sibling fallback instead. Mirrored in both engines: src/domain/graph/resolver/strategy.ts (WASM/TS) and crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs (native). Updates two pre-existing tests whose fixtures/assertions had baked in the old fan-out behavior as expected (#1519's byName suite, #1752's reverse-dep reconnect baseline, which had already flagged this exact scenario as a "separate, pre-existing concern — filed independently"). Impact: 2 functions changed, 10 affected
1 parent 51b9e23 commit a62bab7

5 files changed

Lines changed: 276 additions & 58 deletions

File tree

crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs

Lines changed: 96 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,40 @@ fn find_enclosing_caller<'a>(defs: &[DefWithId<'a>], call_line: u32, file_node_i
934934
(file_node_id, "")
935935
}
936936

937+
/// Step 2 of the scoped (this/self/super or no-receiver) fallback: exact global
938+
/// name lookup. Mirrors `resolveExactGlobalMatch` in
939+
/// `src/domain/graph/resolver/strategy.ts`.
940+
///
941+
/// A bare/this/self/super call carries no type qualifier, so `nodes_by_name`
942+
/// can return every same-named symbol in the codebase, filtered only by the
943+
/// loose directory-proximity confidence threshold. Returning all of them
944+
/// turns a single real call site into N-1 false `calls` edges (#1863). Only
945+
/// a single highest-confidence candidate is trustworthy — a tie at the top
946+
/// confidence (e.g. several files at the same directory depth from the
947+
/// caller) is genuinely ambiguous and returns nothing, letting the caller
948+
/// fall through to the narrower same-class-sibling fallback.
949+
fn resolve_exact_global_match<'a>(
950+
ctx: &EdgeContext<'a>,
951+
call_name: &str,
952+
rel_path: &str,
953+
) -> Vec<&'a NodeInfo> {
954+
let scored: Vec<(&'a NodeInfo, f64)> = ctx.nodes_by_name
955+
.get(call_name)
956+
.map(|v| v.iter()
957+
.map(|&n| (n, resolve::compute_confidence(rel_path, &n.file, None)))
958+
.filter(|&(_, confidence)| confidence >= 0.5)
959+
.collect())
960+
.unwrap_or_default();
961+
if scored.is_empty() { return Vec::new(); }
962+
963+
let best_confidence = scored.iter().map(|&(_, confidence)| confidence).fold(f64::MIN, f64::max);
964+
let best: Vec<&'a NodeInfo> = scored.iter()
965+
.filter(|&&(_, confidence)| confidence == best_confidence)
966+
.map(|&(n, _)| n)
967+
.collect();
968+
if best.len() == 1 { best } else { Vec::new() }
969+
}
970+
937971
/// Multi-strategy call target resolution: import-aware → same-file → type-aware → scoped.
938972
/// `caller_name` is the enclosing function/method name (e.g. `"Shape.describe"`) used to scope
939973
/// `this`/`self`/`super` dispatch to the caller's own class before falling back to a broader scan.
@@ -1180,12 +1214,7 @@ fn resolve_call_targets<'a>(
11801214
}
11811215

11821216
// First try exact name match (e.g. an unqualified function named "area").
1183-
let exact: Vec<&NodeInfo> = ctx.nodes_by_name
1184-
.get(call.name.as_str())
1185-
.map(|v| v.iter()
1186-
.filter(|n| resolve::compute_confidence(rel_path, &n.file, None) >= 0.5)
1187-
.copied().collect())
1188-
.unwrap_or_default();
1217+
let exact = resolve_exact_global_match(ctx, call.name.as_str(), rel_path);
11891218
if !exact.is_empty() { return exact; }
11901219

11911220
// Class-scoped exact lookup: prefer `ClassName.method` when the caller is a qualified
@@ -2854,6 +2883,67 @@ mod call_edge_tests {
28542883
);
28552884
}
28562885

2886+
/// Issue #1863: several same-named object-literal `close() {}` methods
2887+
/// scattered under sibling directories two levels below the caller all
2888+
/// score the same 0.5 "grandparent proximity" confidence. A bare `close()`
2889+
/// call must not fan out into a `calls` edge to every one of them — a
2890+
/// genuine top-confidence tie is ambiguous and must resolve to nothing.
2891+
#[test]
2892+
fn global_fallback_tie_does_not_fan_out() {
2893+
let all_nodes = vec![
2894+
node(1, "caller", "function", "src/presentation/caller.ts", 3),
2895+
node(2, "close", "method", "src/db/connection.ts", 10),
2896+
node(3, "close", "method", "src/domain/target2.ts", 20),
2897+
node(4, "close", "method", "src/features/target3.ts", 30),
2898+
];
2899+
let files = vec![make_file(
2900+
"src/presentation/caller.ts",
2901+
10,
2902+
vec![def("caller", "function", 3, 8)],
2903+
vec![call("close", 5, None)],
2904+
vec![],
2905+
vec![],
2906+
)];
2907+
let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS);
2908+
assert!(
2909+
!edges.iter().any(|e| e.kind == "calls" && e.source_id == 1),
2910+
"ambiguous same-confidence candidates must not fan out into calls edges; got: {:?}",
2911+
edges.iter().map(|e| (&e.kind, e.source_id, e.target_id)).collect::<Vec<_>>()
2912+
);
2913+
}
2914+
2915+
/// Companion to `global_fallback_tie_does_not_fan_out`: when one candidate
2916+
/// has a strictly higher confidence than the rest, the clear single winner
2917+
/// must still resolve — only genuine top-confidence ties are dropped.
2918+
#[test]
2919+
fn global_fallback_resolves_unambiguous_best_match() {
2920+
let all_nodes = vec![
2921+
node(1, "caller", "function", "src/presentation/caller.ts", 3),
2922+
// Same directory as the caller → confidence 0.7, the clear winner.
2923+
node(2, "close", "method", "src/presentation/sibling.ts", 10),
2924+
// Two directories away → confidence 0.5, tied with each other but not with node 2.
2925+
node(3, "close", "method", "src/domain/target2.ts", 20),
2926+
node(4, "close", "method", "src/features/target3.ts", 30),
2927+
];
2928+
let files = vec![make_file(
2929+
"src/presentation/caller.ts",
2930+
10,
2931+
vec![def("caller", "function", 3, 8)],
2932+
vec![call("close", 5, None)],
2933+
vec![],
2934+
vec![],
2935+
)];
2936+
let edges = build_call_edges(files, all_nodes, vec![], MAX_SOLVER_ITERATIONS);
2937+
let call_edges: Vec<_> = edges.iter().filter(|e| e.kind == "calls" && e.source_id == 1).collect();
2938+
assert_eq!(
2939+
call_edges.len(),
2940+
1,
2941+
"expected exactly one calls edge (the unambiguous best match); got: {:?}",
2942+
edges.iter().map(|e| (&e.kind, e.source_id, e.target_id)).collect::<Vec<_>>()
2943+
);
2944+
assert_eq!(call_edges[0].target_id, 2, "expected the same-directory candidate to win");
2945+
}
2946+
28572947
/// Receiver-edge confidence must propagate the stored typeMap confidence
28582948
/// (e.g. 0.85 from a pts property-write) instead of a flat 0.9 — mirrors
28592949
/// `typeConfidence ?? (typeName ? 0.9 : 0.7)` in resolveReceiverEdge.

src/domain/graph/resolver/strategy.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -325,14 +325,47 @@ function resolveViaSameClassSibling(
325325
.filter((t) => t.kind === 'method' && computeConfidence(relPath, t.file, null) >= 0.5);
326326
}
327327

328+
/**
329+
* Step 2: exact global name lookup.
330+
*
331+
* Unlike `resolveByReceiver`'s cascade, a bare (no-receiver) or `this`/`self`/
332+
* `super` call carries no type qualifier to narrow the search — `lookup.byName`
333+
* can return every unrelated symbol in the codebase that happens to share the
334+
* name, filtered only by the loose directory-proximity confidence threshold.
335+
* Emitting a `calls` edge to every one of those turns a single real call site
336+
* into N-1 false edges (#1863). With no receiver/type info to break a tie,
337+
* only a single highest-confidence candidate is trustworthy:
338+
* - a unique highest-confidence candidate wins outright.
339+
* - a tie at the highest confidence (e.g. several files at the same
340+
* directory depth from the caller — the exact #1863 repro) is genuinely
341+
* ambiguous: there is no principled way to prefer one over another, so
342+
* the match is dropped rather than fanning out, letting the caller fall
343+
* through to the narrower resolveViaSameClassSibling fallback.
344+
*/
345+
function resolveExactGlobalMatch(
346+
lookup: StrategyLookup,
347+
call: { name: string },
348+
relPath: string,
349+
): ReadonlyArray<{ id: number; file: string }> {
350+
const scored = lookup
351+
.byName(call.name)
352+
.map((target) => ({ target, confidence: computeConfidence(relPath, target.file, null) }))
353+
.filter((candidate) => candidate.confidence >= 0.5);
354+
if (scored.length === 0) return [];
355+
356+
const bestConfidence = Math.max(...scored.map((candidate) => candidate.confidence));
357+
const best = scored.filter((candidate) => candidate.confidence === bestConfidence);
358+
return best.length === 1 ? [best[0]!.target] : [];
359+
}
360+
328361
/**
329362
* Resolve a call site with no receiver, or whose receiver is `this`, `self`,
330363
* or `super`.
331364
*
332365
* Resolution cascade:
333366
* 1. resolveViaAccessorThisDispatch — Object.defineProperty this-dispatch (Phase 8.3f).
334-
* 2. Exact global name lookup with confidence filter.
335-
* 3. resolveViaSameClassSibling — same-class sibling method fallback.
367+
* 2. resolveExactGlobalMatch — exact global name lookup, unambiguous best match only.
368+
* 3. resolveViaSameClassSibling — same-class sibling method fallback.
336369
*/
337370
export function resolveByGlobal(
338371
lookup: StrategyLookup,
@@ -344,9 +377,7 @@ export function resolveByGlobal(
344377
const viaAccessor = resolveViaAccessorThisDispatch(lookup, typeMap, call, relPath, callerName);
345378
if (viaAccessor.length > 0) return viaAccessor;
346379

347-
const exact = lookup
348-
.byName(call.name)
349-
.filter((t) => computeConfidence(relPath, t.file, null) >= 0.5);
380+
const exact = resolveExactGlobalMatch(lookup, call, relPath);
350381
if (exact.length > 0) return exact;
351382

352383
const sameClass = resolveViaSameClassSibling(lookup, call, relPath, callerName);

tests/integration/issue-1519-confidence-sorted-dedup.test.ts

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,31 @@
55
* buildFileCallEdges (build-edges.ts lines 1126-1132) before the emit loop,
66
* and matching sorts to the Phase 8.3 pts alias loops.
77
*
8-
* WHAT THESE TESTS GUARD
9-
* ──────────────────────
10-
* Both fixtures resolve multiple candidates with distinct node IDs (different
11-
* files → different graph nodes). The `seenCallEdges` / `ptsEdgeRows` dedup key
12-
* is `${caller.id}|${t.id}`, so dedup never fires across distinct nodes.
8+
* WHAT THE byName SUITE GUARDS (updated for #1863)
9+
* ──────────────────────────────────────────────────
10+
* `resolveByGlobal`'s exact-name lookup (the byName fixture below) used to
11+
* emit a `calls` edge to every same-named candidate it found, regardless of
12+
* how many there were. #1863 fixed this: when candidates have a unique
13+
* highest-confidence winner, only that one resolves — a lower-confidence
14+
* candidate is dropped rather than fanning out into a false edge. A genuine
15+
* tie at the top confidence (no candidate strictly wins) resolves to no edge
16+
* at all, since there is no receiver/type info to break it.
1317
*
14-
* These tests therefore verify the confidence SCORING invariant:
15-
* • A near-directory target always receives a higher confidence value than a
16-
* far-directory target for the same call site.
17-
* • Both edges are emitted (neither is suppressed by sort logic).
18+
* The byName suite below now verifies:
19+
* • The near-directory (higher-confidence) candidate is the one that
20+
* resolves, with confidence ≥ 0.7.
21+
* • The far-directory (lower-confidence) candidate does NOT get an edge —
22+
* it is a strictly worse match, not a genuine tie.
23+
*
24+
* WHAT THE pts-alias SUITE GUARDS (unchanged by #1863)
25+
* ──────────────────────────────────────────────────────
26+
* The pts alias fixture resolves through a separate points-to alias
27+
* mechanism (Phase 8.3), not `resolveByGlobal` — #1863 does not touch it.
28+
* That fixture can still resolve multiple candidates with distinct node IDs;
29+
* the `ptsEdgeRows` dedup key is `${caller.id}|${t.id}`, so dedup never fires
30+
* across distinct nodes there. It verifies the confidence SCORING invariant:
31+
* the local, highest-confidence alias target wins the `ptsEdgeRows` slot
32+
* ahead of a lower-confidence one.
1833
*
1934
* WHAT THESE TESTS DO NOT GUARD
2035
* ──────────────────────────────
@@ -42,12 +57,14 @@ import { buildGraph } from '../../src/domain/graph/builder.js';
4257
// src/consumer.js calls helper() without importing it.
4358
// Two files define helper(): one in the same directory (high confidence),
4459
// one in a different directory (low confidence).
45-
// Both edges are expected, each with the correct confidence for its file proximity.
60+
// Since #1863, only the unambiguous highest-confidence candidate (near)
61+
// resolves — the far, lower-confidence candidate is dropped rather than
62+
// fanning out into a false edge.
4663

4764
const MULTI_TARGET_FIXTURE: Record<string, string> = {
4865
'src/consumer.js': `
4966
export function process() {
50-
helper(); // no import — resolves via byName to both src/helper.js and other/helper.js
67+
helper(); // no import — resolves via byName; only the near candidate wins (#1863)
5168
}
5269
`.trimStart(),
5370

@@ -127,7 +144,7 @@ function writeFixture(baseDir: string, files: Record<string, string>): void {
127144

128145
// ── Multi-target byName suite ──────────────────────────────────────────────────
129146

130-
describe('confidence-sorted dedup: multi-target byName resolution (#1519)', () => {
147+
describe('confidence-sorted dedup: multi-target byName resolution (#1519, #1863)', () => {
131148
let tmpDir: string;
132149

133150
beforeAll(async () => {
@@ -166,31 +183,24 @@ describe('confidence-sorted dedup: multi-target byName resolution (#1519)', () =
166183
expect(nearEdge!.confidence).toBeGreaterThanOrEqual(0.7);
167184
});
168185

169-
it('far-target edge (other/helper.js) has confidence < 0.7 (different-directory proximity)', () => {
186+
it('does not emit an edge to the far, lower-confidence helper (other/helper.js) (#1863)', () => {
170187
const edges = readEdgesWithConfidence(path.join(tmpDir, '.codegraph', 'graph.db'));
171188
const farEdge = edges.find(
172189
(e) => e.source === 'process' && e.target === 'helper' && e.target_file === 'other/helper.js',
173190
);
174-
expect(farEdge).toBeDefined();
175-
// computeConfidence: different parent directory → 0.3 or 0.5.
176-
// The far target must score lower than the near target.
177-
expect(farEdge!.confidence).toBeLessThan(0.7);
191+
// computeConfidence: different parent directory → 0.3 or 0.5, strictly below
192+
// the near candidate's 0.7. Since #1863, a strictly-worse candidate is
193+
// dropped rather than emitted alongside the winner as a false edge.
194+
expect(farEdge).toBeUndefined();
178195
});
179196

180-
it('near-target confidence is strictly greater than far-target confidence', () => {
197+
it('emits exactly one calls edge for the ambiguous helper() call site (#1863)', () => {
181198
const edges = readEdgesWithConfidence(path.join(tmpDir, '.codegraph', 'graph.db'));
182-
const nearEdge = edges.find(
183-
(e) => e.source === 'process' && e.target === 'helper' && e.target_file === 'src/helper.js',
184-
);
185-
const farEdge = edges.find(
186-
(e) => e.source === 'process' && e.target === 'helper' && e.target_file === 'other/helper.js',
187-
);
188-
expect(nearEdge).toBeDefined();
189-
expect(farEdge).toBeDefined();
190-
// The sort at build-edges.ts:1126-1132 processes high-confidence targets first.
191-
// Both edges are stored (different node IDs); this assertion verifies the
192-
// confidence ordering invariant that the sort enforces.
193-
expect(nearEdge!.confidence).toBeGreaterThan(farEdge!.confidence);
199+
const helperEdges = edges.filter((e) => e.source === 'process' && e.target === 'helper');
200+
// Only the unambiguous highest-confidence candidate resolves — no fan-out
201+
// to every same-named candidate the global fallback finds.
202+
expect(helperEdges).toHaveLength(1);
203+
expect(helperEdges[0]!.target_file).toBe('src/helper.js');
194204
});
195205
});
196206

0 commit comments

Comments
 (0)