From 1d44f5704806932bdd10310feb52e85bf66efd79 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sun, 26 Jul 2026 19:37:06 -0700 Subject: [PATCH 1/5] fix(workouts): one HR device-id rule for the chart, zones and Avg HR (#856) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three id resolutions were in play on one screen, and all three were wrong somewhere: Swift avg workoutHrDeviceId (single) right for detected, UNDER-READS imported Swift chart hrBuckets (day union) right for imported, reads the WRONG STRAP for a bout detected on a second WHOOP Kotlin chart single active id both failure modes at once So on iOS the Avg HR and the HR curve on the same card could describe different straps. That is not a Kotlin-vs-Swift divergence, it is a live self-inconsistency. Neither existing rule is right on its own. The correct read is source-class-dependent, which workoutHrDeviceId already encoded — it just returned one id where one case needs two: detected -> [recording strap] a union would mix in a strap that never recorded the session (#510) manual / imported -> active u canonical no strap of its own, and the worn strap may bank under either after a re-add (#814) workoutHrDeviceIds now returns that list on both platforms, and the chart, the zone read and the window aggregate all consume it. The aggregate needed real work. A naive `deviceId IN (...)` double-counts a second banked under both ids, inflating n and skewing avg — silently, because both numbers stay plausible. GROUP BY ts with MIN(pri) keeps one row per second and takes the primary's, matching the dedup the chart already did. Fixed 2-arity rather than a bound collection: importedReadIds is 1 or 2 by construction, and Room @Query is static SQL that cannot express per-id priority through an IN list. Passing the same id twice is byte-identical to the old single-id read, so a single-WHOOP install is unchanged and needs no special case. That is the control test. The aggregate lives in WhoopStore, where swift-packages.yml actually runs its tests — the same placement that made #841's Swift half verifiable and its Kotlin half not. Verified against SQLite before commit, and the Kotlin @Query text was extracted from source and executed to confirm it returns the same numbers as the Swift twin: A(0..9) + B(5..19), A primary -> n=20 avg=150.0 max=200 (naive IN would say 25) B primary -> n=20 avg=175.0 PPG anti-join per id -> n=15 max=130 same id twice -> identical to the pre-change read Nine tests: five existing moved to the two-id signature (same id twice, so their assertions are untouched), four added for overlap, precedence, the per-id PPG anti-join and the single-id control. --- .../WhoopStore/Sources/WhoopStore/Reads.swift | 41 +++++++--- .../Tests/WhoopStoreTests/ReadTests.swift | 79 +++++++++++++++++-- Strand/Data/Repository.swift | 63 ++++++++++++--- Strand/Screens/WorkoutDetailView.swift | 2 +- .../src/main/java/com/noop/data/WhoopDao.kt | 26 ++++-- .../java/com/noop/data/WhoopRepository.kt | 28 +++++-- .../src/main/java/com/noop/ui/AppViewModel.kt | 15 +++- .../main/java/com/noop/ui/WorkoutsScreen.kt | 2 +- .../com/noop/data/WorkoutHrDeviceKeyTest.kt | 17 ++-- 9 files changed, 224 insertions(+), 49 deletions(-) diff --git a/Packages/WhoopStore/Sources/WhoopStore/Reads.swift b/Packages/WhoopStore/Sources/WhoopStore/Reads.swift index 4d56e18105..6625c4b8d9 100644 --- a/Packages/WhoopStore/Sources/WhoopStore/Reads.swift +++ b/Packages/WhoopStore/Sources/WhoopStore/Reads.swift @@ -91,20 +91,41 @@ extension WhoopStore { /// /// Same anti-join as [hrSamples]: a measured second is never double-counted by its PPG estimate. /// `avg`/`max` are nil when `n == 0`. Kotlin twin: `WhoopDao.hrWindowStats`. - public func hrWindowStats(deviceId: String, from: Int, to: Int) async throws -> HRWindowStats { + /// + /// #856: aggregates across up to TWO device ids, `primaryId` winning per second. A naive + /// `deviceId IN (…)` is wrong here — a second banked under both ids after a strap re-add would be + /// counted twice, inflating `n` and skewing `avg`, and both numbers would stay plausible so nothing + /// would look wrong. `GROUP BY ts` with `MIN(pri)` keeps one row per second and takes the primary's, + /// matching the dedup `hrBuckets` already does for the chart; SQLite's bare-column rule makes `bpm` + /// come from the row that supplied the `MIN`. + /// + /// Passing the same id for both is byte-identical to the old single-id read, so a single-WHOOP + /// install needs no special case and every existing number is unchanged. + public func hrWindowStats(primaryId: String, secondaryId: String, + from: Int, to: Int) async throws -> HRWindowStats { try syncRead { db in guard let row = try Row.fetchOne(db, sql: """ SELECT COUNT(*) AS n, AVG(bpm) AS avg, MAX(bpm) AS max FROM ( - SELECT bpm FROM hrSample - WHERE deviceId = ? AND ts >= ? AND ts <= ? - UNION ALL - SELECT CAST(ROUND(p.bpm) AS INTEGER) AS bpm FROM ppgHrSample p - WHERE p.deviceId = ? AND p.ts >= ? AND p.ts <= ? - AND NOT EXISTS ( - SELECT 1 FROM hrSample h - WHERE h.deviceId = p.deviceId AND h.ts = p.ts) + SELECT ts, MIN(pri), bpm FROM ( + SELECT ts, bpm, 0 AS pri FROM hrSample + WHERE deviceId = ? AND ts >= ? AND ts <= ? + UNION ALL + SELECT p.ts, CAST(ROUND(p.bpm) AS INTEGER), 0 FROM ppgHrSample p + WHERE p.deviceId = ? AND p.ts >= ? AND p.ts <= ? + AND NOT EXISTS (SELECT 1 FROM hrSample h + WHERE h.deviceId = p.deviceId AND h.ts = p.ts) + UNION ALL + SELECT ts, bpm, 1 FROM hrSample + WHERE deviceId = ? AND ts >= ? AND ts <= ? + UNION ALL + SELECT p.ts, CAST(ROUND(p.bpm) AS INTEGER), 1 FROM ppgHrSample p + WHERE p.deviceId = ? AND p.ts >= ? AND p.ts <= ? + AND NOT EXISTS (SELECT 1 FROM hrSample h + WHERE h.deviceId = p.deviceId AND h.ts = p.ts) + ) GROUP BY ts ) - """, arguments: [deviceId, from, to, deviceId, from, to]) + """, arguments: [primaryId, from, to, primaryId, from, to, + secondaryId, from, to, secondaryId, from, to]) else { return HRWindowStats(n: 0, avg: nil, max: nil) } return HRWindowStats(n: row["n"], avg: row["avg"], max: row["max"]) } diff --git a/Packages/WhoopStore/Tests/WhoopStoreTests/ReadTests.swift b/Packages/WhoopStore/Tests/WhoopStoreTests/ReadTests.swift index 1710dd981d..cc7f4f783b 100644 --- a/Packages/WhoopStore/Tests/WhoopStoreTests/ReadTests.swift +++ b/Packages/WhoopStore/Tests/WhoopStoreTests/ReadTests.swift @@ -88,7 +88,7 @@ final class ReadTests: XCTestCase { for t in 15..<415 { ppg.append(PpgHrSample(ts: t, bpm: 140, conf: 0.8)) } _ = try await store.insert(Streams(hr: hr, ppgHr: ppg), deviceId: "dev1") - let stats = try await store.hrWindowStats(deviceId: "dev1", from: 0, to: 1_000) + let stats = try await store.hrWindowStats(primaryId: "dev1", secondaryId: "dev1", from: 0, to: 1_000) XCTAssertEqual(stats.n, 415, "measured ∪ PPG, with the 5 overlapping seconds counted once") XCTAssertEqual(stats.max, 140) @@ -107,7 +107,7 @@ final class ReadTests: XCTestCase { Streams(hr: (0..<10).map { HRSample(ts: $0, bpm: 100) }, ppgHr: (0..<10).map { PpgHrSample(ts: $0, bpm: 180, conf: 0.9) }), deviceId: "dev1") - let stats = try await store.hrWindowStats(deviceId: "dev1", from: 0, to: 1_000) + let stats = try await store.hrWindowStats(primaryId: "dev1", secondaryId: "dev1", from: 0, to: 1_000) XCTAssertEqual(stats.n, 10, "every second is measured; no PPG row may be added") XCTAssertEqual(try XCTUnwrap(stats.avg), 100, accuracy: 1e-9) XCTAssertEqual(stats.max, 100, "the PPG 180s must not become the peak") @@ -123,7 +123,7 @@ final class ReadTests: XCTestCase { let hr = (0.. [HRBucket] { + guard let store = await ensureStore(), !deviceIds.isEmpty else { return [] } + guard deviceIds.count > 1 else { + return (try? await store.hrBuckets(deviceId: deviceIds[0], from: from, to: to, + bucketSeconds: bucketSeconds)) ?? [] + } + var byStart: [Int: HRBucket] = [:] + for id in deviceIds { + for b in (try? await store.hrBuckets(deviceId: id, from: from, to: to, + bucketSeconds: bucketSeconds)) ?? [] where byStart[b.ts] == nil { + byStart[b.ts] = b + } + } + return byStart.values.sorted { $0.ts < $1.ts } + } + func hrBuckets(from: Int, to: Int, bucketSeconds: Int = 300) async -> [HRBucket] { guard let store = await ensureStore() else { return [] } // UNION the active strap + canonical for the trend chart. Per bucket-start the active strap wins; a @@ -2223,16 +2241,19 @@ final class Repository: ObservableObject { // row's `source` IS its computed strap id ("-noop"), so a bout auto-detected on a 2nd // WHOOP reads "" instead of the active strap's empty window. Resolved on the main actor; // only the resulting Sendable String crosses into the task. - let hrDeviceId = Self.workoutHrDeviceId(source: rows[idx].source, activeStrapId: deviceId) - group.addTask { [hrDeviceId] in + let hrIds = Self.workoutHrDeviceIds(source: rows[idx].source, activeStrapId: deviceId, + importedIds: importedReadIds) + group.addTask { [hrIds] in // #836: aggregate in SQLite over the WHOLE window instead of reducing up to 8000 // materialised rows. The old read capped at 8000, so a workout longer than ~2h13m // at 1 Hz reported the mean of its FIRST 8000 samples as the session average — wrong // on its own terms, and divergent from Kotlin, which aggregates the lot. This also // makes the common path (no strain fill) read no rows at all, which is strictly // better for the #833 freeze this function exists to avoid. - guard let stats = try? await store.hrWindowStats(deviceId: hrDeviceId, - from: startTs, to: endTs), + guard let stats = try? await store.hrWindowStats( + primaryId: hrIds[0], + secondaryId: hrIds.count > 1 ? hrIds[1] : hrIds[0], + from: startTs, to: endTs), stats.n >= minSamples, let mean = stats.avg, let peak = stats.max else { return nil } let avg = Int(mean.rounded()) @@ -2243,7 +2264,7 @@ final class Repository: ObservableObject { // cap stays here: it bounds the strain window, not the average. (Kotlin does the same.) let strain: Double? if wantStrain, let p = strainProfile { - let samples = (try? await store.hrSamples(deviceId: hrDeviceId, + let samples = (try? await store.hrSamples(deviceId: hrIds[0], from: startTs, to: endTs, limit: 8000)) ?? [] strain = StrainScorer.strain(samples, maxHR: p.hrMax, sex: p.sex) @@ -2278,7 +2299,7 @@ final class Repository: ObservableObject { } } - /// #510 (Kotlin twin: `WhoopRepository.workoutHrDeviceId`). The device id whose `hrSample` rows back a + /// #510 (Kotlin twin: `WhoopRepository.workoutHrDeviceIds`). The device ids whose `hrSample` rows back a /// workout's Avg HR / Effort reconcile. A DETECTED row's own `source` IS its computed strap id /// ("-noop"), so strip the suffix to read HR under the raw "" — a bout auto-detected on a /// SECOND WHOOP no longer reads the active strap's empty window (which blanked its reconcile). MANUAL and @@ -2287,9 +2308,21 @@ final class Repository: ObservableObject { /// byte-identical. (The Kotlin twin also keys MANUAL rows on their stored deviceId; the Swift read-model /// `WorkoutRow` carries no deviceId, so a manual session created on a NON-active strap reconciles here /// against the active strap instead — a minor, display-only divergence, never persisted.) - nonisolated static func workoutHrDeviceId(source: String, activeStrapId: String) -> String { - guard WorkoutSource.classify(source) == .detected else { return activeStrapId } - return source.hasSuffix("-noop") ? String(source.dropLast(5)) : source + /// #856: the ids whose HR backs a workout's chart, zones and Avg HR, in read precedence. + /// + /// DETECTED rows return exactly ONE id — the strap that recorded the bout. Unioning here would mix + /// in HR from a strap that never recorded the session, which is what #510 fixed. + /// + /// MANUAL / IMPORTED rows carry no strap of their own, so they reconcile against "the worn strap", + /// which after a re-add may bank under the canonical id rather than the active one. Those get the + /// #814 read-spine union, active first so it wins per timestamp. + /// + /// At most two ids either way (`importedReadIds` is 1 or 2 by construction), and a single-WHOOP + /// install collapses to one in both branches — so every existing number is unchanged. + nonisolated static func workoutHrDeviceIds(source: String, activeStrapId: String, + importedIds: [String]) -> [String] { + guard WorkoutSource.classify(source) == .detected else { return importedIds } + return [source.hasSuffix("-noop") ? String(source.dropLast(5)) : source] } // MARK: - Workout editing (manual add/edit · relabel · dismiss · delete) @@ -2568,11 +2601,19 @@ final class Repository: ObservableObject { /// Downsampled HR over a workout window for the detail HR-curve. A short session wants a finer /// bucket than the Today 24h chart (300 s would flatten a 30-min run to ~6 points), so the bucket /// scales with duration: ~120 buckets across the window, floored at 15 s and capped at 300 s. - func workoutHrBuckets(from: Int, to: Int) async -> [HRBucket] { + /// #856: reads the ids `workoutHrDeviceIds` resolves for this row, not the day-level union. A bout + /// detected on a SECOND WHOOP is charted from the strap that recorded it; previously this delegated + /// to `hrBuckets(from:to:)`, which unions active + canonical — neither of which is that strap — so + /// the curve and the Avg HR on the same card could describe different straps entirely. + /// `source` defaults to "" (⇒ the imported/manual branch, the union) so callers without a row keep + /// today's behaviour. + func workoutHrBuckets(from: Int, to: Int, source: String = "") async -> [HRBucket] { guard to > from else { return [] } let span = to - from let bucket = max(15, min(300, span / 120)) - return await hrBuckets(from: from, to: to, bucketSeconds: bucket) + let ids = Self.workoutHrDeviceIds(source: source, activeStrapId: deviceId, + importedIds: importedReadIds) + return await hrBuckets(deviceIds: ids, from: from, to: to, bucketSeconds: bucket) } /// Raw HR samples binned into per-zone MINUTES for a workout window, using the age-derived diff --git a/Strand/Screens/WorkoutDetailView.swift b/Strand/Screens/WorkoutDetailView.swift index ba46818fac..085bcee50f 100644 --- a/Strand/Screens/WorkoutDetailView.swift +++ b/Strand/Screens/WorkoutDetailView.swift @@ -107,7 +107,7 @@ struct WorkoutDetailView: View { // HR curve over the exact session window — a finer bucket than the 24h chart so a short run // still reads as a curve, not a handful of points. - let buckets = await repo.workoutHrBuckets(from: row.startTs, to: row.endTs) + let buckets = await repo.workoutHrBuckets(from: row.startTs, to: row.endTs, source: row.source) let points = buckets.map { TrendPoint(date: Date(timeIntervalSince1970: TimeInterval($0.ts)), value: $0.bpm) } // Zones: prefer the imported per-workout percentages (a WHOOP-computed split), and only fall diff --git a/android/app/src/main/java/com/noop/data/WhoopDao.kt b/android/app/src/main/java/com/noop/data/WhoopDao.kt index baf756955a..8c8d9bb08c 100644 --- a/android/app/src/main/java/com/noop/data/WhoopDao.kt +++ b/android/app/src/main/java/com/noop/data/WhoopDao.kt @@ -299,16 +299,30 @@ interface WhoopDao : DeviceRegistryDao { * it: its twin reduces `store.hrSamples`, the coalescing read. Same anti-join as [hrSamples], so a * measured second is never double-counted by its PPG estimate. */ @Query( + // #856: up to TWO ids, :primaryId winning per second. A naive `deviceId IN (…)` would count a + // second banked under BOTH ids twice after a strap re-add, inflating n and skewing avg — and + // both numbers would stay plausible, so nothing would look wrong. GROUP BY ts with MIN(pri) + // keeps one row per second and takes the primary's, matching the dedup the chart already does. + // Passing the same id for both is byte-identical to the old single-id read. "SELECT COUNT(*) AS n, AVG(bpm) AS avg, MAX(bpm) AS max FROM (" + - "SELECT bpm FROM hrSample " + - "WHERE deviceId = :deviceId AND ts >= :from AND ts <= :to " + + "SELECT ts, MIN(pri), bpm FROM (" + + "SELECT ts, bpm, 0 AS pri FROM hrSample " + + "WHERE deviceId = :primaryId AND ts >= :from AND ts <= :to " + "UNION ALL " + - "SELECT p.bpm AS bpm FROM ppgHrSample p " + - "WHERE p.deviceId = :deviceId AND p.ts >= :from AND p.ts <= :to " + - "AND NOT EXISTS (SELECT 1 FROM hrSample h WHERE h.deviceId = p.deviceId AND h.ts = p.ts)" + + "SELECT p.ts AS ts, p.bpm AS bpm, 0 AS pri FROM ppgHrSample p " + + "WHERE p.deviceId = :primaryId AND p.ts >= :from AND p.ts <= :to " + + "AND NOT EXISTS (SELECT 1 FROM hrSample h WHERE h.deviceId = p.deviceId AND h.ts = p.ts) " + + "UNION ALL " + + "SELECT ts, bpm, 1 AS pri FROM hrSample " + + "WHERE deviceId = :secondaryId AND ts >= :from AND ts <= :to " + + "UNION ALL " + + "SELECT p.ts AS ts, p.bpm AS bpm, 1 AS pri FROM ppgHrSample p " + + "WHERE p.deviceId = :secondaryId AND p.ts >= :from AND p.ts <= :to " + + "AND NOT EXISTS (SELECT 1 FROM hrSample h WHERE h.deviceId = p.deviceId AND h.ts = p.ts) " + + ") GROUP BY ts" + ")" ) - suspend fun hrWindowStats(deviceId: String, from: Long, to: Long): HrWindowStats + suspend fun hrWindowStats(primaryId: String, secondaryId: String, from: Long, to: Long): HrWindowStats @Query( // #823: `ord` FIRST, so same-second beats come back in EMISSION order. Ordering by rrMs made diff --git a/android/app/src/main/java/com/noop/data/WhoopRepository.kt b/android/app/src/main/java/com/noop/data/WhoopRepository.kt index 780bfaeb9e..aee2060ad0 100644 --- a/android/app/src/main/java/com/noop/data/WhoopRepository.kt +++ b/android/app/src/main/java/com/noop/data/WhoopRepository.kt @@ -742,6 +742,14 @@ class WhoopRepository(private val dao: WhoopDao) { suspend fun hrBuckets(deviceId: String, from: Long, to: Long, bucketSeconds: Long = 300L) = dao.hrBuckets(deviceId, from, to, bucketSeconds) + /** #856: the same dedup over an EXPLICIT id list, so a workout can read its OWN recording strap + * rather than the day-level active ∪ canonical. Order is precedence — earlier ids win per bucket + * start, exactly as [hrBucketsUnion] does. One id ⇒ a plain single-id read. */ + suspend fun hrBucketsFor(deviceIds: List, from: Long, to: Long, bucketSeconds: Long = 300L): + List = + if (deviceIds.isEmpty()) emptyList() + else mergeHrBucketsByStart(deviceIds.map { dao.hrBuckets(it, from, to, bucketSeconds) }) + /** * Downsampled HR buckets over the read-side UNION of the active strap id AND the canonical "my-whoop" * (SPINE / #814 + HIGH-2), deduped by bucket start with the active strap winning. Kotlin twin of the @@ -778,7 +786,7 @@ class WhoopRepository(private val dao: WhoopDao) { rows: List, // HR read key for IMPORTED rows ONLY (Apple/HC/CSV/activity file): they carry no strap HR of their // own, so #77 derives it from the worn strap. STRAP-NATIVE rows ignore this and key on their OWN - // recording strap (see [workoutHrDeviceId]). The canonical "my-whoop" default is the worn strap on a + // recording strap (see [workoutHrDeviceIds]). The canonical "my-whoop" default is the worn strap on a // single-WHOOP install (and every current caller uses it); which strap was worn during an imported // session on a MULTI-strap install is undetermined, so that case is left as-is (not the active strap). strapDeviceId: String = "my-whoop", @@ -807,14 +815,19 @@ class WhoopRepository(private val dao: WhoopDao) { // for a strap-native row (never a hardcoded id), the [strapDeviceId] worn-strap default for an // imported one. A 2nd WHOOP (id "whoop-") used to read the empty "my-whoop" window, so its // strap-native workouts' Avg HR wasn't reconciled from the trace and a null Effort wasn't recomputed. - val hrDeviceId = workoutHrDeviceId(row.source, row.deviceId, strapDeviceId) - val stats = dao.hrWindowStats(hrDeviceId, row.startTs, row.endTs) + // #856: up to two ids, the first winning per second. A detected bout reads only the strap + // that recorded it; an imported row reads the active ∪ canonical union, because it has no + // strap of its own and the worn strap may bank under either after a re-add. + val hrIds = workoutHrDeviceIds(row.source, row.deviceId, strapDeviceId) + val stats = dao.hrWindowStats( + hrIds[0], hrIds.getOrElse(1) { hrIds[0] }, row.startTs, row.endTs, + ) if (stats.n < minSamples || stats.avg == null || stats.max == null) return@map row // #961: recompute Effort from the SAME samples the graph/zones use. Read the raw window ONLY when // this row actually needs a strain (keeps the common no-fill path a single aggregate query), and // let StrainScorer return null on a still-too-thin window (never a fabricated number). val filledStrain = if (needsStrainFill && strainMaxHR != null) { - val samples = dao.hrSamples(hrDeviceId, row.startTs, row.endTs, 8000) + val samples = dao.hrSamples(hrIds[0], row.startTs, row.endTs, 8000) com.noop.analytics.StrainScorer.strain(samples, maxHR = strainMaxHR, sex = strainSex) } else null if (strapNative) { @@ -1575,7 +1588,7 @@ class WhoopRepository(private val dao: WhoopDao) { /** A workout row is STRAP-NATIVE when NOOP recorded/scored it from a strap trace: a "manual" * session or a detected bout (source "-noop"). Everything else (Apple Health / Health Connect / * WHOOP CSV / activity file) is IMPORTED and carries its own avg/max. Single source of truth for the - * classification shared by [fillWorkoutHrFromStrap] and [workoutHrDeviceId]. */ + * classification shared by [fillWorkoutHrFromStrap] and [workoutHrDeviceIds]. */ fun isStrapNativeWorkout(source: String): Boolean { val s = source.lowercase() return s == "manual" || s.endsWith("-noop") @@ -1588,8 +1601,9 @@ class WhoopRepository(private val dao: WhoopDao) { * the active strap [activeStrapId]. Before this both keyed on a hardcoded "my-whoop", so a strap-native * workout on a SECOND WHOOP ("whoop-") read an empty window — its Avg HR went un-reconciled and a * null Effort un-recomputed. (This fill only sets avgHr/maxHr/strain; calories come from the detector.) */ - fun workoutHrDeviceId(source: String, rowDeviceId: String, activeStrapId: String): String = - if (isStrapNativeWorkout(source)) rowDeviceId.removeSuffix("-noop") else activeStrapId + fun workoutHrDeviceIds(source: String, rowDeviceId: String, activeStrapId: String): List = + if (isStrapNativeWorkout(source)) listOf(rowDeviceId.removeSuffix("-noop")) + else importedSourceIdsFor(activeStrapId) /** Default row cap on range reads. Matches the Swift call sites' bounded scans. */ const val DEFAULT_LIMIT = 100_000 diff --git a/android/app/src/main/java/com/noop/ui/AppViewModel.kt b/android/app/src/main/java/com/noop/ui/AppViewModel.kt index 057e80d7c7..61921b7da3 100644 --- a/android/app/src/main/java/com/noop/ui/AppViewModel.kt +++ b/android/app/src/main/java/com/noop/ui/AppViewModel.kt @@ -1651,11 +1651,22 @@ class AppViewModel(app: Application) : AndroidViewModel(app) { * HR-curve. A short session wants a finer bucket than the Today 24h chart (300 s would flatten a * 30-min run to ~6 points), so the bucket scales with duration: ~120 buckets across the window, * floored at 15 s and capped at 300 s. Mirrors macOS Repository.workoutHrBuckets. */ - suspend fun workoutHrBuckets(from: Long, to: Long): List { + suspend fun workoutHrBuckets( + from: Long, + to: Long, + source: String = "", + rowDeviceId: String = deviceId, + ): List { if (to <= from) return emptyList() val span = to - from val bucket = (span / 120).coerceIn(15L, 300L) - return runCatching { repository.hrBuckets(deviceId, from, to, bucket) }.getOrDefault(emptyList()) + // #856: read the ids this row actually belongs to. A bout detected on a SECOND WHOOP charts + // from the strap that recorded it; an imported row gets the active ∪ canonical union, since it + // has no strap of its own and the worn strap may bank under either after a re-add. Previously + // this read the single active id, so both cases could chart the wrong data — and disagree with + // the Avg HR on the same card. Defaults keep any caller without a row on today's behaviour. + val ids = WhoopRepository.workoutHrDeviceIds(source, rowDeviceId, deviceId) + return runCatching { repository.hrBucketsFor(ids, from, to, bucket) }.getOrDefault(emptyList()) } /** Per-zone MINUTES for a workout window, binning the strap's raw HR samples into the age-derived diff --git a/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt b/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt index 2919a1e5fd..86c7e75751 100644 --- a/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt +++ b/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt @@ -1295,7 +1295,7 @@ private fun WorkoutDetailSheet(vm: AppViewModel, row: WorkoutRow, onDismiss: () // so it "fills in after sync". null for non-foot sports or when no strap counter covers the window. var steps by remember(row.startTs) { mutableStateOf(null) } LaunchedEffect(row.startTs, row.endTs) { - hrCurve = vm.workoutHrBuckets(row.startTs, row.endTs).map { it.avgBpm } + hrCurve = vm.workoutHrBuckets(row.startTs, row.endTs, row.source, row.deviceId).map { it.avgBpm } steps = if (WorkoutSport.isOnFoot(row.sport)) vm.workoutSteps(row.startTs, row.endTs) else null val imported = parseZonePercents(row.zonesJSON) if (imported != null) { diff --git a/android/app/src/test/java/com/noop/data/WorkoutHrDeviceKeyTest.kt b/android/app/src/test/java/com/noop/data/WorkoutHrDeviceKeyTest.kt index 169c128050..9902cd60da 100644 --- a/android/app/src/test/java/com/noop/data/WorkoutHrDeviceKeyTest.kt +++ b/android/app/src/test/java/com/noop/data/WorkoutHrDeviceKeyTest.kt @@ -13,6 +13,11 @@ import org.junit.Test */ class WorkoutHrDeviceKeyTest { + // #856: the resolver returns a LIST now — one id for a detected bout (the strap that recorded it), + // the active ∪ canonical union for manual/imported. These cases all assert the id that wins, which + // is `.first()` in both branches, so the original expectations are unchanged. + + @Test fun `strap-native sources are classified as strap-native`() { assertTrue(WhoopRepository.isStrapNativeWorkout("manual")) assertTrue(WhoopRepository.isStrapNativeWorkout("MANUAL")) // case-insensitive @@ -34,19 +39,19 @@ class WorkoutHrDeviceKeyTest { // "whoop-aabbcc". The active strap being something else must NOT redirect the read. assertEquals( "whoop-aabbcc", - WhoopRepository.workoutHrDeviceId("whoop-aabbcc-noop", "whoop-aabbcc-noop", activeStrapId = "my-whoop"), + WhoopRepository.workoutHrDeviceIds("whoop-aabbcc-noop", "whoop-aabbcc-noop", activeStrapId = "my-whoop").first(), ) // Canonical single-WHOOP detected row is unchanged from the old "my-whoop" behaviour. assertEquals( "my-whoop", - WhoopRepository.workoutHrDeviceId("my-whoop-noop", "my-whoop-noop", activeStrapId = "my-whoop"), + WhoopRepository.workoutHrDeviceIds("my-whoop-noop", "my-whoop-noop", activeStrapId = "my-whoop").first(), ) } @Test fun `manual row reads HR under its own strap id`() { assertEquals( "whoop-aabbcc", - WhoopRepository.workoutHrDeviceId("manual", "whoop-aabbcc", activeStrapId = "my-whoop"), + WhoopRepository.workoutHrDeviceIds("manual", "whoop-aabbcc", activeStrapId = "my-whoop").first(), ) } @@ -54,11 +59,11 @@ class WorkoutHrDeviceKeyTest { // An Apple/HC/activity-file row carries no strap HR; #77 fills it from the worn strap = active strap. assertEquals( "whoop-aabbcc", - WhoopRepository.workoutHrDeviceId("apple-health", "apple-health", activeStrapId = "whoop-aabbcc"), + WhoopRepository.workoutHrDeviceIds("apple-health", "apple-health", activeStrapId = "whoop-aabbcc").first(), ) assertEquals( "my-whoop", - WhoopRepository.workoutHrDeviceId("activity-file", "activity-file", activeStrapId = "my-whoop"), + WhoopRepository.workoutHrDeviceIds("activity-file", "activity-file", activeStrapId = "my-whoop").first(), ) } @@ -66,7 +71,7 @@ class WorkoutHrDeviceKeyTest { // removeSuffix is a no-op when the suffix is absent — a manual/base id must not be truncated. assertEquals( "whoop-aabbcc", - WhoopRepository.workoutHrDeviceId("manual", "whoop-aabbcc", activeStrapId = "ignored"), + WhoopRepository.workoutHrDeviceIds("manual", "whoop-aabbcc", activeStrapId = "ignored").first(), ) } } From 399c458987cd1593c8c01d8031f8c655eafacc97 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sun, 26 Jul 2026 19:44:11 -0700 Subject: [PATCH 2/5] =?UTF-8?q?fix(workouts):=20zone=20minutes=20too=20?= =?UTF-8?q?=E2=80=94=20the=20third=20surface,=20which=20I=20had=20missed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-reviewing this PR against what I promised on #856: "the chart, the ZONE MINUTES and hrWindowStats all read that list". Only two of the three were done. Left as it was, the card would have been NEWLY inconsistent rather than fixed — chart and Avg HR reading the recording strap while the zone split still binned someone else's samples: Swift zones hrSamples(from:to:) the day union -> wrong strap for a detected bout Kotlin zones single active id wrong for detected AND imported Both now resolve the same ids. Adds the sample-level twin of the bucket helper — Repository.hrSamples(deviceIds:) and WhoopRepository.hrSamplesFor — deduped by ts with earlier ids winning, so the zone split bins exactly the rows the curve plots. workoutZoneMinutes takes the row's source on both platforms, defaulted so a caller without a row keeps today's behaviour. --- Strand/Data/Repository.swift | 28 +++++++++++++++++-- Strand/Screens/WorkoutDetailView.swift | 3 +- .../java/com/noop/data/WhoopRepository.kt | 8 ++++++ .../src/main/java/com/noop/ui/AppViewModel.kt | 12 ++++++-- .../main/java/com/noop/ui/WorkoutsScreen.kt | 2 +- 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/Strand/Data/Repository.swift b/Strand/Data/Repository.swift index 99421b2065..1e3d54b387 100644 --- a/Strand/Data/Repository.swift +++ b/Strand/Data/Repository.swift @@ -911,6 +911,24 @@ final class Repository: ObservableObject { return await unionDailyMetrics(store: store, from: fromDay, to: toDay) } + /// #856: the same dedup over an EXPLICIT id list, so a workout's zone minutes bin the rows its own + /// recording strap banked rather than the day-level active ∪ canonical. Order is precedence. + func hrSamples(deviceIds: [String], from: Int, to: Int, limit: Int = 8000) async -> [HRSample] { + guard let store = await ensureStore(), !deviceIds.isEmpty else { return [] } + guard deviceIds.count > 1 else { + return (try? await store.hrSamples(deviceId: deviceIds[0], from: from, to: to, + limit: limit)) ?? [] + } + var byTs: [Int: HRSample] = [:] + for id in deviceIds { + for s in (try? await store.hrSamples(deviceId: id, from: from, to: to, + limit: limit)) ?? [] where byTs[s.ts] == nil { + byTs[s.ts] = s + } + } + return byTs.values.sorted { $0.ts < $1.ts } + } + func hrSamples(from: Int, to: Int, limit: Int = 8000) async -> [HRSample] { guard let store = await ensureStore() else { return [] } // UNION the active strap + canonical so the HR trend renders whether the landed day's raw sits under @@ -2622,9 +2640,15 @@ final class Repository: ObservableObject { /// `zonesJSON` still gets a real time-in-zone split. Returns nil when the window carries no HR (so /// the view shows nothing rather than five empty bars). `age <= 0` falls back to a 30 y default , /// the zones are approximate either way and clearly labelled as such in the UI. - func workoutZoneMinutes(from: Int, to: Int, age: Int) async -> [Double]? { + /// #856: bins the same rows the chart plots and the Avg HR aggregates. Previously this read the + /// day-level union, so a bout detected on a second WHOOP had its zones computed from a strap that + /// never recorded it. `source` defaults to "" (⇒ the imported branch, the union) so a caller + /// without a row keeps today's behaviour. + func workoutZoneMinutes(from: Int, to: Int, age: Int, source: String = "") async -> [Double]? { guard to > from else { return nil } - let samples = await hrSamples(from: from, to: to) + let ids = Self.workoutHrDeviceIds(source: source, activeStrapId: deviceId, + importedIds: importedReadIds) + let samples = await hrSamples(deviceIds: ids, from: from, to: to) guard !samples.isEmpty else { return nil } let zoneSet = HRZones.zones(age: age > 0 ? Double(age) : 30) let tiz = HRZones.timeInZone(samples, zoneSet: zoneSet) diff --git a/Strand/Screens/WorkoutDetailView.swift b/Strand/Screens/WorkoutDetailView.swift index 085bcee50f..771b77bcc6 100644 --- a/Strand/Screens/WorkoutDetailView.swift +++ b/Strand/Screens/WorkoutDetailView.swift @@ -123,7 +123,8 @@ struct WorkoutDetailView: View { } } if minutes == nil { - minutes = await repo.workoutZoneMinutes(from: row.startTs, to: row.endTs, age: profile.age) + minutes = await repo.workoutZoneMinutes(from: row.startTs, to: row.endTs, age: profile.age, + source: row.source) } let hrr = await repo.workoutHeartRateRecovery( diff --git a/android/app/src/main/java/com/noop/data/WhoopRepository.kt b/android/app/src/main/java/com/noop/data/WhoopRepository.kt index aee2060ad0..89aae70c14 100644 --- a/android/app/src/main/java/com/noop/data/WhoopRepository.kt +++ b/android/app/src/main/java/com/noop/data/WhoopRepository.kt @@ -689,6 +689,14 @@ class WhoopRepository(private val dao: WhoopDao) { suspend fun hrSamples(deviceId: String, from: Long, to: Long, limit: Int = DEFAULT_LIMIT) = dao.hrSamples(deviceId, from, to, limit) + /** #856: HR samples over an EXPLICIT id list, deduped by ts with earlier ids winning — the sample + * twin of [hrBucketsFor], so a workout's ZONE MINUTES bin the same rows its chart plots and its + * Avg HR aggregates. One id ⇒ a plain single-id read. */ + suspend fun hrSamplesFor(deviceIds: List, from: Long, to: Long, limit: Int = DEFAULT_LIMIT): + List = + if (deviceIds.isEmpty()) emptyList() + else mergeHrByTs(deviceIds.map { dao.hrSamples(it, from, to, limit) }) + /** * HR samples over the read-side UNION of the active strap id AND the canonical "my-whoop" (SPINE / * #814 + HIGH-2), deduped by ts with the active strap winning. This is the Kotlin twin of the Swift diff --git a/android/app/src/main/java/com/noop/ui/AppViewModel.kt b/android/app/src/main/java/com/noop/ui/AppViewModel.kt index 61921b7da3..81fc8fc694 100644 --- a/android/app/src/main/java/com/noop/ui/AppViewModel.kt +++ b/android/app/src/main/java/com/noop/ui/AppViewModel.kt @@ -1674,9 +1674,17 @@ class AppViewModel(app: Application) : AndroidViewModel(app) { * percentages, but from the strap's own samples so a session WITHOUT imported zones still gets a * real time-in-zone split. null when the window carries no HR. age <= 0 falls back to 30 y. * Mirrors macOS Repository.workoutZoneMinutes. */ - suspend fun workoutZoneMinutes(from: Long, to: Long): List? { + suspend fun workoutZoneMinutes( + from: Long, + to: Long, + source: String = "", + rowDeviceId: String = deviceId, + ): List? { if (to <= from) return null - val samples = runCatching { repository.hrSamples(deviceId, from, to) }.getOrDefault(emptyList()) + // #856: the same resolved ids the chart and Avg HR use. Binning a different strap's samples + // than the curve plots would put three different answers on one card. + val ids = WhoopRepository.workoutHrDeviceIds(source, rowDeviceId, deviceId) + val samples = runCatching { repository.hrSamplesFor(ids, from, to) }.getOrDefault(emptyList()) if (samples.isEmpty()) return null val age = profileStore.age.toDouble().takeIf { it > 0 } ?: 30.0 val zoneSet = com.noop.analytics.HrZones.zones(age = age) diff --git a/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt b/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt index 86c7e75751..e7521a2ec4 100644 --- a/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt +++ b/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt @@ -1306,7 +1306,7 @@ private fun WorkoutDetailSheet(vm: AppViewModel, row: WorkoutRow, onDismiss: () } } if (zoneMinutes == null) { - zoneMinutes = vm.workoutZoneMinutes(row.startTs, row.endTs) + zoneMinutes = vm.workoutZoneMinutes(row.startTs, row.endTs, row.source, row.deviceId) zonesFromImport = false } heartRateRecovery = vm.workoutHeartRateRecovery(row.startTs, row.endTs) From 9a3322cf37696478f36d3506f3113a8f57011564 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sun, 26 Jul 2026 20:07:00 -0700 Subject: [PATCH 3/5] test: update the Swift resolver test to the list API, and cover what the rename is for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I renamed workoutHrDeviceId -> workoutHrDeviceIds and updated the KOTLIN resolver test while missing its SWIFT twin, which still called the old name — a compile break in StrandTests. Nothing caught it: StrandTests runs only under xcodebuild on macOS, and app-build.yml is disabled. Anyone running tests locally on a Mac would have hit it, which #854 has just made possible again. The four existing cases keep their expectations, now as single-element lists. Added the two cases none of them covered, which are the whole point of the change: an IMPORTED row on a multi-namespace install resolves to the full #814 union (the case the single-id resolver silently under-read), while a DETECTED row on that same install still resolves to exactly one id — the union must not leak into the branch where it would pull in a strap that never recorded the bout. --- StrandTests/WorkoutHrDeviceKeyTests.swift | 42 ++++++++++++++++++----- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/StrandTests/WorkoutHrDeviceKeyTests.swift b/StrandTests/WorkoutHrDeviceKeyTests.swift index f34083595d..4b492485fb 100644 --- a/StrandTests/WorkoutHrDeviceKeyTests.swift +++ b/StrandTests/WorkoutHrDeviceKeyTests.swift @@ -3,7 +3,7 @@ import XCTest /// #510: `reconcileWorkoutHrWithTrace` used to read every workout's HR window under the Repository's single /// active `deviceId`, so a bout auto-detected on a SECOND WHOOP (its `hrSample` rows banked under that strap) -/// read the active strap's empty window and its Avg HR / Effort went un-reconciled. `Repository.workoutHrDeviceId` +/// read the active strap's empty window and its Avg HR / Effort went un-reconciled. `Repository.workoutHrDeviceIds` /// now resolves the read key per row from the workout's own `source`. Kotlin twin: `WorkoutHrDeviceKeyTest`. /// /// NB the Swift read-model `WorkoutRow` carries no `deviceId`, so only DETECTED rows (whose `source` IS their @@ -15,24 +15,28 @@ final class WorkoutHrDeviceKeyTests: XCTestCase { /// The active strap being something else must NOT redirect the read to an empty window. func testDetectedReadsItsOwnBaseStrap_notActive() { XCTAssertEqual( - Repository.workoutHrDeviceId(source: "whoop-aabbcc-noop", activeStrapId: "my-whoop"), - "whoop-aabbcc") + Repository.workoutHrDeviceIds(source: "whoop-aabbcc-noop", activeStrapId: "my-whoop", + importedIds: ["my-whoop"]), + ["whoop-aabbcc"], + "#856: exactly ONE id — unioning would mix in a strap that never recorded this bout") } /// Single-WHOOP: the canonical detected id strips to the active id, so the read is byte-identical to the /// old `self.deviceId` behaviour. func testDetectedCanonicalIsByteIdenticalToActive() { XCTAssertEqual( - Repository.workoutHrDeviceId(source: "my-whoop-noop", activeStrapId: "my-whoop"), - "my-whoop") + Repository.workoutHrDeviceIds(source: "my-whoop-noop", activeStrapId: "my-whoop", + importedIds: ["my-whoop"]), + ["my-whoop"]) } /// Manual rows carry no strap id in `source`, so they reconcile against the active strap (the documented /// display-only divergence from the Kotlin twin, which keys manual rows on their stored deviceId). func testManualReadsActiveStrap() { XCTAssertEqual( - Repository.workoutHrDeviceId(source: "manual", activeStrapId: "whoop-aabbcc"), - "whoop-aabbcc") + Repository.workoutHrDeviceIds(source: "manual", activeStrapId: "whoop-aabbcc", + importedIds: ["whoop-aabbcc"]), + ["whoop-aabbcc"]) } /// Imported rows (Apple / activity file / lifting / WHOOP CSV) reconcile against the active strap — the @@ -41,9 +45,29 @@ final class WorkoutHrDeviceKeyTests: XCTestCase { func testImportedReadsActiveStrap() { for src in ["apple-health", "apple_health", "activity-file", "lifting", "my-whoop", "Health Connect"] { XCTAssertEqual( - Repository.workoutHrDeviceId(source: src, activeStrapId: "whoop-aabbcc"), - "whoop-aabbcc", + Repository.workoutHrDeviceIds(source: src, activeStrapId: "whoop-aabbcc", + importedIds: ["whoop-aabbcc"]), + ["whoop-aabbcc"], "imported source \(src) should reconcile against the active strap") } } + + /// #856: an imported row on a MULTI-namespace install gets the whole #814 union, not just the + /// active strap — it has no strap of its own, and after a re-add the worn strap's history may sit + /// under the canonical id. This is the case the single-id resolver silently under-read. + func testImportedGetsTheFullUnionWhenTheyDiffer() { + XCTAssertEqual( + Repository.workoutHrDeviceIds(source: "apple-health", activeStrapId: "whoop-aabbcc", + importedIds: ["whoop-aabbcc", "my-whoop"]), + ["whoop-aabbcc", "my-whoop"]) + } + + /// …while a DETECTED row on that same install still resolves to one id. The union must not leak + /// into the branch where it would be wrong. + func testDetectedIgnoresTheUnionEvenWhenOneIsAvailable() { + XCTAssertEqual( + Repository.workoutHrDeviceIds(source: "whoop-aabbcc-noop", activeStrapId: "my-whoop", + importedIds: ["my-whoop", "whoop-zzz"]), + ["whoop-aabbcc"]) + } } From 43046d0207d214847ffcbc0e06a25006eb32347a Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sun, 26 Jul 2026 20:17:17 -0700 Subject: [PATCH 4/5] =?UTF-8?q?fix(workouts):=20HRR=20too=20=E2=80=94=20th?= =?UTF-8?q?e=20fourth=20surface=20on=20the=20same=20card?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asked whether this actually resolves #856 and went looking for surfaces I had not touched. Heart-rate recovery (#516) was reading the day-level union on BOTH platforms: Kotlin via hrSamplesUnion, Swift via hrSamples(from:to:). No parity break there — both were wrong the same way — but it is the same wrong-strap error as the chart, and leaving it would have recreated on a fourth surface exactly the inconsistency this PR removes from the other three. Its window extends PAST the bout, which is the one reason it might have wanted different treatment. It does not: the strap that recorded the session is still the one on the wrist a few minutes later, so a detected bout reads its own strap here too. All four workout HR surfaces now resolve through workoutHrDeviceIds on both platforms: chart, zone minutes, Avg HR / Effort, and HRR. --- Strand/Data/Repository.swift | 10 ++++++++-- .../app/src/main/java/com/noop/ui/AppViewModel.kt | 13 +++++++++++-- .../app/src/main/java/com/noop/ui/WorkoutsScreen.kt | 4 ++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/Strand/Data/Repository.swift b/Strand/Data/Repository.swift index 1e3d54b387..495ff29e6e 100644 --- a/Strand/Data/Repository.swift +++ b/Strand/Data/Repository.swift @@ -2660,11 +2660,17 @@ final class Repository: ObservableObject { /// post-workout minutes. This is a narrow read (at most ~10 minutes), not a whole-workout scan, and the /// pure engine owns every eligibility/coverage guard. Missing post-workout HR therefore returns nil /// instead of fabricating a recovery value. Kotlin twin: `AppViewModel.workoutHeartRateRecovery`. - func workoutHeartRateRecovery(from: Int, to: Int, maxHR: Double) async -> HeartRateRecovery.Result? { + /// #856: reads the ids this row resolves to, like the chart, zones and Avg HR. The recovery window + /// extends PAST the bout, but the strap that recorded it is still the one on the wrist a few + /// minutes later — so a detected bout reads its own strap here too rather than the day union. + func workoutHeartRateRecovery(from: Int, to: Int, maxHR: Double, + source: String = "") async -> HeartRateRecovery.Result? { guard to > from, maxHR > 0 else { return nil } let readFrom = max(from, to - HeartRateRecovery.eligibilityLookbackSeconds) let readTo = to + 5 * 60 + HeartRateRecovery.measurementToleranceSeconds - let samples = await hrSamples(from: readFrom, to: readTo, limit: 2_000) + let ids = Self.workoutHrDeviceIds(source: source, activeStrapId: deviceId, + importedIds: importedReadIds) + let samples = await hrSamples(deviceIds: ids, from: readFrom, to: readTo, limit: 2_000) return HeartRateRecovery.calculate(samples: samples, workoutStart: from, workoutEnd: to, maxHR: maxHR) } diff --git a/android/app/src/main/java/com/noop/ui/AppViewModel.kt b/android/app/src/main/java/com/noop/ui/AppViewModel.kt index 81fc8fc694..82c6234b40 100644 --- a/android/app/src/main/java/com/noop/ui/AppViewModel.kt +++ b/android/app/src/main/java/com/noop/ui/AppViewModel.kt @@ -1696,12 +1696,21 @@ class AppViewModel(app: Application) : AndroidViewModel(app) { /** HRR for one workout (#516), derived from a narrow workout-end + post-workout HR read. The pure * engine owns the intensity and coverage gates, so a disconnect after exercise returns null rather * than an interpolated value. Mirrors macOS Repository.workoutHeartRateRecovery. */ - suspend fun workoutHeartRateRecovery(from: Long, to: Long): com.noop.analytics.HeartRateRecovery.Result? { + suspend fun workoutHeartRateRecovery( + from: Long, + to: Long, + source: String = "", + rowDeviceId: String = deviceId, + ): com.noop.analytics.HeartRateRecovery.Result? { if (to <= from) return null val readFrom = maxOf(from, to - com.noop.analytics.HeartRateRecovery.eligibilityLookbackSeconds) val readTo = to + 5 * 60 + com.noop.analytics.HeartRateRecovery.measurementToleranceSeconds + // #856: the same resolved ids as the chart, zones and Avg HR — the fourth surface on this card. + // The recovery window extends PAST the bout, but the strap that recorded it is still the one on + // the wrist a few minutes later, so a detected bout reads its own strap here too. + val ids = WhoopRepository.workoutHrDeviceIds(source, rowDeviceId, deviceId) val samples = runCatching { - repository.hrSamplesUnion(activeStrapId, readFrom, readTo, limit = 2_000) + repository.hrSamplesFor(ids, readFrom, readTo, limit = 2_000) }.getOrDefault(emptyList()) return com.noop.analytics.HeartRateRecovery.calculate( samples = samples, diff --git a/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt b/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt index e7521a2ec4..b7bd055699 100644 --- a/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt +++ b/android/app/src/main/java/com/noop/ui/WorkoutsScreen.kt @@ -201,7 +201,7 @@ fun WorkoutsScreen(vm: AppViewModel) { LaunchedEffect(recoveryInputKey, vm.activeStrapId, lastHistorySyncAt) { val built = ArrayList() for (row in recoveryRows) { - val result = vm.workoutHeartRateRecovery(row.startTs, row.endTs) ?: continue + val result = vm.workoutHeartRateRecovery(row.startTs, row.endTs, row.source, row.deviceId) ?: continue built += WorkoutRecoveryTrendPoint(row.startTs, result) } recoveryTrend = built @@ -1309,7 +1309,7 @@ private fun WorkoutDetailSheet(vm: AppViewModel, row: WorkoutRow, onDismiss: () zoneMinutes = vm.workoutZoneMinutes(row.startTs, row.endTs, row.source, row.deviceId) zonesFromImport = false } - heartRateRecovery = vm.workoutHeartRateRecovery(row.startTs, row.endTs) + heartRateRecovery = vm.workoutHeartRateRecovery(row.startTs, row.endTs, row.source, row.deviceId) } ModalBottomSheet( From 0d804095817ce9d6aee3adb4884f9ae178ccb3ea Mon Sep 17 00:00:00 2001 From: ryanbr Date: Sun, 26 Jul 2026 20:17:59 -0700 Subject: [PATCH 5/5] fix(workouts): thread the row's source into the two Swift HRR call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit added the parameter and updated Kotlin, but a failed edit meant the Swift call sites never got it. Because `source` is defaulted, that compiled and stayed green while silently keeping HRR on the old union read — a change that looks applied and is not, which is the worst shape for a defect to take. Caught by grepping the call sites rather than trusting the commit. --- Strand/Screens/WorkoutDetailView.swift | 2 +- Strand/Screens/WorkoutsView.swift | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Strand/Screens/WorkoutDetailView.swift b/Strand/Screens/WorkoutDetailView.swift index 771b77bcc6..18ddfdb4f6 100644 --- a/Strand/Screens/WorkoutDetailView.swift +++ b/Strand/Screens/WorkoutDetailView.swift @@ -128,7 +128,7 @@ struct WorkoutDetailView: View { } let hrr = await repo.workoutHeartRateRecovery( - from: row.startTs, to: row.endTs, maxHR: Double(profile.hrMax)) + from: row.startTs, to: row.endTs, maxHR: Double(profile.hrMax), source: row.source) // Steps for an on-foot session (#398), computed at display time over the exact window so it // "fills in after sync": prefer the strap's own counter (MG/5.0) once it has offloaded the window, diff --git a/Strand/Screens/WorkoutsView.swift b/Strand/Screens/WorkoutsView.swift index 9b5a96239c..6fcddb0176 100644 --- a/Strand/Screens/WorkoutsView.swift +++ b/Strand/Screens/WorkoutsView.swift @@ -304,7 +304,8 @@ struct WorkoutsView: View { for row in recoveryTrendRows { if Task.isCancelled { return } if let result = await repo.workoutHeartRateRecovery( - from: row.startTs, to: row.endTs, maxHR: Double(model.profile.hrMax)) { + from: row.startTs, to: row.endTs, maxHR: Double(model.profile.hrMax), + source: row.source) { built.append(WorkoutRecoveryTrendPoint(startTs: row.startTs, result: result)) } }