fix: preserve reasoning_content across request - #6655
Conversation
WalkthroughThe change adds configurable reasoning-content preservation. Upstream model matching enables the conversion option, which propagates through request converters for OpenAI, Claude, Gemini, and Responses formats. Tests cover enabled, disabled, and tool-call scenarios. Web settings expose the model patterns. ChangesReasoning preservation configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
relaykit/relayconvert/internal/oai_chat/to_claude_messages_req.go (1)
346-363: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the
"..."text sentinel with placeholder suppression at the source.Lines 260-262 inject the
"..."placeholder when content is empty. Lines 361-363 then remove that placeholder again by matching the literal text. This drops a genuine assistant text part whose value is exactly"...". Skip the placeholder injection when reasoning content exists, then the sentinel match is unnecessary.♻️ Proposed change at the placeholder source (lines 260-262)
- if fmtMessage.Content == nil || (fmtMessage.IsStringContent() && fmtMessage.StringContent() == "") { + if fmtMessage.GetReasoningContent() == "" && + (fmtMessage.Content == nil || (fmtMessage.IsStringContent() && fmtMessage.StringContent() == "")) { fmtMessage.SetStringContent("...") }Then remove the sentinel check:
for _, mediaMessage := range message.ParseContent() { switch mediaMessage.Type { case "text": - if reasoningEmitted && mediaMessage.Text == "..." { - continue - } if mediaMessage.Text != "" {🤖 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 `@relaykit/relayconvert/internal/oai_chat/to_claude_messages_req.go` around lines 346 - 363, The `"..."` placeholder is being injected unconditionally at the source (lines 260-262) when content is empty, and then removed by matching the literal text in the loop that processes mediaMessage items (the reasoningEmitted && mediaMessage.Text == "..." check). This approach incorrectly drops genuine assistant text parts with that exact value. Modify the placeholder injection at the source to skip injection when reasoning content has been emitted, then remove the sentinel-matching check in the loop around mediaMessage.Text == "..." so the condition is no longer needed.
🤖 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 `@relaykit/relayconvert/internal/oai_responses/to_oai_chat_req.go`:
- Around line 268-279: Update attachPendingReasoningToLastAssistant so the
synthesized assistant dto.Message explicitly sets Content to an empty string
alongside Role and ReasoningContent. Leave the existing behavior unchanged for
pending reasoning and existing assistant messages.
In `@web/src/i18n/locales/fr.json`:
- Line 3661: Update the French translation value for the "Reasoning Content
Models" key in the fr.json file to fully translate "Reasoning Content" into
French instead of leaving it in English. Replace "Reasoning Content" with the
established French terminology used in the surrounding translation entries to
create a complete French-language translation.
---
Nitpick comments:
In `@relaykit/relayconvert/internal/oai_chat/to_claude_messages_req.go`:
- Around line 346-363: The `"..."` placeholder is being injected unconditionally
at the source (lines 260-262) when content is empty, and then removed by
matching the literal text in the loop that processes mediaMessage items (the
reasoningEmitted && mediaMessage.Text == "..." check). This approach incorrectly
drops genuine assistant text parts with that exact value. Modify the placeholder
injection at the source to skip injection when reasoning content has been
emitted, then remove the sentinel-matching check in the loop around
mediaMessage.Text == "..." so the condition is no longer needed.
🪄 Autofix
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: d5933ae9-2cff-47f0-9012-bb3103933abd
📒 Files selected for processing (29)
relay/common/relay_info.gorelaykit/relayconvert/convmeta/options.gorelaykit/relayconvert/internal/claude_messages/to_oai_chat_req.gorelaykit/relayconvert/internal/gemini_chat/to_oai_chat_req.gorelaykit/relayconvert/internal/oai_chat/to_claude_messages_req.gorelaykit/relayconvert/internal/oai_chat/to_gemini_chat_req.gorelaykit/relayconvert/internal/oai_chat/to_oai_responses_req.gorelaykit/relayconvert/internal/oai_chat/to_oai_responses_req_test.gorelaykit/relayconvert/internal/oai_responses/to_claude_messages_req.gorelaykit/relayconvert/internal/oai_responses/to_gemini_chat_req.gorelaykit/relayconvert/internal/oai_responses/to_oai_chat_req.gorelaykit/relayconvert/internal/oai_responses/to_oai_chat_req_test.gorelaykit/relayconvert/reasoning_request_test.gorelaykit/relayconvert/request_compat.gorelaykit/relayconvert/request_registry.goservice/convert_test.gosetting/model_setting/global.goweb/src/features/models/components/drawers/model-mutate-drawer.tsxweb/src/features/system-settings/models/global-settings-card.tsxweb/src/features/system-settings/models/index.tsxweb/src/features/system-settings/models/section-registry.tsxweb/src/features/system-settings/types.tsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.json
| func attachPendingReasoningToLastAssistant(messages []dto.Message, pending *string) []dto.Message { | ||
| if pending == nil || *pending == "" { | ||
| return messages | ||
| } | ||
| reasoning := *pending | ||
| *pending = "" | ||
| if len(messages) == 0 || messages[len(messages)-1].Role != "assistant" { | ||
| return append(messages, dto.Message{Role: "assistant", ReasoningContent: &reasoning}) | ||
| } | ||
| appendReasoningContent(&messages[len(messages)-1], reasoning) | ||
| return messages | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm Content has no omitempty and inspect sibling assistant-message construction.
rg -nP -C2 'Content\s+any\s+`json:"content' relaykit/dto
rg -nP -C3 'dto\.Message\{Role: "assistant"' relaykitRepository: QuantumNous/new-api
Length of output: 6119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline relaykit/relayconvert/internal/oai_responses/to_oai_chat_req.go 2>/dev/null || true
echo "== target function and nearby converter logic =="
sed -n '240,290p' relaykit/relayconvert/internal/oai_responses/to_oai_chat_req.go
sed -n '380,415p' relaykit/relayconvert/internal/oai_responses/to_oai_chat_req.go
echo "== Message types =="
sed -n '290,315p' relaykit/dto/openai_request.go
echo "== Go JSON null behavior probe (read-only temp file only) =="
python3 - <<'PY'
import json, tempfile, pathlib
import subprocess
code = '''package main
import (
"encoding/json"
"fmt"
)
type Message struct {
Role string `"json:"role"`
Content any `"json:"content"`
ReasoningContent *string `"json:"reasoning_content,omitempty"`
}
func main() {
pending := "think"
msg := Message{Role: "assistant", ReasoningContent: &pending}
b, _ := json.Marshal(msg)
fmt.Printf("%s\n", b)
}
'''
with tempfile.NamedTemporaryFile(suffix='.go', delete=False) as f:
f.write(code.encode())
path=f.name
try:
cmd = subprocess.run(["go", "run", path], text=True, capture_output=True, timeout=15)
print("stdout:", cmd.stdout)
print("stderr:", cmd.stderr)
print("rc:", cmd.returncode)
print("parsed:", json.loads(cmd.stdout.strip()))
finally:
pathlib.Path(path).unlink(missing_ok=True)
PYRepository: QuantumNous/new-api
Length of output: 7694
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== helper invocations around responsesRequestMessagesToChat =="
sed -n '128,242p' relaykit/relayconvert/internal/oai_responses/to_oai_chat_req.go
echo "== corrected Go null-content probe =="
tmp="$(mktemp -d)"
cat > "$tmp/probe.go" <<'GO'
package main
import (
"encoding/json"
"fmt"
)
type Message struct {
Role string `json:"role"`
Content any `json:"content"`
ReasoningContent *string `json:"reasoning_content,omitempty"`
}
func main() {
pending := "think"
msg := Message{Role: "assistant", ReasoningContent: &pending}
b, _ := json.Marshal(msg)
var v map[string]any
_ = json.Unmarshal(b, &v)
fmt.Printf("%s\n", b)
fmt.Printf("content=%v contentIsNull=%t\n", v["content"], v["content"] == nil)
}
GO
go run "$tmp/probe.go"
rm -rf "$tmp"Repository: QuantumNous/new-api
Length of output: 4297
Set an explicit content value on the synthesized assistant message.
When pending reasoning remains after an array input item and no assistant message is present, attachPendingReasoningToLastAssistant appends dto.Message{Role: "assistant", ReasoningContent: &reasoning}. dto.Message.Content has no omitempty, so that marshals as "content":null. Set Content: "" on this synthesized message so it does not include null assistant content on the Chat Completions request.
🤖 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 `@relaykit/relayconvert/internal/oai_responses/to_oai_chat_req.go` around lines
268 - 279, Update attachPendingReasoningToLastAssistant so the synthesized
assistant dto.Message explicitly sets Content to an empty string alongside Role
and ReasoningContent. Leave the existing behavior unchanged for pending
reasoning and existing assistant messages.
| "Reason": "Raison", | ||
| "Reason:": "Raison :", | ||
| "Reasoning": "Raisonnement", | ||
| "Reasoning Content Models": "Modèles avec Reasoning Content", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Translate the feature label into French.
"Modèles avec Reasoning Content" leaves the user-facing feature name in English. Use the established French terminology from the surrounding entries.
Proposed fix
- "Reasoning Content Models": "Modèles avec Reasoning Content",
+ "Reasoning Content Models": "Modèles avec contenu de raisonnement",📝 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.
| "Reasoning Content Models": "Modèles avec Reasoning Content", | |
| "Reasoning Content Models": "Modèles avec contenu de raisonnement", |
🤖 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 `@web/src/i18n/locales/fr.json` at line 3661, Update the French translation
value for the "Reasoning Content Models" key in the fr.json file to fully
translate "Reasoning Content" into French instead of leaving it in English.
Replace "Reasoning Content" with the established French terminology used in the
surrounding translation entries to create a complete French-language
translation.
|
感谢你按模型名白名单实现 reasoning 保留方案。 在合并前,希望能准确保留 #6396 / #6395 中 @AmPlace 的原始贡献记录:Responses→Chat 的问题定位、最小复现、reasoning 缓冲与 tool call 关联实现,以及最初的回归测试,均首先在 #6396 / #6395 中提交。#6593 后续也明确说明其该方向的实现思路与测试来源于 #6395。 如果最终方案会重写或整合现有实现,烦请在 PR 描述和最终合并记录中明确引用 #6395 / @AmPlace,并保留相关提交,或添加适当的 另外,当前实现中,模型匹配未启用时会静默跳过不支持的 reasoning item,而不是返回明确的 unsupported conversion error。请确认这一行为是有意设计的。 |
上游 253a74d(QuantumNous#6654) 与 7d09c69(QuantumNous#6861) 新增的测试用例调用 ResponsesRequestToChatCompletionsRequest / ChatCompletionsRequestToResponsesRequest 时使用的是旧签名,而本地 PR QuantumNous#6655 补丁已为这两个函数引入 convmeta.Meta 首参(reasoning_content 跨转换保留的核心机制)。 合并后函数定义取本地版本、测试取上游版本,导致 relaykit 模块 vet 失败。为 5 处新增用例补上 nil meta。 受影响用例: - to_oai_chat_req_test.go: penalty 转换 2 处 - to_oai_responses_req_test.go: prompt_cache_key 2 处 / penalty 1 处 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014AZbTUCaSkfaTLkrfzKGSP
Important
📝 变更描述 / Description
(简述:做了什么?为什么这样改能生效?请基于你对代码逻辑的理解来写,避免粘贴未经整理的内容)
针对部分模型启用reasoning_content的转换逻辑
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告,以证明变更生效)
Summary by CodeRabbit
New Features
Bug Fixes