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) + } +}