SAFETY-41: raise the clamp alarm's re-assert 30 s -> 150 s #52
Merged
admin
merged 4 commits from 2026-09-05 18:31:31 +02:00
SAFETY-41-reassert into release/1.0
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 glenn schrooyenandClaude Opus 5
|
3527c8a04c |
SAFETY-41: escalate a sustained engagement, so a runaway is never held
checks / unit (pull_request) Successful in 4m59s
Review found the case I asked it to construct, and it is not a corner case. The rate limit alone DELAYS A RUNAWAY: a benign one-cycle clamp that spends the budget, then a continuous §10.3-shaped runaway, held the runaway's first notification for up to 140 s - one REASSERT_S minus a cycle. On the live install, where ordinary railing spends the budget essentially all the time, a runaway arriving during normal operation would be delayed with near-certainty. That is the "benign noise suppresses the bug signal" shape this module's docstring is written against, and the family the first attempt at this card died of. THE ESCALATION. An episode that has not yet spoken because an EARLIER, unrelated episode spent the budget speaks anyway once it has engaged SUSTAINED_CYCLES = 3 times in a row. The discriminator is run length and the two populations genuinely differ: the live flood was runs of length ONE (engage, de-assert, engage), while a runaway engages every cycle. Measured, and it costs nothing it was protecting: live scatter flood (305 cycles, 1-in-6) 11 -> 11 notifications chatter 1-engaged/3-clean over 8000 s 50 -> 50 notifications pinned 900 s quarter (floor is 5) 6 -> 6 alarms runaway delay after a benign episode 140 s -> 10 s ⚠️ MY COUNTERFACTUAL CHECK COULD NOT FAIL FOR THE DEFECT IT NAMED, and review caught that too. It read `ClampWatchdog().note(True, ...) is True` - a FRESH object, which only exercises the `last_alarm is None` branch, the trivially-true one. A fresh watchdog has no earlier episode to be held back BY. It now SPENDS THE BUDGET FIRST: benign clamp, gap past CLEAR_CYCLES, then a continuous runaway, and asserts the delay is well under REASSERT_S. That is the third instance in this file of the family recorded at line 216 and at the 224 s sweep, and it was guarding the most safety-critical property in the change. Also pinned: `last_alarm` SURVIVING the de-assert. Review built the mutation - if the de-assert also refreshed it, chatter collapses from 50 notifications to 1 and the alarm never fires again, with the suite green. That is the "alarm that never fires" class, a different bug from one that fires later. Now caught. Both new checks verified against the mutations they name: removing the escalation -> 1 FAILED; refreshing last_alarm on de-assert -> 1 FAILED. Full suite: 21 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU |
||
|
glenn schrooyenandClaude Opus 5
|
8790c4d38f |
SAFETY-41: rate-limit the RISING EDGE, which is where the flood came from
checks / unit (pull_request) Successful in 5m9s
Review falsified the first version of this change, and the correction is the substance of it. Raising REASSERT_S 30 -> 150 removed NONE of the owner's notifications. The re-assert interval governs only the still-engaged path; the rising edge fired unconditionally. At the measured ~5 s cycle the live pattern - 48 engagements in ~25 minutes, about one cycle in six - leaves ~25 s of clean cycles between engagements, past CLEAR_CYCLES x 5 s = 15 s. So the alarm DE-ASSERTED between almost every engagement and each one returned as a fresh rising edge. Measured on the shipped class over that window: 48 episodes, 48 notifications at REASSERT_S = 30, and still 48 at 150. The interval now bounds the NOTIFICATION RATE rather than the repeat rate within one episode: `last_alarm` survives a de-assert, and a new episode speaks only if the interval has elapsed. Same window afterwards: 51 episodes, 51 engagements, 11 notifications. ⚠️ NOTHING IS DROPPED, AND THE TWO PROPERTIES THAT MATTER ARE PINNED: - a PINNED loop still speaks 6 times in the 900 s billing quarter, against a floor of 5 (part 2, unchanged); - a rising edge from a COLD watchdog still fires IMMEDIATELY, so a runaway beginning from quiet is never held back by a budget spent on an earlier, unrelated episode. That check is the counterfactual and it is the half that would go red if the limit ignored elapsed silence. Every engagement is still counted, every episode still increments, and the action-log row is still written on every engaged cycle. The budget is on notifications, never on evidence. The new pin is written in wall-clock seconds and in the shape the field produced, so it does not move when the constant does. Verified by reverting the rising edge to unconditional: 2 checks go RED naming it. Full suite: 21 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU |
||
|
glenn schrooyenandClaude Opus 5
|
1d393ced22 |
SAFETY-41: correct the comment, and fix the check it cites
Review found two false statements in the comment and one weak check. All three are fixed here; the constant itself is unchanged at 150 s and was never in question. 1. "That caps this constant at 900/5 = 180 s" was FALSE ABOUT THE CHECK IT CITED. test_clamp.py part 2 sweeps i = 0..899 and the rising edge at i=0 is free, so it measured `1 + floor(899/R)` - one alarm more than a real quarter earns. Measured: 224 s was GREEN across the whole suite. Anyone raising this to 200 s would have had a green board and a requirement quietly unmet - the same defect recorded at test_clamp.py:216, one notch weaker. The check now grades floor(QUARTER_S / REASSERT_S) directly, so it enforces the ceiling the comment claims for it. Verified: 150 PASS, 180 PASS, 181 FAIL, 224 FAIL. 180 is also the true requirement edge - on the worst sliding 900 s window 180 earns 5 alarms and 181 earns 4. 2. The "60 cycles of the §10.3 runaway" note was attributed to this module and truncated. It is main.py:2479 and reads "60 cycles of the §10.3 runaway produce 0 write-path engagements and 2 loop-side ones". Quoted in full, attributed correctly. 3. "48 engagements in 25 minutes" was left implying a pinned rail. It is roughly one cycle in six - real, and far above the 2-in-60 the guard was sized for, but not continuous. Restated, and labelled as a live-log reading rather than a measurement in this repo. Also recorded: the 1500 W tuning derivation is in the OTHER repo (admin/goodwe NOTES.md, "Simulated 2 kW load step (gain 0.6, clamp 1500)"), so this repo cannot corroborate it and a reader should not go looking here; and MIN_PER_QUARTER = 5 is a test-local integer, not a spec figure - the 900 s quarter is system-wide but the 5 is not, so the 180 s ceiling rests on one number a test author chose. Full suite: 20 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU |
||
|
glenn schrooyenandClaude Opus 5
|
3516947b5e |
SAFETY-41: raise the clamp alarm's re-assert 30 s -> 150 s
The owner reported the OUTPUT_CLAMP notification as a persistent annoyance on the live install. It re-asserted every 30 s, all day. ⚠️ THIS IS A REGIME CHANGE, NOT A CHANGE OF MIND ABOUT THE ALARM. 30 s was chosen when a loop-side engagement was rare - clamp.py's own note records "60 cycles of the §10.3 runaway produce 2 loop-side engagements". The live install produced 48 IN 25 MINUTES of ordinary operation, because `max_w` is 5000 W where the loop's tuning was derived at a 1500 W clamp (SAFETY-39). The integrator therefore reaches its bound constantly and the alarm narrated it every 30 s. An alarm that speaks that often about the normal case is how an operator learns to ignore the one that matters, which is the exact failure clamp.py's docstring is written against. ⚠️ 150 s IS THE REQUIREMENT'S OWN EDGE WITH ONE STEP OF MARGIN, not a comfort choice. test_clamp.py part 2 pins the cadence in WALL-CLOCK terms rather than against this constant: a loop pinned on the rail must speak at least MIN_PER_QUARTER = 5 times inside the 900 s quarter that bills the capacity tariff. That caps the constant at 900/5 = 180 s. Measured: 180 s -> 5 alarms per quarter (exactly the required 5, no margin) 150 s -> 6 alarms per quarter 300 s -> 3, and the floor check goes RED 180 s would sit on the boundary, so any later change to the firing logic would tip it red for a reason unrelated to that change. The FLOOR is the thing that must not move, and it does not. ⚠️ WHAT THIS DOES NOT DO. It changes how often the alarm repeats. It does NOT make the alarm correct: the message still calls the integrator bound a BUG SIGNAL and still pairs a request from one moment with a `self.target` read at another. That is SAFETY-40. And it does not stop the loop railing in the first place; that is SAFETY-39. This is a stop-gap and says so at the line. Nothing was suppressed: every engagement is still counted, the action-log row is unchanged, and the §10.3 runaway still raises the alarm - an earlier attempt at this that split the alarm so only `clamped` notified was REVERTED because test_clamp.py:398 proved it would have disabled runaway detection entirely (a runaway presents as `i_clamped`, and control.py:78-81 says `clamped` is unreachable on a default install). Full suite: 20 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU |