Skip to content

Commit 71e9174

Browse files
committed
fix: replace fixed-depth directory-proximity check with symmetric distance (docs check acknowledged)
computeConfidence (and its Rust mirror compute_confidence) scored call-edge resolution confidence using a fixed-depth check: same-directory, or dirname(dirname(caller)) === dirname(dirname(target)) for a "sibling directory" tier. That equality only holds when both files sit at the same depth, so a file in a subdirectory calling a method on a class declared in its direct parent directory (e.g. graph/algorithms/bfs.ts calling a method on the CodeGraph class in graph/model.ts) was scored as maximally distant (0.3) even though the receiver's type had already been correctly resolved via typeMap (from the parameter's `graph: CodeGraph` type annotation). That score fell below the 0.5 threshold used by resolveViaTypedMethod, silently dropping the call edge. Replaced both fixed-depth checks with a symmetric, depth-independent directory-tree distance (hops to the nearest common ancestor), preserving the existing same-directory (0.7) and sibling (0.5) tiers exactly and adding a new tier for direct parent/child directory nesting (0.6). Verified via the resolution-benchmark suite before/after the fix: all 42 language fixtures produce byte-identical precision/recall/TP/FP/FN numbers (aggregate recall 63.8%, 355/556 edges) — no regression. Fixes #1769 Impact: 3 functions changed, 26 affected
1 parent 986c851 commit 71e9174

4 files changed

Lines changed: 353 additions & 22 deletions

File tree

crates/codegraph-core/src/domain/graph/resolve.rs

Lines changed: 120 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,42 @@ fn resolve_import_path_inner(
247247
relativize_to_root(&resolved.display().to_string().replace('\\', "/"), root_dir)
248248
}
249249

250+
/// All ancestor directories of `dir`, starting with `dir` itself, walking up to the root.
251+
fn ancestor_chain(dir: &str) -> Vec<String> {
252+
let mut chain = vec![dir.to_string()];
253+
let mut cur = dir.to_string();
254+
while let Some(parent) = Path::new(&cur).parent() {
255+
let parent_str = parent.display().to_string();
256+
chain.push(parent_str.clone());
257+
cur = parent_str;
258+
}
259+
chain
260+
}
261+
262+
/// Directory-tree distance between two directories: hops up from `a` to the
263+
/// nearest ancestor shared with `b`, plus hops down from there to `b`.
264+
///
265+
/// Symmetric and depth-independent — unlike a fixed-depth equality check
266+
/// (e.g. comparing the parent-of-parent of `a` to the parent-of-parent of
267+
/// `b`, as `compute_confidence` used to), this correctly scores both sibling
268+
/// directories (common parent) and direct ancestor/descendant directories
269+
/// (one nested inside the other) regardless of how deep either path is. The
270+
/// fixed-depth check only matched when both files sat at the *same* depth,
271+
/// so e.g. a file in `graph/algorithms/*.rs` calling a method declared in
272+
/// the shallower `graph/model.rs` was scored as maximally distant (issue #1769).
273+
fn directory_distance(a: &str, b: &str) -> usize {
274+
let chain_a = ancestor_chain(a);
275+
let chain_b = ancestor_chain(b);
276+
for (i, dir_a) in chain_a.iter().enumerate() {
277+
if let Some(j) = chain_b.iter().position(|dir_b| dir_b == dir_a) {
278+
return i + j;
279+
}
280+
}
281+
usize::MAX
282+
}
283+
250284
/// Compute proximity-based confidence for call resolution.
251-
/// Mirrors `computeConfidence()` in builder.js.
285+
/// Mirrors `computeConfidence()` in resolve.ts.
252286
pub fn compute_confidence(
253287
caller_file: &str,
254288
target_file: &str,
@@ -275,24 +309,12 @@ pub fn compute_confidence(
275309
.map(|p| p.display().to_string())
276310
.unwrap_or_default();
277311

278-
if caller_dir == target_dir {
279-
return 0.7;
280-
}
281-
282-
let caller_parent = Path::new(&caller_dir)
283-
.parent()
284-
.map(|p| p.display().to_string())
285-
.unwrap_or_default();
286-
let target_parent = Path::new(&target_dir)
287-
.parent()
288-
.map(|p| p.display().to_string())
289-
.unwrap_or_default();
290-
291-
if caller_parent == target_parent {
292-
return 0.5;
312+
match directory_distance(&caller_dir, &target_dir) {
313+
0 => 0.7, // same directory
314+
1 => 0.6, // direct parent/child directory
315+
2 => 0.5, // sibling directories, or a grandparent/grandchild pair
316+
_ => 0.3,
293317
}
294-
295-
0.3
296318
}
297319

298320
/// Batch resolve multiple imports (parallelized with rayon).
@@ -430,4 +452,84 @@ mod tests {
430452
);
431453
assert_eq!(result, "src/utils.ts");
432454
}
455+
456+
// Regression tests for #1769: a fixed-depth "grandparent equality" check
457+
// used to compare the parent of `caller_dir` to the parent of `target_dir`,
458+
// which only matched when both files sat at the *same* depth. A file in a
459+
// subdirectory calling a method declared in its direct parent directory
460+
// (e.g. `graph/algorithms/bfs.rs` calling `graph/model.rs`) was scored as
461+
// maximally distant (0.3) purely because the two files were nested at
462+
// different depths — well below the 0.5 threshold used by the call-edge
463+
// resolver's typed-method lookup, silently dropping the call edge.
464+
465+
#[test]
466+
fn compute_confidence_scores_parent_child_dirs_above_resolver_threshold() {
467+
let conf = compute_confidence("src/graph/algorithms/bfs.rs", "src/graph/model.rs", None);
468+
assert!(conf >= 0.5, "expected >= 0.5, got {conf}");
469+
}
470+
471+
#[test]
472+
fn compute_confidence_is_symmetric_for_parent_child_dirs() {
473+
let caller_deeper =
474+
compute_confidence("src/graph/algorithms/bfs.rs", "src/graph/model.rs", None);
475+
let target_deeper =
476+
compute_confidence("src/graph/model.rs", "src/graph/algorithms/bfs.rs", None);
477+
assert_eq!(caller_deeper, target_deeper);
478+
}
479+
480+
#[test]
481+
fn compute_confidence_ranks_parent_child_between_same_dir_and_sibling() {
482+
let same_dir = compute_confidence("src/graph/a.rs", "src/graph/b.rs", None);
483+
let parent_child =
484+
compute_confidence("src/graph/algorithms/bfs.rs", "src/graph/model.rs", None);
485+
// True siblings: both one level below `src`, at equal depth.
486+
let sibling = compute_confidence("src/graph/a.rs", "src/features/b.rs", None);
487+
assert!(same_dir > parent_child);
488+
assert!(parent_child > sibling);
489+
}
490+
491+
#[test]
492+
fn compute_confidence_scores_two_level_nesting_at_or_above_sibling_tier() {
493+
// the graph/algorithms/leiden/*.rs -> graph/model.rs shape from #1769.
494+
let conf = compute_confidence(
495+
"src/graph/algorithms/leiden/cpm.rs",
496+
"src/graph/model.rs",
497+
None,
498+
);
499+
assert!(conf >= 0.5, "expected >= 0.5, got {conf}");
500+
}
501+
502+
#[test]
503+
fn compute_confidence_still_scores_unrelated_deep_files_as_distant() {
504+
let conf = compute_confidence(
505+
"src/graph/algorithms/leiden/cpm.rs",
506+
"src/mcp/server.rs",
507+
None,
508+
);
509+
assert!(conf < 0.5, "expected < 0.5, got {conf}");
510+
}
511+
512+
#[test]
513+
fn directory_distance_same_dir_is_zero() {
514+
assert_eq!(directory_distance("src/graph", "src/graph"), 0);
515+
}
516+
517+
#[test]
518+
fn directory_distance_direct_parent_child_is_one() {
519+
assert_eq!(directory_distance("src/graph/algorithms", "src/graph"), 1);
520+
assert_eq!(directory_distance("src/graph", "src/graph/algorithms"), 1);
521+
}
522+
523+
#[test]
524+
fn directory_distance_siblings_is_two() {
525+
// Both dirs are one level below `src` — true siblings at equal depth.
526+
assert_eq!(directory_distance("src/graph", "src/features"), 2);
527+
}
528+
529+
#[test]
530+
fn directory_distance_unequal_depth_non_siblings_is_three() {
531+
// `algorithms` is nested inside `graph`, which is a sibling of `features` —
532+
// not a direct sibling pair despite sharing the `src` ancestor.
533+
assert_eq!(directory_distance("src/graph/algorithms", "src/features"), 3);
534+
}
433535
}

src/domain/graph/resolve.ts

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,41 @@ function resolveImportPathJS(
461461
return normalizePath(path.relative(rootDir, resolved));
462462
}
463463

464+
/** All ancestor directories of `dir`, starting with `dir` itself, walking up to the root. */
465+
function ancestorChain(dir: string): string[] {
466+
const chain = [dir];
467+
let cur = dir;
468+
for (;;) {
469+
const parent = path.dirname(cur);
470+
if (parent === cur) return chain; // reached root ('.', '/', or a drive root)
471+
chain.push(parent);
472+
cur = parent;
473+
}
474+
}
475+
476+
/**
477+
* Directory-tree distance between two directories: hops up from `a` to the
478+
* nearest ancestor shared with `b`, plus hops down from there to `b`.
479+
*
480+
* Symmetric and depth-independent — unlike a fixed-depth equality check
481+
* (e.g. comparing `dirname(dirname(a))` to `dirname(dirname(b))`, as this
482+
* function used to), this correctly scores both sibling directories (common
483+
* parent) and direct ancestor/descendant directories (one nested inside the
484+
* other) regardless of how deep either path is. The fixed-depth check only
485+
* matched when both files sat at the *same* depth, so e.g. a file in
486+
* `graph/algorithms/*.ts` calling a method declared in the shallower
487+
* `graph/model.ts` was scored as maximally distant (issue #1769).
488+
*/
489+
function directoryDistance(a: string, b: string): number {
490+
const chainA = ancestorChain(a);
491+
const chainB = ancestorChain(b);
492+
for (let i = 0; i < chainA.length; i++) {
493+
const j = chainB.indexOf(chainA[i]!);
494+
if (j !== -1) return i + j;
495+
}
496+
return Infinity;
497+
}
498+
464499
function computeConfidenceJS(
465500
callerFile: string,
466501
targetFile: string,
@@ -471,10 +506,10 @@ function computeConfidenceJS(
471506
if (importedFrom === targetFile) return 1.0;
472507
// Workspace-resolved imports get high confidence even across package boundaries
473508
if (importedFrom && _workspaceResolvedPaths.has(importedFrom)) return 0.95;
474-
if (path.dirname(callerFile) === path.dirname(targetFile)) return 0.7;
475-
const callerParent = path.dirname(path.dirname(callerFile));
476-
const targetParent = path.dirname(path.dirname(targetFile));
477-
if (callerParent === targetParent) return 0.5;
509+
const dist = directoryDistance(path.dirname(callerFile), path.dirname(targetFile));
510+
if (dist === 0) return 0.7; // same directory
511+
if (dist === 1) return 0.6; // direct parent/child directory
512+
if (dist === 2) return 0.5; // sibling directories, or a grandparent/grandchild pair
478513
return 0.3;
479514
}
480515

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/**
2+
* Regression test for #1769: a function parameter typed with a class name,
3+
* declared in a *different, less-deeply-nested directory*, must still
4+
* resolve method calls on that parameter back to the class's declaration.
5+
*
6+
* Root cause: `computeConfidenceJS` (and its Rust mirror `compute_confidence`)
7+
* scored directory proximity using a fixed-depth check — comparing the
8+
* parent of the caller's directory to the parent of the target's directory.
9+
* That check only matched when both files sat at the *same* depth, so a
10+
* subdirectory file (e.g. `graph/algorithms/bfs.ts`) calling a method on a
11+
* class declared in its direct parent directory (`graph/model.ts`) was
12+
* scored as maximally distant (0.3) — well below the 0.5 threshold used by
13+
* the call-edge resolver's typed-method lookup (`resolveViaTypedMethod` in
14+
* src/domain/graph/resolver/strategy.ts), so the call edge was silently
15+
* dropped even though the parameter's type annotation correctly resolved
16+
* `foo: Foo` to `typeMap['foo'] = 'Foo'`.
17+
*
18+
* Setup mirrors the real-world shape from src/graph/algorithms/bfs.ts calling
19+
* src/graph/model.ts:
20+
* - model.ts (parent directory): `export class Foo { bar(x): number {...} }`
21+
* - algorithms/consumer.ts (child directory): a function whose PARAMETER is
22+
* typed `foo: Foo` (via type annotation, not `new Foo()` construction)
23+
* calls `foo.bar(x)`.
24+
*
25+
* Expected: a `calls` edge from `useFoo` to `Foo.bar`, in both engines.
26+
*/
27+
28+
import fs from 'node:fs';
29+
import os from 'node:os';
30+
import path from 'node:path';
31+
import Database from 'better-sqlite3';
32+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
33+
import { buildGraph } from '../../src/domain/graph/builder.js';
34+
35+
const TSCONFIG = JSON.stringify({
36+
compilerOptions: {
37+
target: 'es2022',
38+
module: 'esnext',
39+
moduleResolution: 'bundler',
40+
strict: true,
41+
},
42+
include: ['**/*.ts'],
43+
});
44+
45+
const MODEL_TS = `
46+
export class Foo {
47+
bar(x: number): number {
48+
return x + 1;
49+
}
50+
}
51+
`;
52+
53+
// Nested one directory deeper than model.ts — mirrors graph/algorithms/bfs.ts
54+
// receiving a `graph: CodeGraph` parameter from graph/model.ts.
55+
const CONSUMER_TS = `
56+
import type { Foo } from '../model.js';
57+
58+
export function useFoo(foo: Foo, x: number): number {
59+
return foo.bar(x);
60+
}
61+
`;
62+
63+
let tmpWasm: string;
64+
let tmpNative: string;
65+
66+
function writeFixture(dir: string): void {
67+
fs.writeFileSync(path.join(dir, 'tsconfig.json'), TSCONFIG);
68+
// Both files live under a shared `graph/` directory — NOT at the fixture
69+
// root — so their relative-path depths mirror the real src/graph/model.ts
70+
// vs src/graph/algorithms/bfs.ts shape. Placing model.ts directly at the
71+
// fixture root would make `path.dirname()` hit its "." fixed point at the
72+
// same recursion depth for both files, which accidentally masks the #1769
73+
// bug (the old fixed-depth check happens to coincide at the filesystem
74+
// root regardless of the asymmetry it's supposed to detect).
75+
fs.mkdirSync(path.join(dir, 'graph', 'algorithms'), { recursive: true });
76+
fs.writeFileSync(path.join(dir, 'graph', 'model.ts'), MODEL_TS);
77+
fs.writeFileSync(path.join(dir, 'graph', 'algorithms', 'consumer.ts'), CONSUMER_TS);
78+
}
79+
80+
beforeAll(async () => {
81+
tmpWasm = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1769-wasm-'));
82+
writeFixture(tmpWasm);
83+
84+
tmpNative = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1769-native-'));
85+
writeFixture(tmpNative);
86+
87+
await Promise.all([
88+
buildGraph(tmpWasm, { incremental: false, skipRegistry: true, engine: 'wasm' }),
89+
buildGraph(tmpNative, { incremental: false, skipRegistry: true, engine: 'native' }),
90+
]);
91+
});
92+
93+
afterAll(() => {
94+
fs.rmSync(tmpWasm, { recursive: true, force: true });
95+
fs.rmSync(tmpNative, { recursive: true, force: true });
96+
});
97+
98+
function hasCallEdge(dbPath: string, sourceName: string, targetName: string): boolean {
99+
const db = new Database(dbPath, { readonly: true });
100+
try {
101+
const row = db
102+
.prepare(
103+
`SELECT 1
104+
FROM edges e
105+
JOIN nodes n1 ON e.source_id = n1.id
106+
JOIN nodes n2 ON e.target_id = n2.id
107+
WHERE e.kind = 'calls' AND n1.name = ? AND n2.name = ?`,
108+
)
109+
.get(sourceName, targetName);
110+
return row !== undefined;
111+
} finally {
112+
db.close();
113+
}
114+
}
115+
116+
describe('parameter-typed receiver call to a parent-directory class (#1769)', () => {
117+
it('WASM: resolves foo.bar(x) to Foo.bar across a child->parent directory boundary', () => {
118+
expect(hasCallEdge(path.join(tmpWasm, '.codegraph', 'graph.db'), 'useFoo', 'Foo.bar')).toBe(
119+
true,
120+
);
121+
});
122+
123+
it('Native: resolves foo.bar(x) to Foo.bar across a child->parent directory boundary', () => {
124+
expect(hasCallEdge(path.join(tmpNative, '.codegraph', 'graph.db'), 'useFoo', 'Foo.bar')).toBe(
125+
true,
126+
);
127+
});
128+
});

0 commit comments

Comments
 (0)