feat(dashboard): add OpenRouter model autocomplete to model selection#917
feat(dashboard): add OpenRouter model autocomplete to model selection#917
Conversation
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Clean, well-structured feature with good architectural decisions (server-side caching, graceful degradation, backward-compatible ModelField changes). CI is green and the implementation matches the PR description. Two should-fix items worth addressing.
Code Issues
Should Fix
-
src/openrouter/client.ts:12,65 — The cache is a single global entry (
let cache: CacheEntry | null) that ignores theapiKeyparameter. If project A has an API key (potentially returning different/more models) and project B has none, whichever project fills the cache first determines what all projects see for the next hour. In practice the OpenRouter/modelsendpoint returns the same public list regardless of API key, so this is unlikely to cause visible bugs today — but it's a latent correctness issue if OpenRouter ever returns key-specific model lists (e.g., fine-tuned models). Consider keying the cache by a hash of the API key, or document the assumption explicitly. -
tests/unit/web/model-field.test.ts — The test file re-implements
stripPrefix,addPrefix,formatPrice,formatContext, andmodelGroupas local copies and tests those rather than importing fromopenrouter-model-combobox.tsx. If the production implementations drift from these copies, tests will pass while production code is broken. Extract these utilities to a shared module (e.g.,openrouter-utils.ts) and import in both the component and the tests.
Minor / Dead Code
- web/src/components/settings/openrouter-model-combobox.tsx:11 —
stripPrefixis defined but never used. Can be removed. - web/src/components/settings/openrouter-model-combobox.tsx:100-108 —
handleChangehas a dead conditional: both theifandelsebranches executeonChange(newValue). The function could be simplified to justonChange.
🕵️ claude-code · claude-opus-4-6 · run details
Inline review commentsSince the review tool had issues with inline comments, here are the per-file details: src/openrouter/client.ts:12 — [SHOULD_FIX] The cache is a single global entry that ignores the In practice the tests/unit/web/model-field.test.ts:7 — [SHOULD_FIX] These utility functions ( Consider extracting these utilities to a shared module (e.g., web/src/components/settings/openrouter-model-combobox.tsx:11 — [MINOR] web/src/components/settings/openrouter-model-combobox.tsx:100 — [MINOR] Dead conditional — both branches of the 🕵️ claude-code · claude-opus-4-6 · run details |
- Key cache by API key (or '__public__') to prevent cross-project cache pollution if OpenRouter ever returns key-specific model lists - Extract prefix/formatting/grouping utilities to web/src/lib/openrouter-utils.ts so tests import production implementations directly instead of local copies - Remove unused stripPrefix from openrouter-model-combobox.tsx - Simplify dead handleChange conditional to a direct onChange pass-through Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
LGTM — well-structured feature addition with clean architecture, proper error handling, and comprehensive test coverage.
The implementation correctly layers: generic Combobox UI primitive → OpenRouterModelCombobox wrapper with tRPC data fetching → ModelField integration preserving backward compatibility for engines with select-type model selection (Claude Code, Codex). The openrouter: prefix convention is well-established throughout the backend and the combobox correctly applies it for catalog selections while allowing custom model IDs.
Key things verified:
ModelFieldordering: TheengineDefinition?.modelSelection.type === 'select'check correctly takes precedence before the OpenRouter combobox, ensuring Claude Code and Codex engines still render their fixed dropdown menus- Backward compatibility: The
AgentConfigFormDialog(which lacks project context) omitsprojectId, soModelFieldfalls back to plain<Input>as before - Server-side caching: Per-API-key cache with 1-hour TTL is reasonable; the
Mapwon't grow unbounded in practice since the number of distinct project API keys is small - Graceful degradation chain: OpenRouter API failure → empty model list → combobox still allows custom typing; tRPC query error → falls back to plain
<Input> - Type sharing:
web/tsconfig.jsonincluding../src/openrouter/**/*follows the existing pattern for../src/api/**/*and../src/db/**/* - CI: All 5 checks pass (Docker builds, lint-and-test, integration-tests, validate-commits)
🕵️ claude-code · claude-opus-4-6 · run details
Summary
Adds an OpenRouter model autocomplete combobox to the dashboard's model selection fields, replacing free-text inputs for LLMist/OpenCode engines with a searchable, grouped list of 300+ models with pricing and context length details.
Card: https://trello.com/c/69b83d19683cb1ab8bc0e632
src/openrouter/client.ts,src/openrouter/types.ts) — proxieshttps://openrouter.ai/api/v1/modelswith 1-hour in-memory cache, filters text-capable models, maps to minimal shape with per-million pricingprojects.openRouterModels) — resolvesOPENROUTER_API_KEYper project, serves cached model list; falls back to empty array on errorweb/src/components/ui/combobox.tsx) — reusable Radix Popover + cmdk component with option grouping, pricing detail display, and custom model ID entryweb/src/components/settings/openrouter-model-combobox.tsx) — fetches live model catalog, groups by provider (Anthropic, Google, DeepSeek, etc.), handlesopenrouter:prefix convention, falls back to plain<Input>on query errorprojectIdprop; renders<OpenRouterModelCombobox>when provided, falls back to plain input otherwise (preserves backward compat for the AgentConfigFormDialog which has no project context)project-harness-form.tsx,project-agent-configs.tsxnow passprojectId;project-general-form.tsxuses combobox for the progress model fieldTest plan
Key decisions
allowCustom=true: Users can still type any model ID not in the OpenRouter catalog (e.g. local or other provider models)<Input>openrouter:prefix distinguishes OpenRouter models from direct model IDs, matching the existing backend convention🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details