Skip to content

Preserve function approval message order after empty messages - #7842

Open
valkiara wants to merge 1 commit into
dotnet:mainfrom
valkiara:fix/approval-message-order
Open

valkiara wants to merge 1 commit into
dotnet:mainfrom
valkiara:fix/approval-message-order

Conversation

@valkiara

@valkiara valkiara commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Empty messages after a function approval are removed during extraction but were still counted when positioning the reconstructed function calls and results. This moves the block ahead of earlier conversation messages, or produces a negative insertion index when several empty messages follow the approval.

Count only non-empty trailing messages. Extend the existing approval-order tests with zero, one and three trailing empty messages, covering client-managed and service-managed history, residual approval-message content, and both streaming and non-streaming calls.

Fixes #7834.

Validation: the eight added empty-message cases fail before the fix. Afterward, all 776 tests in Microsoft.Extensions.AI.Tests pass on net10.0. Microsoft.Extensions.AI builds for net8.0, net9.0, net10.0, net462 and netstandard2.0 with no warnings or errors.

Microsoft Reviewers: Open in CodeFlow

@valkiara
valkiara requested a review from a team as a code owner October 14, 2026 14:10
@github-actions github-actions Bot added the area-ai Microsoft.Extensions.AI libraries label Oct 14, 2026
@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.71 🔻
Microsoft.Extensions.AI.OpenAI Line 75 68.24 🔻
Microsoft.Extensions.AI.OpenAI Branch 75 57.54 🔻
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 Line 75 68.11 🔻
Microsoft.Extensions.ServiceDiscovery Branch 75 71.43 🔻
Microsoft.Extensions.ServiceDiscovery.Abstractions Line 75 42.11 🔻
Microsoft.Extensions.ServiceDiscovery.Abstractions Branch 75 42.86 🔻
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.Extensions.Http.Diagnostics 94 95
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 85
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=1635220&view=codecoverage-tab

This branch has not been deployed

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

Labels

area-ai Microsoft.Extensions.AI libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FunctionInvokingChatClient places approved function results before earlier messages when an empty message follows the last approval response

2 participants