Skip to content

fix(stack): two follow-ups from #107 retro-review#114

Merged
Artic0din merged 1 commit into
devfrom
fix/107-followups
May 23, 2026
Merged

fix(stack): two follow-ups from #107 retro-review#114
Artic0din merged 1 commit into
devfrom
fix/107-followups

Conversation

@Artic0din

Copy link
Copy Markdown
Owner

Follow-ups from Claude retro-review of #107 (run against synthetic PR #113).

Changes

File What Why
`aemo_api.py:119-122` Updated stale comment Comment still claimed every filename has `_LEGACY` suffix — the very assumption #107 corrected
`statistics.py:46` `.lower()` on `provider_id` segment Belt-and-suspenders. All current providers (`amber`, `globird`, `dwt_aemo_direct`) are already lowercase so latent, but a future mixed-case provider would silently re-trigger the `Invalid statistic_id` recorder rejection #107 fixed for `entry_id`
`tests/test_external_statistics.py` New test `test_provider_id_lowercased_for_ha_recorder_contract` Regression guard using synthetic `DWT_AEMO_Direct` provider id
`CHANGELOG.md` Unreleased entry Provenance for the retro-review chain

Out of scope (separate issue to be filed)

Test plan

  • `pytest tests/test_external_statistics.py` — all pass
  • `ruff check` — clean
  • Live UAT: not required (no behavioural change for existing providers)

- aemo_api.py:119 — stale comment still claimed `_LEGACY` was part of
  every filename; the very assumption #107 corrected. Updated to describe
  both shapes + why lexical sort still works post-suffix-drop.
- statistics.py:46 — `provider_id` was not `.lower()`-ed. All current
  IDs (`amber`, `globird`, `dwt_aemo_direct`) are already lowercase so
  the gap is latent, but a future mixed-case provider would silently
  re-trigger the same `Invalid statistic_id` recorder rejection #107
  fixed for `entry_id`. Belt-and-suspenders `.lower()` + regression test
  using a synthetic `DWT_AEMO_Direct` provider id.

Surfaced by an @claude review of synthetic retro-PR #113. The
background-task cancellation gap also flagged in that review is
pre-existing (not introduced by #107) and is filed as a separate issue.
@Artic0din Artic0din merged commit 25a12b8 into dev May 23, 2026
9 of 11 checks passed
@Artic0din Artic0din deleted the fix/107-followups branch May 23, 2026 13:08
Artic0din added a commit that referenced this pull request May 24, 2026
)

Bumps manifest to 1.6.0-beta.2 — first HACS-beta tag carrying the full
Phase 7-11 work landed since 1.6.0-beta.1. Live UAT against the production
HA install on 2026-05-24 confirmed the prior fixes (NEMWeb regex #107,
statistic_id sanitization #114/#145, bootstrap block #107, codex P0/P1
work in #109/#110/#111) had not reached users because the last tagged
release was v1.4.0-beta.1 from 2026-05-02.

Also fixes a UAT-surfaced bug not previously caught:

explanation.py — DWT winners no longer produce empty bullets.
  ``build_explanation`` branched on literal provider IDs ("amber",
  "globird", "flow_power", "localvolts") but Dynamic Wholesale Tariff
  providers carry IDs like "dwt_aemo_direct" / "dwt_openelectricity",
  so every DWT win fell through to an empty bullet list. The Best
  Provider sensor's winner_explanation attribute showed bullets=[]
  with margin_aud=0 — no "why" surfaced to the user. Added a
  ``winner_id.startswith("dwt_")`` branch + ``_dwt_won_bullets``
  builder using the standard provider snapshot shape: wholesale spot
  rate (c/kWh derived from $/MWh), today's import volume + cost,
  daily supply charge, and a stale-price warning when the wholesale
  price is over 10 min old. 5 regression tests in
  ``TestDwtWinnerBullets``.

Full test suite: 1125 passing.

Tag + GitHub pre-release follow immediately after this merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant