Skip to content

[dotnet-port-fixes] Align compaction summary marker handling - #539

Merged
gdams merged 2 commits into
mainfrom
dotnet-port/compaction-summary-marker-67b325fede384a29
Jul 20, 2026
Merged

[dotnet-port-fixes] Align compaction summary marker handling#539
gdams merged 2 commits into
mainfrom
dotnet-port/compaction-summary-marker-67b325fede384a29

Conversation

@michelle-clayton-work

Copy link
Copy Markdown
Contributor

Summary

Align Go compaction summary detection with the upstream .NET fix for _is_summary metadata by accepting both native bool values and raw JSON boolean tokens when grouping assistant messages. Added parity tests that keep missing, false, null, and string token values as assistant text while recognizing true in both native and raw-token forms.

Upstream reference:

Ported .NET PRs

Breaking Changes

No.

Tests and Examples

  • go test ./agent/compaction
  • Added TestMessageIndex_SummaryPropertyKeyIsRespected
  • No examples changed

Notes

Skipped broader recent upstream compaction/workflow changes because they were either already tracked in Go porting work or were larger than the narrow nightly PR target. This change is internal-only and does not modify exported Go symbols.

Generated by .NET to Go Fixes and Test Porting Agent · 939.3 AIC · ⌖ 31.2 AIC · ⊞ 21.7K ·

Closes #487

Support raw JSON _is_summary values in compaction message grouping and add parity tests for bool and raw-token cases.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

Aligns the Go compaction message grouping logic with the upstream .NET fix by recognizing _is_summary markers not only as native bool values but also when represented as raw JSON boolean tokens, and adds parity tests to validate the intended grouping behavior.

Changes:

  • Extend summary detection to accept bool, json.RawMessage, *json.RawMessage, and []byte representations.
  • Add table-driven tests to ensure missing/false/null/string values stay as assistant text while true becomes a summary group.

Reviewed changes

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

File Description
agent/compaction/index.go Expands _is_summary detection to support raw JSON boolean token representations in message metadata.
agent/compaction/compaction_test.go Adds parity tests covering expected summary vs assistant-text grouping for various _is_summary value forms.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread agent/compaction/compaction_test.go Outdated
@github-actions

This comment has been minimized.

@michelle-clayton-work

Copy link
Copy Markdown
Contributor Author

@copilot respond to PR comments

…opertyValueParsing

Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

@copilot respond to PR comments

Done — addressed both points in commit test: add *json.RawMessage and []byte cases; rename test to SummaryPropertyValueParsing:

  1. Added missing type variants: *json.RawMessage (nil, false, true) and []byte (false, null, true) are now exercised as explicit subtests, covering all the new branches in isSummaryMessage.
  2. Renamed the test: TestMessageIndex_SummaryPropertyKeyIsRespectedTestMessageIndex_SummaryPropertyValueParsing to reflect that it validates how different value forms are parsed, not the key name itself.

All 14 subtests pass.

@github-actions

Copy link
Copy Markdown
Contributor

Cross-Repo Parity Review — No Issues

This PR is a direct port of upstream .NET fix microsoft/agent-framework#7042 and is already aligned with the upstream change.

Scope check: Only isSummaryMessage (unexported) and the new unexported helper rawSummaryMessageValue were modified. No exported Go symbols, option structs, or user-visible behavior changed. The fix correctly extends the internal JSON deserialization path to match how .NET handles _is_summary as a raw JSON boolean token after deserialization.

Parity verdict: ✅ Aligned. The Go fix mirrors the upstream .NET behavior fix semantically. No Python equivalent was found for this specific _is_summary compaction marker, which is consistent with the .NET-origin of the feature.

Generated by Go API Consistency Review Agent for issue #539 · 29 AIC · ⌖ 11.7 AIC · ⊞ 26.7K ·

@gdams
gdams enabled auto-merge July 20, 2026 09:47
@gdams
gdams added this pull request to the merge queue Jul 20, 2026
Merged via the queue into main with commit f64ebe4 Jul 20, 2026
26 of 28 checks passed
@gdams
gdams deleted the dotnet-port/compaction-summary-marker-67b325fede384a29 branch July 20, 2026 09:51
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.

[dotnet-port-fixes] Align compaction summary marker handling

4 participants