Support tool filtering by model#278904
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements support for filtering language model tools based on the model being used, addressing issue #275679. The feature allows tool authors to specify which models can use their tools through a new supportedModels configuration field.
- Added
supportedModels?: string[]field to tool data structures throughout the codebase - Implemented filtering logic using
isToolAvailableForModel()helper function to check model compatibility - Updated the API to use
Map<LanguageModelToolInformation, boolean>instead ofMap<string, boolean>for better type safety
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/vscode-dts/vscode.proposed.chatParticipantAdditions.d.ts | Updated ChatRequest.tools to use LanguageModelToolInformation as map key instead of string for better type safety |
| src/vs/workbench/contrib/chat/common/tools/languageModelToolsContribution.ts | Added supportedModels field to tool contribution schema and tool data interface |
| src/vs/workbench/contrib/chat/common/languageModelToolsService.ts | Added supportedModels field to IToolData and implemented isToolAvailableForModel() helper function for model-based filtering |
| src/vs/workbench/contrib/chat/browser/actions/chatToolPicker.ts | Integrated model ID filtering in showToolsPicker using isToolAvailableForModel to filter out incompatible tools |
| src/vs/workbench/contrib/chat/browser/actions/chatToolActions.ts | Passed selected model ID to showToolsPicker for filtering |
| src/vs/workbench/api/common/extHostTypeConverters.ts | Updated ChatAgentRequest.to signature to accept Map<LanguageModelToolInformation, boolean> |
| src/vs/workbench/api/common/extHostLanguageModelTools.ts | Implemented isToolAvailableForModel method to check tool availability based on supportedModels |
| src/vs/workbench/api/common/extHostChatAgents2.ts | Updated getToolsForRequest to filter tools by model ID and return Map<LanguageModelToolInformation, boolean> |
| src/vs/workbench/api/common/extHost.protocol.ts | Added supportedModels field to IToolDataDto interface |
| src/vs/workbench/api/browser/mainThreadLanguageModelTools.ts | Updated tool data DTO mapping to include supportedModels field |
| type: 'string', | ||
| pattern: '^(?!copilot_|vscode_)' | ||
| } | ||
| }, |
There was a problem hiding this comment.
Is a static contribution flexible enough? It wouldn't let us control this with a setting/exp, or do more flexible checks like family.contains('-codex') that we do in code.
Also, going along with wanting to do other prompt tweaks in a more centralized place, it would be nice to have one place in code where I can determine the prompts and tools enabled for a certain model family.
How do you see this working for websearch, do you register a tool in api that doesn't have a invoke implementation but can show up in UI?
There was a problem hiding this comment.
I was thinking of piggy backing off ICopilotToolExtension that Bryan added to support alrernativeDefintions and have that dynamically compute the supported models for now.
How do you see this working for websearch, do you register a tool in api that doesn't have a invoke implementation but can show up in UI?
Yes. But I don't think you can't NOT have an invoke with the way things are setup currently though.
There was a problem hiding this comment.
I agree that we probably want more flexibility here. Otherwise every single release of every model will require changes. In the related issue #277472 I brought up the notion of using a when clause.
This does not necessarily mean you need to drive it off global context keys (though maybe using when would overload it, perhaps use a different name?) as there is a standalone Parser in contextkey.ts that returns an expression you can easily evaluate by passing an object with a getValue method. So having something specific there that is evaluated when a request is sent with information about the model, family, etc. is doable.
There was a problem hiding this comment.
How about updating LanguageModelTool to allow tools to dynamically specify which models they support?
declare module 'vscode' {
export interface LanguageModelTool<T> {
/**
* Called to check if this tool supports a specific language model. If this method is not implemented,
* the tool is assumed to support all models.
*
* This method allows extensions to dynamically determine which models a tool can work with,
* enabling fine-grained control over tool availability based on model capabilities.
*
* @param modelId The identifier of the language model (e.g., 'gpt-4o', 'claude-3-5-sonnet')
* @returns `true` if the tool supports the given model, `false` otherwise
*/
supportsModel?(modelId: string): Thenable<boolean>;
}
}
There was a problem hiding this comment.
While that would definitely be a bit easier to author, the downside with that approach is that we must activate every extension that provides language model tools when a chat message is sent in order to filter them appropriately. Previously we've only needed to activate tool-providing extensions when a tool they provide is actually invoked.
There was a problem hiding this comment.
Are you expecting this to be something that would be useful as real API? I was only thinking of this for our core tools. Is MCP considering something like this?
There was a problem hiding this comment.
Was only looking to support this for the builit-in tools we expose. @connor4312 do you anticipate needing this per model activation for MCP tools ?
There was a problem hiding this comment.
For MCP there are two scenarios:
- Tools from MCP servers that are defined in
agents.mdand similar. We're seeing clients start to support inline MCP's in this way and so their tools should only be applied to requests that involve those agent instructions. - Sampling with tools. In these cases tools are purely internal, we get them in core in the MCP sampling request and can register them however we want.
In both cases no particular extension API is needed for the MCP scenarios.
| * A map of all tools that should (`true`) and should not (`false`) be used in this request. | ||
| */ | ||
| readonly tools: Map<string, boolean>; | ||
| readonly tools: Map<LanguageModelToolInformation, boolean>; |
There was a problem hiding this comment.
I think this is needed for the more general cases in #277472 where LanguageModelToolInformation given to a request might not be globally available on vscode.lm.tools, or where there are multiple definitions of a tool that vary per model and the tool name alone is not enough to identify it.
I would pair this with making invokeTool(tool: string | LanguageModelToolInformation so that extensions are able to invoke tools that are only visible in the scope of a single request
There was a problem hiding this comment.
I needed this to be able to filter the tools for a request in the chat extension to just the supported ones.
Will explore updating invokeTools per Connor's suggestion above.
bhavyaus
force-pushed
the
dev/bhavyau/tools-model
branch
from
e3ee5c4 to
d228b3c
Compare
This is a rethinking of the initial API proposed in `languageModelToolSupportsModel.d.ts`. 1. This switches to `models?: LanguageModelChatSelector[];` to control enablement. definitely open to switching this out, but I think a synchronously-analyzable expression is important to retain the data flows in core without too many races. 2. The extension is able to define a tool at runtime via registerToolDefinition. This should let us have entirely service-driven tools from model providers without requiring a static definition for each one. We can also have model-specific variants of tools without a ton of package.json work for each variant of the tool (as initially proposed using `when` clauses) This then propagates that down into the tools service. Currently I have this as just compiling to a `when` expression once it reaches the main thread. Then, for the tools service, it takes an IContextKeyService in cases where tools should be enumerated, and the chat input sets the model keys in its scoped context key service. This allows the tools to be filtered correctly in the tool picker. I initially thought about allowing multiple definitions be registered for the same tool name/id for model-specific variants of tools but I realized that gets really gnarly and we already have a `toolReferenceName` that multiple tools can register into. Todo for tomorrow morning: - Tools don't make it to the ChatRequest yet, or something, still need to investigate - Need to make sure tools in prompts/models all work. For a first pass I think we can let prompts/modes reference all tools by toolReferenceName. - Validate that multiple tools actually can safely share a reference name (and do some priority ordering?) - General further validation - Some unit tests
* chat: wip on model-specific tools This is a rethinking of the initial API proposed in `languageModelToolSupportsModel.d.ts`. 1. This switches to `models?: LanguageModelChatSelector[];` to control enablement. definitely open to switching this out, but I think a synchronously-analyzable expression is important to retain the data flows in core without too many races. 2. The extension is able to define a tool at runtime via registerToolDefinition. This should let us have entirely service-driven tools from model providers without requiring a static definition for each one. We can also have model-specific variants of tools without a ton of package.json work for each variant of the tool (as initially proposed using `when` clauses) This then propagates that down into the tools service. Currently I have this as just compiling to a `when` expression once it reaches the main thread. Then, for the tools service, it takes an IContextKeyService in cases where tools should be enumerated, and the chat input sets the model keys in its scoped context key service. This allows the tools to be filtered correctly in the tool picker. I initially thought about allowing multiple definitions be registered for the same tool name/id for model-specific variants of tools but I realized that gets really gnarly and we already have a `toolReferenceName` that multiple tools can register into. Todo for tomorrow morning: - Tools don't make it to the ChatRequest yet, or something, still need to investigate - Need to make sure tools in prompts/models all work. For a first pass I think we can let prompts/modes reference all tools by toolReferenceName. - Validate that multiple tools actually can safely share a reference name (and do some priority ordering?) - General further validation - Some unit tests * key selected tools based on reference name
|
@aeschli in this PR I have suggested moving from storing the |
|
I see that To do that validation, I should not have to create a IContextKeyService instance. Using the IContextKeyService for globals like confguration settings is IMO ok, but we should not go beyond that. |
|
That is reasonable. I'll adjust some of the logic. |
|
@connor4312 The problem of toolReferenceName is that it is not unique. Only when it is qualified with the source (MCP server, extension id) then it is. We have the concept of the full reference name ( ) but its computation requires to iterate over all tool setsThis is my debt list to improve, but I don't now when... |
|
@aeschli do you have any ideas how you would like to see support for this linking work, then? We could probably ship without it but it would feel buggy that a visually identical 'web search' tool is deselected between modelsl |
|
I like the addition of |
|
Is the main purpose to have specialized tool implementations for a tool? It might be cleaner to model that and have an additional contribution point or provider for tool implementations. A tool implementation has the list of models it supports and points to a tool definition id. It doesn't need to repeat all the properties. |
We already have a mechanism for that in the extension, but there are more scenarios. From a comment a while ago: Some tools have definitions that would be useful to specialize/craft depending on the model. We have overrides for this in copilot chat via Some models have tools that are vendor-provided tools, like Claude's memory tool. This previously could not be represented reasonably in the API. In another scenario, an agent mode file defines MCP servers that apply to that mode. I think if we want to be most specific then that MCP's associated tools should only be included/available for requests that come from that chat mode. This is not in this PR, but the work here could be extended to that scenario. In the MCP sampling scenario, a sampling (LM) request has whatever MCP-server-defined tools and those are only valid to be called by that specific request and should not be exposed otherwise. Again this is not in this PR, but the API is now there to allow this. |
For #275679
Ensures
ChatRequest.toolscorrectly reflects tool availability and enablement state by filtering based on the active model's compatibility.Key changes:
supportedModelsfield toIToolDataDtoand passed through main-to-exthost communicationExtHostChatAgents2.getToolsForRequest()to acceptmodelIdparameter and filter tools usingisToolAvailableForModel()getToolsForRequest()call sites to pass the active model ID