diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift index 15edf923f9..dfd7b9f04b 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift @@ -769,7 +769,33 @@ public enum AnalyticsEngine { // `wholeNight` is the pooled-window mean it equals on single-session nights and the apples-to- // apples baseline for the deepOnly/lastSWS comparison (all three are pooled window means). let reported = avgHRVDaily.map { "\(r2($0))ms" } ?? "nil" - hrvTraceSink("hrv nightSummary reported=\(reported) wholeNight=\(meanMs(withR)) deepOnly=\(meanMs(deepW)) lastSWS=\(meanMs(lastSws)) nWin=\(withR.count) nDeep=\(deepW.count)") + // #2128: say WHY `reported` is nil, but ONLY when the night printed real window means beside + // it. That is the confusing case: a nil next to `wholeNight=31.55ms` reads as a value that went + // missing, when the usual cause is the #1118 gate refusing an over-counted night on purpose. A + // night with no windows at all explains itself and pays nothing here. + // + // The `hrv diag` row above carries `rrIntegrity`, but that verdict is scored over the whole DAY + // while the gate runs per SESSION, so the two disagree exactly when it matters: a day reading + // `underCovered` can still hold a session the gate refused. Reading the adjacent line is what + // led #2128 to be filed against intended behaviour. + // + // Derived from the two existing calls rather than by re-classifying the beats. Inside + // `sessionAvgHRV` the ONLY paths to nil are "no window yielded an RMSSD" and the gate, so + // windows-with-RMSSD plus a nil value IS the gate, with nothing restated that could later + // disagree with it. It says `overCount` rather than naming a verdict because that function pins + // every over-count to crossSecondOverCount internally to avoid a sort, so the specific label + // would be a distinction it does not actually make. + // + // NOT in deep-window mode. That branch re-derives from `sessionHrvWindows` and never reads + // `s.avgHRV`, so the gate plays no part in its nil and naming it would be a diagnostic + // asserting a cause it did not verify. `nDeep` on this same line already explains that case. + let refused = !deepHrvWindow && avgHRVDaily == nil && !withR.isEmpty + && physiologySessions.contains { s in + SleepStager.sessionHrvWindows(start: s.start, end: s.end, rr: rrSorted, stages: []) + .contains { $0.rmssd != nil } + && SleepStager.sessionAvgHRV(start: s.start, end: s.end, rr: rrSorted) == nil + } + hrvTraceSink("hrv nightSummary reported=\(reported) \(refused ? "refused=overCount " : "")wholeNight=\(meanMs(withR)) deepOnly=\(meanMs(deepW)) lastSWS=\(meanMs(lastSws)) nWin=\(withR.count) nDeep=\(deepW.count)") } // Nightly APPROXIMATE respiratory rate (breaths/min) from the R-R stream via diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift index 69b0c69348..46689ca4bf 100644 --- a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift @@ -80,4 +80,31 @@ final class HrvOverCountGateTests: XCTestCase { "verdict \(v) must be judged alike by both gates") } } + + /// #2128: the verdict the nightly trace prints must be the one the value gate actually acted on. + /// + /// Asserted as an AGREEMENT rather than against a fixed label, so the trace deriving its reason from + /// existing calls cannot let the explanation and the decision drift apart. A bare `reported=nil` + /// beside real window means reads as a value that went missing; it is usually this gate refusing an + /// over-counted night on purpose, and the trace can only say so if it asks the same helper. + func testTheTracedVerdictAgreesWithTheGate() { + let start = 1_000, end = 1_600 + var overCounted: [RRInterval] = [] + var plausible: [RRInterval] = [] + for i in 0..<600 { + overCounted.append(RRInterval(ts: start + i, rrMs: 880)) + overCounted.append(RRInterval(ts: start + i, rrMs: 960)) + plausible.append(RRInterval(ts: start + i, rrMs: 900)) + } + for beats in [overCounted, plausible] { + XCTAssertTrue(windowsYieldRMSSD(start, end, beats), "precondition: these beats DO yield an RMSSD") + // The trace derives "the gate refused" from exactly this pair, so pin the pair rather than + // a label: windows that DID yield an RMSSD, beside a nil value, can only be the gate. + let cov = HRVAnalyzer.rrCoverage(tsSec: beats.map { $0.ts }, rrMs: beats.map { Double($0.rrMs) }) + let verdict = HRVAnalyzer.classifyCoverage(coverage: cov, collapsed: cov) + XCTAssertEqual(SleepStager.sessionAvgHRV(start: start, end: end, rr: beats) == nil, + !HRVAnalyzer.successiveDiffIsTrustworthy(verdict), + "a withheld night must be exactly the night whose own beats were refused") + } + } } diff --git a/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt b/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt index ae8a268388..863de4de39 100644 --- a/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt +++ b/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt @@ -644,8 +644,35 @@ object AnalyticsEngine { // `reported` is the value NOOP actually displays (duration-weighted session-mean-of-means); // `wholeNight` is the pooled-window mean it equals on single-session nights and the apples-to- // apples baseline for the deepOnly/lastSWS comparison (all three are pooled window means). + // #2128: say WHY `reported` is nil, but ONLY when the night printed real window means beside + // it. That is the confusing case: a nil next to `wholeNight=31.55ms` reads as a value that + // went missing, when the usual cause is the #1118 gate refusing an over-counted night on + // purpose. A night with no windows at all explains itself and pays nothing here. + // + // The `hrv diag` row above carries `rrIntegrity`, but that verdict is scored over the whole + // DAY while the gate runs per SESSION, so the two disagree exactly when it matters: a day + // reading `underCovered` can still hold a session the gate refused. Reading the adjacent line + // is what led #2128 to be filed against intended behaviour. + // + // Derived from the two existing calls rather than by re-classifying the beats. Inside + // [SleepStager.sessionAvgHRV] the ONLY paths to null are "no window yielded an RMSSD" and the + // gate, so windows-with-RMSSD plus a null value IS the gate, with nothing restated that could + // later disagree with it. It says `overCount` rather than naming a verdict because that + // function pins every over-count to CROSS_SECOND internally to avoid a sort, so the specific + // label would be a distinction it does not actually make. + // + // NOT in deep-window mode. That branch re-derives from `sessionHrvWindows` and never reads + // `s.avgHRV`, so the gate plays no part in its nil and naming it would be a diagnostic + // asserting a cause it did not verify. `nDeep` on this same line already explains that case. + val refused = !deepHrvWindow && avgHRVDaily == null && withR.isNotEmpty() && + physiologySessions.any { s -> + SleepStager.sessionHrvWindows(s.start, s.end, rrSorted, emptyList()) + .any { it.rmssd != null } && + SleepStager.sessionAvgHRV(s.start, s.end, rrSorted) == null + } hrvTraceSink( "hrv nightSummary reported=${avgHRVDaily?.let { "${round2(it)}ms" } ?: "nil"} " + + (if (refused) "refused=overCount " else "") + "wholeNight=${meanMs(withR)} deepOnly=${meanMs(deepW)} " + "lastSWS=${meanMs(lastSws)} nWin=${withR.size} nDeep=${deepW.size}", ) diff --git a/android/app/src/test/java/com/noop/analytics/HrvOverCountGateTest.kt b/android/app/src/test/java/com/noop/analytics/HrvOverCountGateTest.kt index e66d30cd22..6c9acb8988 100644 --- a/android/app/src/test/java/com/noop/analytics/HrvOverCountGateTest.kt +++ b/android/app/src/test/java/com/noop/analytics/HrvOverCountGateTest.kt @@ -84,4 +84,34 @@ class HrvOverCountGateTest { HrvAnalyzer.beatSpreadIsTrustworthy(v), HrvAnalyzer.successiveDiffIsTrustworthy(v)) } } + + /** + * #2128: the verdict the nightly trace prints must be the one the value gate actually acted on. + * + * Asserted as an AGREEMENT rather than against a fixed label, so the trace deriving its reason from + * existing calls cannot let the explanation and the decision drift apart. A bare + * `reported=nil` beside real window means reads as a value that went missing; it is usually this gate + * refusing an over-counted night on purpose, and the trace can only say so if it asks the same helper. + */ + @Test fun theTracedVerdictAgreesWithTheGate() { + val start = 1000L + val end = start + 600 + val overCounted = (0 until 600).flatMap { i -> listOf(rr(start + i, 880), rr(start + i, 960)) } + val plausible = (0 until 600).map { i -> rr(start + i, 900) } + for (beats in listOf(overCounted, plausible)) { + assertTrue("precondition: these beats DO yield an RMSSD", windowsYieldRmssd(start, end, beats)) + // The trace derives "the gate refused" from exactly this pair, so pin the pair rather + // than a label: windows that DID yield an RMSSD, beside a null value, can only be the gate. + assertEquals( + "a withheld night must be exactly the night whose own beats were refused", + SleepStager.sessionAvgHRV(start, end, beats) == null, + !HrvAnalyzer.successiveDiffIsTrustworthy( + HrvAnalyzer.classifyCoverage( + HrvAnalyzer.rrCoverage(beats.map { it.ts }, beats.map { it.rrMs.toDouble() }), + HrvAnalyzer.rrCoverage(beats.map { it.ts }, beats.map { it.rrMs.toDouble() }), + ), + ), + ) + } + } }