From b3d586066acabf16427aa0b2cc635bdca6aeebeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 17 Jul 2026 13:57:03 +0200 Subject: [PATCH] fix(workouts): backfill richer fields on the canonical-picker winner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pickCanonicalWorkout() picked one whole row per cluster by the source ladder and dropped the rest, so a higher-priority twin with a null HR (e.g. a Withings summary) discarded a lower-priority twin's real reading (e.g. WHOOP's HR strap) instead of keeping it. The ladder winner is now the base row, backfilled with avgHeartRate, maxHeartRate, totalEnergyKcal, totalDistanceM, and elevationM from the ladder-highest member that actually has each field — filling gaps, never overwriting a value the base row already carries. The base row also adopts a specific sport type from a cluster member when its own is generic ("other" / unrecognised). Reaching that case needs clustering to accept a generic↔specific sport pairing within the proximity window, alongside the existing exact-match rule — two different SPECIFIC sports (Walking vs Running) still never cluster, so the core invariant holds. --- .../__tests__/pick-canonical-workout.test.ts | 124 ++++++++++++ src/lib/sources/pick-canonical-workout.ts | 181 ++++++++++++++++-- 2 files changed, 286 insertions(+), 19 deletions(-) diff --git a/src/lib/sources/__tests__/pick-canonical-workout.test.ts b/src/lib/sources/__tests__/pick-canonical-workout.test.ts index da22f41cc..e42ed8ef6 100644 --- a/src/lib/sources/__tests__/pick-canonical-workout.test.ts +++ b/src/lib/sources/__tests__/pick-canonical-workout.test.ts @@ -28,11 +28,13 @@ const row = ( source: WorkoutPickerRow["source"], startedAt: string, sportType: WorkoutPickerRow["sportType"] = "running", + extra: Partial = {}, ): WorkoutPickerRow => ({ id, source, startedAt: D(startedAt), sportType, + ...extra, }); describe("DEFAULT_WORKOUT_SOURCE_PRIORITY", () => { @@ -213,3 +215,125 @@ describe("pickCanonicalWorkout — determinism", () => { expect(result.canonical[0].id).toBe("a-1"); }); }); + +describe("pickCanonicalWorkout — field-merge (v1.29.x, WHOOP HR-loss fix)", () => { + it("backfills avgHeartRate/maxHeartRate from a lower-priority twin when the winner's is null", () => { + // Regression fixture from the brief: a WHOOP cycling row carries live + // HR; a higher-priority Apple Health twin for the SAME ride has no HR + // (e.g. a manually-logged / GPS-only Apple entry). Before the merge, + // the picker kept the Apple row whole and threw the WHOOP HR away. + const result = pickCanonicalWorkout([ + row("apple-1", "APPLE_HEALTH", "2026-05-14T07:30:00.000Z", "cycling", { + avgHeartRate: null, + maxHeartRate: null, + }), + row("whoop-1", "WHOOP", "2026-05-14T07:31:00.000Z", "cycling", { + avgHeartRate: 142, + maxHeartRate: 168, + }), + ]); + expect(result.canonical).toHaveLength(1); + const winner = result.canonical[0]; + expect(winner.id).toBe("apple-1"); // ladder winner stays the base row + expect(winner.avgHeartRate).toBe(142); + expect(winner.maxHeartRate).toBe(168); + expect(result.clusters[0].picked.avgHeartRate).toBe(142); + }); + + it("never overwrites a field the base row already has", () => { + const result = pickCanonicalWorkout([ + row("apple-1", "APPLE_HEALTH", "2026-05-14T07:30:00.000Z", "cycling", { + avgHeartRate: 120, + }), + row("whoop-1", "WHOOP", "2026-05-14T07:31:00.000Z", "cycling", { + avgHeartRate: 142, + }), + ]); + expect(result.canonical[0].avgHeartRate).toBe(120); + }); + + it("backfills totalEnergyKcal, totalDistanceM, and elevationM independently", () => { + const result = pickCanonicalWorkout([ + row("apple-1", "APPLE_HEALTH", "2026-05-14T07:30:00.000Z", "cycling", { + totalEnergyKcal: null, + totalDistanceM: 15000, + elevationM: null, + }), + row("whoop-1", "WHOOP", "2026-05-14T07:31:00.000Z", "cycling", { + totalEnergyKcal: 480, + totalDistanceM: 14800, + elevationM: 220, + }), + ]); + const winner = result.canonical[0]; + expect(winner.totalEnergyKcal).toBe(480); // backfilled + expect(winner.totalDistanceM).toBe(15000); // base row's own value kept + expect(winner.elevationM).toBe(220); // backfilled + }); + + it("prefers the ladder-highest member that HAS a field over a lower one, when the base row lacks it", () => { + const result = pickCanonicalWorkout([ + row("apple-1", "APPLE_HEALTH", "2026-05-14T07:30:00.000Z", "cycling", { + avgHeartRate: null, + }), + row("whoop-1", "WHOOP", "2026-05-14T07:30:30.000Z", "cycling", { + avgHeartRate: 142, + }), + row("withings-1", "WITHINGS", "2026-05-14T07:31:00.000Z", "cycling", { + avgHeartRate: 99, + }), + ]); + // WHOOP outranks WITHINGS on the default ladder — its HR wins the + // backfill even though WITHINGS also had one. + expect(result.canonical[0].avgHeartRate).toBe(142); + }); + + it("adopts a specific sport type from a member when the base row's own sportType is generic", () => { + // The confirmed bug's downstream symptom: before the WHOOP sport-map + // fix, a WHOOP cycling workout could carry sportType "other" (or a + // non-canonical raw label) while a lower-priority twin correctly + // tagged "cycling". The merge adopts the specific sport so the + // cluster still surfaces as a bike ride. + const result = pickCanonicalWorkout([ + row("apple-1", "APPLE_HEALTH", "2026-05-14T07:30:00.000Z", "other"), + row("whoop-1", "WHOOP", "2026-05-14T07:31:00.000Z", "cycling"), + ]); + expect(result.canonical[0].sportType).toBe("cycling"); + }); + + it("keeps the base row's own specific sportType even when a member disagrees", () => { + const result = pickCanonicalWorkout([ + row("apple-1", "APPLE_HEALTH", "2026-05-14T07:30:00.000Z", "cycling"), + row("whoop-1", "WHOOP", "2026-05-14T07:31:00.000Z", "cycling"), + ]); + expect(result.canonical[0].sportType).toBe("cycling"); + }); + + it("does not run the merge on a single-element cluster", () => { + const result = pickCanonicalWorkout([ + row("apple-1", "APPLE_HEALTH", "2026-05-14T07:30:00.000Z", "cycling", { + avgHeartRate: null, + }), + ]); + expect(result.canonical[0].avgHeartRate).toBeNull(); + }); + + it("end-to-end: a WHOOP-cycling+HR row clustered with a generic no-HR twin yields a cycling canonical row WITH the HR", () => { + const result = pickCanonicalWorkout([ + row("apple-1", "APPLE_HEALTH", "2026-05-14T07:30:00.000Z", "other", { + avgHeartRate: null, + totalEnergyKcal: null, + }), + row("whoop-1", "WHOOP", "2026-05-14T07:31:00.000Z", "cycling", { + avgHeartRate: 145, + totalEnergyKcal: 512, + }), + ]); + expect(result.canonical).toHaveLength(1); + const winner = result.canonical[0]; + expect(winner.source).toBe("APPLE_HEALTH"); // ladder still picks Apple as the base + expect(winner.sportType).toBe("cycling"); // adopted from WHOOP + expect(winner.avgHeartRate).toBe(145); // backfilled from WHOOP + expect(winner.totalEnergyKcal).toBe(512); // backfilled from WHOOP + }); +}); diff --git a/src/lib/sources/pick-canonical-workout.ts b/src/lib/sources/pick-canonical-workout.ts index 371765c3c..e40f6df6d 100644 --- a/src/lib/sources/pick-canonical-workout.ts +++ b/src/lib/sources/pick-canonical-workout.ts @@ -21,14 +21,31 @@ * ScanWatch passthrough lag plus a small clock-skew buffer). * Sport type must match across the cluster; a "Walking" workout * and a "Running" workout starting at the same instant stay - * distinct rows so the cardio-vs-strength split stays clean. + * distinct rows so the cardio-vs-strength split stays clean. Once + * an integration writes a canonical `sportType` (every source does, + * as of the WHOOP fix below), a cross-source pair of the SAME + * workout always shares a sport bucket and clusters correctly — + * before that fix a WHOOP "whoop_sport_1" row could never cluster + * with an Apple Health "cycling" row. * 2. Per cluster, walk the per-user source-priority ladder for * workouts (default `APPLE_HEALTH > WITHINGS > MANUAL > IMPORT`) - * and keep the first source that has a row in the cluster. All - * other rows from the same cluster are dropped from the - * canonical list. - * 3. Single-element clusters (no near-duplicate) pass through - * unchanged regardless of source. + * and keep the first source that has a row in the cluster as the + * BASE of the canonical row. All other rows from the same cluster + * are dropped from the canonical list. + * 3. Field-merge (multi-element clusters only): the base row is + * backfilled with any of `avgHeartRate`, `maxHeartRate`, + * `totalEnergyKcal`, `totalDistanceM`, `elevationM` it is itself + * missing, from the ladder-highest cluster member that HAS that + * field — so a higher-priority twin with a null HR (e.g. a + * Withings summary) doesn't discard the HR a lower-priority twin + * (e.g. WHOOP) actually recorded. The same step adopts a specific + * sport type from a member when the base row's own sportType is + * generic (`"other"` / unrecognised). Every OTHER field on the + * base row (id, source, startedAt, route, metadata, ...) is + * untouched — the merge only ever fills a gap, never overwrites a + * value the base row already has. + * 4. Single-element clusters (no near-duplicate) pass through + * unchanged regardless of source — no merge step runs. * * Rationale for the default ladder * ──────────────────────────────── @@ -55,14 +72,18 @@ import type { MeasurementSource } from "@/generated/prisma/client"; -import type { WorkoutSportType } from "@/lib/validations/workout"; +import { + workoutSportTypeEnum, + type WorkoutSportType, +} from "@/lib/validations/workout"; /** * Minimum row shape the helper consults. Exposed as an interface so the * Prisma `Workout` model and the not-yet-existing ingest DTO both * compose with the picker without a transform step. Anything else on - * the row (totals, HR aggregates, route blob, metadata) flows through - * untouched — the picker filters but never mutates. + * the row (route blob, metadata, ...) flows through untouched — ONLY the + * fields declared below are eligible for the step-3 field-merge, and even + * those are backfilled (fills a gap on the base row), never overwritten. */ export interface WorkoutPickerRow { /** Stable identifier — `id` for DB rows, externalId+source for @@ -72,8 +93,108 @@ export interface WorkoutPickerRow { startedAt: Date; source: MeasurementSource; /** Canonical sport string. Workouts of different sport types never - * collapse into one cluster — see step 1 of the algorithm. */ + * collapse into one cluster — see step 1 of the algorithm. May be + * adopted from a losing cluster member by the step-3 merge when the + * base row's own value is generic — see `mergeClusterFields()`. */ sportType: WorkoutSportType | string; + /** Backfillable richer fields — present when the source carries live + * sensor data (HR strap, GPS), absent/null on a coarser summary from + * another source recording the same workout. */ + avgHeartRate?: number | null; + maxHeartRate?: number | null; + totalEnergyKcal?: number | null; + totalDistanceM?: number | null; + elevationM?: number | null; +} + +/** + * The numeric fields step 3 of the algorithm may backfill onto the base + * row. Listed explicitly (not derived via `keyof`) so each field's copy + * stays independently type-checked in `mergeClusterFields()`. + */ +const MERGEABLE_NUMERIC_FIELDS = [ + "avgHeartRate", + "maxHeartRate", + "totalEnergyKcal", + "totalDistanceM", + "elevationM", +] as const; + +/** + * `true` for a specific canonical sport bucket; `false` for the generic + * `"other"` bucket or a string outside `workoutSportTypeEnum` entirely (a + * not-yet-normalised integration's raw label). Drives the sport-type + * adoption half of the merge. + */ +function isSpecificSportType(value: string): boolean { + return ( + value !== "other" && + (workoutSportTypeEnum.options as readonly string[]).includes(value) + ); +} + +/** Ladder position of `source`, or `ladder.length` (last) when the source + * isn't on the ladder at all — an unranked source only wins a merge field + * when no ranked member has it. */ +function ladderRank( + source: MeasurementSource, + ladder: readonly MeasurementSource[], +): number { + const idx = ladder.indexOf(source); + return idx === -1 ? ladder.length : idx; +} + +/** + * Step 3 of the algorithm: backfill the ladder-winning base row's missing + * richer fields, and its generic sport type, from other cluster members. + * + * Deterministic: for each candidate field, the ladder-highest member that + * HAS a non-null value wins that field. `members` is already + * startedAt/id-sorted by the caller, so a same-rank tie resolves to + * whichever member sorts first. + * + * A single-element cluster returns `winning` unchanged — nothing to merge. + */ +function mergeClusterFields( + winning: T, + members: readonly T[], + ladder: readonly MeasurementSource[], +): T { + if (members.length <= 1) return winning; + + const merged: T = { ...winning }; + + for (const field of MERGEABLE_NUMERIC_FIELDS) { + if (merged[field] != null) continue; + let best: T | undefined; + let bestRank = Infinity; + for (const member of members) { + const value = member[field]; + if (value == null) continue; + const rank = ladderRank(member.source, ladder); + if (rank < bestRank) { + bestRank = rank; + best = member; + } + } + if (best) merged[field] = best[field]; + } + + if (!isSpecificSportType(merged.sportType)) { + let best: T | undefined; + let bestRank = Infinity; + for (const member of members) { + if (!isSpecificSportType(member.sportType)) continue; + const rank = ladderRank(member.source, ladder); + if (rank < bestRank) { + bestRank = rank; + best = member; + } + } + if (best) merged.sportType = best.sportType; + } + + return merged; } /** @@ -149,8 +270,12 @@ export function pickCanonicalWorkout( * without re-running the picker. */ clusters: Array<{ + /** Every row that clustered together, UNMERGED — the raw inputs the + * merge step read from. */ members: T[]; pickedSource: MeasurementSource; + /** The ladder winner AFTER the step-3 field-merge — the same object + * that landed in `canonical` for this cluster. */ picked: T; }>; } { @@ -175,11 +300,19 @@ export function pickCanonicalWorkout( // Build clusters keyed by sport type (a Walking + Running pair // starting at the same instant must stay distinct — different - // workouts entirely). + // workouts entirely) — WITH one relaxation: a GENERIC sport + // ("other" / an unrecognised raw string) is treated as compatible + // with any SPECIFIC sport, so a thinly-classified row from one + // source (an unmapped import, a not-yet-normalised integration) can + // still cluster with a specific-sport twin from another source + // recording the SAME real-world workout — the merge step below then + // adopts the specific sport onto the canonical row. Two DIFFERENT + // specific sports (Walking vs Running) never become compatible this + // way — the core invariant is unchanged. const clusters: T[][] = []; for (const row of sorted) { - // Find an existing cluster of the same sport type whose latest - // member is within the proximity window. Walking from the most + // Find an existing cluster whose latest member is within the + // proximity window and sport-compatible. Walking from the most // recent cluster backward is O(k) per row where k is the open // cluster count; for the workout volume HealthLog ingests (single // user, tens of workouts per week) this is well below the @@ -188,16 +321,25 @@ export function pickCanonicalWorkout( for (let i = clusters.length - 1; i >= 0; i--) { const cluster = clusters[i]; const head = cluster[0]; - if (head.sportType !== row.sportType) continue; + const sameSport = head.sportType === row.sportType; + if (!sameSport) { + const genericPairing = + !isSpecificSportType(head.sportType) || + !isSpecificSportType(row.sportType); + if (!genericPairing) continue; + } const last = cluster[cluster.length - 1]; if (row.startedAt.getTime() - last.startedAt.getTime() <= proximityMs) { cluster.push(row); placed = true; break; } - // Sorted input + outside-window: every earlier cluster of the - // same sport is even further away, no point continuing. - if (head.sportType === row.sportType) break; + // Sorted input + outside-window, exact sport match: every + // earlier cluster of the same sport is even further away, no + // point continuing. A generic↔specific pairing has no such + // monotonic guarantee once different-sport clusters interleave + // in time, so keep scanning rather than abort the whole search. + if (sameSport) break; } if (!placed) { clusters.push([row]); @@ -247,11 +389,12 @@ export function pickCanonicalWorkout( continue; } const winning = cluster.find((row) => row.source === pickedSource)!; - canonical.push(winning); + const merged = mergeClusterFields(winning, cluster, ladder); + canonical.push(merged); clusterRecords.push({ members: cluster, pickedSource, - picked: winning, + picked: merged, }); }