Skip to content

fix: coerce chat summary to str for RunCompletionEvent#2799

Open
genisis0x wants to merge 1 commit into
ag2ai:mainfrom
genisis0x:fix/1811-summary-coercion
Open

fix: coerce chat summary to str for RunCompletionEvent#2799
genisis0x wants to merge 1 commit into
ag2ai:mainfrom
genisis0x:fix/1811-summary-coercion

Conversation

@genisis0x
Copy link
Copy Markdown
Contributor

Summary

Closes #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}

The single-level if isinstance(summary, dict): summary = str(summary.get("content", "")) guard in _summarize_chat only unwraps one level. When the inner "content" is itself a dict (which is what the reporter in #1811 hit with Ollama llama3.1:8b and again with Cohere command-r), the dict propagates straight to RunCompletionEvent, and pydantic rejects it:

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]

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_str helper:

  • recursively unwraps a "content" key from each nested dict,
  • treats None as "",
  • stringifies any other non-str value (e.g. a callable returning an int).

Route both _summarize_chat (the gate every ChatResult.summary passes through) and the new dict branch of _last_msg_as_summary through it.

Behaviour for the existing single-level dict and plain-string return values is unchanged, so the existing test_summarize_chat_with_dict_summary still 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, plain None, and a non-string scalar
  • test_last_msg_summary_unwraps_dict_content_last_msg_as_summary follows dict-shaped content and still strips TERMINATE on that path
$ AUTOGEN_USE_DOCKER=0 pytest test/agentchat/test_conversable_agent.py \
    -k "summary or summarize or last_msg" -q
... 7 passed, 1 skipped

(AUTOGEN_USE_DOCKER=0 is only needed locally because UserProxyAgent in the existing test is constructed with the default code_execution_config and tries to run docker; CI sets this differently. The new tests pass code_execution_config=False directly.)

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
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 11, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Collaborator

@marklysze marklysze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Input should be a valid string

3 participants