-
Notifications
You must be signed in to change notification settings - Fork 849
Remove AIFunctionDeclaration tools on last iteration in FunctionInvokingChatClient #7207
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: main
Are you sure you want to change the base?
Conversation
- Add PrepareOptionsForLastIteration helper method that removes AIFunctionDeclaration tools from options on the last iteration - Modify GetResponseAsync to call PrepareOptionsForLastIteration when iteration >= MaximumIterationsPerRequest - Modify GetStreamingResponseAsync similarly - If no tools remain after filtering, also clear ToolMode - Add 4 comprehensive tests for the new behavior Co-authored-by: stephentoub <[email protected]>
- Simplify condition logic in PrepareOptionsForLastIteration by using a local variable for remaining count - Simplify test assertions for better readability Co-authored-by: stephentoub <[email protected]>
test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs
Outdated
Show resolved
Hide resolved
test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs
Outdated
Show resolved
Hide resolved
|
@westey-m , @rogerbarreto , @eiriktsarpalis I've opened this after seeing microsoft/agent-framework#3113. Good idea? Better to leave as is? |
- Remove all Arrange/Act/Assert comments from tests - Replace MockNonFunctionTool with HostedWebSearchTool - Delete the MockNonFunctionTool class Co-authored-by: stephentoub <[email protected]>
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.
Pull request overview
This PR addresses an issue where FunctionInvokingChatClient would send function declaration tools to the inner client on the final iteration (when the maximum iterations limit is reached), potentially causing the inner client to return tool call requests that would never be executed.
Changes:
- Added
PrepareOptionsForLastIterationhelper method that filters outAIFunctionDeclarationtools when reaching the iteration limit - Applied this filtering to both
GetResponseAsyncandGetStreamingResponseAsyncmethods - Options are cloned before modification to preserve the original caller's options
ToolModeis cleared when all tools are removed (all wereAIFunctionDeclaration), but preserved when non-function tools remain
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs | Added PrepareOptionsForLastIteration method and integrated it into both streaming and non-streaming code paths to remove AIFunctionDeclaration tools on the last iteration |
| test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs | Added comprehensive tests covering non-streaming, streaming, non-function tool preservation, and original options immutability scenarios |
|
This seems reasonable to me. If an answer cannot be reached after the configured iterations, I do think it's better to have the LLM respond with an inconclusive answer (i.e. I cannot answer this question) rather than produce a result more consistent with manual function invocation. The whole point of maximum iterations is to stop endlessly cycling through functions, and producing yet another one is not consistent with that goal. |
|
In my opinion, a new setting in the FICC should be available to define what is the For scenarios where the caller unintentionally caused tool cycling loop would not possible to discern if there was an infinite iteration loop error or normal finite iteration. If the above is not a concern, looks good. |
FunctionInvokingChatClientreaches its maximum iterations limit, it should not include any AIFunctionDeclaration tools in the ChatOptions.Tools it sends to the inner serviceChanges
PrepareOptionsForLastIterationhelper method that removes AIFunctionDeclaration tools from optionsGetResponseAsyncto call this method wheniteration >= MaximumIterationsPerRequestGetStreamingResponseAsyncsimilarlyTests Added
LastIteration_RemovesFunctionDeclarationTools_NonStreaming- Verifies tools are removed on last iterationLastIteration_RemovesFunctionDeclarationTools_Streaming- Verifies tools are removed on last iteration for streamingLastIteration_PreservesNonFunctionDeclarationTools- Verifies non-AIFunctionDeclaration tools (like HostedWebSearchTool) are preservedLastIteration_DoesNotModifyOriginalOptions- Verifies original options are not mutatedVerification
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Microsoft Reviewers: Open in CodeFlow