From 22ca341459380f6e796f89901580ea2008a82007 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Tue, 8 Sep 2026 15:01:55 +0200 Subject: [PATCH] ARCH-02: assert the invariant, because a fixture cannot enumerate arrangements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review approved and then attacked part 5b, the control that stands between B1's defect and a green suite. It got through in three edits: A1 delete `END LEG` 5b CATCHES it 2 FAIL A2 ...and reorder the legs 5b BLIND 1 FAIL A3 ...and reword the closing summary 0 FAIL, "all checks passed" with a splitter that closed ZERO legs ⚠️ TWO OF THOSE THREE ARE ORDINARY REFACTORS - reordering report legs, rewording a summary line - and neither reads as touching a safety check. 5b caught the unclosed splitter only because the clean leg happened to be printed LAST, and the backstop that saved A2 was a hardcoded transcription of the tool's own sentence, so rewording that sentence blinded it too. FIXED BY ASSERTING THE STRUCTURE RATHER THAN ADDING A FIXTURE: a leg may not open while one is open, and every leg that opens must be explicitly closed. Driven - deleting `END LEG` now fails on the FIRST edit, whatever the order or the wording: AssertionError: leg 'SAME-STEP' opened while 'STAGED' was still open - the tool stopped closing its legs, and a splitter that never closes swallows the closing summary into the last leg. All three attacks include that edit, so one assertion kills the class. Clean tree unchanged: 28/28, all checks passed. ⭐ AND THE RULE IN MUTATION.md IS EXTENDED, because review's finding is the general case of it. It said "a per-thing check that has never seen the things DISAGREE is a total". Part 5b DOES make them disagree, every run, and was still blind. So: ...AND ONE THAT HAS ONLY SEEN THEM DISAGREE IN ONE ARRANGEMENT IS A TOTAL OVER THAT ARRANGEMENT. When a fixture cannot enumerate the arrangements, assert the invariant they share. Also recorded, NOT fixed, with the reason: the reference carries `entity_id unique_id device` and grades all three, but never the discovery TOPIC. Review measured `homeassistant/... -> ha/...` exiting 0 with no findings while every announced entity dies. `discovery_payloads()` already returns the topic and `announced_rows` discards it, so a fourth column closes it - but the prefix is HA's fixed contract rather than a name this project chooses, so it is not what a tidy-up rename reaches. Lower LIKELIHOOD than the N1 gap it resembles, not lower consequence, and it stops being a note the day the topic becomes configurable. Two stale figures corrected: the tool claimed five migration mutations where there are nine, and pointed at a DOCS.md heading that does not exist (the real one is "Before you commission inverter B", verified present at DOCS.md:1577). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/MUTATION.md | 14 +++++++++++ goodwe_controller/test_rehearse.py | 28 +++++++++++++++++++++- goodwe_controller/tools/rehearse_roster.py | 25 +++++++++++++++++-- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/goodwe_controller/MUTATION.md b/goodwe_controller/MUTATION.md index ad2a48c..3e2af21 100644 --- a/goodwe_controller/MUTATION.md +++ b/goodwe_controller/MUTATION.md @@ -283,6 +283,20 @@ same place twice, the check is decoration. has never seen the things DISAGREE is a total.** Part 5b exists to make them disagree on every run. + ⚠️ **AND ONE THAT HAS ONLY SEEN THEM DISAGREE IN ONE ARRANGEMENT IS A TOTAL + OVER THAT ARRANGEMENT.** Review found this by attacking the fix itself. Part + 5b does make the legs disagree, on every run - and was still blind to a build + that merely REORDERS them, because it had only ever seen the clean leg + printed LAST. Three edits, two of them ordinary refactors (reorder the report + legs, reword a summary line), produced `all checks passed` from a splitter + that closed ZERO legs. + + A fixture cannot enumerate the arrangements, so the STRUCTURE is asserted + instead: a leg may not open while one is open, and every leg that opens must + be explicitly closed. That fires on the FIRST edit of all three attacks, + whatever the order or the wording. **When a fixture cannot enumerate the + arrangements, assert the invariant they share.** + * **CFG-04's `tools/supervisor_shape_probe.py` §B** — and this one is the rule reaching outside the suite. The probe is the committed evidence that `config.yaml`'s `inverters: []` default is load-bearing, and four documents diff --git a/goodwe_controller/test_rehearse.py b/goodwe_controller/test_rehearse.py index 7ae1ec2..ba24b51 100644 --- a/goodwe_controller/test_rehearse.py +++ b/goodwe_controller/test_rehearse.py @@ -246,15 +246,41 @@ def legs(out: str) -> dict: leg is genuinely clean: it still reported SAME-STEP disarmed. Part 5b is the running control for exactly that. """ - out_legs, current = {}, None + out_legs, current, closed = {}, None, set() for line in out.splitlines(): if line.startswith(" LEG "): + # ⚠️ AND THIS ASSERTION IS THE ONE THAT KILLS THE CLASS. Review + # attacked part 5b directly and got through in three edits: delete + # `END LEG` (5b catches it), ALSO reorder the legs (5b goes blind - + # it only ever saw them disagree with the clean leg LAST), and ALSO + # reword the closing summary (the backstop is a transcription of the + # tool's own sentence, so rewording blinds that too). Result: "all + # checks passed" with a splitter that closed ZERO legs. + # + # Two of those three are ordinary refactors - reordering report legs + # and rewording a summary line - neither of which reads as touching + # a safety check. A fixture cannot cover every arrangement, so the + # STRUCTURE is asserted instead: a leg may not open while one is + # open, and every leg that opens must be explicitly closed. Under + # all three attacks nothing is ever closed, so this fires on the + # FIRST edit regardless of order or wording. + assert current is None, ( + f"leg {line.strip()[4:].strip()!r} opened while {current!r} was " + "still open - the tool stopped closing its legs, and a splitter " + "that never closes swallows the closing summary into the last " + "leg. That is the defect this function exists to fix.") current = line.strip()[4:].strip() out_legs[current] = [] elif line.startswith(" END LEG "): + assert current is not None, "END LEG without an open leg" + closed.add(current) current = None elif current is not None: out_legs[current].append(line) + assert current is None, f"leg {current!r} was never closed" + assert closed == set(out_legs), ( + f"legs opened {sorted(out_legs)} but closed {sorted(closed)} - a leg " + "that is never closed absorbs everything after it") return {k: "\n".join(v) for k, v in out_legs.items()} diff --git a/goodwe_controller/tools/rehearse_roster.py b/goodwe_controller/tools/rehearse_roster.py index 3c8c539..69e2150 100644 --- a/goodwe_controller/tools/rehearse_roster.py +++ b/goodwe_controller/tools/rehearse_roster.py @@ -37,13 +37,14 @@ lands on the copy. The scratch file is unique per invocation, which is habit 18. ⚠️ IT DOES NOT TOUCH THE LIVE BOX AND CANNOT. There is no SSH, no Supervisor call and no HTTP in this file. Fetching the state file is an OWNER STEP and it -is written down in DOCS.md under "Rehearsing the roster switch (ARCH-02)". +is written down in DOCS.md under "Before you commission inverter B: +tools/rehearse_roster.py (ARCH-02)". EXIT CODES 0 the transition is clean: nothing vanished, nothing was stranded under a name no machine reads, no entity was orphaned, no history was lost. 1 ⭐ AT LEAST ONE OF THOSE HAPPENED. This is the code that must be - reachable: `test_rehearse.py` mutates the shipped migration five + reachable: `test_rehearse.py` mutates the shipped migration nine different ways and requires this exit code for each. 2 the rehearsal could not be performed (bad file, unparseable config, the entity-id model failed its own anchor). ⚠️ NOT A PASS. @@ -292,6 +293,26 @@ class Row: f"{self.device_id})") +# ⚠️ WHAT THE REFERENCE STILL DOES NOT CARRY: THE DISCOVERY TOPIC. +# +# The three columns are `entity_id unique_id device`, and the payload is graded +# against all three. The TOPIC is not. Measured by review: +# +# homeassistant/sensor/... -> ha/sensor/... exit 0, NO FINDINGS +# sensor -> binary_sensor on the unit rows exit 0, NO FINDINGS +# +# In the first, HA never receives any discovery at all and every announced +# entity dies - reported clean. `discovery_payloads()` already returns the +# topic; `announced_rows` discards it, so a fourth column closes it. +# +# ⚠️ RECORDED AND NOT FIXED, deliberately, with the reason: the prefix is HA's +# fixed contract rather than a name this project chooses, so it is not the kind +# of value a tidy-up rename reaches - which is exactly the risk `mqtt.py`'s own +# header warns about for `unique_id`. That makes it lower-likelihood than the +# N1 gap it resembles, not lower-consequence. If the topic ever becomes +# configurable, this stops being a note and becomes the same defect. + + def announced_rows(opts: dict) -> "dict[str, Row]": """`{entity_id: Row}` for every row this add-on announces on `opts`.