fix(anthropic): filter blank text blocks in both normal and replay paths - #70991
Merged
Merged
Conversation
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.
Follow-up per independent review of #68633 (GPT-5.6-sol-xhigh in Codex, reviewer egilewski) on this PR. Two real bugs in the blank-text-block filtering added by that fix: 1. `effective = blocks or content` fell back to the RAW, unfiltered `content` variable whenever every block was filtered out as blank -- which happens precisely when the entire message content WAS the blank/whitespace payload the filter exists to remove (a sole blank text block, a sole cache-marked blank block, or standalone whitespace scalar content with no tool_calls). The fallback silently restored the exact invalid content the filtering just stripped, leaving the message provider-invalid. Fixed: `effective = blocks if blocks else [{"type": "text", "text": "(empty)"}]` -- never falls back to raw `content`. Also moved the cache_control application (both the relocated-from-a-dropped-block marker and the message-level marker) to run against `effective` instead of the pre-fallback `blocks`, so a cache marker on a block that was the ONLY content still lands on the (empty) placeholder rather than being silently lost when `blocks` was empty at the point it would otherwise have been applied. 2. The normal-path blank-text check used `(blk.get("text") or "").strip()`, which is not type-safe for a truthy NON-string, non-None text value (e.g. an int or dict from an invalid upstream payload) -- `or` doesn't substitute for a truthy value, so `(7 or "").strip()` still raises AttributeError. Now checks `isinstance(text, str)` first, matching the replay path's `_sanitize_replay_block()`, which the reviewer confirmed was already correctly type-safe. Added regression tests for: sole blank list block, sole whitespace scalar content, sole cache-marked blank block (marker relocation to the placeholder), a truthy non-string (int) text value both mixed with a surviving tool_use and as the sole content, and a dict-valued text field. 7/7 new tests pass; 193/193 in the full tests/agent/test_anthropic_adapter.py file; 23/23 in tests/agent/test_prompt_caching.py (unaffected, confirmed).
…blank Follow-up to the cherry-picked #68633 commits, closing the final open review point (egilewski): _relocated_replay_cache_control was applied only inside `if replayed:`. When anthropic_content_blocks contained only a blank cache-marked text block, `replayed` came out empty, the function fell through to the main path's placeholder, and the cache marker was lost; signed thinking + a blank marked text block likewise returned with no cacheable carrier for the relocated marker. The replay branch now appends the non-whitespace "(empty)" placeholder when no cacheable (text/tool_use) block survives the blank filter and a blank text block was dropped (or a marker needs a carrier) — so replay stays schema-valid on Bedrock/strict endpoints and the breakpoint survives on the placeholder. Also reconciles the block-level tests from #69517 with the new drop-then-fallback contract (blank blocks are dropped at the block level; the message-level result is still always non-blank). Refs #69512 Co-authored-by: ygd58 <buraysandro9@gmail.com>
alt-glitch
added
type/bug
Contributor
૮ >ﻌ< ა ci reviewran on 503ee13 all good! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Anthropic requests no longer 400 on blank text blocks — they are filtered in both the normal and replay paths, without falling back to raw content when all blocks are filtered.
Changes
Validation
Targeted adapter suites green.
Infographic