Sleep: recover strap-disputed false wakes via the band sleep_state veto (Swift + Kotlin)#738
Sleep: recover strap-disputed false wakes via the band sleep_state veto (Swift + Kotlin)#738vishk23 wants to merge 3 commits into
Conversation
NOOP's EEG-free cardiorespiratory stager over-calls WAKE: it reads still, low-HR but not-quite-asleep epochs as wake far more often than the wearer was actually awake. WHOOP's own per-second sleep-state band (#175) is an independent scored signal, and on real banded nights it scores "asleep" across ~two-thirds of the epochs NOOP calls wake, with the reverse disagreement (NOOP asleep, band wake) an order of magnitude smaller. Add applyBandStateWakeVeto: after staging, reclassify an INTERIOR wake epoch to "light" only when the aligned band state == asleep (2). The leading onset-latency and trailing final-wake blocks are never touched, and only wake is ever turned into sleep (efficiency can only rise). No-op when the band is absent (WHOOP 4.0 / unbanded window) or the flag is off; stager-agnostic (corrects whichever hypnogram V1/V2 produced). Default-on via bandStateWakeVetoEnabled. Validated on 12 real banded nights: recovers ~30 min/night of spurious wake at ~2 min/night reverse error.
Byte-equivalent Kotlin twin of the Swift applyBandStateWakeVeto: same guards, same default-on bandStateWakeVetoEnabled flag, same "light" recover stage, same interior-only (leading/trailing wake never touched) reclassification, wired in at the same point in detectSleepUncached (rawStages -> veto -> stages). Adds SleepStagerBandVetoTest mirroring the Swift band-veto tests. Preserves the cross-platform analytics parity contract (Swift <-> Kotlin).
ryanbr
left a comment
ryanbr
left a comment
There was a problem hiding this comment.
Thanks for this — the design is careful and well-motivated. Leaning on the strap's own independent band verdict (rather than re-deriving a signal), mapping recovered epochs to light so deep/REM — and therefore the recovery gate — stay untouched, and keeping it conservative (interior-only, asleep(2)-only, wake→sleep only) are all the right calls. I verified the Swift↔Kotlin logic is byte-equivalent and StrandAnalytics CI is green. A couple of things before merge:
Blocking: the @73 label names the wrong byte (should be @81)
The band sleep_state is decoded from @81's high nibble (sb>>4)&3 — see Streams.swift:192 ("v18 @81 flag byte (sb>>4)&3 as 0 wake / 1 still / 2 asleep / 3 up") and Streams.swift:227 ("carried verbatim off @81's high nibble"). @73 is the skin-temperature byte (Streams.swift:65, Interpreter.swift:437 — "skin_temp@73").
The PR is internally inconsistent: the header comment correctly says "the persisted v18 @81 high-nibble", but the mnemonic @73 then appears ~19× — the section headers (H9 @73 band-state WAKE-veto), the bandStateAsleep (@73 == 2) doc line, and several test messages — on both platforms.
It's not a runtime bug (the code consumes the already-decoded bandSleepState stream and never touches a raw offset, so behavior is correct and the two engines agree), but it will send the next reader chasing @73 straight to skin temp. Please rename @73 → @81 throughout, or just drop the offset from the name — the existing H8 code simply says "band sleep_state" with no offset, which is the cleaner precedent.
Please add: a Kotlin end-to-end test to match Swift
Swift has testBandStateWakeVetoRaisesEfficiencyEndToEnd (wiring through detectSleep); the Kotlin side only ships the 5 pure-function tests, so the Android wiring (rawStages → veto → efficiency in the scoring loop) is unverified. And since android.yml is disabled, nothing in CI covers the Kotlin side at all — please confirm ./gradlew testFullDebugUnitTest locally and mirror that one end-to-end test so the twin has parity in coverage, not just logic.
Non-blocking / heads-up
- Recovered epochs count as
light, so total sleep time and light minutes rise on banded 5.0/MG nights (intended). Deep/REM are correctly left alone. Just flagging that default-on will shift historical efficiency/light numbers for banded nights — fine by me, but worth a line in the PR body so it's a conscious product call, not a silent one. - I did an adversarial pass on the expand→re-collapse round-trip and it's safe: stager boundaries are on the 30 s grid, and the only edge-case (gap/edge fill) can only ever produce
wake, which the leading/trailing exclusion preserves. Downstream consumers all read the corrected hypnogram (no raw/corrected split-brain), and onset-latency + final-wake are preserved. No correctness issues there.
Otherwise this is good to go once the @81 naming is fixed. Thanks again for keeping the Swift/Kotlin logic in lockstep.
Review follow-ups on the H9 band-state WAKE-veto. The comments and test messages called the sleep_state source "@73". That is the skin-temperature byte; the band sleep_state this veto reads lives at @81. Since nothing in the code keyed off the mnemonic, drop the byte number from the prose entirely and say "band-state" / "band sleep_state" — the offset belongs with the decoder, not with the stager that consumes an already-decoded stream. Comment-only on both platforms; no behaviour change. Kotlin had only the five pure-function tests of applyBandStateWakeVeto, so the Android WIRING (rawStages -> veto -> efficiency inside detectSleep) was unverified — and with android.yml disabled, no CI covers it either. Mirror the Swift testBandStateWakeVetoRaisesEfficiencyEndToEnd as raisesEfficiencyEndToEnd: a 6 h still 02:00 night with a 5-min motion+HR burst at +3 h that the stager scores as interior wake, run through detectSleep with and without an all-asleep band. On this fixture the unbanded run reports 300 s of interior wake at 0.986 efficiency and the banded run 0 s at 1.0, so the assertions (wake cannot rise, efficiency cannot fall, no interior wake survives an all-asleep band) are exercised, not vacuous. Swift: 1116 tests, 0 failures. Kotlin: testFullDebugUnitTest --tests '*SleepStager*' 83 tests, 0 failures.
vishk23
changed the title
2 participants
NOOP's EEG-free cardiorespiratory stager over-calls WAKE — it reads still, low-HR but not-quite-asleep epochs as wake more often than the wearer actually was. WHOOP's own per-second sleep-state band (#175, the persisted v18 high-nibble
(sb>>4)&3: 0 wake / 1 still / 2 asleep / 3 up) is an independent scored signal. On real banded nights the strap scores "asleep" across ~2/3 of the epochs NOOP calls wake, while the reverse disagreement (NOOP asleep, band wake) is an order of magnitude smaller.Change: after staging,
applyBandStateWakeVetoreclassifies an interior wake epoch tolightonly when the aligned band state == asleep (2). Leading onset-latency and trailing final-wake blocks are never touched (onset can't move earlier, final wake can't move later); it only ever turns wake→sleep, so efficiency can only rise. No-op when the band is absent (WHOOP 4.0 / unbanded window) or the flag is off; stager-agnostic (corrects whichever hypnogram V1/V2 produced). Default-on viabandStateWakeVetoEnabled, single flip-point to disable.Parity: shipped in both engines —
StrandAnalytics/SleepStager.swiftandandroid/.../analytics/SleepStager.kt— with byte-equivalent logic, preserving the Swift↔Kotlin analytics contract. Tests on both sides.Validation: on 12 real banded nights, recovers ~30 min/night of spurious wake at ~2 min/night reverse error. Swift suite green (1116 tests); Kotlin SleepStager suite green (82 tests).