Skip to content

fix: handle ollama non-stream tool calls - #5865

Merged
Calcium-Ion merged 2 commits into
QuantumNous:mainfrom
stevenjoezhang:fix/ollama-nonstream-tool-calls
Jul 3, 2026
Merged

fix: handle ollama non-stream tool calls#5865
Calcium-Ion merged 2 commits into
QuantumNous:mainfrom
stevenjoezhang:fix/ollama-nonstream-tool-calls

Conversation

@stevenjoezhang

@stevenjoezhang stevenjoezhang commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

⚠️ 提交说明 / PR Notice

Important

  • 请提供人工撰写的简洁摘要,避免直接粘贴未经整理的 AI 输出。

📝 变更描述 / Description

(简述:做了什么?为什么这样改能生效?请基于你对代码逻辑的理解来写,避免粘贴未经整理的内容)

本次变更增加了新功能,支持了 Ollama 非流式响应中工具调用的功能。

Ollama 在 /api/chat 非流式模式下返回工具调用时,会把结果放在 message.tool_calls 中,但原有的 ollamaChatHandler 只聚合了 message.contentresponse 和 reasoning 内容,没有读取并转换 message.tool_calls,因此最终返回给 OpenAI 兼容客户端的响应中会丢失工具调用信息,finish_reason 也仍然是上游的 stop

本次修改将 Ollama 的 tool_calls 统一转换为 OpenAI 兼容的 ToolCallResponse 结构,这样客户端在收到非流式响应时,可以正确识别模型触发了工具调用,并继续执行后续 tool call 流程。

🚀 变更类型 / Type of change

  • 🐛 Bug 修复 (Bug fix) - 请关联对应 Issue,避免将设计取舍、理解偏差或预期不一致直接归类为 bug
  • ✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
  • ⚡ 性能优化 / 重构 (Refactor)
  • 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

  • Closes # (如有)

✅ 提交前检查项 / Checklist

  • 人工确认: 我已亲自整理并撰写此描述,没有直接粘贴未经处理的 AI 输出。
  • 非重复提交: 我已搜索现有的 IssuesPRs,确认不是重复提交。
  • Bug fix 说明: 若此 PR 标记为 Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
  • 变更理解: 我已理解这些更改的工作原理及可能影响。
  • 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
  • 本地验证: 已在本地运行并通过测试或手动验证,维护者可以据此复核结果。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

📸 运行证明 / Proof of Work

(请在此粘贴截图、关键日志或测试报告,以证明变更生效)

使用真实 Ollama 上游进行了验证:

  • Ollama 上游地址:http://127.0.0.1:11434
  • 模型:qwen3:8b
  • 请求接口:/api/chat
  • 请求模式:"stream": false
  • 验证场景:Ollama 非流式响应返回 message.tool_calls,new-api 应正确转换为 OpenAI 兼容的 message.tool_calls,并将 finish_reason 设置为 "tool_calls"

执行命令:

OLLAMA_BASE_URL=http://127.0.0.1:11434 OLLAMA_MODEL=qwen3:8b \
go test ./relay/channel/ollama -run TestLiveOllamaNonStreamToolCallsThroughHandler -count=1 -v

补丁前,在 upstream/main8874d192)上,同一个 live regression test 会失败:

raw ollama response: ... "tool_calls":[{"id":"call_hj8w65to","function":{"index":0,"name":"get_weather","arguments":{"city":"Paris","days":3}}}] ... "done_reason":"stop"

converted openai response: ... "message":{"role":"assistant","content":null,"reasoning_content":"..."},"finish_reason":"stop" ...

expected finish_reason "tool_calls", got "stop"
--- FAIL: TestLiveOllamaNonStreamToolCallsThroughHandler
FAIL

补丁后,在当前提交 5f183a48 上,同一个 live regression test 通过:

raw ollama response: ... "tool_calls":[{"id":"call_5hd4f57n","function":{"index":0,"name":"get_weather","arguments":{"city":"Paris","days":3}}}] ... "done_reason":"stop"

converted openai response: ... "message":{"role":"assistant","content":null,"reasoning_content":"...","tool_calls":[{"id":"call_0","type":"function","function":{"name":"get_weather","arguments":"{\"city\":\"Paris\",\"days\":3}"}}]},"finish_reason":"tool_calls" ...

--- PASS: TestLiveOllamaNonStreamToolCallsThroughHandler
PASS
ok github.com/QuantumNous/new-api/relay/channel/ollama

Summary by CodeRabbit

  • Bug Fixes
    • Improved Ollama chat parsing so tool calls are reliably surfaced in both streaming and non-streaming responses.
    • Corrected completion/finish status reporting when tool calls are present.
    • Standardized tool-call serialization for consistent argument handling, including safer fallbacks for missing fields.
  • Tests
    • Added coverage for non-stream tool-call responses using both compact and indented upstream payloads, validating finish status, tool-call fields, and decoded arguments.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 45dd83d1-f656-4e0e-ad90-0edccb0bceeb

📥 Commits

Reviewing files that changed from the base of the PR and between 5f183a4 and 8b8a08c.

📒 Files selected for processing (1)
  • relay/channel/ollama/stream_test.go

Walkthrough

Ollama chat handling now uses shared JSON helpers, a typed tool-call shape, and a common conversion path for stream and non-stream responses. Non-stream output now preserves tool calls in the final message and sets finish_reason to tool_calls, with a new test covering compact and pretty payloads.

Changes

Ollama tool call handling

Layer / File(s) Summary
Typed tool call struct and shared conversion helper
relay/channel/ollama/stream.go
Adds the constant import, replaces the inline tool_calls shape with typed OllamaToolCall, and introduces ollamaToolCallsToOpenAI for argument serialization and OpenAI tool-call response conversion.
Stream handler tool call and JSON decode updates
relay/channel/ollama/stream.go
Switches stream decoding to common.Unmarshal, uses the shared helper for delta tool calls, and sets finish_reason to tool_calls when tool calls are emitted.
Non-stream handler tool call aggregation and finish reason
relay/channel/ollama/stream.go
Switches non-stream decoding to common.Unmarshal, accumulates tool calls across parsed frames and fallback parsing, and includes serialized tool calls in the final assistant message with finish_reason overridden.
Non-stream tool call test
relay/channel/ollama/stream_test.go
Adds a test that validates non-stream tool-call decoding, response shape, and the absence of a tool-call index field.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • QuantumNous/new-api#1811: Related Ollama DTO and tool-call shape changes feed the parsing and conversion updates in this PR.

Suggested reviewers: creamlike1024

Poem

A rabbit nibbled JSON in a stream,
Then hopped to helpers with a gleam.
Tool calls formed, neat and bright,
Finish reasons set just right.
🐰🥕 The Ollama meadow now flows clean and keen.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: handling Ollama non-stream tool calls.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
relay/channel/ollama/stream_test.go (1)

18-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the single-chunk fallback path.

The raw body is a single-line JSON, so it's consumed by the multi-line loop's success path (parsedAny=true); the !parsedAny fallback branch in ollamaChatHandler (stream.go lines 265-294), which has its own independent ollamaToolCallsToOpenAI call for single.Message.ToolCalls, remains untested.

Consider adding a second table case with a body that fails per-line JSON parsing (e.g. pretty-printed/multi-line JSON) to exercise the fallback branch's tool-call aggregation.

🤖 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 `@relay/channel/ollama/stream_test.go` around lines 18 - 72, The current test
only covers the multi-line parsing success path in ollamaChatHandler, leaving
the single-chunk fallback branch untested. Add a second test case (or table
entry) in TestOllamaChatHandlerNonStreamToolCalls that uses a body format
causing per-line JSON parsing to fail, so the !parsedAny fallback path is
exercised. Verify that the fallback still aggregates tool calls correctly via
ollamaToolCallsToOpenAI on single.Message.ToolCalls, including finish reason,
tool-call contents, and the absence of Index for non-stream tool calls.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@relay/channel/ollama/stream_test.go`:
- Around line 1-72: The new test in ollamaChatHandlerNonStreamToolCalls uses
manual t.Fatalf checks instead of the required testify helpers. Update this test
to import github.com/stretchr/testify/require and
github.com/stretchr/testify/assert, use require for setup and error/fatal checks
around ollamaChatHandler, common.Unmarshal, and response decoding, and use
assert for value comparisons on usage, FinishReason, tool call fields, and
arguments. Keep the test logic the same but replace the hand-rolled if blocks
with the appropriate require/assert calls throughout the function.

---

Nitpick comments:
In `@relay/channel/ollama/stream_test.go`:
- Around line 18-72: The current test only covers the multi-line parsing success
path in ollamaChatHandler, leaving the single-chunk fallback branch untested.
Add a second test case (or table entry) in
TestOllamaChatHandlerNonStreamToolCalls that uses a body format causing per-line
JSON parsing to fail, so the !parsedAny fallback path is exercised. Verify that
the fallback still aggregates tool calls correctly via ollamaToolCallsToOpenAI
on single.Message.ToolCalls, including finish reason, tool-call contents, and
the absence of Index for non-stream tool calls.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2443349d-296a-4833-a11b-12da748aae4f

📥 Commits

Reviewing files that changed from the base of the PR and between 8874d19 and 5f183a4.

📒 Files selected for processing (2)
  • relay/channel/ollama/stream.go
  • relay/channel/ollama/stream_test.go

Comment thread relay/channel/ollama/stream_test.go
@Calcium-Ion
Calcium-Ion merged commit 0977965 into QuantumNous:main Jul 3, 2026
1 check was pending
YuanQianQ pushed a commit to YuanQianQ/new-api that referenced this pull request Jul 4, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
(cherry picked from commit 0977965)
liulixin-lex pushed a commit to liulixin-lex/xy-api that referenced this pull request Jul 7, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
0401lucky pushed a commit to 0401lucky/new-api that referenced this pull request Jul 7, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
(cherry picked from commit 0977965)
fran0220 pushed a commit to fran0220/you-box that referenced this pull request Jul 9, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
(cherry picked from commit 0977965)
xiaomingchen pushed a commit to xiaomingchen/new-api that referenced this pull request Jul 10, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
Jacobinwwey pushed a commit to Jacobinwwey/new-api that referenced this pull request Jul 11, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
ruanhangjian pushed a commit to ruanhangjian/new-api that referenced this pull request Jul 11, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
noah-wung pushed a commit to noah-wung/new-api that referenced this pull request Jul 17, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
zhaodechao2008 pushed a commit to zhaodechao2008/new-api that referenced this pull request Jul 27, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
330079598 pushed a commit to 330079598/new-api that referenced this pull request Aug 19, 2026
* fix: handle ollama non-stream tool calls

* test: cover ollama non-stream tool call paths

---------

Co-authored-by: CaIon <i@caion.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants