-
Notifications
You must be signed in to change notification settings - Fork 833
Tool grouping prototype #6865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: mbuck/tool-reduction
Are you sure you want to change the base?
Tool grouping prototype #6865
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very clever, I had a couple small questions on behavior.
{ | ||
foreach (var tool in group.Tools) | ||
{ | ||
_ = _allGroupedTools.Add(tool); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we only group tools known at initialization? What about those added per call to GetResponse? I could imagine some sort of callback to assign a group. Or we could let folks annotate a tool with a group and store that in AdditionalProperties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thought is auto-grouping based on similarity of descriptions / embedding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's definitely something to consider. We could have a way to annotate a tool with a "category", similar to how tool descriptions are defined. Then chat clients like ToolGroupingChatClient
can utilize the category to perform grouping on a per-response basis. This keeps the tool configuration completely in ChatOptions
rather than having it be split between ChatOptions
and other middleware.
Or maybe we could change the AIToolGroup
type to extend AITool
. That way you could put tool groups directly in the ChatOptions
. You could even make AIToolGroup
unsealed and have a virtual GetToolsAsync()
method that can be overridden if you want to dynamically expose different sets of tools based on some other logic (this could be used to implement the "get more tools" function you mentioned in your other comment). I'll think about this a bit more.
method: _expansionFunctionDelegate, | ||
name: _options.ExpansionFunctionName, | ||
description: description) | ||
.AsDeclarationOnly(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, so we create a signature that the model can call, but it does nothing. We just observe that in the results when handling them from the inner client and use it to modify the tools we then send back to the client.
When an inner client gets a function invocation like this, does it know to halt communcation with the model and yield the result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep - FuncitonInvokingChatClient
has some logic to detect if an AITool
is not an AIFunction
. Such tools are considered "non-invokable" and terminate the function calling loop to allow the tool to be handled by the caller:
extensions/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
Lines 828 to 832 in d5cb8af
if (tool is not AIFunction) | |
{ | |
// The tool was found but it's not invocable. Regardless of TerminateOnUnknownCallRequests, | |
// we need to break out of the loop so that callers can handle all the call requests. | |
return true; |
} | ||
|
||
var groupName = groupNameArg.ToString(); | ||
if (groupName is null || !_toolGroupsByName.TryGetValue(groupName, out var group)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we always return all Tools in a group, even if those tools were not part of the options for this request? I can see how that might make sense - we have to choose a behavior - are the groupings additive to call, or does the call filter the groupings. Given groupings are new and the call behavior exists, I had assumed the call would filter the groupings (but I wouldn't trust my perspective here 😄).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we could definitely treat the group as a "filter" more than an extra set of available tools if we wanted. Maybe this is another reason to define groups directly on ChatOptions
rather than configuring them on the middleware.
Summary
This PR demonstrates a prototype middleware that allows grouping related tools. Groups are hidden from the inner chat client by default, but a well-known "expand" function can be called to expand one group at a time. By reducing the number of tools available to the model at a time, its ability to select the correct tool improves.
Following is an overview of the changes in this PR:
ToolGroupingChatClient
, which provides tool grouping and expansion functionality.AIToolGroup
to model stable, named batches of tools.Usage example
Implementation notes
This middleware contains a lot of the same complexities present in
FunctionInvokingChatClient
: cloning options, honoringConversationId
, aggregatingUsage
across inner calls, and replaying tool-result messages into the history. The expansion loop is deliberately limited to one active group per top-level request to prevent degradation of tool selection as the number of expanded groups increases.Microsoft Reviewers: Open in CodeFlow