[Kh4L]

Problem

_postprocess_nemo_gym_to_nemo_rl_result asserts that each assistant turn's accumulated token IDs form a prefix of the next turn's prompt. In long multi-turn agentic rollouts, rare tokenization or re-rendering edge cases can break contiguity and fail the rollout task.

Current main already propagates these failures: the legacy path aborts on the first batch-worker failure, and the SC path re-raises after exhausting its data attempts. This PR does not fix a current async-GRPO stall. It adds an opt-in way to tolerate this one benign prefix-divergence case without increasing generic failure budgets.

Change

Add the opt-in env.nemo_gym.truncate_noncontiguous_episodes option. It defaults to false, preserving the existing assertion. When enabled, a non-contiguous turn drops the corrupted tail and returns the last contiguous prefix; that prefix remains trainable with the episode reward.

NemoGymPostprocessConfig owns the defaults. The shared _build_gym_actor_config consumes these fields and validates compatibility after each shard's overrides are merged, without forwarding NeMo-RL-only keys into Gym or mutating the caller's config. The redundant distillation check and dry-run prefetch plumbing have been removed.

Router replay and token-capture receipt mode are rejected with this option during actor-config construction. Router replay needs the next contiguous prompt to repair the retained prefix's final route; receipt mode bypasses the legacy token-prefix check.

Every legacy result includes noncontiguous_truncated (false normally, true when the tail is dropped), surfaced through the existing per-agent scalar metrics as {agent_name}/noncontiguous_truncated/mean.

This PR remains independent of #3127 and the SGLang/quantization stacks. Current head 13c931e0af7d2edcd7dcf8a685573a502ed7ce48 includes the review fixes in f8ea810ddbff2f536d1db7f92a2edb197b634d3c and main through e12d18ba8e115fdcb957329740dae1f12864cb53. Main's structured reasoning/answer token extraction is preserved alongside the truncation metric.

CI follow-up

The NeMo-Gym unit lane at bee756dc344b2e546866b5baee14269e9493a815 finished with 357 passes and one failure: test_run_async_nemo_gym_rollout omitted the six new per-agent noncontiguous_truncated summaries from its expected metric dictionary. Commit 13c931e0af7d2edcd7dcf8a685573a502ed7ce48 adds those six expected entries. Production code, the strict result comparison and all other expectations are unchanged. The updated key set matches the actual CI assertion payload; the old expectation reproduces that mismatch.

Current-head validation

On 13c931e0af7d2edcd7dcf8a685573a502ed7ce48:

• 112 focused CPU checks passed, zero failures/errors/skips: 44 source-selected repository cases and 68 normally imported shard-schema cases.
• Batch-decode coverage crosses both truncation settings with all four structured token-extraction cases.
• Repository-pinned Ruff 0.9.9 lint, import-order and formatting checks pass; the feature diff against the pinned main passes git diff --check.

Scope: source selection bypasses heavy module/conftest imports; actor lifecycle calls use the repository tests' mocks. These checks do not replace full normal-module CI, source-pinned dependency validation or GPU training. Fresh CI for this six-line test-only follow-up is pending. A maintainer must run /ok to test 13c931e0af7d2edcd7dcf8a685573a502ed7ce48; no current-head CI result is claimed here.

Historical evidence — not rerun for this head

The runs below predate the current head and do not validate the latest main merge or review fixes. Their observed stall behavior describes the historical code used for those runs, not current main.

Validated at scale on a 16-node Qwen3-30B MoE SWE-bench async-GRPO run with an SGLang generation backend: 63 training steps across four clean checkpoint/resume segments, about 200 graceful truncations, zero stalls, and reward convergence from 0.10 to a 0.22 plateau. Without the option, single corrupted episodes reproducibly stalled the step until walltime on that historical code.

Stacked HSG job 6595733 also completed successfully (0:0) on 2 nodes / 8 GB200 GPUs with Qwen2.5-Math-1.5B, 4-GPU Megatron training, four TP1 SGLang engines, and real stateful multi-turn NeMo-Gym async GRPO. It validated source 60140cd23496d80c3ae04cd3a5fc276c38d44a60, which contains the same #3063 feature increment as the direct-main head. This is integration evidence rather than exact-current-head evidence.

The strict validator passed 3/3 optimizer steps, 4/4 SGLang refits, and 24/24 trainable trajectory rows with nonzero token-loss masks. truncate_noncontiguous_episodes=true was resolved and forwarded correctly. This small deterministic sample did not encounter a non-contiguous episode, so the 16-node run above remains the branch-behavior evidence.

Tests

• Default-off discontinuities still assert; enabled truncation retains exactly the valid prefix and original reward.
• Absent/false/true option resolution and nonmutation of caller/Gym configuration.
• Router-replay and token-capture incompatibility checks after common and per-shard overrides.
• Truncation metric on every result, both mixed-batch orders, and stale-flag reset.
• Preservation of main's policy API-key, external-service readiness handling and structured token extraction.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[Kh4L]

/ok to test c0075c4

[Kh4L]

/ok to test ce38185

[Kh4L]

/ok to test 3bddb06

[Kh4L]

/ok to test

[copy-pr-bot]

/ok to test

@Kh4L, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

[Kh4L]

/ok to test 6b2eecc

[Kh4L]

/ok to test 6b2eecc

[Kh4L]

/ok to test

[copy-pr-bot]

/ok to test

@Kh4L, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

[Kh4L]

/ok to test 678dfc2

[Kh4L]

/ok to test 4dac1b5

[Kh4L]

/ok to test e448060

[copy-pr-bot]

/ok to test 60140cd23db2ca93d3e3d98957b9370e364e35e4

@Kh4L, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

[Kh4L]

/ok to test 60140cd

[Kh4L]

/ok to test 9468b2b

[Kh4L]

/ok to test 60140cd

[Kh4L]

Exact-head stacked hardware smoke passed.

• HSG job 6595733: COMPLETED 0:0, 2 nodes / 8 GB200 GPUs
• This exact head: 60140cd23496d80c3ae04cd3a5fc276c38d44a60
• 3/3 async-GRPO optimizer steps, 4/4 SGLang refits, and 24/24 trainable four-turn Gym trajectories
• truncate_noncontiguous_episodes=true resolved and forwarded correctly

The small deterministic sample did not hit a non-contiguous episode, so the 16-node run in the PR body remains the branch-behavior evidence; this run validates the exact stacked integration and no-regression path.

[Kh4L]

/ok to test 034f4a3

[Kh4L]

/ok to test 760e87f

[Kh4L]

/ok to test 34c67ac

[Kh4L]

Self-review summary for current head c3f3cd4c805bb69d681d2717a5b6441d132ae4f0:

I completed the full-feature source review at 760e87f49e83e310a0b90b818ee0e7d02345a378, then reviewed the latest main-merge and sharded Gym configuration changes at the current head. No remaining source-level correctness blockers were found in that reviewed scope.

Reviewed:

• Default-off behavior: discontinuities still raise unless truncation is explicitly enabled.
• Truncation stops before the first divergent trainable turn and drops the remaining tail, preserving the retained tokens/log probabilities and original episode reward; no prefix rescoring or reward zeroing.
• Router-replay and token-capture incompatibility guards, including the distillation policy guard.
• Runtime-option plumbing through the shared factory, single/sharded actors and prefetch, without forwarding NeMo-only options into Gym or mutating caller configuration.

Current-head validation: 88 focused tests passed, with zero failures/errors/skips: 21 source-selected config/factory cases and 67 normally imported pure-Python shard-schema cases. Ruff lint/import/format and diff checks also passed.

Scope: the latest review covered the integration delta, not a newly repeated whole-PR audit. The focused tests bypass heavy module/conftest imports and do not replace full CI or GPU training validation.

[Kh4L]

/ok to test c3f3cd4

[Kh4L]

/ok to test b720a03

[yuki-97]

Opt-in truncation of non-contiguous Gym episodes is a welcome escape hatch: a single bad multi-turn trajectory no longer stalls async GRPO, and the default keeps today's hard failure.

Scope: full diff at b720a03: the truncation path in _postprocess_nemo_gym_to_nemo_rl_result, the new runtime-option model and its validation, and the GRPO / single-controller / distillation / prefetch call sites.

Checked:

• Default-off behaviour: with the option unset the contiguity assert and its message are unchanged (nemo_gym.py:1287).
• Truncation semantics: the break drops exactly the divergent turn and everything after it, and since the first trainable turn is always contiguous, the retained prefix is never empty, so the no-generation-data error below cannot fire on this path.
• Putting the default on a BaseModel rather than at the call site is the right shape for a key that lives in the untyped env.nemo_gym dict, and the three keys are kept out of Gym's global config on both the unsharded and sharded paths.
• Incompatibility set: router replay and token_capture are the right two exclusions. The retained prefix's final route can only be repaired by the next turn's prefill (nemo_gym.py:1340-1345), and receipt mode never reaches the token-prefix check.

Remaining comments are on keeping the truncation plumbing in _build_gym_actor_config alone (dropping the distillation and prefetch copies), surfacing truncation as a per-agent metric, and removing a runtime route check that the build-time validator already covers.

[yuki-97]

This PR adds an opt-in that keeps the contiguous prefix of an episode when a later turn's token prefix diverges, while the description frames it as fixing a stall in async GRPO, which no longer happens on main: the legacy path aborts on the first batch-worker failure and the SC path re-raises once the data attempts are used up.
It would be good to update the description to say that the option tolerates this one benign case without raising the generic failure budgets, but this does not need to block the merge.

[yuki-97]

/ok to test bee756d

[Kh4L]

🤦‍♂️ closed by mistake