Measure how a duplicated second's two copies compare (#1505) - #1510
Merged
Conversation
…here are two (#1505) #1505 asks whether the WHOOP 5 v18 historical R-R is in 1/1024-second ticks rather than milliseconds. The strap emits the beat train twice - live over 0x2A37, which the BLE spec fixes at 1/1024 s and we convert on the way in, and again inside its v18 record, which we store as read. If those are the same beat in two units, a second written by two deliveries holds two values 1024/1000 apart. If they are genuinely two beats, the ratios scatter. deliveryHistogram already counts how many deliveries wrote each second. It has never looked at what they wrote, which is the measurement the question turns on. duplicatePairRatios reports the distribution of those ratios and takes no view on the answer. It is restricted to the unambiguous case: seconds carrying EXACTLY two rows, both ord 0. ord restarts per delivery, so that is two deliveries each contributing their first beat, not two consecutive beats from one record's array (which read 0 then 1). This exists because one pair cannot settle it. The field pair from #1451 is `-1s[872#0, 893#0]`, and 893 × 1000/1024 = 872.07 - an exact hit, but 21 ms is also an entirely ordinary difference between consecutive beats. Both readings explain it. A POPULATION separates them: a tight cluster at 1.024 is a unit mismatch, a broad spread is physiology. That is the same discipline the PPG→HR estimate failed in #194, where one matching night looked like validation. No behaviour change, no storage change, nothing scored differently - a diagnostic line beside the existing histogram, on the path that already runs for over-counted nights. Parts-per-thousand in integer arithmetic, and the output string is pinned by value in BOTH test suites, so a capture read on either platform is directly comparable and the twins cannot drift. That is deliberate: the rounding of a tie has silently diverged between these two platforms twice before. 9 Kotlin tests and 9 Swift twins: the real field pair, an ordinary beat-to-beat difference that must NOT read as a mismatch, exact duplicates, consecutive beats from one delivery being excluded, ambiguous >2-row seconds skipped, rows with no ord ignored, and both population shapes. Full Android suite 4182 tests 0 failures; doc lint and i18n --ci clean. The Swift half is StrandAnalytics, which does not build on Linux (GRDB/sqlite3.h), so it is validated by swift-packages CI rather than locally.
Two defects in the diagnostic from the commit before this. The parts-per-thousand multiply was `hi * 1000` in each platform's native Int. Kotlin's is 32-bit and Swift's is 64-bit, so a corrupt row above ~2.15 million ms wraps on Android and does not on Apple. A v18 u16 cannot reach that, so it is not reachable through the decoder today - but a diagnostic whose whole purpose is to be compared across two platforms must not have a width-dependent answer sitting in it, waiting for a stored value nobody bounded. Both sides now do the multiply in 64 bits. The pass also allocated a list per second and built a second dictionary over rows the histogram beside it had already walked. That histogram carries a comment explaining it was deliberately reduced to ONE dictionary with no per-second allocation, because it runs over a night's beats about 21 times per analyzeRecent, every 15 minutes - the pass #1005/#836 spent a release making cheaper. Adding a second allocation-heavy walk beside it, in the same file, under that comment, was not a defensible way to add a diagnostic. It now uses a PairTally per second, mirroring SecondTally directly above it. Behaviour is unchanged: both suites still pass, including the pinned output string that the twins are compared by, which is what makes that safe to assert. Android 4182 tests 0 failures; doc lint clean.
ryanbr
added a commit
that referenced
this pull request
Aug 21, 2026
…ity fixes (#1521) Four changes, all to code introduced during 10.5.1 itself. Build the R-R ord array once (both platforms). The over-count branch of the sleep R-R diagnostics calls three analysers, and each was handed its own freshly mapped copy of the night's ord column -- ~70k elements, on a branch reached ~21 times per analyzeRecent, every 15 minutes. All three expressions were identical, so two copies were waste. Value-identical by construction. (#1510) Read the strap model through @AppStorage (iOS). stepsPipelineActive did a UserDefaults lookup from inside keyMetricTile, once per metric per body pass, on a view that recomposes with live heart rate. SettingsView already declares the same key this way. Also makes the value observed, so the tile re-renders on a strap switch. Key the Android prompt on the strap model it reads. stepsCalibrationPrompt reads noop.selectedWhoopModel and returns null for anything but a 4.0, but the remember around it was keyed only on the three calibration values -- and that key is rewritten under a composed Today screen, from Settings and from live detection in WhoopBleClient. Switching straps left the stale answer on screen. Stop reading an unset strap model as a 4.0 (iOS). #1512 landed this prompt on both platforms and the halves disagreed about one state from the first commit: Android bails when the key is unset, Apple fell back to .whoop4 through WhoopModel.persisted. An import-only user who had never paired was offered "Need N more days where your phone also counted steps" for a strap nobody had seen. Optional-chained now; every real 4.0 owner has the key written by BLEManager.persistSelectedModel, so #1512's intent is untouched. Left for #1523: the calibration-state terms are still OR-ed outside the family check, so leftover sample days can activate the pipeline on a non-4.0 where Android cannot. A design question, not an oversight. Verified: android compileFullDebugKotlin + testFullDebugUnitTest (4189 tests, 0 failures, --no-build-cache --rerun-tasks); app-build green on both legs at a16d885 including the Test Strand step, so StrandTests ran; doc lint and i18n audit clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instrumentation for #1505. No behaviour change, no storage change, nothing scored differently.
What it measures
#1505 asks whether WHOOP 5 v18 historical R-R is in 1/1024-second ticks rather than milliseconds. The
strap emits the beat train twice — live over
0x2A37, which the BLE spec fixes at 1/1024 s and we converton the way in, and again inside its v18 record, which we store as read. If those are the same beat in two
units, a second written by two deliveries holds two values 1024/1000 apart. If they're genuinely two
beats, the ratios scatter.
deliveryHistogramalready counts how many deliveries wrote each second. It has never looked at whatthey wrote — which is the measurement the question turns on.
duplicatePairRatiosreports the distribution of those ratios and takes no view on the answer:Restricted to the unambiguous case — seconds carrying exactly two rows, both
ord 0.ordrestartsper delivery, so that's two deliveries each contributing their first beat, not two consecutive beats from
one record's array (which read
0then1).Why a measurement rather than the fix
One pair can't settle it. The field pair in #1451 is
-1s[872#0, 893#0], and 893 × 1000/1024 = 872.07 — anexact hit. But 21 ms is also an entirely ordinary difference between consecutive beats, so both readings
explain it. A population separates them: a tight cluster at 1.024 is a unit mismatch, a broad spread is
physiology.
That's the discipline the PPG→HR estimate failed in #194, where one matching night looked like validation.
A 2.4% shift to stored R-R moves HRV and the RSA-derived respiratory rate on both platforms; it should
follow evidence, not precede it.
Parity
Parts-per-thousand in integer arithmetic, and the output string is pinned by value in both test suites,
so a capture read on either platform is directly comparable and the twins can't drift. Deliberate: the
rounding of a tie has silently diverged between these two platforms twice before (#1476, #1486).
Verification
9 Kotlin tests and 9 Swift twins — the real field pair, an ordinary beat-to-beat difference that must
not read as a mismatch, exact duplicates, consecutive beats from one delivery excluded, ambiguous
Full Android suite: 4,182 tests, 0 failures.
doc_comment_lintandi18n_audit --ciclean.The Swift half lives in
StrandAnalytics, which doesn't build on Linux (GRDB /sqlite3.h), so it'svalidated by
swift-packagesCI rather than locally.What this doesn't do
It doesn't convert anything, and it doesn't touch the transport-tagging work @gdorgian has for #1008/#1118
— that stays theirs. It just produces the evidence both of those are waiting on.