Repository navigation
hrv: refuse RMSSD when the RR stream banks more beat-time than elapsed - #88
Merged
Merged
Conversation
…lapsed Nothing checked that the stored beats can physically fit in the time they span. A double-ingested or two-device-interleaved RR stream reads Σ RR / wall span ≈ 2.0 and passes every existing gate: duplicated beats add zero differences and deflate RMSSD by ~1/√2, interleaved streams inflate it, and neither looks like jitter to the ACF1 screen or the spectral checks. rrCoverage(rrMs, rrTsMs) measures Σ plausible RR ([300, 2400] ms; the rest are counted, so one 65 535 ms glitch cannot fake an over-count) over the wall span (last − first + the first beat's own interval, when that interval is itself plausible: an implausible one is never summed, so it must not stretch the denominator either), null under 10 minutes where whole-second stamps make the span meaningless. Contiguous runs measure 0.963 (gen4), 0.999 (W5) and 1.001 (MG), so the ceiling of 1.10 has margin; gaps only push coverage below 1 and are never refused. duplicate_beats is reported as a diagnostic only (equal beats in one whole-second record repeat legitimately). The sleep-session headline computes it over the session's own beats and is absent above the ceiling (note rr_overcount:coverage=…), before any window or breathing-line logic runs. It now also has a detail form, sleepSessionRmssdDetail → SessionRmssd (rmssd, windows, diff_acf1, rr_coverage), with sleepSessionWindowedRmssd as a thin wrapper, so callers can store what the headline rests on. A night-wide ratio can be diluted below the ceiling by a strap-off gap elsewhere while one stretch holds every beat twice, so the headline also judges each 5-min window on its own: a window whose beats bank more than 1.10 × its length is dropped (counted in overcounted_windows) rather than averaged in. hrvTime and nocturnalRmssd take an optional coverage and refuse RMSSD/pNN50 (SDNN survives) or the whole metric; in hrvTime an over-counted stream also skips the breathing-line fallback, since a line in a stream that is not one heart's beats proves nothing. nightHrvShape takes the same coverage and is absent on an over-counted stream, since each of its bins is an RMSSD of it. The span uses the earliest and latest beat, so the result does not depend on the order beats arrive in. Callers that pass no coverage are unchanged. Output changes: an over-counted night loses its RMSSD headline; new rr_coverage key on the headline detail. Requires kAlgoVersion bump (batched).
Reviewer's GuideThe PR adds a coverage-based integrity gate that compares plausible RR beat-time with elapsed span, then threads it through sleep-session, nocturnal, time-domain, and nightly-shape RMSSD paths to prevent duplicated or interleaved streams from producing misleading HRV while preserving SDNN and backward compatibility when no over-count is detected. Sequence diagram for sleep-session RMSSD over-count protectionsequenceDiagram
participant Caller
participant Session as sleepSessionRmssdDetail
participant Coverage as rrCoverage
participant Windows as 5-minute windows
Caller->>Session: sleepSessionRmssdDetail(rrMs, rrTsMs)
Session->>Coverage: rrCoverage(session beats)
Coverage-->>Session: RrCoverage
alt coverage > 1.10
Session-->>Caller: absent(rr_overcount:coverage=...)
else coverage acceptable
loop each 5-minute window
Session->>Session: sum plausible RR beat-time
alt banked beat-time > 1.10 × window length
Session->>Windows: drop window
else window acceptable
Session->>Windows: compute RMSSD
end
end
Session-->>Caller: SessionRmssd(rmssd, windows, rrCoverage, overcountedWindows)
end
Flow diagram for RR coverage integrity gatingflowchart TD
A[Raw RR intervals and beat timestamps] --> B[rrCoverage]
B --> C{coverage > 1.10?}
C -->|No| D[RMSSD metrics continue]
C -->|Yes| E[Refuse RMSSD with rr_overcount note]
E --> F[SDNN continues publishing]
D --> G[Backward-compatible behavior]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds RR-stream coverage diagnostics and applies over-count checks to HRV and sleep-session RMSSD calculations. It adds detailed sleep-session results with contributing and skipped window counts, updates the algorithms table, and adds tests for coverage, duplicated beats, gaps, and consumer behavior. ChangesRR Coverage and RMSSD
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The window rule behaves as documented; no identified issue needs to be fixed before merging. 🚥 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 |
This was referenced Oct 6, 2026
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="lib/src/onehz/clinical/hrv_time.dart" line_range="1052" />
<code_context>
+ for (final v in buckets[idx]!) {
+ if (_rrCoverageCounts(v)) bankedMs += v;
+ }
+ if (bankedMs > kRrCoverageCeiling * windowSec * 1000) {
+ overCountedWindows++;
+ continue;
</code_context>
<issue_to_address>
**Partial-window duplicates enter headline**
When the session ends partway through a 5-minute bucket and a gap elsewhere keeps session-wide coverage below the ceiling, `sleepSessionRmssdDetail` compares `bankedMs` with 1.10 times the full `windowSec`, so duplicated beat-time in the shorter bucket passes the local gate and its corrupted RMSSD remains in the headline.
Compare `bankedMs` with 1.10 times the bucket’s actual elapsed span, accounting for beat overhang.
Also at `lib/src/onehz/clinical/hrv_time.dart:1053`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the coverage calculation or 1.10 threshold is wrong, valid RR streams could have RMSSD or sleep-session windows refused, producing incorrect persisted or downstream readiness metrics. Reverting restores the prior behavior, and affected metrics are bounded and can be recomputed.
Blocking findings: lib/src/onehz/clinical/hrv_time.dart:1052
The per-window over-count check compared a window's banked beat-time with 110 % of the full window length. The session's last window can be cut short by its end, so a duplicated tail there (e.g. 2 min of beats stored twice, ~240 s banked) stayed under 330 s and its corrupted RMSSD entered the headline whenever a gap elsewhere kept the night-wide ratio below the ceiling. Each window is now judged against its own span, clipped to the session end, plus its earliest beat's interval (that beat began before its end stamp), the same overhang rrCoverage allows.
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.
Why: nothing checks that the stored beats can physically fit in the time they span. A double-ingested or two-device-interleaved RR stream reads Σ RR ÷ wall span ≈ 2.0 and passes every existing gate. Duplicated beats add zero differences and deflate RMSSD by ~1/√2, and interleaved streams inflate it. Neither looks like jitter to the ACF1 screen or the spectral checks.
What changed:
rrCoverage(rrMs, rrTsMs)→RrCoverage(coverage, sum, span, implausible and duplicate counts).rr_overcount:coverage=…;overcounted_windows), because a strap-off gap elsewhere can dilute the night-wide ratio.sleepSessionRmssdDetail→SessionRmssd(rmssd, windows, diff_acf1, rr_coverage, overcounted_windows).sleepSessionWindowedRmssdstays as a thin wrapper.hrvTime,nocturnalRmssdandnightHrvShapetake an optionalcoverageand refuse when it is over-counted. InhrvTimean over-counted stream also skips the breathing-line fallback. SDNN keeps publishing. Callers that pass no coverage are unchanged.User-visible before → after: a night whose RR was stored twice used to show an RMSSD about 30 % low as if it were real. Now it shows HRV "—" with the reason.
Tests:
rrCoverage: 1.0, 2.0, the null cases, implausible intervals, the implausible first interval, order independence, a gappy night.Output changes: an over-counted night loses its RMSSD headline; new
rr_coverage/overcounted_windowskeys.Requires kAlgoVersion bump (batched).
🤖 Generated with Claude Code
Summary by Sourcery
Reject RMSSD results derived from RR streams that contain more beat-time than elapsed time and surface the integrity diagnostics to callers.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
Bug Fixes
New Features