From 704ef11eab711903c4c4d2416f4a1aaf25dccd05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 9 Sep 2026 22:21:05 +0200 Subject: [PATCH 01/10] test: pin the km-scale walking distance the export import stores as metres The Apple export writes DistanceWalkingRunning in the account's own length unit; the record mapping assumes metres and never reads the attribute, so a 2.484 km day lands as 2.484 m. Refs #944 --- .../import-apple-health-export.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/lib/measurements/__tests__/import-apple-health-export.test.ts b/src/lib/measurements/__tests__/import-apple-health-export.test.ts index ddb4c3c6c..c63637ec9 100644 --- a/src/lib/measurements/__tests__/import-apple-health-export.test.ts +++ b/src/lib/measurements/__tests__/import-apple-health-export.test.ts @@ -1278,3 +1278,35 @@ describe("streamParseExportXml — the archive's own export stamp", () => { expect(stamps).toEqual([]); }); }); + +describe("streamParseExportXml — the record's own unit (issue #944)", () => { + it("stores a km-scale walking distance in metres", async () => { + const { prisma } = await importCumulativeFixture( + cumulativeExportXml( + "HKQuantityTypeIdentifierDistanceWalkingRunning", + "km", + [{ value: 2.484, sourceName: "iPhone", hour: 8 }], + ), + ); + + expect(prisma._measurements).toHaveLength(1); + expect(prisma._measurements[0]).toMatchObject({ + type: "WALKING_RUNNING_DISTANCE", + unit: "m", + }); + expect(prisma._measurements[0].value).toBeCloseTo(2484, 6); + }); + + it("stores a mile-scale walking distance in metres", async () => { + const { prisma } = await importCumulativeFixture( + cumulativeExportXml( + "HKQuantityTypeIdentifierDistanceWalkingRunning", + "mi", + [{ value: 1.543, sourceName: "iPhone", hour: 8 }], + ), + ); + + expect(prisma._measurements).toHaveLength(1); + expect(prisma._measurements[0].value).toBeCloseTo(2483.217792, 6); + }); +}); From 5c7c7afd591b3ef747c254767950d76fcfba5356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 9 Sep 2026 22:31:31 +0200 Subject: [PATCH 02/10] fix(import): honour the unit an Apple Health record carries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apple stamps every quantity `` of an export archive with the account's own display unit, not the unit the mapping table assumes: a metric archive reports walking distance in km, an imperial one in mi, body mass in lb, energy in kJ, temperature in degF. The mapper accepted `input.unit` and never read it, so a 2.484 km day was folded into the day's total as 2.484 m and a decade of imported history read as a couple of metres walked. `hk-units.ts` is now the one place a HealthKit unit is converted, shared by the record path and the workout path (which carried its own km/mi branch — the duplicate is what let the record path stay wrong). It converts only inside a family and only for units Apple can emit, and returns the reading untouched for anything it cannot place, so an archive that already speaks the table's unit is unchanged. Every mapping-table entry whose unit cannot be converted — a dimensionless count, a pinned event, a logarithmic dB level, a compound rate, the percent units whose value rides as a 0..1 fraction — now carries a written reason, and a structural guard fails on an entry that offers neither a convertible unit nor a reason. Refs #944 --- .../apple-health-unit-audit-guard.test.ts | 165 ++++++++++++++++ .../measurements/__tests__/hk-units.test.ts | 77 ++++++++ src/lib/measurements/apple-health-mapping.ts | 86 +++++++- src/lib/measurements/hk-units.ts | 185 ++++++++++++++++++ .../import-apple-health-export.ts | 13 +- 5 files changed, 515 insertions(+), 11 deletions(-) create mode 100644 src/__tests__/apple-health-unit-audit-guard.test.ts create mode 100644 src/lib/measurements/__tests__/hk-units.test.ts create mode 100644 src/lib/measurements/hk-units.ts diff --git a/src/__tests__/apple-health-unit-audit-guard.test.ts b/src/__tests__/apple-health-unit-audit-guard.test.ts new file mode 100644 index 000000000..49e162269 --- /dev/null +++ b/src/__tests__/apple-health-unit-audit-guard.test.ts @@ -0,0 +1,165 @@ +/** + * Structural guard on the Apple Health mapping table's unit column + * (issue #944). + * + * `AppleHealthMapping.hkUnit` records the unit HealthLog ASSUMES a + * HealthKit reading arrives in, and `convertToDbUnit` is written against + * that assumption. Apple's `export.xml` does not honour it: every quantity + * `` carries the account's own display unit, so a metric archive + * reports walking distance in `km` and an imperial one in `mi`. For ten + * years those readings were stored as if the number were already metres — + * the whole history understated by a factor of a thousand — because the + * mapper accepted `input.unit` and never read it. + * + * So an entry now has exactly two legal shapes: + * + * 1. its `hkUnit` is one `convertHkValue()` knows a factor for, in which + * case `mapAppleHealthEntry()` converts a differing record unit into + * it before `convertToDbUnit` runs; or + * 2. it carries a written `unitFixedReason` saying why no unit attribute + * Apple could write would change the reading (a dimensionless count, + * a pinned event, a logarithmic dB level, a compound rate). + * + * The reason arm alone would be a guard that cannot fail: an entry could + * declare a convertible unit while the mapper quietly ignored it. The + * behavioural block below therefore feeds every convertible entry a + * reading in a SIBLING unit and asserts the stored number moved by the + * documented factor — so deleting the conversion turns this guard red, + * not just the one reproduction test. + * + * A tripwire, not a proof: a reviewer who waves through a wrong factor + * defeats it. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { + APPLE_HEALTH_TYPE_MAP, + mapAppleHealthEntry, +} from "@/lib/measurements/apple-health-mapping"; +import { + convertHkValue, + isConvertibleHkUnit, +} from "@/lib/measurements/hk-units"; + +const ENTRIES = Object.values(APPLE_HEALTH_TYPE_MAP); + +/** + * Units from every family the table touches. The behavioural block picks + * the first one that converts into an entry's `hkUnit` and actually moves + * the number, so a new family only needs one representative here. + */ +const SIBLING_UNITS = [ + "km", + "mi", + "cm", + "lb", + "st", + "kJ", + "cal", + "degF", + "K", + "mmol<180.156>/L", + "kPa", + "km/hr", + "hr", + "ms", +]; + +function siblingUnitFor(hkUnit: string): string | null { + for (const candidate of SIBLING_UNITS) { + const converted = convertHkValue(1, candidate, hkUnit); + if (converted !== null && converted !== 1) return candidate; + } + return null; +} + +describe("Apple Health mapping — the record's own unit (issue #944)", () => { + it("examines the whole mapping table", () => { + // An empty match set is the failure mode this repo keeps rediscovering: + // a guard is green because it matched nothing, not because nothing is + // wrong. Pin the floor. + expect(ENTRIES.length).toBeGreaterThan(40); + }); + + it.each(ENTRIES)( + "$hkIdentifier either converts its record unit or says why it cannot", + (mapping) => { + if (isConvertibleHkUnit(mapping.hkUnit)) { + expect(mapping.unitFixedReason).toBeUndefined(); + return; + } + expect( + mapping.unitFixedReason, + `${mapping.hkIdentifier} pins hkUnit "${mapping.hkUnit}" with no conversion and no written reason`, + ).toBeTypeOf("string"); + // Long enough to be a sentence rather than a shrug. + expect(mapping.unitFixedReason!.length).toBeGreaterThan(40); + }, + ); + + const convertible = ENTRIES.filter((mapping) => + isConvertibleHkUnit(mapping.hkUnit), + ); + + it("has convertible entries to exercise", () => { + expect(convertible.length).toBeGreaterThan(10); + }); + + it.each(convertible)( + "$hkIdentifier honours a sibling unit on the record itself", + (mapping) => { + const sibling = siblingUnitFor(mapping.hkUnit); + expect( + sibling, + `no sibling unit listed for the family of "${mapping.hkUnit}"`, + ).not.toBeNull(); + + const base = { + hkIdentifier: mapping.hkIdentifier, + value: 2, + startDate: "2026-05-14T08:00:00.000Z", + endDate: "2026-05-14T08:30:00.000Z", + }; + const native = mapAppleHealthEntry({ ...base, unit: mapping.hkUnit }); + const foreign = mapAppleHealthEntry({ ...base, unit: sibling! }); + expect(native).not.toBeNull(); + expect(foreign).not.toBeNull(); + + const expected = mapping.convertToDbUnit( + convertHkValue(2, sibling!, mapping.hkUnit)!, + ); + expect(foreign!.value).toBeCloseTo(expected, 9); + expect(foreign!.value).not.toBeCloseTo(native!.value, 9); + }, + ); + + it("keeps a unit it cannot place from rescaling the reading", () => { + const out = mapAppleHealthEntry({ + hkIdentifier: "HKQuantityTypeIdentifierDistanceWalkingRunning", + value: 1234, + unit: "furlong", + startDate: "2026-05-14T08:00:00.000Z", + endDate: "2026-05-14T08:30:00.000Z", + }); + expect(out?.value).toBe(1234); + }); + + it("leaves the km/mi factors in one shared module", () => { + // The workout path used to carry its own copy of the km/mi branch; the + // duplicate is what let the `` path stay wrong for ten years. + const shared = join(process.cwd(), "src/lib/measurements/hk-units.ts"); + const owners = [ + "src/lib/measurements/apple-health-mapping.ts", + "src/lib/measurements/import-apple-health-export.ts", + ]; + expect(readFileSync(shared, "utf8")).toContain("1609.344"); + for (const rel of owners) { + const source = readFileSync(join(process.cwd(), rel), "utf8"); + expect(source, `${rel} re-implements a length factor`).not.toMatch( + /1609\.344|\* 1000\b/, + ); + } + }); +}); diff --git a/src/lib/measurements/__tests__/hk-units.test.ts b/src/lib/measurements/__tests__/hk-units.test.ts new file mode 100644 index 000000000..b1162fb6b --- /dev/null +++ b/src/lib/measurements/__tests__/hk-units.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect } from "vitest"; + +import { + convertHkValue, + hkDistanceToMetres, + isConvertibleHkUnit, +} from "../hk-units"; + +describe("convertHkValue", () => { + it("converts the km reading Apple writes on a metric archive", () => { + expect(convertHkValue(2.484, "km", "m")).toBeCloseTo(2484, 9); + }); + + it("converts the mi reading Apple writes on an imperial archive", () => { + expect(convertHkValue(1.543, "mi", "m")).toBeCloseTo(2483.217792, 9); + }); + + it("returns the reading untouched when the units already agree", () => { + expect(convertHkValue(2.484, "m", "m")).toBe(2.484); + }); + + it("converts mass, energy and temperature the same way", () => { + expect(convertHkValue(180, "lb", "kg")).toBeCloseTo(81.6466266, 6); + expect(convertHkValue(1000, "kJ", "kcal")).toBeCloseTo(239.005736, 6); + expect(convertHkValue(98.6, "degF", "degC")).toBeCloseTo(37, 9); + }); + + it("strips the molar mass Apple annotates a glucose unit with", () => { + expect(convertHkValue(5.5, "mmol<180.156>/L", "mg/dL")).toBeCloseTo( + 99.0858, + 4, + ); + }); + + it("keeps the small calorie apart from the large one", () => { + expect(convertHkValue(1, "Cal", "kcal")).toBe(1); + expect(convertHkValue(1000, "cal", "kcal")).toBeCloseTo(1, 9); + }); + + it("refuses a unit it cannot place rather than guessing", () => { + expect(convertHkValue(1, "furlong", "m")).toBeNull(); + expect(convertHkValue(1, "", "m")).toBeNull(); + expect(convertHkValue(1, undefined, "m")).toBeNull(); + }); + + it("refuses to cross families", () => { + expect(convertHkValue(1, "kg", "m")).toBeNull(); + expect(convertHkValue(1, "min", "kcal")).toBeNull(); + }); + + it("refuses a non-finite reading", () => { + expect(convertHkValue(Number.NaN, "km", "m")).toBeNull(); + }); +}); + +describe("hkDistanceToMetres", () => { + it("converts the workout distance units Apple ships", () => { + expect(hkDistanceToMetres(6.5, "km")).toBeCloseTo(6500, 9); + expect(hkDistanceToMetres(3, "mi")).toBeCloseTo(4828.032, 9); + expect(hkDistanceToMetres(500, "m")).toBe(500); + }); + + it("falls back to the raw number for a missing unit attribute", () => { + expect(hkDistanceToMetres(500, "")).toBe(500); + expect(hkDistanceToMetres(500, undefined)).toBe(500); + }); +}); + +describe("isConvertibleHkUnit", () => { + it("recognises the units the mapping table pins", () => { + expect(isConvertibleHkUnit("m")).toBe(true); + expect(isConvertibleHkUnit("kg")).toBe(true); + expect(isConvertibleHkUnit("count")).toBe(false); + expect(isConvertibleHkUnit("event")).toBe(false); + expect(isConvertibleHkUnit("%")).toBe(false); + }); +}); diff --git a/src/lib/measurements/apple-health-mapping.ts b/src/lib/measurements/apple-health-mapping.ts index e665b18bd..4e760acc0 100644 --- a/src/lib/measurements/apple-health-mapping.ts +++ b/src/lib/measurements/apple-health-mapping.ts @@ -28,6 +28,7 @@ import type { SleepStage, RhythmClassification, } from "@/generated/prisma/client"; +import { convertHkValue } from "./hk-units"; /** * How a batch of HealthKit samples for the same identifier should be @@ -47,6 +48,21 @@ export interface AppleHealthMapping { measurementType: MeasurementType; /** Apple's HK unit string (e.g. `kg`, `count/min`, `mL/(kg*min)`). */ hkUnit: string; + /** + * issue #944 — why this entry's `hkUnit` can be trusted as fixed. + * + * Apple's `export.xml` writes the ACCOUNT's display unit on every + * quantity ``, not the unit this table assumes, so + * `mapAppleHealthEntry()` converts the reading from the record's own + * unit into `hkUnit` before `convertToDbUnit` runs. That only works for + * a unit `convertHkValue()` knows a factor for. An entry whose `hkUnit` + * is NOT convertible (a dimensionless count, a pinned event, a + * logarithmic dB level, a compound rate) must say here why no record + * unit can change its reading — the structural guard + * `src/__tests__/apple-health-unit-audit-guard.test.ts` fails on an + * entry that offers neither. + */ + unitFixedReason?: string; /** HealthLog canonical DB unit (must match `getUnitForType()`). */ dbUnit: string; /** Convert a HK sample value into the canonical DB unit. Identity for most. */ @@ -189,6 +205,28 @@ export const APPLE_HEALTH_SLEEP_STAGE_MAP: Record = { * metric: check Apple's HK unit; percent → ×100 path, m/m·s/kg/etc * → identity path. */ +/** + * issue #944 — the written reasons a mapping entry may cite for keeping a + * fixed `hkUnit`. Each one answers the same question: what could Apple + * possibly write in this record's `unit` attribute that would change the + * reading? Shared constants rather than free text so the audit reads as + * one decision per class instead of forty near-duplicates. + */ +const UNIT_FIXED_DIMENSIONLESS_COUNT = + "Dimensionless tally: Apple emits `count` in every locale and HKUnit has no sibling spelling for it, so no record unit can rescale the reading."; +const UNIT_FIXED_PER_MINUTE = + "Per-minute rate: Apple emits `count/min` in every locale and the Health app offers no alternative display unit, so there is nothing to convert from."; +const UNIT_FIXED_PERCENT_FRACTION = + "Apple writes the percent unit but the VALUE rides as a 0..1 fraction; the x100 in `convertToDbUnit` already covers both spellings, so branching on the record unit would double-scale it."; +const UNIT_FIXED_EVENT_PIN = + "Categorical event: `convertToDbUnit` pins the stored value to 1 fired event regardless of the reading, so no unit attribute can move it."; +const UNIT_FIXED_AUDIO_SPL = + "Sound-pressure level: Apple emits `dBASPL` only, and a decibel is logarithmic — a sibling unit would need its own formula, never a factor."; +const UNIT_FIXED_COMPOSITE_VO2 = + "Compound rate Apple emits verbatim as `mL/(kg*min)`; the Health app exposes no other spelling and the compound has no sibling unit to convert from."; +const UNIT_FIXED_SLEEP_CATEGORY = + "Sleep samples carry a symbolic category value, not a quantity: the importer derives the duration in minutes from the sample's own dates, so the record's unit attribute (empty in Apple's archive) says nothing about the reading."; + export const APPLE_HEALTH_TYPE_MAP: Record = { // ── Body composition ──────────────────────────────────────── HKQuantityTypeIdentifierBodyMass: { @@ -203,6 +241,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierBodyFatPercentage", measurementType: "BODY_FAT", hkUnit: "%", + unitFixedReason: UNIT_FIXED_PERCENT_FRACTION, dbUnit: "%", // Apple ships 0..1 fraction; HealthLog stores 0..100. convertToDbUnit: (v) => v * 100, @@ -254,6 +293,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierHeartRate", measurementType: "PULSE", hkUnit: "count/min", + unitFixedReason: UNIT_FIXED_PER_MINUTE, dbUnit: "bpm", convertToDbUnit: (v) => v, aggregation: "latest", @@ -262,6 +302,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierRestingHeartRate", measurementType: "RESTING_HEART_RATE", hkUnit: "count/min", + unitFixedReason: UNIT_FIXED_PER_MINUTE, dbUnit: "bpm", convertToDbUnit: (v) => v, aggregation: "latest", @@ -284,6 +325,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { // duplication; see `.planning/phase-W1-v1423-research.md`). measurementType: "ACTIVITY_STEPS", hkUnit: "count", + unitFixedReason: UNIT_FIXED_DIMENSIONLESS_COUNT, dbUnit: "steps", convertToDbUnit: (v) => v, aggregation: "sum", @@ -300,6 +342,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierFlightsClimbed", measurementType: "FLIGHTS_CLIMBED", hkUnit: "count", + unitFixedReason: UNIT_FIXED_DIMENSIONLESS_COUNT, dbUnit: "flights", convertToDbUnit: (v) => v, aggregation: "sum", @@ -318,6 +361,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierVO2Max", measurementType: "VO2_MAX", hkUnit: "mL/(kg*min)", + unitFixedReason: UNIT_FIXED_COMPOSITE_VO2, dbUnit: "mL/(kg·min)", convertToDbUnit: (v) => v, aggregation: "latest", @@ -337,6 +381,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierOxygenSaturation", measurementType: "OXYGEN_SATURATION", hkUnit: "fraction", + unitFixedReason: UNIT_FIXED_PERCENT_FRACTION, dbUnit: "%", // Apple ships 0..1 fraction; HealthLog stores 0..100. convertToDbUnit: (v) => v * 100, @@ -348,6 +393,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKCategoryTypeIdentifierSleepAnalysis", measurementType: "SLEEP_DURATION", hkUnit: "category", + unitFixedReason: UNIT_FIXED_SLEEP_CATEGORY, dbUnit: "minutes", // value here is duration-in-minutes (caller computes // `endDate - startDate` and converts to minutes); the per-stage @@ -367,6 +413,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierEnvironmentalAudioExposure", measurementType: "AUDIO_EXPOSURE_ENV", hkUnit: "dBASPL", + unitFixedReason: UNIT_FIXED_AUDIO_SPL, dbUnit: "dBA", convertToDbUnit: (v) => v, aggregation: "mean", @@ -379,6 +426,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierHeadphoneAudioExposure", measurementType: "AUDIO_EXPOSURE_HEADPHONE", hkUnit: "dBASPL", + unitFixedReason: UNIT_FIXED_AUDIO_SPL, dbUnit: "dBA", convertToDbUnit: (v) => v, aggregation: "mean", @@ -403,6 +451,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierAppleWalkingSteadiness", measurementType: "WALKING_STEADINESS", hkUnit: "%", + unitFixedReason: UNIT_FIXED_PERCENT_FRACTION, dbUnit: "%", // Apple ships 0..1 fraction; HealthLog stores 0..100. convertToDbUnit: (v) => v * 100, @@ -416,6 +465,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKCategoryTypeIdentifierEnvironmentalAudioExposureEvent", measurementType: "AUDIO_EXPOSURE_EVENT", hkUnit: "count", + unitFixedReason: UNIT_FIXED_EVENT_PIN, dbUnit: "count", convertToDbUnit: () => 1, aggregation: "sum", @@ -427,6 +477,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKCategoryTypeIdentifierHeadphoneAudioExposureEvent", measurementType: "AUDIO_EXPOSURE_EVENT", hkUnit: "count", + unitFixedReason: UNIT_FIXED_EVENT_PIN, dbUnit: "count", convertToDbUnit: () => 1, aggregation: "sum", @@ -449,6 +500,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKCategoryTypeIdentifierIrregularHeartRhythmEvent", measurementType: "IRREGULAR_RHYTHM_NOTIFICATION", hkUnit: "event", + unitFixedReason: UNIT_FIXED_EVENT_PIN, dbUnit: "event", convertToDbUnit: () => 1, aggregation: "sum", @@ -462,6 +514,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKCategoryTypeIdentifierHighHeartRateEvent", measurementType: "HIGH_HEART_RATE_EVENT", hkUnit: "event", + unitFixedReason: UNIT_FIXED_EVENT_PIN, dbUnit: "event", convertToDbUnit: () => 1, aggregation: "sum", @@ -474,6 +527,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKCategoryTypeIdentifierLowHeartRateEvent", measurementType: "LOW_HEART_RATE_EVENT", hkUnit: "event", + unitFixedReason: UNIT_FIXED_EVENT_PIN, dbUnit: "event", convertToDbUnit: () => 1, aggregation: "sum", @@ -487,6 +541,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKCategoryTypeIdentifierAppleWalkingSteadinessEvent", measurementType: "WALKING_STEADINESS_EVENT", hkUnit: "event", + unitFixedReason: UNIT_FIXED_EVENT_PIN, dbUnit: "event", convertToDbUnit: () => 1, aggregation: "sum", @@ -501,6 +556,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKCategoryTypeIdentifierSleepApneaEvent", measurementType: "BREATHING_DISTURBANCE_EVENT", hkUnit: "event", + unitFixedReason: UNIT_FIXED_EVENT_PIN, dbUnit: "event", convertToDbUnit: () => 1, aggregation: "sum", @@ -524,6 +580,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierRespiratoryRate", measurementType: "RESPIRATORY_RATE", hkUnit: "count/min", + unitFixedReason: UNIT_FIXED_PER_MINUTE, dbUnit: "breaths/min", convertToDbUnit: (v) => v, aggregation: "mean", @@ -537,6 +594,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierBodyMassIndex", measurementType: "BODY_MASS_INDEX", hkUnit: "count", + unitFixedReason: UNIT_FIXED_DIMENSIONLESS_COUNT, dbUnit: "kg/m²", convertToDbUnit: (v) => v, aggregation: "latest", @@ -557,6 +615,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierWalkingHeartRateAverage", measurementType: "WALKING_HEART_RATE_AVERAGE", hkUnit: "count/min", + unitFixedReason: UNIT_FIXED_PER_MINUTE, dbUnit: "bpm", convertToDbUnit: (v) => v, aggregation: "mean", @@ -570,6 +629,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierWalkingAsymmetryPercentage", measurementType: "WALKING_ASYMMETRY", hkUnit: "%", + unitFixedReason: UNIT_FIXED_PERCENT_FRACTION, dbUnit: "%", convertToDbUnit: (v) => v * 100, aggregation: "latest", @@ -580,6 +640,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierWalkingDoubleSupportPercentage", measurementType: "WALKING_DOUBLE_SUPPORT", hkUnit: "%", + unitFixedReason: UNIT_FIXED_PERCENT_FRACTION, dbUnit: "%", convertToDbUnit: (v) => v * 100, aggregation: "latest", @@ -622,6 +683,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierHeartRateRecoveryOneMinute", measurementType: "CARDIO_RECOVERY", hkUnit: "count/min", + unitFixedReason: UNIT_FIXED_PER_MINUTE, dbUnit: "bpm", convertToDbUnit: (v) => v, aggregation: "latest", @@ -645,6 +707,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierNumberOfTimesFallen", measurementType: "FALL_COUNT", hkUnit: "count", + unitFixedReason: UNIT_FIXED_DIMENSIONLESS_COUNT, dbUnit: "count", convertToDbUnit: (v) => v, aggregation: "sum", @@ -685,6 +748,7 @@ export const APPLE_HEALTH_TYPE_MAP: Record = { hkIdentifier: "HKQuantityTypeIdentifierAppleSleepingBreathingDisturbances", measurementType: "BREATHING_DISTURBANCES", hkUnit: "count", + unitFixedReason: UNIT_FIXED_DIMENSIONLESS_COUNT, dbUnit: "count", convertToDbUnit: (v) => v, aggregation: "latest", @@ -1140,7 +1204,12 @@ export interface AppleHealthEntryInput { hkIdentifier: string; /** Numeric value as Apple delivers it (pre-conversion). */ value: number; - /** Apple's unit string, captured for audit; not currently validated. */ + /** + * Apple's unit string for this reading. issue #944 — the mapper converts + * the value from this unit into the table's `hkUnit` when the two differ + * and a factor is known (`km` -> `m`, `lb` -> `kg`, `degF` -> `degC`, …); + * an unknown or unplaceable unit leaves the reading untouched. + */ unit: string; /** ISO timestamp string (e.g. HealthKit `startDate`). */ startDate: string; @@ -1221,7 +1290,16 @@ export function mapAppleHealthEntry( const takenAt = new Date(input.endDate); if (Number.isNaN(takenAt.getTime())) return null; - const value = mapping.convertToDbUnit(input.value); + // issue #944 — `convertToDbUnit` is written against the table's fixed + // `hkUnit`, but Apple's `export.xml` stamps each `` with the + // ACCOUNT's own display unit: a metric archive reports walking distance + // in `km`, an imperial one in `mi`, body mass in `lb`, energy in `kJ`. + // Normalise the reading into `hkUnit` first; a unit we cannot place + // leaves the value untouched (see `convertHkValue`). + const toHkUnit = (raw: number): number => + convertHkValue(raw, input.unit, mapping.hkUnit) ?? raw; + + const value = mapping.convertToDbUnit(toHkUnit(input.value)); const out: AppleHealthEntryOutput = { type: mapping.measurementType, @@ -1235,10 +1313,10 @@ export function mapAppleHealthEntry( // hourly HR bucket). A finite check guards a malformed payload; the // caller only persists these on the HR-bucket row. if (typeof input.valueMin === "number" && Number.isFinite(input.valueMin)) { - out.valueMin = mapping.convertToDbUnit(input.valueMin); + out.valueMin = mapping.convertToDbUnit(toHkUnit(input.valueMin)); } if (typeof input.valueMax === "number" && Number.isFinite(input.valueMax)) { - out.valueMax = mapping.convertToDbUnit(input.valueMax); + out.valueMax = mapping.convertToDbUnit(toHkUnit(input.valueMax)); } if (mapping.sleepStageMap) { diff --git a/src/lib/measurements/hk-units.ts b/src/lib/measurements/hk-units.ts new file mode 100644 index 000000000..f500be22d --- /dev/null +++ b/src/lib/measurements/hk-units.ts @@ -0,0 +1,185 @@ +/** + * HealthKit unit normalisation (issue #944). + * + * Apple writes a `unit` attribute on every quantity `` of an + * `export.xml` archive, and the string it writes is the account's OWN + * display unit rather than the unit HealthLog's mapping table assumes: + * a metric account exports walking distance as `km`, an imperial one as + * `mi`, and the same split shows up on body mass (`kg` / `lb` / `st`), + * energy (`kcal` / `kJ`) and temperature (`degC` / `degF`). + * + * Before this module the export import fed the raw number straight into + * `AppleHealthMapping.convertToDbUnit`, which is written against the + * table's fixed `hkUnit` — so a 2.484 km day was stored as 2.484 m. The + * workout path had a hand-rolled km/mi→m branch of its own; both paths + * now share `convertHkValue()`. + * + * Contract, deliberately conservative: + * + * - Same unit in and out → the value is returned untouched, so an + * archive (or an iOS batch) that already speaks the table's unit is + * byte-for-byte unchanged. + * - Unknown unit, empty unit, or two units from different families → + * `null`, and every caller falls back to the raw value. A unit we + * cannot prove a factor for must never silently rescale a reading. + * - Only units Apple's `HKUnit` can actually emit are listed. A unit + * string is matched exactly (after trimming and after stripping the + * molar-mass annotation Apple writes on glucose), never lower-cased: + * `cal` (small calorie) and `Cal` (kilocalorie) are different units + * that differ only in case. + */ + +interface HkUnitSpec { + /** Units convert only within one family. */ + family: string; + /** This unit → the family's base unit. */ + toBase: (value: number) => number; + /** The family's base unit → this unit. */ + fromBase: (value: number) => number; +} + +/** A unit that is a plain multiple of its family's base unit. */ +function scaled(family: string, perBase: number): HkUnitSpec { + return { + family, + toBase: (value) => value * perBase, + fromBase: (value) => value / perBase, + }; +} + +/** + * Every unit string Apple emits that HealthLog can convert, keyed by the + * exact `HKUnit.unitString` spelling. Base units: metre, kilogram, + * kilocalorie, second, degree Celsius, mmHg, mg/dL, metres per second. + */ +const HK_UNITS: Record = { + // ── Length (base: metre) ─────────────────────────────────── + m: scaled("length", 1), + km: scaled("length", 1000), + cm: scaled("length", 0.01), + mm: scaled("length", 0.001), + in: scaled("length", 0.0254), + ft: scaled("length", 0.3048), + yd: scaled("length", 0.9144), + mi: scaled("length", 1609.344), + + // ── Mass (base: kilogram) ────────────────────────────────── + kg: scaled("mass", 1), + g: scaled("mass", 0.001), + lb: scaled("mass", 0.45359237), + oz: scaled("mass", 0.028349523125), + st: scaled("mass", 6.35029318), + + // ── Energy (base: kilocalorie) ───────────────────────────── + kcal: scaled("energy", 1), + Cal: scaled("energy", 1), // Apple's large calorie == kilocalorie + cal: scaled("energy", 0.001), // small calorie — case is load-bearing + kJ: scaled("energy", 1 / 4.184), + J: scaled("energy", 1 / 4184), + + // ── Time (base: second) ──────────────────────────────────── + s: scaled("time", 1), + ms: scaled("time", 0.001), + min: scaled("time", 60), + hr: scaled("time", 3600), + d: scaled("time", 86_400), + + // ── Pressure (base: mmHg) ────────────────────────────────── + mmHg: scaled("pressure", 1), + kPa: scaled("pressure", 7.500_616_827_041_698), + Pa: scaled("pressure", 0.007_500_616_827_041_698), + + // ── Blood glucose (base: mg/dL) ──────────────────────────── + // Apple annotates the molar unit with glucose's molar mass, which the + // normaliser strips before the lookup. + "mg/dL": scaled("glucose", 1), + "mmol/L": scaled("glucose", 18.0156), // 180.156 g/mol ÷ 10 + + // ── Speed (base: metres per second) ──────────────────────── + "m/s": scaled("speed", 1), + "km/hr": scaled("speed", 1000 / 3600), + "mi/hr": scaled("speed", 1609.344 / 3600), + + // ── Temperature (base: degree Celsius) — affine, not scaled ─ + degC: { + family: "temperature", + toBase: (value) => value, + fromBase: (value) => value, + }, + degF: { + family: "temperature", + toBase: (value) => ((value - 32) * 5) / 9, + fromBase: (value) => (value * 9) / 5 + 32, + }, + K: { + family: "temperature", + toBase: (value) => value - 273.15, + fromBase: (value) => value + 273.15, + }, +}; + +/** + * Apple writes molar units with the substance's molar mass inline — + * `mmol<180.156>/L` for blood glucose. The annotation is informational; + * strip it so the lookup sees the plain unit. + */ +const MOLAR_MASS_ANNOTATION = /<[^>]*>/g; + +function normaliseUnitString(unit: string): string { + return unit.trim().replace(MOLAR_MASS_ANNOTATION, ""); +} + +function lookup(unit: string | null | undefined): HkUnitSpec | null { + if (typeof unit !== "string") return null; + const normalised = normaliseUnitString(unit); + if (!normalised) return null; + return HK_UNITS[normalised] ?? null; +} + +/** + * `true` when this module knows a conversion factor for the unit — i.e. + * a `` that carries a sibling unit from the same family will be + * converted rather than trusted blindly. The mapping-table guard in + * `src/__tests__/apple-health-unit-audit-guard.test.ts` uses this to + * decide whether an entry's fixed `hkUnit` needs a written reason. + */ +export function isConvertibleHkUnit(unit: string | null | undefined): boolean { + return lookup(unit) !== null; +} + +/** + * Convert a HealthKit value between two `HKUnit` strings. Returns `null` + * — never a guess — when either unit is unknown or the two belong to + * different families; callers keep the raw value in that case. + */ +export function convertHkValue( + value: number, + fromUnit: string | null | undefined, + toUnit: string, +): number | null { + if (!Number.isFinite(value)) return null; + const from = lookup(fromUnit); + if (!from) return null; + const to = lookup(toUnit); + if (!to) return null; + if (from.family !== to.family) return null; + // Same unit: return the reading untouched rather than round-tripping it + // through the base and picking up a float error. + if (normaliseUnitString(String(fromUnit)) === normaliseUnitString(toUnit)) { + return value; + } + return to.fromBase(from.toBase(value)); +} + +/** + * Convert a distance a HealthKit archive reports in its own unit into + * metres, the canonical DB unit. Falls back to the raw number for a unit + * this module cannot place (including the empty attribute an archive may + * write) — the pre-#944 behaviour of the workout path, preserved. + */ +export function hkDistanceToMetres( + value: number, + unit: string | null | undefined, +): number { + return convertHkValue(value, unit, "m") ?? value; +} diff --git a/src/lib/measurements/import-apple-health-export.ts b/src/lib/measurements/import-apple-health-export.ts index 26a1ce0bb..4a4bb9229 100644 --- a/src/lib/measurements/import-apple-health-export.ts +++ b/src/lib/measurements/import-apple-health-export.ts @@ -42,6 +42,7 @@ import { } from "@/lib/measurements/drain-per-sample-cumulative"; import { reconcileExternalMeasurement } from "@/lib/measurements/reconcile-external-measurement"; import { resolveHkWorkoutSportType } from "@/lib/measurements/hk-workout-activity-type-map"; +import { hkDistanceToMetres } from "@/lib/measurements/hk-units"; import { emitInsertedMeasurementArrivals } from "@/lib/arrivals/measurement-emit"; import { maybeEnqueueMorningRefresh } from "@/lib/daily/morning-refresh-trigger"; import { emitDataArrival } from "@/lib/arrivals/emit-shared"; @@ -947,14 +948,12 @@ export async function streamParseExportXml( const totalDistance = Number.parseFloat(attrs.totalDistance ?? ""); const totalEnergy = Number.parseFloat(attrs.totalEnergyBurned ?? ""); const distanceUnit = attrs.totalDistanceUnit ?? ""; - // Apple ships km for HKWorkout.totalDistance by default; metres - // is the canonical DB unit. Convert when the unit is km. + // Apple ships the ACCOUNT's own length unit on HKWorkout.totalDistance + // (km for a metric account, mi for an imperial one); metres is the + // canonical DB unit. issue #944 — the same conversion the `` + // path now runs, so the two can never drift apart again. const distanceM = Number.isFinite(totalDistance) - ? distanceUnit === "km" - ? totalDistance * 1000 - : distanceUnit === "mi" - ? totalDistance * 1609.344 - : totalDistance + ? hkDistanceToMetres(totalDistance, distanceUnit) : null; const externalId = hashSampleKey( From bc7eff83222a18225e54a602a75e600bd920e0a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 9 Sep 2026 22:45:13 +0200 Subject: [PATCH 03/10] feat(ops): repair walking distances stored a thousand times too small MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unit fix only helps the next import. An account that imported an archive before it carries a decade of days understated 1000x, and the authoritative repair — re-importing the archive on the fixed build — needs an archive the account may no longer have. `scripts/repair-apple-health-distance.ts` is dry-run by default and selects rows only by the stamp the archive fold itself writes: the EXPORT_XML_SOURCE_MAX provenance plus the `stats:` external id minted for that fold. Never by "the value looks small", so a row a native sync has since overwritten is left alone. `--apply` multiplies one account per transaction and writes the audit row in the same transaction, which is what makes a second run a no-op. The archive's own unit is named by the operator (`--unit=mi`) because the discarded attribute is the defect itself, and a row whose repaired value would leave the plausible range is reported rather than written. Refs #944 --- docs/ops/apple-health-distance-repair.md | 121 +++++++++ scripts/README.md | 1 + scripts/repair-apple-health-distance.ts | 138 ++++++++++ .../repair-apple-health-distance.ts | 248 ++++++++++++++++++ .../apple-health-distance-repair.test.ts | 231 ++++++++++++++++ 5 files changed, 739 insertions(+) create mode 100644 docs/ops/apple-health-distance-repair.md create mode 100644 scripts/repair-apple-health-distance.ts create mode 100644 src/lib/measurements/repair-apple-health-distance.ts create mode 100644 tests/integration/apple-health-distance-repair.test.ts diff --git a/docs/ops/apple-health-distance-repair.md b/docs/ops/apple-health-distance-repair.md new file mode 100644 index 000000000..28b5c1c37 --- /dev/null +++ b/docs/ops/apple-health-distance-repair.md @@ -0,0 +1,121 @@ +# Apple Health walking-distance repair + +`scripts/repair-apple-health-distance.ts` heals walking distances that the +`export.zip` import stored a thousand times too small. + +## The defect + +Apple stamps every quantity `` in an `export.xml` with the account's +own display unit: a metric archive reports `HKQuantityTypeIdentifierDistance- +WalkingRunning` in `km`, an imperial one in `mi`. Until this release the record +mapping accepted that attribute and never read it, so the day's total was +folded as if the number were already metres. A 2.484 km day landed as +`2.484 m`, and an imported decade reads as a couple of metres walked. The +workout path was never affected — it converted its own distance unit. + +The code fix converts the record's unit before storage, so every import from +this release on is correct. This script is for the rows already in the +database. + +## Prefer a re-import + +If the account still has its `export.zip`, **re-import it**. The archive fold +UPSERTs the same `stats::` rows, so a re-import on the fixed +build rewrites every affected day with the right number, needs no flags, and +also repairs rows this script deliberately leaves alone (see the criterion +below). The script exists for accounts that no longer have the archive. + +## Which rows it touches + +Only rows carrying the stamp the archive importer itself writes: + +| Column | Value | +| ------------------------ | ------------------------------------------------------------ | +| `type` | `WALKING_RUNNING_DISTANCE` | +| `source` | `APPLE_HEALTH` | +| `aggregation_provenance` | `EXPORT_XML_SOURCE_MAX` | +| `external_id` | `stats:HKQuantityTypeIdentifierDistanceWalkingRunning:` | +| `deleted_at` | `NULL` | + +`EXPORT_XML_SOURCE_MAX` is written in exactly one place in the tree — the +archive fold — and the `stats:` external id is minted for that same fold, so +the pair is a proof of origin rather than a guess. Nothing is ever selected +because a value "looks too small". + +Deliberately out of scope: + +- Rows a native iOS sync has since overwritten (`HEALTHKIT_STATISTICS`): their + value came from HealthKit's own statistics query and is correct. +- Rows whose provenance is `LEGACY_UNKNOWN` or NULL: an archive origin cannot + be proven for them. Re-import the archive to repair those. + +## Run a dry-run first + +Always. The default mode reports and never writes: + +```bash +# from a source checkout, with DATABASE_URL pointing at the instance +pnpm dlx tsx scripts/repair-apple-health-distance.ts +``` + +It prints one line per account: how many rows would be repaired, the date span +they cover, one worked example (`2.484 m -> 2484 m`), and any row whose +repaired value would leave the plausibility range. + +This script imports the application dependency graph, so it runs from a source +checkout, not through the production image's `healthlog-tsx` launcher. + +## Name the archive's unit + +The stored row keeps no record of the unit its archive was written in — that +is the defect — so the operator names it. `--unit=km` is the default and is +what the Health app writes in every metric locale; pass `--unit=mi` for an +imperial archive: + +```bash +pnpm dlx tsx scripts/repair-apple-health-distance.ts --unit=mi +``` + +Check the dry-run's worked example against what the account's Health app shows +for that day before applying. If the two units are mixed across accounts, run +the script once per unit with `--apply` and confirm each account's example +first. + +## Apply + +```bash +pnpm dlx tsx scripts/repair-apple-health-distance.ts --apply +``` + +Per account, in one transaction: every selected row multiplied by the unit's +metre factor, `sync_version` incremented and `updated_at` bumped so paired iOS +clients pick the rows up on their next delta sync, and an audit row +(`measurement.apple_health_distance.repaired`) recording the unit, the factor +and the row count. The DAY/WEEK/MONTH/YEAR rollups and the cached status +insights for the touched days are recomputed afterwards. + +A row whose repaired value would leave the plausible range (0–200 000 m) is +reported and never written — several of those in one account mean the rows are +not the 1000x class and the run should be stopped. + +## It runs once per account + +The audit row is written inside the same transaction as the update, and an +account that carries it is skipped. A second run reports the account as already +repaired and changes nothing, so the multiplier can never be applied twice. + +## Verify + +```sql +SELECT date_trunc('year', measured_at) AS year, + round(avg(value)) AS avg_metres_per_day + FROM measurements + WHERE user_id = '' + AND type = 'WALKING_RUNNING_DISTANCE' + AND aggregation_provenance = 'EXPORT_XML_SOURCE_MAX' + GROUP BY 1 ORDER BY 1; +``` + +A plausible daily average is thousands of metres. Single digits mean the rows +were not repaired; millions mean a factor was applied twice, which the audit +row is there to prevent. diff --git a/scripts/README.md b/scripts/README.md index c4280328d..9eb2b0e6a 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -24,6 +24,7 @@ typecheck. - **`assert-deploy.ts`** — post-deploy `/api/version` assertion. - **`rotate-encryption-key.ts`** — AES-256-GCM key rotation; see `docs/ops/encryption-key-rotation.md`. - **`restore-backup.ts`** — restore an off-host AES-GCM backup from an S3-compatible bucket. +- **`repair-apple-health-distance.ts`** — repair walking-distance rows the `export.zip` import stored 1000x too small; dry-run by default, see `docs/ops/apple-health-distance-repair.md`. - **`repair-intake-anomalies.ts`** — repair historic medication-ledger defects (duplicate dose-slot rows, implausible `taken_at`); dry-run by default, see `docs/ops/intake-repair.md`. - **`backfill-rollups.ts`**, **`drain-per-sample-cumulative.ts`**, **`backfill-mood-note-column.ts`** — historical data backfills (note: the boot-time `rollup-full-backfill` queue handles new accounts automatically; the CLI is the manual fallback). - **`seed-demo.ts`** + **`seed-demo-v15.sql`** — populate a demo tenant. diff --git a/scripts/repair-apple-health-distance.ts b/scripts/repair-apple-health-distance.ts new file mode 100644 index 000000000..2abf0f45e --- /dev/null +++ b/scripts/repair-apple-health-distance.ts @@ -0,0 +1,138 @@ +/** + * scripts/repair-apple-health-distance.ts — operator repair for walking + * distances the `export.zip` import stored 1000x too small (issue #944). + * + * THE DEFECT (fixed in code from this release): the `` mapping read + * the value but never the record's own `unit` attribute, so a walking-distance + * record Apple writes in the account's display unit (`km` on a metric archive, + * `mi` on an imperial one) was folded into the day's total as if the number + * were already metres. Ten years of history read as a couple of metres a day. + * + * WHICH ROWS: only the ones carrying the stamp the archive importer writes — + * `WALKING_RUNNING_DISTANCE` + `APPLE_HEALTH` + provenance + * `EXPORT_XML_SOURCE_MAX` + a `stats:HKQuantityTypeIdentifierDistanceWalking- + * Running:` externalId. Never a guess from "the value looks small". Rows + * a native iOS sync has since overwritten carry `HEALTHKIT_STATISTICS` and are + * correct; rows whose provenance cannot prove an archive origin are left alone. + * + * If the account still has its `export.zip`, RE-IMPORTING it on the fixed build + * is the authoritative repair — the fold UPSERTs the same `stats:` rows with + * the right numbers. This script is for the accounts that no longer have it. + * + * SAFETY: + * - Dry-run by DEFAULT: lists the candidate rows per account and exits. + * - `--apply` writes, one transaction per account, and records an audit row + * in that same transaction. An account carrying that row is skipped, so a + * second run finds nothing. + * - A row whose repaired value would leave the plausibility range is + * reported and never written. + * - `--unit=mi` for an imperial archive (default `km`). The stored row keeps + * no record of the archive's unit — that is the defect — so the operator + * names it; a mismatched unit is why the flag exists rather than a guess. + * + * RUN (never `pnpm tsx` — the standalone image strips tsx): + * pnpm dlx tsx scripts/repair-apple-health-distance.ts # dry run + * pnpm dlx tsx scripts/repair-apple-health-distance.ts --apply + * pnpm dlx tsx scripts/repair-apple-health-distance.ts --unit=mi --apply + */ +import "dotenv/config"; + +import { prisma } from "@/lib/db"; +import { + applyAppleHealthDistanceRepair, + parseArchiveUnit, + planAppleHealthDistanceRepair, + repairFactorFor, +} from "@/lib/measurements/repair-apple-health-distance"; + +function readArchiveUnit(): "km" | "mi" { + const flag = process.argv.find((arg) => arg.startsWith("--unit=")); + if (!flag) return "km"; + const parsed = parseArchiveUnit(flag.slice("--unit=".length)); + if (!parsed) { + throw new Error(`Unsupported --unit value: ${flag}. Use km or mi.`); + } + return parsed; +} + +async function main(): Promise { + const apply = process.argv.includes("--apply"); + const archiveUnit = readArchiveUnit(); + const factor = repairFactorFor(archiveUnit); + + const plans = await planAppleHealthDistanceRepair(prisma, { archiveUnit }); + if (plans.length === 0) { + console.log( + "No export-imported walking-distance rows found — nothing to repair.", + ); + return; + } + + console.log( + `Archive unit ${archiveUnit} (x${factor} to metres). ` + + `${plans.length} account(s) carry export-imported walking distances.\n`, + ); + + let totalRepairable = 0; + let totalUpdated = 0; + let totalSkipped = 0; + + for (const plan of plans) { + if (plan.alreadyRepairedAt) { + console.log( + ` • ${plan.userId}: already repaired on ` + + `${plan.alreadyRepairedAt.toISOString()} — ${plan.candidateCount} row(s) left alone.`, + ); + continue; + } + + totalRepairable += plan.repairable.length; + const first = plan.repairable[0]; + const last = plan.repairable[plan.repairable.length - 1]; + console.log( + ` • ${plan.userId}: ${plan.repairable.length} row(s)` + + (first && last + ? ` from ${first.measuredAt.toISOString().slice(0, 10)} to ` + + `${last.measuredAt.toISOString().slice(0, 10)}` + : "") + + (plan.outOfRange.length > 0 + ? `, ${plan.outOfRange.length} out of plausible range (skipped)` + : ""), + ); + if (first) { + console.log( + ` e.g. ${first.id}: ${first.value} m -> ${first.value * factor} m`, + ); + } + for (const row of plan.outOfRange) { + console.log( + ` ! ${row.id} (${row.measuredAt.toISOString().slice(0, 10)}): ` + + `${row.value} m x ${factor} leaves the plausible range — not written`, + ); + } + + if (!apply) continue; + const outcome = await applyAppleHealthDistanceRepair(prisma, plan, { + archiveUnit, + }); + totalUpdated += outcome.updated; + totalSkipped += outcome.skipped; + console.log( + ` ✓ repaired ${outcome.updated} row(s), skipped ${outcome.skipped}`, + ); + } + + console.log( + apply + ? `\nDone. ${totalUpdated} row(s) repaired, ${totalSkipped} skipped.` + : `\nDry run: ${totalRepairable} row(s) would be repaired. ` + + "Re-run with --apply to write.", + ); +} + +main() + .catch((err) => { + console.error(err); + process.exitCode = 1; + }) + .finally(() => prisma.$disconnect()); diff --git a/src/lib/measurements/repair-apple-health-distance.ts b/src/lib/measurements/repair-apple-health-distance.ts new file mode 100644 index 000000000..7257fa475 --- /dev/null +++ b/src/lib/measurements/repair-apple-health-distance.ts @@ -0,0 +1,248 @@ +/** + * Repair for walking distances the `export.zip` import stored 1000x too + * small (issue #944). + * + * THE DEFECT (fixed in code from this release): `mapAppleHealthEntry()` + * accepted the `` attribute and never read it, so a + * `HKQuantityTypeIdentifierDistanceWalkingRunning` record — which Apple + * writes in the account's own display unit, `km` on a metric archive — + * was folded into the day's total as if the number were already metres. + * Every day of an imported history reads as a couple of metres walked. + * + * THE CODE FIX converts the record's own unit into the table's `hkUnit` + * before storage, so every FUTURE import is right. This module heals what + * is already in the database. + * + * ── The criterion ────────────────────────────────────────────────── + * + * A row is a candidate only when it carries the stamp the export importer + * itself writes, never because its value "looks small": + * + * type = WALKING_RUNNING_DISTANCE + * source = APPLE_HEALTH + * aggregationProvenance = EXPORT_XML_SOURCE_MAX + * externalId LIKE stats:HKQuantityTypeIdentifierDistanceWalkingRunning:% + * deletedAt IS NULL + * + * `EXPORT_XML_SOURCE_MAX` is written in exactly one place — the archive + * fold in `import-apple-health-export.ts` — and the `stats::` + * externalId is minted by `dailyStatsExternalId()` for that same fold. A + * row a native iOS sync has since overwritten carries + * `HEALTHKIT_STATISTICS` and its value came from HealthKit's own + * statistics query, so it is correct and out of scope. A row whose + * provenance is `LEGACY_UNKNOWN` or NULL cannot be PROVEN to have come + * from the archive path and is therefore left alone: re-importing the + * archive on the fixed build repairs those, and is the authoritative + * repair for any account that still has its `export.zip`. + * + * ── Idempotency ──────────────────────────────────────────────────── + * + * Applying the repair writes one `AuditLog` row per account inside the + * same transaction as the update. An account that already carries that + * row is skipped, so a second run finds nothing to do — the multiplier + * can never be applied twice. + */ +import type { PrismaClient } from "@/generated/prisma/client"; +import { auditLog } from "@/lib/auth/audit"; +import { afterMeasurementMutation } from "@/lib/rollups/after-measurement-mutation"; +import { validateMeasurementRange } from "@/lib/validations/measurement"; + +import { dailyStatsExternalId } from "./apple-health-mapping"; +import { convertHkValue } from "./hk-units"; + +/** The audit action that marks an account's distances as repaired. */ +export const APPLE_HEALTH_DISTANCE_REPAIR_ACTION = + "measurement.apple_health_distance.repaired"; + +/** Minted by the same helper the export fold uses, so the two cannot drift. */ +export const EXPORT_DISTANCE_EXTERNAL_ID_PREFIX = dailyStatsExternalId( + "HKQuantityTypeIdentifierDistanceWalkingRunning", + "", +); + +/** + * The unit the affected archive was written in. Apple writes `km` on a + * metric account and `mi` on an imperial one, and the stored row keeps no + * record of which — the importer discarded the attribute, which is the + * defect. The operator names it; `km` is the default because it is what + * the Health app writes for every metric locale. + */ +export type ArchiveDistanceUnit = "km" | "mi"; + +export interface RepairCandidateRow { + id: string; + measuredAt: Date; + value: number; +} + +export interface AccountRepairPlan { + userId: string; + /** Candidate rows that would be multiplied. */ + repairable: RepairCandidateRow[]; + /** + * Candidates whose repaired value would leave the plausibility range + * (`validateMeasurementRange`) — reported, never written. A row here is + * a sign the account's rows are NOT the 1000x class. + */ + outOfRange: RepairCandidateRow[]; + /** Non-null when this account already carries the repair's audit row. */ + alreadyRepairedAt: Date | null; + /** Candidate rows found, whatever the disposition above. */ + candidateCount: number; +} + +/** The DB surface this module needs — the script hands it the real client. */ +export type RepairClient = Pick< + PrismaClient, + "measurement" | "auditLog" | "$transaction" +>; + +const REPAIRED_TYPE = "WALKING_RUNNING_DISTANCE" as const; + +/** metres per unit of the archive's own length unit. */ +export function repairFactorFor(unit: ArchiveDistanceUnit): number { + const factor = convertHkValue(1, unit, "m"); + // The two archive units are length units by construction; the shared + // table is the single source of the factor either way. + if (factor === null) throw new Error(`no metre factor for unit "${unit}"`); + return factor; +} + +/** + * List the repair candidates per account. Read-only — this is what the + * script's dry run prints. + */ +export async function planAppleHealthDistanceRepair( + client: RepairClient, + options: { archiveUnit: ArchiveDistanceUnit }, +): Promise { + const factor = repairFactorFor(options.archiveUnit); + + const rows = await client.measurement.findMany({ + where: { + type: REPAIRED_TYPE, + source: "APPLE_HEALTH", + aggregationProvenance: "EXPORT_XML_SOURCE_MAX", + externalId: { startsWith: EXPORT_DISTANCE_EXTERNAL_ID_PREFIX }, + deletedAt: null, + }, + select: { id: true, userId: true, value: true, measuredAt: true }, + orderBy: [{ userId: "asc" }, { measuredAt: "asc" }], + }); + if (rows.length === 0) return []; + + const userIds = Array.from(new Set(rows.map((row) => row.userId))); + const repaired = await client.auditLog.findMany({ + where: { + action: APPLE_HEALTH_DISTANCE_REPAIR_ACTION, + userId: { in: userIds }, + }, + select: { userId: true, createdAt: true }, + orderBy: { createdAt: "asc" }, + }); + const repairedAt = new Map(); + for (const entry of repaired) { + if (entry.userId) repairedAt.set(entry.userId, entry.createdAt); + } + + const plans = new Map(); + for (const row of rows) { + let plan = plans.get(row.userId); + if (!plan) { + plan = { + userId: row.userId, + repairable: [], + outOfRange: [], + alreadyRepairedAt: repairedAt.get(row.userId) ?? null, + candidateCount: 0, + }; + plans.set(row.userId, plan); + } + plan.candidateCount += 1; + if (plan.alreadyRepairedAt) continue; + const candidate: RepairCandidateRow = { + id: row.id, + measuredAt: row.measuredAt, + value: row.value, + }; + if (validateMeasurementRange(REPAIRED_TYPE, row.value * factor) !== null) { + plan.outOfRange.push(candidate); + } else { + plan.repairable.push(candidate); + } + } + return Array.from(plans.values()); +} + +export interface RepairOutcome { + userId: string; + updated: number; + skipped: number; +} + +/** + * Apply one account's plan: every repairable row multiplied by the + * archive unit's metre factor in ONE transaction, together with the audit + * row that makes a second run a no-op. Returns what it did. + */ +export async function applyAppleHealthDistanceRepair( + client: RepairClient, + plan: AccountRepairPlan, + options: { archiveUnit: ArchiveDistanceUnit }, +): Promise { + const factor = repairFactorFor(options.archiveUnit); + const skipped = plan.outOfRange.length; + if (plan.alreadyRepairedAt || plan.repairable.length === 0) { + return { userId: plan.userId, updated: 0, skipped }; + } + + const ids = plan.repairable.map((row) => row.id); + await client.$transaction(async (tx) => { + // One statement per chunk rather than one per row: an account with a + // decade of history carries thousands of rows and an interactive + // transaction has a clock on it. `sync_version` + `updated_at` are + // bumped by hand because raw SQL bypasses Prisma's `@updatedAt`, and + // the iOS delta feed pages on exactly those two columns. + for (let i = 0; i < ids.length; i += 500) { + const chunk = ids.slice(i, i + 500); + await tx.$executeRaw` + UPDATE measurements + SET value = value * ${factor}, + unit = 'm', + sync_version = sync_version + 1, + updated_at = NOW() + WHERE id = ANY(${chunk})`; + } + await auditLog(APPLE_HEALTH_DISTANCE_REPAIR_ACTION, { + client: tx, + userId: plan.userId, + actorUserId: null, + details: { + type: REPAIRED_TYPE, + archiveUnit: options.archiveUnit, + factor, + rows: ids.length, + skippedOutOfRange: skipped, + issue: 944, + }, + }); + }); + + // Best-effort tail: the DAY/WEEK/MONTH/YEAR rollups and the cached + // status insights hold the pre-repair numbers until they are recomputed. + await afterMeasurementMutation( + plan.userId, + plan.repairable.map((row) => ({ + type: REPAIRED_TYPE, + measuredAt: row.measuredAt, + })), + "repair-apple-health-distance", + ); + + return { userId: plan.userId, updated: ids.length, skipped }; +} + +/** Narrowing helper for the script's `--unit` flag. */ +export function parseArchiveUnit(raw: string): ArchiveDistanceUnit | null { + return raw === "km" || raw === "mi" ? raw : null; +} diff --git a/tests/integration/apple-health-distance-repair.test.ts b/tests/integration/apple-health-distance-repair.test.ts new file mode 100644 index 000000000..d4fa7bbea --- /dev/null +++ b/tests/integration/apple-health-distance-repair.test.ts @@ -0,0 +1,231 @@ +/** + * Integration coverage for the export-import walking-distance repair + * (issue #944). + * + * The defect stored every archive-imported `WALKING_RUNNING_DISTANCE` row a + * thousand times too small. `src/lib/measurements/repair-apple-health-distance.ts` + * heals what is already in the database, and everything that matters about it + * is a database property: which rows the criterion actually selects, that the + * multiply lands in one transaction with its audit row, and that a second run + * finds nothing. None of that can be proven against a mock, so this runs + * against the testcontainer Postgres (Docker / OrbStack; `pnpm test:integration`). + */ +import { beforeEach, describe, expect, it } from "vitest"; + +import { getPrismaClient, truncateAllTables } from "./setup"; +import { + APPLE_HEALTH_DISTANCE_REPAIR_ACTION, + applyAppleHealthDistanceRepair, + planAppleHealthDistanceRepair, +} from "@/lib/measurements/repair-apple-health-distance"; +import { settleBackgroundTasks } from "@/lib/logging/background-tasks"; +import type { + MeasurementAggregationProvenance, + MeasurementSource, + MeasurementType, + PrismaClient, +} from "@/generated/prisma/client"; + +const DISTANCE_PREFIX = "stats:HKQuantityTypeIdentifierDistanceWalkingRunning:"; + +let dayCounter = 0; + +async function seedRow( + prisma: PrismaClient, + userId: string, + overrides: { + value: number; + type?: MeasurementType; + source?: MeasurementSource; + provenance?: MeasurementAggregationProvenance | null; + externalId?: string; + }, +): Promise { + dayCounter += 1; + const measuredAt = new Date(Date.UTC(2026, 0, dayCounter, 12, 0, 0)); + const day = measuredAt.toISOString().slice(0, 10); + const row = await prisma.measurement.create({ + data: { + userId, + type: overrides.type ?? "WALKING_RUNNING_DISTANCE", + value: overrides.value, + unit: "m", + source: overrides.source ?? "APPLE_HEALTH", + measuredAt, + externalId: overrides.externalId ?? `${DISTANCE_PREFIX}${day}`, + aggregationProvenance: + overrides.provenance === undefined + ? "EXPORT_XML_SOURCE_MAX" + : overrides.provenance, + }, + select: { id: true }, + }); + return row.id; +} + +async function readRow(prisma: PrismaClient, id: string) { + return prisma.measurement.findUniqueOrThrow({ + where: { id }, + select: { value: true, unit: true, syncVersion: true }, + }); +} + +async function makeUser(prisma: PrismaClient, tag: string): Promise { + const user = await prisma.user.create({ + data: { + username: `distance-repair-${tag}`, + email: `distance-repair-${tag}@example.test`, + role: "USER", + }, + }); + return user.id; +} + +describe("Apple Health export distance repair — integration", () => { + beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + dayCounter = 0; + }); + + it("selects only rows the archive importer stamped, and repairs them once", async () => { + const prisma = getPrismaClient(); + const userId = await makeUser(prisma, "km"); + + const kmDay = await seedRow(prisma, userId, { value: 2.484 }); + const otherDay = await seedRow(prisma, userId, { value: 7.02 }); + // Decoys, one per arm of the criterion. + const nativeStats = await seedRow(prisma, userId, { + value: 8123, + provenance: "HEALTHKIT_STATISTICS", + }); + const legacyProvenance = await seedRow(prisma, userId, { + value: 3.4, + provenance: null, + }); + const manualRow = await seedRow(prisma, userId, { + value: 4.1, + source: "MANUAL", + }); + const otherType = await seedRow(prisma, userId, { + value: 9000, + type: "ACTIVITY_STEPS", + externalId: "stats:HKQuantityTypeIdentifierStepCount:2026-01-09", + }); + const perSample = await seedRow(prisma, userId, { + value: 1.2, + externalId: "sample:abc123", + }); + + const plans = await planAppleHealthDistanceRepair(prisma, { + archiveUnit: "km", + }); + expect(plans).toHaveLength(1); + expect(plans[0].userId).toBe(userId); + expect(plans[0].repairable.map((row) => row.id).sort()).toEqual( + [kmDay, otherDay].sort(), + ); + expect(plans[0].alreadyRepairedAt).toBeNull(); + + const outcome = await applyAppleHealthDistanceRepair(prisma, plans[0], { + archiveUnit: "km", + }); + await settleBackgroundTasks(); + expect(outcome).toMatchObject({ userId, updated: 2, skipped: 0 }); + + expect((await readRow(prisma, kmDay)).value).toBeCloseTo(2484, 6); + expect((await readRow(prisma, otherDay)).value).toBeCloseTo(7020, 6); + // The delta feed pages on sync_version / updated_at, so a repaired row + // has to look mutated to a paired client. + expect((await readRow(prisma, kmDay)).syncVersion).toBe(2); + expect((await readRow(prisma, kmDay)).unit).toBe("m"); + + // Every decoy untouched. + expect((await readRow(prisma, nativeStats)).value).toBe(8123); + expect((await readRow(prisma, legacyProvenance)).value).toBe(3.4); + expect((await readRow(prisma, manualRow)).value).toBe(4.1); + expect((await readRow(prisma, otherType)).value).toBe(9000); + expect((await readRow(prisma, perSample)).value).toBe(1.2); + + const audit = await prisma.auditLog.findMany({ + where: { action: APPLE_HEALTH_DISTANCE_REPAIR_ACTION, userId }, + }); + expect(audit).toHaveLength(1); + expect(JSON.parse(audit[0].details ?? "{}")).toMatchObject({ + archiveUnit: "km", + factor: 1000, + rows: 2, + issue: 944, + }); + }); + + it("is idempotent — a second run finds nothing to repair", async () => { + const prisma = getPrismaClient(); + const userId = await makeUser(prisma, "twice"); + const rowId = await seedRow(prisma, userId, { value: 2.484 }); + + const first = await planAppleHealthDistanceRepair(prisma, { + archiveUnit: "km", + }); + await applyAppleHealthDistanceRepair(prisma, first[0], { + archiveUnit: "km", + }); + await settleBackgroundTasks(); + + const second = await planAppleHealthDistanceRepair(prisma, { + archiveUnit: "km", + }); + expect(second).toHaveLength(1); + expect(second[0].repairable).toHaveLength(0); + expect(second[0].candidateCount).toBe(1); + expect(second[0].alreadyRepairedAt).toBeInstanceOf(Date); + + const outcome = await applyAppleHealthDistanceRepair(prisma, second[0], { + archiveUnit: "km", + }); + expect(outcome.updated).toBe(0); + expect((await readRow(prisma, rowId)).value).toBeCloseTo(2484, 6); + expect((await readRow(prisma, rowId)).syncVersion).toBe(2); + }); + + it("uses the archive's own unit and never writes an implausible value", async () => { + const prisma = getPrismaClient(); + const userId = await makeUser(prisma, "mi"); + const mile = await seedRow(prisma, userId, { value: 1.543 }); + const absurd = await seedRow(prisma, userId, { value: 250 }); + + const plans = await planAppleHealthDistanceRepair(prisma, { + archiveUnit: "mi", + }); + expect(plans[0].repairable.map((row) => row.id)).toEqual([mile]); + expect(plans[0].outOfRange.map((row) => row.id)).toEqual([absurd]); + + const outcome = await applyAppleHealthDistanceRepair(prisma, plans[0], { + archiveUnit: "mi", + }); + await settleBackgroundTasks(); + expect(outcome).toMatchObject({ updated: 1, skipped: 1 }); + expect((await readRow(prisma, mile)).value).toBeCloseTo(2483.217792, 6); + expect((await readRow(prisma, absurd)).value).toBe(250); + }); + + it("plans one entry per account", async () => { + const prisma = getPrismaClient(); + const first = await makeUser(prisma, "a"); + const second = await makeUser(prisma, "b"); + await seedRow(prisma, first, { value: 1.5 }); + await seedRow(prisma, second, { value: 2.5 }); + await seedRow(prisma, second, { value: 3.5 }); + + const plans = await planAppleHealthDistanceRepair(prisma, { + archiveUnit: "km", + }); + expect( + plans.map((plan) => [plan.userId, plan.repairable.length]).sort(), + ).toEqual( + [ + [first, 1], + [second, 2], + ].sort(), + ); + }); +}); From bdfff049750505af89dc4ce96a135a6d4029fbb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 9 Sep 2026 22:45:17 +0200 Subject: [PATCH 04/10] docs(api): say what the batch ingest does with a sample's unit The field carried no description while the server ignored it. It no longer ignores it, so the contract has to say so. Refs #944 --- docs/api/openapi.yaml | 3 +++ src/lib/openapi/routes/measurements.ts | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 72915fbc3..4e2fbce92 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -21350,6 +21350,9 @@ components: type: string minLength: 1 maxLength: 60 + description: Apple's `HKUnit` string for `value`. The server converts the reading into the unit it stores the identifier + in when the two differ and the factor is known (`km` to `m`, `lb` to `kg`, `degF` to `degC`, and their + siblings); a unit the server cannot place leaves the reading untouched. startDate: type: string format: date-time diff --git a/src/lib/openapi/routes/measurements.ts b/src/lib/openapi/routes/measurements.ts index ff655f406..756618059 100644 --- a/src/lib/openapi/routes/measurements.ts +++ b/src/lib/openapi/routes/measurements.ts @@ -53,7 +53,13 @@ const batchEntrySchema = z .describe( "v1.19.2 (iOS #34 extension) — the bucket's MAXIMUM bpm for a 10-minute heart-rate bucket (see `externalId`). Persisted ONLY on a well-formed `stats:HKQuantityTypeIdentifierHeartRate:` row; ignored (stored null) on every other entry. Omit on a pre-v1.19.2 client — the bucket keeps the avg-only contract.", ), - unit: z.string().min(1).max(60), + unit: z + .string() + .min(1) + .max(60) + .describe( + "Apple's `HKUnit` string for `value`. The server converts the reading into the unit it stores the identifier in when the two differ and the factor is known (`km` to `m`, `lb` to `kg`, `degF` to `degC`, and their siblings); a unit the server cannot place leaves the reading untouched.", + ), startDate: z.iso.datetime({ offset: true }), endDate: z.iso.datetime({ offset: true }), sleepStage: z From e031e2211c5756a5ac48d17f429a718694f6efeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 9 Sep 2026 22:47:05 +0200 Subject: [PATCH 05/10] fix(import): honour the energy unit an archived workout carries `totalEnergyBurned` rides the same account-display-unit contract as the distance beside it: a Health app set to kilojoules writes `kJ`, and the column is kilocalories. Convert it through the shared table. Refs #944 --- .../import-apple-health-export.test.ts | 32 +++++++++++++++++++ .../import-apple-health-export.ts | 14 ++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/lib/measurements/__tests__/import-apple-health-export.test.ts b/src/lib/measurements/__tests__/import-apple-health-export.test.ts index c63637ec9..607a332b2 100644 --- a/src/lib/measurements/__tests__/import-apple-health-export.test.ts +++ b/src/lib/measurements/__tests__/import-apple-health-export.test.ts @@ -1297,6 +1297,38 @@ describe("streamParseExportXml — the record's own unit (issue #944)", () => { expect(prisma._measurements[0].value).toBeCloseTo(2484, 6); }); + it("stores a kilojoule workout burn in kilocalories", async () => { + const tmp = mkdtempSync(join(tmpdir(), "healthlog-parser-energy-test-")); + const xmlPath = join(tmp, "export.xml"); + writeFileSync( + xmlPath, + ` + + +`, + ); + const prisma = makeFakePrisma(); + await streamParseExportXml({ + xmlPath, + userId: "user-energy", + userTimezone: "Europe/Berlin", + prisma: prisma as unknown as PrismaClient, + }); + + expect(prisma._workouts).toHaveLength(1); + expect(prisma._workouts[0].totalDistanceM).toBeCloseTo(6500, 6); + expect(prisma._workouts[0].totalEnergyKcal).toBeCloseTo(412.0458891, 6); + }); + it("stores a mile-scale walking distance in metres", async () => { const { prisma } = await importCumulativeFixture( cumulativeExportXml( diff --git a/src/lib/measurements/import-apple-health-export.ts b/src/lib/measurements/import-apple-health-export.ts index 4a4bb9229..ea14ad8fe 100644 --- a/src/lib/measurements/import-apple-health-export.ts +++ b/src/lib/measurements/import-apple-health-export.ts @@ -42,7 +42,10 @@ import { } from "@/lib/measurements/drain-per-sample-cumulative"; import { reconcileExternalMeasurement } from "@/lib/measurements/reconcile-external-measurement"; import { resolveHkWorkoutSportType } from "@/lib/measurements/hk-workout-activity-type-map"; -import { hkDistanceToMetres } from "@/lib/measurements/hk-units"; +import { + convertHkValue, + hkDistanceToMetres, +} from "@/lib/measurements/hk-units"; import { emitInsertedMeasurementArrivals } from "@/lib/arrivals/measurement-emit"; import { maybeEnqueueMorningRefresh } from "@/lib/daily/morning-refresh-trigger"; import { emitDataArrival } from "@/lib/arrivals/emit-shared"; @@ -948,6 +951,7 @@ export async function streamParseExportXml( const totalDistance = Number.parseFloat(attrs.totalDistance ?? ""); const totalEnergy = Number.parseFloat(attrs.totalEnergyBurned ?? ""); const distanceUnit = attrs.totalDistanceUnit ?? ""; + const energyUnit = attrs.totalEnergyBurnedUnit ?? ""; // Apple ships the ACCOUNT's own length unit on HKWorkout.totalDistance // (km for a metric account, mi for an imperial one); metres is the // canonical DB unit. issue #944 — the same conversion the `` @@ -969,7 +973,13 @@ export async function streamParseExportXml( startedAt: startDate, endedAt: endDate, durationSec, - totalEnergyKcal: Number.isFinite(totalEnergy) ? totalEnergy : null, + totalEnergyKcal: Number.isFinite(totalEnergy) + ? // issue #944 — the energy attribute carries the account's own + // unit too: a Health app set to kilojoules writes `kJ` here, and + // the column is kilocalories. Same shared conversion; an unknown + // unit leaves the number alone. + (convertHkValue(totalEnergy, energyUnit, "kcal") ?? totalEnergy) + : null, totalDistanceM: distanceM, externalId, externalSourceVersion: attrs.sourceVersion ?? null, From 65e9ecbde620d291caba6c63d48ca2ba900d942d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 00:24:59 +0200 Subject: [PATCH 06/10] fix(ingest): keep the batch sample's unit captured, not authoritative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The record-unit conversion is now opt-in per caller and only the `export.xml` archive path opts in. There the attribute is authoritative: Apple stamps every quantity `` with the account's own display unit, so `km` means kilometres and the reading is meaningless without it. `POST /api/measurements/batch` keeps its documented contract — `unit` is captured for audit and never read. Nothing in the tree pins the native client's unit strings for the seventeen convertible identifiers, and a client that reads a sample in metres while stamping the person's display unit into the field would have had every walking distance multiplied by a thousand on ingest. Reading the field there is a wire change and needs the client's strings pinned first. The mapping-table audit and the shared conversion module stay. The batch path's identity behaviour is written down where it is decided and pinned twice: every convertible identifier round-trips a sibling unit unchanged by default, and a walking-distance entry sent as `km` is stored as it arrived. The OpenAPI description says what the server actually does. Review finding H2. --- docs/api/openapi.yaml | 6 +- .../apple-health-unit-audit-guard.test.ts | 75 ++++++++++++++++--- .../__tests__/apple-health-mapping.test.ts | 32 ++++++++ src/lib/measurements/apple-health-mapping.ts | 53 ++++++++++--- .../import-apple-health-export.ts | 23 ++++-- src/lib/openapi/routes/measurements.ts | 2 +- 6 files changed, 161 insertions(+), 30 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 4e2fbce92..2eed38ee6 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -21350,9 +21350,9 @@ components: type: string minLength: 1 maxLength: 60 - description: Apple's `HKUnit` string for `value`. The server converts the reading into the unit it stores the identifier - in when the two differ and the factor is known (`km` to `m`, `lb` to `kg`, `degF` to `degC`, and their - siblings); a unit the server cannot place leaves the reading untouched. + description: "Apple's `HKUnit` string for `value`, captured for audit. The server does NOT convert from it: `value` is + read in the unit HealthLog stores the identifier in (`m` for walking distance, `kg` for body mass, `kcal` + for energy, `degC` for temperature). Send the sample in that unit." startDate: type: string format: date-time diff --git a/src/__tests__/apple-health-unit-audit-guard.test.ts b/src/__tests__/apple-health-unit-audit-guard.test.ts index 49e162269..2975cd92d 100644 --- a/src/__tests__/apple-health-unit-audit-guard.test.ts +++ b/src/__tests__/apple-health-unit-audit-guard.test.ts @@ -20,6 +20,12 @@ * Apple could write would change the reading (a dimensionless count, * a pinned event, a logarithmic dB level, a compound rate). * + * The conversion is opt-in per caller (`convertRecordUnit`), and only the + * archive path opts in: `POST /api/measurements/batch` documents its `unit` + * as captured-not-validated and nothing pins the native client's strings. + * The last block below holds that boundary, so the batch contract cannot be + * widened by accident. + * * The reason arm alone would be a guard that cannot fail: an entry could * declare a convertible unit while the mapper quietly ignored it. The * behavioural block below therefore feeds every convertible entry a @@ -122,8 +128,14 @@ describe("Apple Health mapping — the record's own unit (issue #944)", () => { startDate: "2026-05-14T08:00:00.000Z", endDate: "2026-05-14T08:30:00.000Z", }; - const native = mapAppleHealthEntry({ ...base, unit: mapping.hkUnit }); - const foreign = mapAppleHealthEntry({ ...base, unit: sibling! }); + const native = mapAppleHealthEntry( + { ...base, unit: mapping.hkUnit }, + { convertRecordUnit: true }, + ); + const foreign = mapAppleHealthEntry( + { ...base, unit: sibling! }, + { convertRecordUnit: true }, + ); expect(native).not.toBeNull(); expect(foreign).not.toBeNull(); @@ -136,16 +148,61 @@ describe("Apple Health mapping — the record's own unit (issue #944)", () => { ); it("keeps a unit it cannot place from rescaling the reading", () => { - const out = mapAppleHealthEntry({ - hkIdentifier: "HKQuantityTypeIdentifierDistanceWalkingRunning", - value: 1234, - unit: "furlong", - startDate: "2026-05-14T08:00:00.000Z", - endDate: "2026-05-14T08:30:00.000Z", - }); + const out = mapAppleHealthEntry( + { + hkIdentifier: "HKQuantityTypeIdentifierDistanceWalkingRunning", + value: 1234, + unit: "furlong", + startDate: "2026-05-14T08:00:00.000Z", + endDate: "2026-05-14T08:30:00.000Z", + }, + { convertRecordUnit: true }, + ); expect(out?.value).toBe(1234); }); + it("leaves the batch path's readings alone whatever unit they name", () => { + // The batch contract: `unit` is captured, never read. Every convertible + // entry has to behave that way by DEFAULT, or a native client stamping + // the person's display unit silently rescales their history. + for (const mapping of convertible) { + const sibling = siblingUnitFor(mapping.hkUnit); + if (!sibling) continue; + const base = { + hkIdentifier: mapping.hkIdentifier, + value: 2, + startDate: "2026-05-14T08:00:00.000Z", + endDate: "2026-05-14T08:30:00.000Z", + }; + const declared = mapAppleHealthEntry({ ...base, unit: mapping.hkUnit }); + const foreign = mapAppleHealthEntry({ ...base, unit: sibling }); + expect(declared).not.toBeNull(); + expect( + foreign!.value, + `${mapping.hkIdentifier} rescaled a batch reading from "${sibling}"`, + ).toBeCloseTo(declared!.value, 9); + } + }); + + it("has only the archive path opting into the conversion", () => { + // A grep, not a proof — but the wire change this pins is exactly the + // kind that lands as a one-word argument in an unrelated diff. + const optIn = "convertRecordUnit: true"; + const archive = readFileSync( + join(process.cwd(), "src/lib/measurements/import-apple-health-export.ts"), + "utf8", + ); + expect(archive).toContain(optIn); + const batch = readFileSync( + join(process.cwd(), "src/app/api/measurements/batch/route.ts"), + "utf8", + ); + expect( + batch, + "the batch route opted into record-unit conversion", + ).not.toContain("convertRecordUnit"); + }); + it("leaves the km/mi factors in one shared module", () => { // The workout path used to carry its own copy of the km/mi branch; the // duplicate is what let the `` path stay wrong for ten years. diff --git a/src/lib/measurements/__tests__/apple-health-mapping.test.ts b/src/lib/measurements/__tests__/apple-health-mapping.test.ts index f57427ded..3dd28a7e9 100644 --- a/src/lib/measurements/__tests__/apple-health-mapping.test.ts +++ b/src/lib/measurements/__tests__/apple-health-mapping.test.ts @@ -390,6 +390,38 @@ describe("mapAppleHealthEntry", () => { expect(out!.takenAt.toISOString()).toBe(ts); }); + it("stores a batch reading in the unit the table assumes, whatever `unit` says", () => { + // issue #944 — the batch route's `unit` is captured for audit and never + // read. A client that stamps the person's DISPLAY unit into the field + // must not have its metres multiplied by a thousand: the conversion is + // opt-in and only the `export.xml` path opts in. + const out = mapAppleHealthEntry({ + hkIdentifier: "HKQuantityTypeIdentifierDistanceWalkingRunning", + value: 2484, + unit: "km", + startDate: ts, + endDate: ts, + }); + expect(out).not.toBeNull(); + expect(out!.value).toBe(2484); + expect(out!.unit).toBe("m"); + }); + + it("converts out of the record's own unit when the caller opts in", () => { + const out = mapAppleHealthEntry( + { + hkIdentifier: "HKQuantityTypeIdentifierDistanceWalkingRunning", + value: 2.484, + unit: "km", + startDate: ts, + endDate: ts, + }, + { convertRecordUnit: true }, + ); + expect(out).not.toBeNull(); + expect(out!.value).toBeCloseTo(2484, 9); + }); + it("applies the unit conversion for fraction-shaped quantities", () => { const out = mapAppleHealthEntry({ hkIdentifier: "HKQuantityTypeIdentifierOxygenSaturation", diff --git a/src/lib/measurements/apple-health-mapping.ts b/src/lib/measurements/apple-health-mapping.ts index 4e760acc0..f090a7065 100644 --- a/src/lib/measurements/apple-health-mapping.ts +++ b/src/lib/measurements/apple-health-mapping.ts @@ -54,8 +54,9 @@ export interface AppleHealthMapping { * Apple's `export.xml` writes the ACCOUNT's display unit on every * quantity ``, not the unit this table assumes, so * `mapAppleHealthEntry()` converts the reading from the record's own - * unit into `hkUnit` before `convertToDbUnit` runs. That only works for - * a unit `convertHkValue()` knows a factor for. An entry whose `hkUnit` + * unit into `hkUnit` before `convertToDbUnit` runs — on the archive + * path, which is the caller that opts in. That only works for a unit + * `convertHkValue()` knows a factor for. An entry whose `hkUnit` * is NOT convertible (a dimensionless count, a pinned event, a * logarithmic dB level, a compound rate) must say here why no record * unit can change its reading — the structural guard @@ -1205,10 +1206,12 @@ export interface AppleHealthEntryInput { /** Numeric value as Apple delivers it (pre-conversion). */ value: number; /** - * Apple's unit string for this reading. issue #944 — the mapper converts - * the value from this unit into the table's `hkUnit` when the two differ - * and a factor is known (`km` -> `m`, `lb` -> `kg`, `degF` -> `degC`, …); - * an unknown or unplaceable unit leaves the reading untouched. + * Apple's unit string for this reading, captured for audit. + * + * Read ONLY when the caller opts in with `convertRecordUnit` — see + * `AppleHealthEntryOptions`. Every other caller gets the historical + * contract: the field is recorded and the reading is taken to already + * be in the table's `hkUnit`. */ unit: string; /** ISO timestamp string (e.g. HealthKit `startDate`). */ @@ -1244,6 +1247,33 @@ export interface AppleHealthEntryInput { valueMax?: number; } +/** + * Per-call options for `mapAppleHealthEntry()`. + */ +export interface AppleHealthEntryOptions { + /** + * Convert the reading out of `input.unit` and into the table's `hkUnit` + * before `convertToDbUnit` runs. Default FALSE. + * + * issue #944 — only the `export.xml` archive path passes `true`, because + * only there is the unit attribute known to be authoritative: Apple + * stamps every quantity `` with the ACCOUNT's display unit, so a + * metric archive reports walking distance in `km` and the number is + * meaningless without it. + * + * `POST /api/measurements/batch` deliberately does NOT opt in. Its `unit` + * is documented as captured-for-audit, not validated, and the native + * client's unit strings for the seventeen convertible identifiers are not + * pinned by anything in this repository — no fixture, no contract test, no + * coordination note. A client that reads a sample in metres and stamps the + * person's DISPLAY unit (`km`) into the field would have every walking + * distance multiplied by a thousand on ingest. Reading the field there is + * a wire change, and a wire change needs the client's strings pinned + * first. + */ + convertRecordUnit?: boolean; +} + /** Output of `mapAppleHealthEntry()`. */ export interface AppleHealthEntryOutput { type: MeasurementType; @@ -1283,6 +1313,7 @@ export interface AppleHealthEntryOutput { */ export function mapAppleHealthEntry( input: AppleHealthEntryInput, + options: AppleHealthEntryOptions = {}, ): AppleHealthEntryOutput | null { const mapping = APPLE_HEALTH_TYPE_MAP[input.hkIdentifier]; if (!mapping) return null; @@ -1294,10 +1325,14 @@ export function mapAppleHealthEntry( // `hkUnit`, but Apple's `export.xml` stamps each `` with the // ACCOUNT's own display unit: a metric archive reports walking distance // in `km`, an imperial one in `mi`, body mass in `lb`, energy in `kJ`. - // Normalise the reading into `hkUnit` first; a unit we cannot place - // leaves the value untouched (see `convertHkValue`). + // The archive path opts in and the reading is normalised into `hkUnit` + // first; a unit we cannot place leaves the value untouched (see + // `convertHkValue`). Every other caller keeps the identity behaviour — + // see `AppleHealthEntryOptions.convertRecordUnit` for why. const toHkUnit = (raw: number): number => - convertHkValue(raw, input.unit, mapping.hkUnit) ?? raw; + options.convertRecordUnit + ? (convertHkValue(raw, input.unit, mapping.hkUnit) ?? raw) + : raw; const value = mapping.convertToDbUnit(toHkUnit(input.value)); diff --git a/src/lib/measurements/import-apple-health-export.ts b/src/lib/measurements/import-apple-health-export.ts index ea14ad8fe..9bc4fe8cf 100644 --- a/src/lib/measurements/import-apple-health-export.ts +++ b/src/lib/measurements/import-apple-health-export.ts @@ -855,14 +855,21 @@ export async function streamParseExportXml( return; } - const mapped = mapAppleHealthEntry({ - hkIdentifier: hkType, - value: parsedValue.value, - unit: attrs.unit ?? mapping.hkUnit, - startDate: attrs.startDate, - endDate: attrs.endDate, - sleepStage: parsedValue.sleepStage, - }); + const mapped = mapAppleHealthEntry( + { + hkIdentifier: hkType, + value: parsedValue.value, + unit: attrs.unit ?? mapping.hkUnit, + startDate: attrs.startDate, + endDate: attrs.endDate, + sleepStage: parsedValue.sleepStage, + }, + // issue #944 — the archive is the one caller whose unit attribute + // is authoritative: Apple writes the account's own display unit on + // every quantity ``, so `km` here means kilometres and the + // reading has to be converted before it is stored. + { convertRecordUnit: true }, + ); if (!mapped) { unknown[`${hkType}::map_failed`] = (unknown[`${hkType}::map_failed`] ?? 0) + 1; diff --git a/src/lib/openapi/routes/measurements.ts b/src/lib/openapi/routes/measurements.ts index 756618059..1f28b049b 100644 --- a/src/lib/openapi/routes/measurements.ts +++ b/src/lib/openapi/routes/measurements.ts @@ -58,7 +58,7 @@ const batchEntrySchema = z .min(1) .max(60) .describe( - "Apple's `HKUnit` string for `value`. The server converts the reading into the unit it stores the identifier in when the two differ and the factor is known (`km` to `m`, `lb` to `kg`, `degF` to `degC`, and their siblings); a unit the server cannot place leaves the reading untouched.", + "Apple's `HKUnit` string for `value`, captured for audit. The server does NOT convert from it: `value` is read in the unit HealthLog stores the identifier in (`m` for walking distance, `kg` for body mass, `kcal` for energy, `degC` for temperature). Send the sample in that unit.", ), startDate: z.iso.datetime({ offset: true }), endDate: z.iso.datetime({ offset: true }), From 361e57786402e54996ea562fd896b0c1be7719b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 00:27:43 +0200 Subject: [PATCH 07/10] fix(ops): refuse an account whose rows are not the thousandfold class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provenance criterion proves where a row came from, not which build imported it. An account re-imported on the fixed build carries the same `EXPORT_XML_SOURCE_MAX` stamp and the same `stats:` external id with the right numbers and no audit row, so it is selected exactly like a broken one — and its rest days, under 200 m because the phone stayed at home, survive the plausibility check after another thousandfold multiply. So one candidate row that would leave the range now refuses the whole account: nothing written, no audit row, the offending rows and a printed reason instead. The idempotency guard only ever held against the script's own prior run; this holds against the case it could not see. The runbook says the same in words — a re-imported account must not be repaired, the re-import already did it — and says what the dry run is for: it is where the decision is made, not a formality in front of one already taken. Integration coverage for the re-imported account: correct rows, an `--apply` run, nothing changes and no audit row is left behind. Review finding H1. --- docs/ops/apple-health-distance-repair.md | 27 +++++- scripts/repair-apple-health-distance.ts | 25 ++++-- .../repair-apple-health-distance.ts | 71 ++++++++++++++-- .../apple-health-distance-repair.test.ts | 84 ++++++++++++++++++- 4 files changed, 189 insertions(+), 18 deletions(-) diff --git a/docs/ops/apple-health-distance-repair.md b/docs/ops/apple-health-distance-repair.md index 28b5c1c37..fd24ff16a 100644 --- a/docs/ops/apple-health-distance-repair.md +++ b/docs/ops/apple-health-distance-repair.md @@ -25,6 +25,16 @@ build rewrites every affected day with the right number, needs no flags, and also repairs rows this script deliberately leaves alone (see the criterion below). The script exists for accounts that no longer have the archive. +**An account that has been re-imported must not then be repaired.** The +re-import already fixed it. Its rows keep the same `EXPORT_XML_SOURCE_MAX` +stamp and the same `stats:` external id, and they carry no audit row, so the +criterion selects them exactly as it selects a broken account — the stamp +proves where a row came from, not which build imported it. Multiplying them +again turns a 2.484 km day into 2484 km. The script refuses such an account on +its own (any day whose value would leave the plausible range after the +multiply stops the whole account), but that guard fires on the big days: keep +the two apart yourself before you type `--apply`. + ## Which rows it touches Only rows carrying the stamp the archive importer itself writes: @@ -51,7 +61,13 @@ Deliberately out of scope: ## Run a dry-run first -Always. The default mode reports and never writes: +Always, and read it. The dry run is where the decision is made, not a +formality in front of a decision already taken: it is the only place the +script shows you real numbers from the account before it changes them. If the +worked example does not match what that person's Health app shows for that day, +the run is wrong and `--apply` will make it permanent. + +The default mode reports and never writes: ```bash # from a source checkout, with DATABASE_URL pointing at the instance @@ -94,9 +110,12 @@ clients pick the rows up on their next delta sync, and an audit row and the row count. The DAY/WEEK/MONTH/YEAR rollups and the cached status insights for the touched days are recomputed afterwards. -A row whose repaired value would leave the plausible range (0–200 000 m) is -reported and never written — several of those in one account mean the rows are -not the 1000x class and the run should be stopped. +A row whose repaired value would leave the plausible range (0–200 000 m) +refuses the **whole account**: nothing is written, the offending rows and the +reason are printed, and the run moves on to the next account. One such row is +evidence the account is not the 1000x class — a re-imported account looks +exactly like this — and repairing the rest of it would multiply days that were +already right. ## It runs once per account diff --git a/scripts/repair-apple-health-distance.ts b/scripts/repair-apple-health-distance.ts index 2abf0f45e..a7ef1fe5a 100644 --- a/scripts/repair-apple-health-distance.ts +++ b/scripts/repair-apple-health-distance.ts @@ -24,8 +24,10 @@ * - `--apply` writes, one transaction per account, and records an audit row * in that same transaction. An account carrying that row is skipped, so a * second run finds nothing. - * - A row whose repaired value would leave the plausibility range is - * reported and never written. + * - A row whose repaired value would leave the plausibility range refuses + * the WHOLE account: one such row is evidence the rows are not the 1000x + * class (an account re-imported on the fixed build looks exactly like + * this), and its rest days would otherwise be multiplied again. * - `--unit=mi` for an imperial archive (default `km`). The stored row keeps * no record of the archive's unit — that is the defect — so the operator * names it; a mismatched unit is why the flag exists rather than a guess. @@ -76,6 +78,7 @@ async function main(): Promise { let totalRepairable = 0; let totalUpdated = 0; let totalSkipped = 0; + let refused = 0; for (const plan of plans) { if (plan.alreadyRepairedAt) { @@ -107,7 +110,12 @@ async function main(): Promise { for (const row of plan.outOfRange) { console.log( ` ! ${row.id} (${row.measuredAt.toISOString().slice(0, 10)}): ` + - `${row.value} m x ${factor} leaves the plausible range — not written`, + `${row.value} m x ${factor} leaves the plausible range`, + ); + } + if (plan.outOfRange.length > 0) { + console.log( + " ! this account will be REFUSED — see the reason under --apply", ); } @@ -117,6 +125,11 @@ async function main(): Promise { }); totalUpdated += outcome.updated; totalSkipped += outcome.skipped; + if (outcome.refusedReason) { + refused += 1; + console.log(` ✗ refused: ${outcome.refusedReason}`); + continue; + } console.log( ` ✓ repaired ${outcome.updated} row(s), skipped ${outcome.skipped}`, ); @@ -124,9 +137,11 @@ async function main(): Promise { console.log( apply - ? `\nDone. ${totalUpdated} row(s) repaired, ${totalSkipped} skipped.` + ? `\nDone. ${totalUpdated} row(s) repaired, ${totalSkipped} skipped, ` + + `${refused} account(s) refused.` : `\nDry run: ${totalRepairable} row(s) would be repaired. ` + - "Re-run with --apply to write.", + "Read the worked examples above and decide before re-running with " + + "--apply.", ); } diff --git a/src/lib/measurements/repair-apple-health-distance.ts b/src/lib/measurements/repair-apple-health-distance.ts index 7257fa475..13d760cb7 100644 --- a/src/lib/measurements/repair-apple-health-distance.ts +++ b/src/lib/measurements/repair-apple-health-distance.ts @@ -35,6 +35,16 @@ * archive on the fixed build repairs those, and is the authoritative * repair for any account that still has its `export.zip`. * + * ── The refusal ──────────────────────────────────────────────────── + * + * The criterion proves an archive ORIGIN. It cannot prove the archive was + * imported on the broken build — an account re-imported since carries the + * same stamp with the right numbers, and its rest days still pass the + * plausibility check after another x1000. So any candidate whose repaired + * value would leave the range refuses the whole account: nothing is + * written, the rows are reported, and the operator decides. Re-importing + * the archive is the authoritative repair and needs no script. + * * ── Idempotency ──────────────────────────────────────────────────── * * Applying the repair writes one `AuditLog` row per account inside the @@ -81,8 +91,8 @@ export interface AccountRepairPlan { repairable: RepairCandidateRow[]; /** * Candidates whose repaired value would leave the plausibility range - * (`validateMeasurementRange`) — reported, never written. A row here is - * a sign the account's rows are NOT the 1000x class. + * (`validateMeasurementRange`). One row here refuses the whole account: + * it is evidence the rows are NOT the 1000x class. */ outOfRange: RepairCandidateRow[]; /** Non-null when this account already carries the repair's audit row. */ @@ -178,6 +188,35 @@ export interface RepairOutcome { userId: string; updated: number; skipped: number; + /** + * Why the account was left alone, when it was. Null on a run that wrote. + * The script prints it verbatim. + */ + refusedReason: string | null; + /** The out-of-range candidates — what a refusal was decided on. */ + skippedRows: RepairCandidateRow[]; +} + +/** + * The refusal an out-of-range candidate earns. + * + * The provenance criterion proves an archive origin; it cannot prove the + * archive was imported on the BROKEN build. An account that has since been + * re-imported on the fixed build carries the same stamp with the right + * numbers, and its rest days — under 200 m, phone left at home — pass the + * plausibility check after another x1000 and would be written. So a single + * row that would leave the range is treated as what it is: evidence the + * account is not the 1000x class. The whole account stops. + */ +function outOfRangeRefusal(plan: AccountRepairPlan, factor: number): string { + return ( + `${plan.outOfRange.length} of ${plan.candidateCount} candidate row(s) ` + + `leave the plausible range after x${factor}, so these rows are not the ` + + "1000x class — the account was most likely re-imported on the fixed " + + "build, or its archive was never in the affected unit. Nothing written. " + + "Check the dry run's worked examples against the account's Health app " + + "before forcing anything." + ); } /** @@ -192,8 +231,24 @@ export async function applyAppleHealthDistanceRepair( ): Promise { const factor = repairFactorFor(options.archiveUnit); const skipped = plan.outOfRange.length; - if (plan.alreadyRepairedAt || plan.repairable.length === 0) { - return { userId: plan.userId, updated: 0, skipped }; + const base = { + userId: plan.userId, + updated: 0, + skipped, + skippedRows: plan.outOfRange, + }; + if (plan.alreadyRepairedAt) { + return { + ...base, + refusedReason: `already repaired on ${plan.alreadyRepairedAt.toISOString()}`, + }; + } + // Refuse the ACCOUNT, not just the row: see `outOfRangeRefusal`. + if (plan.outOfRange.length > 0) { + return { ...base, refusedReason: outOfRangeRefusal(plan, factor) }; + } + if (plan.repairable.length === 0) { + return { ...base, refusedReason: "no repairable rows" }; } const ids = plan.repairable.map((row) => row.id); @@ -239,7 +294,13 @@ export async function applyAppleHealthDistanceRepair( "repair-apple-health-distance", ); - return { userId: plan.userId, updated: ids.length, skipped }; + return { + userId: plan.userId, + updated: ids.length, + skipped, + refusedReason: null, + skippedRows: plan.outOfRange, + }; } /** Narrowing helper for the script's `--unit` flag. */ diff --git a/tests/integration/apple-health-distance-repair.test.ts b/tests/integration/apple-health-distance-repair.test.ts index d4fa7bbea..b3ead4daf 100644 --- a/tests/integration/apple-health-distance-repair.test.ts +++ b/tests/integration/apple-health-distance-repair.test.ts @@ -187,25 +187,101 @@ describe("Apple Health export distance repair — integration", () => { expect((await readRow(prisma, rowId)).syncVersion).toBe(2); }); - it("uses the archive's own unit and never writes an implausible value", async () => { + it("uses the archive's own unit", async () => { const prisma = getPrismaClient(); const userId = await makeUser(prisma, "mi"); const mile = await seedRow(prisma, userId, { value: 1.543 }); - const absurd = await seedRow(prisma, userId, { value: 250 }); const plans = await planAppleHealthDistanceRepair(prisma, { archiveUnit: "mi", }); expect(plans[0].repairable.map((row) => row.id)).toEqual([mile]); + + const outcome = await applyAppleHealthDistanceRepair(prisma, plans[0], { + archiveUnit: "mi", + }); + await settleBackgroundTasks(); + expect(outcome).toMatchObject({ updated: 1, skipped: 0 }); + expect((await readRow(prisma, mile)).value).toBeCloseTo(2483.217792, 6); + }); + + it("refuses the whole account when one row would leave the range", async () => { + const prisma = getPrismaClient(); + const userId = await makeUser(prisma, "mixed"); + const plausible = await seedRow(prisma, userId, { value: 1.543 }); + const absurd = await seedRow(prisma, userId, { value: 250 }); + + const plans = await planAppleHealthDistanceRepair(prisma, { + archiveUnit: "mi", + }); + expect(plans[0].repairable.map((row) => row.id)).toEqual([plausible]); expect(plans[0].outOfRange.map((row) => row.id)).toEqual([absurd]); const outcome = await applyAppleHealthDistanceRepair(prisma, plans[0], { archiveUnit: "mi", }); await settleBackgroundTasks(); - expect(outcome).toMatchObject({ updated: 1, skipped: 1 }); - expect((await readRow(prisma, mile)).value).toBeCloseTo(2483.217792, 6); + expect(outcome.updated).toBe(0); + expect(outcome.skipped).toBe(1); + expect(outcome.skippedRows.map((row) => row.id)).toEqual([absurd]); + expect(outcome.refusedReason).toMatch(/leave the plausible range/); + + // Nothing written at all — not the "repairable" row, not the audit row + // that would make a later, correct run a no-op. + expect((await readRow(prisma, plausible)).value).toBe(1.543); + expect((await readRow(prisma, plausible)).syncVersion).toBe(1); expect((await readRow(prisma, absurd)).value).toBe(250); + expect( + await prisma.auditLog.count({ + where: { action: APPLE_HEALTH_DISTANCE_REPAIR_ACTION, userId }, + }), + ).toBe(0); + }); + + it("leaves an account re-imported on the fixed build untouched", async () => { + // The scenario the criterion cannot see: the operator re-imported + // `export.zip` on the fixed build, so the rows are already right. They + // still carry `EXPORT_XML_SOURCE_MAX` + the `stats:` external id and + // still have no audit row, so they are selected exactly like a broken + // account's rows. The rest day survives another x1000; the ordinary days + // do not, and that is what has to stop the account. + const prisma = getPrismaClient(); + const userId = await makeUser(prisma, "reimported"); + const ordinary = await seedRow(prisma, userId, { value: 2484 }); + const longWalk = await seedRow(prisma, userId, { value: 7020 }); + const restDay = await seedRow(prisma, userId, { value: 150 }); + + const plans = await planAppleHealthDistanceRepair(prisma, { + archiveUnit: "km", + }); + expect(plans).toHaveLength(1); + // The rest day would pass the plausibility check on its own. + expect(plans[0].repairable.map((row) => row.id)).toEqual([restDay]); + expect(plans[0].outOfRange.map((row) => row.id).sort()).toEqual( + [ordinary, longWalk].sort(), + ); + + const outcome = await applyAppleHealthDistanceRepair(prisma, plans[0], { + archiveUnit: "km", + }); + await settleBackgroundTasks(); + expect(outcome.updated).toBe(0); + expect(outcome.refusedReason).not.toBeNull(); + + for (const [id, value] of [ + [ordinary, 2484], + [longWalk, 7020], + [restDay, 150], + ] as const) { + const row = await readRow(prisma, id); + expect(row.value).toBe(value); + expect(row.syncVersion).toBe(1); + } + expect( + await prisma.auditLog.count({ + where: { action: APPLE_HEALTH_DISTANCE_REPAIR_ACTION, userId }, + }), + ).toBe(0); }); it("plans one entry per account", async () => { From 69e4ba7b8a6761662d64ac7568c823673931f781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 00:27:54 +0200 Subject: [PATCH 08/10] docs(ops): name the backup precondition on the distance repair An in-place multiplier with no reverse operation was documented with only a dry run in front of it. Point at the backup runbook where the write happens. Review finding M1. --- docs/ops/apple-health-distance-repair.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/ops/apple-health-distance-repair.md b/docs/ops/apple-health-distance-repair.md index fd24ff16a..f67263e5e 100644 --- a/docs/ops/apple-health-distance-repair.md +++ b/docs/ops/apple-health-distance-repair.md @@ -99,6 +99,10 @@ first. ## Apply +Take a backup first — `docs/ops/backup-restore.md`. The multiply is in place +and has no reverse operation: once an account is written wrong, the database +copy you took beforehand is the only way back. + ```bash pnpm dlx tsx scripts/repair-apple-health-distance.ts --apply ``` From 610083aa4dfb4edec0ce8540480ba70ce21ecb4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 00:37:15 +0200 Subject: [PATCH 09/10] fix(ops): stop the distance repair losing its rollup tail in silence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rollups and the cached status insights are recomputed after the transaction, so a throw there left the rows repaired and every chart on the pre-repair numbers — and no later run would notice, because the audit row makes the account skip from then on. The tail is wrapped now. The audit row is written with `rollupsRefreshed: false` and only flipped once the recompute returned, so a re-run can tell "repaired, rollups pending" from "done" and says so. A run that could not finish the tail names the account, prints the backfill to re-run — `scripts/backfill-rollups.ts --user ` — and exits non-zero. Review finding M2. --- docs/ops/apple-health-distance-repair.md | 14 +- scripts/repair-apple-health-distance.ts | 37 +++++ .../repair-apple-health-distance.ts | 127 +++++++++++++++--- .../apple-health-distance-repair.test.ts | 10 ++ 4 files changed, 166 insertions(+), 22 deletions(-) diff --git a/docs/ops/apple-health-distance-repair.md b/docs/ops/apple-health-distance-repair.md index f67263e5e..1b9a10cb6 100644 --- a/docs/ops/apple-health-distance-repair.md +++ b/docs/ops/apple-health-distance-repair.md @@ -112,7 +112,19 @@ metre factor, `sync_version` incremented and `updated_at` bumped so paired iOS clients pick the rows up on their next delta sync, and an audit row (`measurement.apple_health_distance.repaired`) recording the unit, the factor and the row count. The DAY/WEEK/MONTH/YEAR rollups and the cached status -insights for the touched days are recomputed afterwards. +insights for the touched days are recomputed afterwards, outside that +transaction. + +If that recompute fails, the rows are right and every chart is not, and no +later run of this script would notice — it skips a repaired account. So the +audit row is written with `rollupsRefreshed: false` and only flipped to `true` +once the recompute has returned. A run that could not finish it prints the +account and the command to re-run, exits non-zero, and a later dry run repeats +the warning until the backfill has been run: + +```bash +pnpm dlx tsx scripts/backfill-rollups.ts --user +``` A row whose repaired value would leave the plausible range (0–200 000 m) refuses the **whole account**: nothing is written, the offending rows and the diff --git a/scripts/repair-apple-health-distance.ts b/scripts/repair-apple-health-distance.ts index a7ef1fe5a..be77a3b8f 100644 --- a/scripts/repair-apple-health-distance.ts +++ b/scripts/repair-apple-health-distance.ts @@ -28,6 +28,10 @@ * the WHOLE account: one such row is evidence the rows are not the 1000x * class (an account re-imported on the fixed build looks exactly like * this), and its rest days would otherwise be multiplied again. + * - The rollup/status tail runs after the transaction. If it throws, the + * rows are repaired and the derived tier is not; the script names the + * backfill to re-run, the audit row keeps `rollupsRefreshed: false` so a + * later run can still see it, and the exit code is non-zero. * - `--unit=mi` for an imperial archive (default `km`). The stored row keeps * no record of the archive's unit — that is the defect — so the operator * names it; a mismatched unit is why the flag exists rather than a guess. @@ -45,6 +49,7 @@ import { parseArchiveUnit, planAppleHealthDistanceRepair, repairFactorFor, + rollupRepairCommand, } from "@/lib/measurements/repair-apple-health-distance"; function readArchiveUnit(): "km" | "mi" { @@ -79,6 +84,7 @@ async function main(): Promise { let totalUpdated = 0; let totalSkipped = 0; let refused = 0; + let rollupsPending = 0; for (const plan of plans) { if (plan.alreadyRepairedAt) { @@ -86,6 +92,15 @@ async function main(): Promise { ` • ${plan.userId}: already repaired on ` + `${plan.alreadyRepairedAt.toISOString()} — ${plan.candidateCount} row(s) left alone.`, ); + if (plan.rollupsPending) { + rollupsPending += 1; + console.log( + " ! rollups PENDING from that run: the DAY/WEEK/MONTH/YEAR " + + "buckets and the cached status insights still hold the " + + "pre-repair numbers.", + ); + console.log(` re-run: ${rollupRepairCommand(plan.userId)}`); + } continue; } @@ -133,6 +148,18 @@ async function main(): Promise { console.log( ` ✓ repaired ${outcome.updated} row(s), skipped ${outcome.skipped}`, ); + if (outcome.rollupsPending) { + rollupsPending += 1; + console.error( + ` ! ${plan.userId}: rows repaired, but the rollup refresh failed.`, + ); + console.error(outcome.rollupError); + console.error( + " The DAY/WEEK/MONTH/YEAR buckets and the cached status " + + "insights still hold the pre-repair numbers.", + ); + console.error(` re-run: ${rollupRepairCommand(plan.userId)}`); + } } console.log( @@ -143,6 +170,16 @@ async function main(): Promise { "Read the worked examples above and decide before re-running with " + "--apply.", ); + + if (rollupsPending > 0) { + // The rows are right and the derived tier is not, which no later run of + // this script will notice — it skips a repaired account. Fail loudly. + console.error( + `\n${rollupsPending} account(s) need the rollup backfill re-run ` + + "(commands above).", + ); + process.exitCode = 1; + } } main() diff --git a/src/lib/measurements/repair-apple-health-distance.ts b/src/lib/measurements/repair-apple-health-distance.ts index 13d760cb7..bd2947586 100644 --- a/src/lib/measurements/repair-apple-health-distance.ts +++ b/src/lib/measurements/repair-apple-health-distance.ts @@ -97,6 +97,13 @@ export interface AccountRepairPlan { outOfRange: RepairCandidateRow[]; /** Non-null when this account already carries the repair's audit row. */ alreadyRepairedAt: Date | null; + /** + * True when a previous run wrote the rows but its rollup tail did not + * finish — the audit row still says `rollupsRefreshed: false`, so the + * DAY/WEEK/MONTH/YEAR buckets and the cached status insights still hold + * the pre-repair numbers. Meaningless unless `alreadyRepairedAt` is set. + */ + rollupsPending: boolean; /** Candidate rows found, whatever the disposition above. */ candidateCount: number; } @@ -147,12 +154,15 @@ export async function planAppleHealthDistanceRepair( action: APPLE_HEALTH_DISTANCE_REPAIR_ACTION, userId: { in: userIds }, }, - select: { userId: true, createdAt: true }, + select: { userId: true, createdAt: true, details: true }, orderBy: { createdAt: "asc" }, }); const repairedAt = new Map(); + const rollupsPending = new Set(); for (const entry of repaired) { - if (entry.userId) repairedAt.set(entry.userId, entry.createdAt); + if (!entry.userId) continue; + repairedAt.set(entry.userId, entry.createdAt); + if (!rollupsRefreshedIn(entry.details)) rollupsPending.add(entry.userId); } const plans = new Map(); @@ -164,6 +174,7 @@ export async function planAppleHealthDistanceRepair( repairable: [], outOfRange: [], alreadyRepairedAt: repairedAt.get(row.userId) ?? null, + rollupsPending: rollupsPending.has(row.userId), candidateCount: 0, }; plans.set(row.userId, plan); @@ -184,6 +195,31 @@ export async function planAppleHealthDistanceRepair( return Array.from(plans.values()); } +/** + * The command that heals the rollup tail when it did not run. + * + * Named here rather than in the script so the message an operator reads is + * the same one the module can be asked for. + */ +export function rollupRepairCommand(userId: string): string { + return `pnpm dlx tsx scripts/backfill-rollups.ts --user ${userId}`; +} + +/** Did a repair audit row record a finished rollup tail? */ +function rollupsRefreshedIn(details: string | null): boolean { + if (!details) return false; + try { + const parsed: unknown = JSON.parse(details); + return ( + typeof parsed === "object" && + parsed !== null && + (parsed as { rollupsRefreshed?: unknown }).rollupsRefreshed === true + ); + } catch { + return false; + } +} + export interface RepairOutcome { userId: string; updated: number; @@ -195,6 +231,15 @@ export interface RepairOutcome { refusedReason: string | null; /** The out-of-range candidates — what a refusal was decided on. */ skippedRows: RepairCandidateRow[]; + /** + * True when the rows were written but the rollup tail was not. The audit + * row still reads `rollupsRefreshed: false`, so a later run can tell this + * apart from a finished repair; the caller must surface + * `rollupRepairCommand(userId)` and exit non-zero. + */ + rollupsPending: boolean; + /** What the rollup tail threw, when it threw. */ + rollupError: unknown; } /** @@ -236,6 +281,8 @@ export async function applyAppleHealthDistanceRepair( updated: 0, skipped, skippedRows: plan.outOfRange, + rollupsPending: false, + rollupError: null, }; if (plan.alreadyRepairedAt) { return { @@ -252,7 +299,20 @@ export async function applyAppleHealthDistanceRepair( } const ids = plan.repairable.map((row) => row.id); - await client.$transaction(async (tx) => { + const details = { + type: REPAIRED_TYPE, + archiveUnit: options.archiveUnit, + factor, + rows: ids.length, + skippedOutOfRange: skipped, + issue: 944, + // Flipped to true only once the rollup tail below has finished. A row + // left at false says "rows repaired, rollups still pre-repair", which is + // what a re-run has to be able to see: the account is skipped from here + // on, so nothing else would ever notice. + rollupsRefreshed: false, + }; + const auditId = await client.$transaction(async (tx): Promise => { // One statement per chunk rather than one per row: an account with a // decade of history carries thousands of rows and an interactive // transaction has a clock on it. `sync_version` + `updated_at` are @@ -272,27 +332,50 @@ export async function applyAppleHealthDistanceRepair( client: tx, userId: plan.userId, actorUserId: null, - details: { - type: REPAIRED_TYPE, - archiveUnit: options.archiveUnit, - factor, - rows: ids.length, - skippedOutOfRange: skipped, - issue: 944, - }, + details, }); + // Read the row back rather than widening `auditLog`'s return type for + // ~600 call sites. Inside the same transaction, and an account can hold + // at most one of these rows — the second run is refused above — so the + // lookup is exact, not a guess at the newest. + const entry = await tx.auditLog.findFirstOrThrow({ + where: { + action: APPLE_HEALTH_DISTANCE_REPAIR_ACTION, + userId: plan.userId, + }, + select: { id: true }, + }); + return entry.id; }); - // Best-effort tail: the DAY/WEEK/MONTH/YEAR rollups and the cached - // status insights hold the pre-repair numbers until they are recomputed. - await afterMeasurementMutation( - plan.userId, - plan.repairable.map((row) => ({ - type: REPAIRED_TYPE, - measuredAt: row.measuredAt, - })), - "repair-apple-health-distance", - ); + // The tail: the DAY/WEEK/MONTH/YEAR rollups and the cached status insights + // hold the pre-repair numbers until they are recomputed. It runs outside + // the transaction, so a throw here leaves the rows repaired and the derived + // tier stale — and the account is skipped from the next run on. Hence the + // flag: it is flipped only after the tail returned, and the caller is told + // to run the backfill and to exit non-zero. + let rollupsPending = false; + let rollupError: unknown = null; + try { + await afterMeasurementMutation( + plan.userId, + plan.repairable.map((row) => ({ + type: REPAIRED_TYPE, + measuredAt: row.measuredAt, + })), + "repair-apple-health-distance", + ); + await client.auditLog.update({ + where: { id: auditId }, + data: { details: JSON.stringify({ ...details, rollupsRefreshed: true }) }, + }); + } catch (err) { + // Either leg failing leaves the flag at false, which is the safe + // direction: an operator re-running the backfill needlessly costs time, + // one who never learns it is due keeps wrong numbers on every chart. + rollupsPending = true; + rollupError = err; + } return { userId: plan.userId, @@ -300,6 +383,8 @@ export async function applyAppleHealthDistanceRepair( skipped, refusedReason: null, skippedRows: plan.outOfRange, + rollupsPending, + rollupError, }; } diff --git a/tests/integration/apple-health-distance-repair.test.ts b/tests/integration/apple-health-distance-repair.test.ts index b3ead4daf..11d40f1eb 100644 --- a/tests/integration/apple-health-distance-repair.test.ts +++ b/tests/integration/apple-health-distance-repair.test.ts @@ -155,7 +155,17 @@ describe("Apple Health export distance repair — integration", () => { factor: 1000, rows: 2, issue: 944, + // Flipped only after the rollup tail returned. A run that repaired the + // rows and lost the tail leaves this false, and the account is skipped + // from here on — so it is the only place the gap can still be seen. + rollupsRefreshed: true, }); + expect(outcome.rollupsPending).toBe(false); + + const second = await planAppleHealthDistanceRepair(prisma, { + archiveUnit: "km", + }); + expect(second[0].rollupsPending).toBe(false); }); it("is idempotent — a second run finds nothing to repair", async () => { From 779cd3a0c8de40b62a23eaea27d63ffadbbd57e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 00:37:22 +0200 Subject: [PATCH 10/10] test: ban a length rescale, not every literal thousand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard read `* 1000` anywhere in the two mapping files as a re-implemented kilometre factor. Both files also turn seconds into milliseconds, so an unrelated conversion would have failed it for the wrong reason and the next person would have deleted the rule. A rescale now counts only in a length context: a distance identifier on the same line or within the five above it, which covers the body of a distance-named helper. The offending line is named in the failure. The matcher's limit is written next to it — a length conversion in a helper named nothing like a distance, more than five lines below its signature, still slips through. Checked both ways against the real file: a millisecond conversion passes, a `km * 1000` helper fails. Review finding L4. --- .../apple-health-unit-audit-guard.test.ts | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/__tests__/apple-health-unit-audit-guard.test.ts b/src/__tests__/apple-health-unit-audit-guard.test.ts index 2975cd92d..b8799aeaf 100644 --- a/src/__tests__/apple-health-unit-audit-guard.test.ts +++ b/src/__tests__/apple-health-unit-audit-guard.test.ts @@ -206,17 +206,41 @@ describe("Apple Health mapping — the record's own unit (issue #944)", () => { it("leaves the km/mi factors in one shared module", () => { // The workout path used to carry its own copy of the km/mi branch; the // duplicate is what let the `` path stay wrong for ten years. + // + // `* 1000` alone is not the offence — these files also turn seconds into + // milliseconds — so a rescale only counts when it stands in a LENGTH + // context: a distance identifier on the same line, or within the five + // lines above it (which covers a rescale in the body of a + // distance-named helper). The limit is the usual one for a matcher of + // this shape: a length conversion inside a helper named nothing like a + // distance, more than five lines below its own signature, slips it. const shared = join(process.cwd(), "src/lib/measurements/hk-units.ts"); const owners = [ "src/lib/measurements/apple-health-mapping.ts", "src/lib/measurements/import-apple-health-export.ts", ]; expect(readFileSync(shared, "utf8")).toContain("1609.344"); + + const RESCALE = /\*\s*1_?000(?!\d)/; + const LENGTH = /distance|metre|meter|\bkm\b/i; for (const rel of owners) { const source = readFileSync(join(process.cwd(), rel), "utf8"); - expect(source, `${rel} re-implements a length factor`).not.toMatch( - /1609\.344|\* 1000\b/, + expect(source, `${rel} re-implements the mile factor`).not.toContain( + "1609.344", ); + const lines = source.split("\n"); + const offenders = lines + .map((line, i) => ({ line, i })) + .filter( + ({ line, i }) => + RESCALE.test(line) && + lines.slice(Math.max(0, i - 5), i + 1).some((l) => LENGTH.test(l)), + ) + .map(({ line, i }) => `${rel}:${i + 1}: ${line.trim()}`); + expect( + offenders, + `${rel} re-implements a length factor — the km/mi conversion belongs in hk-units.ts`, + ).toEqual([]); } }); });