Skip to content

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

Closed
ygd58 wants to merge 1 commit into
NousResearch:mainfrom
ygd58:fix/anthropic-blank-text-blocks
Closed

fix(anthropic): filter blank text blocks in both normal and replay paths#63228
ygd58 wants to merge 1 commit into
NousResearch:mainfrom
ygd58:fix/anthropic-blank-text-blocks

Conversation

@ygd58

@ygd58 ygd58 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Problem

Bedrock and strict Anthropic-compatible endpoints reject text blocks where text is empty or whitespace-only with HTTP 400. The normal list-content path in _convert_assistant_message() extended blocks without filtering, and _sanitize_replay_block() returned blank text blocks on the ordered-replay fast path.

Fix

  1. Normal path: replace blocks.extend() with a filtering loop that drops any text block whose .strip() is empty.
  2. Replay path: _sanitize_replay_block() now returns None for empty/whitespace-only text blocks.

Both paths now share the same non-empty-text invariant.

Verification

4 regression tests in TestBlankTextBlockFiltering: normal path empty text filtered, whitespace-only filtered, non-empty preserved (no false positive), replay path empty text filtered. 177/177 tests pass (173 existing + 4 new).

Addresses maintainer review feedback on the earlier salvage PR.

@ygd58
fix(anthropic): filter blank text blocks in both normal and replay paths
1268bff 
Bedrock and strict Anthropic-compatible endpoints reject text blocks
where 'text' is empty or whitespace-only with HTTP 400. The normal
list-content path in _convert_assistant_message() extended blocks
without filtering, and _sanitize_replay_block() returned blank text
blocks on the ordered-replay fast path.

Fix:
1. Normal path: replace blocks.extend() with a filtering loop that
   drops any text block whose .strip() is empty, preserving tool_use
   and other block types.
2. Replay path: _sanitize_replay_block() now returns None for text
   blocks with empty or whitespace-only text, so the replay loop
   skips them via 'if clean is None: continue'.

Both paths now share the same non-empty-text invariant.

Added 4 regression tests in TestBlankTextBlockFiltering:
- normal path: empty text + tool_calls -> text dropped, tool kept
- normal path: whitespace-only text filtered
- normal path: non-empty text preserved (no false positive)
- replay path: empty text in anthropic_content_blocks dropped

177/177 tests pass (173 existing + 4 new).

Addresses maintainer review feedback on the earlier salvage PR.
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/anthropic Anthropic native Messages API provider/bedrock AWS Bedrock (boto3, IAM) P2 Medium — degraded but workaround exists labels Jul 12, 2026
@alt-glitch

alt-glitch commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Superset of / follow-up to #57985 (which fixes only the normal _convert_assistant_message path); this PR enforces the same non-empty-text invariant on the ordered-replay _sanitize_replay_block path as well. Related: #9491 (same root cause, different patch point). Competing cluster for the empty-text-block HTTP 400 family -- flagging for a maintainer to pick the canonical one.

@teknium1 teknium1 mentioned this pull request Jul 15, 2026
Open
teknium1
teknium1 reviewed Jul 15, 2026
View reviewed changes

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for covering both the normal and ordered-replay paths; the empty-block premise is still present on current main.

Problems

  • The normal-path condition calls .strip() on blk.get("text", ""), but _convert_content_part_to_anthropic() can preserve None from input text blocks (agent/anthropic_adapter.py:1733-1740). That would turn an invalid provider payload into an AttributeError.
  • The filter drops an attached cache marker. agent/prompt_caching.py:46-49 marks the final content part; dropping that blank part without relocating cache_control loses the breakpoint. The same concern applies when ordered replay drops a cached blank text block.
  • Scalar whitespace content remains unfiltered: agent/anthropic_adapter.py:1982-1988 accepts a truthy whitespace string and emits it as a text block.

Suggested changes

  • Apply a type-safe blank-text normalization across scalar, list, and replay paths, and add None/scalar-whitespace regressions.
  • Transfer a dropped block's cache marker to a surviving cacheable block or the non-empty fallback, with normal and replay cache tests.

Automated hermes-sweeper review.

Comment thread agent/anthropic_adapter.py
# "text" is empty or whitespace-only. The ordered-replay path
# enforces the same invariant via _sanitize_replay_block().
for blk in converted_content:
if (

@teknium1 teknium1 Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_convert_content_part_to_anthropic() preserves None when an input_text or stored text part explicitly has text: null (agent/anthropic_adapter.py:1733-1740). This .strip() will raise for that input; use a type-safe predicate and add a nullable-text regression.

Comment thread agent/anthropic_adapter.py
for blk in converted_content:
if (
isinstance(blk, dict)
and blk.get("type") == "text"

@teknium1 teknium1 Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A blank final content part can own the prompt-cache marker because agent/prompt_caching.py:46-49 annotates the last list item. Dropping it here loses that breakpoint; transfer its cache_control to the surviving cacheable block or fallback and cover the cached case.

@teknium1 teknium1 added sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@ygd58 ygd58 mentioned this pull request Jul 21, 2026
Closed
@ygd58

ygd58 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Ported forward in #68633: fixed the None-crash risk (type-safe strip), added cache_control relocation for a dropped blocks marker on both the normal and replay paths, and now also filters whitespace-only scalar content. Closing in favor of #68633.

@ygd58 ygd58 closed this Jul 21, 2026
teknium1 pushed a commit that referenced this pull request Jul 24, 2026
@ygd58 @teknium1
fix(anthropic): filter blank text blocks in both normal and replay paths
761c5db 
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.
teknium1 pushed a commit that referenced this pull request Jul 24, 2026
@ygd58 @teknium1
fix(anthropic): filter blank text blocks in both normal and replay paths
c55d780 
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.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
@ygd58 @teknium1
fix(anthropic): filter blank text blocks in both normal and replay paths
0ff2951 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teknium1 teknium1 teknium1 left review comments

Assignees

No one assigned

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/anthropic Anthropic native Messages API provider/bedrock AWS Bedrock (boto3, IAM) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ygd58 @alt-glitch @teknium1