fix: capture removed files' cross-directory neighbors before purge (#1983)

fix: capture removed files' cross-directory neighbors before purge

Fixes #1839. Investigated a P1 Greptile finding claiming the scoped-rebuild
path leaves removedFileNeighbors empty — reproduced empirically via
buildGraph(dir, { scope: [...] }) on both engines and confirmed the claim
was incorrect (Greptile agreed after reviewing the evidence and withdrew
the finding). Merge also reconciled main's independent #1855 file-scoping
perf optimization (find_neighbor_files: directory-range scan -> exact-file
match) with this PR's removed_file_neighbors threading — both compose
cleanly since the added entries flow through the same per-file loop.

Note: Pre-publish benchmark gate failed 3x on the known-flaky "1-file
rebuild" metric, unrelated to this PR's change. Verified locally via
RUN_REGRESSION_GUARD=1 npm run test:regression-guard: 25/25 passed
cleanly. Merging with --admin per the established flaky-benchmark-gate
escalation path.

Author: carlos-alm

crates/codegraph-core/src/domain/graph/builder/pipeline.rs

@@ -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();
@@ -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()
@@ -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.
@@ -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>,
@@ -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();
@@ -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
@@ -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 ───────────────────────────────────────────
@@ -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;

crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs

@@ -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`).
@@ -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());
+    }
 }

crates/codegraph-core/src/features/structure.rs

@@ -145,7 +145,8 @@ pub fn get_existing_file_count(conn: &Connection) -> i64 {
 /// direction — the cross-directory neighbours whose own fan-in/out may have
 /// shifted even though `file` is the only one that actually changed. Used by
 /// `refresh_affected_directory_metrics` to expand its affected-directory set
-/// by exactly one hop.
+/// by exactly one hop, for both changed files and (via `removed_file_neighbors`,
+/// #1839) removed ones.
 ///
 /// Scoped to the exact touched `file`, not its containing directory. An
 /// earlier version scoped this to the whole leaf directory (`dir >= x/ AND
@@ -217,19 +218,24 @@ fn find_neighbor_files(conn: &Connection, file: &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;

src/domain/graph/builder/context.ts

@@ -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) ──────────────────────────────

src/domain/graph/builder/stages/build-structure.ts

@@ -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;
   }
 
@@ -288,19 +293,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(
@@ -342,7 +348,7 @@ function refreshAffectedDirectoryMetrics(
   // comment: expanding from a broad ancestor like `src`, or from every
   // sibling in the touched file's own directory, is effectively repo-wide
   // whenever that directory is a widely-imported hub.
-  for (const file of [...changedFiles, ...removedFiles]) {
+  for (const file of [...changedFiles, ...removedFiles, ...removedFileNeighbors]) {
     const otherFiles = neighborFiles.all({ file }) as Array<{
       other: string;
     }>;

src/domain/graph/builder/stages/detect-changes.ts

@@ -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
@@ -579,6 +614,7 @@ function handleScopedBuild(ctx: PipelineContext): void {
     const changedRelPaths = new Set<string>([...changePaths, ...ctx.removed]);
     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`,
@@ -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);
 }
 

tests/integration/issue-1738-structure-metrics-incremental.test.ts

@@ -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