fix(state): stop an options reload pinning the battery to MANUAL, make the override timeout reachable (#934) by jackmcintyre · Pull Request #1014 · jackmcintyre/localshift

jackmcintyre · 2026-09-06T21:26:36Z

The two defects behind the 27 August freeze, where the battery sat pinned in manual for ten hours, the optimizer went blind, computed_at froze, and every health sensor still read ok.

Restore write misclassified as a user pick. Reloading options re-creates the battery-mode select, and its restoration write fired the same handler as a human dropdown pick. A write is now classified as an internal re-assertion when it matches the persisted manual mode, automation is on, and the HA service context carries no user_id, which a frontend pick always does and a restore never does. Comparing against current_option instead was tried and rejected: it only caught the echo when the optimizer's live mode happened to equal the persisted one.
Unstamped override defeated the timeout. Every writer now routes through one stamped entry point on the coordinator, with INFO attribution for what entered manual. The timeout resolves either stamp, self-heals an unstamped override by arming the clock from the current tick, and critically now runs before the automation-disabled early return, since entering manual also turns automation off and the old order meant the timeout could never be reached at all.

The timeout notification also stops claiming automation is resuming when the switch is still off.

Closes #934.

Verification ruff check, ruff format, vulture and 3440 tests all green on this branch standalone.

Worth a close read on one behaviour change. A YAML automation that re-asserts the persisted manual mode while automation is on now carries no user context and will be ignored. If anything in your setup does that deliberately, this changes it.

Provenance Built by routed-build-auto run wf_716d0804-f3b across three implement passes with verify green each time and two review passes' findings addressed. The third review pass died on a session limit, so the final diff was never reviewed by the loop — the gates above were re-run by hand on this exact tree instead.

Merge order Touches state/machine.py and coordinator/data.py, shared with the #940/#941/#943 and #942/#944 PRs, so merge it after those two and rebase.

coderabbitai · 2026-09-06T21:26:52Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 7372f075-34c7-48e8-8edf-26eaf981afbb

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

…ry from_mode, and fix boundary_lag_history testing (#1048) Fix GitHub issues #966, #967, #989, #990 and #991 together: follow-ups from the boundary-lag telemetry work, which is already merged into main (PRs #1009 and #1010). #966: _pending_retry_mode is not cleared when a fresh decision token is granted (_apply_decision_token rewrites _last_grant_source but leaves the marker), nor in _handle_automation_disabled (sets _commanded_mode = MANUAL and returns early) nor in set_commanded_mode (manual button press). Clear it in all three so a genuinely fresh price grant after a failed attempt is tagged price, not retry; add tests for each path. #967: decision_lag_history still records from_mode from data.active_mode, the same dead field #940 fixed in boundary_lag_history; record from_mode from self._commanded_mode.value there too so the two rings agree, and update the decision_lag_history entry docs in custom_components/localshift/coordinator/data.py and docs/ENTITY_REFERENCE.md. #989: custom_components/localshift/sensors/status.py _flatten_boundary_lag_history's cross-bucket chronological merge is untested (mutation proof: deleting the sort passes the suite); add a test with two buckets carrying real interleaved interval_start_utc values asserting the merged output is chronological and the last-20 window crosses bucket boundaries. #990: _BOUNDARY_LAG_PER_SOURCE_CAP = 50 silently reversed a documented 200; raise it to 200 per source (about 1800 entries worst case, still trivial) and state in the comment why that size serves #510 slice 3's sample-size need. #991: tests/test_decision_lag.py fixtures at roughly lines 548 and 572 still construct boundary_lag_history=[] (flat list) after the field became dict[str, list]; they pass only because an empty list is falsy. Change them to {} and add one test that a non-empty dict flows through extra_state_attributes without error. Note: PRs #1014 and #1020 are open and unmerged and both rewrite parts of state/machine.py, so edits there are tightly scoped to the retry-marker clearing and the from_mode line. Coverage kept at or above 95 percent on touched modules. Review findings (not blocking): - [medium] tests/test_boundary_lag.py:941 (test_ties_broken_by_boundary_lag) — The test named and documented as pinning the tiebreak half of the sort key does not test it, and is mutation-blind. Its two entries are built with `_entry("price", 5, second=30)` and `_entry("backstop", 5, second=10)`, but both have the same interval_start_utc and source, so the tiebreak is never exercised; deleting the sort still passes. - [medium] custom_components/localshift/state/machine.py:80 (_BOUNDARY_LAG_PER_SOURCE_CAP) — The #990 cap is raised 50 -> 200 and the number is now hard-coded in two docs (coordinator/data.py docstring, docs/ENTITY_REFERENCE.md line 644), but nothing pins the value in a single place. - [low] custom_components/localshift/state/machine.py (module coverage) — The task set a >=95% per-module coverage bar on touched modules and the handback reports 'gates passed', but state/machine.py measures 93.43% (42 lines missing) on the full run; coordinator/data.py is 99.02% and sensors/status.py is 98.76%. - [low] tests/test_boundary_lag.py:1034, tests/test_decision_lag.py:253 — Both touched test files were ruff format clean at the base commit (verified against copies of 2056492) and are now unformatted: the dt_aware(...) wraps do not follow the column alignment rule. - [info] custom_components/localshift/state/machine.py:73-79 (cap comment) — The comment justifying 200 says the ring is 'never serialised in full since the sensor attribute exposes only the flattened last 20'. Serialisation is indeed bounded, but _flatten_boundary_lag_history chains and sorts all buckets before taking the slice, so the worst case still needs space for all 200 per-source entries.

…e the override timeout reachable (#934) Two defects from the 27 Aug 10-hour freeze (optimizer blind, computed_at frozen, health sensors ok). - Restore write misclassified as a user pick: BatteryModeSelect now classifies a write as an internal re-assertion when the requested option equals the PERSISTED manual mode, automation is currently on, and the HA service context carries no user_id (a frontend pick always does; an options-reload / entity-recreation restore does not). Such writes are logged at WARNING and ignored instead of entering manual override. Comparing against current_option was tried and rejected: it only caught the echo when the optimizer's live mode happened to equal the persisted one. - Unstamped override defeats the timeout: LocalShiftCoordinator gains a single stamped entry point, set_manual_override(active, reason=...), which every writer (user pick, automatic, startup sync) now routes through; it stamps data.manual_override_set_at and mirrors it onto the state machine, with INFO attribution for what entered manual. _handle_manual_override_timeout resolves either stamp, self-heals an unstamped override by arming the clock from the current tick, and now runs BEFORE the automation-disabled early return (entering manual also turns automation off, so the old order could never reach the timeout). The timeout notification reports the automation switch's real state rather than claiming automation is resuming. Behaviour note for review: a YAML automation re-asserting the persisted manual mode while automation is on is now ignored (no user context). Built by routed-build-auto run wf_716d0804-f3b: three implement passes, verify green each time, two review passes' findings addressed; the third review pass died on the Claude session limit. Gates re-run by hand on this exact tree: ruff clean, 3440 tests passed.

This was referenced Sep 7, 2026

fix: run recompute-and-evaluate once per entity write, not twice (#975) #1041

Merged

fix(telemetry): clear the retry marker on every reset path, align decision_lag from_mode, deepen the per-source ring (#966, #967, #989, #990, #991) #1048

Merged

jackmcintyre force-pushed the wf/934-manual-override-restore-echo branch from aca9822 to 6382806 Compare September 8, 2026 04:37

jackmcintyre force-pushed the wf/934-manual-override-restore-echo branch from 6382806 to 30b07fc Compare September 8, 2026 04:41

jackmcintyre merged commit 8c28bef into main Sep 8, 2026
5 checks passed

jackmcintyre deleted the wf/934-manual-override-restore-echo branch September 8, 2026 04:46

Navigation Menu

fix(state): stop an options reload pinning the battery to MANUAL, make the override timeout reachable (#934) - #1014

fix(state): stop an options reload pinning the battery to MANUAL, make the override timeout reachable (#934)#1014
jackmcintyre merged 1 commit into
mainfrom
wf/934-manual-override-restore-echo

jackmcintyre commented Sep 6, 2026

Uh oh!

coderabbitai Bot commented Sep 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jackmcintyre commented Sep 6, 2026

Uh oh!

coderabbitai Bot commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Sep 6, 2026 •

edited

Loading