test(control): record the slew limiter in the golden corpus - #799
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c134fd551c
ℹ️ 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".
| live := goldenLiveBatteryW(r) | ||
| if live <= r.Inputs.State.SlewRateW { |
There was a problem hiding this comment.
Verify the charge-block predicates in named records
When these scenarios are regenerated after their inputs drift, this check only verifies that the aggregate live battery power exceeds one slew step. An idle-slot record can become a non-idle plan, the stale-plan record can become fresh, or ChargeBlocked can be removed while the battery remains above the rate, and all three still pass; because replay uses the newly recorded inputs, it will pass too, leaving the safety cases this assertion claims to preserve uncovered. Validate each named record's mode, slot/gate, and relevant blocked battery in addition to its anchor.
Useful? React with 👍 / 👎.
| if r.Inputs.State.SlewRateW <= goldenSlewRealisticRateW { | ||
| bindingAtRealisticRate++ |
There was a problem hiding this comment.
Exclude non-positive rates from realistic coverage
When a regenerated family accidentally uses zero or negative slew rates, every binding record is counted as production-realistic by this comparison. Those values cannot represent deployed configuration: config.applyDefaults turns zero into 3000 W and validation rejects negative values. Such a corpus can therefore satisfy both 60-record gates without exercising any rate a site actually runs; require a positive lower bound as well as the 3000 W ceiling.
Useful? React with 👍 / 👎.
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>
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>
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>
63bb04e to
6f6bab9
Compare
The limiter anchors every target on the battery's MEASURED output, not on the previous command. That is deliberate and right — it lets dispatch pivot the instant a setpoint reverses instead of ramping away from a stale command. But it also means a battery physically charging drags its own command back toward that charge, whatever the tick above decided. noSelfCharge pins the fleet total to 0 W. The limiter then walks it back. Nothing downstream put it right: applyFuseGuard only shrinks toward zero, floorNegativeTargets covers the discharge side only, and planSignIntent reports "idle, no opinion" for an idle slot. A passive-arbitrage idle slot, meter at -2000 W, battery live at +2000 W, SlewRateW 500, commanded +1500 W of charging on the tick whose entire purpose was to let that surplus reach the meter. The same mechanism re-opened a battery that had just reported charge_capable=false, after the distributor had already handed its share to a capable sibling — the fleet charged that share twice. The fix is a floor, floorBlockedCharge, applied after slew beside the discharge-side floor it mirrors. It bounds the output rather than enumerating the reasons the output was closed. The alternative was to complete the snap-to-zero carve-out inside the slew loop, which today names plannerSelfExportSurplusGate and manual hold. That is the enumeration, and it is how this bug was born: two charge gates were added to ComputeDispatch afterwards and neither was added to the list. It is also weaker than it looks — it only fires when the pre-slew target is already within 1 W of zero, so a total pinned to 0 by a +800/-800 split would slip through it in both directions. A floor cannot be born that way and cannot be split that way. Placed after applyFuseGuard rather than before it, so the guard's predicted grid is unchanged and the blast radius is exactly "targets that command charge into a closed direction". Nothing between the floor and the driver raises charge: applyBatteryBoostReserve touches negative targets only, and forceFuseDischarge only forces discharge. The discharge-side twin of the per-driver half is deliberately left alone. A dischargeBlocked battery re-opened by slew is the same shape, but flooring it has to answer whether the fuse emergency in forceFuseDischarge outranks a driver's "I cannot discharge" — a question this change does not need to answer, because charge has no such override. Every new test runs at a rate a site actually runs (250-1500 W) with the battery measured mid-charge; at SlewRateW 100000, the way most older dispatch tests neutralise the limiter, all four bug tests pass against the broken code. The golden corpus deliberately recorded this bug in #799. It moves 11 records and is re-recorded in the next commit, so the fix and the fixture update read apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Predicted before looking, verified after. Eleven records move, all in the slew_limiter family, all the same law: a target the tick had closed, walked back open by the limiter's measured-power anchor. The eight the corpus named as bugs when #799 recorded them: | record | before | after | |---|---|---| | bug_no_self_charge_idle_slot_leak_250 | +1750 W | 0 W | | bug_no_self_charge_idle_slot_leak_500 | +1500 W | 0 W | | bug_no_self_charge_idle_slot_leak_1500 | +500 W | 0 W | | bug_no_self_charge_idle_slot_leak_with_pv | +2000 W | 0 W | | bug_no_self_charge_arbitrage_idle_slot_leak | +1500 W | 0 W | | bug_no_self_charge_idle_slot_leak_two_batteries | +1500 / +1000 W | 0 / 0 W | | bug_no_self_charge_stale_plan_leak | +2000 W | 0 W | | blocked_sibling_slew_reopens_parked_charge | +1500 / +500 W | 0 / +500 W | The first seven are the site-wide charge block: the fleet total was already pinned to 0 W, so 0 W is the answer the tick computed and the limiter overwrote. The eighth is per-driver: ferroamp reported charge_capable=false and goes to 0; sungrow keeps +500 W, which is its own legitimate one-step ramp from a 0 W anchor and not something anything closed. Three more were predicted by shape rather than by name — the seeded half of the family, where the same gates fire on randomly generated sites: | record | before | after | why | |---|---|---|---| | seeded_039_planner_arbitrage | +3000 W | 0 W | idle arbitrage slot, meter exporting 12.6 kW, battery measured +5377 W | | seeded_068_planner_self | +2525 W | 0 W | stale plan, battery measured +4025 W | | seeded_078_planner_passive_arbitrage | +600 / +787 W | 0 / 0 W | idle slot (28 Wh), meter exporting 3.1 kW, both batteries measured charging | Every one of the eleven is a tick where the site charged while the meter exported, or charged hardware that said it could not. None moved that was not predicted. The records predicted NOT to move did not: carveout_export_surplus_gate_snaps_to_zero and its discharge twin are already 0 W — they are the A/B partners whose difference from the bug rows WAS the bug, and they stay put. no_self_charge_idle_slot_slew_3000_reaches_zero stays at 0 W: a 3000 W rate already reached zero in one step, which is why it never leaked. blocked_sibling_slew_reopens_parked_target stays at -1500 W — the discharge-side twin this change deliberately does not touch. The seven other families are NOT re-recorded. None of their records moved, and rewriting their ftw_commit would claim they were re-examined under this change when they were not. Re-recording them does churn ~150 lines of sub-tolerance float noise (e.g. 3704.0000041155554 -> 3704), which is worth knowing about but is not this commit's business. One record inside slew_limiter changes without moving: seeded_023_planner_arbitrage's derived battery_target_sum_w goes from 235.00000000000006 to 235 while its three per-driver targets stay byte-identical. Isolated by re-dumping with the floor disabled: the shift is already present on this branch's base and belongs to the fuse-relief refactor below it, 6e-14 W, fourteen orders of magnitude under the 0.01 W tolerance. 590 records in 8 families, unchanged. Tolerance unchanged at 0.01 W. slew_limiter.json sha256 f7ff7a54bc49321a5c232d58eed9230b2e25e4b3b7474544e32b46bf0b78885b Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…809) * fix(control): the slew limiter may not re-open a closed charge block The limiter anchors every target on the battery's MEASURED output, not on the previous command. That is deliberate and right — it lets dispatch pivot the instant a setpoint reverses instead of ramping away from a stale command. But it also means a battery physically charging drags its own command back toward that charge, whatever the tick above decided. noSelfCharge pins the fleet total to 0 W. The limiter then walks it back. Nothing downstream put it right: applyFuseGuard only shrinks toward zero, floorNegativeTargets covers the discharge side only, and planSignIntent reports "idle, no opinion" for an idle slot. A passive-arbitrage idle slot, meter at -2000 W, battery live at +2000 W, SlewRateW 500, commanded +1500 W of charging on the tick whose entire purpose was to let that surplus reach the meter. The same mechanism re-opened a battery that had just reported charge_capable=false, after the distributor had already handed its share to a capable sibling — the fleet charged that share twice. The fix is a floor, floorBlockedCharge, applied after slew beside the discharge-side floor it mirrors. It bounds the output rather than enumerating the reasons the output was closed. The alternative was to complete the snap-to-zero carve-out inside the slew loop, which today names plannerSelfExportSurplusGate and manual hold. That is the enumeration, and it is how this bug was born: two charge gates were added to ComputeDispatch afterwards and neither was added to the list. It is also weaker than it looks — it only fires when the pre-slew target is already within 1 W of zero, so a total pinned to 0 by a +800/-800 split would slip through it in both directions. A floor cannot be born that way and cannot be split that way. Placed after applyFuseGuard rather than before it, so the guard's predicted grid is unchanged and the blast radius is exactly "targets that command charge into a closed direction". Nothing between the floor and the driver raises charge: applyBatteryBoostReserve touches negative targets only, and forceFuseDischarge only forces discharge. The discharge-side twin of the per-driver half is deliberately left alone. A dischargeBlocked battery re-opened by slew is the same shape, but flooring it has to answer whether the fuse emergency in forceFuseDischarge outranks a driver's "I cannot discharge" — a question this change does not need to answer, because charge has no such override. Every new test runs at a rate a site actually runs (250-1500 W) with the battery measured mid-charge; at SlewRateW 100000, the way most older dispatch tests neutralise the limiter, all four bug tests pass against the broken code. The golden corpus deliberately recorded this bug in #799. It moves 11 records and is re-recorded in the next commit, so the fix and the fixture update read apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(control): re-record the 11 slew records the charge floor moves Predicted before looking, verified after. Eleven records move, all in the slew_limiter family, all the same law: a target the tick had closed, walked back open by the limiter's measured-power anchor. The eight the corpus named as bugs when #799 recorded them: | record | before | after | |---|---|---| | bug_no_self_charge_idle_slot_leak_250 | +1750 W | 0 W | | bug_no_self_charge_idle_slot_leak_500 | +1500 W | 0 W | | bug_no_self_charge_idle_slot_leak_1500 | +500 W | 0 W | | bug_no_self_charge_idle_slot_leak_with_pv | +2000 W | 0 W | | bug_no_self_charge_arbitrage_idle_slot_leak | +1500 W | 0 W | | bug_no_self_charge_idle_slot_leak_two_batteries | +1500 / +1000 W | 0 / 0 W | | bug_no_self_charge_stale_plan_leak | +2000 W | 0 W | | blocked_sibling_slew_reopens_parked_charge | +1500 / +500 W | 0 / +500 W | The first seven are the site-wide charge block: the fleet total was already pinned to 0 W, so 0 W is the answer the tick computed and the limiter overwrote. The eighth is per-driver: ferroamp reported charge_capable=false and goes to 0; sungrow keeps +500 W, which is its own legitimate one-step ramp from a 0 W anchor and not something anything closed. Three more were predicted by shape rather than by name — the seeded half of the family, where the same gates fire on randomly generated sites: | record | before | after | why | |---|---|---|---| | seeded_039_planner_arbitrage | +3000 W | 0 W | idle arbitrage slot, meter exporting 12.6 kW, battery measured +5377 W | | seeded_068_planner_self | +2525 W | 0 W | stale plan, battery measured +4025 W | | seeded_078_planner_passive_arbitrage | +600 / +787 W | 0 / 0 W | idle slot (28 Wh), meter exporting 3.1 kW, both batteries measured charging | Every one of the eleven is a tick where the site charged while the meter exported, or charged hardware that said it could not. None moved that was not predicted. The records predicted NOT to move did not: carveout_export_surplus_gate_snaps_to_zero and its discharge twin are already 0 W — they are the A/B partners whose difference from the bug rows WAS the bug, and they stay put. no_self_charge_idle_slot_slew_3000_reaches_zero stays at 0 W: a 3000 W rate already reached zero in one step, which is why it never leaked. blocked_sibling_slew_reopens_parked_target stays at -1500 W — the discharge-side twin this change deliberately does not touch. The seven other families are NOT re-recorded. None of their records moved, and rewriting their ftw_commit would claim they were re-examined under this change when they were not. Re-recording them does churn ~150 lines of sub-tolerance float noise (e.g. 3704.0000041155554 -> 3704), which is worth knowing about but is not this commit's business. One record inside slew_limiter changes without moving: seeded_023_planner_arbitrage's derived battery_target_sum_w goes from 235.00000000000006 to 235 while its three per-driver targets stay byte-identical. Isolated by re-dumping with the floor disabled: the shift is already present on this branch's base and belongs to the fuse-relief refactor below it, 6e-14 W, fourteen orders of magnitude under the 0.01 W tolerance. 590 records in 8 families, unchanged. Tolerance unchanged at 0.01 W. slew_limiter.json sha256 f7ff7a54bc49321a5c232d58eed9230b2e25e4b3b7474544e32b46bf0b78885b Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
What this is
A new golden family,
slew_limiter, recorded at slew rates a site actuallyruns, plus a coverage assertion that fails when the corpus stops covering the
slew limiter. Test-only: no production code moves.
Why
The corpus that landed in #790 is blind exactly where the slew limiter lives.
391 of its 435 records set
SlewRateWto 10 kW or 100 kW, against acontrol.NewStatedefault of 500 W and a config default of 3000 W. At thoserates no realistic per-tick move reaches the bound, so the limiter is switched
off in all but name. Measured on master before this PR: switching the limiter
off changes only 16 of 435 records by more than 50 W, and 14 of those 16
are incidental — one seeded family that happened to draw a low rate.
That matters more for this clamp than for the others, because the slew limiter
is the last stage in dispatch that can raise a target's magnitude. Every
other clamp shrinks toward zero. It also anchors on the battery's measured
output (
SmoothedW) rather than the previous command, so a target of 0 Wbecomes ±
SlewRateWwhenever the battery is not already still.KNOWN BUG — these records capture today's behaviour, not desired behaviour
The
slew/bug_*andslew/blocked_sibling_*records show dispatch commandingcharge on a tick that forbids charging:
noSelfChargepins the fleet total to 0 W indispatch.go; the slew limiterthen re-anchors on the measured +2000 W and walks one 500 W step back toward
it. Nothing downstream floors that: the snap-to-zero carve-out in the slew loop
covers
plannerSelfExportSurplusGateand the zero-power manual hold but notthe stale-plan block or the arbitrage-family live-export gate,
applyFuseGuardonly shrinks toward zero,
floorNegativeTargetsis discharge-only, andplanSignIntentreturns 0 for an idle slot. The mirror-image leak on thedischarge side is caught, by
floorNegativeTargets— the asymmetry is thebug.
The corpus records behaviour as of this commit, clamps and quirks and all. When
the fix lands, these records move, and the diff is the fix stated in watts.
That is why they are here. The family's doc comment in
golden_dump_test.gosays the same thing to whoever reads it next, and the coverage assertion
deliberately asserts the shape of those records (a charge block armed, a live
battery on the charge side, a rate one step cannot cover) and not their watts,
so the fix does not have to argue with the test.
Recorded, verbatim, at 500 W:
The last line is the same gate with the carve-out in scope: 0 W, no leak. The
difference between those two rows is the whole bug.
What the family covers
155 records — 50 named, 105 seeded from fixed seeds.
anchoring records run as a ladder across five of them, so a change to the
bound itself reads as a clean progression.
SlewEnabled=falsecontrol pair that says what the tick would have been.small enough that the limiter never binds.
the bug above, and the discharge side that is floored.
forceFuseDischargeruns after thelimiter for a reason; the recorded relief is 3960 W from the anchor on a
250 W/tick ramp. Also the per-phase and export-ceiling variants.
identical tick, where it does not.
MaxCommandW.weighted siblings each get a record.
The coverage gate
TestGoldenCorpusCoveragenow replays every record twice, once as recorded andonce with
SlewEnabled=false, and counts the records where the answer moves bymore than 50 W.
SlewEnabled=falseskips exactly the ramp clamp and nothingelse, so the difference is the limiter's contribution after every downstream
stage has had its say — which is why it is measured rather than stored. A
record where the limiter binds and
forceFuseDischargethen overwrites it doesnot count, and should not.
The floor is 60. Doctoring the new family's rates back to 100 kW — the #790
blindness, reproduced — drops the count to 16 and fails:
Verification
Rebased onto #801, which pins the golden scenario clock, and the family
re-recorded through that seam at 8fe82e4. It was first recorded before the
seam, so its energy-path targets still carried wall-clock jitter; the other
seven families keep their earlier recordings, which is why the 0.01 W tolerance
stays.
Re-recording changes no behaviour. Comparing the two recordings record by
record: no clamp attribution, flag or driver set differs. 26 numeric values
in 9 records move, every one of them a planner mode on the energy path, the
largest by 4.5e-06 W — three orders of magnitude under the tolerance.
Nothing moved by more than 0.01 W.
The bug records are unchanged.
bug_no_self_charge_idle_slot_leak_500still commands +1500 W of charging on a tick that forbids it,
bug_no_self_charge_stale_plan_leak+2000 W,blocked_sibling_slew_reopens_parked_charge+1500 W on ferroamp and +500 Won its sibling, and the A/B partner
carveout_export_surplus_gate_snaps_to_zerostill lands at 0 W on the sameshape.
Deterministic. All 155 records replay clean,
-count=3and under-race. Three fresh recordings differ by at most 6e-14 W, from the orderthe recorder sums per-driver targets before sorting them — the same residual
the seven older families carry, and not what the tolerance is for. Before the
seam the family drifted by ~1e-5 W run to run.
Coverage holds.
slew coverage: 123 records bind (>50 W), 123 of them at a production-realistic rate (<= 3000 W/tick); largest effect 6345 W on slew/seeded_034_priority.The gate still bites. Two perturbations against the re-recorded corpus,
both reverted:
Slew bound loosened 10% (
sign*SlewRateW→sign*SlewRateW*1.1).105 of 590 records flagged — 91 of them in the new family, 14 in the
old corpus. Diff reads, per record:
ferroamp: was +2750.00 W, now +2725.00 W (moved -25.00 W).The family's rates doctored back to 100 kW — the test(control): golden regression corpus for ComputeDispatch #790 blindness,
reproduced.
assertGoldenSlewCoveragefails and names what was lost:make verifyclean.Notes for review
FTW_GOLDEN_OUTand copied in, so theother seven families are untouched — a full re-record would rewrite their
ftw_commitand jitter their energy-path magnitudes for no reason.Regeneration is still the one documented command.
ftw_commitis laterthan theirs. The corpus header used to name one commit for the whole corpus;
it now points at each file's own.
no-changesetlabel, same as test(control): golden regression corpus for ComputeDispatch #790.