Repository navigation
Python: fix(python): wire ChatHistoryReducer into ChatCompletionAgent invocation (#12303) - #14607
Conversation
…ion (microsoft#12303) - Propagate initial chat_history reducer from KernelArguments into ChatHistoryAgentThread - Await thread.reduce() prior to dispatching messages to ChatCompletionClientBase - Trigger auto_reduce via add_message_async in ChatHistoryAgentThread._on_new_message Signed-off-by: sparkysolutions10-pixelmy <sparkysolutions10@gmail.com>
…Reducer (microsoft#12303) - Test ChatCompletionAgent respecting ChatHistoryTruncationReducer during get_response - Test ChatCompletionAgent automatic reduction via KernelArguments Signed-off-by: sparkysolutions10-pixelmy <sparkysolutions10@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
Constructor-configured reducers remain ignored, and incomplete service mocks prevent the new tests from reaching their assertions.
3 open findings
What changed in this PR
This PR connects chat-history reducers to Python ChatCompletionAgent invocations to limit conversation payload size.
Changes:
- Enables automatic reduction when threads receive messages.
- Propagates invocation-provided history and reduces it before model calls.
- Adds tests for thread- and argument-injected reducers.
| File | Description |
|---|---|
| python/tests/unit/agents/chat_completion/test_chat_completion_agent.py | Adds reducer integration tests. |
| python/semantic_kernel/agents/chat_completion/chat_completion_agent.py | Integrates reducers into thread updates and invocation paths. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
|
|
||
| async def test_chat_completion_agent_with_chat_history_reducer(): | ||
| mock_service = AsyncMock(spec=ChatCompletionClientBase) |
| initial_history = ( | ||
| arguments.get("chat_history") | ||
| if arguments and isinstance(arguments.get("chat_history"), ChatHistory) | ||
| else None | ||
| ) |
| assert response.message.content == "Agent answer" | ||
|
|
||
| passed_history = mock_service.get_chat_message_contents.call_args.kwargs["chat_history"] | ||
| assert len([m for m in passed_history.messages if m.role != AuthorRole.SYSTEM]) <= 2 |
|
@microsoft-github-policy-service agree |
PRABHU KIRAN VANDRANKI (VANDRANKI)
left a comment
There was a problem hiding this comment.
Community review. This does not clear the merge gate.
The behavior change works, but the two new tests do not run, and one behavior choice needs a decision.
-
The new tests fail on the PR branch itself. I applied the PR to upstream/main (7a92cd4, separate worktree, venv at
C:/skv2) and ranpython/tests/unit/agents/chat_completionplustests/unit/agents/test_agent.py. The two added tests,test_chat_completion_agent_with_chat_history_reducerandtest_chat_completion_agent_auto_reduce_via_arguments, fail the same way on main and on the PR: 2 failed, 62 passed in both runs. The error isAttributeError: Mock object has no attribute 'service_id', raised fromChatCompletionAgent.__init__->configure_service->kernel_services_extension.add_service, becauseAsyncMock(spec=ChatCompletionClientBase)is passed asservice=. The other tests in this file use thekernel_with_ai_servicefixture fromconftest.pyinstead. So these tests never reach the code they are meant to cover. -
The change itself does what the description says. I wrote my own probe using the same
create_autospeckernel pattern as thechat_completionconftest, with aChatHistoryTruncationReducer(target_count=2, threshold_count=0)and 5 or 6 user messages, and compared what reachesget_chat_message_contents:
- reducer in a
ChatHistoryAgentThread,auto_reduce=True,get_response: main sends M0..M4, PR sends M3, M4. - the same thread with
auto_reduce=False: main sends M0..M4, PR sends M3, M4. - reducer passed as
KernelArguments(chat_history=reducer)with no thread: main sends an empty history (the argument is ignored), PR sends the last two messages. invokewith the auto-reduce thread: main sends M0..M4, PR sends M3, M4.
I did not runinvoke_stream; it has the same code added as the other two, which I read in the diff only.
-
Design question.
thread.reduce()is now called whenever the history is aChatHistoryReducer, including withauto_reduce=False(second bullet above). Ifauto_reduce=Falseis meant to mean "I will call reduce myself", this overrides it. The PR's own second test usesauto_reduce=Falseand expects reduction, so it looks deliberate, but the description does not say so. Worth a maintainer decision. -
Also,
initial_historyfromarguments["chat_history"]is only used when a new thread is created. If a thread is passed in along withchat_historyin the arguments, the argument is ignored; I did not test that case.
Issue #12303 is open and assigned to TaoChenOSU (read from the API), so you may want to confirm with them before this goes further.


Motivation and Context
Fixes #12303
Why is this change required?
Currently, configuring a
ChatHistorySummarizationReducerorChatHistoryTruncationReduceron aChatCompletionAgent(e.g. viaarguments=KernelArguments(chat_history=reducer)or thread injection) fails to reduce the chat history before dispatching calls to the LLM. The full, unbounded conversation history is passed in the request payload, causing context window overflows and unnecessary token consumption.What problem does it solve?
ChatCompletionAgentto respect activeChatHistoryReducerimplementations (such as summarization and truncation reducers).What scenario does it contribute to?
Multi-turn agent interactions and multi-agent group chats where long-running conversations require summarizing or truncating older messages to stay within LLM context boundaries.
Description
ChatHistoryAgentThread._on_new_message, check ifself._chat_historyis aChatHistoryReducerwithauto_reduce=True. If so, invokeawait self._chat_history.add_message_async(new_message)so the reduction boundary triggers as designed.ChatCompletionAgent.get_response(),invoke(), andinvoke_stream(), check ifargumentscontains an activechat_historyreducer and pass it into the constructor when initializingChatHistoryAgentThread.chat_historypayload for_inner_invoke()/_inner_invoke_stream(), check if the active thread contains aChatHistoryReducerand callawait thread.reduce().ChatCompletionAgentrespects both thread-injected and argument-injected reducers duringget_response().Contribution Checklist