Repository navigation
fix(recovery): seed HRV/RHR baseline from on-device nights the import left blank (#393) - #421
Closed
digitalerdude wants to merge 1 commit into
Closed
digitalerdude wants to merge 1 commit into
digitalerdude wants to merge 1 commit into
Conversation
… left blank (ryanbr#393) A strap user whose daily rows come from a Health Connect SLEEP import (HC never carries WHOOP HRV, so every such day is present-but-null) had the on-device HRV/RHR the strap computes from raw HR blocked from seeding the recovery baseline: the union merge filled only KEY-ABSENT days, so a present-but-null day was treated as "covered" and never filled. The HRV baseline starved below the 4-night seed (nValid stuck at 2), so recovery never calibrated and Charge stayed blank despite worn, sleep-recorded nights. Fill a day whose stored value is null (absent OR present-but-null) with a real on-device value, never overriding a real imported value ("cloud wins" preserved). Extracted into a shared NightlyBaselineMerge helper routed by both engines so prod and tests share one rule; this also removes a latent Swift/Kotlin divergence on the (absent, nil) case. Byte-identical across platforms; HRV/RHR/resp aligned. Tests (both platforms) prove the filled baseline crosses the seed and TRACKS a varying injected HRV level (35 vs 70 ms), not one coincidental match. Verified: swift test (StrandAnalytics), gradle testFullDebugUnitTest, and a macOS Strand app build (app-target IntelligenceEngine.swift, which no default CI covers). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
|
Closing as superseded: #401 (merged just before this opened) fixes the same root cause the same way — the nightly-into-history merge only checked KEY presence, so an imported row with a null value shadowed the real strap-computed night, starving the baseline below the 4-night seed ("Needs the strap" despite scored sleep). #401 already covers this on both platforms with shadow tests including the 7/7 starvation shape, so this PR adds nothing. Thanks @TheBoroer. |
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 #393.
The bug
A strap user whose daily rows come from a Health Connect sleep import (HC never carries WHOOP HRV, so every one of those days is
avgHrv = null) had the on-device HRV/RHR the strap computes from raw HR blocked from seeding the recovery baseline. The union merge inIntelligenceEnginefilled only days whose key was absent; a day that already existed as a persisted row with a null value was treated as "covered" and never filled. So the HRV baseline starved below the 4-night seed (nValidstuck at 2), recovery never calibrated, and Charge stayed blank even on worn, sleep-recorded nights.Confirmed from the reporters
report.txt:~6 nights carry a perfectly good computed HRV, yet the baseline counted only 2.
The fix
Fill a day whose stored value is null (absent OR present-but-null) with a real on-device value, and never override a real imported value — so the "imported cloud value wins" intent holds while a strap+HealthConnect users computed HRV/RHR can finally seed the baseline. Extracted into a shared
NightlyBaselineMerge.fillBlankNightsrouted by both engines, so production and tests share one rule; this also removes a latent Swift/Kotlin divergence on the(absent, nil)case.Strand/Data/IntelligenceEngine.swift+android/.../analytics/IntelligenceEngine.kt, newNightlyBaselineMerge.{swift,kt}. HRV/RHR/resp all routed through it.Verification
swift test --filter NightlyBaselineMergeTests(StrandAnalytics) — pass./gradlew testFullDebugUnitTest(NightlyBaselineMergeTest,BaselineSeedingTest) — pass; Android app compilesxcodebuild -scheme Strand … build— BUILD SUCCEEDED (app-targetIntelligenceEngine.swift, which no default CI covers)Note for the reporters specific strap: their most recent nights have no raw samples yet (freshly re-added 4.0, history not fully synced), so "last night" still needs a completed history sync to score; this fix restores Charge for the older nights that already carry HRV.