Skip to content

Python: isolate structured output transform history per invocation - #14487

Open
jiangjiang248 wants to merge 1 commit into
microsoft:mainfrom
jiangjiang248:fix-structured-output-history-14483
Open

jiangjiang248 wants to merge 1 commit into
microsoft:mainfrom
jiangjiang248:fix-structured-output-history-14483

Conversation

@jiangjiang248

Copy link
Copy Markdown

Motivation and Context

Fixes #14483.

structured_outputs_transform retained one mutable ChatHistory across calls. When the same transform was invoked again, the service received messages from earlier invocations. This can leak prior orchestration output into the next structured-output prompt and grow the request over time.

Description

Keep the shared system prompt, but create a new ChatHistory inside each output_transform invocation. A regression test calls the same transform twice and checks that each service request contains only its own input and the system message.

Verification

  • The new regression test failed against unmodified semantic-kernel==1.44.1 because both calls shared the same ChatHistory.
  • python -m pytest .../test_orchestration_tools.py -q -o asyncio_mode=auto: 7 passed after the fix.
  • Ruff 0.9.10 check and format --check passed for both changed files using the repository's python/pyproject.toml configuration.

Contribution Checklist

  • Focused unit tests pass, including the new regression test.
  • Changed files pass the repository's Ruff configuration.
  • Full repository test suite and build were not run locally.

Copilot AI lite review requested due to automatic review settings September 26, 2026 05:35
@jiangjiang248
jiangjiang248 requested a review from a team as a code owner September 26, 2026 05:35

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

🟢 Approval recommended

No unresolved issues were identified, and regression coverage passes.

Review effort: Lite
Findings: None

What changed in this PR

Updates structured-output transforms to prevent chat history from leaking between invocations.

Changes:

  • Creates a fresh ChatHistory for each invocation.
  • Adds regression coverage for repeated calls.
File Summary
python/​tests/​unit/​agents/​orchestration/​test_orchestration_tools.py Verifies invocation histories remain independent.
python/​semantic_kernel/​agents/​orchestration/​tools.py Isolates per-invocation chat history.

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

@jiangjiang248

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@jiangjiang248

Copy link
Copy Markdown
Author

The current head (896e079ecfe40c95a75ab0f7dcdae8376c53361c) has passing Python unit tests, integration tests, coverage, and code-quality checks. The failing review and add_label checks both report:

Branch "main" is not allowed to deploy to github-app-auth due to environment protection rules.

These jobs did not execute their normal steps, and the merge gatekeeper also remains failed. Could a maintainer check the repository's github-app-auth environment protection settings and re-run the affected workflows once the configuration permits them? No workflow or environment settings have been changed in this PR.

Check details: review, add_label.

@VANDRANKI PRABHU KIRAN VANDRANKI (VANDRANKI) left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Community review, not a maintainer review, so this does not clear the merge gate. I read the full diff and ran it against this branch and against origin/main with the PR's test file copied in (Python 3.13, python/ of a local worktree).

The bug is real and the fix is correct for what I ran.

On main, structured_outputs_transform builds one ChatHistory when the transform is created, and output_transform appends to it on every call. The history is never reset and the response is not added to it. I wrapped the real MockChatCompletionService.get_chat_message_content in an AsyncMock and called the transform three times (first, second, third):

  • main: the same ChatHistory object is passed to every call and it keeps growing, so after the third call it holds ['first', 'second', 'third'] after the system message. The second call therefore already sees first, and the third sees both earlier outputs. I then ran three more calls with asyncio.gather on the same transform: they all received that one shared history object, which ended with six messages (the three earlier ones plus c0, c1, c2).
  • this branch: each call gets its own history, and the three calls saw ['first'], ['second'], ['third']. The three concurrent calls saw ['c0'], ['c1'], ['c2'].

Test results:

  • On this branch: tests/unit/agents/orchestration/test_orchestration_tools.py plus tests/unit/contents/test_chat_history.py: 62 passed.
  • The PR's new test_structured_outputs_transform_keeps_invocations_isolated run against main's tools.py fails at assert histories[0] is not histories[1], so it does detect the bug.

Why it matters beyond the unit test: the only caller I found is the sample step1a_concurrent_structured_outputs.py, which passes the transform as output_transform to a concurrent orchestration. Reusing that orchestration for a second run would send the first run's agent outputs to the summarizer too, and concurrent runs would see each other's messages.

Not examined: the settings object is still created once and shared across calls. I did not check whether any service mutates it during a call, so I cannot say whether concurrent calls are fully isolated beyond the history.

The change is small and does what it says.

This branch had an error being deployed

1 failed deployment
github-app-auth — 896e079e Deployed Sep 26, 2026 by jiangjiang248 via add_label #29064
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Python] structured_outputs_transform reuses ChatHistory across calls (prompt pollution)

3 participants