diff --git a/crates/codegraph-core/src/domain/graph/resolve.rs b/crates/codegraph-core/src/domain/graph/resolve.rs index 87ed47ff..36d3ec8e 100644 --- a/crates/codegraph-core/src/domain/graph/resolve.rs +++ b/crates/codegraph-core/src/domain/graph/resolve.rs @@ -247,8 +247,42 @@ fn resolve_import_path_inner( relativize_to_root(&resolved.display().to_string().replace('\\', "/"), root_dir) } +/// All ancestor directories of `dir`, starting with `dir` itself, walking up to the root. +fn ancestor_chain(dir: &str) -> Vec { + let mut chain = vec![dir.to_string()]; + let mut cur = dir.to_string(); + while let Some(parent) = Path::new(&cur).parent() { + let parent_str = parent.display().to_string(); + chain.push(parent_str.clone()); + cur = parent_str; + } + chain +} + +/// Directory-tree distance between two directories: hops up from `a` to the +/// nearest ancestor shared with `b`, plus hops down from there to `b`. +/// +/// Symmetric and depth-independent — unlike a fixed-depth equality check +/// (e.g. comparing the parent-of-parent of `a` to the parent-of-parent of +/// `b`, as `compute_confidence` used to), this correctly scores both sibling +/// directories (common parent) and direct ancestor/descendant directories +/// (one nested inside the other) regardless of how deep either path is. The +/// fixed-depth check only matched when both files sat at the *same* depth, +/// so e.g. a file in `graph/algorithms/*.rs` calling a method declared in +/// the shallower `graph/model.rs` was scored as maximally distant (issue #1769). +fn directory_distance(a: &str, b: &str) -> usize { + let chain_a = ancestor_chain(a); + let chain_b = ancestor_chain(b); + for (i, dir_a) in chain_a.iter().enumerate() { + if let Some(j) = chain_b.iter().position(|dir_b| dir_b == dir_a) { + return i + j; + } + } + usize::MAX +} + /// Compute proximity-based confidence for call resolution. -/// Mirrors `computeConfidence()` in builder.js. +/// Mirrors `computeConfidence()` in resolve.ts. pub fn compute_confidence( caller_file: &str, target_file: &str, @@ -275,24 +309,12 @@ pub fn compute_confidence( .map(|p| p.display().to_string()) .unwrap_or_default(); - if caller_dir == target_dir { - return 0.7; - } - - let caller_parent = Path::new(&caller_dir) - .parent() - .map(|p| p.display().to_string()) - .unwrap_or_default(); - let target_parent = Path::new(&target_dir) - .parent() - .map(|p| p.display().to_string()) - .unwrap_or_default(); - - if caller_parent == target_parent { - return 0.5; + match directory_distance(&caller_dir, &target_dir) { + 0 => 0.7, // same directory + 1 => 0.6, // direct parent/child directory + 2 => 0.5, // sibling directories, or a grandparent/grandchild pair + _ => 0.3, } - - 0.3 } /// Batch resolve multiple imports (parallelized with rayon). @@ -430,4 +452,84 @@ mod tests { ); assert_eq!(result, "src/utils.ts"); } + + // Regression tests for #1769: a fixed-depth "grandparent equality" check + // used to compare the parent of `caller_dir` to the parent of `target_dir`, + // which only matched when both files sat at the *same* depth. A file in a + // subdirectory calling a method declared in its direct parent directory + // (e.g. `graph/algorithms/bfs.rs` calling `graph/model.rs`) was scored as + // maximally distant (0.3) purely because the two files were nested at + // different depths — well below the 0.5 threshold used by the call-edge + // resolver's typed-method lookup, silently dropping the call edge. + + #[test] + fn compute_confidence_scores_parent_child_dirs_above_resolver_threshold() { + let conf = compute_confidence("src/graph/algorithms/bfs.rs", "src/graph/model.rs", None); + assert!(conf >= 0.5, "expected >= 0.5, got {conf}"); + } + + #[test] + fn compute_confidence_is_symmetric_for_parent_child_dirs() { + let caller_deeper = + compute_confidence("src/graph/algorithms/bfs.rs", "src/graph/model.rs", None); + let target_deeper = + compute_confidence("src/graph/model.rs", "src/graph/algorithms/bfs.rs", None); + assert_eq!(caller_deeper, target_deeper); + } + + #[test] + fn compute_confidence_ranks_parent_child_between_same_dir_and_sibling() { + let same_dir = compute_confidence("src/graph/a.rs", "src/graph/b.rs", None); + let parent_child = + compute_confidence("src/graph/algorithms/bfs.rs", "src/graph/model.rs", None); + // True siblings: both one level below `src`, at equal depth. + let sibling = compute_confidence("src/graph/a.rs", "src/features/b.rs", None); + assert!(same_dir > parent_child); + assert!(parent_child > sibling); + } + + #[test] + fn compute_confidence_scores_two_level_nesting_at_or_above_sibling_tier() { + // the graph/algorithms/leiden/*.rs -> graph/model.rs shape from #1769. + let conf = compute_confidence( + "src/graph/algorithms/leiden/cpm.rs", + "src/graph/model.rs", + None, + ); + assert!(conf >= 0.5, "expected >= 0.5, got {conf}"); + } + + #[test] + fn compute_confidence_still_scores_unrelated_deep_files_as_distant() { + let conf = compute_confidence( + "src/graph/algorithms/leiden/cpm.rs", + "src/mcp/server.rs", + None, + ); + assert!(conf < 0.5, "expected < 0.5, got {conf}"); + } + + #[test] + fn directory_distance_same_dir_is_zero() { + assert_eq!(directory_distance("src/graph", "src/graph"), 0); + } + + #[test] + fn directory_distance_direct_parent_child_is_one() { + assert_eq!(directory_distance("src/graph/algorithms", "src/graph"), 1); + assert_eq!(directory_distance("src/graph", "src/graph/algorithms"), 1); + } + + #[test] + fn directory_distance_siblings_is_two() { + // Both dirs are one level below `src` — true siblings at equal depth. + assert_eq!(directory_distance("src/graph", "src/features"), 2); + } + + #[test] + fn directory_distance_unequal_depth_non_siblings_is_three() { + // `algorithms` is nested inside `graph`, which is a sibling of `features` — + // not a direct sibling pair despite sharing the `src` ancestor. + assert_eq!(directory_distance("src/graph/algorithms", "src/features"), 3); + } } diff --git a/src/domain/graph/resolve.ts b/src/domain/graph/resolve.ts index 3d5268ca..3dd05fe3 100644 --- a/src/domain/graph/resolve.ts +++ b/src/domain/graph/resolve.ts @@ -461,6 +461,41 @@ function resolveImportPathJS( return normalizePath(path.relative(rootDir, resolved)); } +/** All ancestor directories of `dir`, starting with `dir` itself, walking up to the root. */ +function ancestorChain(dir: string): string[] { + const chain = [dir]; + let cur = dir; + for (;;) { + const parent = path.dirname(cur); + if (parent === cur) return chain; // reached root ('.', '/', or a drive root) + chain.push(parent); + cur = parent; + } +} + +/** + * Directory-tree distance between two directories: hops up from `a` to the + * nearest ancestor shared with `b`, plus hops down from there to `b`. + * + * Symmetric and depth-independent — unlike a fixed-depth equality check + * (e.g. comparing `dirname(dirname(a))` to `dirname(dirname(b))`, as this + * function used to), this correctly scores both sibling directories (common + * parent) and direct ancestor/descendant directories (one nested inside the + * other) regardless of how deep either path is. The fixed-depth check only + * matched when both files sat at the *same* depth, so e.g. a file in + * `graph/algorithms/*.ts` calling a method declared in the shallower + * `graph/model.ts` was scored as maximally distant (issue #1769). + */ +function directoryDistance(a: string, b: string): number { + const chainA = ancestorChain(a); + const chainB = ancestorChain(b); + for (let i = 0; i < chainA.length; i++) { + const j = chainB.indexOf(chainA[i]!); + if (j !== -1) return i + j; + } + return Infinity; +} + function computeConfidenceJS( callerFile: string, targetFile: string, @@ -471,10 +506,10 @@ function computeConfidenceJS( if (importedFrom === targetFile) return 1.0; // Workspace-resolved imports get high confidence even across package boundaries if (importedFrom && _workspaceResolvedPaths.has(importedFrom)) return 0.95; - if (path.dirname(callerFile) === path.dirname(targetFile)) return 0.7; - const callerParent = path.dirname(path.dirname(callerFile)); - const targetParent = path.dirname(path.dirname(targetFile)); - if (callerParent === targetParent) return 0.5; + const dist = directoryDistance(path.dirname(callerFile), path.dirname(targetFile)); + if (dist === 0) return 0.7; // same directory + if (dist === 1) return 0.6; // direct parent/child directory + if (dist === 2) return 0.5; // sibling directories, or a grandparent/grandchild pair return 0.3; } diff --git a/tests/integration/issue-1769-parent-dir-receiver-typed-call.test.ts b/tests/integration/issue-1769-parent-dir-receiver-typed-call.test.ts new file mode 100644 index 00000000..ef7d03e3 --- /dev/null +++ b/tests/integration/issue-1769-parent-dir-receiver-typed-call.test.ts @@ -0,0 +1,128 @@ +/** + * Regression test for #1769: a function parameter typed with a class name, + * declared in a *different, less-deeply-nested directory*, must still + * resolve method calls on that parameter back to the class's declaration. + * + * Root cause: `computeConfidenceJS` (and its Rust mirror `compute_confidence`) + * scored directory proximity using a fixed-depth check — comparing the + * parent of the caller's directory to the parent of the target's directory. + * That check only matched when both files sat at the *same* depth, so a + * subdirectory file (e.g. `graph/algorithms/bfs.ts`) calling a method on a + * class declared in its direct parent directory (`graph/model.ts`) was + * scored as maximally distant (0.3) — well below the 0.5 threshold used by + * the call-edge resolver's typed-method lookup (`resolveViaTypedMethod` in + * src/domain/graph/resolver/strategy.ts), so the call edge was silently + * dropped even though the parameter's type annotation correctly resolved + * `foo: Foo` to `typeMap['foo'] = 'Foo'`. + * + * Setup mirrors the real-world shape from src/graph/algorithms/bfs.ts calling + * src/graph/model.ts: + * - model.ts (parent directory): `export class Foo { bar(x): number {...} }` + * - algorithms/consumer.ts (child directory): a function whose PARAMETER is + * typed `foo: Foo` (via type annotation, not `new Foo()` construction) + * calls `foo.bar(x)`. + * + * Expected: a `calls` edge from `useFoo` to `Foo.bar`, in both engines. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; + +const TSCONFIG = JSON.stringify({ + compilerOptions: { + target: 'es2022', + module: 'esnext', + moduleResolution: 'bundler', + strict: true, + }, + include: ['**/*.ts'], +}); + +const MODEL_TS = ` +export class Foo { + bar(x: number): number { + return x + 1; + } +} +`; + +// Nested one directory deeper than model.ts — mirrors graph/algorithms/bfs.ts +// receiving a `graph: CodeGraph` parameter from graph/model.ts. +const CONSUMER_TS = ` +import type { Foo } from '../model.js'; + +export function useFoo(foo: Foo, x: number): number { + return foo.bar(x); +} +`; + +let tmpWasm: string; +let tmpNative: string; + +function writeFixture(dir: string): void { + fs.writeFileSync(path.join(dir, 'tsconfig.json'), TSCONFIG); + // Both files live under a shared `graph/` directory — NOT at the fixture + // root — so their relative-path depths mirror the real src/graph/model.ts + // vs src/graph/algorithms/bfs.ts shape. Placing model.ts directly at the + // fixture root would make `path.dirname()` hit its "." fixed point at the + // same recursion depth for both files, which accidentally masks the #1769 + // bug (the old fixed-depth check happens to coincide at the filesystem + // root regardless of the asymmetry it's supposed to detect). + fs.mkdirSync(path.join(dir, 'graph', 'algorithms'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'graph', 'model.ts'), MODEL_TS); + fs.writeFileSync(path.join(dir, 'graph', 'algorithms', 'consumer.ts'), CONSUMER_TS); +} + +beforeAll(async () => { + tmpWasm = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1769-wasm-')); + writeFixture(tmpWasm); + + tmpNative = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1769-native-')); + writeFixture(tmpNative); + + await Promise.all([ + buildGraph(tmpWasm, { incremental: false, skipRegistry: true, engine: 'wasm' }), + buildGraph(tmpNative, { incremental: false, skipRegistry: true, engine: 'native' }), + ]); +}); + +afterAll(() => { + fs.rmSync(tmpWasm, { recursive: true, force: true }); + fs.rmSync(tmpNative, { recursive: true, force: true }); +}); + +function hasCallEdge(dbPath: string, sourceName: string, targetName: string): boolean { + const db = new Database(dbPath, { readonly: true }); + try { + const row = db + .prepare( + `SELECT 1 + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'calls' AND n1.name = ? AND n2.name = ?`, + ) + .get(sourceName, targetName); + return row !== undefined; + } finally { + db.close(); + } +} + +describe('parameter-typed receiver call to a parent-directory class (#1769)', () => { + it('WASM: resolves foo.bar(x) to Foo.bar across a child->parent directory boundary', () => { + expect(hasCallEdge(path.join(tmpWasm, '.codegraph', 'graph.db'), 'useFoo', 'Foo.bar')).toBe( + true, + ); + }); + + it('Native: resolves foo.bar(x) to Foo.bar across a child->parent directory boundary', () => { + expect(hasCallEdge(path.join(tmpNative, '.codegraph', 'graph.db'), 'useFoo', 'Foo.bar')).toBe( + true, + ); + }); +}); diff --git a/tests/unit/resolve.test.ts b/tests/unit/resolve.test.ts index 340d1c94..7ad38e4d 100644 --- a/tests/unit/resolve.test.ts +++ b/tests/unit/resolve.test.ts @@ -184,6 +184,72 @@ describe('computeConfidenceJS', () => { expect(sameDir).toBeGreaterThan(siblingParent); expect(siblingParent).toBeGreaterThan(distant); }); + + // Regression tests for #1769: a fixed-depth "grandparent equality" check used + // to compare `dirname(dirname(callerFile))` to `dirname(dirname(targetFile))`, + // which only matched when both files sat at the *same* depth. A file in a + // subdirectory calling a method declared in its direct parent directory + // (e.g. `graph/algorithms/bfs.ts` calling `graph/model.ts`) was scored as + // maximally distant (0.3) purely because the two files were nested at + // different depths — well below the 0.5 threshold used by the call-edge + // resolver's typed-method lookup, silently dropping the call edge. + describe('directory-nesting distance (#1769)', () => { + it('scores a direct parent/child directory pair above the 0.5 resolver threshold', () => { + // caller one level deeper than target — the exact bfs.ts -> model.ts shape. + const conf = computeConfidenceJS( + 'src/graph/algorithms/bfs.ts', + 'src/graph/model.ts', + undefined, + ); + expect(conf).toBeGreaterThanOrEqual(0.5); + }); + + it('is symmetric: target one level deeper than caller scores the same as the reverse', () => { + const callerDeeper = computeConfidenceJS( + 'src/graph/algorithms/bfs.ts', + 'src/graph/model.ts', + undefined, + ); + const targetDeeper = computeConfidenceJS( + 'src/graph/model.ts', + 'src/graph/algorithms/bfs.ts', + undefined, + ); + expect(targetDeeper).toBe(callerDeeper); + }); + + it('ranks direct parent/child nesting strictly between same-directory and sibling-directory', () => { + const sameDir = computeConfidenceJS('src/graph/a.ts', 'src/graph/b.ts', undefined); + const parentChild = computeConfidenceJS( + 'src/graph/algorithms/bfs.ts', + 'src/graph/model.ts', + undefined, + ); + // True siblings: both one level below `src`, at equal depth. + const sibling = computeConfidenceJS('src/graph/a.ts', 'src/features/b.ts', undefined); + expect(sameDir).toBeGreaterThan(parentChild); + expect(parentChild).toBeGreaterThan(sibling); + }); + + it('scores a two-level-deep subdirectory calling into its grandparent at or above the sibling tier', () => { + // the graph/algorithms/leiden/*.ts -> graph/model.ts shape from #1769. + const conf = computeConfidenceJS( + 'src/graph/algorithms/leiden/cpm.ts', + 'src/graph/model.ts', + undefined, + ); + expect(conf).toBeGreaterThanOrEqual(0.5); + }); + + it('still scores unrelated deeply-nested files as distant', () => { + const conf = computeConfidenceJS( + 'src/graph/algorithms/leiden/cpm.ts', + 'src/mcp/server.ts', + undefined, + ); + expect(conf).toBeLessThan(0.5); + }); + }); }); // ─── computeConfidence (public API, dispatches to native or JS) ─────