Skip to content

[hrv] Same-second R-R beats are read in magnitude order, not emission order — biases RMSSD down #823

Open

Description

@ryanbr

Both platforms read R-R intervals sorted by value within a second, not by the order the heart produced them. RMSSD is built entirely from successive differences, and sorting makes adjacent values similar by construction, so this biases it downward.

android/app/src/main/java/com/noop/data/WhoopDao.kt:303   ORDER BY ts ASC, rrMs ASC, seq ASC
Packages/WhoopStore/Sources/WhoopStore/Reads.swift:115    ORDER BY ts ASC, rrMs ASC, seq ASC

Spotted by @tanarchytan (d95f3f4e on their fork).

Size of the effect

v18 emits 1–6 beats per whole-second timestamp, so every multi-beat second is a candidate. A realistic five-beat second:

emission order   812, 795, 840, 801, 833  → RMSSD 34.85 ms
magnitude order  795, 801, 812, 833, 840  → RMSSD 12.72 ms
                                             bias  −22.14 ms

@tanarchytan measured 1.1 % of same-second groups actually reordering on 5/MG. Small in frequency, but it's a one-directional bias in a headline metric that feeds Charge — not noise that averages out.

Why the obvious fix doesn't work

Their fix changes the clause to ORDER BY ts, seq, on the stated basis that seq is an insertion counter. It isn't. assignRrSeq keys on (ts, rrMs), not ts:

val key = row.ts to row.rrMs
val s = seqByBeat.getOrDefault(key, 0)

seq counts repeats of an identical beat, so every distinct interval in a second carries seq = 0. Under ORDER BY ts, seq all of them tie and the order becomes whatever the scan returns — trading a deterministic wrong order for an undefined one, which is worse: RMSSD could then vary between reads of the same data.

Why the repair isn't trivial either

Making seq a true per-second emission index means keying on ts alone — which WhoopRepository.kt:176-181 warns against explicitly:

Keying by (ts, rrMs) — not ts alone — is deliberate: DISTINCT intervals keep seq 0 and thus their own (deviceId, ts, rrMs, 0) key, so a distinct beat is NEVER dropped even when same-second beats arrive in SEPARATE insert batches or via the live/historical merge (an earlier ts-only index would have restarted per batch and collided distinct beats — a data-loss regression).

So the current keying is load-bearing. A real fix needs a separate emission-order column populated at decode time, plus a Room + GRDB migration, plus a decision about what to do with already-stored rows that have no recorded order.

What would need deciding

  • Historical data. Rows already stored carry no emission order. Backfilling is impossible; a new column would only be right for records decoded after it lands.
  • Scale of the change. Schema change on both platforms with matching migrations, for a 1.1 %-of-groups effect. Worth confirming the real-world RMSSD delta on a full night before committing to that.
  • Interim option. Leaving ORDER BY ts, rrMs, seq is at least deterministic and reproducible. Documenting the known bias may be the right holding position until someone measures a night both ways.

Not filing a PR because the two-line version is a regression and the correct version needs a migration decision that isn't mine to make.

Activity

  1. ryanbr commented on Jul 26, 2026

    @ryanbr
    OwnerAuthor

    Implemented in #830 — all 9 CI jobs green, including WhoopStore, which exercises the fix end-to-end.

    Went with the schema change rather than the holding position, since the two-line version really is a regression and the interim option leaves a known one-directional bias in a headline metric.

    New ord column: the beat's position among all beats sharing its ts, stamped at decode time, leading the read sort. It is deliberately not in the key — the key stays (deviceId, ts, rrMs, seq), because a per-second insertion counter in the key restarts per batch and collides distinct beats, which is the data-loss regression assignRrSeq warns about.

    The historical-data question resolved itself cleanly. Pre-migration rows keep ord NULL — the order was never recorded, so it cannot be backfilled and a guess would assert something nobody observed. SQLite sorts NULL first in ASC, so an all-legacy second ties on ord and falls through to the old (rrMs, seq) order: existing data reads back exactly as before, with no COALESCE and no sentinel. Same on Room and GRDB since both are SQLite.

    Also caught in passing: the raw-CSV export ordered by ts alone, so same-second beats were undefined there — identical data could export two different ways.

    Still open, and the reason I would not call this closed by the fix alone: nobody has measured a full night both ways. The 1.1 % of groups is a frequency, not an effect size on a real night's RMSSD. Worth doing before we treat the bias as quantified.

  2. ryanbr commented on Jul 26, 2026

    @ryanbr
    OwnerAuthor

    Merged as 34c66b66 (#830).

    Leaving this open. The fix landed, but the question the issue itself raised — whether the 1.1 % of same-second groups translates into a meaningful nightly RMSSD delta — is still unanswered, and that is what tells us whether the bias mattered clinically or just arithmetically.

    Two residuals worth recording here rather than losing in the PR:

    1. A second split across two live flushes restarts ord at 0, and ON CONFLICT DO NOTHING keeps the first row — so that second still falls back to magnitude order. Same batch-local caveat as seq. A later DO UPDATE SET ord = excluded.ord WHERE ord IS NULL would both close this and heal legacy rows on re-sync, but it changes insert semantics and the changesCount contract, so it was left out.
    2. The Kotlin half is unrun — no SQLite driver on the JVM test classpath, and android.yml is disabled. The SQL is identical to the clause Swift CI executes, which is an argument, not a test.
  3. vishk23 commented on Jul 30, 2026

    @vishk23

    Leaving this open. The fix landed, but the question the issue itself raised — whether the 1.1 % of same-second groups translates into a meaningful nightly RMSSD delta — is still unanswered.

    I don't have that delta yet, but I do have a corpus measurement that changes how you'd have to go about getting it, so it seems worth recording here before someone spends a night on it.

    One wearer, one strap (WHOOP 5), 959,574 rrInterval rows in a single on-device DB, analysed clean-room against the file rather than through the app.

    ord is NULL on 97 % of rows

    Which is the documented, correct behaviour for v30-rr-ord — those rows predate the migration and the order was never recorded. But the consequence for the open question is sharp:

    On this device the fix currently governs ~3 % of the corpus. The other 97 % still tie on ord and fall through to (rrMs, seq), i.e. they still read back in magnitude order with the #823 bias intact — exactly as designed.

    So a "measure a full night both ways" experiment has to be restricted to rows written after the migration, or it measures nothing: replaying a legacy night through the new clause returns the identical order it always did. That is easy to get wrong, because nothing in the read path signals which regime a given night is in — ord isn't in the projection, so a consumer holding [RRInterval] cannot tell whether it is looking at emission order or magnitude order.

    If it's useful, SELECT COUNT(*) FILTER (WHERE ord IS NULL) * 1.0 / COUNT(*) FROM rrInterval is the one-liner for "how much of my data can this fix even reach", and I'd suggest anyone reporting an RMSSD delta on this issue quote it alongside the delta.

    Emission order and rowid agree, on this corpus

    Since ord is absent for most rows, I checked whether insertion order is recoverable another way. Ordering by rowid yields zero ts-monotonicity violations across all 959,574 rows — rowid never disagrees with ts about which beat came first.

    Being explicit about what that does and does not establish:

    So this is not a backfill route, and I'm not proposing it as one. It is a negative result worth having recorded: the obvious "just use rowid" idea does not rescue legacy rows, and the "cannot be backfilled" call in #830 still looks right.

    On residual #1

    A second split across two live flushes restarts ord at 0, and ON CONFLICT DO NOTHING keeps the first row — so that second still falls back to magnitude order.

    I can't yet say how often this fires in practice — distinguishing "restarted ord" from "legitimately started at 0" needs the flush boundaries, which aren't in the table. If anyone has a capture where flush boundaries are recoverable from the log, that would size the residual.

    Happy to run further queries against this DB if a specific one would help settle the delta question — it's one wearer's device, so it can size a corpus and rule things out, but it can't establish a population effect on its own.

  4. pipiche38 commented on Aug 4, 2026

    @pipiche38

    @ryanbr kindly look at #1072 I have a fix in place but it will required a database migration and would like to have #1076 merged before. I expect to have #1076 validated with hardware tomorrow.

  5. pipiche38 commented on Aug 6, 2026

    @pipiche38

    Not sure if this is only Oura related, or if this also exist with Whoop 4/5, but here are some hardware evidence on this, from an Oura overnight drain — plus a status read, since the pieces this issue asked for have landed in two separate places and it may not be obvious they meet.

    The two things this issue said a real fix would need are both in main now.

    1. "A real fix needs a separate emission-order column populated at decode time, plus a Room + GRDB
      migration"
      — that is ord, added in v30 explicitly for this issue (the comment at
      StreamStore.swift:182 cites [hrv] Same-second R-R beats are read in magnitude order, not emission order — biases RMSSD down #823 by number). It is populated at insert, and it is deliberately
      outside the primary key, so the (ts, rrMs) keying that WhoopRepository.kt:176-181 warns about
      stays load-bearing and untouched — no data-loss regression.
    2. The read clause is now, on both platforms:
      ORDER BY ts ASC, ord ASC, rrMs ASC, seq ASC
      
      (Reads.swift:203, WhoopDao.kt:392). Emission order first; the old magnitude order survives only
      as the deterministic fallback for pre-v30 rows where ord IS NULL — which is the right answer to
      the "historical data carries no emission order" point, since those rows genuinely have no better
      order available and the alternative is the undefined ordering this issue correctly rejected.

    What was still silently broken until last week: the write side on the Oura path.
    ord is batch-local by construction — "position among all beats sharing this ts in this batch".
    Strand/BLE/OuraLiveSource.swift was calling enqueue([e], ts:) one event per batch (7 sites),
    so the counter was fresh for every single beat and every row landed ord = 0. The v30 machinery was
    correct and doing nothing. Fixed in #1082 (a29c4612) by batching a whole record's events into
    one enqueue.

    First hardware confirmation, overnight drain 2026-08-05→06 (Gen-3 Oura ring, 9.1 h, build
    carrying #1082):

    • ord > 0 on 81.4 % of the night's 77,918 R-R rows — pre-fix this was 0 %.
    • 6-beat 0x60 records number ord 0..5 and arrive ~7 s apart; 91.6 % of (second, channel)
      groups carry a complete 0..n-1 run or a clean stack of them. Only 8 all-zero multi-beat groups
      remain out of 14,030.
    • Side effect worth knowing: seq was batch-local too, so two beats sharing an interval value inside
      one record used to collide on the PK and the second was dropped. Post-fix 5,148 rows carry
      seq > 0 and beats-per-record rose 5.79 → 5.91. A post-fix night therefore holds slightly
      more rows than a pre-fix one — that is recovery, not duplication.

    What this does NOT establish, so the close call is yours. This is an Oura capture. The
    measured RMSSD bias in the issue body is on WHOOP 5/MG.

    Cross-ref: #1082 (the Oura write-path fix, merged) and #1071/#1076 (channel labelling, which had to
    land first or ord would have interleaved two optical sensors).

  6. artemc commented on Aug 18, 2026

    @artemc

    A full night measured both ways, plus an ECG arbiter

    Leaving this open. The fix landed, but the question the issue itself raised — whether the 1.1 % of same-second groups translates into a meaningful nightly RMSSD delta — is still unanswered.

    Ran that measurement on seven nights of one WHOOP 4.0 (Android, current release), reading each night's primary sleep session twice out of the same export: once in the shipped emission order (ts, ord, rrMs, seq) and once in the pre-#830 magnitude order (ts, rrMs, seq). Same RMSSD rules both times — physiological intervals only, no difference taken across a gap of more than a second.

    Five of those nights also carry a simultaneous chest-strap ECG recording, so beyond "how big is the delta" there is an answer to "which order is closer to the truth".

    1. The coverage number first, as asked

    I'd suggest anyone reporting an RMSSD delta on this issue quote it alongside the delta.

    On this device: 513,577 rrInterval rows, ord IS NULL on zero of them. srcChannel is null throughout (single merged WHOOP array, as expected) and tsSuspect is never set.

    So this is the opposite regime to the 5.0 corpus reported here — there #830 governs ~3 % of rows, here it governs all of them. The obvious reading is that the corpus predates the v30 migration and mine postdates it, which makes the NULL share a property of an installation's history rather than of a strap generation. Worth keeping the query in any report for exactly that reason.

    Frequency of the thing being fixed, per night: between 2.5k and 17.2k seconds carry more than one beat, and of those, 11.0 % to 43.3 % (median 25.4 %) actually change order between the two clauses. That is an order of magnitude more often than the 1.1 % of groups measured on the 5.0 — the same fix reaches much more of a 4.0's data.

    2. The nightly delta is one-signed — and the sign is the opposite of the one the issue predicts

    Emission order gives a lower RMSSD than magnitude order on 7 nights out of 7:

    value
    emission − magnitude −1.1 to −11.5 ms, median −2.2 ms
    same, relative −1.5 % to −7.0 %, median −2.7 %

    The issue body reasons that magnitude order biases RMSSD down, because sorting makes neighbouring values similar. That is true inside a second and it is not the whole effect. Sorting by value also makes the step across the second boundary as large as it can be: the last beat of second N is the largest in that second, the first beat of second N+1 is the smallest in its own. Every multi-beat second contributes one artificially large successive difference at its right edge, and across a whole night that outweighs the smoothing inside the second.

    So on real data the artifact is better described as moving difference magnitude from inside seconds to the boundaries between them than as a one-directional downward bias. The five-beat example in the issue cannot show this, because it contains no boundary.

    None of that changes the conclusion that emission order is the correct thing to read — it just means the pre-#830 clause was reading RMSSD high on this device, not low, and anyone comparing historical numbers to post-migration ones should expect a small step down rather than up.

    3. Against ECG, the fix moves the number the right way

    Restricted to the seconds both streams cover, error = strap RMSSD − ECG RMSSD on the same window:

    read order mean absolute error vs ECG, n = 5
    emission (ord, shipped) 31.5 ms
    magnitude (pre-#830) 34.5 ms

    Emission order is closer on four of the five nights; on the fifth it is worse by 1.1 ms. So the migration is validated in the direction it was intended, by an independent reference rather than by construction.

    The caveat matters as much as the result: the gap between the two orders is single-digit milliseconds, while the residual error of either order against ECG is 14 to 76 ms on the same nights. Read order is real and it is second-order. What dominates on this strap is the time axis itself — the cross-second over-count and the record anchoring discussed in #1118 — and no choice of within-second ordering touches that.

    4. On residual #1 from the merge comment

    A second split across two live flushes restarts ord at 0, and ON CONFLICT DO NOTHING keeps the first row — so that second still falls back to magnitude order.

    I cannot size it either — the flush boundaries are not in the table, and with ord fully populated here there is no NULL signal to count against. One thing I can offer as a weak upper bound: if restarted-ord seconds were common, I would expect a visible population of multi-beat seconds whose ord values do not form a clean 0..n-1 run. That check is cheap on an export and I am happy to run it if the shape of the diagnostic is specified.

    Method and what this is not

    • One participant, one WHOOP 4.0, seven consecutive-ish nights for the ordering delta and five for the ECG comparison; recorded on a single app build.
    • Values are recomputed from the app's own export, not read from hrv diag, so treat them as directionally comparable rather than numerically identical to a strap log.
    • Two of the seven nights had their sleep boundaries edited by hand. That does not affect the ordering comparison — both orders read the same window — but the absolute RMSSD on those nights describes the edited window.
    • The ECG reference is transmitted R-R with no ectopic-beat correction.
    • This is one device's behaviour. In particular the NULL share, the multi-beat frequency and the sign of the delta are all properties of this corpus, and the sign in section 2 is a claim about a strap that produces many multi-beat seconds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions