fix(aux): title generation recovers from stacked parameter rejections on primary and fallback routes (#78273, #72351, #109774, salvage #72515) - #113958
Merged
Conversation
…2351) Title generation hardcoded temperature=0.3 in its call_llm() call. Models like GPT-5.6 only accept their server-side default temperature and reject explicit values with "Unsupported value: 'temperature'". While call_llm() has a retry that strips temperature on error, the daemon thread races with session cleanup in short-lived CLI sessions, causing the retry to fail with a connection error. Fix: pass temperature=None so the provider uses its own default. This avoids the unsupported-temperature error entirely and eliminates the need for the retry path. Fixes #72351
…ack requests Reasoning models reject several request fields at once (gpt-5: temperature AND max_tokens, #78273), a reasoning-strip retry can then 400 on temperature (#72351), and strict-schema gateways such as Fireworks reject the generic extra_body.reasoning fallback with "Extra inputs are not permitted, field: 'reasoning'" (#109774) — a phrasing none of the unsupported-parameter predicates recognised, so the reasoning rung never fired and every title call 400'd. The parameter rungs were a fixed single-pass order (temperature → structured output → reasoning → max_tokens): a field rejected AFTER an earlier rung had already run was never stripped, and _param_rung_accepts did not admit a temperature 400 raised by a later retry. They now form a table walked until no rung matches, each field stripped at most once, so N rejected fields recover in N retries in whatever order the provider raises them. Fallback candidates (per-task chain, main chain, discovery) got a single shot with the caller's temperature/max_tokens/reasoning fields and raised on the first parameter 400; agent/auxiliary_fallback_recovery.py now runs the same parameter rungs around a candidate's request (sync + async), leaving auth / payment / connection errors to the caller's existing handling. Live: real gpt-5-mini title_generation call (reasoning_effort 400 → temperature 400 → 200), real gpt-5-mini fallback candidate sync+async (temperature 400 → 200), and a stand-in replaying Fireworks' documented 400 (reasoning stripped → 200) — all failed on origin/main.
૮ >ﻌ< ა ci reviewran on 9e6a818 — fix(aux): max_tokens rung retries even when the wire kwargs all good! |
A positional _LadderRoute(...) in auxiliary_fallback_recovery breaks as soon as the route tuple gains a field (#113968 adds timeout); construct by name so fields the parameter ladder never reads default to None regardless of tuple width.
…lds the route by name base_info is the endpoint identity every rung reads for per-route bookkeeping; the fallback ladder left it empty. The test mirrored the positional construction that broke under a wider tuple.
… reasoning_effort The pre-ladder max_tokens rung accepted payment|connection|rate_limit errors on its retry, so a 429 after a stripped retry reached the credential and provider-fallback rungs. The ladder's shared `_param_rung_accepts` dropped rate_limit: a 429 on the retry raised out of the primary call and skipped the whole fallback chain. Accept it there too, with a rung-level test that the stripped kwargs and the 429 are handed on rather than raised. The body claimed `Fixes #109774` while the aux client still sent the generic `extra_body.reasoning` to Fireworks on every call and only recovered reactively (an extra 400 round-trip per request). Port @huklaa's profile override from #109807: Fireworks documents top-level `reasoning_effort` (`none` disables thinking), and overriding `build_api_kwargs_extras` marks the profile reasoning-aware so the transport omits the generic fallback on this route. Extends the salvage with the effort mapping so enabled-with-effort takes the same wire, with a control that a profile-less route keeps the fallback. Salvages #109807 (@huklaa). Co-authored-by: Hukla <129692708+huklaa@users.noreply.github.com>
… carry the cap The rung table refused to re-send an unchanged request, but the Codex Responses route translates the caller cap away and gateways inject their own: the 400 names max_tokens while the kwargs show none, and the identical retry is what completes (tests/agent/test_injected_param_strip_retry_registry.py, red in CI on 99ff3b9).
teknium1
added a commit
that referenced
this pull request
Sep 17, 2026
…ved by the rung table Since #113958 every fallback candidate runs the shared parameter rung table (send_with_parameter_rungs), whose structured-output rung strips the field and records the rejection through its remember column. The fallback-side copy could never fire any more; delete it rather than keep two seams for one behaviour.
This was referenced Sep 17, 2026
teknium1
added a commit
that referenced
this pull request
Sep 17, 2026
…ved by the rung table Since #113958 every fallback candidate runs the shared parameter rung table (send_with_parameter_rungs), whose structured-output rung strips the field and records the rejection through its remember column. The fallback-side copy could never fire any more; delete it rather than keep two seams for one behaviour.
9 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auxiliary title generation (and every other aux task) now recovers when a model rejects more than one request field, when a fallback candidate rejects a field, and when a strict-schema gateway rejects the
reasoningfield — instead of surfacing a misleading first 400 and leaving the session untitled.agent/auxiliary_client.py::_ladder_parameter_rungs: the parameter rungs (temperature / structured-output format / reasoning field / output cap) are a table walked until no rung matches, each field stripped at most once — N rejected fields recover in N retries in whatever order the provider raises them._param_rung_acceptsnow admits a temperature 400 raised by a later retry.agent/auxiliary_client.py::_is_unsupported_parameter_errorrecognises the strict-pydantic phrasingExtra inputs are not permitted, field: '…', so the existing reasoning-strip rung fires for gateways that reject the genericextra_body.reasoningfallback.agent/auxiliary_fallback_recovery.py: fallback candidates (per-task chain, main chain, discovery; sync + async) run the same parameter rungs around their request; auth / payment / connection errors still reach the caller's existing refresh-and-quarantine handling.agent/title_generator.pysends the provider-default temperature instead of forcing0.3(salvage fix(agent): use provider-default temperature for title generation (#72351) #72515), so default-only models (gpt-5.x) accept the first title request and the daemon-thread retry no longer races CLI cleanup.Live repro:
gpt-5-minititle_generationcall_llm):reasoning_effort400 → reasoning stripped →temperature400 re-raised, task failed / after: reasoning stripped → temperature stripped → 200hello.gpt-5-minivia_call_fallback_candidate_syncand_asyncwithtemperature=0.3):BadRequestError: Unsupported value: 'temperature'raised, no retry / after: one retry without temperature → 200 on both.Extra inputs are not permitted, field: 'reasoning'— no Fireworks credential here, so a local OpenAI-compatible server replays the body logged in Auxiliary title_generation sends reasoning extra_body to providers without a reasoning-aware profile (Fireworks HTTP 400: Extra inputs are not permitted, field: 'reasoning') #109774; docs: https://docs.fireworks.ai/api-reference/post-chatcompletions): 1 request,HTTP 400, title failed / after: 2 requests, second withoutreasoning, 200 title.reasoning(the generic fallback is unchanged for gateways that accept it); the temperature-accepting path is untouched (tests/agent/test_unsupported_temperature_retry.pygreen).Root cause: the parameter rungs ran once in a fixed order, so a field rejected after an earlier rung had run was never stripped; fallback candidates bypassed the rungs entirely; and the strict-schema "Extra inputs are not permitted" phrasing matched no unsupported-parameter predicate.
Tests:
tests/agent/test_auxiliary_parameter_rung_chaining.py(2 invariants, red on origin/main via source swap); focused suitestest_unsupported_temperature_retry.py,test_auxiliary_auth_rung_fallthrough.py,test_title_generator.py,test_compression_fallback_budget.py,test_fast_compression_lane.py,test_auxiliary_client.py→ 307 passed.Fixes #78273
Fixes #72351
Fixes #109774
Salvages #72515 (@JonthanaHanh, cherry-picked) and #109807 (@huklaa, Fireworks profile override — ported in the review follow-up, extended with the effort mapping). Supersedes #78321 (@686f6c61, +259/-90 peel loop — same intent, landed here as the table walk on the current generator ladder) and #110809 (@sy0u1ti, host allow-list gate): the reactive rung covers every strict gateway without a per-vendor table.
Dropped hunks
max_tokens→max_completion_tokenstranslation on retry — the preflightauxiliary_max_tokens_paramalready picksmax_completion_tokensfor gpt-5-family model names; the retry drops the cap as before (re-probed: gpt-5-mini receivesmax_completion_tokenson the first request).extra_body.reasoning— replaced by the reactive strip so unknown strict gateways recover too.reasoning_effort(nonedisables thinking), so the profile override removes the extra 400 round-trip the reactive rung alone left in place.Review follow-up (99ff3b9):
_param_rung_acceptslacks_is_rate_limit_error(MAJOR) → fixed. Reproduced on c187a45 with the reviewer's probe (max_tokens strip, retry raisesopenai.RateLimitError429 → raised out of_ladder_parameter_rungs; origin/main's max_tokens rung accepted rate limits). Added_is_rate_limit_error(exc)to the predicate; invariant testtest_rate_limit_after_parameter_strip_falls_through_to_later_rungsintests/agent/test_auxiliary_parameter_rung_chaining.py(red on the old facade via source-file swap, green now).Fixes #109774while the genericextra_body.reasoningstill went to Fireworks unconditionally (MINOR) → fixed by porting fix(auxiliary): map Fireworks reasoning disable #109807 (@huklaa,Co-authored-by):FireworksProfile.build_api_kwargs_extrasemits top-levelreasoning_effort(nonefor thinking-off, the clamped effort otherwise) and marks the profile reasoning-aware, so the transport omits the generic fallback on that route. ParametrizedTestFireworksReasoning::test_auxiliary_reasoning_wire_shapecovers thinking-off, effort, and a control that a profile-less route still getsextra_body.reasoning.Fixes #109774stands._LadderRouteconstruction → refuted / already resolved on c187a45 (route built by field name; test builds it via_LadderRoute._fields).Infographic