Python: fix(ag-ui): dedupe client-replayed transcripts on resume - #8149
Evan Mattson (moonbox3) merged 10 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Empty-message resumes can lose stored history in both agent and workflow paths.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates AG-UI resume handling to deduplicate client-replayed transcripts using snapshot reconstruction.
Changes:
- Applies reconstruction to agent and workflow resume history.
- Adds transcript deduplication regression tests.
File summaries
| File | Review |
|---|---|
python/packages/ag-ui/tests/ag_ui/test_resume_transcript_dedupe.py |
Adds tests, but misses empty-message integration coverage and the required copyright header. |
python/packages/ag-ui/agent_framework_ag_ui/_workflow.py |
Reconstruction drops stored history for empty checkpoint-resume input. |
python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py |
Reconstruction drops stored history for empty approval-resume input. |
Review details
Suppressed comments (1)
python/packages/ag-ui/tests/ag_ui/test_resume_transcript_dedupe.py:1
- This new Python file is missing the required first-line copyright header mandated by
python/CODING_STANDARD.md:453and used throughout this test directory.
"""Regression for AG-UI resume with client-replayed transcript (#8140)."""
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
@microsoft-github-policy-service agree |
Use the transcript reconstructor only when the client sends a non-empty messages list; empty approval/checkpoint resumes still go through resume_seeded_messages so stored history is preserved. Also add the required copyright header and document the empty-input reconstructor contract in tests.
|
Addressed Copilot review:
Existing |
|
Addressed on head
Existing |
Add reconcile_resume_messages for empty vs replayed resume shapes, use it from agent and workflow runners, and mark generic reconstructed resumes as seeded so save-time prepend does not duplicate history.
|
Thanks Evan Mattson (@moonbox3) — addressed both points on this head:
Added a unit test covering the session-owned empty vs replayed shapes. |
|
Please resolve individual comments with a note if addressed or not. Then resolve when complete. |
|
Evan Mattson (@moonbox3) Done — replied on each review thread with addressed/partial notes and resolved all five threads. Please take another look when convenient. |
Evan Mattson (moonbox3)
left a comment
There was a problem hiding this comment.
Please also fix the failing CI/CD checks.
Generic/predictive resumes with messages:[] must not prepend stored history into the turn input — confirm_changes synthesizes only the resume tool message and short-circuits without another model call. Seeding that history caused approval mismatch warnings and an extra stream invocation (test_agent_endpoint_confirm_changes_clears_persisted_interrupt). Only non-empty client-replayed generic resumes go through reconcile_resume_messages + seeded_resume_from_snapshot. Approval and workflow/checkpoint paths still seed empty via reconcile.
|
Evan Mattson (@moonbox3) Fixed on head \�682021\ — empty generic resumes no longer seed via |
|
LI (@ktz03) make sure to use the defined PR template |
|
Eduard van Valkenburg (@eavanvalkenburg) Updated the description to follow the repo PR template — please take another look when convenient. |
Per review: keep resume_seeded_messages as blind prepend (save-time / empty seeds). Use reconstruct only for non-empty client-replayed transcripts. Remove dedicated small test file and fold coverage into test_snapshot_session.
|
Eduard van Valkenburg (@eavanvalkenburg) Addressed both review points on latest head:
This should also clear the confirm_changes / prepend CI regressions from the earlier combine attempt. Please re-check when convenient. |
Annotate stored/incoming as list[dict[str, Any]] so Test Typing Checks pass.
|
Typing CI failed only on the folded replay fixture in |
Apply eavanvalkenburg suggestion: always overlap-merge via _reconstruct_messages_from_thread_snapshot; only set seeded_resume_from_snapshot when resume carries a non-empty transcript.
|
Eduard van Valkenburg (@eavanvalkenburg) Applied your suggestion on |
|
Eduard van Valkenburg (@eavanvalkenburg) Evan Mattson (@moonbox3) Friendly ping: eavan's duplicate-reconstruct suggestion is applied on |
|
Following up: CI is still green after the review threads. Dedupe of client-replayed transcripts on AG-UI resume remains an important HITL correctness fix. Anything left from the earlier maintainer notes that still needs a push, or is this waiting on a final review pass? |
When resume entries are fully handled via retained results, re-emit those results even if reconstruction refilled raw_messages, so a client-replayed approval retry does not invoke the agent again.
|
Eduard van Valkenburg (@eavanvalkenburg) Updated PR body to the repository template and addressed the latest review feedback on this head — please re-review when convenient. |
Motivation & Context
On AG-UI resume, clients often replay the full transcript in
messageswhile also sendingresume. Without dedupe, the same turn is persisted twice and replayed twice after hydrate (#8140). Replayed approval retries can also fall through to a fresh agent run after retained results are available.Fixes #8140.
Description & Review Guide
raw_messages._agent_run.py.Related Issue
Fixes #8140
No other open PR targets this issue.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.