From df1074b4e5fabed28cc00d6c0865e29bf1af3e61 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:55:22 -0700 Subject: [PATCH] fix(aux): structured-output rejection no longer kills fallback candidates or costs a doomed first request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two open atoms of #83390 (DeepSeek "This response_format type is unavailable now"): * `_call_fallback_candidate_sync/_async` only special-cased auth errors, so when the primary aux provider failed (timeout, rate limit, payment) and the fallback landed on a provider that rejects `json_schema`, the 400 re-raised and the whole task died — the primary-path rung from #89589 never applied there. Both fallback paths now retry once without `response_format`. * Every structured aux call (titles, kanban decomposer, goal judge, plugin structured calls) paid a guaranteed-fail request on providers that lack `json_schema` before the retry. A provider profile can now declare `unsupported_response_formats` (DeepSeek: json_schema, per https://api-docs.deepseek.com/guides/json_mode) and the recovery ladder remembers any route that rejected a type once (host:port scoped), so `_build_call_kwargs` — shared by the primary and fallback paths — omits the field before the first request. Dropping rather than downgrading to json_object matches the end state the retry already produced; json_object needs a JSON-mentioning prompt and some relays return empty content under it. New logic lives in agent/auxiliary_structured_output.py; the facade only gains the fallback rung next to the predicate it uses. tests/agent/conftest.py resets the process-level memo per test. Fixes #83390, #105191. Closes duplicates #84976, #88830, #102849, #113064. Co-authored-by: Legion-is-life --- agent/auxiliary_client.py | 55 ++++++++-- agent/auxiliary_structured_output.py | 79 ++++++++++++++ plugins/model-providers/deepseek/__init__.py | 3 + providers/base.py | 2 + tests/agent/conftest.py | 8 ++ .../agent/test_auxiliary_structured_output.py | 101 ++++++++++++++++++ .../developer-guide/model-provider-plugin.md | 1 + 7 files changed, 239 insertions(+), 10 deletions(-) create mode 100644 agent/auxiliary_structured_output.py create mode 100644 tests/agent/test_auxiliary_structured_output.py diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 8caace4019..524c0da390 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -25,6 +25,7 @@ from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple, TYPE_ from urllib.parse import urlparse, parse_qs, urlunparse from agent.error_classifier import _BILLING_PATTERNS, _OVERLOADED_PATTERNS +from agent.auxiliary_structured_output import remember_structured_output_rejection from agent.codex_headers import ( CODEX_AUX_BASE_URL as _CODEX_AUX_BASE_URL, apply_required_codex_headers as _apply_required_codex_headers, @@ -3281,6 +3282,22 @@ def _without_structured_output_format(kwargs: dict) -> Optional[dict]: return retry_kwargs if changed else None +def _fallback_structured_output_retry_kwargs( + fb_err: Exception, fb_kwargs: Dict[str, Any], task: Optional[str], fb_label: str, +) -> Optional[Dict[str, Any]]: + """Fallback candidates get the primary path's structured-output rung: a candidate that rejects + ``response_format`` (DeepSeek: "This response_format type is unavailable now") is retried once + without it instead of aborting the whole task after the primary provider already failed (#83390).""" + if not _is_structured_output_rejection(fb_err): + return None + retry_kwargs = _without_structured_output_format(fb_kwargs) + if retry_kwargs is not None: + logger.info("Auxiliary %s: fallback candidate %s rejected the structured-output format field; " + "retrying once without it (schema enforcement degrades to prompt compliance): %s", + task or "call", fb_label, fb_err) + return retry_kwargs + + def _is_reasoning_field_rejection(exc: Exception) -> bool: """Provider 400 rejecting a reasoning wire control by name (``reasoning_effort``, ``reasoning``, ``thinking``/``think``). Chat-only models behind OpenAI-compatible relays reject the top-level @@ -3999,6 +4016,11 @@ def _call_fallback_candidate_sync( try: return _send_recovering(fb_client, fb_kwargs, destination) except Exception as fb_err: + retry_kwargs = _fallback_structured_output_retry_kwargs(fb_err, fb_kwargs, task, fb_label) + if retry_kwargs is not None: + resp = _send(fb_client, retry_kwargs, destination) + remember_structured_output_rejection(destination.provider, destination.base_url, fb_kwargs) + return resp if not _is_auth_error(fb_err): capacity = fallback_candidate_unavailable_reason(fb_err) if capacity is None: @@ -4049,6 +4071,11 @@ async def _call_fallback_candidate_async( try: return await _send_recovering(fb_client, fb_kwargs, destination) except Exception as fb_err: + retry_kwargs = _fallback_structured_output_retry_kwargs(fb_err, fb_kwargs, task, fb_label) + if retry_kwargs is not None: + resp = await _send(fb_client, retry_kwargs, destination) + remember_structured_output_rejection(destination.provider, destination.base_url, fb_kwargs) + return resp if not _is_auth_error(fb_err): capacity = fallback_candidate_unavailable_reason(fb_err) if capacity is None: @@ -6471,7 +6498,11 @@ def _build_call_kwargs( reasoning_config = clamp_reasoning_config(reasoning_config) projection = _project_provider_profile(provider, provider_norm, model, effective_base, reasoning_config) kwargs.update(projection.top_level) - if merged_extra := _merge_aux_extra_body(extra_body, projection, reasoning_config, provider_norm): + merged_extra = _merge_aux_extra_body(extra_body, projection, reasoning_config, provider_norm) + if "response_format" in merged_extra: + from agent.auxiliary_structured_output import without_unsupported_response_format + merged_extra = without_unsupported_response_format(merged_extra, provider_norm, effective_base, task) + if merged_extra: kwargs["extra_body"] = merged_extra # Anthropic Messages adapters take reasoning via a private kwarg that plain OpenAI SDK clients # would reject; Portal Claude is dual-wire, so include it only when the catalog id selects @@ -7227,22 +7258,23 @@ def _is_max_tokens_rejection(exc: Exception, client: Any) -> bool: def _parameter_rungs(client: Any, max_tokens: Optional[int]) -> tuple: - """Ordered ``(matches, strip, log message)`` parameter rungs; ``strip`` returns None when the - field was not on the wire, so an unchanged request is never re-sent.""" + """Ordered ``(matches, strip, log message, remember)`` parameter rungs; ``strip`` returns None + when the field was not on the wire, so an unchanged request is never re-sent; ``remember`` + (optional) records the rejection per route so the next call omits the field up front.""" return ( (lambda exc: _is_unsupported_parameter_error(exc, "temperature"), _without_temperature, - "provider rejected temperature; retrying without it"), + "provider rejected temperature; retrying without it", None), (_is_structured_output_rejection, _without_structured_output_format, "provider rejected the structured-output format field; retrying without it " - "(schema enforcement degrades to prompt compliance)"), + "(schema enforcement degrades to prompt compliance)", remember_structured_output_rejection), # A chat-only model on an OpenAI-compatible relay rejects the profile's thinking-off encoding # (top-level ``reasoning_effort: none``), and strict-schema gateways reject the generic # ``extra_body.reasoning`` fallback outright (#109774); the caller only wanted "no thinking", # so retry with every reasoning field omitted and let the route default apply (#112781). (_is_reasoning_field_rejection, _without_reasoning_fields, - "provider rejected the reasoning field; retrying without it (route default applies)"), + "provider rejected the reasoning field; retrying without it (route default applies)", None), (lambda exc: max_tokens is not None and _is_max_tokens_rejection(exc, client), _without_max_tokens, - "provider rejected the output cap; retrying without it"), + "provider rejected the output cap; retrying without it", None), ) @@ -7257,17 +7289,20 @@ def _ladder_parameter_rungs( client, task, tag = route.client, route.task, route.tag rungs = list(_parameter_rungs(client, max_tokens)) while rungs: - hit = next(((matches, strip, message) for matches, strip, message in rungs - if matches(first_err) and strip(kwargs) is not None), None) + hit = next((rung for rung in rungs + if rung[0](first_err) and rung[1](kwargs) is not None), None) if hit is None: break rungs.remove(hit) - matches, strip, message = hit + matches, strip, message, remember = hit retry_kwargs = strip(kwargs) logger.info("Auxiliary %s%s: %s: %s", task or "call", tag, message, first_err) + rejection = first_err resp, first_err = yield from _rung( _LadderStep("call", (client, retry_kwargs)), _param_rung_accepts) if first_err is None: + if remember is not None: + remember(route.resolved_provider, route.base_info, kwargs) return resp, None, retry_kwargs kwargs = retry_kwargs return None, first_err, kwargs diff --git a/agent/auxiliary_structured_output.py b/agent/auxiliary_structured_output.py new file mode 100644 index 0000000000..27f929794a --- /dev/null +++ b/agent/auxiliary_structured_output.py @@ -0,0 +1,79 @@ +"""Structured-output (``response_format``) capability for auxiliary requests. + +Two sources decide whether an aux request may carry a ``response_format`` type up front: + +* the provider profile's ``unsupported_response_formats`` (DeepSeek's native API implements only + ``json_object`` — https://api-docs.deepseek.com/guides/json_mode — and answers ``json_schema`` with + HTTP 400 "This response_format type is unavailable now"), and +* a process-level memo of routes that already rejected a type once; the recovery ladder records the + route when its retry without the field succeeded. + +Either way the field is dropped before the first request instead of burning a guaranteed-fail +round-trip per call (#83390, #105191, #113064). Dropping — not downgrading to ``json_object`` — is the +same end state the rejection retry already produces: ``json_object`` needs the prompt to mention JSON +and some relays return empty content under it, so callers already tolerate prompt compliance. +""" +from __future__ import annotations + +import logging +from typing import Any, Dict, Optional +from urllib.parse import urlparse + +logger = logging.getLogger(__name__) + +# (route key, response_format type) pairs a provider rejected in this process. +_REJECTED_ROUTES: set[tuple[str, str]] = set() + + +def _route_key(provider: Optional[str], base_url: Optional[str]) -> str: + """Endpoint host:port when known (a base_url override turns a named provider into ``custom``; local + servers differ by port), else the provider name.""" + return (urlparse(base_url or "").netloc or "").lower() or str(provider or "").strip().lower() + + +def _response_format_type(request_kwargs: Dict[str, Any]) -> Optional[str]: + extra_body = request_kwargs.get("extra_body") + response_format = (extra_body or {}).get("response_format") if isinstance(extra_body, dict) else None + if response_format is None: + response_format = request_kwargs.get("response_format") + return response_format.get("type") if isinstance(response_format, dict) else None + + +def _profile_unsupported_formats(provider: Optional[str]) -> tuple: + try: + from providers import get_provider_profile + profile = get_provider_profile(str(provider or "").strip().lower()) + except Exception: + return () + return tuple(getattr(profile, "unsupported_response_formats", ()) or ()) if profile is not None else () + + +def remember_structured_output_rejection( + provider: Optional[str], base_url: Optional[str], rejected_kwargs: Dict[str, Any], +) -> None: + """Record that this route rejected the ``response_format`` type carried by *rejected_kwargs*.""" + format_type = _response_format_type(rejected_kwargs) + if format_type: + _REJECTED_ROUTES.add((_route_key(provider, base_url), format_type)) + + +def without_unsupported_response_format( + extra_body: Dict[str, Any], provider: Optional[str], base_url: Optional[str], task: Optional[str] = None, +) -> Dict[str, Any]: + """*extra_body* minus a ``response_format`` whose type this route is known to reject; unchanged otherwise.""" + response_format = extra_body.get("response_format") + format_type = response_format.get("type") if isinstance(response_format, dict) else None + if not format_type: + return extra_body + known_unsupported = ( + format_type in _profile_unsupported_formats(provider) + or (_route_key(provider, base_url), format_type) in _REJECTED_ROUTES + ) + if not known_unsupported: + return extra_body + logger.info( + "Auxiliary %s: %s does not accept response_format %s; sending without it " + "(schema enforcement degrades to prompt compliance)", + task or "call", _route_key(provider, base_url) or "provider", format_type, + ) + return {k: v for k, v in extra_body.items() if k != "response_format"} diff --git a/plugins/model-providers/deepseek/__init__.py b/plugins/model-providers/deepseek/__init__.py index 12837848ab..4179713813 100644 --- a/plugins/model-providers/deepseek/__init__.py +++ b/plugins/model-providers/deepseek/__init__.py @@ -48,6 +48,9 @@ deepseek = DeepSeekProfile( description="DeepSeek — native DeepSeek API", signup_url="https://platform.deepseek.com/", fallback_models=("deepseek-v4-pro", "deepseek-flash"), base_url="https://api.deepseek.com/v1", default_aux_model="deepseek-flash", + # Native API implements only ``json_object`` (https://api-docs.deepseek.com/guides/json_mode); + # ``json_schema`` is a guaranteed HTTP 400 "This response_format type is unavailable now". + unsupported_response_formats=("json_schema",), ) register_provider(deepseek) diff --git a/providers/base.py b/providers/base.py index 0941c5d8d7..fef8a65791 100644 --- a/providers/base.py +++ b/providers/base.py @@ -107,6 +107,8 @@ class ProviderProfile: # Temperature: None = use caller's default, OMIT_TEMPERATURE = don't send fixed_temperature: Any = None default_max_tokens: int | None = None + # ``response_format`` types the API rejects outright (e.g. ("json_schema",)); aux requests omit them up front. + unsupported_response_formats: tuple = () default_aux_model: str = ( "" # cheap model for auxiliary tasks (compression, vision, etc.) ) diff --git a/tests/agent/conftest.py b/tests/agent/conftest.py index 2bba1386ad..c29c0f26aa 100644 --- a/tests/agent/conftest.py +++ b/tests/agent/conftest.py @@ -23,6 +23,14 @@ from __future__ import annotations import pytest +@pytest.fixture(autouse=True) +def _fresh_structured_output_memo(monkeypatch): + """The aux client remembers routes that rejected ``response_format`` for the whole process; + a rejection recorded by one test must not strip the field from the next test's request.""" + from agent import auxiliary_structured_output + monkeypatch.setattr(auxiliary_structured_output, "_REJECTED_ROUTES", set()) + + @pytest.fixture(autouse=True) def _fast_retry_backoff(request, monkeypatch): """Short-circuit retry backoff for all tests in this directory. diff --git a/tests/agent/test_auxiliary_structured_output.py b/tests/agent/test_auxiliary_structured_output.py new file mode 100644 index 0000000000..6a022e67da --- /dev/null +++ b/tests/agent/test_auxiliary_structured_output.py @@ -0,0 +1,101 @@ +"""Structured-output (``response_format``) handling on auxiliary routes that reject it. + +Covers NousResearch/hermes-agent#83390 / #105191 / #113064: a fallback candidate that rejects +``json_schema`` gets the same retry-without-the-field rung as the primary path instead of aborting +the task, and a route known to reject a ``response_format`` type (provider profile or a rejection +already seen in this process) never pays the guaranteed-fail first request. +""" +import asyncio +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from agent import auxiliary_structured_output as structured_output +from agent.auxiliary_client import ( + _build_call_kwargs, + _call_fallback_candidate_async, + _call_fallback_candidate_sync, +) + +_JSON_SCHEMA = {"type": "json_schema", "json_schema": {"name": "t", "strict": True, "schema": {"type": "object"}}} +_DEEPSEEK_400 = ("Error code: 400 - {'error': {'message': 'This response_format type is unavailable now', " + "'type': 'invalid_request_error', 'param': None, 'code': 'invalid_request_error'}}") + + +class _Rejects400(Exception): + status_code = 400 + + +def _ok_response(): + return SimpleNamespace(choices=[SimpleNamespace(message=SimpleNamespace(content='{"title": "x"}'))]) + + +def _rejecting_client(base_url, *, async_mode=False): + """Fake OpenAI client: 400 while ``extra_body.response_format`` is present, 200 once it is gone.""" + client = MagicMock(base_url=base_url) + sent = [] + + def create(**kwargs): + sent.append(kwargs) + if "response_format" in (kwargs.get("extra_body") or {}): + raise _Rejects400(_DEEPSEEK_400) + return _ok_response() + + async def acreate(**kwargs): + return create(**kwargs) + + client.chat.completions.create.side_effect = acreate if async_mode else create + return client, sent + + +@pytest.mark.parametrize("async_mode", [False, True]) +def test_fallback_candidate_retries_without_response_format_on_rejection(async_mode): + """The fallback path used to re-raise the 400; now it degrades to prompt compliance like the primary path.""" + client, sent = _rejecting_client("http://relay.example:8080/v1", async_mode=async_mode) + call = _call_fallback_candidate_async if async_mode else _call_fallback_candidate_sync + common = dict( + task="title_generation", messages=[{"role": "user", "content": "Reply with JSON only"}], + temperature=None, max_tokens=64, tools=None, effective_timeout=30.0, + effective_extra_body={"response_format": dict(_JSON_SCHEMA)}, reasoning_config=None, + ) + result = call(client, "relay-model", "fallback_chain[0](custom)", **common) + if async_mode: + result = asyncio.run(result) + + assert result.choices[0].message.content == '{"title": "x"}' + assert [("response_format" in (k.get("extra_body") or {})) for k in sent] == [True, False] + # The rejection is remembered: the next request to this route skips the field up front. + retry_kwargs = _build_call_kwargs( + "custom", "relay-model", [{"role": "user", "content": "hi"}], + extra_body={"response_format": dict(_JSON_SCHEMA)}, base_url="http://relay.example:8080/v1") + assert "response_format" not in retry_kwargs.get("extra_body", {}) + + +def test_known_unsupported_route_skips_response_format_before_first_request(): + """DeepSeek's profile declares json_schema unsupported; json_object and other providers are untouched.""" + messages = [{"role": "user", "content": "hi"}] + deepseek = _build_call_kwargs( + "deepseek", "deepseek-flash", messages, extra_body={"response_format": dict(_JSON_SCHEMA)}, + base_url="https://api.deepseek.com/v1", task="title_generation") + assert "response_format" not in deepseek.get("extra_body", {}) + + json_object = _build_call_kwargs( + "deepseek", "deepseek-flash", messages, extra_body={"response_format": {"type": "json_object"}}, + base_url="https://api.deepseek.com/v1") + assert json_object["extra_body"]["response_format"] == {"type": "json_object"} + + openai_kwargs = _build_call_kwargs( + "openai", "gpt-5-mini", messages, extra_body={"response_format": dict(_JSON_SCHEMA)}, + base_url="https://api.openai.com/v1") + assert openai_kwargs["extra_body"]["response_format"] == _JSON_SCHEMA + + # A remembered rejection is scoped to the endpoint (host:port), not to every local server. + structured_output.remember_structured_output_rejection( + "custom", "http://127.0.0.1:1234/v1", {"extra_body": {"response_format": dict(_JSON_SCHEMA)}}) + same = _build_call_kwargs("custom", "m", messages, extra_body={"response_format": dict(_JSON_SCHEMA)}, + base_url="http://127.0.0.1:1234/v1") + other = _build_call_kwargs("custom", "m", messages, extra_body={"response_format": dict(_JSON_SCHEMA)}, + base_url="http://127.0.0.1:11434/v1") + assert "response_format" not in same.get("extra_body", {}) + assert other["extra_body"]["response_format"] == _JSON_SCHEMA diff --git a/website/docs/developer-guide/model-provider-plugin.md b/website/docs/developer-guide/model-provider-plugin.md index df11914f3f..3b97c05798 100644 --- a/website/docs/developer-guide/model-provider-plugin.md +++ b/website/docs/developer-guide/model-provider-plugin.md @@ -104,6 +104,7 @@ Full definition in `providers/base.py`. The most useful ones: | `default_headers` | `dict[str, str]` | Sent on every request (e.g. Copilot's `Editor-Version`) | | `fixed_temperature` | Any | `None` = use caller's value; `OMIT_TEMPERATURE` sentinel = don't send temperature at all (Kimi) | | `default_max_tokens` | `int \| None` | Provider-level max_tokens cap (Nvidia: 16384) | +| `unsupported_response_formats` | `tuple` | `response_format` types the API rejects outright; auxiliary requests omit them instead of paying a guaranteed 400 (DeepSeek: `("json_schema",)`) | | `default_aux_model` | str | Cheap model for auxiliary tasks (compression, vision, summarization) | ## Overridable hooks