config.yaml shipped max_w: 2000 against maintenance_charge_w: 2500. The cheap-window shaping files Claim.limit(hi = -maintenance_charge_w) at P_MAINT_SHAPE against the supervised SAFETY limit's lo = -max_w. Both bind (both outrank the loop that wins the SET), so lo > hi — the arbiter's clause 3, correctly resolved to 0 W and logged as a CONTRADICTION.
Maintenance silently never charged in a cheap window on a default install, and the only evidence read like a bug in the arbiter rather than like the two numbers the operator set. The live reference install runs max_w: 5000 and is unaffected; this bites a default install, which is every new one.
Measured against the real arbiter, loop asking +500 W:
max_w=2000 hi=-2500 -> +0 W contradiction=True
max_w=2500 hi=-2500 -> -2500 W limited by supervised(max_w)
max_w=3000 hi=-2500 -> -2500 W limited by maintenance(cheap-window charge)
The change
validate_startup_config() gains rule 6: refuse maintenance_charge_w > max_w. Identical shape to CFG-01's sign_disagree_w >= max_w — a number the operator set, overridden by a layer they cannot see. Strictly >: equality is reachable (-max_w is a single valid point) and refusing it would be exactly the trivia that gets a validator deleted.
Not scoped to maintenance_enabled. The pair is contradictory either way, and scoping would leave the trap armed for the one operator action — ticking the box — that cannot be expected to change what a charge rate means.
maintenance_discharge_w — checked, not assumed, and it is NOT the same case
The shaping is not symmetric. The drain phase does not shape the loop, it OWNS the actuator: Claim.set(P_MAINTENANCE, +discharge_w). A SET is clamped by the limits above it, not contradicted by them.
max_w=2000 discharge_w=2500 -> +2000 W contradiction=False maintenance -> 2000 W, limited by supervised(max_w)
So it emits +2000 W, and the arbiter's own explain() prints the clamp on the status page every cycle. That is neither (a) — nothing is silent — nor (b) — it moves energy and the cycle still completes, just slower (13.8 kWh usable at 2000 W is ~6.9 h against the 720 min DRAIN_TIMEOUT_MIN). Warned about, not refused.
AC3 — the defaults
Shipping a default the validator refuses would be worse than the bug, so maintenance_charge_w and maintenance_discharge_w drop to 2000, at the shipped max_w.
max_w was deliberately NOT raised instead: it is the supervised safety ceiling, and raising a default upward ships more power to every new install. Raising max_w is what unlocks a faster maintenance cycle — raise it first, then these. That is written into config.yaml next to the values.
test_validate.py now reads config.yaml itself (regex, not PyYAML — the add-on image has no PyYAML, and a check that skips on ImportError is not a check) and asserts the shipped numbers pass the rule. A default the validator refuses cannot ship again.
Verification
AC1, by starting it rather than reading it. Real entrypoint (app.main.main()), options.json carrying the pre-fix pair:
WARNING goodwe.config: maintenance_discharge_w 2500 W is above max_w 2000 W. Not refused - ...
ERROR goodwe.config: REFUSING TO START: the add-on configuration is not usable.
ERROR goodwe.config: This is a CONFIGURATION problem, not a crash. Nothing is broken;
ERROR goodwe.config: 1. maintenance_charge_w is 2500 W but max_w is 2000 W, so a maintenance
charge in a cheap window can never happen: ... THE CEILING WINS ...
### EXIT CODE = 78
Both behaviours in one run: charge refuses, discharge warns and does not. With both at 2000 the same boot passes the gate and reaches ingress UI listening on :8099.
The check goes red when broken.python3 test_validate.py, four separate breaks:
break
result
delete rule 6
5 FAILED
> → >= (refuse the reachable equality)
25 FAILED
revert config.yaml to maintenance_charge_w: 2500
2 FAILED — the AC3 guard
turn the discharge warning into a refusal
2 FAILED
Restored: exit 0. Whole suite green (test_arbiter, test_control, test_loop, test_maintenance, test_p1, test_validate, test_watchdog).
What to attack hardest
main.py and maintenance.py still fall back to 2500 when the key is absent, while config.yaml and validate.py now say 2000. Left deliberately: those files are SAFETY-18's right now, and the case is unreachable on a Supervisor install (every key is always supplied from config.yaml). But it means a dict with the key MISSING is accepted at 2000 and then built by main.py at 2500 — the exact contradiction this rule exists to catch, in the one case it cannot see. The real fix is one source of truth for option defaults. I picked 2000 over 2500 for the fallback because 2500 refused every minimal options dict in test_loop.py / test_watchdog.py; that tradeoff is the judgement call most worth disagreeing with.
Should the discharge clamp be refused after all? I argue no — it is loud and it still completes. But it does mean an operator's configured drain rate silently is not the rate, and the strict CFG-01 reading of (a) could be stretched to cover it. The reasoning is written out in validate.py's docstring so it can be argued with rather than rediscovered.
test_watchdog.py needed one case amended (max_w: 500 is also below the 2000 W maintenance charge, so it was refused twice). I named maintenance_charge_w there to keep that case about the sign rule. Check I did not weaken it.
Nothing was run against the live inverter. No energy was moved, no add-on restarted. The AC1 boot ran against no Home Assistant — the gate is reached before any of that matters, but the hass.limits() call returned (None, None), so the device-range branch of rule 2 was not exercised in that run (test_validate.py covers it).
Worked in an isolated git worktree; staged by explicit path. No files outside goodwe_controller/{config.yaml, DOCS.md, CHANGELOG.md, app/validate.py, test_validate.py, test_watchdog.py} were touched.
## The bug
`config.yaml` shipped `max_w: 2000` against `maintenance_charge_w: 2500`. The cheap-window shaping files `Claim.limit(hi = -maintenance_charge_w)` at `P_MAINT_SHAPE` against the supervised SAFETY limit's `lo = -max_w`. Both bind (both outrank the loop that wins the SET), so `lo > hi` — the arbiter's clause 3, correctly resolved to **0 W** and logged as a CONTRADICTION.
**Maintenance silently never charged in a cheap window on a default install**, and the only evidence read like a bug in the arbiter rather than like the two numbers the operator set. The live reference install runs `max_w: 5000` and is unaffected; this bites a default install, which is every new one.
Measured against the real arbiter, loop asking +500 W:
```
max_w=2000 hi=-2500 -> +0 W contradiction=True
max_w=2500 hi=-2500 -> -2500 W limited by supervised(max_w)
max_w=3000 hi=-2500 -> -2500 W limited by maintenance(cheap-window charge)
```
## The change
`validate_startup_config()` gains rule 6: refuse `maintenance_charge_w > max_w`. Identical shape to CFG-01's `sign_disagree_w >= max_w` — a number the operator set, overridden by a layer they cannot see. Strictly `>`: equality is reachable (`-max_w` is a single valid point) and refusing it would be exactly the trivia that gets a validator deleted.
Not scoped to `maintenance_enabled`. The pair is contradictory either way, and scoping would leave the trap armed for the one operator action — ticking the box — that cannot be expected to change what a charge rate means.
## `maintenance_discharge_w` — checked, not assumed, and it is NOT the same case
**The shaping is not symmetric.** The drain phase does not shape the loop, it OWNS the actuator: `Claim.set(P_MAINTENANCE, +discharge_w)`. A SET is *clamped* by the limits above it, not contradicted by them.
```
max_w=2000 discharge_w=2500 -> +2000 W contradiction=False maintenance -> 2000 W, limited by supervised(max_w)
```
So it emits +2000 W, and the arbiter's own `explain()` prints the clamp on the status page every cycle. That is neither (a) — nothing is silent — nor (b) — it moves energy and the cycle still completes, just slower (13.8 kWh usable at 2000 W is ~6.9 h against the 720 min `DRAIN_TIMEOUT_MIN`). **Warned about, not refused.**
## AC3 — the defaults
Shipping a default the validator refuses would be worse than the bug, so `maintenance_charge_w` and `maintenance_discharge_w` drop to **2000**, at the shipped `max_w`.
`max_w` was deliberately NOT raised instead: it is the supervised safety ceiling, and raising a default upward ships more power to every new install. Raising `max_w` is what unlocks a faster maintenance cycle — raise it first, then these. That is written into `config.yaml` next to the values.
`test_validate.py` now reads `config.yaml` itself (regex, not PyYAML — the add-on image has no PyYAML, and a check that skips on ImportError is not a check) and asserts the shipped numbers pass the rule. A default the validator refuses cannot ship again.
## Verification
**AC1, by starting it rather than reading it.** Real entrypoint (`app.main.main()`), options.json carrying the pre-fix pair:
```
WARNING goodwe.config: maintenance_discharge_w 2500 W is above max_w 2000 W. Not refused - ...
ERROR goodwe.config: REFUSING TO START: the add-on configuration is not usable.
ERROR goodwe.config: This is a CONFIGURATION problem, not a crash. Nothing is broken;
ERROR goodwe.config: 1. maintenance_charge_w is 2500 W but max_w is 2000 W, so a maintenance
charge in a cheap window can never happen: ... THE CEILING WINS ...
### EXIT CODE = 78
```
Both behaviours in one run: charge refuses, discharge warns and does not. With both at 2000 the same boot passes the gate and reaches `ingress UI listening on :8099`.
**The check goes red when broken.** `python3 test_validate.py`, four separate breaks:
| break | result |
|---|---|
| delete rule 6 | 5 FAILED |
| `>` → `>=` (refuse the reachable equality) | 25 FAILED |
| revert `config.yaml` to `maintenance_charge_w: 2500` | 2 FAILED — the AC3 guard |
| turn the discharge warning into a refusal | 2 FAILED |
Restored: exit 0. Whole suite green (`test_arbiter`, `test_control`, `test_loop`, `test_maintenance`, `test_p1`, `test_validate`, `test_watchdog`).
## What to attack hardest
1. **`main.py` and `maintenance.py` still fall back to 2500** when the key is absent, while `config.yaml` and `validate.py` now say 2000. Left deliberately: those files are SAFETY-18's right now, and the case is unreachable on a Supervisor install (every key is always supplied from `config.yaml`). But it means a dict with the key MISSING is accepted at 2000 and then built by `main.py` at 2500 — the exact contradiction this rule exists to catch, in the one case it cannot see. **The real fix is one source of truth for option defaults.** I picked 2000 over 2500 for the fallback because 2500 refused every minimal options dict in `test_loop.py` / `test_watchdog.py`; that tradeoff is the judgement call most worth disagreeing with.
2. **Should the discharge clamp be refused after all?** I argue no — it is loud and it still completes. But it does mean an operator's configured drain rate silently is not the rate, and the strict CFG-01 reading of (a) could be stretched to cover it. The reasoning is written out in `validate.py`'s docstring so it can be argued with rather than rediscovered.
3. **`test_watchdog.py` needed one case amended** (`max_w: 500` is also below the 2000 W maintenance charge, so it was refused twice). I named `maintenance_charge_w` there to keep that case about the sign rule. Check I did not weaken it.
4. **Nothing was run against the live inverter.** No energy was moved, no add-on restarted. The AC1 boot ran against no Home Assistant — the gate is reached before any of that matters, but the `hass.limits()` call returned `(None, None)`, so the device-range branch of rule 2 was not exercised in that run (`test_validate.py` covers it).
Worked in an isolated git worktree; staged by explicit path. No files outside `goodwe_controller/{config.yaml, DOCS.md, CHANGELOG.md, app/validate.py, test_validate.py, test_watchdog.py}` were touched.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
config.yaml shipped max_w 2000 against maintenance_charge_w 2500. The
cheap-window shaping files Claim.limit(hi = -maintenance_charge_w) against
the supervised SAFETY limit's lo = -max_w, so lo > hi - the arbiter's
clause 3, correctly resolved to 0 W and logged as a CONTRADICTION.
Maintenance silently never charged in a cheap window on a default install,
and the only evidence read like a bug in the arbiter rather than like the
two numbers the operator set.
validate_startup_config() now refuses maintenance_charge_w > max_w, the
same shape as CFG-01's sign_disagree_w >= max_w. Equality is reachable
(-max_w is a single valid point) and is not refused. Demonstrated by
starting the add-on on the pre-fix pair: exit 78 and the refusal banner.
maintenance_discharge_w was checked rather than assumed and is NOT the same
case: the drain phase OWNS the actuator (Claim.set at P_MAINTENANCE), so
the ceiling clamps it to +2000 W and the arbiter's explain() names the
clamp every cycle. Loud, and it still moves energy - warned, not refused.
The shipped maintenance rates drop to 2000 so the defaults pass the rule;
max_w was deliberately not raised, because it is the supervised safety
ceiling. test_validate.py now reads config.yaml itself, so a default the
validator refuses cannot ship again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
Not merging — one thing, and it is the one you flagged. Everything else I checked held up, including every claim I tried hardest to break. The gap is that the cross-file default divergence is guarded by a comment, and this repo's most-repeated defect is a comment where a check belonged. I am not asking you to touch main.py — SAFETY-18 is live in it.
Required: make the divergence a tripwire, not a comment
validate.py's comment says "if config.yaml's value moves, move this and main.py's together". Nothing enforces that. Meanwhile the same PR proves it knows how to enforce exactly this kind of thing — the _shipped regex block. Apply the same technique to the divergence itself. Prototype, run against your branch:
cfg=open("config.yaml",encoding="utf-8").read()shipped=float(re.search(r"^ maintenance_charge_w: (-?\d+(?:\.\d+)?)\s*$",cfg,re.M).group(1))src=open("app/main.py",encoding="utf-8").read()falls={float(m)forminre.findall(r'\.get\("maintenance_charge_w",\s*(-?\d+(?:\.\d+)?)\)',src)}mnt=open("app/maintenance.py",encoding="utf-8").read()dc={float(m)forminre.findall(r'^\s*charge_w:\s*float\s*=\s*(-?\d+(?:\.\d+)?)',mnt,re.M)}check("the KNOWN default divergence is still exactly as recorded (MAINT-04)",shipped==2000.0andfalls=={2500.0}anddc=={2500.0})
config.yaml moves to 1800 (the scenario your comment warns about)
FAIL
someone fixes main.py to 2000 (divergence closed)
FAIL — and the fix is then to delete this check
It records the mismatch instead of hiding it, needs no dependency, touches no file another ticket owns, and fires the moment either number moves. Add it and I merge.
Why I am asking rather than waving it through: the hole is real, not theoretical. Through the actual code:
options dict with maintenance_charge_w ABSENT, max_w=2000
validate_startup_config -> ACCEPTED, no problems
main.py then builds charge_w = 2500.0 (MaintConfig().charge_w = 2500.0)
arbiter -> 0.0 W contradiction=True "CONTRADICTION between limits - commanding 0 W (loop)"
That is precisely the bug rule 6 exists to catch, passing rule 6. You are right that a Supervisor install always supplies every key, so production is safe today — I accept that and I am not asking you to fix it now. But sim/inject.py and the tests are exactly where someone debugging maintenance works, and there the validator says fine while the built config contradicts.
Your reasoning on the fallback value: checked, and it holds
You said a 2500 fallback refused every minimal options dict. I reproduced it — validate.py fallback 2000 → 2500:
So 2000 is the right choice, for the right reason: it matches config.yaml, which is the source of truth. Good call, and matching the tests is a consequence rather than the motive.
The four mutations, re-run by me
mutation
test_validate
test_watchdog
delete rule 6
5 FAILED✅ matches
green
> → >=
25 FAILED✅ matches
4 FAILED
revert config.yaml to 2500
3 FAILED (you said 2)
green
discharge warning → refusal
2 FAILED✅ matches
1 FAILED
Only discrepancy: reverting config.yaml gave me 3, not 2 — I reverted both rates, which also trips "...and the shipped discharge rate too". Red either way. Baseline restored green after each; full suite green on the untouched head (all 7 files).
Three more of my own:
Is the amended test_watchdog case weaker? No — verified. I deleted rule 5 entirely; a threshold above max_w is refusedFAILS. The amended case still bites on the rule it is about. Naming maintenance_charge_w: 500 only removes the second refusal: un-amended the dict returns both problems and len(p) == 1 would be measuring the interaction. The amendment preserves the assertion's intent. This is the one I most expected to find rotten and it is clean.
Regex format-sensitivity.M7 — max_w: 2000 → 1500 in config.yaml alone: 3 FAILED. M10 — append a trailing comment to the shipped maintenance_charge_w line so the \s*$ anchor misses: FAIL "config.yaml really was read (all four defaults found)". The four-key set assertion is what makes a format change loud instead of silent. Regex over PyYAML is the right call here and it is built correctly.
The asymmetry, through the real arbiter, not your table. Every row reproduces, and the reason is structural rather than incidental — resolve() skips limits below the winning SET's priority (arbiter.py:96):
CHARGE (limit hi=-charge_w @P_MAINT_SHAPE, loop SET wins @P_LOOP -> both limits bind)
max_w=2000 charge_w=2500 -> +0.0 W contradiction=True
max_w=2000 charge_w=2000 -> -2000.0 W loop -> -2000 W, limited by supervised(max_w)
max_w=3000 charge_w=2500 -> -2500.0 W loop -> -2500 W, limited by maintenance(cheap-window charge)
DRAIN (SET @P_MAINTENANCE=60 outranks P_MAINT_SHAPE=40 -> only SAFETY binds)
max_w=2000 disch_w=2500 -> +2000.0 W maintenance -> 2000 W, limited by supervised(max_w)
max_w=2000 disch_w=5000 -> +2000.0 W maintenance -> 2000 W, limited by supervised(max_w)
A SET at P_MAINTENANCE can only ever be clamped, never contradicted. Warn-don't-refuse is correct and the equality case in rule 6 is genuinely reachable, so strictly > is right.
AC3 margin (item 4): fine, and conservative.DRAIN_TIMEOUT_MIN = 720; 13.8 kWh at 2000 W = 6.9 h, a 1.7x margin. It is if anything better than that — house load drains the pack alongside the export, and maintenance_soc_floor: 11 means less than the full pack is drained. Lowering the rates rather than raising max_w is the right instinct: max_w is the supervised ceiling and a default upward ships more power to every new install.
Two things to fold in while you are there
The upgrade path is unnamed. An existing install carrying the old maintenance_charge_w: 2500 against max_w: 2000 now meets rule 6 and exits 78 on upgrade — correct behaviour, loud, with the fix in the message, but an add-on that will not start after an update deserves a CHANGELOG line telling the operator what to change. I could not determine whether Supervisor persisted the old default into stored options or re-derives it from config.yaml — /api/hassio/ is not reachable with a long-lived token, so I could not read the live install's stored options. The live reference runs max_w: 5000 and is unaffected either way. If Supervisor re-derives, this is a non-event; if it persists, every existing default install stops on upgrade. Worth one line that covers both.
Nit:maintenance.py:31 says "a 13.5 kWh usable pack"; the PR, CHANGELOG and validator say 13.8 kWh. Same conclusion, two numbers.
Add the tripwire and I will merge.
**Not merging — one thing, and it is the one you flagged.** Everything else I checked held up, including every claim I tried hardest to break. The gap is that the cross-file default divergence is guarded by a **comment**, and this repo's most-repeated defect is a comment where a check belonged. I am **not** asking you to touch `main.py` — SAFETY-18 is live in it.
## Required: make the divergence a tripwire, not a comment
`validate.py`'s comment says "if config.yaml's value moves, move this and main.py's together". Nothing enforces that. Meanwhile the same PR proves it knows how to enforce exactly this kind of thing — the `_shipped` regex block. Apply the same technique to the divergence itself. Prototype, run against your branch:
```python
cfg = open("config.yaml", encoding="utf-8").read()
shipped = float(re.search(r"^ maintenance_charge_w: (-?\d+(?:\.\d+)?)\s*$", cfg, re.M).group(1))
src = open("app/main.py", encoding="utf-8").read()
falls = {float(m) for m in re.findall(r'\.get\("maintenance_charge_w",\s*(-?\d+(?:\.\d+)?)\)', src)}
mnt = open("app/maintenance.py", encoding="utf-8").read()
dc = {float(m) for m in re.findall(r'^\s*charge_w:\s*float\s*=\s*(-?\d+(?:\.\d+)?)', mnt, re.M)}
check("the KNOWN default divergence is still exactly as recorded (MAINT-04)",
shipped == 2000.0 and falls == {2500.0} and dc == {2500.0})
```
I ran it three ways:
| state | result |
|---|---|
| your branch as it stands | **PASS** — `config.yaml=2000.0 main.py={2500.0} MaintConfig={2500.0}` |
| `config.yaml` moves to 1800 (the scenario your comment warns about) | **FAIL** |
| someone fixes `main.py` to 2000 (divergence closed) | **FAIL** — and the fix is then to delete this check |
It records the mismatch instead of hiding it, needs no dependency, touches no file another ticket owns, and fires the moment either number moves. Add it and I merge.
**Why I am asking rather than waving it through:** the hole is real, not theoretical. Through the actual code:
```
options dict with maintenance_charge_w ABSENT, max_w=2000
validate_startup_config -> ACCEPTED, no problems
main.py then builds charge_w = 2500.0 (MaintConfig().charge_w = 2500.0)
arbiter -> 0.0 W contradiction=True "CONTRADICTION between limits - commanding 0 W (loop)"
```
That is precisely the bug rule 6 exists to catch, passing rule 6. You are right that a Supervisor install always supplies every key, so production is safe today — I accept that and I am not asking you to fix it now. But `sim/inject.py` and the tests are exactly where someone debugging maintenance works, and there the validator says fine while the built config contradicts.
## Your reasoning on the fallback value: checked, and it holds
You said a 2500 fallback refused every minimal options dict. I reproduced it — `validate.py` fallback 2000 → 2500:
```
test_watchdog 3 FAILED test_loop 10 FAILED test_validate 0 FAILED
```
So 2000 is the right choice, for the right reason: it matches `config.yaml`, which is the source of truth. Good call, and matching the tests is a consequence rather than the motive.
## The four mutations, re-run by me
| mutation | test_validate | test_watchdog |
|---|---|---|
| delete rule 6 | **5 FAILED** ✅ matches | green |
| `>` → `>=` | **25 FAILED** ✅ matches | 4 FAILED |
| revert `config.yaml` to 2500 | **3 FAILED** (you said 2) | green |
| discharge warning → refusal | **2 FAILED** ✅ matches | 1 FAILED |
Only discrepancy: reverting `config.yaml` gave me 3, not 2 — I reverted *both* rates, which also trips "...and the shipped discharge rate too". Red either way. Baseline restored green after each; full suite green on the untouched head (all 7 files).
Three more of my own:
- **Is the amended `test_watchdog` case weaker? No — verified.** I deleted rule 5 entirely; `a threshold above max_w is refused` **FAILS**. The amended case still bites on the rule it is about. Naming `maintenance_charge_w: 500` only removes the second refusal: un-amended the dict returns both problems and `len(p) == 1` would be measuring the interaction. The amendment preserves the assertion's intent. This is the one I most expected to find rotten and it is clean.
- **Regex format-sensitivity.** `M7` — `max_w: 2000` → `1500` in `config.yaml` alone: **3 FAILED**. `M10` — append a trailing comment to the shipped `maintenance_charge_w` line so the `\s*$` anchor misses: **FAIL "config.yaml really was read (all four defaults found)"**. The four-key set assertion is what makes a format change loud instead of silent. Regex over PyYAML is the right call here and it is built correctly.
- **The asymmetry, through the real arbiter, not your table.** Every row reproduces, and the reason is structural rather than incidental — `resolve()` skips limits *below* the winning SET's priority (`arbiter.py:96`):
```
CHARGE (limit hi=-charge_w @P_MAINT_SHAPE, loop SET wins @P_LOOP -> both limits bind)
max_w=2000 charge_w=2500 -> +0.0 W contradiction=True
max_w=2000 charge_w=2000 -> -2000.0 W loop -> -2000 W, limited by supervised(max_w)
max_w=3000 charge_w=2500 -> -2500.0 W loop -> -2500 W, limited by maintenance(cheap-window charge)
DRAIN (SET @P_MAINTENANCE=60 outranks P_MAINT_SHAPE=40 -> only SAFETY binds)
max_w=2000 disch_w=2500 -> +2000.0 W maintenance -> 2000 W, limited by supervised(max_w)
max_w=2000 disch_w=5000 -> +2000.0 W maintenance -> 2000 W, limited by supervised(max_w)
```
A SET at P_MAINTENANCE can only ever be clamped, never contradicted. **Warn-don't-refuse is correct** and the equality case in rule 6 is genuinely reachable, so strictly `>` is right.
**AC3 margin (item 4): fine, and conservative.** `DRAIN_TIMEOUT_MIN = 720`; 13.8 kWh at 2000 W = 6.9 h, a 1.7x margin. It is if anything better than that — house load drains the pack alongside the export, and `maintenance_soc_floor: 11` means less than the full pack is drained. Lowering the rates rather than raising `max_w` is the right instinct: `max_w` is the supervised ceiling and a default upward ships more power to every new install.
## Two things to fold in while you are there
1. **The upgrade path is unnamed.** An existing install carrying the old `maintenance_charge_w: 2500` against `max_w: 2000` now meets rule 6 and **exits 78 on upgrade** — correct behaviour, loud, with the fix in the message, but an add-on that will not start after an update deserves a CHANGELOG line telling the operator what to change. **I could not determine whether Supervisor persisted the old default into stored options or re-derives it from `config.yaml`** — `/api/hassio/` is not reachable with a long-lived token, so I could not read the live install's stored options. The live reference runs `max_w: 5000` and is unaffected either way. If Supervisor re-derives, this is a non-event; if it persists, every existing default install stops on upgrade. Worth one line that covers both.
2. **Nit:** `maintenance.py:31` says "a 13.5 kWh usable pack"; the PR, CHANGELOG and validator say 13.8 kWh. Same conclusion, two numbers.
Add the tripwire and I will merge.
validate.py's comment said "if config.yaml's value moves, move this and
main.py's together" and nothing enforced it - a comment where a check
belonged, in a PR that elsewhere proves it knows the difference.
test_validate.py now reads main.py and maintenance.py directly and asserts
the divergence EXACTLY AS IT STANDS: config.yaml/validate.py at 2000,
main.py's two fallbacks and MaintConfig's dataclass default at 2500. It
records the mismatch rather than demanding it be closed, so it passes today
and goes red if either side moves - including when somebody closes it in
main.py, at which point the check and validate.py's block are deleted
together. Nothing in a file another ticket owns is touched.
The consequence is asserted through the real code as well as the literals:
with the key absent the validator accepts, MaintConfig builds 2500 W against
a 2000 W ceiling, and the arbiter resolves it to 0 W as a contradiction -
the bug rule 6 exists to catch, passing rule 6. Production-unreachable (the
Supervisor always supplies every key) but fully reachable from sim/inject.py
and the tests, which is where anyone debugging maintenance works.
Also: a CHANGELOG section for the upgrade path, covering both the case where
the Supervisor re-derives the default and the case where it persisted the
old one, since which applies was not verifiable without probing the live
install. And maintenance.py's timeout comment said 13.5 kWh where everything
else says 13.8 (300 Ah x 51.2 V, DoD 90); SAFETY-18 does not touch that file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
Tripwire added — ffb8ec6, pushed to MAINT-04. Built rather than copied, and verified five ways rather than three; the two extra states are ones your prototype would also catch but that I wanted on the record.
The tripwire
test_validate.py, immediately after the _shipped block and using the same technique. It pins three literals, not two — main.py's two opts.get(..., 2500) fallbacks, and MaintConfig's dataclass default separately, because the dataclass is the one that bites when main.py is bypassed entirely (a Maintenance built straight from defaults, which is what sim and several tests do).
It also asserts the consequence through the real code, not just the literals — so if someone "fixes" this by editing a number without understanding it, what goes red is the behaviour rather than the transcript:
with the key ABSENT the validator accepts... validate_startup_config -> []
...while the code would build 2500 W against a 2000 W ceiling MaintConfig().charge_w = 2500.0
...which the arbiter resolves to 0 W as a contradiction resolve() -> 0.0 W, contradiction=True
...and config.yaml still ships 2000, so the divergence is UNCHANGED (shipped 1800, code 2500)
3. main.py fixed → 2000
1
main.py's fallbacks are still found and still 2500 [2000.0, 2000.0]
4. (mine) MaintConfig closed instead
1
MaintConfig's default is still found and still 2500 [2000.0]+ both behavioural assertions
5. (mine) one call site refactored to a named constant
1
main.py's fallbacks are still found and still 2500 [2500.0] — len == 2 catches the literal vanishing
Baseline restored green after each; full suite green on the pushed head (all 7 files). State 4 is the useful one: closing the divergence at the dataclass but not at main.py fails the literal check and both behavioural checks, which is the right amount of noise for a half-fix.
The check records the mismatch rather than demanding it be closed, exactly as you framed it — so when SAFETY-18 is out of main.py and somebody does close it properly, this check and validate.py's ⚠️ block get deleted together. That instruction is now in both places. validate.py's comment no longer says "remember to keep these together"; it points at the check that enforces it.
Nothing in main.py or p1.py was touched.
The 3-vs-2 discrepancy
Yours is right and mine was under-reported: I reverted only maintenance_charge_w, you reverted both rates, which also trips ...and the shipped discharge rate too. Same conclusion, your number is the fuller one.
Upgrade path — CHANGELOG section added, and I did not probe the install
New ### ⚠️ Upgrading an existing default install section. It leads with the fix (maintenance_charge_w to 2000 or lower, ormax_w to 2500 or higher) and then covers both Supervisor cases, because I could not establish which applies without doing the thing you told me not to do.
Reasoning from documented behaviour rather than measurement: the Supervisor merges an add-on's stored options over the defaults in config.yaml, so a key the operator never edited re-derives to the new 2000 and this is a non-event, while a key they did set persists at 2500 and the add-on exits 78 until it is changed. The trap worth naming, and it is in the CHANGELOG: the Configuration page's YAML editor writes the whole options block back, so "I never touched that field" is not evidence the old value is gone — which makes the persisted case more likely than it looks for anyone who has ever edited any option.
I have marked that section as reasoned, not measured. I did not read the live install's stored options and did not touch it; it runs max_w: 5000 and is unaffected either way.
Nit
maintenance.py:31 now says 13.8 kWh (300 Ah × 51.2 V, DoD 90), and the arithmetic after it follows: ~6.9 h at the shipped 2000 W rather than ~5 h at 2500 W, with the note that house load and soc_floor both make it shorter in practice. Checked for collision first:origin/SAFETY-18 touches CHANGELOG.md, DOCS.md, app/freeze.py, app/main.py, config.yaml, test_freeze.py and a scenario fixture — not maintenance.py, so this is clear. We do both touch CHANGELOG.md and config.yaml; those conflicts are unavoidable and textual.
Still worth your scepticism
The tripwire pins a divergence that should not exist. It is the right move while main.py is owned by another ticket, but it is a check whose passing condition is a bug, and that is an unusual thing to ship — if it outlives SAFETY-18 by long, the follow-up (one source of truth for option defaults) has been forgotten. Worth a ticket now rather than trusting the comment on the check.
**Tripwire added — `ffb8ec6`, pushed to `MAINT-04`.** Built rather than copied, and verified five ways rather than three; the two extra states are ones your prototype would also catch but that I wanted on the record.
## The tripwire
`test_validate.py`, immediately after the `_shipped` block and using the same technique. It pins **three** literals, not two — `main.py`'s two `opts.get(..., 2500)` fallbacks, and `MaintConfig`'s dataclass default separately, because the dataclass is the one that bites when `main.py` is bypassed entirely (a `Maintenance` built straight from defaults, which is what `sim` and several tests do).
It also asserts **the consequence through the real code**, not just the literals — so if someone "fixes" this by editing a number without understanding it, what goes red is the behaviour rather than the transcript:
```
with the key ABSENT the validator accepts... validate_startup_config -> []
...while the code would build 2500 W against a 2000 W ceiling MaintConfig().charge_w = 2500.0
...which the arbiter resolves to 0 W as a contradiction resolve() -> 0.0 W, contradiction=True
```
## Five-way transcript
| state | exit | what goes red |
|---|---|---|
| **1. branch as it stands** | **0** | — `main.py=[2500.0, 2500.0] MaintConfig=[2500.0] config.yaml=2000` |
| **2. `config.yaml` → 1800** | **1** | `...and config.yaml still ships 2000, so the divergence is UNCHANGED (shipped 1800, code 2500)` |
| **3. `main.py` fixed → 2000** | **1** | `main.py's fallbacks are still found and still 2500 [2000.0, 2000.0]` |
| **4. (mine) `MaintConfig` closed instead** | **1** | `MaintConfig's default is still found and still 2500 [2000.0]` **+ both behavioural assertions** |
| **5. (mine) one call site refactored to a named constant** | **1** | `main.py's fallbacks are still found and still 2500 [2500.0]` — `len == 2` catches the literal vanishing |
Baseline restored green after each; full suite green on the pushed head (all 7 files). State 4 is the useful one: closing the divergence at the dataclass but not at `main.py` fails the literal check *and* both behavioural checks, which is the right amount of noise for a half-fix.
The check **records** the mismatch rather than demanding it be closed, exactly as you framed it — so when SAFETY-18 is out of `main.py` and somebody does close it properly, this check and `validate.py`'s ⚠️ block get deleted together. That instruction is now in both places. `validate.py`'s comment no longer says "remember to keep these together"; it points at the check that enforces it.
Nothing in `main.py` or `p1.py` was touched.
## The 3-vs-2 discrepancy
Yours is right and mine was under-reported: I reverted only `maintenance_charge_w`, you reverted both rates, which also trips `...and the shipped discharge rate too`. Same conclusion, your number is the fuller one.
## Upgrade path — CHANGELOG section added, and I did not probe the install
New `### ⚠️ Upgrading an existing default install` section. It leads with the fix (`maintenance_charge_w` to 2000 or lower, **or** `max_w` to 2500 or higher) and then covers both Supervisor cases, because I could not establish which applies without doing the thing you told me not to do.
Reasoning from documented behaviour rather than measurement: the Supervisor merges an add-on's **stored** options over the defaults in `config.yaml`, so a key the operator never edited re-derives to the new 2000 and this is a non-event, while a key they did set persists at 2500 and the add-on exits 78 until it is changed. The trap worth naming, and it is in the CHANGELOG: **the Configuration page's YAML editor writes the whole options block back**, so "I never touched that field" is not evidence the old value is gone — which makes the persisted case more likely than it looks for anyone who has ever edited any option.
**I have marked that section as reasoned, not measured.** I did not read the live install's stored options and did not touch it; it runs `max_w: 5000` and is unaffected either way.
## Nit
`maintenance.py:31` now says 13.8 kWh (300 Ah × 51.2 V, DoD 90), and the arithmetic after it follows: ~6.9 h at the shipped 2000 W rather than ~5 h at 2500 W, with the note that house load and `soc_floor` both make it shorter in practice. **Checked for collision first:** `origin/SAFETY-18` touches `CHANGELOG.md`, `DOCS.md`, `app/freeze.py`, `app/main.py`, `config.yaml`, `test_freeze.py` and a scenario fixture — **not `maintenance.py`**, so this is clear. We do both touch `CHANGELOG.md` and `config.yaml`; those conflicts are unavoidable and textual.
## Still worth your scepticism
The tripwire pins a divergence that **should not exist**. It is the right move while `main.py` is owned by another ticket, but it is a check whose passing condition is a bug, and that is an unusual thing to ship — if it outlives SAFETY-18 by long, the follow-up (one source of truth for option defaults) has been forgotten. Worth a ticket now rather than trusting the comment on the check.
admin
merged commit b0a17474a3 into release/1.02026-08-30 03:01:15 +02:00
Merged as b0a1747. The tripwire does what it claims, in every direction I could push it, and going after MaintConfig separately was the right call — it is the default that bites when main.py is bypassed, which is exactly what sim and several tests do.
States 4 and 5, run by me
State 4 — close the divergence in MaintConfig only (charge_w: float = 2500.0 → 2000.0). Red, on the literal and both behavioural assertions, as you said:
FAIL MaintConfig's default is still found and still 2500 [2000.0]
FAIL ...while the code would build 2000 W against a 2000 W ceiling...
FAIL ...which the arbiter resolves to 0 W as a contradiction - the hole, asserted so it cannot widen unnoticed
The second and third are the ones that matter: they are what stops someone "fixing" this by editing a number. Note they go red because the hole closed, which is the correct and slightly disorienting behaviour — the check asserts the bug, and the fix for that red is to delete the check.
State 5 — one call site refactored to a named constant, value unchanged. Red: main.py's fallbacks are still found and still 2500 [2500.0] — len == 2 catches the literal vanishing even though every literal still reads 2500.
Two more of my own, because state 5 only proves it notices one site going away:
mutation
result
both call sites refactored to a constant
FAIL — [], the empty-list case
both literals kept, one silently changed to 2000
FAIL — [2000.0, 2500.0], caught by the set == {2500.0} half
So the len == 2 and set == {2500.0} pair between them catch a literal disappearing, the count changing, and the two sites drifting apart from each other. I could not find a way to move a default without turning it red.
Re-ran the rest for regressions: states 1-3 as you reported (1 green; 2 → shipped 1800, code 2500; 3 → [2000.0, 2000.0]), and the four original mutations still red on the new head, now with the new checks adding to them (M2 26 vs 25, M3 4 vs 3). Full suite green on the branch and on the merge result — I ran all seven files against release/1.0 after merging, not just against the branch, since that is the combination neither side tests.
The YAML-editor trap: no, leave the prominence where it is
You asked. The section is already a ### ⚠️ heading in Unreleased, it leads with the fix and the click path before any explanation, and the trap is spelled out where an operator will read it — "'I never touched that field' is not a guarantee the old value is gone" is the sentence that does the work. The changelog is what the Supervisor shows before an update, which is the one moment this matters, so that is the right home for it. Escalating further would mean putting it in DOCS.md too, where it would be read long after the decision it informs.
Marking it reasoned-not-measured, and covering both branches because you could not determine which applies, is better than what I asked for. I could not read the live install's stored options either — /api/hassio/ refuses a long-lived token — so that limitation stands on the record for both of us.
I merged this one first, deliberately. #11 is +1077 lines across 7 files and still in review; this was 4 files. Rebasing the large branch over two small textual conflicts is the cheaper and more predictable direction, and CFG-02 is blocked on #11 either way so the order costs it nothing.
For whoever rebases #11 — the conflicts are CHANGELOG.md (both add to Unreleased) and config.yaml (both edit options:), both textual. I checked the one thing that could have been worse and it is fine: SAFETY-18 adds 87 lines to main.py but does not touch either pinned maintenance_charge_w call site, so the new tripwire will not fire on it. Re-run test_validate.py after the rebase anyway — it now reads app/main.py and app/maintenance.py as data, which is a coupling that did not exist when #11 was branched.
One nit, already merged, not worth a PR of its own
app/maintenance.py:34 is now 145 characters — the reflow of the 13.5 → 13.8 kWh comment lost a line break, leaving "# 12 h means the battery is not following commands - a fault, not a slow day. The charge phase deliberately waits for sun and for cheap hours, so" on one line. Cosmetic; fold it into whatever next touches the file.
Good work on this one. The two behavioural assertions are the part I would not have thought to ask for.
**Merged as `b0a1747`.** The tripwire does what it claims, in every direction I could push it, and going after `MaintConfig` separately was the right call — it is the default that bites when `main.py` is bypassed, which is exactly what `sim` and several tests do.
## States 4 and 5, run by me
**State 4 — close the divergence in `MaintConfig` only** (`charge_w: float = 2500.0` → `2000.0`). Red, on the literal *and* both behavioural assertions, as you said:
```
FAIL MaintConfig's default is still found and still 2500 [2000.0]
FAIL ...while the code would build 2000 W against a 2000 W ceiling...
FAIL ...which the arbiter resolves to 0 W as a contradiction - the hole, asserted so it cannot widen unnoticed
```
The second and third are the ones that matter: they are what stops someone "fixing" this by editing a number. Note they go red because the hole *closed*, which is the correct and slightly disorienting behaviour — the check asserts the bug, and the fix for that red is to delete the check.
**State 5 — one call site refactored to a named constant, value unchanged.** Red: `main.py's fallbacks are still found and still 2500 [2500.0]` — `len == 2` catches the literal vanishing even though every literal still reads 2500.
Two more of my own, because state 5 only proves it notices *one* site going away:
| mutation | result |
|---|---|
| **both** call sites refactored to a constant | **FAIL** — `[]`, the empty-list case |
| both literals kept, **one silently changed to 2000** | **FAIL** — `[2000.0, 2500.0]`, caught by the `set == {2500.0}` half |
So the `len == 2` and `set == {2500.0}` pair between them catch a literal disappearing, the count changing, and the two sites drifting apart from each other. I could not find a way to move a default without turning it red.
Re-ran the rest for regressions: states 1-3 as you reported (1 green; 2 → `shipped 1800, code 2500`; 3 → `[2000.0, 2000.0]`), and the four original mutations still red on the new head, now with the new checks adding to them (M2 26 vs 25, M3 4 vs 3). Full suite green on the branch **and on the merge result** — I ran all seven files against `release/1.0` after merging, not just against the branch, since that is the combination neither side tests.
## The YAML-editor trap: no, leave the prominence where it is
You asked. The section is already a `### ⚠️` heading in `Unreleased`, it leads with the fix and the click path before any explanation, and the trap is spelled out where an operator will read it — "'I never touched that field' is not a guarantee the old value is gone" is the sentence that does the work. The changelog is what the Supervisor shows before an update, which is the one moment this matters, so that is the right home for it. Escalating further would mean putting it in `DOCS.md` too, where it would be read long after the decision it informs.
Marking it reasoned-not-measured, and covering both branches because you could not determine which applies, is better than what I asked for. I could not read the live install's stored options either — `/api/hassio/` refuses a long-lived token — so that limitation stands on the record for both of us.
## Merge order with SAFETY-18 (#11)
I merged this one first, deliberately. #11 is +1077 lines across 7 files and still in review; this was 4 files. Rebasing the large branch over two small textual conflicts is the cheaper and more predictable direction, and CFG-02 is blocked on #11 either way so the order costs it nothing.
For whoever rebases #11 — the conflicts are `CHANGELOG.md` (both add to `Unreleased`) and `config.yaml` (both edit `options:`), both textual. **I checked the one thing that could have been worse and it is fine: SAFETY-18 adds 87 lines to `main.py` but does not touch either pinned `maintenance_charge_w` call site**, so the new tripwire will not fire on it. Re-run `test_validate.py` after the rebase anyway — it now reads `app/main.py` and `app/maintenance.py` as data, which is a coupling that did not exist when #11 was branched.
## One nit, already merged, not worth a PR of its own
`app/maintenance.py:34` is now 145 characters — the reflow of the 13.5 → 13.8 kWh comment lost a line break, leaving *"# 12 h means the battery is not following commands - a fault, not a slow day. The charge phase deliberately waits for sun and for cheap hours, so"* on one line. Cosmetic; fold it into whatever next touches the file.
Good work on this one. The two behavioural assertions are the part I would not have thought to ask for.
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 bug
config.yamlshippedmax_w: 2000againstmaintenance_charge_w: 2500. The cheap-window shaping filesClaim.limit(hi = -maintenance_charge_w)atP_MAINT_SHAPEagainst the supervised SAFETY limit'slo = -max_w. Both bind (both outrank the loop that wins the SET), solo > hi— the arbiter's clause 3, correctly resolved to 0 W and logged as a CONTRADICTION.Maintenance silently never charged in a cheap window on a default install, and the only evidence read like a bug in the arbiter rather than like the two numbers the operator set. The live reference install runs
max_w: 5000and is unaffected; this bites a default install, which is every new one.Measured against the real arbiter, loop asking +500 W:
The change
validate_startup_config()gains rule 6: refusemaintenance_charge_w > max_w. Identical shape to CFG-01'ssign_disagree_w >= max_w— a number the operator set, overridden by a layer they cannot see. Strictly>: equality is reachable (-max_wis a single valid point) and refusing it would be exactly the trivia that gets a validator deleted.Not scoped to
maintenance_enabled. The pair is contradictory either way, and scoping would leave the trap armed for the one operator action — ticking the box — that cannot be expected to change what a charge rate means.maintenance_discharge_w— checked, not assumed, and it is NOT the same caseThe shaping is not symmetric. The drain phase does not shape the loop, it OWNS the actuator:
Claim.set(P_MAINTENANCE, +discharge_w). A SET is clamped by the limits above it, not contradicted by them.So it emits +2000 W, and the arbiter's own
explain()prints the clamp on the status page every cycle. That is neither (a) — nothing is silent — nor (b) — it moves energy and the cycle still completes, just slower (13.8 kWh usable at 2000 W is ~6.9 h against the 720 minDRAIN_TIMEOUT_MIN). Warned about, not refused.AC3 — the defaults
Shipping a default the validator refuses would be worse than the bug, so
maintenance_charge_wandmaintenance_discharge_wdrop to 2000, at the shippedmax_w.max_wwas deliberately NOT raised instead: it is the supervised safety ceiling, and raising a default upward ships more power to every new install. Raisingmax_wis what unlocks a faster maintenance cycle — raise it first, then these. That is written intoconfig.yamlnext to the values.test_validate.pynow readsconfig.yamlitself (regex, not PyYAML — the add-on image has no PyYAML, and a check that skips on ImportError is not a check) and asserts the shipped numbers pass the rule. A default the validator refuses cannot ship again.Verification
AC1, by starting it rather than reading it. Real entrypoint (
app.main.main()), options.json carrying the pre-fix pair:Both behaviours in one run: charge refuses, discharge warns and does not. With both at 2000 the same boot passes the gate and reaches
ingress UI listening on :8099.The check goes red when broken.
python3 test_validate.py, four separate breaks:>→>=(refuse the reachable equality)config.yamltomaintenance_charge_w: 2500Restored: exit 0. Whole suite green (
test_arbiter,test_control,test_loop,test_maintenance,test_p1,test_validate,test_watchdog).What to attack hardest
main.pyandmaintenance.pystill fall back to 2500 when the key is absent, whileconfig.yamlandvalidate.pynow say 2000. Left deliberately: those files are SAFETY-18's right now, and the case is unreachable on a Supervisor install (every key is always supplied fromconfig.yaml). But it means a dict with the key MISSING is accepted at 2000 and then built bymain.pyat 2500 — the exact contradiction this rule exists to catch, in the one case it cannot see. The real fix is one source of truth for option defaults. I picked 2000 over 2500 for the fallback because 2500 refused every minimal options dict intest_loop.py/test_watchdog.py; that tradeoff is the judgement call most worth disagreeing with.validate.py's docstring so it can be argued with rather than rediscovered.test_watchdog.pyneeded one case amended (max_w: 500is also below the 2000 W maintenance charge, so it was refused twice). I namedmaintenance_charge_wthere to keep that case about the sign rule. Check I did not weaken it.hass.limits()call returned(None, None), so the device-range branch of rule 2 was not exercised in that run (test_validate.pycovers it).Worked in an isolated git worktree; staged by explicit path. No files outside
goodwe_controller/{config.yaml, DOCS.md, CHANGELOG.md, app/validate.py, test_validate.py, test_watchdog.py}were touched.🤖 Generated with Claude Code
https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
Not merging — one thing, and it is the one you flagged. Everything else I checked held up, including every claim I tried hardest to break. The gap is that the cross-file default divergence is guarded by a comment, and this repo's most-repeated defect is a comment where a check belonged. I am not asking you to touch
main.py— SAFETY-18 is live in it.Required: make the divergence a tripwire, not a comment
validate.py's comment says "if config.yaml's value moves, move this and main.py's together". Nothing enforces that. Meanwhile the same PR proves it knows how to enforce exactly this kind of thing — the_shippedregex block. Apply the same technique to the divergence itself. Prototype, run against your branch:I ran it three ways:
config.yaml=2000.0 main.py={2500.0} MaintConfig={2500.0}config.yamlmoves to 1800 (the scenario your comment warns about)main.pyto 2000 (divergence closed)It records the mismatch instead of hiding it, needs no dependency, touches no file another ticket owns, and fires the moment either number moves. Add it and I merge.
Why I am asking rather than waving it through: the hole is real, not theoretical. Through the actual code:
That is precisely the bug rule 6 exists to catch, passing rule 6. You are right that a Supervisor install always supplies every key, so production is safe today — I accept that and I am not asking you to fix it now. But
sim/inject.pyand the tests are exactly where someone debugging maintenance works, and there the validator says fine while the built config contradicts.Your reasoning on the fallback value: checked, and it holds
You said a 2500 fallback refused every minimal options dict. I reproduced it —
validate.pyfallback 2000 → 2500:So 2000 is the right choice, for the right reason: it matches
config.yaml, which is the source of truth. Good call, and matching the tests is a consequence rather than the motive.The four mutations, re-run by me
>→>=config.yamlto 2500Only discrepancy: reverting
config.yamlgave me 3, not 2 — I reverted both rates, which also trips "...and the shipped discharge rate too". Red either way. Baseline restored green after each; full suite green on the untouched head (all 7 files).Three more of my own:
test_watchdogcase weaker? No — verified. I deleted rule 5 entirely;a threshold above max_w is refusedFAILS. The amended case still bites on the rule it is about. Namingmaintenance_charge_w: 500only removes the second refusal: un-amended the dict returns both problems andlen(p) == 1would be measuring the interaction. The amendment preserves the assertion's intent. This is the one I most expected to find rotten and it is clean.M7—max_w: 2000→1500inconfig.yamlalone: 3 FAILED.M10— append a trailing comment to the shippedmaintenance_charge_wline so the\s*$anchor misses: FAIL "config.yaml really was read (all four defaults found)". The four-key set assertion is what makes a format change loud instead of silent. Regex over PyYAML is the right call here and it is built correctly.resolve()skips limits below the winning SET's priority (arbiter.py:96):A SET at P_MAINTENANCE can only ever be clamped, never contradicted. Warn-don't-refuse is correct and the equality case in rule 6 is genuinely reachable, so strictly
>is right.AC3 margin (item 4): fine, and conservative.
DRAIN_TIMEOUT_MIN = 720; 13.8 kWh at 2000 W = 6.9 h, a 1.7x margin. It is if anything better than that — house load drains the pack alongside the export, andmaintenance_soc_floor: 11means less than the full pack is drained. Lowering the rates rather than raisingmax_wis the right instinct:max_wis the supervised ceiling and a default upward ships more power to every new install.Two things to fold in while you are there
maintenance_charge_w: 2500againstmax_w: 2000now meets rule 6 and exits 78 on upgrade — correct behaviour, loud, with the fix in the message, but an add-on that will not start after an update deserves a CHANGELOG line telling the operator what to change. I could not determine whether Supervisor persisted the old default into stored options or re-derives it fromconfig.yaml—/api/hassio/is not reachable with a long-lived token, so I could not read the live install's stored options. The live reference runsmax_w: 5000and is unaffected either way. If Supervisor re-derives, this is a non-event; if it persists, every existing default install stops on upgrade. Worth one line that covers both.maintenance.py:31says "a 13.5 kWh usable pack"; the PR, CHANGELOG and validator say 13.8 kWh. Same conclusion, two numbers.Add the tripwire and I will merge.
Tripwire added —
ffb8ec6, pushed toMAINT-04. Built rather than copied, and verified five ways rather than three; the two extra states are ones your prototype would also catch but that I wanted on the record.The tripwire
test_validate.py, immediately after the_shippedblock and using the same technique. It pins three literals, not two —main.py's twoopts.get(..., 2500)fallbacks, andMaintConfig's dataclass default separately, because the dataclass is the one that bites whenmain.pyis bypassed entirely (aMaintenancebuilt straight from defaults, which is whatsimand several tests do).It also asserts the consequence through the real code, not just the literals — so if someone "fixes" this by editing a number without understanding it, what goes red is the behaviour rather than the transcript:
Five-way transcript
main.py=[2500.0, 2500.0] MaintConfig=[2500.0] config.yaml=2000config.yaml→ 1800...and config.yaml still ships 2000, so the divergence is UNCHANGED (shipped 1800, code 2500)main.pyfixed → 2000main.py's fallbacks are still found and still 2500 [2000.0, 2000.0]MaintConfigclosed insteadMaintConfig's default is still found and still 2500 [2000.0]+ both behavioural assertionsmain.py's fallbacks are still found and still 2500 [2500.0]—len == 2catches the literal vanishingBaseline restored green after each; full suite green on the pushed head (all 7 files). State 4 is the useful one: closing the divergence at the dataclass but not at
main.pyfails the literal check and both behavioural checks, which is the right amount of noise for a half-fix.The check records the mismatch rather than demanding it be closed, exactly as you framed it — so when SAFETY-18 is out of
main.pyand somebody does close it properly, this check andvalidate.py's ⚠️ block get deleted together. That instruction is now in both places.validate.py's comment no longer says "remember to keep these together"; it points at the check that enforces it.Nothing in
main.pyorp1.pywas touched.The 3-vs-2 discrepancy
Yours is right and mine was under-reported: I reverted only
maintenance_charge_w, you reverted both rates, which also trips...and the shipped discharge rate too. Same conclusion, your number is the fuller one.Upgrade path — CHANGELOG section added, and I did not probe the install
New
### ⚠️ Upgrading an existing default installsection. It leads with the fix (maintenance_charge_wto 2000 or lower, ormax_wto 2500 or higher) and then covers both Supervisor cases, because I could not establish which applies without doing the thing you told me not to do.Reasoning from documented behaviour rather than measurement: the Supervisor merges an add-on's stored options over the defaults in
config.yaml, so a key the operator never edited re-derives to the new 2000 and this is a non-event, while a key they did set persists at 2500 and the add-on exits 78 until it is changed. The trap worth naming, and it is in the CHANGELOG: the Configuration page's YAML editor writes the whole options block back, so "I never touched that field" is not evidence the old value is gone — which makes the persisted case more likely than it looks for anyone who has ever edited any option.I have marked that section as reasoned, not measured. I did not read the live install's stored options and did not touch it; it runs
max_w: 5000and is unaffected either way.Nit
maintenance.py:31now says 13.8 kWh (300 Ah × 51.2 V, DoD 90), and the arithmetic after it follows: ~6.9 h at the shipped 2000 W rather than ~5 h at 2500 W, with the note that house load andsoc_floorboth make it shorter in practice. Checked for collision first:origin/SAFETY-18touchesCHANGELOG.md,DOCS.md,app/freeze.py,app/main.py,config.yaml,test_freeze.pyand a scenario fixture — notmaintenance.py, so this is clear. We do both touchCHANGELOG.mdandconfig.yaml; those conflicts are unavoidable and textual.Still worth your scepticism
The tripwire pins a divergence that should not exist. It is the right move while
main.pyis owned by another ticket, but it is a check whose passing condition is a bug, and that is an unusual thing to ship — if it outlives SAFETY-18 by long, the follow-up (one source of truth for option defaults) has been forgotten. Worth a ticket now rather than trusting the comment on the check.Merged as
b0a1747. The tripwire does what it claims, in every direction I could push it, and going afterMaintConfigseparately was the right call — it is the default that bites whenmain.pyis bypassed, which is exactly whatsimand several tests do.States 4 and 5, run by me
State 4 — close the divergence in
MaintConfigonly (charge_w: float = 2500.0→2000.0). Red, on the literal and both behavioural assertions, as you said:The second and third are the ones that matter: they are what stops someone "fixing" this by editing a number. Note they go red because the hole closed, which is the correct and slightly disorienting behaviour — the check asserts the bug, and the fix for that red is to delete the check.
State 5 — one call site refactored to a named constant, value unchanged. Red:
main.py's fallbacks are still found and still 2500 [2500.0]—len == 2catches the literal vanishing even though every literal still reads 2500.Two more of my own, because state 5 only proves it notices one site going away:
[], the empty-list case[2000.0, 2500.0], caught by theset == {2500.0}halfSo the
len == 2andset == {2500.0}pair between them catch a literal disappearing, the count changing, and the two sites drifting apart from each other. I could not find a way to move a default without turning it red.Re-ran the rest for regressions: states 1-3 as you reported (1 green; 2 →
shipped 1800, code 2500; 3 →[2000.0, 2000.0]), and the four original mutations still red on the new head, now with the new checks adding to them (M2 26 vs 25, M3 4 vs 3). Full suite green on the branch and on the merge result — I ran all seven files againstrelease/1.0after merging, not just against the branch, since that is the combination neither side tests.The YAML-editor trap: no, leave the prominence where it is
You asked. The section is already a
### ⚠️heading inUnreleased, it leads with the fix and the click path before any explanation, and the trap is spelled out where an operator will read it — "'I never touched that field' is not a guarantee the old value is gone" is the sentence that does the work. The changelog is what the Supervisor shows before an update, which is the one moment this matters, so that is the right home for it. Escalating further would mean putting it inDOCS.mdtoo, where it would be read long after the decision it informs.Marking it reasoned-not-measured, and covering both branches because you could not determine which applies, is better than what I asked for. I could not read the live install's stored options either —
/api/hassio/refuses a long-lived token — so that limitation stands on the record for both of us.Merge order with SAFETY-18 (#11)
I merged this one first, deliberately. #11 is +1077 lines across 7 files and still in review; this was 4 files. Rebasing the large branch over two small textual conflicts is the cheaper and more predictable direction, and CFG-02 is blocked on #11 either way so the order costs it nothing.
For whoever rebases #11 — the conflicts are
CHANGELOG.md(both add toUnreleased) andconfig.yaml(both editoptions:), both textual. I checked the one thing that could have been worse and it is fine: SAFETY-18 adds 87 lines tomain.pybut does not touch either pinnedmaintenance_charge_wcall site, so the new tripwire will not fire on it. Re-runtest_validate.pyafter the rebase anyway — it now readsapp/main.pyandapp/maintenance.pyas data, which is a coupling that did not exist when #11 was branched.One nit, already merged, not worth a PR of its own
app/maintenance.py:34is now 145 characters — the reflow of the 13.5 → 13.8 kWh comment lost a line break, leaving "# 12 h means the battery is not following commands - a fault, not a slow day. The charge phase deliberately waits for sun and for cheap hours, so" on one line. Cosmetic; fold it into whatever next touches the file.Good work on this one. The two behavioural assertions are the part I would not have thought to ask for.