Post-merge review follow-up for SAFETY-02. Contains the deploy blocker. Nothing here changes what the detector detects.
BLOCKER — the latch did not survive a restart
auto was persisted; sign_wd.tripped was not. Reconstructing the Controller against the same store — what a container restart does — brought it back armed, into the same fault, unattended:
before restart : auto=True tripped=True target=0 W banner='FAILSAFE — …'
after restart : auto=True tripped=False banner='NOT READY — …'
it resumed commanding, unattended: writes=[-1000.0, -2000.0, 0.0]
Then re-tripped 180 s later. Under a Supervisor that restarts a crashed add-on that is a loop: three minutes of wrong-direction power per restart, forever. And "restart the add-on" is the first thing an operator staring at a FAILSAFE banner and a 0 W inverter will try. DOCS.md's "It does not clear itself" was false across exactly that.
sign_tripped and sign_reason are Store keys now, beside the maintenance state and for the reason store.py's own docstring gives. Written the moment the trip happens — reason first, flag second, so a crash between the two writes can never persist a latch with no explanation for the operator who has to decide whether to clear it — restored in Controller.__init__, cleared only by handle_action. A restart now also logs why it is still tripped.
The regression is covered, and proven to be. Reverting just the restore line fails seven checks in test_watchdog.py:
FAIL the new Controller comes back TRIPPED
FAIL ...and says why, restored rather than re-detected
FAIL ...commands 0 W from its very first cycle
FAIL ...and never commanded anything else after the restart
FAIL the banner still leads with the failsafe
FAIL pressing stop does not clear the failsafe
FAIL pressing start DOES clear it, and clears the store with it
The two-press requirement is asserted explicitly: the first press (stop) must NOT clear it, only the transition back to started does.
The arming gate is now tested against the real arbiter
You were right that it was the decision you most wanted checked and was verified by nothing — every case passed following in as a literal. test_watchdog.py now derives it from resolve() across twelve claim configurations:
ok the loop alone, unopposed -> following=True
ok the loop bound by max_w - reduced, not redirected -> following=True
ok charging, bound by max_w on the other side -> following=True
ok maintenance owns the setpoint outright -> following=False
ok maintenance charge-only clamps the loop to 0 W -> following=False
ok cheap-window charge REDIRECTS the loop while it still 'wins' -> following=False
ok ...and the contradiction at the shipped defaults is not following either -> following=False
ok a lo limit RAISES the magnitude without flipping the sign -> following=False
ok a safety stop wins outright -> following=False
ok no claims at all - the arbiter's own failsafe -> following=False
ok the loop asked for zero -> following=False
ok peak shaving at P_PEAK disarms the gate (see main.py's forward hazard) -> following=False
ok no configuration above arms the gate without the loop winning
The P_PEAK row asserts the hazard as it stands today, so whoever lands peak shaving has to come here and change an expectation rather than silently blinding the watchdog. The forward hazard itself is written at the gate, naming the fix: widen it to "the winning strategy follows the meter", not add another name to a comparison.
The fake check is replaced
check("MQTT reports the failsafe state", ctrl.sign_wd.tripped) restated the line above it and never touched the payload. It now captures what publish() hands the broker and asserts status == "failsafe", then "running" after a reset.
The worked example described a config nobody runs
Correct, and thank you — at the defaults maintenance_charge_w 2500 against max_w 2000 is lo > hi, an arbiter contradiction resolving to 0 W, not a −2500 W charge. All four sites are rewritten at 1500 W, which the schema and the arbiter both actually produce, with the contradiction noted where the example lives. The conclusion is unchanged: winner == "loop" alone would still have shipped a false trip.
I have not touched the underlying maintenance bug you are filing — including not adding the maintenance_charge_w > max_w validator rule, even though you are right that it is rule 5's shape exactly. It belongs to that ticket, and a refusal added here would land without the fix it implies.
Smaller
disagrees() is public, so sim/replay_sign.py can stop carrying its own copy of the predicate it grades.
held_s takes the clock instead of reading one, restoring the pure-function property the module advertises — which is why it now has a caller that can be tested.
Both limits are now stated in both places: the downstream-inversion limit is in NOTES.md as well as watchdog.py, and the acts-opposite limit is in watchdog.py as well as DOCS/CHANGELOG/NOTES.
The no-false-positive argument is tied to control.py:219-227 permitting the command to fall through zero and reverse. Under the shrink-only encoding control.py:150-160 records as rejected, the command pins and this watchdog would false-trip on a load step. It is an invariant now, not a coincidence.
Measured figures corrected everywhere: 93.4 % / 4212 of 17280 (was 92.9 % / 4127 of 16349), per-cluster 0.3 / 0.9 / 10.5 / 0.0 %. And the near-circularity of those figures is stated rather than leaned on — see the infra PR.
Tests
All seven suites pass. test_watchdog.py is 39 → 60 checks.
Post-merge review follow-up for SAFETY-02. **Contains the deploy blocker.** Nothing here changes what the detector detects.
## BLOCKER — the latch did not survive a restart
`auto` was persisted; `sign_wd.tripped` was not. Reconstructing the `Controller` against the same store — what a container restart does — brought it back **armed, into the same fault, unattended**:
```
before restart : auto=True tripped=True target=0 W banner='FAILSAFE — …'
after restart : auto=True tripped=False banner='NOT READY — …'
it resumed commanding, unattended: writes=[-1000.0, -2000.0, 0.0]
```
Then re-tripped 180 s later. Under a Supervisor that restarts a crashed add-on that is a loop: three minutes of wrong-direction power per restart, forever. And "restart the add-on" is the first thing an operator staring at a `FAILSAFE` banner and a 0 W inverter will try. `DOCS.md`'s *"It does not clear itself"* was false across exactly that.
`sign_tripped` and `sign_reason` are `Store` keys now, beside the maintenance state and for the reason `store.py`'s own docstring gives. Written the moment the trip happens — **reason first, flag second**, so a crash between the two writes can never persist a latch with no explanation for the operator who has to decide whether to clear it — restored in `Controller.__init__`, cleared only by `handle_action`. A restart now also logs why it is still tripped.
**The regression is covered, and proven to be.** Reverting just the restore line fails seven checks in `test_watchdog.py`:
```
FAIL the new Controller comes back TRIPPED
FAIL ...and says why, restored rather than re-detected
FAIL ...commands 0 W from its very first cycle
FAIL ...and never commanded anything else after the restart
FAIL the banner still leads with the failsafe
FAIL pressing stop does not clear the failsafe
FAIL pressing start DOES clear it, and clears the store with it
```
The two-press requirement is asserted explicitly: the first press (stop) must NOT clear it, only the transition back to started does.
## The arming gate is now tested against the real arbiter
You were right that it was the decision you most wanted checked and was verified by nothing — every case passed `following` in as a literal. `test_watchdog.py` now derives it from `resolve()` across twelve claim configurations:
```
ok the loop alone, unopposed -> following=True
ok the loop bound by max_w - reduced, not redirected -> following=True
ok charging, bound by max_w on the other side -> following=True
ok maintenance owns the setpoint outright -> following=False
ok maintenance charge-only clamps the loop to 0 W -> following=False
ok cheap-window charge REDIRECTS the loop while it still 'wins' -> following=False
ok ...and the contradiction at the shipped defaults is not following either -> following=False
ok a lo limit RAISES the magnitude without flipping the sign -> following=False
ok a safety stop wins outright -> following=False
ok no claims at all - the arbiter's own failsafe -> following=False
ok the loop asked for zero -> following=False
ok peak shaving at P_PEAK disarms the gate (see main.py's forward hazard) -> following=False
ok no configuration above arms the gate without the loop winning
```
The P_PEAK row asserts the hazard **as it stands today**, so whoever lands peak shaving has to come here and change an expectation rather than silently blinding the watchdog. The forward hazard itself is written at the gate, naming the fix: widen it to "the winning strategy follows the meter", not add another name to a comparison.
## The fake check is replaced
`check("MQTT reports the failsafe state", ctrl.sign_wd.tripped)` restated the line above it and never touched the payload. It now captures what `publish()` hands the broker and asserts `status == "failsafe"`, then `"running"` after a reset.
## The worked example described a config nobody runs
Correct, and thank you — at the defaults `maintenance_charge_w` 2500 against `max_w` 2000 is `lo > hi`, an arbiter **contradiction** resolving to 0 W, not a −2500 W charge. All four sites are rewritten at **1500 W**, which the schema and the arbiter both actually produce, with the contradiction noted where the example lives. The conclusion is unchanged: `winner == "loop"` alone would still have shipped a false trip.
I have **not** touched the underlying maintenance bug you are filing — including not adding the `maintenance_charge_w > max_w` validator rule, even though you are right that it is rule 5's shape exactly. It belongs to that ticket, and a refusal added here would land without the fix it implies.
## Smaller
- `disagrees()` is public, so `sim/replay_sign.py` can stop carrying its own copy of the predicate it grades.
- `held_s` takes the clock instead of reading one, restoring the pure-function property the module advertises — which is why it now has a caller that can be tested.
- Both limits are now stated in both places: the downstream-inversion limit is in `NOTES.md` as well as `watchdog.py`, and the acts-opposite limit is in `watchdog.py` as well as DOCS/CHANGELOG/NOTES.
- The no-false-positive argument is tied to `control.py:219-227` permitting the command to fall through zero and reverse. Under the shrink-only encoding `control.py:150-160` records as rejected, the command pins and this watchdog **would** false-trip on a load step. It is an invariant now, not a coincidence.
- Measured figures corrected everywhere: **93.4 % / 4212 of 17280** (was 92.9 % / 4127 of 16349), per-cluster **0.3 / 0.9 / 10.5 / 0.0 %**. And the near-circularity of those figures is stated rather than leaned on — see the infra PR.
## Tests
All seven suites pass. `test_watchdog.py` is 39 → 60 checks.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
BLOCKER. `auto` was persisted and `sign_wd.tripped` was not, so reconstructing
the Controller against the same store - which is what a container restart does -
brought it back ARMED, into the same fault, unattended: tripped=False, banner
back to NOT READY, and it resumed commanding -1000 then -2000 W before
re-tripping 180 s later. "Restart the add-on" is the first thing an operator
looking at a FAILSAFE banner and a 0 W inverter will try, and under a supervisor
that restarts a crashed add-on it is a loop of three-minute wrong-direction
bursts. DOCS.md's "it does not clear itself" was false across a restart.
`sign_tripped` and `sign_reason` are Store keys now, beside the maintenance
state and for the reason store.py's own docstring gives. Written the moment the
trip happens - reason first, flag second, so a crash between the two can never
persist a latch with no explanation - restored in Controller.__init__, cleared
only by the operator's start. The regression is covered: reverting the restore
fails seven checks in test_watchdog.py.
Review follow-ups, no behaviour change:
- The arming gate is tested against the REAL arbiter. It is the decision the
ticket turns on and it was asserted in three comment blocks and verified by
nothing - every case passed `following` in as a literal. Twelve claim
configurations now derive it from resolve(), including the redirect case
`winner == "loop"` alone would have got wrong and the `lo` limit that raises
magnitude without flipping sign.
- The MQTT `status: "failsafe"` value is tested. The check that claimed to cover
it restated the line above it and never touched the payload.
- The worked example described a config that does not ship: at the defaults
maintenance_charge_w 2500 against max_w 2000 is an arbiter contradiction and
0 W, not a -2500 W charge. Rewritten at 1500 W. The contradiction is a real
pre-existing bug - a default cheap-window maintenance charge silently never
happens - filed separately and deliberately not fixed here.
- Forward hazard recorded at the gate: `winner == "loop"` disarms the watchdog
under any higher-priority SET, and a peak-shaving strategy at P_PEAK IS
grid-following in sign, so it would blind this for every peak event.
- disagrees() is public so the replay harness stops carrying its own copy of the
predicate it grades.
- held_s takes the clock instead of reading one.
- The two limits are now each stated in both places rather than one each.
- The no-false-positive argument is tied to control.py's freeze permitting the
command to cross zero; under the shrink-only encoding control.py records as
rejected, this would false-trip on a load step.
- Measured figures corrected: 93.4 % / 4212 of 17280, per-cluster
0.3/0.9/10.5/0.0, and the near-circularity of those figures is now stated
rather than leaned on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
Review finding 2, and it is the same duplication `disagrees()` just removed, so
it gets the same fix rather than a follow-up card.
`following_from()` in test_watchdog.py was a TRANSCRIPTION of the gate in
main.py, carrying "keep the two in step" as an instruction - which is exactly the
arrangement that does not keep two things in step. Measured: editing the shipped
gate alone to `winner in ("loop", "peak")` left every suite green while the
watchdog went blind for the duration of every peak event. The row whose whole job
is to force whoever lands peak shaving to come and change an expectation did not
do that.
The expression is now `watchdog.loop_is_following()`, called by main.py and by
the test. One definition, two callers, same reasoning as `disagrees()`: the copy
that drifts is always the one in the harness doing the grading.
⚠️ AND EXTRACTING IT WAS NOT SUFFICIENT - the drift proof still came out green
after the extraction, which is why it was worth running rather than assuming. The
realistic P_PEAK row has peak shaving asking for MORE than the loop, so the
magnitude term rejects it whatever `winner` says and masks the term under test. A
second row with peak asking for LESS makes `winner` the only deciding term. With
that row, widening the shipped gate fails two checks; without it, none.
Also enforcing the write ordering that was documented and not asserted: swapping
the two `store.set` calls passed everything. A store that fails the second write
now proves the property the ordering exists for - a crash mid-trip leaves a
reason with no latch, never a latch with no explanation. Swapping them now turns
that check red.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
Finding 2 taken here rather than as a follow-up — you are right that it is the shape I just solved with disagrees(), and the same reasoning applies verbatim.
The gate is now watchdog.loop_is_following(), called by main.py and by the test. The forward hazard and the "the loop won is not the question" reasoning moved with it, so they sit next to the expression they constrain.
⚠️ Extracting it was not sufficient, and the drift proof is what showed that
After the extraction I re-ran your simulation — edit the shipped gate to winner in ("loop", "peak") — expecting red. It was still green.
The realistic P_PEAK row has peak shaving asking for more than the loop (900 asked, 1800 claimed), so abs(1800) <= abs(900) rejects it regardless of what winner says. The magnitude term masks the winner term, and that row could never have detected a widened gate even with the duplication removed. Your simulation found a real defect; extracting the function fixed only half of it.
Added a row that isolates winner as the only deciding term — peak claiming less than the loop asked, same sign, so magnitude cannot reject it:
ok peak shaving at P_PEAK disarms the gate (loop_is_following's forward hazard)
ok ...and with peak BELOW the loop's ask, only the winner term separates them
With the shipped gate widened, that row and the sweep assertion both fail:
FAIL ...and with peak BELOW the loop's ask, only the winner term separates them -> following=False
FAIL no configuration above arms the gate without the loop winning
The assertion bites now.
Write ordering enforced
Took the optional one — the reasoning is only worth having if reordering two adjacent lines turns something red. A store that fails the second write asserts the property rather than the order: what survives a crash mid-trip is a reason with no latch, never a latch with no explanation. (Modelled as an exception; in the field it is a power cut, since Store.save() swallows OSError.) Swapping the two lines now fails a crash between the two writes leaves the REASON on disk.
MAINT-04 condition noted
Agreed, and the 1500 W sites already carry the reason inline (main.py, watchdog.py) so whoever picks up MAINT-04 finds the explanation rather than an unexplained number.
All seven suites pass; test_watchdog.py 60 → 70 checks.
**Finding 2 taken here rather than as a follow-up** — you are right that it is the shape I just solved with `disagrees()`, and the same reasoning applies verbatim.
The gate is now `watchdog.loop_is_following()`, called by `main.py` and by the test. The forward hazard and the "the loop won is not the question" reasoning moved with it, so they sit next to the expression they constrain.
### ⚠️ Extracting it was not sufficient, and the drift proof is what showed that
After the extraction I re-ran your simulation — edit the shipped gate to `winner in ("loop", "peak")` — expecting red. **It was still green.**
The realistic P_PEAK row has peak shaving asking for *more* than the loop (900 asked, 1800 claimed), so `abs(1800) <= abs(900)` rejects it regardless of what `winner` says. The magnitude term masks the winner term, and that row could never have detected a widened gate even with the duplication removed. Your simulation found a real defect; extracting the function fixed only half of it.
Added a row that isolates `winner` as the only deciding term — peak claiming *less* than the loop asked, same sign, so magnitude cannot reject it:
```
ok peak shaving at P_PEAK disarms the gate (loop_is_following's forward hazard)
ok ...and with peak BELOW the loop's ask, only the winner term separates them
```
With the shipped gate widened, that row and the sweep assertion both fail:
```
FAIL ...and with peak BELOW the loop's ask, only the winner term separates them -> following=False
FAIL no configuration above arms the gate without the loop winning
```
The assertion bites now.
### Write ordering enforced
Took the optional one — the reasoning is only worth having if reordering two adjacent lines turns something red. A store that fails the second write asserts the **property** rather than the order: what survives a crash mid-trip is a reason with no latch, never a latch with no explanation. (Modelled as an exception; in the field it is a power cut, since `Store.save()` swallows `OSError`.) Swapping the two lines now fails `a crash between the two writes leaves the REASON on disk`.
### MAINT-04 condition noted
Agreed, and the 1500 W sites already carry the reason inline (`main.py`, `watchdog.py`) so whoever picks up MAINT-04 finds the explanation rather than an unexplained number.
All seven suites pass; `test_watchdog.py` 60 → 70 checks.
admin
merged commit bb64f8fdd7 into release/1.02026-08-28 23:09:28 +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.
Post-merge review follow-up for SAFETY-02. Contains the deploy blocker. Nothing here changes what the detector detects.
BLOCKER — the latch did not survive a restart
autowas persisted;sign_wd.trippedwas not. Reconstructing theControlleragainst the same store — what a container restart does — brought it back armed, into the same fault, unattended:Then re-tripped 180 s later. Under a Supervisor that restarts a crashed add-on that is a loop: three minutes of wrong-direction power per restart, forever. And "restart the add-on" is the first thing an operator staring at a
FAILSAFEbanner and a 0 W inverter will try.DOCS.md's "It does not clear itself" was false across exactly that.sign_trippedandsign_reasonareStorekeys now, beside the maintenance state and for the reasonstore.py's own docstring gives. Written the moment the trip happens — reason first, flag second, so a crash between the two writes can never persist a latch with no explanation for the operator who has to decide whether to clear it — restored inController.__init__, cleared only byhandle_action. A restart now also logs why it is still tripped.The regression is covered, and proven to be. Reverting just the restore line fails seven checks in
test_watchdog.py:The two-press requirement is asserted explicitly: the first press (stop) must NOT clear it, only the transition back to started does.
The arming gate is now tested against the real arbiter
You were right that it was the decision you most wanted checked and was verified by nothing — every case passed
followingin as a literal.test_watchdog.pynow derives it fromresolve()across twelve claim configurations:The P_PEAK row asserts the hazard as it stands today, so whoever lands peak shaving has to come here and change an expectation rather than silently blinding the watchdog. The forward hazard itself is written at the gate, naming the fix: widen it to "the winning strategy follows the meter", not add another name to a comparison.
The fake check is replaced
check("MQTT reports the failsafe state", ctrl.sign_wd.tripped)restated the line above it and never touched the payload. It now captures whatpublish()hands the broker and assertsstatus == "failsafe", then"running"after a reset.The worked example described a config nobody runs
Correct, and thank you — at the defaults
maintenance_charge_w2500 againstmax_w2000 islo > hi, an arbiter contradiction resolving to 0 W, not a −2500 W charge. All four sites are rewritten at 1500 W, which the schema and the arbiter both actually produce, with the contradiction noted where the example lives. The conclusion is unchanged:winner == "loop"alone would still have shipped a false trip.I have not touched the underlying maintenance bug you are filing — including not adding the
maintenance_charge_w > max_wvalidator rule, even though you are right that it is rule 5's shape exactly. It belongs to that ticket, and a refusal added here would land without the fix it implies.Smaller
disagrees()is public, sosim/replay_sign.pycan stop carrying its own copy of the predicate it grades.held_stakes the clock instead of reading one, restoring the pure-function property the module advertises — which is why it now has a caller that can be tested.NOTES.mdas well aswatchdog.py, and the acts-opposite limit is inwatchdog.pyas well as DOCS/CHANGELOG/NOTES.control.py:219-227permitting the command to fall through zero and reverse. Under the shrink-only encodingcontrol.py:150-160records as rejected, the command pins and this watchdog would false-trip on a load step. It is an invariant now, not a coincidence.Tests
All seven suites pass.
test_watchdog.pyis 39 → 60 checks.🤖 Generated with Claude Code
https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
Review finding 2, and it is the same duplication `disagrees()` just removed, so it gets the same fix rather than a follow-up card. `following_from()` in test_watchdog.py was a TRANSCRIPTION of the gate in main.py, carrying "keep the two in step" as an instruction - which is exactly the arrangement that does not keep two things in step. Measured: editing the shipped gate alone to `winner in ("loop", "peak")` left every suite green while the watchdog went blind for the duration of every peak event. The row whose whole job is to force whoever lands peak shaving to come and change an expectation did not do that. The expression is now `watchdog.loop_is_following()`, called by main.py and by the test. One definition, two callers, same reasoning as `disagrees()`: the copy that drifts is always the one in the harness doing the grading. ⚠️ AND EXTRACTING IT WAS NOT SUFFICIENT - the drift proof still came out green after the extraction, which is why it was worth running rather than assuming. The realistic P_PEAK row has peak shaving asking for MORE than the loop, so the magnitude term rejects it whatever `winner` says and masks the term under test. A second row with peak asking for LESS makes `winner` the only deciding term. With that row, widening the shipped gate fails two checks; without it, none. Also enforcing the write ordering that was documented and not asserted: swapping the two `store.set` calls passed everything. A store that fails the second write now proves the property the ordering exists for - a crash mid-trip leaves a reason with no latch, never a latch with no explanation. Swapping them now turns that check red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDaFinding 2 taken here rather than as a follow-up — you are right that it is the shape I just solved with
disagrees(), and the same reasoning applies verbatim.The gate is now
watchdog.loop_is_following(), called bymain.pyand by the test. The forward hazard and the "the loop won is not the question" reasoning moved with it, so they sit next to the expression they constrain.⚠️ Extracting it was not sufficient, and the drift proof is what showed that
After the extraction I re-ran your simulation — edit the shipped gate to
winner in ("loop", "peak")— expecting red. It was still green.The realistic P_PEAK row has peak shaving asking for more than the loop (900 asked, 1800 claimed), so
abs(1800) <= abs(900)rejects it regardless of whatwinnersays. The magnitude term masks the winner term, and that row could never have detected a widened gate even with the duplication removed. Your simulation found a real defect; extracting the function fixed only half of it.Added a row that isolates
winneras the only deciding term — peak claiming less than the loop asked, same sign, so magnitude cannot reject it:With the shipped gate widened, that row and the sweep assertion both fail:
The assertion bites now.
Write ordering enforced
Took the optional one — the reasoning is only worth having if reordering two adjacent lines turns something red. A store that fails the second write asserts the property rather than the order: what survives a crash mid-trip is a reason with no latch, never a latch with no explanation. (Modelled as an exception; in the field it is a power cut, since
Store.save()swallowsOSError.) Swapping the two lines now failsa crash between the two writes leaves the REASON on disk.MAINT-04 condition noted
Agreed, and the 1500 W sites already carry the reason inline (
main.py,watchdog.py) so whoever picks up MAINT-04 finds the explanation rather than an unexplained number.All seven suites pass;
test_watchdog.py60 → 70 checks.