fix(pantry-pro): address review feedback from #368 - #371
Merged
uzair401 merged 4 commits intoAug 31, 2026
Conversation
Refuse saves when inventory failed to load so a bad read cannot wipe stock. Merge only on exact or singular/plural names; keep fuzzy match for lookups. Read user_profile.md for recipe suggestions (diet/household), never write it. Co-authored-by: Cursor <cursoragent@cursor.com>
11 tasks
Contributor
🔀 Branch Merge CheckPR direction: ✅ Passed — |
Contributor
✅ Community PR Path Check — PassedAll changed files are inside the |
Contributor
✅ Ability Validation Passed |
Contributor
🔍 Lint ResultsFiles linted: ✅ Flake8 — Passed
|
Read the whole list when asked instead of truncating. Ask and apply expiry dates one item at a time for perishables, and answer when a specific item expires. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
Pushed follow-up live-test fixes onto this branch:
|
does_match() hardcoded a HOTWORDS tuple that duplicated whatever gets configured as this ability's dashboard trigger words -- a second source of truth that can drift from the first with nothing to notice if it does. No other ability in the repo overrides does_match() for this reason; invocation is left entirely to the dashboard. Unlike a couple of other abilities reviewed this same pass, HOTWORDS had no second job here (e.g. no in-conversation restart check reusing it), so removing it needed nothing else adjusted. Verified the three fixes this PR actually addresses are unaffected: strict add-time matching still keeps "almond milk" distinct from "milk", fuzzy lookup still resolves "eggs" to a stocked "egg", and the new item_date / full_list logic added in this PR still runs correctly. File still imports cleanly with no does_match anywhere in the class. Separately, and NOT changed here: _search_meals/_lookup_meal call session_tasks.get_async(), a method name that appears nowhere in the SDK reference docs (only .create() and .sleep() are documented there) and that no other ability in this repo uses -- everything else calls session_tasks.get() or plain requests.get(). That code isn't touched by this PR's diff at all; it's inherited from openhome-dev#368. Flagging it rather than guessing at a fix, since getting a live SDK method name wrong by assumption risks trading a real bug for a different one.
uzair401
approved these changes
Aug 31, 2026
uzair401
left a comment
uzair401
left a comment
Contributor
There was a problem hiding this comment.
Good follow-up, all three review notes from #368 are properly addressed — load/save safety, strict add-matching vs. fuzzy lookups, and the read-only user profile context. Removed the does_match() HOTWORDS duplication for SDK consistency with the rest of the repo. One separate, pre-existing item flagged in a commit note for awareness: _search_meals/_lookup_meal call session_tasks.get_async(), which isn't in the SDK docs and isn't used by any other ability here — inherited from #368, not part of this PR, worth a live check of the recipes feature. Approving.
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
2 participants
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
Follow-up to #368 (@Rizwan-095). Addresses the three review notes that landed after approval/merge:
_load()now returns a success flag. A missing file is fine (empty pantry); a failed read setsload_ok=False, the session refuses to mutate, and_save()will not delete-then-write over real inventory. Same guard on the background daemon._names_match_strict), soalmond milkno longer bumpsmilk. Fuzzy substring match stays for lookups (used/remove/update), preferring the longest name.user_profile.md— recipe LLM fallback reads it for diet/household context (read-only, never written).Test plan
milk, then addalmond milk— both should exist as separate itemsI used the eggswhen stock hasegg(or vice versa) — singular/plural still matchesuser_profile.mdmentions a diet preference — suggestions should respect itload_okguardMade with Cursor