From 93dfd18c9a5f176dd39d2ae094ed6c706fc9dcbc Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 22 Aug 2026 00:38:53 +1200 Subject: [PATCH 1/3] perf: stop rebuilding the R-R ords array and re-reading the strap model on hot paths Two allocations added during the 10.5.0 -> 10.5.1 work that do not need to exist. R-R diagnostic ords (both platforms). The over-count branch of the sleep R-R diagnostics calls three analysers, and each was handed its own freshly mapped copy of the night's `ord` column -- roughly 70k elements a night, on a branch reached about 21 times per `analyzeRecent`, which itself runs every 15 minutes. The three copies are identical by construction, so two of them were pure waste. Built once above the calls now. Same values, same order, same diagnostic strings -- the pinned parity tests (`DuplicatePairRatioTest` / `HRVAnalyzerDuplicatePairTests`) still assert the exact output. Strap-model read (iOS). `stepsPipelineActive` read `WhoopModel.persisted`, a `UserDefaults` lookup, from inside `keyMetricTile` -- once per metric per body pass, on a view that recomposes with live heart rate. Read through `@AppStorage` instead, which is the idiom the rest of `TodayView` already uses. The `?? .whoop4` fallback is preserved exactly: an absent key reads as the empty string, which is not a valid raw value, so it lands on the same default. Secondary benefit -- the value is now observed, so switching straps re-renders the tile instead of waiting for an unrelated recomposition. No behavioural change on either platform; both are refactors of work introduced in 10.5.1 itself. Verified: android compileFullDebugKotlin + testFullDebugUnitTest (4189 tests, 0 failures, --no-build-cache --rerun-tasks); i18n audit clean; doc lint clean. App-target Swift (`TodayView`) validated by app-build, which default CI does not run -- see the PR for the run. --- Strand/Data/IntelligenceEngine.swift | 10 +++++++--- Strand/Screens/TodayView.swift | 16 ++++++++++++---- .../com/noop/analytics/IntelligenceEngine.kt | 10 +++++++--- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/Strand/Data/IntelligenceEngine.swift b/Strand/Data/IntelligenceEngine.swift index f8dd837c0e..85f980b3ba 100644 --- a/Strand/Data/IntelligenceEngine.swift +++ b/Strand/Data/IntelligenceEngine.swift @@ -1082,15 +1082,19 @@ final class IntelligenceEngine: ObservableObject { // over-count's MECHANISM is readable from the always-on log — clean nights stay quiet. // srcChannel rides from the read model. Byte-identical to the Kotlin `hrv rrsample` line. if verdict == .crossSecondOverCount || verdict == .sameSecondOverCount { + // Built ONCE for all three diagnostics below. Each call materialised its own copy + // of the night's ords — ~70k elements, on a path that runs ~21 times per + // analyzeRecent, every 15 minutes — so two of the three were pure waste. (#1510) + let rrOrds = sleepRrRows.map { $0.ord } let sample = HRVAnalyzer.densestSecondWindowSample( tsSec: ts, rrMs: sleepRr, srcCodes: sleepRrRows.map { $0.srcChannel?.rawValue }, - ords: sleepRrRows.map { $0.ord }) + ords: rrOrds) if !sample.isEmpty { diagLine += "\nhrv rrsample day=\(res.daily.day) \(sample)" } // #1331/#1008: the sample above shows ords for a handful of seconds; this counts // them across the WHOLE night, which is what decides whether the fix belongs at // ingest. Kotlin twin. let deliveries = HRVAnalyzer.deliveryHistogram( - tsSec: ts, rrMs: sleepRr, ords: sleepRrRows.map { $0.ord }) + tsSec: ts, rrMs: sleepRr, ords: rrOrds) if !deliveries.isEmpty { diagLine += "\n\(deliveries) day=\(res.daily.day)" } // #1505: the histogram above counts deliveries per second but never compares what // they wrote. If the live and historical copies are the same beat in two units, a @@ -1098,7 +1102,7 @@ final class IntelligenceEngine: ObservableObject { // the ratios scatter. One pair cannot tell those apart — a night's worth can. // Measurement only, takes no view on the answer. Kotlin twin. let dupPairs = HRVAnalyzer.duplicatePairRatios( - tsSec: ts, rrMs: sleepRr, ords: sleepRrRows.map { $0.ord }) + tsSec: ts, rrMs: sleepRr, ords: rrOrds) if !dupPairs.isEmpty { diagLine += "\n\(dupPairs) day=\(res.daily.day)" } // #1331/#1008/#1118 SHADOW: log the DEDUPED stream's HRV + coverage + beat-accuracy // beside the raw (above), so the candidate two-channel de-dup can be validated diff --git a/Strand/Screens/TodayView.swift b/Strand/Screens/TodayView.swift index d849b6d165..9453cd98fc 100644 --- a/Strand/Screens/TodayView.swift +++ b/Strand/Screens/TodayView.swift @@ -224,6 +224,12 @@ struct TodayView: View { // Editable Key-Metrics layout (#251), an ordered list of the enabled tiles, persisted display-only. // Empty/unset shows the full default order. Every edit affordance routes into one customization sheet. @AppStorage(KeyMetricPrefs.layoutKey) private var keyMetricsRaw = "" + // #1502/#1512: the strap family behind the steps-calibration prompt. Read through @AppStorage rather + // than `WhoopModel.persisted` because `stepsPipelineActive` is evaluated inside `keyMetricTile`, once + // per metric per body pass, and this body recomposes with live heart rate — that made it a + // UserDefaults lookup on a ~1 Hz path. The Android twin remembers the equivalent read; this is the + // same fix. Also makes the value observed, so a strap-model change re-renders the tile. + @AppStorage("selectedWhoopModel") private var selectedWhoopModelRaw = "" @AppStorage("today.keyMetricsDetailed") private var keyMetricsDetailed = false @AppStorage("today.keyMetricsWindowDays") private var keyMetricsWindowDays = 14 private var enabledKeyMetrics: [KeyMetric] { KeyMetricPrefs.decodeEnabled(keyMetricsRaw) } @@ -3881,16 +3887,18 @@ struct TodayView: View { /// always estimates — and it answers it on day one. The calibration state it is OR-ed with is /// profile-global rather than per-strap, so both halves are measuring the same user either way. /// - /// Read straight off the persisted selection rather than through `BLEManager.isWhoop4`: `TodayView` - /// holds no `AppModel`, and `BLEManager` writes this same key whenever the model changes - /// (`BLEManager.persistSelectedModel`), so the static is the same answer without the dependency. + /// Read off the persisted selection rather than through `BLEManager.isWhoop4`: `TodayView` holds no + /// `AppModel`, and `BLEManager` writes this same key whenever the model changes + /// (`BLEManager.persistSelectedModel`), so the stored value is the same answer without the dependency. /// /// [hasDayData] is why the family term is not used alone: `WhoopModel.persisted` falls back to /// `.whoop4` when nothing has been selected, so a user who has never paired anything reads as a 4.0 /// owner. Requiring a scored day for the date being shown keeps the "no strap at all" case the /// original gate protected — a user with nothing recorded still sees no prompt. private func stepsPipelineActive(hasDayData: Bool) -> Bool { - (WhoopModel.persisted.deviceFamily == .whoop4 && hasDayData) + // Same fallback `WhoopModel.persisted` applies, so a never-set key still reads as a 4.0 and the + // `hasDayData` guard is what keeps a strapless user out — behaviour is unchanged, only the read is. + ((WhoopModel(rawValue: selectedWhoopModelRaw) ?? .whoop4).deviceFamily == .whoop4 && hasDayData) || profile.stepsCalibrationCoefficient > 0 || profile.stepsManualCoefficient > 0 || profile.stepsCalibrationSampleDays > 0 diff --git a/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt b/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt index 4c3b9d03e0..34cba8a139 100644 --- a/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt +++ b/android/app/src/main/java/com/noop/analytics/IntelligenceEngine.kt @@ -891,21 +891,25 @@ object IntelligenceEngine { verdict == HrvAnalyzer.RrCoverageVerdict.SAME_SECOND_OVER_COUNT) if (verdict == HrvAnalyzer.RrCoverageVerdict.CROSS_SECOND_OVER_COUNT || verdict == HrvAnalyzer.RrCoverageVerdict.SAME_SECOND_OVER_COUNT) { + // Built ONCE for all three diagnostics below. Each call materialised its own copy of + // the night's ords — ~70k elements, on a path that runs ~21 times per analyzeRecent, + // every 15 minutes — so two of the three were pure waste. (#1510) Twin of the Swift hoist. + val rrOrds = sleepRrRows.map { it.ord } val sample = HrvAnalyzer.densestSecondWindowSample( ts, sleepRr, sleepRrRows.map { it.srcChannel }, - sleepRrRows.map { it.ord }, + rrOrds, ) if (sample.isNotEmpty()) dayDiag("hrv rrsample day=${res.daily.day} $sample") // #1331/#1008: the sample above shows ords for a handful of seconds; this counts them // across the WHOLE night, which is what decides whether the fix belongs at ingest. - val deliveries = HrvAnalyzer.deliveryHistogram(ts, sleepRr, sleepRrRows.map { it.ord }) + val deliveries = HrvAnalyzer.deliveryHistogram(ts, sleepRr, rrOrds) if (deliveries.isNotEmpty()) dayDiag("$deliveries day=${res.daily.day}") // #1505: the histogram above counts deliveries per second but never compares what they // wrote. If the live and historical copies are the same beat in two units, a duplicated // second holds two values 1024/1000 apart; if they are different beats, the ratios // scatter. One pair cannot tell those apart — a night's worth can. Measurement only, // takes no view on the answer. Swift twin. - val dupPairs = HrvAnalyzer.duplicatePairRatios(ts, sleepRr, sleepRrRows.map { it.ord }) + val dupPairs = HrvAnalyzer.duplicatePairRatios(ts, sleepRr, rrOrds) if (dupPairs.isNotEmpty()) dayDiag("$dupPairs day=${res.daily.day}") // #1331/#1008/#1118 SHADOW: log the DEDUPED stream's HRV + coverage + beat-accuracy // beside the raw so the candidate de-dup can be validated vs WHOOP + @artemc's Polar From 50aba91954f83ef00f7617fbf3428b6b8dc1f7c2 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 22 Aug 2026 00:42:48 +1200 Subject: [PATCH 2/3] fix: key the Android steps-calibration prompt on the strap model it reads Found re-reviewing the iOS half of this branch. `stepsCalibrationPrompt` returns null for anything that is not a 4.0 -- it reads `noop.selectedWhoopModel` as its first act -- but the `remember` wrapping it was keyed only on the three calibration values. The model key is rewritten under a composed Today screen, both from Settings and from live detection in `WhoopBleClient`, so switching straps left the previous answer on screen until a calibration value happened to move independently. Adding the model to the key list costs a loaded-map lookup per pass and leaves the rest of the prompt memoised, which is where the cost being avoided was. This is the same defect the iOS change in this branch removes from the other side: reading the model through `@AppStorage` makes it observed, so the tile re-renders on a strap switch. Both platforms now react to the same event. Not covered by a test: the repo's Android tests are JVM unit tests and this is a Compose `remember` key, which needs instrumentation to observe. Verified: compileFullDebugKotlin + testFullDebugUnitTest (4189 tests, 0 failures, --no-build-cache --rerun-tasks); doc lint and i18n audit clean. --- Strand/Screens/TodayView.swift | 5 +++-- android/app/src/main/java/com/noop/ui/TodayScreen.kt | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Strand/Screens/TodayView.swift b/Strand/Screens/TodayView.swift index 9453cd98fc..5b96333905 100644 --- a/Strand/Screens/TodayView.swift +++ b/Strand/Screens/TodayView.swift @@ -227,8 +227,9 @@ struct TodayView: View { // #1502/#1512: the strap family behind the steps-calibration prompt. Read through @AppStorage rather // than `WhoopModel.persisted` because `stepsPipelineActive` is evaluated inside `keyMetricTile`, once // per metric per body pass, and this body recomposes with live heart rate — that made it a - // UserDefaults lookup on a ~1 Hz path. The Android twin remembers the equivalent read; this is the - // same fix. Also makes the value observed, so a strap-model change re-renders the tile. + // UserDefaults lookup on a ~1 Hz path. Android memoises the analogous preference reads on this same + // body for the same reason. Also makes the value observed, so a strap-model change re-renders the + // tile rather than waiting on an unrelated recomposition. @AppStorage("selectedWhoopModel") private var selectedWhoopModelRaw = "" @AppStorage("today.keyMetricsDetailed") private var keyMetricsDetailed = false @AppStorage("today.keyMetricsWindowDays") private var keyMetricsWindowDays = 14 diff --git a/android/app/src/main/java/com/noop/ui/TodayScreen.kt b/android/app/src/main/java/com/noop/ui/TodayScreen.kt index 9b62471585..0fde876f7f 100644 --- a/android/app/src/main/java/com/noop/ui/TodayScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TodayScreen.kt @@ -1528,10 +1528,19 @@ fun TodayScreen( // Remembered on the calibration state it reads, matching the same // preference lookup in SettingsScreen: this is a SharedPreferences // read and the Today body recomposes with live HR. + // + // The strap model is a key too, because the prompt returns null for + // anything but a 4.0 -- and that key is rewritten under a composed + // Today, by Settings and by live detection in WhoopBleClient. Keyed + // only on the calibration values, a strap switch left the previous + // answer on screen until one of those happened to move. Re-reading + // the key per pass is a loaded-map lookup; what stays memoised is + // the rest of the prompt. stepsCalibrationPrompt = remember( profileStore.stepsCalibrationSampleDays, profileStore.stepsCalibrationCoefficient, profileStore.stepsCalibrationManual, + NoopPrefs.of(context).getString("noop.selectedWhoopModel", null), ) { stepsCalibrationPrompt(context, profileStore) }, restScore = restScoreForDay, restSpark = restCompositeSpark, From a16d885a0a19b8056be3716814adfa111a3fe0fa Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 22 Aug 2026 00:54:10 +1200 Subject: [PATCH 3/3] fix: don't offer the steps-calibration prompt when no strap has ever been identified (iOS) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parity gap found reviewing this branch. #1512 landed the same prompt on both platforms, and the two halves disagreed about one state from the first commit. `stepsCalibrationPrompt` on Android reads the model key and bails outright when it is unset (`?: return null`). The Apple side read it through `WhoopModel.persisted`, which falls back to `.whoop4` when nothing has been selected — so a user who has never paired a strap read as a 4.0 owner. Given a scored day with no step count on it, which an import-only user has, iOS offered "Need N more days where your phone also counted steps" for a strap nobody has ever seen, and Android showed nothing. Optional-chained now, so an unset key is not a 4.0. The key is written by `BLEManager.persistSelectedModel` as soon as a strap is identified and only ever holds a `WhoopModel` rawValue, so every real 4.0 owner still qualifies and #1512's intent is untouched: what goes away is only the never-paired case. `hasDayData` stays as the second guard. It answers whether the date being shown has anything scored on it, which is a different question from whether a strap is known, and it was carrying both jobs before. Deliberately NOT fixed here, to keep this to the one state: the calibration-state terms are still OR-ed outside the family check, so leftover sample days can activate the pipeline on a non-4.0. Android cannot do that. Filing separately. Verified: app-build (see PR). No Android change — Android already behaves this way, which is what made the gap visible. --- Strand/Screens/TodayView.swift | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/Strand/Screens/TodayView.swift b/Strand/Screens/TodayView.swift index 5b96333905..6242e78eb2 100644 --- a/Strand/Screens/TodayView.swift +++ b/Strand/Screens/TodayView.swift @@ -224,7 +224,7 @@ struct TodayView: View { // Editable Key-Metrics layout (#251), an ordered list of the enabled tiles, persisted display-only. // Empty/unset shows the full default order. Every edit affordance routes into one customization sheet. @AppStorage(KeyMetricPrefs.layoutKey) private var keyMetricsRaw = "" - // #1502/#1512: the strap family behind the steps-calibration prompt. Read through @AppStorage rather + // #1512: the strap family behind the steps-calibration prompt. Read through @AppStorage rather // than `WhoopModel.persisted` because `stepsPipelineActive` is evaluated inside `keyMetricTile`, once // per metric per body pass, and this body recomposes with live heart rate — that made it a // UserDefaults lookup on a ~1 Hz path. Android memoises the analogous preference reads on this same @@ -3892,14 +3892,20 @@ struct TodayView: View { /// `AppModel`, and `BLEManager` writes this same key whenever the model changes /// (`BLEManager.persistSelectedModel`), so the stored value is the same answer without the dependency. /// - /// [hasDayData] is why the family term is not used alone: `WhoopModel.persisted` falls back to - /// `.whoop4` when nothing has been selected, so a user who has never paired anything reads as a 4.0 - /// owner. Requiring a scored day for the date being shown keeps the "no strap at all" case the - /// original gate protected — a user with nothing recorded still sees no prompt. + /// The family term requires the key to have actually been SET. It is written by + /// `BLEManager.persistSelectedModel` the moment a strap is identified, so every real 4.0 owner has one + /// and #1491's intent is untouched — what it excludes is the user who has never paired anything, whose + /// unset key used to read as a 4.0 through the default and earn them a prompt to calibrate a strap + /// nobody has seen. Android's twin has always required the key (`?: return null` in + /// `stepsCalibrationPrompt`); this is the Apple side matching it — the two halves of #1512 disagreed + /// about the unset case from the day it landed. + /// + /// [hasDayData] stays as the second guard: it is what keeps the prompt off a date with nothing scored + /// on it, which is a different question from whether a strap is known. private func stepsPipelineActive(hasDayData: Bool) -> Bool { - // Same fallback `WhoopModel.persisted` applies, so a never-set key still reads as a 4.0 and the - // `hasDayData` guard is what keeps a strapless user out — behaviour is unchanged, only the read is. - ((WhoopModel(rawValue: selectedWhoopModelRaw) ?? .whoop4).deviceFamily == .whoop4 && hasDayData) + // Optional-chained deliberately: an unset (or unparseable) key is NOT a 4.0. The key only ever + // holds a `WhoopModel` rawValue, so nil here means "no strap has been identified", not "4.0". + (WhoopModel(rawValue: selectedWhoopModelRaw)?.deviceFamily == .whoop4 && hasDayData) || profile.stepsCalibrationCoefficient > 0 || profile.stepsManualCoefficient > 0 || profile.stepsCalibrationSampleDays > 0