Repository navigation
Conversation
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.
Problem
chat()buildsresponse_format,top_p, reasoning and provider-routing options from the role configuration. On an ordinary transport exception (429, 503, connection error, or timeout) with no pinned provider order, it then resetkwargs = {}and retried immediately. A transient outage could therefore silently remove reasoning effort, sampling, and JSON output settings from the rest of that request attempt.The shipped Curriculum and Verifier roles use empty provider routing, so the affected path is part of the released configuration. The outer work-clock pause also treats this as successful generation.
Change
Remove the generic option-dropping branch. Every retry now sends the same complete request, consistent with the existing behavior for HTTP-200 embedded provider errors.
The four-attempt limit, backoff, error classification and
LLMTransportErrorcontract are unchanged. A provider that genuinely rejects an optional parameter still ends in the existing explicit fatal transport error; it is no longer silently converted into a different experiment. No role profiles, benchmark locks or public signatures change.Validation
Python 3.12.15, offline fake SDK client; no model, network, VM or benchmark calls.
llm/client.pytests/test_llm_response_errors.pyafter fixgit diff --checkNew cases parameterize 429, 503, connection failure and timeout across empty and pinned provider routing. They compare deep-copied full requests (history, exact image bytes, reasoning,
top_p, response format), then confirm the successful reply and provider provenance. Fatal 400/422 exhaustion cases confirm options survive all four attempts and the error remains non-recoverable.Windows cannot collect the official suite because Unix-only
fcntlis imported at module load. With that one module excluded, the final diagnostic has 640 passed, 141 failed, and 2 skipped. Re-running with the unmodified client and the same expanded tests produced 631 passed, 150 failed, and 2 skipped. All 141 final failures also occur in that baseline; six of the removed failures are the new retry regressions, and three vary with platform/test ordering. These diagnostics contain platform-specific symlink,fcntl, shell and path assumptions. No final-only failures were observed. Linux CI is the authoritative full-suite check; no green full-suite result is claimed here.