Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 16, 2025

When processing FunctionApprovalResponseContent, the client appends reconstructed FunctionCallContent and FunctionResultContent messages to the end of the message list, reordering any messages that came after the approval response.

Example of the issue:

// Input messages
User: "1st message"
Assistant: FunctionApprovalRequestContent
User: FunctionApprovalResponseContent (reject)
User: "2nd message"

// Before fix - "2nd message" incorrectly moved
User: "1st message"
User: "2nd message"  // ❌ Wrong position
Assistant: FunctionCallContent
Tool: FunctionResultContent (rejection)

// After fix - "2nd message" stays at end
User: "1st message"
Assistant: FunctionCallContent
Tool: FunctionResultContent (rejection)
User: "2nd message"  // ✓ Correct position

Changes

  • Track insertion index: ExtractAndRemoveApprovalRequestsAndResponses now returns the index where approval requests were originally located
  • Insert instead of append: ProcessFunctionApprovalResponses uses InsertRange/Insert at the tracked position rather than AddRange/Add
  • Thread index through call chain: ProcessFunctionCallsAsync accepts an insertionIndex parameter to place approved function results at the correct position
  • Handle already-executed approvals: When function results already exist in the message list, new function calls are appended to preserve existing ordering

Tests

Added test cases for rejection and approval scenarios with user messages following the approval response.

Original prompt

Problem

When FunctionInvokingChatClient processes FunctionApprovalResponseContent, it incorrectly reorders chat messages. User messages added after a function approval rejection are moved before the tool call/result instead of remaining at the end of the conversation.

This is reported in issue #7156: #7156

Root Cause

In FunctionInvokingChatClient.cs, the ProcessFunctionApprovalResponses method:

  1. Calls ExtractAndRemoveApprovalRequestsAndResponses which removes FunctionApprovalRequestContent and FunctionApprovalResponseContent from their original positions in the message list
  2. Then appends the reconstructed FunctionCallContent and FunctionResultContent messages at the end of the list using AddRange and Add (lines 1276-1287)

This destroys the relative ordering with any messages that came after the approval response.

Expected message order:

user: 1st message
assistant: tool_call
tool_call: reject
user: 2nd message  ← Should come last

Actual message order:

user: 1st message
user: 2nd message  ← Incorrectly positioned before tool call
assistant: tool_call
tool_call: reject

Required Fix

The ExtractAndRemoveApprovalRequestsAndResponses method needs to track the original index/position of the approval request messages. Then ProcessFunctionApprovalResponses should:

  1. Insert the reconstructed FunctionCallContent message at the original position of the approval request (not at the end)
  2. Insert the rejection FunctionResultContent immediately after
  3. Ensure any user messages that came after the approval response remain at the end

The key change is in ProcessFunctionApprovalResponses - instead of:

originalMessages.AddRange(preDownstreamCallHistory);
// ...
originalMessages.Add(rejectedPreDownstreamCallResultsMessage);

It should insert at the correct position to preserve message ordering relative to any subsequent user messages.

Files to Modify

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

Testing

The fix should be validated with test cases covering:

  1. Rejection with a user message added after the approval response
  2. Approval with a user message added after the approval response
  3. Multiple approval/rejection scenarios with interleaved user messages

Existing tests in test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs should continue to pass, and new tests should be added for the scenarios described in issue #7156.

This pull request was created as a result of the following prompt from Copilot chat.

Problem

When FunctionInvokingChatClient processes FunctionApprovalResponseContent, it incorrectly reorders chat messages. User messages added after a function approval rejection are moved before the tool call/result instead of remaining at the end of the conversation.

This is reported in issue #7156: #7156

Root Cause

In FunctionInvokingChatClient.cs, the ProcessFunctionApprovalResponses method:

  1. Calls ExtractAndRemoveApprovalRequestsAndResponses which removes FunctionApprovalRequestContent and FunctionApprovalResponseContent from their original positions in the message list
  2. Then appends the reconstructed FunctionCallContent and FunctionResultContent messages at the end of the list using AddRange and Add (lines 1276-1287)

This destroys the relative ordering with any messages that came after the approval response.

Expected message order:

user: 1st message
assistant: tool_call
tool_call: reject
user: 2nd message  ← Should come last

Actual message order:

user: 1st message
user: 2nd message  ← Incorrectly positioned before tool call
assistant: tool_call
tool_call: reject

Required Fix

The ExtractAndRemoveApprovalRequestsAndResponses method needs to track the original index/position of the approval request messages. Then ProcessFunctionApprovalResponses should:

  1. Insert the reconstructed FunctionCallContent message at the original position of the approval request (not at the end)
  2. Insert the rejection FunctionResultContent immediately after
  3. Ensure any user messages that came after the approval response remain at the end

The key change is in ProcessFunctionApprovalResponses - instead of:

originalMessages.AddRange(preDownstreamCallHistory);
// ...
originalMessages.Add(rejectedPreDownstreamCallResultsMessage);

It should insert at the correct position to preserve message ordering relative to any subsequent user messages.

Files to Modify

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

Testing

The fix should be validated with test cases covering:

  1. Rejection with a user message added after the approval response
  2. Approval with a user message added after the approval response
  3. Multiple approval/rejection scenarios with interleaved user messages

Existing tests in test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs should continue to pass, and new tests should be added for the scenarios described in issue #7156.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Microsoft Reviewers: Open in CodeFlow

Copilot AI and others added 2 commits December 16, 2025 15:49
…cessing

- Track insertion index for approval request messages before removal
- Insert reconstructed FunctionCallContent and FunctionResultContent at correct position
- Pass insertion index through to ProcessFunctionCallsAsync for approved functions
- Handle edge case where already-executed approvals exist
- Add tests for rejection and approval scenarios with user messages after responses

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix message ordering in FunctionInvokingChatClient Fix message ordering when processing function approval responses Dec 16, 2025
Copilot AI requested a review from stephentoub December 16, 2025 15:55
@stephentoub stephentoub marked this pull request as ready for review December 16, 2025 15:56
@stephentoub stephentoub requested a review from a team as a code owner December 16, 2025 15:56
Copilot AI review requested due to automatic review settings December 16, 2025 15:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a message ordering bug in FunctionInvokingChatClient where user messages added after function approval responses were incorrectly repositioned before tool call/result messages instead of remaining at the end of the conversation.

Key changes:

  • Modified ExtractAndRemoveApprovalRequestsAndResponses to track and return the original index position of approval requests
  • Updated ProcessFunctionApprovalResponses to insert reconstructed function call and result messages at their original positions using InsertRange/Insert instead of appending with AddRange/Add
  • Extended ProcessFunctionCallsAsync to accept an insertionIndex parameter for placing approved function results at the correct position

Reviewed changes

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

File Description
FunctionInvokingChatClient.cs Core fix: tracks approval request positions and uses insertion instead of appending to preserve message ordering
FunctionInvokingChatClientApprovalsTests.cs Adds test coverage for rejection and approval scenarios with user messages following approval responses

…nvokingChatClient.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment on lines +1464 to +1470
// Before:
// [User, FunctionApprovalRequest(A), FunctionApprovalResponse(A), FunctionResult(A), FunctionApprovalRequest(B)]
// After processing approval for B, if we inserted at the original index of B, we'd get:
// [User, FunctionApprovalRequest(A), FunctionApprovalResponse(A), FunctionResult(A), FunctionCall(B), FunctionResult(B)]
// But if there are already function results present (e.g., for A), we want to append new function calls/results for B at the end:
// [User, FunctionApprovalRequest(A), FunctionApprovalResponse(A), FunctionResult(A), FunctionApprovalRequest(B), FunctionCall(B), FunctionResult(B)]
// This preserves the correct chronological order of function calls and results.
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The example comment is confusing because it shows FunctionApprovalRequest and FunctionApprovalResponse in the message list, but these would have been removed by the time this logic runs. Consider clarifying that the "Before" represents the original user input before extraction, or showing the actual state of the messages list at this point (after extraction but before insertion). For example: "Before extraction: [User, FAReq(A), FARes(A), FAReq(B)]; After extraction: [User, FRes(A)]; After insertion with functionResultCallIds present: [User, FRes(A), FCall(B), FRes(B)]"

Suggested change
// Before:
// [User, FunctionApprovalRequest(A), FunctionApprovalResponse(A), FunctionResult(A), FunctionApprovalRequest(B)]
// After processing approval for B, if we inserted at the original index of B, we'd get:
// [User, FunctionApprovalRequest(A), FunctionApprovalResponse(A), FunctionResult(A), FunctionCall(B), FunctionResult(B)]
// But if there are already function results present (e.g., for A), we want to append new function calls/results for B at the end:
// [User, FunctionApprovalRequest(A), FunctionApprovalResponse(A), FunctionResult(A), FunctionApprovalRequest(B), FunctionCall(B), FunctionResult(B)]
// This preserves the correct chronological order of function calls and results.
// Before extraction (original user input and approval messages):
// [User, FunctionApprovalRequest(A), FunctionApprovalResponse(A), FunctionResult(A), FunctionApprovalRequest(B)]
// After extraction of approval requests/responses (state of 'messages' at this point, with existing results only):
// [User, FunctionResult(A)]
// After processing approval for B, if we inserted at the original index of B, we'd conceptually place the new call/result
// where B's approval request was, interleaving them with existing messages:
// [User, FunctionResult(A), FunctionCall(B), FunctionResult(B)]
// But if there are already function results present (e.g., for A), we instead append new function calls/results for B at the end
// of the current list of messages to preserve chronological ordering relative to any prior calls/results.

Copilot uses AI. Check for mistakes.

// Calculate the insertion position: right after the FCC messages we just inserted
int rejectedInsertionIndex = insertionIndex >= 0 && insertionIndex <= originalMessages.Count
? insertionIndex + (allPreDownstreamCallMessages?.Count ?? 0)
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The calculation of rejectedInsertionIndex doesn't account for whether FCC messages were actually inserted into originalMessages. When hasConversationId is true, FCC messages are not inserted (line 1297-1308), but this calculation still adds allPreDownstreamCallMessages.Count to the index. This causes rejection messages to be inserted at the wrong position when there are messages after the approval response. The calculation should only add the FCC count when !hasConversationId. For example: insertionIndex + (!hasConversationId ? (allPreDownstreamCallMessages?.Count ?? 0) : 0)

Suggested change
? insertionIndex + (allPreDownstreamCallMessages?.Count ?? 0)
? insertionIndex + (!hasConversationId ? (allPreDownstreamCallMessages?.Count ?? 0) : 0)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FunctionInvokingChatClient reorder messages when using FunctionApprovalResponseContent

2 participants