don't difference rr beats across holes (irregular rhythm, cardio stager) - #81
abdulsaheel wants to merge 3 commits into
Conversation
Reviewer's GuideUpdates irregular-rhythm and cardio-stager HRV calculations to avoid differencing beats across dropped runs, rejected beats, and timestamp holes by deriving explicit continuity seams; adds regression tests for both paths. Resulting staging and irregular-rhythm outputs may change, requiring an edge kAlgoVersion bump when repinning. Sequence diagram for seam-aware irregular rhythm screeningsequenceDiagram
participant Input as Compacted RR input
participant Screen as irregularBeatScreen
participant Times as nnTimesMs
participant Metrics as HRV metrics
Input->>Screen: receive rrMs and keep mask
Screen->>Times: check elapsed beat time
alt contiguous successor
Times-->>Screen: successive = true
Screen->>Metrics: difference adjacent RR beats
else dropped run or time hole
Times-->>Screen: successive = false
Screen->>Metrics: skip seam difference
end
Screen-->>Input: irregular rhythm result
Sequence diagram for seam-aware cardio staging metricssequenceDiagram
participant Stager as Cardio stager
participant Cleaner as _cleanBeatsInWindow
participant RMSSD as _windowRmssd
participant Rk as _websterRescore
Stager->>Cleaner: clean beats in window
Cleaner-->>Stager: beats and seam markers
Stager->>RMSSD: calculate window differences
RMSSD->>RMSSD: skip seam pairs
Stager->>Rk: calculate R(k) differences
Rk->>Rk: skip seam pairs
RMSSD-->>Stager: seam-aware RMSSD
Rk-->>Stager: seam-aware R(k)
Flow diagram for seam-aware HRV differencingflowchart LR
A[RR beats and timestamps] --> B{True successor?}
B -->|Yes| C[Include successive difference]
B -->|Dropped run, rejected beat, or time hole| D[Mark seam]
D --> E[Skip difference]
C --> F[HRV metrics]
E --> F
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 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 17 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 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. 📝 WalkthroughWalkthroughClinical rhythm screening and sleep cardio window calculations now exclude interval differences across timestamp gaps and rejected beats. Regression tests cover both conditions. ChangesRR Continuity Filtering
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: 🔵 Low · up to Screening results can change incorrectly for some corrected beats. This is a bounded risk to accept or fix before merging; bump the edge algorithm version when deploying the changed metrics. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The changes remain local to metric calculations and do not introduce new privileges or privileged operations. A small test helper becomes publicly accessible, and deployment of the corrected outputs depends on a separate algorithm-version update that was not verified. Retained concerns Security review detailsSecurity Blast Radius
Resilience and Maintainability Implications
🚥 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
1 participant
irregularBeatScreen only broke pairs at its own 300-2000 filter. callers pass correctRr nn which is already compacted, so beats either side of a dropped run or a dropout got differenced and inflated sd1 / pnn70. now uses the same nnTimesMs seam test hrv_time does.
cardio stager window gather had the same thing: a step-rejected beat or an rr hole left its neighbours adjacent in beats, so window rmssd and r(k) took one big diff per seam (a 400ms jump got through a 200ms gate). gather now marks seams and both skip them.
staging and irregular rhythm outputs change, edge needs a kAlgoVersion bump on repin.
Summary by Sourcery
Skip RR differences across temporal seams so irregular-rhythm and cardio-staging outputs remain valid around dropped or rejected beats.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit