fix video task successful upstream result parsing - #5805
Conversation
|
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 nested video success normalization in task polling, per-call-aware relay quota recalculation, per-second billing support across pricing logic and UI, and new Seedance and Grok video documentation. ChangesNested Video Success Normalization
Per-call Relay Quota Recalculation
Per-second Billing Labels
Video Integration Docs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
🧹 Nitpick comments (1)
service/task_polling.go (1)
551-564: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
outputextraction only handles arrays of plain strings.Several upstream video providers return
outputas an array of objects (e.g.[{"url": "..."}]) rather than bare strings. The currentstringValue(item)skips those, so a nested success could be missed. Consider handlingmap[string]anyitems as well if such shapes are expected from your providers.🤖 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 `@service/task_polling.go` around lines 551 - 564, The nested video URL extraction in extractNestedVideoURL only reads output items as plain strings, so object-shaped entries are missed. Update the output loop to also recognize map[string]any items and extract a URL from common fields like url, video_url, or result_url, while preserving the existing string handling and return behavior.
🤖 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.
Nitpick comments:
In `@service/task_polling.go`:
- Around line 551-564: The nested video URL extraction in extractNestedVideoURL
only reads output items as plain strings, so object-shaped entries are missed.
Update the output loop to also recognize map[string]any items and extract a URL
from common fields like url, video_url, or result_url, while preserving the
existing string handling and return behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 001d1143-3c53-4c70-a5b5-000b06c5f61a
📒 Files selected for processing (2)
service/task_billing_test.goservice/task_polling.go
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 `@model/pricing.go`:
- Around line 298-301: The pricing serialization in the quota type branch is
introducing a new value that the existing pricing sync consumer does not
understand. Update the logic around the pricing quota assignment so that the
code in model/pricing.go and the downstream parser in controller/ratio_sync.go
stay in sync: either keep per-second entries serialized as quota_type 1 for now,
or extend the ratio_sync parsing path to treat quota_type 2 as fixed-price and
read model_price instead of ratio fields. Ensure the existing /api/pricing
contract remains consistent with the behavior of the sync code.
🪄 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: e798d1c1-de1e-4654-b72b-b587c28846c1
📒 Files selected for processing (8)
model/pricing.goweb/src/components/table/model-pricing/filter/PricingQuotaTypes.jsxweb/src/components/table/model-pricing/modal/components/ModelPricingTable.jsxweb/src/components/table/model-pricing/view/card/PricingCardView.jsxweb/src/components/table/model-pricing/view/table/PricingTableColumns.jsxweb/src/helpers/utils.jsxweb/src/i18n/locales/en.jsonweb/src/i18n/locales/zh-CN.json
✅ Files skipped from review due to trivial changes (2)
- web/src/i18n/locales/zh-CN.json
- web/src/i18n/locales/en.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/seedance-video-integration.md (1)
140-144: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestructure repeated "Keep" sentences to improve readability.
Lines 140-144 start three successive bullet points with "Keep". Vary the sentence structure or merge related points for better flow.
- - Keep the public aliases in `channels.models`. - - Keep `channels.model_mapping` mapping each public alias to the raw upstream model. - - Keep raw upstream Seedance model names out of `abilities`. - - Keep raw upstream Seedance model metadata disabled with `models.status = 0`. + - Store public aliases in `channels.models`. + - Map each public alias to its raw upstream model via `channels.model_mapping`. + - Exclude raw upstream Seedance model names from `abilities`. + - Disable raw upstream Seedance model metadata with `models.status = 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 `@docs/seedance-video-integration.md` around lines 140 - 144, The repeated “Keep” bullets in the Seedance integration notes should be rewritten for readability. Update the affected bullet list in the documentation so the related constraints are grouped more naturally and the sentence openings vary, while still preserving the guidance about channels.models, channels.model_mapping, abilities, models.status, and channel 17 auto-sync behavior.
🤖 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 `@docs/seedance-video-integration.md`:
- Line 152: The jq verification command is over-escaping the dot in the
model_name regex, so the pattern passed to jq does not match seedance-2.0 as
intended. Update the select(...test(...)) expression in the verification command
to use the correct escaping inside the single-quoted shell string so jq receives
an escaped dot pattern, and confirm the regex still matches only the Seedance
2.0 model names.
---
Nitpick comments:
In `@docs/seedance-video-integration.md`:
- Around line 140-144: The repeated “Keep” bullets in the Seedance integration
notes should be rewritten for readability. Update the affected bullet list in
the documentation so the related constraints are grouped more naturally and the
sentence openings vary, while still preserving the guidance about
channels.models, channels.model_mapping, abilities, models.status, and channel
17 auto-sync behavior.
🪄 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: 3a866e84-7a1e-4cd2-90b5-07840b380864
📒 Files selected for processing (1)
docs/seedance-video-integration.md
|
|
||
| ```bash | ||
| curl -sS 'https://token.mewinyou.shop/api/pricing' \ | ||
| | jq -r '.data[]? | select(.model_name|test("^[0-9]+:seedance-2\\\\.0")) | .model_name' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix jq regex escaping in verification command.
The pattern seedance-2\\\\.0 in a single-quoted shell string produces seedance-2\\\.0 passed to jq, where jq likely sees \\. (literal backslash-dot) rather than \. (escaped dot). Use seedance-2\\\.0 or seedance-2\\.0 inside single quotes so jq receives seedance-2\.0.
- | jq -r '.data[]? | select(.model_name|test("^[0-9]+:seedance-2\\\\.0")) | .model_name'
+ | jq -r '.data[]? | select(.model_name|test("^[0-9]+:seedance-2\\.0")) | .model_name'📝 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.
| | jq -r '.data[]? | select(.model_name|test("^[0-9]+:seedance-2\\\\.0")) | .model_name' | |
| | jq -r '.data[]? | select(.model_name|test("^[0-9]+:seedance-2\\.0")) | .model_name' |
🤖 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 `@docs/seedance-video-integration.md` at line 152, The jq verification command
is over-escaping the dot in the model_name regex, so the pattern passed to jq
does not match seedance-2.0 as intended. Update the select(...test(...))
expression in the verification command to use the correct escaping inside the
single-quoted shell string so jq receives an escaped dot pattern, and confirm
the regex still matches only the Seedance 2.0 model names.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
docs/grok-video-api.md (4)
169-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename "Success Response" to "Initial Response" or "Creation Response".
The example shows
status: "queued", which is the initial creation response, not a successful completion. The title is misleading.🤖 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 `@docs/grok-video-api.md` around lines 169 - 181, The response heading in the Grok video API docs is misleading because the example payload from the queued task creation flow is not a completed success state. Update the section title around the JSON example to “Initial Response” or “Creation Response” so it matches the queued status shown by the video task response.
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify or remove the misspelled domain note.
Documenting
tokne.mewinyou.shopas a possible valid endpoint risks confusing users. If it is a typo, remove it; if it is an actual alternate domain, explain why both spellings exist (e.g., legacy alias, regional deployment).🤖 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 `@docs/grok-video-api.md` around lines 5 - 6, Clarify the endpoint note in the grok-video-api documentation by either removing the misspelled tokne.mewinyou.shop reference or explicitly explaining why it is a valid alternate domain. Update the relevant note text so the URL guidance is unambiguous, and keep the production base URL description in sync with the actual expected endpoint.
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify
secondstype flexibility.
integer or stringis ambiguous. If the API coerces string numerics (e.g.,"4"), document that; otherwise restrict tointeger.🤖 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 `@docs/grok-video-api.md` at line 33, Clarify the `seconds` field type in the video API documentation by updating the schema description to match the actual behavior of the request parser. In the `seconds` row, either explicitly state that numeric strings are accepted and coerced (for example, `"4"`), or change the type to integer only if string values are not supported. Make the wording in `grok-video-api` unambiguous so users know exactly what `seconds` accepts.
209-212: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove or clarify the vague normalization note.
"The exact field may depend on upstream response normalization" undermines API contract confidence. Since this PR specifically fixes normalization, document the guaranteed field (
result_url) or describe the concrete normalization behavior.🤖 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 `@docs/grok-video-api.md` around lines 209 - 212, The “Completed Response” section is too vague because the note about upstream response normalization weakens the API contract; update the documentation in the grok video API spec to either explicitly guarantee the normalized result field as result_url or clearly describe the exact normalization behavior implemented by the response handling path. Refer to the Completed Response wording in the docs and align it with the concrete contract exposed by the NewAPI response so readers know precisely which field to expect.
🤖 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.
Nitpick comments:
In `@docs/grok-video-api.md`:
- Around line 169-181: The response heading in the Grok video API docs is
misleading because the example payload from the queued task creation flow is not
a completed success state. Update the section title around the JSON example to
“Initial Response” or “Creation Response” so it matches the queued status shown
by the video task response.
- Around line 5-6: Clarify the endpoint note in the grok-video-api documentation
by either removing the misspelled tokne.mewinyou.shop reference or explicitly
explaining why it is a valid alternate domain. Update the relevant note text so
the URL guidance is unambiguous, and keep the production base URL description in
sync with the actual expected endpoint.
- Line 33: Clarify the `seconds` field type in the video API documentation by
updating the schema description to match the actual behavior of the request
parser. In the `seconds` row, either explicitly state that numeric strings are
accepted and coerced (for example, `"4"`), or change the type to integer only if
string values are not supported. Make the wording in `grok-video-api`
unambiguous so users know exactly what `seconds` accepts.
- Around line 209-212: The “Completed Response” section is too vague because the
note about upstream response normalization weakens the API contract; update the
documentation in the grok video API spec to either explicitly guarantee the
normalized result field as result_url or clearly describe the exact
normalization behavior implemented by the response handling path. Refer to the
Completed Response wording in the docs and align it with the concrete contract
exposed by the NewAPI response so readers know precisely which field to expect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b51b465-03c0-47ed-9cfa-1edaedfdc263
📒 Files selected for processing (1)
docs/grok-video-api.md
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit