Skip to content

Commit cd25e3a

Browse files
committed
Python: Fix ChatHistoryTruncationReducer deleting system prompt (#12612)
The truncation reducer used `extract_range()` which unconditionally filters out system/developer messages — a function designed for the summarization use case, not truncation. This caused system prompts to be silently deleted when chat history was reduced. Fix: detect the first system/developer message before truncation, pass `has_system_message=True` to `locate_safe_reduction_index` so target_count accounts for the preserved message, use a simple slice instead of `extract_range`, and prepend the system message if it was truncated away. This matches the .NET SDK behavior (PR #10344, already merged). Also adds a guard for `target_count <= 0` after the system message adjustment to prevent IndexError when target_count=1. Note: The summarization reducer (`ChatHistorySummarizationReducer`) has the same bug — it also uses `extract_range` and loses system messages. That should be addressed in a follow-up PR. Closes #12612
1 parent b712ffc commit cd25e3a

3 files changed

Lines changed: 121 additions & 14 deletions

File tree

python/semantic_kernel/contents/history_reducer/chat_history_reducer_utils.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ def locate_safe_reduction_index(
6666
target_count: int,
6767
threshold_count: int = 0,
6868
offset_count: int = 0,
69+
has_system_message: bool = False,
6970
) -> int | None:
7071
"""Identify the index of the first message at or beyond the specified target_count.
7172
@@ -83,11 +84,21 @@ def locate_safe_reduction_index(
8384
threshold_count: The threshold beyond target_count required to trigger reduction.
8485
If total messages <= (target_count + threshold_count), no reduction occurs.
8586
offset_count: Optional number of messages to skip at the start (e.g. existing summary messages).
87+
has_system_message: Whether the history contains a system message that will be preserved
88+
separately. When True, the target_count is adjusted to account for the
89+
system message being re-added after reduction.
8690
8791
Returns:
8892
The index that identifies the starting point for a reduced history that does not orphan
8993
sensitive content. Returns None if reduction is not needed.
9094
"""
95+
# Adjust target_count to account for the system message that will be preserved separately.
96+
# This matches the .NET SDK behavior.
97+
if has_system_message:
98+
target_count -= 1
99+
if target_count <= 0:
100+
return None # Cannot reduce further; only system message would remain
101+
91102
total_count = len(history)
92103
threshold_index = total_count - (threshold_count or 0) - target_count
93104
if threshold_index <= offset_count:

python/semantic_kernel/contents/history_reducer/chat_history_truncation_reducer.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515

1616
from semantic_kernel.contents.history_reducer.chat_history_reducer import ChatHistoryReducer
1717
from semantic_kernel.contents.history_reducer.chat_history_reducer_utils import (
18-
extract_range,
1918
locate_safe_reduction_index,
2019
)
20+
from semantic_kernel.contents.utils.author_role import AuthorRole
2121
from semantic_kernel.utils.feature_stage_decorator import experimental
2222

2323
logger = logging.getLogger(__name__)
@@ -45,15 +45,32 @@ async def reduce(self) -> Self | None:
4545

4646
logger.info("Performing chat history truncation check...")
4747

48-
truncation_index = locate_safe_reduction_index(history, self.target_count, self.threshold_count)
48+
# Preserve system/developer messages so they are not lost during truncation.
49+
# This matches the .NET SDK behavior where system messages are always retained.
50+
system_message = next(
51+
(msg for msg in history if msg.role in (AuthorRole.SYSTEM, AuthorRole.DEVELOPER)),
52+
None,
53+
)
54+
55+
truncation_index = locate_safe_reduction_index(
56+
history,
57+
self.target_count,
58+
self.threshold_count,
59+
has_system_message=system_message is not None,
60+
)
4961
if truncation_index is None:
5062
logger.info(
5163
f"No truncation index found. Target count: {self.target_count}, Threshold: {self.threshold_count}"
5264
)
5365
return None
5466

5567
logger.info(f"Truncating history to {truncation_index} messages.")
56-
truncated_list = extract_range(history, start=truncation_index)
68+
truncated_list = history[truncation_index:]
69+
70+
# Prepend the system/developer message if it was truncated away
71+
if system_message is not None and system_message not in truncated_list:
72+
truncated_list = [system_message, *truncated_list]
73+
5774
self.messages = truncated_list
5875
return self
5976

python/tests/unit/contents/test_chat_history_truncation_reducer.py

Lines changed: 90 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,13 @@ async def test_truncation_reducer_truncation(chat_messages):
8383
reducer = ChatHistoryTruncationReducer(target_count=2)
8484
reducer.messages = chat_messages
8585
result = await reducer.reduce()
86-
# We expect only 2 messages remain after truncation
86+
# We expect 2 messages: system message is preserved + 1 conversation message
87+
# (target_count=2 includes the system message, matching .NET SDK behavior)
8788
assert result is not None
8889
assert len(result) == 2
89-
# They should be the last 4 messages while tool result is not orphaned
90-
assert result[0] == chat_messages[-2]
90+
# System message should be first, followed by the last conversation message
91+
assert result[0] == chat_messages[0] # System message preserved
92+
assert result[0].role == AuthorRole.SYSTEM
9193
assert result[1] == chat_messages[-1]
9294

9395

@@ -96,12 +98,89 @@ async def test_truncation_reducer_truncation_with_tools(chat_messages_with_tools
9698
reducer = ChatHistoryTruncationReducer(target_count=3, threshold_count=0)
9799
reducer.messages = chat_messages_with_tools
98100
result = await reducer.reduce()
99-
# We expect 4 messages remain after truncation
100-
# Tool results are not orphaned, so we expect to keep them
101+
# We expect 3 messages: system message + last 2 conversation messages
102+
# (target_count=3 includes the system message, matching .NET SDK behavior)
101103
assert result is not None
102-
assert len(result) == 4
103-
# They should be the last 4 messages
104-
assert result[0] == chat_messages_with_tools[-4]
105-
assert result[1] == chat_messages_with_tools[-3]
106-
assert result[2] == chat_messages_with_tools[-2]
107-
assert result[3] == chat_messages_with_tools[-1]
104+
assert len(result) == 3
105+
# System message preserved, followed by last user-assistant pair
106+
assert result[0] == chat_messages_with_tools[0] # System message
107+
assert result[0].role == AuthorRole.SYSTEM
108+
assert result[1] == chat_messages_with_tools[-2] # User message 2
109+
assert result[2] == chat_messages_with_tools[-1] # Assistant message 2
110+
111+
112+
async def test_truncation_preserves_system_message():
113+
"""Verify that the system message is preserved after truncation (issue #12612)."""
114+
reducer = ChatHistoryTruncationReducer(
115+
target_count=2,
116+
system_message="a system message",
117+
)
118+
reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="user prompt 1"))
119+
reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="response 1"))
120+
reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="user prompt 2"))
121+
reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="response 2"))
122+
reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="user prompt 3"))
123+
reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="response 3"))
124+
reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="user prompt 4"))
125+
reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="response 4"))
126+
127+
result = await reducer.reduce()
128+
assert result is not None
129+
130+
# System message must be present after reduction
131+
roles = [msg.role for msg in result.messages]
132+
assert AuthorRole.SYSTEM in roles, "System message was deleted by the history reducer"
133+
assert result.messages[0].role == AuthorRole.SYSTEM
134+
assert result.messages[0].content == "a system message"
135+
136+
137+
async def test_truncation_preserves_developer_message():
138+
"""Verify that developer messages are preserved after truncation."""
139+
msgs = [
140+
ChatMessageContent(role=AuthorRole.DEVELOPER, content="Developer instructions."),
141+
ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
142+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant message 1"),
143+
ChatMessageContent(role=AuthorRole.USER, content="User message 2"),
144+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant message 2"),
145+
]
146+
reducer = ChatHistoryTruncationReducer(target_count=2)
147+
reducer.messages = msgs
148+
result = await reducer.reduce()
149+
assert result is not None
150+
assert len(result) == 2
151+
# Developer message should be first
152+
assert result[0].role == AuthorRole.DEVELOPER
153+
assert result[0].content == "Developer instructions."
154+
155+
156+
async def test_truncation_target_count_1_with_system_message():
157+
"""Verify target_count=1 with system message does not crash (edge case from review)."""
158+
reducer = ChatHistoryTruncationReducer(target_count=1, system_message="System prompt")
159+
reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="Hello"))
160+
reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Hi"))
161+
reducer.add_message(ChatMessageContent(role=AuthorRole.USER, content="How are you?"))
162+
reducer.add_message(ChatMessageContent(role=AuthorRole.ASSISTANT, content="Good"))
163+
164+
# Should not crash. Either returns None (no reduction possible)
165+
# or returns just the system message.
166+
result = await reducer.reduce()
167+
if result is not None:
168+
# System message must be preserved
169+
assert any(m.role == AuthorRole.SYSTEM for m in result.messages)
170+
171+
172+
async def test_truncation_without_system_message():
173+
"""Verify truncation works correctly when there is no system message."""
174+
msgs = [
175+
ChatMessageContent(role=AuthorRole.USER, content="User message 1"),
176+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant message 1"),
177+
ChatMessageContent(role=AuthorRole.USER, content="User message 2"),
178+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant message 2"),
179+
]
180+
reducer = ChatHistoryTruncationReducer(target_count=2)
181+
reducer.messages = msgs
182+
result = await reducer.reduce()
183+
assert result is not None
184+
assert len(result) == 2
185+
assert result[0] == msgs[-2]
186+
assert result[1] == msgs[-1]

0 commit comments

Comments
 (0)