fix: coerce chat summary to str for RunCompletionEvent#2799
Open
genisis0x wants to merge 1 commit into
Open
Conversation
Closes ag2ai#1811. RunCompletionEvent.summary is annotated str, but the path that fills it accepted whatever the configured summary_method (or the LLM reflection response) returned. Some providers — Ollama, Cohere, and any model emitting a structured assistant message — surface the completion as a nested dict like {"content": "...", "tool_calls": None}, which then fell through the single-level dict guard in _summarize_chat unchanged when the inner "content" was itself a dict, and pydantic rejected the assembled event with: ValidationError: 1 validation error for RunCompletionEvent summary Input should be a valid string [type=string_type, input_value={'content': '...', 'tool_calls': None}, input_type=dict] The reproducer in the bug report (the run_and_event_processing two-agent comedians notebook with the LLM swapped to Ollama llama3.1:8b, repeated with Cohere command-r) hits this on the very first turn, so the chat is unusable. Add a small _coerce_summary_to_str helper that recursively unwraps a "content" key from each nested dict, treats None as "", and stringifies anything else, then route both _summarize_chat (the gate every ChatResult.summary passes through) and the dict branch of _last_msg_as_summary through it. The behaviour for the existing single-level dict and plain-string returns is unchanged, so the existing test_summarize_chat_with_dict_summary still passes. New regression coverage in test_conversable_agent.py: - parametrised test for nested dict, None, and scalar summary returns - _last_msg_as_summary follows dict-shaped content and still strips TERMINATE on that path
marklysze
approved these changes
May 17, 2026
Collaborator
marklysze
left a comment
There was a problem hiding this comment.
Clean fix. The existing single-level isinstance(value, dict) check was insufficient for nested dict responses from Ollama/Cohere (e.g. {"content": {"content": "..."}}). The recursive _coerce_summary_to_str helper handles the nesting correctly while still falling back to str() for unexpected types. Tests cover the parametrized return-type matrix and the _last_msg_as_summary dict-content unwrap. Approved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #1811.
RunCompletionEvent.summaryis annotatedstr, but the path that fills it accepted whatever the configuredsummary_method(or the LLM reflection response) returned. Some providers — Ollama, Cohere, and any model emitting a structured assistant message — surface the completion as a nested dict like:{"content": "...", "tool_calls": None}The single-level
if isinstance(summary, dict): summary = str(summary.get("content", ""))guard in_summarize_chatonly unwraps one level. When the inner"content"is itself a dict (which is what the reporter in #1811 hit with Ollamallama3.1:8band again with Coherecommand-r), the dict propagates straight toRunCompletionEvent, and pydantic rejects it:Following the run_and_event_processing two-agent comedians notebook with the LLM swapped to Ollama makes the chat unusable from the first turn.
Change
Add a small
_coerce_summary_to_strhelper:"content"key from each nested dict,Noneas"",strvalue (e.g. a callable returning anint).Route both
_summarize_chat(the gate everyChatResult.summarypasses through) and the new dict branch of_last_msg_as_summarythrough it.Behaviour for the existing single-level dict and plain-string return values is unchanged, so the existing
test_summarize_chat_with_dict_summarystill passes.Test plan
New regression coverage in
test/agentchat/test_conversable_agent.py:test_summarize_chat_coerces_arbitrary_returns_to_str— parametrised across single-level dict, nested-dict-content, dict-with-None-content, plainNone, and a non-string scalartest_last_msg_summary_unwraps_dict_content—_last_msg_as_summaryfollows dict-shaped content and still stripsTERMINATEon that path(
AUTOGEN_USE_DOCKER=0is only needed locally becauseUserProxyAgentin the existing test is constructed with the defaultcode_execution_configand tries to run docker; CI sets this differently. The new tests passcode_execution_config=Falsedirectly.)