feat(models): dynamic, animated model selector with provider logos - #586
Open
beng360 wants to merge 10 commits into
Open
feat(models): dynamic, animated model selector with provider logos#586beng360 wants to merge 10 commits into
beng360 wants to merge 10 commits into
Conversation
Rework the chat model selector so its list is driven by the backend instead of a hardcoded array, and give it a polished UI. Backend: - Add a model_catalog module with display metadata (name, provider, context window, pricing) and an ENABLED_MODELS env allowlist. When unset the full catalog is offered; a deployment can restrict the selector to a subset (e.g. Claude-only via Bedrock) with no code change. A bogus allowlist logs and falls back to the full catalog. - Expose GET /api/llm-config/models (require_auth_only) so the selector renders for every user; the existing /supported-models stays admin-only. - Add claude-opus-4-8, claude-sonnet-5, claude-fable-5 to MODEL_MAPPINGS (dash + dot aliases). The Bedrock provider already derives their inference-profile IDs, verified against the AWS Bedrock API. Frontend: - Rebuild ModelSelector: fetches the live catalog (static fallback if the request fails), groups by provider, framer-motion animations (spring dropdown, staggered rows, animated selection check, chevron rotate). - Vendor official Anthropic/OpenAI/Gemini brand marks from @lobehub/icons-static-svg as inline components (the published package requires React 19 + antd, incompatible with this React 18 app). - Add /api/llm-models Next proxy route and document ENABLED_MODELS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
WalkthroughThis PR adds a backend-served model catalog with provider discovery, routing and allowlist filtering, exposes it through authenticated backend and Next.js routes, and updates the model selector to fetch, group, search, and persist catalog-backed model selections. ChangesConfigurable model catalog and selector UI
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant ModelSelector
participant NextRoute
participant BackendRoute
participant ModelCatalog
participant Providers
User->>ModelSelector: Open model selector
ModelSelector->>NextRoute: Fetch model catalog
NextRoute->>BackendRoute: Forward authenticated request
BackendRoute->>ModelCatalog: Build enabled models
ModelCatalog->>Providers: Discover supported models
Providers-->>ModelCatalog: Return model IDs
ModelCatalog-->>BackendRoute: Return filtered catalog
BackendRoute-->>ModelSelector: Return catalog response
User->>ModelSelector: Search or select model
ModelSelector-->>User: Display grouped model options
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![arvo-ai-staging[bot]](https://avatars.githubusercontent.com/in/4088260?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Around line 163-168: Add ENABLED_MODELS to the shared Compose environment
block so the container receives the allowlist setting. Update the environment
passed by both docker-compose.yaml and docker-compose.prod-local.yml so
server/chat/backend/agent/model_catalog.py can read ENABLED_MODELS and apply the
selector restriction instead of falling back to the full catalog.
In `@client/src/app/api/llm-models/route.ts`:
- Around line 19-22: The llm-models route is duplicating backend proxy behavior
with a direct fetch instead of using the shared proxy helper. Update the route
handler in route.ts to delegate to forwardRequest from
client/src/lib/backend-proxy.ts, preserving the existing headers/auth handling
through that helper and removing the manual BACKEND_URL fetch logic. Use the
route handler and forwardRequest symbols to locate and replace the current proxy
call.
In `@client/src/components/ModelSelector.tsx`:
- Around line 183-229: The custom model option elements in ModelSelector are
plain motion.button nodes inside DropdownMenuContent, so they bypass Radix
dropdown keyboard behavior and expose incomplete ARIA state. Update the model
list rendering to use DropdownMenuItem asChild around the existing motion.button
(or otherwise give the item proper menuitem semantics and selection state) so
arrow-key navigation, focus handling, and screen reader state work correctly.
Make sure the selected model state in handleModelSelect is reflected with an
accessible attribute such as aria-selected or aria-checked, not just the check
icon and background styling.
- Around line 195-214: The `AnimatePresence` block in `ModelSelector` only
animates the `Check` because `ProviderIcon` is a plain component and cannot
participate in exit animations. Wrap the `ProviderIcon` branch in a `motion`
element with the same kind of enter/exit transitions used for `motion.span` on
the check mark, so switching between `isSelected` states animates both
directions smoothly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2946324f-d54c-4198-a750-eec84473b6f6
📒 Files selected for processing (7)
.env.exampleclient/src/app/api/llm-models/route.tsclient/src/components/ModelSelector.tsxclient/src/components/icons/provider-icons.tsxserver/chat/backend/agent/model_catalog.pyserver/chat/backend/agent/model_mapper.pyserver/routes/llm_config.py
…r text, accent hover Remove the generic Zap/Gauge reasoning indicators; each row is now just the provider mark, name, context length, and a check on the selected model. Bump row and trigger text (xs -> sm) for legibility, and switch row hover/selection to the standard bg-accent dropdown-item style instead of a muted wash + x-shift. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire the ENABLED_MODELS allowlist into the compose env (default empty), alongside MAIN_MODEL/RCA_MODEL, so the model selector allowlist works in docker-compose deployments — not just helm. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose.yaml`:
- Line 69: The compose configuration is missing the ENABLED_MODELS environment
entry in the prod-local variant, which should stay aligned with
docker-compose.yaml and .env.example. Update docker-compose.prod-local.yml so
the service definition includes ENABLED_MODELS alongside the other environment
variables, matching the existing compose patterns and keeping the variants in
sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0db8ec1c-c6b1-4453-b8c2-cdbcae06f4e4
📒 Files selected for processing (1)
docker-compose.yaml
Make the selector reach everything Aurora can route, not a curated list: - model_catalog: the offered list is now the UNION of the curated flagship catalog (rich display metadata) and live get_supported_models() from every configured+available provider, de-duped by canonical id. Uncatalogued models (Ollama pulls, OpenRouter long tail) get derived display names. ENABLED_MODELS still narrows the result for restricted deployments. - openrouter_provider: get_supported_models() now fetches OpenRouter's live /models catalog (hundreds of models) with a 10-min cache; falls back to the mapped set when the key is unset or the API is unreachable (no hang, never empty). - ModelSelector: rebuilt on a cmdk command palette (Popover + Command) — type to filter across all providers, grouped, keyboard nav, provider logos with a generic fallback. Plus a custom-model row: typing any provider/model id routes it through (passthrough), so truly any model is reachable. - Add shadcn command.tsx wrapper. Verified: 346 models served with OpenRouter configured (0ms cached), graceful 27-model fallback with no key, search filters instantly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
346 models is a data dump, not a picker. Split reach from display: - Backend tags each model featured=true/false. Featured = the curated flagship catalog + models from direct providers (Ollama pulls, direct Anthropic/OpenAI/Google/Bedrock) — short, high-intent lists. The OpenRouter long tail is featured=false. A FEATURED_MODELS env var overrides the default set per deployment (same shape as ENABLED_MODELS). - Selector shows only featured models when the search box is empty, and widens to the entire catalog the moment you type. A 'Search to browse N more models' hint shows how many are behind search. Custom passthrough entry unchanged. This keeps universal reach (any model, incl. the full OpenRouter set and locally-pulled Ollama models, is selectable) without dumping hundreds of rows into the default view. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
'What you see is what you can use' was not actually true: the selector
listed every available provider's models regardless of LLM_PROVIDER_MODE,
so e.g. in bedrock/vertex/direct mode it showed ~300 OpenRouter models that
would error at call time, and in openrouter mode it showed ollama ids that
don't route there.
Filter the catalog through the SAME router the agent uses
(get_provider_for_model under the env LLM_PROVIDER_MODE); drop anything it
can't resolve. Fails open (allow) if the registry is unavailable so the
selector never empties. Verified per mode:
- openrouter: all 346 (everything routes through OpenRouter)
- direct/bedrock/vertex: ~107 (anthropic/openai/google); OpenRouter-only
providers (qwen/deepseek/mistral/…) dropped; bedrock keeps Claude via
native ids + Gemini via Vertex fallback
- ollama ids only appear in direct mode with Ollama reachable
Adds ~1ms (in-memory registry lookup). Custom passthrough entry still lets
power users type any id.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/chat/backend/agent/providers/openrouter_provider.py`:
- Around line 113-140: Cache the fallback result in
OpenRouterProvider.get_supported_models() when the /models request fails or
returns a non-200 response, instead of only caching successful live results.
Reuse the existing _models_cache and _models_cache_time fields so repeated calls
from get_enabled_models() within a short TTL return the mapped fallback list
without reissuing the network request. Update the failure branches in the
requests.get try/except path to store mapped before returning it, while keeping
the success path unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 67b9fa0a-510d-4bb2-a8d5-61ac0b3142f7
📒 Files selected for processing (4)
client/src/components/ModelSelector.tsxclient/src/components/ui/command.tsxserver/chat/backend/agent/model_catalog.pyserver/chat/backend/agent/providers/openrouter_provider.py
- llm-models route: use shared forwardAuthenticatedGet helper instead of hand-rolled fetch (auth/timeout/error-normalization handled centrally). - openrouter_provider: cache the fallback list for 60s on /models failure so a slow/down OpenRouter doesn't re-incur the 5s timeout on every selector load. - model_catalog: replace empty except in _canonical_id with a debug log. - docker-compose.prod-local.yml: pass ENABLED_MODELS through (sync with docker-compose.yaml). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SonarCloud (11 issues): - provider-icons.tsx: mark props Readonly; drop role='img' (icons are decorative, always beside a text label) — now aria-hidden. Fixes the a11y-role and readonly-props smells. - command.tsx: remove redundant manual cmdk-input-wrapper attr (cmdk sets its own; nothing targets it) — fixes unknown-DOM-property smell. - model_mapper.py: hoist the duplicated claude-opus-4.8 mapping to a single _OPUS_4_8 dict referenced by both alias keys — fixes the duplicated-literal (critical) smell. - llm_config.py: logger.exception() instead of logger.error(exc_info=True). CI: - .env.example: document FEATURED_MODELS (was read in code but missing, failing the Validate Environment Variables drift check). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
…LS alias-safe - Remove the FEATURED_MODELS env override (unused, added drift-check surface). The built-in featured rule is unchanged, so the default view is identical (12 flagships). Also removes it from .env.example. - ENABLED_MODELS now matches on canonical id, so a dot/dash alias (claude-opus-4.8 vs claude-opus-4-8) resolves either way instead of silently dropping the model — important for the prod Claude-only lockdown. Verified: prod env (LLM_PROVIDER_MODE=bedrock + claude-only ENABLED_MODELS, no OpenRouter key) yields exactly the 4 Claude models; full catalog (341) unchanged when no allowlist set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/chat/backend/agent/model_catalog.py (1)
314-380: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAllowlist and featured checks should use canonical ids for dash/dot alias consistency.
get_enabled_models()de-duplicates by_canonical_id()soanthropic/claude-opus-4-8andanthropic/claude-opus-4.8collapse to one entry. However, theENABLED_MODELSfilter (line 370) and theFEATURED_MODELSoverride check (line 332) compare rawm["id"]/entry["id"]against the env-var set. If an operator writesENABLED_MODELS="anthropic/claude-opus-4-8"(dash) but the catalog entry id isanthropic/claude-opus-4.8(dot), the model won't match, the allowlist matches nothing, and the selector falls back to the full list with a warning — even though the model is available and_canonical_idwas designed to treat both forms as the same.Canonicalize both the allowlist/override sets and the id checks so alias forms match consistently.
🔧 Proposed fix
featured_override = _featured_model_ids() + if featured_override is not None: + featured_override = {_canonical_id(a) for a in featured_override} def _mark_featured(entry: Dict, is_featured: bool) -> Dict: out = dict(entry) out["featured"] = ( - entry["id"] in featured_override + _canonical_id(entry["id"]) in featured_override if featured_override is not None else is_featured ) return outallowed = _enabled_model_ids() if allowed is None: return models - filtered = [m for m in models if m["id"] in allowed] + allowed = {_canonical_id(a) for a in allowed} + filtered = [m for m in models if _canonical_id(m["id"]) in allowed] if not filtered:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/chat/backend/agent/model_catalog.py` around lines 314 - 380, `get_enabled_models()` is comparing raw model ids against `ENABLED_MODELS` and `FEATURED_MODELS`, which breaks dash/dot aliases even though the function already deduplicates with `_canonical_id()`. Update the allowlist and featured override logic to normalize both the env-var ids and the candidate ids via `_canonical_id()` so `entry["id"]`, `m["id"]`, and alias forms like `anthropic/claude-opus-4-8` and `anthropic/claude-opus-4.8` match consistently. Keep the change localized to `get_enabled_models()`, `_featured_model_ids()`, and the final allowlist filter so the canonical comparison is used everywhere ids are checked.client/src/components/icons/provider-icons.tsx (1)
62-119: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winGeminiIcon uses fixed gradient IDs, colliding when multiple instances render.
The model selector groups entries by provider and renders
ProviderIconper list item plus the selected model, so multiple Google/Vertex models produce multipleGeminiIconinstances sharing the samelobe-icons-gemini-fill-0/1/2IDs. Duplicate SVG IDs are invalid andurl(#id)resolves to the first definition in document order, which can cause subtle rendering bugs across browsers.Use React 18's
useId()to namespace the gradient IDs per instance.🛠️ Proposed fix using `useId()`
export function GeminiIcon({ className, size = 16 }: Readonly<ProviderIconProps>) { + const gid = useId(); const p = "M20.616 10.835a14.147 14.147 0 01-4.45-3.001 14.111 14.111 0 01-3.678-6.452.503.503 0 00-.975 0 14.134 14.134 0 01-3.679 6.452 14.155 14.155 0 01-4.45 3.001c-.65.28-1.318.505-2.002.678a.502.502 0 000 .975c.684.172 1.35.397 2.002.677a14.147 14.147 0 014.45 3.001 14.112 14.112 0 013.679 6.453.502.502 0 00.975 0c.172-.685.397-1.351.677-2.003a14.145 14.145 0 013.001-4.45 14.113 14.113 0 016.453-3.678.503.503 0 000-.975 13.245 13.245 0 01-2.003-.678z"; return ( <svg width={size} height={size} viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" className={className} aria-hidden > <path d={p} fill="`#3186FF`" /> - <path d={p} fill="url(`#lobe-icons-gemini-fill-0`)" /> - <path d={p} fill="url(`#lobe-icons-gemini-fill-1`)" /> - <path d={p} fill="url(`#lobe-icons-gemini-fill-2`)" /> + <path d={p} fill={`url(#${gid}-0)`} /> + <path d={p} fill={`url(#${gid}-1)`} /> + <path d={p} fill={`url(#${gid}-2)`} /> <defs> <linearGradient gradientUnits="userSpaceOnUse" - id="lobe-icons-gemini-fill-0" + id={`${gid}-0`} x1="7" x2="11" y1="15.5" y2="12" > <stop stopColor="`#08B962`" /> <stop offset="1" stopColor="`#08B962`" stopOpacity="0" /> </linearGradient> <linearGradient gradientUnits="userSpaceOnUse" - id="lobe-icons-gemini-fill-1" + id={`${gid}-1`} x1="8" x2="11.5" y1="5.5" y2="11" > <stop stopColor="`#F94543`" /> <stop offset="1" stopColor="`#F94543`" stopOpacity="0" /> </linearGradient> <linearGradient gradientUnits="userSpaceOnUse" - id="lobe-icons-gemini-fill-2" + id={`${gid}-2`} x1="3.5" x2="17.5" y1="13.5" y2="12" > <stop stopColor="`#FABC12`" /> <stop offset=".46" stopColor="`#FABC12`" stopOpacity="0" /> </linearGradient> </defs> </svg> ); }And add the import at the top of the file:
-import React from 'react' +import React, { useId } from 'react'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/icons/provider-icons.tsx` around lines 62 - 119, GeminiIcon currently uses fixed SVG gradient IDs, so multiple rendered instances can collide and make url(#...) resolve to the wrong definition. Update GeminiIcon in provider-icons.tsx to namespace the three linearGradient ids per render using React 18 useId(), and apply that generated prefix to the matching fill references and defs so each icon instance has unique gradient identifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@client/src/components/icons/provider-icons.tsx`:
- Around line 62-119: GeminiIcon currently uses fixed SVG gradient IDs, so
multiple rendered instances can collide and make url(#...) resolve to the wrong
definition. Update GeminiIcon in provider-icons.tsx to namespace the three
linearGradient ids per render using React 18 useId(), and apply that generated
prefix to the matching fill references and defs so each icon instance has unique
gradient identifiers.
In `@server/chat/backend/agent/model_catalog.py`:
- Around line 314-380: `get_enabled_models()` is comparing raw model ids against
`ENABLED_MODELS` and `FEATURED_MODELS`, which breaks dash/dot aliases even
though the function already deduplicates with `_canonical_id()`. Update the
allowlist and featured override logic to normalize both the env-var ids and the
candidate ids via `_canonical_id()` so `entry["id"]`, `m["id"]`, and alias forms
like `anthropic/claude-opus-4-8` and `anthropic/claude-opus-4.8` match
consistently. Keep the change localized to `get_enabled_models()`,
`_featured_model_ids()`, and the final allowlist filter so the canonical
comparison is used everywhere ids are checked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c921e8d4-b730-44e3-b37d-6b651cc9a8f9
📒 Files selected for processing (9)
.env.exampleclient/src/app/api/llm-models/route.tsclient/src/components/icons/provider-icons.tsxclient/src/components/ui/command.tsxdocker-compose.prod-local.ymlserver/chat/backend/agent/model_catalog.pyserver/chat/backend/agent/model_mapper.pyserver/chat/backend/agent/providers/openrouter_provider.pyserver/routes/llm_config.py
OlivierTrudeau
requested changes
Jul 16, 2026
OlivierTrudeau
left a comment
OlivierTrudeau
left a comment
Contributor
There was a problem hiding this comment.
I still need to test
| // the shared helper (auth, timeouts, error normalization handled there). | ||
| export async function GET(request: NextRequest) { | ||
| return forwardAuthenticatedGet(request, '/api/llm-config/models', 'fetch model catalog') | ||
| } |
Contributor
There was a problem hiding this comment.
Instead of making a backend request which will add trafic for no good reason every time someone opens the chat page, why not just inject the env var in the frontend so no request to the api is done on opening the chat page
Contributor
Author
There was a problem hiding this comment.
the catalog isn't static though — it's computed server-side from ENABLED_MODELS + LLM_PROVIDER_MODE + per-model display metadata, and it's auth-gated, so it can't just be a build-time env var (it'd differ per deployment and leak the allowlist to unauthed users). but you're right the per-open fetch is wasteful — cached it in sessionStorage in 4a4a2ea so it only hits the api once per session instead of every chat open.
| every upstream provider), cached for 10 minutes. Falls back to the | ||
| statically mapped set if the API is unreachable or the key is unset, so | ||
| the selector always has something to show. | ||
| """ |
Contributor
There was a problem hiding this comment.
Great but a lot don't work for X or Y reason. When this happens, we should print a friendly error message on the frontend at least.
Contributor
Author
There was a problem hiding this comment.
agreed. done in 4a4a2ea — workflow errors now detect model/provider failures (unsupported, gated, down, 4xx) and show 'the selected model isn't available or rejected this request — try a different model' instead of the generic 'try again'. the frontend already renders backend error messages as
| VISUALIZATION_ENABLED: ${VISUALIZATION_ENABLED:-false} | ||
| RCA_MODEL: ${RCA_MODEL} | ||
| MAIN_MODEL: ${MAIN_MODEL} | ||
| ENABLED_MODELS: ${ENABLED_MODELS:-} |
Contributor
There was a problem hiding this comment.
Forgot to add in the deployment yamls
| "displayName": display or name, | ||
| "provider": provider, | ||
| "tier": "pro", | ||
| "contextLength": "", |
Contributor
There was a problem hiding this comment.
Does it make sense to have a tier if we don't restrict by tier
| "tier": "free", | ||
| "contextLength": "1M", | ||
| "hasReasoning": True, | ||
| "pricing": "Low Cost ($0.30/$2.50 per 1M)", |
Contributor
There was a problem hiding this comment.
I don't think pricing and reasoning are ever used if ever you want to clean up a bit
Addresses review feedback:
- ModelSelector caches the catalog in sessionStorage so it doesn't refetch on
every chat-page open (the catalog only changes with deployment config). The
endpoint stays a dynamic, auth-gated API (honors ENABLED_MODELS/provider mode/
display metadata) — not something injectable as a build-time env var.
- Workflow errors now map common model/provider failures to a friendly,
actionable message ('model unavailable/rejected — try a different model')
instead of the generic 'try again', which doesn't help when the selected
model is the problem.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/main_chatbot.py`:
- Around line 341-347: Update the model-error classification logic around the
model_signals tuple and its any check to remove generic HTTP/resource indicators
such as “400”, “404”, and “not found”, retaining only model- or
provider-specific signals. Add tests covering unrelated missing-resource and
HTTP failures to ensure they are not classified as model failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c31619a-c67d-4c24-9ad9-f2a4c64b8815
📒 Files selected for processing (4)
.env.exampleclient/src/components/ModelSelector.tsxserver/chat/backend/agent/model_catalog.pyserver/main_chatbot.py
💤 Files with no reviewable changes (1)
- .env.example
Comment on lines
+341
to
+347
| text = str(e).lower() | ||
| model_signals = ( | ||
| "model", "not a valid", "not supported", "no endpoints", | ||
| "does not exist", "invalid model", "provider", "openrouter", | ||
| "unsupported", "not found", "404", "400", | ||
| ) | ||
| if any(s in text for s in model_signals): |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid classifying unrelated HTTP errors as model failures.
Signals such as "400", "404", and "not found" can occur in unrelated workflow errors (for example, a missing incident), causing the UI to incorrectly tell users to switch models. Restrict matching to model/provider-specific errors and add tests for unrelated resource and HTTP failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/main_chatbot.py` around lines 341 - 347, Update the model-error
classification logic around the model_signals tuple and its any check to remove
generic HTTP/resource indicators such as “400”, “404”, and “not found”,
retaining only model- or provider-specific signals. Add tests covering unrelated
missing-resource and HTTP failures to ensure they are not classified as model
failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Reworks the chat model selector so its list is driven by the backend rather than a hardcoded array, adds an env-configurable allowlist, registers the newest Claude models, and gives the selector a polished UI with provider logos and animations.
Why
The selector hardcoded its model list in the component, which drifted from what the backend actually served and couldn't be restricted per-deployment. Downstream builds (e.g. a Bedrock-only SaaS deployment that should expose Claude models only) had no way to narrow the list without editing code.
Backend
model_catalog.py— new module holding display metadata (name, provider, context window, pricing) plus anENABLED_MODELSenv allowlist. Unset → full catalog (default OSS behavior). Set → only those IDs. A bogus/empty-matching allowlist logs a warning and falls back to the full catalog so the selector is never empty.GET /api/llm-config/models— newrequire_auth_onlyendpoint returning the catalog with metadata. The existing/supported-modelsis admin-only (llm_config read) and returns bare IDs, so it can't back a selector that renders for every user.MODEL_MAPPINGS— addsanthropic/claude-opus-4-8,claude-sonnet-5,claude-fable-5(dash + dot aliases). The Bedrock provider already derives their inference-profile IDs automatically (bare →us.anthropic.claude-<name>); the derivation was checked against the live AWS Bedrocklist-inference-profilesAPI for all catalogued Claude models.Frontend
ModelSelector.tsxrebuilt: fetches the live catalog (static fallback if the request fails), groups models by provider, and adds framer-motion animations — spring dropdown open, staggered row fade-in, animated selection check, chevron rotate, hover lift.provider-icons.tsx— official Anthropic (Claude), OpenAI, and Gemini brand marks, vendored inline from@lobehub/icons-static-svg(MIT). Inlined rather than added as a dependency because the published@lobehub/iconsrequires React 19 + antd, which conflicts with this app's React 18 stack. Path data is upstream verbatim, not redrawn./api/llm-modelsNext.js proxy route;ENABLED_MODELSdocumented in.env.example.Config
Restrict the selector for a deployment:
Testing
tsc --noEmitclean for all new/changed TS files (repo has pre-existing unrelated TS errors and a broken eslint config, both untouched here).🤖 Generated with Claude Code
Summary by CodeRabbit
provider/modelentries).ENABLED_MODELSconfiguration to control which models appear in the selector (allowlist).