Repository navigation
Python: isolate structured output transform history per invocation - #14487
jiangjiang248 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
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
ChatHistoryfor 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.
|
@microsoft-github-policy-service agree |
|
The current head (
These jobs did not execute their normal steps, and the merge gatekeeper also remains failed. Could a maintainer check the repository's |
There was a problem hiding this comment.
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
ChatHistoryobject 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 seesfirst, and the third sees both earlier outputs. I then ran three more calls withasyncio.gatheron the same transform: they all received that one shared history object, which ended with six messages (the three earlier ones plusc0,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.pyplustests/unit/contents/test_chat_history.py: 62 passed. - The PR's new
test_structured_outputs_transform_keeps_invocations_isolatedrun against main'stools.pyfails atassert 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.
Motivation and Context
Fixes #14483.
structured_outputs_transformretained one mutableChatHistoryacross 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
ChatHistoryinside eachoutput_transforminvocation. A regression test calls the same transform twice and checks that each service request contains only its own input and the system message.Verification
semantic-kernel==1.44.1because both calls shared the sameChatHistory.python -m pytest .../test_orchestration_tools.py -q -o asyncio_mode=auto: 7 passed after the fix.checkandformat --checkpassed for both changed files using the repository'spython/pyproject.tomlconfiguration.Contribution Checklist