Stop reporting a mapped historical layout as undecodable by ryanbr · Pull Request #2017 · ryanbr/noop

ryanbr · 2026-09-09T06:03:26Z

Scoped to ONE of the two defects behind #1992. The other came out on re-review; see the bottom.

The warning fires for a layout NOOP decodes

It asked whether a record decoded any of heart_rate, gravity_x or ppg_waveform. That list has to
be extended by hand for every layout NOOP learns, and twice it silently was not: #156 was v25 and v26
warning after they had decoded for releases, and v20 has warned the same way ever since it was
mapped. mappedWhoop5HistoricalVersions is [18, 20, 21, 26], and the 5/MG optical record decodes
to sensor_block_count and per-block headers, so it matches none of the three names. An iOS or macOS
user with a v20 strap is told NOOP cannot decode a layout it decodes.

Suppressing the line for v20 would be the wrong correction, though, because only half of what it
said is wrong. "This layout does not decode" is false for v20 on Swift. "These records carry no heart
rate or motion, so a night made only of them cannot be staged" is TRUE, on both platforms: the optical
record decodes into raw arrays and the raw IMU record into accel/gyro channels, and no engine reads
either. Silencing the whole line would remove the only thing in the strap log explaining why a user's
nights are empty.

So the question splits, and so does the message. Whether the layout decodes is asked of the dispatch
table the decoder itself switches on, which cannot drift from what NOOP decodes. Whether the record
carries anything scoreable stays the field test, which is what that test was always really measuring.
A mapped layout carrying nothing scoreable gets its own line saying exactly that, and still asks for a
report, because the missing piece is the meaning of those channels.

WHOOP 4.0 keeps the field test unchanged, since it has no equivalent set and every layout it maps does
emit one of the three.

The decision is a pure function in WhoopProtocol with a Kotlin twin, and its test is driven off the
dispatch set rather than a literal list, so a layout added there without a signature field cannot bring
the warning back a third time.

The platform divergence is deliberate and now pinned

Android's MAPPED_WHOOP5_HISTORICAL_VERSIONS is setOf(18, 26) and stays so: it has no v20/v21
historical branch, so v20 genuinely does not decode there. Android therefore answers UNMAPPED for
v20 where Swift answers DECODES_WITHOUT_NAMED_SIGNAL, and both are correct. The two platforms
answering differently for the same version is the divergence itself. Both sides assert their own
answer, so porting Swift's decoder to Android flips a test rather than surprising someone.

Worth saying plainly, since the issue did not until I corrected it: the v20 field map already exists
on Swift. Mapping it on Android is a port, not reverse engineering. What is genuinely missing on both
is the mapping from v20's raw optical channels to a physiological value.

What came out, and why

The first version of this PR also changed the offload frontier: the auto-continue gate measures its
backlog as strapNewest - frontier, the frontier was the newest persisted HR row, and a record that
becomes no rows can never advance it, so the gap never closes and the offload re-kicks to its cap.
That diagnosis stands, and #1144's persistedSensorRows guard genuinely cannot catch it, because a
strap emitting a MIX of layouts banks rows on every pass.

The fix used the unix a HISTORY_END carries as the acked section's end time. The comment at the call
site says otherwise: "A HISTORY_END carries the strap's own clock" (#773). If that is the strap's live
RTC rather than the section's time, the safety property the change rested on inverts. "Taking the later
of the two can only close a gap, never open one" is benign only when the second value describes ground
actually covered; a live clock is approximately now on every ack, so it would close EVERY gap and stop
a strap with real backlog after one session.

I could not settle which it is from the code. clockRef is built from the GET_DATA_RANGE newest, so
the clock-correlation path does not answer it, and #773's own wording reads both ways. That is a bet on
the offload loop, so it is out until the field's meaning is established from a capture. Tracked on
#1992 with the diagnosis intact.

Verification

Android suites: 5,748 tests, 0 failures. WhoopProtocol: 689 tests, 0 failures (local).
Tools/doc_comment_lint.py clean. Swift app target rests on CI.
No BLE behaviour changes in what remains: this alters one diagnostic line's condition and nothing else.

…mapped layout undecodable Two defects behind #1992, neither of which needs the v20 record layout reverse engineered. The frontier. The auto-continue gate measures its backlog as `strapNewest - frontier`, and the frontier was the newest HR row we had persisted. A record in a layout with no field map on this platform is archived and acked but becomes no rows, so on a strap whose NEWEST records are one of those the row frontier can never reach the strap's newest: the gap stays open, the gate keeps answering "backlog remains", and the offload re-kicks to its cap on every connection. A reporter measured roughly an hour of continuous radio and decode per connection on an otherwise healthy 5/MG. #1144's `persistedSensorRows` guard does not catch it, and cannot. A strap emitting a MIX of layouts banks rows on every pass, so that guard passes every time; it asks whether anything landed, not whether the frontier reached the strap's newest. The frontier is now the later of the newest persisted row and the section-end time of the last acked chunk. The second half comes from the HISTORY_END metadata rather than from the sensor records, so it advances for a section nothing could decode, and it is taken at the ack, which is the moment the strap is told it may release those records. Taking the later of the two can only ever CLOSE a gap, never open one: a strap with genuine backlog has acked sections ending behind its newest banked record by definition, so it is unaffected. The warning. It asked whether a record decoded any of `heart_rate`, `gravity_x` or `ppg_waveform`, a list that has to be extended by hand for every layout NOOP learns and twice silently was not. #156 was v25 and v26 warning after they had decoded for releases. v20 has warned the same way ever since it was mapped, because the 5/MG optical record decodes to block counts and per-block headers and to none of those three names. So an iOS or macOS user with a v20 strap is told NOOP cannot decode a layout it decodes. The 5/MG side now asks the dispatch table the decoder itself switches on, which cannot drift from what NOOP decodes. WHOOP 4.0 keeps the field test, since it has no equivalent set and every layout it maps does emit one of the three. Both decisions are pure functions in WhoopProtocol with Kotlin twins, so the BLE classes they came out of are no longer the only place they can be checked. The layout test is driven off the dispatch set rather than a literal list, so a layout added there without a signature field cannot bring the warning back. Android's mapped set is deliberately narrower than Swift's and stays so: it has no v20/v21 historical branch, so v20 genuinely is undecodable there and genuinely should be reported. The two platforms answering differently for the same version is the divergence itself, and is now pinned on both sides so porting the decoder flips a test rather than surprising someone. Not changed: `checkStrapLiveness`'s stuck-detector reads the same row frontier and can therefore false-fire its "strap needs reboot" recovery on the same straps. Same root cause, different path, and it wants its own look rather than being swept in here. Android suites 5,749 green; WhoopProtocol 690 green.

…blished Re-review of my own commit. The frontier change used the `unix` a HISTORY_END carries as "the time this acked section ends at". The comment at the very call site I edited says otherwise: #773: corrupt future-RTC detection. A HISTORY_END carries the strap's own clock If that value is the strap's live RTC rather than the section's end time, then the safety property the whole change rested on inverts. "Taking the later of the two can only close a gap, never open one" is only benign when the second value describes ground actually covered. A live clock is approximately now on every ack, so it would close EVERY gap, on every strap, and stop a strap with real backlog after a single session. That is not a rare edge; it is what the change would do to every offload. I could not settle which it is from the code. `clockRef` is built from the GET_DATA_RANGE newest rather than from this field, so the clock-correlation path does not answer it either, and the #773 comment reads both ways: "the strap's own clock" says live, "always PAST-dated (it's banked history)" says section. Deciding by inference here means betting the offload loop on a guess, so the frontier work comes out until the field's meaning is established from a capture. What stays is the layout warning, which is independent of all of this and was verified on its own terms: a mapped 5/MG layout is no longer reported as undecodable, asked of the dispatch table rather than a hand-maintained list of field names. `offloadFrontier` goes with it rather than sitting unused, along with its tests. The diagnosis in the issue stands and is worth keeping: the row frontier genuinely cannot reach a strap whose newest records do not decode, and #1144's guard genuinely cannot catch it on a mixed-layout strap. What is missing is a consumed marker that is demonstrably a record time. Android suites 5,747 green; WhoopProtocol layout tests 4 green.

Re-review of my own fix. Suppressing the warning for v20 was the wrong correction, because only half of what the line said was wrong. It said: this layout does not decode, AND these records carry no heart rate or motion so a night made only of them cannot be staged. The first half is false for v20 on Swift, which is what the fix was for. The second half is TRUE, on both platforms: the 5/MG optical record decodes into raw arrays and the raw IMU record into accel/gyro channels, and no engine reads either, so those nights genuinely do not stage. Silencing the whole line for v20 would have removed the only thing in the strap log explaining why a user's nights were empty, which is worse than the wrong wording it replaced. So the question splits, and so does the message. Whether the layout decodes is asked of the 5/MG dispatch table, which cannot drift from what the decoder actually handles. Whether the record carries anything scoreable stays the field test, which is what that test was always really measuring. A mapped layout carrying nothing scoreable now gets its own line saying exactly that, and still asks for a report, because the missing piece is the meaning of those channels. The platform divergence is pinned on both sides rather than smoothed over: Android has no v20/v21 historical branch, so it answers UNMAPPED for v20 where Swift answers DECODES_WITHOUT_NAMED_SIGNAL. That is correct on both, and porting Swift's decoder will flip the Android test rather than surprise whoever does it. Android suites 5,748 green; WhoopProtocol 689 green.

…ever had Two more from re-reviewing. The verdict was taken from whichever record of a version came first in the chunk, but the conclusion it prints is about a firmware LAYOUT. One thin or off-wrist v18 record decoding no named field would have condemned v18 for the rest of the session. The old form had the same hole and it matters more now, because the verdict names a specific reason rather than a vague one. It is now taken over every record of that version in the chunk: a layout counts as carrying a signal when any of its records did. And the Kotlin twin added in the previous commit was called by nothing. Android printed a bare "historical records use layout vN" and stopped, so a user whose nights were not staging had the fact in their log and none of the explanation, while the same strap on iOS was told why. That is the housekeeping gap in the issue, which turned out to be missing guidance rather than the stale issue pointer it was filed as. Android now emits the same two messages, from the same shared classification, with the same layout-wide question behind them. The two platforms still answer differently for v20, and correctly so: Android has no v20/v21 historical branch, so it says the layout does not decode, where Swift says it decodes but carries nothing scoreable. Both are true on their own side. Android suites 5,748 green; WhoopProtocol 689 green; doc lint and the i18n gate clean.

…rst record's Parity audit against the Swift side. Swift reads every parsed record and reports each distinct layout it finds. Android sampled the FIRST decodable record and stopped, so a strap emitting v18 alongside an unscoreable layout logged only the v18 and never mentioned the other one. That is exactly the mixed-layout strap the new guidance exists for, so on the platform where it was newly added it would have stayed silent on the case that motivated it. The comment above the block already claimed "log each distinct layout once per session"; now it does. `firmwareLayout` sets one state value and the connection trace names one layout, so both stay bound to the chunk's first version exactly as before. Only the logging widened. The cost is one decode pass over the chunk, beside the one `extractHistoricalStreams` performs immediately above it, which is immaterial next to the transfer that delivered the chunk. Swift pays nothing for the same question because its frames are already parsed by the time this runs; that asymmetry is in how each platform threads the decode, and is noted at the site. Also checked and equal: both log messages are byte-identical across the platforms, compared by content rather than by position, and the enum cases carry the same three meanings. Android suites 5,748 green; doc lint clean.

…d too The reject archive's last test asked `decodeHistorical(...) == null`: did the record decode at all. Swift asks a different question in the same place, whether it yielded a unix AND either a heart rate or a gravity vector, and Swift's is the right one. A record that decodes into channels no engine reads is not data NOOP has kept. It is data NOOP has looked at and dropped, and the strap frees it on the very next trim ack. It is the same wrong question the unmapped-layout rule immediately above exists to replace, one step further down. That rule was introduced because deciding on the decode OUTCOME let a partially-mapped layout slip the screen and be kept nowhere at all; the tail underneath it still decided that way. No behaviour changes today. Every layout mapped on this platform either yields a heart rate (v18) or is short-circuited above as having its own durable stream (v26), so the two questions give the same answer for every record a strap can currently send, and the suites are unmoved at 5,748. They stop agreeing the moment a layout is mapped whose records carry neither, which is precisely what porting the 5/MG optical (v20) and raw IMU (v21) decoders does. Without this, that port would have silently stopped archiving the very bytes needed to work out what those channels mean, on the only platform currently collecting them. Landing it first makes the port safe rather than making it a trade. The test that pins it belongs with that port, where a v20 record is a real input that decodes with a unix and no heart rate or gravity. Asserting it here would need a frame no layout on this platform produces.

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.

ryanbr mentioned this pull request Sep 9, 2026

WHOOP 5/MG historical layout v20 is undecodable, so the offload frontier never closes #1992

Open

ryanbr changed the title ~~Measure the offload backlog by what we consumed, not by what became rows~~ Stop reporting a mapped historical layout as undecodable Sep 9, 2026

ryanbr added 4 commits September 9, 2026 18:14

ryanbr mentioned this pull request Sep 9, 2026

Decode the 5/MG optical and raw-IMU historical layouts on Android #2018

Merged

ryanbr merged commit 35a6512 into main Sep 9, 2026
16 checks passed

ryanbr deleted the fix/1992-frontier-and-layout-warning branch September 9, 2026 07:38

Navigation Menu

Repository navigation

Stop reporting a mapped historical layout as undecodable - #2017

Stop reporting a mapped historical layout as undecodable#2017
ryanbr merged 6 commits into
mainfrom
fix/1992-frontier-and-layout-warning

ryanbr commented Sep 9, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

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

Uh oh!

The warning fires for a layout NOOP decodes

The platform divergence is deliberate and now pinned

What came out, and why

Verification

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ryanbr commented Sep 9, 2026 •

edited

Loading