diff --git a/crates/codegraph-core/src/native_db.rs b/crates/codegraph-core/src/native_db.rs index e773740a..8f55b00c 100644 --- a/crates/codegraph-core/src/native_db.rs +++ b/crates/codegraph-core/src/native_db.rs @@ -720,13 +720,18 @@ impl NativeDatabase { /// Cascade-delete all graph data for the specified files across all tables. /// Order: dependent tables first (embeddings, cfg, dataflow, complexity, /// metrics, ast_nodes), then edges, then nodes, then optionally file_hashes. + /// + /// When `reverse_dep_files` is provided, outgoing edges for those files are + /// also deleted in the same transaction, closing the atomicity gap between + /// purge and reverse-dependency edge cleanup (see #670). #[napi] pub fn purge_files_data( &self, files: Vec, purge_hashes: Option, + reverse_dep_files: Option>, ) -> napi::Result<()> { - if files.is_empty() { + if files.is_empty() && reverse_dep_files.as_ref().map_or(true, |v| v.is_empty()) { return Ok(()); } let conn = self.conn()?; @@ -768,6 +773,22 @@ impl NativeDatabase { } } + // Delete outgoing edges for reverse-dep files in the same transaction (#670). + // These files keep their nodes but need outgoing edges rebuilt. + if let Some(ref rev_files) = reverse_dep_files { + for file in rev_files { + tx.execute( + "DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = ?1)", + params![file], + ) + .map_err(|e| { + napi::Error::from_reason(format!( + "reverse-dep edge purge failed for \"{file}\": {e}" + )) + })?; + } + } + tx.commit() .map_err(|e| napi::Error::from_reason(format!("purge commit failed: {e}")))?; Ok(()) diff --git a/src/domain/graph/builder/stages/detect-changes.ts b/src/domain/graph/builder/stages/detect-changes.ts index ad5d83d7..810c615a 100644 --- a/src/domain/graph/builder/stages/detect-changes.ts +++ b/src/domain/graph/builder/stages/detect-changes.ts @@ -331,26 +331,32 @@ function purgeAndAddReverseDeps( reverseDeps: Set, ): void { const { db, rootDir } = ctx; - if (changePaths.length > 0 || ctx.removed.length > 0) { - const filesToPurge = [...ctx.removed, ...changePaths]; - // Prefer NativeDatabase persistent connection for purge (6.15) + const hasPurge = changePaths.length > 0 || ctx.removed.length > 0; + const hasReverseDeps = reverseDeps.size > 0; + const reverseDepList = hasReverseDeps ? [...reverseDeps] : []; + + if (hasPurge || hasReverseDeps) { + const filesToPurge = hasPurge ? [...ctx.removed, ...changePaths] : []; + // Prefer NativeDatabase: purge + reverse-dep edge deletion in one transaction (#670) if (ctx.nativeDb?.purgeFilesData) { - ctx.nativeDb.purgeFilesData(filesToPurge, false); + ctx.nativeDb.purgeFilesData(filesToPurge, false, hasReverseDeps ? reverseDepList : undefined); } else { - purgeFilesFromGraph(db, filesToPurge, { purgeHashes: false }); + if (hasPurge) { + purgeFilesFromGraph(db, filesToPurge, { purgeHashes: false }); + } + if (hasReverseDeps) { + const deleteOutgoingEdgesForFile = db.prepare( + 'DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = ?)', + ); + for (const relPath of reverseDepList) { + deleteOutgoingEdgesForFile.run(relPath); + } + } } } - if (reverseDeps.size > 0) { - const deleteOutgoingEdgesForFile = db.prepare( - 'DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = ?)', - ); - for (const relPath of reverseDeps) { - deleteOutgoingEdgesForFile.run(relPath); - } - for (const relPath of reverseDeps) { - const absPath = path.join(rootDir, relPath); - ctx.parseChanges.push({ file: absPath, relPath, _reverseDepOnly: true }); - } + for (const relPath of reverseDeps) { + const absPath = path.join(rootDir, relPath); + ctx.parseChanges.push({ file: absPath, relPath, _reverseDepOnly: true }); } } diff --git a/src/types.ts b/src/types.ts index 91b89d10..00870ef9 100644 --- a/src/types.ts +++ b/src/types.ts @@ -2076,7 +2076,7 @@ export interface NativeDatabase { testOnly: number; leaf: number; } | null; - purgeFilesData(files: string[], purgeHashes?: boolean): void; + purgeFilesData(files: string[], purgeHashes?: boolean, reverseDepFiles?: string[]): void; // ── Generic query execution & version validation (6.16) ───────────── /** Execute a parameterized SELECT and return all rows as objects. */ diff --git a/tests/unit/native-db-purge-reverse-deps.test.ts b/tests/unit/native-db-purge-reverse-deps.test.ts new file mode 100644 index 00000000..c58caec7 --- /dev/null +++ b/tests/unit/native-db-purge-reverse-deps.test.ts @@ -0,0 +1,122 @@ +/** + * Unit tests for NativeDatabase.purgeFilesData with reverse_dep_files (#670). + * + * Verifies that file purge + reverse-dep outgoing-edge deletion happen + * atomically in a single transaction when using the native engine. + */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { getNative, isNativeAvailable } from '../../src/infrastructure/native.js'; +import type { NativeDatabase } from '../../src/types.js'; + +const hasNativeDb = + isNativeAvailable() && + typeof getNative().NativeDatabase?.prototype?.purgeFilesData === 'function'; + +describe.skipIf(!hasNativeDb)('NativeDatabase.purgeFilesData with reverseDepFiles', () => { + let nativeDb: NativeDatabase; + let dbPath: string; + + beforeEach(() => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-purge-revdep-')); + dbPath = path.join(tmpDir, 'test.db'); + const NativeDB = getNative().NativeDatabase; + nativeDb = NativeDB.openReadWrite(dbPath); + nativeDb.initSchema(); + + // Seed: three files with cross-file edges + // auth.js: authenticate() --calls--> utils.js: validateToken() + // app.js: main() --calls--> auth.js: authenticate() + // app.js: main() --calls--> utils.js: formatResponse() + nativeDb.exec(` + INSERT INTO nodes (id, name, kind, file, line) VALUES (1, 'authenticate', 'function', 'auth.js', 10); + INSERT INTO nodes (id, name, kind, file, line) VALUES (2, 'validateToken', 'function', 'utils.js', 5); + INSERT INTO nodes (id, name, kind, file, line) VALUES (3, 'main', 'function', 'app.js', 1); + INSERT INTO nodes (id, name, kind, file, line) VALUES (4, 'formatResponse', 'function', 'utils.js', 20); + + INSERT INTO edges (source_id, target_id, kind) VALUES (1, 2, 'calls'); + INSERT INTO edges (source_id, target_id, kind) VALUES (3, 1, 'calls'); + INSERT INTO edges (source_id, target_id, kind) VALUES (3, 4, 'calls'); + `); + }); + + afterEach(() => { + nativeDb.close(); + fs.rmSync(path.dirname(dbPath), { recursive: true, force: true }); + }); + + it('purges files AND deletes reverse-dep outgoing edges in one call', () => { + // Purge auth.js (changed file), delete outgoing edges for app.js (reverse-dep) + nativeDb.purgeFilesData(['auth.js'], false, ['app.js']); + + // auth.js nodes should be gone + const authNodes = nativeDb.queryAll("SELECT * FROM nodes WHERE file = 'auth.js'", []); + expect(authNodes).toHaveLength(0); + + // app.js nodes should still exist (only outgoing edges deleted) + const appNodes = nativeDb.queryAll("SELECT * FROM nodes WHERE file = 'app.js'", []); + expect(appNodes).toHaveLength(1); + + // utils.js nodes should still exist + const utilsNodes = nativeDb.queryAll("SELECT * FROM nodes WHERE file = 'utils.js'", []); + expect(utilsNodes).toHaveLength(2); + + // All edges should be gone: + // - auth.js edges removed by file purge (source_id=1 or target_id=1) + // - app.js outgoing edges removed by reverse-dep purge (source_id=3) + const edges = nativeDb.queryAll('SELECT * FROM edges', []); + expect(edges).toHaveLength(0); + }); + + it('only deletes outgoing edges for reverse-dep files, not incoming', () => { + // Add an incoming edge TO app.js from utils.js + nativeDb.exec(`INSERT INTO edges (source_id, target_id, kind) VALUES (4, 3, 'calls');`); + + // Purge nothing, just clean reverse-dep outgoing edges for app.js + nativeDb.purgeFilesData([], false, ['app.js']); + + // Outgoing edges from app.js (source_id=3) should be gone + const outgoing = nativeDb.queryAll('SELECT * FROM edges WHERE source_id = 3', []); + expect(outgoing).toHaveLength(0); + + // Incoming edge to app.js (target_id=3) should remain + const incoming = nativeDb.queryAll('SELECT * FROM edges WHERE target_id = 3', []); + expect(incoming).toHaveLength(1); + + // Edge within auth.js→utils.js should remain (not a reverse-dep) + const otherEdges = nativeDb.queryAll('SELECT * FROM edges WHERE source_id = 1', []); + expect(otherEdges).toHaveLength(1); + }); + + it('works with no reverse-dep files (backwards-compatible)', () => { + nativeDb.purgeFilesData(['auth.js'], false); + + const authNodes = nativeDb.queryAll("SELECT * FROM nodes WHERE file = 'auth.js'", []); + expect(authNodes).toHaveLength(0); + + // app.js edges should still exist (no reverse-dep cleanup requested) + const appEdges = nativeDb.queryAll('SELECT * FROM edges WHERE source_id = 3', []); + expect(appEdges).toHaveLength(1); // edge to utils.js:formatResponse remains + }); + + it('no-ops when both file list and reverse-dep list are empty', () => { + const before = nativeDb.queryAll('SELECT COUNT(*) as c FROM edges', []); + nativeDb.purgeFilesData([], false, []); + const after = nativeDb.queryAll('SELECT COUNT(*) as c FROM edges', []); + expect(after[0]!.c).toBe(before[0]!.c); + }); + + it('handles reverse-dep-only call (no files to purge)', () => { + nativeDb.purgeFilesData([], false, ['app.js']); + + // All nodes should remain + const nodes = nativeDb.queryAll('SELECT COUNT(*) as c FROM nodes', []); + expect(nodes[0]!.c).toBe(4); + + // Only app.js outgoing edges should be gone + const edges = nativeDb.queryAll('SELECT * FROM edges', []); + expect(edges).toHaveLength(1); // auth.js→utils.js remains + }); +});