Conversation
WalkthroughThis PR adds validation to reject requests with Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7286556cc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| googleSingleImageModels.has(requestedModel) && | ||
| (image_config?.n ?? 1) > 1 | ||
| ) { |
There was a problem hiding this comment.
Scope n>1 rejection to Google providers only
This check rejects purely by requestedModel, but parseModelInput maps unknown provider/model inputs to requestedProvider === "custom" while preserving the model name. As a result, a custom provider request like acme/gemini-3-pro-image-preview with image_config.n: 2 now fails with a 400 even though the gateway cannot assume Google model limits for custom backends. Please gate this condition on provider/family (non-custom Google paths) so custom OpenAI-compatible providers are not incorrectly blocked.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR tightens gateway validation for Google “image-preview” chat-completions models by explicitly rejecting multi-image requests (image_config.n > 1) that aren’t actually supported, preventing the gateway from implying multi-image output will work.
Changes:
- Add a gateway-side
400validation error whenimage_config.n > 1is requested forgemini-3-pro-image-previewandgemini-3.1-flash-image-preview. - Refactor the model check into a shared
Setused for both image input counting and validation. - Add API test coverage to ensure
image_config.n > 1is rejected for both models.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/actions/src/prepare-request-body.ts | Minor adjustment to Google imageConfig initialization when applying image generation options. |
| apps/gateway/src/chat/chat.ts | Introduces the image_config.n > 1 rejection logic for the two Google image-preview models. |
| apps/gateway/src/api.spec.ts | Adds tests asserting the new 400 behavior for both affected models. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/gateway/src/api.spec.ts (1)
1326-1355: Consider usingtest.each()for better test isolation and reporting.The
for...ofloop pattern works but Vitest'stest.each()provides better test isolation, clearer output, and aligns with patterns seen in E2E tests (per learnings about parallel execution with.each()).♻️ Suggested refactor using test.each()
- for (const model of [ - "gemini-3-pro-image-preview", - "gemini-3.1-flash-image-preview", - ]) { - test(`/v1/chat/completions rejects image_config.n > 1 for ${model}`, async () => { + test.each([ + "gemini-3-pro-image-preview", + "gemini-3.1-flash-image-preview", + ])("/v1/chat/completions rejects image_config.n > 1 for %s", async (model) => { const res = await app.request("/v1/chat/completions", { method: "POST", headers: { "Content-Type": "application/json", Authorization: "Bearer fake", }, body: JSON.stringify({ model, messages: [ { role: "user", content: "Generate a simple image of a red circle.", }, ], image_config: { n: 2, }, }), }); expect(res.status).toBe(400); const json = await res.json(); expect(json.message).toContain("image_config.n > 1"); }); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/gateway/src/api.spec.ts` around lines 1326 - 1355, Replace the for...of loop that creates tests for each model with Vitest's table-driven runner by converting the looped test blocks into a single test.each(...) call over the model array; locate the current loop that iterates over ["gemini-3-pro-image-preview", "gemini-3.1-flash-image-preview"] and the inline test(`/v1/chat/completions rejects image_config.n > 1 for ${model}` ...) and change it so each model is passed as a parameter to test.each (improves isolation and reporting) while keeping the same request body shape, assertions (expect(res.status).toBe(400) and expect(json.message).toContain("image_config.n > 1")), and usage of app.request.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/gateway/src/api.spec.ts`:
- Around line 1326-1355: Replace the for...of loop that creates tests for each
model with Vitest's table-driven runner by converting the looped test blocks
into a single test.each(...) call over the model array; locate the current loop
that iterates over ["gemini-3-pro-image-preview",
"gemini-3.1-flash-image-preview"] and the inline test(`/v1/chat/completions
rejects image_config.n > 1 for ${model}` ...) and change it so each model is
passed as a parameter to test.each (improves isolation and reporting) while
keeping the same request body shape, assertions (expect(res.status).toBe(400)
and expect(json.message).toContain("image_config.n > 1")), and usage of
app.request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c7a8db19-c87c-410e-929b-655efb1376c4
📒 Files selected for processing (3)
apps/gateway/src/api.spec.tsapps/gateway/src/chat/chat.tspackages/actions/src/prepare-request-body.ts
What changed
image_config.n > 1forgemini-3-pro-image-previewimage_config.n > 1forgemini-3.1-flash-image-previewWhy
Google image-preview chat completions were accepting
nat the gateway layer, but the local gateway check showed they still returned a single image in practice. The safer behavior is to reject unsupported multi-image requests explicitly instead of implying thatn > 1might work.Impact
Clients now get a clear
400with guidance to usen=1for the two Google image-preview models.Root cause
The gateway exposed
image_config.n, and earlier work attempted to map it into the Google request path. That created the appearance of support even though these models do not provide reliable multi-image output via chat completions.Validation
pnpm vitest run packages/actions/src/prepare-request-body.spec.tspnpm vitest run apps/gateway/src/api.spec.ts -t "rejects image_config.n > 1"pnpm formatpnpm buildhttp://localhost:4001/v1/chat/completionsfor both Google image-preview models withimage_config.n: 2Summary by CodeRabbit
New Features
400error with a clear message.Tests
Refactor