1 - #3475
Conversation
Add video parameters (size/seconds/quality) to UI, payload and DTO
Add video generation support: UI controls, payload handling, and DTO fields
|
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 end-to-end image/video generation support: new DTO fields ( Changes
Sequence DiagramsequenceDiagram
participant User as User / UI
participant Panel as SettingsPanel
participant Hook as useApiRequest
participant Helper as buildApiPayload
participant API as Playground API
participant Relay as Relay/Adaptor
User->>Panel: choose model (image/video) & set options
Panel->>Hook: onInputChange(inputs)
Hook->>Helper: buildApiPayload(inputs)
Helper->>Helper: detect image/video/grok -> set stream=false, add size/seconds/quality/preset
Hook->>Hook: resolveEndpointAndPayload -> IMAGE/VIDEO endpoint, forceNonStream
Hook->>API: POST /pg/... (non-stream payload)
API->>Relay: Relay/Task dispatch
Relay->>API: respond (task_id / data / object:video or images)
API->>Hook: return response
Hook->>Hook: parse response -> replace LOADING with COMPLETE (urls/task info)
Hook->>User: display images or video task link/status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/src/hooks/playground/useApiRequest.jsx (1)
43-46: Centralize video capability detection instead of matchingmodel.includes('video').This assumption is now duplicated here,
web/src/helpers/api.js, andweb/src/components/playground/SettingsPanel.jsx. A single capability flag/helper from the model metadata would keep the UI, payload shaping, and endpoint routing aligned.Also applies to: 98-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/playground/useApiRequest.jsx` around lines 43 - 46, Replace the ad-hoc string check inside isVideoGenerationPayload with a single centralized capability check: create or import a model capability helper (e.g., supportsVideo(model) or getModelCapabilities(model).supportsVideo) and use that here instead of model.includes('video'); update isVideoGenerationPayload in useApiRequest.jsx to read the capability flag (model?.capabilities?.video or the helper) and remove any string-matching logic, and then switch the other duplicated checks in web/src/helpers/api.js and web/src/components/playground/SettingsPanel.jsx to the same helper so UI, payload shaping, and routing all rely on the single authoritative capability flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/playground/SettingsPanel.jsx`:
- Around line 63-66: videoQualityOptions currently uses raw labels ('standard',
'high') that bypass i18next; update SettingsPanel.jsx to import/use the
useTranslation() hook and replace those labels with translated strings via
t('...') (e.g., t('标准') / t('高清') or the correct Chinese keys from
web/src/i18n/locales/{lang}.json) so the array becomes [{ label: t('你的中文key1'),
value: 'standard' }, ...]; ensure you reference useTranslation and t in the same
component where videoQualityOptions is defined.
In `@web/src/hooks/playground/useApiRequest.jsx`:
- Around line 309-315: The summary string currently only localizes the headline
via t() while the field labels (task_id, status, seconds, size) remain raw
English; update the component in useApiRequest.jsx to use the useTranslation()
hook and wrap each label in t('...') with Chinese keys matching our flat locale
JSON (e.g., t('任务ID'), t('状态'), t('秒'), t('大小')), then build the summary using
those t(...) calls (keep the existing headline t('视频任务已创建') and the same
fallbacks like data.task_id || data.id || '-'); ensure the keys you use exist in
web/src/i18n/locales/{lang}.json.
- Around line 62-95: buildVideoRequestPayload currently calls
getImageFromMessageContent which returns only a single image_url, dropping any
additional reference images; change the logic to collect all image_url items
from the message content and preserve them in the payload (e.g., produce an
images array), while optionally keeping the single image property for backward
compatibility. Update getImageFromMessageContent (or add a new helper like
getImagesFromMessageContent) to return an array of image URLs from content items
with type === 'image_url', and modify buildVideoRequestPayload to set both image
(first URL) and images (full array) or at least images when multiple exist so
the relay/common/relay_info.go consumer (Image and Images) receives all
references.
---
Nitpick comments:
In `@web/src/hooks/playground/useApiRequest.jsx`:
- Around line 43-46: Replace the ad-hoc string check inside
isVideoGenerationPayload with a single centralized capability check: create or
import a model capability helper (e.g., supportsVideo(model) or
getModelCapabilities(model).supportsVideo) and use that here instead of
model.includes('video'); update isVideoGenerationPayload in useApiRequest.jsx to
read the capability flag (model?.capabilities?.video or the helper) and remove
any string-matching logic, and then switch the other duplicated checks in
web/src/helpers/api.js and web/src/components/playground/SettingsPanel.jsx to
the same helper so UI, payload shaping, and routing all rely on the single
authoritative capability flag.
🪄 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: 409341e9-0c18-492b-bb87-1681372cc914
📒 Files selected for processing (5)
dto/openai_request.goweb/src/components/playground/SettingsPanel.jsxweb/src/constants/playground.constants.jsweb/src/helpers/api.jsweb/src/hooks/playground/useApiRequest.jsx
| const videoQualityOptions = [ | ||
| { label: 'standard', value: 'standard' }, | ||
| { label: 'high', value: 'high' }, | ||
| ]; |
There was a problem hiding this comment.
Localize the new quality option labels.
standard and high are rendered directly, so they bypass i18next while the rest of the panel is translated.
♻️ Suggested i18n fix
const videoQualityOptions = [
- { label: 'standard', value: 'standard' },
- { label: 'high', value: 'high' },
+ { label: t('标准'), value: 'standard' },
+ { label: t('高质量'), value: 'high' },
];📝 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.
| const videoQualityOptions = [ | |
| { label: 'standard', value: 'standard' }, | |
| { label: 'high', value: 'high' }, | |
| ]; | |
| const videoQualityOptions = [ | |
| { label: t('标准'), value: 'standard' }, | |
| { label: t('高质量'), value: 'high' }, | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/playground/SettingsPanel.jsx` around lines 63 - 66,
videoQualityOptions currently uses raw labels ('standard', 'high') that bypass
i18next; update SettingsPanel.jsx to import/use the useTranslation() hook and
replace those labels with translated strings via t('...') (e.g., t('标准') /
t('高清') or the correct Chinese keys from web/src/i18n/locales/{lang}.json) so
the array becomes [{ label: t('你的中文key1'), value: 'standard' }, ...]; ensure you
reference useTranslation and t in the same component where videoQualityOptions
is defined.
| const getImageFromMessageContent = useCallback((content) => { | ||
| if (!Array.isArray(content)) { | ||
| return ''; | ||
| } | ||
| const imageItem = content.find((item) => item?.type === 'image_url'); | ||
| if (!imageItem) { | ||
| return ''; | ||
| } | ||
| const imageURL = imageItem.image_url; | ||
| if (typeof imageURL === 'string') { | ||
| return imageURL; | ||
| } | ||
| return imageURL?.url || ''; | ||
| }, []); | ||
|
|
||
| const buildVideoRequestPayload = useCallback( | ||
| (payload) => { | ||
| const messages = Array.isArray(payload?.messages) ? payload.messages : []; | ||
| const lastUserMessage = [...messages] | ||
| .reverse() | ||
| .find((m) => m?.role === 'user'); | ||
| const prompt = getTextFromMessageContent(lastUserMessage?.content); | ||
| const image = getImageFromMessageContent(lastUserMessage?.content); | ||
|
|
||
| return { | ||
| model: payload.model, | ||
| prompt, | ||
| seconds: payload.seconds, | ||
| size: payload.size, | ||
| quality: payload.quality, | ||
| ...(image ? { image } : {}), | ||
| }; | ||
| }, | ||
| [getImageFromMessageContent, getTextFromMessageContent], |
There was a problem hiding this comment.
Don't drop extra reference images on the video path.
buildVideoRequestPayload() only keeps the first image_url, but the playground input is plural and relay/common/relay_info.go:666-677 already has both Image and Images. Any additional reference image is silently lost here.
🧩 One way to preserve multi-image input
- const getImageFromMessageContent = useCallback((content) => {
- if (!Array.isArray(content)) {
- return '';
- }
- const imageItem = content.find((item) => item?.type === 'image_url');
- if (!imageItem) {
- return '';
- }
- const imageURL = imageItem.image_url;
- if (typeof imageURL === 'string') {
- return imageURL;
- }
- return imageURL?.url || '';
- }, []);
+ const getImagesFromMessageContent = useCallback((content) => {
+ if (!Array.isArray(content)) {
+ return [];
+ }
+ return content
+ .filter((item) => item?.type === 'image_url')
+ .map((item) => {
+ const imageURL = item.image_url;
+ return typeof imageURL === 'string' ? imageURL : imageURL?.url || '';
+ })
+ .filter(Boolean);
+ }, []);
const buildVideoRequestPayload = useCallback(
(payload) => {
const messages = Array.isArray(payload?.messages) ? payload.messages : [];
const lastUserMessage = [...messages]
.reverse()
.find((m) => m?.role === 'user');
const prompt = getTextFromMessageContent(lastUserMessage?.content);
- const image = getImageFromMessageContent(lastUserMessage?.content);
+ const images = getImagesFromMessageContent(lastUserMessage?.content);
return {
model: payload.model,
prompt,
seconds: payload.seconds,
size: payload.size,
quality: payload.quality,
- ...(image ? { image } : {}),
+ ...(images.length === 1 ? { image: images[0] } : {}),
+ ...(images.length > 1 ? { images } : {}),
};
},
- [getImageFromMessageContent, getTextFromMessageContent],
+ [getImagesFromMessageContent, getTextFromMessageContent],
);📝 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.
| const getImageFromMessageContent = useCallback((content) => { | |
| if (!Array.isArray(content)) { | |
| return ''; | |
| } | |
| const imageItem = content.find((item) => item?.type === 'image_url'); | |
| if (!imageItem) { | |
| return ''; | |
| } | |
| const imageURL = imageItem.image_url; | |
| if (typeof imageURL === 'string') { | |
| return imageURL; | |
| } | |
| return imageURL?.url || ''; | |
| }, []); | |
| const buildVideoRequestPayload = useCallback( | |
| (payload) => { | |
| const messages = Array.isArray(payload?.messages) ? payload.messages : []; | |
| const lastUserMessage = [...messages] | |
| .reverse() | |
| .find((m) => m?.role === 'user'); | |
| const prompt = getTextFromMessageContent(lastUserMessage?.content); | |
| const image = getImageFromMessageContent(lastUserMessage?.content); | |
| return { | |
| model: payload.model, | |
| prompt, | |
| seconds: payload.seconds, | |
| size: payload.size, | |
| quality: payload.quality, | |
| ...(image ? { image } : {}), | |
| }; | |
| }, | |
| [getImageFromMessageContent, getTextFromMessageContent], | |
| const getImagesFromMessageContent = useCallback((content) => { | |
| if (!Array.isArray(content)) { | |
| return []; | |
| } | |
| return content | |
| .filter((item) => item?.type === 'image_url') | |
| .map((item) => { | |
| const imageURL = item.image_url; | |
| return typeof imageURL === 'string' ? imageURL : imageURL?.url || ''; | |
| }) | |
| .filter(Boolean); | |
| }, []); | |
| const buildVideoRequestPayload = useCallback( | |
| (payload) => { | |
| const messages = Array.isArray(payload?.messages) ? payload.messages : []; | |
| const lastUserMessage = [...messages] | |
| .reverse() | |
| .find((m) => m?.role === 'user'); | |
| const prompt = getTextFromMessageContent(lastUserMessage?.content); | |
| const images = getImagesFromMessageContent(lastUserMessage?.content); | |
| return { | |
| model: payload.model, | |
| prompt, | |
| seconds: payload.seconds, | |
| size: payload.size, | |
| quality: payload.quality, | |
| ...(images.length === 1 ? { image: images[0] } : {}), | |
| ...(images.length > 1 ? { images } : {}), | |
| }; | |
| }, | |
| [getImagesFromMessageContent, getTextFromMessageContent], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/playground/useApiRequest.jsx` around lines 62 - 95,
buildVideoRequestPayload currently calls getImageFromMessageContent which
returns only a single image_url, dropping any additional reference images;
change the logic to collect all image_url items from the message content and
preserve them in the payload (e.g., produce an images array), while optionally
keeping the single image property for backward compatibility. Update
getImageFromMessageContent (or add a new helper like
getImagesFromMessageContent) to return an array of image URLs from content items
with type === 'image_url', and modify buildVideoRequestPayload to set both image
(first URL) and images (full array) or at least images when multiple exist so
the relay/common/relay_info.go consumer (Image and Images) receives all
references.
| const summary = [ | ||
| `${t('视频任务已创建')}`, | ||
| `task_id: ${data.task_id || data.id || '-'}`, | ||
| `status: ${data.status || '-'}`, | ||
| `seconds: ${data.seconds || requestPayload.seconds || '-'}`, | ||
| `size: ${data.size || requestPayload.size || '-'}`, | ||
| ].join('\n'); |
There was a problem hiding this comment.
Localize the new video task summary fields.
Only the headline goes through t(). The field labels in the assistant message will still render as raw English.
♻️ Suggested i18n fix
const summary = [
`${t('视频任务已创建')}`,
- `task_id: ${data.task_id || data.id || '-'}`,
- `status: ${data.status || '-'}`,
- `seconds: ${data.seconds || requestPayload.seconds || '-'}`,
- `size: ${data.size || requestPayload.size || '-'}`,
+ `${t('任务 ID')}: ${data.task_id || data.id || '-'}`,
+ `${t('状态')}: ${data.status || '-'}`,
+ `${t('视频时长')}: ${data.seconds || requestPayload.seconds || '-'}`,
+ `${t('视频尺寸')}: ${data.size || requestPayload.size || '-'}`,
].join('\n');📝 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.
| const summary = [ | |
| `${t('视频任务已创建')}`, | |
| `task_id: ${data.task_id || data.id || '-'}`, | |
| `status: ${data.status || '-'}`, | |
| `seconds: ${data.seconds || requestPayload.seconds || '-'}`, | |
| `size: ${data.size || requestPayload.size || '-'}`, | |
| ].join('\n'); | |
| const summary = [ | |
| `${t('视频任务已创建')}`, | |
| `${t('任务 ID')}: ${data.task_id || data.id || '-'}`, | |
| `${t('状态')}: ${data.status || '-'}`, | |
| `${t('视频时长')}: ${data.seconds || requestPayload.seconds || '-'}`, | |
| `${t('视频尺寸')}: ${data.size || requestPayload.size || '-'}`, | |
| ].join('\n'); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/playground/useApiRequest.jsx` around lines 309 - 315, The
summary string currently only localizes the headline via t() while the field
labels (task_id, status, seconds, size) remain raw English; update the component
in useApiRequest.jsx to use the useTranslation() hook and wrap each label in
t('...') with Chinese keys matching our flat locale JSON (e.g., t('任务ID'),
t('状态'), t('秒'), t('大小')), then build the summary using those t(...) calls (keep
the existing headline t('视频任务已创建') and the same fallbacks like data.task_id ||
data.id || '-'); ensure the keys you use exist in
web/src/i18n/locales/{lang}.json.
Playground: add video generation support and refactor token context setup
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
controller/playground.go (1)
93-94: HandleSetupContextForTokenfailure instead of ignoring it.The helper currently drops setup errors and always returns
nil, which can mask context initialization failures.🔧 Suggested fix
- _ = middleware.SetupContextForToken(c, tempToken) - return nil + if err := middleware.SetupContextForToken(c, tempToken); err != nil { + return types.NewError(err, types.ErrorCodeQueryDataError, types.ErrOptionWithSkipRetry()) + } + return nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/playground.go` around lines 93 - 94, The call to middleware.SetupContextForToken(c, tempToken) currently discards its error; change it to capture the returned error, and if non-nil, handle it (e.g., log and return the error or translate it to an appropriate HTTP response) instead of always returning nil. Locate the call to middleware.SetupContextForToken in the playground handler, assign its result to err, and return or propagate a wrapped error so context initialization failures are not silently ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/playground.go`:
- Around line 45-73: Add the same "reject access-token flows" guard to
PlaygroundVideoSubmit and PlaygroundVideoFetch as in Playground: after calling
setupPlaygroundTokenContext (and before RelayTask/RelayTaskFetch), check the
request for the use_access_token flag (the same source Playground uses, e.g.,
query/header/context key "use_access_token") and if it's set/true return a
NewAPIError (same type/format Playground returns) so the handlers reject
access-token flows consistently.
---
Nitpick comments:
In `@controller/playground.go`:
- Around line 93-94: The call to middleware.SetupContextForToken(c, tempToken)
currently discards its error; change it to capture the returned error, and if
non-nil, handle it (e.g., log and return the error or translate it to an
appropriate HTTP response) instead of always returning nil. Locate the call to
middleware.SetupContextForToken in the playground handler, assign its result to
err, and return or propagate a wrapped error so context initialization failures
are not silently ignored.
🪄 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: af72f373-e9ad-4b96-a476-9d72540b9ed8
📒 Files selected for processing (3)
controller/playground.gorouter/relay-router.goweb/src/constants/playground.constants.js
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/constants/playground.constants.js
| func PlaygroundVideoSubmit(c *gin.Context) { | ||
| var newAPIError *types.NewAPIError | ||
| defer func() { | ||
| if newAPIError != nil { | ||
| c.JSON(newAPIError.StatusCode, gin.H{ | ||
| "error": newAPIError.ToOpenAIError(), | ||
| }) | ||
| } | ||
| }() | ||
| if newAPIError = setupPlaygroundTokenContext(c, "playground-video", c.GetString("group")); newAPIError != nil { | ||
| return | ||
| } | ||
| RelayTask(c) | ||
| } | ||
|
|
||
| func PlaygroundVideoFetch(c *gin.Context) { | ||
| var newAPIError *types.NewAPIError | ||
| defer func() { | ||
| if newAPIError != nil { | ||
| c.JSON(newAPIError.StatusCode, gin.H{ | ||
| "error": newAPIError.ToOpenAIError(), | ||
| }) | ||
| } | ||
| }() | ||
| if newAPIError = setupPlaygroundTokenContext(c, "playground-video-fetch", c.GetString("group")); newAPIError != nil { | ||
| return | ||
| } | ||
| RelayTaskFetch(c) | ||
| } |
There was a problem hiding this comment.
Reject access-token flows in video playground handlers for parity with Playground.
PlaygroundVideoSubmit and PlaygroundVideoFetch currently miss the use_access_token guard that Playground enforces. This introduces inconsistent auth behavior for /pg/video/*.
🔧 Suggested fix
func PlaygroundVideoSubmit(c *gin.Context) {
var newAPIError *types.NewAPIError
defer func() {
if newAPIError != nil {
c.JSON(newAPIError.StatusCode, gin.H{
"error": newAPIError.ToOpenAIError(),
})
}
}()
+ if c.GetBool("use_access_token") {
+ newAPIError = types.NewError(errors.New("暂不支持使用 access token"), types.ErrorCodeAccessDenied, types.ErrOptionWithSkipRetry())
+ return
+ }
if newAPIError = setupPlaygroundTokenContext(c, "playground-video", c.GetString("group")); newAPIError != nil {
return
}
RelayTask(c)
}
func PlaygroundVideoFetch(c *gin.Context) {
var newAPIError *types.NewAPIError
defer func() {
if newAPIError != nil {
c.JSON(newAPIError.StatusCode, gin.H{
"error": newAPIError.ToOpenAIError(),
})
}
}()
+ if c.GetBool("use_access_token") {
+ newAPIError = types.NewError(errors.New("暂不支持使用 access token"), types.ErrorCodeAccessDenied, types.ErrOptionWithSkipRetry())
+ return
+ }
if newAPIError = setupPlaygroundTokenContext(c, "playground-video-fetch", c.GetString("group")); newAPIError != nil {
return
}
RelayTaskFetch(c)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func PlaygroundVideoSubmit(c *gin.Context) { | |
| var newAPIError *types.NewAPIError | |
| defer func() { | |
| if newAPIError != nil { | |
| c.JSON(newAPIError.StatusCode, gin.H{ | |
| "error": newAPIError.ToOpenAIError(), | |
| }) | |
| } | |
| }() | |
| if newAPIError = setupPlaygroundTokenContext(c, "playground-video", c.GetString("group")); newAPIError != nil { | |
| return | |
| } | |
| RelayTask(c) | |
| } | |
| func PlaygroundVideoFetch(c *gin.Context) { | |
| var newAPIError *types.NewAPIError | |
| defer func() { | |
| if newAPIError != nil { | |
| c.JSON(newAPIError.StatusCode, gin.H{ | |
| "error": newAPIError.ToOpenAIError(), | |
| }) | |
| } | |
| }() | |
| if newAPIError = setupPlaygroundTokenContext(c, "playground-video-fetch", c.GetString("group")); newAPIError != nil { | |
| return | |
| } | |
| RelayTaskFetch(c) | |
| } | |
| func PlaygroundVideoSubmit(c *gin.Context) { | |
| var newAPIError *types.NewAPIError | |
| defer func() { | |
| if newAPIError != nil { | |
| c.JSON(newAPIError.StatusCode, gin.H{ | |
| "error": newAPIError.ToOpenAIError(), | |
| }) | |
| } | |
| }() | |
| if c.GetBool("use_access_token") { | |
| newAPIError = types.NewError(errors.New("暂不支持使用 access token"), types.ErrorCodeAccessDenied, types.ErrOptionWithSkipRetry()) | |
| return | |
| } | |
| if newAPIError = setupPlaygroundTokenContext(c, "playground-video", c.GetString("group")); newAPIError != nil { | |
| return | |
| } | |
| RelayTask(c) | |
| } | |
| func PlaygroundVideoFetch(c *gin.Context) { | |
| var newAPIError *types.NewAPIError | |
| defer func() { | |
| if newAPIError != nil { | |
| c.JSON(newAPIError.StatusCode, gin.H{ | |
| "error": newAPIError.ToOpenAIError(), | |
| }) | |
| } | |
| }() | |
| if c.GetBool("use_access_token") { | |
| newAPIError = types.NewError(errors.New("暂不支持使用 access token"), types.ErrorCodeAccessDenied, types.ErrOptionWithSkipRetry()) | |
| return | |
| } | |
| if newAPIError = setupPlaygroundTokenContext(c, "playground-video-fetch", c.GetString("group")); newAPIError != nil { | |
| return | |
| } | |
| RelayTaskFetch(c) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/playground.go` around lines 45 - 73, Add the same "reject
access-token flows" guard to PlaygroundVideoSubmit and PlaygroundVideoFetch as
in Playground: after calling setupPlaygroundTokenContext (and before
RelayTask/RelayTaskFetch), check the request for the use_access_token flag (the
same source Playground uses, e.g., query/header/context key "use_access_token")
and if it's set/true return a NewAPIError (same type/format Playground returns)
so the handlers reject access-token flows consistently.
Add playground video generation endpoints and UI; wire video parameters through frontend and backend
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/hooks/playground/useApiRequest.jsx (1)
43-46: Fragile video model detection heuristic.Checking
model.includes('video')may produce false positives (e.g., a model named"video-analysis"for non-generation tasks) or false negatives (video models without "video" in their name). Consider maintaining an explicit allowlist or using a more robust pattern.♻️ Example: use a dedicated constant or regex pattern
+const VIDEO_MODEL_PATTERNS = [ + /^sora/i, + /video[-_]?gen/i, + // add other known video generation model patterns +]; + const isVideoGenerationPayload = useCallback((payload) => { const model = payload?.model; - return typeof model === 'string' && model.includes('video'); + if (typeof model !== 'string') return false; + return VIDEO_MODEL_PATTERNS.some((pattern) => pattern.test(model)); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/playground/useApiRequest.jsx` around lines 43 - 46, The current isVideoGenerationPayload heuristic (function isVideoGenerationPayload) is fragile because it only does model.includes('video'); replace it with a robust check against a maintained allowlist or stricter pattern: define a constant array (e.g., VIDEO_GENERATION_MODELS) or a compiled regex (e.g., VIDEO_MODEL_REGEX) and update isVideoGenerationPayload to validate payload.model against that allowlist/regex (also handle undefined/null safely); ensure the constant is exported/located near related hooks so it can be easily updated as model names change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/hooks/playground/useApiRequest.jsx`:
- Around line 304-362: Remove the duplicated video-response handling block that
introduces a nested identical if and orphaned "].join('\n')" in
useApiRequest.jsx; keep the single correct block that computes taskId,
fallbackVideoURL, videoURL and builds summary (using t() for all labels), then
updates messages via setMessage (referencing MESSAGE_STATUS.LOADING and
applyAutoCollapseLogic) and returns. Specifically, delete the duplicated inner
if/summary/setMessage/return chunk so only the initial conditional handling for
API_ENDPOINTS.VIDEO_GENERATIONS / data.object === 'video' / data.task_id remains
intact.
---
Nitpick comments:
In `@web/src/hooks/playground/useApiRequest.jsx`:
- Around line 43-46: The current isVideoGenerationPayload heuristic (function
isVideoGenerationPayload) is fragile because it only does
model.includes('video'); replace it with a robust check against a maintained
allowlist or stricter pattern: define a constant array (e.g.,
VIDEO_GENERATION_MODELS) or a compiled regex (e.g., VIDEO_MODEL_REGEX) and
update isVideoGenerationPayload to validate payload.model against that
allowlist/regex (also handle undefined/null safely); ensure the constant is
exported/located near related hooks so it can be easily updated as model names
change.
🪄 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: 4a92b898-9508-47d2-a4d9-b3691d5ab2d4
📒 Files selected for processing (1)
web/src/hooks/playground/useApiRequest.jsx
…parameters through frontend and backend"
Revert "Add playground video generation endpoints and UI; wire video parameters through frontend and backend"
Codex/fix grok video url
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
web/src/hooks/playground/useApiRequest.jsx (2)
454-468:⚠️ Potential issue | 🟡 MinorVideo summary field labels are not localized.
The headline uses
t('视频任务已创建'), but field labels (task_id:,status:,seconds:,size:, etc.) remain hardcoded English strings, inconsistent with i18n guidelines.♻️ Localize field labels
const summary = [ `${t('视频任务已创建')}`, - `task_id: ${data.task_id || data.id || '-'}`, - `status: ${data.status || '-'}`, - `seconds: ${data.seconds || requestPayload.seconds || '-'}`, - `size: ${data.size || requestPayload.size || '-'}`, - `quality: ${requestedQuality || upstreamQuality || '-'}`, + `${t('任务ID')}: ${data.task_id || data.id || '-'}`, + `${t('状态')}: ${data.status || '-'}`, + `${t('时长')}: ${data.seconds || requestPayload.seconds || '-'}`, + `${t('尺寸')}: ${data.size || requestPayload.size || '-'}`, + `${t('质量')}: ${requestedQuality || upstreamQuality || '-'}`,As per coding guidelines:
web/src/**/*.{ts,tsx,js,jsx}: Frontend i18n: UseuseTranslation()hook and callt('中文key')in components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/playground/useApiRequest.jsx` around lines 454 - 468, The summary string in useApiRequest.jsx builds field labels ("task_id:", "status:", "seconds:", "size:", "upstream_quality:", "preset:", "url:") as hardcoded English; update the summary construction in the function that builds `summary` to use the i18n `t()` translations (via the component's `useTranslation()` hook) for each label key (e.g., t('任务ID'), t('状态'), t('秒数'), t('大小'), t('上游质量'), t('预设'), t('URL')), and replace the literal labels in the `summary` array with calls to `t(...)` while keeping the same data expressions (`data.task_id || data.id`, `data.status`, `data.seconds || requestPayload.seconds`, `data.size || requestPayload.size`, `requestedQuality || upstreamQuality`, `upstreamQuality`, `requestPayload.preset`, `videoUrl`) so the rest of the code (variables `summary`, `requestPayload`, `requestedQuality`, `upstreamQuality`, `videoUrl`) remains unchanged.
87-100:⚠️ Potential issue | 🟡 MinorOnly first image is extracted, additional reference images are dropped.
getImageFromMessageContentusescontent.find()which returns only the firstimage_urlitem. Any additional images in the message content are silently discarded.♻️ Extract all images
- const getImageFromMessageContent = useCallback((content) => { + const getImagesFromMessageContent = useCallback((content) => { if (!Array.isArray(content)) { - return ''; + return []; } - const imageItem = content.find((item) => item?.type === 'image_url'); - if (!imageItem) { - return ''; - } - const imageURL = imageItem.image_url; - if (typeof imageURL === 'string') { - return imageURL; - } - return imageURL?.url || ''; + return content + .filter((item) => item?.type === 'image_url') + .map((item) => { + const imageURL = item.image_url; + return typeof imageURL === 'string' ? imageURL : imageURL?.url || ''; + }) + .filter(Boolean); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/playground/useApiRequest.jsx` around lines 87 - 100, getImageFromMessageContent currently uses content.find() and only returns the first image_url; change it to collect all image items by using content.filter(item => item?.type === 'image_url') and map them to their URL (item.image_url if string else item.image_url?.url), returning an array of image URLs (or an empty array when none) instead of a single string; update any callers expecting a single string to handle an array (or optionally return the first element for backward compatibility and expose a new getImagesFromMessageContent helper), referencing the getImageFromMessageContent function name to locate and update usage.controller/playground.go (1)
45-103:⚠️ Potential issue | 🟠 MajorMissing
use_access_tokenguard in new playground handlers.
Playground(lines 26-30) rejects access token flows, butPlaygroundVideoSubmit,PlaygroundImageGenerations,PlaygroundImageEdits, andPlaygroundVideoFetchall skip this check. This creates inconsistent auth behavior for/pg/*routes.🔧 Add access token guard to each handler
func PlaygroundVideoSubmit(c *gin.Context) { var newAPIError *types.NewAPIError defer func() { if newAPIError != nil { c.JSON(newAPIError.StatusCode, gin.H{ "error": newAPIError.ToOpenAIError(), }) } }() + if c.GetBool("use_access_token") { + newAPIError = types.NewError(errors.New("暂不支持使用 access token"), types.ErrorCodeAccessDenied, types.ErrOptionWithSkipRetry()) + return + } if newAPIError = setupPlaygroundTokenContext(c, "playground-video", c.GetString("group")); newAPIError != nil {Apply the same pattern to
PlaygroundImageGenerations,PlaygroundImageEdits, andPlaygroundVideoFetch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/playground.go` around lines 45 - 103, The new playground handlers (PlaygroundVideoSubmit, PlaygroundImageGenerations, PlaygroundImageEdits, PlaygroundVideoFetch) are missing the same use_access_token rejection that Playground implements; copy the access-token guard logic from the Playground handler and add it at the top of each of these functions (before calling setupPlaygroundTokenContext) so requests that use the access token flow are rejected consistently across /pg/* routes.
🧹 Nitpick comments (1)
controller/relay.go (1)
590-625: Consider loggingParseTaskResulterrors instead of silently ignoring them.Current fallback behavior is fine, but silent parse failures will hide adaptor-contract regressions and make debugging task-status drift harder.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/relay.go` around lines 590 - 625, The code currently swallows errors from adaptor.ParseTaskResult; update the ParseTaskResult call inside the relay.GetTaskAdaptor block to log any non-nil error instead of ignoring it—capture the error returned by adaptor.ParseTaskResult and emit a clear log entry (using the controller's logger/processLogger) that includes context such as result.Platform, task.ID (or task.TaskID), and the raw result.TaskData to help diagnose adaptor-contract regressions; keep the existing fallback behavior but ensure errors from ParseTaskResult are logged immediately where ParseTaskResult is invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@middleware/distributor.go`:
- Around line 84-85: Requests under /pg/* can hit getModelRequest which only
handles /pg/chat/completions, leaving modelRequest.Model empty and
shouldSelectChannel incorrect for routes like /pg/images/* or /pg/video/*;
update the middleware so when strings.HasPrefix(c.Request.URL.Path, "/pg/") you
normalize the path (or add explicit cases) before calling getModelRequest:
either map known playground prefixes (e.g., "/pg/chat", "/pg/images",
"/pg/video") to appropriate normalized paths or set the relevant fields on the
modelRequest (Model/Relay) so getModelRequest and shouldSelectChannel see the
correct route context; touch distributor.go around the existing /pg/ branch and
adjust getModelRequest/shouldSelectChannel logic to accept the normalized path
or additional /pg/* cases.
In `@relay/common/relay_utils.go`:
- Around line 193-196: The known-field whitelist removed multipart params
(quality, resolution_name, preset, seconds) from Metadata but
validateMultipartTaskRequest never maps those multipart form fields into the
TaskSubmitReq, causing them to be lost; update validateMultipartTaskRequest to
extract these multipart fields (quality, resolution_name, preset, seconds) from
the parsed multipart input and set the corresponding fields on the TaskSubmitReq
(or its Metadata map) so they persist through request processing, ensuring you
reference the TaskSubmitReq struct and the validateMultipartTaskRequest function
when adding the assignments.
In `@router/relay-router.go`:
- Around line 70-71: Path2RelayMode in relay/constant/relay_mode.go is missing
mappings for the playground video routes; update the Path2RelayMode function to
detect paths starting with "/pg/video/generations" and map GET requests to
RelayModeVideoFetchByID and other methods (POST) to RelayModeVideoSubmit (use
strings.HasPrefix(path, "/pg/video/generations") and method == http.MethodGet
for the fetch branch, and a fallback strings.HasPrefix(...) branch for submit),
making sure the GET check comes before the submit check so GETs resolve to
RelayModeVideoFetchByID.
---
Duplicate comments:
In `@controller/playground.go`:
- Around line 45-103: The new playground handlers (PlaygroundVideoSubmit,
PlaygroundImageGenerations, PlaygroundImageEdits, PlaygroundVideoFetch) are
missing the same use_access_token rejection that Playground implements; copy the
access-token guard logic from the Playground handler and add it at the top of
each of these functions (before calling setupPlaygroundTokenContext) so requests
that use the access token flow are rejected consistently across /pg/* routes.
In `@web/src/hooks/playground/useApiRequest.jsx`:
- Around line 454-468: The summary string in useApiRequest.jsx builds field
labels ("task_id:", "status:", "seconds:", "size:", "upstream_quality:",
"preset:", "url:") as hardcoded English; update the summary construction in the
function that builds `summary` to use the i18n `t()` translations (via the
component's `useTranslation()` hook) for each label key (e.g., t('任务ID'),
t('状态'), t('秒数'), t('大小'), t('上游质量'), t('预设'), t('URL')), and replace the
literal labels in the `summary` array with calls to `t(...)` while keeping the
same data expressions (`data.task_id || data.id`, `data.status`, `data.seconds
|| requestPayload.seconds`, `data.size || requestPayload.size`,
`requestedQuality || upstreamQuality`, `upstreamQuality`,
`requestPayload.preset`, `videoUrl`) so the rest of the code (variables
`summary`, `requestPayload`, `requestedQuality`, `upstreamQuality`, `videoUrl`)
remains unchanged.
- Around line 87-100: getImageFromMessageContent currently uses content.find()
and only returns the first image_url; change it to collect all image items by
using content.filter(item => item?.type === 'image_url') and map them to their
URL (item.image_url if string else item.image_url?.url), returning an array of
image URLs (or an empty array when none) instead of a single string; update any
callers expecting a single string to handle an array (or optionally return the
first element for backward compatibility and expose a new
getImagesFromMessageContent helper), referencing the getImageFromMessageContent
function name to locate and update usage.
---
Nitpick comments:
In `@controller/relay.go`:
- Around line 590-625: The code currently swallows errors from
adaptor.ParseTaskResult; update the ParseTaskResult call inside the
relay.GetTaskAdaptor block to log any non-nil error instead of ignoring
it—capture the error returned by adaptor.ParseTaskResult and emit a clear log
entry (using the controller's logger/processLogger) that includes context such
as result.Platform, task.ID (or task.TaskID), and the raw result.TaskData to
help diagnose adaptor-contract regressions; keep the existing fallback behavior
but ensure errors from ParseTaskResult are logged immediately where
ParseTaskResult is invoked.
🪄 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: 3c689ba1-52ef-4011-8859-48deb7890db5
📒 Files selected for processing (25)
common/endpoint_defaults.gocommon/endpoint_type.gocommon/endpoint_type_test.gocommon/model.goconstant/endpoint_type.gocontroller/playground.gocontroller/relay.gomiddleware/distributor.gorelay/channel/task/sora/adaptor.gorelay/channel/task/sora/adaptor_test.gorelay/channel/xai/adaptor.gorelay/channel/xai/adaptor_test.gorelay/channel/xai/constants.gorelay/channel/xai/dto.gorelay/common/relay_info.gorelay/common/relay_utils.gorelay/constant/relay_mode.gorelay/constant/relay_mode_test.gorouter/relay-router.goweb/src/components/playground/SettingsPanel.jsxweb/src/components/table/models/modals/EditModelModal.jsxweb/src/components/table/models/modals/EditPrefillGroupModal.jsxweb/src/constants/playground.constants.jsweb/src/helpers/api.jsweb/src/hooks/playground/useApiRequest.jsx
✅ Files skipped from review due to trivial changes (6)
- constant/endpoint_type.go
- web/src/components/table/models/modals/EditModelModal.jsx
- common/endpoint_defaults.go
- relay/channel/xai/dto.go
- relay/channel/xai/constants.go
- web/src/components/playground/SettingsPanel.jsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/helpers/api.js
- web/src/constants/playground.constants.js
| // playground requests may override group in body | ||
| if strings.HasPrefix(c.Request.URL.Path, "/pg/") { |
There was a problem hiding this comment.
/pg/* expansion exposes missing model/relay parsing for non-chat playground routes.
After broadening to /pg/, requests like /pg/images/edits and /pg/video/generations still rely on getModelRequest, which currently only has explicit /pg/chat/completions handling. This can leave modelRequest.Model empty or shouldSelectChannel incorrect for new playground routes.
🔧 Suggested direction (normalize path once for getModelRequest decisions)
func getModelRequest(c *gin.Context) (*ModelRequest, bool, error) {
+ pathForMatch := c.Request.URL.Path
+ if strings.HasPrefix(pathForMatch, "/pg/") {
+ pathForMatch = "/v1" + strings.TrimPrefix(pathForMatch, "/pg")
+ }
var modelRequest ModelRequest
shouldSelectChannel := true
var err error
- if strings.Contains(c.Request.URL.Path, "/v1/video/generations") {
+ if strings.Contains(pathForMatch, "/v1/video/generations") {
...
}
- if strings.HasPrefix(c.Request.URL.Path, "/v1/images/generations") {
+ if strings.HasPrefix(pathForMatch, "/v1/images/generations") {
...
- } else if strings.HasPrefix(c.Request.URL.Path, "/v1/images/edits") {
+ } else if strings.HasPrefix(pathForMatch, "/v1/images/edits") {
...
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@middleware/distributor.go` around lines 84 - 85, Requests under /pg/* can hit
getModelRequest which only handles /pg/chat/completions, leaving
modelRequest.Model empty and shouldSelectChannel incorrect for routes like
/pg/images/* or /pg/video/*; update the middleware so when
strings.HasPrefix(c.Request.URL.Path, "/pg/") you normalize the path (or add
explicit cases) before calling getModelRequest: either map known playground
prefixes (e.g., "/pg/chat", "/pg/images", "/pg/video") to appropriate normalized
paths or set the relevant fields on the modelRequest (Model/Relay) so
getModelRequest and shouldSelectChannel see the correct route context; touch
distributor.go around the existing /pg/ branch and adjust
getModelRequest/shouldSelectChannel logic to accept the normalized path or
additional /pg/* cases.
| "seconds": true, | ||
| "quality": true, | ||
| "resolution_name": true, | ||
| "preset": true, |
There was a problem hiding this comment.
Known-field whitelist now drops multipart params (quality/resolution_name/preset/seconds).
These keys are now excluded from Metadata, but validateMultipartTaskRequest does not assign them into TaskSubmitReq, so multipart requests lose them entirely.
✅ Proposed fix
req = TaskSubmitReq{
Prompt: formData.Get("prompt"),
Model: formData.Get("model"),
Mode: formData.Get("mode"),
Image: formData.Get("image"),
Size: formData.Get("size"),
+ Seconds: formData.Get("seconds"),
+ Quality: formData.Get("quality"),
+ ResolutionName: formData.Get("resolution_name"),
+ Preset: formData.Get("preset"),
Metadata: make(map[string]interface{}),
}📝 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.
| "seconds": true, | |
| "quality": true, | |
| "resolution_name": true, | |
| "preset": true, | |
| req = TaskSubmitReq{ | |
| Prompt: formData.Get("prompt"), | |
| Model: formData.Get("model"), | |
| Mode: formData.Get("mode"), | |
| Image: formData.Get("image"), | |
| Size: formData.Get("size"), | |
| Seconds: formData.Get("seconds"), | |
| Quality: formData.Get("quality"), | |
| ResolutionName: formData.Get("resolution_name"), | |
| Preset: formData.Get("preset"), | |
| Metadata: make(map[string]interface{}), | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/common/relay_utils.go` around lines 193 - 196, The known-field
whitelist removed multipart params (quality, resolution_name, preset, seconds)
from Metadata but validateMultipartTaskRequest never maps those multipart form
fields into the TaskSubmitReq, causing them to be lost; update
validateMultipartTaskRequest to extract these multipart fields (quality,
resolution_name, preset, seconds) from the parsed multipart input and set the
corresponding fields on the TaskSubmitReq (or its Metadata map) so they persist
through request processing, ensuring you reference the TaskSubmitReq struct and
the validateMultipartTaskRequest function when adding the assignments.
| playgroundRouter.POST("/video/generations", controller.PlaygroundVideoSubmit) | ||
| playgroundRouter.GET("/video/generations/:task_id", controller.PlaygroundVideoFetch) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Path2RelayMode handling for /pg/video paths
rg -n 'pg/video' relay/constant/relay_mode.go
# Check if distributor handles /pg/video paths
rg -n 'pg/video' middleware/distributor.goRepository: QuantumNous/new-api
Length of output: 45
🏁 Script executed:
#!/bin/bash
# First, let's see the context around lines 70-71 in relay-router.go
head -n 85 router/relay-router.go | tail -n 20Repository: QuantumNous/new-api
Length of output: 929
🏁 Script executed:
#!/bin/bash
# Examine the Path2RelayMode function structure
cat relay/constant/relay_mode.goRepository: QuantumNous/new-api
Length of output: 5123
🏁 Script executed:
#!/bin/bash
# Check middleware/distributor.go structure and what /pg/ routes it handles
cat middleware/distributor.go | head -n 300Repository: QuantumNous/new-api
Length of output: 11797
Add relay mode mapping for /pg/video/generations routes.
The /pg/video/generations routes (POST and GET) are registered in relay-router.go but lack corresponding mappings in Path2RelayMode (relay/constant/relay_mode.go). Like other playground routes (/pg/chat/completions, /pg/images/generations), these routes must be explicitly handled to map to RelayModeVideoSubmit and RelayModeVideoFetchByID respectively. Without these mappings, requests to /pg/video/generations will have RelayModeUnknown, causing incorrect routing behavior.
Add the following to Path2RelayMode:
} else if strings.HasPrefix(path, "/pg/video/generations") {
relayMode = RelayModeVideoSubmit
} else if strings.HasPrefix(path, "/pg/video/generations") && method == http.MethodGet {
relayMode = RelayModeVideoFetchByID
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@router/relay-router.go` around lines 70 - 71, Path2RelayMode in
relay/constant/relay_mode.go is missing mappings for the playground video
routes; update the Path2RelayMode function to detect paths starting with
"/pg/video/generations" and map GET requests to RelayModeVideoFetchByID and
other methods (POST) to RelayModeVideoSubmit (use strings.HasPrefix(path,
"/pg/video/generations") and method == http.MethodGet for the fetch branch, and
a fallback strings.HasPrefix(...) branch for submit), making sure the GET check
comes before the submit check so GETs resolve to RelayModeVideoFetchByID.
Summary by CodeRabbit