Skip to content

Commit f0770f5

Browse files
authored
refactor: deduplicate BFS impact traversal and centralize config loading (#463)
* refactor: deduplicate BFS impact traversal and centralize config loading Extract shared bfsTransitiveCallers() into domain/analysis/impact.js, replacing 4 near-identical BFS loops across audit, check, and impact modules. The onVisit callback handles site-specific tracking (edges, affected sets) without duplicating the core traversal. Make feature functions (audit, check, manifesto, complexity, diff-impact) accept opts.config to reuse the config already loaded at CLI entry, falling back to loadConfig() for programmatic API callers. Impact: 16 functions changed, 37 affected * fix: filter minConfidence for Repository path and simplify alias Impact: 7 functions changed, 3 affected * test: add InMemoryRepository tests for dependency builder and document minConfidence trade-off Impact: 1 functions changed, 11 affected * docs: tighten bfsTransitiveCallers db param type to better-sqlite3 Database * docs: document opts.config cwd vs db-root trade-off in checkData Impact: 1 functions changed, 2 affected
1 parent f77a338 commit f0770f5

14 files changed

Lines changed: 175 additions & 124 deletions

File tree

src/cli/commands/audit.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export const command = {
4141
kind: opts.kind,
4242
noTests: ctx.resolveNoTests(opts),
4343
json: opts.json,
44+
config: ctx.config,
4445
});
4546
},
4647
};

src/cli/commands/check.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export const command = {
4141
limit: opts.limit ? parseInt(opts.limit, 10) : undefined,
4242
offset: opts.offset ? parseInt(opts.offset, 10) : undefined,
4343
ndjson: opts.ndjson,
44+
config: ctx.config,
4445
});
4546
return;
4647
}
@@ -56,6 +57,7 @@ export const command = {
5657
depth: opts.depth ? parseInt(opts.depth, 10) : undefined,
5758
noTests: ctx.resolveNoTests(opts),
5859
json: opts.json,
60+
config: ctx.config,
5961
});
6062

6163
if (opts.rules) {
@@ -73,6 +75,7 @@ export const command = {
7375
limit: opts.limit ? parseInt(opts.limit, 10) : undefined,
7476
offset: opts.offset ? parseInt(opts.offset, 10) : undefined,
7577
ndjson: opts.ndjson,
78+
config: ctx.config,
7679
});
7780
}
7881
},

src/cli/commands/complexity.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export const command = {
4040
noTests: ctx.resolveNoTests(opts),
4141
json: opts.json,
4242
ndjson: opts.ndjson,
43+
config: ctx.config,
4344
});
4445
},
4546
};

src/cli/commands/diff-impact.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export const command = {
2525
limit: opts.limit ? parseInt(opts.limit, 10) : undefined,
2626
offset: opts.offset ? parseInt(opts.offset, 10) : undefined,
2727
ndjson: opts.ndjson,
28+
config: ctx.config,
2829
});
2930
},
3031
};

src/db/repository/base.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ export class Repository {
163163
throw new Error('not implemented');
164164
}
165165

166-
/** @returns {{ source_id: number, target_id: number }[]} */
166+
/** @returns {{ source_id: number, target_id: number, confidence: number|null }[]} */
167167
getCallEdges() {
168168
throw new Error('not implemented');
169169
}

src/db/repository/graph-read.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ export function getCallableNodes(db) {
2525
/**
2626
* Get all 'calls' edges.
2727
* @param {object} db
28-
* @returns {{ source_id: number, target_id: number }[]}
28+
* @returns {{ source_id: number, target_id: number, confidence: number|null }[]}
2929
*/
3030
export function getCallEdges(db) {
3131
return cachedStmt(
3232
_getCallEdgesStmt,
3333
db,
34-
"SELECT source_id, target_id FROM edges WHERE kind = 'calls'",
34+
"SELECT source_id, target_id, confidence FROM edges WHERE kind = 'calls'",
3535
).all();
3636
}
3737

src/db/repository/in-memory-repository.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ export class InMemoryRepository extends Repository {
489489
getCallEdges() {
490490
return [...this.#edges.values()]
491491
.filter((e) => e.kind === 'calls')
492-
.map((e) => ({ source_id: e.source_id, target_id: e.target_id }));
492+
.map((e) => ({ source_id: e.source_id, target_id: e.target_id, confidence: e.confidence }));
493493
}
494494

495495
getFileNodesAll() {

src/domain/analysis/impact.js

Lines changed: 53 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,42 @@ import { normalizeSymbol } from '../../shared/normalize.js';
1818
import { paginateResult } from '../../shared/paginate.js';
1919
import { findMatchingNodes } from './symbol-lookup.js';
2020

21+
// ─── Shared BFS: transitive callers ────────────────────────────────────
22+
23+
/**
24+
* BFS traversal to find transitive callers of a node.
25+
*
26+
* @param {import('better-sqlite3').Database} db - Open read-only SQLite database handle (not a Repository)
27+
* @param {number} startId - Starting node ID
28+
* @param {{ noTests?: boolean, maxDepth?: number, onVisit?: (caller: object, parentId: number, depth: number) => void }} options
29+
* @returns {{ totalDependents: number, levels: Record<number, Array<{name:string, kind:string, file:string, line:number}>> }}
30+
*/
31+
export function bfsTransitiveCallers(db, startId, { noTests = false, maxDepth = 3, onVisit } = {}) {
32+
const visited = new Set([startId]);
33+
const levels = {};
34+
let frontier = [startId];
35+
36+
for (let d = 1; d <= maxDepth; d++) {
37+
const nextFrontier = [];
38+
for (const fid of frontier) {
39+
const callers = findDistinctCallers(db, fid);
40+
for (const c of callers) {
41+
if (!visited.has(c.id) && (!noTests || !isTestFile(c.file))) {
42+
visited.add(c.id);
43+
nextFrontier.push(c.id);
44+
if (!levels[d]) levels[d] = [];
45+
levels[d].push({ name: c.name, kind: c.kind, file: c.file, line: c.line });
46+
if (onVisit) onVisit(c, fid, d);
47+
}
48+
}
49+
}
50+
frontier = nextFrontier;
51+
if (frontier.length === 0) break;
52+
}
53+
54+
return { totalDependents: visited.size - 1, levels };
55+
}
56+
2157
export function impactAnalysisData(file, customDbPath, opts = {}) {
2258
const db = openReadonlyOrFail(customDbPath);
2359
try {
@@ -82,31 +118,11 @@ export function fnImpactData(name, customDbPath, opts = {}) {
82118
}
83119

84120
const results = nodes.map((node) => {
85-
const visited = new Set([node.id]);
86-
const levels = {};
87-
let frontier = [node.id];
88-
89-
for (let d = 1; d <= maxDepth; d++) {
90-
const nextFrontier = [];
91-
for (const fid of frontier) {
92-
const callers = findDistinctCallers(db, fid);
93-
for (const c of callers) {
94-
if (!visited.has(c.id) && (!noTests || !isTestFile(c.file))) {
95-
visited.add(c.id);
96-
nextFrontier.push(c.id);
97-
if (!levels[d]) levels[d] = [];
98-
levels[d].push({ name: c.name, kind: c.kind, file: c.file, line: c.line });
99-
}
100-
}
101-
}
102-
frontier = nextFrontier;
103-
if (frontier.length === 0) break;
104-
}
105-
121+
const { levels, totalDependents } = bfsTransitiveCallers(db, node.id, { noTests, maxDepth });
106122
return {
107123
...normalizeSymbol(node, db, hc),
108124
levels,
109-
totalDependents: visited.size - 1,
125+
totalDependents,
110126
};
111127
});
112128

@@ -232,40 +248,27 @@ export function diffImpactData(customDbPath, opts = {}) {
232248

233249
const allAffected = new Set();
234250
const functionResults = affectedFunctions.map((fn) => {
235-
const visited = new Set([fn.id]);
236-
let frontier = [fn.id];
237-
let totalCallers = 0;
238-
const levels = {};
239251
const edges = [];
240252
const idToKey = new Map();
241253
idToKey.set(fn.id, `${fn.file}::${fn.name}:${fn.line}`);
242-
for (let d = 1; d <= maxDepth; d++) {
243-
const nextFrontier = [];
244-
for (const fid of frontier) {
245-
const callers = findDistinctCallers(db, fid);
246-
for (const c of callers) {
247-
if (!visited.has(c.id) && (!noTests || !isTestFile(c.file))) {
248-
visited.add(c.id);
249-
nextFrontier.push(c.id);
250-
allAffected.add(`${c.file}:${c.name}`);
251-
const callerKey = `${c.file}::${c.name}:${c.line}`;
252-
idToKey.set(c.id, callerKey);
253-
if (!levels[d]) levels[d] = [];
254-
levels[d].push({ name: c.name, kind: c.kind, file: c.file, line: c.line });
255-
edges.push({ from: idToKey.get(fid), to: callerKey });
256-
totalCallers++;
257-
}
258-
}
259-
}
260-
frontier = nextFrontier;
261-
if (frontier.length === 0) break;
262-
}
254+
255+
const { levels, totalDependents } = bfsTransitiveCallers(db, fn.id, {
256+
noTests,
257+
maxDepth,
258+
onVisit(c, parentId) {
259+
allAffected.add(`${c.file}:${c.name}`);
260+
const callerKey = `${c.file}::${c.name}:${c.line}`;
261+
idToKey.set(c.id, callerKey);
262+
edges.push({ from: idToKey.get(parentId), to: callerKey });
263+
},
264+
});
265+
263266
return {
264267
name: fn.name,
265268
kind: fn.kind,
266269
file: fn.file,
267270
line: fn.line,
268-
transitiveCallers: totalCallers,
271+
transitiveCallers: totalDependents,
269272
levels,
270273
edges,
271274
};
@@ -310,8 +313,8 @@ export function diffImpactData(customDbPath, opts = {}) {
310313
let boundaryViolations = [];
311314
let boundaryViolationCount = 0;
312315
try {
313-
const config = loadConfig(repoRoot);
314-
const boundaryConfig = config.manifesto?.boundaries;
316+
const cfg = opts.config || loadConfig(repoRoot);
317+
const boundaryConfig = cfg.manifesto?.boundaries;
315318
if (boundaryConfig) {
316319
const result = evaluateBoundaries(db, boundaryConfig, {
317320
scopeFiles: [...changedRanges.keys()],

src/features/audit.js

Lines changed: 12 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import path from 'node:path';
1010
import { openReadonlyOrFail } from '../db/index.js';
11+
import { bfsTransitiveCallers } from '../domain/analysis/impact.js';
1112
import { explainData } from '../domain/queries.js';
1213
import { loadConfig } from '../infrastructure/config.js';
1314
import { isTestFile } from '../infrastructure/test-filter.js';
@@ -17,11 +18,15 @@ import { RULE_DEFS } from './manifesto.js';
1718

1819
const FUNCTION_RULES = RULE_DEFS.filter((d) => d.level === 'function');
1920

20-
function resolveThresholds(customDbPath) {
21+
function resolveThresholds(customDbPath, config) {
2122
try {
22-
const dbDir = path.dirname(customDbPath);
23-
const repoRoot = path.resolve(dbDir, '..');
24-
const cfg = loadConfig(repoRoot);
23+
const cfg =
24+
config ||
25+
(() => {
26+
const dbDir = path.dirname(customDbPath);
27+
const repoRoot = path.resolve(dbDir, '..');
28+
return loadConfig(repoRoot);
29+
})();
2530
const userRules = cfg.manifesto || {};
2631
const resolved = {};
2732
for (const def of FUNCTION_RULES) {
@@ -70,39 +75,6 @@ function checkBreaches(row, thresholds) {
7075
return breaches;
7176
}
7277

73-
// ─── BFS impact (inline, same algorithm as fnImpactData) ────────────
74-
75-
function computeImpact(db, nodeId, noTests, maxDepth) {
76-
const visited = new Set([nodeId]);
77-
const levels = {};
78-
let frontier = [nodeId];
79-
80-
for (let d = 1; d <= maxDepth; d++) {
81-
const nextFrontier = [];
82-
for (const fid of frontier) {
83-
const callers = db
84-
.prepare(
85-
`SELECT DISTINCT n.id, n.name, n.kind, n.file, n.line
86-
FROM edges e JOIN nodes n ON e.source_id = n.id
87-
WHERE e.target_id = ? AND e.kind = 'calls'`,
88-
)
89-
.all(fid);
90-
for (const c of callers) {
91-
if (!visited.has(c.id) && (!noTests || !isTestFile(c.file))) {
92-
visited.add(c.id);
93-
nextFrontier.push(c.id);
94-
if (!levels[d]) levels[d] = [];
95-
levels[d].push({ name: c.name, kind: c.kind, file: c.file, line: c.line });
96-
}
97-
}
98-
}
99-
frontier = nextFrontier;
100-
if (frontier.length === 0) break;
101-
}
102-
103-
return { totalDependents: visited.size - 1, levels };
104-
}
105-
10678
// ─── Phase 4.4 fields (graceful null fallback) ─────────────────────
10779

10880
function readPhase44(db, nodeId) {
@@ -147,7 +119,7 @@ export function auditData(target, customDbPath, opts = {}) {
147119

148120
// 2. Open DB for enrichment
149121
const db = openReadonlyOrFail(customDbPath);
150-
const thresholds = resolveThresholds(customDbPath);
122+
const thresholds = resolveThresholds(customDbPath, opts.config);
151123

152124
let functions;
153125
try {
@@ -189,7 +161,7 @@ function enrichFunction(db, r, noTests, maxDepth, thresholds) {
189161
const nodeId = nodeRow?.id;
190162
const health = nodeId ? buildHealth(db, nodeId, thresholds) : defaultHealth();
191163
const impact = nodeId
192-
? computeImpact(db, nodeId, noTests, maxDepth)
164+
? bfsTransitiveCallers(db, nodeId, { noTests, maxDepth })
193165
: { totalDependents: 0, levels: {} };
194166
const phase44 = nodeId
195167
? readPhase44(db, nodeId)
@@ -260,7 +232,7 @@ function enrichSymbol(db, sym, file, noTests, maxDepth, thresholds) {
260232

261233
const health = nodeId ? buildHealth(db, nodeId, thresholds) : defaultHealth();
262234
const impact = nodeId
263-
? computeImpact(db, nodeId, noTests, maxDepth)
235+
? bfsTransitiveCallers(db, nodeId, { noTests, maxDepth })
264236
: { totalDependents: 0, levels: {} };
265237
const phase44 = nodeId
266238
? readPhase44(db, nodeId)

src/features/check.js

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { execFileSync } from 'node:child_process';
22
import fs from 'node:fs';
33
import path from 'node:path';
44
import { findDbPath, openReadonlyOrFail } from '../db/index.js';
5+
import { bfsTransitiveCallers } from '../domain/analysis/impact.js';
56
import { findCycles } from '../domain/graph/cycles.js';
67
import { loadConfig } from '../infrastructure/config.js';
78
import { isTestFile } from '../infrastructure/test-filter.js';
@@ -96,31 +97,10 @@ export function checkMaxBlastRadius(db, changedRanges, threshold, noTests, maxDe
9697
}
9798
if (!overlaps) continue;
9899

99-
// BFS transitive callers
100-
const visited = new Set([def.id]);
101-
let frontier = [def.id];
102-
let totalCallers = 0;
103-
for (let d = 1; d <= maxDepth; d++) {
104-
const nextFrontier = [];
105-
for (const fid of frontier) {
106-
const callers = db
107-
.prepare(
108-
`SELECT DISTINCT n.id, n.name, n.kind, n.file, n.line
109-
FROM edges e JOIN nodes n ON e.source_id = n.id
110-
WHERE e.target_id = ? AND e.kind = 'calls'`,
111-
)
112-
.all(fid);
113-
for (const c of callers) {
114-
if (!visited.has(c.id) && (!noTests || !isTestFile(c.file))) {
115-
visited.add(c.id);
116-
nextFrontier.push(c.id);
117-
totalCallers++;
118-
}
119-
}
120-
}
121-
frontier = nextFrontier;
122-
if (frontier.length === 0) break;
123-
}
100+
const { totalDependents: totalCallers } = bfsTransitiveCallers(db, def.id, {
101+
noTests,
102+
maxDepth,
103+
});
124104

125105
if (totalCallers > maxFound) maxFound = totalCallers;
126106
if (totalCallers > threshold) {
@@ -240,7 +220,10 @@ export function checkData(customDbPath, opts = {}) {
240220
const maxDepth = opts.depth || 3;
241221

242222
// Load config defaults for check predicates
243-
const config = loadConfig(repoRoot);
223+
// NOTE: opts.config is loaded from process.cwd() at startup (via CLI context),
224+
// which may differ from the DB's parent repo root when --db points to an external
225+
// project. This is an acceptable trade-off to avoid duplicate I/O on the hot path.
226+
const config = opts.config || loadConfig(repoRoot);
244227
const checkConfig = config.check || {};
245228

246229
// Resolve which predicates are enabled: CLI flags ?? config ?? built-in defaults

0 commit comments

Comments
 (0)