Skip to content

Commit 84713fa

Browse files
Copilotstephentoub
andauthored
Add logging to FunctionInvokingChatClient for approval flow, error handling, and loop control (#7228)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent e69705c commit 84713fa

File tree

2 files changed

+270
-5
lines changed

2 files changed

+270
-5
lines changed

src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#pragma warning disable CA2213 // Disposable fields should be disposed
1818
#pragma warning disable S2219 // Runtime type checking should be simplified
1919
#pragma warning disable S3353 // Unchanged local variables should be "const"
20+
#pragma warning disable SA1204 // Static members should appear before non-static members
2021

2122
namespace Microsoft.Extensions.AI;
2223

@@ -934,6 +935,15 @@ private bool ShouldTerminateLoopBasedOnHandleableFunctions(List<FunctionCallCont
934935
// There are functions to call but we have no tools, so we can't handle them.
935936
// If we're configured to terminate on unknown call requests, do so now.
936937
// Otherwise, ProcessFunctionCallsAsync will handle it by creating a NotFound response message.
938+
if (TerminateOnUnknownCalls)
939+
{
940+
// Log each function call that would cause termination
941+
foreach (var fcc in functionCalls)
942+
{
943+
LogFunctionNotFound(fcc.Name);
944+
}
945+
}
946+
937947
return TerminateOnUnknownCalls;
938948
}
939949

@@ -948,6 +958,7 @@ private bool ShouldTerminateLoopBasedOnHandleableFunctions(List<FunctionCallCont
948958
{
949959
// The tool was found but it's not invocable. Regardless of TerminateOnUnknownCallRequests,
950960
// we need to break out of the loop so that callers can handle all the call requests.
961+
LogNonInvocableFunction(fcc.Name);
951962
return true;
952963
}
953964
}
@@ -958,6 +969,7 @@ private bool ShouldTerminateLoopBasedOnHandleableFunctions(List<FunctionCallCont
958969
// creating a NotFound response message.
959970
if (TerminateOnUnknownCalls)
960971
{
972+
LogFunctionNotFound(fcc.Name);
961973
return true;
962974
}
963975
}
@@ -1063,6 +1075,8 @@ private void UpdateConsecutiveErrorCountOrThrow(IList<ChatMessage> added, ref in
10631075
consecutiveErrorCount++;
10641076
if (consecutiveErrorCount > MaximumConsecutiveErrorsPerRequest)
10651077
{
1078+
LogMaxConsecutiveErrorsExceeded(MaximumConsecutiveErrorsPerRequest);
1079+
10661080
var allExceptionsArray = added
10671081
.SelectMany(m => m.Contents.OfType<FunctionResultContent>())
10681082
.Select(frc => frc.Exception!)
@@ -1115,6 +1129,15 @@ private async Task<FunctionInvocationResult> ProcessFunctionCallAsync(
11151129
AIFunctionDeclaration? tool = FindTool(callContent.Name, options?.Tools, AdditionalTools);
11161130
if (tool is not AIFunction aiFunction)
11171131
{
1132+
if (tool is null)
1133+
{
1134+
LogFunctionNotFound(callContent.Name);
1135+
}
1136+
else
1137+
{
1138+
LogNonInvocableFunction(callContent.Name);
1139+
}
1140+
11181141
return new(terminate: false, FunctionInvocationStatus.NotFound, callContent, result: null, exception: null);
11191142
}
11201143

@@ -1151,6 +1174,11 @@ private async Task<FunctionInvocationResult> ProcessFunctionCallAsync(
11511174
exception: e);
11521175
}
11531176

1177+
if (context.Terminate)
1178+
{
1179+
LogFunctionRequestedTermination(callContent.Name);
1180+
}
1181+
11541182
return new(
11551183
terminate: context.Terminate,
11561184
FunctionInvocationStatus.RanToCompletion,
@@ -1345,10 +1373,10 @@ private static bool CurrentActivityIsInvokeAgent
13451373
/// <summary>
13461374
/// 1. Remove all <see cref="FunctionApprovalRequestContent"/> and <see cref="FunctionApprovalResponseContent"/> from the <paramref name="originalMessages"/>.
13471375
/// 2. Recreate <see cref="FunctionCallContent"/> for any <see cref="FunctionApprovalResponseContent"/> that haven't been executed yet.
1348-
/// 3. Genreate failed <see cref="FunctionResultContent"/> for any rejected <see cref="FunctionApprovalResponseContent"/>.
1376+
/// 3. Generate failed <see cref="FunctionResultContent"/> for any rejected <see cref="FunctionApprovalResponseContent"/>.
13491377
/// 4. add all the new content items to <paramref name="originalMessages"/> and return them as the pre-invocation history.
13501378
/// </summary>
1351-
private static (List<ChatMessage>? preDownstreamCallHistory, List<ApprovalResultWithRequestMessage>? approvals) ProcessFunctionApprovalResponses(
1379+
private (List<ChatMessage>? preDownstreamCallHistory, List<ApprovalResultWithRequestMessage>? approvals) ProcessFunctionApprovalResponses(
13521380
List<ChatMessage> originalMessages, bool hasConversationId, string? toolMessageId, string? functionCallContentFallbackMessageId)
13531381
{
13541382
// Extract any approval responses where we need to execute or reject the function calls.
@@ -1399,7 +1427,7 @@ private static (List<ChatMessage>? preDownstreamCallHistory, List<ApprovalResult
13991427
/// We can then use the metadata from these messages when we re-create the FunctionCallContent messages/updates to return to the caller. This way, when we finally do return
14001428
/// the FuncionCallContent to users it's part of a message/update that contains the same metadata as originally returned to the downstream service.
14011429
/// </remarks>
1402-
private static (List<ApprovalResultWithRequestMessage>? approvals, List<ApprovalResultWithRequestMessage>? rejections) ExtractAndRemoveApprovalRequestsAndResponses(
1430+
private (List<ApprovalResultWithRequestMessage>? approvals, List<ApprovalResultWithRequestMessage>? rejections) ExtractAndRemoveApprovalRequestsAndResponses(
14031431
List<ChatMessage> messages)
14041432
{
14051433
Dictionary<string, ChatMessage>? allApprovalRequestsMessages = null;
@@ -1498,6 +1526,8 @@ private static (List<ApprovalResultWithRequestMessage>? approvals, List<Approval
14981526
continue;
14991527
}
15001528

1529+
LogProcessingApprovalResponse(approvalResponse.FunctionCall.Name, approvalResponse.Approved);
1530+
15011531
// Split the responses into approved and rejected.
15021532
ref List<ApprovalResultWithRequestMessage>? targetList = ref approvalResponse.Approved ? ref approvedFunctionCalls : ref rejectedFunctionCalls;
15031533

@@ -1516,10 +1546,12 @@ private static (List<ApprovalResultWithRequestMessage>? approvals, List<Approval
15161546
/// </summary>
15171547
/// <param name="rejections">Any rejected approval responses.</param>
15181548
/// <returns>The <see cref="AIContent"/> for the rejected function calls.</returns>
1519-
private static List<AIContent>? GenerateRejectedFunctionResults(List<ApprovalResultWithRequestMessage>? rejections) =>
1549+
private List<AIContent>? GenerateRejectedFunctionResults(List<ApprovalResultWithRequestMessage>? rejections) =>
15201550
rejections is { Count: > 0 } ?
15211551
rejections.ConvertAll(m =>
15221552
{
1553+
LogFunctionRejected(m.Response.FunctionCall.Name, m.Response.Reason);
1554+
15231555
string result = "Tool call invocation rejected.";
15241556
if (!string.IsNullOrWhiteSpace(m.Response.Reason))
15251557
{
@@ -1679,7 +1711,7 @@ private static bool TryReplaceFunctionCallsWithApprovalRequests(IList<AIContent>
16791711
/// Replaces all <see cref="FunctionCallContent"/> from <paramref name="messages"/> with <see cref="FunctionApprovalRequestContent"/>
16801712
/// if any one of them requires approval.
16811713
/// </summary>
1682-
private static IList<ChatMessage> ReplaceFunctionCallsWithApprovalRequests(
1714+
private IList<ChatMessage> ReplaceFunctionCallsWithApprovalRequests(
16831715
IList<ChatMessage> messages,
16841716
params ReadOnlySpan<IList<AITool>?> toolLists)
16851717
{
@@ -1721,6 +1753,7 @@ private static IList<ChatMessage> ReplaceFunctionCallsWithApprovalRequests(
17211753
message.Contents = [.. message.Contents];
17221754

17231755
var functionCall = (FunctionCallContent)message.Contents[contentIndex];
1756+
LogFunctionRequiresApproval(functionCall.Name);
17241757
message.Contents[contentIndex] = new FunctionApprovalRequestContent(functionCall.CallId, functionCall);
17251758
outputMessages[messageIndex] = message;
17261759

@@ -1785,6 +1818,27 @@ private static TimeSpan GetElapsedTime(long startingTimestamp) =>
17851818
[LoggerMessage(LogLevel.Debug, "Reached maximum iteration count of {MaximumIterationsPerRequest}. Stopping function invocation loop.")]
17861819
private partial void LogMaximumIterationsReached(int maximumIterationsPerRequest);
17871820

1821+
[LoggerMessage(LogLevel.Debug, "Function '{FunctionName}' requires approval. Converting to approval request.")]
1822+
private partial void LogFunctionRequiresApproval(string functionName);
1823+
1824+
[LoggerMessage(LogLevel.Debug, "Processing approval response for '{FunctionName}'. Approved: {Approved}")]
1825+
private partial void LogProcessingApprovalResponse(string functionName, bool approved);
1826+
1827+
[LoggerMessage(LogLevel.Debug, "Function '{FunctionName}' was rejected. Reason: {Reason}")]
1828+
private partial void LogFunctionRejected(string functionName, string? reason);
1829+
1830+
[LoggerMessage(LogLevel.Warning, "Maximum consecutive errors ({MaxErrors}) exceeded. Throwing aggregated exceptions.")]
1831+
private partial void LogMaxConsecutiveErrorsExceeded(int maxErrors);
1832+
1833+
[LoggerMessage(LogLevel.Warning, "Function '{FunctionName}' not found.")]
1834+
private partial void LogFunctionNotFound(string functionName);
1835+
1836+
[LoggerMessage(LogLevel.Debug, "Function '{FunctionName}' is not invocable (declaration only). Terminating loop.")]
1837+
private partial void LogNonInvocableFunction(string functionName);
1838+
1839+
[LoggerMessage(LogLevel.Debug, "Function '{FunctionName}' requested termination of the processing loop.")]
1840+
private partial void LogFunctionRequestedTermination(string functionName);
1841+
17881842
/// <summary>Provides information about the invocation of a function call.</summary>
17891843
public sealed class FunctionInvocationResult
17901844
{

test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2692,4 +2692,215 @@ public async Task RespectsChatOptionsToolsModificationsByFunctionTool_ReplaceWit
26922692

26932693
Assert.Equal(2, callCount);
26942694
}
2695+
2696+
[Fact]
2697+
public async Task LogsFunctionNotFound()
2698+
{
2699+
var collector = new FakeLogCollector();
2700+
ServiceCollection c = new();
2701+
c.AddLogging(b => b.AddProvider(new FakeLoggerProvider(collector)).SetMinimumLevel(LogLevel.Debug));
2702+
2703+
var options = new ChatOptions
2704+
{
2705+
Tools = [AIFunctionFactory.Create(() => "Result 1", "Func1")]
2706+
};
2707+
2708+
List<ChatMessage> plan =
2709+
[
2710+
new ChatMessage(ChatRole.User, "hello"),
2711+
new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "UnknownFunc")]),
2712+
new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Error: Requested function \"UnknownFunc\" not found.")]),
2713+
new ChatMessage(ChatRole.Assistant, "world"),
2714+
];
2715+
2716+
Func<ChatClientBuilder, ChatClientBuilder> configure = b =>
2717+
b.Use((c, services) => new FunctionInvokingChatClient(c, services.GetRequiredService<ILoggerFactory>()));
2718+
2719+
await InvokeAndAssertAsync(options, plan, null, configure, c.BuildServiceProvider());
2720+
2721+
var logs = collector.GetSnapshot();
2722+
Assert.Contains(logs, e => e.Message.Contains("Function 'UnknownFunc' not found") && e.Level == LogLevel.Warning);
2723+
}
2724+
2725+
[Fact]
2726+
public async Task LogsNonInvocableFunction()
2727+
{
2728+
var collector = new FakeLogCollector();
2729+
ServiceCollection c = new();
2730+
c.AddLogging(b => b.AddProvider(new FakeLoggerProvider(collector)).SetMinimumLevel(LogLevel.Debug));
2731+
2732+
var declarationOnly = AIFunctionFactory.Create(() => "Result 1", "Func1").AsDeclarationOnly();
2733+
var options = new ChatOptions
2734+
{
2735+
Tools = [declarationOnly]
2736+
};
2737+
2738+
List<ChatMessage> plan =
2739+
[
2740+
new ChatMessage(ChatRole.User, "hello"),
2741+
new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1")]),
2742+
new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Should not be produced")]),
2743+
new ChatMessage(ChatRole.Assistant, "world"),
2744+
];
2745+
2746+
List<ChatMessage> expected = plan.Take(2).ToList();
2747+
2748+
Func<ChatClientBuilder, ChatClientBuilder> configure = b =>
2749+
b.Use((c, services) => new FunctionInvokingChatClient(c, services.GetRequiredService<ILoggerFactory>()));
2750+
2751+
await InvokeAndAssertAsync(options, plan, expected, configure, c.BuildServiceProvider());
2752+
2753+
var logs = collector.GetSnapshot();
2754+
Assert.Contains(logs, e => e.Message.Contains("Function 'Func1' is not invocable (declaration only)") && e.Level == LogLevel.Debug);
2755+
}
2756+
2757+
[Fact]
2758+
public async Task LogsFunctionRequestedTermination()
2759+
{
2760+
var collector = new FakeLogCollector();
2761+
ServiceCollection c = new();
2762+
c.AddLogging(b => b.AddProvider(new FakeLoggerProvider(collector)).SetMinimumLevel(LogLevel.Debug));
2763+
2764+
var options = new ChatOptions
2765+
{
2766+
Tools = [AIFunctionFactory.Create(() =>
2767+
{
2768+
var context = FunctionInvokingChatClient.CurrentContext!;
2769+
context.Terminate = true;
2770+
return "Terminated";
2771+
}, "TerminatingFunc")]
2772+
};
2773+
2774+
List<ChatMessage> plan =
2775+
[
2776+
new ChatMessage(ChatRole.User, "hello"),
2777+
new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "TerminatingFunc")]),
2778+
new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Terminated")]),
2779+
];
2780+
2781+
Func<ChatClientBuilder, ChatClientBuilder> configure = b =>
2782+
b.Use((c, services) => new FunctionInvokingChatClient(c, services.GetRequiredService<ILoggerFactory>()));
2783+
2784+
await InvokeAndAssertAsync(options, plan, null, configure, c.BuildServiceProvider());
2785+
2786+
var logs = collector.GetSnapshot();
2787+
Assert.Contains(logs, e => e.Message.Contains("Function 'TerminatingFunc' requested termination of the processing loop") && e.Level == LogLevel.Debug);
2788+
}
2789+
2790+
[Fact]
2791+
public async Task LogsFunctionRequiresApproval()
2792+
{
2793+
var collector = new FakeLogCollector();
2794+
ServiceCollection c = new();
2795+
c.AddLogging(b => b.AddProvider(new FakeLoggerProvider(collector)).SetMinimumLevel(LogLevel.Debug));
2796+
2797+
var approvalFunc = new ApprovalRequiredAIFunction(AIFunctionFactory.Create(() => "Result 1", "Func1"));
2798+
var options = new ChatOptions
2799+
{
2800+
Tools = [approvalFunc]
2801+
};
2802+
2803+
List<ChatMessage> plan =
2804+
[
2805+
new ChatMessage(ChatRole.User, "hello"),
2806+
new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1")]),
2807+
];
2808+
2809+
// Expected output includes the user message and the approval request
2810+
List<ChatMessage> expected =
2811+
[
2812+
new ChatMessage(ChatRole.User, "hello"),
2813+
new ChatMessage(ChatRole.Assistant,
2814+
[
2815+
new FunctionApprovalRequestContent("callId1", new FunctionCallContent("callId1", "Func1"))
2816+
])
2817+
];
2818+
2819+
Func<ChatClientBuilder, ChatClientBuilder> configure = b =>
2820+
b.Use((c, services) => new FunctionInvokingChatClient(c, services.GetRequiredService<ILoggerFactory>()));
2821+
2822+
await InvokeAndAssertAsync(options, plan, expected, configure, c.BuildServiceProvider());
2823+
2824+
var logs = collector.GetSnapshot();
2825+
Assert.Contains(logs, e => e.Message.Contains("Function 'Func1' requires approval") && e.Level == LogLevel.Debug);
2826+
}
2827+
2828+
[Fact]
2829+
public async Task LogsProcessingApprovalResponse()
2830+
{
2831+
var collector = new FakeLogCollector();
2832+
using var loggerFactory = LoggerFactory.Create(b => b.AddProvider(new FakeLoggerProvider(collector)).SetMinimumLevel(LogLevel.Debug));
2833+
2834+
var approvalFunc = new ApprovalRequiredAIFunction(AIFunctionFactory.Create(() => "Result 1", "Func1"));
2835+
2836+
using var innerClient = new TestChatClient
2837+
{
2838+
GetResponseAsyncCallback = (messages, opts, ct) =>
2839+
Task.FromResult(new ChatResponse(new ChatMessage(ChatRole.Assistant, "world")))
2840+
};
2841+
2842+
using var client = new FunctionInvokingChatClient(innerClient, loggerFactory);
2843+
2844+
var options = new ChatOptions { Tools = [approvalFunc] };
2845+
2846+
var messages = new List<ChatMessage>
2847+
{
2848+
new ChatMessage(ChatRole.User, "hello"),
2849+
new ChatMessage(ChatRole.Assistant,
2850+
[
2851+
new FunctionApprovalRequestContent("callId1", new FunctionCallContent("callId1", "Func1"))
2852+
]),
2853+
new ChatMessage(ChatRole.User,
2854+
[
2855+
new FunctionApprovalResponseContent("callId1", true, new FunctionCallContent("callId1", "Func1"))
2856+
])
2857+
};
2858+
2859+
await client.GetResponseAsync(messages, options);
2860+
2861+
var logs = collector.GetSnapshot();
2862+
Assert.Contains(logs, e => e.Message.Contains("Processing approval response for 'Func1'. Approved: True") && e.Level == LogLevel.Debug);
2863+
}
2864+
2865+
[Fact]
2866+
public async Task LogsFunctionRejected()
2867+
{
2868+
var collector = new FakeLogCollector();
2869+
using var loggerFactory = LoggerFactory.Create(b => b.AddProvider(new FakeLoggerProvider(collector)).SetMinimumLevel(LogLevel.Debug));
2870+
2871+
var approvalFunc = new ApprovalRequiredAIFunction(AIFunctionFactory.Create(() => "Result 1", "Func1"));
2872+
2873+
using var innerClient = new TestChatClient
2874+
{
2875+
GetResponseAsyncCallback = (messages, opts, ct) =>
2876+
Task.FromResult(new ChatResponse(new ChatMessage(ChatRole.Assistant, "world")))
2877+
};
2878+
2879+
using var client = new FunctionInvokingChatClient(innerClient, loggerFactory);
2880+
2881+
var options = new ChatOptions { Tools = [approvalFunc] };
2882+
2883+
var messages = new List<ChatMessage>
2884+
{
2885+
new ChatMessage(ChatRole.User, "hello"),
2886+
new ChatMessage(ChatRole.Assistant,
2887+
[
2888+
new FunctionApprovalRequestContent("callId1", new FunctionCallContent("callId1", "Func1"))
2889+
]),
2890+
new ChatMessage(ChatRole.User,
2891+
[
2892+
new FunctionApprovalResponseContent("callId1", false, new FunctionCallContent("callId1", "Func1")) { Reason = "User denied" }
2893+
])
2894+
};
2895+
2896+
await client.GetResponseAsync(messages, options);
2897+
2898+
var logs = collector.GetSnapshot();
2899+
Assert.Contains(logs, e => e.Message.Contains("Function 'Func1' was rejected. Reason: User denied") && e.Level == LogLevel.Debug);
2900+
}
2901+
2902+
// Note: LogMaxConsecutiveErrorsExceeded is exercised by the existing
2903+
// ContinuesWithFailingCallsUntilMaximumConsecutiveErrors test which triggers
2904+
// the threshold condition. The logging call is at line 1078 and will execute
2905+
// when MaximumConsecutiveErrorsPerRequest is exceeded.
26952906
}

0 commit comments

Comments
 (0)