Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
42 changes: 21 additions & 21 deletions crates/codegraph-core/src/edge_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,28 +503,28 @@ fn resolve_call_targets<'a>(
.unwrap_or_default();
if !exact.is_empty() { return exact; }

// For this/self/super: prefer class-scoped exact lookup (e.g. `this.area()` in
// `Shape.describe` → try `Shape.area` first). This avoids false edges to unrelated
// classes that happen to have a method with the same name in the same file.
// Fall back to the broader same-file suffix scan only when the class-scoped lookup
// finds nothing (e.g. when the caller is at module scope or the name is unknown).
if call.receiver.is_some() {
// Extract the class prefix from the enclosing caller name (e.g. "Shape" from "Shape.describe").
if let Some(dot_pos) = caller_name.find('.') {
let class_prefix = &caller_name[..dot_pos];
let qualified = format!("{}.{}", class_prefix, call.name);
let class_scoped: Vec<&NodeInfo> = ctx.nodes_by_name
.get(qualified.as_str())
.map(|v| v.iter().filter(|n| n.kind == "method").copied().collect())
.unwrap_or_default();
if !class_scoped.is_empty() { return class_scoped; }
}
// Class-scoped exact lookup: prefer `ClassName.method` when the caller is a qualified
// method (e.g. `this.area()` or plain `area()` in `Shape.describe` → try `Shape.area`).
// Covers both this/self/super dispatch AND no-receiver static sibling calls (e.g.
// `IsValidEmail()` inside `Validators.ValidateUser` → `Validators.IsValidEmail`).
// This avoids false edges to unrelated classes that happen to have a method with the
// same name in the same file.
if let Some(dot_pos) = caller_name.find('.') {
let class_prefix = &caller_name[..dot_pos];
let qualified = format!("{}.{}", class_prefix, call.name);
let class_scoped: Vec<&NodeInfo> = ctx.nodes_by_name
.get(qualified.as_str())
.map(|v| v.iter().filter(|n| n.kind == "method").copied().collect())
.unwrap_or_default();
if !class_scoped.is_empty() { return class_scoped; }
Comment on lines +515 to +522

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 Class-scoped Rust lookup skips confidence filtering

The WASM counterpart in call-resolver.ts (line 192) applies computeConfidence >= 0.5 when building sameClass, but the Rust path here only filters by n.kind == "method". If two separate packages in the same graph both define a ClassName.MethodName method (e.g. same-named utility classes in different sub-projects), the Rust resolver will return all matches with equal weight, while WASM would keep only the ones that pass the confidence threshold. This pre-existed for receiver-based calls but now also fires for no-receiver static siblings, slightly widening the exposure.

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed — added import_resolution::compute_confidence(rel_path, &n.file, None) >= 0.5 to the class-scoped Rust lookup, aligning it with the WASM counterpart in call-resolver.ts. Committed in d7cc1cb.

}

// Broader fallback: same-file suffix scan. Always restrict to the caller's
// own class prefix — regardless of how many matches are found — to avoid
// false-positive edges to unrelated classes in the same file.
// (e.g. this.area() inside Shape.describe must never yield Calculator.area,
// even when Calculator.area is the only method with that name in the file.)
// Broader fallback: same-file suffix scan. Only for this/self/super (not no-receiver
// plain calls) to avoid false positives on global function calls inside class methods.
// Always restricts to the caller's own class prefix to avoid false edges to unrelated
// classes in the same file (e.g. this.area() inside Shape.describe must never yield
// Calculator.area, even when Calculator.area is the only method with that name).
if call.receiver.is_some() {
let suffix = format!(".{}", call.name);
if let Some(file_nodes) = ctx.nodes_by_file.get(rel_path) {
let same_file_methods: Vec<&NodeInfo> = file_nodes.iter()
Expand Down
6 changes: 4 additions & 2 deletions src/domain/graph/builder/call-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,12 @@ export function resolveByMethodOrGlobal(
.filter((t) => computeConfidence(relPath, t.file, null) >= 0.5);
if (exact.length > 0) return exact;

// For this/self/super receiver: try same-class method lookup via callerName.
// Try same-class method lookup via callerName.
// e.g. `this.area()` inside `Shape.describe` → try `Shape.area`.
// Also covers no-receiver calls inside class methods, e.g. `IsValidEmail(x)` inside
// `Validators.ValidateUser` → try `Validators.IsValidEmail` (C#/Java static siblings).
// This seeds the initial edge that runChaPostPass later expands to subclass overrides.
if (call.receiver && callerName) {
if (callerName) {
const dotIdx = callerName.lastIndexOf('.');
if (dotIdx > -1) {
const callerClass = callerName.slice(0, dotIdx);
Expand Down
56 changes: 18 additions & 38 deletions src/domain/graph/builder/stages/native-orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1545,48 +1545,28 @@ export async function tryNativeOrchestrator(
}

// Phase 8.5: expand CHA call edges (interface dispatch → concrete implementations).
// `runPostNativeCha` returns the target node IDs of newly inserted edges so we
// can re-classify roles for the implementation files. The Rust orchestrator ran
// role classification BEFORE this post-pass, so without a re-run the newly-called
// implementor methods stay classified as `dead-ffi` (no incoming edges at Rust time).
// The Rust orchestrator ran role classification BEFORE this post-pass, so without
// a re-run the newly-called implementor methods stay classified as `dead-ffi`.
//
// CHA also changes the global fan-out distribution (callee files gain fan_in, and
// new edges shift the median). A full re-classification is required — not just the
// callee files — because the median shift can change roles in unrelated files whose
// fan-out sits near the old median. (Example: a method that called two siblings
// pre-CHA might be near the median, but post-CHA the median is higher, changing
// its role from utility → core.) Using an incremental pass with a stale median
// cache would produce incorrect roles outside the CHA-affected file set.
const chaTargetIds = runPostNativeCha(ctx.db as unknown as BetterSqlite3Database);
if (chaTargetIds.size > 0) {

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 runPostNativeCha returns a full Set<number> but after this refactor only .size > 0 is ever read — the individual IDs are no longer needed. Building the Set is a minor allocation that now serves no purpose. Checking a returned count or boolean would make the intent clearer and avoid the unused allocation.

Suggested change
const chaTargetIds = runPostNativeCha(ctx.db as unknown as BetterSqlite3Database);
if (chaTargetIds.size > 0) {
const chaEdgeCount = runPostNativeCha(ctx.db as unknown as BetterSqlite3Database).size;
if (chaEdgeCount > 0) {

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed — changed runPostNativeCha return type from Set<number> to number, replacing newTargetIds set with a newEdgeCount counter. The call site now reads the count directly as chaEdgeCount. Committed in d7cc1cb.

try {
const db = ctx.db as unknown as BetterSqlite3Database;
const idArray = Array.from(chaTargetIds);
const CHUNK_SIZE = 500;
const seenFiles = new Set<string>();
const affectedFiles: Array<{ file: string }> = [];
for (let i = 0; i < idArray.length; i += CHUNK_SIZE) {
const chunk = idArray.slice(i, i + CHUNK_SIZE);
const placeholders = chunk.map(() => '?').join(',');
const rows = db
.prepare(
`SELECT DISTINCT file FROM nodes WHERE id IN (${placeholders}) AND file IS NOT NULL`,
)
.all(...chunk) as Array<{ file: string }>;
for (const row of rows) {
if (!seenFiles.has(row.file)) {
seenFiles.add(row.file);
affectedFiles.push(row);
}
}
}
if (affectedFiles.length > 0) {
const { classifyNodeRoles } = (await import('../../../../features/structure.js')) as {
classifyNodeRoles: (
db: BetterSqlite3Database,
changedFiles?: string[] | null,
) => Record<string, number>;
};
classifyNodeRoles(
db,
affectedFiles.map((r) => r.file),
);
debug(
`CHA post-pass: re-classified roles for ${affectedFiles.length} implementation file(s)`,
);
}
const { classifyNodeRoles } = (await import('../../../../features/structure.js')) as {
classifyNodeRoles: (
db: BetterSqlite3Database,
changedFiles?: string[] | null,
) => Record<string, number>;
};
classifyNodeRoles(db);
debug(`CHA post-pass: full role re-classification after ${chaTargetIds.size} new CHA edges`);
} catch (err) {
debug(`CHA post-pass role re-classification failed: ${toErrorMessage(err)}`);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/benchmarks/resolution/resolution-benchmark.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ const THRESHOLDS: Record<string, { precision: number; recall: number }> = {
python: { precision: 0.7, recall: 0.3 },
go: { precision: 0.7, recall: 0.3 },
java: { precision: 0.7, recall: 0.3 },
csharp: { precision: 0.5, recall: 0.2 },
csharp: { precision: 1.0, recall: 0.8 },
kotlin: { precision: 0.6, recall: 0.2 },
// Lower bars — resolution still maturing
rust: { precision: 0.6, recall: 0.2 },
Expand Down
5 changes: 4 additions & 1 deletion tests/integration/prototype-method-resolution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ beforeAll(async () => {
for (const [rel, content] of Object.entries(FIXTURE)) {
fs.writeFileSync(path.join(tmpDir, rel), content);
}
await buildGraph(tmpDir, { incremental: false, skipRegistry: true });
// TODO(#1381): pinned to WASM because the published native binary predates the
// prototype-method fixes landed in #1331. Remove the pin (or add a dual-engine
// variant) once the native binary ships the corresponding Rust-side extraction.
await buildGraph(tmpDir, { incremental: false, skipRegistry: true, engine: 'wasm' });
});

afterAll(() => {
Expand Down
Loading