83911ed63785b6b22217921bc2d6c9107e206891
8
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  adminandClaude Opus 5
|
83911ed637 |
SAFETY-20: measure the freeze rail against the reachable ceiling, not max_w
checks / unit (pull_request) Successful in 3m37s
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>
|
||
|
glenn schrooyenandClaude Opus 5
|
611d3dbcdf |
TEST-06 review follow-ups: two notification surfaces, the real ordering boundary, the schema bounds
1. notes_of() could not tell this guard's alarm from anybody else's. It selected by SERVICE, so against #12's app/ (which adds goodwe_output_clamp) eight assertions went red, AC1's core check among them. Blanket id-keying is not the repair and was measured, not guessed: selector vs #12's app/ id renamed by service (as committed) 8 red 1 red by id alone 3 red 6 red by id or title (shipped) 3 red 1 red all_notes_of() is the whole surface; notes_of() is this guard's, recognised by the id it should carry OR the title it writes - the `or` is what keeps the one assertion whose subject IS the id able to fail on its own. The three surface-wide claims stay surface-wide, each with a comment naming which surface it is about and why the next guard's author must decide rather than narrow them. They are the three that go red against #12, which is the correct signal. 2. The ordering comment named the wrong boundary. N3 - moving the call to immediately before write_setpoint() - is green and correctly so: self.target is assigned two lines earlier, write_setpoint() clamps only a local and can skip the write entirely. So the invariant is "after `self.target = resolution.target_w`", and the detector grades pre-clamp INTENT, not the wire. Corrected in test_freeze.py and in main.py's own note above the call (comment-only; I merge second and own the conflict). N8 - moving the call above the assignment - is RED 4. 3. The schema range's own bounds are now pinned. int(60,3600) -> int(1,86400) survived green: an operator could set meter_freeze_s: 86400 and the guard is dead with nothing red. Both bounds are re-derived rather than restated. 4. find_hwsim() crashed the whole suite on a checkout one level below the root (HERE.parents[1] -> IndexError, reproduced at C:/goodwe_controller). Guarded. And part 3 now names its transport in the summary: the rig hwsim and the stub are not equivalent - the rig path failed 1 run in 3 - so a green log has to say which one backed it. Also: the "revived transport re-rails" check is labelled a PRECONDITION like its sibling; freeze_rows() records that flipping p1_enabled exercises the row's content and never its gate; and the note about the deleted assertion is corrected - it was REDUNDANT, not undecidable. The reviewer falsified it in one line (move the hass.call above `if not fired: return`, RED 9). "I could not construct a falsifier, therefore it cannot fail" was exactly the inference this file exists to distrust. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
glenn schrooyenandClaude Opus 5
|
c34b5a2922 |
TEST-06: close the three gaps that survived SAFETY-18's suite green
None of the three is a bug in the shipped code. They are gaps in what the
suite could detect, and the next person to touch freeze.py had less
protection than the passing count implied.
1. WHERE check_meter_frozen() runs in cycle(). Moving it to the top left
the whole file green. Pinned by CONSEQUENCE, not by source order, so a
refactor that keeps the semantics stays green:
- the alarm must land on the SAME cycle the command reaches the rail
(at the top it grades the previous command against this cycle's
meter and arrives a cycle late);
- the dead-poll-task composition: poll task dies, stale_input_s is
crossed, the arbiter commands 0 W, and on THAT cycle the alarm is
already clear - because the detector ran after the 0 W write.
wired() now returns a per-cycle trace, which is what makes "on which
cycle" an observation rather than a hope.
2. The NOT COVERED status row. The old check only asserted that an
uncovered transport is SILENT, which a page claiming `armed` on a guard
that cannot run also satisfies. The string is now asserted in both
directions: the uncovered row must say NOT COVERED, name the
meter_source and homewizard_local, and warn; the covered row must still
say armed, or "does not say armed" would be satisfied by deleting the
armed branch.
3. meter_freeze_s. Pinned in test_validate.py by MAINT-04's regex
technique (no PyYAML in the image), and in test_freeze.py by its
consequences: the shipped value drives the replays, must stay inside
the 900 s billing quarter, and 3600/hold_s must stay inside the shipped
deadband. A Controller built with the key ABSENT must land on the same
number, which pins main.py's fallback without touching CFG-03.
All three demonstrated red before being accepted green, and every new
check was mutated to confirm it can fail; one candidate assertion ("no new
notification while parked at 0 W") was DELETED because no mutation could
break it, and replaced by the exact notification count across a revived
transport, which can.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
|
||
|
glenn schrooyenandClaude Opus 5
|
ffb8ec6373 |
MAINT-04 review: pin the default divergence with a check, not a comment
validate.py's comment said "if config.yaml's value moves, move this and main.py's together" and nothing enforced it - a comment where a check belonged, in a PR that elsewhere proves it knows the difference. test_validate.py now reads main.py and maintenance.py directly and asserts the divergence EXACTLY AS IT STANDS: config.yaml/validate.py at 2000, main.py's two fallbacks and MaintConfig's dataclass default at 2500. It records the mismatch rather than demanding it be closed, so it passes today and goes red if either side moves - including when somebody closes it in main.py, at which point the check and validate.py's block are deleted together. Nothing in a file another ticket owns is touched. The consequence is asserted through the real code as well as the literals: with the key absent the validator accepts, MaintConfig builds 2500 W against a 2000 W ceiling, and the arbiter resolves it to 0 W as a contradiction - the bug rule 6 exists to catch, passing rule 6. Production-unreachable (the Supervisor always supplies every key) but fully reachable from sim/inject.py and the tests, which is where anyone debugging maintenance works. Also: a CHANGELOG section for the upgrade path, covering both the case where the Supervisor re-derives the default and the case where it persisted the old one, since which applies was not verifiable without probing the live install. And maintenance.py's timeout comment said 13.5 kWh where everything else says 13.8 (300 Ah x 51.2 V, DoD 90); SAFETY-18 does not touch that file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
glenn schrooyenandClaude Opus 5
|
878a6ce452 |
MAINT-04: refuse a maintenance charge the ceiling cannot deliver
config.yaml shipped max_w 2000 against maintenance_charge_w 2500. The cheap-window shaping files Claim.limit(hi = -maintenance_charge_w) against the supervised SAFETY limit's lo = -max_w, so lo > hi - the arbiter's clause 3, correctly resolved to 0 W and logged as a CONTRADICTION. Maintenance silently never charged in a cheap window on a default install, and the only evidence read like a bug in the arbiter rather than like the two numbers the operator set. validate_startup_config() now refuses maintenance_charge_w > max_w, the same shape as CFG-01's sign_disagree_w >= max_w. Equality is reachable (-max_w is a single valid point) and is not refused. Demonstrated by starting the add-on on the pre-fix pair: exit 78 and the refusal banner. maintenance_discharge_w was checked rather than assumed and is NOT the same case: the drain phase OWNS the actuator (Claim.set at P_MAINTENANCE), so the ceiling clamps it to +2000 W and the arbiter's explain() names the clamp every cycle. Loud, and it still moves energy - warned, not refused. The shipped maintenance rates drop to 2000 so the defaults pass the rule; max_w was deliberately not raised, because it is the supervised safety ceiling. test_validate.py now reads config.yaml itself, so a default the validator refuses cannot ship again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
glenn schrooyenandClaude Opus 5
|
9aed43990b |
SAFETY-02: a loop can be fresh, tracking and inside its rating and still push the wrong way
The stale-input watchdog asks whether the meter is still talking. The saturation freeze asks whether the inverter can follow. The output clamp asks whether the number is too big. A loop can pass all three while charging the battery out of the grid, because the only thing wrong is the sign. On 2026-08-20 a commercial controller did exactly that four times in one day, for up to 77 minutes at a stretch, reaching a commanded -14 768 W against a ±5000 W inverter. app/watchdog.py compares the emitted setpoint against the authoritative P1 figure and, when they oppose each other with both over 300 W for 180 s continuously, files a failsafe SET claim at P_SAFETY, commands 0 W and latches there until an operator starts control again. Validated against that day rather than against an argument: sim/replay_sign.py in the infra repo drives this module over the committed captures of both 2026-08-20 and the healthy 2026-08-23 and reaches 18 trips on the fault day - every one inside a window NOTES.md §4.7 accounts for - and 0 on the healthy day. The four episodes do share one signature: 0.3 / 0.9 / 10.5 / 0.0 % sign agreement inside them against 92.9 % healthy. Three decisions the ticket could not make: - It is DISARMED whenever the grid-following loop is not what is driving. Sign agreement is a property of a loop that follows the meter, and a maintenance grid charge opposes the meter by design - 2500 W of charge into a house drawing 500 W is +3000 W on the meter against a -2500 W command, for hours. The gate tests the emitted number, not the winning claim's name, because a limit claim can let the loop "win" and still replace its value. - The trip LATCHES. Commanding 0 W removes the command side of the comparison, so the disagreement always clears the moment the trip works. - sign_disagree_w at or above max_w is refused at startup. The arbiter never lets a command exceed max_w, so such a threshold leaves the watchdog incapable of firing while the status page still reports it armed. What it does not catch, said out loud: an inverter that ACTS opposite to its command moves the meter further the same way, leaving command and meter in agreement. That is the saturation freeze's job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
547d796c5d |
CFG-01 review: widen the rule, refuse the firmware-veto case, fix the stop footgun
Review findings, all confirmed in code before changing anything. THE RULE DID NOT AUTHORISE ITS OWN BEST REFUSAL. The stated test was "makes a safety mechanism inoperative, or makes a stated limit untrue", and a blank setpoint_entity does neither - layer 1 pinning the inverter at 0 W is the failsafe WORKING, as validate.py's own text admitted. The real justification is a second, separate category: the add-on is inert and indistinguishable from healthy. Stated as (a) and (b), with each refusal tagged, and with both halves explicitly bounded - (a) is not "any two numbers that could disagree", (b) is not "anything degraded". Left implicit, the gap is where trivia-creep comes back in. meter_max_age_s ABOVE THE FIRMWARE'S VETO IS NOW REFUSED. The schema allows int(5,300); the firmware's threshold is a compile-time `max_age_s: "30"` and it trips layer 1 at `p1_age_s >= 30`. At 120 the add-on believes a 119 s reading is fresh and keeps commanding while the board has already vetoed and is holding at 0 W - healthy meter, running controller, nothing moving. Structurally identical to the max_w case, and unlike max_w it is settable on this install, so the headline behaviour finally has a live demonstration. That firmware file's own comment says a lower bound on this "belongs in CFG-01, not here". Only ABOVE is refused: below the veto the add-on gives up first, which is fail-safe and visible on its own status page. Scoped to P1 being enabled, because meter_max_age_s governs nothing otherwise. FIRMWARE_INGEST_VETO_S cannot be imported from a compile-time YAML substitution, so it carries a note on the single direction in which it can go stale - and a warning that TEL-06 is editing `ingest_stale`, so a matching integer may stop meaning a matching rule. stop= WAS A LIVE FOOTGUN. It defaulted to None, which made a Controller that cannot stop itself the DEFAULT one. On the late-refusal path that meant: log the banner, return, and leave the process alive forever with no control loop and no exit - the exact "inert but looks healthy" failure this module exists to refuse. Now required and keyword-only, so the broken shape does not construct; test_p1.py's five call sites pass an Event. That whole path had no test and no live demo. It has one now: it drives run_control() against a device that only reports a narrower range once the loop starts, and asserts it refuses, signals, returns, and writes NOTHING - plus the converse, that an agreeing device leads to real writes. This also covers the max_w composition that cannot be reached through the Supervisor on the live install. max_w <= 0 is refused. Schema-blocked in production, but validate_startup_config() is the simulation suite's entry point too, and a non-positive max_w inverts the arbiter's supervised bounds into a contradiction that reads as a safety trip rather than a typo. Verified live 2026-08-28 (backup goodwe_controller-cfg01r2-1787940076): meter_max_age_s 120 refused with a message naming 120, 30, and THE FIRMWARE WINS; no traceback; add-on restored byte-exact and running. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
fc9bd89dca |
CFG-01: refuse to start on a configuration that cannot be honoured
main.py used to read the setpoint device's own +-5000 W rating, notice a
configured max_w above it, log one WARNING, and run anyway with
write_setpoint() quietly clamping. The operator's stated ceiling was never
the real ceiling and the only evidence had scrolled off the log. specs
§1.5 is explicit that such a config is refused at startup, not survived at
runtime.
validate_startup_config() runs in amain() before the broker, the web
server or any task exists, so a refusal cannot leave half a controller
running behind a UI. It refuses exactly three things, all sharing one
property - continuing makes a safety mechanism inoperative or makes a
stated limit untrue:
1. max_w outside the setpoint device's own rating.
2. No setpoint_entity: every write is a no-op, the device rating is
never read, and the ESP32 heartbeat that holds off the layer-1
failsafe never happens. It ran, looked healthy, and moved nothing.
3. A meter_source that can never produce a reading. These were already
caught, but they only DISABLED ingestion, which leaves grid None
forever: permanent inputs-missing, 0 W, and a status page
indistinguishable from a healthy meter nobody has fed yet.
Those last predicates move into p1.validate_meter_options(), which
build_source() also calls - one definition, two callers, not two
competing validators. ha_dsmr with half its entities and mqtt_p1 with a
blank topic gained the check the other two transports already had.
Deliberately not refused, because untidy is not unsafe: integrator_max_w
above max_w (the arbiter still bounds the wire to +-max_w, so the cost is
unwind latency - warned), a missing soc/batt entity (fail-safe and
already a red NOT READY banner), a setpoint entity that has not appeared
on the Core API yet (ESPHome is routinely late; re-checked in
run_control() once the real rating arrives), and a thin meter_poll_s
margin.
A refusal is legible as one: a banner saying in words that this is
configuration and nothing is broken, numbered problems, no traceback, and
exit 78 (EX_CONFIG).
Verified on the live add-on 2026-08-28, not by reading the code: a blank
setpoint_entity and a meter_poll_s equal to meter_max_age_s each stopped
the add-on with the banner and no stack trace.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
|