Repository navigation
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. |
celve
force-pushed
the
LIN-2516/discard-infra-aborts
branch
2 times, most recently
from
November 1, 2026 13:44
bca819e to
8afa138
Compare
An episode that failed because the sandbox or the model service broke trained as a policy failure: the reward model scored it 0 and GRPO pushed down what the policy did. - An agent function raises InfraAbort(exit_status) to mark its sample ABORTED with the cause in metadata["exit_status"]; the cause survives the subprocess boundary. Miles's own session-path aborts record NoModelCalls, CollectFailed or AllTruncated. - apply_aborted_filter names that cause in its drop reason, group_has_aborted_<exit_status>, and rollout/dynamic_filter/drop_<reason> is now the one count of aborted groups, in both synchronous loops and the fully-async buffer, which drops its separate aborted_groups_filtered. - Evaluation treats an aborted attempt like any other without a reward: it scores 0 and counts in eval/<dataset>-none_reward_ratio. The zero-std count skips a group holding a None reward, which crashed evaluation when every attempt of a dataset aborted, and pass@k groups attempts by the dataset's own n_samples_per_eval_prompt rather than the global one. - The Harbor example discards only what the policy cannot cause: a trial that failed before its agent started, or an in-process agent that lost the model service. Timeouts, turn limits and verifier crashes stay scored 0.
celve
force-pushed
the
LIN-2516/discard-infra-aborts
branch
from
November 1, 2026 13:59
8afa138 to
f04ff77
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.
Layer P9 of the CUA upstreaming stack. It is built and reviewed in celve, and goes upstream with the whole stack once every layer is finished. Base:
main(radixark/miles6114c3a10f). Issue: LIN-2516, hub LIN-2504.Problem. An agentic episode that failed because the sandbox or the model service broke trains as a policy failure. An agent function can only return or raise, and either way the session it recorded is scored, usually 0, so GRPO pushes down whatever the policy did in an episode the infrastructure broke. When aborted groups are dropped, nothing says why. And evaluation crashes when every attempt of a dataset aborted.
Fix.
InfraAbort(exit_status, message)inmiles/rollout/agentic/agent_function.py. An agent function raises it to mark its sampleABORTED, with the cause inmetadata["exit_status"]. The cause survives thesubprocboundary. Miles's own session-path aborts now record a cause too:NoModelCalls,CollectFailed,AllTruncated. If collecting the samples also fails after anInfraAbort, the agent's cause wins. If collection succeeded, the collected samples' output-store refs are handed torecord_dropped_samples.apply_aborted_filteralready drops a group with an aborted sample before any dynamic filter runs. It now names the cause in its reason,group_has_aborted_<exit_status>, taken from the first aborted sample, orunknown. The existingrollout/dynamic_filter/drop_<reason>metric is now the one count of aborted groups, by cause. The fully-async buffer's aborted-group drop reports through the sameon_dynamic_filter_drop(reason), and its separateaborted_groups_filteredcounter is removed, so sync and fully async count aborts the same way._discard_causediscards a trial that failed before its agent started (SandboxUnavailableorAgentSetupFailed), and one whose in-process agent lost the model service (litellm'sAPIConnectionError/ServiceUnavailableError→ServerUnreachable). Multi-step trials are checked too. Timeouts, turn and length limits, and verifier crashes stay scored 0. Installed agents'Api*errors also stay scored 0, because Harbor gets them by regex over the agent's own output, which the policy writes.reward=None. The reward model skipsABORTEDsamples, so their reward staysNone, and the existing code already scores them 0, counts them ineval/<ds>-none_reward_ratioand logs its warning ("likely errored/aborted trials"). Nothing is excluded from the score or from pass@k, and no eval metric is added. Two fixes remain inmetrics.py:main, a dataset whose every attempt aborted raisesTypeError: type NoneType doesn't define __round__ methodin_compute_zero_std_metrics: all-Nonerewards look like a zero-std group, and theirNonegets rounded._is_zero_stdnow returns False for a group with aNonereward. This is the same one-line fix as @dzorlu's Eval metrics: a group with an aborted episode no longer crashes the zero-std count radixark/miles#4518.main: pass@k is grouped by the global--n-samples-per-eval-prompteven when the dataset sets its ownn_samples_per_eval_prompt. With 4 per dataset and 3 globally,compute_pass_rateasserts (len(flat_rewards)=8 num_groups=2 group_size=3). With 2 globally, it silently reports pass@1/pass@2 over the wrong groups._eval_samples_per_prompttakes the dataset's own count.agentic-rollout.mddocumentsInfraAbortand gains a "Which failures to discard" table, andfully-async.mddocuments the per-cause drop reason.Metric rename, for chart users.
rollout/fully_async/aborted_groups_filtered→ the sum ofrollout/dynamic_filter/drop_group_has_aborted_*.rollout/dynamic_filter/drop_group_has_aborted→rollout/dynamic_filter/drop_group_has_aborted_<cause>, which is..._unknownwhen the sample recorded no cause.Removed by Linyu's call: the
--max-consecutive-aborted-groupsstreak guard from #7, its separaterollout/aborted/drop_<cause>metric, and its evaluation exclusion (aborted_ratio,aborted/<cause>, attempt-based pass@k, no score for an all-aborted dataset).Tests (CPU devbox, SGLang
~/sglang, Megatron-LM, pytest threads capped at 2):main6114c3a10fand no fix:pytest tests/fast/ray/rollout/test_metrics.py tests/fast/rollout/test_filters.py tests/fast/rollout/test_fully_async_data_buffer.py tests/fast/rollout/test_fully_async_rollout.py tests/fast/rollout/inference_rollout/test_sample_completion_backfill.pygives 17 failed, 280 passed. The failures: the filter's reason carries no cause (test_filters.py×5); the sync loop reports onlydrop_group_has_aborted(test_sample_completion_backfill.py×2); the fully-async buffer counts aborts inaborted_groups_filteredrather than as a drop reason (test_fully_async_data_buffer.py×3,test_fully_async_rollout.py×5); a dataset whose every attempt aborted crashes evaluation withround(None); and pass@k is grouped by the global sample count (test_metrics.py×2). All pass on this head.f04ff77a6b):pytest tests/fast/ray/rollout/test_metrics.py tests/fast/rollout/{test_filters.py,test_fully_async_data_buffer.py,test_fully_async_rollout.py,inference_rollout,generate_hub,agentic} tests/fast/utils/test_arguments.py tests/fast/examples/experimental/{harbor,hud}gives 1,122 passed, 23 skipped, 5 failed. The 5 failures aretest_multi_turn.py[agentic_tool_call], which fail the same way onmain6114c3a10f(asession_idmetadata mismatch).pre-commit runon the changed files: passed, with no files changed.Limits.
GenerateFnOutputand gained contract tests, which require every early return to be aFAILEDwrite-off withremove_sample. The source's HUD hunks would contradict them, so they were dropped (Discard infrastructure failures in agentic training and evaluation #7 marked HUD runs that never launched, or whose first model request failed, asABORTED). Underremove_sample, such a sample is left out of the loss but still counts at 0 in its group's baseline.Related upstream work.
InfraAbort(exit_status)interface and failure vocabulary. This PR keeps that interface by design so the two can converge, and credits him for it.Api*errors for installed agents toServerUnreachable. This PR does not, because those errors come from a regex over output the policy writes. It discards only litellm's exceptions from in-process agents, plus failures before the agent started._is_zero_stdchange for the sameround(None)crash. Whichever lands first, the other drops that hunk. The regression test here driveslog_eval_rollout_datawith a dataset whose every attempt aborted.Origin: #7 (merge
e47e96e027), commits64e3f21d6b,28479341e7,0be4b3cdb7,4e82cfa0d4,5f53498151,f255defc33, squashed and ported ontomain, then reworked to reuse the existing aborted filter.