Skip to content

[Bugfix]: preserve array-formatted system message content (#201) - #214

Open
wuhang2014 wants to merge 4 commits into
vllm-project:mainfrom
wuhang2014:fix/201-system-array-content
Open

[Bugfix]: preserve array-formatted system message content (#201)#214
wuhang2014 wants to merge 4 commits into
vllm-project:mainfrom
wuhang2014:fix/201-system-array-content

Conversation

@wuhang2014

@wuhang2014 wuhang2014 commented Aug 16, 2026

Copy link
Copy Markdown

Purpose

Fixes #201: system messages with OpenAI-style array-formatted content (e.g. [{"type":"text","text":"..."}]) were silently forwarded to upstream vLLM as an empty string "", so the system prompt was completely lost. String-format system content and array-format user content both worked correctly.

Root cause: ChatMessage's custom Deserialize "system" branch coerced the content field with Value::as_str(), which returns None for arrays, falling back to "".

Fix: model ChatMessage::System.content with a role-specific SystemMessageContent (untagged Text | Parts) whose parts are text-only (SystemContentPart), and deserialize it via serde_json::from_value, so both content forms round-trip through the router's parse-and-reserialize forwarding path. Serialization of the untagged enum already handles string vs array.

Strict OpenAI compatibility: system message arrays may only contain text parts. A present but unparseable system content (e.g. image_url parts) is rejected at deserialization time and surfaces as a 4xx (422 via axum's Json extractor) instead of silently degrading to an empty string; missing/null content still defaults to empty text for backward compatibility. User messages remain permissive (UserMessageContent with image_url support).

Test Plan

cargo fmt --check
cargo clippy --all-targets --all-features -- -D warnings
cargo test --lib --bins
cargo test --test '*'

Added unit tests in src/protocols/spec.rs:

  • test_chat_message_system_array_content_roundtrip — deserialize → re-serialize → reparse of array-formatted system content
  • test_chat_message_system_rejects_non_text_partsimage_url (and other non-text) parts are rejected with a deserialization error; missing/null content still defaults to empty text

End-to-end verification: router built with this change, mock upstream worker echoing the received body, POST /v1/chat/completions.

Test Result

Rust unit/integration tests and CI-grade clippy all pass (only pre-existing failure: test_openai_router_health returns 503 on macOS, also fails on pristine main).

End-to-end forwarded content, before → after:

Message content Before After
system array (text parts) "" [{"type":"text","text":"You are GLM-5.2."}]
system string "You are GLM-5.2." "You are GLM-5.2."
system array (image_url part) "" HTTP 422, rejected
user array [{"type":"text","text":"hello array user"}] [{"type":"text","text":"hello array user"}]
user array (image_url part) forwarded forwarded (unchanged, permissive)

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results

Copilot AI lite review requested due to automatic review settings August 16, 2026 02:53
@wuhang2014 wuhang2014 changed the title fix: preserve array-formatted system message content (#201) [Bugfix]: preserve array-formatted system message content (#201) Aug 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a forwarding bug in the OpenAI Chat Completions protocol layer where system messages using OpenAI-style array content parts were deserialized as an empty string and therefore lost when re-serialized and forwarded upstream.

Changes:

  • Change ChatMessage::System.content from String to UserMessageContent (same polymorphic representation already used for User messages).
  • Update the custom Deserialize implementation for ChatMessage to parse system content via serde_json::from_value.
  • Add a regression unit test ensuring system array content survives deserialize → serialize → reparse roundtrips; update benchmark fixtures accordingly.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/protocols/spec.rs Makes system message content polymorphic (string or parts array) and adds a regression test for roundtripping array-formatted system content.
benches/request_processing.rs Updates benchmark request constructors to use UserMessageContent::Text for system messages after the type change.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/protocols/spec.rs Outdated
@wuhang2014
wuhang2014 force-pushed the fix/201-system-array-content branch from 2728200 to 1b2424d Compare August 16, 2026 04:22
…t#201)

Reusing UserMessageContent for system messages would also accept and
forward image_url parts, but the OpenAI spec allows only text parts in
system message arrays. Introduce role-specific SystemMessageContent
(Text | Parts of text-only SystemContentPart) instead.

A present but unparseable system content (image_url or other non-text
parts) is now rejected with a deserialization error (400) rather than
silently degrading to an empty string; missing/null content still
defaults to empty text for backward compatibility.

Adds test_chat_message_system_rejects_non_text_parts.

Signed-off-by: WU Hang <whlbx@hotmail.com>
Signed-off-by: WU Hang <whlbx@hotmail.com>
@wuhang2014
wuhang2014 force-pushed the fix/201-system-array-content branch from 08b7d0e to 6d5ac8f Compare August 16, 2026 09:07
 P2)

SystemContentPart::Text only modeled the text field, so valid
extension fields such as prompt_cache_breakpoint were silently dropped
by the typed parse/reserialize forwarding round-trip. Capture unknown
fields via #[serde(flatten)] and serialize them back unchanged.

P1 (malformed content silently rewritten to "") is already addressed
by the strict deserialization error path added in 7ae97ad; the new
round-trip test also pins that metadata preservation regression.

Signed-off-by: WU Hang <whlbx@hotmail.com>
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.

System message with array-formatted content is silently dropped to empty string when forwarding

2 participants