fw/services/activity: keep short sleep cycles inside a fragmented night#1742
Conversation
The Kraepelin sleep detector discarded any sleep cycle shorter than min_sleep_cycle_len_minutes (60), so when brief mid-night restlessness split a night into fragments, a genuinely slept 20-59 minute chunk displayed as "awake". In the reported incident the watch detected sleep (including 37 minutes of deep sleep) between two recorded sessions and then threw it away, showing the user awake for over an hour of real sleep. The only trace was the misleading "Session to delete not found" warning, which fires because rejected short cycles were never registered. Short cycles were doubly doomed: the running avg-VMC/active-pct checks include the trailing run of awake minutes, and vmc_clip cannot stop that movement from dominating the averages of a short cycle (14 clipped minutes push a 45-minute cycle's average over the threshold on their own), so such cycles were usually rejected before the length check even ran. Keep a short cycle when it is sandwiched inside a night: - Defer the average-based rejections for a cycle that starts within max_adjacent_cycle_gap_m (30) of the previous cycle's end while it is under 60 minutes, so it survives to end-of-cycle validation. - At validation, re-apply those checks over the slept portion only (excluding the trailing awake run). If the cycle slept at least min_adjacent_cycle_len_m (20) and passes, hold it back as a pending cycle together with the deep sleep sessions found inside it. - Register the pending cycle only once a following cycle is kept and starts within the same gap of its end; otherwise drop it. Requiring sleep on both sides keeps post-wake stillness (dozing or reading in bed at the end of the night) from extending sleep, which the captured reference nights sleep_7, sleep_8, pbl_27921 and pbl_37734 guard. The pending cycle is registered when the follower reaches ongoing registration (60 minutes); a follower rejected after that point leaves the pending cycle registered. Accepted: the pending cycle passed the worn and quality checks itself, deleting an already-registered non-ongoing session would trip the ongoing-only assert in activity_sessions.c, and registering later would break the chronological session order the health UI relies on. All captured reference nights pass unchanged except sleep_4, which gains a 23-minute interior cycle that the old code discarded - the exact shape of the reported bug, on real data. New synthetic-night tests cover the kept mid-night cycle, nightstand rejection, far-gap rejection, tiny-cycle rejection and chained fragments. Fixes FIRM-3473 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Eric Migicovsky <eric@repebble.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdae4ae315
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // The deep sleep sessions were captured into the pending cycle; abort the live ones | ||
| prv_deep_sleep_update(alg_state, sample_utc, score, KAlgDeepSleepAction_Abort, | ||
| false /*ok_to_register*/, sessions_cb, context); | ||
| state->last_cycle_end_utc = sleep_end_time; |
There was a problem hiding this comment.
When a short cycle is held back, last_cycle_end_utc is advanced to that unconfirmed cycle. If that pending cycle is later dropped in prv_resolve_pending_cycle because the next candidate is rejected or too far away, this timestamp is not restored, so a subsequent short fragment that is within 30 minutes of the dropped fragment can still pass may_continue_night even though it no longer follows accepted sleep. This admits exactly the kind of post-wake/tiny/not-worn fragmentation the sandwich rule is trying to exclude; the accepted-cycle end and pending-cycle end need to be distinguished or the anchor reset when pending is forgotten.
Useful? React with 👍 / 👎.
|
@jplexer speculative fix - i'm not really sure how to test this? |
1 participant
Fixes FIRM-3473
What changed
The Kraepelin sleep detector discarded any sleep cycle shorter than 60 minutes, so brief mid-night restlessness that split a night into fragments made genuinely-slept 20–59 min chunks display as "awake". Short cycles were doubly doomed: the running avg-VMC / active-pct checks include the trailing awake run, and
vmc_clipcan't stop that movement from dominating a short cycle's averages (14 clipped minutes alone push a 45-min cycle over the threshold), so they usually died before the length check even ran.Now a short cycle is kept when it is sandwiched inside a night:
Why
Deep dive on FIRM-3473 (getafix, fw v4.27.0): overnight the firmware recorded two sleep sessions with a 71-min "awake" gap covering the disputed 3:31 AM. The logs show the algorithm detected sleep in that gap — including a 37-min deep-sleep run — then rejected the cycle (
Session to delete not foundat 10:59:01Z is the rejection's only trace; that warning fires exactly when a never-registered <60-min cycle is discarded). Full analysis in the Linear issue.How I validated
./waf test -M ".*kraepelin.*"— all captured reference nights pass unchanged exceptsleep_4, which gains a 23-min interior cycle the old code discarded (the exact reported bug shape, on real data). The post-wake phantom-sleep guard nights (sleep_7,sleep_8,pbl_27921,pbl_37734,watch_on_table*) are all bit-identical to baseline.qemu_emery; firmware boots to launcher normally.Known limitations (deliberate)
activity_sessions.c) or breaking chronological session order.🤖 Generated with Claude Code