From 6be699d1332fc4518091388ebf4b7b7a570faeab Mon Sep 17 00:00:00 2001 From: Egor <> Date: Thu, 6 Aug 2026 18:37:12 +0200 Subject: [PATCH 1/8] Make a detected nap visible on the Sleep screen A daytime nap is detected, stored, and drawn as a band on the day timeline, but the Sleep screen only ever showed the main night. The screen that does list every sleep of the day was unreachable: its only entry point is an AppScaffold action, and the Sleep tab embeds SleepNightContent (embedded: true), so that scaffold never builds. The periods payload was also keyed wrong. The engine writes is_main / start / end / asleep_min; SleepPeriodsScreen reads onset_ts / wake_ts / duration_min, so every card would have rendered as "0m" with no time range under it. - map sleep_periods onto the keys the screen reads, and give the main period the night's TST, efficiency, stage minutes and hypnogram so the two sleep screens can't print different numbers for the same night - the day total is now the sum of what the cards show - add a naps row under the night summary; tapping it opens the breakdown, and it says the nap is not part of the numbers above - a nap carries no confidence instead of a literal 0, and the confidence dot is hidden when there is none - test/sleep_naps_visible_test.dart pins the mapping and the row No stored analytics output changed, so kAlgoVersion stays put. --- lib/data/local_repository_impl.dart | 72 +++++++++++- lib/ui/sleep/sleep_detail_screen.dart | 60 ++++++++-- test/sleep_naps_visible_test.dart | 153 ++++++++++++++++++++++++++ 3 files changed, 277 insertions(+), 8 deletions(-) create mode 100644 test/sleep_naps_visible_test.dart diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index f606ad4..35e172b 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -573,7 +573,7 @@ class LocalRepositoryImpl extends LocalRepository { } final sleepConf = _sub(b, 'sleep.accounting')?['confidence'] as num?; - return { + final night = { // Shape matches sleep_detail_screen's contract exactly. 'has_sleep': true, 'sleep_source': sleepSource, @@ -643,6 +643,22 @@ class LocalRepositoryImpl extends LocalRepository { // position PROXY, NOT supine/side/prone body position. 'wrist_orientation': b['wrist_orientation'], }; + // Sleep periods (main + naps) for the periods screen, mapped onto the key + // names that screen actually reads. The day total is the sum of what the + // cards show, so the hero can't disagree with them. + final periods = sleepPeriodsForScreen( + (b['sleep_periods'] as Map?)?['periods'], + night: night, + ); + final bundleTotal = (b['sleep_periods'] as Map?)?['total_asleep_min']; + night['periods'] = periods; + night['total_asleep_min'] = periods.isEmpty + ? bundleTotal + : periods.fold( + 0, + (a, p) => a + ((p['duration_min'] as num?)?.toInt() ?? 0), + ); + return night; } /// The persisted sleep periods with the MAIN period's hypnogram and stage @@ -2739,6 +2755,60 @@ class LocalRepositoryImpl extends LocalRepository { } } +/// The `periods` list the Sleep-periods screen reads, mapped from the engine's +/// `sleep_periods` block. The engine writes `is_main` / `start` / `end` / +/// `asleep_min`; the screen reads `onset_ts` / `wake_ts` / `duration_min`, so +/// every card used to render as `0m` with no time range under it. +/// +/// The main period carries the night's own numbers (TST, efficiency, stage +/// minutes, hypnogram) so the two sleep screens can't print different totals +/// for the same night. A nap carries only what we actually have for it: start, +/// end, length. No stages, no efficiency, and no confidence — a nap detected by +/// stillness has no confidence value, and 0 would read as "we're sure it's bad". +/// Pure + public so the mapping is unit-testable without a database. +List> sleepPeriodsForScreen( + Object? rawPeriods, { + Map night = const {}, +}) { + if (rawPeriods is! List) return const []; + num? asNum(Object? v) => + v is num ? v : (v is String ? num.tryParse(v) : null); + final out = >[]; + for (final raw in rawPeriods) { + if (raw is! Map) continue; + final p = raw.cast(); + final start = asNum(p['start'])?.toInt(); + final end = asNum(p['end'])?.toInt(); + if (start == null || end == null || end <= start) continue; + final isMain = p['is_main'] == true; + final asleepMin = + asNum(p['asleep_min'])?.toInt() ?? ((end - start) / 60).round(); + // The main card shows TST (what the Sleep screen shows). A nap has no + // asleep/awake accounting of its own, so its window IS its length. + final durationMin = isMain + ? (asNum(night['duration_min'])?.toInt() ?? asleepMin) + : asleepMin; + final stages = { + if (isMain) + for (final k in const ['light_min', 'deep_min', 'rem_min', 'nrem_min']) + if (night[k] != null) k: night[k], + }; + out.add({ + 'is_main': isMain, + 'onset_ts': start, + 'wake_ts': end, + 'duration_min': durationMin, + if (isMain && night['efficiency'] != null) + 'efficiency': night['efficiency'], + if (isMain && night['stages_confidence'] != null) + 'confidence': night['stages_confidence'], + if (isMain && night['hypnogram'] is List) 'hypnogram': night['hypnogram'], + if (stages.isNotEmpty) 'stages': stages, + }); + } + return out; +} + /// The /today `stress` block from a day bundle — the pipeline's Baevsky block, /// verbatim, with NO fallback substitute when SI couldn't compute a score. /// (Previously mirrored getDayStress's `100 - readiness` fallback; removed for diff --git a/lib/ui/sleep/sleep_detail_screen.dart b/lib/ui/sleep/sleep_detail_screen.dart index c7e8250..01fe0ac 100644 --- a/lib/ui/sleep/sleep_detail_screen.dart +++ b/lib/ui/sleep/sleep_detail_screen.dart @@ -201,6 +201,16 @@ class _SleepDetailScreenState extends State { await _runOverride(() => app.clearSleepOverride(widget.date)); } + /// Every sleep of this day, naps included. The Sleep tab renders this screen + /// embedded, so the AppScaffold action below never builds there — without a + /// second entry point the periods screen was unreachable in the shipped app. + void _openPeriods() { + Navigator.of(context).push( + themedRoute((_) => SleepPeriodsScreen(date: widget.date), + name: 'SleepPeriodsScreen'), + ); + } + /// Run a sleep-override change with a busy state, then reload this night. Future _runOverride(Future Function() action) async { setState(() => _phase = _Phase.loading); @@ -247,6 +257,7 @@ class _SleepDetailScreenState extends State { onEditTimes: _editSleepTimes, onConfirmFallback: _confirmFallback, onClearOverride: _clearOverride, + onOpenPeriods: _openPeriods, showSleepCoach: widget.showSleepCoach, ), ]; @@ -266,13 +277,7 @@ class _SleepDetailScreenState extends State { subtitle: _prettyDate(), actions: [ // All sleeps of the day (naps included) — the multi-period view. - RoundIconButton( - OsIcon.bedtime, - onTap: () => Navigator.of(context).push( - themedRoute((_) => SleepPeriodsScreen(date: widget.date), - name: 'SleepPeriodsScreen'), - ), - ), + RoundIconButton(OsIcon.bedtime, onTap: _openPeriods), ], body: RefreshIndicator( onRefresh: _load, @@ -310,6 +315,10 @@ class SleepNightContent extends StatelessWidget { final VoidCallback onConfirmFallback; final VoidCallback onClearOverride; + /// Open the per-period breakdown (main sleep + naps). Null in tests and + /// anywhere navigation isn't available; the naps row then stays hidden. + final VoidCallback? onOpenPeriods; + /// Render the Sleep Coach card (tonight's need/bedtime/wake/alarm) inline, /// between the Cycles and Nocturnal-heart sections — it only makes sense /// for TODAY's night, so only the Today segment's caller sets this true @@ -325,6 +334,7 @@ class SleepNightContent extends StatelessWidget { required this.onEditTimes, required this.onConfirmFallback, required this.onClearOverride, + this.onOpenPeriods, this.showSleepCoach = false, }); @@ -367,6 +377,20 @@ class SleepNightContent extends StatelessWidget { num? get _cyclesMean => _num(data['cycles_mean_min']); + /// Naps this day: the non-main sleep periods. This screen shows the night + /// only, so without a row for them a daytime sleep is invisible here. + List> get _naps { + final raw = data['periods']; + if (raw is! List) return const []; + return raw + .map((e) => _map(e)) + .where((m) => m.isNotEmpty && m['is_main'] != true) + .toList(); + } + + num get _napMin => _naps.fold( + 0, (a, p) => a + (_num(p['duration_min']) ?? 0)); + List> get _cycleSeries { final raw = data['cycle_series']; if (raw is! List) return const []; @@ -450,6 +474,10 @@ class SleepNightContent extends StatelessWidget { _hero(context), const SizedBox(height: Sp.x4), _summaryBento(context), + if (onOpenPeriods != null && _naps.isNotEmpty) ...[ + const SizedBox(height: Sp.x3), + _napsRow(), + ], // ── ESTIMATED STAGE BLOCK (below the trustworthy numbers) ── const SizedBox(height: Sp.x6), _stagesHeader(), @@ -556,6 +584,24 @@ class SleepNightContent extends StatelessWidget { ); } + /// Daytime sleep, on the way to the per-period breakdown. The numbers above + /// are the night only, so the row says so rather than implying the nap is in + /// them: it isn't in TST, and it doesn't move readiness. It does count toward + /// tonight's sleep need (nap credit, Sleep Coach). + Widget _napsRow() { + final n = _naps.length; + return SurfaceCard( + padding: const EdgeInsets.symmetric(horizontal: Sp.x4, vertical: Sp.x2), + child: ListRow( + icon: OsIcon.bedtime, + title: n == 1 ? 'Daytime nap' : '$n daytime naps', + subtitle: 'Not included in the night above', + value: _hm(_napMin), + onTap: onOpenPeriods, + ), + ); + } + /// Subtle "fix it" affordance shown under an auto-detected night. Widget _editTimesFooter() { if (_sleepSource != 'auto') return const SizedBox.shrink(); diff --git a/test/sleep_naps_visible_test.dart b/test/sleep_naps_visible_test.dart new file mode 100644 index 0000000..76ed071 --- /dev/null +++ b/test/sleep_naps_visible_test.dart @@ -0,0 +1,153 @@ +// A two-hour afternoon nap was detected, stored, and drawn as a band on the +// day timeline — and was still nowhere to be found on the Sleep screen. Two +// separate reasons, both pinned here: +// +// 1. The engine writes each period as `is_main` / `start` / `end` / +// `asleep_min`; the periods screen reads `onset_ts` / `wake_ts` / +// `duration_min`. Every card rendered as "0m" with no time range. +// 2. The only route into that screen was an AppScaffold action, and the Sleep +// tab embeds SleepNightContent (embedded: true), so the scaffold — and the +// action with it — never builds in the shipped app. + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:openstrap_edge/data/local_repository_impl.dart' + show sleepPeriodsForScreen; +import 'package:openstrap_edge/theme/theme.dart'; +import 'package:openstrap_edge/theme/tokens.dart'; +import 'package:openstrap_edge/ui/sleep/sleep_detail_screen.dart' + show SleepNightContent; + +/// Periods exactly as `_sleepPeriods` writes them into the bundle. +const _rawPeriods = [ + {'is_main': true, 'start': 1000000, 'end': 1024600, 'asleep_min': 410}, + {'is_main': false, 'start': 1060000, 'end': 1066060, 'asleep_min': 101}, +]; + +/// The night payload the mapping enriches the main period from. +const _night = { + 'duration_min': 400, // TST — less than the 410-minute window + 'efficiency': 0.93, + 'stages_confidence': 0.62, + 'light_min': 220, + 'deep_min': 60, + 'rem_min': 120, + 'hypnogram': [ + {'t': 1000000, 'stage': 'light'}, + ], +}; + +Widget _host(Widget child) { + AppColors.active = kDarkPalette; + return MaterialApp( + theme: buildOpenStrapTheme(kDarkPalette), + home: Scaffold(body: SingleChildScrollView(child: child)), + ); +} + +String _today() { + final d = DateTime.now(); + return '${d.year.toString().padLeft(4, '0')}-' + '${d.month.toString().padLeft(2, '0')}-' + '${d.day.toString().padLeft(2, '0')}'; +} + +Map _nightWithNap() => { + ..._night, + 'has_sleep': true, + 'sleep_source': 'auto', + 'need_min': 480, + 'onset_ts': 1000000, + 'wake_ts': 1024600, + 'periods': sleepPeriodsForScreen(_rawPeriods, night: _night), + }; + +void main() { + group('sleepPeriodsForScreen', () { + test('maps the engine keys onto the ones the screen reads', () { + final p = sleepPeriodsForScreen(_rawPeriods, night: _night); + expect(p, hasLength(2)); + expect(p[0]['onset_ts'], 1000000); + expect(p[0]['wake_ts'], 1024600); + expect(p[1]['onset_ts'], 1060000); + expect(p[1]['duration_min'], 101); + }); + + test('main period shows TST, not the window length', () { + final main = sleepPeriodsForScreen(_rawPeriods, night: _night).first; + expect(main['duration_min'], 400); + expect(main['efficiency'], 0.93); + expect(main['confidence'], 0.62); + expect((main['stages'] as Map)['deep_min'], 60); + expect(main['hypnogram'], isA()); + }); + + test('a nap carries no invented stages, efficiency or confidence', () { + final nap = sleepPeriodsForScreen(_rawPeriods, night: _night)[1]; + expect(nap.containsKey('stages'), isFalse); + expect(nap.containsKey('efficiency'), isFalse); + expect(nap.containsKey('confidence'), isFalse); + }); + + test('drops junk instead of rendering a zero-length card', () { + final p = sleepPeriodsForScreen([ + {'is_main': false, 'start': 1060000, 'end': 1060000}, + {'is_main': false, 'end': 1066060}, + 'not a period', + ]); + expect(p, isEmpty); + expect(sleepPeriodsForScreen(null), isEmpty); + }); + }); + + group('SleepNightContent naps row', () { + testWidgets('a nap is visible on the night screen and opens the breakdown', + (t) async { + t.view.physicalSize = const Size(390, 3600); + t.view.devicePixelRatio = 1.0; + addTearDown(t.view.reset); + + var opened = 0; + await t.pumpWidget(_host(SleepNightContent( + data: _nightWithNap(), + date: _today(), + onEditTimes: () {}, + onConfirmFallback: () {}, + onClearOverride: () {}, + onOpenPeriods: () => opened++, + ))); + await t.pump(const Duration(milliseconds: 1200)); + + expect(find.text('Daytime nap'), findsOneWidget); + expect(find.text('1h 41m'), findsOneWidget); + await t.tap(find.text('Daytime nap')); + await t.pump(const Duration(milliseconds: 400)); + expect(opened, 1); + expect(t.takeException(), isNull); + }); + + testWidgets('no naps, no row', (t) async { + t.view.physicalSize = const Size(390, 3600); + t.view.devicePixelRatio = 1.0; + addTearDown(t.view.reset); + + final data = _nightWithNap(); + data['periods'] = sleepPeriodsForScreen( + [_rawPeriods.first], + night: _night, + ); + await t.pumpWidget(_host(SleepNightContent( + data: data, + date: _today(), + onEditTimes: () {}, + onConfirmFallback: () {}, + onClearOverride: () {}, + onOpenPeriods: () {}, + ))); + await t.pump(const Duration(milliseconds: 1200)); + + expect(find.text('Daytime nap'), findsNothing); + }); + }); +} From 8961a5832431f013f3136b62544d4133894acee6 Mon Sep 17 00:00:00 2001 From: Egor <> Date: Thu, 6 Aug 2026 19:07:57 +0200 Subject: [PATCH 2/8] Bound a period's reported length to its own window CodeRabbit review: sleepPeriodsForScreen took asleep_min at face value, so a negative or larger-than-window value would print a negative duration on the card and skew the day total. The engine never writes one today (a nap's asleep_min is exactly its window), but this function parses defensively everywhere else, so it should here too. Falls back to the window, which the period's own start/end already vouch for. --- lib/data/local_repository_impl.dart | 10 ++++++++-- test/sleep_naps_visible_test.dart | 9 +++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index 35e172b..438dae9 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -2781,8 +2781,14 @@ List> sleepPeriodsForScreen( final end = asNum(p['end'])?.toInt(); if (start == null || end == null || end <= start) continue; final isMain = p['is_main'] == true; - final asleepMin = - asNum(p['asleep_min'])?.toInt() ?? ((end - start) / 60).round(); + // A length outside the window it came from is malformed: it would print a + // negative duration on the card and skew the day total. Fall back to the + // window, which the period's own start/end already vouch for. + final windowMin = ((end - start) / 60).round(); + final reported = asNum(p['asleep_min'])?.toInt(); + final asleepMin = (reported != null && reported >= 0 && reported <= windowMin) + ? reported + : windowMin; // The main card shows TST (what the Sleep screen shows). A nap has no // asleep/awake accounting of its own, so its window IS its length. final durationMin = isMain diff --git a/test/sleep_naps_visible_test.dart b/test/sleep_naps_visible_test.dart index 76ed071..72556a9 100644 --- a/test/sleep_naps_visible_test.dart +++ b/test/sleep_naps_visible_test.dart @@ -90,6 +90,15 @@ void main() { expect(nap.containsKey('confidence'), isFalse); }); + test('a length outside its own window falls back to the window', () { + final p = sleepPeriodsForScreen([ + {'is_main': false, 'start': 1060000, 'end': 1066060, 'asleep_min': -30}, + {'is_main': false, 'start': 1060000, 'end': 1066060, 'asleep_min': 900}, + {'is_main': false, 'start': 1060000, 'end': 1066060}, + ]); + expect(p.map((e) => e['duration_min']), everyElement(101)); + }); + test('drops junk instead of rendering a zero-length card', () { final p = sleepPeriodsForScreen([ {'is_main': false, 'start': 1060000, 'end': 1060000}, From 091118054d3589ab3d162484e17a1030cc7a824a Mon Sep 17 00:00:00 2001 From: Egor <> Date: Fri, 7 Aug 2026 08:33:23 +0200 Subject: [PATCH 3/8] Bound the main period's TST too, and un-vacuum the fallback test CodeRabbit review, second pass: - the window check covered a nap's asleep_min but not the main period, which took night['duration_min'] straight through. Same bound now: nobody sleeps longer than the window they slept in. - the fallback test asserted everyElement on a list it never sized, so it would have passed if the mapper dropped all three inputs. It checks the length first now. Adds a TST regression case for the negative and oversized values. --- lib/data/local_repository_impl.dart | 11 +++++++---- test/sleep_naps_visible_test.dart | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index 438dae9..d051426 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -2790,10 +2790,13 @@ List> sleepPeriodsForScreen( ? reported : windowMin; // The main card shows TST (what the Sleep screen shows). A nap has no - // asleep/awake accounting of its own, so its window IS its length. - final durationMin = isMain - ? (asNum(night['duration_min'])?.toInt() ?? asleepMin) - : asleepMin; + // asleep/awake accounting of its own, so its window IS its length. TST gets + // the same bound: nobody sleeps longer than the window they slept in. + final tstMin = asNum(night['duration_min'])?.toInt(); + final durationMin = + isMain && tstMin != null && tstMin >= 0 && tstMin <= windowMin + ? tstMin + : asleepMin; final stages = { if (isMain) for (final k in const ['light_min', 'deep_min', 'rem_min', 'nrem_min']) diff --git a/test/sleep_naps_visible_test.dart b/test/sleep_naps_visible_test.dart index 72556a9..170550b 100644 --- a/test/sleep_naps_visible_test.dart +++ b/test/sleep_naps_visible_test.dart @@ -90,15 +90,29 @@ void main() { expect(nap.containsKey('confidence'), isFalse); }); - test('a length outside its own window falls back to the window', () { + test('a nap length outside its own window falls back to the window', () { final p = sleepPeriodsForScreen([ {'is_main': false, 'start': 1060000, 'end': 1066060, 'asleep_min': -30}, {'is_main': false, 'start': 1060000, 'end': 1066060, 'asleep_min': 900}, {'is_main': false, 'start': 1060000, 'end': 1066060}, ]); + // hasLength first: everyElement passes vacuously on an empty list, so + // dropping all three inputs would look like a pass. + expect(p, hasLength(3)); expect(p.map((e) => e['duration_min']), everyElement(101)); }); + test('a TST longer than the window falls back to the window', () { + for (final tst in const [-30, 900]) { + final main = sleepPeriodsForScreen( + [_rawPeriods.first], + night: {..._night, 'duration_min': tst}, + ); + expect(main, hasLength(1)); + expect(main.first['duration_min'], 410); + } + }); + test('drops junk instead of rendering a zero-length card', () { final p = sleepPeriodsForScreen([ {'is_main': false, 'start': 1060000, 'end': 1060000}, From 478e29f8b9d1d2c5226f711a055aba5a309d4dac Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Fri, 7 Aug 2026 21:34:22 +0530 Subject: [PATCH 4/8] Rebase onto main: drop the translator #204 made redundant, keep the invariants Rebased onto main (kAlgoVersion 59). The data-layer half of this PR is now obsolete and actively harmful; the UI half is the part that still matters and is untouched. WHAT CHANGED AND WHY. This branch fixed the writer/reader key mismatch at the SCREEN end, via `sleepPeriodsForScreen`, which translated the producer's `start`/`end`/`asleep_min` onto `onset_ts`/`wake_ts`/`duration_min`. #204 fixed the same mismatch at the WRITER end -- the producer now emits the screen's vocabulary directly, and `_periodsWithMainStages` translates any legacy payload on read, so old finalized days still render. Left as-is, the rebase was silently broken: `night['periods'] = periods` unconditionally overwrote the enriched list with the translator's output, and the translator reads `p['start']`, which no longer exists. Every period would have been skipped -- main sleep included -- leaving an EMPTY periods list under a hero total that still rendered. No error, no log. So the overwrite and the translator are removed; one source per concern, and it is the writer. WHAT WAS KEPT. The later commits on this branch added real defensive invariants at the translator, and those are not obsolete -- they are ported to `_boundedPeriod` at the surviving seam: * a period cannot report more asleep minutes than its own window (clamped to the window, which is the trustworthy half -- `duration_min` and the bounds come from different producers, so nothing else stops a card claiming more sleep than the period it sits in); * a degenerate window is dropped rather than rendered as a zero-length card. Both re-pinned in sleep_periods_legacy_keys_test.dart. The UI half is untouched and is the reason to merge this: the periods screen was UNREACHABLE in the shipped app (its only entry point was an AppScaffold action, and the Sleep tab embeds SleepNightContent, so that scaffold never builds), plus a naps row on the night screen. Those read `data['periods']`, which main now populates correctly. Its test file kept the widget coverage and rebuilt the fixture in the current vocabulary; the six tests that only exercised the deleted translator are gone. flutter analyze clean; full suite 1328 passing, 0 failing. STILL NOT ADDRESSED, and it is this PR's own stated goal: `_daySleep` returns early on `tst == null` BEFORE the periods mapping, so a nap-only / night-shift day still shows "No sleep recorded" with no nap row. Flagged rather than fixed here -- it changes what the Sleep screen claims on a day with no night sleep, which is a product decision. --- lib/data/local_repository_impl.dart | 134 ++++++++--------------- test/sleep_naps_visible_test.dart | 97 +++++----------- test/sleep_periods_legacy_keys_test.dart | 44 ++++++++ 3 files changed, 116 insertions(+), 159 deletions(-) diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index d051426..aeb63d6 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -643,21 +643,18 @@ class LocalRepositoryImpl extends LocalRepository { // position PROXY, NOT supine/side/prone body position. 'wrist_orientation': b['wrist_orientation'], }; - // Sleep periods (main + naps) for the periods screen, mapped onto the key - // names that screen actually reads. The day total is the sum of what the - // cards show, so the hero can't disagree with them. - final periods = sleepPeriodsForScreen( - (b['sleep_periods'] as Map?)?['periods'], - night: night, - ); - final bundleTotal = (b['sleep_periods'] as Map?)?['total_asleep_min']; - night['periods'] = periods; - night['total_asleep_min'] = periods.isEmpty - ? bundleTotal - : periods.fold( - 0, - (a, p) => a + ((p['duration_min'] as num?)?.toInt() ?? 0), - ); + // NOTE: this used to re-map the periods here through + // `sleepPeriodsForScreen` and overwrite `night['periods']`. That translator + // read `start`/`end`/`asleep_min` -- the vocabulary the producer emitted + // when this branch was written. Since #204 the producer emits + // `onset_ts`/`wake_ts`/`duration_min` directly, and `_periodsWithMainStages` + // (above) already attaches the hypnogram, stage minutes and the main + // period's confidence, translating any legacy payload on read. + // + // Keeping the overwrite after that rebase would have read every period + // under keys that no longer exist, produced an EMPTY list, and blanked the + // whole screen -- main sleep included -- while the hero still showed a + // total. One source per concern: the writer-side seam owns this now. return night; } @@ -683,15 +680,16 @@ class LocalRepositoryImpl extends LocalRepository { }; return [ for (final p in raw.whereType()) - if (p['is_main'] != true) - _canonicalPeriod(p) - else - { - ..._canonicalPeriod(p), - if (hypno.isNotEmpty) 'hypnogram': hypno, - if (stages.isNotEmpty) 'stages': stages, - 'confidence': ?mainConfidence, - }, + if (_boundedPeriod(_canonicalPeriod(p)) case final bp?) + if (bp['is_main'] != true) + bp + else + { + ...bp, + if (hypno.isNotEmpty) 'hypnogram': hypno, + if (stages.isNotEmpty) 'stages': stages, + 'confidence': ?mainConfidence, + }, ]; } @@ -729,6 +727,31 @@ class LocalRepositoryImpl extends LocalRepository { }; } + /// A period's reported asleep minutes can never exceed its own window. + /// + /// Ported from #205, which added it at the (now-removed) screen-side + /// translator. It is a real invariant and belongs here, at the one seam the + /// screen reads: `duration_min` and the window come from different producers + /// (staging TST vs the detected bounds), so nothing else stops a card + /// claiming more sleep than the period it sits in. Clamped, not dropped — + /// the window is the trustworthy half. + /// + /// Returns null for a period with no usable window at all, so the caller can + /// drop it rather than render a zero-length card. + Map? _boundedPeriod(Map m) { + final onset = (m['onset_ts'] as num?)?.toInt(); + final wake = (m['wake_ts'] as num?)?.toInt(); + if (onset == null || wake == null || wake <= onset) { + // Keep it only if it carries no window claim at all; a period whose + // window is present but degenerate is junk. + return (onset == null && wake == null) ? m : null; + } + final windowMin = ((wake - onset) / 60).round(); + final dur = (m['duration_min'] as num?)?.toInt(); + if (dur == null || dur <= windowMin) return m; + return {...m, 'duration_min': windowMin}; + } + /// Mean completed-cycle length (min), or null when no cycles. num? _cyclesMeanMin(Map b) { final cyc = _sub(b, 'sleep')?['cycles']; @@ -2755,69 +2778,6 @@ class LocalRepositoryImpl extends LocalRepository { } } -/// The `periods` list the Sleep-periods screen reads, mapped from the engine's -/// `sleep_periods` block. The engine writes `is_main` / `start` / `end` / -/// `asleep_min`; the screen reads `onset_ts` / `wake_ts` / `duration_min`, so -/// every card used to render as `0m` with no time range under it. -/// -/// The main period carries the night's own numbers (TST, efficiency, stage -/// minutes, hypnogram) so the two sleep screens can't print different totals -/// for the same night. A nap carries only what we actually have for it: start, -/// end, length. No stages, no efficiency, and no confidence — a nap detected by -/// stillness has no confidence value, and 0 would read as "we're sure it's bad". -/// Pure + public so the mapping is unit-testable without a database. -List> sleepPeriodsForScreen( - Object? rawPeriods, { - Map night = const {}, -}) { - if (rawPeriods is! List) return const []; - num? asNum(Object? v) => - v is num ? v : (v is String ? num.tryParse(v) : null); - final out = >[]; - for (final raw in rawPeriods) { - if (raw is! Map) continue; - final p = raw.cast(); - final start = asNum(p['start'])?.toInt(); - final end = asNum(p['end'])?.toInt(); - if (start == null || end == null || end <= start) continue; - final isMain = p['is_main'] == true; - // A length outside the window it came from is malformed: it would print a - // negative duration on the card and skew the day total. Fall back to the - // window, which the period's own start/end already vouch for. - final windowMin = ((end - start) / 60).round(); - final reported = asNum(p['asleep_min'])?.toInt(); - final asleepMin = (reported != null && reported >= 0 && reported <= windowMin) - ? reported - : windowMin; - // The main card shows TST (what the Sleep screen shows). A nap has no - // asleep/awake accounting of its own, so its window IS its length. TST gets - // the same bound: nobody sleeps longer than the window they slept in. - final tstMin = asNum(night['duration_min'])?.toInt(); - final durationMin = - isMain && tstMin != null && tstMin >= 0 && tstMin <= windowMin - ? tstMin - : asleepMin; - final stages = { - if (isMain) - for (final k in const ['light_min', 'deep_min', 'rem_min', 'nrem_min']) - if (night[k] != null) k: night[k], - }; - out.add({ - 'is_main': isMain, - 'onset_ts': start, - 'wake_ts': end, - 'duration_min': durationMin, - if (isMain && night['efficiency'] != null) - 'efficiency': night['efficiency'], - if (isMain && night['stages_confidence'] != null) - 'confidence': night['stages_confidence'], - if (isMain && night['hypnogram'] is List) 'hypnogram': night['hypnogram'], - if (stages.isNotEmpty) 'stages': stages, - }); - } - return out; -} - /// The /today `stress` block from a day bundle — the pipeline's Baevsky block, /// verbatim, with NO fallback substitute when SI couldn't compute a score. /// (Previously mirrored getDayStress's `100 - readiness` fallback; removed for diff --git a/test/sleep_naps_visible_test.dart b/test/sleep_naps_visible_test.dart index 170550b..3af0b33 100644 --- a/test/sleep_naps_visible_test.dart +++ b/test/sleep_naps_visible_test.dart @@ -12,18 +12,11 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:openstrap_edge/data/local_repository_impl.dart' - show sleepPeriodsForScreen; import 'package:openstrap_edge/theme/theme.dart'; import 'package:openstrap_edge/theme/tokens.dart'; import 'package:openstrap_edge/ui/sleep/sleep_detail_screen.dart' show SleepNightContent; -/// Periods exactly as `_sleepPeriods` writes them into the bundle. -const _rawPeriods = [ - {'is_main': true, 'start': 1000000, 'end': 1024600, 'asleep_min': 410}, - {'is_main': false, 'start': 1060000, 'end': 1066060, 'asleep_min': 101}, -]; /// The night payload the mapping enriches the main period from. const _night = { @@ -60,70 +53,31 @@ Map _nightWithNap() => { 'need_min': 480, 'onset_ts': 1000000, 'wake_ts': 1024600, - 'periods': sleepPeriodsForScreen(_rawPeriods, night: _night), + // The vocabulary the producer emits since #204. This used to be built by + // `sleepPeriodsForScreen`, which translated the old + // start/end/asleep_min shape; that translator is gone because the writer + // now emits these keys directly and `_periodsWithMainStages` enriches + // them on read. + 'periods': const [ + { + 'is_main': true, + 'onset_ts': 1000000, + 'wake_ts': 1024600, + 'duration_min': 400, + 'efficiency': 0.93, + 'confidence': 0.62, + 'stages': {'light_min': 220, 'deep_min': 60, 'rem_min': 120}, + }, + { + 'is_main': false, + 'onset_ts': 1060000, + 'wake_ts': 1066060, + 'duration_min': 101, + }, + ], }; void main() { - group('sleepPeriodsForScreen', () { - test('maps the engine keys onto the ones the screen reads', () { - final p = sleepPeriodsForScreen(_rawPeriods, night: _night); - expect(p, hasLength(2)); - expect(p[0]['onset_ts'], 1000000); - expect(p[0]['wake_ts'], 1024600); - expect(p[1]['onset_ts'], 1060000); - expect(p[1]['duration_min'], 101); - }); - - test('main period shows TST, not the window length', () { - final main = sleepPeriodsForScreen(_rawPeriods, night: _night).first; - expect(main['duration_min'], 400); - expect(main['efficiency'], 0.93); - expect(main['confidence'], 0.62); - expect((main['stages'] as Map)['deep_min'], 60); - expect(main['hypnogram'], isA()); - }); - - test('a nap carries no invented stages, efficiency or confidence', () { - final nap = sleepPeriodsForScreen(_rawPeriods, night: _night)[1]; - expect(nap.containsKey('stages'), isFalse); - expect(nap.containsKey('efficiency'), isFalse); - expect(nap.containsKey('confidence'), isFalse); - }); - - test('a nap length outside its own window falls back to the window', () { - final p = sleepPeriodsForScreen([ - {'is_main': false, 'start': 1060000, 'end': 1066060, 'asleep_min': -30}, - {'is_main': false, 'start': 1060000, 'end': 1066060, 'asleep_min': 900}, - {'is_main': false, 'start': 1060000, 'end': 1066060}, - ]); - // hasLength first: everyElement passes vacuously on an empty list, so - // dropping all three inputs would look like a pass. - expect(p, hasLength(3)); - expect(p.map((e) => e['duration_min']), everyElement(101)); - }); - - test('a TST longer than the window falls back to the window', () { - for (final tst in const [-30, 900]) { - final main = sleepPeriodsForScreen( - [_rawPeriods.first], - night: {..._night, 'duration_min': tst}, - ); - expect(main, hasLength(1)); - expect(main.first['duration_min'], 410); - } - }); - - test('drops junk instead of rendering a zero-length card', () { - final p = sleepPeriodsForScreen([ - {'is_main': false, 'start': 1060000, 'end': 1060000}, - {'is_main': false, 'end': 1066060}, - 'not a period', - ]); - expect(p, isEmpty); - expect(sleepPeriodsForScreen(null), isEmpty); - }); - }); - group('SleepNightContent naps row', () { testWidgets('a nap is visible on the night screen and opens the breakdown', (t) async { @@ -156,10 +110,9 @@ void main() { addTearDown(t.view.reset); final data = _nightWithNap(); - data['periods'] = sleepPeriodsForScreen( - [_rawPeriods.first], - night: _night, - ); + // Main sleep only — the nap row must not appear. + data['periods'] = + [(data['periods'] as List).first as Map]; await t.pumpWidget(_host(SleepNightContent( data: data, date: _today(), diff --git a/test/sleep_periods_legacy_keys_test.dart b/test/sleep_periods_legacy_keys_test.dart index aa8b5da..b0098ff 100644 --- a/test/sleep_periods_legacy_keys_test.dart +++ b/test/sleep_periods_legacy_keys_test.dart @@ -235,4 +235,48 @@ void main() { expect(periods.first['wake_ts'], isNull); }, ); + + // Ported from #205, which added these at its (now-removed) screen-side + // translator. They are real invariants and belong at the surviving seam. + test( + 'a period cannot report more asleep minutes than its own window', + () async { + await seed({ + 'periods': [ + { + 'is_main': false, + 'onset_ts': napOnset, + 'wake_ts': napOnset + 30 * 60, // a 30-minute window + 'duration_min': 101, // ...claiming 101 minutes of sleep + }, + ], + 'total_asleep_min': 101, + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + final periods = (sleep['periods'] as List).cast>(); + expect( + periods.single['duration_min'], + 30, + reason: 'clamped to the window, which is the trustworthy half', + ); + }, + ); + + test('a degenerate window is dropped, not rendered as a zero-length card', + () async { + await seed({ + 'periods': [ + {'is_main': true, 'onset_ts': onset, 'wake_ts': wake, 'duration_min': 420}, + {'is_main': false, 'onset_ts': napOnset, 'wake_ts': napOnset}, + {'is_main': false, 'onset_ts': napWake, 'wake_ts': napOnset}, + ], + 'total_asleep_min': 420, + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + final periods = (sleep['periods'] as List).cast>(); + expect(periods, hasLength(1), reason: 'only the real main sleep survives'); + expect(periods.single['is_main'], isTrue); + }); } From 1b5d030a7a70c3c1070943f7e30b3d2425e48a1a Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Fri, 7 Aug 2026 21:53:07 +0530 Subject: [PATCH 5/8] Fix a defect I introduced: the hero total disagreed with the cards Found reviewing my own rebase commit. Porting this branch's clamp to `_boundedPeriod` kept the correction but DROPPED the mechanism that kept the total consistent with it -- the original translator recomputed `total_asleep_min` from the mapped periods for exactly this reason, and I did not carry that over. Reproduced: a nap claiming 101 min inside a 30-min window renders as 30, while the hero kept the producer's pre-clamp 521. card durations : [420, 30] sum of cards : 450 hero total : 521 <- a user can add the cards up and see this is wrong `_totalAsleepMin` now recomputes from the RENDERED periods, so a read-side correction can never leave the hero contradicting the cards under it. ABSENT STILL STAYS ABSENT, which is the part that needed care. A null stored total means the producer refused to state one -- usually because nap detection abstained, so the day holds an unknown NUMBER of unmeasured naps (#204). Summing the periods we happen to have would convert that honest "-" into a confident figure that silently omits them. So: null in, null out; and a period whose own duration is unknown makes the sum unknown again, for the same reason it does at the writer. 3 tests added covering all three paths (clamped, absent, unknown-duration). Full suite 1331 passing. --- lib/data/local_repository_impl.dart | 74 +++++++++++++++++++----- test/sleep_periods_legacy_keys_test.dart | 69 ++++++++++++++++++++++ 2 files changed, 127 insertions(+), 16 deletions(-) diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index aeb63d6..830aed4 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -573,6 +573,27 @@ class LocalRepositoryImpl extends LocalRepository { } final sleepConf = _sub(b, 'sleep.accounting')?['confidence'] as num?; + // Sleep periods (main + naps) for the periods screen. The main period is + // enriched HERE with the hypnogram + stage minutes: derivation builds the + // periods in a second isolate that never receives `series.hypnogram`, so + // this is the first point where the whole bundle is in hand. Naps carry + // neither by design — no stage claim is made for them. + // + // Naps carry their own confidence and the screen draws a ConfDot for any + // period that has one, so omitting the main period's left the main card as + // the ONLY one with no dot — reading as "unknown" for the best-evidenced + // period on the screen. Stays null when accounting had no confidence, + // which correctly draws nothing. + final periods = _periodsWithMainStages( + b, + { + 'light_min': min('light_sec'), + 'deep_min': min('deep_sec'), + 'rem_min': min('rem_sec'), + 'nrem_min': min('nrem_sec'), + }, + mainConfidence: sleepConf, + ); final night = { // Shape matches sleep_detail_screen's contract exactly. 'has_sleep': true, @@ -612,22 +633,14 @@ class LocalRepositoryImpl extends LocalRepository { // periods in a second isolate that never receives `series.hypnogram`, so // this is the first point where the whole bundle is in hand. Naps carry // neither by design — no stage claim is made for them. - 'periods': _periodsWithMainStages( - b, - { - 'light_min': min('light_sec'), - 'deep_min': min('deep_sec'), - 'rem_min': min('rem_sec'), - 'nrem_min': min('nrem_sec'), - }, - // Naps carry their own confidence and the screen draws a ConfDot for - // any period that has one, so omitting the main period's left the main - // card as the ONLY one with no dot — reading as "unknown" for the - // best-evidenced period on the screen. Stays null when accounting had - // no confidence, which correctly draws nothing. - mainConfidence: sleepConf, - ), - 'total_asleep_min': (b['sleep_periods'] as Map?)?['total_asleep_min'], + 'periods': periods, + // The hero total must equal the sum of the cards under it — a user can + // add them up. `_boundedPeriod` can CORRECT a period on read (clamping a + // duration to its own window, dropping a degenerate one), which makes the + // stored total stale, so it is recomputed from what is actually + // rendered. See [_totalAsleepMin] for why an absent stored total stays + // absent rather than being recomputed into a confident number. + 'total_asleep_min': _totalAsleepMin(b, periods), // Sleep cycles — Rosenblum 2024 "fractal cycles" (HRV-adapted): peak-to- // peak of the smoothed per-minute RMSSD series (REM peaks / NREM troughs). 'cycles': _sub(b, 'sleep')?['cycles'] ?? const [], @@ -752,6 +765,35 @@ class LocalRepositoryImpl extends LocalRepository { return {...m, 'duration_min': windowMin}; } + /// The day's total asleep minutes, consistent with the cards on screen. + /// + /// ABSENT STAYS ABSENT. A null stored total means the producer could not + /// state one — most often because nap detection abstained, so the day holds + /// an unknown NUMBER of unmeasured naps (see `_sleepPeriods`). Recomputing a + /// sum from the periods we happen to have would turn that honest "—" into a + /// confident figure that silently omits them, which is the exact claim the + /// producer refused to make. + /// + /// Otherwise the total is recomputed from the RENDERED periods rather than + /// trusted verbatim, because `_boundedPeriod` may have corrected one on read + /// and the stored sum would then be stale — leaving the hero disagreeing with + /// the cards a user can add up. A period whose own duration is unknown makes + /// the sum unknown again, for the same reason it does at the writer. + num? _totalAsleepMin( + Map b, + List> periods, + ) { + final stored = (b['sleep_periods'] as Map?)?['total_asleep_min']; + if (stored == null) return null; + var sum = 0; + for (final p in periods) { + final d = (p['duration_min'] as num?)?.toInt(); + if (d == null) return null; + sum += d; + } + return sum; + } + /// Mean completed-cycle length (min), or null when no cycles. num? _cyclesMeanMin(Map b) { final cyc = _sub(b, 'sleep')?['cycles']; diff --git a/test/sleep_periods_legacy_keys_test.dart b/test/sleep_periods_legacy_keys_test.dart index b0098ff..7d86f71 100644 --- a/test/sleep_periods_legacy_keys_test.dart +++ b/test/sleep_periods_legacy_keys_test.dart @@ -279,4 +279,73 @@ void main() { expect(periods, hasLength(1), reason: 'only the real main sleep survives'); expect(periods.single['is_main'], isTrue); }); + + test( + 'the hero total equals the sum of the CARDS after a period is clamped', + () async { + await seed({ + 'periods': [ + {'is_main': true, 'onset_ts': onset, 'wake_ts': wake, 'duration_min': 420}, + // Claims 101 min of sleep inside a 30-minute window. + { + 'is_main': false, + 'onset_ts': napOnset, + 'wake_ts': napOnset + 30 * 60, + 'duration_min': 101, + }, + ], + 'total_asleep_min': 521, // what the producer summed, pre-clamp + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + final periods = (sleep['periods'] as List).cast>(); + final cardSum = periods.fold( + 0, + (a, p) => a + ((p['duration_min'] as num?)?.toInt() ?? 0), + ); + expect(cardSum, 450); + expect( + sleep['total_asleep_min'], + 450, + reason: 'a user can add the cards up; the hero must not disagree', + ); + }, + ); + + test( + 'an ABSENT stored total is NOT recomputed into a confident number', + () async { + // total_asleep_min null = nap detection abstained, so the day holds an + // unknown NUMBER of unmeasured naps (#204). Summing the periods we do + // have would silently omit them. + await seed({ + 'periods': [ + {'is_main': true, 'onset_ts': onset, 'wake_ts': wake, 'duration_min': 420}, + ], + 'total_asleep_min': null, + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + expect((sleep['periods'] as List), hasLength(1)); + expect( + sleep['total_asleep_min'], + isNull, + reason: 'absent stays absent — the screen renders "—"', + ); + }, + ); + + test('a period with an unknown duration makes the total unknown again', + () async { + await seed({ + 'periods': [ + {'is_main': true, 'onset_ts': onset, 'wake_ts': wake, 'duration_min': null}, + {'is_main': false, 'onset_ts': napOnset, 'wake_ts': napWake, 'duration_min': 38}, + ], + 'total_asleep_min': 38, + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + expect(sleep['total_asleep_min'], isNull); + }); } From 5b9e36408d1cf337857009414f770494e704ff0f Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Fri, 7 Aug 2026 22:01:55 +0530 Subject: [PATCH 6/8] Meet this PR's actual goal: a nap-only day no longer says "no sleep" The title case -- a daytime sleep being invisible on the Sleep screen -- was still unmet after the rebase, because it needed TWO changes and only the data half had been discussed: 1. `_daySleep` returned early on `tst == null` BEFORE the periods mapping, so the naps were dropped on the floor. 2. Even with them attached, `sleep_detail_screen` sets `_Phase.empty` when `has_sleep` is false, so `SleepNightContent` -- and the naps row with it -- never builds. Verified end-to-end against a seeded bundle before and after: before: has_sleep=false periods=null after: has_sleep=false periods=1 total=38 `has_sleep` deliberately stays FALSE. It means what it says: there is no NIGHT to render a hypnogram, stages or efficiency for, and promoting a nap into one would be exactly the conflation this file avoids everywhere else. What changes is that the screen stops claiming nothing happened while the same nap is credited against sleep need and drawn as a band on the Timeline -- three surfaces, two answers. The empty state now keeps its honest "No sleep recorded for this night" card (with copy that says why there is no breakdown) and lists the daytime sleep beside it, tapping through to the periods screen. Its total obeys the same absent-is-not-zero rule as the rest: one nap with an unknown duration renders the total as "-" rather than a partial sum presented as complete. A day with neither a night nor naps is untouched -- no periods key, same empty card as before. 4 tests added, mutation-verified (reverting the attach fails exactly the nap-only test). Full suite 1333 passing. --- lib/data/local_repository_impl.dart | 24 ++++++++- lib/ui/sleep/sleep_detail_screen.dart | 62 +++++++++++++++++++++- test/sleep_periods_legacy_keys_test.dart | 65 +++++++++++++++++++----- 3 files changed, 134 insertions(+), 17 deletions(-) diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index 830aed4..8f22a7d 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -558,7 +558,29 @@ class LocalRepositoryImpl extends LocalRepository { // confirmed / none — drives the Sleep screen's confirm prompt + edit affordance. final sleepSource = (b['sleep_source'] as String?) ?? 'auto'; if (tst == null) { - return {'has_sleep': false, 'sleep_source': sleepSource}; + // NO NIGHT SLEEP — but that is not the same as no sleep at all. + // + // A nap-only or night-shift day still has detected daytime periods, and + // this early return used to drop them on the floor: the screen showed + // "No sleep recorded for this night" while the very same nap was stored, + // credited against sleep need, and drawn as a band on the Timeline. + // Three surfaces, two answers. + // + // `has_sleep` stays FALSE — it means what it says, that there is no + // NIGHT to render a hypnogram, stages or efficiency for, and inventing + // one from a nap would be exactly the conflation this file avoids + // elsewhere. The periods ride along so the screen can show what it does + // know instead of claiming nothing happened. + final napPeriods = _periodsWithMainStages(b, const {}); + if (napPeriods.isEmpty) { + return {'has_sleep': false, 'sleep_source': sleepSource}; + } + return { + 'has_sleep': false, + 'sleep_source': sleepSource, + 'periods': napPeriods, + 'total_asleep_min': _totalAsleepMin(b, napPeriods), + }; } final spt = (win?['spt_sec'] as num?); final waso = (acct?['waso_sec'] as num?); diff --git a/lib/ui/sleep/sleep_detail_screen.dart b/lib/ui/sleep/sleep_detail_screen.dart index 01fe0ac..fffd15b 100644 --- a/lib/ui/sleep/sleep_detail_screen.dart +++ b/lib/ui/sleep/sleep_detail_screen.dart @@ -204,6 +204,49 @@ class _SleepDetailScreenState extends State { /// Every sleep of this day, naps included. The Sleep tab renders this screen /// embedded, so the AppScaffold action below never builds there — without a /// second entry point the periods screen was unreachable in the shipped app. + + /// Daytime periods on a day with no detected night, for the empty state. + /// + /// Read straight from the repository payload rather than through + /// [SleepNightContent], which is not built in this phase. + List> _emptyStateNaps() { + final raw = _data['periods']; + if (raw is! List) return const []; + return [ + for (final e in raw) + if (e is Map && e['is_main'] != true) e.cast(), + ]; + } + + Widget _emptyStateNapsCard(List> naps) { + var known = 0; + var anyUnknown = false; + for (final n in naps) { + final d = (n['duration_min'] as num?)?.toInt(); + if (d == null) { + anyUnknown = true; + } else { + known += d; + } + } + // An unknown duration makes the TOTAL unknown — the same rule the producer + // and the periods screen use. A partial sum presented as the total would + // under-report by exactly the part we could not measure. + final value = anyUnknown + ? '—' + : (known >= 60 ? '${known ~/ 60}h ${known % 60}m' : '${known}m'); + return SurfaceCard( + padding: const EdgeInsets.symmetric(horizontal: Sp.x4, vertical: Sp.x2), + child: ListRow( + icon: OsIcon.bedtime, + title: naps.length == 1 ? 'Daytime nap' : '${naps.length} daytime naps', + subtitle: 'Tap for the full breakdown', + value: value, + onTap: _openPeriods, + ), + ); + } + void _openPeriods() { Navigator.of(context).push( themedRoute((_) => SleepPeriodsScreen(date: widget.date), @@ -228,15 +271,30 @@ class _SleepDetailScreenState extends State { List _sections() { if (_phase == _Phase.loading) return [_loading()]; if (_phase == _Phase.empty) { + // "No NIGHT" is not "no sleep". A nap-only or night-shift day has + // detected daytime periods, and showing the bare empty card while the + // same nap is credited against sleep need and drawn on the Timeline is + // the screen contradicting the rest of the app. The night breakdown is + // still genuinely absent — there is no hypnogram, no stages, no + // efficiency — so the card stays; the naps are shown alongside it rather + // than dressed up as a night. + final naps = _emptyStateNaps(); return [ StateCard( icon: OsIcon.sleep, title: 'No sleep recorded for this night', - message: 'Wear your strap overnight and sync — your breakdown ' - 'appears once a night has been recorded.', + message: naps.isEmpty + ? 'Wear your strap overnight and sync — your breakdown ' + 'appears once a night has been recorded.' + : 'No overnight sleep was detected, so there is no stage ' + 'breakdown for this day. Daytime sleep is listed below.', actionLabel: 'Add sleep times', onAction: _editSleepTimes, ), + if (naps.isNotEmpty) ...[ + const SizedBox(height: Sp.x3), + _emptyStateNapsCard(naps), + ], ]; } if (_phase == _Phase.error) { diff --git a/test/sleep_periods_legacy_keys_test.dart b/test/sleep_periods_legacy_keys_test.dart index 7d86f71..669c7c0 100644 --- a/test/sleep_periods_legacy_keys_test.dart +++ b/test/sleep_periods_legacy_keys_test.dart @@ -54,25 +54,27 @@ void main() { const napOnset = onset + 14 * 3600; const napWake = napOnset + 40 * 60; - Future seed(Map sleepPeriods) async { + Future seed(Map sleepPeriods, {bool night = true}) async { await LocalDb.putDayResult( dayId: '2026-06-15', algoVersion: 1, // a pre-rename generation payloadJson: jsonEncode({ 'scalars': {'tst_min': 420.0}, - 'sleep': { - 'accounting': { - 'confidence': 0.7, - 'value': {'tst_sec': 420 * 60, 'efficiency_pct': 92.0}, - }, - 'window': { - 'value': { - 'onset_ms': onset * 1000, - 'offset_ms': wake * 1000, - 'spt_sec': 7 * 3600, - }, - }, - }, + 'sleep': night + ? { + 'accounting': { + 'confidence': 0.7, + 'value': {'tst_sec': 420 * 60, 'efficiency_pct': 92.0}, + }, + 'window': { + 'value': { + 'onset_ms': onset * 1000, + 'offset_ms': wake * 1000, + 'spt_sec': 7 * 3600, + }, + }, + } + : const {}, 'sleep_periods': sleepPeriods, }), windowJson: '{}', @@ -348,4 +350,39 @@ void main() { final sleep = await repo.getDaySleep('2026-06-15'); expect(sleep['total_asleep_min'], isNull); }); + + group('nap-only day — no detected night', () { + test('the naps are attached instead of being dropped on the floor', () async { + // The `tst == null` early return used to drop these, so the screen said + // "No sleep recorded for this night" while the same nap was credited + // against sleep need and drawn on the Timeline. + await seed({ + 'periods': [ + { + 'is_main': false, + 'onset_ts': napOnset, + 'wake_ts': napWake, + 'duration_min': 38, + }, + ], + 'total_asleep_min': 38, + }, night: false); + + final sleep = await repo.getDaySleep('2026-06-15'); + expect( + sleep['has_sleep'], + isFalse, + reason: 'there is genuinely no NIGHT to stage — that stays honest', + ); + expect((sleep['periods'] as List), hasLength(1)); + expect(sleep['total_asleep_min'], 38); + }); + + test('a day with neither night nor naps is unchanged', () async { + await seed({'periods': const [], 'total_asleep_min': null}, night: false); + final sleep = await repo.getDaySleep('2026-06-15'); + expect(sleep['has_sleep'], isFalse); + expect(sleep['periods'], isNull, reason: 'nothing to show, nothing added'); + }); + }); } From 0618c077dfacf7ee20e4b4b7b41e9ccf2923739d Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Fri, 7 Aug 2026 22:36:41 +0530 Subject: [PATCH 7/8] _boundedPeriod: a negative duration is corrupt, not merely small CodeRabbit found a real gap in the clamp I ported: `dur <= windowMin` accepts NEGATIVE values, so a malformed duration passed straight through to both the card and the hero. Reproduced -- a stored `duration_min: -50` rendered as -50 and summed into the total as -50, understating the day. FIXED, but NOT the way the bot proposed. Its patch was `(dur >= 0 && dur <= windowMin)`, which falls through to the existing `return {...m, 'duration_min': windowMin}` -- so a corrupt -50 would be clamped UP to the FULL WINDOW, inventing a whole night of sleep out of garbage. That is a worse claim than the bug it fixes. A negative duration is not a small measurement, it is not a measurement at all. It becomes UNKNOWN: clamping down to 0 would assert "you did not sleep", which we also do not know. Unknown then propagates through `_totalAsleepMin`, so the hero reads "-" rather than a total built on a value we know is nonsense -- consistent with how absence is handled everywhere else on this seam. 1 test added, asserting both that the duration is null AND that it is not the window length, so a future "fix" toward the bot's version fails loudly. Full suite 1334 passing. --- lib/data/local_repository_impl.dart | 10 +++++++- test/sleep_periods_legacy_keys_test.dart | 32 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index 8f22a7d..d3c5004 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -783,7 +783,15 @@ class LocalRepositoryImpl extends LocalRepository { } final windowMin = ((wake - onset) / 60).round(); final dur = (m['duration_min'] as num?)?.toInt(); - if (dur == null || dur <= windowMin) return m; + if (dur == null || (dur >= 0 && dur <= windowMin)) return m; + // NEGATIVE is not "too small", it is CORRUPT — there is no such thing as + // minus fifty minutes of sleep. It is dropped to unknown rather than + // clamped to either end: clamping UP to the window would invent a full + // night out of garbage, and clamping DOWN to 0 would state "you did not + // sleep", which is a measurement we do not have. Unknown then propagates + // through `_totalAsleepMin`, so the hero reads "—" instead of a total + // built on a value we know is nonsense. + if (dur < 0) return {...m, 'duration_min': null}; return {...m, 'duration_min': windowMin}; } diff --git a/test/sleep_periods_legacy_keys_test.dart b/test/sleep_periods_legacy_keys_test.dart index 669c7c0..1643d69 100644 --- a/test/sleep_periods_legacy_keys_test.dart +++ b/test/sleep_periods_legacy_keys_test.dart @@ -385,4 +385,36 @@ void main() { expect(sleep['periods'], isNull, reason: 'nothing to show, nothing added'); }); }); + + test( + 'a NEGATIVE duration is corrupt, not small — it becomes unknown, and is ' + 'never clamped up into a full night', + () async { + await seed({ + 'periods': [ + { + 'is_main': false, + 'onset_ts': onset, + 'wake_ts': wake, // a 7-hour window + 'duration_min': -50, + }, + ], + 'total_asleep_min': -50, + }, night: false); + + final sleep = await repo.getDaySleep('2026-06-15'); + final periods = (sleep['periods'] as List).cast>(); + expect(periods.single['duration_min'], isNull); + expect( + periods.single['duration_min'], + isNot(420), + reason: 'clamping up would invent a full night out of garbage', + ); + expect( + sleep['total_asleep_min'], + isNull, + reason: 'unknown propagates — the hero reads "—"', + ); + }, + ); } From cb4ff6747dc8394781315efc3ac5937fc95698e5 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Fri, 7 Aug 2026 22:51:00 +0530 Subject: [PATCH 8/8] One authoritative nap total, and an unknown one is never rendered as zero CodeRabbit, valid on both counts, and both are the SAME rule I had just applied one layer down reappearing in the UI. 1. `_emptyStateNapsCard` computed its own total by summing the rendered periods, ignoring `_data['total_asleep_min']`. The repository already decides that (`_totalAsleepMin`) and deliberately returns NULL when the producer could not state a complete figure -- usually because nap detection abstained, so the day holds an unknown NUMBER of naps. Re-summing whatever periods happen to be present presented a partial figure as the day's total, which is exactly the claim the layer below refused to make. Now reads the authoritative value and renders "-" when it is absent. 2. `_napMin` (pre-existing) folded with `?? 0`, so a nap whose duration is unknown counted as ZERO and the row showed the remainder as if it were the full nap total -- under-reporting by exactly the part we could not measure. Now returns null if any nap's duration is unknown; `_hm` already renders null as "-", so the row degrades honestly with no call-site change. Both are the absent-is-not-zero rule this seam follows everywhere else. Worth noting the shape of the mistake: I fixed hero-vs-cards at the repository layer earlier in this branch, then introduced the same duplicate-aggregate at the widget layer in the very next commit. 1 widget test added, mutation-verified (restoring `?? 0` fails it). Full suite 1335 passing. --- lib/ui/sleep/sleep_detail_screen.dart | 42 ++++++++++++++++----------- test/sleep_naps_visible_test.dart | 34 ++++++++++++++++++++++ 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/lib/ui/sleep/sleep_detail_screen.dart b/lib/ui/sleep/sleep_detail_screen.dart index fffd15b..fd81972 100644 --- a/lib/ui/sleep/sleep_detail_screen.dart +++ b/lib/ui/sleep/sleep_detail_screen.dart @@ -219,22 +219,17 @@ class _SleepDetailScreenState extends State { } Widget _emptyStateNapsCard(List> naps) { - var known = 0; - var anyUnknown = false; - for (final n in naps) { - final d = (n['duration_min'] as num?)?.toInt(); - if (d == null) { - anyUnknown = true; - } else { - known += d; - } - } - // An unknown duration makes the TOTAL unknown — the same rule the producer - // and the periods screen use. A partial sum presented as the total would - // under-report by exactly the part we could not measure. - final value = anyUnknown + // ONE authoritative total, not a second one computed here. The repository + // already decides this (`_totalAsleepMin`) and deliberately returns null + // when the producer could not state a complete figure — most often because + // nap detection abstained, so the day holds an unknown NUMBER of naps. + // Re-summing the periods that happen to be present would present a partial + // figure as the day's total, which is exactly the claim the layer below + // refused to make. + final total = (_data['total_asleep_min'] as num?)?.toInt(); + final value = total == null ? '—' - : (known >= 60 ? '${known ~/ 60}h ${known % 60}m' : '${known}m'); + : (total >= 60 ? '${total ~/ 60}h ${total % 60}m' : '${total}m'); return SurfaceCard( padding: const EdgeInsets.symmetric(horizontal: Sp.x4, vertical: Sp.x2), child: ListRow( @@ -446,8 +441,21 @@ class SleepNightContent extends StatelessWidget { .toList(); } - num get _napMin => _naps.fold( - 0, (a, p) => a + (_num(p['duration_min']) ?? 0)); + /// Minutes asleep across this day's naps, or NULL if any nap's duration is + /// unknown. + /// + /// `?? 0` here would silently under-report by exactly the part we could not + /// measure, and present the remainder as the full nap total — the same + /// absent-is-not-zero rule the producer and the periods screen follow. + num? get _napMin { + num sum = 0; + for (final p in _naps) { + final d = _num(p['duration_min']); + if (d == null) return null; + sum += d; + } + return sum; + } List> get _cycleSeries { final raw = data['cycle_series']; diff --git a/test/sleep_naps_visible_test.dart b/test/sleep_naps_visible_test.dart index 3af0b33..28ac47e 100644 --- a/test/sleep_naps_visible_test.dart +++ b/test/sleep_naps_visible_test.dart @@ -126,4 +126,38 @@ void main() { expect(find.text('Daytime nap'), findsNothing); }); }); + + group('an unknown nap duration is never rendered as zero', () { + testWidgets('the naps row shows "—" when a nap duration is unknown', + (t) async { + t.view.physicalSize = const Size(390, 3600); + t.view.devicePixelRatio = 1.0; + addTearDown(t.view.reset); + + final data = _nightWithNap(); + final periods = (data['periods'] as List).cast>(); + data['periods'] = [ + periods.first, + {...periods[1], 'duration_min': null}, + ]; + + await t.pumpWidget(_host(SleepNightContent( + data: data, + date: _today(), + onEditTimes: () {}, + onConfirmFallback: () {}, + onClearOverride: () {}, + onOpenPeriods: () {}, + ))); + await t.pumpAndSettle(); + + expect(find.text('Daytime nap'), findsOneWidget); + expect( + find.text('—'), + findsWidgets, + reason: 'summing an unknown as 0 would under-report the nap total', + ); + expect(find.text('0m'), findsNothing); + }); + }); }