From f1801e1ee1842ec86c03bcec7b720fff8bae7115 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Fri, 31 Jul 2026 17:23:10 -0700 Subject: [PATCH] Effort: one resolved figure for every read-out on Today (#1001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Effort was resolved independently in several places and only the hero ring knew about the live in-progress value. The Key Metrics tile and the HR chart's edge badge read DailyMetric.strain straight off the daily row, which is rewritten only when the heavy daily pass runs. The reporter's morning showed 2.3 on the ring and 0.5 in both of the others, with the HR trace climbing from about 09:00 and the screenshot taken at 12:22 — the row predated the whole morning. Recovery and Rest matched everywhere precisely because neither has a live path. Not a 9.3.0 regression: the live hero landed in v4.0.1 on 15 June and the tile has read the stored column unchanged since 14 June. #963 made Effort track an active morning more closely, which widened the gap enough to see. StrainScorer.effectiveEffort(live:stored:) is now the one rule, Swift and Kotlin, with the twin test suites pinning it. The MAX in it is not a tie-break but the never-drop floor from #489/#506, where a sparse-HR live under-read replaced a real 38.3 with 0 — worth naming, because it is the part someone would 'simplify' into preferring live. Android: resolved once in TodayScreen and threaded to MetricGrid and HeartRateTrendCard, alongside the resolved values those already take (caloriesForDay, importedStepsForDay). The near-zero Effort caption had a fourth hand-rolled copy of the same expression; it now uses the shared one and keeps returning null for past days so the caption stays today-only. iOS already had effortStrain() as the single point, and its gauge and near-zero note used it; effortMarker and the Effort StatTile reached past it to the row. Both now resolve through it, and it delegates to the shared rule. --- .../StrandAnalytics/StrainScorer.swift | 21 +++++++ .../EffectiveEffortTests.swift | 50 ++++++++++++++++ Strand/Screens/TodayView.swift | 32 +++++------ .../java/com/noop/analytics/StrainScorer.kt | 23 ++++++++ .../src/main/java/com/noop/ui/TodayScreen.kt | 43 +++++++++----- .../com/noop/analytics/EffectiveEffortTest.kt | 57 +++++++++++++++++++ 6 files changed, 196 insertions(+), 30 deletions(-) create mode 100644 Packages/StrandAnalytics/Tests/StrandAnalyticsTests/EffectiveEffortTests.swift create mode 100644 android/app/src/test/java/com/noop/analytics/EffectiveEffortTest.kt diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/StrainScorer.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/StrainScorer.swift index e7fa4d70c1..e50eae1b0f 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/StrainScorer.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/StrainScorer.swift @@ -138,6 +138,27 @@ public enum StrainScorer { /// cadence we know of (the 5/MG's ~30 s, see `minSparseReadings`), so no genuine cadence is truncated. public static let maxSampleGapMin: Double = 2.0 + /// The one Effort figure every read-out on Today must show (#1001). + /// + /// Effort has two sources. `stored` is the daily row, rewritten only when the heavy daily pass runs. + /// `live` is today's in-progress recompute over the raw HR stream (local midnight → now), which + /// exists precisely because the stored row lags — early in the day it still holds yesterday's Effort + /// or a stale 0.0 (#402). Past days have no live value and use the row. + /// + /// Taking the MAX rather than preferring `live` is not a tie-break: Effort accrues over a day and + /// must never visibly DROP. The live recompute can UNDER-read when today's HR is sparse, or when a + /// logged workout's load is not in the raw stream — a 5/MG user who trained in the morning had a real + /// 38.3 replaced by a live 0 (#489/#506). Flooring at what is already earned is what stops that. + /// + /// Shared so the hero ring, the Key Metrics tile and the chart's edge badge cannot drift apart: they + /// each resolved Effort themselves, and only the ring knew about `live`, so an active morning showed + /// 2.3 on the ring and 0.5 in the other two until the daily pass caught up (#1001). + public static func effectiveEffort(live: Double?, stored: Double?) -> Double? { + guard let live else { return stored } + guard let stored else { return live } + return Swift.max(live, stored) + } + /// Infer per-sample duration (minutes) from the first two timestamps. Falls /// back to 1 s when fewer than two samples or coincident timestamps. /// diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/EffectiveEffortTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/EffectiveEffortTests.swift new file mode 100644 index 0000000000..01116e9626 --- /dev/null +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/EffectiveEffortTests.swift @@ -0,0 +1,50 @@ +import XCTest +@testable import StrandAnalytics + +/// #1001 — the single Effort figure every read-out on Today resolves through. +/// +/// The bug: Effort was resolved independently in three places. Only the hero ring knew about the live +/// in-progress recompute; the Key Metrics tile and the HR chart's edge badge read the stored daily row, +/// which is rewritten only when the heavy daily pass runs. On a morning with a real HR climb the ring +/// showed 2.3 while the other two still showed 0.5. +/// +/// These pin the resolution rule itself, and in particular the MAX — which is not a tie-break but the +/// never-drop floor from #489/#506, where a sparse-HR live under-read replaced a real 38.3 with 0. +final class EffectiveEffortTests: XCTestCase { + + /// The reported case: a live value ahead of a stale row wins, so every read-out moves together. + func testLiveAheadOfAStaleRowWins() { + XCTAssertEqual(StrainScorer.effectiveEffort(live: 2.3, stored: 0.5)!, 2.3, accuracy: 1e-9) + } + + /// The #489/#506 floor: a live UNDER-read must never pull a read-out below what today already earned. + func testAStoredValueFloorsALiveUnderRead() { + XCTAssertEqual(StrainScorer.effectiveEffort(live: 0.0, stored: 38.3)!, 38.3, accuracy: 1e-9) + } + + /// Past days carry no live value and use the row unchanged. + func testNoLiveValueUsesTheStoredRow() { + XCTAssertEqual(StrainScorer.effectiveEffort(live: nil, stored: 12.5)!, 12.5, accuracy: 1e-9) + } + + /// Before the day has enough HR to score there is no row yet, so the live value stands alone. + func testNoStoredRowUsesTheLiveValue() { + XCTAssertEqual(StrainScorer.effectiveEffort(live: 4.0, stored: nil)!, 4.0, accuracy: 1e-9) + } + + /// Neither source is "No Data" — the read-outs must not invent a zero. + func testNeitherSourceIsNil() { + XCTAssertNil(StrainScorer.effectiveEffort(live: nil, stored: nil)) + } + + /// A genuine zero is a value, not an absence: a still day scores 0 and must render as 0, not "—". + func testAGenuineZeroIsKept() { + XCTAssertEqual(StrainScorer.effectiveEffort(live: 0.0, stored: 0.0)!, 0.0, accuracy: 1e-9) + XCTAssertEqual(StrainScorer.effectiveEffort(live: nil, stored: 0.0)!, 0.0, accuracy: 1e-9) + } + + /// Equal sources are stable — resolving twice cannot make a read-out flicker. + func testEqualSourcesResolveToThatValue() { + XCTAssertEqual(StrainScorer.effectiveEffort(live: 7.25, stored: 7.25)!, 7.25, accuracy: 1e-9) + } +} diff --git a/Strand/Screens/TodayView.swift b/Strand/Screens/TodayView.swift index 4d89ffc360..6eb9ea49dc 100644 --- a/Strand/Screens/TodayView.swift +++ b/Strand/Screens/TodayView.swift @@ -2883,17 +2883,12 @@ struct TodayView: View { // `--demo-hour` frame is active. Charge/Rest are intentionally left at their seeded values. if let f = DemoDayHarness.active { return f.effort } #endif - if selectedDayOffset == 0, let live = liveTodayStrain { - // Effort accrues over a day and must never visibly DROP. The in-progress recompute (raw day - // HR, midnight→now) can UNDER-read when today's HR is sparse or a logged workout's load isn't - // in the raw stream, e.g. a 5/MG user who trained this morning saw today's real 38.3 get - // replaced by a live 0 (#489/#506). Floor at the day's already-earned Effort. `d` (displayDay) - // for today is ALWAYS today's row or nil, never a prior day, so this can't resurrect a stale - // day; it only stops the gauge dropping below what's already been counted today. - if let stored = d?.strain { return Swift.max(live, stored) } - return live - } - return d?.strain + // The never-drop floor and the live/stored preference both live in `StrainScorer.effectiveEffort` + // (#1001), shared with the Kotlin twin so the two platforms cannot resolve Effort differently. + // `d` (displayDay) for today is ALWAYS today's row or nil, never a prior day, so the floor cannot + // resurrect a stale day; it only stops a read-out dropping below what today has already earned. + return StrainScorer.effectiveEffort(live: selectedDayOffset == 0 ? liveTodayStrain : nil, + stored: d?.strain) } /// When TODAY's Effort scores a genuine near-zero, there's enough HR to score, but it never @@ -3149,7 +3144,9 @@ struct TodayView: View { /// preference (#268) and reads identically, the stored strain is on the 0–100 axis, so a morning /// "21.2" is 21.2-of-100, not WHOOP's near-max 21-of-21. private var effortMarker: OverviewHRChart.EdgeMarker? { - guard let strain = displayDay?.strain, let date = hrPoints.last?.date else { return nil } + // #1001: the resolved Effort, not the daily row. Reading `displayDay.strain` here put the badge a + // whole active morning behind the hero ring, which resolves through the same `effortStrain`. + guard let strain = effortStrain(displayDay), let date = hrPoints.last?.date else { return nil } return .init(date: date, label: String(localized: "\(UnitFormatter.effortDisplay(strain, scale: effortScale)) Effort"), color: StrandPalette.effortTint(fraction: strain / StrainScorer.maxStrain), alignment: .trailing) @@ -3303,12 +3300,15 @@ struct TodayView: View { case .effort: // Unscored TODAY → a short "building" hint instead of the "of N" axis caption, so a // fresh user reads "coming" not "broken" (#527); a scored day keeps "of N". + // #1001: resolve through `effortStrain` so this tile shows the hero ring's figure. Reading + // `d.strain` straight off the daily row left it behind by the whole morning on an active day. + let effort = effortStrain(d) StatTile( label: "Effort", - value: d?.strain.map { UnitFormatter.effortDisplay($0, scale: effortScale) } ?? "—", - caption: d?.strain != nil ? String(localized: "of \(UnitFormatter.effortScaleMax(effortScale))") - : (buildingHint(.effort) ?? String(localized: "of \(UnitFormatter.effortScaleMax(effortScale))")), - accent: d?.strain.map { StrandPalette.effortTint(fraction: $0 / StrainScorer.maxStrain) } ?? StrandPalette.textPrimary, + value: effort.map { UnitFormatter.effortDisplay($0, scale: effortScale) } ?? "—", + caption: effort != nil ? String(localized: "of \(UnitFormatter.effortScaleMax(effortScale))") + : (buildingHint(.effort) ?? String(localized: "of \(UnitFormatter.effortScaleMax(effortScale))")), + accent: effort.map { StrandPalette.effortTint(fraction: $0 / StrainScorer.maxStrain) } ?? StrandPalette.textPrimary, sparkline: sparks["strain"], sparkColor: StrandPalette.strain066, // Inline ⓘ in the tile header (not a corner overlay) so it never sits over the value (#495). diff --git a/android/app/src/main/java/com/noop/analytics/StrainScorer.kt b/android/app/src/main/java/com/noop/analytics/StrainScorer.kt index cd3750aa6f..f0828e244c 100644 --- a/android/app/src/main/java/com/noop/analytics/StrainScorer.kt +++ b/android/app/src/main/java/com/noop/analytics/StrainScorer.kt @@ -171,6 +171,29 @@ object StrainScorer { */ const val maxSampleGapMin: Double = 2.0 + /** + * The one Effort figure every read-out on Today must show (#1001). + * + * Effort has two sources. [stored] is the daily row, rewritten only when the heavy daily pass runs. + * [live] is today's in-progress recompute over the raw HR stream (local midnight → now), which exists + * precisely because the stored row lags — early in the day it still holds yesterday's Effort or a + * stale 0.0 (#402). Past days have no live value and use the row. + * + * Taking the MAX rather than preferring [live] is not a tie-break: Effort accrues over a day and must + * never visibly DROP. The live recompute can UNDER-read when today's HR is sparse, or when a logged + * workout's load is not in the raw stream — a 5/MG user who trained in the morning had a real 38.3 + * replaced by a live 0 (#489/#506). Flooring at what is already earned is what stops that. + * + * Shared so the hero ring, the Key Metrics tile and the chart's edge badge cannot drift apart: they + * each resolved Effort themselves, and only the ring knew about [live], so an active morning showed + * 2.3 on the ring and 0.5 in the other two until the daily pass caught up (#1001). + */ + fun effectiveEffort(live: Double?, stored: Double?): Double? { + if (live == null) return stored + if (stored == null) return live + return kotlin.math.max(live, stored) + } + /** * Infer per-sample duration (minutes) from the first two timestamps. Falls * back to 1 s when fewer than two samples or coincident timestamps. 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 8b54e7ec42..554d2f29ef 100644 --- a/android/app/src/main/java/com/noop/ui/TodayScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TodayScreen.kt @@ -887,6 +887,15 @@ fun TodayScreen( } } + // #1001: Effort resolved ONCE for the whole screen, so the Key Metrics tile and the HR chart's edge + // badge show what the hero ring shows. Both used to read `displayMetric.strain` straight off the daily + // row, which only refreshes when the heavy daily pass runs — so an active morning read 2.3 on the ring + // and 0.5 in the other two. The ring resolves the same way from the same rule (see ScoreHeroRow). + val effortForDay = StrainScorer.effectiveEffort( + live = if (selectedDayOffset == 0) liveTodayStrain else null, + stored = displayMetric?.strain, + ) + // Recovery cold-start: recovery is null until the HRV baseline crosses the seed gate // (Baselines.minNightsSeed valid nights). Show honest "calibrating, N of 4 nights" progress // instead of a bare "No Data" so a new BLE-only user knows scores are coming, not broken. (PR #85) @@ -1384,11 +1393,9 @@ fun TodayScreen( // near-zero (HR present but never crossed the cardio zone). Effort accrues over // a day and must never visibly drop: floor the in-progress value at the day's // already-earned strain (#489/#506). - val todayEffort = if (selectedDayOffset == 0) { - val liveStrain = liveTodayStrain - val stored = displayMetric?.strain - if (liveStrain != null && stored != null) maxOf(liveStrain, stored) else (liveStrain ?: stored) - } else null + // #1001: the shared resolution, not a fourth hand-rolled copy of it. Stays null + // for a navigated past day so the caption is a TODAY-only explanation. + val todayEffort = if (selectedDayOffset == 0) effortForDay else null if (todayEffort != null && todayEffort < 1.0) { Row( modifier = Modifier.padding(horizontal = 2.dp), @@ -1467,6 +1474,7 @@ fun TodayScreen( spo2CarryDay = lastSpo2Day, unitSystem = unitSystem, effortScale = effortScale, + effortForDay = effortForDay, // #1001: same figure as the hero ring latestWeightKg = weightKg, profileWeightKg = profileWeightKg, importedStepsForDay = importedStepsForDay, @@ -1499,7 +1507,7 @@ fun TodayScreen( modifier = Modifier.fillMaxWidth().staggeredAppear(stagger), verticalArrangement = Arrangement.spacedBy(8.dp), ) { - HeartRateTrendCard(viewModel, days, selectedDay, todayDate, displayMetric, effortScale) + HeartRateTrendCard(viewModel, days, selectedDay, todayDate, displayMetric, effortScale, effortForDay) } // The three hero vitals, HRV / Resting HR / Respiratory. Carried day (#543). TodaySection.RECOVERY_VITALS -> Box(modifier = Modifier.fillMaxWidth().staggeredAppear(stagger)) { @@ -2337,10 +2345,8 @@ private fun ScoreHeroRow( // (#489/#506: a live under-read replaced today's real Effort with 0). The effective value drives the // gauge number AND the has-data / "No Data" branch, so the ring only reads "No Data" when neither // exists. Mirrors the iOS live-Effort gauge. (#402) - val strain = run { - val live = liveTodayStrain; val stored = day?.strain - if (live != null && stored != null) maxOf(live, stored) else (live ?: stored) - } + // #1001: the shared rule, so this ring and the Key Metrics tile / chart badge cannot disagree. + val strain = StrainScorer.effectiveEffort(live = liveTodayStrain, stored = day?.strain) // Effort honours the 0–100 / WHOOP-0–21 toggle (#313). The stored strain is on NOOP's 0–100 Effort // axis; render it on the user's selected scale so the arc and centre number match the app's Effort. val effortOutOf = if (effortScale == EffortScale.WHOOP) 21.0 else 100.0 @@ -4224,6 +4230,10 @@ private fun MetricGrid( spo2CarryDay: DailyMetric? = null, unitSystem: UnitSystem = UnitSystem.METRIC, effortScale: EffortScale = EffortScale.HUNDRED, + // #1001: the day's resolved Effort (live-preferring for today, floored at the stored row). Threaded + // like caloriesForDay/importedStepsForDay because, like those, it is not simply the DailyMetric column + // — reading `d.strain` here is what left this tile behind the hero ring on an active morning. + effortForDay: Double? = null, latestWeightKg: Double? = null, profileWeightKg: Double = 75.0, importedStepsForDay: Int? = null, @@ -4282,13 +4292,15 @@ private fun MetricGrid( }, KeyMetric.EFFORT to KeyTileData( label = uiString(R.string.l10n_today_screen_strain_79fe380e), - value = d?.strain?.let { UnitFormatter.effortDisplay(it, effortScale) } ?: NO_DATA, + // #1001: the resolved Effort, falling back to the stored column only when the caller passes + // none (previews/tests). Reading `d.strain` here is what left this tile behind the hero ring. + value = (effortForDay ?: d?.strain)?.let { UnitFormatter.effortDisplay(it, effortScale) } ?: NO_DATA, // #492: Strain/Effort is a load index (0–21 WHOOP / 0–100 NOOP), NOT a percentage — the "%" // was wrong (esp. on the 0–21 scale). Recovery/Rest ARE 0–100 % and keep it. iOS shows the // strain axis as an "of 21"/"of 100" caption with no % (TodayView effort tile); match that. unit = "", - tint = d?.strain?.let { Palette.effortTint(it / StrainScorer.maxStrain) } ?: Palette.effortColor, - frac = d?.strain?.let { (it / 100.0).coerceIn(0.0, 1.0) }, + tint = (effortForDay ?: d?.strain)?.let { Palette.effortTint(it / StrainScorer.maxStrain) } ?: Palette.effortColor, + frac = (effortForDay ?: d?.strain)?.let { (it / 100.0).coerceIn(0.0, 1.0) }, spark = w.strain, ), KeyMetric.REST to KeyTileData( @@ -4606,6 +4618,9 @@ private fun HeartRateTrendCard( today: LocalDate, displayMetric: DailyMetric? = null, effortScale: EffortScale = EffortScale.HUNDRED, + // #1001: the day's resolved Effort for the chart's edge badge. It read `displayMetric.strain` — the + // daily row — so on an active morning the badge trailed the hero ring by the whole morning's load. + effortForDay: Double? = null, ) { // "Today" here is the LOGICAL day (rolls at 04:00 local), so in the small hours after midnight the // trend keeps the evening's curve, window start at the logical day's own midnight, "since midnight" @@ -4818,7 +4833,7 @@ private fun HeartRateTrendCard( sleep = sleepToday, workouts = workoutsToday, recovery = displayMetric?.recovery, - strain = displayMetric?.strain, + strain = effortForDay ?: displayMetric?.strain, // #1001 effortScale = effortScale, timeTicks = timeTicks, modifier = Modifier diff --git a/android/app/src/test/java/com/noop/analytics/EffectiveEffortTest.kt b/android/app/src/test/java/com/noop/analytics/EffectiveEffortTest.kt new file mode 100644 index 0000000000..b0c4b145a3 --- /dev/null +++ b/android/app/src/test/java/com/noop/analytics/EffectiveEffortTest.kt @@ -0,0 +1,57 @@ +package com.noop.analytics + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * #1001 — the single Effort figure every read-out on Today resolves through. Twin of the Swift + * `EffectiveEffortTests`; the same cases in the same order, because the two platforms must resolve + * Effort identically. + * + * The bug: Effort was resolved independently in three places. Only the hero ring knew about the live + * in-progress recompute; the Key Metrics tile and the HR chart's edge badge read the stored daily row, + * which is rewritten only when the heavy daily pass runs. On a morning with a real HR climb the ring + * showed 2.3 while the other two still showed 0.5. + * + * These pin the resolution rule itself, and in particular the MAX — which is not a tie-break but the + * never-drop floor from #489/#506, where a sparse-HR live under-read replaced a real 38.3 with 0. + */ +class EffectiveEffortTest { + + /** The reported case: a live value ahead of a stale row wins, so every read-out moves together. */ + @Test fun liveAheadOfAStaleRowWins() { + assertEquals(2.3, StrainScorer.effectiveEffort(live = 2.3, stored = 0.5)!!, 1e-9) + } + + /** The #489/#506 floor: a live UNDER-read must never pull a read-out below what today already earned. */ + @Test fun aStoredValueFloorsALiveUnderRead() { + assertEquals(38.3, StrainScorer.effectiveEffort(live = 0.0, stored = 38.3)!!, 1e-9) + } + + /** Past days carry no live value and use the row unchanged. */ + @Test fun noLiveValueUsesTheStoredRow() { + assertEquals(12.5, StrainScorer.effectiveEffort(live = null, stored = 12.5)!!, 1e-9) + } + + /** Before the day has enough HR to score there is no row yet, so the live value stands alone. */ + @Test fun noStoredRowUsesTheLiveValue() { + assertEquals(4.0, StrainScorer.effectiveEffort(live = 4.0, stored = null)!!, 1e-9) + } + + /** Neither source is "No Data" — the read-outs must not invent a zero. */ + @Test fun neitherSourceIsNull() { + assertNull(StrainScorer.effectiveEffort(live = null, stored = null)) + } + + /** A genuine zero is a value, not an absence: a still day scores 0 and must render as 0, not "—". */ + @Test fun aGenuineZeroIsKept() { + assertEquals(0.0, StrainScorer.effectiveEffort(live = 0.0, stored = 0.0)!!, 1e-9) + assertEquals(0.0, StrainScorer.effectiveEffort(live = null, stored = 0.0)!!, 1e-9) + } + + /** Equal sources are stable — resolving twice cannot make a read-out flicker. */ + @Test fun equalSourcesResolveToThatValue() { + assertEquals(7.25, StrainScorer.effectiveEffort(live = 7.25, stored = 7.25)!!, 1e-9) + } +}