Skip to content

Commit 94dd0f9

Browse files
committed
fix: address review feedback in incremental rebuild (#542)
- purgeAncillaryData: only catch "no such table" errors instead of swallowing all exceptions (P1 from Greptile) - Cache barrel resolution prepared statements to avoid re-preparing inside hot loops (P2 from Greptile) - Fix stale @param _db JSDoc tag - Prefix unused db param with underscore in rebuildDirContainment
1 parent 2b9fb3f commit 94dd0f9

1 file changed

Lines changed: 41 additions & 34 deletions

File tree

src/domain/graph/builder/incremental.js

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,7 @@ function insertFileNodes(stmts, relPath, symbols) {
2424
stmts.insertNode.run(def.name, def.kind, relPath, def.line, def.endLine || null);
2525
if (def.children?.length) {
2626
for (const child of def.children) {
27-
stmts.insertNode.run(
28-
child.name,
29-
child.kind,
30-
relPath,
31-
child.line,
32-
child.endLine || null,
33-
);
27+
stmts.insertNode.run(child.name, child.kind, relPath, child.line, child.endLine || null);
3428
}
3529
}
3630
}
@@ -128,7 +122,7 @@ function rebuildReverseDepEdges(db, rootDir, depRelPath, symbols, stmts, skipBar
128122

129123
// ── Directory containment edges ────────────────────────────────────────
130124

131-
function rebuildDirContainment(db, stmts, relPath) {
125+
function rebuildDirContainment(_db, stmts, relPath) {
132126
const dir = normalizePath(path.dirname(relPath));
133127
if (!dir || dir === '.') return 0;
134128
const dirRow = stmts.getNodeId.get(dir, 'directory', dir, 0);
@@ -146,8 +140,8 @@ function purgeAncillaryData(db, relPath) {
146140
const tryExec = (sql, ...args) => {
147141
try {
148142
db.prepare(sql).run(...args);
149-
} catch {
150-
/* table may not exist */
143+
} catch (err) {
144+
if (!err?.message?.includes('no such table')) throw err;
151145
}
152146
};
153147
tryExec(
@@ -176,38 +170,55 @@ function purgeAncillaryData(db, relPath) {
176170

177171
// ── Import edge building ────────────────────────────────────────────────
178172

179-
function isBarrelFile(db, relPath) {
180-
const reexportCount = db
181-
.prepare(
173+
// Lazily-cached prepared statements for barrel resolution (avoid re-preparing in hot loops)
174+
let _barrelDb = null;
175+
let _isBarrelStmt = null;
176+
let _reexportTargetsStmt = null;
177+
let _hasDefStmt = null;
178+
179+
function getBarrelStmts(db) {
180+
if (_barrelDb !== db) {
181+
_barrelDb = db;
182+
_isBarrelStmt = db.prepare(
182183
`SELECT COUNT(*) as c FROM edges e
183184
JOIN nodes n ON e.source_id = n.id
184185
WHERE e.kind = 'reexports' AND n.file = ? AND n.kind = 'file'`,
185-
)
186-
.get(relPath)?.c;
186+
);
187+
_reexportTargetsStmt = db.prepare(
188+
`SELECT DISTINCT n2.file FROM edges e
189+
JOIN nodes n1 ON e.source_id = n1.id
190+
JOIN nodes n2 ON e.target_id = n2.id
191+
WHERE e.kind = 'reexports' AND n1.file = ? AND n1.kind = 'file'`,
192+
);
193+
_hasDefStmt = db.prepare(
194+
`SELECT 1 FROM nodes WHERE name = ? AND file = ? AND kind != 'file' AND kind != 'directory' LIMIT 1`,
195+
);
196+
}
197+
return {
198+
isBarrelStmt: _isBarrelStmt,
199+
reexportTargetsStmt: _reexportTargetsStmt,
200+
hasDefStmt: _hasDefStmt,
201+
};
202+
}
203+
204+
function isBarrelFile(db, relPath) {
205+
const { isBarrelStmt } = getBarrelStmts(db);
206+
const reexportCount = isBarrelStmt.get(relPath)?.c;
187207
return (reexportCount || 0) > 0;
188208
}
189209

190210
function resolveBarrelTarget(db, barrelPath, symbolName, visited = new Set()) {
191211
if (visited.has(barrelPath)) return null;
192212
visited.add(barrelPath);
193213

214+
const { reexportTargetsStmt, hasDefStmt } = getBarrelStmts(db);
215+
194216
// Find re-export targets from this barrel
195-
const reexportTargets = db
196-
.prepare(
197-
`SELECT DISTINCT n2.file FROM edges e
198-
JOIN nodes n1 ON e.source_id = n1.id
199-
JOIN nodes n2 ON e.target_id = n2.id
200-
WHERE e.kind = 'reexports' AND n1.file = ? AND n1.kind = 'file'`,
201-
)
202-
.all(barrelPath);
217+
const reexportTargets = reexportTargetsStmt.all(barrelPath);
203218

204219
for (const { file: targetFile } of reexportTargets) {
205220
// Check if the symbol is defined in this target file
206-
const hasDef = db
207-
.prepare(
208-
`SELECT 1 FROM nodes WHERE name = ? AND file = ? AND kind != 'file' AND kind != 'directory' LIMIT 1`,
209-
)
210-
.get(symbolName, targetFile);
221+
const hasDef = hasDefStmt.get(symbolName, targetFile);
211222
if (hasDef) return targetFile;
212223

213224
// Recurse through barrel chains
@@ -357,7 +368,7 @@ function buildCallEdges(stmts, relPath, symbols, fileNodeRow, importedNames) {
357368
/**
358369
* Parse a single file and update the database incrementally.
359370
*
360-
* @param {import('better-sqlite3').Database} _db
371+
* @param {import('better-sqlite3').Database} db
361372
* @param {string} rootDir - Absolute root directory
362373
* @param {string} filePath - Absolute file path
363374
* @param {object} stmts - Prepared DB statements
@@ -456,11 +467,7 @@ export async function rebuildFile(db, rootDir, filePath, stmts, engineOpts, cach
456467
for (const name of imp.names) {
457468
const cleanName = name.replace(/^\*\s+as\s+/, '');
458469
const actualSource = resolveBarrelTarget(db, resolvedPath, cleanName);
459-
if (
460-
actualSource &&
461-
actualSource !== resolvedPath &&
462-
!resolvedSources.has(actualSource)
463-
) {
470+
if (actualSource && actualSource !== resolvedPath && !resolvedSources.has(actualSource)) {
464471
resolvedSources.add(actualSource);
465472
const actualRow = stmts.getNodeId.get(actualSource, 'file', actualSource, 0);
466473
if (actualRow) {

0 commit comments

Comments
 (0)