feat(models): introduce ModelsSupported unified component - #346
Conversation
…roviders and models
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardRoute
participant ModelsRoute
participant ModelsSupported
User->>DashboardRoute: Navigates to dashboard models
DashboardRoute->>ModelsSupported: Render with isDashboard=true
ModelsSupported->>ModelsSupported: Fetch and group providers/models
ModelsSupported->>User: Display providers and models
User->>ModelsRoute: Navigates to models page
ModelsRoute->>ModelsSupported: Render with isDashboard=false
ModelsSupported->>ModelsSupported: Fetch and group providers/models
ModelsSupported->>User: Display providers and models
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
smakosh
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/ui/src/components/models-supported.tsx (1)
161-163: Consider using a more unique key for model cards.While the current key should work in most cases, using just
${model.providerId}-${model.model}could theoretically have collisions if a provider has duplicate model names. Consider adding an index or using a more unique identifier.- {models.map((model) => ( + {models.map((model, index) => ( <Card - key={`${model.providerId}-${model.model}`} + key={`${model.providerId}-${model.model}-${index}`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/ui/src/components/models-supported.tsx(1 hunks)apps/ui/src/routes/dashboard/_layout/models.tsx(2 hunks)apps/ui/src/routes/models.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/ui/src/routes/dashboard/_layout/models.tsx (1)
apps/ui/src/components/models-supported.tsx (1)
ModelsSupported(105-209)
apps/ui/src/routes/models.tsx (1)
apps/ui/src/components/models-supported.tsx (1)
ModelsSupported(105-209)
apps/ui/src/components/models-supported.tsx (4)
apps/ui/src/components/provider-keys/provider-keys-list.tsx (1)
providerLogoComponents(44-53)packages/models/src/providers.ts (1)
ProviderId(109-109)apps/ui/src/lib/utils.ts (2)
cn(4-6)formatContextSize(13-24)apps/ui/src/lib/env.ts (1)
DOCS_URL(4-5)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e / run
🔇 Additional comments (2)
apps/ui/src/routes/dashboard/_layout/models.tsx (1)
3-3: Clean refactor to use the new ModelsSupported component.The simplification of this route to use the centralized
ModelsSupportedcomponent is a good improvement that reduces code duplication.Also applies to: 16-16
apps/ui/src/routes/models.tsx (1)
5-5: Clean refactor to use the new ModelsSupported component.Good consolidation of the provider/model display logic into a reusable component.
Also applies to: 12-12
| const groupedProviders = modelDefinitions.reduce< | ||
| Record<string, ProviderModel[]> | ||
| >((acc, def) => { | ||
| def.providers.forEach((map) => { | ||
| const provider = providerDefinitions.find((p) => p.id === map.providerId)!; | ||
| if (!acc[provider.name]) { | ||
| acc[provider.name] = []; | ||
| } | ||
| acc[provider.name].push({ | ||
| model: def.model, | ||
| providerId: map.providerId, | ||
| providerName: provider.name, | ||
| inputPrice: map.inputPrice, | ||
| outputPrice: map.outputPrice, | ||
| contextSize: map.contextSize, | ||
| }); | ||
| }); | ||
| return acc; | ||
| }, {}); |
There was a problem hiding this comment.
Add error handling for missing provider definitions.
The non-null assertion on line 79 could cause a runtime error if a provider ID exists in model definitions but not in provider definitions.
- const provider = providerDefinitions.find((p) => p.id === map.providerId)!;
+ const provider = providerDefinitions.find((p) => p.id === map.providerId);
+ if (!provider) {
+ console.warn(`Provider definition not found for ID: ${map.providerId}`);
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const groupedProviders = modelDefinitions.reduce< | |
| Record<string, ProviderModel[]> | |
| >((acc, def) => { | |
| def.providers.forEach((map) => { | |
| const provider = providerDefinitions.find((p) => p.id === map.providerId)!; | |
| if (!acc[provider.name]) { | |
| acc[provider.name] = []; | |
| } | |
| acc[provider.name].push({ | |
| model: def.model, | |
| providerId: map.providerId, | |
| providerName: provider.name, | |
| inputPrice: map.inputPrice, | |
| outputPrice: map.outputPrice, | |
| contextSize: map.contextSize, | |
| }); | |
| }); | |
| return acc; | |
| }, {}); | |
| const groupedProviders = modelDefinitions.reduce< | |
| Record<string, ProviderModel[]> | |
| >((acc, def) => { | |
| def.providers.forEach((map) => { | |
| - const provider = providerDefinitions.find((p) => p.id === map.providerId)!; | |
| + const provider = providerDefinitions.find((p) => p.id === map.providerId); | |
| + if (!provider) { | |
| + console.warn(`Provider definition not found for ID: ${map.providerId}`); | |
| + return; | |
| + } | |
| if (!acc[provider.name]) { | |
| acc[provider.name] = []; | |
| } | |
| acc[provider.name].push({ | |
| model: def.model, | |
| providerId: map.providerId, | |
| providerName: provider.name, | |
| inputPrice: map.inputPrice, | |
| outputPrice: map.outputPrice, | |
| contextSize: map.contextSize, | |
| }); | |
| }); | |
| return acc; | |
| }, {}); |
🤖 Prompt for AI Agents
In apps/ui/src/components/models-supported.tsx around lines 75 to 93, the code
uses a non-null assertion when finding a provider by ID, which can cause a
runtime error if the provider is not found. Modify the code to check if the
provider exists before accessing its properties. If the provider is missing,
handle the case gracefully, such as by skipping that entry or logging a warning,
to prevent runtime exceptions.
| <a | ||
| href={`${DOCS_URL}/v1/models`} | ||
| target="_blank" | ||
| className="inline-flex items-center gap-2 text-sm text-muted-foreground" | ||
| > | ||
| <span>Data sourced from @llmgateway/models</span> | ||
| <ExternalLink className="w-4 h-4" /> | ||
| </a> |
There was a problem hiding this comment.
Add security attributes to external link.
Links with target="_blank" should include rel="noopener noreferrer" to prevent security vulnerabilities.
<a
href={`${DOCS_URL}/v1/models`}
target="_blank"
+ rel="noopener noreferrer"
className="inline-flex items-center gap-2 text-sm text-muted-foreground"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a | |
| href={`${DOCS_URL}/v1/models`} | |
| target="_blank" | |
| className="inline-flex items-center gap-2 text-sm text-muted-foreground" | |
| > | |
| <span>Data sourced from @llmgateway/models</span> | |
| <ExternalLink className="w-4 h-4" /> | |
| </a> | |
| <a | |
| href={`${DOCS_URL}/v1/models`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="inline-flex items-center gap-2 text-sm text-muted-foreground" | |
| > | |
| <span>Data sourced from @llmgateway/models</span> | |
| <ExternalLink className="w-4 h-4" /> | |
| </a> |
🤖 Prompt for AI Agents
In apps/ui/src/components/models-supported.tsx around lines 198 to 205, the
anchor tag with target="_blank" is missing the security attributes rel="noopener
noreferrer". Add rel="noopener noreferrer" to the anchor tag to prevent security
vulnerabilities related to opening links in a new tab.
| const providerLogoComponents: Partial< | ||
| Record<ProviderId, React.FC<React.SVGProps<SVGSVGElement>> | null> | ||
| > = { | ||
| openai: OpenAiLogo, | ||
| anthropic: anthropicLogo, | ||
| "google-vertex": GoogleVertexLogo, | ||
| "inference.net": InferenceLogo, | ||
| "kluster.ai": KlusterLogo, | ||
| "together.ai": TogetherAiLogo, | ||
| "google-ai-studio": GoogleStudioAiLogo, | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify if the "llmgateway" provider logo should be included.
The provider logo mapping is missing the "llmgateway" provider that appears in the similar mapping in provider-keys-list.tsx. Please verify if this omission is intentional.
🏁 Script executed:
#!/bin/bash
# Check if llmgateway provider exists in the models/providers data
ast-grep --pattern 'id: "llmgateway"'Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for occurrences of "llmgateway" in the repository
rg -n '"llmgateway"' .
rg -n llmgateway .Length of output: 20692
🏁 Script executed:
#!/bin/bash
# Show logo imports in models-supported.tsx
sed -n '1,100p' apps/ui/src/components/models-supported.tsx | rg -n 'import .*Logo'Length of output: 619
🏁 Script executed:
#!/bin/bash
# Display the provider logo mapping and getProviderIcon logic
rg -n "providerLogoComponents" -A 15 -B 5 apps/ui/src/components/models-supported.tsx
rg -n "function getProviderIcon" -A 15 -B 5 apps/ui/src/components/models-supported.tsxLength of output: 1447
Add “llmgateway” to the provider logo mapping
The UI currently falls back to the generic logo for the “llmgateway” provider. To match the mapping in provider-keys-list.tsx and ensure the correct icon is displayed, please:
• In apps/ui/src/components/models-supported.tsx, add the import for the LLMGateway logo alongside the other SVG imports (around lines 8–14):
import TogetherAiLogo from "@/assets/models/together-ai.svg?react";
+import LLMGatewayLogo from "@/assets/models/llmgateway.svg?react";• Include the “llmgateway” entry in providerLogoComponents (around lines 26–36):
const providerLogoComponents: Partial<Record<ProviderId, React.FC<React.SVGProps<SVGSVGElement>> | null>> = {
openai: OpenAiLogo,
anthropic: anthropicLogo,
"google-vertex": GoogleVertexLogo,
"inference.net": InferenceLogo,
"kluster.ai": KlusterLogo,
"together.ai": TogetherAiLogo,
"google-ai-studio": GoogleStudioAiLogo,
+ llmgateway: LLMGatewayLogo,
};This ensures getProviderIcon("llmgateway") renders the correct gateway logo instead of the generic one.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const providerLogoComponents: Partial< | |
| Record<ProviderId, React.FC<React.SVGProps<SVGSVGElement>> | null> | |
| > = { | |
| openai: OpenAiLogo, | |
| anthropic: anthropicLogo, | |
| "google-vertex": GoogleVertexLogo, | |
| "inference.net": InferenceLogo, | |
| "kluster.ai": KlusterLogo, | |
| "together.ai": TogetherAiLogo, | |
| "google-ai-studio": GoogleStudioAiLogo, | |
| }; | |
| // (somewhere alongside the other SVG imports) | |
| import TogetherAiLogo from "@/assets/models/together-ai.svg?react"; | |
| import LLMGatewayLogo from "@/assets/models/llmgateway.svg?react"; | |
| // … | |
| const providerLogoComponents: Partial< | |
| Record<ProviderId, React.FC<React.SVGProps<SVGSVGElement>> | null> | |
| > = { | |
| openai: OpenAiLogo, | |
| anthropic: anthropicLogo, | |
| "google-vertex": GoogleVertexLogo, | |
| "inference.net": InferenceLogo, | |
| "kluster.ai": KlusterLogo, | |
| "together.ai": TogetherAiLogo, | |
| "google-ai-studio": GoogleStudioAiLogo, | |
| llmgateway: LLMGatewayLogo, | |
| }; |
🤖 Prompt for AI Agents
In apps/ui/src/components/models-supported.tsx around lines 8 to 14, add an
import statement for the LLMGateway logo SVG component alongside the existing
logo imports. Then, between lines 26 and 36, add an entry for "llmgateway" in
the providerLogoComponents mapping, assigning it the imported LLMGateway logo
component. This will ensure the "llmgateway" provider uses its specific logo
instead of falling back to the generic icon.
2 participants
Summary by CodeRabbit
New Features
Refactor