From da7ef971f5aa1d12b5b5a43847924db91879c09c Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 12 Sep 2026 18:38:40 +1200 Subject: [PATCH 1/5] hrv: say why the nightly summary reported nothing #2128 was filed against intended behaviour, on the strength of this line: hrv nightSummary reported=nil wholeNight=31.55ms deepOnly=28.56ms lastSWS=13.95ms nWin=87 nDeep=22 87 windows, 22 of them deep, three real means, and nothing reported. That reads as a value that went missing. It is the #1118 gate refusing an over-counted night on purpose, which is correct: RMSSD off beats that bank more beat-time than the wall clock is not a number worth keeping. The line now says so: hrv nightSummary reported=nil refused=crossSecondOverCount wholeNight=... The `hrv diag` row immediately above already carries `rrIntegrity`, which is what makes this worth fixing rather than shrugging at: that verdict is scored over the whole DAY while the gate runs per SESSION, so the two disagree exactly when it matters. One night in the field log reads `rrIntegrity=underCovered`, which is trustworthy, and still reported nothing, because a session inside it was not. Reading the adjacent line is precisely how the wrong conclusion got reached. The classification moves into `sessionRrVerdict` so the value gate and the explanation ask the same helper over the same beats. The gate's own comment insists a verdict must not describe a different set of beats than the number it gates, and two copies of it would eventually do that. Printed only when the night produced window means, so a night with no R-R at all explains itself and pays nothing. On a withheld night the cost is one O(n) filter and coverage pass per session. `sessionAvgHRV` warns against buying diagnostic detail here, but what it refuses is a SORT; `rrCoverage` is a single linear pass, so the warning is honoured rather than ignored. Kotlin 7 tests green, Swift 97. The new one on each side asserts an AGREEMENT rather than a fixed label: a withheld night must be exactly the night the traced verdict calls untrustworthy, so the explanation and the decision cannot drift apart. No scoring change. The gate keeps refusing exactly what it refused before; it just stops looking like a malfunction in the log. --- .../StrandAnalytics/AnalyticsEngine.swift | 20 +++++++++++- .../Sources/StrandAnalytics/SleepStager.swift | 28 +++++++++-------- .../HrvOverCountGateTests.swift | 24 ++++++++++++++ .../com/noop/analytics/AnalyticsEngine.kt | 18 +++++++++++ .../java/com/noop/analytics/SleepStager.kt | 31 +++++++++++-------- .../noop/analytics/HrvOverCountGateTest.kt | 24 ++++++++++++++ 6 files changed, 118 insertions(+), 27 deletions(-) diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift index 15edf923f9..033d712402 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift @@ -769,7 +769,25 @@ 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` line one 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. + // + // Cost: one extra O(n) filter and coverage pass per withheld session. `sessionAvgHRV`'s own + // comment warns against buying diagnostic detail on this path, but what it refuses is a SORT + // (`collapsedCoverage` opens with one); `rrCoverage` is a single linear pass, and this runs only + // on nights that already produced nothing. + let refused: String = (avgHRVDaily != nil || withR.isEmpty) ? "" : (physiologySessions + .map { SleepStager.sessionRrVerdict(start: $0.start, end: $0.end, rr: rrSorted) } + .first { !HRVAnalyzer.successiveDiffIsTrustworthy($0) } + .map { "refused=\($0.rawValue) " } ?? "") + hrvTraceSink("hrv nightSummary reported=\(reported) \(refused)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/Sources/StrandAnalytics/SleepStager.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift index 1d8c6bf8bc..d4c385a0ab 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift @@ -2943,6 +2943,20 @@ public enum SleepStager { public let rmssd: Double? } + /// The #1118 coverage verdict for a session's OWN R-R, windowed exactly as `sessionHrvWindows` is. + /// + /// Named once because two callers need the SAME verdict: `sessionAvgHRV`, which withholds the night's + /// HRV when the beats cannot carry a successive-difference statistic, and the nightly trace, which has + /// to say WHY it was withheld. A second copy of the classification could describe a different set of + /// beats than the number it is explaining, which is the one thing the gate's own comment insists on. + static func sessionRrVerdict(start: Int, end: Int, rr: [RRInterval]) -> HRVAnalyzer.RrCoverageVerdict { + let seg = rr.filter { $0.ts >= start && $0.ts <= end } + // `collapsed` is deliberately the SAME figure as `coverage`; see `sessionAvgHRV` for why the + // distinction between the two over-count verdicts is not worth a second sort here. + let coverage = HRVAnalyzer.rrCoverage(tsSec: seg.map { $0.ts }, rrMs: seg.map { Double($0.rrMs) }) + return HRVAnalyzer.classifyCoverage(coverage: coverage, collapsed: coverage) + } + /// Mean RMSSD over 5-min tumbling windows across the session (ms), or nil. /// Uses the same range-filter + ≥2-valid-interval rule as hrv.rmssd(). static func sessionAvgHRV(start: Int, end: Int, rr: [RRInterval]) -> Double? { @@ -2958,19 +2972,7 @@ public enum SleepStager { // Classified over the SAME beats the value was built from, windowed [start, end] exactly as // `sessionHrvWindows` does, so the verdict cannot describe a different set of beats than the number // it is gating. - let seg = rr.filter { $0.ts >= start && $0.ts <= end } - let segTs = seg.map { $0.ts } - let segMs = seg.map { Double($0.rrMs) } - let coverage = HRVAnalyzer.rrCoverage(tsSec: segTs, rrMs: segMs) - // `collapsed` is deliberately the SAME figure as `coverage`, which pins every over-count here to - // crossSecondOverCount. That is not a claim about which kind it is. The collapsed figure exists only - // to choose BETWEEN the two over-count verdicts, and this gate refuses both, so the real one would - // change no outcome — while costing a full sort of the night's ~50-70k beats, since - // `collapsedCoverage` opens with a sort. This runs per session, per day, across ~21 days of every - // analyzeRecent, every 15 minutes; #1510 cut this exact path from six sorts a night to two, and - // buying a distinction the caller discards would hand that back. `rrCoverage` is a single O(n) - // pass. If a future gate ever needs the two over-count cases apart, compute it then. - let verdict = HRVAnalyzer.classifyCoverage(coverage: coverage, collapsed: coverage) + let verdict = sessionRrVerdict(start: start, end: end, rr: rr) guard HRVAnalyzer.successiveDiffIsTrustworthy(verdict) else { return nil } return vals.reduce(0, +) / Double(vals.count) } diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift index 69b0c69348..abcb89d8b8 100644 --- a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift @@ -80,4 +80,28 @@ 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 extracting the classification into + /// `sessionRrVerdict` 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") + let verdict = SleepStager.sessionRrVerdict(start: start, end: end, rr: beats) + XCTAssertEqual(SleepStager.sessionAvgHRV(start: start, end: end, rr: beats) == nil, + !HRVAnalyzer.successiveDiffIsTrustworthy(verdict), + "a withheld night must be exactly the night the traced verdict calls untrustworthy") + } + } } 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..c57d481053 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,26 @@ 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` line one 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. + // + // Cost: one extra O(n) filter and coverage pass per withheld session. `sessionAvgHRV`'s own + // comment warns against buying diagnostic detail on this path, but what it refuses is a SORT + // (`collapsedCoverage` opens with one); `rrCoverage` is a single linear pass, and this runs + // only on nights that already produced nothing. + val refused = if (avgHRVDaily != null || withR.isEmpty()) null else physiologySessions + .map { SleepStager.sessionRrVerdict(it.start, it.end, rrSorted) } + .firstOrNull { !HrvAnalyzer.successiveDiffIsTrustworthy(it) } hrvTraceSink( "hrv nightSummary reported=${avgHRVDaily?.let { "${round2(it)}ms" } ?: "nil"} " + + (refused?.let { "refused=${it.raw} " } ?: "") + "wholeNight=${meanMs(withR)} deepOnly=${meanMs(deepW)} " + "lastSWS=${meanMs(lastSws)} nWin=${withR.size} nDeep=${deepW.size}", ) 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 30202f6f57..5f67c45741 100644 --- a/android/app/src/main/java/com/noop/analytics/SleepStager.kt +++ b/android/app/src/main/java/com/noop/analytics/SleepStager.kt @@ -3267,6 +3267,23 @@ object SleepStager { * beat). Drives both [sessionAvgHRV] and the HRV test-mode trace. */ data class HrvWindow(val startTs: Long, val stage: String, val cleanBeats: Int, val rmssd: Double?) + /** + * The #1118 coverage verdict for a session's OWN R-R, windowed exactly as [sessionHrvWindows] is. + * + * Named once because two callers need the SAME verdict: [sessionAvgHRV], which withholds the night's + * HRV when the beats cannot carry a successive-difference statistic, and the nightly trace, which has + * to say WHY it was withheld. A second copy of the classification could describe a different set of + * beats than the number it is explaining, which is the one thing the gate's own comment insists on. + */ + internal fun sessionRrVerdict(start: Long, end: Long, + rr: List): HrvAnalyzer.RrCoverageVerdict { + val seg = rr.filter { it.ts in start..end } + // `collapsed` is deliberately the SAME figure as `coverage`; see [sessionAvgHRV] for why the + // distinction between the two over-count verdicts is not worth a second sort here. + val coverage = HrvAnalyzer.rrCoverage(seg.map { it.ts }, seg.map { it.rrMs.toDouble() }) + return HrvAnalyzer.classifyCoverage(coverage, coverage) + } + /** * Mean RMSSD over 5-min tumbling windows across the session (ms), or null. * Uses the same range-filter + ≥2-valid-interval rule as hrv.rmssd(). @@ -3284,19 +3301,7 @@ object SleepStager { // Classified over the SAME beats the value was built from, windowed [start, end] exactly as // `sessionHrvWindows` does, so the verdict cannot describe a different set of beats than the number // it is gating. - val seg = rr.filter { it.ts in start..end } - val segTs = seg.map { it.ts } - val segMs = seg.map { it.rrMs.toDouble() } - val coverage = HrvAnalyzer.rrCoverage(segTs, segMs) - // `collapsed` is deliberately the SAME figure as `coverage`, which pins every over-count here to - // CROSS_SECOND. That is not a claim about which kind it is. The collapsed figure exists only to - // choose BETWEEN the two over-count verdicts, and this gate refuses both, so the real one would - // change no outcome — while costing a full sort of the night's ~50-70k beats, since - // [HrvAnalyzer.collapsedCoverage] opens with `sortedWith`. This runs per session, per day, across - // ~21 days of every analyzeRecent, every 15 minutes; #1510 cut this exact path from six sorts a - // night to two, and buying a distinction the caller discards would hand that back. `rrCoverage` is - // a single O(n) pass. If a future gate ever needs the two over-count cases apart, compute it then. - val verdict = HrvAnalyzer.classifyCoverage(coverage, coverage) + val verdict = sessionRrVerdict(start, end, rr) if (!HrvAnalyzer.successiveDiffIsTrustworthy(verdict)) return null return vals.sum() / vals.size.toDouble() } 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..dfa74c5a24 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,28 @@ 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 extracting the classification into + * [SleepStager.sessionRrVerdict] 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)) + val verdict = SleepStager.sessionRrVerdict(start, end, beats) + assertEquals( + "a withheld night must be exactly the night the traced verdict calls untrustworthy", + SleepStager.sessionAvgHRV(start, end, beats) == null, + !HrvAnalyzer.successiveDiffIsTrustworthy(verdict), + ) + } + } } From 1de6a1e259913fc8fe39c98df3cbae90866ecaa6 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 12 Sep 2026 18:43:26 +1200 Subject: [PATCH 2/5] hrv: do not name the over-count gate in a mode that never consults it Re-review. `deepHrvWindow` is a user setting, not a dev flag, and in that mode `avgHRVDaily` re-derives from `sessionHrvWindows` and never reads `s.avgHRV`. So the #1118 gate plays no part in that branch's nil, and the reason line would have named a gate that had nothing to do with it. The exact fault this line exists to correct: a diagnostic asserting a cause it did not verify. `nDeep` on the same line already covers that case, so it now stays quiet there. Worth noting separately, and not changed here because this PR alters no scoring: the deep branch has no over-count protection at all. A wearer who selects the deep-sleep HRV window gets a value built from beats the whole-night path would refuse. --- .../Sources/StrandAnalytics/AnalyticsEngine.swift | 6 +++++- .../app/src/main/java/com/noop/analytics/AnalyticsEngine.kt | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift index 033d712402..5b693c76c8 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift @@ -783,7 +783,11 @@ public enum AnalyticsEngine { // comment warns against buying diagnostic detail on this path, but what it refuses is a SORT // (`collapsedCoverage` opens with one); `rrCoverage` is a single linear pass, and this runs only // on nights that already produced nothing. - let refused: String = (avgHRVDaily != nil || withR.isEmpty) ? "" : (physiologySessions + // + // NOT in deep-window mode. That branch re-derives from `sessionHrvWindows` and never reads + // `s.avgHRV`, so the #1118 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: String = (deepHrvWindow || avgHRVDaily != nil || withR.isEmpty) ? "" : (physiologySessions .map { SleepStager.sessionRrVerdict(start: $0.start, end: $0.end, rr: rrSorted) } .first { !HRVAnalyzer.successiveDiffIsTrustworthy($0) } .map { "refused=\($0.rawValue) " } ?? "") 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 c57d481053..52e2576411 100644 --- a/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt +++ b/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt @@ -658,7 +658,11 @@ object AnalyticsEngine { // comment warns against buying diagnostic detail on this path, but what it refuses is a SORT // (`collapsedCoverage` opens with one); `rrCoverage` is a single linear pass, and this runs // only on nights that already produced nothing. - val refused = if (avgHRVDaily != null || withR.isEmpty()) null else physiologySessions + // + // NOT in deep-window mode. That branch re-derives from `sessionHrvWindows` and never reads + // `s.avgHRV`, so the #1118 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 = if (deepHrvWindow || avgHRVDaily != null || withR.isEmpty()) null else physiologySessions .map { SleepStager.sessionRrVerdict(it.start, it.end, rrSorted) } .firstOrNull { !HrvAnalyzer.successiveDiffIsTrustworthy(it) } hrvTraceSink( From a3781b4368da04527d3bff41054c0ecbd86598c3 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 12 Sep 2026 18:49:45 +1200 Subject: [PATCH 3/5] hrv: declare the sessionRrVerdict twins --- .../StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift | 2 ++ android/app/src/main/java/com/noop/analytics/SleepStager.kt | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift index d4c385a0ab..6f122d948d 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift @@ -2949,6 +2949,8 @@ public enum SleepStager { /// HRV when the beats cannot carry a successive-difference statistic, and the nightly trace, which has /// to say WHY it was withheld. A second copy of the classification could describe a different set of /// beats than the number it is explaining, which is the one thing the gate's own comment insists on. + /// + /// Mirrors Kotlin `sessionRrVerdict`. static func sessionRrVerdict(start: Int, end: Int, rr: [RRInterval]) -> HRVAnalyzer.RrCoverageVerdict { let seg = rr.filter { $0.ts >= start && $0.ts <= end } // `collapsed` is deliberately the SAME figure as `coverage`; see `sessionAvgHRV` for why the 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 5f67c45741..8ff6f316c7 100644 --- a/android/app/src/main/java/com/noop/analytics/SleepStager.kt +++ b/android/app/src/main/java/com/noop/analytics/SleepStager.kt @@ -3274,6 +3274,8 @@ object SleepStager { * HRV when the beats cannot carry a successive-difference statistic, and the nightly trace, which has * to say WHY it was withheld. A second copy of the classification could describe a different set of * beats than the number it is explaining, which is the one thing the gate's own comment insists on. + * + * Mirrors Swift `sessionRrVerdict`. */ internal fun sessionRrVerdict(start: Long, end: Long, rr: List): HrvAnalyzer.RrCoverageVerdict { From 85ada59d98a31496bb045c1d5c75686fc2971bc0 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 12 Sep 2026 19:00:43 +1200 Subject: [PATCH 4/5] hrv: derive the refusal from the calls that already exist Re-review. The shared helper added a declaration on both platforms, and both halves of this pair are INSIDE the parity ledger's scan, so it reported two new one-sided functions. Clearing that needs an authority migration the design deliberately makes hard, and the gate is path filtered to Tools/parity_*, so the drift would have sat here and failed some later unrelated PR instead. Fighting a governance migration to label a diagnostic is not a trade worth making, and it turns out not to be necessary. Inside sessionAvgHRV the only two paths to null are "no window yielded an RMSSD" and the gate. So windows-with-RMSSD beside a null value IS the gate, derived from two calls that already exist, with nothing restated that could later disagree with the thing it explains. That is a better answer than the helper was: no new symbol, no second copy of the classification, and no ledger debt. It now says `refused=overCount` rather than naming a verdict. sessionAvgHRV pins every over-count to CROSS_SECOND internally to avoid a sort, so the specific label was a distinction the code does not actually make, and printing it claimed more than was known. Ledger back to main's counts, 1870 Swift and 2440 Kotlin functions. Kotlin 7 tests green, Swift 7. Both now pin the same pair the trace reads rather than a label. --- .../StrandAnalytics/AnalyticsEngine.swift | 32 +++++++++++-------- .../Sources/StrandAnalytics/SleepStager.swift | 22 +++---------- .../HrvOverCountGateTests.swift | 11 ++++--- .../com/noop/analytics/AnalyticsEngine.kt | 31 ++++++++++-------- .../java/com/noop/analytics/SleepStager.kt | 25 +++------------ .../noop/analytics/HrvOverCountGateTest.kt | 16 +++++++--- 6 files changed, 64 insertions(+), 73 deletions(-) diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift index 5b693c76c8..dfd7b9f04b 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift @@ -774,24 +774,28 @@ public enum AnalyticsEngine { // 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` line one 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. + // 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. // - // Cost: one extra O(n) filter and coverage pass per withheld session. `sessionAvgHRV`'s own - // comment warns against buying diagnostic detail on this path, but what it refuses is a SORT - // (`collapsedCoverage` opens with one); `rrCoverage` is a single linear pass, and this runs only - // on nights that already produced nothing. + // 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 #1118 gate plays no part in its nil and naming it would be a diagnostic + // `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: String = (deepHrvWindow || avgHRVDaily != nil || withR.isEmpty) ? "" : (physiologySessions - .map { SleepStager.sessionRrVerdict(start: $0.start, end: $0.end, rr: rrSorted) } - .first { !HRVAnalyzer.successiveDiffIsTrustworthy($0) } - .map { "refused=\($0.rawValue) " } ?? "") - hrvTraceSink("hrv nightSummary reported=\(reported) \(refused)wholeNight=\(meanMs(withR)) deepOnly=\(meanMs(deepW)) lastSWS=\(meanMs(lastSws)) nWin=\(withR.count) nDeep=\(deepW.count)") + 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/Sources/StrandAnalytics/SleepStager.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift index 6f122d948d..9da0ba2367 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift @@ -2943,22 +2943,6 @@ public enum SleepStager { public let rmssd: Double? } - /// The #1118 coverage verdict for a session's OWN R-R, windowed exactly as `sessionHrvWindows` is. - /// - /// Named once because two callers need the SAME verdict: `sessionAvgHRV`, which withholds the night's - /// HRV when the beats cannot carry a successive-difference statistic, and the nightly trace, which has - /// to say WHY it was withheld. A second copy of the classification could describe a different set of - /// beats than the number it is explaining, which is the one thing the gate's own comment insists on. - /// - /// Mirrors Kotlin `sessionRrVerdict`. - static func sessionRrVerdict(start: Int, end: Int, rr: [RRInterval]) -> HRVAnalyzer.RrCoverageVerdict { - let seg = rr.filter { $0.ts >= start && $0.ts <= end } - // `collapsed` is deliberately the SAME figure as `coverage`; see `sessionAvgHRV` for why the - // distinction between the two over-count verdicts is not worth a second sort here. - let coverage = HRVAnalyzer.rrCoverage(tsSec: seg.map { $0.ts }, rrMs: seg.map { Double($0.rrMs) }) - return HRVAnalyzer.classifyCoverage(coverage: coverage, collapsed: coverage) - } - /// Mean RMSSD over 5-min tumbling windows across the session (ms), or nil. /// Uses the same range-filter + ≥2-valid-interval rule as hrv.rmssd(). static func sessionAvgHRV(start: Int, end: Int, rr: [RRInterval]) -> Double? { @@ -2974,7 +2958,11 @@ public enum SleepStager { // Classified over the SAME beats the value was built from, windowed [start, end] exactly as // `sessionHrvWindows` does, so the verdict cannot describe a different set of beats than the number // it is gating. - let verdict = sessionRrVerdict(start: start, end: end, rr: rr) + let seg = rr.filter { $0.ts >= start && $0.ts <= end } + let segTs = seg.map { $0.ts } + let segMs = seg.map { Double($0.rrMs) } + let coverage = HRVAnalyzer.rrCoverage(tsSec: segTs, rrMs: segMs) + let verdict = HRVAnalyzer.classifyCoverage(coverage: coverage, collapsed: coverage) guard HRVAnalyzer.successiveDiffIsTrustworthy(verdict) else { return nil } return vals.reduce(0, +) / Double(vals.count) } diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift index abcb89d8b8..46689ca4bf 100644 --- a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/HrvOverCountGateTests.swift @@ -83,8 +83,8 @@ final class HrvOverCountGateTests: XCTestCase { /// #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 extracting the classification into - /// `sessionRrVerdict` cannot let the explanation and the decision drift apart. A bare `reported=nil` + /// 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() { @@ -98,10 +98,13 @@ final class HrvOverCountGateTests: XCTestCase { } for beats in [overCounted, plausible] { XCTAssertTrue(windowsYieldRMSSD(start, end, beats), "precondition: these beats DO yield an RMSSD") - let verdict = SleepStager.sessionRrVerdict(start: start, end: end, rr: 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 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 the traced verdict calls untrustworthy") + "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 52e2576411..863de4de39 100644 --- a/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt +++ b/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt @@ -649,25 +649,30 @@ object AnalyticsEngine { // 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` line one 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. + // 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. // - // Cost: one extra O(n) filter and coverage pass per withheld session. `sessionAvgHRV`'s own - // comment warns against buying diagnostic detail on this path, but what it refuses is a SORT - // (`collapsedCoverage` opens with one); `rrCoverage` is a single linear pass, and this runs - // only on nights that already produced nothing. + // 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 #1118 gate plays no part in its nil and naming it would be a diagnostic + // `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 = if (deepHrvWindow || avgHRVDaily != null || withR.isEmpty()) null else physiologySessions - .map { SleepStager.sessionRrVerdict(it.start, it.end, rrSorted) } - .firstOrNull { !HrvAnalyzer.successiveDiffIsTrustworthy(it) } + 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"} " + - (refused?.let { "refused=${it.raw} " } ?: "") + + (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/main/java/com/noop/analytics/SleepStager.kt b/android/app/src/main/java/com/noop/analytics/SleepStager.kt index 8ff6f316c7..da7beb2ab4 100644 --- a/android/app/src/main/java/com/noop/analytics/SleepStager.kt +++ b/android/app/src/main/java/com/noop/analytics/SleepStager.kt @@ -3267,25 +3267,6 @@ object SleepStager { * beat). Drives both [sessionAvgHRV] and the HRV test-mode trace. */ data class HrvWindow(val startTs: Long, val stage: String, val cleanBeats: Int, val rmssd: Double?) - /** - * The #1118 coverage verdict for a session's OWN R-R, windowed exactly as [sessionHrvWindows] is. - * - * Named once because two callers need the SAME verdict: [sessionAvgHRV], which withholds the night's - * HRV when the beats cannot carry a successive-difference statistic, and the nightly trace, which has - * to say WHY it was withheld. A second copy of the classification could describe a different set of - * beats than the number it is explaining, which is the one thing the gate's own comment insists on. - * - * Mirrors Swift `sessionRrVerdict`. - */ - internal fun sessionRrVerdict(start: Long, end: Long, - rr: List): HrvAnalyzer.RrCoverageVerdict { - val seg = rr.filter { it.ts in start..end } - // `collapsed` is deliberately the SAME figure as `coverage`; see [sessionAvgHRV] for why the - // distinction between the two over-count verdicts is not worth a second sort here. - val coverage = HrvAnalyzer.rrCoverage(seg.map { it.ts }, seg.map { it.rrMs.toDouble() }) - return HrvAnalyzer.classifyCoverage(coverage, coverage) - } - /** * Mean RMSSD over 5-min tumbling windows across the session (ms), or null. * Uses the same range-filter + ≥2-valid-interval rule as hrv.rmssd(). @@ -3303,7 +3284,11 @@ object SleepStager { // Classified over the SAME beats the value was built from, windowed [start, end] exactly as // `sessionHrvWindows` does, so the verdict cannot describe a different set of beats than the number // it is gating. - val verdict = sessionRrVerdict(start, end, rr) + val seg = rr.filter { it.ts in start..end } + val segTs = seg.map { it.ts } + val segMs = seg.map { it.rrMs.toDouble() } + val coverage = HrvAnalyzer.rrCoverage(segTs, segMs) + val verdict = HrvAnalyzer.classifyCoverage(coverage, coverage) if (!HrvAnalyzer.successiveDiffIsTrustworthy(verdict)) return null return vals.sum() / vals.size.toDouble() } 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 dfa74c5a24..6c9acb8988 100644 --- a/android/app/src/test/java/com/noop/analytics/HrvOverCountGateTest.kt +++ b/android/app/src/test/java/com/noop/analytics/HrvOverCountGateTest.kt @@ -88,8 +88,8 @@ class HrvOverCountGateTest { /** * #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 extracting the classification into - * [SleepStager.sessionRrVerdict] cannot let the explanation and the decision drift apart. A bare + * 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. */ @@ -100,11 +100,17 @@ class HrvOverCountGateTest { 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)) - val verdict = SleepStager.sessionRrVerdict(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 the traced verdict calls untrustworthy", + "a withheld night must be exactly the night whose own beats were refused", SleepStager.sessionAvgHRV(start, end, beats) == null, - !HrvAnalyzer.successiveDiffIsTrustworthy(verdict), + !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() }), + ), + ), ) } } From dee600ecb49477df4f61fe81ff3752f4ad12b010 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 12 Sep 2026 19:01:13 +1200 Subject: [PATCH 5/5] hrv: put back the comment the helper extraction took with it Restoring SleepStager on both platforms to exactly what main has. The extract-and-revert dropped the eight lines explaining why `collapsed` is passed the same figure as `coverage`, which is the reasoning this PR now leans on to justify saying `overCount` instead of naming a verdict. Neither stager is touched by this PR any more. --- .../Sources/StrandAnalytics/SleepStager.swift | 8 ++++++++ .../app/src/main/java/com/noop/analytics/SleepStager.kt | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift index 9da0ba2367..1d8c6bf8bc 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/SleepStager.swift @@ -2962,6 +2962,14 @@ public enum SleepStager { let segTs = seg.map { $0.ts } let segMs = seg.map { Double($0.rrMs) } let coverage = HRVAnalyzer.rrCoverage(tsSec: segTs, rrMs: segMs) + // `collapsed` is deliberately the SAME figure as `coverage`, which pins every over-count here to + // crossSecondOverCount. That is not a claim about which kind it is. The collapsed figure exists only + // to choose BETWEEN the two over-count verdicts, and this gate refuses both, so the real one would + // change no outcome — while costing a full sort of the night's ~50-70k beats, since + // `collapsedCoverage` opens with a sort. This runs per session, per day, across ~21 days of every + // analyzeRecent, every 15 minutes; #1510 cut this exact path from six sorts a night to two, and + // buying a distinction the caller discards would hand that back. `rrCoverage` is a single O(n) + // pass. If a future gate ever needs the two over-count cases apart, compute it then. let verdict = HRVAnalyzer.classifyCoverage(coverage: coverage, collapsed: coverage) guard HRVAnalyzer.successiveDiffIsTrustworthy(verdict) else { return nil } return vals.reduce(0, +) / Double(vals.count) 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 da7beb2ab4..30202f6f57 100644 --- a/android/app/src/main/java/com/noop/analytics/SleepStager.kt +++ b/android/app/src/main/java/com/noop/analytics/SleepStager.kt @@ -3288,6 +3288,14 @@ object SleepStager { val segTs = seg.map { it.ts } val segMs = seg.map { it.rrMs.toDouble() } val coverage = HrvAnalyzer.rrCoverage(segTs, segMs) + // `collapsed` is deliberately the SAME figure as `coverage`, which pins every over-count here to + // CROSS_SECOND. That is not a claim about which kind it is. The collapsed figure exists only to + // choose BETWEEN the two over-count verdicts, and this gate refuses both, so the real one would + // change no outcome — while costing a full sort of the night's ~50-70k beats, since + // [HrvAnalyzer.collapsedCoverage] opens with `sortedWith`. This runs per session, per day, across + // ~21 days of every analyzeRecent, every 15 minutes; #1510 cut this exact path from six sorts a + // night to two, and buying a distinction the caller discards would hand that back. `rrCoverage` is + // a single O(n) pass. If a future gate ever needs the two over-count cases apart, compute it then. val verdict = HrvAnalyzer.classifyCoverage(coverage, coverage) if (!HrvAnalyzer.successiveDiffIsTrustworthy(verdict)) return null return vals.sum() / vals.size.toDouble()