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
133 changes: 108 additions & 25 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;
use crate::tool_args::parse_tool_call_args_lossy;

#[path = "nearai_tool_message_flattening.rs"]
mod nearai_tool_message_flattening;
Expand Down Expand Up @@ -619,6 +619,7 @@ impl NearAiChatProvider {
state.name = name;
}
if let Some(arguments) = function.arguments {
state.arguments_delta_seen = true;
state.arguments.push_str(&arguments);
}
}
Expand Down Expand Up @@ -1562,28 +1563,40 @@ struct NearAiStreamingToolCallState {
id: String,
name: String,
arguments: String,
arguments_delta_seen: bool,
}

impl NearAiStreamingToolCallState {
fn into_tool_call(self) -> Result<Option<ToolCall>, LlmError> {
if self.id.is_empty() && self.name.is_empty() && self.arguments.is_empty() {
let Self {
id,
name,
arguments: raw_arguments,
arguments_delta_seen,
} = self;

if id.is_empty() && name.is_empty() && raw_arguments.is_empty() && !arguments_delta_seen {
return Ok(None);
}
let arguments = if self.arguments.trim().is_empty() {
serde_json::Value::Object(Default::default())
} else {
parse_tool_call_args(&self.arguments).map_err(|error| LlmError::InvalidResponse {
provider: "nearai_chat".to_string(),
reason: error.reason,
})?
};
let (arguments, arguments_parse_error) =
if raw_arguments.is_empty() && !arguments_delta_seen {
(serde_json::Value::Object(Default::default()), None)
} else {
parse_tool_call_args_lossy(&raw_arguments)
};
let arguments_parse_error = arguments_parse_error.map(|parse_error| {
format!(
"{parse_error}\nRaw malformed tool-call arguments (verbatim, {} bytes):\n{raw_arguments}",
raw_arguments.len()
)
});
Ok(Some(ToolCall {
id: self.id,
name: self.name,
id,
name,
arguments,
reasoning: None,
signature: None,
arguments_parse_error: None,
arguments_parse_error,
}))
}
}
Expand Down Expand Up @@ -2029,7 +2042,7 @@ data: [DONE]
}

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

Expand Down Expand Up @@ -2063,19 +2076,89 @@ data: [DONE]

let provider = NearAiChatProvider::new(test_nearai_config(&base_url), test_session())
.expect("provider");
let result = complete_search_tool_streaming(provider).await;
let response = complete_search_tool_streaming(provider)
.await
.expect("malformed streamed tool arguments should be preserved for host repair");

server_task.await.expect("server task");
match result {
Err(LlmError::InvalidResponse { provider, reason }) => {
assert_eq!(provider, "nearai_chat");
assert!(
reason.starts_with("failed to parse tool-call arguments JSON: "),
"unexpected reason: {reason}"
);
}
other => panic!("expected invalid malformed tool arguments, got {other:?}"),
}
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::Value::Object(Default::default())
);
let parse_error = response.tool_calls[0]
.arguments_parse_error
.as_deref()
.expect("malformed arguments should carry parse error metadata");
assert!(
parse_error.starts_with("failed to parse tool-call arguments JSON: "),
"unexpected parse error: {parse_error}"
);
assert!(
parse_error
.contains("Raw malformed tool-call arguments (verbatim, 9 bytes):\n{\"query\":"),
"raw malformed arguments must be available for model repair: {parse_error}"
);
}

#[tokio::test]
async fn complete_with_tools_streaming_marks_empty_tool_arguments_for_repair() {
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 {
let (mut socket, _) = listener.accept().await.expect("accept request");
let (headers, body) = read_http_request_body(&mut socket).await;
assert!(headers.starts_with("POST /v1/chat/completions "));
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");
socket
.write_all(
br#"data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_search","type":"function","function":{"name":"search","arguments":""}}]},"finish_reason":"tool_calls"}]}

data: [DONE]

"#,
)
.await
.expect("write empty-arguments tool call stream");
});

let provider = NearAiChatProvider::new(test_nearai_config(&base_url), test_session())
.expect("provider");
let response = complete_search_tool_streaming(provider)
.await
.expect("empty streamed tool arguments should be preserved for host repair");

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::Value::Object(Default::default())
);
let parse_error = response.tool_calls[0]
.arguments_parse_error
.as_deref()
.expect("empty arguments should carry parse error metadata");
assert!(parse_error.starts_with("empty arguments string"));
assert!(
parse_error.contains("Raw malformed tool-call arguments (verbatim, 0 bytes):\n"),
"empty raw arguments must still be explicit for model repair: {parse_error}"
);
}

#[test]
Expand Down
93 changes: 87 additions & 6 deletions crates/ironclaw_reborn/src/model_gateway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ use crate::{
const MODEL_CREDITS_EXHAUSTED_SUMMARY: &str = "model provider account is out of credits";
const PROVIDER_TOOL_ARGUMENTS_OMITTED_MARKER: &str =
"arguments omitted because they exceeded the host provider-tool limit";
const PROVIDER_TOOL_ARGUMENTS_PARSE_ERROR_PREFIX: &str =
"failed to parse tool-call arguments JSON:";
const PROVIDER_TOOL_ARGUMENTS_INVALID_SUMMARY: &str = "model returned invalid tool-call arguments";
const PROVIDER_TOOL_ARGUMENTS_INVALID_MARKER: &str =
"arguments omitted because the provider emitted malformed tool-call JSON";
const CONTEXT_SHADOW_TARGET: &str = "ironclaw::reborn::context_shadow";
const UNAVAILABLE_CAPABILITY_REPLY: &str = "That capability is unavailable or disabled for this request, so I will not route it through another tool.";

Expand Down Expand Up @@ -1786,6 +1791,19 @@ fn provider_tool_call_from_llm(
provider_turn_id: String,
replay_identity: &ProviderReplayIdentity,
) -> Result<ProviderToolCall, HostManagedModelError> {
if let Some(parse_error) = tool_call.arguments_parse_error.as_deref() {
let safe_summary = provider_tool_arguments_parse_error_summary(parse_error);
debug!(
provider_call_id = tool_call.id.as_str(),
tool_name = tool_call.name.as_str(),
safe_summary = safe_summary.as_str(),
"reborn model gateway rejected malformed provider tool arguments"
);
return Err(HostManagedModelError::safe(
HostManagedModelErrorKind::InvalidOutput,
safe_summary,
));
}
let name = ProviderToolName::new(tool_call.name).map_err(|error| {
debug!(%error, "reborn model gateway rejected invalid provider tool name");
HostManagedModelError::safe(
Expand All @@ -1806,6 +1824,19 @@ fn provider_tool_call_from_llm(
})
}

fn provider_tool_arguments_parse_error_summary(parse_error: &str) -> String {
let summary_line = parse_error.lines().next().unwrap_or(parse_error);
let sanitized = sanitize_model_visible_text(summary_line.to_string());
let sanitized = sanitized.trim();
if sanitized.starts_with(PROVIDER_TOOL_ARGUMENTS_PARSE_ERROR_PREFIX)
&& LoopSafeSummary::new(sanitized).is_ok()
{
sanitized.to_string()
} else {
PROVIDER_TOOL_ARGUMENTS_INVALID_SUMMARY.to_string()
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

fn provider_turn_id(provider_turn_scope: &str, tool_calls: &[ToolCall]) -> String {
let mut stable = String::new();
stable.push_str(provider_turn_scope);
Expand Down Expand Up @@ -1910,7 +1941,13 @@ fn map_provider_tool_output_error(error: AgentLoopHostError) -> HostManagedModel

fn is_repairable_provider_tool_output_error(error: &HostManagedModelError) -> bool {
error.kind == HostManagedModelErrorKind::InvalidOutput
&& is_provider_arguments_too_large_summary(&error.safe_summary)
&& (is_provider_arguments_too_large_summary(&error.safe_summary)
|| is_provider_tool_arguments_parse_error_summary(&error.safe_summary))
}

fn is_provider_tool_arguments_parse_error_summary(safe_summary: &str) -> bool {
safe_summary.starts_with(PROVIDER_TOOL_ARGUMENTS_PARSE_ERROR_PREFIX)
|| safe_summary == PROVIDER_TOOL_ARGUMENTS_INVALID_SUMMARY
}
Comment on lines 1942 to 1951

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 | 🔵 Trivial | ⚖️ Poor tradeoff

Repairability classification keyed off safe_summary string equality.

is_provider_tool_arguments_parse_error_summary matches on starts_with/== against the safe-summary string rather than a typed error variant/kind on HostManagedModelError. This mirrors the pre-existing is_provider_arguments_too_large_summary pattern, so it's consistent with current file conventions, but as per coding guidelines, "If code matches on string literals, the underlying type should be an enum; do not leave it as a stringly-typed value." Any future safe-summary wording change silently breaks the retry classification with no compiler signal.

🤖 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/ironclaw_reborn/src/model_gateway.rs` around lines 1941 - 1950, The
repairability check in is_provider_tool_arguments_parse_error_summary is
stringly-typed and can silently break if safe_summary wording changes. Update
HostManagedModelError/HostManagedModelErrorKind handling so
is_repairable_provider_tool_output_error and
is_provider_tool_arguments_parse_error_summary rely on a typed enum variant or
dedicated error kind instead of starts_with/== on safe_summary, then keep the
InvalidOutput classification wired through that typed signal.

Source: Coding guidelines


fn provider_tool_repair_messages(
Expand All @@ -1936,16 +1973,30 @@ fn provider_tool_repair_messages(
ChatMessage::tool_result(
tool_call.id.clone(),
tool_call.name.clone(),
format!(
"Tool call batch rejected by host: {safe_summary}. None of this response's tool calls were executed. Retry with smaller arguments or answer directly without this tool if it is not needed."
),
provider_tool_repair_result_content(tool_call, safe_summary),
)
}))
.collect()
}

fn provider_tool_repair_result_content(tool_call: &ToolCall, safe_summary: &str) -> String {
let mut content = format!("Tool call batch rejected by host: {safe_summary}.");
if let Some(parse_error) = tool_call.arguments_parse_error.as_deref() {
content.push_str("\n\nMalformed tool-call argument details for this rejected call:\n");
content.push_str(parse_error);
}
content.push_str(
"\n\nNone of this response's tool calls were executed. Retry with valid JSON arguments or answer directly without this tool if it is not needed.",
);
content
}

fn provider_tool_call_for_repair(tool_call: &ToolCall) -> ToolCall {
let arguments = if provider_arguments_exceed_max_bytes(&tool_call.arguments) {
let arguments = if tool_call.arguments_parse_error.is_some() {
serde_json::json!({
"error": PROVIDER_TOOL_ARGUMENTS_INVALID_MARKER,
})
} else if provider_arguments_exceed_max_bytes(&tool_call.arguments) {
serde_json::json!({
"error": PROVIDER_TOOL_ARGUMENTS_OMITTED_MARKER,
})
Expand All @@ -1959,7 +2010,7 @@ fn provider_tool_call_for_repair(tool_call: &ToolCall) -> ToolCall {
arguments,
reasoning: tool_call.reasoning.clone(),
signature: tool_call.signature.clone(),
arguments_parse_error: tool_call.arguments_parse_error.clone(),
arguments_parse_error: None,
}
}

Expand Down Expand Up @@ -2776,6 +2827,36 @@ mod tests {
);
}

#[test]
fn provider_tool_arguments_parse_error_summary_falls_back_for_unknown_metadata() {
let summary = provider_tool_arguments_parse_error_summary(
"raw_provider_secret api_key=sk-live-secret malformed payload",
);

assert_eq!(summary, PROVIDER_TOOL_ARGUMENTS_INVALID_SUMMARY);
}

#[test]
fn provider_tool_arguments_parse_error_summary_drops_raw_repair_detail() {
let summary = provider_tool_arguments_parse_error_summary(
"failed to parse tool-call arguments JSON: trailing characters at line 1 column 3\nRaw malformed tool-call arguments (verbatim, 42 bytes):\n{\"api_key\":\"sk-live-secret\"",
);

assert_eq!(
summary,
"failed to parse tool-call arguments JSON: trailing characters at line 1 column 3"
);
}

#[test]
fn provider_tool_arguments_parse_error_summary_rejects_unsafe_parser_detail() {
let summary = provider_tool_arguments_parse_error_summary(
"failed to parse tool-call arguments JSON: expected `,` or `}` at line 1 column 2",
);

assert_eq!(summary, PROVIDER_TOOL_ARGUMENTS_INVALID_SUMMARY);
}

#[test]
fn provider_tool_repair_messages_preserves_reasoning_details_on_assistant_message() {
use ironclaw_llm::{ReasoningDetail, ReasoningDetails};
Expand Down
Loading
Loading