Repository navigation
Conversation
An assistant history message carrying both `reasoning` and `reasoning_content` failed deserialization with serde's opaque `duplicate field` error, permanently breaking multi-turn agent sessions on the Rust frontend. Normalize the reasoning aliases to the primary `reasoning` field before dispatching to the derived impl, mirroring the Python frontend's _normalize_messages_before. Fixes vllm-project#63130 Assisted-by: opencode (GLM by Z.ai) Signed-off-by: world-executed <yolohalo@163.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
|
@claude review |
BugenZhao
left a comment
BugenZhao
left a comment
There was a problem hiding this comment.
Thanks! It seems that the serde(alias) attributes are dead now. I’ll remove them.
`normalize_reasoning_aliases` removes `reasoning_content` and `thinking` before the derived impl runs, so the `#[serde(alias)]` attributes on `reasoning` were unreachable. Drop them and keep the alias set in one place. Fold the alias tests into one table at the owning type, adding reversed key order, an empty `reasoning` winning, and a wrong-type alias being dropped; remove the `prepare_chat_request` test, which re-checked unchanged lowering. Note in the docs that `thinking` is a Rust-only, lowest-precedence alias. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CJckCCBHXGwmtTiEEqpHQq Signed-off-by: Bugen Zhao <i@bugenzhao.com>
|
/ci run |
|
✅ @world-executed, CI is now available for this PR.
|
|
✅ Triggered Buildkite CI #99163 for commit ℹ️ This PR is 742 commits behind upstream |
Overview
Fixes #63130: the Rust frontend rejected chat requests whose assistant message carries both
reasoningandreasoning_contentwith an opaque serdeduplicate fieldparse error, while the Python frontend accepts the identical body. This permanently breaks multi-turn agent loops because the offending assistant turn stays in history and every follow-up request fails.The fix normalizes the reasoning aliases to the primary
reasoningfield inChatMessage::deserializebefore dispatching to the derived impl, mirroring the Python frontend's_normalize_messages_before(vllm/entrypoints/openai/chat_completion/protocol.py:556).Claims
reasoning+reasoning_content(in any key order, plus thethinkingalias) is now accepted, matching the Python frontend's behavior matrix from the issue.reasoningwins (including an empty string); a JSON-nullreasoningfalls back toreasoning_content, exactly mirroring the Python normalization semantics.Validation
Reproducer from the issue (both keys together) and the alias precedence matrix (key order, null and empty
reasoning, wrong-type alias,thinking) are covered by one table-driven test at the deserialization layer,assistant_reasoning_aliases_fold_into_reasoning:Regression proof: with the
normalize_reasoning_aliasescall removed,assistant_reasoning_aliases_fold_into_reasoningfails because alias values no longer reachreasoning; restored, all tests pass.cargo fmt --checkandcargo clippy -p vllm-server --all-targets -- -D warningsare clean.Details
Root cause.
ChatMessage::Assistantdeclaresreasoningwith#[serde(alias = "reasoning_content")]/#[serde(alias = "thinking")]. Serde's derivedDeserializetreats every key mapping to one field as a candidate and raisesduplicate fieldwhen two of them appear, before any normalization can run. The Python frontend instead popsreasoning_contentin amodel_validator(mode="before")and fillsreasoningonly when it is absent/null, so the same payload is accepted there.Fix. The manual
Deserializeimpl already materializes each message as aserde_json::Valuefor role dispatch; a smallnormalize_reasoning_aliasesstep runs on that value first (removes both alias keys, promotes the first non-null alias whenreasoningis absent/null). Because it is key-based rather than order-based, it is deterministic regardless of client key order. The#[serde(alias)]attributes are removed, so this step is the only place that defines the alias set.Intentional deviation (Python parity note). The Python frontend does not normalize a top-level
thinkingstring on messages (it is an unknown field there), while the Rust frontend deliberately accepts it as an alias. The collision handling therefore orders precedence asreasoning>reasoning_content>thinking, keeping the existing Rust-onlythinkingextension but making it tolerant of duplicates too.Not duplicating an existing PR. Verified with
gh pr list --search "63130 in:body"(0 results) and issue search; no open PR addresses this. Note: a comment on the issue mentions a similar effort in progress; this PR is a narrow, tested fix at the same layer and is offered in case it is useful — happy to yield or rebase on a competing approach if maintainers prefer.Limitation. This restores wire-level acceptance parity for the reasoning alias family. The Python frontend's separate semantic rejection of typed
thinkingcontent blocks combined with top-levelreasoning(PR #41718) is not ported here; that is a distinct validation concern and would merit its own change.Pull Request Checklist
I used vLLM's
/pr-checklistskill. (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.