Repository navigation
revert(web): revert web search button fix - #2221
Merged
Merged
Conversation
… capabil…" This reverts commit b4438d2.
Contributor
WalkthroughThe PR refactors model capability detection by moving reasoning and web-search support checks from stored object fields to a mappings-based lookup system. The type contract removes these fields, the model mapping function stops generating them, and the chat page implements a new lookup pattern using ChangesCapability Lookup Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
smakosh
changed the title
![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: 1
🧹 Nitpick comments (1)
apps/playground/src/lib/mapmodels.ts (1)
42-50: ⚡ Quick winRemove speculative/stale inline notes around ID construction.
The block at Line 42–Line 50 reads like review discussion and contradicts the final implementation at Line 53, which makes future maintenance harder. Please replace it with a short factual comment (or remove it).
As per coding guidelines, "No unnecessary code comments".
🤖 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/lib/mapmodels.ts` around lines 42 - 50, Remove the speculative/stale discussion block about ID construction in mapModels and replace it with a short factual comment: state that the function constructs IDs using provider-specific p.modelName (not Gateway m.id) to preserve current behavior and compatibility with ModelSelector expectations, or omit the comment entirely; update references to the symbols mapModels, p.modelName, and m.id so the remaining comment clearly reflects the actual implementation without the historical debate.
🤖 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/playground/src/components/playground/chat-page-client.tsx`:
- Around line 368-370: selectedModel is being split with
selectedModel.split("/") and destructured into [providerId, modelId], which
truncates modelId when it contains additional slashes; update parsing to split
only on the first slash by finding the first index (e.g., idx =
selectedModel.indexOf("/")) and set providerId = selectedModel.slice(0, idx) and
modelId = selectedModel.slice(idx + 1) (falling back to "" or selectedModel when
no slash). Apply this fix for every occurrence where you currently do const
[providerId, modelId] = selectedModel.includes("/") ? (selectedModel.split("/")
as [string, string]) : ["", selectedModel]; (the selectedModel parsing sites in
chat-page-client.tsx) so capability detection uses the full remainder of the
modelId.
---
Nitpick comments:
In `@apps/playground/src/lib/mapmodels.ts`:
- Around line 42-50: Remove the speculative/stale discussion block about ID
construction in mapModels and replace it with a short factual comment: state
that the function constructs IDs using provider-specific p.modelName (not
Gateway m.id) to preserve current behavior and compatibility with ModelSelector
expectations, or omit the comment entirely; update references to the symbols
mapModels, p.modelName, and m.id so the remaining comment clearly reflects the
actual implementation without the historical debate.
🪄 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: dc9eb69d-1b9b-4416-a03e-ffb546147b7d
📒 Files selected for processing (3)
apps/playground/src/components/playground/chat-page-client.tsxapps/playground/src/lib/mapmodels.tsapps/playground/src/lib/types.ts
💤 Files with no reviewable changes (1)
- apps/playground/src/lib/types.ts
Comment on lines
+368
to
+370
| const [providerId, modelId] = selectedModel.includes("/") | ||
| ? (selectedModel.split("/") as [string, string]) | ||
| : ["", selectedModel]; |
Contributor
There was a problem hiding this comment.
Fix selected-model parsing to handle model IDs that contain /.
At Line 368/388 (and mirrored at Line 1341/1361), selectedModel.split("/") is destructured as [providerId, modelId]. If modelId contains additional slashes, modelId is truncated, so capability detection can silently fail.
Use first-slash parsing instead (providerId = prefix, modelId = rest).
Suggested patch
-const [providerId, modelId] = selectedModel.includes("/")
- ? (selectedModel.split("/") as [string, string])
- : ["", selectedModel];
+const slash = selectedModel.indexOf("/");
+const providerId = slash === -1 ? "" : selectedModel.slice(0, slash);
+const modelId = slash === -1 ? selectedModel : selectedModel.slice(slash + 1);Also applies to: 388-390, 1341-1343, 1361-1363
🤖 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
368 - 370, selectedModel is being split with selectedModel.split("/") and
destructured into [providerId, modelId], which truncates modelId when it
contains additional slashes; update parsing to split only on the first slash by
finding the first index (e.g., idx = selectedModel.indexOf("/")) and set
providerId = selectedModel.slice(0, idx) and modelId = selectedModel.slice(idx +
1) (falling back to "" or selectedModel when no slash). Apply this fix for every
occurrence where you currently do const [providerId, modelId] =
selectedModel.includes("/") ? (selectedModel.split("/") as [string, string]) :
["", selectedModel]; (the selectedModel parsing sites in chat-page-client.tsx)
so capability detection uses the full remainder of the modelId.
github-actions
Bot
changed the title
smakosh
enabled auto-merge
May 9, 2026 10:05
github-actions
Bot
changed the title
smakosh
added this pull request to the merge queue
May 9, 2026
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
1 participant
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.
Reverts #2216
Summary by CodeRabbit