fix(state): keep maintenance bounded and retain forecast observations - #1283
Conversation
7f12e1e to
11a76ce
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11a76ceca4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func (f *forecastTracker) enqueueObservation(job forecastObservationJob) { | ||
| if len(f.pendingObservations) == maxPendingForecastObservations { | ||
| f.mu.Lock() | ||
| f.observationOverflow = true | ||
| f.mu.Unlock() | ||
| slog.Error("forecast archive: observation queue full", "start_ms", job.observation.StartMS) | ||
| return |
There was a problem hiding this comment.
When the queue contains 64 jobs and persistence recovers before the next observation tick, observe calls this function before attempting flushObservations; the newly completed interval is therefore discarded, and the immediately following flush can successfully empty the old backlog. Capture the new jobs locally and try draining before rejecting them so recovery does not cause avoidable, permanent loss of measurement evidence.
AGENTS.md reference: AGENTS.md:L22-L25
Useful? React with 👍 / 👎.
| if len(f.pendingObservations) == maxPendingForecastObservations { | ||
| f.mu.Lock() | ||
| f.observationOverflow = true | ||
| f.mu.Unlock() |
There was a problem hiding this comment.
After any queue overflow, this flag is set permanently: neither a successful flush nor later healthy observations clear it. Consequently, learningHealth reports observation_queue_full for the rest of the process lifetime even when the queue is empty, persistence is working, and training is current; track a current/full condition or clear the flag after recovery while retaining any historical failure separately.
AGENTS.md reference: AGENTS.md:L22-L25
Useful? React with 👍 / 👎.
Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
11a76ce to
b293a2c
Compare
1 participant
Problem and result
Hourly maintenance can exclude live storage long enough to delay telemetry and saved settings. A completed forecast observation is also discarded after one transient write deadline.
Closes #1149, #1275, #1276, #1277, #1279 and #1280.
Scope and safety
Core storage and its forecast integration only. Preserve FULL durability for goals and telemetry, verified Parquet before source deletion, immutable forecast IDs, exact energy totals and the existing database schema. No device-control commands or UI files change.
The UI task merged #1278 at 792cd21; this branch includes that base. Pending backup PR #1261 retains its existing work; this PR adds status fields in shared files without changing its backup paths. The separate test synchronization fix is in the preceding commit/PR for #1281.
The existing backup latency admission still fails on the Pi: a synthetic 48 MiB backup delayed a durable setting write by 6.43 seconds, above the unchanged two-second limit. A longer control without backup also hit 8.28 seconds; the stall is not specific to backup. #1246 stays open and is under investigation. The proposal in #1287 reduces telemetry history at ingestion and in archives. This PR does not claim all storage work or real charging is validated.
Verification
PATH=/opt/homebrew/bin:$PATH make verifypasses, including the merged feat(web): simple forecast status with details on demand #1278 UI base.These Pi results exercise synthetic data on real hardware while installed Core keeps running. They do not yet establish a clean full maintenance cycle on the installation's historical database. These fixtures do not establish physical charging behavior.
Checklist