From 86c6268d5cb2c0c36d0c168ea2760f745e96172b Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 15 Jun 2026 04:56:11 -0700 Subject: [PATCH] Studio: coalesce orphaned GGUF user turns so strict chat templates stay alternating (#5980) When an empty assistant turn is dropped (a 0-token model reply, or a Stop-button sentinel handled by _drop_empty_assistant_sentinels) the GGUF chat history can be left with two user turns back to back. Strict Jinja chat templates (Gemma 3, some Mistral variants) call raise_exception("Conversation roles must alternate ...") on the first role-parity break, so llama-server returns a 400 and the thread becomes unsendable. Add _coalesce_consecutive_user_turns (merging only adjacent user turns, never assistant/tool turns, preserving multimodal parts) and apply it in _openai_messages_for_gguf_chat. The tool path inherits the fix for free because it rebuilds from this same normalized history via _set_or_prepend_system_message. The passthrough path is left untouched (it forwards messages verbatim). No-op for already-alternating histories. Adds unit and end-to-end coverage. --- studio/backend/routes/inference.py | 51 +++++- .../tests/test_openai_tool_passthrough.py | 171 ++++++++++++++++++ 2 files changed, 220 insertions(+), 2 deletions(-) diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index bf471eeb6e..d7db0e6f8d 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -7796,6 +7796,47 @@ def _drop_empty_assistant_sentinels(messages: list[dict]) -> list[dict]: return out +def _merge_user_content(a: Any, b: Any) -> Any: + """Join two user ``content`` values: strings with a blank line, else as concatenated parts.""" + if isinstance(a, str) and isinstance(b, str): + if not a: + return b + if not b: + return a + return a + "\n\n" + b + + def _parts(c: Any) -> list: + if c is None: + return [] + if isinstance(c, str): + return [{"type": "text", "text": c}] if c else [] + if isinstance(c, list): + return list(c) + return [{"type": "text", "text": str(c)}] + + return _parts(a) + _parts(b) + + +def _coalesce_consecutive_user_turns(messages: list[dict]) -> list[dict]: + """Merge adjacent user turns so the GGUF history stays alternating. + + Dropping an empty assistant turn (0-token reply or Stop-button sentinel) can + leave two user turns in a row, which makes strict templates (Gemma 3, ...) + raise "Conversation roles must alternate" -> llama-server 400. Only user turns + merge (assistant/tool turns may carry tool_calls/tool_call_id); multimodal + parts are preserved; no-op for already-alternating histories. + """ + out: list[dict] = [] + for m in messages: + if m.get("role") == "user" and out and out[-1].get("role") == "user": + prev = dict(out[-1]) + prev["content"] = _merge_user_content(prev.get("content"), m.get("content")) + out[-1] = prev + continue + out.append(m) + return out + + _LOCAL_SERVER_BUILTIN_TOOL_NAMES = frozenset( {"web_search", "web_fetch", "code_execution", "image_generation"} ) @@ -7960,8 +8001,14 @@ def _openai_messages_for_gguf_chat(payload, is_vision: bool) -> tuple[list[dict] per-turn ``image_url`` parts so multi-image chat history keeps each image attached to its original turn. """ - messages = _strip_provider_synthetic_tool_history( - _drop_empty_assistant_sentinels([m.model_dump(exclude_none = True) for m in payload.messages]) + # Coalesce only on the GGUF chat path (strict Jinja template); the tool path + # reuses this via _set_or_prepend_system_message. Passthrough forwards verbatim. + messages = _coalesce_consecutive_user_turns( + _strip_provider_synthetic_tool_history( + _drop_empty_assistant_sentinels( + [m.model_dump(exclude_none = True) for m in payload.messages] + ) + ) ) has_message_image = any( isinstance(msg.get("content"), list) diff --git a/studio/backend/tests/test_openai_tool_passthrough.py b/studio/backend/tests/test_openai_tool_passthrough.py index b54f4c130d..b2b22f8934 100644 --- a/studio/backend/tests/test_openai_tool_passthrough.py +++ b/studio/backend/tests/test_openai_tool_passthrough.py @@ -32,9 +32,13 @@ from routes.inference import ( _build_openai_passthrough_body, _build_passthrough_payload, _clamp_finish_reason, + _coalesce_consecutive_user_turns, + _drop_empty_assistant_sentinels, _effective_max_tokens, _extract_content_parts, _friendly_error, + _merge_user_content, + _openai_messages_for_gguf_chat, _openai_stream_usage_chunk, _set_or_prepend_system_message, openai_chat_completions, @@ -1446,3 +1450,170 @@ class TestResponsesChatTemplateKwargs: ) chat_req = _build_chat_request(payload, self._messages, stream = False) assert chat_req.enable_thinking is None + + +# ===================================================================== +# GGUF chat-template role alternation: coalesce orphaned user turns left +# behind when an empty assistant turn is dropped, so strict templates +# (Gemma 3, ...) do not 400 on a role-parity break. +# ===================================================================== + + +class TestMergeUserContent: + def test_strings_join_with_blank_line(self): + assert _merge_user_content("hi", "again") == "hi\n\nagain" + + def test_empty_sides_passthrough(self): + assert _merge_user_content("", "again") == "again" + assert _merge_user_content("hi", "") == "hi" + + def test_multimodal_parts_concatenate(self): + img = {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}} + out = _merge_user_content([{"type": "text", "text": "look"}, img], "and this?") + assert out == [ + {"type": "text", "text": "look"}, + img, + {"type": "text", "text": "and this?"}, + ] + + +class TestCoalesceConsecutiveUserTurns: + def test_merges_two_string_user_turns(self): + msgs = [ + {"role": "user", "content": "hi"}, + {"role": "user", "content": "again"}, + ] + assert _coalesce_consecutive_user_turns(msgs) == [ + {"role": "user", "content": "hi\n\nagain"}, + ] + + def test_merges_three_consecutive_user_turns(self): + msgs = [ + {"role": "user", "content": "a"}, + {"role": "user", "content": "b"}, + {"role": "user", "content": "c"}, + ] + assert _coalesce_consecutive_user_turns(msgs) == [ + {"role": "user", "content": "a\n\nb\n\nc"}, + ] + + def test_alternating_history_is_unchanged(self): + msgs = [ + {"role": "system", "content": "sys"}, + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "hello"}, + {"role": "user", "content": "bye"}, + ] + assert _coalesce_consecutive_user_turns(msgs) == msgs + + def test_assistant_and_tool_turns_untouched(self): + msgs = [ + {"role": "user", "content": "weather?"}, + { + "role": "assistant", + "tool_calls": [ + { + "id": "call_1", + "type": "function", + "function": {"name": "get_weather", "arguments": "{}"}, + } + ], + }, + {"role": "tool", "tool_call_id": "call_1", "content": "{}"}, + ] + assert _coalesce_consecutive_user_turns(msgs) == msgs + + def test_multimodal_parts_survive_merge(self): + img = {"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}} + msgs = [ + {"role": "user", "content": [{"type": "text", "text": "look"}, img]}, + {"role": "user", "content": "and this?"}, + ] + out = _coalesce_consecutive_user_turns(msgs) + assert len(out) == 1 + assert out[0]["content"] == [ + {"type": "text", "text": "look"}, + img, + {"type": "text", "text": "and this?"}, + ] + + def test_does_not_mutate_input(self): + msgs = [ + {"role": "user", "content": "hi"}, + {"role": "user", "content": "again"}, + ] + _coalesce_consecutive_user_turns(msgs) + assert msgs[0]["content"] == "hi" + + +class TestGgufChatHistoryAlternation: + def test_empty_assistant_turn_dropped_then_users_coalesced(self): + req = ChatCompletionRequest( + model = "default", + messages = [ + ChatMessage(role = "user", content = "hi"), + ChatMessage(role = "assistant", content = ""), + ChatMessage(role = "user", content = "again"), + ], + ) + out, _ = _openai_messages_for_gguf_chat(req, is_vision = False) + roles = [m["role"] for m in out] + assert roles == ["user"] + assert out[0]["content"] == "hi\n\nagain" + + def test_bare_stop_sentinel_also_coalesced(self): + req = ChatCompletionRequest( + model = "default", + messages = [ + ChatMessage(role = "user", content = "hi"), + ChatMessage(role = "assistant"), + ChatMessage(role = "user", content = "again"), + ], + ) + out, _ = _openai_messages_for_gguf_chat(req, is_vision = False) + roles = [m["role"] for m in out] + assert all(roles[i] != roles[i + 1] for i in range(len(roles) - 1)), roles + assert roles == ["user"] + + def test_system_prompt_preserved(self): + req = ChatCompletionRequest( + model = "default", + messages = [ + ChatMessage(role = "system", content = "be brief"), + ChatMessage(role = "user", content = "hi"), + ChatMessage(role = "assistant", content = ""), + ChatMessage(role = "user", content = "again"), + ], + ) + out, _ = _openai_messages_for_gguf_chat(req, is_vision = False) + assert [m["role"] for m in out] == ["system", "user"] + assert out[1]["content"] == "hi\n\nagain" + + def test_normal_history_unchanged(self): + req = ChatCompletionRequest( + model = "default", + messages = [ + ChatMessage(role = "user", content = "hi"), + ChatMessage(role = "assistant", content = "hello"), + ChatMessage(role = "user", content = "again"), + ], + ) + out, _ = _openai_messages_for_gguf_chat(req, is_vision = False) + assert [m["role"] for m in out] == ["user", "assistant", "user"] + + def test_tool_path_rebuild_stays_alternating(self): + # Tool path rebuilds via _set_or_prepend_system_message over the coalesced + # history, so it stays alternating too. + req = ChatCompletionRequest( + model = "default", + messages = [ + ChatMessage(role = "user", content = "hi"), + ChatMessage(role = "assistant", content = ""), + ChatMessage(role = "user", content = "again"), + ], + ) + normalized, _ = _openai_messages_for_gguf_chat(req, is_vision = False) + rebuilt = _set_or_prepend_system_message(normalized, "You have access to tools.") + roles = [m["role"] for m in rebuilt] + assert roles == ["system", "user"] + assert all(roles[i] != roles[i + 1] for i in range(len(roles) - 1)), roles