Repository navigation
feat(harbor): gym dataset pull and committed entry sources (M38, second slice) - #4872
Conversation
|
/ok to test 2f53402 |
|
🌿 Preview your docs: https://nvidia-preview-pr-4872-fb511ae6d921.docs.buildwithfern.com/nemo/gym |
2f53402 to
d843dd8
Compare
|
/ok to test d843dd8 |
|
/ok to test a9a4ffc |
d843dd8 to
a9a4ffc
Compare
|
/ok to test 1789ce5 |
|
/ok to test ac4a014 |
|
/ok to test a100ed1 |
|
Thanks, Adil. I rechecked the latest revision against #4865. The five inline findings are addressed, including protecting copied configuration after preparation without NEMO_GYM_DATASETS_DIR. The reference-spelling and moving registry-pin fixes also look good, and the focused regression tests passed. The remaining cleanup assertion still checks terminal-bench-4.0.0/alpha rather than terminal_bench_4/alpha. The fault-injection reproduction still passes with a partial file in the actual destination. Please correct that assertion or explicitly isolate the unmapped-reference case; I consider this non-blocking. The PR description also still references REPORT.md and a test_global_config.py change absent from this diff. Please clarify those validation references. |
yuhezhang-ai
left a comment
There was a problem hiding this comment.
Correction to my earlier follow-up: I missed the installed-package destination follow-up explicitly deferred from #4827 to this PR. The five resolved findings remain fixed, but the combined stack should wait for the writable entry-destination fallback and its installed-layout regression. Keeping that fix in #4872 is fine. The cleanup-test and validation-description comments remain non-blocking.
|
/ok to test 05e31e4 |
|
/ok to test 12a629c |
|
Re the correction above: the installed-package destination fallback deferred from #4827 is now in this PR (head 12a629c). |
|
/ok to test f43ce06 |
|
/ok to test 849495a |
|
/ok to test 3fafede |
|
/ok to test 9062904 |
|
/ok to test 816e65b |
yuhezhang-ai
left a comment
There was a problem hiding this comment.
Thanks, Adil. Rechecked 816e65bc against the current #4865 parent. Both original installed-package reproductions now pass unchanged: downloads leave a read-only project-local .venv untouched, and later installed-entry changes reach the existing download.
The focused regressions also confirm that edited copies stay protected and user-authored entries retain precedence. I have no remaining blocking findings; the installed-package thread can stay resolved.
The cleanup-test assertion and outdated validation references remain non-blocking follow-ups.
|
/ok to test 4006449 |
|
/ok to test bc68ab5 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40540366340 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40546615675 |
|
/ok to test 8034d45 |
yuhezhang-ai
left a comment
There was a problem hiding this comment.
Rechecked 8034d456 against the current #4865 parent, 90a15c96. This PR's patch is unchanged from the previously approved 816e65bc; the installed-package download and refresh fixes, copy protection, and earlier resolved findings remain intact.
Reused the completed focused regression validation after confirming the entry/download implementation and its tests are unchanged. Current core/server CI, lint, docs and wheel validation passed. I have no remaining blocking findings. The cleanup-test assertion and outdated validation references remain non-blocking follow-ups.
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40612810697 |
|
/ok to test 60b54e6 |
…nd slice) `gym dataset pull TARGET` fetches a Harbor dataset into environments/harbor/ through the existing package-store and registry fetches, without preparing or running anything, and prints where it landed, the task count and the dataset digest; a stale, edited or foreign folder is refused in the fetch's own words, and a second pull of an unchanged dataset downloads nothing and says so. A committed entry under environments/harbor/<entry>/ declares its upstream in its own dataset.toml as the new `[gym] source` key (a harbor: reference). `gym dataset pull <entry>` fetches it into the entry's own folder, wherever the search root that commits it lives, where the pulled task folders and manifest.toml are gitignored beside the committed README, YAML and dataset.toml; a pull of the declared reference itself, by this command or by `gym eval run`, lands in the same folder, so one dataset never lives in two sibling folders, and a hub folder the reference landed in before the entry declared it (terminal-bench-4.0.0/) is named in the report and left alone. A reference no entry declares keeps landing in the hub's own folder name. `gym dataset validate <entry>` and `gym eval run <entry>` resolve the bare name to that folder without fetching; an entry not pulled yet names the pull to run. NEMO_GYM_DATASETS_DIR still moves every pull: an entry then lands in <datasets dir>/<entry>/ with a copy of its dataset.toml beside the tasks, placed before the first task lands and replaced when the entry's file changes, by pull, validate and run alike, while a copy edited in place is refused by all three. The command adds no flag; the registry cache is refreshed by deleting .harbor/registry.json, as before. terminal_bench_4 declares harbor:terminal-bench/terminal-bench@4.0.0 and terminal_bench_2_1 the content-hash pin of the store's TB 2.1. Docs: harbor README, CLI reference, Harbor tasks page, TB 2.1 tutorial, entry READMEs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Adil Asif <adasif@nvidia.com>
|
/ok to test fb511ae |
yuhezhang-ai
left a comment
There was a problem hiding this comment.
Rechecked fb511ae6 against the current #4865 parent, 6c0fbfef. The PR-specific patch is unchanged from the previously approved revision, and the earlier fixes remain intact. No remaining blocking findings.
The cleanup-test assertion and outdated validation references remain non-blocking follow-ups.
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40725069900 |
Stacked on #4865 (M31). Second slice of M38; the first slice is #4827.
Summary
gym dataset pull(M38, second slice): fetch a Harbor dataset intoenvironments/harbor/without preparing or running anything, and a name mapping between a hub dataset and its committed entry so one dataset never lives in two sibling folders. Publishing to the hub is deferred to a future work item.gym dataset pull harbor:<org>/<name>[@<tag> | @sha256:<digest>]andgym dataset pull harbor:<dataset>[@<version>]run the existing package-store and registry fetches (fetch_ref,fetch_package_dataset, the M9/M33 claim and manifest machinery) and print the folder, the task count and the dataset digest; a stale, edited or foreign folder is refused in the same words as the fetch insidegym eval run; a second pull of an unchanged dataset downloads nothing and says "unchanged since the last pull" (a registry reference whose version the registry picks is resolved before the fetch, so that form says it too). The command adds no flag (the registry cache is refreshed by deletingenvironments/harbor/.harbor/registry.json, as before).dataset.toml: the new[gym] sourcekey (aharbor:reference, validated by the hub's own parser).gym dataset pull <entry>(for exampleterminal_bench_4) fetches it into the entry's own folder, in whichever search root commits it, where the task folders andmanifest.tomlare gitignored beside the committed README, YAML anddataset.toml. A pull of the declared reference itself, andgym eval run/gym dataset validateon it, land in the same folder; the match is exact (latestand a tag are different references). A reference no entry declares lands inenvironments/harbor/<hub-folder-name>/as before.gym dataset validate <entry>andgym eval run <entry> ...resolve the bare entry name to that folder without fetching; an entry not pulled yet names the pull to run, task folders there with no fetch record (nomanifest.toml, or one of another source) are refused with the pull to run instead of running as a local dataset under the entry's name, and an existing folder of the same name or an explicit path keeps meaning the path for those two commands (the pull takes no path and finds the entry from any working directory).NEMO_GYM_DATASETS_DIRstill moves every pull: an entry then lands in<datasets dir>/<entry>/with a copy of the entry's Gym layer beside the tasks (itsdataset.tomland whatever dataset-leveltask_data.pyorverifier.pyit commits, the files that enter the dataset digest), placed before the first task lands and replaced when the entry's files change (pull, validate and run do it alike, and bring the manifest beside the tasks up to date with the copies at once, so the three never disagree about one folder or its digest); a copy edited in place is refused by all three, never overwritten.dataset.tomlup to date (manifest.refresh_gym_layer, task records untouched), so the digest it reports after an edit of the entry's table is the one the next run carries and that run rewrites nothing; a change of the Gym layer alone is announced in the manifest check's words (Rewrote manifest.toml: ...; dataset digest <old> -> <new>) whichever step applied it. A fetch or manifest-check refusal that names a committed entry's own folder adds what to delete there (the pulled task folders andmanifest.toml; the README, YAML anddataset.tomlstay; another version is pulled by changingsource); the refusal of an interrupted fetch namesgym dataset pull <entry>to run again instead.terminal_bench_4pinsharbor:terminal-bench/terminal-bench@4.0.0;terminal_bench_2_1pins the store's TB 2.1 by content hash (@sha256:7d7bdc1c…), since the store tags itlatestonly.Behaviour change for existing checkouts.
gym eval run harbor:terminal-bench/terminal-bench@4.0.0now lands inenvironments/harbor/terminal_bench_4/instead ofterminal-bench-4.0.0/: the dataset is fetched afresh (no folder is moved), the taskset, the rows'task_sourceand theresults/harbor/<taskset>/path change fromterminal-bench-4.0.0toterminal_bench_4, and the dataset digest changes because the entry'sdataset.tomlnow enters it. While the old folder is still there, the pull and a run of the reference print aNote:naming it; it can be moved aside or deleted. The README, the Harbor tasks page and the TB4 entry README carry the same note.What changes
nemo_gym/tasks/harbor/entries.py(new): committed entries (find_entry,committed_entries,entry_for_ref,entry_target), the pull plan (plan_pull), the entry's destination (its own folder unlessNEMO_GYM_DATASETS_DIRis set) and the destination with the entry's Gym layer current and its manifest following it (gym_layer,prepare_destination).nemo_gym/tasks/harbor/cli.py:pull_dataset,PulledDataset(with the superseded hub folder note),pull_target(the command),entry_folderwith its fetch-record check,with_entry_hint;prepare_targetfetches aharbor:target throughpull_dataset(so the entry mapping applies togym eval runandgym dataset validate) and resolves an entry name throughentry_folder.nemo_gym/tasks/harbor/manifest.py:refresh_gym_layer(the manifest'sdataset.toml/dataset-files digests recomputed from disk, task digests and pins kept as recorded, written only when changed, announced with the manifest check'sRewrotenotice);gym_layer_changeandreport_rewriteshared withcheck_manifest.nemo_gym/tasks/harbor/hub.py,package_store.py:fetch_ref,fetch_datasetandfetch_package_datasettake an optional destinationfolder;expected_foldernames a reference's folder without resolving it,resolved_folderresolves a versionless registry reference against the cache (for the unchanged report and the note).nemo_gym/tasks/harbor/dataset_config.py:GymDatasetConfig.source(validated as a hub reference; provenance only, shapes no task, stays out of thedatasets:entry).nemo_gym/cli/main.py:gym dataset pullcommand (TARGET only); TARGET help ofdataset validateandeval runnames the entry form..gitignore:environments/harbor/*/dataset.tomlis committed with the entry's README and YAML (in every folder there, a pulled hub folder included, as README.md and*.yamlalready are: adataset.tomlwritten into an old hub folder by hand now shows as untracked; the docs say to move its keys into the entry's file).environments/harbor/terminal_bench_4/dataset.toml,terminal_bench_2_1/dataset.toml(new) and their READMEs;scaffold.pydocuments the key as a comment.reference/cli-commands.mdx(gym dataset pull; thegym eval runTARGET sentence names the entry form),data/harbor-tasks.mdx([gym] source, "gym dataset pulland committed entries", TARGET rows, migration note), TB 2.1 tutorial.Test plan
tests/unit_tests/test_harbor_entries.py(new, 56 tests): thesourcekey (accepted forms, refusals naming the key, defaults, the two committed entries' values); entry lookup (with and without source, unreadable table, earlier root wins, an entry in a second search root pulled into its own folder with the env var unset, hub folders skipped);entry_for_refexact matching and the two-entries refusal;entry_target(bare name, paths and existing folders win);plan_pull;prepare_destinationunder a moved datasets dir (copy placed, refreshed when the entry's file changes and the copy is the last pull's, an in-place edit refused); the pull throughmain()against the package store faked at its HTTP boundary (entry pull, report lines, second pull unchanged with no download, reference and entry name are one folder, reference without an entry lands in the hub folder, registry reference through a local git repo, the refresh kwarg reaching the loader and no--refresh-registryflag, moved datasets dir, entry table edited after the pull reaching pull and prepare alike, a copy edited in place refused by pull, validate and run in the same words, an earlier hub folder named and left alone, edited folder refused by pull and bygym eval runalike, foreign manifest refused, one-line parser errors); target resolution in prepare,gym dataset validate <entry>andgym eval run <entry>(pulled, unpulled, sourceless, same-name folder wins, moved datasets dir, edited pull refused at prepare); the record following the copied table under a moved datasets dir through two successive entry edits (registry), around a failed fetch (first fetch failing with no manifest, a fetch failing after a pull) and after a run refused for the agent; a registry entry pulled in place reporting the edited table's digest with no "Rewrote" at the next prepare; refusals naming a committed entry's own folder ending with what to delete there (pull by name and by reference, run by reference, run by name after a hand edit; no hint under a moved datasets dir); a folder of the entry's name in the working directory not hiding the entry from the pull.tests/unit_tests/test_harbor_manifest.py:refresh_gym_layer(followsdataset.toml, keeps task digests and pins including a pin without a digest, leaves a pins-only manifest and a missing one alone).folder=parameter (test_harbor_tasks.py,test_harbor_package_store.py).test_a_pull_that_lost_its_manifest_is_refused_not_run_as_local(in-place and under a moved dir), the interrupted-fetch branch oftest_a_refusal_naming_the_entry_folder_says_what_to_delete_there,test_resolved_folder_...andtest_a_registry_reference_without_a_version_reports_unchanged_on_the_second_pull,test_a_moved_datasets_dir_gets_the_entry_s_dataset_level_files_too,test_a_pull_whose_fetch_rewrote_the_gym_layer_announces_the_digest_change, the exact-notice assertions intest_refresh_gym_layer_...andtest_a_registry_entry_pulled_in_place_..., andtest_global_config.py::test_cli_parse_restores_hydras_args_parser_instead_of_nesting_wrappers(the one change outsidenemo_gym/tasks/harbor/).environment_servers/harbor/tests -k "not partial_eval and not buildah and not validate_ and not real_image"): 470 passed, 2 deselected. Oracle suite: 38 passed. The ten touched core unit files plustest_global_config.py, in one session: see REPORT.md for the count.ruff check,ruff format --check,pre-commit run --files <changed files>: clean.NEMO_GYM_DATASETS_DIRon Lustre (the home quota on the build node is full):gym dataset pull terminal_bench_4end to end through the entry (66 tasks, digest42f85927f0ad…; second pull and the bareharbor:terminal-bench/terminal-bench@4.0.0pull both land in the entry's folder and report unchanged); the pinned TB 2.1 reference pulled into theterminal_bench_2_1entry and three of its tasks validated with the oracle on OpenSandbox through the entry name (all 1.0); the plainharbor:terminal-bench/terminal-bench-2-1reference pulled intoterminal-bench-2-1/and pulled again unchanged; the entry'sdataset.tomledited after the pull → the next pull refreshes the copy and the digest follows (no download), restored → digest back; the copy edited in place →gym dataset pullandgym dataset validaterefuse in the same one line with nothing written; an oldterminal-bench-4.0.0/beside the entry → theNote:line, gone once moved aside. Repair round 2, same layout: four successive edits of the entry'sdataset.tomlwith a pull between each (one with the package store unreachable), every pull's manifestdataset_toml_digestequal to the copy's digest and the second, third and fourth edits accepted (the case that was refused before); after the fifth edit and its pull,gym dataset validate terminal_bench_2_1 --only-tasks fix-git --sandbox opensandboxthrough the entry name printed noRewroteline and the oracle solved the task (1.0, exit 0);gym dataset pull terminal_bench_4frombenchmarks/(which holdsterminal_bench_4/) pulls the entry. Repair round 3, datasets dir node-local (/raid/scratch): the TB 2.1 pin pulled fresh (89 tasks), itsmanifest.tomlmoved aside,gym dataset validate/eval runby entry name both refused with the pull named and wrote nothing, the pull then recorded the kept folders in 2 s (no re-download) and a second pull said "unchanged"; an edit of the entry'sdataset.tomlmade the pull print exactly oneRewrote ...; dataset digest <old> -> <new>and the following unchanged pull print none. See REPORT.md.Checklist
🤖 Generated with Claude Code