Repository navigation
fix: drain full trajectory batches after completion bursts - #1124
original4422 wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5fd7e2524
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Hi @bradhilton, could you review this batching fix when you have time? The two automated timing concerns are addressed; the PR description includes the regression results and local validation limitations. Thanks! |
Summary
Keep completion bursts bounded while preserving rollout scheduling and failure checks between yielded batches.
Public signatures, skip behavior and exception handling are unchanged. Developed with AI assistance.
Evidence
[2, 6]; seven yielded[2, 5]and twelve[2, 2, 8]. Five burst regressions failed on main.After: batches stay bounded; all 12 public-API regressions and 3 existing localhost trajectory tests passed on macOS/Python 3.12. Event barriers also verify that completed pending failures propagate before another buffered batch and that new rollouts start before the backlog is drained.
uv run prek run --all-filespassed with ty explicitly targeting Linux, matching the quality CI container, via an external config preserving all other repository settings. Default macOS ty has four pre-existing Linux-only attribute errors; ruff/format/lock passed. No hooks skipped. This does not claim Linux runtime, the full manual unit suite or GPU/backend validation.Merge Danger
Door: two-way
Blast Radius: batching
Consumers receive bounded batches while rollout generation can overlap their processing.