Skip to content

Commit 7a7c92f

Browse files
authored
Python: fix(python): preserve non-adjacent tool pairs during history truncation (#14658)
## 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 - [x] The code builds clean without any errors or warnings in the focused local checks. - [x] The PR follows the Semantic Kernel Contribution Guide and local Ruff/Mypy checks. - [x] All unit tests pass, and I added focused tests for the non-adjacent call/result regression. - [x] I did not break anyone. - [x] AI assistance is disclosed. --------- Signed-off-by: hylin <linhongyu510@gmail.com>
1 parent ae13ada commit 7a7c92f

5 files changed

Lines changed: 438 additions & 6 deletions

File tree

‎python/semantic_kernel/contents/history_reducer/chat_history_reducer_utils.py‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ def get_call_result_pairs(history: list[ChatMessageContent]) -> list[tuple[int,
4040
pairs.append((call_index, j))
4141
# Remove the call ID so we don't match it a second time
4242
del call_ids_seen[call_id]
43-
break
4443

4544
return pairs
4645

@@ -132,12 +131,49 @@ def locate_safe_reduction_index(
132131
target_index = message_index
133132

134133
# Attempt to see if there's a user message in the threshold window
135-
while message_index >= threshold_index:
136-
if history[message_index].role == AuthorRole.USER:
137-
return message_index
138-
message_index -= 1
134+
candidate_index = message_index
135+
while candidate_index >= threshold_index and history[candidate_index].role != AuthorRole.USER:
136+
candidate_index -= 1
137+
if candidate_index < threshold_index:
138+
candidate_index = target_index
139+
140+
# Keep matched call/result pairs together even when they are not adjacent. The adjacency
141+
# pass above handles consecutive tool blocks; this final closure uses matching ID pairs so a
142+
# result in the retained tail cannot remain while its call starts before the cut.
143+
suffix_min_call_index = [total_count] * (total_count + 1)
144+
for call_index, result_index in get_call_result_pairs(history):
145+
suffix_min_call_index[result_index] = min(suffix_min_call_index[result_index], call_index)
146+
for index in range(total_count - 1, -1, -1):
147+
suffix_min_call_index[index] = min(suffix_min_call_index[index], suffix_min_call_index[index + 1])
148+
149+
# Iterate adjacency walk and ID closure to a fixed point: moving the cut back to satisfy
150+
# a matched pair may land inside an adjacent call/result block (e.g. a TOOL message with
151+
# plain text but no FunctionResultContent for the earlier call), so re-walk adjacency from
152+
# the new candidate and repeat until neither rule moves the index further back.
153+
while True:
154+
# Re-run adjacency walk from current candidate to avoid cutting inside a tool block.
155+
walk = candidate_index
156+
while walk >= offset_count:
157+
msg = history[walk]
158+
if msg.role in (AuthorRole.DEVELOPER, AuthorRole.SYSTEM):
159+
break
160+
if not contains_function_call_or_result(msg):
161+
break
162+
prev_idx = walk - 1
163+
if (prev_idx < offset_count) or not contains_function_call_or_result(history[prev_idx]):
164+
break
165+
walk -= 1
166+
if walk < candidate_index:
167+
candidate_index = walk
168+
169+
required_call_index = suffix_min_call_index[candidate_index]
170+
if required_call_index >= candidate_index:
171+
break
172+
if required_call_index < offset_count:
173+
return None
174+
candidate_index = required_call_index
139175

140-
return target_index
176+
return candidate_index
141177

142178

143179
@experimental
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# Copyright (c) Microsoft. All rights reserved.
2+
3+
from semantic_kernel.contents.chat_message_content import ChatMessageContent
4+
from semantic_kernel.contents.function_call_content import FunctionCallContent
5+
from semantic_kernel.contents.function_result_content import FunctionResultContent
6+
from semantic_kernel.contents.history_reducer.chat_history_reducer_utils import get_call_result_pairs
7+
from semantic_kernel.contents.utils.author_role import AuthorRole
8+
9+
10+
def _assistant_with(calls):
11+
m = ChatMessageContent(role=AuthorRole.ASSISTANT, content="")
12+
for cid in calls:
13+
m.items.append(FunctionCallContent(id=cid, function_name="f", plugin_name="p", arguments={}))
14+
return m
15+
16+
17+
def _tool_with(results):
18+
m = ChatMessageContent(role=AuthorRole.TOOL, content="")
19+
for rid in results:
20+
m.items.append(FunctionResultContent(id=rid, function_name="f", plugin_name="p", result="r"))
21+
return m
22+
23+
24+
def test_multiple_results_in_one_tool_message_are_all_paired():
25+
history = [
26+
ChatMessageContent(role=AuthorRole.USER, content="u1"),
27+
_assistant_with(["c1"]),
28+
_tool_with(["c1"]),
29+
ChatMessageContent(role=AuthorRole.USER, content="u2"),
30+
_assistant_with(["c2", "c3"]),
31+
_tool_with(["c2", "c3"]),
32+
]
33+
pairs = get_call_result_pairs(history)
34+
call_indices = [c for c, r in pairs]
35+
assert 4 in call_indices, f"Expected call at msg4 in pairs, got {pairs}"
36+
assert call_indices.count(4) == 2, f"Expected two pairs for msg4 (c2,c3), got {pairs}"
37+
38+
39+
def test_multiple_results_across_separate_messages():
40+
history = [
41+
ChatMessageContent(role=AuthorRole.USER, content="u1"),
42+
_assistant_with(["c1", "c2"]),
43+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="text"),
44+
_tool_with(["c1"]),
45+
_tool_with(["c2"]),
46+
]
47+
pairs = get_call_result_pairs(history)
48+
assert (1, 3) in pairs, f"Expected (1,3), got {pairs}"
49+
assert (1, 4) in pairs, f"Expected (1,4), got {pairs}"
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
# Copyright (c) Microsoft. All rights reserved.
2+
3+
from semantic_kernel.contents.chat_message_content import ChatMessageContent
4+
from semantic_kernel.contents.function_call_content import FunctionCallContent
5+
from semantic_kernel.contents.function_result_content import FunctionResultContent
6+
from semantic_kernel.contents.history_reducer.chat_history_reducer_utils import locate_safe_reduction_index
7+
from semantic_kernel.contents.history_reducer.chat_history_truncation_reducer import ChatHistoryTruncationReducer
8+
from semantic_kernel.contents.utils.author_role import AuthorRole
9+
10+
11+
def _assistant_call(call_id: str, function_name: str = "search") -> ChatMessageContent:
12+
message = ChatMessageContent(role=AuthorRole.ASSISTANT, content="")
13+
message.items.append(
14+
FunctionCallContent(id=call_id, function_name=function_name, plugin_name="plugin", arguments={"q": call_id})
15+
)
16+
return message
17+
18+
19+
def _tool_result(call_id: str, function_name: str = "search") -> ChatMessageContent:
20+
message = ChatMessageContent(role=AuthorRole.TOOL, content="")
21+
message.items.append(
22+
FunctionResultContent(id=call_id, function_name=function_name, plugin_name="plugin", result=f"result-{call_id}")
23+
)
24+
return message
25+
26+
27+
def _has_call(history: list[ChatMessageContent], call_id: str) -> bool:
28+
return any(
29+
isinstance(item, FunctionCallContent) and item.id == call_id for message in history for item in message.items
30+
)
31+
32+
33+
def _has_result(history: list[ChatMessageContent], call_id: str) -> bool:
34+
return any(
35+
isinstance(item, FunctionResultContent) and item.id == call_id for message in history for item in message.items
36+
)
37+
38+
39+
async def test_truncation_keeps_nonadjacent_function_call_with_its_result():
40+
history = [
41+
ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
42+
_assistant_call("call-1"),
43+
_tool_result("call-1"),
44+
ChatMessageContent(role=AuthorRole.USER, content="User message 2"),
45+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant text between tool rounds."),
46+
_assistant_call("call-2"),
47+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant text after tool call."),
48+
_tool_result("call-2"),
49+
]
50+
51+
assert history[1].role == AuthorRole.ASSISTANT
52+
assert history[2].role == AuthorRole.TOOL
53+
assert history[5].role == AuthorRole.ASSISTANT
54+
assert history[7].role == AuthorRole.TOOL
55+
56+
reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=0)
57+
reducer.messages = history
58+
result = await reducer.reduce()
59+
60+
assert result is not None
61+
assert _has_result(result.messages, "call-2")
62+
assert _has_call(result.messages, "call-2"), "A function result was retained without its matching function call."
63+
64+
65+
async def test_truncation_keeps_adjacent_function_call_result_pair():
66+
history = [
67+
ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
68+
_assistant_call("call-1"),
69+
_tool_result("call-1"),
70+
ChatMessageContent(role=AuthorRole.USER, content="User message 2"),
71+
_assistant_call("call-2"),
72+
_tool_result("call-2"),
73+
]
74+
75+
reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=0)
76+
reducer.messages = history
77+
result = await reducer.reduce()
78+
79+
assert result is not None
80+
assert _has_call(result.messages, "call-2")
81+
assert _has_result(result.messages, "call-2")
82+
83+
84+
async def test_truncation_keeps_multiple_calls_in_one_assistant_message():
85+
history = [
86+
ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
87+
_assistant_call("call-1"),
88+
_tool_result("call-1"),
89+
ChatMessageContent(role=AuthorRole.USER, content="User message 2"),
90+
ChatMessageContent(role=AuthorRole.ASSISTANT, content=""),
91+
ChatMessageContent(role=AuthorRole.TOOL, content=""),
92+
]
93+
assert history[4].role == AuthorRole.ASSISTANT
94+
assert history[5].role == AuthorRole.TOOL
95+
history[4].items.extend([
96+
FunctionCallContent(id="call-2", function_name="search", plugin_name="plugin", arguments={"q": "2"}),
97+
FunctionCallContent(id="call-3", function_name="lookup", plugin_name="plugin", arguments={"q": "3"}),
98+
])
99+
history[5].items.extend([
100+
FunctionResultContent(id="call-2", function_name="search", plugin_name="plugin", result="result-2"),
101+
FunctionResultContent(id="call-3", function_name="lookup", plugin_name="plugin", result="result-3"),
102+
])
103+
104+
reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=0)
105+
reducer.messages = history
106+
result = await reducer.reduce()
107+
108+
assert result is not None
109+
assert _has_call(result.messages, "call-2")
110+
assert _has_call(result.messages, "call-3")
111+
assert _has_result(result.messages, "call-2")
112+
assert _has_result(result.messages, "call-3")
113+
114+
115+
async def test_truncation_does_not_reduce_when_threshold_window_does_not_trigger():
116+
history = [
117+
_assistant_call("call-a"),
118+
ChatMessageContent(role=AuthorRole.USER, content="User message"),
119+
_tool_result("call-a"),
120+
]
121+
122+
reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=2)
123+
reducer.messages = history
124+
result = await reducer.reduce()
125+
126+
assert result is None
127+
128+
129+
async def test_truncation_keeps_pair_when_threshold_window_selects_user_boundary():
130+
history = [
131+
ChatMessageContent(role=AuthorRole.USER, content="User message 0"),
132+
_assistant_call("call-a"),
133+
_tool_result("call-a"),
134+
_assistant_call("call-b"),
135+
ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
136+
_tool_result("call-b"),
137+
]
138+
139+
reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=1)
140+
reducer.messages = history
141+
result = await reducer.reduce()
142+
143+
assert result is not None
144+
assert _has_result(result.messages, "call-b")
145+
assert _has_call(result.messages, "call-b")
146+
147+
148+
async def test_truncation_closure_keeps_chained_nonadjacent_pairs():
149+
history = [
150+
ChatMessageContent(role=AuthorRole.USER, content="User message 0"),
151+
_assistant_call("call-a"),
152+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant between calls."),
153+
_assistant_call("call-b"),
154+
ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
155+
_tool_result("call-b"),
156+
_tool_result("call-a"),
157+
]
158+
159+
reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=1)
160+
reducer.messages = history
161+
result = await reducer.reduce()
162+
163+
assert result is not None
164+
assert _has_call(result.messages, "call-a")
165+
assert _has_result(result.messages, "call-a")
166+
assert _has_call(result.messages, "call-b")
167+
assert _has_result(result.messages, "call-b")
168+
169+
170+
def test_locate_safe_reduction_index_rejects_pair_crossing_offset_boundary():
171+
history = [
172+
ChatMessageContent(role=AuthorRole.USER, content="User message 0"),
173+
_assistant_call("call-a"),
174+
ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
175+
_tool_result("call-a"),
176+
]
177+
178+
assert (
179+
locate_safe_reduction_index(
180+
history=history,
181+
target_count=1,
182+
threshold_count=0,
183+
offset_count=2,
184+
has_system_message=False,
185+
)
186+
is None
187+
)
188+
189+
190+
async def test_truncation_preserves_system_message_while_pairs_are_safe():
191+
reducer = ChatHistoryTruncationReducer(target_count=3, system_message="System prompt")
192+
reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="User message 1"))
193+
reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant 1"))
194+
reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="User message 2"))
195+
reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant 2"))
196+
197+
result = await reducer.reduce()
198+
199+
assert result is not None
200+
assert result.messages[0].role == AuthorRole.SYSTEM
201+
assert result.messages[0].content == "System prompt"
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# Copyright (c) Microsoft. All rights reserved.
2+
3+
from semantic_kernel.contents.chat_message_content import ChatMessageContent
4+
from semantic_kernel.contents.function_call_content import FunctionCallContent
5+
from semantic_kernel.contents.function_result_content import FunctionResultContent
6+
from semantic_kernel.contents.history_reducer.chat_history_truncation_reducer import ChatHistoryTruncationReducer
7+
from semantic_kernel.contents.utils.author_role import AuthorRole
8+
9+
10+
def _assistant_call(call_id: str) -> ChatMessageContent:
11+
m = ChatMessageContent(role=AuthorRole.ASSISTANT, content="")
12+
m.items.append(FunctionCallContent(id=call_id, function_name="f", plugin_name="p", arguments={}))
13+
return m
14+
15+
16+
def _tool_message_with(results: list[str]) -> ChatMessageContent:
17+
m = ChatMessageContent(role=AuthorRole.TOOL, content="")
18+
for rid in results:
19+
m.items.append(FunctionResultContent(id=rid, function_name="f", plugin_name="p", result=f"r-{rid}"))
20+
return m
21+
22+
23+
def _has_call(history: list[ChatMessageContent], call_id: str) -> bool:
24+
return any(
25+
isinstance(item, FunctionCallContent) and item.id == call_id for message in history for item in message.items
26+
)
27+
28+
29+
def _has_result(history: list[ChatMessageContent], call_id: str) -> bool:
30+
return any(
31+
isinstance(item, FunctionResultContent) and item.id == call_id for message in history for item in message.items
32+
)
33+
34+
35+
async def test_truncation_does_not_orphan_earlier_call_when_results_reversed_in_one_message():
36+
# c2 call is at msg1 (earlier). c3 call is at msg3 (later).
37+
# Both results are in ONE tool message (msg4), but ordered [result_c3, result_c2]
38+
# (later call first). The break bug skipped result_c2 in pair matching, leaving
39+
# c2's result orphaned when the cut removes msg1 but keeps msg4.
40+
history = [
41+
ChatMessageContent(role=AuthorRole.USER, content="u0"),
42+
_assistant_call("c2"),
43+
ChatMessageContent(role=AuthorRole.USER, content="u1"),
44+
_assistant_call("c3"),
45+
_tool_message_with(["c3", "c2"]), # reversed: later call result first
46+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="text"),
47+
]
48+
49+
reducer = ChatHistoryTruncationReducer(target_count=3, threshold_count=0)
50+
reducer.messages = history
51+
result = await reducer.reduce()
52+
53+
assert result is not None
54+
# If c3's result is kept, c3's call must be kept.
55+
if _has_result(result.messages, "c3"):
56+
assert _has_call(result.messages, "c3"), "c3 result kept without c3 call"
57+
# Critical: if c2's result is kept (it is in the same msg as c3's result),
58+
# c2's call must also be kept. Without the fix, c2 is orphaned.
59+
if _has_result(result.messages, "c2"):
60+
assert _has_call(result.messages, "c2"), "c2 result kept without c2 call (orphan)"

0 commit comments

Comments
 (0)