mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-25 17:18:11 +00:00
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.
This commit is contained in:
ygd58
Teknium
parent
6bc8d68ad7
commit
c55d780d48
2 changed files with 239 additions and 6 deletions
|
|
@ -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")
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue