cp: feat(pinchbench): add the benchmark config for the full 147-task suite (2259) + fix(apptainer): keep sandbox env out of argv (2284) into r0.5.0 - #2283
Merged
Conversation
#2259) ## Summary PinchBench shipped as an agent server (`responses_api_agents/pinchbench`, #1810) but has no entry under `benchmarks/`, so it is neither discoverable via `gym list benchmarks` nor preparable via `gym eval prepare`. This adds the benchmark definition. - `config.yaml` chains `config_paths` to the agent config and narrows it to the benchmark dataset via `_inherit_from` — the `benchmarks/tau2` shape, the existing precedent for a benchmark chaining to a `responses_api_agents/` config. - `prepare.py` clones the PinchBench skill at `v2.0.0`, the same ref `Dockerfile.benchmark` bakes into the per-task image, and writes all 147 manifest tasks. - `num_repeats: 3`, matching the n=3 parity validation in the agent README (reward is continuous, and temperature 1.0 + live web search make a single pass noisy). ## Notes for reviewers **Run-time interpolations.** The agent config interpolates eight knobs (`${sandbox_image}`, `${model_base_url}`, judge/brave keys) that do not resolve on their own, so `gym eval prepare` would fail on a bare config. They are declared top-level with `oc.env:...,null` defaults. This does not break the documented `+sandbox_image=...` invocation: Hydra parses CLI args against an empty base *before* `config_paths` loads, and CLI merges last. A `null` still reaching the agent fails fast, since those fields are required `str` in `PinchBenchAgentConfig`. **Listing metadata on the base node.** `domain`/`description`/`value` are added to `responses_api_agents/pinchbench/configs/pinchbench.yaml` rather than the benchmark config, because adding them to the inheriting node raises `ConfigKeyError: Key 'domain' is not in struct`. Worth knowing: `discover_benchmarks()` uses the tolerant non-struct parse, so `ng_list_benchmarks` renders such a key fine and only the real prepare/run path trips the check. The `README.md` row is regenerated by the `update-readme-table` hook. **Drift guards.** Preparation fails loudly if the manifest is not 147 tasks, or if the regenerated rows do not byte-match the committed `responses_api_agents/pinchbench/data/example.jsonl` — so upstream prompt drift and divergence from `dataset_preprocess.py` cannot pass silently. ## Verification - `ng_prepare_benchmark "+config_paths=[benchmarks/pinchbench/config.yaml]"` — 147 rows, via both the live-clone path and `PINCHBENCH_SKILL_DIR` - Rows validated: all carry a non-empty `responses_create_params.input`, `task_id`s unique - `ng_list_benchmarks` lists `pinchbench` with domain `agent` and `num_repeats` 3 - Drift guard negative-tested (tampered prompt raises) - `pre-commit run` clean on all touched files - Generated JSONL is covered by the existing `benchmarks/.gitignore` ## Related Cherry-picked to `r0.5.0` in a companion PR (that release branch carries the agent; `r0.4.0` does not, as PinchBench landed on main after the cut). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: plaszkiewicz <plaszkiewicz@nvidia.com> Signed-off-by: NeMo Bot <nemo-bot@nvidia.com>
Author
|
/ok to test 1e416d1 |
laszkiewiczp
previously approved these changes
Aug 3, 2026
laszkiewiczp
added a commit
that referenced
this pull request
Aug 3, 2026
Authored by @arajfer — I'm only carrying it to `main`. The commit keeps his authorship and `Signed-off-by`; I added mine as the submitter. ## Why this PR exists This change was written on the `cherry-pick-2259-r0.5.0` branch (commit `032ae5184`), stacked on top of the PinchBench benchmark-config cherry-pick. When #2259 merged, the bot regenerated that branch as a single squashed commit for #2283, which dropped the stacked work. The commit was never on `main` — it only ever existed on that branch — so it would have been lost. ## What it does - `nemo_gym/sandbox/providers/apptainer/provider.py` — pass sandbox environment through the environment rather than `argv`, so secrets and env values don't land in the process command line. - Adds `tavily_api_key` plumbing: `${tavily_api_key}` on the agent config, and the matching `${oc.env:PINCHBENCH_TAVILY_API_KEY,null}` top-level declaration on the benchmark config. Both are required together — the agent config interpolates the key, so the benchmark config must declare it or `gym eval prepare` fails to resolve. `PinchBenchAgentConfig` already carries `tavily_api_key: Optional[str] = None`, so nothing else is needed. - Tests: `tests/unit_tests/test_apptainer_provider.py` (+129) covering the argv/env split, and `tests/unit_tests/test_benchmarks.py` (+9) pinning the agent/benchmark tavily pairing so the two configs can't drift apart. ## Verification Cherry-picked onto current `main` (`3e210384a`) with no conflicts, then: - `pytest tests/unit_tests/test_apptainer_provider.py tests/unit_tests/test_benchmarks.py` — **101 passed**, including the new `test_pinchbench_tavily_key_is_derived_from_environment` - `pre-commit run` clean on all 5 touched files @arajfer please confirm this is the version you want on `main` — if you'd rather own the PR yourself, close this and I'll drop the branch. Once it lands it can cherry-pick to `r0.5.0` on its own, independently of #2283. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Adam Rajfer <arajfer@nvidia.com> Signed-off-by: plaszkiewicz <plaszkiewicz@nvidia.com> Co-authored-by: Adam Rajfer <arajfer@nvidia.com>
Authored by @arajfer — I'm only carrying it to `main`. The commit keeps his authorship and `Signed-off-by`; I added mine as the submitter. ## Why this PR exists This change was written on the `cherry-pick-2259-r0.5.0` branch (commit `032ae5184`), stacked on top of the PinchBench benchmark-config cherry-pick. When #2259 merged, the bot regenerated that branch as a single squashed commit for #2283, which dropped the stacked work. The commit was never on `main` — it only ever existed on that branch — so it would have been lost. ## What it does - `nemo_gym/sandbox/providers/apptainer/provider.py` — pass sandbox environment through the environment rather than `argv`, so secrets and env values don't land in the process command line. - Adds `tavily_api_key` plumbing: `${tavily_api_key}` on the agent config, and the matching `${oc.env:PINCHBENCH_TAVILY_API_KEY,null}` top-level declaration on the benchmark config. Both are required together — the agent config interpolates the key, so the benchmark config must declare it or `gym eval prepare` fails to resolve. `PinchBenchAgentConfig` already carries `tavily_api_key: Optional[str] = None`, so nothing else is needed. - Tests: `tests/unit_tests/test_apptainer_provider.py` (+129) covering the argv/env split, and `tests/unit_tests/test_benchmarks.py` (+9) pinning the agent/benchmark tavily pairing so the two configs can't drift apart. ## Verification Cherry-picked onto current `main` (`3e210384a`) with no conflicts, then: - `pytest tests/unit_tests/test_apptainer_provider.py tests/unit_tests/test_benchmarks.py` — **101 passed**, including the new `test_pinchbench_tavily_key_is_derived_from_environment` - `pre-commit run` clean on all 5 touched files @arajfer please confirm this is the version you want on `main` — if you'd rather own the PR yourself, close this and I'll drop the branch. Once it lands it can cherry-pick to `r0.5.0` on its own, independently of #2283. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Adam Rajfer <arajfer@nvidia.com> Signed-off-by: plaszkiewicz <plaszkiewicz@nvidia.com> Co-authored-by: Adam Rajfer <arajfer@nvidia.com> (cherry picked from commit d58f6ef)
laszkiewiczp
changed the title
feat(pinchbench): add the benchmark config for the full 147-task suite (2259) into r0.5.0feat(pinchbench): add the benchmark config for the full 147-task suite (2259) + fix(apptainer): keep sandbox env out of argv (2284) into r0.5.0
Contributor
|
Added a second commit: the cherry-pick of #2284 ( Why it's stacked here rather than its own cp PR. #2284's auto-cherry-pick ran on merge and failed — The two must land together anyway — #2284 adds Verified on this branch (r0.5.0 base, not main): cherry-pick applies with zero conflicts; Sorry for the churn on the approval — the new commit dismisses it. @arajfer is preserved as |
laszkiewiczp
self-requested a review
August 3, 2026 09:54
laszkiewiczp
approved these changes
Aug 3, 2026
Contributor
|
Unit tests failure present on main branch, merging PR - the tests passed earlier |
OlegSudakov
pushed a commit
to OlegSudakov/Gym
that referenced
this pull request
Aug 7, 2026
Authored by @arajfer — I'm only carrying it to `main`. The commit keeps his authorship and `Signed-off-by`; I added mine as the submitter. ## Why this PR exists This change was written on the `cherry-pick-2259-r0.5.0` branch (commit `032ae5184`), stacked on top of the PinchBench benchmark-config cherry-pick. When NVIDIA-NeMo#2259 merged, the bot regenerated that branch as a single squashed commit for NVIDIA-NeMo#2283, which dropped the stacked work. The commit was never on `main` — it only ever existed on that branch — so it would have been lost. ## What it does - `nemo_gym/sandbox/providers/apptainer/provider.py` — pass sandbox environment through the environment rather than `argv`, so secrets and env values don't land in the process command line. - Adds `tavily_api_key` plumbing: `${tavily_api_key}` on the agent config, and the matching `${oc.env:PINCHBENCH_TAVILY_API_KEY,null}` top-level declaration on the benchmark config. Both are required together — the agent config interpolates the key, so the benchmark config must declare it or `gym eval prepare` fails to resolve. `PinchBenchAgentConfig` already carries `tavily_api_key: Optional[str] = None`, so nothing else is needed. - Tests: `tests/unit_tests/test_apptainer_provider.py` (+129) covering the argv/env split, and `tests/unit_tests/test_benchmarks.py` (+9) pinning the agent/benchmark tavily pairing so the two configs can't drift apart. ## Verification Cherry-picked onto current `main` (`3e210384a`) with no conflicts, then: - `pytest tests/unit_tests/test_apptainer_provider.py tests/unit_tests/test_benchmarks.py` — **101 passed**, including the new `test_pinchbench_tavily_key_is_derived_from_environment` - `pre-commit run` clean on all 5 touched files @arajfer please confirm this is the version you want on `main` — if you'd rather own the PR yourself, close this and I'll drop the branch. Once it lands it can cherry-pick to `r0.5.0` on its own, independently of NVIDIA-NeMo#2283. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Adam Rajfer <arajfer@nvidia.com> Signed-off-by: plaszkiewicz <plaszkiewicz@nvidia.com> Co-authored-by: Adam Rajfer <arajfer@nvidia.com>
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
3 participants
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.
beep boop [🤖]: Hi @laszkiewiczp 👋,