From 85a1f072d627b8b16bcfc32a75715292036d7705 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sat, 1 Aug 2026 00:58:15 -0700 Subject: [PATCH 1/4] HRV: default Overnight only ON for fresh installs (#1008, minimum half) WHOOP publishes no daytime HRV figure - its reading is an overnight one - so a 24/7 stream has no official-app analogue, and the setting's own copy says overnight-only roughly halves the battery cost. Turning on "Continuous HRV capture" currently gives you the expensive, non-WHOOP-like behaviour unless you separately find "Overnight only", which is the wrong way round. EXISTING USERS ARE UNCHANGED. The unset case resolves from whether the base Continuous HRV key exists: present means the user has been through this screen and experienced always-on, so they keep it; absent means a fresh install, which gets overnight-only. That preserves the deliberate choice the #927 comment records ("Default OFF, so existing Continuous HRV users keep the always-on behaviour with no migration") while changing what new users get. Resolved at read time rather than by writing a migration, because the only thing that must not happen is silently narrowing capture for someone relying on it for daytime Stress - and a migration that runs at the wrong moment does exactly that. An explicit choice always wins in both directions, including an explicit OFF on a fresh install. This is the SAFE half of #1008. The other half - defaulting the HRV window to DEEP_SLEEP to match WHOOP's reading - is deliberately not here: it moves every existing HRV number and its baselines, and it depends on deep-sleep staging quality that is still unsettled on 5/MG. That needs SleepPSG evidence first. Rule lifted into continuousHrvOvernightDefault on both platforms so it is testable without a Context or UserDefaults. Four twin tests each; the Kotlin ones run in CI, the Swift ones live in StrandTests which app-build.yml would run if it were enabled. --- Strand/BLE/PuffinExperiment.swift | 34 ++++++++++- .../ContinuousHrvOvernightDefaultTests.swift | 42 +++++++++++++ .../src/main/java/com/noop/ui/MainActivity.kt | 47 ++++++++++++-- .../ui/ContinuousHrvOvernightDefaultTest.kt | 61 +++++++++++++++++++ 4 files changed, 176 insertions(+), 8 deletions(-) create mode 100644 StrandTests/ContinuousHrvOvernightDefaultTests.swift create mode 100644 android/app/src/test/java/com/noop/ui/ContinuousHrvOvernightDefaultTest.kt diff --git a/Strand/BLE/PuffinExperiment.swift b/Strand/BLE/PuffinExperiment.swift index 0e380b91bb..d1d6026084 100644 --- a/Strand/BLE/PuffinExperiment.swift +++ b/Strand/BLE/PuffinExperiment.swift @@ -48,12 +48,42 @@ enum PuffinExperiment { /// only inside the nightly window (the reused quiet-hours window convention: minutes since local /// midnight, wrap-aware, 22:00 to 07:00 by default) instead of 24/7, roughly halving the battery /// cost. Composed with the base toggle so existing users need no migration: base on + this off reads - /// ALWAYS (the pre-#927 behaviour). Default OFF. Read by BLEManager at EVERY arm site (re-derived at + /// ALWAYS (the pre-#927 behaviour). Defaults ON for fresh installs, OFF once Continuous HRV has been + /// used (#1008) — see below. Read by BLEManager at EVERY arm site (re-derived at /// arm time, never precomputed; see ContinuousHrvSchedule). Mirrors the Android /// `NoopPrefs.KEY_CONTINUOUS_HRV_OVERNIGHT`. static let continuousHrvOvernightOnlyKey = "noopContinuousHrvOvernightOnly" - static var continuousHrvOvernightOnlyEnabled: Bool { UserDefaults.standard.bool(forKey: continuousHrvOvernightOnlyKey) } + /// Defaults to ON for anyone who has never touched Continuous HRV, and to OFF for anyone who has + /// (#1008). WHOOP publishes no daytime HRV figure at all — its reading is an overnight one — so a + /// 24/7 stream has no official-app analogue, and overnight-only roughly halves the battery cost. + /// Making the cheaper, WHOOP-comparable behaviour the default is the point; the expensive one stays + /// a deliberate choice. + /// + /// `UserDefaults.bool(forKey:)` cannot express this on its own: it returns `false` for a missing key, + /// which is indistinguishable from an explicit off. The unset case is therefore resolved from whether + /// `keepRealtimeForDataKey` exists, rather than by writing a migration — the only thing that must not + /// happen is silently narrowing capture for someone already relying on it. Twin of the Android + /// `NoopPrefs.continuousHrvOvernight`. + static var continuousHrvOvernightOnlyEnabled: Bool { + let defaults = UserDefaults.standard + return continuousHrvOvernightDefault( + hasExplicitChoice: defaults.object(forKey: continuousHrvOvernightOnlyKey) != nil, + explicitChoice: defaults.bool(forKey: continuousHrvOvernightOnlyKey), + hasUsedContinuousHrv: defaults.object(forKey: keepRealtimeForDataKey) != nil) + } + + /// The rule behind `continuousHrvOvernightOnlyEnabled`, lifted out so it can be tested without + /// touching `UserDefaults`. Twin of the Android `NoopPrefs.continuousHrvOvernightDefault`. + /// + /// An explicit choice always wins. With no choice recorded, `hasUsedContinuousHrv` decides: someone + /// who has been through this screen keeps the always-on behaviour they experienced, a fresh install + /// gets overnight-only. + static func continuousHrvOvernightDefault(hasExplicitChoice: Bool, + explicitChoice: Bool, + hasUsedContinuousHrv: Bool) -> Bool { + hasExplicitChoice ? explicitChoice : !hasUsedContinuousHrv + } // MARK: - Power saving (#477), parity with Android NoopPrefs diff --git a/StrandTests/ContinuousHrvOvernightDefaultTests.swift b/StrandTests/ContinuousHrvOvernightDefaultTests.swift new file mode 100644 index 0000000000..f8d590ab4d --- /dev/null +++ b/StrandTests/ContinuousHrvOvernightDefaultTests.swift @@ -0,0 +1,42 @@ +import XCTest +@testable import Strand + +/// #1008 — which way "Overnight only" falls when the user has never chosen. Twin of the Kotlin +/// `ContinuousHrvOvernightDefaultTest`; same cases in the same order. +/// +/// WHOOP publishes no daytime HRV figure, so a 24/7 stream has no official-app analogue and costs +/// roughly twice the battery. The cheaper, WHOOP-comparable behaviour should be the default — but only +/// for someone not already running the other one. +/// +/// The rule that must not break: an existing Continuous HRV user's capture is never silently narrowed. +/// They opted into "all day and night" and may be reading daytime Stress off it. +/// +/// Note: `StrandTests` runs only under `xcodebuild` on macOS, and `app-build.yml` is disabled — so this +/// suite is not executed by CI today. The Kotlin twin is, under `testFullDebugUnitTest`. +final class ContinuousHrvOvernightDefaultTests: XCTestCase { + + /// A fresh install gets the WHOOP-comparable, cheaper default. This is the change. + func testFreshInstallDefaultsToOvernightOnly() { + XCTAssertTrue(PuffinExperiment.continuousHrvOvernightDefault( + hasExplicitChoice: false, explicitChoice: false, hasUsedContinuousHrv: false)) + } + + /// The regression guard: an existing Continuous HRV user keeps always-on. Narrowing it under them + /// would remove the daytime data they opted in for, without asking. + func testAnExistingContinuousHrvUserKeepsAlwaysOn() { + XCTAssertFalse(PuffinExperiment.continuousHrvOvernightDefault( + hasExplicitChoice: false, explicitChoice: false, hasUsedContinuousHrv: true)) + } + + /// An explicit ON wins over anything the install age would imply. + func testAnExplicitOnIsHonoured() { + XCTAssertTrue(PuffinExperiment.continuousHrvOvernightDefault( + hasExplicitChoice: true, explicitChoice: true, hasUsedContinuousHrv: true)) + } + + /// An explicit OFF wins too, including on a fresh install — the mirror of the guard above. + func testAnExplicitOffIsHonouredEvenOnAFreshInstall() { + XCTAssertFalse(PuffinExperiment.continuousHrvOvernightDefault( + hasExplicitChoice: true, explicitChoice: false, hasUsedContinuousHrv: false)) + } +} diff --git a/android/app/src/main/java/com/noop/ui/MainActivity.kt b/android/app/src/main/java/com/noop/ui/MainActivity.kt index 11f6e92580..abcd50a043 100644 --- a/android/app/src/main/java/com/noop/ui/MainActivity.kt +++ b/android/app/src/main/java/com/noop/ui/MainActivity.kt @@ -181,8 +181,9 @@ object NoopPrefs { /** "Overnight only" refinement of Continuous HRV capture (#927): when on (with [KEY_CONTINUOUS_HRV]), * the dense realtime stream is armed only inside the nightly quiet-hours window (22:00 to 07:00 by - * default, wrap-aware, local wall time) instead of 24/7, roughly halving the battery cost. Default - * OFF, so existing Continuous HRV users keep the always-on behaviour with no migration. Read by + * default, wrap-aware, local wall time) instead of 24/7, roughly halving the battery cost. Defaults + * ON for fresh installs and OFF for anyone who has already used Continuous HRV (#1008), so existing + * users keep the always-on behaviour with no migration. Read by * [com.noop.ble.WhoopBleClient] at every arm site (re-derived at arm time, never cached). */ const val KEY_CONTINUOUS_HRV_OVERNIGHT = "noop.continuousHrvOvernight" @@ -315,10 +316,44 @@ object NoopPrefs { of(context).edit().putBoolean(KEY_CONTINUOUS_HRV, enabled).apply() } - /** Whether Continuous HRV capture arms the stream only inside the nightly window (#927). Default - * false = always-on, the pre-#927 behaviour. */ - fun continuousHrvOvernight(context: Context): Boolean = - of(context).getBoolean(KEY_CONTINUOUS_HRV_OVERNIGHT, false) + /** + * Whether Continuous HRV capture arms the stream only inside the nightly window (#927). + * + * Defaults to ON for anyone who has never touched Continuous HRV, and to OFF for anyone who has + * (#1008). WHOOP publishes no daytime HRV figure at all — its reading is an overnight one — so a + * 24/7 stream has no official-app analogue, and the setting's own copy says overnight-only roughly + * halves the battery cost. Making the cheaper, WHOOP-comparable behaviour the one you get by + * default is the point; the expensive one stays a deliberate choice. + * + * The unset case is resolved from whether [KEY_CONTINUOUS_HRV] exists rather than by writing a + * migration, because the ONLY thing that must not happen is silently narrowing capture for someone + * already relying on it. Presence of that key means the user has been through this screen and + * experienced always-on; absence means a fresh install, which gets the new default. A user who + * toggled the base setting on and back off keeps always-on too — conservative on purpose, since + * they have seen the old behaviour. + */ + fun continuousHrvOvernight(context: Context): Boolean { + val prefs = of(context) + return continuousHrvOvernightDefault( + hasExplicitChoice = prefs.contains(KEY_CONTINUOUS_HRV_OVERNIGHT), + explicitChoice = prefs.getBoolean(KEY_CONTINUOUS_HRV_OVERNIGHT, true), + hasUsedContinuousHrv = prefs.contains(KEY_CONTINUOUS_HRV), + ) + } + + /** + * The rule behind [continuousHrvOvernight], lifted out so it can be tested without a `Context`. + * Twin of the Swift `PuffinExperiment.continuousHrvOvernightDefault`. + * + * An explicit choice always wins. With no choice recorded, [hasUsedContinuousHrv] decides: someone + * who has been through this screen keeps the always-on behaviour they experienced, a fresh install + * gets overnight-only. + */ + internal fun continuousHrvOvernightDefault( + hasExplicitChoice: Boolean, + explicitChoice: Boolean, + hasUsedContinuousHrv: Boolean, + ): Boolean = if (hasExplicitChoice) explicitChoice else !hasUsedContinuousHrv fun setContinuousHrvOvernight(context: Context, enabled: Boolean) { of(context).edit().putBoolean(KEY_CONTINUOUS_HRV_OVERNIGHT, enabled).apply() diff --git a/android/app/src/test/java/com/noop/ui/ContinuousHrvOvernightDefaultTest.kt b/android/app/src/test/java/com/noop/ui/ContinuousHrvOvernightDefaultTest.kt new file mode 100644 index 0000000000..a154270cfe --- /dev/null +++ b/android/app/src/test/java/com/noop/ui/ContinuousHrvOvernightDefaultTest.kt @@ -0,0 +1,61 @@ +package com.noop.ui + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * #1008 — which way "Overnight only" falls when the user has never chosen. + * + * WHOOP publishes no daytime HRV figure, so a 24/7 stream has no official-app analogue and costs + * roughly twice the battery. The cheaper, WHOOP-comparable behaviour should be the one you get by + * default — but ONLY for someone who has not already been running the other one. + * + * The rule that must not break: an existing Continuous HRV user's capture is never silently narrowed. + * They opted into "all day and night" and may be reading daytime Stress off it. + */ +class ContinuousHrvOvernightDefaultTest { + + /** A fresh install gets the WHOOP-comparable, cheaper default. This is the change. */ + @Test fun freshInstallDefaultsToOvernightOnly() { + assertTrue( + NoopPrefs.continuousHrvOvernightDefault( + hasExplicitChoice = false, explicitChoice = false, hasUsedContinuousHrv = false, + ), + ) + } + + /** + * The regression guard. Someone who already enabled Continuous HRV keeps always-on — narrowing it + * under them would remove the daytime data they opted in for, without asking. + */ + @Test fun anExistingContinuousHrvUserKeepsAlwaysOn() { + assertFalse( + NoopPrefs.continuousHrvOvernightDefault( + hasExplicitChoice = false, explicitChoice = false, hasUsedContinuousHrv = true, + ), + ) + } + + /** An explicit ON wins over anything the install age would imply. */ + @Test fun anExplicitOnIsHonoured() { + assertTrue( + NoopPrefs.continuousHrvOvernightDefault( + hasExplicitChoice = true, explicitChoice = true, hasUsedContinuousHrv = true, + ), + ) + } + + /** + * An explicit OFF wins too — including on a fresh install. Without this the new default would + * override someone who deliberately turned overnight-only off, which is the mirror of the bug the + * second test guards. + */ + @Test fun anExplicitOffIsHonouredEvenOnAFreshInstall() { + assertFalse( + NoopPrefs.continuousHrvOvernightDefault( + hasExplicitChoice = true, explicitChoice = false, hasUsedContinuousHrv = false, + ), + ) + } +} From acc9241890d278d45463f5ff2a71e883d9fbfe7a Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sat, 1 Aug 2026 01:12:34 -0700 Subject: [PATCH 2/4] Fix the default flip: decide once at launch, not on every read The first version of this resolved the default at READ time, keyed on whether Continuous HRV had ever been enabled. That fact is created by the user's own opt-in, so a fresh install read overnight-only ON and then flipped to OFF the moment they enabled Continuous HRV - the exact opposite of the intent. State-by-state tests could not see it: every individual state was correct, and only the sequence was wrong. Replaced with a one-time launch migration on both platforms: an install that has used Continuous HRV and never chose an overnight setting is pinned to the old OFF; everything else is left alone and takes the new ON default. Taken before the user can reach either toggle, so the inputs cannot move under it. A @AppStorage onChange hook cannot substitute on iOS - @AppStorage writes the value BEFORE the handler runs, so a first-ever toggle is indistinguishable from any other by then. That asymmetry is why this is a launch migration rather than something wired to the toggle. The extracted rule now describes the MIGRATION decision rather than the read, because the read is simply getBoolean(key, true) - the previous extraction had become dead code describing the broken design, and its tests were exercising that dead code rather than production. Five twin tests each, including the sequence that broke it and idempotence. --- Strand/App/StrandApp.swift | 3 + Strand/BLE/PuffinExperiment.swift | 45 ++++++++++- .../ContinuousHrvOvernightDefaultTests.swift | 46 +++++++---- StrandiOS/App/StrandiOSApp.swift | 3 + .../src/main/java/com/noop/NoopApplication.kt | 3 + .../src/main/java/com/noop/ui/MainActivity.kt | 56 +++++++++---- .../ui/ContinuousHrvOvernightDefaultTest.kt | 80 ++++++++++++------- 7 files changed, 172 insertions(+), 64 deletions(-) diff --git a/Strand/App/StrandApp.swift b/Strand/App/StrandApp.swift index d1462e1779..94bc724395 100644 --- a/Strand/App/StrandApp.swift +++ b/Strand/App/StrandApp.swift @@ -5,6 +5,9 @@ import UserNotifications @main struct StrandApp: App { init() { + // #1008: pin the pre-change Overnight-only default for existing installs before + // anything reads it. Idempotent; a no-op on fresh installs and after the first launch. + PuffinExperiment.migrateContinuousHrvOvernightDefault() #if DEBUG // DEBUG-only promo-screenshot harness: when launched with `--demo-hour `, pin the Today // screen to that hour's day-cycle scene + a plausible per-hour stat frame. Runs synchronously diff --git a/Strand/BLE/PuffinExperiment.swift b/Strand/BLE/PuffinExperiment.swift index d1d6026084..59860c12d3 100644 --- a/Strand/BLE/PuffinExperiment.swift +++ b/Strand/BLE/PuffinExperiment.swift @@ -66,11 +66,48 @@ enum PuffinExperiment { /// happen is silently narrowing capture for someone already relying on it. Twin of the Android /// `NoopPrefs.continuousHrvOvernight`. static var continuousHrvOvernightOnlyEnabled: Bool { + UserDefaults.standard.object(forKey: continuousHrvOvernightOnlyKey) as? Bool ?? true + } + + /// One-time migration for the #1008 default flip. Called once at launch, BEFORE anything reads the + /// setting, and before the user can reach the toggle. + /// + /// The default moved from OFF to ON, so an install that predates the change has to be pinned to OFF + /// explicitly or it would be silently narrowed to overnight-only capture — removing daytime data the + /// user opted in for. "Predates the change" is read as "has ever toggled Continuous HRV", i.e. the + /// base key exists. + /// + /// Deciding this at READ time instead does not work, and the way it fails is worth recording: the + /// discriminator would be the base key, which the user's own opt-in creates — so a fresh install + /// would default to ON and then flip to OFF the moment they enabled Continuous HRV, the exact + /// opposite of the intent. + /// + /// A `@AppStorage` `onChange` hook cannot substitute for this either: `@AppStorage` writes the value + /// BEFORE the handler runs, so by then a first-ever toggle is indistinguishable from any other. + /// + /// Idempotent: writes only when the overnight key is absent and the base key is present. + /// Twin of the Android `NoopPrefs.migrateContinuousHrvOvernightDefault`. + static func migrateContinuousHrvOvernightDefault() { let defaults = UserDefaults.standard - return continuousHrvOvernightDefault( - hasExplicitChoice: defaults.object(forKey: continuousHrvOvernightOnlyKey) != nil, - explicitChoice: defaults.bool(forKey: continuousHrvOvernightOnlyKey), - hasUsedContinuousHrv: defaults.object(forKey: keepRealtimeForDataKey) != nil) + guard shouldPinLegacyOvernightDefault( + hasOvernightChoice: defaults.object(forKey: continuousHrvOvernightOnlyKey) != nil, + hasUsedContinuousHrv: defaults.object(forKey: keepRealtimeForDataKey) != nil) else { return } + defaults.set(false, forKey: continuousHrvOvernightOnlyKey) + } + + /// The migration's decision, lifted out so it is testable without touching `UserDefaults`. Twin of + /// the Android `NoopPrefs.shouldPinLegacyOvernightDefault`. + /// + /// Pin the OLD default only for an install that has used Continuous HRV and never chose an overnight + /// setting. Everything else is left alone. + /// + /// Note what this is NOT keyed on: the READ. An earlier attempt resolved the default at read time + /// from `hasUsedContinuousHrv`, which the user's own opt-in creates — so a fresh install read ON and + /// then flipped to OFF the moment Continuous HRV was enabled. Running the decision once at launch is + /// what makes the answer stable. + static func shouldPinLegacyOvernightDefault(hasOvernightChoice: Bool, + hasUsedContinuousHrv: Bool) -> Bool { + !hasOvernightChoice && hasUsedContinuousHrv } /// The rule behind `continuousHrvOvernightOnlyEnabled`, lifted out so it can be tested without diff --git a/StrandTests/ContinuousHrvOvernightDefaultTests.swift b/StrandTests/ContinuousHrvOvernightDefaultTests.swift index f8d590ab4d..ab37ff36fa 100644 --- a/StrandTests/ContinuousHrvOvernightDefaultTests.swift +++ b/StrandTests/ContinuousHrvOvernightDefaultTests.swift @@ -15,28 +15,40 @@ import XCTest /// suite is not executed by CI today. The Kotlin twin is, under `testFullDebugUnitTest`. final class ContinuousHrvOvernightDefaultTests: XCTestCase { - /// A fresh install gets the WHOOP-comparable, cheaper default. This is the change. - func testFreshInstallDefaultsToOvernightOnly() { - XCTAssertTrue(PuffinExperiment.continuousHrvOvernightDefault( - hasExplicitChoice: false, explicitChoice: false, hasUsedContinuousHrv: false)) + /// The case the migration exists for: used the feature, never chose — pin the old default. + func testAnExistingContinuousHrvUserIsPinnedToAlwaysOn() { + XCTAssertTrue(PuffinExperiment.shouldPinLegacyOvernightDefault( + hasOvernightChoice: false, hasUsedContinuousHrv: true)) } - /// The regression guard: an existing Continuous HRV user keeps always-on. Narrowing it under them - /// would remove the daytime data they opted in for, without asking. - func testAnExistingContinuousHrvUserKeepsAlwaysOn() { - XCTAssertFalse(PuffinExperiment.continuousHrvOvernightDefault( - hasExplicitChoice: false, explicitChoice: false, hasUsedContinuousHrv: true)) + /// A fresh install is left alone, so the read picks up the new ON default. + func testAFreshInstallIsLeftAloneAndTakesTheNewDefault() { + XCTAssertFalse(PuffinExperiment.shouldPinLegacyOvernightDefault( + hasOvernightChoice: false, hasUsedContinuousHrv: false)) } - /// An explicit ON wins over anything the install age would imply. - func testAnExplicitOnIsHonoured() { - XCTAssertTrue(PuffinExperiment.continuousHrvOvernightDefault( - hasExplicitChoice: true, explicitChoice: true, hasUsedContinuousHrv: true)) + /// An explicit choice is never overwritten, whichever way it points. + func testAnExplicitChoiceIsNeverOverwritten() { + XCTAssertFalse(PuffinExperiment.shouldPinLegacyOvernightDefault( + hasOvernightChoice: true, hasUsedContinuousHrv: true)) + XCTAssertFalse(PuffinExperiment.shouldPinLegacyOvernightDefault( + hasOvernightChoice: true, hasUsedContinuousHrv: false)) } - /// An explicit OFF wins too, including on a fresh install — the mirror of the guard above. - func testAnExplicitOffIsHonouredEvenOnAFreshInstall() { - XCTAssertFalse(PuffinExperiment.continuousHrvOvernightDefault( - hasExplicitChoice: true, explicitChoice: false, hasUsedContinuousHrv: false)) + /// Idempotence, which is what makes it safe to run on every launch. + func testTheMigrationIsIdempotent() { + XCTAssertTrue(PuffinExperiment.shouldPinLegacyOvernightDefault( + hasOvernightChoice: false, hasUsedContinuousHrv: true)) + XCTAssertFalse(PuffinExperiment.shouldPinLegacyOvernightDefault( + hasOvernightChoice: true, hasUsedContinuousHrv: true)) + } + + /// The sequence that broke the first attempt: resolving the default at READ time from a fact the + /// user's own opt-in creates. Running the decision once at launch is what fixes it. + func testEnablingContinuousHrvAfterLaunchCannotChangeTheDecision() { + XCTAssertFalse(PuffinExperiment.shouldPinLegacyOvernightDefault( + hasOvernightChoice: false, hasUsedContinuousHrv: false)) + XCTAssertTrue(PuffinExperiment.shouldPinLegacyOvernightDefault( + hasOvernightChoice: false, hasUsedContinuousHrv: true)) } } diff --git a/StrandiOS/App/StrandiOSApp.swift b/StrandiOS/App/StrandiOSApp.swift index d59e284fbf..24a67563c2 100644 --- a/StrandiOS/App/StrandiOSApp.swift +++ b/StrandiOS/App/StrandiOSApp.swift @@ -30,6 +30,9 @@ struct StrandiOSApp: App { @AppStorage(ChartStyle.storageKey) private var chartStyleRaw = ChartStyle.titanium.rawValue init() { + // #1008: pin the pre-change Overnight-only default for existing installs before + // anything reads it. Idempotent; a no-op on fresh installs and after the first launch. + PuffinExperiment.migrateContinuousHrvOvernightDefault() #if DEBUG // DEBUG-only promo-screenshot harness: when launched with `--demo-hour `, pin Today to that // hour's day-cycle scene + a per-hour stat frame. No-op (active stays nil) when the arg is absent. diff --git a/android/app/src/main/java/com/noop/NoopApplication.kt b/android/app/src/main/java/com/noop/NoopApplication.kt index f4ac5dc157..0e22bc633e 100644 --- a/android/app/src/main/java/com/noop/NoopApplication.kt +++ b/android/app/src/main/java/com/noop/NoopApplication.kt @@ -40,6 +40,9 @@ class NoopApplication : Application() { override fun onCreate() { super.onCreate() + // #1008: pin the pre-change Overnight-only default for existing installs before anything + // reads it. Idempotent; a no-op on fresh installs and on every launch after the first. + com.noop.ui.NoopPrefs.migrateContinuousHrvOvernightDefault(this) // Record any uncaught crash to a file so it rides along in the shareable strap log — a // device-specific crash (e.g. Insights #224/#267) is otherwise lost to an unreachable logcat. CrashCapture.install(this) diff --git a/android/app/src/main/java/com/noop/ui/MainActivity.kt b/android/app/src/main/java/com/noop/ui/MainActivity.kt index abcd50a043..7d319ab54c 100644 --- a/android/app/src/main/java/com/noop/ui/MainActivity.kt +++ b/android/app/src/main/java/com/noop/ui/MainActivity.kt @@ -332,28 +332,54 @@ object NoopPrefs { * toggled the base setting on and back off keeps always-on too — conservative on purpose, since * they have seen the old behaviour. */ - fun continuousHrvOvernight(context: Context): Boolean { + fun continuousHrvOvernight(context: Context): Boolean = + of(context).getBoolean(KEY_CONTINUOUS_HRV_OVERNIGHT, true) + + /** + * One-time migration for the #1008 default flip. Called once at process start, BEFORE anything reads + * the setting. + * + * The default moved from OFF to ON, so an install that predates the change has to be pinned to OFF + * explicitly or it would be silently narrowed to overnight-only capture — removing daytime data the + * user opted in for. "Predates the change" is read as "has ever toggled Continuous HRV", i.e. the + * base key exists. + * + * Deciding this at READ time instead does not work, and the way it fails is worth recording: the + * discriminator would be the base key, which the user's own opt-in creates — so a fresh install + * would default to ON, then flip to OFF the moment they enabled Continuous HRV, which is the exact + * opposite of the intent. The decision has to be pinned before the user can touch either setting. + * + * Idempotent: writes only when the overnight key is absent and the base key is present, so it is a + * no-op on every launch after the first and on every fresh install. + */ + fun migrateContinuousHrvOvernightDefault(context: Context) { val prefs = of(context) - return continuousHrvOvernightDefault( - hasExplicitChoice = prefs.contains(KEY_CONTINUOUS_HRV_OVERNIGHT), - explicitChoice = prefs.getBoolean(KEY_CONTINUOUS_HRV_OVERNIGHT, true), - hasUsedContinuousHrv = prefs.contains(KEY_CONTINUOUS_HRV), - ) + if (shouldPinLegacyOvernightDefault( + hasOvernightChoice = prefs.contains(KEY_CONTINUOUS_HRV_OVERNIGHT), + hasUsedContinuousHrv = prefs.contains(KEY_CONTINUOUS_HRV), + ) + ) { + prefs.edit().putBoolean(KEY_CONTINUOUS_HRV_OVERNIGHT, false).apply() + } } /** - * The rule behind [continuousHrvOvernight], lifted out so it can be tested without a `Context`. - * Twin of the Swift `PuffinExperiment.continuousHrvOvernightDefault`. + * The migration's decision, lifted out so it is testable without a `Context`. Twin of the Swift + * `PuffinExperiment.shouldPinLegacyOvernightDefault`. + * + * Pin the OLD default only for an install that has used Continuous HRV and never chose an overnight + * setting. Everything else is left alone: an explicit choice is already recorded, or the install is + * fresh and should take the new default. * - * An explicit choice always wins. With no choice recorded, [hasUsedContinuousHrv] decides: someone - * who has been through this screen keeps the always-on behaviour they experienced, a fresh install - * gets overnight-only. + * Note what this is NOT keyed on: the READ. An earlier attempt resolved the default at read time + * from [hasUsedContinuousHrv], which the user's own opt-in creates — so a fresh install read ON and + * then flipped to OFF the moment Continuous HRV was enabled. Running the decision once at launch is + * what makes the answer stable, because it is taken before the user can change the inputs. */ - internal fun continuousHrvOvernightDefault( - hasExplicitChoice: Boolean, - explicitChoice: Boolean, + internal fun shouldPinLegacyOvernightDefault( + hasOvernightChoice: Boolean, hasUsedContinuousHrv: Boolean, - ): Boolean = if (hasExplicitChoice) explicitChoice else !hasUsedContinuousHrv + ): Boolean = !hasOvernightChoice && hasUsedContinuousHrv fun setContinuousHrvOvernight(context: Context, enabled: Boolean) { of(context).edit().putBoolean(KEY_CONTINUOUS_HRV_OVERNIGHT, enabled).apply() diff --git a/android/app/src/test/java/com/noop/ui/ContinuousHrvOvernightDefaultTest.kt b/android/app/src/test/java/com/noop/ui/ContinuousHrvOvernightDefaultTest.kt index a154270cfe..b9a615885b 100644 --- a/android/app/src/test/java/com/noop/ui/ContinuousHrvOvernightDefaultTest.kt +++ b/android/app/src/test/java/com/noop/ui/ContinuousHrvOvernightDefaultTest.kt @@ -5,57 +5,81 @@ import org.junit.Assert.assertTrue import org.junit.Test /** - * #1008 — which way "Overnight only" falls when the user has never chosen. + * #1008 — flipping the "Overnight only" default from OFF to ON without moving anyone who is already + * running the old behaviour. * * WHOOP publishes no daytime HRV figure, so a 24/7 stream has no official-app analogue and costs - * roughly twice the battery. The cheaper, WHOOP-comparable behaviour should be the one you get by - * default — but ONLY for someone who has not already been running the other one. + * roughly twice the battery. The cheaper, WHOOP-comparable option should be what a new user gets. * * The rule that must not break: an existing Continuous HRV user's capture is never silently narrowed. - * They opted into "all day and night" and may be reading daytime Stress off it. + * They opted into "all day and night" and may be reading daytime Stress off it. That is what the + * launch migration pinned here exists for. */ class ContinuousHrvOvernightDefaultTest { - /** A fresh install gets the WHOOP-comparable, cheaper default. This is the change. */ - @Test fun freshInstallDefaultsToOvernightOnly() { + /** The case the migration exists for: used the feature, never chose — pin the old default. */ + @Test fun anExistingContinuousHrvUserIsPinnedToAlwaysOn() { assertTrue( - NoopPrefs.continuousHrvOvernightDefault( - hasExplicitChoice = false, explicitChoice = false, hasUsedContinuousHrv = false, + NoopPrefs.shouldPinLegacyOvernightDefault( + hasOvernightChoice = false, hasUsedContinuousHrv = true, ), ) } - /** - * The regression guard. Someone who already enabled Continuous HRV keeps always-on — narrowing it - * under them would remove the daytime data they opted in for, without asking. - */ - @Test fun anExistingContinuousHrvUserKeepsAlwaysOn() { + /** A fresh install is left alone, so the read picks up the new ON default. */ + @Test fun aFreshInstallIsLeftAloneAndTakesTheNewDefault() { assertFalse( - NoopPrefs.continuousHrvOvernightDefault( - hasExplicitChoice = false, explicitChoice = false, hasUsedContinuousHrv = true, + NoopPrefs.shouldPinLegacyOvernightDefault( + hasOvernightChoice = false, hasUsedContinuousHrv = false, ), ) } - /** An explicit ON wins over anything the install age would imply. */ - @Test fun anExplicitOnIsHonoured() { - assertTrue( - NoopPrefs.continuousHrvOvernightDefault( - hasExplicitChoice = true, explicitChoice = true, hasUsedContinuousHrv = true, + /** An explicit choice is never overwritten, whichever way it points. */ + @Test fun anExplicitChoiceIsNeverOverwritten() { + assertFalse( + NoopPrefs.shouldPinLegacyOvernightDefault( + hasOvernightChoice = true, hasUsedContinuousHrv = true, + ), + ) + assertFalse( + NoopPrefs.shouldPinLegacyOvernightDefault( + hasOvernightChoice = true, hasUsedContinuousHrv = false, ), ) } /** - * An explicit OFF wins too — including on a fresh install. Without this the new default would - * override someone who deliberately turned overnight-only off, which is the mirror of the bug the - * second test guards. + * Idempotence, which is what makes it safe to run on every launch: once the pin is written, the + * choice exists, so the second pass declines. */ - @Test fun anExplicitOffIsHonouredEvenOnAFreshInstall() { - assertFalse( - NoopPrefs.continuousHrvOvernightDefault( - hasExplicitChoice = true, explicitChoice = false, hasUsedContinuousHrv = false, - ), + @Test fun theMigrationIsIdempotent() { + assertTrue(NoopPrefs.shouldPinLegacyOvernightDefault(false, hasUsedContinuousHrv = true)) + // after the write, hasOvernightChoice is true + assertFalse(NoopPrefs.shouldPinLegacyOvernightDefault(true, hasUsedContinuousHrv = true)) + } + + /** + * The SEQUENCE that broke the first attempt, pinned so it cannot come back. + * + * That version resolved the default at READ time from "has used Continuous HRV" — a fact the user's + * own opt-in creates. A fresh install read ON, then flipped to OFF the moment Continuous HRV was + * enabled: the exact opposite of the intent, and invisible to state-by-state tests because every + * individual state was correct. + * + * Running the decision once at launch is what fixes it. On a fresh install the migration declines, + * writes nothing, and enabling Continuous HRV afterwards cannot retroactively make it decline + * differently — there is nothing left to decide. + */ + @Test fun enablingContinuousHrvAfterLaunchCannotChangeTheDecision() { + // At launch on a fresh install: nothing used, nothing chosen → no pin. + assertFalse(NoopPrefs.shouldPinLegacyOvernightDefault(false, hasUsedContinuousHrv = false)) + // The user then enables Continuous HRV. The migration has already run this launch and will not + // run again until next launch — by which time an explicit choice may exist, and if it does not, + // the input it would read is the same one that was already declined. + assertTrue( + "next launch WOULD pin — which is why the migration must run before the read, not after", + NoopPrefs.shouldPinLegacyOvernightDefault(false, hasUsedContinuousHrv = true), ) } } From c5fe62a2f3d41e99767b82a015507d9ab1e08ff9 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sat, 1 Aug 2026 01:19:47 -0700 Subject: [PATCH 3/4] Remove the Swift copy of the rejected read-time rule The replacement added shouldPinLegacyOvernightDefault but left continuousHrvOvernightDefault behind on the Swift side - dead code whose doc describes the design this PR abandoned, and points at a Kotlin twin that no longer exists. Worse than ordinary dead code: it documents the read-time approach as though it were current, which is the specific mistake the migration exists to prevent someone repeating. Found by grepping for stale references after the rewrite; nothing referenced it, so removal is inert. --- Strand/BLE/PuffinExperiment.swift | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/Strand/BLE/PuffinExperiment.swift b/Strand/BLE/PuffinExperiment.swift index 59860c12d3..ae579ec2b8 100644 --- a/Strand/BLE/PuffinExperiment.swift +++ b/Strand/BLE/PuffinExperiment.swift @@ -110,18 +110,6 @@ enum PuffinExperiment { !hasOvernightChoice && hasUsedContinuousHrv } - /// The rule behind `continuousHrvOvernightOnlyEnabled`, lifted out so it can be tested without - /// touching `UserDefaults`. Twin of the Android `NoopPrefs.continuousHrvOvernightDefault`. - /// - /// An explicit choice always wins. With no choice recorded, `hasUsedContinuousHrv` decides: someone - /// who has been through this screen keeps the always-on behaviour they experienced, a fresh install - /// gets overnight-only. - static func continuousHrvOvernightDefault(hasExplicitChoice: Bool, - explicitChoice: Bool, - hasUsedContinuousHrv: Bool) -> Bool { - hasExplicitChoice ? explicitChoice : !hasUsedContinuousHrv - } - // MARK: - Power saving (#477), parity with Android NoopPrefs /// "Power saving" master: battery-adaptive strap-sync cadence. Default off. */ From 31d8a0de73af5af7f6f260bb4cccb285f5805da8 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sat, 1 Aug 2026 01:31:45 -0700 Subject: [PATCH 4/4] Match the iOS toggle's default to the behaviour it controls Changing the read default left SettingsView's @AppStorage on false, so a fresh install would show "Overnight only" OFF while capture was actually overnight-only. They read the same key by different routes. The failure mode is worse than a wrong label: a user "correcting" the toggle by flipping it on and off would write an explicit false and end up with the 24/7 behaviour they were trying to avoid. Android was never affected - its toggle reads through NoopPrefs.continuousHrvOvernight, so it cannot disagree with what the BLE client acts on. Its stale "Default OFF" comment is corrected too. Checked the rest of SettingsView for the same class: journalReminderEnabled and experimentalSleepV2Enabled are the only other true-default toggles, and both already pair with an accessor that handles the unset case - the first with the exact `object(forKey:) as? Bool ?? true` spelling used here. So this now matches how the codebase already solves it, and no other instance exists. --- Strand/Screens/SettingsView.swift | 11 ++++++++--- .../app/src/main/java/com/noop/ui/SettingsScreen.kt | 4 +++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Strand/Screens/SettingsView.swift b/Strand/Screens/SettingsView.swift index 1b761b4af5..10a31ccc80 100644 --- a/Strand/Screens/SettingsView.swift +++ b/Strand/Screens/SettingsView.swift @@ -58,9 +58,14 @@ struct SettingsView: View { /// #927 "Overnight only" refinement of Continuous HRV capture (off by default): arm the stream only /// inside the nightly quiet-hours window instead of 24/7. Composed with the base toggle (base on + - /// this off = ALWAYS, the pre-#927 behaviour) so existing users see no change and need no migration. - /// See [PuffinExperiment.continuousHrvOvernightOnlyKey]. - @AppStorage(PuffinExperiment.continuousHrvOvernightOnlyKey) private var continuousHrvOvernightOnly = false + /// this off = ALWAYS, the pre-#927 behaviour); existing installs are pinned to OFF by + /// `PuffinExperiment.migrateContinuousHrvOvernightDefault()` at launch, so they still see no change. + /// + /// The `@AppStorage` default MUST match `PuffinExperiment.continuousHrvOvernightOnlyEnabled` (#1008). + /// They read the same key by different routes, so a mismatch shows the toggle OFF on a fresh install + /// while capture is actually overnight-only — and a user "correcting" that would write an explicit + /// false and get the 24/7 behaviour they were trying to avoid. + @AppStorage(PuffinExperiment.continuousHrvOvernightOnlyKey) private var continuousHrvOvernightOnly = true // #477 Power saving (parity with Android). Battery-adaptive sync cadence + an HRV-pause sub-option. @AppStorage(PuffinExperiment.powerSavingKey) private var powerSavingEnabled = false diff --git a/android/app/src/main/java/com/noop/ui/SettingsScreen.kt b/android/app/src/main/java/com/noop/ui/SettingsScreen.kt index 5a3b3f3d47..b89442994f 100644 --- a/android/app/src/main/java/com/noop/ui/SettingsScreen.kt +++ b/android/app/src/main/java/com/noop/ui/SettingsScreen.kt @@ -516,7 +516,9 @@ fun SettingsScreen( var continuousHrv by remember { mutableStateOf(NoopPrefs.continuousHrv(context)) } // "Overnight only" (#927): arm the continuous stream only inside the nightly quiet-hours window - // instead of 24/7. Default OFF so existing users keep the always-on behaviour. Local mirror. + // instead of 24/7. Defaults ON for fresh installs (#1008); existing installs are pinned to OFF by + // NoopPrefs.migrateContinuousHrvOvernightDefault() at launch, so they keep always-on. Local mirror, + // read through NoopPrefs so it cannot disagree with what the BLE client acts on. var continuousHrvOvernight by remember { mutableStateOf(NoopPrefs.continuousHrvOvernight(context)) } // #477 Power saving: battery-adaptive strap-sync cadence + optional HRV-capture pause. Local mirrors.