From fb511ae6d9211418beec4711c68becd115fb16be Mon Sep 17 00:00:00 2001 From: Adil Asif Date: Wed, 21 Oct 2026 12:27:53 -0700 Subject: [PATCH] feat(harbor): gym dataset pull and committed entry sources (M38, second 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// declares its upstream in its own dataset.toml as the new `[gym] source` key (a harbor: reference). `gym dataset pull ` 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 ` and `gym eval run ` 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 // 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 Signed-off-by: Adil Asif --- .gitignore | 5 +- environment_servers/harbor/README.md | 80 +- .../harbor/terminal_bench_2_1/README.md | 5 + .../harbor/terminal_bench_2_1/dataset.toml | 10 + .../harbor/terminal_bench_4/README.md | 25 +- .../harbor/terminal_bench_4/dataset.toml | 7 + .../latest/pages/data/harbor-tasks.mdx | 84 +- .../terminal-bench-2-1-harbor.mdx | 12 +- .../latest/pages/reference/cli-commands.mdx | 26 +- nemo_gym/cli/main.py | 29 +- nemo_gym/tasks/harbor/__init__.py | 5 +- nemo_gym/tasks/harbor/cli.py | 292 ++- nemo_gym/tasks/harbor/dataset_config.py | 23 +- nemo_gym/tasks/harbor/entries.py | 473 +++++ nemo_gym/tasks/harbor/hub.py | 209 +- nemo_gym/tasks/harbor/manifest.py | 89 +- nemo_gym/tasks/harbor/package_store.py | 33 +- nemo_gym/tasks/harbor/scaffold.py | 3 + tests/unit_tests/test_harbor_entries.py | 1873 +++++++++++++++++ tests/unit_tests/test_harbor_manifest.py | 68 + tests/unit_tests/test_harbor_package_store.py | 44 +- tests/unit_tests/test_harbor_tasks.py | 124 ++ 22 files changed, 3447 insertions(+), 72 deletions(-) create mode 100644 environments/harbor/terminal_bench_2_1/dataset.toml create mode 100644 environments/harbor/terminal_bench_4/dataset.toml create mode 100644 nemo_gym/tasks/harbor/entries.py create mode 100644 tests/unit_tests/test_harbor_entries.py diff --git a/.gitignore b/.gitignore index 5d804a2de78..5775f0d9e9f 100644 --- a/.gitignore +++ b/.gitignore @@ -248,10 +248,13 @@ resources_servers/cvdp/data/gym_cvdp_nonagentic_code_generation_no_commercial.js datasets/ # Pulled Harbor datasets (nemo_gym/tasks/harbor/hub.py) land in environments/harbor// next to the -# committed per-dataset entries, which keep only their README and YAML files. +# committed per-dataset entries, which keep only their README, YAML files and dataset.toml (whose [gym] source +# names the dataset `gym dataset pull ` fetches into the entry's folder). The three names are kept in every +# folder here, a pulled hub folder included: a dataset.toml written into one by hand shows as untracked, on purpose. environments/harbor/*/* !environments/harbor/*/README.md !environments/harbor/*/*.yaml +!environments/harbor/*/dataset.toml environments/harbor/.harbor/ # Files prepare writes beside the example Harbor datasets under environments/ (recorded image configurations, diff --git a/environment_servers/harbor/README.md b/environment_servers/harbor/README.md index 86ee59e0fb4..572f00940ea 100644 --- a/environment_servers/harbor/README.md +++ b/environment_servers/harbor/README.md @@ -751,7 +751,11 @@ a changed `dataset.toml` fetches nothing: the pull only refreshes the digests th datasets folder (`$NEMO_GYM_DATASETS_DIR`, default `environments/harbor/` in the Gym checkout, or under the working directory when Gym is installed as a package). `harbor:/[@ | @sha256:]` fetches a package-store dataset instead (for example `harbor:terminal-bench/terminal-bench@4.0.0`): every task package is downloaded by content hash into `/` (`latest`), `-/` or `-<12 digest chars>/` (a digest pin), checked against the content hash Harbor published for it and recorded in -`manifest.toml`, whose per-task pins carry the task's package and that content hash, so a later fetch never mixes another package or a moved `latest` into the folder. A local task folder, or a folder of task folders, works the same way; task folders may be +`manifest.toml`, whose per-task pins carry the task's package and that content hash, so a later fetch never mixes another package or a moved `latest` into the folder. +`gym dataset pull ` is that fetch on its own: it prints the folder, the task count and the dataset +digest, refuses a stale, edited or foreign folder in the same words as the fetch inside `gym eval run`, and +prepares, builds and runs nothing. See [Committed dataset entries](#committed-dataset-entries) for +`gym dataset pull terminal_bench_4`, the entry-name form. A local task folder, or a folder of task folders, works the same way; task folders may be grouped in subfolders (the search is recursive, `task_id` stays the task folder's own name and must be unique across the dataset, and `dataset.toml` and the recorded image configurations sit at the dataset root). The taskset mapping records the relative path of a task found below the first level under `paths`. The fetch stages its clone in a hidden `.nemo-gym-harbor-*` directory inside the dataset folder, which the search skips. A fetch interrupted under a `main` build from before this change staged under an unhidden `nemo-gym-harbor-*` name; the recursive search sees that clone and refuses the dataset (a duplicate task ID, or a task the manifest does not record) rather than running it. Delete those directories once; nothing in them is needed. The generated run config declares the materialized rows as the agent's @@ -761,11 +765,78 @@ per task, the verifier's wall time (`verify_s`) and the network it ran under (`n requires a harness that accepts `sandbox_access` in its session seed and leaves the sandbox to the server when its session is closed. +### Committed dataset entries + +`environments/harbor//` commits what a checkout keeps for one dataset: a README, deployment YAML overlays +(`terminal_bench_4/harbor_sandbox.yaml`) and a `dataset.toml` whose `[gym]` table declares `source`, the hub +reference the task folders come from (`terminal_bench_4` declares `harbor:terminal-bench/terminal-bench@4.0.0`, +`terminal_bench_2_1` the content-hash pin of the store's TB 2.1). `gym dataset pull ` fetches that reference +into the entry's own folder, where the pulled task folders and `manifest.toml` are gitignored next to the committed +files, and a pull of the reference itself (`gym dataset pull harbor:terminal-bench/terminal-bench@4.0.0`, or +`gym eval run` on it) lands in the same folder, so one dataset never lives in two sibling folders. The match is +exact: `harbor:terminal-bench/terminal-bench` (the store's `latest`) is another reference and lands in +`environments/harbor/terminal-bench/` as any reference no entry declares does. `gym dataset validate ` and +`gym eval run ...` resolve the bare name to that folder without fetching (an entry not pulled yet names the +pull to run; an existing folder of the same name in the working directory, or an explicit path, keeps meaning the +path for these two commands, which take paths, while `gym dataset pull ` takes none and finds the entry from +any working directory), and the manifest check that follows refuses a pulled folder that was edited; task folders +in the entry's folder with no `manifest.toml`, or with one that records another source or a fetch of another +reference than the entry's `source` declares now (the declaration moved to another tag since the pull), are refused +with the pull to run, never run as a local dataset under the entry's name; the pull re-records a reference that +resolves to the tasks already there without fetching them again. The entry's +`dataset.toml` is the dataset's `dataset.toml`: its other `[gym]` keys apply as on any dataset, and the dataset digest covers it, so +`terminal_bench_4`'s identity differs from that of a bare `harbor:terminal-bench/terminal-bench@4.0.0` pull made +before the entry existed, on purpose. An entry is pulled into its own folder when that folder is under the working +root (the Gym checkout of an editable install, the working directory of a package install) and is not installed. An +installed entry, one a package install ships under `site-packages` or another Python installation prefix (shared, +often read-only; a project-local `.venv` under the working directory counts), and an entry committed outside the +working root (one a `NEMO_GYM_EXTRA_ROOTS` root commits) are pulled into `//` instead +(`environments/harbor//` under the working directory, the default every other pull uses), by name and by the +reference the entry declares alike, and the installation is left untouched; `NEMO_GYM_DATASETS_DIR` moves every +pulled dataset there. An entry's tasks +then live in `//` with a copy of the entry's Gym layer beside them, its `dataset.toml` and whatever dataset-level `task_data.py` or `verifier.py` it +commits, placed before the first task lands and replaced when the entry's files change (the next pull, validate or +run does it alike and brings the manifest beside the tasks up to date with the copies at once, so the three commands +never disagree about one folder or its digest; a copy edited in place is refused, never overwritten: the entry's +files are where the layer is edited, and the copies are checked against the record of what was placed, a hidden +`.gym-layer.json` beside them that only the placing writes, so a run of the folder by its path in a shell without +the variable, which rewrites the manifest's digests over an edit as for any dataset, does not make the next pull +overwrite it), and the entry name resolves to that folder. That copy folder, told by its `.gym-layer.json`, is never +itself an entry: the name and the reference keep resolving past it to the installed entry, so an edit of the +installed entry's `dataset.toml` reaches the next pull, validate or run, while a folder of the entry's name without +that file (one the checkout commits itself) keeps precedence over the installed one. The record file is what marks the +copy: delete it and the folder becomes a user-authored entry of that name and shadows the installed one again, so move +the folder aside instead. A copy whose installed entry was removed or renamed is no longer reached by the name; run it +by its path as a plain dataset. A checkout that fetched +`harbor:terminal-bench/terminal-bench@4.0.0` before the entry declared it still holds `environments/harbor/terminal-bench-4.0.0/`: the reference now lands in `terminal_bench_4/` (fetched afresh; +no folder is moved), the pull and a run of the reference name the old folder while it is there, and it can be +moved aside or deleted. Runs of that reference now carry the entry's name as the taskset, as the rows' +`task_source` and in the `results/harbor/terminal_bench_4/` path, and a dataset digest that covers the entry's +`dataset.toml`, so they differ from runs of the same reference made before the entry declared it. `dataset.toml` is +not gitignored in any folder under `environments/harbor/`, pulled hub folders included, so one written into such a +folder by hand (the `nextjs-performance` override an earlier TB4 README placed in `terminal-bench-4.0.0/`) now shows +as untracked: move its keys into the entry's `dataset.toml` and delete it. + +To move an entry to another version, change `source` in its `dataset.toml`, delete (or move aside) only the pulled +task folders and `manifest.toml` in the entry's folder, and pull again; the README, YAML files and `dataset.toml` +stay. A fetch refused because the folder was written for another version of the dataset, or because a task folder +no longer matches the store, says so in the fetcher's words and, when the folder is a committed entry's own, adds +what to delete there (the same hint follows the manifest check's refusal of a run by entry name; under +`NEMO_GYM_DATASETS_DIR`, or for an entry committed outside the working root, the pull's folder holds nothing git +tracks and the fetcher's words stand alone). A pull whose +fetch downloaded nothing, every folder being in place, still brings the manifest's digest of `dataset.toml` up to +date and announces the change (`Rewrote manifest.toml: dataset.toml changed; dataset digest -> +`, the manifest check's own notice), so the digest a pull reports after an edit of the entry's table is the one +the next run carries, and that run rewrites nothing. A run or pull of an entry whose fetch was interrupted says to run +`gym dataset pull ` again, which finishes it. Publishing a dataset to the hub is not part of this slice. + ### Dataset identity -`manifest.toml` beside the task folders holds derived facts only, never a copy of the name or version +`manifest.toml` beside the task folders holds derived facts only, never a copy of the dataset's name or version (those live in `dataset.toml`): `[dataset]` `source` (`harbor-registry`, `harbor-package-store`, `git` or -`local`), `digest` (the dataset's identity: a hash over the sorted per-task digests, `dataset.toml` and, when the +`local`), `ref` (the hub reference the fetch was made from, a fact of the fetch like the pins: +`harbor:/@` for the store, `harbor:@` with the version +resolved for the registry; absent for a local folder), `digest` (the dataset's identity: a hash over the sorted per-task digests, `dataset.toml` and, when the dataset ships them, the dataset-level `task_data.py` and `verifier.py`), `dataset_toml_digest` on its own, `dataset_files_digest` (the two Gym files, written only when at least one is present, so a dataset without them hashes and reads as before), and per task @@ -1038,7 +1109,8 @@ The loader generates the task mapping and dataset rows for that run. datasets folder (`$NEMO_GYM_DATASETS_DIR`, default `environments/harbor/` in the Gym checkout, or under the working directory when Gym is installed as a package). `harbor:/[@ | @sha256:]` fetches a package-store dataset instead (for example `harbor:terminal-bench/terminal-bench@4.0.0`): every task package is downloaded by content hash into `/` (`latest`), `-/` or `-<12 digest chars>/` (a digest pin), checked against Gym's task digest and recorded in -`manifest.toml`. A local task folder, or a folder of task folders, works the same way. +`manifest.toml`. `gym dataset pull ` is the fetch alone (see +[Committed dataset entries](#committed-dataset-entries)). A local task folder, or a folder of task folders, works the same way. ## Compose template variables and `env_file` diff --git a/environments/harbor/terminal_bench_2_1/README.md b/environments/harbor/terminal_bench_2_1/README.md index fd55ac959b6..c92a72cec76 100644 --- a/environments/harbor/terminal_bench_2_1/README.md +++ b/environments/harbor/terminal_bench_2_1/README.md @@ -4,3 +4,8 @@ The 89 Terminal Bench 2.1 tasks run as published Harbor task folders through the server, with no Gym-side patching; the commands and the known upstream noise are documented in [Evaluate Terminal-Bench 2.1 on the Harbor Path](https://docs.nvidia.com/nemo/gym/main/tutorials/evaluation-tutorials/terminal-bench-2-1-harbor). The legacy benchmark folder, `benchmarks/terminal_bench_2_1/`, is untouched. + +`dataset.toml` here pins the dataset's source, Harbor's package store copy by content hash (the store tags it +`latest` only). `gym dataset pull terminal_bench_2_1` fetches the 89 task folders into this folder (gitignored +beside this README), and `gym dataset validate terminal_bench_2_1 --sandbox opensandbox` and +`gym eval run terminal_bench_2_1 ...` resolve the entry name to it. diff --git a/environments/harbor/terminal_bench_2_1/dataset.toml b/environments/harbor/terminal_bench_2_1/dataset.toml new file mode 100644 index 00000000000..042502832b5 --- /dev/null +++ b/environments/harbor/terminal_bench_2_1/dataset.toml @@ -0,0 +1,10 @@ +# Terminal Bench 2.1 on the Harbor task shape: the committed entry for the package-store dataset. +# `gym dataset pull terminal_bench_2_1` fetches the source below into this folder (the task folders and +# manifest.toml it writes are gitignored); `gym dataset validate terminal_bench_2_1` and +# `gym eval run terminal_bench_2_1 ...` then resolve the entry name to this folder. +[gym] +# Harbor's package store publishes the 89 tasks under the `latest` tag only, so the entry pins the dataset by +# its content hash (the version `latest` resolved to on 29 Apr 2026). A pull of this pinned reference lands +# here; the plain `harbor:terminal-bench/terminal-bench-2-1` reference (the store's current `latest`) is another +# reference and lands in environments/harbor/terminal-bench-2-1/ as before. +source = "harbor:terminal-bench/terminal-bench-2-1@sha256:7d7bdc1cbedad549fc1140404bd4dc45e5fd0ea7c4186773687d177ad3a0699a" diff --git a/environments/harbor/terminal_bench_4/README.md b/environments/harbor/terminal_bench_4/README.md index 82700799d42..2f191856b86 100644 --- a/environments/harbor/terminal_bench_4/README.md +++ b/environments/harbor/terminal_bench_4/README.md @@ -5,21 +5,34 @@ server, without the `terminal_bench_4` benchmark's own server. The dataset comes pinned by content hash. The legacy benchmark folder, `benchmarks/terminal_bench_4/`, is untouched. ```sh -gym dataset validate harbor:terminal-bench/terminal-bench@4.0.0 --sandbox opensandbox \ +gym dataset pull terminal_bench_4 +gym dataset validate terminal_bench_4 --sandbox opensandbox \ --config environments/harbor/terminal_bench_4/harbor_sandbox.yaml -gym eval run harbor:terminal-bench/terminal-bench@4.0.0 --agent miniswe_sandboxed_agent --sandbox opensandbox \ +gym eval run terminal_bench_4 --agent miniswe_sandboxed_agent --sandbox opensandbox \ --config environments/harbor/terminal_bench_4/harbor_sandbox.yaml \ --model-type openai_model --model --model-url ``` +`dataset.toml` here declares the dataset's source, `harbor:terminal-bench/terminal-bench@4.0.0`. The pull fetches +it into this folder (the 66 task folders and `manifest.toml` are gitignored beside this README), a pull of that +reference itself lands here too, and the two commands above resolve the entry name to this folder; the +`harbor:...` reference still works as a target and fetches into the same place. A checkout that fetched the reference +before this entry declared it holds `environments/harbor/terminal-bench-4.0.0/` as well: nothing reads it any more +(the pull says so while it is there), so move it aside or delete it; a `dataset.toml` written into that old folder +(the `nextjs-performance` override an earlier version of this README placed there) is no longer gitignored and shows +as untracked, so move its keys below the `[gym]` table here and delete it. Runs through the entry name use +`terminal_bench_4` as the taskset, as the rows' `task_source` and in the `results/harbor/terminal_bench_4/` path, and +their dataset digest covers this folder's `dataset.toml`, so they differ from runs of the bare reference made before. +To pull another version, change `source` here, delete only the pulled task folders and `manifest.toml` in this +folder, and pull again. + `harbor_sandbox.yaml` overlays the shipped OpenSandbox block with the status-poll cadence the legacy benchmark's server runs at (0.25 s backing off to 2 s). The shipped cap of 30 s is sized for about 1,500 concurrent SWE-bench samples and adds about 11 s per TB4 task between the agent's last command and the trajectory write. -Both commands resolve the same reference through the package store. The first fetch writes the task -folders into `environments/harbor/terminal-bench-4.0.0/` and every later fetch keeps each folder whose content hash -still matches, so the model run reuses the validated tasks. `gym dataset validate` runs every reference +Every fetch keeps each task folder whose content hash still matches and refuses one that was edited, so the +model run reuses the validated tasks. `gym dataset validate` runs every reference solution through the separate verifier. `--agent terminus_2_sandboxed_agent` runs Terminus 2 instead. The 11 Compose tasks start sidecar images whose entrypoint, command, user and ports live in the image, not the task. Preparing the dataset records them from the registry into `compose-images.json` next to the task folders, @@ -36,7 +49,7 @@ with the legacy benchmark's server, which sets `CIRCLE_NODE_TOTAL=3` for `nextjs override beside the fetched tasks; the loader applies it and nothing else changes: ```toml -# environments/harbor/terminal-bench-4.0.0/dataset.toml +# environments/harbor/terminal_bench_4/dataset.toml, below the [gym] source line [gym.tasks."nextjs-performance".environment] env = { CIRCLE_NODE_TOTAL = "3" } ``` diff --git a/environments/harbor/terminal_bench_4/dataset.toml b/environments/harbor/terminal_bench_4/dataset.toml new file mode 100644 index 00000000000..18d4fd6a1ec --- /dev/null +++ b/environments/harbor/terminal_bench_4/dataset.toml @@ -0,0 +1,7 @@ +# Terminal Bench 4 on the Harbor task shape: the committed entry for the package-store dataset. +# `gym dataset pull terminal_bench_4` fetches the source below into this folder (the task folders and +# manifest.toml it writes are gitignored); `gym dataset validate terminal_bench_4` and +# `gym eval run terminal_bench_4 ...` then resolve the entry name to this folder. +[gym] +# Harbor's package store, tag 4.0.0 (66 tasks). A pull of this reference lands here too. +source = "harbor:terminal-bench/terminal-bench@4.0.0" diff --git a/fern/versions/latest/pages/data/harbor-tasks.mdx b/fern/versions/latest/pages/data/harbor-tasks.mdx index a4c966af063..4dd8c1ae3bd 100644 --- a/fern/versions/latest/pages/data/harbor-tasks.mdx +++ b/fern/versions/latest/pages/data/harbor-tasks.mdx @@ -58,6 +58,9 @@ allowed_agents = ["terminus_2_sandboxed_agent", "hermes_agent"] # with the values named), and whether the rewards were baselined and reviewed (false unless declared). domain = "coding" verified = false +# A committed entry under environments/harbor/ names the hub dataset `gym dataset pull ` fetches into its +# folder (see below). An authored dataset, whose task folders are its own, leaves it unset. +# source = "harbor:terminal-bench/terminal-bench@4.0.0" [gym.defaults] # Rollouts per task for an evaluation. `+num_repeats=` on the command line replaces it; it never enters a task's digest. @@ -76,7 +79,8 @@ env = { EXAMPLE = "1" } memory_mb = 4096 ``` -The generated run config's `datasets:` entry carries `type`, `license`, `domain`, `verified` and `allowed_agents` +`source` is provenance for a committed entry and shapes no task (it does not enter the `datasets:` entry; the +dataset digest covers the file that holds it). The generated run config's `datasets:` entry carries `type`, `license`, `domain`, `verified` and `allowed_agents` as declared, its top-level `num_repeats` is `[gym.defaults].num_repeats`, and `gym eval run` starts from that split and repeat count (`+split=` and `+num_repeats=` on the command line replace them); `gym dataset validate` plays each reference solution once unless `+num_repeats=` says otherwise. None of these keys enters a task's `_ng_digest`; only @@ -356,6 +360,79 @@ gym dataset init acme/my-dataset --root ~/work --image ubuntu:24.04 # ~/work/m | `--image IMAGE` | The task's prebuilt image (default: `python:3.12-slim`). | | `--parameterized` | Scaffold a template folder (`task.toml`, `instruction.template.md`, `tasks.jsonl`, `task_data.py`, a Python verifier, the reference answer as `solution/answer.template.md`) instead of one task. | +## `gym dataset pull` and committed entries + +`gym dataset pull TARGET` fetches a Harbor dataset into `environments/harbor/` of the Gym checkout, or under the +working directory when Gym is installed as a package (`$NEMO_GYM_DATASETS_DIR` when set), and prints where it landed, +its task count and its dataset digest. It is the fetch `gym eval run` and `gym dataset validate` make for a `harbor:` +target, on its own: nothing is prepared, built or run, a folder that was edited by hand, left by an interrupted fetch +of another version, or written for another package is refused in the same words, and a second pull of an unchanged +dataset downloads nothing and says so. + +```bash +gym dataset pull terminal_bench_4 # the entry's source, into environments/harbor/terminal_bench_4/ +gym dataset pull harbor:terminal-bench/terminal-bench@4.0.0 # the same folder: the entry declares this reference +gym dataset pull harbor:terminal-bench/terminal-bench-2-1 # no entry declares `latest`: environments/harbor/terminal-bench-2-1/ +gym dataset pull harbor:terminal-bench@2.0 # a registry dataset (registry.json cached under environments/harbor/.harbor/) +``` + +| Option | Description | +| --- | --- | +| `TARGET` | `harbor:/[@ \| @sha256:]` (a package-store dataset), `harbor:[@]` (a registry dataset), or the name of a committed entry. An entry under the working root (the checkout, or the working directory of a package install) is pulled into its own folder; an installed entry (one a package install ships under `site-packages`, a project-local `.venv` included) or one committed outside the working root is pulled into `environments/harbor//` under the working directory (`$NEMO_GYM_DATASETS_DIR` when set), by its name and by the reference it declares alike, and the installation is left untouched. That copy folder never shadows the installed entry (its `.gym-layer.json` tells it apart): lookup keeps resolving to the entry, so its edited `dataset.toml` reaches the next pull, while an entry of the same name the checkout commits itself keeps precedence. The record file marks the copy: delete it and the folder becomes a user-authored entry that shadows the installed one, so move the folder aside instead; a copy whose installed entry is gone is run by its path. | + +A **committed entry** is a folder `environments/harbor//` the checkout keeps for one dataset: a README, +deployment YAML overlays and a `dataset.toml` whose `[gym]` table declares `source`, the hub reference its task +folders come from. `terminal_bench_4` declares `harbor:terminal-bench/terminal-bench@4.0.0`; `terminal_bench_2_1` +pins the store's Terminal-Bench 2.1 by content hash, since the store tags it `latest` only. The pulled task +folders and `manifest.toml` are gitignored next to the committed files, and a pull of the declared reference +itself lands in the same folder, so one dataset never lives in two sibling folders. The match is exact: +`harbor:terminal-bench/terminal-bench` (the store's `latest`) is another reference and lands in +`environments/harbor/terminal-bench/`, as any reference no entry declares does. The entry's `dataset.toml` is the +dataset's: its other `[gym]` keys apply as on any dataset and the dataset digest covers it. An entry is pulled into +its own folder when that folder is under the working root (the checkout of an editable install, the working directory +of a package install) and is not installed; an installed entry, one a package install ships under `site-packages` or +another Python installation prefix (shared, often read-only; a project-local `.venv` under the working directory +counts), or an entry committed outside the working root (one a `NEMO_GYM_EXTRA_ROOTS` root commits), is pulled into +`//` with a copy of its Gym layer, as under `NEMO_GYM_DATASETS_DIR` below, and its own folder is +left untouched. That copy folder, told by its `.gym-layer.json`, is never itself an entry: the name and the reference +keep resolving past it to the installed entry, so an edit of the installed entry's `dataset.toml` reaches the next +pull, validate or run, while a folder of the entry's name without that file (one the checkout commits itself) keeps +precedence over the installed one. `gym dataset validate +` and `gym eval run ` read what the pull left and refuse +task folders in the entry's folder that carry no record of the declared fetch (no `manifest.toml`, or one of another +source, or one recording a fetch of another reference than `source` declares now, since the manifest records the +reference each fetch was made from), with the pull to run, instead of running them as a local dataset under the +entry's name. With `NEMO_GYM_DATASETS_DIR` +set, an entry's tasks live in `//` with a copy of the entry's Gym layer beside them, its +`dataset.toml` and whatever dataset-level `task_data.py` or `verifier.py` it commits, placed before +the first task lands and replaced when the entry's files change (by the next pull, validate or run alike, which +brings the manifest beside the tasks up to date with the copies at once and announces the digest change; a copy +edited in place is refused, never overwritten, however the folder was run since: edit the entry's files. The copies +are checked against a hidden `.gym-layer.json` beside them that records what was placed). + +A checkout that fetched `harbor:terminal-bench/terminal-bench@4.0.0` before the entry declared it still holds +`environments/harbor/terminal-bench-4.0.0/`: the reference now lands in `terminal_bench_4/` (fetched afresh; no +folder is moved), the pull and a run of the reference name the old folder while it is there, and it can be moved +aside or deleted. Runs of that reference now carry the entry's name as the taskset, as the rows' `task_source` and in +the `results/harbor/terminal_bench_4/` path, and a dataset digest that covers the entry's `dataset.toml`, so they +differ from runs of the same reference made before the entry declared it. `dataset.toml` is not gitignored in any +folder under `environments/harbor/`, pulled hub folders included, so one written into such a folder by hand (the +`nextjs-performance` override an earlier TB4 README placed in `terminal-bench-4.0.0/`) now shows as untracked: move +its keys into the entry's `dataset.toml` and delete it. + +To move an entry to another version, change `source` in its `dataset.toml`, delete (or move aside) only the pulled +task folders and `manifest.toml` in the entry's folder, and pull again; the README, YAML files and `dataset.toml` +stay. A fetch refused because the folder was written for another version of the dataset, or because a task folder no +longer matches the store, says so in the fetcher's words and, when the folder is a committed entry's own, adds what +to delete there. A pull whose fetch downloaded nothing still brings the manifest's digest of `dataset.toml` up to +date, so the digest it reports after an edit of the entry's table is the one the next run carries. + +`gym dataset validate ` and `gym eval run ...` resolve the bare name to that folder without +fetching: an entry not pulled yet names the pull to run, and an existing folder of the same name in the working +directory, or an explicit path, keeps meaning the path for these two commands, which take paths (`gym dataset pull +` takes none and finds the entry from any working directory). Publishing a dataset to the hub is not part of +this command. + ## `gym dataset validate` Runs each task's reference solution (`solution/solve.sh`) through the task's verifier with an oracle harness and @@ -366,11 +443,12 @@ evaluation run. ```bash gym dataset validate my-dataset --sandbox opensandbox gym dataset validate harbor:terminal-bench/terminal-bench@4.0.0 --sandbox opensandbox --only-tasks 'hello-*' +gym dataset validate terminal_bench_4 --sandbox opensandbox # the committed entry, after `gym dataset pull terminal_bench_4` ``` | Option | Description | | --- | --- | -| `TARGET` | A task folder, a folder of task folders, or a `harbor:[@]` hub reference. | +| `TARGET` | A task folder, a folder of task folders, a `harbor:[@]` hub reference, or the name of a committed entry under `environments/harbor/` after `gym dataset pull`. | | `--sandbox PROVIDER` | Sandbox provider for the task containers (for example `opensandbox`, `docker`). | | `--config PATH` | Extra config file to load. Repeatable. | | `--only-tasks PATTERNS` | Comma-separated task names or globs; only matching tasks are validated. | @@ -395,7 +473,7 @@ gym eval run harbor:terminal-bench/terminal-bench@4.0.0 --agent hermes_agent --s | Option | Description | | --- | --- | -| `TARGET` | A task folder, a folder of task folders, or `harbor:[@]`. `harbor:/[@]` fetches a package-store dataset. Fetched datasets land in `$NEMO_GYM_DATASETS_DIR` (default `environments/harbor/` in the Gym checkout); the fetch stages its clone in a hidden `.nemo-gym-harbor-*` directory there, which discovery skips. An unhidden `nemo-gym-harbor-*` directory left by a fetch interrupted under a `main` build from before this change is refused as a duplicate task ID: delete it once. | +| `TARGET` | A task folder, a folder of task folders, `harbor:[@]`, or the name of a committed entry under `environments/harbor/` after `gym dataset pull` (for example `terminal_bench_4`). `harbor:/[@]` fetches a package-store dataset. Fetched datasets land in `$NEMO_GYM_DATASETS_DIR` (default `environments/harbor/` in the Gym checkout, or under the working directory when Gym is installed as a package), in the folder of the entry that declares the reference as its `[gym] source` when one does; the fetch stages its clone in a hidden `.nemo-gym-harbor-*` directory there, which discovery skips. An unhidden `nemo-gym-harbor-*` directory left by a fetch interrupted under a `main` build from before this change is refused as a duplicate task ID: delete it once. | | `--agent`, `-a` | The harness that works in the task sandbox. It must accept `sandbox_access` in its session seed, and the dataset's `[gym] allowed_agents`, when set, must name it; otherwise the run stops before any image is built or server started. | | `--sandbox PROVIDER` | Sandbox provider for the task containers. | | `--only-tasks`, `--exclude-tasks` | Select tasks by name or glob. Left-out tasks are listed on stdout. | diff --git a/fern/versions/latest/pages/evaluation-tutorials/terminal-bench-2-1-harbor.mdx b/fern/versions/latest/pages/evaluation-tutorials/terminal-bench-2-1-harbor.mdx index 21ce661ea19..6c1d448e1f8 100644 --- a/fern/versions/latest/pages/evaluation-tutorials/terminal-bench-2-1-harbor.mdx +++ b/fern/versions/latest/pages/evaluation-tutorials/terminal-bench-2-1-harbor.mdx @@ -46,10 +46,16 @@ gym eval run harbor:terminal-bench/terminal-bench-2-1 --agent terminus_2_sandbox ``` The plain reference resolves the store's `latest` tag, writes the task folders into `environments/harbor/terminal-bench-2-1/` -and records the package and the version it resolved in the `manifest.toml` there. A later fetch keeps every folder +and records the package and the version it resolved in the `manifest.toml` there. `gym dataset pull ` +makes the same fetch on its own, printing the folder, the task count and the dataset digest, and the committed +entry `environments/harbor/terminal_bench_2_1/` pins the version below as its `[gym] source`, so +`gym dataset pull terminal_bench_2_1` fetches it into that folder and `gym dataset validate terminal_bench_2_1 +--sandbox opensandbox` then runs it by name. A later fetch keeps every folder whose content still matches, and once `latest` has moved on it refuses the folder and names both versions, so a -stale copy is never mixed with a newer one. To pin the version, name its content hash; the folder then carries the -first twelve characters of the hash (`environments/harbor/terminal-bench-2-1-7d7bdc1cbeda/`): +stale copy is never mixed with a newer one. To pin the version, name its content hash. The pin below is the +`terminal_bench_2_1` entry's `[gym] source`, so it lands in `environments/harbor/terminal_bench_2_1/` +(`gym dataset pull terminal_bench_2_1` is the short form of the same fetch); a pin no entry declares lands in a +folder named after the first twelve characters of its hash (`environments/harbor/-<12 digest chars>/`) instead: ```bash gym dataset validate harbor:terminal-bench/terminal-bench-2-1@sha256:7d7bdc1cbedad549fc1140404bd4dc45e5fd0ea7c4186773687d177ad3a0699a \ diff --git a/fern/versions/latest/pages/reference/cli-commands.mdx b/fern/versions/latest/pages/reference/cli-commands.mdx index 31cd5244f6c..94223caa956 100644 --- a/fern/versions/latest/pages/reference/cli-commands.mdx +++ b/fern/versions/latest/pages/reference/cli-commands.mdx @@ -36,6 +36,7 @@ gym dataset render # generate a dataset preview (materialize prompts) gym dataset collate # validate and collate a dataset gym dataset init # scaffold a Harbor-format dataset (dataset.toml plus one task; --parameterized for a template) gym dataset publish --harbor --output # write a Gym-authored dataset as plain Harbor task folders +gym dataset pull # fetch a Harbor dataset (hub reference or committed entry) into environments/harbor/ gym dataset validate # run each Harbor task's reference solution through its verifier # Environments @@ -355,6 +356,26 @@ instruction, a verifier and a reference solution. Refer to [Harbor Tasks](/main/ gym dataset init my-dataset --image ubuntu:24.04 # dataset.toml names it local/my-dataset ``` +### `gym dataset pull` + +Fetch a Harbor dataset into `environments/harbor/` of the Gym checkout, or under the working directory when Gym is +installed as a package (`$NEMO_GYM_DATASETS_DIR` when set), and print where it landed, its task count and its +dataset digest. Nothing is prepared, built or run. The fetch is the +one `gym eval run` and `gym dataset validate` make for a `harbor:` target, so a folder that was edited by hand, +left by an interrupted fetch of another version, or written for another package is refused in the same words; +a second pull of an unchanged dataset downloads nothing and says so. Refer to +[Harbor Tasks](/main/data/harbor-tasks#gym-dataset-pull-and-committed-entries). + +| Option | Description | +| --- | --- | +| `TARGET` | `harbor:/[@ \| @sha256:]` (a package-store dataset), `harbor:[@]` (a registry dataset), or the name of a committed entry under `environments/harbor/` whose `dataset.toml` declares `[gym] source` (for example `terminal_bench_4`). A reference an entry declares as its source lands in that entry's folder when the entry is under the working root (the checkout, or the working directory of a package install) and is not installed; an installed entry (one a package install ships under `site-packages`, a project-local `.venv` included) or one committed outside the working root is pulled, by name and by reference alike, into `environments/harbor//` under the working directory (`$NEMO_GYM_DATASETS_DIR` when set), leaving the installation untouched. That copy folder never shadows the installed entry (its `.gym-layer.json` tells it apart), so an edit of the installed entry's `dataset.toml` reaches the next pull, while an entry of the same name the checkout commits itself keeps precedence. An entry name is found from any working directory (the pull takes no path). | + +```bash +gym dataset pull terminal_bench_4 # the entry's source, into environments/harbor/terminal_bench_4/ +gym dataset pull harbor:terminal-bench/terminal-bench@4.0.0 # the same folder: the entry declares this reference +gym dataset pull harbor:terminal-bench/terminal-bench-2-1 # no entry declares `latest`: environments/harbor/terminal-bench-2-1/ +``` + ### `gym dataset validate` Run each Harbor task's reference solution (`solution/solve.sh`) through its verifier with the oracle harness @@ -367,7 +388,7 @@ selected task had a solution to run. Refer to [Harbor Tasks](/main/data/harbor-t | Option | Description | | --- | --- | -| `TARGET` | A task folder, a folder of task folders, or a `harbor:[@]` hub reference. | +| `TARGET` | A task folder, a folder of task folders, a `harbor:[@]` hub reference, or the name of a committed entry under `environments/harbor/` (for example `terminal_bench_4`) after `gym dataset pull`. | | `--sandbox PROVIDER` | Sandbox provider for the task containers (for example `opensandbox`, `docker`). | | `--config PATH` | Config file to load. Repeatable. | | `--only-tasks PATTERNS` | Comma-separated task names or globs; only matching tasks are validated. | @@ -379,6 +400,7 @@ selected task had a solution to run. Refer to [Harbor Tasks](/main/data/harbor-t ```bash gym dataset validate my-dataset --sandbox opensandbox gym dataset validate harbor:terminal-bench/terminal-bench@4.0.0 --sandbox opensandbox --only-tasks 'hello-*' +gym dataset validate terminal_bench_4 --sandbox opensandbox # the committed entry, after `gym dataset pull terminal_bench_4` ``` ### `gym dataset upload` @@ -800,7 +822,7 @@ gym eval plan --schema Collate data, start the servers, and collect rollouts. This is the main evaluation command. By default it spins up all required servers. Pass `--no-serve` to collect against servers you already started with `gym env start`. -In place of `--benchmark`, a positional `TARGET` runs a dataset in the Harbor task format: a task folder, a folder of task folders, or a `harbor:[@]` hub reference. Refer to [Harbor Tasks](/main/data/harbor-tasks) for the layout and the `--sandbox`, `--only-tasks` and `--exclude-tasks` options that apply to such a target. +In place of `--benchmark`, a positional `TARGET` runs a dataset in the Harbor task format: a task folder, a folder of task folders, a `harbor:[@]` hub reference, or the name of a committed entry under `environments/harbor/` (for example `terminal_bench_4`) after `gym dataset pull`. Refer to [Harbor Tasks](/main/data/harbor-tasks) for the layout and the `--sandbox`, `--only-tasks` and `--exclude-tasks` options that apply to such a target. | Option | Description | | --- | --- | diff --git a/nemo_gym/cli/main.py b/nemo_gym/cli/main.py index a4003b10108..29c1000a139 100644 --- a/nemo_gym/cli/main.py +++ b/nemo_gym/cli/main.py @@ -729,7 +729,8 @@ def _eval_run(args: argparse.Namespace, overrides: list[str]) -> None: nargs="?", metavar="TARGET", help=( - "Harbor task folder, folder of task folders, or `harbor:[@]` hub reference. " + "Harbor task folder, folder of task folders, `harbor:[@]` hub reference, or the " + "name of a committed entry under environments/harbor/ (e.g. terminal_bench_4) after `gym dataset pull`. " "Existing folders (including expanded ~user paths) and explicit ./, ../, /, or ~/ paths " "take precedence over Hydra overrides. Prefix an ambiguous relative path with ./; " "otherwise key=value and ~key are configuration overrides." @@ -884,6 +885,13 @@ def _dataset_validate(args: argparse.Namespace, overrides: list[str]) -> None: validate_target(args, overrides) +def _dataset_pull(args: argparse.Namespace, overrides: list[str]) -> None: + """`gym dataset pull TARGET`: fetch a Harbor dataset into environments/harbor/ without preparing or running it.""" + from nemo_gym.tasks.harbor.cli import pull_target + + pull_target(args, overrides) + + def _dataset_publish(args: argparse.Namespace, overrides: list[str]) -> None: """`gym dataset publish --harbor TARGET --output DIR`: plain Harbor task folders, checked by Harbor's own loader.""" from nemo_gym.tasks.harbor.errors import HarborTaskError @@ -1104,6 +1112,22 @@ def _dev_compare(args: argparse.Namespace, overrides: list[str]) -> None: ), ), ), + "dataset pull": Command( + target=_dataset_pull, + summary="Fetch a Harbor dataset into environments/harbor/ and report its folder, task count and digest.", + flags=( + Flag( + register=lambda p: p.add_argument( + "target", + metavar="TARGET", + help="`harbor:/[@ | @sha256:]` (package store), " + "`harbor:[@]` (registry), or the name of a committed entry under " + "environments/harbor/ whose dataset.toml declares `[gym] source` (e.g. terminal_bench_4). " + "A reference an entry declares lands in that entry's folder.", + ) + ), + ), + ), "dataset validate": Command( target=_dataset_validate, summary="Run each Harbor task's reference solution through the verifier and report the rewards.", @@ -1112,7 +1136,8 @@ def _dev_compare(args: argparse.Namespace, overrides: list[str]) -> None: register=lambda p: p.add_argument( "target", metavar="TARGET", - help="Harbor task folder, folder of task folders, or `harbor:[@]`.", + help="Harbor task folder, folder of task folders, `harbor:[@]`, or the name " + "of a committed entry under environments/harbor/ (e.g. terminal_bench_4) after `gym dataset pull`.", ) ), SANDBOX, diff --git a/nemo_gym/tasks/harbor/__init__.py b/nemo_gym/tasks/harbor/__init__.py index c755143f94e..25ded63d3a5 100644 --- a/nemo_gym/tasks/harbor/__init__.py +++ b/nemo_gym/tasks/harbor/__init__.py @@ -6,7 +6,10 @@ Harbor is the input format; Gym owns execution. A task is a folder holding ``task.toml``, ``instruction.md``, ``environment/`` and ``tests/``. A dataset is a folder of task folders. ``harbor:[@]`` names a dataset in the -Harbor registry, which this package fetches into ``environments/harbor/`` in the Gym checkout. +Harbor registry, which this package fetches into ``environments/harbor/`` in the Gym checkout, or +under the working directory when Gym is installed as a package (``gym dataset pull``; +:func:`nemo_gym.tasks.harbor.hub.datasets_dir`); a committed entry there declares its dataset as +``[gym] source`` and is pulled into its own folder (:mod:`nemo_gym.tasks.harbor.entries`). """ from nemo_gym.tasks.harbor.digest import DIGEST_KEY, content_hash diff --git a/nemo_gym/tasks/harbor/cli.py b/nemo_gym/tasks/harbor/cli.py index fae79b99869..11761187f37 100644 --- a/nemo_gym/tasks/harbor/cli.py +++ b/nemo_gym/tasks/harbor/cli.py @@ -1,9 +1,11 @@ # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -"""``gym eval run --agent ``: prepare a taskset and run it. +"""``gym eval run --agent ``: prepare a taskset and run it. -Preparation writes, under ``results/harbor//``: +A target is a task folder, a folder of task folders, a ``harbor:`` reference (fetched first) or the name of a +committed entry under ``environments/harbor/`` (resolved to the folder its pull filled, never fetched here; +``gym dataset pull``, :func:`pull_dataset`, is the fetch). Preparation writes, under ``results/harbor//``: - ``tasks.jsonl``: one materialized row per loadable task; - ``skipped.json``: omitted task IDs and reasons (empty when all tasks load); @@ -21,6 +23,7 @@ import fnmatch import json import logging +import shlex import sys from collections.abc import Callable, Mapping from dataclasses import dataclass, field, replace @@ -49,8 +52,26 @@ ) from nemo_gym.tasks.harbor.compose_overlay import HarborComposeOverlay from nemo_gym.tasks.harbor.dataset_config import DATASET_FILE, GymDatasetConfig, read_dataset_config +from nemo_gym.tasks.harbor.entries import ( + DatasetEntry, + PullPlan, + entry_at, + entry_target, + plan_pull, + prepare_destination, +) from nemo_gym.tasks.harbor.errors import HarborTaskError -from nemo_gym.tasks.harbor.hub import HubError, datasets_dir, fetch_ref, is_hub_ref +from nemo_gym.tasks.harbor.hub import ( + REGISTRY_SOURCE, + HubError, + HubRef, + datasets_dir, + declares_recorded_fetch, + expected_folder, + fetch_ref, + is_hub_ref, + resolved_folder, +) from nemo_gym.tasks.harbor.image_configs import ( load_compose_document, record_base_images, @@ -66,6 +87,9 @@ dataset_digest, dataset_files_digest, dataset_toml_digest, + gym_layer_change, + refresh_gym_layer, + report_rewrite, ) from nemo_gym.tasks.harbor.materialize import ( ENVIRONMENT_SERVER_IMPL, @@ -73,6 +97,7 @@ run_config, write_rows, ) +from nemo_gym.tasks.harbor.package_store import PACKAGE_STORE_SOURCE from nemo_gym.tasks.harbor.parameterized import is_template_folder, render_template_folder from nemo_gym.tasks.harbor.prepare_builds import _build_dockerfiles from nemo_gym.tasks.harbor.records import read_image_configs @@ -442,11 +467,25 @@ def prepare_target( which neither ``gym eval run`` nor ``gym dataset validate`` runs. """ build = build or BuildOptions() + entry: DatasetEntry | None = None if is_hub_ref(target): - folder = fetch_ref(target, refresh_registry=refresh_registry) - print(f"Fetched {target} into {folder}") + pulled = pull_dataset(target, refresh_registry=refresh_registry) + print(pulled.fetched_line()) + if pulled.superseded is not None: + print(pulled.superseded_line()) + folder, entry = pulled.folder, pulled.plan.entry + elif (entry := entry_target(target)) is not None: + folder = entry_folder(entry) else: folder = Path(target).expanduser().resolve() + entry = entry_at(folder) + if entry is not None: + # The path names a committed entry's pull elsewhere than its folder (a moved NEMO_GYM_DATASETS_DIR, or an + # entry committed outside the working root): the copy of the entry's Gym layer beside the tasks is kept + # current, or refused as edited in place, exactly as the pull and the entry's name do, so the manifest + # check below never absorbs an edit of the copy that the next pull would then overwrite with the entry's + # file. + prepare_destination(entry, create=False) taskset = folder.name template = folder if folder.is_dir() and is_template_folder(folder) else None if template is not None: @@ -471,7 +510,10 @@ def prepare_target( # The refusal of a hand-edited fetched dataset comes first, so no image of unverified content is # built, pushed or recorded. A single task folder has no manifest of its own (see _dataset_identity). digests: dict[str, str] = {} - identity = check_manifest(folder, digests=digests) if folder.is_dir() and not is_task_folder(folder) else None + try: + identity = check_manifest(folder, digests=digests) if folder.is_dir() and not is_task_folder(folder) else None + except ManifestError as exc: + raise with_entry_hint(exc, entry, folder) from exc # The dataset's TaskData (task_data.py beside dataset.toml) is imported once here, before any image is # recorded or built: a module that does not import refuses the dataset now, and so does a task folder whose # task_data.json the model refuses (the fields need no loaded task, and a dataset refused as a whole should not @@ -614,6 +656,244 @@ def selected(task_id: str) -> bool: ) +def entry_folder(entry: DatasetEntry) -> Path: + """The folder a committed entry's target resolves to: where its pull landed (:attr:`DatasetEntry.destination`). + + Nothing is fetched here: ``gym dataset validate `` and ``gym eval run `` read what + ``gym dataset pull `` left, and the manifest check that follows refuses a folder that no longer + matches that pull. For an entry that declares a source, the folder must hold that pull's record: task folders + with no ``manifest.toml``, or with one that records another source (``local``, or the other hub fetcher's) or a + fetch of another reference than the entry declares now (its ``source`` changed since the pull), are + refused with the pull to run, never run as a local dataset under the entry's name, since nothing then ties them + to the declared reference (the pull of a package-store entry keeps and records the folders that still match the + store, and re-records the reference when only that changed; the registry fetch says in its own words what to move + aside). When the pull lands elsewhere than the entry's folder (``NEMO_GYM_DATASETS_DIR``, or an entry committed + outside the working root) the copy of the entry's Gym layer beside the tasks is brought up to date, or refused, + exactly as the pull would (:func:`~nemo_gym.tasks.harbor.entries.prepare_destination`), so the two commands never + disagree about one folder. A folder without a pulled task names the pull to run (or, for an entry that declares + no source, says the + task folder path is the target). Tasks are looked for as the loader finds them (:func:`iter_task_folders`: + nested under group folders too), so a layout the folder's path prepares, the entry's name prepares as well. + """ + folder = prepare_destination(entry, create=False) + if folder.is_dir() and iter_task_folders(folder): + if entry.source is not None: + _check_fetch_record(entry, folder) + return folder + if entry.source is None: + raise HarborTaskError( + f"Entry {entry.name} has no task folders in {folder} and declares no [gym] source in " + f"{entry.folder / DATASET_FILE} to pull; pass the folder that holds its tasks instead" + ) + raise HarborTaskError( + f"Entry {entry.name} has no task folders in {folder} yet; run `gym dataset pull {entry.name}` " + f"to fetch {entry.source}" + ) + + +def _check_fetch_record(entry: DatasetEntry, folder: Path) -> None: + """Refuse ``folder``'s task folders as ``entry``'s pull unless its manifest records a fetch of the entry's source. + + The manifest's ``source`` says which fetcher wrote it and its ``ref`` which reference the fetch was made from + (:func:`~nemo_gym.tasks.harbor.hub.declares_recorded_fetch` reads the declaration against it); both are checked + offline, so an entry whose ``source`` moved to another tag since the pull names the pull instead of running the + tasks of the tag it had. A manifest that records no reference (written before it was recorded) names the pull + too, and the pull records it without fetching a folder that still matches. + """ + try: + recorded = Manifest.read(folder) + except ManifestError: + # Malformed, or written by a newer Gym: the manifest check that follows refuses it in its own words. + return + if recorded is None: + raise HarborTaskError( + f"Entry {entry.name}'s folder {folder} holds task folders but no fetch record ({MANIFEST_FILE}), so they " + f"cannot be run as its pull of {entry.source}; run `gym dataset pull {entry.name}` to fetch it" + ) + expected = PACKAGE_STORE_SOURCE if HubRef.parse(entry.source).is_package else REGISTRY_SOURCE + if recorded.source != expected: + raise HarborTaskError( + f"Entry {entry.name}'s folder {folder} holds task folders whose {MANIFEST_FILE} records source " + f"{recorded.source!r}, not a fetch of {entry.source} ({expected}); delete, or move aside, the task folders " + f"and {MANIFEST_FILE} there and run `gym dataset pull {entry.name}`" + ) + if recorded.ref is None or not declares_recorded_fetch(entry.source, recorded.ref): + held = ( + f"a fetch of {recorded.ref}" if recorded.ref is not None else "a fetch whose reference it does not record" + ) + raise HarborTaskError( + f"Entry {entry.name}'s folder {folder} holds task folders whose {MANIFEST_FILE} records {held}, not of " + f"{entry.source}, which {entry.folder / DATASET_FILE} declares now; run `gym dataset pull {entry.name}` to " + f"fetch it (a folder holding another version's tasks is refused there, with what to move aside)" + ) + + +# How check_manifest's refusal of an interrupted fetch ends (nemo_gym.tasks.harbor.manifest). +_FETCH_AGAIN = "run the fetch again" + + +def with_entry_hint(exc: Exception, entry: DatasetEntry | None, folder: Path) -> Exception: + """The fetchers' and the manifest check's refusals name the dataset folder and say to delete it, or move it aside, + and run again. When that folder is a committed entry's own (not the hub's folder, nor the entry's pull elsewhere + than its folder, under a moved ``NEMO_GYM_DATASETS_DIR`` or for an entry committed outside the working root, + which hold nothing git tracks), the same refusal says what to delete there: the pulled task folders and + ``manifest.toml``, not the README, YAML files and ``dataset.toml`` the entry commits, and that another version is + pulled by changing ``source`` in that ``dataset.toml``. The manifest check's refusal + of an interrupted fetch (``run the fetch again``) names the pull to run again instead, wherever the folder is: + the fetch finishes it, and deleting what it landed would throw the partial fetch away. Returns ``exc`` itself for + every other folder, and when the message does not name the folder.""" + if entry is None or str(folder) not in str(exc): + return exc + if str(exc).endswith(_FETCH_AGAIN): + return type(exc)(f"{str(exc).removesuffix(_FETCH_AGAIN)}run `gym dataset pull {entry.name}` again") + if folder != entry.folder: + return exc + return type(exc)( + f"{exc}. {folder} is the committed entry {entry.name}'s own folder: delete, or move aside, only the pulled " + f"task folders and {MANIFEST_FILE} in it; its README, YAML files and {DATASET_FILE} are the entry's and stay " + f"(to pull another version, change `source` in that {DATASET_FILE})" + ) + + +@dataclass(frozen=True) +class PulledDataset: + """What ``gym dataset pull`` fetched: the reference, where it landed and the manifest left there, current.""" + + target: str + plan: PullPlan + folder: Path + manifest: Manifest + # The folder's manifest read the same before the fetch: nothing was downloaded, replaced or rewritten. + unchanged: bool + # The hub's own folder for the reference (``terminal-bench-4.0.0/``), still present beside the entry the + # reference now lands in: a fetch from before the entry declared it, which nothing reads any more. + superseded: Path | None = None + + @property + def entry(self) -> str | None: + return self.plan.entry.name if self.plan.entry is not None else None + + @property + def task_count(self) -> int: + return len(self.manifest.tasks) + + def fetched_line(self) -> str: + where = f" (entry {self.entry})" if self.entry is not None else "" + return f"Fetched {self.plan.ref} into {self.folder}{where}" + + def superseded_line(self) -> str: + return ( + f"Note: {self.superseded} holds an earlier fetch of {self.plan.ref}; fetches of it now land in " + f"{self.folder} and nothing reads the old folder, so it can be moved aside or deleted" + ) + + def text(self) -> str: + """The command's report: where the dataset landed, how many tasks, its digest and the next command.""" + suffix = ", unchanged since the last pull" if self.unchanged else "" + next_target = self.entry if self.entry is not None else shlex.quote(str(self.folder)) + lines = [ + self.fetched_line().replace("Fetched", "Pulled", 1), + f"{self.task_count} task(s), dataset digest {self.manifest.digest} ({self.manifest.source}){suffix}", + ] + if self.superseded is not None: + lines.append(self.superseded_line()) + lines.append(f"Next: gym dataset validate {next_target} --sandbox ") + return "\n".join(lines) + + +def pull_dataset(target: str, *, refresh_registry: bool = False) -> PulledDataset: + """Fetch ``target`` (a ``harbor:`` reference or a committed entry's name) and report where it landed. + + The destination comes from :func:`~nemo_gym.tasks.harbor.entries.plan_pull`: the entry that declares the + reference as its source (its own folder, or ``//`` with its ``dataset.toml`` placed first + when ``NEMO_GYM_DATASETS_DIR`` or an entry committed outside the working root moved the destination, + :attr:`~nemo_gym.tasks.harbor.entries.DatasetEntry.destination`), else the hub's own folder under the datasets + dir. The fetch is the one every + ``gym eval run harbor:...`` makes, so a stale, edited or foreign folder is refused in the same words (a refusal + naming a committed entry's own folder says what to delete there, :func:`with_entry_hint`); nothing is prepared, + built or run. The manifest left after the fetch gives the task count and the dataset digest; its Gym-layer + digests are first brought up to date with the folder (:func:`~nemo_gym.tasks.harbor.manifest.refresh_gym_layer`), + since a fetch that downloads nothing rewrites nothing and would otherwise report the digest of a ``dataset.toml`` + that changed since the last pull, which the next prepare would then rewrite. ``unchanged`` compares the manifest + on disk before anything was touched with the one reported; a registry reference whose version the registry picks + is resolved first (:func:`~nemo_gym.tasks.harbor.hub.resolved_folder`) so that folder is known. When the Gym + layer's digests moved while the task record stayed (the entry's ``dataset.toml`` changed, or one written by hand + into a hub folder), the rewrite is announced in the manifest check's words whichever step applied it: the + refresh above, or the fetch itself, which rewrites the manifest without a word when every folder is in place + (the package store's does). A hub folder the reference landed in before an entry declared it + (``terminal-bench-4.0.0/`` beside ``terminal_bench_4/``) is left alone and named in the report, since nothing + reads it any more. ``refresh_registry`` downloads the registry again instead of reading + ``.harbor/registry.json``; it is :func:`prepare_target`'s pass-through, and deleting that cache file does the + same for the command. + """ + plan = plan_pull(target) + if plan is None: + raise HubError( + f"{target!r} is neither a `harbor:` reference nor a committed entry under environments/harbor/; " + "pass `harbor:/[@ | @sha256:]`, `harbor:[@]` or an entry name" + ) + hub_folder = expected_folder(plan.ref) + if hub_folder is None: + # The registry picks the version: resolve it now, against the cache the fetch reads (refreshed here when + # asked, once), so the folder's manifest can be read before the fetch. + hub_folder = resolved_folder(plan.ref, refresh_registry=refresh_registry) + refresh_registry = False + target_folder = plan.folder if plan.folder is not None else hub_folder + before = _manifest_before(target_folder) + # prepare_destination announces the refresh it makes itself; the fetch is measured from what it found. + folder = prepare_destination(plan.entry) if plan.entry is not None else None + staged = _manifest_before(target_folder) + try: + fetched = fetch_ref(plan.ref, folder=folder, refresh_registry=refresh_registry) + except (HubError, ManifestError) as exc: + raise with_entry_hint(exc, plan.entry, folder) from exc + landed = _manifest_before(fetched) + manifest = refresh_gym_layer(fetched) + if manifest is None: + raise HubError(f"{fetched} has no {MANIFEST_FILE} after the fetch; run the pull again") + if ( + staged is not None + and landed is not None + and landed.text() == manifest.text() + and staged.digest != landed.digest + and (staged.source, staged.tasks) == (landed.source, landed.tasks) + ): + # The fetch rewrote the manifest with the same task record and the digest moved: only the Gym layer changed, + # and the refresh above found nothing left to do, so it is announced here (a rewrite that only re-recorded + # the reference moves no digest and announces nothing: the pull's report names the reference). + report_rewrite(fetched, staged, landed, gym_layer_change(staged, landed)) + superseded = hub_folder if plan.entry is not None and hub_folder.is_dir() else None + return PulledDataset( + target=target, + plan=plan, + folder=fetched, + manifest=manifest, + unchanged=before is not None and before.text() == manifest.text(), + superseded=superseded if superseded != fetched else None, + ) + + +def _manifest_before(folder: Path | None) -> Manifest | None: + """The manifest in ``folder`` as it stands, or ``None``; the fetch itself reports an unreadable one.""" + if folder is None or not folder.is_dir(): + return None + try: + return Manifest.read(folder) + except ManifestError: + return None + + +def pull_target(args: argparse.Namespace, overrides: list[str]) -> None: + """Entry point for ``gym dataset pull ``.""" + if overrides: + args._parser.error(f"`gym dataset pull` takes no configuration overrides: {' '.join(overrides)}") + try: + pulled = pull_dataset(args.target) + except (HubError, ManifestError, HarborTaskError, ValueError, OSError) as exc: + args._parser.error(str(exc)) + print(pulled.text()) + + def _dataset_label(folder: Path, dataset_root: Path) -> str: """The dataset label the rows of ``folder`` carry as ``task_source``. diff --git a/nemo_gym/tasks/harbor/dataset_config.py b/nemo_gym/tasks/harbor/dataset_config.py index 724b7d5292c..c86fd15213f 100644 --- a/nemo_gym/tasks/harbor/dataset_config.py +++ b/nemo_gym/tasks/harbor/dataset_config.py @@ -6,7 +6,8 @@ ``task.toml`` is the task author's file and, once fetched, read-only. What a dataset needs on top of it goes here, as data: the facts Harbor has no field for (``license`` and ``type``, which say which split the rows may enter; ``allowed_agents``, the harnesses admitted to run it; ``domain`` and -``verified``, catalog metadata), defaults that scale every task or set how many rollouts an +``verified``, catalog metadata; ``source``, the hub reference a committed entry under ``environments/harbor/`` +is pulled from), defaults that scale every task or set how many rollouts an evaluation takes per task, and per-task overrides expressed with Harbor's own ``[environment]`` fields. The loader applies the task-shaping settings before anything else sees a task, so servers, agents and rows only ever see the effective ``task.toml``; the other keys flow into the generated @@ -87,6 +88,11 @@ class GymDatasetConfig(BaseModel): # (one of Gym's domain values) and whether its rewards were baselined and reviewed. domain: Domain | None = None verified: bool = Field(default=False, strict=True) + # Where a committed entry's tasks come from: a `harbor:/@` or + # `harbor:@` hub reference (:mod:`nemo_gym.tasks.harbor.hub`). `gym dataset pull ` + # fetches it into the entry's folder, and a pull of the reference itself lands there too, so one dataset never + # lives in two sibling folders. Unset for a dataset whose task folders are its own (the usual authored dataset). + source: str | None = None defaults: GymDatasetDefaults = Field(default_factory=GymDatasetDefaults) tasks: dict[str, GymTaskOverride] = Field(default_factory=dict) # The dataset.toml this table was read from, so that errors in applying it name the file. @@ -107,6 +113,20 @@ def _allowed_agents_name_distinct_agents(cls, value: list[str] | None) -> list[s raise ValueError(f"allowed_agents lists {', '.join(duplicates)} more than once") return value + @field_validator("source") + @classmethod + def _source_is_a_hub_reference(cls, value: str | None) -> str | None: + """Refuse a source that is not a well-formed ``harbor:`` reference, in the hub's own words.""" + if value is None: + return value + from nemo_gym.tasks.harbor.hub import HubError, HubRef # hub imports the manifest, which imports this module + + try: + HubRef.parse(value) + except HubError as exc: + raise ValueError(f"source: {exc}") from exc + return value + @field_validator("domain", mode="before") @classmethod def _domain_is_one_of_gyms(cls, value: Any) -> Any: @@ -155,6 +175,7 @@ def is_empty(self) -> bool: and self.allowed_agents is None and self.domain is None and not self.verified + and self.source is None ) @property diff --git a/nemo_gym/tasks/harbor/entries.py b/nemo_gym/tasks/harbor/entries.py new file mode 100644 index 00000000000..09d898677cd --- /dev/null +++ b/nemo_gym/tasks/harbor/entries.py @@ -0,0 +1,473 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Committed dataset entries under ``environments/harbor//`` and where a pull lands. + +An entry is the folder a Gym checkout commits for one dataset: its README, deployment YAML overlays and a +``dataset.toml`` whose ``[gym]`` table may declare ``source``, the hub reference the task folders come from +(``harbor:/@`` for the package store, ``harbor:@`` for the +registry). ``gym dataset pull `` fetches that reference into the entry's own folder, where the pulled task +folders and ``manifest.toml`` are gitignored next to the committed files, and a pull of the reference itself +lands there too, so one dataset never lives in two sibling folders. A reference no entry declares lands in +``/-/`` as before. ``gym dataset validate `` and ``gym eval run `` +resolve the name to that folder without fetching (:func:`entry_target`, where an existing path of that name keeps +meaning the path, since those commands take paths; the pull takes none, so a bare name is always the entry there). + +An entry is pulled into its own folder when that folder is under the working root (:data:`nemo_gym.WORKING_DIR`: +the Gym checkout of an editable install, the working directory of a package install) and is not installed. An +installed entry, one a package install ships under ``site-packages`` or another Python installation prefix +(:func:`_installed_entry`; shared, often read-only, and under the working root too when the project keeps its venv +there), and an entry committed outside the working root (one a ``NEMO_GYM_EXTRA_ROOTS`` root commits) are pulled +into ``//`` (:func:`~nemo_gym.tasks.harbor.hub.datasets_dir`, the writable default every other +pull uses), and ``NEMO_GYM_DATASETS_DIR`` moves every pulled dataset there (an entry without a source is never +pulled, so it stays at its folder). An entry's tasks then live in ``//`` with a copy of the +entry's Gym layer beside them, its ``dataset.toml`` and whatever dataset-level ``task_data.py`` or ``verifier.py`` it +commits (:func:`gym_layer`), kept current with the entry's files (:func:`prepare_destination`), so the files that +shape the tasks, and the dataset digest that covers them, are the same wherever the pull landed. That copy folder, +told by the :data:`LAYER_RECORD_FILE` the placing writes beside the copies, is never itself an entry: lookup by name +and by reference (:func:`find_entry`, :func:`committed_entries`) skips it and goes on to the root that commits the +entry, so the pull keeps following the entry's files, while a folder of the entry's name a user committed in an +earlier root (no record file) keeps precedence, as any shadowing component does. +""" + +import hashlib +import json +import os +import re +import shutil +import site +import sysconfig +from dataclasses import dataclass +from pathlib import Path + +from nemo_gym import WORKING_DIR, component_search_roots +from nemo_gym.tasks.harbor.dataset_config import DATASET_FILE, read_dataset_config +from nemo_gym.tasks.harbor.hub import ( + DATASETS_DIR_ENV, + REGISTRY_SOURCE, + HubError, + canonical_reference, + datasets_dir, + is_hub_ref, +) +from nemo_gym.tasks.harbor.manifest import DATASET_FILES, MANIFEST_FILE, Manifest, ManifestError, refresh_gym_layer +from nemo_gym.tasks.harbor.package_store import PACKAGE_STORE_SOURCE + + +ENTRIES_SUBDIR = Path("environments") / "harbor" +# An entry is named by its folder, one path component; a hub folder name (`terminal-bench-4.0.0`) fits too, which +# is fine: such a folder commits no dataset.toml with a source and is never an entry a pull resolves. +_ENTRY_NAME = re.compile(r"[A-Za-z0-9][A-Za-z0-9._-]*") +# The files of an entry's Gym layer beside the task folders: what enters the +# dataset digest besides the tasks, and so what a destination elsewhere needs a copy of. +LAYER_FILES = (DATASET_FILE, *DATASET_FILES) +# The record of the copies :func:`prepare_destination` placed at a destination elsewhere: the digest of each copy as +# placed, in a hidden file beside them (hidden, so task discovery never looks at it) that nothing else writes. The +# folder's manifest.toml is no such record: every command that prepares the folder as a plain path rewrites its +# Gym-layer digests over whatever the copies hold. +LAYER_RECORD_FILE = ".gym-layer.json" +# The manifests a hub fetch writes; any other one beside differing copies says the folder is not the entry's pull. +_FETCH_SOURCES = (PACKAGE_STORE_SOURCE, REGISTRY_SOURCE) + + +@dataclass(frozen=True) +class DatasetEntry: + """A committed entry: its name (the folder's), its folder and the ``[gym] source`` it declares, if any.""" + + name: str + folder: Path + source: str | None + + @property + def destination(self) -> Path: + """Where the entry's tasks are pulled to and read from: the entry's own folder when it is under the working + root (:data:`nemo_gym.WORKING_DIR`) and not installed, else ``//`` + (:func:`nemo_gym.tasks.harbor.hub.datasets_dir`). An installed entry, one a package install ships under + ``site-packages`` or another installation prefix (:func:`_installed_entry`), falls back to the datasets dir + wherever the working root is, a project-local ``.venv`` under it included, since the installation may be + shared or read-only and is in any case not where the user's pulls go; so does an entry committed outside the + working root (one a ``NEMO_GYM_EXTRA_ROOTS`` root commits), and ``NEMO_GYM_DATASETS_DIR`` moves every pull + there, the working root's entries included. The name is taken + by the first root that commits it (:func:`find_entry` order), so the fallback folder is another committed + entry's own folder (the shadowed one's) only when the same name is committed twice, and that case is the + user's to resolve. An entry without a source has no pull to move: its tasks are wherever its folder holds + them (a checkout the user placed there), so it stays at its folder and resolves by name as it does by path, + wherever it is and whether or not the variable is set.""" + if self.source is None: + return self.folder + if os.environ.get(DATASETS_DIR_ENV) or _installed_entry(self.folder) or not _under_working_root(self.folder): + return datasets_dir() / self.name + return self.folder + + +def _under_working_root(folder: Path) -> bool: + """Whether ``folder`` lies under :data:`nemo_gym.WORKING_DIR`, the root whose ``environments/harbor/`` is the + default datasets dir (the checkout of an editable install, the working directory of a package install). A + project-local ``.venv`` lies under it too, which is why :func:`_installed_entry` is asked first.""" + return _under(folder, Path(WORKING_DIR)) + + +def _installed_entry(folder: Path) -> bool: + """Whether ``folder`` is installed: it lies under a Python installation prefix, so it is a package's, shared and + often read-only, and never where a user's pulls go, whatever the working directory. Installed is a path with a + ``site-packages`` or ``dist-packages`` component (as given or resolved), or one under the running interpreter's + ``purelib``/``platlib`` (:func:`sysconfig.get_paths`), its site-packages folders (:func:`site.getsitepackages`) or + its user site-packages (:func:`site.getusersitepackages`).""" + given = Path(folder) + resolved = given.resolve() + if any(part in ("site-packages", "dist-packages") for path in (given, resolved) for part in path.parts): + return True + return any(_under(resolved, prefix) for prefix in _install_prefixes()) + + +def _install_prefixes() -> list[Path]: + """The running interpreter's installation prefixes: ``purelib`` and ``platlib``, its site-packages folders and + the user site-packages (a venv's ``site`` module may lack ``getsitepackages``: then the first two stand).""" + paths = sysconfig.get_paths() + candidates = [paths.get("purelib"), paths.get("platlib")] + getsitepackages = getattr(site, "getsitepackages", None) + if getsitepackages is not None: + candidates.extend(getsitepackages()) + candidates.append(site.getusersitepackages()) + return [Path(candidate) for candidate in candidates if candidate] + + +def _under(folder: Path, root: Path) -> bool: + """Whether ``folder`` resolves to ``root`` or a path under it (``root`` resolved too).""" + try: + Path(folder).resolve().relative_to(Path(root).resolve()) + except ValueError: + return False + return True + + +def _pull_copy(folder: Path) -> bool: + """Whether ``folder`` is the copy a pull placed at ``//`` for an entry elsewhere, told by + the :data:`LAYER_RECORD_FILE` only :func:`prepare_destination` writes, beside the copies of the entry's Gym layer. + Such a folder is never itself an entry: the entry is in a later root, and lookup goes on to it.""" + return (Path(folder) / LAYER_RECORD_FILE).is_file() + + +@dataclass(frozen=True) +class PullPlan: + """What a pull of a target fetches and where: the hub reference, and the entry it lands in, if one.""" + + ref: str + entry: DatasetEntry | None + + @property + def folder(self) -> Path | None: + """The dataset folder to fetch into, or ``None`` for the hub's own folder name under the datasets dir.""" + return self.entry.destination if self.entry is not None else None + + +def _roots() -> list[Path]: + return component_search_roots() + + +def _entry(folder: Path) -> DatasetEntry: + """The entry at ``folder``; a ``dataset.toml`` that does not read raises ``ValueError`` with the file named.""" + return DatasetEntry(name=folder.name, folder=folder.resolve(), source=read_dataset_config(folder).source) + + +def _committed_folder(name: str) -> Path | None: + """The folder ``environments/harbor/`` of the first search root that has one, or ``None``. + + The copy a pull placed there for an entry committed in a later root (:func:`_pull_copy`: the working root's + ``//`` of an installed entry, after its first pull) is not that folder: it is skipped and the + search goes on, so the name keeps resolving to the entry whose files the copy follows. + """ + if not _ENTRY_NAME.fullmatch(name): + return None + for root in _roots(): + folder = root / ENTRIES_SUBDIR / name + if folder.is_dir() and not _pull_copy(folder): + return folder + return None + + +def find_entry(name: str) -> DatasetEntry | None: + """The entry called ``name`` in the first search root that commits one, or ``None``. + + An entry need not declare a source: one whose tasks come from elsewhere (a checkout the user points + ``gym eval run`` at) still resolves by name to its folder. A ``dataset.toml`` that does not read raises + ``ValueError`` naming the file, as prepare would for the folder. + """ + folder = _committed_folder(name) + return _entry(folder) if folder is not None else None + + +def committed_entries() -> list[DatasetEntry]: + """Every entry that declares a source, across the search roots; an earlier root wins a name. + + A name is taken by the first root that has a folder of it, as :func:`find_entry` resolves it, whether or + not that folder declares a source: a source-less ``tb4`` in an earlier root shadows a sourced one in a later + root for the name, so it shadows it for the reference too, and the pull of that reference lands in the hub's + own folder rather than in an entry the name no longer reaches. The copy a pull placed for an entry of a later + root (:func:`_pull_copy`) takes no name: it is skipped as :func:`find_entry` skips it, so the installed entry it + follows keeps the name and the reference. Only folders with a ``dataset.toml`` are read + (a pulled hub folder has none unless its user wrote one) and one whose table does not read is skipped here: + the mapping must not fail every pull over a stray file, and the entry's own pull reports it. + """ + found: dict[str, DatasetEntry] = {} + taken: set[str] = set() + for root in _roots(): + parent = root / ENTRIES_SUBDIR + if not parent.is_dir(): + continue + for folder in sorted(parent.iterdir()): + if folder.name in taken or folder.name.startswith(".") or not folder.is_dir() or _pull_copy(folder): + continue + taken.add(folder.name) + if not (folder / DATASET_FILE).is_file(): + continue + try: + entry = _entry(folder) + except ValueError: + continue + if entry.source is not None: + found[folder.name] = entry + return list(found.values()) + + +def entry_for_ref(target: str) -> DatasetEntry | None: + """The entry whose source names the same dataset and version as the ``harbor:`` reference ``target``, or ``None``. + + The two are compared as the fetchers record them (:func:`~nemo_gym.tasks.harbor.hub.canonical_reference`), so a + digest with or without its ``sha256:`` prefix, or ``harbor:org/name`` with or without ``@latest``, finds the entry + that declares the other spelling instead of landing beside it in the hub's own folder. ``harbor:org/name`` (the + store's ``latest``) and ``harbor:org/name@4.0.0`` are different references, so a plain reference never lands in + an entry pinned to a version. Two entries declaring one source is a mistake in the checkout and is refused with + both named. + """ + ref = canonical_reference(target) + matches = [entry for entry in committed_entries() if canonical_reference(entry.source) == ref] + if len(matches) > 1: + names = ", ".join(entry.name for entry in matches) + raise HubError( + f"{target} is the source of {len(matches)} entries under environments/harbor/ ({names}); keep one" + ) + return matches[0] if matches else None + + +def entry_target(target: str) -> DatasetEntry | None: + """The entry ``target`` names on a command line, or ``None`` when it is a path or a hub reference. + + A bare name (one path component, no ``./`` or ``/``) that is not an existing path and that a search root + commits under ``environments/harbor/`` is the entry. An existing folder of that name, or an explicit path, + keeps meaning the path, as it does for Hydra overrides. + """ + if is_hub_ref(target) or target in (".", "..") or Path(target).name != target: + return None + if Path(target).exists(): + return None + return find_entry(target) + + +def entry_at(folder: Path) -> DatasetEntry | None: + """The committed entry whose pull ``folder`` holds when that is not the entry's own folder: + ``//`` for the entry called ``name`` under a moved ``NEMO_GYM_DATASETS_DIR``, for an installed + entry (one a package install ships) or for an entry committed outside the working root, or ``None`` for every + other folder (an entry's own folder included: nothing is copied there). The folder is looked up as the name is + (:func:`_committed_folder`), so the copy itself, once placed, is passed over for the entry it follows and the path + form agrees with the name form. A path target naming such a folder is prepared with the entry's + Gym layer kept current beside the tasks, or refused, exactly as the pull and the entry's name are + (:func:`prepare_destination`), so the three commands agree on the folder and an edited copy is refused in the + same words by each. An unreadable ``dataset.toml`` of the entry raises ``ValueError`` naming the file, as + :func:`find_entry` does: it is the table the folder's copy follows. The folder's own table is never read here: + the folder is compared with the first root's ``environments/harbor/`` before any entry is read, so a plain + folder under the datasets dir (every pulled hub folder of an editable checkout, whose datasets dir is that root's + ``environments/harbor/``) is nobody's pull elsewhere, and a ``dataset.toml`` written into it that does not read is + reported by the prepare that follows, as it was before. + """ + folder = Path(folder).resolve() + if folder.parent != datasets_dir(): + return None + committed = _committed_folder(folder.name) + if committed is None or committed.resolve() == folder: + return None + entry = _entry(committed) + if entry.destination != folder: + return None + return entry + + +def plan_pull(target: str) -> PullPlan | None: + """How ``gym dataset pull TARGET`` lands, or ``None`` when ``target`` is neither a hub reference nor an entry. + + A hub reference is fetched into the entry that declares it as its source, else into the hub's own folder. + An entry name is fetched from the source it declares, into its folder; an entry without a source has + nothing to pull and says so. The pull takes no path, so a bare name is looked up as an entry outright: a + folder of the same name in the working directory does not hide it, as it would from validate and run + (:func:`entry_target`). + """ + if is_hub_ref(target): + return PullPlan(ref=target, entry=entry_for_ref(target)) + entry = find_entry(target) + if entry is None: + return None + if entry.source is None: + raise HubError( + f"{entry.folder / DATASET_FILE} declares no [gym] source, so there is nothing to pull for entry " + f'{entry.name}; add source = "harbor:/@" there, or pull a harbor: reference' + ) + return PullPlan(ref=entry.source, entry=entry) + + +def gym_layer(folder: Path) -> dict[Path, bytes]: + """The Gym-layer files under ``folder`` by relative path, with their content: ``dataset.toml``, a dataset-level + ``task_data.py`` or ``verifier.py`` (:data:`LAYER_FILES`), each when present. + These are the files the dataset digest covers besides the task folders, so they are what an entry's destination + elsewhere must hold a copy of for a run there to be the entry's.""" + folder = Path(folder) + paths = [Path(name) for name in LAYER_FILES if (folder / name).is_file()] + return {path: (folder / path).read_bytes() for path in paths} + + +def prepare_destination(entry: DatasetEntry, *, create: bool = True) -> Path: + """``entry``'s destination with the entry's Gym layer current there, ready for a fetch or a prepare. + + The entry's own folder needs nothing. A destination elsewhere (``NEMO_GYM_DATASETS_DIR``, an installed entry or + one committed outside the working root, :attr:`DatasetEntry.destination`) holds a copy of the entry's Gym layer + (:func:`gym_layer`: ``dataset.toml``, a dataset-level ``task_data.py`` or ``verifier.py``), so the files that + shape the tasks and the digest that covers them are the entry's wherever the pull landed: the copies are placed + before the first task lands (``create`` makes the folder; a prepare, which fetches nothing, passes ``False`` and + leaves a missing folder missing) and are replaced, a file the entry no longer commits removed, when the entry's + files changed since, as long as the copies are still the ones on record: + the ones this function placed, whose digests it writes to :data:`LAYER_RECORD_FILE` beside them as it places them + (:func:`_record_copies`; the record is brought up to date whenever the copies are found to be the entry's files). + Only this function writes that record, so no other command can vouch for an edited copy: the manifest's own + Gym-layer digests are rewritten by a prepare of the folder by its path in a shell without + ``NEMO_GYM_DATASETS_DIR``, which cannot know the folder is an entry's pull, and were once what the copies were + checked against, so such a prepare absorbed an edit of the copy and the next pull overwrote it. + Whenever the copies are replaced the manifest is brought up to date with them + (:func:`~nemo_gym.tasks.harbor.manifest.refresh_gym_layer`), so its digests follow the copies even when nothing + rewrites the manifest afterwards (a registry fetch with every folder in place, a fetch that fails, a prepare + refused before its manifest check). A copy that matches neither the entry's file nor the record was edited in + place and is refused, never overwritten, whatever the folder's manifest says: the entry's files are where the + layer is edited. ``gym dataset pull ``, ``gym dataset validate `` and ``gym eval run `` all + come through here, so they agree on the folder. + """ + destination = entry.destination + if destination == entry.folder: + return destination + layer = gym_layer(entry.folder) + if not layer or (not create and not destination.is_dir()): + return destination + destination.mkdir(parents=True, exist_ok=True) + copies = gym_layer(destination) + if copies == layer: + _record_copies(destination, layer) + return destination + if copies: + _refuse_copies_edited_in_place(entry, destination, layer, copies) + for path in sorted(set(copies) - set(layer)): + _remove_copy(destination, path) + for path, content in layer.items(): + if copies.get(path) != content: + (destination / path).parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(entry.folder / path, destination / path) + _record_copies(destination, layer) + try: + refresh_gym_layer(destination) + except ManifestError: + # A manifest this Gym cannot read records nothing to keep current; the fetch or the prepare that follows + # reports it, or rewrites it, in its own words. + pass + return destination + + +def _remove_copy(destination: Path, path: Path) -> None: + """Remove the copy of a file the entry no longer commits, and the folders left empty by it.""" + (destination / path).unlink() + for parent in (destination / path).parents: + if parent == destination: + break + try: + parent.rmdir() + except OSError: + break + + +def _layer_digests(files: dict[Path, bytes]) -> dict[str, str]: + """The digest of each Gym-layer file by its relative path, as the record of the copies holds them.""" + return {path.as_posix(): hashlib.sha256(content).hexdigest() for path, content in files.items()} + + +def _record_copies(destination: Path, layer: dict[Path, bytes]) -> None: + """Record the copies of ``layer`` now at ``destination`` in :data:`LAYER_RECORD_FILE`, when the record differs. + + Written whole and renamed into place, so an interrupted write leaves the previous record, not a torn one. + """ + text = json.dumps(_layer_digests(layer), indent=2, sort_keys=True) + "\n" + record = destination / LAYER_RECORD_FILE + if _recorded_copies(destination) == _layer_digests(layer): + return + staged = record.with_name(record.name + ".tmp") + staged.write_text(text, encoding="utf-8") + staged.replace(record) + + +def _recorded_copies(destination: Path) -> dict[str, str] | None: + """The digests :data:`LAYER_RECORD_FILE` at ``destination`` holds, or ``None`` without a readable record.""" + record = destination / LAYER_RECORD_FILE + if not record.is_file(): + return None + try: + data = json.loads(record.read_text(encoding="utf-8")) + except (OSError, UnicodeDecodeError, ValueError): + return None + if not isinstance(data, dict): + return None + return {str(path): digest for path, digest in data.items() if isinstance(digest, str)} + + +def _refuse_copies_edited_in_place( + entry: DatasetEntry, destination: Path, layer: dict[Path, bytes], copies: dict[Path, bytes] +) -> None: + """Refuse copies of the entry's Gym layer that are neither the entry's files nor the ones this module placed. + + The copies are at a destination elsewhere than the entry's folder (``NEMO_GYM_DATASETS_DIR``, or an entry + committed outside the working root, :attr:`DatasetEntry.destination`). The record of the placed copies is + :data:`LAYER_RECORD_FILE` beside them (:func:`_record_copies`), which only :func:`prepare_destination` writes: + copies whose digests it holds, every one of them, were placed there and may be replaced. The folder's + ``manifest.toml`` is no such record, not even one a hub fetch wrote: a prepare of the folder by its path in a + shell without ``NEMO_GYM_DATASETS_DIR`` rewrites its Gym-layer digests over an edited copy, as it does for any + dataset whose own ``dataset.toml`` changed, so a copy checked against those digests read as the one on record and + the next pull overwrote the edit. A ``local`` manifest beside the differing copies is named as such: it is + prepare's record of a dataset the user put at ``//`` by hand and ran by its path, before + ``NEMO_GYM_DATASETS_DIR`` or an entry committed outside the working root moved the entry's pull there, or in a + shell without the variable, so the folder is not the entry's pull at all; the refusal says which case it is and + what to do (move the folder aside; with the variable, running it by its path without it is the other way), and + either way nothing is overwritten. + """ + recorded = _recorded_copies(destination) + if recorded is not None and all(recorded.get(path) == digest for path, digest in _layer_digests(copies).items()): + return + differing = sorted(path for path in set(layer) | set(copies) if layer.get(path) != copies.get(path)) + names = ", ".join(str(destination / path) for path in differing) + manifest = Manifest.read(destination) + if manifest is not None and manifest.source not in _FETCH_SOURCES: + if os.environ.get(DATASETS_DIR_ENV): + where = f"under {DATASETS_DIR_ENV}" + aside = f"move the folder aside, or run it by its path with {DATASETS_DIR_ENV} unset" + elif _installed_entry(entry.folder): + where = f"(the entry is installed, under {entry.folder.parent})" + aside = "move the folder aside" + else: + where = f"(the entry is committed outside the working root, under {entry.folder.parent})" + aside = "move the folder aside" + raise HubError( + f"{destination} is where entry {entry.name}'s pull lands {where}, but its {MANIFEST_FILE} " + f"records a {manifest.source} dataset, not a fetch, and {names} differ{'s' if len(differing) == 1 else ''} " + f"from the entry's files under {entry.folder}; it is not the entry's pull: {aside}" + ) + if len(differing) == 1: + raise HubError( + f"{destination / differing[0]} was edited in place and differs from the entry's " + f"{entry.folder / differing[0]}; edit the entry's file instead, or move the copy aside and run again" + ) + raise HubError( + f"{names} were edited in place and differ from the entry's files under {entry.folder}; edit the entry's files " + "instead, or move the copies aside and run again" + ) diff --git a/nemo_gym/tasks/harbor/hub.py b/nemo_gym/tasks/harbor/hub.py index b0cd71e0255..7515733b2c8 100644 --- a/nemo_gym/tasks/harbor/hub.py +++ b/nemo_gym/tasks/harbor/hub.py @@ -72,7 +72,11 @@ def datasets_dir() -> Path: a dataset nor the registry cache. Each fetched dataset is one folder under it (``environments/harbor//``), gitignored there - next to the committed per-dataset entries, which keep only their README and YAML files. + next to the committed per-dataset entries, which keep only their README, YAML files and ``dataset.toml``. + An entry that declares a ``[gym] source`` is pulled into its own folder when that folder is under the working + root and not installed, else (an entry a package install ships under ``site-packages``, a project-local ``.venv`` + included, or one an extra search root commits, or any entry when ``NEMO_GYM_DATASETS_DIR`` is set) into + ``//`` (:mod:`nemo_gym.tasks.harbor.entries`). """ return Path(os.environ.get(DATASETS_DIR_ENV) or WORKING_DIR / "environments" / "harbor").expanduser().resolve() @@ -122,6 +126,12 @@ def folder_name(self) -> str: return self.name return f"{self.name}-{_UNSAFE.sub('-', self.version)}" + @property + def reference(self) -> str: + """The reference a fetch of this dataset records in its manifest: ``harbor:@``, the version + resolved, so a fetch of ``harbor:`` records which version the registry picked.""" + return f"{REF_PREFIX}{self.name}@{self.version}" if self.version else f"{REF_PREFIX}{self.name}" + def _numeric_version(version: str) -> tuple[int, ...] | None: parts = version.split(".") @@ -367,8 +377,10 @@ def _read_manifest_pins(manifest: Manifest | None) -> dict[str, tuple[str, str, return pins -def _write_manifest(folder: Path, pins: dict[str, tuple[str, str, str]], digests: dict[str, str]) -> None: - """Publish the pins with the digests of their folders; only pinned tasks are recorded. +def _write_manifest( + folder: Path, pins: dict[str, tuple[str, str, str]], digests: dict[str, str], *, reference: str +) -> None: + """Publish the pins with the digests of their folders, and the ``reference`` fetched; only pinned tasks are recorded. ``digests`` caches the hashing across the per-task checkpoints, and naming the pinned tasks keeps each checkpoint from scanning the dataset folder again: a folder the fetch did not pin stays unrecorded, @@ -382,6 +394,7 @@ def _write_manifest(folder: Path, pins: dict[str, tuple[str, str, str]], digests }, digests=digests, names=pins, + ref=reference, ).write(folder) @@ -399,6 +412,53 @@ def refuse_unrecorded_folders(folder: Path, known: set[str], *, what: str, error ) +def _refuse_dropped_folders( + folder: Path, previous: dict[str, tuple[str, str, str]], names: set[str], dataset: RegistryDataset +) -> None: + """Refuse the task folders a previous fetch pinned for tasks that ``dataset`` no longer lists. + + The manifest a fetch writes pins the dataset's tasks alone, so such a folder would stay on disk unrecorded and + the next prepare would refuse the whole folder as edited by hand, after a pull that reported success. The folder + holds another version's task (a committed entry's ``source`` moved to a version with fewer tasks, the registry + revised this one, or ``harbor:`` now resolves to such a version), so it is refused here, named, with + what to move aside, before anything is cloned or rewritten: the registry's counterpart of the store's refusal of + a manifest written for another version (``tasks removed``, :mod:`nemo_gym.tasks.harbor.package_store`). + """ + dropped = sorted(name for name in previous if name not in names and (folder / name / "task.toml").is_file()) + if dropped: + raise HubError( + f"Task folder(s) {', '.join(dropped)} in {folder} were fetched for another version of {dataset.name}: a " + f"previous fetch pinned them and {dataset.reference} does not list them; move them aside and run again " + f"to fetch {dataset.reference}" + ) + + +def _refuse_repinned_folders(folder: Path, repinned: dict[str, tuple[str, str]], dataset: RegistryDataset) -> None: + """Refuse the task folders a previous fetch pinned at a commit other than the one ``dataset`` pins them at. + + A recorded pin is kept when the registry still names the same repository and path, so a version that pins a + task at another commit (a committed entry's ``source`` moved to it, ``harbor:`` now resolves to it, or + the registry revised this version in place) used to keep the folder of the commit fetched before and record it + under the new reference: the other version's content, run by name as this one's. Harbor caches a git task by + ``(git_url, git_commit_id, path)`` and downloads a task pinned at another commit afresh + (``harbor.models.task.id.GitTaskId``), so the folder is refused here, named with both commits, before anything + is cloned or rewritten; moved aside, the fetch clones the commit the registry pins. A registry pin that is not + a commit (``HEAD``, a branch) keeps the recorded one while the reference stays the same: the fetch pinned what + it pointed at then. A fetch of another reference into the folder (the version changed) resolves such a pin + afresh, as the first fetch of that version into a folder of its own did before entries shared one, and Harbor + never serves a task without a resolved commit from its cache (``harbor.tasks.client``); a commit that moved + since is refused here the same way, named with both commits. + """ + if repinned: + was = ", ".join(f"{name} at {old[:12]}" for name, (old, _) in sorted(repinned.items())) + now = ", ".join(f"{name} at {new[:12]}" for name, (_, new) in sorted(repinned.items())) + raise HubError( + f"Task folder(s) {', '.join(sorted(repinned))} in {folder} were fetched for another version of " + f"{dataset.name}: a previous fetch pinned {was} and {dataset.reference} pins {now}; move them aside and " + f"run again to fetch {dataset.reference}" + ) + + def _check_kept_folders(folder: Path, kept: list[str], previous: Manifest | None, digests: dict[str, str]) -> None: """Hash the task folders a fetch keeps and refuse one whose content differs from the manifest. @@ -418,7 +478,7 @@ def _check_kept_folders(folder: Path, kept: list[str], previous: Manifest | None ) -def fetch_dataset(dataset: RegistryDataset, root: Path) -> Path: +def fetch_dataset(dataset: RegistryDataset, root: Path, *, folder: Path | None = None) -> Path: """Fetch tasks, retaining the recorded source pins of existing folders. Publish a pin after each task, so a later failure leaves completed tasks reusable. @@ -426,6 +486,15 @@ def fetch_dataset(dataset: RegistryDataset, root: Path) -> Path: is refused; the user can move it aside and retry without losing local edits. The manifest records each folder's digest beside its pin; when a fetch adds tasks to an existing folder, the kept folders must still match their recorded digests. + + The tasks land in ``root/-/`` (:attr:`RegistryDataset.folder_name`) unless ``folder`` + names the dataset folder itself: a committed entry under ``environments/harbor/`` whose ``dataset.toml`` + declares this dataset as its ``[gym] source`` (:mod:`nemo_gym.tasks.harbor.entries`). The folder's other + files (the entry's README, YAML overlays and ``dataset.toml``) are not task folders and stay as they are. + A folder holding a task of another version (the entry's source moved to a version that no longer lists it, or + that pins it at another commit, a ``HEAD`` pin that moved since included) is refused with the folder named, as + the store's fetch refuses a manifest written for another version (:func:`_refuse_dropped_folders`, + :func:`_refuse_repinned_folders`), never left unrecorded beside the fetched tasks or recorded as this version's. """ if not _NAME.fullmatch(dataset.name): raise HubError(f"Unsafe Harbor dataset name {dataset.name!r}") @@ -436,7 +505,7 @@ def fetch_dataset(dataset: RegistryDataset, root: Path) -> Path: if task.name in names: raise HubError(f"Duplicate Harbor task name {task.name!r}") names.add(task.name) - folder = Path(root).resolve() / dataset.folder_name + folder = Path(folder).resolve() if folder is not None else Path(root).resolve() / dataset.folder_name if folder.is_symlink(): raise HubError(f"Dataset folder must not be a symlink: {folder}") folder.mkdir(parents=True, exist_ok=True) @@ -444,10 +513,15 @@ def fetch_dataset(dataset: RegistryDataset, root: Path) -> Path: raise HubError(f"Dataset manifest must not be a symlink: {folder / MANIFEST_FILE}") manifest = _read_manifest(folder) previous = _read_manifest_pins(manifest) + # The record names another reference than this fetch's (another version of the dataset: a committed entry's + # source moved, or the registry's pick did), so the folder holds that version's fetch; one that records no + # reference (written before it was recorded) is this fetch's as far as it says. + moved = manifest is not None and manifest.ref is not None and manifest.ref != dataset.reference resolved: dict[tuple[str, str], str] = {} pins: dict[str, tuple[str, str, str]] = {} digests: dict[str, str] = {} kept: list[str] = [] + repinned: dict[str, tuple[str, str]] = {} by_repo: dict[tuple[str, str], list[RegistryTask]] = {} for task in dataset.tasks: destination = folder / task.name @@ -458,6 +532,21 @@ def fetch_dataset(dataset: RegistryDataset, root: Path) -> Path: raise HubError( f"Recorded source pin for {destination} is invalid or differs from the registry; move the folder aside and retry" ) + pinned = task.git_commit_id if _SHA.fullmatch(task.git_commit_id) else None + if pin is not None and pinned is None and moved: + # A pin that is not a commit (`HEAD`, a branch) stays as the recorded fetch resolved it while the + # reference stays the same, but this fetch is another version's, into the folder the previous version's + # fetch filled: its pin is resolved afresh, as a fetch into a folder of this version's own resolved it, + # and a commit that moved since is handled below like one the registry changed. + pinned = resolve_commit(task.git_url, task.git_commit_id, resolved) + if pin is not None and pinned is not None and pinned != pin[1]: + # The registry pins this task at another commit than the recorded fetch. An existing folder holds that + # other commit's content and is refused below (_refuse_repinned_folders); a missing one is no repair of + # the recorded bytes but a fetch of the commit the registry pins. + if destination.exists(): + repinned[task.name] = (pin[1], pinned) + continue + pin = None if pin is not None: # A missing folder is a repair of these pinned bytes. Keep its validated pin in every # publication, even before copying it, so an interrupted repair cannot follow new HEAD. @@ -473,10 +562,17 @@ def fetch_dataset(dataset: RegistryDataset, root: Path) -> Path: commit = pin[1] if pin else resolve_commit(task.git_url, task.git_commit_id, resolved) by_repo.setdefault((task.git_url, commit), []).append(task) - if by_repo or not dataset.tasks: - # The manifest is about to be rewritten: a folder it would not pin must not be there, and the - # kept folders must still be what it recorded. - refuse_unrecorded_folders(folder, names | set(previous), what="registry") + _refuse_repinned_folders(folder, repinned, dataset) + # A manifest that records another reference for these pins (one written before the reference was recorded, or a + # version the registry lists with the same tasks) is rewritten with this fetch's, so a run by the entry's name + # finds the fetch it declares on record (nemo_gym.tasks.harbor.cli._check_fetch_record). + rerecord = manifest is not None and manifest.ref != dataset.reference + if by_repo or not dataset.tasks or rerecord: + # The manifest is about to be rewritten: a folder it would not pin must not be there (one a previous fetch + # pinned for a task this version does not list is named as such, with the version), and the kept folders + # must still be what it recorded. + _refuse_dropped_folders(folder, previous, names, dataset) + refuse_unrecorded_folders(folder, names, what="registry") _check_kept_folders(folder, kept, manifest, digests) for (git_url, commit), tasks in by_repo.items(): with tempfile.TemporaryDirectory(prefix=_STAGING_PREFIX, dir=folder) as tmp: @@ -500,15 +596,97 @@ def fetch_dataset(dataset: RegistryDataset, root: Path) -> Path: raise HubError(f"Task destination appeared during fetch: {destination}; retry preparation") staged.rename(destination) pins[task.name] = (git_url, commit, task.path) - _write_manifest(folder, pins, digests) - if not dataset.tasks: - _write_manifest(folder, pins, digests) + _write_manifest(folder, pins, digests, reference=dataset.reference) + if not dataset.tasks or (rerecord and not by_repo): + _write_manifest(folder, pins, digests, reference=dataset.reference) return folder -def fetch_ref(target: str, root: Path | None = None, *, refresh_registry: bool = False) -> Path: +def expected_folder(target: str, root: Path | None = None) -> Path | None: + """Where a fetch of ``target`` lands when that follows from the reference alone, without resolving it. + + A package-store dataset's folder is named from its reference (```` for ``latest``, ``-``, + ``-<12 digest chars>`` for a digest pin, :meth:`PackageRef.folder_name`), and a registry dataset's from + its name and version; ``None`` for a registry reference whose version the registry picks. ``gym dataset pull`` + reads the folder's manifest here before the fetch to report an unchanged dataset. + """ + root = Path(root) if root is not None else datasets_dir() + ref = HubRef.parse(target) + if ref.is_package: + from nemo_gym.tasks.harbor.package_store import PackageRef + + package = PackageRef.of(ref) + return root / package.folder_name(package.digest or "") + if ref.version is None: + return None + return root / RegistryDataset(name=ref.name, version=ref.version, description="", tasks=()).folder_name + + +def resolved_folder(target: str, root: Path | None = None, *, refresh_registry: bool = False) -> Path: + """Where a fetch of ``target`` lands, resolving a registry reference whose version the registry picks. + + :func:`expected_folder` names the folder when the reference alone does; for ``harbor:`` without a + version the registry's cache under ``root/.harbor/`` (downloaded, or refreshed when ``refresh_registry``) is + read and the version it picks names the folder, as :func:`fetch_ref` will. ``gym dataset pull`` resolves such a + reference here, before the fetch, so it can read the folder's manifest and report an unchanged dataset. + """ + root = Path(root) if root is not None else datasets_dir() + folder = expected_folder(target, root) + if folder is not None: + return folder + registry = load_registry(root / ".harbor", refresh=refresh_registry) + return root / resolve_dataset(HubRef.parse(target), registry).folder_name + + +def canonical_reference(target: str) -> str: + """The ``harbor:`` reference ``target`` as the fetchers record it, so two spellings of one reference compare equal. + + A package-store reference is read as the store reads it (:attr:`~nemo_gym.tasks.harbor.package_store.PackageRef.reference`: + ``harbor:org/name`` is ``@latest``, a digest is its ``sha256:`` form). A registry reference is itself: a version it + names is the version, and ``harbor:`` without one is the registry's pick, another reference than any + version it may resolve to. A malformed reference raises :class:`HubError`, as :meth:`HubRef.parse` does. + """ + ref = HubRef.parse(target) + if ref.is_package: + from nemo_gym.tasks.harbor.package_store import PackageRef + + return PackageRef.of(ref).reference + return f"{REF_PREFIX}{ref.name}@{ref.version}" if ref.version else f"{REF_PREFIX}{ref.name}" + + +def declares_recorded_fetch(declared: str, recorded: str) -> bool: + """Whether the ``harbor:`` reference ``declared`` (a committed entry's ``[gym] source``) names the fetch a manifest + records as ``recorded`` (:attr:`~nemo_gym.tasks.harbor.manifest.Manifest.ref`), without asking the hub. + + A package-store reference matches by package and by tag, ``latest`` or digest, read as the store reads them + (:attr:`~nemo_gym.tasks.harbor.package_store.PackageRef.reference`: ``harbor:org/name`` is ``@latest``, a digest is + its ``sha256:`` form). A registry reference matches by dataset and, when it pins one, by version: the version an + unpinned ``harbor:`` gets is the registry's pick, which the record holds and only a pull can revise. + A record that does not parse as a reference matches nothing. + """ + try: + wanted, found = HubRef.parse(declared), HubRef.parse(recorded) + except HubError: + return False + if wanted.is_package != found.is_package or wanted.name != found.name: + return False + if wanted.is_package: + from nemo_gym.tasks.harbor.package_store import PackageRef + + return PackageRef.of(wanted).reference == PackageRef.of(found).reference + return wanted.version is None or wanted.version == found.version + + +def fetch_ref( + target: str, root: Path | None = None, *, refresh_registry: bool = False, folder: Path | None = None +) -> Path: """Resolve and fetch a ``harbor:`` reference; return the dataset folder. + The dataset lands in its own folder under ``root`` (:func:`datasets_dir` unless given), named after the + dataset and its version, or in ``folder`` when a caller names the destination: the folder of a committed + entry that declares the reference as its ``[gym] source`` (:func:`nemo_gym.tasks.harbor.entries.plan_pull` + decides that; this function does not read entries). The registry cache stays under ``root/.harbor/``. + A package-store fetch refuses a task folder whose content no longer matches the store (see :func:`nemo_gym.tasks.harbor.package_store.fetch_package_dataset`). Whatever the store fetch refuses or cannot read (a ``PackageStoreError``) is raised as a :class:`HubError`, so a caller handles @@ -519,10 +697,9 @@ def fetch_ref(target: str, root: Path | None = None, *, refresh_registry: bool = if ref.is_package: from nemo_gym.tasks.harbor.package_store import PackageRef, PackageStoreError, fetch_package_dataset - org, _, name = ref.name.partition("/") try: - return fetch_package_dataset(PackageRef(org=org, name=name, ref=ref.version or "latest"), root) + return fetch_package_dataset(PackageRef.of(ref), root, folder=folder) except PackageStoreError as exc: raise HubError(str(exc)) from exc registry = load_registry(root / ".harbor", refresh=refresh_registry) - return fetch_dataset(resolve_dataset(ref, registry), root) + return fetch_dataset(resolve_dataset(ref, registry), root, folder=folder) diff --git a/nemo_gym/tasks/harbor/manifest.py b/nemo_gym/tasks/harbor/manifest.py index 6e81f85d041..8ac8915fd7f 100644 --- a/nemo_gym/tasks/harbor/manifest.py +++ b/nemo_gym/tasks/harbor/manifest.py @@ -6,8 +6,10 @@ The file sits beside the task folders and holds only facts computed from them: where the folders came from (``source``), one dataset digest, the digest of ``dataset.toml`` alone, and per task the folder digest (the value rows carry as -``_ng_digest``) with its upstream pin. Name and version live only in ``dataset.toml``, so nothing -appears in both files. Nobody edits the manifest by hand. +``_ng_digest``) with its upstream pin. The dataset's name and version live only in ``dataset.toml``, so nothing +appears in both files; what a fetch records besides is the hub reference it was made from (``ref``), a fact of the +fetch like the pins, which is what a committed entry's declared ``source`` is checked against without the hub +(:func:`nemo_gym.tasks.harbor.cli._check_fetch_record`). Nobody edits the manifest by hand. The output is deterministic: a fixed key order, sorted tasks and no timestamps, so equal manifests mean identical inputs and two runs compare by ``[dataset].digest``. The dataset digest is a hash over @@ -153,6 +155,10 @@ class Manifest: tasks: dict[str, dict[str, str]] = field(default_factory=dict) # The dataset-level Gym files (:data:`DATASET_FILES`); the digest of nothing, and not written, without them. dataset_files_digest: str = _EMPTY_DIGEST + # The hub reference the fetch was made from: ``harbor:/@`` for the + # package store, ``harbor:@`` with the version it resolved for the registry; ``None``, and not + # written, for a local folder. + ref: str | None = None # The task folders :meth:`build` could not hash (recorded by name, no digest): computed, never written or read. unhashable: frozenset[str] = field(default_factory=frozenset) @@ -175,13 +181,15 @@ def build( pins: dict[str, dict[str, str]] | None = None, digests: dict[str, str] | None = None, names: Iterable[str] | None = None, + ref: str | None = None, ) -> "Manifest": """Compute the manifest of ``folder`` from its task folders and ``dataset.toml``. ``pins`` are the upstream pins to record per task; a pinned task whose folder is absent (an interrupted fetch) keeps its pin and gets no digest. ``digests`` are folder digests already computed (by a loader or a fetch); any task folder not in it is hashed here, and the - mapping is extended in place so a caller can reuse the work. + mapping is extended in place so a caller can reuse the work. ``ref`` is the hub reference a fetch + was made from, recorded as the pins are and kept by every rewrite of the manifest. ``names`` are the tasks to record. A fetcher passes the tasks it fetched, so a folder it did not fetch stays unrecorded (and prepare refuses it as not in the manifest) and no folder is scanned @@ -230,6 +238,7 @@ def build( dataset_toml_digest=toml_digest, tasks=tasks, dataset_files_digest=files_digest, + ref=ref, unhashable=frozenset(unhashable), ) @@ -255,11 +264,13 @@ def read(cls, folder: Path) -> "Manifest | None": tasks = data.get("tasks", {}) if not isinstance(dataset, dict) or not isinstance(tasks, dict): raise ManifestError(f"{path} is not a valid manifest: [dataset] and [tasks] must be tables") + ref = dataset.get("ref") return cls( source=str(dataset.get("source", "")), digest=str(dataset.get("digest", "")), dataset_toml_digest=str(dataset.get("dataset_toml_digest", "")), dataset_files_digest=str(dataset.get("dataset_files_digest", _EMPTY_DIGEST)), + ref=ref if isinstance(ref, str) and ref else None, tasks={ str(name): { # The package-store fetcher wrote `digest = "sha256:"` before the schema was numbered; the @@ -295,9 +306,11 @@ def text(self) -> str: "", "[dataset]", f"source = {_quote(self.source)}", - f"digest = {_quote(self.digest)}", - f"dataset_toml_digest = {_quote(self.dataset_toml_digest)}", ] + if self.ref is not None: + lines.append(f"ref = {_quote(self.ref)}") + lines.append(f"digest = {_quote(self.digest)}") + lines.append(f"dataset_toml_digest = {_quote(self.dataset_toml_digest)}") if self.dataset_files_digest != _EMPTY_DIGEST: lines.append(f"dataset_files_digest = {_quote(self.dataset_files_digest)}") lines.append("") @@ -587,6 +600,7 @@ def check_manifest(folder: Path, *, digests: dict[str, str] | None = None) -> Ma ) source = previous.source if previous is not None else LOCAL_SOURCE pins = previous.pins if previous is not None else None + ref = previous.ref if previous is not None else None if previous is not None and source != LOCAL_SOURCE and previous.records_content: # A pin without a digest in a manifest that records digests is a task the fetch never finished: # the claim is complete, the folder is not. Checked before the folders, since a claim-only folder @@ -596,7 +610,7 @@ def check_manifest(folder: Path, *, digests: dict[str, str] | None = None) -> Ma raise ManifestError( f"the fetch of {folder} was interrupted (tasks not yet recorded: {_some(unfetched)}); run the fetch again" ) - fresh = Manifest.build(folder, source=source, pins=pins, digests=digests) + fresh = Manifest.build(folder, source=source, pins=pins, digests=digests, ref=ref) if previous is None: if tracked_by_git(folder): print( @@ -620,18 +634,61 @@ def check_manifest(folder: Path, *, digests: dict[str, str] | None = None) -> Ma ) if not previous.records_content: why = "it recorded pins only" - elif not previous.digest: - # Written before the dataset digest existed (per-task digests only, see Manifest.records_content): - # rewritten for that alone, whatever dataset.toml held when it was written. - why = "it recorded no dataset digest" elif differences is not None: why = f"task folders changed ({differences})" - elif previous.dataset_files_digest != fresh.dataset_files_digest: - why = f"a dataset-level {' or '.join(DATASET_FILES)} changed" else: - why = f"{DATASET_FILE} changed" + why = gym_layer_change(previous, fresh) if _write_or_warn(fresh, folder): - print( - f"Rewrote {folder / MANIFEST_FILE}: {why}; dataset digest {previous.digest[:12] or '-'} -> {fresh.digest[:12]}" - ) + report_rewrite(folder, previous, fresh, why) + return fresh + + +def gym_layer_change(previous: "Manifest", fresh: "Manifest") -> str: + """Which part of the Gym layer differs between two manifests of one task set, for a rewrite notice. + + A manifest from before the dataset digest existed (per-task digests only, see :attr:`Manifest.records_content`) + is rewritten for that alone, whatever ``dataset.toml`` held when it was written. + """ + if not previous.digest: + return "it recorded no dataset digest" + if previous.dataset_files_digest != fresh.dataset_files_digest: + return f"a dataset-level {' or '.join(DATASET_FILES)} changed" + return f"{DATASET_FILE} changed" + + +def report_rewrite(folder: Path, previous: "Manifest", fresh: "Manifest", why: str) -> None: + """The one-line notice that ``folder``'s manifest was rewritten, and how the dataset digest moved.""" + print( + f"Rewrote {Path(folder) / MANIFEST_FILE}: {why}; dataset digest {previous.digest[:12] or '-'} -> {fresh.digest[:12]}" + ) + + +def refresh_gym_layer(folder: Path) -> Manifest | None: + """Bring a fetched manifest's Gym-layer digests up to date with ``folder`` without touching its task records. + + A fetch records ``dataset.toml`` and the dataset-level Gym files as it found them, and + rewrites the manifest only when it fetches something: a registry fetch with every folder in place, or a + pull that only replaced the copy of an entry's ``dataset.toml`` beside the tasks + (:func:`nemo_gym.tasks.harbor.entries.prepare_destination`), leaves ``dataset_toml_digest`` and the dataset + digest describing a file that is no longer there. This recomputes those digests from disk and keeps every + task's recorded digest and pin, and the fetch's reference, as they are (nothing is hashed, so a hand-edited + folder is still caught by :func:`check_manifest`, which rehashes), then writes the manifest when its text + changed, with the notice ``check_manifest`` prints for the same rewrite (``Rewrote ...; dataset digest -> + ``), so a change of + the dataset's identity is announced whichever function applies it. Returns the manifest now on disk: ``None`` + without one, and the old one untouched when it records pins only (``check_manifest`` upgrades those) or names a + source this Gym does not know (``check_manifest`` refuses those). + """ + folder = Path(folder) + previous = Manifest.read(folder) + if previous is None or not previous.records_content or previous.source not in SOURCES: + return previous + digests = {name: digest for name, digest in previous.task_digests.items() if digest is not None} + fresh = Manifest.build( + folder, source=previous.source, pins=previous.pins, digests=digests, names=list(digests), ref=previous.ref + ) + if fresh.text() == previous.text(): + return previous + if _write_or_warn(fresh, folder): + report_rewrite(folder, previous, fresh, gym_layer_change(previous, fresh)) return fresh diff --git a/nemo_gym/tasks/harbor/package_store.py b/nemo_gym/tasks/harbor/package_store.py index fa7c91a8877..d7f07809fb3 100644 --- a/nemo_gym/tasks/harbor/package_store.py +++ b/nemo_gym/tasks/harbor/package_store.py @@ -34,7 +34,7 @@ from typing import Any from nemo_gym.tasks.harbor.digest import archive_hash, content_hash -from nemo_gym.tasks.harbor.hub import contained_path, refuse_unrecorded_folders, validate_task_name +from nemo_gym.tasks.harbor.hub import REF_PREFIX, HubRef, contained_path, refuse_unrecorded_folders, validate_task_name from nemo_gym.tasks.harbor.manifest import MANIFEST_FILE, Manifest, ManifestError, UnsupportedManifestError @@ -61,6 +61,12 @@ class PackageRef: name: str ref: str = "latest" + @classmethod + def of(cls, ref: HubRef) -> "PackageRef": + """The store's reading of a ``harbor:/[@]`` reference: without ``@`` it is ``latest``.""" + org, _, name = ref.name.partition("/") + return cls(org=org, name=name, ref=ref.version or "latest") + @property def package(self) -> str: return f"{self.org}/{self.name}" @@ -70,6 +76,12 @@ def digest(self) -> str | None: match = _DIGEST.fullmatch(self.ref) return match.group(2) if match else None + @property + def reference(self) -> str: + """The reference as a manifest records it: ``harbor:/@``, so that + ``harbor:org/name`` and ``@latest``, or a digest with and without its ``sha256:`` prefix, read as one.""" + return f"{REF_PREFIX}{self.package}@{'sha256:' + self.digest if self.digest else self.ref}" + def folder_name(self, resolved_digest: str) -> str: """```` for ``latest``, ``-`` for a tag, ``-<12 digest chars>`` for a digest pin. @@ -348,7 +360,8 @@ def _read_manifest(folder: Path) -> Manifest | None: def _build_manifest(folder: Path, dataset: ResolvedDataset, digests: dict[str, str]) -> Manifest: - """Derived facts only: per-task digests and package pins; name and version stay in ``dataset.toml``. + """Derived facts only: per-task digests and package pins, and the reference this fetch was made from + (:attr:`PackageRef.reference`); the dataset's name and version stay in ``dataset.toml``. Every task of the store's version is pinned; a digest is recorded only for the tasks in ``digests``, the folders this fetch has verified (or trusts from the previous manifest, see :func:`_checkpoint_digests`). @@ -366,6 +379,7 @@ def _build_manifest(folder: Path, dataset: ResolvedDataset, digests: dict[str, s }, digests=digests, names=list(digests), + ref=dataset.ref.reference, ) @@ -422,7 +436,8 @@ def _claim_folder(folder: Path, dataset: ResolvedDataset, digests: dict[str, str def _check_recorded_identity(folder: Path, previous: Manifest | None, dataset: ResolvedDataset) -> None: """Refuse ``folder`` when its manifest was written for another package than ``dataset``. - The manifest holds no copy of the dataset's name (that lives in ``dataset.toml``), but every task it + The manifest holds no copy of the dataset's name (that lives in ``dataset.toml``), and one written before the + fetch's reference was recorded has no ``ref`` either, but every task it pins carries the task's own package, ``/``, as the store names it. A dataset may collect tasks published under any org (Harbor's ``dataset.toml`` pins each task as ``org/name``, and the store resolves each to its own package), so a pin is compared with the package ``dataset`` resolved that task @@ -489,9 +504,17 @@ def _check_recorded_dataset(folder: Path, previous: Manifest | None, dataset: Re ) -def fetch_package_dataset(ref: PackageRef, root: Path, store: PackageStore | None = None) -> Path: +def fetch_package_dataset( + ref: PackageRef, root: Path, store: PackageStore | None = None, *, folder: Path | None = None +) -> Path: """Fetch every task of a package-store dataset into ``root///``. + The dataset folder is ``root/[-]/`` (:meth:`PackageRef.folder_name`) unless + ``folder`` names it: the folder of a committed entry under ``environments/harbor/`` that declares this + dataset as its ``[gym] source`` (:mod:`nemo_gym.tasks.harbor.entries`). The entry's own files there (README, + YAML overlays, ``dataset.toml``) are not task folders and are left alone; the manifest's claim and the + per-task checks below are the same whichever folder the tasks land in. + A task folder whose digest already matches is kept, so a rerun after a partial fetch completes it: a folder the manifest records is compared with the digest recorded for it (Gym's digest of the folder as fetched, which a package shipping a name that @@ -519,7 +542,7 @@ def fetch_package_dataset(ref: PackageRef, root: Path, store: PackageStore | Non duplicates = sorted(name for name, count in counts.items() if count > 1) if duplicates: raise PackageStoreError(f"{ref.package}@{ref.ref} has tasks sharing a folder name: {', '.join(duplicates)}") - folder = Path(root) / ref.folder_name(dataset.content_hash) + folder = Path(folder) if folder is not None else Path(root) / ref.folder_name(dataset.content_hash) if folder.is_symlink(): raise PackageStoreError(f"Dataset folder must not be a symlink: {folder}") folder.mkdir(parents=True, exist_ok=True) diff --git a/nemo_gym/tasks/harbor/scaffold.py b/nemo_gym/tasks/harbor/scaffold.py index b3c2520c763..b1348c1ccae 100644 --- a/nemo_gym/tasks/harbor/scaffold.py +++ b/nemo_gym/tasks/harbor/scaffold.py @@ -42,6 +42,9 @@ # allowed_agents = ["hermes_agent"] # domain = "coding" # verified = false +# A committed entry under environments/harbor/ names the hub dataset `gym dataset pull ` fetches +# into its folder; an authored dataset, whose task folders are its own, leaves this unset. +# source = "harbor:/@" [gym.defaults] # Rollouts per task for an evaluation; `+num_repeats=` on the command line replaces it. diff --git a/tests/unit_tests/test_harbor_entries.py b/tests/unit_tests/test_harbor_entries.py new file mode 100644 index 00000000000..1eb042d92c3 --- /dev/null +++ b/tests/unit_tests/test_harbor_entries.py @@ -0,0 +1,1873 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""``gym dataset pull`` and committed dataset entries under ``environments/harbor/`` (M38, second slice). + +The checkout's own entries never enter these tests: every test runs against a temporary search root with the +datasets dir inside it, so what a pull lands in is what the test wrote. +""" + +import hashlib +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +from nemo_gym.tasks.harbor import cli as harbor_cli +from nemo_gym.tasks.harbor import entries as entries_module +from nemo_gym.tasks.harbor import hub as hub_module +from nemo_gym.tasks.harbor.cli import PulledDataset, entry_folder, prepare_target, pull_dataset +from nemo_gym.tasks.harbor.dataset_config import GymDatasetConfig, read_dataset_config +from nemo_gym.tasks.harbor.digest import content_hash +from nemo_gym.tasks.harbor.entries import ( + LAYER_RECORD_FILE, + DatasetEntry, + PullPlan, + committed_entries, + entry_at, + entry_for_ref, + entry_target, + find_entry, + plan_pull, + prepare_destination, +) +from nemo_gym.tasks.harbor.errors import HarborTaskError +from nemo_gym.tasks.harbor.hub import HubError, RegistryDataset, RegistryTask, declares_recorded_fetch +from nemo_gym.tasks.harbor.manifest import Manifest, dataset_files_digest, dataset_toml_digest +from nemo_gym.tasks.harbor.task_data import load_task_data_schema +from tests.unit_tests.test_harbor_tasks import ( + commit_repo, + fake_registry, # noqa: F401 + no_real_registry, # noqa: F401 (autouse fixture) + registry_dataset, + registry_repo, # noqa: F401 + write_task, +) + + +TB4_REF = "harbor:terminal-bench/terminal-bench@4.0.0" + + +@pytest.fixture +def root(tmp_path, monkeypatch) -> Path: + """A search root of its own, and the working root (``WORKING_DIR``): its ``environments/harbor/`` is both where + entries are committed and the datasets dir, as in a checkout; the working directory is the root, as on a command + line run from the checkout.""" + monkeypatch.setattr(entries_module, "_roots", lambda: [tmp_path]) + monkeypatch.setattr(entries_module, "WORKING_DIR", tmp_path) + monkeypatch.setattr(hub_module, "WORKING_DIR", tmp_path) + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "environments" / "harbor")) + monkeypatch.chdir(tmp_path) + (tmp_path / "environments" / "harbor").mkdir(parents=True) + return tmp_path + + +def write_entry(root: Path, name: str, source: str | None, *, extra: str = "") -> Path: + """A committed entry: README, a YAML overlay and a dataset.toml declaring ``source`` (none when ``None``).""" + folder = root / "environments" / "harbor" / name + folder.mkdir(parents=True) + (folder / "README.md").write_text(f"# {name}\n") + (folder / f"{name}.yaml").write_text("sandbox: {}\n") + table = f'source = "{source}"\n' if source is not None else "" + (folder / "dataset.toml").write_text(f"[gym]\n{table}{extra}") + return folder + + +def fake_store(tmp_path, monkeypatch, folders): + """Harbor's package store, answering for ``terminal-bench/terminal-bench@4.0.0`` with ``folders`` as its tasks.""" + from tests.unit_tests.test_harbor_package_store import TestPackageStore + + return TestPackageStore().fake_store(tmp_path, monkeypatch, folders) + + +def run_main(monkeypatch, *argv: str) -> None: + from nemo_gym.cli.main import main + + monkeypatch.setattr(sys, "argv", ["gym", *argv]) + main() + + +def main_error(monkeypatch, capsys, *argv: str) -> str: + """The one-line parser error ``gym `` ends with (exit status 2).""" + with pytest.raises(SystemExit) as exc: + run_main(monkeypatch, *argv) + assert exc.value.code == 2 + return capsys.readouterr().err + + +class TestSourceKey: + """``[gym] source`` is the hub reference a committed entry is pulled from; nothing else about the table moves.""" + + @pytest.mark.parametrize( + "source", + [TB4_REF, "harbor:terminal-bench/terminal-bench-2-1@sha256:" + "7d" * 32, "harbor:ds@1.0", "harbor:ds"], + ) + def test_accepts_package_store_and_registry_references(self, tmp_path, source): + (tmp_path / "dataset.toml").write_text(f'[gym]\nsource = "{source}"\n') + config = read_dataset_config(tmp_path) + assert config.source == source + assert not config.is_empty and not config.shapes_tasks # provenance, not a task-shaping setting + + @pytest.mark.parametrize("source", ["terminal-bench/terminal-bench@4.0.0", "harbor:", "harbor:a b", 3]) + def test_refuses_what_is_not_a_hub_reference_naming_the_key(self, tmp_path, source): + value = json.dumps(source) + (tmp_path / "dataset.toml").write_text(f"[gym]\nsource = {value}\n") + with pytest.raises(ValueError, match="source"): + read_dataset_config(tmp_path) + + def test_default_is_unset_and_the_scaffold_documents_it_without_setting_it(self, tmp_path): + from nemo_gym.tasks.harbor.scaffold import init_dataset + + assert GymDatasetConfig().source is None and GymDatasetConfig().is_empty + folder = init_dataset(tmp_path, "my-dataset") + assert '# source = "harbor:/@"' in (folder / "dataset.toml").read_text() + assert read_dataset_config(folder).is_empty + + def test_committed_entries_declare_their_sources(self): + """The two committed entries pin the datasets their READMEs describe, and nothing else in their tables.""" + from nemo_gym import PARENT_DIR + + tb4 = read_dataset_config(PARENT_DIR / "environments" / "harbor" / "terminal_bench_4") + tb21 = read_dataset_config(PARENT_DIR / "environments" / "harbor" / "terminal_bench_2_1") + assert tb4.source == TB4_REF + assert tb21.source == ( + "harbor:terminal-bench/terminal-bench-2-1@sha256:" + "7d7bdc1cbedad549fc1140404bd4dc45e5fd0ea7c4186773687d177ad3a0699a" # pragma: allowlist secret + ) + for config in (tb4, tb21): + assert config.model_copy(update={"source": None}).is_empty + + +class TestEntryLookup: + def test_find_entry_reads_the_source_and_resolves_without_one(self, root): + write_entry(root, "tb4", TB4_REF) + write_entry(root, "swe", None) + assert find_entry("tb4") == DatasetEntry("tb4", root / "environments" / "harbor" / "tb4", TB4_REF) + assert find_entry("swe") == DatasetEntry("swe", root / "environments" / "harbor" / "swe", None) + assert find_entry("missing") is None + assert find_entry("../tb4") is None and find_entry("tb4/") is None + + def test_find_entry_reports_an_unreadable_table_but_the_mapping_skips_it(self, root): + write_entry(root, "tb4", TB4_REF) + broken = root / "environments" / "harbor" / "broken" + broken.mkdir() + (broken / "dataset.toml").write_text("[gym]\nsource = 3\n") + with pytest.raises(ValueError, match="source"): + find_entry("broken") + assert [entry.name for entry in committed_entries()] == ["tb4"] + + def test_committed_entries_need_a_dataset_toml_with_a_source_and_skip_pulled_hub_folders(self, root): + write_entry(root, "tb4", TB4_REF) + write_entry(root, "swe", None) # an entry without a source is not a pull destination + pulled = root / "environments" / "harbor" / "terminal-bench-4.0.0" # a hub folder: no dataset.toml + write_task(pulled / "a") + (root / "environments" / "harbor" / ".harbor").mkdir() # the registry cache + (root / "environments" / "harbor" / "notes.md").write_text("not a folder\n") + assert [entry.name for entry in committed_entries()] == ["tb4"] + + def test_an_earlier_root_wins_a_name(self, root, tmp_path, monkeypatch): + other = tmp_path / "other" + write_entry(root, "tb4", TB4_REF) + write_entry(other, "tb4", "harbor:ds@1.0") + monkeypatch.setattr(entries_module, "_roots", lambda: [other, root]) + assert find_entry("tb4").source == "harbor:ds@1.0" + assert [entry.folder for entry in committed_entries()] == [other / "environments" / "harbor" / "tb4"] + + def test_a_source_less_entry_in_an_earlier_root_shadows_the_reference_too(self, root, tmp_path, monkeypatch): + """The name and the reference resolve through one set of effective entries: a source-less ``tb4`` in a + higher-priority root hides the sourced built-in one from find_entry, so entry_for_ref must not map its + reference to the hidden entry (the pull would then say `gym dataset validate tb4`, which selects other + tasks than it just downloaded). Shadowed, the reference lands in the hub's own folder (round 32).""" + other = tmp_path / "other" + shadow = write_entry(other, "tb4", None).resolve() + write_entry(root, "tb4", TB4_REF) + monkeypatch.setattr(entries_module, "_roots", lambda: [other, root]) + assert find_entry("tb4") == DatasetEntry("tb4", shadow, None) + assert committed_entries() == [] and entry_for_ref(TB4_REF) is None + assert plan_pull(TB4_REF) == PullPlan(ref=TB4_REF, entry=None) + with pytest.raises(HubError, match="declares no .* source"): + plan_pull("tb4") + # A folder without a dataset.toml, or with one that does not read, takes the name the same way. + (other / "environments" / "harbor" / "tb4" / "dataset.toml").unlink() + assert find_entry("tb4").source is None and entry_for_ref(TB4_REF) is None + (other / "environments" / "harbor" / "tb4" / "dataset.toml").write_text("[gym]\nsource = 3\n") + with pytest.raises(ValueError, match="source"): + find_entry("tb4") + assert entry_for_ref(TB4_REF) is None + + def test_an_entry_outside_the_working_root_is_pulled_under_the_datasets_dir(self, root, tmp_path, monkeypatch): + """Without NEMO_GYM_DATASETS_DIR an entry committed outside the working root (a NEMO_GYM_EXTRA_ROOTS root + here; the entry a package install ships under site-packages is the same case) lands in // + under the working root, with a copy of its Gym layer, never in its own folder, which may be shared or + read-only; an entry under the working root stays in its folder; set, the variable moves both.""" + checkout = tmp_path / "checkout" + monkeypatch.setattr(entries_module, "WORKING_DIR", checkout) + monkeypatch.setattr(hub_module, "WORKING_DIR", checkout) + extra = tmp_path / "extra" + monkeypatch.setattr(entries_module, "_roots", lambda: [extra, checkout]) + monkeypatch.delenv("NEMO_GYM_DATASETS_DIR") + folder = write_entry(extra, "tb4", TB4_REF).resolve() + own = write_entry(checkout, "mine", TB4_REF.replace("4.0.0", "3.0.0")).resolve() + destination = (checkout / "environments" / "harbor" / "tb4").resolve() + assert hub_module.datasets_dir() == destination.parent + entry = find_entry("tb4") + assert entry.folder == folder and entry.destination == destination + assert plan_pull(TB4_REF).folder == destination and plan_pull("tb4").folder == destination + assert find_entry("mine").destination == own and plan_pull("mine").folder == own + assert prepare_destination(entry) == destination + assert (destination / "dataset.toml").read_bytes() == (folder / "dataset.toml").read_bytes() + assert sorted(p.name for p in folder.iterdir()) == ["README.md", "dataset.toml", "tb4.yaml"] # untouched + assert entry_at(destination) == entry and entry_at(folder) is None and entry_at(own) is None + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + run_main(monkeypatch, "dataset", "pull", "tb4") + assert (destination / "alpha" / "task.toml").is_file() and Manifest.read(destination) is not None + assert sorted(p.name for p in folder.iterdir()) == ["README.md", "dataset.toml", "tb4.yaml"] + assert prepare_target("tb4", output_root=tmp_path / "out").tasks[0].task_id == "alpha" + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + assert find_entry("tb4").destination == (tmp_path / "shared" / "tb4").resolve() + assert find_entry("mine").destination == (tmp_path / "shared" / "mine").resolve() + + def _installed_layout(self, tmp_path, monkeypatch) -> tuple[Path, Path]: + """A package install: the working directory (``checkout``) is the first search root and the working root, the + installation's site-packages (outside it) the last, as component_search_roots orders them; the variable is + unset, so the datasets dir is ``checkout/environments/harbor/``.""" + checkout = tmp_path / "checkout" + site = tmp_path / "venv" / "lib" / "python3.13" / "site-packages" + monkeypatch.setattr(entries_module, "WORKING_DIR", checkout) + monkeypatch.setattr(hub_module, "WORKING_DIR", checkout) + monkeypatch.setattr(entries_module, "_roots", lambda: [checkout, site]) + monkeypatch.delenv("NEMO_GYM_DATASETS_DIR", raising=False) + return checkout, site + + def test_an_installed_entry_is_told_by_its_installation_prefix(self, tmp_path, monkeypatch): + """Installed is a path with a site-packages or dist-packages component, or one under the interpreter's + purelib/platlib or site-packages folders, whatever the working root; a plain folder is not.""" + monkeypatch.setattr(entries_module, "WORKING_DIR", tmp_path) + assert entries_module._installed_entry( + tmp_path / ".venv" / "lib" / "python3.13" / "site-packages" / "e" / "tb4" + ) + assert entries_module._installed_entry(tmp_path / "usr" / "lib" / "python3" / "dist-packages" / "e" / "tb4") + assert not entries_module._installed_entry(tmp_path / "environments" / "harbor" / "tb4") + prefix = tmp_path / "prefix" / "lib" / "pythonX" / "packages" + monkeypatch.setattr(entries_module.sysconfig, "get_paths", lambda: {"purelib": str(prefix), "platlib": None}) + monkeypatch.delattr(entries_module.site, "getsitepackages", raising=False) + monkeypatch.setattr(entries_module.site, "getusersitepackages", lambda: str(tmp_path / "user")) + assert entries_module._installed_entry(prefix / "environments" / "harbor" / "tb4") + assert entries_module._installed_entry(tmp_path / "user" / "environments" / "harbor" / "tb4") + assert not entries_module._installed_entry(tmp_path / "prefix" / "lib" / "other") + # A link into the installation counts as installed as the link's own path does not. + (tmp_path / "link").symlink_to(prefix / "environments", target_is_directory=True) + assert entries_module._installed_entry(tmp_path / "link" / "harbor" / "tb4") + + def test_an_installed_entry_under_the_working_root_is_pulled_under_the_datasets_dir(self, tmp_path, monkeypatch): + """A project-local .venv lies under the working directory, so its site-packages is under the working root + too; the entry it ships is installed all the same and its pulls go to //, never into + the installation (Yu He, round 36: both pulls wrote into .venv and failed on a read-only install).""" + checkout = tmp_path / "checkout" + site = checkout / ".venv" / "lib" / "python3.13" / "site-packages" + monkeypatch.setattr(entries_module, "WORKING_DIR", checkout) + monkeypatch.setattr(hub_module, "WORKING_DIR", checkout) + monkeypatch.setattr(entries_module, "_roots", lambda: [checkout, site]) + monkeypatch.delenv("NEMO_GYM_DATASETS_DIR", raising=False) + installed = write_entry(site, "tb4", TB4_REF).resolve() + destination = (checkout / "environments" / "harbor" / "tb4").resolve() + entry = find_entry("tb4") + assert entry.folder == installed and entry.destination == destination + assert plan_pull("tb4").folder == destination and plan_pull(TB4_REF).folder == destination + assert prepare_destination(entry) == destination and entry_at(destination) == entry + assert (destination / "dataset.toml").read_bytes() == (installed / "dataset.toml").read_bytes() + assert sorted(p.name for p in installed.iterdir()) == ["README.md", "dataset.toml", "tb4.yaml"] + + def test_the_copy_an_installed_entry_s_pull_placed_does_not_shadow_the_entry(self, root, tmp_path, monkeypatch): + """After the first pull the copy of the installed entry's table sits in the first root's + environments/harbor//, where lookup by name found it ahead of the installed entry and so read a stale + table forever after (Yu He, round 36). The copy folder, told by its .gym-layer.json, is passed over: the name, + the reference and the path all resolve to the installed entry, prepare and pull refresh the copy from its + current files, and a copy edited in place is still refused.""" + checkout, site = self._installed_layout(tmp_path, monkeypatch) + installed = write_entry(site, "tb4", TB4_REF).resolve() + destination = (checkout / "environments" / "harbor" / "tb4").resolve() + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + run_main(monkeypatch, "dataset", "pull", "tb4") + assert (destination / "alpha" / "task.toml").is_file() and (destination / LAYER_RECORD_FILE).is_file() + assert (destination / "dataset.toml").read_bytes() == (installed / "dataset.toml").read_bytes() + entry = find_entry("tb4") + assert entry.folder == installed and entry.destination == destination + assert plan_pull("tb4").entry == entry and plan_pull(TB4_REF).entry == entry + assert [found.folder for found in committed_entries()] == [installed] + assert entry_at(destination) == entry and entry_target("tb4") == entry + # The installed entry's table changes: lookup still reaches it, and a prepare by name re-copies the layer, so + # the override shapes the task and the copy equals the edited file. + edited = f'[gym]\nsource = "{TB4_REF}"\n[gym.tasks."alpha".environment]\nenv = {{ CIRCLE_NODE_TOTAL = "7" }}\n' + (installed / "dataset.toml").write_text(edited) + assert find_entry("tb4").folder == installed and plan_pull(TB4_REF).entry.folder == installed + prepared = prepare_target("tb4", output_root=tmp_path / "out") + assert prepared.folder == destination and prepared.tasks[0].env == {"CIRCLE_NODE_TOTAL": "7"} + assert (destination / "dataset.toml").read_text() == edited + assert prepared.dataset_toml_digest == Manifest.build(installed, source="local").dataset_toml_digest + # A second edit reaches the pull as well, which fetches nothing again. + (installed / "dataset.toml").write_text(edited.replace('"7"', '"9"')) + pull_dataset("tb4") + assert (destination / "dataset.toml").read_bytes() == (installed / "dataset.toml").read_bytes() + assert prepare_target("tb4", output_root=tmp_path / "out").tasks[0].env == {"CIRCLE_NODE_TOTAL": "9"} + # Edited in place, the copy is refused by name, by reference and by path, and kept. + (destination / "dataset.toml").write_text("[gym]\nverified = true\n") + for target in ("tb4", TB4_REF, str(destination)): + with pytest.raises(HubError, match="was edited in place and differs from the entry's"): + prepare_target(target, output_root=tmp_path / "out") + with pytest.raises(HubError, match="was edited in place and differs from the entry's"): + pull_dataset("tb4") + assert (destination / "dataset.toml").read_text() == "[gym]\nverified = true\n" + assert sorted(p.name for p in installed.iterdir()) == ["README.md", "dataset.toml", "tb4.yaml"] # untouched + + def test_a_user_authored_folder_in_the_working_root_still_wins_over_an_installed_entry( + self, tmp_path, monkeypatch + ): + """An entry the working root commits itself (no .gym-layer.json) keeps precedence over the installed one of + the same name, for the name and so for the reference, as any shadowing component does.""" + checkout, site = self._installed_layout(tmp_path, monkeypatch) + installed = write_entry(site, "tb4", TB4_REF).resolve() + own = write_entry(checkout, "tb4", "harbor:ds@1.0").resolve() + entry = find_entry("tb4") + assert entry == DatasetEntry("tb4", own, "harbor:ds@1.0") and entry.destination == own + assert [found.folder for found in committed_entries()] == [own] + assert entry_for_ref(TB4_REF) is None and plan_pull(TB4_REF) == PullPlan(ref=TB4_REF, entry=None) + assert plan_pull("tb4").folder == own and entry_target("tb4") == entry + assert prepare_destination(entry) == own and not (own / LAYER_RECORD_FILE).exists() + assert entry_at(own) is None and entry_at(installed) is None + assert sorted(p.name for p in installed.iterdir()) == ["README.md", "dataset.toml", "tb4.yaml"] + + def test_a_local_dataset_where_an_installed_entry_lands_is_refused_naming_the_installation( + self, tmp_path, monkeypatch + ): + """A dataset the user put at // by hand and ran by its path is the user's own entry of + that name when the working directory is the first root (no .gym-layer.json: it shadows the installed one, as + the test above). When the installed entry comes first instead (NEMO_GYM_EXTRA_ROOTS naming an installed + plugin's root), the folder is where its pull lands and is refused as not the entry's pull, naming the + installation rather than a working root the entry is under anyway.""" + checkout, site = self._installed_layout(tmp_path, monkeypatch) + folder = (checkout / "environments" / "harbor" / "tb4").resolve() + write_task(folder / "mine") + (folder / "dataset.toml").write_text("[gym]\nverified = true\n") + prepare_target(str(folder), output_root=tmp_path / "out") + assert Manifest.read(folder).source == "local" + installed = write_entry(site, "tb4", TB4_REF).resolve() + assert find_entry("tb4") == DatasetEntry("tb4", folder, None) and entry_at(folder) is None # the user's own + monkeypatch.setattr(entries_module, "_roots", lambda: [site, checkout]) + assert find_entry("tb4").folder == installed and entry_at(folder) == find_entry("tb4") + for target in ("tb4", str(folder)): + with pytest.raises(HubError) as exc: + prepare_target(target, output_root=tmp_path / "out") + assert f"(the entry is installed, under {installed.parent})" in str(exc.value), target + assert "records a local dataset, not a fetch" in str(exc.value) + assert "move the folder aside" in str(exc.value) + assert (folder / "dataset.toml").read_text() == "[gym]\nverified = true\n" + + def test_a_pulled_hub_folder_with_an_unreadable_table_is_not_an_entry_s_pull_elsewhere( + self, root, tmp_path, monkeypatch, capsys + ): + """In an editable checkout every pulled hub folder sits in the first root's environments/harbor/, so a path + to one resolves to itself as an entry of that name; entry_at compares the paths before reading any table, so + a dataset.toml written into such a folder that does not read never raises from there, and the prepare of + the path reports the file as it did before entries existed. Checked with the variable unset (the checkout's + own datasets dir) and set to that same folder.""" + folder = root / "environments" / "harbor" / "terminal-bench-4.0.0" + write_task(folder / "alpha") + (folder / "dataset.toml").write_text("[gym]\nsource = 3\n") + for variable in (None, str(root / "environments" / "harbor")): + if variable is None: + monkeypatch.delenv("NEMO_GYM_DATASETS_DIR") + else: + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", variable) + assert hub_module.datasets_dir() == folder.parent.resolve() + assert entry_at(folder) is None + with pytest.raises(ValueError, match="source"): + find_entry(folder.name) # the name form reads the table, as it did + err = main_error(monkeypatch, capsys, "dataset", "validate", str(folder)) + assert str(folder / "dataset.toml") in err and "Traceback" not in err + # A table that reads but declares nothing: a plain folder, prepared by its path as before. + (folder / "dataset.toml").write_text("[gym]\n") + assert entry_at(folder) is None + assert prepare_target(str(folder), output_root=tmp_path / "out").tasks[0].task_id == "alpha" + (folder / "dataset.toml").write_text("[gym]\nsource = 3\n") + + def test_a_local_dataset_where_an_entry_outside_the_working_root_lands_is_refused_in_its_own_words( + self, root, tmp_path, monkeypatch, capsys + ): + """With the variable unset, an entry a NEMO_GYM_EXTRA_ROOTS root commits lands in the checkout's + environments/harbor//. A dataset the user put there by hand and ran by its path (a `local` manifest + hashing the user's own dataset.toml) is refused by path, by name and by pull alike, and the refusal names the + entry's root instead of the variable, since the variable is not what moved the pull.""" + checkout = tmp_path / "checkout" + monkeypatch.setattr(entries_module, "WORKING_DIR", checkout) + monkeypatch.setattr(hub_module, "WORKING_DIR", checkout) + monkeypatch.setattr(entries_module, "_roots", lambda: [checkout]) + monkeypatch.delenv("NEMO_GYM_DATASETS_DIR") + folder = (checkout / "environments" / "harbor" / "tb4").resolve() + write_task(folder / "mine") + own = '[gym]\n[gym.tasks."mine".environment]\nenv = { CIRCLE_NODE_TOTAL = "7" }\n' + (folder / "dataset.toml").write_text(own) + before = prepare_target(str(folder), output_root=tmp_path / "out") + recorded = Manifest.read(folder) + assert recorded.source == "local" and before.tasks[0].env == {"CIRCLE_NODE_TOTAL": "7"} + extra = tmp_path / "extra" + entry = write_entry(extra, "tb4", TB4_REF).resolve() + monkeypatch.setattr(entries_module, "_roots", lambda: [extra, checkout]) + assert entry_at(folder) == find_entry("tb4") and find_entry("tb4").folder == entry + refusal = ( + f"{folder} is where entry tb4's pull lands (the entry is committed outside the working root, under " + f"{entry.parent}), but its manifest.toml records a local dataset, not a fetch, and " + f"{folder / 'dataset.toml'} differs from the entry's files under {entry}; it is not the entry's pull: " + "move the folder aside" + ) + for target in (str(folder), "tb4"): + with pytest.raises(HubError) as exc: + prepare_target(target, output_root=tmp_path / "out") + assert str(exc.value) == refusal, target + with pytest.raises(HubError) as exc: + pull_dataset("tb4") + assert str(exc.value) == refusal + err = main_error(monkeypatch, capsys, "dataset", "validate", str(folder)) + assert refusal in err and "Traceback" not in err + assert (folder / "dataset.toml").read_text() == own and Manifest.read(folder) == recorded + + def test_entry_for_ref_matches_the_same_dataset_and_version_only(self, root): + write_entry(root, "tb4", TB4_REF) + assert entry_for_ref(TB4_REF).name == "tb4" + # `latest` and another tag are other references; a plain reference never lands in a pinned entry. + assert entry_for_ref("harbor:terminal-bench/terminal-bench") is None + assert entry_for_ref("harbor:terminal-bench/terminal-bench@3.0.0") is None + assert entry_for_ref("harbor:terminal-bench@4.0.0") is None # a registry dataset of that name + + def test_entry_for_ref_reads_the_reference_as_the_fetchers_record_it(self, root): + """A digest with or without its ``sha256:`` prefix, and ``harbor:org/name`` with or without ``@latest``, are + one package-store reference (the one its manifest records, ``PackageRef.reference``), so a pull spelled either + way lands in the entry that declares the other, and two entries declaring it either way are the duplicate + they are. A registry reference still matches by version exactly: the version an unpinned one gets is the + registry's pick, so a pinned reference never lands in an unpinned entry, nor the reverse.""" + digest = "d" * 64 + write_entry(root, "tb_digest", f"harbor:terminal-bench/terminal-bench@sha256:{digest}") + write_entry(root, "tb_latest", "harbor:other/terminal-bench") + write_entry(root, "ds_pick", "harbor:ds") + assert entry_for_ref(f"harbor:terminal-bench/terminal-bench@{digest}").name == "tb_digest" + assert entry_for_ref(f"harbor:terminal-bench/terminal-bench@sha256:{digest}").name == "tb_digest" + assert plan_pull(f"harbor:terminal-bench/terminal-bench@{digest}").entry.name == "tb_digest" + assert entry_for_ref("harbor:other/terminal-bench@latest").name == "tb_latest" + assert entry_for_ref("harbor:other/terminal-bench").name == "tb_latest" + assert entry_for_ref("harbor:ds").name == "ds_pick" + assert entry_for_ref("harbor:ds@1.0") is None and entry_for_ref("harbor:other/terminal-bench@1.0") is None + write_entry(root, "tb_digest_copy", f"harbor:terminal-bench/terminal-bench@{digest}") + with pytest.raises(HubError, match=r"2 entries .*\(tb_digest, tb_digest_copy\)"): + entry_for_ref(f"harbor:terminal-bench/terminal-bench@sha256:{digest}") + + def test_two_entries_with_one_source_are_refused_with_both_named(self, root): + write_entry(root, "tb4", TB4_REF) + write_entry(root, "tb4_copy", TB4_REF) + with pytest.raises(HubError, match=r"2 entries .*\(tb4, tb4_copy\)"): + entry_for_ref(TB4_REF) + with pytest.raises(HubError, match="tb4_copy"): + plan_pull(TB4_REF) + + def test_entry_target_is_a_bare_name_that_is_not_a_path(self, root): + write_entry(root, "tb4", TB4_REF) + assert entry_target("tb4").name == "tb4" + assert entry_target("environments/harbor/tb4") is None # a path, resolved as a path + assert entry_target("./tb4") is None and entry_target(TB4_REF) is None + assert entry_target(".") is None and entry_target("..") is None + # An existing folder of the entry's name in the working directory keeps meaning that folder. + (root / "tb4").mkdir() + assert entry_target("tb4") is None + + +class TestPlanPull: + def test_a_reference_an_entry_declares_lands_in_the_entry(self, root): + folder = write_entry(root, "tb4", TB4_REF) + plan = plan_pull(TB4_REF) + assert plan.ref == TB4_REF and plan.entry.name == "tb4" and plan.folder == folder + + def test_a_reference_no_entry_declares_lands_in_the_hub_folder(self, root): + write_entry(root, "tb4", TB4_REF) + plan = plan_pull("harbor:terminal-bench/terminal-bench") + assert plan.entry is None and plan.folder is None + + def test_an_entry_name_pulls_its_source_into_its_folder(self, root): + folder = write_entry(root, "tb4", TB4_REF) + plan = plan_pull("tb4") + assert plan.ref == TB4_REF and plan.folder == folder + + def test_an_entry_without_a_source_has_nothing_to_pull(self, root): + write_entry(root, "swe", None) + with pytest.raises(HubError, match=r"declares no \[gym\] source.*entry swe"): + plan_pull("swe") + + def test_a_path_is_not_a_pull(self, root): + write_entry(root, "tb4", TB4_REF) + assert plan_pull("some/folder") is None and plan_pull("nonsense") is None + assert plan_pull("./tb4") is None and plan_pull("environments/harbor/tb4") is None + + +class TestPrepareDestination: + def test_the_entry_folder_itself_needs_nothing(self, root): + folder = write_entry(root, "tb4", TB4_REF) + before = sorted(path.name for path in folder.iterdir()) + assert prepare_destination(find_entry("tb4")) == folder + assert sorted(path.name for path in folder.iterdir()) == before + + def test_a_moved_datasets_dir_gets_a_copy_of_the_entry_table(self, root, tmp_path, monkeypatch): + folder = write_entry(root, "tb4", TB4_REF, extra="verified = true\n") + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + entry = find_entry("tb4") + destination = prepare_destination(entry) + assert destination == (tmp_path / "shared" / "tb4").resolve() + assert (destination / "dataset.toml").read_bytes() == (folder / "dataset.toml").read_bytes() + # The copy is recorded as placed, beside it, in a file only this function writes. + record = json.loads((destination / LAYER_RECORD_FILE).read_text()) + assert record == {"dataset.toml": hashlib.sha256((folder / "dataset.toml").read_bytes()).hexdigest()} + # The same copy again is fine. With nothing pulled yet (no manifest, no task folder: a first fetch that + # failed before claiming the folder) the copy this function placed follows the entry's file, and the record + # follows the copy. + assert prepare_destination(entry) == destination + (folder / "dataset.toml").write_text(f'[gym]\nsource = "{TB4_REF}"\nverified = false\n') + assert prepare_destination(entry) == destination + assert (destination / "dataset.toml").read_bytes() == (folder / "dataset.toml").read_bytes() + assert json.loads((destination / LAYER_RECORD_FILE).read_text()) != record + # A copy that is not the one on record was edited in place, pulled or not: refused, never overwritten + # (round 32, second look: the record, not the folder's manifest or its emptiness, is what vouches for a copy). + (destination / "dataset.toml").write_text("[gym]\nverified = false\n") + with pytest.raises(HubError, match="was edited in place and differs from the entry's"): + prepare_destination(entry) + assert (destination / "dataset.toml").read_text() == "[gym]\nverified = false\n" + # Beside task folders no pull recorded, the same: a differing copy may be the user's own table. + write_task(destination / "alpha") + with pytest.raises(HubError, match="was edited in place and differs from the entry's"): + prepare_destination(entry) + assert (destination / "dataset.toml").read_text() == "[gym]\nverified = false\n" + # Moved aside, the entry's file is placed again and recorded. + (destination / "dataset.toml").rename(tmp_path / "table-aside") + assert prepare_destination(entry) == destination + assert (destination / "dataset.toml").read_bytes() == (folder / "dataset.toml").read_bytes() + + def test_a_copy_the_last_pull_recorded_follows_the_entry_table(self, root, tmp_path, monkeypatch): + """The copy is replaced when the entry's file changed and the copy is still the one the manifest hashed; + a prepare (``create=False``) does the same and leaves a folder that was never pulled missing.""" + entry = write_entry(root, "tb4", TB4_REF) + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "tb4").resolve() + assert prepare_destination(find_entry("tb4"), create=False) == destination and not destination.exists() + prepare_destination(find_entry("tb4")) + write_task(destination / "alpha") + # What a pull leaves: a fetch's manifest hashing the copy (a `local` one is nobody's record of copies). + Manifest.build(destination, source="harbor-package-store").write(destination) + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{TB4_REF}"\nverified = true\n') + for create in (False, True): + assert prepare_destination(find_entry("tb4"), create=create) == destination + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + # The record follows the copy: the manifest's dataset_toml_digest is the new copy's, the task record is kept + # as it was, and a second edit of the entry's file, with nothing rewriting the manifest in between, follows + # as well instead of reading as an in-place edit (round 2). + recorded = Manifest.read(destination) + assert recorded.dataset_toml_digest == dataset_toml_digest(destination) == dataset_toml_digest(entry) + assert recorded.tasks["alpha"]["digest"] == content_hash(destination / "alpha") + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{TB4_REF}"\nverified = false\n') + assert prepare_destination(find_entry("tb4"), create=False) == destination + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + assert Manifest.read(destination).dataset_toml_digest == dataset_toml_digest(entry) + # Edited in place after that pull: refused by both, and kept. + (destination / "dataset.toml").write_text("[gym]\nverified = false\n") + for create in (False, True): + with pytest.raises(HubError, match="was edited in place and differs from the entry's"): + prepare_destination(find_entry("tb4"), create=create) + assert (destination / "dataset.toml").read_text() == "[gym]\nverified = false\n" + + +class TestPullDataset: + """The pull itself, through the package store (faked at its HTTP boundary) and the registry (a local git repo).""" + + def test_pulls_an_entry_into_its_folder_and_reports_tasks_and_digest(self, root, monkeypatch, capsys): + folders = {name: write_task(root / "src" / name) for name in ("beta", "alpha")} + module, digests, calls = fake_store(root, monkeypatch, folders) + folder = write_entry(root, "tb4", TB4_REF) + committed = {path.name: path.read_bytes() for path in folder.iterdir()} + + run_main(monkeypatch, "dataset", "pull", "tb4") + + out = capsys.readouterr().out.splitlines() + manifest = Manifest.read(folder) + assert sorted(path.name for path in folder.iterdir() if path.is_dir()) == ["alpha", "beta"] + assert content_hash(folder / "alpha") == digests["alpha"] + assert manifest.source == "harbor-package-store" and set(manifest.tasks) == {"alpha", "beta"} + assert out == [ + f"Pulled {TB4_REF} into {folder} (entry tb4)", + f"2 task(s), dataset digest {manifest.digest} (harbor-package-store)", + "Next: gym dataset validate tb4 --sandbox ", + ] + # The committed files are untouched, and nothing was prepared: no rows, records or results. + assert {name: (folder / name).read_bytes() for name in committed} == committed + assert not (folder / "compose-images.json").exists() and not (root / "results").exists() + # The dataset's identity covers the entry's table (dataset.toml is hashed into the manifest). + assert manifest.dataset_toml_digest != Manifest.build(root / "src", source="local").dataset_toml_digest + + def test_a_second_pull_downloads_nothing_and_says_so(self, root, monkeypatch, capsys): + folders = {"alpha": write_task(root / "src" / "alpha")} + _, _, calls = fake_store(root, monkeypatch, folders) + folder = write_entry(root, "tb4", TB4_REF) + run_main(monkeypatch, "dataset", "pull", "tb4") + first = capsys.readouterr().out + downloads = sum(1 for call in calls if call[1].startswith("/storage/")) + manifest = (folder / "manifest.toml").read_bytes() + + run_main(monkeypatch, "dataset", "pull", "tb4") + + second = capsys.readouterr().out + assert sum(1 for call in calls if call[1].startswith("/storage/")) == downloads + assert (folder / "manifest.toml").read_bytes() == manifest + assert "unchanged since the last pull" in second and "unchanged" not in first + assert second.splitlines()[0] == first.splitlines()[0] + + def test_the_reference_and_the_entry_name_are_one_folder(self, root, monkeypatch, capsys): + """Pulling the reference an entry declares lands in the entry, so the dataset never has a sibling copy.""" + folders = {"alpha": write_task(root / "src" / "alpha")} + fake_store(root, monkeypatch, folders) + folder = write_entry(root, "tb4", TB4_REF) + + run_main(monkeypatch, "dataset", "pull", TB4_REF) + + assert capsys.readouterr().out.splitlines()[0] == f"Pulled {TB4_REF} into {folder} (entry tb4)" + harbor = root / "environments" / "harbor" + assert sorted(path.name for path in harbor.iterdir()) == ["tb4"] + run_main(monkeypatch, "dataset", "pull", "tb4") + assert "unchanged since the last pull" in capsys.readouterr().out + assert sorted(path.name for path in harbor.iterdir()) == ["tb4"] + + def test_a_reference_without_an_entry_lands_in_the_hub_folder_as_before(self, root, monkeypatch, capsys): + folders = {"alpha": write_task(root / "src" / "alpha")} + fake_store(root, monkeypatch, folders) + write_entry(root, "other", "harbor:ds@1.0") # an entry for another dataset does not attract this one + + run_main(monkeypatch, "dataset", "pull", TB4_REF) + + folder = (root / "environments" / "harbor" / "terminal-bench-4.0.0").resolve() + out = capsys.readouterr().out.splitlines() + assert out[0] == f"Pulled {TB4_REF} into {folder}" + assert out[2] == f"Next: gym dataset validate {folder} --sandbox " + assert (folder / "alpha" / "task.toml").is_file() and Manifest.read(folder).tasks.keys() == {"alpha"} + # The hub folder's name follows from the reference, so a second pull of it reports unchanged too. + run_main(monkeypatch, "dataset", "pull", TB4_REF) + assert "unchanged since the last pull" in capsys.readouterr().out + + def test_a_reference_spelled_without_the_digest_prefix_lands_in_the_entry_that_declares_it( + self, root, monkeypatch, capsys + ): + """``harbor:org/name@`` without ``sha256:`` is the reference the entry declares with it, read as the + store reads it, so the pull lands in the entry and never beside it in ``-<12 digest chars>/``; the + manifest records the store's form, so the entry's name finds the fetch it declares.""" + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + digest = "d" * 64 + folder = write_entry(root, "tb4", f"harbor:terminal-bench/terminal-bench@sha256:{digest}") + + run_main(monkeypatch, "dataset", "pull", f"harbor:terminal-bench/terminal-bench@{digest}") + + out = capsys.readouterr().out.splitlines() + assert out[0] == f"Pulled harbor:terminal-bench/terminal-bench@{digest} into {folder} (entry tb4)" + harbor = root / "environments" / "harbor" + assert sorted(path.name for path in harbor.iterdir()) == ["tb4"] + assert Manifest.read(folder).ref == f"harbor:terminal-bench/terminal-bench@sha256:{digest}" + run_main(monkeypatch, "dataset", "pull", "tb4") + assert "unchanged since the last pull" in capsys.readouterr().out + assert prepare_target("tb4", output_root=root / "out").dataset_source == "harbor-package-store" + + def test_expected_folder_follows_from_the_reference_alone(self, root): + from nemo_gym.tasks.harbor.hub import expected_folder + + harbor = (root / "environments" / "harbor").resolve() + assert expected_folder(TB4_REF) == harbor / "terminal-bench-4.0.0" + assert expected_folder("harbor:terminal-bench/terminal-bench") == harbor / "terminal-bench" + assert expected_folder("harbor:o/n@sha256:" + "7d" * 32) == harbor / ("n-" + "7d" * 6) + assert expected_folder("harbor:ds@1.0") == harbor / "ds-1.0" + assert expected_folder("harbor:ds") is None # the registry picks the version + assert expected_folder("harbor:ds@1.0", root / "elsewhere") == root / "elsewhere" / "ds-1.0" + + def test_resolved_folder_asks_the_registry_only_when_the_reference_leaves_it_the_version( + self, root, monkeypatch, tmp_path + ): + from nemo_gym.tasks.harbor.hub import RegistryDataset, resolved_folder + + harbor = (root / "environments" / "harbor").resolve() + datasets = [RegistryDataset("ds", version, "", ()) for version in ("1.0", "2.0")] + loads = [] + + def load_registry(cache_dir, *, refresh=False): + loads.append((cache_dir, refresh)) + return datasets + + monkeypatch.setattr(hub_module, "load_registry", load_registry) + assert resolved_folder(TB4_REF) == harbor / "terminal-bench-4.0.0" and loads == [] + assert resolved_folder("harbor:ds@1.0") == harbor / "ds-1.0" and loads == [] + assert resolved_folder("harbor:ds") == harbor / "ds-2.0" # the registry's newest + assert ( + resolved_folder("harbor:ds", tmp_path / "elsewhere", refresh_registry=True) + == tmp_path / "elsewhere" / "ds-2.0" + ) + assert loads == [(harbor / ".harbor", False), (tmp_path / "elsewhere" / ".harbor", True)] + with pytest.raises(HubError, match="not in the Harbor registry"): + resolved_folder("harbor:other") + + def test_a_registry_reference_without_a_version_reports_unchanged_on_the_second_pull( + self, root, monkeypatch, registry_repo, capsys + ): # noqa: F811 + """`harbor:ds` leaves the version to the registry, so the folder is known only after resolution; the pull + resolves first and the second pull says it downloaded nothing, as the versioned form does (round 3).""" + dataset = registry_dataset(registry_repo) + loads = [] + + def load_registry(cache_dir, *, refresh=False): + loads.append(refresh) + return [dataset] + + monkeypatch.setattr(hub_module, "load_registry", load_registry) + folder = (root / "environments" / "harbor" / "ds-1.0").resolve() + run_main(monkeypatch, "dataset", "pull", "harbor:ds") + first = capsys.readouterr().out.splitlines() + assert first[0] == f"Pulled harbor:ds into {folder}" and "unchanged" not in first[1] + run_main(monkeypatch, "dataset", "pull", "harbor:ds") + second = capsys.readouterr().out.splitlines() + assert second[0] == first[0] and second[1] == first[1] + ", unchanged since the last pull" + assert loads == [False] * 4 # the cache is read, never downloaded twice for one pull + # A refresh asked for by prepare_target's kwarg downloads the registry once, for the resolution and the fetch. + pull_dataset("harbor:ds", refresh_registry=True) + assert loads[4:] == [True, False] + + def test_a_registry_reference_pulls_through_git_into_the_entry(self, root, monkeypatch, registry_repo, capsys): # noqa: F811 + dataset = registry_dataset(registry_repo) + monkeypatch.setattr(hub_module, "load_registry", lambda cache_dir, *, refresh=False: [dataset]) + folder = write_entry(root, "ds_entry", "harbor:ds@1.0") + + run_main(monkeypatch, "dataset", "pull", "ds_entry") + + manifest = Manifest.read(folder) + out = capsys.readouterr().out.splitlines() + assert (folder / "a" / "task.toml").is_file() + assert manifest.source == "harbor-registry" and manifest.pins["a"]["path"] == "tasks/a" + assert out[0] == f"Pulled harbor:ds@1.0 into {folder} (entry ds_entry)" + assert out[1] == f"1 task(s), dataset digest {manifest.digest} (harbor-registry)" + # A registry pull with every folder in place writes nothing; the report says so. + run_main(monkeypatch, "dataset", "pull", "ds_entry") + assert "unchanged since the last pull" in capsys.readouterr().out + + def test_refresh_registry_is_prepare_targets_pass_through_not_a_flag( + self, root, monkeypatch, capsys, registry_repo + ): # noqa: F811 + """``prepare_target(refresh_registry=True)`` reaches the registry loader through the pull; the command takes + no such flag (none of the three commands does; deleting ``.harbor/registry.json`` refreshes the cache).""" + seen = [] + dataset = registry_dataset(registry_repo) + + def load(cache_dir, *, refresh=False): + seen.append((Path(cache_dir), refresh)) + return [dataset] + + monkeypatch.setattr(hub_module, "load_registry", load) + write_entry(root, "ds_entry", "harbor:ds@1.0") + pull_dataset("ds_entry", refresh_registry=True) + run_main(monkeypatch, "dataset", "pull", "ds_entry") + cache = (root / "environments" / "harbor").resolve() / ".harbor" + assert seen == [(cache, True), (cache, False)] + err = main_error(monkeypatch, capsys, "dataset", "pull", "ds_entry", "--refresh-registry") + assert "unrecognized arguments: --refresh-registry" in err + + def test_a_moved_datasets_dir_holds_the_tasks_and_the_entry_table(self, root, tmp_path, monkeypatch, capsys): + folders = {"alpha": write_task(root / "src" / "alpha")} + fake_store(root, monkeypatch, folders) + entry = write_entry(root, "tb4", TB4_REF, extra="verified = true\n") + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + + run_main(monkeypatch, "dataset", "pull", "tb4") + + destination = (tmp_path / "shared" / "tb4").resolve() + assert capsys.readouterr().out.splitlines()[0] == f"Pulled {TB4_REF} into {destination} (entry tb4)" + assert (destination / "alpha" / "task.toml").is_file() + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + assert not (entry / "alpha").exists() and not (entry / "manifest.toml").exists() + assert ( + Manifest.read(destination).dataset_toml_digest == Manifest.build(entry, source="local").dataset_toml_digest + ) + + def test_a_moved_datasets_dir_gets_the_entry_s_dataset_level_files_too(self, root, tmp_path, monkeypatch, capsys): + """An entry's task_data.py and verifier.py enter the dataset digest and shape loading as its dataset.toml + does, so they travel with it to a moved destination, follow the entry's files (a file the entry dropped goes + too) and are refused when a copy was edited in place (round 3).""" + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + entry = write_entry(root, "tb4", TB4_REF) + model = "from pydantic import BaseModel\n\n\nclass TaskData(BaseModel):\n answer: str = 'x'\n" + (entry / "task_data.py").write_text(model) + (entry / "verifier.py").write_text("def verify(workspace):\n return 1.0\n") + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "tb4").resolve() + + run_main(monkeypatch, "dataset", "pull", "tb4") + + assert sorted(p.name for p in destination.iterdir()) == [ + LAYER_RECORD_FILE, + "alpha", + "dataset.toml", + "manifest.toml", + "task_data.py", + "verifier.py", + ] + assert set(json.loads((destination / LAYER_RECORD_FILE).read_text())) == { + "dataset.toml", + "task_data.py", + "verifier.py", + } + assert (destination / "verifier.py").read_text() == "def verify(workspace):\n return 1.0\n" + assert load_task_data_schema(destination) is not None + manifest = Manifest.read(destination) + for digest in (dataset_toml_digest, dataset_files_digest): + assert digest(destination) == digest(entry) + assert manifest.dataset_files_digest == dataset_files_digest(entry) + # The entry drops its model and edits the grader: a prepare alone (no pull) brings the copies along, the record + # follows, and the run reads them from the destination. + (entry / "task_data.py").unlink() + (entry / "verifier.py").write_text("def verify(workspace):\n return 0.5\n") + capsys.readouterr() + prepared = prepare_target("tb4", output_root=tmp_path / "out") + out = capsys.readouterr().out + assert prepared.folder == destination and not (destination / "task_data.py").exists() + assert (destination / "verifier.py").read_text() == "def verify(workspace):\n return 0.5\n" + second = Manifest.read(destination) + assert prepared.dataset_digest == second.digest != manifest.digest and second.tasks == manifest.tasks + assert second.dataset_files_digest == dataset_files_digest(entry) and out.count("Rewrote") == 1 + assert "a dataset-level task_data.py or verifier.py changed" in out + # A file the entry no longer has goes, and one it gains arrives. + (entry / "verifier.py").unlink() + (entry / "task_data.py").write_text(model) + run_main(monkeypatch, "dataset", "pull", "tb4") + assert (destination / "task_data.py").exists() and not (destination / "verifier.py").exists() + assert Manifest.read(destination).dataset_files_digest == dataset_files_digest(entry) + # A copy edited in place is refused by pull, validate and run alike, and kept. + (destination / "task_data.py").write_text(model + "# edited here\n") + (destination / "verifier.py").write_text("# placed here\n") + expected = ( + f"{destination / 'task_data.py'}, {destination / 'verifier.py'} were edited in place and " + f"differ from the entry's files under {entry}" + ) + for argv in ( + ("dataset", "pull", "tb4"), + ("dataset", "validate", "tb4"), + ("eval", "run", "tb4", "--agent", "hermes_agent"), + ): + err = main_error(monkeypatch, capsys, *argv) + assert expected in err and "Traceback" not in err, argv + assert (destination / "task_data.py").read_text() == model + "# edited here\n" + (destination / "verifier.py").unlink() + err = main_error(monkeypatch, capsys, "dataset", "pull", "tb4") + assert f"{destination / 'task_data.py'} was edited in place and differs from the entry's" in err + + def test_an_entry_table_edited_after_the_pull_reaches_pull_and_prepare_alike( + self, root, tmp_path, monkeypatch, capsys + ): + """Under a moved datasets dir the copy beside the tasks follows the entry's file, so `gym dataset pull ` + and `gym eval run ` read one table: the TB4 README's `nextjs-performance` override, written into the + entry's dataset.toml after the pull, shapes the next run, and the digest follows.""" + folders = {"alpha": write_task(root / "src" / "alpha")} + _, _, calls = fake_store(root, monkeypatch, folders) + entry = write_entry(root, "tb4", TB4_REF) + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "tb4").resolve() + run_main(monkeypatch, "dataset", "pull", "tb4") + first = Manifest.read(destination) + downloads = sum(1 for call in calls if call[1].startswith("/storage/")) + capsys.readouterr() + + def override(total: str) -> None: + (entry / "dataset.toml").write_text( + f'[gym]\nsource = "{TB4_REF}"\n[gym.tasks."alpha".environment]\nenv = {{ CIRCLE_NODE_TOTAL = "{total}" }}\n' + ) + + override("3") + run_main(monkeypatch, "dataset", "pull", "tb4") + out = capsys.readouterr().out + second = Manifest.read(destination) + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + assert second.digest != first.digest and f"dataset digest {second.digest}" in out and "unchanged" not in out + # Announced once (round 3): by the copy's refresh, not again by the store fetch that then rewrote the same text. + assert out.count("Rewrote") == 1 and f"dataset digest {first.digest[:12]} -> {second.digest[:12]}" in out + assert second.dataset_toml_digest == Manifest.build(entry, source="local").dataset_toml_digest + assert sum(1 for call in calls if call[1].startswith("/storage/")) == downloads # nothing fetched again + # Edited again, a prepare alone (no pull in between) sees the entry's table too. + override("5") + prepared = prepare_target("tb4", output_root=tmp_path / "out") + assert prepared.folder == destination and prepared.tasks[0].env == {"CIRCLE_NODE_TOTAL": "5"} + assert prepared.dataset_toml_digest == Manifest.build(entry, source="local").dataset_toml_digest + assert prepared.dataset_digest == Manifest.read(destination).digest != second.digest + run_main(monkeypatch, "dataset", "pull", "tb4") + assert "unchanged since the last pull" in capsys.readouterr().out + + def test_a_copy_edited_in_place_is_refused_by_pull_and_prepare_alike(self, root, tmp_path, monkeypatch, capsys): + folders = {"alpha": write_task(root / "src" / "alpha")} + fake_store(root, monkeypatch, folders) + entry = write_entry(root, "tb4", TB4_REF) + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "tb4").resolve() + run_main(monkeypatch, "dataset", "pull", "tb4") + capsys.readouterr() + (destination / "dataset.toml").write_text("[gym]\nverified = true\n") + expected = ( + f"{destination / 'dataset.toml'} was edited in place and differs from the entry's {entry / 'dataset.toml'}" + ) + for argv in ( + ("dataset", "pull", "tb4"), + ("dataset", "validate", "tb4"), + ("eval", "run", "tb4", "--agent", "hermes_agent"), + ): + err = main_error(monkeypatch, capsys, *argv) + assert expected in err and "Traceback" not in err, argv + assert (destination / "dataset.toml").read_text() == "[gym]\nverified = true\n" # never overwritten + assert not (tmp_path / "out").exists() and not (root / "results").exists() + + def test_a_path_prepare_of_the_moved_pull_protects_the_copy_as_the_name_does( + self, root, tmp_path, monkeypatch, capsys + ): + """Under a moved datasets dir the pulled folder can be named by its path too. A prepare by that path used to + run the folder as any other and, through the manifest check, record an in-place edit of the copied + dataset.toml as the dataset's; the next pull by name then read the copy as the one on record and overwrote + it with the entry's file, losing a local override (round 32). The path form now keeps the copy current, + or refuses it, exactly as the pull and the name do.""" + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + entry = write_entry(root, "tb4", TB4_REF) + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "tb4").resolve() + pull_dataset("tb4") + recorded = Manifest.read(destination) + capsys.readouterr() + assert entry_at(destination) == find_entry("tb4") + assert entry_at(destination / "alpha") is None and entry_at(entry) is None + assert entry_at(tmp_path / "shared" / "terminal-bench-4.0.0") is None # a hub folder, nobody's entry + edited = f'[gym]\nsource = "{TB4_REF}"\n[gym.tasks."alpha".environment]\nenv = {{ CIRCLE_NODE_TOTAL = "7" }}\n' + (destination / "dataset.toml").write_text(edited) + refusal = ( + f"{destination / 'dataset.toml'} was edited in place and differs from the entry's {entry / 'dataset.toml'}" + ) + for target in (str(destination), "tb4"): + with pytest.raises(HubError) as exc: + prepare_target(target, output_root=tmp_path / "out") + assert refusal in str(exc.value), target + err = main_error(monkeypatch, capsys, "dataset", "validate", str(destination)) + assert refusal in err and "Traceback" not in err + # Nothing absorbed the edit: the manifest still records the pull's copy, and the edit is still there. + assert Manifest.read(destination) == recorded and (destination / "dataset.toml").read_text() == edited + with pytest.raises(HubError, match="edited in place"): + pull_dataset("tb4") + assert (destination / "dataset.toml").read_text() == edited + # The entry's file edited instead: the path form refreshes the copy as the name form does. + (destination / "dataset.toml").write_bytes((entry / "dataset.toml").read_bytes()) + (entry / "dataset.toml").write_text(edited) + prepared = prepare_target(str(destination), output_root=tmp_path / "out") + assert prepared.tasks[0].env == {"CIRCLE_NODE_TOTAL": "7"} + assert (destination / "dataset.toml").read_text() == edited + assert prepare_target("tb4", output_root=tmp_path / "out").dataset_digest == prepared.dataset_digest + # Without the variable the entry's own folder (under the working root, which the fixture makes the temp root) + # is the pull, and a path to it goes through nothing extra. + monkeypatch.delenv("NEMO_GYM_DATASETS_DIR") + assert entry_at(entry) is None + + def test_an_edit_the_manifest_absorbed_in_a_shell_without_the_variable_is_still_refused( + self, root, tmp_path, monkeypatch, capsys + ): + """The other door to the same loss: the pulled folder prepared by its path in a shell without + NEMO_GYM_DATASETS_DIR, which cannot know it is an entry's pull. That prepare runs the edited copy and, as for + any dataset whose dataset.toml changed, rewrites the manifest's Gym-layer digests over it; the copies used to + be checked against those digests, so the next pull with the variable read the edit as the copy on record and + overwrote it (round 32, second look). The copies are now checked against the record of what was placed, + which only the placing writes, so the pull, the name and the path refuse the edit and keep it.""" + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + entry = write_entry(root, "tb4", TB4_REF) + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "tb4").resolve() + pull_dataset("tb4") + pulled = Manifest.read(destination) + placed = json.loads((destination / LAYER_RECORD_FILE).read_text()) + assert placed == {"dataset.toml": hashlib.sha256((entry / "dataset.toml").read_bytes()).hexdigest()} + edited = f'[gym]\nsource = "{TB4_REF}"\n[gym.tasks."alpha".environment]\nenv = {{ CIRCLE_NODE_TOTAL = "7" }}\n' + (destination / "dataset.toml").write_text(edited) + monkeypatch.delenv("NEMO_GYM_DATASETS_DIR") + capsys.readouterr() + prepared = prepare_target(str(destination), output_root=tmp_path / "out") + out = capsys.readouterr().out + absorbed = Manifest.read(destination) + # Without the variable the folder is a plain path: the run reads the edit and the manifest records it. + assert prepared.tasks[0].env == {"CIRCLE_NODE_TOTAL": "7"} and "Rewrote" in out + assert ( + absorbed.dataset_toml_digest == hashlib.sha256(edited.encode()).hexdigest() != pulled.dataset_toml_digest + ) + assert absorbed.tasks == pulled.tasks and absorbed.source == "harbor-package-store" + # The record of the placed copies is untouched by that prepare, so with the variable the edit is still an edit. + assert json.loads((destination / LAYER_RECORD_FILE).read_text()) == placed + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + refusal = ( + f"{destination / 'dataset.toml'} was edited in place and differs from the entry's {entry / 'dataset.toml'}; " + "edit the entry's file instead, or move the copy aside and run again" + ) + with pytest.raises(HubError) as exc: + pull_dataset("tb4") + assert str(exc.value) == refusal + for target in ("tb4", str(destination)): + with pytest.raises(HubError) as exc: + prepare_target(target, output_root=tmp_path / "out") + assert str(exc.value) == refusal, target + err = main_error(monkeypatch, capsys, "dataset", "pull", "tb4") + assert refusal in err and "Traceback" not in err + assert (destination / "dataset.toml").read_text() == edited and Manifest.read(destination) == absorbed + # Moved aside, the pull places the entry's file again, the manifest follows it back, and nothing was fetched. + (destination / "dataset.toml").rename(tmp_path / "override-aside") + run_main(monkeypatch, "dataset", "pull", "tb4") + out = capsys.readouterr().out + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + assert Manifest.read(destination) == pulled and f"dataset digest {pulled.digest}" in out + assert f"dataset digest {absorbed.digest[:12]} -> {pulled.digest[:12]}" in out + assert (tmp_path / "override-aside").read_text() == edited + + def test_a_local_dataset_at_the_entry_destination_is_refused_not_overwritten( + self, root, tmp_path, monkeypatch, capsys + ): + """A dataset the user placed at /tb4 by hand and ran by its path (before the variable moved the + entry's pull there, or in a shell without it) holds a `local` manifest hashing the user's own dataset.toml. + That manifest is nobody's record of copies: by path, by name and by pull alike the folder is refused as it is, + not overwritten with the entry's file (round 32, second look).""" + entry = write_entry(root, "tb4", TB4_REF) + folder = (tmp_path / "shared" / "tb4").resolve() + write_task(folder / "mine") + own = '[gym]\n[gym.tasks."mine".environment]\nenv = { CIRCLE_NODE_TOTAL = "7" }\n' + (folder / "dataset.toml").write_text(own) + monkeypatch.delenv("NEMO_GYM_DATASETS_DIR") + before = prepare_target(str(folder), output_root=tmp_path / "out") + recorded = Manifest.read(folder) + assert recorded.source == "local" and before.tasks[0].env == {"CIRCLE_NODE_TOTAL": "7"} + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + assert entry_at(folder) == find_entry("tb4") + capsys.readouterr() + refusal = ( + f"{folder} is where entry tb4's pull lands under NEMO_GYM_DATASETS_DIR, but its manifest.toml records a " + f"local dataset, not a fetch, and {folder / 'dataset.toml'} differs from the entry's files under {entry}; " + "it is not the entry's pull: move the folder aside, or run it by its path with NEMO_GYM_DATASETS_DIR unset" + ) + for target in (str(folder), "tb4"): + with pytest.raises(HubError) as exc: + prepare_target(target, output_root=tmp_path / "out") + assert str(exc.value) == refusal, target + with pytest.raises(HubError) as exc: + pull_dataset("tb4") + assert str(exc.value) == refusal + err = main_error(monkeypatch, capsys, "dataset", "validate", str(folder)) + assert refusal in err and "Traceback" not in err + # The user's file and its record are untouched, and nothing was announced as rewritten. + assert (folder / "dataset.toml").read_text() == own and Manifest.read(folder) == recorded + assert "Rewrote" not in capsys.readouterr().out + # Moved aside, the entry's destination is free: the pull places the entry's table and fetches into it. + folder.rename(tmp_path / "mine-aside") + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + pull_dataset("tb4") + assert (folder / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + assert Manifest.read(folder).source == "harbor-package-store" and (folder / "alpha" / "task.toml").is_file() + assert (tmp_path / "mine-aside" / "dataset.toml").read_text() == own + + def test_an_earlier_hub_folder_beside_the_entry_is_named_and_left_alone(self, root, tmp_path, monkeypatch, capsys): + """A checkout that fetched the reference before the entry declared it holds `terminal-bench-4.0.0/`; the + reference now lands in the entry, and the pull, and a run of the reference, say so (finding of round 24).""" + folders = {"alpha": write_task(root / "src" / "alpha")} + fake_store(root, monkeypatch, folders) + folder = write_entry(root, "tb4", TB4_REF) + old = (root / "environments" / "harbor" / "terminal-bench-4.0.0").resolve() + write_task(old / "alpha") + (old / "alpha" / "instruction.md").write_text("the old fetch\n") + note = ( + f"Note: {old} holds an earlier fetch of {TB4_REF}; fetches of it now land in {folder} and nothing reads " + "the old folder, so it can be moved aside or deleted" + ) + + run_main(monkeypatch, "dataset", "pull", TB4_REF) + + out = capsys.readouterr().out.splitlines() + assert out[0] == f"Pulled {TB4_REF} into {folder} (entry tb4)" and out[2] == note + assert out[3] == "Next: gym dataset validate tb4 --sandbox " + assert (old / "alpha" / "instruction.md").read_text() == "the old fetch\n" + prepare_target(TB4_REF, output_root=tmp_path / "out") + assert note in capsys.readouterr().out + # Once the old folder is moved aside the note is gone. + old.rename(tmp_path / "moved-aside") + run_main(monkeypatch, "dataset", "pull", "tb4") + assert "Note:" not in capsys.readouterr().out + + def test_an_edited_task_folder_is_refused_as_a_fetch_inside_eval_run_refuses_it(self, root, monkeypatch, capsys): + folders = {"alpha": write_task(root / "src" / "alpha")} + fake_store(root, monkeypatch, folders) + folder = write_entry(root, "tb4", TB4_REF) + run_main(monkeypatch, "dataset", "pull", "tb4") + capsys.readouterr() + (folder / "alpha" / "instruction.md").write_text("edited by hand\n") + + err = main_error(monkeypatch, capsys, "dataset", "pull", "tb4") + + assert f"{folder / 'alpha'} differs from the store" in err and "Traceback" not in err + assert (folder / "alpha" / "instruction.md").read_text() == "edited by hand\n" # never discarded + # `gym eval run` on the same reference stops in the same words, before anything is prepared. + err = main_error(monkeypatch, capsys, "eval", "run", TB4_REF, "--agent", "hermes_agent") + assert f"{folder / 'alpha'} differs from the store" in err + + def test_a_foreign_folder_is_refused(self, root, monkeypatch, capsys): + folders = {"alpha": write_task(root / "src" / "alpha")} + fake_store(root, monkeypatch, folders) + folder = write_entry(root, "tb4", TB4_REF) + write_task(folder / "stray") + (folder / "manifest.toml").write_text( + '[dataset]\nsource = "harbor-registry"\ndigest = "x"\n[tasks]\n[tasks.stray]\ndigest = "x"\n' + ) + err = main_error(monkeypatch, capsys, "dataset", "pull", "tb4") + assert "was written by a harbor-registry fetch, not a package-store fetch" in err + assert (folder / "stray" / "task.toml").is_file() + + @pytest.mark.parametrize( + ("argv", "expected"), + [ + (("nonsense",), "'nonsense' is neither a `harbor:` reference nor a committed entry"), + (("harbor:bad ref",), "Malformed Harbor hub reference"), + (("swe",), "declares no [gym] source, so there is nothing to pull for entry swe"), + (("tb4", "+num_repeats=2"), "takes no configuration overrides: +num_repeats=2"), + ], + ) + def test_refusals_are_one_line_parser_errors(self, root, monkeypatch, capsys, argv, expected): + write_entry(root, "tb4", TB4_REF) + write_entry(root, "swe", None) + err = main_error(monkeypatch, capsys, "dataset", "pull", *argv) + assert expected in err and "Traceback" not in err + + def test_pulled_dataset_report(self, tmp_path): + manifest = Manifest(source="harbor-package-store", digest="d" * 64, dataset_toml_digest="") + plan = entries_module.PullPlan(ref=TB4_REF, entry=None) + pulled = PulledDataset(TB4_REF, plan, tmp_path / "a b", manifest, unchanged=False) + assert pulled.task_count == 0 and pulled.entry is None + assert pulled.text().splitlines()[2] == f"Next: gym dataset validate '{tmp_path / 'a b'}' --sandbox " + assert pull_dataset.__doc__ # the function the command and prepare share + + def test_a_registry_entry_under_a_moved_datasets_dir_follows_two_successive_table_edits( + self, root, tmp_path, monkeypatch, registry_repo, capsys + ): # noqa: F811 + """A registry fetch with every folder in place rewrites no manifest, so the record of the copy beside the tasks + is the pull's own to keep: two edits of the entry's file with a pull between are both taken as such, each pull + reports the digest the table now has, and the task record is never touched (round 2).""" + dataset = registry_dataset(registry_repo) + monkeypatch.setattr(hub_module, "load_registry", lambda cache_dir, *, refresh=False: [dataset]) + entry = write_entry(root, "ds_entry", "harbor:ds@1.0") + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "ds_entry").resolve() + run_main(monkeypatch, "dataset", "pull", "ds_entry") + first = Manifest.read(destination) + digests = [] + for verified in ("true", "false"): + (entry / "dataset.toml").write_text(f'[gym]\nsource = "harbor:ds@1.0"\nverified = {verified}\n') + capsys.readouterr() + run_main(monkeypatch, "dataset", "pull", "ds_entry") + out = capsys.readouterr().out + manifest = Manifest.read(destination) + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + assert manifest.dataset_toml_digest == dataset_toml_digest(entry) + assert manifest.tasks == first.tasks and manifest.source == "harbor-registry" + assert f"dataset digest {manifest.digest}" in out and "unchanged" not in out + digests.append(manifest.digest) + assert len({first.digest, *digests}) == 3 + run_main(monkeypatch, "dataset", "pull", "ds_entry") + assert "unchanged since the last pull" in capsys.readouterr().out + + def test_a_registry_entry_declaration_is_checked_against_the_recorded_version( + self, root, tmp_path, monkeypatch, registry_repo, capsys + ): # noqa: F811 + """The registry fetch records `harbor:@`, the version it resolved: an unpinned declaration + matches whatever version the record holds (the registry's pick, which only a pull revises), a pinned one + must match it, and a pull under a new pin that lists the same tasks re-records the reference without cloning + and without announcing a digest change that did not happen (round 32).""" + v10 = registry_dataset(registry_repo) + v11 = RegistryDataset("ds", "1.1", "", v10.tasks) + monkeypatch.setattr(hub_module, "load_registry", lambda cache_dir, *, refresh=False: [v10, v11]) + entry = write_entry(root, "ds_entry", "harbor:ds@1.0") + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "ds_entry").resolve() + pull_dataset("ds_entry") + first = Manifest.read(destination) + assert first.ref == "harbor:ds@1.0" + assert prepare_target("ds_entry", output_root=tmp_path / "out").dataset_source == "harbor-registry" + # Unpinned: the recorded version is the registry's pick, so the name resolves without a pull. + (entry / "dataset.toml").write_text('[gym]\nsource = "harbor:ds"\n') + assert prepare_target("ds_entry", output_root=tmp_path / "out").dataset_source == "harbor-registry" + assert Manifest.read(destination).ref == "harbor:ds@1.0" + # Pinned to another version: refused until the pull, which re-records it (same pins, nothing cloned). + (entry / "dataset.toml").write_text('[gym]\nsource = "harbor:ds@1.1"\n') + with pytest.raises(HarborTaskError, match=r"records a fetch of harbor:ds@1\.0, not of harbor:ds@1\.1"): + prepare_target("ds_entry", output_root=tmp_path / "out") + git_calls = [] + original_git = hub_module._git + monkeypatch.setattr( + hub_module, "_git", lambda *args, **kwargs: (git_calls.append(args), original_git(*args, **kwargs))[1] + ) + capsys.readouterr() # the refused prepare above already refreshed the copy of the table, and said so + run_main(monkeypatch, "dataset", "pull", "ds_entry") + out = capsys.readouterr().out + second = Manifest.read(destination) + # Pinned at HEAD, the version change asks the remote where it points (the recorded commit) and clones nothing. + assert second.ref == "harbor:ds@1.1" and second.tasks == first.tasks + assert [args[0] for args in git_calls] == ["ls-remote"] + # The re-record moved no digest, so the pull announces no rewrite; it reports the reference and the digest. + assert out.splitlines()[0] == f"Pulled harbor:ds@1.1 into {destination} (entry ds_entry)" + assert "Rewrote" not in out and f"dataset digest {second.digest}" in out and "unchanged" not in out + assert prepare_target("ds_entry", output_root=tmp_path / "out").dataset_digest == second.digest + assert "Rewrote" not in capsys.readouterr().out + + @pytest.mark.parametrize("declared", ["harbor:ds@1.1", "harbor:ds"], ids=["pinned", "unpinned"]) + def test_a_pull_of_a_version_that_drops_a_task_is_refused_with_the_folder_named( + self, root, tmp_path, monkeypatch, registry_repo, capsys, declared + ): # noqa: F811 + """The refusal by name promises that the pull refuses a folder holding another version's tasks, with what to + move aside. The registry fetch used to keep that promise only when the new version added tasks: one that + dropped a task tolerated the task's folder (a previous fetch had pinned it), dropped its pin, reported success + and left the folder unrecorded, so the next run by name was refused as edited by hand (round 32, second look). + Now the pull names the folder before anything is cloned or rewritten, whether the entry pins the version or + `harbor:` resolves to it; moved aside, the pull re-records the version without cloning.""" + + def task(name: str) -> RegistryTask: + return RegistryTask(name, registry_repo.as_uri(), "HEAD", f"tasks/{name}") + + v10 = RegistryDataset("ds", "1.0", "", (task("a"), task("b"))) + v11 = RegistryDataset("ds", "1.1", "", (task("a"),)) + registry = [v10] + monkeypatch.setattr(hub_module, "load_registry", lambda cache_dir, *, refresh=False: list(registry)) + entry = write_entry(root, "ds_entry", "harbor:ds@1.0") + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "ds_entry").resolve() + pull_dataset("ds_entry") + first = Manifest.read(destination) + assert sorted(first.tasks) == ["a", "b"] and first.ref == "harbor:ds@1.0" + registry.append(v11) + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{declared}"\n') + if declared == "harbor:ds@1.1": + with pytest.raises(HarborTaskError, match=r"records a fetch of harbor:ds@1\.0, not of harbor:ds@1\.1"): + prepare_target("ds_entry", output_root=tmp_path / "out") + else: + # Unpinned, the recorded version is the registry's pick until a pull revises it (checked above). + assert prepare_target("ds_entry", output_root=tmp_path / "out").dataset_source == "harbor-registry" + git_calls = [] + original_git = hub_module._git + monkeypatch.setattr( + hub_module, "_git", lambda *args, **kwargs: (git_calls.append(args), original_git(*args, **kwargs))[1] + ) + capsys.readouterr() + refusal = ( + f"Task folder(s) b in {destination} were fetched for another version of ds: a previous fetch pinned them " + "and harbor:ds@1.1 does not list them; move them aside and run again to fetch harbor:ds@1.1" + ) + with pytest.raises(HubError) as exc: + pull_dataset("ds_entry") + assert str(exc.value) == refusal + err = main_error(monkeypatch, capsys, "dataset", "pull", "ds_entry") + assert refusal in err and "Traceback" not in err + # Nothing cloned, no pin dropped: the task record still says 1.0 with both tasks, and both are on disk (the + # copy of the entry's table, whose source line changed, was refreshed beside them as on any pull). + kept = Manifest.read(destination) + assert "clone" not in [args[0] for args in git_calls] and (kept.ref, kept.tasks) == (first.ref, first.tasks) + assert (destination / "a" / "task.toml").is_file() and (destination / "b" / "task.toml").is_file() + # Moved aside, the pull re-records the version it resolved without cloning, and the entry runs `a` alone. + (destination / "b").rename(tmp_path / "b-aside") + run_main(monkeypatch, "dataset", "pull", "ds_entry") + out = capsys.readouterr().out + second = Manifest.read(destination) + assert ( + second.ref == "harbor:ds@1.1" and sorted(second.tasks) == ["a"] and second.tasks["a"] == first.tasks["a"] + ) + assert "clone" not in [args[0] for args in git_calls] + assert out.splitlines()[0] == f"Pulled {declared} into {destination} (entry ds_entry)" + assert "1 task(s), dataset digest" in out + prepared = prepare_target("ds_entry", output_root=tmp_path / "out") + assert [t.path.name for t in prepared.tasks] == ["a"] and prepared.dataset_digest == second.digest + assert (tmp_path / "b-aside" / "task.toml").is_file() + if declared != "harbor:ds@1.1": + return + # The other direction, back to the version that lists `b`: the pull fetches it and records it, and the entry + # runs both; a folder holding another version's tasks is only ever refused when it would go unrecorded. + (entry / "dataset.toml").write_text('[gym]\nsource = "harbor:ds@1.0"\n') + with pytest.raises(HarborTaskError, match=r"records a fetch of harbor:ds@1\.1, not of harbor:ds@1\.0"): + prepare_target("ds_entry", output_root=tmp_path / "out") + pull_dataset("ds_entry") + third = Manifest.read(destination) + assert third.ref == "harbor:ds@1.0" and sorted(third.tasks) == ["a", "b"] and third.tasks == first.tasks + assert any(args[0] == "clone" for args in git_calls) + assert [t.path.name for t in prepare_target("ds_entry", output_root=tmp_path / "out").tasks] == ["a", "b"] + + @pytest.mark.parametrize( + "declared", ["harbor:ds@1.1", "harbor:ds", "harbor:ds@1.0"], ids=["pinned", "unpinned", "revised-in-place"] + ) + def test_a_pull_of_a_version_that_pins_a_task_at_another_commit_is_refused_with_the_folder_named( + self, root, tmp_path, monkeypatch, registry_repo, capsys, declared + ): # noqa: F811 + """The other direction of the promise above: a version that lists the same task, pinned at another commit. + The registry fetch kept a folder whenever the recorded pin named the registry's repository and path at some + commit, without comparing that commit with the one the new version pins, so a pull of `harbor:ds@1.1` kept + the folder fetched for 1.0, cloned nothing and recorded the 1.0 content as a fetch of 1.1, which the run by + name then ran (round 32, third look). Harbor caches a git task by its commit and downloads one pinned at + another commit afresh (GitTaskId). Now the pull names the folder with both commits before anything is cloned + or rewritten, whether the entry pins the version, `harbor:` resolves to it, or the registry revised + the declared version in place; moved aside, the pull clones the commit the registry pins (not the recorded + one, which used to be "repaired") and records it.""" + sha1 = subprocess.check_output(["git", "-C", str(registry_repo), "rev-parse", "HEAD"], text=True).strip() + (registry_repo / "tasks" / "a" / "instruction.md").write_text("Create a file called bye.txt.\n") + sha2 = commit_repo(registry_repo) + + def version(version: str, commit: str) -> RegistryDataset: + return RegistryDataset("ds", version, "", (RegistryTask("a", registry_repo.as_uri(), commit, "tasks/a"),)) + + registry = [version("1.0", sha1)] + monkeypatch.setattr(hub_module, "load_registry", lambda cache_dir, *, refresh=False: list(registry)) + entry = write_entry(root, "ds_entry", "harbor:ds@1.0") + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "ds_entry").resolve() + pull_dataset("ds_entry") + first = Manifest.read(destination) + assert first.ref == "harbor:ds@1.0" and first.pins["a"]["git_commit_id"] == sha1 + assert (destination / "a" / "instruction.md").read_text() == "Create a file called hello.txt.\n" + if declared == "harbor:ds@1.0": + registry[:] = [version("1.0", sha2)] # the registry revised the declared version in place + recorded_ref = "harbor:ds@1.0" + else: + registry.append(version("1.1", sha2)) + recorded_ref = "harbor:ds@1.1" + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{declared}"\n') + if declared == "harbor:ds@1.1": + with pytest.raises(HarborTaskError, match=r"records a fetch of harbor:ds@1\.0, not of harbor:ds@1\.1"): + prepare_target("ds_entry", output_root=tmp_path / "out") + else: + # The record names the declared dataset (and version): the run by name cannot know the registry moved. + assert prepare_target("ds_entry", output_root=tmp_path / "out").dataset_source == "harbor-registry" + git_calls = [] + original_git = hub_module._git + monkeypatch.setattr( + hub_module, "_git", lambda *args, **kwargs: (git_calls.append(args), original_git(*args, **kwargs))[1] + ) + capsys.readouterr() + refusal = ( + f"Task folder(s) a in {destination} were fetched for another version of ds: a previous fetch pinned a at " + f"{sha1[:12]} and {recorded_ref} pins a at {sha2[:12]}; move them aside and run again to fetch " + f"{recorded_ref}" + ) + with pytest.raises(HubError) as exc: + pull_dataset("ds_entry") + assert str(exc.value) == refusal + err = main_error(monkeypatch, capsys, "dataset", "pull", "ds_entry") + assert refusal in err and "Traceback" not in err + # Nothing cloned, nothing re-recorded: the record still says 1.0 at the first commit, and its content is there. + kept = Manifest.read(destination) + assert git_calls == [] and (kept.ref, kept.tasks, kept.pins) == (first.ref, first.tasks, first.pins) + assert (destination / "a" / "instruction.md").read_text() == "Create a file called hello.txt.\n" + # Moved aside, the pull clones the commit the registry pins and records it; the entry runs that content. + (destination / "a").rename(tmp_path / "a-aside") + run_main(monkeypatch, "dataset", "pull", "ds_entry") + out = capsys.readouterr().out + second = Manifest.read(destination) + assert second.ref == recorded_ref and second.pins["a"]["git_commit_id"] == sha2 + assert second.tasks["a"]["digest"] != first.tasks["a"]["digest"] + assert second.tasks["a"]["digest"] == content_hash(destination / "a") + assert [args[0] for args in git_calls].count("clone") == 1 and ("checkout", "--quiet", sha2) == git_calls[ + [args[0] for args in git_calls].index("checkout") + ][:3] + assert out.splitlines()[0] == f"Pulled {declared} into {destination} (entry ds_entry)" + assert (destination / "a" / "instruction.md").read_text() == "Create a file called bye.txt.\n" + prepared = prepare_target("ds_entry", output_root=tmp_path / "out") + assert [t.path.name for t in prepared.tasks] == ["a"] and prepared.dataset_digest == second.digest + assert (tmp_path / "a-aside" / "instruction.md").read_text() == "Create a file called hello.txt.\n" + + @pytest.mark.parametrize("declared", ["harbor:ds@1.1", "harbor:ds"], ids=["pinned", "unpinned"]) + def test_a_pull_of_another_version_that_pins_a_task_at_head_resolves_it_afresh( + self, root, tmp_path, monkeypatch, registry_repo, capsys, declared + ): # noqa: F811 + """A registry pin that is not a commit (`HEAD`, a branch) is kept as the recorded fetch resolved it while the + reference stays the same: a re-pull follows no new HEAD. A pull of another version into the entry's folder + (the entry's source moved, or `harbor:` resolves to it) kept that pin too and recorded the previous + version's content as the new version's, where a fetch into a folder of its own, as each version had before + entries, resolved HEAD then (Harbor never serves a task without a resolved commit from its cache, + harbor.tasks.client). Now a version change resolves the pin afresh: a folder at another commit is refused + with both commits named, moved aside it is cloned at the commit HEAD points at now, and HEAD unmoved the + version is re-recorded without cloning.""" + sha1 = subprocess.check_output(["git", "-C", str(registry_repo), "rev-parse", "HEAD"], text=True).strip() + + def version(version: str) -> RegistryDataset: + return RegistryDataset("ds", version, "", (RegistryTask("a", registry_repo.as_uri(), "HEAD", "tasks/a"),)) + + registry = [version("1.0")] + monkeypatch.setattr(hub_module, "load_registry", lambda cache_dir, *, refresh=False: list(registry)) + entry = write_entry(root, "ds_entry", "harbor:ds@1.0") + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "ds_entry").resolve() + pull_dataset("ds_entry") + first = Manifest.read(destination) + assert first.ref == "harbor:ds@1.0" and first.pins["a"]["git_commit_id"] == sha1 + (registry_repo / "tasks" / "a" / "instruction.md").write_text("Create a file called bye.txt.\n") + sha2 = commit_repo(registry_repo) + git_calls = [] + original_git = hub_module._git + monkeypatch.setattr( + hub_module, "_git", lambda *args, **kwargs: (git_calls.append(args), original_git(*args, **kwargs))[1] + ) + # The same version again: the recorded pin stands and the remote is not asked. + pull_dataset("ds_entry") + assert Manifest.read(destination).pins["a"]["git_commit_id"] == sha1 and git_calls == [] + registry.append(version("1.1")) + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{declared}"\n') + capsys.readouterr() + refusal = ( + f"Task folder(s) a in {destination} were fetched for another version of ds: a previous fetch pinned a at " + f"{sha1[:12]} and harbor:ds@1.1 pins a at {sha2[:12]}; move them aside and run again to fetch harbor:ds@1.1" + ) + with pytest.raises(HubError) as exc: + pull_dataset("ds_entry") + assert str(exc.value) == refusal + # The remote was asked where HEAD points, once, and nothing was cloned or re-recorded. + assert [args[0] for args in git_calls] == ["ls-remote"] + kept = Manifest.read(destination) + assert (kept.ref, kept.tasks, kept.pins) == (first.ref, first.tasks, first.pins) + assert (destination / "a" / "instruction.md").read_text() == "Create a file called hello.txt.\n" + # Moved aside, the pull clones the commit HEAD points at now and records it as this version's. + capsys.readouterr() # the refused pull already refreshed the copy of the table, and said so + (destination / "a").rename(tmp_path / "a-aside") + run_main(monkeypatch, "dataset", "pull", "ds_entry") + out = capsys.readouterr().out + second = Manifest.read(destination) + assert second.ref == "harbor:ds@1.1" and second.pins["a"]["git_commit_id"] == sha2 + assert second.tasks["a"]["digest"] == content_hash(destination / "a") != first.tasks["a"]["digest"] + assert (destination / "a" / "instruction.md").read_text() == "Create a file called bye.txt.\n" + assert out.splitlines()[0] == f"Pulled {declared} into {destination} (entry ds_entry)" + prepared = prepare_target("ds_entry", output_root=tmp_path / "out") + assert [t.path.name for t in prepared.tasks] == ["a"] and prepared.dataset_digest == second.digest + # HEAD unmoved, a further version change asks the remote, finds the recorded commit and re-records the + # version without cloning. + registry.append(version("1.2")) + (entry / "dataset.toml").write_text('[gym]\nsource = "harbor:ds@1.2"\n') + del git_calls[:] + pull_dataset("ds_entry") + third = Manifest.read(destination) + assert third.ref == "harbor:ds@1.2" and third.pins == second.pins and third.tasks == second.tasks + assert [args[0] for args in git_calls] == ["ls-remote"] + + def test_an_entry_table_edited_around_a_failed_fetch_is_not_an_in_place_edit( + self, root, tmp_path, monkeypatch, capsys + ): + """The copy is refreshed before the fetch; a fetch that then fails (store unreachable) rewrites nothing, and the + next edit of the entry's file is still taken as such (round 2). A first fetch that fails leaves only the copy + behind, no manifest, and that copy follows the entry's file too.""" + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + entry = write_entry(root, "tb4", TB4_REF) + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "tb4").resolve() + real = harbor_cli.fetch_ref + + def unreachable(*args, **kwargs): + raise HubError("store unreachable") + + def table(verified: str) -> None: + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{TB4_REF}"\nverified = {verified}\n') + + monkeypatch.setattr(harbor_cli, "fetch_ref", unreachable) + err = main_error(monkeypatch, capsys, "dataset", "pull", "tb4") + assert err.rstrip().endswith("error: store unreachable") # no entry hint: the message names no folder + assert (destination / "dataset.toml").is_file() and Manifest.read(destination) is None + table("true") + monkeypatch.setattr(harbor_cli, "fetch_ref", real) + run_main(monkeypatch, "dataset", "pull", "tb4") + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + assert (destination / "alpha" / "task.toml").is_file() + table("false") + monkeypatch.setattr(harbor_cli, "fetch_ref", unreachable) + main_error(monkeypatch, capsys, "dataset", "pull", "tb4") + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + table("true") + monkeypatch.setattr(harbor_cli, "fetch_ref", real) + run_main(monkeypatch, "dataset", "pull", "tb4") + out = capsys.readouterr().out + manifest = Manifest.read(destination) + assert "unchanged" not in out and f"dataset digest {manifest.digest}" in out + assert manifest.dataset_toml_digest == dataset_toml_digest(entry) + + def test_an_entry_table_fixed_after_a_refused_run_is_not_an_in_place_edit( + self, root, tmp_path, monkeypatch, capsys + ): + """`gym eval run ` refreshes the copy, then refuses for the agent before its manifest check rewrites + anything; the fix of the entry's table that follows pulls and prepares (round 2).""" + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + entry = write_entry(root, "tb4", TB4_REF) + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + destination = (tmp_path / "shared" / "tb4").resolve() + run_main(monkeypatch, "dataset", "pull", "tb4") + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{TB4_REF}"\nallowed_agents = ["other_agent"]\n') + err = main_error(monkeypatch, capsys, "eval", "run", "tb4", "--agent", "hermes_agent") + assert "allowed_agents admits other_agent" in err + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{TB4_REF}"\nallowed_agents = ["hermes_agent"]\n') + run_main(monkeypatch, "dataset", "pull", "tb4") + out = capsys.readouterr().out + assert "unchanged" not in out and f"dataset digest {Manifest.read(destination).digest}" in out + assert (destination / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + prepared = prepare_target("tb4", output_root=tmp_path / "out") + assert prepared.dataset_digest == Manifest.read(destination).digest + assert "Rewrote" not in capsys.readouterr().out + + def test_a_registry_entry_pulled_in_place_reports_the_digest_of_the_edited_table( + self, root, monkeypatch, registry_repo, capsys + ): # noqa: F811 + """A registry fetch with every folder in place writes nothing, so after an edit of the entry's dataset.toml the + pull brings the manifest's Gym layer up to date itself: it reports the new digest, not "unchanged", and the + prepare that follows finds nothing to rewrite (round 2).""" + dataset = registry_dataset(registry_repo) + monkeypatch.setattr(hub_module, "load_registry", lambda cache_dir, *, refresh=False: [dataset]) + folder = write_entry(root, "ds_entry", "harbor:ds@1.0") + run_main(monkeypatch, "dataset", "pull", "ds_entry") + first = Manifest.read(folder) + (folder / "dataset.toml").write_text('[gym]\nsource = "harbor:ds@1.0"\nverified = true\n') + capsys.readouterr() + run_main(monkeypatch, "dataset", "pull", "ds_entry") + out = capsys.readouterr().out + second = Manifest.read(folder) + assert second.digest != first.digest and second.tasks == first.tasks + assert second.dataset_toml_digest == dataset_toml_digest(folder) + assert f"dataset digest {second.digest}" in out and "unchanged" not in out + # The identity change is announced by the pull, in the manifest check's words (round 3). + assert out.splitlines()[0] == ( + f"Rewrote {folder / 'manifest.toml'}: dataset.toml changed; dataset digest " + f"{first.digest[:12]} -> {second.digest[:12]}" + ) + prepared = prepare_target("ds_entry", output_root=root / "out") + assert prepared.dataset_digest == second.digest and "Rewrote" not in capsys.readouterr().out + + def test_a_pull_whose_fetch_rewrote_the_gym_layer_announces_the_digest_change( + self, root, tmp_path, monkeypatch, capsys + ): + """The package-store fetch rewrites the manifest without a word when every folder is in place; a dataset.toml + written by hand into a hub folder (no entry) changed the dataset's identity, and the pull, and a run of the + reference, say so as the manifest check used to (round 3).""" + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + run_main(monkeypatch, "dataset", "pull", TB4_REF) + folder = (root / "environments" / "harbor" / "terminal-bench-4.0.0").resolve() + before = Manifest.read(folder) + (folder / "dataset.toml").write_text('[gym.tasks."alpha".environment]\nenv = { X = "1" }\n') + capsys.readouterr() + run_main(monkeypatch, "dataset", "pull", TB4_REF) + out = capsys.readouterr().out.splitlines() + after = Manifest.read(folder) + assert after.digest != before.digest and after.tasks == before.tasks + assert out[0] == ( + f"Rewrote {folder / 'manifest.toml'}: dataset.toml changed; dataset digest " + f"{before.digest[:12]} -> {after.digest[:12]}" + ) + assert out[2] == f"1 task(s), dataset digest {after.digest} (harbor-package-store)" + (folder / "dataset.toml").write_text('[gym.tasks."alpha".environment]\nenv = { X = "2" }\n') + prepared = prepare_target(TB4_REF, output_root=tmp_path / "out") + out = capsys.readouterr().out + assert prepared.dataset_digest != after.digest and prepared.tasks[0].env == {"X": "2"} + assert ( + out.count("Rewrote") == 1 + and f"dataset digest {after.digest[:12]} -> {prepared.dataset_digest[:12]}" in out + ) + # Unchanged since: nothing announced, nothing rewritten. + run_main(monkeypatch, "dataset", "pull", TB4_REF) + out = capsys.readouterr().out + assert "Rewrote" not in out and "unchanged since the last pull" in out + + def test_a_refusal_naming_the_entry_folder_says_what_to_delete_there(self, root, tmp_path, monkeypatch, capsys): + """The fetchers' "delete the folder, or move it aside" would delete the committed README, YAML and dataset.toml + when the folder is the entry's own; the pull, a run of the reference and the manifest check of a run by entry + name add what to delete (round 2). Under a moved datasets dir the folder holds nothing git tracks and the + hint is not added.""" + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + folder = write_entry(root, "tb4", TB4_REF) + run_main(monkeypatch, "dataset", "pull", "tb4") + hint = ( + f"{folder} is the committed entry tb4's own folder: delete, or move aside, only the pulled task folders and " + "manifest.toml in it; its README, YAML files and dataset.toml are the entry's and stay (to pull another " + "version, change `source` in that dataset.toml)" + ) + # An interrupted fetch is finished by the pull, not by deleting what it landed: that refusal names the pull to + # run again and carries no deletion hint (round 3). + manifest = Manifest.read(folder) + text = (folder / "manifest.toml").read_text() + Manifest( + source=manifest.source, + digest=manifest.digest, + dataset_toml_digest=manifest.dataset_toml_digest, + tasks={**manifest.tasks, "zzz": {"package": "terminal-bench/zzz", "content_hash": "sha256:" + "0" * 64}}, + ref=manifest.ref, + ).write(folder) + err = main_error(monkeypatch, capsys, "eval", "run", "tb4", "--agent", "hermes_agent") + assert ( + f"the fetch of {folder} was interrupted (tasks not yet recorded: zzz); run `gym dataset pull tb4` again" + in err + ) + assert err.rstrip().endswith("run `gym dataset pull tb4` again") and "delete, or move aside" not in err + (folder / "manifest.toml").write_text(text) + # The store's 4.0.0 now has another task set (or the entry's source was bumped to a tag that does). + fake_store(root, monkeypatch, {"alpha": root / "src" / "alpha", "beta": write_task(root / "src" / "beta")}) + for argv in ( + ("dataset", "pull", "tb4"), + ("dataset", "pull", TB4_REF), + ("eval", "run", TB4_REF, "--agent", "hermes_agent"), + ): + err = main_error(monkeypatch, capsys, *argv) + assert "was written for another version" in err and err.rstrip().endswith(hint), argv + assert "Traceback" not in err + # The manifest check of a run by entry name (no fetch) adds the same hint to its refusal. + (folder / "alpha" / "instruction.md").write_text("edited by hand\n") + err = main_error(monkeypatch, capsys, "eval", "run", "tb4", "--agent", "hermes_agent") + assert "does not match its manifest.toml" in err and err.rstrip().endswith(hint) + assert sorted(path.name for path in folder.iterdir()) == [ + "README.md", + "alpha", + "dataset.toml", + "manifest.toml", + "tb4.yaml", + ] + # Under a moved datasets dir the entry's folder there holds the pull alone: the fetcher's words suffice. + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + run_main(monkeypatch, "dataset", "pull", "tb4") + fake_store(root, monkeypatch, {"alpha": root / "src" / "alpha"}) + err = main_error(monkeypatch, capsys, "dataset", "pull", "tb4") + assert "was written for another version" in err and "committed entry" not in err + + def test_a_folder_of_the_entry_name_in_the_working_directory_does_not_hide_it_from_pull( + self, root, monkeypatch, capsys + ): + """The pull takes no path, so `gym dataset pull tb4` run from a folder holding a tb4/ (benchmarks/ holds a + terminal_bench_4/) still pulls the entry; validate and run keep meaning the path there (round 2).""" + fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + folder = write_entry(root, "tb4", TB4_REF) + (root / "tb4").mkdir() + assert entry_target("tb4") is None and plan_pull("tb4").folder == folder + run_main(monkeypatch, "dataset", "pull", "tb4") + assert capsys.readouterr().out.splitlines()[0] == f"Pulled {TB4_REF} into {folder} (entry tb4)" + assert (folder / "alpha" / "task.toml").is_file() and not any((root / "tb4").iterdir()) + + +@pytest.mark.usefixtures("fake_registry") +class TestEntryTargets: + """`gym dataset validate ` and `gym eval run ` resolve the name to the pulled folder, no fetch.""" + + def test_prepare_resolves_an_entry_to_its_pulled_folder(self, root, tmp_path, monkeypatch): + fake_store(root, monkeypatch, {"a": write_task(root / "src" / "a")}) + folder = write_entry(root, "tb4", TB4_REF) + pull_dataset("tb4") + prepared = prepare_target("tb4", output_root=tmp_path / "out") + assert prepared.folder == folder and prepared.taskset == "tb4" and prepared.task_source == "tb4" + assert [task.task_id for task in prepared.tasks] == ["a"] and prepared.dataset_source == "harbor-package-store" + + @pytest.mark.parametrize("moved", [False, True]) + def test_a_pull_that_lost_its_manifest_is_refused_not_run_as_local( + self, root, tmp_path, monkeypatch, capsys, moved + ): + """Task folders in a sourced entry's folder without the pull's record (the manifest deleted, as the deletion + hint's first step, or never written) are not the entry's pull: validate and run by entry name refuse them with + the pull to run, write nothing (no `local` manifest), and the pull repairs the folder (round 3).""" + _, _, calls = fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + entry = write_entry(root, "tb4", TB4_REF) + if moved: + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + folder = (tmp_path / "shared" / "tb4").resolve() if moved else entry + pull_dataset("tb4") + (folder / "manifest.toml").unlink() + downloads = sum(1 for call in calls if call[1].startswith("/storage/")) + capsys.readouterr() + refusal = ( + f"Entry tb4's folder {folder} holds task folders but no fetch record (manifest.toml), so they cannot be " + f"run as its pull of {TB4_REF}; run `gym dataset pull tb4` to fetch it" + ) + with pytest.raises(HarborTaskError) as exc: + prepare_target("tb4", output_root=tmp_path / "out") + assert str(exc.value) == refusal + for argv in (("dataset", "validate", "tb4"), ("eval", "run", "tb4", "--agent", "hermes_agent")): + err = main_error(monkeypatch, capsys, *argv) + assert refusal in err and "Traceback" not in err, argv + assert not (folder / "manifest.toml").exists() and not (tmp_path / "out").exists() + assert not (root / "results").exists() and capsys.readouterr().out == "" + # The pull records the folders that still match the store without downloading them again, and the entry runs. + run_main(monkeypatch, "dataset", "pull", "tb4") + out = capsys.readouterr().out + assert "unchanged" not in out and sum(1 for call in calls if call[1].startswith("/storage/")) == downloads + assert Manifest.read(folder).source == "harbor-package-store" + prepared = prepare_target("tb4", output_root=tmp_path / "out") + assert prepared.dataset_source == "harbor-package-store" and prepared.task_source == "tb4" + # A manifest of another source (one a run wrote before this check existed, or the registry fetcher's) is no + # record of the entry's pull either, and says what to clear. + Manifest.build(folder, source="local").write(folder) + err = main_error(monkeypatch, capsys, "dataset", "validate", "tb4") + assert ( + f"Entry tb4's folder {folder} holds task folders whose manifest.toml records source 'local', not a fetch " + f"of {TB4_REF} (harbor-package-store); delete, or move aside, the task folders and manifest.toml there " + "and run `gym dataset pull tb4`" + ) in err + assert Manifest.read(folder).source == "local" # nothing rewritten + + def test_an_entry_whose_source_moved_since_the_pull_is_refused_by_name_until_pulled( + self, root, tmp_path, monkeypatch, capsys + ): + """After a pull at @4.0.0 and a change of `source` to another tag, validate and run by name reused the 4.0.0 + tasks, since the fetch record check compared the fetcher alone (Yu He, round 32). The manifest now records the + reference the fetch was made from, the name is refused until the pull, and a pull of a reference that resolves + to the tasks already there (the same version by digest) re-records it without fetching them again.""" + _, _, calls = fake_store(root, monkeypatch, {"alpha": write_task(root / "src" / "alpha")}) + entry = write_entry(root, "tb4", TB4_REF) + pull_dataset("tb4") + recorded = Manifest.read(entry) + assert recorded.ref == TB4_REF + assert prepare_target("tb4", output_root=tmp_path / "out").dataset_source == "harbor-package-store" + downloads = sum(1 for call in calls if call[1].startswith("/storage/")) + capsys.readouterr() + + def declare(source: str) -> None: + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{source}"\n') + + other = "harbor:terminal-bench/terminal-bench@3.0.0" + declare(other) + refusal = ( + f"Entry tb4's folder {entry} holds task folders whose manifest.toml records a fetch of {TB4_REF}, not of " + f"{other}, which {entry / 'dataset.toml'} declares now; run `gym dataset pull tb4` to fetch it (a folder " + "holding another version's tasks is refused there, with what to move aside)" + ) + with pytest.raises(HarborTaskError) as exc: + prepare_target("tb4", output_root=tmp_path / "out") + assert str(exc.value) == refusal + for argv in (("dataset", "validate", "tb4"), ("eval", "run", "tb4", "--agent", "hermes_agent")): + err = main_error(monkeypatch, capsys, *argv) + assert refusal in err and "Traceback" not in err, argv + assert Manifest.read(entry) == recorded and capsys.readouterr().out == "" # nothing rewritten, nothing run + # The same version declared by digest: still another reference, so the name waits for the pull, which finds + # every folder in place, re-records the reference and downloads nothing; the bare digest reads the same. + declare("harbor:terminal-bench/terminal-bench@sha256:" + "d" * 64) + with pytest.raises(HarborTaskError, match="records a fetch of"): + prepare_target("tb4", output_root=tmp_path / "out") + run_main(monkeypatch, "dataset", "pull", "tb4") + assert Manifest.read(entry).ref == "harbor:terminal-bench/terminal-bench@sha256:" + "d" * 64 + assert sum(1 for call in calls if call[1].startswith("/storage/")) == downloads + assert prepare_target("tb4", output_root=tmp_path / "out").dataset_source == "harbor-package-store" + declare("harbor:terminal-bench/terminal-bench@" + "d" * 64) + assert prepare_target("tb4", output_root=tmp_path / "out").dataset_source == "harbor-package-store" + # A manifest written before the reference was recorded names the pull too, which records it the same way. + path = entry / "manifest.toml" + path.write_text("\n".join(line for line in path.read_text().splitlines() if not line.startswith("ref = "))) + assert Manifest.read(entry).ref is None + with pytest.raises(HarborTaskError, match="records a fetch whose reference it does not record, not of"): + prepare_target("tb4", output_root=tmp_path / "out") + run_main(monkeypatch, "dataset", "pull", "tb4") + assert Manifest.read(entry).ref == "harbor:terminal-bench/terminal-bench@sha256:" + "d" * 64 + assert sum(1 for call in calls if call[1].startswith("/storage/")) == downloads + assert prepare_target("tb4", output_root=tmp_path / "out").dataset_source == "harbor-package-store" + + @pytest.mark.parametrize( + ("declared", "recorded", "matches"), + [ + (TB4_REF, TB4_REF, True), + ("harbor:terminal-bench/terminal-bench", "harbor:terminal-bench/terminal-bench@latest", True), + ("harbor:terminal-bench/terminal-bench@latest", "harbor:terminal-bench/terminal-bench@latest", True), + ("harbor:terminal-bench/terminal-bench", TB4_REF, False), + ("harbor:terminal-bench/terminal-bench@3.0.0", TB4_REF, False), + ("harbor:other/terminal-bench@4.0.0", TB4_REF, False), + ( + "harbor:terminal-bench/terminal-bench@" + "d" * 64, + "harbor:terminal-bench/terminal-bench@sha256:" + "d" * 64, + True, + ), + ("harbor:ds", "harbor:ds@1.0", True), + ("harbor:ds@1.0", "harbor:ds@1.0", True), + ("harbor:ds@1.1", "harbor:ds@1.0", False), + ("harbor:ds", "harbor:other@1.0", False), + ("harbor:ds@1.0", "harbor:ds", False), + ("harbor:ds@1.0", TB4_REF, False), + ("harbor:ds@1.0", "not a reference", False), + ], + ) + def test_a_declaration_is_read_against_the_record_as_the_fetchers_read_it(self, declared, recorded, matches): + assert declares_recorded_fetch(declared, recorded) is matches + + def test_an_existing_folder_of_the_same_name_wins(self, root, tmp_path): + entry = write_entry(root, "tb4", TB4_REF) + write_task(entry / "a") + local = write_task(root / "tb4" / "b").parent + prepared = prepare_target("tb4", output_root=tmp_path / "out") + assert prepared.folder == local and [task.task_id for task in prepared.tasks] == ["b"] + + def test_an_unpulled_entry_names_the_pull_to_run(self, root, tmp_path, monkeypatch, capsys): + write_entry(root, "tb4", TB4_REF) + with pytest.raises( + HarborTaskError, match=r"run `gym dataset pull tb4` to fetch " + TB4_REF.replace(".", r"\.") + ): + prepare_target("tb4", output_root=tmp_path / "out") + assert not (tmp_path / "out").exists() + err = main_error(monkeypatch, capsys, "dataset", "validate", "tb4") + assert "run `gym dataset pull tb4`" in err + err = main_error(monkeypatch, capsys, "eval", "run", "tb4", "--agent", "hermes_agent") + assert "run `gym dataset pull tb4`" in err + + def test_an_entry_without_a_source_asks_for_the_task_folder(self, root, tmp_path): + write_entry(root, "swe", None) + with pytest.raises(HarborTaskError, match=r"declares no \[gym\] source .* pass the folder"): + prepare_target("swe", output_root=tmp_path / "out") + # Once its tasks are in place (by whatever means), the name resolves like any entry's. + write_task(root / "environments" / "harbor" / "swe" / "a") + assert prepare_target("swe", output_root=tmp_path / "out").taskset == "swe" + + def test_nested_task_folders_resolve_by_entry_name_as_they_do_by_path(self, root, tmp_path): + """A source-less entry laid out as group// prepares by its folder's path through the loader's recursive + discovery; the name looks for tasks the same way instead of at the folder's immediate children (round 32).""" + folder = write_entry(root, "swe", None).resolve() + write_task(folder / "group" / "alpha") + write_task(folder / "other" / "deeper" / "beta") + assert entry_folder(find_entry("swe")) == folder + by_name = prepare_target("swe", output_root=tmp_path / "out") + by_path = prepare_target(str(folder), output_root=tmp_path / "out") + assert ( + [task.task_id for task in by_name.tasks] == [task.task_id for task in by_path.tasks] == ["alpha", "beta"] + ) + assert by_name.folder == by_path.folder == folder and by_name.dataset_digest == by_path.dataset_digest + + def test_an_entry_without_a_source_stays_at_its_folder_under_a_moved_datasets_dir( + self, root, tmp_path, monkeypatch + ): + """NEMO_GYM_DATASETS_DIR moves pulls; an entry with no source has none, so its name keeps resolving to the + folder that holds its tasks, where a run by that folder's path finds them (round 32).""" + folder = write_entry(root, "swe", None).resolve() + write_task(folder / "a") + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + entry = find_entry("swe") + assert entry.destination == folder and prepare_destination(entry) == folder + assert entry_folder(entry) == folder and not (tmp_path / "shared").exists() + by_name = prepare_target("swe", output_root=tmp_path / "out") + by_path = prepare_target(str(folder), output_root=tmp_path / "out") + assert by_name.folder == by_path.folder == folder and by_name.dataset_digest == by_path.dataset_digest + # A sourced entry beside it is still moved, as every pull is. + write_entry(root, "tb4", TB4_REF) + assert find_entry("tb4").destination == (tmp_path / "shared" / "tb4").resolve() + + def test_entry_folder_follows_a_moved_datasets_dir(self, root, tmp_path, monkeypatch): + fake_store(root, monkeypatch, {"a": write_task(root / "src" / "a")}) + entry = write_entry(root, "tb4", TB4_REF) + monkeypatch.setenv("NEMO_GYM_DATASETS_DIR", str(tmp_path / "shared")) + pull_dataset("tb4") + folder = entry_folder(find_entry("tb4")) + assert folder == (tmp_path / "shared" / "tb4").resolve() and (folder / "a" / "task.toml").is_file() + # The copy of the entry's table beside the tasks follows the entry's file, as the pull keeps it. + (entry / "dataset.toml").write_text(f'[gym]\nsource = "{TB4_REF}"\nverified = true\n') + assert entry_folder(find_entry("tb4")) == folder + assert (folder / "dataset.toml").read_bytes() == (entry / "dataset.toml").read_bytes() + + def test_a_pulled_entry_that_was_edited_is_refused_at_prepare(self, root, tmp_path, monkeypatch): + """The path form and the entry form check the pull's manifest alike: an edited folder never prepares.""" + folders = {"alpha": write_task(root / "src" / "alpha")} + fake_store(root, monkeypatch, folders) + folder = write_entry(root, "tb4", TB4_REF) + pull_dataset("tb4") + assert prepare_target("tb4", output_root=tmp_path / "out").dataset_source == "harbor-package-store" + (folder / "alpha" / "instruction.md").write_text("edited by hand\n") + with pytest.raises(Exception, match="does not match its manifest.toml"): + prepare_target("tb4", output_root=tmp_path / "out") diff --git a/tests/unit_tests/test_harbor_manifest.py b/tests/unit_tests/test_harbor_manifest.py index bb35c9c22fb..d73cd1fe9c2 100644 --- a/tests/unit_tests/test_harbor_manifest.py +++ b/tests/unit_tests/test_harbor_manifest.py @@ -36,6 +36,7 @@ dataset_digest, dataset_files_digest, dataset_toml_digest, + refresh_gym_layer, task_differences, ) from nemo_gym.tasks.harbor.materialize import run_config, write_rows @@ -506,6 +507,37 @@ def test_only_the_gym_layer_moving_rewrites_the_manifest_and_keeps_pins(self, tm assert after.tasks["a"]["digest"] == before.tasks["a"]["digest"] assert "dataset.toml changed" in capsys.readouterr().out + def test_refresh_gym_layer_follows_dataset_toml_and_keeps_the_task_record(self, tmp_path, capsys): + """A pull that fetched nothing brings the manifest's Gym-layer digests up to date without hashing a task: an + edited task folder keeps its recorded digest (check_manifest, which rehashes, still refuses it), a pin without + a digest stays one, and a manifest with pins only or none at all is left to check_manifest.""" + folder = tmp_path / "ds" + write_task(folder / "a") + pins = { + name: {"git_url": "https://example.test/tasks.git", "git_commit_id": "a" * 40, "path": f"tasks/{name}"} + for name in ("a", "b") + } + before = Manifest.build(folder, source="harbor-registry", pins=pins, names=["a"]) + before.write(folder) + assert before.tasks["b"] == pins["b"] # pinned, not landed: no digest + assert refresh_gym_layer(tmp_path / "none") is None + assert refresh_gym_layer(folder) == before and (folder / MANIFEST_FILE).read_text() == before.text() + (folder / "dataset.toml").write_text('[gym.tasks."a".environment]\nenv = { X = "1" }\n') + (folder / "a" / "instruction.md").write_text("edited by hand\n") + after = refresh_gym_layer(folder) + assert after.dataset_toml_digest == dataset_toml_digest(folder) != before.dataset_toml_digest + assert after.tasks == before.tasks and after.digest != before.digest and after.source == before.source + # The rewrite is announced as check_manifest announces it (round 3): the identity changed. + assert Manifest.read(folder) == after and capsys.readouterr().out == ( + f"Rewrote {folder / MANIFEST_FILE}: dataset.toml changed; dataset digest " + f"{before.digest[:12]} -> {after.digest[:12]}\n" + ) + with pytest.raises(ManifestError, match="interrupted"): + check_manifest(folder) # the pin without a digest is still the fetch's unfinished business + (folder / MANIFEST_FILE).write_text('[dataset]\nsource = "harbor-registry"\n[tasks."a"]\npath = "tasks/a"\n') + text = (folder / MANIFEST_FILE).read_text() + assert not refresh_gym_layer(folder).records_content and (folder / MANIFEST_FILE).read_text() == text + def test_a_task_that_cannot_be_hashed_is_recorded_by_name_without_a_digest(self, tmp_path, capsys, caplog): folder = tmp_path / "ds" write_task(folder / "good") @@ -574,6 +606,11 @@ def test_a_package_store_manifest_from_before_the_dataset_digest_keeps_its_recor with pytest.raises(ManifestError, match=r"does not match its manifest.toml \(changed: b\)"): check_manifest(folder) assert (folder / MANIFEST_FILE).read_text() == legacy + # A pull that fetched nothing brings the record up to date without hashing: the edit stays caught above. + refreshed = refresh_gym_layer(folder) + assert refreshed.task_digests == previous.task_digests and refreshed.pins == previous.pins + assert "it recorded no dataset digest; dataset digest - ->" in capsys.readouterr().out + (folder / MANIFEST_FILE).write_text(legacy) # Unchanged: upgraded in place, the recorded digests and pins kept, and the next check finds it as it is. (folder / "b" / "instruction.md").write_text("Create a file called hello.txt.\n") @@ -602,6 +639,37 @@ def test_a_package_store_manifest_from_before_the_dataset_digest_keeps_its_recor @pytest.mark.usefixtures("fake_registry") # prepare records base images; never reach Docker Hub +class TestFetchReferenceRecord: + def test_the_fetch_reference_is_recorded_read_back_and_kept_by_every_rewrite(self, tmp_path, capsys): + """A fetch records the reference it was made from beside its source (round 32: what a committed entry's + declared source is checked against without the hub); it stays out of the dataset digest, a local manifest + has none, and check_manifest and refresh_gym_layer carry it through their rewrites as they carry the pins.""" + folder = tmp_path / "ds" + write_task(folder / "a") + pins = {"a": {"package": "org/a", "content_hash": "sha256:" + "a" * 64}} + ref = "harbor:org/ds@1.0.0" + fetched = Manifest.build(folder, source="harbor-package-store", pins=pins, ref=ref) + fetched.write(folder) + lines = (folder / MANIFEST_FILE).read_text().splitlines() + assert lines[2:5] == ["[dataset]", 'source = "harbor-package-store"', f'ref = "{ref}"'] + assert Manifest.read(folder) == fetched and fetched.ref == ref + assert fetched.digest == Manifest.build(folder, source="harbor-package-store", pins=pins).digest + assert check_manifest(folder) == fetched and capsys.readouterr().out == "" + (folder / "dataset.toml").write_text('[gym.tasks."a".environment]\nenv = { X = "1" }\n') + assert refresh_gym_layer(folder).ref == ref and Manifest.read(folder).ref == ref + (folder / "dataset.toml").write_text('[gym.tasks."a".environment]\nenv = { X = "2" }\n') + assert check_manifest(folder).ref == ref and Manifest.read(folder).ref == ref + assert capsys.readouterr().out.count("Rewrote") == 2 + assert Manifest.read(folder).tasks == fetched.tasks + # A local folder records none, and a value that is not a string reads as none. + local = tmp_path / "local" + write_task(local / "a") + assert check_manifest(local).ref is None and "ref = " not in (local / MANIFEST_FILE).read_text() + path = folder / MANIFEST_FILE + path.write_text(path.read_text().replace(f'ref = "{ref}"', "ref = 7")) + assert Manifest.read(folder).ref is None + + class TestPrepareRecordsIdentity: def test_rows_and_run_config_carry_the_dataset_digest(self, tmp_path): folder = tmp_path / "ds" diff --git a/tests/unit_tests/test_harbor_package_store.py b/tests/unit_tests/test_harbor_package_store.py index 5c57dd035a7..da0238f6431 100644 --- a/tests/unit_tests/test_harbor_package_store.py +++ b/tests/unit_tests/test_harbor_package_store.py @@ -139,10 +139,12 @@ def test_fetches_tags_checks_digests_and_writes_manifest(self, tmp_path, monkeyp assert sorted(p.name for p in folder.iterdir() if p.is_dir()) == ["alpha", "beta"] assert content_hash(folder / "alpha") == digests["alpha"] manifest = tomllib.loads((folder / "manifest.toml").read_text()) - # Derived facts only: no copy of the name or version, which live in dataset.toml. + # Derived facts only: no copy of the name or version, which live in dataset.toml; the reference the fetch + # was made from is recorded as the pins are (round 32). assert manifest["schema_version"] == 1 - assert set(manifest["dataset"]) == {"source", "digest", "dataset_toml_digest"} + assert set(manifest["dataset"]) == {"source", "ref", "digest", "dataset_toml_digest"} assert manifest["dataset"]["source"] == "harbor-package-store" + assert manifest["dataset"]["ref"] == "harbor:terminal-bench/terminal-bench@4.0.0" assert manifest["dataset"]["digest"] == manifest_module.dataset_digest( digests, manifest["dataset"]["dataset_toml_digest"] ) @@ -630,15 +632,20 @@ def test_fetch_ref_dispatches_package_references(self, tmp_path, monkeypatch): seen = {} - def fake_fetch(ref, root, store=None): - seen["ref"], seen["root"] = ref, root - return root / "terminal-bench-4.0.0" + def fake_fetch(ref, root, store=None, *, folder=None): + seen["ref"], seen["root"], seen["folder"] = ref, root, folder + return folder or root / "terminal-bench-4.0.0" monkeypatch.setattr(package_store, "fetch_package_dataset", fake_fetch) folder = hub.fetch_ref("harbor:terminal-bench/terminal-bench@4.0.0", tmp_path) assert folder == tmp_path / "terminal-bench-4.0.0" assert seen["ref"] == package_store.PackageRef("terminal-bench", "terminal-bench", "4.0.0") - assert seen["root"] == tmp_path + assert seen["root"] == tmp_path and seen["folder"] is None + # A caller that names the dataset folder (a committed entry's) passes it through to the store fetch. + assert hub.fetch_ref("harbor:terminal-bench/terminal-bench@4.0.0", tmp_path, folder=tmp_path / "tb4") == ( + tmp_path / "tb4" + ) + assert seen["folder"] == tmp_path / "tb4" def test_archive_is_checked_without_exclusions_but_a_noisy_folder_still_matches(self, tmp_path, monkeypatch): folders = {"alpha": write_task(tmp_path / "src" / "alpha")} @@ -771,6 +778,25 @@ def test_a_recorded_digest_vouches_only_for_the_task_version_it_pins(self, tmp_p module.fetch_package_dataset(ref, tmp_path / "datasets") assert sum(1 for c in calls if c[1].startswith("/storage/")) == 1 + @pytest.mark.parametrize( + ("target", "reference"), + [ + ("harbor:org/ds", "harbor:org/ds@latest"), + ("harbor:org/ds@latest", "harbor:org/ds@latest"), + ("harbor:org/ds@4.0.0", "harbor:org/ds@4.0.0"), + ("harbor:org/ds@sha256:" + "a" * 64, "harbor:org/ds@sha256:" + "a" * 64), + ("harbor:org/ds@" + "a" * 64, "harbor:org/ds@sha256:" + "a" * 64), + ], + ) + def test_the_recorded_reference_reads_a_reference_as_the_store_does(self, target, reference): + """What a manifest records for a package-store fetch: ``latest`` spelled out, a digest in its ``sha256:`` form, + so the same dataset declared either way reads as one reference (round 32).""" + from nemo_gym.tasks.harbor.hub import HubRef + from nemo_gym.tasks.harbor.package_store import PackageRef + + ref = PackageRef.of(HubRef.parse(target)) + assert ref.package == "org/ds" and ref.reference == reference + def test_folder_fetched_for_another_package_is_refused_by_its_task_pins(self, tmp_path, monkeypatch): """The manifest holds no copy of the dataset's name, so the per-task `package` pins (`/`) are what tell another org's dataset of the same name and tag, which lands in the same readable folder.""" @@ -780,6 +806,12 @@ def test_folder_fetched_for_another_package_is_refused_by_its_task_pins(self, tm module.PackageRef("terminal-bench", "terminal-bench", "4.0.0"), tmp_path / "datasets" ) manifest_before = (folder / "manifest.toml").read_text() + # The fetch's reference is recorded, but the check reads the pins, so a manifest written before the reference + # was recorded (stripped of it here) is told apart the same way. + recorded = 'ref = "harbor:terminal-bench/terminal-bench@4.0.0"\n' + assert recorded in manifest_before + (folder / "manifest.toml").write_text(manifest_before.replace(recorded, "")) + manifest_before = (folder / "manifest.toml").read_text() assert "terminal-bench/terminal-bench" not in manifest_before # no dataset name to compare by downloads_before = sum(1 for c in calls if c[1].startswith("/storage/")) with pytest.raises(module.PackageStoreError) as info: diff --git a/tests/unit_tests/test_harbor_tasks.py b/tests/unit_tests/test_harbor_tasks.py index 173b9ec1e85..cdc5f23f940 100644 --- a/tests/unit_tests/test_harbor_tasks.py +++ b/tests/unit_tests/test_harbor_tasks.py @@ -1345,6 +1345,7 @@ def test_fetch_dataset_pins_head_and_copies_tasks(self, tmp_path): manifest = (folder / "manifest.toml").read_text() assert f'git_commit_id = "{head}"' in manifest assert "HEAD" not in manifest.replace("hello-world", "") + assert 'ref = "harbor:hello-world@1.0"' in manifest # the reference fetched, the version resolved (round 32) assert [t.task_id for t in discover_tasks(folder)] == ["hello-world", "renamed"] # A rerun keeps existing folders and does not need the repository again. @@ -2373,6 +2374,10 @@ def test_registry_versions_keep_distinct_rows_routes_and_outputs(tmp_path, monke for version in ("1.0", "2.0"): write_task(tmp_path / f"ds-{version}" / "a") + # A fetch always leaves the folder's manifest behind; prepare reads it back for the fetch report. + manifest_module.Manifest.build(tmp_path / f"ds-{version}", source="harbor-registry").write( + tmp_path / f"ds-{version}" + ) monkeypatch.setattr(harbor_cli, "fetch_ref", lambda target, **kw: tmp_path / f"ds-{target.split('@')[1]}") first = prepare_target("harbor:ds@1.0", output_root=tmp_path / "out") first_rows = first.rows_path.read_bytes() @@ -3620,6 +3625,125 @@ def test_pulled_datasets_default_to_the_working_directory_for_a_package_install( assert Path(pulled) == (work / "environments" / "harbor").resolve() +def test_installed_entries_are_pulled_under_the_working_directory_not_the_installation(tmp_path) -> None: + """A committed entry that ships with a package install lives in site-packages, which is shared and often + read-only: with ``NEMO_GYM_DATASETS_DIR`` unset, the pull of its name and of the reference it declares both land + in ``/environments/harbor//`` (``datasets_dir()``), with the entry's ``dataset.toml`` + copied there, and nothing is created under the installation. + + The installed layout is rebuilt in a subprocess as in + :func:`test_pulled_datasets_default_to_the_working_directory_for_a_package_install`: the package and the entry + reached through a ``site/`` folder with no ``pyproject.toml`` beside it, run from a working directory elsewhere. + The plan is inspected (:func:`~nemo_gym.tasks.harbor.entries.plan_pull`) and the destination prepared; nothing + is fetched. + """ + import os + import sys + + from nemo_gym import ROOT_DIR + + ref = "harbor:terminal-bench/terminal-bench@4.0.0" + site = tmp_path / "site" + site.mkdir() + (site / "nemo_gym").symlink_to(ROOT_DIR, target_is_directory=True) + installed = site / "environments" / "harbor" / "tb4_installed" + installed.mkdir(parents=True) + table = f'[gym]\nsource = "{ref}"\n' + (installed / "dataset.toml").write_text(table) + work = tmp_path / "work" + work.mkdir() + probe = ( + "import sys\n" + f"sys.path = [{str(site)!r}, *(p for p in sys.path if '__editable__' not in p and p != '')]\n" + "sys.meta_path = [f for f in sys.meta_path if 'editable' not in getattr(f, '__module__', '').lower()]\n" + "import nemo_gym\n" + "from nemo_gym.tasks.harbor.entries import entry_target, plan_pull, prepare_destination\n" + "print(nemo_gym.PARENT_DIR)\n" + f"by_name, by_ref = plan_pull('tb4_installed'), plan_pull({ref!r})\n" + "assert by_name.entry == by_ref.entry and by_name.ref == by_ref.ref, (by_name, by_ref)\n" + "print(by_name.entry.folder)\n" + "print(by_name.folder)\n" + "print(by_ref.folder)\n" + "print(entry_target('tb4_installed').destination)\n" + "print(prepare_destination(by_name.entry))\n" + ) + env = { + key: value for key, value in os.environ.items() if key not in ("NEMO_GYM_DATASETS_DIR", "NEMO_GYM_EXTRA_ROOTS") + } + completed = subprocess.run( + [sys.executable, "-c", probe], cwd=work, env=env, capture_output=True, text=True, check=True, timeout=300 + ) + parent_dir, folder, by_name, by_ref, by_target, prepared = completed.stdout.splitlines()[-6:] + assert Path(parent_dir) == site # the probe ran the installed layout, not this checkout + assert Path(folder) == installed.resolve() # the entry is the installed one + expected = (work / "environments" / "harbor" / "tb4_installed").resolve() + assert Path(by_name) == Path(by_ref) == Path(by_target) == Path(prepared) == expected + # The destination holds a copy of the entry's table; the installation is left as it was. + assert (expected / "dataset.toml").read_text() == table + assert sorted(p.name for p in installed.iterdir()) == ["dataset.toml"] + assert sorted(p.name for p in (site / "environments" / "harbor").iterdir()) == ["tb4_installed"] + + +def test_installed_entries_in_a_project_local_venv_are_pulled_under_the_working_directory(tmp_path) -> None: + """A project-local ``.venv`` puts site-packages under the working directory, so an entry the package ships there + lies under the working root too; it is installed all the same, and with ``NEMO_GYM_DATASETS_DIR`` unset the pull + of its name and of its reference both land in ``/environments/harbor//``, with the + entry's ``dataset.toml`` copied there, and nothing is written under ``.venv`` (Yu He, round 36: both pulls + targeted the installation and failed on a read-only one). + + The installed layout is rebuilt in a subprocess as in + :func:`test_installed_entries_are_pulled_under_the_working_directory_not_the_installation`, with the ``site/`` + folder placed at ``work/.venv/lib/python3.X/site-packages`` and the probe run from ``work``. + """ + import os + import sys + + from nemo_gym import ROOT_DIR + + ref = "harbor:terminal-bench/terminal-bench@4.0.0" + work = tmp_path / "work" + site = work / ".venv" / "lib" / f"python{sys.version_info.major}.{sys.version_info.minor}" / "site-packages" + site.mkdir(parents=True) + (site / "nemo_gym").symlink_to(ROOT_DIR, target_is_directory=True) + installed = site / "environments" / "harbor" / "tb4_installed" + installed.mkdir(parents=True) + table = f'[gym]\nsource = "{ref}"\n' + (installed / "dataset.toml").write_text(table) + venv_before = sorted(str(path.relative_to(work)) for path in (work / ".venv").rglob("*")) + probe = ( + "import sys\n" + f"sys.path = [{str(site)!r}, *(p for p in sys.path if '__editable__' not in p and p != '')]\n" + "sys.meta_path = [f for f in sys.meta_path if 'editable' not in getattr(f, '__module__', '').lower()]\n" + "import nemo_gym\n" + "from nemo_gym.tasks.harbor.entries import entry_target, plan_pull, prepare_destination\n" + "print(nemo_gym.PARENT_DIR)\n" + "print(nemo_gym.WORKING_DIR)\n" + f"by_name, by_ref = plan_pull('tb4_installed'), plan_pull({ref!r})\n" + "assert by_name.entry == by_ref.entry and by_name.ref == by_ref.ref, (by_name, by_ref)\n" + "print(by_name.entry.folder)\n" + "print(by_name.folder)\n" + "print(by_ref.folder)\n" + "print(entry_target('tb4_installed').destination)\n" + "print(prepare_destination(by_name.entry))\n" + ) + env = { + key: value for key, value in os.environ.items() if key not in ("NEMO_GYM_DATASETS_DIR", "NEMO_GYM_EXTRA_ROOTS") + } + completed = subprocess.run( + [sys.executable, "-c", probe], cwd=work, env=env, capture_output=True, text=True, check=True, timeout=300 + ) + parent_dir, working_dir, folder, by_name, by_ref, by_target, prepared = completed.stdout.splitlines()[-7:] + assert Path(parent_dir) == site # the probe ran the installed layout, not this checkout + assert Path(working_dir) == work # a package install's working root is the working directory, above .venv + assert Path(folder) == installed.resolve() # the entry is the installed one + expected = (work / "environments" / "harbor" / "tb4_installed").resolve() + assert Path(by_name) == Path(by_ref) == Path(by_target) == Path(prepared) == expected + # The destination holds a copy of the entry's table; nothing under .venv was written or created. + assert (expected / "dataset.toml").read_text() == table + assert sorted(str(path.relative_to(work)) for path in (work / ".venv").rglob("*")) == venv_before + assert (installed / "dataset.toml").read_text() == table + + def test_pulled_datasets_default_to_environments_harbor_in_the_checkout(monkeypatch, tmp_path) -> None: """Fetched datasets land in environments/harbor/ of the Gym checkout unless NEMO_GYM_DATASETS_DIR says otherwise.""" from nemo_gym import PARENT_DIR