From e2efc765afbe81ce8fd0e9197af93e676dfa01d5 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Sat, 5 Sep 2026 22:38:40 +0200 Subject: [PATCH] SAFETY-38 round 4: the enumeration covered one sink, and it was not the one this codebase uses The scanner's two censuses between them scored ONE syntactic sink and called it the family. Every one of census B's 16 members was a sentence that is APPENDED, ASSIGNED to a `reason`-shaped name, or passed as a LOG ARGUMENT. Not one was RETURNED - and census A's return bucket only ever held a callee formatting its own PARAMETERS, so a method formatting its own ATTRIBUTES and handing the string back was invisible to both. That is the whole describe() / explain() / reason-builder family, which is where this repository actually keeps its operator sentences. Worse than the gap: it was in NONE of the five blind spots the tool printed about itself. THE SCANNER, WIDENED - and it rediscovered both members without being told: * CENSUS C, the return sink, resolved through the returning method's CALL SITES so a returned KEY (store.unit_key, mqtt.unit_state_key - MAINT-05's card) is not mistaken for a returned SENTENCE. 53 returned pairs, 39 reaching an operator, 16 of those sharing a format spec. * A sentence handed to a constructor as `reason=` or placed under a "text" dict key is a MESSAGE. That one line of the sink walker was hiding SIX more caller-built members, census B 16 -> 22. * Every census B and C row now prints whether the two interpolations share a FORMAT SPEC - the difference between a transposition that is silent and one that merely looks broken. * The blind-spot list is now TWO lists: what it scans (printed from the constants, so they cannot drift) and WHAT IT DOES NOT SCAN AT ALL, largest hole first - `%s`-style lazy logging, MEASURED at 31 calls across 8 modules and each one listed, not asserted. CLOSED, both zero-byte survivors of a green 21-file suite: * clamp.py:253 ClampResult.describe(), requested_w <-> applied_w. No test_*.py mentioned describe() at all. It is the SS15 action-log row (main.py:1366) AND the ClampWatchdog notification reason (main.py:2452) - the alarm SAFETY-40 and SAFETY-41 are about. Under the swap the guard reports a setpoint clamped UPWARD, to +9999 W by a bound whose rating the same sentence gives as 2000 W. Pinned on the ORDERED PAIR on the contiguous literal at BOTH consumers, operands read off ctl.target and hass.writes[-1] - what the loop asked and what the device was sent - not off the ClampResult the swap lives in. 4 checks red. * peakguard.py:493 PeakDecision.explain(), projected_w <-> ceiling_w. "peak guard idle" was in no test_*.py. The row then reads "projects 5013 W under a 2300 W ceiling" - 2713 W OVER - on a row whose own words say idle and whose own flag says ok. Pinned on the decision and on the shipped status row. 3 checks red. * test_arbiter.py's scope-violation check, whose CLAIM was wider than its ASSERTION. Measured this round rather than left at source level: 0 bytes, 21/21 green before, red now. 2 checks. DISCLOSED, NOT CLOSED - 11 measured zero-byte survivors, each with its file fingerprint and its 21/21 verdict, in the decision doc SS7e-SS7g. Three in allocator.py's UnitPlan.explain(), three across arbiter.py's two explain()s, two in tariff.py, two in arbitrage.py's trade reasons, one in main.py's SoC-floor notification. An enumerated list with honest gaps beats a fifth round finding a fifth instance. CORRECTIONS THIS ROUND OWES: * The idle sentence does NOT reach main.py:3299/3302 - those are the floor-limited branch, where explain() returns self.reason before the idle literal is built. It reaches main.py:3313 and, on the release edge, 1951. 1951 is disclosed unpinned. * SS4's "Every one of the ten is red" sat under a twelve-row table: it is the ten site_standing entries. * SS6b said "weaker evidence than A1-A3"; A4 was measured, so SS7d's "A1-A4 and B1" is the right list and SS6b now says it. * AND THE PROBE THAT GRADES THESE MUTATIONS WAS THE DEFECT, ONCE. Its first cut numbered the operands BY ORDER OF APPEARANCE, so after a transposition the other leaf took the same number and all thirteen mutant renders came back byte-identical - which reads exactly like "the edit is inert". Keying the value to the NAME fixed it. Habit 4, in the measuring instrument: MUTATION.md habit 16 is the new entry, and the count there is corrected to eleven rather than inflated. 14 mutations measured: fresh full-repo copy each, mounted :ro, bytecode purged and the purge COUNT printed, the edit re-read from disk, length and sha256 printed on both sides, and every one required to change the EMITTED SENTENCE and not merely the bytes. 12 survived the pre-round suite, 1 was already caught, 3 are caught now. Full 21-file suite green on the final tree. NOT ONE BYTE OF app/ MOVES. This card changes coverage only. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU --- goodwe_controller/CHANGELOG.md | 67 +++ .../DECISIONS/SAFETY-38-site-standing-pin.md | 200 +++++++- goodwe_controller/MUTATION.md | 53 +- goodwe_controller/test_arbiter.py | 29 +- goodwe_controller/test_clamp.py | 99 ++++ goodwe_controller/test_peakguard.py | 80 ++++ .../tools/symmetric_message_scan.py | 451 +++++++++++++++--- 7 files changed, 901 insertions(+), 78 deletions(-) diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index 86b6014..1ec6f33 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -222,6 +222,73 @@ shipped `sum` is correct and must not be "fixed".** it is recorded rather than quietly dropped — unlike the one that *was* believed, two bullets up. Ledger, and an **enumerated list of what was not mutated**, in `DECISIONS/SAFETY-38-site-standing-pin.md` §4a, §4b and §6a. +* **⭐ ROUND 4: THE SCANNER COVERED ONE SYNTACTIC SINK AND CALLED IT THE + FAMILY, AND TWO LIVE MEMBERS WERE SITTING IN THE HOLE.** Every one of census + B's 16 members was a sentence that is **appended, assigned, or logged**. Not + one was **RETURNED** — and census A's return bucket only ever held a callee + formatting its own *parameters*, so a method formatting its own **attributes** + and handing the string back was invisible to both. That is the whole + `describe()` / `explain()` family, which is where this codebase keeps its + operator sentences. **Worse than the gap: it was in none of the five blind + spots the tool printed about itself.** The tool now scans the return sink + (census C), resolving it through the returning method's **call sites** so that + a returned *key* — `store.unit_key()`, `mqtt.unit_state_key()` — is not + mistaken for a returned *sentence*; it also scores a sentence handed to a + constructor as `reason=` or placed under a `"text"` dict key, which added + **six more caller-built members** it had been stepping past. Re-run, it + **rediscovered both of the members this round was sent to fix without being + told where to look**. +* **⭐ `ClampResult.describe()` — the clamp alarm the owner has been complaining + about all day — had a SECOND independent way to lie, and no `test_*.py` + mentioned `describe()` at all.** Transposing `requested_w` and `applied_w` in + `clamp.py:253` is a **zero-byte** edit that leaves the guard reporting a + setpoint clamped **UPWARD**, in the measured case to +9999 W by a bound whose + rating the same sentence gives as 2000 W. It is the §15 action-log row + (`main.py:1366`) **and** the ClampWatchdog notification reason + (`main.py:2452`). Measured **21/21 green** before; now **red on 4 checks** in + `test_clamp.py` part 6d, pinned on the **ordered pair** on the contiguous + literal at **both** consumers, with operands read off `ctl.target` and + `hass.writes[-1]` — what the loop asked and what the device was sent — rather + than off the `ClampResult` the swap lives in. +* **⭐ `PeakDecision.explain()` — the status-page banner — the same shape.** + `projected_w` ↔ `ceiling_w` at `peakguard.py:493`, **zero bytes**, and the + row then reads *"quarter projects 5013 W under a 2300 W ceiling"*: a quarter + **2713 W over** its ceiling, on a row whose own words say *idle* and whose own + flag says **ok**. `"peak guard idle"` appeared in no `test_*.py`. Measured + **21/21 green** before; now **red on 3 checks** in `test_peakguard.py` part 13, + pinned on the decision and on the shipped status row at `main.py:3313`. + ⚠️ **A correction the ticket is owed:** the idle sentence does not reach + `main.py:3299/3302` — those are the *floor-limited* branch, where `explain()` + returns `self.reason` before the idle literal is built. It reaches + **`main.py:3313`** and, on the release edge, `main.py:1951`. +* **`test_arbiter.py`'s scope-violation check made a claim WIDER than its + assertion** — it said `scope_violations` *"names the source and the actuator + it does not belong on"* and asserted `"peak" in scope_errors[0]`, which is + still true after the two swap roles. Measured this round: **0 bytes, 21/21 + green**; now red on 2 checks, pinned on the ordered pair. +* **14 more zero-byte mutations measured, and 11 SURVIVORS ARE DISCLOSED RATHER + THAN CLOSED**, each with its file fingerprint and its 21/21 verdict, in + `DECISIONS/SAFETY-38-site-standing-pin.md` §7e–§7g: three in + `allocator.py`'s `UnitPlan.explain()`, three across `arbiter.py`'s two + `explain()`s, two in `tariff.py`, two in `arbitrage.py`'s trade reasons and + one in `main.py`'s SoC-floor notification. **An enumerated list with honest + gaps beats a fifth round finding a fifth instance.** + ⚠️ **And the probe that grades those mutations was itself the defect, once.** + Its first cut numbered the operands **by order of appearance**, so after a + transposition the other leaf took the same number and all thirteen renders came + back **byte-identical** — which reads exactly like *the edit is inert*. Keying + the value to the **name** fixed it. A measurement comparing the swap against + itself, in the instrument this time. +* **The tool now states what it does NOT scan**, largest hole first: + `%s`-style lazy logging (`_LOG.error("… %s … %s", a, b)` puts the operands in + *arguments*, so no census here looks at them and the transposition is the same + zero-byte edit — **31 such calls across 8 modules, counted and listed by the + tool rather than asserted**), `.format()`/`%`/`+`/`.join()` sentences, a sentence that + leaves through a dict or dataclass and is rendered elsewhere, `yield`ed + sentences, message attributes outside the `reason`-shaped set, and everything + outside `app/`. Its five over/under-report gaps stay, and every census row now + prints whether the two interpolations **share a format spec** — the difference + between a transposition that is silent and one that merely looks broken. **Inverter B gets its own MQTT device, and inverter A keeps the one it has (TEL-07).** MQTT discovery announced **one** device, and every entity hung off diff --git a/goodwe_controller/DECISIONS/SAFETY-38-site-standing-pin.md b/goodwe_controller/DECISIONS/SAFETY-38-site-standing-pin.md index c8f1dbc..c86a774 100644 --- a/goodwe_controller/DECISIONS/SAFETY-38-site-standing-pin.md +++ b/goodwe_controller/DECISIONS/SAFETY-38-site-standing-pin.md @@ -234,7 +234,7 @@ fingerprint, and verifies no cache appears during the run). The mutated line was | **M11** | **argument SWAP** at the SITE call site: `evaluate(site_standing, self.grid, …)` → `evaluate(self.grid, site_standing, …)` (main.py:1620) | **0** | **was a SURVIVOR in round 1 — now RED**, see §4a | | **M12** | **argument SWAP** at the PER-UNIT call site: `evaluate(stood, against, …)` → `evaluate(against, stood, …)` (**main.py:1560**) | **0** | **was a SURVIVOR in round 2 — now RED**, see §4c | -**Every one of the ten is red in `test_watchdog.py`.** Re-measured in round 2 +**Every one of the ten `site_standing` entries is red in `test_watchdog.py`.** Re-measured in round 2 on `release/1.0` @ `86eaef2` (where `app/main.py` is 234 542 bytes, 2 022 more than the `f72c48d` the round-1 figures were taken on): all ten still red, at the byte deltas in the table, each on a fresh copy with the edit re-read from disk. @@ -584,13 +584,73 @@ disk, byte delta printed):** this one. * **Twelve census-B sites** (§7c, B2–B12) determined unpinned at the **source level** — the distinguishing literal appears in no `test_*.py` — and **not** - mutated. Weaker evidence than A1–A3, and written down as such. + mutated. Weaker evidence than A1–A4 and B1, and written down as such + — §7d says the same thing with the same list. * **B6**, SAFETY-31's `circ_reason`, is the same shape as the defect this card closed for SAFETY-02/30 and is *also* persisted and re-read after a restart. §6a already declines to touch `circ_wd`; §7d says what that costs. * Everything §6a lists stays open: `circ_wd`'s own thresholds, the eight other `CLAIM_SCOPE` entries, `actuator_of()`'s body, and the `n_units > 1` guard. +### 6c. Round 4: what was closed, and what is still open + +**The blocker round 3 shipped.** The enumeration read as complete and covered +**one syntactic sink**. Every one of census B's 16 members was a sentence that +is **appended, assigned, or passed as a log argument** (`problems.append`, +`out.append({"text": …})`, `self.reason = …`, `_LOG.info`). **Not one was a +sentence that is RETURNED.** Census A has a return bucket, but only for +symmetric **parameters**, so a method formatting its **own attributes** and +handing the string back was invisible to both — and `describe()` / `explain()` / +the reason-builders are exactly where this codebase keeps its operator +sentences. Worse than the gap: it was in **none of the five blind spots the tool +prints about itself**. A tool that asserts a completeness it does not have is +the recurring defect wearing a new hat. + +**Closed in round 4:** + +1. **`clamp.py:253` `ClampResult.describe()`**, pair `requested_w` / + `applied_w` — §7e C6. A survivor of the round-3 tree at **21/21 green**, + **0 bytes**. It is the §15 action-log row at `main.py:1366` **and** the + ClampWatchdog notification reason at `main.py:2452`; **no `test_*.py` + mentioned `describe()` at all.** Pinned on the ordered pair, on the + contiguous literal, at **both** consumers, with a precondition that the two + operands render differently — `test_clamp.py` part 6d. **4 checks red** under + the transposition. +2. **`peakguard.py:493` `PeakDecision.explain()`**, pair `projected_w` / + `ceiling_w` — §7e C7. Same measurement: **0 bytes, 21/21 green**; the string + `"peak guard idle"` was in no `test_*.py`. Pinned on the decision and on the + shipped status row at `main.py:3313` — `test_peakguard.py` part 13. **3 + checks red.** +3. **`arbiter.py:264` (B12)** — the check whose **claim was wider than its + assertion**, carried forward from §7d. Measured this round rather than left + at source level: **0 bytes, 21/21 green** before, **`test_arbiter.py` red** + after. **2 checks red.** +4. **The tool's sinks are now stated rather than implied.** It prints what it + scans as a MESSAGE, that it now scans the RETURN sink through the returning + method's call sites, and — the part that was missing — **an explicit list of + what it does not scan at all**, largest hole first. + +**⚠️ The correction round 4 owes the ticket.** The brief placed +`PeakDecision.explain()`'s idle sentence at `main.py:3299/3302`. Those two lines +are the **floor-limited** branch, where `explain()` returns `self.reason` before +the idle literal is ever built. The idle sentence reaches the page at +**`main.py:3313`** and the action log at **`main.py:1951`** (edge-logged, so it +is the RELEASE edge that writes it). The pin is on 3313; 1951 is disclosed +below, unpinned. + +**Open, named, and measured — see §7f for the ledger:** + +* **Nine further zero-byte survivors** found by the widened scan, each measured + at 21/21 green on this tree: three in `allocator.py`'s `UnitPlan.explain()`, + three in `arbiter.py`'s two `explain()`s, two in `tariff.py`, and one each in + `arbitrage.py`'s two decision reasons and `main.py`'s SoC-floor notification. + None is a control decision; every one is a sentence an operator reads. +* **The action-log surface of the peak-guard idle line** (`main.py:1951`) is + not pinned. The status row (3313) is, and both come off the same `explain()`, + so the *sentence* is closed and the *second call site* is not — which is + precisely the distinction §4c was written about, recorded rather than assumed. +* Everything §6a and §6b list stays open. + ## 7. The symmetric-predicate family — every call whose argument order is load-bearing for a MESSAGE **Why this section exists, and it is the most useful thing in the card.** Round 2 @@ -664,8 +724,7 @@ symmetry test has nothing to test. The scan pairs interpolated expressions that are **one leaf apart** — `inv.dev_min` / `inv.dev_max`, `cfg.min_w` / `cfg.hold_s` — which is exactly what a transposition swaps, and it keeps only sentences whose sink is a **message**: an f-string that becomes a URL or a store -key is data, and a swap there is an ordinary behavioural change. **16 -candidates**, every one of them a **zero-byte** edit: +key is data, and a swap there is an ordinary behavioural change. **16 candidates** in round 3, every one of them a **zero-byte** edit — and **22** after round 4 widened the sink classifier (B17–B22 below): | # | site | pair | reaches an operator as | pinned? | |---|---|---|---|---| @@ -678,11 +737,22 @@ candidates**, every one of them a **zero-byte** edit: | **B7** | `main.py:2454` | `self.site_max_w` / `self.target` | the clamp action-log line | **NO** — `was clamped to` is in no `test_*.py`. | | **B8** | `main.py:3386` | `clamp_wd.engagements` / `.episodes` | status page, `engaged N time(s) in M episode(s)` | **NO** — `time(s) in` is in no `test_*.py`. | | **B9–B11** | `main.py:3169` (three pairs) | `p1.net_w` / `.samples` / `.parse_errors` | status page, the P1 row | **NO** — `P1 meter (` is in no `test_*.py`. | -| **B12** | `arbiter.py:264` | `c.source` / `c.actuator` | `scope_errors`, the arbiter's mis-routing refusal | **NO, and this one is instructive.** `test_arbiter.py:452` says it *"names the source and the actuator it does not belong on"* and then asserts `"peak" in r_mis.scope_errors[0]`. Under the swap the sentence becomes `"inverter-b: a SITE claim filed on unit 'peak'"` — and `"peak" in …` is **still true**. A claim wider than its assertion, which is habit 14's own shape. | +| **B12** | `arbiter.py:264` | `c.source` / `c.actuator` | `scope_errors`, the arbiter's mis-routing refusal | **ROUND 4: MEASURED, THEN CLOSED.** Round 3 left it at source level; the transposition is **0 bytes**, `arbiter.py` `fdb4ed0520eeb721` → `9111f627ef639ecc`, and it was **21/21 green**. Now red in `test_arbiter.py` on 2 checks. Round 3's reasoning was right: `test_arbiter.py:452` says it *"names the source and the actuator it does not belong on"* and then asserts `"peak" in r_mis.scope_errors[0]`. Under the swap the sentence becomes `"inverter-b: a SITE claim filed on unit 'peak'"` — and `"peak" in …` is **still true**. A claim wider than its assertion, which is habit 14's own shape. | | **B13** | `validate.py:723` | `meter_phases` / `meter_max_age_s` | the startup refusal | **NO** for the ordered pair. | | **B14** | `validate.py:1211` | `arbitrage_soc_floor` / `arbitrage_soc_ceiling` | the startup refusal | **NO** for the ordered pair. | | **B15** | `watchdog.py:667` | `cfg.min_w` / `cfg.hold_s` | the `SIGN_DISAGREEMENT` sentence's `(limits: both over 300 W, held over 180 s)` clause | **YES**, and it is worth naming because it shows the habit works. `test_watchdog.py`'s SAFETY-30 section asserts the site instance's **whole sentence with `==` against a literal**, the `(limits: …)` clause included. Full-string equality is habit 14 taken to its limit. | | B16 | `main.py:237` | `i.name` / `i.max_w` | the roster log line | **not real** — a `str` beside a `:.0f` format. The same over-report as A5–A6: a mixed-type transposition raises rather than lying. | +| **B17** | `arbitrage.py:377` | `v.spread_eur_kwh` / `v.required_spread_eur_kwh` | the DISCHARGE decision's `reason` | **ROUND 4, NEW, and MEASURED: 0 bytes, 21/21 green.** Not visible to round 3 at all — the sentence is handed to a constructor as `reason=`, and the sink walker scored only *assignments*. The row then reads *"0.0400 above the cheapest and clear of the 0.1200 this cycle had to cover"* with the two swapped: a trade justified by a spread it did not clear. | +| **B18** | `arbitrage.py:393` | same pair, CHARGE branch | the CHARGE decision's `reason` | **ROUND 4, NEW, MEASURED: 0 bytes, 21/21 green.** | +| **B19** | `main.py:2652` | `g.reason` / `g.soc_floor_reason` | the `goodwe_peak_soc_floor` notification body | **ROUND 4, NEW, MEASURED: 0 bytes, 21/21 green.** The alarm then opens with the *reason the floor bound* and closes with the *guard's own conclusion*, i.e. both halves of a two-part explanation in the wrong order, in the one message SAFETY-24/25 exist to send. | +| B20–B22 | `main.py:2558`, `main.py:2652` (two more pairs) | `g.reason` / `g.ceiling_w`, `g.ceiling_w` / `g.soc_floor_reason` | the same two notifications | **new in round 4, NOT measured** — mixed types (`str` beside `:.0f`), the same over-report class as B16. | + +**⚠️ Six of these twenty-two were invisible to round 3, and the reason is one +line of the sink classifier.** It scored a sentence a MESSAGE when it was +appended, logged, raised, or assigned to a `reason`-shaped **name**. This +codebase mostly builds operator sentences as `Decision(…, reason=f"…")` and +`out.append({"text": f"…"})` — a **keyword argument** and a **dict key**, which +that walker stepped straight past. Both are message sinks now. ### 7d. What this enumeration does NOT close, with sizes @@ -723,3 +793,123 @@ candidates**, every one of them a **zero-byte** edit: non-f-string payloads, anything outside `app/`, and the one way it **over-reports** — it erases the sentence, so a swap that would make the formatting itself raise is scored as changing no decision (A5–A6 above). + +### 7e. Census C — the sentence an object returns about itself (round 4) + +**Why there is a third census.** A and B between them covered **one syntactic +sink**. Everything either of them reported was a sentence that is appended, +assigned, or logged; **none was returned**. Census A's RETURN bucket only ever +held a callee formatting its own **parameters**, so a method formatting its own +**attributes** and handing the string back fell between them — and that is the +whole `describe()` / `explain()` / reason-builder family, which is where the +operator sentences in this codebase actually live. + +**The test** is census B's — one f-string, two interpolations **one leaf apart** +— run over the RETURN sink, plus one thing B never needed: **a returned string +is not yet a message.** `store.unit_key()` and `mqtt.unit_state_key()` return +f-strings too, and swapping there renames a store key and an MQTT topic, which +every ordinary check can see (MAINT-05). So the sink of a returned sentence is +**resolved through its call sites**: it is in this family only when some caller +in `app/` hands it to an operator. That resolution matches on the **bare method +name**, which over-collects — at least four unrelated classes have an +`explain()` — so every verdict is printed **with the sites that produced it**, +and a MESSAGE verdict is a prompt to read them, not a proof. + +The scan reports **53 returned pairs, 39 reaching an operator, 16 of those +sharing a format spec** (two values rendered alike, so a transposition is silent +rather than visibly broken). It **rediscovered both members this round was sent +to fix without being told where to look**, which is the only reason to trust the +rest of the list. + +| # | site | pair | reaches an operator as | Δ bytes | verdict | +|---|---|---|---|---|---| +| **C6** | `clamp.py:253` `ClampResult.describe()` | `requested_w` / `applied_w` | §15 action log (`main.py:1366`) **and** the ClampWatchdog notification reason (`main.py:2452`) | **0** | **was a SURVIVOR at 21/21 — now RED**, `test_clamp.py` part 6d, 4 checks | +| **C7** | `peakguard.py:493` `PeakDecision.explain()` | `projected_w` / `ceiling_w` | status row `main.py:3313`; action log `main.py:1951` on the release edge | **0** | **was a SURVIVOR at 21/21 — now RED**, `test_peakguard.py` part 13, 3 checks | +| C1 | `allocator.py:154` `UnitPlan.explain()` | `self.span` / `self.why` | the arbiter/allocator log line | **0** | **SURVIVOR, 21/21 green** | +| C2 | `allocator.py:160` | `self.span` / `self.why` | the ENVELOPE VIOLATED line | **0** | **SURVIVOR, 21/21 green** | +| C3 | `allocator.py:162` | `self.span` / `self.why` | the ordinary in-envelope line | **0** | **RED** — `test_alloc.py` already asserts that whole literal. The one member of this census the suite could already see, and it says the habit works. | +| C4a | `arbiter.py:344` `UnitBounds.explain()` | `self.lo` / `self.hi` | `CONTRADICTION within the unit (device lo=… > peak hi=…)` | **0** | **SURVIVOR, 21/21 green** | +| C4b | `arbiter.py:344` | `self.lo_src` / `self.hi_src` | the same sentence, the two SOURCE names | **0** | **SURVIVOR, 21/21 green** | +| C5 | `arbiter.py:392` `Resolution.explain()` | `self.winner` / `self.bound_by` | `main.py:2236`'s `ARBITER:` action-log row and `self.reason` at `main.py:2329` | **0** | **SURVIVOR, 21/21 green** — the winner and the limiter swap names, on the line §10.1 exists to make legible | +| C8 | `tariff.py:279` `PeakReading.explain()` | `self.month` / `self.detail` | the PEAK UNREADABLE row | **0** | **SURVIVOR, 21/21 green** | +| C9 | `tariff.py:528` `Projection.explain()` | `self.projected_w` / `self.elapsed_s` | the projection line | **0** | **SURVIVOR, 21/21 green** — watts printed as seconds and seconds as watts, both `:.0f` | + +**Not in the family, and the scan says why rather than dropping them silently:** +`inverter.py:338` `__repr__()` and `static_tariff.py:378` `words` are reported +**UNRESOLVED — no call site inside `app/`** (a `__repr__` is called by the +interpreter, not by a name the scan can match); `price_forecast.py:421/428` and +`tariff.py:214` `month_key()` resolve to **OTHER** — `month_key` is a **store +key**, which is MAINT-05's case and a swap there is behaviour every check sees. + +### 7f. Round 4's mutation ledger + +Same protocol as §4: a fresh full-repo copy per mutation, `:ro`, bytecode +purged **and the purge count printed** before the edit, the edited file +**re-read from disk**, both sides' length and sha256 printed, and the full +**21-file** suite via `tools/mutation_run.sh`. Every row is a pure +transposition, so **every Δ is 0 bytes** — verified, not assumed: `len(old) == +len(new)` is asserted before the write and `len(after) == len(before)` after it. + +**⚠️ And every row was also required to change the ANSWER.** A byte delta says +the file moved; it does not say the sentence did. Each mutated tree is rendered +through a scratch probe that evaluates the shipped f-string with **distinct +operands** and diffs it against the same render of the clean tree. **The first +cut of that probe numbered the operands by ORDER OF APPEARANCE** — so after a +transposition the other leaf took the same number and all thirteen diffs came +back **empty**, which reads exactly like "the edit is inert". Keying the value +to the **name** fixed it. That is the same defect as everything else in this +card, in the instrument this time: a measurement comparing the swap against +itself. + +| mutation | file, before → after (sha256, 16) | Δ | verdict | +|---|---|---|---| +| C1 `allocator.py:154` span↔why | `10c1b26ed0099654` → `ac0c174de1ec28ad` | 0 | **SURVIVED 21/21** | +| C2 `allocator.py:160` span↔why | `10c1b26ed0099654` → `9b10730f4659befd` | 0 | **SURVIVED 21/21** | +| C3 `allocator.py:162` span↔why | `10c1b26ed0099654` → `07097bf5da31f0c6` | 0 | **RED** — `test_alloc.py` | +| C4a `arbiter.py:344` lo↔hi | `fdb4ed0520eeb721` → `009820de7ef0eff0` | 0 | **SURVIVED 21/21** | +| C4b `arbiter.py:344` lo_src↔hi_src | `fdb4ed0520eeb721` → `04cb8cf69dfc7432` | 0 | **SURVIVED 21/21** | +| C5 `arbiter.py:392` winner↔bound_by | `fdb4ed0520eeb721` → `9238c3db920da8ad` | 0 | **SURVIVED 21/21** | +| **C6** `clamp.py:253` requested↔applied | `0153133741497fa5` → `47e4e371043e3201` | 0 | **SURVIVED 21/21 before — RED after**, `test_clamp.py`, 4 checks | +| **C7** `peakguard.py:493` projected↔ceiling | `d95df61a51716c26` → `2d2ee3d0bcd51251` | 0 | **SURVIVED 21/21 before — RED after**, `test_peakguard.py`, 3 checks | +| C8 `tariff.py:279` month↔detail | `82790523dd5227c9` → `4b2147ca17a28425` | 0 | **SURVIVED 21/21** | +| C9 `tariff.py:528` projected↔elapsed | `82790523dd5227c9` → `517e9148d26a243a` | 0 | **SURVIVED 21/21** | +| B17 `arbitrage.py:377` spread↔required | `67fbf949b2938349` → `cddd8045bdeb5e29` | 0 | **SURVIVED 21/21** | +| B18 `arbitrage.py:393` spread↔required | `67fbf949b2938349` → `04a4f3ad3d255de1` | 0 | **SURVIVED 21/21** | +| B19 `main.py:2652` reason↔soc_floor_reason | `61aa413fb890433e` → `bbcbb28f476a98d0` | 0 | **SURVIVED 21/21** | +| **B12** `arbiter.py:264` source↔actuator | `fdb4ed0520eeb721` → `9111f627ef639ecc` | 0 | **SURVIVED 21/21 before — RED after**, `test_arbiter.py`, 2 checks | + +**14 measured, 12 survived the full suite before this round's checks, 1 was +already caught, 3 are caught now.** The tree these were taken on is +`SAFETY-38` @ `357cb4f` rebased on `release/1.0` @ `9e42cec`; `app/main.py` is +234 542 bytes. + +### 7g. What round 4 does NOT close, with sizes + +* **Eleven measured zero-byte survivors are left open** — C1, C2, C4a, C4b, C5, + C8, C9, B17, B18, B19 and the three already open from round 3 (A3, B1, and the + B2–B11/B13–B14 source-level set). Each is a sentence an operator reads and + none is a control decision. They are disclosed with their measurement rather + than pinned because pinning each one wants a fixture in the file that owns the + module — `test_alloc.py`, `test_arbiter.py`, `test_tariff.py`, + `test_arbitrage.py` — and this card has now spent four rounds inside + `test_watchdog.py`'s subject. **An enumerated list with honest gaps beats a + fifth round finding a fifth instance**; what would NOT be honest is a fifth + round finding one that is **not on this list**. +* **`main.py:1951`, the action-log surface of the peak-guard idle line,** is + unpinned. The status row off the same `explain()` is pinned; the second call + site is not, and §4c is the card about exactly that costing a round. +* **The largest remaining hole is not in any census: `%s`-style lazy logging.** + `_LOG.error("… %s … %s", a, b)` puts the two operands in *arguments*, not in + an f-string, so nothing here looks at them and the transposition is the same + zero-byte edit. The tool now says so first in its NOT-SCANNED list, **with + the size measured rather than asserted: 31 such calls across 8 modules in + `app/`, each one listed**. +* **The tool's own blind-spot list was the round-4 blocker and is now two + lists.** "What this scans, and what it does not" enumerates its message sinks + from the constants themselves and then names, in order, what it never looks at: + `%s`-style logging, `.format()`/`%`/`+`/`.join()` sentences, a sentence that + leaves through a dict or dataclass and is rendered elsewhere, `yield`ed + sentences, attributes outside the `reason`-shaped set, and everything outside + `app/`. "What it can see but gets wrong" keeps the five gaps round 3 printed, + with the over-report gap now backed by a **printed format-spec column** so a + reader can tell a silent same-typed transposition from a noisy mixed-type one. diff --git a/goodwe_controller/MUTATION.md b/goodwe_controller/MUTATION.md index af5e47c..226a792 100644 --- a/goodwe_controller/MUTATION.md +++ b/goodwe_controller/MUTATION.md @@ -135,7 +135,7 @@ 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. TEN instances in two weeks; four +It is not hypothetical and it is not rare. ELEVEN instances in two weeks; four were found by review, two - CFG-04's fixture pair - by an author's own mutation battery, which is the cheaper place, one by review again, in the one place nobody had thought to point the rule at: not a check in the suite, but a @@ -145,6 +145,15 @@ that names the rule in its own text - and the TENTH INSIDE THE FIX WRITTEN TO CLOSE THE NINTH, for the second time in this list. See SAFETY-38 below: the check that closed it asserted a NUMBER and not the number's POSITION. +⚠️ AND THE ELEVENTH IS IN A MEASURING INSTRUMENT RATHER THAN IN A CHECK, which +is where the seventh was too. SAFETY-38 round 4's render probe - the thing that +decides whether a mutation changed the SENTENCE and not merely the bytes - +numbered each interpolated operand BY ORDER OF APPEARANCE. After a transposition +the other operand takes the same number, so all thirteen mutant renders came back +BYTE-IDENTICAL to the clean tree: the probe compared the swap against itself, and +its output read exactly like "the edit is inert". Keying the value to the +operand's NAME fixed it. Habit 16. + ⚠️ **AND IT HAS NOW APPEARED IN SIX CONSECUTIVE CARDS.** The seventh instance, MAINT-05's, is the one that says why: the defect is not carelessness about fixtures, it is that **a check and the thing it checks were built from the same @@ -494,6 +503,48 @@ Cheap habits fall out of it, and all of them are now used in this repository: are in `DECISIONS/SAFETY-38-site-standing-pin.md` §7 — **with two measured, still-open survivors named there rather than rounded off**, both zero bytes and both 21/21 green. + * ⚠️ **AND THE SINK IS HALF THE SCOPE, WHICH IS HOW THE FIRST THREE ROUNDS + OF THIS SCAN MISSED THE PLACE THE CODEBASE ACTUALLY KEEPS ITS SENTENCES.** + The scan classified a sentence as operator-facing when it was **appended, + assigned to a `reason`-shaped name, or logged** — and every one of its 16 + reported members was one of those. **None was RETURNED.** A method + formatting its own **attributes** and handing the string back — + `ClampResult.describe()`, `PeakDecision.explain()`, every reason-builder + in the tree — was outside all of it, and so were sentences handed to a + constructor as `reason=` or placed under a `"text"` dict key. Four + zero-byte survivors of a green 21-file suite were sitting there, two of + them on the alarm SAFETY-40 and SAFETY-41 are about. **The general form: + an enumeration is scoped by a SINK as much as by a PREDICATE, and a sink + list is a hand-written list like any other (habit 12).** Ask of any such + scan: *which syntactic forms does this call a message, and what does the + code under it actually use?* + +16. ⚠️ **AN INSTRUMENT THAT ASSERTS A COMPLETENESS IT DOES NOT HAVE IS WORSE + THAN NO INSTRUMENT, AND ITS BLIND-SPOT LIST IS PART OF THE CODE UNDER + REVIEW.** `tools/symmetric_message_scan.py` printed five honest gaps and a + census count, and a reader — including its author, twice — took the count + for the family. The gap that mattered, the RETURN sink, was **in none of the + five**. A gap list is not a disclaimer; it is a claim, and it is the claim + most likely to be believed because nothing else in the output argues with + it. Two consequences: + * **State what is NOT scanned, largest hole first, as a list of forms** — + not "this under-reports". The tool now names `%s`-style lazy logging + (`_LOG.error("… %s … %s", a, b)`, where the operands are *arguments*, so + no f-string census can ever see them), `.format()`/`%`/`+`/`.join()` + sentences, sentences that leave through a dict or dataclass, `yield`, + message attributes outside its own constant set, and everything outside + `app/`. Where the list is derived from a constant in the file, it is + **printed from that constant**, so the two cannot drift. + * **AND THE PROBE THAT GRADES THE MUTATIONS IS SUBJECT TO EVERY HABIT + ABOVE.** Round 4's render probe gave each interpolated operand a value by + **order of appearance** — so after a transposition the other operand took + the same number and **all thirteen mutant renders came back + byte-identical**, which reads exactly like *the edit changed nothing*. + Keying the value to the operand's **NAME** fixed it. That is habit 4, in + the measuring instrument: the fixture's own values were arranged so the + failure it was built to see could not change the answer. **Eleventh + instance of habit 4 in this file, and the second one in an instrument + rather than in a check.** 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/test_arbiter.py b/goodwe_controller/test_arbiter.py index d8826f3..eff6ccb 100644 --- a/goodwe_controller/test_arbiter.py +++ b/goodwe_controller/test_arbiter.py @@ -449,9 +449,32 @@ mis_peak = SITE_SIDE[:1] + [Claim.limit("peak", P_PEAK, lo=1500.0, r_mis = resolve(mis_peak) print(f" peak floor misfiled on inverter-b: {r_mis.explain()} " f"scope_errors={r_mis.scope_errors}") -check("the mis-routing is CAUGHT: scope_violations names the source and the " - "actuator it does not belong on", len(r_mis.scope_errors) == 1 - and "peak" in r_mis.scope_errors[0]) +# ⚠️ THE ORDERED PAIR, AND THE CHECK THAT USED TO BE HERE WAS WIDER THAN ITS +# OWN CLAIM. It said `scope_violations` "names the source and the actuator it +# does not belong on" and then asserted `"peak" in r_mis.scope_errors[0]`. +# `arbiter.py:264` builds that sentence as +# `f"{c.source}: a SITE claim filed on unit {c.actuator!r} - …"`; transpose the +# two - zero bytes, no decision changes, SAFETY-38 census B row B12 - and it +# reads "inverter-b: a SITE claim filed on unit 'peak'", naming the misfiled +# claim as the actuator and the actuator as the claim. `"peak" in …` is STILL +# TRUE under that, so the old line graded neither half of what it claimed. +# The operands are read off the CLAIM this test built, not off the message. +_mis_c = mis_peak[-1] +check("PRECONDITION: the misfiled claim's source and actuator are DIFFERENT " + "STRINGS, so a transposition genuinely produces a different sentence", + _mis_c.source != _mis_c.actuator and _mis_c.source == "peak" + and _mis_c.actuator == "inverter-b") +check("the mis-routing is CAUGHT, and scope_violations names the source FIRST " + "and the actuator it does not belong on SECOND - the ORDERED PAIR on the " + "contiguous literal, because a substring test finds either name under a " + "transposition that swaps their roles", + len(r_mis.scope_errors) == 1 + and r_mis.scope_errors[0].startswith( + f"{_mis_c.source}: a SITE claim filed on unit {_mis_c.actuator!r}")) +check("...and the TRANSPOSED sentence is not what it says, so the check above " + "discriminates rather than decorates", + f"{_mis_c.actuator}: a SITE claim filed on unit {_mis_c.source!r}" + not in r_mis.scope_errors[0]) check("...and the reason it must be caught, measured rather than argued: with " "the floor on a unit the SITE has no floor, so the loop charges 1500 W " "into the quarter the guard was trying to shave", diff --git a/goodwe_controller/test_clamp.py b/goodwe_controller/test_clamp.py index 1968880..38d5ca1 100644 --- a/goodwe_controller/test_clamp.py +++ b/goodwe_controller/test_clamp.py @@ -1000,6 +1000,105 @@ check("...and main.py:856's consumer is the line that goes quietly false", bool(ctl_w.clamp_last and ctl_w.clamp_last.engaged)) +# --------------------------------------------------------------------------- # +# 6d. SAFETY-38 round 4 - THE ORDERED PAIR INSIDE `ClampResult.describe()` +# --------------------------------------------------------------------------- # +# ⚠️ THIS IS THE ALARM THE OWNER HAS BEEN COMPLAINING ABOUT ALL DAY, AND UNTIL +# THIS BLOCK NO test_*.py MENTIONED `describe()` AT ALL. `clamp.py:253` builds +# the whole §15 action-log row and the whole ClampWatchdog notification reason +# out of its own attributes: +# +# f"{REASON_CODE}: the computed setpoint {self.requested_w:+.0f} W was +# clamped to {self.applied_w:+.0f} W by the {self.bound} bound +# (rating {self.limit_w:.0f} W) - something upstream is winding" +# +# Transpose `requested_w` and `applied_w` and the guard reports clamping a +# setpoint UPWARD - here, to +9999 W by a bound whose rating the SAME SENTENCE +# gives as 2000 W. MEASURED on this tree: a zero-byte edit (22 507 bytes before, +# 22 507 after, sha256 0153133741497fa5 -> 47e4e371043e3201), the emitted +# sentence changes, and the full 21-file suite stayed GREEN. Nothing decides on +# the order, so nothing behavioural can ever see it - which is why the check +# below has to be on the ORDERED PAIR and on the CONTIGUOUS LITERAL. A substring +# test for either number passes under the swap; both numbers are still there. +# +# ⚠️ AND THE OPERANDS ARE READ OFF THE PLANT, NOT OFF `clamp_last`. `ctl.target` +# is what the loop asked for and `hass.writes[-1]` is what the device was +# actually sent; neither is a `ClampResult` field. Grading the sentence against +# `result.requested_w` would compare the swap against itself - DEV-02's defect, +# and the same one that made the first cut of this card's own render probe emit +# thirteen empty diffs. + + +async def _sentence(max_w, dev, wanted): + """One real write over `max_w`, then the guard, on a real Controller.""" + hass = _Hass(0.0) + c = Controller({"setpoint_entity": "number.sp", "meter_source": "off", + "max_w": max_w}, + hass, _Store(), _Pub(), stop=asyncio.Event()) + c.auto = True + c.dev_min, c.dev_max = -dev, dev + c.target = wanted + arm(c, wanted) + await c.write_setpoint(wanted) + await c.check_output_clamp() + return hass, c + + +hass_x, ctl_x = _sentence_run = asyncio.run(_sentence(2000.0, 5000.0, 9999.0)) +notes_x = clamp_notes(hass_x) +_asked_x = ctl_x.target # what the loop asked for +_sent_x = hass_x.writes[-1] # what the device was actually sent +_rating_x = ctl_x.inverters[0].max_w # the bound the sentence names +_rows_x = [e for e in ctl_x.events if REASON_CODE in e] +print(f" max_w {_rating_x:.0f} on a +-5000 W device, asked for " + f"{_asked_x:+.0f} W: the device saw {_sent_x:+.0f} W") +print(f" action log: {_rows_x[0] if _rows_x else '(none)'}") +check("PRECONDITION: the write really clamped on the MAX_W bound, and the two " + "operands are DIFFERENT NUMBERS that RENDER differently - equal ones " + "would make the transposed sentence identical and every check below " + "vacuous. The asked-for magnitude is also strictly ABOVE the rating " + "while the sent one is not, which is what makes the swapped sentence " + "self-contradictory rather than merely wrong", + ctl_x.clamp_last is not None and ctl_x.clamp_last.engaged + and ctl_x.clamp_last.bound == "max_w" + and _asked_x != _sent_x + and f"{_asked_x:+.0f}" != f"{_sent_x:+.0f}" + and abs(_sent_x) <= _rating_x < abs(_asked_x)) +check("⚠️ THE ORDERED PAIR IN THE §15 ACTION-LOG ROW (main.py:1366). The " + "contiguous literal names the setpoint the loop COMPUTED first and the " + "one that REACHED THE WIRE second. Under the transposition this row says " + "the loop asked for +2000 W and the guard clamped it UP to +9999 W - " + "past the 2000 W rating printed four words later - which is the alarm " + "reading backwards on the one line an operator is woken by", + any(f"the computed setpoint {_asked_x:+.0f} W was clamped to " + f"{_sent_x:+.0f} W" in e for e in _rows_x)) +check("⚠️ ...AND IN THE ClampWatchdog NOTIFICATION REASON (main.py:2452), " + "which is a SECOND surface off the same sentence and has to be pinned " + "separately - SAFETY-38 round 3's whole lesson was that pinning one call " + "site leaves the other shipping", + bool(notes_x) + and f"the computed setpoint {_asked_x:+.0f} W was clamped to " + f"{_sent_x:+.0f} W" in notes_x[0]["message"]) +check("...and the TRANSPOSED sentence appears on NEITHER surface, so the two " + "checks above discriminate rather than decorate", + not any(f"the computed setpoint {_sent_x:+.0f} W was clamped to " + f"{_asked_x:+.0f} W" in e for e in _rows_x) + and f"the computed setpoint {_sent_x:+.0f} W was clamped to " + f"{_asked_x:+.0f} W" not in notes_x[0]["message"]) +# The DEVICE bound, off part 6c's controller: same sentence, other stage. The +# unit of coverage is the call site, and `bound` is the only thing that differs +# between the two rows an operator can receive. +_asked_w, _sent_w = ctl_w.target, hass_w.writes[-1] +check("...and the same ordered pair holds on the DEVICE-bound row, where the " + "loop asked for +3000 W and the device took +2500 W. PRECONDITION: those " + "two also render differently", + f"{_asked_w:+.0f}" != f"{_sent_w:+.0f}" + and any(f"the computed setpoint {_asked_w:+.0f} W was clamped to " + f"{_sent_w:+.0f} W" in e for e in ctl_w.events) + and f"the computed setpoint {_asked_w:+.0f} W was clamped to " + f"{_sent_w:+.0f} W" in notes_w[0]["message"]) + + # ============================================================================== print() print("7. SAFETY-28: the write path is gated on the allocation record") diff --git a/goodwe_controller/test_peakguard.py b/goodwe_controller/test_peakguard.py index 51cb72a..380a4c0 100644 --- a/goodwe_controller/test_peakguard.py +++ b/goodwe_controller/test_peakguard.py @@ -5774,6 +5774,86 @@ check("...and so is the last one, which is not a defeat at all but the EDGE OF " not MUT[12][1] and EV["min_soc"] <= BMS_FLOOR + 0.05) +# ============================================================================== +print() +print("13. SAFETY-38 round 4: the ORDERED PAIR inside PeakDecision.explain()") + +# ⚠️ THE STRING "peak guard idle" APPEARED IN NO test_*.py UNTIL THIS PART. +# `peakguard.py:493` formats its OWN ATTRIBUTES and RETURNS the sentence: +# +# f"peak guard idle - quarter projects {self.projected_w:.0f} W under a " +# f"{self.ceiling_w:.0f} W ceiling ({self.ceiling_from})" +# +# Transpose the two and this fixture's row reads "projects 5013 W under a +# 2300 W ceiling" - a quarter projected 2713 W OVER the ceiling, printed on a +# row whose own words say the guard is IDLE and whose own flag says ok. +# MEASURED on this tree: zero-byte edit (50 032 bytes +# before, 50 032 after, sha256 d95df61a51716c26 -> 2d2ee3d0bcd51251), the +# emitted sentence changes, and the full 21-file suite stayed GREEN. +# +# The sentence was invisible to SAFETY-38's scanner for two rounds because both +# censuses scored one syntactic sink - APPEND, ASSIGN, LOG ARG - and this one is +# RETURNED. `tools/symmetric_message_scan.py` census C now scans the return sink +# and resolves it through the method's call sites; it rediscovered this pair and +# clamp.py's on its own, which is the only reason to trust the rest of its list. +# +# ⚠️ THE OPERANDS COME OFF THE PROJECTION AND OFF `quarter_ceiling_w()`, NOT OFF +# THE DECISION. `d.projected_w` and `d.ceiling_w` are the very fields the swap +# exchanges, so grading the sentence against them would compare the swap against +# itself and pass under it. +_S38_PROJ = PeakTracker.project(averager_at(CAP_W - 1200.0, 450.0), + B + timedelta(seconds=450)) +_S38_CEIL_W, _S38_CEIL_FROM = quarter_ceiling_w(CAP_W, obs_hi) +_S38_IDLE = decide(_S38_PROJ, batt_w=0.0, cap_w=CAP_W, reading=obs_hi, + achievable_w=LIVE_MAX_W, max_hold_s=MAX_HOLD_S, + soc=SOC_OK, soc_floor=SOC_FLOOR) +_S38_LITERAL = (f"quarter projects {_S38_PROJ.projected_w:.0f} W under a " + f"{_S38_CEIL_W:.0f} W ceiling") +_S38_SWAPPED = (f"quarter projects {_S38_CEIL_W:.0f} W under a " + f"{_S38_PROJ.projected_w:.0f} W ceiling") +print(f" projection {_S38_PROJ.projected_w:.0f} W against a " + f"{_S38_CEIL_W:.0f} W ceiling ({_S38_CEIL_FROM})") +print(f" {_S38_IDLE.explain()!r}") +check("PRECONDITION: the guard is IDLE with a projection to print, the ceiling " + "came from the MONTH PEAK rather than the cap, and the two operands are " + "DIFFERENT NUMBERS that RENDER differently. Equal operands would make the " + "transposed sentence identical and every check below decoration", + not _S38_IDLE.engaged and _S38_IDLE.projected_w is not None + and _S38_CEIL_FROM == CEILING_MONTH_PEAK + and _S38_PROJ.projected_w != _S38_CEIL_W + and f"{_S38_PROJ.projected_w:.0f}" != f"{_S38_CEIL_W:.0f}" + and _S38_PROJ.projected_w < _S38_CEIL_W) +check("⚠️ THE ORDERED PAIR ON THE CONTIGUOUS LITERAL. The PROJECTION is named " + "first and the CEILING second; a substring test for either number passes " + "under the transposition because both numbers are still in the sentence, " + "so only the ordered pair can tell the two slots apart (MUTATION.md " + "habit 14)", _S38_LITERAL in _S38_IDLE.explain()) +check("...and the TRANSPOSED sentence is NOT what it says, so the check above " + "discriminates rather than decorates", + _S38_SWAPPED not in _S38_IDLE.explain()) +check("...and the row still names WHERE the ceiling came from, which is the " + "difference between a cap an operator set and a peak the month already " + "banked", f"ceiling ({CEILING_MONTH_PEAK})" in _S38_IDLE.explain()) + +# THE SHIPPED CONSUMER. `explain()` is not read by anybody until main.py prints +# it, and round 3's lesson is that the pin belongs on the CALL SITE: the status +# row at main.py:3313 is what a browser shows, and the action-log line at +# main.py:1951 is what the release edge writes. +_c38, _h38 = built(TMP / "s38pin.db") +_c38.peak_guard = _S38_IDLE +_rows38 = [r for r in _c38.checks() if "peak guard" in r["text"]] +print(" status row: " + (_rows38[0]["text"] if _rows38 else "(none)")) +check("⚠️ ...AND THE SAME ORDERED PAIR ON THE STATUS PAGE (main.py:3313), read " + "off the shipped `checks()` rather than off `explain()` a second time. " + "This is the surface the owner actually looks at", + len(_rows38) == 1 and _S38_LITERAL in _rows38[0]["text"] + and _S38_SWAPPED not in _rows38[0]["text"]) +check("...and the row is green and un-warned, which is what makes the " + "transposed version dangerous rather than merely untidy: an OK row " + "carrying a projection 2713 W OVER its own ceiling is the one an " + "operator scrolls past", _rows38[0]["ok"] and not _rows38[0]["warn"] + and _S38_CEIL_W - _S38_PROJ.projected_w == 2713.0) + # ============================================================================== print() shutil.rmtree(TMP, ignore_errors=True) diff --git a/goodwe_controller/tools/symmetric_message_scan.py b/goodwe_controller/tools/symmetric_message_scan.py index 2a22192..c6f5080 100644 --- a/goodwe_controller/tools/symmetric_message_scan.py +++ b/goodwe_controller/tools/symmetric_message_scan.py @@ -228,6 +228,20 @@ def _sink_of(fn, target, follow=True): up): return "MESSAGE" return "OTHER" + # ⚠️ ROUND 4. A SENTENCE HANDED TO A CONSTRUCTOR AS `reason=` IS A + # MESSAGE, and scoring only ASSIGNMENTS missed every one of them. This + # codebase builds most of its operator sentences that way - + # `ArbitrageDecision(action=..., reason=f"...")`, + # `out.append({"text": f"..."})` - so the sentence never touches a name + # this walker was watching. arbitrage.py's two DISCHARGE/CHARGE reasons + # were scored OTHER for exactly this and are genuine members. + if isinstance(up, ast.keyword) and up.arg in _MSG_ATTRS: + return "MESSAGE" + if isinstance(up, ast.Dict): + for k, v in zip(up.keys, up.values): + if (v is cur and isinstance(k, ast.Constant) + and k.value in _MSG_ATTRS): + return "MESSAGE" if isinstance(up, ast.Call): name = (up.func.attr if isinstance(up.func, ast.Attribute) else up.func.id if isinstance(up.func, ast.Name) else "") @@ -377,6 +391,7 @@ def scan(): # inventing members. + class _Blank(ast.NodeTransformer): def visit_Attribute(self, node): self.generic_visit(node) @@ -393,38 +408,196 @@ def _blanked(expr): return ast.dump(_Blank().visit(ast.parse(ast.unparse(expr)))) -def scan_caller(): - rows = [] +def _called_name(func): + return (func.attr if isinstance(func, ast.Attribute) + else func.id if isinstance(func, ast.Name) else "") + + +def _innermost(tree): + """{id(node): INNERMOST enclosing function}, or None at module level. + + ⚠️ NOT `for fn in ast.walk(tree): for node in ast.walk(fn)`, which is what + the first version of census B did. That visits every sentence once per + ENCLOSING def, so a sentence inside a nested function was classified twice - + once against the inner scope and once against the outer - and the two + classifications are not obliged to agree. The rows were de-duplicated + afterwards, which hid the double visit rather than removing it. + """ + m = {} + + def rec(node, fn): + for child in ast.iter_child_nodes(node): + f = (child if isinstance(child, (ast.FunctionDef, + ast.AsyncFunctionDef)) else fn) + m[id(child)] = f + rec(child, f) + + rec(tree, None) + return m + + +def _spec(fv): + """The FORMAT SPEC of one interpolation, as text; '' when there is none. + + ⚠️ IT IS PRINTED BESIDE EVERY PAIR BECAUSE IT IS THE ONE CHEAP TYPE SIGNAL + THIS SCAN HAS, and gap 4 of the second list below is exactly about the pairs + that do not share + one. `{self.requested_w:+.0f}` beside `{self.applied_w:+.0f}` is two floats + rendered identically - a transposition there is silent and reads as a fact. + `{self.limit_w:.0f}` beside `{self.bound}` is a float beside a string: a + transposition there prints `rating max_w W`, which is visibly broken rather + than quietly wrong. Same row in this census, very different hazard, and the + reader cannot tell them apart without this column. + """ + if fv.format_spec is None: + return "" + return "".join(str(v.value) if isinstance(v, ast.Constant) else "{...}" + for v in fv.format_spec.values) + + +def _same_shape_pairs(node): + """Pairs of interpolations in ONE f-string that are exactly one leaf apart.""" + fvs = [v for v in node.values if isinstance(v, ast.FormattedValue)] + for x, y in itertools.combinations(fvs, 2): + ua, ub = ast.unparse(x.value), ast.unparse(y.value) + if ua == ub: + continue + if _blanked(x.value) != _blanked(y.value): + continue + yield ua, ub, _spec(x), _spec(y) + + +def _sentences(): + """(path, innermost fn, JoinedStr, sink) for every f-string in app/.""" for path in sorted(APP.glob("*.py")): tree = ast.parse(path.read_text()) - for fn in ast.walk(tree): - if not isinstance(fn, (ast.FunctionDef, ast.AsyncFunctionDef)): + inner = _innermost(tree) + for node in ast.walk(tree): + if not isinstance(node, ast.JoinedStr): continue - for node in ast.walk(fn): - if not isinstance(node, ast.JoinedStr): - continue - # ⚠️ MESSAGE SINKS ONLY, AND THE FIRST VERSION OF THIS - # CENSUS SAID "anything but a RETURN". That let in - # `p1.py`'s `self.url = f"http://{self.host}:{self.port}…"` - - # an f-string that is DATA, used to make a request, where a - # transposition is an ordinary behavioural change every check - # can see. Same distinction as census A's RETURN bucket: a - # formatted string is only in this family when its only - # consumer is a person. - if _sink_of(fn, node) != "MESSAGE": - continue - exprs = [v.value for v in node.values - if isinstance(v, ast.FormattedValue)] - for a, b in itertools.combinations(exprs, 2): - ua, ub = ast.unparse(a), ast.unparse(b) - if ua == ub: - continue - if _blanked(a) == _blanked(b): - rows.append((path.name, node.lineno, fn.name, ua, ub)) - # de-duplicate: the same sentence reached through several enclosing defs + fn = inner.get(id(node)) + if fn is None: + continue + yield path, fn, node, _sink_of(fn, node) + + +def scan_caller(): + rows = [] + for path, fn, node, sink in _sentences(): + # ⚠️ MESSAGE SINKS ONLY, AND THE FIRST VERSION OF THIS CENSUS SAID + # "anything but a RETURN". That let in `p1.py`'s + # `self.url = f"http://{self.host}:{self.port}…"` - an f-string that is + # DATA, used to make a request, where a transposition is an ordinary + # behavioural change every check can see. A formatted string is only in + # this family when its only consumer is a person. RETURN is not OTHER + # though, and treating it as if it were is what census C below fixes. + if sink != "MESSAGE": + continue + for ua, ub, sa, sb in _same_shape_pairs(node): + rows.append((path.name, node.lineno, fn.name, ua, ub, sa, sb)) return sorted(set(rows), key=lambda r: (r[0], r[1], r[3])) +# ============================================================================= +# CENSUS C - THE SENTENCE THE OBJECT RETURNS ABOUT ITSELF +# ============================================================================= +# ⚠️ SAFETY-38 ROUND 4. CENSUS A AND CENSUS B BETWEEN THEM COVERED ONE +# SYNTACTIC SINK AND CALLED IT THE FAMILY. Every member either census reported +# was a sentence APPENDED, ASSIGNED to a `reason`-shaped attribute, or passed as +# a LOG ARGUMENT. Not one was a sentence that is RETURNED. Census A had a RETURN +# bucket, but only for a callee formatting its own PARAMETERS - so a method +# formatting its own ATTRIBUTES and handing the string back was invisible to +# both, and `describe()` / `explain()` / the reason-builders are precisely where +# this codebase keeps its operator sentences. Two live members were sitting in +# that hole: +# +# clamp.py ClampResult.describe() requested_w <-> applied_w +# peakguard.py PeakDecision.explain() projected_w <-> ceiling_w +# +# Worse than the gap: it was in NONE of the five blind spots this file prints +# about itself. A tool that asserts a completeness it does not have is the +# recurring defect in a new hat, and the list at the bottom now names its sinks. +# +# THE TEST is census B's - one f-string, two interpolations one leaf apart - run +# over the RETURN sink, plus one thing census B never needed: A RETURNED STRING +# IS NOT YET A MESSAGE. `store.unit_key()` and `mqtt.unit_state_key()` return +# f-strings too, and swapping there renames a store key and an MQTT topic, which +# every ordinary check can see (MAINT-05 is the card about that). So the sink of +# a returned sentence is RESOLVED THROUGH ITS CALL SITES: the string is in this +# family only when some caller in `app/` feeds it to an operator. + + +def _return_consumers(func_name, depth=0, seen=frozenset()): + """[(site, sink, verdict)] - where the STRING RETURNED BY `func_name` goes. + + One hop is not enough: `Resolution.explain()` is returned again by wrappers + that are themselves logged, so a call site whose own sink is RETURN is + resolved by recursing on the ENCLOSING function's name, bounded by `depth` + and by `seen` so mutual recursion cannot loop. + + ⚠️ THE MATCH IS ON THE BARE METHOD NAME, exactly as `call_sites()` above, + and `explain` is the name of at least four unrelated methods in this tree + (`PeakDecision`, `UnitPlan`, `Resolution`, the maintenance migration + report). So this OVER-collects sites, which pushes a verdict towards + MESSAGE. That is the deliberate direction for a census whose measured + failure has twice been under-reporting - but it means the verdict is a + PROMPT TO READ THE SITES PRINTED BESIDE IT, not a proof. + """ + if func_name in seen or depth > 2: + return [] + seen = seen | {func_name} + out = [] + for path in sorted(APP.glob("*.py")): + tree = ast.parse(path.read_text()) + inner = _innermost(tree) + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + if _called_name(node.func) != func_name: + continue + fn = inner.get(id(node)) + if fn is None: + continue + sink = _sink_of(fn, node) + label = f"{path.name}:{node.lineno} `{ast.unparse(node.func)}`" + if sink == "RETURN": + sub = _return_consumers(fn.name, depth + 1, seen) + if any(v == "MESSAGE" for _l, _s, v in sub): + verdict = "MESSAGE" + elif not sub: + verdict = "UNRESOLVED" + else: + verdict = "OTHER" + sink = f"RETURN by {fn.name}() -> {verdict}" + else: + verdict = sink + out.append((label, sink, verdict)) + return sorted(set(out)) + + +def scan_returned(): + """Census C rows: a returned sentence, two interpolations one leaf apart.""" + rows = [] + for path, fn, node, sink in _sentences(): + if sink != "RETURN": + continue + pairs = list(_same_shape_pairs(node)) + if not pairs: + continue + sites = _return_consumers(fn.name) + if any(v == "MESSAGE" for _l, _s, v in sites): + verdict = "MESSAGE" + elif not sites: + verdict = "UNRESOLVED - no call site inside app/" + else: + verdict = "OTHER" + for ua, ub, sa, sb in pairs: + rows.append(dict(module=path.name, line=node.lineno, func=fn.name, + a=ua, b=ub, spec_a=sa, spec_b=sb, + verdict=verdict, sites=sites)) + return sorted(rows, key=lambda r: (r["module"], r["line"], r["a"])) + + # ⚠️ THE UNIT OF COVERAGE IS THE CALL SITE, NOT THE FUNCTION - which is the # whole lesson of SAFETY-38 round 3. `SignWatchdog.evaluate()` had its operand # order pinned and the defect still shipped, because the pin was on the call two @@ -437,22 +610,61 @@ def call_sites(name): for node in ast.walk(tree): if not isinstance(node, ast.Call): continue - called = (node.func.attr if isinstance(node.func, ast.Attribute) - else node.func.id if isinstance(node.func, ast.Name) - else "") - if called == name: - # ⚠️ THE RECEIVER IS PRINTED, because matching on the bare - # attribute name conflates methods: `evaluate` is the name of - # SAFETY-02's, SAFETY-18's and SAFETY-31's watchdogs alike, and - # a list of four lines with no receiver tells a reader nothing - # about which detector they are looking at. `inv.sign_wd` and - # `self.freeze_wd` are the disambiguation, and they are the - # caller's own words. - out.append(f"{path.name}:{node.lineno} " - f"`{ast.unparse(node.func)}`") + if _called_name(node.func) != name: + continue + # ⚠️ THE RECEIVER IS PRINTED, because matching on the bare + # attribute name conflates methods: `evaluate` is the name of + # SAFETY-02's, SAFETY-18's and SAFETY-31's watchdogs alike, and + # a list of four lines with no receiver tells a reader nothing + # about which detector they are looking at. `inv.sign_wd` and + # `self.freeze_wd` are the disambiguation, and they are the + # caller's own words. + out.append(f"{path.name}:{node.lineno} `{ast.unparse(node.func)}`") return out +def lazy_log_sites(): + """Log calls with a %-format template and >=2 substitution ARGUMENTS. + + ⚠️ THE SIZE OF THE LARGEST HOLE, MEASURED RATHER THAN ASSERTED. The + NOT-SCANNED list below used to end "and app/ uses that form", which is a + claim about the tree with no number on it - the exact shape this card has + spent four rounds arguing against. `_LOG.error("%s ... %s", a, b)` puts both + operands in ARGUMENTS, so no f-string census here can ever reach them, and + transposing the two costs the same zero bytes. This counts them. + """ + out = [] + for path in sorted(APP.glob("*.py")): + tree = ast.parse(path.read_text()) + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + if _called_name(node.func) not in _LOG_SINKS or len(node.args) < 3: + continue + head = node.args[0] + tmpl = "" + if isinstance(head, ast.Constant) and isinstance(head.value, str): + tmpl = head.value + elif isinstance(head, ast.BinOp) or isinstance(head, ast.JoinedStr): + tmpl = ast.unparse(head) + if tmpl.count("%") >= 2: + out.append((path.name, node.lineno, + f"{path.name}:{node.lineno} " + f"`{ast.unparse(node.func)}` with " + f"{len(node.args) - 1} args")) + return [r[2] for r in sorted(out)] + + +def _pair_line(sa, sb): + same = sa == sb + return (f" format spec: '{sa}' / '{sb}' -> " + + ("SAME - two values rendered alike, so a transposition is " + "SILENT and reads as a fact" + if same else + "DIFFERENT - likely mixed types, so a transposition is more " + "likely to look broken than to lie (gap 4 below)")) + + def main(): quiet = "--quiet" in sys.argv rounds = build_fixpoint() @@ -515,50 +727,151 @@ def main(): "ORDERED PAIR.") cb = scan_caller() print(f"candidates: {len(cb)}") - for mod, line, fn, ua, ub in cb: + for mod, line, fn, ua, ub, sa, sb in cb: print(f" {mod}:{line} in {fn}()") print(f" {ua} <-> {ub}") + print(_pair_line(sa, sb)) print() - print("WHAT THIS CANNOT SEE - the shape of each gap, named rather than " - "rounded off:") + print("=" * 78) + print("CENSUS C - SENTENCES AN OBJECT RETURNS ABOUT ITSELF (SAFETY-38 " + "round 4)") + print("A method formatting its OWN ATTRIBUTES and RETURNING the string was " + "in neither") + print("census above: census A only reads a callee's PARAMETERS, census B " + "only scored a") + print("MESSAGE sink. The verdict below is resolved through the CALL SITES " + "of the") + print("returning method - a returned string is only in this family once " + "some caller") + print("hands it to an operator (contrast store.unit_key(), whose return " + "value is a KEY).") + cc = scan_returned() + fam_c = [r for r in cc if r["verdict"] == "MESSAGE"] + hot = [r for r in fam_c if r["spec_a"] == r["spec_b"]] + print(f"returned sentences with a same-shape pair: {len(cc)} pair(s); " + f"{len(fam_c)} reach an operator;") + print(f"of those, {len(hot)} ALSO share a format spec - two values " + f"rendered alike, which is") + print("the hazard gap 4 below names: a transposition there is silent and " + "reads as a fact.") + print() + print("*** = reaches an operator AND shares a format spec - THE ACTIONABLE " + "SUBSET") + print(" ! = reaches an operator, specs differ (a swap is likelier to look " + "broken)") + print(" = does not reach an operator, or could not be resolved") + # grouped by the sentence, so the CONSUMERS list is printed once per + # method rather than once per pair - the ungrouped form was 20 identical + # lines under each of six pairs and nobody reads that. + for key in sorted({(r["module"], r["line"], r["func"]) for r in cc}): + mod, line, fname = key + group = [r for r in cc if (r["module"], r["line"], r["func"]) == key] + v = group[0]["verdict"] + print(f" {mod}:{line} {fname}() verdict: {v}") + for r in group: + mark = ("***" if r["verdict"] == "MESSAGE" + and r["spec_a"] == r["spec_b"] + else " ! " if r["verdict"] == "MESSAGE" else " ") + print(f" {mark} {r['a']} <-> {r['b']}") + print(f" {_pair_line(r['spec_a'], r['spec_b']).strip()}") + if not quiet: + sites = group[0]["sites"] + print(f" CONSUMERS ({len(sites)}) - bare-name match, so read " + f"the receiver:") + for label, sink, _v in sites or [("(none inside app/)", "", "")]: + print(f" {label} -> {sink}") + print() + print("=" * 78) + print("WHAT THIS SCANS, AND WHAT IT DOES NOT - stated because round 4's " + "blocker was") + print("a completeness this file ASSERTED and did not have. The sink " + "classifier is a") + print("syntactic walk up the parent chain plus ONE step of local data " + "flow, and it") + print("recognises exactly these sinks and no others:") + print(f" SCANNED as a MESSAGE : a call to {sorted(_LOG_SINKS)}") + print(" ...or to `append`; a `raise`; an " + "assignment to an") + print(f" attribute or local named {sorted(_MSG_ATTRS)}") + print(" ...or to a local that is then interpolated " + "into one of") + print(" those (the one data-flow step).") + print(" SCANNED as a RETURN : census C - resolved through the returning " + "method's") + print(" call sites, one hop plus two of recursion.") + print(" NOT SCANNED AT ALL, and each of these can hold a member:") + lazy = lazy_log_sites() + print(" * `%s`-style lazy logging - `_LOG.error('%s ... %s', a, b)`. The " + "arguments") + print(" are not an f-string, so no census here looks at them, and a " + "transposition") + print(" of two `%s` arguments is the same zero-byte edit. THIS IS THE " + "LARGEST") + print(f" REMAINING HOLE, and it is MEASURED, not asserted: " + f"{len(lazy)} such call(s)") + print(f" in app/, across " + f"{len({s.split(':')[0] for s in lazy})} module(s). None of them is " + f"scanned by anything") + print(" above. Listed here so the size of the hole is a number:") + for site in lazy or ["(none)"]: + print(f" {site}") + print(" * `.format()`, `%`-formatting, and sentences built by `+` or " + "`.join()` out of") + print(" non-f-string pieces.") + print(" * A sentence that leaves the function inside a DICT, LIST or " + "dataclass and is") + print(" rendered somewhere else - the one data-flow step is local and " + "follows only") + print(" a bare Name.") + print(" * `yield`ed sentences, and sentences stored on an object by any " + "attribute") + print(f" name outside {sorted(_MSG_ATTRS)}.") + print(" * Anything outside `app/`: `sim/`, `tools/`, `web.py`'s " + "templates, the") + print(" translations JSON, the firmware, the witness.") + print() + print("WHAT IT CAN SEE BUT GETS WRONG - the shape of each gap, named " + "rather than") + print("rounded off:") print(" 1. SEMANTIC symmetry no canonicaliser above spells. The list of " "commutative") print(" forms is finite and hand-written; a function symmetric only " "through a form") print(" not on it is REJECTED and never appears. This under-reports.") - print(" 2. In CENSUS B, two interpolated values of the same KIND that " - "are not the same") - print(" SHAPE - `a.lo` beside `hi`, or one of them behind a local " - "variable. Only") - print(" one-leaf-apart expressions are paired. `%s`-style logging " - "(`_LOG.error('%s") - print(" ... %s', a, b)`) and non-f-string payloads are not scanned at " - "all.") - print(" 3. Anything outside `app/` - `sim/`, `tools/`, the firmware, the " - "witness.") - print(" 4. WHETHER each call site is pinned. The sites are listed above; " + print(" 2. In CENSUS B and C, two interpolated values of the same KIND " + "that are not the") + print(" same SHAPE - `a.lo` beside `hi`, or one of them behind a local " + "variable.") + print(" Only one-leaf-apart expressions are paired.") + print(" 3. WHETHER each call site is pinned. The sites are listed above; " "whether a") print(" check asserts the ORDERED PAIR at each one is a fact about " "`test_*.py` and is") print(" recorded in DECISIONS/SAFETY-38-site-standing-pin.md section " "7. Call sites") - print(" outside `app/` are not listed either, and the site list matches " - "on the bare") - print(" METHOD NAME - three different watchdogs are all called " - "`evaluate`, so the") - print(" receiver is printed beside each one and a reader has to pick.") - print(" 5. ⚠️ IT CAN ALSO OVER-REPORT, IN ONE EXACT WAY. The sentence is " + print(" outside `app/` are not listed either, and every site list here " + "matches on the") + print(" bare METHOD NAME - three watchdogs are all called `evaluate` " + "and at least") + print(" four unrelated classes have an `explain()` - so the receiver is " + "printed") + print(" beside each one and a reader has to pick.") + print(" 4. ⚠️ IT CAN ALSO OVER-REPORT, IN ONE EXACT WAY. The sentence is " "ERASED before") - print(" the comparison, so a swap that would make the FORMATTING " - "ITSELF fail - a") - print(" `DAY_NAMES[start]` where `start` is 1320 - is scored as " - "'changes no decision'.") - print(" `_raise_gap`'s (start, weekday) and (end, weekday) pairs are " - "that: mixed") - print(" types, and a real transposition there raises IndexError rather " - "than lying") - print(" quietly. The hazard is a SAME-TYPED pair; read the list with " - "that in hand.") + print(" census A's comparison, so a swap that would make the FORMATTING " + "ITSELF fail") + print(" - a `DAY_NAMES[start]` where `start` is 1320 - is scored as " + "'changes no") + print(" decision'. `_raise_gap`'s (start, weekday) pairs are that. The " + "hazard is a") + print(" SAME-TYPED pair, which is why every census B and C row now " + "prints whether") + print(" the two format specs match.") + print(" 5. CENSUS C's verdict is a bare-name resolution (gap 3) and " + "therefore reads") + print(" MESSAGE too readily. Every verdict is printed with the sites " + "that produced") + print(" it; a MESSAGE verdict is a prompt to read them, not a proof.") if __name__ == "__main__":