One bar per price step: 96 quarters for a full day (92/100 on clock-change days), 24 for hourly or static tariffs. No averaging; plan.average_tip removed (EN/NL). Cheap/dear marks are the kept priceruns cheap run and give runs (battery modes on) or the arbitrage verdict's two points (modes off), not a thirds split. A missing quarter is an empty bar. Display only; no control, option or version change.
One bar per price step: 96 quarters for a full day (92/100 on clock-change days), 24 for hourly or static tariffs. No averaging; `plan.average_tip` removed (EN/NL). Cheap/dear marks are the kept `priceruns` cheap run and give runs (battery modes on) or the arbitrage verdict's two points (modes off), not a thirds split. A missing quarter is an empty bar. Display only; no control, option or version change.
Report: /home/homelab/projects/goodwe/reports/UX-45-dev-report.md
🤖 Generated with [Claude Code](https://claude.ai/code)
https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
With 15-minute prices the chart now has one bar per quarter (92/96/100),
not hour averages; hourly and static tariffs still draw one bar per hour.
Cheap and dear marks come from the kept priceruns result (battery modes
on) or the arbitrage verdict's two points (off), not a thirds split.
A missing quarter is an empty bar. Removes plan.average_tip (EN/NL).
Co-Authored-By: Claude <noreply@anthropic.com>
Reviewed head 17c60484172c9f8af79468b02a29fcccbc0f8684 (ls-remote checked; base release/1.0 243a756, master ad374ae). CI run 1335 checks / unit and 1336 infra-pin / fresh are both success on this sha. Not merged, as the brief says: the owner still has to decide on day/night colours.
Findings
MEDIUM, blocking. The setup wizard now says "for 96 of 24 hours".goodwe_controller/app/web.py:329-331 counts plan.prices entries that have a price and passes the count to validate._price_check(), which prints f"Prices for today are in, for {prices_known} of 24 hours." (app/validate.py:2083). This PR makes the bars quarters, so a full day of 15-minute prices now reads "Prices for today are in, for 96 of 24 hours." I reproduced it with the same count web.py makes: 243a756 gives 24 of 24 hours and 17c6048 gives 96 of 24 hours. The grep the brief asked for missed this consumer, and no test covers the count against the real bars. Fix: count distinct hours (for example, the distinct UTC hour of start among priced bars), and add a check that 96 quarter prices give "24 of 24 hours". The day length is also a fixed "24" on 23 h and 25 h days, but that was already so on 243a756.
LOW. Holes of three quarters get filled in on the chart.homeview.py:170-171 (_bar_cover): gap in (step, hour) lets a point cover a gap of exactly one hour. In a 15-minute series with 10:15, 10:30 and 10:45 missing, the 10:00 price is drawn on all four quarters. With one or two quarters missing, the bars are left empty. So holes of 1-2 quarters are empty and holes of 3 are filled. The add-on itself (price._cover_table(), TARIFF-14) covers every gap up to an hour with the previous price. The chart therefore differs from what the add-on uses for 1-2 missing quarters (by the brief's choice) and matches it for 3. Pick one rule. Note that _bar_cover, _spans and _cover_table are now three copies of the cover rule.
LOW, for the owner question. Marks are the site's run plan, not what each battery does.homeview.py:221-227: with modes on, the cheap run is marked whenever charge_useful(), and every give run is marked "dear", even when no battery has a price mode or may charge from the grid. mode_runs() (homeview.py:394-396) draws blocks only for those batteries, and _price_runs() builds the plan whatever the modes are. In the dev's own quarter-* shots, both batteries "Follow the house" while 13:45-14:15 is marked low and about 15:00-24:00 is marked high.
LOW. Past marks are kept only if the page was open during that quarter.homeview.py:196-204: kept is written only while /status is built (page poll or setup wizard). If nobody has the page open during a run, that quarter shows as "mid" once it is over, and the same happens after a restart (the dev noted that). The memory is bounded (one entry per bar, reset on a new day in DayPrices.see), so growth and stale state across days are fine. But nothing tests the reset: mutation R3 (no reset) survives.
LOW, test gap. The day/night test only covers the moment before the first cycle.test_home.py:640-651 (c_dn) never stores a tariff plan, so the verdict is "no_prices" and "all mid" is expected. I ran it with the plan the cycle actually stores (static_plan from now, as resolve_tariff does with the provider off). With arbitrage on, the head marks 22:00 cheap and 11:00 dear (the verdict's two hours). With arbitrage off, it marks nothing. Please add that case, so the owner question is pinned down by a test.
LOW, a11y. At 390 px each quarter bar is about 2.4 px wide (.bars.fine, app.css:81-82). Keyboard roving works (unchanged code; Home/End), but tapping one quarter on a phone is not realistic. The chart was already below a 24 px target with 24 bars; it is now about 6x smaller. Optional: a wider tap target or snapping to the nearest bar.
NIT.price_bars scans the cover list once per bar (homeview.py:196, next(...) over cover): about 5 ms per build for 96 bars here, on every 3 s poll per viewer. That is fine, but a single pass would be easy to write.
"No trading on price yet: no prices have arrived." (daynight shots)
This was already on 243a756 and is not new. The dev's screenshot server (serve.py) never sets tariff_plan for the day/night controller, so fresh_tariff_plan() is None. trade_verdict() then returns "no_prices", and that line is the result. This is the state before the first price cycle. The same fixture gives the identical line on 243a756 (lines: ['No trading on price yet: no prices have arrived.'] at both commits). Once a static plan is stored, as in production, the line is gone at both commits. Neither trade_verdict nor the line wording changed in this PR. The no-colours look in those shots also comes from the fixture. In production with arbitrage on, the chart shows one cheap hour and one dear hour.
Checked and fine
96 / 92 / 100 bars; fall-back labels 02:45-02:00, 02:00-02:15; spring-forward 01:45-03:00; day end 23:45-00:00 (the same end label as before). Hourly series and static tariffs give 24 bars (25 on fall-back). One missing quarter gives one "none" bar and nothing shifts.
With modes off, the marks are exactly the verdict's two points that trade_runs() draws. With modes on, they are the kept priceruns cheap run (when charge_useful()) and the give runs, as kept by keep_started.
points / hour_bars / FLAT_EUR_KWH / average_tip have no other reader (JS, ui_text, tests, HA/MQTT). The only other consumer of plan.prices is the setup wizard count (finding 1).
No new strings; the removed plan.average_tip is gone from EN, NL and app.js.
Offline suite in goodwe-addon-checks:1 (--network none, GOODWE_REQUIRE_HWSIM=1, real hwsim at infra pin 3b4ff84): 79/79 PASS on 17c6048; test_freeze part 3 ran against the rig hwsim.
Mutations: the dev's M1, M2, M4, M5, M6 and M9 were re-run and all caught. Mine: hour averaging back (caught, 3 FAIL), thirds back (caught, 4 FAIL), end_label +1 h (caught), first give run only (caught). Survived: no day reset of kept (R3), .fine class dropped (R6, CSS only).
Verdict: needs changes (finding 1). Findings 2-5 are for the dev or the owner. The owner decision on day/night colours is still open.
## UX-45 review (round 1): needs changes
Reviewed head `17c60484172c9f8af79468b02a29fcccbc0f8684` (ls-remote checked; base release/1.0 243a756, master ad374ae). CI run 1335 `checks / unit` and 1336 `infra-pin / fresh` are both success on this sha. Not merged, as the brief says: the owner still has to decide on day/night colours.
### Findings
1. **MEDIUM, blocking. The setup wizard now says "for 96 of 24 hours".** `goodwe_controller/app/web.py:329-331` counts `plan.prices` entries that have a price and passes the count to `validate._price_check()`, which prints `f"Prices for today are in, for {prices_known} of 24 hours."` (`app/validate.py:2083`). This PR makes the bars quarters, so a full day of 15-minute prices now reads **"Prices for today are in, for 96 of 24 hours."** I reproduced it with the same count web.py makes: 243a756 gives `24 of 24 hours` and 17c6048 gives `96 of 24 hours`. The grep the brief asked for missed this consumer, and no test covers the count against the real bars. Fix: count distinct hours (for example, the distinct UTC hour of `start` among priced bars), and add a check that 96 quarter prices give "24 of 24 hours". The day length is also a fixed "24" on 23 h and 25 h days, but that was already so on 243a756.
2. **LOW. Holes of three quarters get filled in on the chart.** `homeview.py:170-171` (`_bar_cover`): `gap in (step, hour)` lets a point cover a gap of exactly one hour. In a 15-minute series with 10:15, 10:30 and 10:45 missing, the 10:00 price is drawn on all four quarters. With one or two quarters missing, the bars are left empty. So holes of 1-2 quarters are empty and holes of 3 are filled. The add-on itself (`price._cover_table()`, TARIFF-14) covers *every* gap up to an hour with the previous price. The chart therefore differs from what the add-on uses for 1-2 missing quarters (by the brief's choice) and matches it for 3. Pick one rule. Note that `_bar_cover`, `_spans` and `_cover_table` are now three copies of the cover rule.
3. **LOW, for the owner question. Marks are the site's run plan, not what each battery does.** `homeview.py:221-227`: with modes on, the cheap run is marked whenever `charge_useful()`, and every give run is marked "dear", even when no battery has a price mode or may charge from the grid. `mode_runs()` (`homeview.py:394-396`) draws blocks only for those batteries, and `_price_runs()` builds the plan whatever the modes are. In the dev's own `quarter-*` shots, both batteries "Follow the house" while 13:45-14:15 is marked low and about 15:00-24:00 is marked high.
4. **LOW. Past marks are kept only if the page was open during that quarter.** `homeview.py:196-204`: `kept` is written only while `/status` is built (page poll or setup wizard). If nobody has the page open during a run, that quarter shows as "mid" once it is over, and the same happens after a restart (the dev noted that). The memory is bounded (one entry per bar, reset on a new day in `DayPrices.see`), so growth and stale state across days are fine. But nothing tests the reset: mutation R3 (no reset) survives.
5. **LOW, test gap. The day/night test only covers the moment before the first cycle.** `test_home.py:640-651` (`c_dn`) never stores a tariff plan, so the verdict is "no_prices" and "all mid" is expected. I ran it with the plan the cycle actually stores (`static_plan` from now, as `resolve_tariff` does with the provider off). With arbitrage on, the head marks **22:00 cheap and 11:00 dear** (the verdict's two hours). With arbitrage off, it marks nothing. Please add that case, so the owner question is pinned down by a test.
6. **LOW, a11y.** At 390 px each quarter bar is about 2.4 px wide (`.bars.fine`, `app.css:81-82`). Keyboard roving works (unchanged code; Home/End), but tapping one quarter on a phone is not realistic. The chart was already below a 24 px target with 24 bars; it is now about 6x smaller. Optional: a wider tap target or snapping to the nearest bar.
7. **NIT.** `price_bars` scans the cover list once per bar (`homeview.py:196`, `next(...)` over `cover`): about 5 ms per build for 96 bars here, on every 3 s poll per viewer. That is fine, but a single pass would be easy to write.
### "No trading on price yet: no prices have arrived." (daynight shots)
This was **already on 243a756 and is not new.** The dev's screenshot server (`serve.py`) never sets `tariff_plan` for the day/night controller, so `fresh_tariff_plan()` is None. `trade_verdict()` then returns `"no_prices"`, and that line is the result. This is the state before the first price cycle. The same fixture gives the identical line on 243a756 (`lines: ['No trading on price yet: no prices have arrived.']` at both commits). Once a static plan is stored, as in production, the line is gone at both commits. Neither `trade_verdict` nor the line wording changed in this PR. The no-colours look in those shots also comes from the fixture. In production with arbitrage on, the chart shows one cheap hour and one dear hour.
### Checked and fine
- 96 / 92 / 100 bars; fall-back labels `02:45-02:00`, `02:00-02:15`; spring-forward `01:45-03:00`; day end `23:45-00:00` (the same end label as before). Hourly series and static tariffs give 24 bars (25 on fall-back). One missing quarter gives one "none" bar and nothing shifts.
- With modes off, the marks are exactly the verdict's two points that `trade_runs()` draws. With modes on, they are the kept `priceruns` cheap run (when `charge_useful()`) and the give runs, as kept by `keep_started`.
- `points` / `hour_bars` / `FLAT_EUR_KWH` / `average_tip` have no other reader (JS, ui_text, tests, HA/MQTT). The only other consumer of `plan.prices` is the setup wizard count (finding 1).
- No new strings; the removed `plan.average_tip` is gone from EN, NL and app.js.
- Offline suite in `goodwe-addon-checks:1` (`--network none`, `GOODWE_REQUIRE_HWSIM=1`, real hwsim at infra pin 3b4ff84): 79/79 PASS on 17c6048; test_freeze part 3 ran against the rig hwsim.
- Mutations: the dev's M1, M2, M4, M5, M6 and M9 were re-run and all caught. Mine: hour averaging back (caught, 3 FAIL), thirds back (caught, 4 FAIL), end_label +1 h (caught), first give run only (caught). Survived: no day reset of `kept` (R3), `.fine` class dropped (R6, CSS only).
**Verdict: needs changes** (finding 1). Findings 2-5 are for the dev or the owner. The owner decision on day/night colours is still open.
UX-45 review (round 2): ready to merge at eb8375157d
CI on eb83751: run 1337 checks / unit (pull_request) success, run 1338 infra-pin / fresh success. Offline suite (goodwe-addon-checks:1, --network none, GOODWE_REQUIRE_HWSIM=1, real hwsim at pin 3b4ff84): 79/79 PASS.
F1 fixed. The wizard now counts priced clock hours (homeview.priced_hours, web.py:331). 96 quarters read "24 of 24 hours". DST days: 23-hour day reads "23 of 24 hours", 25-hour day "24 of 24 hours", for both 15-minute and hourly data. The fixed "of 24" was already there before this PR (nit, not blocking).
F2 fixed._bar_cover is now price._cover_table() itself, not a copy. Gaps of 1-3 missing quarters are filled and longer holes are capped at an hour, exactly as the add-on prices them. The changed carried-tail test (a last provisional point covers one quarter, not an hour) is right: _cover_table's docstring says the last point gets the span of the one before it, and that the missing 45 minutes after a carried hourly tail are meant to raise Absence. The chart now matches that.
F3 / 2a fixed. With modes on, marks show only if a battery is switched on and in a price mode (_price_mode_in_use, the same test mode_runs() uses). Tested both ways, plus a switched-off battery.
F4 fixed. The day reset is now tested (R3 caught).
F5 / 1a fixed. With the stored static plan: arbitrage off gives no marks; arbitrage on gives exactly the verdict's two hours.
Scope: only homeview.py, web.py and test_home.py changed. No strings, CHANGELOG or version change.
Mutations, all caught: R3; dev's N1, N3, N4, N5. Mine: O1 only the first battery's mode counts; O2 carried tail covers an hour again; O3 2a applied to give runs only (cheap still marked).
## UX-45 review (round 2): ready to merge at eb8375157db39c45c65b5db4d0b9a451351fe25e
CI on eb83751: run 1337 `checks / unit (pull_request)` success, run 1338 `infra-pin / fresh` success. Offline suite (`goodwe-addon-checks:1`, `--network none`, `GOODWE_REQUIRE_HWSIM=1`, real hwsim at pin 3b4ff84): 79/79 PASS.
- **F1 fixed.** The wizard now counts priced clock hours (`homeview.priced_hours`, `web.py:331`). 96 quarters read "24 of 24 hours". DST days: 23-hour day reads "23 of 24 hours", 25-hour day "24 of 24 hours", for both 15-minute and hourly data. The fixed "of 24" was already there before this PR (nit, not blocking).
- **F2 fixed.** `_bar_cover` is now `price._cover_table()` itself, not a copy. Gaps of 1-3 missing quarters are filled and longer holes are capped at an hour, exactly as the add-on prices them. The changed carried-tail test (a last provisional point covers one quarter, not an hour) is right: `_cover_table`'s docstring says the last point gets the span of the one before it, and that the missing 45 minutes after a carried hourly tail are meant to raise `Absence`. The chart now matches that.
- **F3 / 2a fixed.** With modes on, marks show only if a battery is switched on and in a price mode (`_price_mode_in_use`, the same test `mode_runs()` uses). Tested both ways, plus a switched-off battery.
- **F4 fixed.** The day reset is now tested (R3 caught).
- **F5 / 1a fixed.** With the stored static plan: arbitrage off gives no marks; arbitrage on gives exactly the verdict's two hours.
- **Scope:** only `homeview.py`, `web.py` and `test_home.py` changed. No strings, CHANGELOG or version change.
- **Mutations, all caught:** R3; dev's N1, N3, N4, N5. Mine: O1 only the first battery's mode counts; O2 carried tail covers an hour again; O3 2a applied to give runs only (cheap still marked).
Merging with `head_commit_id` pinned to eb83751.
admin
merged commit 75f0804f9d into release/1.02026-10-19 12:15:57 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
One bar per price step: 96 quarters for a full day (92/100 on clock-change days), 24 for hourly or static tariffs. No averaging;
plan.average_tipremoved (EN/NL). Cheap/dear marks are the keptpricerunscheap run and give runs (battery modes on) or the arbitrage verdict's two points (modes off), not a thirds split. A missing quarter is an empty bar. Display only; no control, option or version change.Report: /home/homelab/projects/goodwe/reports/UX-45-dev-report.md
🤖 Generated with Claude Code
https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC
UX-45 review (round 1): needs changes
Reviewed head
17c60484172c9f8af79468b02a29fcccbc0f8684(ls-remote checked; base release/1.0243a756, masterad374ae). CI run 1335checks / unitand 1336infra-pin / freshare both success on this sha. Not merged, as the brief says: the owner still has to decide on day/night colours.Findings
MEDIUM, blocking. The setup wizard now says "for 96 of 24 hours".
goodwe_controller/app/web.py:329-331countsplan.pricesentries that have a price and passes the count tovalidate._price_check(), which printsf"Prices for today are in, for {prices_known} of 24 hours."(app/validate.py:2083). This PR makes the bars quarters, so a full day of 15-minute prices now reads "Prices for today are in, for 96 of 24 hours." I reproduced it with the same count web.py makes:243a756gives24 of 24 hoursand17c6048gives96 of 24 hours. The grep the brief asked for missed this consumer, and no test covers the count against the real bars. Fix: count distinct hours (for example, the distinct UTC hour ofstartamong priced bars), and add a check that 96 quarter prices give "24 of 24 hours". The day length is also a fixed "24" on 23 h and 25 h days, but that was already so on243a756.LOW. Holes of three quarters get filled in on the chart.
homeview.py:170-171(_bar_cover):gap in (step, hour)lets a point cover a gap of exactly one hour. In a 15-minute series with 10:15, 10:30 and 10:45 missing, the 10:00 price is drawn on all four quarters. With one or two quarters missing, the bars are left empty. So holes of 1-2 quarters are empty and holes of 3 are filled. The add-on itself (price._cover_table(), TARIFF-14) covers every gap up to an hour with the previous price. The chart therefore differs from what the add-on uses for 1-2 missing quarters (by the brief's choice) and matches it for 3. Pick one rule. Note that_bar_cover,_spansand_cover_tableare now three copies of the cover rule.LOW, for the owner question. Marks are the site's run plan, not what each battery does.
homeview.py:221-227: with modes on, the cheap run is marked whenevercharge_useful(), and every give run is marked "dear", even when no battery has a price mode or may charge from the grid.mode_runs()(homeview.py:394-396) draws blocks only for those batteries, and_price_runs()builds the plan whatever the modes are. In the dev's ownquarter-*shots, both batteries "Follow the house" while 13:45-14:15 is marked low and about 15:00-24:00 is marked high.LOW. Past marks are kept only if the page was open during that quarter.
homeview.py:196-204:keptis written only while/statusis built (page poll or setup wizard). If nobody has the page open during a run, that quarter shows as "mid" once it is over, and the same happens after a restart (the dev noted that). The memory is bounded (one entry per bar, reset on a new day inDayPrices.see), so growth and stale state across days are fine. But nothing tests the reset: mutation R3 (no reset) survives.LOW, test gap. The day/night test only covers the moment before the first cycle.
test_home.py:640-651(c_dn) never stores a tariff plan, so the verdict is "no_prices" and "all mid" is expected. I ran it with the plan the cycle actually stores (static_planfrom now, asresolve_tariffdoes with the provider off). With arbitrage on, the head marks 22:00 cheap and 11:00 dear (the verdict's two hours). With arbitrage off, it marks nothing. Please add that case, so the owner question is pinned down by a test.LOW, a11y. At 390 px each quarter bar is about 2.4 px wide (
.bars.fine,app.css:81-82). Keyboard roving works (unchanged code; Home/End), but tapping one quarter on a phone is not realistic. The chart was already below a 24 px target with 24 bars; it is now about 6x smaller. Optional: a wider tap target or snapping to the nearest bar.NIT.
price_barsscans the cover list once per bar (homeview.py:196,next(...)overcover): about 5 ms per build for 96 bars here, on every 3 s poll per viewer. That is fine, but a single pass would be easy to write."No trading on price yet: no prices have arrived." (daynight shots)
This was already on
243a756and is not new. The dev's screenshot server (serve.py) never setstariff_planfor the day/night controller, sofresh_tariff_plan()is None.trade_verdict()then returns"no_prices", and that line is the result. This is the state before the first price cycle. The same fixture gives the identical line on243a756(lines: ['No trading on price yet: no prices have arrived.']at both commits). Once a static plan is stored, as in production, the line is gone at both commits. Neithertrade_verdictnor the line wording changed in this PR. The no-colours look in those shots also comes from the fixture. In production with arbitrage on, the chart shows one cheap hour and one dear hour.Checked and fine
02:45-02:00,02:00-02:15; spring-forward01:45-03:00; day end23:45-00:00(the same end label as before). Hourly series and static tariffs give 24 bars (25 on fall-back). One missing quarter gives one "none" bar and nothing shifts.trade_runs()draws. With modes on, they are the keptpricerunscheap run (whencharge_useful()) and the give runs, as kept bykeep_started.points/hour_bars/FLAT_EUR_KWH/average_tiphave no other reader (JS, ui_text, tests, HA/MQTT). The only other consumer ofplan.pricesis the setup wizard count (finding 1).plan.average_tipis gone from EN, NL and app.js.goodwe-addon-checks:1(--network none,GOODWE_REQUIRE_HWSIM=1, real hwsim at infra pin 3b4ff84): 79/79 PASS on 17c6048; test_freeze part 3 ran against the rig hwsim.kept(R3),.fineclass dropped (R6, CSS only).Verdict: needs changes (finding 1). Findings 2-5 are for the dev or the owner. The owner decision on day/night colours is still open.
UX-45 review (round 2): ready to merge at
eb8375157dCI on
eb83751: run 1337checks / unit (pull_request)success, run 1338infra-pin / freshsuccess. Offline suite (goodwe-addon-checks:1,--network none,GOODWE_REQUIRE_HWSIM=1, real hwsim at pin 3b4ff84): 79/79 PASS.homeview.priced_hours,web.py:331). 96 quarters read "24 of 24 hours". DST days: 23-hour day reads "23 of 24 hours", 25-hour day "24 of 24 hours", for both 15-minute and hourly data. The fixed "of 24" was already there before this PR (nit, not blocking)._bar_coveris nowprice._cover_table()itself, not a copy. Gaps of 1-3 missing quarters are filled and longer holes are capped at an hour, exactly as the add-on prices them. The changed carried-tail test (a last provisional point covers one quarter, not an hour) is right:_cover_table's docstring says the last point gets the span of the one before it, and that the missing 45 minutes after a carried hourly tail are meant to raiseAbsence. The chart now matches that._price_mode_in_use, the same testmode_runs()uses). Tested both ways, plus a switched-off battery.homeview.py,web.pyandtest_home.pychanged. No strings, CHANGELOG or version change.Merging with
head_commit_idpinned toeb83751.