fix(quick-router): accept a leading please on forget commands - #924
Open
galuis116 wants to merge 1 commit into
Open
fix(quick-router): accept a leading please on forget commands#924galuis116 wants to merge 1 commit into
galuis116 wants to merge 1 commit into
Conversation
memory_forget_query matches its prefixes at token 0 and already stripped
a trailing "please", but never a leading one, so "please forget the wifi
password" matched no prefix.
Unlike the sibling leading-"please" gaps, missing here does not abstain.
`route` runs memory_forget_query and then memory_recall_query, whose
household/note/semantic matchers are contains-based — so the utterance
was claimed by recall and the router emitted a confidently wrong tool:
"please forget the wifi password"
-> memory_recall{query: "please forget the wifi password", limit: 3}
expected
-> memory_forget{query: "wifi password"}
A delete request answered as a lookup is worse than a fallthrough: the
secret is read back rather than erased, the query carries the politeness
token and matches no stored fact, and the user gets a plausible-looking
answer with no signal that the delete never happened.
Strip a single leading "please " before the prefix loop, mirroring
scene_or_routine_activation_request and play_media_request. The
bare-pronoun abstention (that/it/this) and the trailing-"please" trim
inside the loop are untouched, so "please forget about it" still
abstains rather than deleting the literal word.
Closes GeniePod#921
📝 WalkthroughWalkthroughThe deterministic router now handles leading “please” in memory forget and delete commands. Tests verify equivalent query extraction for polite and bare forms while preserving abstention for bare-pronoun references. ChangesMemory Forget Routing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
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 #921.
memory_forget_querymatches its prefixes at token 0 and alreadystripped a trailing
please, but never a leading one — soplease forget the wifi passwordmatched no prefix.Unlike the sibling leading-
pleasegaps, missing here does not abstain.routerunsmemory_forget_queryand thenmemory_recall_query, whosehousehold / note / semantic matchers are
contains-based. The utterance wasclaimed by recall, and the router emitted a confidently wrong tool:
Please forget the wifi passwordmemory_recall{"please forget the wifi password"}memory_forget{"wifi password"}Please forget my old locker combinationmemory_forget{"old locker combination"}Please delete what you know about my carmemory_forget{"my car"}Why this one is worse than the sibling gaps
A missed route costs a fallthrough to the LLM, which can still recover. Here
the user asked to delete a memory and the router reads it back instead:
please forget the wifi password, which is not a stored factand matches nothing useful,
happened.
A privacy-intent utterance silently becoming a retrieval is a correctness bug
with a safety edge, not just a coverage gap.
Changes
pleasebefore the prefix loop inmemory_forget_query, mirroringscene_or_routine_activation_requestandplay_media_request.that/it/this) and thetrailing-
pleasetrim inside the loop are untouched, soplease forget about itstill abstains rather than deleting the literal word.forget_command_accepts_a_leading_please.Real Behavior Proof
Tested profile / hardware (check all that apply):
jetsonraspberry_piportable_sbclaptopmacWhat I ran
x86_64 Linux laptop (Ubuntu 22.04 LTS, kernel 6.8.0-136-generic,
rustc 1.96.0 / cargo 1.96.0). No Jetson available to me — see the validation
gap below.
Plus a temporary probe binding
route()directly, so I could read the actualemitted
ToolCallon bothmainand this branch — the whole point of this bugis which tool fires, so an
is_none()-style assertion would not have shownit. The probe was removed before committing; only the permanent regression test
remains.
What I observed
Probe output on
main— the delete request answered as a lookup:Probe output on this branch:
The polite call is now byte-identical to the bare call, and the bare-pronoun
abstention still holds under a leading
please.cargo test -p genie-core: 1005 lib tests passed, 0 failed, 6 ignored, andall integration suites green.
cargo clippy --all-targetsproduced nowarnings;
cargo fmt --checkproduced no diff.Validation gap
I could not verify on Jetson hardware. The equivalent verification path: this
change is confined to
crates/genie-core/src/tools/quick.rs, a purestring→
ToolCallfunction with no I/O, no async, no hardware dependency, andno model involvement. Its entire contract is which
ToolCallcomes out for agiven utterance, asserted directly above. The emitted call is byte-identical to
the one the bare utterance already produces and already dispatches on Jetson
today, so the memory subsystem downstream sees nothing new. Nothing in the diff
touches audio, the dashboard, or the model prompt.
Test plan
forget_command_accepts_a_leading_pleaseontomainalone andrun it — it fails, and fails loudly:
mainreturnsmemory_recall, notNone."please forget the wifi password", then "what's my wifi password" — confirm
the fact is gone rather than read back.
Notes for reviewers
arguments, not merely that some route exists — a partial fix that stripped
pleasefrom the tool selection but not from the query would still fail.is untouched.
please.