feat(api): use db models + providers - #1424
Conversation
![terragon-labs[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
- Introduce /internal/models and /internal/providers endpoints in API with full OpenAPI and Zod schemas. - Implement routes to fetch active models with mappings and providers sorted by createdAt descending. - Add fetchModels and fetchProviders client utilities with caching. - Update playground and UI apps to consume new internal APIs instead of static or legacy data. - Refactor model types and usages across playground and UI to use API data structures. - Cleanup publishedAt fields, favoring createdAt for sorting and display. - Add React Query hooks in UI for fetching models and providers. - Sync worker service updated to store new fields from model definitions. - Updated database schema to support new model and mapping fields, including stability and discounts. - Remove deprecated publishedAt usage from static models. This change centralizes model and provider data fetching through internal API endpoints, enhancing data freshness and consistency across apps. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughPR adds an internal API for models/providers, introduces ApiModel/ApiProvider types and fetch helpers, migrates DB schema and worker sync to new fields (stability, discount, json/web flags, etc.), removes Changes
Sequence Diagram(s)sequenceDiagram
participant Worker as Worker (sync-models)
participant DB as Database
participant API as API Server (/internal)
participant UI as Frontend (Server/Client)
Note over Worker,DB: periodic sync
Worker->>DB: upsert models & mappings (aliases, releasedAt, discount, stability, json/web flags)
DB-->>Worker: ack
Note over API,DB: internal endpoints read DB
UI->>API: GET /internal/models & /internal/providers
API->>DB: query active models + mappings, query active providers
DB-->>API: models & providers
API-->>UI: JSON payload (models/providers)
Note over UI: server-side fetch or client fetch via helpers
UI->>UI: parse ApiModel shapes, render components (ModelSelector, TimelineClient, etc.)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✏️ Tip: You can disable this entire section by setting 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
There was a problem hiding this comment.
Pull request overview
This PR migrates the data layer from static model definitions to API-driven data, replacing publishedAt with createdAt for sorting models by when they were added to LLM Gateway.
Changes:
- Adds internal API endpoints
/internal/modelsand/internal/providersthat return models with provider mappings sorted bycreatedAtdescending - Removes
publishedAtfield from model definitions and replaces all sorting logic to usecreatedAt(from API) with fallback toreleasedAt - Updates frontend components across UI and Playground to consume new
ApiModel,ApiModelProviderMapping, andApiProvidertypes, with string-based pricing fields instead of numbers
Reviewed changes
Copilot reviewed 43 out of 47 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/db/src/schema.ts | Adds new fields (releasedAt, aliases, description, stability, jsonOutput, jsonOutputSchema, webSearch, discount) to model and modelProviderMapping tables |
| apps/worker/src/services/sync-models.ts | Updates sync logic to populate new database fields from model definitions |
| apps/api/src/routes/internal-models.ts | Implements new internal API endpoints for fetching models and providers |
| apps/api/src/index.ts | Routes /internal to new internal-models handler |
| packages/models/src/models.ts | Removes publishedAt field from ModelDefinition interface |
| packages/models/src/models/*.ts | Removes publishedAt assignments from all model definitions |
| apps/ui/src/lib/fetch-models.ts | New server-side fetch utilities for models/providers with ApiModel types |
| apps/ui/src/hooks/useModels.ts | New React Query hooks for client-side model/provider fetching |
| apps/playground/src/lib/fetch-models.ts | Playground version of fetch utilities |
| packages/shared/src/components/model-selector.tsx | Updates sorting to use createdAt instead of publishedAt |
| apps/playground/src/components/model-selector.tsx | Migrates to ApiModel types, updates price handling to parse strings |
| apps/ui/src/components/models/*.tsx | Updates all model display components to use new API types and string-based prices |
| apps/ui/src/components/shared/model-search.tsx | Updates to use createdAt for sorting and grouping by month |
| apps/ui/src/app/timeline/page.tsx | Migrates to API-fetched models with createdAt handling |
| apps/playground/src/lib/mapmodels.ts | Updates model mapping to handle string prices from API |
| apps/*/lib/api/v1.d.ts | OpenAPI type definitions for new internal endpoints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Free models | ||
| if (basePrice === 0) { | ||
| return { label: "Free", original: "Free" }; | ||
| } | ||
|
|
||
| const discountNum = mapping.discount ? parseFloat(mapping.discount) : 0; |
There was a problem hiding this comment.
The parseFloat() call on line 193 could return NaN if basePriceStr contains invalid numeric data. Consider adding validation to handle NaN cases, such as checking isNaN(basePrice) after parsing and returning an appropriate error or "Unknown" label to prevent incorrect pricing displays.
| // Free models | |
| if (basePrice === 0) { | |
| return { label: "Free", original: "Free" }; | |
| } | |
| const discountNum = mapping.discount ? parseFloat(mapping.discount) : 0; | |
| if (Number.isNaN(basePrice)) { | |
| return { label: "Unknown" }; | |
| } | |
| // Free models | |
| if (basePrice === 0) { | |
| return { label: "Free", original: "Free" }; | |
| } | |
| const parsedDiscount = mapping.discount ? parseFloat(mapping.discount) : 0; | |
| const discountNum = Number.isNaN(parsedDiscount) ? 0 : parsedDiscount; |
| const priceNum = parseFloat(price); | ||
| const discountNum = discount ? parseFloat(discount) : 0; |
There was a problem hiding this comment.
Similar to the playground model-selector, parseFloat() on lines 53, 54, 103-107 could return NaN for invalid price/discount strings. Add NaN validation after parsing to prevent calculation errors and incorrect price displays.
| const priceNum = parseFloat(price); | |
| const discountNum = discount ? parseFloat(discount) : 0; | |
| const priceNumRaw = parseFloat(price); | |
| if (Number.isNaN(priceNumRaw)) { | |
| return "—"; | |
| } | |
| const priceNum = priceNumRaw; | |
| let discountNum = 0; | |
| if (discount != null) { | |
| const discountNumRaw = parseFloat(discount); | |
| if (!Number.isNaN(discountNumRaw)) { | |
| discountNum = discountNumRaw; | |
| } | |
| } |
| if (discount) { | ||
| const discountedPrice = price * 1e6 * (1 - discount); | ||
| const priceNum = parseFloat(price); | ||
| const discountNum = discount ? parseFloat(discount) : 0; |
There was a problem hiding this comment.
parseFloat() calls on lines 53-57 need NaN validation. Invalid price/discount strings would cause calculation errors in pricing display.
| const discountNum = discount ? parseFloat(discount) : 0; | |
| if (Number.isNaN(priceNum)) { | |
| return "—"; | |
| } | |
| let discountNum = discount ? parseFloat(discount) : 0; | |
| if (Number.isNaN(discountNum) || discountNum <= 0) { | |
| discountNum = 0; | |
| } |
| if (discount) { | ||
| const discountedPrice = (price * 1e6 * (1 - discount)).toFixed(2); | ||
| const priceNum = parseFloat(price); | ||
| const discountNum = discount ? parseFloat(discount) : 0; |
There was a problem hiding this comment.
Multiple parseFloat() calls (lines 247, 575-579, etc.) lack NaN validation. This pattern is repeated throughout the file and could cause calculation errors with invalid price data.
| const discountNum = discount ? parseFloat(discount) : 0; | |
| if (Number.isNaN(priceNum)) { | |
| return "—"; | |
| } | |
| let discountNum = 0; | |
| if (discount) { | |
| const parsedDiscount = parseFloat(discount); | |
| if (!Number.isNaN(parsedDiscount)) { | |
| discountNum = parsedDiscount; | |
| } | |
| } |
| if (discount) { | ||
| const discountedPrice = (price * 1e6 * (1 - discount)).toFixed(2); | ||
| const priceNum = parseFloat(price); | ||
| const discountNum = discount ? parseFloat(discount) : 0; |
There was a problem hiding this comment.
parseFloat() on lines 247-251 lacks NaN validation which could cause errors in price calculations.
| const discountNum = discount ? parseFloat(discount) : 0; | |
| if (Number.isNaN(priceNum)) { | |
| return "—"; | |
| } | |
| let discountNum = 0; | |
| if (discount !== null && discount !== undefined) { | |
| const parsedDiscount = parseFloat(discount); | |
| if (!Number.isNaN(parsedDiscount)) { | |
| discountNum = parsedDiscount; | |
| } | |
| } |
| context: p.contextSize ?? undefined, | ||
| inputPrice: p.inputPrice ? parseFloat(p.inputPrice) : undefined, | ||
| outputPrice: p.outputPrice ? parseFloat(p.outputPrice) : undefined, |
There was a problem hiding this comment.
The parseFloat() calls on lines 51-52 for inputPrice and outputPrice need NaN validation to handle invalid price data safely.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
apps/worker/src/services/sync-models.ts (2)
52-86: Use??(not||) to avoid losing valid falsy values (notably booleans likefree).
free: modelDef.free || nullwill storenullwhenfree === false. Same pattern appears for other newly-added fields. This is correctness-affecting givenmodel.freeis a boolean column. (Seepackages/db/src/schema.tslines 665-700.)Proposed fix
id: modelDef.id, name: modelDef.name || null, - aliases: "aliases" in modelDef ? modelDef.aliases || null : null, + aliases: "aliases" in modelDef ? (modelDef.aliases ?? null) : null, description: - "description" in modelDef ? modelDef.description || null : null, + "description" in modelDef ? (modelDef.description ?? null) : null, family: modelDef.family, - free: "free" in modelDef ? modelDef.free || null : null, - output: "output" in modelDef ? modelDef.output || null : null, + free: "free" in modelDef ? (modelDef.free ?? null) : null, + output: "output" in modelDef ? (modelDef.output ?? null) : null, stability: - "stability" in modelDef ? modelDef.stability || null : null, + "stability" in modelDef ? (modelDef.stability ?? null) : null, releasedAt: - "releasedAt" in modelDef ? modelDef.releasedAt || null : null, + "releasedAt" in modelDef ? (modelDef.releasedAt ?? null) : null,
102-176: Fix boolean and numeric field handling to prevent data loss and runtime errors.The code uses incorrect patterns for several field types:
Boolean fields (
jsonOutput,jsonOutputSchema,webSearch,vision,reasoning,tools): The patternmapping.foo || nullwill coercefalsetonull, silently losing data. Use the nullish coalescing operator instead:mapping.foo ?? null.Numeric fields (
discount,inputPrice,outputPrice, etc.): The check!== undefinedbefore calling.toString()will not catchnull. Ifnullslips through,.toString()will throw. Change to!= nullto guard against bothnullandundefined.Integer fields (
contextSize,maxOutput): The patternmapping.foo || nullwill incorrectly convert0tonull. Use??instead.Note:
streamingcorrectly uses explicit=== falsecheck and does not need changes.Both the update block (lines 102–176) and insert block (178–246) need these fixes.
Example fixes
jsonOutput: - "jsonOutput" in mapping ? mapping.jsonOutput || null : null, + "jsonOutput" in mapping ? (mapping.jsonOutput ?? null) : null, discount: - "discount" in mapping && mapping.discount !== undefined + "discount" in mapping && mapping.discount != null ? mapping.discount.toString() : null, contextSize: - "contextSize" in mapping ? mapping.contextSize || null : null, + "contextSize" in mapping ? (mapping.contextSize ?? null) : null,apps/playground/src/app/group/page.tsx (1)
6-31: Prefer removing the “Fetch models and providers…” comment (guidelines: no unnecessary comments).The code is self-explanatory; consider dropping Lines 26-27 comment. As per coding guidelines, no unnecessary code comments.
packages/shared/src/components/model-selector.tsx (1)
490-495: Removeany/as any(repo guideline) by tightening types.
ids.has(p.id as any)andupdateFilter(..., value: any)violate the TypeScript guideline.
As per coding guidelines, avoidany/as anyunless absolutely necessary.Proposed fix
const availableProviders = React.useMemo(() => { - const ids = new Set( + const ids = new Set<ProviderId>( allEntries.filter((e) => e.mapping).map((e) => e.mapping!.providerId), ); - return providers.filter((p) => ids.has(p.id as any)); + return providers.filter((p) => ids.has(p.id)); }, [allEntries, providers]); @@ - const updateFilter = (key: keyof FilterState, value: any) => { - setFilters((prev) => ({ ...prev, [key]: value })); - }; + const updateFilter = <K extends keyof FilterState>( + key: K, + value: FilterState[K], + ) => { + setFilters((prev) => ({ ...prev, [key]: value })); + };Also applies to: 572-574
apps/playground/src/components/playground/chat-page-client.tsx (2)
63-68:availableModelswon’t update when API-drivenmodels/providersprops change.
useState(mapped)only captures the initial value, but the PR introduces 60s revalidation, so the UI can drift from server data.Proposed fix
- const mapped = useMemo( - () => mapModels(models, providers), - [models, providers], - ); - const [availableModels] = useState<ComboboxModel[]>(mapped); + const availableModels = useMemo( + () => mapModels(models, providers), + [models, providers], + );
185-223: Avoidany/as anyin TS here; it’s not necessary.
This violates the TS guideline and also makesmessage.partshandling brittle (e.g., NaN/shape mismatches won’t be caught).Proposed direction (minimal typing via `unknown` + guards)
- const imageUrlParts = (message.parts as any[]) - .filter((p: any) => p.type === "image_url" && p.image_url?.url) + const parts = Array.isArray(message.parts) + ? (message.parts as unknown[]) + : []; + const isImageUrlPart = ( + p: unknown, + ): p is { type: "image_url"; image_url: { url: string } } => + typeof p === "object" && + p !== null && + (p as { type?: unknown }).type === "image_url" && + typeof (p as { image_url?: { url?: unknown } }).image_url?.url === + "string"; + + const imageUrlParts = parts + .filter(isImageUrlPart) .map((p: any) => ({ type: "image_url", image_url: { url: p.image_url.url }, }));- body: bodyToSave as any, + body: bodyToSave,Also applies to: 315-317
apps/ui/src/components/providers/provider-models-grid.tsx (2)
96-122:formatPrice: handle NaN + clamp discount to avoid$NaN/ negative prices.
parseFloatcan produce NaN (bad API data, empty string), anddiscountshould be constrained to[0, 1].Proposed fix
const formatPrice = ( price: string | null | undefined, discount?: string | null, ) => { if (price === null || price === undefined) { return "—"; } - const priceNum = parseFloat(price); - const discountNum = discount ? parseFloat(discount) : 0; + const priceNum = Number.parseFloat(price); + if (!Number.isFinite(priceNum)) { + return "—"; + } + const rawDiscount = discount ? Number.parseFloat(discount) : 0; + const discountNum = Number.isFinite(rawDiscount) + ? Math.min(1, Math.max(0, rawDiscount)) + : 0;
124-137: Add defensive check or guarantee for non-emptyproviderDetailsarray.While the current call site in
[id]/page.tsxconstructs models with guaranteed non-emptyproviderDetails, accessing[0]without validation is unsafe. The type allows an empty array, and this pattern is repeated across multiple components (all-models.tsx,models-supported.tsx). Either filter out models with emptyproviderDetailsbefore rendering or add a guard check.apps/ui/src/components/models/model-detail-card.tsx (1)
36-72:formatPricerenders unformatted numbers (likely a UI bug).
HereoriginalPrice/discountedPriceare raw floats, unlike the other components usingtoFixed(2).Proposed fix
- const originalPrice = priceNum * 1e6; + const originalPrice = (priceNum * 1e6).toFixed(2); if (discountNum > 0) { - const discountedPrice = priceNum * 1e6 * (1 - discountNum); + const discountedPrice = (priceNum * 1e6 * (1 - discountNum)).toFixed(2); return ( <div className="flex flex-col justify-items-center"> <div className="flex items-center gap-1"> <span className="line-through text-muted-foreground text-xs"> ${originalPrice} </span> <span className="text-green-600 font-semibold"> ${discountedPrice} </span>apps/ui/src/components/models/all-models.tsx (4)
100-185:providers.find(...)!can crash the page; handle missing providerInfo.
With API-driven data (and revalidation), it’s safer to tolerate partial responses.Proposed fix
- providerDetails: model.mappings.map((mapping) => ({ - provider: mapping, - providerInfo: providers.find((p) => p.id === mapping.providerId)!, - })), + providerDetails: model.mappings + .map((mapping) => { + const providerInfo = providers.find((p) => p.id === mapping.providerId); + if (!providerInfo) { + return null; + } + return { provider: mapping, providerInfo }; + }) + .filter( + ( + x, + ): x is { + provider: ApiModelProviderMapping; + providerInfo: ApiProvider; + } => x !== null, + ),
282-286: “Discounted” should meandiscount > 0, not “discount string exists”.
Right now"0"will be treated as discounted.Proposed fix
- if ( - filters.capabilities.discounted && - !model.providerDetails.some((p) => p.provider.discount) - ) { + if ( + filters.capabilities.discounted && + !model.providerDetails.some((p) => { + const d = p.provider.discount + ? Number.parseFloat(p.provider.discount) + : 0; + return Number.isFinite(d) && d > 0; + }) + ) { return false; }
367-477: Sorting/filtering with string prices: add NaN guards to prevent unstable ordering.
new Date(invalid).getTime()is NaN (and NaN breaks comparisons), andparseFloatcan also yield NaN.
1275-1304: Request price discount path needs numeric validation/clamping.
provider.discountis a string;parseFloatmay be NaN or > 1.apps/playground/src/components/model-selector.tsx (2)
504-509: Removeas anycast.The
as anycast on line 508 violates the coding guideline for TypeScript files. The Set can be properly typed to avoid the cast.🔧 Proposed fix
const availableProviders = React.useMemo(() => { - const ids = new Set( + const ids = new Set<string>( allEntries.filter((e) => e.mapping).map((e) => e.mapping!.providerId), ); - return providers.filter((p) => ids.has(p.id as any)); + return providers.filter((p) => ids.has(p.id)); }, [allEntries, providers]);Based on coding guidelines, /*.{ts,tsx}: Never use
anyoras anyunless absolutely necessary in TypeScript code.
698-713: Unsafe array access could cause runtime error.Line 711 accesses
selectedModel.mappings[0]without checking if the mappings array is non-empty. If a model has zero mappings, this will throw "Cannot read property 'providerId' of undefined".🐛 Proposed fix
} return getProviderLogo( (selectedProviderId || - selectedModel.mappings[0].providerId) as ProviderId, + selectedModel.mappings[0]?.providerId) as ProviderId, );Or better yet, add a proper fallback:
} + const fallbackProviderId = selectedModel.mappings.length > 0 + ? selectedModel.mappings[0].providerId + : 'openai'; // or another sensible default return getProviderLogo( - (selectedProviderId || - selectedModel.mappings[0].providerId) as ProviderId, + (selectedProviderId || fallbackProviderId) as ProviderId, );
🤖 Fix all issues with AI agents
In @apps/api/src/index.ts:
- Around line 23-26: The /internal routes registered via app.route("/internal",
internalModels) are currently unprotected and therefore public; confirm intent
and make it explicit by either adding a clear inline comment above the
registration stating these endpoints intentionally expose non-sensitive catalog
metadata for frontend/public consumption, or move/wrap the registration so it is
behind the existing authHandler (i.e., register/internalModels after the
auth-protected routes or attach authHandler to the route) to require
authentication; apply the same change to the second /internal registration and
update any related tests or docs to reflect the decision.
In @apps/api/src/routes/internal-models.ts:
- Around line 93-117: The response currently includes both modelProviderMappings
and mappings because transformedModels spreads the entire model and then adds
mappings; update the mapping in internalModels.openapi (getModelsRoute) to
exclude modelProviderMappings—e.g., when mapping models, destructure {
modelProviderMappings, ...rest } = model and return { ...rest, mappings:
modelProviderMappings } or explicitly delete modelProviderMappings before
returning—so the JSON response only contains mappings and not the internal
modelProviderMappings field.
- Around line 70-152: The internal endpoints getModelsRoute and
getProvidersRoute are exposed publicly via internalModels.openapi(...) with no
auth; protect them by applying the same session/auth middleware used by the main
API routes (the session validation wrapper in routes/index.ts) or by checking
the session inside the handler before querying db.query.model.findMany /
db.query.provider.findMany; update internalModels.openapi calls to require the
session/authorization (or early-return 401) for both getModelsRoute and
getProvidersRoute so only authenticated/authorized internal clients can access
mappings/pricing data.
- Around line 11-22: The schema uses z.coerce.date() for response date fields
but responses are serialized as ISO strings; update the response schemas to use
z.string().datetime() instead of z.coerce.date() for the indicated fields (e.g.,
change providerSchema.createdAt and the other response fields releasedAt,
deprecatedAt, deactivatedAt in the related schema objects) so the Zod/OpenAPI
types match the actual JSON wire format expected by the frontend; replace each
z.coerce.date() occurrence with z.string().datetime().
In @apps/ui/src/hooks/useModels.ts:
- Around line 7-39: The queries in useModels and useProviders must include
config.apiUrl in their queryKey and wire React Query's AbortSignal into fetch:
update both queryKey arrays to ["internal-models", config.apiUrl] and
["internal-providers", config.apiUrl], change each queryFn signature to accept
the provided context (e.g., async ({ signal }) => ...) and pass that signal into
fetch (fetch(`${config.apiUrl}/internal/models`, { signal }) and
fetch(`${config.apiUrl}/internal/providers`, { signal })) so requests are
cancellable and the cache keys reflect the apiUrl dependency.
In @apps/ui/src/lib/fetch-models.ts:
- Around line 1-96: This file exports server-only helpers (fetchModels,
fetchProviders) that call getConfig and must not be bundled to the client; add a
top-level import "server-only" as the first import in the module to enforce a
hard failure if accidentally imported into client code, keeping the existing
imports (getConfig) and exported functions unchanged.
🧹 Nitpick comments (9)
apps/worker/src/services/sync-models.ts (1)
88-249: Consider an upsert formodelProviderMappingto avoid per-row SELECT + UPDATE/INSERT branching.Current approach does a SELECT per mapping, then update/insert. Drizzle supports
insert(...).onConflictDoUpdate(...)(as already used for providers/models) and would simplify + reduce round-trips.apps/playground/src/lib/fetch-models.ts (1)
1-95: Mark this module as server-only to prevent accidental client imports.Proposed fix
+import "server-only"; import { cache } from "react";apps/playground/src/lib/mapmodels.ts (2)
34-42: Consider removing or condensing the outdated implementation comment.The comment block spans 9 lines and discusses uncertainty about ID format choices that appear to have already been resolved (the code uses
${p.providerId}/${m.id}on line 45). This extended commentary adds noise and may confuse future maintainers.♻️ Suggested simplification
for (const p of m.mappings) { const providerInfo = providers.find((pr) => pr.id === p.providerId); - // Ensure we use the same ID format as ModelSelector: providerId/modelId - // Note: ModelSelector uses m.id (Gateway ID), not p.modelName (Provider ID) - // We should match that to ensure lookups work if we looked up by provider-specific ID. - // However, ChatPageClient uses mapModels primarily for capabilities lookup. - // If ModelSelector uses providerId/m.id, we should probably align here or support both? - // The existing code used providerId/p.modelName. - // Let's keep p.modelName for now to avoid breaking if p.modelName is expected elsewhere, - // but ideally it should be m.id. - // Let's assume ModelSelector logic is the correct one for "User Selection". entries.push({ id: `${p.providerId}/${m.id}`, // Changed to match ModelSelector
50-54: Optional: Simplify redundant nullish coalescing.The
?? undefinedpattern on lines 50, 53, and 54 is redundant since TypeScript will already treatnullorundefinedasundefined. While explicit, it doesn't add type safety.♻️ Optional simplification
- context: p.contextSize ?? undefined, + context: p.contextSize ?? undefined, // or just: context: p.contextSize, inputPrice: p.inputPrice ? parseFloat(p.inputPrice) : undefined, outputPrice: p.outputPrice ? parseFloat(p.outputPrice) : undefined, - vision: p.vision ?? undefined, - tools: p.tools ?? undefined, + vision: p.vision, + tools: p.tools,Note: If the target type explicitly requires
undefinedinstead ofnull, then the current code is correct.apps/playground/src/components/playground/chat-page-client.tsx (1)
275-313: Capability checks: prefer explicit=== truewith nullable booleans.
Today it works, but this reads cleaner and avoids accidental truthiness issues if the API ever changes types.Example tweak
- return def.mappings.some((p: ApiModelProviderMapping) => p.reasoning); + return def.mappings.some( + (p: ApiModelProviderMapping) => p.reasoning === true, + );Also applies to: 1078-1116
apps/ui/src/components/providers/provider-models-grid.tsx (1)
86-94:shouldShowStabilityWarningshould just returnboolean(noundefined).
Right now it never returnsundefined, so the signature is misleading.Proposed fix
- const shouldShowStabilityWarning = ( - stability?: StabilityLevel | null, - ): boolean | undefined => { - return ( - stability !== null && - stability !== undefined && - ["unstable", "experimental"].includes(stability) - ); - }; + const shouldShowStabilityWarning = ( + stability?: StabilityLevel | null, + ): boolean => stability === "unstable" || stability === "experimental";apps/ui/src/components/models/model-detail-card.tsx (1)
138-142: Avoid doubleparseFloatand clamp discount for the label.
This can display weird values if the API ever sends"1.5"or"NaN".apps/ui/src/components/models/model-card.tsx (1)
355-369:requestPriceparsing: avoid repeatedparseFloatand handle NaN.
Bad/missing API data can render$NaNand the repeated parsing is noisy.Proposed fix
- {provider.requestPrice !== null && - provider.requestPrice !== undefined && - parseFloat(provider.requestPrice) > 0 && ( + {(() => { + const req = provider.requestPrice + ? Number.parseFloat(provider.requestPrice) + : NaN; + if (!Number.isFinite(req) || req <= 0) { + return null; + } + return ( <div className="space-y-1"> <div className="text-xs text-muted-foreground"> Per Request </div> <div className="font-semibold text-foreground text-sm"> - ${parseFloat(provider.requestPrice).toFixed(3)} + ${req.toFixed(3)} <span className="text-muted-foreground text-xs ml-1"> /req </span> </div> </div> - )} + ); + })()}apps/playground/src/components/model-selector.tsx (1)
438-455: Simplify sorting logic for better readability.The nested ternaries make this sorting logic hard to read and maintain. Since
ApiModelalways hascreatedAt: string, the'createdAt' in achecks are unnecessary. Consider extracting a helper function:♻️ Proposed refactor
- // Sort models by createdAt (when added to LLM Gateway), newest first - // Falls back to releasedAt if createdAt is not available - // Note: createdAt comes from API response, releasedAt is in the models package + // Sort models by createdAt (newest first), with fallback to releasedAt + const getModelSortDate = (model: ApiModel): number => { + if (model.createdAt) { + return new Date(model.createdAt).getTime(); + } + if (model.releasedAt) { + return new Date(model.releasedAt).getTime(); + } + return 0; + }; + const sortedModels = [...models].sort((a, b) => { - const dateA = - "createdAt" in a && a.createdAt - ? new Date(a.createdAt as string | Date).getTime() - : a.releasedAt - ? new Date(a.releasedAt).getTime() - : 0; - const dateB = - "createdAt" in b && b.createdAt - ? new Date(b.createdAt as string | Date).getTime() - : b.releasedAt - ? new Date(b.releasedAt).getTime() - : 0; - return dateB - dateA; + return getModelSortDate(b) - getModelSortDate(a); });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (41)
apps/api/src/index.tsapps/api/src/routes/internal-models.tsapps/playground/src/app/group/page.tsxapps/playground/src/app/page.tsxapps/playground/src/components/model-selector.tsxapps/playground/src/components/playground/chat-header.tsxapps/playground/src/components/playground/chat-page-client.tsxapps/playground/src/components/playground/group-chat-client.tsxapps/playground/src/lib/fetch-models.tsapps/playground/src/lib/mapmodels.tsapps/playground/src/lib/model-utils.tsapps/ui/src/app/providers/[id]/page.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/models-supported.tsxapps/ui/src/components/models/all-models.tsxapps/ui/src/components/models/model-card.tsxapps/ui/src/components/models/model-detail-card.tsxapps/ui/src/components/providers/provider-models-grid.tsxapps/ui/src/components/shared/model-search-server.tsxapps/ui/src/components/shared/model-search.tsxapps/ui/src/hooks/useModels.tsapps/ui/src/lib/fetch-models.tsapps/worker/src/services/sync-models.tspackages/db/src/schema.tspackages/models/src/models.tspackages/models/src/models/alibaba.tspackages/models/src/models/anthropic.tspackages/models/src/models/deepseek.tspackages/models/src/models/google.tspackages/models/src/models/llmgateway.tspackages/models/src/models/meta.tspackages/models/src/models/minimax.tspackages/models/src/models/mistral.tspackages/models/src/models/moonshot.tspackages/models/src/models/nousresearch.tspackages/models/src/models/openai.tspackages/models/src/models/perplexity.tspackages/models/src/models/routeway.tspackages/models/src/models/xai.tspackages/models/src/models/zai.tspackages/shared/src/components/model-selector.tsx
💤 Files with no reviewable changes (16)
- packages/models/src/models.ts
- packages/models/src/models/google.ts
- packages/models/src/models/meta.ts
- packages/models/src/models/zai.ts
- packages/models/src/models/xai.ts
- packages/models/src/models/anthropic.ts
- packages/models/src/models/mistral.ts
- packages/models/src/models/moonshot.ts
- packages/models/src/models/routeway.ts
- packages/models/src/models/openai.ts
- packages/models/src/models/llmgateway.ts
- packages/models/src/models/perplexity.ts
- packages/models/src/models/nousresearch.ts
- packages/models/src/models/minimax.ts
- packages/models/src/models/alibaba.ts
- packages/models/src/models/deepseek.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/ui/src/components/shared/model-search-server.tsxapps/playground/src/lib/mapmodels.tsapps/ui/src/hooks/useModels.tsapps/api/src/routes/internal-models.tspackages/db/src/schema.tsapps/worker/src/services/sync-models.tsapps/api/src/index.tsapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/providers/[id]/page.tsxapps/playground/src/components/playground/group-chat-client.tsxapps/playground/src/components/playground/chat-header.tsxapps/ui/src/lib/fetch-models.tspackages/shared/src/components/model-selector.tsxapps/ui/src/components/models-supported.tsxapps/playground/src/app/page.tsxapps/ui/src/components/models/model-card.tsxapps/playground/src/app/group/page.tsxapps/playground/src/lib/fetch-models.tsapps/playground/src/components/playground/chat-page-client.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/models/all-models.tsxapps/ui/src/components/providers/provider-models-grid.tsxapps/playground/src/components/model-selector.tsxapps/playground/src/lib/model-utils.tsapps/ui/src/components/models/model-detail-card.tsx
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
apps/ui/src/components/shared/model-search-server.tsxapps/playground/src/lib/mapmodels.tsapps/ui/src/hooks/useModels.tsapps/api/src/routes/internal-models.tspackages/db/src/schema.tsapps/worker/src/services/sync-models.tsapps/api/src/index.tsapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/providers/[id]/page.tsxapps/playground/src/components/playground/group-chat-client.tsxapps/playground/src/components/playground/chat-header.tsxapps/ui/src/lib/fetch-models.tspackages/shared/src/components/model-selector.tsxapps/ui/src/components/models-supported.tsxapps/playground/src/app/page.tsxapps/ui/src/components/models/model-card.tsxapps/playground/src/app/group/page.tsxapps/playground/src/lib/fetch-models.tsapps/playground/src/components/playground/chat-page-client.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/models/all-models.tsxapps/ui/src/components/providers/provider-models-grid.tsxapps/playground/src/components/model-selector.tsxapps/playground/src/lib/model-utils.tsapps/ui/src/components/models/model-detail-card.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
apps/ui/src/components/shared/model-search-server.tsxapps/playground/src/lib/mapmodels.tsapps/ui/src/hooks/useModels.tsapps/api/src/routes/internal-models.tspackages/db/src/schema.tsapps/worker/src/services/sync-models.tsapps/api/src/index.tsapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/providers/[id]/page.tsxapps/playground/src/components/playground/group-chat-client.tsxapps/playground/src/components/playground/chat-header.tsxapps/ui/src/lib/fetch-models.tspackages/shared/src/components/model-selector.tsxapps/ui/src/components/models-supported.tsxapps/playground/src/app/page.tsxapps/ui/src/components/models/model-card.tsxapps/playground/src/app/group/page.tsxapps/playground/src/lib/fetch-models.tsapps/playground/src/components/playground/chat-page-client.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/models/all-models.tsxapps/ui/src/components/providers/provider-models-grid.tsxapps/playground/src/components/model-selector.tsxapps/playground/src/lib/model-utils.tsapps/ui/src/components/models/model-detail-card.tsx
apps/{ui,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{ui,playground}/**/*.{ts,tsx}: Use cookies for user-settings which are not saved in the database to ensure SSR works
Use Next.js App Router with React Server Components for frontend developmentUse cookies for user-settings which are not saved in the database to ensure SSR works
Files:
apps/ui/src/components/shared/model-search-server.tsxapps/playground/src/lib/mapmodels.tsapps/ui/src/hooks/useModels.tsapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/providers/[id]/page.tsxapps/playground/src/components/playground/group-chat-client.tsxapps/playground/src/components/playground/chat-header.tsxapps/ui/src/lib/fetch-models.tsapps/ui/src/components/models-supported.tsxapps/playground/src/app/page.tsxapps/ui/src/components/models/model-card.tsxapps/playground/src/app/group/page.tsxapps/playground/src/lib/fetch-models.tsapps/playground/src/components/playground/chat-page-client.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/models/all-models.tsxapps/ui/src/components/providers/provider-models-grid.tsxapps/playground/src/components/model-selector.tsxapps/playground/src/lib/model-utils.tsapps/ui/src/components/models/model-detail-card.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
apps/ui/src/components/shared/model-search-server.tsxapps/playground/src/lib/mapmodels.tsapps/ui/src/hooks/useModels.tsapps/api/src/routes/internal-models.tspackages/db/src/schema.tsapps/worker/src/services/sync-models.tsapps/api/src/index.tsapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/providers/[id]/page.tsxapps/playground/src/components/playground/group-chat-client.tsxapps/playground/src/components/playground/chat-header.tsxapps/ui/src/lib/fetch-models.tspackages/shared/src/components/model-selector.tsxapps/ui/src/components/models-supported.tsxapps/playground/src/app/page.tsxapps/ui/src/components/models/model-card.tsxapps/playground/src/app/group/page.tsxapps/playground/src/lib/fetch-models.tsapps/playground/src/components/playground/chat-page-client.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/models/all-models.tsxapps/ui/src/components/providers/provider-models-grid.tsxapps/playground/src/components/model-selector.tsxapps/playground/src/lib/model-utils.tsapps/ui/src/components/models/model-detail-card.tsx
apps/{ui,playground,docs}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
next/linkfor links andnext/navigation's router for programmatic navigation
Files:
apps/ui/src/components/shared/model-search-server.tsxapps/playground/src/lib/mapmodels.tsapps/ui/src/hooks/useModels.tsapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/providers/[id]/page.tsxapps/playground/src/components/playground/group-chat-client.tsxapps/playground/src/components/playground/chat-header.tsxapps/ui/src/lib/fetch-models.tsapps/ui/src/components/models-supported.tsxapps/playground/src/app/page.tsxapps/ui/src/components/models/model-card.tsxapps/playground/src/app/group/page.tsxapps/playground/src/lib/fetch-models.tsapps/playground/src/components/playground/chat-page-client.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/models/all-models.tsxapps/ui/src/components/providers/provider-models-grid.tsxapps/playground/src/components/model-selector.tsxapps/playground/src/lib/model-utils.tsapps/ui/src/components/models/model-detail-card.tsx
apps/{gateway,api}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Hono framework with Zod validation and OpenAPI documentation for backend APIs
Files:
apps/api/src/routes/internal-models.tsapps/api/src/index.ts
{apps/api,apps/gateway,packages/db}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with latest object syntax for database operations
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/api/src/routes/internal-models.tspackages/db/src/schema.tsapps/api/src/index.ts
apps/{gateway,api}/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/{gateway,api}/src/**/*.ts: Use Hono for backend framework in Gateway and API services
Use Zod schemas for validation in Hono services
Files:
apps/api/src/routes/internal-models.tsapps/api/src/index.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Drizzle ORM with latest object syntax for database operations
Files:
packages/db/src/schema.ts
🧠 Learnings (7)
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{gateway,api}/**/*.{ts,tsx} : Use Hono framework with Zod validation and OpenAPI documentation for backend APIs
Applied to files:
apps/api/src/routes/internal-models.ts
📚 Learning: 2025-12-03T12:42:26.162Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T12:42:26.162Z
Learning: Applies to apps/{gateway,api}/src/**/*.ts : Use Zod schemas for validation in Hono services
Applied to files:
apps/api/src/routes/internal-models.ts
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Always sync schema with `pnpm run setup` after table/column changes
Applied to files:
packages/db/src/schema.ts
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to packages/db/**/*.{ts,tsx} : Use Drizzle ORM with latest object syntax for database operations
Applied to files:
packages/db/src/schema.ts
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{tsx} : Use `next/link` for links and `next/navigation`'s router for programmatic navigation
Applied to files:
apps/ui/src/components/shared/model-search.tsx
📚 Learning: 2025-12-03T12:42:26.162Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T12:42:26.162Z
Learning: Applies to apps/{ui,playground,docs}/**/*.{ts,tsx} : Use `next/link` for links and `next/navigation`'s router for programmatic navigation
Applied to files:
apps/ui/src/components/shared/model-search.tsx
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{ts,tsx} : Use Next.js App Router with React Server Components for frontend development
Applied to files:
apps/ui/src/components/shared/model-search.tsxapps/playground/src/components/playground/group-chat-client.tsx
🧬 Code graph analysis (19)
apps/ui/src/hooks/useModels.ts (1)
apps/ui/src/lib/fetch-models.ts (2)
ApiModel(47-60)ApiProvider(5-16)
apps/api/src/routes/internal-models.ts (4)
packages/models/src/models.ts (1)
models(238-255)packages/db/src/db.ts (1)
db(21-25)packages/db/src/schema.ts (1)
model(666-701)packages/models/src/providers.ts (1)
providers(34-444)
apps/worker/src/services/sync-models.ts (1)
packages/db/src/schema.ts (1)
model(666-701)
apps/api/src/index.ts (2)
apps/gateway/src/app.ts (1)
app(56-56)apps/api/src/routes/internal-models.ts (1)
internalModels(8-8)
apps/ui/src/app/providers/[id]/page.tsx (3)
apps/playground/src/lib/fetch-models.ts (3)
ApiModel(45-58)ApiModelProviderMapping(16-43)ApiProvider(3-14)apps/ui/src/lib/fetch-models.ts (3)
ApiModel(47-60)ApiModelProviderMapping(18-45)ApiProvider(5-16)packages/models/src/models.ts (2)
ModelDefinition(180-236)ProviderModelMapping(50-176)
apps/playground/src/components/playground/chat-header.tsx (2)
apps/playground/src/lib/fetch-models.ts (2)
ApiModel(45-58)ApiProvider(3-14)apps/ui/src/lib/fetch-models.ts (2)
ApiModel(47-60)ApiProvider(5-16)
apps/ui/src/lib/fetch-models.ts (2)
apps/playground/src/lib/fetch-models.ts (5)
ApiProvider(3-14)ApiModelProviderMapping(16-43)ApiModel(45-58)fetchModels(63-78)fetchProviders(80-95)apps/api/src/index.ts (1)
config(29-40)
packages/shared/src/components/model-selector.tsx (1)
packages/models/src/models.ts (1)
models(238-255)
apps/ui/src/components/models-supported.tsx (5)
apps/playground/src/lib/fetch-models.ts (3)
ApiModel(45-58)ApiModelProviderMapping(16-43)ApiProvider(3-14)apps/ui/src/lib/fetch-models.ts (3)
ApiModel(47-60)ApiModelProviderMapping(18-45)ApiProvider(5-16)packages/models/src/models.ts (3)
ModelDefinition(180-236)ProviderModelMapping(50-176)models(238-255)apps/ui/src/lib/model-utils.ts (1)
formatPrice(3-14)packages/models/src/providers.ts (1)
ProviderId(446-446)
apps/playground/src/app/page.tsx (4)
packages/models/src/models.ts (1)
models(238-255)packages/models/src/providers.ts (1)
providers(34-444)apps/playground/src/lib/fetch-models.ts (2)
fetchModels(63-78)fetchProviders(80-95)apps/ui/src/lib/fetch-models.ts (2)
fetchModels(62-78)fetchProviders(80-96)
apps/ui/src/components/models/model-card.tsx (3)
apps/playground/src/lib/fetch-models.ts (3)
ApiModel(45-58)ApiModelProviderMapping(16-43)ApiProvider(3-14)apps/ui/src/lib/fetch-models.ts (3)
ApiModel(47-60)ApiModelProviderMapping(18-45)ApiProvider(5-16)packages/models/src/models.ts (1)
StabilityLevel(178-178)
apps/playground/src/app/group/page.tsx (1)
apps/playground/src/lib/fetch-models.ts (2)
fetchModels(63-78)fetchProviders(80-95)
apps/playground/src/lib/fetch-models.ts (1)
apps/ui/src/lib/fetch-models.ts (5)
ApiProvider(5-16)ApiModelProviderMapping(18-45)ApiModel(47-60)fetchModels(62-78)fetchProviders(80-96)
apps/playground/src/components/playground/chat-page-client.tsx (1)
apps/playground/src/lib/fetch-models.ts (3)
ApiModel(45-58)ApiProvider(3-14)ApiModelProviderMapping(16-43)
apps/ui/src/app/timeline/page.tsx (3)
packages/models/src/models.ts (1)
models(238-255)apps/ui/src/hooks/useModels.ts (1)
useModels(7-22)packages/db/src/schema.ts (1)
model(666-701)
apps/ui/src/components/models/all-models.tsx (2)
apps/ui/src/lib/fetch-models.ts (3)
ApiModel(47-60)ApiModelProviderMapping(18-45)ApiProvider(5-16)apps/ui/src/hooks/useModels.ts (1)
useModelsAndProviders(41-51)
apps/ui/src/components/providers/provider-models-grid.tsx (3)
apps/playground/src/lib/fetch-models.ts (3)
ApiModel(45-58)ApiModelProviderMapping(16-43)ApiProvider(3-14)apps/ui/src/lib/fetch-models.ts (3)
ApiModel(47-60)ApiModelProviderMapping(18-45)ApiProvider(5-16)packages/models/src/models.ts (1)
StabilityLevel(178-178)
apps/playground/src/components/model-selector.tsx (3)
apps/playground/src/lib/fetch-models.ts (3)
ApiModel(45-58)ApiProvider(3-14)ApiModelProviderMapping(16-43)apps/ui/src/lib/fetch-models.ts (3)
ApiModel(47-60)ApiProvider(5-16)ApiModelProviderMapping(18-45)apps/playground/src/lib/model-utils.ts (1)
formatPrice(3-19)
apps/ui/src/components/models/model-detail-card.tsx (3)
apps/playground/src/lib/fetch-models.ts (3)
ApiModel(45-58)ApiModelProviderMapping(16-43)ApiProvider(3-14)apps/ui/src/lib/fetch-models.ts (3)
ApiModel(47-60)ApiModelProviderMapping(18-45)ApiProvider(5-16)packages/models/src/models.ts (1)
StabilityLevel(178-178)
⏰ 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). (12)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: e2e-shards (2)
- GitHub Check: e2e-shards (3)
- GitHub Check: e2e-shards (1)
- GitHub Check: e2e-shards (4)
- GitHub Check: e2e-shards (5)
- GitHub Check: lint / run
- GitHub Check: build / run
- GitHub Check: test / run
- GitHub Check: autofix
- GitHub Check: generate / run
🔇 Additional comments (14)
apps/ui/src/components/shared/model-search-server.tsx (1)
1-12: LGTM — clean server-side orchestration with parallel fetch.apps/playground/src/app/page.tsx (1)
6-7: Ensure UI behavior is sane when models/providers fetch returns[].Since
fetchModels()/fetchProviders()appear to “log + return []” on failure,ChatPageClientcan end up with no options; please confirm empty-state handling (and consider surfacing a user-visible error if this blocks core flows).Also applies to: 130-134
apps/playground/src/components/playground/chat-header.tsx (1)
7-12: Type migration toApiModel[]/ApiProvider[]looks consistent.packages/db/src/schema.ts (1)
675-685: Schema change follow-through: migrations/backfill + schema sync.Please ensure migrations (and any needed backfill/defaulting for new nullable columns) are in place, and run the repo’s schema sync step (
pnpm run setup) after the table/column changes. Based on learnings, always sync schema withpnpm run setupafter table/column changes.Also applies to: 731-737
apps/ui/src/app/providers/[id]/page.tsx (1)
42-105: Verify thecreatedAttimestamp strategy andmappingspopulation with team.The review comment raises concerns about using
new Date().toISOString()forcreatedAt(which may break createdAt-based sorting) and leavingmappingsempty during the broader ApiModel migration. However, I cannot access the repository to confirm the current code state, type definitions, howcreatedAtis actually used for sorting, or whethermappingsshould be populated in this context.Please verify:
- Whether
createdAtshould use a stable timestamp (e.g.,releasedAt) instead ofnew Date()- Whether
mappingsshould be populated with provider mapping data here or handled elsewhere in the migration- Any downstream components that depend on these fields being populated
apps/playground/src/lib/model-utils.ts (1)
34-40: LGTM: Provider lookup correctly migrated to mappings.The function correctly uses
model.mappings[0]to get the primary provider mapping and safely handles cases where mappings might be empty.apps/playground/src/components/playground/group-chat-client.tsx (1)
469-469: Verify color fallback behavior aligns with provider icon styling.The nullish coalescing to
undefinedensures the inline style gracefully handles missing color values. This is correct for React's style prop.apps/ui/src/app/timeline/page.tsx (2)
79-85: LGTM: Clamping logic prevents timeline anomalies.The logic correctly clamps
createdAtto the gateway launch date, ensuring models are never shown as "added to LLM Gateway" before the gateway existed. This handles historical models gracefully.
127-137: Good: Loading state properly handled with early return.The loading UI prevents rendering stale or incomplete data while models are being fetched.
apps/ui/src/components/shared/model-search.tsx (2)
93-98: LGTM: Graceful fallback from createdAt to releasedAt.The date handling correctly prioritizes
createdAt(when added to LLM Gateway) and falls back toreleasedAt(provider release date) whencreatedAtis unavailable. This ensures timeline consistency across the UI.
118-121: No changes needed. Themapping.inputPricefield is explicitly typed asstring | null(not includingundefined), so the null check correctly narrows the type tostringbefore callingparseFloat(). The code is type-safe.apps/ui/src/components/models/model-card.tsx (1)
81-89: Stability warning helper looks good after the nullable handling update.
Clearer + consistent with nullable API fields.apps/playground/src/components/model-selector.tsx (2)
168-226: Price parsing and discount logic looks correct.The function properly handles null/undefined price values before parsing, and safely applies discounts. The distinction between per-token and per-request pricing is handled correctly.
240-371: Aggregate info computation is correct.The
getRootAggregateInfofunction properly iterates over mappings, applies discounts, and aggregates min/max values. TheapplyDiscounthelper safely handles string price parsing with proper null checks.
| import { internalModels } from "./routes/internal-models.js"; | ||
| import { referral } from "./routes/referral.js"; | ||
| import { stripeRoutes } from "./stripe.js"; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's get an overview of the API index file structure
wc -l apps/api/src/index.tsRepository: theopenco/llmgateway
Length of output: 89
🏁 Script executed:
# Read the relevant sections of the index file, including lines around 23-26 and 185-186
head -50 apps/api/src/index.ts | cat -nRepository: theopenco/llmgateway
Length of output: 1879
🏁 Script executed:
# Check the internal-models route file to see if it has auth protection
cat -n apps/api/src/routes/internal-models.tsRepository: theopenco/llmgateway
Length of output: 5162
🏁 Script executed:
# Read the rest of index.ts to see the full app routing configuration
tail -150 apps/api/src/index.ts | cat -nRepository: theopenco/llmgateway
Length of output: 4330
🏁 Script executed:
# Also search for all app.route and app.use calls to understand the complete routing order
grep -n "app\.\(route\|use\)" apps/api/src/index.ts | head -30Repository: theopenco/llmgateway
Length of output: 395
🏁 Script executed:
# Check the authHandler implementation to see if it applies middleware or routes
cat -n apps/api/src/auth/handler.tsRepository: theopenco/llmgateway
Length of output: 879
🏁 Script executed:
# Check if there are any authentication checks or middleware in the routes/index.ts
# to see if other protected routes have explicit auth checks
head -100 apps/api/src/routes/index.ts | cat -nRepository: theopenco/llmgateway
Length of output: 2021
🏁 Script executed:
# Search for any middleware or auth checks in the codebase that might protect routes
rg "c\.get\(.*user" apps/api/src/routes/ -A 2Repository: theopenco/llmgateway
Length of output: 9808
Confirm whether /internal/* endpoints should be publicly accessible—they currently bypass all authentication.
Line 142 registers app.route("/internal", internalModels) before the auth-protected routes (line 148+). The internalModels route has no authentication middleware. This means /internal/models and /internal/providers are publicly accessible.
The returned data (active models and providers only) appears to be non-sensitive catalog metadata, which may be intentional for UI/public consumption. However, the naming and routing order should be explicit. Either:
- Add a comment clarifying this is intentional public data (e.g., for frontend consumption)
- Move the route behind
authHandlerif it should require authentication - Or document this design decision
Same issue applies at line 185-186 (the second /internal route registration mentioned).
🤖 Prompt for AI Agents
In @apps/api/src/index.ts around lines 23 - 26, The /internal routes registered
via app.route("/internal", internalModels) are currently unprotected and
therefore public; confirm intent and make it explicit by either adding a clear
inline comment above the registration stating these endpoints intentionally
expose non-sensitive catalog metadata for frontend/public consumption, or
move/wrap the registration so it is behind the existing authHandler (i.e.,
register/internalModels after the auth-protected routes or attach authHandler to
the route) to require authentication; apply the same change to the second
/internal registration and update any related tests or docs to reflect the
decision.
| const providerSchema = z.object({ | ||
| id: z.string(), | ||
| createdAt: z.coerce.date(), | ||
| name: z.string().nullable(), | ||
| description: z.string().nullable(), | ||
| streaming: z.boolean().nullable(), | ||
| cancellation: z.boolean().nullable(), | ||
| color: z.string().nullable(), | ||
| website: z.string().nullable(), | ||
| announcement: z.string().nullable(), | ||
| status: z.enum(["active", "inactive"]), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file content at the specified lines
cd apps/api/src/routes && wc -l internal-models.tsRepository: theopenco/llmgateway
Length of output: 86
🏁 Script executed:
# Read the full file to see schema definitions and usage
cat -n apps/api/src/routes/internal-models.tsRepository: theopenco/llmgateway
Length of output: 5162
🏁 Script executed:
# Check for frontend type definitions
fd -e ts -e tsx | xargs grep -l "ApiModel\|ApiProvider" | head -20Repository: theopenco/llmgateway
Length of output: 808
🏁 Script executed:
# Check package.json for @hono/zod-openapi version
cat package.json | grep -A 2 "@hono/zod-openapi"Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Find package.json files
find . -name package.json -type f | head -5Repository: theopenco/llmgateway
Length of output: 197
🏁 Script executed:
# Read the API package.json
cat apps/api/package.json | grep -A 5 -B 5 "@hono"Repository: theopenco/llmgateway
Length of output: 573
🏁 Script executed:
# Check frontend type definitions for ApiModel and ApiProvider
rg "ApiModel|ApiProvider" --type ts --type tsx -A 5 | head -60Repository: theopenco/llmgateway
Length of output: 91
🏁 Script executed:
# Look for type definitions file
fd -e ts -e tsx "type" | xargs grep -l "ApiModel\|ApiProvider" 2>/dev/null | head -10Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Search for ApiModel/ApiProvider without type restrictions
rg "ApiModel|ApiProvider" -A 3 | head -80Repository: theopenco/llmgateway
Length of output: 4517
🏁 Script executed:
# Check the models package (mentioned in dependencies)
find . -path "*/packages/models*" -o -path "*/models/*" | head -20Repository: theopenco/llmgateway
Length of output: 907
🏁 Script executed:
# Look for generated types or interface definitions
fd "types|model" -e ts | xargs grep -l "createdAt" 2>/dev/null | head -10Repository: theopenco/llmgateway
Length of output: 273
🏁 Script executed:
# Check if there's a shared types file
find . -name "*.ts" -path "*/types/*" -o -name "types.ts" | head -20Repository: theopenco/llmgateway
Length of output: 516
🏁 Script executed:
# Verify the handler behavior - check if dates are returned as raw Date objects
cat -n apps/api/src/routes/internal-models.ts | sed -n '93,117p'Repository: theopenco/llmgateway
Length of output: 776
🏁 Script executed:
# Check how Drizzle ORM returns dates from the database
rg "createdAt|releasedAt|deprecatedAt|deactivatedAt" apps/api/src --type ts -A 2 -B 2 | head -40Repository: theopenco/llmgateway
Length of output: 2442
Use z.string().datetime() instead of z.coerce.date() for response schemas to match actual JSON serialization behavior.
Response schemas use z.coerce.date() for createdAt, releasedAt, deprecatedAt, and deactivatedAt fields, but c.json() serializes JavaScript Date objects to ISO strings. Frontend types already expect these as string, and the OpenAPI schema should match the actual wire format. Replace these with z.string().datetime() at lines 13, 27, 49–50, 57–58.
🤖 Prompt for AI Agents
In @apps/api/src/routes/internal-models.ts around lines 11 - 22, The schema uses
z.coerce.date() for response date fields but responses are serialized as ISO
strings; update the response schemas to use z.string().datetime() instead of
z.coerce.date() for the indicated fields (e.g., change providerSchema.createdAt
and the other response fields releasedAt, deprecatedAt, deactivatedAt in the
related schema objects) so the Zod/OpenAPI types match the actual JSON wire
format expected by the frontend; replace each z.coerce.date() occurrence with
z.string().datetime().
| // GET /internal/models - Returns models with mappings sorted by createdAt desc | ||
| const getModelsRoute = createRoute({ | ||
| operationId: "internal_get_models", | ||
| summary: "Get all models", | ||
| description: | ||
| "Returns all models with their provider mappings, sorted by createdAt descending", | ||
| method: "get", | ||
| path: "/models", | ||
| request: {}, | ||
| responses: { | ||
| 200: { | ||
| content: { | ||
| "application/json": { | ||
| schema: z.object({ | ||
| models: z.array(modelSchema), | ||
| }), | ||
| }, | ||
| }, | ||
| description: "List of all models with their provider mappings", | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| internalModels.openapi(getModelsRoute, async (c) => { | ||
| const models = await db.query.model.findMany({ | ||
| where: { | ||
| status: { eq: "active" }, | ||
| }, | ||
| with: { | ||
| modelProviderMappings: { | ||
| where: { | ||
| status: { eq: "active" }, | ||
| }, | ||
| }, | ||
| }, | ||
| orderBy: { | ||
| createdAt: "desc", | ||
| }, | ||
| }); | ||
|
|
||
| // Transform to match expected schema (rename modelProviderMappings to mappings) | ||
| const transformedModels = models.map((model) => ({ | ||
| ...model, | ||
| mappings: model.modelProviderMappings, | ||
| })); | ||
|
|
||
| return c.json({ models: transformedModels }); | ||
| }); | ||
|
|
||
| // GET /internal/providers - Returns providers sorted by createdAt desc | ||
| const getProvidersRoute = createRoute({ | ||
| operationId: "internal_get_providers", | ||
| summary: "Get all providers", | ||
| description: "Returns all providers, sorted by createdAt descending", | ||
| method: "get", | ||
| path: "/providers", | ||
| request: {}, | ||
| responses: { | ||
| 200: { | ||
| content: { | ||
| "application/json": { | ||
| schema: z.object({ | ||
| providers: z.array(providerSchema), | ||
| }), | ||
| }, | ||
| }, | ||
| description: "List of all providers", | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| internalModels.openapi(getProvidersRoute, async (c) => { | ||
| const providers = await db.query.provider.findMany({ | ||
| where: { | ||
| status: { eq: "active" }, | ||
| }, | ||
| orderBy: { | ||
| createdAt: "desc", | ||
| }, | ||
| }); | ||
|
|
||
| return c.json({ providers }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where internalModels is registered/mounted
echo "=== Searching for internalModels router registration ==="
rg -n 'internalModels' apps/api/src --type ts --type tsx -B3 -A3
echo -e "\n=== Searching for router mount points with /internal path ==="
rg -n '"/internal"' apps/api/src --type ts --type tsx -B2 -A2
echo -e "\n=== Checking main app/router setup files ==="
fd -t f '(index|app|router|setup)' apps/api/src -e ts -e tsx | head -20Repository: theopenco/llmgateway
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Check for auth middleware on internal routes
echo "=== Searching for middleware patterns in internal-models file ==="
cat -n apps/api/src/routes/internal-models.ts | head -80
echo -e "\n=== Searching for auth-related imports in internal-models ==="
rg -n 'auth|middleware|protect|verify' apps/api/src/routes/internal-models.ts
echo -e "\n=== Checking app entry point and how routes are registered ==="
fd -t f 'index' apps/api/src -e ts | xargs cat | head -150Repository: theopenco/llmgateway
Length of output: 7017
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for internalModels route registration in main app ==="
rg -n 'internalModels' apps/api/src -A5 -B2
echo -e "\n=== Check routes/index.ts for route exports and registration ==="
cat apps/api/src/routes/index.ts 2>/dev/null || echo "File not found"
echo -e "\n=== Full app initialization to see how internalModels is registered ==="
tail -n 200 apps/api/src/index.tsRepository: theopenco/llmgateway
Length of output: 8652
Unprotected system inventory endpoints. The /internal/models and /internal/providers routes are currently publicly accessible with no authentication or authorization checks. While they are correctly mounted under the /internal path prefix, the naming implies internal-only access but they lack any protective middleware. Either add authentication middleware to these routes, restrict network access, or reconsider whether exposing all model and provider mappings (including pricing data) should be public. The main API routes in routes/index.ts include explicit session validation—the internal routes should have similar protection if they're meant to be internal-only.
🤖 Prompt for AI Agents
In @apps/api/src/routes/internal-models.ts around lines 70 - 152, The internal
endpoints getModelsRoute and getProvidersRoute are exposed publicly via
internalModels.openapi(...) with no auth; protect them by applying the same
session/auth middleware used by the main API routes (the session validation
wrapper in routes/index.ts) or by checking the session inside the handler before
querying db.query.model.findMany / db.query.provider.findMany; update
internalModels.openapi calls to require the session/authorization (or
early-return 401) for both getModelsRoute and getProvidersRoute so only
authenticated/authorized internal clients can access mappings/pricing data.
| internalModels.openapi(getModelsRoute, async (c) => { | ||
| const models = await db.query.model.findMany({ | ||
| where: { | ||
| status: { eq: "active" }, | ||
| }, | ||
| with: { | ||
| modelProviderMappings: { | ||
| where: { | ||
| status: { eq: "active" }, | ||
| }, | ||
| }, | ||
| }, | ||
| orderBy: { | ||
| createdAt: "desc", | ||
| }, | ||
| }); | ||
|
|
||
| // Transform to match expected schema (rename modelProviderMappings to mappings) | ||
| const transformedModels = models.map((model) => ({ | ||
| ...model, | ||
| mappings: model.modelProviderMappings, | ||
| })); | ||
|
|
||
| return c.json({ models: transformedModels }); | ||
| }); |
There was a problem hiding this comment.
Response includes both mappings and modelProviderMappings; strip the internal field.
Right now you spread model and add mappings, so the payload contains modelProviderMappings too (not in the documented schema). Prefer removing it explicitly.
Proposed fix
- // Transform to match expected schema (rename modelProviderMappings to mappings)
- const transformedModels = models.map((model) => ({
- ...model,
- mappings: model.modelProviderMappings,
- }));
+ // Transform to match expected schema (rename modelProviderMappings to mappings)
+ const transformedModels = models.map(({ modelProviderMappings, ...model }) => ({
+ ...model,
+ mappings: modelProviderMappings,
+ }));📝 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.
| internalModels.openapi(getModelsRoute, async (c) => { | |
| const models = await db.query.model.findMany({ | |
| where: { | |
| status: { eq: "active" }, | |
| }, | |
| with: { | |
| modelProviderMappings: { | |
| where: { | |
| status: { eq: "active" }, | |
| }, | |
| }, | |
| }, | |
| orderBy: { | |
| createdAt: "desc", | |
| }, | |
| }); | |
| // Transform to match expected schema (rename modelProviderMappings to mappings) | |
| const transformedModels = models.map((model) => ({ | |
| ...model, | |
| mappings: model.modelProviderMappings, | |
| })); | |
| return c.json({ models: transformedModels }); | |
| }); | |
| internalModels.openapi(getModelsRoute, async (c) => { | |
| const models = await db.query.model.findMany({ | |
| where: { | |
| status: { eq: "active" }, | |
| }, | |
| with: { | |
| modelProviderMappings: { | |
| where: { | |
| status: { eq: "active" }, | |
| }, | |
| }, | |
| }, | |
| orderBy: { | |
| createdAt: "desc", | |
| }, | |
| }); | |
| // Transform to match expected schema (rename modelProviderMappings to mappings) | |
| const transformedModels = models.map(({ modelProviderMappings, ...model }) => ({ | |
| ...model, | |
| mappings: modelProviderMappings, | |
| })); | |
| return c.json({ models: transformedModels }); | |
| }); |
🤖 Prompt for AI Agents
In @apps/api/src/routes/internal-models.ts around lines 93 - 117, The response
currently includes both modelProviderMappings and mappings because
transformedModels spreads the entire model and then adds mappings; update the
mapping in internalModels.openapi (getModelsRoute) to exclude
modelProviderMappings—e.g., when mapping models, destructure {
modelProviderMappings, ...rest } = model and return { ...rest, mappings:
modelProviderMappings } or explicitly delete modelProviderMappings before
returning—so the JSON response only contains mappings and not the internal
modelProviderMappings field.
| // Convert static ModelDefinition to ApiModel-like structure | ||
| const convertToApiModel = ( | ||
| def: ModelDefinition, | ||
| map: ProviderModelMapping, | ||
| ): ModelWithProviders => { | ||
| const provider = providerDefinitions.find((p) => p.id === map.providerId)!; | ||
| return { | ||
| id: def.id, | ||
| createdAt: new Date().toISOString(), | ||
| releasedAt: def.releasedAt?.toISOString() ?? null, | ||
| name: def.name ?? null, | ||
| aliases: def.aliases ?? null, | ||
| description: def.description ?? null, | ||
| family: def.family, | ||
| free: def.free ?? null, | ||
| output: def.output ?? null, | ||
| stability: def.stability ?? null, | ||
| status: "active", | ||
| mappings: [], | ||
| providerDetails: [ | ||
| { | ||
| provider: { | ||
| id: `${map.providerId}-${def.id}`, | ||
| createdAt: new Date().toISOString(), | ||
| modelId: def.id, | ||
| providerId: map.providerId, | ||
| modelName: map.modelName, | ||
| inputPrice: map.inputPrice?.toString() ?? null, | ||
| outputPrice: map.outputPrice?.toString() ?? null, | ||
| cachedInputPrice: map.cachedInputPrice?.toString() ?? null, | ||
| imageInputPrice: map.imageInputPrice?.toString() ?? null, | ||
| requestPrice: map.requestPrice?.toString() ?? null, | ||
| contextSize: map.contextSize ?? null, | ||
| maxOutput: map.maxOutput ?? null, | ||
| streaming: map.streaming ?? true, | ||
| vision: map.vision ?? null, | ||
| reasoning: map.reasoning ?? null, | ||
| reasoningOutput: map.reasoningOutput ?? null, | ||
| tools: map.tools ?? null, | ||
| jsonOutput: map.jsonOutput ?? null, | ||
| jsonOutputSchema: map.jsonOutputSchema ?? null, | ||
| webSearch: map.webSearch ?? null, | ||
| discount: map.discount?.toString() ?? null, | ||
| stability: map.stability ?? null, | ||
| supportedParameters: map.supportedParameters ?? null, | ||
| deprecatedAt: map.deprecatedAt?.toISOString() ?? null, | ||
| deactivatedAt: map.deactivatedAt?.toISOString() ?? null, | ||
| status: "active", | ||
| }, | ||
| providerInfo: { | ||
| id: provider.id, | ||
| createdAt: new Date().toISOString(), | ||
| name: provider.name ?? null, | ||
| description: provider.description ?? null, | ||
| streaming: provider.streaming ?? null, | ||
| cancellation: provider.cancellation ?? null, | ||
| color: provider.color ?? null, | ||
| website: provider.website ?? null, | ||
| announcement: provider.announcement ?? null, | ||
| status: "active", | ||
| }, | ||
| }, | ||
| ], | ||
| }; | ||
| }; |
There was a problem hiding this comment.
convertToApiModel: avoid ! and avoid “fake” createdAt if it’s user-visible / used for sorting.
A missing provider definition will hard-crash, and createdAt: now can mislead any “newest model” logic.
Proposed fix (safer provider lookup)
- const provider = providerDefinitions.find((p) => p.id === map.providerId)!;
+ const provider = providerDefinitions.find((p) => p.id === map.providerId);
+ if (!provider) {
+ throw new Error(`Unknown providerId in model mapping: ${map.providerId}`);
+ }| export function useModels() { | ||
| const config = useAppConfig(); | ||
|
|
||
| return useQuery<ApiModel[]>({ | ||
| queryKey: ["internal-models"], | ||
| queryFn: async () => { | ||
| const response = await fetch(`${config.apiUrl}/internal/models`); | ||
| if (!response.ok) { | ||
| throw new Error("Failed to fetch models"); | ||
| } | ||
| const data = await response.json(); | ||
| return data.models || []; | ||
| }, | ||
| staleTime: 60 * 1000, // 1 minute | ||
| }); | ||
| } | ||
|
|
||
| export function useProviders() { | ||
| const config = useAppConfig(); | ||
|
|
||
| return useQuery<ApiProvider[]>({ | ||
| queryKey: ["internal-providers"], | ||
| queryFn: async () => { | ||
| const response = await fetch(`${config.apiUrl}/internal/providers`); | ||
| if (!response.ok) { | ||
| throw new Error("Failed to fetch providers"); | ||
| } | ||
| const data = await response.json(); | ||
| return data.providers || []; | ||
| }, | ||
| staleTime: 60 * 1000, // 1 minute | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "useModels.ts" --exactRepository: theopenco/llmgateway
Length of output: 288
🏁 Script executed:
cat -n apps/ui/src/hooks/useModels.tsRepository: theopenco/llmgateway
Length of output: 1750
🏁 Script executed:
# Search for useAppConfig to understand if apiUrl is dynamic
rg "useAppConfig" apps/ui/src -A 5 -B 2 | head -100Repository: theopenco/llmgateway
Length of output: 6133
🏁 Script executed:
# Check for similar useQuery patterns in the codebase
rg "useQuery" apps/ui/src -B 2 -A 8 | head -150Repository: theopenco/llmgateway
Length of output: 8190
🏁 Script executed:
cat -n apps/ui/src/lib/config.tsx | head -80Repository: theopenco/llmgateway
Length of output: 954
🏁 Script executed:
# Check if apiUrl is expected to change at runtime
rg "AppConfigProvider\|AppConfigContext" apps/ui/src -B 3 -A 10 | head -100Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Look at how other parts of the app handle dynamic apiUrl
rg "config\.apiUrl" apps/ui/src -B 2 -A 2Repository: theopenco/llmgateway
Length of output: 1974
🏁 Script executed:
# Check config-server to understand AppConfig type
cat -n apps/ui/src/lib/config-server.ts 2>/dev/null || cat -n apps/ui/src/lib/config.ts | grep -A 20 "type AppConfig\|interface AppConfig"Repository: theopenco/llmgateway
Length of output: 1424
🏁 Script executed:
# Verify React Query's queryFn signature and AbortSignal support
web_search "react query queryFn AbortSignal signal parameter"Repository: theopenco/llmgateway
Length of output: 114
🌐 Web query:
react-query useQuery queryFn AbortSignal signal parameter best practice
💡 Result:
Short best-prices for using the AbortSignal passed into useQuery's queryFn:
- Consume the signal argument from queryFn and pass it into your request library (fetch or axios) so requests are cancelled automatically: useQuery provides ({ signal }) to queryFn. [1]
- With fetch: pass signal in RequestInit; with axios use AbortController/ signal (axios ≥ v0.22.0). [1][2]
- If you do CPU-heavy post-processing, check signal.aborted (or call AbortSignal.throwIfAborted()) before doing work to avoid wasted work after cancel. [1][3]
- Note: if you read/consume the signal, cancelling will abort the request and the query will revert to prior state (so avoid consuming the signal if you intentionally want the query to complete and remain cached). [1]
- Polyfill AbortController/AbortSignal in environments that lack it (Node versions < built-in support). [1][2]
- For manual cancellation you can call queryClient.cancelQueries(...) which triggers the signal. [1]
Sources: TanStack Query docs (Query Cancellation) [1], Axios cancellation docs (signal / AbortController) [2], MDN AbortSignal docs (throwIfAborted / aborted) [3].
🏁 Script executed:
# Check if there's a dependency array or other dependencies that should be included
cat -n apps/ui/src/hooks/useModels.tsRepository: theopenco/llmgateway
Length of output: 1750
🏁 Script executed:
# Search for other useQuery patterns in the codebase to see if signal is being used elsewhere
rg "queryFn.*signal\|signal\)" apps/ui/src --type ts --type tsx | head -20Repository: theopenco/llmgateway
Length of output: 91
🏁 Script executed:
# Search for other raw useQuery usages that might have the same pattern
rg "useQuery<" apps/ui/src --type ts -A 5 | grep -E "useQuery<|queryKey:|queryFn:" | head -40Repository: theopenco/llmgateway
Length of output: 433
🏁 Script executed:
# Verify if config dependencies are actually used in other hooks
cat -n apps/ui/src/lib/fetch-client.tsRepository: theopenco/llmgateway
Length of output: 906
Include apiUrl in queryKey and pass the AbortSignal to fetch().
The queryKey should include config.apiUrl since the codebase treats it as a potential dependency (see useFetchClient.ts line 18). If the config context is updated, the query cache would become stale. Additionally, the queryFn should accept and use the signal parameter that React Query provides to properly cancel requests when queries are invalidated or components unmount, avoiding wasted work.
Proposed fix
export function useModels() {
const config = useAppConfig();
return useQuery<ApiModel[]>({
- queryKey: ["internal-models"],
- queryFn: async () => {
- const response = await fetch(`${config.apiUrl}/internal/models`);
+ queryKey: ["internal-models", config.apiUrl],
+ queryFn: async ({ signal }) => {
+ const response = await fetch(`${config.apiUrl}/internal/models`, { signal });
if (!response.ok) {
throw new Error("Failed to fetch models");
}
const data = await response.json();
return data.models || [];
},
staleTime: 60 * 1000, // 1 minute
});
}
export function useProviders() {
const config = useAppConfig();
return useQuery<ApiProvider[]>({
- queryKey: ["internal-providers"],
- queryFn: async () => {
- const response = await fetch(`${config.apiUrl}/internal/providers`);
+ queryKey: ["internal-providers", config.apiUrl],
+ queryFn: async ({ signal }) => {
+ const response = await fetch(`${config.apiUrl}/internal/providers`, { signal });
if (!response.ok) {
throw new Error("Failed to fetch providers");
}
const data = await response.json();
return data.providers || [];
},
staleTime: 60 * 1000, // 1 minute
});
}📝 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.
| export function useModels() { | |
| const config = useAppConfig(); | |
| return useQuery<ApiModel[]>({ | |
| queryKey: ["internal-models"], | |
| queryFn: async () => { | |
| const response = await fetch(`${config.apiUrl}/internal/models`); | |
| if (!response.ok) { | |
| throw new Error("Failed to fetch models"); | |
| } | |
| const data = await response.json(); | |
| return data.models || []; | |
| }, | |
| staleTime: 60 * 1000, // 1 minute | |
| }); | |
| } | |
| export function useProviders() { | |
| const config = useAppConfig(); | |
| return useQuery<ApiProvider[]>({ | |
| queryKey: ["internal-providers"], | |
| queryFn: async () => { | |
| const response = await fetch(`${config.apiUrl}/internal/providers`); | |
| if (!response.ok) { | |
| throw new Error("Failed to fetch providers"); | |
| } | |
| const data = await response.json(); | |
| return data.providers || []; | |
| }, | |
| staleTime: 60 * 1000, // 1 minute | |
| }); | |
| } | |
| export function useModels() { | |
| const config = useAppConfig(); | |
| return useQuery<ApiModel[]>({ | |
| queryKey: ["internal-models", config.apiUrl], | |
| queryFn: async ({ signal }) => { | |
| const response = await fetch(`${config.apiUrl}/internal/models`, { signal }); | |
| if (!response.ok) { | |
| throw new Error("Failed to fetch models"); | |
| } | |
| const data = await response.json(); | |
| return data.models || []; | |
| }, | |
| staleTime: 60 * 1000, // 1 minute | |
| }); | |
| } | |
| export function useProviders() { | |
| const config = useAppConfig(); | |
| return useQuery<ApiProvider[]>({ | |
| queryKey: ["internal-providers", config.apiUrl], | |
| queryFn: async ({ signal }) => { | |
| const response = await fetch(`${config.apiUrl}/internal/providers`, { signal }); | |
| if (!response.ok) { | |
| throw new Error("Failed to fetch providers"); | |
| } | |
| const data = await response.json(); | |
| return data.providers || []; | |
| }, | |
| staleTime: 60 * 1000, // 1 minute | |
| }); | |
| } |
🤖 Prompt for AI Agents
In @apps/ui/src/hooks/useModels.ts around lines 7 - 39, The queries in useModels
and useProviders must include config.apiUrl in their queryKey and wire React
Query's AbortSignal into fetch: update both queryKey arrays to
["internal-models", config.apiUrl] and ["internal-providers", config.apiUrl],
change each queryFn signature to accept the provided context (e.g., async ({
signal }) => ...) and pass that signal into fetch
(fetch(`${config.apiUrl}/internal/models`, { signal }) and
fetch(`${config.apiUrl}/internal/providers`, { signal })) so requests are
cancellable and the cache keys reflect the apiUrl dependency.
| import { cache } from "react"; | ||
|
|
||
| import { getConfig } from "./config-server"; | ||
|
|
||
| export interface ApiProvider { | ||
| id: string; | ||
| createdAt: string; | ||
| name: string | null; | ||
| description: string | null; | ||
| streaming: boolean | null; | ||
| cancellation: boolean | null; | ||
| color: string | null; | ||
| website: string | null; | ||
| announcement: string | null; | ||
| status: "active" | "inactive"; | ||
| } | ||
|
|
||
| export interface ApiModelProviderMapping { | ||
| id: string; | ||
| createdAt: string; | ||
| modelId: string; | ||
| providerId: string; | ||
| modelName: string; | ||
| inputPrice: string | null; | ||
| outputPrice: string | null; | ||
| cachedInputPrice: string | null; | ||
| imageInputPrice: string | null; | ||
| requestPrice: string | null; | ||
| contextSize: number | null; | ||
| maxOutput: number | null; | ||
| streaming: boolean; | ||
| vision: boolean | null; | ||
| reasoning: boolean | null; | ||
| reasoningOutput: string | null; | ||
| tools: boolean | null; | ||
| jsonOutput: boolean | null; | ||
| jsonOutputSchema: boolean | null; | ||
| webSearch: boolean | null; | ||
| discount: string | null; | ||
| stability: "stable" | "beta" | "unstable" | "experimental" | null; | ||
| supportedParameters: string[] | null; | ||
| deprecatedAt: string | null; | ||
| deactivatedAt: string | null; | ||
| status: "active" | "inactive"; | ||
| } | ||
|
|
||
| export interface ApiModel { | ||
| id: string; | ||
| createdAt: string; | ||
| releasedAt: string | null; | ||
| name: string | null; | ||
| aliases: string[] | null; | ||
| description: string | null; | ||
| family: string; | ||
| free: boolean | null; | ||
| output: string[] | null; | ||
| stability: "stable" | "beta" | "unstable" | "experimental" | null; | ||
| status: "active" | "inactive"; | ||
| mappings: ApiModelProviderMapping[]; | ||
| } | ||
|
|
||
| export const fetchModels = cache(async (): Promise<ApiModel[]> => { | ||
| const config = getConfig(); | ||
| try { | ||
| const response = await fetch(`${config.apiBackendUrl}/internal/models`, { | ||
| next: { revalidate: 60 }, | ||
| }); | ||
| if (!response.ok) { | ||
| console.error("Failed to fetch models:", response.statusText); | ||
| return []; | ||
| } | ||
| const data = await response.json(); | ||
| return data.models || []; | ||
| } catch (error) { | ||
| console.error("Error fetching models:", error); | ||
| return []; | ||
| } | ||
| }); | ||
|
|
||
| export const fetchProviders = cache(async (): Promise<ApiProvider[]> => { | ||
| const config = getConfig(); | ||
| try { | ||
| const response = await fetch(`${config.apiBackendUrl}/internal/providers`, { | ||
| next: { revalidate: 60 }, | ||
| }); | ||
| if (!response.ok) { | ||
| console.error("Failed to fetch providers:", response.statusText); | ||
| return []; | ||
| } | ||
| const data = await response.json(); | ||
| return data.providers || []; | ||
| } catch (error) { | ||
| console.error("Error fetching providers:", error); | ||
| return []; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Next.js 15 React 19 server-only import best practices server component data fetching cache
💡 Result:
Quick best-practices summary for Next.js 15 + React 19 server-only imports, Server Components, data fetching and caching:
- Default to server-first. Keep UI in Server Components and only add "use client" where interactivity is required. This minimizes client bundle size. [1][3]
- Use server-only imports for server-only logic. Use the server-only package (import 'server-only') to guarantee utilities/functions never run on the client. Co-locate these with Server Components. [3]
- Mark client code explicitly with "use client" at file top; avoid importing client modules from server modules unless you intend to create a client boundary. Importing a client module from a server component forces client evaluation of that subtree. [1][3]
- For server-callable functions use "use server" (module or function-level) for Server Functions; they must live in server-only files and are async (network call when invoked from client). Validate/authorize inputs. [2]
- Cache data on the server with React’s cache utilities (React cache / cache(fn)) and the preload pattern to dedupe and warm requests; combine with 'server-only' to ensure safety. Use cache for read-heavy, idempotent fetches. [3]
- Be explicit about fetch caching: Next.js 15 changes made fetch default to no-store (so you must opt into caching/revalidation). Set Cache-Control / revalidate values or wrap fetch with cache/unstable_cache as appropriate. Review route/handler default dynamic/static modes. [4][3]
- Use platform/global cache for production (Vercel Data Cache or equivalent) for low-latency, regionally-distributed caches and on-demand revalidation when available. Tie your server cache keys and revalidation logic to application invariants. [5]
- Keep Server Components pure and side-effect-minimal. Do data-fetching in the component (or a server-only util) rather than inside useEffect; prefer async/await at render time for RSC. Avoid placing event handlers or client-only APIs in server modules. [1][3]
- Security: never leak secrets to client bundles. Ensure server-only modules hold secrets and that Server Functions validate/authorize inputs before performing mutations. [2][3]
- Debugging/dev ergonomics: expect changed defaults in Next.js 15 (server caching behavior and HMR tweaks). Explicitly configure experimental/staleTimes or serverComponentsHmrCache if needed for dev or CDN behavior. [4]
If you want, I can produce a small starter pattern (example files) showing: server-only util + cached fetch, Server Component using it, and a Client Component calling a Server Function.
Add server-only guard to prevent accidental client bundling.
This module depends on ./config-server and exports functions that must only run on the server. Add import "server-only" at the top to guarantee it hard-fails if imported into a client component.
Proposed fix
+import "server-only";
import { cache } from "react";🤖 Prompt for AI Agents
In @apps/ui/src/lib/fetch-models.ts around lines 1 - 96, This file exports
server-only helpers (fetchModels, fetchProviders) that call getConfig and must
not be bundled to the client; add a top-level import "server-only" as the first
import in the module to enforce a hard failure if accidentally imported into
client code, keeping the existing imports (getConfig) and exported functions
unchanged.
| // Sort models by createdAt (when added to LLM Gateway), newest first | ||
| // Falls back to releasedAt if createdAt is not available | ||
| // Note: createdAt comes from API response, releasedAt is in the models package | ||
| const sortedModels = [...models].sort((a, b) => { | ||
| const dateA = a.publishedAt | ||
| ? new Date(a.publishedAt).getTime() | ||
| : a.releasedAt | ||
| ? new Date(a.releasedAt).getTime() | ||
| : 0; | ||
| const dateB = b.publishedAt | ||
| ? new Date(b.publishedAt).getTime() | ||
| : b.releasedAt | ||
| ? new Date(b.releasedAt).getTime() | ||
| : 0; | ||
| const dateA = | ||
| "createdAt" in a && a.createdAt | ||
| ? new Date(a.createdAt as string | Date).getTime() | ||
| : a.releasedAt | ||
| ? new Date(a.releasedAt).getTime() | ||
| : 0; | ||
| const dateB = | ||
| "createdAt" in b && b.createdAt | ||
| ? new Date(b.createdAt as string | Date).getTime() | ||
| : b.releasedAt | ||
| ? new Date(b.releasedAt).getTime() | ||
| : 0; | ||
| return dateB - dateA; | ||
| }); |
There was a problem hiding this comment.
Harden the createdAt/releasedAt sort against invalid dates and avoid ad-hoc type narrowing.
new Date(...).getTime() can return NaN for bad inputs; current comparator would then yield NaN and produce unstable ordering. Also, the "createdAt" in a + casts are a smell—prefer aligning the prop types (e.g., ApiModel) so this becomes straightforward.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/db/migrations/meta/_journal.json:
- Around line 558-563: The JSON entry at idx 79 (version "8", tag
"1768093355_red_black_widow") uses spaces for indentation; update that entry and
convert the entire _journal.json file to use tabs for indentation per the coding
guidelines, ensuring each nested level (including the new entry's keys "idx",
"version", "when", "tag", "breakpoints") is indented with tabs rather than
spaces; use your editor's "convert indentation to tabs" or run a formatter to
replace leading spaces with tabs and verify the file still parses as valid JSON
after the change.
🧹 Nitpick comments (4)
apps/playground/src/components/playground/chat-page-client.tsx (2)
308-346: Extract duplicated capability checking logic into shared utilities.The
supportsReasoningandsupportsWebSearchlogic is duplicated betweenChatPageClientandExtraChatPanel. This violates DRY principles and creates a maintenance burden where changes must be made in multiple places.♻️ Recommended refactor: Extract to shared utility functions
Create a new file
apps/playground/src/lib/model-capabilities.ts:import type { ApiModel, ApiModelProviderMapping } from "@/lib/fetch-models"; export function getModelSupportsReasoning( models: ApiModel[], selectedModel: string, ): boolean { if (!selectedModel) { return false; } const [providerId, modelId] = selectedModel.includes("/") ? (selectedModel.split("/") as [string, string]) : ["", selectedModel]; const def = models.find((m) => m.id === modelId); if (!def) { return false; } if (!providerId) { return def.mappings.some((p) => p.reasoning); } const mapping = def.mappings.find((p) => p.providerId === providerId); return !!mapping?.reasoning; } export function getModelSupportsWebSearch( models: ApiModel[], selectedModel: string, ): boolean { if (!selectedModel) { return false; } const [providerId, modelId] = selectedModel.includes("/") ? (selectedModel.split("/") as [string, string]) : ["", selectedModel]; const def = models.find((m) => m.id === modelId); if (!def) { return false; } if (!providerId) { return def.mappings.some((p) => p.webSearch); } const mapping = def.mappings.find((p) => p.providerId === providerId); return !!mapping?.webSearch; }Then replace the duplicated
useMemoblocks with:const supportsReasoning = useMemo(() => { - if (!selectedModel) { - return false; - } - const [providerId, modelId] = selectedModel.includes("/") - ? (selectedModel.split("/") as [string, string]) - : ["", selectedModel]; - const def = models.find((m) => m.id === modelId); - if (!def) { - return false; - } - if (!providerId) { - return def.mappings.some((p: ApiModelProviderMapping) => p.reasoning); - } - const mapping = def.mappings.find( - (p: ApiModelProviderMapping) => p.providerId === providerId, - ); - return !!mapping?.reasoning; + return getModelSupportsReasoning(models, selectedModel); }, [models, selectedModel]); const supportsWebSearch = useMemo(() => { - if (!selectedModel) { - return false; - } - const [providerId, modelId] = selectedModel.includes("/") - ? (selectedModel.split("/") as [string, string]) - : ["", selectedModel]; - const def = models.find((m) => m.id === modelId); - if (!def) { - return false; - } - if (!providerId) { - return def.mappings.some((p: ApiModelProviderMapping) => p.webSearch); - } - const mapping = def.mappings.find( - (p: ApiModelProviderMapping) => p.providerId === providerId, - ); - return !!mapping?.webSearch; + return getModelSupportsWebSearch(models, selectedModel); }, [models, selectedModel]);Also applies to: 1136-1174
320-324: Optional: Remove redundant type annotations.The explicit
ApiModelProviderMappingtype annotations in the callback parameters are redundant since TypeScript infers these types fromdef.mappings(which is typed asApiModelProviderMapping[]).✨ Optional simplification
-return def.mappings.some((p: ApiModelProviderMapping) => p.reasoning); +return def.mappings.some((p) => p.reasoning);-const mapping = def.mappings.find( - (p: ApiModelProviderMapping) => p.providerId === providerId, -); +const mapping = def.mappings.find((p) => p.providerId === providerId);Apply similarly to all occurrences in both
supportsReasoningandsupportsWebSearch.Also applies to: 340-344, 1148-1152, 1168-1172
packages/db/migrations/1768093355_red_black_widow.sql (2)
1-4: LGTM! Consider timezone-aware timestamp.The new model columns align with the ApiModel type introduced in this PR. All columns are appropriately nullable for backward compatibility with existing data.
Optional: Consider using timestamptz
If
released_atrepresents a global release time, considertimestamptz(timestamp with time zone) instead oftimestampto avoid timezone ambiguity:-ALTER TABLE "model" ADD COLUMN "released_at" timestamp;--> statement-breakpoint +ALTER TABLE "model" ADD COLUMN "released_at" timestamptz;--> statement-breakpoint
5-8: Consider defaults for booleans and constraints for discount.The nullable boolean columns introduce three-state logic (true/false/null) which can complicate queries and application logic. The
discountcolumn lacks validation constraints.Recommended: Add defaults and constraints
-ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output" boolean;--> statement-breakpoint -ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output_schema" boolean;--> statement-breakpoint -ALTER TABLE "model_provider_mapping" ADD COLUMN "web_search" boolean;--> statement-breakpoint -ALTER TABLE "model_provider_mapping" ADD COLUMN "discount" numeric;--> statement-breakpoint +ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output" boolean DEFAULT false;--> statement-breakpoint +ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output_schema" boolean DEFAULT false;--> statement-breakpoint +ALTER TABLE "model_provider_mapping" ADD COLUMN "web_search" boolean DEFAULT false;--> statement-breakpoint +ALTER TABLE "model_provider_mapping" ADD COLUMN "discount" numeric CHECK (discount >= 0 AND discount <= 1);--> statement-breakpointNote: The constraint assumes
discountrepresents a percentage (0.0 to 1.0). Adjust the range if it represents a different unit (e.g., currency amount).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/playground/src/components/playground/chat-page-client.tsxpackages/db/migrations/1768093355_red_black_widow.sqlpackages/db/migrations/meta/1768093355_snapshot.jsonpackages/db/migrations/meta/_journal.json
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
packages/db/migrations/meta/_journal.jsonapps/playground/src/components/playground/chat-page-client.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/playground/src/components/playground/chat-page-client.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
apps/playground/src/components/playground/chat-page-client.tsx
apps/{ui,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{ui,playground}/**/*.{ts,tsx}: Use cookies for user-settings which are not saved in the database to ensure SSR works
Use Next.js App Router with React Server Components for frontend developmentUse cookies for user-settings which are not saved in the database to ensure SSR works
Files:
apps/playground/src/components/playground/chat-page-client.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
apps/playground/src/components/playground/chat-page-client.tsx
apps/{ui,playground,docs}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
next/linkfor links andnext/navigation's router for programmatic navigation
Files:
apps/playground/src/components/playground/chat-page-client.tsx
🧬 Code graph analysis (1)
apps/playground/src/components/playground/chat-page-client.tsx (1)
apps/playground/src/lib/fetch-models.ts (3)
ApiModel(45-58)ApiProvider(3-14)ApiModelProviderMapping(16-43)
⏰ 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: build / run
- GitHub Check: lint / run
- GitHub Check: generate / run
- GitHub Check: e2e-shards (3)
- GitHub Check: e2e-shards (2)
- GitHub Check: e2e-shards (5)
- GitHub Check: e2e-shards (4)
- GitHub Check: e2e-shards (1)
- GitHub Check: autofix
🔇 Additional comments (4)
apps/playground/src/components/playground/chat-page-client.tsx (2)
30-39: LGTM! Clean type migration to API-facing types.The migration from
ModelDefinition/ProviderDefinitiontoApiModel/ApiProvider/ApiModelProviderMappingis consistent and maintains type safety throughout the component props.
308-346: Migration logic correctly updated to use mappings.The transition from
def.providerstodef.mappingspreserves the original logic correctly:
- Checks all mappings via
some()when no provider is specified- Finds specific mapping via
find()when provider ID is presentThe migration maintains type safety and functional correctness.
packages/db/migrations/meta/_journal.json (1)
558-564: LGTM! Journal entry structure is correct.The new migration entry correctly follows the existing pattern with sequential
idx(79), appropriateversion("8"), matchingtag("1768093355_red_black_widow"), and correct timestamp.packages/db/migrations/1768093355_red_black_widow.sql (1)
9-9: Bothstabilitycolumns are intentional. The design allows providers to override model-level stability: if a provider-specific stability is not set, it defaults to the model's stability level. This pattern is consistently used throughout the codebase (e.g., in model-selector.tsx, model-provider-card.tsx, and gateway/models.ts) with the effective stability calculated asproviderStability ?? modelStability. No changes needed.
| { | ||
| "idx": 79, | ||
| "version": "8", | ||
| "when": 1768093355910, | ||
| "tag": "1768093355_red_black_widow", | ||
| "breakpoints": true |
There was a problem hiding this comment.
Fix indentation to use tabs per coding guidelines.
The coding guidelines specify that all JSON files must use tabs for indentation, but this file uses spaces. While the entire file currently uses spaces, the new entry should align with the project standards.
🔧 Fix indentation
Consider converting the entire file to use tabs for consistency with coding guidelines. For the new entry specifically:
- },
- {
- "idx": 79,
- "version": "8",
- "when": 1768093355910,
- "tag": "1768093355_red_black_widow",
- "breakpoints": true
+ },
+ {
+ "idx": 79,
+ "version": "8",
+ "when": 1768093355910,
+ "tag": "1768093355_red_black_widow",
+ "breakpoints": trueNote: The entire file should be converted to tabs for full compliance. Many editors have a "Convert Indentation to Tabs" command that can automate this.
📝 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.
| { | |
| "idx": 79, | |
| "version": "8", | |
| "when": 1768093355910, | |
| "tag": "1768093355_red_black_widow", | |
| "breakpoints": true | |
| { | |
| "idx": 79, | |
| "version": "8", | |
| "when": 1768093355910, | |
| "tag": "1768093355_red_black_widow", | |
| "breakpoints": true |
🤖 Prompt for AI Agents
In @packages/db/migrations/meta/_journal.json around lines 558 - 563, The JSON
entry at idx 79 (version "8", tag "1768093355_red_black_widow") uses spaces for
indentation; update that entry and convert the entire _journal.json file to use
tabs for indentation per the coding guidelines, ensuring each nested level
(including the new entry's keys "idx", "version", "when", "tag", "breakpoints")
is indented with tabs rather than spaces; use your editor's "convert indentation
to tabs" or run a formatter to replace leading spaces with tabs and verify the
file still parses as valid JSON after the change.
…nce models The publishedAt fields with future dates were removed to correct metadata. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
- Update models page to fetch models/providers server-side - Create timeline-client component for client-side interactivity - Update all-models component to receive models/providers as props - Update model-search to support optional props with client fallback - Update shared multi-model-selector and multi-provider-selector to accept both static and API types - Remove unused useModels hook and local multi-model-selector - Fix price comparisons to use parseFloat for string prices 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
steebchen
changed the title
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 (2)
packages/shared/src/components/multi-provider-selector.tsx (1)
132-132: Add fallback for nullablenameto match line 82.
ApiProvider.namecan benull, but this line renders it directly without a fallback. Line 82 correctly handles this withprovider?.name || providerId. Apply the same pattern here for consistency.Proposed fix
- <span className="font-medium">{provider.name}</span> + <span className="font-medium">{provider.name || provider.id}</span>apps/ui/src/components/models/all-models.tsx (1)
1290-1311: Request price display has multiple parseFloat calls without NaN guards.Lines 1292, 1296, 1301-1302, and 1311 all call
parseFloatonrequestPriceanddiscountwithout validating forNaN. Malformed data could display incorrect values.🐛 Proposed fix with NaN validation
-{provider.requestPrice !== null && -provider.requestPrice !== undefined && -parseFloat(provider.requestPrice) > 0 ? ( +{(() => { + if (provider.requestPrice === null || provider.requestPrice === undefined) return "—"; + const reqPrice = parseFloat(provider.requestPrice); + if (Number.isNaN(reqPrice) || reqPrice <= 0) return "—"; + const discountVal = provider.discount ? parseFloat(provider.discount) : 0; + const safeDiscount = Number.isNaN(discountVal) ? 0 : discountVal; + if (safeDiscount > 0) { + return ( + <div className="flex flex-col justify-center items-center"> + <span className="line-through text-muted-foreground text-xs"> + ${reqPrice.toFixed(3)} + </span> + <span className="text-green-600 font-semibold"> + ${(reqPrice * (1 - safeDiscount)).toFixed(3)} + </span> + <span className="text-muted-foreground text-xs">/req</span> + </div> + ); + } + return ( + <> + ${reqPrice.toFixed(3)} + <span className="text-muted-foreground text-xs ml-1">/req</span> + </> + ); +})()}
🤖 Fix all issues with AI agents
In `@apps/playground/src/components/model-selector.tsx`:
- Around line 712-715: The code calls getProviderLogo((selectedProviderId ||
selectedModel.mappings[0].providerId) as ProviderId) which can throw if
selectedModel.mappings is empty; change the expression to use optional chaining
and a safe fallback — e.g., use (selectedProviderId ||
selectedModel.mappings[0]?.providerId || /* fallback provider id or undefined
*/) cast as ProviderId — so getProviderLogo receives a defined/nullable value
handled safely; update the call site in model-selector.tsx accordingly.
In `@apps/ui/src/components/models/all-models.tsx`:
- Line 1404: The playground URL construction uses
model.providerDetails[0]?.provider.providerId which becomes "undefined" when
providerDetails is empty; update the JSX to compute a safe providerId first
(e.g., const providerId = model.providerDetails?.[0]?.provider?.providerId ??
model.providerId ?? 'unknown') and then build the href as
`${config.playgroundUrl}?model=${encodeURIComponent(`${providerId}/${model.id}`)}`
so you never interpolate an undefined provider segment; replace the inline
expression with this guarded variable where the current href uses
model.providerDetails[0]?.provider.providerId and model.id.
- Around line 180-187: The modelsWithProviders useMemo currently uses a non-null
assertion when building providerDetails which will throw if providers.find(...)
returns undefined; update the mapping logic in modelsWithProviders (inside the
useMemo that creates baseModels and providerDetails) to handle missing providers
gracefully by checking the result of providers.find(provider => provider.id ===
mapping.providerId) and either (a) filter out mappings with no matching provider
before creating providerDetails or (b) supply a safe fallback providerInfo
object (e.g., an "unknown" provider stub that includes the mapping.providerId)
so downstream code never receives undefined; ensure the rest of the code that
consumes providerDetails can handle the chosen fallback.
In `@apps/ui/src/components/timeline/timeline-client.tsx`:
- Around line 317-327: The Button currently wraps a Link causing nested
interactive elements; update the Button usage to pass the asChild prop so it
renders a Slot and lets the Link be the actual interactive element — change the
Button containing Link (the element using <Button ...> around <Link ...> with
ArrowUpRight and item.id) to <Button asChild ...> wrapping the same Link markup
and keep the Link props (href, className, children) unchanged so semantics and
styling remain intact.
♻️ Duplicate comments (2)
apps/ui/src/components/models/all-models.tsx (1)
578-604: Missing NaN validation informatPricecan produce invalid output.
parseFloaton lines 585-586 may returnNaNfor malformed strings (e.g.,"N/A", empty strings that pass the null check). This would display$NaNor produce incorrect discount calculations. This concern was previously flagged by Copilot.🐛 Proposed fix to validate parsed numbers
const formatPrice = ( price: string | null | undefined, discount?: string | null, ) => { if (price === null || price === undefined) { return "—"; } const priceNum = parseFloat(price); + if (Number.isNaN(priceNum)) { + return "—"; + } - const discountNum = discount ? parseFloat(discount) : 0; + let discountNum = 0; + if (discount) { + const parsedDiscount = parseFloat(discount); + if (!Number.isNaN(parsedDiscount)) { + discountNum = parsedDiscount; + } + } const originalPrice = (priceNum * 1e6).toFixed(2);apps/playground/src/components/model-selector.tsx (1)
193-200: Add NaN validation after parsing string prices.The
parseFloat()calls on lines 193 and 200 can returnNaNif the string contains invalid numeric data. This would cause incorrect pricing displays (e.g., showing "NaN" or performing broken arithmetic).Proposed fix
const basePrice = parseFloat(basePriceStr); + if (Number.isNaN(basePrice)) { + return { label: "Unknown" }; + } // Free models if (basePrice === 0) { return { label: "Free", original: "Free" }; } - const discountNum = mapping.discount ? parseFloat(mapping.discount) : 0; + const parsedDiscount = mapping.discount ? parseFloat(mapping.discount) : 0; + const discountNum = Number.isNaN(parsedDiscount) ? 0 : parsedDiscount;
🧹 Nitpick comments (8)
packages/shared/src/components/multi-provider-selector.tsx (1)
21-32: Consolidate theApiProviderinterface to a single shared location.This interface is duplicated across four files with identical definitions:
packages/shared/src/components/multi-provider-selector.tsxpackages/shared/src/components/multi-model-selector.tsxapps/playground/src/lib/fetch-models.ts(exported)apps/ui/src/lib/fetch-models.ts(exported)Define
ApiProvideronce inpackages/shared(e.g., in a shared types module) and import it in the component files and app packages to avoid maintenance drift.apps/ui/src/components/models/all-models.tsx (1)
407-437: Price sorting filters and parses correctly but NaN values could corrupt sort order.The filtering with type guards (
filter((p): p is string => ...)) is good, butparseFloaton filtered strings could still returnNaNfor non-numeric strings, causing unpredictable sort behavior.♻️ Add NaN filtering after parseFloat
case "inputPrice": { const aInputPrices = a.providerDetails .map((p) => p.provider.inputPrice) .filter((p): p is string => p !== null && p !== undefined) - .map((p) => parseFloat(p)); + .map((p) => parseFloat(p)) + .filter((n) => !Number.isNaN(n)); const bInputPrices = b.providerDetails .map((p) => p.provider.inputPrice) .filter((p): p is string => p !== null && p !== undefined) - .map((p) => parseFloat(p)); + .map((p) => parseFloat(p)) + .filter((n) => !Number.isNaN(n));Apply the same pattern to
outputPrice,cachedInputPrice, andrequestPricecases.packages/shared/src/components/multi-model-selector.tsx (2)
192-199: Minor: Avoid callinggetModelReleasedAttwice per comparison.Each sort comparison calls
getModelReleasedAttwice per model. Consider caching the result for clarity and minor performance improvement.♻️ Suggested refactor
.sort((a, b) => { - const dateA = getModelReleasedAt(a) - ? new Date(getModelReleasedAt(a)!).getTime() - : 0; - const dateB = getModelReleasedAt(b) - ? new Date(getModelReleasedAt(b)!).getTime() - : 0; + const releasedAtA = getModelReleasedAt(a); + const releasedAtB = getModelReleasedAt(b); + const dateA = releasedAtA ? new Date(releasedAtA).getTime() : 0; + const dateB = releasedAtB ? new Date(releasedAtB).getTime() : 0; return dateB - dateA; })
27-82: Consider consolidating duplicate API type definitions.These interfaces (
ApiModel,ApiModelProviderMapping,ApiProvider) are duplicated across the codebase inapps/playground/src/lib/fetch-models.ts,apps/ui/src/lib/fetch-models.ts, and locally here. Maintaining separate copies creates a risk of drift if the API schema changes.Move these types to
packages/shared/src/lib/api-types.tsand import them in both this component andmulti-provider-selector.tsxto ensure consistency across the codebase.apps/ui/src/components/timeline/timeline-client.tsx (1)
113-127: Consider memoizinggroupedByYearandyearsto avoid recomputation on every render.These computations run on every render since they're outside
useMemo. For large model lists, this could impact performance. Additionally, initializingactiveYearstate withyears[0]from a non-memoized value works on initial render but the pattern is fragile.♻️ Suggested refactor
- // Group by year - const groupedByYear: Record<string, TimelineItem[]> = {}; - for (const item of timelineItems) { - const year = item.releasedAt?.getFullYear()?.toString() ?? "Unknown"; - if (!groupedByYear[year]) { - groupedByYear[year] = []; - } - groupedByYear[year].push(item); - } - - const years = Object.keys(groupedByYear) - .filter((y) => y !== "Unknown") - .sort((a, b) => Number(b) - Number(a)); - - const [activeYear, setActiveYear] = useState<string>(years[0] ?? ""); + // Group by year + const { groupedByYear, years } = useMemo(() => { + const grouped: Record<string, TimelineItem[]> = {}; + for (const item of timelineItems) { + const year = item.releasedAt?.getFullYear()?.toString() ?? "Unknown"; + if (!grouped[year]) { + grouped[year] = []; + } + grouped[year].push(item); + } + const sortedYears = Object.keys(grouped) + .filter((y) => y !== "Unknown") + .sort((a, b) => Number(b) - Number(a)); + return { groupedByYear: grouped, years: sortedYears }; + }, [timelineItems]); + + const [activeYear, setActiveYear] = useState<string>(() => years[0] ?? "");apps/ui/src/components/shared/model-search.tsx (1)
112-157: Logic is correct; consider Map-based provider lookup for better performance.The date parsing and free flag calculation are well-implemented with proper null/undefined guards. Using
model.mappingsaligns with the new API structure.One optional optimization:
providers.find()is called for every mapping, making this O(n×m). If the provider list grows, consider building aMap<string, ApiProvider>once outside the loop.♻️ Optional optimization
const entries = useMemo<ModelSearchEntry[]>(() => { const now = new Date(); const map = new Map<string, ModelSearchEntry>(); + const providerMap = new Map(providers.map((p) => [p.id, p])); for (const model of models) { // ... for (const mapping of model.mappings) { // ... - const provider = providers.find((p) => p.id === mapping.providerId); + const provider = providerMap.get(mapping.providerId); // ... } } // ... }, [models, providers]);apps/playground/src/components/model-selector.tsx (2)
253-269: Consider adding NaN guards inapplyDiscounthelper.Similar to
getMappingPriceInfo,parseFloaton lines 260 and 264 can returnNaN. While the comparison logic may accidentally filter out NaN values (since NaN comparisons return false), explicit handling would be more robust.Proposed fix
const applyDiscount = ( priceStr: string | null | undefined, discountStr?: string | null, ) => { if (priceStr === null || priceStr === undefined) { return undefined; } const price = parseFloat(priceStr); + if (Number.isNaN(price)) { + return undefined; + } if (price === 0) { return 0; } const discount = discountStr ? parseFloat(discountStr) : 0; - if (!discount || discount <= 0) { + if (!discount || Number.isNaN(discount) || discount <= 0) { return price; } return price * (1 - discount); };
568-573: Consider NaN handling in price filter.If
inputPriceorrequestPricecontains invalid data,parseFloatreturnsNaN, which will fail all comparison checks. This effectively excludes malformed entries from all price ranges, which may be acceptable behavior but worth noting.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (11)
apps/playground/src/components/model-selector.tsxapps/ui/src/app/models/page.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/api-keys/multi-model-selector.tsxapps/ui/src/components/models/all-models.tsxapps/ui/src/components/shared/model-search.tsxapps/ui/src/components/timeline/timeline-client.tsxpackages/db/src/schema.tspackages/shared/src/components/model-selector.tsxpackages/shared/src/components/multi-model-selector.tsxpackages/shared/src/components/multi-provider-selector.tsx
💤 Files with no reviewable changes (1)
- apps/ui/src/components/api-keys/multi-model-selector.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/shared/src/components/model-selector.tsx
- packages/db/src/schema.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
packages/shared/src/components/multi-provider-selector.tsxapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/models/page.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/timeline/timeline-client.tsxapps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsxpackages/shared/src/components/multi-model-selector.tsx
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
packages/shared/src/components/multi-provider-selector.tsxapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/models/page.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/timeline/timeline-client.tsxapps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsxpackages/shared/src/components/multi-model-selector.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
packages/shared/src/components/multi-provider-selector.tsxapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/models/page.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/timeline/timeline-client.tsxapps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsxpackages/shared/src/components/multi-model-selector.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
packages/shared/src/components/multi-provider-selector.tsxapps/ui/src/components/shared/model-search.tsxapps/ui/src/app/models/page.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/timeline/timeline-client.tsxapps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsxpackages/shared/src/components/multi-model-selector.tsx
apps/{ui,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{ui,playground}/**/*.{ts,tsx}: Use cookies for user-settings which are not saved in the database to ensure SSR works
Use Next.js App Router with React Server Components for frontend developmentUse cookies for user-settings which are not saved in the database to ensure SSR works
Files:
apps/ui/src/components/shared/model-search.tsxapps/ui/src/app/models/page.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/timeline/timeline-client.tsxapps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsx
apps/{ui,playground,docs}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
next/linkfor links andnext/navigation's router for programmatic navigation
Files:
apps/ui/src/components/shared/model-search.tsxapps/ui/src/app/models/page.tsxapps/ui/src/app/timeline/page.tsxapps/ui/src/components/timeline/timeline-client.tsxapps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{ts,tsx} : Use Next.js App Router with React Server Components for frontend development
Applied to files:
apps/ui/src/components/shared/model-search.tsxapps/ui/src/components/timeline/timeline-client.tsx
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to apps/{ui,playground}/**/*.{tsx} : Use `next/link` for links and `next/navigation`'s router for programmatic navigation
Applied to files:
apps/ui/src/components/shared/model-search.tsx
🧬 Code graph analysis (6)
packages/shared/src/components/multi-provider-selector.tsx (2)
packages/models/src/providers.ts (1)
ProviderDefinition(9-32)packages/db/src/schema.ts (1)
provider(634-666)
apps/ui/src/components/shared/model-search.tsx (1)
apps/ui/src/lib/fetch-models.ts (2)
ApiModel(47-60)ApiProvider(5-16)
apps/ui/src/app/models/page.tsx (3)
packages/models/src/models.ts (1)
models(239-257)packages/models/src/providers.ts (1)
providers(34-460)apps/ui/src/components/models/all-models.tsx (1)
AllModels(99-1639)
apps/ui/src/app/timeline/page.tsx (2)
packages/models/src/models.ts (1)
models(239-257)apps/ui/src/lib/fetch-models.ts (1)
fetchModels(62-78)
apps/ui/src/components/models/all-models.tsx (3)
apps/ui/src/lib/fetch-models.ts (3)
ApiModel(47-60)ApiModelProviderMapping(18-45)ApiProvider(5-16)packages/models/src/models.ts (2)
models(239-257)StabilityLevel(179-179)apps/api/src/index.ts (1)
config(29-40)
packages/shared/src/components/multi-model-selector.tsx (2)
packages/models/src/models.ts (2)
StabilityLevel(179-179)ModelDefinition(181-237)packages/models/src/providers.ts (2)
ProviderDefinition(9-32)providers(34-460)
⏰ 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: lint / run
- GitHub Check: generate / run
- GitHub Check: build / run
- GitHub Check: test / run
- GitHub Check: e2e-shards (5)
- GitHub Check: e2e-shards (4)
- GitHub Check: e2e-shards (2)
- GitHub Check: e2e-shards (1)
- GitHub Check: autofix
- GitHub Check: e2e-shards (3)
🔇 Additional comments (29)
packages/shared/src/components/multi-provider-selector.tsx (1)
136-141: Good defensive handling for nullable color.The conditional rendering correctly prevents displaying a color indicator when
provider.coloris falsy, which properly handles bothnull(fromApiProvider) andundefined(fromProviderDefinition).apps/ui/src/components/models/all-models.tsx (5)
69-87: LGTM! Type definitions align with API response shapes.The type imports and interface definitions correctly align with the new API types from
fetch-models.ts. TheModelWithProvidersinterface properly extendsApiModeland establishes the correct shape forproviderDetailswith both the mapping and provider info.
377-384: Default sorting bycreatedAtis correct.The default sort logic properly handles the new
createdAtfield with descending order (newest first), and correctly falls back to 0 for missing dates.
522-566: Stability helper functions handle nullable types correctly.The updated functions properly handle
StabilityLevel | null | undefinedwith explicit null checks. The logic is sound.
1171-1176: Safe null-coalescing for border color.The use of
providerInfo?.color ?? undefinedcorrectly handles the case whereproviderInfoorcolormight be null, preventing React from receivingnullas a style value (which could cause warnings in some cases).
487-487: Dependencies array is complete.The useMemo dependency array correctly includes all external values:
searchQuery,filters,sortField,sortDirection,models, andproviders.apps/ui/src/app/models/page.tsx (2)
30-32: Server component data fetching pattern is correct.The async server component correctly fetches data and passes it as props to the client component
AllModels. This follows Next.js App Router best practices for data fetching at the page level.
23-27: No action needed - error handling already exists in fetch functions.Both
fetchModels()andfetchProviders()have internal try-catch blocks that handle errors gracefully by returning empty arrays instead of throwing exceptions. The Promise.all() will resolve successfully even if either fetch fails, preventing any unhandled promise rejections and allowing the page to render with empty data rather than crashing.Likely an incorrect or invalid review comment.
packages/shared/src/components/multi-model-selector.tsx (4)
84-90: LGTM!The union type approach for
modelsandprovidersprops provides good flexibility during the migration from legacy types to API-backed types.
92-119: LGTM!The updated function signatures properly handle
nullvalues from the API while maintaining backwards compatibility with the existingStabilityLeveltype.
123-125: LGTM!The type guard using
"mappings" in modelis a reliable discriminator sinceApiModelhasmappingswhileModelDefinitionhasproviders. This approach enables clean type narrowing.
245-252: LGTM!The optional chaining on
firstProvider?.providerInfo?.colorproperly handles the case whereApiProvider.colorcan benull, preventing runtime errors when rendering the provider color indicator.apps/ui/src/components/timeline/timeline-client.tsx (3)
1-21: LGTM!Proper "use client" directive usage, clean imports following the top-level import guideline, and appropriate use of
next/linkfor navigation. The component setup correctly separates concerns with internal UI primitives.
22-70: LGTM!The helper functions and type definitions are well-structured. The
formatDatefunction handles null/undefined safely, andisSignificantprovides a reasonable heuristic for highlighting flagship models.
72-111: LGTM!The component logic is well-structured with proper use of
useMemofor expensive computations inbaseItemsandtimelineItems. The filtering, search, and sorting functionality is clean. The UI rendering with year navigation, month grouping, and card display is well-organized.Also applies to: 131-346
apps/ui/src/app/timeline/page.tsx (1)
1-14: LGTM!Clean implementation following Next.js App Router best practices. The async server component correctly fetches data server-side and delegates client-side rendering to
TimelineClient. The metadata export is properly structured for SEO. ThefetchModelsutility handles errors gracefully by returning an empty array, ensuring the page won't crash on API failures.apps/ui/src/components/shared/model-search.tsx (8)
1-26: LGTM!Imports are well-organized, using top-level imports as required. Correctly uses
next/navigationfor the router.
27-44: LGTM!The
ModelSearchEntryinterface update andformatMonthLabelutility are well-implemented with proper handling for undefined dates.
46-54: LGTM!Clean interface design that supports both prop-based data injection and self-fetching behavior.
91-110: LGTM!Keyboard shortcut handling is robust with proper checks for editable elements and cleanup on unmount.
159-171: LGTM!Sorting logic correctly prioritizes
createdAtdescending with alphabetical name as secondary sort. Items without dates sort to the end as expected.
173-183: LGTM!Grouping logic is clean and correctly uses
createdAtfor month labels.
185-252: LGTM!JSX structure is clean. Correctly uses
router.pushfromnext/navigationfor programmatic navigation as per guidelines.
59-89: Remove this review comment—the shared hooks referenced do not exist in the codebase.The comment assumes that shared hooks (
useModels,useProviders,useModelsAndProviders) exist per PR objectives, but these hooks are not present in the codebase. The only related functions arefetchModelsandfetchProvidersinfetch-models.ts, which are server-side cached functions, not React Query hooks.The inline
useQuerycalls inmodel-search.tsxare the appropriate pattern for client-side fetching in a "use client" component. The implementation includes proper error handling and matches the architecture's 60-second revalidation pattern.Likely an incorrect or invalid review comment.
apps/playground/src/components/model-selector.tsx (5)
58-72: LGTM!Clean migration to the new API-based types. The props interface correctly reflects the new data shape.
438-455: LGTM!The sorting logic correctly prioritizes
createdAtwith fallback toreleasedAt. The runtime check"createdAt" in acombined with the type cast handles the type uncertainty reasonably.
851-853: LGTM!Good null safety handling for the provider color, converting
nulltoundefinedfor the style prop.
991-994: LGTM!The check
parseFloat(p.requestPrice) > 0safely handles invalid strings sinceNaN > 0evaluates tofalse.
1486-1487: LGTM!The truthy check correctly determines whether to display the image pricing section based on the presence of price strings.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| return getProviderLogo( | ||
| (selectedProviderId || | ||
| selectedModel.providers[0].providerId) as ProviderId, | ||
| selectedModel.mappings[0].providerId) as ProviderId, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to see the actual code at those lines
wc -l apps/playground/src/components/model-selector.tsxRepository: theopenco/llmgateway
Length of output: 118
🏁 Script executed:
# Read the relevant section around lines 712-715
sed -n '700,730p' apps/playground/src/components/model-selector.tsxRepository: theopenco/llmgateway
Length of output: 1149
🏁 Script executed:
# Let's also search for the type definition of selectedModel to understand the mappings structure
rg "selectedModel" apps/playground/src/components/model-selector.tsx -B 3 -A 3 | head -100Repository: theopenco/llmgateway
Length of output: 2911
🏁 Script executed:
# Find type definitions for the data structures involved
fd -e ts -e tsx apps/playground/src/components | head -20Repository: theopenco/llmgateway
Length of output: 489
🏁 Script executed:
# Search for type definitions of Model and mappings
rg "interface Model|type Model|mappings.*\[" apps/playground/src --include="*.ts" --include="*.tsx" -B 2 -A 5 | head -150Repository: theopenco/llmgateway
Length of output: 505
🏁 Script executed:
# Look for model schema or type definitions in the codebase
fd . apps/playground/src -name "*.ts" -o -name "*.tsx" | xargs rg -l "mappings" | head -10Repository: theopenco/llmgateway
Length of output: 713
🏁 Script executed:
# Check what the models array contains and where it comes from
rg "const models|const.*models\s*=" apps/playground/src/components/model-selector.tsx -B 2 -A 5Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Look for the models prop and its type definition
rg "models" apps/playground/src/components/model-selector.tsx | head -30Repository: theopenco/llmgateway
Length of output: 1032
🏁 Script executed:
# Search for Model interface definition
rg "interface Model|type Model" --type-listRepository: theopenco/llmgateway
Length of output: 453
🏁 Script executed:
# Let's check if there's a types file
fd "types\|d\.ts" apps/playground/srcRepository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Search for the props interface for the component
sed -n '1,100p' apps/playground/src/components/model-selector.tsxRepository: theopenco/llmgateway
Length of output: 2047
🏁 Script executed:
# Find the fetch-models file
fd fetch-models apps/playground/srcRepository: theopenco/llmgateway
Length of output: 103
🏁 Script executed:
# Read the type definitions from fetch-models
cat apps/playground/src/lib/fetch-models.tsRepository: theopenco/llmgateway
Length of output: 2537
🏁 Script executed:
# Search for other occurrences of mappings[0] in the codebase
rg "mappings\[0\]" apps/playground/srcRepository: theopenco/llmgateway
Length of output: 336
🏁 Script executed:
# Check if there's any validation or filtering of models before displaying
rg "mappings\." apps/playground/src/components/model-selector.tsx | head -30Repository: theopenco/llmgateway
Length of output: 242
🏁 Script executed:
# Check if the API response has any validation
rg "mappings" apps/gateway/src/modelsRepository: theopenco/llmgateway
Length of output: 562
🏁 Script executed:
# Let's examine the model-utils.ts to understand how mappings[0] is used elsewhere
cat apps/playground/src/lib/model-utils.ts | head -200Repository: theopenco/llmgateway
Length of output: 1682
🏁 Script executed:
# Also check if there's any database schema or backend validation that ensures non-empty mappings
rg "mappings" packages/models/src -A 3 -B 1 | head -100Repository: theopenco/llmgateway
Length of output: 1171
Use optional chaining to handle potential empty mappings array.
Accessing selectedModel.mappings[0].providerId without checking array length could throw if a model has no mappings. This pattern is already used defensively elsewhere in the codebase (e.g., model.mappings[0]?.providerId in model-utils.ts), indicating the possibility of empty mappings. Apply the same defensive approach here.
Proposed fix
return getProviderLogo(
(selectedProviderId ||
- selectedModel.mappings[0].providerId) as ProviderId,
+ selectedModel.mappings[0]?.providerId) as ProviderId,
);📝 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.
| return getProviderLogo( | |
| (selectedProviderId || | |
| selectedModel.providers[0].providerId) as ProviderId, | |
| selectedModel.mappings[0].providerId) as ProviderId, | |
| ); | |
| return getProviderLogo( | |
| (selectedProviderId || | |
| selectedModel.mappings[0]?.providerId) as ProviderId, | |
| ); |
🤖 Prompt for AI Agents
In `@apps/playground/src/components/model-selector.tsx` around lines 712 - 715,
The code calls getProviderLogo((selectedProviderId ||
selectedModel.mappings[0].providerId) as ProviderId) which can throw if
selectedModel.mappings is empty; change the expression to use optional chaining
and a safe fallback — e.g., use (selectedProviderId ||
selectedModel.mappings[0]?.providerId || /* fallback provider id or undefined
*/) cast as ProviderId — so getProviderLogo receives a defined/nullable value
handled safely; update the call site in model-selector.tsx accordingly.
| const modelsWithProviders: ModelWithProviders[] = useMemo(() => { | ||
| const baseModels = (models as readonly ModelDefinition[]).map((model) => ({ | ||
| const baseModels = models.map((model) => ({ | ||
| ...model, | ||
| providerDetails: model.providers.map((provider) => ({ | ||
| provider, | ||
| providerInfo: providers.find((p) => p.id === provider.providerId)!, | ||
| providerDetails: model.mappings.map((mapping) => ({ | ||
| provider: mapping, | ||
| providerInfo: providers.find((p) => p.id === mapping.providerId)!, | ||
| })), | ||
| })); |
There was a problem hiding this comment.
Potential runtime error if provider not found.
The non-null assertion (!) on line 185 assumes every mapping.providerId will have a corresponding provider in the providers array. If the API returns a mapping with an unknown provider ID, this will cause undefined access errors downstream.
🐛 Proposed fix to handle missing provider gracefully
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) => {
+ const providerInfo = providers.find((p) => p.id === mapping.providerId);
+ if (!providerInfo) {
+ return null;
+ }
+ return {
+ provider: mapping,
+ providerInfo,
+ };
+ })
+ .filter((detail): detail is NonNullable<typeof detail> => detail !== null),
}));🤖 Prompt for AI Agents
In `@apps/ui/src/components/models/all-models.tsx` around lines 180 - 187, The
modelsWithProviders useMemo currently uses a non-null assertion when building
providerDetails which will throw if providers.find(...) returns undefined;
update the mapping logic in modelsWithProviders (inside the useMemo that creates
baseModels and providerDetails) to handle missing providers gracefully by
checking the result of providers.find(provider => provider.id ===
mapping.providerId) and either (a) filter out mappings with no matching provider
before creating providerDetails or (b) supply a safe fallback providerInfo
object (e.g., an "unknown" provider stub that includes the mapping.providerId)
so downstream code never receives undefined; ensure the rest of the code that
consumes providerDetails can handle the chosen fallback.
| > | ||
| <a | ||
| href={`${config.playgroundUrl}?model=${encodeURIComponent(`${model.providers[0]?.providerId}/${model.id}`)}`} | ||
| href={`${config.playgroundUrl}?model=${encodeURIComponent(`${model.providerDetails[0]?.provider.providerId}/${model.id}`)}`} |
There was a problem hiding this comment.
Playground URL construction assumes providerDetails is non-empty.
model.providerDetails[0]?.provider.providerId uses optional chaining but if providerDetails is empty, this would result in undefined/model.id in the URL.
🐛 Proposed fix to guard against empty providerDetails
-href={`${config.playgroundUrl}?model=${encodeURIComponent(`${model.providerDetails[0]?.provider.providerId}/${model.id}`)}`}
+href={`${config.playgroundUrl}?model=${encodeURIComponent(model.providerDetails[0]?.provider.providerId ? `${model.providerDetails[0].provider.providerId}/${model.id}` : model.id)}`}🤖 Prompt for AI Agents
In `@apps/ui/src/components/models/all-models.tsx` at line 1404, The playground
URL construction uses model.providerDetails[0]?.provider.providerId which
becomes "undefined" when providerDetails is empty; update the JSX to compute a
safe providerId first (e.g., const providerId =
model.providerDetails?.[0]?.provider?.providerId ?? model.providerId ??
'unknown') and then build the href as
`${config.playgroundUrl}?model=${encodeURIComponent(`${providerId}/${model.id}`)}`
so you never interpolate an undefined provider segment; replace the inline
expression with this guarded variable where the current href uses
model.providerDetails[0]?.provider.providerId and model.id.
| <Button variant="ghost" size="sm"> | ||
| <Link | ||
| href={`/models/${encodeURIComponent( | ||
| item.id, | ||
| )}`} | ||
| className="inline-flex items-center gap-1 text-xs font-medium text-primary hover:underline" | ||
| > | ||
| <span>View model details</span> | ||
| <ArrowUpRight className="h-3 w-3" /> | ||
| </Link> | ||
| </Button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate the Button component
fd -e tsx -e ts -p "*button*" apps/ui/src/components/ | head -20Repository: theopenco/llmgateway
Length of output: 351
🏁 Script executed:
#!/bin/bash
# Check for asChild usage in the codebase
rg "asChild" apps/ui/ --type=tsx --type=ts -n | head -20Repository: theopenco/llmgateway
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check the Button component implementation
find apps/ui/src/components -name "*button*" -type fRepository: theopenco/llmgateway
Length of output: 111
🏁 Script executed:
#!/bin/bash
# Read the timeline-client.tsx file around lines 317-327
cat -n apps/ui/src/components/timeline/timeline-client.tsx | sed -n '310,335p'Repository: theopenco/llmgateway
Length of output: 1213
🏁 Script executed:
#!/bin/bash
# Check imports in timeline-client.tsx to find Button source
head -50 apps/ui/src/components/timeline/timeline-client.tsx | grep -n "import.*Button\|from"Repository: theopenco/llmgateway
Length of output: 552
🏁 Script executed:
#!/bin/bash
# Find Button component using glob pattern
fd -g "*button*" --type f apps/ui/src/components/Repository: theopenco/llmgateway
Length of output: 111
🏁 Script executed:
#!/bin/bash
# Find Button component more broadly
fd "button" --type f apps/ui/src/ | grep -i buttonRepository: theopenco/llmgateway
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Read the Button component to check for asChild prop
cat -n apps/ui/src/lib/components/button.tsxRepository: theopenco/llmgateway
Length of output: 2505
Use Button's asChild prop to avoid nested interactive elements.
The <Button> wrapping <Link> creates invalid HTML with nested interactive elements, causing accessibility issues. The Button component supports the asChild prop—use it to render as a Radix <Slot> instead of a <button>, allowing the Link to be the primary interactive element.
Proposed fix
-<Button variant="ghost" size="sm">
+<Button variant="ghost" size="sm" asChild>
<Link
href={`/models/${encodeURIComponent(
item.id,
)}`}
- className="inline-flex items-center gap-1 text-xs font-medium text-primary hover:underline"
+ className="inline-flex items-center gap-1"
>
<span>View model details</span>
<ArrowUpRight className="h-3 w-3" />
</Link>
</Button>📝 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.
| <Button variant="ghost" size="sm"> | |
| <Link | |
| href={`/models/${encodeURIComponent( | |
| item.id, | |
| )}`} | |
| className="inline-flex items-center gap-1 text-xs font-medium text-primary hover:underline" | |
| > | |
| <span>View model details</span> | |
| <ArrowUpRight className="h-3 w-3" /> | |
| </Link> | |
| </Button> | |
| <Button variant="ghost" size="sm" asChild> | |
| <Link | |
| href={`/models/${encodeURIComponent( | |
| item.id, | |
| )}`} | |
| className="inline-flex items-center gap-1" | |
| > | |
| <span>View model details</span> | |
| <ArrowUpRight className="h-3 w-3" /> | |
| </Link> | |
| </Button> |
🤖 Prompt for AI Agents
In `@apps/ui/src/components/timeline/timeline-client.tsx` around lines 317 - 327,
The Button currently wraps a Link causing nested interactive elements; update
the Button usage to pass the asChild prop so it renders a Slot and lets the Link
be the actual interactive element — change the Button containing Link (the
element using <Button ...> around <Link ...> with ArrowUpRight and item.id) to
<Button asChild ...> wrapping the same Link markup and keep the Link props
(href, className, children) unchanged so semantics and styling remain intact.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/ui/src/components/models/all-models.tsx (1)
376-387: Use proper typing for sort comparison values and considerreleasedAtfallback for default sorting.The
anytype at lines 385–386 violates the TypeScript "noany" guideline. Since all switch cases assign eitherstringornumbervalues, use a union type instead:let aValue: string | number; let bValue: string | number;Also, the default sort (lines 379–383) falls back to
0whencreatedAtis missing or invalid. SincereleasedAtexists in the model structure and serves the same semantic purpose, consider falling back to it for more robust "newest first" ordering:if (!sortField) { const aDate = a.createdAt ? new Date(a.createdAt).getTime() : (a.releasedAt ? new Date(a.releasedAt).getTime() : 0); const bDate = b.createdAt ? new Date(b.createdAt).getTime() : (b.releasedAt ? new Date(b.releasedAt).getTime() : 0); return bDate - aDate; }apps/playground/src/components/model-selector.tsx (3)
504-509: Removeany/as anyto match TS guidelines (and fix checkbox typing)
Line 508 (as any) and Line 591 (value: any) violate the “noany” guideline; you can type this precisely and also avoid passing"indeterminate"into boolean filters.Proposed fix
const availableProviders = React.useMemo(() => { - const ids = new Set( - allEntries.filter((e) => e.mapping).map((e) => e.mapping!.providerId), - ); - return providers.filter((p) => ids.has(p.id as any)); + const ids = new Set<string>( + allEntries.flatMap((e) => (e.mapping ? [e.mapping.providerId] : [])), + ); + return providers.filter((p) => ids.has(p.id)); }, [allEntries, providers]); - const updateFilter = (key: keyof FilterState, value: any) => { + const updateFilter = <K extends keyof FilterState>( + key: K, + value: FilterState[K], + ) => { setFilters((prev) => ({ ...prev, [key]: value })); }; <Checkbox id="show-root" checked={filters.showOnlyRoot} onCheckedChange={(checked) => - updateFilter("showOnlyRoot", checked) + updateFilter("showOnlyRoot", checked === true) } /> <Checkbox id="hide-unstable" checked={filters.hideUnstable} onCheckedChange={(checked) => - updateFilter("hideUnstable", checked) + updateFilter("hideUnstable", checked === true) } />-->
Also applies to: 591-593, 808-810, 951-953
1165-1173: Guard provider background color whencoloris missing
Lines 1168-1169 / 1625-1626 can producebackgroundColor: "undefined15".Proposed fix
return ProviderIcon ? ( <div className="p-2 rounded-lg" style={{ - backgroundColor: `${previewEntry.provider?.color}15`, + backgroundColor: previewEntry.provider?.color + ? `${previewEntry.provider.color}15` + : undefined, }} > <ProviderIcon className="h-5 w-5 dark:text-white" /> </div> ) : null; // ... return ProviderIcon ? ( <div className="p-2 rounded-lg" style={{ - backgroundColor: `${selectedDetails.provider?.color}15`, + backgroundColor: selectedDetails.provider?.color + ? `${selectedDetails.provider.color}15` + : undefined, }} > <ProviderIcon className="h-6 w-6 dark:text-white" /> </div> ) : null;-->
Also applies to: 1622-1630
153-160: Remove or completeimageOutputpricing support—currently half-wired
PriceFieldincludes"imageOutput"(line 159) but:
getMappingPriceInfo(lines 176-187) has no case for itApiModelProviderMappinghas noimageOutputPricefieldminImageOutputPrice(line 248) is declared and returned but never computed in the loopEither implement full image output pricing or remove
"imageOutput"fromPriceField,minImageOutputPricefromRootAggregateInfo(line 234), and its UI references.Also applies to: 176-187, 228-238, 247-249
🤖 Fix all issues with AI agents
In `@packages/db/migrations/1768420347_dazzling_lester.sql`:
- Around line 1-9: The boolean columns json_output, json_output_schema, and
web_search on model_provider_mapping need explicit defaults and a backfill to
avoid NULL semantics drift: update existing rows to false, then ALTER TABLE to
set DEFAULT false and finally mark the columns NOT NULL (i.e., backfill
model_provider_mapping.json_output/json_output_schema/model_provider_mapping.web_search
to false, ALTER COLUMN ... SET DEFAULT false, ALTER COLUMN ... SET NOT NULL). Do
this in the migration so existing data is consistent and future inserts get a
sane default.
♻️ Duplicate comments (6)
packages/db/migrations/meta/_journal.json (1)
564-570: Indentation in_journal.jsonshould use tabs (repo guideline).This is the same formatting issue previously flagged for this file—please convert the file (or at least the new entry) to tabs so future diffs stay consistent.
apps/ui/src/components/models/all-models.tsx (3)
180-187: Remove the non-null assertion on provider lookup; handle missing providers and empty mappings safely.
providers.find((p) => p.id === mapping.providerId)!will throw if the API returns a mapping for an unknown provider (and can also cascade intoproviderDetails[0]assumptions elsewhere).Proposed fix (also avoids O(n*m) finds)
const modelsWithProviders: ModelWithProviders[] = useMemo(() => { - const baseModels = models.map((model) => ({ + const providersById = new Map(providers.map((p) => [p.id, p])); + 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) => { + const providerInfo = providersById.get(mapping.providerId); + if (!providerInfo) return null; + return { provider: mapping, providerInfo }; + }) + .filter( + (d): d is { provider: ApiModelProviderMapping; providerInfo: ApiProvider } => + d !== null, + ), }));
281-286: Guard all parseFloat() usage against NaN to avoid broken filters/sorts and “$NaN” UI.There are multiple
parseFloat(...)sites where invalid/empty strings will produceNaNand then flow into comparisons and.toFixed(...).Suggested direction: introduce a small helper (e.g.
parseFiniteFloat(value): number | null) and use it in:
- free/discount checks
- input/output/request price filters
- sort min/max extraction
formatPrice()and requestPrice rendering (also clamp/validatediscountto[0, 1])Also applies to: 309-337, 406-473, 577-603, 1289-1316
1403-1403: Playground URL can still produceundefined/<modelId>when providerDetails is empty.apps/playground/src/components/model-selector.tsx (2)
405-416: Avoid crashing on emptymappingswhen resolving provider logo
Line 714 can throw ifselectedModel.mappingsis empty. Also, if both IDs are missing you can end up callinggetProviderIcon(undefined)indirectly.This is the same issue previously flagged on this file.
Proposed fix
- const getProviderLogo = (providerId: ProviderId) => { + const getProviderLogo = (providerId?: ProviderId) => { + if (!providerId) { + return <div className="h-10 w-10 bg-gray-200 rounded" />; + } const LogoComponent = providerLogoUrls[providerId]; if (LogoComponent) { return <LogoComponent className="h-10 w-10 object-contain" />; } const IconComponent = getProviderIcon(providerId); return IconComponent ? ( <IconComponent className="h-10 w-10" /> ) : ( <div className="h-10 w-10 bg-gray-200 rounded" /> ); }; // ... return ( <Button /* ... */> {selectedModel ? ( <div className="flex items-center gap-2 sm:gap-3 min-w-0 flex-1"> {(() => { if ( selectedModelId === selectedModel.id && !selectedProviderDef ) { return ( <Sparkles className="h-5 w-5 shrink-0 text-primary" /> ); } - return getProviderLogo( - (selectedProviderId || - selectedModel.mappings[0].providerId) as ProviderId, - ); + const resolvedProviderId = + selectedProviderId || selectedModel.mappings[0]?.providerId; + return getProviderLogo(resolvedProviderId as ProviderId | undefined); })()}-->
Also applies to: 632-645, 712-715
168-226: Harden price parsing: guard againstNaNand clamp discount
MultipleparseFloat(...)calls (e.g., Line 193 / Line 200 / Line 260 / Line 264 / Lines 569-573) can yieldNaN, which then leaks into formatting, comparisons, or “free” filtering. This was previously raised forgetMappingPriceInfo, but the same class of issue also exists inapplyDiscountand filters.Proposed fix
function getMappingPriceInfo( mapping: ApiModelProviderMapping | undefined, field: PriceField, ): MappingPriceInfo { if (!mapping) { return { label: "Unknown" }; } let basePriceStr: string | null | undefined; // ... if (basePriceStr === null || basePriceStr === undefined) { return { label: "Unknown" }; } const basePrice = parseFloat(basePriceStr); + if (!Number.isFinite(basePrice)) { + return { label: "Unknown" }; + } // Free models if (basePrice === 0) { return { label: "Free", original: "Free" }; } - const discountNum = mapping.discount ? parseFloat(mapping.discount) : 0; + const discountRaw = mapping.discount ? parseFloat(mapping.discount) : 0; + const discountNum = Number.isFinite(discountRaw) + ? Math.min(Math.max(discountRaw, 0), 1) + : 0; // Request price is a flat per-request fee, not per-token if (field === "request") { const original = `$${basePrice.toFixed(3)}/req`; if (discountNum > 0) { const discountedPrice = basePrice * (1 - discountNum); const discounted = `$${discountedPrice.toFixed(3)}/req`; return { label: discounted, original, discounted }; } return { label: original, original }; } // ... } function getRootAggregateInfo(model: ApiModel): RootAggregateInfo { // ... const applyDiscount = ( priceStr: string | null | undefined, discountStr?: string | null, ) => { if (priceStr === null || priceStr === undefined) { return undefined; } const price = parseFloat(priceStr); + if (!Number.isFinite(price)) { + return undefined; + } if (price === 0) { return 0; } - const discount = discountStr ? parseFloat(discountStr) : 0; - if (!discount || discount <= 0) { + const discountRaw = discountStr ? parseFloat(discountStr) : 0; + const discount = Number.isFinite(discountRaw) + ? Math.min(Math.max(discountRaw, 0), 1) + : 0; + if (discount <= 0) { return price; } return price * (1 - discount); };if (filters.priceRange !== "all") { list = list.filter((e) => { if (!e.mapping) { return false; } - const price = e.mapping.inputPrice - ? parseFloat(e.mapping.inputPrice) - : 0; - const requestPrice = e.mapping.requestPrice - ? parseFloat(e.mapping.requestPrice) - : 0; + const price = e.mapping.inputPrice ? parseFloat(e.mapping.inputPrice) : NaN; + const requestPrice = e.mapping.requestPrice + ? parseFloat(e.mapping.requestPrice) + : NaN; switch (filters.priceRange) { case "free": - return price === 0 && requestPrice === 0; + return ( + Number.isFinite(price) && + Number.isFinite(requestPrice) && + price === 0 && + requestPrice === 0 + ); case "low": - return price > 0 && price <= 0.000001; + return Number.isFinite(price) && price > 0 && price <= 0.000001; case "medium": - return price > 0.000001 && price <= 0.00001; + return Number.isFinite(price) && price > 0.000001 && price <= 0.00001; case "high": - return price > 0.00001; + return Number.isFinite(price) && price > 0.00001; default: return true; } }); }- const hasRequestPrice = model.mappings.some( - (p) => p.requestPrice && parseFloat(p.requestPrice) > 0, - ); + const hasRequestPrice = model.mappings.some((p) => { + if (!p.requestPrice) return false; + const n = parseFloat(p.requestPrice); + return Number.isFinite(n) && n > 0; + }); - const hasRequestPrice = - mapping!.requestPrice && - parseFloat(mapping!.requestPrice) > 0; + const hasRequestPrice = (() => { + if (!mapping!.requestPrice) return false; + const n = parseFloat(mapping!.requestPrice); + return Number.isFinite(n) && n > 0; + })();-->
Also applies to: 253-269, 559-586, 991-994, 1067-1070
🧹 Nitpick comments (3)
packages/db/migrations/1768420347_dazzling_lester.sql (1)
1-9: Constrainstabilityand consider tighter typing fordiscount/ JSON columns.
stability texton both tables: consider aCHECKconstraint (or a dedicated enum type) so invalid values can’t enter the DB.discount numeric: consider a precision/scale (e.g.numeric(5,4)) and aCHECK (discount >= 0 and discount <= 1)if it’s meant to be a fraction.aliases json: if you ever query it,jsonbis usually a better default in Postgres (indexing/operators).apps/playground/src/components/model-selector.tsx (2)
438-455: Simplify createdAt sorting and drop unnecessaryin/casts
The"createdAt" in achecks andas string | Datecasts are likely unnecessary ifApiModel.createdAtis typed (even optional). This reads simpler and avoids surprising behavior whencreatedAtis present-but-empty.Proposed refactor
const sortedModels = [...models].sort((a, b) => { - const dateA = - "createdAt" in a && a.createdAt - ? new Date(a.createdAt as string | Date).getTime() - : a.releasedAt - ? new Date(a.releasedAt).getTime() - : 0; - const dateB = - "createdAt" in b && b.createdAt - ? new Date(b.createdAt as string | Date).getTime() - : b.releasedAt - ? new Date(b.releasedAt).getTime() - : 0; + const dateA = a.createdAt + ? new Date(a.createdAt).getTime() + : a.releasedAt + ? new Date(a.releasedAt).getTime() + : 0; + const dateB = b.createdAt + ? new Date(b.createdAt).getTime() + : b.releasedAt + ? new Date(b.releasedAt).getTime() + : 0; return dateB - dateA; });-->
477-494: Avoidproviders.find(...)in the inner loop
Line 481 does an O(P) lookup for every mapping; easy win is aMapkeyed by provider id.Proposed refactor
const allEntries = React.useMemo(() => { const out: { model: ApiModel; mapping?: ApiModelProviderMapping; provider?: ApiProvider; isRoot?: boolean; searchText: string; }[] = []; const now = new Date(); + const providerById = new Map(providers.map((p) => [p.id, p])); // ... for (const m of sortedModels) { // ... for (const mp of m.mappings) { const isDeactivated = mp.deactivatedAt && new Date(mp.deactivatedAt) <= now; if (!isDeactivated) { - const provider = providers.find((p) => p.id === mp.providerId); + const provider = providerById.get(mp.providerId); const searchText = normalize( [m.name ?? "", m.family ?? "", m.id, provider?.name ?? ""].join( " ", ), ); out.push({ model: m, mapping: mp, provider, isRoot: false, searchText, }); } } }-->
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/playground/src/components/model-selector.tsxapps/ui/src/components/models/all-models.tsxpackages/db/migrations/1768420347_dazzling_lester.sqlpackages/db/migrations/meta/1768420347_snapshot.jsonpackages/db/migrations/meta/_journal.json
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
packages/db/migrations/meta/_journal.jsonapps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
apps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
apps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsx
apps/{ui,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{ui,playground}/**/*.{ts,tsx}: Use cookies for user-settings which are not saved in the database to ensure SSR works
Use Next.js App Router with React Server Components for frontend developmentUse cookies for user-settings which are not saved in the database to ensure SSR works
Files:
apps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
apps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsx
apps/{ui,playground,docs}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
next/linkfor links andnext/navigation's router for programmatic navigation
Files:
apps/ui/src/components/models/all-models.tsxapps/playground/src/components/model-selector.tsx
⏰ 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: e2e-shards (4)
- GitHub Check: e2e-shards (3)
- GitHub Check: e2e-shards (5)
- GitHub Check: e2e-shards (2)
- GitHub Check: e2e-shards (1)
- GitHub Check: test / run
- GitHub Check: lint / run
- GitHub Check: build / run
- GitHub Check: generate / run
- GitHub Check: autofix
🔇 Additional comments (2)
packages/db/migrations/meta/_journal.json (1)
564-571: New journal entry looks consistent (idx/tag/when/breakpoints).apps/ui/src/components/models/all-models.tsx (1)
69-100: Type/props refactor to ApiModel/ApiProvider looks clean and localized.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| ALTER TABLE "model" ADD COLUMN "released_at" timestamp;--> statement-breakpoint | ||
| ALTER TABLE "model" ADD COLUMN "aliases" json;--> statement-breakpoint | ||
| ALTER TABLE "model" ADD COLUMN "description" text;--> statement-breakpoint | ||
| ALTER TABLE "model" ADD COLUMN "stability" text;--> statement-breakpoint | ||
| ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output" boolean;--> statement-breakpoint | ||
| ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output_schema" boolean;--> statement-breakpoint | ||
| ALTER TABLE "model_provider_mapping" ADD COLUMN "web_search" boolean;--> statement-breakpoint | ||
| ALTER TABLE "model_provider_mapping" ADD COLUMN "discount" numeric;--> statement-breakpoint | ||
| ALTER TABLE "model_provider_mapping" ADD COLUMN "stability" text; No newline at end of file |
There was a problem hiding this comment.
Add defaults/backfill (especially for new boolean flags) to avoid NULL semantics drift.
Right now json_output, json_output_schema, and web_search will be NULL for existing rows. If your API/types treat these as booleans, you’ll eventually get mismatches (and filtering/sorting logic will silently treat NULL as falsey).
Proposed migration adjustment
-ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output" boolean;--> statement-breakpoint
-ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output_schema" boolean;--> statement-breakpoint
-ALTER TABLE "model_provider_mapping" ADD COLUMN "web_search" boolean;--> statement-breakpoint
+ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output" boolean NOT NULL DEFAULT false;--> statement-breakpoint
+ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output_schema" boolean NOT NULL DEFAULT false;--> statement-breakpoint
+ALTER TABLE "model_provider_mapping" ADD COLUMN "web_search" boolean NOT NULL DEFAULT false;--> statement-breakpoint📝 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.
| ALTER TABLE "model" ADD COLUMN "released_at" timestamp;--> statement-breakpoint | |
| ALTER TABLE "model" ADD COLUMN "aliases" json;--> statement-breakpoint | |
| ALTER TABLE "model" ADD COLUMN "description" text;--> statement-breakpoint | |
| ALTER TABLE "model" ADD COLUMN "stability" text;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output" boolean;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output_schema" boolean;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "web_search" boolean;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "discount" numeric;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "stability" text; | |
| ALTER TABLE "model" ADD COLUMN "released_at" timestamp;--> statement-breakpoint | |
| ALTER TABLE "model" ADD COLUMN "aliases" json;--> statement-breakpoint | |
| ALTER TABLE "model" ADD COLUMN "description" text;--> statement-breakpoint | |
| ALTER TABLE "model" ADD COLUMN "stability" text;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output" boolean NOT NULL DEFAULT false;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "json_output_schema" boolean NOT NULL DEFAULT false;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "web_search" boolean NOT NULL DEFAULT false;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "discount" numeric;--> statement-breakpoint | |
| ALTER TABLE "model_provider_mapping" ADD COLUMN "stability" text; |
🤖 Prompt for AI Agents
In `@packages/db/migrations/1768420347_dazzling_lester.sql` around lines 1 - 9,
The boolean columns json_output, json_output_schema, and web_search on
model_provider_mapping need explicit defaults and a backfill to avoid NULL
semantics drift: update existing rows to false, then ALTER TABLE to set DEFAULT
false and finally mark the columns NOT NULL (i.e., backfill
model_provider_mapping.json_output/json_output_schema/model_provider_mapping.web_search
to false, ALTER COLUMN ... SET DEFAULT false, ALTER COLUMN ... SET NOT NULL). Do
this in the migration so existing data is consistent and future inserts get a
sane default.
…umns - Added a migration to set default values for several columns in `model` and `model_provider_mapping` tables. - Updated database schema to reflect these defaults. - Defaults include timestamps, booleans, text defaults, JSON arrays, and enumerated types. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
steebchen
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/db/migrations/1768422765_classy_colonel_america.sql`:
- Around line 1-11: Remove this manual migration file and instead add the
default values to the schema definitions in packages/db/src/schema.ts for the
affected tables/columns (the "model" table defaults: released_at, name, aliases,
description, free, output, stability; and the "model_provider_mapping" table
defaults: json_output, web_search, discount, stability), then run the project's
migration generation command (pnpm run setup or pnpm migrations) to
auto-generate the proper migration; delete the handwritten SQL once the
auto-generated migration is created.
♻️ Duplicate comments (1)
packages/db/migrations/meta/_journal.json (1)
564-577: New migration journal entries look correct.The two new entries (idx 80 and 81) are properly sequenced with incrementing indices, version "8", and valid timestamps/tags. The indentation inconsistency (spaces vs. tabs per coding guidelines) was already flagged in a previous review.
🧹 Nitpick comments (2)
packages/db/src/schema.ts (2)
677-686: New model metadata fields look good; minor note on nullable booleans.The schema additions are well-structured with appropriate defaults. One consideration:
free(line 682) is nullable withdefault(false), meaning existing rows remainNULL. In SQL,NULL != false, so queries likeWHERE free = falsewon't match rows withNULLvalues. If this matters for filtering logic, consider either:
- Adding
.notNull()constraint, or- Using
WHERE free IS NOT TRUE/COALESCE(free, false) = falsein queries.This is a minor point if the migration backfills existing data or if query logic already handles NULLs.
733-739: Mapping fields correctly extend provider capabilities.The additions properly model JSON output support, web search capability, discount pricing, and stability tiers. The intentional omission of a default for
jsonOutputSchema(line 734) makes sense—it represents an unknown capability state distinct fromfalse.💡 Optional: Extract shared stability enum
The stability enum is duplicated between
model(line 684) andmodelProviderMapping(line 737). Consider extracting to a shared constant:// At top of file or in types.ts const stabilityEnum = ["stable", "beta", "unstable", "experimental"] as const; // In table definitions stability: text({ enum: stabilityEnum }).default("stable"),
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/db/migrations/1768422765_classy_colonel_america.sqlpackages/db/migrations/meta/1768422765_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/schema.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
packages/db/migrations/meta/_journal.jsonpackages/db/src/schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
packages/db/src/schema.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
packages/db/src/schema.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Drizzle ORM with latest object syntax for database operations
Files:
packages/db/src/schema.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
packages/db/src/schema.ts
{apps/api,apps/gateway,packages/db}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with latest object syntax for database operations
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
packages/db/src/schema.ts
🧠 Learnings (2)
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to packages/db/**/*.{ts,tsx} : Use Drizzle ORM with latest object syntax for database operations
Applied to files:
packages/db/src/schema.ts
📚 Learning: 2025-12-03T12:42:26.162Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T12:42:26.162Z
Learning: Applies to {apps/api,apps/gateway,packages/db}/**/*.ts : Use Drizzle ORM with latest object syntax for database operations
Applied to files:
packages/db/src/schema.ts
⏰ 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: generate / run
- GitHub Check: test / run
- GitHub Check: build / run
- GitHub Check: lint / run
- GitHub Check: autofix
- GitHub Check: e2e-shards (3)
- GitHub Check: e2e-shards (1)
- GitHub Check: e2e-shards (4)
- GitHub Check: e2e-shards (2)
- GitHub Check: e2e-shards (5)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…-from-api-ai3qwv 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
steebchen
changed the title
steebchen
changed the title
steebchen
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/db/migrations/1768524767_flowery_garia.sql`:
- Around line 10-12: The UPDATE in the migration incorrectly assigns a boolean
to the text column: change the backfill to set the "model"."name" column to the
text default value '(empty)' for NULLs (i.e., use UPDATE "model" SET "name" =
'(empty)' WHERE "name" IS NULL) so it matches the ALTER TABLE default and avoids
type mismatch; keep the subsequent ALTER TABLE "model" ALTER COLUMN "name" SET
DEFAULT '(empty)' and ALTER COLUMN "name" SET NOT NULL as-is.
In `@packages/db/src/schema.ts`:
- Around line 700-711: The releasedAt column in packages/db/src/schema.ts is
declared as releasedAt: timestamp().defaultNow().notNull() but the sync logic in
apps/worker/src/services/sync-models.ts treats it as optional; change the schema
to reflect optionality by removing .notNull() and the defaultNow(), e.g., make
releasedAt nullable with no forced default (or explicit default null) so rows
inserted outside sync won't get an incorrect migration timestamp; update any
dependent code or comments in sync-models.ts to document that sync will populate
release dates when available.
♻️ Duplicate comments (1)
packages/db/migrations/meta/_journal.json (1)
578-585: Indentation uses spaces instead of tabs.The new migration entry continues to use space indentation rather than tabs as required by the coding guidelines. This was already flagged in a previous review comment for an earlier entry.
🧹 Nitpick comments (1)
packages/db/src/schema.ts (1)
762-766: Deduplicate the stability enum to prevent drift.
stabilityis defined here and inmodel; consider extracting a shared constant to keep values aligned.Example refactor
+const stabilityEnum = ["stable", "beta", "unstable", "experimental"] as const; + export const model = pgTable( "model", { @@ - stability: text({ - enum: ["stable", "beta", "unstable", "experimental"], - }) + stability: text({ + enum: stabilityEnum, + }) .default("stable") .notNull(), @@ export const modelProviderMapping = pgTable( "model_provider_mapping", { @@ - stability: text({ - enum: ["stable", "beta", "unstable", "experimental"], - }) + stability: text({ + enum: stabilityEnum, + }) .default("stable") .notNull(),
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/db/migrations/1768524767_flowery_garia.sqlpackages/db/migrations/meta/1768524767_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/schema.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use tabs for indentation
Files:
packages/db/migrations/meta/_journal.jsonpackages/db/src/schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never useanyoras anyunless absolutely necessary in TypeScript code
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
packages/db/src/schema.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Always use top-levelimport, never use require or dynamic imports
No unnecessary code comments
Files:
packages/db/src/schema.ts
packages/db/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Drizzle ORM with latest object syntax for database operations
Files:
packages/db/src/schema.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level
import, never use require or dynamic imports
Files:
packages/db/src/schema.ts
{apps/api,apps/gateway,packages/db}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with latest object syntax for database operations
For database reads: Usedb().query.<table>.findMany()ordb().query.<table>.findFirst()
Files:
packages/db/src/schema.ts
🧠 Learnings (2)
📚 Learning: 2025-12-03T12:42:14.219Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T12:42:14.219Z
Learning: Applies to packages/db/**/*.{ts,tsx} : Use Drizzle ORM with latest object syntax for database operations
Applied to files:
packages/db/src/schema.ts
📚 Learning: 2025-12-03T12:42:26.162Z
Learnt from: CR
Repo: theopenco/llmgateway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T12:42:26.162Z
Learning: Applies to {apps/api,apps/gateway,packages/db}/**/*.ts : Use Drizzle ORM with latest object syntax for database operations
Applied to files:
packages/db/src/schema.ts
🔇 Additional comments (3)
packages/db/migrations/1768524767_flowery_garia.sql (3)
1-4: LGTM!New columns added to the
modeltable with sensible defaults. Thereleased_atdefaulting tonow()ensures existing rows get a timestamp, and the other defaults align with the API model expectations described in the PR.
5-9: LGTM!New capability and pricing columns for
model_provider_mappinglook correct. Defaults are appropriate for boolean flags and numeric discount.
13-18: LGTM!The backfill-then-constrain pattern for
freeandoutputcolumns is correct. Data types match their respective columns, and defaults align with the API expectations.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| update "model" set "name" = false where "name" is null;--> statement-breakpoint | ||
| ALTER TABLE "model" ALTER COLUMN "name" SET DEFAULT '(empty)';--> statement-breakpoint | ||
| ALTER TABLE "model" ALTER COLUMN "name" SET NOT NULL;--> statement-breakpoint |
There was a problem hiding this comment.
Type mismatch: setting text column to boolean value.
Line 10 sets name to the boolean false instead of a text value. This will either fail or produce unexpected results depending on the database's implicit casting behavior. Given that line 11 sets the default to '(empty)', the backfill should use the same value.
Proposed fix
-update "model" set "name" = false where "name" is null;--> statement-breakpoint
+update "model" set "name" = '(empty)' where "name" is null;--> statement-breakpoint📝 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.
| update "model" set "name" = false where "name" is null;--> statement-breakpoint | |
| ALTER TABLE "model" ALTER COLUMN "name" SET DEFAULT '(empty)';--> statement-breakpoint | |
| ALTER TABLE "model" ALTER COLUMN "name" SET NOT NULL;--> statement-breakpoint | |
| update "model" set "name" = '(empty)' where "name" is null;--> statement-breakpoint | |
| ALTER TABLE "model" ALTER COLUMN "name" SET DEFAULT '(empty)';--> statement-breakpoint | |
| ALTER TABLE "model" ALTER COLUMN "name" SET NOT NULL;--> statement-breakpoint |
🤖 Prompt for AI Agents
In `@packages/db/migrations/1768524767_flowery_garia.sql` around lines 10 - 12,
The UPDATE in the migration incorrectly assigns a boolean to the text column:
change the backfill to set the "model"."name" column to the text default value
'(empty)' for NULLs (i.e., use UPDATE "model" SET "name" = '(empty)' WHERE
"name" IS NULL) so it matches the ALTER TABLE default and avoids type mismatch;
keep the subsequent ALTER TABLE "model" ALTER COLUMN "name" SET DEFAULT
'(empty)' and ALTER COLUMN "name" SET NOT NULL as-is.
| releasedAt: timestamp().defaultNow().notNull(), | ||
| name: text().default("(empty)").notNull(), | ||
| aliases: json().$type<string[]>().default([]).notNull(), | ||
| description: text().default("(empty)").notNull(), | ||
| family: text().notNull(), | ||
| free: boolean(), | ||
| output: json().$type<string[]>(), | ||
| free: boolean().default(false).notNull(), | ||
| output: json().$type<string[]>().default(["text"]).notNull(), | ||
| stability: text({ | ||
| enum: ["stable", "beta", "unstable", "experimental"], | ||
| }) | ||
| .default("stable") | ||
| .notNull(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's verify the file exists and check the relevant lines
find . -name "schema.ts" -path "*/packages/db/*" | head -5Repository: theopenco/llmgateway
Length of output: 91
🏁 Script executed:
# Read the schema file around the specified lines
if [ -f "packages/db/src/schema.ts" ]; then
wc -l packages/db/src/schema.ts
echo "---"
sed -n '695,720p' packages/db/src/schema.ts
fiRepository: theopenco/llmgateway
Length of output: 987
🏁 Script executed:
# Search for how releasedAt is used in the codebase
rg "releasedAt" --type ts --type tsx -B 2 -A 2Repository: theopenco/llmgateway
Length of output: 91
🏁 Script executed:
# Check if there are any migrations or comments about this field
rg "timestamp.*defaultNow" packages/db/ --type ts -B 2 -A 2Repository: theopenco/llmgateway
Length of output: 7676
🏁 Script executed:
# Search for migrations or migration logic
find . -name "*migrat*" -type f | grep -E "\.(ts|js|sql)$" | head -10Repository: theopenco/llmgateway
Length of output: 92
🏁 Script executed:
# Check for model table usage and releasedAt references in app code
rg "releasedAt" --type ts -B 3 -A 3 -iRepository: theopenco/llmgateway
Length of output: 50376
🏁 Script executed:
# Search for the model table definition context - look for migration guidance or comments
sed -n '680,740p' packages/db/src/schema.ts | cat -nRepository: theopenco/llmgateway
Length of output: 2392
🏁 Script executed:
# Check if there's any migration code that handles this field
rg "model.*migration\|migration.*model" --type ts -i -A 5 -B 5Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Check for backfill migration logic or scripts
rg "releasedAt.*backfill\|backfill.*releasedAt" --type ts -iRepository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Look for model sync/import/populate logic
rg "model.*insert\|model.*upsert\|syncModel\|populateModel" --type ts -i -A 3 | head -50Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Check migration files for backfill statements
find . -path "*/migrations/*" -type f | head -10Repository: theopenco/llmgateway
Length of output: 635
🏁 Script executed:
# Search for where model records are created/updated in the database
rg "db\(\).*model.*insert\|db\(\).*model.*update" --type ts -B 2 -A 5 | head -60Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Read the most recent migration files to see if releasedAt was added
for file in $(ls -t packages/db/migrations/*.sql | head -3); do
echo "=== $file ==="
cat "$file"
echo ""
doneRepository: theopenco/llmgateway
Length of output: 2299
🏁 Script executed:
# Search for model sync, seed, or populate logic in apps
rg "models.*sync\|syncModels\|seed.*model\|populateModels" apps/ --type ts -i -B 2 -A 5Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Check the latest migration specifically for model/releasedAt
cat packages/db/migrations/1748526023_supreme_valkyrie.sqlRepository: theopenco/llmgateway
Length of output: 139
🏁 Script executed:
# List migrations chronologically and show the ones after releasedAt was added
ls -1 packages/db/migrations/*.sql | sort -V | tail -5Repository: theopenco/llmgateway
Length of output: 328
🏁 Script executed:
# Check migrations after 1768524767 for any backfill logic
for file in $(ls -1 packages/db/migrations/*.sql | sort -V | awk '/1768524767/,0'); do
if [ "$file" \> "packages/db/migrations/1768524767_flowery_garia.sql" ]; then
echo "=== $(basename $file) ==="
cat "$file" | head -20
echo ""
fi
doneRepository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Search for code that syncs or updates model release dates from the models package
rg "releasedAt\|released_at" apps/ --type ts -B 3 -A 3 | head -80Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Check if there's an API endpoint or service that syncs models from packages/models
rg "packages/models\|from.*models" apps/ --type ts -B 2 -A 5 | head -60Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Search comprehensively for model insert/update operations
rg "\.insert\(|\.update\(" apps/ --type ts -B 5 -A 5 | rg -A 10 -B 10 "model" | head -100Repository: theopenco/llmgateway
Length of output: 6644
🏁 Script executed:
# Check if models are synced from packages/models on startup or elsewhere
rg "export.*model|from.*models|import.*models" apps/ --type ts | head -20Repository: theopenco/llmgateway
Length of output: 2024
🏁 Script executed:
# Look for any INSERT statements in recent migrations that might populate models
find packages/db/migrations -name "*.sql" -exec grep -l "INSERT.*model" {} \;Repository: theopenco/llmgateway
Length of output: 46
🏁 Script executed:
# Read the sync-models.ts file to check releasedAt handling
cat apps/worker/src/services/sync-models.tsRepository: theopenco/llmgateway
Length of output: 11751
Reconsider the releasedAt DEFAULT without guaranteeing sync.
While the migration adds releasedAt with DEFAULT now(), the sync service (apps/worker/src/services/sync-models.ts) extracts and applies actual release dates from the models package on startup. However, the mitigation creates risk: (1) the field is marked notNull in the schema but treated as optional in sync code (|| null), and (2) if sync doesn't run or if models are inserted outside the sync path, rows will incorrectly default to the migration timestamp. Either make the field nullable to reflect its optional treatment, or add a comment documenting that sync is required for correctness.
🤖 Prompt for AI Agents
In `@packages/db/src/schema.ts` around lines 700 - 711, The releasedAt column in
packages/db/src/schema.ts is declared as releasedAt:
timestamp().defaultNow().notNull() but the sync logic in
apps/worker/src/services/sync-models.ts treats it as optional; change the schema
to reflect optionality by removing .notNull() and the defaultNow(), e.g., make
releasedAt nullable with no forced default (or explicit default null) so rows
inserted outside sync won't get an incorrect migration timestamp; update any
dependent code or comments in sync-models.ts to document that sync will populate
release dates when available.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Changes
Backend API
Frontend/Data Layer
UI/UX and Type Safety
Data Layer and Schemas
Testing / Validation
Why
Migration Notes
🌿 Generated by Terry
ℹ️ Tag @terragon-labs to ask questions and address PR feedback
📎 Task: https://www.terragonlabs.com/task/c46df707-f942-4e3e-882c-824c19af0b96
Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.