Skip to content

Commit dcc6017

Browse files
chalfontchubbyclaudeCI
authored
fix(compare): stop the midnight tariff comparison changing the live plan's next decision (#5465)
* fix(compare): restore the plan in force's pre-clip copy after the tariff comparison Plan selection scores the pre-clip version of each plan (#4403), keeping the plan in force's copy in plan_preclip for the next re-plan to score as the previous plan. The tariff comparison runs calculate_plan() for every tariff, each replacing plan_preclip with that tariff's 48-hour plan, and restored the windows and limits but not plan_preclip. The first re-plan after the midnight comparison therefore scored the last tariff's plan as the plan in force: on 9 Oct 2026 (Sigenergy, IOG) live scored its previous plan at 300.97p where the same plan scores 30.71p without the comparison, and so switched plans regardless of metric_min_improvement_plan. Found with the forward log replay (#5363): running the comparison inside the replay reproduced the inflated score, and restoring each attribute the comparison changes, group by group, isolated plan_preclip. run_all() now saves and restores it with the windows; a test runs run_all() with a tariff that replaces it and checks it comes back (it fails without the fix). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(compare): save and restore a declared set of state, and plan each tariff from scratch The comparison kept a hand-written list of attributes to save and restore, and plan_preclip leaked because it was missing from it; end_record, the two rate_best_cost thresholds and plan_valid / plan_last_updated were missing too, so after the comparison the next cycle published the last tariff's thresholds and treated the restored plan as freshly computed. run_all() now saves and restores the attributes declared in COMPARE_PLAN_STATE, COMPARE_HARDWARE_STATE (also reset after each tariff) and the rest of COMPARE_RESTORED_STATE, with the car state in COMPARE_RESTORED_COPIES deep-copied as before. Each tariff now plans from scratch (plan_valid False before its run): scored against the previous tariff's plan, the keep-or-switch threshold in calculate_plan() could leave one tariff's result as another tariff's plan. Tests: run_all() with a tariff run that changes every saved attribute must restore all of them, reset the hardware values between tariffs and start each tariff with plan_valid False; and every attribute calculate_plan() assigns must be either in COMPARE_PLAN_STATE or listed as rebuilt every run, so the next new piece of plan state fails a test rather than leaking (checked: dropping end_record from the set fails it). T19 and T25 now check the declared sets rather than the old source text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(compare): keep a re-plan requested during the comparison, and restore state even if the config restore raises Two gaps a cold review found in the restore: - Restoring plan_valid put back the True from before the comparison, overwriting a re-plan requested while it ran (an HA event, the web UI or an agent tool changing a setting sets update_pending and clears plan_valid). The next run would then treat the pre-change plan as valid. plan_valid is now restored as it was only if no re-plan is pending. - fetch_config_options() runs first in the final restore; if it raised, none of the saved state was put back - the leak this branch fixes. The attribute restore now runs in a finally around the config restore, still last so it wins. The coverage check now reads calculate_plan() and every optimise pass with ast, so augmented, annotated and tuple assignments to self are found too (nothing new is uncovered today). New tests: a re-plan requested during the comparison leaves the plan invalid, and the plan is restored when the config restore raises; both fail without this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(compare): pin plan_valid and update_pending in the run_all() tests run_all() now ends with plan_valid = saved and not update_pending. The shared fixture starts with update_pending True and several earlier tests leave plan_valid True, so test_run_all_restores_state failed or passed depending on run order. Set a valid plan with no re-plan pending before it, and save and restore both flags in it and in test_run_all_restores_when_config_restore_raises, which left plan_valid False for every later test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: CI <ci@example.com>
1 parent 9e5f002 commit dcc6017

2 files changed

Lines changed: 281 additions & 108 deletions

File tree

‎apps/predbat/compare.py‎

Lines changed: 78 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,60 @@ def valid_compare_list(compare_list, log=None):
8686
return valid
8787

8888

89+
# What Compare.run_all() saves before comparing tariffs and restores afterwards: every tariff fetches, plans and
90+
# predicts on the live instance. The plan state is what calculate_plan() writes that outlives the run - leaving
91+
# plan_preclip as the last tariff's plan once made the first re-plan after midnight score the wrong plan as the
92+
# plan in force. Add any new plan state calculate_plan() keeps across runs here.
93+
COMPARE_PLAN_STATE = (
94+
"charge_window_best",
95+
"charge_limit_best",
96+
"export_window_best",
97+
"export_limits_best",
98+
"plan_preclip",
99+
"end_record",
100+
"plan_valid",
101+
"plan_last_updated",
102+
"plan_last_updated_minutes",
103+
"rate_best_cost_threshold_charge",
104+
"rate_best_cost_threshold_export",
105+
)
106+
# Battery and inverter values a tariff can override, reset after each tariff as well as at the end
107+
COMPARE_HARDWARE_STATE = ("soc_max", "battery_rate_max_charge", "battery_rate_max_charge_dc", "battery_rate_max_discharge", "battery_rate_max_export", "inverter_limit")
108+
COMPARE_RESTORED_STATE = (
109+
COMPARE_PLAN_STATE
110+
+ COMPARE_HARDWARE_STATE
111+
+ (
112+
"forecast_plan_hours",
113+
"forecast_minutes",
114+
"forecast_days",
115+
"manual_charge_times",
116+
"manual_export_times",
117+
"manual_freeze_charge_times",
118+
"manual_freeze_export_times",
119+
"manual_demand_times",
120+
"manual_all_times",
121+
"cost_today_sofar",
122+
"carbon_today_sofar",
123+
"iboost_today",
124+
"iboost_plan",
125+
"import_today_now",
126+
"export_today_now",
127+
"octopus_intelligent_charging",
128+
"soc_kw",
129+
)
130+
)
131+
# Car state the comparison rebuilds per tariff, deep-copied as the lists are changed in place
132+
COMPARE_RESTORED_COPIES = (
133+
"car_charging_plan_smart",
134+
"car_charging_limit",
135+
"car_charging_limit_model",
136+
"car_charging_soc",
137+
"car_charging_battery_size",
138+
"car_charging_slots",
139+
"car_charging_now_slots",
140+
)
141+
142+
89143
class Compare:
90144
"""Tariff comparison engine.
91145
@@ -667,40 +721,9 @@ def run_all(self, debug=False, fetch_sensor=True):
667721

668722
my_predbat = self.pb
669723

670-
save_forecast_plan_hours = my_predbat.forecast_plan_hours
671-
save_forecast_minutes = my_predbat.forecast_minutes
672-
save_forecast_days = my_predbat.forecast_days
673-
save_manual_charge_times = my_predbat.manual_charge_times
674-
save_manual_export_times = my_predbat.manual_export_times
675-
save_manual_freeze_charge_times = my_predbat.manual_freeze_charge_times
676-
save_manual_freeze_export_times = my_predbat.manual_freeze_export_times
677-
save_manual_demand_times = my_predbat.manual_demand_times
678-
save_manual_all_times = my_predbat.manual_all_times
679-
save_charge_window_best = my_predbat.charge_window_best
680-
save_export_window_best = my_predbat.export_window_best
681-
save_export_limits_best = my_predbat.export_limits_best
682-
save_charge_limit_best = my_predbat.charge_limit_best
683-
save_cost_today_sofar = my_predbat.cost_today_sofar
684-
save_carbon_today_sofar = my_predbat.carbon_today_sofar
685-
save_iboost_today = my_predbat.iboost_today
686-
save_iboost_plan = my_predbat.iboost_plan
687-
save_import_today_now = my_predbat.import_today_now
688-
save_export_today_now = my_predbat.export_today_now
689-
save_octopus_intelligent_charging = my_predbat.octopus_intelligent_charging
690-
save_car_charging_plan_smart = copy.deepcopy(my_predbat.car_charging_plan_smart)
691-
save_car_charging_limit = copy.deepcopy(my_predbat.car_charging_limit)
692-
save_car_charging_limit_model = copy.deepcopy(my_predbat.car_charging_limit_model)
693-
save_car_charging_soc = copy.deepcopy(my_predbat.car_charging_soc)
694-
save_car_charging_battery_size = copy.deepcopy(my_predbat.car_charging_battery_size)
695-
save_car_charging_slots = copy.deepcopy(my_predbat.car_charging_slots)
696-
save_car_charging_now_slots = copy.deepcopy(my_predbat.car_charging_now_slots)
697-
save_soc_kw = my_predbat.soc_kw
698-
save_soc_max = my_predbat.soc_max
699-
save_battery_rate_max_charge = my_predbat.battery_rate_max_charge
700-
save_battery_rate_max_charge_dc = my_predbat.battery_rate_max_charge_dc
701-
save_battery_rate_max_discharge = my_predbat.battery_rate_max_discharge
702-
save_battery_rate_max_export = my_predbat.battery_rate_max_export
703-
save_inverter_limit = my_predbat.inverter_limit
724+
# Everything the comparison changes on the live instance, saved here and put back in the finally below
725+
saved = {name: getattr(my_predbat, name) for name in COMPARE_RESTORED_STATE}
726+
saved.update({name: copy.deepcopy(getattr(my_predbat, name)) for name in COMPARE_RESTORED_COPIES})
704727

705728
# Final reports, cut end_record back to 24 hours to ignore the dump at end of day
706729
end_record = int((my_predbat.minutes_now + 24 * 60 + 29) / 30) * 30 - my_predbat.minutes_now
@@ -748,19 +771,18 @@ def run_all(self, debug=False, fetch_sensor=True):
748771
# First ever run for this tariff: start from actual midnight SoC
749772
start_soc = soc_midnight_fallback
750773
self.log("Compare tariff {} starting SoC: {:.2f} kWh".format(tariff_id, start_soc))
774+
# Each tariff plans from scratch: scored against the previous tariff's plan, the keep-or-switch threshold
775+
# in calculate_plan() could leave one tariff's result as another tariff's plan
776+
my_predbat.plan_valid = False
751777
try:
752-
result_data = self.run_single(tariff, rate_import_base, rate_export_base, end_record, debug=debug, fetch_sensor=fetch_sensor, car_charging_slots=save_car_charging_slots, start_soc=start_soc, io_adjusted_base=io_adjusted_base)
778+
result_data = self.run_single(tariff, rate_import_base, rate_export_base, end_record, debug=debug, fetch_sensor=fetch_sensor, car_charging_slots=saved["car_charging_slots"], start_soc=start_soc, io_adjusted_base=io_adjusted_base)
753779
if result_data is not None:
754780
results[tariff_id] = result_data
755781
finally:
756782
# Restore hardware settings after each tariff so overrides don't bleed into the next tariff;
757783
# in a finally so a raised run_single() can't leave compare values applied to the live instance
758-
my_predbat.soc_max = save_soc_max
759-
my_predbat.battery_rate_max_charge = save_battery_rate_max_charge
760-
my_predbat.battery_rate_max_charge_dc = save_battery_rate_max_charge_dc
761-
my_predbat.battery_rate_max_discharge = save_battery_rate_max_discharge
762-
my_predbat.battery_rate_max_export = save_battery_rate_max_export
763-
my_predbat.inverter_limit = save_inverter_limit
784+
for name in COMPARE_HARDWARE_STATE:
785+
setattr(my_predbat, name, saved[name])
764786
# Restore config: overrides after each tariff too, for the same reason - otherwise a
765787
# tariff's config override stays applied to config_index for every subsequent tariff,
766788
# even ones with no config block of their own (issue #4156)
@@ -776,47 +798,18 @@ def run_all(self, debug=False, fetch_sensor=True):
776798
self.save_yaml()
777799
self.publish_data()
778800
finally:
779-
# Restore config values overridden by any tariff's fetch_config() call
780-
if config_snapshot:
781-
for key, orig_value in config_snapshot.items():
782-
item = my_predbat.config_index.get(key)
783-
if item is not None:
784-
item["value"] = orig_value
785-
my_predbat.fetch_config_options()
786-
787-
# Restore original settings
788-
my_predbat.forecast_plan_hours = save_forecast_plan_hours
789-
my_predbat.forecast_minutes = save_forecast_minutes
790-
my_predbat.forecast_days = save_forecast_days
791-
my_predbat.manual_charge_times = save_manual_charge_times
792-
my_predbat.manual_export_times = save_manual_export_times
793-
my_predbat.manual_freeze_charge_times = save_manual_freeze_charge_times
794-
my_predbat.manual_freeze_export_times = save_manual_freeze_export_times
795-
my_predbat.manual_demand_times = save_manual_demand_times
796-
my_predbat.manual_all_times = save_manual_all_times
797-
my_predbat.charge_window_best = save_charge_window_best
798-
my_predbat.export_window_best = save_export_window_best
799-
my_predbat.export_limits_best = save_export_limits_best
800-
my_predbat.charge_limit_best = save_charge_limit_best
801-
my_predbat.cost_today_sofar = save_cost_today_sofar
802-
my_predbat.carbon_today_sofar = save_carbon_today_sofar
803-
my_predbat.iboost_today = save_iboost_today
804-
my_predbat.iboost_plan = save_iboost_plan
805-
my_predbat.import_today_now = save_import_today_now
806-
my_predbat.export_today_now = save_export_today_now
807-
my_predbat.octopus_intelligent_charging = save_octopus_intelligent_charging
808-
my_predbat.car_charging_limit = save_car_charging_limit
809-
my_predbat.car_charging_limit_model = save_car_charging_limit_model
810-
my_predbat.car_charging_soc = save_car_charging_soc
811-
my_predbat.car_charging_battery_size = save_car_charging_battery_size
812-
my_predbat.car_charging_slots = save_car_charging_slots
813-
my_predbat.car_charging_now_slots = save_car_charging_now_slots
814-
my_predbat.car_charging_plan_smart = save_car_charging_plan_smart
815-
my_predbat.soc_kw = save_soc_kw
816-
my_predbat.soc_max = save_soc_max
817-
my_predbat.battery_rate_max_charge = save_battery_rate_max_charge
818-
my_predbat.battery_rate_max_charge_dc = save_battery_rate_max_charge_dc
819-
my_predbat.battery_rate_max_discharge = save_battery_rate_max_discharge
820-
my_predbat.battery_rate_max_export = save_battery_rate_max_export
821-
my_predbat.inverter_limit = save_inverter_limit
822-
my_predbat.io_adjusted = io_adjusted_base
801+
try:
802+
# Restore config values overridden by any tariff's fetch_config() call
803+
if config_snapshot:
804+
for key, orig_value in config_snapshot.items():
805+
item = my_predbat.config_index.get(key)
806+
if item is not None:
807+
item["value"] = orig_value
808+
my_predbat.fetch_config_options()
809+
finally:
810+
# Restore original settings, last so they win over fetch_config_options(), and even if it raised
811+
for name in COMPARE_RESTORED_STATE + COMPARE_RESTORED_COPIES:
812+
setattr(my_predbat, name, saved[name])
813+
# A re-plan requested while the comparison ran (a setting or override changed) must still happen
814+
my_predbat.plan_valid = saved["plan_valid"] and not my_predbat.update_pending
815+
my_predbat.io_adjusted = io_adjusted_base

0 commit comments

Comments
 (0)