check_output_clamp() sits one line below await self.write_setpoint(self.target) in cycle(). Moving it above the write left test_clamp.py, test_freeze.py and test_loop.py all green - measured before this change, and measured again after, where only test_clamp.py part 6 goes red.
The guard has two evidence sources and only one was covered:
self.loop_clamped is set from the Decision well above the write, so it survives the move. On the §10.3 runaway replay it is the source of every engagement the alarm reports (0 write-path vs 2 loop-side) - which is exactly why the move is nearly invisible.
self.clamp_last is assigned by write_setpoint() and by nothing else, is None until it has run, and is what engaged_write reads at main.py:856. Moved above the write that consumer goes quietly false, and a DEVICE-bound clamp - max_w above the inverter's own rating, i.e. SAFETY-20's install - is reported by nothing.
What I chose, and why
The consequence, not the position. The comment at that line warns against tidying SAFETY-03's ordering and SAFETY-18's into one rule, and TEST-06's check is not transferable (freeze grades self.target, so its boundary is the assignment; clamp grades self.clamp_last, so its boundary is the write). So part 6 asserts what self.clamp_last held each time the guard consulted the watchdog, over a slew-limited ramp where every cycle's command is a different number - which is what makes 'this cycle's evidence' and 'last cycle's' distinguishable at all.
The spy subclasses the shipped ClampWatchdog and hooks note() - the consumer - rather than wrapping the guard by name. Hooking the name would make a rename look identical to the defect, which is the nuisance check this ticket is explicitly not allowed to buy its coverage with.
AC 2, verified against main.py itself, not just against a fixture. Renaming check_output_clamp throughout main.pyand moving both the write and the guard into a new _emit_and_guard() helper called from cycle() leaves part 6 fully green. The one cost of a repo-wide rename is the direct call in 6c, a mechanical find/replace that fails as an AttributeError naming the new method - not as an assertion claiming the ordering is wrong. Part 6b re-runs the identical assertions through a _RefactoredController (guard renamed + write in a helper) inside the suite so that stays true.
AC 3 needed a stimulus of its own, and it is not driven through cycle(). Through the real cycle() the write-path half cannot engage: the arbiter files the device range and max_w as P_SAFETY LIMIT claims, so resolution.target_w is already inside both bounds before write_setpoint() re-applies them (deliberate defence in depth, per main.py's own comment). Part 6 now measures those 0 engagements rather than leaving it as a claim in a comment, then drives the write path directly on SAFETY-20's shape - max_w 5000 against a ±2500 W device: the wire carries 2500 W, every loop-side flag stays clean, the alarm fires on clamp_last alone, and the notification names the device bound. The same fixture with the two calls swapped clamps the same value and is silent. That pair of runs is the ticket.
Verified by breaking what it watches
mutation to main.py
test_clamp
test_freeze
test_loop
guard moved above the write
RED (4 checks, naming clamp_last)
green
green
guard renamed + write & guard moved into _emit_and_guard()
green
-
-
unmutated
green
green
green
Full suite in goodwe-addon-ci: 17/17 green.
What I am unsure about
6c does not go through cycle(), and that is a real limitation rather than a shortcut - the measurement above says cycle() cannot produce a write-path engagement today. If a future change lets a claim source bypass the P_SAFETY limits, the honest check would be a wired one and 6c should be replaced, not extended.
The ramp's distinctness is a precondition, and it is asserted - but it depends on the slew limiter and a tracking plant. If either changes, the precondition check goes red first and says so, which I preferred to a silently vacuous comparison.
main.py gains one comment sentence pointing at the check (mirroring SAFETY-18's existing pointer to test_freeze.py part 5). No behaviour changes; if reviewers would rather main.py stayed untouched, that sentence is the only thing to drop.
## The defect
`check_output_clamp()` sits one line below `await self.write_setpoint(self.target)` in `cycle()`. Moving it above the write left **test_clamp.py, test_freeze.py and test_loop.py all green** - measured before this change, and measured again after, where only test_clamp.py part 6 goes red.
The guard has two evidence sources and only one was covered:
- `self.loop_clamped` is set from the `Decision` well above the write, so it survives the move. On the §10.3 runaway replay it is the source of **every** engagement the alarm reports (0 write-path vs 2 loop-side) - which is exactly why the move is nearly invisible.
- `self.clamp_last` is assigned by `write_setpoint()` and by nothing else, is `None` until it has run, and is what `engaged_write` reads at `main.py:856`. Moved above the write that consumer goes quietly false, and a DEVICE-bound clamp - `max_w` above the inverter's own rating, i.e. SAFETY-20's install - is reported by nothing.
## What I chose, and why
**The consequence, not the position.** The comment at that line warns against tidying SAFETY-03's ordering and SAFETY-18's into one rule, and TEST-06's check is not transferable (freeze grades `self.target`, so its boundary is the assignment; clamp grades `self.clamp_last`, so its boundary is the write). So part 6 asserts what `self.clamp_last` held **each time the guard consulted the watchdog**, over a slew-limited ramp where every cycle's command is a different number - which is what makes 'this cycle's evidence' and 'last cycle's' distinguishable at all.
The spy subclasses the shipped `ClampWatchdog` and hooks `note()` - the consumer - rather than wrapping the guard by name. Hooking the name would make a rename look identical to the defect, which is the nuisance check this ticket is explicitly not allowed to buy its coverage with.
**AC 2, verified against main.py itself, not just against a fixture.** Renaming `check_output_clamp` throughout `main.py` **and** moving both the write and the guard into a new `_emit_and_guard()` helper called from `cycle()` leaves part 6 fully green. The one cost of a repo-wide rename is the direct call in 6c, a mechanical find/replace that fails as an `AttributeError` naming the new method - not as an assertion claiming the ordering is wrong. Part 6b re-runs the identical assertions through a `_RefactoredController` (guard renamed + write in a helper) inside the suite so that stays true.
**AC 3 needed a stimulus of its own, and it is not driven through `cycle()`.** Through the real `cycle()` the write-path half *cannot* engage: the arbiter files the device range and `max_w` as `P_SAFETY` LIMIT claims, so `resolution.target_w` is already inside both bounds before `write_setpoint()` re-applies them (deliberate defence in depth, per main.py's own comment). Part 6 now **measures** those 0 engagements rather than leaving it as a claim in a comment, then drives the write path directly on SAFETY-20's shape - `max_w` 5000 against a ±2500 W device: the wire carries 2500 W, every loop-side flag stays clean, the alarm fires on `clamp_last` alone, and the notification names the *device* bound. The same fixture with the two calls swapped clamps the same value and is silent. That pair of runs is the ticket.
## Verified by breaking what it watches
| mutation to `main.py` | test_clamp | test_freeze | test_loop |
|---|---|---|---|
| guard moved above the write | **RED** (4 checks, naming `clamp_last`) | green | green |
| guard renamed + write & guard moved into `_emit_and_guard()` | green | - | - |
| unmutated | green | green | green |
Full suite in `goodwe-addon-ci`: **17/17 green**.
## What I am unsure about
1. **6c does not go through `cycle()`, and that is a real limitation** rather than a shortcut - the measurement above says `cycle()` cannot produce a write-path engagement today. If a future change lets a claim source bypass the P_SAFETY limits, the honest check would be a wired one and 6c should be replaced, not extended.
2. **The ramp's distinctness is a precondition, and it is asserted** - but it depends on the slew limiter and a tracking plant. If either changes, the precondition check goes red first and says so, which I preferred to a silently vacuous comparison.
3. `main.py` gains one comment sentence pointing at the check (mirroring SAFETY-18's existing pointer to test_freeze.py part 5). No behaviour changes; if reviewers would rather main.py stayed untouched, that sentence is the only thing to drop.
check_output_clamp() sits one line below `await self.write_setpoint(self.target)`
in cycle(), and moving it above the write left test_clamp.py, test_freeze.py and
test_loop.py ALL GREEN. Measured on the move itself before this commit; measured
again after, where test_clamp.py part 6 goes red and the other two are still
green - the gap was real and it was only ever in this file.
WHAT IS ACTUALLY LOST. The guard has two evidence sources. `self.loop_clamped`
is set from the Decision well above the write and survives the move, and on the
10.3 runaway replay it is the source of every engagement the alarm reports (0
write-path engagements against 2 loop-side ones) - which is exactly what makes
the move nearly invisible. `self.clamp_last` is assigned by write_setpoint() and
by nothing else, is None until it has run, and is what `engaged_write` reads at
main.py:856. Moved above the write, that consumer goes quietly false and a
DEVICE-bound clamp - max_w above the inverter's own rating, SAFETY-20's install -
is reported by nothing. A guard whose second evidence source can be silently
disconnected is one refactor from being half a guard.
WHAT THE CHECK PINS, AND WHAT IT DELIBERATELY DOES NOT. Not the position: the
comment at that line warns against tidying SAFETY-03's ordering and SAFETY-18's
into one rule, and a check a rename breaks is worse than none. It pins the
CONSEQUENCE - what `self.clamp_last` held each time the guard consulted the
watchdog - using a spy subclassed from the shipped ClampWatchdog over a
slew-limited ramp where every cycle's command is a different number, so "this
cycle's evidence" and "last cycle's" are distinguishable at all. The spy hooks
`clamp_wd.note()`, the consumer, rather than the guard's name: that is what
keeps a rename legitimate. The same assertions are re-run through a controller
whose guard is renamed and whose write lives in a helper, and were verified
against main.py itself - renaming the method throughout AND moving both calls
into a new `_emit_and_guard()` helper keeps part 6 green; swapping the two calls
turns it red on a line that names clamp_last.
WHY AC 3 NEEDED A NEW STIMULUS, AND WHY IT IS NOT DRIVEN THROUGH cycle().
Through the real cycle() the write-path half cannot engage: the arbiter files
the device range and max_w as P_SAFETY LIMIT claims, so resolution.target_w is
already inside both bounds before write_setpoint() re-applies them. That
duplication is deliberate defence in depth; part 6 now MEASURES the zero
engagements rather than leaving it as a claim in a comment, and then drives the
write path directly at max_w 5000 against a +-2500 W device - the wire carries
2500 W, every loop-side flag stays clean, and the alarm fires on clamp_last
alone. With the two calls swapped the same stimulus clamps the same value and
the alarm is silent, which is the ticket in one pair of runs.
main.py gains one comment sentence naming the check, mirroring the one
SAFETY-18's block already carries for test_freeze.py part 5. No behaviour
changes.
Full suite: 17/17 green in goodwe-addon-ci.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
admin
merged commit 48cd5f31ca into release/1.02026-09-01 07:45:16 +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.
The defect
check_output_clamp()sits one line belowawait self.write_setpoint(self.target)incycle(). Moving it above the write left test_clamp.py, test_freeze.py and test_loop.py all green - measured before this change, and measured again after, where only test_clamp.py part 6 goes red.The guard has two evidence sources and only one was covered:
self.loop_clampedis set from theDecisionwell above the write, so it survives the move. On the §10.3 runaway replay it is the source of every engagement the alarm reports (0 write-path vs 2 loop-side) - which is exactly why the move is nearly invisible.self.clamp_lastis assigned bywrite_setpoint()and by nothing else, isNoneuntil it has run, and is whatengaged_writereads atmain.py:856. Moved above the write that consumer goes quietly false, and a DEVICE-bound clamp -max_wabove the inverter's own rating, i.e. SAFETY-20's install - is reported by nothing.What I chose, and why
The consequence, not the position. The comment at that line warns against tidying SAFETY-03's ordering and SAFETY-18's into one rule, and TEST-06's check is not transferable (freeze grades
self.target, so its boundary is the assignment; clamp gradesself.clamp_last, so its boundary is the write). So part 6 asserts whatself.clamp_lastheld each time the guard consulted the watchdog, over a slew-limited ramp where every cycle's command is a different number - which is what makes 'this cycle's evidence' and 'last cycle's' distinguishable at all.The spy subclasses the shipped
ClampWatchdogand hooksnote()- the consumer - rather than wrapping the guard by name. Hooking the name would make a rename look identical to the defect, which is the nuisance check this ticket is explicitly not allowed to buy its coverage with.AC 2, verified against main.py itself, not just against a fixture. Renaming
check_output_clampthroughoutmain.pyand moving both the write and the guard into a new_emit_and_guard()helper called fromcycle()leaves part 6 fully green. The one cost of a repo-wide rename is the direct call in 6c, a mechanical find/replace that fails as anAttributeErrornaming the new method - not as an assertion claiming the ordering is wrong. Part 6b re-runs the identical assertions through a_RefactoredController(guard renamed + write in a helper) inside the suite so that stays true.AC 3 needed a stimulus of its own, and it is not driven through
cycle(). Through the realcycle()the write-path half cannot engage: the arbiter files the device range andmax_wasP_SAFETYLIMIT claims, soresolution.target_wis already inside both bounds beforewrite_setpoint()re-applies them (deliberate defence in depth, per main.py's own comment). Part 6 now measures those 0 engagements rather than leaving it as a claim in a comment, then drives the write path directly on SAFETY-20's shape -max_w5000 against a ±2500 W device: the wire carries 2500 W, every loop-side flag stays clean, the alarm fires onclamp_lastalone, and the notification names the device bound. The same fixture with the two calls swapped clamps the same value and is silent. That pair of runs is the ticket.Verified by breaking what it watches
main.pyclamp_last)_emit_and_guard()Full suite in
goodwe-addon-ci: 17/17 green.What I am unsure about
cycle(), and that is a real limitation rather than a shortcut - the measurement above sayscycle()cannot produce a write-path engagement today. If a future change lets a claim source bypass the P_SAFETY limits, the honest check would be a wired one and 6c should be replaced, not extended.main.pygains one comment sentence pointing at the check (mirroring SAFETY-18's existing pointer to test_freeze.py part 5). No behaviour changes; if reviewers would rather main.py stayed untouched, that sentence is the only thing to drop.