fix(import): store WHOOP's absolute skin temperature as skinTempC, not as the deviation

[Iskrata]

What this PR does

The WHOOP export ships absolute skin temperature (°C), but WhoopImporter wrote it into skinTempDevC, the deviation column, with a NOTE acknowledging the mismatch. Screens that check the magnitude cope with this (VitalBands.isAbsoluteSkinTemp), but callers that trust the column don't:

• the Charge breakdown sheet passes row.skinTempDevC into the drivers (ChargeBreakdownWiring.swift:49), so an imported day's "why" showed a skin-temp driver of about −33;
• the Trends report charts it as a deviation;
• the Coach's context averages the column over 30 days (AICoach.swift:1215).

So an imported night reads as a +33 °C deviation. On one install the imported rows averaged 33.5 in that column, against ~0.0 for NOOP-computed nights.

What it does not reach is the stored Charge score. Imported rows keep the export's own recovery. recomputeRecovery / recomputeChargeDrivers (IntelligenceEngine.swift:3094, :3112, :3131) run only for the nights NOOP scores itself (recomputeRecoveryDaily at :2065, drivers at :2080, both inside the scoredNights loop). There the deviation is recomputed from nightly skin samples against an on-device-only baseline (:1875).

This PR fixes it in three parts:

• Import: the absolute goes into skinTempC. skinTempDevC becomes that absolute minus the personal baseline folded over the preceding nights (Baselines.metricCfg["skin_temp"], the absolute-°C config). It is rounded to 0.01 °C like the on-device deviation (recomputeSkinTempDev), and nil until the baseline is usable.
• Existing installs: a one-time launch repair (repairAbsoluteSkinTempIfNeeded) moves already-imported absolutes out of the deviation column and recomputes the deviations, so nobody has to re-import. No re-score is needed, since stored scores never read the column (above); the repair refreshes the repository so the breakdown, Trends and Coach read the corrected values straight away.
• Interrupted repair: it is one upsertDailyMetrics followed by the flag. A kill before the upsert commits changes nothing, and the repair runs again. A kill between the upsert and the flag means the next launch finds the moved rows (skinTempC set, so the guard skips them) and recomputes identical deviations. It changes nothing and then sets the flag.
• VitalBands.isAbsoluteSkinTemp stays for now. It is how the repair finds old rows, and it covers rows imported on a build without this fix until the repair has run. The rule is physically safe: no wrist skin temperature is below 20 °C, and no nightly deviation reaches ±20 °C. It can be removed once that window has passed.
• No screen changes: the magnitude-sniffing screens already handle both kinds of value.

Export. The WHOOP CSV exporter wrote skinTempDevC into its Skin temp (celsius) column, so a computed night exported its deviation (~0.2) as a skin temperature, and a re-import then stored it as the absolute. Both exporters now write skinTempC, falling back to skinTempDevC only when it is itself an absolute (the shape older imports stored). A true deviation is never written as a temperature. The new exportedSkinTempCelsius helper is the same on both platforms.

Android. The Kotlin importer has the same bug: WhoopCsvImporter.parseCycles wrote skin_temp_celsius into skinTempDevC (WhoopCsvImporter.kt:354→:387). This PR adds the twin:

• withSkinTempDeviations fills the deviation from prior nights, and parseCycles now writes skinTempC;
• skinTempRepair / repairAbsoluteSkinTempIfNeeded is the one-time repair, run from AppViewModel before the Effort rescore behind NoopPrefs.KEY_SKIN_TEMP_REPAIR_DONE, with the same interrupted-repair semantics as Swift;
• WhoopCsvExporter.exportedSkinTempCelsius is the export fix.

Recorded in parity_dispositions.json: the Kotlin repairAbsoluteSkinTempIfNeeded twins a Swift function in the app layer (Strand/Data/WhoopImporter.swift), outside the governed roots.

Type of change

• Bug fix

How it was tested

• Swift. StrandTests/WhoopImportSkinTempTests (3 tests), plus WhoopCsvExporterTests.testTheSkinColumnCarriesTheAbsoluteNeverTheDeviation.
• Android, built and run locally (JDK 17, compileFullDebugKotlin + testFullDebugUnitTest):
  • WhoopCsvImporterTest has 3 new tests (deviation from prior nights and never the absolute; a row without an absolute keeps its deviation; the repair moves an imported absolute once and is idempotent). The two Importing a Fahrenheit WHOOP export stores the °F number as if it were °C #1849 °F/°C tests now assert skinTempC.
  • WhoopCsvExporterTest has a new test for the absolute-never-deviation column.
  • SkinTempFallbackNoteTest still passes.
  • Full suite: 6244 tests, 2 failures, both in RecoveryDriversTest (-0.4999999999999929 vs -0.5 half-tie rounding). They fail identically with this change stashed, so they are unrelated; possibly a JDK or architecture difference on this Mac, given your run passed.
• doc_comment_lint.py is clean.
• On device (iOS): this fork's build runs on an iPhone with a 413-day WHOOP import. The repair ran at launch.

Checklist

• Swift package tests pass for any package I touched
• Android unit tests pass if I touched android/ (see the two unrelated RecoveryDriversTest failures above)
• No new build warnings introduced
• Follows the conventions in docs/CONTRIBUTING.md
• I did not commit generated output (Strand.xcodeproj/) or any secrets/keystores

[ryanbr]

Verified both claims against the code, and the PR is underselling itself.

WhoopImporter.swift:39 does write the absolute into the deviation column, NOTE and all, and CoupledView.swift:554 does consume that column as a deviation. Agreed on both.

But this is not only a reading problem. The same column reaches the Charge SCORE, at IntelligenceEngine.swift:3094, :3112 and :3131, all passing skinTempDev: daily.skinTempDevC into RecoveryScorer.recovery(...), where it enters as a symmetric penalty:

// RecoveryScorer.swift:361, skinTempScaleC = 1.0, wSkinTemp = 0.05
if let dev = skinTempDev {
    terms.append((-abs(dev) / skinTempScaleC, wSkinTemp))
}

At the 33.5 you measured that is a term of about -33, against other terms that live in single digits. So imported nights have not merely displayed a wrong number, they have been SCORED with a large negative skin-temp penalty for as long as the import has existed.

That makes the one-time repair considerably more valuable than "nobody has to re-import", and it is worth saying in the PR body, because a reviewer skimming for risk will otherwise read this as cosmetic.

Three things I would want settled before this lands:

1. Does the repair trigger a re-score? Moving the column corrects the INPUT, but any Charge value already persisted with the -33 penalty stays wrong until that night is scored again. Corrected inputs with stale outputs would be a confusing half-state, and the wearer cannot tell which nights are which.

2. Is the repair safe if it is interrupted? It claims exactly-once via a flag. On a 413-day import the interesting case is a crash or a kill partway through: does a second launch finish the job, or does the flag say it is done?

3. What becomes of the magnitude sniffing? VitalBands.isAbsoluteSkinTemp exists to cope with the old shape. Once imports store the absolute in skinTempC, it is either dead weight or, less comfortably, a rule that could misfire on a genuinely large deviation. Worth a line either way so the next reader knows it was considered.

Design looks right otherwise. Deriving the deviation from the PRECEDING nights is the correct call: folding the night into its own baseline would leak the value into its own deviation, and matching recomputeSkinTempDev's rounding keeps imported and computed rows comparable.

On the Android note in the description, that matches what I would expect, though it is worth confirming rather than assuming since the Kotlin importer may not share this shape.

Thanks @Iskrata, this is a good catch and a well-scoped change.

[Iskrata]

Thanks. One correction first, because it changes what the repair has to do.

The stored Charge on imported nights was not scored with the penalty. Imported rows keep the export's own recovery. recomputeRecovery / recomputeChargeDrivers (IntelligenceEngine.swift:3094, :3112, :3131) are reached only for the nights NOOP scores itself: recomputeRecoveryDaily at :2065 and the drivers at :2080 both sit inside the scoredNights loop. There the deviation is recomputed from nightly skin samples against a baseline that is on-device only (:1875). What did read the bad column:

• the Charge breakdown sheet (ChargeBreakdownWiring.swift:49 passes row.skinTempDevC for any row, so an imported day's "why" showed a skin driver of about −33);
• the Trends report;
• the Coach's context (AICoach.swift:1215 averages skinTempDevC over 30 days).

The description now says this, so it doesn't read as cosmetic or as a scoring bug.

1. Re-score: not needed for stored scores, for the reason above. The repair calls repo.refresh(), so the breakdown, Trends and Coach read the corrected column straight away.
2. Interrupted repair: it does one upsertDailyMetrics, and sets the flag only after that.
   • Killed before the upsert commits: nothing has changed, and the repair runs again at the next launch.
   • Killed between the upsert and the flag: the next launch finds the moved rows (skinTempC set, so the guard skips them) and recomputes identical deviations. It changes nothing and then sets the flag.
3. VitalBands.isAbsoluteSkinTemp: still needed for now. It is how the repair finds old rows, and it covers rows imported on a build without this fix until the repair has run. The rule is physically safe: no wrist skin temperature is below 20 °C, and no nightly deviation reaches ±20 °C. It can go once that window has passed, as a separate cleanup.

Rebased onto current main with no conflicts; the ratchet reports 0 errors.

Android: I'll check the Kotlin importer for the same shape once my Android toolchain is set up, and add the matching fix here if it has it, rather than guess.

[Iskrata]

Checked the Kotlin importer as promised: it has the same shape (WhoopCsvImporter.kt:354→:387 wrote skin_temp_celsius into skinTempDevC). cab7884 adds the Android twin (deviation from prior nights, the one-time repair behind NoopPrefs, tests), built and run locally. While there I found the exporter half of the same bug on both platforms: Skin temp (celsius) was written from skinTempDevC, so a computed night exported its ~0.2 deviation as a skin temperature. Both exporters now write the absolute and never a deviation. Details and test results are in the updated description. Also rebased onto current main.

[ryanbr]

Reviewed at cb4b6ba. A one-shot repair that rewrites stored rows deserves more than a read-through, so here is what I actually checked rather than assumed. All of it came back clean:

• Repair scope is right. It runs under deviceId = "my-whoop", and engine-computed rows live under my-whoop-noop (Repository.swift:144). I walked every upsertDailyMetrics caller: only the importer and the demo seeder write under the raw id. So withSkinTempDeviations cannot overwrite an on-device deviation with an import-baseline one. That was my main worry, because computed rows do carry skinTempC (IntelligenceEngine.swift:3091), and the scoping is what makes it safe.
• Dictionary(uniqueKeysWithValues:) cannot trap. dailyMetric is keyed (deviceId, day), per the importer's own comment at line 195, so there is no duplicate-day crash hiding in the repair.
• No silent field-dropping. with(recovery:skinTempDevC:skinTempC:) enumerates all 21 fields, so the upsert cannot blank a column it did not mention.
• The interrupted-repair semantics hold as described. A failed read or a failed upsert returns before the flag, so it retries; a kill between the upsert and the flag re-runs, recomputes identical values, finds nothing changed, and sets the flag.
• Failure parity holds. The Kotlin call is wrapped in runCatching with cancellation re-thrown, matching the timestamp heal directly above it, so both platforms degrade the same way rather than one crashing where the other shrugs.
• CI is 19 of 19 green.

The finding: the Kotlin rounding reverses a decision this codebase already made

The new withSkinTempDeviations rounds with Math.round(...). Two hundred lines away, recomputeSkinTempDev (IntelligenceEngine.kt:2757) deliberately does not:

// Round HALF-AWAY-FROM-ZERO to 2 dp to match Swift's Double.rounded()
// (IntelligenceEngine.swift:291). Math.round() is half-UP and would diverge on negative
// .5 ties (e.g. -2.5 -> -2 here vs Swift's -3). (Cross-platform parity.)
val r = if (scaled >= 0) Math.floor(scaled + 0.5) else Math.ceil(scaled - 0.5)

Same metric, same 2 dp, same signed column, and the new path uses precisely the function that comment exists to warn against. Confirmed on a JDK rather than reasoned about: a deviation of -0.005 gives +0.00 in Kotlin against -0.01 in Swift; -0.015 gives -0.01 against -0.02. Positives agree. The description says the value is "rounded to 0.01 °C like the on-device deviation", which is true on positives only, and a skin-temp deviation is negative about half the time by construction.

What makes this worth fixing before merge rather than after: the repair is one-shot behind skinTempRepairDone. Whatever it writes is what Android users keep. A later rounding fix would not re-run for anyone already repaired without bumping that flag key to a v2. Two lines now, a migration later.

While you are in there, it is probably worth extracting that rounding into one shared helper that both call, so a third copy cannot drift the same way.

Two smaller notes

1. One column now carries two baselines. Imported deviations fold a baseline over imported absolutes; computed ones use the on-device baseline. That is a large improvement on +33 either way, but the Coach averages this column over 30 days (AICoach.swift:1215), so a window spanning both provenances mixes them. Worth a line in the doc comment so the next reader knows the column is not homogeneous.
2. The branch is dirty, 27 commits behind, and carries a parity_twin_map.json stamp derived on the old base. Main's authority was migrated earlier today in f8ada93f6, so a rebase plus a plain --refresh-derived will now re-derive cleanly, with no --migrate-authority needed. That is worth knowing because it would not have worked an hour ago: the base itself could not be reproduced, which is why fix(health): hold a still-open night out of Apple Health until it closes #2294 needed main repaired first.

On the two local RecoveryDriversTest failures: stashing the change and re-running to show they fail identically is exactly the right way to establish that, and CI agrees with you. Not a concern. Mildly ironic that it is the same half-tie rounding family as the finding above.

Approving once the Kotlin rounding uses the existing half-away-from-zero form and the branch is rebased. The analysis in the description is unusually good, particularly tracing why the stored Charge score never reads the column; I checked that claim and it holds.

[Iskrata]

Thanks. Both done in fe247c7e6, rebased onto main, with a plain --refresh-derived:

• Rounding. The half-away-from-zero form now lives in one helper, Baselines.roundedDelta2dp, on both platforms. recomputeSkinTempDev and withSkinTempDeviations both call it, on Swift and Kotlin, so there's no third copy to drift. Having a Swift twin also lets the ledger pair it rather than needing a disposition. New tests on each side pin the negative tie: −0.125 °C (an exact binary tie at 2 dp) stores −0.13, and the Kotlin test also asserts that Math.round gives −0.12, so it records the divergence it's guarding against.
• Two baselines in one column. Noted in the doc comment on both importers, including the Coach's 30-day average as the place where they mix.

Locally: BaselinesTests (41), WhoopImportSkinTempTests (3), and Kotlin BaselinesSigmaDaytimeTest, WhoopCsvImporterTest and WhoopCsvExporterTest all pass.