71cd7849e4ac7e30bc4fc0f3fecfa4111e38423b
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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
|