fix(workouts): one canonical-workout picker with one source order

Three pickers decided which of two overlapping workouts to keep: the
read-time picker the list, the detail page and the Coach use, a
write-time batch picker with its own 90-second window, and an unwired
field-merge picker. They used different source orders (the `steps`
ladder, or a separate workout default when no order was saved), so the
batch route could keep a row the list then ranked below another.

The batch route now runs the read-time picker over the rows it is about
to insert, with the ladder from `workoutSourceLadder`, and reports what
it hides as duplicates. A session HealthKit hands over twice under two
ids is stored once, as the list already showed it. The write-time and
the unwired picker are removed.
This commit is contained in:
Marc-André Bombeck committed 2026-10-20 04:37:39 +02:00
1 parent 2861075676
commit e355c6120f
8 files changed
+109 -71

No files matched your search

@@ -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<string, string> = {
// 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<string, string> = {
"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",
);
});
});
+15 -27
View File
@@ -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<Response> {
});
}
// 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)) {
@@ -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<T extends WorkoutPickerRow>(
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 extends WorkoutPickerRow>(
): 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,
+8 -6
View File
@@ -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);
});
+8 -6
View File
@@ -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);
});
+4 -5
View File
@@ -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
+34 -1
View File
@@ -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");
+3 -3
View File
@@ -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"),