Fix for broken Codex model selection for 'Text Generation Model'#1543
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
3f42f25 to
943d121
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
juliusmarminge
left a comment
There was a problem hiding this comment.
Feels like the entire function could be simplified now that i read it 😭😭

What Changed
Treat textGenerationModelSelection as a atomic block
Why
Broken switching between Codex models, only 'GPT 5.4 Mini' was selectable
Checklist
Note
Medium Risk
Touches settings persistence logic used for writing
settings.json; a mistake could cause incorrect config serialization or unexpected resets of user selections. Change is small and covered by a new regression test around provider/model switching.Overview
Fixes a persistence bug where
stripDefaultServerSettingscould partially strip nested fields insidetextGenerationModelSelection, causing the chosen Codex model to be dropped when switching providers.Adds
ATOMIC_SETTINGS_KEYS(currently justtextGenerationModelSelection) so those settings are compared and retained as a whole usingEqual.equals, and adds a regression test ensuring provider/model switching preserves the selected model.Written by Cursor Bugbot for commit 943d121. This will update automatically on new commits. Configure here.
Note
Fix
textGenerationModelSelectionbeing partially stripped when switching to Codex providerWhen switching providers,
stripDefaultServerSettingsin serverSettings.ts was recursively stripping nested fields fromtextGenerationModelSelection, causing the model to be dropped.ATOMIC_SETTINGS_KEYS, a set of keys that must be preserved or omitted as a whole rather than recursively stripped field-by-field.textGenerationModelSelectionis added to this set; its value is now compared against defaults using full-object equality, and the entire value is kept or dropped based on that comparison.Macroscope summarized 943d121.