SAFETY-40. The clamp alarm now tells normal anti-windup apart from an over-rating, and every number in it comes from the tick that clamped.
State on release/1.0 @ 59415ea: all 3 defects were still present.
main.py:3274-3278 and 3719-3729: i_clamped or clamped got a single text, BUG SIGNAL included.
main.py:3276: "the loop asked for {decision.i_w}". Since SAFETY-48 this is the lead-capped integrator (e.g. +3000 at a +5000 bound).
main.py:3726: "was clamped to {self.target}", i.e. the value after slew or freeze.
Change
clamp.loop_clamp_note(decision): i_clamped now reads "the loop's integrator reached its +5000 W anti-windup bound; the loop's command this cycle was +2000 W. Brief engagements after a large load or solar step are normal. If it persists, check that the battery is following its commands."clamped reads "the loop computed +5400 W, over the 5000 W rating, and it was clamped to +5000 W; …" and keeps the BUG SIGNAL.
Decision gets i_bound_w, requested_w and clamped_w, all set at the clamp.
Policy:i_clamped still counts and notifies, with the same id, cadence and SAFETY-41 escalation. It is logged as a WARNING, shows an amber status row and has a plain title. It notifies because on a default install it is the only loop-side runaway signal. Over-rating keeps ERROR, the red row and the 14 768 W rationale, and gets its own notification budget so it never waits behind anti-windup notices. See DECISIONS/SAFETY-40-….
DOCS.md: the alarm section, plus the stale "30 s" cadence.
Tests:test_clamp.py part 9 runs Controller.cycle() with compute spied. The fixtures make the bound, i_w, self.target and the requested command all differ. On the old code: defect 1 (4 checks), defect 2 (2), defect 3 (1) and the at/above check all go red. Two part-3 checks that pinned the old BUG text for the default-install runaway were rewritten. Mutations killed: shared text, bound taken from i_w, clamped-to taken from the slewed command, every clamp treated as a bug, and anti-windup overwriting the bug text. The full suite is green locally in the checks image.
SAFETY-40. The clamp alarm now tells normal anti-windup apart from an over-rating, and every number in it comes from the tick that clamped.
**State on release/1.0 @ 59415ea: all 3 defects were still present.**
1. `main.py:3274-3278` and `3719-3729`: `i_clamped or clamped` got a single text, BUG SIGNAL included.
2. `main.py:3276`: "the loop asked for {decision.i_w}". Since SAFETY-48 this is the lead-capped integrator (e.g. +3000 at a +5000 bound).
3. `main.py:3726`: "was clamped to {self.target}", i.e. the value after slew or freeze.
**Change**
- `clamp.loop_clamp_note(decision)`: `i_clamped` now reads *"the loop's integrator reached its +5000 W anti-windup bound; the loop's command this cycle was +2000 W. Brief engagements after a large load or solar step are normal. If it persists, check that the battery is following its commands."* `clamped` reads *"the loop computed +5400 W, over the 5000 W rating, and it was clamped to +5000 W; …"* and keeps the BUG SIGNAL.
- `Decision` gets `i_bound_w`, `requested_w` and `clamped_w`, all set at the clamp.
- **Policy:** `i_clamped` still counts and notifies, with the same id, cadence and SAFETY-41 escalation. It is logged as a WARNING, shows an amber status row and has a plain title. It notifies because on a default install it is the only loop-side runaway signal. Over-rating keeps ERROR, the red row and the 14 768 W rationale, and gets its own notification budget so it never waits behind anti-windup notices. See `DECISIONS/SAFETY-40-…`.
- DOCS.md: the alarm section, plus the stale "30 s" cadence.
**Tests:** `test_clamp.py` part 9 runs `Controller.cycle()` with `compute` spied. The fixtures make the bound, `i_w`, `self.target` and the requested command all differ. On the old code: defect 1 (4 checks), defect 2 (2), defect 3 (1) and the at/above check all go red. Two part-3 checks that pinned the old BUG text for the default-install runaway were rewritten. Mutations killed: shared text, bound taken from `i_w`, clamped-to taken from the slewed command, every clamp treated as a bug, and anti-windup overwriting the bug text. The full suite is green locally in the checks image.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
The clamp alarm gave `i_clamped` (normal anti-windup) the over-rating BUG
SIGNAL text, printed `Decision.i_w` as "the loop asked for", and printed
`self.target` (after slew/freeze) as "was clamped to".
- clamp.loop_clamp_note(): i_clamped says "the loop's integrator reached its
N W anti-windup bound" with this tick's command; clamped keeps BUG SIGNAL
with the requested command and the clamp's own result.
- Decision carries i_bound_w / requested_w / clamped_w, set at the clamp.
- i_clamped still counts and notifies (same id, cadence, SAFETY-41
escalation) but as a WARNING, amber row, plain title. A bug has its own
notification budget and its text is not overwritten within an episode.
- test_clamp.py part 9: red on the old code per defect; 5 mutations killed.
- DOCS.md alarm section (and its stale 30 s cadence), DECISIONS/SAFETY-40.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
This review ran test_clamp.py in the CI image (goodwe-addon-checks:1), ran two mutations, and ran a differential fuzz of ClampWatchdog against release/1.0.
Verified, no change needed
Same-tick numbers.i_bound_w, requested_w and clamped_w are taken at the clamp, before slew and freeze. target_w is the same decision's emitted command. The SAFETY-48 lead cap runs after i_bound_w is captured, so it can lower the carried i_w (part 9a prints bound +5000, i_w +3000) without changing the bound the message reports. "Reached its +5000 W bound" is true for that tick.
"at", not "above". The i-clamped text never says above or over. The clamped text says "over", and that is only reached when target != want, so the request really is over the rating.
Cadence when every engagement is over-rating. I fuzzed the new note() (default bug=True) against release/1.0's ClampWatchdog: 3000 seeds × 400 ticks, mixed tick gaps and engagement densities. Return value, reason and active matched on 0 of 1 200 000 differing steps. The existing SAFETY-41 checks also still run on the default bug=True and pass. Part 9's count-only check is weak on its own, but it is not the only guard.
Mutations. (A) Put BUG_SIGNAL back into the i-clamped text: 2 checks go red. (C) Old app/ with the new tests: 12+ checks go red, and nothing crashes.
The part-3 rewrites are justified. On a default install the §10.3 runaway can only show up as i_clamped, so the old "out of range" wording was exactly defect 1. Part 3 still asserts that it notifies, on its own id, as one coalesced episode, with a row per episode and a flagged status row while engaged.
Point 1 (must fix): an anti-windup notice can overwrite a live BUG SIGNAL notification
Both texts use notification_id: goodwe_output_clamp, and nothing ever dismisses it. So a write-path over-rating (bug) notice can be replaced by a later integrator-bound WARNING. The SAFETY-41 escalation (run_len >= 3, not yet spoken) ignores the budget, so this can happen within seconds. The episode lock only protects text within one episode. On the live site i_clamped fires about 250 times a day, so a genuine BUG SIGNAL would stay on screen only until the next sustained anti-windup episode. control.py already says "the two events want different alarms". Fix: give the WARNING its own id (goodwe_integrator_bound). The bug notice then stays until the operator dismisses it. Budgets are unchanged. I will push this as a small reviewer fix, with a test that fails on fd14569.
Point 2 (small): operator text has no pointer
"integrator" and "anti-windup bound" are jargon on a client site. The WARNING notification should point to DOCS "The winding-loop alarm (the output clamp)". I will add one line to the notification body, not to the log reason.
Point 3 (docs): the cadence sentence
"at most one notification per 150 s" is no longer true: a bug has its own budget, and escalation bypasses the budget. I will reword it.
Noted, not blocking
Default install, real runaway. It is still loud enough in my view. It still produces a persistent notification, an amber row while engaged, and a warn row with the count afterwards. It still escalates on a sustained run, and "If it persists, check that the battery is following its commands" is the correct action. On a default install the command itself cannot exceed max_w, so the 14 768 W rationale does not apply to it.
In a mixed episode the bug text is locked in and re-asserted later. Its "this cycle" then refers to the bug tick, which may be minutes old. That is acceptable.
The post-clear row still says "output clamp: ... engaged N time(s)" for integrator-bound engagements. This is a follow-up candidate.
## Review of fd14569 (SAFETY-40)
This review ran `test_clamp.py` in the CI image (`goodwe-addon-checks:1`), ran two mutations, and ran a differential fuzz of `ClampWatchdog` against release/1.0.
### Verified, no change needed
- **Same-tick numbers.** `i_bound_w`, `requested_w` and `clamped_w` are taken at the clamp, before slew and freeze. `target_w` is the same decision's emitted command. The SAFETY-48 lead cap runs after `i_bound_w` is captured, so it can lower the carried `i_w` (part 9a prints bound +5000, i_w +3000) without changing the bound the message reports. "Reached its +5000 W bound" is true for that tick.
- **"at", not "above".** The i-clamped text never says above or over. The clamped text says "over", and that is only reached when `target != want`, so the request really is over the rating.
- **Cadence when every engagement is over-rating.** I fuzzed the new `note()` (default `bug=True`) against release/1.0's `ClampWatchdog`: 3000 seeds × 400 ticks, mixed tick gaps and engagement densities. Return value, `reason` and `active` matched on **0 of 1 200 000** differing steps. The existing SAFETY-41 checks also still run on the default `bug=True` and pass. Part 9's count-only check is weak on its own, but it is not the only guard.
- **Mutations.** (A) Put `BUG_SIGNAL` back into the i-clamped text: 2 checks go red. (C) Old `app/` with the new tests: 12+ checks go red, and nothing crashes.
- **The part-3 rewrites are justified.** On a default install the §10.3 runaway can only show up as `i_clamped`, so the old "out of range" wording was exactly defect 1. Part 3 still asserts that it notifies, on its own id, as one coalesced episode, with a row per episode and a flagged status row while engaged.
### Point 1 (must fix): an anti-windup notice can overwrite a live BUG SIGNAL notification
Both texts use `notification_id: goodwe_output_clamp`, and nothing ever dismisses it. So a write-path over-rating (bug) notice can be **replaced** by a later integrator-bound WARNING. The SAFETY-41 escalation (`run_len >= 3`, not yet spoken) ignores the budget, so this can happen within seconds. The episode lock only protects text *within* one episode. On the live site `i_clamped` fires about 250 times a day, so a genuine BUG SIGNAL would stay on screen only until the next sustained anti-windup episode. `control.py` already says "the two events want different alarms".
**Fix:** give the WARNING its own id (`goodwe_integrator_bound`). The bug notice then stays until the operator dismisses it. Budgets are unchanged. I will push this as a small reviewer fix, with a test that fails on fd14569.
### Point 2 (small): operator text has no pointer
"integrator" and "anti-windup bound" are jargon on a client site. The WARNING notification should point to DOCS "The winding-loop alarm (the output clamp)". I will add one line to the notification body, not to the log reason.
### Point 3 (docs): the cadence sentence
"at most one notification per 150 s" is no longer true: a bug has its own budget, and escalation bypasses the budget. I will reword it.
### Noted, not blocking
- **Default install, real runaway.** It is still loud enough in my view. It still produces a persistent notification, an amber row while engaged, and a warn row with the count afterwards. It still escalates on a sustained run, and "If it persists, check that the battery is following its commands" is the correct action. On a default install the command itself cannot exceed `max_w`, so the 14 768 W rationale does not apply to it.
- In a mixed episode the bug text is locked in and re-asserted later. Its "this cycle" then refers to the bug tick, which may be minutes old. That is acceptable.
- The post-clear row still says "output clamp: ... engaged N time(s)" for integrator-bound engagements. This is a follow-up candidate.
A later anti-windup notice could replace a write-path BUG SIGNAL notice
under the shared goodwe_output_clamp id (escalation ignores the budget).
The WARNING now uses goodwe_integrator_bound and points to DOCS; the
cadence sentence in DOCS is corrected. Tests pin both ids (red on fd14569).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-up: points 1-3 fixed in 55a6d5d (reviewer push)
Point 1, fixed. The integrator-bound WARNING now uses its own notification_id: goodwe_integrator_bound. Over-ratings keep goodwe_output_clamp, so a later anti-windup notice can no longer replace a BUG SIGNAL notice. Budgets and cadence are unchanged. New checks: 9a asserts every bound notice uses BOUND_ID, 9b asserts every over-rating notice uses CLAMP_ID, and part 3's own-id check is now BOUND_ID for the default-install runaway. I ran fd14569's main.py against these tests and 2 checks go red. test_freeze.py's whole-surface id list was updated to name the new id, not narrowed. I grepped the infra repo at the pin for the old id and found no consumer.
Point 2, fixed. The WARNING notification body now ends "See the add-on documentation, 'The winding-loop alarm'." The log line is unchanged.
Point 3, fixed. DOCS now reads "normally at most one notification per 150 s (... and an over-rating has its own 150 s budget)". "Never waits behind integrator-bound notices" became "has its own notification budget". Strictly, a single-tick bug inside an already-notified episode, within 150 s of a previous bug notice, waits for the shared re-assert, so "never waits" overclaimed. The DECISIONS entry records the separate id.
A correction to comment 1760's wording: the fuzz found 0 differing steps out of 1 200 000.
Full suite, 35 files, run locally in the CI image with the infra sibling at the pin: all pass. CI on 55a6d5d is green: unit run 1567, infra-pin/fresh run 1568.
Every point is fixed or noted as non-blocking. Merging with head_commit_id pinned to 55a6d5d.
## Review follow-up: points 1-3 fixed in 55a6d5d (reviewer push)
- **Point 1, fixed.** The integrator-bound WARNING now uses its own `notification_id: goodwe_integrator_bound`. Over-ratings keep `goodwe_output_clamp`, so a later anti-windup notice can no longer replace a BUG SIGNAL notice. Budgets and cadence are unchanged. New checks: 9a asserts every bound notice uses `BOUND_ID`, 9b asserts every over-rating notice uses `CLAMP_ID`, and part 3's own-id check is now `BOUND_ID` for the default-install runaway. I ran fd14569's `main.py` against these tests and 2 checks go red. `test_freeze.py`'s whole-surface id list was updated to name the new id, not narrowed. I grepped the infra repo at the pin for the old id and found no consumer.
- **Point 2, fixed.** The WARNING notification body now ends "See the add-on documentation, 'The winding-loop alarm'." The log line is unchanged.
- **Point 3, fixed.** DOCS now reads "normally at most one notification per 150 s (... and an over-rating has its own 150 s budget)". "Never waits behind integrator-bound notices" became "has its own notification budget". Strictly, a single-tick bug inside an already-notified episode, within 150 s of a previous bug notice, waits for the shared re-assert, so "never waits" overclaimed. The DECISIONS entry records the separate id.
- A correction to comment 1760's wording: the fuzz found **0 differing steps out of 1 200 000**.
Full suite, 35 files, run locally in the CI image with the infra sibling at the pin: all pass. CI on 55a6d5d is green: unit run 1567, infra-pin/fresh run 1568.
Every point is fixed or noted as non-blocking. Merging with `head_commit_id` pinned to 55a6d5d.
admin
merged commit 2503de124e into release/1.02026-09-26 23:19:12 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
SAFETY-40. The clamp alarm now tells normal anti-windup apart from an over-rating, and every number in it comes from the tick that clamped.
State on release/1.0 @
59415ea: all 3 defects were still present.main.py:3274-3278and3719-3729:i_clamped or clampedgot a single text, BUG SIGNAL included.main.py:3276: "the loop asked for {decision.i_w}". Since SAFETY-48 this is the lead-capped integrator (e.g. +3000 at a +5000 bound).main.py:3726: "was clamped to {self.target}", i.e. the value after slew or freeze.Change
clamp.loop_clamp_note(decision):i_clampednow reads "the loop's integrator reached its +5000 W anti-windup bound; the loop's command this cycle was +2000 W. Brief engagements after a large load or solar step are normal. If it persists, check that the battery is following its commands."clampedreads "the loop computed +5400 W, over the 5000 W rating, and it was clamped to +5000 W; …" and keeps the BUG SIGNAL.Decisiongetsi_bound_w,requested_wandclamped_w, all set at the clamp.i_clampedstill counts and notifies, with the same id, cadence and SAFETY-41 escalation. It is logged as a WARNING, shows an amber status row and has a plain title. It notifies because on a default install it is the only loop-side runaway signal. Over-rating keeps ERROR, the red row and the 14 768 W rationale, and gets its own notification budget so it never waits behind anti-windup notices. SeeDECISIONS/SAFETY-40-….Tests:
test_clamp.pypart 9 runsController.cycle()withcomputespied. The fixtures make the bound,i_w,self.targetand the requested command all differ. On the old code: defect 1 (4 checks), defect 2 (2), defect 3 (1) and the at/above check all go red. Two part-3 checks that pinned the old BUG text for the default-install runaway were rewritten. Mutations killed: shared text, bound taken fromi_w, clamped-to taken from the slewed command, every clamp treated as a bug, and anti-windup overwriting the bug text. The full suite is green locally in the checks image.🤖 Generated with Claude Code
https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
Review of
fd14569(SAFETY-40)This review ran
test_clamp.pyin the CI image (goodwe-addon-checks:1), ran two mutations, and ran a differential fuzz ofClampWatchdogagainst release/1.0.Verified, no change needed
i_bound_w,requested_wandclamped_ware taken at the clamp, before slew and freeze.target_wis the same decision's emitted command. The SAFETY-48 lead cap runs afteri_bound_wis captured, so it can lower the carriedi_w(part 9a prints bound +5000, i_w +3000) without changing the bound the message reports. "Reached its +5000 W bound" is true for that tick.target != want, so the request really is over the rating.note()(defaultbug=True) against release/1.0'sClampWatchdog: 3000 seeds × 400 ticks, mixed tick gaps and engagement densities. Return value,reasonandactivematched on 0 of 1 200 000 differing steps. The existing SAFETY-41 checks also still run on the defaultbug=Trueand pass. Part 9's count-only check is weak on its own, but it is not the only guard.BUG_SIGNALback into the i-clamped text: 2 checks go red. (C) Oldapp/with the new tests: 12+ checks go red, and nothing crashes.i_clamped, so the old "out of range" wording was exactly defect 1. Part 3 still asserts that it notifies, on its own id, as one coalesced episode, with a row per episode and a flagged status row while engaged.Point 1 (must fix): an anti-windup notice can overwrite a live BUG SIGNAL notification
Both texts use
notification_id: goodwe_output_clamp, and nothing ever dismisses it. So a write-path over-rating (bug) notice can be replaced by a later integrator-bound WARNING. The SAFETY-41 escalation (run_len >= 3, not yet spoken) ignores the budget, so this can happen within seconds. The episode lock only protects text within one episode. On the live sitei_clampedfires about 250 times a day, so a genuine BUG SIGNAL would stay on screen only until the next sustained anti-windup episode.control.pyalready says "the two events want different alarms".Fix: give the WARNING its own id (
goodwe_integrator_bound). The bug notice then stays until the operator dismisses it. Budgets are unchanged. I will push this as a small reviewer fix, with a test that fails onfd14569.Point 2 (small): operator text has no pointer
"integrator" and "anti-windup bound" are jargon on a client site. The WARNING notification should point to DOCS "The winding-loop alarm (the output clamp)". I will add one line to the notification body, not to the log reason.
Point 3 (docs): the cadence sentence
"at most one notification per 150 s" is no longer true: a bug has its own budget, and escalation bypasses the budget. I will reword it.
Noted, not blocking
max_w, so the 14 768 W rationale does not apply to it.Review follow-up: points 1-3 fixed in
55a6d5d(reviewer push)notification_id: goodwe_integrator_bound. Over-ratings keepgoodwe_output_clamp, so a later anti-windup notice can no longer replace a BUG SIGNAL notice. Budgets and cadence are unchanged. New checks: 9a asserts every bound notice usesBOUND_ID, 9b asserts every over-rating notice usesCLAMP_ID, and part 3's own-id check is nowBOUND_IDfor the default-install runaway. I ran fd14569'smain.pyagainst these tests and 2 checks go red.test_freeze.py's whole-surface id list was updated to name the new id, not narrowed. I grepped the infra repo at the pin for the old id and found no consumer.Full suite, 35 files, run locally in the CI image with the infra sibling at the pin: all pass. CI on
55a6d5dis green: unit run 1567, infra-pin/fresh run 1568.Every point is fixed or noted as non-blocking. Merging with
head_commit_idpinned to55a6d5d.