feat(service): pricehawk.reset_today (recovery for mid-day cost fixes)#152
Merged
Conversation
Recovery hook for the beta.7 AEMO RRP-row-type fix. The fix corrected the rate going forward, but each DWT provider's _import_cost_today_c already carried inflated values accumulated under the prior 60x bug. Live UAT screenshot showed current_plan_cost_today=$66.78 on a real spend < $2 — the residual would clear naturally at midnight but UX is "sit here until 00:00 AEST waiting for my dashboard to make sense." Adds a simple service that calls reset_daily() on every provider for every PriceHawk entry, then persists. No parameters, no surprises. Use case isn't just this one bug — any future cost-math fix shipped mid-day will leave the same residual in user accumulators. Now there's a one-click recovery path. Manifest bumped to 1.6.0-beta.8. Silver-checklist version assertion updated to match. Full test suite: 1140 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 introduces the pricehawk.reset_today service, which allows users to manually zero daily accumulators for all registered providers to recover from mid-day calculation errors. The implementation includes the service handler in __init__.py, registration in services.yaml, and a version bump to 1.6.0-beta.8. Feedback suggests improving the service handler by raising a HomeAssistantError when no active entries are found, which aligns with the 'action-exceptions' requirement for the Silver quality scale.
Comment on lines
+275
to
+294
| del call # Service takes no parameters; resets every entry. | ||
| for entry in hass.config_entries.async_entries(DOMAIN): | ||
| data: PriceHawkData | None = getattr(entry, "runtime_data", None) | ||
| if data is None: | ||
| continue | ||
| coord = data.coordinator | ||
| for provider in coord._providers.values(): | ||
| try: | ||
| provider.reset_daily() | ||
| except Exception as exc: # noqa: BLE001 — never sink the batch | ||
| _LOGGER.warning( | ||
| "reset_today: %s.reset_daily failed: %s", | ||
| getattr(provider, "id", "?"), exc, | ||
| ) | ||
| await coord.async_persist_state() | ||
| _LOGGER.info( | ||
| "reset_today: zeroed daily accumulators for %d provider(s) " | ||
| "on entry %s", | ||
| len(coord._providers), entry.entry_id, | ||
| ) |
There was a problem hiding this comment.
For consistency with other service handlers in this integration and to adhere to the 'action-exceptions' rule (part of the Silver quality scale requirements), this handler should raise a HomeAssistantError if no active PriceHawk entries are found. This provides better feedback to the user if the service is called before the integration is fully set up or if it has been unloaded.
del call # Service takes no parameters; resets every entry.
entries = [
e for e in hass.config_entries.async_entries(DOMAIN)
if getattr(e, "runtime_data", None) is not None
]
if not entries:
raise HomeAssistantError(
"No PriceHawk entry is currently loaded — service call cannot "
"be routed. Add an entry via Settings → Devices & Services."
)
for entry in entries:
coord = entry.runtime_data.coordinator
for provider in coord._providers.values():
try:
provider.reset_daily()
except Exception as exc: # noqa: BLE001 — never sink the batch
_LOGGER.warning(
"reset_today: %s.reset_daily failed: %s",
getattr(provider, "id", "?"), exc,
)
await coord.async_persist_state()
_LOGGER.info(
"reset_today: zeroed daily accumulators for %d provider(s) "
"on entry %s",
len(coord._providers), entry.entry_id,
)References
- Service handlers should follow the 'action-exceptions' discipline as required by the integration's Silver quality scale checklist, raising HomeAssistantError or ServiceValidationError on failure or invalid state.
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.
Artic0din
added a commit
that referenced
this pull request
May 24, 2026
* test(silver): enumerate handlers + per-handler raise check
The prior assertion src.count("raise HomeAssistantError(") >= 3
hard-coded the handler count at the time of writing. When
handle_reset_today was added in beta.8 without a raise, the test stayed
green because the count was still ≥ 3 — the new handler just didn't
add to it. Gemini caught the compliance gap on PR #152 review.
Test now enumerates handlers via regex on async def handle_<name>,
checks each handler's body contains raise HomeAssistantError(, and
reports specific missing handlers in the assertion message. Threshold
auto-scales: adding a new handler without the raise breaks the test.
Also asserts >= 4 handlers exist so a future refactor that accidentally
removes one trips the test.
No manifest bump — test-only change, no HACS deploy needed.
Full test suite: 1140 passing.
* test(silver): rewrite handler-raise check with ast (gemini PR #154 review)
Gemini flagged the regex-based approach as fragile: the last handler's
sliced 'body' extended to EOF, swallowing post-handler text (service
registrations, future module-level code) into false positives.
Rewrote with ast: parse the file, walk to AsyncFunctionDef nodes named
handle_*, then per-handler walk its body checking for
raise HomeAssistantError(...). Nested AsyncFunctionDef nodes are
skipped (they're their own scope, validated when they're top-level).
Self-test: ast finds 4 handlers (analyze_csv, backfill,
rank_alternatives, reset_today) — same as ground truth.
* test(silver): prune nested defs with manual DFS (gemini PR #154 review)
ast.walk is a flat generator — yields all descendants. My continue
on encountered AsyncFunctionDef only skipped that node, not its
subtree, so a raise HomeAssistantError inside a nested helper would
still satisfy the check for the outer handler.
Replaced with manual stack-based DFS using ast.iter_child_nodes.
Encountering a FunctionDef/AsyncFunctionDef now skips the entire
subtree, not just the def node.
Self-verified: synthesised a handler with nested helper that raises
HAE, check correctly returns False (nested doesn't count for outer).
Per gemini suggestion on PR #154 inline review.
* test(silver): accept ServiceValidationError + prune ClassDef + sync defs (gemini #154 review-3)
Gemini's third pass on PR #154 flagged three remaining gaps:
1. Silver action-exceptions accepts EITHER HomeAssistantError OR
ServiceValidationError per HA docs. The check accepted only HAE,
so a future handler that legitimately raises only SVE for user-
input validation would fail the test. Now accepts both via a
frozenset.
2. ClassDef wasn't in the prune list. A handler with an inline
helper class whose method raises HAE/SVE would wrongly satisfy
the check for the outer handler. Added to the skip set.
3. Sync function handlers (def handle_x, not async def) were missed
entirely. The iter helper only yielded AsyncFunctionDef. Now
yields both — a future non-async handler still gets validated.
Self-verified each: nested-class raise no longer counts toward the
outer handler (negative test in the commit msg below).
Negative test snippet:
async def handle_x(call):
class Helper:
def fail(self): raise HomeAssistantError('inner')
pass
DFS result: False ✓ (correctly does NOT count the class method's raise)
* test(silver): also detect bare-class raises (gemini #154 review-4)
raise HomeAssistantError (no parens) is valid Python and used in some
HA integrations. Prior code only matched ast.Call wrapping ast.Name —
the bare form was an ast.Name directly on node.exc.
Per gemini suggestion: unify both forms by unwrapping ast.Call to its
.func first, then doing the single ast.Name check at the end.
Self-verified: bare raise HomeAssistantError correctly detected.
5 tasks
Artic0din
added a commit
that referenced
this pull request
May 27, 2026
Constitution P17 — tests are part of the fix. The silver-checklist test for `handle_reset_today` (custom_components/pricehawk/__init__.py:264) only verifies that the handler raises HomeAssistantError syntactically via AST inspection. No existing test exercises the actual side effects of the service, so future refactors could regress the behaviour silently. Adds four targeted tests in tests/test_runtime_data.py covering the contract surface: - test_handle_reset_today_raises_HAE_when_no_entries — empty-entry-list contract (PR #152 beta.9 retro-review). - test_handle_reset_today_zeros_each_provider_daily_accumulators — registers two providers, asserts reset_daily fires on each. - test_handle_reset_today_persists_state_after_reset — asserts async_persist_state is awaited so the cleared accumulators survive an HA restart. - test_handle_reset_today_continues_when_one_provider_reset_raises — provider A raises, provider B still resets, persist still runs (pins the noqa: BLE001 — never sink the batch contract). Validation: - uv run ruff check tests/test_runtime_data.py CHANGELOG.md → All checks passed - uv run pyright tests/test_runtime_data.py → 0 errors (1 pre-existing HA-stub warning) - uv run pytest --tb=short -q tests/test_runtime_data.py → 13 passed Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Artic0din
added a commit
that referenced
this pull request
May 27, 2026
* test(services): cover handle_reset_today behaviour Constitution P17 — tests are part of the fix. The silver-checklist test for `handle_reset_today` (custom_components/pricehawk/__init__.py:264) only verifies that the handler raises HomeAssistantError syntactically via AST inspection. No existing test exercises the actual side effects of the service, so future refactors could regress the behaviour silently. Adds four targeted tests in tests/test_runtime_data.py covering the contract surface: - test_handle_reset_today_raises_HAE_when_no_entries — empty-entry-list contract (PR #152 beta.9 retro-review). - test_handle_reset_today_zeros_each_provider_daily_accumulators — registers two providers, asserts reset_daily fires on each. - test_handle_reset_today_persists_state_after_reset — asserts async_persist_state is awaited so the cleared accumulators survive an HA restart. - test_handle_reset_today_continues_when_one_provider_reset_raises — provider A raises, provider B still resets, persist still runs (pins the noqa: BLE001 — never sink the batch contract). Validation: - uv run ruff check tests/test_runtime_data.py CHANGELOG.md → All checks passed - uv run pyright tests/test_runtime_data.py → 0 errors (1 pre-existing HA-stub warning) - uv run pytest --tb=short -q tests/test_runtime_data.py → 13 passed Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(services): tighten handle_reset_today assertions + N802 rename Linus audit on PR #161: - Rename test_handle_reset_today_raises_HAE_when_no_entries → test_handle_reset_today_raises_home_assistant_error_when_no_entries (PEP 8 N802: no UPPERCASE inside snake_case identifiers). - Tighten the no-entries assertion from an OR-gated substring check to a single exact-contract match against the user-visible string ``"no PriceHawk entries with active runtime data"`` so copy regressions cannot slip past. - Hoist the repeated ``custom_components.pricehawk`` / ``custom_components.pricehawk.data`` / ``homeassistant.exceptions`` imports to module-level — the conftest registers HA mocks at collection time, so per-test shadow imports were noise. - Add MagicMock return annotation to ``_capture_reset_today_handler`` for type-consistency with ``_make_provider``. - Reflow the CHANGELOG entry to SemBr (one sentence per line). Validates: uv run ruff check tests/test_runtime_data.py + uv run pytest -q tests/test_runtime_data.py (13 passed). 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.
Recovery hook for the beta.7 AEMO row-type fix. The fix corrected the rate going forward, but DWT providers'
_import_cost_today_ccarried inflated values until midnight rollover.Adds
pricehawk.reset_todayservice. Zeros all providers' daily accumulators, persists state. No params. Generalizable for any future cost-math fix shipped mid-day.Test plan: 1140 passing.