Skip to content

fix(sleep): announce meaningful displayed result changes - #2544

Merged
ryanbr merged 1 commit into
ryanbr:mainfrom
fadkeprasad:codex/sleep-result-change
Sep 29, 2026
Merged

ryanbr merged 1 commit into
ryanbr:mainfrom
fadkeprasad:codex/sleep-result-change

Conversation

@fadkeprasad

Copy link
Copy Markdown

What this PR does

Replacement for the rejected #2530 approach, following the review's request to notify only when the displayed night actually changes. Refs #2108.

The comparison lives in the Sleep screen's presentation state on Apple and Android. It compares fully loaded results for the same displayed wake-day, active strap, and source set after scoring settles. Global computation alone cannot show the notice; the existing freshness priority remains unchanged.

A notice appears when onset, wake time, or total asleep changes by at least five minutes. Smaller shifts accumulate against the last announced baseline; stage redistribution without a meaningful total/boundary change stays quiet. Five minutes is a UX threshold, not an accuracy claim.

The notice says “Sleep result updated” and disappears after eight seconds. Opening the screen, navigating, changing strap/source/day, and manually edited nights establish a baseline without an announcement. State is view-local and does not retrospectively notify about changes made while the screen was closed.

The Apple reload publishes its blocks, learned timing, and motion together after cancellation checks. Android tracks which day/strap snapshot finished loading and checks cancellation before publishing it. Both platforms ignore intermediate scoring results.

Both strings are explicitly included in the Apple catalog (all ten locale entries) and Android resources (default plus eight translations).

Type of change

  • Bug fix

How it was tested

  • macOS app build and StrandTests/SleepResultChangeTests: passed.
  • iOS app build with signing disabled: passed.
  • Android FullDebug compilation and targeted tests under JDK 17: passed after a clean build (9 tests: the comparison oracle plus the 8 existing freshness tests). An incremental rerun initially hit the existing missing generated Room-schema directory; clean regeneration resolved it.
  • Standalone Swift oracle: 16 ordered cases, copied verbatim into the Kotlin expected results. Covers initial/unchanged results, sub-threshold and cumulative shifts, onset/wake/total changes, decreases, in-progress results, scope changes, manual edits, and empty/reset behavior.
  • Localization audit, explicit new-key locale coverage, doc-comment lint, and diff check: passed.

No real-device UI/sync session was performed for this presentation change. The eight-second notice lifecycle is implemented on both platforms; its visual timing still needs device review. No detector, BLE, persistence, or repair flags change.

Checklist

  • Targeted Swift and Android regression tests pass
  • Existing freshness priority and tests retained
  • Existing design components used
  • Both new strings translated on Apple and Android
  • No generated project output or secrets committed

Related issues

Refs #2108. Supersedes the presentation approach in #2530; independent of the history-repair PR #2375.

@fadkeprasad
fadkeprasad force-pushed the codex/sleep-result-change branch from bf4de4c to 8f572f0 Compare September 28, 2026 13:11
@ryanbr

ryanbr commented Sep 29, 2026

Copy link
Copy Markdown
Owner

@fadkeprasad This is good work and I have no blocking objection. Checks below are things I ran or
traced, not things I read in your summary.

The oracle is the strongest part, and it pins more than it looks like it does. I walked all sixteen
steps by hand rather than accepting that the two platforms agreeing means either is right, and the
expected vector is correct. What makes it a real test is the boundary pairs: step 4 at 299/299/4.99
expects false and step 5 at exactly 300 expects true, so the threshold is nailed from both sides on
both fields. Step 7 and step 8 carry identical values across the ready flag, so removing that gate
breaks it. Step 13 would flip true if the edited check went away. And step 5 only reaches 300 by
accumulating from step 1, so an implementation that reset the baseline on every observe would see a
one-second delta and fail. Six behaviours, each pinned by a case that goes red if it regresses. That is
rarer than it should be.

Localization is complete, and it actually takes effect. All nine Android values-* files and all
ten Apple locale entries, which I re-counted against main rather than trusting the number. The part
worth stating out loud is that DataPendingNote declares title and message as LocalizedStringKey,
so the bare literals at the call site really do resolve through the catalog. Had those parameters been
String, the ten locale entries would have been dead weight and nothing would have failed to tell you.
The i18n audit is clean, and the pre-existing ru missing=1 and zh-Hant missing=79 in that catalog
are not yours.

No authority drift, which I verified rather than assumed. Strand/ and com/noop/ui/** both sit
outside the ledger's scanned roots, so the two new files and their declarations cost nothing. Rebased
onto current main the repository-acceptance suite is 50/50 OK. Worth knowing why that mattered: your
CI skipped its "Merge current main" step, so those six green checks describe your base, not today's.
On that rebase I also reran your Kotlin oracle under a forced recompile, and checked the result XML's
timestamp against the clock rather than reading BUILD SUCCESSFUL: 1 test, 0 failures, genuinely
executed. Your Swift oracle did run on your own CI, inside "Test Strand" on the macOS build leg.

The reload guard deserves its own mention. Publishing blocks, learned timing and motion together
behind !Task.isCancelled and a refresh == repo.refreshSeq recheck, and the Android
ensureActive() placed outside the runCatching so a swallowed cancellation still stops the write,
both remove a real interleaving where a superseded pass could leave mixed-generation state on screen.
That is a robustness fix riding along under a notice PR, and your summary undersells it.

Two notes, neither blocking

The shared oracle stops exactly where the platforms start differing. It covers
SleepResultChangeTracker, which is line for line equivalent, so it is the part least likely to
diverge. What is untested on both sides is snapshot construction, and that is where they genuinely
differ: Android takes heroOnsetTs/heroWakeTs, derived from the displayed group as
heroGroup.firstOrNull() and heroGroup.maxOfOrNull { it.endTs }, while Apple reads
night.session.effectiveStartTs and endTs. I checked before calling that a defect and it is not one,
because Apple's screen displays the session values too, so each side tracks what its own platform
shows. But on a grouped or split night those are not the same quantity, and "copied verbatim into the
Kotlin expected results" reads like a cross-platform guarantee that in fact stops at the tracker
boundary. A case per platform over a grouped night would put that where the oracle cannot.

#2108 asked for two things and this is one of them. The reporter wanted the persistent "calculating"
to stop, and wanted a mention when something changes. This builds the second. Your body is explicit
that freshness priority is unchanged and says Refs rather than claiming more, so nothing is
overstated, but the symptom that prompted the report is untouched and that issue should stay open.
Worth being deliberate about it, because on a screen whose complaint was too many standing messages,
the first thing this adds is another message, even one that clears itself after eight seconds.

The eight-second lifecycle still wants a device, as you say. I am not treating that as a gate for a
presentation change that is correct on both platforms and reverts to silence on its own.

@ryanbr
ryanbr merged commit f70de92 into ryanbr:main Sep 29, 2026
6 checks passed
@fadkeprasad

Copy link
Copy Markdown
Author

Thanks Ryan for checking the oracle cases and reload guards directly, and for merging this. Agreed on both limits: the shared oracle proves the tracker’s comparison rules, not equivalence of Apple session values and Android grouped-night snapshot construction. Grouped/split-night snapshot coverage and the eight-second device lifecycle remain follow-up checks.

The cancellation/refresh guard is also a robustness fix: it keeps a superseded reload from publishing mixed-generation screen state. And #2108 should stay open for its persistent calculating-message complaint; this change only supplies the transient notice when the displayed result changes.

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