From 2b728e12748e3a30273acdbef36ecad15a04f2b9 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 28 Apr 2026 03:50:51 -0700 Subject: [PATCH] fix(agent): drop thinking-only assistant turns before provider call (#16959) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a pre-call sanitizer that detects assistant messages containing only reasoning (reasoning / reasoning_content, no visible content, no tool_calls) and drops them from the API copy. Adjacent user messages left behind are merged so role alternation is preserved for the provider. Mirrors Claude Code's approach in src/utils/messages.ts (filterOrphanedThinkingOnlyMessages + mergeAdjacentUserMessages). We drop the whole turn rather than fabricate stub text (the '.' / '(continued)' pattern from contributor PRs #11098, #13010, #16842 that were rejected because they put words in the model's mouth). The stored conversation history (self.messages) is never mutated — only the per-call api_messages copy. Users still see the reasoning block in the CLI/gateway transcript; only the wire copy is cleaned. Session persistence keeps the full trace. Two call sites covered: - Main agent loop, after _sanitize_api_messages (catches every turn). - Iteration-limit-summary fallback path. Tests: tests/run_agent/test_thinking_only_sanitizer.py — 25 cases covering detection (string/list content, whitespace-only, tool_calls, reasoning_details list form), drop behavior, adjacent-user merge (string+string, list+list, mixed), non-mutation of input dicts, and system-message handling. E2E live-tested against 5 providers with a poisoned history (empty assistant message + reasoning_content): OpenRouter→Anthropic/OpenAI/ DeepSeek-R1/Qwen, native Gemini. All 5 accepted the cleaned request. Happy-path regression (5/5) confirms the sanitizer is a noop when no thinking-only turn exists. Related: #16823 (wontfix — stub-text approach rejected). Co-authored-by: teknium1 --- run_agent.py | 153 +++++++++++ .../run_agent/test_thinking_only_sanitizer.py | 249 ++++++++++++++++++ 2 files changed, 402 insertions(+) create mode 100644 tests/run_agent/test_thinking_only_sanitizer.py diff --git a/run_agent.py b/run_agent.py index 568a7f7ac0..387a6d00ed 100644 --- a/run_agent.py +++ b/run_agent.py @@ -4793,6 +4793,145 @@ class AIAgent: ) return messages + @staticmethod + def _is_thinking_only_assistant(msg: Dict[str, Any]) -> bool: + """Return True if ``msg`` is an assistant turn whose only payload is reasoning. + + "Thinking-only" means the model emitted reasoning (``reasoning`` or + ``reasoning_content``) but no visible text and no tool_calls. When sent + back to providers that convert reasoning into thinking blocks (native + Anthropic, OpenRouter Anthropic, third-party Anthropic-compatible + gateways), the resulting message has only thinking blocks — which + Anthropic rejects with HTTP 400 "The final block in an assistant + message cannot be `thinking`." + + Symmetric with Claude Code's ``filterOrphanedThinkingOnlyMessages`` + (src/utils/messages.ts). We drop the whole turn from the API copy + rather than fabricating stub text — the message log (UI transcript) + keeps the reasoning block; only the wire copy is cleaned. + """ + if not isinstance(msg, dict) or msg.get("role") != "assistant": + return False + if msg.get("tool_calls"): + return False + # Does it have any actual output? + content = msg.get("content") + if isinstance(content, str): + if content.strip(): + return False + elif isinstance(content, list): + for block in content: + if not isinstance(block, dict): + if block: # non-empty non-dict string etc. + return False + continue + btype = block.get("type") + if btype in ("thinking", "redacted_thinking"): + continue + if btype == "text": + text = block.get("text", "") + if isinstance(text, str) and text.strip(): + return False + continue + # tool_use, image, document, etc. — real payload + return False + elif content is not None and content != "": + return False + # Content is empty-ish. Is there reasoning to make it thinking-only? + reasoning = msg.get("reasoning_content") or msg.get("reasoning") + if isinstance(reasoning, str) and reasoning.strip(): + return True + # reasoning_details list form + rd = msg.get("reasoning_details") + if isinstance(rd, list) and rd: + return True + return False + + @staticmethod + def _drop_thinking_only_and_merge_users( + messages: List[Dict[str, Any]], + ) -> List[Dict[str, Any]]: + """Drop thinking-only assistant turns; merge any adjacent user messages left behind. + + Runs on the per-call ``api_messages`` copy only. The stored + conversation history (``self.messages``) is never mutated, so the + user still sees the thinking block in the CLI/gateway transcript and + session persistence keeps the full trace. Only the wire copy sent to + the provider is cleaned. + + Why drop-and-merge rather than inject stub text: + - Fabricating ``"."`` / ``"(continued)"`` text lies in the history + and makes future turns see model output the model didn't emit. + - Dropping the turn preserves honesty; merging adjacent user messages + preserves the provider's role-alternation invariant. + - This is the pattern used by Claude Code's ``normalizeMessagesForAPI`` + (filterOrphanedThinkingOnlyMessages + mergeAdjacentUserMessages). + """ + if not messages: + return messages + + # Pass 1: drop thinking-only assistant turns. + kept = [m for m in messages if not AIAgent._is_thinking_only_assistant(m)] + dropped = len(messages) - len(kept) + if dropped == 0: + return messages + + # Pass 2: merge any newly-adjacent user messages. + merged: List[Dict[str, Any]] = [] + merges = 0 + for m in kept: + prev = merged[-1] if merged else None + if ( + prev is not None + and prev.get("role") == "user" + and m.get("role") == "user" + ): + prev_content = prev.get("content", "") + cur_content = m.get("content", "") + # Work on a copy of ``prev`` so the caller's input dicts are + # never mutated. ``_sanitize_api_messages`` upstream already + # hands us per-call copies, but staying pure here means we + # can be called safely from anywhere (tests, other loops). + prev_copy = dict(prev) + # Only string-content merge is meaningful for role-alternation + # purposes. If either side is a list (multimodal), append as a + # separate block rather than collapsing. + if isinstance(prev_content, str) and isinstance(cur_content, str): + sep = "\n\n" if prev_content and cur_content else "" + prev_copy["content"] = prev_content + sep + cur_content + elif isinstance(prev_content, list) and isinstance(cur_content, list): + prev_copy["content"] = list(prev_content) + list(cur_content) + elif isinstance(prev_content, list) and isinstance(cur_content, str): + if cur_content: + prev_copy["content"] = list(prev_content) + [ + {"type": "text", "text": cur_content} + ] + else: + prev_copy["content"] = list(prev_content) + elif isinstance(prev_content, str) and isinstance(cur_content, list): + new_blocks: List[Dict[str, Any]] = [] + if prev_content: + new_blocks.append({"type": "text", "text": prev_content}) + new_blocks.extend(cur_content) + prev_copy["content"] = new_blocks + else: + # Unknown content shape — fall back to appending separately + # (violates alternation, but safer than raising in a hot path). + merged.append(m) + continue + merged[-1] = prev_copy + merges += 1 + else: + merged.append(m) + + logger.debug( + "Pre-call sanitizer: dropped %d thinking-only assistant turn(s), " + "merged %d adjacent user message(s)", + dropped, + merges, + ) + return merged + @staticmethod def _cap_delegate_task_calls(tool_calls: list) -> list: """Truncate excess delegate_task calls to max_concurrent_children. @@ -9498,6 +9637,10 @@ class AIAgent: for idx, pfm in enumerate(self.prefill_messages): api_messages.insert(sys_offset + idx, pfm.copy()) + # Same safety net as the main loop: drop thinking-only assistant + # turns so Anthropic-family providers don't 400 the summary call. + api_messages = self._drop_thinking_only_and_merge_users(api_messages) + summary_extra_body = {} try: from agent.auxiliary_client import _fixed_temperature_for_model, OMIT_TEMPERATURE as _OMIT_TEMP @@ -10208,6 +10351,16 @@ class AIAgent: # manual message manipulation are always caught. api_messages = self._sanitize_api_messages(api_messages) + # Drop thinking-only assistant turns (reasoning but no visible + # output and no tool_calls) and merge any adjacent user messages + # left behind. Prevents Anthropic 400s ("The final block in an + # assistant message cannot be `thinking`.") and equivalent errors + # from third-party Anthropic-compatible gateways that can't replay + # a thinking-only turn. Runs on the per-call copy only — the + # stored conversation history keeps the reasoning block for the + # UI transcript and session persistence. + api_messages = self._drop_thinking_only_and_merge_users(api_messages) + # Normalize message whitespace and tool-call JSON for consistent # prefix matching. Ensures bit-perfect prefixes across turns, # which enables KV cache reuse on local inference servers diff --git a/tests/run_agent/test_thinking_only_sanitizer.py b/tests/run_agent/test_thinking_only_sanitizer.py new file mode 100644 index 0000000000..83cf35f6d1 --- /dev/null +++ b/tests/run_agent/test_thinking_only_sanitizer.py @@ -0,0 +1,249 @@ +"""Tests for the thinking-only assistant message sanitizer. + +Covers _is_thinking_only_assistant() + _drop_thinking_only_and_merge_users() +in run_agent.py. The sanitizer runs on the per-call api_messages copy and +drops assistant turns that contain only reasoning (no visible content, no +tool_calls). Adjacent user messages left behind are merged so role +alternation is preserved for the provider. + +Claude Code uses this exact pattern (filterOrphanedThinkingOnlyMessages + +mergeAdjacentUserMessages in src/utils/messages.ts). See #16823 for the +backstory on why the alternative — fabricating "." stub text — was rejected. +""" + +from run_agent import AIAgent + + +# --------------------------------------------------------------------------- +# _is_thinking_only_assistant — detection +# --------------------------------------------------------------------------- + + +class TestIsThinkingOnlyAssistant: + + def test_plain_assistant_reply_is_not_thinking_only(self): + msg = {"role": "assistant", "content": "Hello there"} + assert not AIAgent._is_thinking_only_assistant(msg) + + def test_assistant_with_tool_calls_is_not_thinking_only(self): + msg = { + "role": "assistant", + "content": "", + "reasoning": "let me use a tool", + "tool_calls": [{"id": "c1", "function": {"name": "terminal", "arguments": "{}"}}], + } + assert not AIAgent._is_thinking_only_assistant(msg) + + def test_empty_content_plus_reasoning_is_thinking_only(self): + msg = {"role": "assistant", "content": "", "reasoning": "thinking..."} + assert AIAgent._is_thinking_only_assistant(msg) + + def test_none_content_plus_reasoning_content_is_thinking_only(self): + msg = {"role": "assistant", "content": None, "reasoning_content": "thinking..."} + assert AIAgent._is_thinking_only_assistant(msg) + + def test_whitespace_only_content_plus_reasoning_is_thinking_only(self): + msg = {"role": "assistant", "content": " \n\n ", "reasoning": "r"} + assert AIAgent._is_thinking_only_assistant(msg) + + def test_empty_content_no_reasoning_is_not_thinking_only(self): + # If there's no reasoning either, this is just an empty turn — let + # other sanitizers handle it (orphan-tool-pair, etc.). We only care + # about the specific thinking-only case. + msg = {"role": "assistant", "content": ""} + assert not AIAgent._is_thinking_only_assistant(msg) + + def test_list_content_all_thinking_blocks_is_thinking_only(self): + # Anthropic-native shape + msg = { + "role": "assistant", + "content": [ + {"type": "thinking", "thinking": "...", "signature": "sig"}, + ], + "reasoning": "...", + } + assert AIAgent._is_thinking_only_assistant(msg) + + def test_list_content_with_real_text_is_not_thinking_only(self): + msg = { + "role": "assistant", + "content": [ + {"type": "thinking", "thinking": "..."}, + {"type": "text", "text": "Hi there"}, + ], + "reasoning": "...", + } + assert not AIAgent._is_thinking_only_assistant(msg) + + def test_list_content_with_tool_use_block_is_not_thinking_only(self): + msg = { + "role": "assistant", + "content": [ + {"type": "thinking", "thinking": "..."}, + {"type": "tool_use", "id": "tu1", "name": "terminal", "input": {}}, + ], + } + assert not AIAgent._is_thinking_only_assistant(msg) + + def test_list_content_thinking_plus_whitespace_text_is_thinking_only(self): + msg = { + "role": "assistant", + "content": [ + {"type": "thinking", "thinking": "..."}, + {"type": "text", "text": " "}, + ], + "reasoning": "...", + } + assert AIAgent._is_thinking_only_assistant(msg) + + def test_reasoning_details_list_form_detected(self): + msg = { + "role": "assistant", + "content": "", + "reasoning_details": [{"type": "thinking", "text": "..."}], + } + assert AIAgent._is_thinking_only_assistant(msg) + + def test_user_message_never_thinking_only(self): + assert not AIAgent._is_thinking_only_assistant({"role": "user", "content": ""}) + + def test_tool_message_never_thinking_only(self): + assert not AIAgent._is_thinking_only_assistant( + {"role": "tool", "content": "", "tool_call_id": "x"} + ) + + def test_non_dict_returns_false(self): + assert not AIAgent._is_thinking_only_assistant(None) + assert not AIAgent._is_thinking_only_assistant("hello") + + +# --------------------------------------------------------------------------- +# _drop_thinking_only_and_merge_users — the full pass +# --------------------------------------------------------------------------- + + +class TestDropThinkingOnlyAndMergeUsers: + + def test_empty_list_passthrough(self): + assert AIAgent._drop_thinking_only_and_merge_users([]) == [] + + def test_no_thinking_only_messages_is_noop_identity(self): + msgs = [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "hello"}, + ] + out = AIAgent._drop_thinking_only_and_merge_users(msgs) + # Should return the original list untouched (identity) when no changes. + assert out is msgs + + def test_drops_thinking_only_between_user_messages_and_merges(self): + msgs = [ + {"role": "user", "content": "help me with X"}, + {"role": "assistant", "content": "", "reasoning": "let me think"}, + {"role": "user", "content": "ok continue"}, + ] + out = AIAgent._drop_thinking_only_and_merge_users(msgs) + assert len(out) == 1 + assert out[0]["role"] == "user" + assert out[0]["content"] == "help me with X\n\nok continue" + + def test_preserves_alternation_after_drop(self): + msgs = [ + {"role": "user", "content": "u1"}, + {"role": "assistant", "content": "", "reasoning": "..."}, + {"role": "user", "content": "u2"}, + {"role": "assistant", "content": "real reply"}, + ] + out = AIAgent._drop_thinking_only_and_merge_users(msgs) + roles = [m["role"] for m in out] + assert roles == ["user", "assistant"] + assert out[0]["content"] == "u1\n\nu2" + assert out[1]["content"] == "real reply" + + def test_does_not_merge_when_drop_leaves_non_adjacent_users(self): + # Thinking-only at end of conversation — no trailing user to merge + msgs = [ + {"role": "user", "content": "u1"}, + {"role": "assistant", "content": "reply"}, + {"role": "user", "content": "u2"}, + {"role": "assistant", "content": "", "reasoning": "..."}, + ] + out = AIAgent._drop_thinking_only_and_merge_users(msgs) + assert [m["role"] for m in out] == ["user", "assistant", "user"] + + def test_multiple_thinking_only_in_sequence_collapses(self): + msgs = [ + {"role": "user", "content": "u1"}, + {"role": "assistant", "content": "", "reasoning": "r1"}, + {"role": "assistant", "content": "", "reasoning": "r2"}, + {"role": "user", "content": "u2"}, + ] + out = AIAgent._drop_thinking_only_and_merge_users(msgs) + assert len(out) == 1 + assert out[0]["content"] == "u1\n\nu2" + + def test_does_not_touch_stored_messages_original_list_unmutated(self): + original_first_user = {"role": "user", "content": "u1"} + original_assistant = {"role": "assistant", "content": "", "reasoning": "..."} + original_second_user = {"role": "user", "content": "u2"} + msgs = [original_first_user, original_assistant, original_second_user] + AIAgent._drop_thinking_only_and_merge_users(msgs) + # Caller passes in a per-call copy already, but the sanitizer itself + # must not rewrite the dicts it was handed on the drop path. + # (It CAN mutate merged dicts — those come from the caller's copy.) + assert original_first_user["content"] == "u1" + assert original_second_user["content"] == "u2" + + def test_tool_result_between_user_and_thinking_preserved(self): + # Tool results shouldn't block a drop — but they do block the merge + # (user/tool are different roles). This scenario shouldn't happen in + # practice because a thinking-only turn won't have tool_calls, but if + # it did somehow, the surrounding tool result stays put. + msgs = [ + {"role": "user", "content": "u1"}, + {"role": "assistant", "tool_calls": [{"id": "c1", "function": {"name": "t", "arguments": "{}"}}]}, + {"role": "tool", "tool_call_id": "c1", "content": "ok"}, + {"role": "assistant", "content": "", "reasoning": "..."}, + {"role": "user", "content": "u2"}, + ] + out = AIAgent._drop_thinking_only_and_merge_users(msgs) + assert [m["role"] for m in out] == ["user", "assistant", "tool", "user"] + + def test_merge_concatenates_list_content_user_messages(self): + msgs = [ + {"role": "user", "content": [{"type": "text", "text": "first"}]}, + {"role": "assistant", "content": "", "reasoning": "..."}, + {"role": "user", "content": [{"type": "text", "text": "second"}]}, + ] + out = AIAgent._drop_thinking_only_and_merge_users(msgs) + assert len(out) == 1 + assert out[0]["content"] == [ + {"type": "text", "text": "first"}, + {"type": "text", "text": "second"}, + ] + + def test_merge_mixed_string_and_list_content(self): + msgs = [ + {"role": "user", "content": "plain text"}, + {"role": "assistant", "content": "", "reasoning": "..."}, + {"role": "user", "content": [{"type": "text", "text": "block text"}]}, + ] + out = AIAgent._drop_thinking_only_and_merge_users(msgs) + assert len(out) == 1 + assert out[0]["content"] == [ + {"type": "text", "text": "plain text"}, + {"type": "text", "text": "block text"}, + ] + + def test_system_messages_ignored_by_pass(self): + msgs = [ + {"role": "system", "content": "sys prompt"}, + {"role": "user", "content": "u1"}, + {"role": "assistant", "content": "", "reasoning": "..."}, + {"role": "user", "content": "u2"}, + ] + out = AIAgent._drop_thinking_only_and_merge_users(msgs) + assert len(out) == 2 + assert out[0]["role"] == "system" + assert out[1]["role"] == "user" + assert out[1]["content"] == "u1\n\nu2"