From 69b996b56891d28b24e22f6c67fd4811b672f67e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Dec 2025 16:12:58 +0000 Subject: [PATCH 1/6] Initial plan From 30d0ee7bb5b7224529b27072c49ae94df58060f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Dec 2025 16:19:57 +0000 Subject: [PATCH 2/6] Add InvocationRequired property to FunctionCallContent with tests Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../Contents/FunctionCallContent.cs | 10 + .../Microsoft.Extensions.AI.Abstractions.json | 4 + .../FunctionInvokingChatClient.cs | 12 +- .../Contents/FunctionCallContentTests.cs | 146 ++++++++++ .../FunctionInvokingChatClientTests.cs | 271 ++++++++++++++++++ 5 files changed, 441 insertions(+), 2 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionCallContent.cs b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionCallContent.cs index 836d5a4110b..3cb75e3627a 100644 --- a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionCallContent.cs +++ b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionCallContent.cs @@ -56,6 +56,16 @@ public FunctionCallContent(string callId, string name, IDictionary + /// Gets or sets a value indicating whether this function call requires invocation. + /// + /// + /// This property defaults to , indicating that the function call should be processed. + /// When set to , it indicates that the function has already been processed and + /// should be ignored by components that process function calls. + /// + public bool InvocationRequired { get; set; } = true; + /// /// Creates a new instance of parsing arguments using a specified encoding and parser. /// diff --git a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json index e401502d82b..fd4e91b623d 100644 --- a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json +++ b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json @@ -1805,6 +1805,10 @@ "Member": "System.Exception? Microsoft.Extensions.AI.FunctionCallContent.Exception { get; set; }", "Stage": "Stable" }, + { + "Member": "bool Microsoft.Extensions.AI.FunctionCallContent.InvocationRequired { get; set; }", + "Stage": "Experimental" + }, { "Member": "string Microsoft.Extensions.AI.FunctionCallContent.Name { get; }", "Stage": "Stable" diff --git a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs index 6a4fd7405a1..e9e842d2418 100644 --- a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs +++ b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs @@ -775,7 +775,7 @@ private static bool CopyFunctionCalls( int count = content.Count; for (int i = 0; i < count; i++) { - if (content[i] is FunctionCallContent functionCall) + if (content[i] is FunctionCallContent functionCall && functionCall.InvocationRequired) { (functionCalls ??= []).Add(functionCall); any = true; @@ -1018,6 +1018,9 @@ private async Task ProcessFunctionCallAsync( { var callContent = callContents[functionCallIndex]; + // Mark the function call as no longer requiring invocation since we're handling it + callContent.InvocationRequired = false; + // Look up the AIFunction for the function call. If the requested function isn't available, send back an error. if (toolMap is null || !toolMap.TryGetValue(callContent.Name, out AITool? tool) || @@ -1107,6 +1110,9 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul functionResult = message; } + // Mark the function call as having been processed + result.CallContent.InvocationRequired = false; + return new FunctionResultContent(result.CallContent.CallId, functionResult) { Exception = result.Exception }; } } @@ -1426,7 +1432,7 @@ private static (List? approvals, ListThe for the rejected function calls. private static List? GenerateRejectedFunctionResults(List? rejections) => rejections is { Count: > 0 } ? - rejections.ConvertAll(m => + rejections.ConvertAll(static m => { string result = "Tool call invocation rejected."; if (!string.IsNullOrWhiteSpace(m.Response.Reason)) @@ -1434,6 +1440,8 @@ private static (List? approvals, List { ["key"] = "value" }) + { + InvocationRequired = false + }; + + // Act + var json = JsonSerializer.SerializeToNode(sut, TestJsonSerializerContext.Default.Options); + + // Assert - InvocationRequired should be in the JSON when it's false to allow roundtrip + Assert.NotNull(json); + var jsonObj = json!.AsObject(); + Assert.True(jsonObj.ContainsKey("invocationRequired") || jsonObj.ContainsKey("InvocationRequired")); + + JsonNode? invocationRequiredValue = null; + if (jsonObj.TryGetPropertyValue("invocationRequired", out var value1)) + { + invocationRequiredValue = value1; + } + else if (jsonObj.TryGetPropertyValue("InvocationRequired", out var value2)) + { + invocationRequiredValue = value2; + } + + Assert.NotNull(invocationRequiredValue); + Assert.False(invocationRequiredValue!.GetValue()); + } + + [Fact] + public void InvocationRequired_SerializedWhenTrue() + { + // Arrange - InvocationRequired defaults to true + var sut = new FunctionCallContent("callId1", "functionName", new Dictionary { ["key"] = "value" }); + + // Act + var json = JsonSerializer.SerializeToNode(sut, TestJsonSerializerContext.Default.Options); + + // Assert - InvocationRequired should be in the JSON when it's true + Assert.NotNull(json); + var jsonObj = json!.AsObject(); + Assert.True(jsonObj.ContainsKey("invocationRequired") || jsonObj.ContainsKey("InvocationRequired")); + + JsonNode? invocationRequiredValue = null; + if (jsonObj.TryGetPropertyValue("invocationRequired", out var value1)) + { + invocationRequiredValue = value1; + } + else if (jsonObj.TryGetPropertyValue("InvocationRequired", out var value2)) + { + invocationRequiredValue = value2; + } + + Assert.NotNull(invocationRequiredValue); + Assert.True(invocationRequiredValue!.GetValue()); + } + + [Fact] + public void InvocationRequired_DeserializedCorrectlyWhenTrue() + { + // Test deserialization when InvocationRequired is true + var json = """{"callId":"callId1","name":"functionName","invocationRequired":true}"""; + var deserialized = JsonSerializer.Deserialize(json, TestJsonSerializerContext.Default.Options); + + Assert.NotNull(deserialized); + Assert.Equal("callId1", deserialized.CallId); + Assert.Equal("functionName", deserialized.Name); + Assert.True(deserialized.InvocationRequired); + } + + [Fact] + public void InvocationRequired_DeserializedCorrectlyWhenFalse() + { + // Test deserialization when InvocationRequired is false + var json = """{"callId":"callId1","name":"functionName","invocationRequired":false}"""; + var deserialized = JsonSerializer.Deserialize(json, TestJsonSerializerContext.Default.Options); + + Assert.NotNull(deserialized); + Assert.Equal("callId1", deserialized.CallId); + Assert.Equal("functionName", deserialized.Name); + Assert.False(deserialized.InvocationRequired); + } + + [Fact] + public void InvocationRequired_DeserializedToTrueWhenMissing() + { + // Test deserialization when InvocationRequired is not in JSON (should default to true from field initializer) + var json = """{"callId":"callId1","name":"functionName"}"""; + var deserialized = JsonSerializer.Deserialize(json, TestJsonSerializerContext.Default.Options); + + Assert.NotNull(deserialized); + Assert.Equal("callId1", deserialized.CallId); + Assert.Equal("functionName", deserialized.Name); + Assert.True(deserialized.InvocationRequired); + } + + [Fact] + public void InvocationRequired_RoundtripTrue() + { + // Test that InvocationRequired=true roundtrips correctly through JSON serialization + var original = new FunctionCallContent("callId1", "functionName") { InvocationRequired = true }; + var json = JsonSerializer.SerializeToNode(original, TestJsonSerializerContext.Default.Options); + var deserialized = JsonSerializer.Deserialize(json, TestJsonSerializerContext.Default.Options); + + Assert.NotNull(deserialized); + Assert.Equal(original.CallId, deserialized.CallId); + Assert.Equal(original.Name, deserialized.Name); + Assert.Equal(original.InvocationRequired, deserialized.InvocationRequired); + Assert.True(deserialized.InvocationRequired); + } + + [Fact] + public void InvocationRequired_RoundtripFalse() + { + // Test that InvocationRequired=false roundtrips correctly through JSON serialization + var original = new FunctionCallContent("callId1", "functionName") { InvocationRequired = false }; + var json = JsonSerializer.SerializeToNode(original, TestJsonSerializerContext.Default.Options); + var deserialized = JsonSerializer.Deserialize(json, TestJsonSerializerContext.Default.Options); + + Assert.NotNull(deserialized); + Assert.Equal(original.CallId, deserialized.CallId); + Assert.Equal(original.Name, deserialized.Name); + Assert.Equal(original.InvocationRequired, deserialized.InvocationRequired); + Assert.False(deserialized.InvocationRequired); } [Fact] diff --git a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs index 3e9d2963291..a7493f64ed8 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs @@ -136,6 +136,7 @@ public async Task SupportsToolsProvidedByAdditionalTools(bool provideOptions) await InvokeAndAssertAsync(options, plan, configurePipeline: configure); + ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -169,6 +170,7 @@ public async Task PrefersToolsProvidedByChatOptions() await InvokeAndAssertAsync(options, plan, configurePipeline: configure); + ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -219,6 +221,7 @@ public async Task SupportsMultipleFunctionCallsPerRequestAsync(bool concurrentIn await InvokeAndAssertAsync(options, plan, configurePipeline: configure); + ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -267,6 +270,7 @@ public async Task ParallelFunctionCallsMayBeInvokedConcurrentlyAsync() await InvokeAndAssertAsync(options, plan, configurePipeline: configure); + ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -308,6 +312,7 @@ public async Task ConcurrentInvocationOfParallelCallsDisabledByDefaultAsync() await InvokeAndAssertAsync(options, plan); + ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan); } @@ -351,6 +356,7 @@ public async Task FunctionInvokerDelegateOverridesHandlingAsync() await InvokeAndAssertAsync(options, plan, configurePipeline: configure); + ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -562,6 +568,7 @@ public async Task KeepsFunctionCallingContent() #pragma warning disable SA1005, S125 Validate(await InvokeAndAssertAsync(options, plan)); + ResetPlanFunctionCallStates(plan); Validate(await InvokeAndAssertStreamingAsync(options, plan)); static void Validate(List finalChat) @@ -597,6 +604,7 @@ public async Task ExceptionDetailsOnlyReportedWhenRequestedAsync(bool detailedEr await InvokeAndAssertAsync(options, plan, configurePipeline: configure); + ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -1077,6 +1085,7 @@ public async Task FunctionInvocations_InvokedOnOriginalSynchronizationContext() .UseFunctionInvocation(configure: c => { c.AllowConcurrentInvocation = true; c.IncludeDetailedErrors = true; }); await InvokeAndAssertAsync(options, plan, configurePipeline: configurePipeline); + ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configurePipeline); } @@ -1110,6 +1119,7 @@ public async Task TerminateOnUnknownCalls_ControlsBehaviorForUnknownFunctions(bo ]; await InvokeAndAssertAsync(options, planForContinue, configurePipeline: configure); + ResetPlanFunctionCallStates(planForContinue); await InvokeAndAssertStreamingAsync(options, planForContinue, configurePipeline: configure); } else @@ -1493,6 +1503,243 @@ public async Task CreatesOrchestrateToolsSpanWhenNoInvokeAgentParent(bool stream } } + [Fact] + public async Task InvocationRequired_SetToFalseAfterProcessing() + { + var options = new ChatOptions + { + Tools = [AIFunctionFactory.Create(() => "Result 1", "Func1")] + }; + + List plan = + [ + new ChatMessage(ChatRole.User, "hello"), + new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1")]), + new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Result 1")]), + new ChatMessage(ChatRole.Assistant, "world"), + ]; + + var chat = await InvokeAndAssertAsync(options, plan); + + // Find the FunctionCallContent in the chat history + var functionCallMessage = chat.First(m => m.Contents.Any(c => c is FunctionCallContent)); + var functionCallContent = functionCallMessage.Contents.OfType().First(); + + // Verify InvocationRequired was set to false after processing + Assert.False(functionCallContent.InvocationRequired); + } + + [Fact] + public async Task InvocationRequired_IgnoresFunctionCallsWithInvocationRequiredFalse() + { + var functionInvokedCount = 0; + var options = new ChatOptions + { + Tools = [AIFunctionFactory.Create(() => { functionInvokedCount++; return "Result 1"; }, "Func1")] + }; + + // Create a function call that has already been processed + var alreadyProcessedFunctionCall = new FunctionCallContent("callId1", "Func1") { InvocationRequired = false }; + + using var innerClient = new TestChatClient + { + GetResponseAsyncCallback = async (contents, actualOptions, actualCancellationToken) => + { + await Task.Yield(); + + // Return a response with a FunctionCallContent that has InvocationRequired = false + var message = new ChatMessage(ChatRole.Assistant, [alreadyProcessedFunctionCall]); + return new ChatResponse(message); + } + }; + + using var client = new FunctionInvokingChatClient(innerClient); + + var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); + + // The function should not have been invoked since InvocationRequired was false + Assert.Equal(0, functionInvokedCount); + + // The response should contain the FunctionCallContent but no FunctionResultContent + Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionCallContent fcc && !fcc.InvocationRequired)); + Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent)); + } + + [Fact] + public async Task InvocationRequired_IgnoresFunctionCallsWithInvocationRequiredFalse_Streaming() + { + var functionInvokedCount = 0; + var options = new ChatOptions + { + Tools = [AIFunctionFactory.Create(() => { functionInvokedCount++; return "Result 1"; }, "Func1")] + }; + + // Create a function call that has already been processed + var alreadyProcessedFunctionCall = new FunctionCallContent("callId1", "Func1") { InvocationRequired = false }; + + using var innerClient = new TestChatClient + { + GetStreamingResponseAsyncCallback = (contents, actualOptions, actualCancellationToken) => + { + // Return a response with a FunctionCallContent that has InvocationRequired = false + var message = new ChatMessage(ChatRole.Assistant, [alreadyProcessedFunctionCall]); + return YieldAsync(new ChatResponse(message).ToChatResponseUpdates()); + } + }; + + using var client = new FunctionInvokingChatClient(innerClient); + + var updates = new List(); + await foreach (var update in client.GetStreamingResponseAsync([new ChatMessage(ChatRole.User, "hello")], options)) + { + updates.Add(update); + } + + // The function should not have been invoked since InvocationRequired was false + Assert.Equal(0, functionInvokedCount); + + // The updates should contain the FunctionCallContent but no FunctionResultContent + Assert.Contains(updates, u => u.Contents.Any(c => c is FunctionCallContent fcc && !fcc.InvocationRequired)); + Assert.DoesNotContain(updates, u => u.Contents.Any(c => c is FunctionResultContent)); + } + + [Fact] + public async Task InvocationRequired_ProcessesMixedFunctionCalls() + { + var func1InvokedCount = 0; + var func2InvokedCount = 0; + + var options = new ChatOptions + { + Tools = + [ + AIFunctionFactory.Create(() => { func1InvokedCount++; return "Result 1"; }, "Func1"), + AIFunctionFactory.Create(() => { func2InvokedCount++; return "Result 2"; }, "Func2"), + ] + }; + + // Create one function call that needs processing and one that doesn't + var needsProcessing = new FunctionCallContent("callId1", "Func1") { InvocationRequired = true }; + var alreadyProcessed = new FunctionCallContent("callId2", "Func2") { InvocationRequired = false }; + + using var innerClient = new TestChatClient + { + GetResponseAsyncCallback = async (contents, actualOptions, actualCancellationToken) => + { + await Task.Yield(); + + if (contents.Count() == 1) + { + // First call - return both function calls + var message = new ChatMessage(ChatRole.Assistant, [needsProcessing, alreadyProcessed]); + return new ChatResponse(message); + } + else + { + // Second call - return final response after processing + var message = new ChatMessage(ChatRole.Assistant, "done"); + return new ChatResponse(message); + } + } + }; + + using var client = new FunctionInvokingChatClient(innerClient); + + var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); + + // Only Func1 should have been invoked (the one with InvocationRequired = true) + Assert.Equal(1, func1InvokedCount); + Assert.Equal(0, func2InvokedCount); + + // The response should contain FunctionResultContent for Func1 but not Func2 + Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId1")); + Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId2")); + } + + [Fact] + public async Task InvocationRequired_InnerClientWithInvocationRequiredFalsePassesThroughUnprocessed() + { + // Test that FunctionCallContent with InvocationRequired=false from inner client passes through without being processed + var functionInvokedCount = 0; + var options = new ChatOptions + { + Tools = [AIFunctionFactory.Create(() => { functionInvokedCount++; return "Result 1"; }, "Func1")] + }; + + var functionCallWithInvocationRequiredFalse = new FunctionCallContent("callId1", "Func1") { InvocationRequired = false }; + + using var innerClient = new TestChatClient + { + GetResponseAsyncCallback = async (contents, actualOptions, actualCancellationToken) => + { + await Task.Yield(); + + // Inner client returns a FunctionCallContent with InvocationRequired = false + return new ChatResponse(new ChatMessage(ChatRole.Assistant, [functionCallWithInvocationRequiredFalse])); + } + }; + + using var client = new FunctionInvokingChatClient(innerClient); + + var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); + + // The function should NOT have been invoked since InvocationRequired was false + Assert.Equal(0, functionInvokedCount); + + // The response should contain the FunctionCallContent with InvocationRequired = false + Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionCallContent fcc && fcc.CallId == "callId1" && !fcc.InvocationRequired)); + + // There should be NO FunctionResultContent since we didn't process the function call + Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent)); + } + + [Fact] + public async Task InvocationRequired_MultipleFunctionInvokingChatClientsOnlyProcessOnce() + { + // Test that when multiple FunctionInvokingChatClients are in a pipeline, + // each FunctionCallContent is only processed once (by the first one that sees it) + var functionInvokedCount = 0; + var options = new ChatOptions + { + Tools = [AIFunctionFactory.Create(() => { functionInvokedCount++; return "Result 1"; }, "Func1")] + }; + + var callCount = 0; + using var innerClient = new TestChatClient + { + GetResponseAsyncCallback = async (contents, actualOptions, actualCancellationToken) => + { + await Task.Yield(); + + // First call: return a FunctionCallContent that needs processing + // Second call: return a final text response + if (callCount++ == 0) + { + return new ChatResponse(new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1")])); + } + else + { + return new ChatResponse(new ChatMessage(ChatRole.Assistant, "Done")); + } + } + }; + + // Create a pipeline with two FunctionInvokingChatClients + using var client1 = new FunctionInvokingChatClient(innerClient); + using var client2 = new FunctionInvokingChatClient(client1); + + var response = await client2.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); + + // The function should have been invoked EXACTLY ONCE, not twice (once per FICC) + Assert.Equal(1, functionInvokedCount); + + // The response should contain the FunctionCallContent with InvocationRequired = false + Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionCallContent fcc && fcc.CallId == "callId1" && !fcc.InvocationRequired)); + + // There should be a FunctionResultContent since the function was processed + Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId1")); + } + private sealed class CustomSynchronizationContext : SynchronizationContext { public override void Post(SendOrPostCallback d, object? state) @@ -1514,6 +1761,9 @@ private static async Task> InvokeAndAssertAsync( { Assert.NotEmpty(plan); + // Reset InvocationRequired for all FunctionCallContent in the plan to allow reuse + ResetPlanFunctionCallStates(plan); + configurePipeline ??= static b => b.UseFunctionInvocation(); using CancellationTokenSource cts = new(); @@ -1584,6 +1834,9 @@ private static async Task> InvokeAndAssertStreamingAsync( { Assert.NotEmpty(plan); + // Reset InvocationRequired for all FunctionCallContent in the plan to allow reuse + ResetPlanFunctionCallStates(plan); + configurePipeline ??= static b => b.UseFunctionInvocation(); using CancellationTokenSource cts = new(); @@ -1624,4 +1877,22 @@ private static async IAsyncEnumerable YieldAsync(params IEnumerable ite yield return item; } } + + /// + /// Resets InvocationRequired to true for all FunctionCallContent in the plan. + /// This is needed when reusing a plan across multiple test invocations. + /// + private static void ResetPlanFunctionCallStates(List plan) + { + foreach (var message in plan) + { + foreach (var content in message.Contents) + { + if (content is FunctionCallContent fcc) + { + fcc.InvocationRequired = true; + } + } + } + } } From 8fa1a6b5f54a063d77f2c90f8da82d9830a20c24 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 26 Jan 2026 20:35:29 +0000 Subject: [PATCH 3/6] Refactor tests per code review feedback - Removed redundant tests (InvocationRequired_DefaultsToTrue, InvocationRequired_CanBeSetToFalse) - Consolidated serialization tests into single theory with true/false parameters - Consolidated deserialization tests into single theory with true/false parameters - Consolidated roundtrip tests into single theory with true/false parameters - Replaced ResetPlanFunctionCallStates with CloneContents approach to avoid sharing mutable state - All 400 AI tests + 1118 abstraction tests passing Co-authored-by: jozkee <16040868+jozkee@users.noreply.github.com> --- .../Contents/FunctionCallContentTests.cs | 108 ++++-------------- .../FunctionInvokingChatClientTests.cs | 48 ++++---- 2 files changed, 44 insertions(+), 112 deletions(-) diff --git a/test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionCallContentTests.cs b/test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionCallContentTests.cs index c6dc821eb06..3b22a92e08d 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionCallContentTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionCallContentTests.cs @@ -77,33 +77,21 @@ public void Constructor_PropsRoundtrip() Assert.False(c.InvocationRequired); } - [Fact] - public void InvocationRequired_DefaultsToTrue() - { - FunctionCallContent c = new("callId1", "name"); - Assert.True(c.InvocationRequired); - } - - [Fact] - public void InvocationRequired_CanBeSetToFalse() - { - FunctionCallContent c = new("callId1", "name") { InvocationRequired = false }; - Assert.False(c.InvocationRequired); - } - - [Fact] - public void InvocationRequired_SerializedWhenFalse() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void InvocationRequired_Serialization(bool invocationRequired) { - // Arrange - Set InvocationRequired to false (to allow roundtrip, it must be serialized even when false) + // Arrange var sut = new FunctionCallContent("callId1", "functionName", new Dictionary { ["key"] = "value" }) { - InvocationRequired = false + InvocationRequired = invocationRequired }; // Act var json = JsonSerializer.SerializeToNode(sut, TestJsonSerializerContext.Default.Options); - // Assert - InvocationRequired should be in the JSON when it's false to allow roundtrip + // Assert - InvocationRequired should always be in the JSON (for roundtrip) Assert.NotNull(json); var jsonObj = json!.AsObject(); Assert.True(jsonObj.ContainsKey("invocationRequired") || jsonObj.ContainsKey("InvocationRequired")); @@ -119,61 +107,22 @@ public void InvocationRequired_SerializedWhenFalse() } Assert.NotNull(invocationRequiredValue); - Assert.False(invocationRequiredValue!.GetValue()); + Assert.Equal(invocationRequired, invocationRequiredValue!.GetValue()); } - [Fact] - public void InvocationRequired_SerializedWhenTrue() - { - // Arrange - InvocationRequired defaults to true - var sut = new FunctionCallContent("callId1", "functionName", new Dictionary { ["key"] = "value" }); - - // Act - var json = JsonSerializer.SerializeToNode(sut, TestJsonSerializerContext.Default.Options); - - // Assert - InvocationRequired should be in the JSON when it's true - Assert.NotNull(json); - var jsonObj = json!.AsObject(); - Assert.True(jsonObj.ContainsKey("invocationRequired") || jsonObj.ContainsKey("InvocationRequired")); - - JsonNode? invocationRequiredValue = null; - if (jsonObj.TryGetPropertyValue("invocationRequired", out var value1)) - { - invocationRequiredValue = value1; - } - else if (jsonObj.TryGetPropertyValue("InvocationRequired", out var value2)) - { - invocationRequiredValue = value2; - } - - Assert.NotNull(invocationRequiredValue); - Assert.True(invocationRequiredValue!.GetValue()); - } - - [Fact] - public void InvocationRequired_DeserializedCorrectlyWhenTrue() - { - // Test deserialization when InvocationRequired is true - var json = """{"callId":"callId1","name":"functionName","invocationRequired":true}"""; - var deserialized = JsonSerializer.Deserialize(json, TestJsonSerializerContext.Default.Options); - - Assert.NotNull(deserialized); - Assert.Equal("callId1", deserialized.CallId); - Assert.Equal("functionName", deserialized.Name); - Assert.True(deserialized.InvocationRequired); - } - - [Fact] - public void InvocationRequired_DeserializedCorrectlyWhenFalse() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void InvocationRequired_Deserialization(bool invocationRequired) { - // Test deserialization when InvocationRequired is false - var json = """{"callId":"callId1","name":"functionName","invocationRequired":false}"""; + // Test deserialization + var json = $$"""{"callId":"callId1","name":"functionName","invocationRequired":{{(invocationRequired ? "true" : "false")}}}"""; var deserialized = JsonSerializer.Deserialize(json, TestJsonSerializerContext.Default.Options); Assert.NotNull(deserialized); Assert.Equal("callId1", deserialized.CallId); Assert.Equal("functionName", deserialized.Name); - Assert.False(deserialized.InvocationRequired); + Assert.Equal(invocationRequired, deserialized.InvocationRequired); } [Fact] @@ -189,26 +138,13 @@ public void InvocationRequired_DeserializedToTrueWhenMissing() Assert.True(deserialized.InvocationRequired); } - [Fact] - public void InvocationRequired_RoundtripTrue() - { - // Test that InvocationRequired=true roundtrips correctly through JSON serialization - var original = new FunctionCallContent("callId1", "functionName") { InvocationRequired = true }; - var json = JsonSerializer.SerializeToNode(original, TestJsonSerializerContext.Default.Options); - var deserialized = JsonSerializer.Deserialize(json, TestJsonSerializerContext.Default.Options); - - Assert.NotNull(deserialized); - Assert.Equal(original.CallId, deserialized.CallId); - Assert.Equal(original.Name, deserialized.Name); - Assert.Equal(original.InvocationRequired, deserialized.InvocationRequired); - Assert.True(deserialized.InvocationRequired); - } - - [Fact] - public void InvocationRequired_RoundtripFalse() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void InvocationRequired_Roundtrip(bool invocationRequired) { - // Test that InvocationRequired=false roundtrips correctly through JSON serialization - var original = new FunctionCallContent("callId1", "functionName") { InvocationRequired = false }; + // Test that InvocationRequired roundtrips correctly through JSON serialization + var original = new FunctionCallContent("callId1", "functionName") { InvocationRequired = invocationRequired }; var json = JsonSerializer.SerializeToNode(original, TestJsonSerializerContext.Default.Options); var deserialized = JsonSerializer.Deserialize(json, TestJsonSerializerContext.Default.Options); @@ -216,7 +152,7 @@ public void InvocationRequired_RoundtripFalse() Assert.Equal(original.CallId, deserialized.CallId); Assert.Equal(original.Name, deserialized.Name); Assert.Equal(original.InvocationRequired, deserialized.InvocationRequired); - Assert.False(deserialized.InvocationRequired); + Assert.Equal(invocationRequired, deserialized.InvocationRequired); } [Fact] diff --git a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs index a7493f64ed8..a85b487e2d4 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs @@ -136,7 +136,6 @@ public async Task SupportsToolsProvidedByAdditionalTools(bool provideOptions) await InvokeAndAssertAsync(options, plan, configurePipeline: configure); - ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -170,7 +169,6 @@ public async Task PrefersToolsProvidedByChatOptions() await InvokeAndAssertAsync(options, plan, configurePipeline: configure); - ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -221,7 +219,6 @@ public async Task SupportsMultipleFunctionCallsPerRequestAsync(bool concurrentIn await InvokeAndAssertAsync(options, plan, configurePipeline: configure); - ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -270,7 +267,6 @@ public async Task ParallelFunctionCallsMayBeInvokedConcurrentlyAsync() await InvokeAndAssertAsync(options, plan, configurePipeline: configure); - ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -312,7 +308,6 @@ public async Task ConcurrentInvocationOfParallelCallsDisabledByDefaultAsync() await InvokeAndAssertAsync(options, plan); - ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan); } @@ -356,7 +351,6 @@ public async Task FunctionInvokerDelegateOverridesHandlingAsync() await InvokeAndAssertAsync(options, plan, configurePipeline: configure); - ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -568,7 +562,6 @@ public async Task KeepsFunctionCallingContent() #pragma warning disable SA1005, S125 Validate(await InvokeAndAssertAsync(options, plan)); - ResetPlanFunctionCallStates(plan); Validate(await InvokeAndAssertStreamingAsync(options, plan)); static void Validate(List finalChat) @@ -604,7 +597,6 @@ public async Task ExceptionDetailsOnlyReportedWhenRequestedAsync(bool detailedEr await InvokeAndAssertAsync(options, plan, configurePipeline: configure); - ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure); } @@ -1085,7 +1077,6 @@ public async Task FunctionInvocations_InvokedOnOriginalSynchronizationContext() .UseFunctionInvocation(configure: c => { c.AllowConcurrentInvocation = true; c.IncludeDetailedErrors = true; }); await InvokeAndAssertAsync(options, plan, configurePipeline: configurePipeline); - ResetPlanFunctionCallStates(plan); await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configurePipeline); } @@ -1119,7 +1110,6 @@ public async Task TerminateOnUnknownCalls_ControlsBehaviorForUnknownFunctions(bo ]; await InvokeAndAssertAsync(options, planForContinue, configurePipeline: configure); - ResetPlanFunctionCallStates(planForContinue); await InvokeAndAssertStreamingAsync(options, planForContinue, configurePipeline: configure); } else @@ -1761,9 +1751,6 @@ private static async Task> InvokeAndAssertAsync( { Assert.NotEmpty(plan); - // Reset InvocationRequired for all FunctionCallContent in the plan to allow reuse - ResetPlanFunctionCallStates(plan); - configurePipeline ??= static b => b.UseFunctionInvocation(); using CancellationTokenSource cts = new(); @@ -1781,7 +1768,7 @@ private static async Task> InvokeAndAssertAsync( var usage = CreateRandomUsage(); expectedTotalTokenCounts += usage.InputTokenCount!.Value; - var message = new ChatMessage(ChatRole.Assistant, [.. plan[contents.Count()].Contents]) + var message = new ChatMessage(ChatRole.Assistant, CloneContents(plan[contents.Count()].Contents)) { MessageId = Guid.NewGuid().ToString("N") }; @@ -1834,9 +1821,6 @@ private static async Task> InvokeAndAssertStreamingAsync( { Assert.NotEmpty(plan); - // Reset InvocationRequired for all FunctionCallContent in the plan to allow reuse - ResetPlanFunctionCallStates(plan); - configurePipeline ??= static b => b.UseFunctionInvocation(); using CancellationTokenSource cts = new(); @@ -1848,7 +1832,7 @@ private static async Task> InvokeAndAssertStreamingAsync( { Assert.Equal(cts.Token, actualCancellationToken); - ChatMessage message = new(ChatRole.Assistant, [.. plan[contents.Count()].Contents]) + ChatMessage message = new(ChatRole.Assistant, CloneContents(plan[contents.Count()].Contents)) { MessageId = Guid.NewGuid().ToString("N"), }; @@ -1879,20 +1863,32 @@ private static async IAsyncEnumerable YieldAsync(params IEnumerable ite } /// - /// Resets InvocationRequired to true for all FunctionCallContent in the plan. - /// This is needed when reusing a plan across multiple test invocations. + /// Clones AIContent objects to avoid sharing mutable state across test invocations. /// - private static void ResetPlanFunctionCallStates(List plan) + private static List CloneContents(IList contents) { - foreach (var message in plan) + var cloned = new List(contents.Count); + foreach (var content in contents) { - foreach (var content in message.Contents) + // Clone FunctionCallContent to avoid sharing InvocationRequired state + if (content is FunctionCallContent fcc) { - if (content is FunctionCallContent fcc) + cloned.Add(new FunctionCallContent(fcc.CallId, fcc.Name, fcc.Arguments) { - fcc.InvocationRequired = true; - } + InvocationRequired = fcc.InvocationRequired, + Exception = fcc.Exception, + AdditionalProperties = fcc.AdditionalProperties, + Annotations = fcc.Annotations, + RawRepresentation = fcc.RawRepresentation + }); + } + else + { + // For other content types, just use the same reference + cloned.Add(content); } } + + return cloned; } } From a4fe095d7a417021384f38ce57ebf5502fb3ce44 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 26 Jan 2026 23:33:34 +0000 Subject: [PATCH 4/6] Address final code review feedback - Changed InvocationRequired from "Experimental" to "Stable" in JSON file - Added Assert.True(c.InvocationRequired) to Constructor_PropsDefault test - Removed redundant InvocationRequired_InnerClientWithInvocationRequiredFalsePassesThroughUnprocessed test (covered by other test) - Converted InvocationRequired tests to theories with streaming parameter where applicable - Fixed static lambda issue in GenerateRejectedFunctionResults - All 428 AI tests + 1183 abstraction tests passing Co-authored-by: jozkee <16040868+jozkee@users.noreply.github.com> --- .../Microsoft.Extensions.AI.Abstractions.json | 2 +- .../FunctionInvokingChatClient.cs | 2 +- .../Contents/FunctionCallContentTests.cs | 1 + .../FunctionInvokingChatClientTests.cs | 145 ++++++++---------- 4 files changed, 67 insertions(+), 83 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json index f0c251772ad..2fd37a4bef7 100644 --- a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json +++ b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json @@ -1819,7 +1819,7 @@ }, { "Member": "bool Microsoft.Extensions.AI.FunctionCallContent.InvocationRequired { get; set; }", - "Stage": "Experimental" + "Stage": "Stable" }, { "Member": "string Microsoft.Extensions.AI.FunctionCallContent.Name { get; }", diff --git a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs index ff6efd3df4e..fa6f1d30d63 100644 --- a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs +++ b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs @@ -1554,7 +1554,7 @@ private static bool CurrentActivityIsInvokeAgent /// The for the rejected function calls. private List? GenerateRejectedFunctionResults(List? rejections) => rejections is { Count: > 0 } ? - rejections.ConvertAll(static m => + rejections.ConvertAll(m => { LogFunctionRejected(m.Response.FunctionCall.Name, m.Response.Reason); diff --git a/test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionCallContentTests.cs b/test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionCallContentTests.cs index 3b22a92e08d..394728b7236 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionCallContentTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionCallContentTests.cs @@ -28,6 +28,7 @@ public void Constructor_PropsDefault() Assert.Null(c.Arguments); Assert.Null(c.Exception); + Assert.True(c.InvocationRequired); } [Fact] diff --git a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs index 0e5617b3811..6118ffd375d 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs @@ -1692,8 +1692,10 @@ public async Task InvocationRequired_SetToFalseAfterProcessing() Assert.False(functionCallContent.InvocationRequired); } - [Fact] - public async Task InvocationRequired_IgnoresFunctionCallsWithInvocationRequiredFalse() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task InvocationRequired_IgnoresFunctionCallsWithInvocationRequiredFalse(bool streaming) { var functionInvokedCount = 0; var options = new ChatOptions @@ -1713,35 +1715,7 @@ public async Task InvocationRequired_IgnoresFunctionCallsWithInvocationRequiredF // Return a response with a FunctionCallContent that has InvocationRequired = false var message = new ChatMessage(ChatRole.Assistant, [alreadyProcessedFunctionCall]); return new ChatResponse(message); - } - }; - - using var client = new FunctionInvokingChatClient(innerClient); - - var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); - - // The function should not have been invoked since InvocationRequired was false - Assert.Equal(0, functionInvokedCount); - - // The response should contain the FunctionCallContent but no FunctionResultContent - Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionCallContent fcc && !fcc.InvocationRequired)); - Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent)); - } - - [Fact] - public async Task InvocationRequired_IgnoresFunctionCallsWithInvocationRequiredFalse_Streaming() - { - var functionInvokedCount = 0; - var options = new ChatOptions - { - Tools = [AIFunctionFactory.Create(() => { functionInvokedCount++; return "Result 1"; }, "Func1")] - }; - - // Create a function call that has already been processed - var alreadyProcessedFunctionCall = new FunctionCallContent("callId1", "Func1") { InvocationRequired = false }; - - using var innerClient = new TestChatClient - { + }, GetStreamingResponseAsyncCallback = (contents, actualOptions, actualCancellationToken) => { // Return a response with a FunctionCallContent that has InvocationRequired = false @@ -1752,22 +1726,38 @@ public async Task InvocationRequired_IgnoresFunctionCallsWithInvocationRequiredF using var client = new FunctionInvokingChatClient(innerClient); - var updates = new List(); - await foreach (var update in client.GetStreamingResponseAsync([new ChatMessage(ChatRole.User, "hello")], options)) + if (streaming) { - updates.Add(update); + var updates = new List(); + await foreach (var update in client.GetStreamingResponseAsync([new ChatMessage(ChatRole.User, "hello")], options)) + { + updates.Add(update); + } + + // The function should not have been invoked since InvocationRequired was false + Assert.Equal(0, functionInvokedCount); + + // The updates should contain the FunctionCallContent but no FunctionResultContent + Assert.Contains(updates, u => u.Contents.Any(c => c is FunctionCallContent fcc && !fcc.InvocationRequired)); + Assert.DoesNotContain(updates, u => u.Contents.Any(c => c is FunctionResultContent)); } + else + { + var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); - // The function should not have been invoked since InvocationRequired was false - Assert.Equal(0, functionInvokedCount); + // The function should not have been invoked since InvocationRequired was false + Assert.Equal(0, functionInvokedCount); - // The updates should contain the FunctionCallContent but no FunctionResultContent - Assert.Contains(updates, u => u.Contents.Any(c => c is FunctionCallContent fcc && !fcc.InvocationRequired)); - Assert.DoesNotContain(updates, u => u.Contents.Any(c => c is FunctionResultContent)); + // The response should contain the FunctionCallContent but no FunctionResultContent + Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionCallContent fcc && !fcc.InvocationRequired)); + Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent)); + } } - [Fact] - public async Task InvocationRequired_ProcessesMixedFunctionCalls() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task InvocationRequired_ProcessesMixedFunctionCalls(bool streaming) { var func1InvokedCount = 0; var func2InvokedCount = 0; @@ -1803,57 +1793,50 @@ public async Task InvocationRequired_ProcessesMixedFunctionCalls() var message = new ChatMessage(ChatRole.Assistant, "done"); return new ChatResponse(message); } + }, + GetStreamingResponseAsyncCallback = (contents, actualOptions, actualCancellationToken) => + { + if (contents.Count() == 1) + { + // First call - return both function calls + var message = new ChatMessage(ChatRole.Assistant, [needsProcessing, alreadyProcessed]); + return YieldAsync(new ChatResponse(message).ToChatResponseUpdates()); + } + else + { + // Second call - return final response after processing + var message = new ChatMessage(ChatRole.Assistant, "done"); + return YieldAsync(new ChatResponse(message).ToChatResponseUpdates()); + } } }; using var client = new FunctionInvokingChatClient(innerClient); - var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); - - // Only Func1 should have been invoked (the one with InvocationRequired = true) - Assert.Equal(1, func1InvokedCount); - Assert.Equal(0, func2InvokedCount); - - // The response should contain FunctionResultContent for Func1 but not Func2 - Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId1")); - Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId2")); - } - - [Fact] - public async Task InvocationRequired_InnerClientWithInvocationRequiredFalsePassesThroughUnprocessed() - { - // Test that FunctionCallContent with InvocationRequired=false from inner client passes through without being processed - var functionInvokedCount = 0; - var options = new ChatOptions + if (streaming) { - Tools = [AIFunctionFactory.Create(() => { functionInvokedCount++; return "Result 1"; }, "Func1")] - }; + var updates = await client.GetStreamingResponseAsync([new ChatMessage(ChatRole.User, "hello")], options).ToChatResponseAsync(); - var functionCallWithInvocationRequiredFalse = new FunctionCallContent("callId1", "Func1") { InvocationRequired = false }; + // Only Func1 should have been invoked (the one with InvocationRequired = true) + Assert.Equal(1, func1InvokedCount); + Assert.Equal(0, func2InvokedCount); - using var innerClient = new TestChatClient + // The response should contain FunctionResultContent for Func1 but not Func2 + Assert.Contains(updates.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId1")); + Assert.DoesNotContain(updates.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId2")); + } + else { - GetResponseAsyncCallback = async (contents, actualOptions, actualCancellationToken) => - { - await Task.Yield(); + var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); - // Inner client returns a FunctionCallContent with InvocationRequired = false - return new ChatResponse(new ChatMessage(ChatRole.Assistant, [functionCallWithInvocationRequiredFalse])); - } - }; - - using var client = new FunctionInvokingChatClient(innerClient); - - var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); + // Only Func1 should have been invoked (the one with InvocationRequired = true) + Assert.Equal(1, func1InvokedCount); + Assert.Equal(0, func2InvokedCount); - // The function should NOT have been invoked since InvocationRequired was false - Assert.Equal(0, functionInvokedCount); - - // The response should contain the FunctionCallContent with InvocationRequired = false - Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionCallContent fcc && fcc.CallId == "callId1" && !fcc.InvocationRequired)); - - // There should be NO FunctionResultContent since we didn't process the function call - Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent)); + // The response should contain FunctionResultContent for Func1 but not Func2 + Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId1")); + Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId2")); + } } [Fact] From 8dbc3232260ca352d5a43ed0d28e0e245c58169e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 00:20:49 +0000 Subject: [PATCH 5/6] Address additional code review feedback - Removed redundant InvocationRequired = false assignment in CreateResponseMessages (already set in ProcessFunctionCallAsync) - Refactored InvocationRequired tests to use ternary operator with ToChatResponseAsync() to reduce code duplication - Added test in FunctionInvokingChatClientApprovalsTests to verify FunctionCallContent with InvocationRequired=false are not replaced with approval requests - Updated approval logic to skip FunctionCallContent where InvocationRequired=false - All 430 AI tests + 1183 abstraction tests passing Co-authored-by: jozkee <16040868+jozkee@users.noreply.github.com> --- .../FunctionInvokingChatClient.cs | 7 +-- ...unctionInvokingChatClientApprovalsTests.cs | 43 +++++++++++++ .../FunctionInvokingChatClientTests.cs | 63 +++++-------------- 3 files changed, 62 insertions(+), 51 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs index fa6f1d30d63..a6e200abafc 100644 --- a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs +++ b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs @@ -1230,9 +1230,6 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul functionResult = message; } - // Mark the function call as having been processed - result.CallContent.InvocationRequired = false; - return new FunctionResultContent(result.CallContent.CallId, functionResult) { Exception = result.Exception }; } } @@ -1704,7 +1701,7 @@ private static bool TryReplaceFunctionCallsWithApprovalRequests(IList { for (int i = 0; i < content.Count; i++) { - if (content[i] is FunctionCallContent fcc) + if (content[i] is FunctionCallContent fcc && fcc.InvocationRequired) { updatedContent ??= [.. content]; // Clone the list if we haven't already updatedContent[i] = new FunctionApprovalRequestContent(fcc.CallId, fcc); @@ -1735,7 +1732,7 @@ private IList ReplaceFunctionCallsWithApprovalRequests( var content = messages[i].Contents; for (int j = 0; j < content.Count; j++) { - if (content[j] is FunctionCallContent functionCall) + if (content[j] is FunctionCallContent functionCall && functionCall.InvocationRequired) { (allFunctionCallContentIndices ??= []).Add((i, j)); diff --git a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs index 01f2e111447..dd77ef370c6 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs @@ -1113,6 +1113,49 @@ async IAsyncEnumerable YieldInnerClientUpdates( } } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task FunctionCallsWithInvocationRequiredFalseAreNotReplacedWithApprovalsAsync(bool streaming) + { + var functionInvokedCount = 0; + var options = new ChatOptions + { + Tools = + [ + new ApprovalRequiredAIFunction( + AIFunctionFactory.Create(() => { functionInvokedCount++; return "Result 1"; }, "Func1")), + ] + }; + + List input = [new ChatMessage(ChatRole.User, "hello")]; + + // FunctionCallContent with InvocationRequired = false should pass through unchanged + var alreadyProcessedFunctionCall = new FunctionCallContent("callId1", "Func1") { InvocationRequired = false }; + List downstreamClientOutput = + [ + new ChatMessage(ChatRole.Assistant, [alreadyProcessedFunctionCall]), + ]; + + // Expected output should contain the same FunctionCallContent, not a FunctionApprovalRequestContent + List expectedOutput = + [ + new ChatMessage(ChatRole.Assistant, [alreadyProcessedFunctionCall]), + ]; + + if (streaming) + { + await InvokeAndAssertStreamingAsync(options, input, downstreamClientOutput, expectedOutput); + } + else + { + await InvokeAndAssertAsync(options, input, downstreamClientOutput, expectedOutput); + } + + // The function should NOT have been invoked since InvocationRequired was false + Assert.Equal(0, functionInvokedCount); + } + private static Task> InvokeAndAssertAsync( ChatOptions? options, List input, diff --git a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs index 6118ffd375d..53a197847db 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs @@ -1726,32 +1726,16 @@ public async Task InvocationRequired_IgnoresFunctionCallsWithInvocationRequiredF using var client = new FunctionInvokingChatClient(innerClient); - if (streaming) - { - var updates = new List(); - await foreach (var update in client.GetStreamingResponseAsync([new ChatMessage(ChatRole.User, "hello")], options)) - { - updates.Add(update); - } - - // The function should not have been invoked since InvocationRequired was false - Assert.Equal(0, functionInvokedCount); + ChatResponse response = streaming + ? await client.GetStreamingResponseAsync([new ChatMessage(ChatRole.User, "hello")], options).ToChatResponseAsync() + : await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); - // The updates should contain the FunctionCallContent but no FunctionResultContent - Assert.Contains(updates, u => u.Contents.Any(c => c is FunctionCallContent fcc && !fcc.InvocationRequired)); - Assert.DoesNotContain(updates, u => u.Contents.Any(c => c is FunctionResultContent)); - } - else - { - var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); + // The function should not have been invoked since InvocationRequired was false + Assert.Equal(0, functionInvokedCount); - // The function should not have been invoked since InvocationRequired was false - Assert.Equal(0, functionInvokedCount); - - // The response should contain the FunctionCallContent but no FunctionResultContent - Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionCallContent fcc && !fcc.InvocationRequired)); - Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent)); - } + // The response should contain the FunctionCallContent but no FunctionResultContent + Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionCallContent fcc && !fcc.InvocationRequired)); + Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent)); } [Theory] @@ -1813,30 +1797,17 @@ public async Task InvocationRequired_ProcessesMixedFunctionCalls(bool streaming) using var client = new FunctionInvokingChatClient(innerClient); - if (streaming) - { - var updates = await client.GetStreamingResponseAsync([new ChatMessage(ChatRole.User, "hello")], options).ToChatResponseAsync(); + ChatResponse response = streaming + ? await client.GetStreamingResponseAsync([new ChatMessage(ChatRole.User, "hello")], options).ToChatResponseAsync() + : await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); - // Only Func1 should have been invoked (the one with InvocationRequired = true) - Assert.Equal(1, func1InvokedCount); - Assert.Equal(0, func2InvokedCount); + // Only Func1 should have been invoked (the one with InvocationRequired = true) + Assert.Equal(1, func1InvokedCount); + Assert.Equal(0, func2InvokedCount); - // The response should contain FunctionResultContent for Func1 but not Func2 - Assert.Contains(updates.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId1")); - Assert.DoesNotContain(updates.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId2")); - } - else - { - var response = await client.GetResponseAsync([new ChatMessage(ChatRole.User, "hello")], options); - - // Only Func1 should have been invoked (the one with InvocationRequired = true) - Assert.Equal(1, func1InvokedCount); - Assert.Equal(0, func2InvokedCount); - - // The response should contain FunctionResultContent for Func1 but not Func2 - Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId1")); - Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId2")); - } + // The response should contain FunctionResultContent for Func1 but not Func2 + Assert.Contains(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId1")); + Assert.DoesNotContain(response.Messages, m => m.Contents.Any(c => c is FunctionResultContent frc && frc.CallId == "callId2")); } [Fact] From 3478f3c31602e1fbd8c7fbee04467bee01986832 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 29 Jan 2026 09:41:18 -0500 Subject: [PATCH 6/6] Update src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionCallContent.cs --- .../Contents/FunctionCallContent.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionCallContent.cs b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionCallContent.cs index 494fb5589df..b8a135b7e1d 100644 --- a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionCallContent.cs +++ b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionCallContent.cs @@ -61,8 +61,8 @@ public FunctionCallContent(string callId, string name, IDictionary /// /// This property defaults to , indicating that the function call should be processed. - /// When set to , it indicates that the function has already been processed and - /// should be ignored by components that process function calls. + /// When set to , it indicates that the function has already been processed or is otherwise + /// purely informational and should be ignored by components that process function calls. /// public bool InvocationRequired { get; set; } = true;