Raise debug_history_count maximum to 500 for multi-week history (#5070)

[springfall2008]

This is an automated draft PR generated from issue #5070 — a maintainer should review it before merging.

Fixes #5070

Summary

Raises the debug_history_count schema maximum from 50 to 500 (apps/predbat/config.py), as requested by @chalfontchubby on the issue, and documents the storage cost of using it, as requested in the same thread.

The default stays at 15 and the minimum stays at 1, so nothing changes for an install that never touches the setting — only the ceiling on what a user can opt into moves. The retained window is already time-based (max_age = debug_history_interval × debug_history_count in _capture_debug_history()), so this alone delivers the 336-snapshots-at-1-hour ≈ 14-day window the reporter asked for, with no new mechanism. No code below the schema needed changing: _capture_debug_history() passes the live config value straight through, and the clamp in load_user_config() reads the schema bound, so it now permits 336 from apps.yaml as well as from the HA entity.

The docs change (docs/customisation.md) states the new maximum, spells out that the window is count × interval (so a fortnight can be had at 336×1h or 112×3h), and adds a CAUTION noting that snapshots are full dumps of roughly 2MB each — a 15-snapshot history measures about 32MB on disk, so the 500 ceiling is of the order of 1GB — and that the download all archive loads every retained snapshot into memory to build the tarball.

Testing

New test_debug_history_count_range in apps/predbat/tests/test_debug_history_capture.py (registered in TEST_REGISTRY as debug_history_count_range). It checks the declared range reaches 336 while the default stays 15 and the minimum stays 1, then drives the real clamp path — expose_config + load_user_config() + get_arg(), the same route _capture_debug_history() reads through — to confirm 336 now survives unclamped without flagging had_errors, and that 501 still clamps to 500 and does flag it.

• tools/triage_test.sh debug_history_count_range with the config.py change stashed: FAILS (debug_history_count max should allow at least 336 snapshots, got 50, and expected debug_history_count 336 to pass through unclamped, got 50).
• Same command with the fix in place: PASSES.
• ./run_pre_commit from coverage/: all hooks pass and the full quick suite passes (exit 0).

Notes

• Not fixed, deliberately, and worth the maintainer's attention: load_all_snapshots()/build_archive() (debug_history.py:258-290) load every retained snapshot's full text into memory and gzip it in one pass, so at 500 snapshots the "download all" endpoint would buffer on the order of 1GB per request. Raising the ceiling makes that reachable where it previously was not. The issue thread scoped this PR to the max bump plus docs warnings, so this is documented rather than changed — capping or streaming the archive build looks like its own ticket.
• debug_history_interval is left at its 24-hour maximum; the issue and the thread both asked only about the count, and 500 × 24h is already far past any useful window.
• Blast radius: the only consumer of this value is _capture_debug_history() (confirmed with impact({target: "_capture_debug_history", direction: "upstream"}) — 1 direct caller, update_pred; flagged HIGH only because it sits on the main loop, and this change alters no behaviour at the unchanged default).

[springfall2008]

Automated review of PR #5071 (x-high effort, 10 finder angles + verification). Findings that cannot be anchored inline to the diff (they live in files this PR does not touch, but are direct consequences of the max it raises):

1. [web.py:3014-3018, debug_history.py:258-290] 'Download all' has no bound at the new 10x maximum - memory and event-loop stall.
load_all_snapshots() materialises every retained snapshot's full YAML text into one list, then build_archive() holds all of it plus a second full-size bytes copy per snapshot plus an in-memory tar+gzip BytesIO, all synchronously inside the aiohttp handler (blocking the event loop - the whole web UI stalls during the build). At the worked example this PR's own docs invite (336 snapshots at 1-hour), and with committed replay dumps measuring ~5MB each (coverage/cases/predbat_debug_agile1.yaml = 5.32MB), peak RSS is of the order of 2-3GB inside the single addon process - enough to OOM-kill Predbat mid-download, taking the plan, the web server and the very history the user was trying to attach with it. The only mitigation shipped is the prose caution in docs/customisation.md:886. A streaming tar (mode="w|gz" to a temp file), a per-archive snapshot cap, or a size guard before building would bound this in code; offloading to an executor would unblock the loop.

2. [docs/customisation.md:888 / web.py:3022-3024] covered inline on the new CAUTION paragraph: the unchanged 'archive itself is usually under 5MB' claim is now self-contradictory with the counts this PR advertises.

Verified clean (no finding): no other code hard-codes the old 50 ceiling; APPS_SCHEMA imposes no parallel bound; existing installs pick up the new max correctly because load_user_config() re-pushes min/max/step attributes via expose_config(force_ha=True) (userinterface.py:1090-1137, 556-571); the clamp path the new test asserts through behaves exactly as expected (336 unclamped, 501->500 clamped with had_errors=True); the window arithmetic in the comments/docs is correct (336x1h = 14 days, 112x3h = 336h, defaults 15x3h = 45h); unit_test.py import (line 317) and TEST_REGISTRY tuple (line 760) are correct with the right slow-flag; the new test has a docstring, snake_case names, and no over-length lines. Inline comments are on: config.py:1234 (size figures understated ~2.5x), config.py:1236 (no code-level footprint guard), docs:886 (contradiction with line 888), test:268 (unasserted interval premise), test:296 (clamp-test duplication), test:305 (had_errors global-flag fragility), test:322 (finally block leaves 500 in coverage/predbat_config.json).

[Copilot]

🟡 Changes recommended

The new ceiling can allow download-all requests to buffer roughly 1 GB in memory; a runtime safeguard or streaming archive path is needed.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Raises the debug_history_count maximum from 50 to 500 to support multi-week history retention.

Changes:

• Documents retention sizing and resource costs.
• Adds configuration range and clamping tests.
• Registers the new test.

File summaries

File	Summary
docs/customisation.md	Documents usage, retention windows, and storage/download costs.
apps/predbat/unit_test.py	Registers the new range test.
apps/predbat/tests/test_debug_history_capture.py	Adds range and clamping coverage.
apps/predbat/config.py	Raises the schema maximum to 500.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Moderate issues remain in test coverage, test-state cleanup, and route compatibility.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

apps/predbat/tests/test_debug_history_capture.py:324

• The out-of-range refresh above calls record_status, which also changes current_status and the status entity's error metadata/error count. The finally block restores only the config value and had_errors; because the registry reuses one PredBat instance, this leaves test-generated warning state behind for later tests. Snapshot and restore the status/HA state as well.

apps/predbat/tests/test_debug_history_capture.py:278

• This assertion accepts any maximum of 336 or greater, so it would pass if the requested public ceiling were later reduced from 500 to 337 or 400. Since this PR specifically changes the ceiling to 500, assert the declared maximum is exactly 500 while keeping the separate 336 pass-through check.

    if item.get("max", 0) < fortnight_hourly:
        print("  FAILED: debug_history_count max should allow at least {} snapshots (14 days hourly), got {}".format(fortnight_hourly, item.get("max", None)))

apps/predbat/web.py:3024

• The PR notes say the “download all” path is deliberately left unchanged and only documented, but this call changes its behavior to load at most 16 snapshots (and the route is renamed above). Please reconcile the PR description with the implementation and the issue-thread scope so maintainers know this is also a download/API behavior change.

        storage = self._storage()
        named_snapshots = await debug_history.load_all_snapshots(storage, DEBUG_HISTORY_DOWNLOAD_MAX)

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Lite