Skip to content

fix(quick-router): "did you get my email" reports the physical mailbox - #875

Merged
matedev01 merged 3 commits into
GeniePod:mainfrom
kai392:fix/critical-issue-email-mailbox-misroute
Jul 29, 2026
Merged

fix(quick-router): "did you get my email" reports the physical mailbox#875
matedev01 merged 3 commits into
GeniePod:mainfrom
kai392:fix/critical-issue-email-mailbox-misroute

Conversation

@kai392

@kai392 kai392 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #874.

Root cause

home_status_target resolved the mailbox with a bare substring test:

if text.starts_with("did ") && text.contains("mail") {
    return Some("mailbox".into());
}

"mail" is a substring of email, voicemail, gmail and mailing, so any
"did …" question that merely mentions email matched. The utterance never has
to name a device, and because the quick-router returns a tool call the turn is
answered from the mailbox sensor instead of reaching the LLM at all.

Fix

Match the delivery words whole, the same shape the ice/iron/cooktop/
cover/lock branches in this function already use. The compounds that really
do name the physical mail (mailbox, mailman, and their plurals) are listed
explicitly, so every utterance the substring test resolved correctly still
resolves — including the "Did the mail come?" case already asserted in the
existing corpus.

One branch changed, plus a regression test. No behavior outside the "did … mail" branch is touched.

Impact

Seven natural utterances stop being answered with a mailbox reading and fall
through to the LLM, where they belong. No genuine mail-delivery question
regresses.

Risk / tradeoffs

Low. The change strictly narrows one predicate: everything it still matches was
already matched before, so the only behavior difference is the false positives
going away. The whitelist is a closed set of compounds (mailbox, mailman,
plurals) rather than a substring, so a future compound that should resolve —
say "mail slot" — needs an explicit entry; that is deliberate, and matches how
the sibling branches in this function are written.

Real Behavior Proof

  • I have built and run the affected code locally (or noted why I could not).
  • I have NOT verified on Jetson hardware, and I explain the equivalent verification path or validation gap below.

Tested profile / hardware:

  • laptop (Windows host — see the gap note below)
  • CI-only / docs-only

What I ran

I do not have a Jetson, and genie-core does not build on my Windows host
(libc::gmtime_r, tokio::signal::unix and friends are Unix-only), so the
routing change was exercised two ways.

1. Direct route() calls. I compiled the four quick-router source files
unmodifiedcrates/genie-core/src/tools/{quick,home_action,calc_input,number_words}.rs,
included by #[path], with only ToolCall and HomeActionKind stubbed — into a
throwaway binary and called tools::quick::route() on the utterances directly.
No logic was copied or reimplemented.

2. cargo test -p genie-core on Linux via GitHub Actions, plus the rest of
the matrix (cargo fmt, cargo clippy -D warnings, --no-default-features,
and the aarch64-unknown-linux-gnu release build), run on this exact branch
before opening this PR: kai392#1 — all eight
checks green. The follow-up commit that tightens the abstention assertion was
validated the same way: kai392#2, also green.

test tools::quick::tests::mail_status_matches_whole_words_not_substrings ... ok
test result: ok. 966 passed; 0 failed; 6 ignored; 0 measured; 0 filtered out

The test asserts route(utterance).is_none() for the email cases rather than
just "not home_status": every Some(call) the router returns is executed, so
only None proves the utterance actually reaches the LLM. That is the same bar
iron_verb_uses_do_not_report_iron_status sets.

The new test is tools::quick::tests::mail_status_matches_whole_words_not_substrings.
tools::quick::tests::routes_shopping_and_temperature_home_requests is the
pre-existing test that pins "Did the mail come?" -> home_status{entity:"mailbox"}
(quick.rs:5517); it still passes.

What I observed

Before, on main @ 9e54db4:

"Did you get my email?"                -> home_status {"entity":"mailbox"}
"Did you email the landlord?"          -> home_status {"entity":"mailbox"}
"Did I get any email from the school?" -> home_status {"entity":"mailbox"}
"Did the email go through?"            -> home_status {"entity":"mailbox"}
"Did the voicemail come through?"      -> home_status {"entity":"mailbox"}
"Did Sarah check her gmail?"           -> home_status {"entity":"mailbox"}
"Did the mailing list go out?"         -> home_status {"entity":"mailbox"}

After, same harness, same build:

"Did you get my email?"                -> (abstain, falls through to the LLM)
"Did you email the landlord?"          -> (abstain, falls through to the LLM)
"Did I get any email from the school?" -> (abstain, falls through to the LLM)
"Did the email go through?"            -> (abstain, falls through to the LLM)
"Did the voicemail come through?"      -> (abstain, falls through to the LLM)
"Did Sarah check her gmail?"           -> (abstain, falls through to the LLM)
"Did the mailing list go out?"         -> (abstain, falls through to the LLM)

"Did the mail come?"                   -> home_status {"entity":"mailbox"}
"Did the mail arrive?"                 -> home_status {"entity":"mailbox"}
"Did the mail come yet?"               -> home_status {"entity":"mailbox"}
"Did we get any mail today?"           -> home_status {"entity":"mailbox"}
"Did the mailman come?"                -> home_status {"entity":"mailbox"}
"Did anyone check the mailbox?"        -> home_status {"entity":"mailbox"}

Validation gap

This is a pure string-routing change with no HA, audio, or model dependency, so
a live Jetson run would exercise nothing the routing tests do not. What a
maintainer with hardware could still add is an end-to-end voice check that
"did you get my email" now reaches the LLM rather than the mailbox sensor.

Summary by CodeRabbit

  • Bug Fixes
    • Improved status-question routing for mail-related requests to avoid false positives from non-delivery phrases.
    • Ensured utterances containing words like “email,” “voicemail,” or “mailing list” are no longer misinterpreted as mailbox delivery status.
    • Kept correct handling for genuine mail-delivery questions, routing them appropriately.

…lbox

`home_status_target` matched the mailbox with a bare
`text.contains("mail")` behind the "did " prefix, so the substring inside
"email", "voicemail", "gmail" and "mailing list" was enough to fire it.
"Did you get my email?" — an utterance with no device word in it at all —
was answered deterministically with home_status{entity:"mailbox"}, and
never reached the LLM that could have handled it.

Match the delivery words whole instead, listing the compounds that really
do name the physical mail ("mailbox", "mailman", and their plurals) so
every utterance the substring test resolved correctly still resolves.
Same treatment the ice/iron/cooktop/cover/lock branches in this function
already got.
@github-actions github-actions Bot added the bug Something isn't working label Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@matedev01, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 28 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 73b347e6-7d71-4f6c-9bb0-cfe24a60ebcd

📥 Commits

Reviewing files that changed from the base of the PR and between c8fe506 and f44146f.

📒 Files selected for processing (1)
  • crates/genie-core/src/tools/quick.rs
📝 Walkthrough

Walkthrough

Mailbox status routing now matches whole-word delivery terms, preventing email-related substrings from triggering mailbox detection. Regression tests cover rejected non-delivery phrases and valid mail-delivery questions.

Changes

Mail status routing

Layer / File(s) Summary
Whole-word mail routing and regression coverage
crates/genie-core/src/tools/quick.rs
home_status_target recognizes delivery terms such as mail, mailbox, and mailman without substring matches, while regression tests verify valid and invalid routing cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: matedev01

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change stops substring matches on 'mail' from routing email-like phrases and preserves valid mailbox routing. [#874]
Out of Scope Changes check ✅ Passed The PR only updates quick-router mailbox matching and adds a regression test, staying within the linked issue scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the quick-router mailbox misrouting fix, using a concrete false-positive example from the change.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
crates/genie-core/src/tools/quick.rs (1)

5799-5809: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the promised plural compounds.

The matcher explicitly supports mailboxes and mailmen, but the regression cases only exercise singular forms. Add one case for each plural to protect that stated contract.

Proposed regression cases
         for utterance in [
             "Did the mail arrive?",
             "Did the mail come yet?",
             "Did we get any mail today?",
             "Did the mailman come?",
             "Did anyone check the mailbox?",
+            "Did the mailboxes get checked?",
+            "Did the mailmen come?",
         ] {
🤖 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 5799 - 5809, Add
regression utterances using the plural compounds “mailboxes” and “mailmen” to
the test loop near the existing mail-delivery cases, and assert each routes to
“home_status” with the “mailbox” entity. Preserve the current singular cases and
assertions.
🤖 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 5791-5796: Update the assertion for the relevant route test to
require route(utterance).is_none(), rather than accepting any tool call whose
name is not "home_status". Preserve the existing failure message or adjust it to
clearly indicate that utterance must abstain from routing entirely.

---

Nitpick comments:
In `@crates/genie-core/src/tools/quick.rs`:
- Around line 5799-5809: Add regression utterances using the plural compounds
“mailboxes” and “mailmen” to the test loop near the existing mail-delivery
cases, and assert each routes to “home_status” with the “mailbox” entity.
Preserve the current singular cases and assertions.
🪄 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: 0278a981-c0f6-48b3-a66f-dfc80a54f4c8

📥 Commits

Reviewing files that changed from the base of the PR and between 9e54db4 and 4f2cbb2.

📒 Files selected for processing (1)
  • crates/genie-core/src/tools/quick.rs

Comment thread crates/genie-core/src/tools/quick.rs
The first assertion only rejected a home_status route, but the server
executes every Some(call) the router returns, so any other tool call
would still short-circuit the LLM. Pin route() == None, matching the bar
iron_verb_uses_do_not_report_iron_status already sets.

@matedev01 matedev01 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — 'mail' was matched as a bare substring, so 'did you get my email/voicemail/gmail' misrouted to the physical mailbox. Fixes #874. Verified: clippy -D warnings clean, quick-router tests (108) pass, fmt clean, live BFCL strict_accuracy 96.15% (no regression).

@matedev01
matedev01 merged commit 19e7d78 into GeniePod:main Jul 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] quick-router: "did you get my email" reports the mailbox — "mail" is matched as a substring of "email"/"voicemail"/"gmail"

2 participants