fix(workout): reject endedAt <= startedAt + zero-duration PR guard

A workout with `endedAt <= startedAt` produced a non-positive
`durationSec` that downstream consumers treated as a real zero —
worst case a fastest-5km PR of zero seconds, which would lock in
until manual cleanup. Add a `.superRefine` clause on
`createWorkoutSchema` so the ingest gate refuses the row.

The PR detector stays defensive against any pre-gate row or future
code path that bypasses the schema: MIN-direction slots now always
filter `durationSec > 0` regardless of which field carries the PR
value. A MIN slot sorts ascending, so the smallest value wins — a
zero-duration row would always be picked first.

Fix-M / code-M2.
This commit is contained in:
Marc-André Bombeck committed 2026-05-14 22:27:32 +02:00
1 parent 733fc2cec7
commit 6f3b901c3e
4 files changed
+91

No files matched your search

@@ -595,6 +595,44 @@ describe("detectPersonalRecordsForUser — workout slots", () => {
});
});
describe("detectPersonalRecordsForUser — zero-duration MIN guard", () => {
// Regression for Fix-M / code-M2: the createWorkoutSchema gate now
// rejects endedAt <= startedAt, but the PR detector stays defensive
// against any historical row (pre-gate) or future code path that
// produces a zero-duration workout. A MIN-direction slot (fastest
// 5 km) sorts ascending — a zero-second row would always win.
it("ignores zero-duration runs when scanning fastest_5km_time", async () => {
const runs: FakeWorkoutRow[] = [];
// Seed: enough legitimate 5 km finishes to clear the warm-up gate.
for (let i = 0; i < PR_DETECTION_WARMUP_THRESHOLD; i++) {
runs.push(
workout("running", 1800 + i * 30, 5100 + i * 100, i + 2, `5k-${i}`),
);
}
// Poison row: a workout with durationSec === 0 at the same
// distance threshold. A MIN-direction selector without the guard
// would pick this row as "fastest".
runs.push(workout("running", 0, 5200, 1, "zero-poison"));
const state = {
measurements: [],
workouts: runs,
personalRecords: [] as FakePersonalRecordRow[],
};
const prisma = makeFakePrisma(state);
await detectPersonalRecordsForUser(USER, { prisma });
const pr = state.personalRecords.find(
(r) => r.metricSlot === "fastest_5km_time",
);
expect(pr).toBeDefined();
// The PR must come from the seeded set, not the poison row.
expect(pr?.value).toBeGreaterThan(0);
expect(pr?.value).toBe(1800);
});
});
describe("detectPersonalRecordsForUser — flags + drift guard", () => {
it("propagates the silent flag onto the result envelope", async () => {
const state = {
@@ -290,12 +290,27 @@ export async function detectPersonalRecordsForUser(
for (const slot of WORKOUT_SLOTS) {
scanned += 1;
// MIN-direction slots sort ascending — the smallest value wins. A
// zero-duration workout would always become "best" before any
// legitimate row could compete. The schema gate on
// `createWorkoutSchema` rejects endedAt <= startedAt at ingest
// time, but the worker stays defensive: any historical row that
// pre-dates the gate (or any future code path that bypasses the
// schema) must not be allowed to lock in a zero-second PR. The
// `durationSec > 0` clause is always present when `slot.field` is
// durationSec; MIN-direction slots additionally enforce it
// regardless of which field carries the PR value, so a hypothetical
// future MIN slot on a distance field still rejects zero-duration
// rows on principle.
const baseWhere = {
userId,
sportType: slot.sportType,
...(slot.minDistanceM !== undefined
? { totalDistanceM: { gte: slot.minDistanceM } }
: {}),
...(slot.direction === PersonalRecordDirection.MIN
? { durationSec: { gt: 0 } as { gt: number } }
: {}),
[slot.field]: { gt: 0 } as { gt: number },
} as const;
@@ -190,6 +190,31 @@ describe("createWorkoutSchema", () => {
).toThrow(/20000-point cap/);
});
it("rejects a workout whose endedAt is before startedAt", () => {
// Regression for the W16b ingest gate: a reversed pair produced a
// negative `durationSec` that downstream consumers (PR detector,
// weekly report) treated as a real zero — a fastest-5km PR of
// zero seconds would lock in until manual cleanup. The schema
// refuses the row at parse time.
expect(() =>
createWorkoutSchema.parse({
...minimalRun,
startedAt: "2026-05-14T07:15:00.000Z",
endedAt: "2026-05-14T06:30:00.000Z",
}),
).toThrow(/endedAt must be strictly after startedAt/);
});
it("rejects a zero-duration workout (endedAt === startedAt)", () => {
expect(() =>
createWorkoutSchema.parse({
...minimalRun,
startedAt: "2026-05-14T06:30:00.000Z",
endedAt: "2026-05-14T06:30:00.000Z",
}),
).toThrow(/endedAt must be strictly after startedAt/);
});
it("rejects a route whose sampleTimestamps length mismatches coordinates", () => {
// Cross-field invariant: per-sample HR / speed must line up against
// the parallel coordinate index, so a desynced pair silently
+13
View File
@@ -97,6 +97,12 @@ export type WorkoutRouteSamples = z.infer<typeof workoutRouteSamplesSchema>;
* locked contract and the v1.4.26 XML import worker has the same target.
*
* Cross-field invariants enforced via `.superRefine()`:
* - `endedAt` MUST be strictly greater than `startedAt`. A workout
* with `endedAt <= startedAt` produces a non-positive
* `durationSec`, which downstream consumers (PR detector, weekly
* report, dashboards) treat as a "real" zero — locking in a
* fastest-5km time of zero seconds is far worse than refusing the
* ingest at the schema gate.
* - When `route.sampleTimestamps` is present, its length MUST equal
* `route.geometry.coordinates.length`. A desynced pair silently
* degrades downstream analytics (per-sample HR / speed would line
@@ -127,6 +133,13 @@ export const createWorkoutSchema = z
.optional(),
})
.superRefine((value, ctx) => {
if (value.endedAt.getTime() <= value.startedAt.getTime()) {
ctx.addIssue({
code: "custom",
path: ["endedAt"],
message: "endedAt must be strictly after startedAt",
});
}
const samples = value.route?.sampleTimestamps;
const coords = value.route?.geometry.coordinates;
if (samples && coords && samples.length !== coords.length) {