Skip to content

[oura][hrv] ord is 0 on every R-R row — the Oura path enqueues one event per batch (root cause for #823) #1072

Closed

Description

@pipiche38

Labels: bug, oura, hrv
Affects: main @ 3b86b6ef (Release 9.3.1). Not branch-specific.
Root-causes: #823 — "Same-second R-R beats are read in magnitude order, not emission order — biases RMSSD down"
Depends on: the R-R channel split (see the two-optical-channels issue) — land that first.


Summary

v30-rr-ord added an ord column so a second's beats could be read in emission order instead of
magnitude order. The store-side machinery is correct. But on the Oura path ord is written 0 for
every row
, so the fix never took effect and #823's symptom is still live.

Observed in a real database: 575,630 / 575,630 rows have ord = 0 — the value 0, not NULL. NULL
would mean "pre-v30, never recorded"; 0 means the write path ran and computed 0 every time.

Why it matters

Five to six beats share each timestamp (records arrive ~every 2.8 s). With ord tied, reads fall
through to the (rrMs, seq) ordering in the primary key — i.e. sorted by value. RMSSD is built
entirely from successive differences, and sorting a sequence minimises those differences by
construction. This is precisely the bias #823 describes.

Root cause

The store is not at fault. Packages/WhoopStore/Sources/WhoopStore/StreamStore.swift:189 keeps a
correct per-batch counter:

var ordByTs: [Int: Int] = [:]
for r in streams.rr {
    …
    let ord = ordByTs[r.ts] ?? 0
    ordByTs[r.ts] = ord + 1
    try stmt.execute(arguments: [deviceId, r.ts, r.rrMs, seq, ord])
}

The counter is batch-local, which the v30 comment already flags:

"Same batch-local caveat as seq: a second split across two live flushes restarts ord at 0 …"

The bug is at the call site. Strand/BLE/OuraLiveSource.swift enqueues one event at a time —
seven such sites, including the R-R arm at :1192:

case .ibi(let ibi):
    …
    if let ts = driver.unixSeconds(forRingTimestamp: ibi.ringTimestamp) {
        enqueue([e], ts: ts)          // ← single-element array
    }

Every batch therefore contains exactly one R-R row, ordByTs is always fresh, and every beat is
written ord = 0. The same applies to seq.

Proposed fix

Accumulate a whole record's events and enqueue them as one batch, so the per-batch seq / ord
counters see that record's beats together. The change is at the call site; the store needs nothing.

Points to keep intact while batching:

  • the per-arm anchoring logic (each stream resolves its own ts from its ring timestamp);
  • the deliberate asymmetry on noteStoredHistoryRingTime — the .ibi arm must still not advance
    the resume cursor, since a live beat could otherwise leap maxStoredRingTime to ~now during a
    force-stopped drain and permanently skip the backlog;
  • the .sleepPhase arm, which is handled at record level by the hypnogram assembler and must stay
    outside the batch.

Sequencing

Land after the R-R channel split. Until rrInterval carries one optical channel per read, ord
would be numbering an interleaving of two sensors measuring the same beats, which is not an emission
order and would make the metric look fixed while still being wrong.

Tests

The helpers for this already exist and are currently unexercised on this path:

  • StreamStore.rrOrdValuesForTest(deviceId:ts:) — assert a multi-beat record yields 0, 1, 2, …
    rather than all zeros.
  • StreamStore.insertLegacyRrWithoutOrdForTest(…) — keeps the pre-v30 NULL branch covered, so the
    read-order fallback for existing user data stays tested.
  • Add an OuraLiveSource-level test that one record's beats reach the store in a single batch.

Verification

⚠️ This touches app-target Swift (Strand/BLE/OuraLiveSource.swift), which no default CI job
builds
— swift-packages.yml covers Packages/** only and app-build.yml is disabled. A compile
error here passes every green check.

  • Build locally: xcodegen generate && xcodebuild -project Strand.xcodeproj -scheme Strand -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO build, and the NOOPiOS scheme too.
  • ⚠️ BLE path — compile success proves nothing. Needs a real drain against a ring: confirm a
    post-fix capture shows ord values 0,1,2,… within a second, and confirm the resume cursor still
    advances correctly across a full offload (bytes_left 0).

Android has the identical defect

Not a Swift-only bug — the Kotlin twin has the same shape and needs the same fix in the same PR:

  • android/…/ble/OuraLiveSource.kt:1024 and :1825 — enqueue(listOf(event), ts), one event per
    batch, exactly as on Apple.
  • :1000 — for (batch in snapshot) { OuraStreamMapping.streams(batch.events) { batch.ts } … persist(out, deviceId) },
    so each single-event batch becomes its own persist call.
  • assignRrSeq (android/…/data/WhoopRepository.kt:217) carries the same batch-local contract as the
    Swift ordByTs counter, so it restarts per beat for the same reason.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions