feat: add provider response model mapping#550
Conversation
What: - add provider-level responseModelMapping config with wildcard matching - rewrite JSON and SSE response model fields after protocol conversion - map recorded responseModel values for normal and provider-scoped proxy paths - preserve existing provider config fields during custom provider edits Why: - let operators expose stable client-facing model names when upstream providers return different model identifiers - keep default behavior unchanged when responseModelMapping is empty - avoid destructive config loss during provider updates Tests: - go test ./... - cd web && pnpm typecheck
What: - add shared response-model mapping setup for provider proxy e2e tests - cover generic backend proxy response model rewriting - cover provider-scoped SSE response model rewriting and request recording Why: - prove provider response model aliases work through both backend proxy paths - guard streaming responses from leaking upstream-visible model names Tests: - go test -count=1 ./internal/executor ./tests/e2e -run 'Test.*Provider|Test.*ModelMapping|Test.*ResponseModelMapping|TestGenericProxyResponseModelMappingE2E' - go test -count=1 ./tests/e2e - go test -count=1 ./... - pnpm -C web typecheck - git diff --check
📝 Walkthrough总体说明Walkthrough此 PR 引入了响应模型映射功能,允许为 Claude provider 配置将API响应中的模型名称重新映射。在 Go 后端中实现了一个响应写入器来拦截和转换模型字段(包括流式响应),同时在 TypeScript 前端添加了配置UI。 Changes响应模型映射功能实现
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as Provider<br/>Handler
participant RespWriter as Response<br/>Mapper Writer
participant Adapter as Provider<br/>Adapter
participant Backend as Provider<br/>Backend API
Client->>Handler: HTTP Request<br/>(stream: true/false)
activate Handler
Handler->>RespWriter: Create Mapper Writer<br/>(provider, clientType, isStream)
activate RespWriter
Handler->>Adapter: Execute with<br/>mapped writer
activate Adapter
Adapter->>Backend: Forward Request
Backend->>Adapter: Response<br/>(model: "claude-3-opus")
Adapter->>RespWriter: Write Response<br/>(buffer data)
RespWriter->>RespWriter: Buffer Response
Adapter->>Handler: Execution Complete
deactivate Adapter
Handler->>RespWriter: Finalize()
RespWriter->>RespWriter: Rewrite Models<br/>(opus → gpt-4-turbo)
RespWriter->>RespWriter: Adjust Content-Length
RespWriter->>Client: Send Rewritten<br/>Response
deactivate RespWriter
deactivate Handler
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 (2)
internal/executor/response_model_mapping_test.go (1)
81-98: ⚡ Quick win建议补充跨 Write 边界与 HTML 字符的覆盖
当前 SSE 测试一次性写入完整 chunk,未覆盖
Write在行中间被截断的场景(实际生产中 chunked 传输非常常见)。同时 JSON 重写测试仅用纯文本"ok",不会暴露json.Marshal默认 HTML 转义带来的字节差异。建议补一个用例:chunk1 := []byte("event: message\ndata: {\"model\":\"upstream\",\"text\":\"<b>hi</b>") chunk2 := []byte("\"}\n\ndata: [DONE]\n") // 分两次 Write,验证最终 body 既包含 "alias" 又不会把 <b> 转义成 \u003c可以同时验证关闭 HTML 转义后的字节级保真(对应另一处对
rewriteResponseModelJSON的修改建议)。🤖 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 `@internal/executor/response_model_mapping_test.go` around lines 81 - 98, Add a new test variant that exercises Write across chunk boundaries and verifies HTML characters are not HTML-escaped: in the existing TestResponseModelMappingWriterRewritesSSE suite (using NewResponseModelMappingWriter) perform two Write calls with chunk1 containing the start of the SSE JSON (e.g. "event: message\ndata: {\"model\":\"upstream\",\"text\":\"<b>hi</b\") and chunk2 completing the JSON and DONE marker, then call Finalize() and assert the recorder body contains the rewritten model value "alias" (so the rewriteResponseModelJSON path ran) and that the literal "<b>hi</b>" appears (not escaped as \u003c), ensuring the writer and rewriteResponseModelJSON handle cross-Write boundaries and JSON marshaling without HTML escaping.internal/executor/response_model_mapping.go (1)
208-228: ⚡ Quick win改用
json.Encoder并关闭 HTML 转义,以保持响应字节完整性Go 标准库
json.Marshal默认将</>/&转为\u003c/\u003e/\u0026,同时map[string]any的键会按字母序重排。当 LLM 响应里content[].text、推理片段、工具调用参数等包含代码片段或 HTML 时,只要触发了model改写就会顺带改变其他字段的字节表示。语义上仍是等价的 JSON,但对依赖原始字节(SSE 比对、缓存哈希、日志比对、严格客户端)的下游会造成不必要的差异。建议显式关闭 HTML 转义:
♻️ 建议改写
- mapped, err := json.Marshal(payload) - if err != nil { - return body - } - return mapped + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + if err := enc.Encode(payload); err != nil { + return body + } + out := buf.Bytes() + // json.Encoder 会在末尾追加 '\n',需要去掉以保持与原 body 形态一致 + if n := len(out); n > 0 && out[n-1] == '\n' { + out = out[:n-1] + } + return out🤖 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 `@internal/executor/response_model_mapping.go` around lines 208 - 228, The function rewriteResponseModelJSON currently uses json.Marshal which escapes HTML chars and changes byte representation; replace the json.Marshal step in rewriteResponseModelJSON with a json.Encoder writing to a bytes.Buffer and call encoder.SetEscapeHTML(false) so `<`, `>` and `&` are not escaped; after encoding, return the buffer bytes (trim a trailing newline if encoder.Encode adds one) instead of the json.Marshal result. Ensure you apply this change where mapped, err := json.Marshal(payload) is used and keep the same error handling path around encoding.
🤖 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 `@internal/executor/response_model_mapping.go`:
- Around line 109-118: Finalize's non-stream path rewrites the body but doesn't
update the Content-Length header, risking mismatch; after computing mapped :=
rewriteResponseModelJSON(body, w.mapping) set the Content-Length header to the
new byte length (e.g. via w.headers.Set("Content-Length",
strconv.Itoa(len(mapped))) or the equivalent header map on
ResponseModelMappingWriter) before calling w.writeHeaderIfNeeded(), and add the
"strconv" import.
In `@internal/handler/provider_proxy.go`:
- Around line 165-169: The current logic in
extractProviderProxyResponseModel(responseCapture.Body()) fails for SSE/streamed
bodies (which are "event:/data:" text), causing the code path to fall back to
executor.MapResponseModel(mappedModel, responseModelMapping) and record
proxyReq.ResponseModel based on the requestModel rather than the actual upstream
model; update the code handling near extractProviderProxyResponseModel and the
branch that sets proxyReq.ResponseModel so that when responseCapture.Body()
appears to be SSE (fast check e.g. not starting with "{"), scan the initial
data: lines, attempt lightweight json.Unmarshal on each data line to extract
"model"/"modelVersion", and if any non-empty model is found use that value (then
map via executor.MapResponseModel if mapping exists) before falling back to the
current mapping behavior; also avoid unmarshalling the entire large body and
only parse individual data lines to minimize overhead.
---
Nitpick comments:
In `@internal/executor/response_model_mapping_test.go`:
- Around line 81-98: Add a new test variant that exercises Write across chunk
boundaries and verifies HTML characters are not HTML-escaped: in the existing
TestResponseModelMappingWriterRewritesSSE suite (using
NewResponseModelMappingWriter) perform two Write calls with chunk1 containing
the start of the SSE JSON (e.g. "event: message\ndata:
{\"model\":\"upstream\",\"text\":\"<b>hi</b\") and chunk2 completing the JSON
and DONE marker, then call Finalize() and assert the recorder body contains the
rewritten model value "alias" (so the rewriteResponseModelJSON path ran) and
that the literal "<b>hi</b>" appears (not escaped as \u003c), ensuring the
writer and rewriteResponseModelJSON handle cross-Write boundaries and JSON
marshaling without HTML escaping.
In `@internal/executor/response_model_mapping.go`:
- Around line 208-228: The function rewriteResponseModelJSON currently uses
json.Marshal which escapes HTML chars and changes byte representation; replace
the json.Marshal step in rewriteResponseModelJSON with a json.Encoder writing to
a bytes.Buffer and call encoder.SetEscapeHTML(false) so `<`, `>` and `&` are not
escaped; after encoding, return the buffer bytes (trim a trailing newline if
encoder.Encode adds one) instead of the json.Marshal result. Ensure you apply
this change where mapped, err := json.Marshal(payload) is used and keep the same
error handling path around encoding.
🪄 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: f6db7213-a398-4556-bf39-07460e0d68b2
📒 Files selected for processing (9)
internal/domain/model.gointernal/executor/executor.gointernal/executor/middleware_dispatch.gointernal/executor/response_model_mapping.gointernal/executor/response_model_mapping_test.gointernal/handler/provider_proxy.gotests/e2e/provider_proxy_test.goweb/src/lib/transport/types.tsweb/src/pages/providers/components/provider-edit-flow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: playwright
- GitHub Check: Backend Checks
- GitHub Check: e2e
🔇 Additional comments (12)
web/src/lib/transport/types.ts (1)
128-128: 类型新增与后端对齐良好可选字段
responseModelMapping?: Record<string, string>与 Go 端json:"responseModelMapping,omitempty"一致,无序列化偏差。web/src/pages/providers/components/provider-edit-flow.tsx (1)
391-410: 保留现有 provider.config 顶层字段的实现合理
...provider.config展开使responseModelMapping、disableErrorCooldown之外的未来顶层字段在保存与克隆路径中也能被保留;provider.config为null时展开为{}不会出错。注意custom仍是整体替换,如后续在ProviderConfigCustom内新增字段(类似新增responseModelMapping那样)需在custom块内显式回填,否则会被覆盖丢失。Also applies to: 444-463
internal/domain/model.go (1)
268-273: 字段定义清晰且向后兼容新字段
ResponseModelMapping为可选(omitempty),空值时executor.ResponseModelMappingForProvider返回 nil,旧 provider 不受影响。文档对通配符语义说明明确。internal/executor/executor.go (1)
308-308: 事件处理路径正确传递并应用 responseModelMapping两个事件循环都在
EventResponseModel触发时通过MapResponseModel转换,空 mapping 时为无副作用直通,行为符合预期。Also applies to: 344-344, 366-366, 426-426
internal/handler/provider_proxy.go (1)
142-158: Mapping writer 包装与 finalize 流程整合得当按需包装(无 mapping 不走包装)并在 adapter 执行后立即
Finalize,对错误路径(writeStreamError/writeProxyError)直接走responseCapture而绕过 mapping writer 也合理——错误响应里包含 mapping 后的模型名意义不大,且能避免在头已写出后再Finalize。tests/e2e/provider_proxy_test.go (2)
28-38: 💤 Low valueE2E 覆盖了流式 / 非流式 / 通用代理三条路径,粒度合适
Mock 在一次 handler 调用里把所有事件一次性写出,虽然能验证 mapping 端到端落库正确,但写入路径上的"跨 Write 行边界"在 E2E 里没有被压到(单元测试也是单次 Write)。如担心 chunked 上游下的
lineBuffer行为,可考虑在 mock 里在两条事件之间显式flusher.Flush()并time.Sleep,或拆成多次fmt.Fprint(当前已经多条 Fprint,但因 httptest 默认会合并到响应缓冲,实际上对客户端是同一批字节)。可作为后续可选增强。Also applies to: 244-296
62-95: Helper 设计良好,后续映射场景测试可直接复用
createMappedClaudeProvider集中了 provider + route 创建,新增映射场景测试只需指定不同 mapping 模式即可扩展。internal/executor/response_model_mapping.go (1)
230-246: 💤 Low value仅顶层
model/modelVersion未覆盖 SSE 嵌套结构中的模型字段,请确认设计意图并补充文档说明代码当前实现只改写根层的
model和modelVersion字段,但不处理嵌套结构。Anthropic 流式响应中message_start事件包含{"type":"message_start","message":{"model":"..."}}的嵌套模型字段,该实现不会对其改写。这可能导致客户端同时看到映射后的顶层模型名和上游原始模型名(在嵌套位置),产生不一致。请确认:
- 这是否为设计意图(与 PR 仅改写顶层的描述一致)
- 若确实需要支持嵌套模型改写,需递归遍历响应结构
- 无论哪种情况,建议在函数注释中明确标注作用范围限制,便于运维理解
internal/executor/middleware_dispatch.go (4)
67-67: 映射上下文传递完整且时机正确。路由粒度计算
responseModelMapping并透传到实时事件处理,能确保事件侧模型记录与响应重写使用同一映射源。Also applies to: 157-157
161-167: 响应写入链路组装合理,转换/非转换路径都被覆盖。
finalWriter先承接映射 writer,再按需外包转换 writer,这个装配顺序清晰且可维护。Also applies to: 170-174
186-190: Finalize 分支控制稳健。流式场景总是 finalize,非流式仅在存在 pending 响应时 finalize,避免了无效收尾调用。
239-242: 响应模型回填逻辑更健壮。优先使用
attemptRecord.ResponseModel,为空再回退mappedModel,与当前计费模型选择策略保持一致。
9850255 to
9b990a3
Compare
89e8bb9 to
7fb71cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/executor/middleware_dispatch.go (1)
239-239:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
proxyReq.ResponseModel应从响应体中提取,而非仅使用请求侧映射结果。middleware_dispatch.go 第 239 行的处理确实不一致。与 provider_proxy.go 相比,此处缺少从响应体提取实际响应模型的步骤。provider_proxy.go 采用的模式是:
- 先设置请求侧映射的模型
- 再从
responseCapture.Body()提取实际响应模型并覆盖middleware_dispatch.go 目前只执行第 1 步,导致
proxyReq.ResponseModel未能反映上游实际响应的内容。建议修改方式需要调整:应参考 provider_proxy.go 直接使用提取的响应模型(不再应用 MapResponseModel),因为 ResponseModelMappingWriter 已在响应流阶段处理了客户端看到的映射:
- proxyReq.ResponseModel = mappedModel + proxyReq.ResponseModel = mappedModel + if respModel := ExtractResponseModel(responseCapture.Body(), matchedRoute.Provider, originalClientType); respModel != "" { + proxyReq.ResponseModel = respModel + }注:原建议中对提取的模型应用 MapResponseModel 可能导致双重映射,应移除。
🤖 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 `@internal/executor/middleware_dispatch.go` at line 239, The code sets proxyReq.ResponseModel = mappedModel but must instead extract the actual upstream response model from the captured response body and assign that to proxyReq.ResponseModel; update the logic in middleware_dispatch.go to mirror provider_proxy.go by calling responseCapture.Body() (or the existing response capture accessor) to deserialize the real response model and assign it to proxyReq.ResponseModel, and do not re-apply MapResponseModel because ResponseModelMappingWriter already transformed the client-visible response; reference proxyReq.ResponseModel, responseCapture.Body(), ResponseModelMappingWriter and remove any extra MapResponseModel application.
🧹 Nitpick comments (8)
internal/executor/response_capture.go (1)
26-29: 💤 Low valueLGTM —— 简单的下游 writer 替换。
实现仅替换
ResponseWriter,保留已捕获的 status/body/headers,符合 provider_proxy.go 错误路径需要"切换底层 writer 同时保留已捕获状态"的语义。需要注意:替换后body缓冲区的旧数据不会同步写入新 writer(这是预期行为,调用方需自行处理写入路径)。建议在 godoc 中明确这一点:📝 建议补充注释
-// SetResponseWriter changes the downstream writer while preserving captured data. +// SetResponseWriter changes the downstream writer while preserving captured data. +// Note: previously buffered body in this ResponseCapture is NOT replayed to the new +// writer. Callers are expected to use this only to swap out a transient middleware +// writer (e.g., on error) and to drive any subsequent writes themselves. func (rc *ResponseCapture) SetResponseWriter(w http.ResponseWriter) { rc.ResponseWriter = w }🤖 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 `@internal/executor/response_capture.go` around lines 26 - 29, 为明确行为并指导调用方,在 ResponseCapture.SetResponseWriter 的 godoc 中补充说明:该方法仅替换结构体内的 ResponseWriter 字段(函数名: ResponseCapture.SetResponseWriter),会保留已捕获的 status、headers 和 body 缓冲区,但不会把已缓冲的 body 自动写入新的 downstream writer,调用方若需要将已捕获数据转发到新 writer 必须显式写入;同时说明这是在 provider_proxy.go 错误路径下用于“切换底层 writer 同时保留已捕获状态”的预期行为。internal/executor/response_model_mapping.go (3)
17-43: ⚡ Quick win
ResponseModelMappingForProvider使用 switch 互斥返回,假设 provider 仅有一个 config 块非空。按 provider.Type 与 config.Custom/Antigravity/... 的常规配对,这个假设成立。但函数本身没有依据
provider.Type校验,而是按Custom → Antigravity → Bedrock → ... → CLIProxyAPI*优先级取第一个非 nil 的子配置返回。风险点:如果上游写入逻辑出现 bug,使得某个 provider 同时设置了
Custom和Bedrock(比如 bedrock provider 因迁移残留 Custom),那么会错误地返回Custom.ResponseModelMapping而非Bedrock.ResponseModelMapping。建议改为按
provider.Type精确分发,与responseModelFieldPathsForProvider保持一致:♻️ 建议改动
func ResponseModelMappingForProvider(provider *domain.Provider) map[string]string { if provider == nil || provider.Config == nil { return nil } config := provider.Config - switch { - case config.Custom != nil: - return config.Custom.ResponseModelMapping - case config.Antigravity != nil: - return config.Antigravity.ResponseModelMapping - case config.Bedrock != nil: - return config.Bedrock.ResponseModelMapping - case config.Kiro != nil: - return config.Kiro.ResponseModelMapping - case config.Codex != nil: - return config.Codex.ResponseModelMapping - case config.Claude != nil: - return config.Claude.ResponseModelMapping - case config.CLIProxyAPIAntigravity != nil: - return config.CLIProxyAPIAntigravity.ResponseModelMapping - case config.CLIProxyAPICodex != nil: - return config.CLIProxyAPICodex.ResponseModelMapping - default: - return nil - } + switch provider.Type { + case "custom": + if config.Custom != nil { + return config.Custom.ResponseModelMapping + } + case "antigravity": + if config.Antigravity != nil { + return config.Antigravity.ResponseModelMapping + } + case "bedrock": + if config.Bedrock != nil { + return config.Bedrock.ResponseModelMapping + } + case "kiro": + if config.Kiro != nil { + return config.Kiro.ResponseModelMapping + } + case "codex": + if config.Codex != nil { + return config.Codex.ResponseModelMapping + } + case "claude": + if config.Claude != nil { + return config.Claude.ResponseModelMapping + } + case "cliproxyapi-antigravity": + if config.CLIProxyAPIAntigravity != nil { + return config.CLIProxyAPIAntigravity.ResponseModelMapping + } + case "cliproxyapi-codex": + if config.CLIProxyAPICodex != nil { + return config.CLIProxyAPICodex.ResponseModelMapping + } + } + return nil }🤖 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 `@internal/executor/response_model_mapping.go` around lines 17 - 43, ResponseModelMappingForProvider currently picks the first non-nil config field (Custom → Antigravity → ...) which can return the wrong mapping if multiple config blocks are set; change the function to dispatch based on provider.Type (the same exact type-based branching used in responseModelFieldPathsForProvider) so each provider type (e.g., "bedrock", "custom", "antigravity", "kiro", "codex", "claude", "cliProxyAPIAntigravity", "cliProxyAPICodex", etc.) returns its corresponding ResponseModelMapping; keep the nil checks for provider and provider.Config and return nil for unknown types or when the specific sub-config is nil.
252-263: ⚖️ Poor tradeoff非流式响应在内存中整体缓冲,超大响应存在堆内存放大风险。
Write在非流式分支将整个上游响应体写入w.buffer,仅在Finalize()时一次性rewriteResponseModelJSON+ 写出。对于:
- 多模态响应(含 base64 编码图片/音频)
- 工具调用返回的大段结构化内容
- 1M context 模型的长输出
可能在单个 goroutine 上瞬时分配几 MB ~ 数十 MB 堆内存,并因
bytes.Buffer增长策略产生多次 grow/copy。短期:可以接受,因为现有
ResponseCapture也在缓冲。中长期建议(仅当用户配置了 mapping 时启用流式 JSON 改写,或使用json.Decoder流式解析):// 可考虑的优化方向: // 1. 仅 mapping 命中时再做 unmarshal/marshal;当前已是这样(changed=false 直接返回 body) // 2. 用 jsonparser/gjson 等流式工具仅替换目标路径的字符串值,避免整体反序列化 // 3. 设置缓冲上限,超过阈值降级为透传 + 记录指标属于运营观察项,不阻塞合并。
Also applies to: 274-286
🤖 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 `@internal/executor/response_model_mapping.go` around lines 252 - 263, The non-stream branch in ResponseModelMappingWriter.Write buffers the entire upstream body into w.buffer (used later by Finalize -> rewriteResponseModelJSON), which can spike heap for large responses; change the logic so when w.mapping is non-empty you either (a) process the response as a stream by invoking writeStream behavior (use the existing writeStream path) or (b) perform streaming JSON processing via json.Decoder/streaming parser inside rewriteResponseModelJSON to avoid full-buffer unmarshal, and add a safety fallback: enforce a configurable buffer size threshold on w.buffer and if exceeded, switch to passthrough write to w.underlying and emit a metric/log; update the Write implementation and rewriteResponseModelJSON to reference these behaviors and ensure isStream/mapping checks trigger the streaming/fallback path.
376-402: 💤 Low valueJSON 重新编码会重新排序对象 key,输出不再保持上游字节顺序。
json.Encoder序列化map[string]any时会按字母顺序输出 map key(而数组顺序保留)。这意味着即便仅改写一个 model 字段,整个 JSON 对象的 key 顺序也会改变。通常对客户端无影响(JSON 语义上 key 无序),但若下游存在以下情况则会出问题:
- 上游签名/HMAC 校验依赖原始字节序
- 客户端按行做字符串匹配或 hash 校验
- 埋点/审计依赖字段出现顺序
建议在代码注释中明确指出这一兼容性影响,便于运维和消费端排查:
建议的代码注释
// Note: This re-encodes the JSON via encoding/json which sorts map keys // alphabetically; the output bytes will differ from upstream even when only // the `model` field is rewritten. Order-sensitive consumers (signing, // byte-exact matching) are not supported when ResponseModelMapping is active.🤖 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 `@internal/executor/response_model_mapping.go` around lines 376 - 402, Add a clear compatibility note to rewriteResponseModelJSON explaining that it re-encodes JSON via encoding/json (using json.Encoder) which sorts map keys alphabetically, so the output byte order will differ from upstream even when only a model field is rewritten; mention that order-sensitive consumers (signing/HMAC, line/string matching, audit/telemetry relying on key order) are therefore not supported when ResponseModelMapping is active and link the note to the rewriting flow that calls rewriteResponseModelValue and performs encoding.web/src/pages/providers/components/provider-edit-flow.tsx (1)
794-811: ⚡ Quick win国际化缺失:标题与描述被硬编码为英文。
文件其余部分均通过
t('...')渲染用户可见文本,新增的 Response Model Mapping 章节却直接写了英文字符串"Response Model Mapping"与"ResponseModel pattern → constant MappedModel, applied only to final API responses.",会导致中文等其他语言下显示不一致。建议为这两段文案补充翻译键并通过
t()引用。♻️ 建议改动
- <h4 className="text-lg font-semibold text-foreground">Response Model Mapping</h4> + <h4 className="text-lg font-semibold text-foreground"> + {t('responseModelMappings.title')} + </h4> </div> <div className="bg-card border border-border rounded-xl p-4"> - <p className="text-xs text-muted-foreground mb-4"> - ResponseModel pattern → constant MappedModel, applied only to final API responses. - </p> + <p className="text-xs text-muted-foreground mb-4"> + {t('responseModelMappings.pageDesc')} + </p>并在各语言资源文件中添加对应键(至少
en与zh-CN)。🤖 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/pages/providers/components/provider-edit-flow.tsx` around lines 794 - 811, The new "Response Model Mapping" block uses hardcoded English strings; replace the literal header and description with i18n keys using the t() helper (e.g., t('provider.responseModelMapping.title') and t('provider.responseModelMapping.description') ) in the provider-edit-flow component where ModelMappingEditor, formData.responseModelMapping, setFormData, saving and cloning are used so behavior remains unchanged; then add corresponding keys and translations to the locale resource files (at minimum en and zh-CN).internal/handler/provider_proxy.go (1)
183-200: 💤 Low value错误路径中
SetResponseWriter切换 underlying 仅在"adapter 出错前未写出任何响应"的情况下安全。当
responseModelWriter已经在上方 Finalize 中写入了 headers + body(非流式有缓冲)或写入了部分流式数据时,下方writeProxyError/writeStreamError通过被切换的responseCapture直写originalWriter,会产生:
- 流式:一段已写出的 SSE 后面追加一个非 SSE 的 JSON error,部分客户端解析会出错
- 非流式:成功 body 之后追加一个 error body,HTTP 协议层非法
实际触发条件较罕见(adapter 既返回 error 又成功写出完整响应),但建议在
Finalize已执行的分支跳过错误响应写入:🛡️ 建议防御
proxyReq.Status = "FAILED" proxyReq.Error = err.Error() - if responseModelWriter != nil { - responseCapture.SetResponseWriter(originalWriter) - } + alreadyCommitted := responseModelWriter != nil && (isStream || responseModelWriter.HasPendingNonStreamResponse()) + if responseModelWriter != nil { + responseCapture.SetResponseWriter(originalWriter) + } + if alreadyCommitted { + // Response already flushed to client; only persist the error metadata. + clearProxyRequestDetail(proxyReq, clearDetail) + _ = h.proxyRequestRepo.Update(proxyReq) + c.Abort() + return + } if proxyErr, ok := err.(*domain.ProxyError); ok { ...注:这是一个小概率防御性改动;如果上层 adapter 契约保证"返回 error 时不会预先写出有效响应",则可保持现状并在 ProxyError 处理路径加注释说明。
internal/executor/response_model_mapping_test.go (1)
11-183: 💤 Low valueLGTM —— 测试覆盖了主要路径。
涵盖了通配符、精确匹配、空目标 fallback、JSON 顶层与嵌套字段改写、SSE 单/跨 chunk 改写、Provider 形状感知(Claude/Codex 的
message.modelvsresponse.model)等关键场景。可以考虑(非必需)补充的边缘用例:
- 上游响应体非合法 JSON(确认
rewriteResponseModelJSON返回原 body)- SSE 使用
\r\n行尾Write与Flush交错调用的流式行为- mapping 目标字符串为
" "(仅空白)的处理🤖 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 `@internal/executor/response_model_mapping_test.go` around lines 11 - 183, Add tests for the suggested edge cases: create new unit tests in internal/executor/response_model_mapping_test.go that exercise MapResponseModel/rewriteResponseModelJSON/NewResponseModelMappingWriter for (1) invalid JSON body (ensure rewriteResponseModelJSON returns original bytes unchanged), (2) SSE lines ending with "\r\n" (ensure SSE parsing and rewrites still occur), (3) interleaved Write and Flush behavior on the ResponseModelMappingWriter (call Write, then Flush, then Write across chunk boundaries and assert correct rewrites/passthrough), and (4) mapping entries whose target is only whitespace (e.g., " ") to assert they are treated like empty targets and fall through to the fallback; reference the functions MapResponseModel, rewriteResponseModelJSON, NewResponseModelMappingWriter and responseModelFieldPath/genericResponseModelFieldPaths in the tests to locate behavior under test.tests/e2e/provider_proxy_test.go (1)
23-23: ⚡ Quick win在 mock 服务里忽略 JSON 解码错误会掩盖回归
Line 23 当前吞掉了解码错误;如果请求体结构变化,测试可能继续通过但语义已偏离。建议在解码失败时直接返回 400,让失败显式暴露。
建议修改
var body map[string]any - _ = json.NewDecoder(r.Body).Decode(&body) + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + http.Error(w, "invalid json body", http.StatusBadRequest) + return + }🤖 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 `@tests/e2e/provider_proxy_test.go` at line 23, The mock handler currently ignores the error from json.NewDecoder(r.Body).Decode(&body); change it to check the returned error and if non-nil write an HTTP 400 response (e.g., via the ResponseWriter) and return, so JSON decode failures fail the test explicitly; locate the decode call (json.NewDecoder(r.Body).Decode(&body)) in the test/mock handler and add an if err != nil { w.WriteHeader(http.StatusBadRequest); return } (or equivalent) to expose regressions.
🤖 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 `@internal/handler/provider_proxy.go`:
- Around line 165-167: Extracted response model from
executor.ExtractResponseModel(responseCapture.Body(), provider, clientType) is
the upstream original name (responseCapture is before
ResponseModelMappingWriter), so update the code that sets proxyReq.ResponseModel
to pass the extracted value through the mapping function (e.g., call
MapResponseModel or the equivalent mapper used by ResponseModelMappingWriter)
when a mapping config exists so persisted proxyReq.ResponseModel matches the
model actually sent to clients; if you intentionally want to keep the upstream
name for auditing, add a clear comment near where proxyReq.ResponseModel is
assigned (and consider storing both the upstream and mapped values separately).
---
Outside diff comments:
In `@internal/executor/middleware_dispatch.go`:
- Line 239: The code sets proxyReq.ResponseModel = mappedModel but must instead
extract the actual upstream response model from the captured response body and
assign that to proxyReq.ResponseModel; update the logic in
middleware_dispatch.go to mirror provider_proxy.go by calling
responseCapture.Body() (or the existing response capture accessor) to
deserialize the real response model and assign it to proxyReq.ResponseModel, and
do not re-apply MapResponseModel because ResponseModelMappingWriter already
transformed the client-visible response; reference proxyReq.ResponseModel,
responseCapture.Body(), ResponseModelMappingWriter and remove any extra
MapResponseModel application.
---
Nitpick comments:
In `@internal/executor/response_capture.go`:
- Around line 26-29: 为明确行为并指导调用方,在 ResponseCapture.SetResponseWriter 的 godoc
中补充说明:该方法仅替换结构体内的 ResponseWriter 字段(函数名:
ResponseCapture.SetResponseWriter),会保留已捕获的 status、headers 和 body 缓冲区,但不会把已缓冲的
body 自动写入新的 downstream writer,调用方若需要将已捕获数据转发到新 writer 必须显式写入;同时说明这是在
provider_proxy.go 错误路径下用于“切换底层 writer 同时保留已捕获状态”的预期行为。
In `@internal/executor/response_model_mapping_test.go`:
- Around line 11-183: Add tests for the suggested edge cases: create new unit
tests in internal/executor/response_model_mapping_test.go that exercise
MapResponseModel/rewriteResponseModelJSON/NewResponseModelMappingWriter for (1)
invalid JSON body (ensure rewriteResponseModelJSON returns original bytes
unchanged), (2) SSE lines ending with "\r\n" (ensure SSE parsing and rewrites
still occur), (3) interleaved Write and Flush behavior on the
ResponseModelMappingWriter (call Write, then Flush, then Write across chunk
boundaries and assert correct rewrites/passthrough), and (4) mapping entries
whose target is only whitespace (e.g., " ") to assert they are treated like
empty targets and fall through to the fallback; reference the functions
MapResponseModel, rewriteResponseModelJSON, NewResponseModelMappingWriter and
responseModelFieldPath/genericResponseModelFieldPaths in the tests to locate
behavior under test.
In `@internal/executor/response_model_mapping.go`:
- Around line 17-43: ResponseModelMappingForProvider currently picks the first
non-nil config field (Custom → Antigravity → ...) which can return the wrong
mapping if multiple config blocks are set; change the function to dispatch based
on provider.Type (the same exact type-based branching used in
responseModelFieldPathsForProvider) so each provider type (e.g., "bedrock",
"custom", "antigravity", "kiro", "codex", "claude", "cliProxyAPIAntigravity",
"cliProxyAPICodex", etc.) returns its corresponding ResponseModelMapping; keep
the nil checks for provider and provider.Config and return nil for unknown types
or when the specific sub-config is nil.
- Around line 252-263: The non-stream branch in ResponseModelMappingWriter.Write
buffers the entire upstream body into w.buffer (used later by Finalize ->
rewriteResponseModelJSON), which can spike heap for large responses; change the
logic so when w.mapping is non-empty you either (a) process the response as a
stream by invoking writeStream behavior (use the existing writeStream path) or
(b) perform streaming JSON processing via json.Decoder/streaming parser inside
rewriteResponseModelJSON to avoid full-buffer unmarshal, and add a safety
fallback: enforce a configurable buffer size threshold on w.buffer and if
exceeded, switch to passthrough write to w.underlying and emit a metric/log;
update the Write implementation and rewriteResponseModelJSON to reference these
behaviors and ensure isStream/mapping checks trigger the streaming/fallback
path.
- Around line 376-402: Add a clear compatibility note to
rewriteResponseModelJSON explaining that it re-encodes JSON via encoding/json
(using json.Encoder) which sorts map keys alphabetically, so the output byte
order will differ from upstream even when only a model field is rewritten;
mention that order-sensitive consumers (signing/HMAC, line/string matching,
audit/telemetry relying on key order) are therefore not supported when
ResponseModelMapping is active and link the note to the rewriting flow that
calls rewriteResponseModelValue and performs encoding.
In `@tests/e2e/provider_proxy_test.go`:
- Line 23: The mock handler currently ignores the error from
json.NewDecoder(r.Body).Decode(&body); change it to check the returned error and
if non-nil write an HTTP 400 response (e.g., via the ResponseWriter) and return,
so JSON decode failures fail the test explicitly; locate the decode call
(json.NewDecoder(r.Body).Decode(&body)) in the test/mock handler and add an if
err != nil { w.WriteHeader(http.StatusBadRequest); return } (or equivalent) to
expose regressions.
In `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Around line 794-811: The new "Response Model Mapping" block uses hardcoded
English strings; replace the literal header and description with i18n keys using
the t() helper (e.g., t('provider.responseModelMapping.title') and
t('provider.responseModelMapping.description') ) in the provider-edit-flow
component where ModelMappingEditor, formData.responseModelMapping, setFormData,
saving and cloning are used so behavior remains unchanged; then add
corresponding keys and translations to the locale resource files (at minimum en
and zh-CN).
🪄 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: 8b0f3ec1-069b-4913-b3a0-95a4aa51505f
📒 Files selected for processing (9)
internal/domain/model.gointernal/executor/middleware_dispatch.gointernal/executor/response_capture.gointernal/executor/response_model_mapping.gointernal/executor/response_model_mapping_test.gointernal/handler/provider_proxy.gotests/e2e/provider_proxy_test.goweb/src/lib/transport/types.tsweb/src/pages/providers/components/provider-edit-flow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: playwright
- GitHub Check: e2e
🔇 Additional comments (7)
web/src/lib/transport/types.ts (1)
45-105: LGTM —— 类型扩展与后端字段一一对应。六个 provider config 接口同步新增可选字段
responseModelMapping?: Record<string, string>,与internal/domain/model.go中各ProviderConfig*上的ResponseModelMapping map[string]string完全对应,向后兼容。internal/domain/model.go (1)
44-45: LGTM —— 八个 ProviderConfig 结构体一致地新增ResponseModelMapping字段。新增字段统一紧跟既有
ModelMapping,使用一致的omitemptyJSON 标签与中文注释(ResponseModelPattern → MappedModelConstant),不破坏现有反序列化路径。Also applies to: 119-120, 147-148, 169-170, 192-193, 233-234, 266-267, 285-286
internal/executor/middleware_dispatch.go (1)
159-190: LGTM —— writer 包裹链与 Finalize 条件判断正确。
originalWriter → responseModelWriter (映射) → responseCapture (捕获) → convertingWriter (协议转换)的包裹顺序合理:responseCapture捕获到的是协议转换后、映射前的 bytes,与现有 attempt 价格 / 事件处理逻辑保持兼容;responseModelMapping不存在时所有现有行为完全不变(finalWriter == originalWriter)。Finalize 条件
isStream || HasPendingNonStreamResponse在非流式 + 无任何写出(adapter 提早 error 返回)的场景下跳过 Finalize,避免错误地写入空 body。internal/handler/provider_proxy.go (1)
141-158: LGTM —— provider 直连路径的 writer 包裹与 Finalize 处理与通用调度路径保持一致。
responseModelMapping为空时 finalWriter 直接是 originalWriter,行为完全无变化;mapping 存在时按(originalWriter ← responseModelWriter ← responseCapture)的顺序包裹,并使用isStream || HasPendingNonStreamResponse条件触发 Finalize,逻辑与middleware_dispatch.go对齐。tests/e2e/provider_proxy_test.go (3)
28-42: SSE mock 分支设计合理,事件形态覆盖完整
message_start/message_delta/[DONE]都被覆盖,并且显式Flush,对流式映射链路的 e2e 验证很到位。
66-100: 提取 provider 创建助手函数是个好改动
createMappedClaudeProvider明显降低了测试样板代码重复,让后续新增映射场景测试更容易维护。
169-301: 新增的三组 e2e 用例覆盖了核心映射路径provider-scoped、generic proxy、SSE 三条路径都验证了返回模型改写与 request record 的
responseModel记录,和本次 PR 目标一致。
aeca825 to
d436481
Compare
What: - add responseModelMapping support for Claude providers only - allow wildcard patterns only on response source models while mapped targets stay constants - rewrite Claude final API response model fields for non-streaming JSON and Claude stream events - keep persisted request records and captured response details on upstream-original values - preserve response byte shape by updating Content-Length and disabling HTML escaping on rewritten JSON - expose Claude response model mapping in Claude provider view Why: - Claude is the first provider with a clear response shape and admin UI path for this feature - narrowing scope to Claude keeps rollout smaller and reduces cross-provider protocol ambiguity - callers need a client-facing Claude response model without changing operational records Tests: - go test -count=1 ./internal/executor ./internal/handler ./tests/e2e -run 'Test.*Provider|Test.*ModelMapping|Test.*ResponseModelMapping|TestProviderScopedProxyRoute' - go test -count=1 ./... - pnpm -C web typecheck - git diff --check
d436481 to
b0e699e
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
internal/executor/responsemodelmapping/claude_response_model_mapping.go (1)
92-116: ⚡ Quick win
mapModel每次调用都重建并排序 patterns 切片当前实现在每次调用
mapModel时从m.mappingmap 中重新构建切片并执行sort.SliceStable。由于 Go map 的迭代顺序不确定,当两个 pattern 具有相同通配符数量和长度时,排序结果也是不确定的。建议在newClaudeMapper构造时完成一次性排序。♻️ 建议的重构
type claudeMapper struct { - mapping map[string]string + patterns []string // pre-sorted: fewer wildcards first, longer first + mapping map[string]string } func newClaudeMapper(provider *domain.Provider, clientType domain.ClientType) *claudeMapper { if clientType != domain.ClientTypeClaude || provider.Config == nil || provider.Config.Claude == nil { return nil } mapping := provider.Config.Claude.ResponseModelMapping if len(mapping) == 0 { return nil } - return &claudeMapper{mapping: mapping} + patterns := make([]string, 0, len(mapping)) + for p := range mapping { + patterns = append(patterns, p) + } + sort.SliceStable(patterns, func(i, j int) bool { + left, right := patterns[i], patterns[j] + lw, rw := strings.Count(left, "*"), strings.Count(right, "*") + if lw != rw { + return lw < rw + } + return len(left) > len(right) + }) + return &claudeMapper{patterns: patterns, mapping: mapping} } func (m *claudeMapper) mapModel(model string) string { model = strings.TrimSpace(model) - patterns := make([]string, 0, len(m.mapping)) - for pattern := range m.mapping { - patterns = append(patterns, pattern) - } - sort.SliceStable(patterns, func(i, j int) bool { - left, right := patterns[i], patterns[j] - leftWildcards, rightWildcards := strings.Count(left, "*"), strings.Count(right, "*") - if leftWildcards != rightWildcards { - return leftWildcards < rightWildcards - } - return len(left) > len(right) - }) - for _, pattern := range patterns { + for _, pattern := range m.patterns {🤖 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 `@internal/executor/responsemodelmapping/claude_response_model_mapping.go` around lines 92 - 116, The mapModel function currently rebuilds and sorts the patterns slice on every call; move that work into the constructor (newClaudeMapper) so the sorted, deterministic patterns list is computed once and stored on the claudeMapper struct (e.g., add a field like sortedPatterns []string). In newClaudeMapper, iterate m.mapping, build the patterns slice, apply the same sort logic (wildcard count asc, length desc) and optionally filter out targets that are empty or contain "*" to match current mapModel behavior; then update mapModel to use the cached sortedPatterns and only perform the domain.MatchWildcard check and return the target, leaving other behavior unchanged. Ensure the field name matches existing types (claudeMapper, mapModel) so callers remain unaffected.
🤖 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 `@internal/executor/middleware_dispatch.go`:
- Around line 162-165: responseCapture currently records the original response
body (pre-mapping) because when responseModelWriter != nil the write chain
buffers into responseModelWriter and responseCapture.Body() returns the
un-mapped content; to fix, after responseCapture.Finalize() detect if
responseModelWriter (created by responsemodelmapping.NewWriter) was used and, if
so, read the rewritten/mapped bytes from that responseModelWriter and assign
them to proxyReq.ResponseInfo.Body instead of using responseCapture.Body();
update the logic around responseCapture, responseModelWriter, Finalize(), and
proxyReq.ResponseInfo.Body to prefer the mapped output so logs match what
clients actually receive.
In `@internal/executor/responsemodelmapping/response_model_mapping.go`:
- Around line 56-65: The Write method currently only writes into w.buffer and
never forwards bytes to w.underlying, so SSE events are buffered until Finalize
and clients don't receive incremental events; change streaming logic to perform
event-by-event incremental rewriting: in Writer.Write implement accumulation by
appending to w.buffer, scan for complete SSE event boundaries ("\n\n"), for each
complete event call rewriteSSE and immediately write the rewritten bytes to
w.underlying (ensuring WriteHeader/status is sent on the first actual write),
then call Flush on w.underlying (if it implements http.Flusher); keep leftover
partial event bytes in w.buffer for the next Write and ensure Finalize flushes
any remaining partial event after rewriteSSE.
In `@internal/handler/provider_proxy.go`:
- Around line 149-155: 当前在 provider_proxy.go 中对 adapter.Execute(c, provider)
之后无论 err 是否为 nil 都调用了 responseModelWriter.Finalize(),导致在 adapter 执行失败但已向
responseCapture 写入部分数据时会把映射后的内容刷入 originalWriter,从而与随后错误处理块重复写响应体;请修改逻辑使
responseModelWriter.Finalize() 仅在 adapter.Execute 返回
nil(即成功路径且响应已完整)的情况下调用,失败路径要跳过 Finalize 并丢弃/忽略 responseCapture
的缓冲内容以避免双写污染;同时在修改后检查 internal/executor/middleware_dispatch.go 中同名的 Finalize()
调用并做相同保护(只在无错时 finalize,出错时丢弃缓冲)。
In `@web/src/pages/providers/components/claude-provider-view.tsx`:
- Around line 287-293: The hard-coded user-facing strings in the
ResponseModelMappings component (e.g., the section title "Response Model
Mapping", the descriptive paragraph, input placeholders, and empty-state text)
must be wrapped with the i18n translator like the existing ProviderModelMappings
usage; update ResponseModelMappings to import and use the same t(...) function
and replace all literal strings (section title, description, all placeholder
props, and empty-state message) with calls to t('...') using descriptive
translation keys matching the pattern used in ProviderModelMappings so the UI is
localized consistently.
- Around line 231-256: saveMappings currently swallows errors from
updateProvider.mutateAsync which causes silent failures; update saveMappings
(the function that calls updateProvider.mutateAsync) to wrap the mutateAsync
call in try/catch, log or surface the error (e.g., via the same error
reporting/toast used elsewhere) and rethrow so callers can react; then update
handleAddMapping to await saveMappings inside a try/catch and only call
setNewPattern('')/setNewTarget('') after a successful save, and on catch
restore/keep inputs and show an error message similar to
handleToggleDisableErrorCooldown's rollback behavior.
---
Nitpick comments:
In `@internal/executor/responsemodelmapping/claude_response_model_mapping.go`:
- Around line 92-116: The mapModel function currently rebuilds and sorts the
patterns slice on every call; move that work into the constructor
(newClaudeMapper) so the sorted, deterministic patterns list is computed once
and stored on the claudeMapper struct (e.g., add a field like sortedPatterns
[]string). In newClaudeMapper, iterate m.mapping, build the patterns slice,
apply the same sort logic (wildcard count asc, length desc) and optionally
filter out targets that are empty or contain "*" to match current mapModel
behavior; then update mapModel to use the cached sortedPatterns and only perform
the domain.MatchWildcard check and return the target, leaving other behavior
unchanged. Ensure the field name matches existing types (claudeMapper, mapModel)
so callers remain unaffected.
🪄 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: 9dc250dc-37ad-4df5-9f91-5201f4eb5e66
📒 Files selected for processing (11)
internal/domain/model.gointernal/executor/middleware_dispatch.gointernal/executor/response_capture.gointernal/executor/responsemodelmapping/claude_response_model_mapping.gointernal/executor/responsemodelmapping/response_model_mapping.gointernal/executor/responsemodelmapping/response_model_mapping_test.gointernal/handler/provider_proxy.gotests/e2e/provider_proxy_test.goweb/src/lib/transport/types.tsweb/src/pages/providers/components/claude-provider-view.tsxweb/src/pages/providers/components/provider-edit-flow.tsx
💤 Files with no reviewable changes (1)
- web/src/pages/providers/components/provider-edit-flow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Backend Checks
- GitHub Check: playwright
- GitHub Check: e2e
🔇 Additional comments (2)
internal/handler/provider_proxy.go (1)
143-147: 写入器链路设置正确,LGTM。
responseCapture → responseModelWriter → originalWriter的分层链路构建合理:仅在 mapper 可创建时(Claude provider + 支持的 clientType)才启用。web/src/lib/transport/types.ts (1)
91-93: 类型变更正确,LGTM。
responseModelMapping?: Record<string, string>与现有的modelMapping字段风格完全一致,且仅添加到ProviderConfigClaude,与 PR 描述"仅对 Claude provider 启用"保持一致,同时与后端 Go 结构体对齐。
| responseModelWriter := responsemodelmapping.NewWriter(originalWriter, matchedRoute.Provider, originalClientType, state.isStream) | ||
| if responseModelWriter != nil { | ||
| responseCapture.SetResponseWriter(responseModelWriter) | ||
| } |
There was a problem hiding this comment.
responseCapture.Body() 捕获的是重写前的原始响应体
当 responseModelWriter != nil 时,写链为:responseCapture.body ←捕获← responseCapture.ResponseWriter = responseModelWriter(仅缓冲,不立即转发)。因此 responseCapture.Body() 返回的是模型名称未被映射的原始响应体,而 Line 245 中存入 proxyReq.ResponseInfo.Body 的内容也是如此。
这意味着管理界面中的请求详情日志显示的是上游返回的模型名称(如 "upstream"),而客户端实际收到的是映射后的名称(如 "alias")。如果希望日志与客户端看到的保持一致,可在 Finalize() 之后从 responseModelWriter 读取重写后的内容。
🤖 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 `@internal/executor/middleware_dispatch.go` around lines 162 - 165,
responseCapture currently records the original response body (pre-mapping)
because when responseModelWriter != nil the write chain buffers into
responseModelWriter and responseCapture.Body() returns the un-mapped content; to
fix, after responseCapture.Finalize() detect if responseModelWriter (created by
responsemodelmapping.NewWriter) was used and, if so, read the rewritten/mapped
bytes from that responseModelWriter and assign them to
proxyReq.ResponseInfo.Body instead of using responseCapture.Body(); update the
logic around responseCapture, responseModelWriter, Finalize(), and
proxyReq.ResponseInfo.Body to prefer the mapped output so logs match what
clients actually receive.
| func (w *Writer) Write(b []byte) (int, error) { | ||
| _, err := w.buffer.Write(b) | ||
| return len(b), err | ||
| } | ||
|
|
||
| func (w *Writer) Flush() { | ||
| if f, ok := w.underlying.(http.Flusher); ok { | ||
| f.Flush() | ||
| } | ||
| } |
There was a problem hiding this comment.
流式 SSE 响应被完全缓冲,客户端无法实时收到 token
Write() 仅将数据写入 w.buffer,从不转发给 w.underlying;Flush() 虽然传递给 underlying,但 underlying 此时并无任何待发数据,因此是无效调用。所有 SSE 事件在 Finalize() 被调用前都会积压在缓冲区中,届时才一次性发出。
实际效果是:启用 responseModelMapping 的流式请求,TTFT(首字时延)变为整个生成时间,等同于把流式接口降级为非流式接口。对于面向最终用户的 Claude 流式场景,这是严重的体验倒退。
Go 默认对响应写入进行缓冲,SSE 的正确做法是每写入一个事件后立即调用 Flush() 将数据推送给客户端。
建议的修复方向:对流式模式改为逐事件(event-by-event)增量重写——按双换行(\n\n)边界分割,对每个完整的 SSE 事件进行 rewriteSSE,随即写入 underlying 并调用 Flush(),以保留实时推送语义。以下是示意性改动:
♻️ 增量流式重写方案(示意)
+// pendingSSE accumulates bytes for the current in-progress SSE event.
type Writer struct {
underlying http.ResponseWriter
mapper mapper
isStream bool
statusCode int
buffer bytes.Buffer
+ pendingSSE bytes.Buffer
}
func (w *Writer) Write(b []byte) (int, error) {
- _, err := w.buffer.Write(b)
- return len(b), err
+ if !w.isStream {
+ _, err := w.buffer.Write(b)
+ return len(b), err
+ }
+ w.pendingSSE.Write(b)
+ // Flush complete SSE events (terminated by blank line) immediately.
+ for {
+ data := w.pendingSSE.Bytes()
+ idx := bytes.Index(data, []byte("\n\n"))
+ if idx < 0 {
+ break
+ }
+ event := data[:idx+2]
+ rewritten := w.mapper.rewrite(event, true)
+ w.underlying.Write(rewritten)
+ if f, ok := w.underlying.(http.Flusher); ok {
+ f.Flush()
+ }
+ remaining := make([]byte, len(data)-idx-2)
+ copy(remaining, data[idx+2:])
+ w.pendingSSE.Reset()
+ w.pendingSSE.Write(remaining)
+ }
+ return len(b), nil
}
func (w *Writer) Finalize() error {
- body := w.mapper.rewrite(w.buffer.Bytes(), w.isStream)
- if !w.isStream && w.underlying.Header().Get("Content-Length") != "" {
+ if w.isStream {
+ // Flush any remaining bytes (e.g., partial last line).
+ if w.pendingSSE.Len() > 0 {
+ remaining := w.mapper.rewrite(w.pendingSSE.Bytes(), true)
+ w.underlying.WriteHeader(w.statusCode)
+ _, err := w.underlying.Write(remaining)
+ return err
+ }
+ w.underlying.WriteHeader(w.statusCode)
+ return nil
+ }
+ body := w.mapper.rewrite(w.buffer.Bytes(), false)
+ if w.underlying.Header().Get("Content-Length") != "" {
w.underlying.Header().Set("Content-Length", strconv.Itoa(len(body)))
}
w.underlying.WriteHeader(w.statusCode)
_, err := w.underlying.Write(body)
return err
}注意:
WriteHeader需要在流式首次Write时调用,上述示意代码未完整处理,实际实现需要在首个事件写入时转发w.statusCode。
🤖 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 `@internal/executor/responsemodelmapping/response_model_mapping.go` around
lines 56 - 65, The Write method currently only writes into w.buffer and never
forwards bytes to w.underlying, so SSE events are buffered until Finalize and
clients don't receive incremental events; change streaming logic to perform
event-by-event incremental rewriting: in Writer.Write implement accumulation by
appending to w.buffer, scan for complete SSE event boundaries ("\n\n"), for each
complete event call rewriteSSE and immediately write the rewritten bytes to
w.underlying (ensuring WriteHeader/status is sent on the first actual write),
then call Flush on w.underlying (if it implements http.Flusher); keep leftover
partial event bytes in w.buffer for the next Write and ensure Finalize flushes
any remaining partial event after rewriteSSE.
| err = adapter.Execute(c, provider) | ||
| c.Writer = originalWriter | ||
| if responseModelWriter != nil { | ||
| if finalizeErr := responseModelWriter.Finalize(); finalizeErr != nil { | ||
| log.Printf("[ProviderProxy] response model mapping finalize failed: %v", finalizeErr) | ||
| } | ||
| } |
There was a problem hiding this comment.
Finalize() 被无条件调用,错误路径下可能产生双写污染
当前 Finalize() 在第 152 行无条件执行,处于 if err == nil 分支之前。若 adapter.Execute 在返回错误前已向 responseCapture(进而写入 responseModelWriter 缓冲区)写入了部分数据,Finalize() 会将这部分映射后内容刷写到 originalWriter;随后错误处理块(第 183–194 行)再次向 originalWriter 写入错误响应,导致 HTTP 响应体被污染。
虽然"适配器出错前未写任何字节"的情况下缓冲区为空不会触发此问题,但流式场景中上游中途断连时会实际命中该路径。
🐛 建议修复
err = adapter.Execute(c, provider)
c.Writer = originalWriter
-if responseModelWriter != nil {
- if finalizeErr := responseModelWriter.Finalize(); finalizeErr != nil {
- log.Printf("[ProviderProxy] response model mapping finalize failed: %v", finalizeErr)
- }
-}
now := time.Now()
proxyReq.EndTime = now
@@ -168,6 +163,11 @@ if responseModelWriter != nil {
if err == nil {
proxyReq.Status = "COMPLETED"
+ if responseModelWriter != nil {
+ if finalizeErr := responseModelWriter.Finalize(); finalizeErr != nil {
+ log.Printf("[ProviderProxy] response model mapping finalize failed: %v", finalizeErr)
+ }
+ }
clearProxyRequestDetail(proxyReq, clearDetail)
_ = h.proxyRequestRepo.Update(proxyReq)
return同时建议确认 internal/executor/middleware_dispatch.go 中的 Finalize() 调用是否有相同问题。
🤖 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 `@internal/handler/provider_proxy.go` around lines 149 - 155, 当前在
provider_proxy.go 中对 adapter.Execute(c, provider) 之后无论 err 是否为 nil 都调用了
responseModelWriter.Finalize(),导致在 adapter 执行失败但已向 responseCapture
写入部分数据时会把映射后的内容刷入 originalWriter,从而与随后错误处理块重复写响应体;请修改逻辑使
responseModelWriter.Finalize() 仅在 adapter.Execute 返回
nil(即成功路径且响应已完整)的情况下调用,失败路径要跳过 Finalize 并丢弃/忽略 responseCapture
的缓冲内容以避免双写污染;同时在修改后检查 internal/executor/middleware_dispatch.go 中同名的 Finalize()
调用并做相同保护(只在无错时 finalize,出错时丢弃缓冲)。
| const saveMappings = async (next: Record<string, string>) => { | ||
| if (!config) return; | ||
| await updateProvider.mutateAsync({ | ||
| id: provider.id, | ||
| data: { | ||
| ...provider, | ||
| config: { | ||
| ...provider.config, | ||
| claude: { | ||
| ...config, | ||
| responseModelMapping: Object.keys(next).length > 0 ? next : undefined, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| }; | ||
|
|
||
| const handleAddMapping = async () => { | ||
| const pattern = newPattern.trim(); | ||
| const target = newTarget.trim(); | ||
| if (!pattern || !target) return; | ||
| const next = { ...(config?.responseModelMapping || {}), [pattern]: target }; | ||
| await saveMappings(next); | ||
| setNewPattern(''); | ||
| setNewTarget(''); | ||
| }; |
There was a problem hiding this comment.
saveMappings 中缺少错误处理
updateProvider.mutateAsync 抛出异常时会静默失败:用户在 handleAddMapping 场景中会看到输入框已清空(Line 254-255),但界面不会显示任何错误提示,且无法回滚。同文件的 handleToggleDisableErrorCooldown(Line 384-399)有完整的 try/catch 回滚逻辑,建议保持一致。
🛠️ 修复建议
const saveMappings = async (next: Record<string, string>) => {
if (!config) return;
- await updateProvider.mutateAsync({
+ try {
+ await updateProvider.mutateAsync({
id: provider.id,
data: {
...provider,
config: {
...provider.config,
claude: {
...config,
responseModelMapping: Object.keys(next).length > 0 ? next : undefined,
},
},
},
- });
+ });
+ } catch (err) {
+ // TODO: expose error to user (e.g., via toast/setError state)
+ console.error('Failed to save response model mappings', err);
+ }
};🤖 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/pages/providers/components/claude-provider-view.tsx` around lines 231
- 256, saveMappings currently swallows errors from updateProvider.mutateAsync
which causes silent failures; update saveMappings (the function that calls
updateProvider.mutateAsync) to wrap the mutateAsync call in try/catch, log or
surface the error (e.g., via the same error reporting/toast used elsewhere) and
rethrow so callers can react; then update handleAddMapping to await saveMappings
inside a try/catch and only call setNewPattern('')/setNewTarget('') after a
successful save, and on catch restore/keep inputs and show an error message
similar to handleToggleDisableErrorCooldown's rollback behavior.
| <h4 className="text-lg font-semibold text-foreground">Response Model Mapping</h4> | ||
| <span className="text-sm text-muted-foreground">({mappings.length})</span> | ||
| </div> | ||
|
|
||
| <div className="bg-card border border-border rounded-xl p-4"> | ||
| <p className="text-xs text-muted-foreground mb-4"> | ||
| Enabled for Claude providers in this rollout. ResponseModel pattern → constant MappedModel, applied only to final API responses. |
There was a problem hiding this comment.
用户可见字符串未使用 i18n
ResponseModelMappings 组件中存在多处硬编码英文字符串,与同文件 ProviderModelMappings 组件中使用 t(...) 的模式不一致,将影响非英语用户界面的本地化。
受影响的字符串:
- Line 287:
"Response Model Mapping"(标题) - Line 293: 描述文本
- Lines 304, 313, 339, 348:
placeholder属性 - Line 331: 空状态文本
🛠️ 修复建议(部分示例)
-<h4 className="text-lg font-semibold text-foreground">Response Model Mapping</h4>
+<h4 className="text-lg font-semibold text-foreground">{t('responseModelMappings.title')}</h4>-<p className="text-xs text-muted-foreground mb-4">
- Enabled for Claude providers in this rollout. ResponseModel pattern → constant MappedModel, applied only to final API responses.
-</p>
+<p className="text-xs text-muted-foreground mb-4">{t('responseModelMappings.pageDesc')}</p>-placeholder="ResponseModel pattern"
+placeholder={t('responseModelMappings.matchPattern')}-<p className="text-muted-foreground text-sm">No response model mappings configured.</p>
+<p className="text-muted-foreground text-sm">{t('responseModelMappings.noMappings')}</p>Also applies to: 304-304, 313-313, 331-331, 339-339, 348-348
🤖 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/pages/providers/components/claude-provider-view.tsx` around lines 287
- 293, The hard-coded user-facing strings in the ResponseModelMappings component
(e.g., the section title "Response Model Mapping", the descriptive paragraph,
input placeholders, and empty-state text) must be wrapped with the i18n
translator like the existing ProviderModelMappings usage; update
ResponseModelMappings to import and use the same t(...) function and replace all
literal strings (section title, description, all placeholder props, and
empty-state message) with calls to t('...') using descriptive translation keys
matching the pattern used in ProviderModelMappings so the UI is localized
consistently.
Summary
responseModelMappingalongside existing ClaudemodelMapping.Changes
responseModelMappingonly for Claude providers and Claude client responses.internal/executor/responsemodelmapping/response_model_mapping.go.internal/executor/responsemodelmapping/claude_response_model_mapping.go.modeland Claude streammessage.modelin final API responses.Content-Lengthand disable JSON HTML escaping during rewrite.ResponseModelMappingsUI component, auto-hidden for unsupported providers and currently mounted on the Claude provider page.Tests
go test -count=1 ./internal/executor/responsemodelmapping ./internal/executor ./internal/handler ./tests/e2e -run 'Test.*Provider|Test.*ModelMapping|Test.*ResponseModelMapping|TestProviderScopedProxyRoute|TestWriter|TestMapModel'— passgo test -count=1 ./...— passpnpm -C web typecheck— passgit diff --check— passRisks
responseModelMappingkeep the existing response path.modeland Claude streammessage.model.Summary by CodeRabbit
新增功能
测试