Skip to content

android: defer morning recap until recorded wake - #2532

Open
kavemang wants to merge 2 commits into
ryanbr:mainfrom
kavemang:fix/morning-recap-wake-timing-2289
Open

kavemang wants to merge 2 commits into
ryanbr:mainfrom
kavemang:fix/morning-recap-wake-timing-2289

Conversation

@kavemang

@kavemang kavemang commented Sep 27, 2026 •

Copy link
Copy Markdown

Refs #2289 and #567.

Defers the Android morning recap until the recorded main sleep has ended and settled, preserving the once-per-reported-night marker, morning floor and mutex. Score-only imports with no sleep session retain the 05:00 fallback. The periodic fallback remains independent of an open Today screen or a database-write callback.

The refresh worker now checks the opt-in preference, earliest morning minute and already-reported day before collecting merged repository days. It resolves logical and calendar keys from one time snapshot; the marker shortcut applies only when both candidate day keys agree, preserving the local-night rollover case. Normal ineligible runs do no repository work and emit no log.

Refresh exceptions now produce a warning with the error. Coroutine cancellation propagates without a failure log. The existing periodic cadence and completion result are retained, so a failure waits for the next periodic opportunity rather than adding a burst of retries. Existing notifier wake-settle and duplicate-post protection remain authoritative.

Validation: 63 private actual-worker cases passed with Android/WorkManager stubs and an injected clock, covering zero-read guards, morning boundaries, day markers, empty sleep, repository failure logging and cancellation propagation. All 23 existing ScheduledReportPolicy tests passed using the production policy. Comment hygiene, diff-scoped i18n and whitespace passed. Guarded parity regeneration passed (289 known findings); all four expected checks passed at 565d47ff0fb305f653177696ec1c2dc85102fb15: Android APK build and full unit suite, Tools Python, hygiene and i18n. The complete four-check roster and unchanged head were rechecked. The private harness does not establish Android/WorkManager runtime behavior or compile the full app.

No Android device notification timing was tested. WorkManager delivery is best effort and may run later than the 15-minute period.

Keep the early-morning fallback for score-only nights and recheck deferred recaps from WorkManager when no new data arrives. Reuse the existing per-night notification marker across the UI and worker.

Refs #2289
@kavemang
kavemang force-pushed the fix/morning-recap-wake-timing-2289 branch from 6ab5600 to 44ad3e6 Compare September 28, 2026 20:28
@ryanbr

ryanbr commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Thanks @kavemang. The behaviour is right and the guards are right: onMorning already refuses a second post for the same night through the persisted reportMorningDay marker (#567), holds the earliest-morning gate, and takes the mutex before shouldNotifyMorning. So deferring until a recorded wake cannot produce a duplicate recap, and that was the thing I checked first.

One question about the mechanism rather than the outcome.

A 15-minute periodic worker for a once-daily event

PeriodicWorkRequestBuilder<MorningRecapRefreshWorker>(15, TimeUnit.MINUTES) is Android's minimum interval, so this is roughly 96 runs a day, every day, for a notification that posts once. It is scheduled from NoopApplication.onCreate and only cancelled when the preference goes off, so it keeps running long after the day's recap has posted.

The guards in onMorning do not spare the work, because the worker does its own reads before reaching them:

val days = app.repository.recentDaysMergedFlow(deviceId).first()
val row = resolveTodayRow(days, logicalDayKeyNow(), LocalDate.now().toString())
if (row?.totalSleepMin != null) { ScheduledReportNotifier.onMorning(...) }

So each of those ~96 runs collects a merged-days flow and resolves a row, and about 95 of them can do nothing useful: either it is not yet morning, or the recap for that night has already gone out. On a phone that is also holding a BLE link and running the connection service, that is a cost users of this app do notice; drain reports are a recurring thread here.

Your reasoning for polling is sound, and I would keep it rather than coupling the recap to a database write or an open Today screen. It is the unbounded duration I would change. Any of these keeps the decoupling:

  • cancel the unique work once reportMorningDay has been set for the night, and re-arm it at the next local midnight;
  • or check the cheap conditions first, the preference and nowMinuteOfDay >= EARLIEST_MORNING_MINUTE and reportMorningDay != today, and return before touching the repository;
  • or bound the periodic work to the morning window rather than the whole day.

The second is the smallest and gets most of it: the same polling shape, but the expensive half only runs while a recap is actually possible.

Smaller note

runCatching { ... }.getOrElse { Result.success() } reports success on every failure, so a worker that can never complete looks identical to one that had nothing to do. Given this path is the fallback for a recap that would otherwise be missed, a line on the failure branch would be worth having when someone reports a recap that never arrives.

Refs #567

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