refactor(gateway): extract validateModelCapabilities - #1571
Conversation
Extract model capability validation logic into a dedicated function for JSON output, reasoning, tools, and web search. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughA large block of inline model capability validation checks in the chat endpoint has been extracted into a new utility. The new validateModelCapabilities function centralizes validation for JSON output, JSON schema, reasoning, tools, web search, and tool choice against provider/model capability mappings and now throws HTTP 400 for unsupported requests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Chat as ChatEndpoint
participant Validator as ValidateModelCapabilities
participant Registry as ProviderRegistry
Client->>Chat: POST /chat (model, provider, options)
Chat->>Validator: validateModelCapabilities(modelInfo, model, provider, options)
Validator->>Registry: fetch model/provider capability mappings
Registry-->>Validator: capability metadata
alt capabilities supported
Validator-->>Chat: OK
Chat->>Chat: proceed with routing/response generation
Chat-->>Client: 200 OK / streamed response
else unsupported capability
Validator-->>Chat: throws HTTP 400 (detailed message)
Chat-->>Client: 400 Bad Request (capability error)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
Pull request overview
This PR refactors model capability validation logic by extracting it from the main chat handler into a dedicated, well-documented function. This improves code maintainability and testability by separating validation concerns into a focused, reusable module.
Changes:
- Extracted validation logic into
validateModelCapabilities()function with clear interface - Reduced complexity in main chat handler by removing ~124 lines of inline validation
- Preserved all validation checks with identical error messages and logic
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/gateway/src/chat/tools/validate-model-capabilities.ts | New module containing extracted validation logic for JSON output, JSON schema, reasoning, tools, and web search capabilities |
| apps/gateway/src/chat/chat.ts | Replaced inline validation code with function call to new validation module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/gateway/src/chat/tools/validate-model-capabilities.ts`:
- Around line 86-115: The reasoning check currently inspects all
modelInfo.providers regardless of requestedProvider, so if a specific provider
was requested you must filter providers the same way as other checks: when
requestedProvider is defined, filter modelInfo.providers to only those with
providerId === requestedProvider before computing supportsReasoning; then
compute supportsReasoning from that filtered list (use reasoning_effort,
requestedModel, requestedProvider, modelInfo.providers, and
ProviderModelMapping.reasoning as referenced) and keep the existing logging and
HTTPException behavior if none of the filtered providers support reasoning.
🧹 Nitpick comments (1)
apps/gateway/src/chat/tools/validate-model-capabilities.ts (1)
46-54: Consider removing redundant type casts.The
providersproperty onModelDefinitionis already typed asProviderModelMapping[], so the(p as ProviderModelMapping)casts are redundant. While harmless, removing them would reduce noise.♻️ Example simplification
const providersToCheck = requestedProvider - ? modelInfo.providers.filter( - (p) => (p as ProviderModelMapping).providerId === requestedProvider, - ) + ? modelInfo.providers.filter((p) => p.providerId === requestedProvider) : modelInfo.providers; - const supportsJsonOutput = providersToCheck.some( - (provider) => (provider as ProviderModelMapping).jsonOutput === true, - ); + const supportsJsonOutput = providersToCheck.some((p) => p.jsonOutput === true);Also applies to: 65-76, 124-132
| // Check if reasoning_effort is specified but model doesn't support reasoning | ||
| // Skip this check for "auto" and "custom" models as they will be resolved dynamically | ||
| if ( | ||
| reasoning_effort !== undefined && | ||
| requestedModel !== "auto" && | ||
| requestedModel !== "custom" | ||
| ) { | ||
| const supportsReasoning = modelInfo.providers.some( | ||
| (provider) => (provider as ProviderModelMapping).reasoning === true, | ||
| ); | ||
|
|
||
| if (!supportsReasoning) { | ||
| logger.error( | ||
| `Reasoning effort specified for non-reasoning model: ${requestedModel}`, | ||
| { | ||
| requestedModel, | ||
| requestedProvider, | ||
| reasoning_effort, | ||
| modelProviders: modelInfo.providers.map((p) => ({ | ||
| providerId: p.providerId, | ||
| reasoning: (p as ProviderModelMapping).reasoning, | ||
| })), | ||
| }, | ||
| ); | ||
|
|
||
| throw new HTTPException(400, { | ||
| message: `Model ${requestedModel} does not support reasoning. Remove the reasoning_effort parameter or use a reasoning-capable model.`, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent provider filtering in reasoning check.
The JSON object check (lines 46-50), JSON schema check (lines 65-69), and tools check (lines 124-128) all filter providers by requestedProvider when specified. However, the reasoning check queries all providers in modelInfo.providers without respecting requestedProvider.
If a user requests a specific provider that doesn't support reasoning while other providers for the same model do, this check would incorrectly pass.
🔧 Proposed fix to add provider filtering for reasoning check
if (
reasoning_effort !== undefined &&
requestedModel !== "auto" &&
requestedModel !== "custom"
) {
- const supportsReasoning = modelInfo.providers.some(
+ const providersToCheck = requestedProvider
+ ? modelInfo.providers.filter(
+ (p) => (p as ProviderModelMapping).providerId === requestedProvider,
+ )
+ : modelInfo.providers;
+
+ const supportsReasoning = providersToCheck.some(
(provider) => (provider as ProviderModelMapping).reasoning === true,
);🤖 Prompt for AI Agents
In `@apps/gateway/src/chat/tools/validate-model-capabilities.ts` around lines 86 -
115, The reasoning check currently inspects all modelInfo.providers regardless
of requestedProvider, so if a specific provider was requested you must filter
providers the same way as other checks: when requestedProvider is defined,
filter modelInfo.providers to only those with providerId === requestedProvider
before computing supportsReasoning; then compute supportsReasoning from that
filtered list (use reasoning_effort, requestedModel, requestedProvider,
modelInfo.providers, and ProviderModelMapping.reasoning as referenced) and keep
the existing logging and HTTPException behavior if none of the filtered
providers support reasoning.
Summary
Extract model capability validation logic into a dedicated function for improved testability and maintainability. Validates JSON output, JSON schema, reasoning, tools, and web search capabilities.
Changes
validateModelCapabilities()function intools/validate-model-capabilities.tsTest Status
All 390 unit tests pass. Production build verified.
Summary by CodeRabbit