From 8a7f130e9a77fb22b88c37bd9976b10715c4306c Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 11 Sep 2026 11:17:20 +0100 Subject: [PATCH 1/9] fix(fetch): exclude saving-session/Axle boosted minutes from automatic rate thresholds (#5050) A saving session / Axle VPP event boosts both the import and export rate tables by the event reward, tagging those minutes "saving" in rate_import_replicated/rate_export_replicated (load_saving_slot() in octopus.py, load_axle_slot() in axle.py). In automatic threshold mode (rate_low_threshold=0), set_rate_thresholds() computed rate_import_cost_threshold from self.rate_max, which includes those event-boosted minutes. On a two-rate tariff (25.95p day / 3.49p night) plus a +100p saving-session event, that pushed the threshold to 125.45p, and rate_scan_window() classified the whole ordinary-price day as "low rate" - binary_sensor.predbat_low_rate_slot stuck ON for ~24h, through the genuine 3.49p night window and the whole expensive day rate, as reported. Added rate_minmax_excluding_saving() and used it for the threshold-stat inputs in set_rate_thresholds() only. self.rate_min/rate_max/rate_average (and the export equivalents) are deliberately left untouched - they feed dashboard sensors, graph scaling, and plan.py pricing, where the real boosted price is exactly what should be shown; this is a narrower, separate scan rather than a change to those. Verified against the exact triage reproduction (48h two-rate tariff, +100p event 17:00-19:00, rate_low_threshold=0): unfixed, rate_import_cost_threshold=125.45 and the 25.95p day blocks come back as low-rate windows; fixed, threshold=25.45 (true day rate - 0.5) and only the genuine 3.49p night rate qualifies. Confirmed the new tests fail against the unfixed wiring and pass with the fix. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/fetch.py | 55 +++++- .../predbat/tests/test_set_rate_thresholds.py | 157 ++++++++++++++++++ apps/predbat/unit_test.py | 2 + 3 files changed, 206 insertions(+), 8 deletions(-) create mode 100644 apps/predbat/tests/test_set_rate_thresholds.py diff --git a/apps/predbat/fetch.py b/apps/predbat/fetch.py index 4ab7e5ecd..39a80ff66 100644 --- a/apps/predbat/fetch.py +++ b/apps/predbat/fetch.py @@ -2272,6 +2272,37 @@ def rate_minmax(self, rates): return dp2(rate_min), dp2(rate_max), dp2(rate_average), rate_min_minute, rate_max_minute + def rate_minmax_excluding_saving(self, rates, rate_replicate): + """ + Work out min/max/average over the forecast window, skipping any minute rate_replicate tags + "saving" - a saving session / Axle VPP event reward baked into the rate table by + load_saving_slot()/load_axle_slot() (GH#5050). Those minutes are a synthetic one-off price, + not the tariff's own rate, so including them in the automatic threshold stats (rate_max used + by set_rate_thresholds() to pick rate_max - 0.5) can push the threshold above every genuine + tariff rate: rate_scan_window() then classifies the whole ordinary-price day as "low rate". + + Falls back to the plain (unfiltered) min/max/average when every minute in range is tagged + "saving" - an event that covers the whole forecast window leaves no genuine tariff minute to + scan, and a 99999/0/0 result would make every downstream comparison in set_rate_thresholds() + behave as if there were no data at all, which is worse than the boosted-but-real numbers. + """ + rate_min = 99999 + rate_max = 0 + rate_total = 0 + rate_n = 0 + + for minute in range(self.minutes_now, self.forecast_minutes + self.minutes_now): + if minute in rates and rate_replicate.get(minute) != "saving": + rate = rates[minute] + rate_min = min(rate_min, rate) + rate_max = max(rate_max, rate) + rate_total += rate + rate_n += 1 + + if rate_n: + return dp2(rate_min), dp2(rate_max), dp2(rate_total / rate_n) + return self.rate_minmax(rates)[:3] + def rate_base_min_max(self, rates): """ Gap-fill `rates` into a "base" import curve (replicated, but without IO-slot/saving-session/ @@ -2357,14 +2388,22 @@ def set_rate_thresholds(self): car_planning_on_rates = self.num_cars > 0 and not self.octopus_intelligent_charging car_charging_max_price = max(self.car_charging_plan_max_price[: self.num_cars]) if car_planning_on_rates else 0.0 + # Threshold stats are computed over the tariff's own rates, excluding minutes a saving + # session / Axle VPP event boosted (GH#5050) - self.rate_min/rate_max/rate_average (and the + # export equivalents) include those synthetic minutes and are used elsewhere (dashboard + # sensors, graph scaling, plan.py pricing) where the real boosted price is exactly what is + # wanted, so this is a separate, narrower scan rather than a change to those. + rate_min, rate_max, rate_average = self.rate_minmax_excluding_saving(self.rate_import, self.rate_import_replicated) + rate_export_min, rate_export_max, rate_export_average = self.rate_minmax_excluding_saving(self.rate_export, self.rate_export_replicated) + if self.rate_low_threshold > 0: - self.rate_import_cost_threshold = dp2(self.rate_average * self.rate_low_threshold) + self.rate_import_cost_threshold = dp2(rate_average * self.rate_low_threshold) else: # In automatic mode select the only rate or everything but the most expensive - if (self.rate_max == self.rate_min) or (self.rate_export_max > self.rate_max) or have_alerts or have_manual_soc: - self.rate_import_cost_threshold = self.rate_max + 0.1 + if (rate_max == rate_min) or (rate_export_max > rate_max) or have_alerts or have_manual_soc: + self.rate_import_cost_threshold = rate_max + 0.1 else: - self.rate_import_cost_threshold = self.rate_max - 0.5 + self.rate_import_cost_threshold = rate_max - 0.5 # When we plan car on the rates we need to include all the rates up to the max car price if car_planning_on_rates: @@ -2372,13 +2411,13 @@ def set_rate_thresholds(self): # Compute the export rate threshold if self.rate_high_threshold > 0: - self.rate_export_cost_threshold = dp2(self.rate_export_average * self.rate_high_threshold) + self.rate_export_cost_threshold = dp2(rate_export_average * self.rate_high_threshold) else: # In automatic mode select the only rate or everything but the most cheapest - if (self.rate_export_max == self.rate_export_min) or (self.rate_export_min > self.rate_min) or have_alerts: - self.rate_export_cost_threshold = self.rate_export_min - 0.1 + if (rate_export_max == rate_export_min) or (rate_export_min > rate_min) or have_alerts: + self.rate_export_cost_threshold = rate_export_min - 0.1 else: - self.rate_export_cost_threshold = self.rate_export_min + 0.5 + self.rate_export_cost_threshold = rate_export_min + 0.5 self.log( "Rate thresholds (for charge/export) are import {}{} ({}{}), export {}{} ({}{})".format( diff --git a/apps/predbat/tests/test_set_rate_thresholds.py b/apps/predbat/tests/test_set_rate_thresholds.py new file mode 100644 index 000000000..c298d95a2 --- /dev/null +++ b/apps/predbat/tests/test_set_rate_thresholds.py @@ -0,0 +1,157 @@ +# ----------------------------------------------------------------------------- +# Predbat Home Battery System +# Copyright Trefor Southwell 2026 - All Rights Reserved +# This application maybe used for personal use only and not for commercial use +# ----------------------------------------------------------------------------- +# fmt: off +# pylint: disable=consider-using-f-string +# pylint: disable=line-too-long +# pylint: disable=attribute-defined-outside-init +"""Tests for set_rate_thresholds and rate_minmax_excluding_saving (GH#5050). + +A saving session / Axle VPP event boosts both the import and export rate tables by the event +reward and tags those minutes "saving" in rate_import_replicated/rate_export_replicated +(load_saving_slot() in octopus.py, load_axle_slot() in axle.py). In automatic threshold mode +(rate_low_threshold=0) set_rate_thresholds() used to compute rate_import_cost_threshold from +self.rate_max, which includes those event-boosted minutes - so a two-rate tariff (25.95p day, +3.49p night) plus a +100p event pushed the threshold to 125.45p, and rate_scan_window() +classified the whole ordinary-price day as "low rate" (binary_sensor.predbat_low_rate_slot stuck +ON for ~24h, the reported symptom). + +set_rate_thresholds() now derives its threshold stats from rate_minmax_excluding_saving(), which +scans the same rates but skips any minute tagged "saving". self.rate_max/rate_min/rate_average +(and the export equivalents) are deliberately left untouched - they feed dashboard sensors, graph +scaling, and plan.py pricing, where the real boosted price is what should be shown. +""" + + +def _setup_two_rate_tariff(my_predbat, event_start, event_end, event_boost=100.0): + """Build a 48h two-rate import tariff (25.95p day 06:00-22:00, 3.49p night) plus a flat 15.0p + export tariff, with a saving-session-style boost applied to import over [event_start, event_end) + and tagged "saving" in rate_import_replicated - the shape load_saving_slot()/load_axle_slot() + produce.""" + my_predbat.minutes_now = 0 + my_predbat.forecast_minutes = 24 * 60 + + rate_import = {} + for minute in range(0, 48 * 60): + hour = (minute // 60) % 24 + rate_import[minute] = 25.95 if 6 <= hour < 22 else 3.49 + + rate_import_replicated = {} + for minute in range(event_start, event_end): + rate_import[minute] += event_boost + rate_import_replicated[minute] = "saving" + + rate_export = {minute: 15.0 for minute in range(0, 48 * 60)} + + my_predbat.rate_import = rate_import + my_predbat.rate_import_replicated = rate_import_replicated + my_predbat.rate_export = rate_export + my_predbat.rate_export_replicated = {} + + my_predbat.rate_min, my_predbat.rate_max, my_predbat.rate_average, _, _ = my_predbat.rate_minmax(rate_import) + my_predbat.rate_export_min, my_predbat.rate_export_max, my_predbat.rate_export_average, _, _ = my_predbat.rate_minmax(rate_export) + + my_predbat.rate_low_threshold = 0 + my_predbat.rate_high_threshold = 0 + my_predbat.alert_active_keep = {} + my_predbat.manual_soc_keep = {} + my_predbat.num_cars = 0 + + return rate_import, rate_import_replicated + + +def test_rate_minmax_excluding_saving_skips_the_boosted_minutes(my_predbat): + """rate_minmax_excluding_saving must report the tariff's own min/max/average, not the + event-inflated figures - reproducing the exact numbers from the GH#5050 triage.""" + print("**** test_rate_minmax_excluding_saving_skips_the_boosted_minutes ****") + failed = False + + rate_import, rate_import_replicated = _setup_two_rate_tariff(my_predbat, event_start=17 * 60, event_end=19 * 60) + + if my_predbat.rate_max != 125.95: + print("ERROR: test setup sanity check failed - contaminated rate_max should be 125.95, got {}".format(my_predbat.rate_max)) + failed = True + + rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_replicated) + if rate_min != 3.49: + print("ERROR: saving-excluded rate_min should be 3.49, got {}".format(rate_min)) + failed = True + if rate_max != 25.95: + print("ERROR: saving-excluded rate_max should be 25.95 (the true day rate), got {}".format(rate_max)) + failed = True + if abs(rate_average - 17.78) > 0.01: + print("ERROR: saving-excluded rate_average should be ~17.78, got {}".format(rate_average)) + failed = True + + if not failed: + print("PASS") + return failed + + +def test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged(my_predbat): + """If an event covers the whole forecast window there is no genuine tariff minute to scan - + fall back to the plain min/max/average rather than returning a 99999/0/0 that would make every + downstream comparison in set_rate_thresholds() behave as if there were no data at all.""" + print("**** test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged ****") + failed = False + + rate_import, rate_import_replicated = _setup_two_rate_tariff(my_predbat, event_start=0, event_end=my_predbat.forecast_minutes) + + rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_replicated) + expected_min, expected_max, expected_average, _, _ = my_predbat.rate_minmax(rate_import) + if (rate_min, rate_max, rate_average) != (expected_min, expected_max, expected_average): + print("ERROR: fully-tagged window should fall back to the plain scan {}, got {}".format((expected_min, expected_max, expected_average), (rate_min, rate_max, rate_average))) + failed = True + + if not failed: + print("PASS") + return failed + + +def test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat): + """End-to-end: with rate_low_threshold=0 (automatic mode), a saving-session boost must not + raise the low-rate threshold above the tariff's own day rate - the exact mechanism reported in + GH#5050 (binary_sensor.predbat_low_rate_slot stuck ON through the day rate for ~24h).""" + print("**** test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode ****") + failed = False + + _setup_two_rate_tariff(my_predbat, event_start=17 * 60, event_end=19 * 60) + + my_predbat.set_rate_thresholds() + + # rate_max - 0.5 on the true day rate, not the event-boosted one: 25.95 - 0.5 = 25.45. + # The unfixed code produced rate_max(125.95) - 0.5 = 125.45, which classified the whole day as low. + expected_threshold = 25.45 + if abs(my_predbat.rate_import_cost_threshold - expected_threshold) > 0.01: + print("ERROR: rate_import_cost_threshold should be {} (true day rate - 0.5), got {} (GH#5050)".format(expected_threshold, my_predbat.rate_import_cost_threshold)) + failed = True + + found, lowest, highest = my_predbat.rate_scan_window(my_predbat.rate_import, 5, my_predbat.rate_import_cost_threshold, False) + window_averages = sorted(set(window["average"] for window in found)) + if 25.95 in window_averages: + print("ERROR: the 25.95p day rate was classified as a low-rate window - binary_sensor.predbat_low_rate_slot would misfire (GH#5050)".format()) + failed = True + if window_averages != [3.49]: + print("ERROR: expected only the genuine 3.49p night rate to qualify as low-rate, got window averages {}".format(window_averages)) + failed = True + + # rate_max/rate_min/rate_average themselves are deliberately left contaminated - other code + # (dashboard sensors, graph scaling, plan.py pricing) wants the real boosted price. + if my_predbat.rate_max != 125.95: + print("ERROR: rate_max should be left untouched at 125.95 for downstream consumers, got {}".format(my_predbat.rate_max)) + failed = True + + if not failed: + print("PASS") + return failed + + +def run_set_rate_thresholds_tests(my_predbat): + """Run the set_rate_thresholds / rate_minmax_excluding_saving tests""" + failed = False + failed |= test_rate_minmax_excluding_saving_skips_the_boosted_minutes(my_predbat) + failed |= test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged(my_predbat) + failed |= test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat) + return failed diff --git a/apps/predbat/unit_test.py b/apps/predbat/unit_test.py index 2bedef581..4616e6a08 100644 --- a/apps/predbat/unit_test.py +++ b/apps/predbat/unit_test.py @@ -52,6 +52,7 @@ from tests.test_basic_rates import test_basic_rates from tests.test_rate_export_max_forward_calc import test_rate_export_max_forward_calc from tests.test_rate_min_forward_calc import test_rate_min_forward_calc +from tests.test_set_rate_thresholds import run_set_rate_thresholds_tests from tests.test_find_charge_curve import run_find_charge_curve_tests from tests.test_find_battery_size import run_find_battery_size_tests from tests.test_optimise_all_windows import run_optimise_all_windows_kernel_tests @@ -435,6 +436,7 @@ def main(): ("charge_hold", run_charge_hold_tests, "Charge freeze hold modelling tests", False), ("basic_rates", test_basic_rates, "Basic rates tests", False), ("rate_min_forward_calc", test_rate_min_forward_calc, "Rate min forward calc tests", False), + ("set_rate_thresholds", run_set_rate_thresholds_tests, "Automatic rate threshold tests - saving session / Axle boost exclusion (#5050)", False), ("rate_export_max_forward_calc", test_rate_export_max_forward_calc, "Rate export max forward calc tests", False), ("window_sort", run_window_sort_tests, "Window sort tests", False), ("window2minutes", test_window2minutes, "Window to minutes tests", False), From 33897cc0f8fd6bda3d4400f24ed80e3dcc638169 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 11 Sep 2026 20:58:55 +0100 Subject: [PATCH 2/9] fix(fetch): only exclude inflated "saving"-tagged minutes from threshold stats (Copilot review on #5052) The "saving" tag on rate_replicate marks two economically opposite things with the same string: a saving-session/Axle event REWARD (load_saving_slot/load_axle_slot's export/reward branches, the #5050 case - a synthetic high that should not raise the automatic threshold), and a free/discounted import session (load_free_slot, load_axle_slot's import branch - a genuinely cheap slot the automatic threshold should be free to pick as "low rate"). rate_minmax_excluding_saving() excluded every "saving"-tagged minute regardless of which. With a flat tariff plus one free slot, that left only the flat rate - rate_max == rate_min - and set_rate_thresholds() took its "everything but the most expensive" branch, setting the threshold above every rate and making the whole ordinary-price day read as low-rate. Reproduced exactly with that scenario before fixing. Now two passes: the first, over untagged minutes only, sets a baseline max. The second excludes a "saving"-tagged minute only if its rate exceeds that baseline - the one shape a reward produces - and admits every other minute, tagged or not, including a "saving"-tagged discount at or below the baseline. Verified both scenarios hold simultaneously: the free-slot case now correctly gives rate_min=0.0/rate_max=20.0 (not 20.0/20.0), and the original #5050 two-rate-plus event-boost scenario still gives rate_max=25.95, not the inflated 125.95. Also fixed test_set_rate_thresholds.py's fixture leak (same review): _setup_two_rate_tariff() overwrote minutes_now, the rate tables/statistics, thresholds and num_cars directly on the shared PredBat fixture with no restore - unit_test.py passes one instance through the whole registry, so a later test would inherit this group's synthetic 48h tariff and faked midnight. Snapshot/restore around the whole group in a try/finally. Both mutation-tested: reverting the direction check reproduces the free-slot regression exactly; reverting the snapshot/restore reproduces the leak (minutes_now 720 -> 0 after the group runs). Co-Authored-By: Claude Sonnet 5 --- apps/predbat/fetch.py | 58 +++++++++---- .../predbat/tests/test_set_rate_thresholds.py | 86 +++++++++++++++++-- 2 files changed, 123 insertions(+), 21 deletions(-) diff --git a/apps/predbat/fetch.py b/apps/predbat/fetch.py index 39a80ff66..a57a7d3c3 100644 --- a/apps/predbat/fetch.py +++ b/apps/predbat/fetch.py @@ -2274,34 +2274,62 @@ def rate_minmax(self, rates): def rate_minmax_excluding_saving(self, rates, rate_replicate): """ - Work out min/max/average over the forecast window, skipping any minute rate_replicate tags - "saving" - a saving session / Axle VPP event reward baked into the rate table by - load_saving_slot()/load_axle_slot() (GH#5050). Those minutes are a synthetic one-off price, - not the tariff's own rate, so including them in the automatic threshold stats (rate_max used - by set_rate_thresholds() to pick rate_max - 0.5) can push the threshold above every genuine - tariff rate: rate_scan_window() then classifies the whole ordinary-price day as "low rate". + Work out min/max/average over the forecast window, excluding only "saving"-tagged + minutes that are inflated relative to the tariff's own rates - not every minute the + "saving" tag touches. + + "saving" (set by load_saving_slot()/load_axle_slot()/load_free_slot()) marks two + economically opposite things with the same string: a saving-session/Axle event REWARD, + added on top of the tariff rate (GH#5050 - a synthetic one-off high that should not be + allowed to raise the automatic threshold above every genuine tariff rate), and a + free/discounted import session, which SUBTRACTS from or floors the rate (a genuinely + cheap slot the automatic threshold should be free to pick as "low rate"). Excluding every + "saving" minute regardless of direction throws the cheap ones away too - with a flat + tariff plus one free slot, that leaves only the flat rate, rate_max == rate_min, and + set_rate_thresholds() takes its rate_max + 0.1 branch, which sets the threshold above + every rate and makes the whole ordinary-price day read as "low" (Copilot review on + #5052 - reproduced with exactly that scenario before fixing). + + Two passes: the first pass over the untagged minutes alone sets a baseline max. The + second pass then excludes a "saving"-tagged minute only if its rate exceeds that + baseline max - the one shape (a reward) that can distort the automatic threshold - and + admits every other minute, tagged or not, including a "saving"-tagged discount at or + below the baseline: a lower rate never inflates rate_max, and rate_min existing to reach + it is the whole point of a free/discount session. Falls back to the plain (unfiltered) min/max/average when every minute in range is tagged "saving" - an event that covers the whole forecast window leaves no genuine tariff minute to scan, and a 99999/0/0 result would make every downstream comparison in set_rate_thresholds() behave as if there were no data at all, which is worse than the boosted-but-real numbers. """ + baseline_max = 0 + baseline_n = 0 + + for minute in range(self.minutes_now, self.forecast_minutes + self.minutes_now): + if minute in rates and rate_replicate.get(minute) != "saving": + baseline_max = max(baseline_max, rates[minute]) + baseline_n += 1 + + if baseline_n == 0: + return self.rate_minmax(rates)[:3] + rate_min = 99999 rate_max = 0 rate_total = 0 rate_n = 0 for minute in range(self.minutes_now, self.forecast_minutes + self.minutes_now): - if minute in rates and rate_replicate.get(minute) != "saving": - rate = rates[minute] - rate_min = min(rate_min, rate) - rate_max = max(rate_max, rate) - rate_total += rate - rate_n += 1 + if minute not in rates: + continue + rate = rates[minute] + if rate_replicate.get(minute) == "saving" and rate > baseline_max: + continue + rate_min = min(rate_min, rate) + rate_max = max(rate_max, rate) + rate_total += rate + rate_n += 1 - if rate_n: - return dp2(rate_min), dp2(rate_max), dp2(rate_total / rate_n) - return self.rate_minmax(rates)[:3] + return dp2(rate_min), dp2(rate_max), dp2(rate_total / rate_n) def rate_base_min_max(self, rates): """ diff --git a/apps/predbat/tests/test_set_rate_thresholds.py b/apps/predbat/tests/test_set_rate_thresholds.py index c298d95a2..294c87435 100644 --- a/apps/predbat/tests/test_set_rate_thresholds.py +++ b/apps/predbat/tests/test_set_rate_thresholds.py @@ -90,6 +90,44 @@ def test_rate_minmax_excluding_saving_skips_the_boosted_minutes(my_predbat): return failed +def test_rate_minmax_excluding_saving_keeps_genuine_free_slots(my_predbat): + """A "saving"-tagged minute that is CHEAPER than the tariff, not more expensive, must still + count towards rate_min - the "saving" tag also marks free/discounted Octopus sessions + (load_free_slot()) and Axle import discounts, not only event boosts (Copilot review on + #5052). + + With a flat 20p tariff and one free (0p) slot tagged "saving", excluding every tagged minute + regardless of direction previously left only the flat rate - rate_max == rate_min - which + made set_rate_thresholds() take its "everything but the most expensive" branch and set the + threshold ABOVE every rate, so the entire ordinary-price day read as low-rate. Reproduced + with this exact scenario before fixing. + """ + print("**** Testing rate_minmax_excluding_saving keeps a genuine free slot ****") + failed = False + + my_predbat.minutes_now = 0 + my_predbat.forecast_minutes = 24 * 60 + rate_import = {minute: 20.0 for minute in range(0, 24 * 60)} + rate_import[100] = 0.0 + rate_import_replicated = {100: "saving"} + + rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_replicated) + + if rate_min != 0.0: + print("ERROR: the free slot's 0.0p should still set rate_min, got {}".format(rate_min)) + failed = True + if rate_max != 20.0: + print("ERROR: rate_max should be the flat tariff rate 20.0, got {}".format(rate_max)) + failed = True + if rate_max == rate_min: + print("ERROR: rate_max == rate_min - the free slot was excluded along with the rest of the flat tariff, which would push the automatic threshold above every rate") + failed = True + + if not failed: + print("PASS") + return failed + + def test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged(my_predbat): """If an event covers the whole forecast window there is no genuine tariff minute to scan - fall back to the plain min/max/average rather than returning a 99999/0/0 that would make every @@ -148,10 +186,46 @@ def test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat): return failed +_SNAPSHOT_FIELDS = ( + "minutes_now", + "forecast_minutes", + "rate_import", + "rate_import_replicated", + "rate_export", + "rate_export_replicated", + "rate_min", + "rate_max", + "rate_average", + "rate_export_min", + "rate_export_max", + "rate_export_average", + "rate_low_threshold", + "rate_high_threshold", + "alert_active_keep", + "manual_soc_keep", + "num_cars", +) + + def run_set_rate_thresholds_tests(my_predbat): - """Run the set_rate_thresholds / rate_minmax_excluding_saving tests""" - failed = False - failed |= test_rate_minmax_excluding_saving_skips_the_boosted_minutes(my_predbat) - failed |= test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged(my_predbat) - failed |= test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat) - return failed + """Run the set_rate_thresholds / rate_minmax_excluding_saving tests. + + _setup_two_rate_tariff() overwrites minutes_now, the rate tables/statistics, thresholds and + num_cars directly on the shared my_predbat fixture, and unit_test.py passes that same + instance through the whole registry - left unrestored, a later test would inherit this + group's synthetic 48h tariff and faked midnight instead of the fixture's own noon state, + making the suite order-dependent (Copilot review on #5052). Snapshot/restore around the + whole group rather than per sub-test, since every sub-test here uses the same helper and + they already run back-to-back with no fixture-clean test expected in between. + """ + snapshot = {field: getattr(my_predbat, field) for field in _SNAPSHOT_FIELDS} + try: + failed = False + failed |= test_rate_minmax_excluding_saving_skips_the_boosted_minutes(my_predbat) + failed |= test_rate_minmax_excluding_saving_keeps_genuine_free_slots(my_predbat) + failed |= test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged(my_predbat) + failed |= test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat) + return failed + finally: + for field, value in snapshot.items(): + setattr(my_predbat, field, value) From c4357818e716e4274e3554f47159699f19424541 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 11 Sep 2026 21:33:36 +0100 Subject: [PATCH 3/9] test(fetch): cover the export side and manual-threshold mode in rate_minmax_excluding_saving tests (Copilot review on #5052) Every existing test in this file used rate_import_replicated only and left rate_high_threshold at 0 (automatic mode) - the export side (rate_export_replicated) and manual mode (rate_high_threshold > 0) were both genuinely untested. set_rate_thresholds()'s manual branch (rate_export_cost_threshold = rate_export_average * rate_high_threshold) multiplies by the average directly, so a boosted minute contaminating it would not be caught by the automatic-mode min/max comparisons the other tests already cover. Added a scenario with a flat 15p export tariff and a +50p Axle-style export event over 2h, rate_high_threshold=1.0 so the threshold equals whichever average was used. Verified directly before writing the assertion: the contaminated average is ~19.17p, the clean one exactly 15.0p, and set_rate_thresholds() correctly produces 15.0. Co-Authored-By: Claude Sonnet 5 --- .../predbat/tests/test_set_rate_thresholds.py | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/apps/predbat/tests/test_set_rate_thresholds.py b/apps/predbat/tests/test_set_rate_thresholds.py index 294c87435..bda81591e 100644 --- a/apps/predbat/tests/test_set_rate_thresholds.py +++ b/apps/predbat/tests/test_set_rate_thresholds.py @@ -186,6 +186,61 @@ def test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat): return failed +def test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predbat): + """The export side and manual-threshold mode (rate_high_threshold > 0) were both untested - + every prior test here used rate_import_replicated only and left rate_high_threshold at 0 + (automatic mode) (Copilot review on #5052). This branch (fetch.py's + "rate_export_cost_threshold = dp2(rate_export_average * self.rate_high_threshold)") multiplies + by rate_export_average directly, so a boosted export minute contaminating that average would + not be masked by the automatic-mode rate_export_max/rate_export_min comparisons the other + tests already cover. + + Flat 15p export tariff, +50p Axle export-side event over 2h out of 24 (rate_export_replicated + tagged "saving", mirroring load_axle_slot()'s export branch): the contaminated average is + ~19.17p, the clean one is 15.0p exactly. rate_high_threshold=1.0 makes the threshold equal + whichever average was used, so the two are trivially distinguishable. + """ + print("**** test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode ****") + failed = False + + my_predbat.minutes_now = 0 + my_predbat.forecast_minutes = 24 * 60 + + rate_import = {minute: 20.0 for minute in range(0, 48 * 60)} + rate_export = {minute: 15.0 for minute in range(0, 48 * 60)} + rate_export_replicated = {} + for minute in range(600, 720): + rate_export[minute] += 50.0 + rate_export_replicated[minute] = "saving" + + my_predbat.rate_import = rate_import + my_predbat.rate_import_replicated = {} + my_predbat.rate_export = rate_export + my_predbat.rate_export_replicated = rate_export_replicated + my_predbat.rate_min, my_predbat.rate_max, my_predbat.rate_average, _, _ = my_predbat.rate_minmax(rate_import) + my_predbat.rate_export_min, my_predbat.rate_export_max, my_predbat.rate_export_average, _, _ = my_predbat.rate_minmax(rate_export) + my_predbat.rate_low_threshold = 0 + my_predbat.rate_high_threshold = 1.0 # manual mode: threshold = rate_export_average * rate_high_threshold + my_predbat.alert_active_keep = {} + my_predbat.manual_soc_keep = {} + my_predbat.num_cars = 0 + + my_predbat.set_rate_thresholds() + + if abs(my_predbat.rate_export_average - 19.17) > 0.01: + print("ERROR: test setup sanity check failed - contaminated rate_export_average should be ~19.17, got {}".format(my_predbat.rate_export_average)) + failed = True + + expected_threshold = 15.0 + if abs(my_predbat.rate_export_cost_threshold - expected_threshold) > 0.01: + print("ERROR: rate_export_cost_threshold should be {} (clean export average x 1.0), got {} - a boosted export event contaminated the manual-mode threshold".format(expected_threshold, my_predbat.rate_export_cost_threshold)) + failed = True + + if not failed: + print("PASS") + return failed + + _SNAPSHOT_FIELDS = ( "minutes_now", "forecast_minutes", @@ -225,6 +280,7 @@ def run_set_rate_thresholds_tests(my_predbat): failed |= test_rate_minmax_excluding_saving_keeps_genuine_free_slots(my_predbat) failed |= test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged(my_predbat) failed |= test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat) + failed |= test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predbat) return failed finally: for field, value in snapshot.items(): From 55c9724728afbeec59aa608c931782419484f42f Mon Sep 17 00:00:00 2001 From: Rik Allen <48563392+chalfontchubby@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:40:02 +0100 Subject: [PATCH 4/9] Add rate_import_cost_threshold and rate_export_cost_threshold Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- apps/predbat/tests/test_set_rate_thresholds.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/predbat/tests/test_set_rate_thresholds.py b/apps/predbat/tests/test_set_rate_thresholds.py index bda81591e..e19a888b9 100644 --- a/apps/predbat/tests/test_set_rate_thresholds.py +++ b/apps/predbat/tests/test_set_rate_thresholds.py @@ -254,8 +254,10 @@ def test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predb "rate_export_min", "rate_export_max", "rate_export_average", + "rate_import_cost_threshold", + "rate_export_cost_threshold", "rate_low_threshold", - "rate_high_threshold", + "rate_high_threshold" "alert_active_keep", "manual_soc_keep", "num_cars", From 5a09c776ab5e24046f411a345a0590f4b2347935 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:43:53 +0000 Subject: [PATCH 5/9] Fix saving-tagged small reward handling in threshold min/max scan Co-authored-by: chalfontchubby <48563392+chalfontchubby@users.noreply.github.com> --- apps/predbat/fetch.py | 29 +++++++++++----- .../predbat/tests/test_set_rate_thresholds.py | 33 +++++++++++++++++-- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/apps/predbat/fetch.py b/apps/predbat/fetch.py index a57a7d3c3..cb85570c4 100644 --- a/apps/predbat/fetch.py +++ b/apps/predbat/fetch.py @@ -2290,12 +2290,15 @@ def rate_minmax_excluding_saving(self, rates, rate_replicate): every rate and makes the whole ordinary-price day read as "low" (Copilot review on #5052 - reproduced with exactly that scenario before fixing). - Two passes: the first pass over the untagged minutes alone sets a baseline max. The - second pass then excludes a "saving"-tagged minute only if its rate exceeds that - baseline max - the one shape (a reward) that can distort the automatic threshold - and - admits every other minute, tagged or not, including a "saving"-tagged discount at or - below the baseline: a lower rate never inflates rate_max, and rate_min existing to reach - it is the whole point of a free/discount session. + Prefer a per-minute comparison against the pre-event/base tariff where available + (rate_import_base/rate_export_base): for a tagged minute above its own base rate, use the + base rate for min/max/average. This catches small rewards on cheap slots too + (e.g. 3.49p + 5p = 8.49p), which can stay below the day's global max yet still contaminate + averages if not mapped back to their base. + + When no base curve is available, fall back to the earlier two-pass heuristic: first pass + over untagged minutes sets a baseline max, second pass excludes tagged minutes above that + baseline max. Falls back to the plain (unfiltered) min/max/average when every minute in range is tagged "saving" - an event that covers the whole forecast window leaves no genuine tariff minute to @@ -2313,6 +2316,12 @@ def rate_minmax_excluding_saving(self, rates, rate_replicate): if baseline_n == 0: return self.rate_minmax(rates)[:3] + rate_base = None + if rates is self.rate_import and self.rate_import_base: + rate_base = self.rate_import_base + elif rates is self.rate_export and self.rate_export_base: + rate_base = self.rate_export_base + rate_min = 99999 rate_max = 0 rate_total = 0 @@ -2322,8 +2331,12 @@ def rate_minmax_excluding_saving(self, rates, rate_replicate): if minute not in rates: continue rate = rates[minute] - if rate_replicate.get(minute) == "saving" and rate > baseline_max: - continue + if rate_replicate.get(minute) == "saving": + if rate_base and minute in rate_base: + if rate > rate_base[minute]: + rate = rate_base[minute] + elif rate > baseline_max: + continue rate_min = min(rate_min, rate) rate_max = max(rate_max, rate) rate_total += rate diff --git a/apps/predbat/tests/test_set_rate_thresholds.py b/apps/predbat/tests/test_set_rate_thresholds.py index e19a888b9..86a7f1aa5 100644 --- a/apps/predbat/tests/test_set_rate_thresholds.py +++ b/apps/predbat/tests/test_set_rate_thresholds.py @@ -33,10 +33,11 @@ def _setup_two_rate_tariff(my_predbat, event_start, event_end, event_boost=100.0 my_predbat.minutes_now = 0 my_predbat.forecast_minutes = 24 * 60 - rate_import = {} + rate_import_base = {} for minute in range(0, 48 * 60): hour = (minute // 60) % 24 - rate_import[minute] = 25.95 if 6 <= hour < 22 else 3.49 + rate_import_base[minute] = 25.95 if 6 <= hour < 22 else 3.49 + rate_import = rate_import_base.copy() rate_import_replicated = {} for minute in range(event_start, event_end): @@ -46,8 +47,10 @@ def _setup_two_rate_tariff(my_predbat, event_start, event_end, event_boost=100.0 rate_export = {minute: 15.0 for minute in range(0, 48 * 60)} my_predbat.rate_import = rate_import + my_predbat.rate_import_base = rate_import_base my_predbat.rate_import_replicated = rate_import_replicated my_predbat.rate_export = rate_export + my_predbat.rate_export_base = rate_export.copy() my_predbat.rate_export_replicated = {} my_predbat.rate_min, my_predbat.rate_max, my_predbat.rate_average, _, _ = my_predbat.rate_minmax(rate_import) @@ -186,6 +189,31 @@ def test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat): return failed +def test_set_rate_thresholds_ignores_small_saving_boost_in_manual_import_mode(my_predbat): + """Manual import threshold mode must also ignore a saving reward added to a cheap slot. + + Reproduces the review case from #5052: +5p on a 3.49p night slot (8.49p total) stays below the + 25.95p day-rate max, so a global-max filter leaves it in rate_average and inflates the manual + threshold. + """ + print("**** test_set_rate_thresholds_ignores_small_saving_boost_in_manual_import_mode ****") + failed = False + + _setup_two_rate_tariff(my_predbat, event_start=60, event_end=180, event_boost=5.0) + my_predbat.rate_low_threshold = 1.0 + + my_predbat.set_rate_thresholds() + + expected_threshold = 18.46 + if abs(my_predbat.rate_import_cost_threshold - expected_threshold) > 0.01: + print("ERROR: rate_import_cost_threshold should be {} (clean import average x 1.0), got {} - small boosted saving minutes contaminated manual mode".format(expected_threshold, my_predbat.rate_import_cost_threshold)) + failed = True + + if not failed: + print("PASS") + return failed + + def test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predbat): """The export side and manual-threshold mode (rate_high_threshold > 0) were both untested - every prior test here used rate_import_replicated only and left rate_high_threshold at 0 @@ -282,6 +310,7 @@ def run_set_rate_thresholds_tests(my_predbat): failed |= test_rate_minmax_excluding_saving_keeps_genuine_free_slots(my_predbat) failed |= test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged(my_predbat) failed |= test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat) + failed |= test_set_rate_thresholds_ignores_small_saving_boost_in_manual_import_mode(my_predbat) failed |= test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predbat) return failed finally: From cdb1d247fa57e9a2ecbef16b7aae040384bf6efe Mon Sep 17 00:00:00 2001 From: Rik Allen <48563392+chalfontchubby@users.noreply.github.com> Date: Sat, 12 Sep 2026 08:38:53 +0100 Subject: [PATCH 6/9] Update expected rate_average value in tests Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- apps/predbat/tests/test_set_rate_thresholds.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/predbat/tests/test_set_rate_thresholds.py b/apps/predbat/tests/test_set_rate_thresholds.py index 86a7f1aa5..ac4be0ead 100644 --- a/apps/predbat/tests/test_set_rate_thresholds.py +++ b/apps/predbat/tests/test_set_rate_thresholds.py @@ -84,8 +84,8 @@ def test_rate_minmax_excluding_saving_skips_the_boosted_minutes(my_predbat): if rate_max != 25.95: print("ERROR: saving-excluded rate_max should be 25.95 (the true day rate), got {}".format(rate_max)) failed = True - if abs(rate_average - 17.78) > 0.01: - print("ERROR: saving-excluded rate_average should be ~17.78, got {}".format(rate_average)) + if abs(rate_average - 18.46) > 0.01: + print("ERROR: saving-excluded rate_average should be ~18.46, got {}".format(rate_average)) failed = True if not failed: From dc7153ad777ff6629472b7b9d2a1d2f14d7cd59b Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sat, 12 Sep 2026 08:54:51 +0100 Subject: [PATCH 7/9] fix(fetch): track saving-session minutes independently of rate_replicate, fix a test-crashing typo, and a snapshot gap (Copilot review on #5052) Three findings from the latest Copilot review of the #5050 saving-session threshold-exclusion fix, all confirmed and fixed. rate_minmax_excluding_saving() could be defeated by a later, supported override ------------------------------------------------------------------------------------ It read rate_replicate.get(minute) == "saving" live, but the production fetch path calls load_saving_slot()/load_free_slot()/load_axle_slot() (which tag rate_replicate "saving") and then basic_rates()/apply_manual_rates() (whose rate_increment branch overwrites that same minute's tag to "increment"/"user") in that order - a documented, supported combination (an override active during a saving session). The boosted price then silently re-entered the threshold stats, since the "saving" provenance was already gone from rate_replicate by the time set_rate_thresholds() ran. Fixed by capturing a frozen rate_import_saving_minutes/rate_export_saving_minutes set immediately after the three loaders run and before any override can touch rate_replicate, and having rate_minmax_excluding_saving() read that set instead of the live, later-overwritable dict. rate_replicate itself is untouched - output.py still needs its final tag for display. A missing comma silently concatenated two snapshot field names ------------------------------------------------------------------ _SNAPSHOT_FIELDS in test_set_rate_thresholds.py had "rate_high_threshold" and "alert_active_keep" on adjacent lines with no comma between them, so Python's implicit string-literal concatenation merged them into one nonexistent attribute name - the snapshot comprehension raised AttributeError before any test in the module could run at all. Added the comma. Two fields the test helper mutates were missing from the snapshot ------------------------------------------------------------------ _setup_two_rate_tariff() sets rate_import_base/rate_export_base on the shared PredBat test instance but neither was in _SNAPSHOT_FIELDS, so the group's own restore left those synthetic 48h curves on the instance for every later test in the registry. Confirmed the leak directly (both fields differed from their original value after the group ran) and added them to the snapshot. All three verified with mutation testing: reverting each fix in turn reproduces exactly the failure its regression test describes (including a new test that reproduces the override-overwrite scenario directly), and only that test fails. ./run_all --quick and ./run_pre_commit both green. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/fetch.py | 49 +++++++---- apps/predbat/predbat.py | 2 + .../predbat/tests/test_set_rate_thresholds.py | 85 ++++++++++++++++--- 3 files changed, 110 insertions(+), 26 deletions(-) diff --git a/apps/predbat/fetch.py b/apps/predbat/fetch.py index cb85570c4..91c21838e 100644 --- a/apps/predbat/fetch.py +++ b/apps/predbat/fetch.py @@ -917,8 +917,10 @@ def fetch_sensor_data(self, save=True): import_rates = {} self.rate_import_replicated = {} + self.rate_import_saving_minutes = set() export_rates = {} self.rate_export_replicated = {} + self.rate_export_saving_minutes = set() self.rate_slots = [] self.io_adjusted = {} self.low_rates = [] @@ -1191,6 +1193,13 @@ def fetch_sensor_data(self, save=True): self.load_saving_slot(self.octopus_saving_slots, import_rates, export=False, rate_replicate=self.rate_import_replicated) self.load_free_slot(self.octopus_free_slots, import_rates, export=False, rate_replicate=self.rate_import_replicated) load_axle_slot(self, self.axle_sessions, import_rates, export=False, rate_replicate=self.rate_import_replicated) + # Snapshot which minutes a saving/free/Axle session tagged "saving" here, before + # basic_rates()/apply_manual_rates() below get a chance to overwrite rate_replicate + # with their own "increment"/"user" tag on the same minute - a supported combination + # (an override active during a saving session) that would otherwise silently defeat + # rate_minmax_excluding_saving()'s exclusion, since it reads rate_replicate live and + # the "saving" provenance would already be gone by the time it runs (#5052 review). + self.rate_import_saving_minutes = {minute for minute, tag in self.rate_import_replicated.items() if tag == "saving"} import_rates = self.basic_rates(self.get_arg("rates_import_override", [], indirect=False), "rates_import_override", import_rates, self.rate_import_replicated) import_rates = self.apply_manual_rates(import_rates, self.manual_import_rates, is_import=True, rate_replicate=self.rate_import_replicated) self.rate_scan(import_rates, print=True) @@ -1213,6 +1222,8 @@ def fetch_sensor_data(self, save=True): if self.rate_export_max > 0: self.load_saving_slot(self.octopus_saving_slots, export_rates, export=True, rate_replicate=self.rate_export_replicated) load_axle_slot(self, self.axle_sessions, export_rates, export=True, rate_replicate=self.rate_export_replicated) + # See the import block's equivalent comment above (#5052 review). + self.rate_export_saving_minutes = {minute for minute, tag in self.rate_export_replicated.items() if tag == "saving"} export_rates = self.basic_rates(self.get_arg("rates_export_override", [], indirect=False), "rates_export_override", export_rates, self.rate_export_replicated) export_rates = self.apply_manual_rates(export_rates, self.manual_export_rates, is_import=False, rate_replicate=self.rate_export_replicated) self.rate_scan_export(export_rates, print=True) @@ -2272,19 +2283,26 @@ def rate_minmax(self, rates): return dp2(rate_min), dp2(rate_max), dp2(rate_average), rate_min_minute, rate_max_minute - def rate_minmax_excluding_saving(self, rates, rate_replicate): + def rate_minmax_excluding_saving(self, rates, saving_minutes): """ - Work out min/max/average over the forecast window, excluding only "saving"-tagged - minutes that are inflated relative to the tariff's own rates - not every minute the - "saving" tag touches. + Work out min/max/average over the forecast window, excluding only saving-session/Axle + minutes that are inflated relative to the tariff's own rates - not every such minute. - "saving" (set by load_saving_slot()/load_axle_slot()/load_free_slot()) marks two - economically opposite things with the same string: a saving-session/Axle event REWARD, + saving_minutes (rate_import_saving_minutes/rate_export_saving_minutes) is a frozen set of + minutes load_saving_slot()/load_axle_slot()/load_free_slot() tagged, captured right after + those calls and before basic_rates()/apply_manual_rates() run - NOT the live + rate_import_replicated/rate_export_replicated dict. A later override active during the + same session (a documented, supported combination - an export rate_increment alongside a + saving session, say) overwrites that dict's "saving" tag with its own "increment"/"user" + tag on the same minute, which would silently defeat a live-dict check here and let the + boosted price back into the threshold stats (Copilot review on #5052). + + These minutes mark two economically opposite things: a saving-session/Axle event REWARD, added on top of the tariff rate (GH#5050 - a synthetic one-off high that should not be allowed to raise the automatic threshold above every genuine tariff rate), and a free/discounted import session, which SUBTRACTS from or floors the rate (a genuinely cheap slot the automatic threshold should be free to pick as "low rate"). Excluding every - "saving" minute regardless of direction throws the cheap ones away too - with a flat + such minute regardless of direction throws the cheap ones away too - with a flat tariff plus one free slot, that leaves only the flat rate, rate_max == rate_min, and set_rate_thresholds() takes its rate_max + 0.1 branch, which sets the threshold above every rate and makes the whole ordinary-price day read as "low" (Copilot review on @@ -2300,16 +2318,17 @@ def rate_minmax_excluding_saving(self, rates, rate_replicate): over untagged minutes sets a baseline max, second pass excludes tagged minutes above that baseline max. - Falls back to the plain (unfiltered) min/max/average when every minute in range is tagged - "saving" - an event that covers the whole forecast window leaves no genuine tariff minute to - scan, and a 99999/0/0 result would make every downstream comparison in set_rate_thresholds() - behave as if there were no data at all, which is worse than the boosted-but-real numbers. + Falls back to the plain (unfiltered) min/max/average when every minute in range is a + saving minute - an event that covers the whole forecast window leaves no genuine tariff + minute to scan, and a 99999/0/0 result would make every downstream comparison in + set_rate_thresholds() behave as if there were no data at all, which is worse than the + boosted-but-real numbers. """ baseline_max = 0 baseline_n = 0 for minute in range(self.minutes_now, self.forecast_minutes + self.minutes_now): - if minute in rates and rate_replicate.get(minute) != "saving": + if minute in rates and minute not in saving_minutes: baseline_max = max(baseline_max, rates[minute]) baseline_n += 1 @@ -2331,7 +2350,7 @@ def rate_minmax_excluding_saving(self, rates, rate_replicate): if minute not in rates: continue rate = rates[minute] - if rate_replicate.get(minute) == "saving": + if minute in saving_minutes: if rate_base and minute in rate_base: if rate > rate_base[minute]: rate = rate_base[minute] @@ -2434,8 +2453,8 @@ def set_rate_thresholds(self): # export equivalents) include those synthetic minutes and are used elsewhere (dashboard # sensors, graph scaling, plan.py pricing) where the real boosted price is exactly what is # wanted, so this is a separate, narrower scan rather than a change to those. - rate_min, rate_max, rate_average = self.rate_minmax_excluding_saving(self.rate_import, self.rate_import_replicated) - rate_export_min, rate_export_max, rate_export_average = self.rate_minmax_excluding_saving(self.rate_export, self.rate_export_replicated) + rate_min, rate_max, rate_average = self.rate_minmax_excluding_saving(self.rate_import, self.rate_import_saving_minutes) + rate_export_min, rate_export_max, rate_export_average = self.rate_minmax_excluding_saving(self.rate_export, self.rate_export_saving_minutes) if self.rate_low_threshold > 0: self.rate_import_cost_threshold = dp2(rate_average * self.rate_low_threshold) diff --git a/apps/predbat/predbat.py b/apps/predbat/predbat.py index 23adc2fd1..bd1811662 100644 --- a/apps/predbat/predbat.py +++ b/apps/predbat/predbat.py @@ -568,8 +568,10 @@ def reset(self): self.carbon_yesterday = 0.0 self.rate_import = {} self.rate_import_replicated = {} + self.rate_import_saving_minutes = set() self.rate_export = {} self.rate_export_replicated = {} + self.rate_export_saving_minutes = set() self.rate_slots = [] self.low_rates = [] self.high_export_rates = [] diff --git a/apps/predbat/tests/test_set_rate_thresholds.py b/apps/predbat/tests/test_set_rate_thresholds.py index ac4be0ead..6b6b6a8b7 100644 --- a/apps/predbat/tests/test_set_rate_thresholds.py +++ b/apps/predbat/tests/test_set_rate_thresholds.py @@ -19,9 +19,14 @@ ON for ~24h, the reported symptom). set_rate_thresholds() now derives its threshold stats from rate_minmax_excluding_saving(), which -scans the same rates but skips any minute tagged "saving". self.rate_max/rate_min/rate_average -(and the export equivalents) are deliberately left untouched - they feed dashboard sensors, graph -scaling, and plan.py pricing, where the real boosted price is what should be shown. +scans the same rates but skips any minute in rate_import_saving_minutes/rate_export_saving_minutes +- a frozen snapshot of "saving"-tagged minutes taken right after the saving/free/Axle loaders run +and before any override (rates_import_override, manual rates) can overwrite that same minute's +rate_import_replicated/rate_export_replicated tag with its own "increment"/"user" tag, silently +losing the "this was a saving minute" provenance the live dict alone can no longer tell (Copilot +review on #5052). self.rate_max/rate_min/rate_average (and the export equivalents) are +deliberately left untouched - they feed dashboard sensors, graph scaling, and plan.py pricing, +where the real boosted price is what should be shown. """ @@ -43,15 +48,18 @@ def _setup_two_rate_tariff(my_predbat, event_start, event_end, event_boost=100.0 for minute in range(event_start, event_end): rate_import[minute] += event_boost rate_import_replicated[minute] = "saving" + rate_import_saving_minutes = set(range(event_start, event_end)) rate_export = {minute: 15.0 for minute in range(0, 48 * 60)} my_predbat.rate_import = rate_import my_predbat.rate_import_base = rate_import_base my_predbat.rate_import_replicated = rate_import_replicated + my_predbat.rate_import_saving_minutes = rate_import_saving_minutes my_predbat.rate_export = rate_export my_predbat.rate_export_base = rate_export.copy() my_predbat.rate_export_replicated = {} + my_predbat.rate_export_saving_minutes = set() my_predbat.rate_min, my_predbat.rate_max, my_predbat.rate_average, _, _ = my_predbat.rate_minmax(rate_import) my_predbat.rate_export_min, my_predbat.rate_export_max, my_predbat.rate_export_average, _, _ = my_predbat.rate_minmax(rate_export) @@ -62,7 +70,7 @@ def _setup_two_rate_tariff(my_predbat, event_start, event_end, event_boost=100.0 my_predbat.manual_soc_keep = {} my_predbat.num_cars = 0 - return rate_import, rate_import_replicated + return rate_import, rate_import_saving_minutes def test_rate_minmax_excluding_saving_skips_the_boosted_minutes(my_predbat): @@ -71,13 +79,13 @@ def test_rate_minmax_excluding_saving_skips_the_boosted_minutes(my_predbat): print("**** test_rate_minmax_excluding_saving_skips_the_boosted_minutes ****") failed = False - rate_import, rate_import_replicated = _setup_two_rate_tariff(my_predbat, event_start=17 * 60, event_end=19 * 60) + rate_import, rate_import_saving_minutes = _setup_two_rate_tariff(my_predbat, event_start=17 * 60, event_end=19 * 60) if my_predbat.rate_max != 125.95: print("ERROR: test setup sanity check failed - contaminated rate_max should be 125.95, got {}".format(my_predbat.rate_max)) failed = True - rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_replicated) + rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_saving_minutes) if rate_min != 3.49: print("ERROR: saving-excluded rate_min should be 3.49, got {}".format(rate_min)) failed = True @@ -112,9 +120,9 @@ def test_rate_minmax_excluding_saving_keeps_genuine_free_slots(my_predbat): my_predbat.forecast_minutes = 24 * 60 rate_import = {minute: 20.0 for minute in range(0, 24 * 60)} rate_import[100] = 0.0 - rate_import_replicated = {100: "saving"} + rate_import_saving_minutes = {100} - rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_replicated) + rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_saving_minutes) if rate_min != 0.0: print("ERROR: the free slot's 0.0p should still set rate_min, got {}".format(rate_min)) @@ -138,9 +146,9 @@ def test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged(my_pr print("**** test_rate_minmax_excluding_saving_falls_back_when_everything_is_tagged ****") failed = False - rate_import, rate_import_replicated = _setup_two_rate_tariff(my_predbat, event_start=0, event_end=my_predbat.forecast_minutes) + rate_import, rate_import_saving_minutes = _setup_two_rate_tariff(my_predbat, event_start=0, event_end=my_predbat.forecast_minutes) - rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_replicated) + rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_saving_minutes) expected_min, expected_max, expected_average, _, _ = my_predbat.rate_minmax(rate_import) if (rate_min, rate_max, rate_average) != (expected_min, expected_max, expected_average): print("ERROR: fully-tagged window should fall back to the plain scan {}, got {}".format((expected_min, expected_max, expected_average), (rate_min, rate_max, rate_average))) @@ -240,11 +248,14 @@ def test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predb for minute in range(600, 720): rate_export[minute] += 50.0 rate_export_replicated[minute] = "saving" + rate_export_saving_minutes = set(range(600, 720)) my_predbat.rate_import = rate_import my_predbat.rate_import_replicated = {} + my_predbat.rate_import_saving_minutes = set() my_predbat.rate_export = rate_export my_predbat.rate_export_replicated = rate_export_replicated + my_predbat.rate_export_saving_minutes = rate_export_saving_minutes my_predbat.rate_min, my_predbat.rate_max, my_predbat.rate_average, _, _ = my_predbat.rate_minmax(rate_import) my_predbat.rate_export_min, my_predbat.rate_export_max, my_predbat.rate_export_average, _, _ = my_predbat.rate_minmax(rate_export) my_predbat.rate_low_threshold = 0 @@ -269,13 +280,62 @@ def test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predb return failed +def test_rate_minmax_excluding_saving_ignores_overwritten_replicate_tag(my_predbat): + """A saving-boosted minute must stay excluded even after something downstream overwrites its + rate_replicate tag - a documented, supported combination (an export rate_increment override + active during the same saving session) rewrites rate_replicate[minute] from "saving" to + "increment"/"user" (fetch.py's basic_rates()), which happens for real in the production fetch + path: load_saving_slot()/load_free_slot()/load_axle_slot() tag rate_replicate, then + basic_rates()/apply_manual_rates() run afterward and can overwrite the tag on the very same + minute (Copilot review on #5052). + + rate_minmax_excluding_saving() must not be fooled by that - it takes the frozen + rate_import_saving_minutes/rate_export_saving_minutes set (captured before any override can + run), not the live, possibly-overwritten rate_replicate dict, so this reproduces the + overwrite directly rather than relying on the full fetch pipeline. + """ + print("**** test_rate_minmax_excluding_saving_ignores_overwritten_replicate_tag ****") + failed = False + + my_predbat.minutes_now = 0 + my_predbat.forecast_minutes = 24 * 60 + rate_import = {minute: 20.0 for minute in range(0, 24 * 60)} + rate_import[100] += 50.0 # a saving-session reward, same shape as load_saving_slot() + rate_import_saving_minutes = {100} + # Simulate basic_rates()'s rate_increment branch overwriting the same minute's tag afterward - + # the live rate_replicate dict no longer says "saving" for minute 100 at all. + rate_import_replicated = {100: "increment"} + + rate_min, rate_max, rate_average = my_predbat.rate_minmax_excluding_saving(rate_import, rate_import_saving_minutes) + + if rate_max != 20.0: + print("ERROR: the boosted minute should still be excluded from rate_max even though its rate_replicate tag was overwritten, got {}".format(rate_max)) + failed = True + if rate_min != 20.0: + print("ERROR: rate_min should be the flat tariff rate, got {}".format(rate_min)) + failed = True + + # The overwritten dict is passed through unused here (rate_minmax_excluding_saving takes the + # saving_minutes set, not rate_replicate) - assert on it anyway so the test documents exactly + # what would have fooled a live-dict-based check. + if rate_import_replicated.get(100) == "saving": + print("ERROR: test setup sanity check failed - the simulated overwrite did not actually overwrite the tag") + failed = True + + if not failed: + print("PASS") + return failed + + _SNAPSHOT_FIELDS = ( "minutes_now", "forecast_minutes", "rate_import", "rate_import_replicated", + "rate_import_saving_minutes", "rate_export", "rate_export_replicated", + "rate_export_saving_minutes", "rate_min", "rate_max", "rate_average", @@ -285,10 +345,12 @@ def test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predb "rate_import_cost_threshold", "rate_export_cost_threshold", "rate_low_threshold", - "rate_high_threshold" + "rate_high_threshold", "alert_active_keep", "manual_soc_keep", "num_cars", + "rate_import_base", + "rate_export_base", ) @@ -312,6 +374,7 @@ def run_set_rate_thresholds_tests(my_predbat): failed |= test_set_rate_thresholds_ignores_saving_boost_in_automatic_mode(my_predbat) failed |= test_set_rate_thresholds_ignores_small_saving_boost_in_manual_import_mode(my_predbat) failed |= test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predbat) + failed |= test_rate_minmax_excluding_saving_ignores_overwritten_replicate_tag(my_predbat) return failed finally: for field, value in snapshot.items(): From e0b9c06092865ab6b148c19ca0d4525d2513b101 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sat, 12 Sep 2026 09:28:22 +0100 Subject: [PATCH 8/9] docs(fetch): fix a stale comment describing the pre-fix behaviour it was explaining (Copilot review on #5052) The comment above the rate_import_saving_minutes snapshot still described rate_minmax_excluding_saving() as reading rate_replicate live and being fooled by a later overwrite - the exact problem the snapshot fixes, written before the fix existed and never updated once it landed. Reworded to describe what the code actually does now: the function reads the frozen snapshot precisely so a later overwrite can't erase the provenance it needs. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/fetch.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/predbat/fetch.py b/apps/predbat/fetch.py index 91c21838e..807b957bc 100644 --- a/apps/predbat/fetch.py +++ b/apps/predbat/fetch.py @@ -1196,9 +1196,10 @@ def fetch_sensor_data(self, save=True): # Snapshot which minutes a saving/free/Axle session tagged "saving" here, before # basic_rates()/apply_manual_rates() below get a chance to overwrite rate_replicate # with their own "increment"/"user" tag on the same minute - a supported combination - # (an override active during a saving session) that would otherwise silently defeat - # rate_minmax_excluding_saving()'s exclusion, since it reads rate_replicate live and - # the "saving" provenance would already be gone by the time it runs (#5052 review). + # (an override active during a saving session). rate_minmax_excluding_saving() reads + # this frozen set, not the live rate_replicate dict, precisely so that a later + # overwrite here can't erase the "this was a saving minute" provenance it depends on + # (#5052 review). self.rate_import_saving_minutes = {minute for minute, tag in self.rate_import_replicated.items() if tag == "saving"} import_rates = self.basic_rates(self.get_arg("rates_import_override", [], indirect=False), "rates_import_override", import_rates, self.rate_import_replicated) import_rates = self.apply_manual_rates(import_rates, self.manual_import_rates, is_import=True, rate_replicate=self.rate_import_replicated) From 7925cf465cd8059ad1a1ed1bd58556929e72a915 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sat, 12 Sep 2026 19:02:16 +0100 Subject: [PATCH 9/9] fix(compare,annual): clear stale saving minutes with the rates they describe rate_import_saving_minutes/rate_export_saving_minutes are populated only in fetch_sensor_data() and hold absolute minute offsets into the live tariff's rate tables. compare.fetch_rates() and annual._apply_rates() both replace rate_import/rate_export with a simulated tariff and then call set_rate_thresholds(), which reads those sets - so after a live cycle containing a saving session the stale offsets excluded whatever unrelated minutes happened to sit at the same positions in the simulated tariff from the min/max/average scan, skewing comparison and annual replay results. Live planning was unaffected. Reset both alongside the existing rate_low_threshold/rate_high_threshold resets in each, so the exclusion is a no-op unless the simulation itself tagged saving minutes. The test asserts on the source of each function rather than calling it: both drive the whole scan pipeline and rewrite ~24 fields on the shared my_predbat fixture (dashboard_values and the window lists among them), which leaked into optimise_levels when driven directly. Co-Authored-By: Claude Opus 5 --- apps/predbat/annual.py | 7 ++++ apps/predbat/compare.py | 8 +++++ .../predbat/tests/test_set_rate_thresholds.py | 36 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/apps/predbat/annual.py b/apps/predbat/annual.py index 4cdb1cfeb..dc1fbe9ba 100644 --- a/apps/predbat/annual.py +++ b/apps/predbat/annual.py @@ -1194,6 +1194,13 @@ def _apply_rates(predbat, rate_import, rate_export): predbat.rate_export = rate_export predbat.rate_low_threshold = 0 predbat.rate_high_threshold = 0 + # The saving-minute sets are provenance for the live tariff's rates, captured in + # fetch_sensor_data() and frozen there so a later rate_replicate()/basic_rates() overwrite + # cannot erase it (#5050). They describe minute offsets in the rates being replaced here, so + # they must not outlive them: set_rate_thresholds() below would otherwise exclude whatever + # happens to sit at those offsets in the simulated tariff from its min/max/average scan. + predbat.rate_import_saving_minutes = set() + predbat.rate_export_saving_minutes = set() if predbat.rate_import: predbat.rate_scan(predbat.rate_import, print=False) diff --git a/apps/predbat/compare.py b/apps/predbat/compare.py index a50ccadde..8494e3381 100644 --- a/apps/predbat/compare.py +++ b/apps/predbat/compare.py @@ -70,6 +70,14 @@ def fetch_rates(self, tariff, rate_import_base, rate_export_base): pb.rate_import = copy.deepcopy(rate_import_base) pb.rate_export = copy.deepcopy(rate_export_base) + # The saving-minute sets are provenance for the live tariff's rates, captured in + # fetch_sensor_data() and frozen there so a later rate_replicate()/basic_rates() overwrite + # cannot erase it (#5050). They describe minute offsets in the rates being replaced here, so + # they must not outlive them: set_rate_thresholds() below would otherwise exclude whatever + # happens to sit at those offsets in this tariff from its min/max/average scan. + pb.rate_import_saving_minutes = set() + pb.rate_export_saving_minutes = set() + # Fetch rates from Octopus Energy API if "rates_import_octopus_url" in tariff: # Fixed URL for rate import diff --git a/apps/predbat/tests/test_set_rate_thresholds.py b/apps/predbat/tests/test_set_rate_thresholds.py index 6b6b6a8b7..204ab142c 100644 --- a/apps/predbat/tests/test_set_rate_thresholds.py +++ b/apps/predbat/tests/test_set_rate_thresholds.py @@ -327,6 +327,41 @@ def test_rate_minmax_excluding_saving_ignores_overwritten_replicate_tag(my_predb return failed +def test_compare_and_annual_clear_stale_saving_minutes(my_predbat): + """The saving-minute sets must not outlive the rates they describe. + + rate_import_saving_minutes/rate_export_saving_minutes are only ever populated in + fetch_sensor_data(), and they hold absolute minute offsets into the live tariff's rate tables. + compare.py's fetch_rates() and annual.py's _apply_rates() both replace rate_import/rate_export + with a simulated tariff and then call set_rate_thresholds() - so after a live cycle containing a + saving session, those stale offsets would exclude whatever unrelated minutes happen to sit at + the same positions in the simulated tariff from the threshold scan (Copilot review on #5052). + + Both resets sit beside the existing rate_low_threshold/rate_high_threshold ones, so assert on + the source of each rather than running the functions: fetch_rates()/_apply_rates() drive the + whole scan pipeline and rewrite ~24 fields on the shared my_predbat fixture (including + dashboard_values and the window lists), which would leak into later tests in the registry. + """ + print("**** test_compare_and_annual_clear_stale_saving_minutes ****") + failed = False + + import inspect + + import annual + from compare import Compare + + for label, func, receiver in (("compare.fetch_rates", Compare.fetch_rates, "pb"), ("annual._apply_rates", annual._apply_rates, "predbat")): + source = inspect.getsource(func) + for field in ("rate_import_saving_minutes", "rate_export_saving_minutes"): + if "{}.{} = set()".format(receiver, field) not in source: + print("ERROR: {}() must reset {} alongside the rates it replaces, or a stale saving-session offset from a live cycle filters the simulated tariff".format(label, field)) + failed = True + + if not failed: + print("PASS") + return failed + + _SNAPSHOT_FIELDS = ( "minutes_now", "forecast_minutes", @@ -375,6 +410,7 @@ def run_set_rate_thresholds_tests(my_predbat): failed |= test_set_rate_thresholds_ignores_small_saving_boost_in_manual_import_mode(my_predbat) failed |= test_set_rate_thresholds_ignores_export_saving_boost_in_manual_mode(my_predbat) failed |= test_rate_minmax_excluding_saving_ignores_overwritten_replicate_tag(my_predbat) + failed |= test_compare_and_annual_clear_stale_saving_minutes(my_predbat) return failed finally: for field, value in snapshot.items():