Retire two superseded R-R shadow measurements (#1331/#1118) - #1487
Merged
Conversation
The shadow de-dup block ran 13 full passes over a night's R-R rows, six of them collapseOverCount, and each collapse SORTS the night's ~50-70k intervals. It runs for EVERY night of an affected strap, and analyzeRecent re-scores ~21 days every 15 minutes - so the phones least able to spare the work were paying the most, for instrumentation that feeds no stored value. Two of those measurements have finished their job. The same-second TOLERANCE SWEEP (20/34/60) was sizing a fix already ruled out. Every affected night reads crossSecondOverCount, so no same-second tolerance can reach duplicates that straddle the boundary - the conclusion recorded on #1331 before this change. `xsec`, the 40 ms collapse widened to a 1-second window, was documented from the start as a strict UPPER BOUND that over-merges real beats, kept only to size how far a cross-second collapse COULD get. It has now produced that number in the field: covXsec 0.80 with beatAccXsec 0.26 - coverage driven BELOW 1.0 and accuracy collapsed, eating real beats exactly as its own comment predicted. What survives is the honest floor (`ex`, exact duplicates only, provably no real-beat loss) and the incumbent candidate (`dd`, 40 ms same-second). The delivery histogram merged in #1486 supersedes what both retired measurements reached for, and costs one pass rather than nine. 13 passes per night became 6, and six sorts became two, identically on both platforms - verified by counting call sites in each rather than by reading the diff. Neither platform still emits `hrv sweep` or the xsec fields, and the replacement fragment in the surviving `hrv dedup` line is byte-identical across the two. An earlier revision of this commit said six sorts "became three". It is two: ex and dd. Corrected in the code rather than left as a plausible-sounding number, since that is the whole failure mode this block exists to avoid. Verification: compileFullDebugKotlin clean, full Android unit suite green, doc_comment_lint clean.
Re-review. The retirement comment and the surviving `hrv dedup` line both told the reader the delivery histogram is "below". It is emitted BEFORE the dedup line - Swift appends it at 1092 against ~1120, Kotlin at 901 against ~928 - so in the log it sits ABOVE. A pointer in a diagnostic is only worth anything if it points the right way, and this one would have sent someone reading a strap log looking past the thing it was recommending. Four references, two per platform, now say above. Nothing else changed. Verified this pass: no orphaned variables survive the deletion on either platform, nothing anywhere consumes `hrv sweep` or the xsec fields (no readout, test or doc parses them), the delivery histogram is still wired on both, and the nine surviving fields of the `hrv dedup` line match across the two platforms name for name. Verification: compileFullDebugKotlin clean, full Android unit suite green, doc_comment_lint clean, emitted fragment byte-identical across platforms.
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.
Follow-up to #1486. No behaviour change — this only removes instrumentation that has finished its job.
The cost
The shadow de-dup block ran 13 full passes over a night's R-R rows, six of them
collapseOverCount— and each collapse sorts the night's ~50–70k intervals.It runs for every night of an affected strap, and
analyzeRecentre-scores ~21 days every 15 minutes. So the phones least able to spare the work were paying the most, for instrumentation that feeds no stored value, permanently.Why these two specifically
The same-second tolerance sweep (20/34/60) was sizing a fix already ruled out. Every affected night reads
crossSecondOverCount— no same-second tolerance can reach duplicates that straddle the boundary. That conclusion is recorded on #1331; the sweep kept measuring it anyway.xsecwas always labelled unshippable — a strict upper bound that over-merges real beats, kept only to size how far a cross-second collapse could get. It has now produced that number in the field:Coverage driven below 1.0 and accuracy collapsed — it eats real beats, exactly as its own comment predicted. The measurement succeeded, so it can stop running.
What survives
ex(exact duplicates only — provably no real-beat loss) as the honest floor, anddd(40 ms same-second) as the incumbent candidate. The delivery histogram from #1486 supersedes what both retired measurements were reaching for, at one pass instead of nine.Parity
Identical on both platforms, verified by counting call sites in each file rather than reading the diff: 6 passes / 2 sorts on each, neither still emits
hrv sweepor thexsecfields, and the replacement fragment in the survivinghrv dedupline is byte-identical across the two.One correction inside this change
An earlier revision of this commit's message said six sorts "became three". It is two —
exanddd. Corrected in the code rather than left as a plausible-sounding number, which is the precise failure mode this whole block exists to avoid.Verification
compileFullDebugKotlinclean · full Android unit suite green ·doc_comment_lintclean.Swift is app-target here, so it needs an
app-buildrun before merge.