fix(coordinator): rebuild explanation every tick (closes empty-bullets cache)#148
Merged
Conversation
…s cache)
Live UAT 2026-05-24: even after beta.3 fixed the NEMWeb regex and the
AEMO spot rate started flowing, the Best Provider winner_explanation
attribute still showed bullets=[]. Root cause: build_explanation only
ran inside the midnight daily-rollover branch in _async_update_data,
not on every tick.
Sequence that exposed it:
1. Midnight: NEMWeb broken (beta.2 deployed late morning), DWT
provider has no _last_price, providers["dwt_aemo_direct"].extras
has wholesale_price=None, _dwt_won_bullets skips every conditional,
returns []. explanation.to_dict() caches bullets=[].
2. Late morning: NEMWeb fixed by beta.3, AEMO fetch starts succeeding,
DWT's _last_price gets populated, today_cost climbs from $1.10 to
$5.04 across ticks. Best Rate sensor reads 549.13 c/kWh — real
wholesale spike data flowing.
3. winner_explanation.bullets still []. Explanation built at midnight
was never rebuilt because the rollover branch is the only caller.
Fix: rebuild explanation on every coordinator tick after the per-
provider update loop, gated on self._providers being non-empty (else
min() over empty dict raises ValueError). The explanation now reflects
the *current* provider snapshot every 30s. Cost is a handful of dict
comprehensions per tick — negligible.
Manifest bumped to 1.6.0-beta.4. Silver-checklist version assertion
updated to match.
Full test suite: 1130 passing.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request bumps the version to 1.6.0-beta.4 and updates the coordinator to rebuild the winner explanation on every 30-second tick, ensuring the dashboard reflects current provider data. Feedback indicates that the new logic is duplicated with the existing daily rollover section, which could prematurely overwrite the previous day's summary. Furthermore, the changelog incorrectly states the logic was moved when it was actually added, leaving the redundant code in place.
Comment on lines
+1164
to
+1175
| if self._providers: | ||
| avg_spot = None | ||
| if self._amber and self._amber.import_kwh_today > 0: | ||
| avg_spot = ( | ||
| self._amber.import_cost_today_c | ||
| / self._amber.import_kwh_today | ||
| ) | ||
| explanation = build_explanation( | ||
| self._build_providers_block(), | ||
| avg_amber_spot_c_kwh=avg_spot, | ||
| ) | ||
| self._last_explanation = explanation.to_dict() |
There was a problem hiding this comment.
The logic for building the winner explanation is now duplicated with the existing block in the daily rollover section (lines 1075-1085).
Because this new block runs every 30 seconds, it will immediately overwrite the captured "Why X won" explanation for the previous day with a new explanation for the current day as soon as the rollover tick completes. This effectively makes the end-of-day summary for the previous day inaccessible to the user.
If the intention is to provide a live explanation for the current day (as suggested by the PR description), the redundant block in the rollover section should be removed to avoid unnecessary execution and semantic confusion at the day boundary.
|
|
||
| ### Fixed | ||
|
|
||
| - **`build_explanation` now runs every coordinator tick.** Moved the call out of the daily-rollover branch into `_async_update_data` after the per-provider tick, gated on `self._providers` being non-empty. The explanation now reflects the *current* provider snapshot every 30s, not whatever the rollover branch saw at midnight. Cost is a handful of dict comprehensions per tick — pays for itself the moment a user opens the dashboard before the next midnight. (`coordinator.py:1145-1166`) |
There was a problem hiding this comment.
The changelog states that the call was moved out of the daily-rollover branch, but the current implementation in coordinator.py actually adds a second call while leaving the original one intact at lines 1075-1085. This discrepancy confirms that the intended cleanup of the rollover branch was missed in the code changes.
Artic0din
added a commit
that referenced
this pull request
May 24, 2026
…#153) * fix(stack): four gemini-review findings I missed across beta.4-beta.8 Ryan caught that I'd been merging today's PRs with --auto without reading gemini-code-assist reviews. These are the legitimate findings from those reviews, fixed in one batch. #148 (gemini): per-tick build_explanation duplicates the rollover branch. Beta.4 added a per-tick rebuild but left the daily-rollover rebuild in place. At midnight the rollover branch built the "yesterday's summary" snapshot, then the per-tick rebuild a few seconds later clobbered it with the post-reset all-zeros snapshot. Per-tick rebuild fires on the midnight tick too, so the rollover one was redundant AND actively harmful. Removed. #147 (gemini): _pick_latest_dispatch_file lex-sort is case-sensitive. _FILE_RE uses re.IGNORECASE — could capture mixed-case filenames. sorted(matches)[-1] then compared Unicode codepoints; uppercase sorts before lowercase. A hypothetical mixed-case listing would put an older uppercase file last after sort. NEMWeb today is all uppercase but the contract should match the regex's tolerance. key=str.upper normalises. #150 (gemini): rebuild_engine never re-reads _grid_power_entity. The options→data fallback added in beta.6 fires once at __init__ and never refreshes. Users editing the grid sensor via options-flow saw the integration silently keep pointing at the prior entity until HA restart. rebuild_engine is the options-update path — now re-resolves. #152 (gemini): reset_today service silently succeeds with no entries. Silver action-exceptions rule: service handlers should raise when they cannot perform the action. A user with all entries in failed-load state would see "Successfully executed pricehawk.reset_today" and observe no dashboard change. Now raises HomeAssistantError telling them to reload the integration. PR #146 (export-credit bullet) and PR #149 (loop optimization) are real findings too but lower priority — left as future work. Manifest bumped to 1.6.0-beta.9. Silver-checklist version assertion updated to match. Full test suite: 1140 passing. * fix(coordinator): drop duplicate _grid_power_entity overwrite in rebuild_engine Per gemini review of PR #153: the fallback I added at the TOP of rebuild_engine was being silently negated by a duplicate options-only assignment at the END of the same function. Removed the second assignment; kept a comment marker so this isn't accidentally reintroduced. Bug effect: rebuild_engine fired on every options-flow save would overwrite my fix with the empty-string default if entry.options didn't have CONF_GRID_POWER_SENSOR. Same regression as the original bug beta.6 was meant to fix. NOTE: gemini also flagged that rebuild_engine creates new provider instances on every options update, losing today's daily accumulators (import_kwh_today, import_cost_today_c, etc.). That's a real concern but a bigger refactor — left for a follow-up PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second post-deploy UAT hotpatch. Even after beta.3 fixed NEMWeb, winner_explanation.bullets stayed []. Root cause: explanation cached at midnight when NEMWeb was still broken; never rebuilt during the day.
Fix
Moved
build_explanationfrom the midnight-only rollover branch into the per-tick_async_update_databody, gated on non-emptyself._providers. Explanation now reflects current provider snapshot every 30s.Test plan
pytest— 1130 passingruff check— clean