fix sleep-session rmssd not catching timestamp gaps - #71
Conversation
sleepSessionWindowedRmssd buckets rr values but drops the timestamps before handing the bucket to _cleanWindowRuns, so it can't tell a real sensor dropout (loose strap, momentary skin-contact loss) from two beats that are actually adjacent. nocturnalRmssd already does this gap check via nnTimesMs, this was the one producer that couldn't. threaded timestamps through to _cleanWindowRuns and added the same gap check, with a wider tolerance than nocturnalRmssd since the real caller quantizes beat times to whole seconds (RrTs.ts rounds), not sub-second.
There was a problem hiding this comment.
Sorry @abdulsaheel, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 6 days and 7 hours by commenting @sourcery-ai review. Upgrade to get a review now.
|
Warning Review limit reachedNext included review available in 22 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
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 |
Reviewer's GuideFix sleep-session windowed RMSSD inflation from sensor dropouts by threading beat timestamps into run cleaning and rejecting differences across real gaps, while using a 1000 ms tolerance to account for whole-second timestamp quantization. Regression tests cover both dropout and control cases; the noted downstream algorithm-version bump remains follow-up work. Sequence diagram for sleep-session RMSSD gap filteringsequenceDiagram
participant Caller as _sessionAvgHRV
participant RMSSD as sleepSessionWindowedRmssd
participant Cleaner as _cleanWindowRuns
participant Metric as RMSSD calculation
Caller->>RMSSD: sleepSessionWindowedRmssd(rrMs, rrTsMs)
RMSSD->>RMSSD: Bucket RR values and beat timestamps
RMSSD->>Cleaner: _cleanWindowRuns(rrBucket, timestampBucket)
Cleaner->>Cleaner: Apply range and median filters
alt Timestamp gap > nn[i] + 1000ms
Cleaner-->>RMSSD: Split into separate runs
else No real gap
Cleaner-->>RMSSD: Preserve run
end
RMSSD->>Metric: Difference values within each run
Metric-->>Caller: Windowed RMSSD
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
sleepSessionWindowedRmssd buckets rr values into 5-min windows but only keeps the value, not the beat timestamp, before handing the bucket to
_cleanWindowRuns. that means it has no way to tell a real sensor dropout (loose strap, brief skin-contact loss) from two beats that just happen to sit next to each other in the bucket. nocturnalRmssd already does this via nnTimesMs, this was the one producer still missing it.threaded the timestamps through to
_cleanWindowRunsand applied the same gap check nocturnalRmssd uses, but with a wider tolerance (1000ms vs 0.5ms) since the real caller (_sessionAvgHRVin advanced_stager.dart) quantizes beat times to whole seconds before calling this, not sub-second precision like the synthetic tests use elsewhere. verified the tighter tolerance broke 3 existing tests due to that quantization before landing on 1000ms.added two tests: one with a genuine ~2min gap inside a bucket (rmssd should not manufacture a cross-gap diff), one control with no gap (rmssd unchanged).
analytics-output change, downstream edge needs a kAlgoVersion bump once this is pinned — not doing that here, flagging as follow-up.
Summary by Sourcery
Preserve beat timestamps during sleep-session RMSSD cleanup so sensor dropouts do not produce artificial HRV differences.
Bug Fixes:
Enhancements:
Tests: