fix: wire response model mapping into provider edit#553
Merged
Conversation
📝 Walkthrough走查该PR为自定义提供商配置添加响应模型映射功能。后端扩展domain模型并重构Claude响应修饰符以支持从自定义配置或Claude配置中选择映射；代理处理程序现在通过ResponseModifierWriter应用映射。前端添加了新的UI组件来编辑映射，并从claude-provider-view迁移到通用的provider-edit-flow。新增国际化支持和相应测试验证功能。 变更响应模型映射功能
预期代码审查工作量🎯 3 (中等) | ⏱️ ~25 分钟 可能相关的PR
建议审查人
诗歌
🚥 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 |
ymkiux
approved these changes
May 8, 2026
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/handler/provider_proxy.go (1)
142-204:⚠️ Potential issue | 🔴 Critical标准路由处理器中缺少响应模型修改集成
PR 声称对"标准路由和 provider 直连路由"都应用了响应模型映射，但实际检查发现：
proxy.go的ProxyHandler.dispatch()方法（第 ~262 行）调用h.executor.ExecuteWith(c)后直接进行简单的错误处理，未使用ResponseModifierWriterprovider_proxy.go的ProviderProxyHandler才集成了responsemodifier.NewResponseModifierWriter和响应捕获机制- 两个处理器的执行模式完全不同，这说明 PR 在标准路由路径上的修改并未完成
标准路由路径应补充以下内容以保持功能一致性：
- 导入
responsemodifier包- 在
ExecuteWith()前后添加ResponseModifierWriter封装和responseCapture机制- 补充
proxyReq跟踪和日志记录（状态、时长、响应头、响应体）🤖 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 142 - 204, The standard route handler (ProxyHandler.dispatch) is missing the response modifier/capture integration present in ProviderProxyHandler: wrap the original c.Writer with responsemodifier.NewResponseModifierWriter (import responsemodifier), fall back to the original when nil, create executor.NewResponseCapture around the chosen writer, assign c.Writer = responseCapture before calling h.executor.ExecuteWith(c) and restore it after, then populate proxyReq fields (EndTime, Duration, StatusCode from responseCapture.StatusCode(), ResponseInfo with CapturedHeaders/Body unless clearDetail) and invoke modifierWriter.Finalize() on success or failure (handling errors like in ProviderProxyHandler), and ensure proxyReq.Status is set to COMPLETED or FAILED and proxyRequestRepo.Update(proxyReq) is called accordingly; use the same helper functions writeProxyError/writeStreamError/writeError to render errors.
🧹 Nitpick comments (1)
internal/executor/responsemodifier/response_modifier_writer_test.go (1)
134-145: ⚡ Quick win
customProvider(nil)未在 Disabled 测试中覆盖
TestResponseModifierWriterDisabled覆盖了claudeProvider(nil)和非 Claude 客户端类型，但未覆盖customProvider(nil)应返回nil的情况。若后续修改responseModelMapping的 fallback 逻辑，此行为可能悄然回归。✅ 建议补充测试用例
func TestResponseModifierWriterDisabled(t *testing.T) { rr := httptest.NewRecorder() if NewResponseModifierWriter(rr, &domain.Provider{Type: "codex"}, domain.ClientTypeCodex, false) != nil { t.Fatal("expected non-claude provider to be disabled") } if NewResponseModifierWriter(rr, claudeProvider(nil), domain.ClientTypeClaude, false) != nil { t.Fatal("expected empty mapping to be disabled") } + if NewResponseModifierWriter(rr, customProvider(nil), domain.ClientTypeClaude, false) != nil { + t.Fatal("expected custom provider with nil mapping to be disabled") + } if NewResponseModifierWriter(rr, claudeProvider(map[string]string{"a": "b"}), domain.ClientTypeCodex, false) != nil { t.Fatal("expected non-claude client shape to be disabled") } }🤖 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/responsemodifier/response_modifier_writer_test.go` around lines 134 - 145, Add a missing assertion in TestResponseModifierWriterDisabled to ensure NewResponseModifierWriter returns nil for a custom provider with no mapping: call NewResponseModifierWriter(rr, customProvider(nil), domain.ClientTypeCustom, false) and fail the test if it is non-nil; this mirrors the existing checks for claudeProvider(nil) and non-claude providers and prevents regressions if responseModelMapping fallback logic changes.
🤖 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 `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Around line 217-231: handleUpdateMapping currently silently returns on invalid
target, drops entries when target is cleared, and overwrites existing patterns;
change it to validate and surface errors instead of returning silently by
invoking the existing error/validation handler (or set a local error state) when
isValidTarget(nextTarget) fails, do not remove the original entry if nextTarget
is empty (re-add the oldPattern -> value pair to next), and before inserting
nextPattern check for conflicts: if nextPattern !== oldPattern and nextPattern
already exists in entries, abort the update and surface a duplicate-key error
rather than overwriting; use the existing symbols handleUpdateMapping, entries,
onChange, and isValidTarget to implement these checks and error callbacks.
- Around line 259-286: The pattern input loses focus because each row uses
key={pattern} and handleUpdateMapping updates the parent on every keystroke;
change the row to a local controlled component (extract a Row component, e.g.,
ResponseModelMappingRow) that keeps pattern and target in useState, render rows
with a stable key (e.g., index or an id), wire ModelInput and Input to the local
state, call handleUpdateMapping only onBlur (or onEnter) to push changes to
formData.responseModelMapping, and keep handleDeleteMapping as-is; this
preserves focus while still syncing updates to the parent.
---
Outside diff comments:
In `@internal/handler/provider_proxy.go`:
- Around line 142-204: The standard route handler (ProxyHandler.dispatch) is
missing the response modifier/capture integration present in
ProviderProxyHandler: wrap the original c.Writer with
responsemodifier.NewResponseModifierWriter (import responsemodifier), fall back
to the original when nil, create executor.NewResponseCapture around the chosen
writer, assign c.Writer = responseCapture before calling
h.executor.ExecuteWith(c) and restore it after, then populate proxyReq fields
(EndTime, Duration, StatusCode from responseCapture.StatusCode(), ResponseInfo
with CapturedHeaders/Body unless clearDetail) and invoke
modifierWriter.Finalize() on success or failure (handling errors like in
ProviderProxyHandler), and ensure proxyReq.Status is set to COMPLETED or FAILED
and proxyRequestRepo.Update(proxyReq) is called accordingly; use the same helper
functions writeProxyError/writeStreamError/writeError to render errors.
---
Nitpick comments:
In `@internal/executor/responsemodifier/response_modifier_writer_test.go`:
- Around line 134-145: Add a missing assertion in
TestResponseModifierWriterDisabled to ensure NewResponseModifierWriter returns
nil for a custom provider with no mapping: call NewResponseModifierWriter(rr,
customProvider(nil), domain.ClientTypeCustom, false) and fail the test if it is
non-nil; this mirrors the existing checks for claudeProvider(nil) and non-claude
providers and prevents regressions if responseModelMapping fallback logic
changes.
🪄 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: e2d9f550-764c-4617-b9ac-90b9c6a7f9eb
📒 Files selected for processing (10)
internal/domain/model.gointernal/executor/responsemodifier/claude_response_modifier.gointernal/executor/responsemodifier/response_modifier_writer.gointernal/executor/responsemodifier/response_modifier_writer_test.gointernal/handler/provider_proxy.goweb/src/lib/transport/types.tsweb/src/locales/en.jsonweb/src/locales/zh.jsonweb/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/claude-provider-view.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
Comment on lines
+217
to
+231
| const handleUpdateMapping = (oldPattern: string, pattern: string, target: string) => { | ||
| const next: Record<string, string> = {}; | ||
| for (const [key, value] of entries) { | ||
| if (key === oldPattern) continue; | ||
| next[key] = value; | ||
| } | ||
|
|
||
| const nextPattern = pattern.trim(); | ||
| const nextTarget = target.trim(); | ||
| if (nextTarget && !isValidTarget(nextTarget)) return; | ||
| if (nextPattern && nextTarget) { | ||
| next[nextPattern] = nextTarget; | ||
| } | ||
| onChange(next); | ||
| }; |
There was a problem hiding this comment.
内联编辑时静默失败与意外删除条目
handleUpdateMapping 存在两个边界问题：
-
静默校验失败（line 226）：当用户在 target 字段输入包含
*的内容时，函数直接return而不显示任何错误提示。与新增按钮（通过禁用状态给出明确反馈）不同，内联编辑完全没有用户可见的错误指示。 -
意外删除条目（lines 227-229）：当
nextTarget为空字符串时，if (nextPattern && nextTarget)为假，条目不会被重新加入next，相当于在用户无意间清空 target 字段时静默删除了该条目。 -
重复 pattern 键冲突（lines 218-230）：若用户将 pattern "A" 重命名为已存在的 "B"，现有 "B" 的值会被覆盖，且没有任何警告。
🛡️ 建议的改进
const handleUpdateMapping = (oldPattern: string, pattern: string, target: string) => {
const next: Record<string, string> = {};
for (const [key, value] of entries) {
if (key === oldPattern) continue;
next[key] = value;
}
const nextPattern = pattern.trim();
const nextTarget = target.trim();
- if (nextTarget && !isValidTarget(nextTarget)) return;
- if (nextPattern && nextTarget) {
- next[nextPattern] = nextTarget;
- }
+ if (!nextPattern || !nextTarget) return; // 不允许空 pattern 或 target
+ if (!isValidTarget(nextTarget)) return; // 无效 target 时不提交（可配合 UI 提示）
+ if (nextPattern in next) return; // 防止覆盖已有 pattern
+ next[nextPattern] = nextTarget;
onChange(next);
};🤖 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 217 -
231, handleUpdateMapping currently silently returns on invalid target, drops
entries when target is cleared, and overwrites existing patterns; change it to
validate and surface errors instead of returning silently by invoking the
existing error/validation handler (or set a local error state) when
isValidTarget(nextTarget) fails, do not remove the original entry if nextTarget
is empty (re-add the oldPattern -> value pair to next), and before inserting
nextPattern check for conflicts: if nextPattern !== oldPattern and nextPattern
already exists in entries, abort the update and surface a duplicate-key error
rather than overwriting; use the existing symbols handleUpdateMapping, entries,
onChange, and isValidTarget to implement these checks and error callbacks.
Comment on lines
+259
to
+286
| {entries.map(([pattern, target], index) => ( | ||
| <div key={pattern} className="flex items-center gap-2"> | ||
| <span className="text-xs text-muted-foreground w-6 shrink-0">{index + 1}.</span> | ||
| <ModelInput | ||
| value={pattern} | ||
| onChange={(value) => handleUpdateMapping(pattern, value, target)} | ||
| placeholder={t('responseModelMappings.matchPattern')} | ||
| disabled={disabled} | ||
| className="flex-1 min-w-0 h-8 text-sm" | ||
| /> | ||
| <ArrowRight className="h-4 w-4 text-muted-foreground shrink-0" /> | ||
| <Input | ||
| value={target} | ||
| onChange={(event) => handleUpdateMapping(pattern, pattern, event.target.value)} | ||
| placeholder={t('responseModelMappings.targetModel')} | ||
| disabled={disabled} | ||
| className="flex-1 min-w-0 h-8 text-sm" | ||
| /> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| onClick={() => handleDeleteMapping(pattern)} | ||
| disabled={disabled} | ||
| > | ||
| <Trash2 className="h-4 w-4 text-destructive" /> | ||
| </Button> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
模式字段（pattern）编辑时每次按键都会丢失焦点
在编辑已有条目的模式（pattern）输入框时，key={pattern} 与 handleUpdateMapping 的组合会触发以下循环：用户输入单个字符 → onChange 触发 → 父组件 formData.responseModelMapping 更新 → entries useMemo 重新计算（旧 key 消失、新 key 出现） → React 以新 key 卸载旧输入框并挂载新输入框 → 焦点丢失。
实际效果是：pattern 输入框每输入一个字符后就会失去焦点，用户完全无法正常编辑该字段。target 的 Input（line 272）不受影响，因为其所在行的 key 不会因 target 变化而改变。
修复思路：将各行的 pattern 和 target 改为局部受控状态（useState），仅在 onBlur 时才调用 handleUpdateMapping 提交到父组件。示例：
🐛 建议的修复方式（抽取行组件，用 onBlur 提交）
+function MappingRow({
+ index,
+ pattern,
+ target,
+ disabled,
+ onUpdate,
+ onDelete,
+}: {
+ index: number;
+ pattern: string;
+ target: string;
+ disabled: boolean;
+ onUpdate: (oldPattern: string, pattern: string, target: string) => void;
+ onDelete: (pattern: string) => void;
+}) {
+ const { t } = useTranslation();
+ const [localPattern, setLocalPattern] = useState(pattern);
+ const [localTarget, setLocalTarget] = useState(target);
+
+ // 当外部 pattern/target 变更时同步（例如删除了其他行）
+ useEffect(() => { setLocalPattern(pattern); }, [pattern]);
+ useEffect(() => { setLocalTarget(target); }, [target]);
+
+ return (
+ <div className="flex items-center gap-2">
+ <span className="text-xs text-muted-foreground w-6 shrink-0">{index + 1}.</span>
+ <ModelInput
+ value={localPattern}
+ onChange={setLocalPattern}
+ onBlur={() => onUpdate(pattern, localPattern, localTarget)}
+ placeholder={t('responseModelMappings.matchPattern')}
+ disabled={disabled}
+ className="flex-1 min-w-0 h-8 text-sm"
+ />
+ <ArrowRight className="h-4 w-4 text-muted-foreground shrink-0" />
+ <Input
+ value={localTarget}
+ onChange={(e) => setLocalTarget(e.target.value)}
+ onBlur={() => onUpdate(pattern, localPattern, localTarget)}
+ placeholder={t('responseModelMappings.targetModel')}
+ disabled={disabled}
+ className="flex-1 min-w-0 h-8 text-sm"
+ />
+ <Button variant="ghost" size="sm" onClick={() => onDelete(pattern)} disabled={disabled}>
+ <Trash2 className="h-4 w-4 text-destructive" />
+ </Button>
+ </div>
+ );
+}然后在 ResponseModelMappings 中：
-{entries.map(([pattern, target], index) => (
- <div key={pattern} className="flex items-center gap-2">
- ...
- </div>
-))}
+{entries.map(([pattern, target], index) => (
+ <MappingRow
+ key={pattern}
+ index={index}
+ pattern={pattern}
+ target={target}
+ disabled={disabled}
+ onUpdate={handleUpdateMapping}
+ onDelete={handleDeleteMapping}
+ />
+))}🤖 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 259 -
286, The pattern input loses focus because each row uses key={pattern} and
handleUpdateMapping updates the parent on every keystroke; change the row to a
local controlled component (extract a Row component, e.g.,
ResponseModelMappingRow) that keeps pattern and target in useState, render rows
with a stable key (e.g., index or an id), wire ModelInput and Input to the local
state, call handleUpdateMapping only onBlur (or onEnter) to push changes to
formData.responseModelMapping, and keep handleDeleteMapping as-is; this
preserves focus while still syncing updates to the parent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tests
Summary by CodeRabbit
发布说明