From 3f3cfc198af013c18313bed3e2ee8bb3744d7ae1 Mon Sep 17 00:00:00 2001 From: Utku Deniz Altiok <93100191+UtkuDenizAltiok@users.noreply.github.com> Date: Wed, 23 Sep 2026 05:19:38 +0200 Subject: [PATCH 1/2] workouts: record one heart-rate sample a second in a manual workout A manual workout scores its Effort -- live on the card and saved with the workout -- from the heart-rate samples it records, and both platforms recorded more than one for the same second. iOS runs captureWorkoutSample from two @Published sinks (heartRate and rr), so a second in which the rate moves arrives twice; Android runs it on every LiveState emission that carries a heart rate. StrainScorer credits a zero gap with a full second (sampleDurationsMinutes), so each repeat counted as another second of effort: twenty minutes of 100-160 bpm with every second arriving twice scored 42.86 instead of 35.3. Calories integrate over the real gaps and were not affected. A second that already has its sample now takes no other (iOS ActiveWorkout.recordSample, the same guard inline on Android), which also stops a repeat rescoring the workout and re-saving its snapshot. Co-Authored-By: Claude Opus 5.5 --- Strand/App/AppModel.swift | 16 +++++++- .../WorkoutSampleOncePerSecondTests.swift | 40 +++++++++++++++++++ .../src/main/java/com/noop/ui/AppViewModel.kt | 8 +++- 3 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 StrandTests/WorkoutSampleOncePerSecondTests.swift diff --git a/Strand/App/AppModel.swift b/Strand/App/AppModel.swift index 588a25d37e..fac0e70012 100644 --- a/Strand/App/AppModel.swift +++ b/Strand/App/AppModel.swift @@ -127,6 +127,19 @@ final class AppModel: ObservableObject { var isPaused: Bool { pausedAt != nil } + /// Adds a heart-rate sample unless this second already has one, and says whether it did. + /// + /// `captureWorkoutSample` runs from two `@Published` sinks (`heartRate` and `rr`), so a strap sends + /// it one call per R-R packet plus another whenever the rate itself moves, which during exercise is + /// most seconds: two samples with one `ts`. Effort credits each sample with the gap to the next and a + /// zero gap with a full second (`StrainScorer.sampleDurationsMinutes`), so every repeat counted as + /// another second of effort, live and in the saved workout. The stream is one reading a second. + mutating func recordSample(_ sample: HRSample) -> Bool { + if let last = samples.last, last.ts == sample.ts { return false } + samples.append(sample) + return true + } + /// Delegates to `ActiveWorkoutClock` so this and the two card surfaces cannot drift apart again. func elapsed(at now: Date = Date()) -> TimeInterval { ActiveWorkoutClock.activeElapsed(start: start, pausedAt: pausedAt, @@ -1045,7 +1058,8 @@ final class AppModel: ObservableObject { /// over the growing window each sample is cheap at the ~1 Hz live-HR cadence. private func captureWorkoutSample() { guard var w = activeWorkout, !w.isPaused, let hr = bpm else { return } - w.samples.append(HRSample(ts: Int(Date().timeIntervalSince1970), bpm: hr)) + // A second that already has its sample changes nothing, so nothing is rescored or re-saved for it. + guard w.recordSample(HRSample(ts: Int(Date().timeIntervalSince1970), bpm: hr)) else { return } w.peakHr = max(w.peakHr, hr) w.avgHr = Int((Double(w.samples.map(\.bpm).reduce(0, +)) / Double(w.samples.count)).rounded()) w.liveStrain = StrainScorer.strain(w.samples, maxHR: Double(profile.hrMax), diff --git a/StrandTests/WorkoutSampleOncePerSecondTests.swift b/StrandTests/WorkoutSampleOncePerSecondTests.swift new file mode 100644 index 0000000000..f76bd2d1a5 --- /dev/null +++ b/StrandTests/WorkoutSampleOncePerSecondTests.swift @@ -0,0 +1,40 @@ +import XCTest +import StrandAnalytics +import WhoopProtocol +@testable import Strand + +/// A manual workout records one heart-rate sample a second. The live heart rate reaches it once per R-R packet and +/// again whenever the rate moves, and Effort credits a repeated second as another second of effort. +final class WorkoutSampleOncePerSecondTests: XCTestCase { + + func testASecondThatAlreadyHasASampleTakesNoOther() { + var workout = AppModel.ActiveWorkout(start: Date(timeIntervalSince1970: 1_000)) + XCTAssertTrue(workout.recordSample(HRSample(ts: 1_000, bpm: 120))) + XCTAssertFalse(workout.recordSample(HRSample(ts: 1_000, bpm: 121))) + XCTAssertTrue(workout.recordSample(HRSample(ts: 1_001, bpm: 122))) + XCTAssertEqual(workout.samples.map(\.ts), [1_000, 1_001]) + XCTAssertEqual(workout.samples.map(\.bpm), [120, 122]) + } + + /// Twenty minutes climbing from 100 to 160 bpm, each second arriving twice (the R-R packet, then the rate + /// moving) as it does during exercise. Recorded through `recordSample`, the workout's Effort is the Effort of + /// the plain once-a-second stream; the repeats, kept, scored higher. + func testRepeatedSecondsNoLongerAddEffort() { + let seconds = 20 * 60 + let clean = (0.. Double { + StrainScorer.strain(samples, maxHR: 190, restingHR: 60) ?? 0 + } + XCTAssertEqual(workout.samples.map(\.ts), clean.map(\.ts)) + XCTAssertEqual(effort(workout.samples), effort(clean)) + XCTAssertGreaterThan(effort(everyArrival), effort(clean)) + } +} 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 d66d6d0f4e..e3d55e59e2 100644 --- a/android/app/src/main/java/com/noop/ui/AppViewModel.kt +++ b/android/app/src/main/java/com/noop/ui/AppViewModel.kt @@ -1801,7 +1801,13 @@ class AppViewModel(app: Application) : AndroidViewModel(app) { @Suppress("UNNECESSARY_SAFE_CALL") val w = _activeWorkout?.value ?: return if (w.pausedAtMs != null) return - val s = w.samples + HrSample(deviceId = deviceId, ts = System.currentTimeMillis() / 1000, bpm = bpm) + // One sample a second. This runs on every LiveState emission that carries a heart rate — any field + // changing, not only the rate — so a second often arrives more than once with one `ts`, and Effort + // credits a zero gap with a full second (`StrainScorer.sampleDurationsMinutes`): each repeat counted + // as another second of effort, live and in the saved workout. Twin of iOS `ActiveWorkout.recordSample`. + val ts = System.currentTimeMillis() / 1000 + if (w.samples.lastOrNull()?.ts == ts) return + val s = w.samples + HrSample(deviceId = deviceId, ts = ts, bpm = bpm) val strain = StrainScorer.strain( s, maxHR = profileStore.hrMax.toDouble(), method = NoopPrefs.effortMethod(appContext), sex = profileStore.sex) ?: 0.0 From fb774dd24af03eb6c2cdb252133f0f25ecc3cc61 Mon Sep 17 00:00:00 2001 From: Utku Deniz Altiok <93100191+UtkuDenizAltiok@users.noreply.github.com> Date: Wed, 23 Sep 2026 12:46:30 +0200 Subject: [PATCH 2/2] workouts: a repeated second still reaches the workout's peak Review on #2416: the sinks fire because the rate moved, so the reading refused for a repeated second is often a different bpm, and returning before the peak discarded a within-second high. A repeat now folds into peakHr (iOS ActiveWorkout.recordSample; Android inline) and only the peak is published for it, with no rescore and no snapshot. Both platforms also saved the workout's maxHr from the samples alone, so the saved max takes the folded peak too (iOS ActiveWorkout.savedPeak, Android maxOf(samples max, w.peakHr)), and Android now grows the live peak by comparison instead of recomputing it from the samples, which would drop the fold at the next second. recordSample's doc says it compares only the last sample: a repeat, not an out-of-order arrival. Co-Authored-By: Claude Opus 5.5 --- Strand/App/AppModel.swift | 25 ++++++++++++++++--- .../WorkoutSampleOncePerSecondTests.swift | 15 +++++++++++ .../src/main/java/com/noop/ui/AppViewModel.kt | 15 ++++++++--- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/Strand/App/AppModel.swift b/Strand/App/AppModel.swift index fac0e70012..e4107eea73 100644 --- a/Strand/App/AppModel.swift +++ b/Strand/App/AppModel.swift @@ -134,12 +134,24 @@ final class AppModel: ObservableObject { /// most seconds: two samples with one `ts`. Effort credits each sample with the gap to the next and a /// zero gap with a full second (`StrainScorer.sampleDurationsMinutes`), so every repeat counted as /// another second of effort, live and in the saved workout. The stream is one reading a second. + /// + /// A refused reading still reaches `peakHr`: the sinks fire because the rate moved, so a repeat is often a + /// different bpm for that second, and a within-second high is part of the workout's peak. Only the last + /// sample is compared, so this drops a repeat of the current second, not an out-of-order arrival; the live + /// stream is monotonic. mutating func recordSample(_ sample: HRSample) -> Bool { - if let last = samples.last, last.ts == sample.ts { return false } + if let last = samples.last, last.ts == sample.ts { + peakHr = max(peakHr, sample.bpm) + return false + } samples.append(sample) return true } + /// The maximum heart rate the workout is saved with: the highest sample, or a higher reading a repeated + /// second folded into `peakHr` (`recordSample`). Nil with no samples. + var savedPeak: Int? { samples.map(\.bpm).max().map { max($0, peakHr) } } + /// Delegates to `ActiveWorkoutClock` so this and the two card surfaces cannot drift apart again. func elapsed(at now: Date = Date()) -> TimeInterval { ActiveWorkoutClock.activeElapsed(start: start, pausedAt: pausedAt, @@ -998,7 +1010,7 @@ final class AppModel: ObservableObject { } let avg = samples.isEmpty ? nil : Int((Double(samples.map(\.bpm).reduce(0, +)) / Double(samples.count)).rounded()) - let peak = samples.map(\.bpm).max() + let peak = w.savedPeak // #983: score the SAVED workout with the wearer's measured resting HR, not the hardcoded // default of 60. %HRR is (bpm - resting) / (max - resting), so the default moves every zone // boundary — at 136 bpm with maxHR 190 it is the difference between zone 1 and zone 2. Today's @@ -1058,8 +1070,13 @@ final class AppModel: ObservableObject { /// over the growing window each sample is cheap at the ~1 Hz live-HR cadence. private func captureWorkoutSample() { guard var w = activeWorkout, !w.isPaused, let hr = bpm else { return } - // A second that already has its sample changes nothing, so nothing is rescored or re-saved for it. - guard w.recordSample(HRSample(ts: Int(Date().timeIntervalSince1970), bpm: hr)) else { return } + // A second that already has its sample moves only the peak: publish that, and skip the rescore and the + // snapshot (the next second's sample carries the peak into the snapshot). + let peakBefore = w.peakHr + guard w.recordSample(HRSample(ts: Int(Date().timeIntervalSince1970), bpm: hr)) else { + if w.peakHr != peakBefore { activeWorkout = w } + return + } w.peakHr = max(w.peakHr, hr) w.avgHr = Int((Double(w.samples.map(\.bpm).reduce(0, +)) / Double(w.samples.count)).rounded()) w.liveStrain = StrainScorer.strain(w.samples, maxHR: Double(profile.hrMax), diff --git a/StrandTests/WorkoutSampleOncePerSecondTests.swift b/StrandTests/WorkoutSampleOncePerSecondTests.swift index f76bd2d1a5..92be51c84e 100644 --- a/StrandTests/WorkoutSampleOncePerSecondTests.swift +++ b/StrandTests/WorkoutSampleOncePerSecondTests.swift @@ -16,6 +16,21 @@ final class WorkoutSampleOncePerSecondTests: XCTestCase { XCTAssertEqual(workout.samples.map(\.bpm), [120, 122]) } + /// The rate moving is often why a second arrives twice, so the refused reading can be that second's high: it is + /// not recorded, but it is the workout's peak, live and saved. + func testARepeatedSecondStillReachesThePeak() { + var workout = AppModel.ActiveWorkout(start: Date(timeIntervalSince1970: 1_000)) + XCTAssertNil(workout.savedPeak) + _ = workout.recordSample(HRSample(ts: 1_000, bpm: 120)) + workout.peakHr = 120 // what captureWorkoutSample does after an accepted sample + XCTAssertFalse(workout.recordSample(HRSample(ts: 1_000, bpm: 131))) + XCTAssertEqual(workout.samples.map(\.bpm), [120]) + XCTAssertEqual(workout.peakHr, 131) + XCTAssertEqual(workout.savedPeak, 131) + XCTAssertFalse(workout.recordSample(HRSample(ts: 1_000, bpm: 125))) // a lower repeat moves nothing + XCTAssertEqual(workout.peakHr, 131) + } + /// Twenty minutes climbing from 100 to 160 bpm, each second arriving twice (the R-R packet, then the rate /// moving) as it does during exercise. Recorded through `recordSample`, the workout's Effort is the Effort of /// the plain once-a-second stream; the repeats, kept, scored higher. 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 e3d55e59e2..ef3792174f 100644 --- a/android/app/src/main/java/com/noop/ui/AppViewModel.kt +++ b/android/app/src/main/java/com/noop/ui/AppViewModel.kt @@ -1732,7 +1732,8 @@ class AppViewModel(app: Application) : AndroidViewModel(app) { val pausedMs = w.pausedDurationMs + (w.pausedAtMs?.let { endMs - it } ?: 0L) val activeDurationMs = (endMs - w.startMs - pausedMs).coerceAtLeast(0L) val avg = if (samples.isNotEmpty()) samples.sumOf { it.bpm } / samples.size else null - val peak = if (samples.isNotEmpty()) samples.maxOf { it.bpm } else null + // `w.peakHr` can exceed every sample: a repeated second's higher reading is folded into it, not recorded. + val peak = if (samples.isNotEmpty()) maxOf(samples.maxOf { it.bpm }, w.peakHr) else null // #983: score the SAVED workout with the wearer's measured resting HR, not the hardcoded // default of 60. %HRR is (bpm - resting) / (max - resting), so the default moves every zone // boundary — at 136 bpm with maxHR 190 it is the difference between zone 1 and zone 2. Today's @@ -1804,15 +1805,21 @@ class AppViewModel(app: Application) : AndroidViewModel(app) { // One sample a second. This runs on every LiveState emission that carries a heart rate — any field // changing, not only the rate — so a second often arrives more than once with one `ts`, and Effort // credits a zero gap with a full second (`StrainScorer.sampleDurationsMinutes`): each repeat counted - // as another second of effort, live and in the saved workout. Twin of iOS `ActiveWorkout.recordSample`. + // as another second of effort, live and in the saved workout. A refused reading still reaches the peak: the + // rate moving is often why this ran, so it can be a within-second high; only the peak is published for it, + // with no rescore and no snapshot. Twin of iOS `ActiveWorkout.recordSample`. val ts = System.currentTimeMillis() / 1000 - if (w.samples.lastOrNull()?.ts == ts) return + if (w.samples.lastOrNull()?.ts == ts) { + if (bpm > w.peakHr) _activeWorkout.value = w.copy(peakHr = bpm) + return + } val s = w.samples + HrSample(deviceId = deviceId, ts = ts, bpm = bpm) val strain = StrainScorer.strain( s, maxHR = profileStore.hrMax.toDouble(), method = NoopPrefs.effortMethod(appContext), sex = profileStore.sex) ?: 0.0 val updated = w.copy( - samples = s, avgHr = s.sumOf { it.bpm } / s.size, peakHr = s.maxOf { it.bpm }, liveStrain = strain, + // Grown by comparison, not recomputed from `s`, so a peak folded in from a repeated second is kept. + samples = s, avgHr = s.sumOf { it.bpm } / s.size, peakHr = maxOf(w.peakHr, bpm), liveStrain = strain, ) _activeWorkout.value = updated // Re-snapshot the durable non-GPS session so a process kill keeps the latest accumulated HR (#529).