Skip to content

fix: guard against empty dict in clean_incrementing_reverse#3608

Merged
springfall2008 merged 2 commits intomainfrom
copilot/fix-clean-incrementing-reverse-crash
Mar 20, 2026
Merged

fix: guard against empty dict in clean_incrementing_reverse#3608
springfall2008 merged 2 commits intomainfrom
copilot/fix-clean-incrementing-reverse-crash

Conversation

Copy link
Contributor

Copilot AI commented Mar 20, 2026

On fresh installs, pv_today history is often < 24 hours old (age.days == 0), producing an empty mdata dict that gets passed to clean_incrementing_reverse(), which immediately crashes on max(data).

Change

Added an early-return guard in utils.py:

def clean_incrementing_reverse(data, max_increment=0):
    new_data = {}
    if not data:
        return new_data  # nothing to clean
    length = max(data) + 1

Returning {} is semantically correct — the caller (minute_data) already handles empty mdata gracefully, and there is nothing to smooth when no data exists.

Original prompt

This section details on the original issue you should resolve

<issue_title>clean_incrementing_reverse crashes with ValueError: max() arg is empty when history < 24 hours</issue_title>
<issue_description>## Description

clean_incrementing_reverse() in utils.py crashes with ValueError: max() arg is an empty sequence when called with an empty dict. This happens during the SolarAPI / PV calibration flow on a fresh install where pv_today history is less than 24 hours old.

Stack trace

File "/config/solcast.py", line 967, in fetch_pv_forecast
    pv_forecast_minute, pv_forecast_minute10, pv_forecast_data = self.pv_calibration(...)
File "/config/solcast.py", line 657, in pv_calibration
    pv_today_hist = self.base.minute_data_import_export(days, self.now_utc, "pv_today", required_unit="kWh", pad=False)
File "/config/fetch.py", line 578, in minute_data_import_export
    import_today, _ = minute_data(...)
File "/config/utils.py", line 624, in minute_data
    mdata = clean_incrementing_reverse(mdata, max_increment)
ValueError: max() arg is an empty sequence

Root cause

In minute_data_import_export(), when pad=False is passed (as pv_calibration does), the function uses age.days instead of the requested max_days_previous:

age = now_utc - last_updated_time
age_days = min(age_days, age.days)

If the oldest history entry is less than 24 hours old, age.days == 0. This is passed to minute_data() which computes minute_min = 0 and minute_max = 0, so no history entries fall within bounds and mdata remains {}.

mdata is then passed to clean_incrementing_reverse(), which immediately calls max(data) on the empty dict:

def clean_incrementing_reverse(data, max_increment=0):
    new_data = {}
    length = max(data) + 1  # <-- crashes here when data is {}

Steps to reproduce

  1. Fresh Predbat install with Solcast enabled
  2. pv_today sensor (e.g. sensor.foxess_solar_energy_today) has less than 24 hours of history
  3. Predbat runs its first cycle with SolarAPI → crash every cycle until history crosses 24h

Suggested fix

Add an empty-data guard at the top of clean_incrementing_reverse():

def clean_incrementing_reverse(data, max_increment=0):
    new_data = {}
    if not data:
        return new_data
    length = max(data) + 1

Workaround applied

I patched utils.py directly on the addon container by adding an empty-data guard to clean_incrementing_reverse():

def clean_incrementing_reverse(data, max_increment=0):
    new_data = {}
    if not data:
        return new_data  # <-- added guard
    length = max(data) + 1

This works because:

  • The function is only called from minute_data() when clean_increment=True and data is backwards (historical). When mdata is empty, there is nothing to clean — returning an empty dict is the correct no-op behaviour.
  • The caller (minute_data) already handles empty mdata gracefully downstream — it simply means no historical data was available for that time window.
  • After applying this patch and restarting Predbat, the SolarAPI crash stopped immediately. Predbat completed its cycle successfully with status "Demand" and no errors.

Note: This patch will be overwritten on the next Predbat addon update since the addon replaces all source files. However, the crash condition is only triggered during the first ~24 hours after a fresh install (while age.days == 0), so it is unlikely to recur after an update on an established system.

Environment

  • Predbat v8.34.5 (standalone addon)
  • Inverter: FoxESS KH10-AUX (foxess_modbus v1.13.6)
  • Solcast: BJReplay/ha-solcast-solar v4.5.1
  • Home Assistant 2026.3.2
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: springfall2008 <48591903+springfall2008@users.noreply.github.com>
Agent-Logs-Url: https://github.com/springfall2008/batpred/sessions/26f8b1a6-4742-493e-b4d7-7ec07e99c62d
Copilot AI changed the title [WIP] Fix crash in clean_incrementing_reverse when history is empty fix: guard against empty dict in clean_incrementing_reverse Mar 20, 2026
Copilot AI requested a review from springfall2008 March 20, 2026 19:09
@springfall2008 springfall2008 marked this pull request as ready for review March 20, 2026 19:10
Copilot AI review requested due to automatic review settings March 20, 2026 19:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a startup crash in Predbat’s historical minute-data smoothing path by making clean_incrementing_reverse() a no-op when called with empty data (which can occur on fresh installs when history age results in days=0).

Changes:

  • Add an early return in clean_incrementing_reverse() when data is empty to avoid max(data) on an empty dict.

Comment on lines 650 to +652
new_data = {}
if not data:
return new_data
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard fixes the crash, but there’s no regression test covering the specific failure mode (clean_increment=True with days=0 / empty mdata). Please add a unit test (e.g., in apps/predbat/tests/test_minute_data.py) that exercises minute_data() with days=0 and clean_increment=True and asserts it returns an empty dict without raising.

Copilot generated this review using guidance from repository custom instructions.
@springfall2008 springfall2008 merged commit f6db0ae into main Mar 20, 2026
5 checks passed
@springfall2008 springfall2008 deleted the copilot/fix-clean-incrementing-reverse-crash branch March 20, 2026 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clean_incrementing_reverse crashes with ValueError: max() arg is empty when history < 24 hours

3 participants