fix(tui/settings): defer 1M toggle until close#557
fix(tui/settings): defer 1M toggle until close#557cbusillo wants to merge 2 commits intojust-every:mainfrom
Conversation
Stage session fast-mode and context-mode changes while the model settings overlay is open, then apply them on close. This prevents the 1M context toggle from reconfiguring the session and compacting history while the user is still inside settings. Add regression tests covering deferred apply and no-op restores.
![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: 4c789762e5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| impl ModelSettingsContent { | ||
| pub(crate) fn new(view: ModelSelectionView) -> Self { | ||
| let mut view = view; | ||
| view.defer_session_mode_toggles_until_close(); |
There was a problem hiding this comment.
Deferring session-mode toggles here causes user changes to be dropped if they leave the Model section before closing settings. In section mode, Tab/arrow navigation is handled by settings_handlers.rs (because ModelSelectionView::handle_key_event_direct returns false for those keys), so the active section can change without closing the overlay; later close_settings_overlay() only calls notify_close() for the current active section, which means ModelSettingsContent::on_close may never run and the staged fast/1M updates are lost when the overlay is dropped.
Useful? React with 👍 / 👎.
Run `on_close` for all mounted settings sections so deferred model-setting updates still apply even if the user navigates away from Model before closing the overlay. Add a regression test covering section-switch then close.
|
Addressed the review concern about deferred updates being lost after navigating away from the Model section. Follow-up commit: What changed:
Validation:
|
1 participant
Summary
Why
Flipping the 1M context setting could immediately reconfigure the session while the user was still inside the settings screen. In practice that meant the session could compact before the user had even exited settings.
Validation
cargo test -p code-tui deferred_ --lib./build-fast.sh