Skip to content

Python: fix(python): preserve non-adjacent tool pairs during history truncation - #14658

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 5 commits into
microsoft:mainfrom
linhongyu510:fix/nonadjacent-tool-pair-truncation
Nov 2, 2026
Merged

Python: fix(python): preserve non-adjacent tool pairs during history truncation#14658
Eduard van Valkenburg (eavanvalkenburg) merged 5 commits into
microsoft:mainfrom
linhongyu510:fix/nonadjacent-tool-pair-truncation

Conversation

@linhongyu510

@linhongyu510 hylin (linhongyu510) commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Title

fix(python): preserve non-adjacent tool pairs during history truncation

Motivation and Context

  • Problem: ChatHistoryTruncationReducer could retain a FunctionResultContent while dropping its matching FunctionCallContent when the two were separated by non-function messages.
  • Existing contract: adjacent tool blocks were already protected, and matched ID pairs are used elsewhere by the reducer utilities. A retained tool result should not become an orphan after truncation.
  • Why this is minimal: the fix chooses the existing user-threshold candidate boundary first, then applies a final matched-ID pair closure.
  • Scope: offline chat-history reduction only; no provider, network, E2E, public API, dependency, lockfile, or documentation-only refactor.
  • Issue: self-discovered during audit; independent dedup found no equivalent open issue/PR.

Description

locate_safe_reduction_index now builds an O(n) suffix minimum-call map from get_call_result_pairs(history). After the existing adjacency and threshold-window candidate selection, it moves the cut back only when a retained result requires an earlier matched call. Chained non-adjacent pairs are closed transitively. If the required call is before offset_count, reduction returns None rather than crossing offset/summary territory.

Offset, threshold, system/developer preservation, unknown/unpaired content, and existing user-boundary search semantics are otherwise unchanged.

Local verification

  • Base: 483ffb487a33a1644886992d56599feae3b69b76.
  • Pre-fix formal regression: 4 failed, 4 passed.
  • Post-fix formal regression: 8 passed.
  • Production-only revert cycle: red exit 1, restore green exit 0.
  • Related chat-history suite: 105 passed.
  • Ruff: 0.9.10; check and format check pass.
  • Mypy: targeted changed production file passes.
  • Multi-Python matrix and wheel build were not run in this preparation step.
  • AI assistance was used; an independent local review was rerun before publication. No external CLI review is claimed.

Contribution Checklist

  • The code builds clean without any errors or warnings in the focused local checks.
  • The PR follows the Semantic Kernel Contribution Guide and local Ruff/Mypy checks.
  • All unit tests pass, and I added focused tests for the non-adjacent call/result regression.
  • I did not break anyone.
  • AI assistance is disclosed.

@semantic-kernel-automation semantic-kernel-automation Bot added the python Pull requests for the Python Semantic Kernel label Oct 24, 2026
@github-actions github-actions Bot changed the title fix(python): preserve non-adjacent tool pairs during history truncation Python: fix(python): preserve non-adjacent tool pairs during history truncation Oct 24, 2026
@linhongyu510
fix(python): preserve non-adjacent tool pairs during history truncation
4c489f2 
Preserve matched FunctionCallContent and FunctionResultContent pairs when ChatHistoryTruncationReducer cuts between a call and its result.

Co-authored-by: AI assistance was used and reviewed.

Signed-off-by: hylin <[email protected]>
@linhongyu510
hylin (linhongyu510) force-pushed the fix/nonadjacent-tool-pair-truncation branch from 3cd8e4b to 4c489f2 Compare October 27, 2026 17:14
@linhongyu510

hylin (linhongyu510) commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream main snapshot b5b526d9b8bd39da41db4a5c0f1aff20d26f7ec8 (2026-10-27).

Local verification on this rebased HEAD (4c489f2d44a9d41164d40c2b27aaf59333421b70), frozen lock env (uv 0.13.1, uv sync --frozen --all-extras --dev):

  • Formal regression suite: 8 passed.
  • Production-only revert (base bytes restored via try/finally): exactly 4 failed / 4 passed, exit 1; restored HEAD green 8 passed.
  • Focused reducer suite: 43 passed.
  • Related chat-history suite: 212 passed.
  • Official type gate mypy -p semantic_kernel --config-file mypy.ini: no issues found in 556 source files (HEAD and base).
  • Ruff check: clean.
  • Official pre-commit (--all-files): 13 hooks passed (exit 0).
  • Warnings: pytest-timeout config warning resolved by installing the locked plugin; Pydantic deprecation warnings are pre-existing on base (40 in focused, 52 in related suite).
  • AI assistance was used and reviewed; this is an independent local verification, not a maintainer review. Multi-Python matrix / wheel build not run in this step.

@linhongyu510
hylin (linhongyu510) marked this pull request as ready for review October 29, 2026 17:03
@linhongyu510
hylin (linhongyu510) requested a review from a team as a code owner October 29, 2026 17:03
Copilot AI balanced review requested due to automatic review settings October 29, 2026 17:03
Copilot AI reviewed Oct 29, 2026
View reviewed changes

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.

🟡 Changes recommended

History truncation can still orphan tool results because only the first matched result per message is collected and backward cut adjustments can split adjacent tool blocks.

2 open issues
What changed in this PR

This PR updates Python chat-history truncation to retain matching function calls when their results appear later, separated by other messages.

Changes:

  • Adds matched-ID boundary adjustment after existing adjacency and user-threshold selection.
  • Handles chained pair dependencies and rejects cuts that require crossing the offset boundary.
  • Adds regression tests for pair preservation, thresholds, offsets, and system-message retention.
File Description
python/​tests/​unit/​contents/​test_chat_history_truncation_pair_invariant.py Adds focused truncation and pair-preservation regression tests.
python/​semantic_kernel/​contents/​history_reducer/​chat_history_reducer_utils.py Adjusts truncation boundaries using suffix minima of matched call indices.

Review effort: Balanced

Comment thread python/semantic_kernel/contents/history_reducer/chat_history_reducer_utils.py Resolved
Comment thread python/semantic_kernel/contents/history_reducer/chat_history_reducer_utils.py Resolved
@linhongyu510
fix(python): match all function results per message in get_call_resul…
c814f99 
…t_pairs

Remove the early break that skipped subsequent FunctionResultContent items
in a single tool message. When parallel tool calls share one result message
with results ordered differently, the break caused earlier call results
to be orphaned from pair tracking, which could leave a result in the
retained tail without its call after truncation.

Adds regression tests for multiple results per message (internal) and a
public ChatHistoryTruncationReducer test demonstrating the orphan with
reversed result ordering.

Responds to Copilot review r4415468144 on PR microsoft#14658.
@linhongyu510
fix(python): iterate adjacency walk with ID closure to prevent tool-b…
0152f3c 
…lock split

When the ID-closure pass moves the cut back to keep a matched pair, the
new candidate may land inside an adjacent call/result block — e.g. a TOOL
message carrying plain text with no FunctionResultContent for the earlier
call. Re-walk adjacency from the candidate and repeat ID closure until a
fixed point, so a text-only TOOL result is never orphaned from its call.

Covers Copilot review r4415468227 (text-only TOOL orphan) and a chained
fixed-point case. Existing pair-invariant controls unchanged.
@linhongyu510
fix(python): correct chat_history_reducer_utils.py bytes to match gol…
9a73657 
…den LF content

Follow-up to 0152f3c: the inline transcription drifted byte-wise from the
golden LF source (blob mismatch). Replace with the exact verified content.
No logic change.
@linhongyu510
fix(python): restore exact validated tool-block closure source bytes
71c6697 
Clarify docstring by correcting phrasing regarding SUMMARY_METADATA_KEY and end index description.
Merged via the queue into microsoft:main with commit 7a7c92f Nov 2, 2026
33 checks passed

This branch was previously deployed

1 inactive deployment
github-app-auth — 71c66978 Deployed Nov 1, 2026 by linhongyu510 via add_label #29530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) eavanvalkenburg approved these changes

Assignees

No one assigned

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@linhongyu510 @eavanvalkenburg