3 Commits

Include renames

Author	SHA1	Message	Date
glenn schrooyenandClaude Opus 5	d5b1ef19b3	CFG-03: give maintenance option defaults one source of truth ... Option defaults for the maintenance cycle were declared in three places that could disagree, and MAINT-04 proved they already did. config.yaml shipped `maintenance_charge_w: 2000`; main.py carried TWO `opts.get(..., 2500)` fallbacks; MaintConfig's dataclass carried a third `2500.0`. So an options dict with the key ABSENT was ACCEPTED by validate.py's rule 6 - which read 2000 - and then built by main.py at 2500, against a 2000 W ceiling: options dict with maintenance_charge_w ABSENT, max_w=2000 validate_startup_config -> [] (accepted, no problems) MaintConfig().charge_w -> 2500.0 arbiter.resolve() -> 0.0 W, contradiction=True That is the bug rule 6 exists to refuse, passing rule 6. It is unreachable on a Supervisor install, which always supplies every key from config.yaml, and fully reachable from sim/inject.py and from the test suite - which is where anyone debugging maintenance actually works. MAINT-04 shipped a TRIPWIRE and said so: test_validate.py pinned all three literals and asserted the consequence through the real arbiter, so the divergence could not silently widen or be half-closed. Its PASSING CONDITION WAS THE BUG. Both it and validate.py's comment said to delete each other when the divergence was closed. It is closed, so both are deleted. THE DIVERGENCE WAS WIDER THAN ONE KEY. `maintenance_discharge_w` had the identical three-way split (config.yaml 2000, MaintConfig 2500) and the card named only the charge side. Closing one and leaving its twin would have satisfied the numbers and left three sources of truth one line down, which is the state that produced the bug. Both are closed, and so is the whole family: all eight `maintenance_*` keys go through the same mechanism. WHAT THE SINGLE SOURCE IS. config.yaml's `options:` block - it is what the Supervisor reads and what the operator edits, so it wins by definition. There is now exactly one in-code copy, `maintenance.MAINT_OPTION_DEFAULTS`, which is TARIFF-02's `PRICE_OPTION_DEFAULTS` and SAFETY-24's `PEAK_OPTION_DEFAULTS` pattern applied to the family that named the card - validate.py's rule 8 comment already called that "CFG-03 read as a rule rather than as a complaint", so this makes the rule hold where it was first broken rather than inventing a fourth mechanism for one option family. A MIRROR RATHER THAN A RUNTIME READ, and the reason is mechanical: the Dockerfile copies `app/` and `run.sh` into the image and NOT config.yaml. The running add-on cannot open the file it is configured by, so there is nothing to read at 03:00 on a client's Pi, and a YAML-subset parser that could fail at import is a new way for the add-on not to start. What makes the copy safe is that test_maintenance.py pins every entry by READING the shipped file - drift is a CI failure, not a comment. EVERY OTHER SITE NOW DERIVES FROM IT OR HAS CEASED TO EXIST: * MaintConfig's eight dataclass defaults are indexed out of the mirror. No numeric literal survives in the class that held the third opinion. * `MaintConfig.from_options()` is the ONLY place in the add-on that reads a `maintenance_*` option. main.py's eight inline `opts.get(key, <literal>)` calls are gone: moving the read next to the mirror means the fallback cannot be anything but the mirror, because there is no literal in sight of it to disagree with. It uses `key in opts` rather than truthiness, so a shipped `false` or `0` is honoured instead of being quietly replaced by the default - this card's own bug in a different costume. * main.py's SECOND fallback site - the cheap-window charge floor at the P_MAINT_SHAPE claim - reads `self.maint.cfg.charge_w` instead of re-opening the options dict with its own literal. The Controller had already parsed this once in __init__; asking the object that owns it cannot drift from it, and the rate the shaping asks for can no longer be a different number from the rate the state machine drains at on the same install. * validate.py's rule 6 and its `maintenance_discharge_w` warning take their fallbacks from the mirror. WHERE THE RANGE LIVES, STATED AND CHECKED. config.yaml's `schema:` block carries `maintenance_charge_w: int(500,5000)` - a fourth declaration site of a different kind. It is a BOUND, not a default, so it is not this bug; but leaving the question open is how a fifth site arrives. The decision recorded in config.yaml and in maintenance.py is: bounds live in the schema block ONLY, because the Supervisor is what enforces them and the Supervisor reads only that file, so a Python copy of `int(500,5000)` would be a place to disagree and nothing else. test_maintenance.py checks both halves of that: every mirrored key must have a schema entry - without one the Supervisor drops the option and the add-on silently runs on the mirror instead of the operator's value - and every shipped default must lie INSIDE its own range, since a default the schema rejects is a config that cannot be re-entered once it is edited. What the schema grammar cannot express is a relation BETWEEN keys (`maintenance_charge_w` against `max_w`), which is exactly why rule 6 is written in Python and why it does not belong in the mirror either. THE REPLACEMENT CHECKS, AND HOW THEY WERE VERIFIED. test_validate.py's new section exercises the same four consumers the defect ran through, the same way: the validator ACCEPTS a dict with every maintenance key missing; a REAL Controller built from that same dict holds 2000 W (through main.py, not through a regex over its source); `MaintConfig()` answers 2000 W with no dict at all; and the real arbiter emits `-2000 W, limited by supervised(max_w)` instead of `0 W, contradiction=True`. test_maintenance.py pins the mirror to config.yaml in both directions - equal values, and full coverage of every `maintenance_*` key the file ships, so a ninth key cannot arrive unmirrored - and asserts the consumers move as a RELATION rather than against numbers, reading MaintConfig's defaults back out of `dataclasses.fields()`. Each new check was verified by BREAKING the thing it watches and observing red, this project's rule: 1. config.yaml -> 1500, mirror left at 2000: test_maintenance 1 FAILED, test_validate 4 FAILED. 2. config.yaml AND the mirror -> 1500: green again, and every consumer moved together - MaintConfig() 1500 W, the Controller 1500 W, and the arbiter `-1500 W, limited by maintenance(cheap-window charge)` where it had said `-2000 W, limited by supervised(max_w)`. That is AC4: change the single source and watch the consumers follow. 3. `charge_w: float = 2500.0` retyped into the dataclass: 2 FAILED in each of the two files. 4. `opts.get("maintenance_charge_w", 2500)` restored in main.py: 1 FAILED, naming the file and the literal it found. 5. `maintenance_hold_min` deleted from config.yaml's options block: 3 FAILED. Full suite 17/17 in goodwe-addon-ci, twice, once under parallel load. Stale prose that asserted the now-closed divergence as current is corrected in app/watchdog.py, test_watchdog.py and test_peakguard.py - watchdog.py's example still uses 1500 W, but because the point of the example is a charge strictly inside the ceiling, not because 2500 is what ships. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-01 06:16:54 +02:00
glenn schrooyenandClaude Opus 5	7b4877d56d	TEST-15: purge the bytecode cache before a mutation run, not after ... FCST-03's developer reverted `self.provisional` to `here.provisional`, re-ran, and reported the mutation as a survivor. It was not one. CPython validates a cached `.pyc` on (source size, source mtime truncated to whole seconds), and that edit moved neither number - same length, same second - so the interpreter loaded the bytecode it already had. Disassembly settled it at the time: the source read `here`, the running code loaded `self`. A mutation run had graded code that was not there, with no error and exit status 0. This does not make the results already on the record wrong. It makes them UNVERIFIED, and that distinction is the whole card - so nothing here re-runs a campaign and nothing here quietly assumes the old numbers hold. REPRODUCED, THEN CLOSED. test_bytecode.py builds a real package whose two return values are the same length, imports it in a real subprocess, rewrites it inside the same second (retrying until both writes land in one whole second - milliseconds, no sleep) and reads back which value ran. Part 1 shows the interpreter running SELF off a file that says HERE. AND THE TWO OBVIOUS MITIGATIONS DO NOT WORK. Measured, not read off documentation: `python3 -B` and PYTHONDONTWRITEBYTECODE=1 are write-side only. Neither stops an EXISTING stale .pyc being read, and a mutation run always has one, because the previous run wrote it. Part 2 grades both still running the old code. The advice in test_control.py's coverage audit led with `-B`; it is corrected here - the purge is the half that works, and `-B` earns its place by keeping THIS run from leaving a cache for the NEXT edit to inherit. NO SLEEP ANYWHERE. One-second mtime resolution is the mechanism, not the enemy. Waiting it out would slow every run down and would still only cover the edit somebody remembered to slow down. THE HARNESS. tools/mutation_run.sh purges, prints a sha256/byte fingerprint of every app/*.py it is about to grade - so a mutation report can say WHICH source produced it, which is the reviewer's addition to this card - runs each suite under `python3 -B`, refuses a tree with no test_*.py (CI-01's rule, for CI-01's reason), and fails if a cache appears during the run. It takes the tree as an argument so the infra repo's replay harnesses can call it without being moved. WHERE IT IS SAID (AC 2). MUTATION.md carries the mechanism, the measurement, the reporting rules and the survey, and every one of the 16 suites now points at it from its docstring. That list is DERIVED FROM THE TREE by test_bytecode.py part 5, so a new suite that joins this directory without the pointer is red - the same failure shape CI-01's file glob exists to prevent. CI WAS ALREADY IMMUNE, AND IS DESCRIBED THAT WAY (AC 4). Verified rather than reasoned: with the checkout bind mounted `-v "$WS:/w:ro"` the interpreter writes no __pycache__ into the tree at all, and it fails SILENTLY - no error, exit status 0 - while the same probe on a writable mount produces one. Each job also rm -rf's its workspace and clones afresh, so nothing crosses between jobs on the shared runner. PYTHONDONTWRITEBYTECODE=1 is added as belt and braces only: the `:ro` is one character wide and the next person debugging a write will be tempted by it. Part 6 grades both ON THE DOCKER COMMAND LINE rather than on the file - the first draft searched the whole workflow and stayed green after the flag was deleted, because the comment explaining it still named it. THE SHIPPED ADD-ON IS NOT AFFECTED and the question is closed: goodwe_controller/Dockerfile has set PYTHONDONTWRITEBYTECODE=1 since it was written. There is no deploy-side finding. THE SURVEY (AC 3), in MUTATION.md, scoped BY THE PROPERTY. The defect is one-directional - a mutation that never took leaves the suite green, so stale bytecode manufactures SURVIVORS and cannot manufacture a red - which puts every "all red, no survivors" claim in the window out of scope, along with mutations executed in-process (SAFETY-07's B1/B2/B3, test_peakguard parts 7 and 10, where no source file is edited) and everything run after the purge discipline landed on 2026-08-31 (SAFETY-24, SAFETY-26, OPT-04, FCST-03's re-run). What is left is the recorded SURVIVORS from source-edit runs before that date. Two were re-run here under the harness and both still stand: TARIFF-01's record_block() ignoring its policy argument, and SAFETY-03's abs() on the clamp limit - 15 suites green each, on the tree as it stood. The rest are labelled still-unverified rather than assumed or re-run; each is one harness run to close, and the moment to spend it is when somebody next touches that module. OUT OF SCOPE AND RECORDED: the infra repo's sim/ replay harnesses import the add-on and write bytecode into both trees (goodwe/sim/__pycache__ exists today), so SAFETY-02's headline replay carries the same exposure. mutation_run.sh takes a tree argument for exactly that, but the change belongs in that repo and some of those harnesses want the rig. RED AND GREEN. Every new check was broken and watched fail: the purge commented out of the harness turns part 4 red (and its non-vacuity probe already proves the fixture would mis-grade without it); one docstring pointer removed turns part 5 red naming the file; `:ro` and the `-e` flag removed from the workflow turn part 6 red. Full suite green, 16 files, in the CI image on a read-only mount - the shape the runner uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-01 02:07:09 +02:00
adminandClaude Opus 5	0a1e61dbc9	Packaged as a Home Assistant add-on, with a field guide ... Turns the reference RS485 controller into something a technician can install at a client site: a typed config form instead of YAML, an ingress UI that names misconfiguration in words, and persistent state that cannot be broken by a timezone. Why an add-on rather than YAML packages or blueprints: - Blueprints cannot create helpers, and the maintenance cycle is a state machine whose phase and completion date must survive restarts. - YAML packages need filesystem access, a configuration.yaml edit and a restart - none of which belong in a client install. - Add-ons authenticate with SUPERVISOR_TOKEN, so there is no long-lived token to generate, store or leak on someone else's machine. - Requires HA OS/Supervised. Container and Core installs cannot run add-ons at all, which is a market decision, not an oversight. The control law and the maintenance machine are pure functions with no Home Assistant imports, and both ship with runnable checks (22 and 22 assertions). Every assertion corresponds to a rule whose absence caused an observed failure on hardware - the saturation duration term, the clamp-before-slew ordering, the deadband, the sign convention. One behaviour deliberately differs from the implementation it replaces: when its inputs go missing this commands 0 W rather than replaying the last setpoint. The reference version kept replaying, which the hardware watchdog cannot catch - from the ESP32's side, Home Assistant is still talking to it. Includes the ESPHome firmware (now parameterised: node name, inverter rating, watchdog timeout) and the optional RS485 e-stop. FIELD-GUIDE.md carries the commissioning gates, all judged on the wire rather than on how Home Assistant looks, plus the written statement a site without an e-stop needs signed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NckgXecasQb2eSsPYNSW6	2026-08-23 01:15:25 +02:00