test(control): golden regression corpus for ComputeDispatch - #790
Conversation
Adds 435 recorded dispatch ticks under go/internal/control/testdata/golden/ and a test that replays every one through the real ComputeDispatch. Zero behaviour change: nothing outside _test.go and testdata/ is touched. ComputeDispatch is where the plan, the fuse, the sibling batteries, the EV and the reserves all meet, and its unit tests pin the cases somebody thought to write down. That leaves the interactions unguarded. The corpus covers seven families — the reactive modes, all four planner modes, fuse pressure in both directions on 16 A and 25 A sites, degraded siblings, EV and boost reserves, and the incident scenarios forecast_scenarios_test.go already names — and records the per-driver targets, clamp attributions and site totals each one produced. The corpus is what dispatch DID at c7fe6c9, not what it should do. Nothing in it was reviewed as correct. Its job is to make a change to dispatch visible in watts, per scenario, so a refactor states its own blast radius. A failure is therefore a question, not a verdict: re-record deliberately with FTW_GOLDEN_DUMP=1 when the change was intended, and read the diff. Comparison is 0.01 W absolute, never exact equality. Energy-path targets are Wh x 3600 / seconds-remaining against a wall-clock-anchored slot, so they carry sub-millisecond jitter; measured run-to-run drift across the whole corpus is under 1.3e-5 W. The recorder was written outside this repo while evaluating a Rust successor to FTW, against this same ComputeDispatch. That experiment is over and the work comes home; this is the part worth keeping. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dae57c6694
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !slot.Present { | ||
| return SlotDirective{}, false | ||
| } | ||
| now := time.Now() |
There was a problem hiding this comment.
Freeze the clock used by golden planner scenarios
On a loaded test runner, this wall-clock read can be separated from ComputeDispatch's later time.Now() by an arbitrary scheduling pause, changing the calculated remainingS and therefore the energy-path targets by more than the 0.01 W tolerance without any code change. I reproduced this by running the replay repeatedly on a contended single CPU; scenarios such as seeded_planner/095_planner_arbitrage intermittently moved by 0.05 W. Freeze/inject the scenario clock, or otherwise make the comparison independent of intra-call scheduling latency, so this regression gate does not flake under CI contention.
Useful? React with 👍 / 👎.
|
Status: PAUSED. No golden-clock fix PR has been created or merged. Local worktree /Users/fredde/.codex/worktrees/ec25/forty-two-watts is detached at 31232f2, the current master base, with uncommitted changes in go/internal/control/dispatch.go, go/internal/control/golden_dump_test.go, and go/internal/control/golden_test.go. Passed: go test ./internal/control -count=1; golden corpus/coverage stress (-count=100); golden race (-race -count=10); and isolated make verify-all with Python 3.12.13 and a materialized 37-driver snapshot. Remaining: read-only scratch merge-tree plus golden regression against #608 head 8f19c7a, followed by independent review. #608 and head 8f19c7a were not changed. |
The corpus that landed in #790 runs 391 of its 435 records at SlewRateW 10 kW or 100 kW. At those rates no realistic per-tick move reaches the bound, so the slew limiter was switched off in all but name: only 16 records change at all when it is disabled, and the gate could not see its interactions. Adds slew_limiter, a 155-record family recorded at rates a site runs — 500 W (control.NewState's default), 3000 W (the config default) and the 250-2000 W band in between. It covers anchoring on measured output in both directions, reversals across zero, the charge and discharge blocks with the battery live on the wrong side, fuse relief out-ranking the ramp, the post-slew re-clamp, and the snap-to-zero carve-out where it applies and where it does not. The bug_* records capture a known bug's behaviour as of this commit, not desired behaviour: noSelfCharge pins the fleet total to 0 W and the limiter then walks one step back toward the battery's live charging power, so a passive_arbitrage idle slot with the meter at -2000 W and the battery live at +2000 W commands +1500 W of charging on a tick that forbids charging. The fix will move these records, which is the point of recording them. TestGoldenCorpusCoverage now measures, for every record, how far the targets move when the limiter is opted out, and fails when fewer than 60 records bind by more than 50 W. A re-recording that drifts back to unreachable slew rates fails CI instead of passing quietly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(control): record the slew limiter in the golden corpus The corpus that landed in #790 runs 391 of its 435 records at SlewRateW 10 kW or 100 kW. At those rates no realistic per-tick move reaches the bound, so the slew limiter was switched off in all but name: only 16 records change at all when it is disabled, and the gate could not see its interactions. Adds slew_limiter, a 155-record family recorded at rates a site runs — 500 W (control.NewState's default), 3000 W (the config default) and the 250-2000 W band in between. It covers anchoring on measured output in both directions, reversals across zero, the charge and discharge blocks with the battery live on the wrong side, fuse relief out-ranking the ramp, the post-slew re-clamp, and the snap-to-zero carve-out where it applies and where it does not. The bug_* records capture a known bug's behaviour as of this commit, not desired behaviour: noSelfCharge pins the fleet total to 0 W and the limiter then walks one step back toward the battery's live charging power, so a passive_arbitrage idle slot with the meter at -2000 W and the battery live at +2000 W commands +1500 W of charging on a tick that forbids charging. The fix will move these records, which is the point of recording them. TestGoldenCorpusCoverage now measures, for every record, how far the targets move when the limiter is opted out, and fails when fewer than 60 records bind by more than 50 W. A re-recording that drifts back to unreachable slew rates fails CI instead of passing quietly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(control): say precisely which stage enlarges a target forceFuseDischarge also enlarges a target, so "the last stage that can invent power" was wrong about the pipeline. What is true of the slew limiter alone is that it enlarges one without a safety reason to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(control): re-record the slew family through the golden clock seam The 155 slew records were recorded before #801 pinned the golden scenario clock, so their energy-path targets still carried the wall-clock jitter that seam removes. Re-recorded against 8fe82e4, where the slot directive and the dispatch calculation read one instant. Nothing about the family's behaviour moves. Comparing the two recordings record by record: no clamp attribution, flag or driver set differs, and the only numeric changes are 26 values in 9 records, all of them planner modes on the energy path, the largest 4.5e-06 W — three orders of magnitude under the 0.01 W replay tolerance. The bug records the family exists for are unchanged: bug_no_self_charge_idle_slot_leak_500 still commands +1500 W of charging on a tick that forbids it, and carveout_export_surplus_gate_snaps_to_zero still lands at 0 W on the same shape. Run to run, the re-recorded family now varies by at most 6e-14 W, from the order the recorder sums per-driver targets. That residual is the same in the seven older families and is not what the tolerance is for. Also correct the corpus header: it named one ftw_commit for the whole corpus, which stopped being true when a family was recorded later than the rest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
) * test(control): record the early exits meeting a binding protection Three branches of ComputeDispatch walk away from a cycle before the safety pipeline: idle mode, the holdoff window, and the reactive deadband. The golden corpus had no record of any of them meeting a protection that binds. Of the 590 records, 33 returned no targets — 25 idle, 1 holdoff, 8 deadband — and every one of the 8 deadband records ran with site_fuse_amps=0 and peak_import_ceiling_w=0. Nothing was over any limit to defend, so 590 recorded ticks could not see the deadband exit skipping every protection. That is the bug #803 fixed, and #803 left this gap open on purpose: recording new records is a corpus change, not a fix. early_exit_protections adds 21 records, 611 in total. Each shape appears twice — once where the protection binds, once where the same site is configured identically and nothing is over its limit. The quiet half is not filler: a corpus of only-firing records cannot tell a fix from an over-fire, and a fuse-saver that discharges on every tick drains the pack as surely as one that never fires trips the breaker. Covered: deadband against a tariff peak ceiling, against a fuse ceiling left above the breaker by a misconfigured peak limit, against a phase over the breaker on three-phase and on single-phase services, with a safety-amp margin, split across two batteries by discharge headroom, denied by a pack under the 5 % floor, and reached through a stale plan's reactive fallback; idle and holdoff against the peak ceiling and against both phase counts. One record is an export-side phase 10 A over the breaker that must stay quiet, because forceFuseDischarge's only lever is more discharge and honouring per-phase relief there would push that phase further over. Single-phase sites get their own records because the phase count is the conversion #812 gave one owner: on one phase the aggregate meter and the phase are the same wire, so relief is the overage once. Under the bare `* 3.0` those records would command three times the overage. This family records the law as fixed, not a bug. slew_limiter deliberately holds a known defect still so its fix reads as a diff in watts; these records were taken after #803 landed and state what dispatch is supposed to do, so a record that moves is a protection that stopped protecting. The family doc comment says so. Verified against the un-fixed code. Reverting #803's dispatch.go hunk locally moves 7 of the 611 records, all of them deadband records where a protection binds, each reading as the commanded discharge disappearing — deadband_per_phase_3p_binds goes from -1380 W clamped to no targets. The 6 quiet deadband records and all 8 idle and holdoff records hold still, because idle and holdoff already ran the fuse-saver before #803. Re-recording to silence the failure does not work either: the coverage assertions fail on what was just written, naming the deadband exit as unguarded again. The eight existing families are byte-identical; only the new file is added. A full re-recording on this machine rewrites 27 of the 590 old records with float noise up to 8.5 microwatts — a thousandth of the 0.01 W tolerance, and nothing a reviewer should have to read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(control): point slew_limiter.json at the commit it was recorded on The family carried ftw_commit fe2c904, a SHA that is not on master and never was. The other eight families name real commits — seven at c7fe6c9, where #790 recorded them, and the new one at c85bfe1. fe2c904 is the pre-squash tip of #809's branch: `git cat-file` finds it locally with #809's subject line, and `git merge-base --is-ancestor fe2c904 origin/master` says it is not an ancestor of master. GitHub squash-merged that branch as c85bfe1, and `git log -- testdata/golden/slew_limiter.json` shows c85bfe1 as the last commit to rewrite the file: #809's fix moved 94 lines of records, and they were re-recorded in the branch worktree before the squash. So the behaviour these 155 records hold is the behaviour at c85bfe1, which is what the field now says. No record changes; only the provenance line. It matters because the field is the one thing telling a reader which dispatch produced these watts, and a SHA that resolves nowhere on master sends anyone tracing the file's history to a dead end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
What this is
435 recorded ticks of
ComputeDispatch, committed undergo/internal/control/testdata/golden/, and a test that replays every one ofthem through the real function and reports what moved.
Zero behaviour change. Nothing outside
_test.gofiles andtestdata/istouched, so this conflicts with nothing currently open — including #608, which
has right of way on
dispatch.goandcontrol_test.go.Why
ComputeDispatchis where the plan, the fuse, the sibling batteries, the EVand the reserves all meet. Its unit tests pin the cases somebody thought to
write down; the interactions between them are unguarded. Today a refactor of
dispatch is reviewed by reading control flow and hoping. This PR replaces the
hoping with a number.
The corpus covers seven families:
incident_scenariosforecast_scenarios_test.goalready names (A1–A6, B7–B12, E22–E25, F26–F30): forecast misses, mode interactions, fuse invariants, SoC floor and ceiling, holdoff, offline sibling, stale plan, EV covertargeted_invariantsforceFuseDischarge(idle and holdoff-override), per-phase overage both directions, plan/exec sign floor both directions, boost-reserve floor and headroom clamp, DC-locality charge routing, joint fuse+EV allocator,MaxExportWandPeakImportCeilingWceilings, stale planseeded_reactiveseeded_plannerseeded_fuseseeded_siblingsseeded_ev_boostBatteryCoversEVboth ways, surplus-only EV reserve, battery-boost reserve leases around the reserve SoCEach record stores the full inputs to one call — telemetry,
State, slotdirective, driver capacities, fuse ceiling — next to the per-driver targets,
the clamp attributions and the site totals that call returned. An empty
per_driver_targetsis a real outcome, not a gap: deadband hold, idle with nofuse overage, holdoff window.
What the corpus is not
It is what dispatch did at
c7fe6c98(recorded asftw_commitin everyfile), clamps and quirks and all. Nothing in it was reviewed as correct. It
describes behaviour, not intent.
So a failure here is a question, not a verdict. It tells you which scenarios
your change moved and by how many watts:
Read them. If they are the ones you meant to move, and they moved the way you
meant, re-record and commit the new corpus with your change:
Then the diff on
testdata/golden/is your behaviour change, stated in watts,and a reviewer sees it instead of inferring it. If scenarios you did not expect
also moved, you found something before a user did.
Two things never fix a failure: deleting the record and widening the tolerance.
Tolerance
0.01 W absolute, never exact equality. Energy-path targets are
Wh × 3600 / seconds-remainingagainst a slot anchored to the wall clock, sothey carry sub-millisecond jitter. Measured run-to-run drift across the whole
corpus is under 1.3e-5 W with no flag flips — four orders of magnitude inside
the tolerance. Anything above 0.01 W is a real change.
Verification
was recorded on 2026-08-03 at 16:31 and replays clean on 2026-08-04 at a
different hour, which also rules out a time-of-day dependence.
-count=10and-race: clean.make verify: clean.reverting. Scaling
effectiveExportCeilingWby 0.9 lit up every record withthe ceiling moved by 1104 W; widening the reactive deadband 3× caught exactly
three records, naming each battery that stopped being dispatched. Both
reverted;
dispatch.gois untouched in this branch.TestGoldenCorpusCoverageruns on every test run and asserts the corpusstill exercises floored-sibling reallocation,
forceFuseDischarge, per-phaseoverage attribution and the plan/exec sign floor, so a careless re-record
cannot quietly shrink the net. Current counts:
fuse_saturated=9 hold_latched=56 per_phase=32 stale_plan=18 blocked_batteries=24 no_dispatch=30.Honest limits
State(PI integral zero,
PrevTargetsempty). Multi-tick convergence — PI settling,slew ramps over cycles, hold-window decay — is not in this corpus. Adding it
is worth doing and is not this PR.
ComputeDispatchmutates but the record does not store —
PrevTargets, the PI integral, theslot accumulators — is not checked.
scenario is built by the same function that recorded it, so the two cannot
drift apart. The cost is that the corpus cannot catch a bug in the
scaffolding itself, only a change in
ComputeDispatch.it — this is the only thing in the repo that can tell a future engineer what
their dispatch refactor actually did — but it is worth saying out loud rather
than hiding in the diff stat.
Provenance
The recorder was written outside this repo while a Rust successor to FTW was
being evaluated, and it drove this same
ComputeDispatchto produce areference the new allocator could be checked against. That experiment is over
and building on FTW won. This is the part of it worth keeping, and it is worth
more here than it ever was there: over there it described a system being left
behind; here it protects the one being kept.
Release notes
No changeset. Test-only — no shipped behaviour changes, so there is nothing to
put in a changelog. Per
.changeset/README.mdthis needs theno-changesetlabel rather than an empty entry.
🤖 Generated with Claude Code