feat(channel): OpenAI 兼容渠道支持双向推理字段改名开关 - #626
Conversation
📝 WalkthroughWalkthrough本次变更为 OpenAI 兼容渠道增加 Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🟡 Moderate · up to Existing configurations can fail validation after upgrade, and enabled request or response rewriting can alter malformed payloads by dropping trailing data. These should be corrected before merge; the remaining settings and documentation issues can also mislead users. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation PR 已实现 Full details: Docstring CoverageExplanation Docstring coverage is 40.74% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 54 functions across 21 files. (4 skipped: 4 unsupported.) 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.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
internal/execution/bifrost/executor.go-629-631 (1)
629-631: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win在 WebUI 中说明
request_reasoning_alias的适用范围。ProviderOpenAICompatible只有在base_url以/v1结尾时才进入包含RewriteRequestMessages的 passthrough 分支。其他配置会进入 typed 路径,该路径将两种字段统一到内部Reasoning,再固定输出为reasoning_content。因此,reasoning_content → reasoning在该路径上不会生效;reasoning → reasoning_content可能由 typed 转换自然实现。请在 WebUI 中说明该方向和base_url限制,或让 typed 路径执行相同的请求改名逻辑。web/src/features/groups/settings/GroupSettingsTab.vue-630-630 (1)
630-630: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win在不支持推理字段时将深链接重定向到
general。当
selectedChannel已解析且没有reasoning_content_alias或request_reasoning_alias时,GroupSettingsTab.vue会同时移除settings-reasoning导航项和对应的GroupSettingsBaseForm。group-route.ts仍会接受section=reasoning,因此该 URL 没有匹配内容;GroupSettingsBaseForm中的group.settings.reasoning.notSupported也无法渲染。请在确认渠道不支持后使用router.replace将section改为general。
🧹 Nitpick comments (1)
internal/execution/bifrost/model_alias.go (1)
152-181: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win缓存请求和响应的 alias mode
当前 OpenAI passthrough 请求在 alias 全部关闭时执行 2 次
TargetConfig的json.Unmarshal。在请求和响应 alias 都启用时执行 5 次,其中请求路径执行 3 次,响应路径执行 2 次。因此“功能关闭时每个请求多出 3 次解析”不准确,但启用 alias 时确实存在重复的完整配置解析。请让
responseReasoningAliasMode和requestReasoningAliasMode各自只解析一次,并让needsResponseReasoningAlias、needsRequestReasoningAlias复用对应的 mode。executor.go应缓存一次requestReasoningAliasMode的结果,再将该结果传给RewriteRequestMessages,避免先调用needsRequestReasoningAlias、再重复计算 mode。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Advanced
Run ID: 348b37de-2139-444e-8b6e-abb5118d8928
📒 Files selected for processing (27)
README.mdREADME_CN.mdREADME_JP.mdinternal/channel/channel.gointernal/channel/compiler.gointernal/channel/modules/openai_compatible.gointernal/channel/reasoning_alias_params_test.gointernal/channel/registry_test.gointernal/channel/spec/definition.gointernal/channel/spec/normalize.gointernal/execution/bifrost/executor.gointernal/execution/bifrost/model_alias.gointernal/execution/bifrost/passthrough.gointernal/execution/bifrost/reasoning_alias_modes_test.gointernal/execution/responsealias/reasoning_alias.gointernal/execution/responsealias/reasoning_alias_test.gointernal/execution/responsealias/request_alias.gointernal/execution/responsealias/request_alias_test.gointernal/execution/responsealias/response_alias.goweb/src/app/resources/channels.tsweb/src/features/groups/group-route.tsweb/src/features/groups/settings/GroupSettingsBaseForm.vueweb/src/features/groups/settings/GroupSettingsTab.vueweb/src/features/import/ImportConnectionSection.vueweb/src/i18n/locales/en-US/group.tsweb/src/i18n/locales/ja-JP/group.tsweb/src/i18n/locales/zh-CN/group.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
该问题属实,覆盖范围说明如下。 请求方向的重命名目前仅在直通模式下生效。原因是非直通模式的请求体由外部依赖 Bifrost 组装并发送,gpt-load 无法介入其序列化过程。 Bifrost 自身的处理方式覆盖了其中大部分场景:它解析客户端回传的历史消息时同时接受
未覆盖的仅剩一种组合:上游仅接受 响应方向不存在此问题:两种模式下客户端均可正常获取推理内容,非直通模式下 Bifrost 会在响应中同时输出两种拼写。 如需完全覆盖该组合,需要使 Bifrost 出站 wire 类型的拼写可配置,属于上游依赖改动,不在本 PR 范围内。 |
e1aa6ab to
6beb6a0
Compare
已按建议在 WebUI 中补充适用范围说明(请求方向 help 文案),见 commit 6beb6a0: |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/features/groups/settings/GroupSettingsTab.vue (1)
630-630: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win当动态导航不包含请求的
section时回退当 channel 没有
reasoning_content_alias或request_reasoning_alias时,navItems不包含settings-reasoning,且 reasoning form 不渲染。路由仍接受section=reasoning。GroupSettingsTab将该值传给useSectionNavigation,而selectSection会保留不存在的 ID,并跳过对缺失 DOM 元素的滚动。因此深链接无法滚动到可用的设置 section,桌面导航也没有 active 项。请在GroupSettingsTab的动态导航边界统一归一化请求值,并在initialId和路由 watcher 中复用该结果;当请求值不在navItems中时,使用navItems[0].id。
🟡 Other comments (1)
web/src/i18n/locales/en-US/group.ts-221-222 (1)
221-222: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win统一三种语言中请求方向的作用范围说明。
三种语言的
reasoning区域文案都把请求方向概括为出站请求,但本 PR 只在直通模式改写出站历史消息。请在以下位置加入“直通模式”和“历史消息”的限制:
web/src/i18n/locales/en-US/group.ts#L221-L222: 明确 request alias 只改写直通模式发送的历史消息。web/src/i18n/locales/ja-JP/group.ts#L221-L222: 使用对应日文文案明确直通模式和历史消息范围。web/src/i18n/locales/zh-CN/group.ts#L218-L219: 使用对应中文文案明确直通模式和历史消息范围。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Advanced
Run ID: e8eae1da-6e4c-481c-8abf-3057979f8605
📒 Files selected for processing (3)
web/src/i18n/locales/en-US/group.tsweb/src/i18n/locales/ja-JP/group.tsweb/src/i18n/locales/zh-CN/group.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
|
提交后我们复盘了方案的覆盖面,发现响应方向的设计有一个问题,计划在本 PR 中修正,先在此说明。 问题:响应方向不应该强迫管理员二选一。 当前的响应开关要求管理员在 对策:响应方向改为"关闭 / 双写"两态。 开启后响应中同时输出 请求方向则还存在一个未解决的问题。 客户端这一侧双写可以覆盖所有情况,上游这一侧更复杂一些:如果上游各模型提供统一的输入格式,端点级改名够用;但如果上游本身是另一个中转站,很可能每个模型需要的推理字段都不一样——模型 A 只接受 |
- 响应方向 reasoning_content_alias 两态:off / duplicate,开启后在 choices[].message/delta 同时输出 reasoning 与 reasoning_content(仅填充缺失、null 或空串目标),非流式 JSON 与流式 SSE 均生效 - 请求方向 request_reasoning_alias 三态:off / reasoning_to_content / reasoning_content_to_reasoning,按移动语义改写出站历史消息 - 参数校验只接受规范值,关闭时零开销 - WebUI「推理字段」章节(响应方向两态、请求方向三态,中/英/日三语),三份 README 同步
6beb6a0 to
fe5eb0d
Compare
|
按上一条评论的方案完成调整并推送。PR 已整理为单一提交,描述同步更新为最终设计:
|
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
web/src/i18n/locales/en-US/group.ts-220-223 (1)
220-223: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win将
/v1限制仅说明为请求方向的限制。当前文案将该限制同时应用到响应方向。响应别名处理仍适用于 OpenAI Chat Completions 的 JSON 和 SSE 响应。用户会因此误以为非直通渠道不能使用响应双写,并无法配置此功能。
web/src/i18n/locales/en-US/group.ts#L220-L223:将 “only takes effect when base_url ends with /v1” 限定为请求方向。web/src/i18n/locales/ja-JP/group.ts#L220-L223:将/v1条件限定为请求方向。web/src/i18n/locales/zh-CN/group.ts#L217-L220:明确只有请求方向仅在base_url以/v1结尾时生效。README.md-61-61 (1)
61-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win统一修正三个 README 的推理字段兼容说明。
reasoning_content_alias=duplicate才会启用响应双写。request_reasoning_alias设为reasoning_to_content或reasoning_content_to_reasoning才会启用请求改名。- 请求改名只处理聊天请求顶层
messages数组中的消息对象,不处理其他出站字段。- 两项能力仅适用于 OpenAI Chat Completions;请求改名还仅在原生
/v1直通路径生效。请在
README.md、README_CN.md和README_JP.md的对应条目中补充这些配置和范围限制。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: QUIET
Plan: Advanced
Run ID: 3abb8d2c-ac1b-4052-8207-a36ab5288829
📒 Files selected for processing (20)
README.mdREADME_CN.mdREADME_JP.mdinternal/channel/modules/openai_compatible.gointernal/channel/reasoning_alias_params_test.gointernal/channel/registry_test.gointernal/channel/spec/definition.gointernal/channel/spec/normalize.gointernal/execution/bifrost/executor.gointernal/execution/bifrost/model_alias.gointernal/execution/bifrost/reasoning_alias_modes_test.gointernal/execution/responsealias/reasoning_alias.gointernal/execution/responsealias/reasoning_alias_test.gointernal/execution/responsealias/request_alias.gointernal/execution/responsealias/request_alias_test.gointernal/execution/responsealias/response_alias.goweb/src/features/groups/settings/GroupSettingsBaseForm.vueweb/src/i18n/locales/en-US/group.tsweb/src/i18n/locales/ja-JP/group.tsweb/src/i18n/locales/zh-CN/group.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| func normalizeReasoningAliasOption(value string, options []string) (string, error) { | ||
| normalized := strings.ToLower(strings.TrimSpace(value)) | ||
| if normalized == "" { | ||
| return "", nil | ||
| } | ||
| for _, option := range options { | ||
| if normalized == option { | ||
| return option, nil | ||
| } | ||
| } | ||
| return "", fmt.Errorf("must be one of %s", strings.Join(options, ", ")) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 检查历史 reasoning_content_alias 布尔值是否在参数验证前转换。
rg -n -C 5 \
'reasoning_content_alias|NormalizeResponseReasoningAlias|ValidateParams|TargetConfig|ReasoningAliasDuplicate' \
internal/channel/compiler.go internal/channel/channel.go internal/execution/bifrost/model_alias.go
# 检查是否已有旧布尔值配置的迁移或读取回归测试。
rg -n -C 4 \
'"reasoning_content_alias":(true|false)|reasoning_content_alias.*true|reasoning_content_alias.*false' \
internal --glob '*_test.go'Repository: tbphp/gpt-load
Length of output: 14890
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ValidateParams and parameter decoding ---'
sed -n '470,545p' internal/channel/channel.go
printf '%s\n' '--- parameter schema and normalization symbols ---'
rg -n -C 4 'type Params|func \(.*\) Value|CanonicalJSON|normalizeReasoningAliasOption|reasoning_content_alias|ResolveExecutionTarget|\\.Resolve\\(' internal/channel internal/execution --glob '*.go'
printf '%s\n' '--- persisted group-parameter read paths ---'
rg -n -C 5 'TargetConfig|target_config|group.*param|params.*group|ResolveExecutionTarget|Resolve\\(' internal --glob '*.go' --glob '*_test.go'
printf '%s\n' '--- relevant tests ---'
sed -n '1,130p' internal/channel/reasoning_alias_params_test.goRepository: tbphp/gpt-load
Length of output: 2918
🤖 get_repo_knowledge executed:
get_repo_knowledge tbphp/gpt-load /tmp/coderabbit-repo-knowledge/tbphp-gpt-load-62eb2efe/learnings
Length of output: 727
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- parameter types and validators ---'
rg -n -C 5 'type Params|func .*Value|CanonicalJSON|normalizeReasoningAliasOption|reasoning_content_alias' internal/channel internal/execution --glob '*.go'
printf '%s\n' '--- target resolution callers and persisted configuration paths ---'
rg -n -C 5 'ResolveExecutionTarget|Resolve\(|TargetConfig|target_config|group_params|GroupParams|group.*params|params.*group' internal --glob '*.go' --glob '*_test.go'
printf '%s\n' '--- channel tests around historical values ---'
sed -n '1,130p' internal/channel/reasoning_alias_params_test.goRepository: tbphp/gpt-load
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact validator definitions ---'
rg -l 'normalizeReasoningAliasOption|type Params|func .*validate' internal/channel
for f in $(rg -l 'normalizeReasoningAliasOption|type Params|func .*validate' internal/channel); do
printf '\n--- %s ---\n' "$f"
rg -n -C 8 'normalizeReasoningAliasOption|type Params|func .*validate|reasoning_content_alias|request_reasoning_alias' "$f"
done
printf '%s\n' '--- state loading and snapshot parameter flow ---'
rg -n -C 6 'ChannelRegistry.ValidateParams|group.Params|Params:|params:' internal/state/loader internal/state --glob '*.go' --glob '*_test.go' | head -n 320
printf '%s\n' '--- group model and loader conversion ---'
rg -n -C 8 'type Group|Params .*JSON|GroupConfig|group.*Params|Params.*group' internal/models internal/state/loader internal/state --glob '*.go' --glob '*_test.go' | head -n 360Repository: tbphp/gpt-load
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- reasoning alias binding ---'
rg -l 'NormalizeResponseReasoningAlias|NormalizeReasoningAlias|reasoning_content_alias' internal/channel --glob '*.go'
rg -n -C 12 'NormalizeResponseReasoningAlias|NormalizeReasoningAlias|reasoning_content_alias|request_reasoning_alias' internal/channel/modules internal/channel --glob '*.go' | head -n 260
printf '%s\n' '--- strict object decoding and field normalization ---'
sed -n '600,690p' internal/channel/channel.go
rg -n -C 8 'func decodeStrictObject|type fieldSpec|Normalizer|normalize' internal/channel --glob '*.go'
printf '%s\n' '--- loader pass-through and compile validation ---'
sed -n '588,670p' internal/state/loader/loader.go
sed -n '248,275p' internal/state/snapshot.goRepository: tbphp/gpt-load
Length of output: 50371
在加载持久化组参数时迁移历史布尔值。
如果启用的历史组参数包含 reasoning_content_alias: true 或 false,internal/state/loader/loader.go 会将 row.Params 原样传入 GroupConfig.Params。随后 internal/state/snapshot.go 调用 Registry.ValidateParams,该验证器要求每个字段值为 JSON 字符串,因此会在执行 normalizeReasoningAliasOption 前返回 must be a string。请在验证前按既定映射转换历史布尔值,并添加加载路径的回归测试。
| decoder := json.NewDecoder(bytes.NewReader(payload)) | ||
| decoder.UseNumber() | ||
| var doc map[string]any | ||
| if err := decoder.Decode(&doc); err != nil || doc == nil { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge tbphp/gpt-load /tmp/coderabbit-repo-knowledge/tbphp-gpt-load-62eb2efe
Length of output: 418
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- reasoning_alias.go ---'
cat -n internal/execution/responsealias/reasoning_alias.go
printf '%s\n' '--- request_alias.go ---'
cat -n internal/execution/responsealias/request_alias.go
printf '%s\n' '--- responsealias files ---'
git ls-files 'internal/execution/responsealias/*'
printf '%s\n' '--- relevant contracts and callers ---'
rg -n --glob '*.go' 'responsealias|ReasoningAlias|RequestAlias|Decode\(&doc\)|json\.NewDecoder' internalRepository: tbphp/gpt-load
Length of output: 28936
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- responsealias tests: trailing and invalid-input behavior ---'
rg -n -C 5 'trailing|invalid|malformed|byte-identical|unchanged|junk|RewriteRequestMessages|normalizeOpenAIReasoning' \
internal/execution/responsealias/*_test.go
printf '%s\n' '--- response_alias.go ---'
cat -n internal/execution/responsealias/response_alias.go
printf '%s\n' '--- call-site slices ---'
sed -n '600,645p' internal/execution/bifrost/executor.go
sed -n '180,215p' internal/execution/bifrost/model_alias.go
sed -n '1,90p' internal/execution/responsealias/request_alias_test.go
sed -n '1,100p' internal/execution/responsealias/reasoning_alias_test.goRepository: tbphp/gpt-load
Length of output: 26871
🌐 Web query:
Go encoding/json Decoder.Decode documentation trailing JSON values
💡 Result:
In Go's encoding/json package, the Decoder.Decode method is designed to read only the next single JSON-encoded value from the input stream [1][2]. It does not consume or validate any data that may exist in the stream after that value [3][4]. If your input contains trailing JSON values or other data, the Decoder will silently ignore them [3]. Because Decode only processes the first complete JSON object it encounters, any subsequent data remains in the reader or the decoder's internal buffer [1][2]. To detect or handle trailing data, you can use the following approach: 1. After calling Decode, use the Decoder.Token method to check for additional input [3]. 2. If Token returns anything other than io.EOF (after skipping any whitespace), it indicates that there is extra, unparsed data in the stream [3]. Example implementation: decoder:= json.NewDecoder(r) if err:= decoder.Decode(&v); err!= nil { // Handle error } // Check for trailing data if _, err:= decoder.Token; err!= io.EOF { // Found trailing data in the stream } Note that Decoder.Buffered can be used to inspect any data that has already been read from the underlying io.Reader into the decoder's internal buffer but has not yet been processed by Decode [1][2]. However, Token is generally the preferred, more idiomatic way to verify the end of the expected input [3].
Citations:
- 1: https://pkg.go.dev/encoding/json
- 2: https://go.dev/src/encoding/json/stream.go?m=text
- 3: GitHub issue 36225 in golang/go (link omitted to avoid creating a cross-reference)
- 4: https://groups.google.com/g/golang-nuts/c/cclzwOWdXPQ
确保输入只包含一个 JSON 值。
json.Decoder.Decode 只读取下一个 JSON 值。对于带有尾随数据的输入,第一次 Decode 仍可能成功。若消息或推理字段发生改写,json.Marshal 会丢弃尾随数据。
请在 internal/execution/responsealias/reasoning_alias.go 和 internal/execution/responsealias/request_alias.go 中,在第一次 Decode 后执行第二次 Decode。只有第二次 Decode 返回 io.EOF 时,才允许改写;否则返回原始字节。
📍 Affects 2 files
internal/execution/responsealias/reasoning_alias.go#L28-L28(this comment)internal/execution/responsealias/request_alias.go#L32-L32
关联 Issue / Related Issue
Closes #627
变更内容 / Change Content
不同上游对"思考内容"字段的拼写不一致:vLLM 等只返回
reasoning,SGLang 等只返回reasoning_content;而客户端/调用方往往只识别其中一种拼写,导致思考内容在链路中被静默丢弃。背景与调查详见 #627。本 PR 为 OpenAI Compatible 渠道新增两个推理字段开关,在网关侧统一上下游的拼写:
reasoning_content_alias):off/duplicate两态。开启后在choices[].message/delta中同时输出reasoning与reasoning_content(值相同),各类客户端各取所需。request_reasoning_alias):off/reasoning → reasoning_content/reasoning_content → reasoning三态,改写出站历史消息中的推理字段拼写,供只识别另一种拼写的上游回放思考内容。实现要点:
null或空字符串的目标拼写,两个非空拼写并存时保持原样;请求方向改名采用移动语义:源字段值写入目标字段(目标已有非空值时保留目标值),源字段始终移除,与开关字面意义一致。reasoning_content_alias/request_reasoning_alias),默认关闭;关闭时零开销——按字段标记做快速路径,未命中直接原样穿透。Content-Encoding或非法 JSON 的响应不做改动,保证字节级安全。性能(Apple M4,
go test -bench):兼容性说明
WebUI 截图
自查清单 / Checklist
make check,或在说明中写明无法运行的原因和未验证范围。