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__":