Skip to content

fix(gym): make the gym venv prefetch dry run pass again - #3785

Merged
yuki-97 merged 5 commits into
mainfrom
yifu/fix-gym-prefetch-dry-run
Aug 25, 2026
Merged

fix(gym): make the gym venv prefetch dry run pass again#3785
yuki-97 merged 5 commits into
mainfrom
yifu/fix-gym-prefetch-dry-run

Conversation

@yfw

@yfw yfw commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

What does this PR do ?

Gym d980441d7 ("fix(cli): fail a dry run when a server's venv setup exits non-zero", NVIDIA-NeMo/Gym#2476) turned previously-swallowed dry-run failures into hard errors. Six servers in the prefetch configs had been failing all along; once that commit arrived via a Gym submodule bump, the rl-gym image build started failing with:

RuntimeError: Dry run failed to set up 6 servers.
  `prefetch_local_vllm_model` / `policy_model_reasoning_off` / `judge_model`
  / `safety_judge_model` / `swe_agents_val` / `swe_agents_train` exited with 1

Two independent causes, both ours:

  1. Missing required config fields. Gym's VLLMModelConfig declares return_token_id_information and uses_reasoning_parser without defaults, and LocalVLLMModelConfig additionally requires vllm_serve_env_vars. The thin judge/reasoning-off clients and prefetch_local_vllm_model in prefetch_{super,ultra}_all_envs.yaml omitted them, so model_validate raised. Gym builds each server as setup_env_command(...) && ... python app.py (nemo_gym/cli/env.py:417-420), so the venv was built successfully and only the exit code was wrong -- this cause never damaged a venv. Cause 2 below is the one that leaves an image needing a rebake, since r2egym really was left uninstalled. Other RL configs (e.g. nemotron-3-super/ stage1_rlvr.yaml) already set these; the prefetch configs had simply drifted.

  2. UV_LINK_MODE leaking into Gym's setup scripts. docker/Dockerfile exports UV_LINK_MODE=symlink for the gym prefetch RUN, and prefetch_venvs.py forwards the whole os.environ into the Gym Ray actor, so it reached swe_agents' r2e_gym.sh, which runs uv pip install --no-cache. uv rejects that pairing:

    Symlink-based installation is not supported with --no-cache.

    leaving r2egym uninstalled and SWEBenchWrapper asserting. Pin UV_LINK_MODE to hardlink for the Gym actor only, rather than changing the Dockerfile, so the outer prefetch keeps symlink semantics. hardlink is what the Dockerfile already uses for comparable nested builds and, unlike symlink, survives uv cache pruning. It is as space-efficient as copy but not as symlink: across Docker layers, linking out of the lower-layer uv cache makes overlayfs copy the bytes up. There is no cheaper mode available -- symlink is what breaks under --no-cache, and clone still copies up from a lower layer. Note docs/guides/models/nemotron/nemotron-3-super.md invokes this script the same way, which is why the fix belongs in the script rather than the Dockerfile. Upstream, r2e_gym.sh is the only uv --no-cache in Gym while its siblings swebench.sh / swebench_multilingual.sh omit it, so an upstream fix there would retire this pin.

prefetch_omni_envs.yaml needs no change; it defines no thin vllm clients.

Verified the YAML parses and every previously-missing field is present, and added
tests/unit/environments/test_nemo_gym_prefetch_configs.py, which runs Gym's own
model_validate over every standalone server block in examples/nemo_gym/prefetch_*.yaml
so this class of drift fails at PR time rather than in a manual image build. The test
deliberately does not restate the field list, so it tracks the Gym submodule pin.

Limits, kept honest: the test does not cover the UV_LINK_MODE/r2e_gym.sh half at all,
and passing it does not imply the image build succeeds. The build itself is the real test
for the link-mode half.

Issues

List issues that this PR closes (syntax):

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you run the unit tests and functional tests locally? Visit our Testing Guide for how to run tests
  • Did you add or update any necessary documentation? Visit our Document Development Guide for how to write, build and test the docs.

Additional Information

  • ...

Gym d980441d7 ("fix(cli): fail a dry run when a server's venv setup exits
non-zero", NVIDIA-NeMo/Gym#2476) turned previously-swallowed dry-run failures
into hard errors. Six servers in the prefetch configs had been failing all
along; once that commit arrived via a Gym submodule bump, the rl-gym image
build started failing with:

    RuntimeError: Dry run failed to set up 6 servers.
      `prefetch_local_vllm_model` / `policy_model_reasoning_off` / `judge_model`
      / `safety_judge_model` / `swe_agents_val` / `swe_agents_train` exited with 1

Two independent causes, both ours:

1. Missing required config fields. Gym's VLLMModelConfig declares
   return_token_id_information and uses_reasoning_parser without defaults, and
   LocalVLLMModelConfig additionally requires vllm_serve_env_vars. The thin
   judge/reasoning-off clients and prefetch_local_vllm_model in
   prefetch_{super,ultra}_all_envs.yaml omitted them, so model_validate raised
   before the venv was built. Other RL configs (e.g. nemotron-3-super/
   stage1_rlvr.yaml) already set these; the prefetch configs had simply drifted.

2. UV_LINK_MODE leaking into Gym's setup scripts. docker/Dockerfile exports
   UV_LINK_MODE=symlink for the gym prefetch RUN, and prefetch_venvs.py forwards
   the whole os.environ into the Gym Ray actor, so it reached swe_agents'
   r2e_gym.sh, which runs `uv pip install --no-cache`. uv rejects that pairing:

     Symlink-based installation is not supported with `--no-cache`.

   leaving r2egym uninstalled and SWEBenchWrapper asserting. Pin UV_LINK_MODE
   to hardlink for the Gym actor only, rather than changing the Dockerfile, so
   the outer prefetch keeps symlink semantics. hardlink is equally space
   efficient, is what the Dockerfile already uses for comparable nested builds,
   and unlike symlink survives uv cache pruning.

prefetch_omni_envs.yaml needs no change; it defines no thin vllm clients.

Verified the YAML parses and every previously-missing field is present. The
build itself is the real test for the link-mode half.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown

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.

@yfw yfw left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is correct, minimally scoped, and in the right place. Two inline comments below: one test gap (worth acting on), one comment-wording fix. Nothing blocking.

Both causes verified end-to-end:

  • Cause 1 — 3-way check against the real gate (VLLMModelConfig.model_validate): PR head ACCEPTED, fields-deleted REJECTED on exactly ['return_token_id_information', 'uses_reasoning_parser']. Note Gym's parse_no_environment alone does not reject, so the parser isn't the gate — only model_validate is.
  • Cause 2 — reproduced on uv 0.11.19: UV_LINK_MODE=symlink + --no-cacheerror: Symlink-based installation is not supported with --no-cache.; hardlink installs fine. r2e_gym.sh is genuinely reached in a dry run: SWEBenchWrapper.model_post_init calls all four dataset processors' setup() unconditionally, so dataset_path: /dev/null doesn't matter. The "SWEBenchWrapper asserting" wording is literally accurate — app.py:447 is assert return_code == 0.
  • Coverage is complete. Walking Gym's config MRO, the only other default-less fields are host/port/name (launcher-injected) and entrypoint (already present), so these three were exactly the full set — the fix isn't one field short of a second failure. These two prefetch files are also the only configs in the repo that define these servers from scratch; every other apparent gap inherits via _copy: or defaults:. prefetch_omni_envs.yaml correctly needs no change.
  • The values can't bake a wrong artifact. They match Gym's own bundled local_vllm_model.yaml exactly (false/false/{}), and the venv is provably independent of them: the generated build command is byte-identical whichever way they're set, and all three thin clients share one venv keyed by server directory.

One wrong clause in the commit message. "so model_validate raised before the venv was built" is reversed — Gym builds the subprocess as setup_env_command(...) && ... python app.py (cli/env.py:417-420), so the venv was built successfully and only the exit code was wrong. The consequence matters more than the wording: cause 2 is why existing prefetched images need rebaking (r2egym really was left uninstalled); cause 1 never damaged a venv. The rest of the message holds, including "other RL configs already set these" — nemotron-3-super/stage1_rlvr.yaml sets all three.

Also checked clean: pre-commit (all hooks), PR title, sign-off, copyright, no paired tests/unit/reference_configs/ twin, and the two YAMLs' added blocks are byte-identical to each other. No performance evidence needed — build fix, no perf claim made.

Generated by Claude Code

Comment thread examples/nemo_gym/prefetch_super_all_envs.yaml
Comment thread examples/nemo_gym/prefetch_venvs.py Outdated
yfw and others added 2 commits August 23, 2026 23:43
Addresses review feedback on #3785.

examples/nemo_gym/prefetch_*.yaml are only exercised by the rl-gym image build
(docker/Dockerfile, gated on NEMO_GYM_PREFETCH_CONFIGS), which no PR-time CI job
runs, so a Gym field gaining a new default-less entry breaks the image build
weeks later rather than the PR that drifts away from it. Neither existing gate
covers these files: tests/unit/test_config_validation.py globs examples/configs
only, and tests/unit/utils/test_config.py validates against RL's MasterConfig,
which could not catch a failure in Gym's VLLMModelConfig.

Add tests/unit/environments/test_nemo_gym_prefetch_configs.py, which runs Gym's
own model_validate over every standalone model server block in those configs. It
deliberately does not restate the field list, so it tracks the submodule pin and
cannot go stale. Blocks that are overlays onto a bundled config (no entrypoint)
or that carry _copy/_inherit_from are skipped, since Gym merges a source server
in before validating and the literal block is not self-contained.

Also correct the UV_LINK_MODE comment. "hardlink is equally space-efficient" is
true against copy but false against symlink: the uv cache is populated in the
hermetic stage and the gym prefetch RUN is a later layer, so hardlinking out of
it makes overlayfs copy the bytes up -- exactly what symlink avoided. hardlink
remains the right call (symlink breaks under --no-cache, clone still copies up),
so only the wording changes. Note the second caller in the nemotron-3-super
guide, and that the explicit override is load-bearing rather than redundant:
Ray merges runtime_env env_vars per key over the inherited environment, so
filtering the key out instead would leave the raylet's symlink in place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
@yfw yfw added the CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) label Aug 24, 2026
@yfw

yfw commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 0473d40

@yfw
yfw marked this pull request as ready for review August 24, 2026 07:24
@yfw
yfw requested review from a team as code owners August 24, 2026 07:24

@yuki-97 yuki-97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some nits.

Comment thread tests/unit/environments/test_nemo_gym_prefetch_configs.py
Comment thread examples/nemo_gym/prefetch_super_all_envs.yaml Outdated
`spinup_server` is not a field Gym has ever read. It is absent from
nemo_gym/server_utils.py at both the pin it was written against
(d67ad6611, via #2680) and the current pin (c3bac9631), and nothing on the
config MRO overrides pydantic's default extra="ignore" -- the only model_config
in the chain is ConfigDict(arbitrary_types_allowed=True) at server_utils.py:295
-- so the key was silently discarded on every load.

What actually decides whether a server is launched is the presence of an
entrypoint (cli/env.py:407, unchanged since d67ad6611:188). These thin clients
carry one, so they do spin up; the dry run simply returns early at
server_utils.py:698 before serving. The conclusion in the previous comment was
right -- validation is unconditional -- but the reason given described a
mechanism that does not exist, which made a dead key read as load-bearing.

Drop the six occurrences and reword the comment to the real mechanism. Upstream
Gym's own resources_servers/jailbreak_detection/configs/safety_judge_model.yaml
defines the same server without the key, and it has no reader anywhere in this
repo.

No behaviour change: tests/unit/environments/test_nemo_gym_prefetch_configs.py
still reports 7 passed, 2 skipped against the edited configs, which is the
expected result if the key was genuinely inert.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
The docstring said prefetch_local_vllm_model 'is skipped where vllm is absent',
which reads as environment-dependent. In CI it is unconditional:
L0_Unit_Tests_Nemo_Gym.sh is the only shard collecting nemo_gym-marked tests and
runs 'uv run --extra nemo_gym', vllm is a separate extra, and Gym's
local_vllm_model/app.py imports it at module scope. vllm_serve_env_vars --
required only by LocalVLLMModelConfig, and one of the three fields this PR adds
-- therefore has no PR-time guard.

Spell that out and link the tracking issue so a green run is not misread as
covering the local half.

Tracked in #3806

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
@yfw

yfw commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test b212df0

yfw added a commit that referenced this pull request Aug 25, 2026
Addresses review feedback on #3785.

examples/nemo_gym/prefetch_*.yaml are only exercised by the rl-gym image build
(docker/Dockerfile, gated on NEMO_GYM_PREFETCH_CONFIGS), which no PR-time CI job
runs, so a Gym field gaining a new default-less entry breaks the image build
weeks later rather than the PR that drifts away from it. Neither existing gate
covers these files: tests/unit/test_config_validation.py globs examples/configs
only, and tests/unit/utils/test_config.py validates against RL's MasterConfig,
which could not catch a failure in Gym's VLLMModelConfig.

Add tests/unit/environments/test_nemo_gym_prefetch_configs.py, which runs Gym's
own model_validate over every standalone model server block in those configs. It
deliberately does not restate the field list, so it tracks the submodule pin and
cannot go stale. Blocks that are overlays onto a bundled config (no entrypoint)
or that carry _copy/_inherit_from are skipped, since Gym merges a source server
in before validating and the literal block is not self-contained.

Also correct the UV_LINK_MODE comment. "hardlink is equally space-efficient" is
true against copy but false against symlink: the uv cache is populated in the
hermetic stage and the gym prefetch RUN is a later layer, so hardlinking out of
it makes overlayfs copy the bytes up -- exactly what symlink avoided. hardlink
remains the right call (symlink breaks under --no-cache, clone still copies up),
so only the wording changes. Note the second caller in the nemotron-3-super
guide, and that the explicit override is load-bearing rather than redundant:
Ray merges runtime_env env_vars per key over the inherited environment, so
filtering the key out instead would leave the raylet's symlink in place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
(cherry picked from commit fd91464)
@yuki-97
yuki-97 enabled auto-merge (squash) August 25, 2026 04:54
@yuki-97
yuki-97 merged commit 48d71a3 into main Aug 25, 2026
85 checks passed
@yuki-97
yuki-97 deleted the yifu/fix-gym-prefetch-dry-run branch August 25, 2026 04:54
terrykong added a commit that referenced this pull request Sep 4, 2026
The dependency stage builds the actor venvs by hand rather than calling
prefetch_venvs.py, and nothing said why. The reason is that prefetch_venvs.py
installs the nemo_rl project, and the source tree is not in that layer yet --
it arrives in the release stage on purpose, so a source edit does not
invalidate the hour-long dependency build. So phase 1 does the expensive
cacheable half (third-party packages, hardlinked, --no-install-project) and
phase 2 does the cheap half (editable install plus the wrapper scripts).

Also corrects the NeMo Gym note: UV_LINK_MODE=symlink applies to the outer
uv run, but the gym server venvs are hardlinked -- prefetch_venvs.py pins
UV_LINK_MODE=hardlink in the Gym actor's runtime_env (#3785), because Gym's
setup scripts use 'uv pip install --no-cache', which uv refuses to combine
with symlink installs.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Sep 5, 2026
The dependency stage builds the actor venvs by hand rather than calling
prefetch_venvs.py, and nothing said why. The reason is that prefetch_venvs.py
installs the nemo_rl project, and the source tree is not in that layer yet --
it arrives in the release stage on purpose, so a source edit does not
invalidate the hour-long dependency build. So phase 1 does the expensive
cacheable half (third-party packages, hardlinked, --no-install-project) and
phase 2 does the cheap half (editable install plus the wrapper scripts).

Also corrects the NeMo Gym note: UV_LINK_MODE=symlink applies to the outer
uv run, but the gym server venvs are hardlinked -- prefetch_venvs.py pins
UV_LINK_MODE=hardlink in the Gym actor's runtime_env (#3785), because Gym's
setup scripts use 'uv pip install --no-cache', which uv refuses to combine
with symlink installs.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Sep 7, 2026
The dependency stage builds the actor venvs by hand rather than calling
prefetch_venvs.py, and nothing said why. The reason is that prefetch_venvs.py
installs the nemo_rl project, and the source tree is not in that layer yet --
it arrives in the release stage on purpose, so a source edit does not
invalidate the hour-long dependency build. So phase 1 does the expensive
cacheable half (third-party packages, hardlinked, --no-install-project) and
phase 2 does the cheap half (editable install plus the wrapper scripts).

Also corrects the NeMo Gym note: UV_LINK_MODE=symlink applies to the outer
uv run, but the gym server venvs are hardlinked -- prefetch_venvs.py pins
UV_LINK_MODE=hardlink in the Gym actor's runtime_env (#3785), because Gym's
setup scripts use 'uv pip install --no-cache', which uv refuses to combine
with symlink installs.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Sep 7, 2026
The dependency stage builds the actor venvs by hand rather than calling
prefetch_venvs.py, and nothing said why. The reason is that prefetch_venvs.py
installs the nemo_rl project, and the source tree is not in that layer yet --
it arrives in the release stage on purpose, so a source edit does not
invalidate the hour-long dependency build. So phase 1 does the expensive
cacheable half (third-party packages, hardlinked, --no-install-project) and
phase 2 does the cheap half (editable install plus the wrapper scripts).

Also corrects the NeMo Gym note: UV_LINK_MODE=symlink applies to the outer
uv run, but the gym server venvs are hardlinked -- prefetch_venvs.py pins
UV_LINK_MODE=hardlink in the Gym actor's runtime_env (#3785), because Gym's
setup scripts use 'uv pip install --no-cache', which uv refuses to combine
with symlink installs.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Sep 8, 2026
The dependency stage builds the actor venvs by hand rather than calling
prefetch_venvs.py, and nothing said why. The reason is that prefetch_venvs.py
installs the nemo_rl project, and the source tree is not in that layer yet --
it arrives in the release stage on purpose, so a source edit does not
invalidate the hour-long dependency build. So phase 1 does the expensive
cacheable half (third-party packages, hardlinked, --no-install-project) and
phase 2 does the cheap half (editable install plus the wrapper scripts).

Also corrects the NeMo Gym note: UV_LINK_MODE=symlink applies to the outer
uv run, but the gym server venvs are hardlinked -- prefetch_venvs.py pins
UV_LINK_MODE=hardlink in the Gym actor's runtime_env (#3785), because Gym's
setup scripts use 'uv pip install --no-cache', which uv refuses to combine
with symlink installs.

Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong added a commit that referenced this pull request Sep 8, 2026
The dependency stage builds the actor venvs by hand rather than calling
prefetch_venvs.py, and nothing said why. The reason is that prefetch_venvs.py
installs the nemo_rl project, and the source tree is not in that layer yet --
it arrives in the release stage on purpose, so a source edit does not
invalidate the hour-long dependency build. So phase 1 does the expensive
cacheable half (third-party packages, hardlinked, --no-install-project) and
phase 2 does the cheap half (editable install plus the wrapper scripts).

Also corrects the NeMo Gym note: UV_LINK_MODE=symlink applies to the outer
uv run, but the gym server venvs are hardlinked -- prefetch_venvs.py pins
UV_LINK_MODE=hardlink in the Gym actor's runtime_env (#3785), because Gym's
setup scripts use 'uv pip install --no-cache', which uv refuses to combine
with symlink installs.

Signed-off-by: Terry Kong <terryk@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants