fix: retry transient corruption in default (bash+edit) harness program#1955
Open
samsja wants to merge 1 commit into
Open
fix: retry transient corruption in default (bash+edit) harness program#1955samsja wants to merge 1 commit into
samsja wants to merge 1 commit into
Conversation
2865e35 to
d9fcd7f
Compare
ApprovabilityVerdict: Needs human review This PR reverses a deliberate design decision by adding retry logic where it was explicitly avoided to prevent 'dead-end branch' issues in trace handling. The runtime behavior change and its potential impact on the tracing system warrant human review. You can customize Macroscope's approvability policy. Learn more. |
Apply the same retryable-error fix from rlm-harness PR #111 to the default harness's program.py (the bash+edit chat loop): - Add _RETRYABLE tuple with the same error types: APIConnectionError, APITimeoutError, InternalServerError, NotFoundError, RateLimitError, APIResponseValidationError, ValidationError, ConnectionResetError - Add _RETRY_DELAYS backoff schedule (15, 30, 60, 90, 120s) - Wrap the chat() model call with retry-on-_RETRYABLE, mirroring rlm's call_with_retries - Update the max_retries=0 comment to explain the distinction: SDK retries stay off (avoid re-sampling committed turns), but transient corruption that fails *before* the turn is committed is safely retried Same rationale as PR #111: the interception server resets its pending-turn state before each model turn, so a retried call doesn't double-record a turn.
d9fcd7f to
df52693
Compare
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
The default harness (
verifiers/v1/harnesses/default/program.py) — the fallback harness withbashandedittools — makes model API calls withmax_retries=0and no retry logic. Intermittent corruption of model responses on a flaky interception tunnel crashes the whole rollout on a single bad response.A corrupted response surfaces as:
openai.APIResponseValidationError— SDK response-schema validation failurepydantic.ValidationError— raw schema failures (e.g.finish_reason='error')ConnectionResetError— raw resets / truncated streamsFix
Apply the same fix from rlm-harness PR #111 to the default harness program:
_RETRYABLEtuple with the same error types:APIConnectionError,APITimeoutError,InternalServerError,NotFoundError,RateLimitError,APIResponseValidationError,ValidationError,ConnectionResetError_RETRY_DELAYSbackoff schedule(15, 30, 60, 90, 120)— same as rlmchat()model call with retry-on-_RETRYABLE, mirroring rlm'scall_with_retriesmax_retries=0comment to explain the distinction: SDK retries stay off (avoid re-sampling committed turns), but transient corruption that fails before the turn is committed is safely retriedWhy this is safe
The interception server resets its pending-turn state before each model turn, so a retried call doesn't double-record a turn. This is the same reasoning as rlm-harness PR #111: corruption errors happen before the turn is committed (the response is malformed, so it can't be committed), so retrying is safe. Legitimate 4xx (auth, bad request) are NOT in
_RETRYABLEand fail fast.Verification
Note
Medium Risk
Changes rollout behavior under network or schema glitches—worst case adds several minutes of backoff per model turn—but scope is limited to the default harness program and mirrors an existing rlm-harness pattern.
Overview
Adds application-level retries around model calls in the default harness
chat()so a single corrupted or dropped response on a flaky interception tunnel no longer aborts the whole rollout.chat()now catches a fixed set of transient failures (APIConnectionError, timeouts, rate limits, 5xx,APIResponseValidationError, PydanticValidationError,ConnectionResetError), sleeps through backoff 15 → 30 → 60 → 90 → 120 seconds between attempts, then makes one finalchat.completions.createif all delayed retries fail. The OpenAI client still usesmax_retries=0so the SDK does not re-sample turns that may already be committed; only errors that fail before a valid turn is returned are retried.The previous inline comment that ruled out client-side retries was removed from
main(); behavior is unchanged there aside from relying on the newchat()loop.Reviewed by Cursor Bugbot for commit df52693. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Retry transient errors in default harness
chat()with exponential backoffWraps the
client.chat.completions.createcall in program.py with a retry loop over delays of 15, 30, 60, 90, and 120 seconds before a final attempt. Retries on OpenAI transient errors (APIConnectionError,APITimeoutError,InternalServerError,RateLimitError,APIResponseValidationError),pydantic.ValidationError, andConnectionResetError.Macroscope summarized df52693.