fix(onboarding): stop the setup flow asking for units the account already holds

The spec's sixth question is skipped when the account already carries the
preferences, and the flow had no way to know: `User.unitPreference` is
nullable with null meaning "default metric", and the account payload coerces
that away, so nothing downstream could tell a chosen metric from an unset
column. The two columns are read raw on the server now, into the same
vocabulary the wire publishes, and a record whose account holds both reads its
`units` step as done — the question answers itself and is never put again.

Read-side only: the stored ledger keeps saying pending, because the person
really has not answered it, and an answer given later still lands.

The two writes the units answer makes — the display columns and the record of
the answer — go in one transaction, so a failure between them can no longer
leave the columns changed with no record of what changed them.

docs/api/openapi.yaml

@@ -63210,8 +63210,14 @@ components:
         - id
         - status
       additionalProperties: false
-      description: One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather
-        than refusing the list, so the flow may grow a screen without a client release.
+      description: >-
+        One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather than
+        refusing the list, so the flow may grow a screen without a client release.
+
+
+        `units` carries one resolution the others do not: a record whose account already holds BOTH unit preferences
+        reads as `done` whether or not the question was ever put, which is what stops the flow re-asking a value the
+        account holds.
     OnboardingNeeds:
       type: object
       properties:

src/app/api/auth/me/route.ts

@@ -54,7 +54,11 @@ import {
   resolveModuleMap,
   getOperatorModuleAvailability,
 } from "@/lib/modules/gate";
-import { loadOnboardingState } from "@/lib/onboarding/needs-store";
+import { readHeldUnitPreferences } from "@/lib/onboarding/needs";
+import {
+  loadOnboardingRecordRow,
+  toOnboardingStateDto,
+} from "@/lib/onboarding/needs-store";
 import { parseTourProgress } from "@/lib/onboarding/tour-progress";
 import { parseNotificationPrefs } from "@/lib/validations/notification-prefs";
 import { resolveAccountAccess } from "@/lib/sharing/account-access";
@@ -113,7 +117,7 @@ export const GET = apiHandler(async () => {
     cycleProfile,
     resolvedModules,
     moduleAvailability,
-    onboarding,
+    onboardingRow,
   ] = await Promise.all([
     // The actor's own row is already in hand; a switched session needs the
     // record's `gender`, which is the column the cycle gate derives from.
@@ -150,7 +154,7 @@ export const GET = apiHandler(async () => {
     // act on and never complete. A guardian's route into a managed record's
     // flow arrives with the rest of that record's configuration (#939 / C2);
     // the storage is already keyed by record for it.
-    loadOnboardingState(prisma, user.id),
+    loadOnboardingRecordRow(prisma, user.id),
   ]);
 
   // The masking step, and the one place the record scoping is narrowed rather
@@ -165,6 +169,14 @@ export const GET = apiHandler(async () => {
   // exactly as it was, and `moduleAccess` is the same answer with the reason
   // attached — the record's own switch, the grant's edge, or the operator's.
   // Every client that only wants "paint it or not" keeps reading the boolean.
+  // v1.39 (C1) — the published steps carry the Q6 resolution: an account that
+  // already holds both unit preferences reads `units` as `done`, so the flow
+  // never re-asks a value the account holds.
+  const onboarding = toOnboardingStateDto(
+    onboardingRow,
+    readHeldUnitPreferences(user),
+  );
+
   const { modules, moduleAccess } = buildModuleDisclosure(
     resolvedModules,
     moduleAvailability,

src/app/api/onboarding/answers/route.ts

@@ -33,6 +33,7 @@ import {
 import { prisma, toJson } from "@/lib/db";
 import { annotate } from "@/lib/logging/context";
 import { applyOnboardingAnswer } from "@/lib/onboarding/needs-apply";
+import { readHeldUnitPreferences } from "@/lib/onboarding/needs";
 import {
   ONBOARDING_RECORD_SELECT,
   readOnboardingRecordState,
@@ -98,18 +99,17 @@ export const PATCH = apiHandler(async (request: NextRequest) => {
   // second source of truth. What is kept beside them in `needs.units` is the
   // answer as given, so a restart can show it back without re-deriving it from
   // a column Settings may have changed since.
+  const unitData: { glucoseUnit?: string; unitPreference?: string } = {};
   if (answer.step === "units" && answer.status !== "skipped") {
-    const data: { glucoseUnit?: string; unitPreference?: string } = {};
-    if (answer.units.glucoseUnit) data.glucoseUnit = answer.units.glucoseUnit;
-    if (answer.units.unitPreference) {
-      data.unitPreference = answer.units.unitPreference;
+    if (answer.units.glucoseUnit) {
+      unitData.glucoseUnit = answer.units.glucoseUnit;
     }
-    if (Object.keys(data).length > 0) {
-      await prisma.user.update({ where: { id: user.id }, data });
+    if (answer.units.unitPreference) {
+      unitData.unitPreference = answer.units.unitPreference;
     }
   }
 
-  const written = await prisma.onboardingRecord.upsert({
+  const upsert = prisma.onboardingRecord.upsert({
     where: { userId: user.id },
     // Field-by-field, never a spread of the parsed body: the three columns are
     // the pure function's output, and nothing the client sent reaches Prisma
@@ -130,6 +130,22 @@ export const PATCH = apiHandler(async (request: NextRequest) => {
     select: ONBOARDING_RECORD_SELECT,
   });
 
+  // One transaction, because the two writes are one answer: a failure between
+  // them would leave the display columns changed with no record of the answer
+  // that changed them.
+  const [written] =
+    Object.keys(unitData).length > 0
+      ? await prisma.$transaction([
+          upsert,
+          prisma.user.update({ where: { id: user.id }, data: unitData }),
+        ])
+      : await prisma.$transaction([upsert]);
+
+  const held = readHeldUnitPreferences({
+    glucoseUnit: unitData.glucoseUnit ?? user.glucoseUnit,
+    unitPreference: unitData.unitPreference ?? user.unitPreference,
+  });
+
   annotate({
     action: { name: "onboarding.answer.save" },
     meta: {
@@ -138,5 +154,5 @@ export const PATCH = apiHandler(async (request: NextRequest) => {
     },
   });
 
-  return apiSuccess({ onboarding: toOnboardingStateDto(written) });
+  return apiSuccess({ onboarding: toOnboardingStateDto(written, held) });
 });

src/app/api/onboarding/complete/route.ts

@@ -10,6 +10,11 @@ import {
   deriveOnboardingModuleDefaults,
   mergeDerivedModulePreferences,
 } from "@/lib/modules/registry";
+import {
+  readHeldUnitPreferences,
+  resolveOnboardingSteps,
+  type HeldUnitPreferences,
+} from "@/lib/onboarding/needs";
 import {
   ONBOARDING_RECORD_SELECT,
   readOnboardingRecordState,
@@ -87,7 +92,10 @@ export const POST = apiHandler(async (request: NextRequest) => {
   // next navigation drops the /onboarding redirect immediately.
   await setOnboardingPendingCookie(false);
 
-  const onboarding = await completeNeedsFlow(user.id);
+  const onboarding = await completeNeedsFlow(
+    user.id,
+    readHeldUnitPreferences(user),
+  );
 
   return apiSuccess({ completed: true, ...(onboarding ? { onboarding } : {}) });
 });
@@ -100,7 +108,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
  * needs flow — a legacy wizard completion then answers exactly the body it
  * always did.
  */
-async function completeNeedsFlow(userId: string) {
+async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) {
   const record = await prisma.onboardingRecord.findUnique({
     where: { userId },
     select: ONBOARDING_RECORD_SELECT,
@@ -108,6 +116,10 @@ async function completeNeedsFlow(userId: string) {
   if (!record) return null;
 
   const state = readOnboardingRecordState(record);
+  // The units question answers itself for an account that already holds both
+  // preferences, so the gate and the published state read the same resolved
+  // ledger rather than the raw one.
+  const steps = resolveOnboardingSteps(state.steps, held);
   if (state.needs.recordTarget === null) {
     // The row exists but Q1 was never answered, so there is nothing to derive
     // from. Stamping a completion here would claim a setup that did not happen.
@@ -115,7 +127,7 @@ async function completeNeedsFlow(userId: string) {
       action: { name: "onboarding.needs.complete" },
       meta: { outcome: "no_answers" },
     });
-    return toOnboardingStateDto(record);
+    return toOnboardingStateDto(record, held);
   }
 
   let derived = false;
@@ -162,7 +174,7 @@ async function completeNeedsFlow(userId: string) {
   }
 
   const now = new Date();
-  const steps = state.steps.map((step) =>
+  const confirmed = steps.map((step) =>
     step.id === "confirm" && step.status === "pending"
       ? { ...step, status: "done" as const }
       : step,
@@ -170,7 +182,7 @@ async function completeNeedsFlow(userId: string) {
   const written = await prisma.onboardingRecord.update({
     where: { userId },
     data: {
-      stepsJson: toJson(steps),
+      stepsJson: toJson(confirmed),
       completedAt: now,
       ...(derived ? { modulesDerivedAt: now } : {}),
     },
@@ -182,5 +194,5 @@ async function completeNeedsFlow(userId: string) {
     meta: { outcome: derived ? "derived" : "already_derived" },
   });
 
-  return toOnboardingStateDto(written);
+  return toOnboardingStateDto(written, held);
 }

src/app/api/onboarding/restart/route.ts

@@ -37,7 +37,10 @@ import {
 import { auditLog } from "@/lib/auth/audit";
 import { prisma, toJson } from "@/lib/db";
 import { annotate } from "@/lib/logging/context";
-import { defaultOnboardingSteps } from "@/lib/onboarding/needs";
+import {
+  defaultOnboardingSteps,
+  readHeldUnitPreferences,
+} from "@/lib/onboarding/needs";
 import {
   ONBOARDING_RECORD_SELECT,
   toOnboardingStateDto,
@@ -105,5 +108,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
     meta: { outcome: "reset" },
   });
 
-  return apiSuccess({ onboarding: toOnboardingStateDto(written) });
+  return apiSuccess({
+    onboarding: toOnboardingStateDto(written, readHeldUnitPreferences(user)),
+  });
 });

src/lib/onboarding/__tests__/needs.test.ts

@@ -14,6 +14,7 @@ import { describe, expect, it } from "vitest";
 
 import { applyOnboardingAnswer } from "../needs-apply";
 import {
+  accountHoldsBothUnits,
   defaultOnboardingSteps,
   emptyOnboardingNeeds,
   isOnboardingSettled,
@@ -21,6 +22,8 @@ import {
   parseOnboardingFirstResult,
   parseOnboardingNeeds,
   parseOnboardingSteps,
+  readHeldUnitPreferences,
+  resolveOnboardingSteps,
   type OnboardingStateDto,
 } from "../needs";
 
@@ -260,3 +263,94 @@ describe("isOnboardingSettled", () => {
     ).toBe(false);
   });
 });
+
+describe("the unit preferences the account already holds", () => {
+  it("reads the two raw columns into the wire vocabulary", () => {
+    expect(
+      readHeldUnitPreferences({
+        glucoseUnit: "mmol/L",
+        unitPreference: "imperial",
+      }),
+    ).toEqual({ glucoseUnit: "mmol/L", unitPreference: "imperial" });
+  });
+
+  it("answers null for a column that was never chosen", () => {
+    const held = readHeldUnitPreferences({
+      glucoseUnit: null,
+      unitPreference: null,
+    });
+    expect(held).toEqual({ glucoseUnit: null, unitPreference: null });
+    expect(accountHoldsBothUnits(held)).toBe(false);
+  });
+
+  it("does not mistake a value it does not know for a chosen one", () => {
+    expect(
+      readHeldUnitPreferences({
+        glucoseUnit: "stones",
+        unitPreference: "metrick",
+      }),
+    ).toEqual({ glucoseUnit: null, unitPreference: null });
+  });
+
+  it("needs both preferences before the question answers itself", () => {
+    expect(
+      accountHoldsBothUnits({ glucoseUnit: "mg/dL", unitPreference: null }),
+    ).toBe(false);
+    expect(
+      accountHoldsBothUnits({ glucoseUnit: null, unitPreference: "metric" }),
+    ).toBe(false);
+    expect(
+      accountHoldsBothUnits({
+        glucoseUnit: "mg/dL",
+        unitPreference: "metric",
+      }),
+    ).toBe(true);
+  });
+});
+
+describe("resolveOnboardingSteps", () => {
+  const held = {
+    glucoseUnit: "mg/dL",
+    unitPreference: "metric",
+  } as const;
+
+  it("marks units done when the account already holds both preferences", () => {
+    const resolved = resolveOnboardingSteps(defaultOnboardingSteps(), held);
+    expect(resolved.find((s) => s.id === "units")?.status).toBe("done");
+  });
+
+  it("leaves units pending while either preference is unset", () => {
+    for (const partial of [
+      { glucoseUnit: "mg/dL", unitPreference: null },
+      { glucoseUnit: null, unitPreference: "metric" },
+      { glucoseUnit: null, unitPreference: null },
+    ] as const) {
+      const resolved = resolveOnboardingSteps(
+        defaultOnboardingSteps(),
+        partial,
+      );
+      expect(resolved.find((s) => s.id === "units")?.status).toBe("pending");
+    }
+  });
+
+  it("never overwrites an answer that was actually given", () => {
+    const skipped = defaultOnboardingSteps().map((s) =>
+      s.id === "units" ? { ...s, status: "skipped" as const } : s,
+    );
+    expect(
+      resolveOnboardingSteps(skipped, held).find((s) => s.id === "units")
+        ?.status,
+    ).toBe("skipped");
+  });
+
+  it("touches no other step", () => {
+    const resolved = resolveOnboardingSteps(defaultOnboardingSteps(), held);
+    expect(
+      resolved.filter((s) => s.id !== "units").map((s) => s.status),
+    ).toEqual(
+      defaultOnboardingSteps()
+        .filter((s) => s.id !== "units")
+        .map((s) => s.status),
+    );
+  });
+});

src/lib/onboarding/needs-store.ts

@@ -17,6 +17,8 @@ import {
   parseOnboardingFirstResult,
   parseOnboardingNeeds,
   parseOnboardingSteps,
+  resolveOnboardingSteps,
+  type HeldUnitPreferences,
   type OnboardingStateDto,
 } from "./needs";
 import type { OnboardingRecordState } from "./needs-apply";
@@ -56,13 +58,21 @@ export function readOnboardingRecordState(
   };
 }
 
-/** The shape published on the account payload. */
+/**
+ * The shape published on the account payload.
+ *
+ * `held` is what the RECORD's account already carries in its two unit columns,
+ * and it is a required argument rather than an optional one: the Q6 "skipped
+ * when the account already holds them" rule has to hold for every reader of
+ * this DTO, and a caller that has not thought about it should fail to compile.
+ */
 export function toOnboardingStateDto(
   row: OnboardingRecordRow | null,
+  held: HeldUnitPreferences,
 ): OnboardingStateDto {
   const state = readOnboardingRecordState(row);
   return {
-    steps: state.steps,
+    steps: resolveOnboardingSteps(state.steps, held),
     needs: state.needs,
     completedAt: row?.completedAt?.toISOString() ?? null,
     firstResult: state.firstResult,
@@ -70,19 +80,24 @@ export function toOnboardingStateDto(
 }
 
 /**
- * Load the published state for one record.
+ * Load one record's stored onboarding row, or `null` when it never entered the
+ * flow.
+ *
+ * The row rather than the DTO, because the DTO needs the record's unit columns
+ * as well and the caller is the one holding them — the account payload already
+ * reads that user row for the cycle gate, and a second read of the same row on
+ * the hottest endpoint in the app would buy nothing.
  *
  * Takes the delegate rather than the whole client, matching the section
  * builders next door, so the route's global client and a worker's local one
  * share the same read.
  */
-export async function loadOnboardingState(
+export async function loadOnboardingRecordRow(
   prisma: Pick<PrismaClient, "onboardingRecord">,
   recordId: string,
-): Promise<OnboardingStateDto> {
-  const row = await prisma.onboardingRecord.findUnique({
+): Promise<OnboardingRecordRow | null> {
+  return prisma.onboardingRecord.findUnique({
     where: { userId: recordId },
     select: ONBOARDING_RECORD_SELECT,
   });
-  return toOnboardingStateDto(row);
 }

src/lib/onboarding/needs.ts

@@ -323,3 +323,60 @@ export function isOnboardingSettled(
   if (state.completedAt === null) return false;
   return state.firstResult === null || state.firstResult.completedAt !== null;
 }
+
+/**
+ * The unit columns an account already holds, read as the vocabulary the wire
+ * publishes.
+ *
+ * `User.unitPreference` is nullable and `null` means "default metric", which
+ * the account payload coerces away (`unitPreference === "imperial" ?
+ * "imperial" : "metric"`). That coercion is fine to compare a CHOSEN value
+ * against and useless for telling a chosen metric from an unset column, so the
+ * server reads the raw column here and answers `null` for "never chosen".
+ */
+export interface HeldUnitPreferences {
+  glucoseUnit: "mg/dL" | "mmol/L" | null;
+  unitPreference: "metric" | "imperial" | null;
+}
+
+/** Read the two raw columns into the wire vocabulary; anything else is unset. */
+export function readHeldUnitPreferences(row: {
+  glucoseUnit: string | null;
+  unitPreference: string | null;
+}): HeldUnitPreferences {
+  return {
+    glucoseUnit: member(["mg/dL", "mmol/L"] as const, row.glucoseUnit),
+    unitPreference: member(["metric", "imperial"] as const, row.unitPreference),
+  };
+}
+
+/** True once the account carries both unit preferences as deliberate values. */
+export function accountHoldsBothUnits(held: HeldUnitPreferences): boolean {
+  return held.glucoseUnit !== null && held.unitPreference !== null;
+}
+
+/**
+ * The steps as they should be READ, given what the account already holds.
+ *
+ * The spec's Q6 is "skipped when the account already holds them", and the
+ * evidence for that is the two unit columns, not the questionnaire's own
+ * memory of having asked. So a record whose account carries both preferences
+ * publishes `units` as `done` even though nobody answered it here — which is
+ * what stops the flow re-asking a value the account holds, and what stops the
+ * completion gate below waiting for an answer to a question the flow will
+ * never show.
+ *
+ * Read-side only: the stored ledger keeps saying `pending`, because the person
+ * really has not answered it, and an answer given later still lands.
+ */
+export function resolveOnboardingSteps(
+  steps: readonly OnboardingStepState[],
+  held: HeldUnitPreferences,
+): OnboardingStepState[] {
+  if (!accountHoldsBothUnits(held)) return [...steps];
+  return steps.map((step) =>
+    step.id === "units" && step.status === "pending"
+      ? { ...step, status: "done" as const }
+      : step,
+  );
+}

src/lib/openapi/routes/onboarding.ts

@@ -91,7 +91,7 @@ const onboardingStepResource = z
   .meta({
     id: "OnboardingStepState",
     description:
-      "One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather than refusing the list, so the flow may grow a screen without a client release.",
+      "One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather than refusing the list, so the flow may grow a screen without a client release.\n\n`units` carries one resolution the others do not: a record whose account already holds BOTH unit preferences reads as `done` whether or not the question was ever put, which is what stops the flow re-asking a value the account holds.",
   });
 
 const onboardingFirstResultResource = z

tests/integration/onboarding-needs.test.ts

@@ -226,6 +226,35 @@ describe("PATCH /api/onboarding/answers", () => {
     expect(row).toEqual({ glucoseUnit: "mmol/L", unitPreference: "imperial" });
   });
 
+  it("does not re-ask the units the account already holds", async () => {
+    const user = await makeUser("units-held");
+    await signIn(user.id);
+    await getPrismaClient().user.update({
+      where: { id: user.id },
+      data: { glucoseUnit: "mg/dL", unitPreference: "metric" },
+    });
+
+    const state = await readState(
+      await patchAnswer({ step: "who", recordTarget: "me" }),
+    );
+    // Nobody answered Q6; the account's own columns did.
+    expect(statusOf(state, "units")).toBe("done");
+    expect(state.needs.units).toEqual({
+      glucoseUnit: null,
+      unitPreference: null,
+    });
+
+    // Half a preference is not an answer.
+    await getPrismaClient().user.update({
+      where: { id: user.id },
+      data: { unitPreference: null },
+    });
+    const half = await readState(
+      await patchAnswer({ step: "who", recordTarget: "me" }),
+    );
+    expect(statusOf(half, "units")).toBe("pending");
+  });
+
   it("refuses a body that names a step it does not answer", async () => {
     const user = await makeUser("invalid");
     await signIn(user.id);