diff --git a/apps/api/src/lib/recipe-translation.ts b/apps/api/src/lib/recipe-translation.ts index 1dd97c1..257f25c 100644 --- a/apps/api/src/lib/recipe-translation.ts +++ b/apps/api/src/lib/recipe-translation.ts @@ -1,3 +1,4 @@ +import type { UnitType } from "@batch-cooking/shared"; import { type IngredientMatchEntry, type UnitMatchEntry, @@ -138,6 +139,111 @@ export function translateRecipeIngredients( }); } +/** What {@link mergeDuplicateIngredients} needs to know about a `Unit` to combine two of them — the `type`/`toBaseFactor` slice of `UnitView` (`packages/shared`). */ +export interface UnitConversionEntry { + id: number; + type: UnitType; + toBaseFactor: number; +} + +/** + * Combines `a`/`b` — two lines already confirmed to resolve to the same + * ingredient — into one, summing their quantities, or returns `null` when + * that can't be done safely. `null` (quantity or unit missing on either + * side, unit not found in `unitById`, mismatched `UnitType`, or either side + * a `COUNT` unit) means "don't merge", not "error" — see + * {@link mergeDuplicateIngredients}. + * + * Same unit on both sides sums directly. Different units of the same + * measurable *type* (`MASS`/`VOLUME`) convert `b`'s quantity into `a`'s + * unit via `toBaseFactor` first — the groundwork that field's own doc + * comment (`UnitView`, `packages/shared`) already anticipated ("a future + * conversion feature ... summing '500g' + '0.5kg'"). `COUNT` units are + * never converted against each other even when `toBaseFactor` matches — a + * "pincée" isn't a fixed fraction of a "gousse" (same doc comment) — so two + * different `COUNT` units for the same ingredient are left unmerged. + * Rounded to 2 decimal places (`RecipeIngredient.quantity` is + * `Decimal(10, 2)`, schema.prisma) to avoid floating-point noise from the + * conversion. + */ +function combineIngredientLines( + a: TranslatedRecipeIngredient, + b: TranslatedRecipeIngredient, + unitById: Map, +): TranslatedRecipeIngredient | null { + if (a.quantity === null || b.quantity === null || a.unitId === null || b.unitId === null) { + return null; + } + if (a.unitId === b.unitId) { + return { ...a, quantity: a.quantity + b.quantity, rawText: `${a.rawText} + ${b.rawText}` }; + } + + const unitA = unitById.get(a.unitId); + const unitB = unitById.get(b.unitId); + if (!unitA || !unitB) return null; + if (unitA.type !== unitB.type || unitA.type === "COUNT") return null; + + const combinedInBaseUnit = a.quantity * unitA.toBaseFactor + b.quantity * unitB.toBaseFactor; + const quantity = Math.round((combinedInBaseUnit / unitA.toBaseFactor) * 100) / 100; + return { ...a, quantity, rawText: `${a.rawText} + ${b.rawText}` }; +} + +/** + * Folds `ingredients` down to one line per resolved `ingredientId`, + * concatenating (summing the quantity of) every duplicate into the first + * line it matches — see issue #53's follow-up: two raw source lines (e.g. + * TheMealDB's "Egg Yolks"/"Eggs", or "100g Sugar" used in two different + * steps) can independently resolve to the same catalog `Ingredient`, and + * `RecipeIngredient`'s primary key (`recipeId`, `ingredientId`) only + * allows one row per ingredient per recipe — the review form used to + * either crash on submit (before `createRecipeSchema` rejected it) or + * require the person to manually delete every extra line by hand. + * + * Unresolved lines (`ingredientId: null`) are never merged with one + * another or with anything else — nothing reliable to key them on. Two + * lines that resolve to the same ingredient but can't be combined safely + * (see {@link combineIngredientLines} — mismatched quantity/unit, or + * genuinely incompatible units) are left as separate, still-duplicate + * lines: `createRecipeSchema` still rejects the result, and + * `RecipeImportForm` still highlights them, same safety net as before this + * merge step existed — merging never *invents* a number it isn't confident + * in. + * + * Pure — testable with a hand-built `unitCatalog`, no database involved. + * Order-preserving: a merged line keeps its first occurrence's position. + */ +export function mergeDuplicateIngredients( + ingredients: TranslatedRecipeIngredient[], + unitCatalog: UnitConversionEntry[], +): TranslatedRecipeIngredient[] { + const unitById = new Map(unitCatalog.map((unit) => [unit.id, unit])); + const merged: TranslatedRecipeIngredient[] = []; + const mergedIndexByIngredientId = new Map(); + + for (const line of ingredients) { + const existingIndex = + line.ingredientId !== null ? mergedIndexByIngredientId.get(line.ingredientId) : undefined; + + const existingLine = existingIndex !== undefined ? merged[existingIndex] : undefined; + if (existingIndex === undefined || existingLine === undefined) { + if (line.ingredientId !== null) { + mergedIndexByIngredientId.set(line.ingredientId, merged.length); + } + merged.push(line); + continue; + } + + const combined = combineIngredientLines(existingLine, line, unitById); + if (combined === null) { + merged.push(line); + } else { + merged[existingIndex] = combined; + } + } + + return merged; +} + /** * Convenience wrapper around {@link translateRecipeSteps}/ * {@link translateRecipeIngredients} that loads every catalog itself — diff --git a/apps/api/src/modules/sources/sources.service.ts b/apps/api/src/modules/sources/sources.service.ts index 0fb533b..1605a46 100644 --- a/apps/api/src/modules/sources/sources.service.ts +++ b/apps/api/src/modules/sources/sources.service.ts @@ -19,7 +19,10 @@ import { import { type RecipeSourceAdapter, markAlreadyImported } from "../../lib/recipe-source-adapter.js"; import { RecipeSourceError } from "../../lib/recipe-source-errors.js"; import { getRecipeSource } from "../../lib/recipe-source-registry.js"; -import { translateRecipeIngredients } from "../../lib/recipe-translation.js"; +import { + mergeDuplicateIngredients, + translateRecipeIngredients, +} from "../../lib/recipe-translation.js"; import { loadTechStepMappingRules, matchTechStepSpans } from "../../lib/tech-step-matcher.js"; import { getHouseSourceIds } from "../house/house.service.js"; import { createImportedRecipe } from "../recipe/recipe.service.js"; @@ -162,7 +165,15 @@ export async function previewSourceItem( const ingredientById = new Map(ingredientViews.map((view) => [view.id, view])); const unitById = new Map(unitViews.map((view) => [view.id, view])); - const ingredients: DraftRecipeIngredientView[] = translatedIngredients.map((ingredient) => ({ + // A source's raw ingredient lines aren't deduplicated by the matcher — + // two different lines (e.g. "Egg Yolks"/"Eggs") can resolve to the same + // catalog ingredient. Folded into one line per ingredient (quantities + // summed where that's safe) before the draft ever reaches the review + // screen, rather than surfacing the recipe with two rows for "Œuf" and + // making the person sort it out — see issue #53's follow-up. + const mergedIngredients = mergeDuplicateIngredients(translatedIngredients, unitViews); + + const ingredients: DraftRecipeIngredientView[] = mergedIngredients.map((ingredient) => ({ rawText: ingredient.rawText, quantity: ingredient.quantity, ingredient: diff --git a/apps/api/test/recipe-translation.test.ts b/apps/api/test/recipe-translation.test.ts index 52205ab..f3d3ffd 100644 --- a/apps/api/test/recipe-translation.test.ts +++ b/apps/api/test/recipe-translation.test.ts @@ -3,6 +3,9 @@ import { prisma } from "../src/db/prisma.js"; import type { IngredientMatchEntry, UnitMatchEntry } from "../src/lib/ingredient-matcher.js"; import type { ParsedRecipe, ParsedRecipeIngredient } from "../src/lib/recipe-source-adapter.js"; import { + type TranslatedRecipeIngredient, + type UnitConversionEntry, + mergeDuplicateIngredients, translateRecipe, translateRecipeIngredients, translateRecipeSteps, @@ -243,6 +246,111 @@ describe("recipe-translation", () => { }); }); + describe("mergeDuplicateIngredients", () => { + const gram: UnitConversionEntry = { id: 1, type: "MASS", toBaseFactor: 1 }; + const kilogram: UnitConversionEntry = { id: 2, type: "MASS", toBaseFactor: 1000 }; + const milliliter: UnitConversionEntry = { id: 3, type: "VOLUME", toBaseFactor: 1 }; + const piece: UnitConversionEntry = { id: 4, type: "COUNT", toBaseFactor: 1 }; + const slice: UnitConversionEntry = { id: 5, type: "COUNT", toBaseFactor: 1 }; + + function buildLine( + overrides: Partial & { rawText: string }, + ): TranslatedRecipeIngredient { + return { + name: overrides.rawText, + quantity: null, + unit: null, + ingredientId: null, + unitId: null, + ...overrides, + }; + } + + it("sums the quantity of two lines resolving to the same ingredient, same unit (issue #53 follow-up)", () => { + const merged = mergeDuplicateIngredients( + [ + buildLine({ rawText: "100g Sugar", ingredientId: 1, quantity: 100, unitId: gram.id }), + buildLine({ rawText: "45g Sugar", ingredientId: 1, quantity: 45, unitId: gram.id }), + ], + [gram, kilogram, milliliter, piece, slice], + ); + + expect(merged).to.have.length(1); + expect(merged[0].quantity).to.equal(145); + expect(merged[0].unitId).to.equal(gram.id); + expect(merged[0].rawText).to.equal("100g Sugar + 45g Sugar"); + }); + + it("converts through toBaseFactor when the duplicate uses a different unit of the same type", () => { + const merged = mergeDuplicateIngredients( + [ + buildLine({ rawText: "500g Flour", ingredientId: 1, quantity: 500, unitId: gram.id }), + buildLine({ + rawText: "0.5kg Flour", + ingredientId: 1, + quantity: 0.5, + unitId: kilogram.id, + }), + ], + [gram, kilogram, milliliter, piece, slice], + ); + + expect(merged).to.have.length(1); + expect(merged[0].quantity).to.equal(1000); + expect(merged[0].unitId).to.equal(gram.id); + }); + + it("keeps duplicate COUNT-unit lines separate rather than guessing — a slice isn't a piece", () => { + const merged = mergeDuplicateIngredients( + [ + buildLine({ rawText: "2 piece Bread", ingredientId: 1, quantity: 2, unitId: piece.id }), + buildLine({ rawText: "3 slice Bread", ingredientId: 1, quantity: 3, unitId: slice.id }), + ], + [gram, kilogram, milliliter, piece, slice], + ); + + expect(merged).to.have.length(2); + }); + + it("keeps duplicate lines with incompatible unit types separate (MASS vs VOLUME)", () => { + const merged = mergeDuplicateIngredients( + [ + buildLine({ rawText: "200g Milk", ingredientId: 1, quantity: 200, unitId: gram.id }), + buildLine({ + rawText: "200ml Milk", + ingredientId: 1, + quantity: 200, + unitId: milliliter.id, + }), + ], + [gram, kilogram, milliliter, piece, slice], + ); + + expect(merged).to.have.length(2); + }); + + it("never merges two unresolved lines (ingredientId: null) together", () => { + const merged = mergeDuplicateIngredients( + [ + buildLine({ rawText: "1 vanilla pod", quantity: 1 }), + buildLine({ rawText: "1 vanilla pod", quantity: 1 }), + ], + [gram, kilogram, milliliter, piece, slice], + ); + + expect(merged).to.have.length(2); + }); + + it("leaves non-duplicate lines untouched and preserves order", () => { + const sugar = buildLine({ rawText: "Sugar", ingredientId: 1, quantity: 1, unitId: gram.id }); + const salt = buildLine({ rawText: "Salt", ingredientId: 2, quantity: 1, unitId: gram.id }); + + const merged = mergeDuplicateIngredients([sugar, salt], [gram, kilogram, milliliter]); + + expect(merged).to.deep.equal([sugar, salt]); + }); + }); + describe("translateRecipe", () => { beforeEach(async () => { await resetDatabase(); diff --git a/apps/api/test/sources.test.ts b/apps/api/test/sources.test.ts index 5441943..e869cab 100644 --- a/apps/api/test/sources.test.ts +++ b/apps/api/test/sources.test.ts @@ -78,6 +78,45 @@ function buildFakeAdapter(key = "fakeSource"): RecipeSourceAdapter<{ externalId: }; } +/** + * A fake adapter whose two ingredient lines both resolve to the same real + * seeded ingredient ("onion"), in the same unit (grams) — exercises + * `previewSourceItem`'s duplicate-merging (`mergeDuplicateIngredients`, + * see issue #53's follow-up) through the real HTTP endpoint/catalog, + * rather than only as a pure unit test of the merge function itself. + */ +function buildDuplicateIngredientAdapter(key = "duplicateFakeSource"): RecipeSourceAdapter<{ + externalId: string; +}> { + return { + key, + name: "Fake Source With Duplicates", + official: true, + iconUrl: null, + locale: "en", + async list(): Promise { + return { items: [], nextCursor: null }; + }, + async fetchDetail(externalId: string): Promise<{ externalId: string }> { + return { externalId }; + }, + parse(raw: { externalId: string }): ParsedRecipe { + return { + name: `Fake recipe ${raw.externalId}`, + description: null, + picture: null, + portions: 4, + sourceUrl: `https://fake.test/${raw.externalId}`, + ingredients: [ + { rawText: "100g Onion", quantity: null, unit: null, name: "onion" }, + { rawText: "50g Onion", quantity: null, unit: null, name: "onion" }, + ], + steps: [{ description: "Chop the onions finely", picture: null }], + }; + }, + }; +} + /** Resolves a reference ingredient's id by its `reference-seed-data.ts` uid (also its DB `key`) — same reasoning as `recipe.test.ts`'s own helper. */ async function ingredientId(key: string): Promise { const ingredient = await prisma.ingredient.findFirstOrThrow({ where: { key } }); @@ -262,6 +301,28 @@ describe("Sources", () => { expect(res.status).to.equal(404); expect(res.body.code).to.equal(ErrorCode.RECIPE_NOT_FOUND); }); + + it("merges two lines that resolve to the same ingredient, summing their quantity (issue #53 follow-up)", async () => { + const { agent } = await signupWithHouse(); + registerRecipeSource(buildDuplicateIngredientAdapter()); + await syncRecipeSources(prisma); + const source = await prisma.source.findUniqueOrThrow({ + where: { key: "duplicateFakeSource" }, + }); + await agent.patch("/house/current/sources").send({ sourceIds: [source.id] }); + const onion = await prisma.ingredient.findFirstOrThrow({ where: { key: "onion" } }); + const gram = await prisma.unit.findFirstOrThrow({ where: { key: "gram" } }); + + const res = await agent.get("/sources/duplicateFakeSource/preview/1"); + + expect(res.status).to.equal(200); + expect(res.body.ingredients).to.have.length(1); + const [merged] = res.body.ingredients; + expect(merged.ingredient).to.deep.include({ id: onion.id, key: "onion" }); + expect(merged.unit).to.deep.include({ id: gram.id, key: "gram" }); + expect(merged.quantity).to.equal(150); + expect(merged.rawText).to.equal("100g Onion + 50g Onion"); + }); }); describe("POST /sources/:sourceKey/import/:externalId", () => {