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

python/semantic_kernel/contents/history_reducer/chat_history_reducer_utils.py

@@ -40,7 +40,6 @@ def get_call_result_pairs(history: list[ChatMessageContent]) -> list[tuple[int,
                     pairs.append((call_index, j))
                     # Remove the call ID so we don't match it a second time
                     del call_ids_seen[call_id]
-                    break
 
     return pairs
 
@@ -132,12 +131,30 @@ def locate_safe_reduction_index(
     target_index = message_index
 
     # Attempt to see if there's a user message in the threshold window
-    while message_index >= threshold_index:
-        if history[message_index].role == AuthorRole.USER:
-            return message_index
-        message_index -= 1
+    candidate_index = message_index
+    while candidate_index >= threshold_index and history[candidate_index].role != AuthorRole.USER:
+        candidate_index -= 1
+    if candidate_index < threshold_index:
+        candidate_index = target_index
+
+    # Keep matched call/result pairs together even when they are not adjacent. The adjacency
+    # pass above handles consecutive tool blocks; this final closure uses matching ID pairs so a
+    # result in the retained tail cannot remain while its call starts before the cut.
+    suffix_min_call_index = [total_count] * (total_count + 1)
+    for call_index, result_index in get_call_result_pairs(history):
+        suffix_min_call_index[result_index] = min(suffix_min_call_index[result_index], call_index)
+    for index in range(total_count - 1, -1, -1):
+        suffix_min_call_index[index] = min(suffix_min_call_index[index], suffix_min_call_index[index + 1])
+
+    while True:
+        required_call_index = suffix_min_call_index[candidate_index]
+        if required_call_index >= candidate_index:
+            break
+        if required_call_index < offset_count:
+            return None
+        candidate_index = required_call_index
 
-    return target_index
+    return candidate_index
 
 
 @experimental

python/tests/unit/contents/test_chat_history_multi_result_pairing.py

@@ -0,0 +1,49 @@
+# Copyright (c) Microsoft. All rights reserved.
+
+from semantic_kernel.contents.chat_message_content import ChatMessageContent
+from semantic_kernel.contents.function_call_content import FunctionCallContent
+from semantic_kernel.contents.function_result_content import FunctionResultContent
+from semantic_kernel.contents.history_reducer.chat_history_reducer_utils import get_call_result_pairs
+from semantic_kernel.contents.utils.author_role import AuthorRole
+
+
+def _assistant_with(calls):
+    m = ChatMessageContent(role=AuthorRole.ASSISTANT, content="")
+    for cid in calls:
+        m.items.append(FunctionCallContent(id=cid, function_name="f", plugin_name="p", arguments={}))
+    return m
+
+
+def _tool_with(results):
+    m = ChatMessageContent(role=AuthorRole.TOOL, content="")
+    for rid in results:
+        m.items.append(FunctionResultContent(id=rid, function_name="f", plugin_name="p", result="r"))
+    return m
+
+
+def test_multiple_results_in_one_tool_message_are_all_paired():
+    history = [
+        ChatMessageContent(role=AuthorRole.USER, content="u1"),
+        _assistant_with(["c1"]),
+        _tool_with(["c1"]),
+        ChatMessageContent(role=AuthorRole.USER, content="u2"),
+        _assistant_with(["c2", "c3"]),
+        _tool_with(["c2", "c3"]),
+    ]
+    pairs = get_call_result_pairs(history)
+    call_indices = [c for c, r in pairs]
+    assert 4 in call_indices, f"Expected call at msg4 in pairs, got {pairs}"
+    assert call_indices.count(4) == 2, f"Expected two pairs for msg4 (c2,c3), got {pairs}"
+
+
+def test_multiple_results_across_separate_messages():
+    history = [
+        ChatMessageContent(role=AuthorRole.USER, content="u1"),
+        _assistant_with(["c1", "c2"]),
+        ChatMessageContent(role=AuthorRole.ASSISTANT, content="text"),
+        _tool_with(["c1"]),
+        _tool_with(["c2"]),
+    ]
+    pairs = get_call_result_pairs(history)
+    assert (1, 3) in pairs, f"Expected (1,3), got {pairs}"
+    assert (1, 4) in pairs, f"Expected (1,4), got {pairs}"

python/tests/unit/contents/test_chat_history_truncation_pair_invariant.py

@@ -0,0 +1,201 @@
+# Copyright (c) Microsoft. All rights reserved.
+
+from semantic_kernel.contents.chat_message_content import ChatMessageContent
+from semantic_kernel.contents.function_call_content import FunctionCallContent
+from semantic_kernel.contents.function_result_content import FunctionResultContent
+from semantic_kernel.contents.history_reducer.chat_history_reducer_utils import locate_safe_reduction_index
+from semantic_kernel.contents.history_reducer.chat_history_truncation_reducer import ChatHistoryTruncationReducer
+from semantic_kernel.contents.utils.author_role import AuthorRole
+
+
+def _assistant_call(call_id: str, function_name: str = "search") -> ChatMessageContent:
+    message = ChatMessageContent(role=AuthorRole.ASSISTANT, content="")
+    message.items.append(
+        FunctionCallContent(id=call_id, function_name=function_name, plugin_name="plugin", arguments={"q": call_id})
+    )
+    return message
+
+
+def _tool_result(call_id: str, function_name: str = "search") -> ChatMessageContent:
+    message = ChatMessageContent(role=AuthorRole.TOOL, content="")
+    message.items.append(
+        FunctionResultContent(id=call_id, function_name=function_name, plugin_name="plugin", result=f"result-{call_id}")
+    )
+    return message
+
+
+def _has_call(history: list[ChatMessageContent], call_id: str) -> bool:
+    return any(
+        isinstance(item, FunctionCallContent) and item.id == call_id for message in history for item in message.items
+    )
+
+
+def _has_result(history: list[ChatMessageContent], call_id: str) -> bool:
+    return any(
+        isinstance(item, FunctionResultContent) and item.id == call_id for message in history for item in message.items
+    )
+
+
+async def test_truncation_keeps_nonadjacent_function_call_with_its_result():
+    history = [
+        ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
+        _assistant_call("call-1"),
+        _tool_result("call-1"),
+        ChatMessageContent(role=AuthorRole.USER, content="User message 2"),
+        ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant text between tool rounds."),
+        _assistant_call("call-2"),
+        ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant text after tool call."),
+        _tool_result("call-2"),
+    ]
+
+    assert history[1].role == AuthorRole.ASSISTANT
+    assert history[2].role == AuthorRole.TOOL
+    assert history[5].role == AuthorRole.ASSISTANT
+    assert history[7].role == AuthorRole.TOOL
+
+    reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=0)
+    reducer.messages = history
+    result = await reducer.reduce()
+
+    assert result is not None
+    assert _has_result(result.messages, "call-2")
+    assert _has_call(result.messages, "call-2"), "A function result was retained without its matching function call."
+
+
+async def test_truncation_keeps_adjacent_function_call_result_pair():
+    history = [
+        ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
+        _assistant_call("call-1"),
+        _tool_result("call-1"),
+        ChatMessageContent(role=AuthorRole.USER, content="User message 2"),
+        _assistant_call("call-2"),
+        _tool_result("call-2"),
+    ]
+
+    reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=0)
+    reducer.messages = history
+    result = await reducer.reduce()
+
+    assert result is not None
+    assert _has_call(result.messages, "call-2")
+    assert _has_result(result.messages, "call-2")
+
+
+async def test_truncation_keeps_multiple_calls_in_one_assistant_message():
+    history = [
+        ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
+        _assistant_call("call-1"),
+        _tool_result("call-1"),
+        ChatMessageContent(role=AuthorRole.USER, content="User message 2"),
+        ChatMessageContent(role=AuthorRole.ASSISTANT, content=""),
+        ChatMessageContent(role=AuthorRole.TOOL, content=""),
+    ]
+    assert history[4].role == AuthorRole.ASSISTANT
+    assert history[5].role == AuthorRole.TOOL
+    history[4].items.extend([
+        FunctionCallContent(id="call-2", function_name="search", plugin_name="plugin", arguments={"q": "2"}),
+        FunctionCallContent(id="call-3", function_name="lookup", plugin_name="plugin", arguments={"q": "3"}),
+    ])
+    history[5].items.extend([
+        FunctionResultContent(id="call-2", function_name="search", plugin_name="plugin", result="result-2"),
+        FunctionResultContent(id="call-3", function_name="lookup", plugin_name="plugin", result="result-3"),
+    ])
+
+    reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=0)
+    reducer.messages = history
+    result = await reducer.reduce()
+
+    assert result is not None
+    assert _has_call(result.messages, "call-2")
+    assert _has_call(result.messages, "call-3")
+    assert _has_result(result.messages, "call-2")
+    assert _has_result(result.messages, "call-3")
+
+
+async def test_truncation_does_not_reduce_when_threshold_window_does_not_trigger():
+    history = [
+        _assistant_call("call-a"),
+        ChatMessageContent(role=AuthorRole.USER, content="User message"),
+        _tool_result("call-a"),
+    ]
+
+    reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=2)
+    reducer.messages = history
+    result = await reducer.reduce()
+
+    assert result is None
+
+
+async def test_truncation_keeps_pair_when_threshold_window_selects_user_boundary():
+    history = [
+        ChatMessageContent(role=AuthorRole.USER, content="User message 0"),
+        _assistant_call("call-a"),
+        _tool_result("call-a"),
+        _assistant_call("call-b"),
+        ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
+        _tool_result("call-b"),
+    ]
+
+    reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=1)
+    reducer.messages = history
+    result = await reducer.reduce()
+
+    assert result is not None
+    assert _has_result(result.messages, "call-b")
+    assert _has_call(result.messages, "call-b")
+
+
+async def test_truncation_closure_keeps_chained_nonadjacent_pairs():
+    history = [
+        ChatMessageContent(role=AuthorRole.USER, content="User message 0"),
+        _assistant_call("call-a"),
+        ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant between calls."),
+        _assistant_call("call-b"),
+        ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
+        _tool_result("call-b"),
+        _tool_result("call-a"),
+    ]
+
+    reducer = ChatHistoryTruncationReducer(target_count=2, threshold_count=1)
+    reducer.messages = history
+    result = await reducer.reduce()
+
+    assert result is not None
+    assert _has_call(result.messages, "call-a")
+    assert _has_result(result.messages, "call-a")
+    assert _has_call(result.messages, "call-b")
+    assert _has_result(result.messages, "call-b")
+
+
+def test_locate_safe_reduction_index_rejects_pair_crossing_offset_boundary():
+    history = [
+        ChatMessageContent(role=AuthorRole.USER, content="User message 0"),
+        _assistant_call("call-a"),
+        ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
+        _tool_result("call-a"),
+    ]
+
+    assert (
+        locate_safe_reduction_index(
+            history=history,
+            target_count=1,
+            threshold_count=0,
+            offset_count=2,
+            has_system_message=False,
+        )
+        is None
+    )
+
+
+async def test_truncation_preserves_system_message_while_pairs_are_safe():
+    reducer = ChatHistoryTruncationReducer(target_count=3, system_message="System prompt")
+    reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="User message 1"))
+    reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant 1"))
+    reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="User message 2"))
+    reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant 2"))
+
+    result = await reducer.reduce()
+
+    assert result is not None
+    assert result.messages[0].role == AuthorRole.SYSTEM
+    assert result.messages[0].content == "System prompt"

python/tests/unit/contents/test_chat_history_truncation_reversed_results.py

@@ -0,0 +1,60 @@
+# Copyright (c) Microsoft. All rights reserved.
+
+from semantic_kernel.contents.chat_message_content import ChatMessageContent
+from semantic_kernel.contents.function_call_content import FunctionCallContent
+from semantic_kernel.contents.function_result_content import FunctionResultContent
+from semantic_kernel.contents.history_reducer.chat_history_truncation_reducer import ChatHistoryTruncationReducer
+from semantic_kernel.contents.utils.author_role import AuthorRole
+
+
+def _assistant_call(call_id: str) -> ChatMessageContent:
+    m = ChatMessageContent(role=AuthorRole.ASSISTANT, content="")
+    m.items.append(FunctionCallContent(id=call_id, function_name="f", plugin_name="p", arguments={}))
+    return m
+
+
+def _tool_message_with(results: list[str]) -> ChatMessageContent:
+    m = ChatMessageContent(role=AuthorRole.TOOL, content="")
+    for rid in results:
+        m.items.append(FunctionResultContent(id=rid, function_name="f", plugin_name="p", result=f"r-{rid}"))
+    return m
+
+
+def _has_call(history: list[ChatMessageContent], call_id: str) -> bool:
+    return any(
+        isinstance(item, FunctionCallContent) and item.id == call_id for message in history for item in message.items
+    )
+
+
+def _has_result(history: list[ChatMessageContent], call_id: str) -> bool:
+    return any(
+        isinstance(item, FunctionResultContent) and item.id == call_id for message in history for item in message.items
+    )
+
+
+async def test_truncation_does_not_orphan_earlier_call_when_results_reversed_in_one_message():
+    # c2 call is at msg1 (earlier). c3 call is at msg3 (later).
+    # Both results are in ONE tool message (msg4), but ordered [result_c3, result_c2]
+    # (later call first). The break bug skipped result_c2 in pair matching, leaving
+    # c2's result orphaned when the cut removes msg1 but keeps msg4.
+    history = [
+        ChatMessageContent(role=AuthorRole.USER, content="u0"),
+        _assistant_call("c2"),
+        ChatMessageContent(role=AuthorRole.USER, content="u1"),
+        _assistant_call("c3"),
+        _tool_message_with(["c3", "c2"]),  # reversed: later call result first
+        ChatMessageContent(role=AuthorRole.ASSISTANT, content="text"),
+    ]
+
+    reducer = ChatHistoryTruncationReducer(target_count=3, threshold_count=0)
+    reducer.messages = history
+    result = await reducer.reduce()
+
+    assert result is not None
+    # If c3's result is kept, c3's call must be kept.
+    if _has_result(result.messages, "c3"):
+        assert _has_call(result.messages, "c3"), "c3 result kept without c3 call"
+    # Critical: if c2's result is kept (it is in the same msg as c3's result),
+    # c2's call must also be kept. Without the fix, c2 is orphaned.
+    if _has_result(result.messages, "c2"):
+        assert _has_call(result.messages, "c2"), "c2 result kept without c2 call (orphan)"