fix(oura): hand the store a record's beats at once so beat order stops being 0 - #1082
Merged
Merged
Conversation
…s being 0 `rrInterval.ord` (v30, ryanbr#823) records a beat's position among the beats that share its second, so a second's beats can be read in emission order instead of sorted by value. The counter that computes it is batch-local by design — one insert is the only place the order still exists — and the Oura transport enqueued one event per batch, so it restarted on every beat and wrote `ord = 0` on every row: 575,630 of 575,630 in a real database (ryanbr#1072). With `ord` tied, the read falls through to `(rrMs, seq)`, i.e. magnitude order, which minimises successive differences by construction and biases RMSSD down — exactly the symptom ryanbr#823 reports. The store was never at fault; the call site was. A record's beats all carry that record's ring time, so the anchored `.ibi` events of one ingest are now grouped by resolved second and enqueued as one batch, on the live path and when parked beats are released after the 0x42 anchor lands. Grouping is a pure `OuraStreamMapping.batched` on both platforms, so the ordering logic is covered by package tests rather than living only in app-target code that no default CI builds. Two behaviours are deliberately unchanged: an IBI still never advances the resume cursor (a live beat could otherwise leap `maxStoredRingTime` past un-drained backlog), and unanchored beats still park rather than being stamped with a guess. Side-effect, pinned by a test: `seq` is batch-local for the same reason, so two beats sharing an interval value in one record used to collide on the primary key and the second was silently dropped. Batched, both are stored — a post-fix night holds slightly more rows because real beats stop being lost, not because duplicates are invented. Verification: `swift test` in WhoopStore (354 pass, incl. new ord/seq batching tests); `./gradlew testFullDebugUnitTest` (3 pre-existing locale failures on `main`, unchanged); both app targets built locally — `xcodebuild` Strand (macOS) and NOOPiOS — since `app-build.yml` is disabled and no default CI compiles them; `StrandTests` shows the same 73 pre-existing locale failures as `main`, no new ones. NOT yet validated on a real drain: `ord = 0,1,2,…` within a second and an unchanged full-offload resume path still need a post-fix overnight capture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AiQZ5XBeAMgnyUugPMqXPi
ryanbr
marked this pull request as ready for review
August 5, 2026 08:04
This was referenced Aug 5, 2026
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
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.
Fixes #1072. Root cause for #823 — that issue's symptom is still live on
maintoday.Branch:
fix/oura-rr-ord-batching-1072(based onmain@7c7b3f0c, commitee79530e)Base:
main— the defect is byte-identical onmain, not an integration-branch regression.Depends on: #1071 (merged as #1076,
fddf9038). Required: while two optical channels wereinterleaved in one stream,
ordwould have numbered an interleaving of two sensors measuring thesame beats, which is not an emission order.
No migration. No schema change, no stored-value redefinition — this is a call-site fix. The
column and its counter have been correct since v30.
The bug
v30-rr-ordaddedrrInterval.ordso a second's beats could be read in emission order ratherthan magnitude order. The store-side machinery is right. On the Oura path
ordis written 0 onevery row: measured in a real database, 575,630 / 575,630 rows — the value 0, not NULL (NULL
would mean "pre-v30, never recorded"; 0 means the write path ran and computed 0 every time).
The counter is batch-local by design — one insert is the only place the order still exists — and
the v30 comment says so:
Strand/BLE/OuraLiveSource.swiftenqueued one event per batch:Every batch therefore held exactly one R-R row,
ordByTswas always fresh, and every beat gotord = 0.android/…/ble/OuraLiveSource.kthad the identical shape (enqueue(listOf(event), ts)).Why it matters
Five to six beats share each timestamp (records arrive ~2.8 s apart). With
ordtied, reads fallthrough to the
(rrMs, seq)ordering in the primary key — sorted by value. RMSSD is builtentirely from successive differences, and sorting a sequence minimises those differences by
construction. That is precisely the bias #823 describes; on that issue's own example, magnitude
order reads 12.72 ms against 34.85 ms in emission order.
The fix
All of one record's beats carry that record's ring time, so grouping the anchored
.ibievents ofone ingest by their resolved second and enqueuing each group once hands the store a record at a
time — and
ordbecomes their real emission order.OuraStreamMapping.batched(SwiftPackages/WhoopStore, Kotlincom.noop.data), so the ordering logic sits where package CI covers it instead of only inapp-target code that no default CI builds.
ingest/emit) and where parked beats are released after the 0x42anchor lands (
drainPendingAnchorEvents), which is where a drain's first beats go..ibiarm now owns only the live readout and parking; it no longer enqueues.Deliberately unchanged:
noteStoredHistoryRingTime) — a live beat couldotherwise leap
maxStoredRingTimeto ~now during a force-stopped drain and permanently skipun-drained backlog;
.sleepPhasestays outside the batch — it is assembled at record level by the hypnogram assembler.One behaviour does change, and it is a recovery
seq(v24) is batch-local for the same reasonordis, and it exists precisely so two beats sharingan interval value in one second survive as distinct rows. Insert-per-beat recomputed
seq = 0forthe second beat, so it collided on
(deviceId, ts, rrMs, seq)and was silently dropped byON CONFLICT DO NOTHING. Batched, both are stored.So a post-fix night holds slightly more R-R rows than a pre-fix one — real beats that were being
lost, not duplicates invented. Worth knowing when reading coverage numbers against the pre-fix
baseline in #1071's validation: a small rise there is expected and is not a regression. Pinned by
testEqualIntervalsInOneRecordSurviveOnlyWhenTheRecordIsOneInsert.Tests
Package-level (runs in
swift-packages.yml/./gradlew testFullDebugUnitTest, no strap, no app):OuraStreamMappingTests/OuraStreamMappingTest— a record's beats become one batch;distinct seconds stay separate and in arrival order; interleaved same-second events fold into one
batch with relative order preserved; empty input yields no batches. The Kotlin twin asserts the
resulting
ordend-to-end throughStreamPersistence.toBatch+assignRrSeq:0,1,2,3,4.MigrationTests.testV30OrdIsBatchLocalSoOneInsertPerBeatRecordsNoOrder— pins both shapes sideby side: one insert per beat can only ever write
ord = 0and reads back in magnitude order; thesame beats in one insert keep emission order. Kotlin twin:
onePersistPerBeatRecordsNoOrder.MigrationTests.testEqualIntervalsInOneRecordSurviveOnlyWhenTheRecordIsOneInsert— theseqrecovery above.
Verification
swift testinPackages/WhoopStore: 354 pass, 0 failures../gradlew compileFullDebugKotlinclean;./gradlew testFullDebugUnitTest: 3,473 tests, thesame 3 pre-existing locale failures as clean
main(AiCoachContextTest,StandardHrSensorFormatTest×2) — verified by re-running them on a stashed tree.app-build.ymlis disabled). Built locally:xcodebuildStrand(macOS) andNOOPiOS(iOS Simulator) — both BUILD SUCCEEDED.StrandTestsshows the same 73 pre-existing locale failures as cleanmain(1010 tests), nonenew.
nothing. A post-fix drain still has to show:
ordrunning0,1,2,…within a second inrrInterval, and the resume cursor still reachingbytes_left 0across a full offload.What this does not fix
It does not by itself make Oura SDNN physiological. The validation night for #1071 showed a third
duplicated channel (0x60/0x44) dominating the scoring read, and beats banked in bursts rather than at
their true times.
ordis the prerequisite for deciding channel selection on beat-accurate datarather than on burst counts — it is not the decision.