Skip to content

sleep: list deleted nights on iOS and macOS, with recompute and hide - #2660

Open
UtkuDenizAltiok wants to merge 2 commits into
ryanbr:mainfrom
UtkuDenizAltiok:ios-deleted-sleep
Open

UtkuDenizAltiok wants to merge 2 commits into
ryanbr:mainfrom
UtkuDenizAltiok:ios-deleted-sleep

Conversation

@UtkuDenizAltiok

@UtkuDenizAltiok UtkuDenizAltiok commented Oct 3, 2026 •

Copy link
Copy Markdown

What this PR does

On iOS and macOS, a deleted night could only be brought back from the undo strip, which lasts seven seconds. After that, a night deleted by mistake was gone for good: the deletion marker keeps the detector from finding it again, and nothing on screen could lift it. Android has had a persistent list since #526 and #679. This brings the same card to the shared SleepView:

  • Deleted sleep windows: each deleted night whose marker still stands, newest first, "1 Oct, 00:15 – 1 Oct, 07:28".
  • Recompute this night lifts the marker (Repository.allowSleepReDetection, which already existed with no caller) and runs the normal scan of the most recent 21 days. The card names that limit and says an imported night without raw samples may not return; the success note reports only "Deletion marker cleared."
  • Hide takes the row off the list and keeps the marker, so a sleep deleted on purpose stays deleted.

Android stores "hidden" as the managementVisible column of dismissedSleep. On Apple platforms the markers live in the defaults (sleep.dismissedSessions), so the hidden ones are a second token list beside them (sleep.dismissedSessions.hiddenFromList). DeletedSleepList (pure, tested) keeps it to markers that still stand. Lifting a marker clears its hidden entry, whether by recompute or undo. A fresh deletion also unhides its window, including when an old hidden token outlived a marker evicted by the 500-marker cap. The detector reads only the markers, so hiding never changes what is detected.

The card, and the undo strip, also show on the empty screen. That is where deleting the only recorded night lands, and nothing there could bring it back before.

Strings: Android's, in all nine locales. In Polish and Portuguese, Android's versions use the computer sense of "sleep" ("uśpienie", "suspensão"), and one Portuguese line leaves "sleep" in English. Those are corrected here to the words the iOS catalog already uses ("sen", "sono"). Android's own strings are untouched.

Apple shows two success notes: marker cleared and row hidden. Android also has two failure toasts because its DAO reports an affected-row count that can be zero. Apple rewrites defaults arrays for these actions, so it has no matching row-count failure to report.

Type of change

  • New feature (parity with Android)

How it was tested

  • Tests (DeletedSleepListTests, 5): list order and malformed markers; hiding keeps suppression; hidden-list pruning; reopening unhides; and the actual Repository.deleteSleepSession path unhides both a standing marker and a hidden token whose marker was evicted by the 500-marker cap. The last test fills the real capped list, evicts the oldest hidden night, makes room, re-deletes it, then checks a repeated delete too. On the pre-fix code its four visibility/unhide assertions fail; the marker and stored-row checks still pass.
  • Simulator, 6 Oct (fresh iPhone 17 Pro, iOS 27.0, synthetic demo data): seeded a visible 50-day-old deletion marker and an orphaned hidden token matching the newest night. The card names the 21-day scan limit. Recompute on the old window removed its marker and showed only "Deletion marker cleared." Then the real Edit → Delete flow on the newest synthetic night produced a visible row after the undo strip expired; persisted defaults held its marker and no hidden token.
  • Full local run on e9ee598e, rebased onto current upstream 9f98f811: WhoopStore 632 · StrandAnalytics 2135 · StrandImport 327 (one existing skip) · doc lint · i18n · parity ledger · parity ratchet · parity governance 127 · macOS StrandTests 2,272 (two existing skips, zero failures on this Mac) · iOS build. Both new keys are in compiler SleepView.stringsdata and all ten built macOS/iOS Localizable.strings files. Every upstream catalogue entry is unchanged.

Checklist

  • No new build warnings introduced
  • UI changes use only StrandDesign tokens — no hardcoded colors, fonts, or spacing
  • Follows the conventions in docs/CONTRIBUTING.md
  • I did not commit generated output (Strand.xcodeproj/) or any secrets/keystores

Related issues

Refs #515

UtkuDenizAltiok added a commit to UtkuDenizAltiok/noop that referenced this pull request Oct 3, 2026
…2661 opened, 3772b93 shipped, findings, README Handover for a ChatGPT engineer
@ryanbr

ryanbr commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Thanks for this, @UtkuDenizAltiok. Careful work, and the write-up made it quick to check. One defect worth addressing before merge, one wording point, and a trivial rebase.

Worth noting first: this wires up two functions that were sitting on main with no callers at all. I confirmed on origin/main that allowSleepReDetection and dismissedSleepManagementWindows each had exactly one hit, their own definition. An escape hatch that nothing could reach was the real gap here.

Verified clean

  • All ten new catalogue keys carry all ten locales (de en es fr it pl pt-PT ru zh-Hans zh-Hant), which matches the catalogue's own locale set exactly, and they are inserted as single lines so nothing reflows.
  • Hiding provably never changes what the detector sees: the engine reads dismissedSleepWindows, which ignores the hidden list.
  • A freshly deleted night does appear in the card. The load is .task(id: repo.refreshSeq) and both the delete and the undo paths call repo.refresh(), so the list follows without needing its own invalidation.
  • Only two production routes called DismissedSleepSpans.removing, and this adds an unhide to both.
  • Group { if ... Text() else Text() } rather than a ternary is a real SwiftUI trap sidestepped, and the comment explaining why is the right thing to leave behind.
  • Only conflict against current main is Localizable.xcstrings, from the translation work that landed yesterday. Nothing structural.

1. An orphaned hidden entry can leave a night deleted with no row to reopen it

There is a third route by which a marker disappears, besides the two that now unhide. DismissedSleepSpans.adding ends in capped(), which drops the oldest marker by end-time once the list passes hardCap = 500. Nothing unhides on that path, so the hidden entry outlives its marker. visible ignores an orphan and hiding prunes opportunistically, so it stays invisible until the same window is deleted a second time, at which point the fresh row is hidden on arrival.

I built a harness over the real DismissedSleepSpans.swift plus DeletedSleepList to check this rather than argue it. All seven assertions in DeletedSleepListTests reproduce there. Then, with the marker list brought back under the cap first so the second delete actually sticks:

hidden after the user hides that row: ["1000:1050"]
after one more delete -> oldest marker evicted: true; hidden still names it: true
marker stored after re-delete:   true
night is suppressed again:       true   <- it IS deleted
night appears on the list:       false  <- but there is no row to reopen it

That is the invariant testReopeningClearsItsHiddenEntry names in its own doc comment: "a night reopened and later deleted again comes back on the list, not hidden from the start". It holds for the undo and reopen routes and not for this one.

It needs roughly 500 tombstones to reach, so this is narrow rather than urgent. The cause is structural though, and worth naming: Apple keeps one fact in two independently mutated lists, where Android's managementVisible is a column on the row and cannot outlive it. The smallest repair is an unhide in the delete path beside the adding call at Repository.swift:1620-1621, which also reads correctly on its own terms: deleting a night should always put it on the list.

2. The recompute note asserts more than the pass can deliver

recomputeDeletedSleep calls analyzeRecent(), whose default is maxDays: 21, and then shows "Sleep detection reran using the data available for this night." unconditionally. Markers are permanent and capped at 500, so rows much older than 21 days are listable. For one of those the marker is lifted, the row leaves the card, the night is not re-detected, and the note says detection reran for it.

Kotlin's analyzeRecent defaults to 21 as well, and the Android cleared flag reports only whether the marker went, not whether the night returned, so this mirrors Android faithfully and is not a regression you introduced. It is still the "a diagnostic may only assert what it can attribute" shape from AGENTS.md, and the card already hedges the import-with-no-raw-samples case one line below, so extending that hedge (or passing a maxDays that reaches the night's own date) would finish the thought.

Not a finding, but worth stating as a decision

Android shows four toasts here and this ports the two success halves. That looks defensible to me: Android needs its two failure strings because the DAO write returns a row count that can be zero, whereas on Apple both the hide and the reopen are pure array rewrites into the defaults with no failure to report. Worth a line in the description so it reads as a choice rather than an oversight.

Happy to take this once the first point is addressed, in whatever form you prefer. A rebase is needed for the catalogue regardless.

UtkuDenizAltiok and others added 2 commits October 6, 2026 15:37
A deleted night keeps a marker so the detector does not bring it back, and
the undo strip lasts seven seconds. After that a night deleted by mistake
could not come back on Apple platforms. Android has had a persistent list
since ryanbr#526/ryanbr#679; this brings the same card to the shared SleepView:

- each deleted night, newest first, "d MMM, HH:mm – d MMM, HH:mm";
- "Recompute this night" lifts the marker and re-scores, so the detector
  finds the night again in the raw data (a footnote says an imported night
  with no raw data may not return);
- "Hide" takes the row off the list and keeps the marker, so a sleep
  deleted on purpose stays deleted. Android stores that as the
  managementVisible column; here the markers live in the defaults, so the
  hidden ones are a second token list beside them, pruned to markers that
  still stand and cleared when a marker is lifted (recompute or undo).

The card and the undo strip also show on the empty screen, which is where
deleting the only recorded night lands. Strings reuse Android's, in all nine
locales, with the computer-sense "sleep" in pl/pt-PT corrected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@UtkuDenizAltiok

Copy link
Copy Markdown
Author

Addressed in e9ee598, after rebasing onto 9f98f81. A fresh delete now clears its hidden entry beside the marker write. The new test uses the actual Repository delete path: it reproduces the 500-marker eviction case, makes room, re-deletes that window, and checks a repeated delete too. Its four visibility/unhide assertions fail before the fix and pass afterwards.

Recompute now reports only "Deletion marker cleared." The card states that the scan covers the most recent 21 days and retains the raw-sample caveat. I kept the normal scan window rather than expanding one click into a full-history pass. The description also explains the two success notes: Apple rewrites defaults arrays, while Android's additional failure toasts correspond to its DAO row counts. The catalogue was merged by key; all upstream entries are unchanged.

Full local verification passed, including 2,272 macOS tests (two existing skips, zero failures on this Mac) and the iOS build. The simulator also showed a fresh deleted row after an orphaned hidden entry and the undo strip expired, and the older-window action showed the revised note. Both new messages are compiled in all ten locales on both Apple targets.

DX23876 added a commit to DX23876/noop that referenced this pull request Oct 7, 2026
A deleted night could only come back through the seven-second undo strip;
after that the deletion marker kept the detector from finding it again for
good. The Sleep screen now lists each deleted night whose marker still
stands. "Recompute this night" lifts the marker (allowSleepReDetection) and
rescans the recent 21 days; "Hide" takes the row off the list and keeps
the marker, so a night deleted on purpose stays deleted. The undo strip
and the list also show on the empty screen, where deleting the only night
lands.

From upstream PR ryanbr#2660 by @UtkuDenizAltiok, fitted to the fork's
Sleep screen: the list sits under the undo strip of the reworked layout,
loads with the fork's night task, and the recompute goes through the
dashboard app model without day reuse, since lifting a marker changes no
raw input a fingerprint would notice. The fork's multi-night undo also
unhides every restored night.

Analysis migration required: no
@UtkuDenizAltiok

Copy link
Copy Markdown
Author

Hey @ryanbr, the changes from your review are in and all checks are passing. This would be a useful safety net for iPhone and Mac users: if someone deletes a night by mistake and misses the short undo window, they still have a way to lift the deletion and try detecting it again. The card also explains the scan limit. I’d appreciate another look when you have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants