diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index dc91af8c..7c5c16cc 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -1456,7 +1456,22 @@ 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 — ACTIVE ENERGY WORKOUT-GAP CREDIT. `applyDayActivity`'s calorie figure +// is built entirely from `daySub.hr` (the day's own continuous 1 Hz trace) +// and never read `sessions` at all — so a workout that scored its own +// calories through the SEPARATE session pipeline (`computeManualSessionStats` +// / the live-tick accumulator) contributed nothing to Active Energy whenever +// the band lost PPG contact for its whole window, which is common for a +// wrist-gripping or arm-swinging session (lifting, a brisk walk) next to a +// low-motion one (Pilates) that keeps contact. `applyDayActivity` now takes +// this day's `sessions` rows and credits a DONE, non-private session's own +// `calories` into the day total ONLY when `daySub` has ZERO real HR samples +// across that session's whole window — never a partial one, so a window the +// trace already priced is never double-billed. THIS CAN RAISE `calories` / +// `calories_total` for a day with a workout the day trace fully missed; +// every other day is unaffected. +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. @@ -4903,6 +4918,20 @@ class DerivationEngine { return (keys: keys, hr: [for (final k in keys) _meanWake(buckets[k]!)!]); } + /// Whether `daySub` has AT LEAST ONE real HR sample (`hr > 0`) inside + /// `[fromSec, toSec)` — the gate for the workout-gap credit in + /// [applyDayActivity]. Same "real sample" test as [_perMinuteMeanWake]'s own + /// bucketing, so "covered" here means exactly what would have priced a + /// minute there. + static bool _hasHrCoverage(Substrate s, int fromSec, int toSec) { + for (var i = 0; i < s.hr.length && i < s.tsSec.length; i++) { + if (s.hr[i] <= 0) continue; + final t = s.tsSec[i]; + if (t >= fromSec && t < toSec) return true; + } + return false; + } + static Map _wakeZoneMinutes( Substrate s, int sleepOnsetSec, @@ -5089,6 +5118,10 @@ class DerivationEngine { int liveStepsFromStrap = 0, int dynHistoryDays = 0, List> stepSpans = const [], + /// This day's `sessions` rows (`LocalDb.sessionsInRange`), for the + /// zero-coverage credit below. Defaults to none — every existing caller + /// keeps its old behaviour until it is threaded through. + List> sessions = const [], }) { final wake = _buildWakeDayFeatures( daySub, @@ -5102,6 +5135,49 @@ class DerivationEngine { dynFloorG: dynFloorG, stepSpans: stepSpans, ); + // ACTIVE ENERGY WORKOUT-GAP CREDIT. `wake['calories']` above is built + // ENTIRELY from `daySub.hr` — the day's own continuous 1 Hz trace — and + // never once reads `sessions`. A workout scores its OWN calorie figure + // through a completely separate pipeline (`computeManualSessionStats` / + // the live-tick accumulator, reconciled in `reconcileSessionScore`), so a + // session whose window the band's PPG lost contact for — commonly a + // wrist-gripping or arm-swinging one, e.g. lifting or a brisk walk, next + // to a low-motion one like Pilates that keeps contact — can be fully + // scored on its own summary card while contributing exactly nothing to + // Active Energy, because that window is a real, total gap in `daySub.hr`. + // + // Credited ONLY when a session's window has ZERO real HR samples in + // `daySub` — never a partial one — which is what makes this safe: a + // window `daySub.hr` already priced any of is left alone, so this can + // never double-bill a minute the trace already counted. A day the trace + // produced no figure for at all is left absent, same as always — this + // fills a gap in an existing number, it does not fabricate one. + if (wake['calories'] != null && sessions.isNotEmpty) { + var credited = 0.0; + for (final s in sessions) { + if (s['status'] != 'done') continue; + if ((s['private'] as num?)?.toInt() == 1) continue; + final sStart = (s['start_ts'] as num?)?.toInt(); + final sEnd = (s['end_ts'] as num?)?.toInt(); + final sCal = (s['calories'] as num?)?.toDouble(); + if (sStart == null || + sEnd == null || + sEnd <= sStart || + sCal == null || + sCal <= 0) { + continue; + } + if (_hasHrCoverage(daySub, sStart, sEnd)) continue; + credited += sCal; + } + if (credited > 0) { + wake['calories'] = (wake['calories'] as num).toDouble() + credited; + final total = wake['calories_total']; + if (total != null) { + wake['calories_total'] = (total as num).toDouble() + credited; + } + } + } _applyWakeDayFeatures(bundle, scalars, wake); _stepsAndEnergy( bundle, @@ -7078,6 +7154,7 @@ class DerivationEngine { liveStepsFromStrap: inp.liveStepsFromStrap, dynHistoryDays: inp.dynHistoryDays, stepSpans: inp.stepSpans, + sessions: inp.savedSessions, ); bundlePatch['daytime_hrv'] = _daytimeHrv(daySub, onset, offset); diff --git a/lib/ui2/activity/summary.dart b/lib/ui2/activity/summary.dart index 5184c43a..ebf17bb7 100644 --- a/lib/ui2/activity/summary.dart +++ b/lib/ui2/activity/summary.dart @@ -29,8 +29,10 @@ import '../grammar.dart'; import '../paint_activity.dart'; import '../profile/profile.dart'; import '../screens/home_screen.dart' show unitsOf; +import '../screens/log_workout.dart' show bumpInsights; import '../theme.dart'; import 'catalogue.dart'; +import 'picker.dart'; // The share card and this screen describe the same session, so they draw its // stats with the same widget and split its values with the same function. // poster.dart imports this file back for [ActivityResult]; that is the seam, @@ -811,6 +813,45 @@ class _ActivitySummaryState extends State { ]), ); + /// Correct a session's activity type — the band's own guess, or a hand-typed + /// one that was wrong. `LocalDb.setSessionType` is the narrow UPDATE this + /// reuses; it used to have no caller at all (lost in the ui2 rewrite along + /// with the screen that called it). + /// + /// Archetype-specific fields on [r] (sets, route, splits…) belong to the OLD + /// type and cannot be salvaged for the new one, so this does not try to + /// patch [r] in place — it hands back to whatever list pushed this screen, + /// which re-reads on the revision bump below. + Future _changeType(BuildContext c) async { + final id = r.sessionId; + if (id == null) return; + // Only true once a pick actually landed — pressing back out of the picker + // must return to this screen, not fall through and pop it too. + var picked = false; + await Navigator.of(c).push(MaterialPageRoute( + builder: (_) => ActivityPicker(onPick: (pc, newActivity) async { + // The stored key everywhere else uses — `startWorkout(type: + // a.typeKey)` is the live path's own write. `a.name` here would still + // resolve through `activityByName`'s normalized lookup, but it would + // store a different string than every other producer of this column. + try { + await LocalDb.setSessionType(id, newActivity.typeKey); + } catch (_) { + // Same rule as `_saveRpe`: the row is unchanged, so leave the + // picker open rather than close it over a write that never + // happened — `onPick` is a `void Function`, so there is no caller + // to hand this failure back to. + return; + } + picked = true; + if (!pc.mounted) return; + bumpInsights(pc); + Navigator.of(pc).pop(); + }), + )); + if (c.mounted && picked) Navigator.of(c).pop(); + } + Future _retrySave() async { if (_saving) return; setState(() => _saving = true); @@ -832,6 +873,11 @@ class _ActivitySummaryState extends State { Widget build(BuildContext c) { final p = P.of(c); _u = unitsOf(c); + // Only a saved session has an id to correct — a draft on screen because + // the write threw has nowhere to put it. Reserving the two-icon width + // for a row that only ever draws one icon would shove the title left on + // every unsaved-session summary for no reason. + final canChangeType = r.sessionId != null; return Scaffold( backgroundColor: p.bg, body: SafeArea( @@ -841,12 +887,27 @@ class _ActivitySummaryState extends State { child: NavBar( a.name, sub: _shortDate(r.start).toUpperCase(), - trailing: Pressable( - semanticLabel: 'Share this ${a.name.toLowerCase()}', - onTap: () => Navigator.of(c).push(MaterialPageRoute( - builder: (_) => ShareSheet(r))), - child: Icon(LucideIcons.share2, size: 19, color: p.ink2), - ), + // Two icons, each a Pressable with S.tap's own 44 pt minimum + // hit box (grammar.dart's accessibility floor, not optional) — + // S.tap * 2 alone is 12 pt short of that plus the gap between + // them, which is exactly the RenderFlex overflow this fixed. + trailingWidth: canChangeType ? S.tap * 2 + S.x3 : S.tap, + trailing: Row(mainAxisSize: MainAxisSize.min, children: [ + if (canChangeType) ...[ + Pressable( + semanticLabel: 'Change activity type', + onTap: () => _changeType(c), + child: Icon(LucideIcons.pencil, size: 18, color: p.ink2), + ), + const SizedBox(width: S.x3), + ], + Pressable( + semanticLabel: 'Share this ${a.name.toLowerCase()}', + onTap: () => Navigator.of(c).push(MaterialPageRoute( + builder: (_) => ShareSheet(r))), + child: Icon(LucideIcons.share2, size: 19, color: p.ink2), + ), + ]), ), ), Padding( diff --git a/lib/ui2/grammar.dart b/lib/ui2/grammar.dart index 231f9a6f..4d6d68bb 100644 --- a/lib/ui2/grammar.dart +++ b/lib/ui2/grammar.dart @@ -2341,12 +2341,18 @@ class NavBar extends StatelessWidget { final Widget? trailing; final VoidCallback? onBack; + /// Width of the trailing slot. Only [ActivitySummary] widens it, to fit a + /// share icon beside an edit-type one — every other caller keeps the + /// one-icon default. + final double trailingWidth; + const NavBar( this.title, { super.key, this.sub = '', this.trailing, this.onBack, + this.trailingWidth = S.tap, }); @override @@ -2382,7 +2388,7 @@ class NavBar extends StatelessWidget { ), ), SizedBox( - width: S.tap, + width: trailingWidth, child: Align(alignment: Alignment.centerRight, child: trailing), ), ], diff --git a/test/daily_energy_consistency_test.dart b/test/daily_energy_consistency_test.dart index 59d411a8..0e748a46 100644 --- a/test/daily_energy_consistency_test.dart +++ b/test/daily_energy_consistency_test.dart @@ -388,6 +388,123 @@ void main() { // silently take movement down with it. expect(bundle['movement'], isNotNull); }); + + group('workout-gap credit (Active Energy missed a session)', () { + // The bug this covers: a workout scores its own calories through the + // SEPARATE session pipeline, but the day trace above never reads + // `sessions` at all — so a session whose window the band's PPG never + // once covered contributed nothing to `calories`/`calories_total`, + // silently, even though it "was recorded" and had a real number of its + // own. `dayEnd` sits one second past this fixture's last covered + // second, so a session starting there is a clean, total gap. + Map session({ + double? calories, + String status = 'done', + int? private, + int startOffsetSec = 0, + int durationSec = 1800, + }) => + { + 'start_ts': dayEnd + startOffsetSec, + 'end_ts': dayEnd + startOffsetSec + durationSec, + 'calories': calories, + 'status': status, + 'private': private, + }; + + test('a session with NO HR coverage at all credits its calories in', + () { + final bundle = {}; + final scalars = {}; + DerivationEngine.applyDayActivity( + bundle: bundle, + scalars: scalars, + daySub: build(), + profile: older, + sleepOnsetSec: sleepOnset, + sleepOffsetSec: sleepOffset, + dayStartSec: dayStart, + dayCalendarEndSec: dayEnd + 3600, + dataNowSec: dayEnd + 3600, + sessions: [session(calories: 250.0)], + ); + final activeNoCredit = 661.58; // from the sibling test above + expect(scalars['calories'], closeTo(activeNoCredit + 250.0, 2.0)); + expect( + scalars['calories_total'], + closeTo( + (scalars['calories'] as double) + + ((bundle['calories_total'] as Map)['basal'] as int), + 1.0, + ), + reason: 'crediting a gap must not break calories_total - calories ' + '== basal', + ); + }); + + test('a session the trace already covers is never double-billed', () { + final bundle = {}; + final scalars = {}; + DerivationEngine.applyDayActivity( + bundle: bundle, + scalars: scalars, + daySub: build(), + profile: older, + sleepOnsetSec: sleepOnset, + sleepOffsetSec: sleepOffset, + dayStartSec: dayStart, + dayCalendarEndSec: dayEnd + 1, + dataNowSec: dayEnd + 1, + // Inside the fixture's own covered span (the hard hour), which + // already has real HR — so this must NOT add on top of it. + sessions: [ + { + 'start_ts': dayStart, + 'end_ts': hardUntil, + 'calories': 999.0, + 'status': 'done', + } + ], + ); + expect(scalars['calories'], closeTo(661.58, 2.0)); + }); + + test('a private session is never credited', () { + final bundle = {}; + final scalars = {}; + DerivationEngine.applyDayActivity( + bundle: bundle, + scalars: scalars, + daySub: build(), + profile: older, + sleepOnsetSec: sleepOnset, + sleepOffsetSec: sleepOffset, + dayStartSec: dayStart, + dayCalendarEndSec: dayEnd + 3600, + dataNowSec: dayEnd + 3600, + sessions: [session(calories: 250.0, private: 1)], + ); + expect(scalars['calories'], closeTo(661.58, 2.0)); + }); + + test('a live (not-yet-done) session is never credited', () { + final bundle = {}; + final scalars = {}; + DerivationEngine.applyDayActivity( + bundle: bundle, + scalars: scalars, + daySub: build(), + profile: older, + sleepOnsetSec: sleepOnset, + sleepOffsetSec: sleepOffset, + dayStartSec: dayStart, + dayCalendarEndSec: dayEnd + 3600, + dataNowSec: dayEnd + 3600, + sessions: [session(calories: 250.0, status: 'live')], + ); + expect(scalars['calories'], closeTo(661.58, 2.0)); + }); + }); }); // The 1 Hz pipeline computes the SAME active quantity for the early-read path