fix(import): select Oura's main sleep session for the daily rollup - #375
Conversation
The per-day fold in OuraApiParser.parseSleep used first-write-wins: whichever session the API listed first for a `day` claimed every daily field, so a short nap/fragment could beat the real night. Audited across a real 24-month dataset: 36 days where totalSleepMin was 0.5-25 min while the true main sleep (250-780 min) existed adjacent, including two impossible daily restingHr values (92 bpm from a 32-min wake-dominated micro-session where the real night was 277 min at 57 bpm; 90 bpm amid 41-48 bpm neighbors) plus five near-misses at 89-100 bpm. A sudden 90+ bpm daily resting-HR spike is the visible symptom of this bug. Select the day's MAIN session instead: a `long_sleep` session always outranks any other type; among same-rank candidates, the longer total_sleep_duration wins. The winner supplies every rollup field as a unit, so fields are never mixed across two sessions on the same day. dailyMetric is upserted keyed by (deviceId, day) (WhoopStore.upsertDailyMetrics), so re-running Oura sync heals every already-imported day in place - no migration needed, the same way the earlier RHR-score corruption healed.
The CSV-side OuraExportParser already skips type == "deleted" rows (a night the user removed in the Oura app, #862); the API parser had no type check at all, so a deleted period still became a session and -- with its full bedtime span -- could even win the day's rollup over the real night. Skip it before it becomes a period, same rule both parsers.
|
Pushed a second commit folding in a directly-related session-type gap found in the same audit: the API parser had no |
|
The API-lane fix here is correct and well-tested — but the same first-write-wins daily-rollup bug exists in the Oura file-export lane, which runs on both platforms, and this PR doesn't touch it. So to answer "is Android applicable?": yes — not for The same bug, in the file-export fold, on both platformsThe per-day sleep rollup in the export importer is per-field first-write-wins:
row.totalSleepMin = row.totalSleepMin ?? session.totalSleepMin
row.deepMin = row.deepMin ?? session.deepMin
...
row.efficiencyPct = row.efficiencyPct ?? session.efficiencyPct
row.avgHrvMs = row.avgHrvMs ?? session.avgHrvMs
if row.restingHr == nil { row.restingHr = session.lowestHr }
d.totalSleepMin = d.totalSleepMin ?: session.totalSleepMin
d.efficiencyPct = d.efficiencyPct ?: session.efficiencyPct
if (d.restingHr == null) d.restingHr = session.lowestHrSame root cause as the API lane — a nap/fragment listed first for a day claims the rollup — and the export carries the same Suggestion
This PR's API-lane fix stands on its own and is mergeable as-is; the file-export twin can be a follow-up (and would pair naturally with #376, which already touches the same export fold for the efficiency scale). Flagging so the file lane doesn't get forgotten — it's the same bug a phone-export user hits. |
…ort lane (Swift + Kotlin) Closes the file-lane exposure flagged in review: the account-export importer folded a day's sleep sessions per-FIELD first-write-wins (OuraExportParser.swift JSON + CSV paths, and the Kotlin WearableExportImporter.parseOura/parseOuraCsv twins), so a nap/fragment listed before the real night could mix its totalSleepMin with the main session's restingHr. Applies the same long_sleep-outranks-any-other-type, then longer-total_sleep_duration rule as OuraApiParser's dayWinner, on both the JSON and the real per-category CSV sleep rows, on both platforms — the winner now supplies every rollup field as a unit, never mixed across sessions. StrandImport: 207/207 (3 new OuraExportParserTests + 1 new WearableExportImporterTests fixture). Kotlin: transcribed carefully (no Android toolchain available to run testFullDebugUnitTest here); 4 new tests added to WearableExportImporterTest mirroring the Swift fixtures.
|
Pushed de900bd extending the main-session day-rollup rule to the file-export lane, closing the exposure flagged in review. What changed, both platforms:
Tests:
Caveat — please run on your side: I have no Android toolchain in this environment (no JDK; |
One conflict, in `OuraApiParserSleepTests.swift` — a purely additive collision with upstream's efficiency-normalization test. Both test blocks kept. Note on the second commit here (`skip Oura 'deleted' sleep periods in the API parser`): upstream landed the equivalent skip in #862 while this sat open, and git resolved the two identical additions to a single line rather than a conflict. The merged `OuraApiParser` has exactly one `deleted` guard, upstream's 0-100 → 0-1 `efficiency` normalization, and this branch's `dayWinner` selection, all intact. Verified: StrandImport 217/217 (1 skipped).
|
Rebuilt on current Worth noting how the two The review's ask — extending the main-session rule to the file-export lane on both platforms — is in the branch already ( Verified: |
|
Kindly check #877 |
|
@pipiche38 — belatedly closing the loop on your 07-27 ping here: I did pick it up, but I replied on #877 itself rather than on this thread, so it looks unanswered from here. Sorry about that. The substance is at #877 (comment) — a sequencing note rather than an objection: #877 is what first routes an Oura night's banked R-R into On this PR: rebased onto current |
|
I'm sorry, @vishk23 i do not understand your comment or review |
|
Reviewed. The fix is right and more complete than the description claims — which is the main thing to flag. Verified
The body understates the PR, and that mattersIt says:
That is worth correcting rather than shrugging at, because the export path was the worse of the two. Its old fold was per-FIELD: row.totalSleepMin = row.totalSleepMin ?? session.totalSleepMin
row.restingHr = row.restingHr ?? session.lowestHr // different session!so it could take sleep minutes from a fragment and resting HR from the real night — mixing two sessions into one day, which is strictly worse than the API path's first-write-wins. Anyone reading the body would think that is still outstanding. One behaviour change worth namingThe winner now overwrites every field unconditionally, including with That is the correct trade — mixing is the bug, and coherence beats completeness here — but it is a change beyond "stop showing impossible values", and it should be in the description rather than discovered. Before mergingCI on this is 17 days old (job IDs from July 13; Otherwise this is ready. Good find, and the session-level confirmation across a 24-month dataset is what makes it convincing rather than plausible. |
Notes covering everything merged since v9.2.1 (43 PRs), the in-app What's
New entry generated for both platforms, and the version bumps.
- docs/releases/v9.3.0.md — release notes plus the whatsnew front-matter,
with title translations for all five locales so the i18n gate has no
English fallback to warn about.
- AppChangelog.{kt,swift} + six strings.xml — generated by
Tools/appchangelog-gen.py, not hand-written, so the two platforms carry
byte-identical items.
- CHANGELOG.md — a 9.3.0 section in the existing house format.
- MARKETING_VERSION 9.2.2 -> 9.3.0, versionName 9.2.2 -> 9.3.0,
versionCode 305 -> 306.
The notes lead with a 'Scores that change' section because three merges
move numbers users have already seen — gap-weighted strain (#963), the
workout resting-HR fix (#983), and the sleep transition rule (#348) — and
the Oura fix (#375) corrects the importer, not rows already written, so it
needs a re-import to take effect.
3 participants
What
OuraApiParser.parseSleep's per-day fold used first-write-wins: whicheversleepsession the API happened to list first for a givendayclaimedevery daily field (
totalSleepMin,restingHr, deep/light/rem/awake,efficiency, HRV, respiratory rate). Oura can return more than one session
per day (a nap or short fragment alongside the real night), and the API
gives no ordering guarantee, so a fragment could — and did — beat the main
sleep.
Audit evidence
Checked across a real 24-month Oura dataset:
totalSleepMinwas 0.5-25 min while thetrue main sleep (250-780 min) existed adjacent in the same session list —
confirmed at the session level in every case.
restingHrvalues produced this way: 92 bpm onone day from a 32-min wake-dominated micro-session (the real night was
277 min at 57 bpm), and 90 bpm on another amid neighboring days of
41-48 bpm.
The visible symptom for a user is a resting-heart-rate reading that spikes
to 90+ bpm for a single day, with normal-looking days on either side — if
you've seen that on an Oura-imported day, this is why.
Fix
Select the day's MAIN session instead of first-write-wins. Oura marks the
primary sleep with
type == "long_sleep":long_sleepsession always outranks any other type;total_sleep_durationwins;mixed across two different sessions on the same day.
The per-session
periodslist (what backs the Sleep tab) is untouched —every detected session, including naps, still shows up there. Only the
single-row daily rollup now picks one session's fields consistently.
The day-key logic (
dayfield, fallback to the wake day) is unchanged.sleepScorecontinues to come fromdaily_sleep, not from this fold.Healing existing data
dailyMetricis upserted keyed by(deviceId, day)(
WhoopStore.upsertDailyMetrics,ON CONFLICT(deviceId, day) DO UPDATE),so this requires no migration — re-running the Oura sync overwrites every
affected day's row in place with the corrected values. This is the same
mechanism that healed the earlier RHR-score corruption.
Tests
Packages/StrandImport/Tests/StrandImportTests/OuraApiParserSleepTests.swift,4 new cases:
long_sleep→ the rolluptakes the
long_sleep'stotalSleepMin/restingHr, not the fragment's;long_sleeplisted first → identical result(order-independent);
long_sleeppresent) → keeps the fragment's ownvalues, honestly, rather than dropping or zeroing the day;
long_sleepsessions on one day → the longertotal_sleep_durationwins.
Verification:
All pre-existing
OuraApiParserSleepTestscases (single-session fold,invalid-span skip, huge-value guards) stay green — no regressions.
Honest verification
Packages/WhoopStore/Sources/WhoopStore/MetricsCache.swift(
upsertDailyMetrics/upsertSleepSessions, both trueON CONFLICT DO UPDATEupserts keyed by natural key, not append-only) andthe write path from
WearableDailyRowthroughStrand/Oura/OuraSyncWriter.swiftintoDailyMetric. I did not run a liveOura re-sync against a real account as part of this change (this package
is pure/network-free and covered by
swift test); the upsert mechanicsare the same ones already exercised by the existing RHR-score heal.
(
Packages/StrandImport/Sources/StrandImport/OuraApiParser.swift). Theseparate file-export parser (
OuraExportParser.swift, used for Oura'sdownloadable account export) has its own, independent per-day fold and
was not touched — left for a follow-up if the same issue is confirmed
there.