Make the copied shopping list match the on-screen amounts and units - #39
Merged
Conversation
The copied shopping list could show a different amount or unit than the on-screen "Need", even after #22 fixed owned items. Three causes, all in the copy/trip path, now fixed so copy always equals the page. Different unit: the page normalizes units (pieces to grams via gramsPerPiece, volume to grams via density) and the copy read the raw cooking timeline, so it could list "4 pieces" while the page showed "20 g". The copy no longer prints the planner's raw amounts. It takes the on-screen remaining and calls the new distributeRemainingAcrossTrips (trip_amount_distributor.dart) to spread that amount across the weeks the planner chose, keeping the on-screen unit. Rounding across trips: the page rounds each total once; the copy rounded each per-trip line, so lines could sum to +/-1 off the page total. The distributor uses a largest-remainder split, so per-trip lines are whole numbers that sum to exactly the page number. Owned stock counted per unit: a single stock was converted into every required unit and subtracted from each, over-subtracting when an ingredient is needed in two units at once. The new computeRemainingQuantities (owned_amount.dart) draws the stock down from one shared grams pool, so it is subtracted only once. Covers every acceptance criterion of #34. Updates ADR 0014 to document the copy-vs-page reconciliation and the single owned pool. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
guplem
added
the
waiting-for-human-check
The multi-trip planner subtracted the full owned stock from each unit independently. When one ingredient was needed in two units at once (for example 4 pieces and 100 g, with gramsPerPiece set and a pieces product), it removed the whole stock from both lines, zeroed every cooking event, and emitted no trip item. The distributor then saw no week for that ingredient and returned nothing, so the ingredient vanished from the copied list while the on-screen page still showed a positive "Need". The on-screen list already subtracted correctly, via a single grams pool drawn down across the units once (computeRemainingQuantities). Extract that logic into a shared OwnedStockConsumer and have the planner use it too, so the planner and the page are one source of truth and can never disagree on how much is still needed. The planner's trip-assignment and shelf-life logic are unchanged. The consumer keys its grams-path branch on whether the owned stock has a grams path (fixed at construction), not on the live pool level: once the pool drains to 0, later needs must stay on the grams path (subtracting nothing more), not fall back to a per-unit conversion that would re-subtract the full stock. Tests: add a planner test and an end-to-end (page + planner + distributor) test proving an owned stock spanning two units is subtracted once and the ingredient stays in the copy. Reframe the distributor's all-zero-weight test as the defensive-only path it guards, since the planner never emits a zero-amount trip item in the real flow. Update ADR 0014 to record that the planner now shares the single-pool subtraction and no longer drops an ingredient the page still needs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
guplem
commented
Jul 22, 2026
guplem
left a comment
guplem
left a comment
Owner
Author
There was a problem hiding this comment.
Reviewed by an automated pass (deep review + a fix round). One real bug was found and fixed on the branch; the change is now correct and ready for a human check.
What is done well
- Right root-cause framing: the page is treated as the single source of truth, and the copy only borrows the planner's chosen weeks to split the same rounded total. So copy == page by construction.
- The largest-remainder split is sound (traced 33.3/33.3/33.4 -> 33/33/34, single trip, zero-need trip, all-equal weights, already-whole totals): per-trip whole numbers always sum to the on-screen total, no negative or spurious amounts.
computeRemainingQuantitiescorrectly subtracts owned stock from one shared grams pool instead of once per unit. Tests assert real values, and the different-unit and multi-trip-rounding acceptance cases are both covered. ADR 0014 was updated.
Bug found and fixed on this branch (was Required)
- In multi-trip mode, an ingredient whose owned stock spanned two units could be dropped from the copy while the page still showed a positive Need. Cause: the page used the new shared-pool subtraction, but the planner still subtracted the full owned stock into each unit separately, zeroed every event, emitted no trip item, and the distributor then dropped the ingredient.
- Fixed at the root: the planner now shares the same single-pool subtraction as the page (extracted into
OwnedStockConsumerinowned_amount.dart), so planner and page can never disagree. A new planner test and a page+planner+distributor end-to-end test cover it. This also resolves the related per-trip pack-split divergence. Full suite green (629 tests).
Verdict: approved pending human review. CI analyze-and-test gate must be green before merge.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #34
The copied shopping list could show a different amount or unit than the page. Now the copy is built from the same on-screen number, so they match by construction.
Root cause: the page normalizes units and rounds each ingredient's total once, but the copy printed the planner's raw per-day amounts. That caused three mismatches (different unit, per-trip rounding drift, and owned stock subtracted once per unit).
trip_amount_distributor.dart: spreads an ingredient's on-screen remaining across the weeks the planner picked, weighting by each week's raw need, using a largest-remainder split so per-trip lines are whole numbers that sum to exactly the on-screen total, in the on-screen unit.owned_amount.dartcomputeRemainingQuantities: subtracts owned stock from a single shared grams pool drawn down across units, instead of converting the full stock into every unit and subtracting from each (fixes over-subtraction).shopping_page.dart: remaining amounts and the copy text now use the two helpers above; the dead_ownedInUnitpath was removed.Key decision
The planner is unchanged; it still owns trip assignment and shelf-life correctness (all its tests stay green). The on-screen remaining is treated as the single source of truth, and the copy only borrows the planner's chosen weeks to split that number.
Test plan
flutter analyzepassesflutter test test/passes (627 tests: 611 prior + 16 new)🤖 Generated with Claude Code