diff --git a/src/__tests__/helpers/narrated-locale.ts b/src/__tests__/helpers/narrated-locale.ts index 7389fb835..a7042029b 100644 --- a/src/__tests__/helpers/narrated-locale.ts +++ b/src/__tests__/helpers/narrated-locale.ts @@ -103,6 +103,50 @@ function isLocaleType(node: ts.TypeNode | undefined, alias: string): boolean { return false; } +/** + * Does the type still let a caller say nothing? `Locale | undefined` and + * `Locale | null` carry no `?` and no default, so a check that only looked at + * the question mark read them as required — and they are the original bug + * wearing a different hat: the caller passes `undefined`, the body picks a + * language, and the sentence comes back in English again. `any` and `unknown` + * are the same hole with the door taken off. + */ +function toleratesAbsence(node: ts.TypeNode | undefined): boolean { + if (!node) return false; + if ( + node.kind === ts.SyntaxKind.UndefinedKeyword || + node.kind === ts.SyntaxKind.NullKeyword || + node.kind === ts.SyntaxKind.AnyKeyword || + node.kind === ts.SyntaxKind.UnknownKeyword || + node.kind === ts.SyntaxKind.VoidKeyword + ) { + return true; + } + if (ts.isLiteralTypeNode(node)) { + return node.literal.kind === ts.SyntaxKind.NullKeyword; + } + if (ts.isUnionTypeNode(node)) return node.types.some(toleratesAbsence); + return false; +} + +/** + * Property names given a default inside an object binding pattern — + * `function narrate({ locale = "en" }: { locale: Locale })`. The default sits + * on the binding, not on the parameter, so `parameter.initializer` is empty + * and the signature reads clean. + */ +function defaultedBindingNames( + parameter: ts.ParameterDeclaration, +): Set { + const names = new Set(); + if (!ts.isObjectBindingPattern(parameter.name)) return names; + for (const element of parameter.name.elements) { + if (element.initializer === undefined) continue; + names.add((element.propertyName ?? element.name).getText()); + } + return names; +} + /** Find a top-level function/interface declaration by name. */ function findDeclaration( source: ts.SourceFile, @@ -147,16 +191,23 @@ export function localeSites( owner: string, ownerOptional: boolean, ownerHasInitializer: boolean, + defaultedNames: Set = new Set(), ) => { for (const member of members) { if (!ts.isPropertySignature(member)) continue; if (!isLocaleType(member.type, alias)) continue; + const name = member.name.getText(); sites.push({ - where: `${owner}.${member.name.getText()}`, + where: `${owner}.${name}`, // The property itself may be required while the object it lives on is - // optional or defaulted — which puts the silence right back. - optional: member.questionToken !== undefined || ownerOptional, - hasInitializer: ownerHasInitializer, + // optional or defaulted — which puts the silence right back. So does a + // type that merely admits `undefined` / `null`: no question mark, and a + // caller can still hand over nothing. + optional: + member.questionToken !== undefined || + ownerOptional || + toleratesAbsence(member.type), + hasInitializer: ownerHasInitializer || defaultedNames.has(name), }); } }; @@ -168,13 +219,24 @@ export function localeSites( const owner = parameter.name.getText(); const optional = parameter.questionToken !== undefined; const hasInitializer = parameter.initializer !== undefined; + const defaultedNames = defaultedBindingNames(parameter); if (isLocaleType(parameter.type, alias)) { - sites.push({ where: owner, optional, hasInitializer }); + sites.push({ + where: owner, + optional: optional || toleratesAbsence(parameter.type), + hasInitializer, + }); continue; } const type = parameter.type; if (type && ts.isTypeLiteralNode(type)) { - collectFromMembers(type.members, owner, optional, hasInitializer); + collectFromMembers( + type.members, + owner, + optional, + hasInitializer, + defaultedNames, + ); } else if (type && ts.isTypeReferenceNode(type)) { const referenced = findDeclaration(source, type.typeName.getText()); if (referenced && ts.isInterfaceDeclaration(referenced)) { @@ -183,6 +245,7 @@ export function localeSites( type.typeName.getText(), optional, hasInitializer, + defaultedNames, ); } } diff --git a/src/__tests__/narrated-locale-guard-defeat.test.ts b/src/__tests__/narrated-locale-guard-defeat.test.ts index 475068986..cd94e11b1 100644 --- a/src/__tests__/narrated-locale-guard-defeat.test.ts +++ b/src/__tests__/narrated-locale-guard-defeat.test.ts @@ -107,6 +107,80 @@ describe("the locale guard cannot be talked around", () => { expect(site.optional).toBe(true); }); + it("catches `Locale | undefined` written WITHOUT the question mark", () => { + // The nastiest disguise, because it reads as required at a glance: no `?`, + // no default, and the field is spelled out in the signature. But the caller + // can still pass `undefined`, the body picks a language, and the sentence + // comes back in English — the original bug, one hat later. + const [site] = localeSites( + parse( + FILE, + `import type { Locale } from "@/lib/i18n/config"; + export function narrate(opts: { locale: Locale | undefined }) { + return opts.locale === undefined ? "en" : opts.locale; + }`, + ), + "narrate", + ); + expect(site.optional).toBe(true); + }); + + it("catches `Locale | null` the same way", () => { + const [site] = localeSites( + parse( + FILE, + `import type { Locale } from "@/lib/i18n/config"; + export function narrate(opts: { locale: Locale | null }) { + return opts.locale === null ? "en" : opts.locale; + }`, + ), + "narrate", + ); + expect(site.optional).toBe(true); + }); + + it("catches a locale widened to `any`", () => { + const [site] = localeSites( + parse( + FILE, + `import type { Locale } from "@/lib/i18n/config"; + export function narrate(opts: { locale: Locale | any }) { return opts; }`, + ), + "narrate", + ); + expect(site.optional).toBe(true); + }); + + it("catches a default hidden in the parameter's own binding pattern", () => { + // The field is required in the type and the parameter has no initializer, + // so both of the earlier checks read clean — the default sits on the + // destructuring instead. + const [site] = localeSites( + parse( + FILE, + `import type { Locale } from "@/lib/i18n/config"; + export function narrate({ locale = "en" }: { locale: Locale }) { return locale; }`, + ), + "narrate", + ); + expect(site.hasInitializer).toBe(true); + }); + + it("leaves a plainly required locale alone", () => { + // The shape the whole chain is supposed to have. If this ever reads as a + // violation the guard stops being a guard and starts being noise. + const [site] = localeSites( + parse( + FILE, + `import type { Locale } from "@/lib/i18n/config"; + export function narrate(opts: { locale: Locale }) { return opts.locale; }`, + ), + "narrate", + ); + expect(site.optional).toBe(false); + expect(site.hasInitializer).toBe(false); + }); + it("refuses to pass a declaration that dropped the locale entirely", () => { expect(() => localeSites( diff --git a/src/app/api/cycle/insights/__tests__/route-locale.test.ts b/src/app/api/cycle/insights/__tests__/route-locale.test.ts new file mode 100644 index 000000000..9cac955dc --- /dev/null +++ b/src/app/api/cycle/insights/__tests__/route-locale.test.ts @@ -0,0 +1,142 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +/** + * The cycle insights route has to say who is reading, too. + * + * Each surviving lagged row carries a finished `interpretation` sentence that + * this route serves straight to the client. The engine underneath can write it + * in any of the six languages, and it only writes the right one if the route + * resolves the reader's language and hands it over — the same handoff the + * metric page's "Coach read" strip was missing when it answered a German page + * in English. + * + * So what is pinned here is the handoff itself: the resolved locale arriving at + * `discoverPhaseCorrelations`, from the cookie a language switch sets and from + * the stored preference when there is no cookie. Without this the route could + * go back to naming one language outright and nothing in the suite would + * notice. + */ + +const discoverPhaseCorrelations = vi.fn(() => ({ + discovered: [], + pairsTested: 0, + fdrQ: 0.1, + minPairs: 20, +})); +vi.mock("@/lib/cycle/phase-crosstab", async (importOriginal) => ({ + ...(await importOriginal()), + discoverPhaseCorrelations: (args: unknown) => discoverPhaseCorrelations(args), +})); + +vi.mock("@/lib/db", () => ({ + prisma: { + auditLog: { create: vi.fn() }, + appSettings: { findUnique: vi.fn().mockResolvedValue(null) }, + menstrualCycle: { findMany: vi.fn().mockResolvedValue([]) }, + cycleDayLog: { findMany: vi.fn().mockResolvedValue([]) }, + measurement: { findMany: vi.fn().mockResolvedValue([]) }, + moodEntry: { findMany: vi.fn().mockResolvedValue([]) }, + user: { + findUnique: vi.fn().mockResolvedValue({ sourcePriorityJson: null }), + }, + }, +})); +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/cycle/gate", () => ({ + requireCycleEnabled: vi.fn(async () => ({ + enabled: true, + profile: { lutealPhaseLength: 14, averageCycleLength: 28 }, + })), +})); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn(async () => ({ allowed: true })), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +const localeCookie = vi.fn<() => string | undefined>(() => undefined); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: (name: string) => + name === "healthlog-locale" && localeCookie() !== undefined + ? { value: localeCookie() } + : undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { GET } from "../route"; +import { getSession } from "@/lib/auth/session"; + +function sessionWithLocale(locale: string | null) { + return { + session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: { + id: "user-1", + username: "testuser", + role: "USER" as const, + gender: "FEMALE" as const, + timezone: "Europe/Berlin", + locale, + }, + }; +} + +beforeEach(() => { + vi.mocked(getSession).mockResolvedValue( + sessionWithLocale(null) as unknown as Awaited< + ReturnType + >, + ); + discoverPhaseCorrelations.mockClear(); + localeCookie.mockReturnValue(undefined); +}); + +/** The locale the route handed the discovery engine on the last call. */ +function passedLocale(): unknown { + const args = discoverPhaseCorrelations.mock.calls.at(-1)?.[0] as + { locale?: unknown } | undefined; + return args?.locale; +} + +describe("GET /api/cycle/insights — the reader's language", () => { + it("passes the language the switcher's cookie names", async () => { + localeCookie.mockReturnValue("de"); + + const res = await GET(new Request("http://localhost/api/cycle/insights")); + + expect(res.status).toBe(200); + expect(passedLocale()).toBe("de"); + }); + + it("falls back to the stored preference when there is no cookie", async () => { + vi.mocked(getSession).mockResolvedValue( + sessionWithLocale("pl") as unknown as Awaited< + ReturnType + >, + ); + + await GET(new Request("http://localhost/api/cycle/insights")); + + expect(passedLocale()).toBe("pl"); + }); + + it("never hard-codes a language of its own", async () => { + // Two readers, two languages, from the same handler. A route that named + // one outright would pass this only by accident. + localeCookie.mockReturnValue("it"); + await GET(new Request("http://localhost/api/cycle/insights")); + const first = passedLocale(); + localeCookie.mockReturnValue("fr"); + await GET(new Request("http://localhost/api/cycle/insights")); + + expect([first, passedLocale()]).toEqual(["it", "fr"]); + }); +}); diff --git a/src/components/cycle/use-cycle.ts b/src/components/cycle/use-cycle.ts index 591971cfd..323a7ee19 100644 --- a/src/components/cycle/use-cycle.ts +++ b/src/components/cycle/use-cycle.ts @@ -204,8 +204,12 @@ export function useCycleProfile() { * insights tab. */ export function useCycleInsights() { + // The lagged rows come back with a narrated `interpretation` in the reader's + // language, so the cache cell belongs to one language — otherwise a switch + // would keep serving the sentence written for the previous one. + const { locale } = useTranslations(); return useQuery({ - queryKey: queryKeys.cycleInsights(), + queryKey: queryKeys.cycleInsights(locale), queryFn: () => apiGet("/api/cycle/insights"), staleTime: 5 * 60_000, }); diff --git a/src/lib/mcp/__tests__/rich-reads.test.ts b/src/lib/mcp/__tests__/rich-reads.test.ts index 907d9722f..923016292 100644 --- a/src/lib/mcp/__tests__/rich-reads.test.ts +++ b/src/lib/mcp/__tests__/rich-reads.test.ts @@ -218,6 +218,10 @@ describe("get_correlation", () => { expect(res.pair?.r).toBe(-0.42); expect(res.association).toBe("descriptive"); expect(res.windowDays).toBe(180); + // The `note` on the pair is a finished sentence an MCP client shows as it + // stands, so the account's language has to reach the reader. Nothing else + // in this file would notice if the read went back to asking in English. + expect(readCoachCorrelations).toHaveBeenCalledWith(USER, "de"); }); it("honest-null when the pair has no surviving association", async () => { diff --git a/src/lib/query-keys/cycle.ts b/src/lib/query-keys/cycle.ts index c4ca520d9..09e394aa7 100644 --- a/src/lib/query-keys/cycle.ts +++ b/src/lib/query-keys/cycle.ts @@ -22,7 +22,14 @@ export const cycleKeys = { * on-ramp reads this so a non-FEMALE account can opt in before the gated * cycle page is reachable. */ cyclePrefs: () => ["cycle", "prefs"] as const, - cycleInsights: () => ["cycle", "insights"] as const, + /** + * The FDR-guarded phase-correlation read (`/api/cycle/insights`). Keyed by + * locale: each surviving lagged row now carries an `interpretation` the + * server writes in the reader's language, so a cached payload belongs to one + * language only. `["cycle"]` stays the prefix, so every cycle write still + * evicts it through `cycleDependentKeys`. + */ + cycleInsights: (locale: string) => ["cycle", "insights", locale] as const, cycleDayLog: (date: string) => ["cycle", "day-log", date] as const, /** The caller's own custom symptoms (decrypted labels) the log-day sheet * merges into the seeded chip grid. */