Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions crates/genie-core/src/tools/quick.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2790,6 +2790,33 @@ fn timer_request(text: &str) -> Option<(u64, String)> {
return None;
}

// A leading cancel/stop verb asks to END a timer, not start one: the
// duration in "cancel the 10 minute timer" / "stop the 5 minute timer"
// *names* which timer, but the parse below read it as a request and did the
// exact opposite of what was asked — the cancellation itself started a
// fresh timer. There is no deterministic cancel tool, so abstain and let
// the LLM ground it. Only the leading verb is gated ("remind me to stop
// the dryer in 10 minutes" is still a set request), and a bare "cancel the
// timer" (no duration) already abstained via the parse below.
let is_cancel_command = matches!(
text.split_whitespace().next(),
Some(
"cancel"
| "stop"
| "end"
| "pause"
| "resume"
| "delete"
| "remove"
| "clear"
| "dismiss"
)
) || text.starts_with("turn off ")
|| text.starts_with("shut off ");
if is_cancel_command {
Comment on lines +2801 to +2816

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Handle leading politeness before matching cancellation verbs.

please cancel the 10 minute timer does not match this guard because the first token is please; the duration parser can then still emit set_timer, recreating the cancellation bug. Strip a leading politeness prefix for this check and add a regression case.

Proposed fix
+    let command = text.strip_prefix("please ").unwrap_or(text);
     let is_cancel_command = matches!(
-        text.split_whitespace().next(),
+        command.split_whitespace().next(),
         Some(
             "cancel"
             // ...
         )
-    ) || text.starts_with("turn off ")
-        || text.starts_with("shut off ");
+    ) || command.starts_with("turn off ")
+        || command.starts_with("shut off ");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let is_cancel_command = matches!(
text.split_whitespace().next(),
Some(
"cancel"
| "stop"
| "end"
| "pause"
| "resume"
| "delete"
| "remove"
| "clear"
| "dismiss"
)
) || text.starts_with("turn off ")
|| text.starts_with("shut off ");
if is_cancel_command {
let command = text.strip_prefix("please ").unwrap_or(text);
let is_cancel_command = matches!(
command.split_whitespace().next(),
Some(
"cancel"
| "stop"
| "end"
| "pause"
| "resume"
| "delete"
| "remove"
| "clear"
| "dismiss"
)
) || command.starts_with("turn off ")
|| command.starts_with("shut off ");
if is_cancel_command {
🤖 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 2801 - 2816, Update the
cancellation detection around is_cancel_command to remove a leading politeness
prefix such as “please” before matching cancellation verbs and existing “turn
off”/“shut off” forms, while preserving normal command handling. Add a
regression case covering “please cancel the 10 minute timer” and verify it does
not fall through to set_timer.

return None;
}

// "remind me/us to <task> in <duration>" phrasing puts the task clause before
// the duration; only these utterances get the task-first label scan so plain
// "<X> timer …" phrasings keep their existing named-timer handling.
Expand Down Expand Up @@ -6647,6 +6674,37 @@ mod tests {
assert_eq!(call.arguments["seconds"], 900);
}

#[test]
fn cancel_verb_timer_commands_abstain_instead_of_setting_a_timer() {
// A leading cancel/stop verb asks to END a timer. The duration in "cancel
// the 10 minute timer" only *names* which timer; timer_request read it as
// a request and did the exact opposite of what was asked — the
// cancellation itself started a fresh 10-minute timer. There is no
// deterministic cancel tool, so these must abstain for the LLM.
for utterance in [
"Cancel the 10 minute timer.",
"Stop the 5 minute timer.",
"Leo: cancel my 10 minute timer",
"delete the 20 minute timer",
"pause the 10 minute timer",
"turn off the 10 minute timer",
] {
assert!(
route(utterance).is_none(),
"{utterance:?} is a cancellation and must abstain, not set a new timer"
);
}

// Genuine set requests are untouched, including ones whose *label*
// merely contains a cancel word mid-utterance.
let call = route("set a 10 minute timer").unwrap();
assert_eq!(call.name, "set_timer");
assert_eq!(call.arguments["seconds"], 600);
let call = route("remind me to stop the dryer in 10 minutes").unwrap();
assert_eq!(call.name, "set_timer");
assert_eq!(call.arguments["label"], "stop the dryer");
}

#[test]
fn routes_fractional_duration_timer() {
// Regression: "half an hour" used to skip "half" and read "an hour" as a
Expand Down
Loading