Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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] }

Expand Down
Original file line number Diff line number Diff line change
@@ -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")
}
}
3 changes: 2 additions & 1 deletion Strand/Data/WearableImporter.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import StrandAnalytics
import WhoopStore
import StrandImport

Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion Strand/Data/XiaomiImporter.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import StrandAnalytics
import WhoopStore
import StrandImport

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
}
4 changes: 2 additions & 2 deletions android/app/src/main/java/com/noop/analytics/SleepStager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@ object SleepStager {
internal fun efficiency(start: Long, end: Long, stages: List<StageSegment>): 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)
}
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ object WakeMotionRefinement {
gravByMinute: Map<Long, List<GravitySample>>,
ticksByMinute: Map<Long, Int>,
): List<StageSegment> {
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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}