Sleep: promote the wake-spelling fold to SleepStageVocabulary.canonicalStage, and apply the #979 rule to SleepBench - #1630
Open
vishk23 wants to merge 1 commit into
Open
Conversation
…alStage, and apply the #979 rule to SleepBench #979 converted eleven segment comparisons in the app to SleepStageVocabulary.isWake. It did not touch Tools/SleepBench, which went on comparing bare "wake" literals in wakeMinutes, sleepMinutes, onsetAndFinalWake, firstRemLatencyMinutes, toSleepWake and bandVetoMirror, and keying stageMinutes / stagePercentages by the raw label. Nothing under Tools/ is built or tested by any default CI job, so that gap shipped with every check green. The consequence is the one #979 describes: a hypnogram whose segments spell wake "awake" — Oura's phase table, generic wearable JSON, or a stage-edited night's stagelock rows, the very rows section E scores against — had every wake epoch scored as SLEEP. That biases wake DOWN and sleep UP in exactly the quantities this harness exists to measure. Confusion.add was worse than misfiling: an "awake" reference matched no class in stageOrder and was DROPPED, so those epochs left the denominator instead of showing up as disagreement. Measured on one real 564-minute stage-locked night (119.5 reference wake minutes), stored under both spellings with identical streams: truth wake minutes 0 -> 120 V2 wake-minute error 0 -> -119.5 (sign flip: "calibrated" -> under-calls) E.1 wake stage bias 0.00 pp -> -21.19 pp (the calibration guard read as perfect) E.0 4-class kappa 0.145 -> 0.091 E.0 sleep/wake accuracy 100.0% -> 78.8% (was scoring against a no-wake reference) E.0 epochs scored 889 -> 1128 (239 wake epochs had been dropped) C stored kappa 0.000 -> 0.029 F human first-REM lat 138.0 -> 131.5 min E.1 is the pointed one: the section added to GUARD stage-fraction calibration after #348/#437 reported zero wake bias where the true bias is -21.2 pp, larger in magnitude than the +17 pp blowout that forced #437's revert. Comparisons go through SleepStageVocabulary.isWake. The sites that KEY by stage (stageMinutes, stagePercentages, Confusion) fold through the NEW SleepStageVocabulary.canonicalStage(_:) — trim, lowercase, either wake spelling onto the segment vocabulary's "wake"; unknown labels tidied, never invented into the vocabulary. Requested in the #988 review: the normaliser as shared code rather than a rule in a doc. It is in-memory only — comparison and bucketing — and its output must never be written back to a store, so neither live vocabulary moves and no persisted hypnogram changes meaning. Android already carries this shape: the UI's canonicalStage folds the same alias set (via the Kotlin twin's isWake) toward "awake" because its consumers key the minutes/colour vocabulary. One alias rule, one canonical spelling per vocabulary; no Android change needed. Also adds the documentation the harness never had: Tools/SleepBench/README.md (how to run it, what each section A-G answers, what each reference is worth, the vocabulary rule, known limitations) — including the other #988-review note, in the section table where the misreading would happen: section C is the IN-SESSION domain, so its kappa is not comparable to a whole-record kappa. Plus a vocabulary + harness pointer in docs/ANALYTICS.md and a CLAUDE.md bullet beside the model- resolution rule. All three flag that Tools/ has no CI. Verified: StrandAnalytics swift test 1601/1601 (3 new canonicalStage cases); sleepbench swift test 26/26 (13 new — the same night scored under both spellings must produce identical results, so they fail against a harness comparing literals). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
The normaliser from the #988 review, as its own PR — plus the harness it was needed for.
What this is
Two things ryanbr asked for in the #988 review, one thing #979 left undone:
SleepStageVocabulary.canonicalStage(_:)(StrandAnalytics): trims, lowercases, and folds either wake spelling onto the segment vocabulary's"wake"; unknown labels are tidied, never invented into the vocabulary. For in-memory comparison and bucketing ONLY — its output must never be written back to a store, so neither live vocabulary moves and no persisted hypnogram changes meaning. The alias rule keeps one definition (isWake); the fold target is a property of the vocabulary being keyed. No Android change needed: the UI'scanonicalStagealready folds the same alias set (via the Kotlin twin'sisWake) toward"awake", because its consumers key the minutes/colour vocabulary.Tools/SleepBenchstops comparing bare literals. burst_index and layout_marker are decoded and then destroyed — the only two decoder fields with no banked home (extends #845 to v26/v20-21) #979 fixed eleven segment comparisons in the app and did not touch the harness — nothing underTools/runs in default CI, so the gap shipped green. Comparisons now go throughisWake; the sites that key by stage (stageMinutes,stagePercentages,Confusion) fold throughcanonicalStage, applied once at DB load and once at epoch expansion.Why it matters, measured
One real 564-minute stage-locked night (119.5 reference wake minutes), stored under both spellings with identical streams:
"wake"night"awake"night (before this PR)Sources that spell wake
"awake"are not hypothetical: Oura's phase table, generic wearable JSON, and theedit_sleep_stagesvocabulary — the verystagelockrows section E uses as its human reference. E.1 is the pointed failure: the section added after #348/#437 to guard stage-fraction calibration reported zero wake bias where the truth is −21.2 pp, larger in magnitude than the +17 pp blowout that forced #437's revert.Verification
Packages/StrandAnalytics:swift test1601/1601 (3 newcanonicalStagecases, incl. a property test thatcanonicalStage(l) == "wake"⟺isWake(l)).Tools/SleepBench:swift test26/26 (13 new — the same night scored under both spellings must produce identical results, so they fail against a harness that compares literals). Run locally on macOS;Tools/has no default CI, which the README/docs now say out loud.Tools/doc_comment_lint.py: OK, no new detached doc comments.🤖 Generated with Claude Code