fix(quick-router): require a status-shaped question for bare-appliance status reads - #862
Conversation
…e status reads
A command that merely contains an appliance noun phrase ("remind me to
check the water pressure", "remind me to buy a baby monitor") was
hijacked into a home_status read: the router answered with a device
report instead of abstaining so the LLM can set the reminder. Gate the
water-pressure/sump-pump/sous-vide/baby-monitor/hot-water branches on a
status-question shape, exactly as the iron block already does, keeping
"how hot is the water" alive via its own prefix.
📝 WalkthroughWalkthroughThe quick router now gates bare appliance status matches on status-shaped wording, preserves “how hot is the water” detection, and adds regression tests for command-like abstention and genuine status queries. ChangesHome status routing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/genie-core/src/tools/quick.rs (1)
5569-5603: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regressions for the remaining false-positive shapes.
Also test
"Did I buy a baby monitor?","Remind me to check the water pressure status", and"How hot should I make the water for tea?"; assert that none resolve tohome_status.🤖 Prompt for 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. In `@crates/genie-core/src/tools/quick.rs` around lines 5569 - 5603, Extend the test function bare_appliance_status_needs_a_status_shaped_question with the three specified utterances, asserting route(utterance) returns None for each. Keep these cases separate from the genuine status-question assertions and preserve the existing home_status expectations.
🤖 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 2486-2496: The status_shaped predicate in the target-selection
logic should only accept live device-state questions, not reminder or
historical/ownership queries. Tighten the logic around looks_like_status_query
and starts_with("did ") to reject reminder/history prefixes while preserving
valid live-status handling for water pressure, sump pump, and sous vide targets.
- Around line 2557-2562: In the water-heater routing condition, narrow the
text-based exception in the status-shaped check from the broad “how hot ” prefix
to the intended interrogative form such as “how hot is ”, while preserving the
existing water and hot checks. Add a negative regression covering advice
questions like “How hot should I make the water for tea?” to ensure they are not
routed to water heater.
---
Nitpick comments:
In `@crates/genie-core/src/tools/quick.rs`:
- Around line 5569-5603: Extend the test function
bare_appliance_status_needs_a_status_shaped_question with the three specified
utterances, asserting route(utterance) returns None for each. Keep these cases
separate from the genuine status-question assertions and preserve the existing
home_status expectations.
🪄 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: 01842f87-6251-4469-9090-00e5d2a0acae
📒 Files selected for processing (1)
crates/genie-core/src/tools/quick.rs
| let status_shaped = looks_like_status_query(text) || text.starts_with("did "); | ||
|
|
||
| if status_shaped && text.contains("water pressure") { | ||
| return Some("water pressure".into()); | ||
| } | ||
|
|
||
| if text.contains("sump pump") { | ||
| if status_shaped && text.contains("sump pump") { | ||
| return Some("sump pump".into()); | ||
| } | ||
|
|
||
| if text.contains("sous vide") { | ||
| if status_shaped && text.contains("sous vide") { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Tighten status_shaped to require a live device-state question.
looks_like_status_query matches any " status" substring, and starts_with("did ") accepts history/ownership questions. For example, "Remind me to check the water pressure status" and "Did I buy a baby monitor?" can still return home_status targets instead of reaching reminder or memory handling. Reject reminder/history prefixes or use a stricter live-status predicate.
Also applies to: 2518-2519
🤖 Prompt for 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.
In `@crates/genie-core/src/tools/quick.rs` around lines 2486 - 2496, The
status_shaped predicate in the target-selection logic should only accept live
device-state questions, not reminder or historical/ownership queries. Tighten
the logic around looks_like_status_query and starts_with("did ") to reject
reminder/history prefixes while preserving valid live-status handling for water
pressure, sump pump, and sous vide targets.
| // "how hot is the water" is a genuine water-heater reading whose "how " | ||
| // prefix none of the gate shapes cover, so admit it alongside them here. | ||
| if (status_shaped || text.starts_with("how hot ")) | ||
| && text.contains("water") | ||
| && text.contains("hot") | ||
| { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Narrow the how hot exception to water-temperature questions.
text.starts_with("how hot ") also matches advice requests such as "How hot should I make the water for tea?", routing them to water heater. Require the intended interrogative form (for example, "how hot is …") rather than the broad prefix, and add a negative regression.
🤖 Prompt for 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.
In `@crates/genie-core/src/tools/quick.rs` around lines 2557 - 2562, In the
water-heater routing condition, narrow the text-based exception in the
status-shaped check from the broad “how hot ” prefix to the intended
interrogative form such as “how hot is ”, while preserving the existing water
and hot checks. Add a negative regression covering advice questions like “How
hot should I make the water for tea?” to ensure they are not routed to water
heater.
matedev01
left a comment
There was a problem hiding this comment.
Confirmed both of CodeRabbit's findings directly against route() (added throwaway tests, not just reading the code) — both defeat this PR's own goal of requiring a genuine status question:
route("remind me to check the water pressure status")→home_status{entity:"water pressure"}— should abstain for the reminder. looks_like_status_query's bare.contains(" status")doesn't require a live-status question shape, so a reminder that happens to say the word "status" still qualifies.route("did i buy a baby monitor")→home_status{entity:"baby monitor"}— should abstain for the purchase-history question. The barestarts_with("did ")accepts any "did ..." phrasing, not just live-state questions ("did i leave the iron on").route("how hot should i make the water for tea")→home_status{entity:"water heater"}— should abstain for the advice question. Thestarts_with("how hot ")exception is too broad, same class as the other two.
All three are the exact regression this PR is meant to close for the bare-appliance branches — worth tightening before merge rather than carrying the gap forward under a title that says it's fixed.
Summary
The bare-appliance branches in
home_status_target— water pressure, sump pump, sous vide, baby monitor, and the water+hot pair — key on the appliance noun phrase alone, with no status-question shape required. Unlike the multi-word household-scenario phrases above them, a command can plausibly contain the noun, and keying on the noun alone hijacks that write into a status read: the router answers with a device report instead of abstaining so the LLM can set the reminder. A wrong tool fires and the reminder is silently never set. Closes #861.mainremind me to check the water pressurehome_status{entity:"water pressure"}remind me to test the sump pumphome_status{entity:"sump pump"}remind me to buy a baby monitorhome_status{entity:"baby monitor"}remind me to buy a sous videhome_status{entity:"sous vide"}how do i cook sous videhome_status{entity:"sous vide"}remind me to buy hot water bottleshome_status{entity:"water heater"}Changes
status_shaped = looks_like_status_query(text) || text.starts_with("did ")once, and require it on the five bare-appliance branches. This is exactly the gate the sibling iron block already applies for exactly this reason ("remind me to iron my shirt" used to hijack the iron branch — see its comment); these branches were the same bug without the gate."how hot "prefix, so the genuine reading "how hot is the water" (whosehowprefix none of the gate shapes cover) keeps resolving towater heater.Check the water pressure(check),Is the sump pump running?/Is the sous vide on?/Is the baby monitor on?/Is the water hot?(is),What's the current water pressure?(what). Thedid-prefixed branches (car locks, package, mailbox, stove) and the multi-word scenario phrases above are untouched.bare_appliance_status_needs_a_status_shaped_question: the six command phrasings abstain, and six genuine status questions (includingHow hot is the water?) keep their exact entities.Real Behavior Proof
Tested profile / hardware (check all that apply):
jetsonraspberry_piportable_sbclaptopmacWhat I ran
x86_64 Linux dev machine (laptop profile),
rustc 1.97.1, branch cut from9bff670. 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 command-shaped utterances containing each appliance noun against unmodifiedmainfirst, then wrote the regression test and confirmed it fails before applying the fix.What I observed
Probe on unmodified
main(each should abstain; each firedhome_statusinstead):Repro before the fix (fix reverted, test kept):
After the fix: all six command phrasings abstain; all six genuine status questions keep their exact entities.
Gate results on this branch:
cargo fmt --all -- --check— clean.cargo clippy --workspace --all-targets --locked -- -D warnings— clean;--no-default-featuresclippy forgenie-core/genie-ctl— clean.GENIE_RUN_RELEASE_TESTS=1 cargo test --workspace --locked --all-targets—tools::quick102 passed / 0 failed, genie-core lib 957 passed / 0 failed / 6 ignored, and every other target green, including the release-gatedtool_dispatch_testtarget (withbinary_size_budgetpassing).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-featurestest forgenie-core/genie-ctl— green (one unrelated flake under full-suite load,llm::provider::tests::loopback_127_range_allowed_without_remote_flag, passes clean in isolation and is untouched by this diff).genie-corebinary: 7,078,648 bytes vs. the 7,078,520-bytemainbaseline built with the same toolchain — +128 bytes for the added gate bool.Jetson gap: no Jetson hardware and no
aarch64cross toolchain on this box, so the cross build is left to theCross-compile (aarch64 / Jetson)CI job. The change is a pure&strprefix/contains gate 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 bare_appliance_status_needs_a_status_shaped_question— passes here, fails onmain(panics on the first command phrasing, which routes tohome_status).cargo test -p genie-core --lib tools::quick— the existingroutes_household_status_targetsassertions for these branches (Check the water pressure,Is the sump pump running?,Is the sous vide on?,Is the baby monitor on?,Is the water hot?,What's the current water pressure?) are unchanged and still pass, along with the other router tests (102 total).Notes for reviewers
speed limit(its misroute is a context question, not a shape question — "What's the speed limit here?" is already status-shaped),printer ink(the command form is intercepted bymemory_recallearlier inroute(), so a gate here changes nothing observable), orbaby breathing/nursery air quality(no realistic command phrasing contains them). Happy to widen if you'd rather gate every bare-noun branch symmetrically.Summary by CodeRabbit