fix(protocol): v18 @36 is a flag byte, @106/@107 two u8 channels

[vishk23]

Three field-model errors in the WHOOP 5/MG v18 historical decoder, each established over 18,650 real v18 records — 18,602 from digitalerdude's public HCI capture (whoop5-hci-capture-2026-07-10) plus 48 from a second phone that cross-validate the structural claims on completely different magnitudes.

All three are corrections to readings, not new offsets: the bytes were already decoded, just modelled wrongly. No new physiological claims are made, and nothing is renamed to a metric without ground truth.

1. @36 is a flag byte, not a fixed-point HR

hr_fixed_8_8 read [36:38] as a u16 with bpm = value/256. The data rejects that:

• Bit 4 of @36 is never set — 0 of 18,650 records. A genuine 8.8 fraction sets it ~50% of the time. It is also the only bit never set.
• 95.02% of @36 values land in 0x80–0x8F (uniform would be 6.25%), across just 40 distinct values; sd 26.5 vs 73.9 for a uniform byte.
• @37 equals heart_rate@22 exactly in 99.575% of records (18,523/18,602), differing only by −6…+2.

The corr 0.989 that justified the old name was circular: the u16 is literally hr@22 (at @37) plus a flag byte over 256, so the residual is a flat +0.504 ± 0.189 — that is @36/256, not a sub-bpm fraction.

Now emitted as hr_quality_flags (u8 @36) and heart_rate_alt (u8 @37). Bit 7 reads as a validity bit: with it clear (n=748) rr_count == 0 in 70.32% of records vs 19.82% with it set, and the @108/@109 sentinel fires in 69.65% vs 1.32% — the two flags report the same quality condition. On records where bit 7 is clear the old model produced absurd output; one fixture in this repo decoded to a 227 bpm "higher-precision heart rate".

2. @106 is two u8 channels, not a u16

optical_baseline_106 read [106:108] as a u16 LE. Structurally impossible:

• Across 18,599 consecutive-second pairs, the high byte changed while the low byte stayed frozen in 3,514 (18.89%).
• There are zero low-byte wrap events in the entire corpus. A real u16 cannot step its high byte without a carry.
• The apparent u16 deltas are exactly 256·Δ@107 + Δ@106, clustering at 0, ±1, ±255, ±256, ±257, ±513.

They are two correlated but independent u8 channels (corr +0.73, moving in opposite directions in 5.8% of pairs where both move), structurally parallel to the @108/@109 pair. Now optical_baseline_a / optical_baseline_b.

The old "128 = invalid" note was also wrong here: 0, not 128, marks off-wrist — both bytes read 0 in exactly the 8 records that also carry HR == 0, while 128 occurs unremarkably while worn (@106 in 10 records, @107 in 103). Magnitudes are device-specific (102–255 / 119–247 on one strap vs 20–66 / 34–81 on another), so no scale is asserted.

3. Two wrong notes on @108/@109

• The 128 sentinel is record-level, not per-channel. amp_a == 128 in 757 records and amp_b == 128 in 757 — the same 757, never one without the other.
• They do not rise with heart rate. The documented climb (~34 at HR 40–49 → ~58 at 80–89) was an averaging artifact of counting the 128 sentinel as a number. Including sentinels: 33.50 → 57.66. Excluding them: 32.45 → 29.52 — flat to slightly declining. The real monotone trend is with motion: ~32.7 while still (dyn_acc < 0.02 g) → 37.4 at 0.05–0.2 g.

Also documented: the sentinel is a usable per-second signal-quality flag. It fires on 4.02% of worn seconds and predicts the band's own beat-detection failure (rr_count == 0) at 79.44% vs 19.40% — a 4.09× lift that survives holding motion constant (4.11× within dyn_acc < 0.009 g), where shuffled and circular-shift nulls all sit at ~1.0×. Deliberately not described as SpO₂, blood pressure or a perfusion substrate: signal-quality/AGC is the supported reading and the wavelength identity is unknown.

Verification

Test-first: each corrected reading got a failing test before the decoder changed, and the new tests pin the model rather than just values —

• Mutating @107 must leave the @106 channel untouched (impossible under the u16 read).
• The two second-device fixtures, both bit-7-clear, carry @37 = 97 and 227 against HR 57 and 63.
• The 128 sentinel must fire on both amp bytes or neither, across all four real fixtures.

Swift and Kotlin twins kept in lockstep, both decoder_oracle.json copies updated identically, plus tools/linux-capture/whoop_activity.py and the field table in docs/BLE_REVERSE_ENGINEERING.md (the retired reading is kept as a struck-through row explaining why the correlation was circular, so it does not get re-derived later).

• swift test (WhoopProtocol): 340 passed, 0 failures
• ./gradlew testFullDebugUnitTest: 3,063 tests, 0 failures attributable to this change
• tools/linux-capture tests: all pass

One pre-existing failure exists on main and is unrelated to this PR: com.noop.ui.SyncChipStateTest > lastSyncedAt_takesPriorityOverHistorySyncExperimental fails on pristine main (verified at cca23d6 in a clean worktree). shortSyncAgo's < 60 s branch is the only one calling uiString(R.string…), which cannot resolve in a plain JVM unit test, and that test passes now - 5. Happy to send a separate PR for it.

No BLE/hardware behaviour is touched — this is decode-only, covered by tests that run with no app, no strap and no CoreBluetooth.

[ryanbr]

Good work, and the evidence is the right kind — structural impossibility rather than correlation.

I had already verified the @36 finding independently from the doc's own worked example: 25997 = 101·256 + 141, so "101.55 bpm" is literally frame[37] + frame[36]/256. The 0.989 correlation was guaranteed by construction. Your +0.504 ± 0.189 residual is the same fact measured properly, and your example has @37=101 against a documented @22=102 — one of the 0.425%, which corroborates rather than undercuts it.

The @106 argument is the strongest of the three. Zero low-byte wraps across 18,599 consecutive pairs while the high byte moves in 18.89% is not a statistical claim, it is arithmetic: a little-endian u16 cannot carry without wrapping. That settles it on its own.

And the sentinel correction is the kind that is easy to miss — the documented "amps rise with HR" reversing from 33.50→57.66 to 32.45→29.52 once the 128s stop being averaged in as numbers. An artifact of including a sentinel in a mean is exactly how a plausible-looking trend gets into a doc.

You also did the three things I asked for on #848: corrected the doc rather than only the decoder, kept the retired reading with why it was circular so nobody re-derives it, and renamed in the same change.

One blocker, and it is a merge-order problem rather than anything wrong here.

#848 is open and mergeable, and it extracts its aux slot by decoder key:

rawU16At36: p["hr_fixed_8_8"]?.intValue      // HistoricalStreams.swift:289

This PR removes that key. If #861 lands first, that lookup returns nil and the slot silently banks nothing — no compile error, because it is an optional dictionary read, and no visible symptom, because absence is a first-class state in the aux format. Silent data loss in a capture format whose entire purpose is preserving fields before the strap trims them.

To be clear this is not an argument against merging #861 first — it should go first, it is the correction. But #848 then needs:

• the lookup pointed at hr_quality_flags / heart_rate_alt
• rawU16At36 (width 2) reconsidered, since after this there is no u16 at @36 to read — probably two u8 slots

Credit where due: #848 already renamed that slot away from hrFixed88 to the neutral rawU16At36 with a comment explaining the circularity, which was my ask. The lookup key just did not follow.

Happy to merge this as soon as you want; flagging so the two do not land in the wrong order.

[vishk23]

Thanks — and the merge-order catch was the valuable half of that review. Agreed on both counts: this should land first, and #848 needed to adapt.

#848 is now adapted and safe for you to merge this whenever you like.

The audit turned up a second instance of the same bug class, not just the one you spotted. Both retired keys were fused u16s that #848 was banking as single slots, so both had to split rather than merely repoint:

retired here	#848 now reads
hr_fixed_8_8 (u16 @36)	hr_quality_flags (u8) + heart_rate_alt (u8)
optical_baseline_106 (u16 @106)	optical_baseline_a (u8) + optical_baseline_b (u8)

The other 13 slot keys and the hist_version gate are untouched by this PR. Side effect: that removes #848's one deliberate slot-name exception — every slot is now the decoder's own name verbatim.

The durable fix, which matters more than the repoint: #848 now carries everySlotDecoderKeyExistsInARealV18Decode and everySlotIsPopulatedFromARealV18Frame on both platforms. The first pins that every slot's decoderKey resolves in a real v18 decode; the second pins that the value actually reaches the sample, since the key string and the extractor literal are separate strings and only the second one is load-bearing. A future key rename now fails a test instead of silently banking nothing — which is the failure mode you identified.

Heads-up so red checks don't surprise you: #848's CI is currently red by exactly those five tripwire tests, because the keys genuinely don't exist yet on main. That's the guard doing its job, and it makes an out-of-order merge mechanically impossible rather than merely discouraged. I verified fix/v18-decode-model merges into #848 with zero conflicts, and post-merge it's green: WhoopProtocol 363/0, WhoopStore 319/0, Android 3092/1 (the one being the pre-existing unrelated SyncChipStateTest), macOS build succeeded. So merging this turns #848 green with no further action from either of us.

One incidental confirmation from the Android run: after #854's aapt2 pin, ./gradlew testFullDebugUnitTest now works on macOS without --dependency-verification=off. Thanks for that one — it removes a step that had been quietly weakening local verification.