tools: SpO2 @82 multi-device validation harness (#103) — with the fixes it needs to run by ryanbr · Pull Request #814 · ryanbr/noop

ryanbr · 2026-07-26T02:45:59Z

Carries @dnesdan's #808 unchanged as the first commit, then the three fixes it needs. Opened here because #808 is stuck at action_required and can't get CI on a fork.

Credit for the harness is entirely @dnesdan's — the design is right, and the offset-specificity scan and multi-device gating are exactly the shape #103 needs. What follows is about the ground it landed on.

1. The suite couldn't run on Linux at all

Tools/linux-capture/ and tools/linux-capture/ are two case-variant directories with zero overlapping filenames — 11 files and 17. On macOS they're one directory and everything works. On Linux they're separate, and every import across the boundary fails. validate_spo2_candidate.py imports whoop_activity (upper) and whoop_frame (lower), so it couldn't run in either. correlate_ground_truth.py was already broken this way on main.

Consolidated into tools/linux-capture/ — the side holding whoop_frame.py, the README, and most of the scripts, and the path the BLE docs already cite. Repo-root Tools/ is untouched.

	before (main)	after
`Tools/linux-capture`	Ran 2, FAILED (errors=2)	— merged away
`tools/linux-capture`	Ran 104, OK	Ran 159, OK (1 skipped)

2. "21 tests, OK" never included the new test

test_whoop_activity.py is pytest-style — 21 bare def test_*, no unittest.TestCase — so python3 -m unittest, the command the README documents and the only runner in requirements.txt, collects nothing from it and exits 0. That's indistinguishable from passing.

The 21 in #808's body is 8 + 13 from the other two files. The new @82 tri-mode decoder test lives in the file contributing zero and had never executed.

Fixed with unittest's documented load_tests hook rather than rewriting 21 working tests. Running them surfaced 4 real failures, all pytest-only dependencies — three took the tmp_path fixture, one called pytest.skip. Rewritten to stdlib (tempfile.TemporaryDirectory, unittest.SkipTest), both of which pytest also honours, so all 21 now run under either runner.

3. My own byte-82 docs from #812 were wrong

I documented @82 in #812 as an unexplained raw byte with a promising third-party lead. It's already decoded on both platforms as spo2_candidate_82 — tri-mode, sleep-only, with a guard test stopping it reaching spo2Pct and a full write-up in WHOOP5_DEEP_DATA.md. I grepped aux_byte_82, read a table row that omits the candidate, and never grepped spo2. "Stays raw and unnamed" and "no source for at all" were false.

Rewritten to the actual position: the identification isn't the open question — the split evidence is. Eight nights at corr +0.99 including a 92 % desat and a 98 % high, offset-specific in a 74–92 scan, against two nights on the #103 device that moved opposite to the app value. That contradiction is precisely what this harness resolves, which makes it the fix for the gap rather than a detour. whoop-local (#715) is recorded as corroboration of the identification, not its origin.

Verification

cd tools/linux-capture
python3 -m unittest discover -s . -p 'test_*.py'
# Ran 159 tests — OK (skipped=1)

Run on Linux, which is the point. The one skip needs a real capture DB. No app code, no BLE path, no schema, no scores, no strings — Python tooling and docs.

#103 stays open — this is the instrument for answering it, not the answer. Deliberately phrased to avoid GitHub's auto-close keywords, which fire regardless of any negation in front of them.

@82

Add validate_spo2_candidate.py to compare nightly mean of in-band v18 @82 samples (sleep_state=asleep) against WHOOP CSV blood_oxygen_pct across one or many straps. Reports r/MAE/bias, offset-specificity (74–92), and a promote checklist; --postable output has no raw SpO2 values for safe #103 sharing. Also: English export header aliases for correlate_ground_truth, spo2_candidate_82 on whoop_activity.decode_v18, docs for the multi-device bar, and unit tests.

@dnesdan

…e-82 docs Builds on @dnesdan's #808 (cherry-picked as the first commit). Three fixes it needs, plus a correction to documentation I got wrong. 1. THE CASE-SPLIT MADE THE SUITE UNRUNNABLE ON LINUX Tools/linux-capture/ and tools/linux-capture/ are two case-variant directories with ZERO overlapping filenames — 11 files and 17 files. On macOS they are one directory and everything works; on Linux they are separate and every import across the boundary fails. validate_spo2_candidate.py imports whoop_activity (upper) AND whoop_frame (lower), so it could not run in either. correlate_ground_truth.py was already broken this way on main. Consolidated into tools/linux-capture/ — the side that holds whoop_frame.py, the README and the majority of the scripts, and the path the BLE docs already cite. Repo-root Tools/ (i18n, release scripts) is untouched. before, on main: Tools/linux-capture Ran 2 FAILED (errors=2) tools/linux-capture Ran 104 OK after: tools/linux-capture Ran 159 OK (skipped=1) 2. THE "21 TESTS, OK" IN #808 NEVER INCLUDED ITS OWN NEW TEST test_whoop_activity.py is pytest-style — 21 bare `def test_*` functions, no unittest.TestCase — so `python3 -m unittest`, the command the README documents and the only runner in requirements.txt, collects NOTHING from it and exits 0. The 21 in the PR body is 8 + 13 from the other two files; the new @82 tri-mode decoder test is in the file contributing zero and had never executed. Added unittest's documented `load_tests` hook rather than rewriting 21 working tests. Running them then surfaced 4 real failures, all pytest-only dependencies: three took the `tmp_path` fixture, one called pytest.skip. Rewritten to stdlib — tempfile.TemporaryDirectory and unittest.SkipTest, both of which pytest also honours. All 21 now run under either runner. 3. THE BYTE-82 DOCS I MERGED IN #812 WERE WRONG I documented @82 as an unexplained raw byte with a promising third-party lead. It is already decoded on both platforms as spo2_candidate_82 — tri-mode, sleep-only, with a guard test stopping it reaching spo2Pct and a full write-up in WHOOP5_DEEP_DATA.md. I had grepped aux_byte_82 and read a table row that omits the candidate, and never grepped spo2. Claims like "stays raw and unnamed" and "no source for at all" were simply false. Rewritten to state the actual position: the identification is not the open question, the split evidence is — 8 nights at corr +0.99 including a 92% desat and a 98% high, offset-specific in a 74-92 scan, against two nights on the #103 device that moved OPPOSITE to the app. That contradiction is what #808's harness exists to resolve, which makes this PR the fix for the gap and not a detour. whoop-local is recorded as corroboration of the identification, not its origin. Suite: 159 tests, OK (1 skipped, needs a real capture DB), verified on Linux.

Consolidating Tools/linux-capture into tools/linux-capture left five references to the old path in WHOOP5_DEEP_DATA.md: a markdown link that would 404, two copy-paste `cd Tools/linux-capture` commands that fail on any case-sensitive filesystem, and two inline script paths. Three of them arrived with #808 itself and two predate it. All now point at the lowercase path; a repo-wide sweep for "Tools/linux-capture" returns nothing, and every relative link in the file resolves. Caught by grepping for the old path AFTER the move rather than assuming git mv was the whole job — the move is mechanical, the references are not.

…r ran (#815) Follow-up to #814, which fixed unittest collection for test_whoop_activity.py and missed the identical instance in the same directory — in a file that PR had moved. test_whoop_spot_hrv.py is also pytest-style: 8 bare `def test_*`, no unittest.TestCase. So `python3 -m unittest` collected nothing from it and exited 0, which is indistinguishable from passing. Its docstring told readers to use pytest, which is not in requirements.txt and is not what the README documents, so in practice these eight ran for nobody. All eight take no fixtures, so the same load_tests hook is enough. All pass. Docstring corrected to name both runners. Suite 159 -> 167, OK (1 skipped). A repo-wide sweep now finds no Python test file that unittest silently skips. Mutation-tested before merging, because a test that has never executed has never been shown to catch anything. Inverting the local-maximum comparison in find_peaks fails 2 of the 8, so the collected tests do guard real behaviour. Two mutations survived and are worth recording as a known coverage gap, not fixed here: dropping the min_dist spacing filter, and dropping the min_prom prominence gate, both leave all 8 passing. find_peaks has three behaviours and these tests pin one. Collecting them is still a strict improvement over running zero.

@dnesdan

…ty signals (#824) Re-implements the landable parts of #807 (@dnesdan) against current main. Their PR picked up a conflict when #814 rewrote the same @82 paragraph; writing fresh avoided moving a bigger conflict onto them. KEPT, AND THEIRS — the SpO2 protocol facts, recorded nowhere else: - No SpO2 read opcode is known. Our CommandNumber catalogue carries 80 commands and none reads oxygen. - Computed on-device during sleep — our own decode corroborates it, aux_byte_82 is nonzero only while the band sleep flag reads asleep. - The CSV blood_oxygen_pct is a per-cycle aggregate, read on the physiological-cycles row beside recovery_score_pct and day_strain (WhoopExportImporter.swift:272), so it will not equal a mean of wire samples. - A night with no export value is a real gap, not a decode failure. DROPPED — their model-id range table (0…37 = MG, 48…85 = 5.0). Not for provenance: #822 settled today that decompile-sourced facts are admissible with attribution, so the original objection no longer applies. Dropped because we already resolve identity a better way and use model-ids nowhere. Whoop5Variant reads the DIS serial prefix (5AM/5AG) and the WG50 hardware-revision token — shipped in #781, sourced from #520 — and that is documented here from our own code, including that contradictory signals return .unknown and that .unknown is not MG. CHANGED — the v18 field map is a pointer, not a copy. Their §8.1 duplicated the authoritative table in BLE_REVERSE_ENGINEERING.md, and drift had already started: theirs predates the dynamic_acceleration@41 row added earlier today. One table, one place to keep correct. FOUR REVIEW PASSES, ALL FINDING MY OWN MISTAKES - Wrong heading level and wrong parent: ## 7.9/7.10 under a section whose subsections are ### 7.1..7.5, and neither topic belongs under "Historical-data offload". Moved to top-level 9 and 10, File map renumbered to 11 after checking nothing cross-references §8/§9 and that the file has no TOC. - Overclaimed negative: "there is no GET_SPO2 opcode" restated as "none is known". §6 is explicitly a safe SUBSET and the 98-vs-87 dispute shows the map is incomplete; the catalogue is evidence, not proof about the strap. - Two inherited claims passed through unverified, now sourced: the per-cycle export shape (cited to our importer) and the DIS characteristics (cited to BLEManager.swift:473-474). - One claim that cannot be checked here — naps/incomplete nights carrying no export SpO2 — kept but explicitly marked a contributor observation, so it is not later cited back as established. Every claim in these sections is now either checked against this tree or labelled unverified. Docs only. Co-authored-by: Dan Urbanek <abc@dnesdan.cz>

Re-reviewing my own PR: the Swift twin's repo.hrBuckets(from:to:bucketSeconds:) UNIONs the active strap with the canonical "my-whoop" and dedupes by bucket start. I called Kotlin's single-id hrBuckets, so after a strap re-add Android would read strictly less data than Apple — plausibly missing the >= 24-bucket floor and rendering no estimate where Swift renders one. hrBucketsUnion is documented as the twin of exactly that overload. Single-WHOOP install resolves to one id either way, so nothing changes for most users; the divergence only appears on the multi-namespace case the read spine exists for. A parity bug in the parity fix.

@mainactor

…rm gap (#855) Body Clock has shipped on iOS/macOS all along: AppModel.computeCircadianPhase pools HR by local hour-of-day as the activity proxy, fits the cosinor, publishes circadianPhase, and SkinTempCardsView renders it. Android has a byte-for-byte twin of the engine, its own full test suite and a finished BodyClockCard — and never fed any of it. V5HealthSignals hardcoded bodyClock = null, so HealthScreen ?.let skipped the card on every device. The comment guarding it was wrong. It said we lack "per-hour rest-activity bins we do not bank here". Swift does not bank them either — it derives them from HR buckets Android also has. The pipe was never built on this side, not missing. Mirrors the Swift maths exactly: 3600 s buckets over 14 days, the >= 24-bucket floor, pooling by LOCAL hour, mean bpm per populated hour, distinct local days as daysObserved, and the >= 6-bin floor before a fit is attempted. The binning lives in AppViewModel rather than CircadianEngine precisely because Swift keeps it in AppModel — the engine stays an untouched byte-for-byte mirror on both platforms. Reads hrBucketsUnion, not hrBuckets: the Swift twin UNIONs the active strap with the canonical my-whoop (#814 read spine). Caught in re-review — the single-id read would have given Android strictly less data than Apple after a strap re-add, enough to miss the 24-bucket floor and render no estimate where Swift renders one. A parity bug inside the parity fix. The pure half is a top-level circadianBinsFrom, unit-testable with no Context, no ViewModel and no database. Six tests pin the floors, local-hour pooling, mean-not-sum, the negative-offset wrap and sparse coverage; every expectation was cross-checked against an independent implementation before commit. Both new parameters default, so every existing evaluate() caller is byte-identical. The synchronous settings-toggle re-evaluate reuses the collector pass cached bins rather than blanking the estimate. That cache needs no synchronisation and the reasoning is recorded at the declaration: the write resumes on viewModelScope (Dispatchers.Main.immediate), the read is a main-thread settings callback, and the Swift twin gets the same guarantee from @mainactor. Supersedes #853, which proposed DELETING all of this on the mistaken belief there was no Swift twin. Closed. KNOWN DIVERGENCE, deliberately not closed here: Swift derives habitualWakeHour from the most recent banked wake and falls back to 07:00; Kotlin has no such helper, so this uses the existing 7.0 default, shifting offsetVsScheduleMinutes for anyone who does not wake at 07:00. Does NOT settle whether this should ship at all. Without an observedTempMinHour the estimate derives the CBT minimum as activityAcrophase - 12.0h, a population constant, and the confidence bands key on data sufficiency rather than on whether that constant holds for this person. That question is about the ALREADY-SHIPPED Apple card. This lands first so the platforms are consistent and the decision can be made once, for both. Not compiled — android.yml is disabled.

…the rename is for I renamed workoutHrDeviceId -> workoutHrDeviceIds and updated the KOTLIN resolver test while missing its SWIFT twin, which still called the old name — a compile break in StrandTests. Nothing caught it: StrandTests runs only under xcodebuild on macOS, and app-build.yml is disabled. Anyone running tests locally on a Mac would have hit it, which #854 has just made possible again. The four existing cases keep their expectations, now as single-element lists. Added the two cases none of them covered, which are the whole point of the change: an IMPORTED row on a multi-namespace install resolves to the full #814 union (the case the single-id resolver silently under-read), while a DETECTED row on that same install still resolves to exactly one id — the union must not leak into the branch where it would pull in a strap that never recorded the bout.

@query

… HRR (#857) Four surfaces on one workout card, no two agreeing. Swift avg workoutHrDeviceId (single) under-reads imported after a strap re-add Swift chart day union WRONG STRAP for a bout detected on a 2nd WHOOP Swift zones day union same Swift HRR day union same Kotlin chart single active id both failure modes Kotlin zones single active id both Kotlin HRR day union wrong strap for detected So on iOS the Avg HR and the HR curve on the same card could describe DIFFERENT straps. Not a Kotlin-vs-Swift divergence — a live self-inconsistency. Neither existing rule is right on its own. The correct read is source-class-dependent, which workoutHrDeviceId already encoded; it just returned one id where one case needs two: detected -> [recording strap] a union mixes in a strap that never recorded it (#510) manual / imported -> active u canonical no strap of its own, and the worn strap may bank under either after a re-add (#814) workoutHrDeviceIds returns that list on both platforms and all FOUR surfaces consume it: chart, zone minutes, Avg HR / Effort, and heart-rate recovery. HRR reads past the bout, which is the one reason it might have wanted different treatment; it does not, since the strap that recorded the session is still on the wrist minutes later. The aggregate needed real work. A naive `deviceId IN (...)` double-counts a second banked under both ids — inflating n and skewing avg, silently, because both numbers stay plausible. GROUP BY ts with MIN(pri) keeps one row per second and takes the primary is, matching the dedup the chart already did. Fixed 2-arity rather than a bound collection: importedReadIds is 1 or 2 by construction, and Room @query is static SQL that cannot express per-id priority through an IN list. Passing the same id twice is byte-identical to the old single-id read, so a single-WHOOP install is unchanged and needs no special case. That is the control test. The aggregate lives in WhoopStore, where swift-packages.yml actually runs its tests — the placement that made #841 Swift half verifiable and its Kotlin half not. Verified against SQLite before commit, and the Kotlin @query was extracted from source and executed to confirm it returns the same numbers as the Swift twin: A(0..9) + B(5..19), A primary -> n=20 avg=150.0 max=200 (naive IN would say 25) B primary -> n=20 avg=175.0 PPG anti-join per id -> n=15 max=130 same id twice -> identical to the pre-change read Tests: four added in WhoopStoreTests (overlap, precedence, per-id PPG anti-join, single-id control); five existing moved to the two-id signature with assertions untouched; both resolver tests moved to the list API, plus two new Swift cases for the behaviour the rename exists for — an imported row getting the full union on a multi-namespace install, and a detected row still resolving to one id on that same install. Day-level hrSamplesUnion / hrBucketsUnion are untouched and still used by Today, the full-day chart and the auto-workout nudge, where the union is the correct read. Swift WorkoutRow carries no deviceId, so its resolver takes only source where Kotlin also takes rowDeviceId — pre-existing, documented in the original #510 note. NOT field-verified: the bug only manifests on a multi-strap install, which nobody here has. The two-id path is covered by CI tests against real SQLite and a single-strap control proving the common case is byte-identical, but no one has confirmed it on two straps. The Kotlin half is not compiled or run by CI.

@82

…lassify a strap that never emits it (#860) tools/linux-capture/validate_spo2_candidate.py (#814, extended by #819) decides whether spo2_candidate_82 graduates to a real spo2Pct. Three properties of @82 made its numbers unreliable. All three established on an 18,650-record v18 corpus: 18,602 from @digitalerdude's public PacketLogger capture of an official-app overnight sync (posted on #103), plus 48 from a NOOP sync on a subscription-free strap. 1. @82 IS DUTY-CYCLED, AND THE HARNESS DID NOT KNOW. Nonzero in 450/18,650 records (2.41 %), 24 distinct values in 1..160, falling in 15 contiguous runs of exactly 30 records each, every run starting at unix % 1200 == 337 with zero phase variance. A 30-second measurement once every 20 minutes. A capture not aligned to that phase reads all zeros and is indistinguishable from a strap with the feature off — sub-sampling the corpus at one record per minute off-phase reproduces it exactly: 310 records, zero nonzero @82, 0/16 windows. A plausible contributor to the contradictory cross-device evidence on #103, where one device correlated +0.99 and another moved opposite; the second may not have disagreed so much as missed the window. detect_duty_cycle() now measures period, phase, window length, count and jitter per capture and classifies it absent / continuous / duty_cycled / irregular. Only duty_cycled changes aggregation, so a capture it cannot characterise is never silently reweighted. The phase is deliberately NOT a constant: the robust fact is that a stable periodic window exists, and 337 is one strap's firmware. Verified independently on four schedules the tests do not use, including phase > period and a 2-minute period — all recovered exactly — and it falls through to irregular on two windows, scattered firings and phase jitter, with the jitter tolerance scaling to capture cadence rather than fixed. Robust to 3 of 10 windows missing without the period collapsing to a multiple. Nightly aggregate is now the mean of per-window means, not of every second: a 30-second burst is one measurement, and per-second averaging hands the night to whichever window was sampled most densely (95.92 vs 93.70 on the real corpus). Coverage is reported per night and warned on below --min-window-coverage. 2. IN-BAND-ONLY SCREENING IS NOT SPECIFIC. Screening on "value in 70-100" is a range, not a signature: six offsets are in-band on a majority of records on a real strap, including @17 — byte 2 of the u32 unix timestamp at @15 — and the already-decoded cardiac_flags, step_cadence and two aux thermal channels. A nightly SpO2 cannot have 2 distinct values. The scan now requires >=5 distinct in-band values and stdev >=0.5 before ranking a correlation, reports rejected offsets rather than dropping them silently, and holds @82 to the same floor. The regression test plants a 2-valued byte that correlates BETTER than @82; without the floor it wins the scan and the tool concludes "not specific to @82" from a byte that cannot be a measurement. 3. A STRAP THAT NEVER EMITS @82 IS CLASSIFIED, NOT FAILED. It is now feature_absent, neither PASS nor FAIL, gated behind >=100 records and >=3 nominal periods of scored sleep so a short capture that merely missed the window stays a plain FAIL. KNOWN GAP, follow-up immediately: that evidence bar measures duration, not phase, so it does not catch a capture whose cadence aliases with the duty period. The same working strap at 1200/337/30 over six nights of eight hours scored sleep classifies duty_cycled/fail at 1 Hz and absent/feature_absent at one sample per five minutes — a 300 s cadence occupies only 4 residues mod 1200, so it either always hits the window or never does. This is finding 1 applied to finding 3. It matters because it is asymmetric: feature_absent removes the device from `eligible`, so all_pass is computed without it and promotion gets easier. record_interval_s is already computed when the mode is absent, so the gate is a few lines. PRIVACY. --postable still carries no raw SpO2 values — verified with distinctive per-night values, none appear. Added columns are device timing and a classification; the phase is the useful thing to share, being what another owner needs to align a capture. TESTS. tools/linux-capture 178 -> 194, validator file 16 -> 32, all 16 pre-existing unchanged and still passing; confirmed on both branches with unittest discover. SCOPE. Python capture tooling plus two doc sections. No app code, no decoder, no schema, no Swift/Kotlin — spo2_candidate_82 remains instrumentation-only and this does not move it. Serves the #103 promote gate, where until now a device could fail for having captured the wrong 30 seconds out of every 20 minutes. Related: #845. Corpus credit to @digitalerdude (#103).

dnesdan and others added 3 commits July 25, 2026 19:32

ryanbr merged commit a446d0b into main Jul 26, 2026
1 check passed

This was referenced Jul 26, 2026

tools: multi-device SpO2 @82 candidate validation harness (#103) #808

Closed

test(tools): collect test_whoop_spot_hrv too — 8 more tests that never ran #815

Merged

This was referenced Jul 26, 2026

docs(whoop5): SpO2 sleep path, SLEEPFLAG, skin-temp wire scale #807

Open

docs(whoop5): record the SpO2 protocol facts and the 5.0-vs-MG identity signals #824

Merged

ryanbr mentioned this pull request Jul 26, 2026

[health-connect] Import per-sample HR, not just a daily average — unblocks Effort/zones/HR chart for strapless users #843

Open

ryanbr mentioned this pull request Jul 27, 2026

[parity] Kotlin workoutHrBuckets reads one device id where Swift reads the #814 union #856

Closed

ryanbr mentioned this pull request Jul 27, 2026

[health][parity] Body Clock ships on iOS/macOS but is inert on Android — the Kotlin twin is never fed #852

Closed

vishk23 mentioned this pull request Jul 27, 2026

tools(spo2): account for @82's duty cycle, screen for variance, and classify a strap that never emits it #860

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools: SpO2 @82 multi-device validation harness (#103) — with the fixes it needs to run#814

tools: SpO2 @82 multi-device validation harness (#103) — with the fixes it needs to run#814
ryanbr merged 3 commits into
mainfrom
tools/spo2-candidate-harness

ryanbr commented Jul 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ryanbr commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. The suite couldn't run on Linux at all

2. "21 tests, OK" never included the new test

3. My own byte-82 docs from #812 were wrong

Verification

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ryanbr commented Jul 26, 2026 •

edited

Loading