Repository navigation
fix(diag): keep the Night line's rr= counting beats banked, on both platforms - #2435
Conversation
5a31b72 to
aef5a4d
Compare
…latforms The one-Oura-channel selection changed what `rr=` in the strap log's `Night …: grav=N hr=N rr=N resp=N skin=N` line counts, from beats banked to beats scored, on BOTH platforms: Swift DebugDataDiagnostics reads WhoopStore.rrIntervals, and Kotlin AndroidDiagnostics reads rrIntervalsForDevice, which routes to the same selecting SQL. Add WhoopStore.rawRrIntervals(deviceId:from:to:limit:), the twin of Kotlin WhoopDao.rawRrIntervals / RAW_RR_INTERVALS_SQL (0x6E and tsSuspect excluded, no selection, same ORDER BY), and point both Night lines at the raw read, so `rr=` means what it meant in every log filed before and the two platforms still agree. This also pairs the Kotlin rawRrIntervals in the parity ledger. Test: RrSourceChannelTests.testTheRawDiagnosticReadKeepsBothBeatChannels, on the same fixture and expected values as Kotlin OuraOneChannelReadSqliteTest.theRawExportReadKeepsBothBeatChannels. Refs ryanbr#2424 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9RCdbENSxgnGFTikJPbBq
…ined night Two consequences of pointing the Night line at the raw read, documented rather than discovered. A WHOOP 5's `rr=` does not go back to what it was; it steps UP. The strict single-transport selection in `rrIntervals` predates the one-Oura-channel change, so a 5/MG's `rr=` was already the scored transport in every older log. The raw read drops that too, and now counts every banked transport. That is the right answer for a line whose job is to say what the strap banked, since a legacy row that cannot be spliced into a scored beat train was still banked, and one number meaning "banked on a ring, scored on a strap" would be worse. But it is a change, on a device this PR's own summary describes as unaffected, so it is written down where the number is produced. The raw read also does not collapse a record the drain served twice, where `rrIntervals` now does (ryanbr#2456). On a redrained night `rr=` therefore counts the duplicates while the `hrv diag` line does not, and the two together are what shows a redrain happened at all. A strap log and the app disagreeing on beat counts is the signal rather than a fault, which is worth stating before the next report reads it as one.
aef5a4d to
10f1aa4
Compare
|
You were right and I was wrong about the scope, so this is yours rather than mine. I said on #2423 that I would add the Swift twin and push it to that branch, and told you not to start on it. What I offered covered one platform. Your reply pointed out that Android's Night line reads the selecting query too, which meant the fix was both platforms or it was nothing, and you had already worked that out while I was still describing a narrower job. Taking your version and dropping the branch I had started. Rebased onto main now that #2423 has landed. Your commit is unchanged; I checked its tree against the rebase you had already pushed and they are byte-identical, so nothing of yours was replaced. I added one commit on top, documenting two consequences rather than changing behaviour. The first is the trade-off you raised for the reviewer, and the answer is your first option, with a correction to how the PR describes it. Keeping The second you could not have known about, because it merged after you filed. #2457 collapses a ring record the drain served twice, at Merging once the WHOOP 5 checks come back; your new test runs there and I cannot run it here. |
…latforms (ryanbr#2435) The one-Oura-channel selection (ryanbr#2423) changed what `rr=` counts in the strap log's `Night …: grav=N hr=N rr=N resp=N skin=N` line, from beats BANKED to beats scored, on both platforms: Swift `DebugDataDiagnostics` read `WhoopStore.rrIntervals`, and Kotlin `AndroidDiagnostics` read `rrIntervalsForDevice`, which routes to the same selecting SQL. The two platforms still agreed with each other and both differed from every log filed before. `WhoopStore.rawRrIntervals` is added as the twin of Kotlin `WhoopDao.rawRrIntervals` (`RAW_RR_INTERVALS_SQL`): the 0x6E duplicate and quarantined beats excluded, no scoring selection, the same ORDER BY. Both Night lines point at it. Scoring is untouched, every scoring reader still goes through `rrIntervals`, and the Kotlin function stops being one-sided in the parity ledger. Two consequences are documented where the number is produced rather than left to be discovered. A WHOOP 5's `rr=` does not go back to what it was; it steps UP. The strict single-transport selection predates ryanbr#2423, so a 5/MG's `rr=` was already the scored transport in every older log, and the raw read drops that too. That is the right answer for a line whose job is to say what the strap banked, since a legacy row that cannot be spliced into a scored beat train was still banked, and one number meaning "banked on a ring, scored on a strap" would be worse. It is still a change, on a device this work otherwise leaves alone. The raw read also does not collapse a record the drain served twice, where `rrIntervals` now does (ryanbr#2456). On a redrained night `rr=` counts the duplicates and the `hrv diag` line does not, so the two disagreeing is the clearest signal that a redrain happened at all, rather than a fault. Tested on the same fixture and expected values as the Kotlin `OuraOneChannelReadSqliteTest.theRawExportReadKeepsBothBeatChannels`, so the two reads cannot drift into agreeing in prose while differing in fact. Authored by @pipiche38. Refs ryanbr#2424, ryanbr#2423.
What this PR does
Stacked on the branch
fix/oura-rr-score-one-channel. Its one commit (bde7b94d6, the one-Oura-channel selection) shows in this diff until that branch merges. This PR adds one commit on top:393cbe20a.The one-channel selection changed what
rr=counts in the strap log'sNight …: grav=N hr=N rr=N resp=N skin=Nline, from beats banked to beats scored, on both platforms:DebugDataDiagnostics.swift:284readsWhoopStore.rrIntervals.AndroidDiagnostics.kt:385readsrrIntervalsForDevice, which routes todao.rrIntervals, the same selecting SQL.The two platforms still agree with each other, but both now differ from every strap log filed before the selection. The only reader of the unselected Kotlin
rawRrIntervalsisRawSensorExport. Swift has no equivalent, so the parity ledger reportsadd-unpaired-function: WhoopDao.kt::rawRrIntervals/4#1.This PR:
WhoopStore.rawRrIntervals(deviceId:from:to:limit:)inReads.swift, the twin of KotlinWhoopDao.rawRrIntervals/RAW_RR_INTERVALS_SQL: the 0x6E duplicate andtsSuspectbeats excluded, no selection, the sameORDER BY.rawRrIntervalsForDevice.So
rr=means what it meant in every log filed before, on both platforms, and the ledger finding is paired. Scoring is untouched: every scoring reader still goes throughrrIntervals.Trade-off for the reviewer. On a WHOOP 5, the raw read also skips the strict single-transport selection, so
rr=on a 5/MG counts every stored transport, not the one scored. If that line should stay as it was on a WHOOP 5, the alternative is to leaverr=as scored on both platforms and add a separaterrBanked=from the raw read.Type of change
How it was tested
RrSourceChannelTests.testTheRawDiagnosticReadKeepsBothBeatChannels(new) uses the same fixture and expected values as KotlinOuraOneChannelReadSqliteTest.theRawExportReadKeepsBothBeatChannels: both Oura beat channels come back, 0x6E is still excluded, 17 rows.RrSourceChannelTests18/0.OuraOneChannelReadSqliteTest,Whoop5RRSqliteTestandcom.noop.testcentre.*, 198/0;compileFullDebugKotlinclean.Strandapp build succeeds (DebugDataDiagnostics.swiftis app-target Swift).Checklist
swift testinPackages/<name>)android/(./gradlew testFullDebugUnitTest)StrandDesigntokens — no hardcoded colors, fonts, or spacing (no UI change)docs/CONTRIBUTING.mdStrand.xcodeproj/) or any secrets/keystoresRelated issues
Refs #2424, #2423