[UtkuDenizAltiok]

What this PR does

A WHOOP 5/MG sends an exact 500 ms R-R interval as a filler when the wearer is at rest, and NOOP stored it as a real heartbeat (#2371). This marks those beats as suspect (tsSuspect = 1), the same flag every scoring read already skips for future-stamped beats (#1073). Nothing is deleted: the rows stay on disk.

The rule: a stored beat of exactly 500 ms, on WHOOP 5 history (srcChannel 5) or standard BLE (7), in a second whose stored heart rate is under 100 bpm. It runs after every R-R insert (only for a batch that carries such a beat), and once over existing rows as GRDB v47-rr-whoop5-fill / Room MIGRATION_40_41 (Room version 41). Channel 6 (live type-40) is left out because scoring never reads it; a beat with no heart rate in its second is left alone.

Why 100 bpm. One WHOOP 5.0's backup (443,896 beats on channels 5 and 7, four nights), each 500 ms beat matched to the strap's own heart rate in the same second, compared with the mean count of its neighbours (495–499 and 501–505 ms):

strap HR	500 ms beats	neighbour mean	ratio
under 70	5	0	—
70–79	312	5.9	52.9×
80–84	224	6.0	37.3×
85–89	155	4.5	34.4×
90–94	167	5.0	33.4×
95–99	10	3.3	3.0×
100–104	4	2.3	1.7×
105–109	0	3.4	0
110–114	8	8.4	1.0×
115–119	16	18.0	0.9×
120–124	8	8.3	1.0×

The filler stops at 100 bpm; above that, a 500 ms beat is as common as its neighbours, so it is a real beat and is kept. Below 100 bpm a real 500 ms beat would be at least 17% shorter than the average interval of its second. Every 500 ms beat in this backup has a heart rate for its second. The issue's own capture (a WHOOP MG, resting ~60 bpm) shows the same thing.

What it changes, measured on that backup (the migration applied to a copy):

• 873 beats marked: 508 of 236,669 on channel 5, 365 of 207,227 on channel 7 (0.2%). 36 beats of 500 ms at ≥ 100 bpm are kept.
• Nightly HRV barely moves. The issue expected cleanRR to reject every filler, but fillers often come in runs (up to 11 in a row), and inside a run the local median is 500 itself: 138 of the 508 channel-5 fillers pass the ectopic filter today (radius 2, 20%). Nightly RMSSD over each of the four sleep sessions (channel 5, with a Python replica of cleanRRGapAware + rmssdGapAware, not the app's own nightly pipeline) goes from 23.68 → 23.73, 29.54 → 29.64, 27.27 → 27.30 and 30.44 → 30.47 ms (+0.1% to +0.3%).
• The readers that do not run the ectopic filter stop seeing them: RhythmScreener range-filters but keeps ectopy on purpose, so each filler read as an irregular beat; SleepStagerV2's RSA respiration term only clamps to 300–2000 ms.

Type of change

• Bug fix (stored data)

How it was tested

• Swift (WhoopStore, 629 tests pass): Whoop5RrFillTests — the statements pinned by literal; at ingest only a 500 ms beat on channel 5/7 under 100 bpm is marked (99 bpm marked, 100 kept, 501 ms, channel 6, WHOOP 4, unlabelled and no-HR beats all kept, nothing deleted); a marked beat drops out of rrIntervals and stays on disk; a re-sync keeps the mark; v47 marks rows stored at v46 and leaves a v35 mark alone. With the insert statement switched off 3 tests fail, with the migration's statement removed 1 fails; both restored byte-identical (sha256). SchemaOracleTests pass with v47 and roomVersion 41.
• Kotlin: Whoop5RrFillTest pins the same literals and checks MIGRATION_40_41 runs exactly the migration statement; Whoop5RRSqliteTest runs the same ingest, scoring, re-sync and migration cases as the Swift tests through the production WhoopRepository.insert and the production SQL on SQLite, with the same expected rows. On the fork's Android CI the app builds and all 6,526 unit tests pass; with the fix switched off (a throwaway branch), exactly these four fail: the three ingest cases and the migration wiring.
• Parity: the SQL is one literal per platform, so the ledger pairs and compares WHOOP5_RR_FILL_FLAG_SQL / WhoopStore.whoop5RrFillFlagSQL and the migration pair value for value. The Room DAO method and the migration's migrate are recorded as one-sided in parity_dispositions.json, like promoteWhoop4HistoricalRr beside them, and the second commit is the output of parity_ledger.py --refresh-derived --base upstream/main on da4526f7. parity_ledger.py: no new findings; parity_ratchet.py: 0 errors.
• Full local run on a5afb4c0 (on da4526f7), every step passed: WhoopStore 629 · StrandAnalytics 2081 · StrandImport 327 · doc lint · i18n · parity ledger · ratchet · governance (124) · macOS StrandTests 2,244 (only the two locale-dependent TodayCarryOverTests) · iOS build.

Checklist

• Swift package tests pass for any package I touched (swift test in Packages/WhoopStore)
• Android unit tests pass if I touched android/ (fork CI, see above)
• No new build warnings introduced
• Follows the conventions in docs/CONTRIBUTING.md
• I did not commit generated output (Strand.xcodeproj/) or any secrets/keystores

Related issues

Refs #2371, #1073

🤖 Generated with Claude Code

[ryanbr]

@UtkuDenizAltiok Reviewed. This is a careful piece of work and the evidence behind it is better than most changes of this shape get.

Rather than read it over, I checked the parts that could be wrong in ways reading would not show.

The guard is load-bearing, and it holds. tsSuspect IS NULL only marks anything if stored rows really are NULL there. Had the insert path ever written an explicit 0, the migration would have been a silent no-op while every literal-pinned test still passed. It does not: the Room entity declares val tsSuspect: Int? = null and the GRDB insert omits the column from its list entirely. So the rule matches real rows, and it will not overwrite a beat already marked by #1073.

Both statements are byte-identical across platforms. I extracted the four literals and diffed them rather than comparing by eye: 277 bytes for the flag statement, 218 for the migration, no differences. Spelling each one out in full on both sides instead of composing it from SCORABLE_WHOOP5_CHANNELS is what lets the ledger pair them, and the re-derived authority confirms it worked: constant_pairs moves 683 to 685 while unpaired_constants does not move at all, so all four literals paired off.

Every version pin moves together. @Database(version), SCHEMA_VERSION, MIGRATION_40_41 both defined and registered, the exported 41.json, and roomVersion in both copies of the schema oracle, plus v47-rr-whoop5-fill in both grdbMigrations lists.

The stated mechanism is true. The comment claims the batch's heart rate is already on disk when the fill statement reads it, which is the difference between marking the batch and silently missing it. Swift orders hrSample then rrInterval then the fill statement; Kotlin orders insertHr, insertRr, flagWhoop5RrFill. whoop5Historical is 5 and whoop5Standard is 7, matching the IN (5, 7) in the SQL.

Your exported schema matches what KSP generates here. Regenerating on this machine produced no diff against your checked-in 41.json.

Full Android suite on the rebased branch: 6534 tests, 0 failures, 0 errors, across 769 classes, every result file written inside the run window. Ratchet: 0 errors.

One thing I changed

tsSuspect = 1 has meant exactly one thing since #1073, and your change gives it a second. Using the same flag is the right call, since every scoring read tests <> 1 and a different value would read as "not suspect" everywhere. But the column was still documented as only the first cause: Entities.kt said "1 when this beat's ts is in the FUTURE (corrupt ring time)", and Reads.swift annotated the filter "#1073: exclude future-stamped beats".

I have added a commit naming both causes, and the part that matters downstream: the mark records THAT a beat is excluded, never WHICH cause excluded it. A diagnostic that counted these and called them clock corruption would be asserting more than the column can attribute.

The rebase

Only your parity refresh commit conflicted, on parity_twin_map.json, because another change landed between your branch and main. I did not hand-merge it. That file is derived, and resolving count and sha256 pairs by hand yields a manifest that matches no tree at all. Resolving to main emptied the commit, so it dropped, and the same commit re-derives the authority from the actual rebased tree. Your code commit rebased cleanly and keeps your authorship.

One judgement call, which is mine and not a change request

The 100 bpm cutoff rests on one strap over four nights. Inside that sample the cliff is convincing, but the band just under it is where your own table is thinnest:

strap HR	500 ms	neighbour mean	ratio
90-94	167	5.0	33.4x
95-99	10	3.3	3.0x
100-104	4	2.3	1.7x

At 95-99 the excess has fallen from 33x to 3x, so roughly a third of what gets marked there is plausibly a real beat, and the "at least 17% shorter than its second's mean" argument is weakest exactly at the boundary. The reader this touches is the one you already flag honestly: RhythmScreener keeps ectopy on purpose, and a genuine premature beat at rest sits at 40 to 80 percent of the normal interval, which at 60 bpm spans 400 to 800 ms.

What keeps it small is the thing your table actually shows: the rule demands EXACTLY 500 ms, and a real premature beat can land anywhere in that span while the fill is always 500, which is why the ratio is 33x rather than 2x. 873 beats in 443,896 is 0.2%, nothing is deleted, and the mark can be re-derived later.

My read is that it ships as written. I am recording the trade rather than asking you to change it.

Thank you for the distribution table, for measuring the blast radius on a real backup instead of asserting it, and for switching the fix off to see which tests go red rather than trusting that they would.