WHOOP 5/MG: passive optical-block phase experiment (harness for #858, on top of #546) - #862
Conversation
(cherry picked from commit 9178cfd)
…ed locales) The four new user-facing strings in SettingsView (the section header and its explanation, the confirmation-dialog title and its message) were English-only, so `Tools/i18n_audit.py --ci` hard-failed on all four as literals absent from Strand/Resources/Localizable.xcstrings. Translated into the four focus locales (de/es/fr/pt-PT), which the audit gates at zero tolerance, AND into it/ru/zh-Hans/zh-Hant: those four carry a ratcheting allowance that blocks GROWTH, so adding focus-only entries would have pushed each of them 4 over (172/168, 149/145, 122/118, 172/168). Appended textually rather than by re-serializing the catalog, so the other 3,255 entries keep their bytes and their Xcode insertion order.
…te branch The status line named a fork branch (`whoop-decode-audit`) that nobody outside one checkout can resolve, and implied a result where there is only a harness. Names the three issues that carry the actual context instead, and states plainly that no labelled run has been analysed yet.
ryanbr
left a comment
There was a problem hiding this comment.
Careful work, and unusually well-disciplined about what it refuses to claim. Verified the load-bearing parts rather than taking them:
"It sends nothing to the strap" holds structurally. markWhoop5OpticalPhase → PuffinDeepBufferLog.appendOpticalPhase → try append(line + "\n"). No peripheral, no characteristic, no command path anywhere on that route. As you say, the BLE safety contract is not respected here so much as unreachable, which is the right shape for an experiment harness.
Timestamp attribution is right, including the part that is easy to get wrong. experiment_end is excluded from phases but retained in markers, so it bounds the final phase without becoming one — and endUnixTs indexes the full marker list while phaseIndex searches only real phases. Those two could easily have drifted apart; they have not. The frame.baseTs == strapTs gate as a consistency check on the envelope rather than a substitute for frame content is also the correct instinct.
The CLI really does default to 10 s settling (var settlingSeconds = 10), matching the description, while the library defaults to 0 — the right split, since a library should not silently discard data.
On i18n: thank you for doing it properly. #850 landed a few hours ago and made it/ru/zh-Hans/zh-Hant gate against a ratcheting allowance. You translated all four new strings into all eight locales and left i18n_extra_locale_baseline.txt untouched. Raising the allowance would have passed CI just as green; you did the thing the gate exists to encourage, on its first day. That is worth saying out loud.
Parity reasoning accepted. The deep-buffer lane has no Android counterpart, so there is nothing on that side for a marker to mark. Stating that the JSONL and report format are the contract — not the Swift — is the right way to leave it portable.
10/10 green. The only thing I would flag is not a defect: this is a harness whose value is entirely in the capture nobody has taken yet. Worth being explicit in #858 about who is expected to run it and on which strap, so it does not sit as unexercised machinery.
No changes requested.
…ame exact (#863) Follow-up to #862. Three fixes to the capture doc, all of which land on the person trying to run the harness — which is the only way #862 produces any value at all. The procedure names buttons ("Off wrist - sensor covered") while the JSONL and the report carry raw values (off_wrist_dark). Those are not always the same words, so grepping your own export for what you tapped can quietly return nothing. Added the full eight-row mapping. Both columns were verified against code rather than transcribed from the prose being corrected: raw values against PuffinOpticalExperimentPhase, button names against the String(localized:) cases. Wrote down that experiment_end is a boundary rather than a phase — it closes the preceding one so later offload records are not attributed past the run, and never appears as a phase in the report. Correct in the analyzer, documented nowhere a capturer would look. Made the export filename exact. FileExport.timestampedName always appends -yyMMdd-HHmm, so noop-whoop5-optical-experiment.jsonl never exists; it is noop-whoop5-optical-experiment-<timestamp>.jsonl. Step 9 now also names the on-device file (puffin-deepbuffers.jsonl) and says why they differ: markers are appended to the existing capture log, and the export renames on the way out. The analyzer reads contents, so either file works. An earlier revision of this PR pointed the analyze command at puffin-deepbuffers.jsonl on the grounds that nothing wrote the other name. That was wrong and was reverted before merge — the export does write it. I had checked only the on-device writer and treated the disagreement as proof the doc was wrong, when both names were real and described different things. Docs only.
#546 gave the 2,140-byte layout-v20 record a neutral structure: five repeated 422-byte blocks, each with a 21-byte header, two 200-byte readout slots and a reserved byte, with sample counts that are always
[25, 0, 0, 25, 25]in the existing corpus. Structure, deliberately without names — no block is called red, IR, green or ambient, because nothing in the bytes says so.This adds the harness that could earn those names: a passive experiment that marks physical phases on the phone and compares what each block does across them, offline. It is the thing #858 asks a capture for and the thing #845 says the v18/v20 fields are missing.
What it does
Phase marking (app). Settings → Advanced → Experimental · WHOOP 5/MG gains "Mark phase…" and "Export experiment…", shown only when frame recording is already on. A marker appends one line to the existing
puffin-deepbuffers.jsonl:{"kind":"optical_phase","label":"off_wrist_dark","unix_ts":…,"ts_ms":…}Eight fixed phases: on-wrist still, gentle pressure, off-wrist covered, off-wrist room light, on-wrist again, slow paced breathing, normal breathing, end.
Offline analysis (
whoop-optical-experimentCLI +OpticalExperimentAnalysis). Reads that JSONL back, attributes each v20 frame to a phase, and emits JSON: per-phase block activation, per-header-byte behaviour, per-channel ADC statistics, and adjacent-phase deltas.Docs.
docs/WHOOP5_OPTICAL_EXPERIMENT.md— the capture procedure, the timestamp model, and what the output does and does not license you to conclude.The two design decisions that matter
It sends nothing to the strap.
markWhoop5OpticalPhasereachesPuffinDeepBufferLog.appendOpticalPhase, which is a file append. There is no peripheral, no characteristic, no command path — not a read, not a write. The BLE safety contract is not merely respected here, it is structurally unreachable. This also means the harness needs no new permission, changes no connection behaviour, and cannot affect scoring.Attribution uses the strap's timestamp, not arrival time. Deep buffers arrive during the connect-time offload burst, often minutes after the seconds they describe. Bucketing by log-arrival time would smear every phase into its neighbours. The analyzer compares the marker's phone Unix time against the Unix timestamp inside each v20 frame. The CLI also drops the first 10 s after each marker by default (
--settling-seconds, 0 keeps everything), so putting the strap on, taking it off, covering it and pressing it stay out of the steady-state summaries.What it deliberately does not do
It does not assign a wavelength to any block, and it does not compute SpO₂ or blood pressure. Per the repo's rule on deriving a physiological signal from raw sensor data, an experiment that produced one matching number would prove nothing; the questions here are narrower and answerable:
Those are the questions that have to be settled before anyone can honestly claim a wavelength, let alone a saturation.
Verification
The app-target half (
BLEManager,SettingsView,PuffinDeepBufferLog) is compiled and tested locally, sinceapp-build.ymlis disabled by default.Not validated on hardware in this PR, and it cannot be: the harness exists precisely because nobody has a labelled capture yet. The BLE surface it adds is a file append, so there is no connection behaviour to test on a strap.
Cross-platform parity
Swift only, on purpose. The parity contract binds analytics and stored values; this stores no analytics and changes no scored number. The capture side (
PuffinDeepBufferLog,PuffinFrameRecorder, the whole R22 deep-buffer lane from #423) has no Android counterpart at all, so there is nothing on Android for a marker to mark or an analyzer to read. If the deep-buffer lane is ever ported, the phase marker is ~30 lines of file append and this doc + CLI apply unchanged — the JSONL and the report format are the contract, not the Swift.Two small commits on top of the harness
Tools/i18n_audit.py --cihard-fails. Translated into de/es/fr/pt-PT (zero-tolerance focus locales) and it/ru/zh-Hans/zh-Hant (each of which would otherwise have gone 4 over its ratcheting allowance). Appended textually so the other 3,255 catalog entries keep their bytes and their order.Related: #423, #858, #845. Builds directly on #546.