fix(bench): give query-benchmark.ts's diffImpact metric a warmup too

[carlos-alm]

Summary

WARMUP_RUNS in scripts/query-benchmark.ts was applied only inside benchDepths, which serves fnDeps and fnImpact. benchDiffImpact called timeMedian directly with no warmup loop, making diffImpact latency the only query metric measured cold.

This is the same defect class #2584 just fixed for benchmark.ts's Full build metric, and that #2436 traced part of the pre-publish gate's non-determinism to — the one metric that measured without this warmup. diffImpact was the remaining instance.

The warmup the earlier benchDepths calls already paid does not carry over: diffImpactData is a distinct query path with its own prepared statements and its own DB pages, and it shells out to git to read the staged diff.

Why this surfaced now

Discovered while diagnosing why the pre-publish benchmark gate kept failing #2586, whose diff (one enum_declaration map arm per engine) provably cannot affect any benchmark — the benchmarked corpus contains zero export enum declarations. Three consecutive runs of that unchanged head each failed on a different metric, with each previously-failing metric returning to baseline:

attempt	native noop	native diffImpact	wasm 1-file (setupMs)	result
1	103 (baseline 32)	26.5	173 (7.8)	FAIL x3
2	24	12.3	349 (221.8)	FAIL x1
3	23	21.8	158 (16.1)	FAIL x1

Native diffImpact latency across the four most recent PR runs — 12.4, 16.5, 16.9, then 12.3 / 21.8 / 26.5 — against a recorded 3.17.0 baseline of 10.1. Recent passing runs at 16.5–16.9 were already +63–67% over the 25% threshold and survived only because the ~10ms MIN_ABSOLUTE_DELTA floor filtered their 6.8ms delta. Full analysis in #2590.

Test plan

• Ran scripts/query-benchmark.ts against this repo, 3 runs per variant, alternating baseline/fixed to control for machine drift:

  variant	native diffImpact (ms)	wasm diffImpact (ms)
  before	10.9 / 10.8 / 11.2	9.4 / 9.2 / 9.6
  after	9.9 / 9.8 / 9.9	9.4 / 9.4 / 9.7

  Both a ~10% drop and a visibly tighter spread on native. WASM is unchanged, which is the expected signature — the cold-start cost this removes is the native NAPI/rusqlite one, exactly as the WARMUP_RUNS docstring describes.

• Output JSON stays well-formed and affectedFunctions / affectedFiles still populate (the warmup loop deliberately does not assign lastResult, so the reported result still comes from a timed run).

• npm run lint clean.

Honest scope note

This fixes a real methodological defect and measurably tightens the metric, but to be straight about what it does not prove: a ~1ms improvement measured on a quiet local machine is not evidence that CI will stop spiking to 21–26ms. The gate's remaining exposure — a stale 10.1 baseline that the metric no longer sits near, and the fact that affectedFunctions: 0 on every historical entry means this benchmark measures near-pure fixed overhead — is tracked in #2590 and deliberately left out of this PR to keep it to one concern.

Note that adding diffImpact latency to NOISY_METRICS would not have helped: a failure needs both the percentage and the ~10ms absolute-delta floor to trip, and 21.8ms clears a 50% threshold on a 10.1 baseline just as it clears 25%.

Refs #2590

[github-actions]

Heads up: this PR references #2590 without a closing keyword (Closes #N / Fixes #N). If this PR fully resolves #2590, update the description so the issue auto-closes on merge — otherwise disregard this comment.

[greptile-apps]

Greptile Summary

Adds three untimed diffImpactData invocations before measuring diff-impact latency so this query follows the same warm-then-measure methodology as the other query benchmarks.

• Expands the shared warmup documentation to cover each query path.
• Preserves result reporting by assigning lastResult only during measured runs.
• Leaves cleanup inside the existing try/finally block.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking documentation correction needed for the prepared-statement warmup claim.

The added calls operate against a stable staged diff through readonly database connections and retain existing cleanup behavior; the only accepted concern is that the explanatory comment overstates which state persists into timed calls.

Files Needing Attention: scripts/query-benchmark.ts

Important Files Changed

Filename	Overview
scripts/query-benchmark.ts	The warmup is read-only and cleanup-safe, but its new rationale incorrectly says it warms connection-local prepared statements.

_{Reviews (1): Last reviewed commit: "Merge origin/main into fix/query-bench-d..." | Re-trigger Greptile}

[greptile-apps]

Prepared-statement warmup claim is inaccurate

The new explanation says these calls warm diffImpact's prepared statements, but every invocation opens and closes a separate database connection and the statement cache is connection-scoped. Only process-wide initialization and OS page-cache state persist, so this wording can mislead future benchmark investigations about which cold-start costs remain in measured runs.

[github-actions]

Codegraph Impact Analysis

1 functions changed → 1 callers affected across 1 files

• benchDiffImpact in scripts/query-benchmark.ts:154 (1 transitive callers)

[carlos-alm]

CI result for this branch, for the record — the full pipeline is green, including the pre-publish benchmark gate (18m50s).

Measured on that run: native diffImpact latency = 16.0ms, wasm 12.4ms; native no-op rebuild 29ms; wasm 1-file rebuild 171ms.

Worth stating plainly rather than claiming a win: 16.0ms is still +58% over the recorded 10.1 baseline, and it passes only because the 5.9ms delta stays under the ~10ms MIN_ABSOLUTE_DELTA floor. That is the same band as the recent passing runs (16.5, 16.9), so on CI this change has not visibly moved the metric the way it did locally (10.8–11.2 → 9.8–9.9).

So: the warmup removes a real methodological defect and is worth landing on its own merits, but it does not on this evidence resolve the gate's fragility. The binding problem is the one still tracked in #2590 — the baseline is stale relative to where this metric actually runs on CI, so the check is riding on the absolute-delta floor rather than on real headroom, and any run landing at ≥20.1ms fails regardless of this fix.