From c55d780d48e5d741ad9cfcbec0e3aa5bd365f71e Mon Sep 17 00:00:00 2001 From: ygd58 Date: Tue, 21 Jul 2026 13:35:18 +0000 Subject: [PATCH] fix(anthropic): filter blank text blocks in both normal and replay paths Ports #63228 forward onto current main per teknium1's review. Bedrock and strict Anthropic-compatible endpoints reject text blocks where text is empty or whitespace-only with HTTP 400. The normal list-content path extended blocks without filtering, and the ordered-replay fast path (_sanitize_replay_block) returned blank text blocks unfiltered. Per review, fixes three gaps in the original port: 1. Type safety: the normal-path filter used blk.get('text', '').strip(), which crashes with AttributeError when text is explicitly None (not absent) -- .get()'s default only applies when the key is missing. _convert_content_part_to_anthropic() can preserve None from an invalid upstream input text block. Now uses (blk.get('text') or '').strip() on both paths. 2. Cache marker loss: prompt_caching.py's _apply_cache_marker() sets cache_control directly on content[-1] for list content. If that last part happens to be blank text, dropping it without relocating cache_control silently loses the breakpoint. Both the normal and replay paths now capture a dropped block's cache_control and reapply it to the new last surviving cacheable block via the existing _apply_assistant_cache_control_to_last_cacheable_block() helper (setdefault semantics, so it never clobbers a legitimately-placed marker). 3. Scalar whitespace: the non-list content branch (blocks.append({'type': 'text', 'text': str(content)})) accepted a truthy whitespace-only string unfiltered. Now filtered the same way as list-content blocks. 8/8 new tests pass in TestBlankTextBlockFiltering (including None-safety, scalar-whitespace, and cache_control-relocation regressions on both paths); 186/186 in the full tests/agent/test_anthropic_adapter.py file. --- agent/anthropic_adapter.py | 61 ++++++++- tests/agent/test_anthropic_adapter.py | 184 ++++++++++++++++++++++++++ 2 files changed, 239 insertions(+), 6 deletions(-) diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 6d6be638fc2d..9341f842f911 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -1920,10 +1920,18 @@ def _sanitize_replay_block(b: Dict[str, Any]) -> Optional[Dict[str, Any]]: return None btype = b.get("type") if btype == "text": - # Coerce empty/whitespace-only text to a non-whitespace placeholder; - # the Messages input schema rejects blank text blocks (#69512), and a - # blank block stored in history replays on every turn → permanent 400. - out: Dict[str, Any] = {"type": "text", "text": _safe_text(b.get("text", ""))} + text_val = b.get("text", "") + # Bedrock and strict Anthropic-compatible endpoints reject text + # blocks where "text" is empty or whitespace-only (#69512). Drop the + # blank block (the caller relocates any cache_control it carried and + # falls back to a non-whitespace placeholder when nothing survives) + # rather than coercing in place — a coerced "(empty)" block would be + # model-visible noise next to surviving thinking/tool_use blocks. + # Type-safe: captured blocks can carry text=None from an invalid + # upstream payload, which a bare .strip() would crash on. + if not isinstance(text_val, str) or not text_val.strip(): + return None + out: Dict[str, Any] = {"type": "text", "text": text_val} # citations is input-valid ONLY when it's a non-empty list; the SDK # emits citations=None on responses, which the input schema rejects. cits = b.get("citations") @@ -2011,9 +2019,14 @@ def _convert_assistant_message(m: Dict[str, Any]) -> Dict[str, Any]: parsed_args = {} redacted_input_by_id[_sanitize_tool_id(tc.get("id", ""))] = parsed_args replayed: List[Dict[str, Any]] = [] + _relocated_replay_cache_control = None for b in ordered_blocks: clean = _sanitize_replay_block(b) if clean is None: + if isinstance(b, dict) and isinstance(b.get("cache_control"), dict): + # A dropped blank text block can still carry the cache + # breakpoint marker -- relocate it rather than losing it. + _relocated_replay_cache_control = b["cache_control"] continue if clean.get("type") == "tool_use": # Override raw (un-redacted) input with the redacted copy when @@ -2024,19 +2037,51 @@ def _convert_assistant_message(m: Dict[str, Any]) -> Dict[str, Any]: clean["input"] = redacted replayed.append(clean) if replayed: + if _relocated_replay_cache_control is not None: + _apply_assistant_cache_control_to_last_cacheable_block( + replayed, _relocated_replay_cache_control + ) _apply_assistant_cache_control_to_last_cacheable_block( replayed, m.get("cache_control") ) return {"role": "assistant", "content": replayed} blocks = _extract_preserved_thinking_blocks(m) + # Cache markers dropped along with a blank block are relocated onto the + # last surviving cacheable block below (via + # _apply_assistant_cache_control_to_last_cacheable_block), rather than + # lost -- prompt_caching.py's _apply_cache_marker() sets cache_control + # directly on content[-1] for list content, so if that last part happens + # to be blank text, dropping it silently would lose the breakpoint. + _relocated_cache_control = None if content: if isinstance(content, list): converted_content = _convert_content_to_anthropic(content) if isinstance(converted_content, list): - blocks.extend(converted_content) + # Bedrock and strict Anthropic-compatible endpoints reject + # text blocks where "text" is empty or whitespace-only. The + # ordered-replay path enforces the same invariant via + # _sanitize_replay_block(). Type-safe: input text blocks can + # carry text=None (_convert_content_part_to_anthropic + # preserves it from an invalid upstream payload), which a + # bare .strip() would crash on. + for blk in converted_content: + if ( + isinstance(blk, dict) + and blk.get("type") == "text" + and not (blk.get("text") or "").strip() + ): + if isinstance(blk.get("cache_control"), dict): + _relocated_cache_control = blk["cache_control"] + continue + blocks.append(blk) else: - blocks.append({"type": "text", "text": str(content)}) + # Scalar (non-list) content: a whitespace-only string is the + # same invalid-payload case as an empty list block -- drop it + # rather than emitting a blank text block. + text_str = str(content) + if text_str.strip(): + blocks.append({"type": "text", "text": text_str}) for tc in m.get("tool_calls", []): if not tc or not isinstance(tc, dict): continue @@ -2052,6 +2097,10 @@ def _convert_assistant_message(m: Dict[str, Any]) -> Dict[str, Any]: "name": fn.get("name", ""), "input": parsed_args, }) + if _relocated_cache_control is not None: + _apply_assistant_cache_control_to_last_cacheable_block( + blocks, _relocated_cache_control + ) _apply_assistant_cache_control_to_last_cacheable_block( blocks, m.get("cache_control") ) diff --git a/tests/agent/test_anthropic_adapter.py b/tests/agent/test_anthropic_adapter.py index ae99f950f739..405e8804f70c 100644 --- a/tests/agent/test_anthropic_adapter.py +++ b/tests/agent/test_anthropic_adapter.py @@ -2634,3 +2634,187 @@ class TestConvertToolsToAnthropicDedup: def test_none_tools_returns_empty(self): assert convert_tools_to_anthropic(None) == [] + + +class TestBlankTextBlockFiltering: + """Regression tests for blank text block filtering in _convert_assistant_message. + + Bedrock and strict Anthropic-compatible endpoints reject text blocks where + "text" is empty or whitespace-only with HTTP 400. Both the normal list- + content path and the ordered-replay fast path must drop such blocks while + preserving tool_use and other block types, and must relocate (not lose) + any cache_control marker attached to the dropped block. + """ + + def _convert(self, message): + from agent.anthropic_adapter import _convert_assistant_message + return _convert_assistant_message(message) + + def test_normal_path_filters_empty_text_block_alongside_tool_calls(self): + """Content list with empty text + tool_calls: empty text must be dropped.""" + msg = { + "role": "assistant", + "content": [ + {"type": "text", "text": ""}, + {"type": "tool_use", "id": "call_1", "name": "web_search", + "input": {"query": "test"}}, + ], + } + result = self._convert(msg) + blocks = result["content"] + text_blocks = [b for b in blocks if b.get("type") == "text"] + tool_blocks = [b for b in blocks if b.get("type") == "tool_use"] + assert len(text_blocks) == 0, f"Empty text block not filtered: {text_blocks}" + assert len(tool_blocks) == 1 + + def test_normal_path_filters_whitespace_only_text_block(self): + """Whitespace-only text (spaces, newlines) must also be filtered.""" + msg = { + "role": "assistant", + "content": [ + {"type": "text", "text": " \n "}, + {"type": "tool_use", "id": "call_2", "name": "terminal", + "input": {"command": "ls"}}, + ], + } + result = self._convert(msg) + blocks = result["content"] + text_blocks = [b for b in blocks if b.get("type") == "text"] + assert len(text_blocks) == 0, f"Whitespace text block not filtered: {text_blocks}" + + def test_normal_path_filters_none_text_block_without_crashing(self): + """Regression (review of #63228): text=None must not raise + AttributeError. _convert_content_part_to_anthropic() can preserve + None from an invalid upstream input text block -- a bare .strip() + on blk.get("text", "") crashes because .get() only substitutes the + default when the key is ABSENT, not when it's present with value None.""" + msg = { + "role": "assistant", + "content": [ + {"type": "text", "text": None}, + {"type": "tool_use", "id": "call_none", "name": "web_search", + "input": {"query": "test"}}, + ], + } + result = self._convert(msg) # must not raise + blocks = result["content"] + text_blocks = [b for b in blocks if b.get("type") == "text"] + tool_blocks = [b for b in blocks if b.get("type") == "tool_use"] + assert len(text_blocks) == 0, f"None text block not filtered: {text_blocks}" + assert len(tool_blocks) == 1 + + def test_normal_path_preserves_non_empty_text_block(self): + """Non-empty text blocks must NOT be filtered.""" + msg = { + "role": "assistant", + "content": [ + {"type": "text", "text": "I will search for that."}, + {"type": "tool_use", "id": "call_3", "name": "web_search", + "input": {"query": "test"}}, + ], + } + result = self._convert(msg) + blocks = result["content"] + text_blocks = [b for b in blocks if b.get("type") == "text"] + assert len(text_blocks) == 1 + assert text_blocks[0]["text"] == "I will search for that." + + def test_normal_path_filters_whitespace_only_scalar_content(self): + """Regression (review of #63228): a truthy whitespace-only scalar + content string must also be filtered, not just list-content blocks.""" + msg = { + "role": "assistant", + "content": " \n\t ", + "tool_calls": [ + {"id": "call_scalar", "function": {"name": "web_search", + "arguments": '{"query": "test"}'}}, + ], + } + result = self._convert(msg) + blocks = result["content"] + text_blocks = [b for b in blocks if b.get("type") == "text"] + tool_blocks = [b for b in blocks if b.get("type") == "tool_use"] + assert len(text_blocks) == 0, f"Whitespace scalar content not filtered: {text_blocks}" + assert len(tool_blocks) == 1 + + def test_normal_path_relocates_cache_control_from_dropped_block(self): + """Regression (review of #63228): prompt_caching.py's _apply_cache_marker + sets cache_control directly on content[-1] for list content. If that + last part is blank text, dropping it must relocate the marker to the + surviving last cacheable block (here: the tool_use), not lose it.""" + msg = { + "role": "assistant", + "content": [ + {"type": "text", "text": "I'll look that up."}, + {"type": "tool_use", "id": "call_cache", "name": "web_search", + "input": {"query": "test"}}, + {"type": "text", "text": "", "cache_control": {"type": "ephemeral"}}, + ], + } + result = self._convert(msg) + blocks = result["content"] + assert not any(b.get("type") == "text" and not b.get("text", "").strip() for b in blocks), ( + "Blank text block must be dropped" + ) + cacheable_with_marker = [b for b in blocks if isinstance(b.get("cache_control"), dict)] + assert len(cacheable_with_marker) == 1, ( + f"cache_control marker must survive on exactly one surviving block: {blocks}" + ) + assert cacheable_with_marker[0]["type"] == "tool_use", ( + f"Marker must relocate to the new last cacheable block: {blocks}" + ) + + def test_replay_path_filters_empty_text_block(self): + """Ordered-replay path (anthropic_content_blocks) must also drop blank text.""" + from agent.anthropic_adapter import _convert_assistant_message + msg = { + "role": "assistant", + "content": "", + "anthropic_content_blocks": [ + {"type": "text", "text": ""}, + {"type": "tool_use", "id": "call_4", "name": "web_search", + "input": {"query": "test"}}, + ], + "tool_calls": [ + { + "id": "call_4", + "function": {"name": "web_search", + "arguments": '{"query": "test"}'}, + } + ], + } + result = _convert_assistant_message(msg) + blocks = result["content"] + text_blocks = [b for b in blocks if b.get("type") == "text"] + tool_blocks = [b for b in blocks if b.get("type") == "tool_use"] + assert len(text_blocks) == 0, f"Empty text in replay not filtered: {text_blocks}" + assert len(tool_blocks) == 1 + + def test_replay_path_relocates_cache_control_from_dropped_block(self): + """Same cache_control-relocation guarantee on the ordered-replay path: + a blank text block carrying cache_control (e.g. a stored, previously + cache-marked turn where prompt_caching later becomes blank on replay) + must not silently lose the breakpoint when dropped.""" + from agent.anthropic_adapter import _convert_assistant_message + msg = { + "role": "assistant", + "content": "", + "anthropic_content_blocks": [ + {"type": "tool_use", "id": "call_5", "name": "web_search", + "input": {"query": "test"}}, + {"type": "text", "text": " ", "cache_control": {"type": "ephemeral"}}, + ], + "tool_calls": [ + { + "id": "call_5", + "function": {"name": "web_search", + "arguments": '{"query": "test"}'}, + } + ], + } + result = _convert_assistant_message(msg) + blocks = result["content"] + assert not any(b.get("type") == "text" for b in blocks), "Blank replay text must be dropped" + cacheable_with_marker = [b for b in blocks if isinstance(b.get("cache_control"), dict)] + assert len(cacheable_with_marker) == 1 + assert cacheable_with_marker[0]["type"] == "tool_use"