fix: buy each perishable on the first trip or the latest trip before use - #78
Merged
Merged
Conversation
The planner gave each perishable the first chosen trip inside its freshness window, which could be an in-between trip. A yogurt for day 15 with a window of [1, 2] went on trip 1, although the user shops on trip 2 anyway and the yogurt is fresher from there. The first trip is usually a home delivery, so it takes everything that it keeps fresh. The other perishables now go on the latest trip in their window, as close to their use as possible. The assignment runs after the trip set is final, so it never adds a trip. It can empty the first greedy trip when trip 0 is added later and keeps all of its items fresh. Closes #77 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Explain why a perishable goes on the first trip when it keeps from there and on the latest trip in its window otherwise: home delivery first, then buy close to use. Record the rejected alternatives and the one case that drops a trip. Update the test count in AGENTS.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
guplem
commented
Oct 3, 2026
guplem
left a comment
Owner
Author
There was a problem hiding this comment.
The new rule is sound: every perishable lands inside its freshness window, lastWhere cannot throw because the greedy pass put a trip in every window, and the fallback and the maxWeekIndex cap behave as before. Consumers compare against trips.first.weekIndex, so a trip that disappears breaks nothing.
- [Required] A later trip can survive for a pantry item alone (
multi_trip_planner.dart, non-perishable loop). Non-perishables get their trip before the trip set is final, in timeline key order. Timeline{milk: day 10 (keeps 30 days), salt: day 10, pepper: day 3}: milk takes trip 1, salt rides trip 1, pepper adds trip 0, then the new pass moves milk to trip 0. Trip 1 now holds only salt, so the user makes an extra trip for an item that keeps forever, and the result depends on map order. Fix: in the final pass, give every non-perishablechosenTrips.first, and add a test that expects one trip. - [Suggestion]
_itemsByTripin the tests drops amount, unit andfreezeOnArrival, so the new tests cannot catch a wrong amount or a lost freeze flag. Include them. - [Suggestion] No test covers one-trip mode (
assumeFreezerForFreezable: true), the main place where trip 0 appears after the greedy pass. Add one: freezable chicken for day 14 plus non-freezable cheese for day 10 (keeps 30 days) end on one trip. - [Suggestion] ADR 0014 line 87 and ADR 0015 line 66 claim the plan is always the minimum number of trips. That holds for perishables only; a trip 0 added later can make the plan bigger. Limit the claim.
- [Nitpick] The planner doc comment (lines 49-61) and the new comments say "keeps it fresh" and "latest trip that still gets the item fresh". The code checks the
[earliestWeek, latestWeek]window, and a fallback window can hold a trip that does not keep the item fresh. Describe the window. - Issue #77 criterion 1 ("the trip set stays the same") is now reworded to allow a later trip to disappear.
Non-perishables got their trip before the trip set was final, in the key order of the timeline. A pantry item could take a later greedy trip before another pantry item added trip 0. The final pass then moved the perishables of that later trip to trip 0, and the later trip stayed open for the pantry item alone. The user made an extra trip for an item that keeps forever, and the plan depended on map order. The non-perishable step now only decides whether trip 0 is needed. Each non-perishable gets the first trip after the trip set is final. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The trip helper of the tests printed only the ingredient and its cook days. A wrong amount, a wrong unit or a lost freeze flag passed every test of the trip assignment. The helper now prints all of them. One-trip mode (assumeFreezerForFreezable) is the main place where trip 0 appears after the greedy pass, because a frozen item pins it. A new test covers it: a cheese that trip 0 keeps fresh joins the frozen chicken on trip 0, and the later greedy trip disappears. Without the final pass of each perishable, the plan keeps two trips, so the test guards the new behavior of this branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The doc comment of planShoppingTrips said that the planner finds the latest trip that gets the item fresh and that the greedy picks the minimum number of trips. The code checks the [earliestWeek, latestWeek] window, the greedy picks the trips for perishables only, and a trip 0 added later can make the plan bigger than needed. A fallback window can also hold a trip that does not keep the item fresh, so the comments now say "inside the window of the event" instead of "keeps it fresh". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both ADRs said that the plan always has the minimum number of trips. The greedy is optimal only for the freshness windows of the perishables. A trip 0 that non-perishables or freezing events add later can make the plan bigger than needed, and the final assignment recovers one trip only in one case. ADR 0014 also records that non-perishables now go on the first trip after the trip set is final, and why the old order-dependent ride-along was rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The review fixes of this branch add three trip planner tests, so the suite now has 1073 tests and the command table showed a stale number. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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 #77
The shopping planner now puts each perishable on the first trip (usually a home delivery) when it stays fresh from there. Otherwise it goes on the latest trip that keeps it fresh, so you buy it close to its use. Before, it went on the earliest trip that kept it fresh, which could be an in-between trip.
multi_trip_planner.dart): the greedy pass still picks the trips. After the trip set is final, a new pass gives each perishable its trip with_tripForPerishable. Freeze-on-arrival items and the "no trip keeps it fresh" fallback do not change. Non-perishables now get the first trip after the trip set is final, so a later trip never stays open for a pantry item alone.AGENTS.mdtest count is 1073; ADR 0014 and 0015 no longer claim the plan is always the minimum number of trips.Test plan
flutter analyze, full suite (1073 tests) pass locally🤖 Generated with Claude Code