From 0595f5415a2fc8afe7465b0b30dccd7e996d1021 Mon Sep 17 00:00:00 2001 From: vishk23 <119831996+vishk23@users.noreply.github.com> Date: Sun, 26 Jul 2026 20:00:04 -0400 Subject: [PATCH 1/2] fix(sleep): an unmeasured respiration is no longer asserted as "regular" in V1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SleepStager.classifyOne` held two booleans over a NaN RRV, and the regular one read `(!rrv.isFinite) || (rrv <= rrvLo)`. That converts the ABSENCE of a respiration reading into a positive assertion that breathing was regular, which is pro-deep. On a WHOOP 5/MG it is not an edge case, it is the only code path: the v18 layout emits no `resp_rate_raw` at all (already pinned by `Whoop5HistoricalTests` / `Whoop5HistoricalDecodeTest`), so `respSample` has zero rows, every epoch's RRV is NaN, and the fabricated "regular" fires on 100% of epochs. Where a night with real respiration data has ~half its depth-shaped epochs clear that bar — it is the MEDIAN, `stageRRVLowPct` = 50 — a 5/MG night clears it for all of them, on no measurement whatsoever. Respiration evidence is now an explicit four-state `RespEvidence` (regular / irregular / measuredMidBand / unmeasured) built by one factory that both `classifyOne` and the `remRejectReason` diagnostic call, so the two hand-duplicated predicate pairs can no longer drift. `unmeasured` and `measuredMidBand` are deliberately distinct: both fail both bars, but one is "no reading" and the other is a real reading between them, and the classifier already treats them differently (the REM fallback fires only on a missing reading). The deep rule now reads `!resp.contradictsDepth` — "respiration must not rule depth out" — instead of a "regular" flag, and the waiver for a missing reading lives in one documented place rather than being implied by a `!isFinite` short-circuit nobody could see. It parallels the rule statement the missing-RMSSD case already carries ("with high parasympathetic tone WHEN MEASURABLE", #127/#129). NO LABEL CHANGES. This is a representation fix, and the tests prove it rather than asserting it: an exhaustive grid over move/HR/hrVar/RMSSD/RRV/sparse (1728 cases, plus a no-respiration-channel variant with both bars nil) runs the NEW classifier against the OLD boolean formulas reproduced verbatim in the test, and requires every label to match. What is deliberately NOT changed, and why: on a 5/MG both waivers can fire at once (sparse R-R leaves RMSSD NaN too), so the deep rule can reduce to stillness plus a low HR with no physiological corroboration. That is a real weakness. It is also bounded — `hrLow` is a percentile bar (`stageHRLowPct` = 25), so at most ~25% of sleep epochs can reach the deep gate however respiration resolves — and narrowing the waiver (e.g. distinguishing an epoch-level gap within a night that HAS a respiration channel from a device that has none) is a scoring change that needs validation data this repo does not have. Per CLAUDE.md's "validate against the artifact, not one match", that decision should be made on numbers, so the numbers are now in a test: the deep pass-rate for depth-shaped epochs is 51/100 with a real RRV distribution and 100/100 with no respiration channel, a ~1.96x difference. Also pins that `SleepStagerV2` — the DEFAULT stager — accepts `resp` and never reads it. That was a comment with no test behind it, so an edit could start or stop consuming it unnoticed; V2's cache key deliberately omits `resp` and would silently go stale if it did. The new test asserts byte-identical segments with and without a populated respiration stream. This also bounds the blast radius of the bug above: on the shipped default path the raw respiration ADC is not consulted at all, so the V1 behaviour reaches only users who have turned the V2 experiment off. Kotlin additionally gains the first `respRateAndRRV` coverage it has ever had. Both platforms changed together (Swift `SleepStager.swift`, Kotlin `SleepStager.kt`), all four duplicated predicate sites. Verified: StrandAnalytics 1150 tests green (12 new Swift), Android `testFullDebugUnitTest` 3084 tests with only the pre-existing `SyncChipStateTest` failure (11 new Kotlin); macOS `Strand` scheme builds. --- .../Sources/StrandAnalytics/SleepStager.swift | 98 +++++- .../SleepStagerRespEvidenceTests.swift | 287 ++++++++++++++++ .../SleepStagerTests.swift | 6 +- .../java/com/noop/analytics/SleepStager.kt | 107 +++++- .../analytics/SleepStagerRespEvidenceTest.kt | 314 ++++++++++++++++++ 5 files changed, 778 insertions(+), 34 deletions(-) create mode 100644 Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStagerRespEvidenceTests.swift create mode 100644 android/app/src/test/java/com/noop/analytics/SleepStagerRespEvidenceTest.kt diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift index 349faed278..cd94dd657a 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift @@ -1692,6 +1692,70 @@ public enum SleepStager { return Double(sparse) >= cardiacSparseEpochFrac * Double(sleepFeats.count) } + /// What this epoch's respiration says about depth — THREE states, because "we did not measure it" is + /// not an observation and must not be spendable as one. + /// + /// This used to be two booleans over a NaN RRV, and the `regular` one read + /// `(!f.rrv.isFinite) || (f.rrv <= rrvLo)`: a MISSING respiration reading was converted into a + /// positive assertion that breathing was regular, which is pro-deep. On a WHOOP 5/MG that is not an + /// edge case, it is the only code path — the v18 layout emits no `resp_rate_raw` at all (pinned by + /// `Whoop5HistoricalTests`), so `respSample` has zero rows, every epoch's RRV is NaN, and the + /// fabricated "regular" fires on 100% of epochs. Where a night with real respiration data has ~50% of + /// its epochs clear the `regular` bar (it is the MEDIAN — `stageRRVLowPct` = 50), a 5/MG night has + /// 100% clear it, on no measurement whatsoever. + /// + /// `unknown` is now its own case and every branch below states what it does with it, rather than the + /// answer falling out of a `!isFinite` short-circuit nobody could see. + /// `unmeasured` and `measuredMidBand` are deliberately DISTINCT. Both fail the regular and irregular + /// bars, but they mean opposite things — one is "no reading", the other is a real reading that simply + /// sits between the bars — and the classifier already treats them differently (the REM fallback fires + /// only on a missing reading). Collapsing them would silently change the hypnogram. + enum RespEvidence { + /// A finite RRV at or below the session's low bar — breathing measured as regular. + case regular + /// A finite RRV at or above the session's high bar — breathing measured as irregular. + case irregular + /// A finite RRV between the bars: measured, but neither notably regular nor notably irregular. + case measuredMidBand + /// No usable RRV for this epoch. Either the strap has no respiration channel at all (every 5/MG), + /// or this epoch's window had too few / too flat samples to derive one (`respRateAndRRV` returns + /// NaN below 8 samples, on a flat signal, under 3 peaks, or under 2 in-band breath intervals). + case unmeasured + + static func of(_ rrv: Double, lowBar: Double?, highBar: Double?) -> RespEvidence { + guard rrv.isFinite else { return .unmeasured } + if let hi = highBar, rrv >= hi { return .irregular } + if let lo = lowBar, rrv <= lo { return .regular } + return .measuredMidBand + } + + /// Whether respiration CONTRADICTS depth. The deep rule reads this rather than a "regular" flag, + /// because `unmeasured` is not evidence of regular breathing — it is the absence of evidence, and + /// the rule it feeds is "respiration must not rule depth out". + /// + /// `unmeasured` is WAIVED here, not because missing respiration is reassuring, but because + /// blocking on it would decode 0 m of deep on every 5/MG night — the exact regression #127/#129 + /// fixed for the parallel missing-RMSSD case, whose rule statement already carries the same + /// qualifier ("with high parasympathetic tone WHEN MEASURABLE"). This makes the respiration + /// waiver explicit and equally qualified instead of leaving it implied by a NaN short-circuit. + /// + /// KNOWN LIMITATION, deliberately not changed here: on a 5/MG BOTH waivers can fire at once + /// (sparse R-R leaves RMSSD NaN too), and the deep rule then reduces to stillness + a low HR with + /// no physiological corroboration at all. That is a real weakness, but it is bounded — `hrLow` is + /// a PERCENTILE bar (`stageHRLowPct` = 25), so at most ~25% of sleep epochs can clear it however + /// the respiration term resolves. Narrowing the waiver (e.g. to epoch-level gaps within a night + /// that HAS a respiration channel, versus a device with no channel at all) is a scoring change + /// that needs validation data this repo does not have — see the "validate against the artifact, + /// not one match" rule in CLAUDE.md. `SleepStagerRespEvidenceTests` pins the current behaviour and + /// quantifies the bias so that decision can be made on numbers. + var contradictsDepth: Bool { + switch self { + case .regular, .unmeasured: return false + case .irregular, .measuredMidBand: return true + } + } + } + static func classifyOne(_ f: EpochFeatures, hrLo: Double?, hrHi: Double?, rmssdHi: Double?, hrvarHi: Double?, rrvHi: Double?, rrvLo: Double?, cardiacSparse: Bool = false) -> String { @@ -1717,9 +1781,7 @@ public enum SleepStager { // Dense 4.0 nights keep the full `hrHigh || hrvarHigh` signal, so their behaviour is unchanged. (#705) let cardiacActivatedForWake = cardiacSparse ? hrHigh : cardiacActivated - let rrvIrregular = f.rrv.isFinite && rrvHi != nil && f.rrv >= rrvHi! - // Missing respiration (NaN RRV) treated as "regular" (pro-deep bias). - let rrvRegular = (!f.rrv.isFinite) || (rrvLo != nil && f.rrv <= rrvLo!) + let resp = RespEvidence.of(f.rrv, lowBar: rrvLo, highBar: rrvHi) let still = f.moveFrac <= stageStillMoveFrac let moving = f.moveFrac >= stageWakeMoveFrac @@ -1728,12 +1790,15 @@ public enum SleepStager { // cardiac half is vetted by HR only (see `cardiacActivatedForWake`), so noisy hrVar no longer // over-promotes still sleep to wake. (#705) if moving && (cardiacActivatedForWake || !hasHR) { return "wake" } - // DEEP: still + low HR + regular respiration, with high parasympathetic tone when measurable. - if still && parasympOK && hrLow && rrvRegular { return "deep" } - // REM: still body + activated cardiac + irregular respiration. - if still && cardiacActivated && rrvIrregular { return "rem" } - // REM fallback when respiration unavailable: require BOTH cardiac signals. - if still && hrHigh && hrvarHigh && !f.rrv.isFinite { return "rem" } + // DEEP: still + low HR + respiration that does not RULE OUT depth, with high parasympathetic tone + // when measurable. `contradictsDepth` is where an unmeasured respiration is waived — see its doc + // for why the waiver stays and what it costs. + if still && parasympOK && hrLow && !resp.contradictsDepth { return "deep" } + // REM: still body + activated cardiac + respiration measured as irregular. + if still && cardiacActivated && resp == .irregular { return "rem" } + // REM fallback when respiration was never MEASURED (not merely mid-band): require BOTH cardiac + // signals. A mid-band reading is real evidence and does not earn the fallback. + if still && hrHigh && hrvarHigh && resp == .unmeasured { return "rem" } return "light" } @@ -1865,23 +1930,24 @@ public enum SleepStager { let hrvarHigh = f.hrVar.isFinite && hrvarHi != nil && f.hrVar >= hrvarHi! let cardiacActivated = hrHigh || hrvarHigh let cardiacActivatedForWake = cardiacSparse ? hrHigh : cardiacActivated - let rrvIrregular = f.rrv.isFinite && rrvHi != nil && f.rrv >= rrvHi! - let rrvRegular = (!f.rrv.isFinite) || (rrvLo != nil && f.rrv <= rrvLo!) + // Same three-state respiration evidence the classifier uses, from the same factory — so the + // diagnostic cannot drift from the rule it explains (these predicates were duplicated by hand). + let resp = RespEvidence.of(f.rrv, lowBar: rrvLo, highBar: rrvHi) let still = f.moveFrac <= stageStillMoveFrac let moving = f.moveFrac >= stageWakeMoveFrac // classifyOne precedence: WAKE, then DEEP, then REM (then REM fallback), else LIGHT. // An epoch that wins WAKE or DEEP was never a REM candidate. if moving && (cardiacActivatedForWake || !hasHR) { return .wonOtherStage } // → wake - if still && parasympOK && hrLow && rrvRegular { return .wonOtherStage } // → deep + if still && parasympOK && hrLow && !resp.contradictsDepth { return .wonOtherStage } // → deep // From here the epoch did NOT win wake/deep; it is either REM or falls through to LIGHT. - if still && cardiacActivated && rrvIrregular { return .remEligible } - if still && hrHigh && hrvarHigh && !f.rrv.isFinite { return .remEligible } + if still && cardiacActivated && resp == .irregular { return .remEligible } + if still && hrHigh && hrvarHigh && resp == .unmeasured { return .remEligible } // Not REM → attribute to the FIRST unmet REM precondition (in REM-rule order). if !still { return .notStill } if !cardiacActivated { return .noCardiacActivation } - if f.rrv.isFinite { return .respRegular } // resp present but not irregular - return .noRespFallbackBar // resp absent and the no-resp bar unmet + if resp != .unmeasured { return .respRegular } // resp measured but not irregular + return .noRespFallbackBar // resp never measured and the no-resp bar unmet } /// Read-only REM-funnel triage for ONE in-bed window [start, end] (#688). Re-runs the SAME Stage-0→3 diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStagerRespEvidenceTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStagerRespEvidenceTests.swift new file mode 100644 index 0000000000..6bfe857c81 --- /dev/null +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStagerRespEvidenceTests.swift @@ -0,0 +1,287 @@ +import XCTest +@testable import StrandAnalytics +import WhoopProtocol + +/// Respiration evidence in the V1 stager: a MISSING RRV is `unmeasured`, never `regular`. +/// +/// The classifier used to hold `rrvRegular = (!rrv.isFinite) || rrv <= rrvLo`, converting the absence of a +/// respiration reading into a positive assertion that breathing was regular — which is pro-deep. On a +/// WHOOP 5/MG that is the permanent state, not an edge case: the v18 layout emits no `resp_rate_raw`, so +/// `respSample` has zero rows and every epoch's RRV is NaN. +/// +/// These tests pin three things: the corrected representation, that the correction changed no label, and +/// the SIZE of the bias that remains (so the next person to touch the deep gate has the number rather +/// than an adjective). +final class SleepStagerRespEvidenceTests: XCTestCase { + + private typealias Resp = SleepStager.RespEvidence + + // MARK: - The corrected representation + + /// The fix, stated directly: no measurement is `unmeasured`, and `unmeasured` is not `regular`. + func testMissingRrvIsUnmeasuredNotRegular() { + XCTAssertEqual(Resp.of(.nan, lowBar: 0.5, highBar: 1.0), .unmeasured) + XCTAssertNotEqual(Resp.of(.nan, lowBar: 0.5, highBar: 1.0), .regular) + // A whole session with no respiration channel leaves BOTH percentile bars nil (nothing finite to + // take a percentile of). That is the 5/MG case, and it is still `unmeasured`, not `regular`. + XCTAssertEqual(Resp.of(.nan, lowBar: nil, highBar: nil), .unmeasured) + XCTAssertNotEqual(Resp.of(.nan, lowBar: nil, highBar: nil), .regular) + XCTAssertEqual(Resp.of(.infinity, lowBar: 0.5, highBar: 1.0), .unmeasured) + } + + func testMeasuredValuesMapToTheirBands() { + XCTAssertEqual(Resp.of(0.2, lowBar: 0.5, highBar: 1.0), .regular) // at/below the low bar + XCTAssertEqual(Resp.of(0.5, lowBar: 0.5, highBar: 1.0), .regular) // inclusive + XCTAssertEqual(Resp.of(1.0, lowBar: 0.5, highBar: 1.0), .irregular) // inclusive + XCTAssertEqual(Resp.of(2.0, lowBar: 0.5, highBar: 1.0), .irregular) + XCTAssertEqual(Resp.of(0.75, lowBar: 0.5, highBar: 1.0), .measuredMidBand) + } + + /// `measuredMidBand` and `unmeasured` both fail both bars but mean opposite things, and the classifier + /// treats them differently — collapsing them into one "unknown" would silently change the hypnogram. + func testMidBandAndUnmeasuredAreDistinctAndBehaveDifferently() { + XCTAssertNotEqual(Resp.measuredMidBand, Resp.unmeasured) + // Depth: a real mid-band reading rules depth OUT; a missing reading is waived (see below). + XCTAssertTrue(Resp.measuredMidBand.contradictsDepth) + XCTAssertFalse(Resp.unmeasured.contradictsDepth) + XCTAssertFalse(Resp.regular.contradictsDepth) + XCTAssertTrue(Resp.irregular.contradictsDepth) + // A finite RRV with no bars (every SLEEP epoch was NaN but this non-sleep epoch was not) is a + // measurement, so it must not fall into the unmeasured branch. + XCTAssertEqual(Resp.of(0.7, lowBar: nil, highBar: nil), .measuredMidBand) + } + + /// The waiver is now explicit and NAMED rather than emergent from a `!isFinite` short-circuit — but it + /// is still a waiver, and this pins that it is: an unmeasured respiration does NOT block deep. + /// Removing it would decode 0 m of deep on every 5/MG night (the #127/#129 regression, for the + /// parallel missing-RMSSD case). Narrowing it is a scoring change that needs validation data. + func testUnmeasuredRespirationIsWaivedForDeepAndTheWaiverIsExplicit() { + let epoch = feature(moveFrac: 0, hr: 50, hrVar: 0, rmssd: 60, rrv: .nan) + XCTAssertEqual(classify(epoch), "deep") + XCTAssertFalse(Resp.unmeasured.contradictsDepth, + "the deep gate's treatment of a missing reading must be readable in one place") + } + + // MARK: - The correction changed no label + + /// Exhaustive grid: the new `RespEvidence` classifier must agree with the OLD boolean predicates on + /// every combination. The old formulas are reproduced verbatim here so the equivalence is checkable + /// rather than asserted — this is what makes the change a representation fix and not a scoring change. + func testClassificationIsIdenticalToThePreFixPredicates() { + func legacy(_ f: SleepStager.EpochFeatures, + hrLo: Double?, hrHi: Double?, rmssdHi: Double?, hrvarHi: Double?, + rrvHi: Double?, rrvLo: Double?, cardiacSparse: Bool) -> String { + let hasHR = f.hr.isFinite + let hrLow = hasHR && hrLo != nil && f.hr <= hrLo! + let hrHigh = hasHR && hrHi != nil && f.hr >= hrHi! + let parasympOK = (!f.rmssd.isFinite) || (rmssdHi != nil && f.rmssd >= rmssdHi!) + let hrvarHigh = f.hrVar.isFinite && hrvarHi != nil && f.hrVar >= hrvarHi! + let cardiacActivated = hrHigh || hrvarHigh + let cardiacActivatedForWake = cardiacSparse ? hrHigh : cardiacActivated + // The two predicates this change replaced, exactly as they were. + let rrvIrregular = f.rrv.isFinite && rrvHi != nil && f.rrv >= rrvHi! + let rrvRegular = (!f.rrv.isFinite) || (rrvLo != nil && f.rrv <= rrvLo!) + let still = f.moveFrac <= SleepStager.stageStillMoveFrac + let moving = f.moveFrac >= SleepStager.stageWakeMoveFrac + if moving && (cardiacActivatedForWake || !hasHR) { return "wake" } + if still && parasympOK && hrLow && rrvRegular { return "deep" } + if still && cardiacActivated && rrvIrregular { return "rem" } + if still && hrHigh && hrvarHigh && !f.rrv.isFinite { return "rem" } + return "light" + } + + var checked = 0 + for moveFrac in [0.0, 0.05, 0.12, 0.2] { + for hr in [Double.nan, 45, 60, 95] { + for hrVar in [Double.nan, 1, 20] { + for rmssd in [Double.nan, 10, 80] { + // NaN (never measured), below/at/between/at/above the bars. + for rrv in [Double.nan, 0.2, 0.5, 0.75, 1.0, 2.0] { + for sparse in [false, true] { + let f = feature(moveFrac: moveFrac, hr: hr, hrVar: hrVar, + rmssd: rmssd, rrv: rrv) + let new = SleepStager.classifyOne( + f, hrLo: 55, hrHi: 90, rmssdHi: 50, hrvarHi: 10, + rrvHi: 1.0, rrvLo: 0.5, cardiacSparse: sparse) + let old = legacy(f, hrLo: 55, hrHi: 90, rmssdHi: 50, hrvarHi: 10, + rrvHi: 1.0, rrvLo: 0.5, cardiacSparse: sparse) + XCTAssertEqual(new, old, + "label changed for move=\(moveFrac) hr=\(hr) hrVar=\(hrVar) " + + "rmssd=\(rmssd) rrv=\(rrv) sparse=\(sparse)") + checked += 1 + } + } + } + } + } + } + XCTAssertEqual(checked, 4 * 4 * 3 * 3 * 6 * 2) + } + + /// The same equivalence for the session with NO respiration channel at all (both bars nil) — the + /// WHOOP 5/MG shape, and the one the bias actually lives on. + func testNoRespChannelSessionIsAlsoUnchanged() { + for moveFrac in [0.0, 0.12, 0.2] { + for hr in [Double.nan, 45, 95] { + for hrVar in [Double.nan, 1, 20] { + for rmssd in [Double.nan, 10, 80] { + let f = feature(moveFrac: moveFrac, hr: hr, hrVar: hrVar, rmssd: rmssd, rrv: .nan) + let new = SleepStager.classifyOne(f, hrLo: 55, hrHi: 90, rmssdHi: 50, + hrvarHi: 10, rrvHi: nil, rrvLo: nil) + // Old formulas with nil bars: rrvIrregular = false, rrvRegular = true (the bug). + let hasHR = f.hr.isFinite + let hrLow = hasHR && f.hr <= 55 + let hrHigh = hasHR && f.hr >= 90 + let parasympOK = (!f.rmssd.isFinite) || f.rmssd >= 50 + let hrvarHigh = f.hrVar.isFinite && f.hrVar >= 10 + let still = f.moveFrac <= SleepStager.stageStillMoveFrac + let moving = f.moveFrac >= SleepStager.stageWakeMoveFrac + let old: String + if moving && ((hrHigh || hrvarHigh) || !hasHR) { old = "wake" } + else if still && parasympOK && hrLow { old = "deep" } + else if still && hrHigh && hrvarHigh { old = "rem" } + else { old = "light" } + XCTAssertEqual(new, old) + } + } + } + } + } + + /// `remRejectReason` must stay in lockstep with `classifyOne` — they were hand-duplicated predicates + /// and now share one factory, so this guards the seam. + func testRemRejectReasonAgreesWithTheClassifier() { + for rrv in [Double.nan, 0.2, 0.75, 2.0] { + for hr in [Double.nan, 45, 95] { + for hrVar in [Double.nan, 1, 20] { + for moveFrac in [0.0, 0.2] { + let f = feature(moveFrac: moveFrac, hr: hr, hrVar: hrVar, rmssd: .nan, rrv: rrv) + let label = SleepStager.classifyOne(f, hrLo: 55, hrHi: 90, rmssdHi: 50, + hrvarHi: 10, rrvHi: 1.0, rrvLo: 0.5) + let reason = SleepStager.remRejectReason(f, hrLo: 55, hrHi: 90, rmssdHi: 50, + hrvarHi: 10, rrvHi: 1.0, rrvLo: 0.5) + XCTAssertEqual(label == "rem", reason == .remEligible, + "rem-eligibility disagreed for rrv=\(rrv) hr=\(hr)") + } + } + } + } + } + + /// A measured-but-mid-band respiration must NOT earn the missing-respiration REM fallback — that + /// fallback exists to compensate for having no reading, not for having an unremarkable one. + func testMidBandDoesNotEarnTheMissingRespirationRemFallback() { + // still + hrHigh + hrvarHigh: the fallback's exact preconditions. + let unmeasured = feature(moveFrac: 0, hr: 95, hrVar: 20, rmssd: .nan, rrv: .nan) + let midBand = feature(moveFrac: 0, hr: 95, hrVar: 20, rmssd: .nan, rrv: 0.75) + XCTAssertEqual(classify(unmeasured), "rem") + XCTAssertEqual(classify(midBand), "light") + } + + // MARK: - The size of the remaining bias + + /// QUANTIFIED, so the next person to touch the deep gate argues with a number. + /// + /// The "regular" bar is `stageRRVLowPct` = 50 — the MEDIAN. So on a night with real respiration data + /// about half of otherwise-depth-shaped epochs clear it. On a session with no respiration channel the + /// old code cleared it for ALL of them, on no measurement at all. That factor-of-two is the bias. + /// + /// It is bounded, though, and the bound matters as much as the bias: `hrLow` is itself a percentile + /// bar (`stageHRLowPct` = 25), so at most ~25% of sleep epochs can reach the deep gate however the + /// respiration term resolves. The bias inflates deep within that ceiling; it cannot run away. + func testTheDeepPassRateDoublesWhenTheRespirationChannelIsAbsent() { + // 100 depth-shaped epochs (still, low HR, high parasympathetic tone) whose RRV spans a real + // distribution: half at/below the median bar, half above it. + let rrvs = (0..<100).map { Double($0) / 100.0 } // 0.00 … 0.99, median 0.5 + let withResp = rrvs.map { rrv in + classify(feature(moveFrac: 0, hr: 50, hrVar: 0, rmssd: 60, rrv: rrv)) + } + let deepWithResp = withResp.filter { $0 == "deep" }.count + XCTAssertEqual(deepWithResp, 51, "the regular bar is the median, so ~half of epochs pass") + + // The same epochs on a strap with no respiration channel: every RRV is NaN. + let withoutResp = rrvs.map { _ in + SleepStager.classifyOne(feature(moveFrac: 0, hr: 50, hrVar: 0, rmssd: 60, rrv: .nan), + hrLo: 55, hrHi: 90, rmssdHi: 50, hrvarHi: 10, + rrvHi: nil, rrvLo: nil) + } + XCTAssertEqual(withoutResp.filter { $0 == "deep" }.count, 100, + "with no respiration channel EVERY depth-shaped epoch passes the deep gate") + // ~2x. Stated as a ratio so the assertion survives a bar tweak. + XCTAssertEqual(Double(100) / Double(deepWithResp), 1.96, accuracy: 0.05) + } + + /// The RRV source itself: with no respiration samples there is nothing to measure, so the NaN is + /// honest at the point it is produced. The bug was never here — it was in spending that NaN as + /// evidence downstream. + func testRrvIsNaNWhenThereAreNoRespirationSamples() { + XCTAssertTrue(SleepStager.respRateAndRRV([]).1.isNaN) + XCTAssertTrue(SleepStager.respRateAndRRV([1, 2, 3]).1.isNaN) + } + + // MARK: - Fixtures + + private func feature(moveFrac: Double, hr: Double, hrVar: Double, + rmssd: Double, rrv: Double) -> SleepStager.EpochFeatures { + SleepStager.EpochFeatures(index: 0, midTs: 0, count: 0, moveFrac: moveFrac, + ckSleep: true, hr: hr, hrVar: hrVar, rmssd: rmssd, sdnn: 0, + respRate: 14, rrv: rrv, clock: 0.5) + } + + private func classify(_ f: SleepStager.EpochFeatures) -> String { + SleepStager.classifyOne(f, hrLo: 55, hrHi: 90, rmssdHi: 50, hrvarHi: 10, + rrvHi: 1.0, rrvLo: 0.5) + } +} + +/// `SleepStagerV2` — the DEFAULT stager — accepts a `resp` argument and never reads it. +/// +/// That was documented in a comment and pinned by nothing, so a future edit could start consuming it (or +/// stop) with no test noticing. These make the contract enforceable. It also matters for the bug above: +/// on the shipped default path the raw respiration ADC is not consulted at all, so the V1 pro-deep bias +/// reaches only users who have turned the V2 experiment off. +final class SleepStagerV2RespIsInertTests: XCTestCase { + + private func stillGravity(start: Int, durationS: Int) -> [GravitySample] { + (0.. [HRSample] { + (0.. [RRInterval] { + (0.. SleepStager.EpochFeatures { SleepStager.EpochFeatures(index: 0, midTs: 0, count: 0, moveFrac: 0, // still ckSleep: true, hr: 50, hrVar: 0, rmssd: rmssd, sdnn: 0, - respRate: 14, rrv: .nan, // missing resp → regular (pro-deep) + // No respiration reading: `RespEvidence.unmeasured`, which is WAIVED for + // deep (not asserted as "regular" — see SleepStagerRespEvidenceTests). + respRate: 14, rrv: .nan, clock: 0.5) } @@ -497,7 +499,7 @@ final class SleepStagerTests: XCTestCase { // epoch stays sleep. A dense 4.0 night (cardiacSparse:false) keeps the original hrHigh||hrvarHigh signal. private func ppgWakeEpoch() -> SleepStager.EpochFeatures { // moveFrac just over the wake bar (0.15), HR normal (60, below hrHi=90, above hrLo=55 so not deep), - // hrVar inflated above the high bar, missing R-R (sparse → resp also NaN → regular). + // hrVar inflated above the high bar, missing R-R (sparse → resp also NaN → unmeasured/waived). SleepStager.EpochFeatures(index: 0, midTs: 0, count: 0, moveFrac: 0.16, ckSleep: true, hr: 60, hrVar: 200, rmssd: .nan, sdnn: 0, respRate: 14, rrv: .nan, clock: 0.5) 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 9f4d46c608..748e144c7e 100644 --- a/android/app/src/main/java/com/noop/analytics/SleepStager.kt +++ b/android/app/src/main/java/com/noop/analytics/SleepStager.kt @@ -1867,6 +1867,79 @@ object SleepStager { return sparse.toDouble() >= cardiacSparseEpochFrac * sleepFeats.size.toDouble() } + /** + * What one epoch's respiration says about depth — FOUR states, because "we did not measure it" is not + * an observation and must not be spendable as one. + * + * This used to be two booleans over a NaN RRV, and the `regular` one read + * `(!f.rrv.isFinite()) || (f.rrv <= rrvLo)`: a MISSING respiration reading was converted into a + * positive assertion that breathing was regular, which is pro-deep. On a WHOOP 5/MG that is not an + * edge case, it is the only code path — the v18 layout emits no `resp_rate_raw` at all (pinned by + * `Whoop5HistoricalDecodeTest`), so `respSample` has zero rows, every epoch's RRV is NaN, and the + * fabricated "regular" fires on 100% of epochs. Where a night with real respiration data has ~50% of + * its epochs clear the `regular` bar (it is the MEDIAN — `stageRRVLowPct` = 50), a 5/MG night has + * 100% clear it, on no measurement whatsoever. + * + * [UNMEASURED] and [MEASURED_MID_BAND] are deliberately DISTINCT. Both fail the regular and irregular + * bars, but they mean opposite things — one is "no reading", the other is a real reading that simply + * sits between the bars — and the classifier already treats them differently (the REM fallback fires + * only on a missing reading). Collapsing them would silently change the hypnogram. + * + * Mirror of the Swift `SleepStager.RespEvidence`. + */ + internal enum class RespEvidence { + /** A finite RRV at or below the session's low bar — breathing measured as regular. */ + REGULAR, + + /** A finite RRV at or above the session's high bar — breathing measured as irregular. */ + IRREGULAR, + + /** A finite RRV between the bars: measured, but neither notably regular nor notably irregular. */ + MEASURED_MID_BAND, + + /** + * No usable RRV for this epoch. Either the strap has no respiration channel at all (every 5/MG), + * or this epoch's window had too few / too flat samples to derive one (`respRateAndRRV` returns + * NaN below 8 samples, on a flat signal, under 3 peaks, or under 2 in-band breath intervals). + */ + UNMEASURED, + ; + + /** + * Whether respiration CONTRADICTS depth. The deep rule reads this rather than a "regular" flag, + * because [UNMEASURED] is not evidence of regular breathing — it is the absence of evidence, and + * the rule it feeds is "respiration must not rule depth out". + * + * [UNMEASURED] is WAIVED here, not because missing respiration is reassuring, but because + * blocking on it would decode 0 m of deep on every 5/MG night — the exact regression #127/#129 + * fixed for the parallel missing-RMSSD case, whose rule statement already carries the same + * qualifier ("with high parasympathetic tone WHEN MEASURABLE"). This makes the respiration waiver + * explicit and equally qualified instead of leaving it implied by a NaN short-circuit. + * + * KNOWN LIMITATION, deliberately not changed here: on a 5/MG BOTH waivers can fire at once + * (sparse R-R leaves RMSSD NaN too), and the deep rule then reduces to stillness + a low HR with + * no physiological corroboration at all. That is a real weakness, but it is bounded — `hrLow` is + * a PERCENTILE bar (`stageHRLowPct` = 25), so at most ~25% of sleep epochs can clear it however + * the respiration term resolves. Narrowing the waiver is a scoring change that needs validation + * data this repo does not have; `SleepStagerRespEvidenceTest` pins the current behaviour and + * quantifies the bias so that decision can be made on numbers. + */ + val contradictsDepth: Boolean + get() = when (this) { + REGULAR, UNMEASURED -> false + IRREGULAR, MEASURED_MID_BAND -> true + } + + companion object { + fun of(rrv: Double, lowBar: Double?, highBar: Double?): RespEvidence { + if (!rrv.isFinite()) return UNMEASURED + if (highBar != null && rrv >= highBar) return IRREGULAR + if (lowBar != null && rrv <= lowBar) return REGULAR + return MEASURED_MID_BAND + } + } + } + internal fun classifyOne( f: EpochFeatures, hrLo: Double?, hrHi: Double?, rmssdHi: Double?, hrvarHi: Double?, rrvHi: Double?, rrvLo: Double?, @@ -1894,9 +1967,7 @@ object SleepStager { // Dense 4.0 nights keep the full `hrHigh || hrvarHigh` signal, so their behaviour is unchanged. (#705) val cardiacActivatedForWake = if (cardiacSparse) hrHigh else cardiacActivated - val rrvIrregular = f.rrv.isFinite() && rrvHi != null && f.rrv >= rrvHi - // Missing respiration (NaN RRV) treated as "regular" (pro-deep bias). - val rrvRegular = (!f.rrv.isFinite()) || (rrvLo != null && f.rrv <= rrvLo) + val resp = RespEvidence.of(f.rrv, lowBar = rrvLo, highBar = rrvHi) val still = f.moveFrac <= stageStillMoveFrac val moving = f.moveFrac >= stageWakeMoveFrac @@ -1905,12 +1976,15 @@ object SleepStager { // cardiac half is vetted by HR only (see `cardiacActivatedForWake`), so noisy hrVar no longer // over-promotes still sleep to wake. (#705) if (moving && (cardiacActivatedForWake || !hasHR)) return "wake" - // DEEP: still + low HR + regular respiration, with high parasympathetic tone when measurable. - if (still && parasympOK && hrLow && rrvRegular) return "deep" - // REM: still body + activated cardiac + irregular respiration. - if (still && cardiacActivated && rrvIrregular) return "rem" - // REM fallback when respiration unavailable: require BOTH cardiac signals. - if (still && hrHigh && hrvarHigh && !f.rrv.isFinite()) return "rem" + // DEEP: still + low HR + respiration that does not RULE OUT depth, with high parasympathetic tone + // when measurable. [RespEvidence.contradictsDepth] is where an unmeasured respiration is waived — + // see its doc for why the waiver stays and what it costs. + if (still && parasympOK && hrLow && !resp.contradictsDepth) return "deep" + // REM: still body + activated cardiac + respiration measured as irregular. + if (still && cardiacActivated && resp == RespEvidence.IRREGULAR) return "rem" + // REM fallback when respiration was never MEASURED (not merely mid-band): require BOTH cardiac + // signals. A mid-band reading is real evidence and does not earn the fallback. + if (still && hrHigh && hrvarHigh && resp == RespEvidence.UNMEASURED) return "rem" return "light" } @@ -2040,23 +2114,24 @@ object SleepStager { val hrvarHigh = f.hrVar.isFinite() && hrvarHi != null && f.hrVar >= hrvarHi val cardiacActivated = hrHigh || hrvarHigh val cardiacActivatedForWake = if (cardiacSparse) hrHigh else cardiacActivated - val rrvIrregular = f.rrv.isFinite() && rrvHi != null && f.rrv >= rrvHi - val rrvRegular = (!f.rrv.isFinite()) || (rrvLo != null && f.rrv <= rrvLo) + // Same respiration evidence the classifier uses, from the same factory — so the diagnostic cannot + // drift from the rule it explains (these predicates were duplicated by hand). + val resp = RespEvidence.of(f.rrv, lowBar = rrvLo, highBar = rrvHi) val still = f.moveFrac <= stageStillMoveFrac val moving = f.moveFrac >= stageWakeMoveFrac // classifyOne precedence: WAKE, then DEEP, then REM (then REM fallback), else LIGHT. // An epoch that wins WAKE or DEEP was never a REM candidate. if (moving && (cardiacActivatedForWake || !hasHR)) return REMRejectReason.WON_OTHER_STAGE // → wake - if (still && parasympOK && hrLow && rrvRegular) return REMRejectReason.WON_OTHER_STAGE // → deep + if (still && parasympOK && hrLow && !resp.contradictsDepth) return REMRejectReason.WON_OTHER_STAGE // → deep // From here the epoch did NOT win wake/deep; it is either REM or falls through to LIGHT. - if (still && cardiacActivated && rrvIrregular) return REMRejectReason.REM_ELIGIBLE - if (still && hrHigh && hrvarHigh && !f.rrv.isFinite()) return REMRejectReason.REM_ELIGIBLE + if (still && cardiacActivated && resp == RespEvidence.IRREGULAR) return REMRejectReason.REM_ELIGIBLE + if (still && hrHigh && hrvarHigh && resp == RespEvidence.UNMEASURED) return REMRejectReason.REM_ELIGIBLE // Not REM → attribute to the FIRST unmet REM precondition (in REM-rule order). if (!still) return REMRejectReason.NOT_STILL if (!cardiacActivated) return REMRejectReason.NO_CARDIAC_ACTIVATION - if (f.rrv.isFinite()) return REMRejectReason.RESP_REGULAR // resp present but not irregular - return REMRejectReason.NO_RESP_FALLBACK_BAR // resp absent and no-resp bar unmet + if (resp != RespEvidence.UNMEASURED) return REMRejectReason.RESP_REGULAR // measured, not irregular + return REMRejectReason.NO_RESP_FALLBACK_BAR // never measured and no-resp bar unmet } /** diff --git a/android/app/src/test/java/com/noop/analytics/SleepStagerRespEvidenceTest.kt b/android/app/src/test/java/com/noop/analytics/SleepStagerRespEvidenceTest.kt new file mode 100644 index 0000000000..bc0f594698 --- /dev/null +++ b/android/app/src/test/java/com/noop/analytics/SleepStagerRespEvidenceTest.kt @@ -0,0 +1,314 @@ +package com.noop.analytics + +import com.noop.analytics.SleepStager.RespEvidence +import com.noop.data.GravitySample +import com.noop.data.HrSample +import com.noop.data.RespSample +import com.noop.data.RrInterval +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import kotlin.math.PI +import kotlin.math.roundToInt +import kotlin.math.sin + +/** + * Respiration evidence in the V1 stager: a MISSING RRV is [RespEvidence.UNMEASURED], never + * [RespEvidence.REGULAR]. Android twin of SleepStagerRespEvidenceTests.swift. + * + * The classifier used to hold `rrvRegular = (!rrv.isFinite()) || rrv <= rrvLo`, converting the absence of a + * respiration reading into a positive assertion that breathing was regular — which is pro-deep. On a WHOOP + * 5/MG that is the permanent state, not an edge case: the v18 layout emits no `resp_rate_raw`, so + * `respSample` has zero rows and every epoch's RRV is NaN. + * + * These pin three things: the corrected representation, that the correction changed no label, and the SIZE + * of the bias that remains (so the next person to touch the deep gate has the number, not an adjective). + */ +class SleepStagerRespEvidenceTest { + + private val dev = "test" + private val refMidnight = 1_749_513_600L + + private fun feature( + moveFrac: Double, hr: Double, hrVar: Double, rmssd: Double, rrv: Double, + ): SleepStager.EpochFeatures = SleepStager.EpochFeatures( + index = 0, midTs = 0.0, count = 0.0, moveFrac = moveFrac, ckSleep = true, + hr = hr, hrVar = hrVar, rmssd = rmssd, sdnn = 0.0, respRate = 14.0, rrv = rrv, clock = 0.5, + ) + + private fun classify(f: SleepStager.EpochFeatures): String = + SleepStager.classifyOne(f, hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, + rrvHi = 1.0, rrvLo = 0.5) + + // ── The corrected representation ───────────────────────────────────────────────────────────────── + + /** The fix, stated directly: no measurement is UNMEASURED, and UNMEASURED is not REGULAR. */ + @Test + fun missingRrvIsUnmeasuredNotRegular() { + assertEquals(RespEvidence.UNMEASURED, RespEvidence.of(Double.NaN, lowBar = 0.5, highBar = 1.0)) + assertNotEquals(RespEvidence.REGULAR, RespEvidence.of(Double.NaN, lowBar = 0.5, highBar = 1.0)) + // A whole session with no respiration channel leaves BOTH percentile bars null (nothing finite to + // take a percentile of). That is the 5/MG case, and it is still UNMEASURED, not REGULAR. + assertEquals(RespEvidence.UNMEASURED, RespEvidence.of(Double.NaN, lowBar = null, highBar = null)) + assertNotEquals(RespEvidence.REGULAR, RespEvidence.of(Double.NaN, lowBar = null, highBar = null)) + assertEquals(RespEvidence.UNMEASURED, RespEvidence.of(Double.POSITIVE_INFINITY, lowBar = 0.5, highBar = 1.0)) + } + + @Test + fun measuredValuesMapToTheirBands() { + assertEquals(RespEvidence.REGULAR, RespEvidence.of(0.2, 0.5, 1.0)) // below the low bar + assertEquals(RespEvidence.REGULAR, RespEvidence.of(0.5, 0.5, 1.0)) // inclusive + assertEquals(RespEvidence.IRREGULAR, RespEvidence.of(1.0, 0.5, 1.0)) // inclusive + assertEquals(RespEvidence.IRREGULAR, RespEvidence.of(2.0, 0.5, 1.0)) + assertEquals(RespEvidence.MEASURED_MID_BAND, RespEvidence.of(0.75, 0.5, 1.0)) + } + + /** + * MEASURED_MID_BAND and UNMEASURED both fail both bars but mean opposite things, and the classifier + * treats them differently — collapsing them into one "unknown" would silently change the hypnogram. + */ + @Test + fun midBandAndUnmeasuredAreDistinctAndBehaveDifferently() { + assertNotEquals(RespEvidence.MEASURED_MID_BAND, RespEvidence.UNMEASURED) + assertTrue(RespEvidence.MEASURED_MID_BAND.contradictsDepth) + assertFalse(RespEvidence.UNMEASURED.contradictsDepth) + assertFalse(RespEvidence.REGULAR.contradictsDepth) + assertTrue(RespEvidence.IRREGULAR.contradictsDepth) + // A finite RRV with no bars is still a measurement, so it must not fall into the unmeasured branch. + assertEquals(RespEvidence.MEASURED_MID_BAND, RespEvidence.of(0.7, null, null)) + } + + /** + * The waiver is now explicit and NAMED rather than emergent from a `!isFinite()` short-circuit — but + * it is still a waiver, and this pins that it is. Removing it would decode 0 m of deep on every 5/MG + * night (the #127/#129 regression, for the parallel missing-RMSSD case). + */ + @Test + fun unmeasuredRespirationIsWaivedForDeepAndTheWaiverIsExplicit() { + assertEquals("deep", classify(feature(0.0, hr = 50.0, hrVar = 0.0, rmssd = 60.0, rrv = Double.NaN))) + assertFalse(RespEvidence.UNMEASURED.contradictsDepth) + } + + // ── The correction changed no label ────────────────────────────────────────────────────────────── + + /** + * Exhaustive grid: the new RespEvidence classifier must agree with the OLD boolean predicates on every + * combination. The old formulas are reproduced verbatim so the equivalence is checkable rather than + * asserted — this is what makes the change a representation fix and not a scoring change. + */ + @Test + fun classificationIsIdenticalToThePreFixPredicates() { + fun legacy( + f: SleepStager.EpochFeatures, hrLo: Double?, hrHi: Double?, rmssdHi: Double?, + hrvarHi: Double?, rrvHi: Double?, rrvLo: Double?, cardiacSparse: Boolean, + ): String { + val hasHR = f.hr.isFinite() + val hrLow = hasHR && hrLo != null && f.hr <= hrLo + val hrHigh = hasHR && hrHi != null && f.hr >= hrHi + val parasympOK = (!f.rmssd.isFinite()) || (rmssdHi != null && f.rmssd >= rmssdHi) + val hrvarHigh = f.hrVar.isFinite() && hrvarHi != null && f.hrVar >= hrvarHi + val cardiacActivated = hrHigh || hrvarHigh + val cardiacActivatedForWake = if (cardiacSparse) hrHigh else cardiacActivated + // The two predicates this change replaced, exactly as they were. + val rrvIrregular = f.rrv.isFinite() && rrvHi != null && f.rrv >= rrvHi + val rrvRegular = (!f.rrv.isFinite()) || (rrvLo != null && f.rrv <= rrvLo) + val still = f.moveFrac <= SleepStager.stageStillMoveFrac + val moving = f.moveFrac >= SleepStager.stageWakeMoveFrac + if (moving && (cardiacActivatedForWake || !hasHR)) return "wake" + if (still && parasympOK && hrLow && rrvRegular) return "deep" + if (still && cardiacActivated && rrvIrregular) return "rem" + if (still && hrHigh && hrvarHigh && !f.rrv.isFinite()) return "rem" + return "light" + } + + var checked = 0 + for (moveFrac in listOf(0.0, 0.05, 0.12, 0.2)) { + for (hr in listOf(Double.NaN, 45.0, 60.0, 95.0)) { + for (hrVar in listOf(Double.NaN, 1.0, 20.0)) { + for (rmssd in listOf(Double.NaN, 10.0, 80.0)) { + // NaN (never measured), below/at/between/at/above the bars. + for (rrv in listOf(Double.NaN, 0.2, 0.5, 0.75, 1.0, 2.0)) { + for (sparse in listOf(false, true)) { + val f = feature(moveFrac, hr, hrVar, rmssd, rrv) + val new = SleepStager.classifyOne( + f, hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, + rrvHi = 1.0, rrvLo = 0.5, cardiacSparse = sparse, + ) + val old = legacy(f, 55.0, 90.0, 50.0, 10.0, 1.0, 0.5, sparse) + assertEquals( + "label changed for move=$moveFrac hr=$hr hrVar=$hrVar " + + "rmssd=$rmssd rrv=$rrv sparse=$sparse", + old, new, + ) + checked++ + } + } + } + } + } + } + assertEquals(4 * 4 * 3 * 3 * 6 * 2, checked) + } + + /** + * The same equivalence for a session with NO respiration channel at all (both bars null) — the WHOOP + * 5/MG shape, and the one the bias actually lives on. + */ + @Test + fun noRespChannelSessionIsAlsoUnchanged() { + for (moveFrac in listOf(0.0, 0.12, 0.2)) { + for (hr in listOf(Double.NaN, 45.0, 95.0)) { + for (hrVar in listOf(Double.NaN, 1.0, 20.0)) { + for (rmssd in listOf(Double.NaN, 10.0, 80.0)) { + val f = feature(moveFrac, hr, hrVar, rmssd, Double.NaN) + val new = SleepStager.classifyOne( + f, hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, + rrvHi = null, rrvLo = null, + ) + // Old formulas with null bars: rrvIrregular = false, rrvRegular = true (the bug). + val hasHR = f.hr.isFinite() + val hrLow = hasHR && f.hr <= 55.0 + val hrHigh = hasHR && f.hr >= 90.0 + val parasympOK = (!f.rmssd.isFinite()) || f.rmssd >= 50.0 + val hrvarHigh = f.hrVar.isFinite() && f.hrVar >= 10.0 + val still = f.moveFrac <= SleepStager.stageStillMoveFrac + val moving = f.moveFrac >= SleepStager.stageWakeMoveFrac + val old = when { + moving && ((hrHigh || hrvarHigh) || !hasHR) -> "wake" + still && parasympOK && hrLow -> "deep" + still && hrHigh && hrvarHigh -> "rem" + else -> "light" + } + assertEquals(old, new) + } + } + } + } + } + + /** + * `remRejectReason` must stay in lockstep with `classifyOne` — they were hand-duplicated predicates + * and now share one factory, so this guards the seam. + */ + @Test + fun remRejectReasonAgreesWithTheClassifier() { + for (rrv in listOf(Double.NaN, 0.2, 0.75, 2.0)) { + for (hr in listOf(Double.NaN, 45.0, 95.0)) { + for (hrVar in listOf(Double.NaN, 1.0, 20.0)) { + for (moveFrac in listOf(0.0, 0.2)) { + val f = feature(moveFrac, hr, hrVar, Double.NaN, rrv) + val label = classify(f) + val reason = SleepStager.remRejectReason( + f, hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, + rrvHi = 1.0, rrvLo = 0.5, + ) + assertEquals( + "rem-eligibility disagreed for rrv=$rrv hr=$hr", + label == "rem", reason == SleepStager.REMRejectReason.REM_ELIGIBLE, + ) + } + } + } + } + } + + /** + * A measured-but-mid-band respiration must NOT earn the missing-respiration REM fallback — that + * fallback exists to compensate for having no reading, not for having an unremarkable one. + */ + @Test + fun midBandDoesNotEarnTheMissingRespirationRemFallback() { + // still + hrHigh + hrvarHigh: the fallback's exact preconditions. + assertEquals("rem", classify(feature(0.0, hr = 95.0, hrVar = 20.0, rmssd = Double.NaN, rrv = Double.NaN))) + assertEquals("light", classify(feature(0.0, hr = 95.0, hrVar = 20.0, rmssd = Double.NaN, rrv = 0.75))) + } + + // ── The size of the remaining bias ─────────────────────────────────────────────────────────────── + + /** + * QUANTIFIED, so the next person to touch the deep gate argues with a number. + * + * The "regular" bar is `stageRRVLowPct` = 50 — the MEDIAN. So on a night with real respiration data + * about half of otherwise-depth-shaped epochs clear it. On a session with no respiration channel the + * old code cleared it for ALL of them, on no measurement at all. That factor-of-two is the bias. + * + * It is bounded, though, and the bound matters as much as the bias: `hrLow` is itself a percentile bar + * (`stageHRLowPct` = 25), so at most ~25% of sleep epochs can reach the deep gate however the + * respiration term resolves. The bias inflates deep within that ceiling; it cannot run away. + */ + @Test + fun theDeepPassRateDoublesWhenTheRespirationChannelIsAbsent() { + val rrvs = (0 until 100).map { it / 100.0 } // 0.00 … 0.99, median 0.5 + val deepWithResp = rrvs.count { + classify(feature(0.0, hr = 50.0, hrVar = 0.0, rmssd = 60.0, rrv = it)) == "deep" + } + assertEquals("the regular bar is the median, so ~half of epochs pass", 51, deepWithResp) + + val deepWithoutResp = rrvs.count { + SleepStager.classifyOne( + feature(0.0, hr = 50.0, hrVar = 0.0, rmssd = 60.0, rrv = Double.NaN), + hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, rrvHi = null, rrvLo = null, + ) == "deep" + } + assertEquals( + "with no respiration channel EVERY depth-shaped epoch passes the deep gate", + 100, deepWithoutResp, + ) + assertEquals(1.96, 100.0 / deepWithResp, 0.05) + } + + /** + * The RRV source itself: with no respiration samples there is nothing to measure, so the NaN is honest + * at the point it is produced. The bug was never here — it was in spending that NaN as evidence + * downstream. (No Kotlin test covered `respRateAndRRV` at all before this.) + */ + @Test + fun rrvIsNaNWhenThereAreNoRespirationSamples() { + assertTrue(SleepStager.respRateAndRRV(emptyList()).second.isNaN()) + assertTrue(SleepStager.respRateAndRRV(listOf(1.0, 2.0, 3.0)).second.isNaN()) + } + + // ── V2 (the DEFAULT stager) accepts `resp` and never reads it ──────────────────────────────────── + + private fun stillGravity(start: Long, durationS: Int): List = + (0 until durationS).map { GravitySample(deviceId = dev, ts = start + it, x = 0.0, y = 0.0, z = 1.0) } + + private fun sleepHR(start: Long, durationS: Int): List = + (0 until durationS).map { HrSample(deviceId = dev, ts = start + it, bpm = 52 + ((it / 60) % 3)) } + + private fun regularRR(start: Long, durationS: Int): List = + (0 until durationS).map { i -> + RrInterval(deviceId = dev, ts = start + i, rrMs = 1000 + (40.0 * sin(2.0 * PI * i / 4.0)).roundToInt()) + } + + /** + * A populated `resp` stream must not move a single segment. If this ever fails, V2 started consuming + * respiration and the "signature-parity only" doc — and V2's cache key, which deliberately omits + * `resp` — are both wrong. It also matters for the bug above: on the shipped default path the raw + * respiration ADC is not consulted at all, so the V1 pro-deep bias reaches only users who have turned + * the V2 experiment off. + */ + @Test + fun v2OutputIsIdenticalWithAndWithoutARespirationStream() { + val start = refMidnight + 3_600L + val dur = 90 * 60 + val grav = stillGravity(start, dur) + val hr = sleepHR(start, dur) + val rr = regularRR(start, dur) + // A resp stream with real structure, not a constant — a consumer would produce different RRVs. + val resp = (0 until dur).map { i -> + RespSample(deviceId = dev, ts = start + i, raw = 1000 + (200.0 * sin(2.0 * PI * i / 4.0)).roundToInt()) + } + + val without = SleepStagerV2.stageSession(start, start + dur, grav, hr, rr, emptyList()) + val with = SleepStagerV2.stageSession(start, start + dur, grav, hr, rr, resp) + assertEquals( + "V2 must ignore `resp` — it recovers respiration regularity from R-R (RSA) instead", + without.map { "${it.start}-${it.end}-${it.stage}" }, + with.map { "${it.start}-${it.end}-${it.stage}" }, + ) + assertFalse("the fixture must actually produce segments", without.isEmpty()) + } +} From 99037e35747537880ffb27920a4ab65a12d409bb Mon Sep 17 00:00:00 2001 From: vishk23 <119831996+vishk23@users.noreply.github.com> Date: Sun, 26 Jul 2026 22:57:36 -0400 Subject: [PATCH 2/2] fix(sleep): make coincident RRV bars an explicit RespEvidence case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch on the equivalence grid: it pinned `rrvLo: 0.5, rrvHi: 1.0` throughout, so it could never reach the combination where the two pre-fix booleans were BOTH true. `rrvRegular` needed `rrv <= rrvLo` and `rrvIrregular` needed `rrv >= rrvHi`, which co-occur exactly when the bars coincide on the reading. The four-state enum could not hold that, and `of` resolved it by testing the high bar first — so those epochs became `.irregular`, `contradictsDepth` fired, and the label moved. It is a real flip, not a theoretical one: with the bar pair added to the grid, deep -> light where the epoch has no cardiac activation and deep -> rem where `hrvarHigh` fires. Coincident bars are reachable for a structural reason. RRV is the population std of breath intervals measured in whole seconds (`respRateAndRRV`, dtS = 1), so it is quantised onto a small discrete lattice and exact ties between epochs are ordinary; `percentile` interpolates between order statistics, so p50 and p65 land on the same value whenever a tie run spans them. And when only one sleep epoch has a finite RRV — that same function returns NaN freely on short, flat or low-peak windows — `percentile` returns the single value for every percentile, so the epoch that set the bars sits on both by construction. Adds `barsDegenerate` / `BARS_DEGENERATE` as the fifth case, and rewrites `of` as the total cross-product of the two pre-fix predicates rather than an ordered `if` chain, since an ordered chain is how this case was being decided by accident. The REM rule now reads `meetsIrregularBar` instead of comparing to `.irregular`, so the both-bars-cleared state keeps both of its pre-fix answers: waived for depth and clearing the irregular bar. Behaviour is pinned to the pre-fix outcome rather than re-decided. That outcome resolves to DEEP only because the deep rule is written before the REM rule, and statement order is not a reason — but re-deciding it is a scoring change on the degenerate-distribution nights, and there are no staged nights of that shape to validate it against. Naming the case makes it a one-line edit with a failing test instead of an invisible consequence of ordering. Tests: the bar pair is now an axis on both equivalence grids (separated, coincident, coincident elsewhere, inverted, and each way of having a null bar), 1728 -> 12096 cases, with an assertion that the grid actually visits the both-true cell. `remRejectReason` gets the same axis, since it reads `meetsIrregularBar` too. Adds a reachability test covering both routes to coincident bars and the end-to-end path through `classifyEpochs`. Also corrects the Swift doc header, which said THREE states and referred to a case called `unknown`; there are five and it is `unmeasured`. Swift + Kotlin twins, same five-state model, same tests both sides. --- .../Sources/StrandAnalytics/SleepStager.swift | 83 ++++++-- .../SleepStagerRespEvidenceTests.swift | 162 ++++++++++++--- .../java/com/noop/analytics/SleepStager.kt | 82 +++++++- .../analytics/SleepStagerRespEvidenceTest.kt | 188 ++++++++++++++---- 4 files changed, 428 insertions(+), 87 deletions(-) diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift index cd94dd657a..6c583da65c 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift @@ -1692,7 +1692,7 @@ public enum SleepStager { return Double(sparse) >= cardiacSparseEpochFrac * Double(sleepFeats.count) } - /// What this epoch's respiration says about depth — THREE states, because "we did not measure it" is + /// What this epoch's respiration says about depth — FIVE states, because "we did not measure it" is /// not an observation and must not be spendable as one. /// /// This used to be two booleans over a NaN RRV, and the `regular` one read @@ -1704,8 +1704,17 @@ public enum SleepStager { /// its epochs clear the `regular` bar (it is the MEDIAN — `stageRRVLowPct` = 50), a 5/MG night has /// 100% clear it, on no measurement whatsoever. /// - /// `unknown` is now its own case and every branch below states what it does with it, rather than the - /// answer falling out of a `!isFinite` short-circuit nobody could see. + /// The five cases are the exact CROSS-PRODUCT of the two predicates this replaced, so the mapping is + /// total and no case is decided by which bar `of` happens to test first: + /// + /// | pre-fix `rrvIrregular` | pre-fix `rrvRegular` | case | + /// |---|---|---| + /// | false | true (via `!isFinite`) | `unmeasured` | + /// | false | true (via the low bar) | `regular` | + /// | true | false | `irregular` | + /// | false | false | `measuredMidBand` | + /// | true | true | `barsDegenerate` | + /// /// `unmeasured` and `measuredMidBand` are deliberately DISTINCT. Both fail the regular and irregular /// bars, but they mean opposite things — one is "no reading", the other is a real reading that simply /// sits between the bars — and the classifier already treats them differently (the REM fallback fires @@ -1721,12 +1730,46 @@ public enum SleepStager { /// or this epoch's window had too few / too flat samples to derive one (`respRateAndRRV` returns /// NaN below 8 samples, on a flat signal, under 3 peaks, or under 2 in-band breath intervals). case unmeasured - + /// A finite RRV that clears BOTH bars at once — at or above the high bar AND at or below the low + /// bar. The session's two percentile bars have collapsed onto this reading, so it satisfies each + /// of them and the pair carries no information at this value. + /// + /// Reachable, not hypothetical, and for a structural reason: RRV is the population std of breath + /// intervals measured in WHOLE SECONDS (`respRateAndRRV`, `dtS` = 1), so it is quantised onto a + /// small discrete lattice and exact ties between epochs are ordinary. `percentile` interpolates + /// between order statistics, so p50 and p65 coincide whenever the tie run spans them — and when + /// only ONE sleep epoch has a finite RRV (the rest NaN, which that same function returns freely on + /// short, flat or low-peak windows) `percentile` returns that single value for both, so the epoch + /// that SET the bars necessarily sits on both of them. + /// + /// Kept as its own case because a four-state enum cannot represent it: both pre-fix booleans were + /// true here, so whichever bar the factory tested first would silently decide the label. Behaviour + /// is pinned to the pre-fix outcome — waived for depth (`rrvRegular` was true, see + /// `contradictsDepth`) and clearing the REM irregular bar (`rrvIrregular` was true, see + /// `meetsIrregularBar`) — which keeps this change a representation fix. + /// + /// That pre-fix outcome resolves to DEEP only because the deep rule is written before the REM rule + /// in `classifyOne`; statement order is not a reason, and re-deciding it is a live question. It is + /// deliberately NOT re-decided here: doing so is a scoring change on the degenerate-distribution + /// nights, and this repo has no staged nights of that shape to validate it against (CLAUDE.md, + /// "validate against the artifact, not one match"). The point of naming the case is that the + /// decision is now a one-line edit in `contradictsDepth` / `meetsIrregularBar` with a test that + /// fails loudly, instead of an invisible consequence of statement order. + case barsDegenerate + + /// Total mapping of the two pre-fix predicates. Written as an exhaustive switch on the pair + /// rather than an ordered `if` chain, because an ordered chain is exactly how the `barsDegenerate` + /// case used to be decided by accident. static func of(_ rrv: Double, lowBar: Double?, highBar: Double?) -> RespEvidence { guard rrv.isFinite else { return .unmeasured } - if let hi = highBar, rrv >= hi { return .irregular } - if let lo = lowBar, rrv <= lo { return .regular } - return .measuredMidBand + let atOrAboveHigh = highBar.map { rrv >= $0 } ?? false + let atOrBelowLow = lowBar.map { rrv <= $0 } ?? false + switch (atOrAboveHigh, atOrBelowLow) { + case (true, true): return .barsDegenerate + case (true, false): return .irregular + case (false, true): return .regular + case (false, false): return .measuredMidBand + } } /// Whether respiration CONTRADICTS depth. The deep rule reads this rather than a "regular" flag, @@ -1739,6 +1782,9 @@ public enum SleepStager { /// qualifier ("with high parasympathetic tone WHEN MEASURABLE"). This makes the respiration /// waiver explicit and equally qualified instead of leaving it implied by a NaN short-circuit. /// + /// `barsDegenerate` is waived too, matching the pre-fix `rrvRegular`; see its doc for why that is + /// preserved rather than re-decided. + /// /// KNOWN LIMITATION, deliberately not changed here: on a 5/MG BOTH waivers can fire at once /// (sparse R-R leaves RMSSD NaN too), and the deep rule then reduces to stillness + a low HR with /// no physiological corroboration at all. That is a real weakness, but it is bounded — `hrLow` is @@ -1750,10 +1796,21 @@ public enum SleepStager { /// quantifies the bias so that decision can be made on numbers. var contradictsDepth: Bool { switch self { - case .regular, .unmeasured: return false + case .regular, .unmeasured, .barsDegenerate: return false case .irregular, .measuredMidBand: return true } } + + /// Whether this epoch clears the session's IRREGULAR bar — the pre-fix `rrvIrregular` predicate, + /// which the main REM rule reads. `barsDegenerate` clears it (it IS at or above the high bar) even + /// though `contradictsDepth` also waives it; that pair of answers is precisely the pre-fix state + /// where both booleans were true, and it is preserved deliberately. + var meetsIrregularBar: Bool { + switch self { + case .irregular, .barsDegenerate: return true + case .regular, .measuredMidBand, .unmeasured: return false + } + } } static func classifyOne(_ f: EpochFeatures, hrLo: Double?, hrHi: Double?, @@ -1794,8 +1851,8 @@ public enum SleepStager { // when measurable. `contradictsDepth` is where an unmeasured respiration is waived — see its doc // for why the waiver stays and what it costs. if still && parasympOK && hrLow && !resp.contradictsDepth { return "deep" } - // REM: still body + activated cardiac + respiration measured as irregular. - if still && cardiacActivated && resp == .irregular { return "rem" } + // REM: still body + activated cardiac + respiration clearing the irregular bar. + if still && cardiacActivated && resp.meetsIrregularBar { return "rem" } // REM fallback when respiration was never MEASURED (not merely mid-band): require BOTH cardiac // signals. A mid-band reading is real evidence and does not earn the fallback. if still && hrHigh && hrvarHigh && resp == .unmeasured { return "rem" } @@ -1930,8 +1987,8 @@ public enum SleepStager { let hrvarHigh = f.hrVar.isFinite && hrvarHi != nil && f.hrVar >= hrvarHi! let cardiacActivated = hrHigh || hrvarHigh let cardiacActivatedForWake = cardiacSparse ? hrHigh : cardiacActivated - // Same three-state respiration evidence the classifier uses, from the same factory — so the - // diagnostic cannot drift from the rule it explains (these predicates were duplicated by hand). + // Same respiration evidence the classifier uses, from the same factory — so the diagnostic cannot + // drift from the rule it explains (these predicates were duplicated by hand). let resp = RespEvidence.of(f.rrv, lowBar: rrvLo, highBar: rrvHi) let still = f.moveFrac <= stageStillMoveFrac let moving = f.moveFrac >= stageWakeMoveFrac @@ -1941,7 +1998,7 @@ public enum SleepStager { if moving && (cardiacActivatedForWake || !hasHR) { return .wonOtherStage } // → wake if still && parasympOK && hrLow && !resp.contradictsDepth { return .wonOtherStage } // → deep // From here the epoch did NOT win wake/deep; it is either REM or falls through to LIGHT. - if still && cardiacActivated && resp == .irregular { return .remEligible } + if still && cardiacActivated && resp.meetsIrregularBar { return .remEligible } if still && hrHigh && hrvarHigh && resp == .unmeasured { return .remEligible } // Not REM → attribute to the FIRST unmet REM precondition (in REM-rule order). if !still { return .notStill } diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStagerRespEvidenceTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStagerRespEvidenceTests.swift index 6bfe857c81..fff28ddae0 100644 --- a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStagerRespEvidenceTests.swift +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SleepStagerRespEvidenceTests.swift @@ -35,6 +35,70 @@ final class SleepStagerRespEvidenceTests: XCTestCase { XCTAssertEqual(Resp.of(1.0, lowBar: 0.5, highBar: 1.0), .irregular) // inclusive XCTAssertEqual(Resp.of(2.0, lowBar: 0.5, highBar: 1.0), .irregular) XCTAssertEqual(Resp.of(0.75, lowBar: 0.5, highBar: 1.0), .measuredMidBand) + // Both bars on the same value: the reading clears each of them. + XCTAssertEqual(Resp.of(0.75, lowBar: 0.75, highBar: 0.75), .barsDegenerate) + // …and only ON that value. Either side of it the pair still separates normally. + XCTAssertEqual(Resp.of(0.74, lowBar: 0.75, highBar: 0.75), .regular) + XCTAssertEqual(Resp.of(0.76, lowBar: 0.75, highBar: 0.75), .irregular) + } + + /// The five cases are the CROSS-PRODUCT of the two predicates this replaced, so every combination + /// including "both true" has a home and none is decided by which bar `of` tests first. + func testEveryCaseIsOneCellOfThePreFixBooleanPair() { + // (rrvIrregular, rrvRegular) → case + XCTAssertEqual(Resp.of(.nan, lowBar: 0.5, highBar: 1.0), .unmeasured) // (false, true) + XCTAssertEqual(Resp.of(0.2, lowBar: 0.5, highBar: 1.0), .regular) // (false, true) + XCTAssertEqual(Resp.of(2.0, lowBar: 0.5, highBar: 1.0), .irregular) // (true, false) + XCTAssertEqual(Resp.of(0.75, lowBar: 0.5, highBar: 1.0), .measuredMidBand) // (false, false) + XCTAssertEqual(Resp.of(0.75, lowBar: 0.75, highBar: 0.75), .barsDegenerate) // (true, true) + // The two readouts the classifier consumes, for the cell a four-state enum could not hold. + XCTAssertFalse(Resp.barsDegenerate.contradictsDepth, "pre-fix `rrvRegular` was true here") + XCTAssertTrue(Resp.barsDegenerate.meetsIrregularBar, "pre-fix `rrvIrregular` was true here") + } + + /// Coincident bars are REACHABLE, not a theoretical corner — which is why the case is preserved + /// rather than waved away. + /// + /// Two independent routes. (1) RRV is the population std of breath intervals measured in WHOLE + /// SECONDS (`respRateAndRRV`, `dtS` = 1), so it is quantised onto a small discrete lattice and exact + /// ties between epochs are ordinary; `percentile` interpolates between order statistics, so p50 and + /// p65 coincide whenever a tie run spans them. (2) With exactly ONE finite RRV in the sleep period — + /// `respRateAndRRV` returns NaN freely, on short, flat or low-peak windows — `percentile` returns + /// that single value for EVERY percentile, so the bars coincide by construction and the epoch that + /// set them necessarily sits on both. + /// + /// Both labels below are the pre-fix answers. Testing the high bar before the low bar would have + /// classified the first as `irregular` and flipped it deep → light. + func testCoincidentBarsAreReachableAndPreserveThePreFixLabels() { + // Route (2), through the same `percentile` the stager uses. + let sessionRrvs: [Double] = [.nan, .nan, 0.75, .nan, .nan] + let lo = SleepStager.percentile(sessionRrvs, SleepStager.stageRRVLowPct) + let hi = SleepStager.percentile(sessionRrvs, SleepStager.stageRRVHighPct) + XCTAssertEqual(lo, 0.75) + XCTAssertEqual(hi, 0.75, "one finite RRV in the session puts both bars on the same value") + XCTAssertEqual(Resp.of(0.75, lowBar: lo, highBar: hi), .barsDegenerate) + + // Route (1): a tie run spanning p50…p65 does it too, with several finite values present. + let tied: [Double] = [0.0, 0.25, 0.5, 0.5, 0.5, 0.5, 0.5, 1.0, 1.5, 2.0] + XCTAssertEqual(SleepStager.percentile(tied, SleepStager.stageRRVLowPct), + SleepStager.percentile(tied, SleepStager.stageRRVHighPct)) + + // Depth-shaped, no cardiac activation → pre-fix "deep" (the deep rule is stated first). + let depthShaped = feature(moveFrac: 0, hr: 50, hrVar: 0, rmssd: 60, rrv: 0.75) + XCTAssertEqual(SleepStager.classifyOne(depthShaped, hrLo: 55, hrHi: 90, rmssdHi: 50, + hrvarHi: 10, rrvHi: 0.75, rrvLo: 0.75), "deep") + // Cardiac-activated, not depth-shaped → the REM rule still sees the irregular bar cleared. + let remShaped = feature(moveFrac: 0, hr: 95, hrVar: 20, rmssd: 10, rrv: 0.75) + XCTAssertEqual(SleepStager.classifyOne(remShaped, hrLo: 55, hrHi: 90, rmssdHi: 50, + hrvarHi: 10, rrvHi: 0.75, rrvLo: 0.75), "rem") + + // End to end, letting `classifyEpochs` compute the bars itself from the session above. + let feats = sessionRrvs.enumerated().map { i, rrv in + SleepStager.EpochFeatures(index: i, midTs: Double(i) * 30, count: 0, moveFrac: 0, + ckSleep: true, hr: 50, hrVar: .nan, rmssd: 60, sdnn: 0, + respRate: 14, rrv: rrv, clock: 0.5) + } + XCTAssertEqual(SleepStager.classifyEpochs(feats), Array(repeating: "deep", count: 5)) } /// `measuredMidBand` and `unmeasured` both fail both bars but mean opposite things, and the classifier @@ -67,6 +131,11 @@ final class SleepStagerRespEvidenceTests: XCTestCase { /// Exhaustive grid: the new `RespEvidence` classifier must agree with the OLD boolean predicates on /// every combination. The old formulas are reproduced verbatim here so the equivalence is checkable /// rather than asserted — this is what makes the change a representation fix and not a scoring change. + /// + /// The BAR PAIR is an axis, not a constant. Holding it at a well-separated `(0.5, 1.0)` hides the one + /// combination where the two pre-fix booleans were BOTH true — `rrvHi <= rrv <= rrvLo`, which the + /// coincident-bar pairs below reach — and hides the nil bars entirely. Those cells are where a + /// four-state enum would have silently changed a label. func testClassificationIsIdenticalToThePreFixPredicates() { func legacy(_ f: SleepStager.EpochFeatures, hrLo: Double?, hrHi: Double?, rmssdHi: Double?, hrvarHi: Double?, @@ -90,32 +159,53 @@ final class SleepStagerRespEvidenceTests: XCTestCase { return "light" } + // (low, high). Separated; coincident on a value the RRV axis hits; coincident elsewhere; + // inverted (defensive — `of` reads the pair, it does not assume low <= high); and each way of + // having no bar at all, up to the 5/MG session where neither exists. + let barPairs: [(lo: Double?, hi: Double?)] = [ + (0.5, 1.0), (0.75, 0.75), (0.5, 0.5), (1.0, 0.5), (nil, 1.0), (0.5, nil), (nil, nil), + ] + var checked = 0 - for moveFrac in [0.0, 0.05, 0.12, 0.2] { - for hr in [Double.nan, 45, 60, 95] { - for hrVar in [Double.nan, 1, 20] { - for rmssd in [Double.nan, 10, 80] { - // NaN (never measured), below/at/between/at/above the bars. - for rrv in [Double.nan, 0.2, 0.5, 0.75, 1.0, 2.0] { - for sparse in [false, true] { - let f = feature(moveFrac: moveFrac, hr: hr, hrVar: hrVar, - rmssd: rmssd, rrv: rrv) - let new = SleepStager.classifyOne( - f, hrLo: 55, hrHi: 90, rmssdHi: 50, hrvarHi: 10, - rrvHi: 1.0, rrvLo: 0.5, cardiacSparse: sparse) - let old = legacy(f, hrLo: 55, hrHi: 90, rmssdHi: 50, hrvarHi: 10, - rrvHi: 1.0, rrvLo: 0.5, cardiacSparse: sparse) - XCTAssertEqual(new, old, - "label changed for move=\(moveFrac) hr=\(hr) hrVar=\(hrVar) " + - "rmssd=\(rmssd) rrv=\(rrv) sparse=\(sparse)") - checked += 1 + var sawBothPreFixBooleansTrue = false + for bars in barPairs { + for moveFrac in [0.0, 0.05, 0.12, 0.2] { + for hr in [Double.nan, 45, 60, 95] { + for hrVar in [Double.nan, 1, 20] { + for rmssd in [Double.nan, 10, 80] { + // NaN (never measured), below/at/between/at/above the bars. + for rrv in [Double.nan, 0.2, 0.5, 0.75, 1.0, 2.0] { + for sparse in [false, true] { + let f = feature(moveFrac: moveFrac, hr: hr, hrVar: hrVar, + rmssd: rmssd, rrv: rrv) + let new = SleepStager.classifyOne( + f, hrLo: 55, hrHi: 90, rmssdHi: 50, hrvarHi: 10, + rrvHi: bars.hi, rrvLo: bars.lo, cardiacSparse: sparse) + let old = legacy(f, hrLo: 55, hrHi: 90, rmssdHi: 50, hrvarHi: 10, + rrvHi: bars.hi, rrvLo: bars.lo, + cardiacSparse: sparse) + XCTAssertEqual(new, old, + "label changed for move=\(moveFrac) hr=\(hr) hrVar=\(hrVar) " + + "rmssd=\(rmssd) rrv=\(rrv) sparse=\(sparse) " + + "bars=(\(String(describing: bars.lo)), " + + "\(String(describing: bars.hi)))") + if rrv.isFinite, + let lo = bars.lo, let hi = bars.hi, rrv >= hi, rrv <= lo { + sawBothPreFixBooleansTrue = true + } + checked += 1 + } } } } } } } - XCTAssertEqual(checked, 4 * 4 * 3 * 3 * 6 * 2) + XCTAssertEqual(checked, 7 * 4 * 4 * 3 * 3 * 6 * 2) + // The grid is only worth more than the old one if it actually visits the cell that motivated it. + XCTAssertTrue(sawBothPreFixBooleansTrue, + "the grid must reach `rrvIrregular && rrvRegular` — otherwise the coincident-bar " + + "case is still untested and a bar-pair axis was added for nothing") } /// The same equivalence for the session with NO respiration channel at all (both bars nil) — the @@ -149,19 +239,29 @@ final class SleepStagerRespEvidenceTests: XCTestCase { } /// `remRejectReason` must stay in lockstep with `classifyOne` — they were hand-duplicated predicates - /// and now share one factory, so this guards the seam. + /// and now share one factory, so this guards the seam. The bar pair is an axis here too: the + /// diagnostic reads `meetsIrregularBar`, so the coincident-bar case has to be exercised on both + /// sides of the seam or only one of them is pinned. func testRemRejectReasonAgreesWithTheClassifier() { - for rrv in [Double.nan, 0.2, 0.75, 2.0] { - for hr in [Double.nan, 45, 95] { - for hrVar in [Double.nan, 1, 20] { - for moveFrac in [0.0, 0.2] { - let f = feature(moveFrac: moveFrac, hr: hr, hrVar: hrVar, rmssd: .nan, rrv: rrv) - let label = SleepStager.classifyOne(f, hrLo: 55, hrHi: 90, rmssdHi: 50, - hrvarHi: 10, rrvHi: 1.0, rrvLo: 0.5) - let reason = SleepStager.remRejectReason(f, hrLo: 55, hrHi: 90, rmssdHi: 50, - hrvarHi: 10, rrvHi: 1.0, rrvLo: 0.5) - XCTAssertEqual(label == "rem", reason == .remEligible, - "rem-eligibility disagreed for rrv=\(rrv) hr=\(hr)") + let barPairs: [(lo: Double?, hi: Double?)] = [(0.5, 1.0), (0.75, 0.75), (nil, nil)] + for bars in barPairs { + for rrv in [Double.nan, 0.2, 0.75, 2.0] { + for hr in [Double.nan, 45, 95] { + for hrVar in [Double.nan, 1, 20] { + for moveFrac in [0.0, 0.2] { + let f = feature(moveFrac: moveFrac, hr: hr, hrVar: hrVar, + rmssd: .nan, rrv: rrv) + let label = SleepStager.classifyOne(f, hrLo: 55, hrHi: 90, rmssdHi: 50, + hrvarHi: 10, + rrvHi: bars.hi, rrvLo: bars.lo) + let reason = SleepStager.remRejectReason(f, hrLo: 55, hrHi: 90, + rmssdHi: 50, hrvarHi: 10, + rrvHi: bars.hi, rrvLo: bars.lo) + XCTAssertEqual(label == "rem", reason == .remEligible, + "rem-eligibility disagreed for rrv=\(rrv) hr=\(hr) " + + "bars=(\(String(describing: bars.lo)), " + + "\(String(describing: bars.hi)))") + } } } } 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 748e144c7e..92da5a886b 100644 --- a/android/app/src/main/java/com/noop/analytics/SleepStager.kt +++ b/android/app/src/main/java/com/noop/analytics/SleepStager.kt @@ -1868,7 +1868,7 @@ object SleepStager { } /** - * What one epoch's respiration says about depth — FOUR states, because "we did not measure it" is not + * What one epoch's respiration says about depth — FIVE states, because "we did not measure it" is not * an observation and must not be spendable as one. * * This used to be two booleans over a NaN RRV, and the `regular` one read @@ -1880,6 +1880,17 @@ object SleepStager { * its epochs clear the `regular` bar (it is the MEDIAN — `stageRRVLowPct` = 50), a 5/MG night has * 100% clear it, on no measurement whatsoever. * + * The five cases are the exact CROSS-PRODUCT of the two predicates this replaced, so the mapping is + * total and no case is decided by which bar `of` happens to test first: + * + * | pre-fix `rrvIrregular` | pre-fix `rrvRegular` | case | + * |------------------------|--------------------------|---------------------| + * | false | true (via `!isFinite()`) | [UNMEASURED] | + * | false | true (via the low bar) | [REGULAR] | + * | true | false | [IRREGULAR] | + * | false | false | [MEASURED_MID_BAND] | + * | true | true | [BARS_DEGENERATE] | + * * [UNMEASURED] and [MEASURED_MID_BAND] are deliberately DISTINCT. Both fail the regular and irregular * bars, but they mean opposite things — one is "no reading", the other is a real reading that simply * sits between the bars — and the classifier already treats them differently (the REM fallback fires @@ -1903,6 +1914,36 @@ object SleepStager { * NaN below 8 samples, on a flat signal, under 3 peaks, or under 2 in-band breath intervals). */ UNMEASURED, + + /** + * A finite RRV that clears BOTH bars at once — at or above the high bar AND at or below the low + * bar. The session's two percentile bars have collapsed onto this reading, so it satisfies each + * of them and the pair carries no information at this value. + * + * Reachable, not hypothetical, and for a structural reason: RRV is the population std of breath + * intervals measured in WHOLE SECONDS (`respRateAndRRV`, `dtS` = 1), so it is quantised onto a + * small discrete lattice and exact ties between epochs are ordinary. `percentile` interpolates + * between order statistics, so p50 and p65 coincide whenever the tie run spans them — and when + * only ONE sleep epoch has a finite RRV (the rest NaN, which that same function returns freely + * on short, flat or low-peak windows) `percentile` returns that single value for both, so the + * epoch that SET the bars necessarily sits on both of them. + * + * Kept as its own case because a four-state enum cannot represent it: both pre-fix booleans were + * true here, so whichever bar the factory tested first would silently decide the label. + * Behaviour is pinned to the pre-fix outcome — waived for depth (`rrvRegular` was true, see + * [contradictsDepth]) and clearing the REM irregular bar (`rrvIrregular` was true, see + * [meetsIrregularBar]) — which keeps this change a representation fix. + * + * That pre-fix outcome resolves to DEEP only because the deep rule is written before the REM + * rule in [classifyOne]; statement order is not a reason, and re-deciding it is a live question. + * It is deliberately NOT re-decided here: doing so is a scoring change on the + * degenerate-distribution nights, and this repo has no staged nights of that shape to validate + * it against (CLAUDE.md, "validate against the artifact, not one match"). The point of naming + * the case is that the decision is now a one-line edit in [contradictsDepth] / + * [meetsIrregularBar] with a test that fails loudly, instead of an invisible consequence of + * statement order. + */ + BARS_DEGENERATE, ; /** @@ -1916,6 +1957,9 @@ object SleepStager { * qualifier ("with high parasympathetic tone WHEN MEASURABLE"). This makes the respiration waiver * explicit and equally qualified instead of leaving it implied by a NaN short-circuit. * + * [BARS_DEGENERATE] is waived too, matching the pre-fix `rrvRegular`; see its doc for why that is + * preserved rather than re-decided. + * * KNOWN LIMITATION, deliberately not changed here: on a 5/MG BOTH waivers can fire at once * (sparse R-R leaves RMSSD NaN too), and the deep rule then reduces to stillness + a low HR with * no physiological corroboration at all. That is a real weakness, but it is bounded — `hrLow` is @@ -1926,16 +1970,38 @@ object SleepStager { */ val contradictsDepth: Boolean get() = when (this) { - REGULAR, UNMEASURED -> false + REGULAR, UNMEASURED, BARS_DEGENERATE -> false IRREGULAR, MEASURED_MID_BAND -> true } + /** + * Whether this epoch clears the session's IRREGULAR bar — the pre-fix `rrvIrregular` predicate, + * which the main REM rule reads. [BARS_DEGENERATE] clears it (it IS at or above the high bar) + * even though [contradictsDepth] also waives it; that pair of answers is precisely the pre-fix + * state where both booleans were true, and it is preserved deliberately. + */ + val meetsIrregularBar: Boolean + get() = when (this) { + IRREGULAR, BARS_DEGENERATE -> true + REGULAR, MEASURED_MID_BAND, UNMEASURED -> false + } + companion object { + /** + * Total mapping of the two pre-fix predicates. Written as an exhaustive branch on the pair + * rather than an ordered `if` chain, because an ordered chain is exactly how the + * [BARS_DEGENERATE] case used to be decided by accident. + */ fun of(rrv: Double, lowBar: Double?, highBar: Double?): RespEvidence { if (!rrv.isFinite()) return UNMEASURED - if (highBar != null && rrv >= highBar) return IRREGULAR - if (lowBar != null && rrv <= lowBar) return REGULAR - return MEASURED_MID_BAND + val atOrAboveHigh = highBar != null && rrv >= highBar + val atOrBelowLow = lowBar != null && rrv <= lowBar + return when { + atOrAboveHigh && atOrBelowLow -> BARS_DEGENERATE + atOrAboveHigh -> IRREGULAR + atOrBelowLow -> REGULAR + else -> MEASURED_MID_BAND + } } } } @@ -1980,8 +2046,8 @@ object SleepStager { // when measurable. [RespEvidence.contradictsDepth] is where an unmeasured respiration is waived — // see its doc for why the waiver stays and what it costs. if (still && parasympOK && hrLow && !resp.contradictsDepth) return "deep" - // REM: still body + activated cardiac + respiration measured as irregular. - if (still && cardiacActivated && resp == RespEvidence.IRREGULAR) return "rem" + // REM: still body + activated cardiac + respiration clearing the irregular bar. + if (still && cardiacActivated && resp.meetsIrregularBar) return "rem" // REM fallback when respiration was never MEASURED (not merely mid-band): require BOTH cardiac // signals. A mid-band reading is real evidence and does not earn the fallback. if (still && hrHigh && hrvarHigh && resp == RespEvidence.UNMEASURED) return "rem" @@ -2125,7 +2191,7 @@ object SleepStager { if (moving && (cardiacActivatedForWake || !hasHR)) return REMRejectReason.WON_OTHER_STAGE // → wake if (still && parasympOK && hrLow && !resp.contradictsDepth) return REMRejectReason.WON_OTHER_STAGE // → deep // From here the epoch did NOT win wake/deep; it is either REM or falls through to LIGHT. - if (still && cardiacActivated && resp == RespEvidence.IRREGULAR) return REMRejectReason.REM_ELIGIBLE + if (still && cardiacActivated && resp.meetsIrregularBar) return REMRejectReason.REM_ELIGIBLE if (still && hrHigh && hrvarHigh && resp == RespEvidence.UNMEASURED) return REMRejectReason.REM_ELIGIBLE // Not REM → attribute to the FIRST unmet REM precondition (in REM-rule order). if (!still) return REMRejectReason.NOT_STILL diff --git a/android/app/src/test/java/com/noop/analytics/SleepStagerRespEvidenceTest.kt b/android/app/src/test/java/com/noop/analytics/SleepStagerRespEvidenceTest.kt index bc0f594698..ec67d99f9a 100644 --- a/android/app/src/test/java/com/noop/analytics/SleepStagerRespEvidenceTest.kt +++ b/android/app/src/test/java/com/noop/analytics/SleepStagerRespEvidenceTest.kt @@ -63,6 +63,89 @@ class SleepStagerRespEvidenceTest { assertEquals(RespEvidence.IRREGULAR, RespEvidence.of(1.0, 0.5, 1.0)) // inclusive assertEquals(RespEvidence.IRREGULAR, RespEvidence.of(2.0, 0.5, 1.0)) assertEquals(RespEvidence.MEASURED_MID_BAND, RespEvidence.of(0.75, 0.5, 1.0)) + // Both bars on the same value: the reading clears each of them. + assertEquals(RespEvidence.BARS_DEGENERATE, RespEvidence.of(0.75, 0.75, 0.75)) + // …and only ON that value. Either side of it the pair still separates normally. + assertEquals(RespEvidence.REGULAR, RespEvidence.of(0.74, 0.75, 0.75)) + assertEquals(RespEvidence.IRREGULAR, RespEvidence.of(0.76, 0.75, 0.75)) + } + + /** + * The five cases are the CROSS-PRODUCT of the two predicates this replaced, so every combination + * including "both true" has a home and none is decided by which bar `of` tests first. + */ + @Test + fun everyCaseIsOneCellOfThePreFixBooleanPair() { + // (rrvIrregular, rrvRegular) → case + assertEquals(RespEvidence.UNMEASURED, RespEvidence.of(Double.NaN, 0.5, 1.0)) // (false, true) + assertEquals(RespEvidence.REGULAR, RespEvidence.of(0.2, 0.5, 1.0)) // (false, true) + assertEquals(RespEvidence.IRREGULAR, RespEvidence.of(2.0, 0.5, 1.0)) // (true, false) + assertEquals(RespEvidence.MEASURED_MID_BAND, RespEvidence.of(0.75, 0.5, 1.0)) // (false, false) + assertEquals(RespEvidence.BARS_DEGENERATE, RespEvidence.of(0.75, 0.75, 0.75)) // (true, true) + // The two readouts the classifier consumes, for the cell a four-state enum could not hold. + assertFalse("pre-fix `rrvRegular` was true here", RespEvidence.BARS_DEGENERATE.contradictsDepth) + assertTrue("pre-fix `rrvIrregular` was true here", RespEvidence.BARS_DEGENERATE.meetsIrregularBar) + } + + /** + * Coincident bars are REACHABLE, not a theoretical corner — which is why the case is preserved rather + * than waved away. + * + * Two independent routes. (1) RRV is the population std of breath intervals measured in WHOLE SECONDS + * (`respRateAndRRV`, `dtS` = 1), so it is quantised onto a small discrete lattice and exact ties + * between epochs are ordinary; `percentile` interpolates between order statistics, so p50 and p65 + * coincide whenever a tie run spans them. (2) With exactly ONE finite RRV in the sleep period — + * `respRateAndRRV` returns NaN freely, on short, flat or low-peak windows — `percentile` returns that + * single value for EVERY percentile, so the bars coincide by construction and the epoch that set them + * necessarily sits on both. + * + * Both labels below are the pre-fix answers. Testing the high bar before the low bar would have + * classified the first as IRREGULAR and flipped it deep → light. + */ + @Test + fun coincidentBarsAreReachableAndPreserveThePreFixLabels() { + // Route (2), through the same `percentile` the stager uses. + val sessionRrvs = listOf(Double.NaN, Double.NaN, 0.75, Double.NaN, Double.NaN) + val lo = SleepStager.percentile(sessionRrvs, SleepStager.stageRRVLowPct) + val hi = SleepStager.percentile(sessionRrvs, SleepStager.stageRRVHighPct) + assertEquals(0.75, lo!!, 1e-12) + assertEquals("one finite RRV in the session puts both bars on the same value", 0.75, hi!!, 1e-12) + assertEquals(RespEvidence.BARS_DEGENERATE, RespEvidence.of(0.75, lo, hi)) + + // Route (1): a tie run spanning p50…p65 does it too, with several finite values present. + val tied = listOf(0.0, 0.25, 0.5, 0.5, 0.5, 0.5, 0.5, 1.0, 1.5, 2.0) + assertEquals( + SleepStager.percentile(tied, SleepStager.stageRRVLowPct)!!, + SleepStager.percentile(tied, SleepStager.stageRRVHighPct)!!, + 1e-12, + ) + + // Depth-shaped, no cardiac activation → pre-fix "deep" (the deep rule is stated first). + assertEquals( + "deep", + SleepStager.classifyOne( + feature(0.0, hr = 50.0, hrVar = 0.0, rmssd = 60.0, rrv = 0.75), + hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, rrvHi = 0.75, rrvLo = 0.75, + ), + ) + // Cardiac-activated, not depth-shaped → the REM rule still sees the irregular bar cleared. + assertEquals( + "rem", + SleepStager.classifyOne( + feature(0.0, hr = 95.0, hrVar = 20.0, rmssd = 10.0, rrv = 0.75), + hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, rrvHi = 0.75, rrvLo = 0.75, + ), + ) + + // End to end, letting `classifyEpochs` compute the bars itself from the session above. + val feats = sessionRrvs.mapIndexed { i, rrv -> + SleepStager.EpochFeatures( + index = i, midTs = i * 30.0, count = 0.0, moveFrac = 0.0, ckSleep = true, + hr = 50.0, hrVar = Double.NaN, rmssd = 60.0, sdnn = 0.0, respRate = 14.0, + rrv = rrv, clock = 0.5, + ) + } + assertEquals(List(5) { "deep" }, SleepStager.classifyEpochs(feats)) } /** @@ -97,6 +180,11 @@ class SleepStagerRespEvidenceTest { * Exhaustive grid: the new RespEvidence classifier must agree with the OLD boolean predicates on every * combination. The old formulas are reproduced verbatim so the equivalence is checkable rather than * asserted — this is what makes the change a representation fix and not a scoring change. + * + * The BAR PAIR is an axis, not a constant. Holding it at a well-separated (0.5, 1.0) hides the one + * combination where the two pre-fix booleans were BOTH true — `rrvHi <= rrv <= rrvLo`, which the + * coincident-bar pairs below reach — and hides the null bars entirely. Those cells are where a + * four-state enum would have silently changed a label. */ @Test fun classificationIsIdenticalToThePreFixPredicates() { @@ -123,33 +211,55 @@ class SleepStagerRespEvidenceTest { return "light" } + // (low, high). Separated; coincident on a value the RRV axis hits; coincident elsewhere; + // inverted (defensive — `of` reads the pair, it does not assume low <= high); and each way of + // having no bar at all, up to the 5/MG session where neither exists. + val barPairs: List> = listOf( + 0.5 to 1.0, 0.75 to 0.75, 0.5 to 0.5, 1.0 to 0.5, null to 1.0, 0.5 to null, null to null, + ) + var checked = 0 - for (moveFrac in listOf(0.0, 0.05, 0.12, 0.2)) { - for (hr in listOf(Double.NaN, 45.0, 60.0, 95.0)) { - for (hrVar in listOf(Double.NaN, 1.0, 20.0)) { - for (rmssd in listOf(Double.NaN, 10.0, 80.0)) { - // NaN (never measured), below/at/between/at/above the bars. - for (rrv in listOf(Double.NaN, 0.2, 0.5, 0.75, 1.0, 2.0)) { - for (sparse in listOf(false, true)) { - val f = feature(moveFrac, hr, hrVar, rmssd, rrv) - val new = SleepStager.classifyOne( - f, hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, - rrvHi = 1.0, rrvLo = 0.5, cardiacSparse = sparse, - ) - val old = legacy(f, 55.0, 90.0, 50.0, 10.0, 1.0, 0.5, sparse) - assertEquals( - "label changed for move=$moveFrac hr=$hr hrVar=$hrVar " + - "rmssd=$rmssd rrv=$rrv sparse=$sparse", - old, new, - ) - checked++ + var sawBothPreFixBooleansTrue = false + for ((barLo, barHi) in barPairs) { + for (moveFrac in listOf(0.0, 0.05, 0.12, 0.2)) { + for (hr in listOf(Double.NaN, 45.0, 60.0, 95.0)) { + for (hrVar in listOf(Double.NaN, 1.0, 20.0)) { + for (rmssd in listOf(Double.NaN, 10.0, 80.0)) { + // NaN (never measured), below/at/between/at/above the bars. + for (rrv in listOf(Double.NaN, 0.2, 0.5, 0.75, 1.0, 2.0)) { + for (sparse in listOf(false, true)) { + val f = feature(moveFrac, hr, hrVar, rmssd, rrv) + val new = SleepStager.classifyOne( + f, hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, + rrvHi = barHi, rrvLo = barLo, cardiacSparse = sparse, + ) + val old = legacy(f, 55.0, 90.0, 50.0, 10.0, barHi, barLo, sparse) + assertEquals( + "label changed for move=$moveFrac hr=$hr hrVar=$hrVar " + + "rmssd=$rmssd rrv=$rrv sparse=$sparse " + + "bars=($barLo, $barHi)", + old, new, + ) + if (rrv.isFinite() && barLo != null && barHi != null && + rrv >= barHi && rrv <= barLo + ) { + sawBothPreFixBooleansTrue = true + } + checked++ + } } } } } } } - assertEquals(4 * 4 * 3 * 3 * 6 * 2, checked) + assertEquals(7 * 4 * 4 * 3 * 3 * 6 * 2, checked) + // The grid is only worth more than the old one if it actually visits the cell that motivated it. + assertTrue( + "the grid must reach `rrvIrregular && rrvRegular` — otherwise the coincident-bar case is " + + "still untested and a bar-pair axis was added for nothing", + sawBothPreFixBooleansTrue, + ) } /** @@ -190,24 +300,32 @@ class SleepStagerRespEvidenceTest { /** * `remRejectReason` must stay in lockstep with `classifyOne` — they were hand-duplicated predicates - * and now share one factory, so this guards the seam. + * and now share one factory, so this guards the seam. The bar pair is an axis here too: the diagnostic + * reads `meetsIrregularBar`, so the coincident-bar case has to be exercised on both sides of the seam + * or only one of them is pinned. */ @Test fun remRejectReasonAgreesWithTheClassifier() { - for (rrv in listOf(Double.NaN, 0.2, 0.75, 2.0)) { - for (hr in listOf(Double.NaN, 45.0, 95.0)) { - for (hrVar in listOf(Double.NaN, 1.0, 20.0)) { - for (moveFrac in listOf(0.0, 0.2)) { - val f = feature(moveFrac, hr, hrVar, Double.NaN, rrv) - val label = classify(f) - val reason = SleepStager.remRejectReason( - f, hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, - rrvHi = 1.0, rrvLo = 0.5, - ) - assertEquals( - "rem-eligibility disagreed for rrv=$rrv hr=$hr", - label == "rem", reason == SleepStager.REMRejectReason.REM_ELIGIBLE, - ) + val barPairs: List> = listOf(0.5 to 1.0, 0.75 to 0.75, null to null) + for ((barLo, barHi) in barPairs) { + for (rrv in listOf(Double.NaN, 0.2, 0.75, 2.0)) { + for (hr in listOf(Double.NaN, 45.0, 95.0)) { + for (hrVar in listOf(Double.NaN, 1.0, 20.0)) { + for (moveFrac in listOf(0.0, 0.2)) { + val f = feature(moveFrac, hr, hrVar, Double.NaN, rrv) + val label = SleepStager.classifyOne( + f, hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, + rrvHi = barHi, rrvLo = barLo, + ) + val reason = SleepStager.remRejectReason( + f, hrLo = 55.0, hrHi = 90.0, rmssdHi = 50.0, hrvarHi = 10.0, + rrvHi = barHi, rrvLo = barLo, + ) + assertEquals( + "rem-eligibility disagreed for rrv=$rrv hr=$hr bars=($barLo, $barHi)", + label == "rem", reason == SleepStager.REMRejectReason.REM_ELIGIBLE, + ) + } } } }