refactor(model): per-provider jsonOutput flag across mappings - #1068
Conversation
![terragon-labs[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
Refactor the jsonOutput attribute from the model definition to individual provider model mappings. This enables more precise specification of JSON output support per provider instead of at the global model level. Adjusted model files and relevant code to check jsonOutput from providers. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
|
Warning Rate limit exceeded@steebchen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughMoved JSON output capability from model-level to provider-level across model catalogs and types; removed Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant ModelCatalog
participant Provider
Note over ModelCatalog #D6EAF8: Models include per-provider flags (jsonOutput/jsonOutputSchema)
Client->>Gateway: Chat request (model id, optional provider)
Gateway->>ModelCatalog: Load model and providers[]
ModelCatalog-->>Gateway: Return model + providers
Note right of Gateway #F7F9F9: Determine JSON support by scanning providers
alt specific provider requested
Gateway->>Provider: Inspect requestedProvider.jsonOutput / jsonOutputSchema
else no specific provider
Gateway->>Provider: Inspect any provider.jsonOutput / jsonOutputSchema
end
alt supports jsonOutput
Gateway->>Gateway: Apply JSON validation/formatting (schema if required)
Gateway-->>Client: Return JSON response
else not supported
Gateway-->>Client: Return unsupported error or fallback
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Potential attention areas:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
github-actions
Bot
changed the title
Previously, the jsonOutput property was stored directly on models and synced as a top-level attribute. This change removes the jsonOutput field from the model schema and instead derives jsonOutput support based on the providers associated with each model. This refactors the handling of the jsonOutput capability to be provider-specific rather than model-wide, improving accuracy of capability reporting and data consistency across components. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/gateway/src/models/models.ts (1)
228-228: Logic is correct; consider removing redundant|| false.The refactor correctly computes
json_outputby checking if any provider hasjsonOutputset to true, which aligns with the PR's goal of moving jsonOutput to per-provider configuration. The logic handles edge cases properly (empty providers array, undefined jsonOutput values).However,
Array.prototype.some()always returns a boolean, so the|| falsefallback is redundant.Apply this diff to simplify:
-json_output: model.providers.some((p) => p.jsonOutput) || false, +json_output: model.providers.some((p) => p.jsonOutput),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/gateway/src/models/models.ts(1 hunks)apps/playground/src/lib/model-utils.ts(1 hunks)apps/worker/src/services/sync-models.ts(0 hunks)packages/db/src/schema.ts(0 hunks)
💤 Files with no reviewable changes (2)
- apps/worker/src/services/sync-models.ts
- packages/db/src/schema.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level import; never use require() or dynamic import()
Files:
apps/playground/src/lib/model-utils.tsapps/gateway/src/models/models.ts
apps/{ui,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/{ui,playground}/**/*.{ts,tsx}: In Next.js apps, use next/link for links and next/navigation’s router for programmatic navigation (avoid tags, window.location, or legacy next/router)
Use cookies for user settings that are not persisted in the database to ensure SSR works
apps/{ui,playground}/**/*.{ts,tsx}: Usenext/linkfor links andnext/navigation's router for programmatic navigation
Use cookies for user settings not saved in the database to ensure SSR works
Files:
apps/playground/src/lib/model-utils.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyin this TypeScript project unless absolutely necessary
Always use top-levelimport; do not userequireor dynamicimport()
Files:
apps/playground/src/lib/model-utils.tsapps/gateway/src/models/models.ts
apps/{gateway,api}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/{gateway,api}/**/*.ts: Use Hono for HTTP routing in Gateway and API services
Use Zod schemas for request/response validation in server routes
Files:
apps/gateway/src/models/models.ts
{apps/{api,gateway}/**/*.ts,packages/db/**/*.ts}
📄 CodeRabbit inference engine (CLAUDE.md)
For read operations, use
db().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/gateway/src/models/models.ts
🧬 Code graph analysis (1)
apps/playground/src/lib/model-utils.ts (1)
packages/db/src/schema.ts (1)
provider(554-587)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: build / run
- GitHub Check: test / run
- GitHub Check: generate / run
- GitHub Check: lint / run
- GitHub Check: e2e-shards (5)
- GitHub Check: e2e-shards (2)
- GitHub Check: e2e-shards (3)
- GitHub Check: e2e-shards (4)
- GitHub Check: e2e-shards (1)
- GitHub Check: autofix
🔇 Additional comments (1)
apps/playground/src/lib/model-utils.ts (1)
53-53: LGTM! Consistent with per-provider capability pattern.The change to check
provider?.jsonOutputaligns with the PR's refactoring objective and is consistent with how other capabilities (streaming, vision, tools, reasoning) are already checked at the provider level.
…bilities The jsonOutput property is now determined by checking if any providers have jsonOutput enabled, rather than relying on a top-level property. This fix adjusts MultiModelSelector, AllModels filtering logic, and ModelComparison to accurately reflect provider-level jsonOutput support. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
…port checks Previously, JSON output support was checked across all model providers regardless of the requested provider. This change filters the providers list by the requestedProvider when specified, ensuring JSON output and schema support checks are performed only on relevant providers. This improves accuracy in determining supported response formats based on the user's requested provider. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
…noutput-model-mapping-qhb3tl
…ders Removed the jsonOutput boolean property from the ProviderDefinition interface, all provider definitions, and related database schema and sync code. This cleans up unused legacy code related to jsonOutput support for providers. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
- Added handling for response_format types 'json_object' and 'json_schema' - Set 'responseMimeType' to 'application/json' accordingly - Prepared groundwork for future JSON Schema conversion for Google format Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
Use explicit type assertion for provider in json_output check to ensure correct boolean evaluation instead of relying on loose truthiness check. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
- Introduced a new capabilities section in ProviderCard component displaying various provider features like streaming, vision, tools, reasoning, and JSON output. - Each capability is represented with an icon and tooltip explaining the feature. - Fixed a variable reference in AllModels component to correctly check jsonOutput capability. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/ui/src/components/models/provider-card.tsx (1)
233-304: Consider refactoring to reduce code duplication.The Capabilities section contains significant duplication across the five capability badges. Each follows an identical pattern with only the property name, icon, label, color scheme, and tooltip text varying.
Consider extracting this into a reusable pattern:
// Define capabilities configuration const capabilities = [ { key: 'streaming' as const, icon: Zap, label: 'Streaming', tooltip: 'Supports streaming responses', colorClasses: 'bg-blue-50 dark:bg-blue-950/30 text-blue-700 dark:text-blue-300' }, { key: 'vision' as const, icon: Eye, label: 'Vision', tooltip: 'Supports vision and image inputs', colorClasses: 'bg-green-50 dark:bg-green-950/30 text-green-700 dark:text-green-300' }, // ... other capabilities ] as const; // In the render: <div className="flex flex-wrap gap-2"> {capabilities.map(({ key, icon: Icon, label, tooltip, colorClasses }) => provider[key] && ( <Tooltip key={key}> <TooltipTrigger asChild> <div className={`flex items-center gap-1.5 px-2 py-1 rounded-md ${colorClasses} text-xs`}> <Icon className="h-3.5 w-3.5" /> <span>{label}</span> </div> </TooltipTrigger> <TooltipContent> <p>{tooltip}</p> </TooltipContent> </Tooltip> ) )} </div>This would reduce ~60 lines to ~20 and make it easier to add new capabilities in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/ui/src/components/models/all-models.tsx(2 hunks)apps/ui/src/components/models/provider-card.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ui/src/components/models/all-models.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level import; never use require() or dynamic import()
Files:
apps/ui/src/components/models/provider-card.tsx
apps/{ui,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/{ui,playground}/**/*.{ts,tsx}: In Next.js apps, use next/link for links and next/navigation’s router for programmatic navigation (avoid tags, window.location, or legacy next/router)
Use cookies for user settings that are not persisted in the database to ensure SSR works
apps/{ui,playground}/**/*.{ts,tsx}: Usenext/linkfor links andnext/navigation's router for programmatic navigation
Use cookies for user settings not saved in the database to ensure SSR works
Files:
apps/ui/src/components/models/provider-card.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyin this TypeScript project unless absolutely necessary
Always use top-levelimport; do not userequireor dynamicimport()
Files:
apps/ui/src/components/models/provider-card.tsx
🧬 Code graph analysis (1)
apps/ui/src/components/models/provider-card.tsx (1)
packages/db/src/schema.ts (1)
provider(554-586)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: test / run
- GitHub Check: lint / run
- GitHub Check: build / run
- GitHub Check: generate / run
- GitHub Check: e2e-shards (5)
- GitHub Check: e2e-shards (4)
- GitHub Check: e2e-shards (1)
- GitHub Check: e2e-shards (2)
- GitHub Check: e2e-shards (3)
- GitHub Check: autofix
🔇 Additional comments (3)
apps/ui/src/components/models/provider-card.tsx (3)
3-24: LGTM! Import additions support the new Capabilities section.The new icon imports and Tooltip components are correctly added to support the capability badges feature.
146-146: LGTM! Spacing adjustment improves layout.The
mb-4addition provides appropriate visual separation before the new Capabilities section.
289-301: Correct implementation of per-provider JSON Output capability.The JSON Output badge correctly reads from
provider.jsonOutput, aligning with this PR's refactoring goal to move JSON output support from model-level to provider-level configuration.
- Import Braces icon from lucide-react - Add hasJsonOutput check for model providers - Display JSON Output item with Braces icon and cyan color in UI Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
1 participant
Summary
Changes
Core types
Model definitions
UI
Gateway logic
End-to-end tests
Migration notes
Test plan
Impact
Notes
📎 Task: https://www.terragonlabs.com/task/b85a3de2-f3cc-49f1-8f37-6a8db343a1e6
Summary by CodeRabbit
Refactor
UI
Bug Fixes