Skip to content

fix(web): re-selecting the current model keeps its options - #16572

Open
f4llenz wants to merge 2 commits into
pingdotgg:mainfrom
f4llenz:fix/web-repick-current-model-noop
Open

f4llenz wants to merge 2 commits into
pingdotgg:mainfrom
f4llenz:fix/web-repick-current-model-noop

Conversation

@f4llenz

@f4llenz f4llenz commented Oct 6, 2026

Copy link
Copy Markdown

Problem

Choosing the model that is already selected is not a no-op on web. In the composer, pressing Enter on the current model or using a model jump shortcut replaces the thread's options with that model's remembered options from another thread, and marks them as an explicit choice. The label still names the same model, so the effort and context window change without the user noticing. On a Claude thread with background work, the next send then counts as a setting change, and the server refuses it (#14726). Settings › General › Model and the text generation model picker have the same problem: re-selecting the current default drops its saved traits.

Change

resolveModelPick in packages/shared/src/model.ts builds the selection a pick produces. It returns null when the pick names the current instance and model. The callers handle that result:

  • ChatView.onProviderModelSelect compares the pick with the composer's current selection. On a match it only refocuses the composer. It writes no draft, no explicit flag, and no remembered options. Mobile already behaves this way (ThreadComposer.tsx).
  • The default model picker in ProjectDefaultsSettings and the text generation picker in SettingsPanels keep the current selection, with its traits, on a re-pick. A re-pick still saves the selection, so it still pins an automatic default and unifies a mixed scope as before.

Picking a different model is unchanged. The guard lives in the callers and not in the shared picker, because the picker does not know the current options and must keep multi-select and mixed-scope picks working.

Scope and approval

This is a small, focused fix for an obvious bug under the prior-approval exception. Re-selecting what is already selected should not change anything, and mobile already treats it as a no-op. Opus and Sol found the bug while triaging #16406 and listed it in this comment. It adds no settings, contracts, or server changes.

Verification

Environment. macOS 26, vp run dev from this branch with fresh worktree state, headless Chromium at 1280 × 800, and a synthetic demo project. For "before", I swapped in the commit-1 version of packages/shared/src/model.ts, which has the same routing without the guard.

Composer. Thread A runs Claude Sonnet 5.5 at its defaults, High · 200k. In another thread I set Sonnet 5.5 to Low · 1M, so the remembered options are Low · 1M. Back in thread A, I open the picker, search "sonnet 5.5", and press Enter on the model that is already selected.

Re-selecting Claude Sonnet 5.5 while the thread runs High · 200k

Before After
Before: the composer switched to Low · 1M After: the composer stays High · 200k
Switched to Low · 1M Stays High · 200k

Settings › General › Model. The default is Claude Sonnet 5.5 · Low · 1M. I re-select Claude Sonnet 5.5 in the picker and press Enter.

Settings default model: Claude Sonnet 5.5 · Low · 1M

Before After
Before: the default falls back to High · 200k After: the default stays Low · 1M
Traits dropped, saved as {"model":"claude-sonnet-5-5"} Stays Low · 1M

Tests.

  • packages/shared/src/model.test.ts adds two resolveModelPick cases. Commit 2cbf6df782 routes the callers through the helper without the guard. Its test fails with expected null and receives { instanceId: "claudeAgent", model: "claude-opus-5-5", options: [{ id: "effort", value: "high" }] }. With the fix, 28/28 pass.
  • vp test run on ChatView.logic.test.ts, ModelPickerContent.test.ts, ProviderModelPicker.test.tsx, composerDraftStore.test.ts, modelSelection.test.ts and SettingsPanels.logic.test.ts passes, 353 tests. tsc --noEmit passes for apps/web.

I did not check a mouse click on the current row in a client. Base UI's combobox source calls onValueChange for it without an equality check, so it takes the same path. I also did not check the text generation picker in a client. Its code path is the same as the default model picker's.

Investigated by Claude Opus 5.5, cross-checked by GPT-6.1 Sol, and implemented by Claude Opus 5.5 in Claude Code running inside T3 Code.

The composer and the default and text generation model settings now build
the picked selection through one helper, resolveModelPick, which still
behaves as before. The new test shows the bug: picking the model that is
already selected replaces its options with the remembered ones (or drops
them), so Opus 5.5 at Medium becomes Opus 5.5 at High with no visible
model change.

The "changes nothing when the current model is picked again" case fails
until the fix lands in the next commit.
Picking the model that is already selected, with Enter, a model jump
shortcut, or a click on its row, reached the selection handlers like a real
switch. The composer swapped the thread's effort and context for that
model's remembered options and marked the draft explicit, so the next send
could run at a setting the user never chose. On a Claude thread with
background work, that send was refused as a setting change. The default
and text generation model settings dropped the saved traits the same way.

resolveModelPick now returns null when the pick names the current instance
and model. The composer treats that as a no-op, matching mobile. Settings
write the current selection back unchanged, so re-picking still pins an
automatic default or unifies a mixed scope without losing traits.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 6, 2026
@coderabbitai

coderabbitai Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f8aa2424-f07a-4429-b45e-d8258d0cb089
📥 Commits

Reviewing files that changed from the base of the PR and between 1eae9c2 and 8495fe6.

📒 Files selected for processing (5)
  • apps/web/src/components/ChatView.tsx
  • apps/web/src/components/settings/ProjectDefaultsSettings.tsx
  • apps/web/src/components/settings/SettingsPanels.tsx
  • packages/shared/src/model.test.ts
  • packages/shared/src/model.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Added a shared helper to resolve model picks. Chat and settings now use it when selecting models, and tests cover its behavior for repeated picks and provider options.

Changes

Model selection resolution

Layer / File(s) Summary
Resolver behavior and tests
packages/shared/src/model.ts, packages/shared/src/model.test.ts
resolveModelPick returns null when the current selection matches the requested instance and model. Otherwise, it creates a selection with the supplied options. Tests cover same-provider options and picks for another provider or without a current selection.
Chat and settings integrations
apps/web/src/components/ChatView.tsx, apps/web/src/components/settings/ProjectDefaultsSettings.tsx, apps/web/src/components/settings/SettingsPanels.tsx
Chat uses the resolver and restores composer focus when it returns no selection. Settings use the resolver when saving a project default or selecting a text-generation model.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 8495f

Re-selecting the current model should now keep its saved options in the chat composer and the settings pickers. No merge-blocking risk was found. The author did not manually click-test the current row or the text-generation picker in a client.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main fix: re-selecting the current model preserves its options.
Description check ✅ Passed The description covers the problem, change, scope and approval basis, and focused verification. It also states the client checks that were not performed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant