fix(planner): degenerate candidate fallback no longer silently selects no_action - #904
Merged
Merged
Conversation
…s no_action - Fixes #897 _find_by_name(candidates, CANDIDATE_BASELINE) always returned None in production because generate_candidates() never emitted a "baseline" candidate — that heuristic candidate family was already disabled (MILP-only mode). The fallback silently dropped through to candidates[0], which is no_action, and unconditionally marked it valid — violating the spec invariant that no_action never becomes executable. The degenerate "no eligible candidates" branch now falls back to `passive`, the spec's designated executable fail-closed fallback, regardless of its own validation result. Also deletes the confirmed-dead legacy heuristic candidate family (_apply_soc_plan, _apply_aggressive_strategy, _remove_solar_charge, _remove_grid_charge, _remove_all_charge, and their CANDIDATE_* name constants) along with their direct unit tests, since MILP is the sole active optimisation authority and generate_candidates() only ever emits no_action, passive, and milp. Adds a regression test that calls the real generate_candidates() and forces the degenerate branch, asserting the winner is never no_action with is_valid=True.
woopstar
added
the
pr-on-hold
woopstar
marked this pull request as draft
September 3, 2026 06:30
…ndidate-fallback # Conflicts: # custom_components/hsem/planner/candidate_generator.py # tests/planner/test_candidate_generation.py # tests/planner/test_milp_optimizer.py
woopstar
added
major
woopstar
marked this pull request as ready for review
September 11, 2026 10:41
woopstar
pushed a commit
that referenced
this pull request
Sep 11, 2026
Brings the battery-schedule removal branch up to date with ~48 commits on main so it can merge cleanly as part of the 7.0.0 release batch alongside #910 and #904. Conflict resolution: - coordinator_data.py: kept main's current_wait_mode_reserve docstring (issue #914) alongside this branch's already-trimmed docstring; dropped the batteries_schedules doc lines this branch removes. - custom_sensors/config_reader.py: dropped main's still-present build_battery_schedules() (removed by this branch). - custom_sensors/working_mode_sensor.py: kept main's _write_phase_active try/finally wrapper (issue #951) and wait_mode_reserve_kwh threading (issue #914), while applying this branch's 3-arg resolve_current_recommendation() signature (drops the batteries_schedules_remaining_capacity_needed argument this branch removes from recommendation_resolver.py). - translations/da.json: kept main's new unrelated error keys (invalid_wait_mode_behavior, port_conflict, ev_min_power_below_start_current) and dropped the batteries_schedules-only keys/step, matching what this branch already did to en.json. - Modify/delete conflicts (models/battery_schedule.py, tests/planner/test_48h_second_day.py, tests/planner/test_arbitrage_grid_charge.py): took the deletion in all three cases; main's own concurrent edits there were trivial dead-field/ dead-kwarg cleanups (#966, #974) that are moot once the file is removed. Translation sync: removed the now-dead batteries_schedule_* step/entity keys and the schedule-only error keys from de.json and es.json (added by main after this branch's fork point, so they never got this branch's original en.json/da.json cleanup). validate_translations.py now reports 0 missing/stale/placeholder-mismatch keys across en/da/de/es. Also fixes two pre-existing, unrelated breakages inherited from main that blocked a clean quality gate on this branch tip: - tests/planner/test_invariants.py passed a stale is_read_only=True to PlannerInput; main's #974 vulture cleanup (ced164f) removed that dead field from the model entirely, and this branch's own rewritten conversion-loss test happened to still use it. - tests/test_coordinator_tracking_forecast.py and tests/test_coordinator_tracking_solar_corrector.py (both new on main, issues #972/#973) called accumulate_forecast_actuals() with a signature that doesn't match its current implementation (solar_corrector_processed no longer exists; update_interval_minutes is required) -- 4 tests were failing at runtime on main's own tip, independent of this merge. Also repointed one stray generic-example battery-schedule key in tests/test_platform_entity_refactor.py to hsem_ev_deadline_time (the original PR's own "repoint schedule-key example data" pass missed this one occurrence), and updated docs/ev-charge-plan-setup.md's step-flow diagram to drop the removed batteries_schedule_1/2/3 step. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…drift Inherited from main via the merge: two tracking test files called accumulate_forecast_actuals() with a stale signature (a bogus solar_corrector_processed kwarg in one file, a missing required update_interval_minutes in the other) from #976's signature change. Confirmed pre-existing and unrelated to this PR's own diff — fixed here so this branch's typing/test gates are clean, matching #910's identical fix for the same root cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
woopstar
pushed a commit
that referenced
this pull request
Sep 11, 2026
Reconciles with #904 and #873, both merged to main ahead of this PR. #873 (refactor/860-remove-battery-schedules) deleted the battery-schedule switch/time entities (batteries_schedule_1/2/3 switches, schedule_N start/end times) that this branch had also touched to add per-entity _hsem_device dispatch. Resolution: dropped this branch's now-moot hsem_device=BATTERY_ENERGY additions for those entities in switch.py, time.py, and custom_times/description.py (including the now-unused schedule-key imports) since the entities themselves no longer exist. custom_switches/description.py and all four translation files merged cleanly with no manual intervention needed. Also repointed tests/test_device_split.py::test_switch_uses_description_hsem_device from the now-removed "hsem_batteries_enable_batteries_schedule_1" example key to the still-existing "hsem_dynamic_discharge_floor" switch, which is also mapped to HSEMDevice.BATTERY_ENERGY — same coverage, valid key. Full quality gate re-verified post-merge: lint clean, typing clean, quality clean, 3271 tests passed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
woopstar
pushed a commit
that referenced
this pull request
Sep 11, 2026
Reconciles with #904/#873/#910, all merged to main ahead of this PR. Conflicts resolved: - coordinator_cycle.py, coordinator_state.py, tests/test_ha_mock_integration.py: both sides added distinct new fields/state names — kept both. - utils/sensornames/ev.py: kept this branch's new get_ev_soc_economics_sensor_{unique_id,entity_id}() (and second-EV equivalents), but dropped get_ev_soc_economics_sensor_name() / get_ev_second_soc_economics_sensor_name() — main's #912 already retired the _name()-getter convention repo-wide in favour of _attr_translation_key, which these sensors already partially adopted. Integrated with #910's device split (issue #875): - ev_soc_economics_sensor.py / ev_second_soc_economics_sensor.py: removed the leftover self._name assignment and custom `name` property override (dead code once _attr_translation_key is the only naming mechanism — main's other EV sensors don't override `name` either), and added self._hsem_device = HSEMDevice.EV_PRIMARY / EV_SECONDARY so these two new sensors attach to the correct per-subsystem device instead of the default CONTROLLER device. - translations/en.json, da.json: dropped the redundant "2"/"EV 2" marker from ev_second_soc_economics's name to match #910's "device name already disambiguates primary/secondary" convention (identical to how ev_second_optimal_charging_plan reads the same as its primary sibling). - translations/de.json, es.json: added the ev_soc_economics / ev_second_soc_economics entries outright — these two locale files were added by #910/main after this branch forked, so they never got this PR's original en/da-only translation pass. - tests/sensors/test_ev_soc_economics_sensor.py: dropped the two now-invalid `sensor._name = ...` assignments in the object.__new__() test factories (mypy attr-defined errors) and set sensor._hsem_device explicitly instead, matching what the real __init__ now does. - tests/test_device_split.py: added test_ev_primary_soc_economics_sensor / test_ev_secondary_soc_economics_sensor to TestPerEntityDeviceInfo, covering the new sensors' device dispatch the same way the existing EV calculated-power-sensor tests do. Full quality gate re-verified post-merge: lint clean, typing clean, quality clean, translations 0 errors across en/da/de/es, 3320 tests passed (one MILP performance-budget test flaked once under load; confirmed unrelated to this merge, passes in isolation). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
woopstar
pushed a commit
that referenced
this pull request
Sep 13, 2026
…s from main The v6.3.0 tag shipped three test call-sites that no longer matched `coordinator_tracking.accumulate_forecast_actuals()`: one still passed the `solar_corrector_processed` argument dropped by #973, and two omitted the required keyword-only `update_interval_minutes`. Four tests error out on a clean v6.3.0 checkout, and mypy reports three call-arg errors. `coordinator_tracking.py` is byte-identical between v6.3.0 and main, so this is the exact test-only delta already on main (from #904) — no production behaviour is touched. It unblocks CI on the 6.3.x hotfix line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5 tasks
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
Follow-up from #891, resolved per issue #897.
docs/planner-spec.md:1813-1836("Candidate plans") lists onlyno_action,passive, andmilpas required production candidates, and statesno_actionmust never become executable.generate_candidates()matchesthis — the legacy heuristic candidate family was already disabled
(commented out) in MILP-only mode. But the degenerate "no eligible
candidates" fallback in
candidate_selector.pystill targetedCANDIDATE_BASELINE, a candidate name production never emits. Since_find_by_name()always returnedNone, the fallback silently dropped tocandidates[0]— which isno_action— and unconditionally setwinner.is_valid = True, violating the spec invariant.Fix
candidate_selector.py: the degenerate fallback now targetspassive(the spec's designated executable fail-closed fallback), accepted
regardless of its own validation result.
no_actionis used only as anabsolute last resort (and then only via
RuntimeError, never silentlymarked valid) when even
passiveis missing from the candidate list.TestDegenerateFallbackintests/planner/test_candidate_generation.py,which calls the real
generate_candidates()(not a hand-builtcandidate list) and forces every candidate to fail SoC validation via an
unreachable discharge floor, asserting the winner is
passiveand isnever
no_actionwithis_valid=True.Cleanup — confirmed-dead legacy heuristic family
Per
git log -S "MILP-only mode", disabling the heuristic candidates wasan intentional design change, not an oversight — MILP is the sole active
optimisation authority. Deleted:
_apply_soc_plan()(planner/candidates/_soc_plan.py, file removed)_apply_aggressive_strategy()(planner/candidates/_aggressive.py, file removed)_remove_solar_charge,_remove_grid_charge,_remove_all_charge(
planner/candidates/_mutations.py)CANDIDATE_BASELINE,CANDIDATE_SOC_*,_SOC_FRACTIONS,CANDIDATE_GRID_CHARGE,CANDIDATE_SOLAR_ONLY,CANDIDATE_DISCHARGE_ONLY,CANDIDATE_AGGRESSIVE(planner/candidate_generator.py)generate_candidates()tests/planner/test_candidate_generator_placement.py(deleted entirely — exclusively tested
_apply_aggressive_strategy), plusthe corresponding sections in
tests/planner/test_candidate_generation.pyand
tests/planner/test_milp_optimizer.py(the latter's unrelated MILPtests — cycle cost, mutex, main-fuse, etc. — are untouched).
tests/planner/test_hysteresis.pyusedCANDIDATE_BASELINEpurely as anarbitrary label for hand-built test plans (not real production output);
replaced with a local
_CANDIDATE_OTHERtest constant.Docs
docs/planner-spec.md: documented that the degenerate fallback resolvesto
passive, neverno_action.docs/candidate-generation.md: updated the candidate list, marked thehistorical heuristic family as removed (not just disabled), corrected the
no_actiondescription (it was documented as a fallback destination,which was the bug), and documented the degenerate-fallback resolution.
Related PR
#873 (
refactor/860-remove-battery-schedules) also touches the planner but is fully independent of this PR: zero file overlap (candidate_generator.py/candidate_selector.py/docs/candidate-generation.md/docs/planner-spec.mdhere vs.engine_core.py's schedule-application call sites and the battery-schedule config/entities/models there). Verified no merge conflicts either order; #873's.github/memories.mdnote about the "commented out" heuristic candidates has been updated there to point at this PR's full deletion.Test plan
./scripts/quality.sh lint— clean./scripts/quality.sh typing— 0 errors./scripts/quality.sh quality— 0 pyright errors (pre-existing vulturelow-confidence findings unrelated to this change)
./scripts/quality.sh test— 3023 passedtest_degenerate_fallback_never_selects_no_action)exercises the real
generate_candidates()output and confirms the fixFixes #897