feat: 支持腾讯知识引擎原子能力DeepSeek API 接口/阿里云qwen-image/wanx系列文生图和编辑图形支持, 补充通义万相系列视频生成模型,可以使用metadata透传一些视频模型特定参数 #1716
Conversation
WalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Gateway
participant AliAdaptor
participant Mode as ImageProcessMode
participant AliAPI
Client->>Gateway: POST /v1/images (multipart/JSON)
Gateway->>AliAdaptor: forward request + RelayInfo
AliAdaptor->>AliAdaptor: selectImageProcessMode(info)
alt Mode selected
AliAdaptor->>Mode: ProcessRequest(c, info, ImageRequest)
Mode->>AliAPI: POST Mode.Url (X-DashScope-Async if Mode.Async)
AliAPI-->>Mode: HTTP response
Mode->>AliAdaptor: ProcessResponse(...) -> standardized payload
else No mode
AliAdaptor->>AliAPI: POST default image endpoint
AliAPI-->>AliAdaptor: HTTP response
end
AliAdaptor->>Gateway: write response (MarshalWithoutHTMLEscape)
Gateway-->>Client: HTTP response
sequenceDiagram
autonumber
participant Client
participant Gateway
participant TencentAdaptor
participant Signer
participant TencentAPI
Client->>Gateway: Chat request
Gateway->>TencentAdaptor: RelayInfo + request
TencentAdaptor->>TencentAdaptor: detectDeepSeek -> set Version/ChannelBaseUrl
TencentAdaptor->>TencentAdaptor: determine regions from ChannelSettings.Regions
TencentAdaptor->>Signer: getTencentSign(req, adaptor, info, secId, secKey)
Signer->>Signer: parse info.ChannelBaseUrl -> host/service -> sign
Signer-->>TencentAdaptor: Authorization header or error
TencentAdaptor->>TencentAPI: send request with X-TC-Region + Authorization
TencentAPI-->>TencentAdaptor: response/stream
TencentAdaptor->>Gateway: streamResponseTencent2OpenAI(info, response)
Gateway-->>Client: streamed events (model from info.OriginModelName)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
relay/channel/tencent/adaptor.go (1)
66-73: Ensure Content-Type matches signature canonicalization
- In SetupRequestHeader (relay/channel/tencent/adaptor.go), add
req.Set("Content-Type", "application/json")
before setting other headers so the outbound header aligns with getTencentSign’s canonicalization.- In getTencentSign (relay/channel/tencent/relay-tencent.go), replace the hardcoded
"application/json"in canonicalHeaders with the actual header value (e.g.header.Get("Content-Type")) to support any charset parameters.relay/channel/tencent/relay-tencent.go (1)
200-206: Set Content-Type to application/json in Tencent SetupRequestHeader
In relay/channel/tencent/adaptor.go inside SetupRequestHeader, addreq.Set("Content-Type", "application/json")before signing so the actual header matches the hardcoded canonicalHeaders value.
types/error.go (1)
163-169: Fix incorrect errorCode becoming “” in NewOpenAIError.OpenAIError.Code isn’t set; WithOpenAIError stringifies it to “”, polluting errorCode.
Apply:
func NewOpenAIError(err error, errorCode ErrorCode, statusCode int) *NewAPIError { openaiError := OpenAIError{ Message: err.Error(), Type: string(errorCode), + Code: string(errorCode), } return WithOpenAIError(openaiError, statusCode) }relay/channel/ali/image.go (2)
61-66: Add HTTP client timeout and fix log textAvoid unbounded waits and correct the log label.
Apply:
- client := &http.Client{} + client := &http.Client{Timeout: 15 * time.Second}- common.SysError("updateTask NewDecoder err: " + err.Error()) + common.SysError("updateTask Unmarshal err: " + err.Error())Also applies to: 74-74
97-101: Polling bug: empty TaskStatus should not short-circuitReturning on empty status prematurely yields erroneous errors upstream. Keep polling.
Apply:
- if rsp.Output.TaskStatus == "" { - return &taskResponse, responseBody, nil - } + if rsp.Output.TaskStatus == "" { + continue + }
🧹 Nitpick comments (25)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.js (7)
39-40: Fix naming inconsistency: hiddenModeNames vs hiddenModelNames.This typo is confusing and risks future mistakes. Rename the state and all references to hiddenModelNames.
- const [hiddenModeNames, setHiddenModelNames] = useState([]); + const [hiddenModelNames, setHiddenModelNames] = useState([]); - const keepHiddenModels = (modelConf)=> { - return hiddenModeNames.reduce((newConf,hiddenModel)=> { + const keepHiddenModels = (modelConf)=> { + return hiddenModelNames.reduce((newConf,hiddenModel)=> { if(hiddenModel in modelConf){ newConf[hiddenModel] = modelConf[hiddenModel] } return newConf },{}) };Also applies to: 140-147
92-99: Align comment with logic (AND vs OR).Code shows only models missing both price and ratio; the comment says “价格或者倍率”. Adjust the comment to avoid confusion.
- // 找出所有未设置价格和倍率的模型 - // 如果模型没有价格或者没有倍率设置，则显示 + // 找出所有“同时没有”价格和倍率的模型
310-315: Deduplicate FX math and avoid magic number 2.Centralize conversion and base-price constants to one helper to reduce drift.
Example:
+ const PRICE_PER_M_TOKENS_USD = 2; // TODO: move to config or derive from backend + const toUSD = (v) => currencyType === 'CNY' ? v / USDExchangeRate : v; + const fromUSD = (v) => currencyType === 'CNY' ? v * USDExchangeRate : v; const calculateRatioFromTokenPrice = (tokenPrice) => { - if (currencyType === "CNY"){ - tokenPrice = tokenPrice / USDExchangeRate - } - return tokenPrice / 2; + return toUSD(tokenPrice) / PRICE_PER_M_TOKENS_USD; }; const calculateTokenPriceFromRatio = (ratio) => { - if (currencyType === "CNY"){ - ratio = ratio * USDExchangeRate - } - return ratio * 2; + return fromUSD(ratio * PRICE_PER_M_TOKENS_USD); };Also applies to: 419-424
437-449: Remove unused variable and NOP setter.intiialCurrucyType is unused; setCurrencyType(currencyType) is a no-op. Clean up.
- let intiialCurrucyType = 'USD' ... - setCurrencyType(currencyType)
473-497: Rename handleCurrucyTypeChange and coerce numeric types.Fix spelling; ensure numeric coercion to avoid string math in InputNumber fields.
- const handleCurrucyTypeChange = (value) => { + const handleCurrencyTypeChange = (value) => { let newCurrencyType = value let oldCurrencyType = currencyType if(newCurrencyType === oldCurrencyType){ return } - let updatedModel = {... currentModel} + let updatedModel = {...(currentModel || {})} if(newCurrencyType === "USD" && updatedModel.tokenPrice){ - updatedModel.tokenPrice = updatedModel.tokenPrice / USDExchangeRate + updatedModel.tokenPrice = Number(updatedModel.tokenPrice) / USDExchangeRate }else if(updatedModel.tokenPrice) { - updatedModel.tokenPrice = updatedModel.tokenPrice * USDExchangeRate + updatedModel.tokenPrice = Number(updatedModel.tokenPrice) * USDExchangeRate } if(newCurrencyType === "USD" && updatedModel.completionTokenPrice){ - updatedModel.completionTokenPrice = updatedModel.completionTokenPrice / USDExchangeRate + updatedModel.completionTokenPrice = Number(updatedModel.completionTokenPrice) / USDExchangeRate }else if(updatedModel.completionTokenPrice) { - updatedModel.completionTokenPrice = updatedModel.completionTokenPrice * USDExchangeRate + updatedModel.completionTokenPrice = Number(updatedModel.completionTokenPrice) * USDExchangeRate } if(newCurrencyType === "USD" && updatedModel.price){ - updatedModel.price = updatedModel.price / USDExchangeRate + updatedModel.price = Number(updatedModel.price) / USDExchangeRate }else if(updatedModel.price){ - updatedModel.price = updatedModel.price * USDExchangeRate + updatedModel.price = Number(updatedModel.price) * USDExchangeRate } setCurrencyType(newCurrencyType) setCurrentModel(updatedModel) }- <Select field="currencyType" value={currencyType} onChange={handleCurrucyTypeChange}> + <Select field="currencyType" value={currencyType} onChange={handleCurrencyTypeChange}>Also applies to: 743-747, 778-781
455-463: Avoid storing numeric fields as strings.InputNumber prefers numbers. Drop toString conversions to keep state numeric.
- ).toString(); + ); ... - ).toString(); + );Also applies to: 681-689
23-23: Remove unused helper.getQuotaPerUnit and quotaPerUnit are unused.
- import { API, showError, showSuccess, getQuotaPerUnit } from '../../../helpers'; + import { API, showError, showSuccess } from '../../../helpers'; ... - const quotaPerUnit = getQuotaPerUnit(); + // removed unused quotaPerUnitAlso applies to: 43-43
dto/channel_settings.go (1)
4-7: Make Regions optional and clarify semanticsTo avoid emitting null/empty arrays in JSON and to keep consistency with other optional fields, consider marking Regions as omitempty. Also document expected region values (e.g., ap-guangzhou, ap-shanghai) and whether order matters.
- Regions []string `json:"regions"` + Regions []string `json:"regions,omitempty"`Would you like me to add basic validation helpers for region strings?
relay/channel/tencent/dto.go (1)
57-71: Field name mismatch: Messages tag is "Message"Struct field is named Messages but marshals as "Message". Works due to the tag, but is confusing and prone to mistakes (see usage in response conversion). Rename for clarity and update references.
type TencentResponseChoices struct { FinishReason string `json:"FinishReason,omitempty"` // 流式结束标志位，为 stop 则表示尾包 - Messages TencentMessage `json:"Message,omitempty"` // 内容，同步模式返回内容，流模式为 null 输出 content 内容总数最多支持 1024token。 + Message TencentMessage `json:"Message,omitempty"` // 内容，同步模式返回内容，流模式为 null 输出 content 内容总数最多支持 1024token。 Delta TencentMessage `json:"Delta,omitempty"` // 内容，流模式返回内容，同步模式为 null 输出 content 内容总数最多支持 1024token。 } @@ - Content: response.Choices[0].Messages.Content, + Content: response.Choices[0].Message.Content,relay/channel/tencent/adaptor.go (4)
45-47: Make deepseek detection case-insensitiveModel names can vary in case. Lowercasing avoids false negatives.
-func isDeepseekModel(modelName string) bool { - return strings.Contains(modelName, "deepseek") +func isDeepseekModel(modelName string) bool { + return strings.Contains(strings.ToLower(modelName), "deepseek") }
51-58: DeepSeek base URL/version override: confirm defaultsChanging Version to 2024-05-22 and switching base to lkeap when using the default base URL seems right for DeepSeek, but this is brittle if BaseUrl has a trailing slash or minor variations.
- if info.BaseUrl == constant.ChannelBaseURLs[info.ChannelType] { + if strings.TrimRight(info.BaseUrl, "/") == strings.TrimRight(constant.ChannelBaseURLs[info.ChannelType], "/") { info.BaseUrl = "https://lkeap.tencentcloudapi.com" }Would you like me to switch to host-based comparison to be fully robust?
72-83: Region selection should be stable and properly randomUsing math/rand without a seed yields deterministic choices across runs. Also, picking a random region per request harms retry idempotency. Prefer a stable hash-based choice, or at least crypto/rand.
-import "math/rand" +import ( + "crypto/rand" + "math/big" +) @@ -} else if len(regions) > 1 { - selectReg := rand.Intn(len(regions)) - req.Set("X-TC-Region", regions[selectReg]) +} else if len(regions) > 1 { + n, _ := rand.Int(rand.Reader, big.NewInt(int64(len(regions)))) + req.Set("X-TC-Region", regions[int(n.Int64())]) }If you want stable selection per user/request, I can wire in a hash of TokenId/ReqID instead.
88-106: AppID parsed but unused: confirm if requiredAppID is parsed and stored (a.AppID) but never sent. Some Tencent services require it in headers or body. Verify DeepSeek/Hunyuan expectations.
I can add X-TC-AppId or body field wiring if needed once confirmed by docs.
relay/channel/tencent/relay-tencent.go (3)
93-101: Scanner default buffer may truncate long SSE linesbufio.Scanner caps tokens at 64K. Long increments can exceed this.
scanner := bufio.NewScanner(resp.Body) -scanner.Split(bufio.ScanLines) +scanner.Buffer(make([]byte, 0, 64*1024), 2*1024*1024) +scanner.Split(bufio.ScanLines)
187-196: Signing service derivation from BaseUrl: good; add clearer errorParsing BaseUrl can fail; bubbling raw error is fine, but adding context will speed up ops/debug.
- if pu, err := url.Parse(info.BaseUrl); err != nil { - return "", err + if pu, err := url.Parse(info.BaseUrl); err != nil { + return "", fmt.Errorf("parse BaseUrl for signing failed: %w", err) } else { host = pu.Host }
227-233: Avoid shadowing and binary-string handling in HMAC helpersUsing string for binary MACs and shadowing secretKey is confusing. Prefer []byte helpers and clearer names.
-func hmacSha256(s, key string) string { - hashed := hmac.New(sha256.New, []byte(key)) - hashed.Write([]byte(s)) - return string(hashed.Sum(nil)) +func hmacSha256Bytes(msg []byte, key []byte) []byte { + mac := hmac.New(sha256.New, key) + mac.Write(msg) + return mac.Sum(nil) } @@ -secretDate := hmacSha256(date, "TC3"+secKey) -secretService := hmacSha256(serviceName, secretDate) -secretKey := hmacSha256("tc3_request", secretService) -signature := hex.EncodeToString([]byte(hmacSha256(string2sign, secretKey))) +secretDate := hmacSha256Bytes([]byte(date), []byte("TC3"+secKey)) +secretService := hmacSha256Bytes([]byte(serviceName), secretDate) +signingKey := hmacSha256Bytes([]byte("tc3_request"), secretService) +signature := hex.EncodeToString(hmacSha256Bytes([]byte(string2sign), signingKey))relay/image_handler.go (1)
204-206: Don’t log raw request bodies.Current format prints garbage for non-strings and risks large/secret payloads in logs.
Apply:
- if common.DebugEnabled { - println(fmt.Sprintf("image request body: %s", requestBody)) - } + if common.DebugEnabled { + println(fmt.Sprintf("image request body type: %T", requestBody)) + }relay/channel/ali/image.go (2)
21-32: Typos and mapping sanity check for mode selection
- Rename multimoalGenerationMode → multimodalGenerationMode.
- Verify whether qwen-image-edit should route to image2ImageMode instead of multimodal generation.
Apply rename:
- case "qwen-image-edit": - return multimoalGenerationMode() + case "qwen-image-edit": + return multimodalGenerationMode()
184-193: Return 200 for final result and handle write errorsUse 200 for the synthesized final response and check write errors.
Apply:
- c.Writer.Header().Set("Content-Type", "application/json") - c.Writer.WriteHeader(resp.StatusCode) - c.Writer.Write(jsonResponse) - return nil, &dto.Usage{} + c.Writer.Header().Set("Content-Type", "application/json") + c.Writer.WriteHeader(http.StatusOK) + if _, werr := c.Writer.Write(jsonResponse); werr != nil { + return types.NewError(werr, types.ErrorCodeBadResponseBody), nil + } + return nil, &dto.Usage{}relay/channel/ali/image_process_mode.go (5)
87-112: Close files per-iteration to avoid FD buildupAvoid deferring Close inside loops.
Apply:
- defer file.Close() // 读取文件内容 - fileBytes, err := io.ReadAll(file) + fileBytes, err := io.ReadAll(file) + _ = file.Close()
160-199: Rename multimoal → multimodal for consistencyTypos hinder discoverability.
Apply:
-func multimoalGenerationMode() *ImageProcessMode { +func multimodalGenerationMode() *ImageProcessMode {And update callers (selectImageProcessMode in image.go).
199-257: Handle write errors in ProcessResponseMirror aliImageHandler’s write checks.
Apply:
- c.Writer.Header().Set("Content-Type", "application/json") - c.Writer.WriteHeader(resp.StatusCode) - c.Writer.Write(jsonResponse) + c.Writer.Header().Set("Content-Type", "application/json") + c.Writer.WriteHeader(resp.StatusCode) + if _, werr := c.Writer.Write(jsonResponse); werr != nil { + return types.NewError(werr, types.ErrorCodeBadResponseBody), nil + }
265-341: Image2Image: missing size in non-sketch branchesParameters.Size is omitted for mask and description_edit requests; upstream may default unexpectedly.
Apply:
Parameters: struct { N int `json:"n,omitempty"` Size string `json:"size,omitempty"` }{ - N: request.N, + N: request.N, + Size: strings.Replace(request.Size, "x", "*", -1), },Apply to both the mask branch (exists) and the final else branch.
41-55: Dead code: indexOfAnySubStr is unusedSafe to remove unless used elsewhere in package ali.
relay/channel/ali/dto.go (1)
146-169: Struct tag typos (omitemtpy) → omitemptyFix JSON tags.
Apply:
- Code string `json:"code,omitemtpy"` - Message string `json:"message,omitemtpy"` + Code string `json:"code,omitempty"` + Message string `json:"message,omitempty"`
📜 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 selected for processing (13)
dto/channel_settings.go(1 hunks)relay/channel/ali/adaptor.go(6 hunks)relay/channel/ali/dto.go(2 hunks)relay/channel/ali/image.go(5 hunks)relay/channel/ali/image_process_mode.go(1 hunks)relay/channel/tencent/adaptor.go(4 hunks)relay/channel/tencent/dto.go(2 hunks)relay/channel/tencent/relay-tencent.go(8 hunks)relay/image_handler.go(1 hunks)types/error.go(1 hunks)web/src/components/settings/RatioSetting.js(2 hunks)web/src/pages/Setting/Ratio/ModelRationNotSetEditor.js(0 hunks)web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.js(18 hunks)
💤 Files with no reviewable changes (1)
- web/src/pages/Setting/Ratio/ModelRationNotSetEditor.js
🧰 Additional context used
🧬 Code graph analysis (11)
relay/image_handler.go (1)
relay/constant/relay_mode.go (1)
RelayModeImagesEdits(15-15)
relay/channel/ali/image_process_mode.go (5)
common/logger.go (1)
SysError(55-58)relay/common/relay_info.go (1)
RelayInfo(62-112)types/error.go (5)
NewAPIError(75-81)NewError(153-161)ErrorCodeReadResponseBodyFailed(61-61)ErrorCodeBadResponseBody(64-64)WithOpenAIError(181-193)relay/channel/ali/dto.go (10)
AliImageMessageItem(8-10)AliTextMessageItem(11-13)AliInput(26-30)AliMessage(21-24)AliMultimodelGenerationResponse(146-169)TaskResult(81-86)AliResponse(98-102)AliOutput(88-96)AliUsage(75-79)AliImage2ImageImageSynthesisRequest(171-184)common/http.go (1)
CloseResponseBodyGracefully(12-20)
relay/channel/tencent/relay-tencent.go (2)
relay/common/relay_info.go (1)
RelayInfo(62-112)relay/channel/tencent/dto.go (2)
TencentChatResponse(63-72)TencentChatRequest(8-44)
web/src/components/settings/RatioSetting.js (3)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.js (1)
ModelSettingsVisualEditor(26-803)web/src/components/settings/OperationSetting.js (1)
inputs(11-44)web/src/pages/Setting/Ratio/ModelRatioSettings.js (1)
inputs(23-29)
relay/channel/tencent/dto.go (2)
dto/openai_response.go (1)
Usage(186-199)relay/channel/baidu/dto.go (1)
Error(26-29)
relay/channel/tencent/adaptor.go (3)
relay/channel/adapter.go (1)
Adaptor(13-29)relay/common/relay_info.go (1)
RelayInfo(62-112)constant/channel.go (1)
ChannelBaseURLs(56-109)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.js (2)
web/src/helpers/render.js (1)
getQuotaPerUnit(837-841)web/src/helpers/utils.js (1)
showError(94-123)
types/error.go (2)
dto/error.go (1)
OpenAIError(5-10)dto/claude.go (1)
ClaudeError(287-290)
relay/channel/ali/dto.go (2)
dto/openai_request.go (1)
Message(114-125)dto/openai_response.go (1)
Usage(186-199)
relay/channel/ali/adaptor.go (2)
relay/channel/ali/image_process_mode.go (1)
ImageProcessMode(147-152)relay/constant/relay_mode.go (2)
RelayModeImagesGenerations(14-14)RelayModeImagesEdits(15-15)
relay/channel/ali/image.go (6)
relay/common/relay_info.go (1)
RelayInfo(62-112)relay/channel/ali/image_process_mode.go (1)
ImageProcessMode(147-152)relay/channel/ali/adaptor.go (1)
Adaptor(18-20)relay/channel/adapter.go (1)
Adaptor(13-29)dto/dalle.go (1)
ImageRequest(5-19)relay/channel/ali/dto.go (1)
AliImageRequest(104-117)
🔇 Additional comments (18)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.js (3)
13-15: UI imports look good.
499-505: Form sync approach is fine.The ref-driven setValues keeps UI consistent across currency/mode changes.
617-619: Good: normalize per-request price to USD before save.web/src/components/settings/RatioSetting.js (2)
26-26: USDExchangeRate plumbed into options state.Works with the editor’s parseFloat usage; no further changes needed here.
105-109: Switch to the unified editor for unset models looks good.Pairs well with mode="unset_models". Ensure the ModelSettingsVisualEditor dependency fix lands so this tab reflects enabled models correctly.
Would you like me to open a small PR to include enabledModelNames in the editor’s effect dependencies? -->
relay/channel/tencent/dto.go (1)
46-49: Drop Code-type verification for OpenAIError
OpenAIError struct defines only Message and Type fields; Code exists only on TencentError, so no downstream checks needed.Likely an incorrect or invalid review comment.
relay/channel/tencent/relay-tencent.go (2)
76-86: Model in stream chunks sourced from OriginModelNameGood call to reflect the mapped model. Ensure OriginModelName is always set in RelayInfo, including non-mapped cases, to avoid empty model in SSE.
147-151: Error pointer handling LGTMSwitching to pointer-based Error and nil checks aligns with the DTO changes and prevents false positives.
types/error.go (1)
135-151: Harden ToClaudeError: nil receiver + safe type-switch to avoid panics.Unconditional assertions on RelayError can panic; also guard e == nil for consistency with Error().
[ suggest_essential_refactor ]
Apply:func (e *NewAPIError) ToClaudeError() ClaudeError { - switch e.ErrorType { - case ErrorTypeOpenAIError: - openAIError := e.RelayError.(OpenAIError) - return ClaudeError{ - Message: e.Error(), - Type: fmt.Sprintf("%v", openAIError.Code), - } - case ErrorTypeClaudeError: - return e.RelayError.(ClaudeError) - default: - return ClaudeError{ - Message: e.Error(), - Type: string(e.ErrorType), - } - } + if e == nil { + return ClaudeError{Message: "", Type: string(ErrorTypeNewAPIError)} + } + switch e.ErrorType { + case ErrorTypeOpenAIError: + switch v := e.RelayError.(type) { + case OpenAIError: + return ClaudeError{Message: e.Error(), Type: fmt.Sprintf("%v", v.Code)} + case *OpenAIError: + return ClaudeError{Message: e.Error(), Type: fmt.Sprintf("%v", v.Code)} + default: + return ClaudeError{Message: e.Error(), Type: string(e.ErrorType)} + } + case ErrorTypeClaudeError: + switch v := e.RelayError.(type) { + case ClaudeError: + return v + case *ClaudeError: + return *v + default: + return ClaudeError{Message: e.Error(), Type: string(e.ErrorType)} + } + default: + return ClaudeError{Message: e.Error(), Type: string(e.ErrorType)} + } }relay/channel/ali/adaptor.go (5)
39-47: URL selection by mode: LGTM.Correctly routes ImagesGenerations/Edits to mode URL with fallback.
66-68: Async header: LGTM.Conditional X-DashScope-Async based on mode is appropriate.
90-95: Image request conversion: LGTM.Delegates to mode-aware converter with error propagation.
121-124: Response routing for images: LGTM.Unified handling for generations/edits aligns with mode-driven flow.
19-20: Stateful adaptor safe: new instance per call
GetAdaptor constructs and returns a fresh adaptor each invocation (no shared singleton), soimageProcessModelives on per-request instances and poses no data-race risk.relay/channel/ali/image.go (2)
34-46: LGTM: pluggable request path with safe fallbackThe delegation to mode.ProcessRequest with a clean fallback looks good.
197-211: LGTM: no-escape JSON encoderThe helper cleanly avoids HTML escaping and trims the trailing newline.
relay/channel/ali/dto.go (2)
7-19: LGTM: new content item typesThe added message item structs align with multimodal payloads.
21-24: Call sites validated—no action needed.Search for
AliMessagefound only its construction inimage_process_mode.go, wherecontentis astring, and no code asserts or manipulates non-string types forContent. The widenedanytype poses no issues.
|
|
||
| func (a *Adaptor) SetupRequestHeader(c *gin.Context, req *http.Header, info *relaycommon.RelayInfo) error { | ||
| channel.SetupApiRequestHeader(info, c, req) | ||
| req.Set("Content-Type", "application/json") |
There was a problem hiding this comment.
Don’t force application/json; allow mode/body to dictate Content-Type.
Unconditional JSON header breaks multipart/binary image edit requests.
Apply:
- req.Set("Content-Type", "application/json")
+ // Prefer Content-Type provided by the mode/request builder via context.
+ if ct := c.GetString("content-type"); ct != "" {
+ req.Set("Content-Type", ct)
+ } else if !(info.RelayMode == constant.RelayModeImagesEdits && a.imageProcessMode != nil) {
+ // Safe default for JSON-based requests.
+ req.Set("Content-Type", "application/json")
+ }Follow-up: have the image mode’s ProcessRequest set c.Set("content-type", value) when it builds multipart bodies.
🤖 Prompt for AI Agents
In relay/channel/ali/adaptor.go around line 58, remove the unconditional
req.Set("Content-Type", "application/json") so the code does not override the
request's Content-Type; instead, let the request body builder or mode set the
header. Update callers or the mode implementations so that when building
multipart or binary bodies (e.g., image mode), their ProcessRequest function
sets c.Set("content-type", <appropriate value>) on the request before sending.
| func getFormMarkImage(c *gin.Context) (string, bool) { | ||
| var imageFiles []*multipart.FileHeader | ||
| var exists bool | ||
| if imageFiles, exists = c.Request.MultipartForm.File["mask"]; !exists || len(imageFiles) == 0 { | ||
| for i, fileHeader := range imageFiles { | ||
| file, err := fileHeader.Open() | ||
| if err != nil { | ||
| common.SysError(fmt.Sprintf("failed to open image file %d: %v", i, err)) | ||
| continue | ||
| } | ||
| defer file.Close() | ||
|
|
||
| // 读取文件内容 | ||
| fileBytes, err := io.ReadAll(file) | ||
| if err != nil { | ||
| common.SysError(fmt.Sprintf("failed to read image file %d: %v", i, err)) | ||
| continue | ||
| } | ||
|
|
||
| // 确定MIME类型 | ||
| mimeType := detectImageMimeType(fileHeader.Filename) | ||
|
|
||
| // 转换为base64 | ||
| base64Data := base64.StdEncoding.EncodeToString(fileBytes) | ||
| dataURL := fmt.Sprintf("data:%s;base64,%s", mimeType, base64Data) | ||
| return dataURL, true | ||
| } | ||
| } | ||
| return "", false | ||
| } | ||
|
|
There was a problem hiding this comment.
Mask extraction bug: logic inverted; mask never read
The condition ensures imageFiles is empty, then iterates it. As written, mask is never returned.
Apply:
-func getFormMarkImage(c *gin.Context) (string, bool) {
- var imageFiles []*multipart.FileHeader
- var exists bool
- if imageFiles, exists = c.Request.MultipartForm.File["mask"]; !exists || len(imageFiles) == 0 {
- for i, fileHeader := range imageFiles {
+func getFormMarkImage(c *gin.Context) (string, bool) {
+ imageFiles, exists := c.Request.MultipartForm.File["mask"]
+ if !exists || len(imageFiles) == 0 {
+ return "", false
+ }
+ for i, fileHeader := range imageFiles {
file, err := fileHeader.Open()
if err != nil {
common.SysError(fmt.Sprintf("failed to open image file %d: %v", i, err))
continue
}
- defer file.Close()
+ // close promptly after read
fileBytes, err := io.ReadAll(file)
+ _ = file.Close()
if err != nil {
common.SysError(fmt.Sprintf("failed to read image file %d: %v", i, err))
continue
}
mimeType := detectImageMimeType(fileHeader.Filename)
base64Data := base64.StdEncoding.EncodeToString(fileBytes)
dataURL := fmt.Sprintf("data:%s;base64,%s", mimeType, base64Data)
return dataURL, true
- }
- }
- return "", false
+ }
+ return "", false
}📝 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.
| func getFormMarkImage(c *gin.Context) (string, bool) { | |
| var imageFiles []*multipart.FileHeader | |
| var exists bool | |
| if imageFiles, exists = c.Request.MultipartForm.File["mask"]; !exists || len(imageFiles) == 0 { | |
| for i, fileHeader := range imageFiles { | |
| file, err := fileHeader.Open() | |
| if err != nil { | |
| common.SysError(fmt.Sprintf("failed to open image file %d: %v", i, err)) | |
| continue | |
| } | |
| defer file.Close() | |
| // 读取文件内容 | |
| fileBytes, err := io.ReadAll(file) | |
| if err != nil { | |
| common.SysError(fmt.Sprintf("failed to read image file %d: %v", i, err)) | |
| continue | |
| } | |
| // 确定MIME类型 | |
| mimeType := detectImageMimeType(fileHeader.Filename) | |
| // 转换为base64 | |
| base64Data := base64.StdEncoding.EncodeToString(fileBytes) | |
| dataURL := fmt.Sprintf("data:%s;base64,%s", mimeType, base64Data) | |
| return dataURL, true | |
| } | |
| } | |
| return "", false | |
| } | |
| func getFormMarkImage(c *gin.Context) (string, bool) { | |
| // Retrieve the uploaded "mask" files | |
| imageFiles, exists := c.Request.MultipartForm.File["mask"] | |
| if !exists || len(imageFiles) == 0 { | |
| return "", false | |
| } | |
| for i, fileHeader := range imageFiles { | |
| file, err := fileHeader.Open() | |
| if err != nil { | |
| common.SysError(fmt.Sprintf("failed to open image file %d: %v", i, err)) | |
| continue | |
| } | |
| // Read file contents then close promptly | |
| fileBytes, err := io.ReadAll(file) | |
| _ = file.Close() | |
| if err != nil { | |
| common.SysError(fmt.Sprintf("failed to read image file %d: %v", i, err)) | |
| continue | |
| } | |
| // Determine MIME type and encode as Data URL | |
| mimeType := detectImageMimeType(fileHeader.Filename) | |
| base64Data := base64.StdEncoding.EncodeToString(fileBytes) | |
| dataURL := fmt.Sprintf("data:%s;base64,%s", mimeType, base64Data) | |
| return dataURL, true | |
| } | |
| return "", false | |
| } |
types/error.go
Outdated
| func (e *NewAPIError) ToOpenAIError() OpenAIError { | ||
| switch e.ErrorType { | ||
| case ErrorTypeOpenAIError: | ||
| return e.RelayError.(OpenAIError) | ||
| case ErrorTypeClaudeError: | ||
| claudeError := e.RelayError.(ClaudeError) | ||
| return OpenAIError{ | ||
| Message: e.Error(), | ||
| Type: claudeError.Type, | ||
| Param: "", | ||
| Code: e.errorCode, | ||
| } | ||
| default: | ||
| return OpenAIError{ | ||
| Message: e.Error(), | ||
| Type: string(e.ErrorType), | ||
| Param: "", | ||
| Code: e.errorCode, | ||
| if e.RelayError != nil { | ||
| switch e.ErrorType { | ||
| case ErrorTypeOpenAIError: | ||
| return e.RelayError.(OpenAIError) | ||
| case ErrorTypeClaudeError: | ||
| claudeError := e.RelayError.(ClaudeError) | ||
| return OpenAIError{ | ||
| Message: e.Error(), | ||
| Type: claudeError.Type, | ||
| Param: "", | ||
| Code: e.errorCode, | ||
| } | ||
| default: | ||
| return OpenAIError{ | ||
| Message: e.Error(), | ||
| Type: string(e.ErrorType), | ||
| Param: "", | ||
| Code: e.errorCode, | ||
| } | ||
| } | ||
| } | ||
| return OpenAIError{ | ||
| Message: e.Error(), | ||
| Type: string(e.ErrorType), | ||
| Param: "", | ||
| Code: e.errorCode, | ||
| } |
There was a problem hiding this comment.
Prevent panics in ToOpenAIError: add nil receiver guard and safe type-switch (support pointers).
Current code still uses unsafe type assertions when RelayError is non-nil; a mismatched or pointer-valued RelayError will panic. Also, ToOpenAIError doesn’t guard e == nil.
Apply:
func (e *NewAPIError) ToOpenAIError() OpenAIError {
- if e.RelayError != nil {
+ if e == nil {
+ return OpenAIError{
+ Message: "",
+ Type: string(ErrorTypeNewAPIError),
+ Param: "",
+ Code: "",
+ }
+ }
+ if e.RelayError != nil {
- switch e.ErrorType {
- case ErrorTypeOpenAIError:
- return e.RelayError.(OpenAIError)
- case ErrorTypeClaudeError:
- claudeError := e.RelayError.(ClaudeError)
- return OpenAIError{
- Message: e.Error(),
- Type: claudeError.Type,
- Param: "",
- Code: e.errorCode,
- }
- default:
- return OpenAIError{
- Message: e.Error(),
- Type: string(e.ErrorType),
- Param: "",
- Code: e.errorCode,
- }
- }
+ switch e.ErrorType {
+ case ErrorTypeOpenAIError:
+ switch v := e.RelayError.(type) {
+ case OpenAIError:
+ return v
+ case *OpenAIError:
+ return *v
+ }
+ case ErrorTypeClaudeError:
+ switch v := e.RelayError.(type) {
+ case ClaudeError:
+ return OpenAIError{Message: e.Error(), Type: v.Type, Param: "", Code: e.errorCode}
+ case *ClaudeError:
+ return OpenAIError{Message: e.Error(), Type: v.Type, Param: "", Code: e.errorCode}
+ }
+ }
+ // fallback for unexpected RelayError types
+ return OpenAIError{Message: e.Error(), Type: string(e.ErrorType), Param: "", Code: e.errorCode}
}
- return OpenAIError{
- Message: e.Error(),
- Type: string(e.ErrorType),
- Param: "",
- Code: e.errorCode,
- }
+ return OpenAIError{Message: e.Error(), Type: string(e.ErrorType), Param: "", Code: e.errorCode}
}📝 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.
| func (e *NewAPIError) ToOpenAIError() OpenAIError { | |
| switch e.ErrorType { | |
| case ErrorTypeOpenAIError: | |
| return e.RelayError.(OpenAIError) | |
| case ErrorTypeClaudeError: | |
| claudeError := e.RelayError.(ClaudeError) | |
| return OpenAIError{ | |
| Message: e.Error(), | |
| Type: claudeError.Type, | |
| Param: "", | |
| Code: e.errorCode, | |
| } | |
| default: | |
| return OpenAIError{ | |
| Message: e.Error(), | |
| Type: string(e.ErrorType), | |
| Param: "", | |
| Code: e.errorCode, | |
| if e.RelayError != nil { | |
| switch e.ErrorType { | |
| case ErrorTypeOpenAIError: | |
| return e.RelayError.(OpenAIError) | |
| case ErrorTypeClaudeError: | |
| claudeError := e.RelayError.(ClaudeError) | |
| return OpenAIError{ | |
| Message: e.Error(), | |
| Type: claudeError.Type, | |
| Param: "", | |
| Code: e.errorCode, | |
| } | |
| default: | |
| return OpenAIError{ | |
| Message: e.Error(), | |
| Type: string(e.ErrorType), | |
| Param: "", | |
| Code: e.errorCode, | |
| } | |
| } | |
| } | |
| return OpenAIError{ | |
| Message: e.Error(), | |
| Type: string(e.ErrorType), | |
| Param: "", | |
| Code: e.errorCode, | |
| } | |
| func (e *NewAPIError) ToOpenAIError() OpenAIError { | |
| // nil-receiver guard | |
| if e == nil { | |
| return OpenAIError{ | |
| Message: "", | |
| Type: string(ErrorTypeNewAPIError), | |
| Param: "", | |
| Code: "", | |
| } | |
| } | |
| // if there’s a RelayError, try safe type-switching | |
| if e.RelayError != nil { | |
| switch e.ErrorType { | |
| case ErrorTypeOpenAIError: | |
| switch v := e.RelayError.(type) { | |
| case OpenAIError: | |
| return v | |
| case *OpenAIError: | |
| return *v | |
| } | |
| case ErrorTypeClaudeError: | |
| switch v := e.RelayError.(type) { | |
| case ClaudeError: | |
| return OpenAIError{Message: e.Error(), Type: v.Type, Param: "", Code: e.errorCode} | |
| case *ClaudeError: | |
| return OpenAIError{Message: e.Error(), Type: v.Type, Param: "", Code: e.errorCode} | |
| } | |
| } | |
| // fallback for any other/mismatched RelayError types | |
| return OpenAIError{Message: e.Error(), Type: string(e.ErrorType), Param: "", Code: e.errorCode} | |
| } | |
| // default when RelayError is nil | |
| return OpenAIError{Message: e.Error(), Type: string(e.ErrorType), Param: "", Code: e.errorCode} | |
| } |
🤖 Prompt for AI Agents
In types/error.go around lines 105 to 132, ToOpenAIError lacks a nil receiver
guard and uses unsafe type assertions that can panic for pointer-valued or
mismatched RelayError; add an early if e == nil { return OpenAIError{Message:
"nil error", Type: "", Param: "", Code: 0} } (or similar safe default) and
replace the direct assertions with a safe type switch or type assertions with ok
checks that handle both value and pointer forms (e.g., switch v :=
e.RelayError.(type) { case OpenAIError: ... case *OpenAIError: ... case
ClaudeError: ... case *ClaudeError: ... default: ... }) so the function never
panics and always returns a well-formed OpenAIError.
| useEffect(() => { | ||
| // 获取所有启用的模型 | ||
| flushAllEnabledModels(); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
Unset-models list won’t update after enabled model names load (stale state).
models are computed before enabledModelNames arrives, and the effect doesn’t re-run on its change. Add enabledModelNames to the dependency array.
- }, [props.options,props.mode]);
+ }, [props.options, props.mode, enabledModelNames]);Also applies to: 91-109, 117-117
🤖 Prompt for AI Agents
In web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.js around lines 60-64
(and similarly for lines 91-109 and 117), the effects compute the unset-models
list before enabledModelNames is available and use an empty dependency array,
causing stale state; update those useEffect dependency arrays to include
enabledModelNames (and any other values used inside, like flushAllEnabledModels
or model list getters) so the effects re-run when enabledModelNames changes, and
ensure the logic that computes/reset the unset-models is invoked inside those
effects when enabledModelNames updates.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
relay/channel/tencent/relay-tencent.go (2)
187-241: Fix TC3 signing: use URL.Hostname(), harden serviceName, and handle marshal errorsUsing
pu.Hostmay include a port (e.g.,:443), breaking both thehostcanonical header andserviceNamederivation. Also, the request payload marshal error is ignored, which can silently produce an invalid signature.Apply:
func getTencentSign(req TencentChatRequest, adaptor *Adaptor, info *relaycommon.RelayInfo, secId, secKey string) (string, error) { - host := "" - // build canonical request string - if pu, err := url.Parse(info.ChannelBaseUrl); err != nil { - return "", err - } else { - host = pu.Host - } + // build canonical request string + pu, err := url.Parse(info.ChannelBaseUrl) + if err != nil { + return "", fmt.Errorf("invalid ChannelBaseUrl %q: %w", info.ChannelBaseUrl, err) + } + host := pu.Hostname() + if host == "" { + return "", fmt.Errorf("empty host parsed from ChannelBaseUrl %q", info.ChannelBaseUrl) + } - serviceName := strings.SplitN(host, ".", 2)[0] + serviceName := strings.Split(host, ".")[0] httpRequestMethod := "POST" @@ - payload, _ := json.Marshal(req) + payload, err := json.Marshal(req) + if err != nil { + return "", fmt.Errorf("marshal request failed: %w", err) + } @@ - secretDate := hmacSha256(date, "TC3"+secKey) + secretDate := hmacSha256(date, "TC3"+secKey) secretService := hmacSha256(serviceName, secretDate)
95-101: Prevent SSE scan failures on large chunks (bufio.Scanner 64KB limit)Default
bufio.Scannertoken limit (64KB) risksErrTooLongon long SSE lines. Increase the max buffer.scanner := bufio.NewScanner(resp.Body) scanner.Split(bufio.ScanLines) + // Allow large SSE frames + scanner.Buffer(make([]byte, 0, 64*1024), 2*1024*1024)relay/channel/ali/image.go (3)
125-146: File handle leak on early-return pathsimage.Close() is only called on the success path. On read/encode errors the file remains open.
for _, file := range imageFiles { image, err := file.Open() if err != nil { return nil, errors.New("failed to open image file") } + // Ensure the file is closed on all paths in this iteration. + func() { + defer image.Close() - // 读取文件内容 - imageData, err := io.ReadAll(image) - if err != nil { - return nil, errors.New("failed to read image file") - } + // 读取文件内容 + imageData, err := io.ReadAll(image) + if err != nil { + // bubble up after defer closes the file + panic(err) + } - // 获取MIME类型 - mimeType := http.DetectContentType(imageData) + // 获取MIME类型 + mimeType := http.DetectContentType(imageData) - // 编码为base64 - base64Data := base64.StdEncoding.EncodeToString(imageData) + // 编码为base64 + base64Data := base64.StdEncoding.EncodeToString(imageData) - // 构造data URL格式 - dataURL := fmt.Sprintf("data:%s;base64,%s", mimeType, base64Data) - imageBase64s = append(imageBase64s, dataURL) - image.Close() + // 构造data URL格式 + dataURL := fmt.Sprintf("data:%s;base64,%s", mimeType, base64Data) + imageBase64s = append(imageBase64s, dataURL) + }() }If you prefer not to use panic inside the loop, return the error and let the caller handle; key point is deferring Close immediately after Open.
177-186: HTTP polling without timeout/context can hangupdateTask uses a default http.Client and NewRequest without context; a stalled upstream will block indefinitely.
- req, err := http.NewRequest("GET", url, nil) + // Bind to request context and enforce a per-call timeout. + ctx, cancel := context.WithTimeout(c.Request.Context(), 30*time.Second) + defer cancel() + req, err := http.NewRequestWithContext(ctx, "GET", url, nil) if err != nil { return &aliResponse, err, nil } req.Header.Set("Authorization", "Bearer "+info.ApiKey) - client := &http.Client{} + client := &http.Client{Timeout: 30 * time.Second} resp, err := client.Do(req)
223-236: Handle empty TaskStatus by polling until a terminal state
Currently, whenrsp.Output.TaskStatus == "",asyncTaskWaitimmediately returns a zero‐valueAliResponse, causingaliImageHandlerto respond with an empty image list instead of retrying. Replace that early return with polling logic: ifstep < maxStep, sleep andcontinue; otherwise return the non-emptyrsp.
♻️ Duplicate comments (2)
relay/channel/ali/adaptor.go (1)
69-79: Don’t force application/json; let modes/body decide Content-TypeUnconditional JSON header will break multipart/binary image edit requests handled by imageProcessMode.
channel.SetupApiRequestHeader(info, c, req) - req.Set("Content-Type", "application/json") + // Prefer Content-Type provided by the mode/request builder via context. + if ct := c.GetString("content-type"); ct != "" { + req.Set("Content-Type", ct) + } else if !(info.RelayMode == constant.RelayModeImagesEdits && a.imageProcessMode != nil) { + // Safe default for JSON-based requests. + req.Set("Content-Type", "application/json") + } req.Set("Authorization", "Bearer "+info.ApiKey) if info.IsStream { req.Set("X-DashScope-SSE", "enable") } if c.GetString("plugin") != "" { req.Set("X-DashScope-Plugin", c.GetString("plugin")) }Follow-up: ensure the image mode’s ProcessRequest sets c.Set("content-type", value) when it builds multipart bodies.
relay/channel/ali/image_process_mode.go (1)
117-146: Mask extraction bug: condition inverted; mask never readLoop only executes when mask doesn't exist/empty. Also close files immediately after read.
Apply:
-func getFormMarkImage(c *gin.Context) (string, bool) { - var imageFiles []*multipart.FileHeader - var exists bool - if imageFiles, exists = c.Request.MultipartForm.File["mask"]; !exists || len(imageFiles) == 0 { - for i, fileHeader := range imageFiles { +func getFormMarkImage(c *gin.Context) (string, bool) { + imageFiles, exists := c.Request.MultipartForm.File["mask"] + if !exists || len(imageFiles) == 0 { + return "", false + } + for i, fileHeader := range imageFiles { file, err := fileHeader.Open() if err != nil { common.SysError(fmt.Sprintf("failed to open image file %d: %v", i, err)) continue } - defer file.Close() // 读取文件内容 - fileBytes, err := io.ReadAll(file) + fileBytes, err := io.ReadAll(file) + _ = file.Close() if err != nil { common.SysError(fmt.Sprintf("failed to read image file %d: %v", i, err)) continue } // 确定MIME类型 mimeType := detectImageMimeType(fileHeader.Filename) // 转换为base64 base64Data := base64.StdEncoding.EncodeToString(fileBytes) dataURL := fmt.Sprintf("data:%s;base64,%s", mimeType, base64Data) return dataURL, true - } - } - return "", false + } + return "", false }
🧹 Nitpick comments (11)
relay/channel/tencent/relay-tencent.go (2)
76-91: Stream chunk completeness: include id/index and guard deltaPopulate
idfor downstream parity and setindexexplicitly. Minor guard avoids empty delta writes.func streamResponseTencent2OpenAI(info *relaycommon.RelayInfo, tencentResponse *TencentChatResponse) *dto.ChatCompletionsStreamResponse { response := dto.ChatCompletionsStreamResponse{ + Id: tencentResponse.Id, Object: "chat.completion.chunk", Created: common.GetTimestamp(), Model: info.OriginModelName, } if len(tencentResponse.Choices) > 0 { var choice dto.ChatCompletionsStreamResponseChoice - choice.Delta.SetContentString(tencentResponse.Choices[0].Delta.Content) + choice.Index = 0 + if content := tencentResponse.Choices[0].Delta.Content; content != "" { + choice.Delta.SetContentString(content) + } if tencentResponse.Choices[0].FinishReason == "stop" { choice.FinishReason = &constant.FinishReasonStop } response.Choices = append(response.Choices, choice) } return &response }
80-81: Model in stream response sourced from RelayInfo — ensure parity in non-stream pathNon-stream responses currently omit
Model. For client parity, consider passinginfointoresponseTencent2OpenAIand settingModelthere too.relay/channel/ali/dto.go (3)
117-123: Unify N type with upstream request (dto.ImageRequest.N is uint)Avoid int/uint mismatch to prevent unnecessary casts and negative values.
type AliImageParameters struct { Size string `json:"size,omitempty"` - N int `json:"n,omitempty"` + N uint `json:"n,omitempty"` Steps string `json:"steps,omitempty"` Scale string `json:"scale,omitempty"` Watermark *bool `json:"watermark,omitempty"` }
156-181: Naming consistency: “Multimodel” vs “Multimodal”Types and helpers mix “model/modal” spellings. Pick one (“multimodal” is standard) to reduce confusion and future drift. Consider keeping a type alias for compatibility.
Minimal, non-breaking alias you can add elsewhere:
// keep both spellings usable type AliMultimodalGenerationRequest = AliMultimodelGenerationRequest type AliMultimodalGenerationResponse = AliMultimodelGenerationResponse
117-123: Verify parameter types for Steps/ScaleAre Steps/Scale numeric per Ali API? If so, prefer int/float types to avoid string parsing downstream.
relay/channel/ali/image.go (1)
29-31: Typo: multimoalGenerationMode -> multimodalGenerationModeSpelling likely unintended and risks unresolved symbol if the constructor is properly named elsewhere.
- return multimoalGenerationMode() + return multimodalGenerationMode()If the constructor really is “multimoal…”, consider renaming it at the definition for consistency.
relay/channel/ali/image_process_mode.go (5)
88-113: Close files promptly; avoid defers inside loopsDeferring in a loop can leak FDs for large batches.
Apply:
- defer file.Close() - - // 读取文件内容 - fileBytes, err := io.ReadAll(file) + // 读取文件内容 + fileBytes, err := io.ReadAll(file) + _ = file.Close()
201-209: Minor typos: response var nameStandardize respsonseBody → responseBody for readability.
Apply:
- respsonseBody, err := io.ReadAll(resp.Body) + responseBody, err := io.ReadAll(resp.Body) @@ - var multimoalGenerationResp AliMultimodelGenerationResponse - if err := json.Unmarshal(respsonseBody, &multimoalGenerationResp); err != nil { + var multimoalGenerationResp AliMultimodelGenerationResponse + if err := json.Unmarshal(responseBody, &multimoalGenerationResp); err != nil { @@ - }, respsonseBody, info, responseFormat) + }, responseBody, info, responseFormat)Also applies to: 248-248
294-295: Use ReplaceAllCleaner and slightly faster than Replace with -1.
Apply:
- Size: strings.Replace(request.Size, "x", "*", -1), + Size: strings.ReplaceAll(request.Size, "x", "*"),
312-318: Propagate size consistently across edit flowsBoth branches omit Parameters.Size; align behavior with the sketch branch (if API supports it).
Apply:
Parameters: struct { N uint `json:"n,omitempty"` Size string `json:"size,omitempty"` }{ - N: request.N, + N: request.N, + Size: strings.ReplaceAll(request.Size, "x", "*"), },And:
Parameters: struct { N uint `json:"n,omitempty"` Size string `json:"size,omitempty"` }{ - N: request.N, + N: request.N, + Size: strings.ReplaceAll(request.Size, "x", "*"), },Please confirm Ali image2image edit APIs accept size in these modes.
Also applies to: 334-340
42-56: Remove unused helper function indexOfAnySubStrThis function isn’t referenced anywhere in the repo; drop the dead code.
📜 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 selected for processing (7)
dto/channel_settings.go(1 hunks)relay/channel/ali/adaptor.go(6 hunks)relay/channel/ali/dto.go(2 hunks)relay/channel/ali/image.go(4 hunks)relay/channel/ali/image_process_mode.go(1 hunks)relay/channel/tencent/adaptor.go(4 hunks)relay/channel/tencent/relay-tencent.go(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- dto/channel_settings.go
- relay/channel/tencent/adaptor.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T17:12:43.157Z
Learnt from: RedwindA
PR: QuantumNous/new-api#1537
File: relay/gemini_handler.go:330-342
Timestamp: 2025-08-08T17:12:43.157Z
Learning: In the new-api repository, the `GeminiEmbeddingHandler` function in `relay/gemini_handler.go` is designed specifically for native Gemini embedding requests and therefore does not require the `ConvertGeminiRequest` step that is used in the chat handler. The embedding requests are already in the native Gemini format and don't need conversion.
Applied to files:
relay/channel/ali/adaptor.go
🧬 Code graph analysis (5)
relay/channel/ali/image_process_mode.go (5)
common/sys_log.go (1)
SysError(15-18)relay/common/relay_info.go (1)
RelayInfo(74-120)types/error.go (5)
NewAPIError(82-90)NewError(187-199)ErrorCodeReadResponseBodyFailed(65-65)ErrorCodeBadResponseBody(68-68)WithOpenAIError(239-262)relay/channel/ali/dto.go (10)
AliImageMessageItem(8-10)AliTextMessageItem(11-13)AliInput(31-35)AliMessage(21-24)AliMultimodelGenerationResponse(158-181)TaskResult(86-91)AliResponse(104-108)AliOutput(93-102)AliUsage(80-84)AliImage2ImageImageSynthesisRequest(183-196)service/http.go (1)
CloseResponseBodyGracefully(14-22)
relay/channel/tencent/relay-tencent.go (4)
relay/common/relay_info.go (1)
RelayInfo(74-120)relay/channel/tencent/dto.go (2)
TencentChatResponse(63-72)TencentChatRequest(8-44)dto/openai_response.go (2)
ChatCompletionsStreamResponse(136-144)ChatCompletionsStreamResponseChoice(75-80)relay/channel/tencent/adaptor.go (1)
Adaptor(21-27)
relay/channel/ali/image.go (6)
relay/common/relay_info.go (1)
RelayInfo(74-120)relay/channel/ali/image_process_mode.go (1)
ImageProcessMode(148-153)relay/channel/ali/adaptor.go (1)
Adaptor(20-22)dto/openai_image.go (1)
ImageRequest(13-32)types/error.go (3)
NewAPIError(82-90)NewError(187-199)ErrorCodeBadResponseBody(68-68)service/http.go (1)
IOCopyBytesGracefully(24-59)
relay/channel/ali/adaptor.go (1)
relay/channel/ali/image_process_mode.go (1)
ImageProcessMode(148-153)
relay/channel/ali/dto.go (2)
dto/openai_request.go (1)
Message(259-270)dto/openai_response.go (1)
Usage(217-230)
🔇 Additional comments (4)
relay/channel/tencent/relay-tencent.go (2)
114-114: Propagating RelayInfo into stream converter — LGTMPassing
infoenables correctmodelpropagation in SSE responses.
147-152: Error.Code mapping is correct — no changes needed
TencentError.Codeis defined asanyand maps directly toOpenAIError.Code(alsoany), andWithOpenAIErrorsafely asserts string or falls back tofmt.Sprintf.relay/channel/ali/image.go (1)
314-319: Good: avoid HTML escaping in JSON outputUsing a non-escaping encoder fixes the OSSAccessKeyId query param getting mangled (e.g., “&” -> “\u0026”).
relay/channel/ali/image_process_mode.go (1)
22-40: LGTM: simple, safe MIME inferenceReasonable defaults; fallback to PNG is fine.
| return imageContents, len(imageContents) | ||
| } | ||
|
|
||
| func getFormMarkImage(c *gin.Context) (string, bool) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Naming nit: “Mark” → “Mask”
Rename getFormMarkImage to getFormMaskImage for clarity. Update call sites accordingly.
🏁 Script executed:
#!/bin/bash
rg -nP '\bgetFormMarkImage\b' -C2Length of output: 796
Rename getFormMarkImage to getFormMaskImage
- Update the function declaration in relay/channel/ali/image_process_mode.go (line 117)
- Update its single call site in the same file (line 275) from getFormMarkImage to getFormMaskImage
🤖 Prompt for AI Agents
In relay/channel/ali/image_process_mode.go around lines 117 and 275, the
function getFormMarkImage should be renamed to getFormMaskImage: change the
function declaration at line 117 from getFormMarkImage to getFormMaskImage and
update its single call site at line 275 to call getFormMaskImage; ensure the
function comment/any local references reflect the new name and run a quick build
to catch any missed references.
| func multimoalGenerationMode() *ImageProcessMode { | ||
| return &ImageProcessMode{ | ||
| Url: "/api/v1/services/aigc/multimodal-generation/generation", | ||
| Async: false, | ||
| ProcessRequest: func(c *gin.Context, info *relaycommon.RelayInfo, request dto.ImageRequest) (any, error) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Typo: multimoal → multimodal (public-facing identifier risk)
Correct the function name; update references in selector code.
Apply:
-func multimoalGenerationMode() *ImageProcessMode {
+func multimodalGenerationMode() *ImageProcessMode {Then check call sites:
🏁 Script executed:
#!/bin/bash
rg -nP '\bmultimoalGenerationMode\b' -C2Length of output: 734
Rename multimoalGenerationMode to multimodalGenerationMode
Fix the typo in both the definition and its sole call site.
--- a/relay/channel/ali/image_process_mode.go
+++ b/relay/channel/ali/image_process_mode.go
@@ -161,7 +161,7 @@ func text2ImageMode() *ImageProcessMode {
}
-func multimoalGenerationMode() *ImageProcessMode {
+func multimodalGenerationMode() *ImageProcessMode {
return &ImageProcessMode{
Url: "/api/v1/services/aigc/multimodal-generation/generation",
Async: false,--- a/relay/channel/ali/image.go
+++ b/relay/channel/ali/image.go
@@ -28,7 +28,7 @@ func selectImageMode(mode string) *ImageProcessMode {
case "qwen-image-edit":
- return multimoalGenerationMode()
+ return multimodalGenerationMode()
case "wanx2.1-imageedit", "wanx-sketch-to-image-lite":
return image2ImageMode()
default:📝 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.
| func multimoalGenerationMode() *ImageProcessMode { | |
| return &ImageProcessMode{ | |
| Url: "/api/v1/services/aigc/multimodal-generation/generation", | |
| Async: false, | |
| ProcessRequest: func(c *gin.Context, info *relaycommon.RelayInfo, request dto.ImageRequest) (any, error) { | |
| // File: relay/channel/ali/image_process_mode.go | |
| func multimodalGenerationMode() *ImageProcessMode { | |
| return &ImageProcessMode{ | |
| Url: "/api/v1/services/aigc/multimodal-generation/generation", | |
| Async: false, | |
| ProcessRequest: func(c *gin.Context, info *relaycommon.RelayInfo, request dto.ImageRequest) (any, error) { | |
| // …rest of implementation… | |
| }, | |
| } | |
| } |
| func multimoalGenerationMode() *ImageProcessMode { | |
| return &ImageProcessMode{ | |
| Url: "/api/v1/services/aigc/multimodal-generation/generation", | |
| Async: false, | |
| ProcessRequest: func(c *gin.Context, info *relaycommon.RelayInfo, request dto.ImageRequest) (any, error) { | |
| // File: relay/channel/ali/image.go | |
| func selectImageMode(mode string) *ImageProcessMode { | |
| switch mode { | |
| case "qwen-image-edit": | |
| return multimodalGenerationMode() | |
| case "wanx2.1-imageedit", "wanx-sketch-to-image-lite": | |
| return image2ImageMode() | |
| default: | |
| return defaultImageMode() | |
| } | |
| } |
🤖 Prompt for AI Agents
In relay/channel/ali/image_process_mode.go around lines 161 to 165, the function
name multimoalGenerationMode contains a typo and should be renamed to
multimodalGenerationMode; update the function definition name and replace its
sole call site accordingly so all references compile (search the file for
multimoalGenerationMode and change both the declaration and the call to
multimodalGenerationMode).
| return nil, &dto.Usage{} | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Return real usage for quota/accounting
Currently returns empty usage; propagate tokens to upstream accounting.
Apply (fields may need adjusting to your dto.Usage):
- return nil, &dto.Usage{}
+ return nil, &dto.Usage{
+ PromptTokens: usage.InputTokens,
+ CompletionTokens: usage.OutputTokens,
+ TotalTokens: usage.InputTokens + usage.OutputTokens,
+ }Verify dto.Usage fields:
🏁 Script executed:
#!/bin/bash
rg -nP 'type\s+Usage\s+struct' -C3 one-api | sed -n '1,120p'Length of output: 108
🏁 Script executed:
#!/bin/bash
# Show imports and context in image_process_mode.go
sed -n '1,80p' relay/channel/ali/image_process_mode.go
# Show lines around dto.Usage usage
grep -n -C3 "dto.Usage" relay/channel/ali/image_process_mode.go
# Find all Usage struct definitions in Go files
rg -nP 'type\s+Usage\s+struct' -C3 --type=goLength of output: 3265
Use actual usage values instead of returning an empty dto.Usage
The dto.Usage struct (in dto/openai_response.go) has fields PromptTokens, CompletionTokens, and TotalTokens. Replace the empty return with:
- return nil, &dto.Usage{}
+ return nil, &dto.Usage{
+ PromptTokens: usage.PromptTokens,
+ CompletionTokens: usage.CompletionTokens,
+ TotalTokens: usage.TotalTokens,
+ }🤖 Prompt for AI Agents
In relay/channel/ali/image_process_mode.go around lines 257 to 258, the function
currently returns an empty dto.Usage which loses token accounting; replace that
empty return with a dto.Usage populated from the actual token counts the
function computes or receives (fill PromptTokens and CompletionTokens from the
existing prompt/completion token variables or the response usage object, and set
TotalTokens = PromptTokens + CompletionTokens). Ensure you reference the correct
local variables or response fields available in this function and return the
populated dto.Usage instead of an empty struct.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx (11)
55-57: Validate and normalize USD exchange rate.Default is fine, but guard against NaN or ≤0 from props and keep type numeric.
- const [USDExchangeRate, setUSDExchangeRate] = useState(7); + const [USDExchangeRate, setUSDExchangeRate] = useState(7); ... - if(props.options.USDExchangeRate){ - setUSDExchangeRate(parseFloat(props.options.USDExchangeRate)) - } + const v = parseFloat(props.options.USDExchangeRate); + if (!Number.isNaN(v) && v > 0) { + setUSDExchangeRate(v); + }Also applies to: 131-134
58-60: Fix typo in state: hiddenModeNames → hiddenModelNames (and all references).Current spelling is inconsistent and easy to misread.
- const [hiddenModeNames, setHiddenModelNames] = useState([]); + const [hiddenModelNames, setHiddenModelNames] = useState([]); ... - const keepHiddenModels = (modelConf)=> { - return hiddenModeNames.reduce((newConf,hiddenModel)=> { + const keepHiddenModels = (modelConf)=> { + return hiddenModelNames.reduce((newConf,hiddenModel)=> {Also applies to: 158-166
188-213: Simplify save result checks;includes(undefined)is unreachable with Promise.all.Promise rejections jump to catch; keep per-response success checks only.
- // 批量处理请求 - const results = await Promise.all(requestQueue); - - // 验证结果 - if (requestQueue.length === 1) { - if (results.includes(undefined)) return; - } else if (requestQueue.length > 1) { - if (results.includes(undefined)) { - return showError('部分保存失败，请重试'); - } - } + // 批量处理请求 + const results = await Promise.all(requestQueue);
329-333: Avoid magic number for base USD price; centralize conversion helpers.Extract a constant and reuse conversion helpers for symmetry and testability.
+ const BASE_PRICE_PER_M_TOKENS_USD = 2; const calculateRatioFromTokenPrice = (tokenPrice) => { if (currencyType === "CNY"){ tokenPrice = tokenPrice / USDExchangeRate } - return tokenPrice / 2; + return tokenPrice / BASE_PRICE_PER_M_TOKENS_USD; }; const calculateTokenPriceFromRatio = (ratio) => { if (currencyType === "CNY"){ ratio = ratio * USDExchangeRate } - return ratio * 2; + return ratio * BASE_PRICE_PER_M_TOKENS_USD; };Also applies to: 440-445
460-461: Remove unused var and no-op assignment.
intiialCurrucyTypeis unused andsetCurrencyType(currencyType)is a no-op.- let intiialCurrucyType = 'USD' ... - setCurrencyType(currencyType) + // keep existing currencyTypeAlso applies to: 472-472
496-520: Strengthen currency toggle handler and fix typos.
- Rename to handleCurrencyTypeChange.
- Parse numbers explicitly; avoid implicit coercion.
- Deduplicate with a small converter.
- const handleCurrucyTypeChange = (value) => { - let newCurrencyType = value + const handleCurrencyTypeChange = (value) => { + const newCurrencyType = value; let oldCurrencyType = currencyType if(newCurrencyType === oldCurrencyType){ return } - let updatedModel = {... currentModel} - if(newCurrencyType === "USD" && updatedModel.tokenPrice){ - updatedModel.tokenPrice = updatedModel.tokenPrice / USDExchangeRate - }else if(updatedModel.tokenPrice) { - updatedModel.tokenPrice = updatedModel.tokenPrice * USDExchangeRate - } - if(newCurrencyType === "USD" && updatedModel.completionTokenPrice){ - updatedModel.completionTokenPrice = updatedModel.completionTokenPrice / USDExchangeRate - }else if(updatedModel.completionTokenPrice) { - updatedModel.completionTokenPrice = updatedModel.completionTokenPrice * USDExchangeRate - } - if(newCurrencyType === "USD" && updatedModel.price){ - updatedModel.price = updatedModel.price / USDExchangeRate - }else if(updatedModel.price){ - updatedModel.price = updatedModel.price * USDExchangeRate - } + const updatedModel = {...(currentModel || {})}; + const conv = (v, toUSD) => { + const n = parseFloat(v); + if (!Number.isFinite(n)) return v; + return toUSD ? n / USDExchangeRate : n * USDExchangeRate; + }; + if (updatedModel.tokenPrice) { + updatedModel.tokenPrice = conv(updatedModel.tokenPrice, newCurrencyType === "USD"); + } + if (updatedModel.completionTokenPrice) { + updatedModel.completionTokenPrice = conv(updatedModel.completionTokenPrice, newCurrencyType === "USD"); + } + if (updatedModel.price) { + updatedModel.price = conv(updatedModel.price, newCurrencyType === "USD"); + } setCurrencyType(newCurrencyType) setCurrentModel(updatedModel) }Also update callers:
- <Select field="currencyType" value={currencyType} onChange={handleCurrucyTypeChange}> + <Select field="currencyType" value={currencyType} onChange={handleCurrencyTypeChange}>
522-528: Guard Form.setValues against null.Passing null may break some form libs; default to {}.
- if (formRef.current) { - formRef.current.formApi.setValues(currentModel); - } + if (formRef.current) { + formRef.current.formApi.setValues(currentModel || {}); + }
636-636: Safer initValues default.- <Form ref={formRef} initValues={currentModel}> + <Form ref={formRef} initValues={currentModel || {}}>
717-729: Avoid double control of Form fields (value + Form state).Let Form manage values via field; remove explicit value props to prevent conflicts.
- <Form.InputNumber + <Form.InputNumber hideButtons precision={6} field='ratio' label={t('模型倍率')} placeholder={t('输入模型倍率')} onChange={(value) => setCurrentModel((prev) => ({ ...(prev || {}), ratio: value, })) } - value={currentModel?.ratio || ''} /> ... - <Form.InputNumber + <Form.InputNumber hideButtons precision={6} field='completionRatio' label={t('补全倍率')} placeholder={t('输入补全倍率')} onChange={(value) => setCurrentModel((prev) => ({ ...(prev || {}), completionRatio: value, })) } - value={currentModel?.completionRatio || ''} /> ... - <Form.InputNumber + <Form.InputNumber hideButtons - currency={currencyType} precision={6} field='tokenPrice' label={t('输入价格')} onChange={(value) => { handleTokenPriceChange(value); }} - value={currentModel?.tokenPrice || ''} suffix={t('1M tokens')} /> ... - <Form.InputNumber + <Form.InputNumber hideButtons - currency={currencyType} precision={6} field='completionTokenPrice' label={t('输出价格')} onChange={(value) => { handleCompletionTokenPriceChange(value); }} - value={currentModel?.completionTokenPrice || ''} suffix={t('1M tokens')} /> ... - <Form.InputNumber + <Form.InputNumber hideButtons - currency={currencyType} precision={6} field='price' label={t('固定价格(每次)')} placeholder={t('输入每次价格')} onChange={(value) => setCurrentModel((prev) => ({ ...(prev || {}), price: value, })) } - value={currentModel?.price || ''} />Also applies to: 731-744, 754-777, 789-803
750-753: Confirm Semi UI InputNumber supports a currency prop; otherwise use prefix/formatter.Semi InputNumber typically uses prefix/suffix or formatter/parser; unknown props can leak to DOM with warnings.
- <Select field="currencyType" value={currencyType} onChange={handleCurrencyTypeChange}> + <Select field="currencyType" value={currencyType} onChange={handleCurrencyTypeChange}> <Select.Option value={"USD"}>USD</Select.Option> <Select.Option value={"CNY"}>CNY</Select.Option> </Select> ... - <Form.InputNumber hideButtons currency={currencyType} ... /> + <Form.InputNumber hideButtons prefix={currencyType + ' '} ... />Also applies to: 785-789
329-333: Rounding/consistency for ratios/prices.Consider rounding to precision (e.g., 6) when computing ratios and before dividing by exchange rate to avoid long floats.
Also applies to: 601-617, 629-631
📜 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 selected for processing (1)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx(19 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx (2)
web/src/helpers/render.jsx (1)
getQuotaPerUnit(870-874)web/src/helpers/utils.jsx (1)
showError(118-147)
🔇 Additional comments (6)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx (6)
32-32: Import for Select looks correct.
64-77: Enabled-models fetch + error handling: LGTM.
79-83: Mount-side refresh: LGTM.
167-170: Confirm intent: preserving only hidden models’ existing values.
keepHiddenModelsdrops prior values for visible models. If that’s intended for “unset_models” it’s fine; otherwise we may need to merge more fields.
221-224: Refresh enabled models post-save: LGTM.
451-453: Resetting currency to USD on every modal open — confirm UX.If users expect last-used currency to persist, keep it instead of hard-reset.
| if(props.mode === "unset_models"){ | ||
| // 找出所有未设置价格和倍率的模型 | ||
| const {unsetModels,hiddenModels} = enabledModelNames.reduce((groupByUnset,modelName) => { | ||
| const hasPrice = modelPrice[modelName] !== undefined; | ||
| const hasRatio = modelRatio[modelName] !== undefined; | ||
|
|
||
| // 如果模型没有价格或者没有倍率设置，则显示 | ||
| const groupName = !hasPrice && !hasRatio; | ||
| groupByUnset[groupName? "unsetModels": "hiddenModels"].push(modelName); | ||
| return groupByUnset; | ||
| },{"unsetModels":[],"hiddenModels":[]}) | ||
| modelData = unsetModels.map((name) => ({ | ||
| name, | ||
| price: modelPrice[name] || '', | ||
| ratio: modelRatio[name] || '', | ||
| completionRatio: completionRatio[name] || '', | ||
| })); | ||
| setHiddenModelNames(hiddenModels) | ||
| } |
There was a problem hiding this comment.
Unset-models filter uses AND but comment says OR — likely logic bug.
To “显示” models missing price OR ratio, use || instead of &&.
- // 如果模型没有价格或者没有倍率设置，则显示
- const groupName = !hasPrice && !hasRatio;
+ // 如果模型没有价格或者没有倍率设置，则显示
+ const groupName = !hasPrice || !hasRatio;📝 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(props.mode === "unset_models"){ | |
| // 找出所有未设置价格和倍率的模型 | |
| const {unsetModels,hiddenModels} = enabledModelNames.reduce((groupByUnset,modelName) => { | |
| const hasPrice = modelPrice[modelName] !== undefined; | |
| const hasRatio = modelRatio[modelName] !== undefined; | |
| // 如果模型没有价格或者没有倍率设置，则显示 | |
| const groupName = !hasPrice && !hasRatio; | |
| groupByUnset[groupName? "unsetModels": "hiddenModels"].push(modelName); | |
| return groupByUnset; | |
| },{"unsetModels":[],"hiddenModels":[]}) | |
| modelData = unsetModels.map((name) => ({ | |
| name, | |
| price: modelPrice[name] || '', | |
| ratio: modelRatio[name] || '', | |
| completionRatio: completionRatio[name] || '', | |
| })); | |
| setHiddenModelNames(hiddenModels) | |
| } | |
| if(props.mode === "unset_models"){ | |
| // 找出所有未设置价格和倍率的模型 | |
| const {unsetModels,hiddenModels} = enabledModelNames.reduce((groupByUnset,modelName) => { | |
| const hasPrice = modelPrice[modelName] !== undefined; | |
| const hasRatio = modelRatio[modelName] !== undefined; | |
| // 如果模型没有价格或者没有倍率设置，则显示 | |
| const groupName = !hasPrice || !hasRatio; | |
| groupByUnset[groupName ? "unsetModels" : "hiddenModels"].push(modelName); | |
| return groupByUnset; | |
| },{"unsetModels":[],"hiddenModels":[]}); | |
| modelData = unsetModels.map((name) => ({ | |
| name, | |
| price: modelPrice[name] || '', | |
| ratio: modelRatio[name] || '', | |
| completionRatio: completionRatio[name] || '', | |
| })); | |
| setHiddenModelNames(hiddenModels); | |
| } |
🤖 Prompt for AI Agents
In web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx around lines 111 to
129, the reduce currently marks a model as "unset" only when it lacks both price
AND ratio (using &&) but the comment and intended behavior say it should be
unset when it lacks price OR ratio; change the logic to compute a boolean like
const isUnset = !hasPrice || !hasRatio and use that to push into "unsetModels"
or "hiddenModels", leaving the rest of the reduce and subsequent mapping intact
(ensure you still initialize the accumulator as {"unsetModels": [],
"hiddenModels": []} and call setHiddenModelNames(hiddenModels) as before).
|
感谢各位大佬的辛勤付出，期待这个pr |
chi-cat
changed the title
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
relay/channel/ali/image.go (1)
274-318: Response delegation + non-escaping marshal: good; fix polling edge case and add timeout
- Good: ProcessResponse hook and MarshalWithoutHTMLEscape usage.
- Please fix asyncTaskWait returning early when TaskStatus == "" (it causes aliImageHandler to treat it as an error). It should continue polling.
- Add an HTTP client timeout in updateTask to avoid hanging on upstream issues.
Proposed fixes outside this hunk:
In asyncTaskWait (when rsp.Output.TaskStatus == ""):
- if rsp.Output.TaskStatus == "" { - return &taskResponse, responseBody, nil - } + if rsp.Output.TaskStatus == "" { + // keep polling until a valid status is returned + if step >= maxStep { + break + } + time.Sleep(time.Duration(waitSeconds) * time.Second) + continue + }In updateTask:
- client := &http.Client{} + client := &http.Client{Timeout: 15 * time.Second}
♻️ Duplicate comments (4)
relay/channel/ali/image_process_mode.go (3)
161-165: Typo: multimoal → multimodalRename function to avoid public-facing typo; update selector/call sites accordingly.
-func multimoalGenerationMode() *ImageProcessMode { +func multimodalGenerationMode() *ImageProcessMode {
200-258: Return real usage instead of empty structPopulate
dto.Usageusing Ali usage tokens. Also fix minor variable typo.- respsonseBody, err := io.ReadAll(resp.Body) + responseBody, err := io.ReadAll(resp.Body) if err != nil { return types.NewError(err, types.ErrorCodeReadResponseBodyFailed), nil } service.CloseResponseBodyGracefully(resp) var multimoalGenerationResp AliMultimodelGenerationResponse - if err := json.Unmarshal(respsonseBody, &multimoalGenerationResp); err != nil { + if err := json.Unmarshal(responseBody, &multimoalGenerationResp); err != nil { return types.NewError(err, types.ErrorCodeBadResponseBody), nil } @@ - fullTextResponse := responseAli2OpenAIImage(c, &AliResponse{ + fullTextResponse := responseAli2OpenAIImage(c, &AliResponse{ @@ - }, respsonseBody, info, responseFormat) + }, responseBody, info, responseFormat) @@ - return nil, &dto.Usage{} + return nil, &dto.Usage{ + PromptTokens: usage.InputTokens, + CompletionTokens: usage.OutputTokens, + TotalTokens: usage.InputTokens + usage.OutputTokens, + }
117-146: Mask extraction bug and naming: logic inverted; “Mark” → “Mask”Function never iterates when files exist; also rename for clarity. Close file promptly.
-func getFormMarkImage(c *gin.Context) (string, bool) { - var imageFiles []*multipart.FileHeader - var exists bool - if imageFiles, exists = c.Request.MultipartForm.File["mask"]; !exists || len(imageFiles) == 0 { - for i, fileHeader := range imageFiles { +func getFormMaskImage(c *gin.Context) (string, bool) { + imageFiles, exists := c.Request.MultipartForm.File["mask"] + if !exists || len(imageFiles) == 0 { + return "", false + } + for i, fileHeader := range imageFiles { file, err := fileHeader.Open() if err != nil { common.SysError(fmt.Sprintf("failed to open image file %d: %v", i, err)) continue } - defer file.Close() - + // close promptly after read // 读取文件内容 - fileBytes, err := io.ReadAll(file) + fileBytes, err := io.ReadAll(file) + _ = file.Close() if err != nil { common.SysError(fmt.Sprintf("failed to read image file %d: %v", i, err)) continue } // 确定MIME类型 mimeType := detectImageMimeType(fileHeader.Filename) // 转换为base64 base64Data := base64.StdEncoding.EncodeToString(fileBytes) dataURL := fmt.Sprintf("data:%s;base64,%s", mimeType, base64Data) return dataURL, true - } - } - return "", false + } + return "", false }And update call site:
- maskImageContent, exists := getFormMarkImage(c) + maskImageContent, exists := getFormMaskImage(c)relay/channel/ali/adaptor.go (1)
71-83: Don’t force application/json; let mode/body dictate Content-TypeHonor a mode-provided content type via context; fall back to JSON otherwise.
channel.SetupApiRequestHeader(info, c, req) - req.Set("Content-Type", "application/json") + if ct := c.GetString("content-type"); ct != "" { + req.Set("Content-Type", ct) + } else { + req.Set("Content-Type", "application/json") + }
🧹 Nitpick comments (11)
relay/common/relay_info.go (1)
488-493: Use Go naming and avoid anonymous element type in ImageList
- Prefer VideoURL (initialism) over VideoUrl.
- Replace the anonymous struct in ImageList with a named type for reuse and clarity.
Apply within this segment:
- ImageList []struct { - Image string `json:"image"` - } `json:"image_list,omitempty"` - VideoUrl string `json:"video_url,omitempty"` + ImageList []ImageItem `json:"image_list,omitempty"` + VideoURL string `json:"video_url,omitempty"`Add outside this struct:
type ImageItem struct { Image string `json:"image"` }relay/relay_adaptor.go (1)
132-134: Ali TaskAdaptor wiring: add compile-time interface assertionThe switch case looks good. Add an interface satisfaction check in relay/channel/ali/adaptor.go to catch drift at compile time.
Add in relay/channel/ali/adaptor.go:
var _ channel.TaskAdaptor = (*ali.TaskAdaptor)(nil)Also confirm the upstream sets platform to the numeric ChannelTypeAli (17); if the string "ali" is ever used, add a string case in GetTaskAdaptor.
relay/relay_task.go (1)
279-283: Switch to non-HTML-escaping marshal is correctGood choice to preserve URLs/query params in JSON.
Optional: For consistency, consider using common.MarshalWithoutHTMLEscape in sunoFetchRespBodyBuilder and sunoFetchByIDRespBodyBuilder too.
Additionally, per prior learning about redacting large base64 payloads in video tasks, ensure the fetched TaskModel2Dto(Data) is already sanitized before returning here.
common/json.go (1)
48-62: Small API polish and safety note
- Prefer
anyoverinterface{}for consistency with other funcs in this file.- Add a doc comment clarifying this is for API JSON bodies; do not embed raw JSON into HTML without escaping to avoid XSS in UIs that might innerHTML the response.
Apply within this segment:
-func MarshalWithoutHTMLEscape(v interface{}) ([]byte, error) { +func MarshalWithoutHTMLEscape(v any) ([]byte, error) {relay/channel/ali/video_process_mode.go (4)
44-56: Typo in public type: VideoGenrationUsage → VideoGenerationUsageAvoid leaking misspellings into public API; rename and update the field type.
-type VideoGenrationUsage struct { +type VideoGenerationUsage struct { Duration int `json:"duration"` VideoCount int `json:"video_count"` SR int `json:"SR"` } type VideoGenerationResponse struct { Output VideoGenerationOutput `json:"output"` - Usage VideoGenrationUsage `json:"usage,omitempty"` + Usage VideoGenerationUsage `json:"usage,omitempty"` RequestId string `json:"request_id"` Code string `json:"code,omitempty"` Message string `json:"message,omitempty"` }
3-17: Make scheme check case-insensitiveSupport mixed-case schemes; add strings import and use EqualFold.
import ( "bytes" "encoding/json" "fmt" "io" "net/http" "net/url" "one-api/constant" "one-api/dto" relaycommon "one-api/relay/common" "one-api/service" + "strings" "github.com/gin-gonic/gin" "github.com/pkg/errors" ) func isHttpUrl(str string) bool { u, err := url.Parse(str) if err != nil { return false } - return u.Host != "" && (u.Scheme == "http" || u.Scheme == "https") + return u.Host != "" && (strings.EqualFold(u.Scheme, "http") || strings.EqualFold(u.Scheme, "https")) }Also applies to: 25-31
75-81: Avoid double prefixing data URLsIf already a data URL, return as-is.
func normalizeImageUrl(url string) string { - if isHttpUrl(url) { + if strings.HasPrefix(url, "data:") { + return url + } + if isHttpUrl(url) { return url } else { return fmt.Sprintf("data:image/png;base64,%s", url) } }
170-186: Field name typo: Paramters → Parameters (readability)Not functionally breaking due to JSON tag but worth correcting.
aliReq := &struct { Model string `json:"model"` Input any `json:"input"` - Paramters any `json:"parameters"` + Parameters any `json:"parameters"` }{relay/channel/ali/image_process_mode.go (2)
88-113: Close files inside the loop, not via deferAvoid accumulating open FDs for multiple images.
- defer file.Close() - // 读取文件内容 fileBytes, err := io.ReadAll(file) + _ = file.Close()
42-56: Unused helper
indexOfAnySubStris unused; consider removing to reduce noise.relay/channel/ali/adaptor.go (1)
214-224: Defensive type assertion for task_requestAvoid panic if context value is missing/wrong type.
v, exists := c.Get("task_request") if !exists { return nil, fmt.Errorf("request not found in context") } - req := v.(relaycommon.TaskSubmitReq) + req, ok := v.(relaycommon.TaskSubmitReq) + if !ok { + return nil, fmt.Errorf("invalid task_request type") + } if ta.videoProcessMode != nil { return ta.videoProcessMode.ProcessRequest(c, info, req) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
common/json.go(1 hunks)relay/channel/ali/adaptor.go(7 hunks)relay/channel/ali/image.go(3 hunks)relay/channel/ali/image_process_mode.go(1 hunks)relay/channel/ali/video_process_mode.go(1 hunks)relay/common/relay_info.go(1 hunks)relay/relay_adaptor.go(2 hunks)relay/relay_task.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
PR: QuantumNous/new-api#1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, the redactVideoResponseBody function sanitizes video task responses by removing bytesBase64Encoded fields and truncating base64 strings to 256 characters to prevent large binary data from being stored in task.Data.
Applied to files:
relay/relay_task.go
📚 Learning: 2025-08-08T17:12:43.157Z
Learnt from: RedwindA
PR: QuantumNous/new-api#1537
File: relay/gemini_handler.go:330-342
Timestamp: 2025-08-08T17:12:43.157Z
Learning: In the new-api repository, the `GeminiEmbeddingHandler` function in `relay/gemini_handler.go` is designed specifically for native Gemini embedding requests and therefore does not require the `ConvertGeminiRequest` step that is used in the chat handler. The embedding requests are already in the native Gemini format and don't need conversion.
Applied to files:
relay/channel/ali/adaptor.go
🧬 Code graph analysis (6)
relay/relay_adaptor.go (3)
constant/channel.go (1)
ChannelTypeAli(21-21)relay/channel/ali/adaptor.go (1)
TaskAdaptor(171-173)relay/channel/adapter.go (1)
TaskAdaptor(32-51)
relay/relay_task.go (1)
common/json.go (1)
MarshalWithoutHTMLEscape(48-62)
relay/channel/ali/video_process_mode.go (4)
relay/common/relay_info.go (2)
RelayInfo(74-120)TaskSubmitReq(479-493)constant/task.go (2)
TaskActionGenerate(14-14)TaskActionTextGenerate(15-15)relay/channel/ali/adaptor.go (1)
TaskAdaptor(171-173)service/error.go (1)
TaskErrorWrapper(137-153)
relay/channel/ali/adaptor.go (9)
relay/channel/ali/image_process_mode.go (1)
ImageProcessMode(148-153)relay/channel/ali/video_process_mode.go (2)
VideoProcessMode(19-23)VideoGenerationResponse(50-56)relay/channel/adapter.go (1)
TaskAdaptor(32-51)relay/common/relay_info.go (3)
RelayInfo(74-120)TaskSubmitReq(479-493)TaskInfo(495-502)common/gin.go (1)
UnmarshalBodyReusable(30-51)service/error.go (1)
TaskErrorWrapperLocal(131-135)relay/channel/api_request.go (2)
DoRequest(231-233)DoTaskApiRequest(280-302)relay/channel/ali/constants.go (1)
ChannelName(14-14)model/task.go (7)
TaskStatusSubmitted(15-15)TaskStatusInProgress(17-17)TaskStatusSuccess(19-19)TaskStatusFailure(18-18)TaskStatusUnknown(20-20)TaskStatus(11-11)TaskStatusQueued(16-16)
relay/channel/ali/image_process_mode.go (6)
common/sys_log.go (1)
SysError(15-18)relay/common/relay_info.go (1)
RelayInfo(74-120)types/error.go (5)
NewAPIError(82-90)NewError(187-199)ErrorCodeReadResponseBodyFailed(65-65)ErrorCodeBadResponseBody(68-68)WithOpenAIError(239-262)relay/channel/ali/dto.go (10)
AliImageMessageItem(8-10)AliTextMessageItem(11-13)AliInput(31-35)AliMessage(21-24)AliMultimodelGenerationResponse(158-181)TaskResult(86-91)AliResponse(104-108)AliOutput(93-102)AliUsage(80-84)AliImage2ImageImageSynthesisRequest(183-196)service/http.go (1)
CloseResponseBodyGracefully(14-22)common/json.go (1)
MarshalWithoutHTMLEscape(48-62)
relay/channel/ali/image.go (5)
relay/common/relay_info.go (1)
RelayInfo(74-120)relay/channel/ali/image_process_mode.go (1)
ImageProcessMode(148-153)relay/channel/ali/adaptor.go (1)
Adaptor(24-26)dto/openai_image.go (1)
ImageRequest(13-32)common/json.go (1)
MarshalWithoutHTMLEscape(48-62)
🔇 Additional comments (3)
relay/common/relay_info.go (1)
480-480: Confirm whether Prompt is always required across all new modesIf image-to-image or video flows can omit prompt, consider adding omitempty here and validating per mode/adaptor instead.
relay/relay_adaptor.go (1)
39-41: Import change is fineReordered/added gin import is harmless.
relay/channel/ali/image.go (1)
36-39: Delegation to mode.ProcessRequest looks goodCleanly routes mode-specific request shaping when configured; falls back otherwise.
| // Accept only POST /v1/video/generations as "generate" action. | ||
| action := oneapi_constant.TaskActionGenerate | ||
| info.Action = action | ||
|
|
||
| req := relaycommon.TaskSubmitReq{} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Respect mode-specific action
Use videoProcessMode.Action (e.g., textGenerate vs generate) when available.
- action := oneapi_constant.TaskActionGenerate
+ action := oneapi_constant.TaskActionGenerate
+ if ta.videoProcessMode != nil && ta.videoProcessMode.Action != "" {
+ action = ta.videoProcessMode.Action
+ }
info.Action = action📝 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.
| // Accept only POST /v1/video/generations as "generate" action. | |
| action := oneapi_constant.TaskActionGenerate | |
| info.Action = action | |
| req := relaycommon.TaskSubmitReq{} | |
| // Accept only POST /v1/video/generations as "generate" action. | |
| action := oneapi_constant.TaskActionGenerate | |
| if ta.videoProcessMode != nil && ta.videoProcessMode.Action != "" { | |
| action = ta.videoProcessMode.Action | |
| } | |
| info.Action = action | |
| req := relaycommon.TaskSubmitReq{} |
🤖 Prompt for AI Agents
In relay/channel/ali/adaptor.go around lines 181 to 185, the code
unconditionally sets info.Action to the generic TaskActionGenerate; instead, if
a videoProcessMode with a mode-specific Action is available you must use that
(e.g., textGenerate vs generate). Modify the logic to check for the presence of
videoProcessMode (or info.VideoProcessMode) and a non-empty Action field and set
action = videoProcessMode.Action in that case, otherwise fall back to
oneapi_constant.TaskActionGenerate, then assign info.Action = action.
|
|
||
| taskID, ok := body["task_id"].(string) | ||
| if !ok { | ||
| return nil, fmt.Errorf("invalid task_id") | ||
| } | ||
| url := fmt.Sprintf("%s/api/v1/tasks/%s", baseUrl, taskID) | ||
|
|
||
| req, err := http.NewRequest("GET", url, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| req.Header.Set("Authorization", "Bearer "+key) | ||
|
|
||
| client := &http.Client{} | ||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| common.SysLog("updateTask client.Do err: " + err.Error()) | ||
| return nil, err | ||
| } | ||
| return resp, nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout and Accept header for task polling
Prevent hangs and prefer JSON responses.
+ // Optional: set Accept for clarity
+ req.Header.Set("Accept", "application/json")
- client := &http.Client{}
+ client := &http.Client{Timeout: 10 * time.Second}
resp, err := client.Do(req)Add import:
import (
"errors"
"fmt"
"io"
"net/http"
"one-api/common"
oneapi_constant "one-api/constant"
"one-api/dto"
"one-api/model"
@@
"one-api/service"
"one-api/types"
"strings"
+ "time"
)📝 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.
| taskID, ok := body["task_id"].(string) | |
| if !ok { | |
| return nil, fmt.Errorf("invalid task_id") | |
| } | |
| url := fmt.Sprintf("%s/api/v1/tasks/%s", baseUrl, taskID) | |
| req, err := http.NewRequest("GET", url, nil) | |
| if err != nil { | |
| return nil, err | |
| } | |
| req.Header.Set("Authorization", "Bearer "+key) | |
| client := &http.Client{} | |
| resp, err := client.Do(req) | |
| if err != nil { | |
| common.SysLog("updateTask client.Do err: " + err.Error()) | |
| return nil, err | |
| } | |
| return resp, nil | |
| } | |
| // In the imports at the top of relay/channel/ali/adaptor.go | |
| import ( | |
| "errors" | |
| "fmt" | |
| "io" | |
| "net/http" | |
| "one-api/common" | |
| oneapi_constant "one-api/constant" | |
| "one-api/dto" | |
| "one-api/model" | |
| "one-api/service" | |
| "one-api/types" | |
| "strings" | |
| "time" | |
| ) | |
| // … | |
| func (a *Adaptor) updateTask(body map[string]interface{}) (*http.Response, error) { | |
| taskID, ok := body["task_id"].(string) | |
| if !ok { | |
| return nil, fmt.Errorf("invalid task_id") | |
| } | |
| url := fmt.Sprintf("%s/api/v1/tasks/%s", baseUrl, taskID) | |
| req, err := http.NewRequest("GET", url, nil) | |
| if err != nil { | |
| return nil, err | |
| } | |
| req.Header.Set("Authorization", "Bearer "+key) | |
| // Optional: set Accept for clarity | |
| req.Header.Set("Accept", "application/json") | |
| client := &http.Client{Timeout: 10 * time.Second} | |
| resp, err := client.Do(req) | |
| if err != nil { | |
| common.SysLog("updateTask client.Do err: " + err.Error()) | |
| return nil, err | |
| } | |
| return resp, nil | |
| } |
🤖 Prompt for AI Agents
In relay/channel/ali/adaptor.go around lines 246 to 267, the HTTP request for
task polling needs a timeout and should prefer JSON responses: set req.Header
"Accept" to "application/json" and create the client with a timeout (for example
client := &http.Client{Timeout: 10 * time.Second}) to prevent hangs; import the
time package if missing. Ensure the Authorization header remains and return the
resp as before.
| func (ta *TaskAdaptor) ParseTaskResult(responseBody []byte) (*relaycommon.TaskInfo, error) { | ||
|
|
||
| var response VideoGenerationResponse | ||
| err := common.Unmarshal(responseBody, &response) | ||
| if err != nil { | ||
| common.SysLog("updateTask NewDecoder err: " + err.Error()) | ||
| return nil, err | ||
| } | ||
|
|
||
| taskResult := relaycommon.TaskInfo{} | ||
| if response.HasError() { | ||
| taskResult.Code = 5000 // todo uni code | ||
| taskResult.Reason = response.Message | ||
| taskResult.Status = model.TaskStatusFailure | ||
| taskResult.Progress = "100%" | ||
| return &taskResult, nil | ||
| } | ||
|
|
||
| taskResult.Code = 0 | ||
| taskResult.Status = statusMapping[response.Output.TaskStatus] | ||
| switch response.Output.TaskStatus { | ||
| case "PENDING": | ||
| taskResult.Status = model.TaskStatusQueued | ||
| taskResult.Progress = "10%" | ||
| case "RUNNING": | ||
| taskResult.Status = model.TaskStatusInProgress | ||
| taskResult.Progress = "50%" | ||
| case "SUCCEEDED", "FAILED", "CANCELED", "UNKNOWN": | ||
| taskResult.Status = model.TaskStatusSuccess | ||
| taskResult.Progress = "100%" | ||
| } | ||
| taskResult.Url = response.Output.VideoUrl | ||
| return &taskResult, nil | ||
| } |
There was a problem hiding this comment.
Don’t overwrite failure states; set only progress
Switch currently sets SUCCESS for FAILED/CANCELED. Keep status from mapping and set progress only.
taskResult.Code = 0
taskResult.Status = statusMapping[response.Output.TaskStatus]
- switch response.Output.TaskStatus {
- case "PENDING":
- taskResult.Status = model.TaskStatusQueued
- taskResult.Progress = "10%"
- case "RUNNING":
- taskResult.Status = model.TaskStatusInProgress
- taskResult.Progress = "50%"
- case "SUCCEEDED", "FAILED", "CANCELED", "UNKNOWN":
- taskResult.Status = model.TaskStatusSuccess
- taskResult.Progress = "100%"
- }
+ switch response.Output.TaskStatus {
+ case "PENDING":
+ // Align PENDING with internal queued
+ taskResult.Status = model.TaskStatusQueued
+ taskResult.Progress = "10%"
+ case "RUNNING":
+ taskResult.Progress = "50%"
+ default:
+ taskResult.Progress = "100%"
+ }📝 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.
| func (ta *TaskAdaptor) ParseTaskResult(responseBody []byte) (*relaycommon.TaskInfo, error) { | |
| var response VideoGenerationResponse | |
| err := common.Unmarshal(responseBody, &response) | |
| if err != nil { | |
| common.SysLog("updateTask NewDecoder err: " + err.Error()) | |
| return nil, err | |
| } | |
| taskResult := relaycommon.TaskInfo{} | |
| if response.HasError() { | |
| taskResult.Code = 5000 // todo uni code | |
| taskResult.Reason = response.Message | |
| taskResult.Status = model.TaskStatusFailure | |
| taskResult.Progress = "100%" | |
| return &taskResult, nil | |
| } | |
| taskResult.Code = 0 | |
| taskResult.Status = statusMapping[response.Output.TaskStatus] | |
| switch response.Output.TaskStatus { | |
| case "PENDING": | |
| taskResult.Status = model.TaskStatusQueued | |
| taskResult.Progress = "10%" | |
| case "RUNNING": | |
| taskResult.Status = model.TaskStatusInProgress | |
| taskResult.Progress = "50%" | |
| case "SUCCEEDED", "FAILED", "CANCELED", "UNKNOWN": | |
| taskResult.Status = model.TaskStatusSuccess | |
| taskResult.Progress = "100%" | |
| } | |
| taskResult.Url = response.Output.VideoUrl | |
| return &taskResult, nil | |
| } | |
| func (ta *TaskAdaptor) ParseTaskResult(responseBody []byte) (*relaycommon.TaskInfo, error) { | |
| // … previous logic … | |
| taskResult.Code = 0 | |
| taskResult.Status = statusMapping[response.Output.TaskStatus] | |
| switch response.Output.TaskStatus { | |
| case "PENDING": | |
| // Align PENDING with internal queued | |
| taskResult.Status = model.TaskStatusQueued | |
| taskResult.Progress = "10%" | |
| case "RUNNING": | |
| taskResult.Progress = "50%" | |
| default: | |
| taskResult.Progress = "100%" | |
| } | |
| taskResult.Url = response.Output.VideoUrl | |
| return &taskResult, nil | |
| } |
🤖 Prompt for AI Agents
In relay/channel/ali/adaptor.go around lines 279 to 312, the switch currently
overrides the status set from statusMapping and forces terminal states like
FAILED/CANCELED to SUCCESS; change it so the initial taskResult.Status =
statusMapping[...] is authoritative and the switch only adjusts Progress (and
where appropriate Progress+minor status tweaks), e.g., set Progress to "10%" for
PENDING, "50%" for RUNNING, and "100%" for SUCCEEDED/FAILED/CANCELED/UNKNOWN
without changing taskResult.Status; ensure SUCCEEDED remains whatever mapping
provides and FAILED/CANCELED keep their mapped failure statuses while Progress
is set to "100%".
| func selectImageProcessMode(info *relaycommon.RelayInfo) *ImageProcessMode { | ||
| switch info.UpstreamModelName { | ||
| case "qwen-image", "wan2.2-t2i-flash", "wan2.2-t2i-plus", "wanx2.1-t2i-turbo", "wanx2.1-t2i-plus", "wanx2.0-t2i-turbo", "wanx-v1": | ||
| return text2ImageMode() | ||
| case "qwen-image-edit": | ||
| return multimoalGenerationMode() | ||
| case "wanx2.1-imageedit", "wanx-sketch-to-image-lite": | ||
| return image2ImageMode() | ||
| default: | ||
| return nil | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Model identifier typo will disable the intended mode
"wan2.2-…" is likely "wanx2.2-…". As written, these models won’t match and will skip the mode pipeline.
Apply this diff:
- case "qwen-image", "wan2.2-t2i-flash", "wan2.2-t2i-plus", "wanx2.1-t2i-turbo", "wanx2.1-t2i-plus", "wanx2.0-t2i-turbo", "wanx-v1":
+ case "qwen-image", "wanx2.2-t2i-flash", "wanx2.2-t2i-plus", "wanx2.1-t2i-turbo", "wanx2.1-t2i-plus", "wanx2.0-t2i-turbo", "wanx-v1":📝 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.
| func selectImageProcessMode(info *relaycommon.RelayInfo) *ImageProcessMode { | |
| switch info.UpstreamModelName { | |
| case "qwen-image", "wan2.2-t2i-flash", "wan2.2-t2i-plus", "wanx2.1-t2i-turbo", "wanx2.1-t2i-plus", "wanx2.0-t2i-turbo", "wanx-v1": | |
| return text2ImageMode() | |
| case "qwen-image-edit": | |
| return multimoalGenerationMode() | |
| case "wanx2.1-imageedit", "wanx-sketch-to-image-lite": | |
| return image2ImageMode() | |
| default: | |
| return nil | |
| } | |
| } | |
| func selectImageProcessMode(info *relaycommon.RelayInfo) *ImageProcessMode { | |
| switch info.UpstreamModelName { | |
| case "qwen-image", "wanx2.2-t2i-flash", "wanx2.2-t2i-plus", "wanx2.1-t2i-turbo", "wanx2.1-t2i-plus", "wanx2.0-t2i-turbo", "wanx-v1": | |
| return text2ImageMode() | |
| case "qwen-image-edit": | |
| return multimoalGenerationMode() | |
| case "wanx2.1-imageedit", "wanx-sketch-to-image-lite": | |
| return image2ImageMode() | |
| default: | |
| return nil | |
| } | |
| } |
🤖 Prompt for AI Agents
In relay/channel/ali/image.go around lines 23 to 35, there's a model identifier
typo: "wan2.2-…" should be "wanx2.2-…", causing those models to not match and
skip the text2Image pipeline; update the case list to replace "wan2.2-t2i-flash"
with "wanx2.2-t2i-flash" (and any other occurrences of "wan2.2-" to "wanx2.2-")
so the intended models correctly match and return text2ImageMode().
| func copyFromMetaData(req relaycommon.TaskSubmitReq, r any) error { | ||
| metadata := req.Metadata | ||
| medaBytes, err := json.Marshal(metadata) | ||
| if err != nil { | ||
| return errors.Wrap(err, "metadata marshal metadata failed") | ||
| } | ||
| err = json.Unmarshal(medaBytes, &r) | ||
| if err != nil { | ||
| return errors.Wrap(err, "unmarshal metadata failed") | ||
| } |
There was a problem hiding this comment.
Fix metadata copy: Unmarshal into target, not &interface{}
json.Unmarshal(medaBytes, &r) never mutates aliReq; pass r (must be a pointer to struct). Add nil-guard and tighten error text.
func copyFromMetaData(req relaycommon.TaskSubmitReq, r any) error {
- metadata := req.Metadata
- medaBytes, err := json.Marshal(metadata)
+ metadata := req.Metadata
+ if metadata == nil {
+ return nil
+ }
+ medaBytes, err := json.Marshal(metadata)
if err != nil {
- return errors.Wrap(err, "metadata marshal metadata failed")
+ return errors.Wrap(err, "marshal metadata failed")
}
- err = json.Unmarshal(medaBytes, &r)
+ err = json.Unmarshal(medaBytes, r)
if err != nil {
return errors.Wrap(err, "unmarshal metadata failed")
}
return nil
}📝 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.
| func copyFromMetaData(req relaycommon.TaskSubmitReq, r any) error { | |
| metadata := req.Metadata | |
| medaBytes, err := json.Marshal(metadata) | |
| if err != nil { | |
| return errors.Wrap(err, "metadata marshal metadata failed") | |
| } | |
| err = json.Unmarshal(medaBytes, &r) | |
| if err != nil { | |
| return errors.Wrap(err, "unmarshal metadata failed") | |
| } | |
| func copyFromMetaData(req relaycommon.TaskSubmitReq, r any) error { | |
| metadata := req.Metadata | |
| if metadata == nil { | |
| return nil | |
| } | |
| medaBytes, err := json.Marshal(metadata) | |
| if err != nil { | |
| return errors.Wrap(err, "marshal metadata failed") | |
| } | |
| err = json.Unmarshal(medaBytes, r) | |
| if err != nil { | |
| return errors.Wrap(err, "unmarshal metadata failed") | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In relay/channel/ali/video_process_mode.go around lines 62 to 71, the current
code marshals req.Metadata then calls json.Unmarshal(medaBytes, &r) which writes
into the local interface variable rather than the caller's target; change to
unmarshal into r (i.e. json.Unmarshal(medaBytes, r)) and ensure r is non-nil and
is a pointer to the destination struct before calling Marshal/Unmarshal; also
tighten error messages (e.g. "marshal metadata failed" and "unmarshal metadata
into target failed") and return an error if r is nil or not a pointer.
| func videoHandler(ta *TaskAdaptor, c *gin.Context, resp *http.Response, info *relaycommon.RelayInfo) (taskID string, taskData []byte, taskErr *dto.TaskError) { | ||
| responseBody, err := io.ReadAll(resp.Body) | ||
| if err != nil { | ||
| taskErr = service.TaskErrorWrapper(err, "read_response_body_failed", http.StatusInternalServerError) | ||
| return | ||
| } | ||
| _ = resp.Body.Close() | ||
| vgr := &VideoGenerationResponse{} | ||
| err = json.Unmarshal(responseBody, vgr) | ||
| if err != nil { | ||
| taskErr = service.TaskErrorWrapper(errors.Wrapf(err, "body: %s", responseBody), "unmarshal_response_body_failed", http.StatusInternalServerError) | ||
| } | ||
| if vgr.HasError() { | ||
| taskErr = service.TaskErrorWrapper(errors.Errorf("%s(%s)", vgr.Message, vgr.Code), vgr.Code, http.StatusInternalServerError) | ||
| } | ||
| c.JSON(http.StatusOK, gin.H{"task_id": vgr.Output.TaskId}) | ||
| return vgr.Output.TaskId, responseBody, nil | ||
|
|
There was a problem hiding this comment.
Propagate upstream errors; don’t 200/OK on failures
videoHandler sets taskErr but always writes 200 and returns nil error. Return immediately on errors and only write success on happy path. Also use the graceful closer.
func videoHandler(ta *TaskAdaptor, c *gin.Context, resp *http.Response, info *relaycommon.RelayInfo) (taskID string, taskData []byte, taskErr *dto.TaskError) {
responseBody, err := io.ReadAll(resp.Body)
if err != nil {
taskErr = service.TaskErrorWrapper(err, "read_response_body_failed", http.StatusInternalServerError)
- return
+ return "", nil, taskErr
}
- _ = resp.Body.Close()
+ service.CloseResponseBodyGracefully(resp)
vgr := &VideoGenerationResponse{}
- err = json.Unmarshal(responseBody, vgr)
- if err != nil {
+ if err = json.Unmarshal(responseBody, vgr); err != nil {
taskErr = service.TaskErrorWrapper(errors.Wrapf(err, "body: %s", responseBody), "unmarshal_response_body_failed", http.StatusInternalServerError)
+ return "", nil, taskErr
}
if vgr.HasError() {
taskErr = service.TaskErrorWrapper(errors.Errorf("%s(%s)", vgr.Message, vgr.Code), vgr.Code, http.StatusInternalServerError)
+ return "", responseBody, taskErr
}
c.JSON(http.StatusOK, gin.H{"task_id": vgr.Output.TaskId})
return vgr.Output.TaskId, responseBody, nil
}📝 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.
| func videoHandler(ta *TaskAdaptor, c *gin.Context, resp *http.Response, info *relaycommon.RelayInfo) (taskID string, taskData []byte, taskErr *dto.TaskError) { | |
| responseBody, err := io.ReadAll(resp.Body) | |
| if err != nil { | |
| taskErr = service.TaskErrorWrapper(err, "read_response_body_failed", http.StatusInternalServerError) | |
| return | |
| } | |
| _ = resp.Body.Close() | |
| vgr := &VideoGenerationResponse{} | |
| err = json.Unmarshal(responseBody, vgr) | |
| if err != nil { | |
| taskErr = service.TaskErrorWrapper(errors.Wrapf(err, "body: %s", responseBody), "unmarshal_response_body_failed", http.StatusInternalServerError) | |
| } | |
| if vgr.HasError() { | |
| taskErr = service.TaskErrorWrapper(errors.Errorf("%s(%s)", vgr.Message, vgr.Code), vgr.Code, http.StatusInternalServerError) | |
| } | |
| c.JSON(http.StatusOK, gin.H{"task_id": vgr.Output.TaskId}) | |
| return vgr.Output.TaskId, responseBody, nil | |
| func videoHandler(ta *TaskAdaptor, c *gin.Context, resp *http.Response, info *relaycommon.RelayInfo) (taskID string, taskData []byte, taskErr *dto.TaskError) { | |
| responseBody, err := io.ReadAll(resp.Body) | |
| if err != nil { | |
| taskErr = service.TaskErrorWrapper(err, "read_response_body_failed", http.StatusInternalServerError) | |
| return "", nil, taskErr | |
| } | |
| service.CloseResponseBodyGracefully(resp) | |
| vgr := &VideoGenerationResponse{} | |
| if err = json.Unmarshal(responseBody, vgr); err != nil { | |
| taskErr = service.TaskErrorWrapper( | |
| errors.Wrapf(err, "body: %s", responseBody), | |
| "unmarshal_response_body_failed", | |
| http.StatusInternalServerError, | |
| ) | |
| return "", nil, taskErr | |
| } | |
| if vgr.HasError() { | |
| taskErr = service.TaskErrorWrapper( | |
| errors.Errorf("%s(%s)", vgr.Message, vgr.Code), | |
| vgr.Code, | |
| http.StatusInternalServerError, | |
| ) | |
| return "", responseBody, taskErr | |
| } | |
| c.JSON(http.StatusOK, gin.H{"task_id": vgr.Output.TaskId}) | |
| return vgr.Output.TaskId, responseBody, nil | |
| } |
🤖 Prompt for AI Agents
In relay/channel/ali/video_process_mode.go around lines 247 to 264, videoHandler
currently always writes a 200 OK and returns nil even when read/unmarshal or
vgr.HasError() produce taskErr; change the flow to use a graceful closer (defer
resp.Body.Close() right after validating resp.Body), check and handle errors
immediately—after io.ReadAll or json.Unmarshal or if vgr.HasError() set taskErr,
return immediately with the taskErr (do not call c.JSON for success), and only
on the happy path send c.JSON(http.StatusOK, gin.H{"task_id":
vgr.Output.TaskId}) and return vgr.Output.TaskId, responseBody, nil.
|
美元汇率切换这个功能，我们暂时不能接受pr |
出于什么考虑,可以告知一下么; 这里的汇率切换仅仅只是填写倍率时的显示值切换, 保存数据会切换会原来的的倍率信息. |
已移除界面上倍率切换相关功能 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx (1)
109-127: Critical logic error in unset models filter.The filter logic uses AND (
&&) but the comment states OR. Models should be marked as "unset" when they lack price OR ratio, not when they lack both.- // 如果模型没有价格或者没有倍率设置，则显示 - const groupName = !hasPrice && !hasRatio; + // 如果模型没有价格或者没有倍率设置，则显示 + const groupName = !hasPrice || !hasRatio;
🧹 Nitpick comments (2)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx (2)
32-32: Import added but not used in the implementation.The
Selectimport has been added but it's not used anywhere in the component. This suggests incomplete implementation, likely related to the currency selection feature mentioned in the PR objectives.- Select
448-448: Typo in variable name.There's a typo in the variable name
intiialCurrucyTypewhich should beinitialCurrencyType.- let intiialCurrucyType = 'USD' + let initialCurrencyType = 'USD'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
dto/channel_settings.go(1 hunks)relay/common/relay_info.go(1 hunks)relay/relay_adaptor.go(1 hunks)relay/relay_task.go(1 hunks)web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx(16 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- relay/relay_task.go
- dto/channel_settings.go
- relay/common/relay_info.go
- relay/relay_adaptor.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
PR: QuantumNous/new-api#1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the applyModelMapping function transforms the models list by replacing original model names (mapping values) with display names (mapping keys). The database stores this transformed list containing mapped keys. On channel load, data.models contains these mapped display names, making the initialization filter if (data.models.includes(key)) correct.
Applied to files:
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx
🧬 Code graph analysis (1)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx (2)
web/src/helpers/render.jsx (1)
getQuotaPerUnit(870-874)web/src/helpers/utils.jsx (1)
showError(118-147)
🔇 Additional comments (12)
web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx (12)
56-57: State variables for enabled models feature - looks good.Good addition of state management for handling enabled models and hidden model names, which supports the new "unset_models" mode functionality.
62-75: Clean API integration for fetching enabled models.The function properly handles API calls with error handling and internationalization support.
77-80: Good initialization of enabled models on mount.Proper use of useEffect to fetch enabled models when the component mounts.
132-132: Proper dependency management in useEffect.Good addition of
enabledModelNamesto the dependency array to ensure the effect runs when enabled models change.
153-160: Clean helper function for preserving hidden model configurations.The
keepHiddenModelsfunction properly preserves settings for models that are hidden in the current view, preventing data loss.
162-165: Proper integration of hidden models preservation.Good application of the
keepHiddenModelshelper to all three configuration objects to maintain consistency.
218-218: Smart refresh after save operation.Refreshing enabled models after save ensures the UI stays synchronized with backend state changes.
483-488: Form synchronization with proper dependencies.Good implementation of form value synchronization when model data or pricing modes change. The dependencies are correctly specified.
517-527: Conditional UI rendering based on mode.Properly hides the conflict filter checkbox when in "unset_models" mode, which makes sense since unset models wouldn't have conflicts.
589-589: Proper modal state reset before processing.Good practice to reset modal state before adding/updating models to ensure clean state management.
594-594: Form initialization with current model data.Good migration from callback-based form API to ref-based approach with proper initialization.
675-688: Migration to InputNumber components with proper configuration.Good replacement of Input components with InputNumber for numeric fields. The precision, hideButtons, and field name updates are appropriate. The value bindings are correctly updated to match the new field names.
Also applies to: 689-702, 708-718, 719-729, 737-750
| // Calculate and set ratio from token price | ||
| const tokenPrice = parseFloat(currentModel.tokenPrice); | ||
| valuesToSave.ratio = (tokenPrice / 2).toString(); | ||
| valuesToSave.ratio = calculateRatioFromTokenPrice(tokenPrice) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing toString() conversion.
The ratio calculation result should be converted to string for consistency with other fields.
- valuesToSave.ratio = calculateRatioFromTokenPrice(tokenPrice)
+ valuesToSave.ratio = calculateRatioFromTokenPrice(tokenPrice).toString()📝 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.
| valuesToSave.ratio = calculateRatioFromTokenPrice(tokenPrice) | |
| valuesToSave.ratio = calculateRatioFromTokenPrice(tokenPrice).toString() |
🤖 Prompt for AI Agents
In web/src/pages/Setting/Ratio/ModelSettingsVisualEditor.jsx around line 562,
the assignment sets valuesToSave.ratio =
calculateRatioFromTokenPrice(tokenPrice) but the result must be a string;
convert the calculation result to a string before assigning (e.g., use
.toString() or String(...)) so it matches other fields' types and remains
consistent when saved.
|
阿里云qwen-image/wanx系列文生图和编辑图形支持 |
4 participants
PR 类型
PR 是否包含破坏性更新？
PR 描述
API
Summary by CodeRabbit
New Features
Bug Fixes