SAFETY-20: measure the freeze rail against the reachable ceiling, not max_w #28

Merged
admin merged 2 commits from SAFETY-20 into release/1.0 2026-09-01 02:57:59 +02:00
Owner

SAFETY-18's conjunction needs abs(commanded_w) >= 0.95 * max_w, but the
emitted command is clipped to the setpoint device's own [dev_min, dev_max]
range as well as to max_w - in clamp_to_rating()'s second stage and again as
the arbiter's device P_SAFETY limit claim. Measured on the shipped predicate
at the ticket's numbers, max_w 5000 on a +-4700 W inverter:

looks_frozen(unchanged_s=3600, commanded_w=4700, max_w=5000)  ->  False

The command is pinned at the hardware's rail, the meter has been frozen for an
hour, and term 2 is unsatisfiable by construction because 4700 < 4750. Any
device rating at or below 0.95 * max_w disarms the conjunction outright, and
the status page then printed armed (... command is over 4750 W) - a guard
that cannot fire, advertising that it can, which is the failure SAFETY-15,
SAFETY-19 and validate.py rule 5 all exist to prevent.

The rail is now measured against min(max_w, |dev_min|, |dev_max|), through a
new rail_ceiling_w() threaded into looks_frozen(), FreezeWatchdog.evaluate()
and both call sites in main.py. An unread rating leaves the old behaviour
exactly, and correctly: hass.limits() returns None whenever ESPHome is not up,
and on that path no device clamp is applied either, so max_w really is the
ceiling. A rating larger than max_w cannot raise the rail - this is a min(),
not a swap.

THE DELIBERATE CHOICE, since SAFETY-03 has landed. The arithmetic was kept
rather than substituting Decision.i_clamped / Decision.clamped. i_clamped is
the integrator bound (control.py:208, bounded != i_w), not "the command is
at the device rating", and clamped is the max_w output bound, unreachable on
a default install - neither is the device rail. More decisively, both answer
"did the LOOP ask for more than a bound", while the rail term asks "is the
command standing at the ceiling", and those coincide only when the loop is the
claimer that won. A maintenance SET or a peak-guard floor reaches the ceiling
through arbiter LIMIT claims with no clamp engaging at all, so a boolean rail
would go False exactly during a maintenance phase running against a frozen
meter - which freeze.py's own docstring names as the same fault with a worse
actuator behind it. The predicate is also public with two call sites that pass
DIFFERENT command values (self.target, and standing = last_write); a
per-cycle boolean cannot be passed per call site, so substituting it would
have silently decided the pre-clamp-versus-wire question that PM recorded as
still open. AC 1 governs, and it is satisfied literally.

The smaller of the two device bounds, not a per-sign pair. A per-sign rail is
tighter and is not wrong, but it is two rails needing two coverage verdicts on
a status row that carries one number, and the only configuration that
distinguishes them is refused at startup anyway. The symmetric ceiling can only
lower the rail, which widens term 2 rather than disarming it; term 1 carries
the whole no-false-positive argument and is untouched.

A ceiling of 0 W is NOT COVERED, not "armed at 0 W". A device that can command
nothing in one direction would make abs(command) >= 0 true of every command
including a parked one - term 2 degenerately TRUE, collapsing the conjunction
into the bare unchanged_s > N threshold TEL-05 refused for false-tripping at
this controller's own operating point. rail_unreachable() returns the reason,
looks_frozen() stays silent, and the page uses the NOT COVERED row the
unsupported-transport case already has - a second reason to use that surface,
not a new mechanism.

AC 4, decided rather than assumed: NO NEW validate.py RULE. The configuration
is max_w above the device rating, which rule 2 (CFG-01) already refuses on the
identical predicate, re-run by run_control() once the real rating is known:

validate_startup_config({... max_w: 5000}, -4700, 4700)
  -> ['max_w is 5000 W but number.sp accepts at most 4700 W - THE DEVICE
      WINS ...']

A rail-specific rule would fire on a strict subset of that (0.95 * max_w >
dev_max implies max_w > dev_max) and could never be the rule that stopped a
start. The residue is the state where hass.limits() answered (None, None) -
ESPHome not up at boot, range checks skipped rather than guessed at, rating
never re-read - and there is nothing to refuse there by construction, which is
why the answer is a runtime row. Recorded in validate.py's "deliberately not
refused" list and pinned by four checks in test_validate.py, so the decision's
premise goes red if rule 2 is ever softened back to a warning.

HOW REACHABLE THIS IS, stated rather than glossed. Because rule 2 refuses the
configuration whenever the rating is known, min(max_w, dev_max) IS max_w on
every install that reaches the control loop with a rating it could read, and
this change is arithmetically dead there. That is clamp.py's argument for its
own second stage word for word: the case where it bites is the case where the
startup gate did not run or no longer holds, and a guard that quietly assumes
the gate ran is a guard that reports armed in exactly the state it cannot fire
in.

Verified by breaking it, per this project's rule. Reverting rail_w() to bare
max_w fails nine checks; deleting the NOT COVERED handling fails five. The one
that mattered: shipping the corrected predicate while NOT passing the rating
from check_meter_frozen() left the entire suite green, because nothing graded
the caller - so test_freeze.py part 5b now drives the real cycle() on a +-4700 W
device and asserts on the notification that leaves the process, and the
unwiring mutation fails three checks there. The peak-guard call site's wiring
is still ungraded (behind p1_enabled, needs a fed averager for frozen to have
any observable consequence) and that gap is named in the file rather than
implied.

Full suite: 15/15 in goodwe-addon-ci.


What a reviewer should look hardest at

  1. The reachability claim. I am asserting validate.py rule 2 makes this
    arithmetically dead on any install that starts. If there is a path into
    run_control()'s loop with dev_max known and below max_w, the AC 4
    decision changes and a refusal may be warranted after all. I traced
    dev_min/dev_max to a single assignment (main.py:1117) immediately
    followed by the gate, and amain()'s earlier gate, and found none - but it
    is the load-bearing premise of this PR and deserves a second pair of eyes.

  2. Symmetric ceiling vs per-sign rails. On a device range of, say,
    -500..+5000 the symmetric ceiling drops the discharge rail to 475 W too.
    I argued that is the safe direction (term 1 carries the no-false-positive
    argument) and that CFG-01 refuses the case anyway. Someone may reasonably
    prefer the per-sign version; freeze.py records what it would cost.

  3. freeze.py now imports clamp.py for is_commandable(), so a NaN
    rating cannot become a 0 W ceiling. That is a new module dependency in a
    file that was deliberately dependency-light. No cycle (clamp imports
    nothing from freeze), but it is a judgement call.

  4. The ungraded peak-guard call site. looks_frozen(...) inside the
    SAFETY-07 block now takes the device range, and nothing grades that wiring -
    it is behind p1_enabled and blindness() returns BLIND_NO_BLOCK before
    it ever reads frozen, so there is no observable consequence without a fed
    averager. Named in test_freeze.py rather than papered over. If that gate
    is worth covering it is a fixture-widening ticket of its own.

SAFETY-18's conjunction needs `abs(commanded_w) >= 0.95 * max_w`, but the emitted command is clipped to the setpoint device's own [dev_min, dev_max] range as well as to max_w - in clamp_to_rating()'s second stage and again as the arbiter's `device` P_SAFETY limit claim. Measured on the shipped predicate at the ticket's numbers, max_w 5000 on a +-4700 W inverter: looks_frozen(unchanged_s=3600, commanded_w=4700, max_w=5000) -> False The command is pinned at the hardware's rail, the meter has been frozen for an hour, and term 2 is unsatisfiable by construction because 4700 < 4750. Any device rating at or below 0.95 * max_w disarms the conjunction outright, and the status page then printed `armed (... command is over 4750 W)` - a guard that cannot fire, advertising that it can, which is the failure SAFETY-15, SAFETY-19 and validate.py rule 5 all exist to prevent. The rail is now measured against min(max_w, |dev_min|, |dev_max|), through a new `rail_ceiling_w()` threaded into looks_frozen(), FreezeWatchdog.evaluate() and both call sites in main.py. An unread rating leaves the old behaviour exactly, and correctly: hass.limits() returns None whenever ESPHome is not up, and on that path no device clamp is applied either, so max_w really is the ceiling. A rating larger than max_w cannot raise the rail - this is a min(), not a swap. THE DELIBERATE CHOICE, since SAFETY-03 has landed. The arithmetic was kept rather than substituting Decision.i_clamped / Decision.clamped. i_clamped is the integrator bound (control.py:208, `bounded != i_w`), not "the command is at the device rating", and `clamped` is the max_w output bound, unreachable on a default install - neither is the device rail. More decisively, both answer "did the LOOP ask for more than a bound", while the rail term asks "is the command standing at the ceiling", and those coincide only when the loop is the claimer that won. A maintenance SET or a peak-guard floor reaches the ceiling through arbiter LIMIT claims with no clamp engaging at all, so a boolean rail would go False exactly during a maintenance phase running against a frozen meter - which freeze.py's own docstring names as the same fault with a worse actuator behind it. The predicate is also public with two call sites that pass DIFFERENT command values (self.target, and `standing` = last_write); a per-cycle boolean cannot be passed per call site, so substituting it would have silently decided the pre-clamp-versus-wire question that PM recorded as still open. AC 1 governs, and it is satisfied literally. The smaller of the two device bounds, not a per-sign pair. A per-sign rail is tighter and is not wrong, but it is two rails needing two coverage verdicts on a status row that carries one number, and the only configuration that distinguishes them is refused at startup anyway. The symmetric ceiling can only lower the rail, which widens term 2 rather than disarming it; term 1 carries the whole no-false-positive argument and is untouched. A ceiling of 0 W is NOT COVERED, not "armed at 0 W". A device that can command nothing in one direction would make `abs(command) >= 0` true of every command including a parked one - term 2 degenerately TRUE, collapsing the conjunction into the bare unchanged_s > N threshold TEL-05 refused for false-tripping at this controller's own operating point. rail_unreachable() returns the reason, looks_frozen() stays silent, and the page uses the NOT COVERED row the unsupported-transport case already has - a second reason to use that surface, not a new mechanism. AC 4, decided rather than assumed: NO NEW validate.py RULE. The configuration is max_w above the device rating, which rule 2 (CFG-01) already refuses on the identical predicate, re-run by run_control() once the real rating is known: validate_startup_config({... max_w: 5000}, -4700, 4700) -> ['max_w is 5000 W but number.sp accepts at most 4700 W - THE DEVICE WINS ...'] A rail-specific rule would fire on a strict subset of that (0.95 * max_w > dev_max implies max_w > dev_max) and could never be the rule that stopped a start. The residue is the state where hass.limits() answered (None, None) - ESPHome not up at boot, range checks skipped rather than guessed at, rating never re-read - and there is nothing to refuse there by construction, which is why the answer is a runtime row. Recorded in validate.py's "deliberately not refused" list and pinned by four checks in test_validate.py, so the decision's premise goes red if rule 2 is ever softened back to a warning. HOW REACHABLE THIS IS, stated rather than glossed. Because rule 2 refuses the configuration whenever the rating is known, min(max_w, dev_max) IS max_w on every install that reaches the control loop with a rating it could read, and this change is arithmetically dead there. That is clamp.py's argument for its own second stage word for word: the case where it bites is the case where the startup gate did not run or no longer holds, and a guard that quietly assumes the gate ran is a guard that reports armed in exactly the state it cannot fire in. Verified by breaking it, per this project's rule. Reverting rail_w() to bare max_w fails nine checks; deleting the NOT COVERED handling fails five. The one that mattered: shipping the corrected predicate while NOT passing the rating from check_meter_frozen() left the entire suite green, because nothing graded the caller - so test_freeze.py part 5b now drives the real cycle() on a +-4700 W device and asserts on the notification that leaves the process, and the unwiring mutation fails three checks there. The peak-guard call site's wiring is still ungraded (behind p1_enabled, needs a fed averager for `frozen` to have any observable consequence) and that gap is named in the file rather than implied. Full suite: 15/15 in goodwe-addon-ci. --- ## What a reviewer should look hardest at 1. **The reachability claim.** I am asserting `validate.py` rule 2 makes this arithmetically dead on any install that starts. If there is a path into `run_control()`'s loop with `dev_max` known and below `max_w`, the AC 4 decision changes and a refusal may be warranted after all. I traced `dev_min`/`dev_max` to a single assignment (`main.py:1117`) immediately followed by the gate, and `amain()`'s earlier gate, and found none - but it is the load-bearing premise of this PR and deserves a second pair of eyes. 2. **Symmetric ceiling vs per-sign rails.** On a device range of, say, -500..+5000 the symmetric ceiling drops the *discharge* rail to 475 W too. I argued that is the safe direction (term 1 carries the no-false-positive argument) and that CFG-01 refuses the case anyway. Someone may reasonably prefer the per-sign version; freeze.py records what it would cost. 3. **`freeze.py` now imports `clamp.py`** for `is_commandable()`, so a NaN rating cannot become a 0 W ceiling. That is a new module dependency in a file that was deliberately dependency-light. No cycle (clamp imports nothing from freeze), but it is a judgement call. 4. **The ungraded peak-guard call site.** `looks_frozen(...)` inside the SAFETY-07 block now takes the device range, and nothing grades that wiring - it is behind `p1_enabled` and `blindness()` returns `BLIND_NO_BLOCK` before it ever reads `frozen`, so there is no observable consequence without a fed averager. Named in `test_freeze.py` rather than papered over. If that gate is worth covering it is a fixture-widening ticket of its own.
admin added 1 commit 2026-09-01 02:14:30 +02:00
SAFETY-18's conjunction needs `abs(commanded_w) >= 0.95 * max_w`, but the
emitted command is clipped to the setpoint device's own [dev_min, dev_max]
range as well as to max_w - in clamp_to_rating()'s second stage and again as
the arbiter's `device` P_SAFETY limit claim. Measured on the shipped predicate
at the ticket's numbers, max_w 5000 on a +-4700 W inverter:

    looks_frozen(unchanged_s=3600, commanded_w=4700, max_w=5000)  ->  False

The command is pinned at the hardware's rail, the meter has been frozen for an
hour, and term 2 is unsatisfiable by construction because 4700 < 4750. Any
device rating at or below 0.95 * max_w disarms the conjunction outright, and
the status page then printed `armed (... command is over 4750 W)` - a guard
that cannot fire, advertising that it can, which is the failure SAFETY-15,
SAFETY-19 and validate.py rule 5 all exist to prevent.

The rail is now measured against min(max_w, |dev_min|, |dev_max|), through a
new `rail_ceiling_w()` threaded into looks_frozen(), FreezeWatchdog.evaluate()
and both call sites in main.py. An unread rating leaves the old behaviour
exactly, and correctly: hass.limits() returns None whenever ESPHome is not up,
and on that path no device clamp is applied either, so max_w really is the
ceiling. A rating larger than max_w cannot raise the rail - this is a min(),
not a swap.

THE DELIBERATE CHOICE, since SAFETY-03 has landed. The arithmetic was kept
rather than substituting Decision.i_clamped / Decision.clamped. i_clamped is
the integrator bound (control.py:208, `bounded != i_w`), not "the command is
at the device rating", and `clamped` is the max_w output bound, unreachable on
a default install - neither is the device rail. More decisively, both answer
"did the LOOP ask for more than a bound", while the rail term asks "is the
command standing at the ceiling", and those coincide only when the loop is the
claimer that won. A maintenance SET or a peak-guard floor reaches the ceiling
through arbiter LIMIT claims with no clamp engaging at all, so a boolean rail
would go False exactly during a maintenance phase running against a frozen
meter - which freeze.py's own docstring names as the same fault with a worse
actuator behind it. The predicate is also public with two call sites that pass
DIFFERENT command values (self.target, and `standing` = last_write); a
per-cycle boolean cannot be passed per call site, so substituting it would
have silently decided the pre-clamp-versus-wire question that PM recorded as
still open. AC 1 governs, and it is satisfied literally.

The smaller of the two device bounds, not a per-sign pair. A per-sign rail is
tighter and is not wrong, but it is two rails needing two coverage verdicts on
a status row that carries one number, and the only configuration that
distinguishes them is refused at startup anyway. The symmetric ceiling can only
lower the rail, which widens term 2 rather than disarming it; term 1 carries
the whole no-false-positive argument and is untouched.

A ceiling of 0 W is NOT COVERED, not "armed at 0 W". A device that can command
nothing in one direction would make `abs(command) >= 0` true of every command
including a parked one - term 2 degenerately TRUE, collapsing the conjunction
into the bare unchanged_s > N threshold TEL-05 refused for false-tripping at
this controller's own operating point. rail_unreachable() returns the reason,
looks_frozen() stays silent, and the page uses the NOT COVERED row the
unsupported-transport case already has - a second reason to use that surface,
not a new mechanism.

AC 4, decided rather than assumed: NO NEW validate.py RULE. The configuration
is max_w above the device rating, which rule 2 (CFG-01) already refuses on the
identical predicate, re-run by run_control() once the real rating is known:

    validate_startup_config({... max_w: 5000}, -4700, 4700)
      -> ['max_w is 5000 W but number.sp accepts at most 4700 W - THE DEVICE
          WINS ...']

A rail-specific rule would fire on a strict subset of that (0.95 * max_w >
dev_max implies max_w > dev_max) and could never be the rule that stopped a
start. The residue is the state where hass.limits() answered (None, None) -
ESPHome not up at boot, range checks skipped rather than guessed at, rating
never re-read - and there is nothing to refuse there by construction, which is
why the answer is a runtime row. Recorded in validate.py's "deliberately not
refused" list and pinned by four checks in test_validate.py, so the decision's
premise goes red if rule 2 is ever softened back to a warning.

HOW REACHABLE THIS IS, stated rather than glossed. Because rule 2 refuses the
configuration whenever the rating is known, min(max_w, dev_max) IS max_w on
every install that reaches the control loop with a rating it could read, and
this change is arithmetically dead there. That is clamp.py's argument for its
own second stage word for word: the case where it bites is the case where the
startup gate did not run or no longer holds, and a guard that quietly assumes
the gate ran is a guard that reports armed in exactly the state it cannot fire
in.

Verified by breaking it, per this project's rule. Reverting rail_w() to bare
max_w fails nine checks; deleting the NOT COVERED handling fails five. The one
that mattered: shipping the corrected predicate while NOT passing the rating
from check_meter_frozen() left the entire suite green, because nothing graded
the caller - so test_freeze.py part 5b now drives the real cycle() on a +-4700 W
device and asserts on the notification that leaves the process, and the
unwiring mutation fails three checks there. The peak-guard call site's wiring
is still ungraded (behind p1_enabled, needs a fed averager for `frozen` to have
any observable consequence) and that gap is named in the file rather than
implied.

Full suite: 15/15 in goodwe-addon-ci.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
admin added 1 commit 2026-09-01 02:51:54 +02:00
Merge origin/release/1.0 into SAFETY-20
checks / unit (pull_request) Successful in 3m36s
29fa39eda7
# Conflicts:
#	goodwe_controller/CHANGELOG.md
admin merged commit 6ea79f1043 into release/1.0 2026-09-01 02:57:59 +02:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: admin/goodwe-addon#28