Migrate tui to use models manager
#7555
Conversation
aibrahim-oai
changed the title
tui to use models manager
💡 Codex Reviewcodex/codex-rs/core/src/conversation_manager.rs Lines 46 to 50 in 730f4d1  Update available models when auth mode changes
ModelsManager is constructed once with the initial ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review this |
…enai/codex into migrate-tui-to-models-manager
| conversations: Arc<RwLock<HashMap<ConversationId, Arc<CodexConversation>>>>, | ||
| auth_manager: Arc<AuthManager>, | ||
| models_manager: Arc<ModelsManager>, | ||
| pub models_manager: Arc<ModelsManager>, |
There was a problem hiding this comment.
consider a getter method
codex-rs/tui/src/chatwidget/tests.rs
Outdated
| .blocking_read() | ||
| .iter() | ||
| .find(|&preset| preset.model == "gpt-5.1-codex-max") | ||
| .cloned() |
There was a problem hiding this comment.
yeah because we don't want the reference
![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.
ℹ️ 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".
| pub(crate) fn open_model_popup(&mut self) { | ||
| let current_model = self.config.model.clone(); | ||
| let auth_mode = self.auth_manager.auth().map(|auth| auth.mode); | ||
| let presets: Vec<ModelPreset> = builtin_model_presets(auth_mode); | ||
| let presets: Vec<ModelPreset> = self | ||
| .models_manager | ||
| .available_models | ||
| .blocking_read() | ||
| .iter() | ||
| .cloned() | ||
| .collect(); |
There was a problem hiding this comment.
The model popup now reads the presets from models_manager.available_models instead of calling builtin_model_presets with the current auth. ModelsManager seeds available_models only once at construction (ModelsManager::new(auth_manager.get_auth_mode())) and nothing refreshes it when the user logs in or changes auth mode, so the picker keeps showing whatever list was computed at startup. For example, starting unauthenticated and then logging in with an API key will still surface gpt-5.1-codex-max even though builtin_model_presets is supposed to hide it for API key auth. This regresses the model picker’s filtering based on current credentials.
Useful? React with 👍 / 👎.
| let auth_mode = self.auth_manager.auth().map(|auth| auth.mode); | ||
| builtin_model_presets(auth_mode) | ||
| .into_iter() | ||
| let models = self.models_manager.available_models.blocking_read(); |
There was a problem hiding this comment.
this should be an async method at some point
There was a problem hiding this comment.
The issue is ChatWidget methods are mostly sync. Needs some work to convert
| let presets: Vec<ModelPreset> = builtin_model_presets(auth_mode); | ||
| let presets: Vec<ModelPreset> = self | ||
| .models_manager | ||
| .available_models |
There was a problem hiding this comment.
can we have a single method that does all this chain at once? get_available_models() ??
…into migrate-app-server
…enai/codex into migrate-tui-to-models-manager
…enai/codex into migrate-tui-to-models-manager
ModelsManagerlikeAuthManagerand propagate it into the tui, replacing thebuiltin_model_presetsbuiltin_model_presetsbased on #7552