feats: the flexable params override and compatible format - #1603
Conversation
WalkthroughCentralizes JSON parameter-override handling by adding a new operations-capable override engine and ApplyParamOverride, updates multiple relay handlers to call it, adds Tidwall JSON libs to go.mod, and updates the web UI to document and insert both legacy and new override formats. Changes
Sequence Diagram(s)sequenceDiagram
participant Handler
participant OverrideEngine as relaycommon.ApplyParamOverride
participant Parser as Operations Parser
participant Legacy as Legacy Merge
Handler->>OverrideEngine: ApplyParamOverride(jsonData, paramOverride)
alt paramOverride empty
OverrideEngine-->>Handler: original jsonData
else paramOverride has "operations"
OverrideEngine->>Parser: tryParseOperations(paramOverride)
alt parse ok
Parser-->>OverrideEngine: []ParamOperation
OverrideEngine->>OverrideEngine: applyOperations sequentially (conditions checked)
alt apply ok
OverrideEngine-->>Handler: updated jsonData
else apply error
OverrideEngine-->>Handler: error
end
else parse fail
OverrideEngine->>Legacy: applyOperationsLegacy(jsonData, paramOverride)
Legacy-->>OverrideEngine: merged jsonData or error
OverrideEngine-->>Handler: result
end
else no "operations"
OverrideEngine->>Legacy: applyOperationsLegacy(jsonData, paramOverride)
Legacy-->>OverrideEngine: merged jsonData or error
OverrideEngine-->>Handler: result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
web/src/components/table/channels/modals/EditChannelModal.jsx (1)
1656-1700: Add client-side JSON validation for param_override and block stream overridesGood UX addition. To reduce backend round-trips and align with “不支持覆盖 stream 参数”, add a validator that:
- Ensures param_override is valid JSON when provided.
- Blocks top-level "stream" overrides and operations targeting path "stream".
Apply this diff within this TextArea to add a custom validation rule:
<Form.TextArea field='param_override' label={t('参数覆盖')} placeholder={ t('此项可选，用于覆盖请求参数。不支持覆盖 stream 参数') + '\n' + t('旧格式（直接覆盖）：') + '\n{\n "temperature": 0,\n "max_tokens": 1000\n}' + '\n\n' + t('新格式（支持条件判断与json自定义）：') + '\n{\n "operations": [\n {\n "path": "temperature",\n "mode": "set",\n "value": 0.7,\n "conditions": [\n {\n "path": "model",\n "mode": "prefix",\n "value": "gpt"\n }\n ]\n }\n ]\n}' } + rules={[ + { + validator: (_, value) => { + if (!value || `${value}`.trim() === '') return true; + if (!verifyJSON(value)) return t('参数覆盖必须是合法的 JSON 格式'); + try { + const obj = JSON.parse(value); + if (obj && typeof obj === 'object') { + // Block old-format "stream" + if (Object.prototype.hasOwnProperty.call(obj, 'stream')) { + return t('不支持覆盖 stream 参数'); + } + // Block new-format operations that target "stream" + if (Array.isArray(obj.operations)) { + const violates = obj.operations.some(op => typeof op?.path === 'string' && op.path.trim() === 'stream'); + if (violates) return t('不支持覆盖 stream 参数'); + } + } + } catch {} + return true; + } + } + ]} autosize onChange={(value) => handleInputChange('param_override', value)} extraText={relay/claude_handler.go (1)
86-86: Centralized override application: ensure disallowing 'stream' and add testsThe switch to relaycommon.ApplyParamOverride improves consistency and maintainability. Please verify that:
- The override engine rejects attempts to change stream, matching the UI guidance.
- Both legacy and operations formats are covered by tests in this handler path.
If helpful, I can draft unit tests that:
- Assert legacy { "temperature": 0 } merges.
- Assert operations-based set/delete/prepend/append with AND/OR conditions.
- Assert attempts to set "stream" are rejected.
relay/rerank_handler.go (1)
65-65: Good consolidation; confirm parity and stream-protectionUsing ApplyParamOverride here aligns the rerank path with other handlers. Please confirm:
- Behavior parity with previous override semantics (legacy path) for flat keys.
- Engine-level guard blocks any override of "stream".
- Add a test that covers both formats for the rerank request structure.
Happy to help author a focused test that feeds representative rerank payloads through ApplyParamOverride and validates the final JSON.
relay/common/override.go (4)
232-242: Prepend/append to non-existent path currently errorsmodifyValue returns an error if the path does not exist (type is Null). Many users expect prepend/append to create the field when missing (e.g., set string to value or initialize an array). Consider handling the non-existent case by creating the field based on the type of value.
One approach:
func modifyValue(jsonStr, path string, value interface{}, keepOrigin, isPrepend bool) (string, error) { current := gjson.Get(jsonStr, path) + if !current.Exists() { + // Initialize field if missing + switch v := value.(type) { + case []interface{}: + return sjson.Set(jsonStr, path, v) + case map[string]interface{}: + return sjson.Set(jsonStr, path, v) + default: + return sjson.Set(jsonStr, path, fmt.Sprintf("%v", v)) + } + } switch { // ... } }
58-66: Validate required fields per operation typetryParseOperations does not enforce that Path is present for set/delete/prepend/append, or that From/To are present for move. These will fall through to runtime errors in applyOperations and, before the previous comment’s fix, could trigger an unsafe legacy fallback. Prefer early validation with a clear error.
168-181: Legacy merge should ignore “operations” key (defensive)Even with the stricter ApplyParamOverride change, consider excluding the reserved "operations" key from the legacy merge to avoid accidental leakage if applyOperationsLegacy is reused elsewhere.
Example tweak:
for key, value := range paramOverride { - reqMap[key] = value + if key != "operations" { + reqMap[key] = value + } }
145-166: Condition matching is string-only; document or extendcheckSingleCondition coerces values to strings and supports full/prefix/suffix/contains. If numeric/boolean matches are expected, extend modes or document the string-only behavior to avoid confusion.
I can add unit tests covering AND/OR logic, number/boolean comparisons, and array/object targets if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
go.mod(2 hunks)relay/claude_handler.go(1 hunks)relay/common/override.go(1 hunks)relay/gemini_handler.go(1 hunks)relay/image_handler.go(1 hunks)relay/relay-text.go(1 hunks)relay/rerank_handler.go(1 hunks)relay/responses_handler.go(1 hunks)web/src/components/table/channels/modals/EditChannelModal.jsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
relay/responses_handler.go (1)
relay/common/override.go (1)
ApplyParamOverride(28-46)
relay/image_handler.go (1)
relay/common/override.go (1)
ApplyParamOverride(28-46)
relay/claude_handler.go (1)
relay/common/override.go (1)
ApplyParamOverride(28-46)
relay/rerank_handler.go (1)
relay/common/override.go (1)
ApplyParamOverride(28-46)
web/src/components/table/channels/modals/EditChannelModal.jsx (1)
web/src/components/table/channels/modals/EditTagModal.jsx (1)
handleInputChange(77-129)
relay/relay-text.go (1)
relay/common/override.go (1)
ApplyParamOverride(28-46)
relay/gemini_handler.go (1)
relay/common/override.go (1)
ApplyParamOverride(28-46)
🔇 Additional comments (5)
relay/responses_handler.go (1)
56-56: Centralized override: looks good; align pass-through semantics and block 'stream'This change is correct. Two small follow-ups:
- Ensure ApplyParamOverride rejects "stream" overrides (consistent with UI).
- Consider aligning pass-through gating with other handlers (many check both global and channel setting); if intentional, ignore.
Would you like me to scan all handlers and summarize where pass-through gating differs so we can decide whether to unify?
relay/image_handler.go (1)
64-70: Centralized override call: looks goodSwitching to relaycommon.ApplyParamOverride trims duplication and unifies behavior. Error mapping to ChannelParamOverrideInvalid and skip-retry is appropriate.
relay/gemini_handler.go (2)
124-130: Centralized override logic: good replacementUsing relaycommon.ApplyParamOverride here aligns this path with the new engine and preserves existing error semantics.
222-233: ParamOverride helper used consistently across relay handlersAll instances of manual shallow merges for
info.ParamOverridehave been replaced withrelaycommon.ApplyParamOverride, including inrelay/gemini_handler.go. No residual manual merge logic remains.relay/relay-text.go (1)
134-141: Param override centralization: LGTMGood move to delegate to ApplyParamOverride and return ChannelParamOverrideInvalid with skip-retry on error.
| github.com/tidwall/gjson v1.18.0 | ||
| github.com/tidwall/sjson v1.2.5 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New gjson/sjson deps: verify CI/toolchain alignment and transitive integrity
The additions look correct for path-based JSON ops. Please verify:
- CI/go toolchain supports go 1.23.x used here.
- No stale indirects; run a tidy pass to ensure minimal sets.
- All handler sites rely on the centralized override (no lingering bespoke merges).
You can quickly sanity-check call-sites and look for any remaining manual override logic with:
Also applies to: 87-88
🏁 Script executed:
#!/bin/bash
# Show all references to ApplyParamOverride and ParamOverride across handlers
rg -nP --type=go -C2 'ApplyParamOverride\s*\(|\bParamOverride\b'
# Optional: list direct usages of tidwall libs to confirm they're only in the override engine
rg -nP --type=go -C2 '\btidwall/(gjson|sjson)\b'Length of output: 5389
🏁 Script executed:
#!/bin/bash
# Check Go version in go.mod
grep -n '^go ' go.mod
# List indirect dependencies in go.mod
grep -n 'indirect' go.modLength of output: 3197
New tidwall libs added: verify toolchain, tidy deps, and consolidate override logic
Please address the following before merging:
- go.mod sets Go version to 1.23.4—confirm your CI/workflow is running Go ≥ 1.23.
- Run
go mod tidyto prune any stale indirect entries. - We’ve identified a bespoke merge in relay/gemini_handler.go (lines 221–228). Replace this manual Unmarshal/loop logic with the centralized
relaycommon.ApplyParamOverridecall for consistency.
Also apply the same checks to the tidwall/match and tidwall/pretty entries at go.mod lines 87–88.
🤖 Prompt for AI Agents
In go.mod around lines 34-35 (and also review lines 87-88 for the tidwall/match
and tidwall/pretty entries), update the module bookkeeping: confirm CI/workflows
use Go >= 1.23 (go 1.23.4 in go.mod requires the toolchain be >= that), run `go
mod tidy` to remove stale indirect deps, and ensure any replace/override logic
is consolidated; additionally, in relay/gemini_handler.go around lines 221-228
replace the bespoke manual Unmarshal + loop merge logic with a call to
relaycommon.ApplyParamOverride (pass the same parameters/contexts used
elsewhere) so the centralized override behavior is used consistently.
| // 尝试断言为操作格式 | ||
| if operations, ok := tryParseOperations(paramOverride); ok { | ||
| // 使用新方法 | ||
| result, err := applyOperations(string(jsonData), operations) | ||
| if err != nil { | ||
| // 新方法失败，回退到旧方法 | ||
| return applyOperationsLegacy(jsonData, paramOverride) | ||
| } | ||
| return []byte(result), nil | ||
| } | ||
|
|
||
| // 直接使用旧方法 | ||
| return applyOperationsLegacy(jsonData, paramOverride) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t fall back to legacy when operations are present; return an error instead
If an “operations” block is present but fails (parse or apply), falling back to legacy can silently inject the top-level "operations" key into the outbound request or mask user mistakes. This will likely cause upstream API errors with hard-to-trace behavior. Prefer failing fast when operations are intended but invalid.
Apply this diff:
- // 尝试断言为操作格式
- if operations, ok := tryParseOperations(paramOverride); ok {
- // 使用新方法
- result, err := applyOperations(string(jsonData), operations)
- if err != nil {
- // 新方法失败，回退到旧方法
- return applyOperationsLegacy(jsonData, paramOverride)
- }
- return []byte(result), nil
- }
-
- // 直接使用旧方法
- return applyOperationsLegacy(jsonData, paramOverride)
+ // 尝试断言为操作格式
+ if _, hasOps := paramOverride["operations"]; hasOps {
+ operations, ok := tryParseOperations(paramOverride)
+ if !ok {
+ return nil, fmt.Errorf("invalid param_override operations format")
+ }
+ result, err := applyOperations(string(jsonData), operations)
+ if err != nil {
+ return nil, fmt.Errorf("param_override operations failed: %w", err)
+ }
+ return []byte(result), nil
+ }
+ // 直接使用旧方法（仅在未提供 operations 时）
+ return applyOperationsLegacy(jsonData, paramOverride)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 尝试断言为操作格式 | |
| if operations, ok := tryParseOperations(paramOverride); ok { | |
| // 使用新方法 | |
| result, err := applyOperations(string(jsonData), operations) | |
| if err != nil { | |
| // 新方法失败，回退到旧方法 | |
| return applyOperationsLegacy(jsonData, paramOverride) | |
| } | |
| return []byte(result), nil | |
| } | |
| // 直接使用旧方法 | |
| return applyOperationsLegacy(jsonData, paramOverride) | |
| } | |
| // 尝试断言为操作格式 | |
| if _, hasOps := paramOverride["operations"]; hasOps { | |
| operations, ok := tryParseOperations(paramOverride) | |
| if !ok { | |
| return nil, fmt.Errorf("invalid param_override operations format") | |
| } | |
| result, err := applyOperations(string(jsonData), operations) | |
| if err != nil { | |
| return nil, fmt.Errorf("param_override operations failed: %w", err) | |
| } | |
| return []byte(result), nil | |
| } | |
| // 直接使用旧方法（仅在未提供 operations 时） | |
| return applyOperationsLegacy(jsonData, paramOverride) | |
| } |
🤖 Prompt for AI Agents
In relay/common/override.go around lines 33 to 46, the current logic falls back
to applyOperationsLegacy when an "operations" block is present but parsing or
new applyOperations fails; change this to fail fast instead: if
tryParseOperations(paramOverride) returns ok then call
applyOperations(string(jsonData), operations) and if it returns an error
propagate that error (wrap or return it) rather than calling
applyOperationsLegacy; return []byte(result) on success. Also ensure parsing
failures (ok == false) still follow the old path, but any parse success with
subsequent errors must not fallback and must return an explicit error with
context about "operations" handling.
…rams-override feats: the flexable params override and compatible format
…obile-double-render fix(frontend): reduce account usage request fan-out on pagination
2 participants
Summary by CodeRabbit
New Features
UI
Chores