mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-04-14 23:19:55 +00:00
fix(provider): recover trailing assistant message as user to prevent empty request
When a subagent result is injected with current_role="assistant",
_enforce_role_alternation drops the trailing assistant message, leaving
only the system prompt. Providers like Zhipu/GLM reject such requests
with error 1214 ("messages parameter invalid"). Now the last popped
assistant message is recovered as a user message when no user/tool
messages remain.
This commit is contained in:
parent
62bd54ac4a
commit
89ea2375fd
@ -392,8 +392,22 @@ class LLMProvider(ABC):
|
||||
else:
|
||||
merged.append(dict(msg))
|
||||
|
||||
last_popped = None
|
||||
while merged and merged[-1].get("role") == "assistant":
|
||||
merged.pop()
|
||||
last_popped = merged.pop()
|
||||
|
||||
# If removing trailing assistant messages left only system messages,
|
||||
# the request would be invalid for most providers (e.g. Zhipu/GLM
|
||||
# error 1214). Recover by converting the last popped assistant
|
||||
# message to a user message so the LLM can still see the content.
|
||||
if (
|
||||
merged
|
||||
and last_popped is not None
|
||||
and not any(m.get("role") in ("user", "tool") for m in merged)
|
||||
):
|
||||
recovered = dict(last_popped)
|
||||
recovered["role"] = "user"
|
||||
merged.append(recovered)
|
||||
|
||||
return merged
|
||||
|
||||
|
||||
@ -131,6 +131,47 @@ class TestEnforceRoleAlternation:
|
||||
assert msgs[0] == original_first
|
||||
assert len(msgs) == 2
|
||||
|
||||
def test_trailing_assistant_recovered_as_user_when_only_system_remains(self):
|
||||
"""Subagent result injected as assistant message must not be silently dropped.
|
||||
|
||||
When build_messages(current_role="assistant") produces [system, assistant],
|
||||
_enforce_role_alternation would drop the assistant, leaving only [system].
|
||||
Most providers (e.g. Zhipu/GLM error 1214) reject such requests.
|
||||
The trailing assistant should be recovered as a user message instead.
|
||||
"""
|
||||
msgs = [
|
||||
{"role": "system", "content": "You are helpful."},
|
||||
{"role": "assistant", "content": "Subagent completed successfully."},
|
||||
]
|
||||
result = LLMProvider._enforce_role_alternation(msgs)
|
||||
assert len(result) == 2
|
||||
assert result[0]["role"] == "system"
|
||||
assert result[1]["role"] == "user"
|
||||
assert "Subagent completed successfully." in result[1]["content"]
|
||||
|
||||
def test_trailing_assistant_not_recovered_when_user_message_present(self):
|
||||
"""Recovery should NOT happen when a user message already exists."""
|
||||
msgs = [
|
||||
{"role": "system", "content": "You are helpful."},
|
||||
{"role": "user", "content": "Hi"},
|
||||
{"role": "assistant", "content": "Hello!"},
|
||||
]
|
||||
result = LLMProvider._enforce_role_alternation(msgs)
|
||||
assert len(result) == 2
|
||||
assert result[-1]["role"] == "user"
|
||||
|
||||
def test_trailing_assistant_recovered_with_tool_result_preceding(self):
|
||||
"""When only [system, tool, assistant] remains, recovery is not needed
|
||||
because tool messages are valid non-system content."""
|
||||
msgs = [
|
||||
{"role": "system", "content": "You are helpful."},
|
||||
{"role": "tool", "content": "result", "tool_call_id": "1"},
|
||||
{"role": "assistant", "content": "Done."},
|
||||
]
|
||||
result = LLMProvider._enforce_role_alternation(msgs)
|
||||
assert len(result) == 2
|
||||
assert result[-1]["role"] == "tool"
|
||||
|
||||
def test_only_assistant_messages(self):
|
||||
msgs = [
|
||||
{"role": "assistant", "content": "A"},
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user