Repository navigation
Conversation
ng_collect_rollouts merges each rollout's captured model calls, projects ng_trajectory and assembles ng_perf, but run_examples returns the raw /run result, so training frameworks that dispatch through it never see the model-server record of the rollout. Move those steps into a public enrich_run_result() used by the collector, and let run_examples apply it with enrich_results=True (optionally metadata-only, optionally deleting each rollout's capture file after the merge). Trajectory turns built from captured calls now also understand the Anthropic Messages dialect (Claude Code), mapping it onto Responses items; previously those turns had answer=None. Also name a trajectory by its explicit rollout id when the row carries no task index (run_examples rows need not): _trajectory_identity evaluated next()'s default eagerly and raised KeyError even for rows with a task_id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
marcromeyn
reviewed
Oct 27, 2026
marcromeyn
left a comment
Contributor
There was a problem hiding this comment.
Thanks, one shared implementation for ng_collect_rollouts and run_examples is the right base; I'd like to build the RL-path records (FEP-1907) on it. A few points, reviewed at 0b1ee91b:
- Rebase onto main. This head predates #5081 (merged Oct 26), which adds
record_omitted_rollouts: rows marked_ng_omitted_rolloutfor a try killed before it produced a result and for a row the dispatch budget never started, with readers taking a rollout's latest executed try. Theno_resultbranch here sits right next to that code, so worth checking after the rebase thatenrich_run_resultkeeps the same distinction (interrupted vs never started), since a framework calling it will see those results too. - Task id fallback. When a row has neither a task id field nor
_ng_task_index, the trajectory'stask_idbecomes the rollout id. That makes every repeat of a task look like a different task to anything grouping by task. I'd leavetask_idunset in that case (unknown rather than guessed), or have the caller stamp the task index. delete_capturewithinclude_payloads=False. The docstring says that without payloads "the bodies stay in the capture store", butdelete_capturethen removes that store, so the bodies are gone. Either refuse the combination or say in the docstring that deleting drops them.- Identity copy. Copying the row's rollout id, task, repeat and attempt index only when the result lacks them looks right. Once the capture-identity design lands (agent instance,
model_ref,weight_version), those belong in the same copy; no change needed now. - Small: the Anthropic fallback swallows every converter error silently. A debug log with the error type would help when turns lose their normalization.
Happy to look at the tests in more detail after the rebase.
This branch has not been deployed
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.
What
run_examplesreturns the raw/runresult, so a training framework that dispatches through it (NeMo-RL) never gets whatng_collect_rolloutsrecords for each rollout: the model server's captured calls, the normalizedng_trajectory, andng_perf. This PR exposes those collector steps and fixes two gaps that showed up once a framework uses them.enrich_run_result(row, result, *, capture_dirs, observability_enabled, include_payloads=True, rollout_latency_ms=None, delete_capture=False)(public, inrollout_collection.py). It runs the collector's per-rollout steps in place:merge_model_call_capture_into_record, including the Claude Code compaction association;_attach_trajectory_record;_attach_ng_perf.It copies the row's rollout identity onto the result first, because the capture key is read from the record. Reward and harness output are never touched.
delete_captureremoves the rollout's capture file after the merge.ng_collect_rolloutsnow calls this function, so there is one implementation and no behavior change.run_examples(..., enrich_results=False, include_model_call_payloads=True, delete_model_call_capture=False): an opt-in way to haverun_examplesapply the function to each result. With the default off, the documented "exactly what/runreturned" contract is unchanged.Anthropic Messages turns. Turns built from captured calls (
_turn_content) handled only Responses and Chat Completions, so every Claude Code turn hadanswer=None. Anthropic calls are now mapped onto Responses items withAnthropicConverter, which coverssystemin the question,thinkingas reasoning andtool_useasfunction_call. If the converter rejects a block type, the turn falls back to the raw blocks.Fix:
_trajectory_identityraisedKeyErrorwhenever a row had no_ng_task_index, even when it carriedtask_id. Python evaluates the default argument ofnext(...)eagerly.run_examplesrows need not have a task index, so this silently replaced the projection with atrajectory_projection_failedgap. The task id now falls back to the explicit rollout id.Why
NeMo-RL is adding a debug log of every Gym rollout (NVIDIA-NeMo/RL PR, linked below). The only complete record of what a rollout did is the model server's capture: every policy call, subagents, summarizers, and judge calls made through
ServerClient. RL reaches Gym throughrun_examples, so without this the log holds only each agent's self-report, and for many agents that is just the final answer.No issue filed. The motivation and design are in the RL PR and its plan.
Validation
tests/unit_tests/test_rollout_collection.py). The new classes areTestEnrichRunResult: merge, metadata-only plus deletion, identity-only without capture, a missing task index, andrun_examplesopt-in. AlsoTestAnthropicTurns:tool_use+thinking, and the unsupported-block fallback. Run inside the 20261002 nemo-rl nightly-gym image with a baked Gym venv:-k "EnrichRunResult or AnthropicTurns or TurnsFromModelCalls or capture or trajectory"→ 47 passed.ruff checkandruff format --checkon the changed files are clean.pre-commit run --all-fileswas not run.Rollout evidence
Nemotron 3.5 Nano RLVR single-controller smoke: 10 nodes, the amplified-dolphin blend across 11–12 agents including LLM-judge environments. It ran the 17abf2d port of this change through NeMo-RL's actor, calling
enrich_run_resultper result, withobservability_enabled:How NeMo-RL uses it
NVIDIA-NeMo/RL#4890 calls
enrich_run_resultdirectly in its NemoGym actor, on a shallow copy of each/runresult, rather thanrun_examples(enrich_results=True). That keeps the large call bodies in the actor's log, and the result sent on to the trainer is unchanged. The function only replaces top-level keys, so a shallow copy is safe.run_examples(enrich_results=True)remains for callers that want the enriched result itself.RL also clears each rollout's stale capture file before dispatch with
clear_model_call_captures_for_rollouts. Unlike the collector, RL can re-dispatch a row under the same rollout id.Compatibility
All new behavior is additive and opt-in.
ng_collect_rolloutsoutput is unchanged except for two cases:🤖 Generated with Claude Code