diff --git a/.gitea/workflows/checks.yml b/.gitea/workflows/checks.yml index 0d4f451..46fb54f 100644 --- a/.gitea/workflows/checks.yml +++ b/.gitea/workflows/checks.yml @@ -101,10 +101,22 @@ jobs: # hwsim section was skipped - so "test_freeze.py: PASS" below is a weaker # claim than it looks. That belongs to whoever owns test_freeze.py next; # naming it here is the least this gate can do about it. + # ⚠️ PYTHONDONTWRITEBYTECODE=1 IS BELT AND BRACES, NOT A FIX FOR A LIVE + # HOLE. TEST-15 measured this runner's actual exposure and found none: the + # checkout below is bind mounted `:ro`, so the interpreter physically + # cannot write a __pycache__ into the tree it is grading - it fails + # SILENTLY, no error and exit status 0 - and every job `rm -rf`s $WS and + # clones afresh, so nothing survives from a previous job on this shared + # runner either. The variable is here because the `:ro` is the load-bearing + # part and it is one character wide: delete it while debugging a write and + # the gate quietly starts caching bytecode across a rebuild again. Note + # what it does NOT do, per TEST-15 part 2 - it stops bytecode being + # WRITTEN, never a stale one being READ. It is not a substitute for the + # purge in goodwe_controller/tools/mutation_run.sh. - name: the add-on's own tests run: | set -eu - docker run --rm -v "$WS:/w:ro" goodwe-addon-checks:1 sh -eu -c ' + docker run --rm -e PYTHONDONTWRITEBYTECODE=1 -v "$WS:/w:ro" goodwe-addon-checks:1 sh -eu -c ' cd /w/goodwe-addon/goodwe_controller set -- test_*.py if [ "$1" = "test_*.py" ]; then diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index ef47f8b..9e92b23 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +**CI-02 PROBE - THROWAWAY, DO NOT MERGE.** This paragraph exists only to +collide with the entries release/1.0 has gained since this branch point, so +that the pull request is genuinely unmergeable and the gate can be observed +reporting on it. The branch and its pull request are deleted afterwards. + **Thirty days of the real meter, and the acceptance criterion three cards deferred is now a number (DATA-01).** TARIFF-01 and SAFETY-07 both merged with AC 3 - *"monthly peak held under the soft limit in >= 95 % of intervals over a diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 459bc29..14f87da 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -640,7 +640,18 @@ The add-on alarms when **both** of these are true at once: - the meter's own document (power figure *and* both kWh registers) has not changed for `meter_freeze_s` — 300 s by default; **and** -- the command standing on the wire is at the rail, within 5 % of `max_w`. +- the command standing on the wire is at the rail, within 5 % of the ceiling + the command can actually reach. + +**That ceiling is `min(max_w, the setpoint device's own rating)`, not `max_w`.** +The emitted command is clipped to the device's reported range as well as to +`max_w`, so on a 4700 W inverter configured with `max_w: 5000` a rail measured +at 4750 W is one the command can never stand at — the guard could not fire, and +the status page said `armed` anyway. It now measures the rail against the +smaller number and the page names it, saying *"below max_w because …"* when the +device is what capped it. (On an install that started, the two are equal: a +`max_w` above the device rating is refused at startup. This matters when the +rating could not be read at boot — see below.) Neither half is usable alone. A bare "unchanged for N seconds" rule false-trips at the exact operating point this controller aims for — at a converged −10 W a @@ -666,6 +677,13 @@ that reads the meter's own document; on `ha_dsmr`, `mqtt_p1`, `ha_signed` and `off` there is no signal to threshold and the status page says **NOT COVERED** rather than pretending the guard is armed. +The same row appears for a second reason: a setpoint device that reports a range +from which no command magnitude is available in both directions leaves no rail +to measure at all, and a 0 W rail would be satisfied by *every* command. That is +reported as **NOT COVERED** too, and the detector stays silent rather than +degenerating into the bare "unchanged for N seconds" rule this design exists to +avoid. + | option | default | meaning | |---|---|---| | `meter_freeze_s` | 300 | Seconds of an unchanged measurement, *while the command is at the rail*, before the alarm | diff --git a/goodwe_controller/MUTATION.md b/goodwe_controller/MUTATION.md new file mode 100644 index 0000000..54309cd --- /dev/null +++ b/goodwe_controller/MUTATION.md @@ -0,0 +1,153 @@ +# 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. + +## 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. | +| **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. | +| **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. diff --git a/goodwe_controller/app/clamp.py b/goodwe_controller/app/clamp.py index 7b9acba..e7a9a87 100644 --- a/goodwe_controller/app/clamp.py +++ b/goodwe_controller/app/clamp.py @@ -46,8 +46,18 @@ the validation did not run or no longer holds - `hass.limits()` returns (None, None) whenever ESPHome is not up yet, and the range checks are skipped rather than guessed at - and that is a louder fault than the first stage, not a quieter one. It therefore alarms too, through the same channel, with the bound -named. This is NOT SAFETY-20; nothing here changes what `freeze.py` measures its -rail against. +named. + +⚠️ SAFETY-20 UPDATED THE SECOND HALF OF THAT SENTENCE. It used to read "this is +NOT SAFETY-20; nothing here changes what `freeze.py` measures its rail against", +and that was true of this module and false of the system: this stage is exactly +what clips the emitted command below `RAIL_FRAC * max_w`, which is how a device +rating disarmed the freeze detector while the page said `armed`. `freeze.py` now +takes `dev_min`/`dev_max` and measures its rail against +`min(max_w, |dev_min|, |dev_max|)`. THE CLAMP ITSELF IS UNCHANGED - it still +clamps to `max_w` first and to the device range second, for the reasons above, +and it still does not `min()` the two. What changed is only that the detector +downstream now knows this stage exists. ------------------------------------------------------------------------------ WHERE THE WIND ACTUALLY SHOWS UP - READ THIS BEFORE "SIMPLIFYING" THE CALLER diff --git a/goodwe_controller/app/freeze.py b/goodwe_controller/app/freeze.py index 1a11544..16a6f69 100644 --- a/goodwe_controller/app/freeze.py +++ b/goodwe_controller/app/freeze.py @@ -151,12 +151,77 @@ THE RAIL TERM, PRECISELY reason SAFETY-02 uses the emitted value: the wire carries the emitted number. RAIL_FRAC is 0.95 and is a constant rather than an option. The command is -quantised to `step_w` (max 100) and clamped to `max_w` (min 100), so at the rail -the emitted value IS `max_w` on every schema-legal combination; the 5 % is slack -for a device rating that clips `max_w` slightly in `write_setpoint()`, not a -tuning knob. Making it configurable would offer a way to widen the one term that +quantised to `step_w` (max 100), so at the rail the emitted value IS the ceiling +on every schema-legal combination; the 5 % is quantisation slack, not a tuning +knob. Making it configurable would offer a way to widen the one term that carries the whole no-false-positive argument. +⚠️ SAFETY-20 - AND THE CEILING IS NOT `max_w`. THE 5 % USED TO BE DESCRIBED AS +"slack for a device rating that clips `max_w` slightly in `write_setpoint()`", +AND THAT SENTENCE WAS THE BUG. 5 % of slack does not absorb a device rating; it +inverts the guard. `write_setpoint()` clamps the emitted command to `max_w` AND +THEN to the setpoint device's own `[dev_min, dev_max]` range (clamp.py, +`clamp_to_rating()`), and the arbiter files the same range as a P_SAFETY limit +claim, so the largest magnitude that can ever stand on the wire is + + min(max_w, |dev_min|, |dev_max|) + +not `max_w`. Measured on the shipped predicate at `max_w: 5000` against a device +of +-4700 - the exact case the ticket is named for: + + looks_frozen(unchanged_s=3600, commanded_w=4700, max_w=5000) -> False + +The command is pinned at the hardware's rail, the meter has been frozen for an +hour, and term 2 is unsatisfiable BY CONSTRUCTION because 4700 < 4750. Any +device rating at or below `RAIL_FRAC * max_w` disarms the conjunction outright. +The status page then printed `armed (...command is over 4750 W)`, which is the +"disarmed guard that advertises itself as armed" failure validate.py rule 5 and +SAFETY-15/19 all exist to prevent - so `rail_ceiling_w()` below takes the device +range and the rail is measured against the smaller number. + +WHY THE SMALLER OF THE TWO DEVICE BOUNDS, AND NOT A SIDE-AWARE PAIR. The rail is +a MAGNITUDE test against what clamp.py calls a magnitude clamp - it binds import +and export identically - so the ceiling here is a magnitude that is available in +BOTH directions. The alternative considered was a per-sign rail: +`min(max_w, dev_max)` for a positive command, `min(max_w, |dev_min|)` for a +negative one. That is arithmetically tighter and it is NOT wrong - on a device +range of -500..+5000 it would grade a -480 W charge command as railed, which the +symmetric ceiling reaches only by dropping the discharge rail to 475 W too. It +was rejected on cost, not on correctness: a per-sign rail is two rails, and two +rails need two coverage verdicts (`dev_min == 0` is a degenerate charge rail +whatever the discharge side can do) on a status row that carries one number and +in a predicate that takes one magnitude. That is a bigger change than this +defect earns, and the ONLY configuration that distinguishes the two - a device +range narrower than `max_w` on one side - is refused at startup by CFG-01 (see +the reachability note below). Between the two, the symmetric ceiling can only +ever LOWER the rail, which widens term 2 rather than disarming it; term 1 +(`unchanged_s >= hold_s`) is the term carrying the no-false-positive argument, +and it is untouched. Whoever needs the tighter version should take both status +rows with it. + +⚠️ AND WHEN THE CEILING IS ZERO THE RAIL IS NOT "LOW", IT IS GONE. A device that +accepts no command in one direction at all (`dev_max == 0`, or `dev_min == 0`) +gives a ceiling of 0 W, and `abs(commanded_w) >= 0` is TRUE FOR EVERY COMMAND +INCLUDING 0 W. Term 2 would not be disarmed there, it would be degenerately +true, and the detector would collapse into the bare `unchanged_s > N` threshold +TEL-05 refused - false-tripping at the converged operating point we aim for. +That case is reported as NOT COVERED, through the row the unsupported-transport +case already uses, and `looks_frozen()` returns False rather than firing. + +⚠️ HOW REACHABLE IS ANY OF THIS - READ THIS BEFORE "SIMPLIFYING" IT AWAY. +`validate_startup_config()` rule 2 (CFG-01) already REFUSES TO START on +`max_w > dev_max` or `-max_w < dev_min`, and `run_control()` re-runs that gate +once ESPHome is up and the real rating is known. So on any install that reaches +the control loop with a rating it could read, `min(max_w, |dev_min|, |dev_max|)` +IS `max_w` and everything above is arithmetically dead. That is not an argument +against it - it is clamp.py's argument for its own second stage, word for word: +the case where this bites is the case where the startup gate did not run or no +longer holds (`hass.limits()` returns (None, None) whenever ESPHome is not up, +and the range checks are then skipped rather than guessed at, so a rating that +arrives late is never re-read), and a guard that quietly assumes the gate ran is +a guard that reports armed in exactly the state it cannot fire in. See AC 4's +decision, recorded in validate.py under WHAT IS DELIBERATELY *NOT* REFUSED. + ⚠️ NOT GATED ON `following`, and that is deliberate. A maintenance charge commands against the meter on purpose and can sit at the rail for hours - but it does so while pushing kilowatts through the meter, so the FIRST term is what @@ -192,6 +257,8 @@ otherwise would be worse than the gap. from dataclasses import dataclass from datetime import datetime +from .clamp import is_commandable + # See "THE RAIL TERM, PRECISELY" above. A constant, on purpose. RAIL_FRAC = 0.95 @@ -203,13 +270,72 @@ class FreezeWatchdogConfig: hold_s: float = 300.0 -def rail_w(max_w) -> float: - """The magnitude at or above which a command counts as railed.""" - return abs(float(max_w)) * RAIL_FRAC +def rail_ceiling_w(max_w, *, dev_min=None, dev_max=None) -> float: + """The largest command magnitude that can actually stand on the wire. + + SAFETY-20, and the whole point of the ticket: this is NOT `max_w`. The + emitted command is bounded by `max_w` AND by the setpoint device's own + `[dev_min, dev_max]` range - twice, in `clamp_to_rating()` and again as the + arbiter's `device` P_SAFETY limit claim - so the reachable ceiling is the + smallest of the three magnitudes. See "THE RAIL TERM, PRECISELY" for why it + is the smaller of the two device bounds rather than a per-sign pair, and for + why this is arithmetically dead on any install CFG-01 let start. + + `dev_min` / `dev_max` are `hass.limits()`'s answer and are None whenever + ESPHome could not be reached. UNKNOWN IS NOT ZERO: a missing bound means + this add-on applies no device clamp at all on that side (clamp.py's second + stage is skipped unless BOTH are known), so an unknown bound must not lower + the ceiling. `is_commandable()` is write_setpoint()'s own predicate, reused + rather than retyped: a NaN rating must not become a 0 W ceiling. + + Returns 0.0 when the device can command nothing - see rail_unreachable(). + """ + ceiling = abs(float(max_w)) + for bound in (dev_min, dev_max): + if is_commandable(bound): + ceiling = min(ceiling, abs(float(bound))) + return max(0.0, ceiling) + + +def rail_unreachable(max_w, *, dev_min=None, dev_max=None) -> str: + """"" when the rail is a real threshold, else WHY it is not. SAFETY-20 AC2. + + The status page renders this as a NOT COVERED row - the same row, for the + same reason, that an unsupported transport already gets. It is a second + reason to use that surface, not a new mechanism: in both states the honest + report is "this guard is not watching", and the one thing this project has + decided repeatedly it will not ship is a page that says `armed` instead. + + ⚠️ THE UNREACHABLE CASE IS NOT "THE RAIL IS HIGH", IT IS "THE RAIL IS 0 W". + A ceiling of zero makes `abs(commanded_w) >= 0` true for every command + including a parked 0 W one, so term 2 is not disarmed but DEGENERATELY TRUE, + and the conjunction decays into the bare `unchanged_s > N` threshold TEL-05 + refused - which false-trips at the converged operating point this controller + aims for. Silence plus a NOT COVERED row is strictly better than that. + """ + if not max_w or not is_commandable(max_w): + return (f"max_w is {max_w!r}, so there is no ceiling to measure a rail " + f"against") + if rail_ceiling_w(max_w, dev_min=dev_min, dev_max=dev_max) > 0.0: + return "" + return (f"the setpoint device reports a range of {dev_min}...{dev_max} W, " + f"which leaves no command magnitude available in both directions, " + f"so the rail term would be true of every command including 0 W") + + +def rail_w(max_w, *, dev_min=None, dev_max=None) -> float: + """The magnitude at or above which a command counts as railed. + + Measured against the REACHABLE ceiling, not against `max_w` - SAFETY-20. + Callers that do not know the device range pass none and get the old + behaviour, which is correct precisely when there is no device clamp to + account for. + """ + return rail_ceiling_w(max_w, dev_min=dev_min, dev_max=dev_max) * RAIL_FRAC def looks_frozen(unchanged_s, commanded_w, max_w, - cfg: FreezeWatchdogConfig) -> bool: + cfg: FreezeWatchdogConfig, *, dev_min=None, dev_max=None) -> bool: """The predicate, and the ONLY place the conjunction is written down. Public for the same reason `watchdog.disagrees()` is: the checks that grade @@ -220,12 +346,28 @@ def looks_frozen(unchanged_s, commanded_w, max_w, `unchanged_s` seconds since the meter served a different measurement, or None on a transport that cannot know (see the module note). `commanded_w` the EMITTED setpoint standing on the wire. - `max_w` the configured output bound the rail is measured against. + `max_w` the configured output bound. + `dev_min` / the setpoint device's own reported range, or None when it + `dev_max` could not be read. SAFETY-20: the rail is measured against + `min(max_w, |dev_min|, |dev_max|)`, because that - not + `max_w` - is what the wire can carry. + + ⚠️ THE DEVICE BOUNDS ARE KEYWORD-ONLY AND DEFAULT TO None, WHICH IS THE + CONSERVATIVE DIRECTION AND ALSO THE HONEST ONE. A caller that does not know + the rating gets a rail measured against `max_w` - correct, because when this + add-on has no rating it applies no device clamp either (clamp.py's second + stage needs both bounds), so `max_w` really is the ceiling on that path. """ if unchanged_s is None or commanded_w is None or not max_w: return False + # SAFETY-20. A ceiling of 0 W would make term 2 true of EVERY command; that + # is the degenerate case, not the armed one. Silence, and the status page + # says NOT COVERED - see rail_unreachable(). + if rail_unreachable(max_w, dev_min=dev_min, dev_max=dev_max): + return False return (float(unchanged_s) >= cfg.hold_s - and abs(float(commanded_w)) >= rail_w(max_w)) + and abs(float(commanded_w)) + >= rail_w(max_w, dev_min=dev_min, dev_max=dev_max)) class FreezeWatchdog: @@ -249,24 +391,37 @@ class FreezeWatchdog: self.reason = "" self.episodes = 0 - def evaluate(self, unchanged_s, commanded_w, max_w, now: datetime) -> bool: + def evaluate(self, unchanged_s, commanded_w, max_w, now: datetime, *, + dev_min=None, dev_max=None) -> bool: """Advance one cycle. Returns True ONLY on the rising edge. `now` is the caller's clock so this stays testable and replayable - the - same discipline as `SignWatchdog.evaluate()`. + same discipline as `SignWatchdog.evaluate()`. `dev_min`/`dev_max` are + the setpoint device's reported range; see looks_frozen(). """ - if not looks_frozen(unchanged_s, commanded_w, max_w, self.cfg): + if not looks_frozen(unchanged_s, commanded_w, max_w, self.cfg, + dev_min=dev_min, dev_max=dev_max): self.active = False self.since = None self.reason = "" return False + # ⚠️ SAFETY-20. THE REASON QUOTES THE CEILING IT ACTUALLY USED, not + # `max_w`. The old line said "at the 5000 W rail (... command over + # 4750 W)" on an install whose device stopped at 4700 W, i.e. it named + # a threshold the command could not reach - the same untruth the status + # page was telling, printed into the log and the HA notification where + # it would outlive the page. + ceiling = rail_ceiling_w(max_w, dev_min=dev_min, dev_max=dev_max) + capped = ("" if ceiling >= abs(float(max_w)) + else f", capped from max_w {float(max_w):.0f} W by the " + f"setpoint device's own rating") self.reason = ( f"METER_FROZEN: the meter has answered {float(unchanged_s):.0f} s " f"without its measurement changing while the command stands at " - f"{float(commanded_w):+.0f} W, at the {float(max_w):.0f} W rail " + f"{float(commanded_w):+.0f} W, at the {ceiling:.0f} W rail{capped} " f"(limits: unchanged over {self.cfg.hold_s:.0f} s, command over " - f"{rail_w(max_w):.0f} W)") + f"{rail_w(max_w, dev_min=dev_min, dev_max=dev_max):.0f} W)") if self.active: return False self.active = True diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 6cbc269..b416e7a 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -42,7 +42,7 @@ from .clamp import ( is_commandable, ) from .freeze import ( - FreezeWatchdog, FreezeWatchdogConfig, looks_frozen, rail_w, + FreezeWatchdog, FreezeWatchdogConfig, looks_frozen, rail_unreachable, rail_w, ) from .hass import HomeAssistant from .maintenance import IDLE, MaintConfig, Maintenance @@ -557,8 +557,14 @@ class Controller: "peak_soc_floor", peakguard.PEAK_OPTION_DEFAULTS["peak_soc_floor"])), max_hold_s=self.p1.averager.max_hold_s, + # ⚠️ SAFETY-20. The device range goes in TOO, and this call site is + # the one that needed it most: `standing` is `last_write`, i.e. the + # value AFTER `clamp_to_rating()` applied `[dev_min, dev_max]`, so + # grading it against a rail derived from bare `max_w` compares a + # device-clipped number to a ceiling the device cannot reach. frozen=looks_frozen(getattr(self.p1_source, "unchanged_s", None), - standing, max_w, self.freeze_wd.cfg), + standing, max_w, self.freeze_wd.cfg, + dev_min=self.dev_min, dev_max=self.dev_max), ) peak_claim = (peakguard.claim(self.peak_guard) if self.peak_guard is not None else None) @@ -1035,7 +1041,9 @@ class Controller: unchanged = getattr(self.p1_source, "unchanged_s", None) fired = self.freeze_wd.evaluate(unchanged, self.target, float(self.o.get("max_w", 2000)), - datetime.now(timezone.utc)) + datetime.now(timezone.utc), + dev_min=self.dev_min, + dev_max=self.dev_max) if not fired: return self.log_event("METER FROZEN? " + self.freeze_wd.reason) @@ -1270,7 +1278,20 @@ class Controller: self.mqtt.close() # -- UI ------------------------------------------------------------------ - def checks(self) -> list: + def checks(self, when: datetime | None = None) -> list: + """The status rows. `when` is the reference time for anything billed by + the calendar - today's month when it is None, which is what the live + page means; `status_payload()` calls this with no argument. + + ⚠️ IT EXISTS SO THE PEAK ROW CAN BE GRADED (TEST-17). Every other peak + query already takes it - `PeakStore.peak()`, `PeakTracker.peak()`, + `headroom_w()`, `warning_frac()` - and the row built from them was the + one thing that could only be read against `datetime.now()`. A suite + that banks an August quarter then asks this page what the peak is goes + green in August and red in September, which is what happened at 00:00 + on 2026-09-01: TARIFF-01's own section 10, red on a calendar boundary, + on a commit nobody had touched for a day. + """ o = self.o out = [] if self.p1_enabled: @@ -1319,26 +1340,60 @@ class Controller: f"meter_source {o.get('meter_source')} - only " f"homewizard_local sees whether the meter's own " f"document changes"}) + elif rail_unreachable(o.get("max_w", 2000), + dev_min=self.dev_min, dev_max=self.dev_max): + # SAFETY-20, and THE SECOND REASON TO USE THIS ROW RATHER THAN A + # NEW MECHANISM. The transport case above and this one are the + # same statement - "the conjunction cannot be evaluated here" - + # and an operator does not need to learn two vocabularies for + # it. What differs is only which term is missing: there, + # `unchanged_s`; here, a rail the command could ever stand at. + out.append({"ok": False, "warn": True, + "text": f"freeze detector: NOT COVERED - " + + rail_unreachable( + o.get("max_w", 2000), + dev_min=self.dev_min, + dev_max=self.dev_max)}) else: + # ⚠️ SAFETY-20. THE RAIL PRINTED HERE IS THE RAIL THE PREDICATE + # USES, device range included. This row used to interpolate + # `rail_w(max_w)` while `looks_frozen()` graded a command the + # device had already clipped below it, so on any install whose + # inverter is smaller than its configured ceiling the page named + # a number the command could never reach and called it `armed`. + # Both the row and the predicate now read the same three inputs. + _rail = rail_w(o.get("max_w", 2000), + dev_min=self.dev_min, dev_max=self.dev_max) + # And when the device is what set that number, SAY SO - the + # operator configured `max_w` and is owed the reason the guard + # is armed somewhere else. + # ⚠️ Either bound may be None on its own - `hass.limits()` + # reads `min` and `max` independently and returns None for + # whichever attribute is missing or unparseable - so this + # renders whatever is there rather than formatting a None. + _cap = ("" if _rail >= rail_w(o.get("max_w", 2000)) + else f" - below max_w because " + f"{o.get('setpoint_entity') or 'the setpoint device'}" + f" accepts only {self.dev_min}...{self.dev_max} W") out.append({"ok": True, "warn": False, "text": f"freeze detector: armed (alerts after " f"{self.freeze_wd.cfg.hold_s:.0f} s unchanged " f"while the command is over " - f"{rail_w(self.o.get('max_w', 2000)):.0f} W)"}) + f"{_rail:.0f} W{_cap})"}) # TARIFF-01. The month's billed peak, and - the load-bearing half - # a DIFFERENT row when the store could not answer. "no peak yet" is # a normal state on the 1st; "unreadable" is a broken instrument, and # a status page that spelled them the same way would let a 0 W read # look like a quiet month right up until SAFETY-07 shaved nothing. - _pk = self.peak.peak() + _pk = self.peak.peak(when) if _pk.is_unreadable: out.append({"ok": False, "warn": True, "text": "capacity-tariff peak: " + _pk.explain()}) else: # §9.1's "warning entity at 80 %", as a status row rather than # a second entity - see the constructor on not doubling up. - _frac = self.peak.warning_frac() or 0.0 + _frac = self.peak.warning_frac(when) or 0.0 _ok = _frac < 0.8 out.append({"ok": _ok, "warn": not _ok, "text": _pk.explain() diff --git a/goodwe_controller/app/validate.py b/goodwe_controller/app/validate.py index 1dbe6f7..14343dd 100644 --- a/goodwe_controller/app/validate.py +++ b/goodwe_controller/app/validate.py @@ -170,6 +170,29 @@ WHAT IS DELIBERATELY *NOT* REFUSED, because it is untidy rather than unsafe: - `maintenance_discharge_w` above `max_w`. Clamped, not contradicted, and the clamp names itself in the arbiter's explain() - see rule 6. Warned about, not refused. + - A device rating that makes SAFETY-18's freeze rail unreachable. SAFETY-20's + AC 4, decided here rather than left implicit: **NO NEW RULE, and rule 2 is + the reason.** The configuration the ticket is about is `max_w` above the + setpoint device's rating, which is rule 2 EXACTLY - already refused, on the + identical predicate, since CFG-01, and re-run by `run_control()` once + ESPHome is up and the real rating is known. A second rule phrased as + "`RAIL_FRAC * max_w` exceeds the device rating" would fire on a strict + subset of the configurations rule 2 already refuses (`0.95 * max_w > dev_max` + implies `max_w > dev_max`), so it could never be the rule that stopped a + start, and its whole visible effect would be a second entry in a refusal + list that already names the problem. + + ⚠️ AND IT WOULD NOT COVER THE CASE THAT REMAINS, WHICH IS WHY THE STATUS ROW + IS NOT A CONSOLATION PRIZE. The residue after rule 2 is the state where + `hass.limits()` answered `(None, None)` - ESPHome not up when the add-on + booted - so the range checks were SKIPPED rather than guessed at, and the + rating is never re-read afterwards. There is nothing to refuse at startup in + that state, by construction: the number a refusal would need does not exist + yet. That is a runtime coverage question, and the runtime surface - + `freeze.rail_unreachable()` and the NOT COVERED row it feeds - is where it + is answered. Compare rule 5, which CAN be settled at startup because both + `sign_disagree_w` and `max_w` are config and are both known at boot. + - `peak_soc_floor` below `maintenance_soc_floor`, or above it. They are DIFFERENT QUANTITIES (SAFETY-24: a drain target versus a spend limit) and every ordering of the two is a legitimate site configuration. Refusing on diff --git a/goodwe_controller/test_arbiter.py b/goodwe_controller/test_arbiter.py index 3ff0b08..11401b7 100644 --- a/goodwe_controller/test_arbiter.py +++ b/goodwe_controller/test_arbiter.py @@ -3,6 +3,13 @@ Each assertion is a precedence question someone will eventually ask in the field: "why did it charge during a peak?", "why did it discharge while the maintenance cycle was charging?", "what happens if two things disagree?". + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import sys diff --git a/goodwe_controller/test_arbitrage.py b/goodwe_controller/test_arbitrage.py index 1f030cf..f4882c8 100644 --- a/goodwe_controller/test_arbitrage.py +++ b/goodwe_controller/test_arbitrage.py @@ -62,6 +62,13 @@ THREE RULES THIS FILE HOLDS ITSELF TO, because the reviewer will: aged out, an hour the series does not cover, an unread state of charge and a peak guard that never ran are five DIFFERENT reasons - and none of them is an answer. Section 5 drives all five and asserts they are distinguishable. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import ast diff --git a/goodwe_controller/test_bytecode.py b/goodwe_controller/test_bytecode.py new file mode 100644 index 0000000..f7916e9 --- /dev/null +++ b/goodwe_controller/test_bytecode.py @@ -0,0 +1,296 @@ +"""Runnable check for TEST-15 - the mutation harness's own honesty. +`python3 test_bytecode.py` + +Every other file in this directory grades the add-on. This one grades the thing +that grades the add-on, because in this repository a mutation run IS the +evidence: "I broke it, the suite went red, I put it back, the suite went green" +is how nearly every safety claim here was established. + +THE DEFECT, found by FCST-03's developer and reproduced below. CPython decides a +cached `.pyc` is still good from two numbers: the source's SIZE, and its +MTIME TRUNCATED TO WHOLE SECONDS. Reverting `self.provisional` to +`here.provisional` moves neither - same length, same second - so the interpreter +loaded the bytecode it already had. Confirmed then by disassembly: the source +read `here`, the running code loaded `self`. The mutation was reported as a +survivor when in truth it had never run. + +WHICH DIRECTION IT LIES IN. A mutation that never took effect makes the suite +stay GREEN, so it is reported as a survivor: stale bytecode MANUFACTURES +SURVIVORS. The opposite - a revert that never took, leaving a red on correct +source - is possible too but announces itself, because a developer chases an +unexplained failure. That asymmetry is why the coverage-audit table in +test_control.py is a lower bound rather than nonsense, and it is what the survey +in MUTATION.md is scoped by. + +WHAT THIS FILE PINS, in the order the card's acceptance criteria name it: + + AC1 The defect reproduces (part 1), the two mitigations that DO NOT fix it + are shown not to (part 2), the one that does is shown to (part 3), and + the shipped harness is shown to close it end to end on a tree that would + otherwise mis-grade (part 4). Executed, not reasoned: every part builds a + real package, imports it in a real subprocess, and reads back which + value the running code returned. + AC2 The guarantee is where a developer writing a mutation reads it: part 5 + requires EVERY test_*.py in this directory to point at MUTATION.md. The + list is derived from the tree, so a new suite that joins without the + pointer is red - CI-01's rule, for CI-01's reason. + AC4 Part 6 pins the two properties that make CI immune, on the workflow file + itself. + +⚠️ NOTHING HERE SLEEPS. One-second mtime resolution is the mechanism; a harness +that waits out the clock would be slower on every run and would still only cover +the edit somebody remembered to slow down. Part 1 REPRODUCES the collision +deliberately (it retries until two writes land in the same second, which takes +milliseconds) rather than avoiding it. + +⚠️ This file writes only under a temporary directory. It must stay that way: CI +mounts the checkout READ-ONLY, and a check that needs to write into the tree +would fail there for a reason that has nothing to do with what it grades. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or use +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ. See MUTATION.md. +""" + +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +HERE = Path(__file__).resolve().parent +fails = [] + + +def check(name, cond): + if cond: + print(f" ok {name}") + else: + print(f" FAIL {name}") + fails.append(name) + + +# --------------------------------------------------------------------------- +# The fixture. `who()` returns one of two names that are THE SAME LENGTH, so +# swapping them leaves the file byte count identical - the first half of +# CPython's staleness test. The second half is the whole-second mtime, which +# part 1 arranges by writing both versions inside one second. +# +# It is deliberately not a copy of a shipped module: the property under test is +# CPython's, and using real code here would make this file go red whenever that +# module changed for unrelated reasons. +# --------------------------------------------------------------------------- +V_OLD = 'def who():\n self = "SELF"\n here = "HERE"\n return self\n' +V_NEW = 'def who():\n self = "SELF"\n here = "HERE"\n return here\n' +assert len(V_OLD) == len(V_NEW), "the fixture must be size-preserving or it grades nothing" + +PROBE = "from app.m import who\nprint(who())\n" +# The harness runs `test_*.py` files and grades them by exit status, so the +# probe wears that shape in part 4: it FAILS when it sees the pre-mutation value. +TEST_PROBE = ( + "from app.m import who\n" + "v = who()\n" + "print('RUNS:', v)\n" + "raise SystemExit(0 if v == 'HERE' else 1)\n" +) + + +def run(tree, args, env_extra=None, script="probe.py"): + env = dict(os.environ) + env.pop("PYTHONDONTWRITEBYTECODE", None) + if env_extra: + env.update(env_extra) + out = subprocess.run( + [sys.executable] + args + [script], + cwd=tree, env=env, capture_output=True, text=True, + ) + return out.stdout.strip() + + +def caches(tree): + return sorted(str(p.relative_to(tree)) for p in Path(tree).rglob("__pycache__")) + + +def seed(tree): + """Leave `tree` holding V_NEW on disk and V_OLD in its bytecode cache. + + Returns True when the collision was actually arranged. Both writes must land + in the same whole second; at a few milliseconds per cycle that is the normal + case, and the caller retries rather than asserting on one attempt, because a + machine that happens to cross a second boundary mid-cycle would otherwise + report a defect in CPython instead of a slow disk. + """ + app = Path(tree) / "app" + shutil.rmtree(app, ignore_errors=True) + app.mkdir(parents=True) + (app / "__init__.py").write_text("") + (Path(tree) / "probe.py").write_text(PROBE) + (Path(tree) / "test_probe.py").write_text(TEST_PROBE) + + m = app / "m.py" + m.write_text(V_OLD) + t0 = m.stat().st_mtime + run(tree, []) # compiles V_OLD, writes app/__pycache__ + m.write_text(V_NEW) # the "revert": same size, same second + t1 = m.stat().st_mtime + return int(t0) == int(t1) and caches(tree) != [] + + +def seeded_tree(stack): + tree = tempfile.mkdtemp(prefix="test_bytecode.") + stack.append(tree) + for _ in range(40): + if seed(tree): + return tree + return None + + +tmpdirs = [] +print("stale bytecode (TEST-15)") + +# --------------------------------------------------------------------------- +print("\n1. the defect: a same-size edit inside one second is not seen at all") +# --------------------------------------------------------------------------- +tree = seeded_tree(tmpdirs) +check("the collision can be arranged at all (same size, same second)", tree is not None) +if tree is None: + print(" cannot continue - the fixture never landed two writes in one second") + raise SystemExit(1) + +src = (Path(tree) / "app" / "m.py").read_text() +check("the source on disk says HERE", "return here" in src and "return self" not in src) +check("...and a plain interpreter runs SELF anyway", run(tree, []) == "SELF") +# Stated as its own check because this is the whole ticket in one line: what the +# file says and what the process executes have come apart, with no error, no +# warning, and a zero exit status. + +# --------------------------------------------------------------------------- +print("\n2. the two mitigations that do NOT close it") +# --------------------------------------------------------------------------- +# ⚠️ READ THIS BEFORE 'FIXING' A HARNESS WITH -B ALONE. Both of these stop +# bytecode being WRITTEN. Neither stops a stale one being READ, and a mutation +# run always has one already: the previous run wrote it. The advice in +# test_control.py's coverage audit said `-B` first and CLEAR the cache second; +# the order is not a style choice, the clear is the half that works. +check("`python3 -B` over an existing cache still runs SELF", run(tree, ["-B"]) == "SELF") +check( + "PYTHONDONTWRITEBYTECODE=1 over an existing cache still runs SELF", + run(tree, [], {"PYTHONDONTWRITEBYTECODE": "1"}) == "SELF", +) + +# --------------------------------------------------------------------------- +print("\n3. the purge closes it, and -B keeps the next run clean") +# --------------------------------------------------------------------------- +for p in Path(tree).rglob("__pycache__"): + shutil.rmtree(p, ignore_errors=True) +check("after the purge, the plain interpreter runs HERE", run(tree, []) == "HERE") +check("...and it has written a fresh cache back", caches(tree) != []) + +tree2 = seeded_tree(tmpdirs) +check("second fixture seeded", tree2 is not None) +if tree2: + for p in Path(tree2).rglob("__pycache__"): + shutil.rmtree(p, ignore_errors=True) + check("purge + -B runs HERE", run(tree2, ["-B"]) == "HERE") + check("...and leaves NO cache for the next mutation to inherit", caches(tree2) == []) + # The second half is why the harness passes -B at all. Purging before a run + # protects THIS run; not writing one protects the NEXT edit, which is the + # one nobody is thinking about when they are three mutations into a batch. + +# --------------------------------------------------------------------------- +print("\n4. the shipped harness, executed against a tree that would mis-grade") +# --------------------------------------------------------------------------- +harness = HERE / "tools" / "mutation_run.sh" +check("tools/mutation_run.sh exists", harness.is_file()) +check("...and is executable", os.access(harness, os.X_OK)) + +tree3 = seeded_tree(tmpdirs) +check("third fixture seeded", tree3 is not None) +if tree3 and harness.is_file(): + # NON-VACUITY FIRST. If the naked run below were already green, the harness + # run after it would prove nothing - it would be passing on a tree that + # needed no help. This is the same trap test_peakguard.py part 7 documents: + # a mutation harness that patches nothing passes beautifully. + naked = subprocess.run( + [sys.executable, "test_probe.py"], cwd=tree3, capture_output=True, text=True + ) + check("without the harness the tree grades the STALE source (rc!=0)", naked.returncode != 0) + check("...and says so: it saw SELF", "SELF" in naked.stdout) + + got = subprocess.run( + ["sh", str(harness), tree3], cwd=tree3, capture_output=True, text=True + ) + check("the harness grades the source on disk (rc==0)", got.returncode == 0) + check("...having reported the purge", "no bytecode cache" in got.stdout) + check("...and it leaves no cache behind", caches(tree3) == []) + if got.returncode != 0: + print(got.stdout[-2000:]) + print(got.stderr[-2000:]) + + # The harness must also refuse to report on nothing - CI-01's zero-file rule. + empty = tempfile.mkdtemp(prefix="test_bytecode.empty.") + tmpdirs.append(empty) + none = subprocess.run( + ["sh", str(harness), empty], cwd=empty, capture_output=True, text=True + ) + check("the harness refuses a tree with no test_*.py", none.returncode != 0) + +# --------------------------------------------------------------------------- +print("\n5. the guarantee is stated where a mutation gets written") +# --------------------------------------------------------------------------- +# ⚠️ THE LIST IS DERIVED FROM THE TREE. A hand-written one is how a new suite +# joins this directory carrying no warning at all, which is the exact shape of +# failure CI-01's file glob exists to prevent. If you add a test_*.py, put the +# pointer in its docstring; that is the whole cost. +doc = HERE / "MUTATION.md" +check("MUTATION.md exists", doc.is_file()) +if doc.is_file(): + text = doc.read_text() + check("...and it names the purge as the load-bearing half", "__pycache__" in text and "purge" in text.lower()) + check("...and warns that -B alone does not read fresh", "-B" in text) + check("...and points at the harness", "mutation_run.sh" in text) + +suites = sorted(p.name for p in HERE.glob("test_*.py")) +check("there are test suites to survey at all", len(suites) >= 15) +missing = [n for n in suites if "MUTATION.md" not in (HERE / n).read_text()] +check(f"every one of the {len(suites)} suites points at MUTATION.md", missing == []) +if missing: + print(" missing the pointer:", ", ".join(missing)) + +# --------------------------------------------------------------------------- +print("\n6. CI cannot carry a stale artefact between jobs") +# --------------------------------------------------------------------------- +# ⚠️ THE FIRST TWO PROPERTIES ARE THE ONES THAT ACTUALLY DO THE WORK, AND THEY +# PREDATE THIS CARD. Measured for TEST-15, not assumed: with the checkout bind +# mounted READ-ONLY the interpreter writes no __pycache__ into the tree at all, +# and it does so SILENTLY - no error, no warning, exit status 0. The workflow +# also `rm -rf`s its workspace and clones afresh, so nothing survives from a +# previous job even on the same runner. PYTHONDONTWRITEBYTECODE=1 is belt and +# braces on top: it is what keeps this true if somebody ever needs to drop the +# `:ro`, and dropping the `:ro` is a one-character edit that nothing else here +# would notice. +wf = HERE.parent / ".gitea" / "workflows" / "checks.yml" +check("the workflow file is where this expects it", wf.is_file()) +if wf.is_file(): + y = wf.read_text() + # ⚠️ GRADED ON THE COMMAND LINE, NOT ON THE FILE. A `in y` search over the + # whole workflow passes on the PROSE around the step - this file's first + # draft did exactly that, and stayed green after the `-e` flag and the `:ro` + # were deleted, because the comment explaining them still named both. + runs = [l for l in y.splitlines() if "docker run" in l and ":/w" in l] + check("exactly one docker run mounts the checkout", len(runs) == 1) + line = runs[0] if runs else "" + check("the checkout is mounted read-only", ':/w:ro"' in line) + check("PYTHONDONTWRITEBYTECODE=1 is set on that run", "-e PYTHONDONTWRITEBYTECODE=1" in line) + check("the workspace is removed before each checkout", 'rm -rf "$WS"' in y) + check("no sleep was introduced to dodge the mtime resolution", "sleep " not in y) + +for d in tmpdirs: + shutil.rmtree(d, ignore_errors=True) + +print() +if fails: + print(f"{len(fails)} FAILED: " + "; ".join(fails)) + sys.exit(1) +print("all checks passed") diff --git a/goodwe_controller/test_clamp.py b/goodwe_controller/test_clamp.py index b9e3d0c..b8c9958 100644 --- a/goodwe_controller/test_clamp.py +++ b/goodwe_controller/test_clamp.py @@ -47,6 +47,13 @@ WHAT THIS CHECK DOES NOT COVER asserted here over every value that reached `set_number()`, which is the last thing this process controls; proving it at the inverter needs the witness log and belongs with DRIVER-07. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import asyncio diff --git a/goodwe_controller/test_control.py b/goodwe_controller/test_control.py index 9dd7458..77b27c8 100644 --- a/goodwe_controller/test_control.py +++ b/goodwe_controller/test_control.py @@ -6,6 +6,13 @@ its absence caused an observed failure on real hardware. If you change control.py, run this. If it fails, the inverter would have done something you did not intend. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import sys @@ -71,14 +78,22 @@ def check(name, cond): # Hence: A FIXTURE MUST SIT CLEAR OF EVERY RAIL IT IS NOT TESTING. Where a test # names one mechanism, make that mechanism the binding one (see TCLAMP and TF). # -# ⚠️ RUN MUTATIONS WITH `python -B` AND CLEAR app/__pycache__. CPython -# invalidates a .pyc on (source mtime in whole seconds, source size), so a -# same-second rewrite that also preserves the file size reuses stale bytecode -# and the suite reports on code you are no longer running. It under-reported one -# mutation here as 2 where the true figure is 6. The error is one-directional - -# stale bytecode can only under-report - so every figure above is a lower bound -# at worst, and the two zeros ever recorded were both confirmed by fixing them -# and watching the count rise, which a caching artefact cannot do. +# ⚠️ RUN MUTATIONS THROUGH `tools/mutation_run.sh`, WHICH CLEARS __pycache__ +# BEFORE IT RUNS ANYTHING. CPython invalidates a .pyc on (source mtime in whole +# seconds, source size), so a same-second rewrite that also preserves the file +# size reuses stale bytecode and the suite reports on code you are no longer +# running. It under-reported one mutation here as 2 where the true figure is 6. +# +# ⚠️ AND THE CLEAR IS THE HALF THAT WORKS. This note used to lead with +# `python -B`; TEST-15 measured what that flag actually does, and it is +# write-only - it does not stop an EXISTING stale .pyc being read, and a +# mutation run always has one, because the previous run wrote it. -B is still +# worth passing, for the NEXT edit rather than this one. +# +# The error is one-directional - stale bytecode can only under-report - so +# every figure above is a lower bound at worst, and the two zeros ever recorded +# were both confirmed by fixing them and watching the count rise, which a +# caching artefact cannot do. # --------------------------------------------------------------------------- print("control law") diff --git a/goodwe_controller/test_freeze.py b/goodwe_controller/test_freeze.py index d9f6a9f..b870fc7 100644 --- a/goodwe_controller/test_freeze.py +++ b/goodwe_controller/test_freeze.py @@ -62,6 +62,13 @@ load the meter's POWER FIGURE never moved - 10130.0 W for 26 consecutive polls - and only the kWh registers kept `unchanged_s` at zero. See the note in freeze.py; it is why the fingerprint's register terms are the negative case rather than a nicety. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import asyncio @@ -79,7 +86,8 @@ import aiohttp from app.control import Tuning, compute from app.freeze import ( - RAIL_FRAC, FreezeWatchdog, FreezeWatchdogConfig, looks_frozen, rail_w, + RAIL_FRAC, FreezeWatchdog, FreezeWatchdogConfig, looks_frozen, + rail_ceiling_w, rail_unreachable, rail_w, ) from app.main import Controller from app.p1 import HomeWizardLocalSource, P1Ingest @@ -155,6 +163,87 @@ check("no transport signal (unchanged_s None) -> silent, never assumed frozen", check("no command yet -> silent", not looks_frozen(3600, None, 5000, cfg)) check("rail_w tracks RAIL_FRAC", rail_w(5000) == 5000 * RAIL_FRAC) + +# --------------------------------------------------------------------------- # +# 1c. SAFETY-20 - the rail is measured against what the WIRE can carry +# --------------------------------------------------------------------------- # +# ⚠️ EVERY CHECK IN THIS SECTION WAS RED BEFORE THE FIX AND IS THE FIX'S ONLY +# EVIDENCE. The shipped rail was `RAIL_FRAC * max_w` and the emitted command is +# clipped to the SETPOINT DEVICE's range as well (clamp.py's second stage, and +# the arbiter's `device` P_SAFETY claim), so any rating at or below +# `0.95 * max_w` made term 2 unsatisfiable by construction while the status page +# went on printing `armed (... over 4750 W)`. +# +# THIS IS ALSO THE CHECK THAT HAS TO GO RED IF ANYONE REVERTS THE RAIL TO BARE +# `max_w` (AC3). Verified by doing it - reinstating +# `return abs(float(max_w)) * RAIL_FRAC` in `rail_w()` fails the six checks +# below plus three in part 6, and nothing else in the suite. +print("SAFETY-20: the rail against the device rating") + +# The ticket's own numbers: a 5 kW ceiling on a 4.7 kW inverter. +DEV = dict(dev_min=-4700.0, dev_max=4700.0) + +check("the rail is the DEVICE's rail, not max_w's, when the device is smaller", + rail_w(LIVE_MAX_W, **DEV) == 4700.0 * RAIL_FRAC + and rail_w(LIVE_MAX_W, **DEV) != LIVE_MAX_W * RAIL_FRAC) +check("a command pinned at the device rating with a meter frozen for an hour " + "FIRES - this is the ticket, and it returned False before the fix", + looks_frozen(3600, 4700, LIVE_MAX_W, cfg, **DEV)) +check("...and the identical call without the rating is still False, so the " + "check above is measuring the rating and not something else", + not looks_frozen(3600, 4700, LIVE_MAX_W, cfg)) +check("the charge rail moves with it too - the runaway can be either sign", + looks_frozen(3600, -4700, LIVE_MAX_W, cfg, **DEV)) +check("just under the DEVICE rail is still silent, so the fix widened term 2 " + "by exactly the rating and not into a bare unchanged_s threshold", + not looks_frozen(3600, 4700 * RAIL_FRAC - 1, LIVE_MAX_W, cfg, **DEV)) +check("a device BIGGER than max_w cannot raise the rail - max_w is still a " + "ceiling the operator set and this is a min(), not a swap", + rail_w(LIVE_MAX_W, dev_min=-9000.0, dev_max=9000.0) + == LIVE_MAX_W * RAIL_FRAC) +check("an unread rating (ESPHome not up) leaves the old behaviour exactly - " + "no device clamp is applied on that path either, so max_w IS the ceiling", + rail_w(LIVE_MAX_W, dev_min=None, dev_max=None) == LIVE_MAX_W * RAIL_FRAC) +check("one bound alone still lowers it - hass.limits() reads min and max " + "independently and either can come back None on its own", + rail_w(LIVE_MAX_W, dev_max=4000.0) == 4000.0 * RAIL_FRAC) +check("a non-finite rating is IGNORED, never read as a 0 W ceiling", + rail_w(LIVE_MAX_W, dev_min=float("nan"), dev_max=float("inf")) + == LIVE_MAX_W * RAIL_FRAC) +check("the ceiling helper says what it measured", + rail_ceiling_w(LIVE_MAX_W, **DEV) == 4700.0) + +# The degenerate end. A ceiling of 0 W does not disarm term 2, it makes it TRUE +# OF EVERY COMMAND - including a parked 0 W one - which decays the conjunction +# into the bare `unchanged_s > N` threshold TEL-05 refused for false-tripping at +# this controller's own operating point. +check("a device that can command nothing makes the rail UNREACHABLE, with a " + "reason for the status page", + bool(rail_unreachable(LIVE_MAX_W, dev_min=0.0, dev_max=0.0))) +check("...and a reachable rail says so with an empty string, or the row above " + "would be permanent", + rail_unreachable(LIVE_MAX_W, **DEV) == "" + and rail_unreachable(LIVE_MAX_W) == "") +check("...and the predicate is SILENT there rather than degenerately true - " + "0 W would otherwise satisfy `abs(command) >= 0`", + not looks_frozen(3600, 0.0, LIVE_MAX_W, cfg, dev_min=0.0, dev_max=0.0) + and not looks_frozen(3600, 5000, LIVE_MAX_W, cfg, dev_min=0.0, + dev_max=0.0)) + +# The alarm text is the third surface that carried the untrue number - it +# outlives the page, in the log and in the HA notification. +_wd20 = FreezeWatchdog(cfg) +_wd20.evaluate(3600, 4700, LIVE_MAX_W, T0, **DEV) +print(f" at max_w {LIVE_MAX_W:.0f} on a +-4700 W device: " + f"{_wd20.reason}") +check("the alarm reason quotes the rail it actually used and NOT the max_w one " + "the command could never have reached", + f"command over {4700 * RAIL_FRAC:.0f} W" in _wd20.reason + and f"{LIVE_MAX_W * RAIL_FRAC:.0f} W" not in _wd20.reason) +check("...and it says the device is what capped it, so the operator is not " + "left wondering why the guard is armed somewhere other than max_w", + "rating" in _wd20.reason and "max_w" in _wd20.reason) + print("the edge") wd = FreezeWatchdog(cfg) edges = [wd.evaluate(u, c, 5000, T0 + timedelta(seconds=i)) @@ -649,19 +738,24 @@ def notes_by_text_of(hass): if "frozen" in str(d.get("title", "")).lower()] -def wired(unchanged_s, grid, cycles=60, **over): +def wired(unchanged_s, grid, cycles=60, dev=(-5000.0, 5000.0), **over): """Returns (hass, controller, trace). `trace` is one row per cycle - (command after the cycle, the watchdog's level, notifications raised so far) - which is what AC1 needs: the QUESTION IS NOT WHETHER THE ALARM HAPPENS BUT ON WHICH CYCLE. + + ⚠️ `dev` IS THE SETPOINT DEVICE'S OWN REPORTED RANGE, and it is a parameter + rather than a constant because SAFETY-20 is entirely about installs where it + is NOT the default +-5000. It defaults to the live install's rating, which + equals its `max_w` - which is exactly why that install never saw the defect. """ async def go(): hass = _Hass(grid) c = Controller(_opts(**over), hass, _Store(), _Pub(), stop=asyncio.Event()) c.auto = True - c.dev_min, c.dev_max = -5000.0, 5000.0 + c.dev_min, c.dev_max = dev c.p1_source = _Source(unchanged_s) trace = [] for _ in range(cycles): @@ -994,6 +1088,127 @@ check("...and it quotes the hold and the rail it is armed at, derived from the " check("...as an ok row, since a covered armed guard is not a warning", bool(_rows_c) and _rows_c[0]["ok"] and not _rows_c[0]["warn"]) +# --------------------------------------------------------------------------- # +# 5b. SAFETY-20 - is the device range actually WIRED into the guard? (AC1) +# --------------------------------------------------------------------------- # +# ⚠️ THIS SECTION EXISTS BECAUSE THE FIX SURVIVED ITS OWN MUTATION TEST WITHOUT +# IT. Deleting `dev_min=`/`dev_max=` from `check_meter_frozen()`'s call - i.e. +# shipping the corrected predicate and then not telling it the rating - left +# every check in 1c and 6b green, because those grade `freeze.py` and the status +# string and neither of them goes through `cycle()`. A guard that is right in the +# module and unwired in the caller is the defect with an extra step. +# +# So this drives the REAL `Controller.cycle()` on a controller configured the way +# the ticket describes - `max_w: 5000` on a +-4700 W inverter - and asserts on +# the notification that leaves the process. +# +# ⚠️ WHAT IT STILL DOES NOT COVER, stated rather than implied: the OTHER call +# site, `looks_frozen(...)` inside the SAFETY-07 peak-guard block, is behind +# `if self.p1_enabled` and needs a fed averager to have any observable +# consequence (`blindness()` returns BLIND_NO_BLOCK before it ever reads +# `frozen`). That gate is uncovered here for the same reason freeze_rows() says +# it is uncovered there, and widening the fixture belongs to whoever covers it. +print("SAFETY-20: the device range through the REAL cycle()") + +_h20, _c20, _t20 = wired(3600.0, 4000.0, dev=(-4700.0, 4700.0)) +_n20 = notes_of(_h20) +print(f" max_w 5000 on a +-4700 W device, meter frozen an hour: the command " + f"stood at {_c20.target:+.0f} W, {len(_n20)} freeze notification(s), " + f"watchdog active={_c20.freeze_wd.active}") +check("PRECONDITION: the device rating - not max_w - is what stopped the " + "command, so this really is the configuration the ticket is about", + abs(_c20.target) == 4700.0) +check("...and that command is BELOW the max_w rail, which is why the shipped " + "guard could never fire on this install", + abs(_c20.target) < rail_w(LIVE_MAX_W)) +check("...and at or above the device rail, which is why it must", + abs(_c20.target) >= rail_w(LIVE_MAX_W, dev_min=-4700.0, dev_max=4700.0)) +check("THE ALARM FIRES through the real cycle() - the rating reaches the " + "predicate from the controller, not just from a unit call", + len(_n20) == 1 and _c20.freeze_wd.active) +check("...and the notification the operator gets quotes the capped rail, " + "not the max_w one", + bool(_n20) + and f"{rail_w(LIVE_MAX_W, dev_min=-4700.0, dev_max=4700.0):.0f} W" + in str(_n20[0].get("message", ""))) +# p1_enabled is what a homewizard_local install has - set by hand here for the +# same reason part 4 sets it, see freeze_rows(). +_c20.p1_enabled = True +check("...and the red status row is up too, so the page and the notification " + "agree", + any("METER MAY BE FROZEN" in r["text"] for r in _c20.checks())) + + +# --------------------------------------------------------------------------- # +# 6b. SAFETY-20 - what the page says when the DEVICE is the ceiling (AC2) +# --------------------------------------------------------------------------- # +# ⚠️ THIS IS THE ACCEPTANCE CRITERION READ OFF THE RENDERED PAGE, NOT OFF THE +# SOURCE. The card is explicit that reading the code does not settle it, so +# every assertion below goes through the real `Controller.checks()` on a +# controller that was CONFIGURED with a device smaller than its `max_w` and +# then run - three real cycles, the real arbiter, the real write path. +print("what the page says when the device rating is the smaller ceiling") + +# max_w 5000 on a 4700 W inverter - the ticket's own install. Covered transport, +# so the only question is what the ARMED row says. +_ctl_s, _rows_s = freeze_rows(2.0, -14.2, dev=(-4700.0, 4700.0)) +# And the degenerate end: a device that can command nothing at all. +_ctl_z, _rows_z = freeze_rows(2.0, -14.2, dev=(0.0, 0.0)) +print(f" max_w 5000 on a +-4700 W device: {_rows_s[0]['text'] if _rows_s else '(no row)'}") +print(f" a device with no commandable range: {_rows_z[0]['text'] if _rows_z else '(no row)'}") + +check("a device smaller than max_w still gets exactly one freeze-detector row", + len(_rows_s) == 1) +check("...and the page does NOT print the max_w rail the command can never " + "reach - THE DEFECT, in one assertion", + bool(_rows_s) + and f"{rail_w(LIVE_MAX_W):.0f} W" not in _rows_s[0]["text"]) +check("...it prints the rail the predicate actually uses", + bool(_rows_s) + and f"{rail_w(LIVE_MAX_W, dev_min=-4700.0, dev_max=4700.0):.0f} W" + in _rows_s[0]["text"]) +check("...and it names the device range as the reason, since the operator " + "configured max_w and is owed the reason the guard is armed elsewhere", + bool(_rows_s) and "4700" in _rows_s[0]["text"] + and "max_w" in _rows_s[0]["text"]) + +# ⚠️ THE NUMBER IS TAKEN OFF THE ROW AND FED BACK INTO THE SHIPPED PREDICATE. +# Asserting the page against `rail_w(...)` alone proves the two agree, which a +# revert of BOTH to bare max_w also satisfies. What a revert cannot satisfy is +# this: the rail the page advertises has to be a magnitude the wire can carry, +# and the wire stops at the device rating. +_m = re.search(r"command is over (\d+(?:\.\d+)?) W", _rows_s[0]["text"]) if _rows_s else None +_page_rail = float(_m.group(1)) if _m else None +print(f" the page advertises a rail of {_page_rail} W against a device that " + f"stops at {_ctl_s.dev_max:.0f} W") +check("...the row states a rail in watts at all, so the parse below means " + "something", _page_rail is not None) +check("...and THE RAIL THE PAGE ADVERTISES IS ONE THE WIRE CAN ACTUALLY CARRY. " + "This is the whole ticket: 4750 W on a device that stops at 4700 W is a " + "guard that cannot fire, advertising that it can", + _page_rail is not None and _page_rail <= abs(_ctl_s.dev_max)) +check("...and a command at exactly that advertised rail, with the meter frozen, " + "really does trip the shipped predicate the row is describing", + _page_rail is not None + and looks_frozen(3600, _page_rail, LIVE_MAX_W, cfg, + dev_min=_ctl_s.dev_min, dev_max=_ctl_s.dev_max)) +check("...as an ok row - a guard armed at a lower rail is still armed, not a " + "warning", bool(_rows_s) and _rows_s[0]["ok"] and not _rows_s[0]["warn"]) + +check("a device with no commandable range gets exactly one row", len(_rows_z) == 1) +check("...and it says NOT COVERED - the rail is not merely low there, it is " + "0 W, which would be TRUE of every command including a parked one", + bool(_rows_z) and "NOT COVERED" in _rows_z[0]["text"]) +check("...and it NEVER says armed", + bool(_rows_z) and "armed" not in _rows_z[0]["text"]) +check("...flagged as a warning rather than a silent ok, like the uncovered " + "transport row it shares", + bool(_rows_z) and not _rows_z[0]["ok"] and _rows_z[0]["warn"]) +check("...and the predicate agrees with the page rather than firing behind it", + not looks_frozen(3600, 0.0, LIVE_MAX_W, cfg, + dev_min=_ctl_z.dev_min, dev_max=_ctl_z.dev_max)) + + # AC3's other half, through the built object rather than through a literal: the # opts dict does NOT carry meter_freeze_s, so this is the number an install that # leaves the key at its config.yaml default actually lands on. diff --git a/goodwe_controller/test_loop.py b/goodwe_controller/test_loop.py index 61292b6..69eba3a 100644 --- a/goodwe_controller/test_loop.py +++ b/goodwe_controller/test_loop.py @@ -19,6 +19,13 @@ HANDOVER.md. An understated runtime reads as a hang. `cycle_floor_s` is read from options with no lower bound in code, so a huge value reproduces the pre-fix behaviour exactly. Every claim below is therefore an A/B on one binary, not a comparison against a remembered old one. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import asyncio diff --git a/goodwe_controller/test_maintenance.py b/goodwe_controller/test_maintenance.py index 54172a6..b582f0d 100644 --- a/goodwe_controller/test_maintenance.py +++ b/goodwe_controller/test_maintenance.py @@ -3,6 +3,13 @@ Walks a whole cycle in simulated time, which is the test the YAML version never had - and it is exactly the phase transitions that a real cycle takes ten hours to exercise once a month. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import sys diff --git a/goodwe_controller/test_p1.py b/goodwe_controller/test_p1.py index 9d6e5b8..4983200 100644 --- a/goodwe_controller/test_p1.py +++ b/goodwe_controller/test_p1.py @@ -8,6 +8,13 @@ Every assert here is a rule whose absence poisons something downstream: an inverted sign inverts the control loop, a fabricated zero hides a dead meter, a naive per-phase sum misbills the capacity tariff, and a plain mean of samples computes the wrong quarter-hour figure whenever the telegram cadence changes. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import asyncio diff --git a/goodwe_controller/test_peakguard.py b/goodwe_controller/test_peakguard.py index 0485991..15fc55f 100644 --- a/goodwe_controller/test_peakguard.py +++ b/goodwe_controller/test_peakguard.py @@ -72,6 +72,13 @@ Two days is also not thirty. AC 3's 95 %-over-30-days criterion is TARIFF-01's and is still not met; `test_tariff.py`'s part 9 says so at length and this file does not re-litigate it. What part 6 reports is the comparison that two days CAN support: the same quarters, with and without the guard. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import asyncio diff --git a/goodwe_controller/test_price.py b/goodwe_controller/test_price.py index a454d7f..68132d0 100644 --- a/goodwe_controller/test_price.py +++ b/goodwe_controller/test_price.py @@ -34,6 +34,13 @@ TWO RULES THIS FILE HOLDS ITSELF TO, because the reviewer will: - the DST checks run on the 23-hour and 25-hour days, not on a normal 24-hour day where a fixed-24-slot implementation and a correct one agree exactly. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import ast diff --git a/goodwe_controller/test_price_forecast.py b/goodwe_controller/test_price_forecast.py index 8e1e9d7..26e1194 100644 --- a/goodwe_controller/test_price_forecast.py +++ b/goodwe_controller/test_price_forecast.py @@ -48,6 +48,13 @@ THREE RULES THIS FILE HOLDS ITSELF TO, because the reviewer will: and a stale one is not a fresh one: the gate blocks on `no_data`, blocks on an hour the series does not cover, and blocks on a provisional hour - three different reasons, none of them an answer. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import ast diff --git a/goodwe_controller/test_static_tariff.py b/goodwe_controller/test_static_tariff.py index 145b81e..f36a185 100644 --- a/goodwe_controller/test_static_tariff.py +++ b/goodwe_controller/test_static_tariff.py @@ -45,6 +45,13 @@ THREE RULES THIS FILE HOLDS ITSELF TO, because the reviewer will: price, and zero is exactly the value that makes everything look free. So `static_plan` with no profile is checked to produce a read with NO `.points`, and `price_at` on it must RAISE rather than answer 0.0. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import gzip diff --git a/goodwe_controller/test_tariff.py b/goodwe_controller/test_tariff.py index 2060317..966186a 100644 --- a/goodwe_controller/test_tariff.py +++ b/goodwe_controller/test_tariff.py @@ -85,6 +85,13 @@ regression fixture that CI cannot see is not a regression fixture. The recorder's window rolls: 2026-08-01 leaves it around 2026-09-15, which is why the committed copy is the artefact and not the API call. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import asyncio @@ -863,7 +870,12 @@ check("...and syncing the same quarter again banks nothing new, so the count " check("...and the peak is unchanged by the repeat", near(c.peak.peak(B).peak_w, 4998.0, 1e-6)) -rows = [r for r in c.checks() if "capacity-tariff" in r["text"] or "peak" in r["text"]] +# ⚠️ `when=B`, AND THAT ARGUMENT IS THE TICKET (TEST-17). Without it `checks()` +# reads the peak for `datetime.now()`'s month while this fixture banked its +# quarter in August, so every check below passed in August and failed from +# 2026-09-01 - on a commit nobody had touched. The suite must grade the month +# it created, not the month the machine happens to be running in. +rows = [r for r in c.checks(when=B) if "capacity-tariff" in r["text"] or "peak" in r["text"]] peak_rows = [r for r in rows if "peak" in r["text"] and "quarter(s) banked" in r["text"]] print(" status row: " + (peak_rows[0]["text"] if peak_rows else "(none)")) check("the status page carries exactly one capacity-tariff peak row", @@ -879,7 +891,7 @@ check("...and it is FLAGGED, because 4998 W is over §9.1's 80 % warning point " # The half that matters: the same page with a store that cannot answer. c2 = built(TMP / "no-such-dir" / "nope.db") -u_rows = [r for r in c2.checks() if "capacity-tariff peak" in r["text"]] +u_rows = [r for r in c2.checks(when=B) if "capacity-tariff peak" in r["text"]] print(" status row (unreadable store): " + (u_rows[0]["text"] if u_rows else "(none)")) check("an unreadable store gets its OWN row, saying UNREADABLE", len(u_rows) == 1) @@ -891,6 +903,58 @@ check("...and the healthy row and the unreadable row are different text", bool(u_rows) and bool(peak_rows) and u_rows[0]["text"] != peak_rows[0]["text"]) +# --- TEST-17: the page must be graded at a stated time, and prove it ---------- +# Three ways this can regress, so three checks. The first two would both go +# green again if `checks()` quietly went back to reading the clock, which is +# exactly the mutation this section could not previously detect. + +# ⚠️ SELECTED ON "quarter(s) banked", NOT ON "peak". The NO_RECORD row reads +# "2026-10: no completed quarter recorded yet - ..." and contains neither the +# word "peak" nor "capacity-tariff" - only the UNREADABLE branch prefixes the +# family name. Filtering these rows the way the checks above do would drop the +# row and then grade its absence, which is the very mistake this ticket fixes. +# (That asymmetry in the row's own text is worth a look on its own; it is what +# made the original filter brittle. Not changed here - it is product text with +# other suites reading it.) +_peak_rows = lambda rows: [r for r in rows if "quarter(s) banked" in r["text"]] + +_later = B + timedelta(days=45) # same fixture, a later billing month +_other_peak = _peak_rows(c.checks(when=_later)) +print(f" the SAME controller read at {_later.date()}: " + + (_other_peak[0]["text"] if _other_peak else "(no peak row)")) +check("asked for a LATER month, the same controller reports that month's " + "absence of a quarter, not August's 4998 W - the row follows `when`", + bool(_other_peak) and "4998 W" not in _other_peak[0]["text"] + and "no completed quarter recorded yet" in _other_peak[0]["text"]) +check("...and that is NO_RECORD, distinguishable from BOTH an observed peak and " + "an unreadable store - a month with no quarter is not a broken instrument", + bool(_other_peak) and bool(peak_rows) + and "UNREADABLE" not in _other_peak[0]["text"] + and _other_peak[0]["text"] != peak_rows[0]["text"]) +check("...and it is not FLAGGED, because an empty month is not an exceedance", + bool(_other_peak) and _other_peak[0]["ok"] and not _other_peak[0]["warn"]) + +# The mutation guard. `checks()` ignoring `when` is the whole defect, and it is +# invisible to every check above whenever the machine's clock happens to sit in +# the fixture's own month - which is how this shipped. Grading two DIFFERENT +# reference months against each other cannot be satisfied by a clock at all. +check("the peak row differs between two reference months, so a `checks()` that " + "dropped `when` and read datetime.now() would fail HERE regardless of " + "what day the suite is run on", + bool(peak_rows) and bool(_other_peak) + and peak_rows[0]["text"] != _other_peak[0]["text"]) + +# And the production meaning is unchanged: no argument still means today. +_now_peak = _peak_rows(c.checks()) +_today_is_fixture_month = month_key(datetime.now(timezone.utc)) == month_key(B) +check("calling checks() with NO argument still reads the month being billed " + "today - the live status page's meaning is untouched", + bool(_now_peak) and (bool(peak_rows) + and _now_peak[0]["text"] == peak_rows[0]["text"] + if _today_is_fixture_month + else "no completed quarter recorded yet" + in _now_peak[0]["text"])) + # THE MEASUREMENT IS OF DELIVERED POWER (SAFETY-20). Graded on the parsed # SYNTAX, not on the text: the module docstring argues this at length and names # every forbidden word while doing so, so a grep over the source would be diff --git a/goodwe_controller/test_validate.py b/goodwe_controller/test_validate.py index 4645058..eb08fae 100644 --- a/goodwe_controller/test_validate.py +++ b/goodwe_controller/test_validate.py @@ -5,6 +5,13 @@ three refusable configurations are refused. The second asserts that everything on validate.py's "deliberately not refused" list still STARTS - because the way a validator dies is not by missing a fault, it is by refusing something harmless until somebody deletes the call. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import asyncio @@ -76,6 +83,40 @@ check("an unreachable device skips the range check rather than assuming", check("a non-numeric max_w is refused", why(validate_startup_config(base(max_w="lots"), -5000, 5000), "not a number")) +# --------------------------------------------------------------------------- # +# 1b. SAFETY-20's AC4 decision, PINNED HERE RATHER THAN ONLY WRITTEN DOWN +# --------------------------------------------------------------------------- # +# AC4 asked whether a device rating that disarms SAFETY-18's freeze rail +# additionally warrants a refusal of its own, the way `sign_disagree_w >= max_w` +# is refused. THE ANSWER IS NO, AND THE REASON IS RULE 2 ABOVE: the configuration +# is `max_w` above the device rating, which rule 2 already refuses on the +# identical predicate. A rail-specific rule (`0.95 * max_w > dev_max`) would fire +# on a strict SUBSET of what rule 2 refuses, so it could never be the rule that +# stopped a start. +# +# ⚠️ THAT DECISION HAS A PREMISE, AND THIS IS THE PREMISE, EXECUTED. If rule 2 is +# ever softened to a warning - which is exactly what main.py used to do before +# CFG-01 - the reasoning recorded in validate.py's "deliberately not refused" +# list silently stops being true, and SAFETY-18 is left with only its runtime +# NOT COVERED row. Then somebody has to come back and re-decide AC4. +from app.freeze import RAIL_FRAC # noqa: E402 - imported here, beside its use + +_DEV = 4700.0 # the ticket's inverter +_CEIL = 5000.0 # the ticket's configured ceiling +check("SAFETY-20's premise: a rating below max_w disarms the freeze rail, so " + f"{RAIL_FRAC:g} * max_w is unreachable on the ticket's install", + _DEV < RAIL_FRAC * _CEIL) +check("...and rule 2 REFUSES that whole configuration at startup, which is why " + "SAFETY-20 added no rule of its own (AC4)", + len(validate_startup_config(base(max_w=_CEIL), -_DEV, _DEV)) == 1) +check("...on the rating side specifically, not incidentally through some other " + "rule", why(validate_startup_config(base(max_w=_CEIL), -_DEV, _DEV), + "THE DEVICE WINS")) +check("...and the case the refusal CANNOT reach - the rating unread because " + "ESPHome was not up - still starts, which is precisely why SAFETY-20's " + "answer had to be a runtime status row and not a startup gate", + validate_startup_config(base(max_w=_CEIL), None, None) == []) + # --------------------------------------------------------------------------- # print("2. an actuator to write to") # A controller with no actuator: write_setpoint() returns on the blank entity, diff --git a/goodwe_controller/test_watchdog.py b/goodwe_controller/test_watchdog.py index 9d78e69..af422e9 100644 --- a/goodwe_controller/test_watchdog.py +++ b/goodwe_controller/test_watchdog.py @@ -8,6 +8,13 @@ person switches it off. The headline validation is not here - it is `sim/replay_sign.py` in the infra repo, which drives this module over the two captured bus days. This file is the table-driven half: boundaries, timers, the latch, and the arming gate. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. """ import sys diff --git a/goodwe_controller/tools/mutation_run.sh b/goodwe_controller/tools/mutation_run.sh new file mode 100755 index 0000000..37ce243 --- /dev/null +++ b/goodwe_controller/tools/mutation_run.sh @@ -0,0 +1,95 @@ +#!/bin/sh +# The house mutation harness (TEST-15). `tools/mutation_run.sh [TREE] [test_*.py ...]` +# +# WHY THIS EXISTS. Nearly every safety claim in this repository was verified by +# mutation: change the shipped module, run the suite, watch it go red, revert, +# watch it go green. FCST-03's developer found that the middle two steps can be +# a lie. CPython decides a `.pyc` is still valid from (source size, source mtime +# TRUNCATED TO WHOLE SECONDS). Revert `self.provisional` to `here.provisional` +# and neither term moves - same length, same second - so the interpreter loads +# the bytecode it already has. Confirmed by disassembly: the source read `here`, +# the running code loaded `self`. The mutation "survived" because it never ran. +# +# ⚠️ `python3 -B` IS NOT THE FIX, AND NEITHER IS PYTHONDONTWRITEBYTECODE=1. +# Both stop bytecode being WRITTEN. Neither stops a stale `.pyc` that is already +# on disk from being READ - measured, and graded by test_bytecode.py part 2. +# The load-bearing half is the PURGE. `-B` is here so the run leaves no cache +# behind for the NEXT mutation to inherit. +# +# ⚠️ NOT A SLEEP. One-second mtime resolution is the mechanism, not the enemy; +# the answer is to stop trusting mtime, not to make every edit wait a second. +# A sleep also only covers the edit you remembered to slow down. +# +# It prints a fingerprint of the tree it is about to grade, because the other +# half of a mutation report is WHICH SOURCE produced it: a report that cannot +# say what was on disk is the same unverifiable claim in a different dress. +# Diff two fingerprints and the mutated file is the line that changed. +set -eu + +TREE="" +case "${1-}" in + "" ) : ;; + -h|--help ) sed -n '2,30p' "$0"; exit 0 ;; + * ) if [ -d "$1" ]; then TREE=$1; shift; fi ;; +esac +if [ -z "$TREE" ]; then + # Default: the add-on directory this script ships in, so `tools/mutation_run.sh` + # works from anywhere. An explicit TREE is what lets the infra repo's replay + # harnesses purge $GOODWE_ADDON_REPO before they grade it - see MUTATION.md. + TREE=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd) +fi +PY=${PY:-python3} + +cd "$TREE" + +# CI-01's rule, for the same reason: a loop over an empty glob exits 0, and a +# green run that examined nothing is worse than a red one. +if [ $# -eq 0 ]; then set -- test_*.py; fi +if [ ! -f "$1" ]; then + echo "mutation_run: no test_*.py under $TREE - NOTHING has been graded." >&2 + exit 1 +fi + +echo "== purge" +find . -name '__pycache__' -type d -prune -exec rm -rf {} + 2>/dev/null || true +find . -name '*.pyc' -type f -exec rm -f {} + 2>/dev/null || true +left=$(find . -name '__pycache__' -type d -o -name '*.pyc' -type f | head -5) +if [ -n "$left" ]; then + # A read-only tree cannot hold a cache written during this run either, so this + # is only reachable when the purge itself failed on a tree that HAS one - the + # one state where continuing would grade unknown bytecode. + echo "mutation_run: could not purge, and a cache remains:" >&2 + echo "$left" >&2 + exit 1 +fi +echo " no bytecode cache under $TREE" + +echo "== tree graded (sha256, bytes)" +find app -name '*.py' 2>/dev/null | sort | while read -r f; do + printf ' %s %8d %s\n' "$(sha256sum "$f" | cut -c1-16)" "$(wc -c < "$f")" "$f" +done + +echo "== run ($PY -B)" +rc=0 +for f in "$@"; do + start=$(date +%s) + if "$PY" -B "$f" > /tmp/mutation_run.out 2>&1; then + echo " PASS $f ($(( $(date +%s) - start ))s)" + else + echo " FAIL $f ($(( $(date +%s) - start ))s)" + tail -40 /tmp/mutation_run.out + rc=1 + fi +done + +# -B was asked for; check it was honoured rather than assuming. A cache here +# would be inherited by the next mutation in the batch, which is exactly the +# state this script exists to prevent. +after=$(find . -name '__pycache__' -type d -o -name '*.pyc' -type f | head -5) +if [ -n "$after" ]; then + echo "mutation_run: a bytecode cache appeared DURING the run - the next" >&2 + echo "mutation in this batch would inherit it:" >&2 + echo "$after" >&2 + rc=1 +fi +exit $rc