Skip to content

Commit 3db3861

Browse files
committed
fix: address Greptile review — deduplicate relatedTests, hoist prepared stmts, fix .raw() no-op
- queries.js: restore DISTINCT-by-file deduplication for relatedTests in explainFunctionImpl (lost when switching to findCallers) - build-stmts.js: prepare SQL statements once in preparePurgeStmts() and loop with runPurge() instead of calling db.prepare() per file per table - cfg.js: replace misleading .raw() with .get() in hasCfgTables Impact: 7 functions changed, 14 affected
1 parent 784bdf7 commit 3db3861

3 files changed

Lines changed: 77 additions & 63 deletions

File tree

src/db/repository/build-stmts.js

Lines changed: 74 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,82 +1,94 @@
1+
/**
2+
* Prepare all purge statements once, returning an object of runnable stmts.
3+
* Optional tables are wrapped in try/catch — if the table doesn't exist,
4+
* that slot is set to null.
5+
*
6+
* @param {object} db - Open read-write database handle
7+
* @returns {object} prepared statements (some may be null)
8+
*/
9+
function preparePurgeStmts(db) {
10+
const tryPrepare = (sql) => {
11+
try {
12+
return db.prepare(sql);
13+
} catch {
14+
return null;
15+
}
16+
};
17+
18+
return {
19+
embeddings: tryPrepare(
20+
'DELETE FROM embeddings WHERE node_id IN (SELECT id FROM nodes WHERE file = ?)',
21+
),
22+
cfgEdges: tryPrepare(
23+
'DELETE FROM cfg_edges WHERE function_node_id IN (SELECT id FROM nodes WHERE file = ?)',
24+
),
25+
cfgBlocks: tryPrepare(
26+
'DELETE FROM cfg_blocks WHERE function_node_id IN (SELECT id FROM nodes WHERE file = ?)',
27+
),
28+
dataflow: tryPrepare(
29+
'DELETE FROM dataflow WHERE source_id IN (SELECT id FROM nodes WHERE file = ?) OR target_id IN (SELECT id FROM nodes WHERE file = ?)',
30+
),
31+
complexity: tryPrepare(
32+
'DELETE FROM function_complexity WHERE node_id IN (SELECT id FROM nodes WHERE file = ?)',
33+
),
34+
nodeMetrics: tryPrepare(
35+
'DELETE FROM node_metrics WHERE node_id IN (SELECT id FROM nodes WHERE file = ?)',
36+
),
37+
astNodes: tryPrepare('DELETE FROM ast_nodes WHERE file = ?'),
38+
// Core tables — always exist
39+
edges: db.prepare(
40+
'DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = @f) OR target_id IN (SELECT id FROM nodes WHERE file = @f)',
41+
),
42+
nodes: db.prepare('DELETE FROM nodes WHERE file = ?'),
43+
fileHashes: tryPrepare('DELETE FROM file_hashes WHERE file = ?'),
44+
};
45+
}
46+
147
/**
248
* Cascade-delete all graph data for a single file across all tables.
349
* Order: dependent tables first, then edges, then nodes, then hashes.
4-
* Tables that may not exist are wrapped in try/catch.
550
*
651
* @param {object} db - Open read-write database handle
752
* @param {string} file - Relative file path to purge
853
* @param {object} [opts]
954
* @param {boolean} [opts.purgeHashes=true] - Also delete file_hashes entry
1055
*/
1156
export function purgeFileData(db, file, opts = {}) {
57+
const stmts = preparePurgeStmts(db);
58+
runPurge(stmts, file, opts);
59+
}
60+
61+
/**
62+
* Run purge using pre-prepared statements for a single file.
63+
* @param {object} stmts - Prepared statements from preparePurgeStmts
64+
* @param {string} file - Relative file path to purge
65+
* @param {object} [opts]
66+
* @param {boolean} [opts.purgeHashes=true]
67+
*/
68+
function runPurge(stmts, file, opts = {}) {
1269
const { purgeHashes = true } = opts;
1370

14-
// Optional tables — may not exist in older DBs
15-
try {
16-
db.prepare('DELETE FROM embeddings WHERE node_id IN (SELECT id FROM nodes WHERE file = ?)').run(
17-
file,
18-
);
19-
} catch {
20-
/* table may not exist */
21-
}
22-
try {
23-
db.prepare(
24-
'DELETE FROM cfg_edges WHERE function_node_id IN (SELECT id FROM nodes WHERE file = ?)',
25-
).run(file);
26-
} catch {
27-
/* table may not exist */
28-
}
29-
try {
30-
db.prepare(
31-
'DELETE FROM cfg_blocks WHERE function_node_id IN (SELECT id FROM nodes WHERE file = ?)',
32-
).run(file);
33-
} catch {
34-
/* table may not exist */
35-
}
36-
try {
37-
db.prepare(
38-
'DELETE FROM dataflow WHERE source_id IN (SELECT id FROM nodes WHERE file = ?) OR target_id IN (SELECT id FROM nodes WHERE file = ?)',
39-
).run(file, file);
40-
} catch {
41-
/* table may not exist */
42-
}
43-
try {
44-
db.prepare(
45-
'DELETE FROM function_complexity WHERE node_id IN (SELECT id FROM nodes WHERE file = ?)',
46-
).run(file);
47-
} catch {
48-
/* table may not exist */
49-
}
50-
try {
51-
db.prepare(
52-
'DELETE FROM node_metrics WHERE node_id IN (SELECT id FROM nodes WHERE file = ?)',
53-
).run(file);
54-
} catch {
55-
/* table may not exist */
56-
}
57-
try {
58-
db.prepare('DELETE FROM ast_nodes WHERE file = ?').run(file);
59-
} catch {
60-
/* table may not exist */
61-
}
71+
// Optional tables
72+
stmts.embeddings?.run(file);
73+
stmts.cfgEdges?.run(file);
74+
stmts.cfgBlocks?.run(file);
75+
stmts.dataflow?.run(file, file);
76+
stmts.complexity?.run(file);
77+
stmts.nodeMetrics?.run(file);
78+
stmts.astNodes?.run(file);
6279

63-
// Core tables — always exist
64-
db.prepare(
65-
'DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = @f) OR target_id IN (SELECT id FROM nodes WHERE file = @f)',
66-
).run({ f: file });
67-
db.prepare('DELETE FROM nodes WHERE file = ?').run(file);
80+
// Core tables
81+
stmts.edges.run({ f: file });
82+
stmts.nodes.run(file);
6883

6984
if (purgeHashes) {
70-
try {
71-
db.prepare('DELETE FROM file_hashes WHERE file = ?').run(file);
72-
} catch {
73-
/* table may not exist */
74-
}
85+
stmts.fileHashes?.run(file);
7586
}
7687
}
7788

7889
/**
79-
* Purge all graph data for multiple files (transactional).
90+
* Purge all graph data for multiple files.
91+
* Prepares statements once and loops over files for efficiency.
8092
*
8193
* @param {object} db - Open read-write database handle
8294
* @param {string[]} files - Relative file paths to purge
@@ -85,7 +97,8 @@ export function purgeFileData(db, file, opts = {}) {
8597
*/
8698
export function purgeFilesData(db, files, opts = {}) {
8799
if (!files || files.length === 0) return;
100+
const stmts = preparePurgeStmts(db);
88101
for (const file of files) {
89-
purgeFileData(db, file, opts);
102+
runPurge(stmts, file, opts);
90103
}
91104
}

src/db/repository/cfg.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66
export function hasCfgTables(db) {
77
try {
8-
db.prepare('SELECT 1 FROM cfg_blocks LIMIT 0').raw();
8+
db.prepare('SELECT 1 FROM cfg_blocks LIMIT 0').get();
99
return true;
1010
} catch {
1111
return false;

src/queries.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1898,8 +1898,9 @@ function explainFunctionImpl(db, target, noTests, getFileLines) {
18981898
if (noTests) callers = callers.filter((c) => !isTestFile(c.file));
18991899

19001900
const testCallerRows = findCallers(db, node.id);
1901+
const seenFiles = new Set();
19011902
const relatedTests = testCallerRows
1902-
.filter((r) => isTestFile(r.file))
1903+
.filter((r) => isTestFile(r.file) && !seenFiles.has(r.file) && seenFiles.add(r.file))
19031904
.map((r) => ({ file: r.file }));
19041905

19051906
// Complexity metrics

0 commit comments

Comments
 (0)