fix: scope incremental structure/roles neighbor expansion to the changed file

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

@@ -141,31 +141,42 @@ pub fn get_existing_file_count(conn: &Connection) -> i64 {
     .unwrap_or(0)
 }
 
-/// Directories connected to `dir` via a live import/imports-type edge in
-/// either direction — the cross-directory neighbours whose own fan-in/out
-/// may have shifted even though none of their files changed. Used by
+/// Files connected to `file` via a live import/imports-type edge in either
+/// 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.
-fn find_neighbor_files(conn: &Connection, dir: &str) -> Vec<String> {
-    let lo = format!("{dir}/");
-    let hi = format!("{dir}0");
+///
+/// Scoped to the exact touched `file`, not its containing directory. An
+/// earlier version scoped this to the whole leaf directory (`dir >= x/ AND
+/// dir < x0`), which also pulled in edges belonging to unrelated sibling
+/// files that happen to live alongside `file` — harmless when the directory
+/// is small, but when the touched file sits in a widely-imported "hub"
+/// directory (e.g. `src/domain`, imported from dozens of unrelated
+/// directories via sibling files), that range scan discovers hundreds of
+/// neighbour files that have nothing to do with `file`'s own edges, which
+/// then balloon the affected-directory set to include broad, expensive-to-
+/// recompute ancestors like the repo-root `src` (measured: 251 neighbour
+/// files / 29 affected dirs for a single-file change to this repo's own
+/// `src/domain/queries.ts`, a ~55ms hit on the "1-file rebuild" benchmark —
+/// #1855). Scoping to the exact file preserves the cross-directory
+/// detection this function exists for (#1738) while only considering edges
+/// that could plausibly have changed as a result of editing `file` itself
+/// (251 -> 46 neighbour files / 29 -> 13 affected dirs for the same probe).
+fn find_neighbor_files(conn: &Connection, file: &str) -> Vec<String> {
     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 AND n1.file < ?2 \
-             AND NOT (n2.file >= ?1 AND n2.file < ?2) \
+           WHERE e.kind IN ('imports', 'imports-type') AND n1.file = ?1 AND n2.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 AND n2.file < ?2 \
-             AND NOT (n1.file >= ?1 AND n1.file < ?2)",
+           WHERE e.kind IN ('imports', 'imports-type') AND n2.file = ?1 AND n1.file != ?1",
     ) {
         Ok(s) => s,
         Err(_) => return Vec::new(),
     };
-    let result = match stmt.query_map(rusqlite::params![lo, hi], |row| row.get::<_, String>(0)) {
+    let result = match stmt.query_map(rusqlite::params![file], |row| row.get::<_, String>(0)) {
         Ok(rows) => rows.flatten().collect(),
         Err(_) => Vec::new(),
     };
@@ -188,17 +199,20 @@ fn find_neighbor_files(conn: &Connection, dir: &str) -> Vec<String> {
 /// loses) an import into a sibling package shifts that package's
 /// fan-in/fan-out/cohesion even though none of its own files were touched.
 ///
-/// The neighbor-discovery step is seeded only from each touched file's
-/// leaf directory, not from every ancestor up to root — `find_neighbor_files`
-/// is bounded by the number of import edges crossing ONE directory's
-/// boundary, which for a broad, near-root "container" directory (e.g. `src`,
-/// which transitively contains most of the repo) is effectively the size of
-/// the whole codebase, not "path depth". Seeding from every ancestor turned
-/// a single touched file into 50+ affected directories on this repo's own
-/// `src/` tree — a measured 70-90ms hit on the "1-file rebuild" benchmark
-/// (#1738 follow-up). Ancestor rollup (ancestors' own aggregates still get
-/// recomputed) is unaffected; only the expensive cross-directory neighbor
-/// lookup is scoped down.
+/// The neighbor-discovery step is seeded from each touched file itself, not
+/// from every ancestor up to root, and not from the touched file's whole
+/// containing directory either — `find_neighbor_files` is now bounded by the
+/// import edges attached to that ONE file. Seeding from every ancestor
+/// turned a single touched file into 50+ affected directories on this
+/// repo's own `src/` tree — a measured 70-90ms hit on the "1-file rebuild"
+/// benchmark (#1738 follow-up). Seeding from the touched file's *directory*
+/// (an intermediate fix) was still broad enough to pull in unrelated
+/// sibling files' edges whenever that directory was itself a widely-
+/// imported hub (e.g. `src/domain`) — measured 251 neighbour files / 29
+/// affected dirs (~55ms) for a single-file change there, cut to 46 / 13 by
+/// scoping to the exact file (#1855). Ancestor rollup (ancestors' own
+/// aggregates still get recomputed) is unaffected; only the expensive
+/// cross-directory neighbor lookup is scoped down.
 ///
 /// Removed files need no edge/node cleanup of their own — the purge step
 /// already deleted their nodes and every edge referencing them (including
@@ -221,13 +235,14 @@ pub fn refresh_affected_directory_metrics(
         return;
     }
 
-    // Seed neighbor-discovery from each touched file's own leaf directory
-    // only — NOT every entry in `affected_dirs` (which includes the full
-    // ancestor chain up to root). See the function doc comment: expanding
-    // from a broad ancestor like `src` is effectively repo-wide.
-    let leaf_dirs: HashSet<String> = touched.iter().filter_map(|f| parent_dir(f)).collect();
-    for dir in &leaf_dirs {
-        let neighbor_files = find_neighbor_files(conn, dir);
+    // Seed neighbor-discovery from each touched file individually — NOT its
+    // containing directory, and NOT every entry in `affected_dirs` (which
+    // includes the full ancestor chain up to root). See the function doc
+    // 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 file in &touched {
+        let neighbor_files = find_neighbor_files(conn, file);
         for ancestor in get_ancestor_dirs(&neighbor_files) {
             affected_dirs.insert(ancestor);
         }

crates/codegraph-core/src/graph/classifiers/roles.rs

@@ -661,7 +661,133 @@ fn classify_rows(
     }
 }
 
+/// Direct barrels: files that directly re-export (one hop) into any of the
+/// given target files. Used by `do_classify_incremental`'s scoped
+/// alternative to the full `prod_reachable` recursive CTE — see
+/// `is_barrel_prod_reachable`.
+fn find_direct_reexport_barrels(
+    tx: &rusqlite::Transaction,
+    affected_ph: &str,
+    all_affected: &[&str],
+) -> rusqlite::Result<Vec<String>> {
+    let sql = format!(
+        "SELECT DISTINCT n1.file FROM edges e
+           JOIN nodes n1 ON e.source_id = n1.id
+           JOIN nodes n2 ON e.target_id = n2.id
+         WHERE e.kind = 'reexports' AND n2.file IN ({affected_ph})"
+    );
+    let mut stmt = tx.prepare(&sql)?;
+    for (i, f) in all_affected.iter().enumerate() {
+        stmt.raw_bind_parameter(i + 1, *f)?;
+    }
+    let mut rows = stmt.raw_query();
+    let mut result = Vec::new();
+    while let Some(row) = rows.next()? {
+        result.push(row.get::<_, String>(0)?);
+    }
+    Ok(result)
+}
+
+/// True when `barrel_file` is production-reachable — i.e. a member of the
+/// same `prod_reachable` set `do_classify_full`'s recursive CTE computes
+/// globally (a non-test file directly imports it, or reaches it transitively
+/// through a chain of `reexports` edges) — but answered for one specific
+/// file instead of materializing the whole graph's closure.
+///
+/// `do_classify_incremental`'s old approach ran that same global CTE (seeded
+/// from every non-test file's imports — 442 rows on this repo's own
+/// codebase, closing over 703 total) on every incremental role
+/// classification, regardless of how few files actually changed. Once
+/// `find_neighbour_files` legitimately widens the affected set for a
+/// barrel/re-export file (as it must, correctly, since #1849 — see that
+/// function's doc comment), this pre-existing global recursion became a
+/// major cost: ~18-21ms measured on this repo (#1855), because SQLite's
+/// query planner stops being able to short-circuit the CTE once enough
+/// affected-file candidates are in play.
+///
+/// Reachability is symmetric either direction: "is B reachable from any
+/// production import" is answered equivalently by walking *backward* from B
+/// through `reexports` edges (who re-exports into B, transitively) and
+/// checking whether *any* of those backward-ancestors (including B itself)
+/// is directly imported by production — bounded by B's own chain depth
+/// (typically 1-3 hops) rather than the whole graph. Measured ~1.6ms total
+/// (find barrels + per-barrel check + final symbol lookup) for the same
+/// probe that cost ~18-21ms via the global closure.
+fn is_barrel_prod_reachable(
+    tx: &rusqlite::Transaction,
+    barrel_file: &str,
+) -> rusqlite::Result<bool> {
+    // Fast path: barrel_file itself is directly imported by a non-test file
+    // (the base case of the original `prod_reachable` definition).
+    let direct_sql = format!(
+        "SELECT EXISTS(
+           SELECT 1 FROM edges e
+             JOIN nodes src ON e.source_id = src.id
+             JOIN nodes tgt ON e.target_id = tgt.id
+           WHERE e.kind IN ('imports', 'dynamic-imports', 'imports-type')
+             AND src.kind = 'file' AND tgt.kind = 'file' AND tgt.file = ?1
+             {}
+         )",
+        test_file_filter_col("src.file")
+    );
+    // prepare_cached: `direct_sql` is the same text on every call (the
+    // interpolated test-file filter is a fixed set of LIKE patterns keyed
+    // only by column name), so the statement cache turns every call after
+    // the first into a lookup instead of a fresh parse/plan — worthwhile
+    // even at the small 1-3-barrels-per-build cardinality this runs at.
+    let direct: bool = tx
+        .prepare_cached(&direct_sql)?
+        .query_row(rusqlite::params![barrel_file], |row| row.get(0))?;
+    if direct {
+        return Ok(true);
+    }
+
+    // Slow path: walk `reexports` edges backward from barrel_file (who
+    // re-exports INTO barrel_file, transitively) looking for an ancestor
+    // that's directly imported by production. `UNION` (not `UNION ALL`)
+    // dedupes on `file`, which both bounds the search to barrel_file's own
+    // chain and guarantees termination if the chain contains a cycle.
+    let backward_sql = format!(
+        "WITH RECURSIVE ancestors(file) AS (
+           SELECT ?1
+           UNION
+           SELECT DISTINCT n1.file FROM edges e
+             JOIN nodes n1 ON e.source_id = n1.id
+             JOIN nodes n2 ON e.target_id = n2.id
+             JOIN ancestors a ON n2.file = a.file
+           WHERE e.kind = 'reexports'
+         )
+         SELECT EXISTS(
+           SELECT 1 FROM ancestors a
+             JOIN nodes af ON af.file = a.file AND af.kind = 'file'
+             JOIN edges e2 ON e2.target_id = af.id
+             JOIN nodes src2 ON e2.source_id = src2.id
+           WHERE e2.kind IN ('imports', 'dynamic-imports', 'imports-type')
+             AND src2.kind = 'file'
+             {}
+         )",
+        test_file_filter_col("src2.file")
+    );
+    tx.prepare_cached(&backward_sql)?
+        .query_row(rusqlite::params![barrel_file], |row| row.get(0))
+}
+
 /// Find neighbouring files connected by call/imports-type/reexports edges to the changed files.
+///
+/// Written as a UNION of two directional joins rather than a single
+/// OR-based self-join (`JOIN nodes n1 ON (e.source_id = n1.id OR
+/// e.target_id = n1.id)`, likewise for `n2`). The OR-join can't be served
+/// by an index on either side of the OR, so SQLite falls back to scanning
+/// every edge matching `e.kind IN (...)` and probing both endpoints per
+/// row. Each directional half below is a plain equi-join on `source_id`/
+/// `target_id`, letting the planner drive the scan from
+/// `idx_edges_kind_source`/`idx_edges_kind_target` directly. `UNION` (not
+/// `UNION ALL`) dedupes the combined result the same way the original
+/// query's `DISTINCT` did. Same result set, measured ~2.4x faster
+/// (#1855 — this query's cost, compounded by the symbol-level
+/// `reexports` edges added in #1849, was a major contributor to the
+/// "1-file rebuild" benchmark regression when the changed file is itself a
+/// barrel/re-export hub).
 fn find_neighbour_files(
     tx: &rusqlite::Transaction,
     changed_files: &[String],
@@ -672,24 +798,29 @@ fn find_neighbour_files(
         .collect::<Vec<_>>()
         .join(",");
     let sql = format!(
-        "SELECT DISTINCT n2.file FROM edges e
-         JOIN nodes n1 ON (e.source_id = n1.id OR e.target_id = n1.id)
-         JOIN nodes n2 ON (e.source_id = n2.id OR e.target_id = n2.id)
-         WHERE e.kind IN ('calls', 'imports-type', 'reexports')
-           AND n1.file IN ({})
-           AND n2.file NOT IN ({})
-           AND n2.kind NOT IN ('file', 'directory')",
-        seed_ph, seed_ph
+        "SELECT n2.file AS file 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 ('calls', 'imports-type', 'reexports')
+             AND n1.file IN ({seed_ph}) AND n2.file NOT IN ({seed_ph})
+             AND n2.kind NOT IN ('file', 'directory')
+         UNION
+         SELECT n1.file AS file 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 ('calls', 'imports-type', 'reexports')
+             AND n2.file IN ({seed_ph}) AND n1.file NOT IN ({seed_ph})
+             AND n1.kind NOT IN ('file', 'directory')"
     );
     let mut stmt = tx.prepare(&sql)?;
     let mut idx = 1;
-    for f in changed_files {
-        stmt.raw_bind_parameter(idx, f.as_str())?;
-        idx += 1;
-    }
-    for f in changed_files {
-        stmt.raw_bind_parameter(idx, f.as_str())?;
-        idx += 1;
+    // Bound 4 times: (n1 IN, n2 NOT IN) for the first half, then
+    // (n2 IN, n1 NOT IN) for the second half.
+    for _ in 0..4 {
+        for f in changed_files {
+            stmt.raw_bind_parameter(idx, f.as_str())?;
+            idx += 1;
+        }
     }
     let mut rows = stmt.raw_query();
     let mut result = Vec::new();
@@ -797,43 +928,53 @@ pub(crate) fn do_classify_incremental(
 
     // Mark symbols as exported when their files are targets of reexport edges
     // from production-reachable barrels (traces through multi-level chains) (#837).
-    // Same recursive CTE logic as the full-classify path (step 3b), but scoped
-    // to affected files only via the additional `AND n.file IN (...)` filter.
+    //
+    // do_classify_full's step 3b answers this by materializing the global
+    // `prod_reachable` closure (every file reachable from any production
+    // import, extended through reexports chains) because it has no smaller
+    // scope to exploit — it's already processing every file. Here, only
+    // `all_affected` (the changed files plus their one-hop neighbours) can
+    // possibly be reexport targets we care about, so instead: find the
+    // small set of barrels that directly reexport into `all_affected`
+    // (typically 1-3 files), then answer "is this specific barrel
+    // production-reachable" with a backward-scoped check instead of the
+    // whole-graph forward closure (see `is_barrel_prod_reachable`).
     {
-        let reexport_sql = format!(
-            "WITH RECURSIVE prod_reachable(file_id) AS (
-                SELECT DISTINCT e.target_id
-                FROM edges e
-                JOIN nodes src ON e.source_id = src.id
-                WHERE e.kind IN ('imports', 'dynamic-imports', 'imports-type')
-                  AND src.kind = 'file'
-                  {}
-                UNION
-                SELECT e.target_id
-                FROM edges e
-                JOIN prod_reachable pr ON e.source_id = pr.file_id
-                WHERE e.kind = 'reexports'
-              )
-              SELECT DISTINCT n.id
-              FROM nodes n
-              JOIN nodes f ON f.file = n.file AND f.kind = 'file'
-              WHERE f.id IN (
-                SELECT e.target_id FROM edges e
-                WHERE e.kind = 'reexports'
-                  AND e.source_id IN (SELECT file_id FROM prod_reachable)
-              )
-              AND n.kind NOT IN ('file', 'directory', 'parameter', 'property')
-              AND n.file IN ({})",
-            test_file_filter_col("src.file"),
-            affected_ph
-        );
-        let mut stmt = tx.prepare(&reexport_sql)?;
-        for (i, f) in all_affected.iter().enumerate() {
-            stmt.raw_bind_parameter(i + 1, *f)?;
+        let barrels = find_direct_reexport_barrels(&tx, &affected_ph, &all_affected)?;
+        let mut reachable_barrels: Vec<&str> = Vec::with_capacity(barrels.len());
+        for b in &barrels {
+            if is_barrel_prod_reachable(&tx, b)? {
+                reachable_barrels.push(b.as_str());
+            }
         }
-        let mut rrows = stmt.raw_query();
-        while let Some(row) = rrows.next()? {
-            exported_ids.insert(row.get::<_, i64>(0)?);
+
+        if !reachable_barrels.is_empty() {
+            let barrel_ph: String =
+                reachable_barrels.iter().map(|_| "?").collect::<Vec<_>>().join(",");
+            let sql = format!(
+                "SELECT DISTINCT n.id
+                 FROM nodes n
+                 JOIN nodes f ON f.file = n.file AND f.kind = 'file'
+                 JOIN edges e ON e.target_id = f.id
+                 JOIN nodes b ON e.source_id = b.id
+                 WHERE e.kind = 'reexports' AND b.file IN ({barrel_ph})
+                   AND n.kind NOT IN ('file', 'directory', 'parameter', 'property')
+                   AND n.file IN ({affected_ph})"
+            );
+            let mut stmt = tx.prepare(&sql)?;
+            let mut idx = 1;
+            for b in &reachable_barrels {
+                stmt.raw_bind_parameter(idx, *b)?;
+                idx += 1;
+            }
+            for f in &all_affected {
+                stmt.raw_bind_parameter(idx, *f)?;
+                idx += 1;
+            }
+            let mut rrows = stmt.raw_query();
+            while let Some(row) = rrows.next()? {
+                exported_ids.insert(row.get::<_, i64>(0)?);
+            }
         }
     }