Skip to content

Commit 09c812e

Browse files
Ax73objectkit
andauthored
fix(agent): Open AI provider tools enabled (#11)
The default chat_stream fallback on LLMProvider had two bugs that prevented tool calling for any provider without its own chat_stream implementation (currently OpenAIProvider): 1. Tools were dropped — self.chat(messages, None) passed None instead of forwarding the tools parameter. 2. ToolCalls responses were treated as errors instead of being converted to StreamChunk with tool_calls populated. This affected all OpenAI-compatible local backends (llama-server, vLLM, LM Studio) and the OpenAI API when used via the streaming chat CLI. Tested end-to-end with MiniMax M2.1 (Q2_K) via llama-server. All seven agent tools (bash, read_file, write_file, edit_file, memory_search, memory_get, web_fetch) confirmed working. See CHANGELOG.md for full diagnosis. Run unit tests: cargo test --lib agent::providers Co-authored-by: Ax <david@objectkit.com>
1 parent 01eb36d commit 09c812e

3 files changed

Lines changed: 256 additions & 14 deletions

File tree

CHANGELOG.md

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# Changelog
2+
3+
## [Unreleased]
4+
5+
### Fixed
6+
7+
#### OpenAI-compatible provider: tool calls silently dropped during streaming
8+
9+
**Problem**
10+
11+
When using the OpenAI provider with a local llama-server backend (or any
12+
OpenAI-compatible endpoint), tool calls are never executed. The model
13+
reports its available tools correctly, but when asked to use one it emits
14+
raw XML-like text instead of producing structured tool calls:
15+
16+
```
17+
LocalGPT: <tool_call>
18+
<bash>
19+
pwd
20+
</tool_call>
21+
</tool_call>
22+
```
23+
24+
The tools are never executed. The session transcript confirms the
25+
response arrives as plain text content, not as a `tool_calls` array:
26+
27+
```json
28+
{
29+
"content": [
30+
{
31+
"text": "<tool_call>\n<bash>\npwd\n</tool_call>\n</tool_call>",
32+
"type": "text"
33+
}
34+
],
35+
"role": "assistant"
36+
}
37+
```
38+
39+
However, hitting llama-server directly via curl with the same tool
40+
schemas returns a correctly structured response with
41+
`"finish_reason": "tool_calls"` and a valid `tool_calls` array.
42+
43+
**Diagnosis**
44+
45+
The `OpenAIProvider` implements `chat()` and `summarize()` but does not
46+
implement `chat_stream()`. The interactive chat CLI uses the streaming
47+
path (`agent.chat_stream_with_images()`), which falls through to the
48+
default `chat_stream` implementation on the `LLMProvider` trait.
49+
50+
Two bugs in the default fallback (`src/agent/providers.rs`, lines
51+
152-173):
52+
53+
1. **Tools are dropped.** The fallback calls `self.chat(messages, None)`
54+
— passing `None` for the tools parameter instead of forwarding the
55+
tools it received. The model never sees tool schemas in the API
56+
request, so it cannot produce structured `tool_calls` responses. It
57+
falls back to emitting its training-time tool format as raw text.
58+
59+
2. **ToolCalls response is treated as an error.** If `chat()` were to
60+
return `LLMResponseContent::ToolCalls`, the fallback returns
61+
`Err("Tool calls not supported in streaming")` instead of converting
62+
the tool calls into a `StreamChunk` with the `tool_calls` field
63+
populated.
64+
65+
The combination means: the model never receives tool schemas (bug 1),
66+
and even if it did, the response would be discarded (bug 2).
67+
68+
**Impact**
69+
70+
Any provider that relies on the default `chat_stream` fallback — which
71+
currently includes `OpenAIProvider` — cannot execute tools when used via
72+
the interactive chat CLI. This affects all OpenAI-compatible local
73+
backends (llama-server, vLLM, LM Studio, etc.) and the OpenAI API
74+
itself.
75+
76+
The Anthropic and Ollama providers are unaffected because they implement
77+
their own `chat_stream`. (Ollama separately drops tools intentionally.)
78+
79+
**Fix**
80+
81+
1. Forward the `tools` parameter in the default `chat_stream` fallback.
82+
2. Convert `ToolCalls` responses into a `StreamChunk` with `tool_calls`
83+
populated instead of returning an error.

src/agent/providers.rs

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -153,23 +153,30 @@ pub trait LLMProvider: Send + Sync {
153153
async fn chat_stream(
154154
&self,
155155
messages: &[Message],
156-
_tools: Option<&[ToolSchema]>,
156+
tools: Option<&[ToolSchema]>,
157157
) -> Result<StreamResult> {
158158
// Default implementation: single chunk with full response
159-
let resp = self.chat(messages, None).await?;
160-
let text = match resp.content {
161-
LLMResponseContent::Text(t) => t,
162-
LLMResponseContent::ToolCalls(_) => {
163-
return Err(anyhow::anyhow!("Tool calls not supported in streaming"))
159+
let resp = self.chat(messages, tools).await?;
160+
match resp.content {
161+
LLMResponseContent::Text(text) => {
162+
Ok(Box::pin(futures::stream::once(async move {
163+
Ok(StreamChunk {
164+
delta: text,
165+
done: true,
166+
tool_calls: None,
167+
})
168+
})))
164169
}
165-
};
166-
Ok(Box::pin(futures::stream::once(async move {
167-
Ok(StreamChunk {
168-
delta: text,
169-
done: true,
170-
tool_calls: None,
171-
})
172-
})))
170+
LLMResponseContent::ToolCalls(calls) => {
171+
Ok(Box::pin(futures::stream::once(async move {
172+
Ok(StreamChunk {
173+
delta: String::new(),
174+
done: true,
175+
tool_calls: Some(calls),
176+
})
177+
})))
178+
}
179+
}
173180
}
174181
}
175182

@@ -1738,6 +1745,10 @@ impl LLMProvider for ClaudeCliProvider {
17381745
}
17391746
}
17401747

1748+
#[cfg(test)]
1749+
#[path = "./test/unit/openaiprovider_tool_test.rs"]
1750+
mod providers_test;
1751+
17411752
#[cfg(test)]
17421753
mod tests {
17431754
use super::*;
@@ -1798,4 +1809,5 @@ mod tests {
17981809
"custom-model".to_string()
17991810
);
18001811
}
1812+
18011813
}
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
//! Tests for the default `chat_stream` fallback on `LLMProvider`.
2+
//!
3+
//! The default implementation delegates to `chat()` and wraps the result
4+
//! in a single `StreamChunk`. These tests verify that:
5+
//! 1. Tool schemas are forwarded (not dropped)
6+
//! 2. Text responses produce a valid stream chunk
7+
//! 3. ToolCalls responses produce a stream chunk with tool_calls populated
8+
9+
use anyhow::Result;
10+
use async_trait::async_trait;
11+
use futures::StreamExt;
12+
13+
use super::*;
14+
15+
/// Mock provider that returns a configured response from chat(),
16+
/// used to test the default chat_stream fallback on LLMProvider.
17+
struct MockProvider {
18+
response: std::sync::Mutex<Option<LLMResponse>>,
19+
/// Captures whether tools were forwarded to chat()
20+
received_tools: std::sync::Mutex<bool>,
21+
}
22+
23+
impl MockProvider {
24+
fn returning_text(text: &str) -> Self {
25+
Self {
26+
response: std::sync::Mutex::new(Some(LLMResponse::text(text.to_string()))),
27+
received_tools: std::sync::Mutex::new(false),
28+
}
29+
}
30+
31+
fn returning_tool_calls(calls: Vec<ToolCall>) -> Self {
32+
Self {
33+
response: std::sync::Mutex::new(Some(LLMResponse::tool_calls(calls))),
34+
received_tools: std::sync::Mutex::new(false),
35+
}
36+
}
37+
}
38+
39+
#[async_trait]
40+
impl LLMProvider for MockProvider {
41+
async fn chat(
42+
&self,
43+
_messages: &[Message],
44+
tools: Option<&[ToolSchema]>,
45+
) -> Result<LLMResponse> {
46+
if let Some(t) = tools {
47+
if !t.is_empty() {
48+
*self.received_tools.lock().unwrap() = true;
49+
}
50+
}
51+
self.response
52+
.lock()
53+
.unwrap()
54+
.take()
55+
.ok_or_else(|| anyhow::anyhow!("MockProvider exhausted"))
56+
}
57+
58+
async fn summarize(&self, _text: &str) -> Result<String> {
59+
Ok(String::new())
60+
}
61+
}
62+
63+
#[tokio::test]
64+
async fn test_default_chat_stream_forwards_tools() {
65+
let provider = MockProvider::returning_text("hello");
66+
let messages = vec![Message {
67+
role: Role::User,
68+
content: "test".to_string(),
69+
tool_calls: None,
70+
tool_call_id: None,
71+
images: Vec::new(),
72+
}];
73+
let tools = vec![ToolSchema {
74+
name: "bash".to_string(),
75+
description: "Execute a command".to_string(),
76+
parameters: serde_json::json!({"type": "object"}),
77+
}];
78+
79+
let _stream = provider
80+
.chat_stream(&messages, Some(&tools))
81+
.await
82+
.expect("chat_stream should succeed");
83+
84+
assert!(
85+
*provider.received_tools.lock().unwrap(),
86+
"Default chat_stream must forward tools to chat()"
87+
);
88+
}
89+
90+
#[tokio::test]
91+
async fn test_default_chat_stream_returns_text_as_stream_chunk() {
92+
let provider = MockProvider::returning_text("hello world");
93+
let messages = vec![Message {
94+
role: Role::User,
95+
content: "test".to_string(),
96+
tool_calls: None,
97+
tool_call_id: None,
98+
images: Vec::new(),
99+
}];
100+
101+
let mut stream = provider
102+
.chat_stream(&messages, None)
103+
.await
104+
.expect("chat_stream should succeed");
105+
106+
let chunk = stream.next().await.expect("stream should yield a chunk");
107+
let chunk = chunk.expect("chunk should be Ok");
108+
109+
assert_eq!(chunk.delta, "hello world");
110+
assert!(chunk.done);
111+
assert!(chunk.tool_calls.is_none());
112+
}
113+
114+
#[tokio::test]
115+
async fn test_default_chat_stream_returns_tool_calls_as_stream_chunk() {
116+
let calls = vec![ToolCall {
117+
id: "call_1".to_string(),
118+
name: "bash".to_string(),
119+
arguments: "{\"command\":\"pwd\"}".to_string(),
120+
}];
121+
let provider = MockProvider::returning_tool_calls(calls);
122+
let messages = vec![Message {
123+
role: Role::User,
124+
content: "test".to_string(),
125+
tool_calls: None,
126+
tool_call_id: None,
127+
images: Vec::new(),
128+
}];
129+
130+
let mut stream = provider
131+
.chat_stream(&messages, None)
132+
.await
133+
.expect("chat_stream should succeed");
134+
135+
let chunk = stream.next().await.expect("stream should yield a chunk");
136+
let chunk = chunk.expect("chunk should be Ok");
137+
138+
assert!(chunk.done);
139+
assert!(
140+
chunk.delta.is_empty(),
141+
"tool call chunk should have empty delta"
142+
);
143+
let tool_calls = chunk.tool_calls.expect("chunk should contain tool_calls");
144+
assert_eq!(tool_calls.len(), 1);
145+
assert_eq!(tool_calls[0].name, "bash");
146+
assert_eq!(tool_calls[0].arguments, "{\"command\":\"pwd\"}");
147+
}

0 commit comments

Comments
 (0)