Persist SDNN alongside RMSSD (5-min index); fix HealthKit SDNN export mislabel#475
Open
vishk23 wants to merge 3 commits into
Open
Persist SDNN alongside RMSSD (5-min index); fix HealthKit SDNN export mislabel#475vishk23 wants to merge 3 commits into
vishk23 wants to merge 3 commits into
Conversation
…islabel DailyMetric.avgHrv carries RMSSD for WHOOP but SDNN for Apple — a metric overload flagged on Discord. HealthKitBridge compounded it by exporting the RMSSD value under HealthKit's heartRateVariabilitySDNN identifier, the only HRV type HealthKit exposes. - Add dailyMetric.avgSdnn column (migration v29-daily-avg-sdnn), struct field, upsert, and read. - Compute genuine whole-night SDNN in AnalyticsEngine from the in-bed raw R-R via HRVAnalyzer.analyze().sdnn (Task Force ddof=1), persisted per day. - Apple ingest mirrors its native SDNN into avgSdnn; the macOS import lane maps d.hrvSDNN. - Export prefers avgSdnn under heartRateVariabilitySDNN, falling back to avgHrv only when SDNN is absent (pre-rescore rows, or summary-only Oura with no raw R-R to derive SDNN from). - Thread avgSdnn through the fillingNilFields / sleep-field rebuilders so the new column survives row reconstruction. - Tests: v29 column presence, RMSSD/SDNN round-trip distinctness, upsert update.
The nightly SDNN persisted + exported under HealthKit's heartRateVariabilitySDNN must be window-matched to a watch's own SDNN, or it corrupts the user's Health history and makes any cross-check meaningless. Validated on real R-R (a WHOOP night, 20.9k beats): whole-night SD = 135 ms vs the 5-min SDNN index = 94 ms — the whole-night value is inflated ~40% by the slow HR drift across sleep stages, which Apple's short-window SDNN never sees. - Add HRVAnalyzer.sdnnIndex(rr, segmentSec: 300): the Task Force SDNN index, the mean of per-5-min-segment SDNN, each segment cleaned by the same range + Malik ectopic rejection the nightly path uses. - AnalyticsEngine.avgSDNNDaily now computes the index (keeping the R-R timestamps for segmentation) instead of a single whole-night SD. - Doc/comment updates: avgSdnn is the 5-min index; the HealthKit export note explains why the window match matters. - 4 tests, incl. the key property that the index strips inter-segment drift (steady-in-segment / drifting-across → index << whole-night SD). avgSdnn stays nil until a device re-scores from raw R-R (unchanged).
vishk23
changed the title
Upstream moved 260 commits since this branch's merge base. Three conflicts, all resolved keeping the PR's intent: - `Database.swift` — upstream added v27-ppg-waveform, v28-raw-imu, v29-score-input-provenance and v30-rr-ord. This branch's `v29-daily-avg-sdnn` collided with upstream's v29, so it is renumbered to **v31-daily-avg-sdnn** and appended after v30. Purely additive nullable column, unchanged otherwise. - `MetricsCache.swift` — upstream refactored the daily upsert SQL out of the instance method into the `static upsertDailyMetrics(_:deviceId:in:)` transaction-sharing primitive (used by the new score-provenance writer). Took upstream's structure and added `avgSdnn` to the shared SQL instead, so the ordinary cache write and the score+provenance write cannot drift. - `HRVAnalyzerTests.swift` — pure additive collision with upstream's #550 `collapsedCoverage` tests; both test blocks kept. Doc/test references to "v29" updated to v31. Verified: WhoopStore 299/299, StrandAnalytics 1142/1142, and the macOS app target builds (`xcodegen generate` + `xcodebuild -scheme Strand -destination 'platform=macOS'` → BUILD SUCCEEDED), which no CI job covers.
Author
|
Rebuilt on current
The diff against current Verified on the merged tree: Still a draft; happy to mark it ready if the shape looks right. |
vishk23
marked this pull request as ready for review
July 27, 2026 04:50
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
1 participant
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.
What
DailyMetric.avgHrvcarries a different HRV metric depending on the source: strap/WHOOP-derived rows store RMSSD (beat-to-beat, vagally-mediated), while Apple-Health-imported rows store SDNN (whole-night, broad). HealthKit exposes only one HRV type —heartRateVariabilitySDNN— and the write-back exported the strap's RMSSD value under that SDNN identifier, mislabeling it in the user's permanent Apple Health record.HRVAnalyzeralready computed SDNN internally, but the value only reached a diagnostic log line and was never persisted.(Flagged by a community member who traced the pipeline in Discord — thanks for the careful report.)
Changes
dailyMetric.avgSdnncolumn (migrationv29-daily-avg-sdnn), struct field, upsert, and read.avgHrvkeeps carrying RMSSD;avgSdnncarries SDNN — two distinct columns instead of one overloaded field.HRVAnalyzer.sdnnIndexcomputes the Task Force SDNN index — the mean of per-5-min-segment SDNN, each segment cleaned by the same range + Malik ectopic rejection the nightly path uses. This is deliberate: a single whole-night SD is dominated by the slow HR drift across sleep stages and reads far higher than a watch's short-window SDNN, which would corrupt the user's Health history and make any cross-check meaningless. Validated on a real WHOOP night (20.9k beats): whole-night SD = 135 ms vs the 5-min index = 94 ms (RMSSD 72 ms for reference).avgSdnn; the macOS import lane mapshrvSDNN.avgSdnn(the window-matched index) under.heartRateVariabilitySDNN, falling back toavgHrvonly when SDNN is absent — a pre-rescore row, or a summary-only source (e.g. Oura) that provides RMSSD with no raw R-R to derive SDNN from, where HealthKit's single HRV type leaves no better label.avgSdnnthrough thefillingNilFields/ sleep-field rebuilders so the column survives row reconstruction.Tests
HRVAnalyzerTestsadds 4sdnnIndextests, incl. the key property that the index strips inter-segment drift (steady-within-segment / drifting-across → index ≪ whole-night SD).WhoopStoreTestsadds v29 column-presence, RMSSD/SDNN round-trip distinctness, and upsert-update coverage. macOS + iOS targets build clean on this base;WhoopStore+StrandAnalyticssuites pass.Android parity
Swift-only change; the Android/Kotlin twin (Room migration +
sdnnIndex+ the export fix) is being prepared in parallel per the project's parity requirement (#417). Draft until the twin is up.Migration-id note
The migration is registered
v29-daily-avg-sdnneven though this branch's last migration isv26— the id is kept stable to match the contributor's fork lineage, so a later sync in either direction never double-applies theavgSdnncolumn (GRDB tracks applied migrations by id, not by number).