fix(agent): name reasoning_effort in custom-provider stale timeouts - #101672
Open
itsflownium wants to merge 1 commit into
Open
itsflownium wants to merge 1 commit into
itsflownium wants to merge 1 commit into
Conversation
Contributor
Summary: Adds a Findings (all Non-blocking):
LGTM — mainly asking whether |
This branch has not been deployed
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.
Root cause (the diagnostic gap, not a code-change to the request)
vLLM / llama.cpp / SGLang endpoints can't be capability-probed like Ollama's
/api/show(#63315), so Hermes forwards the top-levelreasoning_effortfield toprovider: custombackends unconditionally (#90057). When the backend honours it as extended thinking, a request that took seconds can stall for minutes — with no error, no log line, and the only visible symptom a genericNon-streaming API call timed out after 180s. The reporter lost a fleet of cron jobs to this and had to trace it via manual curl timing experiments.The issue explicitly does NOT ask to gate the parameter (that would kill the feature for vLLM servers that do serve reasoning well) — it asks for the causal link to be visible when the stall happens (their suggestion 1).
Fix
run_agent.py: new_custom_reasoning_hang_hint(api_kwargs)— mirrors the existing_codex_silent_hang_hintshape ([Bug]: All openai-codex / gpt-5.5 primary calls hang silently for full stale timeout #21444). Returns an actionable message when a stale non-streaming call went to acustom/vLLM-family provider carrying a realreasoning_effort(top-level or nested inextra_body;"none"/empty is ignored), naming the effort value, the likely cause (extended-thinking stall vs Ollama's clean 400), the exact workaround (agent.reasoning_effort: ''), and the issue number.agent/chat_completion_helpers.py: new_stale_hang_hint(agent, api_kwargs)resolver (Codex heuristic first — the sharper diagnosis — then the custom-reasoning hint). Wired into all three stale-kill paths: the inlinedirect_api_callwatchdog and both worker-poll branches (TTFB and generic stale detector), replacing the two duplicated codex-only hint blocks. Behaviour for non-matching requests is byte-identical (hint isNone).Why this works
When the 180s/240s timeout now fires on a vLLM-backed custom provider with
reasoning_effort: mediumset, theRuntimeError, the warning log, and the status-line message all sayThis request forwarded reasoning_effort='medium' to a custom OpenAI-compatible endpoint … set agent.reasoning_effort: ''instead of the bare timeout — seconds to diagnose instead of days, and the field forwarding itself is untouched (vLLM reasoning servers keep working).Verification
uv run python -m pytest tests/run_agent/test_custom_reasoning_hang_hint.py tests/run_agent/test_codex_silent_hang_hint.py -o 'addopts=' -q→ 9 passed (7 new: hint fires with value, silent on none/empty/no-effort, non-custom provider excluded, extra_body nesting, codex precedence, fall-through, no-match → None; plus the 2 pre-existing Codex hint tests to prove no regression in the shared slot).Also run:
tests/agent/test_cascading_interrupt_6600.py→ 15 passed;tests/plugins -k custom→ 33 passed, 3 skipped.Not claimed: I could not reproduce the actual vLLM hang locally (no GPU/vLLM server here) — the tests pin the decision logic and message content, and the reporter's production evidence already documents the stall side.
Closes #100841