fix(recipes): persiste les métadonnées de clause des tech-steps à l'édition
`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 <noreply@anthropic.com>
This commit is contained in:
parent
df0791a694
commit
ec01fc0107
9 changed files with 264 additions and 72 deletions
|
|
@ -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"]
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
);
|
||||
|
|
@ -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")
|
||||
}
|
||||
|
|
|
|||
64
apps/api/src/lib/recipe-matching/tech-step-persistence.ts
Normal file
64
apps/api/src/lib/recipe-matching/tech-step-persistence.ts
Normal file
|
|
@ -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,
|
||||
})),
|
||||
},
|
||||
}));
|
||||
}
|
||||
|
|
@ -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 })) },
|
||||
|
|
|
|||
72
apps/api/src/scripts/backfill-runtime.ts
Normal file
72
apps/api/src/scripts/backfill-runtime.ts
Normal file
|
|
@ -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);
|
||||
});
|
||||
|
|
@ -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,19 +76,20 @@ 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);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
Loading…
Reference in a new issue