feat(gateway): gate routes by model output capability - #2828
Merged
Merged
Conversation
Distinguish model types by their declared `output` capability instead of ad-hoc, per-route signals, and enforce it uniformly. - Add `"ocr"` to the model `output` union and tag `mistral-ocr-latest` with `output: ["ocr"]`, so OCR is a first-class output capability rather than something only the per-provider `ocr` flag knows about. - New shared `validateModelOutput` helper: rejects (400) a model whose declared outputs don't intersect what the endpoint serves, pointing the caller at the right endpoint. - Chat (and the `/v1/responses` + `/v1/messages` routes that forward to it) now accept only text/image output, replacing the embedding+OCR special cases. This also closes the gap that let video/audio/image-only models through. - Images route guards that the requested model actually produces image output before forwarding to chat completions. - `/v1/models` surfaces OCR models as `output_modalities: ["text"]` (they return text; per-page pricing already distinguishes them). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughAdds OCR as a model output, introduces shared output-capability validation, maps OCR to text in the models API response, and applies the validator to chat completions and image generation/edit request flows. ChangesModel output capability routing
Estimated review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Guards against a non-text model (embeddings, OCR, speech, video, image gen) being added without a matching `output`, which chat completions would wrongly accept since a missing `output` defaults to ["text"]. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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: 816b49c617
ℹ️ 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".
| // handler to reject as "model not found". | ||
| function assertImageModel(model: string): void { | ||
| const slashIdx = model.indexOf("/"); | ||
| const modelKey = slashIdx > 0 ? model.slice(slashIdx + 1) : model; |
There was a problem hiding this comment.
When callers use provider-qualified names, this local parser does not mirror the chat parser: it leaves region suffixes in modelKey and treats unknown provider prefixes as built-in catalog lookups. For example, alibaba/qwen-plus:cn-beijing will not match the catalog and bypasses the new non-image check entirely, while a custom-provider request like mycompany/gpt-4o can be rejected against the built-in text-only gpt-4o before the custom provider path runs. Normalize with the same provider/model/region rules (and skip custom prefixes) before looking up models.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/gateway/src/images/images.ts`:
- Around line 563-576: `assertImageModel()` only skips the bare `custom`
sentinel, so `custom/...` still reaches the `models.find(...)` catalog lookup
and may be misclassified. Update `assertImageModel` to detect and short-circuit
any custom-provider image model before the catalog search, alongside the
existing `auto` and `custom` handling, so `validateModelOutput(...)` is not
applied to `custom/<model>` values.
In `@apps/gateway/src/models/models.ts`:
- Around line 204-216: The `/v1/models` output fallback in `models.ts` is
inconsistent with router validation because `outputModalities` only defaults
when `model.output` is undefined, while `getModelOutputs()` also treats an empty
array as text. Update the `outputModalities` mapping in the models list logic to
fall back to ["text"] for both undefined and empty `model.output`, keeping the
advertised outputs aligned with routing behavior and preserving the OCR-to-text
normalization.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f5e4ed42-7bf0-4a7e-9ddc-d8efb0e36c8e
📒 Files selected for processing (7)
apps/gateway/src/chat/tools/validate-model-capabilities.spec.tsapps/gateway/src/chat/tools/validate-model-capabilities.tsapps/gateway/src/images/images.tsapps/gateway/src/lib/validate-model-output.tsapps/gateway/src/models/models.tspackages/models/src/models.tspackages/models/src/models/mistral.ts
Comment on lines
+563
to
+576
| function assertImageModel(model: string): void { | ||
| const slashIdx = model.indexOf("/"); | ||
| const modelKey = slashIdx > 0 ? model.slice(slashIdx + 1) : model; | ||
| if (modelKey === "auto" || modelKey === "custom") { | ||
| return; | ||
| } | ||
| const modelInfo = models.find( | ||
| (m) => | ||
| m.id === model || | ||
| m.id === modelKey || | ||
| m.providers.some((p) => p.externalId === modelKey), | ||
| ); | ||
| if (modelInfo) { | ||
| validateModelOutput(modelInfo, modelKey, ["image"]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find how custom providers/models are represented and parsed.
rg -n --type=ts 'requestedProvider\s*===\s*"custom"|["'\'']custom/|indexOf\("/"\)|slice\(slashIdx \+ 1\)|split\("/"\)' apps packages
# Compare the image-route guard with the chat-route custom-provider bypass.
rg -n -C3 --type=ts 'assertImageModel|validateModelCapabilities|requestedProvider === "custom"' apps/gateway packages/modelsRepository: theopenco/llmgateway
Length of output: 25369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the image route and how request.model is parsed/resolved.
ast-grep outline apps/gateway/src/images/images.ts --view expanded
# Show the relevant image-route sections around parsing and the assert call.
sed -n '620,700p' apps/gateway/src/images/images.ts
sed -n '1060,1105p' apps/gateway/src/images/images.ts
# Inspect chat parsing for custom-provider model syntax as a comparison point.
sed -n '1,120p' apps/gateway/src/chat/tools/parse-model-input.ts
sed -n '1,120p' apps/gateway/src/chat/tools/resolve-model-info.tsRepository: theopenco/llmgateway
Length of output: 13316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the image route and how request.model is parsed/resolved.
ast-grep outline apps/gateway/src/images/images.ts --view expanded
# Show the relevant image-route sections around parsing and the assert call.
sed -n '620,700p' apps/gateway/src/images/images.ts
sed -n '1060,1105p' apps/gateway/src/images/images.ts
# Inspect chat parsing for custom-provider model syntax as a comparison point.
sed -n '1,120p' apps/gateway/src/chat/tools/parse-model-input.ts
sed -n '1,120p' apps/gateway/src/chat/tools/resolve-model-info.tsRepository: theopenco/llmgateway
Length of output: 13316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how the image route resolves/forwards the requested model.
sed -n '340,390p' apps/gateway/src/images/images.ts
sed -n '570,620p' apps/gateway/src/images/images.ts
# Find any parsing or special handling of custom/provider-scoped models in images routes.
rg -n --type=ts 'parseModelInput|resolveImageRequestModel|requestedProvider === "custom"|custom/' apps/gateway/src/images apps/gateway/src/chatRepository: theopenco/llmgateway
Length of output: 4037
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how the image route resolves/forwards the requested model.
sed -n '340,390p' apps/gateway/src/images/images.ts
sed -n '570,620p' apps/gateway/src/images/images.ts
# Find any parsing or special handling of custom/provider-scoped models in images routes.
rg -n --type=ts 'parseModelInput|resolveImageRequestModel|requestedProvider === "custom"|custom/' apps/gateway/src/images apps/gateway/src/chatRepository: theopenco/llmgateway
Length of output: 4037
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the image request schema and route docs/comments to see what model strings are allowed.
sed -n '1,120p' apps/gateway/src/images/images.ts
rg -n --type=ts --type=md 'images\.generations|imageGenerationsRequestSchema|request\.model|model:' apps/gateway/src/images apps/gateway/src -g '!**/*.spec.ts'Repository: theopenco/llmgateway
Length of output: 37981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the image request schema and route docs/comments to see what model strings are allowed.
sed -n '1,120p' apps/gateway/src/images/images.ts
rg -n --type=ts --type=md 'images\.generations|imageGenerationsRequestSchema|request\.model|model:' apps/gateway/src/images apps/gateway/src -g '!**/*.spec.ts'Repository: theopenco/llmgateway
Length of output: 37981
Skip custom/... before catalog lookup apps/gateway/src/images/images.ts:563-576
assertImageModel() only bypasses bare custom, so custom/<model> still falls through to models.find(...) and can be classified using unrelated catalog metadata. Short-circuit custom-provider image models first.
🤖 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/images/images.ts` around lines 563 - 576,
`assertImageModel()` only skips the bare `custom` sentinel, so `custom/...`
still reaches the `models.find(...)` catalog lookup and may be misclassified.
Update `assertImageModel` to detect and short-circuit any custom-provider image
model before the catalog search, alongside the existing `auto` and `custom`
handling, so `validateModelOutput(...)` is not applied to `custom/<model>`
values.
Mirror the model catalog 1:1 instead of collapsing OCR to "text", so third-party clients see output_modalities: ["ocr"] and can reference the same taxonomy. Adds "ocr" to the public schema enum. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/models/src/model-metadata.spec.ts (1)
14-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve the
ModelDefinition.outputunion here.
REQUIRED_OUTPUT_BY_FLAG.outputandoutputsare both widened tostring, so a typo in this contract test would still compile and quietly stop validating the intended modality. Keep them typed fromModelDefinition["output"]so output-vocabulary drift fails at compile time.Suggested change
+type ModelOutput = NonNullable<ModelDefinition["output"]>[number]; + const REQUIRED_OUTPUT_BY_FLAG: { flag: keyof ProviderModelMapping; - output: string; + output: ModelOutput; }[] = [ { flag: "imageGenerations", output: "image" }, { flag: "embeddings", output: "embedding" }, { flag: "speechGenerations", output: "audio" }, { flag: "videoGenerations", output: "video" }, { flag: "ocr", output: "ocr" }, ]; ... - const outputs: string[] = model.output ?? ["text"]; + const outputs: readonly ModelOutput[] = model.output ?? ["text"];Also applies to: 63-63
🤖 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 `@packages/models/src/model-metadata.spec.ts` around lines 14 - 23, Keep the contract test tied to the ModelDefinition.output union instead of widening REQUIRED_OUTPUT_BY_FLAG.output and outputs to string; update the REQUIRED_OUTPUT_BY_FLAG declaration and the related outputs usage in model-metadata.spec to reference ModelDefinition["output"] so any typo or new modality mismatch fails at compile time. Locate the check by the REQUIRED_OUTPUT_BY_FLAG constant and the outputs assertion, and preserve the existing modality mapping while tightening the types to the source-of-truth union.
🤖 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 `@packages/models/src/model-metadata.spec.ts`:
- Around line 14-23: Keep the contract test tied to the ModelDefinition.output
union instead of widening REQUIRED_OUTPUT_BY_FLAG.output and outputs to string;
update the REQUIRED_OUTPUT_BY_FLAG declaration and the related outputs usage in
model-metadata.spec to reference ModelDefinition["output"] so any typo or new
modality mismatch fails at compile time. Locate the check by the
REQUIRED_OUTPUT_BY_FLAG constant and the outputs assertion, and preserve the
existing modality mapping while tightening the types to the source-of-truth
union.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 83ccd9da-5699-4c5f-b3f3-0cb44cbd60ee
📒 Files selected for processing (1)
packages/models/src/model-metadata.spec.ts
This was referenced Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Distinguishes model types by their declared
outputcapability instead of ad-hoc, per-route signals, and enforces it uniformly across routes.Background: OCR models (e.g.
mistral-ocr-latest) genuinely return text, so they can't be told apart from chat models by output modality alone — that's why they previously needed a special-case check. Rather than a pricing heuristic, this PR makes the model'soutputcapability the single source of truth: OCR becomes a first-class output type, and every endpoint checks it.Changes
output: ["ocr"]— added"ocr"to theModelDefinition.outputunion and taggedmistral-ocr-latest, so OCR is a declared output capability rather than something only the per-providerocrflag knows about.validateModelOutputhelper (apps/gateway/src/lib/) — rejects (400) a model whose declared outputs don't intersect what the endpoint serves, and points the caller at the right endpoint (e.g. "Model X is an OCR model … Use the /v1/ocr endpoint instead.").validateModelCapabilitiesnow accepts onlytext/imageoutput, replacing the embedding + OCR special cases. Image output is allowed because image generation routes through/v1/chat/completions(so text-only, image-only, and text+image all pass). This also closes the gap that previously let video / audio / image-only models through./v1/responsesand/v1/messagesforward to chat, so they inherit this./v1/models— surfacesoutput_modalities1:1 with the model catalog, including["ocr"](added to the public schema enum), so third-party clients can reference the same modality taxonomy. Per-page pricing (ocr_page) remains alongside.Routes that resolve models through a per-mapping capability flag (
embeddings,speechGenerations,videoGenerations,ocr) already reject mismatched models at resolution, so they were left as-is.Defaults / safety
A missing
outputdefaults to["text"], so the ~290 existing chat models need no change — only non-text models opt in. A newmodel-metadatainvariant test asserts that any model carrying a non-text capability flag (imageGenerations/embeddings/speechGenerations/videoGenerations/ocr) declares the matchingoutput, so a non-text model can't silently default to text and get wrongly accepted on chat. (This invariant would have caught the original OCR bug.)Tests
validateModelCapabilities - output capabilitycases: rejects OCR/video/audio on chat, allows image-output models.model-metadatainvariant: output ⇄ capability-flag consistency./v1/modelsspec assertsmistral-ocr-latest→output_modalities: ["ocr"].pnpm buildandpnpm formatare green.🤖 Generated with Claude Code