Skip to content

[Bugfix][Rust Frontend] Accept assistant turns without content like Python - #64492

Merged
BugenZhao merged 2 commits into
vllm-project:mainfrom
hemuku90:fix/rust-assistant-empty-content
Nov 4, 2026
Merged

BugenZhao merged 2 commits into
vllm-project:mainfrom
hemuku90:fix/rust-assistant-empty-content

Conversation

@hemuku90

@hemuku90 hemuku90 commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Overview

The Rust frontend returns a 400 for an assistant message with no content (content: null, content omitted, or content: []) and no tool calls. The Python frontend accepts all three and renders an empty assistant turn, same as content: "", which Rust already accepts. This brings Rust in line.

I ran into it while diffing render output between the two frontends. {"role": "assistant", "content": null} shows up a lot in replayed history (tool calls stripped by the client, placeholder turns), so it's worth matching.

Claims

  • Assistant turns with null, missing or [] content and no tool calls are accepted on /v1/chat/completions, /v1/chat/completions/render and /tokenize, and render the same as content: "".
  • Token IDs match the Python frontend for these, including when the empty turn is the last message, with and without continue_final_message.

Validation

Qwen/Qwen3-0.6B, /v1/chat/completions/render, messages [user "hi", <assistant>, user "hi"]:

<assistant> Python Rust main Rust this PR
{"role":"assistant","content":""} 200 200 200
{"role":"assistant","content":null} 200 400 200
{"role":"assistant"} 200 400 200
{"role":"assistant","content":[]} 200 400 200
{"role":"assistant","content":null,"tool_calls":[]} 200 400 200

The 400s are Assistant messages must contain text, reasoning content, or tool_calls. All the 200s return the same 20 token IDs. I also checked the empty turn as the final message: 18 tokens with default flags and 13 with continue_final_message: true, add_generation_prompt: false, same as Python in both cases.

The new route test sends all five variants (plus content: null with reasoning: "") and checks for a 200 and identical token IDs. It fails on main at the content: null case and passes with the change.

cd rust
cargo test -p vllm-server --lib -- render_chat_accepts_assistant_turn_without_content
cargo test -p vllm-server   # 546 passed

cargo fmt, clippy and pre-commit are clean on the changed files.

After the follow-up commit that removes the check instead of adding an empty text block: cargo nextest run -p vllm-server -p vllm-chat --lib passes (1060 tests), the route test fails with the check restored, and clippy is clean.

Screenshots (Rust main, this PR, Python)

before

after

python

Details

convert_message in chat_completions/convert.rs builds an assistant message out of reasoning, text and tool-call blocks, and it bailed when that list came out empty. content: "" produces one empty text block, so it passed. The other forms of the same empty turn produced nothing and got rejected. The check is removed, so an assistant message may now have no blocks. Every renderer already treats that like one empty text block: they either join the text blocks or skip empty ones. So these turns render the same as content: "". Leaving out a synthetic empty text block also keeps content: null without a text part, which is what Python passes to openai-format templates ([]). Rust's own non-streaming response returns content: null with no tool calls when the generated text is empty, so a client replaying that history used to get this 400.

Since #62424, an empty reasoning: "" is kept as its own block, and the check also rejected a turn whose only blocks were empty reasoning. With the check gone, {"role": "assistant", "content": null, "reasoning": ""} also renders as an empty turn instead of a 400, like Python; the route test covers it as a sixth variant.

I didn't find an existing issue or PR for this. #54368 is the Python side (null content with tool calls), and #63132 and #63137 are about the reasoning field aliases, so they don't overlap.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

@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.

@mergify mergify Bot added rust bug Something isn't working labels Oct 29, 2026
@hemuku90
hemuku90 force-pushed the fix/rust-assistant-empty-content branch 10 times, most recently from d2d849a to a54d14b Compare October 30, 2026 04:13
@mergify

mergify Bot commented Oct 30, 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, @hemuku90.

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

@hemuku90
hemuku90 force-pushed the fix/rust-assistant-empty-content branch 4 times, most recently from b12822d to 4bc1748 Compare October 30, 2026 13:10
@hemuku90

Copy link
Copy Markdown
Contributor Author

@BugenZhao @njhill Please review

@hemuku90
hemuku90 force-pushed the fix/rust-assistant-empty-content branch 6 times, most recently from ad8f2b3 to 60f6543 Compare November 3, 2026 13:05

@BugenZhao BugenZhao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @hemuku90

@BugenZhao

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

❌ This PR is 115 commits behind upstream main. The bot could not merge upstream main into this branch (the merge commit did not appear in time). No new CI build was started. Either merge or rebase onto the latest main yourself and rerun /ci run, or use /ci run --allow-stale to test this branch as is.

@BugenZhao BugenZhao added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 4, 2026
@BugenZhao

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

✅ @hemuku90, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Run commands test your branch with every commit currently on its upstream target branch. If it is behind, the bot merges the target branch into it first (pull before pushing again). Append --allow-stale to a run command to test an outdated branch as is, at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

❌ This PR is 115 commits behind upstream main. The bot could not merge upstream main into this branch (API returned 422: expected head sha didn’t match current head ref.). No new CI build was started. Either merge or rebase onto the latest main yourself and rerun /ci run, or use /ci run --allow-stale to test this branch as is.

@BugenZhao

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

❌ This PR is 117 commits behind upstream main. The bot could not merge upstream main into this branch (API returned 422: expected head sha didn’t match current head ref.). No new CI build was started. Either merge or rebase onto the latest main yourself and rerun /ci run, or use /ci run --allow-stale to test this branch as is.

@BugenZhao

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #99913 for commit cd2cdf07aef5.

ℹ️ This PR is 4 commits behind upstream main. Merged upstream main into this branch as cd2cdf07aef5 before starting CI. Pull the branch before pushing more changes.

hemuku90 and others added 2 commits November 4, 2026 17:46
…ython

An assistant message with null, missing or `[]` content and no tool
calls got a 400 from the Rust frontend, as did one whose only reasoning
is `""`. Python renders all of these as an empty assistant turn, same
as `content: ""`, which Rust already took.

Push an empty text block instead of bailing, so both frontends build
the same prompt.

Signed-off-by: hemuku90 <25220495+hemuku90@users.noreply.github.com>
…xt block

An assistant message with no blocks already renders like one with an
empty text block: every renderer either joins the text blocks or skips
empty ones. So the check can go rather than be turned into a synthetic
`Text("")`, which keeps `content: null` free of a text part, like the
`[]` Python passes to `openai`-format templates.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CJckCC771BVnAH8BWPyaSV
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@hemuku90
hemuku90 force-pushed the fix/rust-assistant-empty-content branch from cd2cdf0 to 0f96c5d Compare November 4, 2026 12:16
@BugenZhao

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #99921 for commit 0f96c5d1016d.

@BugenZhao
BugenZhao merged commit 5611db5 into vllm-project:main Nov 4, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed rust

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants