diff --git a/crates/codegraph-core/src/db/connection.rs b/crates/codegraph-core/src/db/connection.rs index aca94537b..a897b81be 100644 --- a/crates/codegraph-core/src/db/connection.rs +++ b/crates/codegraph-core/src/db/connection.rs @@ -337,6 +337,25 @@ const MIGRATIONS: &[Migration] = &[ // dataflow_vertices and dataflow_summary on the next `codegraph build`. up: "SELECT 1", }, + // NOTE: TS migration v20 (edges.dynamic_kind) has no Rust counterpart yet — see #2066. + // v21 below is independent of it (a standalone new table), so this gap does not block it. + Migration { + version: 21, + up: r#" + CREATE TABLE IF NOT EXISTS deleted_export_advisories ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + file TEXT NOT NULL, + name TEXT NOT NULL, + kind TEXT NOT NULL, + line INTEGER NOT NULL, + consumer_name TEXT NOT NULL, + consumer_file TEXT NOT NULL, + consumer_line INTEGER NOT NULL, + deleted_at INTEGER NOT NULL + ); + CREATE INDEX IF NOT EXISTS idx_deleted_export_advisories_file ON deleted_export_advisories(file); + "#, + }, ]; // ── napi types ────────────────────────────────────────────────────────── diff --git a/crates/codegraph-core/src/domain/graph/builder/pipeline.rs b/crates/codegraph-core/src/domain/graph/builder/pipeline.rs index 6af4975bc..2205bca8a 100644 --- a/crates/codegraph-core/src/domain/graph/builder/pipeline.rs +++ b/crates/codegraph-core/src/domain/graph/builder/pipeline.rs @@ -253,6 +253,13 @@ fn save_and_purge_changed( let removed_file_neighbors = detect_changes::capture_removed_file_neighbors(conn, &change_result.removed); + // A file about to be (re)inserted can no longer be "deleted" — clear any + // stale advisory left over from a prior removal at this same path before + // capturing this build's actual removals, and before purging deletes the + // live evidence `record_deleted_export_advisories` reads (#1938). + detect_changes::clear_deleted_export_advisories(conn, &changed_paths); + detect_changes::record_deleted_export_advisories(conn, &change_result.removed); + let files_to_purge: Vec = change_result .removed .iter() diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs b/crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs index 324a39c12..99f8f4a5f 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/detect_changes.rs @@ -814,6 +814,138 @@ pub fn capture_removed_file_neighbors(conn: &Connection, removed_files: &[String neighbors.into_iter().collect() } +/// True if `table` can be queried without error. Used for +/// `deleted_export_advisories` (added in migration v21), which an older, +/// not-yet-migrated schema may not have. +fn table_queryable(conn: &Connection, table: &str) -> bool { + // `query_row` returns `Err(QueryReturnedNoRows)` for a query that is + // syntactically/semantically valid but matches zero rows — e.g. a table + // that exists but is still empty — which is NOT the same as the table + // being absent (a real `SqliteFailure`/"no such table" error). Unlike + // `has_embeddings` (which genuinely wants "are there any rows" and can + // conflate the two), an existence probe must treat "exists but empty" + // as queryable, or every advisory capture on a freshly-migrated, + // still-empty table would silently no-op. + match conn.query_row(&format!("SELECT 1 FROM {table} LIMIT 1"), [], |_| Ok(())) { + Ok(_) | Err(rusqlite::Error::QueryReturnedNoRows) => true, + Err(_) => false, + } +} + +/// Current time in epoch milliseconds — mirrors `Date.now()` in the TS +/// `detectChanges` pipeline, used to timestamp deleted-export advisory rows. +fn now_ms() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as i64) + .unwrap_or(0) +} + +/// Snapshots, for each deleted export that still has an external consumer, +/// one row per consumer into `deleted_export_advisories` — captured BEFORE +/// `purge_changed_files` deletes the removed file's `nodes`/`edges` rows, so +/// `codegraph check` can still see the violation regardless of whether some +/// other, separate `codegraph build` invocation already purged the live +/// rows by the time `check` runs. Mirrors `recordDeletedExportAdvisories` in +/// `db/repository/deleted-export-advisories.ts` (#1938). +/// +/// Replaces any pre-existing advisory rows for the same files first — the +/// `file_hashes` row for a removed file is never purged on the incremental +/// path (see `purge_changed_files`'s caller), so a subsequent build keeps +/// re-detecting the same file as "removed" and would otherwise re-run this +/// capture every time; querying live `nodes` for an already-purged file +/// naturally returns nothing on those later runs, so this is a no-op after +/// the first capture in practice — but replacing first keeps this correct +/// even if that changes. +pub fn record_deleted_export_advisories(conn: &Connection, removed_files: &[String]) { + if removed_files.is_empty() || !table_queryable(conn, "deleted_export_advisories") { + return; + } + + let tx = match conn.unchecked_transaction() { + Ok(t) => t, + Err(_) => return, + }; + + for file in removed_files { + let _ = tx.execute("DELETE FROM deleted_export_advisories WHERE file = ?1", [file]); + } + + let defs_result = tx.prepare( + "SELECT id, name, kind, line FROM nodes \ + WHERE file = ?1 AND kind IN ('function', 'method', 'class') AND exported = 1 \ + ORDER BY line", + ); + let consumers_result = tx.prepare( + "SELECT DISTINCT caller.name, caller.file, caller.line \ + FROM edges e JOIN nodes caller ON e.source_id = caller.id \ + WHERE e.target_id = ?1 AND e.kind IN ('calls', 'imports-type') AND caller.file != ?2", + ); + let insert_result = tx.prepare( + "INSERT INTO deleted_export_advisories \ + (file, name, kind, line, consumer_name, consumer_file, consumer_line, deleted_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", + ); + + if let (Ok(mut defs_stmt), Ok(mut consumers_stmt), Ok(mut insert_stmt)) = + (defs_result, consumers_result, insert_result) + { + let now = now_ms(); + for file in removed_files { + let defs: Vec<(i64, String, String, i64)> = match defs_stmt.query_map([file], |row| { + Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?)) + }) { + Ok(rows) => rows.flatten().collect(), + Err(_) => continue, + }; + for (id, name, kind, line) in defs { + let consumers: Vec<(String, String, i64)> = match consumers_stmt + .query_map(rusqlite::params![id, file], |row| { + Ok((row.get(0)?, row.get(1)?, row.get(2)?)) + }) { + Ok(rows) => rows.flatten().collect(), + Err(_) => continue, + }; + for (consumer_name, consumer_file, consumer_line) in consumers { + let _ = insert_stmt.execute(rusqlite::params![ + file, + name, + kind, + line, + consumer_name, + consumer_file, + consumer_line, + now + ]); + } + } + } + } + + let _ = tx.commit(); +} + +/// Clears advisory rows for files that are no longer deleted — e.g. a +/// previously-removed file reappearing under the same path (a revert, or an +/// unrelated new file created at the same location). Called for every file +/// about to be (re)inserted, so a resolved deletion never lingers to +/// misattribute a stale violation to whatever now lives at that path. +/// Mirrors `clearDeletedExportAdvisories` in +/// `deleted-export-advisories.ts` (#1938). +pub fn clear_deleted_export_advisories(conn: &Connection, files: &[String]) { + if files.is_empty() || !table_queryable(conn, "deleted_export_advisories") { + return; + } + let tx = match conn.unchecked_transaction() { + Ok(t) => t, + Err(_) => return, + }; + for file in files { + let _ = tx.execute("DELETE FROM deleted_export_advisories WHERE file = ?1", [file]); + } + let _ = tx.commit(); +} + /// 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`). @@ -1435,4 +1567,200 @@ mod tests { ); assert!(neighbors.is_empty()); } + + // ── deleted-export advisories (#1938) ─────────────────────────────── + + /// Schema for the advisory tests: `test_conn()` plus the `exported` + /// column and `deleted_export_advisories` table neither + /// `record_deleted_export_advisories` nor `clear_deleted_export_advisories` + /// can do without. + fn test_conn_with_advisories() -> Connection { + let conn = test_conn(); + conn.execute_batch( + "ALTER TABLE nodes ADD COLUMN exported INTEGER DEFAULT 0; + CREATE TABLE deleted_export_advisories ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + file TEXT NOT NULL, + name TEXT NOT NULL, + kind TEXT NOT NULL, + line INTEGER NOT NULL, + consumer_name TEXT NOT NULL, + consumer_file TEXT NOT NULL, + consumer_line INTEGER NOT NULL, + deleted_at INTEGER NOT NULL + );", + ) + .unwrap(); + conn + } + + fn insert_exported_node(conn: &Connection, name: &str, kind: &str, file: &str, line: i64) -> i64 { + conn.execute( + "INSERT INTO nodes (name, kind, file, line, exported) VALUES (?1, ?2, ?3, ?4, 1)", + rusqlite::params![name, kind, file, line], + ) + .unwrap(); + conn.last_insert_rowid() + } + + fn advisory_rows(conn: &Connection, file: &str) -> Vec<(String, String)> { + let mut stmt = conn + .prepare("SELECT consumer_name, consumer_file FROM deleted_export_advisories WHERE file = ?1 ORDER BY consumer_file") + .unwrap(); + stmt.query_map([file], |row| Ok((row.get(0)?, row.get(1)?))) + .unwrap() + .flatten() + .collect() + } + + #[test] + fn record_deleted_export_advisories_captures_one_row_per_external_consumer() { + let conn = test_conn_with_advisories(); + let helper = insert_exported_node(&conn, "helper", "function", "src/gone.js", 1); + let caller_a = insert_node(&conn, "callerA", "function", "src/a.js", 1); + let caller_b = insert_node(&conn, "callerB", "function", "src/b.js", 1); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'calls', 1.0, 0)", + rusqlite::params![caller_a, helper], + ) + .unwrap(); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'imports-type', 1.0, 0)", + rusqlite::params![caller_b, helper], + ) + .unwrap(); + + record_deleted_export_advisories(&conn, &["src/gone.js".to_string()]); + + assert_eq!( + advisory_rows(&conn, "src/gone.js"), + vec![ + ("callerA".to_string(), "src/a.js".to_string()), + ("callerB".to_string(), "src/b.js".to_string()), + ] + ); + } + + #[test] + fn record_deleted_export_advisories_skips_export_with_no_external_consumers() { + let conn = test_conn_with_advisories(); + insert_exported_node(&conn, "unused", "function", "src/orphan.js", 1); + + record_deleted_export_advisories(&conn, &["src/orphan.js".to_string()]); + + assert!(advisory_rows(&conn, "src/orphan.js").is_empty()); + } + + #[test] + fn record_deleted_export_advisories_ignores_same_file_caller() { + let conn = test_conn_with_advisories(); + let helper = insert_exported_node(&conn, "helper", "function", "src/gone.js", 1); + let sibling = insert_node(&conn, "sibling", "function", "src/gone.js", 10); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'calls', 1.0, 0)", + rusqlite::params![sibling, helper], + ) + .unwrap(); + + record_deleted_export_advisories(&conn, &["src/gone.js".to_string()]); + + assert!(advisory_rows(&conn, "src/gone.js").is_empty()); + } + + #[test] + fn record_deleted_export_advisories_replaces_prior_snapshot_for_same_file() { + let conn = test_conn_with_advisories(); + let helper = insert_exported_node(&conn, "helper", "function", "src/gone.js", 1); + let caller_a = insert_node(&conn, "callerA", "function", "src/a.js", 1); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'calls', 1.0, 0)", + rusqlite::params![caller_a, helper], + ) + .unwrap(); + record_deleted_export_advisories(&conn, &["src/gone.js".to_string()]); + + // A second capture pass with a different consumer set must replace, + // not accumulate alongside, the first snapshot. + let caller_c = insert_node(&conn, "callerC", "function", "src/c.js", 1); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'calls', 1.0, 0)", + rusqlite::params![caller_c, helper], + ) + .unwrap(); + record_deleted_export_advisories(&conn, &["src/gone.js".to_string()]); + + assert_eq!( + advisory_rows(&conn, "src/gone.js"), + vec![ + ("callerA".to_string(), "src/a.js".to_string()), + ("callerC".to_string(), "src/c.js".to_string()), + ] + ); + } + + #[test] + fn clear_deleted_export_advisories_removes_rows_for_reappeared_file() { + let conn = test_conn_with_advisories(); + let helper = insert_exported_node(&conn, "helper", "function", "src/gone.js", 1); + let caller_a = insert_node(&conn, "callerA", "function", "src/a.js", 1); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'calls', 1.0, 0)", + rusqlite::params![caller_a, helper], + ) + .unwrap(); + record_deleted_export_advisories(&conn, &["src/gone.js".to_string()]); + assert!(!advisory_rows(&conn, "src/gone.js").is_empty()); + + clear_deleted_export_advisories(&conn, &["src/gone.js".to_string()]); + + assert!(advisory_rows(&conn, "src/gone.js").is_empty()); + } + + #[test] + fn clear_deleted_export_advisories_leaves_other_files_untouched() { + let conn = test_conn_with_advisories(); + let helper_a = insert_exported_node(&conn, "helperA", "function", "src/gone-a.js", 1); + let caller_a = insert_node(&conn, "callerA", "function", "src/a.js", 1); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'calls', 1.0, 0)", + rusqlite::params![caller_a, helper_a], + ) + .unwrap(); + let helper_b = insert_exported_node(&conn, "helperB", "function", "src/gone-b.js", 1); + let caller_b = insert_node(&conn, "callerB", "function", "src/b.js", 1); + conn.execute( + "INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?1, ?2, 'calls', 1.0, 0)", + rusqlite::params![caller_b, helper_b], + ) + .unwrap(); + record_deleted_export_advisories( + &conn, + &["src/gone-a.js".to_string(), "src/gone-b.js".to_string()], + ); + + clear_deleted_export_advisories(&conn, &["src/gone-a.js".to_string()]); + + assert!(advisory_rows(&conn, "src/gone-a.js").is_empty()); + assert!(!advisory_rows(&conn, "src/gone-b.js").is_empty()); + } + + #[test] + fn record_and_clear_deleted_export_advisories_are_noops_for_empty_input() { + let conn = test_conn_with_advisories(); + // Must not panic or error when there is nothing to do. + record_deleted_export_advisories(&conn, &[]); + clear_deleted_export_advisories(&conn, &[]); + } + + #[test] + fn record_deleted_export_advisories_noop_when_table_missing() { + // Older/not-yet-migrated schema: no `deleted_export_advisories` table. + // Must not panic or error — just silently skip. + let conn = test_conn(); + conn.execute_batch("ALTER TABLE nodes ADD COLUMN exported INTEGER DEFAULT 0;") + .unwrap(); + insert_exported_node(&conn, "helper", "function", "src/gone.js", 1); + record_deleted_export_advisories(&conn, &["src/gone.js".to_string()]); + clear_deleted_export_advisories(&conn, &["src/gone.js".to_string()]); + } } diff --git a/src/db/index.ts b/src/db/index.ts index e2c548137..82104788b 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -26,8 +26,10 @@ export { NodeQuery, testFilterSQL, } from './query-builder.js'; +export type { DeletedExportAdvisoryEntry } from './repository/index.js'; export { bulkNodeIdsByFile, + clearDeletedExportAdvisories, countCrossFileCallers, countEdges, countFiles, @@ -41,7 +43,9 @@ export { findCallers, findCrossFileCallTargets, findDistinctCallers, + findExportedDefinitions, findExportedNodesByFile, + findExternalConsumers, findFileNodes, findImplementors, findImportDependents, @@ -63,6 +67,7 @@ export { getClassHierarchy, getCoChangeMeta, getComplexityForNode, + getDeletedExportAdvisories, getEmbeddingCount, getEmbeddingMeta, getFileNodesAll, @@ -82,6 +87,7 @@ export { purgeFileData, purgeFilesData, Repository, + recordDeletedExportAdvisories, SqliteRepository, upsertCoChangeMeta, } from './repository/index.js'; diff --git a/src/db/migrations.ts b/src/db/migrations.ts index 37a29b756..47bd3b9b4 100644 --- a/src/db/migrations.ts +++ b/src/db/migrations.ts @@ -331,6 +331,23 @@ export const MIGRATIONS: Migration[] = [ CREATE INDEX IF NOT EXISTS idx_edges_dynamic_kind ON edges(dynamic_kind) WHERE dynamic_kind IS NOT NULL; `, }, + { + version: 21, + up: ` + CREATE TABLE IF NOT EXISTS deleted_export_advisories ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + file TEXT NOT NULL, + name TEXT NOT NULL, + kind TEXT NOT NULL, + line INTEGER NOT NULL, + consumer_name TEXT NOT NULL, + consumer_file TEXT NOT NULL, + consumer_line INTEGER NOT NULL, + deleted_at INTEGER NOT NULL + ); + CREATE INDEX IF NOT EXISTS idx_deleted_export_advisories_file ON deleted_export_advisories(file); + `, + }, ]; interface PragmaColumnInfo { diff --git a/src/db/repository/deleted-export-advisories.ts b/src/db/repository/deleted-export-advisories.ts new file mode 100644 index 000000000..975c97a44 --- /dev/null +++ b/src/db/repository/deleted-export-advisories.ts @@ -0,0 +1,173 @@ +/** + * Deleted-export advisories — a durable, purge-order-independent record of + * exported function/method/class definitions lost when a file is deleted in + * its entirety, for deletions whose exports still have an external + * consumer at the moment the file is removed. + * + * `checkNoDeletedExportsInUse` (features/check.ts) can only see this via a + * live query while the deleted file's `nodes`/`edges` rows still exist in + * the DB. Those rows are purged by the very next `codegraph build` + * (`detectChanges` stage) regardless of whether `codegraph check` has run + * yet — `check` never triggers a rebuild itself, so whether it can still see + * a deleted file's exports depends entirely on external orchestration + * ordering. This module lets the build pipeline snapshot the pre-purge + * state into a durable table, at the exact point `detectChanges` computes + * the removed-file set, so `check` can fall back to it once the live rows + * are gone. See issue #1938. + */ +import type { BetterSqlite3Database, ExternalConsumerRow } from '../../types.js'; +import { findExternalConsumers } from './edges.js'; +import { findExportedDefinitions } from './nodes.js'; + +export interface DeletedExportAdvisoryEntry { + file: string; + name: string; + kind: string; + line: number; + consumers: ExternalConsumerRow[]; +} + +interface DeletedExportAdvisoryRow { + file: string; + name: string; + kind: string; + line: number; + consumer_name: string; + consumer_file: string; + consumer_line: number; +} + +/** + * `deleted_export_advisories` was only added in migration v21 — probe for it + * rather than assuming it exists, matching the try/catch pattern used + * throughout `build-stmts.ts` for other optional tables. A DB opened + * read-only via `openReadonlyOrFail` (as `check` does) never runs + * migrations, so an older DB genuinely may not have this table yet. + */ +function hasAdvisoryTable(db: BetterSqlite3Database): boolean { + try { + db.prepare('SELECT 1 FROM deleted_export_advisories LIMIT 1').get(); + return true; + } catch { + return false; + } +} + +/** + * Snapshots, for each deleted export that still has an external consumer, + * one row per consumer — captured by `detectChanges` BEFORE the build + * pipeline purges the deleted file's `nodes`/`edges` rows. + * + * Replaces any pre-existing advisory rows for the same files first. This + * keeps a single up-to-date snapshot per file rather than accumulating + * duplicates: the `file_hashes` row for a removed file is intentionally + * never purged on the incremental path (`purgeHashes: false` — see + * `purgeAndAddReverseDeps`), so a subsequent build keeps re-detecting the + * same file as "removed" and would otherwise re-run this capture every + * time. In practice this is a no-op after the first capture — querying live + * `nodes` for an already-purged file naturally returns nothing — but + * replacing first keeps this correct even if that changes. + */ +export function recordDeletedExportAdvisories( + db: BetterSqlite3Database, + removedFiles: string[], +): void { + if (removedFiles.length === 0 || !hasAdvisoryTable(db)) return; + + const deleteStmt = db.prepare('DELETE FROM deleted_export_advisories WHERE file = ?'); + const insertStmt = db.prepare( + `INSERT INTO deleted_export_advisories + (file, name, kind, line, consumer_name, consumer_file, consumer_line, deleted_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + ); + + const tx = db.transaction(() => { + const now = Date.now(); + for (const file of removedFiles) { + deleteStmt.run(file); + const defs = findExportedDefinitions(db, file); + for (const def of defs) { + const consumers = findExternalConsumers(db, def.id, file); + for (const consumer of consumers) { + insertStmt.run( + file, + def.name, + def.kind, + def.line, + consumer.name, + consumer.file, + consumer.line, + now, + ); + } + } + } + }); + tx(); +} + +/** + * Clears advisory rows for files that are no longer deleted — e.g. a + * previously-removed file reappearing under the same path (a revert, or an + * unrelated new file created at the same location). Called for every file + * about to be (re)inserted by the build pipeline, so a resolved deletion + * never lingers to misattribute a stale violation to whatever now lives at + * that path (#1938). + */ +export function clearDeletedExportAdvisories(db: BetterSqlite3Database, files: string[]): void { + if (files.length === 0 || !hasAdvisoryTable(db)) return; + const stmt = db.prepare('DELETE FROM deleted_export_advisories WHERE file = ?'); + const tx = db.transaction(() => { + for (const file of files) stmt.run(file); + }); + tx(); +} + +/** + * Reads persisted deleted-export advisories for `files`, grouped back into + * one entry per (file, name, kind, line) with its consumer list — the + * inverse of `recordDeletedExportAdvisories`'s one-row-per-consumer layout. + * + * `excludeConsumerFiles` mirrors the live-DB path's identical filter + * (`checkNoDeletedExportsInUse`): a consumer that is itself part of the same + * deletion batch being checked right now isn't a caller left dangling by the + * diff. This is applied here (against the *current* check invocation's + * deleted-file set) rather than baked in at capture time, since the set of + * files being deleted "together" from `check`'s point of view can differ + * from the build-time removed-file batch that originally captured the + * advisory. + */ +export function getDeletedExportAdvisories( + db: BetterSqlite3Database, + files: string[], + excludeConsumerFiles: Set, +): DeletedExportAdvisoryEntry[] { + if (files.length === 0 || !hasAdvisoryTable(db)) return []; + + const placeholders = files.map(() => '?').join(','); + const rows = db + .prepare( + `SELECT file, name, kind, line, consumer_name, consumer_file, consumer_line + FROM deleted_export_advisories + WHERE file IN (${placeholders}) + ORDER BY file, line`, + ) + .all(...files) as DeletedExportAdvisoryRow[]; + + const grouped = new Map(); + for (const row of rows) { + if (excludeConsumerFiles.has(row.consumer_file)) continue; + const key = `${row.file}|${row.name}|${row.kind}|${row.line}`; + let entry = grouped.get(key); + if (!entry) { + entry = { file: row.file, name: row.name, kind: row.kind, line: row.line, consumers: [] }; + grouped.set(key, entry); + } + entry.consumers.push({ + name: row.consumer_name, + file: row.consumer_file, + line: row.consumer_line, + }); + } + return [...grouped.values()].filter((e) => e.consumers.length > 0); +} diff --git a/src/db/repository/edges.ts b/src/db/repository/edges.ts index 5ddf1fe33..a17390913 100644 --- a/src/db/repository/edges.ts +++ b/src/db/repository/edges.ts @@ -1,6 +1,7 @@ import type { AdjacentEdgeRow, BetterSqlite3Database, + ExternalConsumerRow, ImportEdgeRow, IntraFileCallEdge, NodeRow, @@ -22,6 +23,7 @@ const _findImportSourcesStmt: StmtCache = new WeakMap(); const _findImportDependentsStmt: StmtCache = new WeakMap(); const _findCrossFileCallTargetsStmt: StmtCache<{ target_id: number }> = new WeakMap(); const _countCrossFileCallersStmt: StmtCache<{ cnt: number }> = new WeakMap(); +const _findExternalConsumersStmt: StmtCache = new WeakMap(); const _getClassAncestorsStmt: StmtCache<{ id: number; name: string }> = new WeakMap(); const _findIntraFileCallEdgesStmt: StmtCache = new WeakMap(); const _findImplementorsStmt: StmtCache = new WeakMap(); @@ -208,6 +210,28 @@ export function countCrossFileCallers( ); } +/** + * Find distinct cross-file consumers of a node — callers of a `calls` edge + * or importers of an `imports-type` edge whose own file differs from + * `file`. Backs `checkNoDeletedExportsInUse`'s live-DB path and the + * pre-purge advisory snapshot the build pipeline captures for it before a + * removed file's rows are purged (#1938). + */ +export function findExternalConsumers( + db: BetterSqlite3Database, + nodeId: number, + file: string, +): ExternalConsumerRow[] { + return cachedStmt( + _findExternalConsumersStmt, + db, + `SELECT DISTINCT caller.name, caller.file, caller.line + FROM edges e + JOIN nodes caller ON e.source_id = caller.id + WHERE e.target_id = ? AND e.kind IN ('calls', 'imports-type') AND caller.file != ?`, + ).all(nodeId, file); +} + /** * Get all ancestor class IDs via extends edges (BFS). */ diff --git a/src/db/repository/index.ts b/src/db/repository/index.ts index 5fd5d1ff9..afd9090df 100644 --- a/src/db/repository/index.ts +++ b/src/db/repository/index.ts @@ -8,6 +8,12 @@ export { deleteCfgForNode, getCfgBlocks, getCfgEdges, hasCfgTables } from './cfg export { getCoChangeMeta, hasCoChanges, upsertCoChangeMeta } from './cochange.js'; export { getComplexityForNode } from './complexity.js'; export { hasDataflowTable, hasDataflowVertices } from './dataflow.js'; +export type { DeletedExportAdvisoryEntry } from './deleted-export-advisories.js'; +export { + clearDeletedExportAdvisories, + getDeletedExportAdvisories, + recordDeletedExportAdvisories, +} from './deleted-export-advisories.js'; export { countCrossFileCallers, findAllIncomingEdges, @@ -18,6 +24,7 @@ export { findCallers, findCrossFileCallTargets, findDistinctCallers, + findExternalConsumers, findImplementors, findImportDependents, findImportSources, @@ -35,6 +42,7 @@ export { countEdges, countFiles, countNodes, + findExportedDefinitions, findExportedNodesByFile, findFileNodes, findNodeById, diff --git a/src/db/repository/nodes.ts b/src/db/repository/nodes.ts index b4f12e6e3..635c0a848 100644 --- a/src/db/repository/nodes.ts +++ b/src/db/repository/nodes.ts @@ -3,6 +3,7 @@ import { EVERY_SYMBOL_KIND, VALID_ROLES } from '../../shared/kinds.js'; import type { BetterSqlite3Database, ChildNodeRow, + ExportedDefRow, ListFunctionOpts, NativeDatabase, NodeIdRow, @@ -231,6 +232,27 @@ export function findExportedNodesByFile(db: BetterSqlite3Database, file: string) return symbols.filter((s) => exportedIds.has(s.id)); } +const _findExportedDefinitionsStmt: StmtCache = new WeakMap(); + +/** + * Find exported function/method/class definitions in `file` — the narrower + * "declaration surface" filter used by `check.ts`'s signature-change + * predicates (`checkNoSignatureChanges`, `checkNoDeletedExportsInUse`), as + * distinct from `findExportedNodesByFile`'s broader "any exported kind" used + * by `codegraph exports`/`where --file`. Also backs the deleted-export + * advisory snapshot the build pipeline captures before purging a removed + * file's rows (#1938). + */ +export function findExportedDefinitions(db: BetterSqlite3Database, file: string): ExportedDefRow[] { + return cachedStmt( + _findExportedDefinitionsStmt, + db, + `SELECT id, name, kind, line FROM nodes + WHERE file = ? AND kind IN ('function', 'method', 'class') AND exported = 1 + ORDER BY line`, + ).all(file); +} + /** * Find file-kind nodes matching a LIKE pattern. */ diff --git a/src/domain/graph/builder/stages/detect-changes.ts b/src/domain/graph/builder/stages/detect-changes.ts index 1bb477b57..1da091790 100644 --- a/src/domain/graph/builder/stages/detect-changes.ts +++ b/src/domain/graph/builder/stages/detect-changes.ts @@ -8,7 +8,11 @@ import fs from 'node:fs'; import path from 'node:path'; import { performance } from 'node:perf_hooks'; -import { closeDb } from '../../../../db/index.js'; +import { + clearDeletedExportAdvisories, + closeDb, + recordDeletedExportAdvisories, +} from '../../../../db/index.js'; import { debug, info } from '../../../../infrastructure/logger.js'; import { normalizePath } from '../../../../shared/constants.js'; import { toErrorMessage } from '../../../../shared/errors.js'; @@ -610,6 +614,11 @@ function handleScopedBuild(ctx: PipelineContext): void { reverseDeps = findReverseDependencies(db, changedRelPaths, rootDir, ctx.nativeDb); } ctx.removedFileNeighbors = captureRemovedFileNeighbors(db, ctx.removed); + // A file about to be (re)inserted can no longer be "deleted" — clear any + // stale advisory left over from a prior removal at this same path before + // capturing this build's actual removals (#1938). + clearDeletedExportAdvisories(db, changePaths); + recordDeletedExportAdvisories(db, ctx.removed); purgeAndAddReverseDeps(ctx, changePaths, reverseDeps); info( `Scoped rebuild: ${changePaths.length} changed, ${ctx.removed.length} removed, ${reverseDeps.size} reverse-deps`, @@ -653,6 +662,11 @@ function handleIncrementalBuild(ctx: PipelineContext): void { (item) => item.relPath || normalizePath(path.relative(rootDir, item.file)), ); ctx.removedFileNeighbors = captureRemovedFileNeighbors(db, ctx.removed); + // A file about to be (re)inserted can no longer be "deleted" — clear any + // stale advisory left over from a prior removal at this same path before + // capturing this build's actual removals (#1938). + clearDeletedExportAdvisories(db, changePaths); + recordDeletedExportAdvisories(db, ctx.removed); purgeAndAddReverseDeps(ctx, changePaths, reverseDeps); } diff --git a/src/features/check.ts b/src/features/check.ts index fbd351511..0c101a71d 100644 --- a/src/features/check.ts +++ b/src/features/check.ts @@ -1,7 +1,13 @@ import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; -import { findDbPath, openReadonlyOrFail } from '../db/index.js'; +import { + findDbPath, + findExportedDefinitions, + findExternalConsumers, + getDeletedExportAdvisories, + openReadonlyOrFail, +} from '../db/index.js'; import { bfsTransitiveCallers } from '../domain/analysis/impact.js'; import type { Cycle } from '../domain/graph/cycles.js'; import { findCycles } from '../domain/graph/cycles.js'; @@ -576,14 +582,6 @@ export function checkNoSignatureChanges( return { passed: violations.length === 0, violations }; } -type DeletedDefRow = { - id: number; - name: string; - kind: string; - file: string; - line: number; -}; - /** * Detects exported functions/methods/classes lost when a file is deleted in * its entirety, for deletions whose exports still have consumers elsewhere @@ -601,15 +599,13 @@ type DeletedDefRow = { * some *other*, separate `codegraph build` invocation has already purged it * by the time `check` runs. * - * This closes that gap for the common case: `db` still reflects pre-purge - * state whenever `codegraph check --staged` runs before any rebuild has - * observed the deletion (e.g. this repo's own pre-commit hook, which checks - * staged changes without rebuilding first). Once some rebuild purges the - * deleted file's rows, this predicate has nothing left to find for it — the - * violation silently goes undetected, same as before this predicate - * existed. See issue #1806 for the follow-up tracking a durable, - * purge-order-independent fix (e.g. capturing this at purge time inside the - * build pipeline itself, so it survives regardless of invocation order). + * For a file whose `nodes` rows are still live (no rebuild has purged it + * yet), this queries them directly. Once a rebuild purges them, it falls + * back to `deleted_export_advisories` — a durable snapshot the build + * pipeline captures at the exact point it computes the removed-file set, + * BEFORE purging (see `recordDeletedExportAdvisories` / + * `db/repository/deleted-export-advisories.ts`) — so the violation stays + * visible regardless of build/check invocation order (#1938). * * Unlike `checkNoSignatureChanges` (which flags any touched exported * declaration regardless of caller count, since editing an exported line is @@ -636,36 +632,47 @@ export function checkNoDeletedExportsInUse( const violations: SignatureViolation[] = []; if (deletedFiles.size === 0) return { passed: true, violations }; - const defsStmt = db.prepare( - `SELECT id, name, kind, file, line FROM nodes WHERE file = ? AND kind IN ('function', 'method', 'class') AND exported = 1 ORDER BY line`, - ); - const consumersStmt = db.prepare( - `SELECT DISTINCT caller.name, caller.file, caller.line - FROM edges e - JOIN nodes caller ON e.source_id = caller.id - WHERE e.target_id = ? AND e.kind IN ('calls', 'imports-type') AND caller.file != ?`, - ); - for (const file of deletedFiles) { if (noTests && isTestFile(file)) continue; - const defs = defsStmt.all(file) as DeletedDefRow[]; - for (const def of defs) { - let consumers = consumersStmt.all(def.id, def.file) as ConsumerRef[]; - // A caller that is itself among the files this same diff deletes isn't - // an external caller left dangling by the diff — it's being removed - // too. Mirrors checkNoSignatureChanges's exported-only filter: only a - // caller reachable from outside the set of files this diff removes can - // be a caller the diff doesn't already account for. - consumers = consumers.filter((c) => !deletedFiles.has(c.file)); + const defs = findExportedDefinitions(db, file); + if (defs.length > 0) { + for (const def of defs) { + let consumers: ConsumerRef[] = findExternalConsumers(db, def.id, file); + // A caller that is itself among the files this same diff deletes isn't + // an external caller left dangling by the diff — it's being removed + // too. Mirrors checkNoSignatureChanges's exported-only filter: only a + // caller reachable from outside the set of files this diff removes can + // be a caller the diff doesn't already account for. + consumers = consumers.filter((c) => !deletedFiles.has(c.file)); + if (noTests) consumers = consumers.filter((c) => !isTestFile(c.file)); + if (consumers.length === 0) continue; + + violations.push({ + name: def.name, + kind: def.kind, + file, + line: def.line, + reason: 'file-deleted', + consumers, + }); + } + continue; + } + + // `nodes` rows for `file` are already gone — some rebuild purged them + // before this check ran. Fall back to the pre-purge advisory snapshot. + const advisories = getDeletedExportAdvisories(db, [file], deletedFiles); + for (const advisory of advisories) { + let consumers = advisory.consumers; if (noTests) consumers = consumers.filter((c) => !isTestFile(c.file)); if (consumers.length === 0) continue; violations.push({ - name: def.name, - kind: def.kind, - file: def.file, - line: def.line, + name: advisory.name, + kind: advisory.kind, + file: advisory.file, + line: advisory.line, reason: 'file-deleted', consumers, }); diff --git a/src/types.ts b/src/types.ts index 905524a2e..c97d4f2fd 100644 --- a/src/types.ts +++ b/src/types.ts @@ -208,6 +208,30 @@ export interface AdjacentEdgeRow { edge_kind: EdgeKind; } +/** + * An exported function/method/class definition (from findExportedDefinitions). + * Narrower than `findExportedNodesByFile`'s "any exported kind" — this is the + * declaration-surface filter shared by check.ts's signature-change + * predicates (`checkNoSignatureChanges`, `checkNoDeletedExportsInUse`) and + * the deleted-export advisory capture that backs it (#1938). + */ +export interface ExportedDefRow { + id: number; + name: string; + kind: string; + line: number; +} + +/** + * A cross-file consumer of an exported symbol (from findExternalConsumers), + * or a persisted deleted-export advisory's consumer row (#1938). + */ +export interface ExternalConsumerRow { + name: string; + file: string; + line: number; +} + /** Import target/source row. */ export interface ImportEdgeRow { file: string; diff --git a/tests/integration/check.test.ts b/tests/integration/check.test.ts index 5a12ae53e..cd6b252c9 100644 --- a/tests/integration/check.test.ts +++ b/tests/integration/check.test.ts @@ -795,6 +795,101 @@ describe('checkNoDeletedExportsInUse', () => { }); }); +// ─── checkNoDeletedExportsInUse: advisory fallback (issue #1938) ─────── +// +// `src/purged.js` deliberately has NO rows in `nodes` — simulating a file +// whose exported-symbol rows were already purged by an intervening rebuild +// before `codegraph check` ran. Its persisted `deleted_export_advisories` +// snapshot (captured by that rebuild, before the purge) is what +// `checkNoDeletedExportsInUse` must fall back to. + +function insertAdvisory(db, file, name, kind, line, consumerName, consumerFile, consumerLine) { + db.prepare( + `INSERT INTO deleted_export_advisories + (file, name, kind, line, consumer_name, consumer_file, consumer_line, deleted_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + ).run(file, name, kind, line, consumerName, consumerFile, consumerLine, Date.now()); +} + +describe('checkNoDeletedExportsInUse: advisory fallback after purge (issue #1938)', () => { + test('flags a violation from the persisted advisory snapshot once live nodes are already purged', () => { + insertAdvisory( + db, + 'src/purged.js', + 'purgedHelper', + 'function', + 3, + 'useIt', + 'src/other-consumer.js', + 5, + ); + + const result = checkNoDeletedExportsInUse(db, new Set(['src/purged.js']), false); + expect(result.passed).toBe(false); + const violation = result.violations.find((v) => v.name === 'purgedHelper'); + expect(violation).toBeDefined(); + expect(violation.reason).toBe('file-deleted'); + expect(violation.consumers.map((c) => c.file)).toEqual(['src/other-consumer.js']); + }); + + test('excludes an advisory consumer that is itself part of the current deletion batch', () => { + insertAdvisory( + db, + 'src/purged-2.js', + 'purgedHelper2', + 'function', + 1, + 'useIt2', + 'src/purged-2-consumer.js', + 2, + ); + + const result = checkNoDeletedExportsInUse( + db, + new Set(['src/purged-2.js', 'src/purged-2-consumer.js']), + false, + ); + expect(result.violations.map((v) => v.name)).not.toContain('purgedHelper2'); + }); + + test('respects noTests when filtering advisory consumers', () => { + insertAdvisory( + db, + 'src/purged-3.js', + 'purgedHelper3', + 'function', + 1, + 'testOnlyCaller', + 'tests/purged-3.test.js', + 2, + ); + + const result = checkNoDeletedExportsInUse(db, new Set(['src/purged-3.js']), true); + expect(result.violations.map((v) => v.name)).not.toContain('purgedHelper3'); + }); + + test('prefers the live query over a stale advisory when nodes still exist', () => { + // src/math.js DOES have live nodes in the fixture DB — a stale advisory + // row for it (as if left over from a since-resolved earlier deletion) + // must not be surfaced instead of (or in addition to) the live result. + insertAdvisory( + db, + 'src/math.js', + 'stalePhantomExport', + 'function', + 99, + 'staleCaller', + 'src/stale-caller.js', + 1, + ); + + const result = checkNoDeletedExportsInUse(db, new Set(['src/math.js']), false); + expect(result.violations.map((v) => v.name)).not.toContain('stalePhantomExport'); + // The real live violation (`add`) must still be reported normally. + expect(result.violations.map((v) => v.name)).toContain('add'); + }); +}); + // ─── checkNoBoundaryViolations ──────────────────────────────────────── describe('checkNoBoundaryViolations', () => { diff --git a/tests/integration/issue-1938-deleted-export-advisory-persistence.test.ts b/tests/integration/issue-1938-deleted-export-advisory-persistence.test.ts new file mode 100644 index 000000000..017a2a006 --- /dev/null +++ b/tests/integration/issue-1938-deleted-export-advisory-persistence.test.ts @@ -0,0 +1,168 @@ +/** + * Regression test for #1938: `checkNoDeletedExportsInUse` (#1806) missed a + * deleted file's exported-symbol violation once a *separate* `codegraph + * build` invocation had already purged that file's `nodes`/`edges` rows — + * which `detectChanges` does unconditionally for any file no longer found + * on disk, regardless of whether `codegraph check` has run yet. + * + * Root cause: the predicate only ever queried the *current* DB state, with + * no durable record of what a deleted file's exports/consumers looked like + * before the purge. Fixed by capturing a snapshot into + * `deleted_export_advisories` at the exact point `detectChanges` computes + * the removed-file set — BEFORE purging — so `checkNoDeletedExportsInUse` + * can fall back to it once the live rows are gone. Both the WASM/JS + * pipeline (`db/repository/deleted-export-advisories.ts`, wired into + * `detect-changes.ts`) and the native Rust orchestrator fast path + * (`record_deleted_export_advisories`/`clear_deleted_export_advisories` in + * `detect_changes.rs`, wired into `pipeline.rs`'s `save_and_purge_changed`) + * must produce the same result, since the native path bypasses the JS + * `detectChanges` stage entirely for a plain incremental build. + * + * Strategy: build a real two-file project, delete the file with the + * external consumer, run a real incremental `buildGraph` (which purges the + * deleted file's rows via whichever engine's purge path), THEN run + * `checkData` against the staged deletion and confirm the violation is + * still reported — proving detection survives purge ordering, not just the + * "check before any rebuild" case #1806 already covered. + */ +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, describe, expect, it } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { checkData } from '../../src/features/check.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; + +function git(cwd: string, args: string[]): void { + execFileSync('git', args, { cwd, stdio: 'pipe' }); +} + +function runScenario(engine: 'wasm' | 'native'): void { + describe(`deleted-export advisory survives a purge via the ${engine} engine (#1938)`, () => { + const tmpDirs: string[] = []; + + function mkTmp(prefix: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tmpDirs.push(dir); + return dir; + } + + afterAll(() => { + for (const dir of tmpDirs) fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('flags the violation from the persisted advisory after an intervening rebuild purges the deleted file', async () => { + const projectDir = mkTmp(`cg-1938-${engine}-`); + fs.mkdirSync(path.join(projectDir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(projectDir, 'package.json'), + JSON.stringify({ name: 'test-1938', version: '1.0.0', type: 'module' }), + ); + fs.writeFileSync( + path.join(projectDir, 'src', 'shared.js'), + 'export function sharedHelper() {\n return 1;\n}\n', + ); + fs.writeFileSync( + path.join(projectDir, 'src', 'consumer.js'), + "import { sharedHelper } from './shared.js';\nexport function useShared() {\n return sharedHelper();\n}\n", + ); + + git(projectDir, ['init']); + git(projectDir, ['config', 'user.email', 'test@test.com']); + git(projectDir, ['config', 'user.name', 'Test']); + git(projectDir, ['add', '.']); + git(projectDir, ['commit', '-m', 'init']); + + // Initial full build so the graph reflects the committed state. + await buildGraph(projectDir, { engine, incremental: false, skipRegistry: true }); + + // Stage the deletion of shared.js — consumer.js is left untouched, + // still importing/calling it. + git(projectDir, ['rm', 'src/shared.js']); + + const dbPath = path.join(projectDir, '.codegraph', 'graph.db'); + + // THE KEY STEP: rebuild the graph now (incremental — the default), + // separately from the `checkData` call below. This is what #1806 + // could not survive: detectChanges purges shared.js's nodes/edges + // unconditionally, before check ever runs. + await buildGraph(projectDir, { engine, skipRegistry: true }); + + // Sanity check: the purge actually happened — shared.js's exported + // node must be gone from the live DB by now. If this fails, the test + // isn't exercising the purged-state path at all. + const verifyDb = new Database(dbPath, { readonly: true }); + try { + const liveRows = verifyDb.prepare("SELECT * FROM nodes WHERE file = 'src/shared.js'").all(); + expect(liveRows).toEqual([]); + } finally { + verifyDb.close(); + } + + const data = checkData(dbPath, { + staged: true, + signatures: true, + cycles: false, + boundaries: false, + }); + + expect(data.error).toBeUndefined(); + expect(data.passed).toBe(false); + const sigPred = data.predicates.find((p) => p.name === 'signatures'); + expect(sigPred).toBeDefined(); + expect(sigPred.passed).toBe(false); + const violation = sigPred.violations.find((v) => v.name === 'sharedHelper'); + expect(violation).toBeDefined(); + expect(violation.reason).toBe('file-deleted'); + expect(violation.consumers.map((c) => c.file)).toContain('src/consumer.js'); + }, 60_000); + + it('clears the advisory once the deleted file reappears, so a later unrelated deletion at the same path is not misattributed', async () => { + const projectDir = mkTmp(`cg-1938-revert-${engine}-`); + fs.mkdirSync(path.join(projectDir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(projectDir, 'package.json'), + JSON.stringify({ name: 'test-1938-revert', version: '1.0.0', type: 'module' }), + ); + fs.writeFileSync( + path.join(projectDir, 'src', 'shared.js'), + 'export function sharedHelper() {\n return 1;\n}\n', + ); + fs.writeFileSync( + path.join(projectDir, 'src', 'consumer.js'), + "import { sharedHelper } from './shared.js';\nexport function useShared() {\n return sharedHelper();\n}\n", + ); + + // No git repo needed here — this test only checks the build + // pipeline's own advisory bookkeeping, not `checkData`'s git-diff path. + await buildGraph(projectDir, { engine, incremental: false, skipRegistry: true }); + + // Delete shared.js, rebuild (purges + captures the advisory), then + // bring it back with NO exports at all before the next rebuild. + fs.rmSync(path.join(projectDir, 'src', 'shared.js')); + await buildGraph(projectDir, { engine, skipRegistry: true }); + + fs.writeFileSync(path.join(projectDir, 'src', 'shared.js'), '// no exports here\n'); + await buildGraph(projectDir, { engine, skipRegistry: true }); + + const dbPath = path.join(projectDir, '.codegraph', 'graph.db'); + const verifyDb = new Database(dbPath, { readonly: true }); + try { + const advisoryRows = verifyDb + .prepare("SELECT * FROM deleted_export_advisories WHERE file = 'src/shared.js'") + .all(); + expect(advisoryRows).toEqual([]); + } finally { + verifyDb.close(); + } + }, 60_000); + }); +} + +runScenario('wasm'); + +describe.skipIf(!isNativeAvailable())('native engine coverage', () => { + runScenario('native'); +}); diff --git a/tests/unit/deleted-export-advisories.test.ts b/tests/unit/deleted-export-advisories.test.ts new file mode 100644 index 000000000..09f8becce --- /dev/null +++ b/tests/unit/deleted-export-advisories.test.ts @@ -0,0 +1,170 @@ +/** + * Unit tests for `db/repository/deleted-export-advisories.ts` — the durable + * pre-purge snapshot that lets `checkNoDeletedExportsInUse` still see a + * deleted file's exported-symbol violations once a rebuild has already + * purged its `nodes`/`edges` rows. See issue #1938. + */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeEach, describe, expect, test } from 'vitest'; +import { initSchema } from '../../src/db/index.js'; +import { + clearDeletedExportAdvisories, + getDeletedExportAdvisories, + recordDeletedExportAdvisories, +} from '../../src/db/repository/deleted-export-advisories.js'; + +function insertNode(db, name, kind, file, line, exported = 0) { + return db + .prepare('INSERT INTO nodes (name, kind, file, line, exported) VALUES (?, ?, ?, ?, ?)') + .run(name, kind, file, line, exported).lastInsertRowid; +} + +function insertEdge(db, sourceId, targetId, kind) { + db.prepare( + 'INSERT INTO edges (source_id, target_id, kind, confidence) VALUES (?, ?, ?, 1.0)', + ).run(sourceId, targetId, kind); +} + +let tmpDir: string; +let db: any; + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-deleted-export-advisories-')); + db = new Database(path.join(tmpDir, 'graph.db')); + db.pragma('journal_mode = WAL'); + initSchema(db); +}); + +afterAll(() => { + if (db) db.close(); +}); + +describe('recordDeletedExportAdvisories', () => { + test('captures one row per external consumer of an exported def', () => { + const helperId = insertNode(db, 'helper', 'function', 'src/gone.js', 1, 1); + const callerAId = insertNode(db, 'callerA', 'function', 'src/a.js', 1); + const callerBId = insertNode(db, 'callerB', 'function', 'src/b.js', 1); + insertEdge(db, callerAId, helperId, 'calls'); + insertEdge(db, callerBId, helperId, 'imports-type'); + + recordDeletedExportAdvisories(db, ['src/gone.js']); + + const entries = getDeletedExportAdvisories(db, ['src/gone.js'], new Set()); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('helper'); + expect(entries[0].consumers.map((c) => c.file).sort()).toEqual(['src/a.js', 'src/b.js']); + }); + + test('does not record an advisory for an export with no external consumers', () => { + insertNode(db, 'unusedHelper', 'function', 'src/orphan.js', 1, 1); + + recordDeletedExportAdvisories(db, ['src/orphan.js']); + + expect(getDeletedExportAdvisories(db, ['src/orphan.js'], new Set())).toEqual([]); + }); + + test('ignores a caller living in the same removed file', () => { + const helperId = insertNode(db, 'helper', 'function', 'src/gone.js', 1, 1); + const sameFileCallerId = insertNode(db, 'sibling', 'function', 'src/gone.js', 10); + insertEdge(db, sameFileCallerId, helperId, 'calls'); + + recordDeletedExportAdvisories(db, ['src/gone.js']); + + expect(getDeletedExportAdvisories(db, ['src/gone.js'], new Set())).toEqual([]); + }); + + test('is a no-op for an empty removed-files list', () => { + insertNode(db, 'helper', 'function', 'src/gone.js', 1, 1); + recordDeletedExportAdvisories(db, []); + expect(getDeletedExportAdvisories(db, ['src/gone.js'], new Set())).toEqual([]); + }); + + test('replaces a prior snapshot for the same file rather than accumulating', () => { + const helperId = insertNode(db, 'helper', 'function', 'src/gone.js', 1, 1); + const callerAId = insertNode(db, 'callerA', 'function', 'src/a.js', 1); + insertEdge(db, callerAId, helperId, 'calls'); + recordDeletedExportAdvisories(db, ['src/gone.js']); + + // Simulate a second capture pass (e.g. a redundant re-detection) with a + // different consumer set — the stale first snapshot must not linger + // alongside the fresh one. + const callerCId = insertNode(db, 'callerC', 'function', 'src/c.js', 1); + insertEdge(db, callerCId, helperId, 'calls'); + recordDeletedExportAdvisories(db, ['src/gone.js']); + + const entries = getDeletedExportAdvisories(db, ['src/gone.js'], new Set()); + expect(entries).toHaveLength(1); + expect(entries[0].consumers.map((c) => c.file).sort()).toEqual(['src/a.js', 'src/c.js']); + }); +}); + +describe('getDeletedExportAdvisories', () => { + test('excludes consumers whose file is in excludeConsumerFiles', () => { + const helperId = insertNode(db, 'helper', 'function', 'src/gone.js', 1, 1); + const callerAId = insertNode(db, 'callerA', 'function', 'src/a.js', 1); + const callerBId = insertNode(db, 'callerB', 'function', 'src/b.js', 1); + insertEdge(db, callerAId, helperId, 'calls'); + insertEdge(db, callerBId, helperId, 'calls'); + recordDeletedExportAdvisories(db, ['src/gone.js']); + + const entries = getDeletedExportAdvisories(db, ['src/gone.js'], new Set(['src/a.js'])); + expect(entries).toHaveLength(1); + expect(entries[0].consumers.map((c) => c.file)).toEqual(['src/b.js']); + }); + + test('drops an entry entirely once all its consumers are excluded', () => { + const helperId = insertNode(db, 'helper', 'function', 'src/gone.js', 1, 1); + const callerAId = insertNode(db, 'callerA', 'function', 'src/a.js', 1); + insertEdge(db, callerAId, helperId, 'calls'); + recordDeletedExportAdvisories(db, ['src/gone.js']); + + expect(getDeletedExportAdvisories(db, ['src/gone.js'], new Set(['src/a.js']))).toEqual([]); + }); + + test('returns nothing for a file with no recorded advisory', () => { + expect(getDeletedExportAdvisories(db, ['src/never-deleted.js'], new Set())).toEqual([]); + }); +}); + +describe('clearDeletedExportAdvisories', () => { + test('removes advisory rows for a file that has reappeared', () => { + const helperId = insertNode(db, 'helper', 'function', 'src/gone.js', 1, 1); + const callerAId = insertNode(db, 'callerA', 'function', 'src/a.js', 1); + insertEdge(db, callerAId, helperId, 'calls'); + recordDeletedExportAdvisories(db, ['src/gone.js']); + expect(getDeletedExportAdvisories(db, ['src/gone.js'], new Set())).toHaveLength(1); + + clearDeletedExportAdvisories(db, ['src/gone.js']); + + expect(getDeletedExportAdvisories(db, ['src/gone.js'], new Set())).toEqual([]); + }); + + test('leaves other files untouched', () => { + const helperId = insertNode(db, 'helper', 'function', 'src/gone.js', 1, 1); + const callerAId = insertNode(db, 'callerA', 'function', 'src/a.js', 1); + insertEdge(db, callerAId, helperId, 'calls'); + const otherId = insertNode(db, 'otherHelper', 'function', 'src/other-gone.js', 1, 1); + const otherCallerId = insertNode(db, 'otherCaller', 'function', 'src/d.js', 1); + insertEdge(db, otherCallerId, otherId, 'calls'); + recordDeletedExportAdvisories(db, ['src/gone.js', 'src/other-gone.js']); + + clearDeletedExportAdvisories(db, ['src/gone.js']); + + expect(getDeletedExportAdvisories(db, ['src/gone.js'], new Set())).toEqual([]); + expect(getDeletedExportAdvisories(db, ['src/other-gone.js'], new Set())).toHaveLength(1); + }); + + test('is a no-op for an empty file list', () => { + const helperId = insertNode(db, 'helper', 'function', 'src/gone.js', 1, 1); + const callerAId = insertNode(db, 'callerA', 'function', 'src/a.js', 1); + insertEdge(db, callerAId, helperId, 'calls'); + recordDeletedExportAdvisories(db, ['src/gone.js']); + + clearDeletedExportAdvisories(db, []); + + expect(getDeletedExportAdvisories(db, ['src/gone.js'], new Set())).toHaveLength(1); + }); +});