Skip to content

Keep the v26 optical window's absolute base, not just its deltas - #2020

Merged
ryanbr merged 3 commits into
mainfrom
fix/2019-v26-base-code
Sep 9, 2026
Merged

ryanbr merged 3 commits into
mainfrom
fix/2019-v26-base-code

Conversation

@ryanbr

@ryanbr ryanbr commented Sep 9, 2026

Copy link
Copy Markdown
Owner

The defect reported in #2019. Independent of the #1992 stack; based on main.

What was wrong

The v26 record encodes a 25-sample optical window as ONE absolute ADC code plus 24 deltas. Both
platforms read only the deltas, stored them as if they were the samples, and discarded the base.

The evidence is in a frame already in our fixtures:

frame-abs 23..27, u32 LE 378,307, a valid 20-bit optical code
our 24 stored "samples" -1432, -1332, -1139 ... -164, every one negative
base + cumulative deltas 25 samples, all in the 20-bit domain, 378,307 down to 362,532
excursion 15,775 = 4.2% of base, an ordinary PPG perfusion index

An absolute optical code cannot be negative. Ours all are, and a plausible one sits in the four bytes
immediately before them. 24 deltas plus a base is 25 samples, matching the 25-sample block v20 carries.

Impact

No shipped metric is wrong. Nothing in analytics reads ppgWaveformSample; the strap's own
PPG-derived heart rate comes from ppgHrSample. The damage is to the corpus banked for the work this
stream exists for: a derivative stored as a signal, and the DC level discarded, which is exactly what
an SpO2 ratio-of-ratios needs.

Not repairable retroactively. A delta series cannot be inverted without its starting point, so
every v26 record ingested to date has no recoverable absolute level. Only future records can be right,
which is the argument for landing it rather than waiting.

Change

The base is read and stored BESIDE the deltas rather than folded into them: the sample blob is
little-endian i16 and a real code does not fit in one, and base-plus-deltas is the shape the strap
actually sends. ppgWaveformAbsolute reconstructs, with a Kotlin twin, and returns null for a row
whose base is null rather than inventing a window from a fabricated zero. That null is a true
statement about a legacy row, not a missing value.

Schema, and a correction to my own notes

This needed FIVE things moved in step, not the three I had written down: Room's @Database(version),
SCHEMA_VERSION, the migration chain, and BOTH copies of schema_oracle.json, which pins the room
version AND the per-table column set and is byte-compared across the platforms.

That last guard did its job: it refused the change until the Swift store gained the column too, which
is correct, since the two stores are meant to match. Worth knowing for anyone else touching schema.

Caveat carried in the code

The deltas are SATURATED, clamped at the i16 bounds, so a window holding a clamped delta reconstructs
only approximately. The captured fixture's largest magnitude is 1,833, nowhere near 32,768, so it does
not bite there, but a consumer should treat a delta at the rail as suspect.

Provenance

Found by comparing against OpenStrap's Dart decoder (MIT), which documents the encoding. No code
taken. The offset it names was verified against our own captured frame, and both platforms now
independently decode 378,307 from it.

Verification

  • Android suites: 5,744 tests, 0 failures. WhoopProtocol: 685 tests, 0 failures (local).
  • Tools/doc_comment_lint.py clean. Swift app target and the WhoopStore suites rest on CI.
  • Two tests repinned rather than broken, on both platforms: the captured-frame assertion now carries
    the base, which is itself the end-to-end proof that it is read. A reconstruction case on each side
    asserts the 25 samples land in the 20-bit domain and that a null base reconstructs to nothing.
  • Not run: a real strap. Nothing reads this stream yet, so there is no user-visible behaviour to
    check; what wants confirming on device is that a 5/MG offload writes a non-null baseCode.

…eltas

The WHOOP 5/MG v26 record encodes a 25-sample optical window as ONE absolute ADC code followed by 24
deltas. Both platforms read only the deltas, stored them as if they were the samples, and discarded
the base.

The evidence is in a frame already in the test fixtures. At frame-abs 23 sits 378,307, a valid 20-bit
optical code, in the four bytes immediately before the 24 values we do read. Every one of those 24 is
NEGATIVE, which no absolute optical reading can be. Reconstructed as base plus running sum they give
25 samples, all inside the 20-bit domain, with an excursion of 4.2% of the base: an ordinary PPG
perfusion index. 24 deltas plus a base is also 25 samples, matching the 25-sample optical block v20
carries.

No shipped metric is wrong. Nothing in analytics reads `ppgWaveformSample`, and the strap's own
PPG-derived heart rate comes from `ppgHrSample`. What was wrong is the corpus being banked for the
work this stream exists for: a derivative stored as a signal, and the DC level thrown away, which is
precisely what an SpO2 ratio-of-ratios needs.

The base is now read and stored beside the deltas rather than folded into them, because the sample
blob is little-endian i16 and a real code does not fit in one, and because base-plus-deltas is the
shape the strap actually sends. `ppgWaveformAbsolute` reconstructs, with a Kotlin twin, and returns
null for a row whose base is null rather than inventing a window from a fabricated zero.

That null is the honest part. Every v26 record ingested before this is unrecoverable: a delta series
cannot be inverted without its starting point, and no backfill can supply one. Only future records
can be right, which is the whole argument for landing it rather than waiting.

Schema on both stores, which needed five things moved in step, not the three I had written down:
Room's `@Database(version)`, `SCHEMA_VERSION`, the migration chain, and BOTH copies of
`schema_oracle.json`, which pins the room version AND the per-table column set and is byte-compared
across the platforms. That last guard is what forced the Swift store to gain the column too, which is
correct: the two stores are meant to match.

Caveat carried in the code: the deltas are SATURATED, clamped at the i16 bounds, so a window holding
a clamped delta reconstructs only approximately. The captured fixture's largest magnitude is 1,833,
nowhere near 32,768.

Found by comparing against OpenStrap's Dart decoder (MIT), which documents the encoding. No code
taken; the offset it names was verified against our own captured frame, and both platforms now
independently decode 378,307 from it.

Android 5,744 green; WhoopProtocol 685 green.
…ucted at all

The previous commit documented that a SATURATED delta makes a window reconstruct only approximately,
and then shipped no way to see whether one ever occurred. It also made a null base mean two different
things with no way to tell them apart: a legacy row, or a record that arrived truncated, or a
firmware that does not carry the base at frame-abs 23. All three would have banked silently.

One line per session, beside the R-R census, and null when the session carried no v26 windows so a
4.0 or a strap that banked none stays quiet:

    Backfill: v26 optical census: 412 window(s), 412 with a base, 0 saturated, base 361204..379881

Each number earns its place. The window count is the volume. `with a base` below it is the signal
that matters most, because those windows can NEVER be reconstructed and nothing else would say so.
The saturated count is the caveat made observable rather than left in a doc comment. The base range
is the session's DC level, which is the quantity the whole stream is banked for and the one that used
to be discarded outright, so a log now carries enough to tell a plausible optical run from a rail-
pinned one without pulling the store.

Counted on what the DECODER produced rather than on what the store kept: whether a window carries a
base is a fact about the wire, not about the insert.

Pure and mirrored, pinned on both platforms against the same literals including the empty case, the
missing-base case, the saturation caveat and an absent range reading as absent rather than as a
fabricated zero.

Android 5,750 green; WhoopProtocol 691 green.
…ckstep

CI caught what a machine without SQLite headers cannot: the WhoopStore suite does not build here, so
three things went out unverified and one of them was a real defect.

The defect: Swift's `ppgWaveformSamples` is a NAMED projection, `SELECT ts, samples, burstIndex`. A
new column is invisible to it until it is listed, so the base was banked on every write and every read
handed back nil. Nil is also exactly what a legacy row reads as, so the column would have looked
inert rather than broken, on the platform where nothing yet reads the stream to notice. Kotlin uses
`SELECT *` into the entity and was already correct, which is why the Android suites were green
throughout. The round-trip test now carries a base and asserts it survives, so the projection cannot
silently drop a column again.

The other two were lockstep entries I had not found: `schema_oracle.json` pins the GRDB MIGRATION
IDENTIFIER list as well as the room version and the per-table columns, and `PpgWaveformSampleTests`
pins the table's column set on the Swift side. Both repinned.

That makes SEVEN things a schema change moves in this repo, not the three I had written down and not
the five I corrected it to an hour ago: the Room version, SCHEMA_VERSION, the Room migration chain,
the GRDB migration, and three separate pins inside a schema_oracle.json that exists in two
byte-compared copies. The guard is doing its job each time; my note was the thing that was wrong.

Android 5,750 green; WhoopProtocol 691 green. The WhoopStore suite rests on CI, which is the point.
@ryanbr
ryanbr merged commit 7bf1449 into main Sep 9, 2026
16 checks passed
@ryanbr
ryanbr deleted the fix/2019-v26-base-code branch September 9, 2026 07:49
DX23876 added a commit to DX23876/noop that referenced this pull request Sep 16, 2026
Thirty real conflicts. The Android tree is dropped again per FORK_GUIDE; every
other hunk was resolved by asking which mechanism this fork actually runs, not
by preferring a side.

Taken from upstream:
  • Protocol: ryanbr#2017/ryanbr#2018/ryanbr#2020 — the 5/MG historical optical and raw-IMU
    layouts, and the v26 window's ABSOLUTE base code beside its deltas. Its
    migration lands as `v57-ppg-waveform-base-code` (upstream's v44), since this
    fork's sequence is nine ahead of theirs.
  • BLE diagnostics ryanbr#1998-ryanbr#2003: a bond end that says WHY, named BLE integers, a
    session-budgeted hex dump, and "we hold the link" only when the OS says so.
  • Battery: ryanbr#1951 stop asking a 5/MG for a reading it refuses; ryanbr#1959 poll
    0x2A19 for an unbonded one.
  • Scoring: ryanbr#1978/ryanbr#1982/ryanbr#1990 the RHR/HRV baseline gates, ryanbr#1977 the strain
    denominator's domain, RMSSD withheld on an over-counted night, ryanbr#1943 the
    resting-HR bin gate, the closed final HR/HRV window, HR-recovery requiring
    one CONTINUOUS effort, ryanbr#1984 nights bridged under minSleepMin, ryanbr#2015 motion
    re-folded only when that day's gravity moved, ryanbr#2016 a provisional Rest.
  • Charts ryanbr#2004/ryanbr#2005/ryanbr#2007/ryanbr#2009/ryanbr#2011 and ryanbr#1995's hero-ring detail route,
    ryanbr#1164's "Pending sync" caption under Rest.
  • Widgets: the heart-rate widget with its trace (ryanbr#1957/ryanbr#1964) and the widget
    cost counters.
  • Health: ryanbr#1503 — the Apple Health external UUID drops the strap id, so a
    re-pair no longer strands every prior record, plus the one-off sweep that
    reclaims the stranded ones; ryanbr#1205 GPS routes read back from Apple Health.
  • Storage: rolling retention for ppgWaveformSample, the skin-temp absolute
    backfill outside the 21-night window, per-table byte estimates.
  • Units split (body vs exercise distance), per-day wake overrides, the
    wind-down nudge pinned to its own day, and the German Sie→du conversion
    (29 entries, merged per LOCALE so the fork's own de edits stayed).

Kept on the fork's side, with the reason:
  • ryanbr#1005's in-memory `dayScanCache` stays out, again: the persisted
    `DayScanFingerprint` survives relaunch and its key folds the per-day weight
    and max HR upstream's does not.
  • The steps-motion cache stays PERSISTED for the same reason. Upstream's
    failed-read-vs-empty-day distinction IS adopted into it — on a persisted
    cache a stale zero from a read error would outlive the process that made it.
  • Every stream read in the day loop keeps `limit: Int.max`. All five are
    uncapped together; taking StreamReadCap for three of them would leave the
    loop half-capped, which is worse than either choice.
  • The coach engine. Upstream's K1-K15 layer is not adopted: its single 40-row
    `coachMessage` table, its tool-less pre-baked context and its own brief are
    behind what this fork already ships. Its ADDITIVE half is kept — the
    on-device voice input, the Siri "Ask Coach" intent, the brief scheduler and
    brief widget, and streaming for the OpenAI-compatible providers this fork's
    Anthropic-only StreamingToolClient does not cover. They are wired up in the
    next commit; `CoachBriefWidget` is deliberately not registered until then.
  • Liquid's `DomainTheme` labels, value-sampled Charge tint, `LiquidScoreGauge`
    and `TodayCardSurface`; classic Today's ring order and `LiveEffort.today`.
  • RootTabView's four-tab shell and `MoreCatalog`: Coach hangs on content here,
    not on a tab.

Two duplicate mechanisms collapsed rather than carried:
  • `ChargeBreakdownWiring` (upstream) and `ChargeBreakdownFormat` (fork) are the
    same extraction done twice. The fork's survives, Coupled now reads it, and
    upstream's TEST is kept — it is coverage this path never had.
  • Upstream's `CoachMessageStore` is dropped; `CoachConversationStore` already
    autosaves the whole multi-conversation history.

One defect found while resolving, fixed here: `LiveEffort.today` read HR with no
explicit limit, so `hrSamples`' 8000-row default truncated it — at ~18k rows a
day that covered about ten hours and the live Effort silently stopped climbing.
It failed safe (`effectiveEffort` takes the max), which is why it went unseen.

`Live5RestFrozenTests` is removed by upstream, which replaces it with
`Live5NoGravityRestChainTests`. `local_day_windows_oracle.json` moves out of
upstream's android path into `Tests/StrandAnalyticsTests/Oracles/` — NOT
`Fixtures/`, which this repo's .gitignore excludes everywhere.

Verified: StrandAnalytics 2233, WhoopStore 627, WhoopProtocol 700, StrandImport
293, StrandDesign 120, NoopLocalAccess 14, SemanticMemory 19, OuraProtocol 209
and PolarProtocol 25 all green; macOS and iOS both build; doc_comment_lint clean;
`i18n_audit --ci` byte-identical to its pre-merge output (it is red on main for
reasons that predate this, and the seven new widget literals were given all nine
locales so the gate did not get worse).

Six StrandTests were ALREADY failing on main before this merge and still are —
`CoachMyLogsAndZonesTests` (the tool census pin was not bumped when the Hevy
tools landed) and five in `RawPhysiologyUnionTests`. Every input to them —
CoachTools.swift, ToolConsent.swift, Repository.swift and both test files — is
byte-identical to main, so this merge neither caused nor fixed them. They went
unnoticed because app-build.yml last ran on 2026-08-25.

Analysis migration required: YES. The scoring changes above make stored derived
values stale; the recipe bump and its resumable migration are the next commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant