Repository navigation
Today (liquid): restore the 'workout in progress' card (#105) - #126
Merged
Merged
Conversation
The Liquid Design Home rewrite (LiquidTodayView, default-on since v8.4.0) dropped the active-workout indicator the classic TodayView shows: LiquidTodayView never referenced AppModel.activeWorkout, so after starting a manual workout the Home screen showed no in-progress card. Android's liquid TodayScreen kept it (WorkoutInProgressCard), so this was an iOS-only regression and an iOS/Android parity gap. Fix: render the SAME leaf the classic TodayView uses. Promote ActiveWorkoutIndicatorSection from private to internal and place it in LiquidTodayView right under the hero. It carries its own app/router environment objects (injected at StrandApp root) and is leaf-isolated, so the per-second clock tick re-renders only the card, not the whole dashboard; it renders nothing when no workout is active, auto-appearing/clearing off AppModel.activeWorkout. Sharing one implementation keeps the two Today screens from drifting again. Not a strap issue: activeWorkout is a user-started manual workout, so the reporter's 5.0/MG detail is irrelevant — this is purely a missing view in the liquid rewrite. iOS/macOS only; rides the next build (app target can't compile on WSL). Reuses two existing, shipping implementations (classic TodayView + Android).
ryanbr
added a commit
that referenced
this pull request
Jul 12, 2026
…read
Re-review found a Swift/Kotlin divergence I introduced: replacing Kotlin's
'else null' with 'else futureClockBanner' meant a strap that banked nothing
this pass but isn't yet sustained-empty AND is future-dated showed the
future banner on Android while Swift (bankedNothing branch wins) showed nil.
Restructure the Kotlin lastSyncError to a when{} that matches Swift's
else-if precedence exactly: bankedNothing keeps its sustained-empty logic
(#126/#214), future-dated is checked only on the banked-something path.
Also hoist the Swift wall-clock read to one value (parity with Kotlin's
single nowSec; the banner and its log can't drift across a second boundary).
ryanbr
added a commit
that referenced
this pull request
Jul 12, 2026
…332) * Surface a banner when the strap clock is set in the future (#324) A WHOOP 4.0 whose RTC relatched to a future base banks history dated years ahead. The #928/#1012/#773 guards correctly refuse to misfile it, but because the strap DOES bank records the existing 'clock lost / not banking' banner (gated on empty syncs) never fires - so the sync reported clean while importing nothing, with no reason shown (#324). Add a pure futureDatedStrapBanner (Swift BLEManager + Kotlin WhoopBleClient, byte-identical string) driven by the existing isFutureDatedNewest predicate, wired into the HISTORY_COMPLETE handler after the banked-nothing branch. One detection is decisive (nothing legitimate banks 48h ahead), so no streak gate. Names the strap-side remedy (full charge + power-cycle to relatch the clock). Twin tests pin the string + the 48h threshold on both platforms. * Fix #324 banner: cover the timeout outcome, not just HISTORY_COMPLETE Re-review caught the real gap: a future-dated strap with a deep backlog TIMES OUT before HISTORY_COMPLETE (the reporter's own session ended on timeout). The first cut gated the banner on HISTORY_COMPLETE only, so the reported case would still show the misleading 'strap went quiet' message and the new banner would never fire. Hoist the future-clock banner (computed once from strapNewestTs, which is already populated by the connect-time GET_DATA_RANGE) and apply it to BOTH the HISTORY_COMPLETE branch and the WHOOP-4 timeout branch, on both platforms. The log is now outcome-independent too. * Fix #324 banner parity: bankedNothing precedence + single wall-clock read Re-review found a Swift/Kotlin divergence I introduced: replacing Kotlin's 'else null' with 'else futureClockBanner' meant a strap that banked nothing this pass but isn't yet sustained-empty AND is future-dated showed the future banner on Android while Swift (bankedNothing branch wins) showed nil. Restructure the Kotlin lastSyncError to a when{} that matches Swift's else-if precedence exactly: bankedNothing keeps its sustained-empty logic (#126/#214), future-dated is checked only on the banked-something path. Also hoist the Swift wall-clock read to one value (parity with Kotlin's single nowSec; the banner and its log can't drift across a second boundary). * Add bad-clock strap diagnostics for #324 (dev logs 1 + 2) Two log-only additions to make a future-dated strap's cause legible in the strap-log export, extending the existing #547 dropped-record seam: 1. RTC-state events (RTC_LOST / BOOT / SET_RTC) that the #547 plausibility gate drops for their own implausible timestamp are now CAPTURED before being discarded (Streams.droppedRtcEvents). This is the catch-22 fix: the one event that explains the reset was silently dropped because ITS ts is bad. The Backfiller logs each with the bad epoch the RTC jumped to. 2. The epoch SPAN of the dropped records (oldest -> newest own-timestamp + hours-ahead skew) is captured and logged, so we can tell a whole-range- future strap (safe to fast-forward-discard, the #324 follow-up B) from one mixed with real data. Pure capture in WhoopProtocol.extractHistoricalStreams + a pure, testable BadClockDiagnostics formatter (byte-identical Swift/Kotlin), both twinned. No behavior change: the gate still drops exactly what it dropped before. WhoopProtocol 271 swift test + Android compile/tests green.
ryanbr
added a commit
that referenced
this pull request
Jul 19, 2026
…uble-count (#550) #550 reports resting HR ~5 bpm low and HRV too high on a WHOOP 4.0 (49 vs 30 on WHOOP + a Polar H10). The strap log shows the nightly HRV over-counts beats: coverage=2.12-2.75 with only ~59 EXACT duplicates, and meanNN=1133ms (≈53 bpm) matches the real sleeping HR — so it is ~2x as many R-R rows, not inflated interval magnitudes. Root cause is the live path stamping R-R with a now-anchored ts (untrusted realtime clock, #126) while the historical type-47 path uses the record's true RTC ts, so the same beat persists under two near-but-unequal (ts, rrMs) keys the exact-match dedup can't collapse. A blind read-time collapse is unsafe: R-R ts are stored at SECOND resolution, and at rest genuine consecutive beats are ~1 s apart, so any threshold wide enough to catch cross-second duplicates would also drop real beats. Before writing a behavior-changing fix we need to know whether the duplicates are same-second or cross-second. Add HrvAnalyzer.collapsedCoverage (both platforms) — coverage after collapsing SAME-SECOND near-duplicate beats (equal ts, |Δrr| <= 30 ms) to one representative — and surface it as `collapsedCov=` on the always-on `hrv diag` line. It is DIAGNOSTIC ONLY: it does not touch the shipped nightly HRV. If a reporter's log shows collapsedCov ~1, the over-count is same-second and a dedup fix will work; if it stays high, the duplication is cross-second and the fix must reconcile the two ingest paths instead. On clean data it equals rrCoverage (no-op). Tests: HrvRrCoverageTest gains no-op-on-clean, collapses-same-second, keeps-cross-second (the discriminator), and respects-rr-tolerance. Kotlin compiles and all 9 pass; Swift twin mirrors it (app/package build on macOS only).
ryanbr
added a commit
that referenced
this pull request
Jul 19, 2026
…uble-count (#550) (#595) #550 reports resting HR ~5 bpm low and HRV too high on a WHOOP 4.0 (49 vs 30 on WHOOP + a Polar H10). The strap log shows the nightly HRV over-counts beats: coverage=2.12-2.75 with only ~59 EXACT duplicates, and meanNN=1133ms (≈53 bpm) matches the real sleeping HR — so it is ~2x as many R-R rows, not inflated interval magnitudes. Root cause is the live path stamping R-R with a now-anchored ts (untrusted realtime clock, #126) while the historical type-47 path uses the record's true RTC ts, so the same beat persists under two near-but-unequal (ts, rrMs) keys the exact-match dedup can't collapse. A blind read-time collapse is unsafe: R-R ts are stored at SECOND resolution, and at rest genuine consecutive beats are ~1 s apart, so any threshold wide enough to catch cross-second duplicates would also drop real beats. Before writing a behavior-changing fix we need to know whether the duplicates are same-second or cross-second. Add HrvAnalyzer.collapsedCoverage (both platforms) — coverage after collapsing SAME-SECOND near-duplicate beats (equal ts, |Δrr| <= 30 ms) to one representative — and surface it as `collapsedCov=` on the always-on `hrv diag` line. It is DIAGNOSTIC ONLY: it does not touch the shipped nightly HRV. If a reporter's log shows collapsedCov ~1, the over-count is same-second and a dedup fix will work; if it stays high, the duplication is cross-second and the fix must reconcile the two ingest paths instead. On clean data it equals rrCoverage (no-op). Tests: HrvRrCoverageTest gains no-op-on-clean, collapses-same-second, keeps-cross-second (the discriminator), and respects-rr-tolerance. Kotlin compiles and all 9 pass; Swift twin mirrors it (app/package build on macOS only).
DX23876
pushed a commit
to DX23876/noop
that referenced
this pull request
Jul 22, 2026
…ard-105 Today (liquid): restore the 'workout in progress' card (ryanbr#105)
DX23876
pushed a commit
to DX23876/noop
that referenced
this pull request
Jul 22, 2026
…tics (ryanbr#332) * Surface a banner when the strap clock is set in the future (ryanbr#324) A WHOOP 4.0 whose RTC relatched to a future base banks history dated years ahead. The ryanbr#928/ryanbr#1012/ryanbr#773 guards correctly refuse to misfile it, but because the strap DOES bank records the existing 'clock lost / not banking' banner (gated on empty syncs) never fires - so the sync reported clean while importing nothing, with no reason shown (ryanbr#324). Add a pure futureDatedStrapBanner (Swift BLEManager + Kotlin WhoopBleClient, byte-identical string) driven by the existing isFutureDatedNewest predicate, wired into the HISTORY_COMPLETE handler after the banked-nothing branch. One detection is decisive (nothing legitimate banks 48h ahead), so no streak gate. Names the strap-side remedy (full charge + power-cycle to relatch the clock). Twin tests pin the string + the 48h threshold on both platforms. * Fix ryanbr#324 banner: cover the timeout outcome, not just HISTORY_COMPLETE Re-review caught the real gap: a future-dated strap with a deep backlog TIMES OUT before HISTORY_COMPLETE (the reporter's own session ended on timeout). The first cut gated the banner on HISTORY_COMPLETE only, so the reported case would still show the misleading 'strap went quiet' message and the new banner would never fire. Hoist the future-clock banner (computed once from strapNewestTs, which is already populated by the connect-time GET_DATA_RANGE) and apply it to BOTH the HISTORY_COMPLETE branch and the WHOOP-4 timeout branch, on both platforms. The log is now outcome-independent too. * Fix ryanbr#324 banner parity: bankedNothing precedence + single wall-clock read Re-review found a Swift/Kotlin divergence I introduced: replacing Kotlin's 'else null' with 'else futureClockBanner' meant a strap that banked nothing this pass but isn't yet sustained-empty AND is future-dated showed the future banner on Android while Swift (bankedNothing branch wins) showed nil. Restructure the Kotlin lastSyncError to a when{} that matches Swift's else-if precedence exactly: bankedNothing keeps its sustained-empty logic (ryanbr#126/ryanbr#214), future-dated is checked only on the banked-something path. Also hoist the Swift wall-clock read to one value (parity with Kotlin's single nowSec; the banner and its log can't drift across a second boundary). * Add bad-clock strap diagnostics for ryanbr#324 (dev logs 1 + 2) Two log-only additions to make a future-dated strap's cause legible in the strap-log export, extending the existing ryanbr#547 dropped-record seam: 1. RTC-state events (RTC_LOST / BOOT / SET_RTC) that the ryanbr#547 plausibility gate drops for their own implausible timestamp are now CAPTURED before being discarded (Streams.droppedRtcEvents). This is the catch-22 fix: the one event that explains the reset was silently dropped because ITS ts is bad. The Backfiller logs each with the bad epoch the RTC jumped to. 2. The epoch SPAN of the dropped records (oldest -> newest own-timestamp + hours-ahead skew) is captured and logged, so we can tell a whole-range- future strap (safe to fast-forward-discard, the ryanbr#324 follow-up B) from one mixed with real data. Pure capture in WhoopProtocol.extractHistoricalStreams + a pure, testable BadClockDiagnostics formatter (byte-identical Swift/Kotlin), both twinned. No behavior change: the gate still drops exactly what it dropped before. WhoopProtocol 271 swift test + Android compile/tests green.
DX23876
pushed a commit
to DX23876/noop
that referenced
this pull request
Jul 22, 2026
…the double-count (ryanbr#550) (ryanbr#595) ryanbr#550 reports resting HR ~5 bpm low and HRV too high on a WHOOP 4.0 (49 vs 30 on WHOOP + a Polar H10). The strap log shows the nightly HRV over-counts beats: coverage=2.12-2.75 with only ~59 EXACT duplicates, and meanNN=1133ms (≈53 bpm) matches the real sleeping HR — so it is ~2x as many R-R rows, not inflated interval magnitudes. Root cause is the live path stamping R-R with a now-anchored ts (untrusted realtime clock, ryanbr#126) while the historical type-47 path uses the record's true RTC ts, so the same beat persists under two near-but-unequal (ts, rrMs) keys the exact-match dedup can't collapse. A blind read-time collapse is unsafe: R-R ts are stored at SECOND resolution, and at rest genuine consecutive beats are ~1 s apart, so any threshold wide enough to catch cross-second duplicates would also drop real beats. Before writing a behavior-changing fix we need to know whether the duplicates are same-second or cross-second. Add HrvAnalyzer.collapsedCoverage (both platforms) — coverage after collapsing SAME-SECOND near-duplicate beats (equal ts, |Δrr| <= 30 ms) to one representative — and surface it as `collapsedCov=` on the always-on `hrv diag` line. It is DIAGNOSTIC ONLY: it does not touch the shipped nightly HRV. If a reporter's log shows collapsedCov ~1, the over-count is same-second and a dedup fix will work; if it stays high, the duplication is cross-second and the fix must reconcile the two ingest paths instead. On clean data it equals rrCoverage (no-op). Tests: HrvRrCoverageTest gains no-op-on-clean, collapses-same-second, keeps-cross-second (the discriminator), and respects-rr-tolerance. Kotlin compiles and all 9 pass; Swift twin mirrors it (app/package build on macOS only).
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…tics (ryanbr#332) * Surface a banner when the strap clock is set in the future (ryanbr#324) A WHOOP 4.0 whose RTC relatched to a future base banks history dated years ahead. The ryanbr#928/ryanbr#1012/ryanbr#773 guards correctly refuse to misfile it, but because the strap DOES bank records the existing 'clock lost / not banking' banner (gated on empty syncs) never fires - so the sync reported clean while importing nothing, with no reason shown (ryanbr#324). Add a pure futureDatedStrapBanner (Swift BLEManager + Kotlin WhoopBleClient, byte-identical string) driven by the existing isFutureDatedNewest predicate, wired into the HISTORY_COMPLETE handler after the banked-nothing branch. One detection is decisive (nothing legitimate banks 48h ahead), so no streak gate. Names the strap-side remedy (full charge + power-cycle to relatch the clock). Twin tests pin the string + the 48h threshold on both platforms. * Fix ryanbr#324 banner: cover the timeout outcome, not just HISTORY_COMPLETE Re-review caught the real gap: a future-dated strap with a deep backlog TIMES OUT before HISTORY_COMPLETE (the reporter's own session ended on timeout). The first cut gated the banner on HISTORY_COMPLETE only, so the reported case would still show the misleading 'strap went quiet' message and the new banner would never fire. Hoist the future-clock banner (computed once from strapNewestTs, which is already populated by the connect-time GET_DATA_RANGE) and apply it to BOTH the HISTORY_COMPLETE branch and the WHOOP-4 timeout branch, on both platforms. The log is now outcome-independent too. * Fix ryanbr#324 banner parity: bankedNothing precedence + single wall-clock read Re-review found a Swift/Kotlin divergence I introduced: replacing Kotlin's 'else null' with 'else futureClockBanner' meant a strap that banked nothing this pass but isn't yet sustained-empty AND is future-dated showed the future banner on Android while Swift (bankedNothing branch wins) showed nil. Restructure the Kotlin lastSyncError to a when{} that matches Swift's else-if precedence exactly: bankedNothing keeps its sustained-empty logic (ryanbr#126/ryanbr#214), future-dated is checked only on the banked-something path. Also hoist the Swift wall-clock read to one value (parity with Kotlin's single nowSec; the banner and its log can't drift across a second boundary). * Add bad-clock strap diagnostics for ryanbr#324 (dev logs 1 + 2) Two log-only additions to make a future-dated strap's cause legible in the strap-log export, extending the existing ryanbr#547 dropped-record seam: 1. RTC-state events (RTC_LOST / BOOT / SET_RTC) that the ryanbr#547 plausibility gate drops for their own implausible timestamp are now CAPTURED before being discarded (Streams.droppedRtcEvents). This is the catch-22 fix: the one event that explains the reset was silently dropped because ITS ts is bad. The Backfiller logs each with the bad epoch the RTC jumped to. 2. The epoch SPAN of the dropped records (oldest -> newest own-timestamp + hours-ahead skew) is captured and logged, so we can tell a whole-range- future strap (safe to fast-forward-discard, the ryanbr#324 follow-up B) from one mixed with real data. Pure capture in WhoopProtocol.extractHistoricalStreams + a pure, testable BadClockDiagnostics formatter (byte-identical Swift/Kotlin), both twinned. No behavior change: the gate still drops exactly what it dropped before. WhoopProtocol 271 swift test + Android compile/tests green.
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
…the double-count (ryanbr#550) (ryanbr#595) ryanbr#550 reports resting HR ~5 bpm low and HRV too high on a WHOOP 4.0 (49 vs 30 on WHOOP + a Polar H10). The strap log shows the nightly HRV over-counts beats: coverage=2.12-2.75 with only ~59 EXACT duplicates, and meanNN=1133ms (≈53 bpm) matches the real sleeping HR — so it is ~2x as many R-R rows, not inflated interval magnitudes. Root cause is the live path stamping R-R with a now-anchored ts (untrusted realtime clock, ryanbr#126) while the historical type-47 path uses the record's true RTC ts, so the same beat persists under two near-but-unequal (ts, rrMs) keys the exact-match dedup can't collapse. A blind read-time collapse is unsafe: R-R ts are stored at SECOND resolution, and at rest genuine consecutive beats are ~1 s apart, so any threshold wide enough to catch cross-second duplicates would also drop real beats. Before writing a behavior-changing fix we need to know whether the duplicates are same-second or cross-second. Add HrvAnalyzer.collapsedCoverage (both platforms) — coverage after collapsing SAME-SECOND near-duplicate beats (equal ts, |Δrr| <= 30 ms) to one representative — and surface it as `collapsedCov=` on the always-on `hrv diag` line. It is DIAGNOSTIC ONLY: it does not touch the shipped nightly HRV. If a reporter's log shows collapsedCov ~1, the over-count is same-second and a dedup fix will work; if it stays high, the duplication is cross-second and the fix must reconcile the two ingest paths instead. On clean data it equals rrCoverage (no-op). Tests: HrvRrCoverageTest gains no-op-on-clean, collapses-same-second, keeps-cross-second (the discriminator), and respects-rr-tolerance. Kotlin compiles and all 9 pass; Swift twin mirrors it (app/package build on macOS only).
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
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.
Fixes #105 — after the Liquid Design Home rewrite, starting a workout no longer showed an active-workout card on Home.
Root cause
LiquidTodayView(Liquid Home,liquidTodayEnableddefaults true → shipping since v8.4.0) never referencedAppModel.activeWorkout. The classicTodayViewrenders anActiveWorkoutIndicatorSectionoff it; the liquid rewrite ported the hero/tiles but dropped that indicator. Android's liquidTodayScreenkept it (WorkoutInProgressCard), so this was an iOS-only regression + iOS↔Android parity gap.Fix
Render the same leaf the classic
TodayViewuses: promoteActiveWorkoutIndicatorSectionfromprivateto internal and place it inLiquidTodayViewright under the hero. It:app/routerenvironment objects (injected atStrandApproot),AppModel.activeWorkout),router.openActiveWorkout().Sharing one implementation keeps the two iOS Today screens (and Android's card) from drifting again.
Notes
activeWorkoutis a user-started manual workout, so the reporter's 5.0/MG correction is irrelevant — purely a missing view in the liquid rewrite.TodayView+ Android), and I verified the one runtime risk:AppModel/NavRouterare injected at theStrandApproot, above both Today views, so the section's@EnvironmentObjects resolve.Closes #105.