Add optional model parameter to runSubagent tool#298161
Add optional model parameter to runSubagent tool#298161digitarald wants to merge 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for explicitly selecting a language model when invoking the runSubagent built-in tool, enabling callers to pass a qualified model name (e.g. "Model (Vendor)") and have it participate in the existing model-resolution + cost-guard logic.
Changes:
- Extend
IRunSubagentToolInputParamsand the tool input schema with optionalmodel?: string. - Thread
args.modelthroughprepareToolInvocation()andinvoke(), and prioritize it inresolveSubagentModel(). - Add a new test suite covering schema presence and explicit-model resolution/fallback scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/common/tools/builtinTools/runSubagentTool.ts | Adds model to schema/params; updates model-resolution to accept explicit qualified model name and apply multiplier fallback. |
| src/vs/workbench/contrib/chat/test/common/tools/builtinTools/runSubagentTool.test.ts | Adds tests for explicit-model parameter behavior and schema presence. |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/common/tools/builtinTools/runSubagentTool.ts:100
- The
modelschema description says “If not provided, uses the current model”, but whenagentNameis provided the tool may instead use the agent’s configured model. Please clarify the description to match actual resolution behavior (explicit model → agent-configured model → current/main model).
model: {
type: 'string',
description: 'Optional qualified model name to use for the subagent, in the format "Model Name (Vendor)". If not provided, uses the current model.'
}
|
@digitarald Can you link to an issue that describes the need for this? |
b0f196b to
5bdda14
Compare
|
Created #298380 and linked it. Yes, this is for the March 2026 milestone (already set). |
5bdda14 to
6d01047
Compare
6d01047 to
789488f
Compare
…che eviction - Remove duplicate @ILanguageModelToolsService injection (toolsService), use languageModelToolsService consistently - Improve model schema description with concrete examples for fast vs reasoning models - Add _MaxCachedModels cap (100) with FIFO eviction in prepareToolInvocation to prevent unbounded _resolvedModels growth on cancellation - Remove unnecessary 'outer:' label on single for-loop break - Remove duplicate mockToolsService args in tests
| ) { | ||
| super(); | ||
| this.onDidUpdateToolData = Event.filter(this.configurationService.onDidChangeConfiguration, e => e.affectsConfiguration(ChatConfiguration.SubagentToolCustomAgents)); | ||
| this.onDidUpdateToolData = Event.debounce( |
There was a problem hiding this comment.
what's the rational for the delay?
|
@aeschli |
There was a problem hiding this comment.
The tool description now lists all model names. That's for me around 30 models and I could have many more with BYOK. This increasing the context size and COGS but I'm not sure I'll profit much from that model selection. Again, the question is still, how would the LM chose a model over the other. The model names are not descriptive
I suggest the main agent should explicitly name the models
Run the search subagent with either the 'Opus 4.6 (copilot)' model when extra reasoning is required.
Given this is an experimental feature this is the a cost efficient way.
That way you can also remove the onDidChangeLanguageModels event
| @IProductService private readonly productService: IProductService, | ||
| ) { | ||
| super(); | ||
| // Debounce because onDidChangeLanguageModels fires per vendor/model during |
There was a problem hiding this comment.
This should be improved in the onDidChangeLanguageModels then. Please file a issue against onDidChangeLanguageModels to collapse the events on start up and add the issue number in the comment.
|
@digitarald Happy to make the change, let me know if its ok for you:
|
|
@aeschli sorry for the delay, maybe we should do this after the #306871 . The problem with just providing the model names is lacking context for when the agents should use each. For example Copilot CLI adds another label to clarify that: List:
Thoughts on bringing in the labels? Not sure if we have them or could apply but we could also apply them on multiplier. |
|
+1 didn't realize my PR was a duplicate of this, I second this behavior I like it and think it should be inbuilt. Agents shouldn't by default perform like this but having ability to tune agents for tasks in requests like already happening iun explore or agent instructions files is great and should be a feature, if I want Haiku to write my PR I shouldn't have to create a whole new agent (though in this example it probably would be good practice, it's just an example) I should just be able to say "after you've completed your changes run a haiku agent to upload to github". +1000 |
|
Implemented in #308784 |
Possibly could be worth listing free models as a tier if available (especially for copilot pro+ users), would make users much happier if their requests weren't eaten by saying "use the cheapest/free subagents" and then it calls 35 instances of Opus 4.6 [happened to me, many times, fun times]. |
Adds an optional
modelparameter to therunSubagenttool, allowing the LLM to specify which language model a subagent should use.Fixes #298380
Changes
Model parameter
model?: stringtoIRunSubagentToolInputParamsinterface and tool schema"Model Name (Vendor)"Dynamic model enum
modelproperty includes a dynamicenumof available models filtered bysuitableForAgentMode()(requirestoolCallingcapability andagentMode !== false)Event.any()to merge config changes andonDidChangeLanguageModelsevents, triggering tool re-registration when models changeenum: [])Model resolution priority
modelparam) > Agent configured model > Main modelTests
agentMode: falseexclusion, empty-model omission, explicit model resolution, fallback chains, multiplier guard