Skip to content
Merged
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
71 changes: 71 additions & 0 deletions src/agent/routine_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1630,6 +1630,11 @@ fn build_lightweight_prompt(
"Do not claim you lack messaging integrations or ask the user to set one up when \
a plain reply is sufficient.\n",
);
full_prompt.push_str(
"Return the final user-facing notification as normal assistant text. Do not use the \
`message` tool for the routine's primary delivery unless the task explicitly requires \
an extra follow-up or attachment; even then, still return a concise human-readable summary.\n",
);
}

if !use_tools {
Expand Down Expand Up @@ -1717,6 +1722,7 @@ fn handle_text_response(
total_input_tokens: u32,
total_output_tokens: u32,
) -> Result<(RunStatus, Option<String>, Option<i32>), RoutineError> {
let content = strip_internal_tool_call_text(content);
let content = content.trim();

// Empty content guard — carry consumed tokens so the retry loop can
Expand Down Expand Up @@ -1748,6 +1754,34 @@ fn handle_text_response(
))
}

/// Strip internal `[Called tool ...]` and `[Tool ... returned: ...]` markers
/// from routine summaries before they are persisted or delivered to channels.
fn strip_internal_tool_call_text(text: &str) -> String {
let result = text
.lines()
.filter(|line| {
let trimmed = line.trim();
!((trimmed.starts_with("[Called tool ") && trimmed.ends_with(']'))
|| (trimmed.starts_with("[Tool ")
&& trimmed.contains(" returned:")
&& trimmed.ends_with(']')))
Comment on lines +1764 to +1767

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The logic for stripping tool markers is duplicated in strip_internal_tool_call_text within src/agent/routine_engine.rs and src/agent/dispatcher.rs. This duplication should be refactored into a shared utility function to ensure consistency and maintainability. When implementing the shared utility, ensure that distinct parsing conditions are separated into individual checks to improve code clarity and robustness.

References
  1. Separate checks for distinct conditions to improve code clarity and robustness, particularly when parsing text.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 96a8c5d — extracted to src/agent/text_util.rs with separate is_called_tool_marker() / is_tool_result_marker() helpers. Duplicate tests in dispatcher.rs removed in 2af4fa1.

})
.fold(String::new(), |mut acc, s| {
if !acc.is_empty() {
acc.push('\n');
}
acc.push_str(s);
acc
});

let result = result.trim();
if result.is_empty() {
"I wasn't able to produce a user-facing routine summary.".to_string()
} else {
result.to_string()
}
}

/// Execute a lightweight routine with tool execution support (agentic loop).
///
/// This is a simplified version of the full dispatcher loop:
Expand Down Expand Up @@ -2310,6 +2344,10 @@ mod tests {
prompt.contains("Do not claim you lack messaging integrations"),
"delivery guidance should suppress fake setup chatter: {prompt}",
);
assert!(
prompt.contains("Do not use the `message` tool for the routine's primary delivery"),
"delivery guidance should reserve message tool for non-primary delivery: {prompt}",
);
assert!(
prompt.contains("Tools are disabled for this routine run"),
"prompt should explain that tools are disabled: {prompt}",
Expand Down Expand Up @@ -2540,6 +2578,39 @@ mod tests {
assert_eq!(finish_reason_stop, crate::llm::FinishReason::Stop);
}

#[test]
fn test_handle_text_response_strips_internal_tool_markers() {
let result = super::handle_text_response(
"Here is the report.\n[Called tool `http` with arguments: {\"url\":\"https://example.com\"}]",
crate::llm::FinishReason::Stop,
10,
5,
)
.expect("tool marker text should sanitize");

assert_eq!(result.0, RunStatus::Attention);
assert_eq!(result.1.as_deref(), Some("Here is the report."));
assert_eq!(result.2, Some(15));
}

#[test]
fn test_handle_text_response_replaces_marker_only_text() {
let result = super::handle_text_response(
"[Called tool `http` with arguments: {\"url\":\"https://example.com\"}]",
crate::llm::FinishReason::Stop,
4,
3,
)
.expect("marker-only text should fall back to a user-facing summary");

assert_eq!(result.0, RunStatus::Attention);
assert_eq!(
result.1.as_deref(),
Some("I wasn't able to produce a user-facing routine summary.")
);
assert_eq!(result.2, Some(7));
}

#[test]
fn test_truncate_adds_ellipsis_when_over_limit() {
let input = "abcdefghijk";
Expand Down
Loading