feat: add variant support for commands (#7713) #7907
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: I found related PRs that might be relevant to this variant support implementation: Related PRs (Not exact duplicates, but related features):
These are complementary features rather than direct duplicates. The current PR (#7907) specifically adds variant support for commands, while #7140 and #7156 address variant support for agents/subagents. They may need to be coordinated during integration. No exact duplicate PRs were found for this specific commands variant implementation. |
There was a problem hiding this comment.
Pull request overview
This PR implements variant support for commands as requested in issue #7713. Users can now specify a variant field in command configurations to set a default variant for specific commands, enabling commands to have baked-in reasoning effort levels without requiring manual user selection.
Changes:
- Added optional
variantfield to command configuration schema - Implemented validation to ensure variant is only specified with a model
- Added error handling with helpful messages for invalid variant names
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/opencode/src/config/config.ts | Added variant field to Command schema as optional string |
| packages/opencode/src/command/index.ts | Added variant to Command.Info type and parsed it from config |
| packages/opencode/src/session/prompt.ts | Implemented validation for variant (requires model, checks validity) and passes variant to prompt execution |
| packages/opencode/test/config/config.test.ts | Added test case to verify variant field is correctly parsed from config |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| model: "test_model", | ||
| variant: "test_variant", |
There was a problem hiding this comment.
The test changes only verify that the config can be parsed with model and variant fields. However, there's no test coverage for the new validation logic in prompt.ts that:
- Throws an error when a variant is specified without a model
- Throws an error when an invalid variant name is provided
- Successfully executes a command with valid model and variant
Consider adding tests for these error cases and success scenarios to ensure the validation logic works as intended.
| if (command.variant) { | ||
| const variants = info.variants ?? {} | ||
| const available = Object.keys(variants) | ||
| if (!variants[command.variant]) { | ||
| const hint = available.length ? ` Available variants: ${available.join(", ")}` : "" | ||
| const error = new NamedError.Unknown({ | ||
| message: `Variant not found: ${model.providerID}/${model.modelID} with variant "${command.variant}".${hint}`, | ||
| }) | ||
| Bus.publish(Session.Event.Error, { | ||
| sessionID: input.sessionID, | ||
| error: new NamedError.Unknown({ message: `Model not found: ${providerID}/${modelID}.${hint}` }).toObject(), | ||
| error: error.toObject(), | ||
| }) | ||
| throw error | ||
| } | ||
| throw e | ||
| } |
There was a problem hiding this comment.
The variant validation at lines 1548-1562 only validates command.variant, but the effective variant passed to the prompt function is command.variant ?? input.variant (line 1597). This means that if a command specifies a model but no variant, and a user provides an invalid variant at runtime via input.variant, it won't be validated here and will be silently ignored later in llm.ts. While this matches the existing behavior for runtime variants in other parts of the system, it creates an inconsistency within command execution where configured variants are strictly validated but runtime variants are not. Consider either: (1) documenting this behavior difference, or (2) validating the effective variant regardless of its source.
1 participant
What does this PR do?
Fixes #7713 by the following:
How did you verify your code works?
packages/opencode/test/config/config.test.tsfor config parsing.opencode.jsonin the root:Consideration
When #7156 or #7140 lands, its model-default variant feature might need to be integrated.