Uh oh!
There was an error while loading. Please reload this page.
Python: fix(python): separate provider input from UI snapshot in AG-UI service-session mode - #7770
Conversation
There was a problem hiding this comment.
Pull request overview
Separates service-managed provider history from AG-UI snapshot history.
Changes:
- Prevents stored snapshot replay into service-session provider calls.
- Reconstructs full snapshots for UI hydration.
- Adds split-authority regression coverage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
_agent_run.py | Separates provider input and persisted snapshots. |
test_snapshots.py | Tests incremental provider input and full snapshot history. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| stored_interrupt=stored_snapshot.interrupt, | ||
| ) | ||
| else: | ||
| stored_count = len(stored_snapshot.messages) |
There was a problem hiding this comment.
Could we avoid using len(stored_snapshot.messages) as an unconditional cursor? It assumes every request contains an exact, current stored prefix. Incremental requests then lose the new turn, direct confirm_changes responses can truncate persisted history, and a stale snapshot after a failed write can replay an already-processed turn into the service session. Could we derive a validated suffix by identity while keeping the full input for snapshot reconstruction?
| seeded_resume_from_snapshot = True | ||
| else: | ||
| if not config.use_service_session: |
There was a problem hiding this comment.
Would it make sense for one snapshot/session helper to produce both views for a run? The new branches at _agent_run.py:2273-2286 use a positional len(stored_snapshot.messages) cursor for provider input, while _reconstruct_messages_from_thread_snapshot() at _agent_run.py:2389-2397 separately decides which messages belong in the UI snapshot. Those paths already disagree for incremental requests and approval responses, so a helper such as _split_service_session_input(...) returning the provider suffix and persisted snapshot would keep the prefix, suffix, and resume rules in one place.
Motivation & Context
When
AgentFrameworkAgentis configured with bothuse_service_session=Trueand asnapshot_store, the AG-UI runner unconditionally reconstructs the full stored message history and passes it to the wrapped agent alongside theservice_session_id. Foundry Hosted Agents using the Responses protocol reject replayed assistant messages with an HTTP 400 error, because the server already owns conversation history via the service session. This blocks multi-turn applications that require both Foundry-managed conversation continuation and AG-UI snapshot persistence for UI hydration or approval state.This change enforces a split-authority model consistent with the framework’s “one State Authority” contract and analogous to the fix previously applied to
AgentExecutorin #7682.Fixes#7710
Description & Review Guide
What are the major changes?
run_agent_stream, message reconstruction from the thread snapshot is now conditional onconfig.use_service_session. When service-session mode is active,raw_messages(sent to the provider) contains only incremental input, whilesnapshot_messages(used for UI hydration and persistence) retains the fully reconstructed transcript.latest_messages_snapshotbefore saving, even whenshould_emit_snapshotevaluates toFalse. This prevents UI history collapse when agents emit content types that don’t trigger the standard snapshot emission path.test_service_session_snapshot_split_authorityverifying both halves of the split: provider receives incremental-only input, and the snapshot store retains the complete transcript.What is the impact of these changes?
AgentFrameworkAgent→FoundryAgent→ Foundry Hosted Agent (Responses protocol) with snapshot persistence enabled.use_service_session=False.snapshot_messagesand persisted to the store.What do you want reviewers to focus on?
_reconstruct_messages_from_thread_snapshotandresume_seeded_messages— specifically that no code path leaks reconstructed history intoraw_messageswhenuse_service_session=True.should_emit_snapshotisFalsebut the UI still needs the current turn’s output.Related Issue
Fixes#7710
Contribution Checklist