Skip to content

Commit 0a07066

Browse files
carlos-almcarlos-alm
committed
fix(scripts): resolve benchmark hub targets to a single, kind-filtered node
query-benchmark.ts and benchmark.ts each selected "hub"/"mid"/"leaf" call-graph targets by name via a raw SQL query with no `kind` filter and no deterministic tie-break. A hub-candidate name like `buildGraph` could match a local `const { buildGraph } = await import(...)` binding (kind=constant) as easily as the real function definition, and which node won depended on unspecified SQLite row order. benchDiffImpact then ran a second, independently unfiltered query to resolve the hub's file, which could disagree with the first resolution and mutate/diff-impact the wrong file. Extract the shared selection logic into scripts/lib/hub-selection.ts: filter candidates to function/method kinds, add an explicit id-based ORDER BY tie-break, and return the resolved hub's file alongside its name so benchDiffImpact reuses that exact node instead of re-querying. Fixes #1904
1 parent 438f2da commit 0a07066

4 files changed

Lines changed: 254 additions & 83 deletions

File tree

‎scripts/benchmark.ts‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ import fs from 'node:fs';
1414
import path from 'node:path';
1515
import { performance } from 'node:perf_hooks';
1616
import { fileURLToPath } from 'node:url';
17-
import Database from 'better-sqlite3';
1817
import { resolveBenchmarkExcludes, resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
1918
import { isWorker, workerEngine, workerTargets, forkEngines } from './lib/fork-engine.js';
2019
import { round1, timeMedian, timeMedianWithValue } from './lib/bench-timing.js';
20+
import { selectHubTargets } from './lib/hub-selection.js';
2121

2222
// ── Parent process: fork one child per engine, assemble final output ─────
2323
if (!isWorker()) {
@@ -98,24 +98,6 @@ const QUERY_WARMUP_RUNS = 3;
9898
const PROBE_FILE = path.join(root, 'src', 'domain', 'queries.ts');
9999
const BENCH_EXCLUDE = [...resolveBenchmarkExcludes()];
100100

101-
function selectTargets() {
102-
const db = new Database(dbPath, { readonly: true });
103-
const rows = db
104-
.prepare(
105-
`SELECT n.name, COUNT(e.id) AS cnt
106-
FROM nodes n
107-
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
108-
WHERE n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
109-
GROUP BY n.id
110-
ORDER BY cnt DESC`,
111-
)
112-
.all();
113-
db.close();
114-
115-
if (rows.length === 0) return { hub: 'buildGraph', leaf: 'median' };
116-
return { hub: rows[0].name, leaf: rows[rows.length - 1].name };
117-
}
118-
119101
// Redirect console.log to stderr so only JSON goes to stdout
120102
const origLog = console.log;
121103
console.log = (...args) => console.error(...args);
@@ -191,7 +173,7 @@ try {
191173

192174
// ── Query benchmarks ────────────────────────────────────────────────
193175
console.error(` [${engine}] Benchmarking queries...`);
194-
const targets = workerTargets() || selectTargets();
176+
const targets = workerTargets() || selectHubTargets(dbPath);
195177
console.error(` hub=${targets.hub}, leaf=${targets.leaf}`);
196178

197179
async function benchQuery(fn, ...args) {

‎scripts/lib/hub-selection.ts‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/**
2+
* Deterministic hub/mid/leaf target selection for benchmark scripts.
3+
*
4+
* `query-benchmark.ts` and `benchmark.ts` each pick representative call-graph
5+
* nodes ("hub" = well-connected, "mid"/"leaf" = less connected) by `name` to
6+
* drive their timed queries. Both independently duplicated a `selectTargets`
7+
* query that filtered candidates by file path (`NOT LIKE '%test%'`/`'%spec%'`)
8+
* but not by `kind` — so a hub-candidate name like `buildGraph` could resolve
9+
* to a local `const { buildGraph } = await import(...)` binding (`kind =
10+
* 'constant'`) instead of the real `function buildGraph` definition, and
11+
* which node won depended on unspecified SQLite row order (#1904).
12+
*
13+
* This module is the single source of truth for that selection: it filters
14+
* to `HUB_CANDIDATE_KINDS` (mirroring `CALLABLE_SYMBOL_KINDS` in
15+
* `src/shared/kinds.ts`, #1888 — the same "same-name lookup with no other
16+
* signal" hazard) and adds an explicit `ORDER BY id` tie-break so the choice
17+
* is reproducible across builds that insert the same logical nodes in a
18+
* different physical row order (e.g. worker-thread parse completion order).
19+
*
20+
* Callers that need to act on the resolved hub's file (e.g. `benchDiffImpact`
21+
* writing a probe comment into it) should use the `hubFile` this returns
22+
* instead of re-querying `nodes` by name — a second unfiltered query can
23+
* disagree with this one about which physical node "the hub" is.
24+
*/
25+
26+
import Database from 'better-sqlite3';
27+
28+
// Symbol kinds that represent a real invocable definition. Local variable
29+
// and constant bindings must never win hub selection just because they share
30+
// a candidate's name — mirrors CALLABLE_SYMBOL_KINDS in src/shared/kinds.ts.
31+
export const HUB_CANDIDATE_KINDS: readonly string[] = ['function', 'method'];
32+
33+
export interface HubTargets {
34+
hub: string;
35+
hubFile: string;
36+
mid: string;
37+
leaf: string;
38+
}
39+
40+
interface NodeRow {
41+
name: string;
42+
file: string;
43+
}
44+
45+
interface RankedNodeRow extends NodeRow {
46+
cnt: number;
47+
}
48+
49+
/**
50+
* Selects stable, deterministic hub/mid/leaf targets from a freshly-built
51+
* graph DB at `dbPath`.
52+
*
53+
* `pinnedCandidates` are tried in order (each filtered to
54+
* `HUB_CANDIDATE_KINDS` and non-test files) before falling back to the
55+
* most-connected qualifying node — pinning keeps the "hub" identity stable
56+
* across versions where auto-selection would otherwise drift as files are
57+
* added or removed.
58+
*
59+
* Throws if the graph has no qualifying nodes with edges at all (an empty or
60+
* malformed build), rather than silently returning a name that resolves to
61+
* nothing downstream.
62+
*/
63+
export function selectHubTargets(dbPath: string, pinnedCandidates: readonly string[] = []): HubTargets {
64+
const db = new Database(dbPath, { readonly: true });
65+
try {
66+
const kindPlaceholders = HUB_CANDIDATE_KINDS.map(() => '?').join(', ');
67+
68+
let hub: string | null = null;
69+
let hubFile: string | null = null;
70+
for (const candidate of pinnedCandidates) {
71+
const row = db
72+
.prepare(
73+
`SELECT n.name, n.file FROM nodes n
74+
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
75+
WHERE n.name = ? AND n.kind IN (${kindPlaceholders})
76+
AND n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
77+
ORDER BY n.id ASC
78+
LIMIT 1`,
79+
)
80+
.get(candidate, ...HUB_CANDIDATE_KINDS) as NodeRow | undefined;
81+
if (row) {
82+
hub = row.name;
83+
hubFile = row.file;
84+
break;
85+
}
86+
}
87+
88+
const rows = db
89+
.prepare(
90+
`SELECT n.id, n.name, n.file, COUNT(e.id) AS cnt
91+
FROM nodes n
92+
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
93+
WHERE n.kind IN (${kindPlaceholders})
94+
AND n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
95+
GROUP BY n.id
96+
ORDER BY cnt DESC, n.id ASC`,
97+
)
98+
.all(...HUB_CANDIDATE_KINDS) as RankedNodeRow[];
99+
100+
if (rows.length === 0) {
101+
throw new Error('No nodes with edges found in graph');
102+
}
103+
104+
if (!hub) {
105+
hub = rows[0].name;
106+
hubFile = rows[0].file;
107+
}
108+
109+
const mid = rows[Math.floor(rows.length / 2)].name;
110+
const leaf = rows[rows.length - 1].name;
111+
112+
return { hub, hubFile: hubFile as string, mid, leaf };
113+
} finally {
114+
db.close();
115+
}
116+
}

‎scripts/query-benchmark.ts‎

Lines changed: 14 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ import fs from 'node:fs';
1515
import path from 'node:path';
1616
import { performance } from 'node:perf_hooks';
1717
import { fileURLToPath } from 'node:url';
18-
import Database from 'better-sqlite3';
1918
import { resolveBenchmarkExcludes, resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
2019
import { isWorker, workerEngine, workerTargets, forkEngines } from './lib/fork-engine.js';
2120
import { round1, timeMedian } from './lib/bench-timing.js';
21+
import { selectHubTargets, type HubTargets } from './lib/hub-selection.js';
2222

2323
// ── Parent process: fork one child per engine, assemble final output ─────
2424
if (!isWorker()) {
@@ -123,52 +123,6 @@ const WARMUP_RUNS = 3;
123123
// meaningless when barrel/type files get added or removed.
124124
const PINNED_HUB_CANDIDATES = ['buildGraph', 'openDb', 'loadConfig'];
125125

126-
function selectTargets() {
127-
const db = new Database(dbPath, { readonly: true });
128-
try {
129-
130-
// Try pinned candidates first for a stable hub across versions
131-
let hub = null;
132-
for (const candidate of PINNED_HUB_CANDIDATES) {
133-
const row = db
134-
.prepare(
135-
`SELECT n.name FROM nodes n
136-
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
137-
WHERE n.name = ? AND n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
138-
LIMIT 1`,
139-
)
140-
.get(candidate);
141-
if (row) {
142-
hub = row.name;
143-
break;
144-
}
145-
}
146-
147-
const rows = db
148-
.prepare(
149-
`SELECT n.name, COUNT(e.id) AS cnt
150-
FROM nodes n
151-
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
152-
WHERE n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
153-
GROUP BY n.id
154-
ORDER BY cnt DESC`,
155-
)
156-
.all();
157-
158-
if (rows.length === 0) throw new Error('No nodes with edges found in graph');
159-
160-
// Fall back to most-connected if no pinned candidate found
161-
if (!hub) hub = rows[0].name;
162-
163-
const mid = rows[Math.floor(rows.length / 2)].name;
164-
const leaf = rows[rows.length - 1].name;
165-
return { hub, mid, leaf };
166-
167-
} finally {
168-
db.close();
169-
}
170-
}
171-
172126
async function benchDepths(fn, name, depths) {
173127
const result = {};
174128
for (const depth of depths) {
@@ -197,21 +151,18 @@ function resolveDbFile(rootDir: string, dbFile: string): string | null {
197151
return null;
198152
}
199153

200-
async function benchDiffImpact(hubName) {
201-
const db = new Database(dbPath, { readonly: true });
202-
const row = db
203-
.prepare(`SELECT file FROM nodes WHERE name = ? LIMIT 1`)
204-
.get(hubName);
205-
db.close();
206-
207-
if (!row) return { latencyMs: 0, affectedFunctions: 0, affectedFiles: 0 };
208-
209-
// row.file is normally relative (e.g. 'src/domain/builder.ts'), but some
210-
// environments store absolute-like paths without the leading '/'. Handle
211-
// both cases so the benchmark works regardless of DB path format.
212-
const hubFile = resolveDbFile(root, row.file);
154+
async function benchDiffImpact(targets: HubTargets) {
155+
// Reuse the exact physical node selectHubTargets already resolved for
156+
// `targets.hub` instead of re-querying `nodes` by name — a second,
157+
// independently unfiltered query can disagree with the first about which
158+
// same-named node "the hub" is (#1904).
159+
//
160+
// targets.hubFile is normally relative (e.g. 'src/domain/builder.ts'), but
161+
// some environments store absolute-like paths without the leading '/'.
162+
// Handle both cases so the benchmark works regardless of DB path format.
163+
const hubFile = resolveDbFile(root, targets.hubFile);
213164
if (!hubFile) {
214-
console.error(`[benchDiffImpact] Cannot find hub file for row.file=${row.file}`);
165+
console.error(`[benchDiffImpact] Cannot find hub file for hubFile=${targets.hubFile}`);
215166
return { latencyMs: 0, affectedFunctions: 0, affectedFiles: 0 };
216167
}
217168
const original = fs.readFileSync(hubFile, 'utf8');
@@ -242,7 +193,7 @@ async function benchDiffImpact(hubName) {
242193
if (fs.existsSync(dbPath)) fs.unlinkSync(dbPath);
243194
await buildGraph(root, { engine, incremental: false, exclude: [...resolveBenchmarkExcludes()] });
244195

245-
const targets = workerTargets() || selectTargets();
196+
const targets: HubTargets = workerTargets() || selectHubTargets(dbPath, PINNED_HUB_CANDIDATES);
246197
console.error(`Targets: hub=${targets.hub}, mid=${targets.mid}, leaf=${targets.leaf}`);
247198

248199
const fnDeps = {};
@@ -256,7 +207,7 @@ fnImpact.depth1Ms = (await benchDepths(fnImpactData, targets.hub, [1])).depth1Ms
256207
fnImpact.depth3Ms = (await benchDepths(fnImpactData, targets.hub, [3])).depth3Ms;
257208
fnImpact.depth5Ms = (await benchDepths(fnImpactData, targets.hub, [5])).depth5Ms;
258209

259-
const diffImpact = await benchDiffImpact(targets.hub);
210+
const diffImpact = await benchDiffImpact(targets);
260211

261212
// Restore console.log for JSON output
262213
console.log = origLog;

0 commit comments

Comments
 (0)