Repository navigation
feat(oura): add Skin Temp as a Key Metrics tile option - #1678
Merged
ryanbr merged 3 commits intoAug 27, 2026
Merged
Conversation
Skin Temp was already a "Your Cards" (DashboardCard) option on both platforms, but was never offered as a small "Key Metrics" grid tile - found 2026-08-24 while diagnosing queue 11c's "visible on Deep Timeline, missing from Cards" report: the two "Cards" surfaces turned out to be different screens, and Skin Temp's gap was specifically in the KeyMetric grid (the small tile row), not a display-path bug. New KeyMetric.skinTemp / KeyMetric.SKIN_TEMP case on both platforms, NOT added to defaultOrder - an existing user's saved layout, and a fresh install's default, stays byte-identical to before; the tile only appears once opted into via the layout editor. Reuses the sibling DashboardCard.skinTemp/.SKIN_TEMP tile's existing title string, icon, and value-formatting (SkinTempDisplay, same bimodal absolute-vs-deviation handling, ryanbr#622) and detail-screen route (the Android tap-through reuses the "skin" vital_detail key dashboardCardMetricKey(DashboardCard.SKIN_TEMP) already routes to, a confirmed-working destination, not a new/unverified one) - no new patterns invented, no new i18n debt. LiquidTodayView had no temperature-unit resolution at all before this (added the same @AppStorage + resolveTemperature pattern TodayView/ MetricExplorerView already use). Android's Window struct gained a skinTemp trend field for the tile's sparkline, matching the LiquidTodayView sparks["skin_temp"] added the same way. 4 new tests (2 per platform: the tile-value round-trip pin + KeyMetric raw-token/defaultOrder-exclusion contract). Verified: xcodebuild -scheme Strand build + test (macOS, 1244/1244) and -scheme NOOPiOS build (both app-target Swift - CI doesn't cover this); Android compileFullDebugKotlin clean, com.noop.ui.* + com.noop.analytics.* 2421/2426 (5 known-preexisting, unrelated failures - RecoveryDriversTest's half-tie-rounding issue and TodayExplainabilityTest's 3 failures, both already confirmed present on pristine upstream/main before any of this session's changes, not chased); doc_comment_lint.py and i18n_audit.py --ci both clean.
…carry (Android) carriedDay (lastScoredRecoveryDay) is a whole-row carry that lands on rows with a null skinTempDevC, so the Key Metrics tile fell through to a blank value on a night whose recovery-scored row happened not to carry a skin-temp reading. Mirrors the existing spo2CarryDay/respCarryDay per-field fallbacks and iOS TodayView's lastSkinTempDay chain. Pulled the shared resolveSkinTempDevC(d, carriedDay, skinTempCarryDay) helper out above MetricGrid so the fallback chain is independently testable without Compose/Robolectric, and so it sits ahead of MetricGrid's own doc comment per doc_comment_lint.
pipiche38
marked this pull request as ready for review
August 27, 2026 08:04
…#1654 ryanbr#1654 (merged 2026-08-25, after this branch was opened) added the same @AppStorage-backed unitSystem/temperatureUnit block to LiquidTodayView for the "Your Cards" Skin Temp tile. This PR's Key Metrics tile added an identical block, causing invalid-redeclaration build errors in both Strand (macOS) and NOOPiOS once rebased onto current main. The Key Metrics `case .skinTemp` reuses the existing properties as-is. Per ryanbr's review on ryanbr#1589 (github.com/ryanbr/pull/1589#issuecomment-5436238919).
ryanbr
added a commit
that referenced
this pull request
Aug 27, 2026
The doc on KeyMetricSkinTempTest justified pinning the "skinTemp" raw token "so a backup/restore reads the same layout on either OS". That is not what happens. today.keyMetrics is not in the .noopbak whitelist at all; today.hostedCards is the one layout pref carried across, which BackupSettings.swift says in as many words. So a restore on the other platform reads that platform's own saved layout and never sees this token. The assertion itself is right and stays: the token IS the persisted form, and two independent implementations must not drift on how they spell one. It would also become load-bearing the day that pref is whitelisted. Only the stated reason was wrong, and a wrong reason sitting in a test is what someone reasons from later. The Swift twin (KeyMetricPrefsDecodeTests) never made the claim, so this is one stray sentence rather than a parity gap between the two suites. Comment only, no assertion touched. Ran the class rather than assuming it: 7 tests, 0 failures, with the stale result XMLs cleared first so the count belongs to this run. Follow-up to #1678, spotted at review time and left out of the merge.
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.
(replace the closed #1589 1589)
What this PR does
Skin Temp was already a "Your Cards" (
DashboardCard) option on both platforms, but was never offeredas a small "Key Metrics" grid tile.
Found 2026-08-24 while diagnosing a "Skin Temp visible on the Deep Timeline, missing from the Cards"
report: it turned out there are two different "Cards" screens in this app, and Blood Oxygen / Skin
Temp each work on one and fail on the other. The Key Metrics grid (
KeyMetricenum) simply neverhad a
skinTempcase, on either platform — not a display-path bug, just a tile that was never added.(The mirror-image, genuinely-real bug this same investigation found — Blood Oxygen's "Your Cards"
drill-down having no candidate fallback — is a separate PR:
fix/oura-spo2-cards-explorer-candidate-fallback.)Re-opens #1589, which was closed unmerged. Same change, rebuilt on current
mainas two clean commitsand re-verified; no functional changes from the original.
The fix
New
KeyMetric.skinTemp/KeyMetric.SKIN_TEMPcase on both platforms. Not added todefaultOrder— an existing user's saved layout, and a fresh install's default, staysbyte-identical to before; the tile only appears once opted into via the layout editor.
Reuses the sibling
DashboardCard.skinTemp/.SKIN_TEMPtile's existing title string ("Skin Temp"),icon (
thermometer.medium/Icons.Filled.Thermostat), and value formatting (SkinTempDisplay, thesame bimodal absolute-vs-deviation handling from #622) — no new copy, no new i18n debt. The Android
tap-through reuses the
"skin"vital_detailkeydashboardCardMetricKey(DashboardCard.SKIN_TEMP)already routes to — a confirmed-working destination, not a new/unverified one.
LiquidTodayViewhad no temperature-unit resolution at all before this (added the same@AppStorageresolveTemperaturepatternTodayView/MetricExplorerViewalready use). Android'sWindowstruct gained a
skinTemptrend field for the tile's sparkline, matching the iOSsparks["skin_temp"]added the same way.Per ryanbr's review on #1589,
TodayScreen.kt's Android tile also gained a PER-FIELD skin-tempcarry (
skinTempCarryDay):carriedDay(lastScoredRecoveryDay) is a whole-row carry that lands onrows with a null
skinTempDevC, so without this the Key Metrics tile fell through to a blank value ona night whose recovery-scored row happened not to carry a skin-temp reading. Mirrors the existing
spo2CarryDay/respCarryDayper-field fallbacks and iOSTodayView'slastSkinTempDaychain. Pulledthe shared
resolveSkinTempDevC(d, carriedDay, skinTempCarryDay)helper out so the fallback chain isindependently testable without Compose/Robolectric.
Tests
4 new tests (2 per platform): the raw-token round-trip +
defaultOrder-exclusion contract for the newpersisted enum case (schema-relevant —
KeyMetricrawValues are thetoday.keyMetricslayoutstring), plus the Android
resolveSkinTempDevCcarry-fallback tests added in response to review.Verification (2026-08-27, on top of upstream/main @ 9bdad2d)
upstream/main(the original 3-commit history — feature,review-fix, and a doc-lint reorder fixup — is squashed here to feature + review-fix). Resolving the
original rebase surfaced one conflict in
TodayScreen.kt(unrelated main-branchspo2CandidateByDayparam landed on the same
MetricGridcall site/signature as this PR'sskinTempCarryDay), resolvedas a straight union of both params — nothing dropped from either side; the resulting tree here is
byte-identical to that resolution.
compileFullDebugKotlinclean;com.noop.ui.*all green (JAVA_TOOL_OPTIONS=-Duser.language=ento avoid the pre-existing fr_FR-locale-only failures noted in prior sessions).
Tools/doc_comment_lint.pyandTools/i18n_audit.py --ci upstream/mainboth clean.xcodebuild -scheme Strand build): not verified this session — blocked by a pre-existinglocal environment issue unrelated to this diff (an explicit-module-cache race in the
cmark-gfmthird-party dependency;
_DarwinFoundation2*.pcm not foundunder parallelCompileC). Not chasedhere; same env issue noted against a prior PR's macOS build. iOS build also not verified this
session.