fix(pantry-pro): address review feedback from #368 - #371
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>
🔀 Branch Merge CheckPR direction: ✅ Passed — |
✅ Community PR Path Check — PassedAll changed files are inside the |
✅ Ability Validation Passed |
🔍 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>
|
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
left a comment
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.
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