Repository navigation
health: say when skin temp could not honour the Settings choice (#1847) - #1847
Merged
Merged
Conversation
Setting "Skin temperature" to Temperature and getting the same "0.0 Δ°C" back is
indistinguishable from a broken toggle. It is not broken: `leadReading` falls
back rather than blanking, so a night that only ever recorded a deviation shows
its deviation. That is the right behaviour and it was completely invisible.
The detail screen now says so under the reading:
No measured temperature for these nights — showing the difference from your
baseline. A sync re-scores recent nights and fills it in.
Fires in exactly one case — a temperature was asked for and no night in the
window has one. The reverse fallback stays silent on purpose, and a test pins
that asymmetry: a temperature shown under a plain °C unit is self-describing,
whereas a Δ where a temperature was requested is not.
The decision is a pure `shouldExplainSkinTempFallback`, so it is tested without
Compose.
Why those nights have no temperature, since it is not obvious: the deviation is
DERIVED from the absolute —
val skinTempDevC: Double? = nightlySkinTempC?.let { ... }
— so every night showing a delta had a real temperature at scoring time. It was
computed, used, and discarded, because those nights were scored before #1663
added the column on 2026-08-27. A scoring pass re-derives it from the raw
skinTempSample rows still on disk, which is what the note points at.
Android only: the note lives on the vital-detail screen. Apple's twin
(MetricExplorerView) sources skin temp from `Repository.dailyColumn`, which
returns skinTempDevC for every caller including Trends — repointing that is a
different change with its own design questions, filed separately rather than
folded in here.
Full suite 5117/0 (4 new). Note translated across all 6 locales. Gates clean.
The first note only covered the all-or-nothing case: temperature asked for, no night has one. The re-review found the partial case, which is the one this install is actually heading for. Once a sync refills the 21-night analyzeRecent window, the newest night HAS an absolute, so the screen leads with it — and the series must then read the absolute column, because an absolute plotted against a history of deviations is arithmetic on two scales. Every older deviation-only night silently drops out and the reading count falls, with nothing on screen saying why. History looking like it vanished is a worse outcome than the delta it replaced. A second sentence covers it, chosen in the same `fallbackNote` slot so the screen never shows two competing explanations. Pure `shouldExplainShortenedSkinTempSeries`, true only when rows were actually dropped, so a complete series stays silent and an empty one is left to the empty state. Also hoisted the readings list to a local, so the model and the dropped-row count read the SAME list — computing the count from a second expression is how the two drift and the note starts lying. Full suite 5120/0 (3 new). Second string translated across all 6 locales. Gates clean.
…ened Two ways the explanatory copy could lie about the user's own data. Both are worse than the silence they were added to fix. 1. The shortened-series note had no `leadsAbsolute` guard, so it also fired on the DEVIATION-led branch. That branch drops rows too — calibrating nights that carry only an absolute, and the #622 bimodal partition — but they are the OPPOSITE kind, so "Only nights with a measured temperature are shown. Earlier nights recorded a baseline difference only." was precisely backwards. Reachable the moment a wearer picks "vs baseline" with one calibrating night in the window, which is not an exotic state. Gated inside the rule rather than at the call site: the function name promises the whole rule, and a caller-side guard is one refactor away from being dropped. 2. The fallback note fired whenever the NEWEST night lacked an absolute, but its sentence — "No measured temperature for these nights" — is false if an older night has one. It now requires that none in the window does. That case shows nothing instead. Saying nothing is a gap; saying something untrue about the user's own readings is a defect. Both pinned by tests written from the failing direction, so a future simplification that drops either guard fails in CI rather than in someone's screenshot. Existing cases repinned to the new signatures — same intent, same assertions. Full suite 5122/0 (9 in this file). Gates clean.
…lds it Reading the whole diff cold found the same lying-note class one level deeper, plus real readings being hidden. A WHOOP CSV import writes absolute °C straight into skinTempDevC — the #622 bimodal field, `row.double("skin_temp_celsius", ...)` — so an absolute-led series that read skinTempC alone: - dropped every imported night from a TEMPERATURE chart, though those nights hold temperatures on exactly the scale being plotted; and - then counted them as dropped, so the shortened-series note called real measured temperatures "a baseline difference only". The absolute branch now takes `skinTempC ?: skinTempDevC.takeIf(isAbsoluteSkinTemp)`. Mixing is only unsound ACROSS scales, and these are the same scale — the #1705 dominantKind reasoning, applied in the direction that keeps data rather than discarding it. Genuine deviations are then the only dropped rows, so the note is accurate again. A test pins the 20.0 discriminator directly (34.0 and the inclusive boundary are absolute; 0.5 and -0.6 are not), so moving that threshold surfaces here rather than as a mislabelled chart. Full suite 5122/0, 11 in this file. Gates clean.
ryanbr
added a commit
that referenced
this pull request
Sep 3, 2026
Carries #1847: the skin-temp detail screen now explains when it could not honour the Settings choice, and an absolute-led series keeps imported temperatures instead of dropping them. MARKETING_VERSION and versionName stay at 11.1.1.
This was referenced Sep 3, 2026
9 tasks
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.
Setting Skin temperature to Temperature and getting the same
0.0 Δ°Cback is indistinguishable from a broken toggle.It isn't broken.
leadReadingfalls back rather than blanking, so a night that only ever recorded a deviation shows its deviation — right behaviour, completely invisible. The detail screen now says so under the reading:It fires in exactly one case: a temperature was asked for and no night in the window has one. The reverse fallback stays silent on purpose, and a test pins that asymmetry — a temperature under a plain
°Cunit is self-describing, whereas aΔwhere a temperature was requested is not.The decision is a pure
shouldExplainSkinTempFallback, so it is tested without Compose.Why those nights have no temperature
Worth stating, because it is not obvious and it is what makes the note honest — the deviation is derived from the absolute:
So every night showing a delta had a real temperature at scoring time. It was computed, used to derive the delta, and discarded, because those nights were scored before #1663 added the column on 2026-08-27. A scoring pass re-derives it from the raw
skinTempSamplerows still on disk —analyzeRecentre-scores a 21-night window per post-sync pass — which is exactly what the note points the user at.Scope
Android only, and deliberately. The note lives on the vital-detail screen; Apple's twin is
MetricExplorerView, which sources skin temp throughRepository.dailyColumn— a shared key→column mapper returningskinTempDevCto every caller including Trends. Repointing that changes which series the chart plots and how the band behaves, so it is filed separately rather than folded in here.This also corrects #1845's body, which claimed no Swift twin of the detail screen exists.
MetricExplorerViewis the twin; I checked for the wrong marker string.Verification
Full suite 5117 / 0 (4 new). Note translated across all 6 Android locales. Doc-comment and i18n gates clean.