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"])