From 75c43c74bce8c76d08a01d0ac3f212d5f64daae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 8 Aug 2026 15:30:38 +0200 Subject: [PATCH] docs(mood): state the validation's real leak boundary, drop the unused weekday floor, correct two comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four honesty fixes with no behaviour change to the math. The validation header claimed more than it proves. The ridge FIT is out-of-sample and no training fold sees ahead — that holds. But feature SELECTION and per-user standardisation both run over the whole window before the split, a small documented optimism (measured at about one part in sixty on the error). The header now says exactly what is and is not leak-free, and says to nest the screening if the feature set ever grows or the optimism is ever measured larger. The per-weekday observation floor had no consumer: no weekday board is built on this fit, the forecast only publishes seasonalUnlocked and leaves the weekday view to the insights page, which has its own floors. Removed rather than left as a number nothing reads, with a note to bring it back alongside such a board. Two comments were wrong and are now true: the route's parameter is declared for the exported type, not to steer a method check the wrapper resolves from the Next.js request with a GET fallback; and the label surface imports the key parser from feature-keys.ts, not features.ts. And the write-surface guard now states its own structural blind spot, the way the session-surface guard does: the wipe path deletes this table through a dynamically-resolved delegate the literal matcher cannot see, which is why that deletion is correctly not counted as a writer. The Coach staleness fixture moves above the forecast floor, because the read path now refuses below it: a one-entry account correctly gets an absence rather than the present forecast that case exercises. --- ...ood-prediction-write-surface-guard.test.ts | 13 +++++++++ src/app/api/mood/prognosis/route.ts | 16 ++++++----- .../mood-prognosis/__tests__/gates.test.ts | 19 ------------- src/lib/analytics/mood-prognosis/load.ts | 11 +++++--- .../analytics/mood-prognosis/thresholds.ts | 16 ++++------- src/lib/analytics/mood-prognosis/validate.ts | 28 +++++++++++++++++++ .../mood-dimension-features.test.ts | 19 ++++++++++++- 7 files changed, 81 insertions(+), 41 deletions(-) diff --git a/src/__tests__/mood-prediction-write-surface-guard.test.ts b/src/__tests__/mood-prediction-write-surface-guard.test.ts index 3bacd4a4f..af83e70d0 100644 --- a/src/__tests__/mood-prediction-write-surface-guard.test.ts +++ b/src/__tests__/mood-prediction-write-surface-guard.test.ts @@ -23,6 +23,19 @@ * the legitimate writer really does make. `describe("the matcher sees what it * looks for")` proves the regex against synthetic sources, including the * broken-across-lines form that defeated an earlier guard in this repository. + * + * **What this guard structurally cannot see, stated like the session-surface + * guard states its own limit.** The matcher keys on a literal `moodPrediction` + * delegate access. The "Delete All Data" wipe DOES delete this table — the + * model is in `WIPE_MODELS` (`src/lib/data-wipe/wipe-plan.ts`) — but it does so + * through a delegate resolved dynamically from the model name (`client[key] + * .deleteMany`), so no `moodPrediction.deleteMany(` literal exists for the + * matcher to catch, and that deletion is correctly NOT counted as a writer. + * The wipe path's own completeness is proven elsewhere, by + * `data-wipe-completeness.test.ts` enumerating the schema; this guard is only + * the one about request-time writers, and it cannot be the gate on a dynamic + * delegate. A future writer that reached the table through a computed property + * would slip this matcher for the same reason. */ import { existsSync, readFileSync, readdirSync } from "node:fs"; import { join, sep } from "node:path"; diff --git a/src/app/api/mood/prognosis/route.ts b/src/app/api/mood/prognosis/route.ts index 2129e61e7..8296d7ef3 100644 --- a/src/app/api/mood/prognosis/route.ts +++ b/src/app/api/mood/prognosis/route.ts @@ -36,13 +36,15 @@ import { requireModuleEnabled } from "@/lib/modules/gate"; export const dynamic = "force-dynamic"; -// The request is declared although the handler reads nothing off it: the -// wrapper takes the method and the path from it for the wide event, and -// `requireRecordAuth` reads the method to decide whether a `read` declaration -// really is a read. A handler with no parameter types as zero-arity, and a -// caller that then omits the request turns every method check into "unknown", -// which escalates a read to a write and refuses a delegate for the wrong -// reason. +// The parameter is declared although the handler body reads nothing off it, +// and the reason is the exported type rather than the runtime. `apiHandler` +// infers the type of `GET` from this callback, so a zero-argument handler +// would type `GET` as taking no arguments — and Next.js, and the integration +// tests, call it WITH the request. The wide event's method and path, and the +// read-vs-write check `requireRecordAuth` runs, both come from the request the +// wrapper receives from Next.js (not from this parameter), and both fall back +// to `GET` when no request is present — so an absent request keeps a read a +// read rather than escalating it. export const GET = apiHandler(async (_request: NextRequest) => { // `record`: see the file header. A scoped grant is refused here by // construction rather than served a narrowed payload. diff --git a/src/lib/analytics/mood-prognosis/__tests__/gates.test.ts b/src/lib/analytics/mood-prognosis/__tests__/gates.test.ts index 40be4d6b7..7895459d0 100644 --- a/src/lib/analytics/mood-prognosis/__tests__/gates.test.ts +++ b/src/lib/analytics/mood-prognosis/__tests__/gates.test.ts @@ -27,12 +27,10 @@ import { MIN_ENTRIES_FOR_SEASONAL, MIN_ENTRIES_LEARNING_PHASE, MIN_TAG_OCCURRENCES, - MIN_WEEKDAY_OBSERVATIONS, forecastGate, occurrencesSuffice, prognosisStage, seasonalComparisonsUnlocked, - weekdayObservationsSuffice, } from "../thresholds"; import { CONTEXT_MIN_PRESENT_DAYS } from "@/lib/insights/mood-context-crosstab"; @@ -42,7 +40,6 @@ describe("the ladder is ordered and its rungs are the documented numbers", () => expect(MIN_ENTRIES_FOR_FORECAST).toBe(30); expect(MIN_ENTRIES_FOR_REGULAR_FORECAST).toBe(60); expect(MIN_ENTRIES_FOR_SEASONAL).toBe(90); - expect(MIN_WEEKDAY_OBSERVATIONS).toBe(4); }); it("rises strictly, so no rung can be reached before the one below it", () => { @@ -162,22 +159,6 @@ describe("MIN_ENTRIES_FOR_SEASONAL — below it, no weekday or seasonal view", ( }); }); -describe("MIN_WEEKDAY_OBSERVATIONS — a weekday needs a month of itself", () => { - it(`refuses at ${MIN_WEEKDAY_OBSERVATIONS - 1}`, () => { - expect( - weekdayObservationsSuffice(MIN_WEEKDAY_OBSERVATIONS - 1), - "MIN_WEEKDAY_OBSERVATIONS admitted n-1 observations of one weekday", - ).toBe(false); - }); - - it(`admits at exactly ${MIN_WEEKDAY_OBSERVATIONS}`, () => { - expect( - weekdayObservationsSuffice(MIN_WEEKDAY_OBSERVATIONS), - "MIN_WEEKDAY_OBSERVATIONS refused its own value", - ).toBe(true); - }); -}); - describe("MIN_TAG_OCCURRENCES — a value seen four times is not evidence", () => { it(`refuses at ${MIN_TAG_OCCURRENCES - 1}`, () => { expect( diff --git a/src/lib/analytics/mood-prognosis/load.ts b/src/lib/analytics/mood-prognosis/load.ts index f21bc319a..930326756 100644 --- a/src/lib/analytics/mood-prognosis/load.ts +++ b/src/lib/analytics/mood-prognosis/load.ts @@ -1,10 +1,13 @@ /** * The database read behind the fit, and nothing else. * - * Separate from `features.ts` on purpose: that module is pure and is imported - * by the surface that labels a stored feature key, and a Prisma import in it - * would pull the database client towards a client bundle. The split is the - * boundary, not a preference. + * Separate from `features.ts` on purpose. `features.ts` is pure, and it shares + * `feature-keys.ts` with the client component that labels a stored feature key + * (`components/insights/mood/prognosis-feature-label.ts` imports the parser + * from there) — so those two modules form a Prisma-free island the browser + * bundle can reach. A database import in `features.ts` would drag the client + * towards that bundle; keeping the read here is what holds the line. The split + * is the boundary, not a preference. * * **One row per day, taken whole from one entry.** A day with two entries * contributes the later one — its target, its dimensions and its context diff --git a/src/lib/analytics/mood-prognosis/thresholds.ts b/src/lib/analytics/mood-prognosis/thresholds.ts index ebfbca2ee..88aab2821 100644 --- a/src/lib/analytics/mood-prognosis/thresholds.ts +++ b/src/lib/analytics/mood-prognosis/thresholds.ts @@ -86,11 +86,12 @@ export const MIN_ENTRIES_FOR_SEASONAL = 90; */ export const MIN_TAG_OCCURRENCES = CONTEXT_MIN_PRESENT_DAYS; -/** - * Observations of one weekday before that weekday may be compared to the - * others. Four is a month of Mondays. - */ -export const MIN_WEEKDAY_OBSERVATIONS = 4; +// A per-weekday observation floor was drafted here for a weekday-comparison +// board built on the forecast. That board was not built — the forecast +// publishes `seasonalUnlocked` and leaves the weekday view to the mood +// insights page, which has its own floors — so the constant had no consumer +// and was removed rather than left as a number nothing reads. If a weekday +// comparison on THIS fit is ever built, reintroduce the floor with it. /** Where an account sits on the ladder. */ export type PrognosisStage = @@ -174,11 +175,6 @@ export function seasonalComparisonsUnlocked(entries: number): boolean { return entries >= MIN_ENTRIES_FOR_SEASONAL; } -/** Whether one weekday has been seen often enough to be compared. */ -export function weekdayObservationsSuffice(observations: number): boolean { - return observations >= MIN_WEEKDAY_OBSERVATIONS; -} - /** Whether a tag or context value has been recorded often enough to name. */ export function occurrencesSuffice(occurrences: number): boolean { return occurrences >= MIN_TAG_OCCURRENCES; diff --git a/src/lib/analytics/mood-prognosis/validate.ts b/src/lib/analytics/mood-prognosis/validate.ts index d5568c2e4..fb86a9752 100644 --- a/src/lib/analytics/mood-prognosis/validate.ts +++ b/src/lib/analytics/mood-prognosis/validate.ts @@ -20,6 +20,34 @@ * the honest width. It widens when the model is bad, which is the property the * surface depends on to report low confidence instead of a confident wrong * number. + * + * **What is out-of-sample here, and what is not — stated exactly, because the + * boundary is easy to overclaim.** The RIDGE FIT is honest: within every fold + * the coefficients are solved on the training prefix alone and scored on + * validation rows the fit never saw, and no training fold holds a row dated + * after its validation fold. That is the claim `rolling-origin.test.ts` + * proves, and it is the one that matters for "does λ generalise". + * + * Two steps sit OUTSIDE that guarantee, on purpose: + * + * * **Feature selection.** The screening in `features.ts` runs once over the + * whole window before the split, so the feature set a fold trains on was + * chosen with that fold's validation rows in view. This is the classic + * "screen, then cross-validate" optimism. + * * **Standardisation.** The per-user mean and spread are taken over the + * whole window too, so a validation row is z-scored against statistics its + * own value helped compute. + * + * Both make the reported error slightly better than a fully-nested procedure + * would — measured here at roughly one part in sixty on the mean absolute + * error and a couple of points of coverage, i.e. small enough that the band is + * not dishonest but not zero. Nesting the screening and the standardisation + * inside each fold would remove even that, at the cost of a per-fold feature + * set and a materially larger amount of machinery; the trade was taken the + * cheap way deliberately, and this comment is the price of taking it — a + * reader must not read "no fold sees ahead" as covering the SELECTION, only + * the FIT. If the feature set ever grows past a handful of strong columns, or + * the optimism is ever measured larger, nest it. */ import { fitRidge, predictWithFit, type RidgeFit } from "./ridge"; import type { FeatureMatrix } from "./features"; diff --git a/tests/integration/mood-dimension-features.test.ts b/tests/integration/mood-dimension-features.test.ts index 1baadaf42..2dac517fe 100644 --- a/tests/integration/mood-dimension-features.test.ts +++ b/tests/integration/mood-dimension-features.test.ts @@ -177,7 +177,24 @@ describe("level-A blocks reach the feature set with their own ages", () => { it("carries the day's forecast with its own age, and its own instruction", async () => { await seedUser(); - await seedEntry(0, { mood: "GUT", score: 4, a1: 7, a2: 3 }); + // Enough rated days that the account is above the forecast floor: the read + // path re-checks the ladder and refuses below it, stale row or not, so a + // one-entry account would (correctly) get an absence here rather than the + // present forecast this case is about. One statement rather than a loop of + // awaits, to keep the write short. + await getPrismaClient().moodEntry.createMany({ + data: Array.from({ length: 35 }, (_, d) => ({ + userId: USER, + date: shiftDateKey(todayKey(), -d), + tz: TZ, + mood: "GUT", + score: 4, + moodA1: 7, + stressA2: 3, + source: "MANUAL" as const, + moodLoggedAt: new Date(Date.now() - d * DAY_MS), + })), + }); // A forecast about the day before yesterday. Written straight to the // table: the job's own path is proven in `mood-prognosis-ladder.test.ts`, // and what this case is about is what the assistant is handed.