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
34 changes: 26 additions & 8 deletions crates/codegraph-core/src/domain/graph/builder/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,22 +177,24 @@ fn early_exit_result(

/// Save reverse-dep edges (and reverse-deps of removed files) before purging
/// changed files. Mirrors the JS save-then-purge sequence in `build-edges.ts`
/// (#1012). Returns `(saved_reverse_dep_edges, removal_reverse_deps)` so the
/// pipeline can reconnect them after Stage 5 and reclassify roles in Stage 8.
/// (#1012). Returns `(saved_reverse_dep_edges, removal_reverse_deps,
/// removed_file_neighbors)` so the pipeline can reconnect edges after Stage 5,
/// reclassify roles in Stage 8, and (#1839) fold a removed file's
/// cross-directory neighbors into Stage 8's directory-metrics refresh.
fn save_and_purge_changed(
conn: &Connection,
parse_changes: &[&detect_changes::ChangedFile],
change_result: &detect_changes::ChangeResult,
opts: &BuildOpts,
root_dir: &str,
) -> (Vec<detect_changes::SavedReverseDepEdge>, Vec<String>) {
) -> (Vec<detect_changes::SavedReverseDepEdge>, Vec<String>, Vec<String>) {
let mut saved_reverse_dep_edges: Vec<detect_changes::SavedReverseDepEdge> = Vec::new();
let mut removal_reverse_deps: Vec<String> = Vec::new();

if change_result.is_full_build {
let has_embeddings = detect_changes::has_embeddings(conn);
detect_changes::clear_all_graph_data(conn, has_embeddings);
return (saved_reverse_dep_edges, removal_reverse_deps);
return (saved_reverse_dep_edges, removal_reverse_deps, Vec::new());
}

let changed_paths: Vec<String> = parse_changes.iter().map(|c| c.rel_path.clone()).collect();
Expand All @@ -209,6 +211,12 @@ fn save_and_purge_changed(
}
}

// Capture removed files' cross-directory neighbor set BEFORE purging —
// both directions of their import edges are deleted below, so this is
// the last point they can still be discovered from live evidence (#1839).
let removed_file_neighbors =
detect_changes::capture_removed_file_neighbors(conn, &change_result.removed);

let files_to_purge: Vec<String> = change_result
.removed
.iter()
Expand All @@ -217,7 +225,7 @@ fn save_and_purge_changed(
.collect();
detect_changes::purge_changed_files(conn, &files_to_purge, &[]);

(saved_reverse_dep_edges, removal_reverse_deps)
(saved_reverse_dep_edges, removal_reverse_deps, removed_file_neighbors)
}

/// Parse a changed-file slice in parallel and key the results by relative path.
Expand Down Expand Up @@ -298,7 +306,10 @@ fn reconnect_saved_reverse_dep_edges(
/// `removed_files` is threaded through separately from `parse_changes_len`
/// (which only counts re-parsed files) so the fast path's directory-metrics
/// refresh also covers files deleted from a directory, not just files added
/// or modified within it (#1738).
/// or modified within it (#1738). `removed_file_neighbors` — the
/// cross-directory import neighbors of `removed_files`, captured before they
/// were purged — lets that refresh also reach a directory whose only link to
/// the touched set was an edge to/from one of those removed files (#1839).
fn run_structure_phase(
conn: &Connection,
file_symbols: &BTreeMap<String, FileSymbols>,
Expand All @@ -307,6 +318,7 @@ fn run_structure_phase(
line_count_map: &HashMap<String, i64>,
parse_changes_len: usize,
removed_files: &[String],
removed_file_neighbors: &[String],
is_full_build: bool,
) {
let changed_files: Vec<String> = file_symbols.keys().cloned().collect();
Expand All @@ -317,7 +329,12 @@ fn run_structure_phase(

if use_fast_path {
structure::update_changed_file_metrics(conn, &changed_files, line_count_map, file_symbols);
structure::refresh_affected_directory_metrics(conn, &changed_files, removed_files);
structure::refresh_affected_directory_metrics(
conn,
&changed_files,
removed_files,
removed_file_neighbors,
);
} else {
let changed_for_structure: Option<Vec<String>> = if is_full_build {
None
Expand Down Expand Up @@ -511,7 +528,7 @@ pub fn run_pipeline(
// Stage 3b: save reverse-dep edges (incremental) or clear all (full),
// then purge changed files. Returns the saved edges for Stage 7
// reconnect and the removal reverse-dep set for Stage 8 reclassification.
let (saved_reverse_dep_edges, removal_reverse_deps) =
let (saved_reverse_dep_edges, removal_reverse_deps, removed_file_neighbors) =
save_and_purge_changed(conn, &parse_changes, &change_result, &opts, root_dir);

// ── Stage 4: Parse files ───────────────────────────────────────────
Expand Down Expand Up @@ -645,6 +662,7 @@ pub fn run_pipeline(
&line_count_map,
parse_changes.len(),
&change_result.removed,
&removed_file_neighbors,
change_result.is_full_build,
);
timing.structure_ms = t0.elapsed().as_secs_f64() * 1000.0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,49 @@ pub fn find_reverse_dependencies(
reverse_deps
}

/// Captures the forward+reverse import-neighbor file set for files about to
/// be removed, BEFORE `purge_changed_files` deletes their edges.
///
/// `refresh_affected_directory_metrics` discovers cross-directory neighbors
/// by querying LIVE import edges from the affected directories — this works
/// for added/modified files (their edges are rebuilt and still present) but
/// not for removed files, whose edges in both directions are purged before
/// the structure stage runs. Reading them here, one step earlier in the
/// pipeline, closes that gap. Mirrors `captureRemovedFileNeighbors` in
/// `detect-changes.ts` (#1839).
pub fn capture_removed_file_neighbors(conn: &Connection, removed_files: &[String]) -> Vec<String> {
if removed_files.is_empty() {
return Vec::new();
}
let removed_set: HashSet<&str> = removed_files.iter().map(|s| s.as_str()).collect();
let mut neighbors: HashSet<String> = HashSet::new();

let mut stmt = match conn.prepare(
"SELECT n2.file AS other FROM edges e \
JOIN nodes n1 ON e.source_id = n1.id JOIN nodes n2 ON e.target_id = n2.id \
WHERE e.kind IN ('imports', 'imports-type') AND n1.file != n2.file AND n1.file = ?1 \
UNION \
SELECT n1.file AS other FROM edges e \
JOIN nodes n1 ON e.source_id = n1.id JOIN nodes n2 ON e.target_id = n2.id \
WHERE e.kind IN ('imports', 'imports-type') AND n1.file != n2.file AND n2.file = ?1",
) {
Ok(s) => s,
Err(_) => return Vec::new(),
};

for f in removed_files {
if let Ok(rows) = stmt.query_map([f], |row| row.get::<_, String>(0)) {
for row in rows.flatten() {
if !removed_set.contains(row.as_str()) {
neighbors.insert(row);
}
}
}
}

neighbors.into_iter().collect()
}

/// Purge graph data for changed/removed files and delete outgoing edges for reverse deps.
///
/// Deletion order: analysis dependents → edges → nodes (matches `connection::purge_files_data`).
Expand Down Expand Up @@ -1109,4 +1152,63 @@ mod tests {
.unwrap();
assert_eq!(new_target, target_new_id);
}

// ── capture_removed_file_neighbors (#1839) ──────────────────────────

#[test]
fn capture_removed_file_neighbors_finds_both_directions() {
let conn = test_conn();
// src/pkgA/a1.js imports src/pkgB/b1.js (forward); src/pkgC/c1.js
// imports src/pkgA/a1.js (reverse) — both directions must surface.
let a1 = insert_node(&conn, "a1", "function", "src/pkgA/a1.js", 1);
let b1 = insert_node(&conn, "b1", "function", "src/pkgB/b1.js", 1);
let c1 = insert_node(&conn, "c1", "function", "src/pkgC/c1.js", 1);
// Unrelated file — must not appear in the result.
insert_node(&conn, "d1", "function", "src/pkgD/d1.js", 1);

conn.execute(
"INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'imports', 1.0, 0)",
rusqlite::params![a1, b1],
)
.unwrap();
conn.execute(
"INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'imports', 1.0, 0)",
rusqlite::params![c1, a1],
)
.unwrap();

let mut neighbors =
capture_removed_file_neighbors(&conn, &["src/pkgA/a1.js".to_string()]);
neighbors.sort();
assert_eq!(
neighbors,
vec!["src/pkgB/b1.js".to_string(), "src/pkgC/c1.js".to_string()]
);
}

#[test]
fn capture_removed_file_neighbors_empty_for_no_removed_files() {
let conn = test_conn();
assert!(capture_removed_file_neighbors(&conn, &[]).is_empty());
}

#[test]
fn capture_removed_file_neighbors_excludes_other_removed_files() {
// Two files being removed together that import each other must not
// report each other as "neighbors" — only still-live files count.
let conn = test_conn();
let a1 = insert_node(&conn, "a1", "function", "src/pkgA/a1.js", 1);
let a2 = insert_node(&conn, "a2", "function", "src/pkgA/a2.js", 1);
conn.execute(
"INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'imports', 1.0, 0)",
rusqlite::params![a1, a2],
)
.unwrap();

let neighbors = capture_removed_file_neighbors(
&conn,
&["src/pkgA/a1.js".to_string(), "src/pkgA/a2.js".to_string()],
);
assert!(neighbors.is_empty());
}
}
17 changes: 11 additions & 6 deletions crates/codegraph-core/src/features/structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,19 +193,24 @@ fn find_neighbor_files(conn: &Connection, dir: &str) -> Vec<String> {
/// Removed files need no edge/node cleanup of their own — the purge step
/// already deleted their nodes and every edge referencing them (including
/// their old `contains` edge) earlier in the pipeline; only their ancestor
/// directories' aggregates need recomputing here. Note this expansion can't
/// reach a directory whose ONLY relationship to the touched set was an edge
/// to/from a file that was JUST removed — that edge is already gone by the
/// time this runs, so there's nothing left to discover it from (tracked
/// separately, see #1738 follow-up).
/// directories' aggregates need recomputing here. A removed file's own
/// cross-directory neighbors (files it imported, or that imported it) can no
/// longer be discovered from LIVE edges by the time this runs — those edges
/// are already purged — so the pipeline captures them up front, before the
/// purge, via `detect_changes::capture_removed_file_neighbors` and passes
/// them in as `removed_file_neighbors` (#1839).
pub fn refresh_affected_directory_metrics(
conn: &Connection,
changed_files: &[String],
removed_files: &[String],
removed_file_neighbors: &[String],
) {
let mut touched: Vec<String> = Vec::with_capacity(changed_files.len() + removed_files.len());
let mut touched: Vec<String> = Vec::with_capacity(
changed_files.len() + removed_files.len() + removed_file_neighbors.len(),
);
touched.extend_from_slice(changed_files);
touched.extend_from_slice(removed_files);
touched.extend_from_slice(removed_file_neighbors);
let mut affected_dirs = get_ancestor_dirs(&touched);
if affected_dirs.is_empty() {
return;
Expand Down
8 changes: 8 additions & 0 deletions src/domain/graph/builder/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ export class PipelineContext {
parseChanges!: ParseChange[];
metadataUpdates!: MetadataUpdate[];
removed!: string[];
/**
* Forward+reverse import-neighbor files of `removed`, captured before
* `purgeFilesFromGraph`/`purgeFilesData` deletes those files' edges. Lets
* `refreshAffectedDirectoryMetrics` still discover a removed file's
* cross-directory neighbor even though the live edge evidence for it is
* gone by the time the structure stage runs (#1839).
*/
removedFileNeighbors: string[] = [];
earlyExit: boolean = false;

// ── Parsing (set by parseFiles stage) ──────────────────────────────
Expand Down
20 changes: 13 additions & 7 deletions src/domain/graph/builder/stages/build-structure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ async function buildDirectoryStructure(
): Promise<void> {
if (useSmallIncrementalFastPath) {
updateChangedFileMetrics(ctx, changedFileList!);
refreshAffectedDirectoryMetrics(ctx, changedFileList!, ctx.removed ?? []);
refreshAffectedDirectoryMetrics(
ctx,
changedFileList!,
ctx.removed ?? [],
ctx.removedFileNeighbors ?? [],
);
return;
}

Expand Down Expand Up @@ -275,19 +280,20 @@ function updateChangedFileMetrics(ctx: PipelineContext, changedFiles: string[]):
* Removed files need no edge/node cleanup of their own — `purgeFilesData`
* already deleted their nodes and every edge referencing them (including
* their old `contains` edge) earlier in the pipeline; only their ancestor
* directories' aggregates need recomputing here. Note this expansion can't
* reach a directory whose ONLY relationship to the touched set was an edge
* to/from a file that was JUST removed — that edge is already gone by the
* time this runs, so there's nothing left to discover it from (tracked
* separately, see #1738 follow-up).
* directories' aggregates need recomputing here. A removed file's own
* cross-directory neighbors (files it imported, or that imported it) can no
* longer be discovered from LIVE edges by the time this runs — those edges
* are already purged — so `detectChanges` captures them up front, before the
* purge, and passes them in as `removedFileNeighbors` (#1839).
*/
function refreshAffectedDirectoryMetrics(
ctx: PipelineContext,
changedFiles: string[],
removedFiles: string[],
removedFileNeighbors: string[],
): void {
const { db } = ctx;
const affectedDirs = getAncestorDirs([...changedFiles, ...removedFiles]);
const affectedDirs = getAncestorDirs([...changedFiles, ...removedFiles, ...removedFileNeighbors]);
if (affectedDirs.size === 0) return;

const getDirId = db.prepare(
Expand Down
37 changes: 37 additions & 0 deletions src/domain/graph/builder/stages/detect-changes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,41 @@ function findReverseDependencies(
return reverseDeps;
}

/**
* Captures the forward+reverse import-neighbor file set for files about to be
* removed, BEFORE `purgeFilesFromGraph`/`purgeFilesData` deletes their edges.
*
* `refreshAffectedDirectoryMetrics` discovers cross-directory neighbors by
* querying LIVE import edges from the affected directories — this works for
* added/modified files (their edges are rebuilt and still present) but not
* for removed files, whose edges in both directions are purged before
* `buildStructure` runs. Reading them here, one step earlier in the pipeline,
* closes that gap: the neighbor files' ancestor directories are unioned into
* the affected-directory set so a directory whose only link to the touched
* set was an edge to/from a now-removed file still gets its fan-in/fan-out
* recomputed (#1839).
*/
function captureRemovedFileNeighbors(db: BetterSqlite3Database, removedFiles: string[]): string[] {
if (removedFiles.length === 0) return [];
const removedSet = new Set(removedFiles);
const neighbors = new Set<string>();
const neighborStmt = db.prepare(`
SELECT n2.file AS other FROM edges e
JOIN nodes n1 ON e.source_id = n1.id JOIN nodes n2 ON e.target_id = n2.id
WHERE e.kind IN ('imports', 'imports-type') AND n1.file != n2.file AND n1.file = ?
UNION
SELECT n1.file AS other FROM edges e
JOIN nodes n1 ON e.source_id = n1.id JOIN nodes n2 ON e.target_id = n2.id
WHERE e.kind IN ('imports', 'imports-type') AND n1.file != n2.file AND n2.file = ?
`);
for (const relPath of removedFiles) {
for (const row of neighborStmt.all(relPath, relPath) as Array<{ other: string }>) {
if (!removedSet.has(row.other)) neighbors.add(row.other);
}
}
return [...neighbors];
}

/**
* Computes each node's 1-based ordinal rank (by ascending line) among nodes
* sharing its (name, kind) within `file`, plus the sibling-group size, keyed
Expand Down Expand Up @@ -579,6 +614,7 @@ function handleScopedBuild(ctx: PipelineContext): void {
const changedRelPaths = new Set<string>([...changePaths, ...ctx.removed]);

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.

P1 Scoped Removals Lose Neighbors

When a scoped incremental rebuild deletes a file, this call captures neighbors from ctx.removed, but the scoped path does not go through the normal branch that populates that list from change detection. removedFileNeighbors stays empty before the purge, so cross-directory metrics can remain stale for the deleted file's import neighbors in scoped builds.

Fix in Claude Code

reverseDeps = findReverseDependencies(db, changedRelPaths, rootDir, ctx.nativeDb);
}
ctx.removedFileNeighbors = captureRemovedFileNeighbors(db, ctx.removed);
purgeAndAddReverseDeps(ctx, changePaths, reverseDeps);
info(
`Scoped rebuild: ${changePaths.length} changed, ${ctx.removed.length} removed, ${reverseDeps.size} reverse-deps`,
Expand Down Expand Up @@ -621,6 +657,7 @@ function handleIncrementalBuild(ctx: PipelineContext): void {
const changePaths = ctx.parseChanges.map(
(item) => item.relPath || normalizePath(path.relative(rootDir, item.file)),
);
ctx.removedFileNeighbors = captureRemovedFileNeighbors(db, ctx.removed);
purgeAndAddReverseDeps(ctx, changePaths, reverseDeps);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@
* changed file gaining/losing an import into a sibling package shifts that
* package's fan-in/fan-out too, even though none of its own files changed)
* — cheap because it's bounded by (changed files x path depth) rather than
* the size of the repo. See #1839 for a narrower residual gap this does not
* cover: a directory whose only link to the touched set was an edge to/from
* a file that was itself just removed (that edge's evidence is gone by the
* time the refresh runs).
* the size of the repo. See #1839 (fixed separately, in
* `issue-1839-directory-fanin-fanout-stale-removal.test.ts`) for the related
* case of a directory whose only link to the touched set was an edge to/from
* a file that was itself just removed.
*
* Strategy: build a fixture with >20 files (crossing the fast-path's
* `existingFileCount > 20` gate), mutate it incrementally, then diff the
Expand Down
Loading
Loading