Skip to content

fix(scripts): resolve benchmark hub targets to a single, kind-filtered node #2039

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

Open
carlos-alm wants to merge 1 commit into
base: fix/issue-1901-engine-parity-native-js-extractor-emits-no
Choose a base branch
from
Open

fix(scripts): resolve benchmark hub targets to a single, kind-filtered node #2039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 2 additions & 20 deletions scripts/benchmark.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import fs from 'node:fs';
import path from 'node:path';
import { performance } from 'node:perf_hooks';
import { fileURLToPath } from 'node:url';
import Database from 'better-sqlite3';
import { resolveBenchmarkExcludes, resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
import { isWorker, workerEngine, workerTargets, forkEngines } from './lib/fork-engine.js';
import { round1, timeMedian, timeMedianWithValue } from './lib/bench-timing.js';
import { selectHubTargets } from './lib/hub-selection.js';

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

function selectTargets() {
const db = new Database(dbPath, { readonly: true });
const rows = db
.prepare(
`SELECT n.name, COUNT(e.id) AS cnt
FROM nodes n
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
WHERE n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
GROUP BY n.id
ORDER BY cnt DESC`,
)
.all();
db.close();

if (rows.length === 0) return { hub: 'buildGraph', leaf: 'median' };
return { hub: rows[0].name, leaf: rows[rows.length - 1].name };
}

// Redirect console.log to stderr so only JSON goes to stdout
const origLog = console.log;
console.log = (...args) => console.error(...args);
Expand Down Expand Up @@ -191,7 +173,7 @@ try {

// ── Query benchmarks ────────────────────────────────────────────────
console.error(` [${engine}] Benchmarking queries...`);
const targets = workerTargets() || selectTargets();
const targets = workerTargets() || selectHubTargets(dbPath);

@greptile-apps greptile-apps Bot Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Benchmark hub selection still drifts without pinned candidates

selectHubTargets(dbPath) is called with no second argument, so benchmark.ts always falls back to the most-connected qualifying node. The kind-filter bug is fixed, but the hub identity can silently shift between versions whenever the most-connected function changes (e.g. a new heavily-called utility is added). query-benchmark.ts avoids this with PINNED_HUB_CANDIDATES, but that list was never ported to benchmark.ts. The output JSON records targets, so the drift is visible post-hoc — but back-to-back benchmark runs for version-to-version comparisons may measure different nodes and produce misleading deltas.

Fix in Claude Code

console.error(` hub=${targets.hub}, leaf=${targets.leaf}`);

async function benchQuery(fn, ...args) {
Expand Down
116 changes: 116 additions & 0 deletions scripts/lib/hub-selection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/**
* Deterministic hub/mid/leaf target selection for benchmark scripts.
*
* `query-benchmark.ts` and `benchmark.ts` each pick representative call-graph
* nodes ("hub" = well-connected, "mid"/"leaf" = less connected) by `name` to
* drive their timed queries. Both independently duplicated a `selectTargets`
* query that filtered candidates by file path (`NOT LIKE '%test%'`/`'%spec%'`)
* but not by `kind` — so a hub-candidate name like `buildGraph` could resolve
* to a local `const { buildGraph } = await import(...)` binding (`kind =
* 'constant'`) instead of the real `function buildGraph` definition, and
* which node won depended on unspecified SQLite row order (#1904).
*
* This module is the single source of truth for that selection: it filters
* to `HUB_CANDIDATE_KINDS` (mirroring `CALLABLE_SYMBOL_KINDS` in
* `src/shared/kinds.ts`, #1888 — the same "same-name lookup with no other
* signal" hazard) and adds an explicit `ORDER BY id` tie-break so the choice
* is reproducible across builds that insert the same logical nodes in a
* different physical row order (e.g. worker-thread parse completion order).
*
* Callers that need to act on the resolved hub's file (e.g. `benchDiffImpact`
* writing a probe comment into it) should use the `hubFile` this returns
* instead of re-querying `nodes` by name — a second unfiltered query can
* disagree with this one about which physical node "the hub" is.
*/

import Database from 'better-sqlite3';

// Symbol kinds that represent a real invocable definition. Local variable
// and constant bindings must never win hub selection just because they share
// a candidate's name — mirrors CALLABLE_SYMBOL_KINDS in src/shared/kinds.ts.
export const HUB_CANDIDATE_KINDS: readonly string[] = ['function', 'method'];

export interface HubTargets {
hub: string;
hubFile: string;
mid: string;
leaf: string;
}

interface NodeRow {
name: string;
file: string;
}

interface RankedNodeRow extends NodeRow {
cnt: number;
}

/**
* Selects stable, deterministic hub/mid/leaf targets from a freshly-built
* graph DB at `dbPath`.
*
* `pinnedCandidates` are tried in order (each filtered to
* `HUB_CANDIDATE_KINDS` and non-test files) before falling back to the
* most-connected qualifying node — pinning keeps the "hub" identity stable
* across versions where auto-selection would otherwise drift as files are
* added or removed.
*
* Throws if the graph has no qualifying nodes with edges at all (an empty or
* malformed build), rather than silently returning a name that resolves to
* nothing downstream.
*/
export function selectHubTargets(dbPath: string, pinnedCandidates: readonly string[] = []): HubTargets {
const db = new Database(dbPath, { readonly: true });
try {
const kindPlaceholders = HUB_CANDIDATE_KINDS.map(() => '?').join(', ');

let hub: string | null = null;
let hubFile: string | null = null;
for (const candidate of pinnedCandidates) {
const row = db
.prepare(
`SELECT n.name, n.file FROM nodes n
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
WHERE n.name = ? AND n.kind IN (${kindPlaceholders})
AND n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
ORDER BY n.id ASC
LIMIT 1`,
)
.get(candidate, ...HUB_CANDIDATE_KINDS) as NodeRow | undefined;
if (row) {
hub = row.name;
hubFile = row.file;
break;
}
}

const rows = db
.prepare(
`SELECT n.id, n.name, n.file, COUNT(e.id) AS cnt
FROM nodes n
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
WHERE n.kind IN (${kindPlaceholders})
AND n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
GROUP BY n.id
ORDER BY cnt DESC, n.id ASC`,
)
.all(...HUB_CANDIDATE_KINDS) as RankedNodeRow[];

if (rows.length === 0) {
throw new Error('No nodes with edges found in graph');
}

if (!hub) {
hub = rows[0].name;
hubFile = rows[0].file;
}

const mid = rows[Math.floor(rows.length / 2)].name;
const leaf = rows[rows.length - 1].name;
Comment on lines +109 to +110

@greptile-apps greptile-apps Bot Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 mid is computed and exported but consumed nowhere

mid is included in HubTargets and tested, but neither query-benchmark.ts nor benchmark.ts ever passes targets.mid to a benchDepths/benchQuery call. It flows into the output JSON blob (as part of targets) but drives no actual measurement. If the intent is to benchmark medium-connectivity nodes, the corresponding benchDepths(fnDepsData, targets.mid, ...) calls are missing; if it's purely informational, the HubTargets interface and tests create the impression of an active selection that does not actually feed any benchmark.

Fix in Claude Code


return { hub, hubFile: hubFile as string, mid, leaf };
} finally {
db.close();
}
}
77 changes: 14 additions & 63 deletions scripts/query-benchmark.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ import fs from 'node:fs';
import path from 'node:path';
import { performance } from 'node:perf_hooks';
import { fileURLToPath } from 'node:url';
import Database from 'better-sqlite3';
import { resolveBenchmarkExcludes, resolveBenchmarkSource, srcImport } from './lib/bench-config.js';
import { isWorker, workerEngine, workerTargets, forkEngines } from './lib/fork-engine.js';
import { round1, timeMedian } from './lib/bench-timing.js';
import { selectHubTargets, type HubTargets } from './lib/hub-selection.js';

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

function selectTargets() {
const db = new Database(dbPath, { readonly: true });
try {

// Try pinned candidates first for a stable hub across versions
let hub = null;
for (const candidate of PINNED_HUB_CANDIDATES) {
const row = db
.prepare(
`SELECT n.name FROM nodes n
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
WHERE n.name = ? AND n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
LIMIT 1`,
)
.get(candidate);
if (row) {
hub = row.name;
break;
}
}

const rows = db
.prepare(
`SELECT n.name, COUNT(e.id) AS cnt
FROM nodes n
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
WHERE n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
GROUP BY n.id
ORDER BY cnt DESC`,
)
.all();

if (rows.length === 0) throw new Error('No nodes with edges found in graph');

// Fall back to most-connected if no pinned candidate found
if (!hub) hub = rows[0].name;

const mid = rows[Math.floor(rows.length / 2)].name;
const leaf = rows[rows.length - 1].name;
return { hub, mid, leaf };

} finally {
db.close();
}
}

async function benchDepths(fn, name, depths) {
const result = {};
for (const depth of depths) {
Expand Down Expand Up @@ -197,21 +151,18 @@ function resolveDbFile(rootDir: string, dbFile: string): string | null {
return null;
}

async function benchDiffImpact(hubName) {
const db = new Database(dbPath, { readonly: true });
const row = db
.prepare(`SELECT file FROM nodes WHERE name = ? LIMIT 1`)
.get(hubName);
db.close();

if (!row) return { latencyMs: 0, affectedFunctions: 0, affectedFiles: 0 };

// row.file is normally relative (e.g. 'src/domain/builder.ts'), but some
// environments store absolute-like paths without the leading '/'. Handle
// both cases so the benchmark works regardless of DB path format.
const hubFile = resolveDbFile(root, row.file);
async function benchDiffImpact(targets: HubTargets) {
// Reuse the exact physical node selectHubTargets already resolved for
// `targets.hub` instead of re-querying `nodes` by name — a second,
// independently unfiltered query can disagree with the first about which
// same-named node "the hub" is (#1904).
//
// targets.hubFile is normally relative (e.g. 'src/domain/builder.ts'), but
// some environments store absolute-like paths without the leading '/'.
// Handle both cases so the benchmark works regardless of DB path format.
const hubFile = resolveDbFile(root, targets.hubFile);
if (!hubFile) {
console.error(`[benchDiffImpact] Cannot find hub file for row.file=${row.file}`);
console.error(`[benchDiffImpact] Cannot find hub file for hubFile=${targets.hubFile}`);
return { latencyMs: 0, affectedFunctions: 0, affectedFiles: 0 };
}
const original = fs.readFileSync(hubFile, 'utf8');
Expand Down Expand Up @@ -242,7 +193,7 @@ async function benchDiffImpact(hubName) {
if (fs.existsSync(dbPath)) fs.unlinkSync(dbPath);
await buildGraph(root, { engine, incremental: false, exclude: [...resolveBenchmarkExcludes()] });

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

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

const diffImpact = await benchDiffImpact(targets.hub);
const diffImpact = await benchDiffImpact(targets);

// Restore console.log for JSON output
console.log = origLog;
Expand Down
Loading
Loading