diff --git a/crates/codegraph-core/src/features/structure.rs b/crates/codegraph-core/src/features/structure.rs index f2fbe5d6..2d034fa5 100644 --- a/crates/codegraph-core/src/features/structure.rs +++ b/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 { - 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 { 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 { /// 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 = 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); } diff --git a/crates/codegraph-core/src/graph/classifiers/roles.rs b/crates/codegraph-core/src/graph/classifiers/roles.rs index 1b45f03b..87908f71 100644 --- a/crates/codegraph-core/src/graph/classifiers/roles.rs +++ b/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> { + 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 { + // 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::>() .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::>().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)?); + } } } diff --git a/src/domain/graph/builder/stages/build-structure.ts b/src/domain/graph/builder/stages/build-structure.ts index dcf8de6a..7fdbac7d 100644 --- a/src/domain/graph/builder/stages/build-structure.ts +++ b/src/domain/graph/builder/stages/build-structure.ts @@ -269,17 +269,21 @@ function updateChangedFileMetrics(ctx: PipelineContext, changedFiles: 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 — the neighbor query 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, mirrors the same fix in structure.rs). 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 — the neighbor query 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, mirrors the same fix in structure.rs). + * 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, + * mirrors the same fix in structure.rs). 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 — `purgeFilesData` * already deleted their nodes and every edge referencing them (including @@ -302,33 +306,44 @@ function refreshAffectedDirectoryMetrics( const getDirId = db.prepare( "SELECT id FROM nodes WHERE name = ? AND kind = 'directory' AND file = ? AND line = 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. + // 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. + // + // Scoped to the exact touched file, not its containing directory. An + // earlier version scoped this to the whole leaf directory (`file >= x/ + // AND file < x0`), which also pulled in edges belonging to unrelated + // sibling files that happen to live alongside the touched 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 the + // touched 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 exists for (#1738) + // while only considering edges that could plausibly have changed as a + // result of editing that file (251 -> 46 neighbour files / 29 -> 13 + // affected dirs for the same probe). const neighborFiles = 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 >= @lo AND n1.file < @hi - AND NOT (n2.file >= @lo AND n2.file < @hi) + WHERE e.kind IN ('imports', 'imports-type') AND n1.file = @file AND n2.file != @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 >= @lo AND n2.file < @hi - AND NOT (n1.file >= @lo AND n1.file < @hi) + WHERE e.kind IN ('imports', 'imports-type') AND n2.file = @file AND n1.file != @file `); - // Seed neighbor-discovery from each touched file's own leaf directory - // only — NOT every entry in `affectedDirs` (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. - const leafDirs = new Set(); - for (const f of [...changedFiles, ...removedFiles]) { - const d = normalizePath(path.dirname(f)); - if (d && d !== '.') leafDirs.add(d); - } - for (const dir of leafDirs) { - const otherFiles = neighborFiles.all({ lo: `${dir}/`, hi: `${dir}0` }) as Array<{ + // Seed neighbor-discovery from each touched file individually — NOT its + // containing directory, and NOT every entry in `affectedDirs` (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 (const file of [...changedFiles, ...removedFiles]) { + const otherFiles = neighborFiles.all({ file }) as Array<{ other: string; }>; for (const ancestor of getAncestorDirs(otherFiles.map((r) => r.other))) { diff --git a/src/features/structure.ts b/src/features/structure.ts index 885b1102..015bd6c3 100644 --- a/src/features/structure.ts +++ b/src/features/structure.ts @@ -1,8 +1,17 @@ import path from 'node:path'; import { getBuildMeta, getNodeId, setBuildMeta, testFilterSQL } from '../db/index.js'; +import { cachedStmt } from '../db/repository/cached-stmt.js'; import { debug } from '../infrastructure/logger.js'; import { getAncestorDirs, normalizePath } from '../shared/constants.js'; -import type { BetterSqlite3Database } from '../types.js'; +import type { BetterSqlite3Database, StmtCache } from '../types.js'; + +// isBarrelProdReachable's two queries are identical text on every call (the +// interpolated test-file filter is a fixed set of LIKE patterns keyed only +// by column name) — cache them per-db so repeated calls (1-3 barrels per +// incremental build) skip re-preparing the same statement. Mirrors +// `prepare_cached` in the Rust `is_barrel_prod_reachable`. +const _barrelDirectStmt: StmtCache<{ reachable: number }> = new WeakMap(); +const _barrelBackwardStmt: StmtCache<{ reachable: number }> = new WeakMap(); // ─── Build-time helpers ─────────────────────────────────────────────── @@ -913,6 +922,84 @@ function classifyNodeRolesFull(db: BetterSqlite3Database, emptySummary: RoleSumm return summary; } +/** + * Direct barrels: files that directly re-export (one hop) into any of the + * given target files. Used by `classifyNodeRolesIncremental`'s scoped + * alternative to the full `prod_reachable` recursive CTE — see + * `isBarrelProdReachable`. Mirrors Rust `find_direct_reexport_barrels`. + */ +function findDirectReexportBarrels( + db: BetterSqlite3Database, + allAffectedFiles: string[], +): string[] { + const placeholders = allAffectedFiles.map(() => '?').join(','); + const rows = db + .prepare( + `SELECT DISTINCT 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 = 'reexports' AND n2.file IN (${placeholders})`, + ) + .all(...allAffectedFiles) as { file: string }[]; + return rows.map((r) => r.file); +} + +/** + * True when `barrelFile` is production-reachable — i.e. a member of the same + * `prod_reachable` set `classifyNodeRolesFull`'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. Mirrors Rust + * `is_barrel_prod_reachable` — see that function's doc comment for the + * measured cost difference (#1855) and the reachability argument. + */ +function isBarrelProdReachable(db: BetterSqlite3Database, barrelFile: string): boolean { + // Fast path: barrelFile itself is directly imported by a non-test file + // (the base case of the original `prod_reachable` definition). + const direct = cachedStmt( + _barrelDirectStmt, + db, + `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 = ? + ${testFilterSQL('src.file')} + ) AS reachable`, + ).get(barrelFile) as { reachable: number }; + if (direct.reachable) return true; + + // Slow path: walk `reexports` edges backward from barrelFile (who + // re-exports INTO barrelFile, transitively) looking for an ancestor + // that's directly imported by production. `UNION` (not `UNION ALL`) + // dedupes on `file`, which both bounds the search to barrelFile's own + // chain and guarantees termination if the chain contains a cycle. + const backward = cachedStmt( + _barrelBackwardStmt, + db, + `WITH RECURSIVE ancestors(file) AS ( + SELECT ? AS file + 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' + ${testFilterSQL('src2.file')} + ) AS reachable`, + ).get(barrelFile) as { reachable: number }; + return !!backward.reachable; +} + /** * Incremental role classification: only reclassify nodes from changed files * plus their immediate edge neighbours (callers and callees in other files). @@ -932,18 +1019,39 @@ function classifyNodeRolesIncremental( // Expand affected set: include files containing nodes that are edge neighbours // of changed-file nodes. This ensures that removing a call from file A to a // node in file B causes B's roles to be recalculated (fan_in changed). + // + // 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 better-sqlite3/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). const seedPlaceholders = changedFiles.map(() => '?').join(','); const neighbourFiles = db .prepare( - `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 (${seedPlaceholders}) - AND n2.file NOT IN (${seedPlaceholders}) - AND n2.kind NOT IN ('file', 'directory')`, + `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 (${seedPlaceholders}) AND n2.file NOT IN (${seedPlaceholders}) + 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 (${seedPlaceholders}) AND n1.file NOT IN (${seedPlaceholders}) + AND n1.kind NOT IN ('file', 'directory')`, ) - .all(...changedFiles, ...changedFiles) as { file: string }[]; + .all(...changedFiles, ...changedFiles, ...changedFiles, ...changedFiles) as { file: string }[]; const allAffectedFiles = [...changedFiles, ...neighbourFiles.map((r) => r.file)]; const placeholders = allAffectedFiles.map(() => '?').join(','); @@ -1004,35 +1112,36 @@ function classifyNodeRolesIncremental( ); // 3b. Mark symbols as exported when their files are targets of reexports edges - // from production-reachable barrels (traces through multi-level chains) (#837) - const reexportExported = db - .prepare( - `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' - ${testFilterSQL('src.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) + // from production-reachable barrels (traces through multi-level chains) (#837). + // + // classifyNodeRolesFull'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 + // `allAffectedFiles` (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 `allAffectedFiles` (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 `isBarrelProdReachable`). + const reexportBarrels = findDirectReexportBarrels(db, allAffectedFiles); + const reachableBarrels = reexportBarrels.filter((b) => isBarrelProdReachable(db, b)); + if (reachableBarrels.length > 0) { + const barrelPlaceholders = reachableBarrels.map(() => '?').join(','); + const reexportExported = db + .prepare( + `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 (${barrelPlaceholders}) + AND n.kind NOT IN ('file', 'directory', 'parameter', 'property') + AND n.file IN (${placeholders})`, ) - AND n.kind NOT IN ('file', 'directory', 'parameter', 'property') - AND n.file IN (${placeholders})`, - ) - .all(...allAffectedFiles) as { id: number }[]; - for (const r of reexportExported) exportedIds.add(r.id); + .all(...reachableBarrels, ...allAffectedFiles) as { id: number }[]; + for (const r of reexportExported) exportedIds.add(r.id); + } // 3c. Mark symbols with exported=1 as exported — the extractor sets this flag when the // author writes `export interface Foo { }` / `export type Bar = ...` / `export function`.