diff --git a/adr/0014-multi-trip-shopping-planner.md b/adr/0014-multi-trip-shopping-planner.md index 2a85a41..be8024e 100644 --- a/adr/0014-multi-trip-shopping-planner.md +++ b/adr/0014-multi-trip-shopping-planner.md @@ -34,9 +34,17 @@ Shelf life is read from the same-unit product variants of the ingredient. ADR 00 ### Owned amounts -`planShoppingTrips` accepts an optional `ownedAmounts: Map`, where `OwnedStock` (in `owned_amount.dart`) is the user's stock as one amount plus one selected unit (or null for "packs"). For each cooking event the planner converts that stock into the event's unit via the shared `ownedAmountInUnit` (in `owned_amount.dart`), then consumes it against the matching-unit events in chronological order before trips are computed. `ownedAmountInUnit` handles same-unit, cross-unit (grams <-> pieces via `gramsPerPiece`, weight <-> volume via `density`), and "packs" mode (the product whose unit matches the target unit). The on-screen shopping list (`_ownedInUnit` in `shopping_page.dart`) calls the same function, so the copied trip amounts always equal the on-screen "Need" amounts. When no conversion path exists (for example owned pieces with no `gramsPerPiece`), nothing is subtracted. +`planShoppingTrips` accepts an optional `ownedAmounts: Map`, where `OwnedStock` (in `owned_amount.dart`) is the user's stock as one amount plus one selected unit (or null for "packs"). Both the planner and the on-screen list draw the stock down through the shared `OwnedStockConsumer` (in `owned_amount.dart`): the stock is turned into a single grams pool (via `ownedAmountInUnit`) and consumed across the ingredient's needs, one need at a time, in chronological order. `ownedAmountInUnit` handles same-unit, cross-unit (grams <-> pieces via `gramsPerPiece`, weight <-> volume via `density`), and "packs" mode (the product whose unit matches the target unit). A need whose unit has no grams path, or an owned stock with no grams path at all (for example owned pieces with no `gramsPerPiece`), falls back to a per-unit subtraction. Because both callers use the same consumer, the copied trip amounts always equal the on-screen "Need" amounts. -Earlier this planner did no cross-unit conversion: it took owned as `Map>` and subtracted only when the owned unit exactly matched the event unit, and the shopping page converted "packs" using the ingredient's first product. That diverged from the on-screen list (which converted correctly), so the copied list could list a higher amount to buy than the page showed. The shared `ownedAmountInUnit` removed the divergence. +Earlier this planner did no cross-unit conversion: it took owned as `Map>` and subtracted only when the owned unit exactly matched the event unit, and the shopping page converted "packs" using the ingredient's first product. That diverged from the on-screen list (which converted correctly), so the copied list could list a higher amount to buy than the page showed. Moving both to `ownedAmountInUnit` removed that divergence. + +A later divergence came from converting the same owned stock into each unit independently. When one ingredient is needed in two units at once (for example grams in one recipe and pieces in another, with a pieces product so the normalizer keeps both), subtracting the full stock from each unit over-subtracts. On the page this only inflated how much was marked as owned; in the planner it zeroed every event and dropped the ingredient from the trip list entirely, so the copy was missing an ingredient the page still showed as needed. The single-grams-pool `OwnedStockConsumer` fixes both: it is the one source of truth for owned-stock subtraction, so a single stock is never subtracted more than once and the planner never drops an ingredient the page still needs. + +### Matching the copied amounts to the on-screen list + +The on-screen list normalizes units before display (`quantity_normalizer.dart`: pieces to grams via `gramsPerPiece`, volume to grams via `density`) and rounds each ingredient's total once. The planner instead works on the raw, per-day cooking timeline in the recipe's own units. Feeding the planner's raw per-trip amounts straight into the copied text made the copy diverge from the page in two ways: a different unit (planner "4 pieces" vs page "20 g"), and per-trip lines that rounded separately and summed to one more or less than the page total. + +The copy no longer prints the planner's raw amounts. For each ingredient it takes the on-screen remaining (already normalized, owned-subtracted, and rounded) and calls `distributeRemainingAcrossTrips` (in `trip_amount_distributor.dart`), which spreads that remaining across the weeks the planner chose. Each week's share is weighted by that week's raw need expressed in grams (so pieces and volume compare on one scale), and a largest-remainder split keeps every per-trip line a whole number while guaranteeing the lines sum to exactly the on-screen amount, in the on-screen unit. The planner still owns trip assignment (which weeks, respecting shelf life); the distributor only decides how the page's number is split across those weeks. The on-screen per-trip pack split (`_tripPurchasesForProduct` in `shopping_ingredient.dart`) still rounds each trip's raw amount to compute packs; it agrees with the copy at pack granularity and is left unchanged. ### UI @@ -51,6 +59,7 @@ ADR 0015 supersedes this UI: the OFF mode (flat list, ignore shelf life) was dro - Trips are weekly-only by construction. If the user has a real-world cadence like "I shop on Wednesday and Saturday", the planner cannot match it. Adding configurable trip days would mean exposing trip schedules in the UI; deferred until requested. - The planner originally used the first-matching-unit product for shelf life. ADR 0015 promoted this to an any-match: the longest sealed shelf life among same-unit variants drives trip planning, and any freezable variant makes the ingredient freezable for the freezer-aware mode. The shopping page's pack-display code (`products.first`) is unaffected; only the planner's shelf-life and freezable lookups changed. - Non-perishables defaulting to trip 0 means a menu of only non-perishables produces a single trip 0, matching the prior single-trip behavior exactly. +- The copied per-trip amounts always equal the on-screen list: same unit, and per-trip lines that sum to the on-screen total with no rounding drift. The planner picks the trips; `distributeRemainingAcrossTrips` splits the page's number across them. If the planner and page ever disagree on which weeks buy an ingredient, the page total still wins because the copy is derived from it. - Owned amounts are still tracked at the ingredient level (one number, one selected unit), not per-trip. The planner subtracts owned from earliest events first, which usually means owned reduces what is bought on the earliest trip. There is no way today for the user to say "I have 100g of X but I want to use it on trip 2". If this comes up we can add per-event owned overrides. - The on-screen list does not visually sectionize when the toggle is on. The banner under the AppBar is the only on-screen feedback besides the copy output. If users want section headers on screen we can iterate on the per-ingredient widget without changing the planner. - The `single-shopping-trip` assumption in ADR 0010's menu expiry warning is unchanged: that warning still assumes one purchase the day before menu day 0. Multi-trip mode is a property of the shopping list copy, not of the menu warning. Reconciling them (warning aware of trips) is possible later but not part of this change. The freeze-aware single-trip mode is documented in ADR 0015 and likewise does not change the menu warning's single-trip assumption. diff --git a/menu_management/lib/shopping/multi_trip_planner.dart b/menu_management/lib/shopping/multi_trip_planner.dart index 4ae1772..1135dc2 100644 --- a/menu_management/lib/shopping/multi_trip_planner.dart +++ b/menu_management/lib/shopping/multi_trip_planner.dart @@ -1,5 +1,3 @@ -import "dart:math"; - import "package:menu_management/ingredients/models/ingredient.dart"; import "package:menu_management/ingredients/models/product.dart"; import "package:menu_management/recipes/enums/unit.dart"; @@ -53,9 +51,9 @@ class ShoppingTrip { /// or before the event day. The matching menu warning surfaces this to the user. /// /// [ownedAmounts] holds the user's owned stock per ingredient (one amount + one selected -/// unit, or "packs"). It is converted into each event's unit via the shared -/// [ownedAmountInUnit] and consumed against the earliest events first, so the planner -/// subtracts exactly what the on-screen shopping list subtracts. +/// unit, or "packs"). It is drawn down via the shared [OwnedStockConsumer] (a single grams pool) +/// against the earliest events first, the same subtraction the on-screen shopping list runs, so the +/// planner subtracts exactly what the page subtracts and never drops an ingredient the page still needs. /// /// When [assumeFreezerForFreezable] is true, every event whose matching product has /// [Product.canBeFrozen] set is treated as non-perishable for trip assignment, so freezable @@ -144,8 +142,9 @@ List planShoppingTrips({ } /// Builds per-(ingredient, unit) plan events from the timeline, after applying -/// owned amounts chronologically against each event. Owned stock is converted into -/// the event's unit via the shared [ownedAmountInUnit] the first time that unit is seen. +/// owned amounts chronologically against each event. Owned stock is drawn down via the shared +/// [OwnedStockConsumer], the same single-grams-pool logic the on-screen list uses, so a single +/// stock is subtracted only once even when the ingredient is needed in more than one unit. /// /// When [assumeFreezerForFreezable] is true and the matching product is freezable, /// the event's effective shelf life is null (treated as non-perishable for trip assignment). @@ -165,24 +164,17 @@ List<_PlanEvent> _buildPlanEvents({ Ingredient? ingredient = ingredientsById[ingredientId]; OwnedStock? owned = ownedAmounts[ingredientId]; - // Remaining owned per event unit, converted from the user's stock via the shared - // converter the first time each unit appears, then consumed chronologically. - Map ownedRemainingByUnit = {}; + // One shared owned-stock pool for this ingredient, consumed chronologically across every event + // and unit. This is the same single-grams-pool subtraction the on-screen list runs (see + // computeRemainingQuantities), so the planner and the page never disagree on how much is still + // needed. Consuming full owned stock per unit here would over-subtract and drop the ingredient. + OwnedStockConsumer? consumer = (owned == null || ingredient == null) + ? null + : OwnedStockConsumer(ingredient: ingredient, ownedAmount: owned.amount, ownedUnit: owned.unit); for (CookingEvent event in events) { for (Quantity quantity in event.quantities) { - double remainingNeed = quantity.amount; - double ownedRemaining = ownedRemainingByUnit.putIfAbsent( - quantity.unit, - () => (owned == null || ingredient == null) - ? 0 - : ownedAmountInUnit(ingredient: ingredient, ownedAmount: owned.amount, ownedUnit: owned.unit, targetUnit: quantity.unit), - ); - if (ownedRemaining > 0 && remainingNeed > 0) { - double consumed = min(ownedRemaining, remainingNeed); - ownedRemainingByUnit[quantity.unit] = ownedRemaining - consumed; - remainingNeed -= consumed; - } + double remainingNeed = consumer == null ? quantity.amount : consumer.consumeRemaining(quantity); if (remainingNeed <= 0) continue; // Any-match across same-unit variants: the user can pick the longest-shelf-life diff --git a/menu_management/lib/shopping/owned_amount.dart b/menu_management/lib/shopping/owned_amount.dart index 001e6de..4043f9a 100644 --- a/menu_management/lib/shopping/owned_amount.dart +++ b/menu_management/lib/shopping/owned_amount.dart @@ -1,3 +1,5 @@ +import "dart:math"; + import "package:menu_management/ingredients/models/ingredient.dart"; import "package:menu_management/ingredients/models/product.dart"; import "package:menu_management/recipes/enums/unit.dart"; @@ -46,3 +48,90 @@ double ownedAmountInUnit({required Ingredient ingredient, required double ownedA return 0; } + +/// Draws down a user's owned stock across an ingredient's needs, one need at a time. +/// +/// The stock is turned into a single shared grams pool (via [ownedAmountInUnit]) and consumed across +/// every need in the order [consumeRemaining] is called. This makes a single owned stock get +/// subtracted only once, even when the ingredient is needed in more than one unit at the same time. +/// It is the single source of truth for owned-stock subtraction, shared by the on-screen shopping +/// list ([computeRemainingQuantities]) and the multi-trip planner (`multi_trip_planner.dart`), so the +/// two never disagree on how much is still needed. +/// +/// Create one consumer per (ingredient, owned stock). A need whose unit cannot be related to grams +/// (no density and no gramsPerPiece), or an owned stock with no grams path at all (for example owned +/// pieces with no gramsPerPiece), falls back to a per-unit subtraction that is likewise consumed only +/// once per unit across calls. +class OwnedStockConsumer { + OwnedStockConsumer({required Ingredient ingredient, required double ownedAmount, required Unit? ownedUnit}) + : _ingredient = ingredient, + _ownedAmount = ownedAmount, + _ownedUnit = ownedUnit, + _gramsPool = ownedAmount <= 0 + ? 0 + : ownedAmountInUnit(ingredient: ingredient, ownedAmount: ownedAmount, ownedUnit: ownedUnit, targetUnit: Unit.grams); + + final Ingredient _ingredient; + final double _ownedAmount; + final Unit? _ownedUnit; + + /// Remaining shared grams pool. Drawn down by each need; reaching 0 just means the stock is used up, + /// not that there is no grams path (that is fixed at construction, see [_ownedHasGramsPath]). + double _gramsPool; + + /// Whether the owned stock can be expressed in grams at all. Fixed at construction from the initial + /// pool. The branch must key off this, not the live pool level: once the pool drains to 0, later + /// needs must still stay on the grams path (subtracting nothing more), not fall back to a per-unit + /// conversion that would re-subtract the full owned stock. + late final bool _ownedHasGramsPath = _gramsPool > 0; + + /// Per-unit remaining owned for needs with no grams path, converted on first use of each unit. + final Map _fallbackOwnedByUnit = {}; + + /// Returns how much of [need] still has to be bought after applying the owned stock. + /// The result is raw (not rounded); callers that display whole units round it themselves. + double consumeRemaining(Quantity need) { + if (_ownedAmount <= 0) return need.amount; + + // No grams conversion path from the owned stock: subtract per unit directly. + if (!_ownedHasGramsPath) return _consumeFallback(need); + + double? needGrams = _ingredient.toGrams(need); + // This need's unit cannot be expressed in grams; only a same-unit owned stock can reduce it. + if (needGrams == null) return _consumeFallback(need); + + double consumed = min(_gramsPool, needGrams); + _gramsPool -= consumed; + double remainingGrams = needGrams - consumed; + double? remainingInUnit = _ingredient.fromGrams(remainingGrams, need.unit); + return remainingInUnit ?? need.amount; + } + + double _consumeFallback(Quantity need) { + double owned = _fallbackOwnedByUnit.putIfAbsent( + need.unit, + () => ownedAmountInUnit(ingredient: _ingredient, ownedAmount: _ownedAmount, ownedUnit: _ownedUnit, targetUnit: need.unit), + ); + double consumed = min(owned, need.amount); + _fallbackOwnedByUnit[need.unit] = owned - consumed; + return need.amount - consumed; + } +} + +/// Subtracts the user's owned stock from an ingredient's required amounts and rounds each to a +/// whole unit, producing the "remaining to buy" the on-screen shopping list shows. +/// +/// [requiredQuantities] must already be normalized (the output of `normalizeQuantities`). +/// +/// Delegates to [OwnedStockConsumer] so the stock is consumed a single time across all units. This +/// prevents the old bug where a single stock was fully converted into every unit and subtracted from +/// each, over-subtracting when an ingredient is needed in more than one unit at once. +List computeRemainingQuantities({ + required Ingredient ingredient, + required List requiredQuantities, + required double ownedAmount, + required Unit? ownedUnit, +}) { + OwnedStockConsumer consumer = OwnedStockConsumer(ingredient: ingredient, ownedAmount: ownedAmount, ownedUnit: ownedUnit); + return requiredQuantities.map((Quantity q) => Quantity(amount: max(0, consumer.consumeRemaining(q)).roundToDouble(), unit: q.unit)).toList(); +} diff --git a/menu_management/lib/shopping/shopping_page.dart b/menu_management/lib/shopping/shopping_page.dart index cf54c5a..d205922 100644 --- a/menu_management/lib/shopping/shopping_page.dart +++ b/menu_management/lib/shopping/shopping_page.dart @@ -1,5 +1,3 @@ -import "dart:math"; - import "package:flutter/material.dart"; import "package:flutter/services.dart"; import "package:menu_management/flutter_essentials/library.dart"; @@ -16,6 +14,7 @@ import "package:menu_management/shopping/owned_amount.dart"; import "package:menu_management/shopping/quantity_normalizer.dart"; import "package:menu_management/shopping/ingredient_source.dart"; import "package:menu_management/shopping/shopping_ingredient.dart"; +import "package:menu_management/shopping/trip_amount_distributor.dart"; import "package:menu_management/shopping/waste_optimizer.dart"; class ShoppingPage extends StatefulWidget { @@ -153,23 +152,18 @@ class _ShoppingPageState extends State { ); } - /// Converts owned amount to the actual quantity in [targetUnit] based on the selected owned unit. - /// Delegates to the shared [ownedAmountInUnit] so the on-screen list and the trip planner subtract - /// the same amount. - double _ownedInUnit({required String ingredientId, required Ingredient ingredient, required Unit targetUnit}) { - double amount = ownedAmounts[ingredientId] ?? 0; - OwnedUnit selectedUnit = ownedUnits[ingredientId] ?? OwnedUnit(unit: targetUnit); - return ownedAmountInUnit(ingredient: ingredient, ownedAmount: amount, ownedUnit: selectedUnit.unit, targetUnit: targetUnit); - } - + /// The on-screen "remaining to buy" for one ingredient: the normalized required amounts with the + /// user's owned stock subtracted once and each line rounded to a whole unit. Delegates to the + /// shared [computeRemainingQuantities] so the header, the single list, and the per-trip copy all + /// start from the same numbers. List _remainingAmounts({required String ingredientId, required Ingredient ingredient}) { - List required = ingredientsRequired[ingredientId]!; - - return required.map((Quantity quantityRequired) { - double owned = _ownedInUnit(ingredientId: ingredientId, ingredient: ingredient, targetUnit: quantityRequired.unit); - double amount = quantityRequired.amount - owned; - return Quantity(amount: max(0, amount).roundToDouble(), unit: quantityRequired.unit); - }).toList(); + OwnedUnit selectedUnit = ownedUnits[ingredientId] ?? const OwnedUnit(unit: Unit.grams); + return computeRemainingQuantities( + ingredient: ingredient, + requiredQuantities: ingredientsRequired[ingredientId]!, + ownedAmount: ownedAmounts[ingredientId] ?? 0, + ownedUnit: selectedUnit.unit, + ); } void _copyToClipboard() { @@ -203,24 +197,38 @@ class _ShoppingPageState extends State { List trips = _planTrips(); if (trips.isEmpty) return _buildSingleListCopyText(); - StringBuffer buffer = StringBuffer(); - for (int i = 0; i < trips.length; i++) { - ShoppingTrip trip = trips[i]; - if (i > 0) buffer.writeln(); - buffer.writeln("Week ${trip.weekIndex + 1}"); - buffer.writeln("--------"); + // Spread each ingredient's on-screen remaining across the trip weeks in the on-screen unit, so + // the copied per-trip amounts sum to exactly what the page shows (same unit, no rounding drift). + // Bucket the resulting lines by week, then print the sections in the planner's trip order. + Map> linesByWeek = {for (ShoppingTrip trip in trips) trip.weekIndex: []}; - // Group items by ingredient (one ingredient may have multiple units). - Map> byIngredient = {}; - for (TripItem item in trip.items) { - byIngredient.putIfAbsent(item.ingredientId, () => []).add(item); + for (String ingredientId in ingredientsRequired.keys) { + Ingredient ingredient = IngredientsProvider.instance.get(ingredientId); + List remaining = _remainingAmounts(ingredientId: ingredientId, ingredient: ingredient); + List allocations = distributeRemainingAcrossTrips(ingredient: ingredient, pageRemaining: remaining, trips: trips); + for (TripAllocation allocation in allocations) { + linesByWeek[allocation.weekIndex]!.add((ingredient: ingredient, allocation: allocation)); } + } - for (MapEntry> entry in byIngredient.entries) { - Ingredient ingredient = IngredientsProvider.instance.get(entry.key); - List tripQuantities = entry.value.map((TripItem i) => Quantity(amount: i.amount, unit: i.unit)).toList(); - bool freezeOnArrival = entry.value.any((TripItem i) => i.freezeOnArrival); - _appendIngredientLines(buffer: buffer, ingredient: ingredient, remaining: tripQuantities, freezeOnArrival: freezeOnArrival); + StringBuffer buffer = StringBuffer(); + bool wroteSection = false; + for (ShoppingTrip trip in trips) { + List<({Ingredient ingredient, TripAllocation allocation})> lines = linesByWeek[trip.weekIndex]!; + if (lines.isEmpty) continue; + lines.sort((a, b) => a.ingredient.name.toLowerCase().compareTo(b.ingredient.name.toLowerCase())); + + if (wroteSection) buffer.writeln(); + wroteSection = true; + buffer.writeln("Week ${trip.weekIndex + 1}"); + buffer.writeln("--------"); + for (({Ingredient ingredient, TripAllocation allocation}) line in lines) { + _appendIngredientLines( + buffer: buffer, + ingredient: line.ingredient, + remaining: line.allocation.quantities, + freezeOnArrival: line.allocation.freezeOnArrival, + ); } } diff --git a/menu_management/lib/shopping/trip_amount_distributor.dart b/menu_management/lib/shopping/trip_amount_distributor.dart new file mode 100644 index 0000000..8fb62b7 --- /dev/null +++ b/menu_management/lib/shopping/trip_amount_distributor.dart @@ -0,0 +1,119 @@ +import "package:menu_management/ingredients/models/ingredient.dart"; +import "package:menu_management/recipes/models/quantity.dart"; +import "package:menu_management/shopping/multi_trip_planner.dart"; + +/// One ingredient's amount to buy on a single shopping trip, in the SAME normalized unit(s) +/// the on-screen shopping list shows. +class TripAllocation { + const TripAllocation({required this.weekIndex, required this.quantities, required this.freezeOnArrival}); + + final int weekIndex; + + /// Amounts in the on-screen (normalized) units, each a whole number. + final List quantities; + + /// True when the planner marked this ingredient's trip items as frozen on arrival. + final bool freezeOnArrival; +} + +/// Splits an ingredient's on-screen remaining amount across the trips the planner chose, so the +/// copied per-trip lines match the on-screen list exactly. +/// +/// Why this exists: the on-screen list normalizes units (e.g. pieces to grams via `gramsPerPiece`) +/// and rounds the total once, while the planner works on the raw, per-day cooking timeline in the +/// recipe's own units. Reading the planner output straight into the copied text made the copy show +/// a different unit or a total that was off by one from the page. This function keeps the on-screen +/// unit and total ([pageRemaining]) and only borrows the planner's [trips] to decide how to spread +/// that total across the weeks. +/// +/// [pageRemaining] must be the on-screen remaining (normalized, owned-subtracted, already rounded). +/// Each trip's share of a unit is weighted by that trip's raw need for this ingredient, expressed in +/// grams so pieces and volume compare on the same scale. A largest-remainder split keeps every +/// per-trip amount a whole number while guaranteeing they sum to [pageRemaining]. +List distributeRemainingAcrossTrips({ + required Ingredient ingredient, + required List pageRemaining, + required List trips, +}) { + // Weeks (in trip order) that include this ingredient, each with its raw grams-equivalent weight + // and whether any of its items must be frozen on arrival. + List weeks = []; + Map weightByWeek = {}; + Map freezeByWeek = {}; + + for (ShoppingTrip trip in trips) { + double weight = 0; + bool freeze = false; + bool present = false; + for (TripItem item in trip.items) { + if (item.ingredientId != ingredient.id) continue; + present = true; + weight += ingredient.toGrams(Quantity(amount: item.amount, unit: item.unit)) ?? item.amount; + if (item.freezeOnArrival) freeze = true; + } + if (present) { + weeks.add(trip.weekIndex); + weightByWeek[trip.weekIndex] = weight; + freezeByWeek[trip.weekIndex] = freeze; + } + } + + if (weeks.isEmpty) return const []; + + Map> quantitiesByWeek = {for (int week in weeks) week: []}; + + for (Quantity quantity in pageRemaining) { + int total = quantity.amount.round(); + if (total <= 0) continue; + Map perWeek = _largestRemainderSplit(total: total, weeks: weeks, weights: weightByWeek); + for (int week in weeks) { + int amount = perWeek[week] ?? 0; + if (amount > 0) quantitiesByWeek[week]!.add(Quantity(amount: amount.toDouble(), unit: quantity.unit)); + } + } + + List result = []; + for (int week in weeks) { + List quantities = quantitiesByWeek[week]!; + if (quantities.isEmpty) continue; + result.add(TripAllocation(weekIndex: week, quantities: quantities, freezeOnArrival: freezeByWeek[week] ?? false)); + } + return result; +} + +/// Distributes an integer [total] across [weeks] proportionally to [weights], using the +/// largest-remainder method so the parts always sum to [total]. When every weight is zero, the +/// whole total lands on the first week (it still has to be bought somewhere). +Map _largestRemainderSplit({required int total, required List weeks, required Map weights}) { + double weightSum = weeks.fold(0, (double sum, int week) => sum + (weights[week] ?? 0)); + + Map result = {for (int week in weeks) week: 0}; + if (weightSum <= 0) { + result[weeks.first] = total; + return result; + } + + Map fractional = {}; + int assigned = 0; + for (int week in weeks) { + double exact = total * (weights[week] ?? 0) / weightSum; + int floor = exact.floor(); + result[week] = floor; + fractional[week] = exact - floor; + assigned += floor; + } + + int remaining = total - assigned; + // Give leftover units to the largest fractional parts; break ties by trip order (earliest first). + List order = [...weeks] + ..sort((int a, int b) { + int byFraction = fractional[b]!.compareTo(fractional[a]!); + if (byFraction != 0) return byFraction; + return weeks.indexOf(a).compareTo(weeks.indexOf(b)); + }); + for (int i = 0; i < remaining; i++) { + int week = order[i % order.length]; + result[week] = result[week]! + 1; + } + return result; +} diff --git a/menu_management/test/multi_trip_planner_test.dart b/menu_management/test/multi_trip_planner_test.dart index 312c194..15d7dc6 100644 --- a/menu_management/test/multi_trip_planner_test.dart +++ b/menu_management/test/multi_trip_planner_test.dart @@ -29,6 +29,19 @@ CookingEvent _event({required int day, double amount = 100, Unit unit = Unit.gra ); } +CookingEvent _multiUnitEvent({required int day, required List quantities}) => CookingEvent(dayIndex: day, quantities: quantities); + +double _gramsBoughtFor(List trips, Ingredient ingredient) { + double total = 0; + for (ShoppingTrip trip in trips) { + for (TripItem item in trip.items) { + if (item.ingredientId != ingredient.id) continue; + total += ingredient.toGrams(Quantity(amount: item.amount, unit: item.unit)) ?? item.amount; + } + } + return total; +} + void main() { group("planShoppingTrips", () { test("returns no trips when timeline is empty", () { @@ -257,6 +270,46 @@ void main() { expect(trips.first.items.first.unit, Unit.centiliters); }); + test("single owned stock spanning two units is subtracted only once (ingredient is not dropped)", () { + // Garlic is needed as 4 pieces AND 100 g on the same day. It has gramsPerPiece = 25 and both a + // pieces and a grams product, so the two units stay separate. The user owns 100 g, which equals + // the 4 pieces (4 * 25). A single owned stock must cover only one of the two lines, leaving the + // other still to buy. The on-screen page shows a positive remaining (100 g of need left), so the + // planner must NOT zero out every event and drop the ingredient from the trip list. + Ingredient garlic = _ingredient( + id: "garlic", + name: "Ajo", + gramsPerPiece: 25, + products: [ + _product(unit: Unit.grams, quantityPerItem: 150), + _product(unit: Unit.pieces, quantityPerItem: 1), + ], + ); + Map> timeline = { + "garlic": [ + _multiUnitEvent( + day: 0, + quantities: const [ + Quantity(amount: 4, unit: Unit.pieces), + Quantity(amount: 100, unit: Unit.grams), + ], + ), + ], + }; + + List trips = planShoppingTrips( + cookingTimeline: timeline, + ingredients: [garlic], + ownedAmounts: const {"garlic": OwnedStock(amount: 100, unit: Unit.grams)}, + ); + + expect(trips, isNotEmpty); + // Total still-needed for garlic (grams-equivalent) must be exactly 100 g: 200 g of need minus the + // single 100 g owned stock. The old per-unit subtraction removed 100 g from BOTH lines (200 g) and + // dropped the ingredient entirely. + expect(_gramsBoughtFor(trips, garlic), 100); + }); + test("owned amount in packs mode subtracts using the product matching the event unit", () { // Products: pieces (1/pack) first, grams (500/pack) second. Recipe uses 1000 grams. // Owned is 1 pack. The planner must use the 500 g product, subtracting 500 g -> 500 g left. diff --git a/menu_management/test/owned_amount_test.dart b/menu_management/test/owned_amount_test.dart index 5d5100b..015f3aa 100644 --- a/menu_management/test/owned_amount_test.dart +++ b/menu_management/test/owned_amount_test.dart @@ -2,6 +2,7 @@ import "package:flutter_test/flutter_test.dart"; import "package:menu_management/ingredients/models/ingredient.dart"; import "package:menu_management/ingredients/models/product.dart"; import "package:menu_management/recipes/enums/unit.dart"; +import "package:menu_management/recipes/models/quantity.dart"; import "package:menu_management/shopping/owned_amount.dart"; Product _product({required Unit unit, double quantityPerItem = 100, int itemsPerPack = 1}) { @@ -61,4 +62,62 @@ void main() { expect(ownedAmountInUnit(ingredient: banana, ownedAmount: 3, ownedUnit: Unit.pieces, targetUnit: Unit.grams), 0); }); }); + + group("computeRemainingQuantities", () { + test("subtracts owned once for a single-unit need (matches the old per-unit result)", () { + Ingredient flour = const Ingredient(id: "flour", name: "Flour"); + List remaining = computeRemainingQuantities( + ingredient: flour, + requiredQuantities: const [Quantity(amount: 600, unit: Unit.grams)], + ownedAmount: 250, + ownedUnit: Unit.grams, + ); + + expect(remaining.length, 1); + expect(remaining.first.unit, Unit.grams); + expect(remaining.first.amount, 350); + }); + + test("rounds the remaining amount to whole units", () { + Ingredient flour = const Ingredient(id: "flour", name: "Flour"); + List remaining = computeRemainingQuantities( + ingredient: flour, + requiredQuantities: const [Quantity(amount: 100.6, unit: Unit.grams)], + ownedAmount: 0, + ownedUnit: Unit.grams, + ); + + expect(remaining.first.amount, 101); + }); + + test("does not subtract a single owned stock more than once when the need spans two units", () { + // Garlic is needed as 4 pieces AND 50 g. It has gramsPerPiece = 25 and a pieces product, + // so the normalizer keeps pieces and grams as two separate lines. The user owns 100 g. + // 100 g equals the 4 pieces (4 * 25). The owned stock must cover the pieces line and be + // used up, leaving the 50 g line untouched -- NOT subtracted from both lines. + Ingredient garlic = Ingredient( + id: "garlic", + name: "Ajo", + gramsPerPiece: 25, + products: [ + _product(unit: Unit.grams, quantityPerItem: 150), + _product(unit: Unit.pieces, quantityPerItem: 1), + ], + ); + List remaining = computeRemainingQuantities( + ingredient: garlic, + requiredQuantities: const [ + Quantity(amount: 4, unit: Unit.pieces), + Quantity(amount: 50, unit: Unit.grams), + ], + ownedAmount: 100, + ownedUnit: Unit.grams, + ); + + double pieces = remaining.firstWhere((q) => q.unit == Unit.pieces).amount; + double grams = remaining.firstWhere((q) => q.unit == Unit.grams).amount; + expect(pieces, 0); + expect(grams, 50); + }); + }); } diff --git a/menu_management/test/trip_amount_distributor_test.dart b/menu_management/test/trip_amount_distributor_test.dart new file mode 100644 index 0000000..220a565 --- /dev/null +++ b/menu_management/test/trip_amount_distributor_test.dart @@ -0,0 +1,203 @@ +import "package:flutter_test/flutter_test.dart"; +import "package:menu_management/ingredients/models/ingredient.dart"; +import "package:menu_management/ingredients/models/product.dart"; +import "package:menu_management/recipes/enums/unit.dart"; +import "package:menu_management/recipes/models/quantity.dart"; +import "package:menu_management/shopping/cooking_timeline.dart"; +import "package:menu_management/shopping/multi_trip_planner.dart"; +import "package:menu_management/shopping/owned_amount.dart"; +import "package:menu_management/shopping/trip_amount_distributor.dart"; + +Product _grams({int? shelfLifeDaysClosed, double quantityPerItem = 100}) => + Product(link: "https://example.com/g", unit: Unit.grams, quantityPerItem: quantityPerItem, shelfLifeDaysClosed: shelfLifeDaysClosed); + +ShoppingTrip _trip(int weekIndex, List items) => ShoppingTrip(weekIndex: weekIndex, items: items); + +TripItem _item({String ingredientId = "i", required double amount, Unit unit = Unit.grams, bool freezeOnArrival = false}) => + TripItem(ingredientId: ingredientId, amount: amount, unit: unit, freezeOnArrival: freezeOnArrival); + +double _sumFor(List allocations, Unit unit) { + double total = 0; + for (TripAllocation a in allocations) { + for (Quantity q in a.quantities) { + if (q.unit == unit) total += q.amount; + } + } + return total; +} + +void main() { + group("distributeRemainingAcrossTrips", () { + test("returns no allocations when the ingredient is not on any trip", () { + Ingredient rice = const Ingredient(id: "rice", name: "Rice"); + List trips = [ + _trip(0, [_item(ingredientId: "beans", amount: 100)]), + ]; + + List result = distributeRemainingAcrossTrips( + ingredient: rice, + pageRemaining: const [Quantity(amount: 100, unit: Unit.grams)], + trips: trips, + ); + + expect(result, isEmpty); + }); + + test("multi-trip rounding: per-trip amounts sum to the on-screen whole number (no drift)", () { + // Page shows 100 g. Planner split the raw need as 33.3 + 33.3 + 33.4 across three weeks. + // Rounding each line alone gives 33 + 33 + 33 = 99, one short of the page total. + // The distributor must make the per-trip lines sum to exactly 100. + Ingredient flour = Ingredient(id: "flour", name: "Flour", products: [_grams()]); + List trips = [ + _trip(0, [_item(ingredientId: "flour", amount: 33.3)]), + _trip(1, [_item(ingredientId: "flour", amount: 33.3)]), + _trip(2, [_item(ingredientId: "flour", amount: 33.4)]), + ]; + + List result = distributeRemainingAcrossTrips( + ingredient: flour, + pageRemaining: const [Quantity(amount: 100, unit: Unit.grams)], + trips: trips, + ); + + expect(_sumFor(result, Unit.grams), 100); + // Every per-trip amount is a whole number. + for (TripAllocation a in result) { + for (Quantity q in a.quantities) { + expect(q.amount, q.amount.roundToDouble()); + } + } + }); + + test("different unit: on-screen grams are split across trips even though the planner used pieces", () { + // Ingredient has gramsPerPiece and only a grams product, so the on-screen list normalizes + // pieces to grams (page shows 60 g = 12 pieces * 5 g). The planner timeline is still in + // pieces (4 + 8). The copied per-trip amounts must be in grams and sum to the page's 60 g. + Ingredient garlic = Ingredient(id: "garlic", name: "Ajo", gramsPerPiece: 5, products: [_grams(quantityPerItem: 150)]); + List trips = [ + _trip(0, [_item(ingredientId: "garlic", amount: 4, unit: Unit.pieces)]), + _trip(1, [_item(ingredientId: "garlic", amount: 8, unit: Unit.pieces)]), + ]; + + List result = distributeRemainingAcrossTrips( + ingredient: garlic, + pageRemaining: const [Quantity(amount: 60, unit: Unit.grams)], + trips: trips, + ); + + // All lines are in grams (the on-screen unit), none in pieces. + expect(result.every((a) => a.quantities.every((q) => q.unit == Unit.grams)), isTrue); + expect(_sumFor(result, Unit.grams), 60); + // Weighted by the pieces need: week 0 (4 pieces -> 20 g), week 1 (8 pieces -> 40 g). + expect(result.firstWhere((a) => a.weekIndex == 0).quantities.first.amount, 20); + expect(result.firstWhere((a) => a.weekIndex == 1).quantities.first.amount, 40); + }); + + test("carries the freeze-on-arrival flag from the planner's trip items", () { + Ingredient chicken = Ingredient(id: "chicken", name: "Chicken", products: [_grams(shelfLifeDaysClosed: 3)]); + List trips = [ + _trip(0, [_item(ingredientId: "chicken", amount: 200, freezeOnArrival: true)]), + ]; + + List result = distributeRemainingAcrossTrips( + ingredient: chicken, + pageRemaining: const [Quantity(amount: 200, unit: Unit.grams)], + trips: trips, + ); + + expect(result.single.freezeOnArrival, isTrue); + }); + + test("defensive: whole page amount lands on the earliest trip when raw weights are all zero", () { + // Defensive-only path. In production the planner never emits a zero-amount trip item (events with + // nothing left to buy are skipped), so a present-but-zero-weight week cannot happen through the + // real flow. This still guards the largest-remainder split's weightSum <= 0 branch directly: if + // every weight were zero yet the page showed a remaining, it must still land somewhere (trip 0). + Ingredient flour = Ingredient(id: "flour", name: "Flour", products: [_grams()]); + List trips = [ + _trip(0, [_item(ingredientId: "flour", amount: 0)]), + _trip(1, [_item(ingredientId: "flour", amount: 0)]), + ]; + + List result = distributeRemainingAcrossTrips( + ingredient: flour, + pageRemaining: const [Quantity(amount: 50, unit: Unit.grams)], + trips: trips, + ); + + expect(_sumFor(result, Unit.grams), 50); + expect(result.first.weekIndex, 0); + expect(result.first.quantities.first.amount, 50); + }); + + test("skips a zero page amount entirely", () { + Ingredient flour = Ingredient(id: "flour", name: "Flour", products: [_grams()]); + List trips = [ + _trip(0, [_item(ingredientId: "flour", amount: 100)]), + ]; + + List result = distributeRemainingAcrossTrips( + ingredient: flour, + pageRemaining: const [Quantity(amount: 0, unit: Unit.grams)], + trips: trips, + ); + + expect(result, isEmpty); + }); + }); + + group("copy matches the page end-to-end (planner + distributor)", () { + test("ingredient with owned stock spanning two units is not dropped from the copy", () { + // Reachable regression case (issue #34, case 3). Garlic is needed as 4 pieces AND 100 g on the + // same day, with gramsPerPiece = 25 and both a pieces and a grams product (units stay separate). + // The user owns 100 g. The on-screen page subtracts the stock once (single grams pool), so it + // still shows a positive remaining. The old planner subtracted the full 100 g from EACH unit, + // zeroed every event, produced no trip item, and the distributor then returned nothing -- so the + // ingredient vanished from the copy while the page still showed a "Need". This test runs the real + // page flow (computeRemainingQuantities -> planShoppingTrips -> distributeRemainingAcrossTrips) + // and asserts the copied per-trip amounts sum to exactly what the page shows. + Ingredient garlic = Ingredient( + id: "garlic", + name: "Ajo", + gramsPerPiece: 25, + products: [ + _grams(quantityPerItem: 150), + Product(link: "https://example.com/pc", unit: Unit.pieces, quantityPerItem: 1), + ], + ); + const List required = [Quantity(amount: 4, unit: Unit.pieces), Quantity(amount: 100, unit: Unit.grams)]; + + List pageRemaining = computeRemainingQuantities( + ingredient: garlic, + requiredQuantities: required, + ownedAmount: 100, + ownedUnit: Unit.grams, + ); + + // The page shows a positive remaining (200 g of need minus 100 g owned = 100 g still to buy). + double pageGrams = pageRemaining.fold(0, (double sum, Quantity q) => sum + (garlic.toGrams(q) ?? 0)); + expect(pageGrams, 100); + + Map> timeline = { + "garlic": [CookingEvent(dayIndex: 0, quantities: required)], + }; + List trips = planShoppingTrips( + cookingTimeline: timeline, + ingredients: [garlic], + ownedAmounts: const {"garlic": OwnedStock(amount: 100, unit: Unit.grams)}, + ); + + List allocations = distributeRemainingAcrossTrips(ingredient: garlic, pageRemaining: pageRemaining, trips: trips); + + // The ingredient must appear in the copy, and the copied amounts must sum to the page total. + expect(allocations, isNotEmpty); + double copyGrams = 0; + for (TripAllocation a in allocations) { + for (Quantity q in a.quantities) { + copyGrams += garlic.toGrams(q) ?? 0; + } + } + expect(copyGrams, pageGrams); + }); + }); +}