[Frontend][Responses API] Fold developer-role input messages into system instructions - #43590
Conversation
…tructions Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request introduces message normalization for chat templates, mapping 'developer' roles to 'system' and merging multiple system messages into a single entry at the start of the message list. It also adds comprehensive unit tests for these scenarios. Feedback indicates that the normalization logic is currently too restrictive because it checks for a 'type' field that standard OpenAI messages lack, which may cause them to be skipped. Additionally, the text extraction logic should be updated to support the standard 'text' content part type to ensure compatibility with standard chat messages.
|
I’m not sure whether we need a command-line flag to enable or disable this feature, since dsv4 pro already supports the developer role. |
| 1. Maps the "developer" role to "system" because most HF chat templates | ||
| do not recognize "developer". | ||
| 2. Merges all system messages into a single message so that chat | ||
| templates (which typically expect at most one) do not receive multiples. |
There was a problem hiding this comment.
While I am pretty sure that item 1 is safe to go for most models that vllm supports, item 2 may surprise some users:
there is no all-agreed semantics for a system message appearing in the middle of chat message array - some models simply ignore it entirely, some models raise, some models even allow multiple system messages by nature. With item 2 merged, some models that rely on this trait silently generate a wildly different result or performance degrade.
There was a problem hiding this comment.
c.f. item 1 type approach also appears in bbrowning@fb34cd1#diff-8e595bc43c5d519d4b6dbfdeab1c72f4eac44a220759c99af944b8de506f6118, and I also find this approach nonviolent in our internal fork.
There was a problem hiding this comment.
+1, I think the fix should live in vllm/renderers/hf.py inside safe_apply_chat_template() rather than in responses/utils.py. That function is the shared bottleneck for both the Chat Completion API and the Responses API on the HF template path, and the developer role issue affects both.
Normalizing in safe_apply_chat_template() also avoids interfering with renderers that handle developer natively (DeepSeek V32/V4 have their own render_messages and never call this function, same for Mistral and Harmony).
Additionally, merging all system messages into one seems unnecessary — most HF Jinja2 templates iterate over messages and handle multiple system messages fine.
cc @bbrowning
There was a problem hiding this comment.
@cjackal An important context to the PR you linked above is in my own testing with Qwen 3.6 I had to follow it up with 0fe00a3 to get that model working with Codex CLI.
So, for some models, we're going to need some logic to optionally (by config) or intelligently (by chat template introspection) collapse system roles into a single one. Because that was on my own fork not yet opened here, I did the easy thing that was to collapse.
I do agree your concerns are valid with regards to collapsing or not having impacts for some models.
There was a problem hiding this comment.
Based on @bbrowning's code, I reworked the implementation and submitted an updated version. I also tested it locally, and it works correctly. Additionally, it does not affect DSV4.
… renderer Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-Authored-By: Ben Browning <bbrownin@redhat.com>
There was a problem hiding this comment.
I'm approving this, as I think it's the safe default for the state of our open weight models today that don't support a developer message natively.
As pointed out in a comment thread above, collapsing all the system messages into 1 may not always be the right thing to do. However, in anecdotal testing of Nemotron 3 Super models (which explicitly support more than 1 system message), a system message appearing after a user message seems to send the model in a weird direction and it feels like I'm getting worse trajectories in agentic coding sessions after that. It was likely only trained for multiple system messages that all come before the first user message.
Given that we only trigger that collapsing for models that don't natively support developer message roles, I think it's a pretty safe assumption that those models also weren't trained in system messages that come after user messages in a conversation. This leads me to believing collapsing is the right thing to do in this situation, so approving this.
If we find future open weight models that don't support developer message but are trained for multiple system messages, including ones that come after user messages, then we can figure out something smarter to do here for those models. It's just as likely that more models will standardize on supporting developer role natively for those kind of mid-conversation steering adjustments, in which case our consolidation logic wouldn't apply and the right thing would just happen.
| "does not define one." | ||
| ) | ||
|
|
||
| if any( |
There was a problem hiding this comment.
Am I missing something? Doesn't seem you "moved" anything, you only added this logic without removing it from the original place
There was a problem hiding this comment.
Oh, sorry, the PR title was indeed incorrect. I've revised the title
… renderer (vllm-project#43590) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Co-authored-by: Ben Browning <bbrownin@redhat.com> Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
… renderer (vllm-project#43590) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Co-authored-by: Ben Browning <bbrownin@redhat.com> Signed-off-by: JisoLya <523420504@qq.com>
… renderer (vllm-project#43590) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Co-authored-by: Ben Browning <bbrownin@redhat.com>
… renderer (vllm-project#43590) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Co-authored-by: Ben Browning <bbrownin@redhat.com> Signed-off-by: Waqar Ahmed <waqar.ahmed@amd.com>
… renderer (vllm-project#43590) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Co-authored-by: Ben Browning <bbrownin@redhat.com>
… renderer (vllm-project#43590) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Co-authored-by: Ben Browning <bbrownin@redhat.com> Signed-off-by: divineearthly <divineearthly@gmail.com>
… renderer (vllm-project#43590) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Co-authored-by: Ben Browning <bbrownin@redhat.com>
… renderer (vllm-project#43590) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Co-authored-by: Ben Browning <bbrownin@redhat.com>
… renderer (vllm-project#43590) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: kdcyberdude <kdsingh.cyberdude@gmail.com> Co-authored-by: Ben Browning <bbrownin@redhat.com>
Co-authored-by: kdcyberdude kdsingh.cyberdude@gmail.com
Co-Authored-By: Ben Browning bbrownin@redhat.com
Signed-off-by: chaunceyjiang chaunceyjiang@gmail.com
Purpose
OpenAI's Responses API allows
role: "developer"items in theinputarray. Clients such as Codex send developer messages for harness / policy text. vLLM's non-harmony path previously forwarded those items into the chat template, which only accepts standard roles and returnedUnexpected message role.fix #42475
fix #42407
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.