sleep: detect naps with a nap detector, not the one built to reject them - #38
Merged
Merged
Conversation
`detectNaps` delegated to `AdvancedSleepStager.detectSleep`, whose
`minSleepMin = 60` exists — in its own comment — "so daytime naps and stray
still-blocks stay excluded", plus a 90-min + HR-dip guard for anything centred
11:00–20:00 local. The canonical 20–45 min afternoon nap was therefore
STRUCTURALLY undetectable: the advertised 20-min floor was unreachable dead
code. It failed silently, returning an empty list with the note "no qualifying
naps (20 min–3 h)" — wrong about the range (the constant was 6 h) and crediting
van Hees to a path that never called van_hees.dart. The same 75-min nap was
detected at 10:00 and not at 14:00.
Those floors are load-bearing for NIGHT accuracy (#90, and the episode where
gravity-magnitude stillness produced 389 of 421 false positives), so they are
not loosened. New `sleep/nap.dart` instead, run strictly on the complement of
the main sleep window:
- van Hees z-angle immobility, via the new `immobilityMask` extracted from
van_hees.dart so night and nap share ONE primitive. An angle is
orientation-invariant, so it does not inherit the ~13% spread in |accel|
across static wrist postures that motivated those guards.
- every bout, not just the longest, bridged across brief arousals
- an HR dip against the AWAKE-DAYTIME baseline: seconds that are neither the
main sleep nor any detected bout. Excluding only the main sleep left the
candidate's own low-HR seconds, and the hours of tonight's sleep the nap
window borrows, inside the median it is judged against — which makes the
gate self-suppressing, since the quieter the sleep the lower the bar.
- TST and in-bed reported SEPARATELY. Time in bed was the only figure before,
and it is what fed the sleep-need credit.
- a real confidence (HR coverage, dip depth, still fraction, per-bout wear
corroboration), not sleep efficiency wearing the name
- abstain, never guess: under 10 min of awake HR, or under 50% HR coverage in
a bout, the answer is absent rather than a number
No sleep-stage claim, deliberately: a 30-min nap holds no complete cycle and
the daytime HR duty cycle cannot support a 4-class partition. No timezone
dependence either — corroboration is physiological, not clock-based, so a nap
no longer appears or vanishes with the machine's local offset.
Durations are WALL CLOCK, and a run breaks at a timestamp discontinuity. The
substrate is a positional array with pruning and sync holes, so measuring in
sample counts let 20 minutes of evidence report a 2-hour nap.
A bout the record cannot see the end of is DEFERRED, and so is any bout chained
to it. Deferring only the bout touching the array end is not enough: an
ordinary 6-min awakening splits an unfinished night and only the trailing half
touches the end, leaving the leading multi-hour fragment to be emitted as a nap.
Adds tool/nap_harness.dart, scoring the shipped detector on hand-labelled days
and reporting EVENT-level sensitivity and PPV together, TST error on matched
pairs, and the per-subject spread. Per-second accuracy is deliberately not
printed: naps are a few percent of a day, so a detector that reports nothing
scores >97%. The honest caveat is in the header — there is no PSG nap corpus,
because PSG is a nocturnal protocol, so this measures agreement with an
annotator on a small self-collected set and cannot support a population claim.
The old nap tests are removed with the old detector: all five used blocks of
150 min or more, sized (per their own comment) to clear the very gates that
made the real nap regime unreachable, so none could ever have exercised it.
📝 WalkthroughWalkthroughThe PR moves nap detection from ChangesNap detection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant detectNaps
participant immobilityMask
participant HRSamples
participant NapWindowMetric
Caller->>detectNaps: provide acceleration, HR, and exclusion spans
detectNaps->>immobilityMask: calculate sustained z-angle immobility
detectNaps->>HRSamples: build awake baseline and check HR dips
detectNaps->>NapWindowMetric: emit TIB, TST, confidence, and metadata
NapWindowMetric-->>Caller: return judged or absent metric
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ALGORITHMS.md`:
- Line 77: Update the human/coaching.dart row in ALGORITHMS.md to remove
detectNaps from its function list, leaving sleep/nap.dart as the documented nap
source and avoiding references to the removed function.
In `@lib/src/onehz/sleep/nap.dart`:
- Around line 303-311: Rename the inner List<double> variable in the nap
coverage loop from inBout to boutHr, and update its add and length references;
leave the outer boolean inBout baseline mask unchanged.
- Around line 382-394: Update _overlapFraction to clip spans to [start, end),
sort or otherwise order the clipped intervals, merge overlapping or adjacent
coverage, and sum each covered second only once before dividing by dur. Preserve
the existing zero result for non-positive durations, empty spans, and invalid
intervals.
- Around line 287-293: In the nap candidate filtering logic, add outOfRange and
inMainSleep counters alongside the existing rejection counters, increment them
before the duration and mainSleep continue paths respectively, and include
nonzero counts in the skipped note using the specified descriptions.
In `@tool/nap_harness.dart`:
- Around line 22-28: Revise the caveat comment in the nap harness to state that
this evaluation lacks a PSG-labelled nap corpus, without claiming PSG is limited
to nocturnal protocols or that a physiological nap gold standard cannot exist.
Preserve the existing distinction between annotator agreement and
population-level precision claims.
- Around line 134-143: Update the scoring flow around the abstention branch and
summary reporting to track the number of labelled naps excluded from tp, fp, fn,
and perSubject when m.present is false. Report abstained-label coverage and
clearly label sensitivity and PPV as non-abstained-day metrics, including the
related summary paths at the additional referenced sections.
- Around line 164-165: Update the duration-error calculations in the nap
evaluation logic to compare each ground-truth interval duration with nap.tibSec
rather than nap.tstSec, reporting the absolute TIB error in minutes at both
affected locations. Do not infer TST from the start/end-only truth labels; only
use independently labelled TST if the fixture schema is extended.
- Around line 62-72: Harden the fixture-loading and validation flow around
fixture, _validate, and rawAccel before any casts or indexed access. Catch JSON
decode failures and reject non-map roots, then validate each day, acceleration
row shape and numeric values, HR values, labels, and optional spans before
converting them. Ensure malformed acceleration rows cannot reach rawAccel[i][2],
and report invalid fixtures through the existing error path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cdac5abd-4b76-4091-972d-fc4bc96d0cb9
📒 Files selected for processing (8)
ALGORITHMS.mdlib/src/onehz/human/coaching.dartlib/src/onehz/sleep/nap.dartlib/src/onehz/sleep/sleep.dartlib/src/onehz/sleep/van_hees.darttest/onehz/coaching_test.darttest/onehz/nap_test.darttool/nap_harness.dart
💤 Files with no reviewable changes (2)
- lib/src/onehz/human/coaching.dart
- test/onehz/coaching_test.dart
…tions Addresses the review on OpenStrap#38. The two that mattered are both in the harness — the numbers that justify the detector, not the detector itself. THE HARNESS SCORED TST AGAINST A TIB LABEL. A fixture label is a [startSec, endSec] INTERVAL, so its length is the whole episode: time in bed. It was compared against `nap.tstSec`, so every matched nap was charged its own awake time as error — a perfectly measured 2 h episode at 70% efficiency reported a 36-minute miss. That is precisely the TST/TIB conflation this PR exists to end, reappearing in the tool that validates it. Now compares `tibSec`, and the metric is renamed `|TIB error|` rather than left mislabelled. The honest consequence is stated rather than papered over: an interval label cannot score TST at all, so this corpus does not validate the field that actually feeds the sleep-need credit. Said in the header and in the output. ABSTENTIONS WERE FREE. Labelled naps on an abstained day never reached tp/fp/fn, so `labelled naps` under-reported the corpus and sensitivity/PPV were silently conditional on the days the detector agreed to judge — a detector that abstains on its hard days outscored one that tries. Abstained labels are now counted and printed, the two conditional metrics say JUDGED days only, and a new end-to-end recall charges abstentions as misses, which is what a user experiences: an abstention shows them no nap. PSG CLAIM CORRECTED. "PSG is a nocturnal protocol" is false — the MSLT stages daytime naps, and PSG nap corpora exist. The true statement is narrower: we do not HAVE a PSG-labelled nap corpus. Overclaiming that a gold standard cannot exist is still overclaiming. Fixed in the header and the printed footer. _overlapFraction summed spans instead of unioning them, so two overlapping entries counted the same second twice, the fraction could exceed 1, and a bout only half contradicted was rejected outright with `corroborated` driven to 0 — while the doc comment above it has always promised "covered by ANY of [spans]". LATENT, not live: edge's `_toggleSpans` is a correct toggle machine that cannot emit overlapping spans within a list, and wristOff/exclude are evaluated independently. Fixed as a contract guarantee for other callers, with a test verified to fail against the summing version. Two rejection paths — duration out of range, and overlap with the main sleep window — dropped their bout without incrementing a counter, while every other path reports itself in `skipped`. A day whose only candidate was a 7-hour still block therefore answered "no qualifying nap" with an empty tail, telling the caller nothing. Both counted now. The inner `inBout` HR list shadowed the whole-day boolean mask of the same name declared above the loop. Not a live bug — the mask is not read inside the loop — but the next edit that reaches for it would get a List<double>. Renamed to `boutHr`. ALGORITHMS.md still listed `detectNaps` under `human/coaching.dart`, which this PR empties, contradicting its own new `sleep/nap.dart` row. Not taken: defensive validation of the fixture JSON before casting. It is a dev-only tool reading a hand-written file; a stack trace is adequate signal and the guard is unearned ceremony. dart analyze clean; 400 tests pass (was 399).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
detectNapsdelegated toAdvancedSleepStager.detectSleep, whoseminSleepMin = 60exists — in its own comment — "so daytime naps and stray still-blocks stay excluded", plus a 90-min + HR-dip guard for anything centred 11:00–20:00 local.The canonical 20–45 min afternoon nap was therefore structurally undetectable: the advertised 20-min floor was unreachable dead code. It failed silently, returning an empty list with the note "no qualifying naps (20 min–3 h)" — wrong about the range (the constant was 6 h) and crediting van Hees to a path that never called
van_hees.dart. The same 75-min nap was detected at 10:00 and not at 14:00.Approach
Those floors are load-bearing for NIGHT accuracy (#90, and the episode where gravity-magnitude stillness produced 389 of 421 false positives), so they are not loosened. New
sleep/nap.dartinstead, run strictly on the complement of the main sleep window:immobilityMaskextracted fromvan_hees.dartso night and nap share ONE primitive. An angle is orientation-invariant, so it does not inherit the ~13% spread in|accel|across static wrist postures that motivated those guards.No sleep-stage claim, deliberately: a 30-min nap holds no complete cycle and the daytime HR duty cycle cannot support a 4-class partition. No timezone dependence either — corroboration is physiological, not clock-based, so a nap no longer appears or vanishes with the machine's local offset.
Durations are wall clock, and a run breaks at a timestamp discontinuity. The substrate is a positional array with pruning and sync holes, so measuring in sample counts let 20 minutes of evidence report a 2-hour nap.
A bout the record cannot see the end of is deferred, and so is any bout chained to it. Deferring only the bout touching the array end is not enough: an ordinary 6-min awakening splits an unfinished night and only the trailing half touches the end, leaving the leading multi-hour fragment to be emitted as a nap.
Measurement
Adds
tool/nap_harness.dart, scoring the shipped detector on hand-labelled days and reporting EVENT-level sensitivity and PPV together, TIB error on matched pairs, and the per-subject spread.Per-second accuracy is deliberately not printed: naps are a few percent of a day, so a detector that reports nothing scores >97%.
Three limits are stated in the header and in the printed output rather than left implicit:
[start, end]interval, so its length is the whole episode. This corpus therefore cannot validatetstSec— the field that actually feeds the sleep-need credit.Removed tests
The old nap tests are removed with the old detector: all five used blocks of 150 min or more, sized (per their own comment) to clear the very gates that made the real nap regime unreachable, so none could ever have exercised it.
Verification
dart analyze— No issues founddart test— 400 passedDownstream
detectNapsgainswristOff:/exclude:named params andNapWindowgainststSec/tibSec/efficiency, so this is a breaking change for edge.Companion PR: OpenStrap/edge#204 — held in draft until this one merges. It consumes the new signature and does not compile against edge's current analytics pin (
f0d1153), so once this lands, edge'spubspec.yamlmust be repinned to this PR's merge SHA and itspubspec.lockregenerated. Merge order is therefore this PR first, edge#204 second.Summary by CodeRabbit
New Features
Bug Fixes
Tests