fix: support gemini-3.1-flash-image-preview image generation in chat#13335
fix: support gemini-3.1-flash-image-preview image generation in chat#13335cherry-ai-bot[bot] wants to merge 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
Thanks for the update — overall this PR is on the right track and addresses #13332 with clear tests.
I left a few non-blocking suggestions:
- clarify or narrow broad gemini-3.x flash-image regex matching scope,
- ensure model list/capability metadata remains fully aligned,
- consider removing OpenRouter-specific image-model gating and reusing unified capability detection to avoid duplicated logic.
No blocking issues from my side.
| 'kandinsky(?:-[\\w-]+)?' | ||
| ] | ||
|
|
||
| const GEMINI_FLASH_IMAGE_MODELS = ['gemini-2.5-flash-image(?:-[\\w-]+)?', 'gemini-3(?:\\.\\d+)?-flash-image(?:-[\\w-]+)?'] |
There was a problem hiding this comment.
Suggestion: The new matcher is intentionally broad. If product scope is strictly for now, consider tightening to explicit known IDs (or adding a comment explaining why broad matching is desired) to avoid accidental enablement for future unknown model variants.
| @@ -9,7 +11,7 @@ export function buildGeminiGenerateImageParams(): Record<string, any> { | |||
|
|
|||
There was a problem hiding this comment.
Suggestion: Agree with reviewer concern that OpenRouter-specific model gating may be unnecessary here. If image-generation capability is already determined upstream by unified model capability detection, consider removing this provider-specific matcher and reusing that capability signal to avoid duplicated source-of-truth.
EurFelux
left a comment
There was a problem hiding this comment.
Overall the PR looks good — clean refactoring with solid test coverage.
One minor suggestion: replace the .slice(1) with a named constant for better maintainability.
| const OPENAI_IMAGE_GENERATION_MODELS = [...OPENAI_TOOL_USE_IMAGE_GENERATION_MODELS, 'gpt-image-1'] | ||
|
|
||
| const MODERN_IMAGE_MODELS = ['gemini-3(?:\\.\\d+)?-pro-image(?:-[\\w-]+)?'] | ||
| const MODERN_IMAGE_MODELS = [...GEMINI_FLASH_IMAGE_MODELS.slice(1), ...GEMINI_PRO_IMAGE_MODELS] |
There was a problem hiding this comment.
GEMINI_FLASH_IMAGE_MODELS.slice(1) relies on array index position to exclude gemini-2.5-flash-image — this is fragile and hard to read. If someone later inserts a new entry at the beginning of the array, the semantics silently change.
Suggest extracting a named constant to make the intent explicit:
const GEMINI_3X_FLASH_IMAGE_MODELS = ['gemini-3(?:\\.\\d+)?-flash-image(?:-[\\w-]+)?']
const GEMINI_FLASH_IMAGE_MODELS = [
'gemini-2.5-flash-image(?:-[\\w-]+)?',
...GEMINI_3X_FLASH_IMAGE_MODELS
]Then use GEMINI_3X_FLASH_IMAGE_MODELS here directly instead of .slice(1).
|
Manual test passed. |
Summary
Related