fix(stack): five regressions caught by codex review#104
Merged
Conversation
…e 11 PR-18)
PR-18 — Final v2.0 GA plank. Property-based tests cover the five
invariants from v2 research § 7.3 for tariff_engine pure functions
(calc_stepped_cost, get_stepped_import_rate, get_current_tou_period).
- 1. Monotonic stepped cost: cost(k1) <= cost(k2) for k1 <= k2.
- 2. Threshold equality: cost(threshold) == threshold * step1_rate.
- 3. Step composition above threshold: cost = step1_cost +
(k - threshold) * step2_rate.
- 4. Stepped rate dichotomy: returned rate is exactly step1 or step2.
- 5. TOU period closure: returned period name is in the supplied
dict OR "unknown"; rate matches the period when found; full-day
coverage produces no "unknown".
≥ 200 fuzzed examples per invariant via hypothesis. 9 test classes.
1054 total pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stack-wide bug sweep flagged by `codex review --base dev` across PRs #88-#102. All five are real functional regressions, not nits. - P1 (config_flow): dashboard_token entry builder did not copy DWT setup fields from self._data into entry.data/options. Every new DWT install hit ConfigEntryNotReady at first refresh because _build_dwt_provider() found CONF_DWT_OE_ENABLED missing (AC-10c). - P2#2 (config_flow): comparator form exposed static_prd for Amber/ Flow Power/LocalVolts unconditionally. No flow writes CONF_*_STATIC_PLAN, so selecting static_prd bombed the reload (Amber/LV: ConfigEntryNotReady; FP: silent live_api fallback with warning). Form now gates static_prd per comparator on stored-static-plan presence. - P2#3 (config_flow): reauth dispatcher read coordinator._reauth_provider_id via entry.runtime_data, but runtime_data is set AFTER first refresh. Auth failures during startup (expired key + HA restart) aborted with reauth_provider_unknown. Dispatcher now falls back to entry.data[CONF_CURRENT_PROVIDER] when the coordinator tag is None. - P2#4 (config_flow): reconfigure dispatcher read coordinator._current_plan_provider.id, but CdrPlanProvider.id is `{brand}_{plan_id}` for CDR users — never the literal PROVIDER_AMBER / PROVIDER_LOCALVOLTS slug. Every CDR Amber/LV entry hit reconfigure_unsupported. Dispatcher now reads entry.data[CONF_CURRENT_PROVIDER]. - P2#5 (coordinator): _async_update_data triggered _fetch_today_price_schedule() on every 30s tick for static/off Amber entries because _maybe_poll_amber's early-return never updated _last_amber_poll. DWT + static-Amber users hammered the Amber API endpoint with stale credentials. First-run guard now combines the sentinel check with PRICING_MODE_LIVE_API. Tests: 13 regression tests in tests/test_codex_regression_fixes.py; existing test_reconfigure.py dispatcher test updated for new contract. 1067 passing (was 1054). Ruff clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Base automatically changed from
feat/tariff-engine-hypothesis
to
feat/hassfest-hacs-ci
May 22, 2026 04:50
This was referenced May 23, 2026
Artic0din
added a commit
that referenced
this pull request
May 23, 2026
… Jinja (#145) Two real bugs surfaced by a Copilot-CLI retro-review of the 22 merged PRs that the prior @claude batch couldn't reach (OIDC workflow-validation against stale main). statistics.py — external_statistic_id now sanitizes via regex: CDR-derived provider_ids carry the plan id verbatim (e.g. ``agl_AGL-CDR-N0001`` for AGL via CDR), so `.lower()` alone left hyphens that the recorder's ``[a-z0-9_]+`` regex silently rejected. Every CDR user's dual-write would fail and the Energy Dashboard would never receive their cost data — same silent-failure class #107 fixed for uppercase ULIDs. Added ``_STATISTIC_ID_OBJECT_SAFE`` compiled regex that coerces ANY character outside ``[a-z0-9_]`` to underscore. New regression test ``test_cdr_plan_id_with_hyphens_is_sanitized`` + adjusted ``test_entry_id_sliced_to_8_chars`` for the post-sanitization shape. Surfaced by retro-review of PRs #93, #95. blueprints — variables block replaces !input inside Jinja: ``daily_7pm_summary.yaml`` and ``wholesale_spike_alert.yaml`` had templates like ``{{ states(!input today_cost_sensor) }}``. ``!input`` is a YAML tag (resolved at YAML parse time) and is invalid inside Jinja ``{{ }}`` — Jinja parses ``!`` as an invalid operator and the template never renders. Replaced with the standard HA pattern: a ``variables:`` block at action level that binds blueprint inputs as Jinja identifiers (``today_cost_entity: !input today_cost_sensor`` etc.), then ``{{ states(today_cost_entity) }}`` in the message. Surfaced by retro-review of PRs #99, #100. 22 Copilot reviews ran (PRs #85, #87-#101, #104, #105, #108-#111). Most findings were false positives — Copilot flagged ``ServiceValidationError`` and ``async_items`` as broken HA APIs (they're both fine), and several findings duplicated bugs already fixed by codex P0/P1 work (token-in-URL #109, DWT reset #109). Findings library + triage notes archived in ``.planning/copilot-retro/``. Full test suite: 1120 passing.
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.
Summary
codex review --base devagainst the tip of the Phases 7-11 PR stack (#86-#102, ~6777 LOC across 46 files) surfaced five functional regressions that block real user paths. This PR fixes all of them on top of the stack so the cascade can land cleanly.Stacked on
feat/tariff-engine-hypothesis(#102). Merge after #103 + the cascade.Findings + Fixes
config_flow.pyasync_step_dashboard_tokenself._databut never copied into the final entry'sdata/options. New DWT installs fail at first refresh withConfigEntryNotReady(AC-10c).CONF_DWT_OE_API_KEY/CONF_DWT_REGIONintodata;CONF_DWT_OE_ENABLED/CONF_DWT_OE_DAILY_SUPPLYintooptions(plus AEMO variants). Gated on the setup-step flags so CDR-flow entries are unaffected.config_flow.pyasync_step_comparatorsstatic_prdrendered for all three comparators unconditionally. No flow writesCONF_*_STATIC_PLAN, so selecting it bricks the reload (ConfigEntryNotReadyfor Amber/LV; silent fallback-with-warning for Flow Power)._modes_for(static_key)helper. Excludesstatic_prdunless the matching static plan is present inentry.options.config_flow.pyasync_step_reauthcoordinator._reauth_provider_idviaentry.runtime_data, which is set afterasync_config_entry_first_refresh(). Startup auth failures (expired key + HA restart) aborted withreauth_provider_unknown.entry.data[CONF_CURRENT_PROVIDER]when the coordinator tag isNone. Original runtime_data read is preserved for the comparator-failure case.config_flow.pyasync_step_reconfigurecoordinator._current_plan_provider.id.CdrPlanProvider.idis{brand}_{plan_id}(e.g.amber_brokerage-xyz), never the literalPROVIDER_AMBER/PROVIDER_LOCALVOLTS— every CDR Amber/LV user hitreconfigure_unsupported.entry.data[CONF_CURRENT_PROVIDER](a stable, literal slug).coordinator.py_async_update_data_maybe_poll_amber()returns early in static/off mode without updating_last_amber_poll. The first-run sentinel at the top of_async_update_datatherefore re-triggers_fetch_today_price_schedule()every 30s, hammering Amber's API with stale credentials for DWT + static-Amber entries._last_amber_poll == 0.0 and self._amber_mode == PRICING_MODE_LIVE_API.Tests
tests/test_codex_regression_fixes.py— 13 source-level + behavioural regression tests, one or more per finding.tests/test_reconfigure.py—test_dispatcher_reads_active_provider_id_from_coordinatorupdated →test_dispatcher_reads_provider_from_entry_data; pinned with a "must not regress" guard against the old_current_plan_providerlookup.(was 1054 — +13 new regression tests)
Test plan
pytest tests/— full suite green (1067 passed)ruff checkon changed files — cleantar | ssh root@homeassistant.local)Source
/tmp/codex-review-stack.log— full codex output (kept locally; not committed).