Skip to content

.NET: Fix CompactionMessageIndex.IsSummaryMessage#7042

Merged
westey-m merged 2 commits into
microsoft:mainfrom
pwoosam:pwoosam/fix-IsSummaryMessage
Jul 10, 2026
Merged

.NET: Fix CompactionMessageIndex.IsSummaryMessage#7042
westey-m merged 2 commits into
microsoft:mainfrom
pwoosam:pwoosam/fix-IsSummaryMessage

Conversation

@pwoosam

@pwoosam pwoosam commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

Deserialized chat messages do not group summary chat messages appropriately because IsSummaryMessage will always return false when the additional property type is JsonElement.

Description & Review Guide

  • What are the major changes? No major changes
  • What is the impact of these changes? Supports deserialized ChatMessage.AdditionalProperties, which may be JsonElement instead of bool
  • What do you want reviewers to focus on? Should IsSummaryMessage handle “bools and JsonElement” or JsonElement only?

Related Issue

Fixes #7003

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI review requested due to automatic review settings July 10, 2026 07:19
@giles17 giles17 added the .NET Usage: [Issues, PRs], Target: .Net label Jul 10, 2026
@github-actions github-actions Bot changed the title Fix CompactionMessageIndex.IsSummaryMessage .NET: Fix CompactionMessageIndex.IsSummaryMessage Jul 10, 2026

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 summary-message detection in the .NET compaction pipeline so assistant messages tagged with _is_summary can be recognized as CompactionGroupKind.Summary even when ChatMessage.AdditionalProperties values come back as JsonElement after deserialization.

Changes:

  • Update CompactionMessageIndex.IsSummaryMessage to treat _is_summary values stored as JsonElement as summary flags.
  • Add a new theory in CompactionMessageIndexTests to validate summary grouping across several _is_summary value shapes.

Reviewed changes

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

File Description
dotnet/src/Microsoft.Agents.AI/Compaction/CompactionMessageIndex.cs Adjusts summary-message detection logic to account for deserialized JsonElement values.
dotnet/tests/Microsoft.Agents.AI.UnitTests/Compaction/CompactionMessageIndexTests.cs Adds parameterized coverage for summary-flag handling when _is_summary is present in AdditionalProperties.

Comment thread dotnet/src/Microsoft.Agents.AI/Compaction/CompactionMessageIndex.cs
@pwoosam

pwoosam commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

cc @westey-m

@westey-m
westey-m enabled auto-merge July 10, 2026 11:15
@westey-m
westey-m added this pull request to the merge queue Jul 10, 2026
Merged via the queue into microsoft:main with commit e677ccc Jul 10, 2026
27 checks passed
@westey-m

Copy link
Copy Markdown
Contributor

Thanks for the contribution @pwoosam!

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

Labels

.NET Usage: [Issues, PRs], Target: .Net

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.NET: [Bug]: CompactionMessageIndex.IsSummaryMessage always returns false because TryGetValue returns value as JsonElement

5 participants