From 7f22a34eb2e8f3cae2004a7fbde238ec88ef945a Mon Sep 17 00:00:00 2001 From: Eli Date: Tue, 1 Sep 2026 23:34:34 -0400 Subject: [PATCH 1/6] Never construct an empty content block (#762) Anthropic rejects an empty text block ("messages: text content blocks must be non-empty") and rejects it again when a cache breakpoint lands on it ("cache_control cannot be set for empty text blocks" -- the error #762 reports). An ordinary `Skill` call reached both. `to_content_blocks` emitted a block for the empty string, so a tool returning `""` encoded to a single empty block -- and, being the last block of the last input message, to the one place `_add_cache_control` puts its second breakpoint. The trigger in a long-running session is `exec_code`, which returns the empty string for a snippet that printed nothing and is in the default `harness()` stack. `to_content_blocks` no longer builds a block with nothing in it. The linearization law is unaffected: it is stated for non-string encoded values, and an empty string nested inside one still renders as `""`. `HistoryBuilder.append_message` asserts the invariant, and `_mark` declines a message with no non-empty block, letting the breakpoint fall back to an earlier message instead of being dropped. Anthropic accepts a tool_result whose content list is empty, confirmed live. The assertion in `call_assistant` now also rejects a whitespace-only reply and reports the `finish_reason`, which is the only thing that distinguishes a truncation from a filtered response or a broken proxy. Two claims were checked against the live API rather than assumed, and one was wrong. Anthropic *accepts* consecutive same-role turns, so a dropped assistant turn is a transcript-integrity bug, not a 400; the `_assert_valid_anthropic_request` oracle therefore asserts that no turn is dropped rather than that roles alternate, which would have pinned provider behaviour that does not exist. The suite could not have caught this: every offline test asserts on the OpenAI-shaped list reaching `completion`, and the live tests run against `EFFECTFUL_LLM_MODEL` -- an OpenAI model by default, where litellm strips `cache_control` and empty text is tolerated. The new oracle runs litellm's Anthropic transform offline and is applied to the existing caching tests too. Co-Authored-By: Claude Opus 5 (1M context) --- .../llm/harness/durability/transaction.py | 27 +- effectful/handlers/llm/harness/hooks.py | 5 +- .../handlers/llm/harness/provision/litellm.py | 65 ++-- .../handlers/llm/harness/serialization.py | 23 +- tests/test_handlers_llm_harness_provision.py | 304 +++++++++++++++++- 5 files changed, 394 insertions(+), 30 deletions(-) diff --git a/effectful/handlers/llm/harness/durability/transaction.py b/effectful/handlers/llm/harness/durability/transaction.py index 067fc19e2..315dfa1e9 100644 --- a/effectful/handlers/llm/harness/durability/transaction.py +++ b/effectful/handlers/llm/harness/durability/transaction.py @@ -13,7 +13,10 @@ call_tool, call_user, ) -from effectful.handlers.llm.harness.serialization import ToolCallID +from effectful.handlers.llm.harness.serialization import ( + ToolCallID, + _is_empty_text_block, +) from effectful.ops.semantics import fwd, handler from effectful.ops.syntax import ObjectInterpretation, implements from effectful.ops.types import Operation @@ -36,12 +39,18 @@ def get_history(cls) -> collections.abc.MutableSequence[Message]: def append_message(cls, message: Message) -> None: """Append `message` to the ambient history, if it is legal where it lands. - Both checks are about position rather than content, which is why they sit - here: every message the harness records passes through this method, - including the ones a failed attempt records on its way out, and those are - the ones that get a history into a shape no provider will accept. + Two of the checks are about position rather than content, which is why + they sit here: every message the harness records passes through this + method, including the ones a failed attempt records on its way out, and + those are the ones that get a history into a shape no provider will + accept. The third holds by construction -- `to_content_blocks` and + `_render_prompt_section` are the only sources of blocks and neither + builds an empty one -- so a violation is a bug in a producer. """ history = cls.get_history() + assert cls._carries_no_empty_block(message), ( + f"a message may not carry an empty text block: {message}" + ) if message["role"] == "tool": assert cls._tool_call_answers_request(message, history) elif message["role"] == "assistant": @@ -51,6 +60,14 @@ def append_message(cls, message: Message) -> None: ) history.append(message) + @staticmethod + def _carries_no_empty_block(message: Message) -> bool: + """Whether `message` is free of the empty text blocks Anthropic rejects.""" + content = message.get("content") + return not isinstance(content, list) or not any( + _is_empty_text_block(block) for block in content + ) + @staticmethod def _assistant_speaks_in_turn( history: collections.abc.Sequence[Message], diff --git a/effectful/handlers/llm/harness/hooks.py b/effectful/handlers/llm/harness/hooks.py index d45441976..b30e78236 100644 --- a/effectful/handlers/llm/harness/hooks.py +++ b/effectful/handlers/llm/harness/hooks.py @@ -267,7 +267,10 @@ def call_assistant[T]( result = None if not tool_calls: serialized_result = message.get("content") or message.get("reasoning_content") - assert isinstance(serialized_result, str) + assert isinstance(serialized_result, str) and serialized_result.strip(), ( + f"the model replied with neither content nor a tool call " + f"(finish_reason={choice.finish_reason!r})" + ) try: # A text answer is the model's own prose, and anything else is JSON # shaped like the response format. Both are boxed and validated diff --git a/effectful/handlers/llm/harness/provision/litellm.py b/effectful/handlers/llm/harness/provision/litellm.py index 05e1aa203..cf68d0998 100644 --- a/effectful/handlers/llm/harness/provision/litellm.py +++ b/effectful/handlers/llm/harness/provision/litellm.py @@ -11,6 +11,7 @@ ResultDecodingError, completion, ) +from effectful.handlers.llm.harness.serialization import _is_empty_text_block from effectful.ops.semantics import fwd from effectful.ops.syntax import ObjectInterpretation, implements @@ -28,32 +29,47 @@ def __init__(self, model="gpt-4o", **config): } @staticmethod - def _mark(msg: Message) -> Message: - """`msg`, with a `cache_control` breakpoint on its last content block. + def _mark(msg: Message) -> Message | None: + """`msg`, with a `cache_control` breakpoint on its last non-empty content + block -- or `None` if it has none. A message whose content is a plain string -- which the assembled system prompt is not, but a hand-written or externally supplied message may be -- takes the message-level key instead, the only form litellm reads a breakpoint from for string content. + + Empty blocks are skipped because Anthropic answers ``cache_control + cannot be set for empty text blocks``. `to_content_blocks` no longer + builds one, so this is reached only by messages supplied from outside -- + the same messages the string case above exists for. `None` lets + `_add_cache_control` fall back to an earlier message; a message that is + already marked returns itself, since that mark is the breakpoint. """ content = msg.get("content") if isinstance(content, str): + if not content.strip(): + return None return typing.cast(Message, {**msg, "cache_control": {"type": "ephemeral"}}) - if not isinstance(content, list) or not content: - return msg - last_block = content[-1] - if not isinstance(last_block, dict) or "cache_control" in last_block: - return msg - return typing.cast( - Message, - { - **msg, - "content": [ - *content[:-1], - {**last_block, "cache_control": {"type": "ephemeral"}}, - ], - }, - ) + if not isinstance(content, list): + return None + for i in reversed(range(len(content))): + block = content[i] + if not isinstance(block, dict) or _is_empty_text_block(block): + continue + if "cache_control" in block: + return msg + return typing.cast( + Message, + { + **msg, + "content": [ + *content[:i], + {**block, "cache_control": {"type": "ephemeral"}}, + *content[i + 1 :], + ], + }, + ) + return None def _add_cache_control( self, @@ -81,15 +97,26 @@ def _add_cache_control( Returns a new list, leaving the caller's messages untouched, so these transport-level annotations never reach the stored history -- and so never reach an `Agent`'s checkpointed transcript. + + A message `_mark` declines is stepped over: the scan keeps walking back + until one takes the breakpoint, so a request whose newest message has + nothing markable in it caches a shorter prefix rather than going out + with one breakpoint. """ out = list(messages) for i in reversed(range(len(out))): if out[i]["role"] in ("user", "tool"): - out[i] = self._mark(out[i]) + marked = self._mark(out[i]) + if marked is None: + continue + out[i] = marked break for i in range(len(out)): if out[i]["role"] == "system": - out[i] = self._mark(out[i]) + # Only one system message is ever sent: stop either way. + marked = self._mark(out[i]) + if marked is not None: + out[i] = marked break return out diff --git a/effectful/handlers/llm/harness/serialization.py b/effectful/handlers/llm/harness/serialization.py index a1a229800..6e041718b 100644 --- a/effectful/handlers/llm/harness/serialization.py +++ b/effectful/handlers/llm/harness/serialization.py @@ -73,17 +73,20 @@ def to_content_blocks( Inside JSON structures, separators match ``json.dumps`` defaults so that the linearization law holds for non-string encoded values: ``linearize(to_content_blocks(v)) == json.dumps(v)``. + + No block is empty: Anthropic rejects an empty text block outright, and + rejects it again when a cache breakpoint lands on one. """ if isinstance(value, str): - return [ChatCompletionTextObject(type="text", text=value)] + return [ChatCompletionTextObject(type="text", text=value)] if value else [] buf: list[str] = [] blocks: list[OpenAIMessageContentListBlock] = [] def flush() -> None: - if buf: - blocks.append(ChatCompletionTextObject(type="text", text="".join(buf))) - buf.clear() + if text := "".join(buf): + blocks.append(ChatCompletionTextObject(type="text", text=text)) + buf.clear() def walk(v: typing.Any) -> None: if isinstance(v, dict) and v.get("type") in CONTENT_BLOCK_TYPES: @@ -112,6 +115,18 @@ def walk(v: typing.Any) -> None: return blocks +def _is_empty_text_block(block: typing.Any) -> bool: + """Whether `block` is a text block Anthropic will reject as empty. + + Whitespace-only counts, matching litellm's own ``.strip()`` test. + """ + return ( + isinstance(block, dict) + and block.get("type") == "text" + and not (block.get("text") or "").strip() + ) + + def format_as_content_blocks( template: str, env: collections.abc.Mapping[str, typing.Any], diff --git a/tests/test_handlers_llm_harness_provision.py b/tests/test_handlers_llm_harness_provision.py index f10ef9494..a108a03a8 100644 --- a/tests/test_handlers_llm_harness_provision.py +++ b/tests/test_handlers_llm_harness_provision.py @@ -51,7 +51,7 @@ ) from effectful.handlers.llm.harness.observability.rich import RichTerminalRenderer from effectful.handlers.llm.harness.provision.litellm import LiteLLMConfigurer -from effectful.handlers.llm.harness.serialization import _NameAndTool +from effectful.handlers.llm.harness.serialization import _NameAndTool, to_content_blocks from effectful.handlers.llm.harness.synthesis.body import ( FinalBodySynthesizer, ) @@ -3094,6 +3094,97 @@ def _has_block_cache_control(msg: dict) -> bool: ) +def _assert_valid_anthropic_request(msgs) -> None: + """Assert `msgs` survives litellm's Anthropic transform as a legal request. + + The check the suite was missing when GitHub issue #762 was filed: every test + here asserts on the OpenAI-shaped list that reaches `completion`, and the + live tests run against `EFFECTFUL_LLM_MODEL` -- an OpenAI model by default, + where litellm strips `cache_control` and empty text is tolerated. Nothing + looked at what Anthropic, the only provider that reads the annotation, would + be sent. + + The two block assertions are upstream 400s, confirmed against the live API: + ``messages: text content blocks must be non-empty`` and ``cache_control + cannot be set for empty text blocks``. Role alternation is deliberately not + asserted -- Anthropic accepts consecutive same-role turns, also confirmed + live. The no-turn-dropped check is the harness's invariant instead: + `anthropic_messages_pt` emits an assistant turn only ``if assistant_content``, + so a message with nothing in it vanishes from the request silently. + """ + from litellm.llms.anthropic.chat.transformation import AnthropicConfig + + # A deep copy, because the transform rewrites messages in place and `msgs` + # is the captured request the caller goes on to assert against. + transformed = AnthropicConfig().transform_request( + model="claude-sonnet-4-5", + messages=json.loads(json.dumps(list(msgs))), + optional_params={}, + litellm_params={}, + headers={}, + ) + + def walk(blocks): + """Every text block in `blocks`, including those nested in a tool_result.""" + for block in blocks: + if not isinstance(block, dict): + continue + if block.get("type") == "text": + yield block + elif isinstance(block.get("content"), list): + yield from walk(block["content"]) + + breakpoints = 0 + for message in transformed["messages"]: + content = message.get("content") + if not isinstance(content, list): + continue + for block in walk(content): + assert (block.get("text") or "").strip(), ( + f"empty text block sent to Anthropic: {block} in {message}" + ) + breakpoints += sum(1 for b in content if "cache_control" in b) + + for block in transformed.get("system") or []: + assert (block.get("text") or "").strip(), f"empty system block: {block}" + breakpoints += "cache_control" in block + + assert breakpoints <= 4, ( + f"Anthropic allows four cache breakpoints per request; got {breakpoints}" + ) + + # Consecutive same-role turns merge, so compare role runs, not counts. + def runs(roles): + out = [] + for role in roles: + if not out or out[-1] != role: + out.append(role) + return out + + sent = runs( + "user" if m["role"] in ("user", "tool") else m["role"] + for m in msgs + if m["role"] != "system" + ) + assert runs(m["role"] for m in transformed["messages"]) == sent, ( + f"a turn was dropped by the Anthropic transform: sent {sent}, " + f"got {[m['role'] for m in transformed['messages']]}" + ) + + +def _empty_text_blocks(msgs) -> list: + """Every empty text block in `msgs`, with whether it carries a breakpoint.""" + return [ + (msg["role"], "cache_control" in block) + for msg in msgs + if isinstance(msg.get("content"), list) + for block in msg["content"] + if isinstance(block, dict) + and block.get("type") == "text" + and not (block.get("text") or "").strip() + ] + + class CachingAgent(Agent): """A test agent with persistent history.""" @@ -3215,6 +3306,7 @@ def test_exactly_one_breakpoint_beyond_the_system_message(self): assert [m["role"] for m in marked] == ["system", "tool"], ( f"Expected the system message plus the last input message. Got: {marked}" ) + _assert_valid_anthropic_request(msgs) def test_breakpoint_advances_to_the_newest_message(self): """The breakpoint tracks the end of the conversation across turns, so @@ -3245,6 +3337,7 @@ def test_breakpoint_advances_to_the_newest_message(self): assert marked == [0, len(second) - 1], ( f"Expected the system message and the last message only. Got: {marked}" ) + _assert_valid_anthropic_request(second) def test_cache_control_never_enters_stored_history(self): """The annotation is added to the outgoing request, not the transcript, @@ -3375,6 +3468,215 @@ def test_litellm_strips_cache_control_for_openai(self): assert "cache_control" not in block +# ============================================================================ +# Empty content blocks +# +# Regression tests for GitHub issue #762: Anthropic rejects an empty text block +# ("messages: text content blocks must be non-empty"), and rejects it again when +# a cache breakpoint lands on it ("cache_control cannot be set for empty text +# blocks"). An ordinary Skill call reached both. +# ============================================================================ + + +@Tool.define +def silent_tool() -> str: + """A tool whose output is empty -- an empty file, a search with no hits.""" + return "" + + +@Skill.define +def use_silent_tool() -> str: + """Consult the note and report what it said.""" + raise NotHandled + + +class TestEmptyContentBlocks: + """No message the harness builds may carry an empty text block.""" + + @staticmethod + def _consult_the_note(): + capture = MockCompletionHandler( + [ + make_tool_call_response("silent_tool", "{}"), + make_text_response("it was empty"), + ] + ) + # `capture` is terminal, so it goes below the provider: the breakpoint is + # only added if `LiteLLMConfigurer.completion` runs and forwards into it. + with ( + handler(capture), + handler(AgentLoop()), + handler(LexicalToolExtractor()), + handler(LiteLLMConfigurer(model="claude-sonnet-4-5")), + handler(HistoryBuilder()), + ): + use_silent_tool() + return capture + + def test_empty_tool_result_carries_no_empty_block(self): + """A tool returning ``""`` used to encode to a single empty text block -- + and, being the last block of the last input message, to the one place + `_add_cache_control` puts its second breakpoint.""" + sent = self._consult_the_note().received_messages[-1] + + assert _empty_text_blocks(sent) == [], ( + f"empty text block(s) in the request: {_empty_text_blocks(sent)}" + ) + _assert_valid_anthropic_request(sent) + + def test_breakpoint_moves_off_an_empty_tool_result(self): + """With no block to mark, the newest message cannot carry the breakpoint; + it falls back to the previous one rather than being dropped.""" + sent = self._consult_the_note().received_messages[-1] + + marked = [m["role"] for m in sent if _has_cache_control(m)] + assert marked == ["system", "user"], ( + f"expected the breakpoint to fall back to the user message. Got: {marked}" + ) + + def test_a_value_that_merely_contains_an_empty_string_is_unchanged(self): + """Only a block with nothing in it is dropped: `to_content_blocks` still + satisfies its linearization law, so an empty string *inside* an encoded + value keeps its JSON quotes.""" + assert to_content_blocks("") == [] + assert to_content_blocks({"a": ""}) == [{"type": "text", "text": '{"a": ""}'}] + assert to_content_blocks([]) == [{"type": "text", "text": "[]"}] + + def test_empty_block_never_enters_stored_history(self): + """`SQLitePersister` checkpoints the transcript, so a block repaired only + on the way out would still be durable.""" + capture = MockCompletionHandler( + [ + make_tool_call_response("silent_tool", "{}"), + make_text_response("it was empty"), + ] + ) + agent = CachingAgent() + + with ( + handler(capture), + handler(AgentLoop()), + handler(LexicalToolExtractor()), + handler(LiteLLMConfigurer(model="claude-sonnet-4-5")), + handler(HistoryBuilder()), + ): + agent.ask("what does the note say?") + + assert _empty_text_blocks(agent.__history__) == [] + + def test_exec_code_with_no_output_is_sendable(self): + """The trigger in a long-running session: `exec_code` returns the empty + string for a snippet that printed nothing, and it is in the default + `harness()` stack.""" + + def run_silent(exec_code): + bound_args = inspect.signature(exec_code).bind( + pydantic.TypeAdapter(Encodable[CodeType]).validate_python("x = 1 + 1") + ) + tc = DecodedToolCall(exec_code, bound_args, "call_exec", "exec_code") + return call_tool(tc)[0] + + msg = _drive_repl(run_silent) + assert msg["role"] == "tool" + assert _empty_text_blocks([msg]) == [], ( + f"exec_code produced an empty block: {msg}" + ) + + def test_call_user_never_emits_an_empty_block(self): + """`call_user` and `call_system` render through `_render_prompt_section`, + which drops empty text. Pinned because nothing else checks that path.""" + + @Skill.define + def ask_about(topic: str, note: str) -> str: + """Say something about {topic}. {note}""" + raise NotHandled + + capture = MockCompletionHandler([make_text_response("ok")]) + with ( + handler(capture), + handler(AgentLoop()), + handler(LexicalToolExtractor()), + handler(LiteLLMConfigurer(model="claude-sonnet-4-5")), + handler(HistoryBuilder()), + ): + # Both holes encode to "", so the rendered prompt ends on one. + ask_about("", "") + + sent = capture.received_messages[0] + assert [m["role"] for m in sent] == ["system", "user"] + assert _empty_text_blocks(sent) == [] + _assert_valid_anthropic_request(sent) + + def test_breakpoint_skips_an_unmarkable_message(self): + """The same fallback, over messages the harness did not build: a caller + may bind `HistoryBuilder.get_history` or pass `messages` directly.""" + provider = LiteLLMConfigurer(model="claude-sonnet-4-5") + msgs = [ + {"role": "system", "content": [{"type": "text", "text": "sys"}]}, + {"role": "user", "content": [{"type": "text", "text": "q"}]}, + {"role": "user", "content": [{"type": "text", "text": " "}]}, + ] + + marked = provider._add_cache_control(msgs) + + assert [_has_block_cache_control(m) for m in marked] == [True, True, False], ( + f"breakpoint should fall back to the previous message. Got: {marked}" + ) + + +class TestEmptyReply: + """A reply with neither text nor a tool call fails loudly, naming why.""" + + @staticmethod + def _response(content, finish_reason="stop", **extra): + return ModelResponse( + id="test", + choices=[ + { + "index": 0, + "message": {"role": "assistant", "content": content, **extra}, + "finish_reason": finish_reason, + } + ], + model="test-model", + ) + + @pytest.mark.parametrize("content", [None, "", " "]) + def test_contentless_reply_names_the_finish_reason(self, content): + """An empty reply is always a symptom of something else -- a truncation, + a filtered response, a broken proxy -- and the finish_reason is the only + thing that says which.""" + capture = MockCompletionHandler( + [self._response(content, finish_reason="length")] + ) + + with ( + handler(capture), + handler(AgentLoop()), + handler(LexicalToolExtractor()), + handler(LiteLLMConfigurer(model="claude-sonnet-4-5")), + handler(HistoryBuilder()), + ): + with pytest.raises(AssertionError, match="finish_reason='length'"): + simple_prompt("test") + + def test_a_reply_carrying_only_reasoning_content_still_decodes(self): + """The `content or reasoning_content` fallback is untouched: only a reply + with nothing anywhere trips the assertion.""" + capture = MockCompletionHandler( + [self._response("", reasoning_content=json.dumps({"value": 7}))] + ) + + with ( + handler(capture), + handler(AgentLoop()), + handler(LexicalToolExtractor()), + handler(LiteLLMConfigurer(model="claude-sonnet-4-5")), + handler(HistoryBuilder()), + ): + assert generate_number(10) == 7 + + # ============================================================================ # Scoping a call to a different model # ============================================================================ From 3fadf832c27c826cf93693241d2bd0a7bfd06557 Mon Sep 17 00:00:00 2001 From: Eli Date: Wed, 2 Sep 2026 10:26:28 -0400 Subject: [PATCH 2/6] Address review on #768 Two bugs, both found by @jfeser. `_is_empty_text_block` used `.strip()`, so it called a whitespace block empty while `to_content_blocks` kept one. A tool returning whitespace therefore tripped the `append_message` assert. Whitespace is content, and Anthropic accepts a block of it (checked against the live API, plain and carrying a cache breakpoint); only `""` is rejected. The predicate now tests exactly what `to_content_blocks` guarantees, as do `_mark`'s string branch and the test helpers. `to_content_blocks` returning no block for `""` meant a conversion or format spec on an empty value never ran, so `{x!r}` rendered nothing where it used to render `''`, and `{x:>5}` lost its padding. `format_as_content_blocks` now formats such a value itself, and its `flush_text` guards on the formatted text, so a hole that formats to nothing still contributes no block. The assertion on empty model output keeps its original condition and gains only the finish_reason. An empty string collapses to `None` through the `or` and is caught here; anything with text in it goes on to decoding and fails there as a `ResultDecodingError`, which is retried. Rewrote the comments and docstrings added by this PR. Co-Authored-By: Claude Opus 5 (1M context) --- .../llm/harness/durability/transaction.py | 15 ++- effectful/handlers/llm/harness/hooks.py | 2 +- .../handlers/llm/harness/provision/litellm.py | 26 ++--- .../handlers/llm/harness/serialization.py | 28 ++++-- tests/test_handlers_llm_harness_provision.py | 96 ++++++++++++++++--- 5 files changed, 126 insertions(+), 41 deletions(-) diff --git a/effectful/handlers/llm/harness/durability/transaction.py b/effectful/handlers/llm/harness/durability/transaction.py index 315dfa1e9..e52c84582 100644 --- a/effectful/handlers/llm/harness/durability/transaction.py +++ b/effectful/handlers/llm/harness/durability/transaction.py @@ -43,9 +43,12 @@ def append_message(cls, message: Message) -> None: they sit here: every message the harness records passes through this method, including the ones a failed attempt records on its way out, and those are the ones that get a history into a shape no provider will - accept. The third holds by construction -- `to_content_blocks` and - `_render_prompt_section` are the only sources of blocks and neither - builds an empty one -- so a violation is a bug in a producer. + accept. + + The third check is about content. It holds already: the only sources of + content blocks are `to_content_blocks` and `_render_prompt_section`, and + neither builds an empty one. It is here to catch a producer that stops + holding to that. """ history = cls.get_history() assert cls._carries_no_empty_block(message), ( @@ -62,7 +65,11 @@ def append_message(cls, message: Message) -> None: @staticmethod def _carries_no_empty_block(message: Message) -> bool: - """Whether `message` is free of the empty text blocks Anthropic rejects.""" + """Whether `message` is free of the empty text blocks Anthropic rejects. + + String content is not checked. An empty string is a message with nothing + in it, which is a different problem from a block with nothing in it. + """ content = message.get("content") return not isinstance(content, list) or not any( _is_empty_text_block(block) for block in content diff --git a/effectful/handlers/llm/harness/hooks.py b/effectful/handlers/llm/harness/hooks.py index b30e78236..49b687af3 100644 --- a/effectful/handlers/llm/harness/hooks.py +++ b/effectful/handlers/llm/harness/hooks.py @@ -267,7 +267,7 @@ def call_assistant[T]( result = None if not tool_calls: serialized_result = message.get("content") or message.get("reasoning_content") - assert isinstance(serialized_result, str) and serialized_result.strip(), ( + assert isinstance(serialized_result, str), ( f"the model replied with neither content nor a tool call " f"(finish_reason={choice.finish_reason!r})" ) diff --git a/effectful/handlers/llm/harness/provision/litellm.py b/effectful/handlers/llm/harness/provision/litellm.py index cf68d0998..7f0351898 100644 --- a/effectful/handlers/llm/harness/provision/litellm.py +++ b/effectful/handlers/llm/harness/provision/litellm.py @@ -31,23 +31,23 @@ def __init__(self, model="gpt-4o", **config): @staticmethod def _mark(msg: Message) -> Message | None: """`msg`, with a `cache_control` breakpoint on its last non-empty content - block -- or `None` if it has none. + block, or `None` if it has no block to put one on. A message whose content is a plain string -- which the assembled system prompt is not, but a hand-written or externally supplied message may be -- takes the message-level key instead, the only form litellm reads a breakpoint from for string content. - Empty blocks are skipped because Anthropic answers ``cache_control - cannot be set for empty text blocks``. `to_content_blocks` no longer - builds one, so this is reached only by messages supplied from outside -- - the same messages the string case above exists for. `None` lets - `_add_cache_control` fall back to an earlier message; a message that is - already marked returns itself, since that mark is the breakpoint. + Empty blocks are skipped: Anthropic answers ``cache_control cannot be + set for empty text blocks``. `to_content_blocks` does not build one, so + only messages supplied from outside can contain one. Returning `None` + lets `_add_cache_control` mark an earlier message instead of losing the + breakpoint. An already-marked message returns itself, since that mark is + the breakpoint. """ content = msg.get("content") if isinstance(content, str): - if not content.strip(): + if not content: return None return typing.cast(Message, {**msg, "cache_control": {"type": "ephemeral"}}) if not isinstance(content, list): @@ -98,10 +98,9 @@ def _add_cache_control( transport-level annotations never reach the stored history -- and so never reach an `Agent`'s checkpointed transcript. - A message `_mark` declines is stepped over: the scan keeps walking back - until one takes the breakpoint, so a request whose newest message has - nothing markable in it caches a shorter prefix rather than going out - with one breakpoint. + If `_mark` declines a message, the scan continues to the one before it. + The request then caches a shorter prefix, rather than going out with + only one breakpoint. """ out = list(messages) for i in reversed(range(len(out))): @@ -113,7 +112,8 @@ def _add_cache_control( break for i in range(len(out)): if out[i]["role"] == "system": - # Only one system message is ever sent: stop either way. + # There is only one system message, so there is no earlier one + # to fall back to. marked = self._mark(out[i]) if marked is not None: out[i] = marked diff --git a/effectful/handlers/llm/harness/serialization.py b/effectful/handlers/llm/harness/serialization.py index 6e041718b..479e25bec 100644 --- a/effectful/handlers/llm/harness/serialization.py +++ b/effectful/handlers/llm/harness/serialization.py @@ -74,8 +74,9 @@ def to_content_blocks( the linearization law holds for non-string encoded values: ``linearize(to_content_blocks(v)) == json.dumps(v)``. - No block is empty: Anthropic rejects an empty text block outright, and - rejects it again when a cache breakpoint lands on one. + No block has empty text. Anthropic rejects a request containing one, and + rejects it again if a cache breakpoint sits on it. Whitespace is content, so + a block of spaces is kept. """ if isinstance(value, str): return [ChatCompletionTextObject(type="text", text=value)] if value else [] @@ -116,14 +117,15 @@ def walk(v: typing.Any) -> None: def _is_empty_text_block(block: typing.Any) -> bool: - """Whether `block` is a text block Anthropic will reject as empty. + """Whether `block` is a text block with no text. - Whitespace-only counts, matching litellm's own ``.strip()`` test. + This is the invariant `to_content_blocks` establishes, tested the same way: + a block of whitespace has content and is not empty. Anthropic accepts one. """ return ( isinstance(block, dict) and block.get("type") == "text" - and not (block.get("text") or "").strip() + and not block.get("text") ) @@ -135,6 +137,11 @@ def format_as_content_blocks( Format a template applied to arguments into a list of content blocks. This is similar to str.format() but produces a list of content blocks instead of a single string, so that non-text content is preserved. + + A conversion or format spec runs on the encoded value even when that value + is the empty string, which `to_content_blocks` emits no block for: ``{x!r}`` + renders ``''`` and ``{x:>5}`` renders five spaces. Text that formats to + nothing still produces no block. """ formatter = string.Formatter() parts: list[OpenAIMessageContentListBlock] = [] @@ -142,9 +149,9 @@ def format_as_content_blocks( buf: list[str] = [] def flush_text() -> None: - if buf: - parts.append(ChatCompletionTextObject(type="text", text="".join(buf))) - buf.clear() + if text := "".join(buf): + parts.append(ChatCompletionTextObject(type="text", text=text)) + buf.clear() for literal, field_name, format_spec, conversion in formatter.parse( textwrap.dedent(template) @@ -160,7 +167,10 @@ def flush_text() -> None: Encodable[nested_type(obj).value] # type: ignore[misc] ) encoded_obj = encoder.dump_python(obj, mode="json", context=env) - for part in to_content_blocks(encoded_obj): + encoded_parts = to_content_blocks(encoded_obj) + if not encoded_parts and isinstance(encoded_obj, str): + encoded_parts = [ChatCompletionTextObject(type="text", text=encoded_obj)] + for part in encoded_parts: if part["type"] == "text": text = ( formatter.convert_field(part["text"], conversion) diff --git a/tests/test_handlers_llm_harness_provision.py b/tests/test_handlers_llm_harness_provision.py index a108a03a8..31a4c94df 100644 --- a/tests/test_handlers_llm_harness_provision.py +++ b/tests/test_handlers_llm_harness_provision.py @@ -51,7 +51,11 @@ ) from effectful.handlers.llm.harness.observability.rich import RichTerminalRenderer from effectful.handlers.llm.harness.provision.litellm import LiteLLMConfigurer -from effectful.handlers.llm.harness.serialization import _NameAndTool, to_content_blocks +from effectful.handlers.llm.harness.serialization import ( + _NameAndTool, + format_as_content_blocks, + to_content_blocks, +) from effectful.handlers.llm.harness.synthesis.body import ( FinalBodySynthesizer, ) @@ -3106,9 +3110,12 @@ def _assert_valid_anthropic_request(msgs) -> None: The two block assertions are upstream 400s, confirmed against the live API: ``messages: text content blocks must be non-empty`` and ``cache_control - cannot be set for empty text blocks``. Role alternation is deliberately not - asserted -- Anthropic accepts consecutive same-role turns, also confirmed - live. The no-turn-dropped check is the harness's invariant instead: + cannot be set for empty text blocks``. A block of whitespace is not one of + them; Anthropic accepts it. + + Role alternation is not asserted. Anthropic accepts consecutive same-role + turns, also confirmed live, so requiring alternation would pin behaviour the + provider does not have. The no-turn-dropped check stands in for it: `anthropic_messages_pt` emits an assistant turn only ``if assistant_content``, so a message with nothing in it vanishes from the request silently. """ @@ -3140,13 +3147,13 @@ def walk(blocks): if not isinstance(content, list): continue for block in walk(content): - assert (block.get("text") or "").strip(), ( + assert block.get("text"), ( f"empty text block sent to Anthropic: {block} in {message}" ) breakpoints += sum(1 for b in content if "cache_control" in b) for block in transformed.get("system") or []: - assert (block.get("text") or "").strip(), f"empty system block: {block}" + assert block.get("text"), f"empty system block: {block}" breakpoints += "cache_control" in block assert breakpoints <= 4, ( @@ -3173,7 +3180,11 @@ def runs(roles): def _empty_text_blocks(msgs) -> list: - """Every empty text block in `msgs`, with whether it carries a breakpoint.""" + """Every empty text block in `msgs`, with whether it carries a breakpoint. + + Empty means no text at all. Whitespace is content, and Anthropic accepts a + block of it -- see `_is_empty_text_block`. + """ return [ (msg["role"], "cache_control" in block) for msg in msgs @@ -3181,7 +3192,7 @@ def _empty_text_blocks(msgs) -> list: for block in msg["content"] if isinstance(block, dict) and block.get("type") == "text" - and not (block.get("text") or "").strip() + and not block.get("text") ] @@ -3535,13 +3546,33 @@ def test_breakpoint_moves_off_an_empty_tool_result(self): ) def test_a_value_that_merely_contains_an_empty_string_is_unchanged(self): - """Only a block with nothing in it is dropped: `to_content_blocks` still + """Only a block with nothing in it is dropped. `to_content_blocks` still satisfies its linearization law, so an empty string *inside* an encoded value keeps its JSON quotes.""" assert to_content_blocks("") == [] assert to_content_blocks({"a": ""}) == [{"type": "text", "text": '{"a": ""}'}] assert to_content_blocks([]) == [{"type": "text", "text": "[]"}] + @pytest.mark.parametrize( + ("template", "expected"), + [ + ("a{x}b", "ab"), + ("a{x!r}b", "a''b"), + ("a{x:>5}b", "a b"), + ("a{x!r:>6}b", "a ''b"), + ], + ) + def test_a_conversion_still_runs_on_an_empty_value(self, template, expected): + """`to_content_blocks` emits no block for the empty string, but a + conversion or format spec can still turn it into something -- `{x!r}` + renders ``''``. Formatting has to run either way.""" + assert format_as_content_blocks(template, {"x": ""}) == [ + {"type": "text", "text": expected} + ] + + def test_a_hole_that_formats_to_nothing_produces_no_block(self): + assert format_as_content_blocks("{x}", {"x": ""}) == [] + def test_empty_block_never_enters_stored_history(self): """`SQLitePersister` checkpoints the transcript, so a block repaired only on the way out would still be durable.""" @@ -3614,7 +3645,7 @@ def test_breakpoint_skips_an_unmarkable_message(self): msgs = [ {"role": "system", "content": [{"type": "text", "text": "sys"}]}, {"role": "user", "content": [{"type": "text", "text": "q"}]}, - {"role": "user", "content": [{"type": "text", "text": " "}]}, + {"role": "user", "content": [{"type": "text", "text": ""}]}, ] marked = provider._add_cache_control(msgs) @@ -3623,9 +3654,24 @@ def test_breakpoint_skips_an_unmarkable_message(self): f"breakpoint should fall back to the previous message. Got: {marked}" ) + def test_whitespace_is_content(self): + """A block of spaces is not empty: Anthropic accepts one, so it keeps its + block and can carry the breakpoint like any other.""" + provider = LiteLLMConfigurer(model="claude-sonnet-4-5") + blank = {"role": "user", "content": [{"type": "text", "text": " "}]} + + assert to_content_blocks(" ") == [{"type": "text", "text": " "}] + assert _empty_text_blocks([blank]) == [] + assert _has_block_cache_control(provider._mark(blank)) + class TestEmptyReply: - """A reply with neither text nor a tool call fails loudly, naming why.""" + """A reply with nothing to decode fails loudly, naming the finish_reason. + + Only a reply with no usable text at all reaches the assertion. A reply that + has text but does not decode -- whitespace where JSON was asked for, say -- + is a `ResultDecodingError`, which `TenacityRetryer` retries as usual. + """ @staticmethod def _response(content, finish_reason="stop", **extra): @@ -3641,10 +3687,10 @@ def _response(content, finish_reason="stop", **extra): model="test-model", ) - @pytest.mark.parametrize("content", [None, "", " "]) + @pytest.mark.parametrize("content", [None, ""]) def test_contentless_reply_names_the_finish_reason(self, content): - """An empty reply is always a symptom of something else -- a truncation, - a filtered response, a broken proxy -- and the finish_reason is the only + """An empty reply is a symptom of something else -- a truncation, a + filtered response, a broken proxy -- and the finish_reason is the only thing that says which.""" capture = MockCompletionHandler( [self._response(content, finish_reason="length")] @@ -3660,6 +3706,28 @@ def test_contentless_reply_names_the_finish_reason(self, content): with pytest.raises(AssertionError, match="finish_reason='length'"): simple_prompt("test") + def test_a_blank_reply_is_retried_like_any_other_bad_output(self): + """Whitespace is text, so it decodes rather than asserting -- and fails + that decode, which is the retryable path.""" + capture = MockCompletionHandler( + [self._response(" "), make_text_response(json.dumps({"value": 4}))] + ) + + # `TenacityRetryer` goes inside `HistoryBuilder`: it appends the reply + # that finally succeeded itself, so wrapping the other way records it + # twice. + with ( + handler(capture), + handler(AgentLoop()), + handler(LexicalToolExtractor()), + handler(LiteLLMConfigurer(model="claude-sonnet-4-5")), + handler(HistoryBuilder()), + handler(TenacityRetryer(stop=tenacity.stop_after_attempt(3))), + ): + assert generate_number(10) == 4 + + assert capture.call_count == 2, "the blank reply should have been retried" + def test_a_reply_carrying_only_reasoning_content_still_decodes(self): """The `content or reasoning_content` fallback is untouched: only a reply with nothing anywhere trips the assertion.""" From 7db14d84965f818747f0caaa4766838a22b52a5b Mon Sep 17 00:00:00 2001 From: Eli Date: Wed, 2 Sep 2026 10:39:54 -0400 Subject: [PATCH 3/6] Cut the comments back Co-Authored-By: Claude Opus 5 (1M context) --- .../llm/harness/durability/transaction.py | 20 +---- .../handlers/llm/harness/provision/litellm.py | 11 +-- .../handlers/llm/harness/serialization.py | 16 +--- tests/test_handlers_llm_harness_provision.py | 86 ++++++------------- 4 files changed, 34 insertions(+), 99 deletions(-) diff --git a/effectful/handlers/llm/harness/durability/transaction.py b/effectful/handlers/llm/harness/durability/transaction.py index e52c84582..4e078f403 100644 --- a/effectful/handlers/llm/harness/durability/transaction.py +++ b/effectful/handlers/llm/harness/durability/transaction.py @@ -37,19 +37,7 @@ def get_history(cls) -> collections.abc.MutableSequence[Message]: @classmethod def append_message(cls, message: Message) -> None: - """Append `message` to the ambient history, if it is legal where it lands. - - Two of the checks are about position rather than content, which is why - they sit here: every message the harness records passes through this - method, including the ones a failed attempt records on its way out, and - those are the ones that get a history into a shape no provider will - accept. - - The third check is about content. It holds already: the only sources of - content blocks are `to_content_blocks` and `_render_prompt_section`, and - neither builds an empty one. It is here to catch a producer that stops - holding to that. - """ + """Append `message` to the ambient history, if it is legal where it lands.""" history = cls.get_history() assert cls._carries_no_empty_block(message), ( f"a message may not carry an empty text block: {message}" @@ -65,11 +53,7 @@ def append_message(cls, message: Message) -> None: @staticmethod def _carries_no_empty_block(message: Message) -> bool: - """Whether `message` is free of the empty text blocks Anthropic rejects. - - String content is not checked. An empty string is a message with nothing - in it, which is a different problem from a block with nothing in it. - """ + """Whether `message` is free of the empty text blocks Anthropic rejects.""" content = message.get("content") return not isinstance(content, list) or not any( _is_empty_text_block(block) for block in content diff --git a/effectful/handlers/llm/harness/provision/litellm.py b/effectful/handlers/llm/harness/provision/litellm.py index 7f0351898..d77ad3536 100644 --- a/effectful/handlers/llm/harness/provision/litellm.py +++ b/effectful/handlers/llm/harness/provision/litellm.py @@ -39,11 +39,7 @@ def _mark(msg: Message) -> Message | None: breakpoint from for string content. Empty blocks are skipped: Anthropic answers ``cache_control cannot be - set for empty text blocks``. `to_content_blocks` does not build one, so - only messages supplied from outside can contain one. Returning `None` - lets `_add_cache_control` mark an earlier message instead of losing the - breakpoint. An already-marked message returns itself, since that mark is - the breakpoint. + set for empty text blocks``. An already-marked message returns itself. """ content = msg.get("content") if isinstance(content, str): @@ -99,8 +95,6 @@ def _add_cache_control( never reach an `Agent`'s checkpointed transcript. If `_mark` declines a message, the scan continues to the one before it. - The request then caches a shorter prefix, rather than going out with - only one breakpoint. """ out = list(messages) for i in reversed(range(len(out))): @@ -112,8 +106,7 @@ def _add_cache_control( break for i in range(len(out)): if out[i]["role"] == "system": - # There is only one system message, so there is no earlier one - # to fall back to. + # No earlier system message to fall back to. marked = self._mark(out[i]) if marked is not None: out[i] = marked diff --git a/effectful/handlers/llm/harness/serialization.py b/effectful/handlers/llm/harness/serialization.py index 479e25bec..a4c371c5c 100644 --- a/effectful/handlers/llm/harness/serialization.py +++ b/effectful/handlers/llm/harness/serialization.py @@ -74,9 +74,7 @@ def to_content_blocks( the linearization law holds for non-string encoded values: ``linearize(to_content_blocks(v)) == json.dumps(v)``. - No block has empty text. Anthropic rejects a request containing one, and - rejects it again if a cache breakpoint sits on it. Whitespace is content, so - a block of spaces is kept. + No block has empty text; Anthropic rejects a request containing one. """ if isinstance(value, str): return [ChatCompletionTextObject(type="text", text=value)] if value else [] @@ -117,11 +115,7 @@ def walk(v: typing.Any) -> None: def _is_empty_text_block(block: typing.Any) -> bool: - """Whether `block` is a text block with no text. - - This is the invariant `to_content_blocks` establishes, tested the same way: - a block of whitespace has content and is not empty. Anthropic accepts one. - """ + """Whether `block` is a text block with no text.""" return ( isinstance(block, dict) and block.get("type") == "text" @@ -138,10 +132,8 @@ def format_as_content_blocks( This is similar to str.format() but produces a list of content blocks instead of a single string, so that non-text content is preserved. - A conversion or format spec runs on the encoded value even when that value - is the empty string, which `to_content_blocks` emits no block for: ``{x!r}`` - renders ``''`` and ``{x:>5}`` renders five spaces. Text that formats to - nothing still produces no block. + A conversion or format spec runs even on a value that encodes to ``""``, so + ``{x!r}`` renders ``''``. Text that formats to nothing produces no block. """ formatter = string.Formatter() parts: list[OpenAIMessageContentListBlock] = [] diff --git a/tests/test_handlers_llm_harness_provision.py b/tests/test_handlers_llm_harness_provision.py index 31a4c94df..39a0a530c 100644 --- a/tests/test_handlers_llm_harness_provision.py +++ b/tests/test_handlers_llm_harness_provision.py @@ -3101,23 +3101,11 @@ def _has_block_cache_control(msg: dict) -> bool: def _assert_valid_anthropic_request(msgs) -> None: """Assert `msgs` survives litellm's Anthropic transform as a legal request. - The check the suite was missing when GitHub issue #762 was filed: every test - here asserts on the OpenAI-shaped list that reaches `completion`, and the - live tests run against `EFFECTFUL_LLM_MODEL` -- an OpenAI model by default, - where litellm strips `cache_control` and empty text is tolerated. Nothing - looked at what Anthropic, the only provider that reads the annotation, would - be sent. - - The two block assertions are upstream 400s, confirmed against the live API: - ``messages: text content blocks must be non-empty`` and ``cache_control - cannot be set for empty text blocks``. A block of whitespace is not one of - them; Anthropic accepts it. - - Role alternation is not asserted. Anthropic accepts consecutive same-role - turns, also confirmed live, so requiring alternation would pin behaviour the - provider does not have. The no-turn-dropped check stands in for it: - `anthropic_messages_pt` emits an assistant turn only ``if assistant_content``, - so a message with nothing in it vanishes from the request silently. + The block assertions are upstream 400s: ``messages: text content blocks must + be non-empty`` and ``cache_control cannot be set for empty text blocks``. + Roles are not required to alternate; Anthropic accepts consecutive same-role + turns. A turn must not be dropped, which is how a message with nothing in it + used to disappear from a request. """ from litellm.llms.anthropic.chat.transformation import AnthropicConfig @@ -3180,11 +3168,7 @@ def runs(roles): def _empty_text_blocks(msgs) -> list: - """Every empty text block in `msgs`, with whether it carries a breakpoint. - - Empty means no text at all. Whitespace is content, and Anthropic accepts a - block of it -- see `_is_empty_text_block`. - """ + """Every empty text block in `msgs`, with whether it carries a breakpoint.""" return [ (msg["role"], "cache_control" in block) for msg in msgs @@ -3482,10 +3466,7 @@ def test_litellm_strips_cache_control_for_openai(self): # ============================================================================ # Empty content blocks # -# Regression tests for GitHub issue #762: Anthropic rejects an empty text block -# ("messages: text content blocks must be non-empty"), and rejects it again when -# a cache breakpoint lands on it ("cache_control cannot be set for empty text -# blocks"). An ordinary Skill call reached both. +# Regression tests for GitHub issue #762. # ============================================================================ @@ -3525,9 +3506,8 @@ def _consult_the_note(): return capture def test_empty_tool_result_carries_no_empty_block(self): - """A tool returning ``""`` used to encode to a single empty text block -- - and, being the last block of the last input message, to the one place - `_add_cache_control` puts its second breakpoint.""" + """A tool returning ``""`` used to encode to a single empty text block, + in the one place `_add_cache_control` puts its second breakpoint.""" sent = self._consult_the_note().received_messages[-1] assert _empty_text_blocks(sent) == [], ( @@ -3536,8 +3516,8 @@ def test_empty_tool_result_carries_no_empty_block(self): _assert_valid_anthropic_request(sent) def test_breakpoint_moves_off_an_empty_tool_result(self): - """With no block to mark, the newest message cannot carry the breakpoint; - it falls back to the previous one rather than being dropped.""" + """With no block to mark, the breakpoint falls back to the previous + message rather than being dropped.""" sent = self._consult_the_note().received_messages[-1] marked = [m["role"] for m in sent if _has_cache_control(m)] @@ -3546,9 +3526,8 @@ def test_breakpoint_moves_off_an_empty_tool_result(self): ) def test_a_value_that_merely_contains_an_empty_string_is_unchanged(self): - """Only a block with nothing in it is dropped. `to_content_blocks` still - satisfies its linearization law, so an empty string *inside* an encoded - value keeps its JSON quotes.""" + """`to_content_blocks` still satisfies its linearization law: an empty + string inside an encoded value keeps its JSON quotes.""" assert to_content_blocks("") == [] assert to_content_blocks({"a": ""}) == [{"type": "text", "text": '{"a": ""}'}] assert to_content_blocks([]) == [{"type": "text", "text": "[]"}] @@ -3563,9 +3542,7 @@ def test_a_value_that_merely_contains_an_empty_string_is_unchanged(self): ], ) def test_a_conversion_still_runs_on_an_empty_value(self, template, expected): - """`to_content_blocks` emits no block for the empty string, but a - conversion or format spec can still turn it into something -- `{x!r}` - renders ``''``. Formatting has to run either way.""" + """A conversion or format spec can turn an empty value into something.""" assert format_as_content_blocks(template, {"x": ""}) == [ {"type": "text", "text": expected} ] @@ -3574,8 +3551,8 @@ def test_a_hole_that_formats_to_nothing_produces_no_block(self): assert format_as_content_blocks("{x}", {"x": ""}) == [] def test_empty_block_never_enters_stored_history(self): - """`SQLitePersister` checkpoints the transcript, so a block repaired only - on the way out would still be durable.""" + """`SQLitePersister` checkpoints the transcript, so a block repaired + only on the way out would still be durable.""" capture = MockCompletionHandler( [ make_tool_call_response("silent_tool", "{}"), @@ -3596,9 +3573,8 @@ def test_empty_block_never_enters_stored_history(self): assert _empty_text_blocks(agent.__history__) == [] def test_exec_code_with_no_output_is_sendable(self): - """The trigger in a long-running session: `exec_code` returns the empty - string for a snippet that printed nothing, and it is in the default - `harness()` stack.""" + """`exec_code` returns the empty string for a snippet that printed + nothing, and is in the default `harness()` stack.""" def run_silent(exec_code): bound_args = inspect.signature(exec_code).bind( @@ -3615,7 +3591,7 @@ def run_silent(exec_code): def test_call_user_never_emits_an_empty_block(self): """`call_user` and `call_system` render through `_render_prompt_section`, - which drops empty text. Pinned because nothing else checks that path.""" + which drops empty text.""" @Skill.define def ask_about(topic: str, note: str) -> str: @@ -3639,8 +3615,7 @@ def ask_about(topic: str, note: str) -> str: _assert_valid_anthropic_request(sent) def test_breakpoint_skips_an_unmarkable_message(self): - """The same fallback, over messages the harness did not build: a caller - may bind `HistoryBuilder.get_history` or pass `messages` directly.""" + """The same fallback, over messages the harness did not build.""" provider = LiteLLMConfigurer(model="claude-sonnet-4-5") msgs = [ {"role": "system", "content": [{"type": "text", "text": "sys"}]}, @@ -3655,8 +3630,7 @@ def test_breakpoint_skips_an_unmarkable_message(self): ) def test_whitespace_is_content(self): - """A block of spaces is not empty: Anthropic accepts one, so it keeps its - block and can carry the breakpoint like any other.""" + """A block of spaces is not empty; Anthropic accepts one.""" provider = LiteLLMConfigurer(model="claude-sonnet-4-5") blank = {"role": "user", "content": [{"type": "text", "text": " "}]} @@ -3666,12 +3640,7 @@ def test_whitespace_is_content(self): class TestEmptyReply: - """A reply with nothing to decode fails loudly, naming the finish_reason. - - Only a reply with no usable text at all reaches the assertion. A reply that - has text but does not decode -- whitespace where JSON was asked for, say -- - is a `ResultDecodingError`, which `TenacityRetryer` retries as usual. - """ + """A reply with nothing to decode fails loudly, naming the finish_reason.""" @staticmethod def _response(content, finish_reason="stop", **extra): @@ -3689,9 +3658,7 @@ def _response(content, finish_reason="stop", **extra): @pytest.mark.parametrize("content", [None, ""]) def test_contentless_reply_names_the_finish_reason(self, content): - """An empty reply is a symptom of something else -- a truncation, a - filtered response, a broken proxy -- and the finish_reason is the only - thing that says which.""" + """The finish_reason is the only thing that says why the reply is empty.""" capture = MockCompletionHandler( [self._response(content, finish_reason="length")] ) @@ -3707,8 +3674,8 @@ def test_contentless_reply_names_the_finish_reason(self, content): simple_prompt("test") def test_a_blank_reply_is_retried_like_any_other_bad_output(self): - """Whitespace is text, so it decodes rather than asserting -- and fails - that decode, which is the retryable path.""" + """Whitespace is text, so it reaches decoding and fails there, which is + the retryable path.""" capture = MockCompletionHandler( [self._response(" "), make_text_response(json.dumps({"value": 4}))] ) @@ -3729,8 +3696,7 @@ def test_a_blank_reply_is_retried_like_any_other_bad_output(self): assert capture.call_count == 2, "the blank reply should have been retried" def test_a_reply_carrying_only_reasoning_content_still_decodes(self): - """The `content or reasoning_content` fallback is untouched: only a reply - with nothing anywhere trips the assertion.""" + """The `content or reasoning_content` fallback is untouched.""" capture = MockCompletionHandler( [self._response("", reasoning_content=json.dumps({"value": 7}))] ) From 28340303c3c1750ce6e504c867a28745a01485d3 Mon Sep 17 00:00:00 2001 From: Eli Date: Wed, 2 Sep 2026 10:46:29 -0400 Subject: [PATCH 4/6] Derive the empty-block invariant from one predicate `to_content_blocks` inlined the emptiness test at both of its construction sites and `_is_empty_text_block` stated it a third time, so nothing tied the three together. That is how the copies came apart in the first place: one of them said `.strip()` and the other two did not, and a tool returning whitespace tripped the assert in `append_message`. `_text_blocks` is now the only place a text block is built, and it decides by asking `_is_empty_text_block`. `to_content_blocks` and `format_as_content_blocks` both go through it, so no caller can produce a block the predicate rejects. `format_as_content_blocks` no longer fabricates a block for an empty string to keep its conversion running; it formats the string directly and lets `flush_text` decide whether anything is left to emit. Co-Authored-By: Claude Opus 5 (1M context) --- .../handlers/llm/harness/serialization.py | 59 +++++++++++-------- tests/test_handlers_llm_harness_provision.py | 8 +++ 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/effectful/handlers/llm/harness/serialization.py b/effectful/handlers/llm/harness/serialization.py index a4c371c5c..2df501130 100644 --- a/effectful/handlers/llm/harness/serialization.py +++ b/effectful/handlers/llm/harness/serialization.py @@ -60,6 +60,22 @@ ) +def _is_empty_text_block(block: typing.Any) -> bool: + """Whether `block` is a text block with no text.""" + return ( + isinstance(block, dict) + and block.get("type") == "text" + and not block.get("text") + ) + + +def _text_blocks(text: str) -> list[OpenAIMessageContentListBlock]: + """`text` as one content block, or no block at all when `_is_empty_text_block` + would reject it.""" + block = ChatCompletionTextObject(type="text", text=text) + return [] if _is_empty_text_block(block) else [block] + + @pydantic.validate_call(validate_return=True) def to_content_blocks( value: typing.Any, @@ -74,17 +90,17 @@ def to_content_blocks( the linearization law holds for non-string encoded values: ``linearize(to_content_blocks(v)) == json.dumps(v)``. - No block has empty text; Anthropic rejects a request containing one. + Every text block goes through `_text_blocks`, so none of them is empty; + Anthropic rejects a request containing one. """ if isinstance(value, str): - return [ChatCompletionTextObject(type="text", text=value)] if value else [] + return _text_blocks(value) buf: list[str] = [] blocks: list[OpenAIMessageContentListBlock] = [] def flush() -> None: - if text := "".join(buf): - blocks.append(ChatCompletionTextObject(type="text", text=text)) + blocks.extend(_text_blocks("".join(buf))) buf.clear() def walk(v: typing.Any) -> None: @@ -114,15 +130,6 @@ def walk(v: typing.Any) -> None: return blocks -def _is_empty_text_block(block: typing.Any) -> bool: - """Whether `block` is a text block with no text.""" - return ( - isinstance(block, dict) - and block.get("type") == "text" - and not block.get("text") - ) - - def format_as_content_blocks( template: str, env: collections.abc.Mapping[str, typing.Any], @@ -141,8 +148,7 @@ def format_as_content_blocks( buf: list[str] = [] def flush_text() -> None: - if text := "".join(buf): - parts.append(ChatCompletionTextObject(type="text", text=text)) + parts.extend(_text_blocks("".join(buf))) buf.clear() for literal, field_name, format_spec, conversion in formatter.parse( @@ -154,22 +160,25 @@ def flush_text() -> None: if field_name is None: continue + def formatted(text: str, conversion=conversion, spec=format_spec) -> str: + if conversion: + text = formatter.convert_field(text, conversion) + return formatter.format_field(text, spec or "") + obj, _ = formatter.get_field(field_name, (), env) encoder: pydantic.TypeAdapter[typing.Any] = pydantic.TypeAdapter( Encodable[nested_type(obj).value] # type: ignore[misc] ) encoded_obj = encoder.dump_python(obj, mode="json", context=env) - encoded_parts = to_content_blocks(encoded_obj) - if not encoded_parts and isinstance(encoded_obj, str): - encoded_parts = [ChatCompletionTextObject(type="text", text=encoded_obj)] - for part in encoded_parts: + if isinstance(encoded_obj, str): + # Formatted here rather than through `to_content_blocks`, which + # drops an empty string before a conversion or format spec could + # turn it into something. + buf.append(formatted(encoded_obj)) + continue + for part in to_content_blocks(encoded_obj): if part["type"] == "text": - text = ( - formatter.convert_field(part["text"], conversion) - if conversion - else part["text"] - ) - buf.append(formatter.format_field(text, format_spec or "")) + buf.append(formatted(part["text"])) else: flush_text() parts.append(part) diff --git a/tests/test_handlers_llm_harness_provision.py b/tests/test_handlers_llm_harness_provision.py index 39a0a530c..36a02703d 100644 --- a/tests/test_handlers_llm_harness_provision.py +++ b/tests/test_handlers_llm_harness_provision.py @@ -52,6 +52,7 @@ from effectful.handlers.llm.harness.observability.rich import RichTerminalRenderer from effectful.handlers.llm.harness.provision.litellm import LiteLLMConfigurer from effectful.handlers.llm.harness.serialization import ( + _is_empty_text_block, _NameAndTool, format_as_content_blocks, to_content_blocks, @@ -3532,6 +3533,13 @@ def test_a_value_that_merely_contains_an_empty_string_is_unchanged(self): assert to_content_blocks({"a": ""}) == [{"type": "text", "text": '{"a": ""}'}] assert to_content_blocks([]) == [{"type": "text", "text": "[]"}] + @pytest.mark.parametrize( + "value", ["", " ", "x", {}, [], {"a": ""}, {"a": [1, ""]}, 0, None] + ) + def test_to_content_blocks_agrees_with_is_empty_text_block(self, value): + """The invariant `HistoryBuilder.append_message` asserts.""" + assert not any(_is_empty_text_block(b) for b in to_content_blocks(value)) + @pytest.mark.parametrize( ("template", "expected"), [ From fe007ca92479bba677534468fa14b5e32a833303 Mon Sep 17 00:00:00 2001 From: Eli Date: Wed, 2 Sep 2026 11:08:17 -0400 Subject: [PATCH 5/6] Retry a reply with nothing to decode Moving the assertion inside the try makes an empty reply a `ResultDecodingError` like any other undecodable one: `HistoryBuilder` turns it into feedback and `TenacityRetryer` retries it, then fails with the finish_reason still in the message. This is what @jfeser asked for on the PR. Co-Authored-By: Claude Opus 5 (1M context) --- effectful/handlers/llm/harness/hooks.py | 8 ++++---- tests/test_handlers_llm_harness_provision.py | 13 ++++++------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/effectful/handlers/llm/harness/hooks.py b/effectful/handlers/llm/harness/hooks.py index 49b687af3..dcbf340e0 100644 --- a/effectful/handlers/llm/harness/hooks.py +++ b/effectful/handlers/llm/harness/hooks.py @@ -267,11 +267,11 @@ def call_assistant[T]( result = None if not tool_calls: serialized_result = message.get("content") or message.get("reasoning_content") - assert isinstance(serialized_result, str), ( - f"the model replied with neither content nor a tool call " - f"(finish_reason={choice.finish_reason!r})" - ) try: + assert isinstance(serialized_result, str), ( + f"the model replied with neither content nor a tool call " + f"(finish_reason={choice.finish_reason!r})" + ) # A text answer is the model's own prose, and anything else is JSON # shaped like the response format. Both are boxed and validated # through the same model, so whatever the return annotation carries diff --git a/tests/test_handlers_llm_harness_provision.py b/tests/test_handlers_llm_harness_provision.py index 36a02703d..5046d33ff 100644 --- a/tests/test_handlers_llm_harness_provision.py +++ b/tests/test_handlers_llm_harness_provision.py @@ -3648,7 +3648,7 @@ def test_whitespace_is_content(self): class TestEmptyReply: - """A reply with nothing to decode fails loudly, naming the finish_reason.""" + """A reply with nothing to decode is retried, and reports the finish_reason.""" @staticmethod def _response(content, finish_reason="stop", **extra): @@ -3678,14 +3678,13 @@ def test_contentless_reply_names_the_finish_reason(self, content): handler(LiteLLMConfigurer(model="claude-sonnet-4-5")), handler(HistoryBuilder()), ): - with pytest.raises(AssertionError, match="finish_reason='length'"): + with pytest.raises(ResultDecodingError, match="finish_reason='length'"): simple_prompt("test") - def test_a_blank_reply_is_retried_like_any_other_bad_output(self): - """Whitespace is text, so it reaches decoding and fails there, which is - the retryable path.""" + @pytest.mark.parametrize("content", [None, "", " "]) + def test_an_empty_reply_is_retried_like_any_other_bad_output(self, content): capture = MockCompletionHandler( - [self._response(" "), make_text_response(json.dumps({"value": 4}))] + [self._response(content), make_text_response(json.dumps({"value": 4}))] ) # `TenacityRetryer` goes inside `HistoryBuilder`: it appends the reply @@ -3701,7 +3700,7 @@ def test_a_blank_reply_is_retried_like_any_other_bad_output(self): ): assert generate_number(10) == 4 - assert capture.call_count == 2, "the blank reply should have been retried" + assert capture.call_count == 2, "the empty reply should have been retried" def test_a_reply_carrying_only_reasoning_content_still_decodes(self): """The `content or reasoning_content` fallback is untouched.""" From 41dd6e91abd3ea3eb88511a61593b1bf8ef0c9e1 Mon Sep 17 00:00:00 2001 From: Eli Date: Wed, 2 Sep 2026 20:07:11 -0400 Subject: [PATCH 6/6] Stop asserting what Anthropic rejects The oracle asserted no empty text block reached the transformed request, no cache breakpoint sat on one, and no more than four breakpoints were present. Those are claims about the provider, taken from probes run while writing the change and checked by nothing afterwards: they can fail when our request changes, never when the belief behind them was wrong. What is left is our own invariant, checked by running the transform: no turn we put in the history is dropped on the way out. The regression test for the bug itself is unaffected -- it asserts our messages carry no empty block, which is a fact about `to_content_blocks`. Neither litellm nor the Anthropic SDK offers a local validator to check against instead. litellm's `count_tokens` support is proxy-side only, its client-side validators cover the OpenAI shape, and the one place it encodes the empty-block rule is `_sanitize_empty_text_content`, a repair that skips tool messages -- the case this bug was in. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_handlers_llm_harness_provision.py | 54 +++++--------------- 1 file changed, 12 insertions(+), 42 deletions(-) diff --git a/tests/test_handlers_llm_harness_provision.py b/tests/test_handlers_llm_harness_provision.py index 5046d33ff..eb7064268 100644 --- a/tests/test_handlers_llm_harness_provision.py +++ b/tests/test_handlers_llm_harness_provision.py @@ -3099,14 +3099,13 @@ def _has_block_cache_control(msg: dict) -> bool: ) -def _assert_valid_anthropic_request(msgs) -> None: - """Assert `msgs` survives litellm's Anthropic transform as a legal request. - - The block assertions are upstream 400s: ``messages: text content blocks must - be non-empty`` and ``cache_control cannot be set for empty text blocks``. - Roles are not required to alternate; Anthropic accepts consecutive same-role - turns. A turn must not be dropped, which is how a message with nothing in it - used to disappear from a request. +def _assert_no_turn_dropped(msgs) -> None: + """Assert litellm's Anthropic transform carries every turn of `msgs` through. + + `anthropic_messages_pt` emits an assistant turn only ``if assistant_content``, + so a message with nothing in it disappears from the request silently. This + checks our own messages survive the transform; it makes no claim about which + requests Anthropic accepts. """ from litellm.llms.anthropic.chat.transformation import AnthropicConfig @@ -3120,35 +3119,6 @@ def _assert_valid_anthropic_request(msgs) -> None: headers={}, ) - def walk(blocks): - """Every text block in `blocks`, including those nested in a tool_result.""" - for block in blocks: - if not isinstance(block, dict): - continue - if block.get("type") == "text": - yield block - elif isinstance(block.get("content"), list): - yield from walk(block["content"]) - - breakpoints = 0 - for message in transformed["messages"]: - content = message.get("content") - if not isinstance(content, list): - continue - for block in walk(content): - assert block.get("text"), ( - f"empty text block sent to Anthropic: {block} in {message}" - ) - breakpoints += sum(1 for b in content if "cache_control" in b) - - for block in transformed.get("system") or []: - assert block.get("text"), f"empty system block: {block}" - breakpoints += "cache_control" in block - - assert breakpoints <= 4, ( - f"Anthropic allows four cache breakpoints per request; got {breakpoints}" - ) - # Consecutive same-role turns merge, so compare role runs, not counts. def runs(roles): out = [] @@ -3302,7 +3272,7 @@ def test_exactly_one_breakpoint_beyond_the_system_message(self): assert [m["role"] for m in marked] == ["system", "tool"], ( f"Expected the system message plus the last input message. Got: {marked}" ) - _assert_valid_anthropic_request(msgs) + _assert_no_turn_dropped(msgs) def test_breakpoint_advances_to_the_newest_message(self): """The breakpoint tracks the end of the conversation across turns, so @@ -3333,7 +3303,7 @@ def test_breakpoint_advances_to_the_newest_message(self): assert marked == [0, len(second) - 1], ( f"Expected the system message and the last message only. Got: {marked}" ) - _assert_valid_anthropic_request(second) + _assert_no_turn_dropped(second) def test_cache_control_never_enters_stored_history(self): """The annotation is added to the outgoing request, not the transcript, @@ -3514,7 +3484,7 @@ def test_empty_tool_result_carries_no_empty_block(self): assert _empty_text_blocks(sent) == [], ( f"empty text block(s) in the request: {_empty_text_blocks(sent)}" ) - _assert_valid_anthropic_request(sent) + _assert_no_turn_dropped(sent) def test_breakpoint_moves_off_an_empty_tool_result(self): """With no block to mark, the breakpoint falls back to the previous @@ -3620,7 +3590,7 @@ def ask_about(topic: str, note: str) -> str: sent = capture.received_messages[0] assert [m["role"] for m in sent] == ["system", "user"] assert _empty_text_blocks(sent) == [] - _assert_valid_anthropic_request(sent) + _assert_no_turn_dropped(sent) def test_breakpoint_skips_an_unmarkable_message(self): """The same fallback, over messages the harness did not build.""" @@ -3638,7 +3608,7 @@ def test_breakpoint_skips_an_unmarkable_message(self): ) def test_whitespace_is_content(self): - """A block of spaces is not empty; Anthropic accepts one.""" + """A block of spaces is not empty, so it keeps its block.""" provider = LiteLLMConfigurer(model="claude-sonnet-4-5") blank = {"role": "user", "content": [{"type": "text", "text": " "}]}