fix(stack): grid sensor options-fallback + state from DWT region#150
Merged
Conversation
Two bugs surfaced by live UAT 2026-05-24 for the DWT-only user path
(users who configured Dynamic Wholesale Tariff but never completed
the CDR wizard).
coordinator.py — grid_power_entity now options→data fallback.
The config-flow wizard writes CONF_GRID_POWER_SENSOR to entry.data
during initial setup (config_flow.py:2020). Only the options-flow
dialog mirrors it into entry.options later. The coordinator read
entry.options.get(CONF_GRID_POWER_SENSOR, "") exclusively, so users
who completed initial setup but never opened the options dialog had
``_grid_power_entity = ""``. Consequences:
- backfill silently no-op'd (backfill.py:317 early-returns on
empty entity → days_loaded=0)
- _read_grid_power returned None for every coordinator tick
Fix matches the existing _get_opt helper (line 532) and the API-key
reads two lines below: ``entry.options.get(K) or entry.data.get(K, "")``.
cdr/ranking_job.py — state filter falls back to DWT region.
get_user_geography previously returned state=None always — comment
said "derived later" but matches_geography(state=None) is wildcard.
Result: a VIC DWT user's top-K included plans flagged for other states
the user can't purchase from.
New _AEMO_REGION_TO_STATE map + _state_from_dwt_region helper. State
derived from dwt_region (VIC1→VIC, NSW1→NSW, etc.) when explicit
cdr_postcode is unset. CDR-wizard users unaffected — explicit state
is additive, not exclusive.
4 new regression tests pin the fallback:
- test_state_derived_from_dwt_region_when_cdr_postcode_unset
- test_state_falls_back_to_dwt_region_when_cdr_plan_present
- test_unknown_dwt_region_returns_none_state
- test_dwt_region_case_insensitive
Manifest bumped to 1.6.0-beta.6. Silver-checklist version assertion
updated to match.
Full test suite: 1138 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 updates the integration to version 1.6.0-beta.6, addressing issues for users who configured Dynamic Wholesale Tariffs (DWT) without completing the CDR wizard. Key changes include adding a fallback for the grid_power_sensor configuration and deriving the user's state from the DWT region for plan ranking. Feedback highlights that this fallback logic should also be applied in rebuild_engine and async_run_ranking_job to prevent configuration loss and ensure consistent filtering when options are updated.
Comment on lines
+439
to
+442
| self._grid_power_entity: str = ( | ||
| entry.options.get(CONF_GRID_POWER_SENSOR) | ||
| or entry.data.get(CONF_GRID_POWER_SENSOR, "") | ||
| ) |
There was a problem hiding this comment.
This fallback logic correctly addresses the issue for _grid_power_entity during initialization. However, the same logic is missing in rebuild_engine (line 2162), where self._grid_power_entity is updated using only new_options. This will cause the sensor configuration to be lost if the options flow is saved without explicitly re-selecting the sensor. Additionally, the same integration issue exists for the ranking job: in async_run_ranking_job (line 1892), only entry.options is passed. For DWT-only users who haven't run the options flow, entry.options is empty, meaning get_user_geography will fail to find the dwt_region and won't apply the intended state filter.
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.
6 tasks
Artic0din
added a commit
that referenced
this pull request
May 27, 2026
Both `__init__` (initial setup) and `rebuild_engine` (options-flow reload) duplicated grid-sensor resolution, the DWT vs CDR current-plan slot, every comparator's pricing-mode logic, and the named comparator. That duplication had already drifted twice in production (#150 grid sensor; #153 grid-sensor double-assign). Constitution P14 — "If the same issue appears in multiple places, fix the underlying abstraction." Both call sites now flow through one projector. Strict mode (init-time) raises ConfigEntryNotReady on missing required config; non-strict mode (rebuild-time) degrades gracefully — preserves existing semantics on both paths. `_build_dwt_provider` now accepts `(options, data)` mappings instead of a `ConfigEntry` so the same builder serves both paths. Tests: parametrised `TestApplyOptionsToStateEquivalence` asserts the init-time and rebuild-time paths produce identical observable state across four scenario fixtures plus strict/non-strict gate semantics and the #150 grid-sensor regression case. `tests/conftest.py` now stubs `DataUpdateCoordinator` with a real subclassable class so `PriceHawkCoordinator` resolves as an actual type for unit testing (was a `_MockModule` MagicMock). Validation: ruff clean on touched files; pyright net -2 errors (2 pre-existing CdrPlanProvider protocol mismatches remain, was 4 duplicates before); pytest 1108/1108 passing including 8 new equivalence tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Artic0din
added a commit
that referenced
this pull request
May 27, 2026
* refactor(coordinator): extract _apply_options_to_state Both `__init__` (initial setup) and `rebuild_engine` (options-flow reload) duplicated grid-sensor resolution, the DWT vs CDR current-plan slot, every comparator's pricing-mode logic, and the named comparator. That duplication had already drifted twice in production (#150 grid sensor; #153 grid-sensor double-assign). Constitution P14 — "If the same issue appears in multiple places, fix the underlying abstraction." Both call sites now flow through one projector. Strict mode (init-time) raises ConfigEntryNotReady on missing required config; non-strict mode (rebuild-time) degrades gracefully — preserves existing semantics on both paths. `_build_dwt_provider` now accepts `(options, data)` mappings instead of a `ConfigEntry` so the same builder serves both paths. Tests: parametrised `TestApplyOptionsToStateEquivalence` asserts the init-time and rebuild-time paths produce identical observable state across four scenario fixtures plus strict/non-strict gate semantics and the #150 grid-sensor regression case. `tests/conftest.py` now stubs `DataUpdateCoordinator` with a real subclassable class so `PriceHawkCoordinator` resolves as an actual type for unit testing (was a `_MockModule` MagicMock). Validation: ruff clean on touched files; pyright net -2 errors (2 pre-existing CdrPlanProvider protocol mismatches remain, was 4 duplicates before); pytest 1108/1108 passing including 8 new equivalence tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(coordinator): preserve _dwt_provider on bad-options early-return + add DWT equivalence cases Linus PR #170 audit fixed two real defects in _apply_options_to_state: 1. Orphan-state bug. The projector cleared self._dwt_provider = None BEFORE the strict-mode guard decided whether to bail. A non-strict rebuild with neither cdr_plan nor a DWT enable flag would null _dwt_provider, then early-return, leaving _current_plan_provider pointing at the stale DWT instance — a half-rebuilt coordinator. Fix moves the reset INSIDE the CDR branch, past the early-return, so a bad rebuild keeps every provider slot intact. P13 no-regression: every prior call site that depended on the CDR branch nulling _dwt_provider still sees that behaviour, because the reset still happens — just not in the orphan window. 2. DWT branch untested. _EQUIVALENCE_CASES were all CDR — the DWT vs CDR branch of _build_dwt_provider had no init-vs-rebuild parity assertion. Added dwt_oe_enabled_options_only (OpenElectricity path, every setting in options) and dwt_aemo_enabled_data_fallback (AEMO Direct, every setting in data so the opt() fallback inside _build_dwt_provider is what makes the assertion pass). 3. New regression test test_nonstrict_rebuild_preserves_dwt_provider_on_bad_options pre-seeds a DWT provider, fires a bad rebuild, asserts all three slots survive. Confirmed to fail pre-fix. Minor (Linus also flagged): FlowPowerProvider(dict(options)) and LocalVoltsProvider(dict(options)) verified safe — both constructors accept dict[str, Any] and dict(...) coerces any Mapping to a plain dict without behaviour change. Validation: - uv run ruff check on changed files: passes (pre-existing scripts/ failures unrelated) - uv run pyright custom_components/pricehawk/coordinator.py: same 2 pre-existing errors / 12 warnings — no new typing issues - uv run pytest tests/test_coordinator.py tests/test_reconfigure.py tests/test_runtime_data.py: 54 passed P13 no-regression + P17 tests are part of the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(coordinator): preserve non-strict graceful degrade on inconsistent DWT options (codex follow-up) Codex P2 follow-up on PR #170. ``_apply_options_to_state(strict=False)`` unconditionally called ``_build_dwt_provider``, which raises ``ConfigEntryNotReady`` (AC-10c) when an entry's ``current_provider`` marker says DWT but the matching enable/API-key fields are missing. Pre-refactor ``rebuild_engine`` never built a DWT provider from scratch, so the same inconsistent options update would log and keep the existing providers — the systemic-fix refactor regressed that behaviour and now tears the integration down mid-edit. Fix wraps the builder call in ``try/except ConfigEntryNotReady`` that only swallows the raise on the non-strict (rebuild) path — strict mode (``__init__``) still re-raises so HA surfaces the failure at initial setup. On the rebuild path we log and bail BEFORE touching ``_dwt_provider`` / ``_current_plan_provider`` / ``_providers``, matching the existing missing-``cdr_plan`` early-return contract (P13 no-regression). Tests: ``tests/test_reconfigure.py::TestRebuildGracefulDegradeOnInconsistentDwt`` pins the regression — non-strict rebuild does not raise, keeps existing providers, AND strict init still raises. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Two DWT-only-user bugs surfaced by UAT.
coordinator.py—_grid_power_entitynow falls back toentry.datawhenentry.optionsis empty (config-flow wizard writes to data; options-flow mirrors later). Fixes backfilldays_loaded=0for users who never opened options dialog.cdr/ranking_job.py—get_user_geographyderives state fromdwt_region(VIC1→VIC etc.) whencdr_postcodeunset. Ranking now scopes to user's state instead of returning national plans they can't buy.4 new tests. 1138 passing.