diff --git a/src/__tests__/workout-canonical-identity-guard.test.ts b/src/__tests__/workout-canonical-identity-guard.test.ts index 73fb493fb..416faa7e5 100644 --- a/src/__tests__/workout-canonical-identity-guard.test.ts +++ b/src/__tests__/workout-canonical-identity-guard.test.ts @@ -31,7 +31,7 @@ const SRC = join(process.cwd(), "src"); const PICKER = "lib/measurements/pick-canonical-workout-rows.ts"; /** - * Every production call site of the read-time picker, frozen. + * Every production call site of the picker, frozen. * * Adding a surface that collapses workouts is a deliberate act: it must fetch * the identity fields or it silently disagrees with every other surface. Adding @@ -48,6 +48,18 @@ const IDENTITY_OWNER: Record = { // block's data. Recording the owner here rather than assuming the call site // owns its own select is the whole point of this map. "lib/ai/coach/snapshot-blocks/workouts-block.ts": "lib/ai/coach/snapshot.ts", + // The write path: the batch route runs the same picker over the rows it is + // about to insert, so a duplicate inside one batch is dropped by the rule + // the list later applies. + "app/api/workouts/batch/route.ts": "app/api/workouts/batch/route.ts", +}; + +/** + * How a call site proves its rows carry the duration. A read selects the + * column; the batch route maps it off the row it computed for the insert. + */ +const IDENTITY_PROOF: Record = { + "app/api/workouts/batch/route.ts": "durationSec: p.row.durationSec", }; const EXPECTED_CALL_SITES = Object.keys(IDENTITY_OWNER).sort(); @@ -65,10 +77,8 @@ function read(rel: string): string { } /** - * Lines that actually invoke the picker. Comment lines are excluded because the - * write-time sibling (`lib/workouts/canonical-rows.ts`) names the read-time - * picker in its docblock to explain how the two layer, and that mention is - * documentation rather than a call. + * Lines that actually invoke the picker. Comment lines are excluded: a + * docblock that names the picker is documentation rather than a call. */ function invokesPicker(source: string): boolean { return source @@ -104,7 +114,9 @@ describe("every call site fetches the session identity", () => { // rows came from. Coarse in the safe direction — it can only fail to catch // a second unrelated select in the same file, never pass a caller that // fetches no duration at all. - expect(read(IDENTITY_OWNER[rel])).toContain("durationSec: true"); + expect(read(IDENTITY_OWNER[rel])).toContain( + IDENTITY_PROOF[rel] ?? "durationSec: true", + ); }); }); diff --git a/src/app/api/workouts/batch/route.ts b/src/app/api/workouts/batch/route.ts index 6d3e30d85..db2d1b1c7 100644 --- a/src/app/api/workouts/batch/route.ts +++ b/src/app/api/workouts/batch/route.ts @@ -91,7 +91,7 @@ import { UNSTABLE_EXTERNAL_ID_REASON, type UnstableExternalIdShape, } from "@/lib/validations/external-id"; -import { dedupeWorkoutBatch } from "@/lib/workouts/canonical-rows"; +import { pickCanonicalWorkoutRows } from "@/lib/measurements/pick-canonical-workout-rows"; import { WORKOUTS_WRITE_SCOPE } from "@/lib/workouts/scopes"; import { EXTERNAL_SOURCE } from "@/lib/measurements/external-source"; import { encryptRouteGeometry } from "@/lib/workouts/route-geometry-cipher"; @@ -494,41 +494,29 @@ async function postBatch(request: NextRequest): Promise { }); } - // v1.4.42 W5 — write-time cross-source dedup. The v1.5 iOS app - // drains a HealthKit observer queue that frequently carries the - // SAME logical workout from two paired sensors (Apple Watch + - // Withings ScanWatch). Without this pass, both rows persist (their - // `externalId`s differ → the `(userId, source, externalId)` unique - // index never fires) and the read-time canonical-picker has to - // carry the duplicate forward on every dashboard render. - // - // The helper is pure (no Prisma, no user lookups). It groups by - // `(userId, activityType, startedAt ± 90 s)`, prefers the canonical - // source ladder (APPLE_HEALTH > WITHINGS > MANUAL > IMPORT), and - // breaks ties on calories > earliest createdAt > input order. Rows - // dropped here surface to the iOS client as `duplicate` so the - // sync cursor advances past them identically to the externalId - // case below. - const canonicalPrepared = dedupeWorkoutBatch( + // Write-time cross-source dedup through the one workout picker the reads + // use, with the same source ladder: one logical workout from two sources + // in one batch (or one HealthKit session re-sent under a new id) lands + // once. Rows the picker hides surface to the client as `duplicate`, so + // the sync cursor advances past them like the externalId case below. + const canonicalPrepared = pickCanonicalWorkoutRows( prepared.map((p) => ({ + index: p.index, userId: p.row.userId, - activityType: p.row.sportType, + sportType: p.row.sportType, startedAt: p.row.startedAt as Date, + durationSec: p.row.durationSec, // Zod's `measurementSourceEnum.optional().default("MANUAL")` // guarantees `source` is set post-parse, but Prisma's - // `WorkoutCreateManyInput` types it optional. Default through - // the same fallback the validator uses so the picker's source - // ladder still resolves on the rare row where TypeScript - // can't prove the union. + // `WorkoutCreateManyInput` types it optional. source: (p.row.source ?? "MANUAL") as MeasurementSource, - caloriesKcal: p.row.totalEnergyKcal ?? null, - // No createdAt yet — the row is pre-insert; the tie-breaker - // falls through to input order via the `index` field below. - index: p.index, + avgHeartRate: p.row.avgHeartRate ?? null, + maxHeartRate: p.row.maxHeartRate ?? null, + metadata: p.row.metadata ?? null, })), userPriority?.sourcePriorityJson ?? null, ); - const survivingIndices = new Set(canonicalPrepared.map((r) => r.index ?? -1)); + const survivingIndices = new Set(canonicalPrepared.map((r) => r.index)); const droppedByWriteDedup: number[] = []; for (const p of prepared) { if (!survivingIndices.has(p.index)) { diff --git a/src/lib/measurements/pick-canonical-workout-rows.ts b/src/lib/measurements/pick-canonical-workout-rows.ts index cb97fd0ad..50fda8fdf 100644 --- a/src/lib/measurements/pick-canonical-workout-rows.ts +++ b/src/lib/measurements/pick-canonical-workout-rows.ts @@ -14,11 +14,9 @@ * logical run start within a minute of each other on both * sources; the rounding gives the cross-source matcher slack * without collapsing distinct back-to-back workouts. - * 2. For each bucket, walk the SOURCE priority ladder - * (`APPLE_HEALTH ≻ WITHINGS ≻ MANUAL ≻ IMPORT` per the existing - * source-priority validator's default) and pick the FIRST - * source that has any row in that bucket. Keep every row from - * the winning source; drop the rest. + * 2. For each bucket, walk the source ladder (`workoutSourceLadder`) + * and pick the FIRST source that has any row in that bucket. Keep + * every row from the winning source; drop the rest. * 3. Rows whose source isn't in the ladder pass through untouched * (fallback intent — never silently drop signal). * @@ -26,10 +24,10 @@ * (typically `ORDER BY startedAt ASC, id ASC` from the caller's * Prisma read). The output preserves input order. * - * Future v1.5.x tune: metric-aware picker per R-F open question #4 — - * route → Apple wins, HR zones → Withings wins. Out of scope for - * v1.4.30; the existing ladder ships first so the dashboard tile is - * unambiguous. + * This is the one workout picker. The reads use it, and so does the batch + * write route before it inserts: a write-time picker with its own window + * and a different default ladder kept or dropped a different row than the + * list then showed. */ import type { MeasurementSource } from "@/generated/prisma/client"; @@ -249,6 +247,17 @@ function enrichWinnersFromWhoop( return enriched; } +/** + * The source order for workouts: the `steps` metric's ladder, the + * canonical-activity bucket the Sources settings configure, falling back to + * its default when the user saved none or it does not parse. + */ +export function workoutSourceLadder( + userPriorityJson: unknown, +): readonly MeasurementSource[] { + return getSourceLadder(parseSourcePriority(userPriorityJson), "steps"); +} + /** * Pick canonical rows across sources for a user's workouts. * @@ -266,14 +275,7 @@ export function pickCanonicalWorkoutRows( ): T[] { if (rows.length === 0) return []; - // Workouts ride the `steps` metric's source ladder — the - // canonical-activity bucket. The user's iOS Settings → Sources UI - // configures the same ladder for the workout dashboard tile; - // unifying on "steps" keeps a single configuration surface rather - // than introducing a new `workouts` metric key. Future ladders for - // workout-specific tunes (route-vs-zones) ride a v1.5.x change. - const resolved = parseSourcePriority(userPriorityJson); - const sourceLadder = getSourceLadder(resolved, "steps"); + const sourceLadder = workoutSourceLadder(userPriorityJson); const buckets = new Map< string, diff --git a/src/lib/polar/__tests__/dedup.test.ts b/src/lib/polar/__tests__/dedup.test.ts index 0352b40be..21b379f01 100644 --- a/src/lib/polar/__tests__/dedup.test.ts +++ b/src/lib/polar/__tests__/dedup.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it } from "vitest"; -import { pickCanonicalWorkoutRows } from "@/lib/measurements/pick-canonical-workout-rows"; -import { DEFAULT_WORKOUT_SOURCE_PRIORITY } from "@/lib/sources/pick-canonical-workout"; +import { + pickCanonicalWorkoutRows, + workoutSourceLadder, +} from "@/lib/measurements/pick-canonical-workout-rows"; import { DEFAULT_SOURCE_PRIORITY } from "@/lib/validations/source-priority"; /** @@ -68,10 +70,10 @@ describe("POLAR workout dedup — read-time canonical picker", () => { }); describe("POLAR sits in the workout source ladders", () => { - it("is present in the workout default ladder, below WITHINGS, above STRAVA", () => { - const polar = DEFAULT_WORKOUT_SOURCE_PRIORITY.indexOf("POLAR"); - const withings = DEFAULT_WORKOUT_SOURCE_PRIORITY.indexOf("WITHINGS"); - const strava = DEFAULT_WORKOUT_SOURCE_PRIORITY.indexOf("STRAVA"); + it("is present in the workout ladder, below WITHINGS, above STRAVA", () => { + const polar = workoutSourceLadder(null).indexOf("POLAR"); + const withings = workoutSourceLadder(null).indexOf("WITHINGS"); + const strava = workoutSourceLadder(null).indexOf("STRAVA"); expect(polar).toBeGreaterThan(withings); expect(polar).toBeLessThan(strava); }); diff --git a/src/lib/strava/__tests__/dedup.test.ts b/src/lib/strava/__tests__/dedup.test.ts index 889c2e2a2..e74f9075f 100644 --- a/src/lib/strava/__tests__/dedup.test.ts +++ b/src/lib/strava/__tests__/dedup.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it } from "vitest"; -import { pickCanonicalWorkoutRows } from "@/lib/measurements/pick-canonical-workout-rows"; -import { DEFAULT_WORKOUT_SOURCE_PRIORITY } from "@/lib/sources/pick-canonical-workout"; +import { + pickCanonicalWorkoutRows, + workoutSourceLadder, +} from "@/lib/measurements/pick-canonical-workout-rows"; import { DEFAULT_SOURCE_PRIORITY } from "@/lib/validations/source-priority"; /** @@ -67,10 +69,10 @@ describe("STRAVA workout dedup — read-time canonical picker", () => { }); describe("STRAVA sits in the workout source ladders", () => { - it("is present in the workout default ladder, below the wearables, above MANUAL", () => { - const strava = DEFAULT_WORKOUT_SOURCE_PRIORITY.indexOf("STRAVA"); - const withings = DEFAULT_WORKOUT_SOURCE_PRIORITY.indexOf("WITHINGS"); - const manual = DEFAULT_WORKOUT_SOURCE_PRIORITY.indexOf("MANUAL"); + it("is present in the workout ladder, below the wearables, above MANUAL", () => { + const strava = workoutSourceLadder(null).indexOf("STRAVA"); + const withings = workoutSourceLadder(null).indexOf("WITHINGS"); + const manual = workoutSourceLadder(null).indexOf("MANUAL"); expect(strava).toBeGreaterThan(withings); expect(strava).toBeLessThan(manual); }); diff --git a/src/lib/strava/sport-map.ts b/src/lib/strava/sport-map.ts index c3702e9f8..b60c7a675 100644 --- a/src/lib/strava/sport-map.ts +++ b/src/lib/strava/sport-map.ts @@ -7,11 +7,10 @@ * to write `summary.sport_type ?? summary.type ?? detail?.sport_type ?? * "workout"` straight into `Workout.sportType`, so a Strava "Run" or * "VirtualRide" never matched a canonical bucket — `/insights/workouts` - * rendered every Strava row generic, and the read-time picker in - * `src/lib/sources/pick-canonical-workout.ts` treats any two GENERIC rows - * as sport-compatible, so a same-source brick session (a Strava "Ride" - * ending 15:00 immediately followed by a Strava "Run" starting 15:03) - * clustered into one and silently dropped a leg. Fitbit + * rendered every Strava row generic, and a picker that treats two generic + * rows as the same sport clusters a same-source brick session (a Strava + * "Ride" ending 15:00 immediately followed by a Strava "Run" starting + * 15:03) into one and silently drops a leg. Fitbit * (`mapFitbitSportType()` in `src/lib/fitbit/client.ts`), Google Health * (`mapGoogleHealthSportType()` in `src/lib/google-health/mappers.ts`), and * WHOOP (`mapWhoopSportType()` in `src/lib/whoop/sport-map.ts`) all diff --git a/tests/integration/workout-batch-create.test.ts b/tests/integration/workout-batch-create.test.ts index b7f7f3274..190fa34ef 100644 --- a/tests/integration/workout-batch-create.test.ts +++ b/tests/integration/workout-batch-create.test.ts @@ -356,8 +356,16 @@ describe("POST /api/workouts/batch (real Postgres)", () => { it("preserves same-source workouts and surfaces exact re-posts as duplicates", async () => { const { POST } = await import("@/app/api/workouts/batch/route"); + // Two sessions of one source in the same five-minute slot: different + // start and length, so two workouts. const body = { - workouts: [baseWorkout("hk-uuid-dup-1"), baseWorkout("hk-uuid-dup-2")], + workouts: [ + baseWorkout("hk-uuid-dup-1"), + baseWorkout("hk-uuid-dup-2", { + startedAt: "2026-05-14T06:32:00.000Z", + endedAt: "2026-05-14T06:50:00.000Z", + }), + ], }; const first = await POST(makeRequest(body)); @@ -393,6 +401,31 @@ describe("POST /api/workouts/batch (real Postgres)", () => { expect(stored).toBe(2); }); + it("stores a session re-sent under a new id once, the way the list shows it", async () => { + // HealthKit can hand the same session over twice with two external ids. + // The list collapses it to one workout; the write keeps one row and + // reports the other as a duplicate instead of storing what no read shows. + const { POST } = await import("@/app/api/workouts/batch/route"); + const res = await POST( + makeRequest({ + workouts: [ + baseWorkout("hk-uuid-resend-1"), + baseWorkout("hk-uuid-resend-2"), + ], + }), + ); + expect(res.status).toBe(200); + const json = (await res.json()) as { + data: { inserted: number; duplicates: number }; + }; + expect(json.data.inserted).toBe(1); + expect(json.data.duplicates).toBe(1); + const stored = await getPrismaClient().workout.count({ + where: { userId: TEST_USER_ID }, + }); + expect(stored).toBe(1); + }); + it("replays a cached response when the same Idempotency-Key is reused", async () => { const { POST } = await import("@/app/api/workouts/batch/route"); diff --git a/tests/integration/workout-batch-race.test.ts b/tests/integration/workout-batch-race.test.ts index bc5a9361c..d55aed62b 100644 --- a/tests/integration/workout-batch-race.test.ts +++ b/tests/integration/workout-batch-race.test.ts @@ -182,9 +182,9 @@ describe("POST /api/workouts/batch — concurrent-write race", () => { const { POST } = await import("@/app/api/workouts/batch/route"); // Batch A — a cross-source pair one minute apart plus one shared key. - // The v1.4.42 `dedupeWorkoutBatch` picker collapses the Apple Health + - // Manual pair (same activity, within the 90 s window) to one - // surviving row, leaving 2 effective rows per batch. + // The workout picker collapses the Apple Health + Manual pair (same + // activity, same five-minute slot) to one surviving row, leaving 2 + // effective rows per batch. const batchA: WorkoutFixture[] = [ workout("hk-uuid-only-a-0", 1), workout("hk-uuid-only-a-1", 2, "MANUAL"),