extensions icon indicating copy to clipboard operation
extensions copied to clipboard
verified publisher icon dotnet

Add Reason property to FunctionApprovalResponseContent for custom rejection messages

Open Copilot opened this issue 3 weeks ago • 11 comments

Implementation: Add Reason property to FunctionApprovalResponseContent

Changes Completed

  • [x] Add nullable Reason property to FunctionApprovalResponseContent class

    • Added "optional" to property documentation as suggested
    • Removed verbose <remarks> section for cleaner documentation

  • [x] Update FunctionInvokingChatClient.GenerateRejectedFunctionResults method

    • Changed rejection message format to always include "Tool call invocation rejected."
    • Appends custom reason when provided: "Tool call invocation rejected. {Reason}"
    • Uses IsNullOrWhiteSpace check as suggested

  • [x] Update all tests for new message format

    • Updated default rejection message expectations
    • Updated custom reason expectations to include prefix
    • All FunctionInvokingChatClientApprovalsTests pass across .NET 8, 9, and 10

  • [x] Add serialization tests for Reason property

    • Updated FunctionApprovalResponseContentTests.Serialization_Roundtrips to test with both null and custom reason
    • Ensures Reason property correctly serializes and deserializes through JSON

  • [x] Add edge case tests for Reason property

    • Added RejectedApprovalResponsesWithEmptyOrWhitespaceReasonUsesDefaultMessageAsync test
    • Tests null, empty string, and whitespace-only values
    • Validates that IsNullOrWhiteSpace logic treats all as default message

  • [x] Add optional reason parameter to FunctionApprovalRequestContent.CreateResponse

    • Added string? reason = null parameter to the method
    • Sets the Reason property on the created FunctionApprovalResponseContent
    • Added comprehensive tests covering both approved/rejected with/without reasons
    • Provides a convenient way to set the reason during response creation

  • [x] Add API compatibility suppressions

    • Added suppressions in CompatibilitySuppressions.xml for the method signature change
    • Suppressed CP0002 errors for net462, net8.0, net9.0, and netstandard2.0 frameworks
    • Fixes CI build failures related to breaking API changes

  • [x] Build and test validation

    • All builds passed successfully (Debug configuration)
    • All 22 FunctionInvokingChatClientApprovalsTests pass
    • All 6 FunctionApprovalResponseContentTests pass
    • All 11 FunctionApprovalRequestContentTests pass
    • API compatibility checks pass with suppressions

  • [x] Code review feedback addressed

    • Simplified property documentation
    • Improved rejection message format to avoid ambiguity
    • Added comprehensive test coverage per reviewer suggestions
    • Added convenience method parameter for setting reason
    • Fixed CI build failures with proper API compatibility suppressions

Summary

All PR review feedback has been addressed. The rejection message now clearly indicates it's a rejection while allowing custom context to be appended. The implementation includes proper serialization support, comprehensive edge case testing, and a convenient way to set the reason via the CreateResponse method. API compatibility suppressions have been added to handle the method signature change.

Original prompt

This section details on the original issue you should resolve

<issue_title>Provide ability to specify reject message for FunctionInvokingChatClient</issue_title> <issue_description>The reject message for FunctionApprovalResponseContent with Approved = false is currently a hard-coded string:

"Error: Tool call invocation was rejected by user."

However, this situation is not necessarily an error. It should be possible to provide a custom rejection message, allowing callers to specify the actual reason for rejection.

Relevant code: https://github.com/dotnet/extensions/blob/cc328a764d1b0200a852772b1f163ebecc7f4a22/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs#L1421 </issue_description>

<agent_instructions>Please add a nullable string Reason get/set property to FunctionApprovalResponseContent. If such a reason is set, FunctionInvokingChatClient should factor that into the FunctionResultContent message it creates for rejected tool calls. The property can currently be ignored for Approval=true, though it's conceivable the message might be used for something in the future, and if nothing else, it'll show up in tracing/logging/etc. when the content is serialized.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

  • Fixes dotnet/extensions#7139

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Microsoft Reviewers: Open in CodeFlow

Copilot avatar Dec 12 '25 15:12 Copilot

@NoofSaeidh, does this address your scenarios / concerns from https://github.com/dotnet/extensions/issues/7139?

stephentoub avatar Dec 12 '25 16:12 stephentoub

cc: @rogerbarreto, @westey-m, @jozkee

stephentoub avatar Dec 12 '25 16:12 stephentoub

@copilot, please address the feedback

stephentoub avatar Dec 12 '25 16:12 stephentoub

@NoofSaeidh, does this address your scenarios / concerns from #7139?

Yes, thank you!

NoofSaeidh avatar Dec 12 '25 23:12 NoofSaeidh

@stephentoub actually looks like it is worth to add Reason into the CreateResponse method in FunctionApprovalRequestContent https://github.com/dotnet/extensions/blob/eedc644224ec1e84a52fb303c0e613f059ce5a8d/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionApprovalRequestContent.cs#L40

NoofSaeidh avatar Dec 15 '25 13:12 NoofSaeidh

@stephentoub actually looks like it is worth to add Reason into the CreateResponse method in FunctionApprovalRequestContent

https://github.com/dotnet/extensions/blob/eedc644224ec1e84a52fb303c0e613f059ce5a8d/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionApprovalRequestContent.cs#L40

Why not just set the property on the resulting object?

stephentoub avatar Dec 15 '25 13:12 stephentoub

@stephentoub it is of course would work. It just makes it easier to use, because setting property for resulting object doesn't support an object initializer patter and it requires to create variable first, which makes it ugly in certain situations.

For instance:

// with argument
messages.Add(new ChatMessage(ChatRole.User, [response.Messages.Last().Contents.OfType<FunctionApprovalRequestContent>().First().CreateResponse(false, reason: "reason")]));

// without argument
var approval = response.Messages.Last().Contents.OfType<FunctionApprovalRequestContent>().First().CreateResponse(false);
approval.Reason = "reason";
messages.Add(new ChatMessage(ChatRole.User, [approval]));

NoofSaeidh avatar Dec 16 '25 18:12 NoofSaeidh

@copilot, please add a string? reason = null optional parameter to FunctionApprovalRequestContent.CreateResponse (https://github.com/dotnet/extensions/blob/72b108788bd3e098f277604a23ac826b9b62ce9d/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionApprovalRequestContent.cs#L40)

stephentoub avatar Dec 17 '25 02:12 stephentoub

@copilot, please add a string? reason = null optional parameter to FunctionApprovalRequestContent.CreateResponse (https://github.com/dotnet/extensions/blob/72b108788bd3e098f277604a23ac826b9b62ce9d/s...

Done in commit fb45994. Added string? reason = null parameter to CreateResponse method which sets the Reason property on the returned FunctionApprovalResponseContent.

Copilot avatar Dec 17 '25 02:12 Copilot

@copilot this is a breaking change, you need to update src/Libraries/Microsoft.Extensions.AI.Abstractions/CompatibilitySuppressions.xml to fix CI errors

/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : API compatibility errors between 'lib/net462/Microsoft.Extensions.AI.Abstractions.dll' (/mnt/vss/_work/1/s/.packages/microsoft.extensions.ai.abstractions/9.10.0/microsoft.extensions.ai.abstractions.9.10.0.nupkg) and 'lib/net462/Microsoft.Extensions.AI.Abstractions.dll' (/mnt/vss/_work/1/s/artifacts/packages/Release/Shipping/Microsoft.Extensions.AI.Abstractions.10.2.0-ci.nupkg): [/mnt/vss/_work/1/s/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj]
/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error CP0002: Member 'Microsoft.Extensions.AI.FunctionApprovalResponseContent Microsoft.Extensions.AI.FunctionApprovalRequestContent.CreateResponse(bool)' exists on [Baseline] lib/net462/Microsoft.Extensions.AI.Abstractions.dll but not on lib/net462/Microsoft.Extensions.AI.Abstractions.dll [/mnt/vss/_work/1/s/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj]
/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : API compatibility errors between 'lib/net8.0/Microsoft.Extensions.AI.Abstractions.dll' (/mnt/vss/_work/1/s/.packages/microsoft.extensions.ai.abstractions/9.10.0/microsoft.extensions.ai.abstractions.9.10.0.nupkg) and 'lib/net8.0/Microsoft.Extensions.AI.Abstractions.dll' (/mnt/vss/_work/1/s/artifacts/packages/Release/Shipping/Microsoft.Extensions.AI.Abstractions.10.2.0-ci.nupkg): [/mnt/vss/_work/1/s/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj]
/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error CP0002: Member 'Microsoft.Extensions.AI.FunctionApprovalResponseContent Microsoft.Extensions.AI.FunctionApprovalRequestContent.CreateResponse(bool)' exists on [Baseline] lib/net8.0/Microsoft.Extensions.AI.Abstractions.dll but not on lib/net8.0/Microsoft.Extensions.AI.Abstractions.dll [/mnt/vss/_work/1/s/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj]
/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : API compatibility errors between 'lib/net9.0/Microsoft.Extensions.AI.Abstractions.dll' (/mnt/vss/_work/1/s/.packages/microsoft.extensions.ai.abstractions/9.10.0/microsoft.extensions.ai.abstractions.9.10.0.nupkg) and 'lib/net9.0/Microsoft.Extensions.AI.Abstractions.dll' (/mnt/vss/_work/1/s/artifacts/packages/Release/Shipping/Microsoft.Extensions.AI.Abstractions.10.2.0-ci.nupkg): [/mnt/vss/_work/1/s/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj]
/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error CP0002: Member 'Microsoft.Extensions.AI.FunctionApprovalResponseContent Microsoft.Extensions.AI.FunctionApprovalRequestContent.CreateResponse(bool)' exists on [Baseline] lib/net9.0/Microsoft.Extensions.AI.Abstractions.dll but not on lib/net9.0/Microsoft.Extensions.AI.Abstractions.dll [/mnt/vss/_work/1/s/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj]
/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : API compatibility errors between 'lib/netstandard2.0/Microsoft.Extensions.AI.Abstractions.dll' (/mnt/vss/_work/1/s/.packages/microsoft.extensions.ai.abstractions/9.10.0/microsoft.extensions.ai.abstractions.9.10.0.nupkg) and 'lib/netstandard2.0/Microsoft.Extensions.AI.Abstractions.dll' (/mnt/vss/_work/1/s/artifacts/packages/Release/Shipping/Microsoft.Extensions.AI.Abstractions.10.2.0-ci.nupkg): [/mnt/vss/_work/1/s/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj]
/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error CP0002: Member 'Microsoft.Extensions.AI.FunctionApprovalResponseContent Microsoft.Extensions.AI.FunctionApprovalRequestContent.CreateResponse(bool)' exists on [Baseline] lib/netstandard2.0/Microsoft.Extensions.AI.Abstractions.dll but not on lib/netstandard2.0/Microsoft.Extensions.AI.Abstractions.dll [/mnt/vss/_work/1/s/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj]
/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : API breaking changes found. If those are intentional, the APICompat suppression file can be updated by rebuilding with '/p:ApiCompatGenerateSuppressionFile=true' [/mnt/vss/_work/1/s/src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.csproj]

jozkee avatar Dec 17 '25 17:12 jozkee

@copilot this is a breaking change, you need to update src/Libraries/Microsoft.Extensions.AI.Abstractions/CompatibilitySuppressions.xml to fix CI errors

/mnt/vss/_work/1/s/.dotnet/sdk/10.0.100/S...

Done in commit 92bfbdb. Added CP0002 suppressions for the CreateResponse(bool) method signature change across all target frameworks (net462, net8.0, net9.0, netstandard2.0) in CompatibilitySuppressions.xml to fix the CI errors.

Copilot avatar Dec 17 '25 17:12 Copilot