From ddb50c3cb39628805855bebac557e3f17f793b00 Mon Sep 17 00:00:00 2001 From: Brummel Date: Wed, 20 May 2026 16:07:53 +0200 Subject: [PATCH] =?UTF-8?q?plan:=20bench-harness-recalibration.1=20?= =?UTF-8?q?=E2=80=94=20drop=206=20latency=20entries=20+=20recapture?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One terminal iteration covering the whole spec. Four tasks, all on `bench/baseline.json`: - Task 1: pre-recapture JSON edit — drop `max_us` + `p99_9_us` × 3 latency arms (6 entries) and rewrite the `note` field forward- looking. The 6 entries must go before `--update-baseline` runs because `write_new_baseline` faithfully re-baselines anything left in the file (recon Open Q). The note rewrite is folded in here because `--update-baseline` preserves the existing note as- is, so any change has to happen before recapture (or as a separate post-recapture edit; one-edit is cleaner). - Task 2: `bench/check.py --update-baseline` regenerates every remaining `baseline` value from a fresh `bench/run.sh -n 5`, updating `captured` → 2026-05-20 and `captured_via`. - Task 3: acceptance §1 — fresh-HEAD replay → exit 0, 0 regressed, 57 metrics in summary (= pre-edit 63 minus the 6 drops). - Task 4: acceptance §2 — synthetic injection (halve `bench_list_sum.bump_s.baseline` via `jq`) → exit 1 + REGRESSION row on that metric, then restore via `jq` and confirm a final exit-0 replay. Recon adjudications: - Replay-source (recon Open Q1): fresh `bench/run.sh -n 5` for replay, not the same output that produced the recapture. Realistic acceptance scenario; with 10% throughput tolerance vs. ~1-2% measured run-to-run variance on the affected metrics (run-1 / run-2 reproduction data in spec body), there is ample margin. - Note rewrite (recon Open Q2): forward-looking — drop stale refs (JOURNAL workflow retired 8e586f4; the `*.max_us` tolerance convention is now moot since the metric is gone), replace with pointer to docs/specs/2026-05-20-bench-harness- recalibration.md + closed issues #15 / #16. Spec lives where the rationale lives; the note carries the gate-policy one-liner only. No Rust crate touched. `bench/check.py` / `bench/run.sh` / `bench/latency_harness.py` unmodified. Plan self-review (all 8 checklist items): clean. Step granularity checked; Task 3 collapsed from two `bench/check.py` runs to one (single 3-minute run captures both summary and exit code). Ready for handoff to `skills/implement`. --- docs/plans/bench-harness-recalibration.1.md | 299 ++++++++++++++++++++ 1 file changed, 299 insertions(+) create mode 100644 docs/plans/bench-harness-recalibration.1.md diff --git a/docs/plans/bench-harness-recalibration.1.md b/docs/plans/bench-harness-recalibration.1.md new file mode 100644 index 0000000..6b9a2b5 --- /dev/null +++ b/docs/plans/bench-harness-recalibration.1.md @@ -0,0 +1,299 @@ +# Bench harness recalibration — Implementation Plan (iteration 1, terminal) + +> **Parent spec:** `docs/specs/2026-05-20-bench-harness-recalibration.md` +> +> **For agentic workers:** REQUIRED SUB-SKILL: use `skills/implement` +> to run this plan. Steps use `- [ ]` checkboxes for tracking. + +**Goal:** Close Gitea #15 + #16 by removing `max_us` and `p99_9_us` +from `bench/baseline.json` (6 entries across 3 latency arms) and +regenerating the remaining baselines from current HEAD, so +`bench/check.py` stops false-positive-firing on environmentally +drifted no-op milestones. + +**Architecture:** Single artefact (`bench/baseline.json`); no code +change. `bench/check.py`'s `--update-baseline` mode preserves +`version` / `note` / per-metric `tolerance_pct` and overwrites only +the `baseline` numeric + `captured` date + `captured_via`. The 6 +unreliable entries are dropped *before* `--update-baseline` (the +recapture iterates `existing.get(...).items()` and would faithfully +re-baseline any entry left in the file). The rewritten `note` field +is committed in the same pre-recapture edit so `--update-baseline`'s +note-preservation logic carries the new text forward. + +**Tech Stack:** `bench/baseline.json` (data file only). +`bench/check.py` (read-only; invoked twice — once to recapture, +once to replay-verify) and `bench/run.sh` (read-only; invoked by +`bench/check.py` for both runs). No Rust crate touched. + +--- + +**Files this plan creates or modifies:** + +- Modify: `bench/baseline.json` — the sole artefact. + - `note` (line 5): rewrite to drop stale references (JOURNAL workflow retired 8e586f4, `*.max_us` tolerance convention now-moot) and add forward pointer to this spec + closed issues #15 / #16. + - `latency.implicit_at_gc` (lines 71–77): delete `p99_9_us` (line 74) and `max_us` (line 75) entries. + - `latency.explicit_at_rc` (lines 78–84): delete `p99_9_us` (line 81) and `max_us` (line 82) entries. + - `latency.implicit_at_rc` (lines 85–91): delete `p99_9_us` (line 88) and `max_us` (line 89) entries. + - `throughput.*.baseline` (lines 7–67): regenerated by `bench/check.py --update-baseline` against today's bench run. + - `latency.*.baseline` (remaining 3 metrics × 3 arms = 9 values after drops): regenerated by the same recapture. + - `captured` (line 3): `2026-05-09` → `2026-05-20` (auto, via `bench/check.py:255`). + - `captured_via` (line 4): set to `bench/run.sh -n 5` (auto, via `bench/check.py:305`). +- Read-only verification target: `bench/check.py:177-220` (`collect_measurements` — iterates only baseline.json entries; the JSON drop is sufficient). +- Read-only verification target: `bench/check.py:249-284` (`write_new_baseline` — preserves `version` / `note` / `tolerance_pct`). + +No other file changes. No Rust crate touched. + +--- + +### Task 1: Pre-recapture JSON edit — drop 6 entries + rewrite `note` + +**Files:** +- Modify: `bench/baseline.json` (lines 5, 74–75, 81–82, 88–89) + +The recapture preserves the existing `note` field as-is, so the +`note` rewrite happens here, *before* `bench/check.py --update-baseline` +runs. The 6 latency entries must also be dropped here — leaving them +in the file would cause `write_new_baseline` to faithfully re-baseline +them on today's measurements (recon Open Q). + +- [ ] **Step 1: Rewrite the `note` field** + +Edit `bench/baseline.json` line 5 — the entire `"note": "..."` value — +to the following exact string (single line in the JSON; newlines below +are for readability only; the JSON value must NOT contain literal +newlines, escape them as needed or keep it one logical sentence): + +``` +Baseline for bench/check.py regression detection. The language-invariant thresholds (rc/bump <= 1.3x throughput, p99/median <= 5x latency) are NOT the regression-check tolerances; the per-metric tolerances below are tuned to absorb run-to-run noise on a quiet developer machine. To update after an intentional change, re-run bench/run.sh and replace the values, recording the reason in the commit body that ships the baseline bump. The latency arms gate only on median / p99 / p99_over_median — max_us and p99_9_us were removed on the 2026-05-20 recapture (Gitea #15 / #16) because tail-of-distribution latency metrics are dominated by OS-level jitter (THP defrag, scheduler preemption, IRQ load), not allocator behaviour, and produced 3+ consecutive false-positive REGRESSION rows on byte-identical no-op milestones. See docs/specs/2026-05-20-bench-harness-recalibration.md. +``` + +- [ ] **Step 2: Delete the 6 latency entries** + +Open `bench/baseline.json` and delete these six lines (line numbers +relative to the pre-edit file; the editor's "find this exact JSON +key" is sufficient — line numbers are guidance only): + +- Line 74: ` "p99_9_us": { "baseline": 8131.2, "tolerance_pct": 25 },` +- Line 75: ` "max_us": { "baseline": 8343.7, "tolerance_pct": 25 },` +- Line 81: ` "p99_9_us": { "baseline": 404.1, "tolerance_pct": 25 },` +- Line 82: ` "max_us": { "baseline": 413.0, "tolerance_pct": 25 },` +- Line 88: ` "p99_9_us": { "baseline": 452.0, "tolerance_pct": 25 },` +- Line 89: ` "max_us": { "baseline": 477.3, "tolerance_pct": 30 },` + +For each removed line, also remove its trailing comma from the +previous line if needed to keep the JSON valid (the entry above +`p99_9_us` is `p99_us`, which currently has a trailing comma; after +the deletion, `p99_us` is fine as-is because `p99_over_median` +follows it — re-check the comma chain per arm after edit). + +- [ ] **Step 3: Verify JSON validity** + +Run: + +```bash +python3 -m json.tool bench/baseline.json > /dev/null && echo OK +``` + +Expected stdout: `OK`. Any other output (a Python traceback) means +the JSON is malformed — fix the comma chain. + +- [ ] **Step 4: Verify the 6 entries are gone** + +Run: + +```bash +jq '[.latency | to_entries[] | .value | keys[]] | unique' bench/baseline.json +``` + +Expected stdout: + +``` +[ + "median_us", + "p99_over_median", + "p99_us" +] +``` + +If `max_us` or `p99_9_us` is in the list, Step 2 was incomplete. + +- [ ] **Step 5: Verify the `note` field rewrite landed** + +Run: + +```bash +jq -r '.note' bench/baseline.json | grep -c 'docs/specs/2026-05-20-bench-harness-recalibration.md' +``` + +Expected stdout: `1`. Zero hits means the rewrite from Step 1 was +not saved. + +--- + +### Task 2: Recapture remaining baselines via `--update-baseline` + +**Files:** +- Modify: `bench/baseline.json` (all `baseline` numerics + `captured` + `captured_via`) + +- [ ] **Step 1: Confirm HEAD is the bench-relevant clean state** + +Run: + +```bash +git status --short && git log -1 --oneline +``` + +Expected stdout includes `M bench/baseline.json` (or `M bench/baseline.json` — staged or unstaged from Task 1) as the only change. HEAD should be a workflow / spec / plan commit, NOT a runtime-touching commit. If any `crates/` or `runtime/` file is modified or HEAD is a runtime-touching commit, STOP and re-evaluate — the recapture must run from a clean bench-relevant state. + +- [ ] **Step 2: Run the recapture** + +Run: + +```bash +bench/check.py --update-baseline +``` + +Expected: takes ~3-4 minutes (release-mode build of all bench fixtures + `-n 5` runs across throughput + latency arms). Final stderr line: `>>> wrote new baseline to .../bench/baseline.json`. Exit code 0. + +- [ ] **Step 3: Verify the recapture preserved structure** + +Run: + +```bash +jq '.captured, .captured_via' bench/baseline.json +``` + +Expected stdout: + +``` +"2026-05-20" +"bench/run.sh -n 5" +``` + +(Or whatever today's date is when the implement run executes — the field is `$(date +%Y-%m-%d)` per `bench/check.py:255`.) + +Also confirm note + entry deletions still hold: + +```bash +jq -r '.note' bench/baseline.json | grep -c 'docs/specs/2026-05-20-bench-harness-recalibration.md' +jq '[.latency | to_entries[] | .value | keys[]] | unique' bench/baseline.json +``` + +Expected: `1` and the 3-element list `["median_us", "p99_over_median", "p99_us"]`. If either of those checks fails, `--update-baseline` somehow regenerated the dropped entries (would mean the Task-1 deletion didn't actually land before recapture) — STOP, restart from Task 1. + +--- + +### Task 3: Acceptance §1 — same-HEAD replay → exit 0 + +**Files:** None modified. Read-only check. + +- [ ] **Step 1: Run check.py and capture exit code** + +Run: + +```bash +bench/check.py; echo "EXIT=$?" +``` + +Expected: takes ~3 minutes (build cached from Task 2). Final summary line of stdout matches the pattern: + +``` +summary: 57 metrics; 0 regressed, improved beyond tolerance, stable +``` + +(57 = pre-edit 63 minus 6 removed entries; verify the actual reported count matches.) + +Final line: `EXIT=0`. + +Any other code is a failure — investigate the per-metric REGRESSION rows shown by `bench/check.py` and decide whether to (a) re-run once more (single-run noise) or (b) widen the offending tolerance after fresh investigation. Two consecutive non-zero exits mean the recapture itself was on a noisy run; redo Task 2. + +--- + +### Task 4: Acceptance §2 — synthetic injection on bump_s → REGRESSION fires + +**Files:** `bench/baseline.json` (temporary patch, reverted at end of task). + +- [ ] **Step 1: Capture the current `bump_s` baseline value** + +Run: + +```bash +jq '.throughput.bench_list_sum.bump_s.baseline' bench/baseline.json +``` + +Expected stdout: a single float, the value captured in Task 2 (≈ 0.05 — exact number from the recapture). Save this number; call it `$ORIG`. + +- [ ] **Step 2: Patch the baseline to half its value** + +Run: + +```bash +jq '.throughput.bench_list_sum.bump_s.baseline *= 0.5' bench/baseline.json > /tmp/baseline_injection.json && mv /tmp/baseline_injection.json bench/baseline.json +``` + +Verify the patch landed: + +```bash +jq '.throughput.bench_list_sum.bump_s.baseline' bench/baseline.json +``` + +Expected stdout: approximately half of `$ORIG`. + +- [ ] **Step 3: Run check.py against the patched baseline** + +Run: + +```bash +bench/check.py; echo "EXIT=$?" +``` + +Expected stdout includes a row matching this pattern (exact diff and tolerance vary): + +``` +throughput.bench_list_sum.bump_s +<~100>% 10.0% REGRESSION +``` + +And the last line is `EXIT=1`. If `EXIT=0`, the injection did not fire — investigate the gating path; the synthetic injection acceptance check has failed and the iter is BLOCKED. If `EXIT=1` and the row shows REGRESSION on the patched metric, the gate is functioning correctly. + +- [ ] **Step 4: Restore the baseline to `$ORIG`** + +Run: + +```bash +jq --argjson v "$ORIG" '.throughput.bench_list_sum.bump_s.baseline = $v' bench/baseline.json > /tmp/baseline_restored.json && mv /tmp/baseline_restored.json bench/baseline.json +``` + +Where `$ORIG` is the value from Step 1. Verify: + +```bash +jq '.throughput.bench_list_sum.bump_s.baseline' bench/baseline.json +``` + +Expected stdout: `$ORIG` (the pre-patch value, recaptured in Task 2). + +- [ ] **Step 5: Final replay to confirm restoration** + +Run: + +```bash +bench/check.py; echo "EXIT=$?" +``` + +Expected last line: `EXIT=0` and `0 regressed` in the summary. Confirms the synthetic injection was fully reverted. + +--- + +## Closing notes for `implement` + +This is a one-task-conceptually iter split into four task-templates +for review granularity (each Task is one self-contained +spec-compliance unit). All four Tasks land in a single working-tree +diff (only `bench/baseline.json` is modified). Boss commits the +whole thing as one iter-level commit at the end. + +Acceptance criteria from the spec are mapped to tasks: +- Spec §"max_us / p99_9_us removed" → Task 1. +- Spec §"captured field reads 2026-05-20" → Task 2. +- Spec §"replay exit 0" → Task 3. +- Spec §"synthetic injection fires REGRESSION" → Task 4.