feat(site): add searchable agent model picker - #26927
Conversation
|
/coder-agents-review |
|
Chat: Review posted | View chat Review historydeep-review v0.9.0 | Round 2 | Last posted: Round 2, 10 findings (3 P3, 3 Nit, 4 Note), COMMENT. Review Finding inventoryFindings
Contested and acknowledgedCRF-4 (P3, ModelSelector.tsx:1) - Hand-assembles Popover+Command pattern that Combobox.tsx wraps
CRF-5 (Nit, ModelSelector.tsx:159) - Search placeholder "Search..."
CRF-7 (Note, ModelSelector.tsx:50) - Context limit label
CRF-9 (Note, ModelSelector.tsx:49) - formatContextLimit duplication
Round logRound 1Panel. 3 P3, 2 Nit, 4 Note. 13 reviewers (Bisky, Hisoka, Mafu-san, Mafuuu, Pariston, Ging-TS, Ging-React, Nami, Robin, Gon, Leorio, Komugi, Meruem). Reviewed against 88329db..bbd4f8c. Round 2Panel. 5 fixed (verified), 3 contested (panel closed 3/3), 1 acknowledged. 1 new Nit. 6 reviewers (Bisky, Hisoka, Mafu-san, Mafuuu, Pariston, Meruem). Reviewed against 88329db..0435afb. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean pattern adoption. The migration from Radix Select to Popover+Command is well-scoped, proportional, and follows the established codebase pattern from AI Settings. The API surface is preserved, existing Storybook stories remain compatible (ARIA roles match), and the new search behavior works correctly. Hisoka traced the controlled/uncontrolled state, CSS specificity, re-selection behavior, and mobile keyboard implications without finding issues.
Severity count: 3 P3, 2 Nit, 4 Note.
Pariston on the approach: "I tried to build a case for [alternative framings] and could not."
Commit bodies are empty for a 172-line component swap. The subject is good, but git blame readers won't know why Select was replaced or that this adopts the AI Settings Command pattern.
🤖 This review was automatically generated with Coder Agents.
I am not going to rewrite the pushed draft-PR history just to add bodies to existing commits. The PR body already captures the rationale for replacing |
|
/coder-agents-review |
There was a problem hiding this comment.
R1 fixes all verified correct. Panel unanimously closes all three contested findings (CRF-4, CRF-5, CRF-7).
CRF-4 (Combobox reuse): Pariston found three other components in the codebase already hand-assemble Popover+Command for the same reason (grouped headings, non-toggle select). Mafuuu and Meruem confirmed the structural gaps in Combobox are real. The Combobox consolidation is future work, not a defect in this PR. This needs a human decision: file a ticket for the consolidation or accept the parallel implementations.
CRF-5 (placeholder) and CRF-7 (context limit label): Figma design is the authority on visual copy. The accessible name and contextual placement are adequate.
One new Nit from Netero on a stale comment in a non-diff file.
Mafu-san: "Contested defenses cite concrete, verifiable sources: Figma spec, existing AI Settings Command pattern, 10+ Combobox consumers. Each defense engages with the substance of the finding rather than dismissing it."
site/src/pages/AgentsPage/components/ContextUsageIndicator.tsx:589
Nit [CRF-10] Comment references ModelSelector's tooltip pattern, which this PR removed.
// nested tooltips to the right (same pattern as ModelSelector).
ModelSelector no longer uses tooltips; it was rewritten to use a Popover+Command combobox. The parenthetical "(same pattern as ModelSelector)" is now factually incorrect. Remove the parenthetical or update the reference.
(Netero)
🤖
🤖 This review was automatically generated with Coder Agents.
For this PR, I am accepting the parallel implementation. Combobox consolidation is broader than this Figma-driven picker change because the shared component does not currently support grouped headings plus required non-toggle selection. If we want to consolidate the existing Popover+Command variants, that should be scoped as a separate follow-up. |
Good catch. I removed the stale ModelSelector reference from the ContextUsageIndicator comment. Fixed in |
3 participants
Updates the AgentsPage model picker to use the searchable popover command pattern from AI Settings while preserving the existing
ModelSelectorAPI.The picker now groups results by provider, filters by provider, model name, model ID, and context size, and uses compact AgentsPage-specific typography and spacing for the chat composer.