fix(coding-agent): resolve thinking level overrides in temporary model switcher#5291
fix(coding-agent): resolve thinking level overrides in temporary model switcher#5291PGupta-Git wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c28eed36e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Iterating only MODEL_ROLE_IDS missed any user-defined roles (e.g. modelRoles.review, modelRoles.coding) because that constant only lists the built-in set. Switch to settings.getModelRoles() which returns every configured role string, built-in and custom alike, making the temporary-switcher override lookup complete. Addresses review feedback on can1357#5291.
|
Good catch @chatgpt-codex-connector. Fixed in e5624e0 — replaced the |
|
To use Codex here, create a Codex account and connect to github. |
roboomp
left a comment
roboomp
left a comment
There was a problem hiding this comment.
P3 — superseded by merged PR #5297 (a57244830), which fixes the same temporary-picker path in AgentSession and adds regression coverage.
This implementation also scans only built-in MODEL_ROLE_IDS, leaving custom configured roles unfixed; see the inline finding.
Thanks for the focused report and patch.
| const matchPreferences = getModelMatchPreferences(this.ctx.settings); | ||
| let resolvedThinkingLevel: ConfiguredThinkingLevel | undefined; | ||
|
|
||
| for (const [, roleModelStr] of Object.entries(this.ctx.settings.getModelRoles())) { |
There was a problem hiding this comment.
blocking: MODEL_ROLE_IDS contains only the ten built-in roles, while Settings.getModelRoles() may contain custom roles and getKnownRoleIds(settings) is the existing enumeration that includes them. A model assigned only as modelRoles.critic: …:high therefore never reaches this loop, so the temporary picker still falls back to the model default. Please resolve across the known/configured roles rather than the static built-in list; merged PR #5297 does this in AgentSession.resolveTemporaryModelThinkingLevel().
2 participants
What
config.ymlwhen picking a model temporarily (e.g.Alt+Por/switch).docs/directory ingenerate-docs-index.tsso the build/bundle works outside the monorepo context.Why
Fixes #5290. When switching a model temporarily via
Alt+Por/switch, the switcher immediately sets the session model to its native default reasoning level (e.g.,mediumforgoogle-antigravity/gemini-3.5-flash), completely ignoring custom role-specific thinking level overrides defined inconfig.yml(e.g.smol: google-antigravity/gemini-3.5-flash:highortiny: google-antigravity/gemini-3.5-flash:high).With this change, the switcher looks up your role configurations in
config.ymlto see if a custom thinking level (like:high) is configured for the selected model, and applies it.Testing
bun run check:typesand Biome checks pass cleanly.packages/coding-agent/CHANGELOG.md.bun checkpasses