PRICE-01: own price server as a price source, with fallbacks #190

Merged
admin merged 7 commits from PRICE-01 into release/1.0 2026-10-14 22:47:44 +02:00
Owner

PRICE-01: the owner's price server as a price source, with fallbacks

New provider price_provider: price_service. Full record: goodwe_controller/DECISIONS/PRICE-01-own-price-server.md.

What it does

  • Reads https://prices.kammenstraatha.duckdns.org/v1/dayahead/BE with a bearer token (new password option price_service_token). Host fixed in code, HTTPS only. Wholesale energy only, so markup and taxes are added as for generic_dayahead.
  • Chain: server, then energy-charts.info, then the prices the history store kept. A step counts only if it has a price for now. One log line per step down and per step up. With every source down a site plans from what it saved.
  • No price for now: one status row (Prices), one Home plan line and one notification (No prices), English and Dutch, cleared when a price returns.
  • With this provider the maintenance charge gate reads the provider's prices and keeps the Home Assistant rule: current 15-minute price at or below today's average (Brussels day). Moving a site from the HA pair to the provider does not change when the charge runs; test_price_service.py section 8 feeds the same days both ways.
  • The token is hidden on the settings page, refused when missing, and removed from any error text.

Unchanged: off, generic_dayahead, custom_formula, manual_csv, static_profile and the HA pair. The HA pair beside a provider is still refused. A scripted-day probe run against 0ce7d35 and this head differs in one line only (the refusal that lists the known providers now lists price_service).

Not done / open: no real answer from the server was seen (no token), so the tests use a mock; the first live capture belongs to the rig test. "No price" means none for now (not for the coming hours); see the decision record. No version bump or CHANGELOG entry (release step).

Tests: new test_price_service.py (mock server, failure matrix, 24 h outage, once-only notification, token secrecy, gate equivalence, real transport against a local server). Full suite (76 files) green locally in goodwe-addon-checks:1 with --network none. 25 deliberate breakages, all caught.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC

## PRICE-01: the owner's price server as a price source, with fallbacks New provider `price_provider: price_service`. Full record: `goodwe_controller/DECISIONS/PRICE-01-own-price-server.md`. **What it does** - Reads `https://prices.kammenstraatha.duckdns.org/v1/dayahead/BE` with a bearer token (new password option `price_service_token`). Host fixed in code, HTTPS only. Wholesale energy only, so markup and taxes are added as for `generic_dayahead`. - Chain: server, then energy-charts.info, then the prices the history store kept. A step counts only if it has a price for now. One log line per step down and per step up. With every source down a site plans from what it saved. - No price for now: one status row (*Prices*), one Home plan line and one notification (*No prices*), English and Dutch, cleared when a price returns. - With this provider the maintenance charge gate reads the provider's prices and keeps the Home Assistant rule: current 15-minute price at or below today's average (Brussels day). Moving a site from the HA pair to the provider does not change when the charge runs; `test_price_service.py` section 8 feeds the same days both ways. - The token is hidden on the settings page, refused when missing, and removed from any error text. **Unchanged**: `off`, `generic_dayahead`, `custom_formula`, `manual_csv`, `static_profile` and the HA pair. The HA pair beside a provider is still refused. A scripted-day probe run against 0ce7d35 and this head differs in one line only (the refusal that lists the known providers now lists `price_service`). **Not done / open**: no real answer from the server was seen (no token), so the tests use a mock; the first live capture belongs to the rig test. "No price" means none for *now* (not for the coming hours); see the decision record. No version bump or CHANGELOG entry (release step). **Tests**: new `test_price_service.py` (mock server, failure matrix, 24 h outage, once-only notification, token secrecy, gate equivalence, real transport against a local server). Full suite (76 files) green locally in `goodwe-addon-checks:1` with `--network none`. 25 deliberate breakages, all caught. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
admin added 1 commit 2026-10-14 16:22:36 +02:00
PRICE-01: price_service provider, fallback chain, and the gate read from it
checks / unit (pull_request) Successful in 39m31s
infra-pin / fresh (pull_request) Successful in 1s
9ac3a9d2bc
A new price source, `price_provider: price_service`: the owner's price server,
one fixed HTTPS host, a bearer token (new password option
`price_service_token`), wholesale energy only so markup and taxes are added as
for generic_dayahead.

The chain is server, then energy-charts, then the prices the history store
kept. A step counts only if it has a price for now. Each step down and back up
is one log line. With every source down a site plans from what it saved.

No price for now: one status row, one Home plan line and one notification, in
English and Dutch, cleared when a price returns. With this provider the
maintenance charge gate reads the provider's prices and keeps the Home
Assistant rule: the current 15-minute price is at or below today's average.

The token is hidden on the settings page, refused when missing, and taken out
of any error text. Other providers, `off` and the Home Assistant pair take the
road they always took (scripted-day probe against 0ce7d35: one line differs,
the refusal that lists the known providers). See
DECISIONS/PRICE-01-own-price-server.md.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
admin added 1 commit 2026-10-14 16:54:14 +02:00
PRICE-01: Dutch dates, no redirects with the token, more checks
checks / unit (pull_request) Successful in 39m2s
infra-pin / fresh (pull_request) Successful in 1s
87ad958842
- The notification, status row and Home line name an earlier day in the
  language of the text ("13 okt om 21:00"); the Dutch title is "Prijzen
  ontbreken", so the body no longer repeats it.
- The token request does not follow redirects: the host is fixed in code.
- A step back up to the saved prices is one INFO line.
- Tests: whole status payload free of the token, a redirect, the 5-decimal
  edge of the old average, modes plan equal to generic_dayahead's.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
admin added 1 commit 2026-10-14 17:03:07 +02:00
PRICE-01: migration note, and the tests drive the real control cycle
checks / unit (pull_request) Successful in 39m36s
infra-pin / fresh (pull_request) Successful in 1s
4eb88f7fda
- DOCS: the migration note said markup and taxes do not matter; with the
  provider they are added to the wholesale price. Step 4 and 5 now say what
  they set and what static_profile does while no source has a price.
- The gap tests run the notification through cycle(), where the check is
  called, so a check that is never called goes red.
- test header: the real-transport checks are section 11.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
Author
Owner

Review of 4eb88f7: not merged, one finding changes behaviour

I reviewed: 4eb88f7fdac6429ef2fd3ed7cdcf73dbda9983f0, with base release/1.0 at 0ce7d35. Both were checked with ls-remote.
CI: run 1276 checks / unit (pull_request) passed in 39m36s. Run 1277 infra-pin / fresh passed too.
Local run: all 76 test_*.py files pass in goodwe-addon-checks:1 (--network none, read-only mount, own uid). The run took 8m44s.

F1, medium, blocks the merge: the token shows in plain text on the add-on's settings pages

  • app/config_page.py:224-229: makeInput() draws every string field as <input type=text>. The Classic settings page (/config, linked from the settings view at static/index.html:120) therefore shows price_service_token unmasked. Neither uilayout.HIDDEN nor the schema's password type reaches this page.

  • app/web.py:234-236: GET /config/model puts the token in options, running (controller.o) and ui. A stale-save refusal also returns it, in stored (web.py:377).

  • My probe drove web.build_app with a stored price_service site: the token appears in options, running and ui, and the classic field model lists the token as a plain string.

  • So the DOCS line "never drawn on the add-on page" and the dev report's "token only on the Configuration tab" are not true for the classic page.

  • Suggested fix, for the dev:

    1. Mask secret options on the classic page (type=password, or "kept, not shown").
    2. Take secret values out of the /config/model and stale-refusal JSON.
    3. Make a save keep the stored token when the posted value is the placeholder or absent. The new page already does this: see below.
    4. Add a test that sends both pages a stored token and fails if it comes back.

    This changes behaviour, so I did not push it.

Low

  • F2: static_profile with price_service is accepted, but it no longer drives Cheap window or the maintenance gate (main.py:3449). Nothing tells the operator. That is the "silent loser" case TARIFF-10 §4(1) warns about. Fix: a config note, or one DOCS line.
  • F3: the "No prices" notification, the Prices row and the Home line say the maintenance cycle waits even when maintenance_enabled is false, which is the default. _provider_gate() and check_price_gap() never look at it. The owner should choose: say it only when maintenance is on, or use different words.
  • F4 (tests): two of my mutations survived.
    • R01: deleting here.provisional or in gate_for() is caught by no test. It is probably unreachable, because both parsers mark no point provisional.
    • R10: changing the plain https_get (no headers) is caught neither by the suite nor by the identity probe. This gap existed before this PR.
  • F5: scrub() leaves tokens shorter than 4 characters in place, and validate sets no minimum length. This is negligible.

Confirmed

  • Fallback chain: own server, then energy-charts, then saved prices. My probe compared 192 common quarters at each step: the prices are equal, so markup and taxes are added exactly once. The saved step ignores a later change of markup, as documented.
  • Gate: at every 5 minutes from 23:00 to 01:00 it matches the HA rule, and the average moves to the new day at 00:00. It also matches over 100 periods × 3 instants on 2026-10-25 and 92 × 3 on 2026-03-29. With tomorrow missing, it is shut at 00:05 and says since 00:00 Brussels time. The next refresh covers the new day.
  • Settings save: a site set to price_service on the Configuration tab, then edited on the new settings page, keeps its token. I checked this with and without base.
  • "Covers now" reading: sound. A look-ahead would alarm every morning before the ~13:00 auction. If the owner wants a "tomorrow still missing late in the day" warning, the price server already sends it to him.
  • Text: test_messages.py passes and every new string has EN and NL.

My mutations (none is in the dev's table)

R02 average over the UTC day: RED. R03 a step down filed as the own server: RED. R04 no Prices row: RED. R05 Dutch date in English: RED. R06 a token with spaces accepted: RED. R07 last_end_before strict: RED. R08 3xx read as a body: RED. R09 saved step without cover: RED. R11 scrub floor 64: RED. R01 and R10: GREEN (see F4).

I pushed nothing. Not merged: F1 needs a code change. It goes back to dev.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC

## Review of 4eb88f7: not merged, one finding changes behaviour **I reviewed:** `4eb88f7fdac6429ef2fd3ed7cdcf73dbda9983f0`, with base `release/1.0` at `0ce7d35`. Both were checked with ls-remote. **CI:** run 1276 `checks / unit (pull_request)` passed in 39m36s. Run 1277 `infra-pin / fresh` passed too. **Local run:** all 76 `test_*.py` files pass in `goodwe-addon-checks:1` (`--network none`, read-only mount, own uid). The run took 8m44s. ### F1, medium, blocks the merge: the token shows in plain text on the add-on's settings pages - `app/config_page.py:224-229`: `makeInput()` draws every string field as `<input type=text>`. The **Classic settings** page (`/config`, linked from the settings view at `static/index.html:120`) therefore shows `price_service_token` unmasked. Neither `uilayout.HIDDEN` nor the schema's `password` type reaches this page. - `app/web.py:234-236`: `GET /config/model` puts the token in `options`, `running` (`controller.o`) and `ui`. A stale-save refusal also returns it, in `stored` (`web.py:377`). - My probe drove `web.build_app` with a stored `price_service` site: the token appears in `options`, `running` and `ui`, and the classic field model lists the token as a plain `string`. - So the DOCS line "never drawn on the add-on page" and the dev report's "token only on the Configuration tab" are not true for the classic page. - Suggested fix, for the dev: 1. Mask secret options on the classic page (`type=password`, or "kept, not shown"). 2. Take secret values out of the `/config/model` and stale-refusal JSON. 3. Make a save keep the stored token when the posted value is the placeholder or absent. The new page already does this: see below. 4. Add a test that sends both pages a stored token and fails if it comes back. This changes behaviour, so I did not push it. ### Low - **F2:** `static_profile` with `price_service` is accepted, but it no longer drives *Cheap window* or the maintenance gate (`main.py:3449`). Nothing tells the operator. That is the "silent loser" case TARIFF-10 §4(1) warns about. Fix: a config note, or one DOCS line. - **F3:** the "No prices" notification, the *Prices* row and the Home line say the maintenance cycle waits even when `maintenance_enabled` is false, which is the default. `_provider_gate()` and `check_price_gap()` never look at it. The owner should choose: say it only when maintenance is on, or use different words. - **F4 (tests):** two of my mutations survived. - R01: deleting `here.provisional or` in `gate_for()` is caught by no test. It is probably unreachable, because both parsers mark no point provisional. - R10: changing the plain `https_get` (no headers) is caught neither by the suite nor by the identity probe. This gap existed before this PR. - **F5:** `scrub()` leaves tokens shorter than 4 characters in place, and validate sets no minimum length. This is negligible. ### Confirmed - **Fallback chain:** own server, then energy-charts, then saved prices. My probe compared 192 common quarters at each step: the prices are equal, so markup and taxes are added exactly once. The saved step ignores a later change of markup, as documented. - **Gate:** at every 5 minutes from 23:00 to 01:00 it matches the HA rule, and the average moves to the new day at 00:00. It also matches over 100 periods × 3 instants on 2026-10-25 and 92 × 3 on 2026-03-29. With tomorrow missing, it is shut at 00:05 and says since 00:00 Brussels time. The next refresh covers the new day. - **Settings save:** a site set to `price_service` on the Configuration tab, then edited on the new settings page, keeps its token. I checked this with and without `base`. - **"Covers now" reading:** sound. A look-ahead would alarm every morning before the ~13:00 auction. If the owner wants a "tomorrow still missing late in the day" warning, the price server already sends it to him. - **Text:** `test_messages.py` passes and every new string has EN and NL. ### My mutations (none is in the dev's table) R02 average over the UTC day: RED. R03 a step down filed as the own server: RED. R04 no *Prices* row: RED. R05 Dutch date in English: RED. R06 a token with spaces accepted: RED. R07 `last_end_before` strict: RED. R08 3xx read as a body: RED. R09 saved step without cover: RED. R11 scrub floor 64: RED. R01 and R10: GREEN (see F4). I pushed nothing. Not merged: F1 needs a code change. It goes back to dev. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
admin added 1 commit 2026-10-14 18:56:55 +02:00
PRICE-01 round 2: the token on no page, fixed-tariff note, token length, F3 words, test gaps
checks / unit (pull_request) Successful in 39m0s
infra-pin / fresh (pull_request) Successful in 2s
0801776645
F1: every password option is shown as ******** (set) or empty in /config/model
(options, running, ui) and in a stale refusal; the classic page draws it as a
password field; a save or validate that sends ******** or nothing keeps the
stored value. F2: a static_profile beside price_service is said once at start
(notification and log), not refused. F3 (owner, 2026-10-14): the no-price row,
Home line and notification only while maintenance_enabled is on. F4: tests for
the reviewer's R01 and R10. F5: price_service_token must be 16+ characters.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
admin added 1 commit 2026-10-14 19:58:53 +02:00
PRICE-01 review: spaced-token check passes the length rule; F3 default pinned
checks / unit (pull_request) Successful in 39m51s
infra-pin / fresh (pull_request) Successful in 1s
2f261f8ba4
The spaced-token check used a 5-character token, which the new 16-character
rule refuses on its own, so a validator that stopped checking spaces passed
(reviewer mutation R06). It now uses a long token and checks the reason.

Also pin F3 with no maintenance_enabled key: the default (off) holds and no
no-price words show (reviewer mutation S04). Tests only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
Author
Owner

Review round 2 (head 0801776): not merged

Verdict: F1, F3, F4 and F5 are fixed. The fix for F2 adds a new problem that needs a behaviour change (G1 below), so this goes back to dev. I pushed one tests-only commit, 2f261f8.

Round 1 findings

# Status
F1 token on the settings pages Fixed. My probe ran the real web app over a fake Supervisor with three schemas: format: password, type: password, and no marker. It covered 10 routes and 33 answers per schema: /, /status (en/nl), /config, /config/model (en/nl), /ha/entities, validate, a good save, a 422, a 409 and a re-save after the 409. No answer held the old or the new token. A placeholder, an empty field or a missing key keeps the stored token, and a new value replaces it. With nothing stored, a posted placeholder is saved as "", never as ********. A non-password option is not masked. With no schema marker, the token is still masked by name, but the classic page draws a text box showing ********. That is acceptable.
F2 fixed tariff beside price_service Fixed as briefed, but see G1.
F3 no-price words Fixed as the owner decided. They show only while maintenance_enabled is on, and a raised notification is cleared when it goes off. The wording is unchanged.
F4 R01 / R10 Fixed. Both mutations are now RED.
F5 token min length Fixed (16 characters). The refusal is English only. That is true of every refusal: the refusal strings exist only in validate.py, with no Dutch layer.

New findings

# Severity Where What
G1 medium, blocking app/messages.py fixed_tariff_unused, app/main.py check_fixed_unused, app/price_chain.py:53-57, DOCS Fixed tariff not used and migration step 5, DECISIONS rule 7 The Fixed tariff not used notification is wrong in three ways. (a) The text is false. static_profile is still used: it is the last fallback for the plan (static_tariff.resolve_tariff -> _degrade). DOCS migration step 5 and the option row ("Fill it in even on a dynamic contract, as the fallback") say so, and so does this PR's own test ("a static_profile beside it is accepted (it is the fallback for trading)"). It only no longer decides Cheap window or the maintenance gate. (b) No setup is quiet. The recommended setup gets an orange warning at every start. Following the DOCS advice to clear it removes the fallback, and then validate.py:1652-1660 warns that "static_profile is empty, so the site has no tariff at all while the price feed is down". (c) It is never taken down. After the operator clears the setting and restarts, the old notification stays, because there is no dismiss path. Suggested fix: no notification. Say it the way validate already gives config advice: one start log line plus a settings-page note (_advise(notes, "info", ("static_profile",), ...)), with true words, e.g. "With the price service, the fixed tariff is used only while no price source answers." Also dismiss goodwe_fixed_tariff_unused at start, in case an earlier version raised it. Then fix DOCS and DECISIONS to match. If the owner wants a notification after all, it must not fire on the recommended setup.
G2 low app/main.py:4037-4038 The F3 check reads self.o.get("maintenance_enabled") is True inline. CFG-03 (maintenance.py:175, main.py:607) says maintenance_* options are read only through MaintConfig.from_options(). The value is the same in practice. Use MaintConfig.from_options(self.o).enabled, or the controller's own MaintConfig, in the next round.
G3 low (tests) test_price_service.py Two reviewer mutations survived on 0801776. R06: the spaced-token check used "ab cd", which the new length rule now refuses on its own, so a validator that stopped checking spaces passed. S04: a missing maintenance_enabled key was not tested. Both are fixed by my commit 2f261f8 (tests only), and both mutations are now RED. S05 is still open: a whitespace-only static_profile raises the F2 notice, and no test catches it. Cover it in the G1 rework.

Tests

  • Full suite in goodwe-addon-checks:1 (--network none, own uid, read-only mount) on 0801776: 77/77 PASS.
  • CI on 0801776: run 1278 checks / unit success and run 1279 infra-pin / fresh success, both real runs (attempt 1).
  • Mutations: 11 from round 1 re-run, plus 7 new ones (S01-S07). 15 were RED and 3 GREEN (R06, S04, S05). After 2f261f8, R06 and S04 are RED. See G3.

Text (UX-08): the new strings pass test_messages.py (allowlist 0) and STYLE.md in form. G1 is about whether they are true, not their shape.

## Review round 2 (head 0801776): not merged **Verdict:** F1, F3, F4 and F5 are fixed. The fix for F2 adds a new problem that needs a behaviour change (G1 below), so this goes back to dev. I pushed one tests-only commit, `2f261f8`. ### Round 1 findings | # | Status | |---|---| | F1 token on the settings pages | **Fixed.** My probe ran the real web app over a fake Supervisor with three schemas: `format: password`, `type: password`, and no marker. It covered 10 routes and 33 answers per schema: `/`, `/status` (en/nl), `/config`, `/config/model` (en/nl), `/ha/entities`, validate, a good save, a 422, a 409 and a re-save after the 409. No answer held the old or the new token. A placeholder, an empty field or a missing key keeps the stored token, and a new value replaces it. With nothing stored, a posted placeholder is saved as `""`, never as `********`. A non-password option is not masked. With no schema marker, the token is still masked by name, but the classic page draws a text box showing `********`. That is acceptable. | | F2 fixed tariff beside `price_service` | **Fixed as briefed, but see G1.** | | F3 no-price words | **Fixed** as the owner decided. They show only while `maintenance_enabled` is on, and a raised notification is cleared when it goes off. The wording is unchanged. | | F4 R01 / R10 | **Fixed.** Both mutations are now RED. | | F5 token min length | **Fixed** (16 characters). The refusal is English only. That is true of every refusal: the refusal strings exist only in `validate.py`, with no Dutch layer. | ### New findings | # | Severity | Where | What | |---|---|---|---| | G1 | **medium, blocking** | `app/messages.py` `fixed_tariff_unused`, `app/main.py` `check_fixed_unused`, `app/price_chain.py:53-57`, DOCS *Fixed tariff not used* and migration step 5, DECISIONS rule 7 | The *Fixed tariff not used* notification is wrong in three ways. **(a) The text is false.** `static_profile` *is* still used: it is the last fallback for the plan (`static_tariff.resolve_tariff` -> `_degrade`). DOCS migration step 5 and the option row ("Fill it in even on a dynamic contract, as the fallback") say so, and so does this PR's own test ("a static_profile beside it is accepted (it is the fallback for trading)"). It only no longer decides *Cheap window* or the maintenance gate. **(b) No setup is quiet.** The recommended setup gets an orange warning at every start. Following the DOCS advice to clear it removes the fallback, and then `validate.py:1652-1660` warns that "static_profile is empty, so the site has no tariff at all while the price feed is down". **(c) It is never taken down.** After the operator clears the setting and restarts, the old notification stays, because there is no dismiss path. Suggested fix: no notification. Say it the way validate already gives config advice: one start log line plus a settings-page note (`_advise(notes, "info", ("static_profile",), ...)`), with true words, e.g. "With the price service, the fixed tariff is used only while no price source answers." Also dismiss `goodwe_fixed_tariff_unused` at start, in case an earlier version raised it. Then fix DOCS and DECISIONS to match. If the owner wants a notification after all, it must not fire on the recommended setup. | | G2 | low | `app/main.py:4037-4038` | The F3 check reads `self.o.get("maintenance_enabled") is True` inline. CFG-03 (`maintenance.py:175`, `main.py:607`) says `maintenance_*` options are read only through `MaintConfig.from_options()`. The value is the same in practice. Use `MaintConfig.from_options(self.o).enabled`, or the controller's own `MaintConfig`, in the next round. | | G3 | low (tests) | `test_price_service.py` | Two reviewer mutations survived on 0801776. **R06:** the spaced-token check used `"ab cd"`, which the new length rule now refuses on its own, so a validator that stopped checking spaces passed. **S04:** a missing `maintenance_enabled` key was not tested. **Both are fixed by my commit 2f261f8 (tests only), and both mutations are now RED.** **S05** is still open: a whitespace-only `static_profile` raises the F2 notice, and no test catches it. Cover it in the G1 rework. | ### Tests - Full suite in `goodwe-addon-checks:1` (`--network none`, own uid, read-only mount) on 0801776: **77/77 PASS**. - CI on 0801776: run 1278 `checks / unit` success and run 1279 `infra-pin / fresh` success, both real runs (attempt 1). - Mutations: 11 from round 1 re-run, plus 7 new ones (S01-S07). 15 were RED and 3 GREEN (R06, S04, S05). After 2f261f8, R06 and S04 are RED. See G3. Text (UX-08): the new strings pass `test_messages.py` (allowlist 0) and STYLE.md in form. G1 is about whether they are true, not their shape.
admin added 1 commit 2026-10-14 20:27:19 +02:00
PRICE-01 round 3: replace the fixed-tariff notification with a log line and a settings note
checks / unit (pull_request) Successful in 39m6s
infra-pin / fresh (pull_request) Successful in 1s
c68fd275cc
G1: remove the false 'Fixed tariff not used' notification. One INFO line at
start and an info note beside the setting (EN/NL) when price_service has a
non-blank static_profile; the old notification id is dismissed at start.
DOCS, migration step 5 and DECISIONS rule 7 now say what the code does.
S05: whitespace-only static_profile counts as not set.
G2: maintenance_enabled read through MaintConfig.from_options().

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
admin added 1 commit 2026-10-14 22:07:12 +02:00
PRICE-01 review 3: tests for the round 3 note and dismiss; doc wording
checks / unit (pull_request) Successful in 39m4s
infra-pin / fresh (pull_request) Successful in 2s
6862dea52a
- test_price_secret.py: the fixed-tariff note on the real /config/validate
  route, EN and NL, and none for a spaces-only profile (web.py lang wiring).
- test_price_service.py: the start line is INFO, not WARNING; a dismiss that
  raises does not stop the cycle.
- test_maint13/test_setup_only/test_watchdog: ignore only the dismiss of the
  old card, so a create with that id is still counted.
- DECISIONS rule 7 quotes the settings note as it is; messages.py comment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
Author
Owner

Review, round 3 (head c68fd27, plus my test commit 6862dea)

Verdict: right. I merge once checks / unit is green on 6862dea.

Round 2 findings

  • G1 fixed. The notification is gone.
    • The INFO line and the settings note appear only with price_service and a non-blank profile, once.
    • The recommended setup (price service, token, fixed tariff) gets no warning: no WARNING log line from validate, and no orange or red note. The validate.py:1653 warning shows only when the profile is cleared, and DOCS step 5 now says so.
    • DOCS step 5, the installer line and DECISIONS rule 7 match the code: the fixed tariff is the plan's last fallback, and _provider_gate reads only the provider's published series.
  • S05 fixed. A spaces-only profile gives no log line and no note. The S05 mutation is now caught.
  • G2 fixed. MaintConfig.from_options(self.o).enabled is used, and S04 is caught on the new line.

The start-up dismiss

I accept it in every mode.

  • Shadow: shadow mode already sends notification calls (check_price_gap and the others). It holds back control, not housekeeping, so a dismiss of a card id is not a command.
  • Setup-only: this mode builds no Controller, so check_fixed_note never runs, and the read-only hass.call refusal is never hit.
  • Can it fail a cycle? No. It runs once per process. hass.call deals with its own HTTP errors (10 s timeout), and the try/except Exception catches the rest. CancelledError still passes through.
  • The three test edits hid one thing: they left out any call with the old id. A create that raised the old card again would pass test_maint13, test_setup_only and test_watchdog (my mutation U05 left all three green). My commit narrows the filter to that one dismiss.

anchors(lang)

  • There are three callers: derive_messages and web.py's plain branch pass lang. web.py:408 (a refused save) passes no notes, so it needs none.
  • The real route shows the note in Dutch on a Dutch page (probe). No test covered that branch: mutation U01 was green. It is now in test_price_secret.py §5.

Pushed: 6862dea (tests and wording only)

  • test_price_secret.py §5: the note on the real /config/validate, in EN and NL, beside the same field, and none for a spaces-only profile.
  • test_price_service.py §14: the start line is INFO, not WARNING (U03 was green). A dismiss that raises does not stop the cycle (U04 was caught only by accident, through a fake with no call).
  • The three older tests now ignore only the dismiss of the old id.
  • DECISIONS rule 7 now quotes the settings note as it is. The comment in messages.py says the note has NL.

Checks

  • Full suite in goodwe-addon-checks:1: 77/77 PASS on c68fd27.
  • probe_settings2.py: no answer holds the token. The 5 FAIL lines are the same expected ones as in round 2.
  • mutate2.py: all 18 RED.
  • My U01–U07: all RED after 6862dea. U01, U03 and U05 were green on c68fd27.
## Review, round 3 (head c68fd27, plus my test commit 6862dea) **Verdict: right.** I merge once `checks / unit` is green on 6862dea. ### Round 2 findings - **G1 fixed.** The notification is gone. - The INFO line and the settings note appear only with `price_service` and a non-blank profile, once. - The recommended setup (price service, token, fixed tariff) gets no warning: no WARNING log line from validate, and no orange or red note. The `validate.py:1653` warning shows only when the profile is cleared, and DOCS step 5 now says so. - DOCS step 5, the installer line and DECISIONS rule 7 match the code: the fixed tariff is the plan's last fallback, and `_provider_gate` reads only the provider's `published` series. - **S05 fixed.** A spaces-only profile gives no log line and no note. The S05 mutation is now caught. - **G2 fixed.** `MaintConfig.from_options(self.o).enabled` is used, and S04 is caught on the new line. ### The start-up dismiss I accept it in every mode. - **Shadow:** shadow mode already sends notification calls (`check_price_gap` and the others). It holds back control, not housekeeping, so a dismiss of a card id is not a command. - **Setup-only:** this mode builds no Controller, so `check_fixed_note` never runs, and the read-only `hass.call` refusal is never hit. - **Can it fail a cycle?** No. It runs once per process. `hass.call` deals with its own HTTP errors (10 s timeout), and the `try/except Exception` catches the rest. `CancelledError` still passes through. - **The three test edits hid one thing:** they left out *any* call with the old id. A `create` that raised the old card again would pass `test_maint13`, `test_setup_only` and `test_watchdog` (my mutation U05 left all three green). My commit narrows the filter to that one `dismiss`. ### `anchors(lang)` - There are three callers: `derive_messages` and web.py's plain branch pass `lang`. web.py:408 (a refused save) passes no notes, so it needs none. - The real route shows the note in Dutch on a Dutch page (probe). No test covered that branch: mutation U01 was green. It is now in `test_price_secret.py` §5. ### Pushed: 6862dea (tests and wording only) - `test_price_secret.py` §5: the note on the real `/config/validate`, in EN and NL, beside the same field, and none for a spaces-only profile. - `test_price_service.py` §14: the start line is INFO, not WARNING (U03 was green). A dismiss that raises does not stop the cycle (U04 was caught only by accident, through a fake with no `call`). - The three older tests now ignore only the `dismiss` of the old id. - DECISIONS rule 7 now quotes the settings note as it is. The comment in `messages.py` says the note has NL. ### Checks - Full suite in `goodwe-addon-checks:1`: 77/77 PASS on c68fd27. - `probe_settings2.py`: no answer holds the token. The 5 FAIL lines are the same expected ones as in round 2. - `mutate2.py`: all 18 RED. - My U01–U07: all RED after 6862dea. U01, U03 and U05 were green on c68fd27.
admin merged commit bf9019e70d into release/1.0 2026-10-14 22:47:44 +02:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: admin/goodwe-addon#190