Skip to content

adding /model to set the current model in TUI #1714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

pap-openai
Copy link
Contributor

@pap-openai pap-openai commented Jul 28, 2025

/model slash command

image

/model "xxx" sets the model, can be custom or selected from the dropdown shown below when pressing enter

image (the event shown above is shown for both dropdown and setting)

/model returns a navigable dropdown of models

image

/model also reads from config.toml to list models defined locally as per the last model on screenshot above

/model dropdown is searchable

image

@pap-openai pap-openai added the codex-rust-review Perform a detailed review of Rust changes. label Jul 31, 2025
@pap-openai pap-openai added codex-rust-review Perform a detailed review of Rust changes. and removed codex-rust-review Perform a detailed review of Rust changes. labels Jul 31, 2025
@github-actions github-actions bot added codex-rust-review-in-progress and removed codex-rust-review Perform a detailed review of Rust changes. labels Jul 31, 2025
Copy link

Summary

Adds an interactive /model popup to the TUI so users can fuzzy-search and pick a model at runtime. Core gains Config::to_configure_session_op, dynamic model hand-off, a curated OpenAI model list, and the TUI now depends on toml.

Notes

  • ModelSelectionPopup (+519 LOC) and wiring in chat_composer.rs.
  • Core refactor: reuseable helper replaces hard-coded Op::ConfigureSession.
  • openai_model_info::get_all_model_names() lists common models.
  • codex-rs/tui/Cargo.toml adds toml = "0.8" (after uuid).

Review

Nice ergonomic improvement—runtime model switching feels smooth! A few nits before merge:

  • PR body: please add a short motivation (& maybe a GIF screenshot) for future readers.
  • Keep [dependencies] alphabetized; move toml above uuid.
  • chat_composer.rs is getting large (>1 k LOC); consider extracting popup-specific logic.
  • The ad-hoc fuzzy_match helper might be reusable elsewhere—maybe promote to common.
  • Minor: get_all_model_names() could live next to ModelInfo or be data-driven to avoid drift.

Overall, functionally solid—just tidy up the small issues and this is good to go!


View workflow run

@pap-openai
Copy link
Contributor Author

pap-openai commented Jul 31, 2025

Keep [dependencies] alphabetized; move toml above uuid.

moved

The ad-hoc fuzzy_match helper might be reusable elsewhere—maybe promote to common.

done and changed refactored file search so it uses it too

@easong-openai
Copy link
Contributor

Appears to be broken in the new UI? can type /model and hit enter and nothing appears after that.

@easong-openai
Copy link
Contributor

This does work, but there's a common pattern between several of our slash commands (and the @ menu) (including the base / itself), which is pressing /, then getting a dropdown that you navigate between, and then up/downing between them. In most cases we're going to want very similar scrolling/resizing primitives. I think we should make this some kind of component (I am terminally react brained) so that we don't have a multiple implementations running at the same time. Are you opposed to me merging this PR with the scrollable one and then we can reuse the same scrolling component?

@pap-openai
Copy link
Contributor Author

@easong-openai sounds good – are you bringing the scrolling component directly in this branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants