feat: add --model flag to ucode copilot - #385
Conversation
Copilot had no way to pick a model per launch or per session: the CLI had no --model flag (unlike claude), and Copilot's own /models picker can't be trusted to work against the Databricks gateway, plus ucode overwrites COPILOT_MODEL on every token refresh anyway. The only override was an admin-only managed config. Thread an explicit model through past that refresh loop so it stays pinned for the whole session, instead of writing it once at config time and letting default_model() silently reclaim it 30 minutes later. Closes databricks#384
There was a problem hiding this comment.
Pull request overview
Adds per-launch --model selection to ucode copilot and preserves it across token refreshes.
Changes:
- Adds the
--modelCLI option. - Threads model overrides through Copilot dispatch and refresh logic.
- Adds CLI, dispatch, and refresh-loop tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Summary | Findings |
|---|---|---|
tests/test_cli.py |
Tests CLI model handling. | None |
tests/test_agents_init.py |
Tests dispatch behavior. | None |
tests/test_agent_copilot.py |
Tests model selection and refresh pinning. | None |
src/ucode/cli.py |
Defines and forwards --model. |
Critical (2 votes): Explicit models can fail when no fallback model exists because resolution occurs before the explicit override. |
src/ucode/agents/copilot.py |
Pins the model across refreshes. | None |
src/ucode/agents/__init__.py |
Dispatches Copilot model overrides. | None |
Suppressed comments (1)
src/ucode/cli.py:1961
resolved_modelcan differ from the raw CLImodel: for example, a managed budget recommendation is selected bymanaged_launch_model, written byconfigure_tool, but not stored in state. PassingNonefor a Copilot launch without an explicit--modelmakescopilot.launchcalldefault_model(state)again and immediately rewrite that selection, and refreshes keep doing so. Pass the already resolved model here so managed/recommended choices survive the initial write and refresh loop.
launch_agent(tool, state, ctx.args, model=model if tool == "copilot" else None)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Without an explicit --model, this passed None to launch_agent, so copilot.launch() recomputed default_model(state) from scratch instead of reusing what configure_tool had already written — silently diverging from a managed config's default or a budget recommendation, and repeating that drift on every 30-minute token refresh. resolved_model already absorbs --model, the managed default, and any budget recommendation, so it's the correct value to pin. Flagged by review on databricks#385.
|
Fixed in 0ff75e7 — |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/ucode/cli.py:2284
- This option is applied only after
_launch_toolhas completed its setup and model resolution. On a firstucode copilot --model X(or when Copilot is not yet inavailable_tools),_auto_configure_tool("copilot")runs first, andconfigure_single_tool/validate_envselect and validatedefault_model(state)without seeingX; if discovery has no default or the automatic model is unsupported, the command exits before the requested model is ever written or launched. Thread the explicit model through the auto-config/validation path, or skip default-model setup when--modelis supplied.
help="Launch on a specific Databricks model id (e.g. a UC "
"`<catalog>.<schema>.<name>`). Outranks the automatic sonnet/opus/haiku/codex "
"pick and stays pinned across ucode's automatic token refreshes.",
tests/test_agents_init.py:182
- This class is inserted before the existing
test_pi_available_*methods, so those availability tests are now collected underTestLaunchinstead ofTestCheckGatewayEndpoint. They still run, but the grouping is misleading and makes future fixture/setup changes error-prone; place the newTestLaunchclass after the complete gateway-endpoint test class.
class TestLaunch:
On a first-time `ucode copilot --model X` (copilot not yet in available_tools, or no workspace configured), _auto_configure_tool ran before the --model handling further down _launch_tool, so it wrote and smoke-tested the automatic sonnet/opus/haiku/codex default instead of X. If that automatic pick failed validation, the launch aborted before X was ever tried — even though X itself may have been perfectly valid. Thread the explicit model through configure_single_tool/_configure_one (so resolve_launch_model's explicit_model wins) and through validate_tool (so copilot.validate_env smoke-tests X, not the default). Also fixes a test-ordering issue: TestLaunch had been inserted in the middle of TestCheckGatewayEndpoint, splitting its test_pi_available_* tests into a misleadingly-named group. Flagged by review on databricks#385.
|
Addressed both review findings in 321e693:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/ucode/cli.py:1742
- This forwards the explicit model into first-time setup, but the normal launch path still calls
resolve_launch_model(tool, state, managed_model)before applyingmodelat line 1900. If discovery/state has no fallback model (for example with--skip-preflightor a failed model listing), that resolver raises before the requested Copilot model is ever considered, soucode copilot --model Xcannot launch. Pass the Copilot override into model resolution or bypass the fallback resolution when it is present.
_auto_configure_tool(tool, model=model if tool == "copilot" else None)
The docstring claimed explicit_model keeps a bad automatic pick from failing the availability check. It does not: check_gateway_endpoint runs before _configure_one sees explicit_model, and takes no model at all. explicit_model only changes what gets written and smoke-tested. Flagged by review on databricks#385.
Copilot has no way to pick a model per launch: there's no
--modelflag (unlikeclaude), Copilot's own/modelspicker can't be trusted against the Databricks gateway, anducodeoverwritesCOPILOT_MODELon every 30-minute token refresh anyway. The only override today is an admin-only managed config.Adds
--modeltoucode copilot, mirroringclaude. The explicit model is threaded past the refresh loop (_refresh_forever/_refresh_token_once) so it stays pinned for the whole session instead of being silently reclaimed bydefault_model()at the next refresh.Closes #384.