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 twoopts.get("maintenance_charge_w", 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. Unreachable on a Supervisor install, which always supplies every key from config.yaml; fully reachable from sim/inject.py and from the test suite, which is where anyone debugging maintenance actually is.
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. It and validate.py's ⚠️ block each said to delete the other when the divergence was closed.
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 — the state that produced the bug. Both are closed, and so is the whole family: all eight maintenance_* keys now go through one mechanism.
What I chose, and why
The single source is config.yaml's options: block — the file the Supervisor reads and the operator edits, so it wins by definition. There is now exactly one in-code copy, maintenance.MAINT_OPTION_DEFAULTS.
Why a mirror rather than a runtime read of config.yaml — this is the judgement call in the PR and it is mechanical, not aesthetic: the Dockerfile copies app/ and run.sh into the image and notconfig.yaml. The running add-on cannot open the file it is configured by. A runtime read would need a Dockerfile change plus a YAML-subset parser in production code that can fail at import — a new way for the add-on not to start at 03:00 on a client's Pi. 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.
Why this pattern and not a new one — the repo has already answered this question twice (price.PRICE_OPTION_DEFAULTS, peakguard.PEAK_OPTION_DEFAULTS), and validate.py's rule 8 comment already calls it "CFG-03 read as a rule rather than as a complaint". This makes that rule hold where it was first broken, rather than inventing a third mechanism for one option family.
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. It uses key in opts rather than truthiness, so a shipped false or 0 is honoured instead of 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, main.py:696) reads self.maint.cfg.charge_w instead of re-opening the options dict. The Controller already parsed this in __init__; 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.
The fourth site: where the RANGE lives
config.yaml:390 carries maintenance_charge_w: int(500,5000) — a bound, not a default, so not this bug; but leaving the question open is how a fifth site arrives. The decision recorded in config.yaml and in maintenance.py: bounds live in the schema: block only, because the Supervisor is what enforces them and reads only that file, so a Python copy would be a place to disagree and nothing else.
test_maintenance.py checks both halves: 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 (a default the schema rejects is a config that cannot be re-entered once edited). What the schema grammar cannot express is a relation between keys — maintenance_charge_w against max_w — which is precisely why rule 6 is written in Python and why it does not belong in the mirror either.
The tripwire is deleted, and what stands in its place
test_validate.py's new section exercises the same four consumers the defect ran through, the same way, now agreeing: 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.yamlin 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 the dataclass defaults back out of dataclasses.fields().
Verified by breaking it
Each new check was verified by breaking the thing it watches and observing red:
break
result
config.yaml → 1500, mirror left at 2000
test_maintenance1 FAILED, test_validate4 FAILED
config.yamland the mirror → 1500
green again, and every consumer moved together: MaintConfig() 1500 W, the Controller 1500 W, the arbiter -1500 W, limited by maintenance(cheap-window charge) where it had said -2000 W, limited by supervised(max_w) — that is AC4
charge_w: float = 2500.0 retyped into the dataclass
2 FAILED in each of the two files
opts.get("maintenance_charge_w", 2500) restored in main.py
1 FAILED, naming the file and the literal it found
maintenance_hold_min deleted from config.yaml's options block
3 FAILED
Full suite 17/17 in goodwe-addon-ci, run twice, once with the unmodified origin/release/1.0 suite in parallel on the same host.
What I am unsure about, and one thing to look hardest at
The mirror is still, strictly, a second textual copy. AC1 says "a default exists in exactly one place". With config.yaml outside the image I could not do better without shipping a parser; what I did instead is make the copy unable to drift silently. If the reviewer wants the literal reading of AC1, the change is COPY config.yaml ./config.yaml plus a parser in app/, and it is a bigger risk surface than this card is worth — but it is a real disagreement, not a detail, so I am naming it rather than burying it.
MaintConfig's defaults bind at import, so a runtime mutation of MAINT_OPTION_DEFAULTS does not propagate to MaintConfig(). That is why the AC4 break-it demonstration is done at the source level (edit the file, re-run) rather than by monkeypatching the dict in a test. I think that is correct for a frozen config object, but it is worth a second opinion.
One flake seen, and I could not reproduce it. On the first full-suite run test_peakguard.py reported 2 FAILED — both the "the guard genuinely engaged" checks that follow wired_over_limit(), which builds its fixture against the real wall clock and retries across quarter boundaries. It has not recurred in five subsequent runs of that file (including under parallel load and under libfaketime at several offsets around a quarter boundary), and the pristine origin/release/1.0 tree passes and fails the same way. Nothing in this diff can reach those fixtures — maintenance is IDLE in one and the shaping is suspended in the other, and self.cheap is False so the line I changed at main.py:696 is never evaluated — but I would rather record it than let the next person meet it cold.
Behaviour change worth naming:MaintConfig() and MaintConfig(enabled=...) now answer 2000 W for both rates where they answered 2500 W. That is the point of the card — those objects were the wrong answer — but test_maintenance.py's and test_price_forecast.py's fixtures construct them, so it is a real change in what those suites exercise. Both still pass; test_maintenance.py's explicit MaintConfig(..., discharge_w=2500, ...) fixture is left explicit on purpose.
Concurrency with TEST-10: it pins shipped config.yaml values in test_freeze.py and has been told not to pin any maintenance_* key. I added comment blocks (no value changes beyond none at all) to config.yaml's maintenance options and schema sections. Whichever of us merges second merges release/1.0 and re-runs.
## The defect
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("maintenance_charge_w", 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**. Unreachable on a Supervisor install, which always supplies every key from `config.yaml`; fully reachable from `sim/inject.py` and from the test suite, which is where anyone debugging maintenance actually is.
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.** It and `validate.py`'s ⚠️ block each said to delete the other when the divergence was closed.
**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 — the state that produced the bug. Both are closed, and so is the whole family: all eight `maintenance_*` keys now go through one mechanism.
## What I chose, and why
**The single source is `config.yaml`'s `options:` block** — the file the Supervisor reads and the operator edits, so it wins by definition. There is now exactly one in-code copy, `maintenance.MAINT_OPTION_DEFAULTS`.
**Why a mirror rather than a runtime read of `config.yaml`** — this is the judgement call in the PR and it is mechanical, not aesthetic: 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. A runtime read would need a Dockerfile change plus a YAML-subset parser in production code that can fail at import — a new way for the add-on not to start at 03:00 on a client's Pi. 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.
**Why this pattern and not a new one** — the repo has already answered this question twice (`price.PRICE_OPTION_DEFAULTS`, `peakguard.PEAK_OPTION_DEFAULTS`), and `validate.py`'s rule 8 comment already calls it *"CFG-03 read as a rule rather than as a complaint"*. This makes that rule hold where it was first broken, rather than inventing a third mechanism for one option family.
**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. It uses `key in opts` rather than truthiness, so a shipped `false` or `0` is honoured instead of 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, `main.py:696`) reads `self.maint.cfg.charge_w` instead of re-opening the options dict. The Controller already parsed this in `__init__`; 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.
## The fourth site: where the RANGE lives
`config.yaml:390` carries `maintenance_charge_w: int(500,5000)` — a **bound**, not a default, so not this bug; but leaving the question open is how a fifth site arrives. The decision recorded in `config.yaml` and in `maintenance.py`: **bounds live in the `schema:` block only**, because the Supervisor is what enforces them and reads only that file, so a Python copy would be a place to disagree and nothing else.
`test_maintenance.py` checks both halves: 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 (a default the schema rejects is a config that cannot be re-entered once edited). What the schema grammar cannot express is a relation *between* keys — `maintenance_charge_w` against `max_w` — which is precisely why rule 6 is written in Python and why it does not belong in the mirror either.
## The tripwire is deleted, and what stands in its place
`test_validate.py`'s new section exercises the same four consumers the defect ran through, the same way, now agreeing: 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 the dataclass defaults back out of `dataclasses.fields()`.
## Verified by breaking it
Each new check was verified by breaking the thing it watches and observing red:
| break | result |
|---|---|
| `config.yaml` → 1500, mirror left at 2000 | `test_maintenance` **1 FAILED**, `test_validate` **4 FAILED** |
| `config.yaml` **and** the mirror → 1500 | green again, **and every consumer moved together**: `MaintConfig()` 1500 W, the Controller 1500 W, the arbiter `-1500 W, limited by maintenance(cheap-window charge)` where it had said `-2000 W, limited by supervised(max_w)` — that is AC4 |
| `charge_w: float = 2500.0` retyped into the dataclass | **2 FAILED** in each of the two files |
| `opts.get("maintenance_charge_w", 2500)` restored in `main.py` | **1 FAILED**, naming the file and the literal it found |
| `maintenance_hold_min` deleted from `config.yaml`'s options block | **3 FAILED** |
Full suite **17/17** in `goodwe-addon-ci`, run twice, once with the unmodified `origin/release/1.0` suite in parallel on the same host.
## What I am unsure about, and one thing to look hardest at
1. **The mirror is still, strictly, a second textual copy.** AC1 says "a default exists in exactly one place". With `config.yaml` outside the image I could not do better without shipping a parser; what I did instead is make the copy unable to drift silently. If the reviewer wants the literal reading of AC1, the change is `COPY config.yaml ./config.yaml` plus a parser in `app/`, and it is a bigger risk surface than this card is worth — but it is a real disagreement, not a detail, so I am naming it rather than burying it.
2. **`MaintConfig`'s defaults bind at import, so a *runtime* mutation of `MAINT_OPTION_DEFAULTS` does not propagate to `MaintConfig()`.** That is why the AC4 break-it demonstration is done at the source level (edit the file, re-run) rather than by monkeypatching the dict in a test. I think that is correct for a frozen config object, but it is worth a second opinion.
3. **One flake seen, and I could not reproduce it.** On the first full-suite run `test_peakguard.py` reported 2 FAILED — both the "the guard genuinely engaged" checks that follow `wired_over_limit()`, which builds its fixture against the real wall clock and retries across quarter boundaries. It has not recurred in five subsequent runs of that file (including under parallel load and under `libfaketime` at several offsets around a quarter boundary), and the pristine `origin/release/1.0` tree passes and fails the same way. Nothing in this diff can reach those fixtures — maintenance is `IDLE` in one and the shaping is suspended in the other, and `self.cheap` is False so the line I changed at `main.py:696` is never evaluated — but I would rather record it than let the next person meet it cold.
4. **Behaviour change worth naming:** `MaintConfig()` and `MaintConfig(enabled=...)` now answer **2000 W** for both rates where they answered 2500 W. That is the point of the card — those objects were the wrong answer — but `test_maintenance.py`'s and `test_price_forecast.py`'s fixtures construct them, so it is a real change in what those suites exercise. Both still pass; `test_maintenance.py`'s explicit `MaintConfig(..., discharge_w=2500, ...)` fixture is left explicit on purpose.
5. **Concurrency with TEST-10:** it pins shipped `config.yaml` values in `test_freeze.py` and has been told not to pin any `maintenance_*` key. I added comment blocks (no value changes beyond none at all) to `config.yaml`'s maintenance options and schema sections. Whichever of us merges second merges `release/1.0` and re-runs.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
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>
admin
merged commit f630ed949b into release/1.02026-09-01 06:28:57 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The defect
Option defaults for the maintenance cycle were declared in three places that could disagree, and MAINT-04 proved they already did.
config.yamlshippedmaintenance_charge_w: 2000;main.pycarried twoopts.get("maintenance_charge_w", 2500)fallbacks;MaintConfig's dataclass carried a third2500.0.So an options dict with the key absent was ACCEPTED by
validate.py's rule 6 — which read 2000 — and then built bymain.pyat 2500, against a 2000 W ceiling:That is the bug rule 6 exists to refuse, passing rule 6. Unreachable on a Supervisor install, which always supplies every key from
config.yaml; fully reachable fromsim/inject.pyand from the test suite, which is where anyone debugging maintenance actually is.MAINT-04 shipped a tripwire and said so:
test_validate.pypinned 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. It andvalidate.py's ⚠️ block each said to delete the other when the divergence was closed.The divergence was wider than one key.
maintenance_discharge_whad the identical three-way split (config.yaml2000,MaintConfig2500) 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 — the state that produced the bug. Both are closed, and so is the whole family: all eightmaintenance_*keys now go through one mechanism.What I chose, and why
The single source is
config.yaml'soptions:block — the file the Supervisor reads and the operator edits, so it wins by definition. There is now exactly one in-code copy,maintenance.MAINT_OPTION_DEFAULTS.Why a mirror rather than a runtime read of
config.yaml— this is the judgement call in the PR and it is mechanical, not aesthetic: theDockerfilecopiesapp/andrun.shinto the image and notconfig.yaml. The running add-on cannot open the file it is configured by. A runtime read would need a Dockerfile change plus a YAML-subset parser in production code that can fail at import — a new way for the add-on not to start at 03:00 on a client's Pi. What makes the copy safe is thattest_maintenance.pypins every entry by reading the shipped file: drift is a CI failure, not a comment.Why this pattern and not a new one — the repo has already answered this question twice (
price.PRICE_OPTION_DEFAULTS,peakguard.PEAK_OPTION_DEFAULTS), andvalidate.py's rule 8 comment already calls it "CFG-03 read as a rule rather than as a complaint". This makes that rule hold where it was first broken, rather than inventing a third mechanism for one option family.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 amaintenance_*option.main.py's eight inlineopts.get(key, <literal>)calls are gone. It useskey in optsrather than truthiness, so a shippedfalseor0is honoured instead of 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 theP_MAINT_SHAPEclaim,main.py:696) readsself.maint.cfg.charge_winstead of re-opening the options dict. The Controller already parsed this in__init__; 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 itsmaintenance_discharge_wwarning take their fallbacks from the mirror.The fourth site: where the RANGE lives
config.yaml:390carriesmaintenance_charge_w: int(500,5000)— a bound, not a default, so not this bug; but leaving the question open is how a fifth site arrives. The decision recorded inconfig.yamland inmaintenance.py: bounds live in theschema:block only, because the Supervisor is what enforces them and reads only that file, so a Python copy would be a place to disagree and nothing else.test_maintenance.pychecks both halves: 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 (a default the schema rejects is a config that cannot be re-entered once edited). What the schema grammar cannot express is a relation between keys —maintenance_charge_wagainstmax_w— which is precisely why rule 6 is written in Python and why it does not belong in the mirror either.The tripwire is deleted, and what stands in its place
test_validate.py's new section exercises the same four consumers the defect ran through, the same way, now agreeing: the validator accepts a dict with every maintenance key missing; a realControllerbuilt from that same dict holds 2000 W (throughmain.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 of0 W, contradiction=True.test_maintenance.pypins the mirror toconfig.yamlin both directions — equal values, and full coverage of everymaintenance_*key the file ships, so a ninth key cannot arrive unmirrored — and asserts the consumers move as a relation rather than against numbers, reading the dataclass defaults back out ofdataclasses.fields().Verified by breaking it
Each new check was verified by breaking the thing it watches and observing red:
config.yaml→ 1500, mirror left at 2000test_maintenance1 FAILED,test_validate4 FAILEDconfig.yamland the mirror → 1500MaintConfig()1500 W, the Controller 1500 W, the arbiter-1500 W, limited by maintenance(cheap-window charge)where it had said-2000 W, limited by supervised(max_w)— that is AC4charge_w: float = 2500.0retyped into the dataclassopts.get("maintenance_charge_w", 2500)restored inmain.pymaintenance_hold_mindeleted fromconfig.yaml's options blockFull suite 17/17 in
goodwe-addon-ci, run twice, once with the unmodifiedorigin/release/1.0suite in parallel on the same host.What I am unsure about, and one thing to look hardest at
The mirror is still, strictly, a second textual copy. AC1 says "a default exists in exactly one place". With
config.yamloutside the image I could not do better without shipping a parser; what I did instead is make the copy unable to drift silently. If the reviewer wants the literal reading of AC1, the change isCOPY config.yaml ./config.yamlplus a parser inapp/, and it is a bigger risk surface than this card is worth — but it is a real disagreement, not a detail, so I am naming it rather than burying it.MaintConfig's defaults bind at import, so a runtime mutation ofMAINT_OPTION_DEFAULTSdoes not propagate toMaintConfig(). That is why the AC4 break-it demonstration is done at the source level (edit the file, re-run) rather than by monkeypatching the dict in a test. I think that is correct for a frozen config object, but it is worth a second opinion.One flake seen, and I could not reproduce it. On the first full-suite run
test_peakguard.pyreported 2 FAILED — both the "the guard genuinely engaged" checks that followwired_over_limit(), which builds its fixture against the real wall clock and retries across quarter boundaries. It has not recurred in five subsequent runs of that file (including under parallel load and underlibfaketimeat several offsets around a quarter boundary), and the pristineorigin/release/1.0tree passes and fails the same way. Nothing in this diff can reach those fixtures — maintenance isIDLEin one and the shaping is suspended in the other, andself.cheapis False so the line I changed atmain.py:696is never evaluated — but I would rather record it than let the next person meet it cold.Behaviour change worth naming:
MaintConfig()andMaintConfig(enabled=...)now answer 2000 W for both rates where they answered 2500 W. That is the point of the card — those objects were the wrong answer — buttest_maintenance.py's andtest_price_forecast.py's fixtures construct them, so it is a real change in what those suites exercise. Both still pass;test_maintenance.py's explicitMaintConfig(..., discharge_w=2500, ...)fixture is left explicit on purpose.Concurrency with TEST-10: it pins shipped
config.yamlvalues intest_freeze.pyand has been told not to pin anymaintenance_*key. I added comment blocks (no value changes beyond none at all) toconfig.yaml's maintenance options and schema sections. Whichever of us merges second mergesrelease/1.0and re-runs.🤖 Generated with Claude Code
https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
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>