Repository navigation
Oura SpO2 candidate tile + Deep Timeline single-channel fix (rebase of #1568) - #1584
Merged
Merged
Conversation
…n empty chart The Deep Timeline plotted SpO2 as a red/IR ratio and skipped any sample with ir <= 0. A WHOOP 4.0 v24 banks BOTH optical channels so that works, but an Oura ring reports only ONE channel: OuraStreamMapping stores it in `red` and leaves ir = 0 (an unread channel, never a fabricated second reading). The ratio path therefore discarded 100% of a ring's SpO2 - 18,688 rows on the reporting device - and drew an empty chart with no explanation (the #623 "unsupported strap" notice only fires for the 5.0 family, so a ring owner got no notice either). Adds Repository.spo2TimelineValue(red:ir:), a pure decision: two-channel keeps the unchanged unitless ratio proxy (#166 - there is still no calibrated %), single-channel plots the reading itself. A real overnight capture (2026-08-01) shows the ring's channel clustering at 95-105, i.e. already a genuine %SpO2, not an ADC awaiting calibration. RANGE GATE, single-channel only: the `unit` tag that separates the ring's true-percentage channel from its different-scale perfusion channel is NOT persisted (spo2Sample stores only red/ir), so rows banked before that split was fixed are distinguishable only by magnitude - the reporting device holds values from -1016 to 11,709,098 beside real ones. Gating to 50...110 keeps every genuine reading and drops the mis-scaled ones, so one legacy outlier cannot flatten the whole y-axis. It is a DISPLAY gate on a metric that is never scored, changes no stored row, and mirrors the idiom already guarding temp (20-45 C) and HR (0-300 bpm) at their decoders. The bounds are deliberately permissive rather than a cosmetic clamp: the channel is uncalibrated and genuinely overshoots 100 (1,948 of 5,562 in-range rows sit at 101-110 and track the rest of the series), so clamping at 100 would silently flatten real data. The two-channel ratio is NOT gated - a ratio has no comparable physiological range - so WHOOP output stays byte-identical. Effect on the reporting device: 0 rows plotted before, 5,562 after, averaging 96.9%. Apple-only: the Deep Timeline has no Android twin (Android surfaces SpO2 through HealthVitalsLogic, which reads the daily rollup, not this per-sample path), so there is no parity change to make. Tested: 4 new DeepTimelineFacadeTests pinning the two-channel ratio (unchanged), the single-channel reading, the legacy-row drop, and that the gate never touches the ratio path; DeepTimelineFacadeTests 13/13; macOS Strand builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfZdZnmEmYya6AgzYwH9Xh
…ft+Kotlin) Blood Oxygen tile falls back to a device-conditional, opt-in nightly SpO2 estimate when no calibrated spo2Pct exists: WHOOP keeps its existing @82 V18Aux candidate, an Oura owner gets a new ceiling@100 mean of the ring's own 0x6F reading (min(sample,100) applied per-sample before averaging, gated to the existing contamination-exclusion range so a mis-scaled dc_raw row can't drag the mean down). Ceiling@100 is queue 11a's starting transform per OURA_PROTOCOL.md §6.5.0's Oura-app-groundtruth tally (3/3 full-tier nights vs raw's 1/3 and the offset+clamp fit's 2/3). Reuses the WHOOP spo2_candidate_82 precedent exactly: opt-in, default-off, labelled "strap estimate (unverified)", written to a metricSeries sidecar key, never to spo2Pct, never scored. Adds AnalyticsEngine.nightlySpo2CeilingMean (Swift + Kotlin twin, 6 tests each) and wires it into both platforms' IntelligenceEngine, UI candidate-fallback paths, and the shared toggle's copy. LiquidTodayView's Blood Oxygen tile previously read spo2Pct only, with no candidate fallback at all; it now has one via a new optional caption param on its shared tile helper. Verified: swift test (1519/1519), Android com.noop.analytics.* suite, xcodebuild for the macOS app target (no default CI covers app-target Swift), gradlew compileFullDebugKotlin, and the i18n gate (clean except one pre-existing, unrelated pl-locale gap). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBBrN3NH9uQ63XN3tgkvkq
…nly install The toggle (spo2CandidateDisplayEnabled) has covered Oura since 89c8533 (label reads "Blood Oxygen: strap estimate (WHOOP 5/MG, Oura)"), but stayed nested inside fiveMGCard, gated by showFiveMGControls (selectedWhoopModelRaw == whoop5mg). An install that has never had a real WHOOP 5/MG connect — WHOOP 4.0 + Oura paired here — never sees that section at all, so the toggle could never be reached. metricSeries confirmed zero spo2_candidate rows ever written despite pass-2 scoring running daily, and a full screenshot sweep of Settings confirmed the section never renders. Splits the toggle into its own spo2CandidateCard, shown for a 5/MG OR an active Oura device (mirrors rawSensorDiagnosticsCard's existing #22 split for controls that outgrew the 5/MG-only card). No logic change: same @AppStorage key, same copy, same re-score-on-flip behavior, just relocated. Checked Android first: it does not have this bug — the Kotlin toggle (SettingsScreen.kt) already sits outside its own showFiveMGControls gate, inside the always-visible Diagnostics card. iOS/macOS-only fix. Verified: Strand (macOS) and NOOPiOS (iOS Simulator) both build clean (xcodebuild ... build — no default CI covers app-target Swift), i18n gate clean except the pre-existing, unrelated pl-locale gap. Related, not fixed here: stressPersonalBaselineEnabled (#463) is also still inside fiveMGCard despite not being WHOOP-5/MG-specific — same class of bug, left for a separate change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TEX9vyGzsBECX7ECkDL1rg
Integration fix for @pipiche38's #1568, rebased onto current main. Their branch passed CI and so did main, but the MERGE of the two failed: analyzeRecentOnCpu instrumented Code length 61565 exceeds the 61535-byte budget Thirty bytes. `analyzeRecentOnCpu` sits within ~100 bytes of the JVM's 64 KB per-method ceiling, and their device-conditional SpO2 candidate branch plus the trace recorders from #1575 both add to it. Neither exceeded the budget alone — only together, which is invisible to either PR's own CI. #1568's checks were also ~10 hours stale by then, predating all six commits main had gained, and GitHub reported the branch CLEAN because the conflict is not textual. Their 14-line branch becomes a call to a new private `spo2CandidateMean`. Behaviour is unchanged: same two paths, same `?.first`, same null cases, still gated by the caller's toggle, still never written to `spo2Pct` and never scored. Also worth recording for them: the "2 pre-existing failures in RecoveryDriversTest" in the PR description do not reproduce. That class is 10/10 on main and 10/10 on their own head, and their full suite is 4270/0 rather than the 2404/2 reported — a stale local Gradle/KSP cache, which this repo has a documented history of. Nothing was broken. Integrated tree: 4283 tests, 0 failures. Release lint, doc-comments and i18n clean.
This was referenced Aug 24, 2026
ryanbr
added a commit
that referenced
this pull request
Aug 24, 2026
Post-merge parity fix for #1584 (@pipiche38's #1568), found while re-reviewing the merged state. The Oura SpO2 candidate branch resolved "is this owner an Oura device" differently on each platform: Kotlin DeviceBrandCatalog.isOura(owner) -> device-id PREFIX Swift regDevices.first { $0.id == owner }?.brand == "Oura" -> registry BRAND string Not stylistic. The two keys can disagree, and when they do it is an analytics split: a device with an `oura-` id but no registry row, or a row whose brand text differs, was Oura on Android and not on iOS — so Android computed the ceiling@100 candidate for that night and iOS did not. A byte-identical Swift twin already existed and is what Repository.activeDeviceIsOura uses, with a comment saying it is there "rather than an ad-hoc 'oura' literal". Swift now uses it too. The id-prefix key is the safer one, not just the consistent one: Repository.swift:178 records that the registry mints every non-WHOOP id as "<idPrefix>-<uuid>", so the prefix IS the brand key and is always present, while the brand compare needs a registry row to exist AND its text to match. Verified Oura ids really are minted that way before changing anything — SourceCoordinator.swift:453 builds "\(ExperimentalBrand.oura.idPrefix)-\(serial)" — because otherwise this would have silently disabled the Oura path it exists to protect. WHOOP owners unaffected: "my-whoop" and "my-whoop-noop" fail the prefix test exactly as they failed the brand test, so they still take the v18 aux branch. Checked and left alone: skinTempWornToleranceSec keys on the brand string on BOTH platforms (Swift :2363, Kotlin :77), so that pair is symmetric — a different convention from the catalog, not a divergence. It predates #1568. Kotlin untouched; it already used the catalog. Verified: app-build green on macOS and iOS, doc-comments clean. No strings, no schema. Not seen on a device — @pipiche38's owed capture with the toggle flipped on now exercises this resolver too.
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
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.
@pipiche38's #1568, rebased onto current main with one integration fix on top. Their three commits
are preserved with their authorship — this is a merge of their work, not a replacement of it.
Why it needed rebasing rather than merging
#1568 showed green and
MERGEABLE / CLEAN, but the green was ~10 hours stale — every checkcompleted at
2026-08-23T14:0x, and main had since gained six commits starting2026-08-24T00:27Z.GitHub does not re-run checks when the base advances, and it reported CLEAN because the conflict was
never textual.
Merging the two locally is what surfaced it:
mainaloneThirty bytes.
analyzeRecentOnCpusits within ~100 bytes of the JVM's 64 KB per-method ceiling.Their device-conditional SpO2 candidate branch and the trace recorders from #1575 both add to it;
neither exceeds the budget alone. No single PR's CI can see this.
The fix
Their 14-line branch becomes a call to a new private
spo2CandidateMean. Behaviour unchanged —same two paths, same
?.first, same null cases, still gated by the caller's toggle, still neverwritten to
spo2Pctand never scored.Same remedy as #1575, which hit this ceiling from the other side.
A note for @pipiche38
The "2 pre-existing failures in
RecoveryDriversTest" in the PR description do not reproduce.That class is 10/10 on main and 10/10 on your own head, and your full suite is 4270/0 rather than the
2404/2 reported. That looks like a stale local Gradle/KSP cache — a documented trap in this repo, which
is why
--no-build-cache --rerun-tasksmatters here. Nothing was broken, and main was fine.What the PR itself gets right
metricSeriessidecar, never into
spo2Pct, never scored — reusing thespo2_candidate_82precedent rather thaninventing a pattern.
gate justified against real contamination ranges.
ceiling@100rests on n=3 nights on one subject — which is precisely why default-OFF and unvalidatedis the correct landing, and why it should not graduate to a default on that evidence.
Verification
local Xcode builds were pre-merge.