fix(scripts): resolve benchmark hub targets to a single, kind-filtered node#2039
Conversation
…d 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
Greptile SummaryThis PR fixes non-deterministic hub/mid/leaf selection in the benchmark scripts by extracting a shared
Confidence Score: 4/5Safe to merge — the core non-determinism bug is correctly fixed in both scripts and is well-tested. The kind-filter and ORDER BY tie-break correctly address the root cause, and the double-query disagreement in benchDiffImpact is cleanly resolved by reusing hubFile. The remaining gap is that benchmark.ts still auto-selects the most-connected node without pinned candidates, so its hub identity can shift across graph versions — the same pre-existing drift, just no longer caused by wrong kind matching. The mid field is also computed and tested but never actually fed into a benchmark query. scripts/benchmark.ts — hub target can still drift across versions without pinned candidates; scripts/lib/hub-selection.ts — mid field is exported and tested but not consumed by any benchmark query. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant P as Parent process
participant W1 as Worker 1 (wasm)
participant W2 as Worker 2 (native)
participant DB as graph.db
participant HS as hub-selection.ts
P->>W1: fork (no TARGETS env)
W1->>DB: buildGraph()
W1->>HS: selectHubTargets(dbPath, PINNED_HUB_CANDIDATES)
HS->>DB: SELECT name,file WHERE kind IN ('function','method') ORDER BY id ASC [pinned]
HS->>DB: SELECT name,file,cnt WHERE kind IN ('function','method') ORDER BY cnt DESC,id ASC [ranked]
HS-->>W1: "{hub, hubFile, mid, leaf}"
W1->>DB: benchDiffImpact(targets.hubFile) — no second query
W1-->>P: "JSON {targets, ...}"
P->>W2: "fork (TARGETS=JSON.stringify(firstResult.targets))"
W2->>HS: workerTargets() → reuse wasm targets
W2->>DB: benchmark queries using targets.hub / targets.leaf
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant P as Parent process
participant W1 as Worker 1 (wasm)
participant W2 as Worker 2 (native)
participant DB as graph.db
participant HS as hub-selection.ts
P->>W1: fork (no TARGETS env)
W1->>DB: buildGraph()
W1->>HS: selectHubTargets(dbPath, PINNED_HUB_CANDIDATES)
HS->>DB: SELECT name,file WHERE kind IN ('function','method') ORDER BY id ASC [pinned]
HS->>DB: SELECT name,file,cnt WHERE kind IN ('function','method') ORDER BY cnt DESC,id ASC [ranked]
HS-->>W1: "{hub, hubFile, mid, leaf}"
W1->>DB: benchDiffImpact(targets.hubFile) — no second query
W1-->>P: "JSON {targets, ...}"
P->>W2: "fork (TARGETS=JSON.stringify(firstResult.targets))"
W2->>HS: workerTargets() → reuse wasm targets
W2->>DB: benchmark queries using targets.hub / targets.leaf
Reviews (1): Last reviewed commit: "fix(scripts): resolve benchmark hub targ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // ── Query benchmarks ──────────────────────────────────────────────── | ||
| console.error(` [${engine}] Benchmarking queries...`); | ||
| const targets = workerTargets() || selectTargets(); | ||
| const targets = workerTargets() || selectHubTargets(dbPath); |
There was a problem hiding this comment.
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.
| const mid = rows[Math.floor(rows.length / 2)].name; | ||
| const leaf = rows[rows.length - 1].name; |
There was a problem hiding this comment.
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.
Codegraph Impact Analysis9 functions changed → 3 callers affected across 2 files
|
1 participant
Summary
query-benchmark.tsandbenchmark.tseach picked "hub"/"mid"/"leaf" call-graph targets bynamevia a raw SQL query with nokindfilter and no deterministic tie-break:PINNED_HUB_CANDIDATES = ['buildGraph', 'openDb', 'loadConfig']) could match a localconst { buildGraph } = await import(...)binding (kind = 'constant') just as easily as the realfunction buildGraphdefinition.benchDiffImpact(hubName)then ran a second, independently unfilteredSELECT file FROM nodes WHERE name = ? LIMIT 1query with no test/spec/kind filter at all, so it could resolve to a different physical node/file than whateverselectTargets()had already picked in the same run — with noORDER BY, which node won was unspecified SQLite row order.scripts/benchmark.tshad its own independently-duplicatedselectTargets()with the same unfiltered-kind query, feeding the same ambiguity into itsfnDepsData/pathDatabenchmark calls.Verified end-to-end against this repo's own graph (WASM engine, this branch's extractor code):
buildGraphcurrently resolves to 8 same-named nodes — 6constant-kind local bindings acrossscripts/*.ts, 1parameter-kind, and the 1 realfunctiondefinition atsrc/domain/graph/builder/pipeline.ts.Fix
Extracted the selection logic into a single shared
scripts/lib/hub-selection.ts:HUB_CANDIDATE_KINDS = ['function', 'method']— mirrorsCALLABLE_SYMBOL_KINDSinsrc/shared/kinds.ts(Same-file bare-name lookup (resolveCallTargets) is unfiltered by kind and runs before type-aware dispatch, causing false calls-edges to unrelated classes/variables #1888), the same "same-name lookup with no other signal" hazard.ORDER BY id ASCtie-break, so selection is reproducible regardless of physical row insertion order.selectHubTargetsnow also returns the resolved hub'sfile(hubFile) as part of the same query that resolved itsname, sobenchDiffImpactreuses that exact node instead of re-queryingnodesby name a second time — eliminating the disagreement risk entirely.query-benchmark.tsandbenchmark.tsboth now call the shared helper instead of maintaining their own near-duplicateselectTargets()implementations.Test plan
tests/unit/hub-selection.test.ts— fixture DB with a same-namedconstantbinding (higher raw edge count) alongside the realfunction/methoddefinitions; verifies pinned-candidate selection, most-connected fallback, mid/leaf ranking, determinism across repeated calls, and the empty-graph error pathnpx vitest run tests/unit/hub-selection.test.ts— 5 passednpm run lint— clean (no new warnings/errors)npm run build— cleanscripts/query-benchmark.tsdirectly —hubnow resolves deterministically tosrc/domain/graph/builder/pipeline.ts(the real definition) across repeated runs, confirmed via directselectHubTargets()calls against the built DBnpm test— pre-existing failures in this worktree are all native-engine tests failing onCannot find module '@optave/codegraph-darwin-arm64'(no native addon installed in this fresh worktree); none touch the files changed hereStacked on #1901 (base branch
fix/issue-1901-engine-parity-native-js-extractor-emits-no) — only thescripts//tests/unit/diff is this issue's change.Fixes #1904