From 81b67b932cfede93d9cfc35b061092ca20d3d2ae Mon Sep 17 00:00:00 2001 From: Alex Wagner Date: Wed, 26 Aug 2026 17:56:32 +0200 Subject: [PATCH 1/4] fix(zones): anchor zone edges on max(observed ceiling, age estimate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 25-year-old's 16-minute run (avg 148, max 166) reported Z4 2m / Z5 13m — thirteen minutes of a steady run as "Max effort" — under a footnote claiming the edges came from his age. Both halves of that card were wrong, and they were wrong for different reasons. `trainingZones` preferred `observedCeilingBpm` whenever one existed, in either direction. But that number is one-sided evidence: holding 195 proves the ceiling is at least 195, which the age line does not know and which should win. Holding 166 proves nothing about the top — "never went maximal" and "maxes at 166" produce the identical number, and on a wrist PPG the first is far commoner. Preferring the lower of the two because it is "measured" inverts the direction in which the measurement bounds anything, so Z5 landed at 90 % of the user's own hard-run HR and every hard session graded most of itself as maximal. On the session-detail path the ceiling is even set by the session being graded: `_zoneAnchors` takes the all-time max including today. (The day pipeline and the live tick already avoid that — `observedHrCeilingBpm` is strictly-before-today, and the live `zoneSet` is pinned at session start.) The ceiling is now `max(observed, estimate)`. Deliberately no tolerance band below the estimate: a switch at `estimate − k` puts a cliff in the middle of the range the ceiling creeps through — at 30, an observed 177 would anchor at 177 and 176.9 at 187, moving every edge 10 bpm on a 0.1 bpm change, with nothing on screen to explain it. `max` is continuous; either side of the line only the label moves. `observed`/`karvonen` now mean "this user beat the population line", which is narrower and truer. The cost is chosen, not overlooked: someone whose HRmax is genuinely below their age line gets a Z5 they cannot reach. That is an under-report where the old behaviour was an affirmative "max effort" the data did not support, and this app abstains before it asserts. The set says `tanaka`, and the zones screen now names the held ceiling and why it is not being used. Second half of the card: the session summary hardcoded `kZonesWhy` while the day screen switched on the persisted `source`, so the same bands were described two ways and the reported card misattributed its own edges. Both now go through one `zonesWhy(source, maxHr)`, and `ActivityResult` carries the stamp and the ceiling — filled from the persisted `zone_bands` on the detail path and from the pinned live `zoneSet` on the stop-summary, so all call sites agree. The minutes now come from the same `getWorkout` read as the provenance beside them; they were still the month-list row, which can predate the rescore that opening a session performs. Also here: - `kZonesWhy` said "estimated from your age and your strap". `estimatedMaxHr` takes `deviceFamily` and deliberately ignores it — Tanaka is a regression on age alone — so the sentence named an input that provably cannot move the number it describes. - A ceiling that exists but is rejected as an anchor no longer reports `need_input:name=observed_ceiling`, which asked the user for the number displayed two rows above it. New `maximal_effort` reason. - kAlgoVersion 76 -> 77. `zone_timeline` and `zone_source` move; the day's `zones` do not, and the changelog entry says why (see below); sessions rebin only within the rescore/raw-retention window. Found while fixing this and NOT fixed here, filed separately: the day's zone bars have never sat on the observed ceiling at all. The second derivation half recomputes `zones` from `estimatedMaxHr` alone (`_wakeZoneMinutes`) and `bundle['zones'] = wake['zones']` overwrites the pure pipeline's set, so the day screen has been drawing Tanaka-binned bars under a footnote that reads `zone_source`. Fixing it means threading the anchors through `_DayBlocksInput` across the isolate boundary and moves `zones` for every user, which is its own change. Closes #290 --- lib/compute/derivation_engine.dart | 55 +++++++++++- lib/compute/hr_max.dart | 40 +++++++-- lib/data/local_repository_impl.dart | 10 ++- lib/models/metric.dart | 8 ++ lib/ui2/activity/catalogue.dart | 33 ++++++- lib/ui2/activity/day_strain.dart | 14 +-- lib/ui2/activity/live.dart | 11 +++ lib/ui2/activity/summary.dart | 19 +++- lib/ui2/screens/workout_screen.dart | 32 +++++++ test/hr_ceiling_zones_test.dart | 130 ++++++++++++++++++++++------ 10 files changed, 305 insertions(+), 47 deletions(-) diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index b4c231ac..92f0cb1e 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -1456,8 +1456,59 @@ import 'substrate.dart'; // (OpenStrap/analytics#52). Do not release with a pubspec pin that predates // that merge — recomputing v77+ against a sibling that cannot price walking // would burn the version on nothing. -const int kAlgoVersion = 80; - +// +// v81 — THE ZONE CEILING IS `max(observed, age estimate)`. +// `trainingZones` (compute/hr_max.dart) preferred `observedCeilingBpm` whenever +// one existed, in either direction. But that number is one-sided evidence: +// holding 195 proves the ceiling is at least 195, while holding 166 cannot tell +// "never went maximal" from "maxes at 166" — and on a wrist the first is far +// commoner. So a 25-year-old whose hardest HELD effort was 166 was banded at +// Z5 = 0.9·166 ≈ 149 bpm and banked 13 of the 16 minutes of a steady run in +// "Max effort". On the session-detail path the ceiling is even set by the +// session being graded — `_zoneAnchors` takes the all-time max including today +// (the day pipeline and the live tick do NOT: `observedHrCeilingBpm` is +// strictly-before, and `LiveWorkoutState.zoneSet` is pinned at start). +// +// The observed ceiling now anchors zones only when it REACHES the age line, +// which is the only direction in which it bounds anything. Below it, zones stay +// on `208 − 0.7·age`, LABELLED as the estimate. There is deliberately no +// tolerance band: a switch at `estimate − k` would move every edge k bpm on a +// 0.1 bpm change in a ceiling that creeps up over months, while `max` is +// continuous. `observed`/`karvonen` therefore now mean "this user beat the +// population line", which is a narrower and truer claim than before. +// +// WHAT ACTUALLY MOVES, for affected users only — narrower than it first looks, +// and the two exceptions are both worth knowing: +// +// * `zone_timeline` and `zone_source` move: both come off the pure pipeline's +// `trainingZones` set (onehz_pipeline.dart:630). +// * The day's `zones` DO NOT, because they never sat on the observed ceiling +// in the first place. The second derivation half recomputes them from +// `estimatedMaxHr` alone (`_wakeZoneMinutes` at :4662, `zonesFromMaxHr` at +// :4680) and `bundle['zones'] = wake['zones']` at :4920 overwrites the +// pipeline's. So the day BARS have always been Tanaka-binned while the +// footnote beside them read `zone_source`. That is a separate, pre-existing +// one-source-per-concern break (§3.8) and it is NOT fixed here; fixing it +// means threading the anchors through `_DayBlocksInput` across the isolate +// boundary, which moves `zones` for every user and is its own change. +// * Sessions rebin only where they still can. `getWorkout` recomputes +// `zone_bands` on every open, from the substrate or the frozen trace, so +// the DETAIL card is always current. The persisted `zone_min` behind the +// bars is rewritten by `rescoreRecentSessions(sinceDays: 3)` and by the +// rescore on open — but a session whose raw aged out past +// `rawRetentionDays` keeps the split it was scored with. Old cards are not +// healed by this bump, and no bump can heal them. +// +// TS-05's 28-day distribution disappears for anyone it was drawn for off a +// below-line ceiling, which is that gate working: `zonesAreMeasured` was never +// true of bands whose 100 % nobody reached. Strain, TRIMP and calories DO NOT +// MOVE — they anchor on `estimatedMaxHr`, never on the observed ceiling. +// +// `hr_ceiling_bpm` itself is untouched: the ceiling and its date are still +// measured, still stored and still served, so the zones screen keeps saying +// "highest we have seen: 166 on 3 Aug". It just no longer becomes everyone's +// 100 %. No sibling pin moves — this is entirely an edge-side anchor choice. +const int kAlgoVersion = 81; /// The sibling SHAs this version was derived against, asserted against /// pubspec.yaml in test/db_serve_version_and_reads_test.dart. /// diff --git a/lib/compute/hr_max.dart b/lib/compute/hr_max.dart index 9d0fab90..197200a3 100644 --- a/lib/compute/hr_max.dart +++ b/lib/compute/hr_max.dart @@ -81,9 +81,37 @@ double? estimatedMaxHr(num? age, String? deviceFamily) { /// * `observed` — the ceiling is measured, the resting-HR history is still too /// short for a reserve anchor ([HeartRateZones.reserveMinDays]), so these are /// %HRmax bands off the observed ceiling. -/// * `tanaka` — no observed ceiling yet: %HRmax off `208 − 0.7·age`, i.e. the -/// AGE ESTIMATE. Byte-for-byte what this app did before TS-03 landed, so a -/// user with no observed ceiling sees no number move. +/// * `tanaka` — no observed ceiling yet, OR one that does not reach the age +/// line: %HRmax off `208 − 0.7·age`, i.e. the AGE ESTIMATE. Byte-for-byte +/// what this app did before TS-03 landed. +/// +/// THE CEILING IS `max(observed, estimate)`, AND THAT ASYMMETRY IS THE WHOLE +/// RULE. An observed ceiling is ONE-SIDED evidence. Holding 195 proves the +/// ceiling is AT LEAST 195 — real information the age line does not have, and +/// it wins. Holding 166 proves nothing about the top: "has never gone maximal" +/// and "genuinely maxes at 166" produce the identical number, and on a wrist +/// the first is by far the commoner. Preferring it in BOTH directions is how a +/// 25-year-old whose hardest HELD effort was 166 got Z5 at 0.9·166 ≈ 149 bpm +/// and banked 13 of the 16 minutes of a steady run in "Max effort", captioned +/// as the age estimate. +/// +/// On the session-detail path the ceiling was even set by the session being +/// graded — `_zoneAnchors` takes the all-time max, today included. The day +/// pipeline and the live tick do not: `observedHrCeilingBpm` is +/// strictly-before-today by design, and the live `zoneSet` is pinned at start. +/// +/// There is deliberately NO tolerance band below the estimate. A threshold at +/// `estimate − k` puts a cliff in the middle of the range the ceiling creeps +/// through: at 30 an observed 177 would anchor at 177 and 176.9 at 187, moving +/// every edge 10 bpm on a 0.1 bpm change, with nothing on screen to explain it. +/// `max` is continuous — at 186.9 vs 187.1 only the LABEL moves. +/// +/// The cost, which is chosen and not overlooked: someone whose HRmax is +/// genuinely below their age line gets a Z5 they cannot reach. That is an +/// under-report — an absence — where the old behaviour was an affirmative +/// "max effort" the data did not support, and this app abstains before it +/// asserts. The set says `tanaka`, and the zones screen names the held ceiling +/// and why it is not being used (`maximal_effort`). /// /// Null only when there is no ceiling at all — no observed ceiling AND no age. /// An unstamped strap is no longer one of those cases: Tanaka does not read a @@ -99,7 +127,10 @@ ana.HeartRateZoneSet? trainingZones({ double? observedCeilingBpm, List restingHrHistory = const [], }) { - if (observedCeilingBpm != null && observedCeilingBpm > 0) { + final est = estimatedMaxHr(age, deviceFamily); + if (observedCeilingBpm != null && + observedCeilingBpm > 0 && + (est == null || observedCeilingBpm >= est)) { return ana.HeartRateZones.reserveZones( restingHrHistory: restingHrHistory, maxHr: observedCeilingBpm, @@ -109,7 +140,6 @@ ana.HeartRateZoneSet? trainingZones({ source: 'observed', ); } - final est = estimatedMaxHr(age, deviceFamily); return est == null ? null : ana.HeartRateZones.zonesFromMaxHr(est, source: 'tanaka'); diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index eac6cdbb..11fcf72f 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -3806,7 +3806,15 @@ class LocalRepositoryImpl extends LocalRepository { // The ceiling's OWN reason outranks "no hard session yet" — // on all three real databases it refused for an unstamped // strap, which a hard session cannot fix. - ? ceilingNote ?? needInputNote('observed_ceiling') + ? ceilingNote ?? + // A ceiling that EXISTS and was rejected as an anchor + // ([kCeilingCredibleGapBpm]) is not a missing one, and + // this card shows it two rows up with its date. Asking + // for the number already on screen is the false reason + // the note grammar exists to prevent. + needInputNote( + ceiling != null ? 'maximal_effort' : 'observed_ceiling', + ) : !measured ? needInputNote( 'resting_hr_days', diff --git a/lib/models/metric.dart b/lib/models/metric.dart index e89745c0..5a75b7ab 100644 --- a/lib/models/metric.dart +++ b/lib/models/metric.dart @@ -216,6 +216,14 @@ const _inputWhy = { 'observed_ceiling': 'The band has not yet held a high enough heart rate through a hard ' 'effort to measure a ceiling from.', + // DISTINCT from `observed_ceiling`, and the distinction is the whole point: + // there IS a held ceiling, it is on the screen with its date, and the card + // would otherwise ask for the thing it is simultaneously showing. + 'maximal_effort': + 'The highest heart rate held so far sits well below what your age ' + 'predicts, so it reads as an effort that was never maximal rather ' + 'than as your ceiling — the zones stay on the age estimate until the ' + 'band sees a harder one.', 'resting_hr_days': 'Not enough nights of resting heart rate behind the reserve yet.', 'sessions': diff --git a/lib/ui2/activity/catalogue.dart b/lib/ui2/activity/catalogue.dart index 92c7cf0a..8baa5b40 100644 --- a/lib/ui2/activity/catalogue.dart +++ b/lib/ui2/activity/catalogue.dart @@ -235,8 +235,39 @@ const kCalorieWhy = 'MET value × your weight, refined by heart rate.'; /// /// Never "fat burning zone", never "aerobic threshold": these are convention /// edges on a guessed ceiling, not measurements of anything metabolic. +/// NOT "your age and your strap": [estimatedMaxHr] takes `deviceFamily` and +/// DELIBERATELY IGNORES IT (hr_max.dart) — Tanaka is a population regression on +/// age alone, and swapping the strap does not move it by one bpm. The sentence +/// named an input that provably has no effect on the number it describes. const kZonesWhy = 'Zone edges are percentages of a maximum heart rate ' - 'estimated from your age and your strap — not one measured on you.'; + 'estimated from your age — not one measured on you.'; + +/// THE sentence a zone chart carries, for the anchors THAT chart was banded on. +/// +/// [source] is the set's own stamp (`karvonen` · `observed` · `tanaka`) and +/// [maxHr] the ceiling it is 100 % of. One function because the day-strain +/// detail and a session's summary card draw the same bands off the same +/// `trainingZones` set, and only one of them was reading the stamp: the summary +/// hard-coded [kZonesWhy] and so told a user whose zones were banded on a +/// MEASURED ceiling that they came from their age. A card that reports 13 +/// minutes above a boundary its own footnote misattributes is unanswerable — +/// there is no number on screen to check it against. +/// +/// [kZonesWhy] is the fallback rather than a fourth branch: an unknown stamp +/// and a measured stamp with no ceiling to name are both "we cannot say this +/// was measured on you", which is what the estimate sentence already says. +String zonesWhy(String? source, num? maxHr) => maxHr == null + ? kZonesWhy + : switch (source) { + 'karvonen' => + 'Zone edges span the gap between your measured resting heart rate ' + 'and the highest we have seen (${maxHr.round()} bpm). Both ' + 'measured on you.', + 'observed' => + 'Zone edges are percentages of the highest heart rate we have seen ' + '(${maxHr.round()} bpm) — measured, not estimated.', + _ => kZonesWhy, + }; /// The row that means most people never open the catalogue. const quickStart = [ diff --git a/lib/ui2/activity/day_strain.dart b/lib/ui2/activity/day_strain.dart index 7a973fa4..613f8a4f 100644 --- a/lib/ui2/activity/day_strain.dart +++ b/lib/ui2/activity/day_strain.dart @@ -27,7 +27,7 @@ import '../../models/metric.dart' show whyFromNote; import '../screens/home_screen.dart' show repoOf; import '../screens/metric_detail.dart' show detailScaffold; import '../ui2.dart'; -import 'catalogue.dart' show kZonesWhy; +import 'catalogue.dart' show zonesWhy; import 'zones.dart' show ZonesDetail; /// Below this the day is not comparable to a full one and the screen says so. @@ -333,17 +333,7 @@ class _DayStrainDetailState extends State { // "estimated from your age" would then be false. The 28-day // distribution is NOT here — it lives one tap away and is gated on // the same anchors (TS-05). - footnote: switch (d.zoneSource) { - 'karvonen' => - 'Zone edges span the gap between your measured resting heart ' - 'rate and the highest we have seen (${d.zoneMaxHr?.round()} ' - 'bpm). Both measured on you.', - 'observed' => - 'Zone edges are percentages of the highest heart rate we have ' - 'seen (${d.zoneMaxHr?.round()} bpm) — measured, not ' - 'estimated.', - _ => kZonesWhy, - }, + footnote: zonesWhy(d.zoneSource, d.zoneMaxHr), child: CustomPaint( size: Size.infinite, painter: ZoneBar([for (final v in z) v / total], p), diff --git a/lib/ui2/activity/live.dart b/lib/ui2/activity/live.dart index f84ae00c..f6361952 100644 --- a/lib/ui2/activity/live.dart +++ b/lib/ui2/activity/live.dart @@ -56,6 +56,13 @@ class LiveFeed { final int? steps; final List zoneMinutes; // five, Z1..Z5 + /// The stamp on the set [zoneMinutes] is binned with, and the ceiling it is + /// 100 % of — `LiveWorkoutState.zoneSet`, pinned at session start. Carried so + /// the summary this screen hands over on stop describes the same anchors the + /// live bar was drawn against. + final String? zoneSource; + final num? zoneMaxHr; + /// Per-minute mean heart rate for the session so far, DENSE — one slot per /// session minute, `null` where the band recorded nothing. /// @@ -97,6 +104,8 @@ class LiveFeed { this.distanceKm, this.steps, this.zoneMinutes = const [], + this.zoneSource, + this.zoneMaxHr, this.hrCurve = const [], this.route = const [], this.gpsActive = false, @@ -929,6 +938,8 @@ ActivityResult _baseResult( strain: feed.strain, hr: feed.hrCurve, zoneMinutes: feed.zoneMinutes, + zoneSource: feed.zoneSource, + zoneMaxHr: feed.zoneMaxHr, // The same count the live screen has printed all session, carried onto // the summary it hands over to. Null stays null: `stopWorkout` only banks // `sessions.steps` when there is one, so an unmeasured session reads the diff --git a/lib/ui2/activity/summary.dart b/lib/ui2/activity/summary.dart index 5184c43a..03a6fe98 100644 --- a/lib/ui2/activity/summary.dart +++ b/lib/ui2/activity/summary.dart @@ -237,6 +237,17 @@ class ActivityResult { final List hr; final List zoneMinutes; // five, Z1..Z5 + /// WHICH anchors [zoneMinutes] was binned against — `karvonen` · `observed` · + /// `tanaka`, the stamp `trainingZones` puts on the set — and the ceiling that + /// set is 100 % of. + /// + /// Carried rather than re-derived so this card cannot describe the split it + /// is drawing as coming from anchors it did not use. Null while the split has + /// not been enriched (the history row, a preview): the footnote then says the + /// estimate, which is the claim that needs no ceiling to back it. + final String? zoneSource; + final num? zoneMaxHr; + /// Steps the strap's own 100 Hz pedometer counted over this session — /// `AppState.workoutStepsMeasured` while it runs, `sessions.steps` once it is /// banked. That column has exactly one producer (an import and a hand-logged @@ -303,6 +314,8 @@ class ActivityResult { this.strain, this.hr = const [], this.zoneMinutes = const [], + this.zoneSource, + this.zoneMaxHr, this.steps, this.traceCoveragePct, this.route = const [], @@ -334,6 +347,8 @@ class ActivityResult { int? hrr60, List? hr, List? zoneMinutes, + String? zoneSource, + num? zoneMaxHr, int? traceCoveragePct, List? route, List<(double lat, double lng)>? geo, @@ -359,6 +374,8 @@ class ActivityResult { strain: strain, hr: hr ?? this.hr, zoneMinutes: zoneMinutes ?? this.zoneMinutes, + zoneSource: zoneSource ?? this.zoneSource, + zoneMaxHr: zoneMaxHr ?? this.zoneMaxHr, // Carried, never re-derived: every enrichment pass on this object goes // through here, and a field left off this list is a measurement the // detail screen silently loses the moment it opens. @@ -1400,7 +1417,7 @@ class _ActivitySummaryState extends State { for (var i = 0; i < 5; i++) ('Z${i + 1} · ${r.zoneMinutes[i].round()}m', ZoneBar.cols(p)[i]), ], - footnote: kZonesWhy, + footnote: zonesWhy(r.zoneSource, r.zoneMaxHr), child: CustomPaint( size: Size.infinite, painter: ZoneBar(_zoneFractions(), p)), ); diff --git a/lib/ui2/screens/workout_screen.dart b/lib/ui2/screens/workout_screen.dart index b48bc6a2..b34a1a06 100644 --- a/lib/ui2/screens/workout_screen.dart +++ b/lib/ui2/screens/workout_screen.dart @@ -1117,6 +1117,11 @@ LiveFeed _feedOf(AppState app) { strain: w?.strain, steps: app.workoutStepsMeasured, zoneMinutes: w?.zoneMinutes() ?? const [], + // The SET those minutes were binned with, not a second resolution of the + // anchors: `zoneSet` is pinned at session start precisely so a mid-session + // anchor change cannot rebrand a split already on screen. + zoneSource: w?.zoneSet?.source, + zoneMaxHr: w?.zoneSet?.maxHr, // DENSE, not the hole-free variant: this feeds the summary's chart, whose // x axis is the session clock. `perMinuteHr()` is for statistics. hrCurve: _curveOverSession(w), @@ -1373,6 +1378,16 @@ List _denseMinutes(Object? hr, [Duration? session]) { return out; } +/// Zone 5 of a persisted `zone_bands` list — the row carrying the set's own +/// `source` stamp and, as its `hi`, the ceiling the whole set is a percentage +/// of. Null for a session that banked no split, and then the footnote says the +/// estimate: the one claim that stands with no ceiling to name. +Map? _topBand(Object? bands) { + if (bands is! List || bands.length != 5) return null; + final top = bands.last; + return top is Map ? top.cast() : null; +} + /// One past session, opened from history — built from what the stores hold /// rather than from the six columns the list row carries. Future _detailOf(AppState app, _PastWorkout w) async { @@ -1381,6 +1396,7 @@ Future _detailOf(AppState app, _PastWorkout w) async { if (repo == null) return out; try { final b = await repo.getWorkout(w.id); + final band = _topBand(b['zone_bands']); out = out.copyWith( hr: _denseMinutes(b['hr'], w.duration), // The session's own mean, computed over its heart-rate stream. @@ -1391,6 +1407,22 @@ Future _detailOf(AppState app, _PastWorkout w) async { // one frozen mid-dropout has to read as partial rather than draw a // confident line across the gap. traceCoveragePct: (b['trace_coverage_pct'] as num?)?.toInt(), + // TS-04 — the anchors the bands on this card were binned against, read + // off the bands themselves. `_zoneBands` stamps every row with the set's + // `source`, and zone 5's `hi` IS the ceiling (both `zonesFromMaxHr` and + // `reserveZones` put 100 % of the anchor there), so the footnote needs no + // second read and cannot describe a different set from the bars. + zoneSource: band?['source'] as String?, + zoneMaxHr: band?['hi'] as num?, + // …and the MINUTES from the same read, not from the list row. Opening a + // session rescores it (`_rescoreSessionFromSubstrate`), so the row loaded + // with the month list can be a split binned before that correction — and + // pairing those bars with the provenance of the bands just recomputed + // beside them is the exact mismatch this card is being fixed for. + zoneMinutes: [ + for (final z in (b['zone_min'] as List? ?? const [])) + if (z is num) z.toDouble(), + ], ); } catch (_) { // Enrichment is best-effort; the scalars on the row still render. diff --git a/test/hr_ceiling_zones_test.dart b/test/hr_ceiling_zones_test.dart index 0b252e8b..a3a40814 100644 --- a/test/hr_ceiling_zones_test.dart +++ b/test/hr_ceiling_zones_test.dart @@ -23,6 +23,8 @@ import 'package:openstrap_analytics/onehz.dart' as ana; import 'package:openstrap_edge/compute/derivation_engine.dart' show kAlgoVersion; import 'package:openstrap_edge/compute/hr_max.dart'; +import 'package:openstrap_edge/ui2/activity/catalogue.dart' + show kZonesWhy, zonesWhy; import 'package:openstrap_edge/compute/manual_session.dart'; import 'package:openstrap_edge/compute/onehz_pipeline.dart'; import 'package:openstrap_edge/data/day_label.dart'; @@ -48,13 +50,13 @@ void main() { final z = trainingZones( age: 30, deviceFamily: 'gen4', - observedCeilingBpm: 184, + observedCeilingBpm: 196, restingHrHistory: rhr28, )!; expect(z.source, 'karvonen'); - expect(z.maxHr, 184); - // 50 + 0.50·(184−50) = 117, not 0.50·184 = 92. - expect(z.zones.first.lower, closeTo(117, 0.01)); + expect(z.maxHr, 196); + // 50 + 0.50·(196−50) = 123, not 0.50·196 = 98. + expect(z.zones.first.lower, closeTo(123, 0.01)); expect(zonesAreMeasured(z.source), isTrue); }); @@ -74,18 +76,71 @@ void main() { final z = trainingZones( age: 30, deviceFamily: 'gen4', - observedCeilingBpm: 184, + observedCeilingBpm: 196, restingHrHistory: List.filled( ana.HeartRateZones.reserveMinDays - 1, 50, ), )!; expect(z.source, 'observed'); - expect(z.zones.first.lower, closeTo(92, 0.01)); + expect(z.zones.first.lower, closeTo(98, 0.01)); // NOT measured for TS-05's purposes: only one of the two anchors is. expect(zonesAreMeasured(z.source), isFalse); }); + // ── the reported bug: 13 of 16 minutes of a steady run in "Max effort" ── + // + // 25 y/o, WHOOP 4.0, 16 min run, avg 148 / max 166. The band had never held + // anything above ~166, so the ceiling WAS this session's own peak and Z5 + // started at 0.9·166 ≈ 149 — below the run's average. Every hard session + // re-armed the ceiling it was then graded against. + test('an observed ceiling far below the age line does not anchor zones', + () { + final z = trainingZones( + age: 25, + deviceFamily: 'gen4', + observedCeilingBpm: 166, + restingHrHistory: rhr28, + )!; + // The estimate, LABELLED as the estimate — not Karvonen off 166. + expect(z.source, 'tanaka'); + expect(z.maxHr, closeTo(190.5, 0.01)); // 208 − 0.7·25 + expect(zonesAreMeasured(z.source), isFalse); + // Z5 is 171.45, so the 166 bpm peak is the top of Z4 and the 148 bpm + // average is Z3. Before the fix both were Z5. + expect(z.zones.last.lower, closeTo(171.45, 0.01)); + expect(z.zoneNumber(166), 4); + expect(z.zoneNumber(148), 3); + }); + + test('the ceiling is max(observed, estimate) — no tolerance band', () { + // 208 − 0.7·30 = 187. The switch is AT the estimate, and being at the + // estimate is the whole rule: a threshold BELOW it would move every edge + // by k bpm on a 0.1 bpm change in a ceiling that creeps up over months. + set(double bpm) => trainingZones( + age: 30, + deviceFamily: 'gen4', + observedCeilingBpm: bpm, + restingHrHistory: rhr28, + )!; + expect(set(187).source, 'karvonen'); // reaches the line ⇒ it is a ceiling + expect(set(186.9).source, 'tanaka'); // …does not ⇒ only a lower bound + // CONTINUOUS ACROSS THE SWITCH: 0.2 bpm of ceiling either side of the + // line may move the LABEL, but it must not move the edges. + expect(set(186.9).maxHr, closeTo(set(187.1).maxHr, 0.11)); + // ABOVE the line is the case the observed ceiling exists for: real + // information about a top the age line underestimates, and it wins. + expect(set(196).source, 'karvonen'); + expect(set(196).maxHr, 196); + }); + + test('with no age there is no line to hold the ceiling to', () { + // The comparison needs both numbers. No age ⇒ the measured ceiling is + // the only anchor there is, and it still stands on its own. + expect(trainingZones(observedCeilingBpm: 166)?.source, 'observed'); + expect(trainingZones(observedCeilingBpm: 166)?.maxHr, 166); + }); + test('no AGE refuses; an unknown strap does not', () { // Tanaka is a population regression on age, not a calibration constant — // an unstamped strap gets the estimate, labelled as the estimate. @@ -98,6 +153,31 @@ void main() { }); }); + // ── TS-04 — the footnote a zone chart carries names the RIGHT anchors ───── + // + // The session summary hard-coded `kZonesWhy` while the day screen switched on + // the stamp, so the same bands were described two ways and the card in the + // report claimed the age estimate for edges that came off a measured ceiling. + group('zonesWhy', () { + test('names the measured ceiling when the set was measured', () { + expect(zonesWhy('observed', 184), contains('184 bpm')); + expect(zonesWhy('observed', 184), contains('measured, not estimated')); + expect(zonesWhy('karvonen', 184), contains('184 bpm')); + expect(zonesWhy('karvonen', 184), contains('Both measured on you')); + // The estimate's sentence is the one thing a measured set must not say. + expect(zonesWhy('observed', 184), isNot(kZonesWhy)); + expect(zonesWhy('karvonen', 184), isNot(kZonesWhy)); + }); + + test('falls back to the estimate sentence, never to a "null bpm"', () { + expect(zonesWhy('tanaka', 190.5), kZonesWhy); + expect(zonesWhy(null, null), kZonesWhy); + // A measured stamp with no ceiling to name has no measured sentence. + expect(zonesWhy('observed', null), kZonesWhy); + expect(zonesWhy('karvonen', null), isNot(contains('null'))); + }); + }); + // ── one definition — a session's persisted split and its recomputed bands ── group('zoneMinutesFor', () { // 120 bpm for 120 minutes. On %HRmax off 187 that is Z2; on Karvonen off @@ -152,8 +232,8 @@ void main() { // ── compute — the day's zones are binned on what they were handed ───────── group('deriveDayBundle zone anchors', () { - // A flat 6 h day at 120 bpm. Under the age estimate (187) that is 64% of - // HRmax = Z2; under Karvonen off 184 with a 50 bpm rest it is 52% of + // A flat 6 h day at 128 bpm. Under the age estimate (187) that is 68% of + // HRmax = Z2; under Karvonen off 196 with a 50 bpm rest it is 53% of // reserve = Z1. The SAME heartbeats, and the bundle has to say which // convention binned them. Map bundle({double? ceiling, int rhrDays = 28}) { @@ -163,7 +243,7 @@ void main() { DayBundleInput( date: '2026-06-01', dayTsSec: [for (var i = 0; i < n; i++) t0 + i], - dayHr: List.filled(n, 120), + dayHr: List.filled(n, 128), sleepTsSec: const [], sleepHr: const [], sleepRrTsMs: const [], @@ -196,10 +276,10 @@ void main() { }); test('an observed ceiling + resting history rebins the same day on %HRR', () { - final b = bundle(ceiling: 184); + final b = bundle(ceiling: 196); expect(b['zone_source'], 'karvonen'); - expect(b['zone_max_hr'], 184); - // 120 bpm is now Z1 (52% of reserve), not Z2. + expect(b['zone_max_hr'], 196); + // 128 bpm is now Z1 (53% of reserve), not Z2. expect((b['zones'] as Map)['z1'], greaterThan(300)); expect((b['zones'] as Map)['z2'], 0); // `max_hr_used` is DELIBERATELY untouched: TRIMP/calories are not moved @@ -210,7 +290,7 @@ void main() { }); test('the zone timeline is binned by the same set as the zone minutes', () { - final tl = (bundle(ceiling: 184)['series'] as Map)['zone_timeline']; + final tl = (bundle(ceiling: 196)['series'] as Map)['zone_timeline']; expect((tl as List), isNotEmpty); expect(tl.every((e) => (e as Map)['z'] == 1), isTrue); }); @@ -297,7 +377,7 @@ void main() { 'set it', () async { await seedRhr(28); await LocalDb.putMetricSeriesValue('2026-08-01', 'hr_ceiling_bpm', 171); - await LocalDb.putMetricSeriesValue('2026-08-03', 'hr_ceiling_bpm', 184); + await LocalDb.putMetricSeriesValue('2026-08-03', 'hr_ceiling_bpm', 196); await LocalDb.putMetricSeriesValue('2026-08-09', 'hr_ceiling_bpm', 176); await seedStampedDay(); // The day that set it carries the envelope with the session behind it. @@ -307,7 +387,7 @@ void main() { payloadJson: jsonEncode({ 'hr_ceiling': { 'value': { - 'bpm': 184.0, + 'bpm': 196.0, 'ts_ms': 0, 'held_seconds': 22, 'motion_g': 0.2, @@ -325,22 +405,22 @@ void main() { final z = await repo.getZones(); final c = z['ceiling'] as Map; - expect(c['bpm'], 184); // not 176, the most RECENT one + expect(c['bpm'], 196); // not 176, the most RECENT one expect(c['date'], '2026-08-03'); expect(c['session_type'], 'Running'); expect(c['held_seconds'], 22); // Both anchors measured ⇒ Karvonen, and the screen can print both. expect(z['source'], 'karvonen'); - expect(z['max_hr'], 184); + expect(z['max_hr'], 196); expect(z['resting_hr'], 50); - expect((z['zones'] as List).first, containsPair('lo', 117)); + expect((z['zones'] as List).first, containsPair('lo', 123)); }); test( 'measured anchors but too few sessions ⇒ still no distribution', () async { await seedRhr(28); - await LocalDb.putMetricSeriesValue('2026-08-03', 'hr_ceiling_bpm', 184); + await LocalDb.putMetricSeriesValue('2026-08-03', 'hr_ceiling_bpm', 196); final now = DateTime.now().millisecondsSinceEpoch ~/ 1000; // Three sessions, each with a frozen per-minute trace. Real anchors, real // traces, and it still refuses: three workouts are not a pattern. @@ -367,10 +447,10 @@ void main() { test('enough sessions with measured anchors ⇒ the distribution, described ' 'and not prescribed', () async { await seedRhr(28); - await LocalDb.putMetricSeriesValue('2026-08-03', 'hr_ceiling_bpm', 184); + await LocalDb.putMetricSeriesValue('2026-08-03', 'hr_ceiling_bpm', 196); final now = DateTime.now().millisecondsSinceEpoch ~/ 1000; - // 10 sessions: 40 easy minutes at 120 bpm (Z1 on %HRR off 50/184) and - // 10 hard minutes at 175 bpm (Z5) each — polarised by construction. + // 10 sessions: 40 easy minutes at 128 bpm (Z1 on %HRR off 50/196) and + // 10 hard minutes at 185 bpm (Z5) each — polarised by construction. for (var i = 0; i < 10; i++) { await LocalDb.putSession({ 'id': 'w-$i', @@ -382,8 +462,8 @@ void main() { 'created_at': (now - (i + 1) * 86400) * 1000, 'trace_json': jsonEncode({ 'hr': [ - for (var m = 0; m < 40; m++) {'t': m * 60, 'v': 120}, - for (var m = 40; m < 50; m++) {'t': m * 60, 'v': 175}, + for (var m = 0; m < 40; m++) {'t': m * 60, 'v': 128}, + for (var m = 40; m < 50; m++) {'t': m * 60, 'v': 185}, ], }), }); @@ -399,7 +479,7 @@ void main() { test('a session outside the 28-day window is not counted', () async { await seedRhr(28); - await LocalDb.putMetricSeriesValue('2026-08-03', 'hr_ceiling_bpm', 184); + await LocalDb.putMetricSeriesValue('2026-08-03', 'hr_ceiling_bpm', 196); final now = DateTime.now().millisecondsSinceEpoch ~/ 1000; for (var i = 0; i < 10; i++) { await LocalDb.putSession({ From 4ef548b3e421c1bc1767430a87f7fa571fd08255 Mon Sep 17 00:00:00 2001 From: Mohammad Abdul Sahil <127765312+abdulsaheel@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:37:47 +0530 Subject: [PATCH 2/4] Update lib/ui2/screens/workout_screen.dart Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --- lib/ui2/screens/workout_screen.dart | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/ui2/screens/workout_screen.dart b/lib/ui2/screens/workout_screen.dart index b34a1a06..c3a1a306 100644 --- a/lib/ui2/screens/workout_screen.dart +++ b/lib/ui2/screens/workout_screen.dart @@ -1419,10 +1419,13 @@ Future _detailOf(AppState app, _PastWorkout w) async { // with the month list can be a split binned before that correction — and // pairing those bars with the provenance of the bands just recomputed // beside them is the exact mismatch this card is being fixed for. - zoneMinutes: [ - for (final z in (b['zone_min'] as List? ?? const [])) - if (z is num) z.toDouble(), - ], + zoneMinutes: (() { + final decoded = [ + for (final z in (b['zone_min'] as List? ?? const [])) + if (z is num) z.toDouble(), + ]; + return decoded.length == 5 ? decoded : out.zoneMinutes; + })(), ); } catch (_) { // Enrichment is best-effort; the scalars on the row still render. From 0cdb6e488d4e2e2d742581814f0caba2e4197a79 Mon Sep 17 00:00:00 2001 From: Alex Wagner Date: Thu, 27 Aug 2026 12:28:09 +0200 Subject: [PATCH 3/4] fix(zones): don't name a ceiling that did not bin these bars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit on #293: `getWorkout` recomputes `zone_bands` from the CURRENT anchors on every open, while the `zone_min` it serves can be a kept LIVE split. `reconcileSessionScore` keeps whichever side saw more minutes when the band only partly handed the window over, and that side was binned against whatever ceiling was current when it was written — so the detail card could put a footnote naming today's ceiling under bars binned against yesterday's. This branch is what makes that bite: it MOVES the anchor. `_rescoreSessionFromSubstrate` now reports `zoneMinutesRebinned` — were the minutes on the returned row binned by this pass, i.e. by the same zone set `_zoneBands` is about to use? `identical` against the substrate vector answers it, the same test the reconcile's own `changed` flag is built on. It rides out on the bundle as `zone_min_rebinned`, and the card names no ceiling when it is false, falling back to the estimate — the one claim that stands without one. Every path that never ran the reconcile (unfinished, no substrate, row moved) reports true: those serve the FROZEN trace, whose bands were banked beside the same minutes, so there is nothing to correct for. Suppressing there would blank the footnote for every session past the 3-day raw retention, which is most of history. NOT deriving the bars from `zone_bands` instead, which was the other option: the bands cover only the surviving trace, so a partly-handed-over session would show a fraction of its minutes while the strain, calories and duration beside them stayed the kept values — bars at odds with every number next to them. The durable fix is to persist the set alongside `zone_min_json` so the footnote can always describe the bars; that is a schema change and a follow-up. This is the behaviour those stamped rows would need for legacy rows anyway. --- lib/data/local_repository_impl.dart | 42 ++++++++++++++++++++++++----- lib/ui2/screens/workout_screen.dart | 16 +++++++++-- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index 11fcf72f..ef09ba30 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -2089,6 +2089,12 @@ class LocalRepositoryImpl extends LocalRepository { // list and the share card see the corrected value too. final rescored = await _rescoreSessionFromSubstrate(stored); final w = _workoutOf(rescored.row); + // TS-04 — whether `zone_min` below and the `zone_bands` added further down + // describe the SAME zone set. They are recomputed from the current anchors + // while the minutes can be a kept live split binned against an older + // ceiling, and the detail card names the bands' ceiling under the minutes' + // bars. False means it must not. + w['zone_min_rebinned'] = rescored.zoneMinutesRebinned; final startTs = w['start_ts'] as int?; if (startTs == null) return w; final endTs = @@ -2651,7 +2657,22 @@ class LocalRepositoryImpl extends LocalRepository { /// improves on each pass and converges. Returns the row with the reconciled /// values applied (never null-out a stored value), writing back only on a /// real change. Best-effort — never throws into a read path. - Future<({Map row, List>? hrRows})> + /// [zoneMinutesRebinned] answers ONE question for the caller: were the zone + /// minutes on the returned row binned by THIS pass, i.e. by the same zone set + /// [_zoneBands] is about to band the detail card's bars with? False when the + /// reconcile kept the LIVE split — a session the band only partly handed over + /// keeps whichever side saw more minutes, and that side was binned against + /// whatever ceiling was current when it was written. True on every path that + /// did not run the reconcile at all (no substrate, unfinished, row moved): + /// those serve the FROZEN trace, whose bands were banked beside the same + /// minutes, so there is nothing for the caller to correct for. + Future< + ({ + Map row, + List>? hrRows, + bool zoneMinutesRebinned, + }) + > _rescoreSessionFromSubstrate(Map row) async { final id = row['id']; final startTs = (row['start_ts'] as num?)?.toInt(); @@ -2662,13 +2683,15 @@ class LocalRepositoryImpl extends LocalRepository { endTs == null || endTs <= startTs || (row['status']?.toString() ?? '') != 'done') { - return (row: row, hrRows: null); + return (row: row, hrRows: null, zoneMinutesRebinned: true); } try { // Returned to the caller: `getWorkout` enriches from the SAME 1 Hz window // straight after this, and a two-hour session is ~7200 rows to scan twice. final hrRows = await LocalDb.hrSamplesInRange(startTs, endTs); - if (hrRows.isEmpty) return (row: row, hrRows: hrRows); + if (hrRows.isEmpty) { + return (row: row, hrRows: hrRows, zoneMinutesRebinned: true); + } final profile = Profile.fromMap(getProfileMap()); final hrBpm = [for (final e in hrRows) (e['hr'] as num).toInt()]; @@ -2719,8 +2742,12 @@ class LocalRepositoryImpl extends LocalRepository { final storedSamples = (row['trace_samples'] as num?)?.toInt(); final needsTrace = storedSamples == null || stats.hrSampleCount > storedSamples; + // `identical`, not `==`: `reconcileSessionScore` returns one of the two + // vectors it was handed, so identity IS the answer to which side won — + // the same test its own `changed` flag is built on. + final rebinned = identical(merged.zoneMinutes, stats.zoneMinutes); if (!merged.changed && !needsAvgBackfill && !needsTrace) { - return (row: row, hrRows: hrRows); + return (row: row, hrRows: hrRows, zoneMinutesRebinned: rebinned); } // `putSession` is INSERT-OR-REPLACE on the whole row, and everything @@ -2738,7 +2765,7 @@ class LocalRepositoryImpl extends LocalRepository { // read — they describe the old window, and `getWorkout` would enrich // the new one with them (a negative time-to-peak, zones over the wrong // span). The next pass scores the new window. - return (row: current ?? row, hrRows: null); + return (row: current ?? row, hrRows: null, zoneMinutesRebinned: true); } final zoneJson = jsonEncode( @@ -2793,9 +2820,10 @@ class LocalRepositoryImpl extends LocalRepository { 'trace_json': ?traceJson, if (traceJson != null) 'trace_samples': stats.hrSampleCount, }; - return (row: updated, hrRows: hrRows); + return (row: updated, hrRows: hrRows, zoneMinutesRebinned: rebinned); } catch (_) { - return (row: row, hrRows: null); // best-effort: the stored row renders + // best-effort: the stored row renders + return (row: row, hrRows: null, zoneMinutesRebinned: true); } } diff --git a/lib/ui2/screens/workout_screen.dart b/lib/ui2/screens/workout_screen.dart index b34a1a06..5c6411db 100644 --- a/lib/ui2/screens/workout_screen.dart +++ b/lib/ui2/screens/workout_screen.dart @@ -1397,6 +1397,7 @@ Future _detailOf(AppState app, _PastWorkout w) async { try { final b = await repo.getWorkout(w.id); final band = _topBand(b['zone_bands']); + final rebinned = b['zone_min_rebinned'] != false; out = out.copyWith( hr: _denseMinutes(b['hr'], w.duration), // The session's own mean, computed over its heart-rate stream. @@ -1412,8 +1413,19 @@ Future _detailOf(AppState app, _PastWorkout w) async { // `source`, and zone 5's `hi` IS the ceiling (both `zonesFromMaxHr` and // `reserveZones` put 100 % of the anchor there), so the footnote needs no // second read and cannot describe a different set from the bars. - zoneSource: band?['source'] as String?, - zoneMaxHr: band?['hi'] as num?, + // + // …EXCEPT WHEN IT WOULD. The bands are recomputed from the current + // anchors on every open, while the minutes below can be a KEPT LIVE + // split: a session the band only partly handed over keeps whichever side + // saw more minutes, and that side was binned against whatever ceiling was + // current when it was written. `zone_min_rebinned` is false exactly + // there, and then this card has no ceiling it can name for these bars — + // so it names none, and the footnote falls back to the estimate, the one + // claim that stands without one. Understating the bars to match the + // bands instead would put them at odds with the strain, calories and + // duration beside them, which are the kept values too. + zoneSource: rebinned ? (band?['source'] as String?) : null, + zoneMaxHr: rebinned ? (band?['hi'] as num?) : null, // …and the MINUTES from the same read, not from the list row. Opening a // session rescores it (`_rescoreSessionFromSubstrate`), so the row loaded // with the month list can be a split binned before that correction — and From e89398b7edc45e66dfb5447b0a05b9fadd3a2dde Mon Sep 17 00:00:00 2001 From: Mohammad Abdul Sahil <127765312+abdulsaheel@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:02:01 +0530 Subject: [PATCH 4/4] fix pr293 outside-diff finding: _detailOf must not throw on a malformed zone_bands field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _topBand casts an already-untyped band map with `as String?`/`as num?`, which throws (not returns null) on a non-null wrong-typed source/hi field. The whole getWorkout enrichment sits under one best-effort try/catch, so a single malformed band field was silently discarding hr/avgHr/maxHr/zoneMinutes too, not just the zone ceiling. Switched to `is`-checks that degrade to null instead of throwing. Left the copyWith sentinel-for-clearing suggestion (summary.dart) alone: the only caller (_detailOf) always starts from toResult(), whose zoneSource/ zoneMaxHr are null before this call, so `?? this.field` never actually needs to clear a set value today — it's a real footgun for a hypothetical future caller, not a live bug. --- lib/ui2/screens/workout_screen.dart | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/ui2/screens/workout_screen.dart b/lib/ui2/screens/workout_screen.dart index a6421368..28326ad5 100644 --- a/lib/ui2/screens/workout_screen.dart +++ b/lib/ui2/screens/workout_screen.dart @@ -1435,8 +1435,14 @@ Future _detailOf(AppState app, _PastWorkout w) async { // claim that stands without one. Understating the bars to match the // bands instead would put them at odds with the strain, calories and // duration beside them, which are the kept values too. - zoneSource: rebinned ? (band?['source'] as String?) : null, - zoneMaxHr: rebinned ? (band?['hi'] as num?) : null, + // `band?['source'] as String?` / `as num?` would THROW on a + // wrong-typed (not just missing) field, and the catch below is + // best-effort for the WHOLE enrichment — one bad band field must not + // also blank the hr/avgHr/zoneMinutes reads beside it. `is`-checks + // degrade to null instead of throwing. + zoneSource: + rebinned && band?['source'] is String ? band!['source'] as String : null, + zoneMaxHr: rebinned && band?['hi'] is num ? band!['hi'] as num : null, // …and the MINUTES from the same read, not from the list row. Opening a // session rescores it (`_rescoreSessionFromSubstrate`), so the row loaded // with the month list can be a split binned before that correction — and