fix(ollama): preserve reasoning and tool-call context - #6605
Conversation
WalkthroughOllama request conversion now preserves assistant reasoning, response formats, tool-call identifiers, and tool names. Chat and generate requests always serialize stream settings. Stream tests validate upstream and generated tool-call IDs. ChangesOllama conversion updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/relay-ollama.go`:
- Around line 183-198: In the tool-call parsing loop around common.Unmarshal,
stop discarding JSON unmarshal errors and log the error with sufficient context,
including the affected tool call or function name. Preserve the existing
fallback behavior for nil arguments while ensuring malformed
tc.Function.Arguments produces a visible diagnostic.
- Around line 42-69: Update the effort switch in openAIChatToOllamaChat to map
OpenAI’s “minimal” value to Ollama’s “low” and “xhigh” to “max”; remove the
converter-level error for other unsupported values so they pass through for
Ollama to validate, while preserving existing mappings and error handling for
reasoning unmarshalling and think marshaling.
🪄 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 Plus
Run ID: 2da144aa-6702-4142-b767-651328fb300b
📒 Files selected for processing (4)
relay/channel/ollama/dto.gorelay/channel/ollama/relay-ollama.gorelay/channel/ollama/stream.gorelay/channel/ollama/stream_test.go
| func openAIChatToOllamaChat(c *gin.Context, r *dto.GeneralOpenAIRequest) (*OllamaChatRequest, error) { | ||
| think := r.Think | ||
| if len(think) == 0 { | ||
| effort := r.ReasoningEffort | ||
| if len(r.Reasoning) > 0 { | ||
| var reasoning dto.Reasoning | ||
| if err := common.Unmarshal(r.Reasoning, &reasoning); err != nil { | ||
| return nil, fmt.Errorf("invalid ollama reasoning: %w", err) | ||
| } | ||
| effort = lo.CoalesceOrEmpty(reasoning.Effort, effort) | ||
| } | ||
| if effort != "" { | ||
| var thinkValue any | ||
| switch effort { | ||
| case "none": | ||
| thinkValue = false | ||
| case "low", "medium", "high", "max": | ||
| thinkValue = effort | ||
| default: | ||
| return nil, fmt.Errorf("unsupported ollama reasoning effort %q", effort) | ||
| } | ||
| var err error | ||
| think, err = common.Marshal(thinkValue) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("marshal ollama think: %w", err) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What values does Ollama's think API parameter currently accept, and what reasoning_effort values does OpenAI currently support across models?
💡 Result:
Ollama's think API parameter accepts boolean values (true/false) and reasoning level strings: "low", "medium", "high", and "max" [1][2]. While these values are supported at the top level for most models, the "max" level is sometimes specifically designated for use within the options object (e.g., options: {"think": "max"}) depending on the specific model and API implementation [3]. Some models, such as GPT-OSS, strictly require one of the reasoning levels ("low", "medium", or "high") and do not accept boolean values [1][4][5]. OpenAI supports several reasoning_effort values across its reasoning-capable models: "none", "minimal", "low", "medium", "high", "xhigh", and "max" [6][7][8]. The availability of these values is model-dependent [6][7]. For instance, certain models may default to "medium" and not support "none", while others might support "xhigh" or "max" only on specific advanced versions [8][9]. Users should consult model-specific documentation to verify which effort levels are compatible with a chosen model [6][7].
Citations:
- 1: https://docs.ollama.com/capabilities/thinking
- 2: https://github.com/ollama/ollama/blob/main/docs/api.md
- 3: API: Add max as valid value for top-level think parameter ollama/ollama#15831
- 4: https://github.com/ollama/ollama/blob/8a0016f8/docs/capabilities/thinking.mdx
- 5: Does the Ollama /api/chat endpoint support setting Reasoning Effort? ollama/ollama#11752
- 6: https://developers.openai.com/api/docs/guides/reasoning
- 7: https://github.com/openai/openai-python/blob/f16fbbd2/src/openai/types/shared_params/reasoning.py
- 8: https://modelparams.dev/parameters/reasoning_effort
- 9: https://github.com/openai/openai-java/blob/b855c3b3/openai-java-core/src/main/kotlin/com/openai/models/Reasoning.kt
🌐 Web query:
site:developers.openai.com/api/docs/guides/reasoning reasoning_effort minimal xhigh
💡 Result:
The reasoning_effort parameter is a configuration setting in OpenAI's reasoning models (such as the o-series) that controls the amount of computational processing, or thinking, the model performs before generating a final response [1]. By adjusting this parameter, you can influence the trade-off between latency, cost, and output quality [1]. The available settings typically range from minimal to max, with higher settings instructing the model to spend more time planning, analyzing, and reasoning through complex problems [1]. Key characteristics of these effort levels include: minimal: Prioritizes speed and lower token usage, making it suitable for simpler tasks where immediate, direct answers are preferred [1]. xhigh and max: Direct the model to engage in deeper, more comprehensive reasoning, which is recommended for complex debugging, long-horizon research, high-stakes decision-making, and other intellectually demanding workflows where accuracy and quality are more critical than low latency [1]. In practice, the models are designed to be adaptive, meaning that even with a specific effort setting, they may use fewer tokens for simple tasks and "think harder" only when the complexity of the input requires it [1]. The default effort setting, such as medium, is generally considered the best starting point for balancing performance, quality, and reliability [1].
Citations:
🌐 Web query:
site:docs.ollama.com/capabilities/thinking Ollama think low medium high max false true
💡 Result:
In Ollama, the think parameter is used to control the reasoning trace for supported models [1]. Users can specify the level of reasoning or toggle the feature on and off using the following options: Boolean values: - true: Enables the thinking process (default for supported models) [1]. - false: Disables the reasoning trace [1]. Thinking levels: - low, medium, high: These levels allow users to tune the length and depth of the model's reasoning trace [1]. - max: This option requests the highest level of thinking available for the model [1]. Note that some specific models, such as GPT-OSS, only accept the levels (low, medium, or high) rather than boolean values [1]. In addition to the API parameters, users can manage thinking in the CLI using commands like /set think, /set nothink, or the --hidethinking flag to perform reasoning without displaying the trace in the final output [1].
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the converter and nearby stream/channel support without executing repository code.
wc -l relay/channel/ollama/relay-ollama.go
sed -n '1,140p' relay/channel/ollama/relay-ollama.go
printf '\n--- streamSupportedChannels references ---\n'
rg -n "streamSupportedChannels|OpenAIChatToOllamaChat|openAIChatToOllamaChat" relay/channel/ollama relay -g '*.go' || trueRepository: QuantumNous/new-api
Length of output: 4646
Allow unsupported OpenAI reasoning-effort values to pass through.
OpenAI accepts minimal and xhigh, but this converter only maps none, low, medium, high, and max. Those values hit default, return an error, and abort the Ollama request conversion. Map minimal to low and xhigh to max, and let Ollama reject any other think value it does not support.
🤖 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/relay-ollama.go` around lines 42 - 69, Update the effort
switch in openAIChatToOllamaChat to map OpenAI’s “minimal” value to Ollama’s
“low” and “xhigh” to “max”; remove the converter-level error for other
unsupported values so they pass through for Ollama to validate, while preserving
existing mappings and error handling for reasoning unmarshalling and think
marshaling.
| for _, tc := range parsed { | ||
| var args interface{} | ||
| if tc.Function.Arguments != "" { | ||
| _ = json.Unmarshal([]byte(tc.Function.Arguments), &args) | ||
| _ = common.Unmarshal([]byte(tc.Function.Arguments), &args) | ||
| } | ||
| if args == nil { | ||
| args = map[string]any{} | ||
| } | ||
| oc := OllamaToolCall{} | ||
| oc := OllamaToolCall{ID: tc.ID} | ||
| oc.Function.Name = tc.Function.Name | ||
| oc.Function.Arguments = args | ||
| calls = append(calls, oc) | ||
| if tc.ID != "" { | ||
| toolNamesByCallID[tc.ID] = tc.Function.Name | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Malformed tool-call arguments fail silently instead of surfacing an error.
common.Unmarshal errors on line 186 are discarded with _ =. If tc.Function.Arguments is malformed JSON, args silently falls back to map[string]any{}, and the tool call is forwarded to Ollama with empty arguments instead of the original (possibly still partially useful) payload or a visible error. This can silently drop tool-call context in multi-turn flows, which is the exact failure mode this PR aims to fix for reasoning content.
Log the unmarshal error so malformed payloads are diagnosable, instead of dropping them without a trace.
🐛 Proposed fix to surface unmarshal failures
var args interface{}
if tc.Function.Arguments != "" {
- _ = common.Unmarshal([]byte(tc.Function.Arguments), &args)
+ if err := common.Unmarshal([]byte(tc.Function.Arguments), &args); err != nil {
+ logger.LogError(c, "ollama tool call arguments decode error: "+err.Error())
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for _, tc := range parsed { | |
| var args interface{} | |
| if tc.Function.Arguments != "" { | |
| _ = json.Unmarshal([]byte(tc.Function.Arguments), &args) | |
| _ = common.Unmarshal([]byte(tc.Function.Arguments), &args) | |
| } | |
| if args == nil { | |
| args = map[string]any{} | |
| } | |
| oc := OllamaToolCall{} | |
| oc := OllamaToolCall{ID: tc.ID} | |
| oc.Function.Name = tc.Function.Name | |
| oc.Function.Arguments = args | |
| calls = append(calls, oc) | |
| if tc.ID != "" { | |
| toolNamesByCallID[tc.ID] = tc.Function.Name | |
| } | |
| } | |
| for _, tc := range parsed { | |
| var args interface{} | |
| if tc.Function.Arguments != "" { | |
| if err := common.Unmarshal([]byte(tc.Function.Arguments), &args); err != nil { | |
| logger.LogError(c, "ollama tool call arguments decode error: "+err.Error()) | |
| } | |
| } | |
| if args == nil { | |
| args = map[string]any{} | |
| } | |
| oc := OllamaToolCall{ID: tc.ID} | |
| oc.Function.Name = tc.Function.Name | |
| oc.Function.Arguments = args | |
| calls = append(calls, oc) | |
| if tc.ID != "" { | |
| toolNamesByCallID[tc.ID] = tc.Function.Name | |
| } | |
| } |
🤖 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/relay-ollama.go` around lines 183 - 198, In the
tool-call parsing loop around common.Unmarshal, stop discarding JSON unmarshal
errors and log the error with sufficient context, including the affected tool
call or function name. Preserve the existing fallback behavior for nil arguments
while ensuring malformed tc.Function.Arguments produces a visible diagnostic.
* fix(relay): set Request.GetBody so the HTTP/2 transport can transparently retry after an upstream stream reset (QuantumNous#6249) * fix(relay): set Request.GetBody so the HTTP/2 transport can transparently retry after an upstream stream reset The outbound request body is a type-erased io.Reader over BodyStorage, so net/http cannot derive Request.GetBody (it only does so for *bytes.Reader, *bytes.Buffer and *strings.Reader). With GetBody nil, the HTTP/2 transport cannot transparently retry a request once the body has been written and the upstream resets the stream with a retryable error (REFUSED_STREAM, or a connection-level GOAWAY); the relay request then fails with: http2: Transport: cannot retry err [...] after Request.Body was written; define Request.GetBody to avoid this error This affects every relay path that goes through DoApiRequest (chat, claude, gemini, responses, embedding, image, rerank). BodyStorage (memory and disk) already implements io.Seeker, so replay support only needed wiring: - NewOutboundJSONBody additionally returns a getBody that rewinds the storage and hands out a fresh non-closing reader. The transport only calls GetBody after the previous attempt's body has been abandoned, so the rewind cannot race an in-flight read. - RelayInfo carries it in the new UpstreamRequestGetBody field, set alongside UpstreamRequestBodySize by the handlers that build storage-backed bodies. - applyUpstreamGetBody (symmetric with applyUpstreamContentLength) wires it into DoApiRequest/DoFormRequest/DoTaskApiRequest, only when req.GetBody is still nil. Also remove the hand-rolled GetBody override in DoTaskApiRequest: it returned the same already-consumed reader, so any transport-level replay would have silently sent an empty body, and it clobbered the correct snapshot-based GetBody that net/http derives from the *bytes.Reader bodies the task adaptors pass in. For non-replayable bodies GetBody now stays nil, so a retry fails loudly instead of corrupting the request. Covered by unit tests plus an end-to-end raw-frame HTTP/2 test that resets the first stream with REFUSED_STREAM after the body is written and asserts the transport transparently retries with the complete body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(relay): hand out independent readers from GetBody (address review) Per the http.Request.GetBody contract ("returns a new copy of Body"), each call must yield a reader with its own cursor. The previous implementation rewound and reused the shared BodyStorage, so two consecutive GetBody readers would interfere with each other, and a replay could disturb the primary body's offset under extreme transport timing (e.g. attempt N's body write not yet fully abandoned when the transport builds attempt N+1). Instead of snapshotting the payload (an extra copy), add BodyStorage.NewReader, which returns an independent zero-copy reader: - memory mode: a fresh bytes.Reader over the same immutable backing array; - disk mode: a separate file descriptor over the cache file, so the transport closing a replayed body only closes that descriptor. NewOutboundJSONBody's getBody now simply hands out storage.NewReader, and once the handler releases the storage, GetBody fails with ErrStorageClosed instead of replaying stale data. Tests: interleaved reads across two replay readers and the primary body each observe exactly their own byte stream, for both the memory and the disk-backed storage; the existing GetBody and HTTP/2 retry suites still pass (h2 e2e tests flake-free with -count=20). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(relay): bind replayable metadata on pass-through requests * fix(relay): reset upstream body metadata between channels * test(relay): cover replay across retries and channel attempts * fix(relay): stop following upstream redirects --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * refactor(relay): move replay metadata onto request bodies * Merge commit from fork * feat(channels): refine fetched model categorization (QuantumNous#6632) * feat(channels): refine fetched model categorization * fix: channel category * fix: hy3 category * fix: test Claude/Gemini endpoints with native request format (QuantumNous#6698) * feat(rate-limit): add user critical rate limit middleware for access token and aff transfer routes * fix: 修复兑换码额度精度损失 (QuantumNous#6685) * fix: 修复兑换码额度精度损失(QuantumNous#6680) * fix(redemption): guard update data integrity * CI: enhance release synchronization workflow with optional file syncing * fix(ali): stop injecting top_p into requests that omit it (QuantumNous#6674) * fix(channels): classify Qwen TTS models correctly (QuantumNous#6711) * feat(channels): add auto-disable-only channel test mode (QuantumNous#6728) * perf(web): debounce server and large-list searches (QuantumNous#6727) * fix: record reasoning effort consistently in usage logs (QuantumNous#6641) * feat(relay): expose user and group context to parameter overrides (QuantumNous#6534) * fix(ollama): preserve reasoning and tool-call context (QuantumNous#6605) * fix: backend length validation (QuantumNous#5548) * feat(billing): highlight matched conditional multipliers in logs (QuantumNous#6561) * feat(billing): highlight matched conditional multipliers in usage logs * fix(billing): make request rule tracing stable and type-safe * fix(web): require confirmation before rotating access token (QuantumNous#6749) --------- Co-authored-by: Lucas <hepo.lucas@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: CaIon <i@caion.me> Co-authored-by: RedwindA <128586631+RedwindA@users.noreply.github.com> Co-authored-by: Seefs <40468931+seefs001@users.noreply.github.com> Co-authored-by: lihu-001 <lihu9048@gmail.com> Co-authored-by: ENCHIGO <38551565+ENCHIGO@users.noreply.github.com>
2 participants
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
对OpenAI chat completions -> ollama chat 缺失的部分进行补全
reasoning_content/reasoning 转 ollama thinking
reasoning_effort/reasoning.effort -> think( none -> think=none)
json_object -> json
stream 显式处理
tool_call_id 补全
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
New Features
Bug Fixes