Skip to content

Python: Fix active mixed-pause Host response correlation - #8582

Closed
RongJie G (CorgiBoyG) wants to merge 1 commit into
microsoft:mainfrom
CorgiBoyG:fix/8573-active-host-correlation
Closed

RongJie G (CorgiBoyG) wants to merge 1 commit into
microsoft:mainfrom
CorgiBoyG:fix/8573-active-host-correlation

Conversation

@CorgiBoyG

@CorgiBoyG RongJie G (CorgiBoyG) commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Motivation & Context

An active session-backed mixed approval/Host batch can be resumed with a transcript that also contains an older id-less Host result using the same call_id. The matcher previously considered every response in the supplied transcript, so that historical result could be consumed as an active duplicate or assigned to another active occurrence.

This revision was rebuilt on the latest main after review feedback. It reduces the PR from 1,512 additions / 34 deletions to 758 additions / 17 deletions, with 99 additions / 3 deletions in production code. It does not add a session-state schema, transcript watermark, custom serializer, or provider-history protocol, and it does not change the stateless path.

Description & Review Guide

  • What are the major changes?
    • In _stage_pending_mixed_pause_responses, distinguish current inputs from HistoryProvider-injected messages using the framework's _attribution marker. Attributed history can neither establish an anchor nor be collected as an active response.
    • Select the active response window from the earliest current input that can answer the persisted batch: a bound approval, an exact Host occurrence, or an id-less result for an unresolved active call_id. This supports Host-first ordering in the same message, across current messages, and across partial resumes.
    • Only unresolved approval and exact Host items may establish a strong anchor. Replays of responses staged by an earlier partial resume remain available for equivalence checks but cannot move the active window back into stale history.
    • In _match_mixed_pause_responses, remember which response slots were already staged before the current resume. An equivalent id-less replay cannot fill a different sibling occurrence that shares the same call_id; a distinct result can still fill the unique unanswered occurrence.
    • Fail closed when conflicting id-less Host results in the same active window would otherwise be ordered by guesswork, while preserving exact occurrence-id precedence and equivalent duplicate handling.
    • Add focused regressions for historical id-less results before the active boundary, Host-first ordering, conflicting id-less results, identified/id-less precedence, partial-resume replay ambiguity, split approval/Host resumes, and exact Host results split across messages.
  • What is the impact of these changes?
    • Session-backed mixed batches correlate only responses with active-batch provenance.
    • Existing model ordering, active equivalent-duplicate handling, conflict rejection, and stateless matching remain unchanged.
    • There is no persisted-state format change and no migration requirement.
  • What do you want reviewers to focus on?

Validation:

  • Python 3.13 workspace suite: 16,719 passed, 421 skipped, 2 xfailed
  • Python 3.11 CI test typing: 205/205 package tasks and 3/3 sample tasks passed
  • test_function_invocation_logic.py: 387 passed, 1 skipped
  • Pyright: 0 errors and 0 warnings
  • Ruff, formatting, and git diff --check: passed

Related Issue

Fixes #8573

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.

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.

Copilot review overview

🟡 Changes recommended

Two correlation edge cases can reject valid responses or incorrectly reuse a staged Host result.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Scopes mixed-pause response correlation to the active persisted batch.

Changes:

  • Adds response-history watermarks and fail-closed validation.
  • Prioritizes occurrence-identified Host results and expands regression coverage.
  • Supports full-transcript and delta partial resumes.
File Description
python/​packages/​core/​agent_framework/​_tools.py Implements active-batch response ownership and replay handling.
python/​packages/​core/​tests/​core/​test_function_invocation_logic.py Adds mixed-pause ownership and conflict regressions.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +3466 to +3470
if any(historical == candidate_state for historical in historical_responses):
raise RuntimeError(
"The supplied response is indistinguishable from history that predates the active "
"mixed-pause batch."
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2e0e317. The historical-payload ambiguity check now applies only when no unique history watermark boundary can be established. When distinct message identities prove the active suffix, an equal current response is accepted. Added a regression covering distinct historical/current message IDs with the same occurrence ID and payload.

Comment on lines +3487 to +3492
candidate_state = candidate.to_dict()
for item in items:
stored_response = item.get("response")
stored_content = _content_from_state(stored_response)
if stored_content is None or stored_content.to_dict() != candidate_state:
continue

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2e0e317. Matching now distinguishes responses persisted before the current resume from responses matched during this resume. An id-less result equivalent to a preexisting identified Host response fails closed if it could fill another occurrence, while identified responses still reserve their occurrences first. The regression matrix also covers fresh versus partial resumes, stateful versus stateless matching, equal and distinct payloads, both input orders, and approval terminal-result preservation.

max_function_calls = self.function_invocation_configuration.get("max_function_calls")
max_duration_seconds = self.function_invocation_configuration.get("max_duration_seconds")
prepared_messages = _copy_messages_for_function_invocation(messages)
response_ownership_history = _copy_messages_for_function_invocation(messages)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RongJie G (@CorgiBoyG) This watermark history is captured before approval replay normalizes prepared_messages, so it can include a resolved function_approval_response that HistoryProvider later filters out. If an approval replay is followed by a new mixed approval/Host batch, the next full-history resume cannot match the stored watermark and equal current/historical Host payloads can fail as indistinguishable. Please base response_ownership_history on the post-_resolve_approval_responses transcript in both streaming and non-streaming paths before storing the batch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 7d9f0a1. I rebuilt the correlation change on top of the post-#8579 implementation rather than carrying the old approval-normalization path forward. Both streaming and non-streaming now seed ownership from the post-_resolve_approval_responses prepared_messages, and the watermark is refreshed after the creating model response is normalized so a same-turn terminal result cannot satisfy a newly created Host occurrence. The persisted raw and canonical boundaries cover both public full transcripts and HistoryProvider-filtered history while rejecting ambiguous boundaries. Added three-stage streaming/non-streaming regressions for approval replay followed by a mixed batch, equal historical/current Host payloads, metadata-only streaming calls, and the no-current-Host case remaining incomplete. Core and affected package suites pass.

@eavanvalkenburg

Copy link
Copy Markdown
Member

RongJie G (@CorgiBoyG) this is far too large of a change to a core area of the framework, please provide more details about why all these changes are needed to solve the issue, and make sure to use the defined PR template

@CorgiBoyG

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback. I rebuilt the change from the latest main rather than extending the previous implementation. The PR is now reduced from 1,512 additions / 34 deletions to 162 additions / 19 deletions, with only 51 additions / 3 deletions in production code. The custom session codec, transcript watermark, provider-history handling, state-schema changes, and unrelated corruption hardening have all been removed. The remaining production changes are limited to (1) selecting a content-level active response window from a bound approval or exact Host occurrence and (2) preventing an id-less replay of a previously staged result from filling a sibling occurrence with the same call_id. I also rewrote the PR description using the repository template, including the review guide, impact, issue link, and validation results. The latest Python 3.13 workspace suite passes with 15,461 passed, 414 skipped, and 2 xfailed; the focused invocation suite and static checks also pass. When convenient, could you please take another look at the reduced patch?

for content in message.contents
if content.type in {"function_approval_response", "function_result"}
for index, content in indexed_contents
if response_start is not None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RongJie G (@CorgiBoyG) A valid Host-first mixed resume can still be dropped here: if the current reply contains an id-less Host result followed by the bound approval response, response_start anchors at the approval and filters out the preceding Host result. The completed batch is then staged as incomplete. Please retain supported current-message id-less Host results that precede a later approval anchor, without admitting older history, and add a Host-first regression.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 99b27f8. The active window now starts at the beginning of the message containing the first bound approval or exact Host occurrence, so an id-less Host result that precedes the approval in the same current message is retained, while matching results in earlier messages remain inert. I added a Host-first regression that includes a prior-message historical result with the same call_id to verify that boundary. During adversarial review I also added a fail-closed guard for conflicting id-less results competing for the same occurrence, plus coverage confirming that an exact occurrence-id result remains authoritative. The branch is rebased onto the latest main, including #8750. Python 3.13 workspace tests pass with 16,365 passed, 414 skipped, and 2 xfailed; the Python 3.11 CI typing matrix passes 205/205 package tasks and 3/3 sample tasks.

if (
content.type == "function_approval_response"
and approval_anchor is None
and bind_approval_response(content) is not None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RongJie G (@CorgiBoyG) This anchor can come from an approval response that was already staged by an earlier partial resume. On a later full-history retry, replaying that old approval moves response_start back before the current reply, so an intervening stale id-less Host result with the active call_id can fill the remaining Host slot before the actual current result. Could the approval and exact-Host anchors ignore pending items that already have a response, with a regression covering a replayed staged approval plus stale and current Host results?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 91c4978. Strong anchors are now derived only from pending approval and exact Host items whose response is still unset. Replays of responses staged by an earlier partial resume remain available to the matcher for equivalence and conflict checks, but they can no longer move the active response window back into stale history. I added regressions for both cases: (1) a replayed staged approval followed by an intervening stale id-less Host result and the current result, and (2) a replayed staged exact Host result with a remaining sibling occurrence sharing the same call_id. Both verify that the stale message remains inert and the final current message completes the batch. The branch is rebased onto the latest main. Python 3.13 workspace tests pass with 16,547 passed, 414 skipped, and 2 xfailed; the Python 3.11 CI typing matrix passes 205/205 package tasks and 3/3 sample tasks.

default=None,
)
approval_is_staged = any(item.get("kind") == "approval" and item.get("response") is not None for item in items)
if response_start is None and approval_is_staged and messages:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RongJie G (@CorgiBoyG) A Host-first partial resume can still drop an id-less Host result when it arrives in an earlier request than the approval. With no approval or exact-Host anchor yet, this approval_is_staged gate leaves response_start unset, so the result is not persisted; the later approval then cannot recover it unless the caller resends the Host result. Could the current/delta id-less Host result establish the active window even before approval is staged, with a split-resume regression?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 79d1d69. A current id-less Host result can now establish the active window before its approval has been staged, so a Host-first split resume retains the result and a later approval completes the batch without requiring a resend. The same behavior is covered when Host and approval arrive in separate current messages and when an empty message follows the Host result. To keep this from widening the historical matching surface, correlation now uses the framework attribution boundary: messages injected by a ContextProvider carry _attribution and can neither establish an anchor nor be collected as active responses; only un-attributed current inputs participate. Regressions cover attributed approval/exact/id-less history, reused call IDs, staged replays, same-message and cross-message Host-first ordering, and partial resumes. Python 3.13 workspace tests pass with 16,719 passed, 421 skipped, and 2 xfailed; the Python 3.11 CI typing matrix passes 205/205 package tasks and 3/3 sample tasks.

@eavanvalkenburg

Copy link
Copy Markdown
Member

RongJie G (@CorgiBoyG) Thanks for the work on this. The outstanding Host-first split-resume concern from September 30 is still unresolved and there hasn't been further progress, so I'm closing this for now. Please reopen it once you're ready to address the remaining concern.

@CorgiBoyG

Copy link
Copy Markdown
Contributor Author

GitHub would not allow this PR to be reopened because its head branch had been updated while the PR was closed. The completed Host-first split-resume fix and its regression coverage are now available in #9203, which supersedes this PR.

This branch was previously deployed

1 inactive deployment
github-app-auth — 91c4978b Deployed Oct 8, 2026 by CorgiBoyG via add_label #24745
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: Scope mixed-pause Host response correlation to the active batch

3 participants