Repository navigation
.Net: Fix chat history reducers crashing when target count is one and a system message is present - #14655
Conversation
… a system message is present
|
John Karam (@johnkaram02) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
1 similar comment
|
John Karam (@johnkaram02) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
🟢 Approval recommended
The localized fix addresses the invalid index, with focused regression coverage for both reducers and no unresolved findings.
No open issues 🎉
What changed in this PR
Fixes #14654 by preventing chat history reducers from indexing past the history when targetCount: 1 and a system message is present.
Changes:
- Keeps at least one non-system message while preserving behavior for other valid target counts.
- Adds regression tests confirming truncation retains the system and latest messages, and summarization also retains a summary.
| File | Description |
|---|---|
dotnet/src/SemanticKernel.UnitTests/AI/ChatCompletion/ChatHistoryTruncationReducerTests.cs |
Tests truncation with a system message and target count of one. |
dotnet/src/SemanticKernel.UnitTests/AI/ChatCompletion/ChatHistorySummarizationReducerTests.cs |
Tests summarization with a system message and target count of one. |
dotnet/src/SemanticKernel.Core/AI/ChatCompletion/ChatHistoryReducerExtensions.cs |
Clamps the adjusted target count to at least one. |
Review effort: Balanced
Motivation and Context
Fixes #14654.
ChatHistoryTruncationReducerandChatHistorySummarizationReducerboth accepttargetCount: 1. When the history contains a system message,ReduceAsyncthrowsArgumentOutOfRangeException.LocateSafeReductionIndexsubtracts the system message from the target count, which leaves0, and then readschatHistory[chatHistory.Count]. So a reducer configured this way fails as soon as the history exceeds the target.Description
ChatHistoryReducerExtensions.LocateSafeReductionIndexstill counts the system message toward the target, but now always keeps at least one non-system message. WithtargetCount: 1, truncation keeps the system message and the latest message, and summarization keeps the system message, the summary, and the latest message.targetCountvalues greater than 1, and for histories without a system message.targetCount: 1with a system message. Both tests fail onmainwithArgumentOutOfRangeExceptionand pass with this change.Contribution Checklist