fix(aux): structured-output rejection no longer kills fallback candidates or costs a doomed first request
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 <Legion-is-life@users.noreply.github.com>
This commit is contained in:
@@ -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
|
||||
|
||||
79
agent/auxiliary_structured_output.py
Normal file
79
agent/auxiliary_structured_output.py
Normal file
@@ -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"}
|
||||
@@ -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)
|
||||
|
||||
@@ -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.)
|
||||
)
|
||||
|
||||
@@ -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.
|
||||
|
||||
101
tests/agent/test_auxiliary_structured_output.py
Normal file
101
tests/agent/test_auxiliary_structured_output.py
Normal file
@@ -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
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user