From c0b2b6a796e8888f00c1fd962a87a49ec1969f8a Mon Sep 17 00:00:00 2001 From: rezaho Date: Sun, 12 Jul 2026 14:54:15 +0200 Subject: [PATCH] fix(models): an empty Anthropic end_turn is a silent turn, not an API error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A model that runs to natural completion and produces no content blocks is returning a success: "I have nothing to say." Both Anthropic adapters raised a typed ModelAPIError on it instead, and the classifier pinned that error non-retryable — so every silent turn died terminally. This is a behaviour callers explicitly ask for. An agent instructed to stay quiet when it has nothing to report (a heartbeat that found no work, an inbox check that found no mail) obeys, ends the turn with zero content blocks and stop_reason 'end_turn', and the provider bills it as a successful response. Confirmed on the wire: message_start (content []) -> message_delta (stop_reason end_turn, output_tokens 2) -> message_stop, no content_block_start. The raise was a symptom. The root is that HarmonizedResponse could not represent the outcome: its validator rejects content=None with no tool_calls, the adapters normalize empty text to None, and the empty-output branch had only two arms (deterministic truncation -> placeholder; everything else -> raise). The typed error was added to replace an UNKNOWN ValidationError, which improved the error's quality while cementing a success as an error class. The validator already blesses the shape this needs: it checks `content is None` specifically, because an empty string is a valid response from some providers. The API-key adapter already uses that escape for thinking-only responses. So the empty-output branch gains its correct third arm — end_turn harmonizes to content="" — beside the two that exist, rather than a parallel path. Refusal and a stream that closed with no terminal at all keep raising: those are genuine anomalies, and that distinction is the value the typed-error work added. The test asserting empty end_turn raises is INVERTED, not removed: it encoded the wrong contract. Its replacement pins the silent turn, and a twin covers the API-key adapter so one provider keeps one behaviour. The now-unreachable end_turn classification arm is deleted. --- src/marsys/agents/exceptions.py | 19 ++----- src/marsys/models/adapters/anthropic.py | 21 ++++---- src/marsys/models/adapters/anthropic_oauth.py | 38 +++++++++----- tests/models/test_adapter_harmonize.py | 52 ++++++++++++++----- 4 files changed, 81 insertions(+), 49 deletions(-) diff --git a/src/marsys/agents/exceptions.py b/src/marsys/agents/exceptions.py index 2f4e7d34..4d84dd2d 100644 --- a/src/marsys/agents/exceptions.py +++ b/src/marsys/agents/exceptions.py @@ -1029,9 +1029,11 @@ def from_provider_response( # ValidationError with the provider's terminal signal destroyed). # Classification branches on stop_reason — the documented contract; # stop_details is NULLABLE decoration, appended to the message when - # present and never keyed on. max_tokens/model_context_window_exceeded - # never arrive here: harmonization routes them to the truncation - # placeholder. + # present and never keyed on. Two terminals never arrive here, because + # harmonization represents them instead of raising: + # max_tokens/model_context_window_exceeded (→ truncation placeholder) + # and end_turn (→ a silent turn, content="": the model finished and + # chose to say nothing, which is a success, not a fault). stop_reason = raw_response.get("stop_reason") details = raw_response.get("stop_details") details = details if isinstance(details, dict) else {} @@ -1049,17 +1051,6 @@ def from_provider_response( "The provider declined to answer this request. Modify or " "rephrase it; retrying unmodified will be refused again." ) - elif stop_reason == "end_turn": - # Anthropic's documented guidance: don't retry empty responses - # without modification — the model already decided it was done. - classification = APIErrorClassification.EMPTY_COMPLETION.value - is_retryable = False - message = "Anthropic returned an empty response (stop_reason 'end_turn', no content)" - suggested_action = ( - "Do not retry unmodified — the model decided it was done. " - "Send a modified request, e.g. a continuation prompt asking " - "it to produce the response." - ) else: # stop_sequence, never-seen stop reasons, or NO terminal at all # (stream closed before message_delta): transient — retry. diff --git a/src/marsys/models/adapters/anthropic.py b/src/marsys/models/adapters/anthropic.py index 5f1716e0..8b36d47b 100644 --- a/src/marsys/models/adapters/anthropic.py +++ b/src/marsys/models/adapters/anthropic.py @@ -496,8 +496,9 @@ def harmonize_response( # Empty-output contract (twin of anthropic_oauth.py): deterministic # truncation gets the cross-adapter placeholder (openai.py's convention) - # so callers see one shape, never None; every OTHER fully-empty terminal - # (refusal / empty end_turn / no stop_reason) raises a typed + # so callers see one shape, never None; a natural-completion terminal + # (end_turn) is a SILENT TURN and takes the content="" path below; every + # OTHER fully-empty terminal (refusal / no stop_reason) raises a typed # ModelAPIError classified by stop_reason instead of constructing a # content=None shell the model validator rejects as an UNKNOWN # ValidationError. Thinking-only responses are NOT empty — they take the @@ -517,7 +518,7 @@ def harmonize_response( "[Response truncated due to token limit. Please increase max_tokens " "or continue the conversation.]" ) - else: + elif stop_reason_raw != "end_turn": from marsys.agents.exceptions import ModelAPIError raise ModelAPIError.from_provider_response( @@ -526,12 +527,14 @@ def harmonize_response( ) content = text_content if text_content else None - # Thinking-only response (the latent gap anthropic_oauth.py:766 records, - # reachable now that thinking is enableable): the validator requires - # content-or-tool_calls and ignores thinking. An empty STRING is a valid - # content shape (the None check is what fails), so a response that is - # all thinking harmonizes instead of dying in validation. - if content is None and not tool_calls and (thinking_parts or reasoning_details): + # An empty STRING is a valid content shape (the validator's None check is + # what fails), so two responses that carry no text still harmonize rather + # than dying in validation: a thinking-only response, and a SILENT TURN — + # the model ran to natural completion (end_turn) and chose to produce + # nothing, which callers ask for and the provider bills as a success. + if content is None and not tool_calls and ( + thinking_parts or reasoning_details or stop_reason_raw == "end_turn" + ): content = "" # Build harmonized response diff --git a/src/marsys/models/adapters/anthropic_oauth.py b/src/marsys/models/adapters/anthropic_oauth.py index 4f7f8aa0..f7435ad4 100644 --- a/src/marsys/models/adapters/anthropic_oauth.py +++ b/src/marsys/models/adapters/anthropic_oauth.py @@ -822,15 +822,19 @@ def harmonize_response( """Convert streaming response to HarmonizedResponse. Empty-output contract: a stream that terminated with NO text, NO tool - calls, and NO thinking either takes the truncation placeholder + calls, and NO thinking takes one of three arms. Deterministic truncation (normalized finish_reason ``length`` — ``max_tokens`` or - ``model_context_window_exceeded``) or raises a typed, classified - ``ModelAPIError`` built from the terminal signal (``refusal``, empty - ``end_turn``, no terminal at all). Together with the run paths' - in-stream ``error`` handling, every stream outcome maps to a valid - ``HarmonizedResponse`` or a typed ``ModelAPIError`` — never a - ``content=None`` shell that dies in the model validator as an - UNKNOWN ValidationError with the provider signal destroyed. + ``model_context_window_exceeded``) gets the truncation placeholder. A + natural-completion terminal (``end_turn``) is a SILENT TURN: the model + finished and chose to say nothing, which is a success, and harmonizes to + the empty-string content shape (the validator rejects ``None``, not + ``""``). Every OTHER empty terminal (``refusal``, no terminal at all) + raises a typed, classified ``ModelAPIError`` built from the terminal + signal. Together with the run paths' in-stream ``error`` handling, every + stream outcome maps to a valid ``HarmonizedResponse`` or a typed + ``ModelAPIError`` — never a ``content=None`` shell that dies in the model + validator as an UNKNOWN ValidationError with the provider signal + destroyed. (Latent gap, recorded 2026-06-11, still open: a thinking-only response — ``thinking`` set, no text, no tool calls, NON-length stop_reason — is @@ -892,14 +896,24 @@ def harmonize_response( # Empty-output contract (docstring above). Deterministic truncation gets # the cross-adapter placeholder (openai.py's convention) so callers see - # one shape, never None; every OTHER empty terminal is a typed failure - # classified by stop_reason (refusal / empty end_turn / no terminal). + # one shape, never None; a natural-completion terminal is a silent turn; + # every OTHER empty terminal is a typed failure classified by stop_reason + # (refusal / no terminal). + content = text_content if text_content else None if not text_content and not tool_calls and not raw_response.get("thinking"): if finish_reason == "length": - text_content = ( + content = ( "[Response truncated due to token limit. Please increase max_tokens " "or continue the conversation.]" ) + elif stop_reason_raw == "end_turn": + # A silent turn: the model ran to natural completion and produced + # nothing. Callers ask for this (an agent told to stay quiet when + # it has nothing to report), the provider bills it as a success, + # and the empty STRING is the content shape that carries it — the + # validator's rejection is of None, never of "". The API-key twin + # uses the same escape for its thinking-only responses. + content = "" else: from marsys.agents.exceptions import ModelAPIError from marsys.models.adapters.streaming import empty_completion_payload @@ -912,7 +926,7 @@ def harmonize_response( # Build response return HarmonizedResponse( role="assistant", - content=text_content if text_content else None, + content=content, tool_calls=tool_calls, thinking=raw_response.get("thinking") or None, metadata=metadata, diff --git a/tests/models/test_adapter_harmonize.py b/tests/models/test_adapter_harmonize.py index 6f4fc9c5..6c9257c8 100644 --- a/tests/models/test_adapter_harmonize.py +++ b/tests/models/test_adapter_harmonize.py @@ -287,10 +287,16 @@ def test_anthropic_truncation_empty_harmonizes_valid_with_placeholder(): # empty `model_context_window_exceeded`, or a stream that closed without a terminal — # used to construct HarmonizedResponse(content=None), die in the model validator, and # surface as an UNKNOWN ValidationError with the provider's terminal signal destroyed -# (the boot-replay crash). Contract now: deterministic truncation (max_tokens AND -# model_context_window_exceeded) takes the placeholder; every OTHER empty terminal -# raises a typed ModelAPIError classified by stop_reason. stop_details is nullable -# decoration: captured by the readers, surfaced in messages, never keyed on. +# (the boot-replay crash). +# +# Contract now, in three arms: +# - deterministic truncation (max_tokens AND model_context_window_exceeded) takes +# the placeholder; +# - `end_turn` is a SILENT TURN — the model ran to natural completion and chose to +# say nothing. A success, harmonized to content="" (see below); +# - every OTHER empty terminal raises a typed ModelAPIError classified by stop_reason. +# stop_details is nullable decoration: captured by the readers, surfaced in messages, +# never keyed on. def _empty_oauth_raw(stop_reason, stop_details=None, **overrides): @@ -334,16 +340,34 @@ def test_oauth_empty_refusal_without_stop_details_still_classifies(): assert "category" not in str(err) -def test_oauth_empty_end_turn_raises_typed_with_recovery_action(): - """Empty end_turn is non-retryable (Anthropic: don't retry empty responses - without modification); the suggested action carries the documented recovery.""" - with pytest.raises(ModelAPIError) as exc: - _oauth_adapter().harmonize_response(_empty_oauth_raw("end_turn"), request_start_time=0.0) - err = exc.value - assert err.classification == APIErrorClassification.EMPTY_COMPLETION.value - assert err.is_retryable is False - assert "end_turn" in str(err) - assert "modif" in (err.suggested_action or "").lower() +def test_oauth_empty_end_turn_is_a_silent_turn_not_an_error(): + """A SILENT TURN, not a failure. An agent instructed to stay quiet when it has + nothing to report ends the turn with zero content blocks and stop_reason + 'end_turn'; the provider bills that as a success. It must harmonize to the + empty-STRING content shape (the validator rejects None, never ""), so a caller + that supports a contentless reply gets one instead of a raised turn. + + This INVERTS the original 2026-06-12 assertion (empty end_turn → non-retryable + ModelAPIError). That contract was wrong: it classified a success as a fault and + terminally killed every silent turn, which is a behaviour the prompt layer + explicitly asks for.""" + resp = _oauth_adapter().harmonize_response( + _empty_oauth_raw("end_turn"), request_start_time=0.0 + ) + assert resp.content == "" # the empty-string shape, NOT None + assert resp.tool_calls == [] + assert resp.metadata.stop_reason == "end_turn" + assert resp.metadata.finish_reason == "end_turn" + + +def test_anthropic_empty_end_turn_is_a_silent_turn_not_an_error(): + """The API-key twin holds the same contract — one provider, one behaviour.""" + raw = {"role": "assistant", "content": [], "stop_reason": "end_turn", + "usage": {"input_tokens": 1, "output_tokens": 2}} + resp = _adapter().harmonize_response(raw, request_start_time=0.0) + assert resp.content == "" + assert resp.tool_calls == [] + assert resp.metadata.stop_reason == "end_turn" @pytest.mark.parametrize("stop_reason", [None, "stop_sequence", "never_seen_terminal"])