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.
This commit is contained in:
Daniel Han
GitHub
parent
9d7740a82f
commit
86c6268d5c
2 changed files with 220 additions and 2 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue