fix(onboarding): never switch a module off on a record that already uses it

The derivation wrote an explicit off for every module the answers did not
name, which is right for a record being set up and wrong for one with years of
rows behind it. An established self-hoster has, for most modules, no stored
preference at all — they were default-on and nobody ever touched the toggle —
so the first confirm took away the navigation entry, the dashboard widget and
the settings entry for domains full of content. The rows survived; the
surfaces did not.

A derived off now asks first, and only for the modules it would actually
retract. Ownership is not re-answered: the measurement domains come from the
same map the MCP reads and the coach snapshot gates off, and each remaining
module names the one table its content lives in, exhaustively, so a module
added later fails to compile until somebody says where its content is. A
module whose domain holds rows keeps whatever the record already had, because
absence is what "on" looks like in the stored map.
This commit is contained in:
Marc-André Bombeck committed 2026-09-10 11:48:27 +02:00
1 parent cc6e6ca221
commit 2d2350252f
6 files changed
+372 -5

No files matched your search

+21 -1
View File
@@ -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);
@@ -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<string> = new Set();
function needs(
overrides: Partial<OnboardingModuleNeeds> = {},
): 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");
});
+225
View File
@@ -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<boolean>;
/** One indexed existence read; a soft-deleted row is not content. */
async function anyRow(find: PromiseLike<unknown>): Promise<boolean> {
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<OwnedModuleKey, DomainProbe> = {
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<Set<OwnedModuleKey>> {
const holding = new Set<OwnedModuleKey>();
await Promise.all(
candidates.map(async (key) => {
if (await MODULE_DOMAIN_PROBES[key](client, recordId)) holding.add(key);
}),
);
return holding;
}
+17
View File
@@ -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;
}
+15 -1
View File
@@ -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<Record<string, boolean>>,
derived: Readonly<Record<OwnedModuleKey, boolean>>,
holdsData: ReadonlySet<string>,
): Record<string, boolean> {
const merged: Record<string, boolean> = {};
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;
@@ -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);