diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStageVocabulary.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStageVocabulary.swift new file mode 100644 index 0000000000..53fe93d28b --- /dev/null +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStageVocabulary.swift @@ -0,0 +1,30 @@ +/// Which stage strings mean "awake" in a stored hypnogram. +/// +/// The tree carries TWO stage vocabularies, and that is deliberate rather than sloppy: +/// +/// - **Segment `stage` strings** (`StageSegment.stage`, hypnogram rows) canonicalise to `"wake"`. +/// `SleepStagerV2` models its own states as `"awake"` internally and renames to `"wake"` on the way +/// out for exactly this reason. +/// - **Minutes-dictionary keys** (`SleepStageTotals`, `SleepWindowReclip`) canonicalise to `"awake"`. +/// +/// The bug this exists to close is the dictionary vocabulary reaching a SEGMENT comparison. Imports do +/// not pass through `SleepStagerV2`: Oura's phase table is `["deep","light","rem","awake"]`, and generic +/// wearable JSON carries whatever the source app wrote. A consumer written `stage == "wake"` then +/// silently misfiles those segments, and — worse — `stage != "wake"` counts them as SLEEP. +/// +/// Six sites already defended with `case "wake", "awake"` while five did not, which is what makes this +/// a missing shared rule rather than a missing idea. +/// +/// A PREDICATE, deliberately, not a canonicaliser: it fixes the comparisons without rewriting any +/// stored string, so no persisted hypnogram changes meaning and neither vocabulary above moves. +public enum SleepStageVocabulary { + + /// True for either spelling of the wake stage, ignoring case and surrounding whitespace. + /// + /// Use on a SEGMENT stage string. Minutes dictionaries are keyed `"awake"` by construction and do + /// not need it. + public static func isWake(_ stage: String) -> Bool { + let s = stage.trimmingCharacters(in: .whitespaces).lowercased() + return s == "wake" || s == "awake" + } +} diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift index bf74efc971..c04fe68d02 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift @@ -1072,7 +1072,7 @@ public enum SleepStager { static func efficiency(start: Int, end: Int, stages: [StageSegment]) -> Double { let inBed = Double(end - start) if inBed <= 0 { return 0 } - let wake = stages.filter { $0.stage == "wake" }.reduce(0.0) { $0 + Double($1.end - $1.start) } + let wake = stages.filter { SleepStageVocabulary.isWake($0.stage) }.reduce(0.0) { $0 + Double($1.end - $1.start) } let asleep = max(0.0, inBed - wake) return min(1.0, asleep / inBed) } @@ -2190,7 +2190,7 @@ public enum SleepStager { var waso = 0.0 var disturbances = 0 - for s in segs where s.stage == "wake" { + for s in segs where SleepStageVocabulary.isWake(s.stage) { let w0 = max(Double(s.start), onset) let w1 = min(Double(s.end), sptEnd) if w1 > w0 { waso += (w1 - w0); disturbances += 1 } diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/WakeMotionRefinement.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/WakeMotionRefinement.swift index 266a085183..d8703124b0 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/WakeMotionRefinement.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/WakeMotionRefinement.swift @@ -185,7 +185,7 @@ public enum WakeMotionRefinement { /// this pass only ever acts when BOTH read "hot-but-still". static func refineSegment(_ seg: StageSegment, gravByMinute: [Int: [GravitySample]], ticksByMinute: [Int: Int]) -> [StageSegment] { - guard seg.stage == "wake", seg.end - seg.start >= minWakeSegmentSeconds else { return [seg] } + guard SleepStageVocabulary.isWake(seg.stage), seg.end - seg.start >= minWakeSegmentSeconds else { return [seg] } let mins = minutes(from: seg.start, to: seg.end) guard !mins.isEmpty else { return [seg] } diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStageVocabularyTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStageVocabularyTests.swift new file mode 100644 index 0000000000..32da39a990 --- /dev/null +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStageVocabularyTests.swift @@ -0,0 +1,80 @@ +import XCTest +@testable import StrandAnalytics + +/// #979 — both spellings of the wake stage occur in stored hypnograms, and five segment comparisons +/// only recognised one of them. +/// +/// The damaging shape is `stage != "wake"`, used to mean "asleep": an imported `"awake"` segment fell +/// through it and was counted as SLEEP, inflating the efficiency figure. The mirror shape, +/// `stage == "wake"`, under-counted wake time and made the #987 wake refinement skip those segments. +/// +/// Twin of the Kotlin `SleepStageVocabularyTest`; same cases in the same order. +final class SleepStageVocabularyTests: XCTestCase { + + /// Both spellings are wake. This is the whole point. + func testBothSpellingsAreWake() { + XCTAssertTrue(SleepStageVocabulary.isWake("wake")) + XCTAssertTrue(SleepStageVocabulary.isWake("awake")) + } + + /// Sleep stages are not wake — the predicate must not swallow the rest of the vocabulary. + func testSleepStagesAreNotWake() { + for s in ["deep", "light", "rem"] { + XCTAssertFalse(SleepStageVocabulary.isWake(s), "\(s) must not read as wake") + } + } + + /// Imported JSON is not guaranteed tidy; casing and padding must not decide a sleep score. + func testCasingAndWhitespaceAreFolded() { + XCTAssertTrue(SleepStageVocabulary.isWake("Awake")) + XCTAssertTrue(SleepStageVocabulary.isWake(" WAKE ")) + XCTAssertTrue(SleepStageVocabulary.isWake("\tAwAkE")) + } + + /// An absent or unknown stage is NOT wake, which preserves the existing behaviour of the callers + /// that treat "anything that is not wake" as asleep. Widening that would be a separate change. + func testUnknownAndEmptyAreNotWake() { + XCTAssertFalse(SleepStageVocabulary.isWake("")) + XCTAssertFalse(SleepStageVocabulary.isWake(" ")) + XCTAssertFalse(SleepStageVocabulary.isWake("restless")) + } + + /// The regression itself, in the shape the importers use: a night of `awake` + `deep` must count + /// only the `deep` span as asleep. Before the fix the `awake` span fell through `!= "wake"` and was + /// added to the asleep total, so this asserted 2x the true value. + func testAwakeSegmentIsNotCountedAsAsleep() { + let segs: [(stage: String, seconds: Int)] = [("awake", 1800), ("deep", 1800)] + let asleep = segs.filter { !SleepStageVocabulary.isWake($0.stage) }.reduce(0) { $0 + $1.seconds } + XCTAssertEqual(asleep, 1800) + } + + /// And the mirror shape: wake time must include the `awake` span, which `== "wake"` dropped. + func testWakeTotalIncludesBothSpellings() { + let segs: [(stage: String, seconds: Int)] = [("wake", 600), ("awake", 300), ("rem", 1200)] + let wake = segs.filter { SleepStageVocabulary.isWake($0.stage) }.reduce(0) { $0 + $1.seconds } + XCTAssertEqual(wake, 900) + } + + /// INTEGRATION, not the predicate. The tests above pass whether or not the five call sites were + /// actually changed — they exercise the rule, not its users. This one exercises a real caller, so + /// it is the test that fails if a site is reverted. + /// + /// The same night as `SleepStagerTests.testHypnogramMetricsAASM`, with the WASO segment spelled + /// `awake`. `tst` is computed from a POSITIVE list (`light || deep || rem`) so it is immune either + /// way at 1080 s; WASO and the disturbance count are not, and read 0 before the fix. + func testWasoAndDisturbancesCountAnAwakeSegment() { + let stages = [ + StageSegment(start: 0, end: 60, stage: "wake"), // pre-onset, clipped out of WASO + StageSegment(start: 60, end: 600, stage: "light"), + StageSegment(start: 600, end: 900, stage: "deep"), + StageSegment(start: 900, end: 960, stage: "awake"), // the other spelling — 60 s of WASO + StageSegment(start: 960, end: 1200, stage: "rem"), + ] + let session = SleepSession(start: 0, end: 1200, efficiency: 0.95, + stages: stages, restingHR: 50, avgHRV: 60) + let m = SleepStager.hypnogramMetrics(session) + XCTAssertEqual(m.tstS, 1080, accuracy: 1e-9, "sleep total must be unaffected either way") + XCTAssertEqual(m.wasoS, 60, accuracy: 1e-9, "an awake segment is wake after sleep onset") + XCTAssertEqual(m.disturbances, 1, "and counts as one disturbance") + } +} diff --git a/Strand/Data/WearableImporter.swift b/Strand/Data/WearableImporter.swift index 8106b7cfe6..02524d97a3 100644 --- a/Strand/Data/WearableImporter.swift +++ b/Strand/Data/WearableImporter.swift @@ -1,4 +1,5 @@ import Foundation +import StrandAnalytics import WhoopStore import StrandImport @@ -131,7 +132,7 @@ enum WearableImporter { var asleep = 0 for seg in segs { guard let s = seg["start"] as? Int, let e = seg["end"] as? Int, - let stage = seg["stage"] as? String, stage != "wake" else { continue } + let stage = seg["stage"] as? String, !SleepStageVocabulary.isWake(stage) else { continue } asleep += max(0, e - s) } return min(100, Double(asleep) / Double(end - start) * 100) diff --git a/Strand/Data/XiaomiImporter.swift b/Strand/Data/XiaomiImporter.swift index ac63d65e31..c6ebf9b7e3 100644 --- a/Strand/Data/XiaomiImporter.swift +++ b/Strand/Data/XiaomiImporter.swift @@ -1,4 +1,5 @@ import Foundation +import StrandAnalytics import WhoopStore import StrandImport @@ -133,7 +134,7 @@ enum XiaomiImporter { var asleep = 0 for seg in segs { guard let s = seg["start"] as? Int, let e = seg["end"] as? Int, - let stage = seg["stage"] as? String, stage != "wake" else { continue } + let stage = seg["stage"] as? String, !SleepStageVocabulary.isWake(stage) else { continue } asleep += max(0, e - s) } return min(100, Double(asleep) / Double(end - start) * 100) diff --git a/android/app/src/main/java/com/noop/analytics/SleepStageVocabulary.kt b/android/app/src/main/java/com/noop/analytics/SleepStageVocabulary.kt new file mode 100644 index 0000000000..07aec1a0d1 --- /dev/null +++ b/android/app/src/main/java/com/noop/analytics/SleepStageVocabulary.kt @@ -0,0 +1,32 @@ +package com.noop.analytics + +/** + * Which stage strings mean "awake" in a stored hypnogram. Twin of the Swift `SleepStageVocabulary`. + * + * The tree carries TWO stage vocabularies, and that is deliberate rather than sloppy: + * + * - **Segment `stage` strings** (hypnogram rows) canonicalise to `"wake"`. [SleepStagerV2] models its + * own states as `"awake"` internally and renames to `"wake"` on the way out for exactly this reason. + * - **Minutes-dictionary keys** ([SleepStageTotals]) canonicalise to `"awake"`. + * + * The bug this closes is the dictionary vocabulary reaching a SEGMENT comparison. Imports do not pass + * through [SleepStagerV2]: Oura's phase table is `["deep","light","rem","awake"]`, and generic wearable + * JSON carries whatever the source app wrote. A consumer written `stage == "wake"` then silently + * misfiles those segments, and — worse — `stage != "wake"` counts them as SLEEP. + * + * A PREDICATE, deliberately, not a canonicaliser: it fixes the comparisons without rewriting any stored + * string, so no persisted hypnogram changes meaning and neither vocabulary above moves. + */ +object SleepStageVocabulary { + + /** + * True for either spelling of the wake stage, ignoring case and surrounding whitespace. + * + * Use on a SEGMENT stage string. Minutes dictionaries are keyed `"awake"` by construction and do + * not need it. The UI's `canonicalStage` folds through this so the alias rule has one definition. + */ + fun isWake(stage: String): Boolean { + val s = stage.trim().lowercase() + return s == "wake" || s == "awake" + } +} diff --git a/android/app/src/main/java/com/noop/analytics/SleepStager.kt b/android/app/src/main/java/com/noop/analytics/SleepStager.kt index 15c3a863bc..46a54642da 100644 --- a/android/app/src/main/java/com/noop/analytics/SleepStager.kt +++ b/android/app/src/main/java/com/noop/analytics/SleepStager.kt @@ -1154,7 +1154,7 @@ object SleepStager { internal fun efficiency(start: Long, end: Long, stages: List): Double { val inBed = (end - start).toDouble() if (inBed <= 0) return 0.0 - val wake = stages.filter { it.stage == "wake" }.sumOf { (it.end - it.start).toDouble() } + val wake = stages.filter { SleepStageVocabulary.isWake(it.stage) }.sumOf { (it.end - it.start).toDouble() } val asleep = maxOf(0.0, inBed - wake) return minOf(1.0, asleep / inBed) } @@ -2375,7 +2375,7 @@ object SleepStager { var waso = 0.0 var disturbances = 0 for (s in segs) { - if (s.stage != "wake") continue + if (!SleepStageVocabulary.isWake(s.stage)) continue val w0 = maxOf(s.start.toDouble(), onset) val w1 = minOf(s.end.toDouble(), sptEnd) if (w1 > w0) { diff --git a/android/app/src/main/java/com/noop/analytics/WakeMotionRefinement.kt b/android/app/src/main/java/com/noop/analytics/WakeMotionRefinement.kt index 8c6e5e9ea4..779553682d 100644 --- a/android/app/src/main/java/com/noop/analytics/WakeMotionRefinement.kt +++ b/android/app/src/main/java/com/noop/analytics/WakeMotionRefinement.kt @@ -220,7 +220,7 @@ object WakeMotionRefinement { gravByMinute: Map>, ticksByMinute: Map, ): List { - if (seg.stage != "wake" || seg.end - seg.start < MIN_WAKE_SEGMENT_SECONDS) return listOf(seg) + if (!SleepStageVocabulary.isWake(seg.stage) || seg.end - seg.start < MIN_WAKE_SEGMENT_SECONDS) return listOf(seg) val mins = minutes(seg.start, seg.end) if (mins.isEmpty()) return listOf(seg) diff --git a/android/app/src/main/java/com/noop/ingest/WearableExportImporter.kt b/android/app/src/main/java/com/noop/ingest/WearableExportImporter.kt index 77c15e8baf..a4c1de8867 100644 --- a/android/app/src/main/java/com/noop/ingest/WearableExportImporter.kt +++ b/android/app/src/main/java/com/noop/ingest/WearableExportImporter.kt @@ -16,6 +16,7 @@ import java.time.LocalDateTime import java.time.ZoneOffset import java.time.format.DateTimeFormatter import java.util.zip.ZipInputStream +import com.noop.analytics.SleepStageVocabulary /** * Offline file-import of a user's OWN Oura / Fitbit / Garmin data export — fully offline, no cloud @@ -785,7 +786,7 @@ object WearableExportImporter { var asleep = 0L for (i in 0 until arr.length()) { val o = arr.optJSONObject(i) ?: continue - if (o.optString("stage") == "wake") continue + if (SleepStageVocabulary.isWake(o.optString("stage"))) continue asleep += (o.optLong("end") - o.optLong("start")).coerceAtLeast(0) } return minOf(100.0, asleep.toDouble() / (end - start) * 100.0) diff --git a/android/app/src/main/java/com/noop/ingest/XiaomiBandImporter.kt b/android/app/src/main/java/com/noop/ingest/XiaomiBandImporter.kt index a829e6cd99..2ab0e935f3 100644 --- a/android/app/src/main/java/com/noop/ingest/XiaomiBandImporter.kt +++ b/android/app/src/main/java/com/noop/ingest/XiaomiBandImporter.kt @@ -15,6 +15,7 @@ import java.time.Instant import java.time.ZoneOffset import java.time.format.DateTimeFormatter import java.util.zip.ZipInputStream +import com.noop.analytics.SleepStageVocabulary /** * Imports a **Xiaomi Smart Band (Mi Band)** history from the Mi Fitness app's on-device @@ -332,7 +333,7 @@ object XiaomiBandImporter { var asleep = 0L for (i in 0 until arr.length()) { val o = arr.optJSONObject(i) ?: continue - if (o.optString("stage") == "wake") continue + if (SleepStageVocabulary.isWake(o.optString("stage"))) continue asleep += (o.optLong("end") - o.optLong("start")).coerceAtLeast(0) } return minOf(100.0, asleep.toDouble() / (end - start) * 100.0) diff --git a/android/app/src/main/java/com/noop/ui/SleepStageTimelineLogic.kt b/android/app/src/main/java/com/noop/ui/SleepStageTimelineLogic.kt index 9635cf8c49..503ea9117e 100644 --- a/android/app/src/main/java/com/noop/ui/SleepStageTimelineLogic.kt +++ b/android/app/src/main/java/com/noop/ui/SleepStageTimelineLogic.kt @@ -1,6 +1,7 @@ package com.noop.ui import org.json.JSONArray +import com.noop.analytics.SleepStageVocabulary /** One persisted per-epoch stage segment (wall-clock unix seconds). */ internal data class PersistedSegment(val start: Long, val end: Long, val stage: String) @@ -130,8 +131,10 @@ internal fun displaySmoothed( * repeating the rule, so adding an alias here is all that is needed. Written as code rather than a * KDoc link because the target is private in another file and would not resolve. */ internal fun canonicalStage(name: String): String { + // #979: the alias rule has ONE definition — SleepStageVocabulary. This still folds toward + // "awake" because that is the key the stage-colour table and the minutes dictionaries use. val n = name.trim().lowercase() - return if (n == "wake") "awake" else n + return if (SleepStageVocabulary.isWake(n)) "awake" else n } /** diff --git a/android/app/src/test/java/com/noop/analytics/SleepStageVocabularyTest.kt b/android/app/src/test/java/com/noop/analytics/SleepStageVocabularyTest.kt new file mode 100644 index 0000000000..2d64c24b53 --- /dev/null +++ b/android/app/src/test/java/com/noop/analytics/SleepStageVocabularyTest.kt @@ -0,0 +1,91 @@ +package com.noop.analytics + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * #979 — both spellings of the wake stage occur in stored hypnograms, and five segment comparisons + * only recognised one of them. + * + * The damaging shape is `stage != "wake"`, used to mean "asleep": an imported `"awake"` segment fell + * through it and was counted as SLEEP, inflating the efficiency figure. The mirror shape, + * `stage == "wake"`, under-counted wake time and made the #987 wake refinement skip those segments. + * + * Twin of the Swift `SleepStageVocabularyTests`; same cases in the same order. + */ +class SleepStageVocabularyTest { + + /** Both spellings are wake. This is the whole point. */ + @Test fun bothSpellingsAreWake() { + assertTrue(SleepStageVocabulary.isWake("wake")) + assertTrue(SleepStageVocabulary.isWake("awake")) + } + + /** Sleep stages are not wake — the predicate must not swallow the rest of the vocabulary. */ + @Test fun sleepStagesAreNotWake() { + for (s in listOf("deep", "light", "rem")) { + assertFalse("$s must not read as wake", SleepStageVocabulary.isWake(s)) + } + } + + /** Imported JSON is not guaranteed tidy; casing and padding must not decide a sleep score. */ + @Test fun casingAndWhitespaceAreFolded() { + assertTrue(SleepStageVocabulary.isWake("Awake")) + assertTrue(SleepStageVocabulary.isWake(" WAKE ")) + assertTrue(SleepStageVocabulary.isWake("\tAwAkE")) + } + + /** + * An absent or unknown stage is NOT wake, which preserves the existing behaviour of the callers + * that treat "anything that is not wake" as asleep. Widening that would be a separate change. + */ + @Test fun unknownAndEmptyAreNotWake() { + assertFalse(SleepStageVocabulary.isWake("")) + assertFalse(SleepStageVocabulary.isWake(" ")) + assertFalse(SleepStageVocabulary.isWake("restless")) + } + + /** + * The regression itself, in the shape the importers use: a night of `awake` + `deep` must count + * only the `deep` span as asleep. Before the fix the `awake` span fell through `!= "wake"` and was + * added to the asleep total, so this asserted 2x the true value. + */ + @Test fun awakeSegmentIsNotCountedAsAsleep() { + val segs = listOf("awake" to 1800, "deep" to 1800) + val asleep = segs.filter { !SleepStageVocabulary.isWake(it.first) }.sumOf { it.second } + assertEquals(1800, asleep) + } + + /** And the mirror shape: wake time must include the `awake` span, which `== "wake"` dropped. */ + @Test fun wakeTotalIncludesBothSpellings() { + val segs = listOf("wake" to 600, "awake" to 300, "rem" to 1200) + val wake = segs.filter { SleepStageVocabulary.isWake(it.first) }.sumOf { it.second } + assertEquals(900, wake) + } + + /** + * INTEGRATION, not the predicate. The tests above pass whether or not the five call sites were + * actually changed — they exercise the rule, not its users. This one exercises a real caller, so it + * is the test that fails if a site is reverted. Twin of the Swift + * `testWasoAndDisturbancesCountAnAwakeSegment`. + * + * `tst` is computed from a POSITIVE list (light/deep/rem) so it is immune either way at 1080 s; + * WASO and the disturbance count are not, and read 0 before the fix. + */ + @Test fun wasoAndDisturbancesCountAnAwakeSegment() { + val stages = listOf( + StageSegment(0L, 60L, "wake"), // pre-onset, clipped out of WASO + StageSegment(60L, 600L, "light"), + StageSegment(600L, 900L, "deep"), + StageSegment(900L, 960L, "awake"), // the other spelling — 60 s of WASO + StageSegment(960L, 1200L, "rem"), + ) + val session = DetectedSleep(0L, 1200L, 0.95, stages, 50, 60.0) + val m = SleepStager.hypnogramMetrics(session) + assertEquals("sleep total must be unaffected either way", 1080.0, m.tstS, 1e-9) + assertEquals("an awake segment is wake after sleep onset", 60.0, m.wasoS, 1e-9) + assertEquals("and counts as one disturbance", 1, m.disturbances) + } +}