Merge pull request 'fix(recipes): persiste les métadonnées de clause des tech-steps à l'édition' (#26) from fix/recipe-tech-step-metadata-on-update into main

Reviewed-on: #26

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"]

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")
+);

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")
+}

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,
+      })),
+    },
+  }));
+}

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 })) },

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<void> {
+  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);
+  });

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);

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);

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");