From e06906d65f93320f5a456f444d2daf3a4bf6ed74 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Tue, 8 Sep 2026 13:15:05 +0200 Subject: [PATCH 1/7] ARCH-02: rehearse the flat-to-roster switch, and it comes up disarmed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing had ever run the real `Controller` through the transition inverter B's commissioning performs on 2026-09-12. `tools/rehearse_roster.py` does, over a COPY of a state file given as an input path, and reports the difference: every store key that moved, appeared, vanished or was STRANDED under a name no machine reads; every entity id before and after with orphans named; and whether the maintenance history MAINT-05 moved once survived being moved again. ⭐ WHAT IT FOUND, AND IT IS SAFETY-44's ANSWER. A store that ran pre-DEV-02 flat and declares a multi-inverter roster comes up with its SAFETY-02 latch DISARMED unless the first roster entry happens to be called `inverter-a`. `Controller.__init__` hands `migrate_unit_keys()` the hardcoded `UNIT_A`, so the latch lands in `unit/inverter-a/sign_tripped`; a roster of ["garage","shed"] reads `unit/garage/sign_tripped`, which is absent. And `_grade_store_migration()` - the guard written for exactly this - sees the migration's OWN misdirected key satisfy its `namespaced_latched` test and returns without re-arming. Measured, not argued: the file says `sign_tripped: true`, no inverter is latched, nothing is held at 0 W. The same state file onto a roster whose first entry IS `inverter-a` comes up latched, so the finding is attributable to the name. The staged upgrade reaches it too: the flat start stamps `schema_version`, and the roster start's migration then takes its early return. ⚠️ NO RUNTIME CODE IS CHANGED HERE. The fix to `migrate_unit_keys()` belongs to SAFETY-44 reopened as a defect, shown red before it; this commit is the instrument that shows it red. THE CHECK CAN FAIL, AND WAS SEEN TO. `test_rehearse.py` establishes two CLEAN CONTROLS first - two transitions the shipped code gets right, on which the harness must say nothing - and then breaks the shipped migration and the entity-id composition six ways in a private copy of the tree, requiring each to turn its control from green to red WITH THE RIGHT FINDING NAMED. One mutation is required to SURVIVE, with the reason written down. The synthetic state carries maintenance history and a legacy-shaped latched `sign_tripped` with no `schema_version` beside it, because state built by the happy path cannot detect a migration that only mishandles legacy shapes. ⚠️ THE FIRST DRAFT OF THE HARNESS HAD HABIT 4 IN IT and its own battery caught it - MUTATION.md's thirteenth instance, third in an instrument. The entity diff composed both sides with the same build, so renaming the MQTT device moved every id equally and it reported "no orphans" about a change that orphans everything. `--live-entities` is the fix: a reference that does not move with the code. THE RETREAT is supported and does not corrupt state - the flat rollback view keeps the latch and the first pack's history live - but it orphans eight HA entities, and the ROUND TRIP is not supported: a maintenance cycle completed while rolled back is discarded on re-advance. Stated in DOCS.md as a decision. Fetching /data/state.json, the options blob and the entity list are OWNER STEPS and are written down. ⚠️ Nothing here was run against real state and nothing here claims to have been. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/DOCS.md | 106 +++ goodwe_controller/MUTATION.md | 26 + goodwe_controller/test_rehearse.py | 410 +++++++++ goodwe_controller/tools/rehearsal/flat.yaml | 22 + .../rehearsal/live-entities-recorded.txt | 34 + .../tools/rehearsal/roster-inverter-a.yaml | 42 + goodwe_controller/tools/rehearsal/roster.yaml | 42 + .../tools/rehearsal/state-flat-070.json | 11 + .../tools/rehearsal/state-legacy.json | 10 + goodwe_controller/tools/rehearse_roster.py | 840 ++++++++++++++++++ 10 files changed, 1543 insertions(+) create mode 100644 goodwe_controller/test_rehearse.py create mode 100644 goodwe_controller/tools/rehearsal/flat.yaml create mode 100644 goodwe_controller/tools/rehearsal/live-entities-recorded.txt create mode 100644 goodwe_controller/tools/rehearsal/roster-inverter-a.yaml create mode 100644 goodwe_controller/tools/rehearsal/roster.yaml create mode 100644 goodwe_controller/tools/rehearsal/state-flat-070.json create mode 100644 goodwe_controller/tools/rehearsal/state-legacy.json create mode 100644 goodwe_controller/tools/rehearse_roster.py diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index e6e3aa7..6eb01ac 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1574,6 +1574,112 @@ battery power, circulating power, state of charge, maintenance phase and controller status by MQTT discovery. This is observability only — the controller works fine without a broker, and MQTT problems can never affect control. +## Before you commission inverter B: `tools/rehearse_roster.py` (ARCH-02) + +The day the `inverters:` roster is declared, two things change that are +**one-way on a live box and invisible until after the restart that applies +them**: the store migration renames the keys that hold the SAFETY-02 latch and +the maintenance history, and the entity ids Home Assistant composes change +shape. This tool runs the **real `Controller`** through that transition against +a **copy** of the state file and reports the difference. + + docker run --rm -e PYTHONDONTWRITEBYTECODE=1 -v :/w:ro \ + goodwe-addon-checks:1 sh -c 'cd /w/goodwe_controller && \ + python3 tools/rehearse_roster.py \ + --state /w/state-copy.json \ + --before /w/live-options.json \ + --after /w/roster-options.json \ + --live-entities /w/live-entities.txt \ + --reverse --round-trip' + +Exit `0` clean · `1` at least one finding · `2` could not run. ⚠️ **`2` is not a +pass.** + +### The owner steps, because this session could not take them + +⚠️ **The rehearsal is only as good as the state it is pointed at, and nothing in +this repository can fetch that.** SSH to the live box is blocked by policy and +that is correct — `/data/state.json` is the file the running add-on is writing. +So three things have to be exported by hand, onto the machine that runs the +check, **before** the roster is declared: + +1. **The state file.** From the Supervisor host: + + ha addons info local_goodwe_controller # confirm the slug first + docker cp addon_local_goodwe_controller:/data/state.json ./state-copy.json + + or take it out of a full add-on backup, which is the same bytes. ⚠️ **Copy + it, never move it, and never write to the original.** The tool opens the path + it is given exactly once and does all of its writing on a scratch copy — but + the safe habit is to hand it a file the add-on has never heard of. + +2. **The options blob**, as `--before`. It is not reachable through the Home + Assistant REST API and no entity carries it; export it from the Supervisor, + the same way `tools/predeploy_check.py` needs it. Write the roster you intend + to deploy into a second file and pass it as `--after`. + +3. **The entity list**, as `--live-entities` — one entity id per line: + + Developer tools -> Template: + {{ states | map(attribute='entity_id') | select('search','goodwe') | join('\n') }} + + ⚠️ **Without it the tool cannot see a change to the id composition at all**, + because both sides of its own before/after diff are composed by the same + build: rename the MQTT device and every id moves equally on both sides, and + the diff reports "nothing orphaned" about a change that orphans everything. + `tools/rehearsal/live-entities-recorded.txt` is the recorded stand-in that + keeps the check live in CI; the real export is what makes it true of *this* + install. The install carries roughly 49 `goodwe*` entities and this add-on + announces eight of them — the tool separates the ids it owns from the ids it + does not, rather than claiming orphans it cannot cause. + +⚠️ **No result in this repository was produced against real state.** Everything +recorded here and in `test_rehearse.py` was measured on the synthetic fixtures +in `tools/rehearsal/`. + +### What it found on the synthetic fixtures + +⚠️ **A pre-DEV-02 flat store that declares a multi-inverter roster comes up with +its SAFETY-02 latch DISARMED, unless the first roster entry happens to be named +`inverter-a`.** `Controller.__init__` calls `migrate_unit_keys(store, UNIT_A)` +with a hardcoded constant, so the latch lands in `unit/inverter-a/sign_tripped`; +a roster of `["garage", "shed"]` reads `unit/garage/sign_tripped`, which is +absent. And `_grade_store_migration()` — the guard written for exactly this — +sees the migration's own misdirected key satisfy its `namespaced_latched` test +and returns without re-arming. The file says `sign_tripped: true`, the banner is +green, and nothing is held at 0 W. **This is SAFETY-44, and the answer to it is +that the asymmetry is not safe on that path.** It is reported here and pinned by +`test_rehearse.py` part 5; the fix belongs to its own card, shown red first. + +The same end state is reached whether the roster is declared in the same step as +the upgrade or a restart later — in the staged case because the flat start +stamps `schema_version` and the roster start's migration then returns early. + +### The reverse direction: the retreat is supported, the round trip is not + +If commissioning B goes wrong, **backing out to the flat single-inverter config +is supported and does not corrupt state.** Measured on the fixtures: + +* the SAFETY-02 latch and the first pack's maintenance history both survive, + because `store.py` rule 2 keeps the flat spellings **live** rather than merely + present — `maintenance.UnitHistory.mirror_flat` writes them on every change, + so a single-FSM add-on finds a current history rather than one dated the day + the roster was declared; +* **it orphans eight Home Assistant entities** — the four per-unit rows for each + of the two units. Their history stops and any automation naming them stops + firing, silently. That is the price of the retreat and it is not recoverable + by advancing again: HA keeps the id it composed at first discovery. + +⚠️ **What is NOT supported is the round trip.** A maintenance cycle that +completes *while the install is rolled back* is **discarded** when it +re-advances: `maint_schema_version` is already stamped, so `migrate_maint_keys()` +takes its early return and the pack reads the namespaced history from before the +retreat. The pack is then recalibrated on a schedule that does not know it +already was. **So: retreat freely, but treat a retreat as ending the current +maintenance bookkeeping — and if a cycle ran during it, correct +`unit//last_completed` by hand before advancing again.** This is a stated +decision, not a discovery made at the inverter. + ## Before you deploy: `tools/predeploy_check.py` (PKG-09) Two ways a deploy has gone wrong on this system, both of which the Supervisor diff --git a/goodwe_controller/MUTATION.md b/goodwe_controller/MUTATION.md index 9f3c565..fd741ab 100644 --- a/goodwe_controller/MUTATION.md +++ b/goodwe_controller/MUTATION.md @@ -225,6 +225,32 @@ same place twice, the check is decoration. forever, *including after the call site is fixed*. The check was true and passed for the wrong reason. Habit **9** below is what falls out of it. +* ⚠️ **ARCH-02's OWN REHEARSAL HARNESS, CAUGHT BY ITS OWN MUTATION BATTERY — + the THIRTEENTH instance, and the third in an INSTRUMENT rather than in a + check.** `tools/rehearse_roster.py` reports orphaned Home Assistant entities + by composing the ids the add-on announces **before** the roster switch and + **after** it, and diffing. Both sides are composed by the same build. So the + two mutations that change the id composition itself — renaming the MQTT device + (`"GoodWe"` -> `"GoodWe Controller"`), renaming one site sensor (`"Grid power"` + -> `"Grid power W"`) — moved **every id on both sides equally**, and the + harness reported *"no orphans"* about a change that orphans the entire live + entity list. It read exactly like a clean transition. The first draft's + M5/M6 came back `exit 0` and `exit 2`, and neither was the failure they name. + + The fix is habit 2 with a filename: **the reference has to be data that does + not move with the code.** `--live-entities` takes a list of the ids the + install actually has, `tools/rehearsal/live-entities-recorded.txt` is the + recorded stand-in that keeps it live in CI, and a build that announces an id + the list does not carry is now a `COMPOSITION DRIFT` finding rather than a + silence. ⚠️ **The anchor moved for the same reason**: it had been + `compose(mqtt.DEVICE["name"], p1["name"]) == "sensor.goodwe_p1_sample_age"`, + which any renaming of the device satisfies by moving both operands, and it is + now composed from two recorded **literals**. + + Ask of an instrument what habit 4 asks of a check: *where did each side of + this comparison come from* — and if a mutation of the subject moves both + sides, the instrument is decoration. + * **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 new file mode 100644 index 0000000..7114a3c --- /dev/null +++ b/goodwe_controller/test_rehearse.py @@ -0,0 +1,410 @@ +"""Runnable check for the roster rehearsal. `python3 test_rehearse.py` + +ARCH-02. `tools/rehearse_roster.py` is the instrument that decides whether the +flat -> roster switch on 2026-09-12 loses a safety latch, a month of maintenance +history, or a Home Assistant entity that an automation is watching. This file +grades the instrument. + +⚠️ AN INSTRUMENT THAT HAS NEVER BEEN SEEN TO GO RED IS NOT EVIDENCE. This +repository's most-repeated defect, `MUTATION.md` habit 4 and now THIRTEEN +recorded instances, is "a check whose DATA cannot reach the failure it names" - +and a rehearsal that reports "migration OK" because it compared a structure to +itself is the purest example of it there is. So this file does two things in +order, and the first one is the one that makes the second mean anything: + + Part 2 TWO CLEAN CONTROLS. Two (state file, roster) pairs on which the + shipped code is CORRECT and the harness must report NOTHING. A + harness that is red on every input has measured nothing, and every + red below it would be free. + + Part 3 ⭐ SIX DELIBERATE MUTATIONS OF THE SHIPPED MIGRATION AND OF THE + ENTITY-ID COMPOSITION, each applied to a private copy of the tree, + each required to flip its control from green to red, AND each + required to produce the SPECIFIC finding it should - not merely a + non-zero exit. A mutation that goes red for the wrong reason is a + harness that would go red on the real defect for the wrong reason + too. + + Part 4 ONE MUTATION THAT DELIBERATELY SURVIVES, with the reason written + down. A battery in which everything dies is a battery whose author + stopped as soon as the number looked good. + + Part 5 SAFETY-44's exact path, pinned as a finding, by name, in both + directions. + +⚠️ THE MUTATIONS ARE SOURCE EDITS IN A COPIED TREE, NOT MONKEYPATCHES. A +monkeypatch grades the name a module was asked for; an edit grades the code that +runs. Each copy is made with `mkdtemp()`, so two runs of this file cannot +overwrite each other's work - habit 18 - and `__pycache__` is never copied, so +no mutant is ever graded through a stale `.pyc` - habit 1 / TEST-15. +""" + +import json +import os +import re +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +HERE = Path(__file__).resolve().parent +FIX = HERE / "tools" / "rehearsal" + +fails = [] + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +# --------------------------------------------------------------------------- # +print("1. the harness's own preconditions") +# --------------------------------------------------------------------------- # +sys.path.insert(0, str(HERE / "tools")) +import rehearse_roster as RR # noqa: E402 + +check("the tool refuses rather than passing when it cannot run: its three exit " + "codes are distinct and 2 is not 0", + (RR.EXIT_OK, RR.EXIT_FINDING, RR.EXIT_COULD_NOT_RUN) == (0, 1, 2)) + +# ⚠️ THE ANCHOR. `app/mqtt.py` records `sensor.goodwe_p1_sample_age` as VERIFIED +# against the live system on 2026-08-26, and the ESP32's ingest guard subscribes +# to it. The tool's entity-id model is written independently of `mqtt._slug` on +# purpose (habit 4), so this is the one external fact that says the independent +# model is right. If it fails, every orphan claim the tool makes is worthless +# and the tool exits 2 rather than reporting. +try: + RR._check_anchor() + anchored = True +except RR.Refused: + anchored = False +check("the entity-id model reproduces the ONE id verified against the live " + f"system ({RR.ANCHOR_ID})", anchored) + +check("...and it is NOT built out of `mqtt._slug`, so a mutation in the " + "shipped slugifier moves one side of the comparison and not both", + "_slug" not in RR.compose.__code__.co_names) + +_opts = RR.load_options(str(FIX / "roster.yaml")) +check("the built-in YAML reader parses the roster fixture the check image " + "cannot parse with PyYAML (there is no `yaml` module in it)", + [e["name"] for e in _opts["inverters"]] == ["garage", "shed"] + and _opts["max_w"] == 5000 and _opts["batt_invert"] is False + and _opts["meter_source"] == "off") +try: + RR._mini_yaml("inverters: [a, b]\n") + _refused = False +except RR.Refused: + _refused = True +check("...and it REFUSES a construct it does not understand rather than " + "guessing, because a config silently mis-parsed here rehearses a " + "configuration nobody is going to deploy", _refused) + +_flat = RR.load_options(str(FIX / "flat.yaml")) +check("⚠️ the two configs differ in the ONE way the transition is about: the " + "BEFORE roster is empty and the AFTER roster is declared - if they did " + "not, every diff below would be a structure compared to itself", + _flat.get("inverters") == [] and len(_opts["inverters"]) == 2) +check("...and the AFTER roster's first entry is NOT `inverter-a`, which is " + "habit 4's seventh instance in one line: with it, `UNIT_A` and " + "`inverters[0].name` are the same string and a misdirected namespace " + "cannot be seen", + _opts["inverters"][0]["name"] != RR.UNIT_A) + +_legacy = json.loads((FIX / "state-legacy.json").read_text()) +check("⚠️ the synthetic state carries MAINTENANCE HISTORY - state built by the " + "happy path cannot detect a migration that drops it", + _legacy.get("last_completed") and _legacy.get("phase")) +check("⚠️ ...and a LEGACY-SHAPED KEY WHOSE LOSS WOULD BE SILENT: a LATCHED " + "`sign_tripped` with no `schema_version` beside it, i.e. written by a " + "controller that had never heard of unit namespaces", + _legacy.get("sign_tripped") is True + and "schema_version" not in _legacy + and "maint_schema_version" not in _legacy) + + +# --------------------------------------------------------------------------- # +print() +print("2. THE CLEAN CONTROLS: two transitions on which the shipped code is " + "right and the harness must say nothing") +# --------------------------------------------------------------------------- # +# ⚠️ THIS PART IS WHAT MAKES PART 3 EVIDENCE. Six reds prove nothing about an +# instrument that is red on its own fixtures. Each control below is a genuine +# flat -> roster switch that the shipped migration handles correctly, and the +# required answer is EXIT 0. +# +# CONTROL-A a PRE-DEV-02 flat store, latch tripped, onto a roster whose +# first entry IS `inverter-a`. This is the path SAFETY-44's +# documented reasoning describes, and on it that reasoning holds. +# CONTROL-B a store that has already run flat under 0.7.0 (schema stamped, +# no latch), onto a roster named `garage`/`shed`. This is the +# MAINT-05 half, and the names are deliberately not `inverter-a`. +CONTROLS = { + "CONTROL-A": ("state-legacy.json", "roster-inverter-a.yaml"), + "CONTROL-B": ("state-flat-070.json", "roster.yaml"), +} + + +def run(tree: Path, state: str, after: str, *extra) -> "tuple[int, str]": + """The harness, as the owner runs it, over `tree`. + + ⚠️ ALWAYS WITH `--live-entities`. That file is the only part of the + entity-id comparison that does not move with the code, so a run without it + is a run that cannot see a renamed device. + """ + fixtures = tree / "tools" / "rehearsal" + proc = subprocess.run( + [sys.executable, str(tree / "tools" / "rehearse_roster.py"), + "--state", str(fixtures / state), + "--before", str(fixtures / "flat.yaml"), + "--after", str(fixtures / after), + "--live-entities", str(fixtures / "live-entities-recorded.txt"), + *extra], + capture_output=True, text=True, + env={**os.environ, "PYTHONDONTWRITEBYTECODE": "1"}) + return proc.returncode, proc.stdout + proc.stderr + + +for label, (state, after) in CONTROLS.items(): + code, out = run(HERE, state, after) + check(f"{label}: the shipped code, {state} -> {after}, exits 0 with no " + f"findings", code == RR.EXIT_OK and "NO FINDINGS" in out) + +# ⚠️ AND THE INPUT FILE IS NEVER WRITTEN. The owner runs this against a copy of +# a live state file; a tool that edited it would be a tool that had to be +# trusted rather than one that cannot do harm. +_before_bytes = (FIX / "state-legacy.json").read_bytes() +run(HERE, "state-legacy.json", "roster.yaml", "--reverse", "--round-trip") +check("the state file the harness was pointed at is BYTE-IDENTICAL afterwards, " + "including after the reverse and round-trip legs, which write", + (FIX / "state-legacy.json").read_bytes() == _before_bytes) + + +# --------------------------------------------------------------------------- # +print() +print("3. ⭐ THE MUTATIONS: each one breaks the shipped migration or the " + "entity-id composition, and each must turn its control RED") +# --------------------------------------------------------------------------- # +def mutant_tree(edits) -> str: + """A private copy of the add-on with `edits` applied. + + ⚠️ EVERY EDIT IS ASSERTED TO HAVE MATCHED. Four defects in this session came + from `str.replace` calls whose pattern did not match: the call silently + returns the original string, the mutation never happens, and the harness + reports a SURVIVOR on code it never changed. That is a fabricated result. + """ + tmp = tempfile.mkdtemp(prefix="rehearse-mut-") + dest = Path(tmp) / "tree" + ignore = shutil.ignore_patterns("__pycache__", "*.pyc") + (dest / "tools").mkdir(parents=True) + shutil.copytree(HERE / "app", dest / "app", ignore=ignore) + shutil.copytree(FIX, dest / "tools" / "rehearsal", ignore=ignore) + shutil.copyfile(HERE / "tools" / "rehearse_roster.py", + dest / "tools" / "rehearse_roster.py") + for rel, old, new in edits: + path = dest / rel + text = path.read_text(encoding="utf-8") + hits = text.count(old) + if hits != 1: + raise AssertionError( + f"mutation target appears {hits} times in {rel}, expected 1: " + f"{old!r} - the mutation did NOT happen and any result from it " + f"would be fabricated") + path.write_text(text.replace(old, new), encoding="utf-8") + return str(dest) + + +MUTATIONS = [ + # (id, control, what it breaks, edits, the finding it must produce) + ("M1", "CONTROL-B", + "the maintenance migration is handed the hardcoded `UNIT_A` instead of " + "the roster's own first name - MAINT-05's exact shipped regression, which " + "left all 19 suites green", + [("app/main.py", + "store, self.inverters[0].name, [inv.name for inv in self.inverters])", + "store, UNIT_A, [inv.name for inv in self.inverters])")], + "MAINTENANCE HISTORY LOST"), + + ("M2", "CONTROL-B", + "the maintenance namespace is DROPPED: the migration moves no keys at all, " + "so a month of history stays under a flat spelling the new packs do not " + "read", + [("app/store.py", + "_move_flat_keys(store, unit, MAINT_UNIT_KEYS, rep)", + "_move_flat_keys(store, unit, (), rep)")], + "MAINTENANCE HISTORY LOST"), + + ("M3", "CONTROL-A", + "the SAFETY-02 pair loses HALF of itself: `sign_reason` drops out of " + "`LEGACY_UNIT_KEYS`, so the latch migrates without the sentence that says " + "why it tripped", + [("app/store.py", + 'LEGACY_UNIT_KEYS = ("sign_tripped", "sign_reason")', + 'LEGACY_UNIT_KEYS = ("sign_tripped",)')], + "LATCH KEY NOT UNDER A ROSTER NAME"), + + ("M4", "CONTROL-A", + "the READER's key composition changes while the migration's does not - " + "`Inverter.key()` upper-cases the unit - so the latch is written " + "perfectly, read back perfectly by the migration, and looked for under a " + "name that is not there", + [("app/inverter.py", + "return unit_key(self.name, name)", + "return unit_key(self.name.upper(), name)")], + "LATCH CAME UP DISARMED"), + + ("M5", "CONTROL-A", + "the entity-id composition changes on the DEVICE half: the MQTT device is " + "renamed, which is the 'tidy it up' change `app/mqtt.py` spends thirty " + "lines in capitals refusing. ⚠️ THE BEFORE/AFTER DIFF IS BLIND TO THIS - " + "both sides are composed by the mutant, so every id moves equally and " + "nothing looks orphaned. Only the recorded list catches it", + [("app/mqtt.py", '"name": "GoodWe",', '"name": "GoodWe Controller",')], + "COMPOSITION DRIFT"), + + ("M6", "CONTROL-A", + "the entity-id composition changes on the ENTITY half: one site sensor is " + "renamed, which orphans exactly one id and leaves the other seven alone", + [("app/mqtt.py", + '("grid", "goodwe_grid_power", "Grid power", "W", "power", "measurement", None),', + '("grid", "goodwe_grid_power", "Grid power W", "W", "power", "measurement", None),')], + "COMPOSITION DRIFT: this build announces sensor.goodwe_grid_power_w"), +] + +for mid, control, what, edits, must_say in MUTATIONS: + state, after = CONTROLS[control] + tree = mutant_tree(edits) + code, out = run(Path(tree), state, after) + red = code == RR.EXIT_FINDING + named = must_say in out + print() + print(f" --- {mid} ({control}) {what}") + print(f" exit {code} (0=clean 1=finding 2=could-not-run)") + for line in out.splitlines(): + if line.strip().startswith("⚠️ "): + print(f" {line.strip()[:150]}") + check(f"{mid} turns {control} RED", red) + check(f"{mid} says WHY, and it is the right why: {must_say!r}", named) + shutil.rmtree(Path(tree).parent, ignore_errors=True) + + +# --------------------------------------------------------------------------- # +print() +print("4. THE MUTATION THAT SURVIVES, ON PURPOSE, WITH THE REASON WRITTEN DOWN") +# --------------------------------------------------------------------------- # +# ⚠️ A BATTERY IN WHICH EVERYTHING DIES IS A BATTERY SOMEBODY STOPPED WRITING +# AS SOON AS THE NUMBER LOOKED GOOD. `store.unit_key()` is the ONE spelling of +# the namespaced key, read by `Inverter.key()`, by `UnitHistory.key()` and by +# both migrations - that is store.py's stated design, "one spelling, in one +# place". Changing it therefore moves the writer AND every reader together, and +# the store comes out internally consistent under a different name. +# +# The harness is BLIND to it and cannot be otherwise: nothing inside one version +# of this add-on can see that the key shape stopped matching what an OLDER +# version reads. That is a ROLLBACK contract, and grading it needs two versions +# of the code, which is a different instrument (`tools/predeploy_check.py`'s +# territory). Stated here rather than left as an unexplained green. +_tree = mutant_tree([("app/store.py", + 'return f"unit/{unit}/{key}"', + 'return f"ns.{unit}.{key}"')]) +_code, _out = run(Path(_tree), *CONTROLS["CONTROL-A"]) +print(f" renaming the ONE key spelling -> exit {_code}") +check("renaming `unit_key()` survives, because writer and readers move " + "together - a fact about the design, recorded, not a gap being hidden", + _code == RR.EXIT_OK) +check("...and the store really did change shape underneath it, so this is a " + "survivor of a mutation that HAPPENED rather than of one that did not", + "ns.inverter-a.sign_tripped" in _out and "unit/inverter-a/" not in _out) +shutil.rmtree(Path(_tree).parent, ignore_errors=True) + + +# --------------------------------------------------------------------------- # +print() +print("5. SAFETY-44: a pre-DEV-02 flat store meeting a DECLARED ROSTER, which " + "is the path nobody had established either way") +# --------------------------------------------------------------------------- # +# The question on the card: `migrate_unit_keys(store, UNIT_A)` takes a hardcoded +# constant and has no `misdirected` guard, while `migrate_maint_keys(store, +# unit, roster)` takes the roster and does. Is that asymmetry safe? +# +# ⚠️ MEASURED, NOT ARGUED. The two checks below are the answer, and they are +# written as REQUIREMENTS so that a fix to `migrate_unit_keys` turns them red +# and this file has to be updated deliberately rather than drifting. +code44, out44 = run(HERE, "state-legacy.json", "roster.yaml") +print(f" flat store + roster ['garage','shed'] -> exit {code44}") +check("⚠️ SAFETY-44 IS A DEFECT AGAIN: a store that ran pre-DEV-02 flat and " + "declares a multi-inverter roster comes up with its SAFETY-02 latch " + "DISARMED - the file says `sign_tripped: true` and not one inverter is " + "latched", + code44 == RR.EXIT_FINDING and "LATCH CAME UP DISARMED" in out44) +check("...and the harness names the two latch keys SAFETY-44 asks about, by " + "the namespace they were stranded in", + "unit/inverter-a/sign_tripped" in out44 + and "unit/inverter-a/sign_reason" in out44) +check("...and the mechanism is visible in the report: the keys are stranded " + "under `inverter-a`, which is not on the roster, so no machine reads them", + "STRANDED STORE KEY" in out44 and "['garage', 'shed']" in out44) +check("⚠️ AND IT IS NOT THE SAME-STEP PATH ALONE. Running the flat config " + "first and declaring the roster afterwards - the staged upgrade - " + "reaches the same disarmed latch, because the first start stamps " + "`schema_version` and the second start's migration then returns early", + out44.count("LATCH CAME UP DISARMED") >= 2) + +# ⚠️ AND THE CONTRAST, WHICH IS WHAT STOPS THE CHECK ABOVE BEING VACUOUS. The +# SAME state file onto a roster whose first entry IS `inverter-a` comes up +# LATCHED. The two runs differ in one thing - the first roster name - so the +# finding is attributable to it and to nothing else. +code_ok, out_ok = run(HERE, "state-legacy.json", "roster-inverter-a.yaml") +check("the SAME state file onto a roster whose first entry IS `inverter-a` " + "comes up latched and clean - so the finding above is caused by the " + "roster NAME and not by the fixture being tripped", + code_ok == RR.EXIT_OK and "LATCH CAME UP DISARMED" not in out_ok) + + +# --------------------------------------------------------------------------- # +print() +print("6. THE REVERSE DIRECTION: the retreat, and the return from it") +# --------------------------------------------------------------------------- # +code_r, out_r = run(HERE, "state-flat-070.json", "roster.yaml", + "--reverse", "--round-trip") +_orphaned = set(re.findall(r"ORPHANED ENTITY: (sensor\.\S+)", out_r)) +print(f" the retreat orphans: {sorted(_orphaned)}") +check("the RETREAT is rehearsed at all, and it reports the entities it " + "orphans - the eight per-unit rows, which is the cost of backing out", + "THE RETREAT" in out_r and _orphaned == { + f"sensor.goodwe_{u}_{k}" + for u in ("garage", "shed") + for k in ("setpoint", "battery_power", "battery_soc", "sign_latch")}) +check("...and NOT one site entity: the eight ids the install has today survive " + "the retreat, which is what makes the eight above attributable to the " + "per-unit rows and not to a blanket rename", + not any(o in _orphaned for o in + ("sensor.goodwe_grid_power", "sensor.goodwe_setpoint", + "sensor.goodwe_battery_power", "sensor.goodwe_p1_sample_age"))) +check("...and the first pack's maintenance history SURVIVES the retreat, " + "because `UnitHistory.mirror_flat` keeps the flat keys live - so the " + "retreat itself does not lose state", + "MAINTENANCE HISTORY LOST" not in out_r.split("THE ROUND TRIP")[0] + .split("THE RETREAT")[-1]) +check("⚠️ ...but the ROUND TRIP does not: a maintenance cycle completed while " + "the install is rolled back is DISCARDED when it re-advances, because " + "`maint_schema_version` is already stamped and the migration skips", + "RETREAT-WINDOW WORK DISCARDED" in out_r) +check("...and that is a leg whose data can reach the failure it names: the " + "retreat window actually contains a completion, written through the " + "shipped `UnitHistory`, at a timestamp the starting fixture does not have", + RR.RETREAT_WINDOW_COMPLETION in out_r + and RR.RETREAT_WINDOW_COMPLETION + not in (FIX / "state-flat-070.json").read_text()) + + +print() +if fails: + print(f"FAILED {len(fails)}:") + for f in fails: + print(f" - {f}") + sys.exit(1) +print("all checks passed") diff --git a/goodwe_controller/tools/rehearsal/flat.yaml b/goodwe_controller/tools/rehearsal/flat.yaml new file mode 100644 index 0000000..b81b063 --- /dev/null +++ b/goodwe_controller/tools/rehearsal/flat.yaml @@ -0,0 +1,22 @@ +# ARCH-02 fixture: THE SHAPE THE LIVE BOX IS IN TODAY (a flat, single-inverter +# config). ⚠️ ILLUSTRATIVE ENTITY NAMES, NOT THE LIVE ONES - the owner runs the +# rehearsal against a copy of the real options blob, and this file exists so +# that the harness has a synthetic transition it can be graded on. +setpoint_entity: number.goodwe_setpoint +soc_entity: sensor.goodwe_battery_soc_pct +batt_entity: sensor.goodwe_battery_power_w +batt_invert: false +max_w: 5000 +bms_soc_floor: 10 +meter_source: 'off' +meter_entity: sensor.p1_active_power +inverters: [] +gain: 0.6 +slew_w: 0 +deadband_w: 15 +target_grid_w: 0 +step_w: 10 +heartbeat_s: 10 +stale_input_s: 15 +maintenance_discharge_w: 2000 +maintenance_charge_w: 2000 diff --git a/goodwe_controller/tools/rehearsal/live-entities-recorded.txt b/goodwe_controller/tools/rehearsal/live-entities-recorded.txt new file mode 100644 index 0000000..d991ef4 --- /dev/null +++ b/goodwe_controller/tools/rehearsal/live-entities-recorded.txt @@ -0,0 +1,34 @@ +# The entity ids this add-on ANNOUNCES at the shipped roster of one. +# +# ⚠️ THIS FILE IS EXTERNAL DATA AND THAT IS ITS ENTIRE PURPOSE. The before/after +# entity diff inside `rehearse_roster.py` cannot see a change to the id +# COMPOSITION, because both sides of that diff are composed by the same build: +# rename the MQTT device and every id moves on both sides equally, and the diff +# reports "nothing orphaned" about a change that orphans everything. That is +# `MUTATION.md` habit 4 - a check whose data cannot reach the failure it names - +# and a list that does NOT move with the code is the only thing that closes it. +# +# PROVENANCE, stated exactly: +# - recorded 2026-09-08 by running this tool against `release/1.0` at +# `meter_source: firmware`, roster of one. It is what the code announces. +# - `sensor.goodwe_p1_sample_age` is additionally recorded in `app/mqtt.py` as +# VERIFIED AGAINST THE LIVE SYSTEM on 2026-08-26, and the ESP32's ingest +# guard subscribes to that exact string. +# - ⚠️ IT IS NOT A DUMP OF THE LIVE INSTALL'S ENTITY REGISTRY. Live HA carries +# roughly 49 `goodwe*` entities (card ARCH-02, read 2026-09-06); the other +# ~41 belong to the ESPHome firmware, to template sensors and to helpers, +# and this add-on announces none of them. THE OWNER STEP in DOCS.md replaces +# this file with the real export, and the tool then separates the ids it +# owns from the ids it does not rather than claiming orphans it cannot +# cause. +# +# Editing this file is how you say "the ids changed on purpose". Do not edit it +# to make a check pass. +sensor.goodwe_battery_power +sensor.goodwe_battery_soc +sensor.goodwe_circulating_power +sensor.goodwe_controller_status +sensor.goodwe_grid_power +sensor.goodwe_maintenance_phase +sensor.goodwe_p1_sample_age +sensor.goodwe_setpoint diff --git a/goodwe_controller/tools/rehearsal/roster-inverter-a.yaml b/goodwe_controller/tools/rehearsal/roster-inverter-a.yaml new file mode 100644 index 0000000..13dc3e6 --- /dev/null +++ b/goodwe_controller/tools/rehearsal/roster-inverter-a.yaml @@ -0,0 +1,42 @@ +# ARCH-02 fixture: THE SHAPE AFTER INVERTER B IS COMMISSIONED. +# +# ⚠️ THE FIRST ROSTER ENTRY IS **NOT** CALLED `inverter-a`, AND THAT IS THE +# WHOLE FIXTURE. `MUTATION.md` habit 4's seventh instance is MAINT-05 shipping +# `migrate_maint_keys(store, UNIT_A, ...)` under a comment saying it must not: +# every two-pack fixture in the suite named its first entry `inverter-a`, so +# `UNIT_A` and `inverters[0].name` were the same string in every check that +# existed and the defect was invisible. Names that are not `inverter-a` are what +# make a misdirected namespace something a rehearsal can SEE. +# +# `roster-inverter-a.yaml` beside this file is the same roster with the first +# entry named `inverter-a`, so the two can be compared - see DOCS.md. +setpoint_entity: number.goodwe_setpoint +soc_entity: sensor.goodwe_battery_soc_pct +batt_entity: sensor.goodwe_battery_power_w +batt_invert: false +max_w: 5000 +bms_soc_floor: 10 +meter_source: 'off' +meter_entity: sensor.p1_active_power +gain: 0.6 +slew_w: 0 +deadband_w: 15 +target_grid_w: 0 +step_w: 10 +heartbeat_s: 10 +stale_input_s: 15 +maintenance_discharge_w: 2000 +maintenance_charge_w: 2000 +inverters: + - name: inverter-a + setpoint_entity: number.goodwe_setpoint + soc_entity: sensor.goodwe_battery_soc_pct + batt_entity: sensor.goodwe_battery_power_w + max_w: 5000 + bms_soc_floor: 10 + - name: inverter-b + setpoint_entity: number.goodwe_b_setpoint + soc_entity: sensor.goodwe_b_battery_soc_pct + batt_entity: sensor.goodwe_b_battery_power_w + max_w: 4600 + bms_soc_floor: 10 diff --git a/goodwe_controller/tools/rehearsal/roster.yaml b/goodwe_controller/tools/rehearsal/roster.yaml new file mode 100644 index 0000000..0060f78 --- /dev/null +++ b/goodwe_controller/tools/rehearsal/roster.yaml @@ -0,0 +1,42 @@ +# ARCH-02 fixture: THE SHAPE AFTER INVERTER B IS COMMISSIONED. +# +# ⚠️ THE FIRST ROSTER ENTRY IS **NOT** CALLED `inverter-a`, AND THAT IS THE +# WHOLE FIXTURE. `MUTATION.md` habit 4's seventh instance is MAINT-05 shipping +# `migrate_maint_keys(store, UNIT_A, ...)` under a comment saying it must not: +# every two-pack fixture in the suite named its first entry `inverter-a`, so +# `UNIT_A` and `inverters[0].name` were the same string in every check that +# existed and the defect was invisible. Names that are not `inverter-a` are what +# make a misdirected namespace something a rehearsal can SEE. +# +# `roster-inverter-a.yaml` beside this file is the same roster with the first +# entry named `inverter-a`, so the two can be compared - see DOCS.md. +setpoint_entity: number.goodwe_setpoint +soc_entity: sensor.goodwe_battery_soc_pct +batt_entity: sensor.goodwe_battery_power_w +batt_invert: false +max_w: 5000 +bms_soc_floor: 10 +meter_source: 'off' +meter_entity: sensor.p1_active_power +gain: 0.6 +slew_w: 0 +deadband_w: 15 +target_grid_w: 0 +step_w: 10 +heartbeat_s: 10 +stale_input_s: 15 +maintenance_discharge_w: 2000 +maintenance_charge_w: 2000 +inverters: + - name: garage + setpoint_entity: number.goodwe_setpoint + soc_entity: sensor.goodwe_battery_soc_pct + batt_entity: sensor.goodwe_battery_power_w + max_w: 5000 + bms_soc_floor: 10 + - name: shed + setpoint_entity: number.goodwe_b_setpoint + soc_entity: sensor.goodwe_b_battery_soc_pct + batt_entity: sensor.goodwe_b_battery_power_w + max_w: 4600 + bms_soc_floor: 10 diff --git a/goodwe_controller/tools/rehearsal/state-flat-070.json b/goodwe_controller/tools/rehearsal/state-flat-070.json new file mode 100644 index 0000000..8b9069c --- /dev/null +++ b/goodwe_controller/tools/rehearsal/state-flat-070.json @@ -0,0 +1,11 @@ +{ + "_comment": "ARCH-02 SYNTHETIC FIXTURE - a store that has ALREADY run the flat config under 0.7.0+, so DEV-02's `schema_version` is stamped and the SAFETY-02 latch is not tripped. NOT a copy of the live box's /data/state.json. This is the CLEAN CONTROL for the MAINT-05 half of the rehearsal: the flat->roster transition over it must come out with NO findings, which is what makes each deliberate mutation's RED mean something. A harness that is red on every input has not measured anything.", + "schema_version": 1, + "phase": "idle", + "phase_started": "2026-08-07T10:00:00+00:00", + "last_completed": "2026-08-07T18:42:00+00:00", + "last_start_attempt": "2026-08-07T10:00:00+00:00", + "sign_tripped": false, + "sign_reason": "", + "auto": true +} diff --git a/goodwe_controller/tools/rehearsal/state-legacy.json b/goodwe_controller/tools/rehearsal/state-legacy.json new file mode 100644 index 0000000..ba75d1a --- /dev/null +++ b/goodwe_controller/tools/rehearsal/state-legacy.json @@ -0,0 +1,10 @@ +{ + "_comment": "ARCH-02 SYNTHETIC FIXTURE - a PRE-DEV-02 flat state file. NOT a copy of the live box's /data/state.json; this session cannot read that one and did not try. It is built to carry, deliberately, the two shapes that state built by the happy path does not have: a LATCHED SAFETY-02 failsafe under the flat spelling, and a real maintenance history under the flat spelling. Both are LEGACY-SHAPED: there is no schema_version and no maint_schema_version, so this file was last written by a controller that had never heard of unit namespaces. Losing either one is silent.", + "phase": "idle", + "phase_started": "2026-08-07T10:00:00+00:00", + "last_completed": "2026-08-07T18:42:00+00:00", + "last_start_attempt": "2026-08-07T10:00:00+00:00", + "sign_tripped": true, + "sign_reason": "SIGN_DISAGREEMENT: commanded -1800 W against a meter reading -1450 W for 190 s", + "auto": true +} diff --git a/goodwe_controller/tools/rehearse_roster.py b/goodwe_controller/tools/rehearse_roster.py new file mode 100644 index 0000000..5ff9f20 --- /dev/null +++ b/goodwe_controller/tools/rehearse_roster.py @@ -0,0 +1,840 @@ +"""ARCH-02: rehearse the flat -> roster switch against a COPY of real state. + + python3 tools/rehearse_roster.py --state \ + --before --after + +WHAT THIS IS FOR, in one sentence: on the day inverter B is commissioned the +add-on stops reading the flat single-inverter options and starts reading a +declared `inverters:` roster, and two things happen at that restart that are +ONE-WAY and INVISIBLE UNTIL AFTERWARDS - the store migration renames the keys +that hold a safety latch and a month of maintenance history, and the entity ids +Home Assistant composes change shape. + + THE STORE. /data/state.json on the live box was written by a flat-config + controller. A latch that migrates to a name nothing reads is a + SAFETY GUARD THAT COMES UP DISARMED on an install whose state + file says it is protected (store.py's own header, and SAFETY-02: + "a latch that a restart clears is not a latch"). MAINT-05 had to + fix exactly this shape once already. + + THE ENTITIES. Home Assistant keeps the entity_id it composed at FIRST + discovery. An id that stops being announced is not deleted - it + is ORPHANED: history stays behind on the dead id, and every + automation whose trigger names it SILENTLY STOPS FIRING. An + automation with a vanished trigger entity does not error. It + never runs again. + +⚠️ THIS TOOL RUNS THE REAL `Controller`, NOT A MODEL OF IT. `snapshot()` builds +`app.main.Controller` over the real `app.store.Store`, so the migration under +test is the shipped one and the readers that are asked "can you still find your +latch" are the shipped readers. A rehearsal that re-implemented the migration +would be comparing a structure to itself, which is `MUTATION.md` habit 4 and +this repository's most-repeated defect. + +⚠️ IT NEVER WRITES TO THE FILE IT IS GIVEN. `--state` is opened once, copied to +a scratch file made with `mkstemp()`, and every `Store.save()` in the rehearsal +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)". + +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 + 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. +""" + +import argparse +import json +import os +import shutil +import sys +import tempfile +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from app.inverter import UNIT_A, UNIT_STORE_KEYS, Inverter, roster_names # noqa: E402 +from app.main import Controller # noqa: E402 +from app import mqtt # noqa: E402 +from app.p1 import is_enabled # noqa: E402 +from app.store import (DEFAULTS, MAINT_UNIT_KEYS, Store, # noqa: E402 + unit_key) + +EXIT_OK, EXIT_FINDING, EXIT_COULD_NOT_RUN = 0, 1, 2 + + +class Refused(Exception): + """The rehearsal could not be performed. Exit 2, never exit 0.""" + + +# --------------------------------------------------------------------------- # +# 1. THE CONFIGS +# --------------------------------------------------------------------------- # +# The live box's options blob is JSON (`/data/options.json`); the card's command +# line spells the two configs as `.yaml` because that is what an operator edits. +# Both are accepted. ⚠️ THE YAML READER REFUSES WHAT IT DOES NOT UNDERSTAND +# RATHER THAN GUESSING: a config silently mis-parsed here would produce a +# rehearsal of a configuration nobody is going to deploy, which is worse than no +# rehearsal at all. PyYAML is used when it is importable; the add-on's check +# image has no `yaml` module, so the subset below is what actually runs in CI. + +def _scalar(text: str): + raw = text.strip() + if raw.startswith("#"): + raise Refused("comment where a value was expected") + if (raw.startswith('"') and raw.endswith('"') and len(raw) >= 2) or \ + (raw.startswith("'") and raw.endswith("'") and len(raw) >= 2): + return raw[1:-1] + low = raw.lower() + if low in ("true", "yes", "on"): + return True + if low in ("false", "no", "off"): + return False + if low in ("null", "~", ""): + return None + if raw == "[]": + return [] + if raw == "{}": + return {} + try: + return int(raw) + except ValueError: + pass + try: + return float(raw) + except ValueError: + pass + if raw.startswith("[") or raw.startswith("{"): + raise Refused(f"inline collection {raw!r} is not supported by the " + "built-in reader - install PyYAML or use JSON") + return raw + + +def _mini_yaml(text: str) -> dict: + """The subset an add-on options file is written in, and nothing else. + + Block mappings, and lists of block mappings (`inverters:`). Two indent + levels of nesting, which is all `config.yaml`'s option surface has. Every + other construct raises `Refused`. + """ + root: dict = {} + cur_list = None + cur_entry = None + list_indent = None + for lineno, line in enumerate(text.splitlines(), 1): + if not line.strip() or line.lstrip().startswith("#"): + continue + indent = len(line) - len(line.lstrip(" ")) + if "\t" in line[:indent]: + raise Refused(f"line {lineno}: tab in indentation") + body = line.strip() + try: + if body.startswith("- "): + if cur_list is None: + raise Refused(f"line {lineno}: list item outside a key") + if list_indent is None: + list_indent = indent + if indent != list_indent: + raise Refused(f"line {lineno}: ragged list indentation") + cur_entry = {} + cur_list.append(cur_entry) + item = body[2:].strip() + if item: + k, _, v = item.partition(":") + if not _: + raise Refused(f"line {lineno}: bare list scalar") + cur_entry[k.strip()] = _scalar(v) + continue + if ":" not in body: + raise Refused(f"line {lineno}: {body!r} is not `key: value`") + key, _, value = body.partition(":") + key, value = key.strip(), value.strip() + if cur_entry is not None and list_indent is not None \ + and indent > list_indent: + cur_entry[key] = _scalar(value) + continue + cur_list = cur_entry = None + list_indent = None + if value == "": + cur_list = root[key] = [] + continue + root[key] = _scalar(value) + except Refused: + raise + # A key written `foo:` with nothing under it is an EMPTY list here, which is + # exactly what `inverters: []` means to `roster_entries()`. + return root + + +def load_options(path: str) -> dict: + text = Path(path).read_text(encoding="utf-8") + try: + obj = json.loads(text) + except ValueError: + try: + import yaml # noqa: PLC0415 + except ImportError: + obj = _mini_yaml(text) + else: + obj = yaml.safe_load(text) + if not isinstance(obj, dict): + raise Refused(f"{path}: options must be a mapping, got {type(obj).__name__}") + return obj + + +# --------------------------------------------------------------------------- # +# 2. THE STORE, AND THE READ LOG THAT MAKES "NOTHING READS THIS" MEASURABLE +# --------------------------------------------------------------------------- # +class _ReadLog(dict): + """`Store.data`, with every value-read remembered. + + ⚠️ THIS IS THE INSTRUMENT THE WHOLE SAFETY-44 QUESTION TURNS ON. "A key + nothing reads" cannot be established by reasoning about which name the + migration was handed; it is established by asking the SHIPPED READERS to + restore themselves and recording which keys they actually touched. A key in + the file that no reader touched is a key whose value is not information. + + `__contains__` is deliberately NOT recorded: `_move_flat_keys()` tests + `target in data` to decide whether a namespaced value already exists, and + that is a write-side decision, not a machine reading its latch. + """ + + def __init__(self, *a, **kw): + super().__init__(*a, **kw) + self.reads = set() + + def get(self, key, default=None): + self.reads.add(key) + return super().get(key, default) + + def __getitem__(self, key): + self.reads.add(key) + return super().__getitem__(key) + + +class RehearsalStore(Store): + """The SHIPPED `Store`, pointed at a scratch copy of the operator's file. + + ⚠️ THE REAL CLASS, NOT A STAND-IN. `load()`, `save()`, `get_time()` and + `set_time()` are the ones the add-on runs, so the ISO-8601 round trip that + the module docstring says once took the controller down for hours is the + round trip under rehearsal. A `FakeStore` here would rehearse the fake. + """ + + def __init__(self, path: str): + super().__init__(path) + self.data = _ReadLog(self.data) + self.data.reads.clear() + + +def _scratch_copy(state_path: str, tmpdir: str, tag: str) -> str: + """A per-invocation copy. ⚠️ The operator's file is never opened for write.""" + dest = os.path.join(tmpdir, f"state-{tag}.json") + if state_path is None: + Path(dest).write_text("{}", encoding="utf-8") + else: + shutil.copyfile(state_path, dest) + return dest + + +# --------------------------------------------------------------------------- # +# 3. THE ENTITY IDS +# --------------------------------------------------------------------------- # +# ⚠️ THE SLUGIFIER IS DELIBERATELY NOT `mqtt._slug`. Importing the module's own +# spelling would make the model and the subject move together, so a mutation in +# the composition would change both sides equally and this tool would report +# "no orphans" about a rename that orphaned everything - habit 4, written in one +# import. `test_mqtt.py` part 6 makes the same move for the same reason. +import re # noqa: E402 + + +def compose(device_name, entity_name) -> str: + """The entity_id Home Assistant composes, per `app/mqtt.py`'s own note.""" + s = re.sub(r"[^a-z0-9]+", "_", f"{device_name} {entity_name}".lower()) + return "sensor." + s.strip("_") + + +# The ONE id verified against the live system (2026-08-26, recorded in +# `app/mqtt.py`) and subscribed to by the ESP32 ingest guard. If the model above +# cannot reproduce it, the model is wrong and every orphan claim below it is +# worthless - so the tool REFUSES rather than reporting. +ANCHOR_ID = "sensor.goodwe_p1_sample_age" + + +def entity_ids(opts: dict) -> "dict[str, str]": + """`{entity_id: unique_id}` for every row this add-on announces on `opts`. + + ⚠️ `discovery_payloads()` IS THE SHIPPED ANNOUNCER and `roster_names()` is + the shipped naming rule, so the roster shape decision is not restated here. + """ + payloads = mqtt.discovery_payloads( + units=roster_names(opts), + omit=() if is_enabled(opts) else ("p1_age",)) + out = {} + for _topic, cfg in payloads: + out[compose(cfg["device"]["name"], cfg["name"])] = cfg["unique_id"] + return out + + +# The device name and entity name that composed it, recorded as LITERALS. +# ⚠️ LITERALS, NOT `mqtt.DEVICE["name"]`. Anchoring on the shipped dict would +# make this check pass for any renaming of the device - the model and the +# subject would move together and the anchor would be decoration. These two +# strings are what `release/1.0` had on the day the id was verified. +ANCHOR_DEVICE_NAME = "GoodWe" +ANCHOR_ENTITY_NAME = "P1 sample age" + + +def _check_anchor() -> None: + """Is the MODEL right? Nothing about the shipped code is asked here.""" + got = compose(ANCHOR_DEVICE_NAME, ANCHOR_ENTITY_NAME) + if got != ANCHOR_ID: + raise Refused( + f"the entity-id model composes {got!r} where app/mqtt.py records " + f"{ANCHOR_ID!r} as verified against the live system on 2026-08-26. " + "The model is wrong; no orphan claim from it can be believed.") + + +def load_entity_list(path: str) -> set: + """The entity ids that exist on the install, one per line, `#` comments. + + ⚠️ EXTERNAL DATA, AND THE TOOL IS BLIND WITHOUT IT. The before/after diff is + composed twice by the SAME build, so a change to the id composition itself - + a renamed MQTT device, a renamed sensor - moves both sides equally and the + diff reports nothing. `tools/rehearsal/live-entities-recorded.txt` carries + the recorded list and DOCS.md's owner step replaces it with a real export. + """ + out = set() + for line in Path(path).read_text(encoding="utf-8").splitlines(): + line = line.split("#", 1)[0].strip() + if line: + out.add(line) + if not out: + raise Refused(f"{path}: no entity ids in it") + return out + + +# --------------------------------------------------------------------------- # +# 4. ONE CONTROLLER START, OBSERVED +# --------------------------------------------------------------------------- # +class Hass: + """No I/O. `Controller.__init__` performs none; this only has to exist.""" + + async def number(self, entity, invert=False): + return None + + async def limits(self, entity): + return (None, None, None) + + async def set_number(self, entity, value): + return False + + async def call(self, domain, service, data): + return False + + +class Pub: + enabled = False + + def publish(self, *a, **k): + pass + + def close(self): + pass + + +class Snapshot: + """What one `Controller` start over one state file looks like.""" + + def __init__(self, label, opts, store, controller): + self.label = label + self.opts = opts + self.store = store + self.units = [inv.name for inv in controller.inverters] + self.data = dict(store.data) + self.read = set(store.data.reads) + self.entities = entity_ids(opts) + self.latched = {inv.name: bool(inv.sign_wd.tripped) + for inv in controller.inverters} + self.latch_reason = {inv.name: inv.sign_wd.reason + for inv in controller.inverters} + self.flat_tripped = bool(store.data.get("sign_tripped", False)) + self.history = {} + for m in controller.maints: + self.history[m.name] = {k: m.store.get(k) for k in MAINT_UNIT_KEYS} + self.store_migration = controller.store_migration + self.maint_migration = controller.maint_migration + self.machine_read = machine_reads(controller, store) + + @property + def unit_keys(self): + return sorted(k for k in self.data if k.startswith("unit/")) + + +def machine_reads(controller, store) -> set: + """Every store key the CONSTRUCTED machines read, measured not transcribed. + + ⚠️ THE PROBE IS THE SHIPPED CONSTRUCTOR, RUN AGAIN. `Inverter.__init__` is + the code that restores a unit's SAFETY-02 and SAFETY-42 latches, so building + one more `Inverter` per roster entry against the post-migration store + replays EXACTLY the reads a restart performs - including any key this file + has never heard of. Writing out the key names here instead would be a second + transcription of `UNIT_STORE_KEYS`, and the copy that drifts is always the + one that reads (store.py, `unit_key()`). + + The maintenance half iterates `MAINT_UNIT_KEYS`, which is the same tuple + `migrate_maint_keys()` moves and `maintenance.UnitHistory` indexes - so a + key renamed there cannot silently stop being probed here either. + """ + store.data.reads.clear() + for inv in controller.inverters: + Inverter(inv.name, store=store) + for m in controller.maints: + for key in MAINT_UNIT_KEYS: + m.store.get(key) + m.store.get_time(key) + seen = set(store.data.reads) + store.data.reads.clear() + return seen + + +def snapshot(label: str, opts: dict, state_path: str, tmpdir: str) -> Snapshot: + store = RehearsalStore(_scratch_copy(state_path, tmpdir, label)) + controller = Controller(dict(opts), Hass(), store, Pub(), stop=None) + return Snapshot(label, opts, store, controller) + + +def snapshot_over(label: str, opts: dict, prior: Snapshot) -> Snapshot: + """A second start over the store the previous start LEFT BEHIND. + + ⚠️ THE SAME FILE, NOT A SECOND COPY OF THE ORIGINAL. This is the sequence a + real box performs: the add-on restarts against `/data/state.json` as its own + previous run wrote it. + """ + store = RehearsalStore(prior.store.path) + controller = Controller(dict(opts), Hass(), store, Pub(), stop=None) + return Snapshot(label, opts, store, controller) + + +# --------------------------------------------------------------------------- # +# 5. THE DIFF +# --------------------------------------------------------------------------- # +class Finding: + def __init__(self, kind, text): + self.kind = kind + self.text = text + + def __str__(self): + return f"{self.kind}: {self.text}" + + +def diff(before: Snapshot, after: Snapshot) -> list: + """Everything that changed, and which of those changes is a defect.""" + findings = [] + + # -- store keys --------------------------------------------------------- + gone = sorted(set(before.data) - set(after.data)) + for key in gone: + findings.append(Finding( + "VANISHED STORE KEY", + f"{key} = {before.data[key]!r} was in the store before the " + f"transition and is not there afterwards")) + + # ⭐ THE ONE SAFETY-44 IS ABOUT. A key that is present, correct, verified by + # the migration's own read-back - and that no machine on the new roster will + # ever look under. `MigrationReport.misdirected` says the same thing about + # the keys the migration itself wrote; this says it about the WHOLE FILE, + # including keys written by a version that is no longer installed. + for key in sorted(after.data): + if not key.startswith("unit/"): + continue + if key in after.machine_read: + continue + findings.append(Finding( + "STRANDED STORE KEY", + f"{key} = {after.data[key]!r} is in the store and NO machine on " + f"the roster {after.units!r} read it during a full start. Its " + f"value is not information.")) + + # -- the SAFETY-02 latch, by name, because SAFETY-44 asks for it by name -- + for key in ("sign_tripped", "sign_reason"): + findings.extend(_latch_finding(key, before, after)) + + if before.flat_tripped and not any(after.latched.values()): + findings.append(Finding( + "LATCH CAME UP DISARMED", + "the state file carries a LATCHED SAFETY-02 sign-disagreement " + "failsafe under the pre-DEV-02 flat key `sign_tripped`, and NOT " + f"ONE inverter on the roster {after.units!r} came up tripped. " + "SAFETY-02: a latch that a restart clears is not a latch.")) + + # -- maintenance history ------------------------------------------------- + findings.extend(_history_findings(before, after)) + + # -- entity ids ---------------------------------------------------------- + orphans = sorted(set(before.entities) - set(after.entities)) + for eid in orphans: + findings.append(Finding( + "ORPHANED ENTITY", + f"{eid} (unique_id {before.entities[eid]}) is announced before the " + "transition and NOT afterwards. Home Assistant does not delete it: " + "its history stops, and every automation whose trigger names it " + "silently stops firing - without an error.")) + return findings + + +def _latch_finding(key, before, after): + """Did the two SAFETY-44 keys land somewhere a machine reads? + + ⚠️ THE FIRST ROSTER ENTRY, AND ONLY THE FIRST. The flat pair belongs to + whichever inverter the single-inverter install was driving, which is + `inverters[0]` - that is what `migrate_maint_keys()` targets for the history + and what `Controller._grade_store_migration()` re-arms. Asking whether the + flat latch reached inverter B's namespace would report a finding on a + CORRECT migration, and an instrument that is red on the right answer is + worth nothing when it is red on the wrong one. + """ + out = [] + if not after.units or key not in UNIT_STORE_KEYS: + return out + unit = after.units[0] + target = unit_key(unit, key) + was = before.data.get(key) + if target not in after.data and was not in (None, DEFAULTS.get(key)): + out.append(Finding( + "LATCH KEY NOT UNDER A ROSTER NAME", + f"the flat `{key}` = {was!r} did not reach {target}, which is the " + f"key `Inverter({unit!r})` restores from")) + elif target in after.data and target not in after.machine_read: + out.append(Finding( + "LATCH KEY NOT READ BY ANY MACHINE", + f"{target} = {after.data[target]!r} exists but no machine on the " + f"roster {after.units!r} read it during a full start")) + return out + + +def entity_list_findings(recorded: set, before: Snapshot, + after: Snapshot) -> list: + """Grade both snapshots against a list that does NOT move with the code.""" + out = [] + announced_before = set(before.entities) + announced_after = set(after.entities) + for eid in sorted(announced_before - recorded): + out.append(Finding( + "COMPOSITION DRIFT", + f"this build announces {eid} in the BEFORE shape, and that id is " + "not on the recorded list of ids the install has. The id " + "composition has changed under the install: Home Assistant keeps " + "the id it composed at first discovery, so what this actually " + "creates is a NEW entity beside a dead one.")) + owned = recorded & announced_before + for eid in sorted(owned - announced_after): + out.append(Finding( + "ORPHANED ENTITY (against the recorded list)", + f"{eid} exists on the install and is announced in the BEFORE " + "shape, and the AFTER shape does not announce it.")) + return out + + +def report_entity_list(recorded: set, before: Snapshot, after: Snapshot) -> None: + announced_before = set(before.entities) + foreign = sorted(recorded - announced_before) + print() + print(" -- AGAINST THE RECORDED ENTITY LIST ------------------------------") + print(f" {len(recorded)} ids on the list, " + f"{len(recorded & announced_before)} of them announced by this " + f"add-on in the BEFORE shape") + print(f" {len(foreign)} on the list that this add-on announces in " + "NEITHER shape - the roster switch cannot orphan these, they belong " + "to the") + print(" ESPHome firmware, to template sensors or to helpers:") + for eid in foreign[:60]: + print(f" {eid}") + if len(foreign) > 60: + print(f" ... and {len(foreign) - 60} more") + + +def _history_findings(before, after): + """MAINT-05 moved this once. A second move must not drop it.""" + out = [] + if not before.history or not after.history: + return out + was = list(before.history.values())[0] + now = list(after.history.values())[0] + inheritor = list(after.history)[0] + for key in ("phase", "last_completed"): + old, new = was.get(key), now.get(key) + if old in (None, DEFAULTS.get(key)): + continue + if old != new: + out.append(Finding( + "MAINTENANCE HISTORY LOST", + f"`{key}` read {old!r} through the first pack's own history " + f"before the transition and reads {new!r} through " + f"{inheritor!r}'s afterwards")) + return out + + +# --------------------------------------------------------------------------- # +# 6. THE REPORT +# --------------------------------------------------------------------------- # +RETREAT_WINDOW_COMPLETION = "2026-09-20T18:00:00+00:00" + + +def _recalibrate_while_retreated(retreated: Snapshot) -> str: + """A maintenance cycle finishes while the install is rolled back to flat. + + ⚠️ WRITTEN THROUGH THE SHIPPED WRITER. `Maintenance.store` is a + `maintenance.UnitHistory`, which is the one object that decides which key a + pack's history lands under and whether the flat rollback view is mirrored. + A raw `store.set("last_completed", ...)` here would be this file's own + opinion about where a completion goes, and the round trip would then be + grading that opinion instead of the add-on. + """ + store = RehearsalStore(retreated.store.path) + controller = Controller(dict(retreated.opts), Hass(), store, Pub(), stop=None) + controller.maints[0].store.set("last_completed", RETREAT_WINDOW_COMPLETION) + return RETREAT_WINDOW_COMPLETION + + +def _round_trip_findings(completed: str, readvanced: Snapshot) -> list: + """Can the re-advanced roster still see work done during the retreat?""" + out = [] + if not readvanced.history: + return out + inheritor = list(readvanced.history)[0] + seen = readvanced.history[inheritor].get("last_completed") + if seen != completed: + out.append(Finding( + "RETREAT-WINDOW WORK DISCARDED", + f"a maintenance cycle completed at {completed} while the install " + f"was rolled back to the flat config, and after re-advancing to " + f"the roster {inheritor!r} reads `last_completed` = {seen!r}. The " + "pack will be recalibrated again on a schedule that does not know " + "it already was.")) + return out + + +def report_transition(name, before, after, findings) -> None: + print() + print("=" * 78) + print(f" {name}") + print("=" * 78) + print(f" roster before : {before.units}") + print(f" roster after : {after.units}") + print(f" DEV-02 migration : {after.store_migration.explain()}") + print(f" MAINT-05 migration: {after.maint_migration.explain()}") + + print() + print(" -- STORE KEYS ----------------------------------------------------") + keys = sorted(set(before.data) | set(after.data)) + width = max([len(k) for k in keys] + [10]) + print(f" {'key'.ljust(width)} {'before':<26} {'after':<26} status") + for key in keys: + inb, ina = key in before.data, key in after.data + b = repr(before.data[key]) if inb else "-" + a = repr(after.data[key]) if ina else "-" + if inb and not ina: + status = "VANISHED" + elif ina and not inb: + status = "appeared" + elif before.data[key] != after.data[key]: + status = "changed" + else: + status = "" + if key.startswith("unit/") and ina and key not in after.machine_read: + status = (status + " " if status else "") + "STRANDED-nothing-reads-it" + print(f" {key.ljust(width)} {b[:26]:<26} {a[:26]:<26} {status}") + + print() + print(" keys the AFTER roster's machines actually read, measured by " + "replaying") + print(" the shipped constructors against the post-transition store:") + for key in sorted(after.machine_read): + mark = " " if key in after.data else " (absent, defaulted) " + print(f" {key}{mark}") + + print() + print(" -- MAINTENANCE HISTORY (MAINT-05: a second move must not drop it) -") + for label, hist in (("before", before.history), ("after", after.history)): + for unit, vals in hist.items(): + print(f" {label:<7} {unit:<12} " + + " ".join(f"{k}={vals.get(k)!r}" for k in MAINT_UNIT_KEYS)) + + print() + print(" -- SAFETY-02 LATCH (SAFETY-44 asks about these two keys by name) -") + print(f" flat `sign_tripped` in the file : {before.flat_tripped}") + print(f" flat `sign_reason` in the file : " + f"{before.data.get('sign_reason')!r}") + print(f" before: which units came up latched : " + f"{[u for u, v in before.latched.items() if v] or 'none'}") + print(f" after : which units came up latched : " + f"{[u for u, v in after.latched.items() if v] or 'none'}") + for unit in after.units: + target = unit_key(unit, "sign_tripped") + print(f" {target} = {after.data.get(target, '')!r} " + f"(read by a machine: {target in after.machine_read})") + + print() + print(" -- ENTITY IDS ----------------------------------------------------") + allids = sorted(set(before.entities) | set(after.entities)) + for eid in allids: + inb, ina = eid in before.entities, eid in after.entities + mark = "both" if (inb and ina) else ("ORPHANED" if inb else "new") + print(f" {eid:<48} {mark}") + print(f" {len(before.entities)} announced before, " + f"{len(after.entities)} after") + + print() + print(" -- FINDINGS ------------------------------------------------------") + if not findings: + print(" none: nothing vanished, nothing stranded, nothing orphaned, " + "no history lost") + for f in findings: + print(f" ⚠️ {f}") + + +def main(argv=None) -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("--state", required=True, + help="path to a COPY of /data/state.json. Never written to.") + ap.add_argument("--before", required=True, + help="the options in force now (flat single-inverter)") + ap.add_argument("--after", required=True, + help="the options being switched to (declared roster)") + ap.add_argument("--reverse", action="store_true", + help="also rehearse the RETREAT: roster -> flat") + ap.add_argument("--live-entities", default=None, + help="a file of entity ids the install actually has, one " + "per line. ⚠️ WITHOUT IT THE TOOL CANNOT SEE A CHANGE " + "TO THE ID COMPOSITION, because both sides of its own " + "diff are composed by the same build. " + "tools/rehearsal/live-entities-recorded.txt is the " + "recorded default; DOCS.md's owner step replaces it " + "with a real export.") + ap.add_argument("--round-trip", action="store_true", + help="also rehearse RETREAT then RE-ADVANCE, with a " + "maintenance cycle completing while retreated. ⚠️ A " + "retreat is only a retreat if you can come back from " + "it, so the round trip is the leg that decides " + "whether the reverse direction is supported.") + ap.add_argument("--paths", default="both", choices=("staged", "same-step", + "both"), + help="staged: the box runs the BEFORE config once, then " + "restarts on AFTER. same-step: the roster is declared " + "in the SAME step as the upgrade, so the BEFORE " + "controller never runs against this store. " + "Default: both.") + args = ap.parse_args(argv) + + try: + _check_anchor() + before_opts = load_options(args.before) + after_opts = load_options(args.after) + if not Path(args.state).is_file(): + raise Refused(f"{args.state}: not a file") + except Refused as err: + print(f"REFUSED: {err}", file=sys.stderr) + return EXIT_COULD_NOT_RUN + + print(__doc__.split("EXIT CODES")[0].rstrip()) + print() + print(f" state file (copied, never written): {args.state}") + print(f" before config: {args.before}") + print(f" after config: {args.after}") + + findings = [] + recorded = None + try: + if args.live_entities: + recorded = load_entity_list(args.live_entities) + except (OSError, Refused) as err: + print(f"REFUSED: {err}", file=sys.stderr) + return EXIT_COULD_NOT_RUN + tmpdir = tempfile.mkdtemp(prefix="rehearse-roster-") + try: + if args.paths in ("staged", "both"): + b = snapshot("staged-before", before_opts, args.state, tmpdir) + a = snapshot_over("staged-after", after_opts, b) + f = diff(b, a) + if recorded: + f += entity_list_findings(recorded, b, a) + report_transition( + "STAGED: the box runs the flat config, then restarts on the " + "roster", b, a, f) + if recorded: + report_entity_list(recorded, b, a) + print(" (findings from the recorded list are in the list " + "above)") + findings += f + + if args.paths in ("same-step", "both"): + # ⚠️ SAFETY-44's EXACT PATH. A store that ran pre-DEV-02 flat, + # upgraded straight to 0.7.0+, AND declared a multi-inverter roster + # in the same step - so nothing ever ran the flat config under a + # version that knew about namespaces. + b2 = snapshot("samestep-before", before_opts, args.state, tmpdir) + a2 = snapshot("samestep-after", after_opts, args.state, tmpdir) + f2 = diff(b2, a2) + if recorded: + f2 += entity_list_findings(recorded, b2, a2) + report_transition( + "SAME-STEP (SAFETY-44): a pre-DEV-02 flat store meets a " + "declared roster on its FIRST start under this version", + b2, a2, f2) + findings += f2 + + if args.reverse or args.round_trip: + r1 = snapshot("retreat-before", after_opts, args.state, tmpdir) + r2 = snapshot_over("retreat-after", before_opts, r1) + fr = diff(r1, r2) + report_transition( + "THE RETREAT: roster -> flat, which is the back-out if " + "commissioning B goes wrong", r1, r2, fr) + findings += fr + + if args.round_trip: + # ⚠️ THE RETREAT WINDOW HAS TO CONTAIN WORK, OR THIS LEG IS HABIT 4. + # A round trip over a store that nothing changed in between compares + # a structure to itself and reports "clean" whatever the code does. + # So a maintenance cycle COMPLETES while the install is retreated - + # written through `maintenance.UnitHistory`, which is the shipped + # writer, not through a raw `store.set()` - and the question this + # leg asks is whether the pack can still see it after re-advancing. + q1 = snapshot("roundtrip-roster", after_opts, args.state, tmpdir) + q2 = snapshot_over("roundtrip-retreated", before_opts, q1) + worked = _recalibrate_while_retreated(q2) + q3 = snapshot_over("roundtrip-readvanced", after_opts, q2) + fq = diff(q2, q3) + _round_trip_findings(worked, q3) + report_transition( + "THE ROUND TRIP: roster -> flat -> roster, with a maintenance " + f"cycle completing at {worked!r} while retreated", + q2, q3, fq) + findings += fq + except Refused as err: + print(f"REFUSED: {err}", file=sys.stderr) + return EXIT_COULD_NOT_RUN + finally: + shutil.rmtree(tmpdir, ignore_errors=True) + + print() + print("=" * 78) + if findings: + print(f" {len(findings)} FINDING(S) - this transition is not clean") + for f in findings: + print(f" ⚠️ {f}") + print("=" * 78) + return EXIT_FINDING + print(" NO FINDINGS in the rehearsed transitions") + print("=" * 78) + return EXIT_OK + + +if __name__ == "__main__": + sys.exit(main()) -- 2.54.0 From 018da00b748ab9f99b0f7b9246004c5fdda31a01 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Tue, 8 Sep 2026 13:16:47 +0200 Subject: [PATCH 2/7] ARCH-02: show the mutation evidence, not the tool's own docstring The mutation battery printed every line of the harness's output starting with a warning sign, and the harness's header is four paragraphs of them - so each mutation's evidence arrived buried under "THIS TOOL RUNS THE REAL Controller" repeated six times, and each finding appeared once per transition leg plus once in the summary. The filter now matches a finding's KIND and de-duplicates, so what part 3 prints is the set of distinct findings the mutation caused. No behaviour change: the same six mutations go red for the same reasons. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/test_rehearse.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/goodwe_controller/test_rehearse.py b/goodwe_controller/test_rehearse.py index 7114a3c..e919e57 100644 --- a/goodwe_controller/test_rehearse.py +++ b/goodwe_controller/test_rehearse.py @@ -283,9 +283,18 @@ for mid, control, what, edits, must_say in MUTATIONS: print() print(f" --- {mid} ({control}) {what}") print(f" exit {code} (0=clean 1=finding 2=could-not-run)") + # ⚠️ THE FINDINGS, DE-DUPLICATED AND IN ORDER. The tool prints each finding + # once per transition leg and once more in its summary, and its own header + # is full of ⚠️ paragraphs - printing every line that starts with one would + # bury the evidence this part exists to show under the tool's docstring. + seen = [] for line in out.splitlines(): - if line.strip().startswith("⚠️ "): - print(f" {line.strip()[:150]}") + text = line.strip() + if text.startswith("⚠️ ") and re.match(r"⚠️ [A-Z][A-Z -]+[A-Z()]:", text): + if text not in seen: + seen.append(text) + for text in seen: + print(f" {text[:160]}") check(f"{mid} turns {control} RED", red) check(f"{mid} says WHY, and it is the right why: {must_say!r}", named) shutil.rmtree(Path(tree).parent, ignore_errors=True) -- 2.54.0 From cb8f80018061da8dedde2daa2ea75f2e1e2d53e2 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Tue, 8 Sep 2026 13:20:49 +0200 Subject: [PATCH 3/7] ARCH-02: the harness was red on every fresh install, and a third control says so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A store the add-on has never written has no latch and no history. The DEV-02 migration still runs on it - `schema_version` is absent, so `from_version` is 0 - and it writes `sign_tripped = False` and `sign_reason = ""` under `inverter-a` before `build_roster()` exists. On a declared roster nothing reads either of them, so the first draft called both STRANDED and exited 1. Every fresh install that grows a second inverter would have been reported as a failed transition. ⚠️ THAT IS A DEFECT IN THE ALARM, NOT A CASE TO SUPPRESS, and store.py already carries the distinction it needed: `MigrationReport.lost` reports a dropped `sign_tripped=True` and NOT a dropped `sign_tripped=False`, because "absent" and "never tripped" are the same statement - and because an alarm that fires on the harmless case is one an operator learns to dismiss before the real one arrives. The stranded check now makes the same split, on the VALUE and not on the key name: a key stranded at its default is printed under its own heading, visibly, and is not a finding; the same key stranded at a non-default still is. SAFETY-44 turns on `sign_tripped = True`, so it is untouched, and a check pins both halves against each other so the softening cannot widen into a hole. CONTROL-C is the fresh install, and it is a third clean control rather than a line in a comment: the shipped code is right about it and the harness must say nothing. It is the control that found this. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/test_rehearse.py | 25 ++++++++++++++ .../tools/rehearsal/state-fresh.json | 3 ++ goodwe_controller/tools/rehearse_roster.py | 34 ++++++++++++++++--- 3 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 goodwe_controller/tools/rehearsal/state-fresh.json diff --git a/goodwe_controller/test_rehearse.py b/goodwe_controller/test_rehearse.py index e919e57..1367519 100644 --- a/goodwe_controller/test_rehearse.py +++ b/goodwe_controller/test_rehearse.py @@ -142,9 +142,18 @@ print("2. THE CLEAN CONTROLS: two transitions on which the shipped code is " # CONTROL-B a store that has already run flat under 0.7.0 (schema stamped, # no latch), onto a roster named `garage`/`shed`. This is the # MAINT-05 half, and the names are deliberately not `inverter-a`. +# CONTROL-C a FRESH INSTALL - an empty state file - onto `garage`/`shed`. +# ⚠️ THIS ONE FOUND A DEFECT IN THE HARNESS. A fresh store's +# migration writes `sign_tripped = False` and `sign_reason = ""` +# under `inverter-a` before any roster exists, and the first draft +# reported both as STRANDED - red on every fresh install. Losing a +# DEFAULT loses nothing; that is `MigrationReport.lost`'s own +# distinction, and an alarm that fires on the harmless case is one +# an operator learns to dismiss before the real one arrives. CONTROLS = { "CONTROL-A": ("state-legacy.json", "roster-inverter-a.yaml"), "CONTROL-B": ("state-flat-070.json", "roster.yaml"), + "CONTROL-C": ("state-fresh.json", "roster.yaml"), } @@ -379,6 +388,22 @@ print("6. THE REVERSE DIRECTION: the retreat, and the return from it") # --------------------------------------------------------------------------- # code_r, out_r = run(HERE, "state-flat-070.json", "roster.yaml", "--reverse", "--round-trip") +# ⚠️ AND THE SOFTENING IS NOT A HOLE. The SAME key stranded with a NON-default +# value is still a finding - that is the whole of part 5 - so the two cases are +# separated by the VALUE and not by the key name. +_fresh_code, _fresh_out = run(HERE, "state-fresh.json", "roster.yaml") +check("a fresh install DOES strand `unit/inverter-a/sign_tripped` - the key is " + "written and nothing on the roster reads it - and the report SAYS SO", + _fresh_code == RR.EXIT_OK + and "unit/inverter-a/sign_tripped = False" in _fresh_out) +check("...and does NOT call it a finding, because the value is the default; " + "the same key at True is the finding part 5 turns on, so the two cases " + "are separated by the VALUE and not by the key name", + "STRANDED STORE KEY: unit/inverter-a/sign_tripped" not in _fresh_out + and "STRANDED STORE KEY: unit/inverter-a/sign_tripped = True" + in "\n".join(l.strip() for l in + run(HERE, "state-legacy.json", "roster.yaml")[1].splitlines())) + _orphaned = set(re.findall(r"ORPHANED ENTITY: (sensor\.\S+)", out_r)) print(f" the retreat orphans: {sorted(_orphaned)}") check("the RETREAT is rehearsed at all, and it reports the entities it " diff --git a/goodwe_controller/tools/rehearsal/state-fresh.json b/goodwe_controller/tools/rehearsal/state-fresh.json new file mode 100644 index 0000000..b6884d1 --- /dev/null +++ b/goodwe_controller/tools/rehearsal/state-fresh.json @@ -0,0 +1,3 @@ +{ + "_comment": "ARCH-02 SYNTHETIC FIXTURE - a FRESH INSTALL. No maintenance history, no latch, no schema markers: what /data/state.json looks like before the add-on has ever completed a cycle. It is a clean control, and it earned its place: the first draft of the harness reported this transition RED, because the DEV-02 migration writes both SAFETY-02 defaults under `inverter-a` before a roster exists and the tool called them stranded. Losing a default loses nothing." +} diff --git a/goodwe_controller/tools/rehearse_roster.py b/goodwe_controller/tools/rehearse_roster.py index 5ff9f20..a86fdf2 100644 --- a/goodwe_controller/tools/rehearse_roster.py +++ b/goodwe_controller/tools/rehearse_roster.py @@ -371,6 +371,9 @@ class Snapshot: self.store_migration = controller.store_migration self.maint_migration = controller.maint_migration self.machine_read = machine_reads(controller, store) + # Keys no machine reads whose value is the default `Store.load()` would + # have supplied. Reported, never a finding: see `diff()`. + self.stranded_at_default = [] @property def unit_keys(self): @@ -451,14 +454,27 @@ def diff(before: Snapshot, after: Snapshot) -> list: # ever look under. `MigrationReport.misdirected` says the same thing about # the keys the migration itself wrote; this says it about the WHOLE FILE, # including keys written by a version that is no longer installed. + # + # ⚠️ ONLY WHERE THE VALUE IS NOT THE DEFAULT, WHICH IS `MigrationReport`'s + # OWN DISTINCTION AND NOT A SOFTENING OF THIS ONE. A stranded + # `sign_tripped = False` has lost the DEFAULT - the same statement "absent" + # already makes - so nothing is gone; a stranded `sign_tripped = True` has + # lost A LATCH. Reporting the first would make this tool red on a FRESH + # INSTALL, whose migration writes both defaults under `inverter-a` before + # any roster exists, and an alarm that fires on the harmless case is an + # alarm an operator learns to dismiss before the real one arrives - which is + # the argument `MigrationReport.lost` already carries in store.py. for key in sorted(after.data): - if not key.startswith("unit/"): + if not key.startswith("unit/") or key in after.machine_read: continue - if key in after.machine_read: + value = after.data[key] + bare = key.rsplit("/", 1)[-1] + if value in (None, DEFAULTS.get(bare)): + after.stranded_at_default.append(key) continue findings.append(Finding( "STRANDED STORE KEY", - f"{key} = {after.data[key]!r} is in the store and NO machine on " + f"{key} = {value!r} is in the store and NO machine on " f"the roster {after.units!r} read it during a full start. Its " f"value is not information.")) @@ -511,7 +527,8 @@ def _latch_finding(key, before, after): "LATCH KEY NOT UNDER A ROSTER NAME", f"the flat `{key}` = {was!r} did not reach {target}, which is the " f"key `Inverter({unit!r})` restores from")) - elif target in after.data and target not in after.machine_read: + elif (target in after.data and target not in after.machine_read + and after.data[target] not in (None, DEFAULTS.get(key))): out.append(Finding( "LATCH KEY NOT READ BY ANY MACHINE", f"{target} = {after.data[target]!r} exists but no machine on the " @@ -660,6 +677,15 @@ def report_transition(name, before, after, findings) -> None: mark = " " if key in after.data else " (absent, defaulted) " print(f" {key}{mark}") + if after.stranded_at_default: + print() + print(" keys no machine reads whose value IS the default, so " + "nothing is lost by") + print(" stranding them - reported rather than hidden, not counted " + "as findings:") + for key in after.stranded_at_default: + print(f" {key} = {after.data[key]!r}") + print() print(" -- MAINTENANCE HISTORY (MAINT-05: a second move must not drop it) -") for label, hist in (("before", before.history), ("after", after.history)): -- 2.54.0 From 1841416aa4e5d7fd9444b64b8373198ec180b0ad Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Tue, 8 Sep 2026 13:23:03 +0200 Subject: [PATCH 4/7] ARCH-02: put the fresh-install checks in the part they belong to They were printing under "6. THE REVERSE DIRECTION", which is not what they are about, and part 2's heading still said two controls when there are three. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/test_rehearse.py | 37 +++++++++++++++--------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/goodwe_controller/test_rehearse.py b/goodwe_controller/test_rehearse.py index 1367519..3b3c54d 100644 --- a/goodwe_controller/test_rehearse.py +++ b/goodwe_controller/test_rehearse.py @@ -128,13 +128,14 @@ check("⚠️ ...and a LEGACY-SHAPED KEY WHOSE LOSS WOULD BE SILENT: a LATCHED " # --------------------------------------------------------------------------- # print() -print("2. THE CLEAN CONTROLS: two transitions on which the shipped code is " +print("2. THE CLEAN CONTROLS: three transitions on which the shipped code is " "right and the harness must say nothing") # --------------------------------------------------------------------------- # # ⚠️ THIS PART IS WHAT MAKES PART 3 EVIDENCE. Six reds prove nothing about an # instrument that is red on its own fixtures. Each control below is a genuine # flat -> roster switch that the shipped migration handles correctly, and the -# required answer is EXIT 0. +# required answer is EXIT 0. CONTROL-C earned its place by failing: it found the +# harness reporting a defect on every fresh install. # # CONTROL-A a PRE-DEV-02 flat store, latch tripped, onto a roster whose # first entry IS `inverter-a`. This is the path SAFETY-44's @@ -182,6 +183,22 @@ for label, (state, after) in CONTROLS.items(): check(f"{label}: the shipped code, {state} -> {after}, exits 0 with no " f"findings", code == RR.EXIT_OK and "NO FINDINGS" in out) +# ⚠️ AND THE SOFTENING IS NOT A HOLE. The SAME key stranded with a NON-default +# value is still a finding - that is the whole of part 5 - so the two cases are +# separated by the VALUE and not by the key name. +_fresh_code, _fresh_out = run(HERE, "state-fresh.json", "roster.yaml") +check("a fresh install DOES strand `unit/inverter-a/sign_tripped` - the key is " + "written and nothing on the roster reads it - and the report SAYS SO", + _fresh_code == RR.EXIT_OK + and "unit/inverter-a/sign_tripped = False" in _fresh_out) +check("...and does NOT call it a finding, because the value is the default; " + "the same key at True is the finding part 5 turns on, so the two cases " + "are separated by the VALUE and not by the key name", + "STRANDED STORE KEY: unit/inverter-a/sign_tripped" not in _fresh_out + and "STRANDED STORE KEY: unit/inverter-a/sign_tripped = True" + in "\n".join(l.strip() for l in + run(HERE, "state-legacy.json", "roster.yaml")[1].splitlines())) + # ⚠️ AND THE INPUT FILE IS NEVER WRITTEN. The owner runs this against a copy of # a live state file; a tool that edited it would be a tool that had to be # trusted rather than one that cannot do harm. @@ -388,22 +405,6 @@ print("6. THE REVERSE DIRECTION: the retreat, and the return from it") # --------------------------------------------------------------------------- # code_r, out_r = run(HERE, "state-flat-070.json", "roster.yaml", "--reverse", "--round-trip") -# ⚠️ AND THE SOFTENING IS NOT A HOLE. The SAME key stranded with a NON-default -# value is still a finding - that is the whole of part 5 - so the two cases are -# separated by the VALUE and not by the key name. -_fresh_code, _fresh_out = run(HERE, "state-fresh.json", "roster.yaml") -check("a fresh install DOES strand `unit/inverter-a/sign_tripped` - the key is " - "written and nothing on the roster reads it - and the report SAYS SO", - _fresh_code == RR.EXIT_OK - and "unit/inverter-a/sign_tripped = False" in _fresh_out) -check("...and does NOT call it a finding, because the value is the default; " - "the same key at True is the finding part 5 turns on, so the two cases " - "are separated by the VALUE and not by the key name", - "STRANDED STORE KEY: unit/inverter-a/sign_tripped" not in _fresh_out - and "STRANDED STORE KEY: unit/inverter-a/sign_tripped = True" - in "\n".join(l.strip() for l in - run(HERE, "state-legacy.json", "roster.yaml")[1].splitlines())) - _orphaned = set(re.findall(r"ORPHANED ENTITY: (sensor\.\S+)", out_r)) print(f" the retreat orphans: {sorted(_orphaned)}") check("the RETREAT is rehearsed at all, and it reports the entities it " -- 2.54.0 From 638c213944296d8daec7f8fbbd88df154efd7191 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Tue, 8 Sep 2026 14:21:34 +0200 Subject: [PATCH 5/7] ARCH-02 round 2: the pin could not tell one red leg from two, and the tool went quiet when a flag was forgotten MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both blocking findings were reproduced before anything was changed, and both are the class this card exists to guard against. B1. THE PIN WAS A COUNT, AND A COUNT CANNOT SEPARATE TWO ROUTES. The rehearsal reaches the disarmed latch by two independent paths and prints every finding TWICE per red leg - once in the leg's block, once in the closing summary - so ONE red leg satisfied `count(...) >= 2`. Measured: shipped code 4; with `migrate_unit_keys()` handed the roster's own name (SAME-STEP fixed) 2, PASSES; with a guard-side fix (both fixed) 0. The check named "NOT THE SAME-STEP PATH ALONE" reported ok on the build whose only remaining failure IS the staged path. The tool now prints `LEG ` / `END LEG ` and the pin requires the finding in EACH leg by name. ⚠️ AND THE FIRST SPLITTER CARRIED THE SAME DEFECT ONE LEVEL DOWN - it opened on the marker and never closed, so the last leg absorbed the closing summary and read red whenever ANY leg was red. Caught by building the one fixture that separates them. Part 5b is now that fixture, run every time: a tree in which SAME-STEP is genuinely clean and STAGED is not. A per-thing check that has never seen the things disagree is a total with extra steps. ⚠️ AND IT CORRECTS A CLAIM THIS CARD MADE. `main.py:250` is NOT the whole fix. Measured on that exact change: LEG SAME-STEP clean, LEG STAGED still DISARMED - the staged path stamps `schema_version` on the flat start and the roster start's migration returns early before the name it is handed matters. DOCS.md now says so, with the measurement. B2. A FORGOTTEN FLAG WAS A CLEAN BILL OF HEALTH. Omitting `--live-entities` printed "nothing orphaned" and exited 0, with `grep -ci` for the words returning 0 - while an UNREADABLE file exited 2. The asymmetry was backwards. Omitting it now refuses (exit 2); `--no-live-entities` is the deliberate opt-out and carries a standing banner in the header AND on the exit line, because a report is skimmed from the bottom. N1. THE REFERENCE DID NOT CARRY WHAT HA KEYS ON. It held entity_ids only, so renaming `mqtt.BASE` (every unique_id moves) and changing `DEVICE["identifiers"]` (HA sees a new device) both came back exit 0 - real orphanings with every composed id byte-identical. The reference is now three columns (`entity_id unique_id device`), is loaded on EVERY run including a degraded one, and M7/M8 pin both. `object_id` is deliberately not a column and its rename is required to SURVIVE, which is what bounds the four mqtt.py mutations. Also found while fixing the above, and fixed with its own paired positive: the VANISHED check did not carry the default/non-default rule the STRANDED check does, so a pair of migrated DEFAULTS moving namespace read as two findings and CONTROL-C went red on a CORRECT build. M9 drops non-default keys and must still be a finding. N2/N3. DOCS.md withdraws the unmeasured "not recoverable by advancing again" - its own reasoning cut the other way and nothing here tested a re-advance; the measured part (eight rows stop updating, history gaps, automations stop firing) stands. And the byte-identical-input property is `test_rehearse.py`'s assertion, not the tool's. FRAMING. SAFETY-44 is ALREADY RECORDED on the merge base - `test_maintenance.py` section 13, with the "guard is defeated by the very key the defect wrote" sentence, in a file this card does not touch. DOCS.md now says this is independent re-confirmation through a second instrument, reachable from an owner-supplied state file rather than only from a fixture, and not a discovery. app/ is still untouched. 59 checks in test_rehearse.py, 9 mutations red, 2 required survivors, 3 clean controls. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/DOCS.md | 127 ++++-- goodwe_controller/MUTATION.md | 32 ++ goodwe_controller/test_rehearse.py | 372 ++++++++++++++--- .../tools/rehearsal/announced-reference.txt | 48 +++ .../rehearsal/live-entities-recorded.txt | 51 +-- goodwe_controller/tools/rehearse_roster.py | 391 ++++++++++++++---- 6 files changed, 831 insertions(+), 190 deletions(-) create mode 100644 goodwe_controller/tools/rehearsal/announced-reference.txt diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 6eb01ac..a97875a 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -1623,37 +1623,99 @@ check, **before** the roster is declared: Developer tools -> Template: {{ states | map(attribute='entity_id') | select('search','goodwe') | join('\n') }} - ⚠️ **Without it the tool cannot see a change to the id composition at all**, - because both sides of its own before/after diff are composed by the same - build: rename the MQTT device and every id moves equally on both sides, and - the diff reports "nothing orphaned" about a change that orphans everything. - `tools/rehearsal/live-entities-recorded.txt` is the recorded stand-in that - keeps the check live in CI; the real export is what makes it true of *this* - install. The install carries roughly 49 `goodwe*` entities and this add-on - announces eight of them — the tool separates the ids it owns from the ids it - does not, rather than claiming orphans it cannot cause. + ⚠️ **Omitting it is a REFUSAL (exit 2), not a quieter run.** The orphan check + is graded against the ids the install actually has, and without that file it + is not graded against reality at all — so the tool will not run rather than + hand back a clean bill of health for a half of the transition it did not + check. If you genuinely have to run without it, pass `--no-live-entities`: + the run then carries a standing banner in the report header **and** on the + exit line saying the orphan check was not graded. The asymmetry used to be + the other way round — an unreadable file refused, a missing flag passed with + exit 0 and no disclosure at all — and the owner runs this once, at + commissioning, under time pressure. + + The install carries roughly 49 `goodwe*` entities and this add-on announces + eight of them. The tool separates the ids it owns from the ids it does not, + rather than claiming orphans it cannot cause. + +### The reference, which is loaded on every run + +`--reference` defaults to `tools/rehearsal/announced-reference.txt` and is read +**whether or not** you pass an entity list. It records what the build is +supposed to announce, three columns per row: + + + +⚠️ **All three, and the second and third are the ones an entity_id-only list +cannot see.** The tool's own before/after diff is composed twice by the same +build, so a change to the composition moves both sides equally and the diff +reports nothing. Worse, two changes orphan an install *without moving a single +composed entity_id*: renaming `mqtt.BASE` (every `unique_id` changes) and +changing `DEVICE["identifiers"]` (HA sees a new device). Home Assistant keys its +entity registry on `unique_id` and its device registry on the identifier, so +either one creates a **new** entity beside the old — and because discovery is +published `retain=True` with no removal payload, the old config stays on the +broker and the old entity stays in the registry, still named by every automation +that referenced it, never updating again. Both were measured coming back +**exit 0** before the reference carried three columns. + +`object_id` is deliberately **not** a column: `app/mqtt.py` records that HA +2026.8 does not honour it in a discovery payload, so renaming one is a true +no-op, and `test_rehearse.py` requires that mutation to **survive** — otherwise +"the reference notices any edit to `mqtt.py`" would be indistinguishable from +"the reference notices what HA keys on". ⚠️ **No result in this repository was produced against real state.** Everything recorded here and in `test_rehearse.py` was measured on the synthetic fixtures in `tools/rehearsal/`. -### What it found on the synthetic fixtures +### What it confirms on the synthetic fixtures -⚠️ **A pre-DEV-02 flat store that declares a multi-inverter roster comes up with -its SAFETY-02 latch DISARMED, unless the first roster entry happens to be named -`inverter-a`.** `Controller.__init__` calls `migrate_unit_keys(store, UNIT_A)` -with a hardcoded constant, so the latch lands in `unit/inverter-a/sign_tripped`; -a roster of `["garage", "shed"]` reads `unit/garage/sign_tripped`, which is -absent. And `_grade_store_migration()` — the guard written for exactly this — -sees the migration's own misdirected key satisfy its `namespaced_latched` test -and returns without re-arming. The file says `sign_tripped: true`, the banner is -green, and nothing is held at 0 W. **This is SAFETY-44, and the answer to it is -that the asymmetry is not safe on that path.** It is reported here and pinned by -`test_rehearse.py` part 5; the fix belongs to its own card, shown red first. +⚠️ **This is a SECOND INSTRUMENT reaching a conclusion this repository already +holds, not a discovery.** `test_maintenance.py` section 13 — *"RECORDED, NOT +FIXED: SAFETY-02's migration has the same wrong-namespace hole, and it is a +LATCH"* — already states it, already names `namespaced_latched`, and already +carries the sentence *"the guard is defeated by the very key the defect wrote"*. +That file is on `release/1.0` and this card does not touch it. What is new here +is that the finding is now reachable **from a state file the owner supplies** +rather than only from a fixture built in a test, and that it is reached by a +different route: a full `Controller` start diffed against its predecessor, +rather than assertions over a `MemStore`. Two instruments, one answer. -The same end state is reached whether the roster is declared in the same step as -the upgrade or a restart later — in the staged case because the flat start -stamps `schema_version` and the roster start's migration then returns early. +**The finding.** A pre-DEV-02 flat store that declares a multi-inverter roster +comes up with its SAFETY-02 latch **DISARMED**, unless the first roster entry +happens to be named `inverter-a`. `Controller.__init__` calls +`migrate_unit_keys(store, UNIT_A)` with a hardcoded constant, so the latch lands +in `unit/inverter-a/sign_tripped`; a roster of `["garage", "shed"]` reads +`unit/garage/sign_tripped`, which is absent. And `_grade_store_migration()` — +the guard written for exactly this — sees the migration's own misdirected key +satisfy its `namespaced_latched` test and returns without re-arming. The file +says `sign_tripped: true`, the banner is green, and nothing is held at 0 W. + +**Both routes reach it, and they are NOT the same defect.** The rehearsal +reports them as separate legs because they need separate fixes: + +| leg | how it gets there | +|---|---| +| `SAME-STEP` | the roster is declared in the same step as the upgrade. The migration runs, and is handed `UNIT_A`. | +| `STAGED` | the flat config runs once under 0.7.0+ first. That start stamps `schema_version = 1`, so the roster start's migration takes its `from_version >= SCHEMA_VERSION` early return and **the latch never moves at all**. | + +⚠️ **So "hand `migrate_unit_keys()` the roster's own first name" is NOT the +whole fix, and the rehearsal measures that rather than asserting it.** +`test_rehearse.py` part 5b builds exactly that change and reports: + + LEG STAGED: DISARMED + LEG SAME-STEP: clean + +The staged path never reaches the line that was changed. A fix that stops there +closes the half an operator is least likely to take and leaves the half that +"upgrade first, declare the roster later" produces. The guard side — +`namespaced_latched` counting a namespace that is not on the roster — is what +covers both, and a change there turns every SAFETY-44 check in part 5 red, which +is what a fix should do to a pin that specifies a defect. + +The fix belongs to its own card, shown red first. Part 5 pins the current +behaviour as a requirement so it cannot drift green by accident. ### The reverse direction: the retreat is supported, the round trip is not @@ -1666,9 +1728,20 @@ is supported and does not corrupt state.** Measured on the fixtures: so a single-FSM add-on finds a current history rather than one dated the day the roster was declared; * **it orphans eight Home Assistant entities** — the four per-unit rows for each - of the two units. Their history stops and any automation naming them stops - firing, silently. That is the price of the retreat and it is not recoverable - by advancing again: HA keeps the id it composed at first discovery. + of the two units. **Measured:** the add-on stops announcing them and stops + publishing their state, so their history gaps and any automation whose + trigger names them stops firing, silently and without an error. + + ⚠️ **What is NOT measured, and was previously overstated here:** whether + advancing again restores them. An earlier draft of this section said the + orphaning was "not recoverable by advancing again: HA keeps the id it composed + at first discovery" — that reasoning cuts the other way, since keeping the + composed id is precisely what would let a re-advertised entity land back on + it, and discovery is published `retain=True` with no removal payload. Nothing + in this repository has tested a re-advance against a real Home Assistant, so + the claim is withdrawn rather than restated in the other direction. Plan the + retreat around the measured part: **the eight rows stop updating for as long + as you are retreated.** ⚠️ **What is NOT supported is the round trip.** A maintenance cycle that completes *while the install is rolled back* is **discarded** when it diff --git a/goodwe_controller/MUTATION.md b/goodwe_controller/MUTATION.md index fd741ab..ad2a48c 100644 --- a/goodwe_controller/MUTATION.md +++ b/goodwe_controller/MUTATION.md @@ -251,6 +251,38 @@ same place twice, the check is decoration. this comparison come from* — and if a mutation of the subject moves both sides, the instrument is decoration. + ⚠️ **AND REVIEW FOUND TWO MORE IN THE SAME CARD, ONE OF THEM INSIDE THE FIX + FOR THE OTHER — the FOURTEENTH and FIFTEENTH.** Both are the pin, not the + tool, and both are the same sentence in a new costume: *a check whose data + cannot reach the failure it names.* + + * **Fourteenth — a COUNT cannot tell one red leg from two.** The rehearsal + reports two independent routes to the disarmed latch (`SAME-STEP` and + `STAGED`) and prints every finding **twice per red leg**: once in that + leg's own block and once in the closing summary. The check named *"NOT THE + SAME-STEP PATH ALONE"* was written `out.count("LATCH CAME UP DISARMED") + >= 2`, so **one** red leg always satisfied it. Measured on three builds: + shipped code (both legs red) → 4; `migrate_unit_keys()` handed the roster + name (SAME-STEP fixed) → 2, **passes**; a guard-side fix (both fixed) → 0. + The check reported `ok` on the build whose only remaining failure *is* the + path it says is not the only one. The fix is to parse the legs and require + the finding in **each**, which is habit 2 again: grade the thing, not a + total over the thing. + + * **Fifteenth — and it is INSIDE THAT FIX, which is the third time this file + has had to write that sentence.** The first leg-splitter opened on a + `LEG ` marker and never closed, so every line after the *last* marker + was attributed to that leg — including the closing summary, which repeats + every finding from every leg. The last leg therefore read red whenever + **any** leg was red, and the new per-leg check was the old count with extra + steps. It was caught by building the one fixture that separates them: a + tree in which `SAME-STEP` is genuinely clean and `STAGED` is not. The tool + now prints `END LEG ` and the splitter closes on it. + + The rule that falls out, and it is the cheap one: **a per-thing check that + has never seen the things DISAGREE is a total.** Part 5b exists to make + them disagree on every run. + * **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 3b3c54d..e68d0c6 100644 --- a/goodwe_controller/test_rehearse.py +++ b/goodwe_controller/test_rehearse.py @@ -6,7 +6,7 @@ history, or a Home Assistant entity that an automation is watching. This file grades the instrument. ⚠️ AN INSTRUMENT THAT HAS NEVER BEEN SEEN TO GO RED IS NOT EVIDENCE. This -repository's most-repeated defect, `MUTATION.md` habit 4 and now THIRTEEN +repository's most-repeated defect, `MUTATION.md` habit 4 and now FIFTEEN recorded instances, is "a check whose DATA cannot reach the failure it names" - and a rehearsal that reports "migration OK" because it compared a structure to itself is the purest example of it there is. So this file does two things in @@ -25,9 +25,18 @@ order, and the first one is the one that makes the second mean anything: harness that would go red on the real defect for the wrong reason too. - Part 4 ONE MUTATION THAT DELIBERATELY SURVIVES, with the reason written + Part 4 TWO MUTATIONS THAT DELIBERATELY SURVIVE, with the reason written down. A battery in which everything dies is a battery whose author - stopped as soon as the number looked good. + stopped as soon as the number looked good - and the `object_id` one + is what BOUNDS the four `app/mqtt.py` mutations above it: without a + change to that file that must NOT be caught, "the reference notices + any edit to mqtt.py" and "the reference notices what HA keys on" are + the same green. + + Part 5b THE PER-LEG CHECK'S OWN POSITIVE CONTROL: a build in which exactly + ONE of the two legs is clean. ⚠️ A per-thing check that has never + seen the things DISAGREE is a total with extra steps, and this file + shipped that defect twice - see `legs()`. Part 5 SAFETY-44's exact path, pinned as a finding, by name, in both directions. @@ -172,48 +181,13 @@ def run(tree: Path, state: str, after: str, *extra) -> "tuple[int, str]": "--before", str(fixtures / "flat.yaml"), "--after", str(fixtures / after), "--live-entities", str(fixtures / "live-entities-recorded.txt"), + "--reference", str(fixtures / "announced-reference.txt"), *extra], capture_output=True, text=True, env={**os.environ, "PYTHONDONTWRITEBYTECODE": "1"}) return proc.returncode, proc.stdout + proc.stderr -for label, (state, after) in CONTROLS.items(): - code, out = run(HERE, state, after) - check(f"{label}: the shipped code, {state} -> {after}, exits 0 with no " - f"findings", code == RR.EXIT_OK and "NO FINDINGS" in out) - -# ⚠️ AND THE SOFTENING IS NOT A HOLE. The SAME key stranded with a NON-default -# value is still a finding - that is the whole of part 5 - so the two cases are -# separated by the VALUE and not by the key name. -_fresh_code, _fresh_out = run(HERE, "state-fresh.json", "roster.yaml") -check("a fresh install DOES strand `unit/inverter-a/sign_tripped` - the key is " - "written and nothing on the roster reads it - and the report SAYS SO", - _fresh_code == RR.EXIT_OK - and "unit/inverter-a/sign_tripped = False" in _fresh_out) -check("...and does NOT call it a finding, because the value is the default; " - "the same key at True is the finding part 5 turns on, so the two cases " - "are separated by the VALUE and not by the key name", - "STRANDED STORE KEY: unit/inverter-a/sign_tripped" not in _fresh_out - and "STRANDED STORE KEY: unit/inverter-a/sign_tripped = True" - in "\n".join(l.strip() for l in - run(HERE, "state-legacy.json", "roster.yaml")[1].splitlines())) - -# ⚠️ AND THE INPUT FILE IS NEVER WRITTEN. The owner runs this against a copy of -# a live state file; a tool that edited it would be a tool that had to be -# trusted rather than one that cannot do harm. -_before_bytes = (FIX / "state-legacy.json").read_bytes() -run(HERE, "state-legacy.json", "roster.yaml", "--reverse", "--round-trip") -check("the state file the harness was pointed at is BYTE-IDENTICAL afterwards, " - "including after the reverse and round-trip legs, which write", - (FIX / "state-legacy.json").read_bytes() == _before_bytes) - - -# --------------------------------------------------------------------------- # -print() -print("3. ⭐ THE MUTATIONS: each one breaks the shipped migration or the " - "entity-id composition, and each must turn its control RED") -# --------------------------------------------------------------------------- # def mutant_tree(edits) -> str: """A private copy of the add-on with `edits` applied. @@ -243,6 +217,189 @@ def mutant_tree(edits) -> str: return str(dest) +def legs(out: str) -> dict: + """The report, split into its legs on the tool's `LEG ` markers. + + ⚠️ THIS FUNCTION IS THE FIX FOR A DEFECT IN THIS FILE, AND THE DEFECT IS + HABIT 4 ONE LEVEL UP. Part 5's check that the disarmed latch is "NOT THE + SAME-STEP PATH ALONE" was written as + `out.count("LATCH CAME UP DISARMED") >= 2`. The tool prints every finding + TWICE per red leg - once in that leg's own FINDINGS block and once in the + closing summary - so ONE red leg yields exactly 2 and the check passes. + Measured on three builds: + + shipped code, both legs red count 4 + fix at main.py:250 only -> STAGED alone red count 2 ok + partial guard fix on rep.ran -> SAME-STEP alone red count 2 ok + + The check named "NOT THE SAME-STEP PATH ALONE" reported `ok` on the build + whose remaining failure IS the same-step path alone - and, worse, applying + the fix DOCS.md names as the cause left all five part-5 checks green. A + count over a whole report cannot distinguish one leg from two; only reading + the legs separately can. + + ⚠️ AND IT CLOSES ON `END LEG`, WHICH IS THE SAME DEFECT ONE LEVEL DOWN. + The first version of this splitter opened on `LEG ` and never closed, + so every line after the LAST marker went to that leg - including the + closing summary, which repeats every finding from every leg. The last leg + then read red whenever ANY leg was red. Measured on a build whose SAME-STEP + leg is genuinely clean: it still reported SAME-STEP disarmed. Part 5b is + the running control for exactly that. + """ + out_legs, current = {}, None + for line in out.splitlines(): + if line.startswith(" LEG "): + current = line.strip()[4:].strip() + out_legs[current] = [] + elif line.startswith(" END LEG "): + current = None + elif current is not None: + out_legs[current].append(line) + return {k: "\n".join(v) for k, v in out_legs.items()} + + +for label, (state, after) in CONTROLS.items(): + code, out = run(HERE, state, after) + check(f"{label}: the shipped code, {state} -> {after}, exits 0 with no " + f"findings", code == RR.EXIT_OK and "NO FINDINGS" in out) + +# ⚠️ AND THE SOFTENING IS NOT A HOLE. The SAME key stranded with a NON-default +# value is still a finding - that is the whole of part 5 - so the two cases are +# separated by the VALUE and not by the key name. +_fresh_code, _fresh_out = run(HERE, "state-fresh.json", "roster.yaml") +# ⚠️ THE NAMESPACE IS READ OUT OF THE TOOL'S OWN REPORT, NOT SPELLED HERE. A +# hardcoded `unit/inverter-a/...` makes this check fail for the RIGHT reason on +# the shipped code and for the WRONG reason on a build that fixes the migration +# to use the roster's own name - it would look like the default-softening broke +# when what changed is the namespace. What this check is about is the VALUE. +_stranded_default = set(re.findall( + r"^\s+stranded\s+(unit/\S+/sign_tripped) = False$", _fresh_out, re.M)) +check("a fresh install DOES strand a `.../sign_tripped` key - it is written " + "and nothing on the roster reads it - and the report SAYS SO under its " + f"own heading (found: {sorted(_stranded_default)})", + _fresh_code == RR.EXIT_OK and len(_stranded_default) == 1) +check("...and does NOT call it a finding, because the value is the DEFAULT; " + "the same key at True is the finding part 5 turns on, so the two cases " + "are separated by the VALUE and not by the key name", + not any(f"STRANDED STORE KEY: {k}" in _fresh_out + for k in _stranded_default) + and "STRANDED STORE KEY: unit/inverter-a/sign_tripped = True" + in run(HERE, "state-legacy.json", "roster.yaml")[1]) + +# --------------------------------------------------------------------------- # +print() +print("2b. ⚠️ OMITTING THE INSTALL'S ENTITY LIST IS A REFUSAL, NOT A QUIETER " + "RUN") +# --------------------------------------------------------------------------- # +# ⚠️ THIS PART EXISTS BECAUSE THE ASYMMETRY WAS BACKWARDS AND IT WAS MEASURED. +# A `--live-entities` file that could not be READ refused with exit 2; the flag +# simply not being PASSED produced "NO FINDINGS" and exit 0 - and +# `grep -ci "live-entities|recorded list|composition"` over that report returned +# ZERO, so nothing in the artifact an owner reads said half the check had not +# run. Measured on the two entity mutations at the time: +# +# M5 device rename with the flag: exit 1 without: exit 0 +# M6 sensor rename with the flag: exit 1 without: exit 0 +# +# The owner runs this ONCE, at commissioning, under time pressure. A forgotten +# flag must not hand back a clean bill of health for the half of the transition +# that orphans entities. +def raw(*args): + """`(exit code, THE REPORT, the log)` - and the split matters. + + ⚠️ THE REPORT IS STDOUT AND THE CONTROLLER'S LOGGING IS STDERR. Grading + "is the degraded banner near the end of the report" over the two + CONCATENATED reads the last lines of the LOG - `SAFETY-02 was TRIPPED...`, + `unparseable timestamp...` - and reports a missing banner that is in fact + printed. That is not a hypothetical; it is how this check first failed. + """ + fx = HERE / "tools" / "rehearsal" + proc = subprocess.run( + [sys.executable, str(HERE / "tools" / "rehearse_roster.py"), + "--state", str(fx / "state-legacy.json"), + "--before", str(fx / "flat.yaml"), + "--after", str(fx / "roster-inverter-a.yaml"), *args], + capture_output=True, text=True, + env={**os.environ, "PYTHONDONTWRITEBYTECODE": "1"}) + return proc.returncode, proc.stdout, proc.stderr + + +_no_code, _no_out, _no_err = raw() +check("omitting --live-entities REFUSES with exit 2 rather than reporting a " + "clean transition it did not check", + _no_code == RR.EXIT_COULD_NOT_RUN and "REFUSED" in _no_out + _no_err) + +_deg_code, _deg_out, _ = raw("--no-live-entities") +check("...and the explicit opt-out RUNS, so the escape hatch exists", + _deg_code in (RR.EXIT_OK, RR.EXIT_FINDING)) +check("⚠️ ...carrying a standing banner in the report HEADER and again in " + "the closing block, because a report is skimmed from the bottom", + _deg_out.count("DEGRADED RUN") >= 2 + and "DEGRADED RUN" in "\n".join( + _deg_out.strip().splitlines()[-4:])) +check("...and the degraded report is not silent about what it did not do - " + "the words are in it, which is the grep that returned 0 before", + sum(w in _deg_out.lower() for w in + ("live-entities", "orphan check", "not graded")) == 3) +check("contradictory flags are refused rather than one silently winning", + raw("--no-live-entities", "--live-entities", + str(FIX / "live-entities-recorded.txt"))[0] + == RR.EXIT_COULD_NOT_RUN) +check("...and the banner really is at the END of the report, which is the " + "half of that claim a concatenated stdout+stderr could not have graded", + "DEGRADED RUN" in "\n".join(_deg_out.strip().splitlines()[-4:])) + +# ⚠️ AND THE COMPOSITION HALF IS NOT BEHIND THE FLAG AT ALL ANY MORE. The +# reference is loaded on EVERY run, so a degraded run still catches a renamed +# device - which is what makes `--no-live-entities` a genuinely bounded +# concession rather than "half the tool off". +_ref_missing = raw("--no-live-entities", "--reference", "/nonexistent.txt") +check("a missing or malformed reference is exit 2 on every run, because it is " + "the only reference in the tool that does not move with the code", + _ref_missing[0] == RR.EXIT_COULD_NOT_RUN) + +# ⚠️ AND THE VANISHED CHECK CARRIES THE SAME DEFAULT/NON-DEFAULT RULE, BOUNDED +# THE SAME WAY. `M9` below drops a NON-default key and must still be a finding; +# without it, "vanished at the default is not a finding" could widen into +# "vanished is not a finding" and nobody would see it. +_m9 = mutant_tree([("app/store.py", + " value = data.get(legacy, DEFAULTS.get(legacy))\n" + " store.set(target, value)", + " value = data.get(legacy, DEFAULTS.get(legacy))\n" + " store.data.pop(legacy, None)\n" + " store.set(target, value)")]) +_m9_code, _m9_out = run(Path(_m9), "state-legacy.json", "roster-inverter-a.yaml") +_m9_vanished = set(re.findall(r"VANISHED STORE KEY: (\S+) =", _m9_out)) +print(f" deleting the flat key as it migrates -> exit {_m9_code}, " + f"vanished {sorted(_m9_vanished)}") +check("⭐ a NON-default key that vanishes is STILL a finding - a migration " + "made to DELETE the flat rollback view store.py rule 2 requires it to " + "leave in place is caught, so the default-softening above is bounded by " + "its own positive", + _m9_code == RR.EXIT_FINDING + and {"last_completed", "phase_started", "last_start_attempt"} + <= _m9_vanished) +check("...and `phase` is NOT among them, because its value 'idle' IS the " + "default - the same mutation, the same file, and the rule splitting the " + "two on the VALUE rather than on the key", + "phase" not in _m9_vanished) +shutil.rmtree(Path(_m9).parent, ignore_errors=True) + +# ⚠️ AND THE INPUT FILE IS NEVER WRITTEN. The owner runs this against a copy of +# a live state file; a tool that edited it would be a tool that had to be +# trusted rather than one that cannot do harm. +_before_bytes = (FIX / "state-legacy.json").read_bytes() +run(HERE, "state-legacy.json", "roster.yaml", "--reverse", "--round-trip") +check("the state file the harness was pointed at is BYTE-IDENTICAL afterwards, " + "including after the reverse and round-trip legs, which write", + (FIX / "state-legacy.json").read_bytes() == _before_bytes) + + +# --------------------------------------------------------------------------- # +print() +print("3. ⭐ THE MUTATIONS: each one breaks the shipped migration or the " + "entity-id composition, and each must turn its control RED") +# --------------------------------------------------------------------------- # MUTATIONS = [ # (id, control, what it breaks, edits, the finding it must produce) ("M1", "CONTROL-B", @@ -298,6 +455,26 @@ MUTATIONS = [ '("grid", "goodwe_grid_power", "Grid power", "W", "power", "measurement", None),', '("grid", "goodwe_grid_power", "Grid power W", "W", "power", "measurement", None),')], "COMPOSITION DRIFT: this build announces sensor.goodwe_grid_power_w"), + + # ⚠️ M7 AND M8 ARE THE TWO CHANGES THAT ORPHAN AN INSTALL WITHOUT MOVING A + # SINGLE COMPOSED entity_id, so an entity_id-only reference cannot see them. + # Both came back exit 0 before the reference carried three columns. + ("M7", "CONTROL-A", + "the `unique_id` composition changes and NOT the entity_id: `mqtt.BASE` " + "is renamed, so every announced unique_id moves while every composed " + "entity_id stays byte-identical. HA's MQTT entity registry keys on " + "unique_id, so this is a NEW entity beside a dead one - and discovery is " + "retained with no removal payload, so the dead one stays", + [("app/mqtt.py", 'BASE = "goodwe_ctl"', 'BASE = "goodwe_controller"')], + "UNIQUE_ID DRIFT"), + + ("M8", "CONTROL-A", + "the DEVICE IDENTIFIER changes: HA's device registry keys on it, so the " + "whole install moves onto a new device and the old one keeps its dead " + "entities. Again no composed entity_id moves", + [("app/mqtt.py", '"identifiers": ["goodwe_rs485_controller"],', + '"identifiers": ["goodwe_rs485_ctl_v2"],')], + "DEVICE IDENTIFIER DRIFT"), ] for mid, control, what, edits, must_say in MUTATIONS: @@ -355,6 +532,26 @@ check("...and the store really did change shape underneath it, so this is a " "ns.inverter-a.sign_tripped" in _out and "unit/inverter-a/" not in _out) shutil.rmtree(Path(_tree).parent, ignore_errors=True) +# ⚠️ THE SECOND DECLARED SURVIVOR, AND IT IS THE ONE THAT BOUNDS M5-M8. Four +# mutations of `app/mqtt.py` are now required to go red; without a mutation of +# the same file that must NOT, "the reference notices any edit to mqtt.py" +# would be indistinguishable from "the reference notices what HA keys on". +# `object_id` is the case: `app/mqtt.py` lines 36-42 and 61-76 record that HA +# 2026.8 does NOT honour it in a discovery payload - the entity_id is composed +# from device name plus entity name - so renaming one changes nothing an +# install can see, and a tool that refused it would refuse a free change. +_tree2 = mutant_tree([("app/mqtt.py", + '("grid", "goodwe_grid_power", "Grid power",', + '("grid", "goodwe_grid_pwr", "Grid power",')]) +_code2, _out2 = run(Path(_tree2), *CONTROLS["CONTROL-A"]) +print(f" renaming an object_id (HA does not honour it) -> exit {_code2}") +check("renaming an `object_id` survives, because HA composes the entity_id " + "from the device and entity NAMES and ignores it - so the four mqtt.py " + "mutations above are red for what HA keys on, not for being edits to " + "mqtt.py", + _code2 == RR.EXIT_OK) +shutil.rmtree(Path(_tree2).parent, ignore_errors=True) + # --------------------------------------------------------------------------- # print() @@ -382,17 +579,82 @@ check("...and the harness names the two latch keys SAFETY-44 asks about, by " check("...and the mechanism is visible in the report: the keys are stranded " "under `inverter-a`, which is not on the roster, so no machine reads them", "STRANDED STORE KEY" in out44 and "['garage', 'shed']" in out44) -check("⚠️ AND IT IS NOT THE SAME-STEP PATH ALONE. Running the flat config " - "first and declaring the roster afterwards - the staged upgrade - " - "reaches the same disarmed latch, because the first start stamps " - "`schema_version` and the second start's migration then returns early", - out44.count("LATCH CAME UP DISARMED") >= 2) +# ⚠️ PER LEG, NOT A COUNT OVER THE WHOLE REPORT. See `legs()` for the three +# builds this distinction was measured on and for what the count missed. +_L = legs(out44) +print(f" legs in the report: {sorted(_L)}") +for _leg in ("STAGED", "SAME-STEP"): + print(f" LEG {_leg}: " + f"{'DISARMED' if 'LATCH CAME UP DISARMED' in _L.get(_leg, '') else 'clean'}") +check("the report is split into legs this check can grade SEPARATELY - the " + "tool prints a `LEG ` marker, because counting a finding's text over " + "the whole report cannot tell one red leg from two", + set(_L) == {"STAGED", "SAME-STEP"}) +check("⚠️ SAME-STEP is disarmed: the roster is declared in the SAME step as " + "the upgrade, the migration runs and misdirects the latch to " + "`inverter-a`", + "LATCH CAME UP DISARMED" in _L.get("SAME-STEP", "")) +check("⚠️ AND IT IS NOT THE SAME-STEP PATH ALONE - the STAGED leg reaches the " + "same disarmed latch by a DIFFERENT route: the flat start stamps " + "`schema_version`, so the roster start's migration takes its early " + "return and the latch never moves at all", + "LATCH CAME UP DISARMED" in _L.get("STAGED", "")) # ⚠️ AND THE CONTRAST, WHICH IS WHAT STOPS THE CHECK ABOVE BEING VACUOUS. The # SAME state file onto a roster whose first entry IS `inverter-a` comes up # LATCHED. The two runs differ in one thing - the first roster name - so the # finding is attributable to it and to nothing else. code_ok, out_ok = run(HERE, "state-legacy.json", "roster-inverter-a.yaml") +check("...and no leg's text has swallowed the CLOSING SUMMARY, which repeats " + "every finding from every leg - the failure that made the last leg read " + "red whenever any leg was", + not any("FINDING(S) - this transition is not clean" in v + for v in _L.values())) + +# --------------------------------------------------------------------------- # +print() +print("5b. THE SPLITTER'S POSITIVE CONTROL: a build in which exactly ONE leg " + "is clean") +# --------------------------------------------------------------------------- # +# ⚠️ A PER-LEG CHECK THAT HAS NEVER SEEN THE LEGS DISAGREE IS A COUNT WITH +# EXTRA STEPS. Every fixture above has both legs red, so nothing so far +# separates "reads each leg" from "reads the whole report twice". This part +# builds the one case that does: `migrate_unit_keys()` handed the roster's own +# first name instead of the hardcoded `UNIT_A`. +# +# ⚠️ AND WHAT IT MEASURES IS ALSO THE ANSWER TO "IS THAT THE FIX". It is NOT. +# It repairs the SAME-STEP path only. The STAGED path is untouched, because +# there the flat start stamps `schema_version = 1` and the roster start's +# migration takes its `from_version >= SCHEMA_VERSION` early return before the +# name it would be handed matters at all. A fix that stops at this line would +# close the half an operator is least likely to take and leave the half that +# "upgrade first, declare the roster later" produces. +_one_leg = mutant_tree([( + "app/main.py", + " self.store_migration = migrate_unit_keys(store, UNIT_A)", + " self.store_migration = migrate_unit_keys(store, " + "roster_names(opts)[0])")]) +_ol_code, _ol_out = run(Path(_one_leg), "state-legacy.json", "roster.yaml") +_OL = legs(_ol_out) +for _leg in ("STAGED", "SAME-STEP"): + print(f" LEG {_leg}: " + f"{'DISARMED' if 'LATCH CAME UP DISARMED' in _OL.get(_leg, '') else 'clean'}") +check("⭐ with `migrate_unit_keys()` handed the roster's own first name, the " + "SAME-STEP leg comes up CLEAN - so the splitter really does read the " + "legs separately, and a check that only counted would have missed this", + "LATCH CAME UP DISARMED" not in _OL["SAME-STEP"]) +check("⚠️ ...while the STAGED leg is STILL DISARMED on the same build: that " + "line is not the whole fix, because the staged path never reaches it - " + "the first start stamps `schema_version` and the second start's " + "migration returns early", + "LATCH CAME UP DISARMED" in _OL["STAGED"]) +check("...and the run as a whole is still a finding, so nobody reads this " + "partial fix as done", + _ol_code == RR.EXIT_FINDING) +shutil.rmtree(Path(_one_leg).parent, ignore_errors=True) + +print() +print("5c. back to the shipped code") check("the SAME state file onto a roster whose first entry IS `inverter-a` " "comes up latched and clean - so the finding above is caused by the " "roster NAME and not by the fixture being tripped", @@ -405,11 +667,15 @@ print("6. THE REVERSE DIRECTION: the retreat, and the return from it") # --------------------------------------------------------------------------- # code_r, out_r = run(HERE, "state-flat-070.json", "roster.yaml", "--reverse", "--round-trip") -_orphaned = set(re.findall(r"ORPHANED ENTITY: (sensor\.\S+)", out_r)) -print(f" the retreat orphans: {sorted(_orphaned)}") -check("the RETREAT is rehearsed at all, and it reports the entities it " - "orphans - the eight per-unit rows, which is the cost of backing out", - "THE RETREAT" in out_r and _orphaned == { +_RL = legs(out_r) +print(f" legs in the reverse run: {sorted(_RL)}") +_orphaned = set(re.findall(r"ORPHANED ENTITY: (sensor\.\S+)", + _RL.get("RETREAT", ""))) +print(f" the RETREAT leg orphans: {sorted(_orphaned)}") +check("the RETREAT is rehearsed as its own leg, and THAT LEG reports the " + "entities it orphans - the eight per-unit rows, which is the cost of " + "backing out", + {"RETREAT", "ROUND-TRIP"} <= set(_RL) and _orphaned == { f"sensor.goodwe_{u}_{k}" for u in ("garage", "shed") for k in ("setpoint", "battery_power", "battery_soc", "sign_latch")}) @@ -421,13 +687,13 @@ check("...and NOT one site entity: the eight ids the install has today survive " "sensor.goodwe_battery_power", "sensor.goodwe_p1_sample_age"))) check("...and the first pack's maintenance history SURVIVES the retreat, " "because `UnitHistory.mirror_flat` keeps the flat keys live - so the " - "retreat itself does not lose state", - "MAINTENANCE HISTORY LOST" not in out_r.split("THE ROUND TRIP")[0] - .split("THE RETREAT")[-1]) + "retreat itself does not lose state. ⚠️ Read from the RETREAT LEG " + "alone, so the round trip's own loss cannot be mistaken for it", + "MAINTENANCE HISTORY LOST" not in _RL["RETREAT"]) check("⚠️ ...but the ROUND TRIP does not: a maintenance cycle completed while " "the install is rolled back is DISCARDED when it re-advances, because " "`maint_schema_version` is already stamped and the migration skips", - "RETREAT-WINDOW WORK DISCARDED" in out_r) + "RETREAT-WINDOW WORK DISCARDED" in _RL["ROUND-TRIP"]) check("...and that is a leg whose data can reach the failure it names: the " "retreat window actually contains a completion, written through the " "shipped `UnitHistory`, at a timestamp the starting fixture does not have", diff --git a/goodwe_controller/tools/rehearsal/announced-reference.txt b/goodwe_controller/tools/rehearsal/announced-reference.txt new file mode 100644 index 0000000..7e2aa5a --- /dev/null +++ b/goodwe_controller/tools/rehearsal/announced-reference.txt @@ -0,0 +1,48 @@ +# WHAT THIS BUILD IS SUPPOSED TO ANNOUNCE, recorded so that a change to the +# composition has something to disagree with. THREE COLUMNS: +# +# +# +# ⚠️ ALL THREE, AND THE SECOND AND THIRD ARE THE HALF THE FIRST DRAFT MISSED. +# A reference that carries only entity_ids cannot see the two changes that +# orphan an install without changing a single composed id: +# +# `BASE` renamed (`goodwe_ctl` -> anything) every unique_id changes +# `DEVICE["identifiers"]` changed HA sees a NEW device +# +# Home Assistant's MQTT integration keys its entity registry on `unique_id` and +# its device registry on the device identifier. Change either and HA creates a +# NEW entity beside the old one - and because discovery is published with +# `retain=True` and nothing publishes a removal payload, the OLD config stays on +# the broker and the dead entity stays in the registry, still named by every +# automation that referenced it, never updating again. The composed entity_id is +# identical on both, so an entity_id-only comparison reports a clean transition. +# That is the same shape as the habit-4 this file's own card already caught +# once: a reference that does not carry the thing that moved. +# +# ⚠️ AND `object_id` IS DELIBERATELY NOT A COLUMN. `app/mqtt.py` lines 36-42 and +# 61-76 record that HA 2026.8 does NOT honour `object_id` in a discovery +# payload - the id is composed from device name plus entity name - so renaming +# one is a true no-op and a column for it would make this file refuse a change +# that costs nothing. The check is graded on what HA actually keys on. +# +# PROVENANCE, stated exactly: +# - recorded 2026-09-08 from `release/1.0` at `meter_source: firmware`, +# roster of one, by reading `mqtt.discovery_payloads(units=[])`. It is what +# the code announces. +# - `sensor.goodwe_p1_sample_age` is additionally recorded in `app/mqtt.py` as +# VERIFIED AGAINST THE LIVE SYSTEM on 2026-08-26, and the ESP32's ingest +# guard subscribes to that exact string. +# - ⚠️ IT IS NOT A DUMP OF THE LIVE INSTALL'S ENTITY REGISTRY. That is what +# `--live-entities` is for. This file is about THE BUILD. +# +# Editing this file is how you say "the announcement changed on purpose". Do not +# edit it to make a check pass. +sensor.goodwe_setpoint goodwe_ctl_setpoint goodwe_rs485_controller +sensor.goodwe_grid_power goodwe_ctl_grid goodwe_rs485_controller +sensor.goodwe_battery_power goodwe_ctl_battery goodwe_rs485_controller +sensor.goodwe_battery_soc goodwe_ctl_soc goodwe_rs485_controller +sensor.goodwe_circulating_power goodwe_ctl_circulation goodwe_rs485_controller +sensor.goodwe_maintenance_phase goodwe_ctl_phase goodwe_rs485_controller +sensor.goodwe_controller_status goodwe_ctl_status goodwe_rs485_controller +sensor.goodwe_p1_sample_age goodwe_ctl_p1_age goodwe_rs485_controller diff --git a/goodwe_controller/tools/rehearsal/live-entities-recorded.txt b/goodwe_controller/tools/rehearsal/live-entities-recorded.txt index d991ef4..0a07fd8 100644 --- a/goodwe_controller/tools/rehearsal/live-entities-recorded.txt +++ b/goodwe_controller/tools/rehearsal/live-entities-recorded.txt @@ -1,29 +1,34 @@ -# The entity ids this add-on ANNOUNCES at the shipped roster of one. +# THE ENTITY IDS THE INSTALL HAS — a stand-in for the owner's real export. # -# ⚠️ THIS FILE IS EXTERNAL DATA AND THAT IS ITS ENTIRE PURPOSE. The before/after -# entity diff inside `rehearse_roster.py` cannot see a change to the id -# COMPOSITION, because both sides of that diff are composed by the same build: -# rename the MQTT device and every id moves on both sides equally, and the diff -# reports "nothing orphaned" about a change that orphans everything. That is -# `MUTATION.md` habit 4 - a check whose data cannot reach the failure it names - -# and a list that does NOT move with the code is the only thing that closes it. +# ⚠️ THIS FILE AND `announced-reference.txt` ANSWER DIFFERENT QUESTIONS AND ARE +# NOT INTERCHANGEABLE: # -# PROVENANCE, stated exactly: -# - recorded 2026-09-08 by running this tool against `release/1.0` at -# `meter_source: firmware`, roster of one. It is what the code announces. -# - `sensor.goodwe_p1_sample_age` is additionally recorded in `app/mqtt.py` as -# VERIFIED AGAINST THE LIVE SYSTEM on 2026-08-26, and the ESP32's ingest -# guard subscribes to that exact string. -# - ⚠️ IT IS NOT A DUMP OF THE LIVE INSTALL'S ENTITY REGISTRY. Live HA carries -# roughly 49 `goodwe*` entities (card ARCH-02, read 2026-09-06); the other -# ~41 belong to the ESPHome firmware, to template sensors and to helpers, -# and this add-on announces none of them. THE OWNER STEP in DOCS.md replaces -# this file with the real export, and the tool then separates the ids it -# owns from the ids it does not rather than claiming orphans it cannot -# cause. +# announced-reference.txt what the BUILD announces. Loaded on EVERY run. +# Three columns, because HA keys on `unique_id` and +# on the device identifier as well as on the +# composed entity_id. +# this file (--live-entities) +# what the INSTALL has. One id per line. Orphans +# are graded against it: an id the install has and +# the AFTER shape does not announce is an orphan +# whatever the build's own history says. # -# Editing this file is how you say "the ids changed on purpose". Do not edit it -# to make a check pass. +# ⚠️ OMITTING `--live-entities` IS A REFUSAL (exit 2), NOT A QUIETER RUN. It +# used to be the other way round - an unreadable file refused, a missing flag +# passed with exit 0 and said nothing about the half it had not checked - and +# the owner runs this ONCE, at commissioning, under time pressure. +# `--no-live-entities` is the deliberate opt-out and it prints a standing +# banner in the report header and again on the exit line. +# +# PROVENANCE: these eight are what `release/1.0` announces at +# `meter_source: firmware`, roster of one, recorded 2026-09-08. They stand in +# for the export so the checks run in CI. ⚠️ THE REAL INSTALL CARRIES ROUGHLY 49 +# `goodwe*` ENTITIES (card ARCH-02, read 2026-09-06); the other ~41 belong to +# the ESPHome firmware, to template sensors and to helpers, and this add-on +# announces none of them. The tool separates the ids it owns from the ids it +# does not rather than claiming orphans it cannot cause - which is the half of +# this file the eight-line stand-in cannot exercise, and the reason DOCS.md's +# owner step asks for the real export. sensor.goodwe_battery_power sensor.goodwe_battery_soc sensor.goodwe_circulating_power diff --git a/goodwe_controller/tools/rehearse_roster.py b/goodwe_controller/tools/rehearse_roster.py index a86fdf2..3c8c539 100644 --- a/goodwe_controller/tools/rehearse_roster.py +++ b/goodwe_controller/tools/rehearse_roster.py @@ -267,8 +267,33 @@ def compose(device_name, entity_name) -> str: ANCHOR_ID = "sensor.goodwe_p1_sample_age" -def entity_ids(opts: dict) -> "dict[str, str]": - """`{entity_id: unique_id}` for every row this add-on announces on `opts`. +class Row: + """One announced entity, in the three terms Home Assistant keys on. + + ⚠️ THE UNIQUE_ID AND THE DEVICE IDENTIFIER ARE NOT DECORATION, AND LEAVING + THEM OUT OF THE COMPARISON WAS THIS TOOL'S SECOND HABIT-4. HA's MQTT + integration keys its ENTITY registry on `unique_id` and its DEVICE registry + on the device identifier. Change either one and HA creates a NEW entity + beside the old - while the COMPOSED entity_id is byte-identical, so a + comparison over composed ids alone reports a clean transition about a change + that orphans the whole install. Measured: renaming `mqtt.BASE` and changing + `DEVICE["identifiers"]` both came back exit 0 before this class existed. + """ + + __slots__ = ("entity_id", "unique_id", "device_id") + + def __init__(self, entity_id, unique_id, device_id): + self.entity_id = entity_id + self.unique_id = unique_id + self.device_id = device_id + + def __repr__(self): + return (f"{self.entity_id} (unique_id {self.unique_id}, device " + f"{self.device_id})") + + +def announced_rows(opts: dict) -> "dict[str, Row]": + """`{entity_id: Row}` for every row this add-on announces on `opts`. ⚠️ `discovery_payloads()` IS THE SHIPPED ANNOUNCER and `roster_names()` is the shipped naming rule, so the roster shape decision is not restated here. @@ -278,7 +303,9 @@ def entity_ids(opts: dict) -> "dict[str, str]": omit=() if is_enabled(opts) else ("p1_age",)) out = {} for _topic, cfg in payloads: - out[compose(cfg["device"]["name"], cfg["name"])] = cfg["unique_id"] + eid = compose(cfg["device"]["name"], cfg["name"]) + out[eid] = Row(eid, cfg["unique_id"], + (cfg["device"].get("identifiers") or [""])[0]) return out @@ -301,20 +328,56 @@ def _check_anchor() -> None: "The model is wrong; no orphan claim from it can be believed.") -def load_entity_list(path: str) -> set: - """The entity ids that exist on the install, one per line, `#` comments. +# Shipped beside this file, and LOADED ON EVERY RUN. See `load_reference()`. +DEFAULT_REFERENCE = (Path(__file__).resolve().parent / "rehearsal" + / "announced-reference.txt") - ⚠️ EXTERNAL DATA, AND THE TOOL IS BLIND WITHOUT IT. The before/after diff is - composed twice by the SAME build, so a change to the id composition itself - - a renamed MQTT device, a renamed sensor - moves both sides equally and the - diff reports nothing. `tools/rehearsal/live-entities-recorded.txt` carries - the recorded list and DOCS.md's owner step replaces it with a real export. + +def load_reference(path) -> "dict[str, Row]": + """What this build is SUPPOSED to announce: ` `. + + ⚠️ EXTERNAL DATA, AND THE TOOL IS BLIND WITHOUT IT - which is why it is + loaded unconditionally and a missing one is EXIT 2 rather than a quieter + run. The before/after diff is composed twice by the SAME build, so a change + to the composition itself moves both sides equally and the diff reports + nothing. Only a reference that does not move with the code can see it. + + ⚠️ THREE COLUMNS ARE REQUIRED AND A SHORT LINE IS REFUSED. A two-column + reference would silently stop grading device identifiers, which is exactly + the blind spot this file exists to close - and it would do it without + saying so, which is the failure mode the whole card is about. + """ + rows = {} + for lineno, line in enumerate( + Path(path).read_text(encoding="utf-8").splitlines(), 1): + line = line.split("#", 1)[0].strip() + if not line: + continue + parts = line.split() + if len(parts) != 3: + raise Refused( + f"{path} line {lineno}: expected ` " + f"`, got {len(parts)} field(s). A short " + f"line would silently stop grading what HA keys on.") + rows[parts[0]] = Row(*parts) + if not rows: + raise Refused(f"{path}: no rows in it") + return rows + + +def load_entity_list(path: str) -> set: + """The entity ids that EXIST ON THE INSTALL, one per line, `#` comments. + + This is the owner's export from Home Assistant, and it answers a different + question from `load_reference()`: that one says what the BUILD announces, + this one says what the INSTALL has. An id the install has and the new shape + does not announce is an orphan whatever the build's own history says. """ out = set() for line in Path(path).read_text(encoding="utf-8").splitlines(): line = line.split("#", 1)[0].strip() if line: - out.add(line) + out.add(line.split()[0]) if not out: raise Refused(f"{path}: no entity ids in it") return out @@ -359,7 +422,7 @@ class Snapshot: self.units = [inv.name for inv in controller.inverters] self.data = dict(store.data) self.read = set(store.data.reads) - self.entities = entity_ids(opts) + self.entities = announced_rows(opts) self.latched = {inv.name: bool(inv.sign_wd.tripped) for inv in controller.inverters} self.latch_reason = {inv.name: inv.sign_wd.reason @@ -371,9 +434,11 @@ class Snapshot: self.store_migration = controller.store_migration self.maint_migration = controller.maint_migration self.machine_read = machine_reads(controller, store) - # Keys no machine reads whose value is the default `Store.load()` would - # have supplied. Reported, never a finding: see `diff()`. + # Keys no machine reads, and keys that disappeared, whose value is the + # default `Store.load()` would have supplied. Reported, never a + # finding: see `diff()`. self.stranded_at_default = [] + self.vanished_at_default = [] @property def unit_keys(self): @@ -400,8 +465,15 @@ def machine_reads(controller, store) -> set: Inverter(inv.name, store=store) for m in controller.maints: for key in MAINT_UNIT_KEYS: + # ⚠️ `get()` AND NOT ALSO `get_time()`. Both route through + # `store.data.get(self.key(name))`, so either one records the read + # this probe exists to record - but `get_time()` on `phase`, which + # holds "idle" and not a timestamp, makes `Store.get_time()` log + # "unparseable timestamp" at WARNING. A probe that manufactures + # alarming log lines about a healthy store teaches an operator to + # ignore this tool's stderr, which is where the controller's own + # real warnings appear. m.store.get(key) - m.store.get_time(key) seen = set(store.data.reads) store.data.reads.clear() return seen @@ -442,11 +514,24 @@ def diff(before: Snapshot, after: Snapshot) -> list: findings = [] # -- store keys --------------------------------------------------------- - gone = sorted(set(before.data) - set(after.data)) - for key in gone: + # ⚠️ THE SAME DEFAULT/NON-DEFAULT RULE THE STRANDED CHECK USES, AND FOR THE + # SAME REASON - `MigrationReport.lost`'s, in store.py: a dropped + # `sign_tripped = False` has lost the DEFAULT, which is the statement + # "absent" already makes, so nothing is gone; a dropped `True` has lost a + # LATCH. Applying it to STRANDED and not here was an inconsistency with a + # cost: a store whose flat start wrote the two SAFETY-02 defaults under one + # name and whose roster start writes them under another reported two + # VANISHED findings about a pair of defaults, and CONTROL-C went red on a + # build that was CORRECT. + for key in sorted(set(before.data) - set(after.data)): + value = before.data[key] + bare = key.rsplit("/", 1)[-1] + if value in (None, DEFAULTS.get(bare)): + after.vanished_at_default.append(f"{key} = {value!r}") + continue findings.append(Finding( "VANISHED STORE KEY", - f"{key} = {before.data[key]!r} was in the store before the " + f"{key} = {value!r} was in the store before the " f"transition and is not there afterwards")) # ⭐ THE ONE SAFETY-44 IS ABOUT. A key that is present, correct, verified by @@ -536,38 +621,91 @@ def _latch_finding(key, before, after): return out -def entity_list_findings(recorded: set, before: Snapshot, - after: Snapshot) -> list: - """Grade both snapshots against a list that does NOT move with the code.""" +# ⚠️ THE REFERENCE IS GRADED AT A FIXED, CANONICAL SHAPE AND NOT AT THE +# OPERATOR'S CONFIG. The reference records what `release/1.0` announces at +# `meter_source: firmware`, roster of one. Grading it against the BEFORE config +# instead asks two questions at once and gets a wrong answer to both: a site +# with `meter_source: off` legitimately omits `p1_age` (see `discovery_payloads` +# and `is_enabled`), so the tool reported "the reference records +# sensor.goodwe_p1_sample_age and this build does not announce it" about a +# perfectly correct build. The question here is ONLY "has this build's +# composition drifted", which is a property of the CODE, so it is asked of the +# code at the shape the reference was taken at. +REFERENCE_PROBE_OPTS = {"meter_source": "firmware"} + + +def reference_findings(reference: "dict[str, Row]") -> list: + """Does this BUILD still announce what the reference records? + + ⚠️ RUN UNCONDITIONALLY, AND OVER ALL THREE TERMS HA KEYS ON. The + before/after diff cannot see a change to the composition, because both of + its sides are composed by the same build. This is the only check in the + tool whose reference does not move with the code, so it is the only one + that can - and it is graded on the entity_id, the `unique_id` AND the + device identifier, because a change to either of the latter two orphans an + install while leaving every composed entity_id byte-identical. + """ out = [] - announced_before = set(before.entities) - announced_after = set(after.entities) - for eid in sorted(announced_before - recorded): + built = announced_rows(REFERENCE_PROBE_OPTS) + for eid in sorted(set(built) - set(reference)): out.append(Finding( "COMPOSITION DRIFT", - f"this build announces {eid} in the BEFORE shape, and that id is " - "not on the recorded list of ids the install has. The id " - "composition has changed under the install: Home Assistant keeps " - "the id it composed at first discovery, so what this actually " - "creates is a NEW entity beside a dead one.")) - owned = recorded & announced_before - for eid in sorted(owned - announced_after): + f"this build announces {eid} and the reference does not record " + f"it. The composed entity_id has changed: Home Assistant keeps the " + f"id it composed at FIRST discovery, so what this creates is a NEW " + f"entity beside one that stops updating.")) + for eid in sorted(set(reference) - set(built)): out.append(Finding( - "ORPHANED ENTITY (against the recorded list)", + "COMPOSITION DRIFT", + f"the reference records {eid} and this build does NOT announce it " + f"at the shape the reference was taken at " + f"({REFERENCE_PROBE_OPTS}).")) + for eid in sorted(set(reference) & set(built)): + want, got = reference[eid], built[eid] + if want.unique_id != got.unique_id: + out.append(Finding( + "UNIQUE_ID DRIFT", + f"{eid} is announced with unique_id {got.unique_id!r} where " + f"the reference records {want.unique_id!r}. ⚠️ THE COMPOSED " + f"ENTITY_ID IS UNCHANGED, WHICH IS WHY THIS NEEDS ITS OWN " + f"CHECK: HA's MQTT entity registry keys on `unique_id`, so a " + f"changed one is a NEW entity - and discovery is retained with " + f"no removal payload, so the old config stays on the broker " + f"and the old entity stays in the registry, named by every " + f"automation that referenced it and never updating again.")) + if want.device_id != got.device_id: + out.append(Finding( + "DEVICE IDENTIFIER DRIFT", + f"{eid} is announced on device {got.device_id!r} where the " + f"reference records {want.device_id!r}. HA's device registry " + f"keys on the identifier, so this is a NEW device carrying new " + f"entities, beside the old device and its dead ones.")) + return out + + +def install_findings(installed: set, before: Snapshot, + after: Snapshot) -> list: + """Grade the transition against the ids the INSTALL actually has.""" + out = [] + owned = installed & set(before.entities) + for eid in sorted(owned - set(after.entities)): + out.append(Finding( + "ORPHANED ENTITY (against the install's own entity list)", f"{eid} exists on the install and is announced in the BEFORE " "shape, and the AFTER shape does not announce it.")) return out -def report_entity_list(recorded: set, before: Snapshot, after: Snapshot) -> None: +def report_entity_list(installed: set, before: Snapshot, + after: Snapshot) -> None: announced_before = set(before.entities) - foreign = sorted(recorded - announced_before) + foreign = sorted(installed - announced_before) print() - print(" -- AGAINST THE RECORDED ENTITY LIST ------------------------------") - print(f" {len(recorded)} ids on the list, " - f"{len(recorded & announced_before)} of them announced by this " + print(" -- AGAINST THE INSTALL'S OWN ENTITY LIST -------------------------") + print(f" {len(installed)} ids on the install, " + f"{len(installed & announced_before)} of them announced by this " f"add-on in the BEFORE shape") - print(f" {len(foreign)} on the list that this add-on announces in " + print(f" {len(foreign)} on the install that this add-on announces in " "NEITHER shape - the roster switch cannot orphan these, they belong " "to the") print(" ESPHome firmware, to template sensors or to helpers:") @@ -638,9 +776,19 @@ def _round_trip_findings(completed: str, readvanced: Snapshot) -> list: return out -def report_transition(name, before, after, findings) -> None: +def report_transition(leg_id, name, before, after, findings) -> None: + """One leg of the rehearsal. + + ⚠️ `LEG ` IS A MACHINE-PARSEABLE MARKER AND IT IS LOAD-BEARING. Every + finding is printed twice - once here and once in the closing summary - so + COUNTING a finding's text across the whole report cannot tell ONE red leg + from TWO. `test_rehearse.py` part 5 was written that way and reported `ok` + on a build whose failure was a single leg. The pin now splits the report on + these markers and asks each leg separately. + """ print() print("=" * 78) + print(f" LEG {leg_id}") print(f" {name}") print("=" * 78) print(f" roster before : {before.units}") @@ -677,14 +825,16 @@ def report_transition(name, before, after, findings) -> None: mark = " " if key in after.data else " (absent, defaulted) " print(f" {key}{mark}") - if after.stranded_at_default: + if after.stranded_at_default or after.vanished_at_default: print() - print(" keys no machine reads whose value IS the default, so " - "nothing is lost by") - print(" stranding them - reported rather than hidden, not counted " - "as findings:") + print(" keys whose value IS the default `Store.load()` supplies, so " + "nothing is lost") + print(" by stranding or dropping them - reported rather than " + "hidden, not findings:") for key in after.stranded_at_default: - print(f" {key} = {after.data[key]!r}") + print(f" stranded {key} = {after.data[key]!r}") + for text in after.vanished_at_default: + print(f" vanished {text}") print() print(" -- MAINTENANCE HISTORY (MAINT-05: a second move must not drop it) -") @@ -710,20 +860,31 @@ def report_transition(name, before, after, findings) -> None: print() print(" -- ENTITY IDS ----------------------------------------------------") allids = sorted(set(before.entities) | set(after.entities)) + print(f" {'entity_id':<44} {'unique_id':<30} {'device':<26} state") for eid in allids: + row = after.entities.get(eid) or before.entities[eid] inb, ina = eid in before.entities, eid in after.entities mark = "both" if (inb and ina) else ("ORPHANED" if inb else "new") - print(f" {eid:<48} {mark}") + print(f" {eid:<44} {row.unique_id:<30} {row.device_id:<26} {mark}") print(f" {len(before.entities)} announced before, " f"{len(after.entities)} after") print() - print(" -- FINDINGS ------------------------------------------------------") + print(f" -- FINDINGS in LEG {leg_id} " + + "-" * max(0, 50 - len(str(leg_id)))) if not findings: - print(" none: nothing vanished, nothing stranded, nothing orphaned, " - "no history lost") + print(f" none in LEG {leg_id}: nothing vanished, nothing stranded, " + "nothing orphaned, no history lost") for f in findings: print(f" ⚠️ {f}") + # ⚠️ AN EXPLICIT CLOSE, AND IT IS NOT TIDINESS. Without it a reader that + # splits on the opening markers alone gives every line after the LAST one + # to that leg - including the closing summary, which repeats every finding + # from EVERY leg. The last leg then reads red whenever any leg is red, and + # a per-leg check built on that is the same defect it was written to fix, + # one level down. Measured: with only the opening marker, a build whose + # SAME-STEP leg was clean still reported SAME-STEP disarmed. + print(f" END LEG {leg_id}") def main(argv=None) -> int: @@ -736,20 +897,29 @@ def main(argv=None) -> int: help="the options being switched to (declared roster)") ap.add_argument("--reverse", action="store_true", help="also rehearse the RETREAT: roster -> flat") - ap.add_argument("--live-entities", default=None, - help="a file of entity ids the install actually has, one " - "per line. ⚠️ WITHOUT IT THE TOOL CANNOT SEE A CHANGE " - "TO THE ID COMPOSITION, because both sides of its own " - "diff are composed by the same build. " - "tools/rehearsal/live-entities-recorded.txt is the " - "recorded default; DOCS.md's owner step replaces it " - "with a real export.") ap.add_argument("--round-trip", action="store_true", help="also rehearse RETREAT then RE-ADVANCE, with a " "maintenance cycle completing while retreated. ⚠️ A " "retreat is only a retreat if you can come back from " "it, so the round trip is the leg that decides " "whether the reverse direction is supported.") + ap.add_argument("--reference", default=str(DEFAULT_REFERENCE), + help="what this build is SUPPOSED to announce: " + "` ` per " + "line. Loaded on EVERY run; a missing or malformed " + "one is exit 2, because it is the only reference in " + "this tool that does not move with the code.") + ap.add_argument("--live-entities", default=None, + help="a file of the entity ids the INSTALL actually has, " + "one per line - the owner's export from Home " + "Assistant. Orphans are graded against it.") + ap.add_argument("--no-live-entities", action="store_true", + help="⚠️ RUN WITHOUT THE INSTALL'S ENTITY LIST, ACCEPTING " + "THAT THE ORPHAN CHECK IS NOT GRADED AGAINST " + "REALITY. Required to omit --live-entities; the " + "report then carries a standing banner and the exit " + "line says so, because a forgotten flag must never " + "look like a clean bill of health.") ap.add_argument("--paths", default="both", choices=("staged", "same-step", "both"), help="staged: the box runs the BEFORE config once, then " @@ -759,45 +929,81 @@ def main(argv=None) -> int: "Default: both.") args = ap.parse_args(argv) + # ⚠️ THE ASYMMETRY THAT USED TO BE HERE WAS BACKWARDS AND IT IS THE WHOLE + # REASON THIS BLOCK EXISTS. A `--live-entities` file that could not be READ + # refused with exit 2; the flag simply not being PASSED produced "NO + # FINDINGS" and exit 0, with nothing in the report to say half the orphan + # check had not run - measured, `grep -ci` for the words returned 0. The + # owner runs this ONCE, at commissioning, under time pressure. A forgotten + # flag must not hand back a clean bill of health for the half of the + # transition that orphans entities. So omitting it is now a REFUSAL, and + # accepting the weaker run has to be said out loud. try: + if not args.live_entities and not args.no_live_entities: + raise Refused( + "no --live-entities. The orphan check is graded against the " + "ids the INSTALL has, and without that file it is not graded " + "against reality at all. Export it (DOCS.md, 'Before you " + "commission inverter B'), or pass --no-live-entities to " + "accept the weaker run - which is then stated in the report " + "and on the exit line rather than left to be assumed.") + if args.live_entities and args.no_live_entities: + raise Refused("--live-entities and --no-live-entities are " + "contradictory; pass one.") _check_anchor() + reference = load_reference(args.reference) before_opts = load_options(args.before) after_opts = load_options(args.after) + installed = (load_entity_list(args.live_entities) + if args.live_entities else None) if not Path(args.state).is_file(): raise Refused(f"{args.state}: not a file") - except Refused as err: + except (OSError, Refused) as err: print(f"REFUSED: {err}", file=sys.stderr) return EXIT_COULD_NOT_RUN + degraded = installed is None + print(__doc__.split("EXIT CODES")[0].rstrip()) print() print(f" state file (copied, never written): {args.state}") print(f" before config: {args.before}") print(f" after config: {args.after}") + print(f" reference : {args.reference} ({len(reference)} rows, graded " + f"on entity_id + unique_id + device identifier)") + if degraded: + print() + print(" " + "!" * 74) + print(" !! DEGRADED RUN: --no-live-entities. The ORPHAN CHECK IS NOT " + "GRADED AGAINST") + print(" !! THE INSTALL. This run can still see the composition " + "drifting away from") + print(" !! the reference, and it can see the store migration - but it " + "cannot tell") + print(" !! you that an entity the install HAS stops being announced, " + "because it") + print(" !! has not been told what the install has. ⚠️ A CLEAN RESULT " + "BELOW IS NOT A") + print(" !! CLEAN BILL OF HEALTH FOR THE ENTITIES.") + print(" " + "!" * 74) + else: + print(f" install list : {args.live_entities} ({len(installed)} ids)") findings = [] - recorded = None - try: - if args.live_entities: - recorded = load_entity_list(args.live_entities) - except (OSError, Refused) as err: - print(f"REFUSED: {err}", file=sys.stderr) - return EXIT_COULD_NOT_RUN tmpdir = tempfile.mkdtemp(prefix="rehearse-roster-") try: if args.paths in ("staged", "both"): b = snapshot("staged-before", before_opts, args.state, tmpdir) a = snapshot_over("staged-after", after_opts, b) - f = diff(b, a) - if recorded: - f += entity_list_findings(recorded, b, a) + f = diff(b, a) + reference_findings(reference) + if installed is not None: + f += install_findings(installed, b, a) report_transition( - "STAGED: the box runs the flat config, then restarts on the " - "roster", b, a, f) - if recorded: - report_entity_list(recorded, b, a) - print(" (findings from the recorded list are in the list " - "above)") + "STAGED", + "the box runs the flat config, then restarts on the roster", + b, a, f) + if installed is not None: + report_entity_list(installed, b, a) findings += f if args.paths in ("same-step", "both"): @@ -807,12 +1013,13 @@ def main(argv=None) -> int: # version that knew about namespaces. b2 = snapshot("samestep-before", before_opts, args.state, tmpdir) a2 = snapshot("samestep-after", after_opts, args.state, tmpdir) - f2 = diff(b2, a2) - if recorded: - f2 += entity_list_findings(recorded, b2, a2) + f2 = diff(b2, a2) + reference_findings(reference) + if installed is not None: + f2 += install_findings(installed, b2, a2) report_transition( - "SAME-STEP (SAFETY-44): a pre-DEV-02 flat store meets a " - "declared roster on its FIRST start under this version", + "SAME-STEP", + "SAFETY-44's path: a pre-DEV-02 flat store meets a declared " + "roster on its FIRST start under this version", b2, a2, f2) findings += f2 @@ -820,9 +1027,12 @@ def main(argv=None) -> int: r1 = snapshot("retreat-before", after_opts, args.state, tmpdir) r2 = snapshot_over("retreat-after", before_opts, r1) fr = diff(r1, r2) + if installed is not None: + fr += install_findings(installed, r1, r2) report_transition( - "THE RETREAT: roster -> flat, which is the back-out if " - "commissioning B goes wrong", r1, r2, fr) + "RETREAT", + "roster -> flat, which is the back-out if commissioning B " + "goes wrong", r1, r2, fr) findings += fr if args.round_trip: @@ -839,8 +1049,9 @@ def main(argv=None) -> int: q3 = snapshot_over("roundtrip-readvanced", after_opts, q2) fq = diff(q2, q3) + _round_trip_findings(worked, q3) report_transition( - "THE ROUND TRIP: roster -> flat -> roster, with a maintenance " - f"cycle completing at {worked!r} while retreated", + "ROUND-TRIP", + f"roster -> flat -> roster, with a maintenance cycle " + f"completing at {worked!r} while retreated", q2, q3, fq) findings += fq except Refused as err: @@ -855,11 +1066,17 @@ def main(argv=None) -> int: print(f" {len(findings)} FINDING(S) - this transition is not clean") for f in findings: print(f" ⚠️ {f}") - print("=" * 78) - return EXIT_FINDING - print(" NO FINDINGS in the rehearsed transitions") + else: + print(" NO FINDINGS in the rehearsed transitions") + # ⚠️ ON THE EXIT LINE, NOT ONLY IN THE HEADER. A report is skimmed from the + # bottom. + if degraded: + print(" ⚠️ DEGRADED RUN (--no-live-entities): the orphan check was " + "NOT graded against") + print(" the install's own entity list. This result does not cover " + "orphaned entities.") print("=" * 78) - return EXIT_OK + return EXIT_FINDING if findings else EXIT_OK if __name__ == "__main__": -- 2.54.0 From 9c1a9e0f178eb67d9f626152318f79dce053fe68 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Tue, 8 Sep 2026 14:23:35 +0200 Subject: [PATCH 6/7] ARCH-02: M7's evidence was filtered out by its own finding kind The mutation battery prints the distinct findings each mutation caused, matched on the finding's KIND. The pattern's character class was `[A-Z -]`, which has no underscore - so `UNIQUE_ID DRIFT:` did not match, and M7, the mutation that renames `mqtt.BASE`, printed its verdict with no evidence under it at all. A filter that silently drops the very finding a mutation exists to produce is the same defect as the rest of this card, at display scale. No behaviour change: M7 was red before and is red now, and it now shows why. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/test_rehearse.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/goodwe_controller/test_rehearse.py b/goodwe_controller/test_rehearse.py index e68d0c6..7ae1ec2 100644 --- a/goodwe_controller/test_rehearse.py +++ b/goodwe_controller/test_rehearse.py @@ -493,7 +493,13 @@ for mid, control, what, edits, must_say in MUTATIONS: seen = [] for line in out.splitlines(): text = line.strip() - if text.startswith("⚠️ ") and re.match(r"⚠️ [A-Z][A-Z -]+[A-Z()]:", text): + # ⚠️ `_` IS IN THE CLASS. Without it `UNIQUE_ID DRIFT:` does not match + # its own finding kind, and M7 - the mutation that renames `mqtt.BASE` + # - printed its verdict with NO evidence under it. A filter that + # silently drops the very finding the mutation exists to produce is a + # small version of the same defect this file is about. + if text.startswith("⚠️ ") and re.match(r"⚠️ [A-Z][A-Z _-]+[A-Z()]:", + text): if text not in seen: seen.append(text) for text in seen: -- 2.54.0 From 22ca341459380f6e796f89901580ea2008a82007 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Tue, 8 Sep 2026 15:01:55 +0200 Subject: [PATCH 7/7] 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`. -- 2.54.0