fix(quick-router): match lock and door tokens as whole words in home_status - #832
Conversation
…status home_status_target matched the lock/door group with a bare contains, so a common word that merely contains the token misrouted to home_status "locks" instead of abstaining: "is the clock on" (c[lock]) reported the door locks, and "out[door]" collided the same way. Mirrors the existing ice/iron/cooktop/cover whole-word fixes: match lock/locks/door/doors via split_whitespace().any(...). The multi-word "door lock"/"door locks" entries are redundant once the words match, and are dropped.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesLock and door status routing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/genie-core/src/tools/quick.rs`:
- Around line 5565-5575: Update the route assertions in the relevant test to
require route(utterance).is_none(), ensuring each unrelated query abstains
entirely rather than routing to another entity; also add an explicit “outdoor”
utterance case to the test inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f1a4e7df-fa5b-4c92-991d-376f26ce9ebf
📒 Files selected for processing (1)
crates/genie-core/src/tools/quick.rs
Strengthen the substring-collision assertions from "entity is not locks" to route().is_none(), so the test fails if any of these ever route through home_status with a different (garbled) entity. Add the "are the outdoor cameras on" case, which the substring door match misrouted to home_status "outdoor cameras".
|
Good call — applied in All four now abstain; |
matedev01
left a comment
matedev01
left a comment
There was a problem hiding this comment.
LGTM — lock/door tokens in home_status_target now match as whole words, so 'is the clock on' (c[lock]) no longer misroutes to door locks. Same substring-safety class as the prior whole-word fixes. Verified: clippy -D warnings clean, quick-router tests (99) pass, fmt clean, live BFCL strict_accuracy 96.15% (no regression).
2 participants
Summary
home_status_targetmatched its lock/door device group with a barecontains, so a word that merely contains "lock"/"door" as a substring misrouted tohome_status—"is the clock on"(c[lock]) reported the door locks, and"out[door]"collided the same way. Match the tokens as whole words, mirroring the existing ice/iron/cooktop/cover fixes. Closes #831.Changes
home_status_target, replace the substringcontains_any(&target, &["lock", "locks", "door lock", "door locks", "door"])with a whole-wordtarget.split_whitespace().any(|word| matches!(word, "lock" | "locks" | "door" | "doors")). The multi-word"door lock"/"door locks"entries are redundant once the words match and are dropped;"doors"is added so"are the doors locked"keeps resolving.lock_and_door_status_match_whole_words_not_substrings: three substring cases (clock/block) must not resolve to"locks"; four genuine lock/door queries (bare →"locks", named → full entity) must still resolve.Real Behavior Proof
Tested profile / hardware (check all that apply):
jetsonraspberry_piportable_sbclaptopmacWhat I ran
x86_64 Linux dev machine (laptop profile),
rustc 1.98.0-nightly, branch cut from7c67906. The changed path is pure string routing inquick::route— no audio/HA/hardware dependency — soroute()unit tests exercise it end-to-end; the validation gap is only that I did not run the voice loop on a device.main:cargo test -p genie-core --lib lock_and_door_statuscargo fmt --all -- --checkcargo clippy -p genie-core --all-targets --locked -- -D warningscargo test -p genie-core --libandcargo test -p genie-core --lib --no-default-featuresWhat I observed
Before the fix (new test against unmodified
main):i.e.
route("is the clock on")→home_status{entity:"locks"}.After the fix:
route("is the clock on")→ abstainsroute("is the wall clock right")→ abstainsroute("is the block heater on")→ abstains"are the doors locked"/"is the door locked"→home_status{entity:"locks"};"is the side door locked"→"side door";"is the garage door closed"→"garage door"Gate results: fmt clean; clippy clean under
-D warnings; lib tests 938 passed / 0 failed (default) and 840 passed / 0 failed (--no-default-features). The existing door/lock status tests ("Is the garage door closed?"→"garage door","Is the side door locked?"→"side door","What doors are unlocked?"→"unlocked doors") continue to pass.Test plan
cargo test -p genie-core --lib lock_and_door_status— the new case fails onmain, passes here.cargo test -p genie-core --lib status— the home_status suite (including the existing whole-word sibling tests) stays green.Notes for reviewers
This is the same substring→whole-word conversion applied to the neighbouring groups in #792 (ice/iron/cooktop) and #802 (cover/gate); the lock/door group was the remaining substring matcher in
home_status_target."outdoor"no longer trips thedoorarm, and"clock"/"block"no longer trip thelockarm, while"doors"was added to the token set so the bare-plural"are the doors locked"still collapses to"locks".Summary by CodeRabbit