[kajalj22]

Removing bundled codecs from the deps.

[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.

[kajalj22]

/ok to test 9a7cb65

[kajalj22]

/claude review

[claude]

SHIP WITH CARE — packaging change to exclude royalty-bearing codec binaries from the shipped package/container, with per-server re-enable. Mechanism is sound; one container-build risk to confirm.

What I verified

• Root pyproject.toml marks opencv-python-headless/torchvision/torchaudio as sys_platform == 'never' in override-dependencies, so they drop everywhere by default (and from server venvs, which inherit the list per the existing PR-1835 behavior). uv.lock correctly reflects the markers and drops the wheel tables.
• Re-enable paths both check out: requirements.txt servers use overrides.txt → wired via --override in nemo_gym/cli/setup_command.py:153-156; pyproject.toml servers use their own [tool.uv] override-dependencies (vlm_eval_kit, osworld_agent).
• docker/install_codec_deps.sh regex extracts versions from uv.lock (tested against the base lock: opencv 5.0.0.93 / torchvision 0.26.0 / torchaudio 2.11.0) and installs with --no-config to bypass the never override.
• Servers depending on these deps (longmt_eval, wmt_translation, vlm_eval_kit, osworld_agent) all re-declare them; no import of cv2/torchvision/torchaudio found elsewhere that would silently lose a dependency.

Findings

• RISK (inline, gdpval.def:131): --no-binary av forces a PyAV source build that needs FFmpeg -dev headers, but this diff removed ffmpeg from apt without adding libav*-dev. Likely a loud build failure of the gdpval .sif. Needs an actual container build to confirm before merge.
• NOTE: install_codec_deps.sh — if the uv.lock regex ever fails to match, PKGS is empty and echo "" | xargs uv pip install runs uv pip install with no packages, which errors under set -e. Low likelihood; a guard on empty PKGS would fail more clearly.

No verifier/scorer, async, or public-API surface touched. No test-coverage concern (packaging-only).

[kajalj22]

/claude review

[kajalj22]

/claude review

[claude]

SHIP WITH CARE — mechanism is sound (strip royalty-bearing codec binaries from the shipped package/container via a sys_platform=='never' override; re-enable per-server), but one consumer applies the strip instead of the re-enable, and the container restore script drifts from what consumers validate against.

RISK (verify before merge) — responses_api_agents/osworld_agent/pyproject.toml:74. The added opencv-python-headless; sys_platform == 'never' override replaces the spec for opencv globally in resolution, including osworld's own hard runtime dep (opencv-python-headless~=4.8.1.78, line 47). That empties cv2 from the venv on every real platform. The new tests/conftest.py force-reinstalls cv2 before test collection — which confirms the strip — but nothing restores it on the server runtime path, so a rollout that loads mm_agents/desktop_env will hit ModuleNotFoundError in the Ray worker. Compare vlm_eval_kit/pyproject.toml, which correctly re-enables its codec deps with real specs. Fix: re-enable opencv here with a real spec (or add a runtime install hook), not the strip marker. Inline comment on the line.

NOTE — docker/install_codec_deps.sh. Pins opencv-python-headless==5.0.0.93 / torchvision==0.26.0 / torchaudio==2.11.0 as literals and claims CI catches drift from uv.lock. But the PR removed all wheel entries for these from uv.lock, so nothing mechanically ties the script to the lock, and there's no test importing all three at these versions. Also the restore version (5.0.0.93) differs from what osworld (4.8.1.78) and vlm_eval_kit (opencv-python>=4.7.0.72, non-headless) validate against — restoring via this script inside the container gives those servers a different opencv than expected.

The rest (README, Dockerfile apt purge, longmt_eval/wmt_translation overrides.txt re-enabling torchvision==0.27.0, uv.lock marker changes, [dependency-groups] codec) is consistent and correctly scoped. No async/verifier/public-API concerns — this is packaging only.

[kajalj22]

/claude review

[claude]

SHIP WITH CARE — packaging/licensing change; no verifier, scorer, or async code touched, so no data-corruption or hang risk. The mechanism is sound but rests on one unenforced invariant.

What this does: marks opencv-python-headless, torchvision, torchaudio as sys_platform == 'never' in the root nemo-gym override so the codec-bearing binaries are dropped from the shipped package and container (MPEG-LA royalty avoidance), and provides docker/install_codec_deps.sh to restore them. Since every editable server venv inherits the root override (per the existing PR #1835 footgun), the four servers that genuinely need these — vlm_eval_kit, osworld_agent (pyproject override-dependencies), longmt_eval, wmt_translation (new overrides.txt) — re-enable them locally. I confirmed no server directly imports cv2/torchvision/torchaudio; all usage is transitive, and the four declaring servers are all covered.

The load-bearing assumption (verify before merge): correctness here depends entirely on uv's per-server override-dependencies / --override winning over the inherited root sys_platform=='never' override. If that precedence does not hold, these four servers silently lose the package at install time and fail at runtime — the exact failure mode PR #1835 documents. This is testable: gym env test --resources-server vlm_eval_kit (and the other three) must show the package actually installed in the venv. If CI already runs those server tests on this PR, that's the gate — confirm they pass.

One inline RISK on install_codec_deps.sh: the script pins container versions (torchvision 0.26.0) that no CI job installs — server tests use 0.27.0 — so the "CI will catch a version mismatch" comment overstates the guarantee. Silent drift risk on the manual codec-restore path only; narrow blast radius.

Everything else (README, Dockerfile apt purge, uv.lock wheel removal) is consistent with the stated intent.

[kajalj22]

/claude review

[claude]

SHIP WITH CARE — clean packaging change; one operability risk in the recovery script.

This drops the codec-bearing packages (opencv-python-headless, torchvision, torchaudio) from the main venv and, as the existing [tool.uv] comment warns, from every inheriting server venv. I verified the mechanism holds together:

• All four real consumers re-enable what they need: longmt_eval + wmt_translation via new overrides.txt (torchvision==0.27.0, matching their requirements.txt torch 2.12 pin), vlm_eval_kit + osworld_agent via [tool.uv] override-dependencies.
• uv.lock, the Dockerfile apt purge, and the README instructions all line up with the pyproject overrides.
• No core nemo_gym/ code imports cv2/torchvision/torchaudio unconditionally, so removing them from the [all]/image closure won't break base imports.

Note (not blocking): docker/install_codec_deps.sh pins versions by hand (opencv 5.0.0.93, torchvision 0.26.0, torchaudio 2.11.0) and the header acknowledges these must be bumped manually when uv.lock moves. There's no CI check tying them to the lock, so they will drift. Consider a lightweight lock-consistency test, or deriving the pins from uv.lock at runtime, so a future vllm bump doesn't silently install a stale codec set.

One inline RISK on the script's install target. Nothing here touches verifiers, scoring, or async paths.