Repository navigation
hrv: a 5-min window needs 20 clean differences to join the nightly RMSSD - #89
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 hardens sleep-session HRV by requiring 20 clean successive differences per headline window, reporting excluded-window diagnostics, and adding RR coverage integrity gates that refuse RMSSD-family metrics for duplicated or interleaved streams while preserving secondary metrics where appropriate. Flow diagram for sleep-session RMSSD window eligibilityflowchart TD
A[sleepSessionRmssdDetail] --> B[rrCoverage]
B --> C{coverage over ceiling?}
C -->|yes| D[Absent: rr_overcount note]
C -->|no| E[Split sleep session into 5-min windows]
E --> F[Clean each window with _cleanWindowRuns]
F --> G{Window beat-time over ceiling?}
G -->|yes| H[Count overCountedWindows]
G -->|no| I[Count clean successive differences]
I --> J{nd less than minDiffsPerWindow?}
J -->|yes| K[Count thinWindows]
J -->|no| L[Include window RMSSD]
H --> M[Build dropped-window note]
K --> M
L --> N[Arithmetic mean of kept window RMSSDs]
M --> N
N --> O{Any kept windows?}
O -->|no| P[Absent with drop reasons]
O -->|yes| Q[Publish SessionRmssd diagnostics]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 57 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (4)
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 |
![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 5 issues
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="622" />
<code_context>
List<double> nnMs, {
List<double>? nnTimesMs,
double artifactFraction = 0.0,
+ RrCoverage? coverage,
}) {
const inputs = ['rr_cleaned'];
</code_context>
<issue_to_address>
**Over-count gate is opt-in**
When a caller computes cleaned NN metrics from an over-counted raw RR stream but does not explicitly pass its raw-stream coverage, when callers use the existing `hrvTime`, `nocturnalRmssd`, or `nightHrvShape` calls without supplying `coverage`, those functions skip the new over-count refusal and publish RMSSD from duplicated or interleaved beats. The public API leaves the check easy to omit, so callers still see the wrong metric.
Make the raw-RR integrity check part of the normal metric orchestration, or change these APIs so callers cannot silently request RMSSD without supplying the raw RR coverage.
Also at `lib/src/onehz/clinical/hrv_time.dart:801`, `lib/src/onehz/sleep/night_hrv_shape.dart:121`.
</issue_to_address>
### Comment 2
<location path="lib/src/onehz/clinical/hrv_time.dart" line_range="559" />
<code_context>
+ 'implausible_beats': implausibleBeats,
+ 'duplicate_beats': duplicateBeats,
+ };
+}
+
+/// [RrCoverage] of raw RR [rrMs] against their beat-END epoch times [rrTsMs]
</code_context>
<issue_to_address>
**Duplicate count depends on order**
When an exact repeated `(timestamp, interval)` pair is non-adjacent in the input, `rrCoverage` counts duplicates only when matching pairs are adjacent, so `RrCoverage.toJson()` reports an incorrect `duplicate_beats` count for shuffled entries.
Count exact repeated `(timestamp, interval)` pairs independently of their positions in the input.
</issue_to_address>
### Comment 3
<location path="lib/src/onehz/clinical/hrv_time.dart" line_range="1004-1008" />
<code_context>
+ minDiffsPerWindow: minDiffsPerWindow);
+ return m.present
+ ? Metric<double>(
+ value: m.value!.rmssd,
+ confidence: m.confidence,
+ tier: m.tier,
+ inputs_used: m.inputs_used,
+ note: m.note,
+ )
+ : Metric<double>.absent(
</code_context>
<issue_to_address>
**Sleep staging loses diagnostics**
When sleep sessions are produced through `AdvancedSleepStager.detectSleep` and windows fall below the new floor, the scalar wrapper and `_sessionAvgHRV` retain only `metric.value`, so `SleepSession.avgHrv` loses the thin-window count, coverage diagnostics, and explanatory note when windows are omitted.
Preserve and expose the `SessionRmssd` diagnostics through the normal sleep-staging result.
Also at `lib/src/onehz/clinical/hrv_time.dart:931`.
</issue_to_address>
### Comment 4
<location path="lib/src/onehz/clinical/hrv_time.dart" line_range="939" />
<code_context>
+ final double rmssd; // ms — the headline
+ final int windows; // 5-min windows that contributed
+ final int overCountedWindows; // windows dropped for more beat-time than time
+ final int thinWindows; // windows dropped for 1..floor−1 differences
+ final int minDiffsPerWindow; // the floor, which travels with the number
+ final double? diffAcf1; // pooled over the session's windows
</code_context>
<issue_to_address>
**Zero-difference windows counted unexpectedly**
When a window's cleaner leaves no successive differences, callers reading `SessionRmssd.thinWindows` as the documented count of windows with 1–19 differences exclude windows with zero differences, but `sleepSessionRmssdDetail` increments the count when `nd == 0`; their interpretation of the diagnostic does not match the returned count.
Document `thinWindows` as counting windows with 0 through `minDiffsPerWindow - 1` differences.
</issue_to_address>
### Comment 5
<location path="test/onehz/clinical_test.dart" line_range="566" />
<code_context>
+ required this.beats,
+ required this.implausibleBeats,
+ required this.duplicateBeats,
+ });
+ bool get overCounted => coverage > kRrCoverageCeiling;
+ Map<String, dynamic> toJson() => {
</code_context>
<issue_to_address>
**Gappy coverage refusal goes untested**
When a gappy stream has coverage below 1, the test named `a gappy night reads well under 1 and is never refused for it` checks only `rrCoverage`'s ratio. A regression that refuses `hrvTime`, `nocturnalRmssd`, or the session headline for any coverage below 1 still passes, so the claimed no-refusal behavior is untested.
Pass the computed coverage to the relevant estimator(s) and assert their metrics remain present.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 5 findings to address first, and a wrong coverage gate or 20-difference floor can drop or materially change the nightly RMSSD and therefore the readiness value derived from it. Reverting restores future calculations, but already persisted or consumed derived metrics would remain wrong until recomputed; the impact is bounded and repairable rather than inherently irreversible.
Blocking findings: lib/src/onehz/clinical/hrv_time.dart:622, lib/src/onehz/clinical/hrv_time.dart:559, lib/src/onehz/clinical/hrv_time.dart:1008, lib/src/onehz/clinical/hrv_time.dart:939, test/onehz/clinical_test.dart:566
added 2 commits
October 8, 2026 13:00
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.
…ly RMSSD The nightly headline is the arithmetic mean of per-5-minute-window RMSSDs. Sparse windows were recently kept out below 5 successive differences, which stops a single |Δ| across a dropout edge from counting like a full window. A window of 5–19 differences still counts at full weight, though: RMSSD's relative sampling error is ~1/√(2n), 32 % at n = 5 against 4 % for a full window of ~300, and such windows sit where the signal is disturbed (session edges, sensor dropouts, heavy ectopy), so the error is biased upward: six clean windows at 5.9 ms plus one 9-difference window holding a 300 ms step published 19.3 ms. Nothing reported how many windows were dropped either. A window now joins the mean only with ≥ kMinDiffsPerRmssdWindow (20) clean successive differences within contiguous runs: 20–25 s at a sleeping 48–60 bpm, inside the 10–30 s the ultra-short RMSSD literature supports (Munoz et al. 2015; Baek et al. 2015), and 16 % relative error. It is a floor, not a weight: weighting by n weights by heart rate, so high-HR, low-RMSSD REM/arousal windows would pull the headline down. nocturnalRmssd, a secondary median estimator under its own key, keeps its own floor of 5. Thin windows, including those the cleaner reduced to no difference at all, are counted (SessionRmssd.thinWindows → thin_windows, with min_diffs_per_window travelling alongside, both defaulted), and their differences contribute nothing to the pooled jitter verdict. Every note the headline gives, present or absent, now says how many windows were dropped and why (thin, or more beat-time than elapsed), so a blank or a thin headline can be told apart from a jitter refusal. The floor is a parameter (minDiffsPerWindow); the mechanics tests that use tiny windows pass a small floor explicitly. Tests: a 9-difference window with a 300 ms step cannot move the headline (it moved it to 19.3 ms at a floor of 5); 19 differences do not count, 20 do; the floor is a parameter; a discarded 20-beat window alternating 910/1090 ms leaves the pooled ACF1 (≈ 0.866), RMSSD and confidence of the clean windows unchanged (fails if the pooling moves above the floor). Output changes: the nightly RMSSD headline drops windows with < 20 successive differences; nights made only of such windows go absent; notes name the dropped windows. Requires kAlgoVersion bump (batched).
DropTabl
force-pushed
the
fix/rmssd-window-floor-solo
branch
from
October 8, 2026 11:01
05efb07 to
68a20b2
Compare
…nclude zero rrCoverage counted an exact (ts, rr) repeat only when it sat right after its twin, so the duplicate_beats diagnostic depended on input order. It now counts every repeat of an earlier pair. SessionRmssd.thinWindows counts windows with 0 to minDiffsPerWindow - 1 differences, including one the cleaner left with none; the field and function docs said 1 to floor - 1. The gappy-night test now also checks that hrvTime, nocturnalRmssd and the session headline still publish at coverage well below 1.
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: the nightly headline is the arithmetic mean of 5-min-window RMSSDs. The current floor of 5 differences stops a single |Δ| from counting like a full window, but a 5–19-difference window still counts at full weight. RMSSD's relative error is ~1/√(2n): 32 % at n = 5 against 4 % for a full window. Such windows sit where the signal is disturbed, so the bias is upward. Six clean windows at 5.9 ms plus one 9-difference window holding a 300 ms step publish 19.3 ms today.
What changed:
kMinDiffsPerRmssdWindow = 20clean differences within contiguous runs: 20–25 s at a sleeping 48–60 bpm, inside the 10–30 s the ultra-short-RMSSD literature supports (Munoz et al. 2015; Baek et al. 2015).thin_windows, withmin_diffs_per_window). Their differences stay out of the pooled jitter verdict.minDiffsPerWindowis a parameter.nocturnalRmssd(secondary, under its own key) keeps its floor of 5.User-visible before → after: HRV 19.3 ms → 5.9 ms in the case above. A night made only of thin windows shows "—" with a note saying so.
Tests:
Depends on: #88 (stacked; same function).
Output changes: windows with < 20 successive differences are dropped; nights made only of them go absent.
Requires kAlgoVersion bump (batched).
🤖 Generated with Claude Code
Summary by Sourcery
Harden nightly HRV RMSSD reporting by excluding statistically thin windows and refusing streams whose beat-time exceeds elapsed time.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: