Skip to content

Python: Give the AG-UI Thread Snapshot lifecycle a single owner module - #7479

Open
moonbox3 wants to merge 2 commits into
microsoft:mainfrom
moonbox3:moonbox3/agui-thread-snapshot-session
Open

Python: Give the AG-UI Thread Snapshot lifecycle a single owner module#7479
moonbox3 wants to merge 2 commits into
microsoft:mainfrom
moonbox3:moonbox3/agui-thread-snapshot-session

Conversation

@moonbox3

@moonbox3 moonbox3 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

The AG-UI Thread Snapshot is the package's central continuity record, but its lifecycle had no single owner: the agent runner and the workflow runner each carried their own implementation of hydration replay, the load-once stored read, resume message seeding, the stored/request/deferred-default state overlay, and the save whose storage failure must never surface on an already-streamed run. The copies had already drifted in small ways: one hydrate helper re-checked a store its caller had verified, and the two cancelled-resume-id helpers differed on missing-id handling (the workflow copy would raise KeyError on a cancelled interrupt entry with no id; the agent copy skips it).

Description & Review Guide

  • New private module _snapshot_session.py with ThreadSnapshotSession, opened once per run. When no store or scope is configured the session is inert (reads return nothing, writes are no-ops), so runners no longer branch on configuration at every call site. Interface: open, enabled, stored, hydrate_events, effective_state, resume_seeded_messages, save, clear_interrupts.
  • Both runners are rewired onto the session. Deleted the superseded per-runner copies: _hydrate_thread_snapshot, _save_thread_snapshot (agent), _hydrate_workflow_thread_snapshot (workflow), plus the duplicated _event_messages_to_snapshot_dicts (now in _snapshot_session.py) and _cancelled_resume_interrupt_ids (now in _run_common.py, consolidated on the defensive variant).
  • Developed test-first: 20 new tests drive the session interface only, using the real InMemoryAGUIThreadSnapshotStore plus a failing-store stub for the swallow-and-log path. No mocks of internals.
  • _WorkflowSnapshotBuilder intentionally stays in _workflow.py; moving it is out of scope here.

Impact: none intended at runtime. 977 tests pass (957 baseline plus 20 new); ruff, formatting, and pyright are clean; the 3 remaining mypy errors are identical on main. Public exports are unchanged. One observable difference: the save-failure log line is now emitted from _snapshot_session instead of the runner modules; the message text is unchanged.

Review focus: the equivalence of the rewired runner paths to the deleted inline logic, especially the agent runner's approval-gated resume seeding and the workflow save guard conditions.

Related Issue

Internal refactor, no linked issue.

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.

Both the agent and workflow runners independently implemented the thread
snapshot lifecycle: hydration replay, the load-once stored read, resume
message seeding, the stored/request/deferred-default state overlay, and
the save whose storage failures must never surface on an already-streamed
run. The two copies had already drifted in small ways (one hydrate helper
re-checked a store the caller had verified; the two cancelled-resume-id
helpers differed on missing-id handling).

Introduce ThreadSnapshotSession in _snapshot_session.py as the one owner
of that lifecycle, opened once per run and inert when no store or scope
is configured so callers stop branching on configuration. Rewire both
runners onto it, consolidate _cancelled_resume_interrupt_ids in
_run_common (defensive variant) and _event_messages_to_snapshot_dicts in
the new module, and delete the superseded per-runner copies. The session
interface is covered by dedicated tests; existing suites pin runner
behavior. Public exports are unchanged.
Copilot AI review requested due to automatic review settings August 3, 2026 01:41
@moonbox3
moonbox3 temporarily deployed to github-app-auth August 3, 2026 01:41 — with GitHub Actions Inactive
@moonbox3
moonbox3 temporarily deployed to github-app-auth August 3, 2026 01:41 — with GitHub Actions Inactive
@moonbox3
moonbox3 temporarily deployed to github-app-auth August 3, 2026 01:41 — with GitHub Actions Inactive
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Aug 3, 2026
@moonbox3
moonbox3 temporarily deployed to github-app-auth August 3, 2026 01:42 — with GitHub Actions Inactive

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

This PR centralizes the AG-UI Thread Snapshot lifecycle for Python runs into a single owner module (ThreadSnapshotSession), removing duplicated snapshot hydration/state overlay/resume seeding/save logic from both the agent runner and workflow runner. This aims to keep snapshot behavior consistent across runners while preserving the existing runtime semantics (including “swallow-and-log” snapshot save failures after streaming has completed).

Changes:

  • Introduces ThreadSnapshotSession in a new private module to own snapshot open/load, hydration replay, effective-state overlay, resume message seeding, saving, and interrupt clearing.
  • Rewires both the agent runner and workflow runner to use the session, deleting the previously duplicated per-runner helpers and consolidating cancelled-resume interrupt-id extraction.
  • Adds a new test suite covering the session interface (including save-failure swallow-and-log semantics and interrupt clearing behavior).

Reviewed changes

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

Show a summary per file
File Description
python/packages/ag-ui/tests/ag_ui/test_snapshot_session.py Adds interface-level tests for ThreadSnapshotSession covering open/hydrate/effective_state/resume seeding/save/clear semantics.
python/packages/ag-ui/agent_framework_ag_ui/_workflow.py Switches workflow snapshot lifecycle operations to ThreadSnapshotSession and removes duplicated helpers.
python/packages/ag-ui/agent_framework_ag_ui/_snapshot_session.py New per-run session module that becomes the single owner of snapshot lifecycle interactions.
python/packages/ag-ui/agent_framework_ag_ui/_run_common.py Moves and consolidates _cancelled_resume_interrupt_ids for shared use across runners.
python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py Switches agent runner snapshot lifecycle operations to ThreadSnapshotSession and removes duplicated helpers.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/ag-ui/agent_framework_ag_ui
   _agent_run.py113111589%186–193, 240–241, 248, 357, 361, 363, 380, 407–408, 521, 535, 539, 543, 546, 551, 556, 565, 568, 575–581, 614, 626, 637, 640, 675, 729–733, 786, 801, 804, 806, 832, 858–860, 918, 920, 922, 925–929, 943, 951–956, 965–966, 1015–1018, 1029, 1037, 1069, 1084, 1098, 1110, 1140, 1144, 1147, 1149, 1189–1191, 1255, 1261–1262, 1267, 1271–1272, 1470, 1478, 1508, 1512, 1557, 1759, 1887, 1947, 1964, 1984–1985, 1992, 2097, 2125, 2133, 2135, 2138, 2144, 2199, 2202, 2212–2213, 2220, 2267
   _run_common.py6004792%76, 151–152, 154, 156, 159, 165, 167, 181, 188, 212–213, 226, 245, 281, 325, 337, 339, 341, 344–348, 526, 806–807, 813–818, 1129–1130, 1135, 1137–1139, 1148, 1156, 1168, 1170–1173, 1236
   _snapshot_session.py70198%40
   _workflow.py175895%109, 114, 153, 255, 295, 307–309
TOTAL47346451690% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
9763 34 💤 0 ❌ 0 🔥 2m 33s ⏱️

@github-actions github-actions Bot 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.

Automated Code Review

Reviewers: 5 | Confidence: 51% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Failure Modes, Design Approach


Automated review by moonbox3's agents

The hydration test accessed run_id, snapshot, and messages on values
typed as BaseEvent, which fails the tests/samples type checkers. Narrow
each event with isinstance assertions before reading its fields.
@moonbox3
moonbox3 temporarily deployed to github-app-auth August 3, 2026 02:24 — with GitHub Actions Inactive
@moonbox3
moonbox3 enabled auto-merge August 3, 2026 02:53
@moonbox3 moonbox3 added the ag-ui Usage: [Issues, PRs], Target: AG-UI protocol integration label Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ag-ui Usage: [Issues, PRs], Target: AG-UI protocol integration python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants