fix: clean stale rollouts on fresh runs - #2304
Merged
Merged
Conversation
Previously `clean_future_steps` only ran when resuming from a checkpoint, so a fresh run started in an output_dir containing stale rollouts or broadcasts from a previous run would consume them: the trainer would train on stale data and the orchestrator would compute a negative async level because it sees a trainer that is seemingly ahead of it. Run the same cleanup from step 0 when training from scratch so these artifacts are removed before training begins.
mikasenghaas
marked this pull request as ready for review
April 17, 2026 15:47
samsja
approved these changes
Apr 17, 2026
mikasenghaas
added a commit
that referenced
this pull request
Aug 9, 2026
Bumps deps/verifiers 29e3a0f7 -> a298bcfe, picking up two commits: a298bcfe fix(v1): restore the unbounded model-call timeout (#2304) 9ca7f5d6 Fix plain HTTP CONNECT for Docker framework routes (#2298) verifiers#2218 had consolidated client construction into build_async_openai/DEFAULT_TIMEOUT and, in doing so, replaced the relay client's explicit `timeout=None` with a 600s read timeout mirroring the OAI SDK default. That cap reached every model-call path, since DEFAULT_TIMEOUT feeds clients/eval.py as well as build_async_openai, which judge.py and clients/train.py use. 600s is fine for chat-shaped evals and wrong for RL: a single agentic turn on a long SWE trajectory routinely runs past ten minutes, and when it does the rollout dies as `ProviderError: Request timed out` even though the model was still producing. Because it truncates the longest trajectories specifically, the loss is biased rather than uniform. Seen on the laguna-s SWE RL run here. #2304 restores unbounded read/write/pool and keeps connect at 5.0 so an unreachable endpoint still fails fast. No lockfile change: verifiers is an editable path dependency (`source = { editable = "deps/verifiers" }`), and neither commit touches its pyproject, so only the submodule pointer moves. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Summary
output_dirthat still contains stale rollouts or broadcasts from a previous run (but no checkpoint), the trainer would consume those stale rollouts and the orchestrator would see a negative async level because the trainer appears ahead of it.clean_future_stepspreviously only ran when resuming from a checkpoint. Now it also runs from step 0 (i.e.resume_step=-1) when training from scratch, wiping every existing rollout/broadcast step directory before training begins.clean_future_steps' docstring to document the newresume_step=-1semantics.Repro
Note
Medium Risk
Moderate risk because it changes startup behavior to delete existing rollout/broadcast step directories, which could remove data if an
output_diris reused unexpectedly. The logic is localized to pre-run cleanup and should reduce mis-synchronization issues when starting from a dirty directory.Overview
Pre-run cleanup now always invokes
clean_future_steps: when resuming it deletes steps after the resolvedresume_step, and when training from scratch it callsclean_future_steps(..., -1)to wipe all existing rollout/broadcast step directories in theoutput_dir.Updates
clean_future_steps’s docstring to document the newresume_step=-1semantics for fresh runs.Reviewed by Cursor Bugbot for commit 8fb4011. Bugbot is set up for automated code reviews on this repo. Configure here.