-
Notifications
You must be signed in to change notification settings - Fork 849
Skip function invocation when FunctionResultContent already exists #7011
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
Conversation
Co-authored-by: stephentoub <[email protected]>
Co-authored-by: stephentoub <[email protected]>
stephentoub
left a comment
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.
@copilot, please address the diagnostics:
Build FAILED.
D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\ChatCompletion\FunctionInvokingChatClientTests.cs(1332,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\ChatCompletion\FunctionInvokingChatClientTests.cs(1359,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\ChatCompletion\FunctionInvokingChatClientTests.cs(1368,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\ChatCompletion\FunctionInvokingChatClientTests.cs(1332,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net462]
D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\ChatCompletion\FunctionInvokingChatClientTests.cs(1359,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net462]
D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\ChatCompletion\FunctionInvokingChatClientTests.cs(1368,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net462]
D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\ChatCompletion\FunctionInvokingChatClientTests.cs(1332,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\ChatCompletion\FunctionInvokingChatClientTests.cs(1359,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\ChatCompletion\FunctionInvokingChatClientTests.cs(1368,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [D:\a\_work\1\s\test\Libraries\Microsoft.Extensions.AI.Tests\Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
0 Warning(s)
9 Error(s)
This also needs much more extensive testing, cover both streaming and non-streaming, FunctionResultContent coming back in the same or separate messages or updates from the corresponding FunctionCallContent, multiple FunctionCallContents some of which have FunctionResultContent and some of which don't, etc. I would also like to see a test that has multiple FunctionInvokingChatClients layered in and validating that functions are only ever invoked once and not once per FunctionInvokingChatClient.
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
Show resolved
Hide resolved
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 adds functionality to prevent redundant function invocation when server-side function calling results are already present in the response. The FunctionInvokingChatClient now checks for existing FunctionResultContent with matching CallIds and skips invoking those functions client-side.
Key Changes:
- Modified
CopyFunctionCallsmethods to filter out function calls that already have corresponding results - Added tracking of
FunctionResultContentCallIds in both streaming and non-streaming code paths - Comprehensive test coverage for various scenarios involving pre-existing function results
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| FunctionInvokingChatClient.cs | Added logic to collect FunctionResultContent CallIds and filter out FunctionCallContent with matching CallIds to prevent redundant function invocations |
| FunctionInvokingChatClientTests.cs | Added four comprehensive test cases covering partial results, all results present, server-side execution, and mixed scenarios across streaming and non-streaming paths |
Comments suppressed due to low confidence (1)
test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs:1557
- The
YieldAsyncmethod acceptsparams IEnumerable<T> itemsbut iterates over it as a single collection. This appears to be a bug - if multiple IEnumerable arguments are passed, only items from the first enumerable will be yielded. Theparamskeyword creates an array of IEnumerable, so the outer foreach iterates over enumerables, not their items. Either removeparamsor use nested foreach loops:foreach (var enumerable in items) foreach (var item in enumerable).
private static async IAsyncEnumerable<T> YieldAsync<T>(params IEnumerable<T> items)
test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
Show resolved
Hide resolved
…ncapsulation Co-authored-by: stephentoub <[email protected]>
... Fixed the formatting issues and added extensive testing. Commit 83f3393 includes: Formatting fixes: Added blank lines at lines 1332, 1359, and 1368 as required. Extensive testing: Added 10 new comprehensive tests covering:
All tests validate that functions are invoked exactly once even with multiple layered FunctionInvokingChatClients. All 49 FunctionInvokingChatClient tests and all 365 total AI tests pass on both net8.0 and net9.0. |
|
This needs more thought. Closing for now. |
FunctionInvokingChatClient Enhancement
This PR updates
FunctionInvokingChatClientto check for existingFunctionResultContentbefore invoking functions, enabling proper handling of server-side function calls and supporting multiple FunctionInvokingChatClient instances in a pipeline.Changes completed:
Implementation details:
Core changes to FunctionInvokingChatClient.cs:
CopyFunctionCallsfor streaming that both collects FRCs and filters FCCsTest coverage (49 total tests, all passing):
All 365 tests in Microsoft.Extensions.AI.Tests pass on both net8.0 and net9.0.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Microsoft Reviewers: Open in CodeFlow