diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 58dc9d0be..1013348a3 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -29688,11 +29688,22 @@ components: - analyte - reason additionalProperties: false + outcome: + type: string + enum: + - empty + - failed + - partial + - success + description: How the commit reads, resolved from the counts rather than the status code. `success` only when every + confirmed row was written; `partial` when some were skipped; `failed` when nothing was written and at least + one row was skipped; `empty` when there was nothing to write. required: - inserted - skipped + - outcome additionalProperties: false - description: 'The rows written (`source: "OCR"`) and those skipped as commit-time duplicates.' + description: 'The rows written (`source: "OCR"`), those skipped as commit-time duplicates, and the resolved outcome.' OcrCommittedLabResult: type: object properties: diff --git a/src/app/api/fitbit/sync/route.ts b/src/app/api/fitbit/sync/route.ts index 2f7641fc7..00be8309a 100644 --- a/src/app/api/fitbit/sync/route.ts +++ b/src/app/api/fitbit/sync/route.ts @@ -3,6 +3,7 @@ import { apiSuccess, apiError } from "@/lib/api-response"; import { checkRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; import { syncUserFitbit } from "@/lib/fitbit/sync"; +import { resolveSyncOutcome } from "@/lib/outcome/written-outcome"; import { NextRequest } from "next/server"; /** @@ -58,6 +59,9 @@ export const POST = apiHandler(async (request: NextRequest) => { } } - const { imported } = await syncUserFitbit(user.id, { fullSync }); - return apiSuccess({ imported, fullSync }); + // The verdict is resolved here, not in the card: a run that imported nothing + // because every resource failed is not the same answer as a quiet hour, and + // both used to render as a green tick with a count. + const result = await syncUserFitbit(user.id, { fullSync }); + return apiSuccess({ ...resolveSyncOutcome(result), fullSync }); }); diff --git a/src/app/api/google-health/sync/__tests__/route.test.ts b/src/app/api/google-health/sync/__tests__/route.test.ts index 63b9686ab..e51d4df0d 100644 --- a/src/app/api/google-health/sync/__tests__/route.test.ts +++ b/src/app/api/google-health/sync/__tests__/route.test.ts @@ -56,6 +56,28 @@ describe("POST /api/google-health/sync", () => { expect(envelope.error).toBe("Google Health sync failed"); }); + it("reports a run that failed after writing some resources as a partial", async () => { + // A 502 threw the honest half away: rows DID land, and the card showed the + // generic error instead of "imported N, some data is behind". + vi.mocked(syncUserGoogleHealth).mockResolvedValue({ + imported: 7, + failed: true, + }); + + const response = await POST(request()); + const envelope = (await response.json()) as { + data: { imported: number; failed: boolean; outcome: string }; + error: string | null; + }; + + expect(response.status).toBe(200); + expect(envelope.data).toMatchObject({ + imported: 7, + failed: true, + outcome: "partial", + }); + }); + it("keeps the successful imported-count response unchanged", async () => { vi.mocked(syncUserGoogleHealth).mockResolvedValue({ imported: 7, @@ -70,7 +92,7 @@ describe("POST /api/google-health/sync", () => { expect(response.status).toBe(200); expect(envelope).toEqual({ - data: { imported: 7, fullSync: false }, + data: { imported: 7, failed: false, outcome: "success", fullSync: false }, error: null, }); expect(syncUserGoogleHealth).toHaveBeenCalledWith("u1", { diff --git a/src/app/api/google-health/sync/route.ts b/src/app/api/google-health/sync/route.ts index b504ba329..2cb7b99b3 100644 --- a/src/app/api/google-health/sync/route.ts +++ b/src/app/api/google-health/sync/route.ts @@ -3,6 +3,7 @@ import { apiSuccess, apiError } from "@/lib/api-response"; import { checkRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; import { syncUserGoogleHealth } from "@/lib/google-health/sync"; +import { resolveSyncOutcome } from "@/lib/outcome/written-outcome"; import { NextRequest } from "next/server"; /** @@ -55,11 +56,12 @@ export const POST = apiHandler(async (request: NextRequest) => { } } - const { imported, failed } = await syncUserGoogleHealth(user.id, { - fullSync, - }); - if (failed) { + const result = await syncUserGoogleHealth(user.id, { fullSync }); + // A run that failed AND wrote nothing is an error, as it always was. A run + // that failed after writing some of its resources is a partial, and answering + // 502 for it threw away the honest half of the result. + if (result.failed && result.imported === 0) { return apiError("Google Health sync failed", 502); } - return apiSuccess({ imported, fullSync }); + return apiSuccess({ ...resolveSyncOutcome(result), fullSync }); }); diff --git a/src/app/api/labs/ocr/commit/route.ts b/src/app/api/labs/ocr/commit/route.ts index 5d3f37b82..42b6b1d77 100644 --- a/src/app/api/labs/ocr/commit/route.ts +++ b/src/app/api/labs/ocr/commit/route.ts @@ -9,6 +9,11 @@ * * `userId` is always narrowed from the session — never a body field. The write * `data` object is built field-by-field (no mass assignment). + * + * The response carries the resolved `outcome` alongside the two lists. The + * dialog used to raise a green toast on any 200, so a re-scan in which every + * confirmed row turned out to be a duplicate reported "saved 0 readings" under + * a tick. The verdict is the server's to compute; the dialog only renders it. */ import { NextRequest } from "next/server"; import { fireAndForget } from "@/lib/logging/fire-and-forget"; @@ -33,6 +38,7 @@ import { type InsertedLabForLink, } from "@/lib/labs/vault-link"; import { annotate } from "@/lib/logging/context"; +import { classifyWrittenOutcome } from "@/lib/outcome/written-outcome"; import { ocrCommitSchema, type OcrCommitRow, @@ -235,5 +241,12 @@ async function commitOcrRows(request: NextRequest) { } } - return apiSuccess({ inserted, skipped }); + return apiSuccess({ + inserted, + skipped, + outcome: classifyWrittenOutcome({ + written: inserted.length, + skipped: skipped.length, + }), + }); } diff --git a/src/app/api/nightscout/sync/__tests__/route.test.ts b/src/app/api/nightscout/sync/__tests__/route.test.ts index a133cb8c5..15585a50b 100644 --- a/src/app/api/nightscout/sync/__tests__/route.test.ts +++ b/src/app/api/nightscout/sync/__tests__/route.test.ts @@ -57,18 +57,65 @@ beforeEach(() => { describe("POST /api/nightscout/sync", () => { it("syncs the session user and answers with the freshly inserted count", async () => { - vi.mocked(syncUserNightscout).mockResolvedValue(36); + vi.mocked(syncUserNightscout).mockResolvedValue({ + imported: 36, + failed: false, + }); const response = await post(request()); expect(response.status).toBe(200); expect(await envelope(response)).toEqual({ - data: { imported: 36 }, + data: { imported: 36, failed: false, outcome: "success" }, error: null, }); expect(syncUserNightscout).toHaveBeenCalledWith("u1"); }); + it("does not call a window whose every row was refused a success", async () => { + // The defect: 200 + `imported: 0` rendered as a green tick with a count, + // when in fact not one SGV row reached the database. + vi.mocked(syncUserNightscout).mockResolvedValue({ + imported: 0, + failed: true, + }); + + const response = await post(request()); + + expect(response.status).toBe(200); + expect((await envelope(response)).data).toEqual({ + imported: 0, + failed: true, + outcome: "failed", + }); + }); + + it("calls a window that wrote some rows and lost others a partial", async () => { + vi.mocked(syncUserNightscout).mockResolvedValue({ + imported: 30, + failed: true, + }); + + const response = await post(request()); + + expect((await envelope(response)).data).toMatchObject({ + outcome: "partial", + }); + }); + + it("calls a window that found nothing new empty, not a success", async () => { + vi.mocked(syncUserNightscout).mockResolvedValue({ + imported: 0, + failed: false, + }); + + const response = await post(request()); + + expect((await envelope(response)).data).toMatchObject({ + outcome: "empty", + }); + }); + it("refuses an unauthenticated caller before syncing anything", async () => { vi.mocked(requireAuth).mockRejectedValue( new HttpError(401, "Not authenticated"), diff --git a/src/app/api/nightscout/sync/route.ts b/src/app/api/nightscout/sync/route.ts index 1c379ce79..99504bb4d 100644 --- a/src/app/api/nightscout/sync/route.ts +++ b/src/app/api/nightscout/sync/route.ts @@ -3,12 +3,15 @@ import { apiSuccess, apiError } from "@/lib/api-response"; import { checkRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; import { syncUserNightscout } from "@/lib/nightscout/sync"; +import { resolveSyncOutcome } from "@/lib/outcome/written-outcome"; /** * Manually trigger a Nightscout sync for the current user (v1.32.28). * * Pulls the recent SGV window from the configured instance; `imported` counts - * the readings that were genuinely new this run, not the ones re-confirmed. + * the readings that were genuinely new this run, not the ones re-confirmed, + * and `outcome` says how that count reads — a window in which every row was + * refused by a constraint imports 0 and must not answer like a quiet night. * * No body and no `fullSync` flag: there is no full-history arm to call. * @@ -34,8 +37,8 @@ export const POST = apiHandler(async () => { } try { - const imported = await syncUserNightscout(user.id); - return apiSuccess({ imported }); + const result = await syncUserNightscout(user.id); + return apiSuccess(resolveSyncOutcome(result)); } catch { return apiError("Nightscout sync failed", 502); } diff --git a/src/app/api/oura/sync/__tests__/route.test.ts b/src/app/api/oura/sync/__tests__/route.test.ts index 8dd1fd51a..eea02fca6 100644 --- a/src/app/api/oura/sync/__tests__/route.test.ts +++ b/src/app/api/oura/sync/__tests__/route.test.ts @@ -53,19 +53,30 @@ beforeEach(() => { describe("POST /api/oura/sync", () => { it("syncs the session user and answers with the imported count", async () => { - vi.mocked(syncUserOura).mockResolvedValue(12); + vi.mocked(syncUserOura).mockResolvedValue({ imported: 12, failed: false }); const response = await post(request()); expect(response.status).toBe(200); expect(await envelope(response)).toEqual({ - data: { imported: 12 }, + data: { imported: 12, failed: false, outcome: "success" }, error: null, }); // The id comes from the session, never from the request. expect(syncUserOura).toHaveBeenCalledWith("u1"); }); + it("does not call a run whose every collection failed a success", async () => { + vi.mocked(syncUserOura).mockResolvedValue({ imported: 0, failed: true }); + + const response = await post(request()); + + expect(response.status).toBe(200); + expect((await envelope(response)).data).toMatchObject({ + outcome: "failed", + }); + }); + it("refuses an unauthenticated caller before syncing anything", async () => { vi.mocked(requireAuth).mockRejectedValue( new HttpError(401, "Not authenticated"), diff --git a/src/app/api/oura/sync/route.ts b/src/app/api/oura/sync/route.ts index 98602ed80..3f47ab631 100644 --- a/src/app/api/oura/sync/route.ts +++ b/src/app/api/oura/sync/route.ts @@ -2,6 +2,7 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiSuccess, apiError } from "@/lib/api-response"; import { checkRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; +import { resolveSyncOutcome } from "@/lib/outcome/written-outcome"; import { syncUserOura } from "@/lib/oura/sync"; /** @@ -31,8 +32,8 @@ export const POST = apiHandler(async () => { } try { - const imported = await syncUserOura(user.id); - return apiSuccess({ imported }); + const result = await syncUserOura(user.id); + return apiSuccess(resolveSyncOutcome(result)); } catch { // The failure is already on the `oura` ledger, classified by the provider // that owns the error shape. Answer generically — an upstream message can diff --git a/src/app/api/polar/sync/__tests__/route.test.ts b/src/app/api/polar/sync/__tests__/route.test.ts index f18d71c4f..a3f870aed 100644 --- a/src/app/api/polar/sync/__tests__/route.test.ts +++ b/src/app/api/polar/sync/__tests__/route.test.ts @@ -67,26 +67,40 @@ beforeEach(() => { vi.clearAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true } as never); vi.mocked(requireAuth).mockResolvedValue({ user: { id: "u1" } } as never); - syncUserPolarMock.mockResolvedValue(0); + syncUserPolarMock.mockResolvedValue({ imported: 0, failed: false }); syncUserPolarWorkoutsMock.mockResolvedValue(0); }); describe("POST /api/polar/sync", () => { it("runs both Polar legs and totals what they imported", async () => { - syncUserPolarMock.mockResolvedValue(5); + syncUserPolarMock.mockResolvedValue({ imported: 5, failed: false }); syncUserPolarWorkoutsMock.mockResolvedValue(2); const response = await post(request()); expect(response.status).toBe(200); expect(await envelope(response)).toEqual({ - data: { imported: 7 }, + data: { imported: 7, failed: false, outcome: "success" }, error: null, }); expect(syncUserPolarMock).toHaveBeenCalledWith("u1"); expect(syncUserPolarWorkoutsMock).toHaveBeenCalledWith("u1"); }); + it("does not call a run whose vitals collections all failed a success", async () => { + syncUserPolarMock.mockResolvedValue({ imported: 0, failed: true }); + syncUserPolarWorkoutsMock.mockResolvedValue(0); + + const response = await post(request()); + + expect(response.status).toBe(200); + expect((await envelope(response)).data).toEqual({ + imported: 0, + failed: true, + outcome: "failed", + }); + }); + it("goes through the shared legs entry point, not the provider internals", () => { const source = readFileSync(join(__dirname, "..", "route.ts"), "utf8"); expect(source).toContain("syncUserPolarLegs"); diff --git a/src/app/api/polar/sync/route.ts b/src/app/api/polar/sync/route.ts index 01b528dcd..f743459af 100644 --- a/src/app/api/polar/sync/route.ts +++ b/src/app/api/polar/sync/route.ts @@ -3,6 +3,7 @@ import { apiSuccess, apiError } from "@/lib/api-response"; import { checkRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; import { syncUserPolarLegs } from "@/lib/jobs/reminder/polar-sync"; +import { resolveSyncOutcome } from "@/lib/outcome/written-outcome"; /** * Manually trigger a Polar sync for the current user (v1.32.28). @@ -31,8 +32,8 @@ export const POST = apiHandler(async () => { } try { - const imported = await syncUserPolarLegs(user.id); - return apiSuccess({ imported }); + const result = await syncUserPolarLegs(user.id); + return apiSuccess(resolveSyncOutcome(result)); } catch { // Both legs record their own classified failure on the `polar` ledger. // Answer generically so no upstream detail rides the response body. diff --git a/src/app/api/strava/sync/__tests__/route.test.ts b/src/app/api/strava/sync/__tests__/route.test.ts index 45fcd1bcf..c72617871 100644 --- a/src/app/api/strava/sync/__tests__/route.test.ts +++ b/src/app/api/strava/sync/__tests__/route.test.ts @@ -60,7 +60,7 @@ describe("POST /api/strava/sync", () => { expect(response.status).toBe(200); expect(await envelope(response)).toEqual({ - data: { imported: 4 }, + data: { imported: 4, failed: false, outcome: "success" }, error: null, }); expect(syncUserStrava).toHaveBeenCalledWith("u1"); diff --git a/src/app/api/strava/sync/route.ts b/src/app/api/strava/sync/route.ts index 0c36326cd..8810ccb64 100644 --- a/src/app/api/strava/sync/route.ts +++ b/src/app/api/strava/sync/route.ts @@ -2,6 +2,7 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiSuccess, apiError } from "@/lib/api-response"; import { checkRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; +import { resolveSyncOutcome } from "@/lib/outcome/written-outcome"; import { syncUserStrava } from "@/lib/strava/sync"; /** @@ -28,7 +29,11 @@ export const POST = apiHandler(async () => { try { const imported = await syncUserStrava(user.id); - return apiSuccess({ imported }); + // `failed: false` is a statement, not a placeholder: the Strava walk has no + // settled-but-partial arm. `upsertStravaWorkouts` rethrows the first row + // error after attempting the batch and the page walk rethrows its own, so + // reaching this line means every fetched activity was written. + return apiSuccess(resolveSyncOutcome({ imported, failed: false })); } catch { // Already recorded on the `strava` ledger with its classification. Answer // generically so no upstream detail rides the response body. diff --git a/src/app/api/whoop/sync/route.ts b/src/app/api/whoop/sync/route.ts index d3a331aca..1a7579cfe 100644 --- a/src/app/api/whoop/sync/route.ts +++ b/src/app/api/whoop/sync/route.ts @@ -1,6 +1,7 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiSuccess, apiError } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; +import { resolveSyncOutcome } from "@/lib/outcome/written-outcome"; import { syncUserWhoop } from "@/lib/whoop/sync"; import { NextRequest } from "next/server"; @@ -27,6 +28,8 @@ export const POST = apiHandler(async (request: NextRequest) => { // no body provided -> default incremental sync } - const { imported } = await syncUserWhoop(user.id, { fullSync }); - return apiSuccess({ imported, fullSync }); + // Resolved server-side so the card and the native client cannot disagree on + // how the same run reads. + const result = await syncUserWhoop(user.id, { fullSync }); + return apiSuccess({ ...resolveSyncOutcome(result), fullSync }); }); diff --git a/src/app/api/withings/sync/route.ts b/src/app/api/withings/sync/route.ts index f881d62d8..508f8cc4a 100644 --- a/src/app/api/withings/sync/route.ts +++ b/src/app/api/withings/sync/route.ts @@ -1,6 +1,7 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiSuccess, apiError } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; +import { resolveSyncOutcome } from "@/lib/outcome/written-outcome"; import { syncUserMeasurements } from "@/lib/withings/sync"; import { NextRequest } from "next/server"; @@ -24,8 +25,6 @@ export const POST = apiHandler(async (request: NextRequest) => { // no body provided -> default incremental sync } - const imported = await syncUserMeasurements(user.id, { - fullSync, - }); - return apiSuccess({ imported, fullSync }); + const result = await syncUserMeasurements(user.id, { fullSync }); + return apiSuccess({ ...resolveSyncOutcome(result), fullSync }); }); diff --git a/src/lib/jobs/reminder/__tests__/polar-sync-legs.test.ts b/src/lib/jobs/reminder/__tests__/polar-sync-legs.test.ts index 557cf14e5..9ea8874ce 100644 --- a/src/lib/jobs/reminder/__tests__/polar-sync-legs.test.ts +++ b/src/lib/jobs/reminder/__tests__/polar-sync-legs.test.ts @@ -17,15 +17,18 @@ import { syncUserPolarLegs } from "../polar-sync"; beforeEach(() => { vi.clearAllMocks(); - syncUserPolar.mockResolvedValue(0); + syncUserPolar.mockResolvedValue({ imported: 0, failed: false }); syncUserPolarWorkouts.mockResolvedValue(0); }); describe("syncUserPolarLegs — composite independence on the shared polar ledger", () => { it("runs both legs and returns the summed count when both pass", async () => { - syncUserPolar.mockResolvedValue(5); + syncUserPolar.mockResolvedValue({ imported: 5, failed: false }); syncUserPolarWorkouts.mockResolvedValue(2); - await expect(syncUserPolarLegs("u1")).resolves.toBe(7); + await expect(syncUserPolarLegs("u1")).resolves.toEqual({ + imported: 7, + failed: false, + }); expect(syncUserPolar).toHaveBeenCalledWith("u1"); expect(syncUserPolarWorkouts).toHaveBeenCalledWith("u1"); }); @@ -35,9 +38,15 @@ describe("syncUserPolarLegs — composite independence on the shared polar ledge // its healthy-collections import count rather than throwing. The wrapper // must sum it with the workout leg and complete cleanly — a partial vitals // failure never starves the user's success accounting at the cohort level. - syncUserPolar.mockResolvedValue(4); + syncUserPolar.mockResolvedValue({ imported: 4, failed: true }); syncUserPolarWorkouts.mockResolvedValue(2); - await expect(syncUserPolarLegs("u1")).resolves.toBe(6); + // The partial is carried, not swallowed: the wrapper completes with the + // summed count AND says a leg did not settle, so the card cannot paint a + // run in which four of five collections failed as a clean one. + await expect(syncUserPolarLegs("u1")).resolves.toEqual({ + imported: 6, + failed: true, + }); expect(syncUserPolar).toHaveBeenCalledWith("u1"); expect(syncUserPolarWorkouts).toHaveBeenCalledWith("u1"); }); @@ -52,7 +61,7 @@ describe("syncUserPolarLegs — composite independence on the shared polar ledge }); it("leaves the vitals leg's result intact when the workout leg fails", async () => { - syncUserPolar.mockResolvedValue(4); + syncUserPolar.mockResolvedValue({ imported: 4, failed: false }); syncUserPolarWorkouts.mockRejectedValue(new Error("workouts down")); // The vitals leg ran to completion (its own success already recorded on // the shared ledger); the workout error is surfaced without undoing it. diff --git a/src/lib/jobs/reminder/nightscout-sync.ts b/src/lib/jobs/reminder/nightscout-sync.ts index 76cbc7b57..01bb0b25c 100644 --- a/src/lib/jobs/reminder/nightscout-sync.ts +++ b/src/lib/jobs/reminder/nightscout-sync.ts @@ -27,7 +27,9 @@ export const handleNightscoutSync = makePollCohortHandler({ }); return users.map((u) => u.id); }, - syncUser: syncUserNightscout, + // The cohort accounts in rows imported; the failure half of the result is + // recorded on the `nightscout` ledger by the sync itself. + syncUser: async (userId) => (await syncUserNightscout(userId)).imported, // The fetch path already records + marks its own failure (token-redacted), so // this fires only for a future UNMARKED escape — and redacts the token before // the message reaches the ledger. diff --git a/src/lib/jobs/reminder/oura-sync.ts b/src/lib/jobs/reminder/oura-sync.ts index a22d2f9ad..0b7c0288d 100644 --- a/src/lib/jobs/reminder/oura-sync.ts +++ b/src/lib/jobs/reminder/oura-sync.ts @@ -21,7 +21,9 @@ export const handleOuraSync = makePollCohortHandler({ }); return users.map((u) => u.id); }, - syncUser: syncUserOura, + // The cohort accounts in rows imported; the failure half of the result is + // recorded on the `oura` ledger by the sync itself. + syncUser: async (userId) => (await syncUserOura(userId)).imported, // Catches an UNMARKED escape — today the `upsertOuraMeasurements` write-path // throw (the fetch/refresh path already records + marks its own failures). recordFailure: recordOuraSyncFailure, diff --git a/src/lib/jobs/reminder/polar-sync.ts b/src/lib/jobs/reminder/polar-sync.ts index ee7c80253..97dd8dd50 100644 --- a/src/lib/jobs/reminder/polar-sync.ts +++ b/src/lib/jobs/reminder/polar-sync.ts @@ -15,6 +15,7 @@ */ import { recordPolarSyncFailure, syncUserPolar } from "@/lib/polar/sync"; import { syncUserPolarWorkouts } from "@/lib/polar/sync-workouts"; +import type { SyncWriteResult } from "@/lib/outcome/written-outcome"; import { makePollCohortHandler, type PollCohortPayload } from "./poll-cohort"; export type PolarSyncPayload = PollCohortPayload; @@ -34,13 +35,21 @@ export type PolarSyncPayload = PollCohortPayload; * healthy collections. So the vitals leg reaching this wrapper's catch means a * write-path/unexpected error, not a collection fetch failure. */ -export async function syncUserPolarLegs(userId: string): Promise { +export async function syncUserPolarLegs( + userId: string, +): Promise { let total = 0; let firstError: unknown; let hadError = false; + // A settled-but-partial vitals leg neither throws nor reduces the count, so + // it has to be carried explicitly — otherwise a run in which four of five + // collections failed reads exactly like a clean one. + let partial = false; try { - total += await syncUserPolar(userId); + const vitals = await syncUserPolar(userId); + total += vitals.imported; + partial = vitals.failed; } catch (err) { firstError = err; hadError = true; @@ -56,7 +65,7 @@ export async function syncUserPolarLegs(userId: string): Promise { } if (hadError) throw firstError; - return total; + return { imported: total, failed: partial }; } export const handlePolarSync = makePollCohortHandler({ @@ -68,7 +77,9 @@ export const handlePolarSync = makePollCohortHandler({ }); return users.map((u) => u.id); }, - syncUser: syncUserPolarLegs, + // The cohort accounts in rows imported; a partial vitals leg is already on + // the `polar` ledger, recorded by the leg that saw it. + syncUser: async (userId) => (await syncUserPolarLegs(userId)).imported, // Catches an UNMARKED escape — the `upsertPolarMeasurements` / // `upsertPolarWorkouts` write-path throw. The vitals leg records its own // per-collection partial failures inline and returns without rethrowing; the diff --git a/src/lib/jobs/reminder/withings-sync.ts b/src/lib/jobs/reminder/withings-sync.ts index 96be0b354..d5ac1ab35 100644 --- a/src/lib/jobs/reminder/withings-sync.ts +++ b/src/lib/jobs/reminder/withings-sync.ts @@ -76,7 +76,7 @@ export async function handleWithingsFallbackSync( for (const connection of connections) { try { - const imported = await syncUserMeasurements(connection.userId); + const { imported } = await syncUserMeasurements(connection.userId); usersSynced++; measurementsImported += imported; // v1.18.1 — a fresh reading landed; resolve this user's Vorsorge diff --git a/src/lib/nightscout/__tests__/sync.test.ts b/src/lib/nightscout/__tests__/sync.test.ts index 770eb67d4..2d903a3ec 100644 --- a/src/lib/nightscout/__tests__/sync.test.ts +++ b/src/lib/nightscout/__tests__/sync.test.ts @@ -90,18 +90,18 @@ afterEach(() => { }); describe("syncUserNightscout", () => { - it("returns 0 and records nothing when the user is not configured", async () => { + it("returns an empty result and records nothing when the user is not configured", async () => { getCredsMock.mockResolvedValue(null); - const n = await syncUserNightscout("u1"); - expect(n).toBe(0); + const result = await syncUserNightscout("u1"); + expect(result).toEqual({ imported: 0, failed: false }); expect(fetchSgvEntriesMock).not.toHaveBeenCalled(); expect(recordSuccessMock).not.toHaveBeenCalled(); }); it("writes each SGV entry as a BLOOD_GLUCOSE mg/dL measurement", async () => { fetchSgvEntriesMock.mockResolvedValue([ENTRY_A, ENTRY_B]); - const n = await syncUserNightscout("u1"); - expect(n).toBe(2); + const result = await syncUserNightscout("u1"); + expect(result).toEqual({ imported: 2, failed: false }); expect(upsertMock).toHaveBeenCalledTimes(2); const first = upsertMock.mock.calls[0]![0]; expect(first.create.type).toBe("BLOOD_GLUCOSE"); @@ -146,11 +146,11 @@ describe("syncUserNightscout", () => { // ENTRY_A already exists (a re-confirm); ENTRY_B is genuinely new. findManyMock.mockResolvedValue([{ externalId: "ns:abc" }]); - const n = await syncUserNightscout("u1"); + const result = await syncUserNightscout("u1"); // The inflated insert+reconfirm count used to fire the reminder-satisfy // trigger every tick; the honest count is the single fresh insert. - expect(n).toBe(1); + expect(result).toEqual({ imported: 1, failed: false }); // Both rows are still written (idempotent), but only the fresh one emits. expect(upsertMock).toHaveBeenCalledTimes(2); expect(emitArrivalsMock).toHaveBeenCalledTimes(1); @@ -172,10 +172,12 @@ describe("syncUserNightscout", () => { .mockResolvedValueOnce({ id: "r1" }) .mockRejectedValueOnce(new Error("bad row")); - const n = await syncUserNightscout("u1"); + const result = await syncUserNightscout("u1"); - // The one good row still landed; the tick is honest about the failed row. - expect(n).toBe(1); + // The one good row still landed; the tick is honest about the failed row — + // the count alone used to narrow that away, so the card called a window in + // which nothing wrote a success. + expect(result).toEqual({ imported: 1, failed: true }); expect(recordSuccessMock).not.toHaveBeenCalled(); expect(recordFailureMock).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/src/lib/nightscout/sync.ts b/src/lib/nightscout/sync.ts index 44588d512..724ccf46b 100644 --- a/src/lib/nightscout/sync.ts +++ b/src/lib/nightscout/sync.ts @@ -30,6 +30,7 @@ import { type FailureKind, } from "@/lib/integrations/status"; import type { IntegrationClassification } from "@/lib/integrations/http-status-classifier"; +import type { SyncWriteResult } from "@/lib/outcome/written-outcome"; import { collapseToTypeDayKeys, recomputeBucketsForMeasurement, @@ -122,19 +123,28 @@ export function classifyNightscoutFailure(err: unknown): FailureKind { } /** - * Sync one user's recent SGV entries. Returns the count of rows FRESHLY - * INSERTED this tick (re-confirmed rows the upsert merely touched are not - * counted — the old insert+reconfirm total inflated the cohort's - * `measurements_imported` and fired the reminder-satisfy trigger every tick - * regardless of whether new glucose actually arrived). A user with no - * configured instance is a clean no-op (returns 0, touches no status row). + * Sync one user's recent SGV entries. + * + * `imported` counts the rows FRESHLY INSERTED this tick (re-confirmed rows the + * upsert merely touched are not counted — the old insert+reconfirm total + * inflated the cohort's `measurements_imported` and fired the reminder-satisfy + * trigger every tick regardless of whether new glucose actually arrived). + * `failed` is true when any SGV row did not write. + * + * The count used to be the whole return value, which narrowed the failures + * away: a tick where every row was refused answered the same `0` as a tick + * that found nothing new, and the settings card rendered both as a green tick + * with a count. The caller now gets both halves and decides. + * + * A user with no configured instance is a clean no-op (nothing imported, + * nothing failed, no status row touched). */ export async function syncUserNightscout( userId: string, opts: { count?: number } = {}, -): Promise { +): Promise { const creds = await getUserNightscoutCredentials(userId); - if (!creds) return 0; + if (!creds) return { imported: 0, failed: false }; let entries: ParsedSgvEntry[]; try { @@ -168,11 +178,11 @@ export async function syncUserNightscout( kind: "transient", message: `${failedRows} of ${entries.length} SGV rows failed to write`, }); - return inserted; + return { imported: inserted, failed: true }; } await recordSyncSuccess(userId, "nightscout"); - return inserted; + return { imported: inserted, failed: false }; } /** diff --git a/src/lib/openapi/routes/ocr.ts b/src/lib/openapi/routes/ocr.ts index 6850f0f0a..2493c1f56 100644 --- a/src/lib/openapi/routes/ocr.ts +++ b/src/lib/openapi/routes/ocr.ts @@ -105,11 +105,15 @@ const commitResponse = z reason: z.literal("duplicate"), }), ), + outcome: z.enum(["empty", "failed", "partial", "success"]).meta({ + description: + "How the commit reads, resolved from the counts rather than the status code. `success` only when every confirmed row was written; `partial` when some were skipped; `failed` when nothing was written and at least one row was skipped; `empty` when there was nothing to write.", + }), }) .meta({ id: "OcrCommitResponse", description: - 'The rows written (`source: "OCR"`) and those skipped as commit-time duplicates.', + 'The rows written (`source: "OCR"`), those skipped as commit-time duplicates, and the resolved outcome.', }); export const ocrPaths: NonNullable = { diff --git a/src/lib/oura/__tests__/sync.test.ts b/src/lib/oura/__tests__/sync.test.ts index 2e13277b9..7851a749a 100644 --- a/src/lib/oura/__tests__/sync.test.ts +++ b/src/lib/oura/__tests__/sync.test.ts @@ -220,7 +220,7 @@ function createdRows() { describe("syncUserOura", () => { it("no-ops cleanly when unconnected", async () => { getConnMock.mockResolvedValue(null); - expect(await syncUserOura("u1")).toBe(0); + expect(await syncUserOura("u1")).toEqual({ imported: 0, failed: false }); expect(recordSuccessMock).not.toHaveBeenCalled(); }); @@ -229,7 +229,7 @@ describe("syncUserOura", () => { fetchReadinessMock.mockResolvedValue([ { id: "x", day: "2026-06-10", score: 84 }, ]); - const imported = await syncUserOura("u1"); + const { imported } = await syncUserOura("u1"); expect(imported).toBe(1); const row = createdRows()[0]; expect(row).toMatchObject({ @@ -253,7 +253,7 @@ describe("syncUserOura", () => { refresh_token: "newRef", expires_in: 86400, }); - const imported = await syncUserOura("u1"); + const { imported } = await syncUserOura("u1"); expect(refreshMock).toHaveBeenCalledWith("ref", expect.anything()); expect(storeTokensMock).toHaveBeenCalledWith( "u1", @@ -278,7 +278,7 @@ describe("syncUserOura", () => { // A concurrent sync rotated first → the persist returns the PEER's token. storeTokensMock.mockResolvedValue("peerAcc"); - const imported = await syncUserOura("u1"); + const { imported } = await syncUserOura("u1"); // The retry runs with the peer's token rather than the invalidated one. expect(fetchReadinessMock).toHaveBeenLastCalledWith( @@ -461,7 +461,7 @@ describe("syncUserOura", () => { fetchResilienceMock.mockResolvedValue([ { id: "r", day: "2026-06-10", level: "strong" }, ]); - const imported = await syncUserOura("u1"); + const { imported } = await syncUserOura("u1"); expect(imported).toBe(1); const row = createdRows()[0]; expect(row).toMatchObject({ @@ -478,7 +478,7 @@ describe("syncUserOura", () => { fetchResilienceMock.mockResolvedValue([ { id: "r", day: "2026-06-10", level: "godlike" }, ]); - const imported = await syncUserOura("u1"); + const { imported } = await syncUserOura("u1"); expect(imported).toBe(0); expect(reconcileMock).not.toHaveBeenCalled(); }); @@ -561,10 +561,13 @@ describe("syncUserOura", () => { { id: "o", day: "2026-06-10", spo2_percentage: { average: 97 } }, ]); - const imported = await syncUserOura("u1"); + const result = await syncUserOura("u1"); - // The healthy collections still wrote their rows — the source is not blanked. - expect(imported).toBe(2); + // The healthy collections still wrote their rows — the source is not + // blanked — and the partial rides back with the count, so the card can + // report a warning rather than a tick over a run that lost a collection. + expect(result).toEqual({ imported: 2, failed: true }); + const imported = result.imported; const written = createdRows().map((row) => row.type); expect(written).toContain("SLEEP_SCORE"); expect(written).toContain("OXYGEN_SATURATION"); @@ -596,7 +599,7 @@ describe("syncUserOura", () => { { id: "o", day: "2026-06-10", spo2_percentage: { average: 96 } }, ]); - const imported = await syncUserOura("u1"); + const { imported } = await syncUserOura("u1"); expect(imported).toBe(1); const written = createdRows().map((row) => row.type); @@ -620,7 +623,7 @@ describe("syncUserOura", () => { reason: "http_403", }), ); - const imported = await syncUserOura("u1"); + const { imported } = await syncUserOura("u1"); expect(imported).toBe(1); expect(recordSuccessMock).toHaveBeenCalledWith("u1", "oura"); expect(recordFailureMock).not.toHaveBeenCalled(); @@ -647,7 +650,7 @@ describe("syncUserOura", () => { { id: "s", day: "2026-06-10", score: 80 }, ]); fetchCyclePhasesMock.mockRejectedValue(new Error("boom")); - const imported = await syncUserOura("u1"); + const { imported } = await syncUserOura("u1"); expect(imported).toBe(1); expect(recordSuccessMock).toHaveBeenCalledWith("u1", "oura"); }); diff --git a/src/lib/oura/sync.ts b/src/lib/oura/sync.ts index fd6c1e2ff..4dfefaa83 100644 --- a/src/lib/oura/sync.ts +++ b/src/lib/oura/sync.ts @@ -52,6 +52,7 @@ import { type InsertedMeasurementArrivalRow, } from "@/lib/arrivals/measurement-emit"; import { invalidateStatusInsightsForTypes } from "@/lib/insights/comprehensive-generate"; +import type { SyncWriteResult } from "@/lib/outcome/written-outcome"; import { maybeEnqueueMorningRefresh } from "@/lib/daily/morning-refresh-trigger"; import { sweepStaleSleepSegments, @@ -411,16 +412,21 @@ async function fetchAll( } /** - * Sync one user's Oura data. Returns the count of measurement rows written. - * A user with no Oura connection is a clean no-op (returns 0, touches no - * status row). + * Sync one user's Oura data. + * + * `imported` is the count of measurement rows written; `failed` is true when a + * collection did not settle. The count used to be the whole return value, + * which hid the partial: a run where every collection failed answered `0`, the + * same as a run that found nothing new, and the settings card reported both as + * a success. A user with no Oura connection is a clean no-op (nothing + * imported, nothing failed, no status row touched). */ export async function syncUserOura( userId: string, opts: { lookbackDays?: number } = {}, -): Promise { +): Promise { const conn = await getOuraConnection(userId); - if (!conn) return 0; + if (!conn) return { imported: 0, failed: false }; // An explicit window from the caller wins; otherwise derive it from what we // already hold so a gap widens the window instead of being skipped over. @@ -516,11 +522,11 @@ export async function syncUserOura( ? String(firstErr.httpStatus) : undefined, }); - return imported; + return { imported, failed: true }; } await recordSyncSuccess(userId, "oura"); - return imported; + return { imported, failed: false }; } export async function upsertOuraMeasurements( diff --git a/src/lib/polar/__tests__/sync.test.ts b/src/lib/polar/__tests__/sync.test.ts index f5d12ff9c..3a1351d1d 100644 --- a/src/lib/polar/__tests__/sync.test.ts +++ b/src/lib/polar/__tests__/sync.test.ts @@ -164,7 +164,7 @@ function createdRows() { describe("syncUserPolar", () => { it("no-ops cleanly for an unconnected user", async () => { getConnMock.mockResolvedValue(null); - expect(await syncUserPolar("u1")).toBe(0); + expect(await syncUserPolar("u1")).toEqual({ imported: 0, failed: false }); expect(createManyAndReturnMock).not.toHaveBeenCalled(); expect(recordSuccessMock).not.toHaveBeenCalled(); }); @@ -186,7 +186,7 @@ describe("syncUserPolar", () => { fetchRechargesMock.mockResolvedValue([ { date: "2026-06-10", nightly_recharge_status: 6 }, ]); - const imported = await syncUserPolar("u1"); + const { imported } = await syncUserPolar("u1"); expect(imported).toBe(1); const row = createdRows()[0]; expect(row).toMatchObject({ @@ -206,7 +206,7 @@ describe("syncUserPolar", () => { fetchCardioLoadsMock.mockResolvedValue([ { date: "2026-06-10", cardio_load: 123.45 }, ]); - const imported = await syncUserPolar("u1"); + const { imported } = await syncUserPolar("u1"); expect(imported).toBe(1); const row = createdRows()[0]; expect(row).toMatchObject({ @@ -348,7 +348,7 @@ describe("syncUserPolar", () => { { date: "2026-06-10", cardio_load: 42 }, ]); - const imported = await syncUserPolar("u1"); + const { imported } = await syncUserPolar("u1"); // No rethrow now — a fetch failure is isolated per-collection. expect(imported).toBe(1); @@ -397,8 +397,11 @@ describe("syncUserPolar", () => { { date: "2026-06-10", cardio_load: 55 }, ]); - const imported = await syncUserPolar("u1"); + const result = await syncUserPolar("u1"); + const imported = result.imported; + // The partial rides back with the count rather than being narrowed away. + expect(result.failed).toBe(true); // The healthy four still wrote their rows — the source is not blanked. const written = createdRows().map((row) => row.type); expect(written).toContain("RECOVERY_SCORE"); @@ -438,7 +441,7 @@ describe("syncUserPolar", () => { { date: "2026-06-10", cardio_load: 77 }, ]); - const imported = await syncUserPolar("u1"); + const { imported } = await syncUserPolar("u1"); expect(imported).toBe(1); expect(createdRows().map((row) => row.type)).toContain("CARDIO_LOAD"); @@ -463,7 +466,7 @@ describe("syncUserPolar", () => { { start_time: "2026-06-10T00:00:00", steps: 8000 }, ]); - const imported = await syncUserPolar("u1"); + const { imported } = await syncUserPolar("u1"); // The sweep never ran — a failed sleep collection pushes no sweep entries. expect(updateManyMock).not.toHaveBeenCalled(); diff --git a/src/lib/polar/sync.ts b/src/lib/polar/sync.ts index e6f62a920..17b6505c5 100644 --- a/src/lib/polar/sync.ts +++ b/src/lib/polar/sync.ts @@ -51,6 +51,7 @@ import { recomputeBucketsForMeasurement, } from "@/lib/rollups/measurement-rollups"; import { invalidateStatusInsightsForTypes } from "@/lib/insights/comprehensive-generate"; +import type { SyncWriteResult } from "@/lib/outcome/written-outcome"; import { maybeEnqueueMorningRefresh } from "@/lib/daily/morning-refresh-trigger"; import { emitInsertedMeasurementArrivals, @@ -161,9 +162,11 @@ export interface PolarCollectionFailure { } /** - * Sync one user's Polar data. Returns the count of measurement rows written. - * A user with no Polar connection is a clean no-op (returns 0, touches no - * status row). + * Sync one user's Polar data. Returns the rows written AND whether a + * collection did not settle — the bare count used to narrow the partial away, + * so a run whose every collection failed answered the same `0` as a quiet + * night. A user with no Polar connection is a clean no-op (nothing imported, + * nothing failed, no status row touched). * * The five vitals collections are settled independently: the healthy ones * import, a failing one is recorded as a single partial failure (no success @@ -171,9 +174,9 @@ export interface PolarCollectionFailure { * escapes now is a write-path/unexpected error out of `upsertPolarMeasurements` * — the poll-cohort boundary records that unmarked escape once. */ -export async function syncUserPolar(userId: string): Promise { +export async function syncUserPolar(userId: string): Promise { const conn = await getPolarConnection(userId); - if (!conn) return 0; + if (!conn) return { imported: 0, failed: false }; // Filled by the sleep collection's closure as it maps records; carried so the // caller can run the record-scoped sweep before the upsert. A collection that @@ -301,11 +304,11 @@ export async function syncUserPolar(userId: string): Promise { ? String(firstErr.httpStatus) : undefined, }); - return imported; + return { imported, failed: true }; } await recordSyncSuccess(userId, "polar", { leg: POLAR_LEG_VITALS }); - return imported; + return { imported, failed: false }; } /** diff --git a/src/lib/withings/__tests__/sync-measure-watermark.test.ts b/src/lib/withings/__tests__/sync-measure-watermark.test.ts index c597f80b4..e8f37cd3e 100644 --- a/src/lib/withings/__tests__/sync-measure-watermark.test.ts +++ b/src/lib/withings/__tests__/sync-measure-watermark.test.ts @@ -171,10 +171,10 @@ describe("syncUserMeasurements — watermark hold on hard row failure (F4)", () .mockRejectedValueOnce(new Error("connection reset by peer")) .mockResolvedValue({} as never); - const imported = await syncUserMeasurements("user-1"); + const result = await syncUserMeasurements("user-1"); - // Only the second row persisted. - expect(imported).toBe(1); + // Only the second row persisted, and the caller is told the run lost one. + expect(result).toEqual({ imported: 1, failed: true }); // The watermark is HELD — no stamp — so the next tick refetches the window. expect(prisma.withingsConnection.update).not.toHaveBeenCalled(); expect(recordSyncSuccess).not.toHaveBeenCalled(); @@ -195,7 +195,7 @@ describe("syncUserMeasurements — watermark hold on hard row failure (F4)", () ] as never); vi.mocked(prisma.measurement.create).mockRejectedValueOnce(P2002); - const imported = await syncUserMeasurements("user-1"); + const { imported } = await syncUserMeasurements("user-1"); // P2002 means the row is already present — nothing lost — so the cycle is // still healthy: stamp the watermark + success, record no failure. @@ -216,7 +216,7 @@ describe("syncUserMeasurements — watermark hold on hard row failure (F4)", () }, ] as never); - const imported = await syncUserMeasurements("user-1"); + const { imported } = await syncUserMeasurements("user-1"); expect(imported).toBe(1); expect(prisma.withingsConnection.update).toHaveBeenCalledTimes(1); diff --git a/src/lib/withings/sync.ts b/src/lib/withings/sync.ts index 3da585814..fcee1cc7c 100644 --- a/src/lib/withings/sync.ts +++ b/src/lib/withings/sync.ts @@ -32,6 +32,7 @@ import { } from "@/lib/rollups/measurement-rollups"; import { invalidateStatusInsightsForTypes } from "@/lib/insights/comprehensive-generate"; import { invalidateUserDashboardSnapshot } from "@/lib/cache/invalidate"; +import type { SyncWriteResult } from "@/lib/outcome/written-outcome"; import { acquireProviderTokenRefreshLock, PROVIDER_REFRESH_TRANSACTION_OPTIONS, @@ -180,6 +181,9 @@ export const WITHINGS_INCREMENTAL_OVERLAP_MS = 10 * 60 * 1000; * Sync measurements from Withings for a given user. * Fetches data since last sync (or last 30 days if first sync). * + * Returns what was written AND whether any part of the run did not land, so + * the caller can tell a quiet week apart from a run that refused every row. + * * Status-tracking contract: * - On any failure (refresh failure, fetch failure, downstream * measurement-upsert that fails ALL items) we call @@ -193,26 +197,29 @@ export const WITHINGS_INCREMENTAL_OVERLAP_MS = 10 * 60 * 1000; export async function syncUserMeasurements( userId: string, opts: { fullSync?: boolean } = {}, -): Promise { +): Promise { // Park: if the last burst said reauth_required, do nothing until - // the OAuth callback flips state back to "connected". Returning 0 - // matches the existing contract for "no-op sync". + // the OAuth callback flips state back to "connected". The run wrote + // nothing AND did not complete, so it reports as a failure — a bare 0 + // read as "nothing new", which is how a parked connection could keep + // answering with a green tick. if (await isReauthRequired(userId, "withings")) { getEvent()?.addWarning( `withings sync skipped for ${userId}: parked at error_reauth`, ); - return 0; + return { imported: 0, failed: true }; } const tokenInfo = await getValidToken(userId); // Note: getValidToken already calls recordSyncFailure on the refresh // path, so we don't double-record here. - if (!tokenInfo) return 0; + if (!tokenInfo) return { imported: 0, failed: true }; const connection = await prisma.withingsConnection.findUnique({ where: { userId }, }); - if (!connection) return 0; + // No connection row is not a failure — there is nothing to sync. + if (!connection) return { imported: 0, failed: false }; const startDate = opts.fullSync ? undefined @@ -390,7 +397,7 @@ export async function syncUserMeasurements( firstRowError ?? new Error("Withings measure row write failed"), WITHINGS_LEG_MEASURES, ); - return imported; + return { imported, failed: true }; } // Update last synced timestamp @@ -407,7 +414,7 @@ export async function syncUserMeasurements( leg: WITHINGS_LEG_MEASURES, }); - return imported; + return { imported, failed: false }; } /**