feat: sora 增加参数校验与计费 - #2006
Conversation
WalkthroughIntroduces sora-2/sora-2-pro handling in multipart validation, augments PriceData with OtherRatios, switches default pricing lookups from ratio to price maps, applies multi-parameter price adjustments during task submission, and exposes default model price map with new entries for sora models. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant R as Relay (ValidateMultipartDirect)
participant V as Validator
participant P as PriceData
C->>R: multipart/form-data (model, seconds?, size?)
R->>V: Parse form fields
V->>V: If model startsWith "sora-2"
alt sora-2/sora-2-pro
V->>V: Set defaults (seconds=4, size=720x1280)
V->>V: Override from form if present
V->>V: Validate size against allowed set
alt invalid size
V-->>R: error task_invalid_size
R-->>C: 400 with error
else valid
V->>P: Update OtherRatios: {"seconds": s, "size": 1 or 1.666667}
R-->>C: proceed
end
else other models
V-->>R: proceed (existing flow)
R-->>C: proceed
end
sequenceDiagram
autonumber
participant RT as RelayTask
participant RS as ratio_setting
participant PH as Price Helper
participant LG as Logger
RT->>PH: Get per-call model price
alt configured
PH-->>RT: price
else not configured
PH->>RS: GetDefaultModelPriceMap()
RS-->>PH: default price map
PH-->>RT: default price or 0.1 if absent
end
RT->>RT: baseRatio = price
RT->>RT: For each (k,v) in OtherRatios where v != 1.0: baseRatio *= v
RT->>LG: log model, price, OtherRatios (non-1.0)
RT-->>LG: deferred consume log includes parameters
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
types/price_data.go (1)
32-34: Consider adding OtherRatios to ToSetting() for consistency.The
ToSetting()method doesn't include the newOtherRatiosfield, which may hinder debugging when these ratios are present.Apply this diff to include OtherRatios in the output:
func (p PriceData) ToSetting() string { - return fmt.Sprintf("ModelPrice: %f, ModelRatio: %f, CompletionRatio: %f, CacheRatio: %f, GroupRatio: %f, UsePrice: %t, CacheCreationRatio: %f, ShouldPreConsumedQuota: %d, ImageRatio: %f, AudioRatio: %f, AudioCompletionRatio: %f", p.ModelPrice, p.ModelRatio, p.CompletionRatio, p.CacheRatio, p.GroupRatioInfo.GroupRatio, p.UsePrice, p.CacheCreationRatio, p.ShouldPreConsumedQuota, p.ImageRatio, p.AudioRatio, p.AudioCompletionRatio) + return fmt.Sprintf("ModelPrice: %f, ModelRatio: %f, CompletionRatio: %f, CacheRatio: %f, GroupRatio: %f, UsePrice: %t, CacheCreationRatio: %f, ShouldPreConsumedQuota: %d, ImageRatio: %f, AudioRatio: %f, AudioCompletionRatio: %f, OtherRatios: %v", p.ModelPrice, p.ModelRatio, p.CompletionRatio, p.CacheRatio, p.GroupRatioInfo.GroupRatio, p.UsePrice, p.CacheCreationRatio, p.ShouldPreConsumedQuota, p.ImageRatio, p.AudioRatio, p.AudioCompletionRatio, p.OtherRatios) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
relay/common/relay_utils.go(2 hunks)relay/helper/price.go(1 hunks)relay/relay_task.go(3 hunks)setting/ratio_setting/model_ratio.go(2 hunks)types/price_data.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
relay/common/relay_utils.go (2)
common/str.go (1)
String2Int(66-72)types/price_data.go (1)
PriceData(11-24)
relay/relay_task.go (2)
setting/ratio_setting/model_ratio.go (1)
GetDefaultModelPriceMap(457-459)types/price_data.go (1)
PriceData(11-24)
relay/helper/price.go (1)
setting/ratio_setting/model_ratio.go (1)
GetDefaultModelPriceMap(457-459)
🔇 Additional comments (8)
types/price_data.go (1)
20-20: LGTM! New field supports multi-parameter pricing.The
OtherRatiosfield enables flexible per-parameter price adjustments (e.g., seconds, size for sora models), correctly integrated with downstream price calculation logic.setting/ratio_setting/model_ratio.go (2)
293-294: LGTM! Sora model pricing added.The default prices for sora-2 ($0.3) and sora-2-pro ($0.5) are now defined, enabling proper billing for these models.
457-459: Add mutex lock to prevent race conditions.Unlike
GetModelPriceMap()(lines 368-372), this function accessesdefaultModelPricewithout acquiring a read lock. SinceUpdateModelPriceByJSONString()(lines 385-394) can modifymodelPriceMapand potentially affect shared state, concurrent reads without synchronization risk race conditions.Apply this diff to add proper locking:
func GetDefaultModelPriceMap() map[string]float64 { + modelPriceMapMutex.RLock() + defer modelPriceMapMutex.RUnlock() return defaultModelPrice }Likely an incorrect or invalid review comment.
relay/helper/price.go (1)
117-122: LGTM! Default price lookup now uses the price map.The switch from
GetDefaultModelRatioMap()toGetDefaultModelPriceMap()aligns default pricing retrieval with the per-call pricing path, ensuring consistency across the codebase.relay/common/relay_utils.go (1)
13-13: LGTM! Lo library import added.The
lo.Contains()helper (used on lines 146, 149, 156) provides cleaner slice membership checks compared to manual loops. Based on learnings.relay/relay_task.go (3)
56-62: LGTM! Default price lookup now uses the price map.The switch from
GetDefaultModelRatioMap()toGetDefaultModelPriceMap()ensures consistent default pricing across per-call and token-based billing paths.
73-79: LGTM! Multi-parameter price adjustments applied.The code correctly multiplies the base ratio by each non-1.0 value in
OtherRatios, enabling flexible pricing for parameters like seconds and size.
153-163: LGTM! Enhanced logging includes parameter details.The log content now includes calculated parameters from
OtherRatios, improving observability for multi-parameter pricing adjustments.
| if strings.HasPrefix(model, "sora-2") { | ||
| seconds := 4 | ||
| size := "720x1280" | ||
| if ss, ok := form.Value["seconds"]; ok { | ||
| sInt := common.String2Int(ss[0]) | ||
| if sInt > seconds { | ||
| seconds = common.String2Int(ss[0]) | ||
| } | ||
| } | ||
| if s, ok := form.Value["size"]; ok { | ||
| size = s[0] | ||
| } | ||
|
|
||
| if model == "sora-2" && !lo.Contains([]string{"720x1280", "1280x720"}, size) { | ||
| return createTaskError(fmt.Errorf("sora-2 size is invalid"), "invalid_size", http.StatusBadRequest, true) | ||
| } | ||
| if model == "sora-2-pro" && !lo.Contains([]string{"720x1280", "1280x720", "1792x1024", "1024x1792"}, size) { | ||
| return createTaskError(fmt.Errorf("sora-2 size is invalid"), "invalid_size", http.StatusBadRequest, true) | ||
| } | ||
| info.PriceData.OtherRatios = map[string]float64{ | ||
| "seconds": float64(seconds), | ||
| "size": 1, | ||
| } | ||
| if lo.Contains([]string{"1792x1024", "1024x1792"}, size) { | ||
| info.PriceData.OtherRatios["size"] = 1.666667 | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify maximum duration limits for sora models.
The code validates size but not the upper bound for seconds. Ensure that OpenAI's sora-2 family models have documented maximum duration limits and enforce them to prevent overcharging or API errors.
Run the following query to check the official documentation:
🌐 Web query:
What are the maximum video duration limits for OpenAI sora-2 and sora-2-pro models?
💡 Result:
- sora-2 (API): maximum clip length = 12 seconds. [1][2]
- sora-2-pro (API): maximum clip length = 12 seconds. [1][2]
Note: the Sora product/app/interface can generate videos up to 20 seconds in some UIs, but the API model parameters for sora-2 / sora-2‑pro accept seconds values of 4, 8, or 12. [3][1]
Sources:
[1] OpenAI Sora prompting guide (API parameters: seconds = 4, 8, 12).
[2] Sora-2 prompting docs / community guide (model size/seconds options).
[3] OpenAI Help Center — Generating videos on Sora (notes Sora videos up to 20 seconds).
Enforce maximum duration limit for Sora models
After parsing seconds in relay/common/relay_utils.go, validate that it’s one of {4, 8, 12} (max 12 s); return an error if it exceeds 12 s.
🤖 Prompt for AI Agents
In relay/common/relay_utils.go around lines 133 to 159, the parsed `seconds` for
sora models must be restricted to the set {4, 8, 12} with a maximum of 12;
modify the parsing logic so after reading `seconds` from form.Value you convert
it once to an int, clamp or validate that it is one of the allowed values (4, 8,
or 12) and if it exceeds 12 or is not in the allowed set return
createTaskError(fmt.Errorf("sora-2 seconds is invalid"), "invalid_seconds",
http.StatusBadRequest, true); ensure you use the parsed int to set
info.PriceData.OtherRatios["seconds"] (as float64) and avoid calling String2Int
twice.
| if ss, ok := form.Value["seconds"]; ok { | ||
| sInt := common.String2Int(ss[0]) | ||
| if sInt > seconds { | ||
| seconds = common.String2Int(ss[0]) | ||
| } | ||
| } |
There was a problem hiding this comment.
Clarify seconds override logic.
The current logic only updates seconds if the form value is greater than the default (4). This means a user cannot explicitly request a shorter duration (e.g., 3 seconds) even if valid.
If the intent is to enforce a minimum of 4 seconds, consider:
if ss, ok := form.Value["seconds"]; ok {
sInt := common.String2Int(ss[0])
- if sInt > seconds {
+ if sInt >= 4 {
seconds = common.String2Int(ss[0])
+ } else {
+ return createTaskError(fmt.Errorf("seconds must be at least 4"), "invalid_seconds", http.StatusBadRequest, true)
}
}Or, if any positive value should be allowed, simply:
if ss, ok := form.Value["seconds"]; ok {
sInt := common.String2Int(ss[0])
- if sInt > seconds {
- seconds = common.String2Int(ss[0])
+ if sInt > 0 {
+ seconds = sInt
}
}| if model == "sora-2" && !lo.Contains([]string{"720x1280", "1280x720"}, size) { | ||
| return createTaskError(fmt.Errorf("sora-2 size is invalid"), "invalid_size", http.StatusBadRequest, true) | ||
| } | ||
| if model == "sora-2-pro" && !lo.Contains([]string{"720x1280", "1280x720", "1792x1024", "1024x1792"}, size) { | ||
| return createTaskError(fmt.Errorf("sora-2 size is invalid"), "invalid_size", http.StatusBadRequest, true) | ||
| } |
There was a problem hiding this comment.
Differentiate error messages per model.
Both sora-2 and sora-2-pro return the same error message "sora-2 size is invalid". For clarity, specify which model's size constraint was violated.
Apply this diff:
if model == "sora-2" && !lo.Contains([]string{"720x1280", "1280x720"}, size) {
- return createTaskError(fmt.Errorf("sora-2 size is invalid"), "invalid_size", http.StatusBadRequest, true)
+ return createTaskError(fmt.Errorf("sora-2 size must be 720x1280 or 1280x720"), "invalid_size", http.StatusBadRequest, true)
}
if model == "sora-2-pro" && !lo.Contains([]string{"720x1280", "1280x720", "1792x1024", "1024x1792"}, size) {
- return createTaskError(fmt.Errorf("sora-2 size is invalid"), "invalid_size", http.StatusBadRequest, true)
+ return createTaskError(fmt.Errorf("sora-2-pro size must be one of: 720x1280, 1280x720, 1792x1024, 1024x1792"), "invalid_size", http.StatusBadRequest, true)
}📝 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 model == "sora-2" && !lo.Contains([]string{"720x1280", "1280x720"}, size) { | |
| return createTaskError(fmt.Errorf("sora-2 size is invalid"), "invalid_size", http.StatusBadRequest, true) | |
| } | |
| if model == "sora-2-pro" && !lo.Contains([]string{"720x1280", "1280x720", "1792x1024", "1024x1792"}, size) { | |
| return createTaskError(fmt.Errorf("sora-2 size is invalid"), "invalid_size", http.StatusBadRequest, true) | |
| } | |
| if model == "sora-2" && !lo.Contains([]string{"720x1280", "1280x720"}, size) { | |
| return createTaskError(fmt.Errorf("sora-2 size must be 720x1288 or 1280x720"), "invalid_size", http.StatusBadRequest, true) | |
| } | |
| if model == "sora-2-pro" && !lo.Contains([]string{"720x1280", "1280x720", "1792x1024", "1024x1792"}, size) { | |
| return createTaskError(fmt.Errorf("sora-2-pro size must be one of: 720x1280, 1280x720, 1792x1024, 1024x1792"), "invalid_size", http.StatusBadRequest, true) | |
| } |
🤖 Prompt for AI Agents
In relay/common/relay_utils.go around lines 146 to 151, the error message for
both checks currently says "sora-2 size is invalid"; change the message to
reflect the actual model that failed validation (e.g., use "sora-2 size is
invalid" for the sora-2 branch and "sora-2-pro size is invalid" for the
sora-2-pro branch, or interpolate model into the fmt.Errorf call) so the
returned error clearly identifies which model's size constraint was violated;
keep the same error code, http status and retriable flag when calling
createTaskError.
feat: sora 增加参数校验与计费
…xplicit-session fix(openai): avoid implicit image sticky sessions
2 participants
Summary by CodeRabbit
New Features
Chores