diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index 69d125b..612931c 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -27,8 +27,11 @@ without a bump is a schema nobody reads.** reasoning are in `DECISIONS/CFG-04-inverter-roster.md`). It accepts the list, enforces per-entry types, refuses a missing `name` / `setpoint_entity` / `max_w` **by name**, and renders the option in the UI as a repeatable section. - It also answers **`Missing option 'inverters' in root`** for an options blob - that has no such key — which every existing install's does. `App.options` is + And the shipped `options:` block validates as it stands while the **same + block with only `inverters` removed** is answered with **`Missing option + 'inverters' in root`** — one key varied, so the refusal is about that key — + which is every options blob written before this version, since the key did not + exist when they were written. `App.options` is the shipped `options:` block deep-merged with the operator's stored one, so `inverters: []` in `config.yaml` is the difference between an upgrade nobody notices and an add-on that cannot start with a schema pinned to the version diff --git a/goodwe_controller/DECISIONS/CFG-04-inverter-roster.md b/goodwe_controller/DECISIONS/CFG-04-inverter-roster.md index 2971fca..097f632 100644 --- a/goodwe_controller/DECISIONS/CFG-04-inverter-roster.md +++ b/goodwe_controller/DECISIONS/CFG-04-inverter-roster.md @@ -42,6 +42,17 @@ not want to discover on a running site. ### What it printed, 2026-09-04 +⚠️ **This transcript was RE-RUN after review, against the same image tag, and +it is not the one this record first carried.** The first §B validated the +reconstructed live blob — a blob missing many required keys that have nothing +to do with this card — and quoted its refusal as evidence about `inverters`. +It never was: that refusal names whichever unrelated key the validator reaches +first, and it has printed `meter_freeze_s`, `p1_phase_export_entities` and +`sign_disagree_w` on three different runs. §B is now the isolated +counterfactual — the shipped `options:` block, and the same block with +`inverters` and nothing else removed — and it asserts on the KEY NAME in the +refusal. The mis-cited half is kept, relabelled, as §B2. See §4. + ``` App 'GoodWe RS485 Controller' uses deprecated 'arch' values: ['armv7'] Option 'inverters' does not exist in the schema for GoodWe RS485 Controller (local_goodwe_controller) @@ -50,10 +61,15 @@ Option 'inverters' does not exist in the schema for GoodWe RS485 Controller (loc schema declares 66 keys; inverters -> [{"name": "str", "setpoint_entity": "str", "max_w": "int(100,5000)", "soc_entity": "str?", "batt_entity": "str?", "batt_invert": "bool?"}] options ship inverters -> [] -== B. AC3: the live install upgrades into this version == - PASS the live blob RAW is REFUSED - this is the forward schema conflict, and the next line is what stops it happening - Missing option 'meter_freeze_s' in root in GoodWe RS485 Controller (local_goodwe_controller) - PASS the live blob MERGED with these defaults - what App.options actually is +== B. THE LOAD-BEARING ONE: an options blob with no `inverters` key == + PASS the shipped `options:` block validates AS IT STANDS - so anything refused on the next line is refused for the one key that was removed + PASS ...and the SAME block with ONLY `inverters` removed is REFUSED, BY NAME - which is every options blob written before 0.6.0, since the key did not exist when they were written + Missing option 'inverters' in root in GoodWe RS485 Controller (local_goodwe_controller) + +== B2. AC3: a reconstructed pre-0.6.0 blob upgrades into this version == + PASS the reconstructed raw blob is REFUSED - for an unrelated missing key, because the reconstruction is incomplete; see the refusal printed below and §B for the `inverters` measurement + Missing option 'sign_disagree_w' in root in GoodWe RS485 Controller (local_goodwe_controller) + PASS the same blob MERGED with these defaults - what App.options actually is inverters=[] setpoint_entity='number.kelder_goodwe_master_stage1_goodwe_setpoint_w' max_w=5000 == C. AC2: a two-inverter blob an operator would POST == @@ -77,6 +93,8 @@ Option 'inverters' does not exist in the schema for GoodWe RS485 Controller (loc ['arbitrage_enabled', 'arbitrage_soc_ceiling', 'arbitrage_soc_floor', 'auto_start', 'batt_entity', 'batt_invert'] PASS Supervisor's UI renderer turns it into a repeatable section rather than choking - an operator can add the second inverter in the Configuration tab, which the JSON-string alternative could not offer [{"name": "inverters", "type": "schema", "optional": true, "multiple": true, "schema": [{"name": "name", "required": true, "type": "string"}, {"name": "setpoint_entity", "required": true, "type": "string"}, {"name": "max_w", "lengthMin": 100.0, "lengthMax": 5000.0, "required": true, "type": "integer + +== all cases passed == ``` *(`arch: armv7` deprecation is pre-existing and unrelated.)* @@ -90,17 +108,20 @@ Option 'inverters' does not exist in the schema for GoodWe RS485 Controller (loc | A missing required key is refused **by name** | §D — *"Missing option 'name' in inverters"* | | A two-inverter blob validates | §C | | The operator can edit it in the UI | §E — `UiOptions` renders `{"type": "schema", "multiple": true}` | -| A downgrade is safe | §E — a schema without `inverters` **drops the key with a warning**; it is not the §B trap in reverse | +| The `inverters: []` default is what makes a pre-0.6.0 blob validate | §B — the shipped defaults **minus only that key** are refused with *"Missing option 'inverters' in root"*; unmodified they pass | +| A downgrade drops the key rather than refusing | §E — a schema without `inverters` **drops the key with a warning**; it is not the §B trap in reverse | ## 4. ⚠️ The one that decided the default: `inverters: []` -§B is the load-bearing measurement. `supervisor/apps/options.py`'s +§B is the load-bearing measurement, and it is an **isolated counterfactual**: +exactly one key differs between its two runs. `supervisor/apps/options.py`'s `_check_missing_options()` treats a nested list as **required** unless its element is an optional scalar — a list whose element is a dict is never -optional. So an options blob with no `inverters` key is answered with: +optional. So the shipped `options:` block validates as it stands, and the same +block with **only** `inverters` removed is answered with: ``` -Missing option 'inverters' in root +Missing option 'inverters' in root in GoodWe RS485 Controller (local_goodwe_controller) ``` The live install's stored options have no such key, because it did not exist @@ -112,8 +133,27 @@ version they cannot install. What makes it safe is that `App.options` is `deepmerge(config.yaml's options:, the operator's persisted options)`, so a -shipped `[]` fills the gap before validation ever runs. §B shows both halves: -the raw blob **refused**, the merged blob **accepted, with `inverters: []`**. +shipped `[]` fills the gap before validation ever runs. §B shows the halves that +carry the claim — the defaults **minus this one key refused, by this key's +name**, and the unmodified defaults **accepted** — and §B2 shows the merge doing +its job on a reconstructed pre-0.6.0 blob. + +⚠️ **What §B2 does NOT show, and what this record used to claim it did.** §B2's +`LIVE` dict is an incomplete reconstruction: it carries the keys `HANDOVER.md` +and `docs/deploy-rollback.md` record and no others, so it is missing many +required keys unrelated to this card. Its RAW half is therefore refused for +whichever of *those* the validator reaches first — three separate runs have +printed `meter_freeze_s`, `p1_phase_export_entities` and `sign_disagree_w` — +and the earlier version of this section quoted that refusal as if it named +`inverters`. It never did. The string above was produced by §B, where the +`inverters` key is the only thing that varies. + +The probe's assertion on that line is on the **key name in the refusal**, not on +"a refusal happened". With the weaker assertion §B would have gone on printing +PASS after someone deleted `inverters: []` from `config.yaml` — blind to the one +regression it is the committed evidence for. It is now checked both ways: with +the default deleted the probe prints `2 CASE(S) FAILED` and exits 1; restored, +it prints `all cases passed` and exits 0. The merge strategy was checked too, because it is the other way this could have gone wrong: `_OPTIONS_MERGER` is `type_strategies=[(dict, ["merge"])]` with an @@ -169,5 +209,5 @@ It loses on three things: - **SPIKE-04 measuring inverter B.** It supplies a real `max_w` for the second entry. It does not change the shape. - **A second `Maintenance` FSM.** `validate.py` rule 6 grades - `maintenance_charge_w` against `caps[0]` because `Controller.maint_unit` is - `inverters[0]`. When that stops being true, that line moves with it. + `maintenance_charge_w` against the first entry of `unit_caps_indexed()` + because `Controller.maint_unit` is `inverters[0]`. When that stops being true, that line moves with it. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 1f9990b..99547f3 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -116,8 +116,20 @@ have. ⚠️ **Update the add-on first, then add the roster.** Home Assistant validates the options you save against the schema of the version that is *installed*, so `inverters` cannot be set before 0.6.0 is installed. The order is: update, then -configure. Going the other way — downgrading with a roster configured — is safe: -the older schema drops the key with a warning rather than refusing. +configure. + +⚠️ **Going the other way — downgrading with a roster configured — is recoverable +but NOT uneventful, and the add-on will not start until you refill a field.** +The *schema* side is fine: the older version's schema has no `inverters` key, so +it drops the key with a warning rather than refusing the whole blob (measured — +`DECISIONS/CFG-04-inverter-roster.md` §E). The *add-on* side is not. Rule 9(e) +requires the flat `setpoint_entity` be **cleared** when a roster is declared, so +a config that downgrades cleanly is by construction a config whose flat +actuator is blank — and the older version has nothing else to write to. It stops +at startup validation with a refusal naming `setpoint_entity`, which is loud and +in the log rather than silent, but it is an outage until someone puts the +single-inverter entity back in that field. **Before downgrading, write down the +`setpoint_entity` of the unit you want to keep running.** ### P1 meter ingestion diff --git a/goodwe_controller/MUTATION.md b/goodwe_controller/MUTATION.md index 94d991a..13dad66 100644 --- a/goodwe_controller/MUTATION.md +++ b/goodwe_controller/MUTATION.md @@ -135,9 +135,11 @@ manufactures them from a **fixture**: the check runs, the assertion is evaluated, the sentence above it is true — and the numbers it compares are arranged so that the failure it describes could not have changed the answer. -It is not hypothetical and it is not rare. Five instances in two weeks; the -first three were found by review rather than by a red line, and the last two - -CFG-04's - by an author's own mutation battery, which is the cheaper place: +It is not hypothetical and it is not rare. Six instances in two weeks; three +were found by review, two - CFG-04's fixture pair - by an author's own mutation +battery, which is the cheaper place, and the sixth by review again, in the one +place nobody had thought to point the rule at: not a check in the suite, but a +COMMITTED EVIDENCE ARTIFACT four documents cite. * **ARB-03** shipped a call-site scan asserting that every `claims.append` in `cycle()` routes through `actuator_of()`. Its own parse **saw 3 of the 8 @@ -165,6 +167,26 @@ CFG-04's - by an author's own mutation battery, which is the cheaper place: than against its behaviour. It now drives the shipped `run_control()` on a two-inverter config, **under a deadline**, because the mutation's real symptom is that the controller enters its loop and never returns. +* **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 + quoted it as answering *"Missing option 'inverters' in root"*. §B validated + the reconstructed LIVE options blob, which is missing many required keys + unrelated to this card, so its refusal named whichever of THOSE the validator + reached first — `meter_freeze_s`, `p1_phase_export_entities` and + `sign_disagree_w` on three different runs. The quoted string was never + produced by the instrument. And the assertion was a bare **`not ok`**: delete + `inverters: []` from `config.yaml` and §B went on printing **PASS**, blind to + the exact regression it was cited for. §B is now an isolated counterfactual — + the shipped defaults, and the same defaults minus that one key — asserting on + the **key name in the refusal**; the mis-cited blob survives, relabelled, as + §B2. Re-run with the default deleted it prints `2 CASE(S) FAILED` and exits 1. + + ⚠️ **A committed transcript is a check, and it decays like one.** Nothing in + this document had been pointed at `tools/`, because the artifacts there are + filed as *measurements* rather than as *checks* — and a measurement whose + assertion cannot fail is exactly the thing this section is about. Two habits + follow, and both are habit 5 and 6 below. Note what these have in common with a masked rung: the mutation *runs*, and the suite is *honestly* green. Nothing in a purge, a fingerprint or a disassembly @@ -177,7 +199,7 @@ check was handed. > **Any check asserting "X differs from Y" must be run once with X and Y > actually made different.** -Three cheap habits fall out of it, and all three are now used in this repository: +Cheap habits fall out of it, and all of them are now used in this repository: 1. **Write the PRECONDITION as its own check.** `test_peakguard.py` 10g asserts that A and B have different ratings *before* it asserts that B's bound is @@ -202,6 +224,23 @@ Three cheap habits fall out of it, and all three are now used in this repository 5. **A check whose failure mode is a HANG needs a deadline.** Otherwise the mutation that defeats it produces a CI timeout with no name attached, which reads like flaky infrastructure and gets retried rather than read. +6. **Assert on the CONTENT of a refusal, not on the fact that one happened.** + `not ok` is satisfied by any error at all, including one about a different + key on a different blob. `"Missing option 'inverters' in root" in out` goes + red the moment the refusal stops being about the thing it is cited for. This + is habit 2 in the negative direction: a refusal, like a difference, has to be + asserted against a literal. +7. **Vary ONE thing in a counterfactual, and vary it from an ACCEPTED + baseline.** `supervisor_shape_probe.py` §B validates the shipped defaults + first, precisely so the refusal on the next line has nowhere else to come + from. Without the accepted baseline the second line is a refusal of unknown + provenance, which is what §B used to print. +8. **A committed transcript is a check.** Anything under `tools/` that a + document quotes is subject to every rule above, and to the mutation battery: + break the property, re-run the instrument, and confirm the transcript goes + red. `inverters: []` was deleted from `config.yaml` and the probe re-run + against `ghcr.io/home-assistant/amd64-hassio-supervisor:2026.08.0` for + exactly that reason. And the mutation-table consequence: **a variant is not covered because a near-miss of it is.** DEV-02's table listed the *actuator* variant of the diff --git a/goodwe_controller/app/validate.py b/goodwe_controller/app/validate.py index 6361d2c..13d2e9a 100644 --- a/goodwe_controller/app/validate.py +++ b/goodwe_controller/app/validate.py @@ -352,17 +352,35 @@ def unit_caps(opts: dict) -> list[float]: (`inverter.roster_entries()`, not a second reading of the option), and fall back to the flat `max_w` only where that roster is the flat one too. """ + return [cap for _, cap in unit_caps_indexed(opts)] + + +def unit_caps_indexed(opts: dict) -> list[tuple[int, float]]: + """`unit_caps()`, but each cap carries the ROSTER INDEX it came from. + + ⚠️ WHY THE INDEX HAS TO TRAVEL WITH THE NUMBER. This function SKIPS entries + with no usable `max_w`, so position i in the caps list is not position i in + the roster the moment one entry is malformed. Rules 5 and 6 used to feed a + caps-list index straight into `_unit_label()`, which indexes `entries` - so + on a roster whose first entry has a junk `max_w`, the refusal named the + WRONG INVERTER. An operator told to lower a number on `inverter-a` when the + number is on `inverter-b` is being sent to the wrong device by the very + message that exists to stop that. Rule 9(d) already refuses such a config so + the two lists cannot desynchronise in shipped use - but "unreachable" is a + claim about today's rule ordering, not a property of this function, and + `validate_startup_config()` is also `sim/inject.py`'s entry point. + """ entries = roster_entries(opts) if not entries: cap = _number(opts, "max_w", 2000.0) - return [] if cap is None else [cap] - caps: list[float] = [] - for entry in entries: + return [] if cap is None else [(0, cap)] + caps: list[tuple[int, float]] = [] + for index, entry in enumerate(entries): if not isinstance(entry, dict): continue cap = _number(entry, "max_w", None) if cap is not None: - caps.append(cap) + caps.append((index, cap)) return caps @@ -699,12 +717,19 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] # nothing about it. At one inverter `unit_caps()` is `[max_w]` and this is # the identical comparison it always was. sign_w = _number(opts, "sign_disagree_w", 300.0) - caps = unit_caps(opts) + # ⚠️ (index, cap) PAIRS, NOT BARE CAPS. `unit_caps()` skips entries with no + # usable `max_w`, so `caps.index(weakest)` was an index into the CAPS list + # being handed to `_unit_label()`, which indexes the ROSTER - and the message + # named the wrong inverter as soon as the two lengths differed. The index + # travels with the number now, so the name is the name of the unit the + # number came off. + caps_ix = unit_caps_indexed(opts) + caps = [cap for _, cap in caps_ix] weakest = min(caps) if caps else None if sign_w is None: problems.append(f"sign_disagree_w {opts.get('sign_disagree_w')!r} is not a number") elif weakest is not None and weakest > 0 and sign_w >= weakest: - where = _unit_label(opts, caps.index(weakest)) + where = _unit_label(opts, min(caps_ix, key=lambda p: p[1])[0]) problems.append( f"sign_disagree_w is {sign_w:.0f} W but max_w{where} is " f"{weakest:.0f} W, so " @@ -756,8 +781,12 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] # being `inverters[0]`, this line moves with it. charge_w = _number(opts, "maintenance_charge_w", float(MAINT_OPTION_DEFAULTS["maintenance_charge_w"])) - maint_cap = caps[0] if caps else None - maint_where = _unit_label(opts, 0) + # ⚠️ THE SAME PAIRING AS RULE 5, FOR THE SAME REASON. `caps[0]` is the first + # unit WITH A USABLE CAP, which is `inverters[0]` only while every earlier + # entry has one; `_unit_label(opts, 0)` was hard-wired to `inverters[0]` + # regardless. Both come off the one pair now. + maint_index, maint_cap = caps_ix[0] if caps_ix else (0, None) + maint_where = _unit_label(opts, maint_index) if charge_w is None: problems.append( f"maintenance_charge_w {opts.get('maintenance_charge_w')!r} is not a number") diff --git a/goodwe_controller/test_inverter.py b/goodwe_controller/test_inverter.py index e0a7f54..4f6f125 100644 --- a/goodwe_controller/test_inverter.py +++ b/goodwe_controller/test_inverter.py @@ -1383,8 +1383,11 @@ check("...and the key ABSENT and the key EMPTY are the same configuration, not " # ⚠️ AND THE SHIPPED DEFAULT IS NOT COSMETIC - IT IS WHAT MAKES THE UPGRADE # SURVIVABLE, AND IT WAS MEASURED. Supervisor 2026.08.0's own AppOptions # validator - the version this site runs, run against this very file by -# tools/supervisor_shape_probe.py - answers `Missing option 'inverters' in root` -# for an options blob that has no `inverters` key, and `App.test_update_schema()` +# tools/supervisor_shape_probe.py section B - answers `Missing option +# 'inverters' in root` for the shipped `options:` block with ONLY this key +# removed (one key varied, so the refusal is about this key; section B2's +# reconstructed blob is refused for unrelated missing keys and is NOT this +# measurement), and `App.test_update_schema()` # turns that into "new schema is not compatible" at UPDATE time. The Supervisor # merges `options:` into the operator's stored blob before validating, so this # one line is the difference between an upgrade nobody notices and an add-on @@ -1448,7 +1451,8 @@ print("10. CFG-04: rule 9, and SAFETY-28's hazard now expressible for the " "first time") from app.validate import ( # noqa: E402 - actuator_writers, unit_caps, validate_startup_config, validate_unit_ranges, + actuator_writers, unit_caps, unit_caps_indexed, validate_startup_config, + validate_unit_ranges, _unit_label, ) @@ -1558,6 +1562,52 @@ check("...and at one inverter `unit_caps()` IS `[max_w]`, so the rule is the " "identical comparison it always was and no existing install changes " "verdict", unit_caps(_opts()) == [A_RATING]) +# --- rules 5 and 6 named the WRONG INVERTER when the two lists desynchronised - +# ⚠️ WHAT THE DEFECT WAS. `unit_caps()` SKIPS an entry with no usable `max_w`, +# so caps position i stopped being roster position i the moment one entry was +# malformed. Rule 5 did `_unit_label(opts, caps.index(weakest))` and rule 6 did +# `caps[0]` beside a hard-wired `_unit_label(opts, 0)` - both feeding a CAPS +# index to a function that indexes the ROSTER. The number in the message was +# right and the NAME beside it was another inverter's: an operator sent to the +# wrong device by the message that exists to stop exactly that. +# +# It is latent, not live - rule 9(d) refuses a junk `max_w` in the same pass, so +# the operator sees both refusals. But "unreachable" was a property of today's +# rule ordering rather than of the code, and `validate_startup_config()` is also +# `sim/inject.py`'s entry point. The index travels with the cap now. +_skew = _opts(setpoint_entity="", sign_disagree_w=1500, maintenance_charge_w=3000, + maintenance_discharge_w=1000, inverters=[ + dict(name="inverter-a", setpoint_entity="number.sp_a", + max_w="junk"), # SKIPPED by unit_caps() + dict(name="inverter-b", setpoint_entity="number.sp_b", max_w=2000), + dict(name="inverter-c", setpoint_entity="number.sp_c", max_w=1000)]) +_p_skew = validate_startup_config(_skew) +print(f" caps {unit_caps(_skew)} but pairs {unit_caps_indexed(_skew)} - " + f"roster is {len(_skew['inverters'])} long") +check("the fixture actually desynchronises the two lists, or the two checks " + "below prove nothing: one entry is skipped, so caps is SHORTER than the " + "roster and every caps index is off by one", + unit_caps(_skew) == [2000.0, 1000.0] + and unit_caps_indexed(_skew) == [(1, 2000.0), (2, 1000.0)]) +check("⚠️ RULE 5 NAMES THE UNIT THE NUMBER CAME OFF. The weakest cap is " + "inverter-c's 1000 W; it sits at caps index 1, and `_unit_label(opts, 1)` " + "is inverter-B. The refusal says inverter-c", + _why(_p_skew, "max_w (inverter-c) is 1000 W") + and not _why(_p_skew, "max_w (inverter-b) is 1000 W")) +check("⚠️ RULE 6 TOO. `caps[0]` is the first entry WITH a usable cap - " + "inverter-b - while `_unit_label(opts, 0)` was hard-wired to inverter-a, " + "so the maintenance refusal quoted B's rating under A's name", + _why(_p_skew, "max_w (inverter-b) is 2000 W") + and not _why(_p_skew, "max_w (inverter-a) is 2000 W")) +check("...and the counterfactual, so this is not a check that would pass on a " + "validator that named nothing: the OLD expressions still evaluate, on " + "this same config, to the wrong names", + _unit_label(_skew, unit_caps(_skew).index(1000.0)) == " (inverter-b)" + and _unit_label(_skew, 0) == " (inverter-a)") +check("...and rule 9(d) refuses this config anyway - which is WHY the defect " + "was latent, and is recorded rather than used as a reason not to fix it", + _why(_p_skew, "inverter-a")) + # --- validate_unit_ranges: rule 2, per unit, against ITS OWN device ----------- _c10, _ = built(_TWO_OPTS) _c10.inverters[0].dev_min, _c10.inverters[0].dev_max = -A_RATING, A_RATING @@ -2038,7 +2088,12 @@ MUTATIONS = [ "this suite at all - Supervisor answers `Missing option 'inverters' in " "root` and the add-on will not start after the upgrade - which is why the " "check is on the file and the measurement is in " - "DECISIONS/CFG-04-inverter-roster.md"), + "DECISIONS/CFG-04-inverter-roster.md §B. ⚠️ THE PROBE ITSELF WAS " + "REGRESSION-BLIND TO THIS MUTATION until the review: its section B " + "asserted `not ok` on a DIFFERENT blob, so deleting the default left it " + "printing PASS. Section B now varies one key and asserts on the KEY NAME " + "in the refusal, and the mutation was re-run against the real image: " + "`2 CASE(S) FAILED`, exit 1"), ("validate.py allowing two roster entries with the SAME name", "part 10: 'two entries with the SAME name are refused' - one Store " "namespace for two units is DEV-02's defect rebuilt in YAML"), diff --git a/goodwe_controller/tools/supervisor_shape_probe.py b/goodwe_controller/tools/supervisor_shape_probe.py index f232b8e..49d7356 100644 --- a/goodwe_controller/tools/supervisor_shape_probe.py +++ b/goodwe_controller/tools/supervisor_shape_probe.py @@ -30,8 +30,15 @@ WHAT IT ANSWERED, 2026-09-04, on 2026.08.0 - the full output is in DECISIONS/CFG-04-inverter-roster.md: A. The manifest, list-of-objects schema and all, is ACCEPTED. - B. The live install's options blob MERGED WITH THIS FILE'S DEFAULTS validates, - and the same blob RAW does not - which is why `inverters: []` is shipped. + B. The shipped `options:` block validates as it stands, and the SAME block + with ONLY `inverters` removed is refused BY NAME - `Missing option + 'inverters' in root`. That is the one measurement the `inverters: []` + default rests on, and it is an ISOLATED counterfactual: exactly one key + varies between the two runs, so the refusal cannot be about anything else. + B2. A reconstructed pre-0.6.0 options blob, merged with those defaults, + validates. Its RAW form is refused - but for whichever required key the + reconstruction happens to lack FIRST, which is an artefact of the + reconstruction and NOT evidence about `inverters`. B is that evidence. C. A two-inverter blob validates, per-entry types enforced. D. An entry missing `name`, `setpoint_entity` or `max_w` is refused BY NAME, and a JSON string where the list belongs is refused outright. @@ -59,7 +66,9 @@ except vol.Invalid as e: schema, defaults = cfg["schema"], cfg["options"] print(" schema declares", len(schema), "keys; inverters ->", json.dumps(schema["inverters"])) -print(" options ship inverters ->", json.dumps(defaults["inverters"])) +# .get, not [], deliberately: if someone deletes the `inverters: []` default +# this line must not crash the probe before section B gets to REFUSE it. +print(" options ship inverters ->", json.dumps(defaults.get("inverters"))) def validate(options): v = vol.Schema(vol.All(dict, AppOptions(None, schema, cfg["name"], "local_goodwe_controller"))) @@ -68,9 +77,12 @@ def validate(options): except vol.Invalid as e: return False, str(e) +FAILED = [] + def show(t, ok, d=""): print(f" {'PASS' if ok else 'FAIL'} {t}") if d: print(" " + str(d)[:300]) + if not ok: FAILED.append(t) # The live install's persisted options, reconstructed from HANDOVER.md §1 and # docs/deploy-rollback.md §0/§B.5. No `inverters` key: it did not exist. @@ -87,17 +99,48 @@ LIVE = { "maintenance_enabled": False, "estop_fitted": True, "log_level": "info", } -print("\n== B. AC3: the live install upgrades into this version ==") +print("\n== B. THE LOAD-BEARING ONE: an options blob with no `inverters` key ==") +# ⚠️ THIS IS AN ISOLATED COUNTERFACTUAL AND IT HAS TO STAY ISOLATED. The claim +# the `inverters: []` default rests on is about ONE key, so exactly ONE key +# varies between these two runs: the full shipped `options:` block, and the same +# block with `inverters` removed and nothing else touched. Feeding the validator +# some other, INCOMPLETE blob would also produce a refusal - but about whichever +# required key that blob lacks first, which is a different measurement wearing +# this one's label. §B2 below is that other blob, and it is labelled as such. +ok, out = validate(dict(defaults)) +show("the shipped `options:` block validates AS IT STANDS - so anything refused " + "on the next line is refused for the one key that was removed", ok, + "" if ok else out) +_no_inv = {k: v for k, v in defaults.items() if k != "inverters"} +ok_ni, out_ni = validate(_no_inv) +# ⚠️ THE ASSERTION IS ON THE KEY NAME, NOT ON `not ok`, AND THAT IS THE POINT. +# A refusal is only evidence for THIS key if it NAMES this key. Asserted as a +# bare `not ok`, this line would go on printing PASS after someone deleted +# `inverters: []` from config.yaml - blind to the exact regression it is the +# committed evidence for. Asserted on the string, it goes red: with the default +# gone the line above fails too, because then nothing distinguishes the two runs. +_names_it = (not ok_ni) and "Missing option 'inverters' in root" in str(out_ni) +show("...and the SAME block with ONLY `inverters` removed is REFUSED, BY NAME - " + "which is every options blob written before 0.6.0, since the key did not " + "exist when they were written", _names_it, out_ni) + +print("\n== B2. AC3: a reconstructed pre-0.6.0 blob upgrades into this version ==") +# ⚠️ WHAT THIS SECTION IS AND IS NOT. LIVE is an INCOMPLETE reconstruction - it +# carries the keys HANDOVER.md and deploy-rollback.md record and no others, so +# it is missing many required keys that have nothing to do with this card. Its +# raw refusal is therefore about whichever of THOSE the validator reaches first +# (it has printed `meter_freeze_s` and `p1_phase_export_entities` on different +# runs), NOT about `inverters`. It is kept because the MERGED half is a real +# demonstration that `deepmerge(options:, stored)` is what App.options is; the +# raw half is printed only so the merge has something to be contrasted with. +# The `inverters` measurement is §B, where one key and only one key varies. ok, out = validate(LIVE) -# ⚠️ EXPECTED TO BE REFUSED, AND THAT IS THE WHOLE POINT OF THE LINE. This is -# what `persist` holds on its own; the Supervisor never validates that, it -# validates it merged with `options:` below. Printed so the trap is visible -# rather than described. -show("the live blob RAW is REFUSED - this is the forward schema conflict, and " - "the next line is what stops it happening", not ok, out) +show("the reconstructed raw blob is REFUSED - for an unrelated missing key, " + "because the reconstruction is incomplete; see the refusal printed below " + "and §B for the `inverters` measurement", not ok, out) merged = MERGER.merge(copy.deepcopy(defaults), copy.deepcopy(LIVE)) ok, out = validate(merged) -show("the live blob MERGED with these defaults - what App.options actually is", ok, +show("the same blob MERGED with these defaults - what App.options actually is", ok, f"inverters={out.get('inverters')!r} setpoint_entity={out.get('setpoint_entity')!r} " f"max_w={out.get('max_w')!r}" if ok else out) @@ -153,3 +196,15 @@ try: bool(_ui) and _ui[0].get("multiple") is True, json.dumps(_ui)) except Exception as e: # noqa: BLE001 show("Supervisor's UI renderer survives the shape", False, f"{type(e).__name__}: {e}") + + +# ⚠️ THE EXIT CODE IS PART OF THE INSTRUMENT. Without it the probe prints FAIL +# and still succeeds, and a transcript is only evidence if a wrong answer is +# distinguishable from a right one by something other than reading it. +print() +if FAILED: + print(f"== {len(FAILED)} CASE(S) FAILED ==") + for t in FAILED: + print(" - " + t) + sys.exit(1) +print("== all cases passed ==")