Built in goodwe_controller/app/, not at specs.md §2.2's ems/ingest/prices/, per the product owner's decision of 2026-08-30 on the card. The behaviour §9.3 / §8.3 / §11.4 specifies is unchanged and still authoritative; only the path moved.
Nothing consumes a price yet, and that is the shape of the ticket. This is the seam — the five plugins, the canonical format, the config plumbing. The arbitrage strategy (OPT-04), the static fallback (TARIFF-03) and the peak tracker (TARIFF-01) are separate cards and none of them is touched here. Turning a provider on today fetches, validates and publishes prices and changes no charging decision.
What ships
goodwe_controller/app/price.py — the framework and five providers: generic_dayahead (wholesale EPEX BE), engie_dynamic, totalenergies_dynamic, eneco_eddy, manual_csv.
goodwe_controller/test_price.py — 12 sections, ~130 runnable checks, no network.
Every plugin emits exactly {"prices": [{"start", "eur_per_kwh", "provisional"}]} — three keys, start tz-aware at the Brussels offset, EUR/kWh, all-in. The check derives the key set from the emitted objects rather than asserting a typed literal, so a fourth field would be red.
The three traps the card named
1. Double-counting the markup is refused, not discouraged — given a correct declaration
A plugin does not return a price. It returns RawPoints carrying the figure its source published, and declares in capabilities().sources which of {ENERGY, MARKUP, TAXES} that figure already contains. assemble() — the framework, never a plugin — adds exactly the complement.
Two mechanisms, because one was not enough — and review of the first push proved it.PluginOptions raises FrameworkOption on the framework-owned keys, which closes the documented path. Review then wrote a double-counting plugin three more ways, one of them touching nothing but PRICE_OPTION_DEFAULTS, a public dict in the module every plugin must already import for register. One closed door in a room with three is not a guarantee.
So register() also refuses at import any plugin whose own code mentions a framework-owned name (FRAMEWORK_KEYS plus PRICE_OPTION_DEFAULTS, price_option, _o). code_leaks() parses the class and drops every docstring node before searching — every plugin is required to name its sourced components in prose, so a raw text scan false-positives on engie_dynamic's own SOURCES: line, and a check that fires on required documentation is a check somebody deletes. All three attacks are written out as plugins in the suite; honest wholesale baseline 0.188, every attack 0.219.
The precise shape of the guarantee, stated because an overclaim is worse than no claim: the framework adds the complement exactly once, given a correct sources declaration. A plugin that declares {ENERGY} while its feed is really all-in still comes out one markup high while touching no framework name at all — measured at 0.276 in the suite — and no code can catch that, because the declaration is the plugin's unverifiable claim about somebody else's tariff. What is mechanised is reachability; what stays human is truthfulness.
plugin
sources
framework adds
generic_dayahead
energy
markup + taxes
engie_dynamic
energy + markup
taxes
totalenergies_dynamic
energy + markup
taxes
eneco_eddy
all-in
nothing
manual_csv
all-in
nothing
Each plugin's docstring states this in two fixed lines, and the checks parse those lines out of the docstring and compare them against the declaration, which verifies that the plugin's two claims agree, not that either is true.
A plugin whose feed is already all-in logs that the configured markup is being ignored. Silently discarding an operator's number is its own defect.
2. Product type is declared, never inferred from a name
capabilities().product is a PriceProduct enum; eneco_eddy declares REALTIME, and volatility_regime() — the only place in shipped code that says which products are volatile — maps that to "wide". The regime rides on the series a consumer is handed.
AC2 is checked mechanically two ways:
Negative:inspect.getsource() over the framework's own functions and classes, asserting the set of plugin ids appearing in them is empty. If assemble or load_prices ever grows a name comparison, it goes red.
Positive: the test file defines a sixth provider inside itself, registers it, and asserts it resolves through the registry, lands in the wide regime purely from its declaration, and gets its markup applied — with price.py untouched. That is AC2 as an executable statement rather than a claim.
volatility_regime("eneco_eddy") raises TypeError, so the AC2 violation cannot be written accidentally later.
3. Absence of prices is a type
Following the rework sim/riglock.py took under TEST-04 this week — same _Outcome shape, down to the fatal .get():
PriceRead.series(...)refuses an empty point list. "I have no prices" cannot be constructed wearing the shape of an answer.
PriceRead.no_data(reason) carries .reasonand nothing else. No .points to iterate, no .product to switch on, no .at(), and .get("points", []) raises. __iter__ raises too, so for p in read: cannot be a harmless empty loop.
.match(*, series, no_data) takes both handlers as required keyword arguments — a forgotten arm is a TypeError at the call site, not a fall-through.
A blank reason is refused; no_data's one field is the operator's only evidence.
The distinction that matters: a series of all-zero prices is a series, not no_data. Arbitrage is pointless under zero prices and forbidden under no data (§11.4), and an implementation that returns [] on a failed fetch cannot tell a caller which it is in. The checks drive both and assert arbitrage_allowed / plan_is_provisional disagree across them.
Six site-level failure modes (unreachable source, non-JSON body, empty feed, unknown id, provider off, missing CSV) all return no_data — load_prices() never raises into the tick. The one exception is FrameworkOption, which is re-raised: a plugin reaching for the markup is a code defect, and degrading it to "no price data" would hide exactly what trap 1 exists to catch.
The HA-side overlap, named and decided
price_now_entity + price_avg_entity are the existing delegation of pricing to Home Assistant. main.py reduces them to one boolean, cheap = now <= avg, read by nothing except the maintenance cycle's cheap-window charge floor. That is all they have ever done.
Decision: kept, not retired, and made non-overlapping by refusal. Retiring them would take the maintenance cycle's only price signal away on a live install before TARIFF-03 or OPT-04 has landed to replace it. But price_provider set alongside them is refused at startup (validate.py rule 7) — because in that configuration the HA entities silently win, the provider series drives nothing, and the operator's only evidence that they picked a supplier is a status page saying they picked a supplier. That is case (a) in validate.py's own taxonomy, and it is the "silently double up" the PM warned about. Either alone still starts.
Rule 7 also refuses an unknown price_provider id (rather than defaulting to off, per the §14 warning box) and manual_csv with no price_csv_path. price_provider: "off" is not refused and the docstring says why it must never become so: §11.4 makes no-price-data a supported state and §9.2 puts a large share of the installed base on a fixed or static tariff.
AC4 — injection priced separately
export_price_eur_kwh, a scalar, not a second series. Negative values accepted (Belgian injection has gone below zero). It is in FRAMEWORK_KEYS, so a consumption plugin cannot fold it into a consumption price, and a check asserts it appears in no plugin's emitted series.
CFG-03 — how this avoids widening it
The add-on already carries option defaults in three places, held apart by a tripwire in test_validate.py. This ticket adds one in-code mirror, not another three:
every price default is in PRICE_OPTION_DEFAULTS, and only there;
validate.pyimports it rather than restating any number;
main.py gets no o.get("price_...", X) fallbacks at all;
no dataclass carries a default copy;
test_price.pyreads config.yaml and asserts the shipped default equals the mirror key for key, that every key is in the schema: block, and — derived from the registry — that the schema's list(...) of providers is exactly the registered plugins plus off.
config.yaml stays the source of truth. Same pattern test_validate.py already uses for max_w and meter_freeze_s.
Two design notes worth reviewing
No zoneinfo. The shipped Alpine image installs py3-aiohttp py3-paho-mqtt from apk and has no tzdata — ZoneInfo("Europe/Brussels") raises there. Offsets come from the EU rule (CEST from 01:00 UTC on the last Sunday of March to 01:00 UTC on the last Sunday of October), every instant is carried as UTC and rendered at the offset that applies to it, and brussels() refuses a naive datetime rather than assuming one. Fixed offsets have no fold, so the October hour that happens twice is two instants rendering as 02:00+02:00 and 02:00+01:00.
Egress is declared by the plugin and enforced. §16 allow-lists outbound; check_egress() refuses non-TLS, refuses any host not in capabilities().egress_hosts, and is not fooled by a userinfo@ prefix. An operator-supplied price_feed_url pointing at another supplier's host is refused. manual_csv declares () and imports no HTTP stack at all — aiohttp is imported lazily inside https_get.
Checks: run, then broken
python3 test_price.py — 1 s, no network. The whole add-on suite is green (test_freeze 34 s, test_loop 93 s as documented).
Two standards this file holds itself to, since they will be checked:
Every verdict is derived. There is no check("...", True) anywhere. Expected all-in figures are computed in the test from the fixture's energy component and the plugin's own declaration, by different arithmetic than assemble() uses. manual_csv's expected prices and provisional flags are parsed back out of the CSV body rather than typed. Key sets, offset sets, regime maps and the schema's provider list are built from the objects. A derived() helper turns a computation that raises into a _Raised sentinel that equals nothing and prints the exception — explicitly not a default, so a mutation that makes a derivation impossible reports as a failed check naming what raised, instead of aborting the file before later sections run.
No stimulus is graded where the quantity is degenerate, and this is the real hazard here:
The shipped defaults are markup 0.03 / taxes 0.06 — and 0.06 is exactly twice 0.03. Under that pair several markup confusions collapse into the same number. The arithmetic checks therefore use 0.031 / 0.057, and section 0 asserts the non-degeneracy conditions as checks in their own right (both non-zero, unequal, and 2·markup ≠ markup+taxes) plus the fact that the shipped pair is degenerate.
The negative-price fixture is −95 EUR/MWh, not −12.5. All-in adds 0.088 EUR/kWh, so a −12.5 hour comes out at +0.0755 and the "a negative price survives" check would have been grading a positive number — it passed for the wrong reason on the first run and the fixture was changed. −95 is realistic for a windy Belgian Sunday and still negative after markup and taxes.
Every provisional fixture is mixed (one provisional point, two settled), and plan_is_provisional is checked against a fully-settled series too. An all-true or all-false fixture returns the same answer under any bug.
The DST checks run on the 23-hour and 25-hour days (2026-03-29, 2026-10-25), with an ordinary January day as the control. On a normal day a fixed 24-slot array, a naive-local-hour key and a correct implementation all agree exactly.
The raw prices are non-zero and distinct, so a plugin that dropped the energy component entirely would not pass.
Twenty-four mutations were driven through it after the review pass; all twenty-four go red. A selection:
mutation
what went red
assemble() ignores caps.adds, always adds both
9 checks — every retail plugin's all-in figure, and the rogue-plugin delta collapses to 0.0
empty PriceRead.series becomes constructible
"an EMPTY series cannot be constructed at all"
_NoData grows points = ()
"no_data has no .points"
match() lets the no_data arm default
"a caller who forgets the no_data arm gets a TypeError"
_Outcome.get() returns the default again (the TEST-04 defect)
".get('points', []) … raises rather than returning the default"
a failed fetch degrades to a zero-price series
4 checks, including the reason field
Brussels is always CET
6 DST checks, both transition days
volatility_regime returns "normal" always
3 checks incl. the sixth-supplier seam
PluginOptions stops refusing framework keys
3 checks incl. the re-raise
duplicate / out-of-order starts accepted
the DST-collision boundary checks
negative prices clamped at the boundary
26 checks (the fixture stops parsing)
config.yaml default drifts from the mirror
the CFG-03 tripwire
the HA-overlap refusal removed
rule 7, incl. "the message says which one actually wins"
a plugin docstring drifts from its declaration
the docstring/declaration agreement check
Not in scope, deliberately
No optimizer, no arbitrage strategy, no peak tracker (OPT-04 / TARIFF-01 / TARIFF-03). No persistence of fetched series (§15) and no fetch scheduler — both belong with the consumer that needs them, and adding a scheduler nothing reads would be a background task on a live controller for no benefit. No main.py wiring, so the running add-on's behaviour is byte-identical unless an operator sets price_provider. No live system was written to, restarted or flashed.
What is not verified, and cannot be by an offline suite
Every fixture is synthetic. The five feed URLs and JSON shapes are checked against a belief about what those suppliers publish, not against the feeds — no check reaches the network, which is correct for CI but means the parse layer is verified against my reading of each API rather than against the API. Expect the first live fetch of each supplier to need a shape fix.
The mitigating property, and the reason this is an acceptable limit rather than a hidden one: a wrong feed shape degrades to no-price-data, not to a wrong price._points_from_json raises SourceUnavailable on a missing key, and load_prices turns that into PriceRead.no_data(reason) — so being wrong about a supplier lands on the §11.4 row (static profile, plan provisional, no arbitrage) rather than on a confidently mispriced day.
manual_csv is unaffected: its format is defined by this repo, not by a third party, and it is fully exercised.
Open question #10 (§20) still stands and is for the product owner: which dynamic suppliers are most common in the target base. The five here are the spec's list; the framework makes a sixth a class and a registry line.
Closes TARIFF-02 (P1, 8 SP, §9.3).
Built in `goodwe_controller/app/`, **not** at specs.md §2.2's `ems/ingest/prices/`, per the product owner's decision of 2026-08-30 on the card. The behaviour §9.3 / §8.3 / §11.4 specifies is unchanged and still authoritative; only the path moved.
**Nothing consumes a price yet, and that is the shape of the ticket.** This is the seam — the five plugins, the canonical format, the config plumbing. The arbitrage strategy (OPT-04), the static fallback (TARIFF-03) and the peak tracker (TARIFF-01) are separate cards and none of them is touched here. Turning a provider on today fetches, validates and publishes prices and changes no charging decision.
## What ships
- `goodwe_controller/app/price.py` — the framework and five providers: `generic_dayahead` (wholesale EPEX BE), `engie_dynamic`, `totalenergies_dynamic`, `eneco_eddy`, `manual_csv`.
- `goodwe_controller/test_price.py` — 12 sections, ~130 runnable checks, no network.
- `app/validate.py` rule 7, `config.yaml` options + schema, `DOCS.md`, `CHANGELOG.md`.
Every plugin emits exactly `{"prices": [{"start", "eur_per_kwh", "provisional"}]}` — three keys, `start` tz-aware at the Brussels offset, EUR/kWh, all-in. The check derives the key set from the emitted objects rather than asserting a typed literal, so a fourth field would be red.
## The three traps the card named
### 1. Double-counting the markup is refused, not discouraged — given a correct declaration
A plugin **does not return a price**. It returns `RawPoint`s carrying the figure its source published, and declares in `capabilities().sources` which of `{ENERGY, MARKUP, TAXES}` that figure already contains. `assemble()` — the framework, never a plugin — adds exactly the complement.
**Two mechanisms, because one was not enough — and review of the first push proved it.** `PluginOptions` raises `FrameworkOption` on the framework-owned keys, which closes the documented path. Review then wrote a double-counting plugin three more ways, one of them touching nothing but `PRICE_OPTION_DEFAULTS`, a public dict in the module every plugin must already import for `register`. One closed door in a room with three is not a guarantee.
So `register()` also **refuses at import any plugin whose own code mentions a framework-owned name** (`FRAMEWORK_KEYS` plus `PRICE_OPTION_DEFAULTS`, `price_option`, `_o`). `code_leaks()` parses the class and drops every docstring node before searching — every plugin is *required* to name its sourced components in prose, so a raw text scan false-positives on `engie_dynamic`'s own `SOURCES:` line, and a check that fires on required documentation is a check somebody deletes. All three attacks are written out as plugins in the suite; honest wholesale baseline **0.188**, every attack **0.219**.
**The precise shape of the guarantee**, stated because an overclaim is worse than no claim: *the framework adds the complement exactly once, given a correct `sources` declaration.* A plugin that declares `{ENERGY}` while its feed is really all-in still comes out one markup high while touching no framework name at all — measured at **0.276** in the suite — and no code can catch that, because the declaration is the plugin's unverifiable claim about somebody else's tariff. What is mechanised is reachability; what stays human is truthfulness.
| plugin | sources | framework adds |
|---|---|---|
| `generic_dayahead` | energy | markup + taxes |
| `engie_dynamic` | energy + markup | taxes |
| `totalenergies_dynamic` | energy + markup | taxes |
| `eneco_eddy` | all-in | nothing |
| `manual_csv` | all-in | nothing |
Each plugin's docstring states this in two fixed lines, and the checks **parse those lines out of the docstring and compare them against the declaration**, which verifies that the plugin's two claims *agree*, not that either is true.
A plugin whose feed is already all-in logs that the configured markup is being ignored. Silently discarding an operator's number is its own defect.
### 2. Product type is declared, never inferred from a name
`capabilities().product` is a `PriceProduct` enum; `eneco_eddy` declares `REALTIME`, and `volatility_regime()` — the only place in shipped code that says which products are volatile — maps that to `"wide"`. The regime rides on the series a consumer is handed.
AC2 is checked mechanically two ways:
- **Negative:** `inspect.getsource()` over the framework's own functions and classes, asserting the set of plugin ids appearing in them is empty. If `assemble` or `load_prices` ever grows a name comparison, it goes red.
- **Positive:** the test file **defines a sixth provider inside itself**, registers it, and asserts it resolves through the registry, lands in the wide regime purely from its declaration, and gets its markup applied — with `price.py` untouched. That is AC2 as an executable statement rather than a claim.
`volatility_regime("eneco_eddy")` raises `TypeError`, so the AC2 violation cannot be written accidentally later.
### 3. Absence of prices is a type
Following the rework `sim/riglock.py` took under TEST-04 this week — same `_Outcome` shape, down to the fatal `.get()`:
- `PriceRead.series(...)` **refuses an empty point list.** "I have no prices" cannot be constructed wearing the shape of an answer.
- `PriceRead.no_data(reason)` carries `.reason` **and nothing else**. No `.points` to iterate, no `.product` to switch on, no `.at()`, and `.get("points", [])` raises. `__iter__` raises too, so `for p in read:` cannot be a harmless empty loop.
- `.match(*, series, no_data)` takes both handlers as required keyword arguments — a forgotten arm is a `TypeError` at the call site, not a fall-through.
- A blank reason is refused; `no_data`'s one field is the operator's only evidence.
**The distinction that matters:** a series of all-zero prices is a `series`, not `no_data`. Arbitrage is *pointless* under zero prices and *forbidden* under no data (§11.4), and an implementation that returns `[]` on a failed fetch cannot tell a caller which it is in. The checks drive both and assert `arbitrage_allowed` / `plan_is_provisional` disagree across them.
Six site-level failure modes (unreachable source, non-JSON body, empty feed, unknown id, provider off, missing CSV) all return `no_data` — `load_prices()` never raises into the tick. The one exception is `FrameworkOption`, which is re-raised: a plugin reaching for the markup is a code defect, and degrading it to "no price data" would hide exactly what trap 1 exists to catch.
## The HA-side overlap, named and decided
`price_now_entity` + `price_avg_entity` are the existing delegation of pricing to Home Assistant. `main.py` reduces them to **one boolean**, `cheap = now <= avg`, read by nothing except the maintenance cycle's cheap-window charge floor. That is all they have ever done.
**Decision: kept, not retired, and made non-overlapping by refusal.** Retiring them would take the maintenance cycle's only price signal away on a live install before TARIFF-03 or OPT-04 has landed to replace it. But `price_provider` set *alongside* them is refused at startup (`validate.py` rule 7) — because in that configuration the HA entities silently win, the provider series drives nothing, and the operator's only evidence that they picked a supplier is a status page saying they picked a supplier. That is case (a) in validate.py's own taxonomy, and it is the "silently double up" the PM warned about. Either alone still starts.
Rule 7 also refuses an unknown `price_provider` id (rather than defaulting to `off`, per the §14 warning box) and `manual_csv` with no `price_csv_path`. `price_provider: "off"` is **not** refused and the docstring says why it must never become so: §11.4 makes no-price-data a supported state and §9.2 puts a large share of the installed base on a fixed or static tariff.
## AC4 — injection priced separately
`export_price_eur_kwh`, a scalar, not a second series. Negative values accepted (Belgian injection has gone below zero). It is in `FRAMEWORK_KEYS`, so a consumption plugin cannot fold it into a consumption price, and a check asserts it appears in no plugin's emitted series.
## CFG-03 — how this avoids widening it
The add-on already carries option defaults in three places, held apart by a tripwire in `test_validate.py`. This ticket adds **one** in-code mirror, not another three:
- every price default is in `PRICE_OPTION_DEFAULTS`, and only there;
- `validate.py` **imports** it rather than restating any number;
- `main.py` gets no `o.get("price_...", X)` fallbacks at all;
- no dataclass carries a default copy;
- `test_price.py` **reads `config.yaml`** and asserts the shipped default equals the mirror key for key, that every key is in the `schema:` block, and — derived from the registry — that the schema's `list(...)` of providers is exactly the registered plugins plus `off`.
`config.yaml` stays the source of truth. Same pattern `test_validate.py` already uses for `max_w` and `meter_freeze_s`.
## Two design notes worth reviewing
**No `zoneinfo`.** The shipped Alpine image installs `py3-aiohttp py3-paho-mqtt` from apk and has **no `tzdata`** — `ZoneInfo("Europe/Brussels")` raises there. Offsets come from the EU rule (CEST from 01:00 UTC on the last Sunday of March to 01:00 UTC on the last Sunday of October), every instant is carried as UTC and rendered at the offset that applies to it, and `brussels()` refuses a naive datetime rather than assuming one. Fixed offsets have no fold, so the October hour that happens twice is two instants rendering as `02:00+02:00` and `02:00+01:00`.
**Egress is declared by the plugin and enforced.** §16 allow-lists outbound; `check_egress()` refuses non-TLS, refuses any host not in `capabilities().egress_hosts`, and is not fooled by a `userinfo@` prefix. An operator-supplied `price_feed_url` pointing at another supplier's host is refused. `manual_csv` declares `()` and imports no HTTP stack at all — `aiohttp` is imported lazily inside `https_get`.
## Checks: run, then broken
`python3 test_price.py` — 1 s, no network. The whole add-on suite is green (`test_freeze` 34 s, `test_loop` 93 s as documented).
Two standards this file holds itself to, since they will be checked:
**Every verdict is derived.** There is no `check("...", True)` anywhere. Expected all-in figures are computed in the test from the fixture's energy component and the plugin's own declaration, by different arithmetic than `assemble()` uses. `manual_csv`'s expected prices and provisional flags are parsed back out of the CSV body rather than typed. Key sets, offset sets, regime maps and the schema's provider list are built from the objects. A `derived()` helper turns a computation that *raises* into a `_Raised` sentinel that equals nothing and prints the exception — explicitly **not** a default, so a mutation that makes a derivation impossible reports as a failed check naming what raised, instead of aborting the file before later sections run.
**No stimulus is graded where the quantity is degenerate**, and this is the real hazard here:
- The shipped defaults are `markup 0.03` / `taxes 0.06` — and **0.06 is exactly twice 0.03**. Under that pair several markup confusions collapse into the same number. The arithmetic checks therefore use `0.031` / `0.057`, and **section 0 asserts the non-degeneracy conditions as checks in their own right** (both non-zero, unequal, and `2·markup ≠ markup+taxes`) plus the fact that the shipped pair is degenerate.
- The negative-price fixture is **−95 EUR/MWh, not −12.5**. All-in adds 0.088 EUR/kWh, so a −12.5 hour comes out at *+0.0755* and the "a negative price survives" check would have been grading a positive number — it passed for the wrong reason on the first run and the fixture was changed. −95 is realistic for a windy Belgian Sunday and still negative after markup and taxes.
- Every provisional fixture is **mixed** (one provisional point, two settled), and `plan_is_provisional` is checked against a fully-settled series too. An all-true or all-false fixture returns the same answer under any bug.
- The DST checks run on the **23-hour and 25-hour days** (2026-03-29, 2026-10-25), with an ordinary January day as the control. On a normal day a fixed 24-slot array, a naive-local-hour key and a correct implementation all agree exactly.
- The raw prices are non-zero and distinct, so a plugin that dropped the energy component entirely would not pass.
**Twenty-four mutations were driven through it after the review pass; all twenty-four go red.** A selection:
| mutation | what went red |
|---|---|
| `assemble()` ignores `caps.adds`, always adds both | 9 checks — every retail plugin's all-in figure, and the rogue-plugin delta collapses to 0.0 |
| empty `PriceRead.series` becomes constructible | "an EMPTY series cannot be constructed at all" |
| `_NoData` grows `points = ()` | "`no_data` has no `.points`" |
| `match()` lets the `no_data` arm default | "a caller who forgets the no_data arm gets a TypeError" |
| `_Outcome.get()` returns the default again (the TEST-04 defect) | "`.get('points', [])` … raises rather than returning the default" |
| a failed fetch degrades to a zero-price series | 4 checks, including the reason field |
| Brussels is always CET | 6 DST checks, both transition days |
| `volatility_regime` returns `"normal"` always | 3 checks incl. the sixth-supplier seam |
| `PluginOptions` stops refusing framework keys | 3 checks incl. the re-raise |
| duplicate / out-of-order starts accepted | the DST-collision boundary checks |
| negative prices clamped at the boundary | 26 checks (the fixture stops parsing) |
| `config.yaml` default drifts from the mirror | the CFG-03 tripwire |
| the HA-overlap refusal removed | rule 7, incl. "the message says which one actually wins" |
| a plugin docstring drifts from its declaration | the docstring/declaration agreement check |
## Not in scope, deliberately
No optimizer, no arbitrage strategy, no peak tracker (OPT-04 / TARIFF-01 / TARIFF-03). No persistence of fetched series (§15) and no fetch scheduler — both belong with the consumer that needs them, and adding a scheduler nothing reads would be a background task on a live controller for no benefit. No `main.py` wiring, so the running add-on's behaviour is byte-identical unless an operator sets `price_provider`. No live system was written to, restarted or flashed.
## What is not verified, and cannot be by an offline suite
**Every fixture is synthetic.** The five feed URLs and JSON shapes are checked against a *belief* about what those suppliers publish, not against the feeds — no check reaches the network, which is correct for CI but means the parse layer is verified against my reading of each API rather than against the API. Expect the first live fetch of each supplier to need a shape fix.
The mitigating property, and the reason this is an acceptable limit rather than a hidden one: **a wrong feed shape degrades to no-price-data, not to a wrong price.** `_points_from_json` raises `SourceUnavailable` on a missing key, and `load_prices` turns that into `PriceRead.no_data(reason)` — so being wrong about a supplier lands on the §11.4 row (static profile, plan provisional, no arbitrage) rather than on a confidently mispriced day.
`manual_csv` is unaffected: its format is defined by this repo, not by a third party, and it is fully exercised.
Open question #10 (§20) still stands and is for the product owner: which dynamic suppliers are most common in the target base. The five here are the spec's list; the framework makes a sixth a class and a registry line.
Five provider plugins - generic_dayahead, engie_dynamic,
totalenergies_dynamic, eneco_eddy, manual_csv - all emitting exactly
{"prices": [{"start", "eur_per_kwh", "provisional"}]}, tz-aware at the
Brussels offset, EUR/kWh, all-in. Nothing consumes it yet; the seam
lands first, per the PM's ordering.
It is built in goodwe_controller/app/ rather than at specs.md 2.2's
ems/ingest/prices/, per the product owner's decision of 2026-08-30. The
behaviour 9.3 / 8.3 / 11.4 specifies is unchanged.
Three things shape the design, and each is the mechanism rather than a
comment saying to be careful:
Double-counting the markup is unreachable. A plugin does not return a
price - it returns the figure its source published and declares which of
{energy, markup, taxes} that figure already contains. assemble() adds
the complement, once, in the framework. A plugin is never handed
price_markup_eur_kwh: PluginOptions raises FrameworkOption on it, so the
line that adds the margin a second time cannot be written. Engie and
TotalEnergies publish a retail figure with their margin inside; adding
the configured 3 c/kWh on top would have made every arbitrage decision
wrong by that amount with nothing reporting it.
The product type is declared, not inferred from the plugin's name.
eneco_eddy is real-time/imbalance and gets 8.3's widened thresholds
through capabilities().product and volatility_regime(), never through a
name comparison - which is AC2. test_price.py asserts mechanically that
no plugin id appears anywhere in the framework's code, and drives a
sixth provider defined inside the test file to the right regime with
price.py untouched.
"No price data" is a type, not an empty list - the same rework
sim/riglock.py took under TEST-04. A fetch returns PriceRead.series(...)
or PriceRead.no_data(reason), and only the first carries fields. An
empty series is not constructible; no_data has no .points, no .at(), and
.get("points", []) raises rather than returning the default. A day of
free electricity and a day with no prices are different instructions -
arbitrage is pointless under one and forbidden under the other - and an
implementation returning [] cannot tell them apart. Every site-level
failure returns no_data; nothing raises into the control tick.
Also: injection stays a separate scalar (export_price_eur_kwh, AC4),
negative prices survive the boundary validator (17's regression case),
egress is allow-listed against each plugin's own declaration (16), and
no tzdata is used - the shipped Alpine image has none, so Brussels
offsets come from the EU rule and instants are carried as UTC.
validate.py grows rule 7: an unknown provider id, manual_csv with no
path, and - the HA overlap the PM told us to decide about -
price_provider set alongside price_now_entity/price_avg_entity, which is
two price sources where the HA pair silently wins. The HA sensors are
kept and still work alone; TARIFF-03 and OPT-04 are what replace them.
CFG-03 is not widened: the price defaults have ONE in-code mirror,
PRICE_OPTION_DEFAULTS, which validate.py imports rather than restating
and which test_price.py pins to config.yaml by reading the file.
test_price.py is runnable, assert-free in the house style, and every
verdict it prints is derived. It grades the markup arithmetic with
0.031/0.057 rather than the shipped 0.03/0.06, because 0.06 is exactly
twice 0.03 and a degenerate pair would make double-counting
indistinguishable from correct addition - and it asserts that
non-degeneracy as its first section. The DST checks run on the 23-hour
and 25-hour days, not on a normal day where a fixed 24-slot
implementation and a correct one agree. Thirteen mutations were driven
through it; all thirteen go red.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
Not merging, narrowly — and the reason is the one thing you were asked to make true. The code is correct, the absence type is excellent, the DST arithmetic is right, and I could not fault the config discipline. But I can write a double-counting plugin four different ways, one of them touching nothing but a public module constant. AC1's headline is "unreachable, not discouraged", and it is discouraged on one path of four.
The fix I am asking for is ~12 lines and I have already written and measured it. Everything else below is verification that came out clean.
1. Double-counting: reachable
Honest baseline for a wholesale plugin: energy 0.10 + markup + taxes = 0.188 (at your test constants).
attack
what the plugin touches
all-in
verdict
A6 opts.get("price_markup_eur_kwh")
the documented path
—
FrameworkOption — blocked ✅
A1 opts._o["price_markup_eur_kwh"]
private attribute of PluginOptions
0.219
double-counted
A2 price_option(o, "price_markup_eur_kwh")
a public framework function
0.219
double-counted
A2b PRICE_OPTION_DEFAULTS["price_markup_eur_kwh"]
a public dict, nothing else
0.22 vs 0.19
double-counted
A3 all-in figure declared sources={ENERGY}
no framework option at all
0.276
double-counted
A4 energy-only figure declared all-in
no framework option at all
0.100
under-counted
A2b is the one that decides this. The plugin reaches for no private attribute, needs no options dict, and calls no framework helper — it reads a module-level public constant in the module it must already import for PriceProvider, RawPoint and register. So this sentence, in the PR and in PluginOptions' docstring, is not true:
a plugin is never handed the markup... the plugin author who wants to add their supplier's margin a second time cannot write the line, because the number is not in scope.
The number is in scope. PluginOptions closes one door in a room with three.
What I am asking for — verified, not suggested. You already wrote the negative source scan for AC2 (framework source must contain no plugin ids). Its mirror closes A1 and A2/A2b: plugin code must contain no framework-key names. Docstrings have to be stripped, or engie_dynamic false-positives on its own SOURCES: line — I hit that and fixed it by popping docstring nodes before ast.unparse:
--- shipped plugins (must be clean) ---
eneco_eddy clean
engie_dynamic clean <- clean only after stripping docstrings
generic_dayahead clean
manual_csv clean
totalenergies_dynamic clean
--- the reachable attacks (must be caught) ---
a1 ['_o', 'price_markup_eur_kwh']
a2 ['_o', 'price_markup_eur_kwh', 'price_option']
A3/A4 I am explicitly not asking you to close. No code can verify whether a supplier's published figure already contains their margin — the declaration is the plugin's unverifiable claim about its source, and that is inherent, not sloppy. But it means the guarantee has a precise shape and the docstring should state it: the framework adds the complement exactly once, given a correct sources declaration. As written, the docstring-parse check (prose vs declaration) reads as if it verified more than it does: it verifies that two claims agree, not that either is true.
2. The absence type: the best version of this shape I have reviewed
Every mutation I carried over from TEST-04 and RIG-01:
Absence not being an AttributeError is the property the whole shape rests on, and you kept it.
The all-zero boundary holds exactly as §11.4 requires, which is the thing I most expected to find wrong:
all-zero series .kind = series
arbitrage_allowed(all-zero) = True (pointless, but ALLOWED)
arbitrage_allowed(no_data) = False (FORBIDDEN)
plan_is_provisional(zero) = False
plan_is_provisional(no_data) = True
Two escapes, both the vars() class and neither accidental: vars(nd).get("points",[]) returns [], and bool(nd) is True — so if read: is true for both variants and reads like a check without being one. Worth a line in the docstring, not code.
3. Mutations — 15 run, 3 survive
M1 arbitrage veto removed (no_data -> True) RED 2
M2 plan_is_provisional inverted RED 1
M3 _NoData reason gets a DEFAULT *** SURVIVED ***
M4b _Series accepts an empty point list RED 1
M5 capabilities: ENERGY no longer required *** SURVIVED ***
M6b assemble ignores caps.adds, always adds both RED 10
M7 check_egress allows non-TLS *** SURVIVED *** (see below)
M8 userinfo split("@")[0] instead of [-1] RED 1
M9 DST end month October -> November RED (raises)
M10 strictly-increasing check dropped RED 2
M11 PluginOptions returns the framework option RED 3
M12 MAX_PLAUSIBLE 10 -> 10000 RED 1
M13b assemble inverted (caps.sources) RED 14
M14 REALTIME no longer wide RED 3
M15 product type check dropped RED 1
Three survivors, all one-line asserts:
M3 — _NoData.__init__(self, reason="no price data") goes green. This is the identical survivor I left open on RIG-01's _Unread.cause; the shape was borrowed including its blind spot. raises(lambda: _NoData()) closes it.
M5 — deleting the Component.ENERGY not in self.sources guard goes green. The consequence is real: adds becomes {ENERGY, TAXES}, assemble() never adds ENERGY, and the emitted price silently omits the energy component.
M7 — deleting the TLS refusal goes green and the check named for it still passes. check_egress("http://api.energy-charts.info/p", ...) is still refused, but by the host parse: url[len("https://"):] mangles an http:// URL into pi.energy-charts.info, which then fails the allow-list. So "a non-TLS price feed is refused" passes with the TLS guard deleted. That is an assertion that does not test what it names — this project's own recurring defect. Assert on the reason, or use a URL whose mangled host would still be declared.
Two corrections to my own numbers. Three of my first-pass mutations (M4, M6, M13) used wrong anchors — indentation, and the fact that the empty-series guard lives in _Series.__init__ rather than the staticmethod. They read as survivors and were not; re-run with correct anchors they are RED 1 / RED 10 / RED 14. Recording that rather than quietly dropping it.
4. Verified clean, independently
DST — the hand-rolled rule is right. My box has tzdata even though Alpine does not, so I cross-checked against the real thing:
149,040 hourly instants, 2020-2036 inclusive
mismatches vs zoneinfo Europe/Brussels: 0
rendered wall-time mismatches across every March/October hour 2026-2028: 0
2026-03-29 local 01:00+01:00 -> 2026-03-30 02:00+02:00 (23 h)
2026-10-25 local 02:00+02:00 -> 2026-10-26 01:00+01:00 (25 h)
Seventeen years, both transitions every year, zero disagreement. The "right for three years, wrong in the fourth" worry does not apply here. The residual is political rather than arithmetic: if the EU abolishes the change the rule needs editing — the same as tzdata needing an update, and worth one line saying so.
Degeneracy discipline, applied everywhere and not only where you noticed. I swept every fixture constant and every pairwise sum for near-collisions:
Section 0's four guards are the right ones, and the -95 fixture is genuinely negative all-in. Both self-caught issues are really fixed.
derived() cannot mask a failure._Raised is falsy, compares unequal to everything including itself, and arithmetic on it raises rather than defaulting. Every call site guards with isinstance(..., float) before arithmetic, so a raise cannot abort the file either.
CFG-03 not widened. No numeric literal anywhere in validate.py; no o.get("price_...", X) in main.py; aiohttp genuinely lazy — I re-imported app.price with aiohttp blocked at the import hook and it loaded clean, with 'aiohttp' in sys.modules False afterwards. AC3's offline claim is real.
5. The HA overlap — I agree with keeping and refusing
Retiring the entities now would remove the maintenance cycle's only price signal from a live install before anything replaces it. Allowing both is the worst of the three, because the provider decides nothing while the status page names it — the operator's evidence and the operator's reality come apart, which is this project's signature defect moved to the config layer. Refusal is loud, reversible, names the winner, and gives both remedies. Right call.
One thing to carry forward, and it is the same shape as TEST-06's boundary comment. The refusal's stated reason is "the HA entities win and the provider decides nothing". The moment TARIFF-03 or OPT-04 lands that stops being true — the provider series will decide something, and "both set" becomes a genuine two-writer conflict rather than a silently-ignored one. The refusal stays correct; its explanation expires. Whoever lands the first consumer has to update that message in the same commit, or a future reader trusts a reason that is no longer the reason.
6. The unwired seam — right call
main.py untouched, zero deleted lines in the entire diff, so the add-on regulating a real battery is byte-identical unless an operator opts in. For a P1 touching pricing that is the strongest safety property available, and wiring it now would put an unproven path into the 30 s tick with no consumer able to use it.
It is also not dead code: validate.py rule 7 imports PROVIDERS and sits on the startup path, so the registry is exercised on every boot. That is a real anchor against rot, and it is what makes "unread by anything" acceptable rather than speculative.
7. TARIFF-01
No branch is pushed, so I could not diff it. The collision surface is predictable and worth naming now: this PR appends to the tail of options:, the tail of schema:, the rule list inside validate_startup_config, and the tail of validate.py — exactly where a peak-tracker card will append too. Expect a textual conflict in both files, and expect it to be mechanical, since everything here is additive.
Order: TARIFF-02 first, once the item in §1 is fixed. It is finished and green; TARIFF-01 has no branch yet so it cannot be ready first, and holding a complete P1 for unpushed work is the wrong trade. TARIFF-01 then rebases by appending after this block in both files.
For the product owner
§20 open question #10 — which suppliers ship first — is raised in this PR and is not mine to settle. Flagging it upward rather than answering it.
What I could not verify
That any of the five shipped feed URLs or JSON shapes match what those suppliers actually publish today. Every fixture is synthetic and no check reaches the network, which is correct for CI but means the parse layer is verified against a belief about the feeds rather than against the feeds. Inherent to an offline suite, and worth stating rather than discovering when the first one returns 200 with a different key.
Fix the plugin-side scan and restate the AC1 guarantee, and I will merge on the next pass. Everything else here is good work — the absence type and the DST verification in particular are the strongest parts of this PR, and I could not break either.
**Not merging, narrowly — and the reason is the one thing you were asked to make true.** The code is correct, the absence type is excellent, the DST arithmetic is right, and I could not fault the config discipline. But **I can write a double-counting plugin four different ways**, one of them touching nothing but a public module constant. AC1's headline is "unreachable, not discouraged", and it is discouraged on one path of four.
The fix I am asking for is ~12 lines and I have already written and measured it. Everything else below is verification that came out clean.
## 1. Double-counting: reachable
Honest baseline for a wholesale plugin: energy 0.10 + markup + taxes = **0.188** (at your test constants).
| attack | what the plugin touches | all-in | verdict |
|---|---|---|---|
| A6 `opts.get("price_markup_eur_kwh")` | the documented path | — | **`FrameworkOption` — blocked ✅** |
| A1 `opts._o["price_markup_eur_kwh"]` | private attribute of `PluginOptions` | **0.219** | double-counted |
| A2 `price_option(o, "price_markup_eur_kwh")` | a **public** framework function | **0.219** | double-counted |
| **A2b `PRICE_OPTION_DEFAULTS["price_markup_eur_kwh"]`** | **a public dict, nothing else** | **0.22** vs 0.19 | double-counted |
| A3 all-in figure declared `sources={ENERGY}` | no framework option at all | **0.276** | double-counted |
| A4 energy-only figure declared all-in | no framework option at all | **0.100** | under-counted |
**A2b is the one that decides this.** The plugin reaches for no private attribute, needs no options dict, and calls no framework helper — it reads a module-level public constant in the module it must already import for `PriceProvider`, `RawPoint` and `register`. So this sentence, in the PR and in `PluginOptions`' docstring, is not true:
> *a plugin is never handed the markup... the plugin author who wants to add their supplier's margin a second time cannot write the line, because the number is not in scope.*
The number is in scope. `PluginOptions` closes one door in a room with three.
**What I am asking for — verified, not suggested.** You already wrote the negative source scan for AC2 (framework source must contain no plugin ids). Its mirror closes A1 and A2/A2b: plugin *code* must contain no framework-key names. Docstrings have to be stripped, or `engie_dynamic` false-positives on its own `SOURCES:` line — I hit that and fixed it by popping docstring nodes before `ast.unparse`:
```
--- shipped plugins (must be clean) ---
eneco_eddy clean
engie_dynamic clean <- clean only after stripping docstrings
generic_dayahead clean
manual_csv clean
totalenergies_dynamic clean
--- the reachable attacks (must be caught) ---
a1 ['_o', 'price_markup_eur_kwh']
a2 ['_o', 'price_markup_eur_kwh', 'price_option']
```
Banned set: `FRAMEWORK_KEYS` + `price_option` + `PRICE_OPTION_DEFAULTS` + `_o`.
**A3/A4 I am explicitly not asking you to close.** No code can verify whether a supplier's published figure already contains their margin — the declaration *is* the plugin's unverifiable claim about its source, and that is inherent, not sloppy. But it means the guarantee has a precise shape and the docstring should state it: **the framework adds the complement exactly once, given a correct `sources` declaration.** As written, the docstring-parse check (prose vs declaration) reads as if it verified more than it does: it verifies that two claims agree, not that either is true.
## 2. The absence type: the best version of this shape I have reviewed
Every mutation I carried over from TEST-04 and RIG-01:
```
Absence MRO: (Absence, Exception, BaseException) issubclass(Absence, AttributeError): False
getattr(nd,"points",None) -> RAISED hasattr(nd,"points") -> RAISED
nd.get("points",[]) -> RAISED list(nd) -> RAISED
nd.at(T0) -> RAISED nd.product -> RAISED
no_data("") / no_data(None)-> ValueError series([],...) -> ValueError
match missing either arm -> TypeError match positional -> TypeError
```
`Absence` not being an `AttributeError` is the property the whole shape rests on, and you kept it.
**The all-zero boundary holds exactly as §11.4 requires**, which is the thing I most expected to find wrong:
```
all-zero series .kind = series
arbitrage_allowed(all-zero) = True (pointless, but ALLOWED)
arbitrage_allowed(no_data) = False (FORBIDDEN)
plan_is_provisional(zero) = False
plan_is_provisional(no_data) = True
```
Two escapes, both the `vars()` class and neither accidental: `vars(nd).get("points",[])` returns `[]`, and `bool(nd)` is `True` — so `if read:` is true for both variants and reads like a check without being one. Worth a line in the docstring, not code.
## 3. Mutations — 15 run, 3 survive
```
M1 arbitrage veto removed (no_data -> True) RED 2
M2 plan_is_provisional inverted RED 1
M3 _NoData reason gets a DEFAULT *** SURVIVED ***
M4b _Series accepts an empty point list RED 1
M5 capabilities: ENERGY no longer required *** SURVIVED ***
M6b assemble ignores caps.adds, always adds both RED 10
M7 check_egress allows non-TLS *** SURVIVED *** (see below)
M8 userinfo split("@")[0] instead of [-1] RED 1
M9 DST end month October -> November RED (raises)
M10 strictly-increasing check dropped RED 2
M11 PluginOptions returns the framework option RED 3
M12 MAX_PLAUSIBLE 10 -> 10000 RED 1
M13b assemble inverted (caps.sources) RED 14
M14 REALTIME no longer wide RED 3
M15 product type check dropped RED 1
```
**Three survivors, all one-line asserts:**
- **M3** — `_NoData.__init__(self, reason="no price data")` goes green. This is the *identical* survivor I left open on RIG-01's `_Unread.cause`; the shape was borrowed including its blind spot. `raises(lambda: _NoData())` closes it.
- **M5** — deleting the `Component.ENERGY not in self.sources` guard goes green. The consequence is real: `adds` becomes `{ENERGY, TAXES}`, `assemble()` never adds ENERGY, and the emitted price silently omits the energy component.
- **M7** — deleting the TLS refusal goes green **and the check named for it still passes**. `check_egress("http://api.energy-charts.info/p", ...)` is still refused, but by the host parse: `url[len("https://"):]` mangles an `http://` URL into `pi.energy-charts.info`, which then fails the allow-list. So *"a non-TLS price feed is refused"* passes with the TLS guard deleted. That is an assertion that does not test what it names — this project's own recurring defect. Assert on the reason, or use a URL whose mangled host would still be declared.
**Two corrections to my own numbers.** Three of my first-pass mutations (`M4`, `M6`, `M13`) used wrong anchors — indentation, and the fact that the empty-series guard lives in `_Series.__init__` rather than the staticmethod. They read as survivors and were not; re-run with correct anchors they are RED 1 / RED 10 / RED 14. Recording that rather than quietly dropping it.
## 4. Verified clean, independently
**DST — the hand-rolled rule is right.** My box has `tzdata` even though Alpine does not, so I cross-checked against the real thing:
```
149,040 hourly instants, 2020-2036 inclusive
mismatches vs zoneinfo Europe/Brussels: 0
rendered wall-time mismatches across every March/October hour 2026-2028: 0
2026-03-29 local 01:00+01:00 -> 2026-03-30 02:00+02:00 (23 h)
2026-10-25 local 02:00+02:00 -> 2026-10-26 01:00+01:00 (25 h)
```
Seventeen years, both transitions every year, zero disagreement. The "right for three years, wrong in the fourth" worry does not apply here. The residual is political rather than arithmetic: if the EU abolishes the change the rule needs editing — the same as `tzdata` needing an update, and worth one line saying so.
**Degeneracy discipline, applied everywhere and not only where you noticed.** I swept every fixture constant and every pairwise sum for near-collisions:
```
MARKUP 0.031 TAXES 0.057 M+T 0.088 2M 0.062 2T 0.114 M+2T 0.145
EPEX/1000 [0.0614, -0.095, 0.183] RETAIL [0.1341, 0.0872, 0.261]
near-collisions (|a-b| < 1e-6): none
```
Section 0's four guards are the right ones, and the -95 fixture is genuinely negative all-in. Both self-caught issues are really fixed.
**`derived()` cannot mask a failure.** `_Raised` is falsy, compares unequal to everything *including itself*, and arithmetic on it raises rather than defaulting. Every call site guards with `isinstance(..., float)` before arithmetic, so a raise cannot abort the file either.
**CFG-03 not widened.** No numeric literal anywhere in `validate.py`; no `o.get("price_...", X)` in `main.py`; `aiohttp` genuinely lazy — I re-imported `app.price` with `aiohttp` blocked at the import hook and it loaded clean, with `'aiohttp' in sys.modules` False afterwards. AC3's offline claim is real.
## 5. The HA overlap — I agree with keeping and refusing
Retiring the entities now would remove the maintenance cycle's only price signal from a live install before anything replaces it. Allowing both is the worst of the three, because the provider decides nothing while the status page names it — the operator's evidence and the operator's reality come apart, which is this project's signature defect moved to the config layer. Refusal is loud, reversible, names the winner, and gives both remedies. Right call.
**One thing to carry forward, and it is the same shape as TEST-06's boundary comment.** The refusal's *stated reason* is "the HA entities win and the provider decides nothing". The moment TARIFF-03 or OPT-04 lands that stops being true — the provider series will decide something, and "both set" becomes a genuine two-writer conflict rather than a silently-ignored one. The refusal stays correct; its explanation expires. Whoever lands the first consumer has to update that message in the same commit, or a future reader trusts a reason that is no longer the reason.
## 6. The unwired seam — right call
`main.py` untouched, **zero deleted lines in the entire diff**, so the add-on regulating a real battery is byte-identical unless an operator opts in. For a P1 touching pricing that is the strongest safety property available, and wiring it now would put an unproven path into the 30 s tick with no consumer able to use it.
It is also not dead code: `validate.py` rule 7 imports `PROVIDERS` and sits on the startup path, so the registry is exercised on every boot. That is a real anchor against rot, and it is what makes "unread by anything" acceptable rather than speculative.
## 7. TARIFF-01
No branch is pushed, so I could not diff it. The collision surface is predictable and worth naming now: this PR appends to **the tail of `options:`, the tail of `schema:`, the rule list inside `validate_startup_config`, and the tail of `validate.py`** — exactly where a peak-tracker card will append too. Expect a textual conflict in both files, and expect it to be mechanical, since everything here is additive.
**Order: TARIFF-02 first**, once the item in §1 is fixed. It is finished and green; TARIFF-01 has no branch yet so it cannot be ready first, and holding a complete P1 for unpushed work is the wrong trade. TARIFF-01 then rebases by appending after this block in both files.
## For the product owner
§20 open question #10 — which suppliers ship first — is raised in this PR and is not mine to settle. Flagging it upward rather than answering it.
## What I could not verify
That any of the five shipped feed URLs or JSON shapes match what those suppliers actually publish today. Every fixture is synthetic and no check reaches the network, which is correct for CI but means the parse layer is verified against a belief about the feeds rather than against the feeds. Inherent to an offline suite, and worth stating rather than discovering when the first one returns 200 with a different key.
**Fix the plugin-side scan and restate the AC1 guarantee, and I will merge on the next pass.** Everything else here is good work — the absence type and the DST verification in particular are the strongest parts of this PR, and I could not break either.
Review of PR #16 wrote a double-counting plugin four ways. PluginOptions
blocked one. The one that decided it touched nothing but
PRICE_OPTION_DEFAULTS - a public dict in the module every plugin must
already import for register() - so "the number is not in scope" was
simply false, and an absolute guarantee a reviewer trusts is worse than
a stated limit, because they stop checking.
register() now refuses, AT IMPORT, any plugin whose own code mentions a
framework-owned name: FRAMEWORK_KEYS plus PRICE_OPTION_DEFAULTS,
price_option and _o. code_leaks() parses the class and drops every
docstring node before searching, because every plugin is REQUIRED to
name its sourced components in prose - engie_dynamic says "markup" in
its docstring and a raw text scan false-positives on the documentation
the same file mandates. A check that fires on required documentation is
a check somebody deletes. test_price.py asserts that a raw scan really
would have false-positived, so the strip is proven load-bearing rather
than asserted.
It fails OPEN if inspect.getsource() cannot find the .py, which the
add-on image always can. Refusing to start a battery controller because
a static analysis could not run would trade a real outage for a
hypothetical accounting error - so instead the checks assert source was
found for all five shipped plugins, and a build that lost it is a red
check rather than a quietly disarmed guard.
All three reachable attacks are written out as plugins in the suite and
each must be refused at registration. The cost is measured, not
asserted: honest wholesale 0.188, every attack 0.219 - one markup high,
every hour, invisible in every log this add-on writes.
The guarantee is now stated at its true size everywhere it appears: the
framework adds the complement exactly once, GIVEN A CORRECT sources
DECLARATION. A plugin that declares {ENERGY} while its feed is all-in
still doubles while touching no framework name, and no code can catch
that - the declaration is an unverifiable claim about somebody else's
tariff. Measured in the suite (0.276) so the limit is a number rather
than a caveat. The docstring/declaration check verifies that a plugin's
two claims AGREE, not that either is true; that now says so.
The three surviving mutations:
M3 - _NoData's reason taking a default. This is the identical survivor
left open on RIG-01's _Unread.cause: the shape was borrowed from
sim/riglock.py including its blind spot, which is what borrowing a shape
does if nobody re-runs the mutations against the copy. Closed by
SIGNATURE - no constructor or handler of an outcome type may carry a
default - so the next one is red without anyone writing a new case.
M5 - the ENERGY-required guard was untested. Its consequence is that
adds becomes {ENERGY, TAXES} and the emitted price silently omits the
energy it is pricing.
M7 - deleting the TLS refusal went green AND the check named for it
still passed, because the old parser stripped a fixed len("https://")
and mangled http://api.energy-charts.info/p into the host
pi.energy-charts.info, which then failed the ALLOW-LIST. An assertion
that does not test what it names, inside a security check. Fixed on both
sides: the scheme is parsed as a scheme, so an http URL now resolves to
its real and declared host and only the TLS rule can refuse it; and the
check asserts the REASON. Fixing it surfaced M7b - the no-scheme guard
was survivable the same way - found by mutating my own fix, and closed
the same way.
Also carried forward: rule 7's refusal is correct forever but its stated
reason ("the provider decides nothing") expires the moment TARIFF-03 or
OPT-04 lands. test_price.py section 12 now greps app/ by AST for a
load_prices consumer and goes red when one appears, telling whoever
lands it to update the message in the same commit. The first draft of
that tripwire used a substring and matched the comment in validate.py
explaining the tripwire - the same false-positive class as the plugin
docstrings, hit twice in one ticket, and fixed the same way.
Documented rather than coded around: vars(read).get("points", []) and
bool(read) both still get past the absence type, and neither is worth
code - defining __bool__ would turn a line that reads like a guard into
one that IS a guard by accident, which is how .get(k, default) got here
in the first place.
Stated plainly in DOCS/CHANGELOG/PR: every fixture is synthetic, so the
five feed URLs and JSON shapes are verified against a belief about what
those suppliers publish rather than against the feeds. A wrong shape
degrades to no-price-data, not to a wrong price.
Review's own DST verification recorded: 149,040 hourly instants,
2020-2036, zero mismatches against zoneinfo. The residual is political,
not arithmetic, and the docstring now says so.
24 mutations driven through the suite, 0 survivors. Whole add-on suite
green. No deleted lines outside this ticket's own files; main.py still
untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
Blocker fixed, three survivors closed, and the guarantee restated at its true size. Pushed as e102cde — one push, no force, main.py still untouched.
You were right that this was the one thing I was asked to make true, and A2b is the row that shows I hadn't. PluginOptions was one door; I described it as the room.
1. The blocker — register() now refuses at import
I built the mirror scan rather than pasting yours, and it found the same false-positive you hit.
code_leaks(cls) parses the class, drops every docstring node, ast.unparses what's left, and searches for FRAMEWORK_KEYS ∪ {PRICE_OPTION_DEFAULTS, price_option, _o}. register() raises FrameworkOption on a non-empty result — at import, not at fetch, so a plugin that can reach the markup does not get to exist.
the banned set covers the framework keys AND the three public routes review actually used
['PRICE_OPTION_DEFAULTS', '_o', 'export_price_eur_kwh', 'price_markup_eur_kwh', 'price_option', 'price_taxes_eur_kwh']
the scan really read every shipped plugin's source, so its fail-open path is not silently in use
{'generic_dayahead': 1093, 'engie_dynamic': 1112, 'totalenergies_dynamic': 939, 'eneco_eddy': 1448, 'manual_csv': 2957}
...and all five shipped plugins are clean {}
a RAW text scan would false-positive on the mandated docstrings ['engie_dynamic'],
which is why code_leaks() strips them before searching
A1 private dict: the scan names what it reached for ['_o', 'price_markup_eur_kwh']
A2 public helper: ['_o', 'price_markup_eur_kwh', 'price_option']
A2b public constant: ['PRICE_OPTION_DEFAULTS', 'price_markup_eur_kwh']
...each REFUSED by register() at import, and never enters the registry
an unrefused attack would bill 0.219 against an honest 0.188 - one markup high, every hour, silently
a MIS-DECLARED all-in feed still doubles (0.276 vs 0.188) with no framework name touched
Three things I did beyond the scan itself, because each was a way for it to become theatre:
The docstring strip is proven, not asserted. A check asserts a raw text scan would have flagged engie_dynamic — so the strip is demonstrably load-bearing, and the reason it exists is in the check's own output rather than only in a comment.
code_leaks() fails open when inspect.getsource() can't find the .py. Refusing to start a battery controller because a static analysis couldn't run trades a real outage for a hypothetical accounting error. But a fail-open guard nobody checks is a disarmed guard, so CI asserts source was found for all five and is long enough to be the class — a build that loses source is a red check, not a silent hole.
Your A3/A4 stay open, and now say so in three places (module docstring, DOCS.md, CHANGELOG) with the shape you named: the complement is added exactly once, given a correct sources declaration. The mis-declaration cost is measured at 0.276 rather than hedged in prose, so the limit is a number. And the docstring/declaration check now states that it verifies two claims agree, not that either is true.
2. The three survivors
M3 — closed by signature, not by one call: no constructor or handler of an outcome type may carry a default (no_data, series, both __init__s, match). Any default added later is red without anyone remembering to write a case. Plus no_data(), _NoData(), _Series([...]) bare → TypeError, and match with positional handlers → TypeError.
You're right that this was inherited: I copied riglock's shape without re-running its mutations against the copy. This one is also still open on RIG-01 / TEST-07 — the signature-scan form ports over directly if that's useful there.
M5 — the guard is now tested four ways (no ENERGY, empty set, unknown component, string product), plus the consequence you named, derived across all five plugins: Component.ENERGY in caps.adds must be empty everywhere, because the framework must never be positioned to invent an energy price.
M7 — fixed on both sides, because the parser alone would have left the assertion still lying. check_egress now partitions the scheme properly, so http://api.energy-charts.info/p resolves to its real and declared host and only the TLS rule can refuse it; and the check asserts the reason ("non-TLS" in ...), with two preceding checks establishing that the host is on the allow-list and parses identically over http — which is what makes the third able to fail.
Fixing it surfaced M7b: dropping the if not sep guard was survivable exactly the same way (no scheme → the whole string becomes the "scheme" → TLS refuses it → the check named for the scheme guard passes). Found by mutating my own fix, closed the same way — assert on the reason.
3. Mutation battery: 24 driven, 0 survive
M3 _NoData reason gets a DEFAULT RED 2 NEW2 code_leaks forgets PRICE_OPTION_DEFAULTS RED 1
M3b PriceRead.no_data gets a DEFAULT RED 2 NEW3 code_leaks forgets the private _o route RED 1
M5 ENERGY no longer required RED 2 NEW4 code_leaks stops stripping docstrings RED
M7 check_egress allows non-TLS RED 2 NEW5 code_leaks fails open on every plugin RED 11
M7b no-scheme guard dropped RED 1 R1..R14 regression sweep all RED
NEW1 register() stops refusing leaky plugins RED 7
Two corrections of my own, in the same spirit as yours: three anchors missed on the first run (indentation — mine were 8 spaces against module-level functions) and read as survivors; re-run correctly they are RED 2 / RED 1 / RED 7. And M7b was a genuine survivor I introduced, not an anchor error — caught, then closed.
4. Carried forward: the expiring reason
Rule 7's refusal stays correct; its explanation expires when TARIFF-03 or OPT-04 lands. That's now a tripwire, not a hope: section 12 walks app/by AST for a load_prices consumer and goes red the moment one appears, with the message telling whoever lands it to update validate.py's wording in the same commit. There's a matching ⚠️ block at the refusal itself.
The first draft of that tripwire used a substring and matched the comment in validate.py explaining the tripwire — the same false-positive class as the plugin docstrings, hit twice in one ticket. AST both times.
5. Taken as read
vars() and bool() — documented on _NoData, not coded around. Defining __bool__ would turn a line that reads like a guard into one that is a guard by accident, which is how .get(k, default) got into this codebase to begin with.
DST — your 149,040-instant cross-check is recorded in the module docstring, with the residual named as political rather than arithmetic.
The synthetic-fixture limit is now in the PR body and CHANGELOG in my own words, as you asked. Worth adding: a wrong feed shape degrades to no-price-data, not to a wrong price — so the failure mode of my being wrong about a supplier is the §11.4 row, which is the one I'd choose.
TARIFF-01 collision surface — agreed and expected: tail of options:, tail of schema:, the rule list, tail of validate.py. All additive; mechanical rebase.
Whole add-on suite green locally including test_freeze (34 s) and test_loop (93 s).
**Blocker fixed, three survivors closed, and the guarantee restated at its true size.** Pushed as `e102cde` — one push, no force, `main.py` still untouched.
You were right that this was the one thing I was asked to make true, and A2b is the row that shows I hadn't. `PluginOptions` was one door; I described it as the room.
## 1. The blocker — `register()` now refuses at import
I built the mirror scan rather than pasting yours, and it found the same false-positive you hit.
`code_leaks(cls)` parses the class, drops every docstring node, `ast.unparse`s what's left, and searches for `FRAMEWORK_KEYS ∪ {PRICE_OPTION_DEFAULTS, price_option, _o}`. `register()` raises `FrameworkOption` on a non-empty result — **at import, not at fetch**, so a plugin that can reach the markup does not get to exist.
```
the banned set covers the framework keys AND the three public routes review actually used
['PRICE_OPTION_DEFAULTS', '_o', 'export_price_eur_kwh', 'price_markup_eur_kwh', 'price_option', 'price_taxes_eur_kwh']
the scan really read every shipped plugin's source, so its fail-open path is not silently in use
{'generic_dayahead': 1093, 'engie_dynamic': 1112, 'totalenergies_dynamic': 939, 'eneco_eddy': 1448, 'manual_csv': 2957}
...and all five shipped plugins are clean {}
a RAW text scan would false-positive on the mandated docstrings ['engie_dynamic'],
which is why code_leaks() strips them before searching
A1 private dict: the scan names what it reached for ['_o', 'price_markup_eur_kwh']
A2 public helper: ['_o', 'price_markup_eur_kwh', 'price_option']
A2b public constant: ['PRICE_OPTION_DEFAULTS', 'price_markup_eur_kwh']
...each REFUSED by register() at import, and never enters the registry
an unrefused attack would bill 0.219 against an honest 0.188 - one markup high, every hour, silently
a MIS-DECLARED all-in feed still doubles (0.276 vs 0.188) with no framework name touched
```
Three things I did beyond the scan itself, because each was a way for it to become theatre:
- **The docstring strip is proven, not asserted.** A check asserts a raw text scan *would* have flagged `engie_dynamic` — so the strip is demonstrably load-bearing, and the reason it exists is in the check's own output rather than only in a comment.
- **`code_leaks()` fails open** when `inspect.getsource()` can't find the `.py`. Refusing to start a battery controller because a static analysis couldn't run trades a real outage for a hypothetical accounting error. But a fail-open guard nobody checks is a disarmed guard, so CI asserts source was found for all five and is long enough to be the class — a build that loses source is a red check, not a silent hole.
- **Your A3/A4 stay open, and now say so in three places** (module docstring, DOCS.md, CHANGELOG) with the shape you named: *the complement is added exactly once, given a correct `sources` declaration.* The mis-declaration cost is **measured** at 0.276 rather than hedged in prose, so the limit is a number. And the docstring/declaration check now states that it verifies two claims agree, not that either is true.
## 2. The three survivors
**M3** — closed by **signature**, not by one call: no constructor or handler of an outcome type may carry a default (`no_data`, `series`, both `__init__`s, `match`). Any default added later is red without anyone remembering to write a case. Plus `no_data()`, `_NoData()`, `_Series([...])` bare → `TypeError`, and `match` with positional handlers → `TypeError`.
You're right that this was inherited: I copied `riglock`'s shape without re-running its mutations against the copy. **This one is also still open on RIG-01 / TEST-07** — the signature-scan form ports over directly if that's useful there.
**M5** — the guard is now tested four ways (no ENERGY, empty set, unknown component, string product), plus the consequence you named, derived across all five plugins: `Component.ENERGY in caps.adds` must be empty everywhere, because the framework must never be positioned to invent an energy price.
**M7** — fixed on both sides, because the parser alone would have left the assertion still lying. `check_egress` now partitions the scheme properly, so `http://api.energy-charts.info/p` resolves to its **real and declared** host and *only* the TLS rule can refuse it; and the check asserts the **reason** (`"non-TLS" in ...`), with two preceding checks establishing that the host is on the allow-list and parses identically over http — which is what makes the third able to fail.
Fixing it surfaced **M7b**: dropping the `if not sep` guard was survivable exactly the same way (no scheme → the whole string becomes the "scheme" → TLS refuses it → the check named for the scheme guard passes). Found by mutating my own fix, closed the same way — assert on the reason.
## 3. Mutation battery: 24 driven, 0 survive
```
M3 _NoData reason gets a DEFAULT RED 2 NEW2 code_leaks forgets PRICE_OPTION_DEFAULTS RED 1
M3b PriceRead.no_data gets a DEFAULT RED 2 NEW3 code_leaks forgets the private _o route RED 1
M5 ENERGY no longer required RED 2 NEW4 code_leaks stops stripping docstrings RED
M7 check_egress allows non-TLS RED 2 NEW5 code_leaks fails open on every plugin RED 11
M7b no-scheme guard dropped RED 1 R1..R14 regression sweep all RED
NEW1 register() stops refusing leaky plugins RED 7
```
Two corrections of my own, in the same spirit as yours: three anchors missed on the first run (indentation — mine were 8 spaces against module-level functions) and read as survivors; re-run correctly they are RED 2 / RED 1 / RED 7. And **M7b was a genuine survivor I introduced**, not an anchor error — caught, then closed.
## 4. Carried forward: the expiring reason
Rule 7's refusal stays correct; its explanation expires when TARIFF-03 or OPT-04 lands. That's now a tripwire, not a hope: section 12 walks `app/` **by AST** for a `load_prices` consumer and goes red the moment one appears, with the message telling whoever lands it to update `validate.py`'s wording in the same commit. There's a matching ⚠️ block at the refusal itself.
The first draft of that tripwire used a substring and matched the comment in `validate.py` explaining the tripwire — the same false-positive class as the plugin docstrings, hit twice in one ticket. AST both times.
## 5. Taken as read
- **`vars()` and `bool()`** — documented on `_NoData`, not coded around. Defining `__bool__` would turn a line that *reads* like a guard into one that *is* a guard by accident, which is how `.get(k, default)` got into this codebase to begin with.
- **DST** — your 149,040-instant cross-check is recorded in the module docstring, with the residual named as political rather than arithmetic.
- **The synthetic-fixture limit is now in the PR body and CHANGELOG in my own words**, as you asked. Worth adding: a wrong feed shape degrades to *no-price-data*, not to a wrong price — so the failure mode of my being wrong about a supplier is the §11.4 row, which is the one I'd choose.
- **TARIFF-01 collision surface** — agreed and expected: tail of `options:`, tail of `schema:`, the rule list, tail of `validate.py`. All additive; mechanical rebase.
Whole add-on suite green locally including `test_freeze` (34 s) and `test_loop` (93 s).
TARIFF-01 (#17) landed on release/1.0 first, so this branch was two
commits behind and Gitea reported the PR unmergeable. Integrating rather
than rebasing: the two reviewed commits stay addressable, and the branch
is protected against force-push anyway.
ONE CONFLICT, AND IT IS THE BORING HALF OF THE ONE REVIEW PREDICTED.
Review of #16 expected collisions in four places - the tail of
`options:`, the tail of `schema:`, the rule list inside
validate_startup_config(), and the tail of validate.py. None of them
happened: TARIFF-01 touched neither config.yaml nor validate.py. It
added app/tariff.py, test_tariff.py, two P1 scenario captures, and edits
to main.py and p1.py, none of which this branch goes near.
The only conflict was CHANGELOG.md, where both branches prepended a
block under the same `## Unreleased` heading. Both blocks kept, TARIFF-02
above TARIFF-01 since it lands second. Nothing was dropped or reworded on
either side, and the resolution adds no line of its own beyond the blank
one between them.
Checked rather than assumed after the merge:
- all ELEVEN test files pass, including TARIFF-01's own test_tariff.py
(which this branch had never run) and the 93 s test_loop.py;
- the 24-mutation battery still has 0 survivors;
- no semantic overlap in config: TARIFF-01's peak tracker reads the
peak_cap_w / peak_forecast_entity options that already existed, so
the price options added here neither shadow nor duplicate anything.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
Rebased onto the new release/1.0 — TARIFF-01 (#17) landed first, so this branch was two commits behind and Gitea had flipped to mergeable: false. Now 862f0fa, mergeable: true, diff back to 2750 insertions, 0 deletions.
Integrated rather than rebased: the two reviewed commits stay addressable at the SHAs you read, and the branch is force-push-protected anyway.
Your predicted collision surface did not materialise, and that is worth recording since it was a reasonable prediction. You expected conflicts in four places — tail of options:, tail of schema:, the rule list inside validate_startup_config(), and the tail of validate.py. TARIFF-01 touched neither config.yaml nor validate.py: it added app/tariff.py, test_tariff.py, two P1 scenario captures, and edits to main.py / p1.py, none of which this branch goes near. Its peak tracker reads the peak_cap_w / peak_forecast_entity options that already existed, so the price options added here neither shadow nor duplicate anything.
The single conflict was CHANGELOG.md, where both branches prepended under the same ## Unreleased. Both blocks kept verbatim, TARIFF-02 above TARIFF-01 since it lands second; the resolution adds nothing but the blank line between them.
Re-verified on the merged tree, not assumed:
all eleven test files pass, including TARIFF-01's test_tariff.py — which this branch had never run — and the 93 s test_loop.py;
the 24-mutation battery still has 0 survivors;
git diff --stat origin/release/1.0..HEAD is additive-only again, confirming the merge pulled nothing out.
Nothing in the review-response commit changed. Ready for another look whenever the check clears.
**Rebased onto the new `release/1.0` — TARIFF-01 (#17) landed first, so this branch was two commits behind and Gitea had flipped to `mergeable: false`.** Now `862f0fa`, `mergeable: true`, diff back to **2750 insertions, 0 deletions**.
Integrated rather than rebased: the two reviewed commits stay addressable at the SHAs you read, and the branch is force-push-protected anyway.
**Your predicted collision surface did not materialise, and that is worth recording since it was a reasonable prediction.** You expected conflicts in four places — tail of `options:`, tail of `schema:`, the rule list inside `validate_startup_config()`, and the tail of `validate.py`. TARIFF-01 touched **neither `config.yaml` nor `validate.py`**: it added `app/tariff.py`, `test_tariff.py`, two P1 scenario captures, and edits to `main.py` / `p1.py`, none of which this branch goes near. Its peak tracker reads the `peak_cap_w` / `peak_forecast_entity` options that already existed, so the price options added here neither shadow nor duplicate anything.
The single conflict was `CHANGELOG.md`, where both branches prepended under the same `## Unreleased`. Both blocks kept verbatim, TARIFF-02 above TARIFF-01 since it lands second; the resolution adds nothing but the blank line between them.
Re-verified on the merged tree, not assumed:
- **all eleven test files pass**, including TARIFF-01's `test_tariff.py` — which this branch had never run — and the 93 s `test_loop.py`;
- the **24-mutation battery still has 0 survivors**;
- `git diff --stat origin/release/1.0..HEAD` is additive-only again, confirming the merge pulled nothing out.
Nothing in the review-response commit changed. Ready for another look whenever the check clears.
admin
merged commit 91ea676af4 into release/1.02026-08-30 22:35:49 +02:00
Merged as 91ea676. The blocker is closed: all three routes I used are refused at import, my numbers reproduce exactly, and every survivor I left is gone. Three findings below are follow-ups, not objections — one of them has a three-line fix I verified.
The blocker, re-tested
E0 P.PRICE_OPTION_DEFAULTS["price_markup_eur_kwh"] REFUSED at import
M3 _NoData reason gets a DEFAULT RED 2 (closed by signature)
M5 capabilities: ENERGY no longer required RED 2 (closed four ways)
M7a TLS rule deleted RED 2 "refused BY THE TLS RULE"
M7b no-scheme guard deleted (yours) RED 1
M7c userinfo split("@")[0] RED 1
M7 is the one I most wanted to see, and the fix is right for the right reason: the assertions now name which rule refused, so "plain http is refused" can no longer pass because the host parser mangled the URL. You found M7b by mutating your own fix — that is the habit, not the check.
Fail-open, verified by sabotage rather than by reading. I patched inspect.getsource to raise for the five shipped classes and ran the suite:
FAIL the scan really read every shipped plugin's source, so its fail-open path
is not silently in use <raised OSError: source code not available>
Red, naming the exception, with derived() doing exactly its job — the file keeps running instead of aborting. A build that loses source is loud.
Python floor, checked independently.p1.py has nofrom __future__ import annotations and 33 PEP-604 unions in annotation position (telegram_ts: datetime | None is a dataclass field, evaluated at class creation), and main.py imports it. So ≥3.10 is already forced on the live startup path, comfortably above ast.unparse's 3.9. Your reasoning holds.
The merge carried only the base
Per-file diff-of-diffs, e102cde..862f0fa against 9aadf46..0dbd485:
IDENTICAL app/main.py app/p1.py app/tariff.py test_tariff.py both scenarios/*.gz
DIFFERS CHANGELOG.md <- the one file that conflicted
CHANGELOG lines removed vs e102cde: 0
CHANGELOG lines removed vs 0dbd485: 0
Both blocks kept verbatim, nothing dropped from either side. And against current release/1.0 the whole PR is 2750 insertions, 0 deletions, main.py untouched — the additive property survived the integration, which is what mattered most.
Three ways the scan can still be evaded
I could not get past it the way I got past PluginOptions, but the scan is a substring search over one class's unparsed source, and that has edges. Honest baseline 0.188:
route
result
E1
getattr(P, "PRICE_OPTION_" + "DEFAULTS"), key built from fragments
registered, 0.218
E2
the arithmetic in a module-level helper, called from fetch
registered, 0.218
E3
fetch() inherited from an unregistered base class
registered, 0.218
E1 is inherent. No static scan beats a determined author, and chasing it would be theatre.
E3 is worth three lines, and I verified them. Walk cls.__mro__ instead of the class alone, skipping PriceProvider and object:
shipped five under an MRO walk: all clean
E3 subclass, class-only scan : clean <-- evades
E3 subclass, MRO walk : ['PRICE_OPTION_DEFAULTS', 'price_markup_eur_kwh']
A shared base for two energy+markup suppliers is a refactor somebody will actually reach for, so this one is worth closing.
E2 is structural and I do not think you should chase it. A module-scoped scan is the obvious answer and it cannot work here: the shipped plugins live inprice.py, alongside PRICE_OPTION_DEFAULTS, so scanning the module would flag all five. The class scope is forced by the file layout, and that is worth a sentence in code_leaks() so the next person does not "fix" it into a false positive.
One wording change to go with it. DOCS says "the numbers are not reachable from plugin code", and test_price.py:448 still says "unreachable". E2 is plugin code. The true statement is narrower and still worth having: a plugin class cannot mention the framework-owned names, and a plugin that does fails to load. You sized the A3/A4 claim honestly in four places; this is the last one still slightly over.
None of this blocks, and I want to be clear why, because I bounced last time on what looks like the same thing. Last time the natural path was open — a public dict, reached the obvious way. Now every natural path is shut and what remains needs an author who is already trying to add the markup twice. That is strictly less reachable than the mis-declaration case (A3/A4) we both agreed is inherent, where the plugin needs no suspicious code at all — just a wrong sources line.
Verified clean, again
All eleven suites green on the merged tree and again from release/1.0 after merge, test_tariff.py included. Tree 6f46338d on release/1.0 is byte-identical to the branch tree I reviewed.
On porting the signature scan to RIG-01 / TEST-07
Yes — port it. I left that survivor open on RIG-01 (_Unread.cause gets a default, suite stays green) and your form is strictly better than the per-case assert I suggested: it catches the next outcome type without anyone writing a case, which is the whole complaint I had about that class of check.
Two conditions, and they are the coordinator's, which I agree with. Port the AST form, not a substring draft — you hit that false-positive class twice in this ticket alone (plugin docstrings, then the section-12 tripwire matching its own explanatory comment), and riglock.py is even more comment-dense than price.py. And re-run riglock's own mutations against the ported version rather than assuming a good pattern stays good in a new home; the thing that made this ticket work was that you mutated your own fix.
Good work. The absence type, the DST rule and the egress fix are all stronger than what they replaced, and section 4b now reads as a record of what was actually tried rather than a claim.
**Merged as `91ea676`.** The blocker is closed: all three routes I used are refused at import, my numbers reproduce exactly, and every survivor I left is gone. Three findings below are follow-ups, not objections — one of them has a three-line fix I verified.
## The blocker, re-tested
```
E0 P.PRICE_OPTION_DEFAULTS["price_markup_eur_kwh"] REFUSED at import
M3 _NoData reason gets a DEFAULT RED 2 (closed by signature)
M5 capabilities: ENERGY no longer required RED 2 (closed four ways)
M7a TLS rule deleted RED 2 "refused BY THE TLS RULE"
M7b no-scheme guard deleted (yours) RED 1
M7c userinfo split("@")[0] RED 1
```
M7 is the one I most wanted to see, and the fix is right for the right reason: the assertions now name *which rule* refused, so "plain http is refused" can no longer pass because the host parser mangled the URL. You found M7b by mutating your own fix — that is the habit, not the check.
**Fail-open, verified by sabotage rather than by reading.** I patched `inspect.getsource` to raise for the five shipped classes and ran the suite:
```
FAIL the scan really read every shipped plugin's source, so its fail-open path
is not silently in use <raised OSError: source code not available>
```
Red, naming the exception, with `derived()` doing exactly its job — the file keeps running instead of aborting. A build that loses source is loud.
**Python floor, checked independently.** `p1.py` has **no** `from __future__ import annotations` and **33** PEP-604 unions in annotation position (`telegram_ts: datetime | None` is a dataclass field, evaluated at class creation), and `main.py` imports it. So ≥3.10 is already forced on the live startup path, comfortably above `ast.unparse`'s 3.9. Your reasoning holds.
## The merge carried only the base
Per-file diff-of-diffs, `e102cde..862f0fa` against `9aadf46..0dbd485`:
```
IDENTICAL app/main.py app/p1.py app/tariff.py test_tariff.py both scenarios/*.gz
DIFFERS CHANGELOG.md <- the one file that conflicted
CHANGELOG lines removed vs e102cde: 0
CHANGELOG lines removed vs 0dbd485: 0
```
Both blocks kept verbatim, nothing dropped from either side. And against current `release/1.0` the whole PR is **2750 insertions, 0 deletions, `main.py` untouched** — the additive property survived the integration, which is what mattered most.
## Three ways the scan can still be evaded
I could not get past it the way I got past `PluginOptions`, but the scan is a substring search over one class's unparsed source, and that has edges. Honest baseline 0.188:
| | route | result |
|---|---|---|
| **E1** | `getattr(P, "PRICE_OPTION_" + "DEFAULTS")`, key built from fragments | registered, **0.218** |
| **E2** | the arithmetic in a module-level helper, called from `fetch` | registered, **0.218** |
| **E3** | `fetch()` inherited from an unregistered base class | registered, **0.218** |
**E1 is inherent.** No static scan beats a determined author, and chasing it would be theatre.
**E3 is worth three lines, and I verified them.** Walk `cls.__mro__` instead of the class alone, skipping `PriceProvider` and `object`:
```
shipped five under an MRO walk: all clean
E3 subclass, class-only scan : clean <-- evades
E3 subclass, MRO walk : ['PRICE_OPTION_DEFAULTS', 'price_markup_eur_kwh']
```
A shared base for two energy+markup suppliers is a refactor somebody will actually reach for, so this one is worth closing.
**E2 is structural and I do not think you should chase it.** A module-scoped scan is the obvious answer and it cannot work here: the shipped plugins live *in* `price.py`, alongside `PRICE_OPTION_DEFAULTS`, so scanning the module would flag all five. The class scope is forced by the file layout, and that is worth a sentence in `code_leaks()` so the next person does not "fix" it into a false positive.
**One wording change to go with it.** DOCS says *"the numbers are not reachable from plugin code"*, and `test_price.py:448` still says *"unreachable"*. E2 is plugin code. The true statement is narrower and still worth having: **a plugin class cannot mention the framework-owned names, and a plugin that does fails to load.** You sized the A3/A4 claim honestly in four places; this is the last one still slightly over.
None of this blocks, and I want to be clear why, because I bounced last time on what looks like the same thing. Last time the *natural* path was open — a public dict, reached the obvious way. Now every natural path is shut and what remains needs an author who is already trying to add the markup twice. That is strictly less reachable than the mis-declaration case (A3/A4) we both agreed is inherent, where the plugin needs no suspicious code at all — just a wrong `sources` line.
## Verified clean, again
All eleven suites green on the merged tree and again from `release/1.0` after merge, `test_tariff.py` included. Tree `6f46338d` on `release/1.0` is byte-identical to the branch tree I reviewed.
## On porting the signature scan to RIG-01 / TEST-07
**Yes — port it.** I left that survivor open on RIG-01 (`_Unread.cause` gets a default, suite stays green) and your form is strictly better than the per-case assert I suggested: it catches the *next* outcome type without anyone writing a case, which is the whole complaint I had about that class of check.
Two conditions, and they are the coordinator's, which I agree with. Port the **AST** form, not a substring draft — you hit that false-positive class twice in this ticket alone (plugin docstrings, then the section-12 tripwire matching its own explanatory comment), and `riglock.py` is even more comment-dense than `price.py`. And **re-run riglock's own mutations against the ported version** rather than assuming a good pattern stays good in a new home; the thing that made this ticket work was that you mutated your own fix.
Good work. The absence type, the DST rule and the egress fix are all stronger than what they replaced, and section 4b now reads as a record of what was actually tried rather than a claim.
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.
Closes TARIFF-02 (P1, 8 SP, §9.3).
Built in
goodwe_controller/app/, not at specs.md §2.2'sems/ingest/prices/, per the product owner's decision of 2026-08-30 on the card. The behaviour §9.3 / §8.3 / §11.4 specifies is unchanged and still authoritative; only the path moved.Nothing consumes a price yet, and that is the shape of the ticket. This is the seam — the five plugins, the canonical format, the config plumbing. The arbitrage strategy (OPT-04), the static fallback (TARIFF-03) and the peak tracker (TARIFF-01) are separate cards and none of them is touched here. Turning a provider on today fetches, validates and publishes prices and changes no charging decision.
What ships
goodwe_controller/app/price.py— the framework and five providers:generic_dayahead(wholesale EPEX BE),engie_dynamic,totalenergies_dynamic,eneco_eddy,manual_csv.goodwe_controller/test_price.py— 12 sections, ~130 runnable checks, no network.app/validate.pyrule 7,config.yamloptions + schema,DOCS.md,CHANGELOG.md.Every plugin emits exactly
{"prices": [{"start", "eur_per_kwh", "provisional"}]}— three keys,starttz-aware at the Brussels offset, EUR/kWh, all-in. The check derives the key set from the emitted objects rather than asserting a typed literal, so a fourth field would be red.The three traps the card named
1. Double-counting the markup is refused, not discouraged — given a correct declaration
A plugin does not return a price. It returns
RawPoints carrying the figure its source published, and declares incapabilities().sourceswhich of{ENERGY, MARKUP, TAXES}that figure already contains.assemble()— the framework, never a plugin — adds exactly the complement.Two mechanisms, because one was not enough — and review of the first push proved it.
PluginOptionsraisesFrameworkOptionon the framework-owned keys, which closes the documented path. Review then wrote a double-counting plugin three more ways, one of them touching nothing butPRICE_OPTION_DEFAULTS, a public dict in the module every plugin must already import forregister. One closed door in a room with three is not a guarantee.So
register()also refuses at import any plugin whose own code mentions a framework-owned name (FRAMEWORK_KEYSplusPRICE_OPTION_DEFAULTS,price_option,_o).code_leaks()parses the class and drops every docstring node before searching — every plugin is required to name its sourced components in prose, so a raw text scan false-positives onengie_dynamic's ownSOURCES:line, and a check that fires on required documentation is a check somebody deletes. All three attacks are written out as plugins in the suite; honest wholesale baseline 0.188, every attack 0.219.The precise shape of the guarantee, stated because an overclaim is worse than no claim: the framework adds the complement exactly once, given a correct
sourcesdeclaration. A plugin that declares{ENERGY}while its feed is really all-in still comes out one markup high while touching no framework name at all — measured at 0.276 in the suite — and no code can catch that, because the declaration is the plugin's unverifiable claim about somebody else's tariff. What is mechanised is reachability; what stays human is truthfulness.generic_dayaheadengie_dynamictotalenergies_dynamiceneco_eddymanual_csvEach plugin's docstring states this in two fixed lines, and the checks parse those lines out of the docstring and compare them against the declaration, which verifies that the plugin's two claims agree, not that either is true.
A plugin whose feed is already all-in logs that the configured markup is being ignored. Silently discarding an operator's number is its own defect.
2. Product type is declared, never inferred from a name
capabilities().productis aPriceProductenum;eneco_eddydeclaresREALTIME, andvolatility_regime()— the only place in shipped code that says which products are volatile — maps that to"wide". The regime rides on the series a consumer is handed.AC2 is checked mechanically two ways:
inspect.getsource()over the framework's own functions and classes, asserting the set of plugin ids appearing in them is empty. Ifassembleorload_pricesever grows a name comparison, it goes red.price.pyuntouched. That is AC2 as an executable statement rather than a claim.volatility_regime("eneco_eddy")raisesTypeError, so the AC2 violation cannot be written accidentally later.3. Absence of prices is a type
Following the rework
sim/riglock.pytook under TEST-04 this week — same_Outcomeshape, down to the fatal.get():PriceRead.series(...)refuses an empty point list. "I have no prices" cannot be constructed wearing the shape of an answer.PriceRead.no_data(reason)carries.reasonand nothing else. No.pointsto iterate, no.productto switch on, no.at(), and.get("points", [])raises.__iter__raises too, sofor p in read:cannot be a harmless empty loop..match(*, series, no_data)takes both handlers as required keyword arguments — a forgotten arm is aTypeErrorat the call site, not a fall-through.no_data's one field is the operator's only evidence.The distinction that matters: a series of all-zero prices is a
series, notno_data. Arbitrage is pointless under zero prices and forbidden under no data (§11.4), and an implementation that returns[]on a failed fetch cannot tell a caller which it is in. The checks drive both and assertarbitrage_allowed/plan_is_provisionaldisagree across them.Six site-level failure modes (unreachable source, non-JSON body, empty feed, unknown id, provider off, missing CSV) all return
no_data—load_prices()never raises into the tick. The one exception isFrameworkOption, which is re-raised: a plugin reaching for the markup is a code defect, and degrading it to "no price data" would hide exactly what trap 1 exists to catch.The HA-side overlap, named and decided
price_now_entity+price_avg_entityare the existing delegation of pricing to Home Assistant.main.pyreduces them to one boolean,cheap = now <= avg, read by nothing except the maintenance cycle's cheap-window charge floor. That is all they have ever done.Decision: kept, not retired, and made non-overlapping by refusal. Retiring them would take the maintenance cycle's only price signal away on a live install before TARIFF-03 or OPT-04 has landed to replace it. But
price_providerset alongside them is refused at startup (validate.pyrule 7) — because in that configuration the HA entities silently win, the provider series drives nothing, and the operator's only evidence that they picked a supplier is a status page saying they picked a supplier. That is case (a) in validate.py's own taxonomy, and it is the "silently double up" the PM warned about. Either alone still starts.Rule 7 also refuses an unknown
price_providerid (rather than defaulting tooff, per the §14 warning box) andmanual_csvwith noprice_csv_path.price_provider: "off"is not refused and the docstring says why it must never become so: §11.4 makes no-price-data a supported state and §9.2 puts a large share of the installed base on a fixed or static tariff.AC4 — injection priced separately
export_price_eur_kwh, a scalar, not a second series. Negative values accepted (Belgian injection has gone below zero). It is inFRAMEWORK_KEYS, so a consumption plugin cannot fold it into a consumption price, and a check asserts it appears in no plugin's emitted series.CFG-03 — how this avoids widening it
The add-on already carries option defaults in three places, held apart by a tripwire in
test_validate.py. This ticket adds one in-code mirror, not another three:PRICE_OPTION_DEFAULTS, and only there;validate.pyimports it rather than restating any number;main.pygets noo.get("price_...", X)fallbacks at all;test_price.pyreadsconfig.yamland asserts the shipped default equals the mirror key for key, that every key is in theschema:block, and — derived from the registry — that the schema'slist(...)of providers is exactly the registered plugins plusoff.config.yamlstays the source of truth. Same patterntest_validate.pyalready uses formax_wandmeter_freeze_s.Two design notes worth reviewing
No
zoneinfo. The shipped Alpine image installspy3-aiohttp py3-paho-mqttfrom apk and has notzdata—ZoneInfo("Europe/Brussels")raises there. Offsets come from the EU rule (CEST from 01:00 UTC on the last Sunday of March to 01:00 UTC on the last Sunday of October), every instant is carried as UTC and rendered at the offset that applies to it, andbrussels()refuses a naive datetime rather than assuming one. Fixed offsets have no fold, so the October hour that happens twice is two instants rendering as02:00+02:00and02:00+01:00.Egress is declared by the plugin and enforced. §16 allow-lists outbound;
check_egress()refuses non-TLS, refuses any host not incapabilities().egress_hosts, and is not fooled by auserinfo@prefix. An operator-suppliedprice_feed_urlpointing at another supplier's host is refused.manual_csvdeclares()and imports no HTTP stack at all —aiohttpis imported lazily insidehttps_get.Checks: run, then broken
python3 test_price.py— 1 s, no network. The whole add-on suite is green (test_freeze34 s,test_loop93 s as documented).Two standards this file holds itself to, since they will be checked:
Every verdict is derived. There is no
check("...", True)anywhere. Expected all-in figures are computed in the test from the fixture's energy component and the plugin's own declaration, by different arithmetic thanassemble()uses.manual_csv's expected prices and provisional flags are parsed back out of the CSV body rather than typed. Key sets, offset sets, regime maps and the schema's provider list are built from the objects. Aderived()helper turns a computation that raises into a_Raisedsentinel that equals nothing and prints the exception — explicitly not a default, so a mutation that makes a derivation impossible reports as a failed check naming what raised, instead of aborting the file before later sections run.No stimulus is graded where the quantity is degenerate, and this is the real hazard here:
markup 0.03/taxes 0.06— and 0.06 is exactly twice 0.03. Under that pair several markup confusions collapse into the same number. The arithmetic checks therefore use0.031/0.057, and section 0 asserts the non-degeneracy conditions as checks in their own right (both non-zero, unequal, and2·markup ≠ markup+taxes) plus the fact that the shipped pair is degenerate.plan_is_provisionalis checked against a fully-settled series too. An all-true or all-false fixture returns the same answer under any bug.Twenty-four mutations were driven through it after the review pass; all twenty-four go red. A selection:
assemble()ignorescaps.adds, always adds bothPriceRead.seriesbecomes constructible_NoDatagrowspoints = ()no_datahas no.points"match()lets theno_dataarm default_Outcome.get()returns the default again (the TEST-04 defect).get('points', [])… raises rather than returning the default"volatility_regimereturns"normal"alwaysPluginOptionsstops refusing framework keysconfig.yamldefault drifts from the mirrorNot in scope, deliberately
No optimizer, no arbitrage strategy, no peak tracker (OPT-04 / TARIFF-01 / TARIFF-03). No persistence of fetched series (§15) and no fetch scheduler — both belong with the consumer that needs them, and adding a scheduler nothing reads would be a background task on a live controller for no benefit. No
main.pywiring, so the running add-on's behaviour is byte-identical unless an operator setsprice_provider. No live system was written to, restarted or flashed.What is not verified, and cannot be by an offline suite
Every fixture is synthetic. The five feed URLs and JSON shapes are checked against a belief about what those suppliers publish, not against the feeds — no check reaches the network, which is correct for CI but means the parse layer is verified against my reading of each API rather than against the API. Expect the first live fetch of each supplier to need a shape fix.
The mitigating property, and the reason this is an acceptable limit rather than a hidden one: a wrong feed shape degrades to no-price-data, not to a wrong price.
_points_from_jsonraisesSourceUnavailableon a missing key, andload_pricesturns that intoPriceRead.no_data(reason)— so being wrong about a supplier lands on the §11.4 row (static profile, plan provisional, no arbitrage) rather than on a confidently mispriced day.manual_csvis unaffected: its format is defined by this repo, not by a third party, and it is fully exercised.Open question #10 (§20) still stands and is for the product owner: which dynamic suppliers are most common in the target base. The five here are the spec's list; the framework makes a sixth a class and a registry line.
Five provider plugins - generic_dayahead, engie_dynamic, totalenergies_dynamic, eneco_eddy, manual_csv - all emitting exactly {"prices": [{"start", "eur_per_kwh", "provisional"}]}, tz-aware at the Brussels offset, EUR/kWh, all-in. Nothing consumes it yet; the seam lands first, per the PM's ordering. It is built in goodwe_controller/app/ rather than at specs.md 2.2's ems/ingest/prices/, per the product owner's decision of 2026-08-30. The behaviour 9.3 / 8.3 / 11.4 specifies is unchanged. Three things shape the design, and each is the mechanism rather than a comment saying to be careful: Double-counting the markup is unreachable. A plugin does not return a price - it returns the figure its source published and declares which of {energy, markup, taxes} that figure already contains. assemble() adds the complement, once, in the framework. A plugin is never handed price_markup_eur_kwh: PluginOptions raises FrameworkOption on it, so the line that adds the margin a second time cannot be written. Engie and TotalEnergies publish a retail figure with their margin inside; adding the configured 3 c/kWh on top would have made every arbitrage decision wrong by that amount with nothing reporting it. The product type is declared, not inferred from the plugin's name. eneco_eddy is real-time/imbalance and gets 8.3's widened thresholds through capabilities().product and volatility_regime(), never through a name comparison - which is AC2. test_price.py asserts mechanically that no plugin id appears anywhere in the framework's code, and drives a sixth provider defined inside the test file to the right regime with price.py untouched. "No price data" is a type, not an empty list - the same rework sim/riglock.py took under TEST-04. A fetch returns PriceRead.series(...) or PriceRead.no_data(reason), and only the first carries fields. An empty series is not constructible; no_data has no .points, no .at(), and .get("points", []) raises rather than returning the default. A day of free electricity and a day with no prices are different instructions - arbitrage is pointless under one and forbidden under the other - and an implementation returning [] cannot tell them apart. Every site-level failure returns no_data; nothing raises into the control tick. Also: injection stays a separate scalar (export_price_eur_kwh, AC4), negative prices survive the boundary validator (17's regression case), egress is allow-listed against each plugin's own declaration (16), and no tzdata is used - the shipped Alpine image has none, so Brussels offsets come from the EU rule and instants are carried as UTC. validate.py grows rule 7: an unknown provider id, manual_csv with no path, and - the HA overlap the PM told us to decide about - price_provider set alongside price_now_entity/price_avg_entity, which is two price sources where the HA pair silently wins. The HA sensors are kept and still work alone; TARIFF-03 and OPT-04 are what replace them. CFG-03 is not widened: the price defaults have ONE in-code mirror, PRICE_OPTION_DEFAULTS, which validate.py imports rather than restating and which test_price.py pins to config.yaml by reading the file. test_price.py is runnable, assert-free in the house style, and every verdict it prints is derived. It grades the markup arithmetic with 0.031/0.057 rather than the shipped 0.03/0.06, because 0.06 is exactly twice 0.03 and a degenerate pair would make double-counting indistinguishable from correct addition - and it asserts that non-degeneracy as its first section. The DST checks run on the 23-hour and 25-hour days, not on a normal day where a fixed 24-slot implementation and a correct one agree. Thirteen mutations were driven through it; all thirteen go red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPCNot merging, narrowly — and the reason is the one thing you were asked to make true. The code is correct, the absence type is excellent, the DST arithmetic is right, and I could not fault the config discipline. But I can write a double-counting plugin four different ways, one of them touching nothing but a public module constant. AC1's headline is "unreachable, not discouraged", and it is discouraged on one path of four.
The fix I am asking for is ~12 lines and I have already written and measured it. Everything else below is verification that came out clean.
1. Double-counting: reachable
Honest baseline for a wholesale plugin: energy 0.10 + markup + taxes = 0.188 (at your test constants).
opts.get("price_markup_eur_kwh")FrameworkOption— blocked ✅opts._o["price_markup_eur_kwh"]PluginOptionsprice_option(o, "price_markup_eur_kwh")PRICE_OPTION_DEFAULTS["price_markup_eur_kwh"]sources={ENERGY}A2b is the one that decides this. The plugin reaches for no private attribute, needs no options dict, and calls no framework helper — it reads a module-level public constant in the module it must already import for
PriceProvider,RawPointandregister. So this sentence, in the PR and inPluginOptions' docstring, is not true:The number is in scope.
PluginOptionscloses one door in a room with three.What I am asking for — verified, not suggested. You already wrote the negative source scan for AC2 (framework source must contain no plugin ids). Its mirror closes A1 and A2/A2b: plugin code must contain no framework-key names. Docstrings have to be stripped, or
engie_dynamicfalse-positives on its ownSOURCES:line — I hit that and fixed it by popping docstring nodes beforeast.unparse:Banned set:
FRAMEWORK_KEYS+price_option+PRICE_OPTION_DEFAULTS+_o.A3/A4 I am explicitly not asking you to close. No code can verify whether a supplier's published figure already contains their margin — the declaration is the plugin's unverifiable claim about its source, and that is inherent, not sloppy. But it means the guarantee has a precise shape and the docstring should state it: the framework adds the complement exactly once, given a correct
sourcesdeclaration. As written, the docstring-parse check (prose vs declaration) reads as if it verified more than it does: it verifies that two claims agree, not that either is true.2. The absence type: the best version of this shape I have reviewed
Every mutation I carried over from TEST-04 and RIG-01:
Absencenot being anAttributeErroris the property the whole shape rests on, and you kept it.The all-zero boundary holds exactly as §11.4 requires, which is the thing I most expected to find wrong:
Two escapes, both the
vars()class and neither accidental:vars(nd).get("points",[])returns[], andbool(nd)isTrue— soif read:is true for both variants and reads like a check without being one. Worth a line in the docstring, not code.3. Mutations — 15 run, 3 survive
Three survivors, all one-line asserts:
_NoData.__init__(self, reason="no price data")goes green. This is the identical survivor I left open on RIG-01's_Unread.cause; the shape was borrowed including its blind spot.raises(lambda: _NoData())closes it.Component.ENERGY not in self.sourcesguard goes green. The consequence is real:addsbecomes{ENERGY, TAXES},assemble()never adds ENERGY, and the emitted price silently omits the energy component.check_egress("http://api.energy-charts.info/p", ...)is still refused, but by the host parse:url[len("https://"):]mangles anhttp://URL intopi.energy-charts.info, which then fails the allow-list. So "a non-TLS price feed is refused" passes with the TLS guard deleted. That is an assertion that does not test what it names — this project's own recurring defect. Assert on the reason, or use a URL whose mangled host would still be declared.Two corrections to my own numbers. Three of my first-pass mutations (
M4,M6,M13) used wrong anchors — indentation, and the fact that the empty-series guard lives in_Series.__init__rather than the staticmethod. They read as survivors and were not; re-run with correct anchors they are RED 1 / RED 10 / RED 14. Recording that rather than quietly dropping it.4. Verified clean, independently
DST — the hand-rolled rule is right. My box has
tzdataeven though Alpine does not, so I cross-checked against the real thing:Seventeen years, both transitions every year, zero disagreement. The "right for three years, wrong in the fourth" worry does not apply here. The residual is political rather than arithmetic: if the EU abolishes the change the rule needs editing — the same as
tzdataneeding an update, and worth one line saying so.Degeneracy discipline, applied everywhere and not only where you noticed. I swept every fixture constant and every pairwise sum for near-collisions:
Section 0's four guards are the right ones, and the -95 fixture is genuinely negative all-in. Both self-caught issues are really fixed.
derived()cannot mask a failure._Raisedis falsy, compares unequal to everything including itself, and arithmetic on it raises rather than defaulting. Every call site guards withisinstance(..., float)before arithmetic, so a raise cannot abort the file either.CFG-03 not widened. No numeric literal anywhere in
validate.py; noo.get("price_...", X)inmain.py;aiohttpgenuinely lazy — I re-importedapp.pricewithaiohttpblocked at the import hook and it loaded clean, with'aiohttp' in sys.modulesFalse afterwards. AC3's offline claim is real.5. The HA overlap — I agree with keeping and refusing
Retiring the entities now would remove the maintenance cycle's only price signal from a live install before anything replaces it. Allowing both is the worst of the three, because the provider decides nothing while the status page names it — the operator's evidence and the operator's reality come apart, which is this project's signature defect moved to the config layer. Refusal is loud, reversible, names the winner, and gives both remedies. Right call.
One thing to carry forward, and it is the same shape as TEST-06's boundary comment. The refusal's stated reason is "the HA entities win and the provider decides nothing". The moment TARIFF-03 or OPT-04 lands that stops being true — the provider series will decide something, and "both set" becomes a genuine two-writer conflict rather than a silently-ignored one. The refusal stays correct; its explanation expires. Whoever lands the first consumer has to update that message in the same commit, or a future reader trusts a reason that is no longer the reason.
6. The unwired seam — right call
main.pyuntouched, zero deleted lines in the entire diff, so the add-on regulating a real battery is byte-identical unless an operator opts in. For a P1 touching pricing that is the strongest safety property available, and wiring it now would put an unproven path into the 30 s tick with no consumer able to use it.It is also not dead code:
validate.pyrule 7 importsPROVIDERSand sits on the startup path, so the registry is exercised on every boot. That is a real anchor against rot, and it is what makes "unread by anything" acceptable rather than speculative.7. TARIFF-01
No branch is pushed, so I could not diff it. The collision surface is predictable and worth naming now: this PR appends to the tail of
options:, the tail ofschema:, the rule list insidevalidate_startup_config, and the tail ofvalidate.py— exactly where a peak-tracker card will append too. Expect a textual conflict in both files, and expect it to be mechanical, since everything here is additive.Order: TARIFF-02 first, once the item in §1 is fixed. It is finished and green; TARIFF-01 has no branch yet so it cannot be ready first, and holding a complete P1 for unpushed work is the wrong trade. TARIFF-01 then rebases by appending after this block in both files.
For the product owner
§20 open question #10 — which suppliers ship first — is raised in this PR and is not mine to settle. Flagging it upward rather than answering it.
What I could not verify
That any of the five shipped feed URLs or JSON shapes match what those suppliers actually publish today. Every fixture is synthetic and no check reaches the network, which is correct for CI but means the parse layer is verified against a belief about the feeds rather than against the feeds. Inherent to an offline suite, and worth stating rather than discovering when the first one returns 200 with a different key.
Fix the plugin-side scan and restate the AC1 guarantee, and I will merge on the next pass. Everything else here is good work — the absence type and the DST verification in particular are the strongest parts of this PR, and I could not break either.
Blocker fixed, three survivors closed, and the guarantee restated at its true size. Pushed as
e102cde— one push, no force,main.pystill untouched.You were right that this was the one thing I was asked to make true, and A2b is the row that shows I hadn't.
PluginOptionswas one door; I described it as the room.1. The blocker —
register()now refuses at importI built the mirror scan rather than pasting yours, and it found the same false-positive you hit.
code_leaks(cls)parses the class, drops every docstring node,ast.unparses what's left, and searches forFRAMEWORK_KEYS ∪ {PRICE_OPTION_DEFAULTS, price_option, _o}.register()raisesFrameworkOptionon a non-empty result — at import, not at fetch, so a plugin that can reach the markup does not get to exist.Three things I did beyond the scan itself, because each was a way for it to become theatre:
engie_dynamic— so the strip is demonstrably load-bearing, and the reason it exists is in the check's own output rather than only in a comment.code_leaks()fails open wheninspect.getsource()can't find the.py. Refusing to start a battery controller because a static analysis couldn't run trades a real outage for a hypothetical accounting error. But a fail-open guard nobody checks is a disarmed guard, so CI asserts source was found for all five and is long enough to be the class — a build that loses source is a red check, not a silent hole.sourcesdeclaration. The mis-declaration cost is measured at 0.276 rather than hedged in prose, so the limit is a number. And the docstring/declaration check now states that it verifies two claims agree, not that either is true.2. The three survivors
M3 — closed by signature, not by one call: no constructor or handler of an outcome type may carry a default (
no_data,series, both__init__s,match). Any default added later is red without anyone remembering to write a case. Plusno_data(),_NoData(),_Series([...])bare →TypeError, andmatchwith positional handlers →TypeError.You're right that this was inherited: I copied
riglock's shape without re-running its mutations against the copy. This one is also still open on RIG-01 / TEST-07 — the signature-scan form ports over directly if that's useful there.M5 — the guard is now tested four ways (no ENERGY, empty set, unknown component, string product), plus the consequence you named, derived across all five plugins:
Component.ENERGY in caps.addsmust be empty everywhere, because the framework must never be positioned to invent an energy price.M7 — fixed on both sides, because the parser alone would have left the assertion still lying.
check_egressnow partitions the scheme properly, sohttp://api.energy-charts.info/presolves to its real and declared host and only the TLS rule can refuse it; and the check asserts the reason ("non-TLS" in ...), with two preceding checks establishing that the host is on the allow-list and parses identically over http — which is what makes the third able to fail.Fixing it surfaced M7b: dropping the
if not sepguard was survivable exactly the same way (no scheme → the whole string becomes the "scheme" → TLS refuses it → the check named for the scheme guard passes). Found by mutating my own fix, closed the same way — assert on the reason.3. Mutation battery: 24 driven, 0 survive
Two corrections of my own, in the same spirit as yours: three anchors missed on the first run (indentation — mine were 8 spaces against module-level functions) and read as survivors; re-run correctly they are RED 2 / RED 1 / RED 7. And M7b was a genuine survivor I introduced, not an anchor error — caught, then closed.
4. Carried forward: the expiring reason
Rule 7's refusal stays correct; its explanation expires when TARIFF-03 or OPT-04 lands. That's now a tripwire, not a hope: section 12 walks
app/by AST for aload_pricesconsumer and goes red the moment one appears, with the message telling whoever lands it to updatevalidate.py's wording in the same commit. There's a matching ⚠️ block at the refusal itself.The first draft of that tripwire used a substring and matched the comment in
validate.pyexplaining the tripwire — the same false-positive class as the plugin docstrings, hit twice in one ticket. AST both times.5. Taken as read
vars()andbool()— documented on_NoData, not coded around. Defining__bool__would turn a line that reads like a guard into one that is a guard by accident, which is how.get(k, default)got into this codebase to begin with.options:, tail ofschema:, the rule list, tail ofvalidate.py. All additive; mechanical rebase.Whole add-on suite green locally including
test_freeze(34 s) andtest_loop(93 s).Rebased onto the new
release/1.0— TARIFF-01 (#17) landed first, so this branch was two commits behind and Gitea had flipped tomergeable: false. Now862f0fa,mergeable: true, diff back to 2750 insertions, 0 deletions.Integrated rather than rebased: the two reviewed commits stay addressable at the SHAs you read, and the branch is force-push-protected anyway.
Your predicted collision surface did not materialise, and that is worth recording since it was a reasonable prediction. You expected conflicts in four places — tail of
options:, tail ofschema:, the rule list insidevalidate_startup_config(), and the tail ofvalidate.py. TARIFF-01 touched neitherconfig.yamlnorvalidate.py: it addedapp/tariff.py,test_tariff.py, two P1 scenario captures, and edits tomain.py/p1.py, none of which this branch goes near. Its peak tracker reads thepeak_cap_w/peak_forecast_entityoptions that already existed, so the price options added here neither shadow nor duplicate anything.The single conflict was
CHANGELOG.md, where both branches prepended under the same## Unreleased. Both blocks kept verbatim, TARIFF-02 above TARIFF-01 since it lands second; the resolution adds nothing but the blank line between them.Re-verified on the merged tree, not assumed:
test_tariff.py— which this branch had never run — and the 93 stest_loop.py;git diff --stat origin/release/1.0..HEADis additive-only again, confirming the merge pulled nothing out.Nothing in the review-response commit changed. Ready for another look whenever the check clears.
Merged as
91ea676. The blocker is closed: all three routes I used are refused at import, my numbers reproduce exactly, and every survivor I left is gone. Three findings below are follow-ups, not objections — one of them has a three-line fix I verified.The blocker, re-tested
M7 is the one I most wanted to see, and the fix is right for the right reason: the assertions now name which rule refused, so "plain http is refused" can no longer pass because the host parser mangled the URL. You found M7b by mutating your own fix — that is the habit, not the check.
Fail-open, verified by sabotage rather than by reading. I patched
inspect.getsourceto raise for the five shipped classes and ran the suite:Red, naming the exception, with
derived()doing exactly its job — the file keeps running instead of aborting. A build that loses source is loud.Python floor, checked independently.
p1.pyhas nofrom __future__ import annotationsand 33 PEP-604 unions in annotation position (telegram_ts: datetime | Noneis a dataclass field, evaluated at class creation), andmain.pyimports it. So ≥3.10 is already forced on the live startup path, comfortably aboveast.unparse's 3.9. Your reasoning holds.The merge carried only the base
Per-file diff-of-diffs,
e102cde..862f0faagainst9aadf46..0dbd485:Both blocks kept verbatim, nothing dropped from either side. And against current
release/1.0the whole PR is 2750 insertions, 0 deletions,main.pyuntouched — the additive property survived the integration, which is what mattered most.Three ways the scan can still be evaded
I could not get past it the way I got past
PluginOptions, but the scan is a substring search over one class's unparsed source, and that has edges. Honest baseline 0.188:getattr(P, "PRICE_OPTION_" + "DEFAULTS"), key built from fragmentsfetchfetch()inherited from an unregistered base classE1 is inherent. No static scan beats a determined author, and chasing it would be theatre.
E3 is worth three lines, and I verified them. Walk
cls.__mro__instead of the class alone, skippingPriceProviderandobject:A shared base for two energy+markup suppliers is a refactor somebody will actually reach for, so this one is worth closing.
E2 is structural and I do not think you should chase it. A module-scoped scan is the obvious answer and it cannot work here: the shipped plugins live in
price.py, alongsidePRICE_OPTION_DEFAULTS, so scanning the module would flag all five. The class scope is forced by the file layout, and that is worth a sentence incode_leaks()so the next person does not "fix" it into a false positive.One wording change to go with it. DOCS says "the numbers are not reachable from plugin code", and
test_price.py:448still says "unreachable". E2 is plugin code. The true statement is narrower and still worth having: a plugin class cannot mention the framework-owned names, and a plugin that does fails to load. You sized the A3/A4 claim honestly in four places; this is the last one still slightly over.None of this blocks, and I want to be clear why, because I bounced last time on what looks like the same thing. Last time the natural path was open — a public dict, reached the obvious way. Now every natural path is shut and what remains needs an author who is already trying to add the markup twice. That is strictly less reachable than the mis-declaration case (A3/A4) we both agreed is inherent, where the plugin needs no suspicious code at all — just a wrong
sourcesline.Verified clean, again
All eleven suites green on the merged tree and again from
release/1.0after merge,test_tariff.pyincluded. Tree6f46338donrelease/1.0is byte-identical to the branch tree I reviewed.On porting the signature scan to RIG-01 / TEST-07
Yes — port it. I left that survivor open on RIG-01 (
_Unread.causegets a default, suite stays green) and your form is strictly better than the per-case assert I suggested: it catches the next outcome type without anyone writing a case, which is the whole complaint I had about that class of check.Two conditions, and they are the coordinator's, which I agree with. Port the AST form, not a substring draft — you hit that false-positive class twice in this ticket alone (plugin docstrings, then the section-12 tripwire matching its own explanatory comment), and
riglock.pyis even more comment-dense thanprice.py. And re-run riglock's own mutations against the ported version rather than assuming a good pattern stays good in a new home; the thing that made this ticket work was that you mutated your own fix.Good work. The absence type, the DST rule and the egress fix are all stronger than what they replaced, and section 4b now reads as a record of what was actually tried rather than a claim.