Android: mergeSleep richness exception — stage-less import yields to computed stages (parity with #241) - #247
Merged
Merged
Conversation
…computed stages (parity with #241) Re-review of the #241/#246 merge surfaced a missed twin. Android's WhoopRepository.mergeSleep is the direct twin of Swift WhoopStore.SleepMerge.merge (its own comment says "Mirrors WhoopStore.SleepMerge"), but it still had the plain imported-wins-per-day logic — #241 added the richness exception to the Swift side only, so the shared-logic twins diverged. Port the exception: a sparse import (no stage data on ANY of its sessions that day) no longer clobbers a computed day that HAS stage data; days where the import carries stages, or where neither side does, keep the imported-wins rule unchanged. Adds hasStages() (null/""/"[]"/blank = no stages), byte-parity with Swift's hasStages. Not covered by #240: that's a Health-Connect-specific write-gate + heal-purge; it never touches mergeSleep and doesn't protect non-HC sparse imports (Apple/CSV). This is the mergeSleep-level twin. Mirrors Swift SleepMergeTests' 5 richness cases in MergeSleepLocalDayTest; compileFullDebugKotlin + MergeSleepLocalDayTest pass (8/0).
…view) Re-review of the first commit found it incomplete: mergeSleep feeds only CSV export + HealthConnectWriter. The Sleep SCREEN builds its night list with its OWN inline imported-wins merge (SleepScreen.kt, two sites: the main loader + the post-nap-edit reload), which had no richness exception — so a stage-less import would still blank stages on the Android sleep screen, while iOS gets richness via repo.sleeps (WhoopStore.SleepMerge). That's the exact user-visible surface the fix is meant to protect. Extract mergeSleepRichness (the imported-wins + #241 richness rule, returned UNSORTED) and route both SleepScreen sites through it, keeping their effectiveStartTs sort (#395) and the #814 union inputs unchanged. mergeSleep is now that helper keyed by wake-day + sorted by startTs, so all three consumers (browse, CSV, screen) share ONE richness implementation. compileFullDebugKotlin + MergeSleepLocalDayTest pass (9/0, +1 mergeSleepRichness case).
DX23876
pushed a commit
to DX23876/noop
that referenced
this pull request
Jul 22, 2026
…computed stages (parity with ryanbr#241) (ryanbr#247) * Android: mergeSleep richness exception — stage-less import yields to computed stages (parity with ryanbr#241) Re-review of the ryanbr#241/ryanbr#246 merge surfaced a missed twin. Android's WhoopRepository.mergeSleep is the direct twin of Swift WhoopStore.SleepMerge.merge (its own comment says "Mirrors WhoopStore.SleepMerge"), but it still had the plain imported-wins-per-day logic — ryanbr#241 added the richness exception to the Swift side only, so the shared-logic twins diverged. Port the exception: a sparse import (no stage data on ANY of its sessions that day) no longer clobbers a computed day that HAS stage data; days where the import carries stages, or where neither side does, keep the imported-wins rule unchanged. Adds hasStages() (null/""/"[]"/blank = no stages), byte-parity with Swift's hasStages. Not covered by ryanbr#240: that's a Health-Connect-specific write-gate + heal-purge; it never touches mergeSleep and doesn't protect non-HC sparse imports (Apple/CSV). This is the mergeSleep-level twin. Mirrors Swift SleepMergeTests' 5 richness cases in MergeSleepLocalDayTest; compileFullDebugKotlin + MergeSleepLocalDayTest pass (8/0). * Android: route the Sleep screen through the richness merge too (re-review) Re-review of the first commit found it incomplete: mergeSleep feeds only CSV export + HealthConnectWriter. The Sleep SCREEN builds its night list with its OWN inline imported-wins merge (SleepScreen.kt, two sites: the main loader + the post-nap-edit reload), which had no richness exception — so a stage-less import would still blank stages on the Android sleep screen, while iOS gets richness via repo.sleeps (WhoopStore.SleepMerge). That's the exact user-visible surface the fix is meant to protect. Extract mergeSleepRichness (the imported-wins + ryanbr#241 richness rule, returned UNSORTED) and route both SleepScreen sites through it, keeping their effectiveStartTs sort (ryanbr#395) and the ryanbr#814 union inputs unchanged. mergeSleep is now that helper keyed by wake-day + sorted by startTs, so all three consumers (browse, CSV, screen) share ONE richness implementation. compileFullDebugKotlin + MergeSleepLocalDayTest pass (9/0, +1 mergeSleepRichness case).
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.
Re-review follow-up to the merged #241 + #246. Closes the last parity gap from that pair.
The gap
#241 added a richness exception to Swift
WhoopStore.SleepMerge.merge— a stage-less import must not blank a computed day that has stage data. But the direct Kotlin twin,WhoopRepository.mergeSleep(whose own comment says "Mirrors WhoopStore.SleepMerge (SleepMergeTests)"), still had the plain imported-wins logic:So after the merge, the two shared-logic twins diverged: on iOS a stage-less WHOOP/Apple re-import yields to computed stages; on Android it still clobbered them.
Why #240 doesn't cover it
#240 (still open) fixes the Android side a different way — a Health-Connect-specific
coveredDayswrite-gate pluspurgeHcShadowed*heal that deletes sparse HC rows. It never touchesmergeSleep, and it only guards Health Connect imports — a sparse Apple/CSV import would still clobber computed stages at the merge. This PR is themergeSleep-level twin that matches Swift for all import sources.The fix
Port the exception into
mergeSleep(group by end-day; if the import for a day has no stages and the computed day does, the computed sessions win — otherwise imported wins, unchanged). AddshasStages()with byte-identical semantics to Swift (null/""/"[]"/ blank = no stages).Tests
Mirror Swift
SleepMergeTests' 5 richness cases inMergeSleepLocalDayTest(stage-less→computed, import-with-stages-wins, neither-side,[]/blank stageless, keep-every-session-of-winning-day).compileFullDebugKotlin+MergeSleepLocalDayTestpass (8/0). Single-day imported-wins and #715/#304 behaviour unchanged.Re-review update (2nd commit): the first commit fixed
mergeSleep(CSV export + HealthConnectWriter) but the Sleep screen builds its night list with its own inline imported-wins merge (two sites) that had no richness — so the actual sleep display would still blank stages on Android while iOS doesn't. ExtractedmergeSleepRichnessand routed bothSleepScreensites through it (preserving theireffectiveStartTssort). All three sleep consumers now share one richness rule. Tests 9/0.