Feat/qwen minimax tts voice clone - #5198
Closed
youyao666 wants to merge 18 commits into
Closed
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an audio voice-clone relay mode end-to-end: DTOs and validation, unified audio handler, Ali and MiniMax adapters/handlers, model/ratio configuration (voice_clone_unlock_ratio, video_resolution_ratio), quota integration for fixed unlock price, controller/model option wiring, tests, and frontend settings/UI for ratio editing and upstream sync. ChangesVoice-clone relay & routing
Provider adapters and handlers
Pricing, ratios, and quota
Frontend settings and UI
Sequence Diagram(s) sequenceDiagram
participant Client
participant Router
participant RelayController
participant AudioHelper
participant ProviderAdapter
participant Provider
Client->>Router: POST /v1/audio/voice-clone
Router->>RelayController: controller.Relay (RelayModeAudioVoiceClone)
RelayController->>AudioHelper: GetAndValidAudioVoiceCloneRequest
AudioHelper->>AudioHelper: build local dto.AudioRequest
AudioHelper->>ProviderAdapter: ConvertAudioRequest (Ali or MiniMax)
ProviderAdapter->>Provider: POST to provider voice-clone endpoint
Provider-->>ProviderAdapter: JSON/audio response
ProviderAdapter->>AudioHelper: aliVoiceCloneHandler / handleVoiceCloneResponse (usage, fixed price)
AudioHelper->>RelayController: return audio/json to client
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
types/price_data.go (1)
42-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInclude
VoiceCloneUnlockRatioin the debug output.The
ToSetting()method does not include the newly addedVoiceCloneUnlockRatiofield in its formatted output string. For consistency with other ratio fields likeAudioRatioandAudioCompletionRatio, and to aid debugging, this field should be added.🛠️ Proposed fix
func (p *PriceData) ToSetting() string { - return fmt.Sprintf("ModelPrice: %f, ModelRatio: %f, CompletionRatio: %f, CacheRatio: %f, GroupRatio: %f, UsePrice: %t, CacheCreationRatio: %f, CacheCreation5mRatio: %f, CacheCreation1hRatio: %f, QuotaToPreConsume: %d, ImageRatio: %f, AudioRatio: %f, AudioCompletionRatio: %f", p.ModelPrice, p.ModelRatio, p.CompletionRatio, p.CacheRatio, p.GroupRatioInfo.GroupRatio, p.UsePrice, p.CacheCreationRatio, p.CacheCreation5mRatio, p.CacheCreation1hRatio, p.QuotaToPreConsume, p.ImageRatio, p.AudioRatio, p.AudioCompletionRatio) + return fmt.Sprintf("ModelPrice: %f, ModelRatio: %f, CompletionRatio: %f, CacheRatio: %f, GroupRatio: %f, UsePrice: %t, CacheCreationRatio: %f, CacheCreation5mRatio: %f, CacheCreation1hRatio: %f, QuotaToPreConsume: %d, ImageRatio: %f, AudioRatio: %f, AudioCompletionRatio: %f, VoiceCloneUnlockRatio: %f", p.ModelPrice, p.ModelRatio, p.CompletionRatio, p.CacheRatio, p.GroupRatioInfo.GroupRatio, p.UsePrice, p.CacheCreationRatio, p.CacheCreation5mRatio, p.CacheCreation1hRatio, p.QuotaToPreConsume, p.ImageRatio, p.AudioRatio, p.AudioCompletionRatio, p.VoiceCloneUnlockRatio) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@types/price_data.go` at line 42, The ToSetting() method's formatted debug string omits the new VoiceCloneUnlockRatio field; update the fmt.Sprintf call inside ToSetting() to append the VoiceCloneUnlockRatio value (e.g., include p.VoiceCloneUnlockRatio alongside AudioRatio and AudioCompletionRatio) so the returned string reports that ratio for debugging; locate the ToSetting() function in types/price_data.go and modify its format string and argument list to include the new field.controller/ratio_sync.go (1)
384-445:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
VoiceCloneUnlockRatiosupport to type2 upstream parsing.The type2 upstream pricing struct (lines 384–397) and conversion logic (lines 404–445) do not include
VoiceCloneUnlockRatio, even though it was added topricingSyncFields(line 72) and local sync data (line 141). This means upstream pricing endpoints returningvoice_clone_unlock_ratiodata will silently ignore it during sync.🔧 Proposed fix
Add the field to the parsing struct:
var pricingItems []struct { ModelName string `json:"model_name"` QuotaType int `json:"quota_type"` ModelRatio float64 `json:"model_ratio"` ModelPrice float64 `json:"model_price"` CompletionRatio float64 `json:"completion_ratio"` CacheRatio *float64 `json:"cache_ratio"` CreateCacheRatio *float64 `json:"create_cache_ratio"` ImageRatio *float64 `json:"image_ratio"` AudioRatio *float64 `json:"audio_ratio"` AudioCompletionRatio *float64 `json:"audio_completion_ratio"` + VoiceCloneUnlockRatio *float64 `json:"voice_clone_unlock_ratio"` BillingMode string `json:"billing_mode"` BillingExpr string `json:"billing_expr"` }Then add conversion logic after line 444:
if item.AudioCompletionRatio != nil { audioCompletionRatioMap[item.ModelName] = *item.AudioCompletionRatio } + if item.VoiceCloneUnlockRatio != nil { + voiceCloneUnlockRatioMap[item.ModelName] = *item.VoiceCloneUnlockRatio + } }Initialize the map after line 413:
audioRatioMap := make(map[string]float64) audioCompletionRatioMap := make(map[string]float64) + voiceCloneUnlockRatioMap := make(map[string]float64) modelPriceMap := make(map[string]float64)And add it to the converted result after line 478:
if len(audioCompletionRatioMap) > 0 { converted["audio_completion_ratio"] = valueMap(audioCompletionRatioMap) } + if len(voiceCloneUnlockRatioMap) > 0 { + converted["voice_clone_unlock_ratio"] = valueMap(voiceCloneUnlockRatioMap) + } if len(modelPriceMap) > 0 {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/ratio_sync.go` around lines 384 - 445, The type2 upstream parsing struct (pricingItems) is missing the VoiceCloneUnlockRatio field and the sync never stores it; add a VoiceCloneUnlockRatio *float64 `json:"voice_clone_unlock_ratio"` entry to the anonymous pricingItems struct, create and initialize a new voiceCloneUnlockRatioMap (map[string]float64) alongside the other maps (e.g., modelRatioMap, cacheRatioMap), populate voiceCloneUnlockRatioMap[item.ModelName] = *item.VoiceCloneUnlockRatio when item.VoiceCloneUnlockRatio != nil inside the loop, and include this map in the converted result payload the code builds for sync (the same place other maps like imageRatioMap/audioRatioMap/billingModeMap are added).web/default/src/features/system-settings/models/model-ratio-visual-editor.tsx (1)
1069-1083:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd
voiceCloneUnlockRatioto memo equality check.The custom equality function is missing a comparison for
voiceCloneUnlockRatio. This will prevent the component from re-rendering when onlyvoiceCloneUnlockRatiochanges, leading to stale UI.🔧 Proposed fix
(prevProps, nextProps) => { return ( prevProps.modelPrice === nextProps.modelPrice && prevProps.modelRatio === nextProps.modelRatio && prevProps.cacheRatio === nextProps.cacheRatio && prevProps.createCacheRatio === nextProps.createCacheRatio && prevProps.completionRatio === nextProps.completionRatio && prevProps.imageRatio === nextProps.imageRatio && prevProps.audioRatio === nextProps.audioRatio && prevProps.audioCompletionRatio === nextProps.audioCompletionRatio && + prevProps.voiceCloneUnlockRatio === nextProps.voiceCloneUnlockRatio && prevProps.billingMode === nextProps.billingMode && prevProps.billingExpr === nextProps.billingExpr && prevProps.onChange === nextProps.onChange ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/system-settings/models/model-ratio-visual-editor.tsx` around lines 1069 - 1083, The memo equality function used for the component is missing a comparison for voiceCloneUnlockRatio, so add a check like prevProps.voiceCloneUnlockRatio === nextProps.voiceCloneUnlockRatio to the returned conjunction alongside the other prop comparisons in the custom comparator (the arrow function currently comparing modelPrice, modelRatio, cacheRatio, createCacheRatio, completionRatio, imageRatio, audioRatio, audioCompletionRatio, billingMode, billingExpr, onChange) so the component will re-render when voiceCloneUnlockRatio changes.web/default/src/features/system-settings/models/model-ratio-form.tsx (1)
130-150:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPass
voiceCloneUnlockRatioprop toModelRatioVisualEditor.The
ModelRatioVisualEditorcomponent expects avoiceCloneUnlockRatioprop (defined inmodel-ratio-visual-editor.tsxline 79), but it's not being passed here. This will cause the visual editor to be unable to display or edit voice clone unlock pricing.🔧 Proposed fix
<ModelRatioVisualEditor modelPrice={form.watch('ModelPrice')} modelRatio={form.watch('ModelRatio')} cacheRatio={form.watch('CacheRatio')} createCacheRatio={form.watch('CreateCacheRatio')} completionRatio={form.watch('CompletionRatio')} imageRatio={form.watch('ImageRatio')} audioRatio={form.watch('AudioRatio')} audioCompletionRatio={form.watch('AudioCompletionRatio')} + voiceCloneUnlockRatio={form.watch('VoiceCloneUnlockRatio')} billingMode={form.watch('BillingMode')} billingExpr={form.watch('BillingExpr')} onChange={(field, value) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/system-settings/models/model-ratio-form.tsx` around lines 130 - 150, The ModelRatioVisualEditor usage is missing the voiceCloneUnlockRatio prop; update the JSX where ModelRatioVisualEditor is rendered to pass voiceCloneUnlockRatio={form.watch('VoiceCloneUnlockRatio')} so the component (ModelRatioVisualEditor) receives the expected value and can display/edit voice clone unlock pricing; locate the prop list around ModelPrice/ModelRatio and add this prop name/value pair (keeping the existing onChange handling).
🧹 Nitpick comments (2)
relay/channel/task/ali/adaptor.go (1)
581-584: 💤 Low valueConsider explicit rounding for billing precision.
The truncation via
int(float64(...))always rounds toward zero. For billing calculations, this subtly favors the user in both refund and additional-charge scenarios. If precise proportional billing is intended, consider usingmath.Round()for consistent rounding:♻️ Suggested change
+import "math" + // 计算实际应扣额度 // 公式：实际额度 = 预扣额度 × (实际时长 / 请求时长) preConsumedQuota := task.Quota -actualQuota := int(float64(preConsumedQuota) * float64(actualDuration) / float64(requestedDuration)) +actualQuota := int(math.Round(float64(preConsumedQuota) * float64(actualDuration) / float64(requestedDuration)))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/task/ali/adaptor.go` around lines 581 - 584, The current calculation for actualQuota uses int(float64(...)) which truncates toward zero; to ensure consistent billing rounding use math.Round when computing actualQuota: compute the proportional value using float64(preConsumedQuota) * float64(actualDuration) / float64(requestedDuration), apply math.Round(...) to that float, then convert to int and assign to actualQuota (update references: preConsumedQuota, actualQuota, task.Quota, actualDuration, requestedDuration), and add the math import if missing.relay/channel/minimax/adaptor.go (1)
46-55: ⚡ Quick winUse
commonJSON wrappers instead ofencoding/json.The new voice-clone path calls
json.Unmarshal/json.Marshaldirectly. Per project convention, application-level (de)serialization must go through thecommon/json.gowrappers.♻️ Proposed change
- var payload map[string]interface{} - if err := json.Unmarshal(body, &payload); err != nil { + var payload map[string]interface{} + if err := common.Unmarshal(body, &payload); err != nil { return nil, err } delete(payload, "model") - jsonData, err := json.Marshal(payload) + jsonData, err := common.Marshal(payload) if err != nil { return nil, err }As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions from
common/json.go... Do NOT directly import or callencoding/jsonin business code."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/minimax/adaptor.go` around lines 46 - 55, Replace direct calls to encoding/json with the project's common JSON wrappers: use common.JSONUnmarshal(body, &payload) instead of json.Unmarshal(body, &payload) and common.JSONMarshal(payload) instead of json.Marshal(payload); remove the encoding/json import, keep error handling the same, and ensure the resulting jsonData (from common.JSONMarshal) is returned via bytes.NewReader(jsonData) as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dto/audio.go`:
- Around line 55-69: In GetTokenCountMeta (method
AudioVoiceCloneRequest.GetTokenCountMeta) replace the direct call to
json.Unmarshal when decoding r.Input into the local input struct with the
repository wrapper common.Unmarshal: create the same local struct { Text string
`json:"text"` }, call common.Unmarshal(r.Input, &input) and check its error
(instead of json.Unmarshal == nil) before assigning text = input.Text so the
code follows the common/json.go unmarshal wrapper requirement.
In `@relay/channel/ali/tts.go`:
- Around line 135-156: The convertAliVoiceClone function is using the stdlib
json package directly (json.Unmarshal and json.Marshal); replace those calls
with the repository wrappers common.Unmarshal and common.Marshal: after reading
body from storage.Bytes() use common.Unmarshal(body, &payload) to decode into
the payload map, and use common.Marshal(payload) to produce jsonData before
returning the reader; keep the existing error handling and return values
unchanged and reference the payload, jsonData and storage variables when making
the replacement.
- Around line 55-94: In convertOpenAIToAliTTS, replace the direct call to
json.Unmarshal when decoding request.Metadata into the metadata map with the
repository wrapper common.Unmarshal (i.e., call
common.Unmarshal(request.Metadata, &metadata)); ensure you import/use the common
package's Unmarshal function and preserve the existing error handling and error
message text (update the error wrap message if desired) and keep the rest of the
parameters merging logic unchanged; target the json unmarshal at the metadata
variable inside convertOpenAIToAliTTS.
- Around line 96-133: In convertOpenAIToAliMiniMaxTTS, replace the direct call
to json.Unmarshal with the repository wrapper common.Unmarshal to comply with
coding guidelines: locate the metadata unmarshalling block where
json.Unmarshal(request.Metadata, &metadata) is used and change it to use
common.Unmarshal(request.Metadata, &metadata), preserving the existing error
handling and variable types so the rest of the function (building input, aliReq,
and marshalling via common.Marshal) remains unchanged.
In `@relay/channel/minimax/tts.go`:
- Around line 209-216: The code silently swallows JSON unmarshal failures in the
MiniMax voice clone response check: replace the direct encoding/json.Unmarshal
call with the project common JSON wrapper (use common/json.go helpers) and
surface any unmarshal error instead of treating it as success; validate the
parsed MiniMaxVoiceCloneResponse.BaseResp.StatusCode only after successful
unmarshalling and return a clear error (including the unmarshal error) when
parsing fails, referencing the MiniMaxVoiceCloneResponse type and the existing
error-return path that uses types.NewErrorWithStatusCode so billing/success
logic only runs on a valid parsed payload.
In `@web/default/src/features/system-settings/models/model-pricing-sheet.tsx`:
- Around line 217-222: The current implementation mixes a fixed price and a
ratio for voiceCloneUnlock: load assigns data.voiceCloneUnlockRatio directly
(displaying a price), while save uses deriveLaneRatio(price, inputPrice) to
store a ratio, causing mismatches; choose one behavior and make code consistent:
if voiceCloneUnlock is a fixed price, update sync/save logic in syncLaneRatios
(and where deriveLaneRatio is invoked for the voiceCloneUnlock path) to skip
deriveLaneRatio and persist the price directly (keep data.voiceCloneUnlockRatio
as the price shown), otherwise if it should be a ratio, change the load/display
path to call deriveLanePrice(data.voiceCloneUnlockRatio, promptPrice) (same
pattern as audioInput) so the UI shows a price derived from the stored ratio;
adjust only the code paths referencing voiceCloneUnlock,
data.voiceCloneUnlockRatio, syncLaneRatios, deriveLaneRatio, and deriveLanePrice
so round-trip load/save behavior is consistent.
In `@work/enable_minimax_channel.go`:
- Around line 1-11: The three files in the work/ directory (including
work/enable_minimax_channel.go) each declare package main and define func
main(), causing duplicate main symbol build failures; either move each
standalone program into its own subdirectory (e.g.,
work/enable_minimax_channel/main.go) so each has its own package main, or add a
build tag (for example //go:build ignore) at the top of each file to exclude
them from normal builds; alternatively consider removing these one-off scripts
from the PR if they don't belong in the repository.
- Line 12: Replace the hardcoded Windows DB path used in the sql.Open call (db,
err := sql.Open("sqlite", `F:/aicoding/newapi/one-api.db?_busy_timeout=30000`))
by reading the SQLite file location from configuration (e.g. environment
variable ONEAPI_DB_PATH or a flag) and construct the DSN with the ?_busy_timeout
suffix before calling sql.Open; make the same change in query_billing_state.go
and query_tokens.go so all uses of sql.Open use the configurable dbPath (and
fail fast with a clear error if the env/flag is empty).
In `@work/query_tokens.go`:
- Around line 18-31: The code prints raw token credentials via the local
variable key (rows.Scan and fmt.Printf); remove the sensitive value from output
by either (A) dropping key from the SELECT and rows.Scan (e.g., select id, name,
remain_quota, used_quota and stop scanning into key) or (B) keeping it in the
query but never printing it—instead print a non-sensitive identifier such as a
hash or masked prefix (compute a SHA256/sha1 hex or fmt.Sprintf("%s…", key[:4]))
and update fmt.Printf and rows.Scan accordingly so the raw token is never
emitted to stdout/logs.
In `@work/test_tts_billing.ps1`:
- Around line 3-7: Replace hardcoded secrets and absolute paths by reading
values from environment variables or script parameters: stop committing the API
key in $dashKey (read from env like DASH_KEY or a parameter), make $resultPath
relative to $PSScriptRoot or accept a --ResultPath parameter instead of the
absolute "F:\..." path, and parameterize or read $adminUser, $adminPass and
$base from environment variables or script parameters (with safe defaults for
local dev). Update the script to validate that required env/parameters are
present and fail with a clear message if missing.
- Line 61: Remove the unused variable assignment by deleting the call that sets
$selfBefore (the Invoke-Json GET to "$base/api/user/self" assigned to
$selfBefore), since quota is later captured into $selfMid; ensure no other code
relies on $selfBefore and keep the remaining $selfMid-based quota capture
intact.
---
Outside diff comments:
In `@controller/ratio_sync.go`:
- Around line 384-445: The type2 upstream parsing struct (pricingItems) is
missing the VoiceCloneUnlockRatio field and the sync never stores it; add a
VoiceCloneUnlockRatio *float64 `json:"voice_clone_unlock_ratio"` entry to the
anonymous pricingItems struct, create and initialize a new
voiceCloneUnlockRatioMap (map[string]float64) alongside the other maps (e.g.,
modelRatioMap, cacheRatioMap), populate voiceCloneUnlockRatioMap[item.ModelName]
= *item.VoiceCloneUnlockRatio when item.VoiceCloneUnlockRatio != nil inside the
loop, and include this map in the converted result payload the code builds for
sync (the same place other maps like imageRatioMap/audioRatioMap/billingModeMap
are added).
In `@types/price_data.go`:
- Line 42: The ToSetting() method's formatted debug string omits the new
VoiceCloneUnlockRatio field; update the fmt.Sprintf call inside ToSetting() to
append the VoiceCloneUnlockRatio value (e.g., include p.VoiceCloneUnlockRatio
alongside AudioRatio and AudioCompletionRatio) so the returned string reports
that ratio for debugging; locate the ToSetting() function in types/price_data.go
and modify its format string and argument list to include the new field.
In `@web/default/src/features/system-settings/models/model-ratio-form.tsx`:
- Around line 130-150: The ModelRatioVisualEditor usage is missing the
voiceCloneUnlockRatio prop; update the JSX where ModelRatioVisualEditor is
rendered to pass voiceCloneUnlockRatio={form.watch('VoiceCloneUnlockRatio')} so
the component (ModelRatioVisualEditor) receives the expected value and can
display/edit voice clone unlock pricing; locate the prop list around
ModelPrice/ModelRatio and add this prop name/value pair (keeping the existing
onChange handling).
In
`@web/default/src/features/system-settings/models/model-ratio-visual-editor.tsx`:
- Around line 1069-1083: The memo equality function used for the component is
missing a comparison for voiceCloneUnlockRatio, so add a check like
prevProps.voiceCloneUnlockRatio === nextProps.voiceCloneUnlockRatio to the
returned conjunction alongside the other prop comparisons in the custom
comparator (the arrow function currently comparing modelPrice, modelRatio,
cacheRatio, createCacheRatio, completionRatio, imageRatio, audioRatio,
audioCompletionRatio, billingMode, billingExpr, onChange) so the component will
re-render when voiceCloneUnlockRatio changes.
---
Nitpick comments:
In `@relay/channel/minimax/adaptor.go`:
- Around line 46-55: Replace direct calls to encoding/json with the project's
common JSON wrappers: use common.JSONUnmarshal(body, &payload) instead of
json.Unmarshal(body, &payload) and common.JSONMarshal(payload) instead of
json.Marshal(payload); remove the encoding/json import, keep error handling the
same, and ensure the resulting jsonData (from common.JSONMarshal) is returned
via bytes.NewReader(jsonData) as before.
In `@relay/channel/task/ali/adaptor.go`:
- Around line 581-584: The current calculation for actualQuota uses
int(float64(...)) which truncates toward zero; to ensure consistent billing
rounding use math.Round when computing actualQuota: compute the proportional
value using float64(preConsumedQuota) * float64(actualDuration) /
float64(requestedDuration), apply math.Round(...) to that float, then convert to
int and assign to actualQuota (update references: preConsumedQuota, actualQuota,
task.Quota, actualDuration, requestedDuration), and add the math import if
missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec58f03f-41c2-40ca-87c9-c9a777dd7a09
⛔ Files ignored due to path filters (2)
work/newapi.stderr.logis excluded by!**/*.logwork/newapi.stdout.logis excluded by!**/*.log
📒 Files selected for processing (34)
controller/ratio_sync.gocontroller/relay.godto/audio.gorelay/audio_handler.gorelay/channel/ali/adaptor.gorelay/channel/ali/constants.gorelay/channel/ali/dto.gorelay/channel/ali/tts.gorelay/channel/minimax/adaptor.gorelay/channel/minimax/constants.gorelay/channel/minimax/relay-minimax.gorelay/channel/minimax/tts.gorelay/channel/task/ali/adaptor.gorelay/constant/relay_mode.gorelay/helper/valid_request.gorouter/relay-router.gosetting/ratio_setting/exposed_cache.gosetting/ratio_setting/model_ratio.gotypes/price_data.goweb/default/src/features/system-settings/billing/index.tsxweb/default/src/features/system-settings/billing/section-registry.tsxweb/default/src/features/system-settings/models/constants.tsweb/default/src/features/system-settings/models/index.tsxweb/default/src/features/system-settings/models/model-pricing-sheet.tsxweb/default/src/features/system-settings/models/model-ratio-form.tsxweb/default/src/features/system-settings/models/model-ratio-visual-editor.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/features/system-settings/models/upstream-ratio-sync-helpers.tsweb/default/src/features/system-settings/models/upstream-ratio-sync.tsxweb/default/src/features/system-settings/types.tswork/enable_minimax_channel.gowork/query_billing_state.gowork/query_tokens.gowork/test_tts_billing.ps1
Comment on lines
+55
to
+69
| func (r *AudioVoiceCloneRequest) GetTokenCountMeta() *types.TokenCountMeta { | ||
| text := r.Text | ||
| if text == "" && len(r.Input) > 0 { | ||
| var input struct { | ||
| Text string `json:"text"` | ||
| } | ||
| if json.Unmarshal(r.Input, &input) == nil { | ||
| text = input.Text | ||
| } | ||
| } | ||
| return &types.TokenCountMeta{ | ||
| CombineText: text, | ||
| TokenType: types.TokenTypeTextNumber, | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Use common.Unmarshal instead of json.Unmarshal.
Line 61 directly calls json.Unmarshal, which violates the repository coding guideline requiring all JSON marshal/unmarshal operations to use wrapper functions from common/json.go.
🔧 Proposed fix
func (r *AudioVoiceCloneRequest) GetTokenCountMeta() *types.TokenCountMeta {
text := r.Text
if text == "" && len(r.Input) > 0 {
var input struct {
Text string `json:"text"`
}
- if json.Unmarshal(r.Input, &input) == nil {
+ if common.Unmarshal(r.Input, &input) == nil {
text = input.Text
}
}As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions from common/json.go: common.Marshal(), common.Unmarshal(), common.UnmarshalJsonStr(), common.DecodeJson(), or common.GetJsonType(). Do NOT directly import or call encoding/json in business code."
📝 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.
Suggested change
| func (r *AudioVoiceCloneRequest) GetTokenCountMeta() *types.TokenCountMeta { | |
| text := r.Text | |
| if text == "" && len(r.Input) > 0 { | |
| var input struct { | |
| Text string `json:"text"` | |
| } | |
| if json.Unmarshal(r.Input, &input) == nil { | |
| text = input.Text | |
| } | |
| } | |
| return &types.TokenCountMeta{ | |
| CombineText: text, | |
| TokenType: types.TokenTypeTextNumber, | |
| } | |
| } | |
| func (r *AudioVoiceCloneRequest) GetTokenCountMeta() *types.TokenCountMeta { | |
| text := r.Text | |
| if text == "" && len(r.Input) > 0 { | |
| var input struct { | |
| Text string `json:"text"` | |
| } | |
| if common.Unmarshal(r.Input, &input) == nil { | |
| text = input.Text | |
| } | |
| } | |
| return &types.TokenCountMeta{ | |
| CombineText: text, | |
| TokenType: types.TokenTypeTextNumber, | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dto/audio.go` around lines 55 - 69, In GetTokenCountMeta (method
AudioVoiceCloneRequest.GetTokenCountMeta) replace the direct call to
json.Unmarshal when decoding r.Input into the local input struct with the
repository wrapper common.Unmarshal: create the same local struct { Text string
`json:"text"` }, call common.Unmarshal(r.Input, &input) and check its error
(instead of json.Unmarshal == nil) before assigning text = input.Text so the
code follows the common/json.go unmarshal wrapper requirement.
Comment on lines
+209
to
+216
| var cloneResp MiniMaxVoiceCloneResponse | ||
| if unmarshalErr := json.Unmarshal(body, &cloneResp); unmarshalErr == nil && cloneResp.BaseResp.StatusCode != 0 { | ||
| return nil, types.NewErrorWithStatusCode( | ||
| fmt.Errorf("minimax voice clone error: %d - %s", cloneResp.BaseResp.StatusCode, cloneResp.BaseResp.StatusMsg), | ||
| types.ErrorCodeBadResponse, | ||
| http.StatusBadRequest, | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Unmarshal error is silently swallowed.
unmarshalErr == nil && ... means a JSON parse failure is ignored: the code then forwards the body and bills the user. Surface the parse error explicitly so malformed upstream payloads aren't billed as success. Also prefer the common JSON wrapper here per project convention.
🛠️ Proposed change
- var cloneResp MiniMaxVoiceCloneResponse
- if unmarshalErr := json.Unmarshal(body, &cloneResp); unmarshalErr == nil && cloneResp.BaseResp.StatusCode != 0 {
- return nil, types.NewErrorWithStatusCode(
- fmt.Errorf("minimax voice clone error: %d - %s", cloneResp.BaseResp.StatusCode, cloneResp.BaseResp.StatusMsg),
- types.ErrorCodeBadResponse,
- http.StatusBadRequest,
- )
- }
+ var cloneResp MiniMaxVoiceCloneResponse
+ if unmarshalErr := common.Unmarshal(body, &cloneResp); unmarshalErr != nil {
+ return nil, types.NewErrorWithStatusCode(
+ fmt.Errorf("failed to unmarshal minimax voice clone response: %w", unmarshalErr),
+ types.ErrorCodeBadResponseBody,
+ http.StatusInternalServerError,
+ )
+ }
+ if cloneResp.BaseResp.StatusCode != 0 {
+ return nil, types.NewErrorWithStatusCode(
+ fmt.Errorf("minimax voice clone error: %d - %s", cloneResp.BaseResp.StatusCode, cloneResp.BaseResp.StatusMsg),
+ types.ErrorCodeBadResponse,
+ http.StatusBadRequest,
+ )
+ }As per coding guidelines: JSON marshal/unmarshal must use the common/json.go wrappers rather than encoding/json.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@relay/channel/minimax/tts.go` around lines 209 - 216, The code silently
swallows JSON unmarshal failures in the MiniMax voice clone response check:
replace the direct encoding/json.Unmarshal call with the project common JSON
wrapper (use common/json.go helpers) and surface any unmarshal error instead of
treating it as success; validate the parsed
MiniMaxVoiceCloneResponse.BaseResp.StatusCode only after successful
unmarshalling and return a clear error (including the unmarshal error) when
parsing fails, referencing the MiniMaxVoiceCloneResponse type and the existing
error-return path that uses types.NewErrorWithStatusCode so billing/success
logic only runs on a valid parsed payload.
Comment on lines
+1
to
+11
| package main | ||
|
|
||
| import ( | ||
| "database/sql" | ||
| "log" | ||
| "strings" | ||
|
|
||
| _ "modernc.org/sqlite" | ||
| ) | ||
|
|
||
| func main() { |
Contributor
There was a problem hiding this comment.
Build break: three package main files with func main() in work/ won't compile.
work/enable_minimax_channel.go, work/query_billing_state.go, and work/query_tokens.go all declare package main and func main() in the same directory, so Go treats them as one package with duplicate main symbols. golangci-lint confirms main redeclared in this block. The whole work/ package fails to compile, which can break CI for the module.
Each standalone program needs its own package directory (e.g. work/enable_minimax_channel/main.go, work/query_tokens/main.go, …), or use //go:build ignore build tags so they're excluded from the normal build. That said, these look like personal one-off scripts (hardcoded F:/aicoding/... path, direct SQLite access bypassing the project's DB abstraction) — consider whether they belong in the PR at all rather than committing them to main.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 11-11: : # github.com/QuantumNous/new-api/work
work/query_billing_state.go:11:6: main redeclared in this block
work/enable_minimax_channel.go:11:6: other declaration of main
work/query_tokens.go:11:6: main redeclared in this block
work/enable_minimax_channel.go:11:6: other declaration of main
(typecheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@work/enable_minimax_channel.go` around lines 1 - 11, The three files in the
work/ directory (including work/enable_minimax_channel.go) each declare package
main and define func main(), causing duplicate main symbol build failures;
either move each standalone program into its own subdirectory (e.g.,
work/enable_minimax_channel/main.go) so each has its own package main, or add a
build tag (for example //go:build ignore) at the top of each file to exclude
them from normal builds; alternatively consider removing these one-off scripts
from the PR if they don't belong in the repository.
| ) | ||
|
|
||
| func main() { | ||
| db, err := sql.Open("sqlite", `F:/aicoding/newapi/one-api.db?_busy_timeout=30000`) |
Contributor
There was a problem hiding this comment.
Hardcoded absolute Windows DB path.
F:/aicoding/newapi/one-api.db is a developer-machine-specific path that won't exist anywhere else; the program is unusable as committed. Read the path from an env var / flag (os.Getenv, flag.String) instead. This same hardcoded path appears in query_billing_state.go and query_tokens.go.
🔧 Suggested approach
dbPath := os.Getenv("ONEAPI_DB_PATH")
if dbPath == "" {
log.Fatal("ONEAPI_DB_PATH not set")
}
db, err := sql.Open("sqlite", dbPath+"?_busy_timeout=30000")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@work/enable_minimax_channel.go` at line 12, Replace the hardcoded Windows DB
path used in the sql.Open call (db, err := sql.Open("sqlite",
`F:/aicoding/newapi/one-api.db?_busy_timeout=30000`)) by reading the SQLite file
location from configuration (e.g. environment variable ONEAPI_DB_PATH or a flag)
and construct the DSN with the ?_busy_timeout suffix before calling sql.Open;
make the same change in query_billing_state.go and query_tokens.go so all uses
of sql.Open use the configurable dbPath (and fail fast with a clear error if the
env/flag is empty).
Comment on lines
+18
to
+31
| rows, err := db.Query(`select id, name, key, remain_quota, used_quota from tokens order by id desc limit 10`) | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| defer rows.Close() | ||
|
|
||
| for rows.Next() { | ||
| var id int | ||
| var name, key string | ||
| var remainQuota, usedQuota int | ||
| if err := rows.Scan(&id, &name, &key, &remainQuota, &usedQuota); err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| fmt.Printf("%d\t%s\t%s\t%d\t%d\n", id, name, key, remainQuota, usedQuota) |
Contributor
There was a problem hiding this comment.
Avoid printing raw token key values.
The query selects key and prints it to stdout in cleartext. API token keys are credentials; emitting them to terminal/CI logs risks leakage and lands them in shell history or log aggregation. If the script only needs to identify tokens, print a prefix/hash or omit the column entirely.
🔒 Suggested change
- rows, err := db.Query(`select id, name, key, remain_quota, used_quota from tokens order by id desc limit 10`)
+ rows, err := db.Query(`select id, name, remain_quota, used_quota from tokens order by id desc limit 10`)
...
- var id int
- var name, key string
- var remainQuota, usedQuota int
- if err := rows.Scan(&id, &name, &key, &remainQuota, &usedQuota); err != nil {
+ var id int
+ var name string
+ var remainQuota, usedQuota int
+ if err := rows.Scan(&id, &name, &remainQuota, &usedQuota); err != nil {
log.Fatal(err)
}
- fmt.Printf("%d\t%s\t%s\t%d\t%d\n", id, name, key, remainQuota, usedQuota)
+ fmt.Printf("%d\t%s\t%d\t%d\n", id, name, remainQuota, usedQuota)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@work/query_tokens.go` around lines 18 - 31, The code prints raw token
credentials via the local variable key (rows.Scan and fmt.Printf); remove the
sensitive value from output by either (A) dropping key from the SELECT and
rows.Scan (e.g., select id, name, remain_quota, used_quota and stop scanning
into key) or (B) keeping it in the query but never printing it—instead print a
non-sensitive identifier such as a hash or masked prefix (compute a SHA256/sha1
hex or fmt.Sprintf("%s…", key[:4])) and update fmt.Printf and rows.Scan
accordingly so the raw token is never emitted to stdout/logs.
Comment on lines
+3
to
+7
| $base = "http://127.0.0.1:3000" | ||
| $adminUser = "root" | ||
| $adminPass = "Test12345!" | ||
| $dashKey = "sk-65cef572bc6e4b06ab77b5e768bac6c2" | ||
| $resultPath = "F:\aicoding\newapi\work\test_tts_billing_result.json" |
Contributor
There was a problem hiding this comment.
Avoid committing hardcoded secrets and absolute paths.
Several issues with this configuration block:
-
Line 6: The hardcoded API key
sk-65cef...should not be committed to source control, even in test scripts. Use environment variables or a local config file excluded from version control. -
Line 7: The absolute Windows path
F:\aicoding\newapi\work\...breaks portability. Use a relative path or derive from$PSScriptRoot. -
Lines 4-5: Hardcoded credentials are acceptable for local test scripts but consider parameterizing or using environment variables.
Proposed fix
$base = "http://127.0.0.1:3000"
-$adminUser = "root"
-$adminPass = "Test12345!"
-$dashKey = "sk-65cef572bc6e4b06ab77b5e768bac6c2"
-$resultPath = "F:\aicoding\newapi\work\test_tts_billing_result.json"
+$adminUser = $env:TTS_TEST_ADMIN_USER ?? "root"
+$adminPass = $env:TTS_TEST_ADMIN_PASS ?? "Test12345!"
+$dashKey = $env:DASHSCOPE_API_KEY
+if (-not $dashKey) { throw "DASHSCOPE_API_KEY environment variable required" }
+$resultPath = Join-Path $PSScriptRoot "test_tts_billing_result.json"📝 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.
Suggested change
| $base = "http://127.0.0.1:3000" | |
| $adminUser = "root" | |
| $adminPass = "Test12345!" | |
| $dashKey = "sk-65cef572bc6e4b06ab77b5e768bac6c2" | |
| $resultPath = "F:\aicoding\newapi\work\test_tts_billing_result.json" | |
| $base = "http://127.0.0.1:3000" | |
| $adminUser = $env:TTS_TEST_ADMIN_USER ?? "root" | |
| $adminPass = $env:TTS_TEST_ADMIN_PASS ?? "Test12345!" | |
| $dashKey = $env:DASHSCOPE_API_KEY | |
| if (-not $dashKey) { throw "DASHSCOPE_API_KEY environment variable required" } | |
| $resultPath = Join-Path $PSScriptRoot "test_tts_billing_result.json" |
🧰 Tools
🪛 Betterleaks (1.3.1)
[high] 6-6: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@work/test_tts_billing.ps1` around lines 3 - 7, Replace hardcoded secrets and
absolute paths by reading values from environment variables or script
parameters: stop committing the API key in $dashKey (read from env like DASH_KEY
or a parameter), make $resultPath relative to $PSScriptRoot or accept a
--ResultPath parameter instead of the absolute "F:\..." path, and parameterize
or read $adminUser, $adminPass and $base from environment variables or script
parameters (with safe defaults for local dev). Update the script to validate
that required env/parameters are present and fail with a clear message if
missing.
| $userId = [int]$loginResp.data.id | ||
| $authHeaders = @{ "New-Api-User" = "$userId" } | ||
|
|
||
| $selfBefore = Invoke-Json -Method "GET" -Url "$base/api/user/self" -WebSession $session -Headers $authHeaders |
Contributor
There was a problem hiding this comment.
Remove unused variable.
$selfBefore is assigned but never used. The quota capture happens later at line 114 via $selfMid. Remove this line to avoid confusion.
Proposed fix
-$selfBefore = Invoke-Json -Method "GET" -Url "$base/api/user/self" -WebSession $session -Headers $authHeaders📝 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.
Suggested change
| $selfBefore = Invoke-Json -Method "GET" -Url "$base/api/user/self" -WebSession $session -Headers $authHeaders |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] 61-61: The variable 'selfBefore' is assigned but never used.
(PSUseDeclaredVarsMoreThanAssignments)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@work/test_tts_billing.ps1` at line 61, Remove the unused variable assignment
by deleting the call that sets $selfBefore (the Invoke-Json GET to
"$base/api/user/self" assigned to $selfBefore), since quota is later captured
into $selfMid; ensure no other code relies on $selfBefore and keep the remaining
$selfMid-based quota capture intact.
…port - Fix aliVoiceCloneHandler to properly handle unmarshal errors - Use cloneResp.Usage.Count instead of estimated prompt tokens to avoid overcharging Qwen list/delete operations - Set VoiceCloneFixedPrice context for MiniMax speech model clone responses - Add ContextKeyVoiceCloneFixedPrice and fixed-price billing in PostAudioConsumeQuota - Add unit test for MiniMax voice clone unlock price setting - Add VoiceCloneUnlockRatio support in classic frontend pricing settings - Fix classic frontend build compatibility (pin deps, remove vitePluginSemi, add icon fallbacks for new providers) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… and upstream error Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
youyao666
force-pushed
the
feat/qwen-minimax-tts-voice-clone
branch
from
May 30, 2026 16:54
4160d22 to
e435519
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@web/default/src/features/system-settings/models/model-ratio-visual-editor.tsx`:
- Line 79: The memo equality function for ModelRatioVisualEditor is missing a
comparison for voiceCloneUnlockRatio causing stale renders; update the custom
memo comparator (the equality function passed to React.memo used around
ModelRatioVisualEditor) to include a strict comparison of
prevProps.voiceCloneUnlockRatio !== nextProps.voiceCloneUnlockRatio (or the
equivalent path where voiceCloneUnlockRatio is read) alongside the existing prop
comparisons so any change to voiceCloneUnlockRatio triggers a re-render.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d4f9b06d-8ea2-40e4-a75c-9bf2f3f13f14
📒 Files selected for processing (33)
controller/option.gocontroller/ratio_sync.gomodel/option.gorelay/channel/ali/constants.gorelay/channel/ali/tts.gorelay/channel/ali/tts_test.gorelay/channel/minimax/tts.gorelay/channel/task/ali/adaptor.goservice/quota.gosetting/ratio_setting/exposed_cache.gosetting/ratio_setting/model_ratio.gotypes/price_data.goweb/classic/package.jsonweb/classic/src/components/settings/RatioSetting.jsxweb/classic/src/helpers/render.jsxweb/classic/src/pages/Home/index.jsxweb/classic/src/pages/Setting/Ratio/ModelRatioSettings.jsxweb/classic/src/pages/Setting/Ratio/UpstreamRatioSync.jsxweb/classic/src/pages/Setting/Ratio/components/ModelPricingEditor.jsxweb/classic/src/pages/Setting/Ratio/hooks/useModelPricingEditorState.jsweb/classic/vite.config.jsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsxweb/default/src/features/system-settings/billing/index.tsxweb/default/src/features/system-settings/billing/section-registry.tsxweb/default/src/features/system-settings/models/constants.tsweb/default/src/features/system-settings/models/index.tsxweb/default/src/features/system-settings/models/model-pricing-sheet.tsxweb/default/src/features/system-settings/models/model-ratio-form.tsxweb/default/src/features/system-settings/models/model-ratio-visual-editor.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/features/system-settings/models/upstream-ratio-sync-helpers.tsweb/default/src/features/system-settings/models/upstream-ratio-sync.tsxweb/default/src/features/system-settings/types.ts
💤 Files with no reviewable changes (1)
- web/classic/vite.config.js
✅ Files skipped from review due to trivial changes (4)
- web/default/src/features/system-settings/models/constants.ts
- web/default/src/features/models/components/drawers/model-mutate-drawer.tsx
- web/classic/src/components/settings/RatioSetting.jsx
- web/default/src/features/system-settings/models/upstream-ratio-sync-helpers.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- web/default/src/features/system-settings/models/index.tsx
- web/default/src/features/system-settings/billing/section-registry.tsx
- controller/option.go
- web/default/src/features/system-settings/types.ts
- controller/ratio_sync.go
- web/default/src/features/system-settings/models/upstream-ratio-sync.tsx
- web/default/src/features/system-settings/models/model-ratio-form.tsx
- web/classic/src/pages/Setting/Ratio/components/ModelPricingEditor.jsx
- web/default/src/features/system-settings/billing/index.tsx
- model/option.go
- web/classic/src/pages/Home/index.jsx
- types/price_data.go
- web/classic/package.json
- relay/channel/minimax/tts.go
- relay/channel/task/ali/adaptor.go
- web/default/src/features/system-settings/models/ratio-settings-card.tsx
- service/quota.go
- relay/channel/ali/constants.go
- web/classic/src/pages/Setting/Ratio/UpstreamRatioSync.jsx
- web/classic/src/helpers/render.jsx
- web/default/src/features/system-settings/models/model-pricing-sheet.tsx
- relay/channel/ali/tts.go
- setting/ratio_setting/model_ratio.go
- web/classic/src/pages/Setting/Ratio/hooks/useModelPricingEditorState.js
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relay/channel/task/ali/adaptor.go`:
- Around line 249-257: The current branch uses
ratio_setting.GetVideoResolutionRatio(aliReq.Model) and if that map exists but
lacks the specific resolution it stops and leaves otherRatios unset; change the
logic so when configRatios is present but configRatios[resolution] is missing
you fall back to the built-in map aliRatios for the same aliReq.Model and
resolution; in other words, inside the branch that checks configRatios (from
ratio_setting.GetVideoResolutionRatio) attempt to read configRatios[resolution]
and if not found then check aliRatios[aliReq.Model][resolution] before giving
up, and in either case set otherRatios[fmt.Sprintf("resolution-%s", resolution)]
to the found ratio.
In `@web/default/src/features/system-settings/models/model-pricing-sheet.tsx`:
- Around line 229-234: The videoResolution lane is currently treated like a
price (rendered by PriceLane, validated by numericDraftRegex and mapped in
laneConfigs.map()), which breaks JSON values; change handling so videoResolution
is rendered with a JSON editor instead of PriceLane: update the map at
laneConfigs.map() to filter out the item with key 'videoResolution' and render
it separately using the JSON editor component (or a dedicated ModelPricingSheet
JSON field renderer) and remove/skip numericDraftRegex validation for that key
so JSON characters are accepted and editable.
In `@web/default/src/features/system-settings/models/upstream-ratio-sync.tsx`:
- Around line 351-353: The VideoResolutionRatio parsing expects a nested shape
Record<string, Record<string, number>> but the generic sync loop currently
writes finalRatios[optionKey][model] = value as if it were a scalar, which
overwrites the entire nested object; change the sync logic for
VideoResolutionRatio so it treats the inner map correctly — when updating an
upstream resolution for a given model, ensure you initialize
finalRatios[optionKey][model] as an object if missing and set
finalRatios[optionKey][model][resolution] = value (merging/preserving existing
resolution entries) instead of assigning the primitive value; apply the same fix
to the other similar blocks referenced around the 388-440 range that handle
nested resolution maps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9be4054a-4f35-429c-86a7-e3f055d25f2e
📒 Files selected for processing (24)
controller/option.gocontroller/ratio_sync.gomodel/option.gorelay/channel/task/ali/adaptor.gorelay/channel/task/ali/billing_test.gosetting/ratio_setting/exposed_cache.gosetting/ratio_setting/model_ratio.goweb/classic/src/components/settings/RatioSetting.jsxweb/classic/src/pages/Setting/Ratio/ModelRatioSettings.jsxweb/classic/src/pages/Setting/Ratio/UpstreamRatioSync.jsxweb/classic/src/pages/Setting/Ratio/components/ModelPricingEditor.jsxweb/classic/src/pages/Setting/Ratio/hooks/useModelPricingEditorState.jsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsxweb/default/src/features/system-settings/billing/index.tsxweb/default/src/features/system-settings/billing/section-registry.tsxweb/default/src/features/system-settings/models/constants.tsweb/default/src/features/system-settings/models/index.tsxweb/default/src/features/system-settings/models/model-pricing-sheet.tsxweb/default/src/features/system-settings/models/model-ratio-form.tsxweb/default/src/features/system-settings/models/model-ratio-visual-editor.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/features/system-settings/models/upstream-ratio-sync-helpers.tsweb/default/src/features/system-settings/models/upstream-ratio-sync.tsxweb/default/src/features/system-settings/types.ts
✅ Files skipped from review due to trivial changes (1)
- web/default/src/features/system-settings/billing/index.tsx
🚧 Files skipped from review as they are similar to previous changes (13)
- web/classic/src/components/settings/RatioSetting.jsx
- web/default/src/features/system-settings/models/constants.ts
- setting/ratio_setting/exposed_cache.go
- web/default/src/features/system-settings/models/index.tsx
- controller/ratio_sync.go
- controller/option.go
- web/default/src/features/system-settings/billing/section-registry.tsx
- web/default/src/features/system-settings/types.ts
- web/default/src/features/system-settings/models/model-ratio-form.tsx
- model/option.go
- web/classic/src/pages/Setting/Ratio/UpstreamRatioSync.jsx
- web/classic/src/pages/Setting/Ratio/components/ModelPricingEditor.jsx
- web/default/src/features/system-settings/models/ratio-settings-card.tsx
Comment on lines
+249
to
257
| // 优先使用管理员配置的分辨率倍率，未配置时回退到内置硬编码 | ||
| if configRatios, ok := ratio_setting.GetVideoResolutionRatio(aliReq.Model); ok { | ||
| if ratio, ok := configRatios[resolution]; ok { | ||
| otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio | ||
| } | ||
| } else if otherRatio, ok := aliRatios[aliReq.Model]; ok { | ||
| if ratio, ok := otherRatio[resolution]; ok { | ||
| otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio | ||
| } |
Contributor
There was a problem hiding this comment.
Fall back per resolution, not just per model.
If the admin override exists for aliReq.Model but omits the requested resolution, this path now leaves otherRatios empty and the task is billed without any resolution multiplier. Please keep the built-in/default lookup as a fallback when the selected resolution key is missing.
Suggested fix
- if configRatios, ok := ratio_setting.GetVideoResolutionRatio(aliReq.Model); ok {
- if ratio, ok := configRatios[resolution]; ok {
- otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio
- }
- } else if otherRatio, ok := aliRatios[aliReq.Model]; ok {
+ if configRatios, ok := ratio_setting.GetVideoResolutionRatio(aliReq.Model); ok {
+ if ratio, ok := configRatios[resolution]; ok {
+ otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio
+ return otherRatios, nil
+ }
+ }
+ if otherRatio, ok := aliRatios[aliReq.Model]; ok {
if ratio, ok := otherRatio[resolution]; ok {
otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio
}
}📝 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.
Suggested change
| // 优先使用管理员配置的分辨率倍率，未配置时回退到内置硬编码 | |
| if configRatios, ok := ratio_setting.GetVideoResolutionRatio(aliReq.Model); ok { | |
| if ratio, ok := configRatios[resolution]; ok { | |
| otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio | |
| } | |
| } else if otherRatio, ok := aliRatios[aliReq.Model]; ok { | |
| if ratio, ok := otherRatio[resolution]; ok { | |
| otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio | |
| } | |
| // 优先使用管理员配置的分辨率倍率，未配置时回退到内置硬编码 | |
| if configRatios, ok := ratio_setting.GetVideoResolutionRatio(aliReq.Model); ok { | |
| if ratio, ok := configRatios[resolution]; ok { | |
| otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio | |
| } else if otherRatio, ok := aliRatios[aliReq.Model]; ok { | |
| if ratio, ok := otherRatio[resolution]; ok { | |
| otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio | |
| } | |
| } | |
| } else if otherRatio, ok := aliRatios[aliReq.Model]; ok { | |
| if ratio, ok := otherRatio[resolution]; ok { | |
| otherRatios[fmt.Sprintf("resolution-%s", resolution)] = ratio | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@relay/channel/task/ali/adaptor.go` around lines 249 - 257, The current branch
uses ratio_setting.GetVideoResolutionRatio(aliReq.Model) and if that map exists
but lacks the specific resolution it stops and leaves otherRatios unset; change
the logic so when configRatios is present but configRatios[resolution] is
missing you fall back to the built-in map aliRatios for the same aliReq.Model
and resolution; in other words, inside the branch that checks configRatios (from
ratio_setting.GetVideoResolutionRatio) attempt to read configRatios[resolution]
and if not found then check aliRatios[aliReq.Model][resolution] before giving
up, and in either case set otherRatios[fmt.Sprintf("resolution-%s", resolution)]
to the found ratio.
Comment on lines
+229
to
+234
| { | ||
| key: 'videoResolution', | ||
| titleKey: 'Video resolution ratio', | ||
| descriptionKey: 'Model-specific resolution multipliers (e.g., 720P: 2, 1080P: 3.33).', | ||
| placeholder: '{}', | ||
| }, |
Contributor
There was a problem hiding this comment.
videoResolution lane cannot be edited due to data type mismatch.
The videoResolution lane is configured as a JSON object (placeholder '{}', description mentions 720P: 2, 1080P: 3.33), but it's rendered using PriceLane which:
- Prepends
$to the value - Validates input with
numericDraftRegex(/^(\d+(\.\d*)?|\.\d*)?$/) which rejects JSON input - Displays as a price input instead of a JSON editor
Users will see malformed display (e.g., ${"720P": 2}) and cannot edit the value because JSON characters are rejected by the regex.
Consider either:
- Rendering
videoResolutionwith a JSON editor instead ofPriceLane, or - Filtering it out of
laneConfigs.map()at line 867 and handling it separately
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/default/src/features/system-settings/models/model-pricing-sheet.tsx`
around lines 229 - 234, The videoResolution lane is currently treated like a
price (rendered by PriceLane, validated by numericDraftRegex and mapped in
laneConfigs.map()), which breaks JSON values; change handling so videoResolution
is rendered with a JSON editor instead of PriceLane: update the map at
laneConfigs.map() to filter out the item with key 'videoResolution' and render
it separately using the JSON editor component (or a dedicated ModelPricingSheet
JSON field renderer) and remove/skip numericDraftRegex validation for that key
so JSON characters are accepted and editable.
Comment on lines
+351
to
+353
| VideoResolutionRatio: parseJsonRecord<Record<string, number>>( | ||
| modelRatios.VideoResolutionRatio | ||
| ), |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
VideoResolutionRatio still syncs the wrong JSON shape.
This field is parsed as { [model]: { [resolution]: number } }, but the generic sync loop writes finalRatios[optionKey][model] = value like a scalar option. Selecting a single upstream resolution will therefore replace the whole nested object with one primitive value and drop the other resolution entries for that model.
Also applies to: 388-440
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/default/src/features/system-settings/models/upstream-ratio-sync.tsx`
around lines 351 - 353, The VideoResolutionRatio parsing expects a nested shape
Record<string, Record<string, number>> but the generic sync loop currently
writes finalRatios[optionKey][model] = value as if it were a scalar, which
overwrites the entire nested object; change the sync logic for
VideoResolutionRatio so it treats the inner map correctly — when updating an
upstream resolution for a given model, ensure you initialize
finalRatios[optionKey][model] as an object if missing and set
finalRatios[optionKey][model][resolution] = value (merging/preserving existing
resolution entries) instead of assigning the primitive value; apply the same fix
to the other similar blocks referenced around the 388-440 range that handle
nested resolution maps.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docker-build-branch.yml:
- Around line 24-25: The workflow step "Set up Docker Buildx" currently uses the
unpinned reference docker/setup-buildx-action@v3 which is flagged as a
supply-chain risk; update the uses field in that step to the action pinned to a
commit SHA (e.g., docker/setup-buildx-action@<commit-sha>) so the action is
immutably referenced, ensuring you replace the `@v3` tag with the specific SHA for
the version you intend to use and commit the change; locate the step by the step
name "Set up Docker Buildx" to make this edit.
- Around line 18-19: Update the "Check out" GitHub Action step to pin the
checkout action to a specific commit SHA instead of the tag (`@v4`) and add
persist-credentials: false to the step; specifically replace the uses reference
for actions/checkout (the step named "Check out") with the corresponding
SHA-pinned ref and add the persist-credentials: false key under that step to
disable credential persistence.
- Around line 27-34: The workflow step named "Build (no push)" currently uses
the unpinned action reference docker/build-push-action@v6 and sets push: false
and load: false; update the uses value to the specific commit SHA (pin the
action to the same v6 SHA used in .github/workflows/docker-image-alpha.yml) to
mitigate supply-chain risk, and add a brief comment above or next to the step
explaining that push: false and load: false are intentional to perform
build/validation-only (no publish or runner image load).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b503ce1c-54a3-40c3-8692-b879c4597ef3
📒 Files selected for processing (1)
.github/workflows/docker-build-branch.yml
Comment on lines
+18
to
+19
| - name: Check out | ||
| uses: actions/checkout@v4 |
Contributor
There was a problem hiding this comment.
Security: Pin action to SHA hash and disable credential persistence.
The checkout action has two security concerns:
- Unpinned action reference: The action uses a tag reference (
@v4) instead of a SHA hash, creating supply chain attack risk. As per static analysis, pinning to hashes is required by blanket policy. - Missing credential protection: Without
persist-credentials: false, credentials may be exposed through GitHub Actions artifacts.
🔒 Proposed security hardening
- name: Check out
- uses: actions/checkout@v4
+ uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+ with:
+ persist-credentials: falseAs per static analysis hints: unpinned action reference (unpinned-uses) and credential persistence through GitHub Actions artifacts (artipacked).
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 18-19: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 19-19: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docker-build-branch.yml around lines 18 - 19, Update the
"Check out" GitHub Action step to pin the checkout action to a specific commit
SHA instead of the tag (`@v4`) and add persist-credentials: false to the step;
specifically replace the uses reference for actions/checkout (the step named
"Check out") with the corresponding SHA-pinned ref and add the
persist-credentials: false key under that step to disable credential
persistence.
Comment on lines
+24
to
+25
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 |
Contributor
There was a problem hiding this comment.
Security: Pin action to SHA hash.
The Docker Buildx setup action uses a tag reference (@v3) instead of a SHA hash, creating supply chain attack risk. As per static analysis, pinning to hashes is required by blanket policy.
🔒 Proposed fix
- name: Set up Docker Buildx
- uses: docker/setup-buildx-action@v3
+ uses: docker/setup-buildx-action@c47758b77c9736f4b2ef4073d4d51994fabfe349 # v3.7.1As per static analysis hints: unpinned action reference (unpinned-uses).
📝 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.
Suggested change
| - name: Set up Docker Buildx | |
| uses: docker/setup-buildx-action@v3 | |
| - name: Set up Docker Buildx | |
| uses: docker/setup-buildx-action@c47758b77c9736f4b2ef4073d4d51994fabfe349 # v3.7.1 |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 25-25: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docker-build-branch.yml around lines 24 - 25, The workflow
step "Set up Docker Buildx" currently uses the unpinned reference
docker/setup-buildx-action@v3 which is flagged as a supply-chain risk; update
the uses field in that step to the action pinned to a commit SHA (e.g.,
docker/setup-buildx-action@<commit-sha>) so the action is immutably referenced,
ensuring you replace the `@v3` tag with the specific SHA for the version you
intend to use and commit the change; locate the step by the step name "Set up
Docker Buildx" to make this edit.
Comment on lines
+27
to
+34
| - name: Build (no push) | ||
| uses: docker/build-push-action@v6 | ||
| with: | ||
| context: . | ||
| push: false | ||
| load: false | ||
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the workflow file and print the relevant section with line numbers
ls -la .github/workflows || true
nl -ba .github/workflows/docker-build-branch.yml | sed -n '1,120p'
# Search for other usages of docker/build-push-action in the repo
rg -n "docker/build-push-action@" -S . || trueRepository: QuantumNous/new-api
Length of output: 779
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the workflow file with line numbers (cat -n is available)
cat -n .github/workflows/docker-build-branch.yml | sed -n '1,160p'
# Find all docker/build-push-action usages in the repo
rg -n "docker/build-push-action@" -S .github/workflows || trueRepository: QuantumNous/new-api
Length of output: 1443
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant sections around docker/build-push-action usage
for f in \
.github/workflows/docker-build.yml \
.github/workflows/docker-image-nightly.yml \
.github/workflows/docker-image-alpha.yml \
.github/workflows/docker-build-branch.yml
do
echo "===== $f ====="
# Print ~40 lines around first match
line=$(rg -n "docker/build-push-action@" "$f" | head -n1 | cut -d: -f1)
start=$(( line>25 ? line-25 : 1 ))
end=$(( line+40 ))
sed -n "${start},${end}p" "$f" | nl -ba -w2 -s: | sed "s/^/${f##*\/}:/"
doneRepository: QuantumNous/new-api
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
.github/workflows/docker-build.yml \
.github/workflows/docker-image-nightly.yml \
.github/workflows/docker-image-alpha.yml \
.github/workflows/docker-build-branch.yml
do
echo "===== $f ====="
# Print ~50 lines around first occurrence
line=$(rg -n "docker/build-push-action@" "$f" | head -n1 | cut -d: -f1 || true)
if [ -z "${line:-}" ]; then
echo "(no match)"
continue
fi
start=$(( line>25 ? line-25 : 1 ))
end=$(( line+60 ))
awk -v start="$start" -v end="$end" 'NR>=start && NR<=end {printf "%4d:%s\n", NR, $0}' "$f"
doneRepository: QuantumNous/new-api
Length of output: 11108
Security: Pin docker/build-push-action to a commit SHA; clarify validation-only intent
- Unpinned action reference:
.github/workflows/docker-build-branch.ymlusesdocker/build-push-action@v6(tag) instead of a commit SHA; this creates a supply-chain risk. Pin it (the repo already pins v6 to a SHA in.github/workflows/docker-image-alpha.yml). - No publish/load: With
push: falseandload: false, the step won’t publish to a registry or load the image into the runner—only build/validation and cache behavior. If that’s intended, add a brief comment explaining it.
🧰 Tools
🪛 zizmor (1.25.2)
[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docker-build-branch.yml around lines 27 - 34, The workflow
step named "Build (no push)" currently uses the unpinned action reference
docker/build-push-action@v6 and sets push: false and load: false; update the
uses value to the specific commit SHA (pin the action to the same v6 SHA used in
.github/workflows/docker-image-alpha.yml) to mitigate supply-chain risk, and add
a brief comment above or next to the step explaining that push: false and load:
false are intentional to perform build/validation-only (no publish or runner
image load).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rontends Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add VoiceCloneUnlockRatio field to Pricing struct - Populate voice clone unlock ratio in updatePricing() - Display voice clone unlock in calculateModelPrice and getModelPriceItems - Add i18n entry for voice clone unlock label Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
API Key Leak Detected! @youyao666 Your API key has been exposed in this pr. Service: DashScope Immediate Actions Required:
Source: #5198 This message was sent by LLMApiCheckBot - Repository: Colorful-glassblock/Dont-Be-Stupid-Leaker |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
New Features
Bug Fixes
Tests