From 4662805639f4bfcb47d10e4b63fcb3c2ad8b3edc Mon Sep 17 00:00:00 2001 From: ryanbr Date: Thu, 30 Jul 2026 19:05:13 -0700 Subject: [PATCH] =?UTF-8?q?Circadian:=20the=20input=20is=20heart=20rate,?= =?UTF-8?q?=20not=20motion=20=E2=80=94=20say=20so,=20and=20pin=20what=20th?= =?UTF-8?q?e=20gate=20costs=20(#982)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ActivityBin.activity was documented as "the motion volume in that bin (e.g. StepsEstimateEngine.dayMotionIntensity per hour)". It has never been fed that. The only production caller pools per-hour MEAN HEART RATE in bpm (AppModel.swift, sums[hour] += b.bpm), which is the right choice on this hardware — WHOOP 4.0 motion is too sparse to stage sleep at all (#345) — but the doc has been telling readers the opposite since it was written. That matters because minRelativeAmplitude gates on amplitude / |mesor|, and HR arrives with a ~45-75 bpm DC offset a motion volume does not have. The effective bar is therefore an absolute 0.10 x mesor bpm. NOT re-tuned to an absolute floor, deliberately. #982 raised this as the gate penalising the fittest wearers, and that direction does not hold: because the bar scales WITH the mesor, a low-resting wearer faces a LOWER absolute requirement, not a higher one. The same 5 bpm swing is arrhythmic at a 65 bpm mesor and readable at 45. The concern only applies to someone whose amplitude is disproportionately small for their mesor, which is a single observation, and nobody is currently silenced by the gate. Shipping an invented bpm constant into everyone's analytics on that basis is exactly what the project's own rule about unvalidated tuning warns against. So: correct the record, and make the trade executable instead of arguable. A parity test pair pins what the gate costs at 65 and at 45 bpm, including the pair that shows the direction, so whoever does have the data to re-tune it starts from a fact. Both platforms; docs and tests only, no behaviour change. --- .../StrandAnalytics/CircadianEngine.swift | 25 +++++++++++++-- .../CircadianEngineTests.swift | 29 +++++++++++++++++ .../com/noop/analytics/CircadianEngine.kt | 16 +++++++++- .../com/noop/analytics/CircadianEngineTest.kt | 31 +++++++++++++++++++ 4 files changed, 98 insertions(+), 3 deletions(-) diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/CircadianEngine.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/CircadianEngine.swift index b2e5f560e..f6278a238 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/CircadianEngine.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/CircadianEngine.swift @@ -27,6 +27,18 @@ public enum CircadianEngine { /// Days at/above which the fit reads as full-confidence. public static let goodDaysForFit: Int = 14 /// A cosinor fit with amplitude below this fraction of the mesor is "arrhythmic" — too flat to phase. + /// + /// #982 — RELATIVE, applied to a signal (mean HR, see `ActivityBin`) whose mesor is ~45-75 bpm rather + /// than the near-zero mesor a motion volume would have. So the effective bar is an absolute amplitude + /// of `0.10 x mesor`: about 6.5 bpm at a 65 bpm mesor, about 4.5 bpm at 45. It scales WITH the mesor, + /// so a low-resting wearer faces a LOWER absolute bar, not a higher one — the opposite of the concern + /// raised in #982, which assumed the gate penalises the fittest. + /// + /// Deliberately NOT re-tuned to an absolute bpm floor. The shape is arguably wrong for this domain, + /// but every candidate value is unvalidated, nobody is currently silenced by it, and the direction of + /// the harm is not established (it needs a wearer whose amplitude is disproportionately small for + /// their mesor — an n=1 observation). `CircadianEngineTests` pins what the gate costs at each mesor so + /// the trade is visible to whoever does have the data to change it. public static let minRelativeAmplitude: Double = 0.10 /// Max clock-shift the planner steps per day (hours) — the well-established ~1 h/day re-entrainment rate. public static let maxShiftPerDayHours: Double = 1.0 @@ -39,8 +51,17 @@ public enum CircadianEngine { // MARK: - Inputs - /// One per-hour rest-activity sample: the local clock hour (0..<24, may be fractional) and the motion - /// volume in that bin (e.g. StepsEstimateEngine.dayMotionIntensity per hour). Higher = more active. + /// One per-hour rest-activity sample: the local clock hour (0..<24, may be fractional) and the + /// rhythm signal in that bin. Higher = more active. + /// + /// #982 — this said "motion volume (e.g. StepsEstimateEngine.dayMotionIntensity per hour)". It has + /// never been fed that. The only production caller pools per-hour MEAN HEART RATE in bpm + /// (`AppModel.swift`, `sums[hour] += b.bpm`), and that is the right choice on this hardware: WHOOP 4.0 + /// motion is too sparse to stage sleep at all (#345), while HR carries a strong circadian rhythm. + /// + /// The domain matters because `minRelativeAmplitude` gates on `amplitude / |mesor|`, and HR arrives + /// with a large DC offset that motion does not have — see that constant for what the gate actually + /// costs in bpm. public struct ActivityBin: Equatable, Sendable { public let hour: Double public let activity: Double diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/CircadianEngineTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/CircadianEngineTests.swift index 95382a455..6c2f57b17 100644 --- a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/CircadianEngineTests.swift +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/CircadianEngineTests.swift @@ -122,4 +122,33 @@ final class CircadianEngineTests: XCTestCase { XCTAssertEqual(CircadianEngine.clock(-1.0), "23:00") // wraps XCTAssertEqual(CircadianEngine.clock(7.25), "07:15") } + + // MARK: - #982: what the RELATIVE gate costs, in bpm, at a real HR mesor + + /// The engine is fed mean HEART RATE, not the motion volume its doc used to claim, and + /// `minRelativeAmplitude` gates on `amplitude / |mesor|`. Against a signal carrying a ~45-75 bpm DC + /// offset that makes the real bar an ABSOLUTE `0.10 x mesor` bpm — which these pin. + /// + /// The pair that matters is the last two: the SAME 5 bpm swing is arrhythmic at a 65 bpm mesor and + /// readable at 45. The bar scales WITH the mesor, so a low-resting wearer faces a LOWER absolute + /// requirement, not a higher one. #982 raised the opposite concern — that the gate penalises the + /// fittest — and that only holds for someone whose amplitude is disproportionately small for their + /// mesor. Pinned here so the direction is a fact rather than an argument, for whoever re-tunes it. + private func confidence(mesor: Double, amp: Double) -> CircadianEngine.PhaseConfidence { + CircadianEngine.estimatePhase(bins: profile(mesor: mesor, amp: amp, acrophase: 16), + daysObserved: CircadianEngine.goodDaysForFit, + habitualWakeHour: 7)!.confidence + } + + func testTypicalMesorNeedsAboutSixAndAHalfBpmOfSwing() { + XCTAssertNotEqual(confidence(mesor: 65, amp: 8), .unreadable) // 0.123 - clears 0.10 + XCTAssertEqual(confidence(mesor: 65, amp: 5), .unreadable) // 0.077 - does not + } + + func testTheSameSwingIsReadableAtALowerMesor() { + // 5 bpm: arrhythmic at 65, rhythmic at 45. The gate favours the low-resting wearer. + XCTAssertEqual(confidence(mesor: 65, amp: 5), .unreadable) + XCTAssertNotEqual(confidence(mesor: 45, amp: 5), .unreadable) // 0.111 + XCTAssertEqual(confidence(mesor: 45, amp: 4), .unreadable) // 0.089 + } } diff --git a/android/app/src/main/java/com/noop/analytics/CircadianEngine.kt b/android/app/src/main/java/com/noop/analytics/CircadianEngine.kt index 473d12d43..1b3a5f625 100644 --- a/android/app/src/main/java/com/noop/analytics/CircadianEngine.kt +++ b/android/app/src/main/java/com/noop/analytics/CircadianEngine.kt @@ -27,6 +27,13 @@ object CircadianEngine { // ── Tuning constants (pinned by test; mirror the Swift twin exactly) ── const val minDaysForFit: Int = 7 const val goodDaysForFit: Int = 14 + /** + * #982 — RELATIVE, applied to mean HR (mesor ~45-75 bpm), not to a near-zero-mesor motion volume. The + * effective bar is `0.10 x mesor`: ~6.5 bpm at a 65 bpm mesor, ~4.5 bpm at 45. It scales WITH the + * mesor, so a low-resting wearer faces a LOWER absolute bar — the opposite of the concern in #982. + * Deliberately NOT re-tuned: every candidate value is unvalidated and nobody is currently silenced. + * `CircadianEngineTest` pins what it costs at each mesor. Mirrors the Swift twin exactly. + */ const val minRelativeAmplitude: Double = 0.10 const val maxShiftPerDayHours: Double = 1.0 const val cbtMinBeforeWakeHours: Double = 2.5 @@ -34,7 +41,14 @@ object CircadianEngine { // ── Inputs ── - /** One per-hour rest-activity sample: local clock hour (0..<24, may be fractional) + motion volume. */ + /** + * One per-hour rest-activity sample: local clock hour (0..<24, may be fractional) + the rhythm signal. + * + * #982 — this said "motion volume". It has never been fed that: the only production caller pools + * per-hour MEAN HEART RATE in bpm, and that is the right choice on this hardware (WHOOP 4.0 motion is + * too sparse to stage sleep at all, #345). The domain matters because [minRelativeAmplitude] gates on + * `amplitude / |mesor|` and HR arrives with a large DC offset motion does not have — see that constant. + */ data class ActivityBin(val hour: Double, val activity: Double) // ── Cosinor ── diff --git a/android/app/src/test/java/com/noop/analytics/CircadianEngineTest.kt b/android/app/src/test/java/com/noop/analytics/CircadianEngineTest.kt index f8664d941..c4e9cf238 100644 --- a/android/app/src/test/java/com/noop/analytics/CircadianEngineTest.kt +++ b/android/app/src/test/java/com/noop/analytics/CircadianEngineTest.kt @@ -107,4 +107,35 @@ class CircadianEngineTest { assertEquals("23:00", CircadianEngine.clock(-1.0)) assertEquals("07:15", CircadianEngine.clock(7.25)) } + + // ── #982: what the RELATIVE gate costs, in bpm, at a real HR mesor ── + + /** + * The engine is fed mean HEART RATE, not the motion volume its doc used to claim, and + * [CircadianEngine.minRelativeAmplitude] gates on `amplitude / |mesor|`. Against a signal carrying a + * ~45-75 bpm DC offset that makes the real bar an ABSOLUTE `0.10 x mesor` bpm. + * + * The pair that matters is [theSameSwingIsReadableAtALowerMesor]: the SAME 5 bpm swing is arrhythmic + * at a 65 bpm mesor and readable at 45, so the bar scales WITH the mesor and a low-resting wearer + * faces a LOWER absolute requirement. #982 raised the opposite concern. Pinned so the direction is a + * fact rather than an argument. Twin of the Swift `CircadianEngineTests` pair. + */ + private fun confidence(mesor: Double, amp: Double): CircadianEngine.PhaseConfidence = + CircadianEngine.estimatePhase( + profile(mesor, amp, 16.0), + CircadianEngine.goodDaysForFit, + 7.0, + )!!.confidence + + @Test fun typicalMesorNeedsAboutSixAndAHalfBpmOfSwing() { + assertTrue(confidence(65.0, 8.0) != CircadianEngine.PhaseConfidence.UNREADABLE) // 0.123 + assertEquals(CircadianEngine.PhaseConfidence.UNREADABLE, confidence(65.0, 5.0)) // 0.077 + } + + @Test fun theSameSwingIsReadableAtALowerMesor() { + // 5 bpm: arrhythmic at 65, rhythmic at 45. The gate favours the low-resting wearer. + assertEquals(CircadianEngine.PhaseConfidence.UNREADABLE, confidence(65.0, 5.0)) + assertTrue(confidence(45.0, 5.0) != CircadianEngine.PhaseConfidence.UNREADABLE) // 0.111 + assertEquals(CircadianEngine.PhaseConfidence.UNREADABLE, confidence(45.0, 4.0)) // 0.089 + } }