diff --git a/src/app/api/onboarding/complete/route.ts b/src/app/api/onboarding/complete/route.ts index d34e644f8..aa9eb6813 100644 --- a/src/app/api/onboarding/complete/route.ts +++ b/src/app/api/onboarding/complete/route.ts @@ -6,9 +6,11 @@ import { setOnboardingPendingCookie } from "@/lib/auth/session"; import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; import { getOrCreateCycleProfile } from "@/lib/cycle/profile"; import { normalisePrefs } from "@/lib/modules/gate"; +import { modulesHoldingRecordData } from "@/lib/modules/domain-data"; import { deriveOnboardingModuleDefaults, mergeDerivedModulePreferences, + OWNED_MODULE_KEYS, } from "@/lib/modules/registry"; import { everyOnboardingQuestionSettled, @@ -148,6 +150,7 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { } let derived = false; + let keptForData: string[] = []; if (record.modulesDerivedAt === null) { const defaults = deriveOnboardingModuleDefaults({ recordTarget: state.needs.recordTarget, @@ -163,9 +166,22 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { where: { id: userId }, select: { modulePreferencesJson: true }, }); + // Only the modules the answers would switch OFF are worth asking about, + // and a record that already holds rows in one of those domains keeps its + // surface: the flow orders a record, and it cannot order away content + // somebody already has. + const wouldSwitchOff = OWNED_MODULE_KEYS.filter( + (key) => defaults.preferences[key] === false, + ); + const holdsData = await modulesHoldingRecordData( + prisma, + userId, + wouldSwitchOff, + ); const merged = mergeDerivedModulePreferences( normalisePrefs(row?.modulePreferencesJson), defaults.preferences, + holdsData, ); await prisma.user.update({ where: { id: userId }, @@ -187,6 +203,7 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { // A module toggle adds or removes a Health Score pillar, so the cached // composite was computed from a composition that no longer holds. invalidateUserHealthScore(userId); + keptForData = [...holdsData]; derived = true; } @@ -208,7 +225,10 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { annotate({ action: { name: "onboarding.needs.complete" }, - meta: { outcome: derived ? "derived" : "already_derived" }, + meta: { + outcome: derived ? "derived" : "already_derived", + keptForData: keptForData.length, + }, }); return toOnboardingStateDto(written, held); diff --git a/src/lib/modules/__tests__/onboarding-needs-map.test.ts b/src/lib/modules/__tests__/onboarding-needs-map.test.ts index 79dbaa825..342fc2bd0 100644 --- a/src/lib/modules/__tests__/onboarding-needs-map.test.ts +++ b/src/lib/modules/__tests__/onboarding-needs-map.test.ts @@ -24,6 +24,9 @@ import { type OnboardingModuleNeeds, } from "../registry"; +/** A record with nothing in it — the state a first setup describes. */ +const NO_DATA: ReadonlySet = new Set(); + function needs( overrides: Partial = {}, ): OnboardingModuleNeeds { @@ -174,6 +177,7 @@ describe("mergeDerivedModulePreferences", () => { const merged = mergeDerivedModulePreferences( { mcp: true, labs: true }, preferences, + NO_DATA, ); expect(merged.mcp).toBe(true); expect(merged.labs).toBe(true); @@ -181,7 +185,7 @@ describe("mergeDerivedModulePreferences", () => { it("does turn off a module the record only had by default", () => { const { preferences } = deriveOnboardingModuleDefaults(needs()); - const merged = mergeDerivedModulePreferences({}, preferences); + const merged = mergeDerivedModulePreferences({}, preferences, NO_DATA); expect(merged.labs).toBe(false); expect(merged.workouts).toBe(false); }); @@ -190,7 +194,11 @@ describe("mergeDerivedModulePreferences", () => { const { preferences } = deriveOnboardingModuleDefaults( needs({ areas: ["labs"] }), ); - const merged = mergeDerivedModulePreferences({ labs: false }, preferences); + const merged = mergeDerivedModulePreferences( + { labs: false }, + preferences, + NO_DATA, + ); expect(merged.labs).toBe(true); }); @@ -199,16 +207,61 @@ describe("mergeDerivedModulePreferences", () => { const merged = mergeDerivedModulePreferences( { weight: false, nonsense: true }, preferences, + NO_DATA, ); expect(merged).not.toHaveProperty("weight"); expect(merged).not.toHaveProperty("nonsense"); }); + it("never switches off a domain the record already holds rows in", () => { + const { preferences } = deriveOnboardingModuleDefaults(needs()); + expect(preferences.labs).toBe(false); + expect(preferences.mood).toBe(false); + + // An established record with no stored preference at all: default-on, and + // nobody ever touched the toggle. A derived `false` would take the nav + // entry, the dashboard widget and the settings entry away from a domain + // holding years of rows, so the key is left exactly as it was — and + // absence means on. + const merged = mergeDerivedModulePreferences( + {}, + preferences, + new Set(["labs", "mood"]), + ); + expect(merged).not.toHaveProperty("labs"); + expect(merged).not.toHaveProperty("mood"); + expect(merged.workouts).toBe(false); + }); + + it("keeps a stored explicit off where the domain holds rows", () => { + const { preferences } = deriveOnboardingModuleDefaults(needs()); + // Somebody switched labs off by hand and still has old results. The flow + // must not switch it back on, and must not restate the decision either. + const merged = mergeDerivedModulePreferences( + { labs: false }, + preferences, + new Set(["labs"]), + ); + expect(merged.labs).toBe(false); + }); + + it("still switches a domain ON when the answers name it, rows or not", () => { + const { preferences } = deriveOnboardingModuleDefaults( + needs({ areas: ["labs"] }), + ); + const merged = mergeDerivedModulePreferences( + {}, + preferences, + new Set(["labs"]), + ); + expect(merged.labs).toBe(true); + }); + it("leaves the two delegated keys alone in both directions", () => { const { preferences } = deriveOnboardingModuleDefaults( needs({ areas: ["cycle"] }), ); - const merged = mergeDerivedModulePreferences({}, preferences); + const merged = mergeDerivedModulePreferences({}, preferences, NO_DATA); expect(merged).not.toHaveProperty("cycle"); expect(merged).not.toHaveProperty("coach"); }); diff --git a/src/lib/modules/domain-data.ts b/src/lib/modules/domain-data.ts new file mode 100644 index 000000000..cadb16efd --- /dev/null +++ b/src/lib/modules/domain-data.ts @@ -0,0 +1,225 @@ +/** + * v1.39 (C1) — does a record already hold data in a module's domain? + * + * The setup flow's derivation writes an explicit `false` for every module the + * answers did not name, and that is the right answer for a record that is + * being set up. It is the wrong answer for a record that already has years of + * rows: an established self-hoster upgrading into the flow has, for most + * modules, no stored preference at all — they were default-on and nobody ever + * touched the toggle — so the first confirm would switch off the nav entry, + * the dashboard widget and the settings entry for domains that hold content. + * The rows would survive; the surfaces would not. The design spec's second + * principle is that nothing is lost by a choice, and it says that about a NEW + * record; it says nothing about one with content, so the derivation asks here + * before it retracts anything. + * + * The ownership question is not re-answered here. The measurement-level + * domains come from {@link measurementTypesOwnedBy}, which is the same map the + * MCP reads and the Coach snapshot gate off, so a type that changes hands + * cannot drift out of sync with this. What this file adds is the per-module + * table each domain's non-measurement content lives in, one named entry per + * module, exhaustive over `OwnedModuleKey` so a module added next year fails + * to compile until somebody names where its content lives. + */ +import type { PrismaClient } from "@/generated/prisma/client"; + +import { measurementTypesOwnedBy } from "./measurement-scope"; +import type { OwnedModuleKey } from "./registry"; + +/** The delegates this file reads. Narrow so a worker's client fits too. */ +type DomainDataClient = Pick< + PrismaClient, + | "measurement" + | "moodEntry" + | "workout" + | "labResult" + | "illnessEpisode" + | "userAchievement" + | "insightNarrative" + | "medication" + | "encounter" + | "environmentContext" + | "mcpOAuthConnection" + | "inboundDocument" + | "mentalHealthAssessment" + | "nutrientIntakeDay" + | "vaccinationRecord" +>; + +type DomainProbe = ( + client: DomainDataClient, + recordId: string, +) => Promise; + +/** One indexed existence read; a soft-deleted row is not content. */ +async function anyRow(find: PromiseLike): Promise { + return (await find) !== null; +} + +/** Does the record hold any measurement in the types this module owns? */ +function measurementProbe(key: OwnedModuleKey): DomainProbe { + const types = measurementTypesOwnedBy(key); + return async (client, recordId) => { + if (types.length === 0) return false; + return anyRow( + client.measurement.findFirst({ + where: { userId: recordId, deletedAt: null, type: { in: types } }, + select: { id: true }, + }), + ); + }; +} + +/** Either the module's own table or the measurements it owns. */ +function eitherProbe(a: DomainProbe, b: DomainProbe): DomainProbe { + return async (client, recordId) => + (await a(client, recordId)) || (await b(client, recordId)); +} + +/** + * Where each module's content lives. + * + * Exhaustive by type. Every entry names one table (or one measurement domain) + * whose rows mean "this record uses this module", and nothing here reads a + * preference, a connection state or a cache — the question is content, not + * configuration. + */ +const MODULE_DOMAIN_PROBES: Record = { + mood: eitherProbe( + (client, recordId) => + anyRow( + client.moodEntry.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + measurementProbe("mood"), + ), + sleep: measurementProbe("sleep"), + glucose: measurementProbe("glucose"), + workouts: eitherProbe( + (client, recordId) => + anyRow( + client.workout.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + measurementProbe("workouts"), + ), + recovery: measurementProbe("recovery"), + labs: (client, recordId) => + anyRow( + client.labResult.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + illness: (client, recordId) => + anyRow( + client.illnessEpisode.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + achievements: (client, recordId) => + anyRow( + client.userAchievement.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + insights: (client, recordId) => + anyRow( + client.insightNarrative.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + medications: (client, recordId) => + anyRow( + client.medication.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + // The doctor report is built from the visits, so a recorded encounter is + // what "this record uses the report surface" looks like in the data. + doctorReport: (client, recordId) => + anyRow( + client.encounter.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + environment: eitherProbe( + (client, recordId) => + anyRow( + client.environmentContext.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + measurementProbe("environment"), + ), + // The endpoint holds no content of its own; a live authorisation is the + // nearest thing, and switching it off under a connected assistant would + // break a wire somebody set up on purpose. + mcp: (client, recordId) => + anyRow( + client.mcpOAuthConnection.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + inboundDocuments: (client, recordId) => + anyRow( + client.inboundDocument.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + mentalHealth: (client, recordId) => + anyRow( + client.mentalHealthAssessment.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + nutrients: (client, recordId) => + anyRow( + client.nutrientIntakeDay.findFirst({ + where: { userId: recordId }, + // Keyed on the day and the nutrient, not on an id of its own. + select: { userId: true }, + }), + ), + vaccinations: (client, recordId) => + anyRow( + client.vaccinationRecord.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), +}; + +/** + * Which of `candidates` already hold data for this record. + * + * Only the candidates are read, because the caller only ever asks about the + * modules it is about to switch off — a confirm that names everything reads + * nothing. Each probe is one indexed existence read. + */ +export async function modulesHoldingRecordData( + client: DomainDataClient, + recordId: string, + candidates: readonly OwnedModuleKey[], +): Promise> { + const holding = new Set(); + await Promise.all( + candidates.map(async (key) => { + if (await MODULE_DOMAIN_PROBES[key](client, recordId)) holding.add(key); + }), + ); + return holding; +} diff --git a/src/lib/modules/measurement-scope.ts b/src/lib/modules/measurement-scope.ts index 58d4b58ac..2ff632964 100644 --- a/src/lib/modules/measurement-scope.ts +++ b/src/lib/modules/measurement-scope.ts @@ -224,3 +224,20 @@ export function moduleForMeasurementType( ): ModuleKey | null { return OWNER_BY_MEASUREMENT_TYPE.get(type) ?? null; } + +/** + * The inverse: every measurement type `key` owns, or an empty list for a + * module that owns no measurement-level domain. + * + * Derived from the same map {@link moduleForMeasurementType} answers from, so + * the two can never disagree about who owns what. Read by the setup flow's + * derivation, which asks "does this record already hold data in that domain" + * before it switches a module off. + */ +export function measurementTypesOwnedBy(key: ModuleKey): MeasurementType[] { + const out: MeasurementType[] = []; + for (const [type, owner] of OWNER_BY_MEASUREMENT_TYPE) { + if (owner === key) out.push(type); + } + return out; +} diff --git a/src/lib/modules/registry.ts b/src/lib/modules/registry.ts index b2346dc6e..281944342 100644 --- a/src/lib/modules/registry.ts +++ b/src/lib/modules/registry.ts @@ -579,7 +579,7 @@ export function deriveOnboardingModuleDefaults( /** * Merge a derived map into the one the record already holds, without ever - * retracting a decision the person made by hand. + * retracting a decision the person made by hand or a domain they already use. * * The rule is one-directional: a derived `true` always lands, a derived * `false` lands only where the stored map does not already carry an explicit @@ -587,6 +587,18 @@ export function deriveOnboardingModuleDefaults( * and did not tick anything that implies it, keeps it on — the questionnaire * orders a record, it does not undo it. * + * `holdsData` is the second thing a derived `false` cannot cross, and it is + * about a record with content rather than a record with an opinion. An + * established account upgrading into the setup flow has, for most modules, no + * stored preference at all — they were default-on and nobody ever touched the + * toggle — so without this the first confirm would switch off the navigation + * entry, the dashboard widget and the settings entry for every domain the + * person did not tick, including the ones holding years of rows. The design + * spec's "nothing is lost by a choice" is written about a NEW record; for one + * that already has content the honest answer is to write nothing and leave the + * stored value exactly where it was. Absence means ON, so leaving the key + * alone is what keeps the surface. + * * Keys the stored map carries that are not module keys are dropped; the caller * is expected to have normalised it (`normalisePrefs`), and this is the second * belt on the same trousers rather than a place to be clever. @@ -594,6 +606,7 @@ export function deriveOnboardingModuleDefaults( export function mergeDerivedModulePreferences( existing: Readonly>, derived: Readonly>, + holdsData: ReadonlySet, ): Record { const merged: Record = {}; for (const [key, value] of Object.entries(existing)) { @@ -605,6 +618,7 @@ export function mergeDerivedModulePreferences( continue; } if (merged[key] === true) continue; // switched on by hand — never retracted + if (holdsData.has(key)) continue; // the domain holds rows — never retracted merged[key] = false; } return merged; diff --git a/tests/integration/onboarding-needs.test.ts b/tests/integration/onboarding-needs.test.ts index e4277eacc..01b5ae15f 100644 --- a/tests/integration/onboarding-needs.test.ts +++ b/tests/integration/onboarding-needs.test.ts @@ -331,6 +331,44 @@ describe("POST /api/onboarding/complete", () => { expect((await modulePrefs(user.id)).mcp).toBe(true); }); + it("never switches off a domain the record already holds rows in", async () => { + const user = await makeUser("established"); + await signIn(user.id); + // An account that has been running for years: mood entries and workouts, + // no stored module preference at all, because both were default-on and + // nobody ever touched a toggle. The questions below name neither. + await getPrismaClient().moodEntry.create({ + data: { + userId: user.id, + date: "2026-09-01", + mood: "GUT", + score: 4, + moodLoggedAt: new Date("2026-09-01T18:00:00.000Z"), + }, + }); + await getPrismaClient().workout.create({ + data: { + userId: user.id, + sportType: "RUNNING", + startedAt: new Date("2026-09-01T06:00:00.000Z"), + endedAt: new Date("2026-09-01T06:30:00.000Z"), + durationSec: 1800, + }, + }); + + await answerTheQuestions(); + await postComplete(); + + const prefs = await modulePrefs(user.id); + // Left alone rather than written false: absence is what "on" looks like + // in the stored map, and the surfaces stay. + expect(prefs).not.toHaveProperty("mood"); + expect(prefs).not.toHaveProperty("workouts"); + // A domain with nothing in it is still switched off by the answers. + expect(prefs.illness).toBe(false); + expect(prefs.vaccinations).toBe(false); + }); + it("switches cycle tracking on through its own column when the area is chosen", async () => { const user = await makeUser("cycle"); await signIn(user.id);