fix(quick-router): accept a leading please on remind me/us commands - #923
fix(quick-router): accept a leading please on remind me/us commands#923joaovictor712 wants to merge 1 commit into
Conversation
The timer gate mixes a substring test with two prefix tests:
text.contains("timer")
|| text.starts_with("remind me ")
|| text.starts_with("remind us ")
so one politeness token was harmless on one arm and fatal on the others.
"please set a timer for 10 minutes" still routed — it contains "timer" —
while every polite reminder fell through to the LLM:
"please remind me to check the pasta in 5 minutes" -> ABSTAIN
"please remind me to take the trash out in 20 minutes" -> ABSTAIN
"please remind us to leave in 10 minutes" -> ABSTAIN
Strip a single leading "please " at the top of timer_request, before the
gate, mirroring scene_or_routine_activation_request, play_media_request,
and shopping_list_add_request.
Placing the strip ahead of reminder_style matters as much as the gate.
That flag is derived from the same two prefixes and selects the
task-first label scan, so a reminder that squeaked past the gate on the
"timer" arm with "please" still attached would be labelled "timer"
rather than "check the pasta" — a silently degraded reminder instead of
an absent one. The label fallback reads the stripped text for the same
reason, so a polite reminder with no extractable task still defaults to
"reminder".
Tokenization now runs on the stripped text, so every duration and label
index stays consistent. The trailing-"please" label trim is unchanged.
Closes GeniePod#920
|
Closing this PR automatically because @galuis116 currently has 3 open PRs in this repository. The current limit is 2 open PRs per contributor. Please keep only the highest-signal PRs open, finish or close existing work, then reopen this PR if it is still needed. Current PR: #923 |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe timer/reminder quick router now removes a leading ChangesTimer and reminder routing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Summary
Closes #920. The timer gate mixes a substring test with two prefix tests:
so one politeness token was harmless on one arm and fatal on the others.
please set a timer for 10 minutesstill routed — it containstimer— whileevery polite reminder fell through to the LLM.
Please remind me to check the pasta in 5 minutesset_timer{300, "check the pasta"}Please remind me to take the trash out in 20 minutesset_timer{1200, "take the trash out"}Please remind us to leave in 10 minutesset_timer{600, "leave"}Please remind me in 5 minutes to check the ovenset_timer{300, "check the oven"}Please set a timer for 10 minutesset_timer{600, "timer"}Changes
pleaseat the top oftimer_request, mirroringscene_or_routine_activation_request,play_media_request, andshopping_list_add_request.reminder_styleis derived fromthe same two prefixes and selects the task-first label scan, so stripping
only in front of the gate would not be enough: a reminder that squeaked
through on the
timerarm withpleasestill attached would be labelledtimerinstead ofcheck the pasta— a silently degraded reminder ratherthan an absent one, which is the harder failure to notice. Stripping before
both the gate and
reminder_stylefixes seconds and label together.index stays consistent. The label fallback reads the stripped text too, so a
polite reminder with no extractable task still defaults to
reminderratherthan
timer. The trailing-pleaselabel trim is untouched.reminder_accepts_a_leading_please.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 available to me — see the validation
gap below.
Plus a temporary probe binding
route()directly over the affected utterances,so I could read the emitted
ToolCall— specifically the label, since thedegraded-label failure mode above is invisible to a "does it route" check. The
probe was removed before committing; only the permanent regression test
remains.
What I observed
Probe output on this branch:
The first three printed
ABSTAINonmain. The fourth confirms the fallbacklabel is
reminder, nottimer— i.e. the strip landed ahead ofreminder_style, not merely ahead of the gate. The fifth confirms thatstripping politeness does not invent a timer for an utterance with no duration
to parse.
cargo test -p genie-core: 1005 lib tests passed, 0 failed, 6 ignored, andall integration suites green.
cargo clippy --all-targetsproduced nowarnings;
cargo fmt --checkproduced no diff.Validation gap
I could not verify on Jetson hardware. The equivalent verification path: this
change is confined to
crates/genie-core/src/tools/quick.rs, a purestring→
ToolCallfunction with no I/O, no async, no hardware dependency, andno model involvement. Its entire contract is which
ToolCallcomes out for agiven utterance, asserted directly above. The emitted call is byte-identical to
the one the bare utterance already produces and already dispatches on Jetson
today, so the timer subsystem downstream sees nothing new. Nothing in the diff
touches audio, the dashboard, or the model prompt.
Test plan
reminder_accepts_a_leading_pleaseontomainalone and run it— fails on the first case.
confirm the timer arms with the label "check the pasta" (not "timer") and
without an LLM round-trip.
Notes for reviewers
arguments for three utterances, so a partial fix that got the duration right
and the label wrong would still fail.
is untouched.
please.