diff --git a/src/marsys/models/adapters/anthropic.py b/src/marsys/models/adapters/anthropic.py index d480a4eb..b8641e48 100644 --- a/src/marsys/models/adapters/anthropic.py +++ b/src/marsys/models/adapters/anthropic.py @@ -103,6 +103,58 @@ def _anthropic_model_requires_adaptive_thinking(model_name: str) -> bool: CACHE_EXEMPT_KEY = "cache_exempt" +def apply_structured_output( + payload: Dict[str, Any], + user_messages: List[Dict[str, Any]], + *, + response_schema: Optional[Dict[str, Any]] = None, + json_mode: bool = False, + native: bool = True, +) -> None: + """Put a caller's structured-output request on an Anthropic payload, in place. + + THE implementation for both Anthropic payload builders — the api-key adapter below + and the OAuth one, which is not a subclass of it and builds its own payload. The two + are deliberately parallel, and this branch is exactly where the parallelism drifted: + the merge fix and the schema-in-prompt fallback landed on one leg and not the other, + so the OAuth leg clobbered ``output_config.effort`` with the schema and degraded a + schema request to a bare "reply with JSON" nudge. One function, one shape. + + ``native`` is the ENDPOINT's capability (``supports_structured_output``): the + first-party Messages API enforces ``output_config.format``, while the Bedrock + endpoints reject the key outright. Without it the schema goes into the PROMPT, in + full — a bare "valid JSON" hint would satisfy the caller's parser only by luck. + """ + if response_schema and native: + # Merge, never assign: `effort` may already own output_config, and the API + # takes exactly one such object per request. + payload.setdefault("output_config", {})["format"] = { + "type": "json_schema", + "schema": APIProviderAdapter._ensure_additional_properties_false(response_schema), + } + return + if not (json_mode or response_schema) or not user_messages: + return + last_msg = user_messages[-1] + if last_msg.get("role") != "user": + return + if response_schema: + hint = ( + "\n\nRespond with valid JSON only — no prose, no code fence — " + "conforming exactly to this JSON Schema:\n" + + json.dumps( + APIProviderAdapter._ensure_additional_properties_false(response_schema) + ) + ) + else: + hint = "\n\nPlease respond with valid JSON only." + content = last_msg["content"] + if isinstance(content, list): + last_msg["content"] = content + [{"type": "text", "text": hint}] + else: + last_msg["content"] = str(content) + hint + + def mark_conversation_tail_for_cache( messages: List[Dict[str, Any]], *, volatile_tail: int = 0 ) -> None: @@ -493,39 +545,16 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An else system_message ) - # Handle structured output — native output_config.format (GA). - # `supports_structured_output` is False where the endpoint rejects the - # key (Bedrock); there the schema degrades to the prompt-based fallback - # below rather than putting an illegal field on the wire. - response_schema = kwargs.get("response_schema") - if response_schema and self.supports_structured_output: - # Merge, never assign: `effort` may already own output_config, and - # the API takes exactly one such object per request. - payload.setdefault("output_config", {})["format"] = { - "type": "json_schema", - "schema": self._ensure_additional_properties_false(response_schema), - } - elif (kwargs.get("json_mode") or response_schema) and user_messages: - # No native json_object mode in Anthropic — use prompt-based fallback. - # When a schema was requested but the endpoint cannot enforce it, the - # schema goes into the prompt: a bare "valid JSON" hint would satisfy - # the caller's parser only by luck. - last_msg = user_messages[-1] - if last_msg.get("role") == "user": - hint = "\n\nPlease respond with valid JSON only." - if response_schema: - hint = ( - "\n\nRespond with valid JSON only — no prose, no code fence — " - "conforming exactly to this JSON Schema:\n" - + json.dumps( - self._ensure_additional_properties_false(response_schema) - ) - ) - content = last_msg["content"] - if isinstance(content, list): - last_msg["content"] = content + [{"type": "text", "text": hint}] - else: - last_msg["content"] = str(content) + hint + # Structured output — native `output_config.format` where the endpoint + # enforces it, the schema-in-prompt fallback where it does not. Shared with + # the OAuth builder (see ``apply_structured_output``). + apply_structured_output( + payload, + user_messages, + response_schema=kwargs.get("response_schema"), + json_mode=bool(kwargs.get("json_mode")), + native=self.supports_structured_output, + ) # Handle tools - convert OpenAI format to Anthropic format # OpenAI: {"type": "function", "function": {"name": ..., "description": ..., "parameters": ...}} diff --git a/src/marsys/models/adapters/anthropic_oauth.py b/src/marsys/models/adapters/anthropic_oauth.py index 8111bb3e..4af3bf0f 100644 --- a/src/marsys/models/adapters/anthropic_oauth.py +++ b/src/marsys/models/adapters/anthropic_oauth.py @@ -10,6 +10,7 @@ CACHE_EXEMPT_KEY, _anthropic_model_rejects_temperature, _anthropic_model_requires_adaptive_thinking, + apply_structured_output, mark_conversation_tail_for_cache, ) from marsys.models.adapters.base import APIProviderAdapter, AsyncBaseAPIAdapter @@ -53,6 +54,14 @@ class AnthropicOAuthAdapter(APIProviderAdapter): # Enable streaming mode - Claude OAuth uses SSE streaming streaming = True + # Endpoint capability, not model capability — the same flag the api-key adapter + # declares, read by the shared ``apply_structured_output``. This endpoint IS the + # first-party Messages API, which enforces `output_config.format`: verified live + # 2026-08-07 (haiku 4.5 under the Claude Code prefix returned schema-conformant + # output). A leg that turns out not to enforce it flips this to False and inherits + # the schema-in-prompt fallback, the way Bedrock does. + supports_structured_output = True + # Anthropic's documented bounds for fixed-budget thinking: budget_tokens >= 1024 and # strictly less than max_tokens (thinking spends from the same output allowance). # Mirrors AnthropicAdapter — the two payload builders are kept deliberately parallel. @@ -626,25 +635,17 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An if anthropic_tools: payload["tools"] = anthropic_tools - # Handle structured output — native output_config.format (GA) - response_schema = kwargs.get("response_schema") - if response_schema: - payload["output_config"] = { - "format": { - "type": "json_schema", - "schema": self._ensure_additional_properties_false(response_schema) - } - } - elif kwargs.get("json_mode") and converted_messages: - # No native json_object mode — prompt-based fallback - last_msg = converted_messages[-1] - if last_msg.get("role") == "user": - hint = "\n\nPlease respond with valid JSON only." - content = last_msg.get("content") - if isinstance(content, list): - last_msg["content"] = content + [{"type": "text", "text": hint}] - elif isinstance(content, str): - last_msg["content"] = content + hint + # Structured output — the SAME implementation the api-key builder uses. This + # branch used to be a hand-copied twin that ASSIGNED `output_config`, dropping + # the `effort` set above it, and degraded a schema request to a bare "reply with + # JSON" nudge. + apply_structured_output( + payload, + converted_messages, + response_schema=kwargs.get("response_schema"), + json_mode=bool(kwargs.get("json_mode")), + native=self.supports_structured_output, + ) # The conversation-tail prompt-cache breakpoint (mirrors the api-key twin; # see ``mark_conversation_tail_for_cache``). Placed LAST, after the json-mode diff --git a/tests/models/test_oauth_claude5_payload_shape.py b/tests/models/test_oauth_claude5_payload_shape.py index bc9472ca..c0308c47 100644 --- a/tests/models/test_oauth_claude5_payload_shape.py +++ b/tests/models/test_oauth_claude5_payload_shape.py @@ -11,22 +11,63 @@ that is a deliberate choice, not a forced one. """ +import json +import time + import pytest +from marsys.models.adapters.anthropic import AnthropicAdapter from marsys.models.adapters.anthropic_oauth import AnthropicOAuthAdapter MESSAGES = [{"role": "user", "content": "hi"}] +SCHEMA = { + "type": "object", + "properties": {"units": {"type": "array", "items": {"type": "string"}}}, + "required": ["units"], +} + + +@pytest.fixture(autouse=True) +def _dummy_credentials(tmp_path, monkeypatch): + """A credentials file the real constructor can load, at the path it already reads from + the environment. The alternative — building the adapter through ``__new__`` and hand-setting + the five attributes the payload builder happens to read today — measures a hand-assembled + object: anything ``__init__`` does that shapes a payload (alias resolution, defaulting, a + capability read) is invisible to it, and the parity arms then compare that object against a + real ``AnthropicAdapter``. Nothing here reaches the network.""" + path = tmp_path / ".credentials.json" + path.write_text(json.dumps({"claudeAiOauth": { + "accessToken": "dummy-access-token", + "refreshToken": "dummy-refresh-token", + "expiresAt": int((time.time() + 3600) * 1000), + "subscriptionType": "max", + }})) + monkeypatch.setenv("CLAUDE_AUTH_PATH", str(path)) + + +def _oauth(model_name: str, *, budget: int = 0, enable: bool = False) -> AnthropicOAuthAdapter: + """The real constructor, on the dummy credentials above. ``auto_refresh`` is off because a + refresh is an OAuth round-trip against a token this file invented — the only step in + ``__init__`` a payload-shape test has to keep out of.""" + return AnthropicOAuthAdapter( + model_name=model_name, + max_tokens=8192, + temperature=0.7, + enable_thinking=enable, + thinking_budget=budget, + auto_refresh=False, + ) -def _oauth(model_name: str, *, budget: int = 0, enable: bool = False): - """Build the adapter without touching the credentials file on disk.""" - adapter = AnthropicOAuthAdapter.__new__(AnthropicOAuthAdapter) - adapter.model_name = AnthropicOAuthAdapter.MODEL_ALIASES.get(model_name, model_name) - adapter.max_tokens = 8192 - adapter.temperature = 0.7 - adapter.enable_thinking = enable - adapter.thinking_budget = budget - return adapter +def _api(model_name: str, *, max_tokens: int = 8192) -> AnthropicAdapter: + """The api-key twin, for the parity arms: the two builders are separate code and + only a test that drives BOTH can show they still agree.""" + return AnthropicAdapter( + model_name=model_name, + api_key="not-a-real-key", + base_url="https://api.anthropic.com/v1", + max_tokens=max_tokens, + ) @pytest.mark.parametrize("model_name", ["claude-opus-5", "claude-sonnet-5"]) @@ -98,3 +139,80 @@ def test_thinking_flag_without_budget_sends_no_null_budget(): adapter = _oauth("claude-haiku-4-5-20251001", enable=True, budget=0) payload = adapter.format_request_payload(MESSAGES) assert "thinking" not in payload + + +# --- structured output ------------------------------------------------------ + + +def test_schema_rides_output_config_natively(): + """This endpoint IS the first-party Messages API, so the schema goes on the wire + rather than into the prompt.""" + payload = _oauth("claude-opus-5").format_request_payload( + MESSAGES, thinking_budget=8192, response_schema=SCHEMA + ) + assert payload["output_config"]["format"]["type"] == "json_schema" + assert payload["output_config"]["format"]["schema"]["additionalProperties"] is False + assert payload["output_config"]["format"]["schema"]["required"] == ["units"] + # …and the request text is untouched: no prompt fallback rides along with it. + assert "JSON Schema" not in str(payload["messages"][-1]["content"]) + + +def test_effort_and_schema_share_one_output_config(): + """The regression this leg shipped: `output_config` was ASSIGNED here, so a request + carrying both a reasoning effort and a schema lost the effort silently. The API + takes exactly one such object per request — merge, never assign.""" + payload = _oauth("claude-opus-5").format_request_payload( + MESSAGES, thinking_budget=8192, reasoning_effort="low", response_schema=SCHEMA + ) + assert payload["output_config"]["effort"] == "low" + assert payload["output_config"]["format"]["type"] == "json_schema" + + +def test_a_leg_without_native_enforcement_puts_the_whole_schema_in_the_prompt(monkeypatch): + """The house fallback (Bedrock's convention): an endpoint that cannot enforce + schemas must not put the illegal field on the wire, and must not degrade the ask to + a bare "please emit JSON" — the caller's parser would only be satisfied by luck.""" + adapter = _oauth("claude-opus-5") + monkeypatch.setattr(adapter, "supports_structured_output", False, raising=False) + payload = adapter.format_request_payload( + MESSAGES, thinking_budget=8192, reasoning_effort="low", response_schema=SCHEMA + ) + assert "format" not in payload.get("output_config", {}) + assert payload["output_config"]["effort"] == "low" # the effort still survives + text = str(payload["messages"][-1]["content"]) + assert "JSON Schema" in text + assert '"properties"' in text and '"units"' in text + + +# --- parity between the two builders ---------------------------------------- + + +@pytest.mark.parametrize("model_name", ["claude-opus-5", "claude-sonnet-4-6"]) +@pytest.mark.parametrize("native", [True, False]) +def test_both_anthropic_builders_emit_the_same_structured_output_shape(model_name, native, monkeypatch): + """The two payload builders are separate code kept deliberately parallel, and this + is the branch where the parallelism drifted twice. Identical inputs must produce an + identical structured-output shape — native config and prompt fallback alike.""" + oauth, api = _oauth(model_name), _api(model_name) + for adapter in (oauth, api): + monkeypatch.setattr(adapter, "supports_structured_output", native, raising=False) + kwargs = dict(thinking_budget=8192, reasoning_effort="low", response_schema=SCHEMA) + oauth_payload = oauth.format_request_payload([dict(m) for m in MESSAGES], **kwargs) + api_payload = api.format_request_payload([dict(m) for m in MESSAGES], **kwargs) + + assert oauth_payload.get("output_config") == api_payload.get("output_config") + assert oauth_payload["messages"][-1]["content"] == api_payload["messages"][-1]["content"] + + +def test_both_builders_clamp_a_fixed_budget_to_the_same_number(): + """The repair path's landmine: a background model built at max_tokens=4096 with the + default 8192 budget. The api-key leg clamped and the OAuth leg did not, so the same + settings were legal on one leg and a 400 on the other.""" + oauth = _oauth("claude-haiku-4-5-20251001", budget=8192) + oauth.max_tokens = 4096 + oauth_payload = oauth.format_request_payload(MESSAGES, thinking_budget=8192) + api_payload = _api("claude-haiku-4-5-20251001", max_tokens=4096).format_request_payload( + MESSAGES, thinking_budget=8192 + ) + assert oauth_payload["thinking"] == api_payload["thinking"] + assert oauth_payload["thinking"] == {"type": "enabled", "budget_tokens": 3072}