Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 120 additions & 18 deletions crates/codegraph-core/src/domain/graph/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> {
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
}
Comment on lines +273 to +282

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Same O(n²) pattern as the TypeScript twin: chain_b.iter().position(...) does a linear scan for each element of chain_a. Collecting chain_b into a HashMap<&str, usize> first would bring this to O(n). At typical path depths the cost is negligible, but the Rust engine also runs this on the rayon-parallelised hot path.

Suggested change
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
}
fn directory_distance(a: &str, b: &str) -> usize {
let chain_a = ancestor_chain(a);
let chain_b = ancestor_chain(b);
let index_in_b: std::collections::HashMap<&str, usize> =
chain_b.iter().enumerate().map(|(j, d)| (d.as_str(), j)).collect();
for (i, dir_a) in chain_a.iter().enumerate() {
if let Some(&j) = index_in_b.get(dir_a.as_str()) {
return i + j;
}
}
usize::MAX
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code


/// 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,
Expand All @@ -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).
Expand Down Expand Up @@ -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);
}
}
43 changes: 39 additions & 4 deletions src/domain/graph/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Comment on lines +489 to +497

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 directoryDistance scans chainB linearly (indexOf) for every element of chainA, giving O(|chainA| × |chainB|) string comparisons. A single pre-built Map<string, number> from directory to its index in chainB would reduce this to O(|chainA| + |chainB|). At typical repository path depths (< 20) the difference is invisible, but this function is on the hot path for every call-edge resolution candidate.

Suggested change
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 directoryDistance(a: string, b: string): number {
const chainA = ancestorChain(a);
const chainB = ancestorChain(b);
const indexInB = new Map<string, number>(chainB.map((d, idx) => [d, idx]));
for (let i = 0; i < chainA.length; i++) {
const j = indexInB.get(chainA[i]!);
if (j !== undefined) return i + j;
}
return Infinity;
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code


function computeConfidenceJS(
callerFile: string,
targetFile: string,
Expand All @@ -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;
}

Expand Down
128 changes: 128 additions & 0 deletions tests/integration/issue-1769-parent-dir-receiver-typed-call.test.ts
Original file line number Diff line number Diff line change
@@ -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,
);
});
});
Loading
Loading