Repository navigation
feat(protocols)!: accept the root chat message role - #414
ayerofieiev-tt wants to merge 2 commits into
Conversation
MiniMax-M3's chat template reads messages[0].role == "root" as its top-priority system prompt, so the role has to reach the template verbatim; requests carrying it were rejected as an unknown variant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Artem Yerofieiev <ayerofieiev@tenstorrent.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Artem Yerofieiev <ayerofieiev@tenstorrent.com>
|
@GuanLuo @keivenchang let me know if this approach does not work. It would be helpful to get this reviewed soon. Thank you! |
| Developer(ChatCompletionRequestDeveloperMessage), | ||
| /// MiniMax-M3's top-priority system prompt. Kept distinct from `System` so | ||
| /// chat templates see `role: "root"`; it cannot carry Kimi `tools`. | ||
| Root(ChatCompletionRequestSystemMessage), |
There was a problem hiding this comment.
Reusing ChatCompletionRequestSystemMessage exposes its public tools field on Root, while the enum still derives Serialize. A caller can therefore construct Root with tools and serialize {"role":"root","tools":[...]}, but this deserializer rejects that same shape, so valid Rust values do not round-trip and the stated no-tools contract is unenforced on output. Give Root a payload that cannot contain tools, or reject such values during serialization.
🤖 AI Fix
Introduce a root-specific message payload without tools (preserving supported content/name fields) and use it for both serialization and deserialization.
luckyq
left a comment
There was a problem hiding this comment.
Summary: Fixes MiniMax-M3 requests whose first message carries "role": "root" being rejected at parse time with unknown variant 'root', expected one of developer, system, user, assistant, tool, function (HTTP 400), which makes M3 deployments fail MiniMax-Provider-Verifier tests 11_01 / 11_02 / 11_04. It adds a Root(ChatCompletionRequestSystemMessage) variant to ChatCompletionRequestMessage plus a matching Wire::Root arm with the same tools / partial guards as developer, so the literal root role reaches the Jinja template and the model keeps its root-over-system priority. No blockers.
Problem / current state on main
protocols/src/types/chat.rs—ChatCompletionRequestMessageis a closed serde-tagged enum with six variants; the hand-writtenDeserializemirrors them inWire, so any other role string is a hard error before the renderer runs.- Remapping
roottoSystemwould not be enough: the HF renderer serializes typed messages back to JSON and the M3 template branches on the literal role string (messages[0].role == "root"→ system block; a followingsystem/developerturn → demoted to the developer block).
What the PR changes
- New public variant
Root(serialized"role": "root", reuses the system payload type) and theWire::Rootdeserializer arm rejectingtoolsandpartial. - Tests: round-trip, unknown-role rejection, forbidden-field cases, schema validity under
protocol-schema, renderer test showingrootreaches a Jinja template verbatim. - No renderer or parser logic changes needed; they operate on JSON role strings.
Non-blocking
- Semver: new variant in an exhaustive public enum, so the
!is correct.dynamo-parsersanddynamo-rendererneed a release against the new protocols major, and ai-dynamo/dynamo must add aRoot(_)arm to the exhaustive match inlib/llm/src/protocols/common/input_trigger.rswhen it moves off thedynamo-protocols = "=8.1.0"pin. Worth stating in the PR description so the downstream bump is not missed.
Non-blocking (housekeeping before merge)
- The bot thread from 2026-10-21 (
Rootreuses the system payload, so atoolsfield serializes but will not deserialize) is still unanswered; a one-line reply or a note in the description would close it. - The PR description carries an AI-assistant attribution footer and the commits carry AI co-author trailers; repo convention is to strip both.
- No Rust CI has run on this fork yet (
pull-request/414mirror branch does not exist); a maintainer needs to issue/ok to testbefore merge, and ai-dynamo/dynamo needs aRoot(_)arm ininput_trigger.rswhen it bumps the protocols pin.
Checked
- Enum and custom
Deserializeat PR head. - Every
ChatCompletionRequestMessage::match in protocols, renderer and parsers: none exhaustive outsidechat.rs; thedynamic_system_toolswildcard is correct forRoot. - Renderer
normalize_system_messagesagainst the actual MiniMax-M3chat_template.jinja: the template never raises on a non-leading system message, so the demote path cannot fire for M3. - Schema snapshots: none committed, nothing to regenerate.
- Clean merge onto current main; CI state and existing review threads.
|
/ok to test 08e8c43 |
|
Two things to clear before this can merge:
|
Why
MiniMax-M3 adds a
rootmessage role: a system-prompt channel that ranks abovesystem. Its chat template expectsmessages[0].role == "root", renders it as the system prompt, and demotes a followingsystem/developerturn to a lower-priority developer prompt (chat_template.jinjaL99-L114). MiniMax's provider verifier requires endpoints to accept it (TestRoleRoot,m3_text_tests.pyL953-L1117:test_11_01_role_root_acceptedL1050,test_11_02_root_overrides_systemL1070,test_11_04_only_root_identityL1104).Today any such request fails during deserialization with HTTP 400:
Mapping
rootontoSystemwould not help: the template only sees the role string, sorootmust reach it unchanged. This is the same situation asdeveloper, which is a first-class variant re-serialized as"developer".What changed
ChatCompletionRequestMessage::Root(ChatCompletionRequestSystemMessage), serialized with"role": "root". It reuses the system message payload, so consumers that treat root like system can writeSystem(m) | Root(m).Wiredeserializer acceptsroot, with the same field guards asdeveloper: Kimitoolsandpartialare rejected on root.Fromimpl: the payload type already converts intoSystem, and a second impl would conflict. Construct it withChatCompletionRequestMessage::Root(...).rootreaches Jinja unchanged. The model-specific native renderers (DeepSeek, Kimi K3, Inkling) are left as they are because their reference encoders have norootrole.Semver: adding a variant to this exhaustive public enum is a breaking change, hence
!. As with #375,dynamo-parsersanddynamo-rendererwill need a release against the new protocols major.Tests
root_message_round_trips_with_its_role: string and text-part content deserialize toRootand serialize back to the same JSON with"role": "root".request_keeps_root_ahead_of_system: a full request with root + system + user, including the verifier'sthinking/reasoning_splitextras, keeps the order and roles when serialized back.message_rejects_unknown_role: an unknown role is still rejected, and the error listsroot.message_rejects_tools_and_partial_on_wrong_roles: adds root cases.protocol_schema::custom_deserializers_produce_schema_valid_canonical_messages: the canonical root message validates against the exported schema.renderer::template::oai::root_role_reaches_the_template_verbatim: an excerpt of M3's role-mapping logic renders root as the system prompt and system as the developer prompt.chat_template.jinjaputs the root content in thesystemblock and the system content in thedeveloperblock.Dynamo follow-up
Dynamo pins
dynamo-protocols = "=8.1.0". After the bump, this exhaustive match stops compiling:lib/llm/src/protocols/common/input_trigger.rs:41-49(classify_create_chat_completion_request): addRoot(_)to theSystem(_) | Developer(_) => InputTrigger::Otherarm.Every other
ChatCompletionRequestMessagematch inlib/llmhas a wildcard arm or usesmatches!. Root cannot carrytools, sopreprocessor.rs:3400, which detects dynamic tools on system messages, is still correct.🤖 Generated with Claude Code