From 63e19100ced90540f1246806c33cb3312f9c808c Mon Sep 17 00:00:00 2001 From: Utku Deniz Altiok <93100191+UtkuDenizAltiok@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:24:22 +0300 Subject: [PATCH] fix(stress): discard results from cancelled screen loads --- Strand/Data/StressLoadCancellation.swift | 14 ++ Strand/Screens/StressView.swift | 55 +++++--- StrandTests/StressLoadCancellationTests.swift | 116 +++++++++++++++++ .../StressPersonalBaselineSurfaceTests.swift | 9 +- .../com/noop/ui/StressLoadCancellation.kt | 24 ++++ .../src/main/java/com/noop/ui/StressScreen.kt | 12 +- .../com/noop/ui/StressLoadCancellationTest.kt | 121 ++++++++++++++++++ .../ui/StressPersonalBaselineSurfaceTest.kt | 12 +- 8 files changed, 336 insertions(+), 27 deletions(-) create mode 100644 Strand/Data/StressLoadCancellation.swift create mode 100644 StrandTests/StressLoadCancellationTests.swift create mode 100644 android/app/src/main/java/com/noop/ui/StressLoadCancellation.kt create mode 100644 android/app/src/test/java/com/noop/ui/StressLoadCancellationTest.kt diff --git a/Strand/Data/StressLoadCancellation.swift b/Strand/Data/StressLoadCancellation.swift new file mode 100644 index 0000000000..2eb173d946 --- /dev/null +++ b/Strand/Data/StressLoadCancellation.swift @@ -0,0 +1,14 @@ +import Foundation + +/// A cancelled screen load must not publish a result from a read or detached calculation that kept +/// running. Check both sides of the suspension so obsolete loads also stop before their next read. +/// Kotlin twin: `com.noop.ui.StressLoadCancellation`. +@MainActor +enum StressLoadCancellation { + static func read(_ work: () async -> Value) async -> Value? { + guard !Task.isCancelled else { return nil } + let value = await work() + guard !Task.isCancelled else { return nil } + return value + } +} diff --git a/Strand/Screens/StressView.swift b/Strand/Screens/StressView.swift index 553056c699..7ead63dbae 100644 --- a/Strand/Screens/StressView.swift +++ b/Strand/Screens/StressView.swift @@ -96,7 +96,10 @@ struct StressView: View { } private func load() async { - storedSeries = await repo.series(key: "stress", source: "my-whoop") + guard let series = await StressLoadCancellation.read({ + await repo.series(key: "stress", source: "my-whoop") + }) else { return } + storedSeries = series loaded = true rebuildModelIfNeeded() await loadDaytime() @@ -112,7 +115,9 @@ struct StressView: View { let to = Int(Date().timeIntervalSince1970) let tz = TimeZone.current.secondsFromGMT(for: Date()) - let hr = await repo.hrSamples(from: from, to: to, limit: 200_000) + guard let hr = await StressLoadCancellation.read({ + await repo.hrSamples(from: from, to: to, limit: 200_000) + }) else { return } // Too few HR samples: empty the timeline AND clear the advanced readouts in lockstep. Without this // reset a later refresh that hits this path would leave the Advanced HRV card showing stale values // next to an empty timeline (the readouts are only recomputed past this guard). @@ -122,11 +127,15 @@ struct StressView: View { freqHRV = nil return } - let rr = await repo.rrIntervals(from: from, to: to, limit: 200_000) + guard let rr = await StressLoadCancellation.read({ + await repo.rrIntervals(from: from, to: to, limit: 200_000) + }) else { return } // Wrist accelerometer for the motion gate: an ambulatory hour is EXERTION, not stress, so it // is masked rather than scored (DaytimeStress). Same store read as R-R; empty on hardware or // imports with no gravity, which is exactly the "no masking, prior behaviour" degradation. - let gravity = await repo.gravitySamplesUnion(from: from, to: to, limit: 200_000) + guard let gravity = await StressLoadCancellation.read({ + await repo.gravitySamplesUnion(from: from, to: to, limit: 200_000) + }) else { return } // Score today's hours against the PERSONAL cross-day daytime baseline ONLY when the user has // opted in (Settings → Experimental) AND enough worn history exists (Oura-style @@ -135,14 +144,14 @@ struct StressView: View { // chooseable lens, not a silent default. The mode is resolved only AFTER the HR-count guard above, // so the trailing-history reads are never paid on a day with no scorable timeline — and are never // paid at all while the toggle is OFF (the default), keeping the read byte-identical to before. - let mode = await DaytimeStressMode.selected( - repo: repo, - startOfToday: startOfDay, - calendar: cal, - personalBaseline: PuffinExperiment.stressPersonalBaselineEnabled - ) - if case .baselineRelative = mode { daytimeUsesPersonalBaseline = true } - else { daytimeUsesPersonalBaseline = false } + guard let mode = await StressLoadCancellation.read({ + await DaytimeStressMode.selected( + repo: repo, + startOfToday: startOfDay, + calendar: cal, + personalBaseline: PuffinExperiment.stressPersonalBaselineEnabled + ) + }) else { return } // includeTimeline: the SLIDING read, so the screen's line moves in half-hours instead of // stepping through whole clock hours (#2144). The scored unit is still a full hour; this only // decides how often that hour is re-read, so a thin ten minutes costs the windows that overlap @@ -159,10 +168,16 @@ struct StressView: View { // detached task is decorative and the work races the UI for cores anyway. StressDayCurve // learned that on this same issue; the continuation in UnescalatedWork is what keeps the // priority honest. - daytime = await runUnescalated(priority: .userInitiated) { - DaytimeStress.analyze(hr: hr, rr: rr, gravity: gravity, tzOffsetSeconds: tz, mode: mode, - includeTimeline: true) - } + guard let timeline = await StressLoadCancellation.read({ + await runUnescalated(priority: .userInitiated) { + DaytimeStress.analyze(hr: hr, rr: rr, gravity: gravity, tzOffsetSeconds: tz, mode: mode, + includeTimeline: true) + } + }) else { return } + daytime = timeline + // Publish the mode with the timeline it describes, after the same cancellation check. + if case .baselineRelative = mode { daytimeUsesPersonalBaseline = true } + else { daytimeUsesPersonalBaseline = false } // ADDITIVE advanced readouts, computed on-demand from the SAME `rr` (no extra fetch, no // DB / schema change, and no effect on the 0..3 score above). Each engine returns nil when @@ -177,9 +192,11 @@ struct StressView: View { // main actor and publish when done; their card is hidden until then, exactly as it is when a gate // is unmet. Same `runUnescalated` reasoning as above, and the default `.utility` is real here // because nothing escalates it: this is the phase that must yield to the UI. - let advanced = await runUnescalated { - (index: StressIndex.components(rr: rr), freq: HRVFreqDomain.freqDomain(rr: rr)) - } + guard let advanced = await StressLoadCancellation.read({ + await runUnescalated { + (index: StressIndex.components(rr: rr), freq: HRVFreqDomain.freqDomain(rr: rr)) + } + }) else { return } stressIndex = advanced.index freqHRV = advanced.freq } diff --git a/StrandTests/StressLoadCancellationTests.swift b/StrandTests/StressLoadCancellationTests.swift new file mode 100644 index 0000000000..6a80245cd1 --- /dev/null +++ b/StrandTests/StressLoadCancellationTests.swift @@ -0,0 +1,116 @@ +import XCTest +@testable import Strand + +@MainActor +final class StressLoadCancellationTests: XCTestCase { + func testSuccessfulReadAndTwoPhaseOrderStayIntact() async { + var phases: [Int] = [] + if let core = await StressLoadCancellation.read({ 21 }) { phases.append(core) } + if let lenses = await StressLoadCancellation.read({ 34 }) { phases.append(lenses) } + XCTAssertEqual(phases, [21, 34]) + } + + func testSuccessfulAbsentValueIsNotCancellation() async { + let result: Int?? = await StressLoadCancellation.read({ nil as Int? }) + XCTAssertEqual(result, .some(.none)) + } + + func testAlreadyCancelledLoadDoesNotStartARead() async { + var reads = 0 + let task = Task { @MainActor in + await StressLoadCancellation.read { + reads += 1 + return 7 + } + } + task.cancel() + let result = await task.value + XCTAssertNil(result) + XCTAssertEqual(reads, 0) + } + + func testLateCancelledReadCannotReplaceFreshState() async { + let gate = StressFixtureSuspension() + var published: [String] = [] + let old = Task { @MainActor in + if let value = await StressLoadCancellation.read({ + await gate.holdStressFixture() + return "old" + }) { published.append(value) } + } + await gate.waitForStressFixture() + old.cancel() + if let fresh = await StressLoadCancellation.read({ "fresh" }) { published.append(fresh) } + await gate.releaseStressFixture() + await old.value + XCTAssertEqual(published, ["fresh"]) + } + + func testCancelledCoreDoesNotStartAdvancedWork() async { + let gate = StressFixtureSuspension() + var advancedReads = 0 + var published: [Int] = [] + let task = Task { @MainActor in + guard let core = await StressLoadCancellation.read({ + await gate.holdStressFixture() + return 1 + }) else { return } + published.append(core) + if let lenses = await StressLoadCancellation.read({ + advancedReads += 1 + return 2 + }) { published.append(lenses) } + } + await gate.waitForStressFixture() + task.cancel() + await gate.releaseStressFixture() + await task.value + XCTAssertEqual(published, []) + XCTAssertEqual(advancedReads, 0) + } + + func testLateAdvancedReadCannotRestoreOldLenses() async { + let gate = StressFixtureSuspension() + var published: [String] = [] + let old = Task { @MainActor in + guard let core = await StressLoadCancellation.read({ "old-core" }) else { return } + published.append(core) + if let lenses = await StressLoadCancellation.read({ + await gate.holdStressFixture() + return "old-lenses" + }) { published.append(lenses) } + } + await gate.waitForStressFixture() + old.cancel() + if let fresh = await StressLoadCancellation.read({ "fresh-core" }) { published.append(fresh) } + await gate.releaseStressFixture() + await old.value + XCTAssertEqual(published, ["old-core", "fresh-core"]) + } +} + +/// Deliberately ignores task cancellation, like a detached calculation or a completed database read. +private actor StressFixtureSuspension { + private var held: CheckedContinuation? + private var waiting: CheckedContinuation? + private var started = false + + func holdStressFixture() async { + await withCheckedContinuation { continuation in + held = continuation + started = true + waiting?.resume() + waiting = nil + } + } + + func waitForStressFixture() async { + if started { return } + await withCheckedContinuation { waiting = $0 } + } + + func releaseStressFixture() { + held?.resume() + held = nil + } +} diff --git a/StrandTests/StressPersonalBaselineSurfaceTests.swift b/StrandTests/StressPersonalBaselineSurfaceTests.swift index f75334cd18..2e5849335c 100644 --- a/StrandTests/StressPersonalBaselineSurfaceTests.swift +++ b/StrandTests/StressPersonalBaselineSurfaceTests.swift @@ -18,7 +18,14 @@ final class StressPersonalBaselineSurfaceTests: XCTestCase { let today = try source("Strand/Screens/TodayView.swift") let liquidToday = try source("Strand/Liquid/LiquidTodayView.swift") - XCTAssertTrue(detail.contains("let mode = await DaytimeStressMode.selected(")) + XCTAssertNotNil(detail.range( + of: #"DaytimeStressMode\.selected\([\s\S]*?personalBaseline:\s*PuffinExperiment\.stressPersonalBaselineEnabled"#, + options: .regularExpression + )) + XCTAssertNotNil(detail.range( + of: #"DaytimeStress\.analyze\([\s\S]*?mode:\s*mode,"#, + options: .regularExpression + )) XCTAssertTrue(producer.contains("let mode = await DaytimeStressMode.selected(")) XCTAssertTrue(producer.contains("tzOffsetSeconds: tz, mode: mode,")) for body in [today, liquidToday] { diff --git a/android/app/src/main/java/com/noop/ui/StressLoadCancellation.kt b/android/app/src/main/java/com/noop/ui/StressLoadCancellation.kt new file mode 100644 index 0000000000..0d6d48f10f --- /dev/null +++ b/android/app/src/main/java/com/noop/ui/StressLoadCancellation.kt @@ -0,0 +1,24 @@ +package com.noop.ui + +import kotlin.coroutines.coroutineContext +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.ensureActive + +/** + * Keep failed-read fallback behaviour, but never turn cancellation into a screen update. Check both + * sides of the suspension because CPU work and some reads may finish after their caller is cancelled. + * Swift twin: StressLoadCancellation. + */ +internal object StressLoadCancellation { + suspend fun read(work: suspend () -> Value): Value? { + coroutineContext.ensureActive() + val value = try { + work() + } catch (error: Throwable) { + if (error is CancellationException) throw error + null + } + coroutineContext.ensureActive() + return value + } +} diff --git a/android/app/src/main/java/com/noop/ui/StressScreen.kt b/android/app/src/main/java/com/noop/ui/StressScreen.kt index f3da60c686..2f1b2cd8ac 100644 --- a/android/app/src/main/java/com/noop/ui/StressScreen.kt +++ b/android/app/src/main/java/com/noop/ui/StressScreen.kt @@ -115,9 +115,9 @@ fun StressScreen(vm: AppViewModel, onBreathe: () -> Unit = {}) { var stored by remember { mutableStateOf>(emptyMap()) } var storedLoaded by remember { mutableStateOf(false) } androidx.compose.runtime.LaunchedEffect(Unit) { - val rows = runCatching { + val rows = StressLoadCancellation.read { vm.repo.metricSeries("my-whoop", "stress", "0000-01-01", "9999-12-31") - }.getOrDefault(emptyList()) + }.orEmpty() stored = rows.associate { it.day to it.value.coerceIn(0.0, 3.0) } storedLoaded = true } @@ -151,9 +151,9 @@ fun StressScreen(vm: AppViewModel, onBreathe: () -> Unit = {}) { while (true) { val nowSeconds = System.currentTimeMillis() / 1000L val window = stressLocalDayWindowContaining(nowSeconds, ZoneId.systemDefault()) - val fingerprint = runCatching { + val fingerprint = StressLoadCancellation.read { vm.repo.hrFingerprintWindow(vm.activeStrapId, window.fromEpochSecond, nowSeconds) - }.getOrNull() + } // A failed fingerprint reads as "cannot tell", which loads rather than skips: being // wrong about this costs one pass, being wrong the other way freezes the screen. if (fingerprint == null || fingerprint != lastHrFingerprint) { @@ -169,11 +169,11 @@ fun StressScreen(vm: AppViewModel, onBreathe: () -> Unit = {}) { daytime = core?.daytime ?: DaytimeStress.Result.EMPTY daytimeUsesPersonalBaseline = core?.usesPersonalBaseline == true val beats = core?.rr.orEmpty() - val lenses = if (beats.isEmpty()) null else runCatching { + val lenses = if (beats.isEmpty()) null else StressLoadCancellation.read { withContext(Dispatchers.Default) { StressIndex.components(beats) to HrvFreqDomain.freqDomain(beats) } - }.getOrNull() + } stressIndex = lenses?.first freqHrv = lenses?.second } diff --git a/android/app/src/test/java/com/noop/ui/StressLoadCancellationTest.kt b/android/app/src/test/java/com/noop/ui/StressLoadCancellationTest.kt new file mode 100644 index 0000000000..3ea570e12a --- /dev/null +++ b/android/app/src/test/java/com/noop/ui/StressLoadCancellationTest.kt @@ -0,0 +1,121 @@ +package com.noop.ui + +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class StressLoadCancellationTest { + @Test fun successfulReadAndTwoPhaseOrderStayIntact() = runTest { + val phases = mutableListOf() + StressLoadCancellation.read { 21 }?.let { phases += it } + StressLoadCancellation.read { 34 }?.let { phases += it } + assertEquals(listOf(21, 34), phases) + } + + @Test fun successfulAbsentValueIsNotCancellation() = runTest { + var reads = 0 + assertNull(StressLoadCancellation.read { reads += 1; null }) + assertEquals(1, reads) + } + + @Test fun alreadyCancelledLoadDoesNotStartARead() = runTest { + val entered = CompletableDeferred() + val released = CompletableDeferred() + var reads = 0 + val old = launch { + withContext(NonCancellable) { entered.complete(Unit); released.await() } + StressLoadCancellation.read { reads += 1; 7 } + } + entered.await() + old.cancel() + released.complete(Unit) + old.join() + assertEquals(0, reads) + } + + @Test fun lateCancelledReadCannotReplaceFreshState() = runTest { + val entered = CompletableDeferred() + val released = CompletableDeferred() + val published = mutableListOf() + val old = launch { + StressLoadCancellation.read { + withContext(NonCancellable) { entered.complete(Unit); released.await() } + "old" + }?.let { published += it } + } + entered.await() + old.cancel() + StressLoadCancellation.read { "fresh" }?.let { published += it } + released.complete(Unit) + old.join() + assertEquals(listOf("fresh"), published) + } + + @Test fun cancelledReadDoesNotStartTheNextPhase() = runTest { + val entered = CompletableDeferred() + val released = CompletableDeferred() + var advancedReads = 0 + val published = mutableListOf() + val old = launch { + val core = StressLoadCancellation.read { + withContext(NonCancellable) { entered.complete(Unit); released.await() } + 1 + } ?: return@launch + published += core + StressLoadCancellation.read { advancedReads += 1; 2 }?.let { published += it } + } + entered.await() + old.cancel() + released.complete(Unit) + old.join() + assertEquals(emptyList(), published) + assertEquals(0, advancedReads) + } + + @Test fun lateAdvancedReadCannotRestoreOldLenses() = runTest { + val entered = CompletableDeferred() + val released = CompletableDeferred() + val published = mutableListOf() + val old = launch { + val core = StressLoadCancellation.read { "old-core" } ?: return@launch + published += core + StressLoadCancellation.read { + withContext(NonCancellable) { entered.complete(Unit); released.await() } + "old-lenses" + }?.let { published += it } + } + entered.await() + old.cancel() + StressLoadCancellation.read { "fresh-core" }?.let { published += it } + released.complete(Unit) + old.join() + assertEquals(listOf("old-core", "fresh-core"), published) + } + + @Test fun ordinaryReadFailureStillUsesAbsentFallback() = runTest { + var reads = 0 + val value = StressLoadCancellation.read { reads += 1; throw IllegalStateException("fixture") } + assertNull(value) + assertEquals(1, reads) + } + + @Test fun cancellationIsNeverAFailedReadFallback() = runTest { + var fallbackPublished = false + var cancellationRethrown = false + try { + val value = StressLoadCancellation.read { throw CancellationException("fixture") } + fallbackPublished = value == null + } catch (_: CancellationException) { + cancellationRethrown = true + } + assertTrue(cancellationRethrown) + assertEquals(false, fallbackPublished) + } +} diff --git a/android/app/src/test/java/com/noop/ui/StressPersonalBaselineSurfaceTest.kt b/android/app/src/test/java/com/noop/ui/StressPersonalBaselineSurfaceTest.kt index 7c00254ddb..162216a895 100644 --- a/android/app/src/test/java/com/noop/ui/StressPersonalBaselineSurfaceTest.kt +++ b/android/app/src/test/java/com/noop/ui/StressPersonalBaselineSurfaceTest.kt @@ -89,7 +89,17 @@ class StressPersonalBaselineSurfaceTest { val liquidToday = source("Strand/Liquid/LiquidTodayView.swift") val widget = source("StrandiOS/Widgets/WidgetPublish.swift") - assertTrue(detail.contains("let mode = await DaytimeStressMode.selected(")) + assertTrue( + "Stress detail must pass the selected personal-baseline preference", + Regex("DaytimeStressMode\\.selected\\([\\s\\S]*?personalBaseline:\\s*" + + "PuffinExperiment\\.stressPersonalBaselineEnabled") + .containsMatchIn(detail), + ) + assertTrue( + "Stress detail must analyze with the selected mode", + Regex("DaytimeStress\\.analyze\\([\\s\\S]*?mode:\\s*mode,") + .containsMatchIn(detail), + ) assertTrue(producer.contains("let mode = await DaytimeStressMode.selected(")) assertTrue( "the shared Apple producer must analyze with the selected mode",