fix(export): make the encoding's producers and its readers agree

Review of the split found two places where a limit was produced in a shape the rest of the
encoding does not accept, and both only bite at the top of the target range - exactly where
the packed encoding used to run out of room, which is why neither side's tests saw them.

calculate_yesterday's history reconstruction still appended a bare SoC percentage for a real
export, while its freeze sibling had been converted. A bare number decodes through the legacy
compatibility path, so it works by accident below 99 and every mode assertion passes either
way - but a slot that ended at 99% reads back as a freeze and one at 100% as an idle window,
dropping it from the History view.

The stored-form validator rejected any target at or above the freeze sentinel, a bound the
tuple form has no reason to keep. clip_export_slots genuinely produces the top of the range:
it narrows a target towards the SoC the simulation says is reachable, so a near-full battery
on a derated discharge rate clips to 99 or 100. export_limit_to_stored wrote those out
faithfully and export_limit_from_stored read them back as idle, so a restored plan or a
replayed debug dump silently lost the export. The bound is now the representable range.

Also from review, none of it behavioural:

- create_debug_yaml converted only the two Predbat-level limit lists. Each inverter keeps its
  own copy, which rode into the dump as a bare sequence and came back from a replay as a list,
  where export_mode_of compares it against a float and raises. Written and read the same way
  as the other two now.
- execute.py asked for a target as `export_target_of(...) or 0` in five places. That reads as
  a guard against a falsy target, which is not what is being guarded - the callers are all
  inside a target-mode branch, so it is a type normalisation with an unreachable fallback.
  Spelled out as export_target_percent_or_zero().
- prediction_kernel checks at import that struct's "@iid" layout really does match
  PkExportLimit, rather than reasoning about it in a comment. from_buffer_copy already catches
  a size mismatch, but not two layouts of the same size with the double at a different offset,
  and armv7l/i686 are cross-built so nobody runs them here. A mismatch drops to per-field
  assignment rather than raising, matching how this module treats a kernel it cannot trust.
- export_limit_sort_key documents that it is deliberately lossy at the top of the range: a 99%
  target and a freeze both key as 99.0 because the display paths print this number. Ordering
  and display only - every mode test goes through export_mode_of, which reads the field.
- inverter.py uses FULL_EXPORT_POWER rather than a bare 1.0.

Tests: the encoding round trips now sweep the full 0-100 target range; test_clip_export_slots
pairs the clip pass with the validator so the two cannot disagree again; calculate_yesterday
pins that a rebuilt limit is an instruction rather than a bare number; and the marshaller's
fast and fallback paths are compared field for field. Each was confirmed to fail with its fix
reverted. Full quick suite green with PREDBAT_KERNEL_REQUIRED=1, pre-commit clean.

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

Author: 2 people

apps/predbat/execute.py

@@ -93,6 +93,23 @@ def build_status_extra(status_extra_parts):
     return status_extra
 
 
+def export_target_percent_or_zero(export_limit):
+    """The SoC percentage an export instruction targets, or 0 where it carries no target.
+
+    Only EXPORT_MODE_TARGET has a target; export_target_of returns None for the other two modes so a
+    caller cannot use their sentinels as if they were one. Every value this feeds here - the
+    discharge floor, the displayed target, the inverter target register - is a percentage, so the
+    two modes resolve to the bottom of the range rather than to None.
+
+    Spelled out rather than written `or 0` at each site: that reads as a guard against a falsy
+    target, which is not what is being guarded. The distinction matters because the callers all sit
+    inside a target-mode branch already, so the fallback is unreachable for them and a reader needs
+    to see that it is a type normalisation and not a live default.
+    """
+    target = export_target_of(export_limit)
+    return 0 if target is None else target
+
+
 class Execute:
     """Execution mixin for applying optimised plans to physical inverters.
 
@@ -478,7 +495,7 @@ def execute_plan(self):
                 # Turn minutes into time
                 discharge_start_time = self.midnight_utc + timedelta(minutes=minutes_start)
                 discharge_end_time = self.midnight_utc + timedelta(minutes=(minutes_end + export_adjust))  # Add in 1 minute margin to allow Predbat to restore demand mode
-                discharge_soc = max(((export_target_of(self.export_limits_best[0]) or 0) * self.soc_max) / 100.0, self.reserve, self.best_soc_min)
+                discharge_soc = max((export_target_percent_or_zero(self.export_limits_best[0]) * self.soc_max) / 100.0, self.reserve, self.best_soc_min)
                 self.log("Next export window will be: {} - {} at reserve {}".format(discharge_start_time, discharge_end_time, self.export_limits_best[0]))
                 if (self.minutes_now >= minutes_start) and (self.minutes_now < minutes_end) and (export_mode_of(self.export_limits_best[0]) != EXPORT_MODE_IDLE):
                     if not self.set_export_freeze_only and export_mode_of(self.export_limits_best[0]) == EXPORT_MODE_TARGET and (self.soc_kw > discharge_soc):
@@ -500,7 +517,7 @@ def execute_plan(self):
                         # instruction's own target rather than to the instruction itself
                         target = self.export_window_best[0].get("target")
                         if target is None:
-                            target = export_target_of(self.export_limits_best[0]) or 0
+                            target = export_target_percent_or_zero(self.export_limits_best[0])
                         self.isExporting_Target = int(target)
 
                         status = "Exporting"
@@ -530,14 +547,14 @@ def execute_plan(self):
                             isExporting = True
                             target = self.export_window_best[0].get("target")
                             if target is None:
-                                target = export_target_of(self.export_limits_best[0]) or 0
+                                target = export_target_percent_or_zero(self.export_limits_best[0])
                             self.isExporting_Target = int(target)
                         else:
                             status = "Hold exporting"
                             status_per_inverter[inverter.id] = status
                             target = self.export_window_best[0].get("target")
                             if target is None:
-                                target = export_target_of(self.export_limits_best[0]) or 0
+                                target = export_target_percent_or_zero(self.export_limits_best[0])
                             status_extra_parts.append((inverter.id, "target", status, "{}%-{}%".format(inverter.soc_percent, inverter.soc_percent)))  # append multi-inverter target SoC's together
                             self.isExporting_Target = inverter.soc_percent
                             self.log("Export Hold (Demand mode) as export is now at/below target or freeze only is set - current SoC {}kWh and target {}kWh".format(self.soc_kw, discharge_soc))
@@ -843,7 +860,7 @@ def export_target_soc_percent(self):
         Returns:
         - int: export target as a percentage of the battery
         """
-        target = export_target_of(self.export_limits_best[0]) or 0
+        target = export_target_percent_or_zero(self.export_limits_best[0])
         if not self.set_reserve_enable:
             target = max(target, calc_percent_limit(max(self.reserve, self.best_soc_min), self.soc_max))
         return target

apps/predbat/inverter.py

@@ -32,6 +32,7 @@
     INVERTER_MAX_RETRY,
     EXPORT_MODE_TARGET,
     EXPORT_MODE_IDLE,
+    FULL_EXPORT_POWER,
     INVERTER_WRITE_POLL_INTERVAL,
     INVERTER_WRITE_POLL_MAX_INTERVAL,
     INVERTER_CLOCK_SKEW_RESTART_MINUTES,
@@ -1751,7 +1752,7 @@ def update_status(self, minutes_now, quiet=False):
 
         # Pre-fill best discharge enables
         if self.discharge_enable_time:
-            self.export_limits = [pack_export_limit(EXPORT_MODE_TARGET, 0, 1.0) for i in range(len(self.export_window))]
+            self.export_limits = [pack_export_limit(EXPORT_MODE_TARGET, 0, FULL_EXPORT_POWER) for i in range(len(self.export_window))]
         else:
             self.export_limits = [pack_export_limit(EXPORT_MODE_IDLE) for i in range(len(self.export_window))]
 

apps/predbat/output.py

@@ -3483,7 +3483,12 @@ def calculate_yesterday(self):
                     else:
                         soc_was = battery_soc_yesterday_array.get(export_end_minute, 0.0)
                         soc_percent = calc_percent_limit(soc_was, self.soc_max)
-                        self.export_limits_best.append(soc_percent)
+                        # A target instruction, not the bare percentage this used to append. A bare
+                        # number still decodes through the legacy path, but only by accident, and
+                        # not at the top of the range: a slot that ended at 99% read back as a
+                        # freeze and one at 100% as an idle window, dropping it from the History
+                        # view entirely.
+                        self.export_limits_best.append(pack_export_limit(EXPORT_MODE_TARGET, soc_percent))
 
                 if "charging" in charge_during_slot:
                     # Assume charging at this time

apps/predbat/prediction_kernel.py

@@ -405,6 +405,22 @@ def double_array(values):
 # because it is one C-level pack per window instead of three Python attribute stores.
 _EXPORT_LIMIT_STRUCT = struct.Struct("@iid")
 
+# That reasoning is checked rather than trusted. from_buffer_copy already rejects a total-size
+# mismatch, but two layouts of the same size with the double at a different offset would pack wrong
+# and run on silently - and the platforms most likely to differ (armv7l, i686) are cross-built, so
+# nobody exercises them here. Both the size and the field offsets are compared, the offsets derived
+# from the same format string rather than written out, so this cannot drift from the Struct above.
+#
+# A mismatch disables the fast path instead of raising: the struct packing is an optimisation, and
+# the per-field fallback below is correct on any layout. That matches how the rest of this module
+# treats a kernel it cannot trust - fall back to something slower that works, rather than take the
+# process down.
+_EXPORT_LIMIT_STRUCT_USABLE = _EXPORT_LIMIT_STRUCT.size == ctypes.sizeof(PkExportLimit) and tuple(getattr(PkExportLimit, name).offset for name in ("mode", "target", "power")) == (
+    0,
+    struct.calcsize("@i"),
+    struct.calcsize("@ii0d"),
+)
+
 # Contents-keyed cache of marshalled export-limit buffers for the single-run path
 # (run_prediction_kernel). The batch path caches by list identity, which never hits here because
 # every prediction is handed a fresh list; a search fans out thousands of simulations over the same
@@ -423,9 +439,20 @@ def _build_export_limit_array(export_limits):
     below, not here, because an unnormalised legacy element (a list or a dict) is unhashable and
     would break tuple(export_limits) itself, not just this packing loop.
     """
+    buffer = PkExportLimit * len(export_limits)
+    if not _EXPORT_LIMIT_STRUCT_USABLE:
+        # Struct packing is not layout-identical to the ctypes struct on this platform, so fill the
+        # fields one at a time instead. Slower, and never taken on any platform Predbat ships.
+        packed = buffer()
+        for index, (mode, target, power) in enumerate(export_limits):
+            entry = packed[index]
+            entry.mode = mode
+            entry.target = target if mode == EXPORT_MODE_TARGET else 0
+            entry.power = power if mode == EXPORT_MODE_TARGET else FULL_EXPORT_POWER
+        return packed
     pack = _EXPORT_LIMIT_STRUCT.pack
     raw = b"".join(pack(mode, target, power) if mode == EXPORT_MODE_TARGET else pack(mode, 0, FULL_EXPORT_POWER) for mode, target, power in export_limits)
-    return (PkExportLimit * len(export_limits)).from_buffer_copy(raw)
+    return buffer.from_buffer_copy(raw)
 
 
 def export_limit_array(export_limits):

apps/predbat/tests/test_calculate_yesterday.py

@@ -1627,6 +1627,15 @@ def _capture_publish_html_plan(*args, **kwargs):
         if unmarked:
             print("ERROR: {} of {} rebuilt export slots held two states but were not marked as mixed".format(len(unmarked), len(captured["export_window_best"])))
             failed = True
+        # A rebuilt limit has to be a real export instruction, not the bare SoC percentage this used
+        # to append. A bare number still decodes through the legacy compatibility path, so every
+        # mode assertion above passes either way - but only for targets below 99, where the packed
+        # encoding's reserved values begin: a slot that ended at 99% would read back as a freeze and
+        # one at 100% as an idle window, dropping it from the History view.
+        bare = [limit for limit in captured["export_limits_best"] if not isinstance(limit, tuple)]
+        if bare:
+            print("ERROR: {} of {} rebuilt export limits are bare numbers rather than (mode, target, power) instructions: {!r}".format(len(bare), len(captured["export_limits_best"]), bare))
+            failed = True
 
     _restore_methods(my_predbat, original_run_pred)
     my_predbat.savings_last_updated = None

apps/predbat/tests/test_clip_export_slots.py

@@ -9,7 +9,7 @@
 # pylint: disable=attribute-defined-outside-init
 from tests.test_infra import reset_inverter
 from const import EXPORT_MODE_FREEZE, EXPORT_MODE_IDLE, EXPORT_MODE_TARGET
-from utils import export_mode_of, export_target_of, export_power_of, pack_export_limit
+from utils import export_mode_of, export_target_of, export_power_of, pack_export_limit, export_limits_to_stored, export_limits_from_stored
 
 
 def run_clip_export_slots_tests(my_predbat):
@@ -24,6 +24,7 @@ def run_clip_export_slots_tests(my_predbat):
     failed |= test_normal_export_clipped_up_when_soc_above_reserve_with_zero_limit(my_predbat)
     failed |= test_normal_export_clipped_up_when_soc_flat_above_limit(my_predbat)
     failed |= test_export_power_does_not_shift_the_clip_target(my_predbat)
+    failed |= test_clipped_high_target_is_storable(my_predbat)
     failed |= test_disabled_window_ignored(my_predbat)
     failed |= test_passed_window_clipped(my_predbat)
     failed |= test_multiple_windows_mixed(my_predbat)
@@ -250,6 +251,40 @@ def test_export_power_does_not_shift_the_clip_target(my_predbat):
     return failed
 
 
+def test_clipped_high_target_is_storable(my_predbat):
+    """A target clipped to the top of the range survives being saved and reloaded.
+
+    The clip pass narrows a target towards the SoC the simulation says is reachable, so a battery
+    predicted to sit near full with a derated discharge rate clips to 99% or 100%. The stored-form
+    validator used to reject anything at or above the old freeze sentinel, so the write side emitted
+    those faithfully and the read side turned them into an idle window - the export vanished when a
+    plan was restored after a restart, or when a debug dump was replayed. Paired with the encoding
+    test of the same name so the clip pass and the validator cannot disagree again unnoticed.
+    """
+    print("**** test_clipped_high_target_is_storable ****")
+    failed = False
+    setup(my_predbat)
+
+    for rate_kw, soc_kwh in ((0.06, 9.95), (0.06, 9.99), (0.3, 9.99)):
+        my_predbat.battery_rate_max_discharge = rate_kw / 60.0
+        window = [make_window(0, 60)]
+        limits = [pack_export_limit(EXPORT_MODE_TARGET, 50)]
+        _, clipped = my_predbat.clip_export_slots(0, make_predict_soc(0, soc_kwh), window, limits, 1, 5)
+        limit = clipped[0]
+        if export_mode_of(limit) != EXPORT_MODE_TARGET:
+            print("ERROR: clipping a {}kWh SoC at {}kW gave mode {}, expected a target".format(soc_kwh, rate_kw, export_mode_of(limit)))
+            failed = True
+            continue
+        if export_target_of(limit) < 99:
+            print("ERROR: clipping a {}kWh SoC of 10.0kWh at {}kW gave target {}, expected the top of the range - the fixture no longer exercises the case".format(soc_kwh, rate_kw, export_target_of(limit)))
+            failed = True
+        restored = export_limits_from_stored(export_limits_to_stored([limit]))[0]
+        if restored != limit:
+            print("ERROR: the clipped limit {} did not survive a store/load round trip, became {}".format(limit, restored))
+            failed = True
+    return failed
+
+
 def test_disabled_window_ignored(my_predbat):
     """Windows with limit=100 should be ignored and left unchanged"""
     print("**** test_disabled_window_ignored ****")