Repository navigation
Sleep: only say a night's recordings were removed when they were #688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -527,6 +527,11 @@ class _SleepDetailState extends State<SleepDetail> with RevisionReload { | |
| /// Unknown counts as correctable, which is how the screen behaved before. | ||
| bool _correctable = true; | ||
|
|
||
| /// Whether the day's recordings were actually pruned, as opposed to never | ||
| /// recorded (a fresh install, a night without the band). Only the first may | ||
| /// say "no longer on the phone"; both leave the day uncorrectable. | ||
| bool _recordingsPruned = false; | ||
|
|
||
| /// Whether the night's whole derive window is still kept. False on the | ||
| /// oldest kept day: the prune cut at its midnight took the evening half, so | ||
| /// confirming the detected times or going back to automatic would be | ||
|
|
@@ -605,10 +610,14 @@ class _SleepDetailState extends State<SleepDetail> with RevisionReload { | |
| var correctable = true; | ||
| var wholeNightKept = true; | ||
| var confirmable = true; | ||
| var recordingsPruned = false; | ||
| if (app != null && d.day != null) { | ||
| try { | ||
| correctable = | ||
| await app.sleepCorrectable(d.day!, sleepSource: 'rejected'); | ||
| if (!correctable) { | ||
| recordingsPruned = await app.sleepRecordingsPruned(d.day!); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium · Prune explanation disappears on read failure When the database cursor lookup fails after Represent a failed prune-state lookup separately from Prompt for AI agentsIn `lib/ui2/screens/sleep_detail.dart` at line 620:
**Prune explanation disappears on read failure**
When the database cursor lookup fails after `sleepCorrectable` has returned false, `recordingsPruned` stays false when the cursor read throws, and the surrounding catch at line 629 suppresses the error; the uncorrectable card then omits the removal note without indicating that the prune state could not be checked.
Represent a failed prune-state lookup separately from `false` and show an explicit unknown/check-failed explanation instead of silently omitting the note.
Comment on lines
+618
to
+620
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not use the prune cutoff as proof that recordings left the phone.
|
||
| wholeNightKept = | ||
| correctable && await app.sleepCorrectable(d.day!); | ||
| final onset = (d.night['onset_ts'] as num?)?.toInt(); | ||
|
|
@@ -617,7 +626,9 @@ class _SleepDetailState extends State<SleepDetail> with RevisionReload { | |
| onset != null && | ||
| await app.sleepCorrectable(d.day!, | ||
| sleepSource: 'confirmed', onsetSec: onset)); | ||
| } catch (_) {} | ||
| } catch (e) { | ||
| debugPrint('sleep detail: correctability read failed: $e'); | ||
| } | ||
| } | ||
| // LAST NIGHT ONLY. The card names the luteal phase, and `getCycle` knows | ||
| // today's phase and no other day's — so a card offered while stepping | ||
|
|
@@ -634,6 +645,7 @@ class _SleepDetailState extends State<SleepDetail> with RevisionReload { | |
| _lastGood = d, | ||
| _rough = rough, | ||
| _correctable = correctable, | ||
| _recordingsPruned = recordingsPruned, | ||
| _wholeNightKept = wholeNightKept, | ||
| _confirmable = confirmable, | ||
| _loading = false, | ||
|
|
@@ -738,8 +750,18 @@ class _SleepDetailState extends State<SleepDetail> with RevisionReload { | |
| // the write is refused (and said so) when it doesn't. | ||
| if (rejectedDay != null && | ||
| (!_correctable || (rejected && !_wholeNightKept))) ...[ | ||
| const SizedBox(height: S.x2), | ||
| _correctable ? _eveningPrunedNote(c, l) : _uncorrectableNote(c, l), | ||
| // Uncorrectable with nothing ever recorded (a fresh install, a | ||
| // night without the band) offers no button and says nothing more: | ||
| // the card above already says why there is no night. "No longer on | ||
| // the phone" is only true once the prune has actually run past it; | ||
| // a rejected night was recorded by definition. | ||
| if (_correctable) ...[ | ||
| const SizedBox(height: S.x2), | ||
| _eveningPrunedNote(c, l), | ||
| ] else if (rejected || _recordingsPruned) ...[ | ||
| const SizedBox(height: S.x2), | ||
| _uncorrectableNote(c, l), | ||
| ], | ||
|
Comment on lines
+761
to
+764
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium · Changed messaging lacks coverage When the suite is used to verify the changed messaging behavior, the new UI tests do not exercise either conditional note branch: the detail-page fixture supplies Add widget tests that load the screen through AppState and verify the note is shown or omitted in both changed branches. Also at Prompt for AI agentsIn `lib/ui2/screens/sleep_detail.dart` at lines 759-762:
**Changed messaging lacks coverage**
When the suite is used to verify the changed messaging behavior, the new UI tests do not exercise either conditional note branch: the detail-page fixture supplies `SleepDetail(data: d)` directly, while the added test checks only the database helper. A regression in either branch can therefore pass the test suite unnoticed.
Add widget tests that load the screen through AppState and verify the note is shown or omitted in both changed branches.
Also at `lib/ui2/screens/sleep_detail.dart:1067`.From `readiness_baseline_pollution_test`). A behavior change with no accompanying
test is a real finding — CI passing doesn't mean the right test exists. |
||
| ] else if (rejected) ...[ | ||
| const SizedBox(height: S.x2), | ||
| TextButton( | ||
|
|
@@ -1040,8 +1062,12 @@ class _SleepDetailState extends State<SleepDetail> with RevisionReload { | |
| ), | ||
| ], | ||
| const SizedBox(height: S.x2), | ||
| // No buttons when uncorrectable; the note says why: pruned | ||
| // recordings, or (imported, never worn) nothing to re-analyse. | ||
| if (!_correctable) | ||
| _uncorrectableNote(c, l) | ||
| _recordingsPruned | ||
| ? _uncorrectableNote(c, l) | ||
| : _noRecordingsNote(c, l) | ||
| else | ||
| Wrap(spacing: S.x2, children: [ | ||
| if (fallback && _confirmable) | ||
|
|
@@ -1098,6 +1124,14 @@ class _SleepDetailState extends State<SleepDetail> with RevisionReload { | |
| style: F.cap.copyWith(color: P.of(c).ink3), | ||
| ); | ||
|
|
||
| /// An uncorrectable night whose recordings were never here to prune. | ||
| Widget _noRecordingsNote(BuildContext c, AppLocalizations? l) => Text( | ||
| l?.sleepDetailCorrectionNoRecordings ?? | ||
| 'This night has no detailed recordings on the phone, so its sleep ' | ||
| 'can\u2019t be corrected.', | ||
| style: F.cap.copyWith(color: P.of(c).ink3), | ||
| ); | ||
|
|
||
| /// On the oldest kept night: says why only some corrections are offered. | ||
| Widget _eveningPrunedNote(BuildContext c, AppLocalizations? l) => Text( | ||
| l?.sleepDetailCorrectionEveningPruned ?? | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| // The Sleep screen used to tell a fresh install, and any night the band was | ||
| // never worn, that "this night's detailed recordings are no longer on the | ||
| // phone". Both are uncorrectable because there is no substrate, but only a | ||
| // day the raw prune has actually cut may say its recordings were removed. | ||
| // `LocalDb.decodedPrunedFor` is that distinction. | ||
|
|
||
| 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/day_label.dart'; | ||
| import 'package:openstrap_edge/data/db.dart'; | ||
|
|
||
| void main() { | ||
| setUpAll(() async { | ||
| sqfliteFfiInit(); | ||
| databaseFactory = databaseFactoryFfi; | ||
| LocalDb.dbName = 'openstrap_sleep_recordings_pruned_test.db'; | ||
| await databaseFactory.deleteDatabase( | ||
| p.join(await databaseFactory.getDatabasesPath(), LocalDb.dbName), | ||
| ); | ||
| }); | ||
|
|
||
| tearDownAll(() async { | ||
| await LocalDb.close(); | ||
| await databaseFactory.deleteDatabase( | ||
| p.join(await databaseFactory.getDatabasesPath(), LocalDb.dbName), | ||
| ); | ||
| }); | ||
|
|
||
| setUp(() async { | ||
| await (await LocalDb.instance) | ||
| .delete('sync_cursor', where: 'name = ?', whereArgs: [ | ||
| LocalDb.kDecodedPrunedBeforeCursor, | ||
| ]); | ||
| }); | ||
|
|
||
| test('nothing pruned yet (a fresh install): no day counts as pruned', | ||
| () async { | ||
| expect(await LocalDb.decodedPrunedFor(todayLabel()), isFalse); | ||
| expect(await LocalDb.decodedPrunedFor('2020-01-01'), isFalse); | ||
| }); | ||
|
|
||
| test('days before the cut are pruned; the cut day and later are not', | ||
| () async { | ||
| // The prune only ever cuts at a local midnight. | ||
| await LocalDb.setCursor(LocalDb.kDecodedPrunedBeforeCursor, | ||
| '${localDayStartSec('2026-06-10')}'); | ||
|
|
||
| expect(await LocalDb.decodedPrunedFor('2026-06-09'), isTrue); | ||
| expect(await LocalDb.decodedPrunedFor('2026-06-10'), isFalse); | ||
| expect(await LocalDb.decodedPrunedFor('2026-06-11'), isFalse); | ||
| }); | ||
|
|
||
| test('an imported day before the cut never had recordings to prune', | ||
| () async { | ||
| await LocalDb.setCursor(LocalDb.kDecodedPrunedBeforeCursor, | ||
| '${localDayStartSec('2026-06-10')}'); | ||
| final db = await LocalDb.instance; | ||
| await db.insert('metric_series_version', | ||
| {'date': '2026-06-08', 'algo_version': 1, 'source': 'whoop_csv'}); | ||
| addTearDown(() => db.delete('metric_series_version')); | ||
|
|
||
| expect(await LocalDb.decodedPrunedFor('2026-06-08'), isFalse); | ||
| expect(await LocalDb.decodedPrunedFor('2026-06-09'), isTrue); | ||
| }); | ||
|
|
||
| test('a malformed day label is never reported as pruned', () async { | ||
| await LocalDb.setCursor(LocalDb.kDecodedPrunedBeforeCursor, | ||
| '${localDayStartSec('2026-06-10')}'); | ||
|
|
||
| expect(await LocalDb.decodedPrunedFor('not-a-day'), isFalse); | ||
| }); | ||
|
|
||
| test('a day straddling the cut still has recordings, so is not pruned', | ||
| () async { | ||
| final mid = localDayStartSec('2026-06-09')! + 12 * 3600; | ||
| await LocalDb.setCursor(LocalDb.kDecodedPrunedBeforeCursor, '$mid'); | ||
|
|
||
| expect(await LocalDb.decodedPrunedFor('2026-06-09'), isFalse); | ||
| expect(await LocalDb.decodedPrunedFor('2026-06-08'), isTrue); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium · Pruned night note is hidden
When the cutoff equals the displayed day’s midnight and the night’s derive window includes the previous evening,
decodedPrunedForcompares the calendar-day start with the cutoff and returns false at equality, even though the night’s earlier recordings were pruned. The Sleep screen hides the removal note for an uncorrectable night.Check the displayed night’s derive window against the prune state rather than comparing only the calendar-day start.
Also at
lib/ui2/screens/sleep_detail.dart:1067.Prompt for AI agents