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 `