Skip to content

Python: fix: ChatHistoryTruncationReducer orphans TOOL role messages#13608

Open
giulio-leone wants to merge 1 commit intomicrosoft:mainfrom
giulio-leone:fix/truncation-reducer-orphaned-tool-messages
Open

Python: fix: ChatHistoryTruncationReducer orphans TOOL role messages#13608
giulio-leone wants to merge 1 commit intomicrosoft:mainfrom
giulio-leone:fix/truncation-reducer-orphaned-tool-messages

Conversation

@giulio-leone
Copy link
Contributor

Summary

Fixes #12708

ChatHistoryTruncationReducer.reduce() can orphan TOOL role messages by truncating the preceding assistant message that contains the tool_calls, causing OpenAI to reject the history with:

messages with role 'tool' must be a response to a preceding message with 'tool_calls'

Root Cause

locate_safe_reduction_index() uses contains_function_call_or_result() to detect tool-related messages during its backward scan. This function only checks msg.items for FunctionCallContent/FunctionResultContent instances.

However, TOOL role messages can contain only text content (no FunctionResultContent in items), which causes the backward scan to treat them as regular messages. The truncation point then lands between the tool_calls assistant message and its TOOL responses, orphaning the tool results.

Fix

Added AuthorRole.TOOL check to contains_function_call_or_result():

if msg.role == AuthorRole.TOOL:
    return True

This ensures any TOOL role message is recognized as part of a tool call/result pair, regardless of whether it has FunctionResultContent in its items.

Test

Added test_locate_safe_reduction_index_tool_role_without_function_result_content that verifies TOOL role messages with only text content are not separated from their tool call.

@giulio-leone giulio-leone requested a review from a team as a code owner February 28, 2026 17:55
@moonbox3 moonbox3 added the python Pull requests for the Python Semantic Kernel label Feb 28, 2026
@giulio-leone
Copy link
Contributor Author

Friendly ping — CI is green and this is ready for review. Happy to address any feedback. Thanks!

Copilot AI review requested due to automatic review settings March 4, 2026 04:29
@giulio-leone giulio-leone force-pushed the fix/truncation-reducer-orphaned-tool-messages branch from da24b50 to 095face Compare March 4, 2026 04:29
Copy link
Contributor

Copilot AI left a comment

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 fixes a truncation edge-case in the Python chat history reducers where truncation could leave AuthorRole.TOOL messages “orphaned” by dropping the preceding assistant message containing the associated tool_calls, which can cause OpenAI chat-completions requests to be rejected.

Changes:

  • Treat any AuthorRole.TOOL message as “tool-related” content when computing a safe truncation boundary.
  • Add a regression unit test covering TOOL-role messages that don’t carry FunctionResultContent in items.

Reviewed changes

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

File Description
python/semantic_kernel/contents/history_reducer/chat_history_reducer_utils.py Expands tool-related detection to include all TOOL role messages to prevent unsafe truncation boundaries.
python/tests/unit/contents/test_chat_history_reducer_utils.py Adds a regression test ensuring TOOL messages aren’t separated from their preceding tool-call assistant message during reduction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@giulio-leone
Copy link
Contributor Author

Hi! Gentle ping — this PR is rebased, CI passes, and ready for review. Happy to address any feedback. Thanks!

@giulio-leone giulio-leone force-pushed the fix/truncation-reducer-orphaned-tool-messages branch 2 times, most recently from 38adf27 to ce320d8 Compare March 9, 2026 14:12
@giulio-leone
Copy link
Contributor Author

Friendly ping — CI is green, tests pass, ready for review whenever convenient. Happy to address any feedback. Thanks! 🙏

@giulio-leone
Copy link
Contributor Author

Friendly follow-up in case this is still relevant — happy to update, rebase, or address any feedback if useful. Thanks!

auto-merge was automatically disabled March 19, 2026 14:46

Head branch was pushed to by a user without write access

@giulio-leone giulio-leone force-pushed the fix/truncation-reducer-orphaned-tool-messages branch from 5fbd12c to 9a20b3d Compare March 19, 2026 14:46
@giulio-leone
Copy link
Contributor Author

Refreshed this branch onto current origin/main (new head 9a20b3da9d047b37cde4b11317074d3bf5a9245e).

Local validation on the refreshed branch:

  • cd python && uv run --python 3.13 --extra pandas pytest tests/unit/contents/test_chat_history_reducer_utils.py -q -> 10 passed (rerun twice; only existing Pydantic deprecation warnings).
  • I first tried the lighter native path without the extra, but tests/conftest.py imports pandas, so adding the repo-declared optional extra was the highest-fidelity targeted pytest path available here.

Direct latest-main vs refreshed-branch proof using the real chat_history_reducer_utils.py from origin/main and from this refreshed head, loaded with lightweight message stubs:

  • origin/main: tool_msg_detected=false, reduction_index=2, kept_roles=["tool","user","assistant"], preserved_tool_call=false
  • refreshed branch: tool_msg_detected=true, reduction_index=1, kept_roles=["assistant","tool","user","assistant"], preserved_tool_call=true

So latest main still cuts at the plain-text TOOL message and orphans its paired assistant tool-call, while this PR still backs the cut up to the assistant/tool boundary after refresh.

@giulio-leone giulio-leone force-pushed the fix/truncation-reducer-orphaned-tool-messages branch from 9a20b3d to bafc816 Compare March 20, 2026 23:49
@giulio-leone
Copy link
Contributor Author

Rebased this branch onto the current main and force-pushed the refreshed head (bafc81669).

Local validation — double consecutive clean passes

Pass 1 and Pass 2 were run back-to-back with no code changes between them:

  • cd python && uv run --python 3.13 --extra pandas ruff check semantic_kernel/contents/history_reducer/chat_history_reducer_utils.py semantic_kernel/contents/history_reducer/chat_history_truncation_reducer.py tests/unit/contents/test_chat_history_reducer_utils.py
  • cd python && uv run --python 3.13 --extra pandas pytest tests/unit/contents/test_chat_history_reducer_utils.py::test_locate_safe_reduction_index_tool_role_without_function_result_content -q

Both passes were clean.

Direct runtime proof on the public reducer

Using ChatHistoryTruncationReducer with a real assistant tool-call message followed by a plain-text TOOL role result:

  • latest origin/main still reduces to ['tool', 'user', 'assistant'] and drops the preceding assistant tool-call
  • this rebased branch reduces to ['assistant', 'tool', 'user', 'assistant'] and preserves the assistant tool-call/tool-result pair

So the rebased branch still fixes the orphaned TOOL-message bug after refresh onto current main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: Bug: ChatHistoryTruncationReducer cuts off TOOL_CALLS role message making the following TOOL role messages orphan in AgentThread history

4 participants