Skip to content

Fix FICC tool_calls/tool ordering with approvals and service-managed chat history#7617

Open
westey-m wants to merge 1 commit into
dotnet:mainfrom
westey-m:ficc-fcc-frc-ordering-with-approvals-and-service-chat-history
Open

Fix FICC tool_calls/tool ordering with approvals and service-managed chat history#7617
westey-m wants to merge 1 commit into
dotnet:mainfrom
westey-m:ficc-fcc-frc-ordering-with-approvals-and-service-chat-history

Conversation

@westey-m

@westey-m westey-m commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #7616

Summary

When FunctionInvokingChatClient (FICC) resumes an approval-gated function call in service-managed history mode (a non-empty ConversationId is set), it omits the reconstructed assistant(tool_calls) message (the service already holds it) but still appends the executed tool result to the tail of the outgoing message list. If the caller supplies any message after the ToolApprovalResponseContent on the continuation turn, that trailing message ends up wedged between the service-held assistant(tool_calls) and its tool result:

assistant(tool_calls) -> user(<trailing message>) -> tool(result)

Providers enforcing tool_callstool adjacency (e.g. OpenAI) reject this with HTTP 400.

Fix

Instead of always appending the reconstructed tool-call/tool-result messages at the tail, they are now inserted just before any caller-supplied trailing messages, keeping the tool result adjacent to the assistant message that owns the matching tool_call_id:

  • ProcessFunctionCallsAsync gains an optional int? insertIndex — when set, results are inserted at that index rather than appended.
  • ProcessFunctionApprovalResponses computes the trailing-message count (messages after the last approval-content message) and inserts the reconstructed assistant(tool_calls) + rejected results just before them, returning the running insert index.
  • The index is threaded through InvokeApprovedFunctionApprovalResponsesAsync so approved tool results land adjacent to the tool-call.
  • Applied to both the streaming and non-streaming approval paths.

When there are no trailing messages the insert position degrades to the original tail-append behavior, so existing scenarios are unaffected. The invoked function still receives the full input (trailing messages remain in context.Messages during invocation; only the result placement changes).

Resulting valid ordering:

assistant(tool_calls) -> tool(result) -> user(<trailing message>)

Tests

Added regression tests (streaming + non-streaming) covering trailing messages after an approval response in both service-managed and client-managed history modes:

  • ApprovedResponsesStayAdjacentToToolCallWhenTrailingMessagesPresentWithServiceThreadsAsync
  • ApprovedResponsesStayAdjacentToToolCallWhenTrailingMessagesPresentWithClientHistoryAsync

All existing FunctionInvokingChatClient approval tests continue to pass.

Microsoft Reviewers: Open in CodeFlow

CC @jozkee

Copilot AI review requested due to automatic review settings July 9, 2026 15:28
@westey-m westey-m requested a review from a team as a code owner July 9, 2026 15:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an ordering bug in FunctionInvokingChatClient when resuming approval-gated tool calls in service-managed history mode (ConversationId set): tool results are now inserted ahead of any caller-supplied trailing messages so providers that require strict assistant(tool_calls) -> tool(result) adjacency don’t reject the request.

Changes:

  • Add an insertion-index path for generated tool result messages so they can be placed before trailing caller messages rather than always appended.
  • Compute and thread an “approved result insert index” through the approval-response handling (streaming + non-streaming) to keep tool results adjacent to their tool-call.
  • Add regression tests for both service-managed and client-managed history modes when trailing messages are present after approval responses.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs Inserts reconstructed tool-call / tool-result messages at the approval anchor (before trailing messages) and threads an insert index through approval execution to preserve tool_callstool adjacency.
test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs Adds streaming and non-streaming regression tests validating correct tool result placement with trailing messages in both service-managed and client-managed history modes.

@dotnet-comment-bot

Copy link
Copy Markdown
Collaborator

‼️ Found issues ‼️

Project Coverage Type Expected Actual
Microsoft.Extensions.Diagnostics.Testing Line 99 98.65 🔻
Microsoft.Extensions.Telemetry Line 93 92.64 🔻
Microsoft.Extensions.AI Line 89 88.67 🔻
Microsoft.Extensions.AI Branch 89 88.59 🔻
Microsoft.Extensions.AI.OpenAI Line 75 62.75 🔻
Microsoft.Extensions.AI.OpenAI Branch 75 50.26 🔻
Microsoft.Extensions.DataIngestion.MarkItDown Line 75 4.46 🔻
Microsoft.Extensions.DataIngestion.MarkItDown Branch 75 0 🔻
Microsoft.Extensions.Diagnostics.ResourceMonitoring Line 99 96.03 🔻
Microsoft.Extensions.Diagnostics.ResourceMonitoring Branch 99 92.76 🔻
Microsoft.Extensions.Diagnostics.ResourceMonitoring.Kubernetes Line 99 97.73 🔻
Microsoft.Extensions.ServiceDiscovery.Dns Line 75 69.93 🔻
Microsoft.Extensions.ServiceDiscovery.Abstractions Line 75 42.11 🔻
Microsoft.Extensions.ServiceDiscovery.Abstractions Branch 75 42.86 🔻
Microsoft.Extensions.ServiceDiscovery Line 75 67.96 🔻
Microsoft.Extensions.ServiceDiscovery Branch 75 71.43 🔻
Microsoft.Extensions.ServiceDiscovery.Yarp Line 75 73.85 🔻
Microsoft.Extensions.ServiceDiscovery.Yarp Branch 75 70 🔻
Microsoft.Extensions.VectorData.Abstractions Line 75 37.39 🔻
Microsoft.Extensions.VectorData.Abstractions Branch 75 22.73 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project Expected Actual
Microsoft.Gen.BuildMetadata 97 100
Microsoft.Gen.MetadataExtractor 57 73
Microsoft.Gen.MetricsReports 67 69
Microsoft.Extensions.AI.Abstractions 82 86
Microsoft.Extensions.AI.Evaluation.NLP 0 78
Microsoft.Extensions.Caching.Hybrid 82 84
Microsoft.Extensions.DataIngestion 75 89
Microsoft.Extensions.DataIngestion.Markdig 75 90
Microsoft.Extensions.Http.Resilience 97 100

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1501578&view=codecoverage-tab

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants