Files
goodwe-addon/goodwe_controller/DECISIONS/SAFETY-37-freshness-policy.md
T
glenn schrooyenandClaude Opus 5 8e241a3243
checks / unit (pull_request) Successful in 5m32s
SAFETY-37: correct a wall-clock tick and a false size claim
Two blocking findings from review, both in prose, both corrected at every
site.

1. "trips on tick 7" was not what the fixture prints. It prints 11, five
   runs out of five, and the developer's own report says 11. ⚠️ But the
   deeper problem is that the number should never have been quoted: the
   tick is counted on the WALL CLOCK inside cycle() (hold_s 2 s, ticks
   0.20 s apart), so it is load-dependent - 7 is what a box slow enough
   to stretch each tick to ~0.29 s would print - and nothing pins it.
   The DECISIONS transcript now says 11 because it is a transcript of a
   run; the CHANGELOG stops quoting a tick at all and states the
   checkable thing instead: it trips without the gate and never with it,
   which part 3 grades.

2. ⚠️ SURVIVOR 2's SIZE CLAIM WAS FALSE FOR ONE OF THE TWO CONSUMERS THIS
   CARD EXISTS TO PROTECT. It read "one contributing at most 250 W to a
   loop whose reporting floor is 700 W" at three sites. That bounds the
   COMMAND. Once the register is frozen its REPORTED value is unrelated
   to the command - which is exactly HW-01's shape: commanded ~0 W,
   register stuck on the last large number.

   Measured in review: a unit frozen at -1200 W beside a healthy +900 W
   sibling yields circulating_w = 1800 W against the 700 W floor, 2.6x
   OVER, with no gate armed.

   So the scoping is TRUE for the per-unit sign check and FALSE for
   circulation. Corrected to say exactly that, and the circulation
   consumer's exposure below MOVE_W is now recorded as a KNOWN GAP rather
   than a covered one.

   It survived because part 9's S2 check asserts only `not stale` and
   part 4 pairs the frozen unit with a RESTING sibling
   (circulating_w([A_FROZEN_W, 0.0]) == 0.0) - neither grades the size
   sentence.

No behaviour change. Full suite: 22 passed, 0 failed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
2026-09-06 12:35:27 +02:00

377 lines
22 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# SAFETY-37 — the freshness policy, and the one sentence in the card that had to be rewritten
**Status: DECIDED 2026-09-06.** A per-unit reading that has stopped following its
own command is reported to its consumers as **`None`**, never as a held number.
The verdict is a **conjunction**, not an age threshold, and this document records
why an age threshold is not available at this layer — which is the one place the
card's wording could not be implemented as written.
Filed with SAFETY-37. Card `6a9a7eda7932387c3acbd700`, scope widened by the
architect pass of 2026-09-06 to carry the single policy for [[TEL-06]] and
[[TEL-08]] as well.
Every figure below was measured or read out of this repository on **2026-09-06**
and is re-derived by `test_freshness.py` on every run. Where a number is carried
forward from an earlier measurement, the section and the date it was taken are
given at the number.
---
## 0. The three faces, and which one this card built
| where | the defect | this card |
|---|---|---|
| **Layer 1** — `goodwe/master/goodwe-master-stage4-failsafe.yaml:690` | the ingest-age veto tests the **VALUE** of `sensor.goodwe_p1_sample_age`, not its arrival. A dead publisher freezes it at a healthy number and the veto can never fire | **not touched.** It is firmware, behind a flash. §5 says what it owes |
| **The add-on** — `app/hass.py:69-82` | returns a held value as a value. Nothing separates "read 40 %" from "still says 40 % because nothing updated it" | **built here** |
| **The evidence** — [[HW-01]] | SoC held 40 % for 6 h 25 m while the pack drained to its floor, every detector reading healthy | the case the policy is answerable to |
The two consumers the card names — SAFETY-30's per-unit sign check and
SAFETY-31's circulating-power sensor — both read `Inverter.batt` (register
`0x0518`) and **neither had a freshness gate of any kind**. Both already do the
right thing with a `None`: `watchdog.SignWatchdog.evaluate` **clears** its window
on a `None` measurement, and `circulation.circulating_w` returns `None` for a
roster with a missing term, which clears that window too. **Neither could ever
receive one.** The gate is the piece that was missing *between* the shipped read
and the shipped checks, which is why the change to each consumer is zero lines.
---
## 1. ⚠️ The sentence that had to be rewritten, and why saying so is the finding
The card says: *"A pack legitimately resting at 40 % and a dead sensor reading
40 % are the same number; **only arrival separates them**."*
**For a signal this add-on polls out of Home Assistant's state cache, arrival
does not separate them, and cannot.** `read_inputs()` runs about once a second
(`run_control()`'s `await asyncio.sleep(1)` bounds the loop body), and while the
entity exists Home Assistant answers `200` with a number **whether or not the
publisher behind it is alive**. So both members of the pair arrive, identically,
for ever. `test_freshness.py` part 1a builds an arrival-age gate and grades it:
the age since arrival is `0.0 s` at every instant of **both** runs. A threshold
on it is a threshold that can never fire — which is TEL-06's own defect rebuilt
one layer up.
The other half of the card's rule survives intact and is honoured absolutely:
> ⚠️ Home Assistant's `last_updated` does **not** move when a value is rewritten
> unchanged, so it cannot be the source of truth.
Neither does `last_changed`, and this project measured on the ENV-01 rig (70 s of
a frozen meter) and independently against the live house that `last_reported`
does not advance on a repeat on either serialiser either —
`p1.HomeWizardLocalSource`'s docstring records both. **`app/freshness.py` reads
no Home Assistant timestamp at all.** Every clock in the verdict is
`time.monotonic()` inside this process, stamped at the instant an arrival carried
a different number. That is what "arrival-stamped" can mean here, and it is
graded (part 1a asserts the module's body contains no `last_updated`).
**So the criterion was rewritten rather than approximated:** *a reading is stale
when it has not moved through a change it was obliged to follow.* The obligation,
not the age, is what separates the pair.
---
## 2. The verdict
```
STALE ⟺ unchanged for ≥ 35.0 s
AND |standing command now| ≥ 250 W
AND |standing command now − standing command when the reading
last changed| ≥ 250 W
```
Latched: **only an arrival carrying a different number clears it.**
`0x0518` is the inverter reporting **itself** — r = 0.998 with battery power
against r = 0.09 with the real meter (`goodwe/NOTES.md` §3.3) — and it tracks its
own command: command and actual agreeing within 4 W once settled (§5.10,
measured 2026-08-22), the register dithering ±1 W while holding (§5.11, same
day). A unit told to move 250 W and still answering the same integer watt is not
tracking.
### 2.1 Why a bare "unchanged for N seconds" threshold is not available
**Measured, not feared.** Replaying the committed 30-day capture of this house's
own 1 W-resolution power entity — `scenarios/p1-net-30d-2026-08/`, 485,841
samples straight out of Home Assistant's recorder, where a row exists only when
the state string **changed**, so the gap between two rows *is* the unchanged
duration every consumer saw:
| | |
|---|---|
| longest unchanged run over 30 days | **263.2 s** (held −899.0 W, 2026-08-11 17:04) |
| runs ≥ 35 s | 10 of 485,840 (0.0021 %) |
| runs ≥ 17 s | 386 of 485,840 (0.0794 %) |
A real power figure at 1 W resolution held **one value for four and a half
minutes with nothing whatever wrong**. So an unchanged threshold free of false
positives must be over 263 s — and 263 s is **3.5× too slow** to protect a
consumer that trips at 75 s. There is no admissible threshold. The conjunction
is not a refinement; without it the card has no implementable verdict.
⚠️ **That is the METER's register, not `0x0518`, and it is used as a surrogate
and named as one.** No 30-day capture of the inverter's own power exists — it is
not in the recorder export. The single thing this measurement is used for is the
statement above: that the legitimate unchanged duration of a power entity in this
house is *minutes*. It is not used to set the bound.
⚠️ **And every one of those long runs happened under an unmoving load** — the
worst is 263 s of a steady −899 W export, a house doing one thing. That is why
the conjunction keys on command travel: none of them arms it.
### 2.2 Why the magnitude term exists — a surviving mutation put it there
Deleting `abs(standing_w) < move_w` from `_grade()` left **every check in the
first draft green**, because in all of them a command that had *travelled*
250 W had also *ended up* above 250 W. The case that separates them is a site
being brought to **rest**: the write path is serialised, so a register can reach
its new value on the same read at which the *old* command is still standing —
`standing_at_change` then captures 1300 W, the site is commanded to 0 W, and the
travel term alone reads 1300 W of unfollowed command for as long as the site
rests. It would call a perfectly truthful +5 W reading dead. The magnitude term
is what says *a site at rest is not owed movement*, and part 4 now grades it.
**The hole that term leaves cannot mislead either consumer**, which is why it is
the right trade: a unit commanded under 250 W is one `watchdog.disagrees()` is
`False` for whatever the measurement says, and a frozen term beside a live unit
that is itself resting nets to **zero** circulating power rather than to a false
alarm. Both halves are asserted, not argued.
---
## 3. The bounds — one per signal, each with its derivation
⚠️ **One constant for both signals would be useless on SoC or trigger-happy on
power**, which is the third clause of the policy. `freshness.BOUNDS` carries them
as data with a `wired` column, and `test_freshness.py` part 7 walks the AST of
every shipped module to check that column tells the truth.
### 3.1 `batt` — `0x0518`, S16, 1 W, published every 10 s — **WIRED**
**Floor, 17.0 s** — the fastest a healthy register can be *obliged* to have
answered. Three terms, each read out of this repository:
| | | source |
|---|---|---|
| 6.0 s | the plant | `goodwe/NOTES.md` §5.9, measured 2026-08-22 with the poll temporarily at 1 s: 26 W at +1.43 s, 469 W (94 %) at +3.34 s, settled 507 W at +6.05 s |
| 10.0 s | the publisher | `firmware/goodwe-master.yaml:181`, `update_interval: 10s` on the `modbus_controller` carrying both `0x0518` and `0x050E` |
| 1.0 s | this add-on's read period | `main.run_control()`'s `await asyncio.sleep(1)` |
**Ceiling, 75.0 s** — the tighter of the two consumers, both read out of the
shipped `config.yaml`: `circulation_hold_s` **75** (line 512) and
`unit_sign_disagree_s` **85** (line 451). A gate slower than its consumer hands
over the frozen number and explains afterwards.
**Chosen: 35.0 s.** The admissible band `[2 × 17.0, 0.5 × 75.0] = [34.0, 37.5]`
contains four integers, and **the band is what is derived — 35 is a choice inside
it**, deliberately not 30 (that is `validate.FIRMWARE_INGEST_VETO_S`, and would
read as a borrowed constant nobody re-derived) and not 60 (round). Part 6
computes both ends **from `config.yaml`** and goes red if an operator default
moves: drop `circulation_hold_s` to 60 and this fails.
**`MOVE_W` = 250 W** is the **largest** admissible value, not a sensitivity
preference. The gate must arm everywhere a stale reading could mislead a
consumer; the earliest either consumer speaks is `unit_sign_disagree_w`, 250 W
(config.yaml:450), because below that `watchdog.disagrees()` is `False` whatever
the measurement says. Taking the ceiling of the admissible range minimises
spurious arming. It is pinned to `config.yaml` by part 6, not to a literal.
### 3.2 `soc` — `0x050E`, U16, 1 %, published every 10 s — **STATED, NOT WIRED**
SoC **cannot use the conjunction above.** A pack resting at its floor holds one
percent for hours with nothing wrong — the card's own example — and the standing
command says nothing about it, because 0 W is exactly what a rested pack is
given. The obliged movement for SoC is **energy, not command travel**: one
percent of pack A is ≈ **153.6 Wh** (15.36 kWh nominal, `goodwe/NOTES.md` §1.1b's
table — ⚠️ **NOMINAL and FROM DATASHEET**, not a measured usable capacity), so at
pack A's 5000 W ceiling one percent takes at least **110 s**, and at a realistic
1000 W about **553 s**.
So the bound is *"SoC unchanged while ∫|battery power| dt since its last change
exceeded K × 153.6 Wh"*, and **K cannot be chosen from anything written down
today**. Wiring it with a guessed K would be a gate that fires on a rested pack,
which is the hazard, not the fix. `Freshness` takes its obligation term from the
caller precisely so an energy integral can be handed in where command travel is
handed in for `batt` — TEL-08 needs no second mechanism.
---
## 4. What changes on a running install
| | before | after |
|---|---|---|
| healthy register, N=1 | value reaches consumers | **identical** — 60 reads of a dithering register under a reversing command, asserted in part 5 |
| healthy register, N≥2 | value reaches consumers | identical |
| **frozen register, N=1** | reads as a healthy measurement, for ever | **site stops**: `inputs_ok` False → `stale_input_s` hold → 0 W, and keeps commanding |
| **frozen register, N≥2** | per-unit sign check false-trips at 85 s and parks a healthy inverter; circulation sensor reports a loop that is not happening | both consumers get `None`, both windows **clear** |
⚠️ **The N=1 row is a behaviour change on the roster that is running in a house
today, and it is named rather than hidden.** It is also the point of the card:
HW-01 was a one-inverter site. The direction it fails in is the safe one — a
false verdict commands 0 W and keeps commanding (§11.1 rule 1), behind the
existing `stale_input_s` hold, and clears itself on the register's next move.
The counterfactual is run, not argued. `test_freshness.py` part 3 drives real
`cycle()` calls at N=2 with A's register frozen at −1200 W and B's live at
+900 W, the command travel produced by the shipped `compute()` and allocator:
```
standing on A: [0, 445, 890, 1335, 1335, ...] (the shipped control law)
gated : sign trip tick=None circulating_w=[1800, 1800, 1800, None, None, ...]
counterfact : sign trip tick=11 circulating_w=[1800, 1800, 1800, 1800, 1800, ...]
```
---
## 5. What TEL-06 and TEL-08 still owe
**[[TEL-06]] — layer 1, Blocked, and nothing in this card can reach it.** The
ESPHome veto tests the *value* of `sensor.goodwe_p1_sample_age`. This policy
lives in the add-on, on the other side of a flash, and cannot help. What TEL-06
owes is the same mechanism **inside the firmware**: a `millis()` stamp taken when
the setpoint/age entity is *written*, and a veto on `millis() − last_write_ms`
rather than on the published number. It is narrowed to that one substitution —
the policy question ("arrival, not value") is answered here; the remaining work
is a firmware stamp and a flash, which is a supervised human step.
**[[TEL-08]] — stale SoC.** It owes exactly three things, and no mechanism:
1. a **measured** usable capacity for pack A (or a defensible bound on it) — the
153.6 Wh per percent above is nominal and from the datasheet;
2. the **energy integral** since the reading last changed;
3. the constant **K** — how many percent-equivalents of throughput must pass
before an unmoved SoC is a fault.
It should consume `freshness.Freshness` with `BOUNDS["soc"]` filled in, not
restate the policy. The row is already in the table with `wired=False` and the
reason, and part 7 goes red the day that column stops being true.
**⚠️ Neither of them, and not this card, closes HW-01's whole class.** See §6.
---
## 6. Known survivors, with their size
Graded in `test_freshness.py` part 9, so they are residuals rather than
surprises.
| # | kind | what survives | size |
|---|---|---|---|
| 1 | **false positive** | an inverter physically unable to follow — saturating, thermally derating, or a BMS refusing a charge — is declared stale while telling the truth | one spurious **site stop**, behind `stale_input_s`, self-clearing the moment the register moves. It cannot drive the battery anywhere |
| 2 | **miss** | a register frozen under a command that never travels 250 W is never detected, for 400 s or for ever | ⚠️ scoped for ONE consumer, NOT both, and the earlier claim here was wrong. TRUE for the per-unit sign check: no such check can speak about that unit (`unit_sign_disagree_w` = 250 W). **FALSE for circulation.** "Contributing at most 250 W" bounds the COMMAND; once the register is frozen its REPORTED value is unrelated to the command, which is exactly HW-01's shape - commanded ~0 W, register stuck on the last large number. Measured: a unit frozen at −1200 W beside a healthy +900 W sibling yields `circulating_w` = **1800 W against the 700 W floor, 2.6× over**, with no gate armed. Found in review; part 9's S2 check asserts only `not stale` and part 4 pairs the frozen unit with a RESTING sibling, so neither graded the sentence |
| 3 | **miss — the largest** | a dead publisher whose last values **alternate**, or a cache replaying two old samples, never starts the unchanged clock at all | **this gate detects a value that STOPPED; it cannot detect a value that is WRONG.** Nothing in this add-on can. TEL-06's arrival stamp inside the firmware is the only layer that could |
Recorded because it looks like a survivor and is not: a publisher failure that
freezes **both** registers is caught on both units independently, on each unit's
own command travel and each unit's own number.
---
## 7. Mutation record — 2026-09-06
`tools/mutation_run.sh`'s rule, honoured on every run: purge first (count printed,
`0` remaining), tree mounted `:ro`, `python3 -B`, fingerprint the tree, **and
fingerprint again after the revert**. Clean tree `app/freshness.py`
`sha256:f40fa3e8819014ce` / 20,065 B, `app/inverter.py` `7e760c6e8cdfa5b7` /
30,778 B, `app/main.py` `7d7ff4ba0697ea64` / 238,535 B. Each mutant graded by
`test_freshness.py` + `test_inverter.py`.
| # | mutation | file | Δ B | result |
|---|---|---|---|---|
| M0 | none (control) | freshness | +0 | **green** — the harness is not red on everything |
| M1 | `reading()` returns `self.last` unconditionally | freshness | −24 | red, 3 named checks |
| M2 | drop the command-travel term | freshness | −59 | red |
| M3 | drop the magnitude term | freshness | −54 | **SURVIVED**, then red — §2.2. This is the finding |
| M4 | latch never holds | freshness | +13 | red, 3 named checks |
| M5 | bound 90 s (slower than both consumers) | freshness | **+0** | red |
| M6 | bound 10 s (under the physical floor) | freshness | **+0** | red |
| M7 | `MOVE_W` 700 W | freshness | **+0** | red, on part 6's `config.yaml` pin |
| M8 | the wiring in `_read_unit` removed | main | −5 | red, 3 named checks |
| M9 | reset the unchanged clock on every arrival (value-stamped) | freshness | −43 | red |
| M10 | `standing_w()` drops `last_write` | inverter | −73 | red, and takes `test_inverter.py` with it |
| M11 | `standing_at_change` refreshed on every arrival | freshness | +121 | ⚠️ **DISCOUNTED — the mutant does not parse** |
| M11b | the same defect, written parseably | freshness | +105 | red |
| M12 | `BOUNDS["soc"].wired = True` — the table lies | freshness | −1 | red |
| M13 | the wrong signal submitted (`inv.soc` for `inv.batt`) | main | +4 | red, and `test_inverter.py` too |
| M14 | one `Freshness` shared by every unit | inverter | +29 | red |
| M15 | latch cleared by a failed read | freshness | +31 | **SURVIVED**, then red — see below |
⚠️ **M5, M6 and M7 are `+0 B`, which MUTATION.md names as the risky delta.** All
three are **reds**, which is the direction stale bytecode cannot fabricate, and
every run purged with the count printed and graded a `:ro` tree that physically
cannot hold a cache.
⚠️ **M11 IS NOT A RESULT AND IS NOT COUNTED.** Its edit dedents a statement into
the middle of an `if` body, so the module raises `IndentationError` at line 312
and both files go red without a single check being evaluated. Verified by
`ast.parse` rather than assumed. **M11b** is the same defect written so it
parses, and it is a real red.
### The two survivors, and what each one taught
**M3, the magnitude term** — see §2.2. Two terms of a conjunction that move
together in every fixture are one term as far as the checks are concerned.
Recorded as `MUTATION.md` **habit 17**, twelfth instance of "a check whose DATA
cannot reach the failure it names".
**M15, the latch cleared by a failed read** — found only because the sweep went
looking for what the checks would NOT see. No fixture had ever injected a read
failure *while latched*, and it matters: a stale verdict stops the site, the site
is then commanded 0 W, and one unreachable-Home-Assistant read would drop the
latch into a state where the conjunction is no longer armed — so the held
−1200 W would come back to the consumers on the very next successful read. Part 4
now grades exactly that and M15 is red.
### And one process defect worth more than the ledger
One mutation's container run overran a two-minute command budget and the wrapper
was killed **between the edit and the revert**, leaving `app/freshness.py`
mutated on disk. Nothing announced it; the next mutation's harness cheerfully
fingerprinted and graded the mutant *plus* a second mutation. It was caught by
comparing the tree fingerprint against the recorded clean one, restored, and
verified back to byte and `sha256` before anything else ran. Same defect as stale
bytecode, different cause. Recorded as `MUTATION.md` **habit 18**: *fingerprint
after the revert, not only before the run.*
### A third, in this file's own fixture
M2 originally reported "red" with **no named check behind it**. The mutant makes
the FRESH run stale, so `test_freshness.py`'s own diagnostic
`print(f"… {sorted(set(_fresh_read))}")` raised `TypeError` comparing `None` with
a float **before check one ran**. MUTATION.md's reporting rule is *"which suite
went red, and on which named check"*, and a print that can crash first destroys
exactly that. Sorted with `key=str` now; M2 reports its check.
### Re-verified at the committed head
The 16-row sweep above graded `test_freshness.py` as it stood when each mutation
ran; the two closures (M3's part-4 check, M15's failed-read check), the `key=str`
fix and the wall-clock hardening all landed afterwards. So six mutations were
re-run against **the exact tree that is committed** —
`test_freshness.py sha256:a62f3e1f1ac856cf`, 44,913 B:
| # | result at head |
|---|---|
| M0 control | **green** |
| M2 drop the command-travel term | red, on part 1's "FRESH AND UNCHANGED is not stale" |
| M3 drop the magnitude term | red, on part 4's magnitude check |
| M8 wiring removed | red, on three named checks in parts 2 and 3 |
| M11b `standing_at_change` drifts | red, on three named checks in part 1 |
| M15 latch cleared by a failed read | red, on part 4's failed-read check |
The other ten are unaffected by those edits: each is red on a check in parts 1,
2, 4, 6, 7 or 9 that none of them touches, and a mutation red on one check
cannot be made green by adding another.
### The wall clock, which part 3 cannot avoid and therefore measures
The freshness verdict runs on an injected monotonic clock and is deterministic.
The sign watchdog inside `cycle()` cannot be injected, so part 3 runs against the
real one — which is CI-06's failure mode exactly (`test_peakguard.py` answered
two checks with the wall clock, flaked at ~20 %, and cost a wrong merge
diagnosis). So the margin is measured and asserted rather than assumed:
gate closed after 0.61 s of WALL clock, against the 2 s watchdog window (3.3x margin)
If a loaded machine ever narrows that, the check fails **naming the number**,
which is the difference between "the box was slow" and "the code is broken".