checks / unit (pull_request) Successful in 4m30s
DEV-02 made every per-device value per-instance - rating, SoC, watchdogs, heartbeat, store keys - so the code stopped assuming one inverter. The options schema did not. `build_roster()` returned a list of exactly one and said so in its own docstring, and there was no `inverters:` key for an operator to write, so the roster could only ever be a roster of one. THE SHAPE, AND WHY IT WAS MEASURED RATHER THAN CHOSEN config.yaml had no list-of-objects anywhere, and `static_profile` carries a whole tariff structure as a JSON string in a `str?` precisely because a nested schema was doubted. That is a question about somebody else's software, so it was answered by running it: `tools/supervisor_shape_probe.py` (committed) runs Supervisor 2026.08.0's OWN `AppOptions` validator and `SCHEMA_APP_CONFIG`, out of the release image, against this repository's config.yaml. 2026.08.0 is the version the live install reports. The full transcript and the argument are in DECISIONS/CFG-04-inverter-roster.md. It accepts the list, enforces per-entry types, refuses a missing `name` / `setpoint_entity` / `max_w` BY NAME, renders the option in the UI as a repeatable section, and drops the key with a warning under an older schema - so a downgrade is not the forward trap in reverse. ⚠️ AND THE `inverters: []` DEFAULT IS LOAD-BEARING, WHICH IS THE MEASUREMENT THAT MATTERED MOST. Supervisor answers `Missing option 'inverters' in root` for an options blob that has no such key - which every existing install's has, since the key did not exist when they were written. `App.options` is the shipped `options:` block deep-merged with the operator's stored one, so the empty default is the difference between an upgrade nobody notices and an add-on that cannot start with a schema pinned to the version that would fix it. That is docs/deploy-rollback.md §4's trap in the FORWARD direction, found before it was sprung. The merger was checked too: lists override rather than concatenate, so a non-empty default could not have grown a roster behind the operator's back. NOTHING IS INHERITED, WHICH IS DEV-02 RESTATED AS A CONFIG RULE `name`, `setpoint_entity` and `max_w` are required of every entry. An entry that omits `soc_entity` has no SoC sensor; it does not borrow the flat one. A three-line fallback would have rebuilt the sibling-grading defect DEV-02 spent a card removing, in YAML. Defaults still live in exactly one place (CFG-03): there is no per-entry default anywhere, because there is no optional per-entry number. A flat `setpoint_entity` left set beside a declared roster is refused rather than ignored - it names an actuator nothing will write. TWO PERMISSIVE READERS IN SHIPPED CODE, AND THREE MORE IN THE CHECKS validate.py rules 5 and 6 compared their thresholds against the FLAT `max_w`. At a roster of two 2000 W units with `max_w: 5000` left over from a single-inverter install, `sign_disagree_w: 3000` passed rule 5 while SAFETY-02's watchdog could fire on NEITHER unit - rule 5's own failure mode, reached through rule 5. They read `unit_caps()` now, which at one inverter is `[max_w]` exactly. main.py's `run_control()` re-ran the device-rating check behind `if self.o.get("setpoint_entity")` - the flat key, which a roster leaves blank - so on a two-inverter site no inverter's `max_w` was graded against the device it is written to, and the skip was silent. `validate_unit_ranges()` grades each unit against its own ESPHome and runs unconditionally. test_peakguard.py's `read_option()` and `_schema_bound()` and test_inverter.py's `read_schema_max()` matched a STRIPPED line and took the first hit, so the nested per-inverter `max_w:` began outranking the flat one - silently, and correctly, because the two bounds are the same int(100,5000) today. A reader that is right by coincidence is this project's named failure mode. All three are anchored at exactly two spaces now; the file's two halves disambiguate `peak_cap_w` instead of first-match, and an ambiguous key raises rather than picking one. test_tariff.py's copy of the idiom went with them. THE AUDIT IS A CHECK, NOT A PROMISE test_inverter.py part 11 finds every config.yaml-reading function in the suite - transitively, because two of the readers CFG-04 had to change never name the file - EXECUTES each against a synthetic two-inverter config.yaml whose per-inverter numbers differ from its flat ones, and fails if any answers with the per-inverter value. The table is not maintained by hand: the signature decides, so the eleventh reader is named on its first run. config.yaml declares `inverters:` BEFORE `max_w:` on purpose and part 11 asserts that ordering, so the trap stays baited. SAFETY-28's `actuator_writers()` needed no change - it was already a structural, case-folding census - but until now there was no configuration in which two writers on one actuator could be WRITTEN DOWN. There is now, and it is refused. VERIFIED BY BREAKING IT Ten mutations run through tools/mutation_run.sh and observed red, listed in test_inverter.py part 12 with the check that bites each. Two survived the first run and the CHECKS were changed, not the table: the inheritance check had blank flat keys, so "inherited" and "not inherited" were the same empty string; and the `run_control()` check asserted a SOURCE LINE was present, which re-indenting it does not remove. Both are recorded in MUTATION.md's "the check's data cannot reach the failure it names" section, which now carries two more habits: assert on behaviour rather than on source text, and give a check whose failure mode is a hang a deadline. 19/19 green in the goodwe-addon-ci image. The one-inverter path is byte- identical: the whole existing suite runs unmodified against it. Version bumped to 0.6.0 - the Supervisor keys the built image by `version:`, so a schema change without a bump is a schema nobody reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
268 lines
18 KiB
Markdown
268 lines
18 KiB
Markdown
# Mutation runs in this repository — and the one way they lie
|
|
|
|
Nearly every safety claim in this add-on was established the same way: change
|
|
the shipped module, run the suites, watch one go **red**, put the module back,
|
|
watch it go **green**. There is no coverage tool here and there is not meant to
|
|
be — the check that matters is whether a *check* notices when the behaviour it
|
|
names is removed.
|
|
|
|
That method has exactly one silent failure mode, and TEST-15 exists because we
|
|
walked into it.
|
|
|
|
## The defect
|
|
|
|
FCST-03's developer reverted `self.provisional` to `here.provisional`, re-ran,
|
|
and reported the mutation as a **survivor**. It was not. CPython decides a
|
|
cached `.pyc` is still valid from two numbers:
|
|
|
|
* the source file's **size in bytes**, and
|
|
* the source file's **mtime truncated to whole seconds**.
|
|
|
|
`self` and `here` are the same length, and the revert happened in the same
|
|
second as the edit. Neither number moved, so the interpreter loaded the
|
|
bytecode it already had. Confirmed at the time by disassembly: **the source read
|
|
`here`, the running code loaded `self`.** The suite had graded code that was not
|
|
there — no error, no warning, exit status 0.
|
|
|
|
The same collision, in the other direction, can leave a **red** standing on
|
|
correct source after a revert that never took.
|
|
|
|
### Which way it lies, and why that scopes everything
|
|
|
|
A mutation that never took effect leaves the suite **green**, and a green run of
|
|
a mutated tree is reported as a survivor. So stale bytecode **manufactures
|
|
survivors**; it cannot manufacture a red. The false-red case exists but
|
|
announces itself, because a developer chases a failure they cannot explain.
|
|
|
|
Two consequences, and both are used below:
|
|
|
|
* a recorded **"all red, no survivors"** result is not at risk from this defect;
|
|
* a recorded **survivor** — and a recorded *zero*, such as a coverage-audit
|
|
count of 0 — is exactly what this defect produces.
|
|
|
|
`test_control.py`'s coverage-audit table says the same thing in its own terms:
|
|
the error is one-directional, so every figure there is a lower bound at worst.
|
|
|
|
## The rule
|
|
|
|
**Purge `__pycache__` before every mutation run.** Not afterwards, not usually,
|
|
not when the edit looks big enough.
|
|
|
|
```sh
|
|
cd goodwe_controller
|
|
tools/mutation_run.sh # every test_*.py in this directory
|
|
tools/mutation_run.sh . test_tariff.py # or just the ones you are watching
|
|
tools/mutation_run.sh "$GOODWE_ADDON_REPO/goodwe_controller" # from elsewhere
|
|
```
|
|
|
|
The harness purges, prints a `sha256`/byte-count fingerprint of every `app/*.py`
|
|
it is about to grade, runs each suite under `python3 -B`, and fails if a cache
|
|
appeared during the run. Diff two fingerprints and the file you mutated is the
|
|
line that changed — which is also how a mutation report proves *which source*
|
|
produced it.
|
|
|
|
### What does **not** fix this
|
|
|
|
| | writes bytecode | reads a stale `.pyc` |
|
|
|---|---|---|
|
|
| `python3 -B` | no | **yes** |
|
|
| `PYTHONDONTWRITEBYTECODE=1` | no | **yes** |
|
|
| purging `__pycache__` | (unchanged) | **no — nothing to read** |
|
|
|
|
Both flags are *write-side only*. A mutation run always has a cache already,
|
|
because the previous run wrote it, so a harness that only adds `-B` closes
|
|
nothing. This is measured, not read off documentation: `test_bytecode.py`
|
|
part 2 arranges the collision and shows both flags still running the old code.
|
|
|
|
`-B` is still worth passing — it stops **this** run leaving a cache for the
|
|
**next** edit to inherit, which is the one nobody is thinking about three
|
|
mutations into a batch. Purge for this run, `-B` for the next one.
|
|
|
|
### And not a `sleep`
|
|
|
|
One-second mtime resolution is the *mechanism*, not the enemy. Waiting out the
|
|
clock would slow every run down, and would still only cover the edit somebody
|
|
remembered to slow down. Stop trusting mtime instead.
|
|
|
|
## Reporting a mutation
|
|
|
|
State the things a reviewer cannot reconstruct afterwards:
|
|
|
|
1. **the byte delta** of the mutated file (`+5 B`, `+0 B`). `+0 B` is the risky
|
|
one — say so;
|
|
2. **that the mutated line was on disk** when the run started (the harness
|
|
fingerprint, or a `grep` of the line);
|
|
3. **which suite went red, and on which named check** — "the suite is red" and
|
|
"the check that watches this is red" are different claims;
|
|
4. for a **survivor**, that the run was purged. An unpurged survivor is not a
|
|
result.
|
|
|
|
## The other thing a mutation sweep cannot see: masking (TEST-16)
|
|
|
|
Stale bytecode is not the only way a survivor lies. **Two guards can mask each
|
|
other**, and a sweep that mutates one line at a time is structurally blind to it.
|
|
|
|
TEST-16 is the worked example. `static_tariff.arbitrage()` refuses a
|
|
one-firm-hour series at `len(firm) < 2`, and refuses a flat series at
|
|
`spread == 0.0`. A single firm hour *also* has a spread of zero. So:
|
|
|
|
* weaken the firm-pair rung alone → the flat rung refuses the stimulus anyway;
|
|
* weaken the flat rung alone → the tiny-spread stimulus is still refused, just
|
|
for the wrong reason;
|
|
* and the suite, grading `kind == "refused"`, reports **nothing either way**.
|
|
|
|
Both rungs could then be weakened *together* and still nothing would report —
|
|
which is precisely the state in which the next edit to either one opens a real
|
|
hole. It survived four rounds of review on TARIFF-03 and FCST-03.
|
|
|
|
Two rules come out of it, and both are cheap:
|
|
|
|
1. **Grade the reason, not just the verdict.** "Refused" and "refused for the
|
|
right reason" are different claims, and a masked rung is the case where the
|
|
first is true and the second is false. A refusal sentence is what the
|
|
operator reads; it is also the only observable that moves when one rung
|
|
quietly absorbs another's work.
|
|
2. **When two rungs can answer the same stimulus, mutate them together.** One
|
|
extra run per suspected pair. If a sweep finds two survivors in the same
|
|
function, that pair is the first thing to try — TEST-16's two survivors were
|
|
adjacent lines in one function and neither was individually live.
|
|
|
|
## The third way a check lies: its DATA cannot reach the failure it names
|
|
|
|
Stale bytecode manufactures survivors from a mutation that never ran. Masking
|
|
manufactures them from a second guard doing the first one's work. This one
|
|
manufactures them from a **fixture**: the check runs, the assertion is
|
|
evaluated, the sentence above it is true — and the numbers it compares are
|
|
arranged so that the failure it describes could not have changed the answer.
|
|
|
|
It is not hypothetical and it is not rare. Five instances in two weeks; the
|
|
first three were found by review rather than by a red line, and the last two -
|
|
CFG-04's - by an author's own mutation battery, which is the cheaper place:
|
|
|
|
* **ARB-03** shipped a call-site scan asserting that every `claims.append` in
|
|
`cycle()` routes through `actuator_of()`. Its own parse **saw 3 of the 8
|
|
appends**; the other five were invisible to it and would have passed however
|
|
they were routed.
|
|
* **ALLOC-01** shipped a replay generator to hunt for envelope escapes. It found
|
|
**0**. A widened generator over the same allocator found **39,991**.
|
|
* **DEV-02** shipped `"each unit's supervised bound is ITS OWN max_w — give
|
|
inverter B a different rating and the claim filed on inverter B moves with
|
|
it"`, comparing inverter B's bound against `_b2.max_w`. The fixture defaulted
|
|
B's rating to A's, so it compared `(-5000, 5000)` against `(-5000, 5000)` —
|
|
**a number against itself**. Filing both per-unit bounds from
|
|
`self.inverters[0]` left **all 19 files green**.
|
|
* **CFG-04** wrote *"an entry that omits `soc_entity` has NO SoC sensor and does
|
|
not borrow the flat one"* against a fixture whose **flat `soc_entity` was
|
|
blank**. Inherited and not-inherited were both `""`, so the mutation that
|
|
makes `build_roster()` fall back to the flat key survived. The fixture now
|
|
sets a real entity there, and the check asserts that the flat value is present
|
|
as well as that the entry's is empty.
|
|
* **CFG-04** also wrote *"main.py runs the per-unit range check
|
|
UNCONDITIONALLY"* as a **substring test over `app/main.py`'s source**. The
|
|
mutation that defeats it puts the call back inside `if entity:` — one level of
|
|
indentation — and the substring is still there. That is habit 4 below in a new
|
|
costume: the check asserted against the TEXT of the thing under test rather
|
|
than against its behaviour. It now drives the shipped `run_control()` on a
|
|
two-inverter config, **under a deadline**, because the mutation's real symptom
|
|
is that the controller enters its loop and never returns.
|
|
|
|
Note what these have in common with a masked rung: the mutation *runs*, and the
|
|
suite is *honestly* green. Nothing in a purge, a fingerprint or a disassembly
|
|
can see it, because the code under test really did behave identically on the
|
|
stimulus it was given. The defect is upstream of the assertion, in what the
|
|
check was handed.
|
|
|
|
**The rule, and it is one line:**
|
|
|
|
> **Any check asserting "X differs from Y" must be run once with X and Y
|
|
> actually made different.**
|
|
|
|
Three cheap habits fall out of it, and all three are now used in this repository:
|
|
|
|
1. **Write the PRECONDITION as its own check.** `test_peakguard.py` 10g asserts
|
|
that A and B have different ratings *before* it asserts that B's bound is
|
|
B's. If a later edit makes them equal again, the precondition goes red rather
|
|
than the claim going quietly vacuous.
|
|
2. **Assert against literals, not against the expression under test.**
|
|
`== [(-_b2.max_w, _b2.max_w)]` cannot distinguish "B's number" from "the
|
|
number this fixture happens to have twice"; `== (-3000.0, 3000.0)` beside a
|
|
printed `A = 5000` can.
|
|
3. **Print what the check saw.** Every one of the three above would have been
|
|
caught by a human reading one line of output — `supervised bounds by
|
|
actuator: {'inverter-a': (-5000, 5000), 'inverter-b': (-5000, 5000)}` is
|
|
self-evidently not evidence of a difference.
|
|
|
|
4. **Assert on the BEHAVIOUR, not on the source text of the thing under test.**
|
|
A substring test over a module is a check on how the code is spelled;
|
|
whitespace, an alias or a moved line defeats it while the property it names
|
|
is gone. `test_inverter.py` part 10 has one of each — a source pin for a
|
|
claim that genuinely IS about the code's shape, and a driven `run_control()`
|
|
for a claim about behaviour — and the second replaced the first after the
|
|
mutation battery walked straight through it.
|
|
5. **A check whose failure mode is a HANG needs a deadline.** Otherwise the
|
|
mutation that defeats it produces a CI timeout with no name attached, which
|
|
reads like flaky infrastructure and gets retried rather than read.
|
|
|
|
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
|
|
per-unit claim defect (`device`/`supervised` filed without `inv`), which the
|
|
static routing pin catches. The *bound* variant — right actuator, wrong
|
|
numbers — is a different defect with a different fix, and it was absent. Both
|
|
are in the table now.
|
|
|
|
## Where each surface stands
|
|
|
|
* **Local and dev-harness runs — the exposed one.** This is where FCST-03 hit
|
|
it. `goodwe_controller/__pycache__` and `app/__pycache__` are gitignored and
|
|
are sitting in most working trees right now. Use the harness.
|
|
* **CI — structurally immune, and it was before this card.** `.gitea/workflows/
|
|
checks.yml` bind mounts the checkout `-v "$WS:/w:ro"`, so the interpreter
|
|
cannot write a `__pycache__` into the tree under test at all; measured for
|
|
TEST-15, and it fails *silently* — no error, exit status 0. Each job also
|
|
`rm -rf`s its workspace and clones afresh, so nothing crosses between jobs on
|
|
the shared runner. `PYTHONDONTWRITEBYTECODE=1` is now set there too, as belt
|
|
and braces: the `:ro` is one character wide and the next person debugging a
|
|
write will be tempted by it.
|
|
* **The shipped add-on — not affected, and this is closed.**
|
|
`goodwe_controller/Dockerfile` has carried `PYTHONDONTWRITEBYTECODE=1` since
|
|
it was written. There is no deploy-side finding here; do not re-open it.
|
|
* **The infra repo's `sim/` harnesses — exposed, and NOT fixed by this card.**
|
|
`sim/replay_sign.py` and friends import the add-on out of
|
|
`$GOODWE_ADDON_REPO` and write `__pycache__` into *both* trees
|
|
(`goodwe/sim/__pycache__` exists today). SAFETY-02's headline validation is a
|
|
replay, not a suite, so a mutation graded through it carries the same
|
|
exposure. `tools/mutation_run.sh` takes the tree to purge as its first
|
|
argument precisely so that repo can call it, but the change belongs in that
|
|
repo and some of those harnesses want the rig. **Anyone mutating a module
|
|
graded by a replay must purge both trees by hand until then.**
|
|
|
|
## Survey — mutation claims of 2026-08-25 … 2026-09-01
|
|
|
|
Scoped **by the property, not by re-running everything**. A claim is at risk
|
|
only if a mutation could have preserved the file's byte count *and* been
|
|
reverted inside the same second, *and* the claim is of the kind stale bytecode
|
|
can fabricate (see "which way it lies"). Sources: `git log release/1.0` and the
|
|
commit and PR bodies.
|
|
|
|
| claim | verdict | why |
|
|
|---|---|---|
|
|
| **FCST-03** M9 `self.provisional`→`here.provisional` (dd40c56, 71cd784) | **re-verified** | this is the original false survivor. Re-run with the cache cleared, M9 is caught. The shipped source was correct throughout. |
|
|
| **FCST-03** X1, X6 degenerate stimuli (71cd784) | **not at risk** | run after the purge discipline landed in that branch. |
|
|
| **SAFETY-24** 11 author + 9 reviewer mutations (e709554, 4601682) | **not at risk** | the commit records them run under `python -B` with a purged `__pycache__` and real byte deltas. Its one survivor is an equivalent mutant, argued structurally rather than by a green run. |
|
|
| **SAFETY-26** tolerance sweep over `arbitrage.py` and `static_tariff.py`, 2 survivors (2b8b8cb) | **not at risk** | purge, printed `+5 B` delta, mutated line asserted present, tree re-read before each run. One survivor is explicitly recorded at `+0 B` — the risky delta — and was produced *under* the purge. **Both survivors are now closed by TEST-16** (`test_static_tariff.py` section 9), which also found that the two of them *masked each other* — each was individually non-live only because the other rung caught its stimulus, an arrangement single-mutation testing cannot see. Its own ledger is 7 mutations, 4 of them at `+0 B`, all red. |
|
|
| **OPT-04** mutations reported on PR #22 (bdd7f18) | **not at risk** | `test_arbitrage.py`'s header records the exact command: purge, then `python3 -B`. |
|
|
| **TEST-17** `checks()` dropping `when` (504e8da) | **not at risk** | a *red* claim, and dropping an argument changes the file length. Neither half of the collision is available. |
|
|
| **SAFETY-07** wiring mutations B1/B2/B3 and `test_peakguard.py` parts 7 and 10 (2d570d8, e7c95bf) | **not at risk** | those mutations are **executed in-process**, monkeypatched onto the imported module. No source file is edited, so no `.pyc` is consulted. |
|
|
| every **"all red, no survivors"** claim in the window — TARIFF-02 (24/0), TARIFF-03 (11), MAINT-04 (10), SAFETY-18 (6+4), SAFETY-03 (6), TEL-04 (10), TEL-05 (6) | **not at risk** | direction. Stale bytecode turns a red into a green survivor, never the reverse. A red is evidence that *something* changed under the check. |
|
|
| **TEST-06**'s deleted assertion, "no new notification while parked at 0 W" (c34b5a2) | **not at risk / superseded** | it was recorded as undecidable and then *falsified* — `test_freeze.py` records the one-line mutation that takes the file red 9 times. The stronger later claim is a red one. |
|
|
| **TARIFF-01** reviewer survivor: `record_block()` ignoring its `policy` argument (0dbd485) | **re-verified — still a survivor** | re-run here through `tools/mutation_run.sh` against the 15 suites that were on the record, all green with `billed_w(block, policy)` mutated to `billed_w(block, POLICY_NET)`. The recorded diagnosis holds: every block reaching the store is single-phase, where NET, SUM and MAX collapse. **Now CLOSED by TEST-13** (`test_tariff.py` section 7), which round-trips section 7's own unbalanced `(3000, 500, -200) W` block THROUGH the store under each policy - three distinct banked figures, so a hardcoded policy takes at least two of them red. Third instance of "a stimulus evaluated where the quantity it grades is degenerate", after WITNESS-02's span normalisation and TARIFF-01's own DST window. |
|
|
| **SAFETY-25** 8 mutations against `test_peakguard.py` part 10, then 9 more after review, all red | **not at risk** | each run against a tree copied fresh with `__pycache__` excluded and mounted `:ro`, so no `.pyc` existed to go stale - the same structural immunity recorded for CI above. Byte deltas recorded on the PR; none is `+0 B`. All are **reds**, which is the direction this defect cannot fabricate. ⚠️ The second round exists because the first round's mutations were all aimed at DISARMING the guard, and the claim part 10 actually made was about the guard being FIXED - the reviewer built the fix and the suite stayed green. Bytecode was never the issue there; an unaimed battery was. |
|
|
| **SAFETY-03** reviewer survivor: `abs()` dropped from `clamp_to_rating`'s limit (6aabb83) | **re-verified — still a survivor** | re-run the same way, all 15 green. Also as recorded: unreachable under the `int(100, 5000)` schema. |
|
|
| **TARIFF-01**'s other three survivors, **SAFETY-03**'s `describe()` survivor, **TARIFF-02**'s M3/M5/M7, **SAFETY-18**'s three | **still unverified** | recorded before the purge discipline, with no byte delta on the record, so a `+0 B` same-second revert cannot be excluded. All are survivors, which is the direction this defect fabricates. TARIFF-02's and SAFETY-18's were **closed by later commits whose claims are reds**, so what is unverified is the *diagnosis*, not the current state of the code. |
|
|
|
|
**Nothing here says a past result is wrong.** The two survivors re-run above
|
|
both stood. What changed is that they are now on the record as measured rather
|
|
than assumed, and the rest are labelled honestly. The still-unverified rows are
|
|
cheap to close — one harness run each — and the right moment is when somebody
|
|
next touches that module, not in a re-run campaign nobody will read.
|