From ec01fc0107e8c777b5141d6b6d74c9102a5a0e55 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Fri, 4 Sep 2026 09:08:15 +0200 Subject: [PATCH] =?UTF-8?q?fix(recipes):=20persiste=20les=20m=C3=A9tadonn?= =?UTF-8?q?=C3=A9es=20de=20clause=20des=20tech-steps=20=C3=A0=20l'=C3=A9di?= =?UTF-8?q?tion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `createRecipeInternal` écrivait, pour chaque technique détectée, ses ingrédients / ustensiles / températures de clause ; `updateRecipe` et le script `backfill-tech-steps.ts` n'écrivaient que le span de la technique. Toute recette ré-enregistrée via l'UI (ou touchée par le backfill) perdait donc silencieusement ces métadonnées — et `cooking-optimizer.ts` ne pouvait plus détecter de conflit de four entre deux préchauffages dès qu'une recette avait été éditée une fois (ni sur les imports antérieurs à la détection des températures, jamais ré-enregistrés). - `toStepTechStepCreateInputs` (nouveau, `lib/recipe-matching/`) : seule source de vérité du payload `techSteps.create`, utilisée par les trois appelants pour qu'ils ne divergent plus. - `backfill-tech-steps.ts` : `createMany` -> `create` par ligne (createMany ne fait pas les écritures imbriquées) + doc à jour. - Rattrapage automatique au déploiement : `model BackfillState` + migration `backfill_state`, script `backfill-runtime.ts` ajouté au CMD Docker après `seed-runtime`. Gardé par un marqueur (tourne une seule fois), n'échoue jamais le boot (une panne du backfill est loggée, le serveur démarre quand même, retente au prochain start). - Test `PATCH /recipes/:id` : une température de clause survit à l'édition. - `reset-db.ts` : `backfill_state` ajouté au TRUNCATE. Vérifié : `tsc --noEmit` + `pnpm --filter api build` + `pnpm lint` OK ; migration = SQL généré par Prisma pour le modèle, à l'identique. Mocha `recipe.test.ts` non lancé (Postgres + intent-service requis, indisponibles ici) — à jouer en CI / sur la machine de dev. Co-Authored-By: Claude Sonnet 5 --- apps/api/Dockerfile | 13 ++-- .../migration.sql | 11 +++ apps/api/prisma/schema.prisma | 19 +++++ .../recipe-matching/tech-step-persistence.ts | 64 +++++++++++++++++ apps/api/src/modules/recipe/recipe.service.ts | 59 ++++----------- apps/api/src/scripts/backfill-runtime.ts | 72 +++++++++++++++++++ apps/api/src/scripts/backfill-tech-steps.ts | 45 ++++++------ apps/api/test-support/reset-db.ts | 2 +- apps/api/test/recipe.test.ts | 51 +++++++++++++ 9 files changed, 264 insertions(+), 72 deletions(-) create mode 100644 apps/api/prisma/migrations/20260904120000_backfill_state/migration.sql create mode 100644 apps/api/src/lib/recipe-matching/tech-step-persistence.ts create mode 100644 apps/api/src/scripts/backfill-runtime.ts diff --git a/apps/api/Dockerfile b/apps/api/Dockerfile index 1a31a6b..6d0c1f7 100644 --- a/apps/api/Dockerfile +++ b/apps/api/Dockerfile @@ -60,8 +60,11 @@ COPY --from=build /repo/apps/web/dist ./apps/web/dist WORKDIR /repo/apps/api EXPOSE 3000 -# Applies pending migrations, then seeds the reference data (Diet/Category/ -# Allergy — see src/scripts/seed-runtime.ts) before starting. Both steps are -# safe to repeat on every container start: migrate deploy only applies -# pending migrations, and the seed upserts by unique name. -CMD ["sh", "-c", "node_modules/.bin/prisma migrate deploy && node dist/scripts/seed-runtime.js && node dist/server.js"] +# Applies pending migrations, seeds the reference data (Diet/Category/ +# Allergy — see src/scripts/seed-runtime.ts), then runs any pending one-time +# data backfill (src/scripts/backfill-runtime.ts) before starting. migrate +# deploy only applies pending migrations and the seed upserts by unique +# name, so both are safe to repeat every start; the backfill step gates +# itself on a `backfill_state` row (no-op once done) and is written never to +# fail the boot — see its doc comment. +CMD ["sh", "-c", "node_modules/.bin/prisma migrate deploy && node dist/scripts/seed-runtime.js && node dist/scripts/backfill-runtime.js && node dist/server.js"] diff --git a/apps/api/prisma/migrations/20260904120000_backfill_state/migration.sql b/apps/api/prisma/migrations/20260904120000_backfill_state/migration.sql new file mode 100644 index 0000000..0a9b94e --- /dev/null +++ b/apps/api/prisma/migrations/20260904120000_backfill_state/migration.sql @@ -0,0 +1,11 @@ +-- CreateTable: idempotency ledger for one-time data backfills run from the +-- production image's start command (apps/api/Dockerfile CMD -> +-- src/scripts/backfill-runtime.ts). One row per completed backfill, keyed +-- by an opaque backfill name; presence of the row is the "already done, +-- don't repeat" signal. See the `BackfillState` model doc comment. +CREATE TABLE "backfill_state" ( + "key" TEXT NOT NULL, + "completed_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "backfill_state_pkey" PRIMARY KEY ("key") +); diff --git a/apps/api/prisma/schema.prisma b/apps/api/prisma/schema.prisma index 01c87e0..bf18b9e 100644 --- a/apps/api/prisma/schema.prisma +++ b/apps/api/prisma/schema.prisma @@ -1064,3 +1064,22 @@ model WorkerHeartbeat { @@map("worker_heartbeats") } + +/// Idempotency ledger for one-time data backfills run from the production +/// image's start command (`src/scripts/backfill-runtime.ts`, invoked in +/// apps/api/Dockerfile's CMD after `prisma migrate deploy` + `seed-runtime`). +/// +/// The CMD re-runs on every container start, but a backfill here *rewrites* +/// existing rows (unlike `seed-runtime`, which only upserts and is safe to +/// repeat) — e.g. `step_tech_step_metadata_20260904` recomputes every +/// `StepTechStep` sequence against the intent service. So each backfill +/// gates itself on a row in this table: `key` present → already done, skip; +/// absent → run it, then insert the row. A schema migration only ever +/// creates the table structure; it can't run TypeScript against the intent +/// service, which is why this is a start-command step and not a migration. +model BackfillState { + key String @id + completedAt DateTime @default(now()) @map("completed_at") + + @@map("backfill_state") +} diff --git a/apps/api/src/lib/recipe-matching/tech-step-persistence.ts b/apps/api/src/lib/recipe-matching/tech-step-persistence.ts new file mode 100644 index 0000000..5f56c5a --- /dev/null +++ b/apps/api/src/lib/recipe-matching/tech-step-persistence.ts @@ -0,0 +1,64 @@ +import type { Prisma } from "@prisma/client"; +import type { TechStepMatch } from "./tech-step-matcher.js"; + +/** + * Turns one step's {@link TechStepMatch}es into the Prisma nested-`create` + * payload for its `techSteps` relation — the technique row **and** all three + * of its clause-metadata child sets (`ingredients` / `utensils` / + * `temperatures`). + * + * The single source of truth for that shape. It used to be written out by + * hand in three places — `recipe.service.ts`'s `createRecipeInternal` and + * `updateRecipe`, plus `src/scripts/backfill-tech-steps.ts` — and they had + * drifted: only the create path wrote the metadata children, so **every + * recipe resaved through the UI, and every recipe the backfill touched, + * silently lost every ingredient / utensil / temperature the classifier had + * attributed to its steps**. That left `cooking-optimizer.ts`'s + * temperature-aware `preheat` grouping (the "don't preheat to 150 °C and + * 200 °C at once" note) with no data to work from on any recipe that wasn't + * a pristine, never-edited import. Building the payload once, here, is what + * stops the three call sites diverging again. + * + * `order` is each match's index in `matches` (0-based). Callers pass matches + * in reading order already; a caller that needs a different base (the + * backfill, which offsets past untouched `"manual"` rows) overrides `order` + * on the returned objects before handing them to Prisma. + */ +export function toStepTechStepCreateInputs( + matches: TechStepMatch[], +): Prisma.StepTechStepUncheckedCreateWithoutStepInput[] { + return matches.map((match, order) => ({ + techStepId: match.techStepId, + start: match.start, + end: match.end, + contextStart: match.contextStart, + contextEnd: match.contextEnd, + order, + ingredients: { + create: match.ingredients.map((ingredient) => ({ + ingredientId: ingredient.ingredientId, + quantity: ingredient.quantity, + unitId: ingredient.unitId, + start: ingredient.start, + end: ingredient.end, + })), + }, + utensils: { + create: match.utensils.map((utensil) => ({ + utensilId: utensil.utensilId, + start: utensil.start, + end: utensil.end, + })), + }, + temperatures: { + create: match.temperatures.map((temperature) => ({ + start: temperature.start, + end: temperature.end, + celsius: temperature.celsius, + gasMark: temperature.gasMark, + qualitative: temperature.qualitative, + raw: temperature.raw, + })), + }, + })); +} diff --git a/apps/api/src/modules/recipe/recipe.service.ts b/apps/api/src/modules/recipe/recipe.service.ts index fcea207..92440ea 100644 --- a/apps/api/src/modules/recipe/recipe.service.ts +++ b/apps/api/src/modules/recipe/recipe.service.ts @@ -20,6 +20,7 @@ import { type TechStepMatch, techStepClassifier, } from "../../lib/recipe-matching/tech-step-matcher.js"; +import { toStepTechStepCreateInputs } from "../../lib/recipe-matching/tech-step-persistence.js"; // No user-language preference exists anywhere in the app yet (a single // "fr" translation file, no locale field on User/UserProfile) — steps are @@ -694,42 +695,11 @@ async function createRecipeInternal( description: step.description, picture: step.picture ?? null, order: index, - techSteps: { - create: matches.map((match, order) => ({ - techStepId: match.techStepId, - start: match.start, - end: match.end, - contextStart: match.contextStart, - contextEnd: match.contextEnd, - order, - ingredients: { - create: match.ingredients.map((ingredient) => ({ - ingredientId: ingredient.ingredientId, - quantity: ingredient.quantity, - unitId: ingredient.unitId, - start: ingredient.start, - end: ingredient.end, - })), - }, - utensils: { - create: match.utensils.map((utensil) => ({ - utensilId: utensil.utensilId, - start: utensil.start, - end: utensil.end, - })), - }, - temperatures: { - create: match.temperatures.map((temperature) => ({ - start: temperature.start, - end: temperature.end, - celsius: temperature.celsius, - gasMark: temperature.gasMark, - qualitative: temperature.qualitative, - raw: temperature.raw, - })), - }, - })), - }, + // Technique rows + their clause metadata (ingredients / + // utensils / temperatures) — see `toStepTechStepCreateInputs`. + // `updateRecipe` below builds the exact same payload from the + // same helper so a resave never drops what a create persisted. + techSteps: { create: toStepTechStepCreateInputs(matches) }, })), }, diets: { create: input.dietIds.map((dietId) => ({ dietId })) }, @@ -820,16 +790,13 @@ export async function updateRecipe( description: step.description, picture: step.picture ?? null, order: index, - techSteps: { - create: matches.map((match, order) => ({ - techStepId: match.techStepId, - start: match.start, - end: match.end, - contextStart: match.contextStart, - contextEnd: match.contextEnd, - order, - })), - }, + // Same builder as `createRecipeInternal` — a resave must + // persist the clause metadata (ingredients / utensils / + // temperatures), not just the technique span. Omitting it + // here is what left every edited recipe with temperature-less + // `preheat` steps, so `cooking-optimizer.ts` could never spot + // an oven-temperature conflict between two of them. + techSteps: { create: toStepTechStepCreateInputs(matches) }, })), }, diets: { create: input.dietIds.map((dietId) => ({ dietId })) }, diff --git a/apps/api/src/scripts/backfill-runtime.ts b/apps/api/src/scripts/backfill-runtime.ts new file mode 100644 index 0000000..708e11d --- /dev/null +++ b/apps/api/src/scripts/backfill-runtime.ts @@ -0,0 +1,72 @@ +import { prisma } from "../db/prisma.js"; +import { backfillTechSteps } from "./backfill-tech-steps.js"; + +/** + * `backfill_state.key` for the tech-step clause-metadata backfill. Opaque, + * dated so a future metadata rework can add a second key rather than + * re-using this one. + * + * Why it's needed: `updateRecipe` and `backfill-tech-steps.ts` used to + * persist only a detected technique's span, never its ingredient / utensil / + * temperature children (only `createRecipeInternal` did). So every recipe + * ever edited through the UI, and every recipe imported before the create + * path learned temperatures, is missing the clause metadata that + * `cooking-optimizer.ts` needs to spot an oven-temperature conflict between + * two `preheat` steps. {@link backfillTechSteps} recomputes the whole + * `StepTechStep` sequence with the now-complete writer. + */ +const TECH_STEP_METADATA_BACKFILL_KEY = "step_tech_step_metadata_20260904"; + +/** + * One-time data backfills for the production image — run via + * `node dist/scripts/backfill-runtime.js` in apps/api/Dockerfile's CMD, + * after `seed-runtime` and before the server starts. + * + * Unlike `seed-runtime` (idempotent by construction — it only upserts, so + * repeating it every container start is free), a backfill here *rewrites* + * existing rows and would be wasteful-to-harmful to repeat: the tech-step + * one re-hits `services/tech-step-intent-service` once per `Step`. So each + * backfill gates itself on a `backfill_state` row — present means "done, + * skip"; absent means "run it, then record it". + * + * A backfill failing must **not** keep the API down: it's a data-quality + * catch-up, not a correctness prerequisite (contrast `seed-runtime`, whose + * failure genuinely means missing reference data). So a failure here is + * logged, the marker is left unwritten (the next container start retries), + * and the process still exits 0 so the CMD chain reaches `node dist/server.js`. + * Only a failure to even read `backfill_state` propagates — at that point + * the database is unreachable and the server wouldn't start anyway. + */ +async function runOneTimeBackfills(): Promise { + const alreadyDone = await prisma.backfillState.findUnique({ + where: { key: TECH_STEP_METADATA_BACKFILL_KEY }, + }); + if (alreadyDone !== null) { + console.info( + `Backfill "${TECH_STEP_METADATA_BACKFILL_KEY}" already completed at ${alreadyDone.completedAt.toISOString()} — skipping.`, + ); + return; + } + + console.info(`Running one-time backfill "${TECH_STEP_METADATA_BACKFILL_KEY}"...`); + try { + const { total, changed } = await backfillTechSteps(); + await prisma.backfillState.create({ data: { key: TECH_STEP_METADATA_BACKFILL_KEY } }); + console.info( + `Backfill "${TECH_STEP_METADATA_BACKFILL_KEY}" done — ${changed}/${total} step(s) recomputed.`, + ); + } catch (err) { + console.error( + `Backfill "${TECH_STEP_METADATA_BACKFILL_KEY}" failed — leaving it unrecorded so the next start retries. The API will still start.`, + err, + ); + } +} + +runOneTimeBackfills() + .then(() => prisma.$disconnect()) + .catch(async (err) => { + console.error(err); + await prisma.$disconnect(); + process.exit(1); + }); diff --git a/apps/api/src/scripts/backfill-tech-steps.ts b/apps/api/src/scripts/backfill-tech-steps.ts index 45753aa..b9700ff 100644 --- a/apps/api/src/scripts/backfill-tech-steps.ts +++ b/apps/api/src/scripts/backfill-tech-steps.ts @@ -1,23 +1,27 @@ import { pathToFileURL } from "node:url"; import { prisma } from "../db/prisma.js"; import { techStepClassifier } from "../lib/recipe-matching/tech-step-matcher.js"; +import { toStepTechStepCreateInputs } from "../lib/recipe-matching/tech-step-persistence.js"; import { renumberStepTechSteps } from "../modules/recipe/recipe-tech-step-correction.service.js"; /** * Recomputes every existing `Step`'s `"auto"`-sourced `StepTechStep` - * entries against the *current* classifier + * entries — technique spans **and** their clause metadata (ingredients, + * utensils, temperatures) — against the *current* classifier * (`tech-step-matcher.ts`, delegating to `services/tech-step-intent-service`), - * the same way `updateRecipe` does when a user resaves a recipe through the UI — - * always `"fr"` (`DEFAULT_TECH_STEP_LOCALE` in `recipe.service.ts`; there's - * no persisted per-recipe locale to recover for a step that already - * exists, so this matches real resave behavior exactly rather than - * guessing). + * the same way `updateRecipe` does when a user resaves a recipe through the + * UI: both build the row payload from the shared + * {@link toStepTechStepCreateInputs}. Always `"fr"` + * (`DEFAULT_TECH_STEP_LOCALE` in `recipe.service.ts`; there's no persisted + * per-recipe locale to recover for a step that already exists, so this + * matches real resave behavior exactly rather than guessing). * * Needed because tech-step detection only ever runs at create/update time * (`recipe.service.ts`'s `matchStepsTechSteps`), never retroactively — a * step saved before a classifier/corpus change (new vocabulary, or the - * `contextStart`/`contextEnd` columns a previous session added) keeps - * whatever it was matched with at the time until it's next resaved. + * `contextStart`/`contextEnd` columns a previous session added), or before + * the clause-metadata writers were wired into the *update* path at all, + * keeps whatever it was matched with at the time until it's next resaved. * * `"manual"`-sourced entries (a viewer's correction, applied immediately — * see `recipe-tech-step-correction.service.ts`'s `applyManualCorrection`) @@ -72,18 +76,19 @@ export async function backfillTechSteps(): Promise<{ total: number; changed: num // into a clean 0..N-1 sequence right after — these just need to // not collide with `@@id([stepId, order])` for this insert). const startOrder = manualRows.reduce((max, row) => Math.max(max, row.order), -1) + 1; - await tx.stepTechStep.createMany({ - data: nonOverlappingMatches.map((match, index) => ({ - stepId: step.id, - techStepId: match.techStepId, - order: startOrder + index, - start: match.start, - end: match.end, - contextStart: match.contextStart, - contextEnd: match.contextEnd, - source: "auto", - })), - }); + // One `create` per row rather than a single `createMany`: + // `createMany` cannot write the nested `ingredients` / `utensils` / + // `temperatures` children, and silently dropping those is exactly + // the bug this script shared with the old `updateRecipe` — see + // `toStepTechStepCreateInputs`. A backfill that leaves recipes + // without their clause temperatures defeats the whole point of + // running it (`cooking-optimizer.ts` needs them). + const inputs = toStepTechStepCreateInputs(nonOverlappingMatches); + for (const [index, input] of inputs.entries()) { + await tx.stepTechStep.create({ + data: { ...input, stepId: step.id, order: startOrder + index, source: "auto" }, + }); + } } await renumberStepTechSteps(tx, step.id); diff --git a/apps/api/test-support/reset-db.ts b/apps/api/test-support/reset-db.ts index c543730..f283a32 100644 --- a/apps/api/test-support/reset-db.ts +++ b/apps/api/test-support/reset-db.ts @@ -48,7 +48,7 @@ export async function resetDatabase() { "recipe_ingredient", "step_tech_step", "step", "tech_step", "recipe", "ingredients", "sources", "unit", "user_profiles", "diet", "house", - "admin_users", "analytics_events", "worker_heartbeats" + "admin_users", "analytics_events", "worker_heartbeats", "backfill_state" RESTART IDENTITY CASCADE; `); await seedReferenceData(prisma); diff --git a/apps/api/test/recipe.test.ts b/apps/api/test/recipe.test.ts index 472833f..d4e50ec 100644 --- a/apps/api/test/recipe.test.ts +++ b/apps/api/test/recipe.test.ts @@ -883,6 +883,57 @@ describe("Recipes", () => { expect(orphanedTechSteps).to.deep.equal([]); }); + it("carries a replaced step's clause temperatures through the edit, not just its technique span", async () => { + const { agent } = await signup(); + const tomate = await ingredientId("tomato"); + const piece = await unitId("piece"); + const created = await agent.post("/recipes").send({ + name: "Gratin", + portions: 4, + dietIds: [], + ingredients: [{ ingredientId: tomate, quantity: 1, unitId: piece }], + steps: [{ description: "Servir immédiatement" }], + }); + + // The pre-fix bug: `updateRecipe` persisted only each detected + // technique's span on a resave and silently dropped every + // temperature / utensil / ingredient the classifier attributed to the + // clause (only the *create* path wrote them). A recipe edited even + // once therefore had temperature-less `preheat`/`bake` steps, so + // `cooking-optimizer.ts` could never flag an oven-temperature + // conflict between two of them. Same assertion as the POST test + // "extracts a step's temperature mentions...", but reached via PATCH. + const editedDescription = "Enfourner à 180°C puis finir à feu doux"; + const res = await agent.patch(`/recipes/${created.body.id}`).send({ + name: "Gratin", + portions: 4, + dietIds: [], + ingredients: [{ ingredientId: tomate, quantity: 1, unitId: piece }], + steps: [{ description: editedDescription }], + }); + + expect(res.status).to.equal(200); + expect(res.body.steps[0].techSteps.length).to.be.greaterThan(0); + const temperatures = res.body.steps[0].techSteps.flatMap( + (techStep: { temperatures: unknown[] }) => techStep.temperatures, + ); + const celsius = temperatures.find( + (temperature: { celsius: number | null }) => temperature.celsius !== null, + ); + expect(celsius).to.include({ + celsius: 180, + gasMark: null, + qualitative: null, + source: "auto", + }); + expect(editedDescription.slice(celsius.start, celsius.end)).to.equal("180°C"); + expect( + temperatures.some( + (temperature: { qualitative: string | null }) => temperature.qualitative === "low", + ), + ).to.equal(true); + }); + it("returns 404 RECIPE_NOT_FOUND for an unknown id", async () => { const { agent } = await signup(); const tomate = await ingredientId("tomato"); -- 2.45.2