Skip to content

today: lead skin temp with the measured temperature, with a Settings choice (#1844, #1846) - #1845

Merged
ryanbr merged 5 commits into
mainfrom
fix/1844-skin-temp-absolute-leads
Sep 3, 2026
Merged

ryanbr merged 5 commits into
mainfrom
fix/1844-skin-temp-absolute-leads

Conversation

@ryanbr

@ryanbr ryanbr commented Sep 3, 2026 •

Copy link
Copy Markdown
Owner

Two changes to how skin temperature reads. Reported on #1842 → #1844, with the Settings choice as #1846.

Lead with the temperature, not the delta

The Today card and the detail screen showed 0.0 Δ°C — a deviation against a baseline neither screen names. The Health tile already solved this, and the rule is the whole argument:

A deviation with no anchor cannot be read — "+0.9" is a fever or a warm bedroom and nothing on the tile says which — so the absolute leads and the deviation becomes the context beneath it.

#1663 stored the nightly absolute (dailyMetric.skinTempC); #1665 applied it to the Health tile. Three surfaces never came onto that pattern and still read skinTempDevC only — Today's "Your Cards" row, the Key Metrics tile, and the HealthScreen detail (title, unit, chart and every readings row). Apple's two Today layouts had the same gap.

TodayView.dashboardValue was worse than the Android card that was reported: .hrv, .restingHr and .skinTemp had no carry at all while their neighbours carried, so a straight port of the Android change would have left it broken.

Δ°C is not the defect and stays wherever a deviation is genuinely what is shown (#622 added it so -0.1 °C is not read as a broken thermometer).

The carry had to change with it

lastSkinTempRow selects on skinTempDevC, so it walks past a CALIBRATING night: the deviation is null until the baseline is usable (~4 nights), while the absolute is measured from night one. Those wearers saw an empty card with a real temperature sitting behind it.

lastSkinTempReadingRow takes a row holding EITHER number. That OR predicate is deliberate and is not the #1842 defect, and both selectors say so in their docs: #1842 read field X off a row chosen on (X or Y), so a row holding only Y blanked X. Here the value is "whichever of the two this night has", and the caller reads both fields off that row — so an absolute is never paired with another night's deviation.

The setting (#1846)

Absolute is the right default but not the right answer for everyone — the ±baseline move is the more sensitive illness signal. Settings → Units → Skin temperature: Temperature (default) or "vs baseline", both platforms.

It is a PREFERENCE, not a filter: leadReading gains prefer, which picks the number tried FIRST; the other stays the fallback, so a choice can never blank a card whose night measured only the other. The unit always names the scale ACTUALLY shown, never the one asked for.

Wired at the call sites, not defaulted inside — dashboardCardValue, buildVitalDetail and BodyVitalSigns.readings take the resolved preference the way they already take tempUnit, and on the detail screen it is also a remember key so flipping it rebuilds rather than serving a cached model.

Caught before shipping: Kotlin persisted Kind.name ("DEVIATION") while Swift persisted rawValue ("deviation") under the same units.skinTempDisplay key. Kotlin's Kind gains a raw matching Swift byte for byte plus fromRaw, with a test pinning the tokens — the drift KeyMetricSkinTempTest exists to catch.

What the re-review changed

  • The Health tile ignored the setting on both platforms; it now goes through the same leadReading rule. A setting reaching two of three surfaces reads as broken rather than absent.
  • Six latestVital(...) calls took the new parameter's default; zero passed it.
  • That row's predicate was skinTempDevC != null, so it walked past a calibrating night — the same bug as the carry, one layer down.
  • units.skinTempDisplay was whitelisted for backup but export and import are explicit per key, so it would never have restored. Both halves wired, "" = key-absent like units.temperature.
  • resolveSkinTempDevC had no caller left; deleted, and its four carry-order cases repinned onto the new resolver.
  • The two Kotlin Today surfaces did not agree while the iOS twin's comment claimed they could never disagree — Your Cards read vd (carriedDay ?: day), so a live carry hid a temperature measured today, which todaysOwnReadingWinsOverEitherCarry has asserted against all along.

Verification

  • Kotlin: full suite 5113 / 0, 19 new cases.
  • Swift: WhoopStore XCTest for the new selector. StrandAnalytics will not build on Linux (GRDB's CSQLite), so leadReading/formatReading were run by extracting them verbatim into an isolated harness against the Kotlin's own fixtures — all pass, including that Today: HRV and Resting HR cards read "No data" while the tile above shows a value #1842's signed zero still holds.
  • i18n and doc-comment gates clean. One new Android string in all 6 locales; one Apple a11y label spliced across all 9 (58 insertions, 0 deletions — no JSON round-trip). Segment labels reuse existing translated keys.

Note for testing

skinTempC landed 2026-08-27 and refills on the next scoring pass, re-derived from raw skinTempSample rows still on disk — analyzeRecent re-scores a 21-night window per post-sync pass. On an install whose newest reading predates 27 Aug this is correctly invisible until then, and nights whose raw samples were already pruned stay deviation-only for good.

…1844)

The Today card and the detail screen still showed "0.0 Δ°C" — a deviation
against a baseline neither screen names. A temperature is what a temperature
reading should say.

The Health tile already solved this and the rule is worth quoting, because it is
the whole argument:

    A deviation with no anchor cannot be read — "+0.9" is a fever or a warm
    bedroom and nothing on the tile says which — so the absolute leads and the
    deviation becomes the context beneath it.

#1663 stored the nightly absolute (dailyMetric.skinTempC) and #1665 applied that
to the Health tile. Three surfaces never came onto it and still read
skinTempDevC only: the Today "Your Cards" row, the Key Metrics tile, and the
HealthScreen detail (whose title, unit, chart and every readings row all keyed
off the deviation series). Apple's two Today surfaces had the same gap on both
the classic and Liquid layouts.

`leadReading` makes the rule pure and shared; `formatReading` keeps the number
and its unit on the same scale, which matters because °F converts an absolute
with the full formula and a deviation by the factor alone.

Δ°C is NOT the defect and stays wherever a deviation is genuinely what is shown
(#622 added it so "-0.1 °C" is not read as a broken thermometer). The defect was
leading with a deviation on a night that measured a real temperature.

The carry had to change with it. lastSkinTempRow selects on skinTempDevC, so it
walks straight past a CALIBRATING night — deviation nil until the baseline is
usable (~4 nights), absolute measured from night one. Those wearers were seeing
an empty card with a real temperature sitting behind it. lastSkinTempReadingRow
takes a row with EITHER number.

That OR predicate is deliberate and is NOT the #1842 defect I just fixed; both
selectors say so in their docs. #1842 read field X off a row chosen on (X or Y),
so a row holding only Y blanked X. Here the value IS "whichever of the two this
night has", and the caller reads both fields off THAT row — so the chosen row
always supplies the number shown, and an absolute is never paired with another
night's deviation.

The detail screen's series follows the same choice: an absolute plotted against
a history of deviations is arithmetic on two scales.

Kotlin: full suite 5108/0 (10 new). Swift: WhoopStore XCTest added; StrandAnalytics
will not build on Linux (GRDB CSQLite), so leadReading/formatReading were also run
here by extracting them verbatim into an isolated harness against the Kotlin's own
fixtures — all pass. i18n and doc-comment gates clean; no new strings.
…ad resolver

Three things the re-review found in my own change.

1. `resolveSkinTempDevC` had no production caller left once both Today surfaces
   moved onto the reading resolver, and the doc I wrote said it "stays for the
   deviation-only call sites" — which was simply untrue; there were none. Deleted
   it and repinned its four carry-order cases onto `resolveSkinTempReading`: same
   fixtures, same ORDER, reading `.value` off the resolved reading. The order is
   the part worth guarding and it did not change. Deleting the function and
   keeping the test that way is the opposite of deleting the test to keep a
   function nothing calls.

2. The two Kotlin surfaces did not agree, and the iOS twin's comment has claimed
   for a while that they "can never disagree". Your Cards read `vd`, which is
   `carriedDay ?: day`, while the Key Metrics tile read today-first. Both now go
   through `resolveSkinTempReading`, so the claim holds on both platforms.

3. That also fixes an order bug the existing tests already asserted against:
   `carriedDay ?: day` means a live carry HID a temperature measured today, while
   `todaysOwnReadingWinsOverEitherCarry` has pinned today-first all along. The
   test passed because it only ever exercised the extracted resolver, never the
   Your Cards chain.

Verified the two variable scopes the substitutions relied on rather than assuming
them: Swift `keyMetricTile` opens with `let d = displayDay`, so routing it through
`skinTempLeadReading` is equivalent; Kotlin `dashboardCardValue` computes
`val vd = carriedDay ?: day`, which is what finding 3 is about.

Full suite 5108/0. Doc-comment and i18n gates clean.
app-build: "ambiguous use of 'skinTempCardValue(_:fahrenheit:)'" at
StrandTests/TodayVitalCardTests.swift:44. I added an OVERLOAD taking
`SkinTempDisplay.Reading?` beside the existing `Double?` one, and a call passing
`nil` cannot choose between two optional parameter types.

The new one now takes an argument LABEL — `skinTempCardValue(reading:fahrenheit:)`
— which removes the ambiguity and, unlike an overload, is greppable as its own
name. Twin-name overloads are how a Swift/Kotlin parity audit loses a call site.

Compile-only; the Double? version and its test are untouched.
…seline delta (#1846)

#1844 made the cards lead with the measured temperature. That is the right
default, but it is not the right answer for everyone: the ±baseline move is the
more sensitive illness signal, and someone tracking it should not lose it.

Settings > Units, beside the °C/°F row: "Skin temperature" — Temperature (default)
or "vs baseline".

It is a PREFERENCE, not a filter. `leadReading` gains a `prefer` argument that
picks which number is tried FIRST; the other stays the fallback, so choosing one
can never blank a card whose night measured only the other. The unit always names
the scale ACTUALLY shown, never the one asked for — otherwise a fallback would
print a temperature under a Δ label, or a delta as a wrist temperature. Tested
both directions.

Wired at the CALL SITES, not defaulted inside. `dashboardCardValue` and
`buildVitalDetail` take the resolved preference the way they already take
tempUnit, because a defaulted parameter no caller passes is a setting that looks
wired and does nothing — which I have now shipped enough times this session to
stop trusting the pattern. On the detail screen the preference is also a
`remember` KEY, so flipping it rebuilds the model instead of serving a cached one.

Caught before it shipped: Kotlin was persisting `Kind.name` ("DEVIATION") while
Swift persisted `rawValue` ("deviation") under the SAME `units.skinTempDisplay`
key. Kotlin's Kind gains a `raw` matching Swift byte for byte, plus `fromRaw`,
the pattern `TemperatureUnit`/`KeyMetric` already use — and a test pins the
tokens, since a stored token that two implementations spell differently is
exactly the drift `KeyMetricSkinTempTest` was written to catch.

i18n: one new Android string in all 6 locales; the two segment labels reuse
existing translated keys. One new Apple a11y label text-spliced into the catalog
across all 9 locales (58 insertions, 0 deletions — never a JSON round-trip).

Kotlin: full suite 5113/0 (5 new). Gates clean.
… and survives a restore

Four findings, all the same shape: things that looked wired and were not.

1. The HEALTH TILE ignored the setting entirely, on BOTH platforms. Android's
   `skinTempLeadsWithAbsolute` was `absC != null` and Swift's was
   `skinAbsRow ?? skinRowDeviation`, each unconditional. So picking "vs baseline"
   changed Today and the detail screen and left the Health tile showing a
   temperature. Both now go through the one `leadReading` rule, threaded from the
   call site the way tempUnit already is. A setting that reaches two of three
   surfaces is worse than no setting: it looks broken rather than absent.

2. Six `latestVital(...)` calls took the new parameter's DEFAULT — zero passed it.
   Only the "skin" row reads it, and that one now does explicitly.

3. That same "skin" row's predicate was `it.skinTempDevC != null`, so it walked
   past a CALIBRATING night exactly like `lastSkinTempRow` did — measured absolute,
   no deviation yet. Now takes either number, same reasoning as
   `lastSkinTempReadingRow`.

4. BACKUP. I added `units.skinTempDisplay` to the whitelist and nearly stopped
   there — but export and import are both EXPLICIT per key in this file, so a
   whitelisted-but-unwired pref silently never restores. Both halves are wired,
   with the same "" = key-absent convention `units.temperature` uses, so an older
   backup without the key restores to the default rather than to nothing.

Also verified rather than assumed: both `@AppStorage` declarations landed in the
struct that actually reads them (TodayView / LiquidTodayView / VitalsSection), and
the new `latestVital` parameter sits BEFORE the trailing `hasValue` lambda — put
after it, every trailing-lambda call site silently rebinds and stops compiling.

Full suite 5113/0. Doc-comment and i18n gates clean.
@ryanbr ryanbr changed the title today: lead skin temp with the measured temperature, not the delta (#1844) today: lead skin temp with the measured temperature, with a Settings choice (#1844, #1846) Sep 3, 2026
@ryanbr
ryanbr merged commit 28d57b1 into main Sep 3, 2026
17 checks passed
@ryanbr
ryanbr deleted the fix/1844-skin-temp-absolute-leads branch September 3, 2026 07:18
ryanbr added a commit that referenced this pull request Sep 3, 2026
Carries the skin-temp work from #1845: the cards lead with the measured
temperature, and Settings > Units > Skin temperature switches between that and
the baseline delta. MARKETING_VERSION and versionName stay at 11.1.1 — a build
bump, not a version bump.
@ryanbr

ryanbr commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

Correction to this PR's body: it states that no Swift twin of the Android vital-detail screen exists, so the detail-screen change was Android-only.

That is wrong. MetricExplorerView is the twin — same "as of <date>" hero, "N readings" caption and readings table. I searched for a marker string that only Android has and concluded from its absence.

The consequence is a real parity gap, not just a documentation slip: Apple's explorer sources skin temp via Repository.dailyColumn → skinTempDevC, so it still leads with the deviation and ignores the #1846 setting. Filed separately, with why it needs its own branch rather than a change to the shared mapper (dailyColumn also feeds Trends and the catalog availability check).

Nothing merged here is wrong — the Android and Today-surface changes stand. The gap is what #1845 did not do.

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.

1 participant