reset filter suppression state between tool calls (still broken)

This commit is contained in:
Dhanji Prasanna
2025-10-15 21:15:24 +11:00
parent c9037ede22
commit beccc8fa15
3 changed files with 125 additions and 101 deletions

View File

@@ -4,8 +4,8 @@
// 3. Only elide JSON content between first '{' and last '}' (inclusive)
// 4. Return everything else as the final filtered string
use std::cell::RefCell;
use regex::Regex;
use std::cell::RefCell;
use tracing::debug;
// Thread-local state for tracking JSON tool call suppression
@@ -23,8 +23,7 @@ struct FixedJsonToolState {
}
impl FixedJsonToolState {
fn new() -> Self {
fn new() -> Self {
Self {
suppression_mode: false,
brace_depth: 0,
@@ -34,8 +33,7 @@ fn new() -> Self {
}
}
fn reset(&mut self) {
fn reset(&mut self) {
self.suppression_mode = false;
self.brace_depth = 0;
self.buffer.clear();
@@ -50,10 +48,10 @@ pub fn fixed_filter_json_tool_calls(content: &str) -> String {
if content.is_empty() {
return String::new();
}
FIXED_JSON_TOOL_STATE.with(|state| {
let mut state = state.borrow_mut();
// Add new content to buffer
state.buffer.push_str(content);
@@ -68,17 +66,20 @@ pub fn fixed_filter_json_tool_calls(content: &str) -> String {
// Exit suppression mode when all braces are closed
if state.brace_depth <= 0 {
debug!("JSON tool call completed - exiting suppression mode");
// Extract the complete result with JSON filtered out
let result = extract_fixed_content(&state.buffer, state.json_start_in_buffer.unwrap_or(0));
// Return only the part we haven't returned yet
let result = extract_fixed_content(
&state.buffer,
state.json_start_in_buffer.unwrap_or(0),
);
// Return only the part we haven't returned yet
let new_content = if result.len() > state.content_returned_up_to {
result[state.content_returned_up_to..].to_string()
} else {
String::new()
};
state.reset();
return new_content;
}
@@ -89,34 +90,37 @@ pub fn fixed_filter_json_tool_calls(content: &str) -> String {
// Still in suppression mode, return empty string (content is being accumulated)
return String::new();
}
// Check for tool call pattern using corrected regex
// More flexible than the strict specification to handle real-world JSON
let tool_call_regex = Regex::new(r#"(?m)^.*\{\s*"tool"\s*:\s*""#).unwrap();
let tool_call_regex = Regex::new(r#"(?m)^\s*\{\s*"tool"\s*:\s*""#).unwrap();
if let Some(captures) = tool_call_regex.find(&state.buffer) {
let match_text = captures.as_str();
// Find the position of the opening brace in the match
if let Some(brace_offset) = match_text.find('{') {
let json_start = captures.start() + brace_offset;
debug!("Detected JSON tool call at position {} - entering suppression mode", json_start);
debug!(
"Detected JSON tool call at position {} - entering suppression mode",
json_start
);
// Return content before JSON that we haven't returned yet
let content_before_json = if json_start >= state.content_returned_up_to {
state.buffer[state.content_returned_up_to..json_start].to_string()
} else {
String::new()
};
state.content_returned_up_to = json_start;
// Enter suppression mode
state.suppression_mode = true;
state.brace_depth = 0;
state.json_start_in_buffer = Some(json_start);
// Count braces from the JSON start to see if it's complete
let buffer_clone = state.buffer.clone();
for ch in buffer_clone[json_start..].chars() {
@@ -128,15 +132,16 @@ pub fn fixed_filter_json_tool_calls(content: &str) -> String {
// JSON is complete in this chunk
debug!("JSON tool call completed in same chunk");
let result = extract_fixed_content(&buffer_clone, json_start);
// Return content before JSON plus content after JSON
let content_after_json = if result.len() > json_start {
&result[json_start..]
} else {
""
};
let final_result = format!("{}{}", content_before_json, content_after_json);
let final_result =
format!("{}{}", content_before_json, content_after_json);
state.reset();
return final_result;
}
@@ -144,7 +149,7 @@ pub fn fixed_filter_json_tool_calls(content: &str) -> String {
_ => {}
}
}
// JSON is incomplete, return only the content before JSON
return content_before_json;
}
@@ -158,7 +163,7 @@ pub fn fixed_filter_json_tool_calls(content: &str) -> String {
} else {
String::new()
};
new_content
})
}
@@ -172,13 +177,13 @@ fn extract_fixed_content(full_content: &str, json_start: usize) -> String {
let mut json_end = json_start;
let mut in_string = false;
let mut escape_next = false;
for (i, ch) in full_content[json_start..].char_indices() {
if escape_next {
escape_next = false;
continue;
}
match ch {
'\\' if in_string => escape_next = true,
'"' if !escape_next => in_string = !in_string,
@@ -195,7 +200,7 @@ fn extract_fixed_content(full_content: &str, json_start: usize) -> String {
_ => {}
}
}
// Return content before and after the JSON (excluding the JSON itself)
let before = &full_content[..json_start];
let after = if json_end < full_content.len() {
@@ -203,7 +208,7 @@ fn extract_fixed_content(full_content: &str, json_start: usize) -> String {
} else {
""
};
format!("{}{}", before, after)
}
@@ -214,4 +219,4 @@ pub fn reset_fixed_json_tool_state() {
let mut state = state.borrow_mut();
state.reset();
});
}
}

View File

@@ -17,7 +17,7 @@ mod fixed_filter_tests {
let input = r#"Some text before
{"tool": "shell", "args": {"command": "ls"}}
Some text after"#;
let result = fixed_filter_json_tool_calls(input);
let expected = "Some text before\n\nSome text after";
assert_eq!(result, expected);
@@ -26,22 +26,22 @@ Some text after"#;
#[test]
fn test_streaming_chunks() {
reset_fixed_json_tool_state();
// Simulate streaming where the tool call comes in multiple chunks
let chunks = vec![
"Some text before\n",
"{\"tool\": \"",
"shell\", \"args\": {",
"\"command\": \"ls\"",
"}}\nText after"
"}}\nText after",
];
let mut results = Vec::new();
for chunk in chunks {
let result = fixed_filter_json_tool_calls(chunk);
results.push(result);
}
// The final accumulated result should have the JSON filtered out
let final_result: String = results.join("");
let expected = "Some text before\n\nText after";
@@ -51,11 +51,11 @@ Some text after"#;
#[test]
fn test_nested_braces_in_tool_call() {
reset_fixed_json_tool_state();
let input = r#"Text before
{"tool": "write_file", "args": {"file_path": "test.json", "content": "{\"nested\": \"value\"}"}}
Text after"#;
let result = fixed_filter_json_tool_calls(input);
let expected = "Text before\n\nText after";
assert_eq!(result, expected);
@@ -64,42 +64,64 @@ Text after"#;
#[test]
fn test_regex_pattern_specification() {
// Test the corrected regex pattern that's more flexible with whitespace
let pattern = Regex::new(r#"(?m)^.*\{\s*"tool"\s*:"#).unwrap();
let pattern = Regex::new(r#"(?m)^\s*\{\s*"tool"\s*:"#).unwrap();
let test_cases = vec![
(r#"line
{"tool":"#, true),
(r#"line
{"tool" :"#, true),
(r#"line
{ "tool":"#, true), // Space after { DOES match with \s*
(r#"line
abc{"tool":"#, true),
(r#"line
{"tool123":"#, false), // "tool123" is not exactly "tool"
(r#"line
{"tool" : "#, true),
(
r#"line
{"tool":"#,
true,
),
(
r#"line
{"tool" :"#,
true,
),
(
r#"line
{ "tool":"#,
true,
), // Space after { DOES match with \s*
(
r#"line
abc{"tool":"#,
true,
),
(
r#"line
{"tool123":"#,
false,
), // "tool123" is not exactly "tool"
(
r#"line
{"tool" : "#,
true,
),
];
for (input, should_match) in test_cases {
let matches = pattern.is_match(input);
assert_eq!(matches, should_match, "Pattern matching failed for: {}", input);
assert_eq!(
matches, should_match,
"Pattern matching failed for: {}",
input
);
}
}
#[test]
fn test_newline_requirement() {
reset_fixed_json_tool_state();
// According to spec, tool call should be detected "on the very next newline"
// According to spec, tool call should be detected "on the very next newline"
// Our current regex matches any line that contains the pattern, not just after newlines
let input_with_newline = "Text\n{\"tool\": \"shell\", \"args\": {\"command\": \"ls\"}}";
let input_without_newline = "Text {\"tool\": \"shell\", \"args\": {\"command\": \"ls\"}}";
let result1 = fixed_filter_json_tool_calls(input_with_newline);
reset_fixed_json_tool_state();
let result2 = fixed_filter_json_tool_calls(input_without_newline);
// Both cases currently trigger suppression due to regex pattern
// TODO: Fix regex to only match after actual newlines
assert_eq!(result1, "Text\n");
@@ -110,11 +132,11 @@ abc{"tool":"#, true),
#[test]
fn test_json_with_escaped_quotes() {
reset_fixed_json_tool_state();
let input = r#"Text
{"tool": "write_file", "args": {"content": "He said \"hello\" to me"}}
More text"#;
let result = fixed_filter_json_tool_calls(input);
let expected = "Text\n\nMore text";
assert_eq!(result, expected);
@@ -123,12 +145,12 @@ More text"#;
#[test]
fn test_edge_case_malformed_json() {
reset_fixed_json_tool_state();
// Test what happens with malformed JSON that starts like a tool call
let input = r#"Text
{"tool": "shell", "args": {"command": "ls"
More text"#;
let result = fixed_filter_json_tool_calls(input);
// Should handle gracefully - since JSON is incomplete, it should return content before JSON
let expected = "Text\n";
@@ -138,7 +160,7 @@ More text"#;
#[test]
fn test_multiple_tool_calls_sequential() {
reset_fixed_json_tool_state();
// Test processing multiple tool calls one at a time
let input1 = r#"First text
{"tool": "shell", "args": {"command": "ls"}}
@@ -146,7 +168,7 @@ Middle text"#;
let result1 = fixed_filter_json_tool_calls(input1);
let expected1 = "First text\n\nMiddle text";
assert_eq!(result1, expected1);
// Reset and process second tool call
reset_fixed_json_tool_state();
let input2 = r#"More text
@@ -160,11 +182,11 @@ Final text"#;
#[test]
fn test_tool_call_with_complex_args() {
reset_fixed_json_tool_state();
let input = r#"Before
{"tool": "str_replace", "args": {"file_path": "test.rs", "diff": "--- old\n-old line\n+++ new\n+new line", "start": 0, "end": 100}}
After"#;
let result = fixed_filter_json_tool_calls(input);
let expected = "Before\n\nAfter";
assert_eq!(result, expected);
@@ -173,10 +195,10 @@ After"#;
#[test]
fn test_tool_call_only() {
reset_fixed_json_tool_state();
let input = r#"
{"tool": "final_output", "args": {"summary": "Task completed successfully"}}"#;
let result = fixed_filter_json_tool_calls(input);
let expected = "\n";
assert_eq!(result, expected);
@@ -185,12 +207,12 @@ After"#;
#[test]
fn test_brace_counting_accuracy() {
reset_fixed_json_tool_state();
// Test complex nested structure
let input = r#"Start
{"tool": "write_file", "args": {"content": "function() { return {a: 1, b: {c: 2}}; }", "file_path": "test.js"}}
End"#;
let result = fixed_filter_json_tool_calls(input);
let expected = "Start\n\nEnd";
assert_eq!(result, expected);
@@ -199,12 +221,12 @@ End"#;
#[test]
fn test_string_escaping_in_json() {
reset_fixed_json_tool_state();
// Test JSON with escaped quotes and braces in strings
let input = r#"Text
{"tool": "shell", "args": {"command": "echo \"Hello {world}\" > file.txt"}}
More"#;
let result = fixed_filter_json_tool_calls(input);
let expected = "Text\n\nMore";
assert_eq!(result, expected);
@@ -213,13 +235,13 @@ More"#;
#[test]
fn test_specification_compliance() {
reset_fixed_json_tool_state();
// Test the exact specification requirements:
// 1. Detect start with regex '\w*{\w*"tool"\w*:\w*"' on newline
// 2. Enter suppression mode and use brace counting
// 3. Elide only JSON between first '{' and last '}' (inclusive)
// 4. Return everything else
let input = "Before text\nSome more text\n{\"tool\": \"test\", \"args\": {}}\nAfter text\nMore after";
let result = fixed_filter_json_tool_calls(input);
let expected = "Before text\nSome more text\n\nAfter text\nMore after";
@@ -229,7 +251,7 @@ More"#;
#[test]
fn test_no_false_positives() {
reset_fixed_json_tool_state();
// Test that we don't incorrectly identify non-tool JSON as tool calls
let input = r#"Some text
{"not_tool": "value", "other": "data"}
@@ -242,14 +264,14 @@ More text"#;
#[test]
fn test_partial_tool_patterns() {
reset_fixed_json_tool_state();
// Test patterns that look like tool calls but aren't complete
let test_cases = vec![
"Text\n{\"too\": \"value\"}", // "too" not "tool"
"Text\n{\"too\": \"value\"}", // "too" not "tool"
"Text\n{\"tools\": \"value\"}", // "tools" not "tool"
"Text\n{\"tool\": }", // Missing value after colon
"Text\n{\"tool\": }", // Missing value after colon
];
for input in test_cases {
reset_fixed_json_tool_state();
let result = fixed_filter_json_tool_calls(input);
@@ -261,29 +283,18 @@ More text"#;
#[test]
fn test_streaming_edge_cases() {
reset_fixed_json_tool_state();
// Test streaming with very small chunks
let chunks = vec![
"Text\n",
"{",
"\"",
"tool",
"\"",
":",
" ",
"\"",
"test",
"\"",
"}",
"\nAfter"
"Text\n", "{", "\"", "tool", "\"", ":", " ", "\"", "test", "\"", "}", "\nAfter",
];
let mut results = Vec::new();
for chunk in chunks {
let result = fixed_filter_json_tool_calls(chunk);
results.push(result);
}
let final_result: String = results.join("");
// This test currently fails because the JSON is incomplete across chunks
// The function doesn't handle this edge case properly yet
@@ -294,28 +305,28 @@ More text"#;
#[test]
fn test_streaming_debug() {
reset_fixed_json_tool_state();
// Debug the exact failing case
let chunks = vec![
"Some text before\n",
"{\"tool\": \"",
"shell\", \"args\": {",
"\"command\": \"ls\"",
"}}\nText after"
"}}\nText after",
];
let mut results = Vec::new();
for (i, chunk) in chunks.iter().enumerate() {
let result = fixed_filter_json_tool_calls(chunk);
println!("Chunk {}: {:?} -> {:?}", i, chunk, result);
results.push(result);
}
let final_result: String = results.join("");
println!("Final result: {:?}", final_result);
println!("Expected: {:?}", "Some text before\n\nText after");
let expected = "Some text before\n\nText after";
assert_eq!(final_result, expected);
}
}
}

View File

@@ -706,6 +706,10 @@ impl<W: UiWriter> Agent<W> {
show_timing: bool,
cancellation_token: CancellationToken,
) -> Result<TaskResult> {
// Reset the JSON tool call filter state at the start of each new task
// This prevents the filter from staying in suppression mode between user interactions
fixed_filter_json::reset_fixed_json_tool_state();
// Generate session ID based on the initial prompt if this is a new session
if self.session_id.is_none() {
self.session_id = Some(self.generate_session_id(description));
@@ -1635,6 +1639,10 @@ The tool will execute immediately and you'll receive the result (success or erro
}
tool_executed = true;
// Reset the JSON tool call filter state after each tool execution
// This ensures the filter doesn't stay in suppression mode for subsequent streaming content
fixed_filter_json::reset_fixed_json_tool_state();
// Reset parser for next iteration
parser.reset();
// Clear current_response for next iteration to prevent buffered text