Skip to content

fix: hoist prepared statement out of BFS loop in getClassHierarchy #403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlos-alm merged 23 commits into from
Mar 11, 2026
+144 −41
Merged

fix: hoist prepared statement out of BFS loop in getClassHierarchy #403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
322b527
feat: unified AST analysis framework with pluggable visitor pattern
carlos-alm Mar 9, 2026
e4a5ac0
merge: resolve BACKLOG.md conflict, renumber dynamic import item to I…
carlos-alm Mar 9, 2026
7480476
revert: restore original check-dead-exports.sh hook
carlos-alm Mar 9, 2026
cd7f0e1
fix: address Greptile review — indexOf→m.index, precise publicAPI reg…
carlos-alm Mar 9, 2026
445a04a
Merge branch 'main' into feat/unified-ast-analysis-framework
carlos-alm Mar 9, 2026
c196c8e
fix: address Greptile review — halsteadSkip depth counter, debug logg…
carlos-alm Mar 9, 2026
89344de
Merge remote-tracking branch 'origin/main' into feat/unified-ast-anal…
carlos-alm Mar 9, 2026
a47eb47
fix: remove function nodes from nestingNodeTypes and eliminate O(n²) …
carlos-alm Mar 9, 2026
77b4516
fix: remove function nesting inflation in computeAllMetrics and pass …
carlos-alm Mar 9, 2026
a84b7e4
fix: guard runAnalyses call, fix nested function nesting, rename _eng…
carlos-alm Mar 9, 2026
6d0f34e
fix: remove redundant processed Set and fix multi-line destructuring …
carlos-alm Mar 9, 2026
784bdf7
refactor: migrate raw SQL into repository pattern (Phase 3.3)
carlos-alm Mar 10, 2026
3db3861
fix: address Greptile review — deduplicate relatedTests, hoist prepar…
carlos-alm Mar 10, 2026
d9085b9
Merge remote-tracking branch 'origin/main' into feat/unified-ast-anal…
carlos-alm Mar 11, 2026
d62f0bc
fix: hoist prepared statement out of BFS loop in getClassHierarchy
carlos-alm Mar 11, 2026
f2339f1
Merge remote-tracking branch 'origin/main' into fix/hoist-class-hiera…
carlos-alm Mar 11, 2026
8f046d9
fix: re-apply hoisted prepared statement after merge resolution
carlos-alm Mar 11, 2026
23ae531
Merge branch 'main' into fix/hoist-class-hierarchy-stmt
carlos-alm Mar 11, 2026
cf8d292
fix: cache all hot-path prepared statements in edge repository
carlos-alm Mar 11, 2026
4a814f3
Merge branch 'fix/hoist-class-hierarchy-stmt' of https://github.com/o…
carlos-alm Mar 11, 2026
9e7922b
Merge branch 'main' into fix/hoist-class-hierarchy-stmt
carlos-alm Mar 11, 2026
f18859a
style: fix biome formatting in repository test
carlos-alm Mar 11, 2026
d84e429
Merge branch 'main' into fix/hoist-class-hierarchy-stmt
carlos-alm Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 63 additions & 41 deletions src/db/repository/edges.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,22 @@
// ─── Prepared-statement caches (one per db instance) ────────────────────
// WeakMap keys on the db object so statements are GC'd when the db closes.
const _findCalleesStmt = new WeakMap();
const _findCallersStmt = new WeakMap();
const _findDistinctCallersStmt = new WeakMap();
const _findCalleeNamesStmt = new WeakMap();
const _findCallerNamesStmt = new WeakMap();
const _getClassAncestorsStmt = new WeakMap();

/** Resolve a cached prepared statement, compiling on first use per db. */
function _cached(cache, db, sql) {
let stmt = cache.get(db);
if (!stmt) {
stmt = db.prepare(sql);
cache.set(db, stmt);
}
return stmt;
}
Comment on lines +11 to +18
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sql argument silently ignored after first compilation

_cached() accepts a sql parameter on every call but only uses it during the first compile for a given db. After that, the cached statement is returned regardless of what sql is passed. In the current codebase this is fine because each WeakMap is exclusively paired with one SQL string, but if someone later adds a second call site that passes a different SQL string to the same WeakMap, they'll get the first statement back with no error or warning.

Consider asserting that the sql argument matches in dev/test, or documenting the contract explicitly:

Suggested change
function _cached(cache, db, sql) {
let stmt = cache.get(db);
if (!stmt) {
stmt = db.prepare(sql);
cache.set(db, stmt);
}
return stmt;
}
/** Resolve a cached prepared statement, compiling on first use per db.
* IMPORTANT: each `cache` WeakMap must always be called with the same `sql`.
* The `sql` argument is only used on the first compile; subsequent calls ignore it.
*/
function _cached(cache, db, sql) {
let stmt = cache.get(db);
if (!stmt) {
stmt = db.prepare(sql);
cache.set(db, stmt);
}
return stmt;
}


// ─── Call-edge queries ──────────────────────────────────────────────────

/**
Expand All @@ -8,13 +27,13 @@
* @returns {{ id: number, name: string, kind: string, file: string, line: number, end_line: number|null }[]}
*/
export function findCallees(db, nodeId) {
return db
.prepare(
`SELECT DISTINCT n.id, n.name, n.kind, n.file, n.line, n.end_line
FROM edges e JOIN nodes n ON e.target_id = n.id
WHERE e.source_id = ? AND e.kind = 'calls'`,
)
.all(nodeId);
return _cached(
_findCalleesStmt,
db,
`SELECT DISTINCT n.id, n.name, n.kind, n.file, n.line, n.end_line
FROM edges e JOIN nodes n ON e.target_id = n.id
WHERE e.source_id = ? AND e.kind = 'calls'`,
).all(nodeId);
}

/**
Expand All @@ -24,13 +43,13 @@ export function findCallees(db, nodeId) {
* @returns {{ id: number, name: string, kind: string, file: string, line: number }[]}
*/
export function findCallers(db, nodeId) {
return db
.prepare(
`SELECT n.id, n.name, n.kind, n.file, n.line
FROM edges e JOIN nodes n ON e.source_id = n.id
WHERE e.target_id = ? AND e.kind = 'calls'`,
)
.all(nodeId);
return _cached(
_findCallersStmt,
db,
`SELECT n.id, n.name, n.kind, n.file, n.line
FROM edges e JOIN nodes n ON e.source_id = n.id
WHERE e.target_id = ? AND e.kind = 'calls'`,
).all(nodeId);
}

/**
Expand All @@ -40,13 +59,13 @@ export function findCallers(db, nodeId) {
* @returns {{ id: number, name: string, kind: string, file: string, line: number }[]}
*/
export function findDistinctCallers(db, nodeId) {
return db
.prepare(
`SELECT DISTINCT n.id, n.name, n.kind, n.file, n.line
FROM edges e JOIN nodes n ON e.source_id = n.id
WHERE e.target_id = ? AND e.kind = 'calls'`,
)
.all(nodeId);
return _cached(
_findDistinctCallersStmt,
db,
`SELECT DISTINCT n.id, n.name, n.kind, n.file, n.line
FROM edges e JOIN nodes n ON e.source_id = n.id
WHERE e.target_id = ? AND e.kind = 'calls'`,
).all(nodeId);
}

// ─── All-edge queries (no kind filter) ─────────────────────────────────
Expand Down Expand Up @@ -92,13 +111,14 @@ export function findAllIncomingEdges(db, nodeId) {
* @returns {string[]}
*/
export function findCalleeNames(db, nodeId) {
return db
.prepare(
`SELECT DISTINCT n.name
FROM edges e JOIN nodes n ON e.target_id = n.id
WHERE e.source_id = ? AND e.kind = 'calls'
ORDER BY n.name`,
)
return _cached(
_findCalleeNamesStmt,
db,
`SELECT DISTINCT n.name
FROM edges e JOIN nodes n ON e.target_id = n.id
WHERE e.source_id = ? AND e.kind = 'calls'
ORDER BY n.name`,
)
.all(nodeId)
.map((r) => r.name);
}
Expand All @@ -110,13 +130,14 @@ export function findCalleeNames(db, nodeId) {
* @returns {string[]}
*/
export function findCallerNames(db, nodeId) {
return db
.prepare(
`SELECT DISTINCT n.name
FROM edges e JOIN nodes n ON e.source_id = n.id
WHERE e.target_id = ? AND e.kind = 'calls'
ORDER BY n.name`,
)
return _cached(
_findCallerNamesStmt,
db,
`SELECT DISTINCT n.name
FROM edges e JOIN nodes n ON e.source_id = n.id
WHERE e.target_id = ? AND e.kind = 'calls'
ORDER BY n.name`,
)
.all(nodeId)
.map((r) => r.name);
}
Expand Down Expand Up @@ -218,16 +239,17 @@ export function countCrossFileCallers(db, nodeId, file) {
* @returns {Set<number>}
*/
export function getClassHierarchy(db, classNodeId) {
const stmt = _cached(
_getClassAncestorsStmt,
db,
`SELECT n.id, n.name FROM edges e JOIN nodes n ON e.target_id = n.id
WHERE e.source_id = ? AND e.kind = 'extends'`,
);
const ancestors = new Set();
const queue = [classNodeId];
while (queue.length > 0) {
const current = queue.shift();
const parents = db
.prepare(
`SELECT n.id, n.name FROM edges e JOIN nodes n ON e.target_id = n.id
WHERE e.source_id = ? AND e.kind = 'extends'`,
)
.all(current);
const parents = stmt.all(current);
for (const p of parents) {
if (!ancestors.has(p.id)) {
ancestors.add(p.id);
Expand Down
81 changes: 81 additions & 0 deletions tests/unit/repository.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Database from 'better-sqlite3';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { initSchema } from '../../src/db/migrations.js';
import { getClassHierarchy } from '../../src/db/repository/edges.js';
import {
countEdges,
countFiles,
Expand Down Expand Up @@ -163,6 +164,86 @@ describe('repository', () => {
});
});

describe('getClassHierarchy', () => {
it('returns empty set for node with no extends edges', () => {
const fooId = db.prepare("SELECT id FROM nodes WHERE name = 'foo'").get().id;
const ancestors = getClassHierarchy(db, fooId);
expect(ancestors.size).toBe(0);
});

it('resolves single-level class hierarchy', () => {
// Create Parent class and extends edge: Baz -> Parent
const insertNode = db.prepare(
'INSERT INTO nodes (name, kind, file, line, role) VALUES (?, ?, ?, ?, ?)',
);
insertNode.run('Parent', 'class', 'src/parent.js', 1, null);

const bazId = db.prepare("SELECT id FROM nodes WHERE name = 'Baz'").get().id;
const parentId = db.prepare("SELECT id FROM nodes WHERE name = 'Parent'").get().id;
db.prepare('INSERT INTO edges (source_id, target_id, kind) VALUES (?, ?, ?)').run(
bazId,
parentId,
'extends',
);

const ancestors = getClassHierarchy(db, bazId);
expect(ancestors.size).toBe(1);
expect(ancestors.has(parentId)).toBe(true);
});

it('resolves multi-level class hierarchy', () => {
const insertNode = db.prepare(
'INSERT INTO nodes (name, kind, file, line, role) VALUES (?, ?, ?, ?, ?)',
);
insertNode.run('Parent', 'class', 'src/parent.js', 1, null);
insertNode.run('Grandparent', 'class', 'src/grandparent.js', 1, null);

const bazId = db.prepare("SELECT id FROM nodes WHERE name = 'Baz'").get().id;
const parentId = db.prepare("SELECT id FROM nodes WHERE name = 'Parent'").get().id;
const grandparentId = db.prepare("SELECT id FROM nodes WHERE name = 'Grandparent'").get().id;

const insertEdge = db.prepare(
'INSERT INTO edges (source_id, target_id, kind) VALUES (?, ?, ?)',
);
insertEdge.run(bazId, parentId, 'extends');
insertEdge.run(parentId, grandparentId, 'extends');

const ancestors = getClassHierarchy(db, bazId);
expect(ancestors.size).toBe(2);
expect(ancestors.has(parentId)).toBe(true);
expect(ancestors.has(grandparentId)).toBe(true);
});

it('handles diamond inheritance without infinite loops', () => {
const insertNode = db.prepare(
'INSERT INTO nodes (name, kind, file, line, role) VALUES (?, ?, ?, ?, ?)',
);
insertNode.run('A', 'class', 'src/a.js', 1, null);
insertNode.run('B', 'class', 'src/b.js', 1, null);
insertNode.run('Top', 'class', 'src/top.js', 1, null);

const bazId = db.prepare("SELECT id FROM nodes WHERE name = 'Baz'").get().id;
const aId = db.prepare("SELECT id FROM nodes WHERE name = 'A'").get().id;
const bId = db.prepare("SELECT id FROM nodes WHERE name = 'B'").get().id;
const topId = db.prepare("SELECT id FROM nodes WHERE name = 'Top'").get().id;

const insertEdge = db.prepare(
'INSERT INTO edges (source_id, target_id, kind) VALUES (?, ?, ?)',
);
// Baz -> A, Baz -> B, A -> Top, B -> Top
insertEdge.run(bazId, aId, 'extends');
insertEdge.run(bazId, bId, 'extends');
insertEdge.run(aId, topId, 'extends');
insertEdge.run(bId, topId, 'extends');

const ancestors = getClassHierarchy(db, bazId);
expect(ancestors.size).toBe(3); // A, B, Top
expect(ancestors.has(aId)).toBe(true);
expect(ancestors.has(bId)).toBe(true);
expect(ancestors.has(topId)).toBe(true);
});
});

describe('countNodes / countEdges / countFiles', () => {
it('countNodes returns total', () => {
expect(countNodes(db)).toBe(5);
Expand Down
Loading