fix(anthropic): filter blank text blocks in both normal and replay paths by ygd58 · Pull Request #68633 · NousResearch/hermes-agent

ygd58 · 2026-07-21T13:35:40Z

Context

Ports #63228 forward onto current main per @teknium1's review.

Problem

Bedrock and strict Anthropic-compatible endpoints reject text blocks where text is empty or whitespace-only with HTTP 400.

Fix

Per review, fixes three gaps in the original port:

Type safety: the filter used blk.get("text", "").strip(), which crashes with AttributeError when text is explicitly None (not absent). Now uses (blk.get("text") or "").strip() on both the normal and replay paths.
Cache marker loss: prompt_caching.py's _apply_cache_marker() sets cache_control directly on content[-1]. If that last part is blank text, dropping it without relocating cache_control silently loses the breakpoint. Both paths now capture a dropped block's cache_control and reapply it to the new last surviving cacheable block.
Scalar whitespace: the non-list content branch accepted a truthy whitespace-only string unfiltered. Now filtered the same way.

Verification

8/8 new tests pass (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.

Ports NousResearch#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.

egilewski · 2026-07-22T16:26:16Z

suggesting changes

The normal-path filtering is bypassed when it removes every block. After blocks becomes empty, effective = blocks or content restores the original content, so a sole whitespace text block, a sole cache-marked blank block, and standalone whitespace scalar content still leave convert_messages_to_anthropic unchanged and provider-invalid. The new tests all leave a tool or nonblank block behind, so they miss this path. Please base the empty fallback on the sanitized result and add standalone list/scalar cases with no surviving block.
The normal-path predicate is still not type-safe for truthy non-string text: {"type": "text", "text": 7} raises AttributeError at (blk.get("text") or "").strip(), while the replay sanitizer correctly drops all non-string text values. Please apply the same string-type guard on the normal path and cover it with a regression test.

Security evidence:

trust boundary: Assistant message content is converted into requests for Anthropic, Bedrock, or compatible provider APIs.
source/sink/invariant: messages[*].content enters the adapter; the returned provider payload must contain only non-empty string text blocks while preserving cache placement safely.
current-main reproduction: Standalone blank list, whitespace scalar, and cache-marked blank content remain provider-invalid on current main.
PR-head or patch-replay validation: The exact PR head and a clean replay onto current main fix mixed blank-plus-survivor cases, but both restore all-filtered content and raise on truthy non-string text.
positive/negative cases: Non-empty text and tool blocks survive, mixed blank blocks are dropped, and replay-only blank content falls back safely; normal-only blank/scalar and non-string cases fail.
residual bypass search: Normal lists and scalars, ordered replay, cache-marked blanks, all-filtered fallback, None, non-string text, and surviving tool/text blocks were exercised.
reviewer validation: The focused adapter suite passed on both trees, and independent public-converter probes reproduced both failures on the exact PR head and current-main replay.

Signed: GPT-5.6-sol-xhigh in Codex

Follow-up per independent review of NousResearch#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).

ygd58 · 2026-07-23T05:57:11Z

Thanks for the thorough review -- both were real bugs. Fixed in the latest commit:

effective = blocks or content fell back to the raw, unfiltered content whenever every block was filtered as blank -- exactly the case where the entire message content WAS the blank payload being filtered. Now effective = blocks if blocks else [{"type": "text", "text": "(empty)"}], never touching raw content. Also moved cache_control application to run against effective (post-fallback) so a marker on a sole dropped block still lands on the placeholder.
(blk.get("text") or "").strip() was not type-safe for a truthy non-string value (e.g. an int) -- switched to an isinstance(text, str) check first, matching the already-correct replay-path sanitizer.

Added 7 regression tests covering both (sole blank list/scalar, sole cache-marked blank block placeholder-relocation, non-string int/dict text values standalone and mixed with a surviving tool_use). 193/193 pass in the full adapter test file; 23/23 in prompt_caching (confirmed unaffected).

egilewski · 2026-07-23T20:11:40Z

suggesting changes

The normal-path all-filtered fallback and truthy non-string crash are fixed, but the equivalent all-filtered cache-marker case is still broken in ordered replay. _relocated_replay_cache_control is applied only inside if replayed:. For anthropic_content_blocks containing only a blank cache-marked text block, replayed becomes empty, the function falls through to the (empty) fallback, and the marker is lost. A signed-thinking block plus the blank marker also returns without any relocated marker. Please preserve the marker when no cacheable replay block survives (for example by resolving a cacheable placeholder within the replay branch) and add a sole cache-marked blank replay regression test.

Security evidence:

trust boundary: Stored assistant content is replayed into requests sent to Anthropic, Bedrock, or strict Anthropic-compatible provider APIs.
source/sink/invariant: anthropic_content_blocks enters _convert_assistant_message; blank text must be removed while a cache breakpoint on a removed block is preserved.
current-main reproduction: A sole blank cache-marked replay block is emitted unchanged as provider-invalid blank text on current main.
PR-head or patch-replay validation: The exact PR head replaces that blank block with (empty) but drops its cache_control; normal-path sole-blank relocation succeeds.
positive/negative cases: All 193 adapter tests pass, including replay relocation when a tool block survives; an exact sole-block replay probe loses the marker.
residual bypass search: Normal and replay paths, sole and mixed blanks, surviving tool blocks, signed thinking, marker placement, whitespace, None, and truthy non-string text were checked.
reviewer validation: _relocated_replay_cache_control is consumed only under if replayed:, and an independent exact-head probe reproduced the loss.

Signed: GPT-5.6-sol-xhigh in Codex

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>

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>

teknium1 · 2026-07-24T23:07:53Z

Merged via #70991 with your authorship preserved on both commits — blank-block filtering in both paths plus the no-raw-fallback guard, rebased over the branch conflict. Thanks.

Follow-up per independent review of NousResearch#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 NousResearch#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 NousResearch#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 NousResearch#69512 Co-authored-by: ygd58 <buraysandro9@gmail.com>

ygd58 mentioned this pull request Jul 21, 2026

fix(anthropic): filter blank text blocks in both normal and replay paths #63228

Closed

SHL0MS mentioned this pull request Jul 23, 2026

fix(anthropic): sanitize empty/whitespace-only text blocks to prevent permanent HTTP 400 after compression #69512

Closed

teknium1 mentioned this pull request Jul 24, 2026

fix(anthropic): filter blank text blocks in both normal and replay paths #70991

Merged

teknium1 closed this Jul 24, 2026

Navigation Menu

Uh oh!

fix(anthropic): filter blank text blocks in both normal and replay paths - #68633

fix(anthropic): filter blank text blocks in both normal and replay paths#68633
ygd58 wants to merge 2 commits into
NousResearch:mainfrom
ygd58:fix/anthropic-blank-text-blocks-v2

ygd58 commented Jul 21, 2026

Uh oh!

egilewski commented Jul 22, 2026

Uh oh!

ygd58 commented Jul 23, 2026

Uh oh!

egilewski commented Jul 23, 2026

Uh oh!

teknium1 commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

ygd58 commented Jul 21, 2026

Context

Problem

Fix

Verification

Uh oh!

egilewski commented Jul 22, 2026

Uh oh!

ygd58 commented Jul 23, 2026

Uh oh!

egilewski commented Jul 23, 2026

Uh oh!

teknium1 commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants