fix(litellm): retry transient upstream 5xx at the proxy#882
Conversation
…pass) The proxy deliberately runs num_retries=0 (#830: BenchFlow owns task-level retry classification; never multiply deterministic provider rejects). But a transient upstream InternalServerError is not a deterministic reject — and some agents silently swallow a passed-through 500 into a zero-event turn (observed: MiMo Code ending turns with no error on a deepseek 500, repro x3 in the 64-agent campaign log). Add the one narrow exception the #830 rationale permits: retry_policy {InternalServerErrorRetries: 2}. Every deterministic class (4xx, auth, content) keeps zero retries and fails fast. Companion: benchflow-ai/agents#47 consolidates mimo on the upstream-native `mimo acp` adapter (manifest takes the bare name; core wrapper = fallback).
bingran-you
left a comment
There was a problem hiding this comment.
Daily scan review (2026-07-03): requesting changes. The PR is correctly scoped to benchflow-ai/benchflow and CI is green, but the configured LiteLLM retry policy appears to be a no-op with the locked LiteLLM router path. Direct probing shows RetryPolicy(InternalServerErrorRetries=2) is accepted as config shape, but LiteLLM does not apply it for InternalServerError, so with BenchFlow keeping num_retries=0, transient 500s still resolve to no retry. The current tests only assert generated config shape, so they would pass even though the runtime retry behavior is unchanged. Please switch to a LiteLLM hook/patch/version path that actually retries 500s, and add behavioral coverage proving: 500 gets the intended retry count, while context-window/permanent 400 errors still follow the #830 fail-fast behavior.
|
Users Simulation review (2026-07-03): blocked. Targeted checks passed, but the intended retry fix appears to be a no-op with the pinned LiteLLM behavior:
Blocker: No trajectory artifacts were generated. |
|
Addressing blocker #882 (comment) and CHANGES_REQUESTED review #882 (review). Pushed commit f7e3267 to fix/consolidate-mimo-native. What changed:
Validation:
Leaving status:blocked in place until GitHub CI is green. |
|
Automation status update (2026-07-04): GitHub checks are green on |
|
Users Simulation automation review (2026-07-04): ready by simulation on current head The retry fix was validated at runtime rather than only by config shape:
Shared sweep canary produced a healthy Docker OpenHands + DeepSeek artifact bundle with ACP+LLM trajectories, |
|
Users Simulation automation review (2026-07-05): ready by simulation on head Validated CLI help, SDK LiteLLM config construction, Shared gates passed: uv sync, ruff, ty, 71-test CLI/task/usage slice, bench help/task check/SDK smoke. Live Docker/OpenHands + Gemini flash-lite canary is healthy: ACP+LLM trajectories, results.jsonl training_ready=1, reward 1.0, 24,869 tokens, $0.00079715, 69.8s at /tmp/benchflow-users-simulation-20260705/shared-openhands-gemini-flash-lite-default/2026-07-05__05-11-14/hello-world-task__40312589. OpenHands max-effort was quarantined because that ACP agent does not declare reasoning_effort=max. Labels: keep |
|
Users Simulation automation review (2026-07-06): ready by simulation on head Evidence:
No code blocker found. Live GitHub still shows |
|
Daily scan follow-up (2026-07-08): fixed the thermo-nuclear file-size blocker on current branch head The retry behavioral coverage added by this PR pushed Validation in isolated worktree
Keeping |
|
Users Simulation automation review (2026-07-08): simulation ready, waiting on human re-review on head Evidence:
Thermo-nuclear review: no blocker. The retry patch is isolated and direct. Minor cleanup opportunity only: Metadata caveat: GLM artifact token/timing coverage is healthy, but Labels: |
|
Users Simulation automation review (2026-07-10): blocked on head What passed:
Artifact gate:
Blocking thermo-nuclear finding:
Please add an explicit startup probe/fail-closed check, similar in spirit to the Bedrock preflight, or move the retry behavior into a normal startup path that cannot disappear silently. |
|
Users Simulation automation follow-up (2026-07-10): current head New evidence by inspection:
I am leaving |
|
Users Simulation automation follow-up (2026-07-10): ready by simulation on current head The retry-patch fail-open blocker from #882 (comment) is addressed by
Validation:
Moving the PR back to |
The one narrow exception #830's rationale permits:
num_retries: 0stays (deterministic rejects never multiply), but transient upstreamInternalServerErrors are absorbed (retry_policy: {InternalServerErrorRetries: 2}).Why: some agents silently swallow a passed-through 500 into a zero-event turn — observed as the MiMo Code flake (turn ends with no error on a deepseek 500, repro ×3 in the 64-agent campaign log). With the proxy absorbing transient 500s, the agent never sees them. 4xx/auth/content classes keep zero retries and fail fast.
Companion: benchflow-ai/agents#47 — mimo consolidates on the upstream-native
mimo acpadapter (manifest takes the baremimoname via the decoupled-manifest override; core wrapper remains only as fallback).tests/test_litellm_config.pyupdated + 20 green; ruff check+format clean.