fix(agent): surface model refusals instead of retrying them as errors
A Claude refusal (HTTP 200, stop_reason="refusal", empty content) was
laundered into a generic retry loop and surfaced as a misleading
"rate limited / invalid response" or "no content after retries" error,
burning paid attempts reproducing a deterministic refusal.
This hit two distinct paths:
- Direct Anthropic (anthropic_messages): validate_response rejected the
empty-content refusal *before* normalize_response mapped refusal ->
content_filter, so it fell into the invalid-response retry loop.
- Nous Portal / OpenAI-compatible (chat_completions): the portal surfaces
a Claude refusal via message.refusal with empty content, which sailed
past validation and died in the empty-response retry loop.
Fix (one unified content_filter dispatch for all backends):
- AnthropicTransport.validate_response: accept empty content when
stop_reason == "refusal" so it flows to normalize_response.
- ChatCompletionsTransport.normalize_response: promote message.refusal to
content + a content_filter finish reason.
- conversation_loop: handle finish_reason == "content_filter" - fire the
api_request_error hook (content_policy_blocked), try a configured
fallback once, else return a clear terminal refusal message. Never retry
a deterministic refusal.
Supersedes #43084, which fixed only the direct-Anthropic path and could
not reach the chat_completions/portal path.
Tests: transport-level (validate_response refusal, message.refusal
promotion) + end-to-end loop (refusal surfaced, exactly one API call).
(cherry picked from commit 01f546f92c)
This commit is contained in:
@@ -843,6 +843,81 @@ class TestChatCompletionsNormalize:
|
||||
nr = transport.normalize_response(r)
|
||||
assert nr.provider_data == {"reasoning_content": "model-extra scratchpad"}
|
||||
|
||||
def test_refusal_field_promoted_to_content_filter(self, transport):
|
||||
"""OpenAI-compatible proxies (e.g. Nous Portal fronting Anthropic) can
|
||||
surface a Claude refusal via ``message.refusal`` with empty content and
|
||||
``finish_reason="stop"``. Promote it to content + a ``content_filter``
|
||||
finish reason so the agent loop's refusal handler surfaces it instead
|
||||
of retrying an empty response three times and giving up."""
|
||||
r = SimpleNamespace(
|
||||
choices=[SimpleNamespace(
|
||||
message=SimpleNamespace(
|
||||
content=None, tool_calls=None, reasoning_content=None,
|
||||
refusal="I can't help with that.",
|
||||
),
|
||||
finish_reason="stop",
|
||||
)],
|
||||
usage=None,
|
||||
)
|
||||
nr = transport.normalize_response(r)
|
||||
assert nr.finish_reason == "content_filter"
|
||||
assert nr.content == "I can't help with that."
|
||||
assert nr.provider_data == {"refusal": "I can't help with that."}
|
||||
|
||||
def test_refusal_none_is_noop(self, transport):
|
||||
"""The common case: ``refusal`` is None → behavior unchanged."""
|
||||
r = SimpleNamespace(
|
||||
choices=[SimpleNamespace(
|
||||
message=SimpleNamespace(
|
||||
content="hello", tool_calls=None, reasoning_content=None,
|
||||
refusal=None,
|
||||
),
|
||||
finish_reason="stop",
|
||||
)],
|
||||
usage=None,
|
||||
)
|
||||
nr = transport.normalize_response(r)
|
||||
assert nr.finish_reason == "stop"
|
||||
assert nr.content == "hello"
|
||||
assert nr.provider_data is None
|
||||
|
||||
def test_refusal_preserves_explicit_content_filter_finish_reason(self, transport):
|
||||
"""When the proxy already sets ``finish_reason="content_filter"`` and
|
||||
also provides refusal text, surface the text without disturbing the
|
||||
finish reason."""
|
||||
r = SimpleNamespace(
|
||||
choices=[SimpleNamespace(
|
||||
message=SimpleNamespace(
|
||||
content=None, tool_calls=None, reasoning_content=None,
|
||||
refusal="declined",
|
||||
),
|
||||
finish_reason="content_filter",
|
||||
)],
|
||||
usage=None,
|
||||
)
|
||||
nr = transport.normalize_response(r)
|
||||
assert nr.finish_reason == "content_filter"
|
||||
assert nr.content == "declined"
|
||||
assert nr.provider_data == {"refusal": "declined"}
|
||||
|
||||
def test_refusal_does_not_clobber_existing_content(self, transport):
|
||||
"""If the model emitted partial text *and* a refusal, keep the visible
|
||||
text as content but still flag the refusal via content_filter."""
|
||||
r = SimpleNamespace(
|
||||
choices=[SimpleNamespace(
|
||||
message=SimpleNamespace(
|
||||
content="partial answer", tool_calls=None,
|
||||
reasoning_content=None, refusal="cannot continue",
|
||||
),
|
||||
finish_reason="stop",
|
||||
)],
|
||||
usage=None,
|
||||
)
|
||||
nr = transport.normalize_response(r)
|
||||
assert nr.content == "partial answer"
|
||||
assert nr.finish_reason == "content_filter"
|
||||
assert nr.provider_data == {"refusal": "cannot continue"}
|
||||
|
||||
|
||||
class TestChatCompletionsCacheStats:
|
||||
|
||||
|
||||
@@ -128,6 +128,15 @@ class TestAnthropicTransport:
|
||||
r = SimpleNamespace(content=[], stop_reason="tool_use")
|
||||
assert transport.validate_response(r) is False
|
||||
|
||||
def test_validate_response_empty_content_with_refusal_is_valid(self, transport):
|
||||
# Claude 4.5+ returns an empty content list with stop_reason="refusal"
|
||||
# when it declines to respond. It must validate so the response flows
|
||||
# through to normalize_response (which maps refusal → content_filter)
|
||||
# and the loop's refusal handler — instead of being rejected as an
|
||||
# "invalid response" and retried as a deterministic refusal.
|
||||
r = SimpleNamespace(content=[], stop_reason="refusal")
|
||||
assert transport.validate_response(r) is True
|
||||
|
||||
def test_validate_response_valid(self, transport):
|
||||
r = SimpleNamespace(content=[SimpleNamespace(type="text", text="hello")])
|
||||
assert transport.validate_response(r) is True
|
||||
|
||||
@@ -4660,6 +4660,47 @@ class TestRetryExhaustion:
|
||||
assert "error" in result
|
||||
assert "Invalid API response" in result["error"]
|
||||
|
||||
def test_content_filter_refusal_surfaced_not_retried(self, agent):
|
||||
"""A model refusal must be surfaced immediately, NOT laundered into
|
||||
the empty-response retry loop and reported as "rate limited" / "no
|
||||
content after retries".
|
||||
|
||||
Regression: running a Claude refusal through an OpenAI-compatible
|
||||
portal (Nous Portal fronting Anthropic) returns ``message.refusal``
|
||||
with empty content. The transport now promotes that to a
|
||||
``content_filter`` finish reason and the loop surfaces it as a terminal
|
||||
``content_policy_blocked`` result instead of retrying a deterministic
|
||||
refusal three times.
|
||||
"""
|
||||
self._setup_agent(agent)
|
||||
refusal_resp = SimpleNamespace(
|
||||
choices=[SimpleNamespace(
|
||||
message=SimpleNamespace(
|
||||
content=None, tool_calls=None, reasoning=None,
|
||||
reasoning_content=None, refusal="I won't help with that.",
|
||||
),
|
||||
finish_reason="stop",
|
||||
)],
|
||||
model="test/model",
|
||||
usage=None,
|
||||
id="resp_1",
|
||||
)
|
||||
agent.client.chat.completions.create.return_value = refusal_resp
|
||||
with (
|
||||
patch.object(agent, "_persist_session"),
|
||||
patch.object(agent, "_save_trajectory"),
|
||||
patch.object(agent, "_cleanup_task_resources"),
|
||||
):
|
||||
result = agent.run_conversation("please do something disallowed")
|
||||
assert result.get("completed") is False
|
||||
assert result.get("failed") is True
|
||||
assert "content_policy_blocked" in result.get("error", "")
|
||||
# The model's refusal text is surfaced to the user, not swallowed.
|
||||
assert "I won't help with that." in (result.get("final_response") or "")
|
||||
# Crucial regression guard: a deterministic refusal is NOT retried —
|
||||
# exactly one API call, no empty-response retry loop.
|
||||
assert agent.client.chat.completions.create.call_count == 1
|
||||
|
||||
def test_api_error_returns_gracefully_after_retries(self, agent):
|
||||
"""Exhausted retries on API errors must return error result, not crash."""
|
||||
self._setup_agent(agent)
|
||||
|
||||
Reference in New Issue
Block a user