Skip to content

acp-xcode: handle malformed Chinese-model tool calls #102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
setoelkahfi merged 3 commits into from
Sep 16, 2026
Merged

acp-xcode: handle malformed Chinese-model tool calls #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1f3787
Fix ACP handling of malformed Chinese-model tool calls
setoelkahfi Sep 14, 2026
76e8848
Aggregate suppressed tool-call log lines
setoelkahfi Sep 15, 2026
8198f4c
Stop a history-replay test from racing on the process cwd
setoelkahfi Sep 15, 2026
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
103 changes: 74 additions & 29 deletions src/backend.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,15 @@ pub trait InferenceBackend: Send + Sync {
sink: Option<&TokenSink>,
) -> Result<TurnResult, BackendError>;

/// Continue the turn by returning tool results. `tools` may be `None` on the
/// final round to force a text answer. `sink` streams that text when set.
/// Continue the turn by returning tool results. `allow_tool_calls` controls
/// whether `tools` are offered again; the complete catalog remains available
/// so a disabled round can recognize and suppress tool-shaped model output.
/// `sink` streams assistant text when set.
async fn send_tool_results(
&self,
results: Vec<ToolResult>,
tools: Option<&[ToolSpec]>,
tools: &[ToolSpec],
allow_tool_calls: bool,
sink: Option<&TokenSink>,
) -> Result<TurnResult, BackendError>;

Expand Down Expand Up @@ -257,7 +260,8 @@ impl InferenceBackend for LocalBackend {
async fn send_tool_results(
&self,
results: Vec<ToolResult>,
tools: Option<&[ToolSpec]>,
tools: &[ToolSpec],
allow_tool_calls: bool,
sink: Option<&TokenSink>,
) -> Result<TurnResult, BackendError> {
let onde_results: Vec<onde::inference::ToolResult> = results
Expand All @@ -268,10 +272,10 @@ impl InferenceBackend for LocalBackend {
})
.collect();

// The final round passes `tools = None` to force a text answer; that's
// the only round onde can stream, since no further tool calls are parsed.
// A forced-text round is the only round onde can stream, since no
// further tool calls are parsed.
if let Some(sink) = sink
&& tools.is_none()
&& !allow_tool_calls
{
let rx = self
.engine
Expand All @@ -281,7 +285,7 @@ impl InferenceBackend for LocalBackend {
return drain_onde_stream(rx, sink).await;
}

let onde_tools = tools.map(to_onde_tools);
let onde_tools = allow_tool_calls.then(|| to_onde_tools(tools));
let result = self
.engine
.send_tool_results(onde_results, onde_tools.as_deref())
Expand Down Expand Up @@ -485,12 +489,14 @@ impl OpenAiBackend {
.collect()
}

/// POST the current history (plus `tools`) and apply the assistant reply to
/// history, returning the neutral turn result. Streams via SSE when `sink`
/// is set; otherwise reads a single JSON response.
/// POST the current history and apply the assistant reply to history.
/// `tools` is always the known catalog, while `allow_tool_calls` determines
/// whether it is advertised to the model and whether returned calls may run.
/// Streams via SSE when `sink` is set; otherwise reads a single JSON response.
async fn complete(
&self,
tools: Option<&[ToolSpec]>,
tools: &[ToolSpec],
allow_tool_calls: bool,
sink: Option<&TokenSink>,
) -> Result<TurnResult, BackendError> {
let url = format!("{}/chat/completions", self.base_url.trim_end_matches('/'));
Expand All @@ -501,9 +507,7 @@ impl OpenAiBackend {
"messages": *self.history.lock().await,
Comment thread
setoelkahfi marked this conversation as resolved.
"stream": streaming,
});
if let Some(tools) = tools
&& !tools.is_empty()
{
if allow_tool_calls && !tools.is_empty() {
body["tools"] = serde_json::Value::Array(Self::tools_json(tools));
// OpenAI specifies `auto` as the default when tools are present,
// but not every OpenAI-compatible gateway implements that default.
Expand All @@ -528,13 +532,11 @@ impl OpenAiBackend {
return Err(describe_api_error(status, &body));
}

// The specs are needed downstream to type the arguments of any tool
// call the model emitted as text rather than as a structured call.
let tools = tools.unwrap_or(&[]);
if let Some(sink) = sink {
self.consume_stream(response, sink, tools).await
self.consume_stream(response, sink, tools, allow_tool_calls)
.await
} else {
self.consume_json(response, tools).await
self.consume_json(response, tools, allow_tool_calls).await
}
}

Expand All @@ -543,6 +545,7 @@ impl OpenAiBackend {
&self,
response: reqwest::Response,
tools: &[ToolSpec],
allow_tool_calls: bool,
) -> Result<TurnResult, BackendError> {
let parsed: ChatCompletion = response
.json()
Expand All @@ -556,7 +559,7 @@ impl OpenAiBackend {
.map(|choice| choice.message)
.ok_or_else(|| "endpoint returned no choices".to_string())?;

let text = message.content.clone().unwrap_or_default();
let mut text = message.content.clone().unwrap_or_default();
let tool_calls: Vec<ToolCall> = message
.tool_calls
.iter()
Expand All @@ -568,6 +571,31 @@ impl OpenAiBackend {
})
.collect();

if !allow_tool_calls {
Comment thread
setoelkahfi marked this conversation as resolved.
Comment thread
setoelkahfi marked this conversation as resolved.
Comment thread
setoelkahfi marked this conversation as resolved.
let (cleaned, recovered) = crate::inline_tool_calls::extract(&text, tools);
Comment thread
setoelkahfi marked this conversation as resolved.
text = cleaned;
let suppressed = tool_calls.len() + recovered.len();
if suppressed > 0 {
let names = tool_calls
.iter()
.map(|call| call.name.as_str())
.chain(recovered.iter().map(|call| call.name.as_str()))
.collect::<Vec<_>>()
.join(", ");
log::warn!(
"suppressed {suppressed} tool call(s) from a forced-text response: {names}"
Comment thread
setoelkahfi marked this conversation as resolved.
Comment thread
setoelkahfi marked this conversation as resolved.
);
}
self.history
.lock()
.await
.push(streamed_assistant_history(&text, &[]));
return Ok(TurnResult {
text,
tool_calls: Vec::new(),
});
}
Comment thread
setoelkahfi marked this conversation as resolved.

// Some models write a tool call out as literal `<tool_call>` text
// instead of using the structured field (see `inline_tool_calls`).
// Recover it, or the turn ends with the tag rendered as prose and
Expand Down Expand Up @@ -617,6 +645,7 @@ impl OpenAiBackend {
response: reqwest::Response,
sink: &TokenSink,
tools: &[ToolSpec],
allow_tool_calls: bool,
) -> Result<TurnResult, BackendError> {
use futures::StreamExt;

Expand Down Expand Up @@ -691,10 +720,12 @@ impl OpenAiBackend {
}
}
crate::inline_tool_calls::ScanEvent::ToolCall(call) => {
log::warn!(
"recovered tool call '{}' the model emitted as text instead of a structured call",
call.name
);
if allow_tool_calls {
log::warn!(
"recovered tool call '{}' the model emitted as text instead of a structured call",
call.name
);
}
recovered.push(ToolCall {
id: format!("call_recovered_{}", recovered.len()),
name: call.name,
Expand Down Expand Up @@ -755,6 +786,19 @@ impl OpenAiBackend {
.collect();
tool_calls.extend(recovered);

if !allow_tool_calls && !tool_calls.is_empty() {
Comment thread
setoelkahfi marked this conversation as resolved.
Comment thread
setoelkahfi marked this conversation as resolved.
let names = tool_calls
.iter()
.map(|call| call.name.as_str())
.collect::<Vec<_>>()
.join(", ");
log::warn!(
"suppressed {} tool call(s) from a forced-text response: {names}",
tool_calls.len()
);
tool_calls.clear();
}

// Record the assistant turn so later tool results have context.
self.history
.lock()
Expand Down Expand Up @@ -813,13 +857,14 @@ impl InferenceBackend for OpenAiBackend {
.lock()
.await
.push(serde_json::json!({ "role": "user", "content": text }));
self.complete(Some(tools), sink).await
self.complete(tools, true, sink).await
Comment thread
setoelkahfi marked this conversation as resolved.
Comment thread
setoelkahfi marked this conversation as resolved.
}

async fn send_tool_results(
&self,
results: Vec<ToolResult>,
tools: Option<&[ToolSpec]>,
tools: &[ToolSpec],
allow_tool_calls: bool,
sink: Option<&TokenSink>,
) -> Result<TurnResult, BackendError> {
{
Expand All @@ -832,7 +877,7 @@ impl InferenceBackend for OpenAiBackend {
}));
}
}
self.complete(tools, sink).await
self.complete(tools, allow_tool_calls, sink).await
}

async fn record_cancelled_tool_results(&self, results: Vec<ToolResult>) {
Expand Down Expand Up @@ -887,7 +932,7 @@ impl InferenceBackend for OpenAiBackend {
}));
*self.history.lock().await = request;

let summary = match self.complete(None, None).await {
let summary = match self.complete(&[], false, None).await {
Ok(result) => result.text,
Err(error) => {
// Roll back the summarization request; the turn never happened.
Expand Down
10 changes: 3 additions & 7 deletions src/chat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3346,20 +3346,16 @@ mod tui {

// on the last round, pass no tools so the model must produce text —
// that's also the round we can stream on-device.
let next_tools = if round < MAX_TOOL_ROUNDS {
Some(tools.as_slice())
} else {
None
};
let sink = if next_tools.is_none() {
let allow_tool_calls = round < MAX_TOOL_ROUNDS;
let sink = if !allow_tool_calls {
streamed = true;
Some(&delta_tx)
} else {
None
};

match backend
.send_tool_results(tool_results, next_tools, sink)
.send_tool_results(tool_results, &tools, allow_tool_calls, sink)
.await
{
Ok(r) => result = r,
Expand Down
8 changes: 2 additions & 6 deletions src/headless.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,18 +304,14 @@ async fn run_prompt(
});
}

let next_tools = if round < crate::MAX_TOOL_ROUNDS {
Some(tools.as_slice())
} else {
None // last round: force text
};
let allow_tool_calls = round < crate::MAX_TOOL_ROUNDS;

// Whatever this round says starts a new paragraph rather than
// continuing the sentence the tool calls interrupted.
reply.interrupt();

result = drain_to_stdout(
backend.send_tool_results(tool_results, next_tools, sink_opt),
backend.send_tool_results(tool_results, &tools, allow_tool_calls, sink_opt),
&mut sink_rx,
&mut reply,
)
Expand Down
Loading