Skip to content

[Bugfix] Handle empty or refusal content when reconstructing Responses input messages - #49130

Open
TimurRakhmatullin86 wants to merge 1 commit into
vllm-project:mainfrom
TimurRakhmatullin86:fix/responses-output-message-empty-content
Open

TimurRakhmatullin86 wants to merge 1 commit into
vllm-project:mainfrom
TimurRakhmatullin86:fix/responses-output-message-empty-content

Conversation

@TimurRakhmatullin86

Copy link
Copy Markdown

Purpose

The Responses API returns an unhandled HTTP 500 when a client replays a prior assistant
turn whose content is a refusal or is empty.

_construct_message_from_response_item (vllm/entrypoints/openai/responses/utils.py) reads
item.content[0].text unguarded for a ResponseOutputMessage:

elif isinstance(item, ResponseOutputMessage):
    output_text = item.content[0].text

Two ordinary inputs break this:

  • content is List[Content] with no minimum length, so an assistant message with
    content=[] raises IndexError.
  • A content block may be a ResponseOutputRefusal (fields refusal/type, no text),
    so replaying a refusal turn raises AttributeError.

The request-level input_item_parsing validator coerces assistant dicts with list content
into typed ResponseOutputMessages, so both reach this branch and are not caught by the
guarded dict-fallback path. Its sibling reasoning branch already guards (elif item.content and len(item.content) >= 1); this branch now does too.

elif isinstance(item, ResponseOutputMessage):
    output_text = ""
    if item.content:
        first = item.content[0]
        output_text = getattr(first, "text", None) or getattr(first, "refusal", "")

Empty content → ""; a refusal block → its refusal text; a normal text block → unchanged.

Test Plan

Added TestOutputMessageContent to
tests/entrypoints/openai/responses/test_responses_utils.py:

  • test_empty_content_yields_empty_string
  • test_refusal_content_uses_refusal_text
pytest tests/entrypoints/openai/responses/test_responses_utils.py -k OutputMessageContent

Test Result

Both new tests pass with the fix and fail against the current code (IndexError /
AttributeError respectively). Verified the exact old vs. new expressions against the real
openai (2.46.0) types: for a text block both yield the same string; for a refusal block the
old code raises AttributeError while the new returns the refusal text; for empty content the
old code raises IndexError while the new returns "". ruff check and ruff format --check
pass on both files. (test_responses_utils.py is a CPU-only module — no GPU required.)

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@TimurRakhmatullin86

Copy link
Copy Markdown
Author

Friendly ping — let me know if there's anything I should update here.

@mergify

mergify Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @TimurRakhmatullin86.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 15, 2026
@chudinovAI

chudinovAI commented Oct 17, 2026 •

Copy link
Copy Markdown

Thanks for this. One adjacent problem in the same function that this PR could fix with little extra work:
_construct_message_from_response_item reads only the first content part, so input items with
several parts are silently truncated (the request still returns 200):

  • ResponseOutputMessage with [output_text "Part one. ", output_text "Part two."] → assistant content "Part one. "
  • assistant easy-input dict with content=[{"type": "input_text", "text": "A. "}, {"type": "input_text", "text": "B."}] → "A. "
  • ResponseReasoningItem with two reasoning_text parts → only the first one

Joining the text parts instead of picking content[0] covers both the multi-part case and the
empty/refusal crash this PR targets, e.g.

output_text = "".join(
    part.text for part in item.content if part.type == "output_text"
)

(and "\n".join(part.text for part in item.content) for reasoning, matching
responses/harmony.py, which already joins all reasoning parts). Whether a refusal part should
contribute its refusal text is your call — I don't have a strong opinion. I left the summary
fallback alone since #36516 deliberately de-prioritized it.

Happy to send this as a follow-up instead if you'd rather keep this PR narrow.

@TimurRakhmatullin86
TimurRakhmatullin86 force-pushed the fix/responses-output-message-empty-content branch from a7b52a9 to bc1ab66 Compare October 21, 2026 04:29
@TimurRakhmatullin86

Copy link
Copy Markdown
Author

Rebased on the latest main (resolved a trivial add/add in the test file — the new TestBuildResponseOutputItemsStatus and this PR's TestOutputMessageContent are both kept). Thanks @chudinovAI for the pointer to the adjacent _construct_message_from_response_item case — happy to fold that in here if the slightly wider scope is OK, otherwise I'll leave this PR focused on the empty/refusal reconstruction.

@mergify mergify Bot removed the needs-rebase label Oct 21, 2026
@chudinovAI

Copy link
Copy Markdown

@TimurRakhmatullin86 Yes, please fold it in — joining all text parts instead of content[0] covers the multi-part case together with the empty/refusal one, so it fits this PR well. Thanks!

@TimurRakhmatullin86
TimurRakhmatullin86 force-pushed the fix/responses-output-message-empty-content branch from bc1ab66 to 3634c43 Compare October 27, 2026 02:27
@TimurRakhmatullin86

Copy link
Copy Markdown
Author

Folded it in, thanks @chudinovAI — it now joins all output_text parts (and all reasoning parts, matching responses/harmony.py) and falls back to a refusal's text, so empty, multi-part, and refusal messages all reconstruct. Added a multi-part test alongside the empty/refusal ones; pre-commit (ruff check + format) is clean now. I left the summary fallback and the easy-input dict path alone, per your note.

…s input messages

_construct_message_from_response_item unconditionally accessed
content[0].text for both ResponseOutputMessage and
ResponseReasoningItem, crashing with an IndexError when content was
empty and silently truncating multi-part content. The same content[0]
truncation affected the assistant easy-input dict path.

Fix by joining all text parts:
- ResponseOutputMessage: join output_text parts, fall back to refusal
  text for refusal-only messages, empty string for truly empty content.
- ResponseReasoningItem: join all reasoning parts with newlines,
  matching the convention in responses/harmony.py.
- Easy-input assistant dicts: join text from all content parts.

Add tests covering empty content, refusal-only, multi-part, and
easy-input dict multi-part cases.
@TimurRakhmatullin86

Copy link
Copy Markdown
Author

@chudinovAI thanks for the suggestion — folded it in. The rebased version now joins all text parts in all three paths:

  • ResponseOutputMessage: "".join(part.text for ... if part.type == "output_text") with refusal fallback
  • ResponseReasoningItem: "\n".join(part.text for part in item.content) matching harmony.py
  • Easy-input assistant dicts (the remaining content[0] at L452): "".join(part.get("text", "") for part in content)

Added a test for the easy-input dict multi-part case as well. Rebase onto current main resolves the merge conflicts.

This branch has not been deployed

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

Labels

bug Something isn't working frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants