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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -4546,6 +4546,20 @@ class LocalDb { | |
| return v == null ? null : int.tryParse(v); | ||
| } | ||
|
|
||
| /// Whether the raw prune has already deleted [day]'s decoded recordings: | ||
| /// the day starts before the [kDecodedPrunedBeforeCursor] cut, which the | ||
| /// prune only ever sets at a local midnight. False when nothing has been | ||
| /// pruned yet, so "no recordings" on a fresh install or an unworn night is | ||
| /// not mistaken for "deleted", and false for a day that was imported. | ||
| static Future<bool> decodedPrunedFor(String day) async { | ||
| final cut = await getCursorInt(kDecodedPrunedBeforeCursor); | ||
| final start = localDayStartSec(day); | ||
| if (cut == null || start == null || start >= cut) return false; | ||
| // An imported day sits before the cut without ever having had recordings | ||
| // here, so there was nothing for the prune to remove. | ||
| return !(await importedDates()).contains(day); | ||
| } | ||
|
Comment on lines
+4554
to
+4561
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 · Pruned night note is hidden When the cutoff equals the displayed day’s midnight and the night’s derive window includes the previous evening, Check the displayed night’s derive window against the prune state rather than comparing only the calendar-day start. Also at Prompt for AI agentsIn `lib/data/db.dart` at lines 4554-4558:
**Pruned night note is hidden**
When the cutoff equals the displayed day’s midnight and the night’s derive window includes the previous evening, `decodedPrunedFor` compares 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`. |
||
|
|
||
| /// The primary device's durable record frontier (`rec_ts_hw`), or null when | ||
| /// no record has been banked. Builds before the commit guard could persist | ||
| /// '0' from a record-less commit, so a non-positive value reads as unset. | ||
|
|
||
| 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(); | ||
|
|
@@ -634,6 +643,7 @@ class _SleepDetailState extends State<SleepDetail> with RevisionReload { | |
| _lastGood = d, | ||
| _rough = rough, | ||
| _correctable = correctable, | ||
| _recordingsPruned = recordingsPruned, | ||
| _wholeNightKept = wholeNightKept, | ||
| _confirmable = confirmable, | ||
| _loading = false, | ||
|
|
@@ -738,8 +748,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
+759
to
+762
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 +1060,11 @@ class _SleepDetailState extends State<SleepDetail> with RevisionReload { | |
| ), | ||
| ], | ||
| const SizedBox(height: S.x2), | ||
| // No buttons either way when uncorrectable; the note only when the | ||
| // prune really removed this night's recordings (not, say, a night | ||
| // imported from elsewhere that never had any here). | ||
| if (!_correctable) | ||
| _uncorrectableNote(c, l) | ||
| _recordingsPruned ? _uncorrectableNote(c, l) : const SizedBox.shrink() | ||
|
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. Explain why a nonpruned scored night cannot be corrected. If a scored imported night has no local substrate and no prune cutoff,
|
||
| else | ||
| Wrap(spacing: S.x2, children: [ | ||
| if (fallback && _confirmable) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| // 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); | ||
| }); | ||
| } |
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 · Invalid dates are accepted
When a numeric date label has out-of-range month or day components and its normalized date falls before the cutoff,
localDayStartSecnormalizes invalid components, sodecodedPrunedForcan treat an impossible date as pruned instead of returning false.Validate the date components in
localDayStartSecand reject labels that do not form a real calendar date.Prompt for AI agents