Skip to content

Commit ecb80bd

Browse files
alphasafalalphasafal
authored
Python: keep existing summaries after the system message in the summa… (#14661)
### Motivation and Context `ChatHistorySummarizationReducer(use_single_summary=False)` is meant to keep earlier summaries and add a new one on each reduction. That doesn't happen when the chat history starts with a system prompt, which is the common case. After the first reduction the reducer puts the preserved system message at index 0: `[system, summary1, ...]`. On the next reduction `locate_summarization_boundary(history)` starts at index 0, sees the system message (not a summary) and returns 0. So `keep_existing_summaries` stays empty, `summary1` falls into the range that gets summarized, and the result is `[system, summary2(summary1 + messages), ...]`, exactly what `use_single_summary=True` produces. ### Description When `use_single_summary=False` and the preserved system message is at index 0, locate the summary boundary after it: `insertion_point = 1 + locate_summarization_boundary(history[1:])`. `keep_existing_summaries` then contains `[system, summary1]`, the old summary is no longer sent to the summarization call, and the existing identity check still prevents the system message from being prepended twice. `use_single_summary=True` (the default) is unchanged, so its `offset_count` passed to `locate_safe_reduction_index` doesn't move. Test: `test_summarization_keeps_existing_summaries_after_system_message` builds `[system, earlier summary, 6 messages]` with `target_count=2, use_single_summary=False` and checks that the result starts with the same system message and earlier summary objects followed by the new summary, and that the earlier summary is not in the history sent to `get_chat_message_content`. It fails on `main` and passes with this change. ``` pytest tests/unit/contents 395 passed ``` ### Contribution Checklist - [x] The code builds clean without any errors or warnings - [x] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations (ruff 0.9.6 check and format clean) - [x] All unit tests pass, and I have added new tests where possible (I ran `tests/unit/contents`, not the full suite) - [x] I didn't break anyone 😄
1 parent df34878 commit ecb80bd

2 files changed

Lines changed: 39 additions & 0 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ async def reduce(self) -> Self | None:
107107

108108
# 1. Identify where existing summary messages end
109109
insertion_point = locate_summarization_boundary(history)
110+
if not self.use_single_summary and system_message_index == 0:
111+
# Earlier reductions keep the system message in front of their summaries, so look past it
112+
insertion_point = 1 + locate_summarization_boundary(history[1:])
110113
if insertion_point == len(history):
111114
# fallback fix: force boundary to something reasonable
112115
logger.warning("All messages are summaries, forcing boundary to 0.")

‎python/tests/unit/contents/test_chat_history_summarization_reducer.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,3 +312,39 @@ async def test_summarization_preserves_developer_message(mock_service):
312312
# Developer message must be preserved
313313
assert result.messages[0].role == AuthorRole.DEVELOPER
314314
assert result.messages[0].content == "Developer instructions"
315+
316+
317+
async def test_summarization_keeps_existing_summaries_after_system_message(mock_service):
318+
"""With use_single_summary=False, summaries after a preserved system prompt are kept, not re-summarized."""
319+
system_message = ChatMessageContent(role=AuthorRole.SYSTEM, content="Important system prompt")
320+
existing_summary = ChatMessageContent(
321+
role=AuthorRole.ASSISTANT, content="Earlier summary.", metadata={SUMMARY_METADATA_KEY: True}
322+
)
323+
messages = [
324+
system_message,
325+
existing_summary,
326+
ChatMessageContent(role=AuthorRole.USER, content="User says hello"),
327+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant responds"),
328+
ChatMessageContent(role=AuthorRole.USER, content="User says more"),
329+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant responds again"),
330+
ChatMessageContent(role=AuthorRole.USER, content="User says even more"),
331+
ChatMessageContent(role=AuthorRole.ASSISTANT, content="Assistant responds yet again"),
332+
]
333+
334+
reducer = ChatHistorySummarizationReducer(
335+
service=mock_service, target_count=2, threshold_count=0, use_single_summary=False
336+
)
337+
reducer.messages = messages
338+
339+
new_summary = ChatMessageContent(role=AuthorRole.ASSISTANT, content="New summary.")
340+
mock_service.get_chat_message_content.return_value = new_summary
341+
mock_service.get_prompt_execution_settings_from_settings.return_value = PromptExecutionSettings()
342+
343+
result = await reducer.reduce()
344+
345+
assert result is not None
346+
assert result.messages[0] is system_message
347+
assert result.messages[1] is existing_summary
348+
assert result.messages[2] is new_summary
349+
summarized_history = mock_service.get_chat_message_content.call_args.kwargs["chat_history"]
350+
assert all(msg.content != "Earlier summary." for msg in summarized_history.messages)

0 commit comments

Comments
 (0)