Skip to content

fix(aux): retry fallback candidates once without response_format on structured-output rejection - #92908

Closed
Legion-is-life wants to merge 1 commit into
NousResearch:mainfrom
Legion-is-life:fix/aux-fallback-structured-output-rejection
Closed

Legion-is-life wants to merge 1 commit into
NousResearch:mainfrom
Legion-is-life:fix/aux-fallback-structured-output-rejection

Conversation

@Legion-is-life

@Legion-is-life Legion-is-life commented Aug 23, 2026 •

Copy link
Copy Markdown

Problem

When a configured auxiliary endpoint (for example, a local Ollama instance) fails, call_llm can fall back to the main agent model. The fallback-candidate helpers (_call_fallback_candidate_sync and _call_fallback_candidate_async) previously special-cased auth errors only. Any other error—including DeepSeek's HTTP 400 "This response_format type is unavailable now", vLLM gateways without xgrammar, and strict Anthropic-wire gateways—escaped immediately and aborted the auxiliary task.

The primary call_llm / async_call_llm path already retries once without the structured-output field via _is_structured_output_rejection and _without_structured_output_format; the fallback-candidate path did not.

Live repro: title-generation request → local Ollama timeout → fallback to the main DeepSeek model → HTTP 400 on response_format → title generation fails. This fallback-candidate gap was reported in #83390 and documented in #83390 (comment).

Fix

  • Add the same structured-output rejection rung to both fallback-candidate helpers.
  • Retry once without response_format while preserving sibling extra_body fields.
  • If that retry exposes an auth error, continue through the existing credential-refresh rung without reintroducing response_format.
  • If that retry fails for another reason, propagate the retry error rather than the original structured-output rejection.

Tests

Regression coverage includes sync and async cases for:

  • retry success without response_format;
  • unrelated 400 responses remaining untouched;
  • propagation of a distinct second-attempt failure;
  • structured-output rejection → no-format retry → 401 → credential refresh, with the refreshed request still omitting response_format.

Verification on current main:

  • tests/agent/test_structured_output_rejection_retry.py: 32 passed;
  • broader auxiliary regression selection: 236 passed, 0 failed;
  • Ruff and git diff --check: passed.

Relationship to #85424

PR #85424 removes response_format from title generation, addressing the title-specific failure. This PR is complementary: it closes the generic fallback-candidate leak for other auxiliary tasks and plugin structured completions that legitimately keep structured output.

Related to #83390.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/deepseek DeepSeek API labels Aug 23, 2026
…tructured-output rejection

_call_fallback_candidate_sync and _call_fallback_candidate_async only
special-cased auth errors — any other error (including DeepSeek's HTTP 400
"This response_format type is unavailable now" on json_schema) re-raised
immediately and aborted the whole auxiliary task. The primary call_llm path
already retries once without the field; this mirrors that degradation for
the fallback path.

Affects every aux task whose fallback candidate rejects response_format:
title_generation, vision, compression, web_extract, plugin structured
completions, etc.

Closes the fallback-candidate gap not covered by NousResearch#85424 (which drops the
field for title generation but does not protect other aux tasks that
legitimately keep structured output).
@Legion-is-life
Legion-is-life force-pushed the fix/aux-fallback-structured-output-rejection branch from 05ee8bc to 31579ed Compare August 23, 2026 15:59
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference, author can ignore or act on any point.

Overall: correct parity fix — the fallback-candidate path now degrades response_format exactly like the primary path, and the auth_retry_extra_body threading ensures a post-auth-retry request doesn't resurrect the rejected field. The test class covering rejection→success, unrelated-400 passthrough, retry-failure propagation, and the auth-refresh interplay (tests/agent/test_structured_output_rejection_retry.py:205-273) is exemplary. Points:

  1. ~30 lines duplicated across sync/async — agent/auxiliary_client.py:5251-5280 vs agent/auxiliary_client.py:5390-5417: the detection + logging + auth_retry_extra_body bookkeeping is copy-paste modulo await. A small shared helper returning (should_retry, retry_kwargs) would keep the two rungs from drifting (e.g., if the rejection classifier later grows a second field-scrub). Not blocking, but this file already carries several such mirrored blocks.

  2. Retry inherits everything else in fb_kwargs, including params the first attempt may have partially mutated — agent/auxiliary_client.py:5254: confirm _without_structured_output_format() deep-copies or rebuilds rather than mutating fb_kwargs in place; if it mutates, a subsequent auth-refresh path building from the original kwargs would silently see the scrubbed version even where effective_extra_body is expected (your auth_retry_extra_body default suggests the invariant matters).

  3. Single-rung interaction worth documenting — after a successful no-format retry, schema enforcement relies purely on prompt compliance; downstream validators (_validate_llm_response) will surface malformed JSON as validation errors attributed to the model rather than to the degraded transport. Consider one line in the log message or docstring pointing at that consequence for debugging noisy tasks (title generation truncation etc.).

  4. Nit — the log message interpolates the raw exception at INFO (auxiliary_client.py:5259-5264); DeepSeek's message plus body can be long and repeat per fallback candidate. DEBUG might be the better home once the behavior stabilizes.

Nice catch on keeping sibling extra_body keys intact — the test asserting metadata survives the scrub pins an easy-to-break detail.

@Jeffgithub0029

Jeffgithub0029 commented Sep 17, 2026 •

Copy link
Copy Markdown

Thanks — this matches my current-main code-path verification. On current main, the structured-output rejection retry from #89589 is present on the primary call paths, while _call_fallback_candidate_sync and _call_fallback_candidate_async still re-raise the DeepSeek 400 instead of retrying without response_format. PR #92908 targets exactly that remaining gap.

I support the retry-without-response_format approach over a generic json_object downgrade. Direct DeepSeek accepts json_object, but other gateway/model combinations have returned empty content under that mode; omitting the field is the safer generic fallback.

Please rebase this PR onto current main (GitHub currently reports the branch as dirty) and rerun the sync/async fallback and auth-refresh regression tests, especially verifying that the scrubbed extra_body is preserved after credential refresh. Once those pass, I support this as the generic fallback-path fix. #85424 remains complementary for title-specific behavior.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @Legion-is-life — this landed on main via #113966 (5cc8177) with a Co-authored-by credit for your change. Closing this PR as merged-through-salvage; the fix is on main.

@teknium1 teknium1 closed this Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have provider/deepseek DeepSeek API type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants