Respiratory rate: prefer PPG-derived estimate over RSA when available (#103) - #339
Respiratory rate: prefer PPG-derived estimate over RSA when available (#103)#339digitalerdude wants to merge 5 commits into
Conversation
|
Nice result, and the honesty about the evidence base is appreciated. We've been working the same v26 buffer from the Linux side (the spot-HRV pair, NoopApp/noop#537 for Android and #541 for the Linux reference tool) and can independently confirm two of your observations. First, v26 stretches really are a sparse minority of any night, with v18 taking the rest and never both at once. Second, the aggregation shape you landed on (one estimate per burst, filter on a per-burst quality score, median of the top slice) is the same shape that turned out to be necessary to get a stable RMSSD out of those same bursts. Two independent metrics converging on the same aggregation suggests it's the burst structure forcing it rather than a coincidence. Offer: we have v26 bursts from two WHOOP 5 straps across a decent number of nights, each with a per-night reference reading to score against. Happy to run Two things I'd want to check before trusting the estimator broadly: RIAV envelope window. Low-band leakage. The detrend is a 12.5 s trailing mean, so the high-pass corner sits around 0.08 Hz, not far below the 0.15 Hz band floor. Residual perfusion or posture drift can leak into the lowest bins and pin the peak at the band edge, and Neither is a blocker for prefer-PPG-with-RSA-fallback, which reads as the right call given the evidence you have. |
|
Really strong, honest work — clean decode → storage → estimator → fusion on both platforms, well gated (≥9 bursts else NaN, median of top-third confidence), and refreshingly candid about the evidence. Two things I want to credit up front, because they matter given the history here:
The ask: gate the fusion behind a default-off Experimental toggleThe remaining gap is the real-data validation — 2 nights, one person "whose respiratory rate barely moves," weak correlation (all your words). The synthetic tests prove the method tracks synthetic modulation; they don't prove the peak it locks onto on real PPG is respiration vs. a coincidental band peak near this person's stable ~14/min. Given #194, that's thin ground to make PPG override the proven RSA estimator for everyone by default. A default-off Experimental toggle ("Prefer PPG respiratory estimate") solves this cleanly and lets the feature ship now:
One thing the toggle alone doesn't fix — keep the illness gate off the mixed baseline
Keep the instrumentationRetain the PPG-vs-RSA logging (or add it) so opt-in nights emit PPG vs RSA vs the value actually used — that's what turns the toggle into a real experiment and produces the evidence to promote it to default later. Nits
Coordination note (not this PR's fault)This uses Room Net: great method, honestly scoped. Land it behind a default-off Experimental toggle with the illness gate kept on a single estimator and the logging retained, and flip the preference to default once the opt-in field data shows PPG actually tracks real variation. |
Codifies the #194 lesson: PPG/motion buffers are fixed-N-per-record, so autocorrelation/spectral methods can manufacture a record-period peak that matches WHOOP on a stable night without being real (why PPG→HR #194 was withdrawn). One matched night isn't validation — prove it tracks a varying input, else ship as instrumentation or behind a default-off toggle, never as the default or into a downstream gate. Prompted by the #339 respiratory-rate review. Also notes the 4.0 sparse-motion limit (#345).
bb4e574 to
4991d94
Compare
|
Thanks @ryanbr — this is exactly the right call, and I've addressed all of it. Toggle: a default-off Illness gate now reads a single estimator, always. Nits:
Coordination note: confirmed — (Also confirmed & fixed a real band-edge leakage issue sunny-noop flagged — details in the reply to that comment.) |
|
@sunny-noop Really appreciate this — the convergent-aggregation observation (independent RMSSD work landing on the same one-estimate-per-burst / quality-filter / top-slice-median shape) is a genuinely reassuring signal that it's the burst structure forcing the aggregation, not a coincidence. And yes please to the offer: v26 bursts from two straps across a decent number of nights, each with a per-night reference, is exactly what turns this two-night check into a real n across a second wearer. That's the evidence that would let the diagnostic toggle become the default. I ran both diagnostics you suggested against the real capture data: RIAV envelope window (conf/estimate vs. burst mean HR): weak and inconsistent across the two nights — corr(HR, conf) was −0.14 / +0.02 and corr(HR, bpm) was −0.10 / −0.39. Not a clean smoking gun for the beat-period-vs-window concern, but the sign flip on n=2 means I can't rule it out either. I left the RIAV path as-is rather than do a speculative rewrite (your per-beat-amplitude-resample idea via the existing Low-band leakage: confirmed, and fixed. This one was real — without a boundary check, 32% (night 1) / 15% (night 2) of bursts peaked in the bottom one or two bins (≤9.5 breaths/min), far more than plausible slow breathing. Added |
4991d94 to
a8afc56
Compare
…ryanbr#103) Adds PpgResp, a burst-level spectral estimator on the WHOOP5 v26 optical PPG buffer (RIAV/RIIV band search 0.15-0.40Hz), alongside the existing per-second PpgHr estimator on the same buffer. Validated against two real overnight BLE captures against the WHOOP app's own reading: lands within 2-6%, vs. the existing R-R/RSA estimator's 6-11% low bias on the same nights. AnalyticsEngine now prefers the PPG estimate per sleep session when it has enough burst coverage (>= 9 qualifying bursts), falling back to RSA otherwise -- not a blend, since blending two estimates of different, uncharacterized accuracy risks being confidently wrong in a new way neither alone is. v26 coverage is a sparse minority of any night, so RSA fallback stays the common case. New GRDB migration v26-ppg-resp-sample / Room v18->v19 (ppgRespSample table), wired into device-scoped delete, timestamp healing, and the raw-sensor CSV export on both platforms. Verification: swift test across WhoopProtocol/WhoopStore/StrandAnalytics (1539 tests), gradlew testFullDebugUnitTest (2563 tests), local xcodebuild of the macOS app target (IntelligenceEngine.swift touched, app-build CI is disabled by default). No BLE/hardware surface -- this is a pure historical- offload + analytics path.
The project's derived-biosignal validation standard (added to CLAUDE.md while this PR was in flight) is explicit: a single stable-night validation against the WHOOP app is not proof a method tracks real signal, and until that's shown, land it as instrumentation or behind a default-off toggle -- never as the default feeding a downstream gate. The prefer-PPG-else-RSA fusion in the prior commit was validated on exactly 2 nights from one subject whose own respiratory rate barely varies -- the single-stable-night trap the standard calls out -- while feeding DailyMetric.respRateBpm and the ReadinessEngine illness-detection signal by default. New PuffinExperiment.ppgRespRateKey / ppgRespRate (default false), mirroring the existing ppgHrSubLagInterp/hrvReadiness precedent. AnalyticsEngine and SleepStager stay pure and unaware of the toggle; IntelligenceEngine (the Context-aware app layer) reads it and only supplies the real ppgRespSample stream when enabled, else an empty list -- which the fusion already treats as "no PPG data available", falling back to RSA. Default path is therefore byte-identical to shipped behavior. The stream is still decoded and persisted unconditionally (pure instrumentation), so opting in later needs no backfill. Toggle added to the Experimental Algorithms card on both platforms.
Per ryanbr's review: - DailyMetric.respRateBpm (and hence ReadinessEngine's illness-detection z-gate) now ALWAYS comes from the R-R/RSA estimator, never the PPG one. The prior prefer-PPG-else-RSA fusion let the persisted value silently switch estimator night to night; since RSA and PPG read ~1-1.5 bpm apart even when both are "right," a mere estimator switch for a stable sleeper (resp SD ~1 bpm) could inject a step large enough to brush the illness WATCH z-threshold on its own -- a false signal from the switch, not physiology. - The Experimental toggle (PuffinExperiment.ppgRespRate) is now purely diagnostic: when on, it logs RSA vs PPG vs the value actually used (always RSA) through the existing hrvTraceSink hook, for comparison. It never changes what's stored or displayed. Toggle/doc copy on both platforms updated to describe this accurately. - Fixed the AnalyticsEngineTests/AnalyticsEngineRespRateFusionTest.kt fixtures accordingly: respRateBpm must stay RSA-only even with ample contradicting PPG data, and the comparison trace line must still fire. Per sunny-noop's review (independent v26-buffer work, thank you for the convergent-methodology data point and the offer of more nights): - Ran both suggested diagnostics against the real capture data. The burst-mean-HR correlation with confidence/estimate was weak and inconsistent between the two nights (-0.14/-0.10 for confidence, -0.10/-0.39 for the estimate itself) -- not a clear smoking gun for the RIAV-envelope-vs-beat-period concern, so left as a documented open question rather than a speculative DSP rewrite on ambiguous evidence. Sunny-noop's offer to score PpgResp/respRateFromPpg against a second strap and more nights would settle this properly. - The band-edge leakage concern WAS confirmed: without a boundary guard, 32%/15% of bursts across the two nights landed exactly at the band floor (9 breaths/min) -- far more than plausible slow breathing. Added PpgResp.bandEdgeGuardHz (0.02 Hz): a peak within that margin of loHz/hiHz is rejected (nil for that channel) rather than reported, since a real breathing peak is a local maximum while residual low-frequency drift monotonically increases toward 0 Hz and so always "wins" at a bounded edge regardless of physiology. This ALSO improved accuracy against known ground truth: top-confidence-burst median moved from 14.04 to 14.52 breaths/min against the 14.6 truth (was tested in isolation against a longer detrend window too; the boundary guard was the more effective and more targeted fix). Ported to both platforms with a matching test fixed (the synthetic slow-breather case moved from 10 to 11 bpm, clear of the new guard) plus the requested upper-band (19 bpm) case added. - Softened the "0.24 breaths/min resolution" comment, which oversold precision: a ~40s/~960-sample burst's native DFT bin spacing is ~1.5 breaths/min; the 0.004 Hz scan step interpolates the direct-sum DTFT more finely between bins but doesn't add real resolution beyond that. Re-verified: swift test across WhoopProtocol (288) / WhoopStore (262) / StrandAnalytics (1029), gradlew testFullDebugUnitTest, and a local xcodebuild of the macOS app target (PuffinExperiment.swift / IntelligenceEngine.swift / TestCentreView.swift are app-target files app-build.yml CI doesn't cover by default).
Upstream landed feat(store): persist v26 raw PPG waveform (ryanbr#415), which took GRDB v27-ppg-waveform and Room MIGRATION_19_20 — the exact slots this PR previously used. Renumbered the ppgRespSample migration to GRDB v28-ppg-resp-sample / Room MIGRATION_20_21 (schemaVersion 21), keeping both platforms' migration counts aligned, and updated the two migration tests' version-pair assertions + doc comments to match. The waveform table (ryanbr#415) and this resp table are three streams off the same v26 buffer, each in its own table, no schema overlap.
The i18n regression gate (landed upstream during rebase) requires every user-facing UI string to route through the localization catalogs with de/es/fr translations. Wired the Experimental-algorithms toggle's title through uiString(R.string.…) on Android + added the key to values/ + values-de/es/fr, and added the title + description to the Swift Localizable.xcstrings catalog with de/es/fr. Code identifiers in the copy (DailyMetric.respRateBpm, R-R, PPG, HRV, WHOOP5 v26) are kept in English by convention. Translations are best-effort for maintainer review. `python3 Tools/i18n_audit.py --ci main` now passes; compileFullDebugKotlin green.
a8afc56 to
0cc9176
Compare
Codifies the ryanbr#194 lesson: PPG/motion buffers are fixed-N-per-record, so autocorrelation/spectral methods can manufacture a record-period peak that matches WHOOP on a stable night without being real (why PPG→HR ryanbr#194 was withdrawn). One matched night isn't validation — prove it tracks a varying input, else ship as instrumentation or behind a default-off toggle, never as the default or into a downstream gate. Prompted by the ryanbr#339 respiratory-rate review. Also notes the 4.0 sparse-motion limit (ryanbr#345).
Co-lay the Start-workout and Add-workout controls as an equal-width action row on the Workouts screen, matching the iOS workoutActionRow (#1068). When a strap is bonded they sit side by side; unbonded shows Add alone (moved up out of the range bar so it stays reachable without a strap), which also frees the range pill from the old Add-vs-5-segment width fight (#234/#339). The active-workout banner is unchanged. WorkoutStartSection now provides Add in every idle state, so the empty-state card no longer renders its own Add (that would double it) and the dead `loaded` gate is dropped. Add is paused only during an in-progress live session (the banner owns that state) and returns the moment the workout ends. Display-only: Start is still bonded-gated (a live session needs the strap), no change to when it's available. Android-only. compileFullDebugKotlin + i18n audit green; layout not device-verified (Compose does not render on Linux).
Co-lay the Start-workout and Add-workout controls as an equal-width action row on the Workouts screen, matching the iOS workoutActionRow (#1068). When a strap is bonded they sit side by side; unbonded shows Add alone (moved up out of the range bar so it stays reachable without a strap), which also frees the range pill from the old Add-vs-5-segment width fight (#234/#339). Add stays visible in every state: beside Start when bonded-idle, alone when unbonded, and below the live banner during an active workout — so no control disappears in any state. WorkoutStartSection is now the single home for Add, so the empty-state card no longer renders its own (that would double it) and the dead `loaded` gate is dropped. Display-only: Start is still bonded-gated (a live session needs the strap), no change to when it's available. Android-only. compileFullDebugKotlin + i18n audit green; layout not device-verified (Compose does not render on Linux).
Bring the Android Workouts screen closer to the iOS redesign (#1068): - Co-lay Start + Add as an equal-width action row (EXP-018). Bonded → side by side; unbonded → Add alone (moved up out of the range bar so it's reachable without a strap, which also retires the #234/#339 Add-vs-range-pill fit fix); during an active workout the live banner keeps Add below it. Add stays visible in every state; WorkoutStartSection is its single home so the empty-state card no longer doubles it, and the dead `loaded` gate is dropped. - Give the sport picker per-sport glyphs (shared `sportIcon` catalogue) so it reads by icon like the iOS workout selection screen and the Workouts list rows, not a bare text list. - Move the Clear-filters chip onto the search-field row with a 48dp hit target, matching iOS's search-row placement + 44pt target. Display-only: Start is still bonded-gated (a live session needs the strap), no change to when it's available. Android-only (iOS via #1068). No new strings. compileFullDebugKotlin + i18n audit green; layout not device-verified.
Summary
Follows up on the #103 investigation. That thread ruled out SpO2 and landed
a proof-of-concept: respiratory rate can be recovered from the WHOOP5 v26
single-wavelength 24Hz optical PPG buffer via burst-level spectral analysis
(RIAV/RIIV band search, 0.15–0.40Hz).
NOOP already ships a respiratory-rate estimator for WHOOP5 —
SleepStager.respRateFromRR(RSA, from R-R intervals) — feedingDailyMetric.respRateBpmand an illness-detection signal inReadinessEngine. So this is a second, more accurate estimator, addedas a default-off, diagnostic-only Experimental toggle (never the shipped
default, per the project's derived-biosignal validation standard):
PpgResp(SwiftPackages/WhoopProtocol, Kotlincom.noop.protocol): a burst-level estimator on the same v26 bufferPpgHralready reads, one estimate per ~40s burst. Validated againsttwo real overnight BLE captures vs. the WHOOP app's own reading:
within ~2–6%, vs. the RSA estimator's 6–11% low bias on the same
nights. A band-edge guard (
bandEdgeGuardHz) rejects peaks pinned atthe band floor by residual low-frequency drift — confirmed on real data
(see the review-response below) and it improved accuracy (top-burst
median 14.04 → 14.52 vs. 14.6 truth).
SleepStager.respRateFromPpg: filters bursts to a sleep session,requires ≥9 qualifying bursts (else NaN), median of the top-third by
confidence.
DailyMetric.respRateBpm(and hence the
ReadinessEnginez-gate) always comes from RSA. ThePPG estimator never overrides it — a value that silently switched
estimator night to night could inject a ~1–1.5 bpm step and brush the
illness WATCH threshold on its own (a false signal from the switch, not
physiology). When the toggle is on, PPG is only computed alongside RSA
and logged (
rsa=… ppg=… used=rsa) through the existing diagnostic sink —the "keep the instrumentation" ask.
PuffinExperiment.ppgRespRate(default false), mirroring theexisting
ppgHrSubLagInterp/hrvReadinesspattern, with a Test CentreExperimental-algorithms row on both platforms. Default path is
byte-identical to what's shipped today.
ppgRespSampletable, wired into device-scopeddelete, timestamp healing, and the raw-sensor CSV export on both
platforms. Decoded + persisted unconditionally as instrumentation
regardless of the toggle. Migration numbers: GRDB
v27-ppg-resp-sample/ RoomMIGRATION_19_20(schemaVersion 20),renumbered from v26/18→19 after
v26-efficiency-heallanded in that slotfirst (the coordination note in the review — resolved).
Test plan
swift test— WhoopProtocol (288), WhoopStore (263), StrandAnalytics(1048), all green
./gradlew compileFullDebugKotlin+testFullDebugUnitTest— greenxcodebuildof the macOSStrandapp target — BUILDSUCCEEDED (touches app-target files
app-build.ymlCI doesn't coverby default)
actual production code path
path.