From e68f8300c8303e833703abfa45ffb08f57cc9e5b Mon Sep 17 00:00:00 2001 From: SATHVIK SVS Date: Tue, 4 Aug 2026 11:17:13 +0530 Subject: [PATCH 1/6] naps: one detector, honest minutes, and no phantom nap (kAlgoVersion 55) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PIN NOT YET UPDATED. pubspec.yaml still points analytics at f0d1153, which does NOT contain the new nap detector; local builds resolve it via the gitignored pubspec_overrides.yaml. Re-pin to the analytics nap commit (locally 54ba3c6) before this ships — AGENTS §3.5, and the v43 changelog that described an analytics fix its pin never contained, leaving the bug live three releases. Requires analytics `sleep/nap.dart`: naps were "detected" by the NOCTURNAL detector, which rejects them by design, so the 20–45 min nap was structurally undetectable. Edge side: ONE NAP SOURCE (§3.8). `_sleepPeriods` ran its own second detector — 20-min runs of still, on-wrist minutes — beside `detectNaps`. They disagreed on real days: the committed payload.json has a 21-minute period from one and `naps.count: 0` from the other, feeding the Sleep-periods screen and the Timeline respectively. Naps are now passed in, not re-derived. THE SCREEN NEVER WORKED. sleep_periods_screen read onset_ts/wake_ts/ duration_min/efficiency/confidence/stages/hypnogram; the producer wrote start/end/asleep_min. Every nap card rendered "0m" with a red low-confidence dot no matter what was detected. Periods now speak that contract, and duration_min is minutes ASLEEP for the main sleep and naps alike — they were different units under one label, and then summed. nap_min IS TST, NOT TIME IN BED, and is subtracted 1:1 from sleep need, so crediting in-bed minutes over-credited every nap by its awake time and always erred toward recommending LESS sleep than the user needs. WRIST_OFF/WRIST_ON and CHARGING_ON/OFF now reach the detector, from band_events. A band on a table or a charger is motionless and reads as deep rest — the dominant nap false positive. These events have been decoded and persisted all along, and detectSleep has always taken a `wristOff` argument; nothing ever supplied one (§4.7). ABSENT IS NOT ZERO (§3.3/§4.1). When nap detection cannot judge a day, nap_min is left UNWRITTEN and the naps block carries a null value. The sleep-need credit reads TODAY only, via an explicit `is_today` stamp — it previously fell through `_lastNum` to YESTERDAY's nap minutes, and taking the last record positionally is also yesterday on any day not yet derived. total_asleep_min is null when any component is unknown, instead of a confident total short by the unmeasured part. A period with no asleep minutes renders "—", and an unknown confidence draws no dot rather than a red one. THE CREDIT IS DISCLOSED. sleep_coach.nap_credit_min carries the reduction that was actually APPLIED — sleepNeed clamps to [6h, 11h] after subtracting, so the raw nap minutes are not always what came off — and the coach card shows it instead of silently shrinking both the need and the "% of need" ring. Main-sleep TST/efficiency are carried into the day-blocks isolate. It builds its own scMap seeded with rhr alone, so reading scMap['tst_min'] there is null forever, which would have made every main-sleep card read "—". Efficiency is normalized from the stored percent to the 0..1 the card wants. The hypnogram is attached at the read seam instead, the first point holding the whole bundle. Tests: test/nap_credit_test.dart pins the today-scoping and the applied-credit disclosure, each verified to FAIL against the pre-fix code (need dropped to 22628s from 28028s — exactly 5400s, yesterday's nap). Note test/workout_reliability_test.dart has one PRE-EXISTING failure, confirmed identical on clean main via a worktree; it is unrelated to this change. --- lib/compute/crossday_pipeline.dart | 48 +++- lib/compute/derivation_engine.dart | 338 +++++++++++++++++++------ lib/data/db.dart | 76 ++++++ lib/data/local_repository_impl.dart | 45 +++- lib/ui/insights/coach_cards.dart | 17 ++ lib/ui/sleep/sleep_periods_screen.dart | 37 ++- test/nap_credit_test.dart | 140 ++++++++++ 7 files changed, 608 insertions(+), 93 deletions(-) create mode 100644 test/nap_credit_test.dart diff --git a/lib/compute/crossday_pipeline.dart b/lib/compute/crossday_pipeline.dart index e2d874db..c467050e 100644 --- a/lib/compute/crossday_pipeline.dart +++ b/lib/compute/crossday_pipeline.dart @@ -195,13 +195,33 @@ Map buildCrossDayBundle( final debtSec = (sleepDebt.present ? (sleepDebt.value!.debtHours ?? 0.0) : 0.0) * 3600.0; final todayStrain = _lastNum(days, 'strain') ?? 0.0; - final todayNapSec = (_lastNum(days, 'nap_min') ?? 0.0) * 60.0; + // TODAY's naps only, and minutes ASLEEP (the analytics detector reports TST + // and in-bed separately now). No reading means NO credit — that leaves the + // recommendation slightly high, which is the safe direction; reaching back a + // day to find a number would be the unsafe one. + final todayNapMin = _todayNum(days, 'nap_min'); + final todayNapSec = (todayNapMin ?? 0.0) * 60.0; final need = ana.sleepNeed( baselineNeedSec: baselineNeedSec, sleepDebtSec: debtSec < 0 ? 0.0 : debtSec, dayStrain: todayStrain, napCreditSec: todayNapSec, ); + // What the credit ACTUALLY changed. `sleepNeed` clamps to [6 h, 11 h] AFTER + // subtracting, so a large credit against a low baseline is only partly + // realized — a 3 h nap does not remove 3 h of need. Disclosing the raw nap + // minutes would state a reduction the number above never took. + final needNoNap = ana.sleepNeed( + baselineNeedSec: baselineNeedSec, + sleepDebtSec: debtSec < 0 ? 0.0 : debtSec, + dayStrain: todayStrain, + napCreditSec: 0.0, + ); + final appliedNapCreditMin = (todayNapMin == null || + !need.present || + !needNoNap.present) + ? null + : ((needNoNap.value!.needSec - need.value!.needSec) / 60).round(); // last night's TST (sec) for performance. final lastTstMin = _lastNum(days, 'tst_min'); final perf = (need.present && lastTstMin != null) @@ -330,6 +350,12 @@ Map buildCrossDayBundle( // ── Coaching + fitness (forward-looking, today) ── 'sleep_coach': { 'need': need.toJson((v) => v.toJson()), + // Minutes the nap credit ACTUALLY removed from `need` — not the raw nap + // minutes, which the clamp can partly swallow. Lets the card show the + // adjustment instead of applying it invisibly. Null means today produced + // no nap reading, which is different from a confident zero: the UI must + // not render "−0m" for "we do not know". + 'nap_credit_min': appliedNapCreditMin, 'performance': perf.toJson((v) => v.toJson()), 'bedtime': bedtime.toJson((v) => v.toJson()), 'wake': wakeRec.toJson((v) => v.toJson()), @@ -372,6 +398,26 @@ double? _median(List xs) { return s.length.isOdd ? s[mid] : (s[mid - 1] + s[mid]) / 2.0; } +/// The value of [key] on the MOST RECENT day only, or null if that day did not +/// produce one. +/// +/// Unlike [_lastNum] this never reaches back to an earlier day. For a +/// TODAY-scoped quantity that is the difference between "we have no reading" +/// and a fabricated one: `_lastNum(days, 'nap_min')` would credit YESTERDAY's +/// naps against tonight's sleep need whenever today's nap detection abstained, +/// which is imputation (AGENTS §3.3) and always errs toward recommending less +/// sleep than the user needs. +/// Requires the last record to be explicitly stamped `is_today` (see +/// `_refreshCrossDayInputArtifact`). Taking `days.last` positionally is not +/// enough: on a day with no derived row yet, the most recent record IS +/// yesterday, so a positional read reproduces the very imputation this replaces. +double? _todayNum(List> days, String key) { + if (days.isEmpty) return null; + final last = days.last; + if (last['is_today'] != true) return null; + return _numOrNull(last[key]); +} + /// The last non-null value of [key] across the (oldest-first) day records. double? _lastNum(List> days, String key) { for (var i = days.length - 1; i >= 0; i--) { diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 0653cc67..e0316ccd 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -404,7 +404,83 @@ import 'substrate.dart'; // output moves. NOTE this bump does not retro-fix an existing import — imported // days are force-finalized snapshots with no stored raw to recompute from, so // an already-imported day needs a re-import to pick its steps up. -const int kAlgoVersion = 54; +// v55: NAPS. Daytime naps were "detected" by the NOCTURNAL detector, which +// rejects them on purpose — `AdvancedSleepStager.minSleepMin = 60` exists so +// "daytime naps and stray still-blocks stay excluded", and anything centred +// 11:00–20:00 local additionally needed ≥90 min plus an HR dip. So the 20–45 +// min afternoon nap was STRUCTURALLY undetectable, and `detectNaps` advertised +// a 20-min floor it could never reach, returning an empty list with a +// reassuring (and false) "no qualifying naps (20 min–3 h)" note. +// +// SIBLING (analytics): new `sleep/nap.dart` — the only nap source. Enumerates +// every van Hees z-angle immobility bout on the complement of the main sleep +// window (an ANGLE, so it does not inherit the ~13% |accel| spread across +// static postures), requires an HR dip against the AWAKE-DAYTIME baseline +// rather than a night-dominated whole-day median, and reports TST and in-bed +// separately. No sleep-stage claim: a 30-min nap holds no complete cycle and +// the daytime HR duty cycle will not support a 4-class partition. The shared +// immobility primitive is factored out as `immobilityMask` so night and nap +// run one implementation. Specifics worth knowing: +// - The awake baseline excludes the main sleep AND every detected bout. It +// cannot include the candidate's own seconds or the hours of tonight's +// sleep the nap window borrows, or the dip gate becomes self-suppressing — +// the quieter the sleep, the lower the bar it must beat. +// - Under 10 min of awake HR, the day is not judged at all. A median over a +// handful of samples is not a baseline, and every verdict hangs off it. +// - Durations are WALL CLOCK, not sample counts. The substrate is a +// positional array with pruning/sync holes, so a run also breaks at a +// timestamp discontinuity; otherwise an unobserved hour reads as unbroken +// stillness and 20 min of evidence reports a 2 h nap. +// - Deferral is CHAIN-aware. Deferring only the bout that touches the array +// end is not enough: an ordinary 6-min awakening at 01:50 splits tonight's +// sleep, and only the trailing half touches the end. Every bout chained to +// an unfinished one (within napChainGapSec) is unfinished too. +// Verify the analytics pin actually contains this before shipping the bump +// (AGENTS §3.5). +// +// EDGE-LOCAL changes, which ship the moment this constant lands: +// - `_sleepPeriods` no longer runs its OWN nap detector (20-min stillness +// runs). That second notion disagreed with `detectNaps` on real days — +// the committed `payload.json` shows a 21-minute period alongside +// `naps.count: 0` — and fed a different screen. One source now (§3.8). +// - Periods speak the contract the Sleep-periods screen actually reads +// (`onset_ts`/`wake_ts`/`duration_min`/`efficiency`), which it never did: +// every nap card rendered "0m" with a red confidence dot regardless of +// what was detected. `duration_min` is minutes ASLEEP for both the main +// sleep and naps, which were previously different units under one label. +// - `nap_min` is TST, not the in-bed span. It is subtracted 1:1 from sleep +// need, so crediting in-bed minutes over-credited every nap by its awake +// time and always erred toward recommending LESS sleep. +// - An unfinished bout is DEFERRED, not emitted (see the sibling notes). With +// a 3 h post-midnight buffer the first hours of TONIGHT'S sleep were being +// written as a multi-hour "nap" for the day that was ending, then counted +// again as tomorrow's main sleep. +// - The main sleep period's TST/efficiency are carried into the day-blocks +// isolate. That isolate builds its own `scMap` seeded with `rhr` alone, so +// reading `scMap['tst_min']` there yields null forever — which would have +// made every main-sleep card read "—". Efficiency is normalized from the +// stored percent to the 0..1 the card contract uses. +// - `total_asleep_min` is null when any listed period's minutes are unknown. +// Summing a null as 0 printed a confident total short by exactly the part +// we could not measure, and the hero arc divides by it. +// - `sleep_coach.nap_credit_min` is the credit ACTUALLY applied, not the raw +// nap minutes: `sleepNeed` clamps to [6 h, 11 h] after subtracting, so a +// large credit is only partly realized. +// - Today-scoped reads require an explicit `is_today` stamp on the cross-day +// record. Taking the last record positionally is yesterday on any day whose +// row has not been derived yet. +// - Off-wrist and charging spans are passed to the detector from the strap's +// own WRIST_OFF/WRIST_ON and CHARGING_ON/OFF events. These were decoded +// and persisted to `band_events` all along and never used; a band on a +// table or charger is motionless and is the dominant nap false positive. +// - Absent ≠ zero: when nap detection cannot judge a day, `nap_min` is left +// UNWRITTEN, and the sleep-need credit reads TODAY only. It previously +// fell back through `_lastNum` to YESTERDAY's nap minutes (§3.3). +// - `sleep_coach.nap_credit_min` exposes the credit that was subtracted, so +// the coach card can show it instead of silently shrinking the ring. +// +// Days re-derive so naps, nap_min, sleep_periods and sleep need are rebuilt. +const int kAlgoVersion = 55; // Fold idempotency, the minimum-nights warm-up, and legacy-payload handling // all live in SleepProfilePolicy (pure, unit-tested) — see @@ -2148,6 +2224,17 @@ class DerivationEngine { final stepCalib = await LocalDb.getStepCalibration(); final savedSessions = await LocalDb.sessionsInRange(dayLo, dayHi); + // Off-wrist / charging spans over the NAP window (which runs past this + // day's end), read here because the isolate has no DB handle. These are + // the strap's own reports: a band on a table or a charger is perfectly + // still and otherwise reads as deep rest to a motion-based detector. + final napLo = + day.napSub.length == 0 ? dayLo : day.napSub.tsSec.first; + final napHi = + day.napSub.length == 0 ? dayHi : day.napSub.tsSec.last + 60; + final wristOffSpans = await LocalDb.wristOffSpans(napLo, napHi); + final chargingSpans = await LocalDb.chargingSpans(napLo, napHi); + // PERSONAL ambulatory floor, from days STRICTLY BEFORE this one (the same // self-exclusion every other baseline uses — a day must not help set the // threshold it is then scored against). Anchoring on trailing days is the @@ -2174,6 +2261,15 @@ class DerivationEngine { dynFloorG: dynFloorG, dynHistoryDays: dynHistory.length, savedSessions: savedSessions, + wristOffSpans: wristOffSpans, + chargingSpans: chargingSpans, + mainTstMin: (scMap?['tst_min'] as num?)?.round(), + // The scalar is a PERCENT (onehz_pipeline.dart:914); the period + // contract and the card both want 0..1, the same normalization + // `_daySleep` does on read. + mainEfficiency: (scMap?['efficiency'] as num?) == null + ? null + : (scMap!['efficiency'] as num).toDouble() / 100.0, date: day.date, dayEndSec: day.endSec, dataNowSec: dataNowSec, @@ -2628,6 +2724,11 @@ class DerivationEngine { if (row['day_id'] == today && (row['finalized'] as num?) != 1) { rec['unsettled'] = true; } + // Explicit identity for TODAY-scoped reads. `unsettled` cannot serve + // this purpose — it is only set while today is unfinalized. Without a + // flag, a today-scoped consumer can only take the LAST record + // positionally, which on a day with no derived row is YESTERDAY's. + if (row['day_id'] == today) rec['is_today'] = true; days.add(rec); } return (days, jsonEncode({'algo_version': kAlgoVersion, 'days': days})); @@ -3611,99 +3712,95 @@ class DerivationEngine { }; } - /// Sleep periods: the main sleep + any NAPS (still, on-wrist minute-runs ≥20 - /// min OUTSIDE the main window). Conservative — naps need sustained stillness. + /// Sleep periods: the main sleep plus the naps [_attachNaps] already found. + /// + /// This used to run its OWN nap detector — 20-min runs of still, on-wrist + /// minutes — in parallel with `detectNaps`. Two detectors, two answers, two + /// screens: `payload.json` shipped a 21-minute period here on the very day + /// `naps` reported `count: 0`. One source per concern (AGENTS §3.8), so the + /// naps are now passed in rather than re-derived. + /// + /// Every period speaks the SAME contract the Sleep-periods screen reads + /// (`onset_ts`/`wake_ts`/`duration_min`/`efficiency`/`confidence`), and + /// `duration_min` is minutes ASLEEP for both the main sleep and naps — they + /// were previously different units under one label, then summed. static Map _sleepPeriods( - Substrate s, int onsetSec, - int offsetSec, { - int? attributionEndSec, + int offsetSec, + List> naps, { + int? mainTstMin, + double? mainEfficiency, }) { final periods = >[]; + // Null-if-any-component-unknown. Summing a null duration as 0 would print a + // confident total that is short by exactly the part we could not measure — + // and the hero tile divides it by need, so the "% of need" arc understates + // too. An unknown component makes the SUM unknown. var totalAsleep = 0; + var totalKnown = true; if (offsetSec > onsetSec) { - final mainMin = (offsetSec - onsetSec) ~/ 60; periods.add({ 'is_main': true, - 'start': onsetSec, - 'end': offsetSec, - 'asleep_min': mainMin, + 'onset_ts': onsetSec, + 'wake_ts': offsetSec, + // Null when staging did not produce a TST. The screen renders "—"; + // substituting the in-bed span would silently relabel time in bed as + // time asleep, which is the same conflation this change removes. + 'duration_min': mainTstMin, + 'in_bed_min': (offsetSec - onsetSec) ~/ 60, + 'efficiency': ?mainEfficiency, + // No hypnogram here on purpose: it lives in the isolate-1 bundle's + // `series.hypnogram`, which this isolate does not receive. The read + // seam attaches it (see `_daySleep`), where the whole bundle is in + // hand. Passing it from here would only ever have written null. }); - totalAsleep += mainMin; - } - final n = s.length; - if (n >= 60) { - const moveDeg = 5.0; - // Per-minute "still + on-wrist", excluding the main window. - final still = {}; // minute → still - final mTot = {}, mMove = {}, mOn = {}; - for (var i = 1; i < n; i++) { - final t = s.tsSec[i]; - if (offsetSec > onsetSec && t >= onsetSec && t < offsetSec) continue; - final m = t ~/ 60; - mTot[m] = (mTot[m] ?? 0) + 1; - if (s.hr[i] > 0) mOn[m] = (mOn[m] ?? 0) + 1; - final d = - (ana.zAngle(s.ax[i], s.ay[i], s.az[i]) - - ana.zAngle(s.ax[i - 1], s.ay[i - 1], s.az[i - 1])) - .abs(); - if (d > moveDeg) mMove[m] = (mMove[m] ?? 0) + 1; - } - final keys = mTot.keys.toList()..sort(); - for (final m in keys) { - final tot = mTot[m] ?? 1; - still[m] = (mMove[m] ?? 0) / tot < 0.10 && (mOn[m] ?? 0) / tot > 0.5; + if (mainTstMin != null) { + totalAsleep += mainTstMin; + } else { + totalKnown = false; } - // Runs of ≥20 contiguous still minutes → a nap. - var i = 0; - while (i < keys.length) { - if (still[keys[i]] != true) { - i++; - continue; - } - var j = i; - while (j < keys.length && - still[keys[j]] == true && - keys[j] - keys[i] == j - i) { - j++; - } - final lenMin = j - i; - final start = keys[i] * 60; - // A run STARTING at/after the real day boundary belongs to tomorrow's - // own (unbuffered) window — only count runs that started today. - final startsToday = - attributionEndSec == null || start < attributionEndSec; - if (lenMin >= 20 && startsToday) { - final end = keys[j - 1] * 60 + 60; - periods.add({ - 'is_main': false, - 'start': start, - 'end': end, - 'asleep_min': lenMin, - }); - totalAsleep += lenMin; - } - i = j; + } + for (final nap in naps) { + periods.add(nap); + final d = (nap['duration_min'] as num?)?.toInt(); + if (d != null) { + totalAsleep += d; + } else { + totalKnown = false; } } - return {'periods': periods, 'total_asleep_min': totalAsleep}; + return { + 'periods': periods, + 'total_asleep_min': totalKnown ? totalAsleep : null, + }; } - /// Principled daytime naps via the analytics `detectNaps` (van Hees immobility - /// + HR-dip over the WAKE span, the main nocturnal window carved out). Writes a - /// rich `naps` block (per-nap start/end epoch-sec + duration + confidence) and a - /// `nap_min` scalar (total nap minutes) used by the Sleep Coach + Timeline. - static void _attachNaps( + /// Daytime naps via the analytics `detectNaps` — the ONLY nap source. + /// + /// Writes the `naps` block (per-nap epoch bounds + TST/TIB + confidence) and + /// the `nap_min` scalar (total minutes ASLEEP) used by the Sleep Coach and + /// Timeline, and returns period maps for [_sleepPeriods] so the Sleep-periods + /// screen lists exactly the same naps the Timeline draws. There used to be a + /// second, coarser nap notion in `_sleepPeriods` built from 20-min stillness + /// runs; the two disagreed on real days (`payload.json` shipped a 21-min + /// period alongside `naps.count: 0`) and fed two different screens. + /// + /// ABSENT is not ZERO. When the detector cannot judge the day, `nap_min` is + /// left UNWRITTEN rather than set to 0 — a written 0 is a claim that there + /// were no naps, and it would also be picked up as a real value downstream. + static List> _attachNaps( Map bundle, Map? scMap, Substrate s, int onsetSec, int offsetSec, { int? attributionEndSec, + List> wristOff = const [], + List> charging = const [], }) { try { final n = s.length; - if (n < 60) return; + if (n < 60) return const []; final accel = [ for (var i = 0; i < n; i++) ana.AccelSample(s.tsSec[i] * 1000.0, s.ax[i], s.ay[i], s.az[i]), @@ -3719,22 +3816,46 @@ class DerivationEngine { } if (lo >= 0 && hi > lo) main = ana.SleepWindowSpan(lo, hi); } - final m = ana.detectNaps(accel, hr, mainSleep: main); + final m = ana.detectNaps( + accel, + hr, + mainSleep: main, + wristOff: wristOff, + exclude: charging, + ); + + if (!m.present) { + bundle['naps'] = { + 'value': null, + 'count': null, + 'confidence': 0, + 'tier': m.tier, + 'inputs_used': m.inputs_used, + 'note': m.note, + }; + return const []; + } + final t0 = s.tsSec.first; // A nap STARTING at/after the real day boundary is tomorrow's — its own // (unbuffered) window finds it independently, so keeping it here too // would double-count it. - final naps = (m.value ?? const []).where((nap) { + final naps = m.value!.where((nap) { if (attributionEndSec == null) return true; return t0 + nap.startSec < attributionEndSec; }).toList(); + bundle['naps'] = { 'value': [ for (final nap in naps) { 'start': t0 + nap.startSec, 'end': t0 + nap.endSec, - 'duration_min': (nap.durationSec / 60).round(), + // Minutes ASLEEP. `duration_min` kept as the asleep figure so + // existing readers do not silently switch to in-bed minutes. + 'duration_min': (nap.tstSec / 60).round(), + 'in_bed_min': (nap.tibSec / 60).round(), + 'efficiency': nap.efficiency, 'confidence': nap.confidence, }, ], @@ -3744,10 +3865,32 @@ class DerivationEngine { 'inputs_used': m.inputs_used, 'note': m.note, }; - final napMin = naps.fold(0, (a, nap) => a + (nap.durationSec ~/ 60)); + + // TST, never TIB. Crediting in-bed minutes against sleep need + // over-credits every nap by its awake time and always errs toward + // recommending LESS sleep than the user needs. + // Rounded, matching the two display paths exactly. Truncating here while + // the cards round made the credit disagree with the sum of the minutes + // shown — up to a minute per nap, in a number the user can add up. + final napMin = + naps.fold(0, (a, nap) => a + (nap.tstSec / 60).round()); scMap?['nap_min'] = napMin.toDouble(); + + return [ + for (final nap in naps) + { + 'is_main': false, + 'onset_ts': t0 + nap.startSec, + 'wake_ts': t0 + nap.endSec, + 'duration_min': (nap.tstSec / 60).round(), + 'in_bed_min': (nap.tibSec / 60).round(), + 'efficiency': nap.efficiency, + 'confidence': nap.confidence, + }, + ]; } catch (e) { if (kDebugMode) debugPrint('[derive] naps FAILED/skipped: $e'); + return const []; } } @@ -3972,10 +4115,25 @@ class DerivationEngine { // actually STARTS in that borrowed buffer belongs to tomorrow (which sees // it in its own regular window), so both helpers drop anything starting // at/after dayEndSec to avoid double-counting. - bundlePatch['sleep_periods'] = - _sleepPeriods(inp.napSub, onset, offset, attributionEndSec: inp.dayEndSec); - _attachNaps(bundlePatch, scMap, inp.napSub, onset, offset, - attributionEndSec: inp.dayEndSec); + // Naps FIRST — `_sleepPeriods` lists exactly these, so the Timeline bands + // and the Sleep-periods cards can never disagree again. + final napPeriods = _attachNaps( + bundlePatch, + scMap, + inp.napSub, + onset, + offset, + attributionEndSec: inp.dayEndSec, + wristOff: inp.wristOffSpans, + charging: inp.chargingSpans, + ); + bundlePatch['sleep_periods'] = _sleepPeriods( + onset, + offset, + napPeriods, + mainTstMin: inp.mainTstMin, + mainEfficiency: inp.mainEfficiency, + ); // Overrides wake's activity_curve (same value, computed once here). bundlePatch['activity_curve'] = _activityCurve(daySub); bundlePatch['detected_workouts'] = const >[]; @@ -4364,6 +4522,24 @@ class _DayBlocksInput { /// How many trailing days backed [dynFloorG] — only for the cold-start note. final int dynHistoryDays; final List> savedSessions; + + /// Strap-reported off-wrist spans ([startSec, endSec]) over the nap window. + /// A band on a table is motionless and reads as deep rest — this is the + /// dominant nap false positive, and the strap already tells us about it. + final List> wristOffSpans; + + /// Strap-reported charging spans — off-wrist by definition, and motionless. + final List> chargingSpans; + + /// Main-sleep TST (minutes) and efficiency (0..1) from ISOLATE 1. + /// + /// Carried explicitly because `_computeDayBlocks` builds its own fresh + /// `scMap` seeded with `rhr` alone — reading `scMap['tst_min']` in there + /// silently yields null forever, which is how the main sleep period came to + /// report "—" for its duration. + final int? mainTstMin; + final double? mainEfficiency; + final String date; final int dayEndSec; final int dataNowSec; @@ -4382,6 +4558,10 @@ class _DayBlocksInput { required this.dynFloorG, required this.dynHistoryDays, required this.savedSessions, + required this.wristOffSpans, + required this.chargingSpans, + required this.mainTstMin, + required this.mainEfficiency, required this.date, required this.dayEndSec, required this.dataNowSec, diff --git a/lib/data/db.dart b/lib/data/db.dart index d76a8426..df450624 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -834,6 +834,82 @@ class LocalDb { ]; } + /// Spans ([startSec, endSec]) in [loSec, hiSec) during which the band was NOT + /// on the wrist, from the strap's own WRIST_OFF/WRIST_ON events. + /// + /// A band sitting on a table is PERFECTLY still and reads as deep rest to any + /// motion-based detector — it is the dominant nap false positive. The strap + /// already tells us; these events have been decoded and persisted all along, + /// and `AdvancedSleepStager.detectSleep` has always accepted a `wristOff` + /// argument, but nothing ever supplied one. + /// + /// State is carried in from BEFORE [loSec] so a window that opens mid-removal + /// is still covered, and an unterminated removal extends to [hiSec] rather + /// than being dropped (absent evidence of return is not evidence of return). + static Future>> wristOffSpans(int loSec, int hiSec) => + _toggleSpans( + loSec, + hiSec, + onId: proto.EventId.wristOn, + offId: proto.EventId.wristOff, + ); + + /// Spans ([startSec, endSec]) in [loSec, hiSec) during which the band was on + /// the charger — off-wrist by definition, and motionless. + static Future>> chargingSpans(int loSec, int hiSec) => + _toggleSpans( + loSec, + hiSec, + onId: proto.EventId.chargingOff, + offId: proto.EventId.chargingOn, + ); + + /// Build "state active" spans from a pair of toggle events, clipped to + /// [loSec, hiSec). [offId] opens a span; [onId] closes it. + static Future>> _toggleSpans( + int loSec, + int hiSec, { + required int onId, + required int offId, + }) async { + if (hiSec <= loSec) return const []; + final db = await instance; + // One row before the window establishes the state we open in. + final prior = await db.query( + 'band_events', + columns: ['ts', 'event_id'], + where: 'ts < ? AND event_id IN (?, ?)', + whereArgs: [loSec, onId, offId], + orderBy: 'ts DESC', + limit: 1, + ); + final rows = await db.query( + 'band_events', + columns: ['ts', 'event_id'], + where: 'ts >= ? AND ts < ? AND event_id IN (?, ?)', + whereArgs: [loSec, hiSec, onId, offId], + orderBy: 'ts ASC', + ); + + final spans = >[]; + int? openAt = + (prior.isNotEmpty && (prior.first['event_id'] as num).toInt() == offId) + ? loSec + : null; + for (final r in rows) { + final ts = (r['ts'] as num).toInt(); + final id = (r['event_id'] as num).toInt(); + if (id == offId) { + openAt ??= ts; + } else if (openAt != null) { + if (ts > openAt) spans.add([openAt, ts]); + openAt = null; + } + } + if (openAt != null && hiSec > openAt) spans.add([openAt, hiSec]); + return spans; + } + /// Read a sync-cursor value (null if unset). static Future getCursor(String name) async { final db = await instance; diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index a30e09fe..002006aa 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -605,8 +605,17 @@ class LocalRepositoryImpl extends LocalRepository { 'debt_min': ((480 - (tst / 60)).clamp(0, 480)).round(), 'regularity': null, // needs ≥several nights (honest null → "Need N nights") - // Sleep periods (main + naps) for the periods screen. - 'periods': (b['sleep_periods'] as Map?)?['periods'] ?? const [], + // 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. + 'periods': _periodsWithMainStages(b, { + 'light_min': min('light_sec'), + 'deep_min': min('deep_sec'), + 'rem_min': min('rem_sec'), + 'nrem_min': min('nrem_sec'), + }), 'total_asleep_min': (b['sleep_periods'] as Map?)?['total_asleep_min'], // Sleep cycles — Rosenblum 2024 "fractal cycles" (HRV-adapted): peak-to- // peak of the smoothed per-minute RMSSD series (REM peaks / NREM troughs). @@ -625,6 +634,38 @@ class LocalRepositoryImpl extends LocalRepository { }; } + /// The persisted sleep periods with the MAIN period's hypnogram and stage + /// minutes attached. + /// + /// Naps are returned untouched: they have no stages, and inventing an empty + /// stage map would make the card draw a stage bar for sleep we never + /// classified. Absent stage minutes are dropped rather than zeroed for the + /// same reason — `StageBars` renders 0 as an invisible gap, which reads as + /// "no deep sleep" instead of "not measured". + List> _periodsWithMainStages( + Map b, + Map stageMin, + ) { + final raw = (b['sleep_periods'] as Map?)?['periods']; + if (raw is! List) return const []; + final hypno = _hypnoPoints(b); + final stages = { + for (final e in stageMin.entries) + if (e.value != null) e.key: e.value, + }; + return [ + for (final p in raw.whereType()) + if (p['is_main'] != true) + p.cast() + else + { + ...p.cast(), + if (hypno.isNotEmpty) 'hypnogram': hypno, + if (stages.isNotEmpty) 'stages': stages, + }, + ]; + } + /// Mean completed-cycle length (min), or null when no cycles. num? _cyclesMeanMin(Map b) { final cyc = _sub(b, 'sleep')?['cycles']; diff --git a/lib/ui/insights/coach_cards.dart b/lib/ui/insights/coach_cards.dart index 7260c0bc..a9a1d864 100644 --- a/lib/ui/insights/coach_cards.dart +++ b/lib/ui/insights/coach_cards.dart @@ -27,6 +27,10 @@ String _hhmm(num minOfDay) { String _dur(num sec) { final total = sec.round(); final h = total ~/ 3600, m = (total % 3600) ~/ 60; + // Sub-hour durations read as "25m", not "0h 25m". The existing callers all + // pass a 6-11 h sleep need where h > 0, so this only affects the new + // sub-hour caller (the nap credit). + if (h == 0) return '${m}m'; return m == 0 ? '${h}h' : '${h}h ${m}m'; } @@ -163,6 +167,15 @@ class _SleepCoachCardState extends State { ? 'Tonight you need ${_dur(needSec)}' : 'Tonight you need ${_dur(needSec)} · $pct% of need'; + // Naps are already SUBTRACTED from `need_sec`. Showing the adjustment is + // the difference between a number the user can reason about and one that + // silently shrank — a nap also inflates "% of need" through the same + // subtraction, so an unexplained credit moves both figures at once. + final napCreditMin = (_coach?['nap_credit_min'] as num?)?.round(); + final napLine = (napCreditMin != null && napCreditMin > 0) + ? '−${_dur(napCreditMin * 60)} credited from today\'s nap' + : null; + return ProCard( child: Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ Row(children: [ @@ -177,6 +190,10 @@ class _SleepCoachCardState extends State { // behind it — with bedtime/wake both absent (need computed, but the // schedule recommendation isn't yet) a Disclosure here would show // "Bedtime & alarm" and expand into an empty column. + if (napLine != null) ...[ + Text(napLine, style: AppText.caption), + const SizedBox(height: Sp.x2), + ], if (bedMin == null && wakeMin == null) _needSummary(needSec, pct, accent) else diff --git a/lib/ui/sleep/sleep_periods_screen.dart b/lib/ui/sleep/sleep_periods_screen.dart index b667850a..bee05794 100644 --- a/lib/ui/sleep/sleep_periods_screen.dart +++ b/lib/ui/sleep/sleep_periods_screen.dart @@ -72,7 +72,10 @@ class _SleepPeriodsScreenState extends State { } int get _needMin => (_num(_data['need_min'])?.toInt()) ?? 480; - int get _totalAsleep => (_num(_data['total_asleep_min'])?.toInt()) ?? 0; + + /// Null when any listed period's asleep minutes are unknown — the total is + /// then unknown too, not zero. + int? get _totalAsleep => _num(_data['total_asleep_min'])?.toInt(); bool get _beta => _data['stages_beta'] == true; @override @@ -130,9 +133,10 @@ class _SleepPeriodsScreenState extends State { // Day hero: total asleep across all periods vs need — the board's ink tile. Widget _summary() { final n = _periods.length; - final t = _needMin <= 0 + final total = _totalAsleep; + final t = (total == null || _needMin <= 0) ? double.nan - : (_totalAsleep / _needMin).clamp(0.0, 1.0).toDouble(); + : (total / _needMin).clamp(0.0, 1.0).toDouble(); return BentoTile( tone: BentoTone.ink, accent: DomainAccent.sleep, @@ -147,7 +151,7 @@ class _SleepPeriodsScreenState extends State { const TileHeader('Total sleep'), const SizedBox(height: Sp.x2), BigStat( - value: _hm(_totalAsleep), + value: total == null ? '—' : _hm(total), size: BigStatSize.xl, caption: 'need ${_hm(_needMin)} · $n sleep${n == 1 ? '' : 's'}', ), @@ -174,9 +178,12 @@ class _SleepPeriodsScreenState extends State { final isMain = p['is_main'] == true; final onset = _num(p['onset_ts'])?.toInt(); final wake = _num(p['wake_ts'])?.toInt(); - final dur = _num(p['duration_min'])?.toInt() ?? 0; + // Null, NOT 0. A period whose asleep minutes we don't have renders "—"; + // defaulting to 0 printed a confident "0m" on every real nap for as long + // as the producer and this screen disagreed about the key name. + final dur = _num(p['duration_min'])?.toInt(); final eff = _num(p['efficiency'])?.toDouble(); - final conf = _num(p['confidence'])?.toDouble() ?? 0; + final conf = _num(p['confidence'])?.toDouble(); final stages = (p['stages'] is Map) ? (p['stages'] as Map).cast() : null; @@ -199,12 +206,20 @@ class _SleepPeriodsScreenState extends State { trailing: _beta ? const Tag('est') : null, ), ), - ConfDot(conf), + // No dot at all when confidence is unknown — a ConfDot(0) is a + // red "we are sure this is bad" dot, which is a claim. + if (conf != null) ConfDot(conf), InfoDot( title: isMain ? 'Main sleep' : 'Nap', - body: - 'Stages are a wrist estimate from heart rate + motion (no ' - 'EEG). The dot shows detection confidence for this window.', + body: isMain + ? 'Stages are a wrist estimate from heart rate + motion ' + '(no EEG).' + : 'Detected from wrist stillness plus a drop in heart ' + 'rate against your awake daytime baseline. Time ' + 'asleep excludes brief wake-ups. No stage breakdown: ' + 'a short nap has no complete sleep cycle, and daytime ' + 'heart rate is too intermittent to split one. The dot ' + 'shows detection confidence.', ), ], ), @@ -214,7 +229,7 @@ class _SleepPeriodsScreenState extends State { children: [ Expanded( child: BigStat( - value: _hm(dur), + value: dur == null ? '—' : _hm(dur), caption: (onset != null && wake != null) ? '${_clock(onset)} – ${_clock(wake)}' : null, diff --git a/test/nap_credit_test.dart b/test/nap_credit_test.dart new file mode 100644 index 00000000..11ee9981 --- /dev/null +++ b/test/nap_credit_test.dart @@ -0,0 +1,140 @@ +// HONESTY REGRESSION — nap credit against tonight's sleep need. +// +// Naps are subtracted 1:1 from sleep need. Two bugs made that subtraction +// wrong in the same direction — always recommending LESS sleep than the user +// needs — and both were invisible, because the credit was applied inside +// `need_sec` with nothing surfacing it: +// +// 1. `nap_min` carried the nap's IN-BED span rather than time ASLEEP, so a +// 2 h lie-down at 70% efficiency credited 120 min instead of 84. +// 2. Today's credit was read with `_lastNum`, which walks BACKWARD through +// the day records and returns the last non-null. On any day where nap +// detection abstained, that silently credited YESTERDAY's naps. +// +// This file pins (2) and the new disclosure field. (1) is pinned in the +// analytics package, where TST and in-bed are now separate fields. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/compute/crossday_pipeline.dart'; + +/// Minimal oldest-first day series with the fields sleep need actually reads. +/// +/// [todayDerived] false models the common real case where today has no derived +/// row yet, so the most recent record in the list is YESTERDAY. +List> _days( + int n, { + double? napMinToday, + bool todayDerived = true, +}) { + final out = >[]; + var dt = DateTime(2024, 1, 1); + for (var i = 0; i < n; i++) { + final last = i == n - 1; + out.add({ + // Stamped by _refreshCrossDayInputArtifact for today's row only. + if (last && todayDerived) 'is_today': true, + 'date': '${dt.year.toString().padLeft(4, '0')}-' + '${dt.month.toString().padLeft(2, '0')}-' + '${dt.day.toString().padLeft(2, '0')}', + 'rhr': 55.0, + 'rmssd': 45.0, + 'readiness': 70.0, + 'onset_sec': 23 * 3600, + 'wake_sec': 31 * 3600, + 'tst_min': 450, + 'strain': 8.0, + // Every day EXCEPT today reports a big nap. Today's is caller-controlled. + if (!last) 'nap_min': 90.0, + if (last && napMinToday != null) 'nap_min': napMinToday, + }); + dt = dt.add(const Duration(days: 1)); + } + return out; +} + +double _needSec(Map bundle) { + final coach = bundle['sleep_coach'] as Map; + final need = (coach['need'] as Map)['value'] as Map; + return (need['need_sec'] as num).toDouble(); +} + +Object? _napCredit(Map bundle) => + ((bundle['sleep_coach'] as Map)['nap_credit_min']); + +void main() { + const profile = {}; + + group('sleep need — nap credit is TODAY-scoped', () { + test("a day with no nap reading is NOT credited yesterday's nap", () { + // Today abstained; the 6 days before it each report a 90-minute nap. + final noReading = buildCrossDayBundle(_days(7), profile); + // Same series, but today explicitly reports zero nap minutes. + final explicitZero = + buildCrossDayBundle(_days(7, napMinToday: 0), profile); + + expect( + _needSec(noReading), + _needSec(explicitZero), + reason: 'an absent nap reading must credit nothing — reaching back a ' + 'day for a number is imputation, and it shortens the ' + 'recommendation by a nap the user did not take today', + ); + }); + + test('a real nap today IS credited, minute for minute', () { + final without = buildCrossDayBundle(_days(7, napMinToday: 0), profile); + final with45 = buildCrossDayBundle(_days(7, napMinToday: 45), profile); + + expect( + _needSec(without) - _needSec(with45), + closeTo(45 * 60, 1e-6), + reason: '45 min asleep should remove exactly 45 min of need', + ); + }); + }); + + group('sleep need — the credit is disclosed, not silent', () { + test('nap_credit_min reports the minutes that were subtracted', () { + final b = buildCrossDayBundle(_days(7, napMinToday: 45), profile); + expect(_napCredit(b), 45); + }); + + test('nap_credit_min is null when today produced no nap reading', () { + final b = buildCrossDayBundle(_days(7), profile); + expect(_napCredit(b), isNull, + reason: 'null and a confident 0 are different claims; the card ' + 'must not render "−0m" for "we do not know"'); + }); + + test('the disclosed credit is what was APPLIED, not the raw nap minutes', + () { + // sleepNeed clamps to a 6 h floor AFTER subtracting, so an enormous nap + // credit is only partly realized. Disclosing the raw minutes would state + // a reduction the published need never took. + final b = buildCrossDayBundle(_days(7, napMinToday: 600), profile); + final credit = (_napCredit(b) as num).toDouble(); + final applied = + _needSec(buildCrossDayBundle(_days(7, napMinToday: 0), profile)) - + _needSec(b); + + expect(credit * 60, closeTo(applied, 60)); + expect(credit, lessThan(600), + reason: '10 h of nap cannot remove 10 h of need — the floor binds'); + }); + }); + + group('sleep need — today must be identified, not assumed positional', () { + test("with no derived row for today, yesterday's nap is not credited", () { + // The most recent record in the list is YESTERDAY, and it reports a + // 90-minute nap. Reading days.last positionally would credit it. + final noToday = + buildCrossDayBundle(_days(7, todayDerived: false), profile); + final explicitZero = + buildCrossDayBundle(_days(7, napMinToday: 0), profile); + + expect(_needSec(noToday), _needSec(explicitZero), + reason: 'no row for today means no nap reading for today'); + expect(_napCredit(noToday), isNull); + }); + }); +} From 60fcc5a61280d5dbce3a59d1f9518387d5f4ecda Mon Sep 17 00:00:00 2001 From: SATHVIK SVS Date: Tue, 4 Aug 2026 21:49:07 +0530 Subject: [PATCH 2/6] sleep need: read TODAY's strain, and disclose the bonus it adds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v55 fixed `nap_min`'s cross-day read and left the other today-scoped input to `sleepNeed` two lines above it still going through `_lastNum`, which walks BACKWARD through the oldest-first day records and returns the last non-null. On any day whose strain compute abstained, tonight's strain bonus was therefore built from an EARLIER day's workout — the identical §3.3 imputation, in the identical function. Measured on a 7-day fixture: a carried strain of 18 inflated `need_sec` by 2314 s (38.6 min). The direction is the OPPOSITE of the nap bug, and that difference is the whole reason this needs more than a one-word substitution. Naps are SUBTRACTED and strain is ADDED, so while both inputs floor at 0, that floor is an upper bound on need for naps and a LOWER bound for strain. Abstaining to 0 strain recommends up to 45 min LESS sleep — it is NOT the cautious direction, and the "no credit is the safe direction" reasoning that justified the nap fix does not transfer. It is still correct, on two other grounds: - Carrying yesterday forward is not a safety margin either. It inflates need only when yesterday happened to be harder than today and deflates it when yesterday was a rest day — noise around the true value, not a conservative bound, and forbidden regardless. - Strain is a same-day ACCUMULATING quantity that starts at 0 and only rises. Before today logs anything, 0 is where it genuinely sits, not a substituted default. Making `need` abstain entirely was rejected: there is no `is_today` row until today derives, so the whole Sleep Coach card — need, performance, bedtime, wake — would blank every morning, and a false empty state is itself a §4.1 bug pattern. Because 0 is not the cautious direction, it is not allowed to be silent: - `sleep_coach.strain_bonus_min` reports the minutes the bonus ACTUALLY added, measured like `nap_credit_min` (re-run with strain zeroed, then diff), so the [6 h, 11 h] clamp cannot make the card claim an increase `need_sec` never took. - It is NULL, never 0, when today produced no strain reading. A confident 0 says "you rested"; null says "we could not measure today's strain, so tonight's need is short by up to 45 min". Collapsing those would re-hide exactly what the today-scoping fix exposed. - The Sleep Coach card renders the applied bonus via `strainBonusCaption`, mirroring the nap credit. It stays SILENT on null, matching that precedent — surfacing "today's strain was not measured" is a product decision, and the bundle now carries the distinction for whoever takes it. 12 tests, each watched fail first. The absent-reading test asserts the KEY is present before asserting the value is null, so it cannot pass against a bundle that never emitted the field — that guard fired during the red run. One test pins that the strain-18 bonus is fully realized for the default fixture, so the regression above cannot pass because the clamp flattened everything. nap_credit_test's fixture stamped `strain` on every row including today's. Under `_lastNum` that cancelled between the two series it compares; once strain is today-scoped it no longer does, so the fixture was coupling an unrelated input into the nap assertions. Removed — the strain path has its own file now. kAlgoVersion 55 -> 56 so affected days re-derive; `need_sec` changes and `day_result` rows are immutable per version. --- lib/compute/crossday_pipeline.dart | 51 ++++++- lib/compute/derivation_engine.dart | 34 ++++- lib/ui/insights/coach_cards.dart | 29 ++++ test/nap_credit_test.dart | 6 +- test/strain_bonus_test.dart | 217 +++++++++++++++++++++++++++++ 5 files changed, 334 insertions(+), 3 deletions(-) create mode 100644 test/strain_bonus_test.dart diff --git a/lib/compute/crossday_pipeline.dart b/lib/compute/crossday_pipeline.dart index c467050e..481eaad0 100644 --- a/lib/compute/crossday_pipeline.dart +++ b/lib/compute/crossday_pipeline.dart @@ -194,7 +194,28 @@ Map buildCrossDayBundle( final baselineNeedSec = ((osdH ?? 8.0).clamp(7.0, 9.5)) * 3600.0; final debtSec = (sleepDebt.present ? (sleepDebt.value!.debtHours ?? 0.0) : 0.0) * 3600.0; - final todayStrain = _lastNum(days, 'strain') ?? 0.0; + // TODAY's strain only. `_lastNum` walked backward to the last non-null, so a + // day whose strain compute abstained built tonight's bonus out of an EARLIER + // day's workout — imputation (AGENTS §3.3), and invisible, since the number + // lands inside `need_sec` with nothing surfacing it. + // + // Unlike the nap credit below, 0 here is NOT the cautious direction: strain is + // ADDED (up to 45 min via sleepNeed's strainBonusSec), so abstaining removes + // sleep from the recommendation rather than adding it. It is still right, on + // two grounds that are not "it's safe": + // - Carrying yesterday forward is not a safety margin either. It inflates + // need only when yesterday happened to be harder than today, and deflates + // it when yesterday was a rest day — noise around the true value, not a + // conservative bound, and forbidden regardless. + // - Strain is a same-day ACCUMULATING quantity that starts at 0 and only + // rises. Before today logs anything, 0 is where it genuinely sits, not a + // substituted default. The bonus grows as the day's real strain arrives. + // Because that direction is not the cautious one, the substitution is not + // allowed to be silent: `strain_bonus_min` below reports what the bonus + // actually added, and stays NULL (never 0) when today produced no reading — + // which is the case where up to 45 min of need went missing. + final todayStrainNum = _todayNum(days, 'strain'); + final todayStrain = todayStrainNum ?? 0.0; // TODAY's naps only, and minutes ASLEEP (the analytics detector reports TST // and in-bed separately now). No reading means NO credit — that leaves the // recommendation slightly high, which is the safe direction; reaching back a @@ -222,6 +243,28 @@ Map buildCrossDayBundle( !needNoNap.present) ? null : ((needNoNap.value!.needSec - need.value!.needSec) / 60).round(); + // What the strain bonus ACTUALLY added, measured the same way `nap_credit_min` + // measures the nap: re-run at the real operating point with the strain zeroed + // and diff. The [6 h, 11 h] clamp applies AFTER adding, so against a high + // baseline + debt the bonus is only partly realized — disclosing the raw + // (strain/21)*45 would state an increase `need_sec` never took. + // + // Null when today produced no strain reading. That is the ONE case that + // matters most here: a confident 0 says "you rested today", while null says + // "we could not measure today's strain, so tonight's need is short by up to + // 45 min". Collapsing the two would re-hide exactly what the today-scoping + // fix above exposed. + final needNoStrain = ana.sleepNeed( + baselineNeedSec: baselineNeedSec, + sleepDebtSec: debtSec < 0 ? 0.0 : debtSec, + dayStrain: 0.0, + napCreditSec: todayNapSec, + ); + final appliedStrainBonusMin = (todayStrainNum == null || + !need.present || + !needNoStrain.present) + ? null + : ((need.value!.needSec - needNoStrain.value!.needSec) / 60).round(); // last night's TST (sec) for performance. final lastTstMin = _lastNum(days, 'tst_min'); final perf = (need.present && lastTstMin != null) @@ -356,6 +399,12 @@ Map buildCrossDayBundle( // no nap reading, which is different from a confident zero: the UI must // not render "−0m" for "we do not know". 'nap_credit_min': appliedNapCreditMin, + // Minutes the strain bonus ACTUALLY added to `need`, same measure-what- + // was-applied rule as `nap_credit_min` (the clamp can swallow part of it). + // Null means today produced no strain reading — NOT a rest day. The UI + // must not render "+0m" for "we do not know", and the missing bonus is + // worth up to 45 min of need. + 'strain_bonus_min': appliedStrainBonusMin, 'performance': perf.toJson((v) => v.toJson()), 'bedtime': bedtime.toJson((v) => v.toJson()), 'wake': wakeRec.toJson((v) => v.toJson()), diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index e0316ccd..95a31553 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -480,7 +480,39 @@ import 'substrate.dart'; // the coach card can show it instead of silently shrinking the ring. // // Days re-derive so naps, nap_min, sleep_periods and sleep need are rebuilt. -const int kAlgoVersion = 55; +// v56: the STRAIN half of the same today-scoping bug. v55 fixed `nap_min` but +// left `sleep_coach.need`'s other today-scoped input reading through +// `_lastNum`, so a day whose strain compute abstained built tonight's strain +// bonus out of an EARLIER day's workout — the identical §3.3 imputation, in the +// identical function, two lines apart. Measured on a 7-day fixture: a carried +// strain of 18 inflated `need_sec` by 2314 s (38.6 min) over a today-abstained +// day. Now `_todayNum`. +// +// Direction note, because it differs from v55 and the difference matters: naps +// are SUBTRACTED and strain is ADDED, so while both inputs floor at 0, that +// floor is an upper bound on need for naps and a LOWER bound for strain. +// Abstaining to 0 strain therefore recommends up to 45 min LESS sleep, not +// more. It is still correct — carrying yesterday forward is not a safety margin +// but noise around the true value (it inflates need only when yesterday +// happened to be harder than today), and strain is a same-day accumulating +// quantity that genuinely starts at 0 — but it is not the cautious direction. +// Because it is not, it is not allowed to be silent either: +// - `sleep_coach.strain_bonus_min` reports the minutes the bonus ACTUALLY +// added, measured like `nap_credit_min` (re-run with strain zeroed and +// diff), so the [6 h, 11 h] clamp cannot make the card claim an increase +// `need_sec` never took. +// - It is NULL, never 0, when today produced no strain reading. A confident 0 +// says "you rested"; null says "we could not measure today's strain, so +// tonight's need is short by up to 45 min". Collapsing those would re-hide +// exactly what the today-scoping fix exposed. +// - The Sleep Coach card renders the applied bonus as a "+Xm added for +// today's strain" line (`strainBonusCaption`), mirroring the nap credit. +// The card stays SILENT on null, matching the nap precedent — surfacing +// "today's strain was not measured" to the user is a product decision, and +// the bundle carries the distinction for whoever takes it. +// +// Days re-derive so sleep need, bedtime, wake and sleep performance are rebuilt. +const int kAlgoVersion = 56; // Fold idempotency, the minimum-nights warm-up, and legacy-payload handling // all live in SleepProfilePolicy (pure, unit-tested) — see diff --git a/lib/ui/insights/coach_cards.dart b/lib/ui/insights/coach_cards.dart index a9a1d864..029d61fc 100644 --- a/lib/ui/insights/coach_cards.dart +++ b/lib/ui/insights/coach_cards.dart @@ -34,6 +34,26 @@ String _dur(num sec) { return m == 0 ? '${h}h' : '${h}h ${m}m'; } +/// Caption for the strain bonus ALREADY folded into `sleep_coach.need_sec`. +/// +/// [strainBonusMin] is `sleep_coach.strain_bonus_min` — the minutes the bonus +/// actually ADDED, after `sleepNeed`'s 11 h ceiling took its cut, not the raw +/// `(strain/21)*45`. +/// +/// Null caption (no line) in two different situations, which the BUNDLE keeps +/// apart even though the card does not: +/// - `null` — today produced no strain reading, so no bonus was applied and +/// tonight's need is short by up to 45 min. The card stays silent here to +/// match the nap credit; surfacing it is a product decision, and +/// `strain_bonus_min` carries the distinction for anything that wants it. +/// - `0` — a measured rest day, or a bonus the ceiling swallowed whole. +/// Nothing was added, so there is nothing to disclose; "+0m" would be noise. +String? strainBonusCaption(num? strainBonusMin) { + final m = strainBonusMin?.round(); + if (m == null || m <= 0) return null; + return '+${_dur(m * 60)} added for today\'s strain'; +} + Map? _val(Object? metric) { if (metric is! Map) return null; final v = metric['value']; @@ -176,6 +196,11 @@ class _SleepCoachCardState extends State { ? '−${_dur(napCreditMin * 60)} credited from today\'s nap' : null; + // Same reasoning in the other direction: today's strain is already ADDED to + // `need_sec` (up to 45 min), so an unexplained bonus moves both the need and + // "% of need" at once. See strainBonusCaption for what silence means here. + final strainLine = strainBonusCaption(_coach?['strain_bonus_min'] as num?); + return ProCard( child: Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ Row(children: [ @@ -194,6 +219,10 @@ class _SleepCoachCardState extends State { Text(napLine, style: AppText.caption), const SizedBox(height: Sp.x2), ], + if (strainLine != null) ...[ + Text(strainLine, style: AppText.caption), + const SizedBox(height: Sp.x2), + ], if (bedMin == null && wakeMin == null) _needSummary(needSec, pct, accent) else diff --git a/test/nap_credit_test.dart b/test/nap_credit_test.dart index 11ee9981..e9f4ab58 100644 --- a/test/nap_credit_test.dart +++ b/test/nap_credit_test.dart @@ -42,7 +42,11 @@ List> _days( 'onset_sec': 23 * 3600, 'wake_sec': 31 * 3600, 'tst_min': 450, - 'strain': 8.0, + // No `strain` anywhere. Strain is ALSO today-scoped, so stamping it on + // every row (as this fixture used to) made the strain bonus differ + // between a series with a today row and one without — coupling an + // unrelated input into the nap assertions below, which compare exactly + // those two shapes. The strain path is pinned in strain_bonus_test.dart. // Every day EXCEPT today reports a big nap. Today's is caller-controlled. if (!last) 'nap_min': 90.0, if (last && napMinToday != null) 'nap_min': napMinToday, diff --git a/test/strain_bonus_test.dart b/test/strain_bonus_test.dart new file mode 100644 index 00000000..2448ee90 --- /dev/null +++ b/test/strain_bonus_test.dart @@ -0,0 +1,217 @@ +// HONESTY REGRESSION — the strain bonus against tonight's sleep need. +// +// `sleepNeed` adds a strain bonus of `(strain/21) * 45 min` on top of baseline +// need + debt. Today's strain was read with `_lastNum`, which walks BACKWARD +// through the oldest-first day records and returns the last non-null value, so +// on any day where today's strain compute abstained the bonus was built from an +// EARLIER day's strain. That is imputation (AGENTS §3.3, the most-violated rule +// in the repo per §4.1) and it is invisible: the substituted number lands +// inside `need_sec` with nothing surfacing it. +// +// The direction here is the OPPOSITE of the nap bug pinned in +// nap_credit_test.dart, which is exactly why it needs its own pin. Naps are +// SUBTRACTED, so carrying one forward under-recommends sleep; strain is ADDED, +// so carrying it forward over-recommends. Neither direction is a safety +// margin — a rule that inflates need only when yesterday happened to be harder +// than today is noise, not caution. Both must read TODAY or abstain. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/compute/crossday_pipeline.dart'; +import 'package:openstrap_edge/ui/insights/coach_cards.dart'; + +/// Minimal oldest-first day series with the fields sleep need actually reads. +/// +/// Every record EXCEPT today's carries a heavy strain (18 of 21). Today's is +/// caller-controlled, so `strainToday: null` models the real case where today's +/// strain compute abstained. +/// +/// [todayDerived] false models the common real case where today has no derived +/// row yet, so the most recent record in the list is YESTERDAY — and that row +/// carries the heavy strain, so a positional `days.last` read would pick it up. +/// [weekdayTstMin] / [freeTstMin] split Mon–Fri from Sat/Sun so a test can push +/// baseline need + debt up against `sleepNeed`'s 11 h ceiling. Equal by default, +/// which yields zero debt and a 7.5 h baseline — comfortably mid-band. +List> _days( + int n, { + double? strainToday, + bool todayDerived = true, + int weekdayTstMin = 450, + int freeTstMin = 450, +}) { + final out = >[]; + var dt = DateTime(2024, 1, 1); + for (var i = 0; i < n; i++) { + final last = i == n - 1; + final isToday = last && todayDerived; + final isFree = + dt.weekday == DateTime.saturday || dt.weekday == DateTime.sunday; + out.add({ + // Stamped by _refreshCrossDayInputArtifact for today's row only. + if (isToday) 'is_today': true, + 'date': '${dt.year.toString().padLeft(4, '0')}-' + '${dt.month.toString().padLeft(2, '0')}-' + '${dt.day.toString().padLeft(2, '0')}', + 'rhr': 55.0, + 'rmssd': 45.0, + 'readiness': 70.0, + 'onset_sec': 23 * 3600, + 'wake_sec': 31 * 3600, + 'tst_min': isFree ? freeTstMin : weekdayTstMin, + // Every record that is not today reports heavy strain. + if (!isToday) 'strain': 18.0, + if (isToday && strainToday != null) 'strain': strainToday, + // No nap_min anywhere: the nap path is pinned in nap_credit_test.dart and + // is held constant here so `need_sec` moves only with strain. + }); + dt = dt.add(const Duration(days: 1)); + } + return out; +} + +double _needSec(Map bundle) { + final coach = bundle['sleep_coach'] as Map; + final need = (coach['need'] as Map)['value'] as Map; + return (need['need_sec'] as num).toDouble(); +} + +Object? _strainBonus(Map bundle) => + ((bundle['sleep_coach'] as Map)['strain_bonus_min']); + +bool _hasStrainBonusKey(Map bundle) => + (bundle['sleep_coach'] as Map).containsKey('strain_bonus_min'); + +/// The bonus `sleepNeed` adds for a given strain: (strain/21) * 45 min. +double _bonusSec(double strain) => (strain / 21.0) * 45.0 * 60.0; + +void main() { + const profile = {}; + + group('sleep need — the strain bonus is TODAY-scoped', () { + test("a day with no strain reading is NOT given yesterday's bonus", () { + // Today abstained; the 6 days before it each report a strain of 18. + final noReading = buildCrossDayBundle(_days(7), profile); + // Same series, but today explicitly reports zero strain. + final explicitZero = + buildCrossDayBundle(_days(7, strainToday: 0), profile); + + expect( + _needSec(noReading), + _needSec(explicitZero), + reason: 'an absent strain reading must add nothing — reaching back a ' + 'day for a number is imputation, and it inflates tonight\'s ' + 'recommendation with a workout the user did not do today', + ); + }); + + test('that assertion can actually see the difference', () { + // Guard on the test itself: `sleepNeed` clamps to [6 h, 11 h] AFTER + // adding, so a series that saturated the ceiling would make the pin above + // pass no matter which day's strain was read. Prove the bonus is live and + // unclamped for this series before trusting the pin. + final zero = buildCrossDayBundle(_days(7, strainToday: 0), profile); + final heavy = buildCrossDayBundle(_days(7, strainToday: 18), profile); + + expect( + _needSec(heavy) - _needSec(zero), + closeTo(_bonusSec(18), 1e-6), + reason: 'a strain of 18 must add its full (18/21)*45 min here, or the ' + 'regression above is passing for the wrong reason', + ); + }); + + test("with no derived row for today, yesterday's strain is not used", () { + // The most recent record in the list is YESTERDAY, and it reports a + // strain of 18. Reading days.last positionally would apply its bonus. + final noToday = + buildCrossDayBundle(_days(7, todayDerived: false), profile); + final explicitZero = + buildCrossDayBundle(_days(7, strainToday: 0), profile); + + expect( + _needSec(noToday), + _needSec(explicitZero), + reason: 'no row for today means no strain reading for today', + ); + }); + }); + + group('sleep need — the strain bonus is disclosed, not silent', () { + test('strain_bonus_min reports the minutes that were added', () { + final b = buildCrossDayBundle(_days(7, strainToday: 18), profile); + // (18/21)*45 min = 38.57 min. + expect(_strainBonus(b), 39); + }); + + test('strain_bonus_min is null when today produced no strain reading', () { + final b = buildCrossDayBundle(_days(7), profile); + // Assert the KEY is present and its value is null. Without the key check + // this passes trivially against a bundle that never emitted the field. + expect(_hasStrainBonusKey(b), isTrue); + expect(_strainBonus(b), isNull, + reason: 'null and a confident 0 are different claims: an absent ' + 'strain reading silently withholds up to 45 min of need, and ' + 'that is exactly what this field exists to surface'); + }); + + test('strain_bonus_min is 0, not null, on a genuine rest day', () { + // Today reported strain and it was zero. That is a measured rest day, not + // an absent reading — the distinction the null case above depends on. + final b = buildCrossDayBundle(_days(7, strainToday: 0), profile); + expect(_strainBonus(b), 0); + }); + + test('the disclosed bonus is what was APPLIED, not the raw formula value', + () { + // sleepNeed clamps to an 11 h CEILING after adding, so against a high + // baseline + debt the strain bonus is only partly realized. Disclosing + // the raw (strain/21)*45 would state an increase the published need never + // took. 8.75 h weekdays / 10 h weekends => habitual 8.75, OSD 10, + // debt 1.25 h, baseline clamped to 9.5 h — leaving only part of the + // bonus room under the ceiling. + final b = buildCrossDayBundle( + _days(7, strainToday: 18, weekdayTstMin: 525, freeTstMin: 600), + profile, + ); + final zero = buildCrossDayBundle( + _days(7, strainToday: 0, weekdayTstMin: 525, freeTstMin: 600), + profile, + ); + final bonus = (_strainBonus(b) as num).toDouble(); + final applied = _needSec(b) - _needSec(zero); + + expect(bonus * 60, closeTo(applied, 60)); + expect(bonus * 60, lessThan(_bonusSec(18)), + reason: 'the ceiling binds, so a strain of 18 cannot add its full ' + '38.6 min of need here'); + }); + }); + + group('strainBonusCaption — what the coach card actually says', () { + test('an applied bonus is spelled out with its sign', () { + expect(strainBonusCaption(39), "+39m added for today's strain"); + }); + + test('an hour-plus bonus reads in h/m', () { + // Not reachable through sleepNeed's 45 min cap today, but _dur is shared + // and the caption must not render "75m" if that cap ever moves. + expect(strainBonusCaption(75), "+1h 15m added for today's strain"); + }); + + test('no strain reading today produces no line', () { + // Matches the nap credit: the card stays silent rather than inventing + // "+0m". The bundle still distinguishes null from 0 for anything that + // needs to act on it. + expect(strainBonusCaption(null), isNull); + }); + + test('a measured rest day produces no line', () { + expect(strainBonusCaption(0), isNull); + }); + + test('a bonus fully swallowed by the clamp produces no line', () { + // Applied 0 against a real strain reading: nothing was added, so there is + // nothing to disclose. Claiming "+0m" would be noise. + expect(strainBonusCaption(0), isNull); + }); + }); +} From 5e57df5179cda52732d07e7c46eb0408515bbe82 Mon Sep 17 00:00:00 2001 From: SATHVIK SVS Date: Thu, 6 Aug 2026 23:41:06 +0530 Subject: [PATCH 3/6] =?UTF-8?q?repin=20analytics=20to=20#38=20merge=20?= =?UTF-8?q?=E2=80=94=20the=20pin=20v55/v56=20always=20described?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kAlgoVersion 55 and 56 both cite the new nap detector as their sibling change, and until this commit neither was backed by the pin. pubspec.yaml still pointed at #34 (f0d1153), which has no `sleep/nap.dart`, no `wristOff:`/`exclude:` on `detectNaps`, and no `tstSec`/`tibSec`/`efficiency` on `NapWindow`. This is the §3.5 failure mode the v43 changelog is remembered for, except worse in kind: v43 shipped a changelog describing a fix its pin merely lacked, while this branch did not COMPILE against its own pin — 10 analyzer errors in derivation_engine.dart. It went unnoticed because pubspec_overrides.yaml is gitignored and resolves both siblings to local working copies, so every local build and test run silently used analytics HEAD rather than the pinned SHA. Confirmed by moving the override aside and running `flutter pub get` against the real pin. Verified present at c3a30be, per §3.5: git show c3a30be:lib/src/onehz/sleep/nap.dart | grep -cE 'wristOff|exclude|tibSec' -> 16 git show c3a30be:lib/src/onehz/sleep/van_hees.dart | grep -c immobilityMask -> 2 pubspec.lock regenerated with the override moved aside, so it locks the git SHA rather than `path: ../analytics`. A path-source lock fails CI `flutter pub get` (exit 66) and is the reason that file must never be regenerated with the override in place. No kAlgoVersion bump: 56 is already the version describing this analytics behaviour, and it has not shipped. The pin and the version now land together, which is the whole point. Against the real pin: flutter analyze lib/ clean, 1150 tests pass. The single failure is workout_reliability_test.dart's queued-job case, pre-existing and identical on clean main. --- pubspec.lock | 4 ++-- pubspec.yaml | 31 ++++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/pubspec.lock b/pubspec.lock index 550fd1b6..e99f208a 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -964,8 +964,8 @@ packages: dependency: "direct main" description: path: "." - ref: f0d115308aa5e9e3c82ee113c3d52e59756121d4 - resolved-ref: f0d115308aa5e9e3c82ee113c3d52e59756121d4 + ref: c3a30be1e36e33426c83cead9ea106fa3071b082 + resolved-ref: c3a30be1e36e33426c83cead9ea106fa3071b082 url: "https://github.com/OpenStrap/analytics.git" source: git version: "1.0.0" diff --git a/pubspec.yaml b/pubspec.yaml index 422986ad..fe8bfb63 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -68,7 +68,36 @@ dependencies: # DREAMT-optimal values under-called REM badly on real WHOOP data. # Verified present: `git show :lib/src/onehz/sleep/cardio_stager.dart # | grep -E 'classifyCardioEpochs|_remScoreCut = 0.5'`. - ref: f0d115308aa5e9e3c82ee113c3d52e59756121d4 + # + # analytics main @ #38 merge — the nap detector this release is built on. + # kAlgoVersion 55 and 56 BOTH cite it, and until this line moved neither + # was backed: edge called `detectNaps(..., wristOff:, exclude:)` and read + # `NapWindow.tstSec`/`tibSec`/`efficiency`, none of which exist at #34. + # This was not a silently-missing behaviour like v43's — it was a hard + # compile break (10 analyzer errors), masked locally by the gitignored + # pubspec_overrides.yaml pointing at a working copy. Repinning here is + # what makes v55/v56 real rather than aspirational (§3.5). + # + # #38 adds `sleep/nap.dart` as the ONLY nap source: van Hees z-angle + # immobility on the complement of the main sleep window, corroborated by + # an HR dip against the AWAKE-DAYTIME baseline, reporting TST and in-bed + # SEPARATELY. The old path delegated to the nocturnal stager, whose + # `minSleepMin = 60` exists to REJECT naps, so the 20-45 min afternoon nap + # was structurally undetectable. `immobilityMask` is factored out of + # van_hees.dart so night and nap share one primitive. + # + # Also carries the #38 review pass: the scoring harness compared `tstSec` + # against interval labels (which are in-bed spans), charging every matched + # nap its own awake time as error, and abstained days were excluded from + # sensitivity/PPV without saying so. Neither affects shipped output — both + # are measurement-tool fixes — but they are why the accuracy figures in + # #38's description differ from its first revision. + # + # Verified present: `git show :lib/src/onehz/sleep/nap.dart + # | grep -E 'wristOff|exclude|tibSec'` (16 hits) and + # `git show :lib/src/onehz/sleep/van_hees.dart + # | grep -c immobilityMask` (2). + ref: c3a30be1e36e33426c83cead9ea106fa3071b082 # BLE — flutter_blue_plus is the maintained cross-platform GATT client. flutter_blue_plus: ^1.36.8 From 6fc96e6e52dfaaeb00831480c8155cafd09e99a4 Mon Sep 17 00:00:00 2001 From: SATHVIK SVS Date: Thu, 6 Aug 2026 23:56:17 +0530 Subject: [PATCH 4/6] review: a day-relative stamp must not outlive its day MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review on #204. `is_today` OUTLIVING ITS DAY — the real find, and it reopened this PR's own bug through a different door. `_refreshCrossDayInputArtifact` stamps `is_today: true` on the most recent record so `_todayNum` can tell "today abstained" from "today has no row yet". That is a fact ABOUT A DAY stored as a bare boolean, and it goes into the DURABLE `crossday_input` baseline row. `_crossDayInputDays()` then preferred that cache whenever it merely PARSED — no day check, no algo_version check. A cache written yesterday hands back a series whose last record still claims to be today, so `_todayNum` reports yesterday's strain and nap minutes as today's and they land inside `need_sec`: exactly the imputation this PR removes, arriving through the cache instead of `_lastNum`. NOT reachable today, and I checked before deciding how to fix it: all four `_runCrossDay` call sites refresh the artifact immediately beforehand, and the two conditional ones (`if (done > 0)`) skip both together. But that is an unenforced ordering coincidence, not a guarantee — one new caller, or one early return inside `_refreshBaselines`, makes it live and silent. A day-relative fact should not depend on call ordering to stay true. The envelope now carries `built_for_day`, and `crossDayArtifactUsableToday` is the only thing allowed to declare a cached artifact reusable. Pure and static so it is unit-testable without a database, which the seam that consumes it is not. An artifact with no `built_for_day` — anything written before this field — cannot be SHOWN to be fresh, so it is rebuilt rather than assumed fresh. Costs nothing in the normal path, since the refresh already runs first. MAIN SLEEP HAD NO CONFIDENCE DOT. `_periodsWithMainStages` enriches the main period with the hypnogram and stage minutes but emitted no `confidence`, while every nap carries one, and sleep_periods_screen draws a ConfDot for any period that has one. So the best-evidenced period on the screen was the only one rendering as unknown. `sleep.accounting.confidence` was already in hand two lines above. Null stays null and correctly draws nothing. NAP CAPTION EXTRACTED to `napCreditCaption`, mirroring `strainBonusCaption`. The strain line was extracted and unit-tested when it was added; leaving its twin inline meant the two disclosures on the same card were built and covered differently for no reason. Same silence rule, now pinned by tests. flutter analyze lib/ test/ clean; 1158 tests pass (was 1150). The single failure is workout_reliability_test.dart's queued-job case, pre-existing and identical on clean main. --- lib/compute/derivation_engine.dart | 56 ++++++++++++--- lib/data/local_repository_impl.dart | 27 +++++--- lib/ui/insights/coach_cards.dart | 20 ++++-- test/crossday_artifact_freshness_test.dart | 79 ++++++++++++++++++++++ test/nap_credit_test.dart | 20 ++++++ 5 files changed, 181 insertions(+), 21 deletions(-) create mode 100644 test/crossday_artifact_freshness_test.dart diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index 95a31553..ecdb35f6 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -2676,6 +2676,32 @@ class DerivationEngine { static const Duration _crossDayTimeout = Duration(seconds: 30); static const int _crossDayWindow = 90; + /// Whether a persisted `crossday_input` artifact may be reused AS-IS today. + /// + /// Pure, so it is unit-testable without a database — the seam that consumes it + /// ([_crossDayInputDays]) cannot be. + /// + /// The artifact stamps `is_today: true` on the row that was today WHEN IT WAS + /// BUILT. That is a fact about a day stored as a bare boolean, in a DURABLE + /// row, so a cached artifact served on a later day hands `_todayNum` a record + /// that still claims to be today — and yesterday's strain and nap minutes land + /// inside tonight's `need_sec`. That is the exact imputation the stamp exists + /// to prevent (§3.3), arriving through the cache instead of through `_lastNum`. + /// + /// Every current `_runCrossDay` call site refreshes the artifact immediately + /// beforehand, so the stale read is not reachable today. That is an unenforced + /// ordering coincidence and not something to rely on: one new caller, or one + /// early return inside `_refreshBaselines`, makes it live and silent. + /// + /// An artifact with no `built_for_day` (written before this field existed) + /// cannot be SHOWN to be fresh, so it is rebuilt rather than assumed fresh. + static bool crossDayArtifactUsableToday(Object? decoded, String today) { + if (decoded is! Map) return false; + if (decoded['days'] is! List) return false; + final builtFor = decoded['built_for_day']; + return builtFor is String && builtFor.isNotEmpty && builtFor == today; + } + Future _runCrossDay(Profile profile) async { try { final days = await _crossDayInputDays(); @@ -2708,14 +2734,16 @@ class DerivationEngine { if (raw is String && raw.isNotEmpty) { try { final decoded = jsonDecode(raw); - if (decoded is Map) { - final rows = decoded['days']; - if (rows is List) { - return [ - for (final row in rows) - if (row is Map) row.cast(), - ]; - } + // Day-gated, NOT just well-formed. The rows carry `is_today`, which is a + // fact about the day the artifact was BUILT on; serving them on a later + // day makes `_todayNum` read yesterday's strain and nap minutes as + // today's (§3.3). See [crossDayArtifactUsableToday]. + if (crossDayArtifactUsableToday(decoded, LocalDb.localDayLabelNow())) { + final rows = (decoded as Map)['days'] as List; + return [ + for (final row in rows) + if (row is Map) row.cast(), + ]; } } catch (_) { // Fall through to rebuild from day_result. @@ -2763,7 +2791,17 @@ class DerivationEngine { if (row['day_id'] == today) rec['is_today'] = true; days.add(rec); } - return (days, jsonEncode({'algo_version': kAlgoVersion, 'days': days})); + // `built_for_day` is what makes the `is_today` stamps inside `days` + // interpretable later. Without it the envelope carries day-relative facts + // with no day attached, and any reader has to assume freshness. + return ( + days, + jsonEncode({ + 'algo_version': kAlgoVersion, + 'built_for_day': today, + 'days': days, + }) + ); }, _crossDayTimeout, label: 'crossday-input'); await LocalDb.putBaseline('crossday_input', json); return days; diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index 002006aa..453a1db3 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -610,12 +610,21 @@ 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'), - }), + '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'], // Sleep cycles — Rosenblum 2024 "fractal cycles" (HRV-adapted): peak-to- // peak of the smoothed per-minute RMSSD series (REM peaks / NREM troughs). @@ -644,8 +653,9 @@ class LocalRepositoryImpl extends LocalRepository { /// "no deep sleep" instead of "not measured". List> _periodsWithMainStages( Map b, - Map stageMin, - ) { + Map stageMin, { + num? mainConfidence, + }) { final raw = (b['sleep_periods'] as Map?)?['periods']; if (raw is! List) return const []; final hypno = _hypnoPoints(b); @@ -662,6 +672,7 @@ class LocalRepositoryImpl extends LocalRepository { ...p.cast(), if (hypno.isNotEmpty) 'hypnogram': hypno, if (stages.isNotEmpty) 'stages': stages, + 'confidence': ?mainConfidence, }, ]; } diff --git a/lib/ui/insights/coach_cards.dart b/lib/ui/insights/coach_cards.dart index 029d61fc..315e1188 100644 --- a/lib/ui/insights/coach_cards.dart +++ b/lib/ui/insights/coach_cards.dart @@ -34,6 +34,21 @@ String _dur(num sec) { return m == 0 ? '${h}h' : '${h}h ${m}m'; } +/// Caption for the nap credit ALREADY folded into `sleep_coach.need_sec`. +/// +/// [napCreditMin] is `sleep_coach.nap_credit_min` — the minutes the credit +/// actually REMOVED, after `sleepNeed`'s 6 h floor took its cut, not the raw +/// nap minutes. +/// +/// Null caption (no line) when today produced no nap reading (`null`) or when +/// nothing was subtracted (`0`). Same silence rule as [strainBonusCaption], and +/// the bundle keeps null and 0 apart even though the card does not. +String? napCreditCaption(num? napCreditMin) { + final m = napCreditMin?.round(); + if (m == null || m <= 0) return null; + return '−${_dur(m * 60)} credited from today\'s nap'; +} + /// Caption for the strain bonus ALREADY folded into `sleep_coach.need_sec`. /// /// [strainBonusMin] is `sleep_coach.strain_bonus_min` — the minutes the bonus @@ -191,10 +206,7 @@ class _SleepCoachCardState extends State { // the difference between a number the user can reason about and one that // silently shrank — a nap also inflates "% of need" through the same // subtraction, so an unexplained credit moves both figures at once. - final napCreditMin = (_coach?['nap_credit_min'] as num?)?.round(); - final napLine = (napCreditMin != null && napCreditMin > 0) - ? '−${_dur(napCreditMin * 60)} credited from today\'s nap' - : null; + final napLine = napCreditCaption(_coach?['nap_credit_min'] as num?); // Same reasoning in the other direction: today's strain is already ADDED to // `need_sec` (up to 45 min), so an unexplained bonus moves both the need and diff --git a/test/crossday_artifact_freshness_test.dart b/test/crossday_artifact_freshness_test.dart new file mode 100644 index 00000000..65afce25 --- /dev/null +++ b/test/crossday_artifact_freshness_test.dart @@ -0,0 +1,79 @@ +// HONESTY REGRESSION — a day-relative stamp must not outlive its day. +// +// `_refreshCrossDayInputArtifact` marks the most recent record `is_today: true` +// so today-scoped reads (`_todayNum`) can tell "today abstained" from "today has +// no row yet". That stamp is a fact ABOUT A DAY stored as a bare boolean, and it +// is written into the DURABLE `crossday_input` baseline row. +// +// `_crossDayInputDays()` prefers that cached row whenever it parses. So a cache +// written yesterday hands back a series whose last record still claims +// `is_today: true` — and `_todayNum` then reports YESTERDAY's strain and nap +// minutes as today's, landing them inside `need_sec`. That is precisely the +// imputation the stamp exists to prevent (AGENTS §3.3), re-entering through the +// cache rather than through `_lastNum`. +// +// Today the four `_runCrossDay` call sites each refresh the artifact immediately +// beforehand, so the stale read is not reachable in practice. That is an +// unenforced ordering coincidence, not a guarantee: one new caller, or one early +// return inside `_refreshBaselines`, makes it live and silent. The envelope now +// carries the day it was built for, and this predicate is the only thing allowed +// to declare a cached artifact reusable. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/compute/derivation_engine.dart'; + +void main() { + Map envelope(String? builtFor) => { + 'algo_version': 56, + 'built_for_day': ?builtFor, + 'days': [ + {'date': '2024-03-01', 'strain': 14.0}, + {'date': '2024-03-02', 'strain': 18.0, 'is_today': true}, + ], + }; + + group('crossDayArtifactUsableToday', () { + test('an artifact built today is reusable', () { + expect( + DerivationEngine.crossDayArtifactUsableToday(envelope('2024-03-02'), '2024-03-02'), + isTrue, + ); + }); + + test("an artifact built YESTERDAY is not reusable today", () { + // The whole bug: its last record still says `is_today: true`, and that + // record is yesterday's. + expect( + DerivationEngine.crossDayArtifactUsableToday(envelope('2024-03-02'), '2024-03-03'), + isFalse, + reason: "a stamp that says 'today' must not be believed on a later " + "day — that is how yesterday's strain becomes tonight's sleep need", + ); + }); + + test('an artifact with no day stamped is not reusable', () { + // Written before `built_for_day` existed. It cannot be SHOWN to be fresh, + // so it is rebuilt rather than assumed fresh. + expect(DerivationEngine.crossDayArtifactUsableToday(envelope(null), '2024-03-02'), isFalse); + }); + + test('a malformed or empty envelope is not reusable', () { + expect(DerivationEngine.crossDayArtifactUsableToday(null, '2024-03-02'), isFalse); + expect(DerivationEngine.crossDayArtifactUsableToday('not a map', '2024-03-02'), isFalse); + expect( + DerivationEngine.crossDayArtifactUsableToday( + {'built_for_day': '2024-03-02'}, // no `days` + '2024-03-02', + ), + isFalse, + ); + expect( + DerivationEngine.crossDayArtifactUsableToday( + {'built_for_day': '', 'days': const []}, + '2024-03-02', + ), + isFalse, + ); + }); + }); +} diff --git a/test/nap_credit_test.dart b/test/nap_credit_test.dart index e9f4ab58..5bb94e08 100644 --- a/test/nap_credit_test.dart +++ b/test/nap_credit_test.dart @@ -16,6 +16,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:openstrap_edge/compute/crossday_pipeline.dart'; +import 'package:openstrap_edge/ui/insights/coach_cards.dart'; /// Minimal oldest-first day series with the fields sleep need actually reads. /// @@ -127,6 +128,25 @@ void main() { }); }); + group('napCreditCaption — what the coach card actually says', () { + test('an applied credit is spelled out with its sign', () { + expect(napCreditCaption(45), "−45m credited from today's nap"); + }); + + test('an hour-plus credit reads in h/m', () { + expect(napCreditCaption(90), "−1h 30m credited from today's nap"); + }); + + test('no nap reading today produces no line', () { + expect(napCreditCaption(null), isNull); + }); + + test('a day with no nap produces no line', () { + // Zero credit is nothing to disclose; "−0m" would be noise. + expect(napCreditCaption(0), isNull); + }); + }); + group('sleep need — today must be identified, not assumed positional', () { test("with no derived row for today, yesterday's nap is not credited", () { // The most recent record in the list is YESTERDAY, and it reports a From 0a917b2d53b22bafd33b76f1b5eee51b2bd60835 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Fri, 7 Aug 2026 00:05:50 +0530 Subject: [PATCH 5/6] naps: don't claim a total we didn't judge, and stop double-counting midnight Three review findings on the nap -> sleep-periods seam. All three verified against the code first; the pin (a) was already fixed by 5e57df5. 1. ABSENT IS NOT ZERO, AND IT IS NOT A CONFIDENT TOTAL EITHER. `_attachNaps` returned `const []` from FOUR places: the day was judged and held no nap, the substrate was too short, the detector abstained, and an exception. `_sleepPeriods` could not tell them apart, so it left `totalKnown = true` and published `total_asleep_min = mainTstMin` as a complete day total on days whose naps were never assessed -- in the very same bundle where `naps.value` is null and `nap_min` is (correctly) left unwritten. The PR already applied "an unknown component makes the SUM unknown" to unknown DURATIONS; this extends it to unknown EXISTENCE. `_attachNaps` now returns null for the three unjudged cases. 2. THE MIDNIGHT DOUBLE-COUNT. Attribution was guarded on the trailing edge only (`start < attributionEndSec`, plus the analytics-side backward `unfinished` walk). Nothing guarded the leading edge: `napSub` opens AT local midnight, and analytics' `stillAt(0)` short-circuits its discontinuity check at `k == 0` (nap.dart), so the post-midnight remainder of a nap yesterday already emitted whole is re-detected today as a fresh bout at index 0 and credited a second time -- phantom nap card, phantom Timeline band, and its minutes subtracted from today's sleep need. Couch 23:40-00:40 reproduces it. This was unreachable while the old nocturnal detector needed 60+ min and an HR dip; `minNapSec` at 15 min makes it reachable. Gated on CONTIGUITY, not on index alone. If the record only starts hours into the day, yesterday's detector broke on that same recording discontinuity and dropped the bout too -- dropping it here as well would trade a double-count for silent data loss. `napLeadingEdgeContiguitySec`. 3. PRE-RENAME DAYS RENDER BLANK CARDS FOREVER. The producer moved to `onset_ts`/`wake_ts`/`duration_min`, but days derived before that keep `start`/`end`/`asleep_min` and are never rewritten: a day finalizes ~48 h behind the data edge and raw is pruned after `rawRetentionDays`, so once its substrate is gone a kAlgoVersion bump cannot re-derive it and `dayResult()` serves that payload forever. Every such day showed "--" for onset, wake AND duration under a still-confident hero total, which reads as data loss rather than an old schema. Translated on READ, which also makes this independent of the merge order of the two PRs touching this seam. Also corrected `napBoundaryBufferSec`'s doc comment, which asserted the buffer "can't double-count" -- finding 2 is exactly the case where it does. 9 tests added, each mutation-verified (reverting the guard fails the test and only that test). Full suite: 1156 tests, the 6 failures in notification_dedupe_test are pre-existing and reproduce on origin/main unmodified. kAlgoVersion deliberately NOT touched here -- findings 1 and 2 do change derived output, so whatever number this lands on must be new. --- lib/compute/derivation_engine.dart | 123 +++++++++-- lib/compute/derive_prepare.dart | 19 +- lib/data/local_repository_impl.dart | 30 ++- test/nap_attribution_test.dart | 264 +++++++++++++++++++++++ test/sleep_periods_legacy_keys_test.dart | 181 ++++++++++++++++ 5 files changed, 601 insertions(+), 16 deletions(-) create mode 100644 test/nap_attribution_test.dart create mode 100644 test/sleep_periods_legacy_keys_test.dart diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index ecdb35f6..b6211776 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -2303,6 +2303,12 @@ class DerivationEngine { ? null : (scMap!['efficiency'] as num).toDouble() / 100.0, date: day.date, + // Local midnight from the day LABEL, not from the substrate — this has + // to be where `napSub`'s slice window opens (`_localDayLabelToSec`), + // not where its first surviving sample happens to land, or the + // contiguity test compares a timestamp against itself and is + // vacuously true on every day with a gap at the boundary. + dayStartSec: _localDayLabelToSec(day.date), dayEndSec: day.endSec, dataNowSec: dataNowSec, ); @@ -3794,10 +3800,18 @@ class DerivationEngine { /// (`onset_ts`/`wake_ts`/`duration_min`/`efficiency`/`confidence`), and /// `duration_min` is minutes ASLEEP for both the main sleep and naps — they /// were previously different units under one label, then summed. + /// + /// [naps] is NULL when the nap detector could not judge the day at all (as + /// opposed to an empty list, which means "judged, and there were none"). An + /// unjudged day has an unknown NUMBER of periods, not just unknown durations, + /// so the total is unknown for exactly the same reason a null `duration_min` + /// makes it unknown — and `nap_min` is already left unwritten in that case. + /// Publishing `total_asleep_min = mainTstMin` there would state a complete + /// day total while `naps.value` is null, which is internally inconsistent. static Map _sleepPeriods( int onsetSec, int offsetSec, - List> naps, { + List>? naps, { int? mainTstMin, double? mainEfficiency, }) { @@ -3830,13 +3844,20 @@ class DerivationEngine { totalKnown = false; } } - for (final nap in naps) { - periods.add(nap); - final d = (nap['duration_min'] as num?)?.toInt(); - if (d != null) { - totalAsleep += d; - } else { - totalKnown = false; + if (naps == null) { + // Not judged. The day may hold any number of unmeasured naps, so no + // total can be stated — the screen renders "—" rather than a confident + // figure that silently omits them. + totalKnown = false; + } else { + for (final nap in naps) { + periods.add(nap); + final d = (nap['duration_min'] as num?)?.toInt(); + if (d != null) { + totalAsleep += d; + } else { + totalKnown = false; + } } } return { @@ -3858,19 +3879,23 @@ class DerivationEngine { /// ABSENT is not ZERO. When the detector cannot judge the day, `nap_min` is /// left UNWRITTEN rather than set to 0 — a written 0 is a claim that there /// were no naps, and it would also be picked up as a real value downstream. - static List> _attachNaps( + /// Returns NULL for that unjudged case and a (possibly empty) list when the + /// day really was judged, so [_sleepPeriods] can make the same distinction + /// instead of reading "no naps returned" as "no naps happened". + static List>? _attachNaps( Map bundle, Map? scMap, Substrate s, int onsetSec, int offsetSec, { + int? attributionStartSec, int? attributionEndSec, List> wristOff = const [], List> charging = const [], }) { try { final n = s.length; - if (n < 60) return const []; + if (n < 60) return null; final accel = [ for (var i = 0; i < n; i++) ana.AccelSample(s.tsSec[i] * 1000.0, s.ax[i], s.ay[i], s.az[i]), @@ -3903,14 +3928,34 @@ class DerivationEngine { 'inputs_used': m.inputs_used, 'note': m.note, }; - return const []; + return null; } final t0 = s.tsSec.first; + // The window opens AT local midnight and is contiguous into it, so a bout + // that begins at the very first sample was already in progress when we + // started looking — it is the tail of something that started YESTERDAY, + // and yesterday's buffered window (which runs `napBoundaryBufferSec` past + // its own midnight) saw it whole and emitted it whole. + // + // Analytics guards the trailing edge only: `unfinished` walks BACKWARD + // from the array end (nap.dart), while `stillAt(0)` short-circuits its + // discontinuity check at `k == 0` — so a bout at index 0 is always + // emitted, with no way for the detector to know what preceded it. Before + // `minNapSec` dropped to 15 min this was unreachable (the old nocturnal + // detector needed 60+ min and an HR dip); it is reachable now. + // + // Gated on contiguity, NOT on index alone: if the record only STARTS + // hours into the day (band off overnight), yesterday's detector broke on + // that same discontinuity and dropped the bout too, so dropping it here + // as well would lose a real nap rather than de-duplicate one. + final leadingEdgeOwnedByYesterday = attributionStartSec != null && + t0 <= attributionStartSec + napLeadingEdgeContiguitySec; // A nap STARTING at/after the real day boundary is tomorrow's — its own // (unbuffered) window finds it independently, so keeping it here too // would double-count it. final naps = m.value!.where((nap) { + if (leadingEdgeOwnedByYesterday && nap.startSec == 0) return false; if (attributionEndSec == null) return true; return t0 + nap.startSec < attributionEndSec; }).toList(); @@ -3960,7 +4005,7 @@ class DerivationEngine { ]; } catch (e) { if (kDebugMode) debugPrint('[derive] naps FAILED/skipped: $e'); - return const []; + return null; } } @@ -4193,6 +4238,7 @@ class DerivationEngine { inp.napSub, onset, offset, + attributionStartSec: inp.dayStartSec, attributionEndSec: inp.dayEndSec, wristOff: inp.wristOffSpans, charging: inp.chargingSpans, @@ -4541,6 +4587,52 @@ class DerivationEngine { @visibleForTesting (int, int) debugTargetDayWindow(String dayId) => _targetDayWindow(dayId); + /// Test seam for [_sleepPeriods] — "an unjudged day publishes no total" is a + /// one-line invariant guarding a user-visible number, so it is pinned + /// directly rather than through a full derive pass. + @visibleForTesting + static Map debugSleepPeriods( + int onsetSec, + int offsetSec, + List>? naps, { + int? mainTstMin, + double? mainEfficiency, + }) => + _sleepPeriods( + onsetSec, + offsetSec, + naps, + mainTstMin: mainTstMin, + mainEfficiency: mainEfficiency, + ); + + /// Test seam for [_attachNaps] — the day-boundary attribution rules (drop + /// tomorrow's leading nap, drop yesterday's trailing one) decide which day a + /// nap's minutes are credited to, and are cheap to state directly. + @visibleForTesting + static List>? debugAttachNaps( + Map bundle, + Map? scMap, + Substrate s, + int onsetSec, + int offsetSec, { + int? attributionStartSec, + int? attributionEndSec, + List> wristOff = const [], + List> charging = const [], + }) => + _attachNaps( + bundle, + scMap, + s, + onsetSec, + offsetSec, + attributionStartSec: attributionStartSec, + attributionEndSec: attributionEndSec, + wristOff: wristOff, + charging: charging, + ); + void _log(String m) { if (kDebugMode) debugPrint('[derive] $m'); log?.call('[derive] $m'); @@ -4611,6 +4703,12 @@ class _DayBlocksInput { final double? mainEfficiency; final String date; + + /// Local midnight opening this calendar day — where `napSub` starts. Nap + /// attribution needs BOTH boundaries: [dayEndSec] pushes a nap starting in + /// the borrowed buffer onto tomorrow, and this one drops the tail of a nap + /// yesterday already owns. See `_attachNaps`. + final int dayStartSec; final int dayEndSec; final int dataNowSec; const _DayBlocksInput({ @@ -4633,6 +4731,7 @@ class _DayBlocksInput { required this.mainTstMin, required this.mainEfficiency, required this.date, + required this.dayStartSec, required this.dayEndSec, required this.dataNowSec, }); diff --git a/lib/compute/derive_prepare.dart b/lib/compute/derive_prepare.dart index 7892a9d9..cd02dc7b 100644 --- a/lib/compute/derive_prepare.dart +++ b/lib/compute/derive_prepare.dart @@ -87,10 +87,25 @@ class PreparedDerivationDay { /// How far past a day's calendar end nap detection is allowed to look, so a /// nap/secondary-sleep block spanning midnight is seen whole by the day it /// started on. Naps starting inside this buffer belong to tomorrow, which -/// sees them anyway in its own regular (unbuffered) window — so this can't -/// double-count. +/// sees them anyway in its own regular (unbuffered) window. +/// +/// That buffer stops the day that OWNS the nap from bisecting it, but it does +/// NOT by itself stop the double-count: tomorrow re-detects the post-midnight +/// remainder as a fresh bout starting at index 0 of its own window. See +/// [napLeadingEdgeContiguitySec] for the guard that drops it. const int napBoundaryBufferSec = 3 * 3600; +/// How close a day's first sample must be to local midnight for the record to +/// count as CONTIGUOUS into the day boundary. +/// +/// Within this tolerance, a nap bout beginning at the very first sample is the +/// tail of one yesterday already saw (through [napBoundaryBufferSec]) and +/// emitted whole, so today must not count it again. Beyond it there is a real +/// recording gap at the boundary — yesterday's detector broke on that same +/// discontinuity and dropped the bout too, so today is its only chance to be +/// counted at all. +const int napLeadingEdgeContiguitySec = 60; + class PreparedDerivationPayload { final int dataNowSec; final List days; diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index 453a1db3..25ee611d 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -666,10 +666,10 @@ class LocalRepositoryImpl extends LocalRepository { return [ for (final p in raw.whereType()) if (p['is_main'] != true) - p.cast() + _canonicalPeriod(p) else { - ...p.cast(), + ..._canonicalPeriod(p), if (hypno.isNotEmpty) 'hypnogram': hypno, if (stages.isNotEmpty) 'stages': stages, 'confidence': ?mainConfidence, @@ -677,6 +677,32 @@ class LocalRepositoryImpl extends LocalRepository { ]; } + /// Reads a persisted period under EITHER key vocabulary. + /// + /// The producer emits `onset_ts`/`wake_ts`/`duration_min`, but day results + /// written before that change hold `start`/`end`/`asleep_min` and are never + /// rewritten: a day finalizes ~48 h behind the data edge and raw is pruned + /// after `rawRetentionDays`, so once its substrate is gone a kAlgoVersion + /// bump cannot re-derive it — the old payload is what that day will serve + /// forever. Without this the Sleep-periods cards for every such day render + /// "—" for onset, wake AND duration, underneath a hero total that is still + /// confident, which reads as data loss rather than an old schema. + /// + /// Translating on READ (rather than migrating on write) also means this and + /// the parallel fix at the other end of the seam are order-independent. + Map _canonicalPeriod(Map p) { + final m = p.cast(); + // Only fill what's missing — a period already speaking the current + // vocabulary passes through byte-for-byte. + return { + ...m, + if (m['onset_ts'] == null && m['start'] != null) 'onset_ts': m['start'], + if (m['wake_ts'] == null && m['end'] != null) 'wake_ts': m['end'], + if (m['duration_min'] == null && m['asleep_min'] != null) + 'duration_min': m['asleep_min'], + }; + } + /// Mean completed-cycle length (min), or null when no cycles. num? _cyclesMeanMin(Map b) { final cyc = _sub(b, 'sleep')?['cycles']; diff --git a/test/nap_attribution_test.dart b/test/nap_attribution_test.dart new file mode 100644 index 00000000..d25e0803 --- /dev/null +++ b/test/nap_attribution_test.dart @@ -0,0 +1,264 @@ +// HONESTY + DOUBLE-COUNT REGRESSIONS on the nap → sleep-periods seam. +// +// Three separate ways the nap path stated something it did not know: +// +// 1. `_attachNaps` collapsed "judged, and there were no naps" with "could not +// judge this day at all" — both returned an empty list. `_sleepPeriods` +// then published `total_asleep_min = mainTstMin` as a CONFIDENT day total +// on a day whose naps were never assessed, in the very same bundle where +// `naps.value` is null and `nap_min` was (correctly) left unwritten. +// +// 2. The midnight double-count. Attribution was guarded on the TRAILING edge +// only (`start < attributionEndSec`, plus the analytics-side backward +// `unfinished` walk). Nothing guarded the LEADING edge: analytics' +// `stillAt(0)` short-circuits its discontinuity check at `k == 0` +// (nap.dart), so the post-midnight remainder of a nap yesterday already +// emitted whole is re-detected today as a fresh bout at index 0 and +// credited a second time. Unreachable until `minNapSec` dropped to 15 min. +// +// 3. The leading-edge guard must NOT fire when the record only starts hours +// into the day: yesterday's detector broke on that same recording +// discontinuity and dropped the bout too, so today is its only chance to +// count it. Dropping it there would trade a double-count for data loss. + +import 'dart:math' as math; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:openstrap_edge/compute/derivation_engine.dart'; +import 'package:openstrap_edge/compute/substrate.dart'; + +/// A 1 Hz substrate with a single still, low-HR block — the shape `detectNaps` +/// scores as a nap — laid over an otherwise moving, awake-HR day. +/// +/// [startSec] is the epoch second of the FIRST sample (so a test can model a +/// record that opens exactly at local midnight, or hours after it). +/// [napFromSec]/[napToSec] are offsets from [startSec]. +Substrate _daySubstrate({ + required int startSec, + required int lengthSec, + required int napFromSec, + required int napToSec, +}) { + final ts = []; + final hr = []; + final ax = []; + final ay = []; + final az = []; + for (var i = 0; i < lengthSec; i++) { + ts.add(startSec + i); + final inNap = i >= napFromSec && i < napToSec; + // Awake baseline ~78 bpm; the nap sits well under napRestingHrMult (0.95). + hr.add(inNap ? 56 : 78); + if (inNap) { + // Perfectly still: a constant gravity vector → zero z-angle delta. + ax.add(0.0); + ay.add(0.0); + az.add(1.0); + } else { + // Awake movement. It has to be a RAMP, not an alternation: the mask + // smooths the z-angle with a 5-second rolling MEDIAN, which erases a + // 1 Hz square wave entirely and would make the whole day read immobile. + // A 10°/s sweep survives the median and clears the 5° threshold. + final deg = (i % 9) * 10.0; + final rad = deg * math.pi / 180.0; + ax.add(math.cos(rad)); + ay.add(0.0); + az.add(math.sin(rad)); + } + } + return Substrate( + tsSec: ts, + hr: hr, + rrTsMs: const [], + rrMs: const [], + ax: ax, + ay: ay, + az: az, + spo2Red: List.filled(lengthSec, 0), + spo2Ir: List.filled(lengthSec, 0), + skinTemp: List.filled(lengthSec, 0), + skinContact: List.filled(lengthSec, 0), + ); +} + +void main() { + // ── 1. absent is not zero, and it is not a confident total either ──────── + group('_sleepPeriods distinguishes "no naps" from "naps not judged"', () { + test( + 'a JUDGED day with no naps still publishes a confident total', + () { + final out = DerivationEngine.debugSleepPeriods( + 1000, + 1000 + 7 * 3600, + const >[], // judged; there were none + mainTstMin: 420, + ); + expect( + out['total_asleep_min'], + 420, + reason: 'nothing is unknown here — the day was assessed', + ); + }, + ); + + test( + 'an UNJUDGED day publishes NO total, even though the main sleep is known', + () { + final out = DerivationEngine.debugSleepPeriods( + 1000, + 1000 + 7 * 3600, + null, // detector abstained / threw — naps never assessed + mainTstMin: 420, + ); + expect( + out['total_asleep_min'], + isNull, + reason: + 'the day may hold unmeasured naps; 420 would be a claim that it ' + 'does not. The screen renders "—" instead.', + ); + // The main sleep is still listed — only the SUM is withheld. + expect((out['periods'] as List), hasLength(1)); + expect((out['periods'] as List).first['duration_min'], 420); + }, + ); + }); + + // ── 2 + 3. day-boundary attribution ────────────────────────────────────── + group('_attachNaps day-boundary attribution', () { + const midnight = 1750000800; // arbitrary local-midnight-ish epoch second + const napLen = 40 * 60; + + test( + 'a nap in progress at the first sample of a CONTIGUOUS record is ' + 'yesterday\'s and is not counted again today', + () { + // The record opens exactly at midnight and the block is already + // underway — i.e. it started before midnight, where yesterday's + // buffered window saw it whole and emitted it. + final s = _daySubstrate( + startSec: midnight, + lengthSec: 6 * 3600, + napFromSec: 0, + napToSec: napLen, + ); + final bundle = {}; + final sc = {}; + + final periods = DerivationEngine.debugAttachNaps( + bundle, + sc, + s, + 0, + 0, + attributionStartSec: midnight, + attributionEndSec: midnight + 86400, + ); + + expect( + periods, + isNotNull, + reason: 'the day WAS judged — this is a real empty, not an abstain', + ); + expect( + periods, + isEmpty, + reason: 'yesterday already credited these minutes', + ); + expect( + sc['nap_min'], + 0.0, + reason: 'judged, and none of it belongs to today', + ); + }, + ); + + test( + 'the same bout IS counted when the record only starts hours into the ' + 'day — yesterday could not have seen it', + () { + // A recording gap across the boundary: the first sample is 08:00. + // Yesterday's detector broke on that same discontinuity and dropped + // the bout, so today is its only chance to be counted. + const firstSample = midnight + 8 * 3600; + final s = _daySubstrate( + startSec: firstSample, + lengthSec: 6 * 3600, + napFromSec: 0, + napToSec: napLen, + ); + final bundle = {}; + final sc = {}; + + final periods = DerivationEngine.debugAttachNaps( + bundle, + sc, + s, + 0, + 0, + attributionStartSec: midnight, + attributionEndSec: midnight + 86400, + ); + + expect(periods, isNotNull); + expect( + periods, + hasLength(1), + reason: 'dropping this would be data loss, not de-duplication', + ); + expect((sc['nap_min'] as num) > 0, isTrue); + }, + ); + + test( + 'a mid-day nap is unaffected by the leading-edge guard', + () { + // Same contiguous-at-midnight record, but the block starts 2 h in. + final s = _daySubstrate( + startSec: midnight, + lengthSec: 6 * 3600, + napFromSec: 2 * 3600, + napToSec: 2 * 3600 + napLen, + ); + final bundle = {}; + final sc = {}; + + final periods = DerivationEngine.debugAttachNaps( + bundle, + sc, + s, + 0, + 0, + attributionStartSec: midnight, + attributionEndSec: midnight + 86400, + ); + + expect(periods, hasLength(1)); + expect(periods!.first['is_main'], false); + expect((periods.first['onset_ts'] as int) > midnight, isTrue); + }, + ); + + test( + 'a day too short to judge returns NULL, not an empty list', + () { + final s = _daySubstrate( + startSec: midnight, + lengthSec: 30, // < the 60-sample floor + napFromSec: 0, + napToSec: 0, + ); + final sc = {}; + expect( + DerivationEngine.debugAttachNaps({}, sc, s, 0, 0), + isNull, + ); + expect( + sc.containsKey('nap_min'), + isFalse, + reason: 'absent is not zero', + ); + }, + ); + }); +} diff --git a/test/sleep_periods_legacy_keys_test.dart b/test/sleep_periods_legacy_keys_test.dart new file mode 100644 index 00000000..1d48245a --- /dev/null +++ b/test/sleep_periods_legacy_keys_test.dart @@ -0,0 +1,181 @@ +// SCHEMA-DRIFT REGRESSION — Sleep-periods cards for days derived BEFORE the +// period key rename. +// +// The producer used to write `start`/`end`/`asleep_min`; it now writes +// `onset_ts`/`wake_ts`/`duration_min`, which is what the screen reads. +// +// Old rows are NOT re-derived into the new shape. A day finalizes ~48 h behind +// the data edge and raw is pruned after `rawRetentionDays`, so once its +// substrate is gone a kAlgoVersion bump cannot recompute it — `dayResult()` +// keeps serving that stored payload forever. Without a read-side translation +// every such day renders "—" for onset, wake AND duration on every card, +// underneath a hero total that is still confident: it reads as data loss +// rather than as an old schema. +// +// Doing this on READ rather than as a write migration is also what makes this +// fix independent of the ordering of the two PRs touching this seam. + +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as p; +import 'package:sqflite_common_ffi/sqflite_ffi.dart'; +import 'package:openstrap_edge/data/db.dart'; +import 'package:openstrap_edge/data/local_repository_impl.dart'; + +void main() { + late LocalRepositoryImpl repo; + + setUpAll(() async { + sqfliteFfiInit(); + databaseFactory = databaseFactoryFfi; + LocalDb.dbName = 'openstrap_periods_legacy_test.db'; + await databaseFactory.deleteDatabase( + p.join(await databaseFactory.getDatabasesPath(), LocalDb.dbName), + ); + repo = LocalRepositoryImpl(getProfileMap: () => const {}); + }); + + tearDownAll(() async { + await LocalDb.close(); + await databaseFactory.deleteDatabase( + p.join(await databaseFactory.getDatabasesPath(), LocalDb.dbName), + ); + }); + + setUp(() async { + final db = await LocalDb.instance; + await db.delete('day_result'); + await db.delete('metric_series'); + }); + + const onset = 1750000000; + const wake = onset + 7 * 3600; + const napOnset = onset + 14 * 3600; + const napWake = napOnset + 40 * 60; + + Future seed(Map sleepPeriods) 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_periods': sleepPeriods, + }), + windowJson: '{}', + ); + } + + test( + 'a period stored under the LEGACY keys still renders its times and ' + 'duration', + () async { + await seed({ + 'periods': [ + { + 'is_main': true, + 'start': onset, + 'end': wake, + 'asleep_min': 420, + }, + { + 'is_main': false, + 'start': napOnset, + 'end': napWake, + 'asleep_min': 38, + }, + ], + 'total_asleep_min': 458, + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + final periods = (sleep['periods'] as List).cast>(); + + expect(periods, hasLength(2)); + + final main = periods.firstWhere((p) => p['is_main'] == true); + expect(main['onset_ts'], onset); + expect(main['wake_ts'], wake); + expect( + main['duration_min'], + 420, + reason: 'the card printed "—" for every pre-rename day', + ); + + final nap = periods.firstWhere((p) => p['is_main'] != true); + expect(nap['onset_ts'], napOnset); + expect(nap['wake_ts'], napWake); + expect(nap['duration_min'], 38); + }, + ); + + test( + 'a period already using the CURRENT keys passes through untouched', + () async { + await seed({ + 'periods': [ + { + 'is_main': true, + 'onset_ts': onset, + 'wake_ts': wake, + 'duration_min': 415, + 'in_bed_min': 430, + }, + ], + 'total_asleep_min': 415, + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + final periods = (sleep['periods'] as List).cast>(); + + expect(periods, hasLength(1)); + expect(periods.first['onset_ts'], onset); + expect(periods.first['wake_ts'], wake); + expect(periods.first['duration_min'], 415); + expect(periods.first['in_bed_min'], 430); + expect( + periods.first.containsKey('start'), + isFalse, + reason: 'the translation must not invent legacy keys going the other way', + ); + }, + ); + + test( + 'an honestly-null duration is NOT back-filled from a legacy key that is ' + 'also absent', + () async { + await seed({ + 'periods': [ + { + 'is_main': true, + 'onset_ts': onset, + 'wake_ts': wake, + // staging produced no TST — the screen must keep rendering "—" + 'duration_min': null, + }, + ], + 'total_asleep_min': null, + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + final periods = (sleep['periods'] as List).cast>(); + + expect(periods.first['duration_min'], isNull); + expect(sleep['total_asleep_min'], isNull); + }, + ); +} From 87d32580d999d127b2c753400f0bc5d03a9643be Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Fri, 7 Aug 2026 01:07:39 +0530 Subject: [PATCH 6/6] naps: two real defects CodeRabbit found in my own previous commit Both verified against the code (and one against a repro) before fixing. 1. EXPLICIT NULL WAS BEING PROMOTED INTO A MEASUREMENT. `_canonicalPeriod`'s legacy-key fallback tested `m['onset_ts'] == null`, which cannot tell an ABSENT key from a key present with an explicit null. On a mixed payload -- `duration_min: null` (the new producer's honest "not measured") sitting beside a stale `asleep_min: 40` -- the null test back-filled 40 and the card rendered a confident duration for a period nobody measured. Reproduced directly: current guard -> duration_min = 40 containsKey -> duration_min = null That is exactly the dishonesty this seam was added to remove, reintroduced one layer up. Now `containsKey` on all three fields. 2. "UNKNOWN" HAD TWO DIFFERENT ENCODINGS. `_computeDayBlocks` starts from an empty bundlePatch and `_attachNaps` is the only writer of `naps`. The `!m.present` path wrote `naps.value: null`, but the short-input and error paths returned without writing anything, so the key was missing entirely. Which encoding a day got depended on HOW the abstention happened, and a reader checking `bundle['naps']?['value'] == null` and one checking `bundle.containsKey('naps')` would disagree about the same day. `_writeUnknownNaps` now publishes one explicit envelope on every path. Also added the wrist-off / charging coverage CodeRabbit asked for. Fair hit: the PR threads both spans into `detectNaps` and nothing exercised either, yet a band on a charger is perfectly still and is the dominant nap false positive -- so that guard is doing real work and was untested. 5 tests added, each mutation-verified (restoring the null test fails exactly the mixed-payload test; removing the envelope fails exactly the envelope test). Suite 1169 passing; the 6 failures in notification_dedupe_test are pre-existing and reproduce on origin/main unmodified. --- lib/compute/derivation_engine.dart | 30 ++++++++- lib/data/local_repository_impl.dart | 18 +++-- test/nap_attribution_test.dart | 86 ++++++++++++++++++++++++ test/sleep_periods_legacy_keys_test.dart | 57 ++++++++++++++++ 4 files changed, 185 insertions(+), 6 deletions(-) diff --git a/lib/compute/derivation_engine.dart b/lib/compute/derivation_engine.dart index b6211776..ba98bafe 100644 --- a/lib/compute/derivation_engine.dart +++ b/lib/compute/derivation_engine.dart @@ -3882,6 +3882,30 @@ class DerivationEngine { /// Returns NULL for that unjudged case and a (possibly empty) list when the /// day really was judged, so [_sleepPeriods] can make the same distinction /// instead of reading "no naps returned" as "no naps happened". + /// The explicit "nap assessment unknown" envelope. + /// + /// Every abstention path must publish this, not just the detector's own + /// `!m.present` branch. `_computeDayBlocks` starts from an EMPTY bundlePatch + /// and `_attachNaps` is the only writer of `naps`, so a path that returns + /// without writing leaves the key missing entirely — and "key absent" and + /// "judged, value null" are then two different encodings of the same fact, + /// distinguishable only by HOW the abstention happened. A reader that checks + /// `bundle['naps']?['value'] == null` and one that checks + /// `bundle.containsKey('naps')` would disagree. + static void _writeUnknownNaps( + Map bundle, + String note, + ) { + bundle['naps'] = { + 'value': null, + 'count': null, + 'confidence': 0, + 'tier': 'ESTIMATE', + 'inputs_used': const [], + 'note': note, + }; + } + static List>? _attachNaps( Map bundle, Map? scMap, @@ -3895,7 +3919,10 @@ class DerivationEngine { }) { try { final n = s.length; - if (n < 60) return null; + if (n < 60) { + _writeUnknownNaps(bundle, 'too little 1 Hz data to assess naps'); + return null; + } final accel = [ for (var i = 0; i < n; i++) ana.AccelSample(s.tsSec[i] * 1000.0, s.ax[i], s.ay[i], s.az[i]), @@ -4005,6 +4032,7 @@ class DerivationEngine { ]; } catch (e) { if (kDebugMode) debugPrint('[derive] naps FAILED/skipped: $e'); + _writeUnknownNaps(bundle, 'nap detection failed for this day'); return null; } } diff --git a/lib/data/local_repository_impl.dart b/lib/data/local_repository_impl.dart index 25ee611d..d1cba12b 100644 --- a/lib/data/local_repository_impl.dart +++ b/lib/data/local_repository_impl.dart @@ -692,13 +692,21 @@ class LocalRepositoryImpl extends LocalRepository { /// the parallel fix at the other end of the seam are order-independent. Map _canonicalPeriod(Map p) { final m = p.cast(); - // Only fill what's missing — a period already speaking the current - // vocabulary passes through byte-for-byte. + // Fill only keys that are genuinely ABSENT — `containsKey`, never a null + // check. A current-schema key present with an explicit null is an honest + // "we did not measure this", and a null test cannot tell that apart from a + // missing key. On a mixed payload (`duration_min: null` sitting alongside a + // stale `asleep_min: 40`) a null test promotes an unknown into a + // measurement — the precise dishonesty this seam exists to remove. + // + // A period already speaking the current vocabulary passes through + // byte-for-byte either way. return { ...m, - if (m['onset_ts'] == null && m['start'] != null) 'onset_ts': m['start'], - if (m['wake_ts'] == null && m['end'] != null) 'wake_ts': m['end'], - if (m['duration_min'] == null && m['asleep_min'] != null) + if (!m.containsKey('onset_ts') && m['start'] != null) + 'onset_ts': m['start'], + if (!m.containsKey('wake_ts') && m['end'] != null) 'wake_ts': m['end'], + if (!m.containsKey('duration_min') && m['asleep_min'] != null) 'duration_min': m['asleep_min'], }; } diff --git a/test/nap_attribution_test.dart b/test/nap_attribution_test.dart index d25e0803..17c6a182 100644 --- a/test/nap_attribution_test.dart +++ b/test/nap_attribution_test.dart @@ -260,5 +260,91 @@ void main() { ); }, ); + + + // CodeRabbit, on the first version of this commit: the abstention paths + // disagreed about how they encode "unknown". `!m.present` wrote + // `naps.value: null`; the short-input and error paths returned without + // writing `naps` at all, so the key was simply missing. Two encodings of + // one fact, told apart only by HOW the abstention happened. + test( + 'EVERY abstention publishes the same explicit unknown envelope', + () { + final s = _daySubstrate( + startSec: midnight, + lengthSec: 30, // below the 60-sample floor + napFromSec: 0, + napToSec: 0, + ); + final bundle = {}; + expect( + DerivationEngine.debugAttachNaps(bundle, {}, s, 0, 0), + isNull, + ); + expect( + bundle.containsKey('naps'), + isTrue, + reason: 'the key must exist, not be silently missing', + ); + final naps = bundle['naps'] as Map; + expect(naps['value'], isNull); + expect(naps['count'], isNull); + expect(naps['confidence'], 0); + expect(naps['note'], isNotNull); + }, + ); + + // Also CodeRabbit: the PR threads wristOff/charging into detectNaps but + // nothing exercised either. A band on a charger is perfectly still and is + // the dominant nap false positive, so this is the guard doing real work. + test('a nap-shaped block fully inside an OFF-WRIST span is not a nap', () { + final s = _daySubstrate( + startSec: midnight, + lengthSec: 6 * 3600, + napFromSec: 2 * 3600, + napToSec: 2 * 3600 + napLen, + ); + final sc = {}; + final periods = DerivationEngine.debugAttachNaps( + {}, + sc, + s, + 0, + 0, + attributionStartSec: midnight, + attributionEndSec: midnight + 86400, + wristOff: [ + [midnight + 2 * 3600 - 60, midnight + 2 * 3600 + napLen + 60], + ], + ); + expect(periods, isNotNull, reason: 'judged — the day had data'); + expect(periods, isEmpty, reason: 'a band off the wrist is not asleep'); + expect(sc['nap_min'], 0.0); + }); + + test('a nap-shaped block fully inside a CHARGING span is not a nap', () { + final s = _daySubstrate( + startSec: midnight, + lengthSec: 6 * 3600, + napFromSec: 2 * 3600, + napToSec: 2 * 3600 + napLen, + ); + final sc = {}; + final periods = DerivationEngine.debugAttachNaps( + {}, + sc, + s, + 0, + 0, + attributionStartSec: midnight, + attributionEndSec: midnight + 86400, + charging: [ + [midnight + 2 * 3600 - 60, midnight + 2 * 3600 + napLen + 60], + ], + ); + expect(periods, isNotNull); + expect(periods, isEmpty, reason: 'a band on a charger is not asleep'); + expect(sc['nap_min'], 0.0); + }); }); } diff --git a/test/sleep_periods_legacy_keys_test.dart b/test/sleep_periods_legacy_keys_test.dart index 1d48245a..aa8b5da4 100644 --- a/test/sleep_periods_legacy_keys_test.dart +++ b/test/sleep_periods_legacy_keys_test.dart @@ -178,4 +178,61 @@ void main() { expect(sleep['total_asleep_min'], isNull); }, ); + + test( + 'an EXPLICIT null current-schema field is never back-filled from a legacy ' + 'key that does have a value', + () async { + // The mixed-payload case: the current producer recorded an honest + // "not measured", and a stale legacy value sits beside it. A null test + // (rather than containsKey) would promote 40 into a measurement -- + // exactly the dishonesty this whole seam removes. + await seed({ + 'periods': [ + { + 'is_main': true, + 'onset_ts': onset, + 'wake_ts': wake, + 'duration_min': null, // honest unknown + 'asleep_min': 40, // stale legacy value + }, + ], + 'total_asleep_min': null, + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + final periods = (sleep['periods'] as List).cast>(); + + expect( + periods.first['duration_min'], + isNull, + reason: 'unknown must stay unknown; the card renders "-"', + ); + }, + ); + + test( + 'an explicit null onset/wake is likewise preserved over legacy start/end', + () async { + await seed({ + 'periods': [ + { + 'is_main': true, + 'onset_ts': null, + 'wake_ts': null, + 'start': onset, + 'end': wake, + 'duration_min': 420, + }, + ], + 'total_asleep_min': 420, + }); + + final sleep = await repo.getDaySleep('2026-06-15'); + final periods = (sleep['periods'] as List).cast>(); + + expect(periods.first['onset_ts'], isNull); + expect(periods.first['wake_ts'], isNull); + }, + ); }