Name debug-history snapshots .yaml.txt so they attach to GitHub without renaming - #4941
Conversation
…veats, test gaps Co-Authored-By: Claude Code <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
resolve_and_load_snapshot()’s documented (None, None) return contract isn’t enforced when all fallbacks miss, which can leak a resolved id with missing data and should be made consistent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates Predbat’s rolling debug-history snapshot naming so snapshots written to debug/ end in .yaml.txt, allowing users to attach them to GitHub issues without manual renaming (while keeping .yaml in the middle for recognisability). It also adds backwards-compatible loading/pruning for pre-rename snapshots and updates the bug-report guidance/docs accordingly.
Changes:
- Change debug-history snapshot filenames from
predbat_debug_<id>.yamltopredbat_debug_<id>.yaml.txt, with fallback support for legacy on-disk.yamlnames and pre-#4720 cache keys. - Ensure eviction cleans up both the new and legacy on-disk filenames to avoid disk leaks after upgrade.
- Update docs/issue template and expand tests to cover the new naming and fallback behaviors (including web route
Content-Disposition).
File summaries
| File | Description |
|---|---|
| apps/predbat/debug_history.py | Implements .yaml.txt naming plus legacy filename fallback and dual-name deletion on eviction. |
| apps/predbat/tests/test_debug_history.py | Adds/updates unit coverage for new naming, fallback loading, and eviction behavior. |
| apps/predbat/tests/test_web_debug_history_routes.py | Verifies web download filenames end in .yaml.txt and legacy-only on-disk snapshots still serve. |
| docs/customisation.md | Updates debug-history documentation to reflect the new on-disk snapshot name and legacy behavior. |
| docs/web-interface.md | Updates Dash tab guidance about GitHub attachment restrictions and renaming requirements. |
| .github/ISSUE_TEMPLATE/bug_report.md | Removes the “rename download” step and documents the debug/ folder workaround for HA Companion app users. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| data = await storage.load_debug_copy(snapshot_filename(snapshot_id)) | ||
| if data is None: | ||
| if not data: | ||
| data = await storage.load_debug_copy(_legacy_snapshot_filename(snapshot_id)) | ||
| if not data: | ||
| data = await storage.load(STORAGE_MODULE, _legacy_snapshot_key(snapshot_id)) | ||
| return snapshot_id, data |
Distils the 62 triage/PR logs that had accumulated since the last pass (2026-08-27 through 09-05) into the journal, verifying every claim against main at ce47a1e before keeping it. Corrects two entries that had gone stale: - The component auto-config gate claimed GH#4786 was still live and the solar sibling unreproduced. The GE Cloud half was fixed by shared_config in v8.54.0; the solar one has since been reproduced and still fires, and shared_config cannot fix it because pv_forecast_today is in that component's own required_or. - The Companion-app bullet still pointed at .txt snapshots in cache/. They moved to debug/ in #4743 and were renamed .yaml.txt in #4941; create_debug_yaml()'s per-cycle dump is still a bare .yaml. Extends the Fox, Solis, Sigenergy, Solcast, Octopus, GE Cloud, Teslemetry, HA write/verify and Predheat rows, and adds rows for GivTCP REST power, Compare, car charging, Web/MCP/Chat, self-update and the cloud/divergence model - including that metric_load_divergence and the central/PV90 cloud modulation were inert before v8.55.0, which rules them out as an explanation in any older log. Adds three traps: check a report's config keys exist before verifying its logic (two reports in a week were built on keys that do not), grid_power is negative when importing and predbat_metrics.py is the outlier that says otherwise, and a consolidated "the fixture encodes the bug" check with five instances. Findings that were mechanisms rather than confirmed causes are recorded as such, or left out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This is an automated draft PR generated from issue #4932 — a maintainer should review it before merging.
Fixes #4932
Summary
Since #4720 the rolling debug-history snapshots are written into
debug/(so HA Companion-app users can pick them up over Samba/File editor, the download links being unusable in that webview) — but undersnapshot_filename()'spredbat_debug_<id>.yamlname. GitHub refuses a bare.yamlas an issue attachment, so getting one onto a bug report meant SSHing in to rename it first.snapshot_filename()now returnspredbat_debug_<id>.yaml.txt, matching what the interactive web download has always served (predbat_debug.yaml.txt). The content is unchanged — still plain YAML — and keeping.yamlin the middle meansunit_test.py --debug_fileand editors still recognise it.Because renaming the on-disk file orphans everything an upgrading install already has in
debug/, a_legacy_snapshot_filename()fallback was added alongside the existing pre-#4720 cache-key one:resolve_and_load_snapshot()falls back to the old.yamlname, so snapshots captured before the upgrade stay downloadable for the rest of their normal time in the ring._discard_snapshot()deletes both names, so eviction still prunes them. Without this an upgrade would drop them from the index but leave them on disk forever — up todebug_history_countfiles at several MB each.Also, as the issue asks, the stale instruction in the bug-report template: the Dash-tab download already arrives as
predbat_debug.yaml.txt, so the "rename the download file" step is no longer needed. The template now says to upload it as-is, and adds where to find the files indebug/for anyone the download link doesn't work for.docs/customisation.md's debug-history section is updated to the new filename.Testing
From
coverage/:./run_pre_commit— all 12 hooks Passed, and the quick suite it runs finishedAll tests passed (4 slow tests skipped, total time: 109.20s).From the repo root:
tools/triage_test.sh debug_history— PASSED (exit 0), including the six new cases: the.yaml.txtname, that a fresh capture writes only the new name, that a pre-rename.yamlfile still loads through the fallback (both againstFakeStorageand a realStorageLocalFilesbackend on disk), and that eviction deletes the old name off disk.tools/triage_test.sh web_debug_history_routes— PASSED (exit 0), with a new assertion that the single-snapshot download route'sContent-Dispositionfilename ends in.yaml.txt.Notes
GitNexus MCP tools were not available in this session, so the blast radius for
snapshot_filename()was established by search instead: five call sites —debug_history.py:87,121,160,228andweb.py:2946— all of which are covered above. The archive member names insidepredbat_debug_history.tgz.dmpchange to.yaml.txtas a consequence, which seems desirable for the same reason.