Skip to content

fix(sleep): hand the #899 heal's bank-recency witness to the computed id only, never to a ring's own rows - #2372

Merged
ryanbr merged 1 commit into
ryanbr:mainfrom
pipiche38:fix/sleep-heal-witness-computed-only
Sep 23, 2026
Merged

ryanbr merged 1 commit into
ryanbr:mainfrom
pipiche38:fix/sleep-heal-witness-computed-only

Conversation

@pipiche38

Copy link
Copy Markdown

What the user saw

An Oura night (iOS 11.8.0, 2026-09-19 → 20) displayed as ending 04:48 when the wearer woke at ~08:18. The ring had served the full night — the strap log shows sleep session persisted [22:23:51 → 08:21:51] at 08:37:22 — and the #899 heal deleted it one second later:

Dedup(#1284): dropped [1789874631 -> 1789910511] min=598 … kept [1789874453 -> 1789894673] min=337 … - heal

The same heal had already dropped the 388 / 471 / 514-min re-serves earlier that morning, each time keeping the 337-min row (22:20 → 03:57). The day then scored on that row plus a 26-min no-onset fragment (04:22 → 04:48): sleep day=2026-09-20 totalSleepMin=315, night ending 04:48.

Root cause

The post-upsert heal runs SleepSessionDedup.dedupe(healable, freshStarts: keptStarts) for every id in healDeviceIds (#1248), including the ring's own. keptStarts is the computed pass's bank-recency witness, and the comment claimed it "only matches the computedId rows". That is false on an Oura day: the pass's sessions are the ring's providedSleep rows with startTs copied verbatim (analyzeDay folds provided sessions unchanged), so the same key names the row the pass read in the ring's own table. In the ring's sweep that row ranks "fresh" (rank 2 in rankKey) and beats "longest" (rank 3) — the heal prefers whatever the pass consumed at read time over anything the ring persisted afterwards.

It bites hard on a phone because the OS suspends the app between a pass's read and its heal (04:14 read → 07:22 heal here), and the app wakes precisely for the BLE drain that banks the fuller re-serve. It is self-perpetuating: the next pass reads only the survivor, re-witnesses it, and deletes the next fuller row.

Not a regression from a recent build — #1248 landed 2026-08-11; long/suspended passes just widened the window.

Fix

One shared rule, SleepSessionDedup.healWitness(for:computedId:keptStarts:) (Kotlin twin healWitness(healId, computedId, keptStarts)): the witness is handed to the computedId sweep only; every other id gets none and falls back to longest-wins, the read-side default — which the #1284 corpus already showed is the WHOOP-matching row on every night. The heal's existing re-arm re-pass then scores the kept full night. No schema change, no behaviour change for a WHOOP strap (its rows live only under computedId).

Verification

  • Packages/WhoopStore: swift test --filter SleepSessionDedupTests — 33/33, two new: the witness rule, and the 09-19/20 scenario (read row 337 min in keptStarts, fuller 598-min re-serve) keeps the fuller row through the ring's sweep; the leaked-witness outcome is pinned in the same test so the regression cannot creep back.
  • Android: ./gradlew testFullDebugUnitTest --tests com.noop.analytics.SleepSessionDedupTest — 33/33, same two tests case-for-case; compileFullDebugKotlin green.
  • macOS app target compiled locally (xcodebuild … -scheme Strand … build), since swift-packages.yml does not cover Strand/Data/IntelligenceEngine.swift.
  • Hardware: not yet — needs one night on a ring with the pass suspended across a drain. The strap-log proof is the Dedup(#1284): dropped … kept … line reading dropped min=<shorter> kept min=<longer> after a wake, which the 09-19/20 export shows inverted.

Refs #899, #1248, #1284.

…mputed id only, never to a ring's own rows

The post-upsert heal ran `dedupe(healable, freshStarts: keptStarts)` for every
id in `healDeviceIds` (ryanbr#1248), including an Oura ring's own. `keptStarts` is the
computed pass's bank witness, and the comment claimed it "only matches the
computedId rows" - false on an Oura day, where the pass's sessions ARE the
ring's `providedSleep` rows with `startTs` copied verbatim. The ring row the
pass had READ was therefore ranked "fresh" in the ring's own sweep and outranked
every fuller re-serve the ring banked while the pass was in flight (hours on a
phone, where the OS suspends the app between the read and the heal and wakes it
precisely for the drain that persists the fuller row). On 2026-09-19/20 the heal
deleted the 598-min full night (22:23 -> 08:21) one second after it landed and
kept the 337-min row read at 04:14; the day scored 315 min ending 04:48.

`SleepSessionDedup.healWitness` is now the one shared rule: the witness goes to
the computedId sweep only; every other id gets none and falls back to
longest-wins, the read-side default. The heal's existing re-arm re-pass then
scores the kept full night. Kotlin twin case-for-case; no schema change and no
behaviour change for a WHOOP strap, whose rows live only under computedId.

Tests: SleepSessionDedupTests (Swift 33/33) and SleepSessionDedupTest (Kotlin
33/33) gain the witness rule and the 09-19/20 scenario, with the leaked-witness
outcome pinned so it cannot creep back. macOS `Strand` target compiled locally.

Refs ryanbr#899, ryanbr#1248, ryanbr#1284.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcXLwCpyu7skJQcfYAoDzi
@ryanbr
ryanbr force-pushed the fix/sleep-heal-witness-computed-only branch from 8dbfbf0 to e6bc54d Compare September 23, 2026 04:40
@ryanbr
ryanbr merged commit 720187a into ryanbr:main Sep 23, 2026
16 checks passed
@pipiche38
pipiche38 deleted the fix/sleep-heal-witness-computed-only branch September 23, 2026 06:42
@pipiche38

Copy link
Copy Markdown
Author

The heal did exactly what #2372 intended

Three heals fired in your capture, and every one of them drops the shorter row and keeps the fuller:

Dedup(#1284): dropped [1790048761 -> 1790060641] min=198 segs=20 kept [1790048801 -> 1790072201] min=390 segs=52 - heal
Dedup(#1284): dropped [1790048714 -> 1790074934] min=437 segs=52 kept [1790048694 -> 1790079834] min=519 segs=57 - heal
Dedup(#1284): dropped [1790048801 -> 1790072201] min=390 segs=52 kept [1790048694 -> 1790079834] min=519 segs=57 - heal

That is the inverse of the 9/20 report, where the line read dropped min=598 kept min=337 — the
fuller row was the one being deleted. The 519-minute row survives to the end of the capture: the three
Dedup(#899) sweeps that follow all report removed 0, so nothing re-took it.

Worth saying explicitly for anyone reading later: we could not produce this shape on our own
hardware.
Our 09-21/22 night was fully 0x49-anchored, every persist keyed the same generation, and
the heal reported removed 0 on all four passes — so the fix went in with the defect never having
been observed against it. Your night is the first evidence that the corrected witness behaves the way
the change argued it would.

The SpO2 line — you found a real bug, but not the one it looks like

Short version: SpO2 was working the whole time. Two separate things are confusing the log.

1. subscription=0 is not the off switch. Counter-intuitive, and we should probably say so in the
line itself. The ring reports subscription=0 for features that are plainly running — daytime HR
streams at subscription=0 — so the code deliberately does not treat it as a gate. What the code
reads as "the cloud never enabled this" is mode, status and state all zero, and it prints an
explicit - INACTIVE (server-gated off; …) suffix when that happens. Your lines read
mode=1 (automatic) with no such suffix, so nothing in that line claimed SpO2 was off.

On our own ring, where SpO2 is unambiguously working (Oura app 96 %, NOOP's Blood Oxygen card 96 % the
same morning), every feature-status line all night also reads subscription=0.

2. dc_raw is a different channel from the SpO2 percentage. The ring sends two things:

  • 0x6F — the firmware-computed SpO2 percentage, tagged raw (a legacy channel label)
  • 0x77 — a raw DC perfusion channel, tagged dc_raw, whose magnitudes run from about
    −1,000 to 11.7 million

first SpO2 decoded (last night) fires on whichever .spo2 event the drain happens to serve first,
and prints its value and unit without distinguishing the two. In your capture the first one was a
0x77, hence value 41132 (dc_raw) — a perfusion count, not a percentage, and not evidence of
anything being off.

Both channels were flowing in your bundle. Walking the raw sidecar gives 5,089 0x6F packets (the
percentage channel) against 12,646 0x77 — so the percentage channel was live the whole night.
Your 360,847 stored spo2 rows are consistent with that.

You can see the coin-flip directly on our side. Same line, same ring, consecutive reconnects:

[23:50:43] Oura: first SpO2 decoded (last night) - value 93 (raw)        <- 0x6F, a percentage
[02:32:27] Oura: first SpO2 decoded (last night) - value -288 (dc_raw)   <- 0x77, perfusion
[02:39:56] Oura: first SpO2 decoded (last night) - value 101144 (dc_raw)
[06:53:30] Oura: first SpO2 decoded (last night) - value 93 (raw)

Nothing changed about the ring between 23:50 and 02:32; only which packet arrived first.

So there is a defect here, and it is the log line. Calling a five-digit perfusion count "SpO2" is
exactly the kind of diagnostic that asserts more than it observed, and it cost you a false alarm.
It should name the channel — and arguably report the percentage channel and the raw channel
separately, since one of them is the number a reader actually wants. Happy to open that as a small
PR if it is wanted; it is a log-copy change, no behaviour.

(Unrelated but worth knowing while you are looking at SpO2: roughly 47 % of decoded 0x6F samples
read above 100 %, which no ground truth yet explains. Those land in the raw channel only and never
reach the Blood Oxygen card, so they do not surface as readings — it is a known open question in
docs/OURA_PROTOCOL.md §6.5, not something your test introduced.)

pipiche38 added a commit to pipiche38/noop that referenced this pull request Sep 23, 2026
`Oura: first SpO2 decoded (last night) - value N (unit)` fired on whichever
`.spo2` event the history drain happened to serve first. Two decoders feed that
event with quantities three orders of magnitude apart — 0x6F/0x7B carry a
firmware-computed percentage, 0x77 carries a raw DC perfusion magnitude — so on
one ring, minutes apart, the log read `value 93 (raw)` and then `value 101144
(dc_raw)`. A reporter read the five-digit one as a percentage and opened a
defect against SpO2 that was never wrong; their capture in fact carried 5,089
0x6F packets beside 12,646 0x77, with the percentage channel live all night.

`OuraSpO2Channel` (OuraProtocol + com.noop.oura) resolves the channel from the
unit tag and owns both the label and the log line, so the two platforms cannot
disagree about what they call these numbers. One latch per channel replaces the
single `loggedFirstSpo2` flag, so each line names one quantity and a session
that only ever saw perfusion says so instead of implying a percentage arrived:

  Oura: first SpO2 percentage decoded (last night) - 93 % (channel "raw")
  Oura: first SpO2 raw DC perfusion (NOT a percentage) decoded (last night) - 101144 (channel "dc_raw")

The `%` is appended on the percentage channel only — `-288 %` would be the same
bug in a new costume, and a test pins that. `OuraStreamMapping` keeps its own
`unit == "raw"` allow-list on both platforms and is untouched: it is a
persistence gate, so an unrecognised future unit must fall on the "do not store"
side, whereas this resolver has to name every sample it is handed. Log copy
only; no behaviour, no stored value, no new strings.

Verified: OuraProtocol 262/0 (10 new, incl. the decoders really stamping those
tags); Kotlin `OuraSpO2ChannelOracleTest` 3/3 asserting the verbatim stdout of
the shipped Swift `firstDecodedLogLine`; Android 6381 tests, the 8 failures are
pre-existing on `upstream/main`; macOS `Strand` build + `StrandTests` TEST
SUCCEEDED; iOS `NOOPiOS` BUILD SUCCEEDED; doc lint and `i18n_audit --ci` clean.

Refs ryanbr#2372

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RQ5haNgsWbrEFMpjviYns8
pipiche38 added a commit to pipiche38/noop that referenced this pull request Sep 23, 2026
`Oura: first SpO2 decoded (last night) - value N (unit)` fired on whichever
`.spo2` event the history drain happened to serve first. Two decoders feed that
event with quantities three orders of magnitude apart — 0x6F/0x7B carry a
firmware-computed percentage, 0x77 carries a raw DC perfusion magnitude — so on
one ring, minutes apart, the log read `value 93 (raw)` and then `value 101144
(dc_raw)`. A reporter read the five-digit one as a percentage and opened a
defect against SpO2 that was never wrong; their capture in fact carried 5,089
0x6F packets beside 12,646 0x77, with the percentage channel live all night.

`OuraSpO2Channel` (OuraProtocol + com.noop.oura) resolves the channel from the
unit tag and owns both the label and the log line, so the two platforms cannot
disagree about what they call these numbers. One latch per channel replaces the
single `loggedFirstSpo2` flag, so each line names one quantity and a session
that only ever saw perfusion says so instead of implying a percentage arrived:

  Oura: first SpO2 percentage decoded (last night) - 93 % (channel "raw")
  Oura: first SpO2 raw DC perfusion (NOT a percentage) decoded (last night) - 101144 (channel "dc_raw")

The `%` is appended on the percentage channel only — `-288 %` would be the same
bug in a new costume, and a test pins that. `OuraStreamMapping` keeps its own
`unit == "raw"` allow-list on both platforms and is untouched: it is a
persistence gate, so an unrecognised future unit must fall on the "do not store"
side, whereas this resolver has to name every sample it is handed. Log copy
only; no behaviour, no stored value, no new strings.

Verified: OuraProtocol 262/0 (10 new, incl. the decoders really stamping those
tags); Kotlin `OuraSpO2ChannelOracleTest` 3/3 asserting the verbatim stdout of
the shipped Swift `firstDecodedLogLine`; Android 6381 tests, the 8 failures are
pre-existing on `upstream/main`; macOS `Strand` build + `StrandTests` TEST
SUCCEEDED; iOS `NOOPiOS` BUILD SUCCEEDED; doc lint and `i18n_audit --ci` clean.

Refs ryanbr#2372

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RQ5haNgsWbrEFMpjviYns8
pipiche38 added a commit to pipiche38/noop that referenced this pull request Sep 23, 2026
…tag as unknown

Review follow-up. `forUnit` had no `Kotlin twin:` reference, so the parity ledger
counted both sides as one-sided (`add-unpaired-function ... forUnit/1#1`, Swift
and Kotlin). The reference is now on both declarations.

`forUnit` also treated anything that is not `dc_raw` as a percentage, so a case
variant or a future tag would print its magnitude with a `%` on it: the defect
this type exists to stop. Both known tags (`raw`, `dc_raw`) now match exactly
and anything else resolves to a third `unknown` channel, which names its tag and
never appends `%`. Nothing reaches it today (the ring emits only those two tags);
this is durability. The per-channel latch gains the matching third slot for free.

Verified: OuraProtocol 263/0; Kotlin `OuraSpO2ChannelOracleTest` 3/3 against the
re-captured stdout of the shipped Swift (15 rows, 5 of them unknown tags);
`parity_ledger.py` leaves only `twin-map-authority-drift|unpaired_properties`,
which the reviewer kept; doc lint clean.

Refs ryanbr#2372

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01484Ef871BnAuniyDJYYHoV
pipiche38 added a commit to pipiche38/noop that referenced this pull request Sep 23, 2026
…tag as unknown

Review follow-up. `forUnit` had no `Kotlin twin:` reference, so the parity ledger
counted both sides as one-sided (`add-unpaired-function ... forUnit/1#1`, Swift
and Kotlin). The reference is now on both declarations.

`forUnit` also treated anything that is not `dc_raw` as a percentage, so a case
variant or a future tag would print its magnitude with a `%` on it: the defect
this type exists to stop. Both known tags (`raw`, `dc_raw`) now match exactly
and anything else resolves to a third `unknown` channel, which names its tag and
never appends `%`. Nothing reaches it today (the ring emits only those two tags);
this is durability. The per-channel latch gains the matching third slot for free.

Verified: OuraProtocol 263/0; Kotlin `OuraSpO2ChannelOracleTest` 3/3 against the
re-captured stdout of the shipped Swift (15 rows, 5 of them unknown tags);
`parity_ledger.py` leaves only `twin-map-authority-drift|unpaired_properties`,
which the reviewer kept; doc lint clean.

Refs ryanbr#2372

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01484Ef871BnAuniyDJYYHoV
(cherry picked from commit fa1d651)
pipiche38 added a commit to pipiche38/noop that referenced this pull request Sep 23, 2026
`Oura: first SpO2 decoded (last night) - value N (unit)` fired on whichever
`.spo2` event the history drain happened to serve first. Two decoders feed that
event with quantities three orders of magnitude apart — 0x6F/0x7B carry a
firmware-computed percentage, 0x77 carries a raw DC perfusion magnitude — so on
one ring, minutes apart, the log read `value 93 (raw)` and then `value 101144
(dc_raw)`. A reporter read the five-digit one as a percentage and opened a
defect against SpO2 that was never wrong; their capture in fact carried 5,089
0x6F packets beside 12,646 0x77, with the percentage channel live all night.

`OuraSpO2Channel` (OuraProtocol + com.noop.oura) resolves the channel from the
unit tag and owns both the label and the log line, so the two platforms cannot
disagree about what they call these numbers. One latch per channel replaces the
single `loggedFirstSpo2` flag, so each line names one quantity and a session
that only ever saw perfusion says so instead of implying a percentage arrived:

  Oura: first SpO2 percentage decoded (last night) - 93 % (channel "raw")
  Oura: first SpO2 raw DC perfusion (NOT a percentage) decoded (last night) - 101144 (channel "dc_raw")

The `%` is appended on the percentage channel only — `-288 %` would be the same
bug in a new costume, and a test pins that. `OuraStreamMapping` keeps its own
`unit == "raw"` allow-list on both platforms and is untouched: it is a
persistence gate, so an unrecognised future unit must fall on the "do not store"
side, whereas this resolver has to name every sample it is handed. Log copy
only; no behaviour, no stored value, no new strings.

Verified: OuraProtocol 262/0 (10 new, incl. the decoders really stamping those
tags); Kotlin `OuraSpO2ChannelOracleTest` 3/3 asserting the verbatim stdout of
the shipped Swift `firstDecodedLogLine`; Android 6381 tests, the 8 failures are
pre-existing on `upstream/main`; macOS `Strand` build + `StrandTests` TEST
SUCCEEDED; iOS `NOOPiOS` BUILD SUCCEEDED; doc lint and `i18n_audit --ci` clean.

Refs ryanbr#2372

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RQ5haNgsWbrEFMpjviYns8
pipiche38 added a commit to pipiche38/noop that referenced this pull request Sep 23, 2026
…tag as unknown

Review follow-up. `forUnit` had no `Kotlin twin:` reference, so the parity ledger
counted both sides as one-sided (`add-unpaired-function ... forUnit/1#1`, Swift
and Kotlin). The reference is now on both declarations.

`forUnit` also treated anything that is not `dc_raw` as a percentage, so a case
variant or a future tag would print its magnitude with a `%` on it: the defect
this type exists to stop. Both known tags (`raw`, `dc_raw`) now match exactly
and anything else resolves to a third `unknown` channel, which names its tag and
never appends `%`. Nothing reaches it today (the ring emits only those two tags);
this is durability. The per-channel latch gains the matching third slot for free.

Verified: OuraProtocol 263/0; Kotlin `OuraSpO2ChannelOracleTest` 3/3 against the
re-captured stdout of the shipped Swift (15 rows, 5 of them unknown tags);
`parity_ledger.py` leaves only `twin-map-authority-drift|unpaired_properties`,
which the reviewer kept; doc lint clean.

Refs ryanbr#2372

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01484Ef871BnAuniyDJYYHoV
(cherry picked from commit fa1d651)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants