feat: add /model meta command to select model #502
Conversation
RayZhao1998
force-pushed
the
feature/support-model-metacmd
branch
from
63c84f1 to
1dbd911
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new /model meta command to enable quick model switching without re-running setup, and refactors model persistence and capability checking to support this feature.
Key Changes:
- Setup now persists all discovered models (not just the selected one) in the config
- New
/modelmeta command allows interactive model selection from configured models - Model capabilities are now accessed dynamically via a provider function instead of being cached at initialization
- Automatic thinking mode disabling when switching to models that don't support it
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/kimi_cli/ui/shell/setup.py | Modified to save all filtered models from API response instead of only the selected model |
| src/kimi_cli/ui/shell/model.py | New file implementing the /model meta command with interactive model selection and switching |
| src/kimi_cli/ui/shell/prompt.py | Changed from static model capabilities to dynamic provider pattern; added thinking mode validation |
| src/kimi_cli/ui/shell/metacmd.py | Added import for the new model module to register the /model command |
| src/kimi_cli/ui/shell/init.py | Updated to use model_capabilities_provider instead of static capabilities |
| src/kimi_cli/soul/kimisoul.py | Added switch_model method; minor formatting improvements |
| src/kimi_cli/soul/agent.py | Added switch_model method to Runtime class; added necessary imports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| assert isinstance(app.soul, KimiSoul) |
There was a problem hiding this comment.
Using a bare assert statement for type narrowing in production code is problematic. If Python is run with optimization flags (-O or -OO), assert statements are removed, which would cause this code to fail. Instead, use an explicit isinstance check with proper error handling. Consider replacing this with:
if not isinstance(app.soul, KimiSoul):
console.print("[red]Model switching is only available for Kimi Soul.[/red]")
return
| try: | |
| assert isinstance(app.soul, KimiSoul) | |
| if not isinstance(app.soul, KimiSoul): | |
| console.print("[red]Model switching is only available for Kimi Soul.[/red]") | |
| return | |
| try: |
| try: | ||
| assert isinstance(app.soul, KimiSoul) | ||
| app.soul.switch_model(provider_for_runtime, model_for_runtime) | ||
| except Exception as e: | ||
| console.print(f"[red]Failed to switch model: {e}[/red]") | ||
| return |
There was a problem hiding this comment.
The exception handling here is too broad. Catching all exceptions with a generic Exception will mask different types of errors (like AttributeError if switch_model doesn't exist, or runtime errors from the model switching logic itself). Consider catching more specific exceptions and providing more informative error messages to help users debug issues.
| try: | |
| assert isinstance(app.soul, KimiSoul) | |
| app.soul.switch_model(provider_for_runtime, model_for_runtime) | |
| except Exception as e: | |
| console.print(f"[red]Failed to switch model: {e}[/red]") | |
| return | |
| if not isinstance(app.soul, KimiSoul): | |
| console.print("[red]Cannot switch model: active soul is not a KimiSoul instance.[/red]") | |
| return | |
| try: | |
| app.soul.switch_model(provider_for_runtime, model_for_runtime) | |
| except AttributeError: | |
| console.print("[red]Failed to switch model: current soul does not support model switching.[/red]") | |
| return | |
| except (TypeError, ValueError) as e: | |
| console.print(f"[red]Failed to switch model due to invalid configuration: {e}[/red]") | |
| return |
| @meta_command | ||
| async def model(app: Shell, args: list[str]): | ||
| config = load_config() | ||
| if not config.models: | ||
| console.print("[yellow]No models configured. Run /setup first.[/yellow]") | ||
| return | ||
|
|
||
| current_model = app.soul.model_name if isinstance(app.soul, KimiSoul) else "" | ||
|
|
||
| options: list[tuple[str, str]] = [] | ||
| for model_id in sorted(config.models.keys()): | ||
| label = f"{model_id}" | ||
| if model_id == current_model: | ||
| label += " (current)" | ||
| elif model_id == config.default_model: | ||
| label += " (default)" | ||
| options.append((model_id, label)) | ||
|
|
||
| default_choice = current_model or config.default_model or options[0][0] | ||
| try: | ||
| selection = await ChoiceInput( | ||
| message="Select the model (↑↓ navigate, Enter select, Ctrl+C cancel):", | ||
| options=options, | ||
| default=default_choice, | ||
| ).prompt_async() | ||
| except (EOFError, KeyboardInterrupt): | ||
| return | ||
|
|
||
| if not selection: | ||
| return | ||
|
|
||
| model_config = config.models.get(selection) | ||
| if model_config is None: | ||
| console.print(f"[red]Model {selection} not found in config.[/red]") | ||
| return | ||
|
|
||
| provider_config = config.providers.get(model_config.provider) | ||
| if provider_config is None: | ||
| console.print(f"[red]Provider {model_config.provider} not found in config.[/red]") | ||
| return | ||
|
|
||
| provider_for_runtime = provider_config.model_copy(deep=True) | ||
| model_for_runtime = model_config.model_copy(deep=True) | ||
|
|
||
| try: | ||
| assert isinstance(app.soul, KimiSoul) | ||
| app.soul.switch_model(provider_for_runtime, model_for_runtime) | ||
| except Exception as e: | ||
| console.print(f"[red]Failed to switch model: {e}[/red]") | ||
| return | ||
|
|
||
| console.print(f"[green]✓[/green] Model switched to {selection}") |
There was a problem hiding this comment.
The new /model meta command lacks test coverage. Given that the repository has comprehensive test coverage for other meta commands and features (see tests/test_metacmd.py), this new functionality should also have tests. Consider adding tests that verify:
- Model selection UI behavior
- Successful model switching
- Error handling when models or providers are not found
- Behavior when config.models is empty
- Correct handling of current/default model labels
| elif model_id == config.default_model: | ||
| label += " (default)" | ||
| options.append((model_id, label)) | ||
|
|
There was a problem hiding this comment.
This line could raise an IndexError if the options list is empty. While the code checks if config.models is empty at line 19, if all model_ids exist in config.models but for some reason the options list construction fails to produce any entries, this would crash. Consider using a safer default or ensuring options is never empty before this line.
| if not options: | |
| console.print("[yellow]No selectable models available.[/yellow]") | |
| return |
|
|
||
|
|
||
| @meta_command | ||
| async def model(app: Shell, args: list[str]): |
There was a problem hiding this comment.
The model meta command is missing a docstring. All other meta commands in the codebase have docstrings that describe their functionality (e.g., setup has "Setup Kimi CLI", help has "Show help information"). This docstring is used in the help system to describe what the command does. Add a docstring such as "Switch between configured models" or "Select a different model".
| async def model(app: Shell, args: list[str]): | |
| async def model(app: Shell, args: list[str]): | |
| """Switch between configured models.""" |
1 participant
Related Issue
#444
Motivation
Currently, switching models in Kimi CLI requires running
/setup, which forces users to re enter the API key each time. This makes model switching unnecessarily heavy during daily usage.This PR introduces a new
/modelmeta command that allows users to quickly switch between already configured models without re running setup.Key Changes
1. Persist all models during setup
Previously,
setuponly stored the user selected model in the config.This PR changes the behavior to:
config.modelsThis ensures the model list is available for later selection.
2. Add
/modelmeta command/modelmeta commandBehavior to Be Aware Of
If the current model supports
thinkingand thinking mode is enabled, then the user switches to a model that does not supportthinkingvia/model:Known Issue
If the current session has already used certain model capabilities, such as image input, switching to a model that does not support those capabilities may cause subsequent requests to fail.
At the moment, all available Kimi models do not support image or similar advanced capabilities, it can be tracked separately in a follow up issue if needed.