feat(models): redesign table with flattened structure - #1521
Conversation
- Flatten table to show one row per provider-model combination - Add expandable accordion for additional pricing (web search) - Add copy to clipboard button for model IDs - Optimize performance with React.memo and pre-computed data - Replace capability filter checkboxes with Toggle components - Add new Toggle component using @radix-ui/react-toggle - Remove unused code and simplify table rendering Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughAdds a Radix-based Toggle and dependency; refactors models listing into flattened provider-model rows with capability icons and per-row UI; introduces ProviderModelCard and ProvidersGrid and swaps Providers page to use ProvidersGrid; plus navbar, playground chat, settings context, and minor navigation-menu UI adjustments. Changes
Sequence Diagram(s)(No sequence diagrams generated — changes are primarily UI/component refactors without a cross-component sequential control flow that meets the diagram criteria.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/components/models/all-models.tsx (1)
700-701: Avoidanytype for sort values.Using
anyforaValueandbValueviolates the coding guideline to avoidanytype assertions. Consider using a proper union type.Suggested fix
- let aValue: any; - let bValue: any; + let aValue: string | number; + let bValue: string | number;Based on coding guidelines: "Never use
anyoras anytype assertions in TypeScript code unless absolutely necessary."
🧹 Nitpick comments (4)
apps/ui/src/components/models/all-models.tsx (4)
175-333: Consider addingdisplayNamefor the memoized component.The memoized
ModelTableRowcomponent lacks adisplayName, which can make debugging in React DevTools harder. While not critical, it's a good practice for consistency with other components like theTogglecomponent.Suggested addition after line 333
}, ); + +ModelTableRow.displayName = "ModelTableRow";
797-904: Duplicate sorting logic betweenmodelsWithProvidersandflattenedRows.The sorting logic at lines 823-903 largely duplicates the sorting in
modelsWithProviders(lines 692-784). SinceflattenedRowsis derived frommodelsWithProviders, sorting happens twice:
modelsWithProviderssorts models (for grid view)flattenedRowsre-sorts the flattened provider-model pairs (for table view)This duplication could cause inconsistent sort behavior between views and is computationally redundant. Consider either:
- Removing the sort from
modelsWithProvidersand only sorting inflattenedRows- Or extracting a shared sort comparator function
1489-1502: Inline callback in render loop may reduce memoization benefits.The
onNavigatecallback is defined inline, creating a new function reference on each render. This undermines theReact.memooptimization onModelTableRowsince the prop will always appear "changed."Consider memoizing the navigation handler or passing the row data and handling navigation inside
ModelTableRow.Option: Pass route data instead of callback
- onNavigate={() => - router.push( - `/models/${encodeURIComponent(row.model.id)}/${row.provider.providerId}`, - ) - } + modelId={row.model.id} + providerId={row.provider.providerId}Then handle navigation inside
ModelTableRowusinguseRouter, or create a memoized callback map.
994-1056: Duplicate capability icon logic withcomputeCapabilities.
getCapabilityIconsduplicates the logic fromcomputeCapabilities(lines 114-172) with minor label differences:
- "Structured JSON Output" vs "Structured JSON"
- "Native Web Search" vs "Web Search"
These inconsistencies could confuse users when switching between table and grid views. Consider consolidating into a single function used by both views.
- Add Products mega menu with AI Gateway, Observability, Chat Playground, Guardrails, and Integrations - Reorganize Resources dropdown menu items - Fix navigation menu z-index layering - Add ProvidersGrid and ProviderModelCard components - Fix dashboard context imports in settings - Fix playground chat state updates on load - Adjust floating input padding in chat UI Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/components/landing/navbar.tsx (1)
493-510: Mobile resources menu also usesLinkfor external items.Similar to the products menu,
resourcesItemsincludes items marked asexternal: true(e.g., Docs), but the mobile menu renders all items withLink. Theexternalflag should be checked to render external links as anchor tags.🔧 Suggested fix
{isMobileResourcesOpen ? ( <ul id="mobile-resources-menu" className="space-y-3 pl-4 pt-1" > {resourcesItems.map((item, index) => ( <li key={index}> - <Link - href={item.href as Route} - className="text-muted-foreground hover:text-accent-foreground block duration-150" - prefetch={true} - > - {item.name} - </Link> + {item.external ? ( + <a + href={item.href} + target="_blank" + rel="noopener noreferrer" + className="text-muted-foreground hover:text-accent-foreground block duration-150" + > + {item.name} + </a> + ) : ( + <Link + href={item.href as Route} + className="text-muted-foreground hover:text-accent-foreground block duration-150" + prefetch={true} + > + {item.name} + </Link> + )} </li> ))} </ul> ) : null}
🤖 Fix all issues with AI agents
In `@apps/ui/src/components/landing/navbar.tsx`:
- Around line 139-148: The Guardrails navigation item (title "Guardrails")
incorrectly marks an internal route as external; either remove the external
property from that object so the internal href "/features/guardrails" is treated
as an internal route, or if it should be an external docs link, replace the href
value with the full external URL (keeping external: true); update the object
containing title "Guardrails" and its external and href properties accordingly.
- Around line 455-472: The mobile products menu renders every entry from
productsItems using the Next.js Link component (see productsItems,
isMobileProductsOpen and Link in navbar.tsx), which breaks external URLs like
config.playgroundUrl; update the rendering to detect external links (either add
an external boolean to productsItems or detect href starting with http/https)
and for external entries render a standard anchor element with rel="noopener
noreferrer" and target="_blank" while using Link for internal routes—ensure the
same behavior/attributes as the desktop productsLinks dropdown.
In `@apps/ui/src/components/providers/provider-model-card.tsx`:
- Around line 70-71: The code accesses model.providerDetails[0].provider
directly which can throw when providerDetails is empty; update
provider-model-card.tsx to guard before using it (e.g., in the component that
defines provider and providerModelId) by checking model.providerDetails?.length
> 0 and either early-returning a fallback/placeholder UI or using a safe default
for provider and providerModelId; adjust the logic around the provider constant
and providerModelId interpolation so they are only computed when providerDetails
has at least one element.
- Around line 143-145: The Badge always uses hardcoded green styling while
displaying provider.stability; update the Badge in provider-model-card.tsx to
compute className based on provider.stability (or provider.stability ||
"STABLE") and reuse the same logic/flag used elsewhere
(shouldShowStabilityWarning) to apply different styles for "STABLE" vs
"EXPERIMENTAL"/"UNSTABLE" (e.g., green for stable, yellow/orange for
experimental, red for unstable). Locate the Badge render (the JSX using
provider.stability) and replace the static className with a small mapping or
conditional that returns appropriate tailwind classes for each stability level
so the visual color matches the reported stability.
🧹 Nitpick comments (4)
apps/ui/src/components/providers/provider-model-card.tsx (2)
10-15: Duplicate import ofTooltipProvider.
TooltipProvideris imported twice - once on line 10 and again within the destructured import on lines 11-15.🔧 Suggested fix
-import { TooltipProvider } from "@/lib/components/tooltip"; import { Tooltip, TooltipContent, + TooltipProvider, TooltipTrigger, } from "@/lib/components/tooltip";
64-68: Consider handling clipboard API errors gracefully.The
copyToClipboardfunction doesn't handle the case wherenavigator.clipboard.writeTextfails (e.g., in non-secure contexts or when permission is denied). The similar function inmodel-code-example-dialog.tsxshows proper error handling with try-catch and toast notifications.🔧 Suggested improvement
const copyToClipboard = (text: string) => { - navigator.clipboard.writeText(text); - setCopiedModel(text); - setTimeout(() => setCopiedModel(null), 2000); + navigator.clipboard.writeText(text) + .then(() => { + setCopiedModel(text); + setTimeout(() => setCopiedModel(null), 2000); + }) + .catch((err) => { + console.error("Failed to copy text:", err); + }); };apps/ui/src/components/providers/provider-models-grid.tsx (1)
36-122: Consider extracting shared utility functions.The
getCapabilityIcons,shouldShowStabilityWarning, andformatPricefunctions appear to be duplicated acrossprovider-models-grid.tsxandmodel-card.tsx(as seen in the relevant code snippets). Consider extracting these to a shared utility module to reduce code duplication.apps/ui/src/components/providers/providers-grid.tsx (1)
45-46: Consider consistent filtering criteria.The filter uses
p.name !== "LLM Gateway"(name comparison) alongsidep.id !== "custom"(id comparison). For consistency and robustness, consider using id-based filtering for both conditions if "LLM Gateway" has a stable id.
Add green dot placeholder for rows without expandable content to align with chevron icons on expandable rows. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ui/src/components/models/all-models.tsx (2)
421-428: Non-null assertion may cause runtime errors.Line 427 uses
!to assertproviders.find()always returns a result. If a model references a provider ID not in theprovidersarray, this will beundefinedand cause crashes when accessingproviderInfo.nameorproviderInfo.color.🛡️ Add defensive handling
const modelsWithProviders: ModelWithProviders[] = useMemo(() => { const baseModels = models.map((model) => ({ ...model, - providerDetails: model.mappings.map((mapping) => ({ - provider: mapping, - providerInfo: providers.find((p) => p.id === mapping.providerId)!, - })), + providerDetails: model.mappings + .map((mapping) => ({ + provider: mapping, + providerInfo: providers.find((p) => p.id === mapping.providerId), + })) + .filter((detail): detail is { provider: ApiModelProviderMapping; providerInfo: ApiProvider } => + detail.providerInfo !== undefined + ), }));
704-705: Avoidanytype - use proper union type.Per coding guidelines, avoid using
any. TheflattenedRowssorting (lines 841-842) already demonstrates the correct approach.♻️ Use proper type
- let aValue: any; - let bValue: any; + let aValue: string | number; + let bValue: string | number;
🧹 Nitpick comments (5)
apps/ui/src/components/models/all-models.tsx (5)
113-172: Consider consolidating withgetCapabilityIcons.
computeCapabilitiesduplicates nearly all logic fromgetCapabilityIcons(lines 998-1060). The only differences are minor label variations (e.g., "Structured JSON" vs "Structured JSON Output", "Web Search" vs "Native Web Search").Consider removing
getCapabilityIconsand reusingcomputeCapabilitiesthroughout, or extracting a shared capability mapping array.♻️ Suggested approach
// Define capability mappings once const CAPABILITY_MAPPINGS = [ { key: 'streaming', icon: Zap, label: 'Streaming', color: 'text-blue-500' }, { key: 'vision', icon: Eye, label: 'Vision', color: 'text-green-500' }, // ... rest of capabilities ] as const; // Reuse in both places function computeCapabilities(provider: ApiModelProviderMapping, model: ApiModel): CapabilityIcon[] { return CAPABILITY_MAPPINGS.filter(cap => { if (cap.key === 'imageGeneration') return model?.output?.includes('image'); return provider[cap.key as keyof ApiModelProviderMapping]; }).map(({ icon, label, color }) => ({ icon, label, color })); }
175-337: AdddisplayNamefor easier debugging.Memoized components should have a
displayNameproperty to aid debugging in React DevTools.♻️ Add displayName
const ModelTableRow = React.memo( ({ row, isExpanded, copiedModel, onToggleExpand, onCopy, onNavigate, }: { // ... props }) => { // ... component body }, ); +ModelTableRow.displayName = "ModelTableRow";
290-297: Truncated capabilities may hide important features.Only the first 4 capabilities are shown. If a model has more (e.g., streaming, vision, tools, reasoning, jsonOutput, webSearch), users won't see webSearch in the row.
Consider adding a "+N more" indicator or a tooltip showing all capabilities when truncated.
♻️ Show overflow indicator
<div className="flex justify-center gap-1"> {row.capabilities .slice(0, 4) .map(({ icon: Icon, label, color }) => ( <div key={label} className="p-0.5" title={label}> <Icon className={`h-4 w-4 ${color}`} /> </div> ))} + {row.capabilities.length > 4 && ( + <div + className="p-0.5 text-xs text-muted-foreground" + title={row.capabilities.slice(4).map(c => c.label).join(', ')} + > + +{row.capabilities.length - 4} + </div> + )} </div>
826-907: Duplicate sorting logic betweenmodelsWithProvidersandflattenedRows.Both
modelsWithProviders(lines 696-788) andflattenedRows(lines 826-907) contain similar sorting logic. WhilemodelsWithProviderssorting is needed for grid view andflattenedRowsfor table view, the duplication increases maintenance burden.Consider extracting a shared comparison function or computing sorting only in the view that needs it.
923-935: Consider user feedback on clipboard failure.The
catchblock logs the error but doesn't inform the user that copying failed. This could be confusing in restricted environments where clipboard access is denied.♻️ Add user feedback
} catch (err) { console.error("Failed to copy text:", err); + // Optionally show a toast notification or visual feedback }
- Updated products and resources items in the Navbar to include external links with appropriate handling. - Improved stability badge rendering in ProviderModelCard by introducing a utility function for dynamic class assignment based on stability levels. - Simplified the ModelTableRow component by removing unnecessary placeholder elements for better alignment.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/components/landing/navbar.tsx (1)
297-307: Handle external top‑level menu links on desktop.
menuItemsincludes external links (e.g., Docs) but the desktop render path always usesLink, so external items don’t gettarget="_blank"/reland bypass the external link handling you already apply on mobile. Consider mirroring the mobile approach here. As per coding guidelines, usenext/linkfor internal links and anchors for external ones.🔧 Proposed fix
{menuItems.map((item, index) => ( <NavigationMenuItem key={index}> <NavigationMenuLink asChild> - <Link - href={item.href as Route} - className="text-muted-foreground hover:text-accent-foreground block duration-150 px-4 py-2" - prefetch={true} - > - {item.name} - </Link> + {item.external ? ( + <a + href={item.href} + target="_blank" + rel="noopener noreferrer" + className="text-muted-foreground hover:text-accent-foreground block duration-150 px-4 py-2" + > + {item.name} + </a> + ) : ( + <Link + href={item.href as Route} + className="text-muted-foreground hover:text-accent-foreground block duration-150 px-4 py-2" + prefetch={true} + > + {item.name} + </Link> + )} </NavigationMenuLink> </NavigationMenuItem> ))}
🤖 Fix all issues with AI agents
In `@apps/ui/src/components/models/all-models.tsx`:
- Around line 203-215: The expand icon-only button (the element using
onToggleExpand and isExpanded) and the similar copy icon buttons should include
accessible labels and explicit button types; update the button elements to add
aria-label attributes that describe their action (e.g., "Expand model" /
"Collapse model" for the button that references onToggleExpand and "Copy model
ID" or similar for the copy buttons) and add type="button" to prevent accidental
form submission. Ensure the aria-label text changes based on isExpanded where
appropriate so screen readers get the correct action.
In `@apps/ui/src/components/providers/provider-model-card.tsx`:
- Around line 80-84: The copyToClipboard function currently calls
navigator.clipboard.writeText and unconditionally sets the copied state; update
copyToClipboard to await navigator.clipboard.writeText(text) inside a try-catch
(and check for navigator.clipboard availability), only call setCopiedModel(text)
and start the timeout when the write succeeds, and on failure log or silently
handle the error and do not set the copied state (optionally show an error UI);
reference the copyToClipboard function and setCopiedModel to locate where to
change behavior.
🧹 Nitpick comments (3)
apps/ui/src/components/landing/navbar.tsx (1)
95-168: DeriveproductsItemsfromproductsLinksto avoid drift.The two arrays duplicate the same source data, which risks inconsistencies between mobile and desktop (e.g., differing fallbacks for
config.playgroundUrl). Consider defining a single source (productsLinks) and deriving the mobile list from it.♻️ Proposed refactor
- const productsItems: Array<{ - name: string; - href: string; - external?: boolean; - }> = [ - { name: "AI Gateway", href: "/features/unified-api-interface" }, - { name: "Observability", href: "/features/performance-monitoring" }, - { - name: "Chat Playground", - href: config.playgroundUrl ?? "", - external: true, - }, - { - name: "Guardrails", - href: "/features/guardrails", - }, - { name: "Integrations", href: "/guides" }, - ]; - const productsLinks: Array<{ title: string; href: string; description: string; icon: React.ElementType; gradient: string; external?: boolean; }> = [ { title: "AI Gateway", href: "/features/unified-api-interface", @@ { title: "Chat Playground", href: config.playgroundUrl ?? "#", description: "Test prompts and compare model responses side by side, instantly.", icon: MessagesSquare, gradient: "hover:from-blue-500/20 hover:to-cyan-600/30 hover:shadow-blue-500/10 group-hover/product:text-blue-500 dark:group-hover/product:text-blue-400", external: true, }, @@ ]; + + const productsItems = productsLinks.map((product) => ({ + name: product.title, + href: product.href, + external: product.external, + }));apps/ui/src/components/providers/provider-model-card.tsx (1)
283-297: Usenext/linkfor links in apps/ui components.This component uses a raw
<a>tag. Per project guidance, prefernext/linkeven for external URLs.♻️ Suggested refactor
import { AlertTriangle, Copy, Check, Play } from "lucide-react"; +import Link from "next/link"; import { useState } from "react"; @@ <Button variant="default" size="default" className="w-full gap-2 font-semibold" onClick={(e) => e.stopPropagation()} asChild > - <a + <Link href={`${config.playgroundUrl}?model=${encodeURIComponent(providerModelId)}`} target="_blank" rel="noopener noreferrer" > <Play className="h-4 w-4" /> Try in Playground - </a> + </Link> </Button>As per coding guidelines, ...
apps/ui/src/components/models/all-models.tsx (1)
113-171: Avoid duplicate capability mappings to prevent label drift.
computeCapabilitiesandgetCapabilityIconsimplement nearly the same mapping with different labels. Consider reusingcomputeCapabilitiesso table/grid stay consistent.♻️ Suggested refactor
-function computeCapabilities( - provider: ApiModelProviderMapping, - model: ApiModel, -): CapabilityIcon[] { +function computeCapabilities( + provider: ApiModelProviderMapping, + model?: ApiModel, +): CapabilityIcon[] { @@ - const getCapabilityIcons = ( - provider: ApiModelProviderMapping, - model?: ApiModel, - ) => { - const capabilities = []; - if (provider.streaming) { - capabilities.push({ - icon: Zap, - label: "Streaming", - color: "text-blue-500", - }); - } - if (provider.vision) { - capabilities.push({ - icon: Eye, - label: "Vision", - color: "text-green-500", - }); - } - if (provider.tools) { - capabilities.push({ - icon: Wrench, - label: "Tools", - color: "text-purple-500", - }); - } - if (provider.reasoning) { - capabilities.push({ - icon: MessageSquare, - label: "Reasoning", - color: "text-orange-500", - }); - } - if (provider.jsonOutput) { - capabilities.push({ - icon: Braces, - label: "JSON Output", - color: "text-cyan-500", - }); - } - if (provider.jsonOutputSchema) { - capabilities.push({ - icon: FileJson2, - label: "Structured JSON Output", - color: "text-teal-500", - }); - } - if (model?.output?.includes("image")) { - capabilities.push({ - icon: ImagePlus, - label: "Image Generation", - color: "text-pink-500", - }); - } - if (provider.webSearch) { - capabilities.push({ - icon: Globe, - label: "Native Web Search", - color: "text-sky-500", - }); - } - return capabilities; - }; + const getCapabilityIcons = ( + provider: ApiModelProviderMapping, + model?: ApiModel, + ) => { + return computeCapabilities(provider, model); + };Also applies to: 996-1057
## Summary - Flatten models table to show one row per provider-model combination instead of grouped view - Add expandable accordion for additional pricing details (web search) - Add copy to clipboard button for model IDs - Optimize table performance with React.memo and pre-computed data - Replace capability filter checkboxes with Toggle components ## Changes - Added new Toggle component using @radix-ui/react-toggle - Refactored `all-models.tsx` for better performance and cleaner UX - Pre-compute capabilities and provider icons in useMemo - Use memoized row component to prevent unnecessary re-renders ## Test plan - [ ] Navigate to `/models` page - [ ] Verify table shows one row per provider-model combination - [ ] Click copy button next to model ID and verify clipboard copy works - [ ] Click chevron on rows with web search capability to expand additional pricing - [ ] Toggle capability filters and verify filtering works - [ ] Verify grid view still works unchanged - [ ] Run `pnpm build` to ensure no type errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Redesigned models table with per-provider rows, capability toggles, expanded pricing (Web Search, Per Request), and a dedicated toggle control for capability filtering. * New Providers grid and detailed provider model cards (copy model ID, pricing, capability icons, "Try in Playground"). * Products dropdown added to the navbar. * **UI Improvements** * Updated table labels, provider icons, navbar visuals, and mobile dropdown behavior. * Rendering optimizations for smoother models table performance. * **Bug Fixes** * Chat initialization refined to avoid overwriting selections. * Floating input padding adjusted in floating mode. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
Changes
all-models.tsxfor better performance and cleaner UXTest plan
/modelspagepnpm buildto ensure no type errors🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.