Consolidate PredictionSnapshotStore into the unified per-day DailyView format - #490
Open
johanzander wants to merge 9 commits into
Open
Consolidate PredictionSnapshotStore into the unified per-day DailyView format#490johanzander wants to merge 9 commits into
johanzander wants to merge 9 commits into
Conversation
First of two planned PRs folding PredictionSnapshotStore and ScheduleStore into the DailyView per-day file format, split by risk since ScheduleStore sits on the hot path for inverter writes. This covers the lower-risk store.
… file (#409) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses final whole-branch review findings on the prediction snapshot
consolidation.
- PredictionSnapshotStore now tracks the calendar day its in-memory list
belongs to and re-anchors on every public read/write. The store is
created once per process, so folding persistence into the per-day file
rolled the *file* over at midnight but left self._snapshots growing
across days (unbounded rewrite churn plus cross-day leakage into the
prediction-analysis API). clear() stays as an optional manual reset.
- Serialize the shared {date}.json load-mutate-write cycle behind a
process-wide lock exported from daily_view_store (container_transaction
/ read_container), reused by both stores, and give each write a unique
temp filename so concurrent writers cannot share a temp path.
- Best-effort cleanup of orphaned *.tmp siblings before each write; they
are invisible to the *.json housekeeping globs.
- Refresh stale docstrings in both modules.
Tests: day-rollover regression on a single long-lived store with a moving
mocked date, and a threaded interleaved-write test asserting no lost
update. Both fail against the pre-fix code.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Plan doc served its purpose driving subagent-driven implementation; dropping it now that the code and tests are the source of truth. Spec stays as the design record.
johanzander
marked this pull request as ready for review
August 7, 2026 15:16
…er race container_lock only serialized the shared file's read-modify-write cycle, not self._current_date/self._snapshots. A scheduler-thread call straddling midnight could have its snapshot appended to the wrong day's in-memory list after a concurrent request-thread call rolled the store over, silently misfiling or losing it. Add an instance-level lock around each public method's full ensure-day/mutate/save critical section. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PredictionSnapshotStore's persistence intoDailyViewStore's existing per-day file (/data/daily_views/{date}.json), under a new"snapshots"key alongside the existing"view"key, instead of its own bespoke/data/bess_prediction_snapshots.json.ScheduleStoreconsolidation is deliberately deferred to a separate PR given its hot-path role in inverter writes (discharge-rate gating, export curtailment) vs. this store's purely display/diagnostic role. Issue Consolidate PredictionSnapshotStore + ScheduleStore into the unified per-day DailyView format #409 stays open until that second PR lands.PredictionSnapshotStoreis unchanged — no call-site edits beyond the necessary import-path fix for two relocated deserialization helpers.Root cause
Per #409 (follow-up to #408's
HistoricalDataStorepersistence work): the "today" store landscape was still fragmented across three persistence formats/locations.PredictionSnapshotStorehad its own file, discarded at midnight, separate from theDailyViewschemaHistoricalDataStore/DailyViewStorealready share.Design
Full design doc:
docs/superpowers/specs/2026-08-06-prediction-snapshot-consolidation-design.md. Key points:_load_container/_write_containerhelpers indaily_view_store.pydo an atomic (temp-file +os.replace) read-modify-write, soDailyViewStore(touches only"view") andPredictionSnapshotStore(touches only"snapshots") never clobber each other._daily_view_from_dict,_period_data_from_dict) relocated fromprediction_snapshot.pytodaily_view_builder.py, which already owns the dataclasses they construct."view"/"snapshots"wrapper) still load correctly.clear()call is redundant" — only held for the on-disk side.PredictionSnapshotStoreis a long-lived process singleton; without a rollover mechanism in the in-memory cache itself, snapshots would have accumulated forever (unbounded growth, stale cross-day data leaking into API responses). Fixed with date-aware lazy rollover (_ensure_current_day(), checked before every state-touching method) instead of relying on an externalclear()call.Test plan
./scripts/quality-check.sh/.venv/bin/pytest -m "not slow"passes locally (1537 passed, 13 skipped).venv/bin/pytest -m slowpasses locally (391 passed, 3 skipped)test_prediction_snapshot_store.py(round-trip persistence, ordering, nearest-match, day-rollover with a single long-lived instance across a date change, concurrent-write locking with a realthreading.Barrier-forced interleaving), extendedtest_daily_view_store.py(wrapped container shape, legacy-format fallback), regression test intest_bsm_settings_and_lifecycle.pyproving the redundantclear()call is gone.docker-compose.ci.yml,ci-growatt-vppscenario, faketime-pinned to the scenario's date): ran a real optimization cycle, confirmed/data/daily_views/{date}.jsoncontains both"view"and"snapshots"keys with real data (96 periods, 2 captured snapshots), confirmedGET /api/prediction-analysis/snapshotsand/timelinereturn the expected shape, confirmed no legacy/data/bess_prediction_snapshots.jsonfile is ever created.Part of #409 — this PR covers
PredictionSnapshotStoreonly.ScheduleStoreconsolidation is tracked as a follow-up PR against the same issue.