diff --git a/Packages/StrandAnalytics/Sources/StrandAnalytics/SkinTempDisplay.swift b/Packages/StrandAnalytics/Sources/StrandAnalytics/SkinTempDisplay.swift index 3f71229e92..5b8f2aa018 100644 --- a/Packages/StrandAnalytics/Sources/StrandAnalytics/SkinTempDisplay.swift +++ b/Packages/StrandAnalytics/Sources/StrandAnalytics/SkinTempDisplay.swift @@ -22,6 +22,26 @@ public enum SkinTempDisplay { VitalBands.isAbsoluteSkinTemp(value) ? .absolute : .deviation } + /// The one kind a MIXED series must be reduced to before any aggregate is taken (#1705). + /// + /// A CSV import writes absolute °C and the computed pipeline writes a deviation, both into this + /// same field, so one window can hold both. Formatting stays honest per value — `kind(of:)` is + /// re-derived for each — but a min, max, mean or window-over-window delta drawn across both is + /// arithmetic on two different scales, and it looks plausible: a handful of ~34 °C readings lift a + /// should-be-near-zero deviation average past a full degree, and the mean is then itself below 20 + /// so it gets labelled Δ°C. + /// + /// The newest entry decides, matching what `VitalSignsSummary` already does for its sparkline — + /// the reading the user is actually looking at sets the scale, and older entries of the other kind + /// drop out rather than being converted, because converting needs a baseline that a fresh + /// import-only install does not have. + /// + /// - Parameter values: the window's values, ascending by day. + /// - Returns: the kind to keep, or nil for an empty window. + public static func dominantKind(valuesAscendingByDay values: [Double]) -> Kind? { + values.last.map(kind(of:)) + } + /// Trailing unit chip: `"°C"` / `"°F"` for absolute, `"Δ°C"` / `"Δ°F"` for deviation. public static func unitSymbol(kind: Kind, fahrenheit: Bool) -> String { let base = fahrenheit ? "°F" : "°C" diff --git a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SkinTempDisplayTests.swift b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SkinTempDisplayTests.swift index a42d607409..2793cd2859 100644 --- a/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SkinTempDisplayTests.swift +++ b/Packages/StrandAnalytics/Tests/StrandAnalyticsTests/SkinTempDisplayTests.swift @@ -66,4 +66,37 @@ final class SkinTempDisplayTests: XCTestCase { XCTAssertEqual(SkinTempDisplay.kind(of: v) == .absolute, abs, "v=\(v)") } } + + // MARK: - dominantKind (#1705) + + func testDominantKindIsTheNewestEntrys() { + XCTAssertEqual(SkinTempDisplay.dominantKind(valuesAscendingByDay: [34.6, 35.1, -0.2]), .deviation) + XCTAssertEqual(SkinTempDisplay.dominantKind(valuesAscendingByDay: [-0.2, 0.1, 34.6]), .absolute) + XCTAssertNil(SkinTempDisplay.dominantKind(valuesAscendingByDay: [])) + } + + func testDominantKindOfASingleKindWindowIsThatKind() { + XCTAssertEqual(SkinTempDisplay.dominantKind(valuesAscendingByDay: [-0.24, 0.21, 0.01]), .deviation) + XCTAssertEqual(SkinTempDisplay.dominantKind(valuesAscendingByDay: [32.39, 34.60, 36.64]), .absolute) + } + + /// The regression guard the issue asked for: filtering by `dominantKind` must leave a window that + /// no aggregate can straddle. Values are the reported ones — a 313-row absolute import + /// (32.39…36.64, mean 34.60) coexisting with computed deviations inside ±0.3. + func testFilteringByDominantKindLeavesOneScale() { + let mixed: [Double] = [34.60, 35.12, 32.39, 36.64, -0.24, 0.21, 0.01, 0.11, 0.0] + guard let keep = SkinTempDisplay.dominantKind(valuesAscendingByDay: mixed) else { + return XCTFail("a non-empty window must have a kind") + } + let kept = mixed.filter { SkinTempDisplay.kind(of: $0) == keep } + XCTAssertEqual(kept, [-0.24, 0.21, 0.01, 0.11, 0.0]) + XCTAssertTrue(kept.allSatisfy { SkinTempDisplay.kind(of: $0) == keep }) + // The defect: unfiltered, the mean of a should-be-near-zero deviation window clears a degree + // and then reads BELOW 20, so it gets labelled Δ°C — plausible-looking and wrong. + let unfilteredMean = mixed.reduce(0, +) / Double(mixed.count) + XCTAssertGreaterThan(unfilteredMean, 1.0) + XCTAssertEqual(SkinTempDisplay.kind(of: unfilteredMean), .deviation) + let filteredMean = kept.reduce(0, +) / Double(kept.count) + XCTAssertLessThan(abs(filteredMean), 0.3) + } } diff --git a/Strand/Data/Repository.swift b/Strand/Data/Repository.swift index eeff3df288..5bd913a6b3 100644 --- a/Strand/Data/Repository.swift +++ b/Strand/Data/Repository.swift @@ -2160,7 +2160,9 @@ final class Repository: ObservableObject { /// byte-identical. The flag is forwarded to the non-strap `series(...)` delegation below so every source /// path honours it. func exploreSeries(key: String, source: String, days: Int = 4000, fullHistory: Bool = false) async -> [(day: String, value: Double)] { - guard source == "my-whoop" else { return await series(key: key, source: source, days: days, fullHistory: fullHistory) } + guard source == "my-whoop" else { + return Self.oneSkinTempScale(key: key, await series(key: key, source: source, days: days, fullHistory: fullHistory)) + } guard let store = await ensureStore() else { return [] } let now = Date() let from = fullHistory ? "0000-01-01" : Self.dayString(now.addingTimeInterval(-Double(days) * 86_400)) @@ -2187,7 +2189,29 @@ final class Repository: ObservableObject { for p in (try? await store.metricSeries(deviceId: id, key: key, from: from, to: to)) ?? [] { byDay[p.day] = p.value } } - return byDay.sorted { $0.key < $1.key }.map { (day: $0.key, value: $0.value) } + return Self.oneSkinTempScale(key: key, byDay.sorted { $0.key < $1.key }.map { (day: $0.key, value: $0.value) }) + } + + /// #1705: reduce a `skin_temp` window to a single scale; every other key passes through untouched. + /// + /// The key is bimodal — a CSV import writes ABSOLUTE °C where the computed layers write a baseline + /// DEVIATION, so one window can hold both. Every consumer of an Explore series aggregates it + /// (min/max/mean, the window-over-window delta, the correlation scan), and those are arithmetic + /// across two scales: on the reported account a few ~34 °C readings lifted a should-be-near-zero + /// average past a full degree, and the mean then read below 20 so it was labelled Δ°C. + /// + /// Applied on BOTH of `exploreSeries`' return paths, deliberately. Today the only `skin_temp` + /// descriptor is `my-whoop`, so the delegating path is unreachable for it — but a guard that is + /// correct only because of a fact in another file is how this bug happened in the first place. + /// `series(day:value:)` is ordered ascending by day, which `dominantKind` relies on. + private static func oneSkinTempScale( + key: String, + _ series: [(day: String, value: Double)] + ) -> [(day: String, value: Double)] { + guard key == "skin_temp", + let keep = SkinTempDisplay.dominantKind(valuesAscendingByDay: series.map(\.value)) + else { return series } + return series.filter { SkinTempDisplay.kind(of: $0.value) == keep } } /// Every catalog metric's Explore series at once, memoized — the cross-catalog scan behind the diff --git a/android/app/src/main/java/com/noop/analytics/SkinTempDisplay.kt b/android/app/src/main/java/com/noop/analytics/SkinTempDisplay.kt index d6564aff5d..8d222d02de 100644 --- a/android/app/src/main/java/com/noop/analytics/SkinTempDisplay.kt +++ b/android/app/src/main/java/com/noop/analytics/SkinTempDisplay.kt @@ -19,6 +19,27 @@ object SkinTempDisplay { DEVIATION, } + /** + * The one kind a MIXED series must be reduced to before any aggregate is taken (#1705). + * + * A CSV import writes absolute °C and the computed pipeline writes a deviation, both into this + * same field, so one window can hold both. Formatting stays honest per value — [kind] is + * re-derived for each — but a min, max, mean or window-over-window delta drawn across both is + * arithmetic on two different scales, and it looks plausible: a handful of ~34 °C readings lift a + * should-be-near-zero deviation average past a full degree, and the mean is then itself below 20 + * so it gets labelled Δ°C. + * + * The newest entry decides, matching what `HealthVitalsLogic` already does for its sparkline — the + * reading the user is actually looking at sets the scale, and older entries of the other kind drop + * out rather than being converted, because converting needs a baseline that a fresh import-only + * install does not have. + * + * @param valuesAscendingByDay the window's values, ascending by day. + * @return the kind to keep, or null for an empty window. + */ + fun dominantKind(valuesAscendingByDay: List): Kind? = + valuesAscendingByDay.lastOrNull()?.let { kind(it) } + fun kind(value: Double): Kind = if (VitalBands.isAbsoluteSkinTemp(value)) Kind.ABSOLUTE else Kind.DEVIATION diff --git a/android/app/src/main/java/com/noop/data/WhoopRepository.kt b/android/app/src/main/java/com/noop/data/WhoopRepository.kt index 80b7b2dea4..b4b92d35a5 100644 --- a/android/app/src/main/java/com/noop/data/WhoopRepository.kt +++ b/android/app/src/main/java/com/noop/data/WhoopRepository.kt @@ -1823,7 +1823,24 @@ class WhoopRepository( // Book / any resolver read agrees with the mergeDaily dashboards instead of re-surfacing the // schedule target the merge already rejects. val perCandidate = candidates.map { it to resolvedRows(it, from, to) } - return MetricSeriesResolution(preferredSource, candidates, resolveFirstWins(perCandidate)) + val points = resolveFirstWins(perCandidate) + // #1705: skin_temp is bimodal — a CSV import writes ABSOLUTE °C into skinTempDevC while the + // computed pipeline writes a baseline DEVIATION, and the candidate list unions the imported and + // computed identities, so first-wins can resolve older days to one kind and recent days to the + // other. Every consumer aggregates these points (Compare's min/max and its min-max chart + // normalisation, Lab Book), and that is arithmetic across two scales. Reduce to the newest + // entry's kind HERE, at the one place the candidates are resolved, rather than at each screen. + if (key == "skin_temp") { + val keep = com.noop.analytics.SkinTempDisplay.dominantKind(points.map { it.value }) + if (keep != null) { + return MetricSeriesResolution( + preferredSource, + candidates, + points.filter { com.noop.analytics.SkinTempDisplay.kind(it.value) == keep }, + ) + } + } + return MetricSeriesResolution(preferredSource, candidates, points) } /** diff --git a/android/app/src/test/java/com/noop/analytics/SkinTempDisplayTest.kt b/android/app/src/test/java/com/noop/analytics/SkinTempDisplayTest.kt index 53d97ca1af..450ec4e40a 100644 --- a/android/app/src/test/java/com/noop/analytics/SkinTempDisplayTest.kt +++ b/android/app/src/test/java/com/noop/analytics/SkinTempDisplayTest.kt @@ -1,6 +1,7 @@ package com.noop.analytics import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test @@ -73,4 +74,36 @@ class SkinTempDisplayTest { ) } } -} + + // ---- dominantKind (#1705) ---- + + @Test fun dominantKindIsTheNewestEntrys() { + assertEquals(SkinTempDisplay.Kind.DEVIATION, SkinTempDisplay.dominantKind(listOf(34.6, 35.1, -0.2))) + assertEquals(SkinTempDisplay.Kind.ABSOLUTE, SkinTempDisplay.dominantKind(listOf(-0.2, 0.1, 34.6))) + assertNull(SkinTempDisplay.dominantKind(emptyList())) + } + + @Test fun dominantKindOfASingleKindWindowIsThatKind() { + assertEquals(SkinTempDisplay.Kind.DEVIATION, SkinTempDisplay.dominantKind(listOf(-0.24, 0.21, 0.01))) + assertEquals(SkinTempDisplay.Kind.ABSOLUTE, SkinTempDisplay.dominantKind(listOf(32.39, 34.60, 36.64))) + } + + /** + * The regression guard the issue asked for: filtering by [SkinTempDisplay.dominantKind] must leave a + * window that no aggregate can straddle. Values are the reported ones — a 313-row absolute import + * (32.39..36.64, mean 34.60) coexisting with computed deviations inside +/-0.3. + */ + @Test fun filteringByDominantKindLeavesOneScale() { + val mixed = listOf(34.60, 35.12, 32.39, 36.64, -0.24, 0.21, 0.01, 0.11, 0.0) + val keep = SkinTempDisplay.dominantKind(mixed)!! + val kept = mixed.filter { SkinTempDisplay.kind(it) == keep } + assertEquals(listOf(-0.24, 0.21, 0.01, 0.11, 0.0), kept) + assertTrue(kept.all { SkinTempDisplay.kind(it) == keep }) + // The defect: unfiltered, the mean of a should-be-near-zero deviation window clears a degree and + // then reads BELOW 20, so it gets labelled deviation — plausible-looking and wrong. + val unfilteredMean = mixed.average() + assertTrue(unfilteredMean > 1.0) + assertEquals(SkinTempDisplay.Kind.DEVIATION, SkinTempDisplay.kind(unfilteredMean)) + assertTrue(kotlin.math.abs(kept.average()) < 0.3) + } +} \ No newline at end of file