mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-13 14:32:03 +00:00
fix(workouts): resolve duplicate clusters the same way on every surface
The cross-source picker anchors each duplicate cluster on its first start, so which cluster a workout lands in depends on the workouts around it. The detail route still read the old fixed five-minute clock slot, and every other reader handed the picker only the rows of its own range. The detail of a twin that straddled a slot boundary declared itself canonical while the list hid it, and runs at 09:58, 10:02 and 10:06 were grouped one way by a read starting at 09:55 and another by a read starting at 10:00. Every reader that decides which workout is canonical (list, detail and its previous session, activity calendar, day view, doctor report, Coach, briefing and correlation blocks, strain, per-sport context, the activity insight's history, the batch write) now reads through one windowed reader. It reads a duplicate window past the end of the range and reaches back until every sport in the range starts at a workout that is an anchor whatever came before it, then picks over everything read and keeps the rows that start inside the range. The per-sport baseline and the insight history leave out the whole cluster of the current session instead of the row alone, which left its twin behind as a session of its own. The batch write clusters its rows together with the stored workouts around them and drops only a row that loses to another row of the same batch. A property test checks the windowed read against a whole-history read for random dense histories and ranges; route tests pin list, detail, deep link, previous session and the batch write against the database.
This commit is contained in:
1 parent
3decab47a0
commit
466dfb3785
33 files changed
+1486
-472
No files matched your search
@@ -6,9 +6,9 @@
|
||||
* canonical envelope per workout. The endpoint:
|
||||
*
|
||||
* - Returns the canonical row when the requested id wins its
|
||||
* `(startedAt slot, sportType)` cluster against the user's
|
||||
* source-priority ladder. The picker reuses
|
||||
* `pickCanonicalWorkoutRows()` from v1.4.30 so the dedup contract
|
||||
* duplicate cluster against the user's source-priority ladder. The
|
||||
* cluster is resolved through the same windowed read the list uses
|
||||
* (`src/lib/workouts/canonical-read.ts`), so the dedup contract
|
||||
* stays in sync with the list endpoint.
|
||||
* - When the requested id is a non-canonical twin (e.g. the user
|
||||
* opened a deep-link to a Withings row but the cluster's winner is
|
||||
@@ -40,7 +40,14 @@ import {
|
||||
lockPersonalRecordsForUser,
|
||||
workoutPrSlotsForSport,
|
||||
} from "@/lib/personal-records/pr-detection-worker";
|
||||
import { pickCanonicalWorkoutRows } from "@/lib/measurements/pick-canonical-workout-rows";
|
||||
import {
|
||||
canonicalClusterOf,
|
||||
canonicalWorkoutsInRange,
|
||||
readWorkoutsAroundRange,
|
||||
type CanonicalReadRow,
|
||||
type WorkoutBoundedRead,
|
||||
type WorkoutStartBounds,
|
||||
} from "@/lib/workouts/canonical-read";
|
||||
import { requireModuleEnabled } from "@/lib/modules/gate";
|
||||
import { aiCapabilityToServe } from "@/lib/ai/capabilities/gate";
|
||||
import { getAgeFromDateOfBirth } from "@/lib/analytics/pulse-targets";
|
||||
@@ -60,6 +67,38 @@ import { userDayKey } from "@/lib/tz/format";
|
||||
|
||||
type RouteParams = { params: Promise<{ id: string }> };
|
||||
|
||||
/** How many earlier rows one step of the previous-session search reads. */
|
||||
const PREVIOUS_SESSION_BATCH = 20;
|
||||
/** Steps before the search gives up; each reaches past the last one. */
|
||||
const PREVIOUS_SESSION_MAX_STEPS = 5;
|
||||
|
||||
/**
|
||||
* The newest canonical workout of the sport that starts before `before` (the
|
||||
* first start of the current session's cluster). A batch of earlier rows names
|
||||
* the span to resolve; the span is read through the windowed reader so its
|
||||
* twins are resolved the way the list resolves them. A span whose rows are all
|
||||
* hidden by a twin further back moves the search back one batch.
|
||||
*/
|
||||
async function previousCanonicalWorkoutId<T extends CanonicalReadRow>(
|
||||
read: WorkoutBoundedRead<T>,
|
||||
latestStartsBefore: (to: Date, take: number) => Promise<Date[]>,
|
||||
before: Date,
|
||||
priorityJson: unknown,
|
||||
): Promise<string | null> {
|
||||
let to = before;
|
||||
for (let step = 0; step < PREVIOUS_SESSION_MAX_STEPS; step += 1) {
|
||||
const earlier = await latestStartsBefore(to, PREVIOUS_SESSION_BATCH);
|
||||
if (earlier.length === 0) return null;
|
||||
const from = earlier[earlier.length - 1]!;
|
||||
const windowed = await readWorkoutsAroundRange({ from, to: before }, read);
|
||||
const canonical = canonicalWorkoutsInRange(windowed, priorityJson);
|
||||
const newest = canonical[canonical.length - 1];
|
||||
if (newest) return newest.id;
|
||||
to = from;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export const GET = apiHandler(
|
||||
async (request: NextRequest, { params }: RouteParams) => {
|
||||
const { user } = await requireAuth();
|
||||
@@ -113,38 +152,38 @@ export const GET = apiHandler(
|
||||
return apiError("Workout not found", 404);
|
||||
}
|
||||
|
||||
// Resolve the canonical winner for the row's cluster. The picker
|
||||
// reads every workout in the same 5-minute slot and sport-type
|
||||
// bucket; in practice that's ≤ 4 rows (Apple + Withings + the two
|
||||
// legacy sources). The query is bounded so the round-trip is cheap.
|
||||
const slotMs = 5 * 60 * 1000;
|
||||
const slotStart = new Date(
|
||||
Math.floor(row.startedAt.getTime() / slotMs) * slotMs,
|
||||
// Resolve the canonical winner for the row's cluster. The read goes
|
||||
// through the same windowed reader as the list, so the cluster drawn here
|
||||
// is the one the list draws: a twin that straddles a clock slot boundary,
|
||||
// or one whose cluster begins with an earlier workout, resolves to the row
|
||||
// the list shows.
|
||||
const readSameSport = (bounds: WorkoutStartBounds) =>
|
||||
prisma.workout.findMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
sportType: row.sportType,
|
||||
startedAt: bounds,
|
||||
},
|
||||
orderBy: [{ startedAt: "asc" }, { id: "asc" }],
|
||||
select: {
|
||||
id: true,
|
||||
source: true,
|
||||
startedAt: true,
|
||||
sportType: true,
|
||||
// Part of the canonical session identity consulted by the picker —
|
||||
// without it a HealthKit re-send of this very workout stays in the
|
||||
// cluster and the deep link can resolve to the duplicate instead of
|
||||
// the row the list shows.
|
||||
durationSec: true,
|
||||
avgHeartRate: true,
|
||||
maxHeartRate: true,
|
||||
metadata: true,
|
||||
},
|
||||
});
|
||||
const around = await readWorkoutsAroundRange(
|
||||
{ from: row.startedAt, to: row.startedAt, inclusiveEnd: true },
|
||||
readSameSport,
|
||||
);
|
||||
const slotEnd = new Date(slotStart.getTime() + slotMs);
|
||||
|
||||
const clusterRows = await prisma.workout.findMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
sportType: row.sportType,
|
||||
startedAt: { gte: slotStart, lt: slotEnd },
|
||||
},
|
||||
orderBy: [{ startedAt: "asc" }, { id: "asc" }],
|
||||
select: {
|
||||
id: true,
|
||||
source: true,
|
||||
startedAt: true,
|
||||
sportType: true,
|
||||
// Part of the canonical session identity consulted by
|
||||
// `pickCanonicalWorkoutRows` below — without it a HealthKit re-send of
|
||||
// this very workout stays in the cluster and the deep link can resolve
|
||||
// to the duplicate instead of the row the list shows.
|
||||
durationSec: true,
|
||||
avgHeartRate: true,
|
||||
maxHeartRate: true,
|
||||
metadata: true,
|
||||
},
|
||||
});
|
||||
|
||||
const userRow = await prisma.user.findUnique({
|
||||
where: { id: user.id },
|
||||
@@ -158,17 +197,15 @@ export const GET = apiHandler(
|
||||
const timezone = await resolveUserTimezone(user.id);
|
||||
const dayKey = userDayKey(row.startedAt, timezone);
|
||||
|
||||
const canonicalCluster = pickCanonicalWorkoutRows(
|
||||
clusterRows,
|
||||
const cluster = canonicalClusterOf(
|
||||
around,
|
||||
row.id,
|
||||
userRow?.sourcePriorityJson ?? null,
|
||||
);
|
||||
// The canonical pick may carry multiple rows (same winning source);
|
||||
// pick the closest in time to the requested row so deep-link
|
||||
// redirects land on the most similar twin.
|
||||
const canonical =
|
||||
canonicalCluster.find((c) => c.id === row.id) ??
|
||||
canonicalCluster[0] ??
|
||||
null;
|
||||
// The cluster's canonical rows, the requested row first when it is one of
|
||||
// them and otherwise the nearest in time, so a deep link to a hidden twin
|
||||
// lands on the most similar row.
|
||||
const canonical = cluster?.canonical[0] ?? null;
|
||||
const effectiveAvgHeartRate = canonical?.avgHeartRate ?? row.avgHeartRate;
|
||||
const effectiveMaxHeartRate = canonical?.maxHeartRate ?? row.maxHeartRate;
|
||||
|
||||
@@ -233,43 +270,29 @@ export const GET = apiHandler(
|
||||
// The session before this one in the same sport. "How does this compare
|
||||
// to last time?" is the most likely next question after reading a
|
||||
// workout, and today it costs a trip back to the list plus a scan.
|
||||
// Scoped to rows that start before this cluster's slot, so a twin of THIS
|
||||
// session can never be offered as the previous one, and run through the
|
||||
// same canonical picker so the link lands on the row the list shows.
|
||||
const previousRows = await prisma.workout.findMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
sportType: row.sportType,
|
||||
startedAt: { lt: slotStart },
|
||||
},
|
||||
orderBy: [{ startedAt: "desc" }, { id: "desc" }],
|
||||
// Enough to survive a run of same-session twins and re-sends without
|
||||
// reading the whole history; the picker collapses them to a handful.
|
||||
take: 20,
|
||||
select: {
|
||||
id: true,
|
||||
source: true,
|
||||
startedAt: true,
|
||||
sportType: true,
|
||||
durationSec: true,
|
||||
avgHeartRate: true,
|
||||
maxHeartRate: true,
|
||||
metadata: true,
|
||||
},
|
||||
});
|
||||
// The picker takes rows in ascending order and does not promise to hand
|
||||
// them back sorted, so pick the newest survivor explicitly.
|
||||
const previousWorkoutId =
|
||||
pickCanonicalWorkoutRows(
|
||||
[...previousRows].reverse(),
|
||||
userRow?.sourcePriorityJson ?? null,
|
||||
).reduce<{ id: string; startedAt: Date } | null>(
|
||||
(newest, candidate) =>
|
||||
newest === null || candidate.startedAt > newest.startedAt
|
||||
? candidate
|
||||
: newest,
|
||||
null,
|
||||
)?.id ?? null;
|
||||
// Scoped to rows that start before this session's cluster, so a twin of
|
||||
// THIS session can never be offered as the previous one, and resolved
|
||||
// through the same windowed read so the link lands on the row the list
|
||||
// shows.
|
||||
const clusterStart = cluster?.members[0]?.startedAt ?? row.startedAt;
|
||||
const previousWorkoutId = await previousCanonicalWorkoutId(
|
||||
readSameSport,
|
||||
async (to, take) =>
|
||||
(
|
||||
await prisma.workout.findMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
sportType: row.sportType,
|
||||
startedAt: { lt: to },
|
||||
},
|
||||
orderBy: [{ startedAt: "desc" }, { id: "desc" }],
|
||||
take,
|
||||
select: { startedAt: true },
|
||||
})
|
||||
).map((earlier) => earlier.startedAt),
|
||||
clusterStart,
|
||||
userRow?.sourcePriorityJson ?? null,
|
||||
);
|
||||
|
||||
// A route whose track cannot be read (a sealed value that no longer
|
||||
// opens, or an old row stored as a JSON null) is reported as no route at
|
||||
|
||||
@@ -128,6 +128,19 @@ function mockInsertReturning(
|
||||
}) => (args.where === undefined && args.select?.id ? rows : [])) as never);
|
||||
}
|
||||
|
||||
/**
|
||||
* Rows the pre-flight `(source, externalId)` probe finds. The probe is the
|
||||
* read with an `OR` of keys; the windowed read of the stored workouts around
|
||||
* the batch (a `startedAt` range) finds nothing here.
|
||||
*/
|
||||
function mockExistingByKey(
|
||||
rows: Array<{ source: string; externalId: string }>,
|
||||
) {
|
||||
vi.mocked(prisma.workout.findMany).mockImplementation((async (args: {
|
||||
where?: { OR?: unknown };
|
||||
}) => (args.where?.OR ? rows : [])) as never);
|
||||
}
|
||||
|
||||
function validWorkout(
|
||||
externalId: string,
|
||||
overrides: Record<string, unknown> = {},
|
||||
@@ -345,9 +358,9 @@ describe("POST /api/workouts/batch — per-entry status envelope", () => {
|
||||
});
|
||||
|
||||
it("flags entries already in the DB as duplicate without re-inserting", async () => {
|
||||
vi.mocked(prisma.workout.findMany).mockResolvedValue([
|
||||
mockExistingByKey([
|
||||
{ source: "APPLE_HEALTH", externalId: "uuid-existing" },
|
||||
] as never);
|
||||
]);
|
||||
vi.mocked(prisma.workout.createMany).mockResolvedValue({ count: 1 });
|
||||
const res = await POST(
|
||||
makeRequest({
|
||||
@@ -825,9 +838,7 @@ describe("POST /api/workouts/batch — PR detection enqueue (v1.4.25 W16c)", ()
|
||||
// stays 0 but duplicateCount becomes 1. The hook fires anyway —
|
||||
// the detector is cheap and a duplicate batch can still reveal a
|
||||
// PR if the row was written by another path between dispatches.
|
||||
vi.mocked(prisma.workout.findMany).mockResolvedValue([
|
||||
{ source: "APPLE_HEALTH", externalId: "uuid-dup" } as never,
|
||||
]);
|
||||
mockExistingByKey([{ source: "APPLE_HEALTH", externalId: "uuid-dup" }]);
|
||||
vi.mocked(prisma.workout.createMany).mockResolvedValue({ count: 0 });
|
||||
const res = await POST(
|
||||
makeRequest({ workouts: [validWorkout("uuid-dup")] }),
|
||||
|
||||
@@ -358,9 +358,11 @@ describe("GET /api/workouts — canonical dedup", () => {
|
||||
where: {
|
||||
userId: "user-1",
|
||||
sportType: "running",
|
||||
// The range plus one duplicate window on either side, so a twin
|
||||
// just outside the filter still decides which row is canonical.
|
||||
startedAt: {
|
||||
gte: new Date("2026-01-01T00:00:00.000Z"),
|
||||
lte: new Date("2026-01-02T00:00:00.000Z"),
|
||||
gte: new Date("2025-12-31T23:55:00.000Z"),
|
||||
lt: new Date("2026-01-02T00:05:00.000Z"),
|
||||
},
|
||||
},
|
||||
}),
|
||||
|
||||
@@ -92,11 +92,11 @@ import {
|
||||
UNSTABLE_EXTERNAL_ID_REASON,
|
||||
type UnstableExternalIdShape,
|
||||
} from "@/lib/validations/external-id";
|
||||
import { pickCanonicalWorkoutRows } from "@/lib/measurements/pick-canonical-workout-rows";
|
||||
import { batchWorkoutIndicesToKeep } from "@/lib/workouts/batch-dedup";
|
||||
import { WORKOUTS_WRITE_SCOPE } from "@/lib/workouts/scopes";
|
||||
import { EXTERNAL_SOURCE } from "@/lib/measurements/external-source";
|
||||
import { encryptRouteGeometry } from "@/lib/workouts/route-geometry-cipher";
|
||||
import { Prisma, type MeasurementSource } from "@/generated/prisma/client";
|
||||
import { Prisma } from "@/generated/prisma/client";
|
||||
|
||||
// v1.4.25 — push-suppression threshold for workout PRs. A batch
|
||||
// larger than this fires the detection job with `silent: true` so a
|
||||
@@ -508,29 +508,16 @@ async function postBatch(request: NextRequest): Promise<Response> {
|
||||
});
|
||||
}
|
||||
|
||||
// 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,
|
||||
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.
|
||||
source: (p.row.source ?? "MANUAL") as MeasurementSource,
|
||||
avgHeartRate: p.row.avgHeartRate ?? null,
|
||||
maxHeartRate: p.row.maxHeartRate ?? null,
|
||||
metadata: p.row.metadata ?? null,
|
||||
})),
|
||||
// Write-time cross-source dedup through the one workout clustering the
|
||||
// reads use (see `batchWorkoutIndicesToKeep`): one logical workout from two
|
||||
// sources in one batch, or one HealthKit session re-sent under a new id,
|
||||
// lands once. Rows hidden here surface to the client as `duplicate`, so the
|
||||
// sync cursor advances past them like the externalId case below.
|
||||
const survivingIndices = await batchWorkoutIndicesToKeep(
|
||||
user.id,
|
||||
prepared,
|
||||
userPriority?.sourcePriorityJson ?? null,
|
||||
);
|
||||
const survivingIndices = new Set(canonicalPrepared.map((r) => r.index));
|
||||
const droppedByWriteDedup: number[] = [];
|
||||
for (const p of prepared) {
|
||||
if (!survivingIndices.has(p.index)) {
|
||||
|
||||
Reference in new issue
Block a user