Repository navigation
feat(terminal_bench_2_1): Terminal Bench 2.1 on the Harbor task shape (M8) - #3730
Conversation
adil-a
changed the title
a244db1 to
a453d04
Compare
a453d04 to
20857c2
Compare
20857c2 to
81f0e76
Compare
81f0e76 to
359dc64
Compare
359dc64 to
07dba43
Compare
NVIDIA-NeMo#4135) ## What does this PR do? `benchmarks/terminal_bench_2_1/prepare.py` wrote rows in `glob()` order (filesystem order) and without an id field. Gym's task identity (`_trajectory_identity`, `materialize_task`) therefore fell back to the row position, so "task 0" could name a different task on another machine or after a re-clone. This PR: - adds `task_id` to every row, set to the task's folder name (e.g. `path-tracing`). That is the id the Harbor task format uses for the same task (NVIDIA-NeMo#3673 takes the folder name as `task_id`; NVIDIA-NeMo#3730 keeps TB2.1's folder names), so the ids stay the same when TB2.1 moves to that format. Folder names are unique by construction; `task_name` (`terminal-bench/path-tracing`) is unchanged; - writes rows in sorted order; - checks that the 89 task ids are unique (`NUM_TASKS = 89` replaces the literal in the existing count check). Servers are unaffected: `TaskData` is `extra="allow"`, the Terminus and OpenCode run requests accept extra fields, and nothing reads `task_id` from TB2.1 rows today. Motivation: stable ids are required by `gym eval plan` (NVIDIA-NeMo#4131), which refuses position-based ids. With this change TB2.1 can be planned. No separate issue. ## Validation - Ran `prepare.py` via `gym eval plan --benchmark terminal_bench_2_1/terminus_2` (with NVIDIA-NeMo#4131 applied locally): 89 tasks, ids `adaptive-rejection-sampler`, `bn-fit-modify`, …; two runs produce byte-identical plans. Without this change the same command is refused with "89 of 89 tasks have no task id". - `pre-commit run --files benchmarks/terminal_bench_2_1/prepare.py`: pass. ## Rollouts N/A: data-only change. The added field is ignored by the TB2.1 resources server and both agents; the instruction, image and task folder of every row are unchanged. Happy to run a smoke rollout if reviewers want one. ## Compatibility and benchmark impact - Rollout outputs for TB2.1 now report the task's folder name as the task id instead of the row index, so tooling that joins TB2.1 results across runs by task id will see different keys for runs before and after this change. - Row order changes, so `_ng_task_index` values change. Don't resume (`--resume`) a TB2.1 run started before this change after re-preparing the data. - Scores are unaffected. ## Checklist - [x] I have read the [contributing guidelines](https://github.com/NVIDIA-NeMo/Gym/blob/main/CONTRIBUTING.md). - [x] The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs. - [x] Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable). N/A: no existing tests cover this prepare script; validated by running it. - [x] Documentation added or updated, or N/A with justification. N/A: no documented behaviour changes. - [x] Pre-commit checks pass locally (`pre-commit run --all-files`) (so CI lint/format checks pass). Ran on the changed file. - [x] New source files include the required Apache-2.0 SPDX header. N/A: no new files. - [x] All commits have DCO sign-off (`git commit -s`) (so the DCO check passes). Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
07dba43 to
6e7bcc3
Compare
6e7bcc3 to
c5d7165
Compare
yuhezhang-ai
left a comment
yuhezhang-ai
left a comment
There was a problem hiding this comment.
One additional nonblocking filename case from the current-head verification; the previously reported comma/parenthesis cases are fixed.
|
Re-review status at Resolved: the comma/parenthesis output-path fix is verified; the preparation-test and legacy startup-repair requests are obsolete under the rewritten scope. Those threads are now closed. The earlier output-summary-path fix remains resolved. Still needs fixing:
Please also refresh the PR description to reflect the removed preparation script and current scope. Existing findings were kept in their original threads; only the additional filename case received a new thread. |
c0041eb to
5bc0c0b
Compare
|
/ok to test 5bc0c0b |
|
🌿 Preview your docs: https://nvidia-preview-pr-3730-f782ce934585.docs.buildwithfern.com/nemo/gym |
|
/ok to test 765622b |
|
/ok to test fc9258b |
|
/ok to test 6ea3bf6 |
|
Status at the new head 6ea3bf6. Your four remaining items:
The benchmark folder moved from benchmarks/harbor/ to environments/harbor/ in the rebase. The description still describes the removed preparation script, I am refreshing it to the current scope. |
yuhezhang-ai
left a comment
yuhezhang-ai
left a comment
There was a problem hiding this comment.
Rechecked 6ea3bf6f against M7. Previous findings are addressed or superseded by the documented scope change; no remaining blockers from this review. Required DCO, lint and CI checks pass. The optional harness-conformance check still reports unavailable or invalid probe evidence.
|
/ok to test f6a36db |
|
/ok to test 24916f9 |
|
/ok to test 022ad8f |
ffrujeri
left a comment
ffrujeri
left a comment
There was a problem hiding this comment.
Approving 022ad8f9. All threads are resolved and Yu has no remaining blockers. The --output quoting round-trips through Hydra in the tests, and the summary reads the effective output_jsonl_fpath. CLI, validation, Harbor task and Harbor environment-server suites pass locally (589 passed). CI is green apart from the non-blocking harness conformance probes. One non-blocking docs note inline.
|
/ok to test f41a064 |
ffrujeri
left a comment
ffrujeri
left a comment
There was a problem hiding this comment.
Re-approving f41a064b. Against the approved 022ad8f9, the patch adds the docs note on the expected non-zero exit for the two broken upstream tasks (thanks) and rebases the README context. Stack tests pass locally; CI was still running at approval time.
|
/ok to test eb9535e |
…b path (M8) `gym dataset validate` accepts `--output/-o` for its rollouts file, so a later `gym dev compare` can find oracle runs where the caller put them. The Terminal Bench 2.1 README documents the Harbor task shape: the 89 tasks fetched from the Harbor hub as published, through the `harbor` environment server, with no Gym-side patching of `tests/test.sh` or `solution/solve.sh`. Results on that path are compared with Harbor's own numbers for the dataset rather than one to one with this benchmark's server. The earlier `prepare_harbor_taskset.py`, which materialized the pinned fork with the server's run-time fixes applied as plain files, is dropped: the Harbor path reuses Harbor's format and loader semantics, and Harbor-side task fixes belong upstream. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Adil Asif <adasif@nvidia.com>
|
/ok to test f782ce9 |
ffrujeri
left a comment
ffrujeri
left a comment
There was a problem hiding this comment.
Re-approving f782ce93 after the restack. git range-diff shows the patch is identical to the f41a064b I approved.
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/39680941039 |
What does this PR do?
M8 of the Harbor tasks RFC (GitLab MR 340, MR 354): Terminal Bench 2.1 runs through the generic
harborenvironment server as published Harbor task folders, with no Gym-side patching, andgym dataset validategets an output flag. Stacked on #3729 (M7).gym dataset validate --output(-o). Rollouts can go to a chosen file and the summary reads from the effectiveoutput_jsonl_fpath, whether it came from the flag, the command line or a config file (_override_valueinnemo_gym/tasks/harbor/cli.py). The flag is translated to a quoted Hydra override through a new_hydra_quotehelper innemo_gym/cli/main.py, which single-quotes the value and escapes only what Hydra itself un-escapes, so a filename with parentheses, commas, Unicode or backslashes reaches collection under its own name.gym search +query=uses the same helper.fern/versions/latest/pages/evaluation-tutorials/terminal-bench-2-1-harbor.mdx, clonesharbor-framework/terminal-bench-2-1, pins the commit the legacy benchmark runs from (7131e43), and runsgym dataset validate terminal-bench-2-1/tasksandgym eval run terminal-bench-2-1/tasks --agent terminus_2_sandboxed_agenton that folder. It states that results compare with Harbor's own numbers rather than one to one with theterminal_bench_2_1Resources Server, whose run-time fixes stay on its own path, and listsqemu-startupandqemu-alpine-sshas known upstream noise (Bullseye security repository). The package-store form of the same dataset is added to this page in feat(harbor): fetch package-store datasets by content hash (M9) #3743, where it first resolves.environments/harbor/terminal_bench_2_1/README.mdis a short pointer to that page (the folder moved frombenchmarks/harbor/in the rebase).environment_servers/harbor/README.mdgains the paragraph on what a registry fetch reuses: an existing folder on its recorded pin (full commit SHA, atask.toml, no symlink leaving the folder), with no content hashing, the recorded commit kept when the registry moves, and move the folder aside to fetch the current copy.There is no preparation script and nothing is patched or checked in. Tasks come from the Harbor hub or the clone exactly as published.
Validation
pytest tests/unit_tests/test_cli_main.py tests/unit_tests/test_harbor_validation.py tests/unit_tests/test_harbor_tasks.pyat6ea3bf6f: 468 passed.TestDatasetValidateFlags::test_output_reaches_the_validation_run_quoted(test_cli_main.py) is parametrized overcurrent(1).jsonl,current,failed.jsonl,résumé.jsonl,日本語.jsonl,dir\name.jsonl, a Windows path,it's.jsonl,a\'b.jsonland a trailing double backslash, and asserts the override parses back through Hydra'sOverridesParserto the exact name. The newflagcase oftest_validate_reads_effective_output_and_preserves_evaluation_artifacts(test_harbor_validation.py) runs the real argparse and Hydra parse with--output résumé(1),back\slash.jsonland checks that collection receives that path and that the summary reads it, next to the existingcliandfilecases.ruff checkandruff format --check(ruff 0.9.9) clean on the changed files at6ea3bf6f.Rollouts
All tables in this section were produced before the 2026-10-04 rebase onto
mainand before the preparation script was dropped, so the Harbor-path side ran the pinned clone with the legacy fixes applied. The documented commands now run the folders exactly as published, which is whyqemu-startupandqemu-alpine-sshare listed as upstream noise on the docs page rather than repaired. The tables have not been rerun since.Parity rerun after the earlier rebase onto
mainand M15 (2026-10-01). Both paths ran 30 September's code. 89 tasks on both sides, 80 identical, 9 flips, each with a byte-identical prompt, divergence at the model's first reply, and a wrong answer in the losing side's verifier output. Two tasks are refused by the endpoint's content moderation on both sides and score 0 on both. Full per-task table below. The section below is the earlier, pre-rebase measurement.Full per-task table (both sides on 30 September's code, latest rerun per task):
Per-task rewards, old path vs Harbor path
Terminal Bench 2.1 parity: old server vs Harbor shape (x86, OpenSandbox, 1 rollout per task).
Reference solutions (old golden-patch mode vs
gym dataset validate)Terminus 2 with
openai/openai/eccn-gpt-6-solNotes on the differences:
MASTER_PORT=12355undermp.spawn, and when a test's rank 0 binds before the previous test's processes let go, torch waits out its 30-minute rendezvous timeout, past the task's 900 s verifier budget. It hit 2 of 4 runs on the new path and 0 of 1 on the old. A standalone run passed 13 of 13 tests in 82 s.remote_tmux_binary_pathavoids. The old run got a working install for the same task.Run details: x86 OpenSandbox nodes, one rollout per task, both paths sized identically (4 CPU, 16 GiB, 30 GiB, requests 0.5 CPU, 512 MiB), overlay in
results/tb21_parity/new_overlay.yamlon the run host. Old path:benchmarks/terminal_bench_2_1config withnum_repeats: 1(golden mode withmax_turns: 1for the reference run). New path:gym dataset validate datasets/terminal-bench-2-1andgym eval run datasets/terminal-bench-2-1 --agent terminus_2_sandboxed_agent.Compatibility and benchmark impact
The existing
terminal_bench_2_1Resources Server, its benchmark folder underbenchmarks/terminal_bench_2_1/and its configs are unchanged. Nothing is checked in: the task folders come from the clone or the hub into the datasets folder, and the Harbor path applies none of the legacy server's run-time fixes.gym dataset validatewithout--outputbehaves as before.Checklist
pre-commit run --all-files) (so CI lint/format checks pass).git commit -s) (so the DCO check passes).🤖 Generated with Claude Code