Fix: drop trailing assistant message when rstrip leaves it empty - #8029
Mayuri (mayuriphad) wants to merge 2 commits into
Conversation
_rstrip_last_assistant_message() is documented as removing the last assistant message when it is empty, but it only called .rstrip() on the content and left the (now possibly empty) message in place. When the last AssistantMessage content was whitespace-only, this produced an empty-string content block, which the Anthropic API rejects (text content blocks must be non-empty). Now, after stripping, if the content becomes an empty string the message is dropped from the returned list, matching the function's documented behavior. Non-empty trailing assistant messages are still only whitespace-stripped, preserving existing prefill behavior and the pre-existing passing tests. Same fix applied to both the Anthropic and OpenAI clients, which share an identical copy of this helper. Fixes #7768
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in autogen-ext where _rstrip_last_assistant_message() could leave a trailing AssistantMessage in the history with content == "" after .rstrip(). That empty assistant content can be rejected by Claude/Anthropic message validation, so the helper now removes the assistant message entirely when stripping results in an empty string.
Changes:
- Update
_rstrip_last_assistant_message()in both the Anthropic and OpenAI clients to (a) handle emptymessagessafely and (b) drop the trailing assistant message ifrstrip()produces"". - Add regression tests covering whitespace-only trailing assistant messages for both clients.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| python/packages/autogen-ext/src/autogen_ext/models/openai/_openai_client.py | Drops trailing assistant message when rstrip() leaves empty content (and avoids messages[-1] on empty input). |
| python/packages/autogen-ext/src/autogen_ext/models/anthropic/_anthropic_client.py | Same helper fix applied to Anthropic client implementation. |
| python/packages/autogen-ext/tests/models/test_openai_model_client.py | Adds regression test ensuring whitespace-only trailing assistant message is removed. |
| python/packages/autogen-ext/tests/models/test_anthropic_model_client.py | Adds regression test ensuring whitespace-only trailing assistant message is removed. |
Anuj Bolewar (anujbolewar)
left a comment
Anuj Bolewar (anujbolewar)
left a comment
There was a problem hiding this comment.
Correct — dropping a last assistant message that becomes empty after rstrip avoids the API rejection of empty text blocks, and guarding on a non-empty list fixes the prior index error. Two small notes: mutating messages then removing it changes the list identity returned, which is fine, but the same logic is now duplicated across the Anthropic and OpenAI clients — worth extracting the shared helper so the behavior cannot diverge; and confirm stripping affects only the trailing message, never an interleaved one.
|
@microsoft-github-policy-service agree |
Addresses review feedback on #8029: the empty-trailing-assistant-message handling was duplicated identically across the Anthropic and OpenAI clients, risking divergence over time (the same codebase already has one such duplicated helper, normalize_stop_reason, that diverged when only one client was updated to import the shared version). Moved the logic to autogen_ext.models._utils.rstrip_last_assistant_message as a plain function, matching the existing _utils pattern used by normalize_stop_reason and parse_r1_content, and updated both clients and their tests to use it. Also added a regression test confirming the strip only affects the trailing message, not interleaved assistant messages earlier in the sequence.
19afb96 to
0125134
Compare
|
Addressed: extracted the shared |
|
I have recreated this PR here: #8250 |
Fixes #7768
Root cause
_rstrip_last_assistant_message()is documented as removing the last assistant message when it is empty, but it only called.rstrip()on the content and left the (now possibly empty) message in the list. When the trailingAssistantMessage.contentwas whitespace-only, this produced an empty-string content block, which the Anthropic API rejects (text content blocks must be non-empty).Fix
After stripping, if the content becomes an empty string, drop the message entirely — matching the function's documented behavior. Non-empty trailing assistant messages are still only whitespace-stripped, preserving the existing "prefill" behavior and pre-existing passing tests. Same fix applied to both the Anthropic and OpenAI clients, which share an identical copy of this helper.
Verification
Added regression tests in both
test_anthropic_model_client.pyandtest_openai_model_client.py. Ran the full local suite: all pre-existing and new tests pass (unrelated failures due to missingOPENAI_API_KEYin this environment, confirmed unrelated via traceback inspection).