Repository navigation
chore: enhanced model selector and improved filter UI - #2304
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR adds provider-key-aware model filtering to the playground's model selector, persists selected models in localStorage across chat/image/video pages, exposes an API endpoint to list active provider keys, and refines validation to exclude embedding-capable models. ChangesProvider-Key-Aware Model Selection
Model Selection Persistence Across Pages
Validation Model Selection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/playground/src/components/playground/chat-page-client.tsx (1)
276-290:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse lazy initialization and add error handling for localStorage access.
useState(getInitialModel())executesgetInitialModelon every render; use a function initializer instead. Also, bothlocalStorage.getItemandlocalStorage.setItemcalls lack error handling and can throw in restricted browser contexts (e.g., private browsing mode).Proposed fix
- const [selectedModel, setSelectedModel] = useState(getInitialModel()); + const [selectedModel, setSelectedModel] = useState(() => getInitialModel());const getInitialModel = () => { const modelFromUrl = searchParams.get("model"); if (modelFromUrl) { return modelFromUrl; } - if (typeof window !== "undefined") { - const stored = localStorage.getItem(CHAT_MODEL_KEY); - if (stored) { - return stored; - } + if (typeof window !== "undefined") { + try { + const stored = localStorage.getItem(CHAT_MODEL_KEY); + if (stored) { + return stored; + } + } catch { + // ignore storage access failures + } } return "auto"; };useEffect(() => { - if (selectedModel) { - localStorage.setItem(CHAT_MODEL_KEY, selectedModel); - } + if (!selectedModel) { + return; + } + try { + localStorage.setItem(CHAT_MODEL_KEY, selectedModel); + } catch { + // ignore storage access failures + } }, [selectedModel]);Also applies to: 1345–1349
🤖 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 `@apps/playground/src/components/playground/chat-page-client.tsx` around lines 276 - 290, Replace the eager call to getInitialModel in useState with a lazy initializer and wrap all localStorage accesses in try/catch (and guard with typeof window !== "undefined" and window.localStorage) to avoid exceptions in restricted browsers; specifically update the useState(initializer) for selectedModel to use a function that calls getInitialModel lazily, and modify getInitialModel (and any localStorage.setItem/getItem usages such as those referencing CHAT_MODEL_KEY) to catch/storage errors and fall back to "auto" (or silently ignore writes) while returning a safe default; apply the same pattern to the other occurrence referenced in the comment.
🤖 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 `@apps/api/src/routes/keys-provider.ts`:
- Around line 391-400: The DB query that populates providerKeys via
db.query.providerKey.findMany is missing a filter on the status field, so
inactive/deleted keys are returned; update the where clause to include a status
equality filter (e.g., status: 'ACTIVE' or the project's active enum value) so
only active provider keys are fetched, keeping provider and status in columns as
before.
In `@apps/playground/src/components/model-selector.tsx`:
- Around line 1077-1085: The root-entry filtering for filters.showOnlyWithKeys
currently checks all e.model.mappings when deciding visibility, which can
include deactivated mappings; update the filter to only consider active/usable
mappings the same way non-root entries do: inside the list.filter callback for
entries where e.isRoot, change the predicate to check e.model.mappings for an
active mapping (e.g., a mapping property like isActive/active flag or the same
presence test used for e.mapping) and then confirm
providersWithKeys.has(mapping.providerId) for that active mapping; keep the
existing non-root branch that uses e.mapping and providersWithKeys unchanged so
both root and non-root use consistent active-mapping logic (reference:
filters.showOnlyWithKeys, e.isRoot, e.model.mappings, providersWithKeys,
e.mapping).
In `@apps/playground/src/components/playground/image-page-client.tsx`:
- Around line 90-98: Wrap all localStorage accesses in a try-catch to avoid
throwing in privacy/restricted contexts: around the block that reads
IMAGE_MODEL_KEY (the current if (typeof window !== "undefined") { const stored =
localStorage.getItem(IMAGE_MODEL_KEY); ... } ) and the other read/write spots
referenced (lines ~219-223), catch any errors and fall back to a safe default
(e.g., return [] or ignore the write). Ensure you still check typeof window !==
"undefined" but perform getItem/setItem inside try-catch so failures in
private/iframe contexts degrade gracefully.
In `@apps/playground/src/components/playground/video-page-client.tsx`:
- Around line 101-109: The new localStorage access for VIDEO_MODEL_KEY needs the
same defensive try/catch used for VIDEO_GALLERY_KEY: wrap the retrieval (the
getItem + split/filter logic that builds `models`) and any saves (setItem where
models are written back) in try/catch blocks, returning a safe default (empty
array) on error and logging or silently ignoring quota/private-mode errors;
update the code paths that read VIDEO_MODEL_KEY and that persist it (the
model-loading logic that returns `models` and the model-saving code) to use
these guarded storage operations.
---
Outside diff comments:
In `@apps/playground/src/components/playground/chat-page-client.tsx`:
- Around line 276-290: Replace the eager call to getInitialModel in useState
with a lazy initializer and wrap all localStorage accesses in try/catch (and
guard with typeof window !== "undefined" and window.localStorage) to avoid
exceptions in restricted browsers; specifically update the useState(initializer)
for selectedModel to use a function that calls getInitialModel lazily, and
modify getInitialModel (and any localStorage.setItem/getItem usages such as
those referencing CHAT_MODEL_KEY) to catch/storage errors and fall back to
"auto" (or silently ignore writes) while returning a safe default; apply the
same pattern to the other occurrence referenced in the comment.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 40b22295-d171-43ef-ad00-eda4923db6cc
⛔ Files ignored due to path filters (4)
apps/code/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.tsee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (6)
apps/api/src/routes/keys-provider.tsapps/playground/src/components/model-selector.tsxapps/playground/src/components/playground/chat-page-client.tsxapps/playground/src/components/playground/image-page-client.tsxapps/playground/src/components/playground/video-page-client.tsxpackages/actions/src/validate-provider-key.ts
Summary by CodeRabbit
New Features
Improvements