Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions agent/moa_loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,18 +491,32 @@ def _reference_messages(messages: list[dict[str, Any]]) -> list[dict[str, Any]]:
if role == "system":
continue
if role == "user":
if not text.strip() and content not in (None, "", []):
if not text.strip() and isinstance(content, list) and content:
# Structured content with no extractable text (e.g. an
# image-only turn). Emitting an empty user message would be
# dropped/rejected by strict providers (Anthropic 400s on
# empty text blocks — the original "closed" preset failure
# mode), and silently skipping the turn would break
# user/assistant alternation in the advisory view. Substitute
# a placeholder so the reference knows a non-text turn
# happened.
# happened. Only structured content qualifies — an empty or
# whitespace-only STRING turn carries nothing and is dropped
# below instead.
text = "[user sent non-text content (e.g. an image attachment)]"
if text.strip():
last_user_content = text
if not text.strip():
# Genuinely empty user turn (content="" / None). It carries
# nothing advisory, and strict providers (Kimi/Moonshot, ZAI,
# and others that enforce non-empty user content) reject it
# with 400 "message ... with role 'user' must not be empty" —
# the same way the assistant branch below drops turns with no
# parts. Lenient providers (DeepSeek) accept the empty turn,
# which is why a MoA fan-out would fail on one reference and
# pass on another for the identical rendered view. The
# advisory view is already not strictly alternating (adjacent
# assistant turns occur in every tool loop), so dropping a
# contentless turn is safe.
continue
last_user_content = text
rendered.append({"role": "user", "content": text})
elif role == "assistant":
parts: list[str] = []
Expand Down
1 change: 1 addition & 0 deletions scripts/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@

# Auto-extracted from noreply emails + manual overrides
AUTHOR_MAP = {
"neo@neodeMac-mini.local": "neo-claw-bot", # PR #58465 salvage (moa: drop empty user turns from advisory view)
"m.guttmann@journaway.com": "mguttmann", # PR #63738 salvage (Anthropic setup-token pool auth normalization)
"VrtxOmega@pm.me": "VrtxOmega", # PR #43809 salvage (desktop: WSL folder-picker path bridge)
"jake.long.vu@vucar.net": "jakelongvu-bot", # PR #36683 partial salvage (approval: honor canonical approvals.timeout in gateway waits)
Expand Down
62 changes: 62 additions & 0 deletions tests/run_agent/test_moa_loop_mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,44 @@ def test_reference_messages_fresh_user_turn_ends_on_that_user():
assert view[-1] == {"role": "user", "content": "q2 current"}


def test_reference_messages_drops_empty_user_turns():
"""Empty user turns must not leak into the advisory view.

A user message whose content is "" or a non-string/multimodal payload
(flattened to "" by the text-extraction step) carries nothing advisory.
Strict providers (Kimi/Moonshot and others that enforce non-empty user
content) reject such a message with
400 "message ... with role 'user' must not be empty", while lenient
providers (DeepSeek) accept it — so a fan-out over the identical rendered
view fails on one reference and passes on another. The renderer must emit
NO empty user turn, mirroring how empty assistant turns are dropped.
"""
from agent.moa_loop import _reference_messages

messages = [
{"role": "system", "content": "sys"},
{"role": "user", "content": "real question"},
{"role": "assistant", "content": "", "tool_calls": [
{"function": {"name": "read_file", "arguments": '{"path":"c.yaml"}'}}
]},
{"role": "tool", "content": "some result"},
{"role": "user", "content": ""}, # empty string user turn
{"role": "user", "content": [{"type": "text", "text": "multimodal"}]}, # non-string -> ""
]

view = _reference_messages(messages)

# No user turn in the view may be empty/whitespace-only.
empty_users = [
m for m in view
if m.get("role") == "user" and not str(m.get("content", "")).strip()
]
assert empty_users == [], f"empty user turn leaked into advisory view: {empty_users}"
# The real user prompt survives and the view still ends on a user turn.
assert view[0] == {"role": "user", "content": "real question"}
assert view[-1]["role"] == "user"


def test_run_reference_prepends_advisory_system_prompt(monkeypatch):
"""Each reference call gets the advisory-role system prompt first.

Expand Down Expand Up @@ -1203,3 +1241,27 @@ def test_reference_guidance_appends_text_part_to_decorated_trailing_user():
assert content[0] == marked_part
# The guidance rides as a trailing text part outside the cached span.
assert content[1] == {"type": "text", "text": "\n\nREFERENCE BLOCK"}


def test_reference_messages_drops_whitespace_only_string_user_turn():
"""A whitespace-only STRING user turn is dropped, not placeholdered.

The non-text placeholder exists for structured content (image-only turns)
where a real turn happened that the reference should know about. A bare
whitespace string carries nothing — emitting it would 400 strict
providers (Kimi/Moonshot 'role user must not be empty'), and
placeholdering it would fabricate an attachment that never existed.
"""
from agent.moa_loop import _reference_messages

messages = [
{"role": "user", "content": " "},
{"role": "assistant", "content": "a"},
{"role": "user", "content": "real"},
]

view = _reference_messages(messages)

assert view[0] == {"role": "assistant", "content": "a"}
assert view[-1] == {"role": "user", "content": "real"}
assert all(str(m["content"]).strip() for m in view)
Loading