From abc5b0a0e9921b9ad09537b6d395dca5845e976b Mon Sep 17 00:00:00 2001 From: Nicolas Date: Fri, 21 Aug 2026 07:55:00 +0200 Subject: [PATCH] fix(recipes): corrige plusieurs bugs d'import TheMealDB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Les instructions TheMealDB numérotées sur leur propre ligne ("1\n\ntexte...\n\n2\n\ntexte...") créaient des étapes parasites ne contenant qu'un chiffre — filtrées désormais (#52). - Un ingrédient compté sans mot d'unité dans le texte source (ex. "4 Egg Yolks") laissait l'import bloqué sur "Importer" indéfiniment, sans indication visuelle de la ligne en cause — matchUnit retombe maintenant sur l'unité générique "piece" quand une quantité a été extraite, et RecipeImportForm/RecipeFormPage surlignent désormais toute ligne dont l'unité manque, avec un message explicite (#53). - Ajout de INGREDIENT_LABEL_SYNONYMS_EN pour reconnaître des formulations alternatives fréquentes chez les sources anglophones ("vanilla pod" en plus de "vanilla bean") sans élargir INGREDIENT_LABELS_EN à un tableau pour ses ~550 entrées (#54). - Effet de bord découvert en vérifiant #53 de bout en bout : deux lignes source résolues vers le même ingrédient catalogue (ex. "Egg Yolks"/"Eggs" -> "Œuf") faisaient planter la création en 500 (contrainte unique recipe_id+ingredient_id) au lieu d'un 400 propre. createRecipeSchema rejette maintenant les ingredientId en double, et le formulaire d'import surligne les doublons avant même de soumettre. Vérifié de bout en bout dans le navigateur (import réel de la recette "Flan" depuis TheMealDB, jusqu'au planning) en plus des tests ajoutés. Closes #52, #53, #54 Co-Authored-By: Claude Sonnet 5 --- apps/api/src/lib/ingredient-matcher.ts | 14 ++++-- apps/api/src/lib/recipe-translation.ts | 23 ++++++++- apps/api/src/sources/the-meal-db.ts | 7 +++ apps/api/test/ingredient-matcher.test.ts | 31 ++++++++++-- apps/api/test/recipe-translation.test.ts | 26 ++++++++++ apps/api/test/recipe.test.ts | 26 ++++++++++ apps/api/test/sources.test.ts | 17 +++++++ apps/api/test/the-meal-db.test.ts | 14 ++++++ .../src/features/recipes/IngredientRow.tsx | 22 ++++++++- .../src/features/recipes/RecipeImportForm.tsx | 33 +++++++++++++ apps/web/src/features/recipes/recipes.scss | 14 ++++++ apps/web/src/locales/fr/translation.json | 4 ++ apps/web/src/pages/RecipeFormPage.tsx | 10 ++++ packages/shared/src/data/catalog-labels-en.ts | 18 +++++++ packages/shared/src/schemas/recipe.ts | 48 ++++++++++++++----- 15 files changed, 287 insertions(+), 20 deletions(-) diff --git a/apps/api/src/lib/ingredient-matcher.ts b/apps/api/src/lib/ingredient-matcher.ts index 467f13e..0ba0c3c 100644 --- a/apps/api/src/lib/ingredient-matcher.ts +++ b/apps/api/src/lib/ingredient-matcher.ts @@ -1,4 +1,8 @@ -import { INGREDIENT_LABELS_EN, UNIT_LABELS_EN } from "@batch-cooking/shared"; +import { + INGREDIENT_LABELS_EN, + INGREDIENT_LABEL_SYNONYMS_EN, + UNIT_LABELS_EN, +} from "@batch-cooking/shared"; import { prisma } from "../db/prisma.js"; import { normalizeText } from "./tech-step-matcher.js"; @@ -168,13 +172,17 @@ export function extractQuantity(rawText: string): ExtractedQuantity { return { quantity, remainder: trimmed.slice(match[0].length).trim() }; } -/** Loads the full `Ingredient` catalog as {@link IngredientMatchEntry}s — one entry per key with an authored English label (see `INGREDIENT_LABELS_EN`); an ingredient with no English label yet is silently skipped, never a matching target. Meant to be fetched once per request and reused across every ingredient line, not re-queried per line. */ +/** Loads the full `Ingredient` catalog as {@link IngredientMatchEntry}s — one entry per key with an authored English label (see `INGREDIENT_LABELS_EN`), plus one extra entry per alternate wording (`INGREDIENT_LABEL_SYNONYMS_EN`, e.g. "Vanilla pod" alongside "Vanilla bean" — see issue #54) sharing the same `ingredientId`; `matchIngredientName` doesn't need to know synonyms exist, it just sees more candidate labels for the same ingredient. An ingredient with no English label yet is silently skipped, never a matching target. Meant to be fetched once per request and reused across every ingredient line, not re-queried per line. */ export async function loadIngredientCatalog(): Promise { const ingredients = await prisma.ingredient.findMany({ select: { id: true, key: true } }); const catalog: IngredientMatchEntry[] = []; for (const ingredient of ingredients) { const label = INGREDIENT_LABELS_EN[ingredient.key]; - if (label !== undefined) catalog.push({ ingredientId: ingredient.id, label }); + if (label === undefined) continue; + catalog.push({ ingredientId: ingredient.id, label }); + for (const synonym of INGREDIENT_LABEL_SYNONYMS_EN[ingredient.key] ?? []) { + catalog.push({ ingredientId: ingredient.id, label: synonym }); + } } return catalog; } diff --git a/apps/api/src/lib/recipe-translation.ts b/apps/api/src/lib/recipe-translation.ts index b93c493..1dd97c1 100644 --- a/apps/api/src/lib/recipe-translation.ts +++ b/apps/api/src/lib/recipe-translation.ts @@ -92,6 +92,14 @@ export function translateRecipeSteps( }; } +/** + * English label {@link matchUnit} is fed when a quantity was found but no + * unit word was — see the `unitId` fallback below. `"piece"` (`UNIT_LABELS_EN`, + * `packages/shared`) is the catalog's generic "counted, no further unit" + * entry (French "unité"). + */ +const FALLBACK_COUNT_UNIT_LABEL = "piece"; + /** * Resolves each of `ingredients`' free-text `name`/`unit`/`quantity` * against `ingredientCatalog`/`unitCatalog` (see `ingredient-matcher.ts`). @@ -101,6 +109,17 @@ export function translateRecipeSteps( * same for `unit` falling back to `extractQuantity`'s `remainder` before * being matched against `unitCatalog` — a source that already states a * clean unit/quantity is trusted over re-deriving it from `rawText`. + * + * When a quantity was found but nothing in the remaining text matched a + * unit (e.g. `"4 Egg Yolks"` — quantity `4`, remainder `"Egg Yolks"`, no + * unit word anywhere in it), `unitId` falls back to the catalog's generic + * `piece` unit rather than staying `null`: a bare count with no explicit + * unit word is overwhelmingly "N of them" (eggs, onions, cloves not + * spelled out as "clove") in practice, not a genuinely missing unit — see + * issue #53, where this previously left the import review form's submit + * button disabled with no indication why on almost any recipe with a + * whole-item ingredient. No fallback when `quantity` itself is `null` + * (e.g. `"To taste"`) — there's nothing to count, so nothing to default. */ export function translateRecipeIngredients( ingredients: ParsedRecipeIngredient[], @@ -112,7 +131,9 @@ export function translateRecipeIngredients( const extracted = extractQuantity(ingredient.rawText); const quantity = ingredient.quantity ?? extracted.quantity; const unitText = ingredient.unit ?? extracted.remainder; - const unitId = matchUnit(unitText, unitCatalog); + const unitId = + matchUnit(unitText, unitCatalog) ?? + (quantity !== null ? matchUnit(FALLBACK_COUNT_UNIT_LABEL, unitCatalog) : null); return { ...ingredient, quantity, ingredientId, unitId }; }); } diff --git a/apps/api/src/sources/the-meal-db.ts b/apps/api/src/sources/the-meal-db.ts index 4789c4f..d4841d2 100644 --- a/apps/api/src/sources/the-meal-db.ts +++ b/apps/api/src/sources/the-meal-db.ts @@ -129,10 +129,17 @@ export const theMealDbAdapter: RecipeSourceAdapter = { // Free-text instructions, usually one step per line — splitting on // blank/newlines is the closest this source gets to discrete steps. + // TheMealDB frequently numbers each step on its own line ahead of the + // paragraph that follows (e.g. "…melted.\n\n2\n\nPreheat oven…") rather + // than inline ("1. Preheat oven…") — the blank-line split above turns + // that lone number into its own "line", which would otherwise become a + // bogus step containing nothing but a digit. Drop those rather than + // keep them as steps in their own right (see issue #52). const steps = (meal.strInstructions ?? "") .split(/\r?\n+/) .map((line) => line.trim()) .filter((line) => line.length > 0) + .filter((line) => !/^\d+\.?$/.test(line)) .map((description) => ({ description, picture: null })); if (steps.length === 0) { throw new RecipeSourceParseError( diff --git a/apps/api/test/ingredient-matcher.test.ts b/apps/api/test/ingredient-matcher.test.ts index f50858e..072d0fc 100644 --- a/apps/api/test/ingredient-matcher.test.ts +++ b/apps/api/test/ingredient-matcher.test.ts @@ -1,3 +1,4 @@ +import { INGREDIENT_LABEL_SYNONYMS_EN } from "@batch-cooking/shared"; import { expect } from "chai"; import { prisma } from "../src/db/prisma.js"; import { @@ -75,6 +76,15 @@ describe("ingredient-matcher", () => { expect(matchIngredientName("", catalog)).to.equal(null); }); + it("matches an alternate wording of the same ingredient via a second catalog entry sharing its ingredientId (issue #54)", () => { + const vanillaBean: IngredientMatchEntry = { ingredientId: 8, label: "Vanilla bean" }; + const vanillaBeanSynonym: IngredientMatchEntry = { ingredientId: 8, label: "Vanilla pod" }; + const synonymCatalog = [vanillaBean, vanillaBeanSynonym]; + + expect(matchIngredientName("1 vanilla pod", synonymCatalog)).to.equal(8); + expect(matchIngredientName("1 vanilla bean", synonymCatalog)).to.equal(8); + }); + it("breaks a same-specificity tie by the lowest ingredientId", () => { const onionA: IngredientMatchEntry = { ingredientId: 20, label: "Onion" }; const onionB: IngredientMatchEntry = { ingredientId: 21, label: "Onion" }; @@ -190,18 +200,33 @@ describe("ingredient-matcher", () => { await prisma.$disconnect(); }); - it("loads one entry per Ingredient that has an English label, keyed by real ingredientId", async () => { + it("loads one entry per Ingredient that has an English label, plus one per alternate wording (INGREDIENT_LABEL_SYNONYMS_EN), keyed by real ingredientId", async () => { const tomato = await prisma.ingredient.findFirstOrThrow({ where: { key: "tomato" } }); + const vanillaBean = await prisma.ingredient.findFirstOrThrow({ + where: { key: "vanillaBean" }, + }); const ingredientCount = await prisma.ingredient.count(); + const synonymCount = Object.values(INGREDIENT_LABEL_SYNONYMS_EN).reduce( + (sum, synonyms) => sum + synonyms.length, + 0, + ); const catalog = await loadIngredientCatalog(); // Every seeded ingredient has an authored English label (verified at // generation time — see packages/shared/src/data/catalog-labels-en.ts), - // so nothing should be silently skipped. - expect(catalog).to.have.length(ingredientCount); + // so nothing should be silently skipped — plus one extra entry per + // synonym (issue #54), sharing the same ingredientId as the primary + // label's entry. + expect(catalog).to.have.length(ingredientCount + synonymCount); const tomatoEntry = catalog.find((entry) => entry.ingredientId === tomato.id); expect(tomatoEntry?.label).to.equal("Tomato"); + + const vanillaBeanEntries = catalog.filter((entry) => entry.ingredientId === vanillaBean.id); + expect(vanillaBeanEntries.map((entry) => entry.label)).to.deep.equal([ + "Vanilla bean", + "Vanilla pod", + ]); }); it("loads one entry per Unit that has English synonyms, keyed by real unitId", async () => { diff --git a/apps/api/test/recipe-translation.test.ts b/apps/api/test/recipe-translation.test.ts index 636fad8..52205ab 100644 --- a/apps/api/test/recipe-translation.test.ts +++ b/apps/api/test/recipe-translation.test.ts @@ -196,6 +196,32 @@ describe("recipe-translation", () => { expect(translated[0].unitId).to.equal(gram.unitId); }); + it("falls back to the generic 'piece' unit when a quantity was found but no unit word was (issue #53)", () => { + const piece: UnitMatchEntry = { unitId: 12, synonyms: ["piece", "pieces", "pc", "pcs"] }; + + const translated = translateRecipeIngredients( + [buildIngredient({ rawText: "4 Egg Yolks", name: "Egg Yolks" })], + [], + [gram, cup, piece], + ); + + expect(translated[0].quantity).to.equal(4); + expect(translated[0].unitId).to.equal(piece.unitId); + }); + + it("does not fall back to 'piece' when there's no quantity at all to count", () => { + const piece: UnitMatchEntry = { unitId: 12, synonyms: ["piece", "pieces", "pc", "pcs"] }; + + const translated = translateRecipeIngredients( + [buildIngredient({ rawText: "salt to taste", name: "salt" })], + [], + [piece], + ); + + expect(translated[0].quantity).to.equal(null); + expect(translated[0].unitId).to.equal(null); + }); + it("leaves quantity/unitId null when rawText has neither a leading number nor a recognizable unit", () => { const translated = translateRecipeIngredients( [buildIngredient({ rawText: "salt to taste", name: "salt" })], diff --git a/apps/api/test/recipe.test.ts b/apps/api/test/recipe.test.ts index d35c720..c78c3f5 100644 --- a/apps/api/test/recipe.test.ts +++ b/apps/api/test/recipe.test.ts @@ -568,6 +568,32 @@ describe("Recipes", () => { expect(res.body.code).to.equal(ErrorCode.VALIDATION_ERROR); }); + it("rejects the same ingredientId listed twice with 400 VALIDATION_ERROR, not a 500 (issue #53 follow-up)", async () => { + // `RecipeIngredient`'s primary key is `(recipeId, ingredientId)` — a + // manual creation can't reach this via the web UI (`IngredientPicker` + // hides an already-picked ingredient), but nothing stops a raw + // request (or a source import, whose lines aren't deduplicated) from + // sending it — must fail cleanly instead of crashing on the DB's + // unique-constraint violation. + const { agent } = await signup(); + const tomate = await ingredientId("tomato"); + const piece = await unitId("piece"); + + const res = await agent.post("/recipes").send({ + name: "Test", + portions: 4, + dietIds: [], + ingredients: [ + { ingredientId: tomate, quantity: 1, unitId: piece }, + { ingredientId: tomate, quantity: 2, unitId: piece }, + ], + steps: [{ description: "Étape" }], + }); + + expect(res.status).to.equal(400); + expect(res.body.code).to.equal(ErrorCode.VALIDATION_ERROR); + }); + it("rejects a missing or non-positive portions with 400 VALIDATION_ERROR", async () => { const { agent } = await signup(); const tomate = await ingredientId("tomato"); diff --git a/apps/api/test/sources.test.ts b/apps/api/test/sources.test.ts index 9107128..5441943 100644 --- a/apps/api/test/sources.test.ts +++ b/apps/api/test/sources.test.ts @@ -357,6 +357,23 @@ describe("Sources", () => { expect(res.body.code).to.equal(ErrorCode.INGREDIENT_NOT_FOUND); }); + it("rejects the same ingredientId listed twice with 400 VALIDATION_ERROR, not a 500 (issue #53 follow-up)", async () => { + // A source's raw ingredient lines aren't deduplicated by the matcher + // (see `ingredient-matcher.ts`) — two different lines (e.g. "Egg + // Yolks" and "Eggs") can resolve to the same catalog ingredient, same + // as `recipe.test.ts`'s equivalent for a manual creation, just + // reached here through the review screen's pre-filled payload + // instead. + const { agent } = await enableFakeSource(); + const payload = await buildImportPayload(); + payload.ingredients.push({ ...payload.ingredients[0] }); + + const res = await agent.post("/sources/fakeSource/import/1").send(payload); + + expect(res.status).to.equal(400); + expect(res.body.code).to.equal(ErrorCode.VALIDATION_ERROR); + }); + it("rejects a second household's import of the same item too — the item's identity is global, not per-household", async () => { // Registers/syncs the adapter once — enableFakeSource() itself does // this too, and registerRecipeSource() throws on a duplicate key, so diff --git a/apps/api/test/the-meal-db.test.ts b/apps/api/test/the-meal-db.test.ts index 7427146..e0e0d99 100644 --- a/apps/api/test/the-meal-db.test.ts +++ b/apps/api/test/the-meal-db.test.ts @@ -154,6 +154,20 @@ describe("theMealDbAdapter", () => { ]); }); + it("drops lone step-number lines instead of turning them into bogus steps (issue #52)", () => { + const parsed = theMealDbAdapter.parse({ + ...baseMeal, + strInstructions: + "For the caramel, melt the sugar.\r\n\r\n2\r\n\r\nPreheat the oven.\r\n\r\n3\r\n\r\nBake it.", + }); + + expect(parsed.steps).to.deep.equal([ + { description: "For the caramel, melt the sugar.", picture: null }, + { description: "Preheat the oven.", picture: null }, + { description: "Bake it.", picture: null }, + ]); + }); + it("throws RecipeSourceParseError when the meal has no name", () => { expect(() => theMealDbAdapter.parse({ ...baseMeal, strMeal: null })).to.throw( RecipeSourceParseError, diff --git a/apps/web/src/features/recipes/IngredientRow.tsx b/apps/web/src/features/recipes/IngredientRow.tsx index 5fc47e1..ea2c360 100644 --- a/apps/web/src/features/recipes/IngredientRow.tsx +++ b/apps/web/src/features/recipes/IngredientRow.tsx @@ -12,6 +12,7 @@ export function IngredientRow({ quantity, unitId, unitsCatalog, + duplicate = false, onQuantityChange, onUnitChange, onRemove, @@ -20,14 +21,24 @@ export function IngredientRow({ quantity: string; unitId: number | null; unitsCatalog: UnitView[]; + /** This ingredient is also picked by another line in the same form — `RecipeIngredient`'s primary key is `(recipeId, ingredientId)`, one row per ingredient (schema.prisma), so submitting two lines for it would fail. Never true for a manually-added line (`IngredientPicker`'s `excludeIds` already prevents picking the same ingredient twice) — only reachable via a source import, whose raw lines aren't deduplicated (e.g. "Egg Yolks" and "Eggs" both resolving to "Egg" — see issue #53's `RecipeImportForm`). */ + duplicate?: boolean; onQuantityChange: (quantity: string) => void; onUnitChange: (unitId: number) => void; onRemove: () => void; }) { const { t } = useTranslation(); + // No unit picked yet — surfaced with a highlighted `