feat(web): small_model picker in settings — Model roles section - #139
Conversation
Expose config.small_model in the web/desktop settings UI (Settings →
Providers), closing the v2 gap left by the small_model feature PR
(config-file only until now).
- POST /api/small-model sets/clears small_model; validates the provider
exists and that provider/model are set-or-empty together. The handler
mutates the live shared config pointer IN PLACE (browser-config
pattern) so call-time readers — ModelFactory ("small" alias),
automation override, title refiner — see the change immediately; the
providers-style s.cfg reassign would leave buildWebTask's closure on
the stale value until restart. A failed SaveConfig restores the
previous value so memory never advertises a value disk doesn't have.
- GET /api/config now returns small_model.
- Settings → Providers gains a "Model roles" section: enabled models
grouped by provider (native select + optgroup, mirroring the language
row), "not set (follow main model)" clears, save-on-change with
inline saved/failed feedback. A configured ref that is no longer
enabled renders as its raw ref marked unavailable so it can still be
cleared. i18n: en / zh-Hans / zh-Hant / ja / ko.
- Docs: models.md mentions the settings path; UI design doc + hi-fi
mockup rationale under internal-doc/.
Verified: handler unit tests (in-place mutate + persist + GET
round-trip, clear, 4 invalid-request cases leave config untouched,
nil-config 500); go test ./internal/... green; golangci-lint clean;
web tsc clean; live e2e against an isolated-HOME backend through the
vite dev server — enable model → pick as small → config.json persisted
→ full page reload round-trips the selection.
Generated with Jack AI bot
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a small-model configuration API, persistence and validation tests, and a localized Providers settings picker that updates the live configuration without restart. Documentation and changelog entries describe the new role-selection workflow. ChangesSmall model settings
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ProvidersTab
participant SmallModelAPI
participant Config
User->>ProvidersTab: Select small model
ProvidersTab->>SmallModelAPI: POST provider and model
SmallModelAPI->>Config: Update and save SmallModel
Config-->>SmallModelAPI: Save result
SmallModelAPI-->>ProvidersTab: Return configured small_model
ProvidersTab-->>User: Show saved or failed status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/components/SettingsDialog.tsx (1)
462-466: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd defensive check for missing separator.
If the backend configuration file was manually edited and lacks a
/,indexOf('/')returns-1. The subsequentslice(0, -1)would silently strip the last character from the string before passing it as the provider. Adding a fallback ensures malformed inputs are correctly rejected by the API's validation.🛡️ Proposed defensive fix
- // Provider ids never contain '/'; model ids may (custom endpoints). - const i = value.indexOf('/') - provider = value.slice(0, i) - model = value.slice(i + 1) + // Provider ids never contain '/'; model ids may (custom endpoints). + const i = value.indexOf('/') + if (i >= 0) { + provider = value.slice(0, i) + model = value.slice(i + 1) + } else { + provider = '' + model = value + }🤖 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 `@web/src/components/SettingsDialog.tsx` around lines 462 - 466, Update the provider/model parsing logic around indexOf('/') to detect a missing separator before slicing; reject or otherwise pass the malformed value through the existing API validation path instead of deriving provider and model from an index of -1. Preserve the current first-separator parsing behavior for valid values.
🤖 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.
Nitpick comments:
In `@web/src/components/SettingsDialog.tsx`:
- Around line 462-466: Update the provider/model parsing logic around
indexOf('/') to detect a missing separator before slicing; reject or otherwise
pass the malformed value through the existing API validation path instead of
deriving provider and model from an index of -1. Preserve the current
first-separator parsing behavior for valid values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85d644b9-ab0b-436f-affd-da763541d7aa
📒 Files selected for processing (14)
CHANGELOG.mdinternal-doc/small-model-settings-ui-design.mdinternal/web/models.gointernal/web/server.gointernal/web/small_model_test.gosite/docs/overview/models.mdweb/src/app/store.tsweb/src/components/SettingsDialog.tsxweb/src/i18n/locales/en.tsweb/src/i18n/locales/ja.tsweb/src/i18n/locales/ko.tsweb/src/i18n/locales/zh-Hans.tsweb/src/i18n/locales/zh-Hant.tsweb/src/lib/api.ts
A hand-edited config can hold a ref without '/', which surfaces through the "unavailable" select option; indexOf returning -1 made slice(0, -1) chop the last character off the provider name before the API rejected it. Pass such values as model-only so the backend's set-or-empty validation rejects them intact. Review: coderabbit nitpick on PR #139. Generated with Jack AI bot Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed the CodeRabbit nitpick in 30ac116: a separator-less 🤖 Addressed by Claude Code |
1 participant
Summary
config.small_modelin the web/desktop settings UI. Until now the small-model role shipped in feat(model): small_model role — "small" alias, LLM session titles, routing fixes #137 was config-file only — desktop users had no way to set it. Settings → Providers gains a Model roles section with a grouped picker; changes persist and take effect immediately (no restart).Related Issues / Tickets
small_modelrole), which left the settings UI as a recorded v2 gap.Type of Change
Changes Made
POST /api/small-modelsets/clearssmall_model(both fields empty = clear; unknown provider and half-set requests are 400). The handler mutates the live shared config pointer in place (same discipline as the browser-config handler) so call-time readers — the ModelFactory"small"alias, the automation model override, the title refiner — see the change immediately; the providers-styles.cfgreassign would leavebuildWebTask's closure reading the stale value until restart. A failedSaveConfigrestores the previous value so memory never advertises a value disk doesn't have.GET /api/confignow returnssmall_model.select+optgroup, mirroring the existing language-row pattern), "Not set (follow main model)" clears, save-on-change with inline saved/failed feedback. A configured ref whose model was since disabled/removed still renders as its raw ref marked unavailable so it can be seen and cleared. Localized in en / zh-Hans / zh-Hant / ja / ko.site/docs/overview/models.mdmentions the settings path; design rationale (API shape, effect semantics, the in-place-vs-reassign pitfall) recorded ininternal-doc/small-model-settings-ui-design.md.Testing
internal/web/small_model_test.go): in-place mutate + disk persist + GET round-trip; clear; four invalid-request cases leave config untouched; nil-config → 500.go test ./internal/...green,golangci-lint run internal/web/clean,tsc --noEmitclean.config.jsongains"small_model": "zai/glm-4.5-flash"→ full page reload round-trips the selection; set/clear/validation also exercised over raw HTTP.Screenshots / Recordings
The section renders as: ⚡ Small model + purpose description on the left, grouped picker on the right, directly above the provider cards (both light and dark themes verified).
Checklist
Additional Notes
memory.model, and auto-recommending a small model.internal/model/registry_generated.gohas unrelated pending churn in the working tree from a separate task; it is intentionally not part of this PR.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation