fix(gateway): reject image input for non-vision models - #2230
Conversation
Previously, requests with image_url content to a non-vision provider (e.g. deepseek/deepseek-v4-flash) were forwarded to the upstream API, which returned a cryptic 400 like "unknown variant `image_url`, expected `text`". Validate vision capability in the same pass that checks tools / json output / web_search and return a clear gateway-side error instead. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughWhen a chat request contains images the gateway now passes ChangesVision Capability Validation
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/gateway/src/chat/tools/validate-model-capabilities.spec.ts (1)
65-106: ⚡ Quick winStrengthen rejection assertions to verify error contract (status/message), not just exception type.
On Line 73, Line 81, and Line 105, asserting only
toThrow(HTTPException)is a bit loose. Please also assert the 400 status and expected message shape so the clear gateway error contract can’t regress silently.Proposed test tightening
it("rejects when explicit provider does not support vision", () => { - expect(() => - validateModelCapabilities( - noVisionModel, - "deepseek-v4-flash", - "deepseek", - { hasImages: true }, - ), - ).toThrow(HTTPException); + try { + validateModelCapabilities( + noVisionModel, + "deepseek-v4-flash", + "deepseek", + { hasImages: true }, + ); + expect.fail("Expected HTTPException"); + } catch (error) { + expect(error).toBeInstanceOf(HTTPException); + const httpError = error as HTTPException; + expect(httpError.status).toBe(400); + expect(httpError.message).toContain("does not support image input"); + } });🤖 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 `@apps/gateway/src/chat/tools/validate-model-capabilities.spec.ts` around lines 65 - 106, The three rejection tests that call validateModelCapabilities (the ones named "rejects when explicit provider does not support vision", "rejects when no provider in the model supports vision", and "rejects when explicit non-vision provider is picked even if a sibling has vision") should not only assert toThrow(HTTPException) but also verify the HTTP error contract: capture the thrown error from validateModelCapabilities (or use expect(() => ...).toThrowError and then inspect the caught error), assert error instanceof HTTPException, assert error.status === 400, and assert the error.message/shape contains the expected user-facing text (e.g., includes "vision" or the specific rejection message your gateway emits). Use the validateModelCapabilities symbol and HTTPException class to locate the code under test and tighten each failing test accordingly.
🤖 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 `@apps/gateway/src/chat/tools/validate-model-capabilities.spec.ts`:
- Around line 65-106: The three rejection tests that call
validateModelCapabilities (the ones named "rejects when explicit provider does
not support vision", "rejects when no provider in the model supports vision",
and "rejects when explicit non-vision provider is picked even if a sibling has
vision") should not only assert toThrow(HTTPException) but also verify the HTTP
error contract: capture the thrown error from validateModelCapabilities (or use
expect(() => ...).toThrowError and then inspect the caught error), assert error
instanceof HTTPException, assert error.status === 400, and assert the
error.message/shape contains the expected user-facing text (e.g., includes
"vision" or the specific rejection message your gateway emits). Use the
validateModelCapabilities symbol and HTTPException class to locate the code
under test and tighten each failing test accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 12fdfece-279c-4753-b75e-c88b577235b4
📒 Files selected for processing (3)
apps/gateway/src/chat/chat.tsapps/gateway/src/chat/tools/validate-model-capabilities.spec.tsapps/gateway/src/chat/tools/validate-model-capabilities.ts
There was a problem hiding this comment.
Pull request overview
This PR adds an explicit “vision capability” validation so chat requests containing image_url / image parts fail fast (with a clear gateway 400) when routed to non-vision model/provider mappings—especially for explicit provider-prefixed model requests that bypass the auto-router’s vision filtering.
Changes:
- Add
hasImagesoption tovalidateModelCapabilities()and reject image-containing requests when the selected model/provider mapping(s) are not vision-capable. - Pass
hasImagesfrom the chat route into capability validation. - Add unit tests covering vision-capability validation scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/gateway/src/chat/tools/validate-model-capabilities.ts | Adds a new vision capability check based on hasImages and provider mapping vision flags. |
| apps/gateway/src/chat/tools/validate-model-capabilities.spec.ts | Introduces unit tests for the new vision validation behavior. |
| apps/gateway/src/chat/chat.ts | Threads hasImages into validateModelCapabilities() from the request message analysis. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Validate vision capability when the request contains images. | ||
| // Skip this check for "auto" and "custom" models as they will be resolved dynamically. | ||
| if (hasImages && requestedModel !== "auto" && requestedModel !== "custom") { | ||
| const providersToCheck = requestedProvider | ||
| ? modelInfo.providers.filter( | ||
| (p) => (p as ProviderModelMapping).providerId === requestedProvider, | ||
| ) | ||
| : modelInfo.providers; | ||
|
|
||
| const supportsVision = providersToCheck.some( | ||
| (provider) => (provider as ProviderModelMapping).vision === true, | ||
| ); | ||
|
|
||
| if (!supportsVision) { | ||
| throw new HTTPException(400, { | ||
| message: requestedProvider | ||
| ? `Provider ${requestedProvider} does not support image input for model ${requestedModel}. Remove the image content or use a vision-capable model.` | ||
| : `Model ${requestedModel} does not support image input. Remove the image content or use a vision-capable model.`, | ||
| }); |
| expect(() => | ||
| validateModelCapabilities( | ||
| noVisionModel, | ||
| "deepseek-v4-flash", | ||
| "deepseek", | ||
| { hasImages: true }, | ||
| ), | ||
| ).toThrow(HTTPException); | ||
| }); | ||
|
|
||
| it("rejects when no provider in the model supports vision", () => { | ||
| expect(() => | ||
| validateModelCapabilities(noVisionModel, "deepseek-v4-flash", undefined, { | ||
| hasImages: true, | ||
| }), | ||
| ).toThrow(HTTPException); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0689441de
ℹ️ 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".
|
|
||
| // Validate vision capability when the request contains images. | ||
| // Skip this check for "auto" and "custom" models as they will be resolved dynamically. | ||
| if (hasImages && requestedModel !== "auto" && requestedModel !== "custom") { |
There was a problem hiding this comment.
When callers use a named custom provider such as my-custom/gpt-4o-mini with image_url content, requestedModel is the raw model name (gpt-4o-mini), not the literal custom, while requestedProvider is custom. This means the new check no longer skips custom providers and rejects every image request because the mock custom mapping is created with vision: false in resolve-model-info.ts; that regresses the documented behavior that custom provider models are not validated and breaks vision-capable OpenAI-compatible custom endpoints.
Useful? React with 👍 / 👎.
Use deepseek-v4-flash (no providers will gain vision support upstream) and qwen3-max (mixed alibaba/novita/embercloud) instead of synthetic fixtures. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
image_urlcontent sent to a non-vision provider (e.g.deepseek/deepseek-v4-flash) was forwarded to the upstream, which then 400'd with a cryptic "unknown variant `image_url`, expected `text`". The auto-router already filters non-vision providers, but explicit-provider requests bypassed that check.validateModelCapabilitiesso we error upfront with a clear, actionable message ("Provider X does not support image input for model Y") instead of forwarding bad payloads to upstream.Verification (local
pnpm devagainst real DeepSeek upstream)deepseek/deepseek-v4-flashtext-only → upstream 200, content returneddeepseek/deepseek-v4-flash+image_url→ gateway 400 with the new message, no upstream call (previously: upstream 400 with garbled error)deepseek/deepseek-v4-flashtext-only without provider prefix still worksTest plan
pnpm exec vitest run apps/gateway/src/chat/tools/validate-model-capabilities.spec.ts— 7 new unit tests passSummary by CodeRabbit
New Features
Tests