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 }),