fix a pile of stuff from the issue backlog (#129, #130, #123, #147, #80)#150
Merged
Conversation
went through the open issues and fixed what i could, here's the rundown: - coach kept mixing up "today's workout" (#129) because v_sessions only had raw epoch start_ts/end_ts and the model had to guess how to turn a local date back into a utc range itself. just gave the view a real local `date` column (same strftime(...,'localtime') trick dataHistoryDays() already uses) and told the coach to filter on that instead of doing the math. - apple health was never getting today's workout (#130) even after forcing a sync, because export only ever ran as a side effect of the day_result derive pass, which needs a ble connection + a scheduled derive to happen. if you finish a workout with the band disconnected, nothing runs. added a proper exportWorkout() that writes just that one workout immediately when it finishes, no waiting around for the derive pipeline. - the "time to move" nudge only ever fired the instant you opened the app (#123) because it was foreground-only, immediate-emit, no wall clock timer behind it at all. per the issue's own suggested fix: now it schedules a real one-shot OS notification for now+2h whenever movement is seen, and reschedules on every new movement. so it actually fires while the app is closed like it's supposed to. - if you got killed mid-workout the only thing left to do was delete it, no way to actually finish it (filed as #147, fixed same breath). turns out activeWorkout was 100% in-memory and nothing ever restored it from the db on restart. now on boot it checks for a stranded 'live' session row and either resumes it (if recent) or quietly finalizes it (if it's been hours, don't want to show a "still live" workout from 3 days ago). - ios sideload was failing for someone with "invalid value 'openstrap_edge' for appIdName" (#80) - turned out CFBundleName had an underscore in it and that's apparently a thing some of these free-signing tools choke on when registering an app id with apple. renamed it to "OpenStrap Edge", only place that string was used anywhere in the project so should be safe. - answered #82 (rrIntervalsMs is beat-to-beat RR in ms, and yeah there's already an irregular rhythm screen built on poincare sd1/sd2 + pnn70, just doesn't specifically flag single dropped beats the way mobitz would need) - small design system bug: SectionHeader showed a trailing label with the same tappable-looking accent color + chevron whether or not it actually had an onTap handler, so "select days" (just a status text) looked exactly as clickable as "select all" (an actual button) right next to it. now it only gets the coral/chevron treatment when there's really something to tap. - also found 3 pre-existing test failures on main that had nothing to do with any of this (checked by stashing my changes and running against clean main first) - the crosshair readout on the merged timeline chart is deliberately always mounted now (just hidden via opacity, not offstage) so its layout slot doesn't jump around on scrub start/stop. that's correct behavior, but it means there are now genuinely two "Heart rate" text widgets in the tree at once (the chip + the readout row), which made the old find.text('Heart rate') asserts ambiguous. keyed the chip row and scoped the finders to it instead of touching the (correct) ui behavior. filed 3 new issues along the way that weren't tracked yet: #147 (workout stop bug above), #148 (stress score shows a confident number on ~20min of data while stress index correctly shows "-" — same family as the readiness honesty fixes, just a different scalar that didn't get the memo), #149 (workout auto-detect is a little trigger-happy and there's no way to turn it off). also closed #139 and #26 after actually re-checking the code matched what the old maintainer comments claimed, not just taking their word for it. 611/611 tests green, flutter analyze clean (same 8 pre-existing baseline issues as before, nothing new).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds local-day session filtering guidance, centralized immediate health export, startup recovery for live workouts, scheduled stillness reminders, posture-focused inactivity detection, clearer trailing UI affordances, and scoped timeline vital-chip tests. ChangesWorkout lifecycle and health export
Local session date querying
Notification scheduling and inactivity detection
Presentation and test targeting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)Workout export flowsequenceDiagram
participant WorkoutSuggestionScreen
participant AppState
participant HealthExporter
participant HealthConnect
WorkoutSuggestionScreen->>AppState: exportWorkoutToHealth(session)
AppState->>HealthExporter: exportWorkout(session)
HealthExporter->>HealthConnect: delete and write workout samples
Stillness reminder flowsequenceDiagram
participant AppState
participant NotificationService
participant NotificationPlugin
AppState->>NotificationService: cancel and schedule stillness reminder
NotificationService->>NotificationPlugin: zonedSchedule absolute notification
AppState->>NotificationService: reschedule after movement
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/health/health_export.dart`:
- Around line 554-559: Update _writeOneWorkout and the _exportDay export loops
so a live or malformed workout is represented as a skipped/no-op result distinct
from an attempted write failure. Make _exportDay set success to false only for
genuine write failures or thrown errors, while preserving skipped rows without
advancing retry or give-up state; apply the same distinction to the related
export paths noted in the diff.
In `@lib/state/app_state.dart`:
- Line 1004: Update the today string construction near the date-handling logic
to zero-pad month and day to two digits, preserving the YYYY-MM-DD format used
by _markMomentFromGesture and todayLabel().
- Around line 3104-3160: Initialize _workoutRawBase when rehydrating the session
in _reconcileOrphanedLiveWorkout, using the same reset/start pedometer setup
established by startWorkout() or _resetLivePedometer(), so workoutSteps and
final stopWorkout() persistence count steps from zero onward. Also guard the
reconciliation path with an early return when activeWorkout is already set,
preventing it from replacing a concurrently started workout and leaking its
timer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 20881b3b-5a75-4e69-815c-fd23927da141
📒 Files selected for processing (11)
ios/Runner/Info.plistlib/coach/coach_engine.dartlib/coach/coach_prompt.dartlib/data/db.dartlib/health/health_export.dartlib/notify/notification_service.dartlib/state/app_state.dartlib/ui/kit/kit.dartlib/ui/timeline/timeline_screen.dartlib/ui/workouts/workouts_screen.darttest/history_screens_redesign_test.dart
went through all 3 things it flagged, all were real, fixed all of them:
- health_export.dart: _writeOneWorkout was returning false for both "this
row is still live/malformed, skip it" AND "we actually tried to write
and it failed" - same value, two totally different meanings. that
matters because _exportDay feeds a false into the attempts/backoff/
give-up retry machinery, which is explicitly reserved for genuine
thrown errors per the comment above _kRetryCursor. so an in-progress
workout could trip that on every single drain/derive pass and
eventually get the WHOLE DAY given up on - not just the workout, real
stuff like RHR/HRV/steps/sleep export for that day too. switched it to
bool? (null = skip, not a failure) and fixed both call sites.
- app_state.dart: the posture-check notification's dedupeKey date wasn't
zero-padded ('2026-7-5' instead of '2026-07-05'), unlike literally
every other date string in this file. just swapped it for the
todayLabel() helper that's already used elsewhere here.
- app_state.dart: the orphaned-workout resume path never set
_workoutRawBase, so a resumed workout's step count stays stuck at 0
for its whole remaining duration, even once real pedometer data starts
flowing again after the band reconnects, and stopWorkout() ends up
persisting 0 steps at the end too. added the same snapshot
startWorkout() already does. also added a guard at the top so this
can't clobber a workout that got started for real while the reconcile
was still awaiting its db query (unawaited from _init(), so technically
possible even if unlikely).
611/611 still green, analyze still clean.
This was referenced Jul 25, 2026
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.
went digging through the open issues and fixed what i could get through. rundown below, commit message has the same thing if you want it inline with the diff.
fixes
#129 — coach mis-dates workouts
v_sessionsonly had raw epochstart_ts/end_ts, so the coach had to guess how to turn "today" (a local date) into a utc epoch range itself — that's exactly how it got today's workout wrong. added a real localdatecolumn to the view (samestrftime(...,'localtime')trickdataHistoryDays()already uses) and told the coach to filter on that column instead of doing the conversion itself.#130 — apple health never gets today's workout, even after forcing a sync
export only ever ran as a side effect of the day_result derive pass, which needs a BLE connection + a scheduled derive to actually happen. finish a workout with the band disconnected (or just before the next derive tick) and it silently never exports. added
HealthExporter.exportWorkout()— writes just that one workout immediately when it finishes, independent of the derive pipeline.#123 — "time to move" nudge only fires when you open the app
foreground-only, immediate-emit, no wall-clock timer behind it at all — so it could only ever "fire" the instant you happened to open the app. implemented the issue's own suggested fix: schedule a real one-shot OS notification for
now + 2hwhenever movement is detected, cancel + reschedule on the next movement. now it actually fires while the app is closed.#147 (new, fixed same pass) — stuck workout, delete is the only option
if the app got killed mid-workout,
activeWorkout(100% in-memory) never got restored on relaunch, so the only reachable action on it was delete. now on boot the app checks for a strandedstatus='live'session row: resumes it if recent, or quietly finalizes it if it's been hours (don't want a 3-day-old workout showing as "still live").#80 — ios sideload fails with "invalid value 'openstrap_edge' for appIdName"
CFBundleNamehad an underscore in it, which some free-signing tools choke on when registering an App ID with apple's API. renamed to "OpenStrap Edge" — only place that literal string was used in the project.design-system bug (found in passing)
SectionHeader's trailing label got the tappable accent-color + chevron treatment whether or not it actually had anonTap— so a plain status label ("Select days") looked exactly as clickable as a real button ("Select all") sitting right next to it. now the tappable styling only shows up when there's really something to tap.3 pre-existing test failures, unrelated to any of the above
found these by stashing my changes and running against clean
mainfirst — not something i introduced. root cause: the merged timeline chart's crosshair readout is deliberately always mounted (Visibility(maintainState: true), just hidden via opacity notOffstage) so its layout slot doesn't jump around when you start/stop scrubbing — correct behavior, but it means there are genuinely two "Heart rate" text widgets in the tree at once now (the chip + the readout row), so the oldfind.text('Heart rate')asserts were ambiguous. keyed the chip row and scoped the test finders to it instead of touching the (correct) UI.answered, no code needed
#82 — confirmed
rrIntervalsMsis beat-to-beat RR in ms, pointed at the existing irregular-rhythm screen (poincare sd1/sd2 + pnn70). doesn't specifically flag single dropped beats the way mobitz detection would need, called that out.filed along the way
closed
#139, #26 — re-verified the actual code matches what the old maintainer comments claimed before closing, didn't just take their word for it.
testing
611/611 tests green,
flutter analyzeclean (same 8 pre-existing baseline issues as before this branch, nothing new introduced).Summary by CodeRabbit