fix(plan): value left-over battery on the base tariff, not saving-session prices (#4488)

battery_value_rate ceilinged the end-of-plan battery credit on rate_max and took its
export-recovery ratio from rate_export_max, both whole-horizon figures that include
saving sessions. On a flat tariff running a session that put the credit above what
discharging a stored kWh can realise, so the planner scored freeze charge as profit
and froze every window up to end_record while the battery sat nearly full.

A reported case had a flat 29.2p import with a 2p session bonus the next evening
(rate_max 31.2 vs rate_max_base 29.2) and a 100p export session expiring 19 minutes
after minutes_now (rate_export_max 100, export 0p for the rest of the horizon). The
session bonus raised the ceiling while the expiring export event switched the discount
off, leaving freeze charge worth +2.00p per kWh of load - exactly the session bonus -
and 5 of 6 charge windows frozen.

Read the base tariff for both terms instead. rate_max_base is already captured before
sessions inflate rate_max; rate_export_max_forward is new, built by fetch from
rate_export_base at the point that copy is taken, and forward-looking so an export
price that has passed stops counting. On the reported case the credit drops from
29.05p to 21.75p per kWh, freeze charge goes to -5.84p per kWh of load, and no window
freezes. Both terms fall back to their whole-horizon equivalents when the base data is
absent, so replaying an older debug file is unchanged.

run_single_debug now resets both fields alongside the existing dynamic_load_baseline
resets: the planner consumes them now, debug files written before they existed carry
neither, and inside the full suite they leaked from earlier tests into the replay.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: springfall2008

apps/predbat/fetch.py

@@ -1001,6 +1001,9 @@ def fetch_sensor_data(self, save=True):
             self.rate_scan_export(export_rates, print=False)
             export_rates, self.rate_export_replicated = self.rate_replicate(export_rates, is_import=False)
             self.rate_export_base = export_rates.copy()
+            # Built here, from the base rates, so the saving session and overrides applied below stay
+            # out of it - battery_value_rate needs the tariff's own export price, not an event price
+            self.rate_export_max_forward = self.rate_export_max_forward_calc(self.rate_export_base)
             # For export tariff only load the saving session if enabled
             if self.rate_export_max > 0:
                 self.load_saving_slot(self.octopus_saving_slots, export_rates, export=True, rate_replicate=self.rate_export_replicated)
@@ -1946,6 +1949,34 @@ def rate_scan(self, rates, print=True):
             self.rate_min_forward = self.rate_min_forward_calc(rates)
             self.log("Import rates: min {}{}, max {}{}, average {}{}".format(self.rate_min, curr, self.rate_max, curr, self.rate_average, curr))
 
+    def rate_export_max_forward_calc(self, rates):
+        """
+        Work out the highest export rate from each minute forwards
+
+        The export mirror of rate_min_forward_calc. Fed from the base export tariff (see
+        rate_export_base) so a saving session cannot stand in for the tariff's general ability to sell
+        surplus - that question is what the export haircut in battery_value_rate asks, and answering it
+        with a one-off event price makes stored energy look fully recoverable on a system that in fact
+        cannot sell a single kWh.
+        """
+        rate_array = []
+        rate_export_max_forward = {}
+        rate = 0.0
+
+        for minute in range(self.forecast_minutes + self.minutes_now + 48 * 60):
+            if minute in rates:
+                rate = rates[minute]
+            rate_array.append(rate)
+
+        # Work out the max rate going forward — O(n) right-to-left scan avoids O(n²) slice allocations
+        running_max = rate_array[-1] if rate_array else 0.0
+        for minute in range(len(rate_array) - 1, self.minutes_now - 1, -1):
+            running_max = max(running_max, rate_array[minute])
+            if minute < self.forecast_minutes + 24 * 60 + self.minutes_now:
+                rate_export_max_forward[minute] = running_max
+
+        return rate_export_max_forward
+
     def rate_scan_gas(self, rates, print=True):
         """
         Scan the gas rates and work out min/max

apps/predbat/plan.py

@@ -1597,6 +1597,14 @@ def battery_value_rate(self, minute):
         what the grid would ever charge for it, and floored at the export arbitrage margin and at
         1p/kWh.
 
+        Both the cap and the export recovery ratio read the base tariff (rate_max_base,
+        rate_export_max_forward), captured before saving sessions and overrides are layered on. A
+        session is a one-off event, not evidence about what the tariff charges for a kWh or pays for a
+        surplus one, and letting one set either term values stored energy above what discharging it can
+        realise - which the planner spends as profit by freeze charging every window up to end_record.
+        Both fall back to their whole-horizon equivalents when the base data is absent, so replaying an
+        older debug file behaves as it did before.
+
         Note `rate_export_min` here is not the export rate - it is the export earnings less the
         replacement cost, so it only raises the value when exporting beats re-importing. It can
         never lower it, which is why a zero export rate does not reduce the credit.
@@ -1605,8 +1613,9 @@ def battery_value_rate(self, minute):
         attributes and the savings report, so all three agree on what a stored kWh is worth.
         """
         rate_min_raw = self.rate_min_forward.get(minute, self.rate_min)
+        rate_max = self.rate_max_base or self.rate_max
         rate_min = rate_min_raw / self.inverter_loss / self.battery_loss + self.metric_battery_cycle
-        rate_min = max(min(rate_min, self.rate_max * self.inverter_loss * self.battery_loss - self.metric_battery_cycle), 0)
+        rate_min = max(min(rate_min, rate_max * self.inverter_loss * self.battery_loss - self.metric_battery_cycle), 0)
 
         # Replacement cost assumes the energy can always be redeployed. That holds while surplus can
         # be sold, but if export pays less than it cost to import then anything the house cannot use
@@ -1615,7 +1624,11 @@ def battery_value_rate(self, minute):
         # matches or beats it, down to metric_battery_value_export_scaling when export is worthless.
         # Setting that to 1.0 disables this entirely.
         if rate_min_raw > 0 and self.metric_battery_value_export_scaling < 1.0:
-            recovery = min(max(self.rate_export_max / rate_min_raw, 0.0), 1.0)
+            # Forward-looking like rate_min_raw, so an export price that has already passed stops
+            # counting once it has - a saving session ending in ten minutes says nothing about what
+            # the energy still in the battery can be sold for over the rest of the plan.
+            export_max = self.rate_export_max_forward.get(minute, 0.0) if self.rate_export_max_forward else self.rate_export_max
+            recovery = min(max(export_max / rate_min_raw, 0.0), 1.0)
             rate_min *= self.metric_battery_value_export_scaling + (1.0 - self.metric_battery_value_export_scaling) * recovery
 
         rate_export_min = self.rate_export_min * self.inverter_loss * self.battery_loss_discharge - self.metric_battery_cycle - rate_min

apps/predbat/predbat.py

@@ -432,6 +432,7 @@ def reset(self):
         self.rate_export_min_minute = 0
         self.rate_export_max = 0
         self.rate_export_max_minute = 0
+        self.rate_export_max_forward = {}
         self.rate_export_average = 0
         self.rate_gas_min = 0
         self.rate_gas_max = 0

apps/predbat/tests/test_compute_metric.py

@@ -8,6 +8,22 @@
 # pylint: disable=line-too-long
 # pylint: disable=attribute-defined-outside-init
 
+# Everything battery_value_rate reads - snapshotted and restored around each test that pokes at it
+VALUE_RATE_STATE = (
+    "rate_min_forward",
+    "rate_min",
+    "rate_max",
+    "rate_max_base",
+    "inverter_loss",
+    "battery_loss",
+    "battery_loss_discharge",
+    "rate_export_min",
+    "rate_export_max",
+    "rate_export_max_forward",
+    "metric_battery_cycle",
+    "metric_battery_value_export_scaling",
+)
+
 
 def compute_metric_test(
     my_predbat,
@@ -61,6 +77,11 @@ def compute_metric_test(
     my_predbat.metric_self_sufficiency = metric_self_sufficiency
     my_predbat.rate_min = rate_min
     my_predbat.rate_max = rate_max
+    # battery_value_rate prefers the session-free base rate for its ceiling, so pin it alongside
+    # rate_max or a value left behind by an earlier test would decide these metrics instead.
+    my_predbat.rate_max_base = rate_max
+    # Empty means "no forward export data", which sends battery_value_rate back to rate_export_max
+    my_predbat.rate_export_max_forward = {}
     if not end_record:
         end_record = my_predbat.forecast_minutes
 
@@ -114,9 +135,11 @@ def save_state(my_predbat):
         "metric_self_sufficiency",
         "rate_min",
         "rate_max",
+        "rate_max_base",
         "carbon_enable",
         "carbon_metric",
         "rate_min_forward",
+        "rate_export_max_forward",
     ]
     for item in save_items:
         state_dict[item] = getattr(my_predbat, item)
@@ -136,21 +159,20 @@ def battery_value_rate_test(my_predbat):
     re-splitting the formula would reintroduce that divergence, and only the ceiling case catches it.
     """
     failed = False
-    saved = {
-        name: getattr(my_predbat, name) for name in ("rate_min_forward", "rate_min", "rate_max", "inverter_loss", "battery_loss", "battery_loss_discharge", "rate_export_min", "rate_export_max", "metric_battery_cycle", "metric_battery_value_export_scaling")
-    }
+    saved = {name: getattr(my_predbat, name) for name in VALUE_RATE_STATE}
     try:
         my_predbat.inverter_loss = 0.96
         my_predbat.battery_loss = 0.97
         my_predbat.battery_loss_discharge = 0.97
         my_predbat.metric_battery_cycle = 0.0
         my_predbat.rate_export_min = 0.0
         my_predbat.rate_export_max = 6.9
+        my_predbat.rate_export_max_forward = {}
         my_predbat.metric_battery_value_export_scaling = 1.0
         my_predbat.rate_min = 6.9
 
         # Normal spread: the gross-up applies and the ceiling does not bind
-        my_predbat.rate_max = 28.85
+        my_predbat.rate_max = my_predbat.rate_max_base = 28.85
         my_predbat.rate_min_forward = {10: 6.9}
         expected = 6.9 / 0.96 / 0.97
         got = my_predbat.battery_value_rate(10)
@@ -160,15 +182,15 @@ def battery_value_rate_test(my_predbat):
 
         # Flat tariff: rate_min_forward == rate_max, so the ceiling MUST bind and pull the value
         # below the gross-up. Without the ceiling this returns 1.0739*R instead of 0.9312*R.
-        my_predbat.rate_max = 6.9
+        my_predbat.rate_max = my_predbat.rate_max_base = 6.9
         expected_capped = 6.9 * 0.96 * 0.97
         got = my_predbat.battery_value_rate(10)
         if abs(got - expected_capped) > 1e-6:
             print("ERROR: battery_value_rate is {} on a flat tariff, expected the capped {} - the rate_max ceiling is not being applied".format(got, expected_capped))
             failed = True
 
         # The 1p floor applies when the forward rate is negative (plunge pricing)
-        my_predbat.rate_max = 28.85
+        my_predbat.rate_max = my_predbat.rate_max_base = 28.85
         my_predbat.rate_min_forward = {10: -5.0}
         got = my_predbat.battery_value_rate(10)
         if abs(got - 1.0) > 1e-6:
@@ -180,6 +202,52 @@ def battery_value_rate_test(my_predbat):
     return failed
 
 
+def battery_value_rate_ceiling_base_test(my_predbat):
+    """Pin the ceiling to the base import tariff, not the session-inflated one.
+
+    load_saving_slot writes the session price into the import rates too, so rate_max can be a one-off
+    event price rather than the tariff's real peak. Ceiling on that and a stored kWh gets credited far
+    above anything the tariff will ever pay for it, which is what makes freeze charge look profitable
+    on a flat tariff. rate_max_base is captured before the session is layered on.
+    """
+    failed = False
+    saved = {name: getattr(my_predbat, name) for name in VALUE_RATE_STATE}
+    try:
+        my_predbat.inverter_loss = 0.96
+        my_predbat.battery_loss = 0.97
+        my_predbat.battery_loss_discharge = 0.97
+        my_predbat.metric_battery_cycle = 0.0
+        my_predbat.rate_export_min = 0.0
+        my_predbat.rate_export_max = 0.0
+        my_predbat.rate_export_max_forward = {}
+        my_predbat.metric_battery_value_export_scaling = 1.0
+        my_predbat.rate_min = 29.2
+        my_predbat.rate_min_forward = {10: 29.2}
+
+        # A 100p saving session sets rate_max, but the real tariff is flat at 29.2p. The ceiling must
+        # come from the base rate, so the value stays at the flat tariff's 0.9312*R.
+        my_predbat.rate_max = 100.0
+        my_predbat.rate_max_base = 29.2
+        expected = 29.2 * 0.96 * 0.97
+        got = my_predbat.battery_value_rate(10)
+        if abs(got - expected) > 1e-6:
+            print("ERROR: battery_value_rate is {} with a session-inflated rate_max, expected the base-capped {}".format(got, expected))
+            failed = True
+
+        # Unset base (an older debug file being replayed) falls back to rate_max so nothing regresses
+        my_predbat.rate_max = 31.2
+        my_predbat.rate_max_base = 0
+        expected = 31.2 * 0.96 * 0.97
+        got = my_predbat.battery_value_rate(10)
+        if abs(got - expected) > 1e-6:
+            print("ERROR: battery_value_rate is {} with rate_max_base unset, expected the rate_max fallback {}".format(got, expected))
+            failed = True
+    finally:
+        for name, value in saved.items():
+            setattr(my_predbat, name, value)
+    return failed
+
+
 def battery_value_export_scaling_test(my_predbat):
     """Pin the export-risk haircut: full value when export can recover the import cost, discounted when it cannot.
 
@@ -189,17 +257,17 @@ def battery_value_export_scaling_test(my_predbat):
     plans that have no export problem at all.
     """
     failed = False
-    names = ("rate_min_forward", "rate_min", "rate_max", "inverter_loss", "battery_loss", "battery_loss_discharge", "rate_export_min", "rate_export_max", "metric_battery_cycle", "metric_battery_value_export_scaling")
-    saved = {name: getattr(my_predbat, name) for name in names}
+    saved = {name: getattr(my_predbat, name) for name in VALUE_RATE_STATE}
     try:
         my_predbat.inverter_loss = 0.96
         my_predbat.battery_loss = 0.97
         my_predbat.battery_loss_discharge = 0.97
         my_predbat.metric_battery_cycle = 0.0
         my_predbat.rate_export_min = 0.0
         my_predbat.rate_min = 6.9
-        my_predbat.rate_max = 28.85
+        my_predbat.rate_max = my_predbat.rate_max_base = 28.85
         my_predbat.rate_min_forward = {10: 6.9}
+        my_predbat.rate_export_max_forward = {}
         my_predbat.metric_battery_value_export_scaling = 0.8
         full = 6.9 / 0.96 / 0.97
 
@@ -231,14 +299,82 @@ def battery_value_export_scaling_test(my_predbat):
     return failed
 
 
+def battery_value_export_forward_test(my_predbat):
+    """Pin the export-recovery ratio to the forward base export tariff.
+
+    The haircut asks "can I sell surplus?", which is a property of the tariff. Answering it with
+    rate_export_max - the highest export price anywhere in the horizon, saving sessions included -
+    let a single 2-hour 100p session switch the haircut off for a whole 48-hour plan on a system whose
+    export tariff pays nothing. The value then sat above what a stored kWh can realise, and the planner
+    freeze charged every window up to end_record because holding energy scored better than using it.
+
+    Scoping to the forward base tariff fixes both halves: sessions never enter it, and a session that
+    has already passed cannot keep counting.
+    """
+    failed = False
+    saved = {name: getattr(my_predbat, name) for name in VALUE_RATE_STATE}
+    try:
+        my_predbat.inverter_loss = 0.96
+        my_predbat.battery_loss = 0.97
+        my_predbat.battery_loss_discharge = 0.97
+        my_predbat.metric_battery_cycle = 0.0
+        my_predbat.rate_export_min = 0.0
+        my_predbat.rate_min = 6.9
+        my_predbat.rate_max = my_predbat.rate_max_base = 28.85
+        my_predbat.rate_min_forward = {10: 6.9}
+        my_predbat.metric_battery_value_export_scaling = 0.8
+        full = 6.9 / 0.96 / 0.97
+
+        # A saving session pushes rate_export_max to 100p, but the export tariff itself pays nothing,
+        # so the forward base view is 0 and the full haircut must still apply.
+        my_predbat.rate_export_max = 100.0
+        my_predbat.rate_export_max_forward = {10: 0.0}
+        expected = full * 0.8
+        got = my_predbat.battery_value_rate(10)
+        if abs(got - expected) > 1e-6:
+            print("ERROR: battery_value_rate is {} with a saving session in rate_export_max, expected the haircut {} - a one-off event is standing in for the tariff".format(got, expected))
+            failed = True
+
+        # Base tariff that genuinely recovers half the import cost takes half the haircut
+        my_predbat.rate_export_max_forward = {10: 3.45}
+        expected = full * 0.9
+        got = my_predbat.battery_value_rate(10)
+        if abs(got - expected) > 1e-6:
+            print("ERROR: battery_value_rate is {} with a base export of 3.45, expected {}".format(got, expected))
+            failed = True
+
+        # A real export tariff matching the cheapest import still takes no haircut - the forward view
+        # must not be read as "export is always worthless"
+        my_predbat.rate_export_max_forward = {10: 6.9}
+        got = my_predbat.battery_value_rate(10)
+        if abs(got - full) > 1e-6:
+            print("ERROR: battery_value_rate is {} with a base export matching import, expected the undiscounted {}".format(got, full))
+            failed = True
+
+        # No forward data at all (an older debug file being replayed) falls back to rate_export_max
+        my_predbat.rate_export_max = 3.45
+        my_predbat.rate_export_max_forward = {}
+        expected = full * 0.9
+        got = my_predbat.battery_value_rate(10)
+        if abs(got - expected) > 1e-6:
+            print("ERROR: battery_value_rate is {} with no forward export data, expected the rate_export_max fallback {}".format(got, expected))
+            failed = True
+    finally:
+        for name, value in saved.items():
+            setattr(my_predbat, name, value)
+    return failed
+
+
 def run_compute_metric_tests(my_predbat):
     """
     Test the compute metric function
     """
     failed = False
     print("**** Running compute metric tests ****")
     failed |= battery_value_rate_test(my_predbat)
+    failed |= battery_value_rate_ceiling_base_test(my_predbat)
     failed |= battery_value_export_scaling_test(my_predbat)
+    failed |= battery_value_export_forward_test(my_predbat)
     state_dict = save_state(my_predbat)
     failed |= compute_metric_test(my_predbat, "zero", assert_metric=0)
     failed |= compute_metric_test(my_predbat, "cost", cost=10.0, assert_metric=10)