Skip to content

fix(derive): cap the live-workout derive hold so a forgotten session cannot blank Home - #281

Merged
abdulsaheel merged 1 commit into
OpenStrap:mainfrom
DropTabl:fix/forgotten-workout-derive-hold
Aug 26, 2026
Merged

fix(derive): cap the live-workout derive hold so a forgotten session cannot blank Home#281
abdulsaheel merged 1 commit into
OpenStrap:mainfrom
DropTabl:fix/forgotten-workout-derive-hold

Conversation

@DropTabl

@DropTabl DropTabl commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

The bug

Start a workout and forget to finish it, and Home degrades into the bare-day empty state: no stats, and "Nothing recorded for today — Sync the band" — while the strap is connected and syncing fine. Tapping the sync CTA completes a real sync and changes nothing.

Root cause

startWorkout() calls DeriveScheduler.setWorkoutActive(true), which parks all derive work — deliberately, so a heavy isolate spawn can't land mid-ride against GPS, the live map and the BLE drain. The hold's own justification is "a workout is minutes long and its own results are derived at the end anyway, so deferring costs nothing."

That assumption inverts the moment the session is forgotten:

  • capture keeps running (the offload is not gated on the workout), records keep landing,
  • every derive job — including the ones the "Sync the band" CTA queues — stays parked,
  • today's day_result is never built, so Home's bare branch renders the first-run/no-data card,
  • and the card's claim is false on both halves: data was recorded, and syncing cannot help.

Even a relaunch doesn't clear it: _reconcileOrphanedLiveWorkout rehydrates a live session up to 6 h old and re-arms the hold.

The fix

Two changes, one per half of the symptom:

  1. DeriveScheduler gains a workoutHoldCap (default 6 h). Past the cap the session is treated as forgotten and held work drains even though it is still live; jobs arriving after expiry run too, so the pipeline is unwedged for the rest of the session. Ending the workout clears the expiry, so the next session holds again from scratch. Inside the cap nothing changes. Six hours matches AppState._kMaxLiveWorkoutAgeMs, the existing ceiling past which a live session row from a previous run is already judged "almost certainly not something the user is still in".

  2. Home's bare-day card branches on the live workout. While a session holds derivation, the card says "A workout is still running" and points at the session bar pinned directly below it (the door back into the session), instead of claiming nothing was recorded and offering a sync that cannot help. The card clears itself: finishing the workout releases the hold, the derive lands, insightsRevision ticks, and RevisionReload re-reads. Read via context.select (not watch) so the 1 Hz live tick doesn't rebuild Home; in goldens (no AppState) it resolves to false, so existing goldens are byte-identical.

Tests

  • workout_reliability_test.dart: the hold is time-capped (a parked job runs past the cap with the workout still live, and later jobs run too), and ending a workout re-arms the cap for the next session. Existing gate tests unchanged and green.
  • ui2_wiring_r2_test.dart: a bare day during a live workout renders the workout card and neither "Nothing recorded for today" nor "Sync the band".

flutter analyze clean; full suite green (2971 passed, --concurrency=1) against the pinned protocol SHA.

Summary by CodeRabbit

  • New Features

    • Added a six-hour hold limit for live workouts, allowing data calculations to resume automatically if a workout remains active beyond the limit.
    • Added a home-screen status card explaining when values will be calculated after a workout ends.
    • Home-screen workout status now reflects the currently active session.
  • Bug Fixes

    • Cleared workout hold status correctly when workouts end or the app is closed.

…cannot blank Home

Starting a workout holds all derivation (DeriveScheduler.setWorkoutActive)
on the stated assumption that "a workout is minutes long and its own results
are derived at the end anyway". Forget to finish the session and that
assumption inverts: capture keeps landing records, every queued derive job
stays parked, today's day_result is never built, and Home spends the day on
"Nothing recorded for today — Sync the band" while the strap is connected
and syncing fine. The sync CTA is a false answer — the sync completes and
its derive jobs are parked too.

Two changes, one per half of the symptom:

- DeriveScheduler gains a workoutHoldCap (6 h, matching
  AppState._kMaxLiveWorkoutAgeMs, the ceiling past which a live session row
  from a previous run is already judged "almost certainly not something the
  user is still in"). Past the cap the session is treated as forgotten and
  held work drains even though it is still live; ending the workout re-arms
  the cap for the next session. Inside the cap the hold is unchanged.

- Home's bare-day card branches on the live workout: while a session holds
  derivation it says "A workout is still running" and points at the session
  bar pinned below, instead of claiming nothing was recorded and offering a
  sync that cannot help.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015TxisrChNFxtVMXFFNRGtV
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 280de20c-c5b9-4016-9f56-cc259d7f0d7b

📥 Commits

Reviewing files that changed from the base of the PR and between d4d4cde and 9b073d5.

⛔ Files ignored due to path filters (2)
  • test/ui2_wiring_r2_test.dart is excluded by !test/**
  • test/workout_reliability_test.dart is excluded by !test/**
📒 Files selected for processing (2)
  • lib/compute/derive_scheduler.dart
  • lib/ui2/screens/home_screen.dart

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

Changes

Workout hold handling

Layer / File(s) Summary
Workout hold lifecycle
lib/compute/derive_scheduler.dart
DeriveScheduler adds a six-hour default hold cap, expiry state, timer management, snapshot output, and disposal cleanup.
Cap-aware derivation gates
lib/compute/derive_scheduler.dart
Arming and queue draining use _workoutHeld, allowing derivation after the hold cap expires.
Home workout-hold rendering
lib/ui2/screens/home_screen.dart
HomeScreen detects live workouts, supports test overrides, and displays a workout-hold card on bare days.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 9b073

The change caps forgotten workout holds and replaces the misleading Home empty state with a live-workout message; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant WorkoutState
  participant DeriveScheduler
  participant HomeScreen
  participant StatusCard
  WorkoutState->>DeriveScheduler: setWorkoutActive(true)
  DeriveScheduler->>DeriveScheduler: start hold-cap timer
  DeriveScheduler->>DeriveScheduler: expire hold and re-arm derivation
  WorkoutState->>HomeScreen: expose active workout state
  HomeScreen->>StatusCard: render workoutHoldCard()
Loading

Suggested reviewers: abdulsaheel

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: limiting the live-workout derivation hold so Home does not remain blank after a forgotten session.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@abdulsaheel
abdulsaheel merged commit 5d50806 into OpenStrap:main Aug 26, 2026
2 checks passed
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.

2 participants