Skip to content

fix(oura): gate RSA respiration to nil when the R‑R stream isn't beat-accurate - #883

Closed
pipiche38 wants to merge 1 commit into
ryanbr:mainfrom
pipiche38:fix/oura-resp-rate-gate
Closed

fix(oura): gate RSA respiration to nil when the R‑R stream isn't beat-accurate#883
pipiche38 wants to merge 1 commit into
ryanbr:mainfrom
pipiche38:fix/oura-resp-rate-gate

Conversation

@pipiche38

Copy link
Copy Markdown

As described in #882 adding a beat-accuracy precondition to SleepStager.respRateFromRR (both platforms): a beat is "time-accurate" when its wall-clock gap matches its own R‑R value within beatAccuracyToleranceS (0.5 s); if fewer than beatAccuracyMinFraction (50 %) of beats qualify, return NaN (honest no-data) instead of a wrong ~10. General, not Oura-specific — WHOOP's beat-accurate R‑R (~100 %) and the synthetic RSA fixtures pass unchanged; Oura's banked IBI (~2 %) is gated. So the Oura night simply shows no respiration rather than a confidently-wrong one.

@vishk23

vishk23 commented Jul 28, 2026

Copy link
Copy Markdown

Sequencing note between this PR and #877 — not an objection to either, and both are yours, so this is really just a "these two want an order" flag. They touch disjoint files (#877 is AnalyticsEngine + IntelligenceEngine, this one is SleepStager), so nothing in git will enforce it.

Short version: this one wants to land first.

The chain, verified on main @ e9326000:

AnalyticsEngine.swift:580-585
    let perSession = matched
        .map { SleepStager.respRateFromRR(rr, start: $0.start, end: $0.end) }
        .filter { $0.isFinite }

respRateFromRR has exactly one production call site per platform (AnalyticsEngine.swift:582, AnalyticsEngine.kt:413), and its input is matched — the day's sleep sessions.

Today a ring night contributes nothing to matched, because detectSleep bails before it starts:

SleepStager.swift:909-910
    let grav = gravity.sorted { $0.ts < $1.ts }
    if grav.count < 2 { return [] }

So respRateBpm currently comes out nil for a gravity-less owner — which #877's own new test asserts (testEmptyProvidedSleepIsByteIdenticalToOmitting). #877 is precisely the change that makes matched non-empty for that owner, so it is what first routes the ring's banked R-R into the RSA estimator. This PR's gate sits inside respRateFromRR, upstream of everything else in that function, so landing it first covers #877's new path on day one with no follow-up. The reverse order leaves a window where the ring's stream is estimated ungated.

Two things that make this less alarming than it first looks, both worth having on the record:

There is already a partial guard. respRateFromRR clamps its output to respPlausibleRangeBpm (8.0...25.0, defined SleepStager.swift:1484, applied :1589 — outside the band it returns NaN). This PR's description cites a collapse to "a confidently-wrong ~7–10"; the sub-8 part of that is already NaN'd on main today. The genuinely exposed window is roughly 8–10, not the whole range.

If a value does land in that window, the consequence isn't a false illness signal. I want to correct this before it becomes received wisdom, because I had it wrong myself until I read the code. Every live respiration consumer is one-sided upward:

  • IllnessSignalEngine.swift:159-160let over = r.zIllnessward - signalZThreshold; guard over > 0 else { continue }. A negative z contributes nothing and isn't counted toward the corroboration gate.
  • ReadinessEngine.swift:172,176if z >= 2.0 { .bad } else if z >= 1.5 { .watch }. No low branch.
  • AppModel.swift:1465signal({ $0.respRateBpm }, cfgKey: "resp", illnessUp: true).

A spurious ~10 against a ~14–16 baseline is a negative z and fires no illness signal anywhere. What it actually does is:

  1. Inflate Charge. The recovery respiration term is lower-is-better (RecoveryScorer.swift:341-343, commented "Resp term: lower is better"), so a fabricated low respiration reads as better recovery.
  2. Contaminate the personal respiration baseline the illness z-scores are later measured against (IntelligenceEngine.swift:818:845-853:871; the "resp" baseline config accepts 4.0–40.0, so 8–10 folds in silently). Since that baseline is keyed by day rather than by device, a user running both a ring and a strap pulls the shared baseline down.

(2) is the one that eventually produces an illness false positive — but second-order and only after enough contaminated nights have shifted the baseline, not on the night itself.

None of this argues against either PR. It argues for #883 before #877, and I thought the reasoning was worth writing down since the two of them read independently.

Everything above is code-reading on main @ e9326000. I have no Oura ring, so I have not independently reproduced your ~2% beat-accuracy figure or the observed ~7–10 — those remain your device observations. Thanks for splitting these into two focused PRs; it made the interaction much easier to see. @pipiche38

@pipiche38

Copy link
Copy Markdown
Author

As pointed by @vishk23 , @ryanbr if you are happy to merge the PR, please do merge #883 first and then #877

@pipiche38

Copy link
Copy Markdown
Author

@vishk23 Agreed on all three points, and confirmed against the code:

So: land #883 before #877. No code change needed here — the NaN path is the complete fix; the only concern is merge order.

…-accurate

Oura nights reported a respiratory rate ~9-14 bpm while a WHOOP strap worn on the
SAME nights measured a steady ~16.4. Respiration is derived by RSA (respiratory
sinus arrhythmia) from the R-R stream, and the Oura ring's banked overnight IBI is
not a beat-accurate time series: it stores R-R VALUES with coarse batch timestamps
(7/27: 25456 beats whose values sum to 7.87 h stamped inside a 3.46 h timestamp
span, median inter-beat gap 0 s; only ~2% of beats are time-accurate, no accurate
run >= 150 s). RSA is frequency-domain and needs beat TIMING, so it collapses to a
confidently-wrong ~7-10 bpm. Time-domain HRV (RMSSD, order-only) is unaffected and
matches WHOOP well - this is respiration only.

Fix: a beat-accuracy precondition in SleepStager.respRateFromRR (both platforms) -
a beat is time-accurate when its wall-clock gap matches its own R-R value within
beatAccuracyToleranceS (0.5 s); if fewer than beatAccuracyMinFraction (0.5) of beats
qualify, return NaN (honest no-data) instead of a wrong number. General, not
Oura-specific: WHOOP R-R (~100% accurate) and the synthetic RSA fixtures pass
unchanged; Oura banked IBI (~2%) is gated, so the night shows NO respiration rather
than ~10. There is no salvageable beat-accurate segment to compute on (longest 12 s),
so this is a whole-night gate.

Tests: a batched-timestamp stream (same R-R values as the recovering fixture,
timestamps banked 6-to-a-second) gates to NaN; the beat-accurate recovery fixture
still returns ~15. Swift StrandAnalytics 1168/0 (RespRateRsaTests 4/4); Kotlin
RespRateRsaTest green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AtXcBU1t6Xk1qJhaQEeDx6
@pipiche38

Copy link
Copy Markdown
Author

I'm sorry , but this is not going as my expected speed, and this is an handicap for my development. I don't want to spend too much time in just rebasing all my PRs.
I understand that @ryanbr is the only maintenair. Appreciate the work you are doing.

So I'm closing all those PR .

Feel free to take over what I was doing ...

ryanbr pushed a commit that referenced this pull request Aug 6, 2026
… is over-counted (#1108)

#1085 taught the app to refuse SDNN on an over-counted capture. A drain the night
after it merged showed that gate is necessary but not sufficient: the 2026-08-06
Oura night measured coverage 1.03, `rrIntegrity=plausible` — no duplication at all,
its records tiling the timeline at a fill ratio of 0.990 — and still printed
**SDNN 174 ms** against a 40-100 ms physiological range.

Over-counting was never what made that number wrong. A BANKED stream stamps a whole
record of intervals on one coarse timestamp, so the stored values are a decomposition
of a record period rather than beat-to-beat measurements. The per-record SUM is right
to ~1%, which is why meanNN and resting HR stay correct and WHOOP-validated, while the
individual intervals are not. Coverage cannot see that fault, by construction: it
compares beat-time against wall-clock, and a banked night can be textbook.

Measured on that night, after the shipped Malik ectopic filter:
  - within-5-minute SDNN 123 ms, against 30-80 ms physiological;
  - only 94 ms of the whole-night figure is genuine trend (HR really moves 47-86 bpm);
  - widening the ectopic window does not reach it — radius 2 -> 20 moves the
    within-window figure just 124 -> 99 ms. Each interval sits within 20% of its own
    local median, so no per-beat artifact rule can see this: the fault is in the
    decomposition, not in outliers.

So the gate has to be on the stream's nature, not on a spread statistic:

  - `beatAccurateFraction(tsSec:rrMs:)` — the fraction of consecutive beats whose
    wall-clock gap matches their own R-R value. Beat-accurate streams step one
    interval per beat and measure ~1.0; a banked stream's gaps are 0 s against ~1 s
    values and it collapses toward 0.
  - `beatValuesAreTrustworthy(beatAccurateFraction:)` — SDNN is withheld below the
    boundary. Independent of `beatSpreadIsTrustworthy`; neither implies the other, and
    both now gate.

The boundary is not tuned. The two populations do not overlap near it: a beat-accurate
stream measures ~100%, and every banked Oura overnight measured to date sits at
**2.6-6.6%** (five nights, 2026-07-29 -> 08-06). RMSSD/pNN50 stay ungated for the same
reason they are ungated by the coverage verdict. `unmeasurable` live spot readings are
untouched: too-short or mismatched input returns 1.0 and stays trusted, so an honest
live capture is never suppressed.

The constants duplicate the ones the respiration gate uses for the same judgement
(#882/#883) rather than sharing them — that gate lives in `SleepStager` on a branch
that is not upstream. If it lands, the two should collapse onto this definition; the
boundary is worth drawing once, in one place, for both.

`hrv diag` now carries `beatAccurate=` so the distribution can be gathered from traces
that already exist, the same way `coverage=` was added before acting on it.

Verification: `swift test` StrandAnalytics 1248, incl. 6 new tests — the decisive one
pins a PERFECTLY COVERED banked night (coverage 1.0, verdict plausible) passing the
over-count gate and being refused by the new one, which is the case that motivated this.
Kotlin twins of each in `HrvRrCoverageTest`. `./gradlew testFullDebugUnitTest` 3,516
tests with the same 3 pre-existing locale failures as clean main (verified on a clean
worktree, not assumed). `Strand` (macOS) built locally since `IntelligenceEngine.swift`
is app-target Swift that no default CI job compiles.


Claude-Session: https://claude.ai/code/session_01Kyxz22d4v7QHWFCSWTvdJq

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ryanbr pushed a commit that referenced this pull request Aug 8, 2026
…ked (#1127)

An Oura night reports a respiratory rate that looks entirely normal while a WHOOP
strap worn on the SAME nights measures ~16.4. Respiration is derived by RSA from
the R-R stream, and the ring's banked overnight IBI cannot support RSA: it stores
R-R VALUES against coarse per-record timestamps, so the beat-to-beat variation RSA
reads is not in the data.

Fix: respRateFromRR (both platforms) refuses a stream that is not beat-accurate,
reusing HRVAnalyzer.beatAccurateFraction / beatValuesAreTrustworthy (#1108) rather
than carrying a second copy of the same judgement and its two constants. That is the
collapse #1108's PR body promised: one boundary, one definition, so a threshold
change moves respiration and SDNN together. WHOOP R-R and the synthetic RSA fixtures
measure ~100% and pass unchanged.

MEASURED 2026-08-07 on two real nights (31,460 and 30,754 in-bed beats; the real
SleepStager.respRateFromRR, plus a validated port to switch the gate off):

  beatAccurateFraction        0.0246 / 0.0235   (threshold 0.50)
  shipped path                NaN / NaN
  both protections disabled   13.33 bpm / 13.33 bpm

Three things this corrects in the original #883 writeup, all of which strengthen it:

1. The number is 13.33, not the "~7-10" first reported. It sits inside
   respPlausibleRangeBpm (8-25) and inside the real adult sleeping range, so the
   range clamp is no protection whatsoever. Only 1 of 113 and 0 of 114 windows fall
   below 10 bpm.

2. The estimate carries zero information. SHUFFLING or REVERSING the night's R-R
   values returns the same 13.3333 to four decimals, on both nights. The tachogram's
   breath band is a flat 1/f shelf with no peak; 13.33 is the peak-picker's own floor
   on the 4 Hz grid. This is the #194 bar failed outright, which is why the gate is on
   BANKED-ness and not on whether the output looks sane.

3. The mechanism in the old comment was wrong. The time AXIS is not corrupted:
   sum(R-R) over wall span is 1.030 / 1.008, so beat-time reconstructs the night to
   1-3%. What is unusable is the interval VALUES - the ~6.6 s record decomposed into
   ~6 intervals whose sum is right to ~1% while the individuals are not beat-to-beat
   measurements, the same decomposition documented on beatValuesAreTrustworthy.

Also documents, and pins with a test, that this gate and #977's splice skip catch
OPPOSITE banking geometries and neither subsumes the other: #977 catches banking that
TILES time (the real ring - it independently discards 113/113 and 114/114 windows on
these nights), this catches banking that COMPRESSES it (the batched fixture, where no
gap ever exceeds rsaGapToleranceS and #977 is blind).

And one known limitation, pinned deliberately: the gate detects banking by its
symptom, and that symptom is repairable. Re-timing each record's beats by cumsum from
the record's own timestamp moves the fraction 0.0246/0.0235 -> 0.875/0.863, defeating
both this gate and #977 - while the estimate stays 13.3333 and stays unchanged under
shuffling. A well-meant decoder change would silently switch respiration back on for a
stream carrying no breathing information. testRetiming...knownLimitation fails the day
that happens, and says what to do: re-base the gate on provenance, not timestamp shape.

Tests: Swift StrandAnalytics 1270/0 (RespRateRsaTests 7/7 incl. 3 new); Android
compileFullDebugKotlin clean, 3554 tests / 3 failed - all 3 (AiCoachContextTest, 2x
StandardHrSensorFormatTest locale) reproduce on clean upstream/main, pre-existing.
No hardware behaviour changes: this is analytics-only, no BLE path touched.


Claude-Session: https://claude.ai/code/session_01XXdjctbkxqo359NuuJascK

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants