fix(quick-router): match switch/plug/outlet tokens as whole words in home_status - #842
Conversation
|
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)
📝 WalkthroughWalkthroughThe quick router now matches switch, plug, and outlet terms as whole whitespace-delimited tokens. Tests verify substring collisions abstain while genuine device-status queries continue routing correctly. ChangesHome status routing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
matedev01
left a comment
There was a problem hiding this comment.
LGTM — switch/plug/outlet tokens in home_status_target now match as whole words, so 'what is the switchboard status' no longer misroutes. 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).
|
Reviewed and this looks good — approving in spirit — but it now conflicts with main in crates/genie-core/src/tools/quick.rs (several other whole-word status fixes landed just ahead of this one). Please rebase onto main and push; I'll merge once it's mergeable and green. |
…home_status The switch/plug/outlet branch of home_status_target used a substring contains_any, so "switchboard"/"switchgear", "plugin" and "unplugged"/"earplugs" all fired it. A single-word target collapses to the canonical "switches", so "what is the switchboard status" confidently reported every switch in the house, and the multi-word cases leaked the raw utterance out as a home_status entity instead of abstaining. Match the tokens as whole words, exactly as the sibling ice/iron/cooktop (GeniePod#792), car (GeniePod#772), fan/fireplace (GeniePod#746) and cover/gate (GeniePod#802) groups already do.
2e852a0 to
d54e606
Compare
matedev01
left a comment
There was a problem hiding this comment.
LGTM, rebased clean — switch/plug/outlet tokens in home_status_target now match as whole words. Verified: clippy -D warnings clean, quick-router tests (102) pass, fmt clean, live BFCL strict_accuracy 96.15% (no regression).
Summary
home_status_targetmatched its switch/plug/outlet device group with a substringcontains_any, so any word merely containing those letters fired it. Because a single-word target collapses to the canonical"switches","what is the switchboard status"confidently reported every switch and plug in the house; the multi-word cases leaked the raw utterance out as ahome_statusentity. Match the tokens as whole words instead. Closes #841.mainwhat is the switchboard statushome_status{entity:"switches"}is the switchgear okhome_status{entity:"switchgear ok"}is the plugin enabledhome_status{entity:"plugin enabled"}is the toaster unpluggedhome_status{entity:"toaster unplugged"}are the earplugs in the drawerhome_status{entity:"earplugs in the drawer"}Changes
contains_any(&target, &["switch", "switches", "plug", "plugs", "outlet", "outlets"])with asplit_whitespace().any(matches!(...))whole-word test — the identical shape already used by the sibling groups: ice/icy, iron and cooktop stove/oven (fix(quick-router): match iron and cooktop tokens as whole words in home_status #792), car (fix(quick-router): match car and light device names as whole words #772), fan/fireplace (fix(quick-router): match fan/fireplace as whole words, not substrings #746), cover/gate (fix(quick-router): match cover and gate tokens as whole words in home_status #802). Switch/plug/outlet was one of the two remaining substring groups (lock/door is being handled separately in fix(quick-router): match lock and door tokens as whole words in home_status #832, in a different branch of the same function — no overlapping hunk)."switches", a multi-word target is still returned verbatim.switch_and_outlet_status_match_whole_words_not_substrings, asserting the five collisions abstain and that six genuine phrasings (are the switches on,is the switch on,are the outlets on,check the outlet,is the kitchen plug on,are the kitchen plugs on) resolve to byte-identical entities.One branch condition changed; every other path in
home_status_targetis untouched.Real Behavior Proof
Tested profile / hardware (check all that apply):
jetsonraspberry_piportable_sbclaptopmacWhat I ran
x86_64 Linux dev machine (laptop profile),
rustc 1.94.0, branch cut from7c67906. The changed path is pure string routing insidequick::route— no audio, Home Assistant, or hardware dependency — soroute()tests exercise it end-to-end. I probedroute()over the collision and genuine phrasings against unmodifiedmainfirst, then wrote the regression test and confirmed it fails onmainbefore applying the fix.What I observed
Probe on unmodified
main:The new test fails on
main:After the fix: all five abstain; all six genuine phrasings return byte-identical entities to
main.Gate results on this branch:
cargo fmt --all -- --check— clean.cargo clippy --workspace --all-targets --locked -- -D warnings— clean.cargo test --workspace --locked --all-targets—tools::quick98 passed / 0 failed, genie-core lib 938 passed / 0 failed / 6 ignored, every other target green.cargo test --workspace --locked --doc— green.strict_accuracy: 96.15%(25/26) against the--min-strict 96floor, the single holdout being the documented two-callmulti-homework-timercase (quick-router multi-intent prompts emit a single, wrong-priority tool #533). Byte-identical tomain.--no-default-featuresclippy + test forgenie-core/genie-ctl— clean / green.On the
binary_size_budgetrelease test: it fails identically on unmodifiedmainwith my localrustc 1.94.0(6.80 MBagainst the6.8 MBbudget), so it is a toolchain artifact on my box, not a regression here. I measured both sides to be sure the change does not push it further:main= 7,131,736 bytes, this branch = 7,131,128 bytes — 608 bytes smaller, sincematches!over pre-split tokens compiles tighter than the sixcontainscalls it replaces.Jetson gap: no Jetson hardware and no
aarch64cross toolchain on this box (cargo check --target aarch64-unknown-linux-gnustops atcc-rs: failed to find tool "aarch64-linux-gnu-gcc"), so I could not run the cross build locally — theCross-compile (aarch64 / Jetson)CI job covers it. The change is a pure&strtoken comparison with no platform-dependent behaviour, no I/O, and no new dependency, so it is architecture-independent by construction.Test plan
cargo test -p genie-core --lib switch_and_outlet_status_match_whole_words_not_substrings— passes here, fails onmain.cargo test -p genie-core --lib tools::quick— the other 97 router tests, including the sibling whole-word testsice_status_matches_whole_words_not_substrings,cooktop_status_matches_whole_words_not_substringsandcover_and_gate_status_match_whole_words_not_substrings, are unchanged.Notes for reviewers
"outlets"/"switches"/"plugs"in the token list even though whole-word matching makes them explicit plurals rather than substrings of themselves — dropping them would silently stop matching the plural forms, which several of the genuine cases above use.home_status_targetand do not conflict.Summary by CodeRabbit