[dotnet-port-api] Add compaction-backed history provider - #1092
Michelle Clayton (michelle-clayton-work) wants to merge 2 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Two critical correctness issues remain in the history provider.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a compaction-backed history provider with persistence, tests, an updated example, and refreshed .NET parity documentation.
Changes:
- Adds
compaction.NewHistoryProvider. - Adds compaction persistence and summary replay tests.
- Updates the additional-context example.
- Refreshes the .NET/Go feature comparison.
File summaries
| File | Summary |
|---|---|
examples/02-agents/agents/step17_additional_ai_context/main.go |
Uses the new bounded history provider. |
docs/dotnet-go-sdk-feature-comparison.md |
Updates chat-history parity status. |
agent/compaction/historyprovider.go |
Implements persisted compaction. Critical issues remain around including current input before compaction and applying filters before generated summaries; a concurrency test is also needed. |
agent/compaction/historyprovider_test.go |
Tests compacted persistence and summary replay. |
agent/compaction/doc.go |
Documents history-provider support. |
Review details
Suppressed comments (1)
agent/compaction/historyprovider.go:146
- This provider rewrites shared session state after appending and compacting each turn, but the new tests only exercise sequential
Invokedcalls. The existing in-memory provider has a same-session concurrent store/race test (agent/history_test.go:80-118); add the analogous test here with a real truncation strategy to verify that concurrent turns are not lost while compaction runs.
compacted, err := compactHistory(ctx, cfg.Strategy, messages, cfg.TokenCounter, cfg.Logger)
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This comment has been minimized.
This comment has been minimized.
|
Copilot address PR feedback |
Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
Addressed the review feedback in |
|
Scope: public API, user-visible behavior Changed Go contract: New exported Upstream evidence reviewed:
Result: findings reported Finding: Everything else (source stamping, request/response filters, session-state keying, example usage) lines up conceptually with
|
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent for #1092 · copilot · auto · 117 AIC · ⌖ 6.8 AIC · ⊞ 9.6K
| } | ||
| messages = append(messages, filteredRequest...) | ||
| messages = append(messages, filteredResponse...) | ||
|
|
There was a problem hiding this comment.
NewHistoryProvider applies Strategy.Compact in both Invoking (line ~137, transient history returned to the model) and Invoked (line ~199, persisted session state), i.e. twice per turn with no way to select a single trigger point.
Upstream InMemoryChatHistoryProviderOptions.ReducerTriggerEvent (dotnet/src/Microsoft.Agents.AI.Abstractions/InMemoryChatHistoryProviderOptions.cs) only ever fires the reducer at one configurable point — BeforeMessagesRetrieval (default) or AfterMessageAdded — precisely to avoid redundant reducer invocations. InMemoryChatHistoryProvider.cs (ProvideChatHistoryAsync/StoreChatHistoryAsync) shows the reducer is gated by if (this.ReducerTriggerEvent is ... ) at each site, never both.
For SummarizationStrategy (LLM-backed), this divergence causes an extra summarizer call every turn versus the .NET pattern. Consider adding a trigger-point option (defaulting to pre-retrieval only, matching .NET) or applying the strategy at just one of the two lifecycle points, unless the double application to keep persisted state always-compacted is an intentional, documented divergence.
Tip
Your pull request is ready to create! 🎉 ✅
Everything is OK—the changes have been pushed to branch
copilot/dotnet-port-api-compaction-history-provider-fdbbd9f2ce32e8b7. Please review the changes, including any protected files, before creating the pull request.Create the pull request
The original pull request description is below.
Summary
Add a compaction-backed history provider in
agent/compactionso Go agents can apply reducer-triggered history compaction directly on the history-provider surface instead of only through a separate context provider. The change adds the new provider, covers compacted persistence and summary replay in tests, updates the additional-context sample to use the new API, and refreshes the parity doc to reflect the reduced .NET chat-history gap.Ported .NET PRs
5996105a1bf2726918101adc6e9c9857b7f68b98, with workflow/test delta previously checked through6a0773ba2180e8036d138dbb9794ae64ec2d978b.Breaking Changes
No.
Tests and Examples
go test ./agent ./agent/compaction ./examples/02-agents/agents/step17_additional_ai_contextagent/compaction/historyprovider_test.goexamples/02-agents/agents/step17_additional_ai_context/main.goNotes
upstream-agent-framework/mainfetch was blocked in this environment, so this PR ports the documented chat-history parity gap as a narrow Go realignment instead of tying the change to a freshly inspected upstream PR.Closes #1053