fix(voice): match intent needles as whole words, not prefixes - #928
fix(voice): match intent needles as whole words, not prefixes#928philluiz2323 wants to merge 2 commits into
Conversation
The shared-room intent gate matched its command needles with a bare `contains`. GeniePod#854 gave the ambient-narration negation list a leading space so "sunset"/"returned" stopped standing in for "set"/"turn", but the right-hand side was left open — so the mirror-image words still defeated the filter, and the direct-request list was never bounded on either side. Ordinary room chatter was therefore classified as a command and forwarded to the LLM: "the playground was empty this morning after the storm passed through" -> Accept ("play") "the turnout for the school concert was much smaller than last year" -> Accept ("turn") "the setback in the negotiations was severe for everyone involved today" -> Accept ("set") "the musical was wonderful and everyone loved the songs performed tonight" -> Accept ("music") "the alarming news about the storm kept everyone awake last night" -> Accept ("alarm") "the assistants gathered in the hall before the ceremony began today" -> Accept ("assistant") All six now reject as ambient narration. That is the whole point of the gate: on a Jetson, every false accept spends LLM and tool budget on a conversation nobody addressed to the assistant, and sends it to the model. Replace `contains_any` with `contains_any_word`, which requires the needle to sit between non-alphanumeric boundaries. Boundaries rather than space padding because `normalize_transcript` deliberately keeps punctuation — a space-padded needle would miss "turn on the lights." and "hey, genie", both of which match today. The scan continues past a rejected occurrence, so "the playground and play music" still matches on the real "play". Needles lose their leading space and the plural/inflected forms that prefix matching used to cover for free are listed explicitly, so "alarms", "reminders", and "timers" still read as commands while "alarming" and "assistants" no longer do. Closes GeniePod#926
|
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)
📝 WalkthroughWalkthroughVoice intent detection now matches complete words and phrases instead of embedded substrings. Command and ambient-signal checks include explicit plural forms. Regression tests cover prefixes, punctuation, empty needles, and later valid matches. ChangesVoice intent boundary matching
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
🧹 Nitpick comments (1)
crates/genie-core/src/voice/intent.rs (1)
303-380: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the empty-needle guard; the current tests don't cover it.
The
ai_summaryand the line-range change details both state that the new regression tests cover "empty needles." The only related assertion is at line 371:contains_word_or_phrase("", "play"). This call uses an emptytext, not an emptyneedle. The actual empty-needle guard incontains_word_or_phrase(Line 237-239,if needle.is_empty() { return false; }) has no test that calls it with a non-emptytextand an emptyneedle.Separately, the new plural additions to the ambient-negation list (
timersandremindersat Line 154 and Line 157) have no test where they are the sole reason for the expected result. "the timer in the kitchen" (Line 339) passes because "kitchen" also matches; the test does not prove thetimerneedle works on its own. The same applies toreminders(only singular "reminder" is tested at Line 340).Add focused assertions for these cases:
🧪 Proposed test additions
assert!(contains_word_or_phrase( "the playground and play music", "play" )); + + // The empty-needle guard must reject unconditionally, independent of + // text content — otherwise str::match_indices("") would match at + // every position. + assert!(!contains_word_or_phrase("turn on the lights", "")); }for text in [ "hey genie, dim the lights.", "hey genie the lights are too bright", "any word on the weather", "my alarm did not go off", - "the timer in the kitchen", + "the timer in her old house rang loudly", "the reminder about the dentist", + "the reminders on her phone kept going off all day", "the thermostat is set too high in the living room", "the tv is still on in the bedroom", "the music in the garage is too loud right now", ] {As per the PR objectives, "Add focused regression tests that fail on the existing implementation and pass with the fix" and "Explicitly retain intended plural or inflected commands such as
alarms,reminders, andtimers" — the current tests don't isolatetimers/remindersfrom co-occurring needles, so a future regression in those specific entries would not be caught.🤖 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/voice/intent.rs` around lines 303 - 380, Add focused regression assertions in the existing word-boundary and intent test functions: call contains_word_or_phrase with non-empty text and an empty needle and verify it returns false, and add ambient narration cases where “timers” and “reminders” are the only matching needles so assess_transcript rejects them. Keep the existing command-acceptance coverage unchanged.
🤖 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.
Nitpick comments:
In `@crates/genie-core/src/voice/intent.rs`:
- Around line 303-380: Add focused regression assertions in the existing
word-boundary and intent test functions: call contains_word_or_phrase with
non-empty text and an empty needle and verify it returns false, and add ambient
narration cases where “timers” and “reminders” are the only matching needles so
assess_transcript rejects them. Keep the existing command-acceptance coverage
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dcacb07a-c56e-40fa-b85c-2b9d1f659bb6
📒 Files selected for processing (1)
crates/genie-core/src/voice/intent.rs
Follow-up to the review on GeniePod#928. Two gaps, both real. The empty-needle guard had no test. The existing assertion passes an empty *text*, which is a different branch; nothing called contains_word_or_phrase with a non-empty text and an empty needle. Without that guard match_indices("") yields a match at every position, so every needle would appear present. Now asserted directly, including through contains_any_word. The plural entries added to both lists were never the sole reason for any expected result — "the timer in the kitchen" passes on "kitchen". Each of timer, timers, alarms, reminders, thermostats, and lights now has a transcript where it is the only needle present, paired with a control sentence that differs only in that word and must reject. The control is asserted first, so a change that made everything accept cannot quietly pass the whole test. Also pinned the reason the explicit plurals are necessary rather than redundant: with both sides bounded, the singular needle no longer matches its own plural. Tests only — no behavior change.
|
Both points were correct — addressed in the latest commit. Empty needle. You are right that the existing assertion passes an empty text, not an empty needle, so the guard had no coverage. Added direct assertions with a non-empty text and an empty needle, and one through Plural entries in isolation. Also correct — Also added an assertion that a singular needle no longer matches its own plural, which is what makes the explicit plural entries necessary rather than redundant. |
1 participant
Summary
Closes #926. The shared-room intent gate matched its command needles with a bare
contains. #854 gave the ambient-narration negation list a leading space sosunset/returnedstopped standing in forset/turn— but theright-hand side was left open, and the direct-request list was never bounded on
either side.
So the mirror-image words still defeat the filter: an ordinary word that merely
starts with a command word is read as a command, and room chatter is forwarded
to the LLM.
the playground was empty this morning after the storm passed through" play"the turnout for the school concert was much smaller than last year" turn"the setback in the negotiations was severe for everyone involved today" set"the crowd remembered the old song and sang along together all evening"remember"the musical was wonderful and everyone loved the songs performed tonight" music"the alarming news about the storm kept everyone awake last night" alarm"the assistants gathered in the hall before the ceremony began today" assistant"This is the cost the gate exists to avoid. On a Jetson every false accept spends
LLM and tool budget on a conversation nobody addressed to the assistant — and
sends that conversation to the model.
Changes
contains_any_word/contains_word_or_phrase: a needle matches onlybetween non-alphanumeric boundaries (or string ends). Replaces
contains_anyin both
looks_like_direct_requestandlooks_like_ambient_narration.normalize_transcriptdeliberately keepspunctuation, so a space-padded needle would miss
turn on the lights.andhey, genie— both of which match today. A naive" lights "fix would havetraded this bug for a worse one.
the playground and play musicstill matches on the realplay. Stopping atthe first embedded hit would have introduced a second bug.
matching covered for free are now listed explicitly:
alarms,reminders,timersstay commands, whilealarmingandassistantsstop being.and ends on a char boundary, and an adjacent non-ASCII byte is treated as a
boundary rather than as a letter continuing the word.
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 and no microphone/wake-word device
available to me — see the validation gap below.
Plus a temporary probe calling
assess_transcriptdirectly, run onmainandagain on this branch over the same corpus, so the flip was observed rather than
inferred. The probe was removed before committing; only the permanent tests
remain.
What I observed
On
main— six natural narration lines classified as commands, with twocontrols confirming the filter itself works:
On this branch:
The pre-existing
assess_transcript_decisions_unchanged_across_corpusandrejects_ambient_narration_with_embedded_command_substrings(the #854 guard)both still pass unmodified — the fix tightens the rule without moving any
decision those corpora pin down.
Full suite: 1005 lib tests + all integration suites green on default
features and on
--no-default-features; clippy clean on both;cargo fmt --checkclean.Validation gap
I could not verify on Jetson hardware or with a live microphone. The equivalent
verification path:
assess_transcriptis a pure&str -> VoiceIntentDecisionfunction with no I/O, no async, no audio dependency and no model involvement.
Its entire contract is the decision it returns for a transcript, which the tests
assert directly. Nothing upstream (STT) or downstream (the wake-word / follow-up
flows that consume the decision) is touched by this diff — the change can only
turn a false
Acceptinto aReject, never the reverse for any input theexisting corpora cover.
Test plan
main— the first two fail.containing "playground", "turnout", or "musical" and confirm the assistant no
longer wakes into an LLM turn.
Notes for reviewers
still_accepts_the_real_command_words_the_needles_are_foris the test thatmatters most for review: tightening a filter is only safe if no genuine
command is lost, so it pins nine command forms including the trailing-keyword
and punctuated shapes that a space-padded fix would have broken.
is untouched.
Summary by CodeRabbit