Skip to content
Merged
Show file tree
Hide file tree
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: 69 additions & 2 deletions crates/ironclaw_llm/src/nearai_chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::provider::{
ChatMessage, CompletionRequest, CompletionResponse, CompletionStreamSink, FinishReason,
LlmProvider, Role, ToolCall, ToolCompletionRequest, ToolCompletionResponse,
};
use crate::tool_args::parse_tool_call_args_lossy;
use crate::tool_args::parse_tool_call_args_allow_trailing_lossy;

#[path = "nearai_tool_message_flattening.rs"]
mod nearai_tool_message_flattening;
Expand Down Expand Up @@ -1604,7 +1604,7 @@ impl NearAiStreamingToolCallState {
if raw_arguments.is_empty() && !arguments_delta_seen {
(serde_json::Value::Object(Default::default()), None)
} else {
parse_tool_call_args_lossy(&raw_arguments)
parse_tool_call_args_allow_trailing_lossy(&raw_arguments)
};
let arguments_parse_error = arguments_parse_error.map(|parse_error| {
format!(
Expand Down Expand Up @@ -2339,6 +2339,73 @@ data: [DONE]
);
}

#[tokio::test]
async fn complete_with_tools_streaming_recovers_args_with_trailing_content() {
use tokio::io::AsyncWriteExt;
use tokio::net::TcpListener;

let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let base_url = format!("http://{}", listener.local_addr().unwrap());
let server_task = tokio::spawn(async move {
loop {
let (mut socket, _) = listener.accept().await.expect("accept request");
let (headers, body) = read_http_request_body(&mut socket).await;
// `NearAiChatProvider::new` fires a background pricing fetch to the
// same base URL; answer it harmlessly and keep serving until the
// chat completion request arrives (avoids a single-accept race).
if !headers.starts_with("POST /v1/chat/completions ") {
write_http_json_response(&mut socket, serde_json::json!({ "models": [] }))
.await;
continue;
}
let request_json: serde_json::Value =
serde_json::from_str(&body).expect("request json");
assert_eq!(request_json["stream"], true);

socket
.write_all(
b"HTTP/1.1 200 OK\r\ncontent-type: text/event-stream\r\ncache-control: no-cache\r\nconnection: close\r\n\r\n",
)
.await
.expect("write sse headers");
// A reasoning model streams a complete arguments object followed by
// a stray trailing token. The leading object must be recovered, not
// collapsed to an empty `{}` (which would call the tool with no args).
socket
.write_all(
br#"data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_search","type":"function","function":{"name":"search","arguments":"{\"query\":\"test\"}trailing"}}]},"finish_reason":"tool_calls"}]}

data: [DONE]

"#,
)
.await
.expect("write trailing-content tool call stream");
break;
}
});

let provider = NearAiChatProvider::new(test_nearai_config(&base_url), test_session())
.expect("provider");
let response = complete_search_tool_streaming(provider)
.await
.expect("trailing content after valid args should be recovered, not fail the turn");

server_task.await.expect("server task");
assert_eq!(response.tool_calls.len(), 1);
assert_eq!(response.tool_calls[0].id, "call_search");
assert_eq!(response.tool_calls[0].name, "search");
assert_eq!(
response.tool_calls[0].arguments,
serde_json::json!({"query": "test"}),
"the leading valid object must be recovered, not dropped to empty"
);
assert!(
response.tool_calls[0].arguments_parse_error.is_none(),
"trailing content after a valid object must not mark the args for repair"
);
}

#[tokio::test]
async fn complete_with_tools_streaming_marks_empty_tool_arguments_for_repair() {
use tokio::io::AsyncWriteExt;
Expand Down
54 changes: 54 additions & 0 deletions crates/ironclaw_llm/src/tool_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,36 @@ pub(crate) fn parse_tool_call_args_lossy(raw: &str) -> (Value, Option<String>) {
}
}

/// Streaming-salvage variant of [`parse_tool_call_args_lossy`].
///
/// Some streaming providers (e.g. NearAI reasoning models like DeepSeek) append
/// a stray token after a complete arguments object (`{...}` + junk). This parses
Comment on lines +73 to +76

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed — updated both PR descriptions to match the implementation after the refactor:

  • parse_tool_call_args stays strict; recovery lives in the new, explicitly-named parse_tool_call_args_allow_trailing_lossy (called only from NearAiStreamingToolCallState::into_tool_call).
  • Referenced test names corrected to allow_trailing_lossy_recovers_first_value and parse_tool_call_args_stays_strict_on_trailing_content, plus the caller-level complete_with_tools_streaming_recovers_args_with_trailing_content.

Thanks for catching the drift.

/// the **first** complete JSON value and ignores any trailing content, then
/// falls back to the same silent-`{}` + captured-`reason` behavior as
/// [`parse_tool_call_args_lossy`] on empty or genuinely malformed input.
///
/// Kept as a separate, explicitly-named helper so this permissive "ignore
/// trailing junk" behavior stays scoped to the streaming path and does **not**
/// loosen the shared, fail-loud [`parse_tool_call_args`] boundary that other
/// providers rely on.
pub(crate) fn parse_tool_call_args_allow_trailing_lossy(raw: &str) -> (Value, Option<String>) {
// A clean parse (no trailing junk) is handled by the shared lossy helper.
let lossy = parse_tool_call_args_lossy(raw);
if lossy.1.is_none() {
return lossy;
}
// The strict parse rejected the input. If it is a valid JSON value followed
// by a stray trailing token (streaming reasoning models do this), recover the
// leading value; otherwise keep the lossy `{}` + captured reason.
match serde_json::Deserializer::from_str(raw)
.into_iter::<Value>()
.next()
{
Some(Ok(value)) => (value, None),
_ => lossy,
}
}

/// Ordered probe over candidate reasoning-field names.
///
/// Iterates `fields` in order; returns a borrowed reference to the first value
Expand Down Expand Up @@ -138,6 +168,30 @@ mod tests {
assert!(err.is_none());
}

#[test]
fn allow_trailing_lossy_recovers_first_value() {
// The streaming salvage helper recovers the leading object when a
// reasoning model appends a stray token after it, instead of dropping
// the arguments to `{}`.
let (val, err) = parse_tool_call_args_allow_trailing_lossy(r#"{"query": "test"}trailing"#);
assert_eq!(val["query"], "test");
assert!(err.is_none());

// Genuinely malformed input still fails closed to `{}` + a captured reason.
let (val, err) = parse_tool_call_args_allow_trailing_lossy(r#"{not valid"#);
assert!(val.as_object().expect("object").is_empty());
assert!(err.is_some());
}

#[test]
fn parse_tool_call_args_stays_strict_on_trailing_content() {
// The shared, fail-loud primitive must NOT accept concatenated or
// trailing output — only the streaming salvage helper tolerates it.
// Guards against loosening the canonical boundary for other providers.
assert!(parse_tool_call_args(r#"{"a": 2} x"#).is_err());
assert!(parse_tool_call_args(r#"{}{}"#).is_err());
}

#[test]
fn probe_reasoning_field_first_present_wins() {
let json = json!({"reasoning": "first", "reasoning_content": "second"});
Expand Down
Loading