From 7dc5637ce3cb18b74fc98b0a02cbffddf50c0347 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 11 Sep 2026 15:19:03 +0100 Subject: [PATCH 1/3] fix(output): savings_today_predbat feeds the total with the same figure the sensor state shows (#3894) self.savings_today_predbat = saving used the unadjusted (real) figure, but it feeds three things that should all track the same quantity sensor.predbat_savings_yesterday_predbat's own *state* already publishes - saving_adjusted: - savings_total_predbat (predbat.py), the running total sensor - the predbat_savings_today_predbat Prometheus gauge - the metrics dashboard's "Savings Yesterday (PredBat)" tile So the running total accumulated positive real savings while the daily HA chart showed the adjusted (frequently negative) figure - a real, climbing total coexisting with daily bars sitting below zero, exactly the reported symptom. The sibling metric, savings_today_pvbat, already used the adjusted figure correctly (self.savings_today_pvbat = saving_no_pvbat_adjusted, matching its own sensor's state=saving_no_pvbat_adjusted) - this was the one inconsistent case. Scoped to this one root cause. The triage identified a second, larger defect - the battery-value adjustment bills the counterfactual battery's closing SoC *level* against the real one every day in perpetuity, rather than the day's *change* in level, so any export strategy that empties the battery overnight sees a permanent ~100p+/day penalty - plus two secondary issues (savings_total_soc unclamped to soc_max; a one-off wrong-baseline window in the first ~2h after midnight). None of those are addressed here; they need a redesign of the adjustment term rather than a one-line fix and are left for a separate PR. Verified with mutation testing: the new test forces a real saving_real/saving_adjusted divergence (soc_kwh_history set to a closing SoC that differs from the mocked prediction's final SoC, since the default fixture has no adjustment to diverge on) and reproduces the reported shape numerically - savings_today_predbat reads 400.0 (real) against a published 315.0 (adjusted) with the bug reintroduced, matching only with the fix in place. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/output.py | 9 +++- .../predbat/tests/test_calculate_yesterday.py | 53 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/apps/predbat/output.py b/apps/predbat/output.py index 152131157..8927c8309 100644 --- a/apps/predbat/output.py +++ b/apps/predbat/output.py @@ -3487,7 +3487,14 @@ def calculate_yesterday(self): dp2(battery_value_baseline), ) ) - self.savings_today_predbat = saving + # saving_adjusted, not the unadjusted saving: this feeds savings_total_predbat (predbat.py) + # and the Prometheus/metrics-dashboard gauge, both of which must track the same figure the + # sensor.predbat_savings_yesterday_predbat *state* already publishes (set to saving_adjusted + # a few lines above), or the running total and the daily value it is meant to be a sum of + # measure two different quantities - a real total climbing while daily bars sit negative + # (GH#3894). The sibling savings_today_pvbat already does this correctly with + # saving_no_pvbat_adjusted; this was the one inconsistent case. + self.savings_today_predbat = saving_adjusted self.savings_today_predbat_soc = final_soc self.savings_today_actual = cost_yesterday self.cost_yesterday_car = cost_yesterday_car diff --git a/apps/predbat/tests/test_calculate_yesterday.py b/apps/predbat/tests/test_calculate_yesterday.py index d363db611..49d8ab57a 100644 --- a/apps/predbat/tests/test_calculate_yesterday.py +++ b/apps/predbat/tests/test_calculate_yesterday.py @@ -359,6 +359,58 @@ def _test_basic_no_car(my_predbat, failed): return failed +def _test_savings_today_predbat_matches_published_adjusted_value(my_predbat, failed): + """self.savings_today_predbat - what accumulates into sensor.predbat_savings_total_predbat - + must equal the same saving_adjusted figure sensor.predbat_savings_yesterday_predbat's own + *state* publishes, not the unadjusted saving_real (GH#3894). + + Before the fix, the running total summed saving_real (self.savings_today_predbat = saving) + while the daily sensor's state - the value a HA dashboard bar chart actually shows - was + saving_adjusted. The two series measure different things, so a real, climbing total could + coexist with daily bars sitting negative: exactly the symptom reported. The sibling metric, + savings_today_pvbat, already used the adjusted figure correctly - this was the one + inconsistent case. + + Forcing a real divergence: with the default fixture, battery_value_yesterday and + battery_value_baseline both come out 0 (no soc_kwh_history is set, so nothing to adjust for) + and saving_real == saving_adjusted, which would pass even with the bug reverted. Setting + soc_kwh_history to a closing SoC that differs from run_prediction's mocked final_soc gives + the real and baseline sides different battery-value adjustments, so the two figures only + match here if the code is actually reading the adjusted value. + """ + print("calculate_yesterday: Test - savings_today_predbat matches the published saving_adjusted, not saving_real (#3894)") + now_utc = _setup_base(my_predbat) + + minutes_back = my_predbat.minutes_now + my_predbat.soc_kwh_history = {minutes_back: 2.0} # differs from the mock's FIXED_FINAL_SOC (5.0) + + captured_load, original_run_pred = _apply_mocks(my_predbat, now_utc, cost_value=100.0, soc_value=5.0) + my_predbat.calculate_yesterday() + + saving_real = my_predbat.get_state_wrapper(my_predbat.prefix + ".savings_yesterday_predbat", attribute="saving_real") + saving_adjusted = my_predbat.get_state_wrapper(my_predbat.prefix + ".savings_yesterday_predbat", attribute="saving_adjusted") + + if saving_real is None or saving_adjusted is None: + print("ERROR: saving_real/saving_adjusted attributes were not published") + failed = True + else: + if abs(saving_real - saving_adjusted) < 1e-9: + print("ERROR: test setup did not produce a real vs adjusted divergence (saving_real={}, saving_adjusted={}) - cannot distinguish the bug".format(saving_real, saving_adjusted)) + failed = True + if abs(my_predbat.savings_today_predbat - saving_adjusted) > 1e-9: + print( + "ERROR: savings_today_predbat ({}) should equal the published saving_adjusted ({}), not saving_real ({}) - the running total (predbat.py) would sum a different quantity than the daily sensor state shows (#3894)".format( + my_predbat.savings_today_predbat, saving_adjusted, saving_real + ) + ) + failed = True + + my_predbat.soc_kwh_history = {} + _restore_methods(my_predbat, original_run_pred) + my_predbat.savings_last_updated = None + return failed + + def _test_forecast_minutes_widened_before_step_data(my_predbat, failed): """Regression test for #4418. @@ -2025,6 +2077,7 @@ def test_calculate_yesterday(my_predbat): failed = _test_early_exit(my_predbat, failed) failed = _test_basic_no_car(my_predbat, failed) + failed = _test_savings_today_predbat_matches_published_adjusted_value(my_predbat, failed) failed = _test_forecast_minutes_widened_before_step_data(my_predbat, failed) failed = _test_car_slot_subtraction(my_predbat, failed) failed = _test_car_slot_from_energy_sensor(my_predbat, failed) From 602730706164dea3e25b652a58f0ecbfcf876c60 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 11 Sep 2026 20:43:05 +0100 Subject: [PATCH 2/3] test(output): fix the #3894 regression test's soc_kwh_history key (Copilot review on #5061) calculate_yesterday() reads battery_soc_yesterday from soc_kwh_history at key minutes_now + 1 (output.py: "minutes_back = self.minutes_now + 1"), not minutes_now. The new test keyed its injected closing SoC at minutes_now, so the lookup silently fell through to the 0.0 default instead of the injected 2.0 - the test still passed, but against the divergence that fallback produced against the mock's 5.0 final_soc, not the soc_kwh_history-driven divergence the docstring describes. Confirmed directly: with the wrong key, actual_final_soc published 0.0; with minutes_now + 1, it correctly publishes the injected 2.0. Re-verified the corrected test still fails when the #3894 fix is reverted, now via the intended path. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/tests/test_calculate_yesterday.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/predbat/tests/test_calculate_yesterday.py b/apps/predbat/tests/test_calculate_yesterday.py index 49d8ab57a..e0ca76551 100644 --- a/apps/predbat/tests/test_calculate_yesterday.py +++ b/apps/predbat/tests/test_calculate_yesterday.py @@ -377,11 +377,17 @@ def _test_savings_today_predbat_matches_published_adjusted_value(my_predbat, fai soc_kwh_history to a closing SoC that differs from run_prediction's mocked final_soc gives the real and baseline sides different battery-value adjustments, so the two figures only match here if the code is actually reading the adjusted value. + + The key soc_kwh_history is read back from is minutes_now + 1 (output.py's + "minutes_back = self.minutes_now + 1"), not minutes_now - keyed at minutes_now this + fixture's actual_final_soc silently fell through to the 0.0 default instead of the injected + value, and the test still passed (against the 0.0-vs-mocked-5.0 divergence that produced by + accident, not the one the docstring above describes) - caught by Copilot review on #5061. """ print("calculate_yesterday: Test - savings_today_predbat matches the published saving_adjusted, not saving_real (#3894)") now_utc = _setup_base(my_predbat) - minutes_back = my_predbat.minutes_now + minutes_back = my_predbat.minutes_now + 1 my_predbat.soc_kwh_history = {minutes_back: 2.0} # differs from the mock's FIXED_FINAL_SOC (5.0) captured_load, original_run_pred = _apply_mocks(my_predbat, now_utc, cost_value=100.0, soc_value=5.0) From ada9a6c9460527d214fb0b75bf471cadf66a5f91 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 11 Sep 2026 21:07:39 +0100 Subject: [PATCH 3/3] test(output): stop the #3894 regression test discarding shared soc_kwh_history (Copilot review on #5061) unit_test.py runs every registered test against one shared PredBat instance, but this test's cleanup reset soc_kwh_history to {} unconditionally rather than restoring whatever an earlier test may have left there - the same state-leak pattern already fixed in the #5052 review. Snapshot before mutating, restore what was actually there on cleanup. Verified directly: seeded soc_kwh_history with unrelated data before running the test, confirmed it comes back unchanged afterward. Co-Authored-By: Claude Sonnet 5 --- apps/predbat/tests/test_calculate_yesterday.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/predbat/tests/test_calculate_yesterday.py b/apps/predbat/tests/test_calculate_yesterday.py index e0ca76551..68a07563f 100644 --- a/apps/predbat/tests/test_calculate_yesterday.py +++ b/apps/predbat/tests/test_calculate_yesterday.py @@ -387,6 +387,11 @@ def _test_savings_today_predbat_matches_published_adjusted_value(my_predbat, fai print("calculate_yesterday: Test - savings_today_predbat matches the published saving_adjusted, not saving_real (#3894)") now_utc = _setup_base(my_predbat) + # unit_test.py runs every registered test against the same shared PredBat instance + # (see create_predbat()), so an earlier test could have left soc_kwh_history non-empty - + # save it, rather than assuming/resetting to {} unconditionally on cleanup, so this test + # does not discard state it did not itself put there (Copilot review on #5061). + soc_kwh_history_before = my_predbat.soc_kwh_history minutes_back = my_predbat.minutes_now + 1 my_predbat.soc_kwh_history = {minutes_back: 2.0} # differs from the mock's FIXED_FINAL_SOC (5.0) @@ -411,7 +416,7 @@ def _test_savings_today_predbat_matches_published_adjusted_value(my_predbat, fai ) failed = True - my_predbat.soc_kwh_history = {} + my_predbat.soc_kwh_history = soc_kwh_history_before _restore_methods(my_predbat, original_run_pred) my_predbat.savings_last_updated = None return failed