diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index df0a58d..83e9385 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -95,6 +95,7 @@ what happens when nobody does. | | | |---|---| | _this branch_ | UX-02 - a configuration page in the ingress panel, where the validator's refusals can be seen | +| _this branch_ | UX-04 - UX-01's three unmet acceptance criteria: one withdrawn, one built, one re-attributed | ⚠️ **UX-02 ADDS A SECOND PLACE TO CONFIGURE THIS ADD-ON, AND NO NEW WAY TO COMMAND THE INVERTER.** The ingress panel gains a Configuration page beside @@ -131,6 +132,47 @@ already declared, and the panel was already live. the Configuration tab **can** be removed - at the cost of all 71 options, which is why it stays. `DECISIONS/UX-02-two-config-surfaces.md` carries both. +⚠️ **UX-04 CHANGES NO BEHAVIOUR, NO SCHEMA AND NO DEFAULT.** It adds one check +file, three assertions to a probe, and a decisions record; `config.yaml`, +`translations/` and every module under `app/` are byte-identical to `6e7b233`. +It closes out [[UX-01]], which shipped in **0.7.0 and is live**, with three +criteria that were never met: + +* **AC 3 - "the visible count drops to ~15" - is WITHDRAWN as unsatisfiable.** + `options:` and `schema:` declare the same 71 keys, so the set HA's *show + unused optional configuration options* toggle can hide is **empty**, and + Supervisor's own `UiOptions` marks **59 of the 71 required** - the frontend + shows those unconditionally, so 59 is the floor even with `options:` emptied + altogether. The target was ~4x below what any manifest can reach. ⚠️ And the + 12 hides that *are* expressible would be bought with the deep-merge: a key + absent from `options:` has no default for the Supervisor to merge into a + 0.7.0 box's stored blob, which is exactly the `delivery_hold_s` mechanism + that makes this release's upgrade safe rather than exit 78. The count was + **printed** by `tools/supervisor_shape_probe.py` §F from the day it was + written and never graded, which is how this passed review; it is now three + assertions, and removing one key from `options:` turns two of them red. +* **AC 4's third clause - the AST check for stray in-code literal fallbacks - + is BUILT**, as `test_defaults_pin.py`. It discovers the six + `*_OPTION_DEFAULTS` mirrors from the tree rather than naming them, grades all + 30 mirrored keys against `config.yaml`, and freezes the CFG-03 backlog at + exactly today's **27 (module, key) pairs** so it can only shrink. It found + one previously unreported divergence: `main.py` answers a missing + `target_grid_w` with **0** where `config.yaml` ships **-10** - unreachable on + a Supervisor install, recorded rather than corrected. Its red direction runs + on every invocation, and a literal planted in the real tree was shown red. +* **AC 5 / the CFG-04 double-literal `max_w` hole is closed, by + [[UX-02]]'s `bcee235` and not by UX-01's `fef6ef2`.** Re-measured on this + branch: the `2000 -> 2500` triple mutation is **RED at `test_selfconfig.py`**, + where it survived 21/21 green on 0.7.0. The attribution is corrected in + `DECISIONS/UX-04-the-configuration-tab-and-the-defaults-pin.md`; nothing was + re-done. + +The 8 per-entry `inverters:` translations **stay**. They are shipped, validated +and measured *not* to reach the Supervisor tab - `ha-selector-object` labels +those fields `fields[name].label || name` and the converter sets no `label` - +and UX-02's ingress panel is the surface that can render them. A written +decision, not a change. + ## 0.8.0 ⚠️ **A CONFIG THAT STARTED ON 0.7.0 CAN BE REFUSED ON 0.8.0, AND THAT IS WHY diff --git a/goodwe_controller/DECISIONS/UX-04-the-configuration-tab-and-the-defaults-pin.md b/goodwe_controller/DECISIONS/UX-04-the-configuration-tab-and-the-defaults-pin.md new file mode 100644 index 0000000..ec3163f --- /dev/null +++ b/goodwe_controller/DECISIONS/UX-04-the-configuration-tab-and-the-defaults-pin.md @@ -0,0 +1,379 @@ +# UX-04 — what the Configuration tab renders, and the pin that was never built + +**Status: DECIDED 2026-09-07.** Three decisions and one confirmation, in the +order [[UX-01]]'s unmet acceptance criteria are numbered: + +1. **UX-01 AC 3 (hide the optional fields behind HA's toggle) is WITHDRAWN as + unsatisfiable**, not deferred. The target was arithmetically unreachable, and + the part of it that *was* reachable would be bought with the upgrade path. + The measurement is now **graded** in `tools/supervisor_shape_probe.py` §F + rather than printed, so the withdrawal cannot be quietly reversed. +2. **UX-01 AC 4's third clause is now BUILT**: `test_defaults_pin.py`, an AST + check, with its red direction executed on every run. +3. **UX-01 AC 5 / the CFG-04 double-literal hole is CONFIRMED CLOSED on + `release/1.0`, by [[UX-02]]'s `bcee235`, not by UX-01's `fef6ef2`.** The + attribution is corrected here; no work was re-done. +4. **The 8 per-entry `inverters:` translations STAY.** They do not reach the + Supervisor tab and cannot be made to; [[UX-02]]'s ingress panel is where they + become visible. + +Filed with UX-04. Card `6a9ec94617e9c48ed74ca5dd`. Every number below was +measured on `release/1.0` at `6e7b233` on 2026-09-07, against +`ghcr.io/home-assistant/amd64-hassio-supervisor:2026.08.0` — the version +`ha info` reports on the live install — and against the 26-file check suite in +`goodwe-addon-checks:1`. Nothing here contacted the live Supervisor at +192.168.2.158, posted options, or restarted anything. + +--- + +## 1. Hiding: withdrawn, and why that is the *safe* answer + +### 1.1 What AC 3 claimed + +> "The visible count drops to ~15, with the rest behind HA's *show unused +> optional configuration options* toggle." + +### 1.2 What the tab actually renders + +`config.yaml` declares **71 keys under `options:` and the same 71 under +`schema:`**. The set that HA's toggle is *able* to hide — schema-only keys — is +therefore **empty**: + +``` + hideable = set(schema) - set(options) -> [] (0 of 71) +``` + +On the deployed 0.7.0 tree the same measurement is 70/70. Nothing was hidden +before UX-01, nothing is hidden after it, and no line of `fef6ef2` moved a key +either way. AC 3 is not partially met; it is unstarted. + +### 1.3 The rule that decides it, replayed rather than paraphrased + +From the frontend bundle shipped **in this image**, +`api/panel/frontend_latest/5680.*.js` (frontend 20250925.1): + +```js +_filteredSchema = (opts, schema) => schema.filter(e => e.name in opts || e.required) +``` + +`opts` is the add-on's options **as Supervisor serves them** — the operator's +stored blob after `options:` has been deep-merged into it. So a field is hidden +only when it is **both** absent from `options:` **and** not `required`. The +probe now runs that filter on this manifest rather than quoting it. + +### 1.4 Reason one: the target was never reachable + +Supervisor's own `UiOptions` marks `required` on **59** of the 71 fields; 12 are +optional. The filter above shows a required field unconditionally. So the +**floor** — what the tab renders with `options:` emptied *altogether*, which is +the most hiding that is even expressible — is 59 fields. + +| `options:` block | fields rendered | +|---|---| +| as shipped (71 keys) | **71** | +| emptied entirely (0 keys) | **59** | +| AC 3's target | ~15 | + +AC 3 asked for a number roughly four times below the floor. No arrangement of +`options:` reaches it. The criterion could not have been met by any +implementation; it was mis-specified, and the honest disposal of a +mis-specified criterion is withdrawal, not a partial gesture toward it. + +### 1.5 Reason two: the 12 hides that *are* expressible cost the deep-merge + +Moving a key out of `options:` is not free. `App.options` is +`deepmerge(config.yaml defaults, the stored user options)`, so **a key absent +from `options:` has no default for the Supervisor to merge**. + +That is exactly the mechanism the 0.9.0 upgrade rests on. From `CHANGELOG.md`, +the pre-deploy check run against this release on 2026-09-07: + +> The reference install's stored blob has **70 keys, not 71**. +> `delivery_hold_s` arrived with SAFETY-42 in 0.8.0, so an install running +> 0.7.0 has never stored it — and it is REQUIRED in the schema, no trailing +> `?`, which is the shape that becomes **exit 78**. It resolves to its +> `options:` default of **185** because the Supervisor deep-merges. + +Hiding a key is done by taking it *out* of `options:` and marking it `?`. That +is, precisely, the edit that removes a key's merge default — the thing standing +between a 0.7.0 box and exit 78 on upgrade. It would be traded for at most 12 +fewer rows on a form, on a site where the add-on is holding a real house's grid +exchange at zero. + +### 1.6 Reason three: it would promote 27 unpinned literals to load-bearing + +And the cost compounds with §2 of this document. `test_defaults_pin.py` part 4 +ledgers **27 (module, key) pairs** in `app/` that still answer a missing option +with an in-code literal. Today those literals are unreachable on a Supervisor +install, because the deep-merge guarantees every `options:` key is present in +the served blob. Remove keys from `options:` to hide them and those literals +stop being dead code: they become **the actual source of the defaults on a live +install** — the CFG-03 defect the same card's AC 4 exists to police, made +load-bearing by the card's own AC 3. + +### 1.7 What now grades this + +`tools/supervisor_shape_probe.py` §F. The count was printed there from the day +it was written — *"the Configuration tab renders 71 fields"* — and the probe +exited 0 through UX-01's whole review while displaying the number that +contradicted the criterion it was evidence for. **A measurement that is +displayed but not asserted is how this passed review.** Three assertions +replace the bare print: + +``` +PASS every one of the 71 `schema:` keys is ALSO in `options:`, so schema-only + - the only set HA's toggle can hide - is EMPTY. Hideable: [] +PASS ...so replaying the frontend's own _filteredSchema on the served options + hides NOTHING: 71 of 71 fields render +PASS ...and with `options:` EMPTIED ENTIRELY - the maximum hiding that is + expressible - the tab still renders 59 required fields. THAT is the + floor, and AC 3 asked for ~15. +``` + +Every number in them is computed from the manifest and from Supervisor's own +`required` flag; none is typed. **The red direction was run**: deleting one key +(`price_feed_url`) from `options:` and nothing else turns the first two to FAIL +— `Hideable: ['price_feed_url']`, `70 of 71 fields render` — and the probe +exits 1. The counterfactual is isolated: one key varied, nothing else. + +So if a future editor empties `options:` to shorten the form, this goes red and +somebody has to read §1.5 before merging. + +--- + +## 2. The defaults pin: `test_defaults_pin.py` + +### 2.1 What AC 4 asked for and what shipped + +AC 4 has three clauses. + +| clause | state before UX-04 | +|---|---| +| "every `schema:` key absent from `options:` has exactly one defaults-table entry" | **vacuous** — that set is empty (§1.2), so the clause is true of any tree, including one with no defaults at all | +| "the table changed → red" | **works**, six times over, once per mirror in its own suite | +| "no module carries its own literal fallback for a schema default — an AST check, not a grep" | **never built** | + +The only literal scan in the repository before this card is +`test_validate.py:1025`: a `re.findall` restricted to eight `maintenance_*` +keys across three named files. It is a grep, it predates UX-01, and it was not +written for this. + +### 2.2 What an "in-code literal fallback" is, structurally + +A call in `app/` taking a string literal that names a `schema:` key, with a +constant in the next positional slot: + +```python +opts.get("max_w", 2000) _number(opts, "max_w", 2000.0) +``` + +Found by walking `ast.Call` nodes — so a multi-line call and a renamed `opts` +are found, and the same text inside a comment or docstring is not. Two +exclusions, both structural and both argued at the site: + +* **`getattr(obj, "max_w", 0.0)`** reads an attribute off an object, not an + option off a blob. Excluded by callee name. +* **the empty string.** `opts.get("static_profile", "")` is Python's "not + stated", and all six such sites in `validate.py` normalise it immediately — + `str(opts.get(k, "") or "").strip()`. It cannot be a stale copy of a *number*, + which is the whole of the CFG-04 defect class. Excluded by value. **`0` is + not excluded**: `slew_w: 0` and `integrator_max_w: 0` are real shipped + defaults with real meanings. + +### 2.3 What it grades, measured on `release/1.0` @ `6e7b233` + +``` +2. the mirrors, DISCOVERED from the tree rather than listed here + 6 table(s): circulation.py:CIRC_OPTION_DEFAULTS, delivery.py:DELIVERY_OPTION_DEFAULTS, + maintenance.py:MAINT_OPTION_DEFAULTS, peakguard.py:PEAK_OPTION_DEFAULTS, + price.py:PRICE_OPTION_DEFAULTS, watchdog.py:SIGN_OPTION_DEFAULTS + 30 mirrored key(s) of 71 in `schema:` +3. the AST scan + scanned 29 modules, 34 literal fallback site(s): 0 over mirrored keys, + 34 over unmirrored ones +4. the CFG-03 ledger + 27 (module, key) pair(s) carry a literal for a key no mirror answers for +``` + +Four properties, none of them vacuous: + +1. **The mirrors are discovered, not listed.** Six suites already pin their own + table to `config.yaml` — and every one of those pins names its table by + hand, so a *seventh* table added tomorrow would be pinned by nobody and + nothing would say so. This walks `app/` for module-level `*_OPTION_DEFAULTS` + assignments and grades whatever it finds against `config.yaml`. That is the + property the six hand-written pins cannot have. +2. **No literal survives for a key a mirror already answers for.** 30 keys, + 0 violations. +3. **The CFG-03 ledger is FROZEN, by (module, key) pair, not by count.** A + count goes green again the moment somebody deletes one site and adds + another. Line numbers are deliberately *not* in the key: a ledger that goes + red on an unrelated reformat is a ledger people learn to re-bless without + reading. The 27 pairs are the open CFG-03 backlog; they are **not** removed + here, because that is behaviour change across `main.py`, `p1.py`, + `inverter.py` and `validate.py` that no single card should make in passing. + The set can now only shrink. +4. **Every remaining literal is graded against `config.yaml`** — the half that + catches CFG-04's actual failure mode, two copies of one number and one moves. + +### 2.4 One divergence found, and it is new + +``` + main.py target_grid_w: code 0 vs config.yaml -10 +``` + +`main.py:290` reads `target_grid_w=float(opts.get("target_grid_w", 0))` while +`config.yaml` ships **-10**. A dict handed to `Controller` without the key holds +net exchange at 0 W instead of the shipped -10 W bias into import. **It cannot +reach an operator**: on a Supervisor install the deep-merge guarantees the key +is present, and the only paths that hand over a partial dict are +`sim/inject.py` and this repository's own fixtures. It is recorded in the check +as the single permitted divergence, with the reason, rather than corrected — +changing it alters the control loop's target and belongs to a card that grades +the loop. A *second* divergence goes red. + +### 2.5 ⭐ The red direction, shown rather than claimed + +A check that has never been shown failing is not a check — which is exactly +what clause one of this AC was. Two things follow from that: + +**(a) The red direction runs on every invocation.** Part 5 feeds the scanner a +synthetic module containing a deliberate violation of each rule and *requires* +it to be flagged, in-process, through the same `literal_fallbacks()` the graded +parts call. A scanner that goes blind — a renamed helper, an `ast` change, a +`SCHEMA` that comes back empty — stops working here too, and the total site +count is additionally graded against a floor. + +**(b) A literal was planted in the real tree and the check went red.** Planted +at `main.py:290`, `float(opts.get("maintenance_charge_w", 2000))` — a mirrored +key: + +``` +3. the AST scan: no module carries its own literal fallback for a MIRRORED schema default + scanned 29 modules, 35 literal fallback site(s): 1 over mirrored keys, 34 over unmirrored ones + ok the scanner is not blind: it found 35 sites (floor 20) + main.py:290 get('maintenance_charge_w', 2000) + FAIL no literal fallback survives for any of the 30 keys a mirror already + answers for - those sites must index the mirror + +1 FAILED (exit 1) +``` + +And part 4's two directions, planted the same way with +`opts.get("p1_host", "x")` and `opts.get("slew_w", 999)`: + +``` +4. the CFG-03 ledger + 28 (module, key) pair(s) carry a literal for a key no mirror answers for + FAIL no NEW module/key pair has started carrying a literal fallback [('main.py', 'p1_host')] + main.py p1_host: code 'x' vs config.yaml '' + main.py slew_w: code 999 vs config.yaml 0 + main.py target_grid_w: code 0 vs config.yaml -10 + FAIL every remaining literal still AGREES with config.yaml, except the 1 + divergence(s) recorded above it + + (exit 1) +``` + +The second of those is the CFG-04 defect class itself: a literal that has +drifted away from `config.yaml` while the file still says something else. Both +plants were removed; the tree is byte-identical to `6e7b233` apart from this +card's own files. + +--- + +## 3. CFG-04: closed on `release/1.0`, by UX-02, not by UX-01 + +UX-01's AC 5 claimed the double-literal `max_w` hole. It did not close it. +`fef6ef2` touched `config.yaml`, `translations/`, `test_translations.py` and the +probe — **no defaults code at all** — and on the 0.7.0 tree the triple mutation +survives: 21 files, 0 RED. + +**Confirmed on current `release/1.0` @ `6e7b233`, 2026-09-07.** The same triple +mutation, `2000 -> 2500` at the three descendants of 0.7.0's +`inverter.py:438` / `validate.py:383` / `validate.py:684`: + +| 0.7.0 | `release/1.0` @ `6e7b233` | +|---|---| +| `inverter.py:438` `max_w=float(opts.get("max_w", 2000))` | `inverter.py:640` `raw = opts.get("max_w", 2000)` | +| `validate.py:383` `cap = _number(opts, "max_w", 2000.0)` | `validate.py:466`, same line | +| `validate.py:684` `max_w = _number(opts, "max_w", 2000.0)` | `validate.py:836`, same line | + +Result — 26 files, **RED at `test_selfconfig.py`**, exit 1: + +``` +1 FAILED: ⚠️ CLEARING max_w ON THE LIVE SHAPE IS REFUSED - rule 10 catches the +rail collapsing to the 2000 default under an explicit 400 W slew, so the box +cannot be emptied into a quietly halved rail +``` + +`git log -L 310,325:goodwe_controller/test_selfconfig.py` attributes that check +to **`bcee235`, "UX-02: pin both halves of the max_w fix, because neither was +graded", glenn schrooyen, 2026-09-06** — not to UX-01. The hole ships closed +with the pending 0.9.0 deploy. **The AC is met by UX-02.** No work was re-done +here; the mutation was run once, on a throwaway copy, to confirm it. + +--- + +## 4. The 8 per-entry translations: they stay + +### 4.1 The measurement, and it is negative + +`translations/{en,nl}.yaml` carry a name and a description for all 8 per-entry +keys of the repeatable `inverters:` section. They are loaded by Supervisor's own +loader, they pass `SCHEMA_APP_TRANSLATIONS`, and `test_translations.py` grades +their coverage. **They do not reach the screen**, and this is not a defect in +them. `UiOptions` gives `inverters` `type: "schema"` with `multiple: True`, and +the frontend's `_convertSchemaElement` sends exactly that case to a selector +rather than to a nested `ha-form`: + +```js +if ("schema" === t.type && !t.multiple) return {...type:"expandable"...} // not us +if ("schema" === t.type) { const o = {}; for (const a of t.schema) + o[a.name] = {required: a.required, selector: ...}; return {object:{multiple:t.multiple, fields:o}} } +``` + +and `ha-selector-object` labels those fields `fields[name].label || name`, while +`_convertSchemaElementToSelector` sets **neither `label` nor `translation_key`**. +So an operator sees raw `name`, `setpoint_entity`, `max_w`, `bms_soc_floor`. The +probe already asserts this, negatively and by name. + +### 4.2 Decision: keep them. Four reasons. + +1. **The Supervisor tab cannot be made to render them.** The gap is in + somebody else's converter, not in this repository. There is no change here + that fixes it; deleting the translations does not fix it either. +2. **[[UX-02]]'s ingress panel is where they become visible**, and it renders + the roster itself rather than handing it to `ha-selector-object`. The + `fields:` block is the shape Supervisor's own schema defines for exactly + this, and it is what a surface that wires `label` through consumes. +3. **They cost nothing.** No runtime path reads them; they are two YAML blocks + in files the Supervisor already parses, and the parse is graded green. +4. **Deleting them would remove the only thing that catches a per-entry key + added without a translation.** `test_translations.py` grades top-level and + per-entry coverage symmetrically; drop the per-entry half and SAFETY-32's + `bms_soc_floor` — a *required* per-entry key added after UX-01 — would have + joined the roster with no label and nothing to say so. + +This is a written decision, not a change. The tree is unmodified in +`translations/`. + +### 4.3 Not re-filed + +`tools/supervisor_selfapi_probe.py` has been known red since OPT-09 — it +hardcodes `len(_sub) == 7` against 8 per-entry keys. That is **TOOL-04**, +already filed, and it is not re-filed or fixed here. It is named only so a +reader of §4.1 knows why the *other* probe is the one quoted. + +--- + +## 5. What this card did not do + +* It did not remove a key from `options:`. See §1.5 — the deep-merge default is + what makes the pending 0.9.0 upgrade safe on a 0.7.0 box. +* It did not pay down the 27-pair CFG-03 ledger. See §2.3. +* It did not correct `main.py`'s `target_grid_w` fallback. See §2.4. +* It did not touch `translations/`. See §4.2. +* It did not re-do CFG-04. See §3. +* It did not touch hardware, the live Home Assistant at 192.168.2.158, or + `sim/`. Both probe runs were throwaway containers over a read-only mount. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 0ed60bf..e6e3aa7 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -110,7 +110,8 @@ so Home Assistant's *Configuration* tab renders human labels and a description for every option instead of the raw option keys. What that does and does not buy you is worth stating plainly, because it was **measured against the Supervisor image this site runs** (`ghcr.io/home-assistant/amd64-hassio-supervisor:2026.08.0`) -rather than assumed — `tools/supervisor_shape_probe.py` section F prints the +rather than assumed — `tools/supervisor_shape_probe.py` section F GRADES (it +used only to print) the whole rendered list, in both languages. | | | diff --git a/goodwe_controller/test_defaults_pin.py b/goodwe_controller/test_defaults_pin.py new file mode 100644 index 0000000..3865051 --- /dev/null +++ b/goodwe_controller/test_defaults_pin.py @@ -0,0 +1,372 @@ +"""UX-04 - the defaults pin UX-01 AC 4 promised and never built. +`python3 -B test_defaults_pin.py` + +WHAT UX-01 AC 4 ASKED FOR, IN ITS OWN WORDS: "every `schema:` key absent from +`options:` has exactly one defaults-table entry ... the table changed goes red +... and NO MODULE CARRIES ITS OWN LITERAL FALLBACK FOR A SCHEMA DEFAULT - an AST +check, not a grep." + +WHAT SHIPPED. Clause one is VACUOUS: `options:` and `schema:` declare the same +71 keys, so "every schema key absent from options" quantifies over the empty set +and is true of any tree, including one with no defaults at all. Clause two - +"the table changed goes red" - does work, six times over, once per mirror in its +own suite. Clause three, THE ONE WITH THE CONTENT, was never written. The only +literal scan in this repository before this file is +`test_validate.py`'s `re.findall(r'"(maintenance_[a-z_]+)"...')`, which is a +grep, is restricted to eight `maintenance_*` keys and three named files, and +predates UX-01. + +⚠️ AND A CHECK THAT CANNOT GO RED IS NOT A CHECK, so this file does not repeat +clause one's mistake. Every part below states the size of the set it quantifies +over and grades that size against a floor derived from the tree, and part 5 +plants a literal in a synthetic module and REQUIRES the scanner to catch it, so +the red direction is exercised on every run rather than attested in a commit +message. MUTATION.md habit 4. + +--------------------------------------------------------------------------- # +WHAT AN "IN-CODE LITERAL FALLBACK" IS, STRUCTURALLY, so a reader can argue with +the definition rather than guess it. It is a call in `app/` that takes a STRING +LITERAL naming a `schema:` key and, in the next positional slot, a CONSTANT: + + opts.get("max_w", 2000) _number(opts, "max_w", 2000.0) + +Two shapes, one rule, found by walking `ast.Call` nodes - not by matching text, +so `opts.get(\n "max_w",\n 2000,\n)` and a renamed `opts` are found too and +a `max_w` inside a comment or a docstring is not. + +TWO EXCLUSIONS, both structural and both narrow: + + * `getattr(obj, "max_w", 0.0)` reads an ATTRIBUTE off an object, not an option + off the operator's blob. Its third argument is an absent-attribute guard and + has no relationship to config.yaml. Excluded by function name. + + * THE EMPTY STRING. `opts.get("static_profile", "")` is Python's "this key is + not stated", and every one of the six sites in `validate.py` immediately + normalises it - `str(opts.get(k, "") or "").strip()`. It cannot be a stale + copy of a NUMBER, which is the whole of the CFG-04 defect class, and + rewriting those six to index a mirror would replace a sentence that reads + "absent" with one that reads "the shipped default" at sites where absent is + what is meant. Excluded by value. `0` is NOT excluded: `slew_w: 0` and + `integrator_max_w: 0` are real shipped defaults with real meanings. + +--------------------------------------------------------------------------- # +WHAT THIS FILE DOES *NOT* DO, said before the parts rather than after them. + +It does not remove the 27 (module, key) pairs in part 4's ledger. That is +CFG-03's open ticket - option defaults living in four places - and it is a body +of behaviour change across `main.py`, `p1.py`, `inverter.py` and `validate.py` +that no single card should make in passing. What this file does instead is +FREEZE the ledger: the set is graded EXACTLY, so the twenty-eighth pair is red +on the commit that introduces it and the ledger can only ever shrink. A backlog +that cannot grow is a backlog being paid down; one that is merely written in a +comment is the state UX-01 left. + +MUTATION.md is the method for the red-direction evidence in part 5, and +`tools/mutation_run.sh` is the harness. +""" + +import ast +import io +import re +import sys +from pathlib import Path + +HERE = Path(__file__).resolve().parent +APP = HERE / "app" + +FAILED = 0 + + +def check(what, ok): + global FAILED + print((" ok " if ok else " FAIL ") + what) + if not ok: + FAILED += 1 + + +# --------------------------------------------------------------------------- # +# config.yaml, parsed by hand. +# +# ⚠️ NO PyYAML, AND NOT AS A PREFERENCE. The add-on image installs py3-aiohttp +# and py3-paho-mqtt only; `import yaml` raises in the checks container and a +# check that skips on ImportError is not a check. `test_maintenance.py` and +# `test_price.py` each carry their own two-line reader for the same reason, and +# this is the third - which is itself an instance of the defect this file +# grades, one level up. It is written down rather than tidied away because +# tidying it into a shared helper is a change to two suites that are green. +CFG_TEXT = io.open(HERE / "config.yaml", encoding="utf-8").read() +_SCHEMA_AT = CFG_TEXT.index("\nschema:") +_OPTS_BLOCK, _SCHEMA_BLOCK = CFG_TEXT[:_SCHEMA_AT], CFG_TEXT[_SCHEMA_AT:] + + +def _scalar(raw): + """One `options:` value, as the yaml module this image lacks would read it.""" + raw = raw.split("#", 1)[0].strip().strip('"').strip("'") + if raw in ("true", "false"): + return raw == "true" + if raw == "[]": + return [] + for cast in (int, float): + try: + return cast(raw) + except ValueError: + pass + return raw + + +def _block_keys(block): + return [m.group(1) for m in re.finditer(r"^ ([a-z0-9_]+):", block, re.M)] + + +SHIPPED = {m.group(1): _scalar(m.group(2)) + for m in re.finditer(r"^ ([a-z0-9_]+): *(.*)$", _OPTS_BLOCK, re.M)} +SCHEMA = set(_block_keys(_SCHEMA_BLOCK)) + +print("UX-04 - the defaults pin\n") +print("1. the surface this file quantifies over") +print(f" config.yaml: {len(SHIPPED)} keys under `options:`, {len(SCHEMA)} " + f"under `schema:`") +# The floor, for part 1's own reason: a reader that returned {} would make every +# check below vacuously true. 60 is the count of REQUIRED schema keys and is the +# number `tools/supervisor_shape_probe.py` section F now grades; any floor well +# inside the real size does the job, and this one cannot be reached by accident. +check(f"the hand reader found a real options block ({len(SHIPPED)} keys, floor 60)", + len(SHIPPED) >= 60) +check(f"...and a real schema block ({len(SCHEMA)} keys, floor 60)", + len(SCHEMA) >= 60) +check("every shipped default names a schema key, so a mirror pinned to one is " + "pinned to the other", + sorted(set(SHIPPED) - SCHEMA) == []) + + +# --------------------------------------------------------------------------- # +print() +print("2. the mirrors, DISCOVERED from the tree rather than listed here") +# ⚠️ WHY DISCOVERED. Six suites already pin their own mirror to config.yaml - +# test_maintenance, test_price, test_watchdog (twice), test_peakguard, +# test_delivery. Every one of those pins names its table by hand, so a SEVENTH +# table added tomorrow is pinned by nobody and nothing says so. This walks +# `app/` for module-level assignments whose name ends in OPTION_DEFAULTS and +# grades whatever it finds, so the seventh is covered on the commit that +# introduces it. That is the property the six hand-written pins cannot have. +MODULES = sorted(p.name for p in APP.glob("*.py")) +TREES = {name: ast.parse(io.open(APP / name, encoding="utf-8").read(), name) + for name in MODULES} + +MIRROR = {} # schema key -> (module, table name, value) +TABLES = [] +for name in MODULES: + for node in TREES[name].body: + if not (isinstance(node, ast.Assign) and len(node.targets) == 1 + and isinstance(node.targets[0], ast.Name) + and node.targets[0].id.endswith("OPTION_DEFAULTS")): + continue + table = node.targets[0].id + TABLES.append((name, table)) + for k, v in zip(node.value.keys, node.value.values): + # A literal key with a literal value is the only shape a mirror + # has; anything else is not a table this file can grade, and + # saying so out loud beats skipping it silently. + MIRROR[k.value] = (name, table, ast.literal_eval(v)) + +print(f" {len(TABLES)} table(s): " + ", ".join(f"{m}:{t}" for m, t in TABLES)) +print(f" {len(MIRROR)} mirrored key(s) of {len(SCHEMA)} in `schema:`") +check(f"the walk found the mirrors that exist ({len(TABLES)} tables, floor 6)", + len(TABLES) >= 6) +_unschemad = sorted(k for k in MIRROR if k not in SCHEMA) +check(f"every mirrored key is a real schema key {_unschemad or ''}", + _unschemad == []) +_unshipped = sorted(k for k in MIRROR if k not in SHIPPED) +check(f"...and is shipped in the options block {_unshipped or ''}", + _unshipped == []) +_drift = sorted(k for k, (m, t, v) in MIRROR.items() + if k in SHIPPED and v != SHIPPED[k]) +check(f"...and reproduces config.yaml exactly, so ANY mirror - including one no " + f"suite names by hand - goes red on drift {_drift or ''}", + _drift == []) + + +# --------------------------------------------------------------------------- # +print() +print("3. the AST scan: no module carries its own literal fallback for a " + "MIRRORED schema default") + + +def literal_fallbacks(tree, name): + """Every `f(..., "", , ...)` in one module. + + Yields (module, lineno, callee, key, value). See the header for the two + exclusions and the argument for each. + """ + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + fn = node.func + callee = fn.attr if isinstance(fn, ast.Attribute) else getattr(fn, "id", "") + if callee == "getattr": + continue + for i, arg in enumerate(node.args[:-1]): + if not (isinstance(arg, ast.Constant) and isinstance(arg.value, str) + and arg.value in SCHEMA): + continue + default = node.args[i + 1] + if not isinstance(default, ast.Constant): + continue + if default.value is None or default.value == "": + continue + yield (name, node.lineno, callee, arg.value, default.value) + + +ALL_SITES = [s for name in MODULES for s in literal_fallbacks(TREES[name], name)] +MIRRORED_SITES = [s for s in ALL_SITES if s[3] in MIRROR] +UNMIRRORED_SITES = [s for s in ALL_SITES if s[3] not in MIRROR] + +print(f" scanned {len(MODULES)} modules, {len(ALL_SITES)} literal fallback " + f"site(s): {len(MIRRORED_SITES)} over mirrored keys, " + f"{len(UNMIRRORED_SITES)} over unmirrored ones") +# ⚠️ THE SCANNER'S OWN FLOOR, and it is the line that stops this whole file +# rotting into clause one. A scanner that silently stopped matching - a renamed +# helper, an ast API change, a `SCHEMA` that came back empty - would report ZERO +# sites and every check below would pass. So the total is graded against a floor +# the tree already exceeds by a wide margin, and part 5 additionally feeds the +# scanner a source it MUST flag. +check(f"the scanner is not blind: it found {len(ALL_SITES)} sites (floor 20)", + len(ALL_SITES) >= 20) +for site in sorted(MIRRORED_SITES): + print(f" {site[0]}:{site[1]} {site[2]}({site[3]!r}, {site[4]!r})") +check(f"no literal fallback survives for any of the {len(MIRROR)} keys a mirror " + f"already answers for - those sites must index the mirror", + MIRRORED_SITES == []) + + +# --------------------------------------------------------------------------- # +print() +print("4. the CFG-03 ledger: the unmirrored keys that still carry a literal, " + "frozen at exactly today's set") +# ⚠️ WHY A FROZEN SET AND NOT A COUNT. A count goes green again the moment +# somebody deletes one site and adds another, which is the same-size drift this +# repository keeps finding. The pair (module, key) is stable under every edit +# that is not the one being guarded against: reformatting, moving the call +# within the file, renaming the local `opts`. It changes only when a module +# starts or stops carrying a literal for a key - which is exactly the event. +# +# LINE NUMBERS ARE DELIBERATELY NOT IN THE KEY. A ledger that goes red when an +# unrelated edit shifts a line is a ledger people learn to re-bless without +# reading, and a ledger re-blessed without reading grades nothing. +LEDGER = { + ("inverter.py", "max_w"), + ("main.py", "auto_start"), + ("main.py", "cycle_floor_s"), + ("main.py", "deadband_w"), + ("main.py", "estop_fitted"), + ("main.py", "gain"), + ("main.py", "heartbeat_s"), + ("main.py", "log_level"), + ("main.py", "meter_freeze_s"), + ("main.py", "meter_max_age_s"), + ("main.py", "meter_phases"), + ("main.py", "peak_cap_w"), + ("main.py", "saturation_cycles"), + ("main.py", "saturation_w"), + ("main.py", "sign_disagree_s"), + ("main.py", "sign_disagree_w"), + ("main.py", "slew_w"), + ("main.py", "stale_input_s"), + ("main.py", "step_w"), + ("main.py", "target_grid_w"), + ("p1.py", "meter_poll_s"), + ("p1.py", "meter_source"), + ("validate.py", "max_w"), + ("validate.py", "meter_max_age_s"), + ("validate.py", "meter_phases"), + ("validate.py", "sign_disagree_w"), + ("validate.py", "slew_w"), +} +_found = {(s[0], s[3]) for s in UNMIRRORED_SITES} +_new = sorted(_found - LEDGER) +_gone = sorted(LEDGER - _found) +print(f" {len(_found)} (module, key) pair(s) carry a literal for a key no " + f"mirror answers for") +check(f"no NEW module/key pair has started carrying a literal fallback {_new or ''}", + _new == []) +check(f"...and none has been paid off without the ledger being shortened - " + f"shrink LEDGER when it has {_gone or ''}", + _gone == []) + +# --- and every one of them is graded AGAINST config.yaml, which is the half +# that catches CFG-04's actual failure: two copies of one number, one moves. +DIVERGENT = { + # main.py:290 `target_grid_w=float(opts.get("target_grid_w", 0))`. + # config.yaml ships -10 W. MEASURED here for the first time, 2026-09-07, + # and it is a real divergence, not a sentinel: a dict handed to + # `Controller` WITHOUT the key holds net exchange at 0 W instead of the + # shipped -10 W bias into import. On a Supervisor install the key is always + # present - the deep-merge of `options:` guarantees it - so this cannot + # reach an operator; the paths that CAN hand over a partial dict are + # `sim/inject.py` and this repository's own fixtures. It is recorded rather + # than corrected because changing it is a behaviour change to the control + # loop's target, which belongs to a card that grades the loop. + ("main.py", "target_grid_w"): (0, -10), +} +_diverge = {} +for mod, line, callee, key, value in UNMIRRORED_SITES: + want = SHIPPED.get(key) + same = (isinstance(value, bool) == isinstance(want, bool)) and value == want + if not same: + _diverge[(mod, key)] = (value, want) +for pair, (got, want) in sorted(_diverge.items()): + print(f" {pair[0]} {pair[1]}: code {got!r} vs config.yaml {want!r}") +check(f"every remaining literal still AGREES with config.yaml, except the " + f"{len(DIVERGENT)} divergence(s) recorded above it", + _diverge == DIVERGENT) + + +# --------------------------------------------------------------------------- # +print() +print("5. the red direction, RUN on every invocation") +# ⚠️ A CHECK THAT HAS NEVER BEEN SHOWN FAILING IS NOT A CHECK. Parts 3 and 4 +# both pass on a clean tree, which is exactly what UX-01 AC 4's clause one did +# while grading nothing. So the scanner is fed a source that is a deliberate +# violation of each rule and is REQUIRED to flag it - in-process, against the +# same `literal_fallbacks()` the parts above call, so a scanner that stops +# working stops working here too. +_PLANTED = ''' +def build(opts): + a = opts.get("maintenance_charge_w", 2000) # mirrored key -> part 3 + b = _number(opts, "cycle_floor_s", 15) # unmirrored, main.py's key + c = opts.get("static_profile", "") # the "" sentinel: NOT a hit + d = getattr(unit, "max_w", 0.0) # attribute guard: NOT a hit + e = opts.get("peak_soc_floor", 20) # mirrored key -> part 3 + return a, b, c, d, e +''' +_hits = list(literal_fallbacks(ast.parse(_PLANTED, ""), "")) +_keys = sorted(h[3] for h in _hits) +for h in _hits: + print(f" caught {h[2]}({h[3]!r}, {h[4]!r}) at line {h[1]}") +check("a planted literal for a MIRRORED key is caught - this is part 3's red " + "direction, exercised", + "maintenance_charge_w" in _keys and "peak_soc_floor" in _keys) +check("a planted literal for an unmirrored key is caught too - part 4's", + "cycle_floor_s" in _keys) +check('...and the two documented exclusions still hold: the "" sentinel and ' + "getattr are not reported", + "static_profile" not in _keys and _keys.count("max_w") == 0) +check(f"exactly the three intended sites, no more {_keys}", + _keys == ["cycle_floor_s", "maintenance_charge_w", "peak_soc_floor"]) + +# ...and the mirror comparison's red direction, likewise executed rather than +# claimed: part 2 goes red when a table and config.yaml disagree by one. +_drifted = dict(MIRROR) +_pick = sorted(MIRROR)[0] +_mod, _tab, _val = _drifted[_pick] +_drifted[_pick] = (_mod, _tab, "###drifted###") +check(f"part 2's comparison rejects a mirror that has drifted (simulated on " + f"{_pick!r})", + any(v != SHIPPED.get(k) for k, (_, _, v) in _drifted.items())) + + +print() +if FAILED: + print(f"{FAILED} FAILED") + sys.exit(1) +print("all checks passed") diff --git a/goodwe_controller/tools/supervisor_shape_probe.py b/goodwe_controller/tools/supervisor_shape_probe.py index 99df882..e1fa976 100644 --- a/goodwe_controller/tools/supervisor_shape_probe.py +++ b/goodwe_controller/tools/supervisor_shape_probe.py @@ -315,6 +315,114 @@ print(f" the Configuration tab renders {len(ui)} fields, in `schema:` order " "(App.schema_ui is UiOptions(...)(data['schema']), so the ORDER IS THE " "SCHEMA BLOCK'S and not the options block's)") +# --------------------------------------------------------------------------- # +# ⚠️ UX-04: THAT COUNT IS NOW GRADED, AND IT IS WHY UX-01's AC 3 IS WITHDRAWN. +# +# UX-01 AC 3 said the visible count would drop to ~15, with the rest behind +# HA's "show unused optional configuration options" toggle. The line above has +# printed the refutation since the day it was written - 71, not 15 - and PRINTED +# IS NOT GRADED: this probe exited 0 through the whole of UX-01's review while +# displaying the number that contradicted the criterion it was evidence for. +# That is the defect, not the count. +# +# THE FRONTEND RULE, transcribed from the bundle in THIS image at the top of +# this section and replayed rather than paraphrased: +# +# _filteredSchema = (opts, schema) => schema.filter(e => e.name in opts +# || e.required) +# +# `opts` is the add-on's OPTIONS AS SUPERVISOR SERVES THEM - the operator's +# stored blob after `options:` has been deep-merged into it - so a key is +# hideable only when it is BOTH absent from `options:` AND not required. +# +# WHY THE CRITERION IS WITHDRAWN RATHER THAN IMPLEMENTED, in two measurements: +# +# 1. IT IS ARITHMETICALLY UNREACHABLE. Supervisor's own UiOptions marks +# `required` on every non-`?` key, and the filter above shows a required +# key UNCONDITIONALLY. So the floor - what the tab renders with `options:` +# EMPTIED ALTOGETHER, which is the most hiding that is even expressible - +# is the required count below. It is nowhere near 15. No arrangement of +# `options:` can reach the target; the criterion was never satisfiable. +# +# 2. THE 12 KEYS THAT *COULD* BE HIDDEN COST THE DEEP-MERGE DEFAULT. A key +# absent from `options:` has nothing for the Supervisor to merge into an +# existing box's stored blob. That is exactly the `delivery_hold_s` +# mechanism the 0.9.0 upgrade rests on: a key that 0.7.0's stored blob +# cannot contain resolves to its `options:` default instead of failing +# validation, and moving it out of `options:` is precisely how you get the +# exit 78 that mechanism exists to prevent. Worse, it would promote the 27 +# unpinned in-code literal fallbacks that `test_defaults_pin.py` part 4 +# ledgers into the LOAD-BEARING source of those defaults on a live install +# - which is the CFG-03 defect the same card's AC 4 exists to police. +# +# So: four fewer clicks, bought with the upgrade path and with the defaults +# pin. The decision is written up in +# DECISIONS/UX-04-the-configuration-tab-and-the-defaults-pin.md +# and the three assertions below are what keeps it a decision rather than an +# omission - if anyone empties `options:` to shorten the form, this goes red. +_hideable = sorted(set(schema) - set(defaults)) +_required = [n["name"] for n in ui if n.get("required")] + + +def _filtered(opts): + """The frontend's own `_filteredSchema`, replayed on this manifest.""" + return [n for n in ui if n["name"] in opts or n.get("required")] + + +show(f"every one of the {len(schema)} `schema:` keys is ALSO in `options:`, so " + f"schema-only - the only set HA's toggle can hide - is EMPTY. UX-01 AC 3 " + f"is not partially met, it is unstarted. Hideable: {_hideable}", + not _hideable and len(schema) == len(defaults)) +show(f"...so replaying the frontend's own _filteredSchema on the served options " + f"hides NOTHING: {len(_filtered(defaults))} of {len(ui)} fields render", + len(_filtered(defaults)) == len(ui)) +# ⚠️ DERIVED, NOT TYPED, AND NOT A FLOOR. The assertion above read +# `len(_required) > 15` while the prose said 59 - so a manifest change dropping +# the required count to 20 would have passed while the sentence beside it stayed +# wrong. That is the same shape as the count this repo already fixed in +# `test_translations.py`: a number displayed but not graded. +# +# So the expected value is computed from config.yaml by Supervisor's OWN rule - +# a key is optional iff its schema string ends in `?`, plus `inverters`, which +# `UiOptions` marks optional because it is a list - and the assertion is an +# EQUALITY against that. Typing 59 here would rot the first time a key is added. +_OPTIONAL = {k for k, v in schema.items() + if isinstance(v, str) and v.rstrip().endswith("?")} | {"inverters"} +_EXPECT_REQ = len(schema) - len(_OPTIONAL) +show(f"...and that floor is DERIVED: {len(schema)} schema keys minus " + f"{len(_OPTIONAL)} optional ones (the `?` suffix, plus `inverters`) is " + f"{_EXPECT_REQ}, and Supervisor's own required flag agrees", + _EXPECT_REQ == len(_required)) + +# ⚠️ AND THE WITHDRAWAL'S OWN PREMISE, GRADED SEPARATELY. The equality above is +# deliberately NOT enough on its own: both sides read the same `?` convention, +# so marking a key optional moves BOTH and it stays green. That is correct - it +# makes the printed number impossible to leave stale - but it grades Supervisor +# agreeing with us, not the thing AC 3 was withdrawn for. +# +# AC 3 was withdrawn because the floor is FAR above the ~15 it asked for. If +# enough keys ever became optional that the floor approached 15, the withdrawal +# stops being true and this file should say so rather than keep printing a +# sentence that has quietly become wrong. +_AC3_ASKED = 15 +show(f"...and the floor of {_EXPECT_REQ} is still far above the ~{_AC3_ASKED} " + f"AC 3 asked for, which is the whole reason it was withdrawn. If this ever " + f"goes red the withdrawal needs re-arguing, not re-asserting", + _EXPECT_REQ > 2 * _AC3_ASKED) + + +# ⚠️ AN ISOLATED COUNTERFACTUAL, and the number is Supervisor's own `required` +# flag rather than a count typed here. `options:` is emptied and NOTHING else +# varies, so the answer cannot be about anything but the merge. +show(f"...and with `options:` EMPTIED ENTIRELY - the maximum hiding that is " + f"expressible - the tab still renders {len(_filtered({}))} required " + f"fields. THAT is the floor, and AC 3 asked for ~15. The criterion is " + f"withdrawn as unsatisfiable, not deferred.", + len(_filtered({})) == len(_required) and len(_required) == _EXPECT_REQ) + + + + def _entry(lang, name, path=()): node = TRANSLATIONS.get(lang, {}).get("configuration")