Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion apps/predbat/output.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 64 additions & 0 deletions apps/predbat/tests/test_calculate_yesterday.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,69 @@ 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.

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)

# 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)

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 = soc_kwh_history_before
_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.

Expand Down Expand Up @@ -2025,6 +2088,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)
Expand Down
Loading