fix(responses): merge leading system messages in user_note_safe fallback#1923
Merged
jundot merged 1 commit intoJun 22, 2026
Merged
Conversation
When `_downgrade_mid_system_to_user_notes` succeeds, it preserves leading system blocks as-is (`rewritten.extend(messages[start:i])`). If there are multiple leading system messages (e.g. `instructions` + input `system`/developer message), they survive as separate messages. The subsequent `_merge_consecutive_roles` only merges user/assistant roles, leaving consecutive system messages unmerged. Strict chat templates like Qwen3.6 validate that system messages only appear at the first position, so two leading system messages trigger: "System message must be at the beginning." Fix: call `_merge_consecutive_system_messages` in `unsupported_fallback` before `_merge_consecutive_roles` when the user_note_safe path succeeds. Closes jundot#1908
Owner
|
Thanks for the fix. The change is scoped to the user_note_safe fallback and uses the existing system-message merge path after mid-system notes are downgraded, so it addresses #1908 without changing the native-preserve path. I verified the API utility tests locally and CI is green. This looks good to me, and I'm going to merge it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When Codex App Desktop sends
/v1/responsesrequests with bothinstructionsand asystem/developermessage ininput, two leading system messages are produced. If the conversation also contains a mid-system message (e.g. viaprevious_response_idchain or multi-turn input), theuser_note_safedowngrade path inprepare_system_messages_for_templatepreserves the leading system block as-is without merging.Since
_merge_consecutive_rolesonly mergesuser/assistantroles, the two consecutive system messages survive into the chat template. Strict templates like Qwen3.6 validate that system messages can only appear at the first position, causing:Root Cause
In
omlx/api/utils.py,_downgrade_mid_system_to_user_notespreserves leading system blocks viarewritten.extend(messages[start:i])at line 601-602. The callerunsupported_fallbackthen only applies_merge_consecutive_roles, which doesn't touch system messages.Fix
Call
_merge_consecutive_system_messagesinunsupported_fallbackbefore_merge_consecutive_roleswhen theuser_note_safepath succeeds. This merges consecutive leading system messages into one, satisfying strict template validation.Reproduction
Closes #1908