From 2c73b277e9c6e128bcef13d75fad27d36cb2bc2e Mon Sep 17 00:00:00 2001 From: kavemang Date: Mon, 28 Sep 2026 02:23:45 -0500 Subject: [PATCH] analytics: restore primary-night RHR floor (Refs #2522) --- .../StrandAnalytics/AnalyticsEngine.swift | 14 ++--- .../PrimarySessionRestingHR.swift | 27 ++++----- .../PrimaryNightRestingHRDailyTests.swift | 54 +++++++++++++++++ .../com/noop/analytics/AnalyticsEngine.kt | 14 ++--- .../noop/analytics/PrimarySessionRestingHR.kt | 21 +++---- .../PrimaryNightRestingHRDailyTest.kt | 58 +++++++++++++++++++ 6 files changed, 144 insertions(+), 44 deletions(-) create mode 100644 Packages/StrandAnalytics/Tests/StrandAnalyticsTests/PrimaryNightRestingHRDailyTests.swift create mode 100644 android/app/src/test/java/com/noop/analytics/PrimaryNightRestingHRDailyTest.kt diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift index 81f2737f00..a30efa6c1b 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/AnalyticsEngine.swift @@ -705,15 +705,13 @@ public enum AnalyticsEngine { // call site" a scattered filter invites. let physiologyOnly = matched.filter { !$0.hrOnly } let physiologySessions = physiologyOnly.isEmpty ? matched : physiologyOnly - // Resting Heart Rate: Use PrimarySessionRestingHR (arithmetic sample mean of the longest/primary - // sleep session, #1169), eliminating daytime nap floor distortion. - // #804: Preserve ring/device-provided resting HR when present in `providedSleep`. - // Cleanly falls back to physiologySessions.compactMap { $0.restingHR }.min() when coverage is sparse. - let providedPrimaryRHR = physiologySessions.max(by: { ($0.end - $0.start) < ($1.end - $1.start) }) + // #2522: use the gated lowest five-minute bin from the primary session, not its whole-session + // mean. Choosing the session first preserves #2358's nap protection; a shorter nap must not + // supply the daily RHR when the main night has no HR. #804's device-provided value still wins. + let primarySession = physiologySessions.max(by: { ($0.end - $0.start) < ($1.end - $1.start) }) + let providedPrimaryRHR = primarySession .flatMap { p in providedSleep.first(where: { $0.start == p.start && $0.end == p.end })?.restingHR } - let restingHRDaily: Int? = providedPrimaryRHR - ?? primarySessionRestingHR(sessions: physiologySessions, hr: hr).map { Int($0.rounded()) } - ?? physiologySessions.compactMap { $0.restingHR }.min() + let restingHRDaily: Int? = providedPrimaryRHR ?? primarySession?.restingHR // Daily avg HRV = in-bed-weighted mean of per-session avg HRV. let avgHRVDaily: Double? = { if deepHrvWindow { diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/PrimarySessionRestingHR.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/PrimarySessionRestingHR.swift index c64eac27fe..e29b69ec5d 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/PrimarySessionRestingHR.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/PrimarySessionRestingHR.swift @@ -1,32 +1,27 @@ import Foundation -/// #1169: an alternative headline resting-HR definition — the arithmetic MEAN of valid HR samples in the -/// LONGEST (primary) sleep session, rather than the lowest-per-session floor `AnalyticsEngine` ships today. +/// #1169: a shadow resting-HR definition — the arithmetic MEAN of valid HR samples in the longest +/// (primary) sleep session, rather than the five-minute floor `AnalyticsEngine` ships today. /// /// ## Why (issue #1169, artemc) -/// The shipped daily RHR is `restingHRDaily = matched…restingHR.min()` (`AnalyticsEngine`) — a nightly HR -/// FLOOR, and the `.min()` lets a short low-HR nap replace the main overnight session. A clean-room, +/// The original daily RHR took the minimum floor across sessions, letting a short low-HR nap replace +/// the main overnight session. A clean-room, /// single-participant 5-night experiment (official WHOOP RHR + a Polar H10 ECG mean as independent /// references, a pre-declared dev/holdout split, no fitted offset) found the primary-session sample mean /// tracked both references far better: rounded MAE vs the official target 6.0→2.0 (dev) / 7.5→0.8 (holdout). /// -/// ## WIRED as of #2358 — this sets the shipped daily resting HR -/// It was landed pure and unwired, on the reasoning that switching the consumers is a re-baselining of core -/// scores and that the issue asks for a larger multi-participant holdout first. #2358 made the switch anyway, -/// as a maintainer call: `AnalyticsEngine.restingHRDaily` now prefers a device-provided primary-session value, -/// then THIS mean, and falls back to the old `restingHR.min()` floor only when coverage is sparse. So the -/// headline resting HR and everything reading it — recovery, strain, workout detection, energy — come from -/// here on any day with a covered primary session. +/// ## Shadow metric after #2522 +/// #2358 temporarily wired this mean into the daily headline while selecting the primary session to +/// avoid the nap problem. #2522 keeps that session selection but takes its existing five-minute floor. +/// This mean remains in `rhr_primary_session` for comparison and does not feed scoring. /// /// What that means for the evidence below: the MAE figures are from ONE participant over five nights against -/// a pre-declared split, which is the holdout the issue says is not yet large enough. They justified building -/// the metric; they are thinner than the change they now carry. #2284 separately replaces what a session's -/// `restingHR` IS (deep-sleep mean rather than lowest 5-minute bin), which moves the `.min()` fallback under -/// this, so the comparison baseline those numbers were measured against no longer exists unchanged. +/// a pre-declared split, which is the holdout the issue says is not yet large enough. #2284 proposes +/// changing a session's `restingHR` definition separately; this shadow comparison does not enact it. /// /// ## Definition (documented per the issue) /// - **Primary session**: the LONGEST session by duration; ties resolve to the FIRST (stable). A shorter nap -/// never replaces the main night — this is the half the shipped `.min()` gets wrong. +/// never replaces the main night — the selection rule now used by the shipped daily RHR too. /// - **Valid sample**: bpm within `validBpm` (default 30…220, matching `AnalyticsEngine`'s worn-HR range). /// Anything outside the range — or a missing sample (simply absent from the array) — is excluded. /// - **Mean**: the arithmetic SAMPLE mean (unweighted). The experiment validated the sample mean; a diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/PrimaryNightRestingHRDailyTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/PrimaryNightRestingHRDailyTests.swift new file mode 100644 index 0000000000..926b8515c2 --- /dev/null +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/PrimaryNightRestingHRDailyTests.swift @@ -0,0 +1,54 @@ +import XCTest +@testable import StrandAnalytics +import WhoopProtocol + +/// #2522: the daily RHR takes the primary night's five-minute floor, while the #2358 nap +/// selection and #804 provided-value precedence remain intact. Kotlin has the same fixtures. +final class PrimaryNightRestingHRDailyTests: XCTestCase { + private let day = "2026-07-27" + private var nightStart: Int { AnalyticsEngine.dayStartUtcSeconds(day) - 4 * 3600 } + private var napStart: Int { AnalyticsEngine.dayStartUtcSeconds(day) + 13 * 3600 } + private let profile = UserProfile(weightKg: 75, heightCm: 178, age: 30, sex: "male") + + private func sessions(primaryRHR: Int? = nil) -> [SleepSession] { + let nightEnd = nightStart + 600 * 60 + let napEnd = napStart + 40 * 60 + return [ + SleepSession(start: nightStart, end: nightEnd, efficiency: 0.9, + stages: [StageSegment(start: nightStart, end: nightEnd, stage: "light")], + restingHR: primaryRHR, avgHRV: nil), + SleepSession(start: napStart, end: napEnd, efficiency: 0.9, + stages: [StageSegment(start: napStart, end: napEnd, stage: "light")], + restingHR: nil, avgHRV: nil), + ] + } + + private func heartRate(includeNight: Bool = true) -> [HRSample] { + let night = includeNight ? (0..<1200).map { i in + HRSample(ts: nightStart + i * 30, bpm: i < 10 ? 55 : 70) + } : [] + let nap = (0..<80).map { i in HRSample(ts: napStart + i * 30, bpm: 45) } + return night + nap + } + + func testMainNightFloorWinsOverItsMeanAndALowerNap() { + let hr = heartRate() + XCTAssertEqual(SleepStager.sessionRestingHR(start: nightStart, end: nightStart + 600 * 60, hr: hr), 55) + XCTAssertGreaterThan(AnalyticsEngine.primarySessionRestingHR(sessions: sessions(), hr: hr) ?? 0, 69) + let result = AnalyticsEngine.analyzeDay(day: day, hr: hr, rr: [], profile: profile, + providedSleep: sessions()) + XCTAssertEqual(result.daily.restingHr, 55) + } + + func testProvidedPrimaryValueStillWins() { + let result = AnalyticsEngine.analyzeDay(day: day, hr: heartRate(), rr: [], profile: profile, + providedSleep: sessions(primaryRHR: 62)) + XCTAssertEqual(result.daily.restingHr, 62) + } + + func testNapDoesNotFillAnUnmeasuredPrimaryNight() { + let result = AnalyticsEngine.analyzeDay(day: day, hr: heartRate(includeNight: false), rr: [], + profile: profile, providedSleep: sessions()) + XCTAssertNil(result.daily.restingHr) + } +} 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 f0e87a5463..6bffbf7359 100644 --- a/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt +++ b/android/app/src/main/java/com/noop/analytics/AnalyticsEngine.kt @@ -576,15 +576,13 @@ object AnalyticsEngine { // only way to scope them is through the session set itself — which is precisely the "one forgotten // call site" a scattered filter invites. val physiologySessions = matched.filter { !it.hrOnly }.ifEmpty { matched } - // Resting Heart Rate: Use PrimarySessionRestingHR (arithmetic sample mean of the longest/primary - // sleep session, #1169), eliminating daytime nap floor distortion. - // #804: Preserve ring/device-provided resting HR when present in `providedSleep`. - // Cleanly falls back to physiologySessions.mapNotNull { it.restingHR }.minOrNull() when coverage is sparse. - val providedPrimaryRHR = physiologySessions.maxByOrNull { it.end - it.start } + // #2522: use the gated lowest five-minute bin from the primary session, not its whole-session + // mean. Choosing the session first preserves #2358's nap protection; a shorter nap must not + // supply the daily RHR when the main night has no HR. #804's device-provided value still wins. + val primarySession = physiologySessions.maxByOrNull { it.end - it.start } + val providedPrimaryRHR = primarySession ?.let { p -> providedSleep.firstOrNull { it.start == p.start && it.end == p.end }?.restingHR } - val restingHRDaily: Int? = providedPrimaryRHR - ?: primarySessionRestingHR(physiologySessions, hr)?.roundToInt() - ?: physiologySessions.mapNotNull { it.restingHR }.minOrNull() + val restingHRDaily: Int? = providedPrimaryRHR ?: primarySession?.restingHR // Daily avg HRV = in-bed-weighted mean of per-session avg HRV. val avgHRVDaily: Double? = if (deepHrvWindow) { // #141: WHOOP-style HRV — pool RMSSD over DEEP-stage 5-min windows only (slow-wave sleep), diff --git a/android/app/src/main/java/com/noop/analytics/PrimarySessionRestingHR.kt b/android/app/src/main/java/com/noop/analytics/PrimarySessionRestingHR.kt index 20e8d98e56..9d5d187524 100644 --- a/android/app/src/main/java/com/noop/analytics/PrimarySessionRestingHR.kt +++ b/android/app/src/main/java/com/noop/analytics/PrimarySessionRestingHR.kt @@ -1,27 +1,24 @@ package com.noop.analytics /** - * #1169: an alternative headline resting-HR definition — the arithmetic MEAN of valid HR samples in the - * LONGEST (primary) sleep session, rather than the lowest-per-session floor `AnalyticsEngine` ships today. + * #1169: a shadow resting-HR definition — the arithmetic MEAN of valid HR samples in the longest + * (primary) sleep session, rather than the five-minute floor `AnalyticsEngine` ships today. * * ## Why (issue #1169, artemc) - * The shipped daily RHR is `restingHRDaily = matched…restingHR.min()` — a nightly HR FLOOR, and the `.min()` - * lets a short low-HR nap replace the main overnight session. A clean-room, single-participant 5-night + * The original daily RHR took the minimum floor across sessions, letting a short low-HR nap replace + * the main overnight session. A clean-room, single-participant 5-night * experiment (official WHOOP RHR + a Polar H10 ECG mean as independent references, a pre-declared dev/holdout * split, no fitted offset) found the primary-session sample mean tracked both far better: rounded MAE vs the * official target 6.0->2.0 (dev) / 7.5->0.8 (holdout). * - * ## WIRED as of #2358 — this sets the shipped daily resting HR - * It was landed pure and unwired, on the reasoning that switching the consumers is a re-baselining of - * core scores and that the issue asks for a larger multi-participant holdout first. #2358 made the - * switch anyway, as a maintainer call: `AnalyticsEngine.restingHRDaily` now prefers a device-provided - * primary-session value, then THIS mean, and falls back to the old `restingHR.min()` floor only when - * coverage is sparse. So the headline resting HR and everything reading it — recovery, strain, workout - * detection, energy — come from here on any day with a covered primary session. + * ## Shadow metric after #2522 + * #2358 temporarily wired this mean into the daily headline while selecting the primary session to + * avoid the nap problem. #2522 keeps that session selection but takes its existing five-minute floor. + * This mean remains in `rhr_primary_session` for comparison and does not feed scoring. * * What that means for the evidence below: the MAE figures are from ONE participant over five nights * against a pre-declared split, which is the holdout the issue says is not yet large enough. #2284 - * separately replaces what a session's `restingHR` IS, which moves the fallback under this. + * proposes changing a session's `restingHR` definition separately; this shadow comparison does not enact it. * * ## Definition * - **Primary session**: the LONGEST by duration; ties resolve to the FIRST. A nap never replaces the night. diff --git a/android/app/src/test/java/com/noop/analytics/PrimaryNightRestingHRDailyTest.kt b/android/app/src/test/java/com/noop/analytics/PrimaryNightRestingHRDailyTest.kt new file mode 100644 index 0000000000..8b684874ad --- /dev/null +++ b/android/app/src/test/java/com/noop/analytics/PrimaryNightRestingHRDailyTest.kt @@ -0,0 +1,58 @@ +package com.noop.analytics + +import com.noop.data.HrSample +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import java.time.LocalDate +import java.time.ZoneOffset + +/** #2522 daily-RHR parity twin of Swift PrimaryNightRestingHRDailyTests. */ +class PrimaryNightRestingHRDailyTest { + private val day = "2026-07-27" + private val dayStart = LocalDate.parse(day).atStartOfDay(ZoneOffset.UTC).toEpochSecond() + private val nightStart = dayStart - 4 * 3600 + private val napStart = dayStart + 13 * 3600 + private val profile = UserProfile(weightKg = 75.0, heightCm = 178.0, age = 30.0, sex = "male") + + private fun sessions(primaryRhr: Int? = null): List { + val nightEnd = nightStart + 600 * 60 + val napEnd = napStart + 40 * 60 + return listOf( + DetectedSleep(nightStart, nightEnd, 0.9, + listOf(StageSegment(nightStart, nightEnd, "light")), restingHR = primaryRhr, avgHRV = null), + DetectedSleep(napStart, napEnd, 0.9, + listOf(StageSegment(napStart, napEnd, "light")), restingHR = null, avgHRV = null), + ) + } + + private fun heartRate(includeNight: Boolean = true): List { + val night = if (includeNight) (0 until 1200).map { i -> + HrSample("t", nightStart + i * 30L, if (i < 10) 55 else 70) + } else emptyList() + val nap = (0 until 80).map { i -> HrSample("t", napStart + i * 30L, 45) } + return night + nap + } + + @Test fun mainNightFloorWinsOverItsMeanAndALowerNap() { + val hr = heartRate() + assertEquals(55, SleepStager.sessionRestingHR(nightStart, nightStart + 600 * 60, hr)) + assertTrue((AnalyticsEngine.primarySessionRestingHR(sessions(), hr) ?: 0.0) > 69.0) + val result = AnalyticsEngine.analyzeDay(day = day, hr = hr, rr = emptyList(), + profile = profile, providedSleep = sessions()) + assertEquals(55, result.daily.restingHr) + } + + @Test fun providedPrimaryValueStillWins() { + val result = AnalyticsEngine.analyzeDay(day = day, hr = heartRate(), rr = emptyList(), + profile = profile, providedSleep = sessions(primaryRhr = 62)) + assertEquals(62, result.daily.restingHr) + } + + @Test fun napDoesNotFillAnUnmeasuredPrimaryNight() { + val result = AnalyticsEngine.analyzeDay(day = day, hr = heartRate(includeNight = false), + rr = emptyList(), profile = profile, providedSleep = sessions()) + assertNull(result.daily.restingHr) + } +}