Skip to content

fix(config): aggregated error for unset '???' config values - #1575

Merged
wprazuch merged 5 commits into
mainfrom
wprazuch/config-missing-values
Jun 24, 2026
Merged

wprazuch merged 5 commits into
mainfrom
wprazuch/config-missing-values

Conversation

@wprazuch

@wprazuch wprazuch commented Jun 11, 2026 •

Copy link
Copy Markdown
Contributor

What

A required config value left unset (OmegaConf MISSING, ???) currently surfaces as an opaque
omegaconf.errors.MissingMandatoryValue — one field at a time, raised deep in parsing
(while resolving inheritance), with no guidance on how to set it.

This adds a fast-fail check in parse() that raises a single ConfigMissingValuesError listing
every unset value with a ready-to-use override example.

Before / after

# before
omegaconf.errors.MissingMandatoryValue: Missing mandatory value:
swe_agents.responses_api_agents.swe_agents.container_formatter

# after
2 required config value(s) are unset (still '???') after merging:
  - swe_agents.responses_api_agents.swe_agents.container_formatter
  - swe_agents.responses_api_agents.swe_agents.dataset_path

Provide each value via a CLI override, in env.yaml, or in a config you pass via config_paths.
For example, on the command line:
  ++swe_agents.responses_api_agents.swe_agents.container_formatter=<value>
  ++swe_agents.responses_api_agents.swe_agents.dataset_path=<value>

How

  • New ConfigMissingValuesError(ValueError) in config_types.py.
  • collect_missing_value_paths() / raise_on_missing_values() in global_config.py. The scan
    runs after all sources are merged (CLI + env.yaml + config_paths) and after
    _recursively_swap_keys — so that the _delete_key / _inherit_from / _copy directives have
    been applied first. By that point any remaining ??? is genuinely unset (not a value that is
    about to be deleted, or moved/filled by a swap), so it's reported with no false positives.
    _recursively_swap_keys itself is made missing-tolerant (items_ex(resolve=False)) so a real
    ??? doesn't trip the opaque MissingMandatoryValue before the aggregated scan reports it.
  • The walk uses OmegaConf.to_container(resolve=False, throw_on_missing=False), so it never
    raises on MISSING values or unresolved ${...} interpolations.

Scope / safety

Base configs that intentionally ship ??? (api keys, container paths, model names — ~33 files)
are unaffected: they're filled at run time via CLI/env, and no test parses a bare ??? config.
This only changes the error a user sees when they forget to supply one.

Testing

  • test_collect_missing_value_paths — nested dict + list, asserts ["a", "b.d", "e[1]"].
  • test_get_global_config_dict_raises_on_missing_values — asserts the dotted path and the
    ++...=<value> hint appear in the message.
  • pytest tests/unit_tests/test_global_config.py 28/28; related test_train_data_utils /
    test_config_types_help / test_rollout_collection 54/54. ruff clean. No new dependencies.

Part of #1205 (friction point 10). Companion to #1561 (friction 3).

@copy-pr-bot

copy-pr-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

ko3n1g pushed a commit that referenced this pull request Jun 12, 2026
…ite) (#1576)

## Problem

The **Full test suite** (`Test`) job has been red on every PR, failing 7
servers
(`math_with_code`, `newton_bench`, `arena_judge`, `reasoning_gym`,
`ether0`, `aviary`,
`stirrup_agent`) with `ModuleNotFoundError` at test collection —
`scipy`, `scikit-learn`,
`matplotlib`, `PIL`.

These deps **are** declared (and pinned) in each server's
`requirements.txt`. The real cause:

**`uv 0.11.20` (released 2026-06-10) has a resolver regression** — it
silently drops pinned
direct dependencies from `uv pip install -r requirements.txt` when the
requirements also
include an editable `-e` install (as every server's `-e nemo-gym[dev] @
../../` does). No error,
no conflict — the package is just omitted.

CI installs uv **unpinned** (`curl -LsSf https://astral.sh/uv/install.sh
| sh`), so it picked up
0.11.20 the day it released — which is exactly when the suite started
failing. The first
(passing) run used an earlier uv.

## Evidence (reproduced locally with the exact CI install command)

For `resources_servers/reasoning_gym` (`source .venv/bin/activate && uv
pip install -r requirements.txt openai==2.7.2`):

| uv version | Resolved | `matplotlib==3.10.6` |
|---|---|---|
| 0.11.19 | 154 packages | ✅ installed → `import matplotlib` OK |
| **0.11.20** | 150 packages | ❌ dropped → `ModuleNotFoundError` |

Bisected 0.10.2 → 0.11.20: every version **through 0.11.19 works**; only
**0.11.20** is broken.

## Fix

Pin the uv installer to **0.11.19** (latest known-good) in
`full-test-suite.yml` (Test + wheel
jobs) and `unit-tests.yml`, with a comment explaining why. Once uv ships
a fix, the pin can be
bumped.

Not caused by — and unblocks — any PR that triggers the full matrix
(e.g. #1561, #1575). Worth
also reporting the regression upstream to astral-sh/uv.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
wprazuch force-pushed the wprazuch/config-missing-values branch from 0ccb3cf to edc9b02 Compare June 15, 2026 13:07
@wprazuch

Copy link
Copy Markdown
Contributor Author

/ok to test edc9b02

@wprazuch

Copy link
Copy Markdown
Contributor Author

/ok to test 713dffe

Comment thread nemo_gym/global_config.py Resolved
@wprazuch

Copy link
Copy Markdown
Contributor Author

/ok to test 1168c9f

@wprazuch
wprazuch force-pushed the wprazuch/config-missing-values branch from 1168c9f to f49c6c2 Compare June 16, 2026 11:32
Comment thread tests/unit_tests/test_global_config.py Resolved
Comment thread nemo_gym/global_config.py Resolved
Comment thread tests/unit_tests/test_global_config.py Resolved
Comment thread nemo_gym/global_config.py Resolved

@ananthsub ananthsub left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly LGTM, but requesting changes to add more test coverage to ensure we dont have regressions from these changes

wprazuch added a commit that referenced this pull request Jun 17, 2026
…erage

Address review on #1575:
- _recursive_index_dict_using_path now navigates with _get_node and propagates
  MISSING for an unset referenced value, so '${copy:source.model}' where
  source.model is '???' reports the missing value instead of an opaque
  'path does not exist' error.
- Tests: _copy/_inherit_from carry '???' into the target (reported); copy of a
  missing leaf is reported not opaque; plain '${a.b.c}' interpolation still
  resolves through full parse (no regression); e2e get_global_config_dict()
  aggregates multiple missing values.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
wprazuch added a commit that referenced this pull request Jun 22, 2026
…ite) (#1576)

## Problem

The **Full test suite** (`Test`) job has been red on every PR, failing 7
servers
(`math_with_code`, `newton_bench`, `arena_judge`, `reasoning_gym`,
`ether0`, `aviary`,
`stirrup_agent`) with `ModuleNotFoundError` at test collection —
`scipy`, `scikit-learn`,
`matplotlib`, `PIL`.

These deps **are** declared (and pinned) in each server's
`requirements.txt`. The real cause:

**`uv 0.11.20` (released 2026-06-10) has a resolver regression** — it
silently drops pinned
direct dependencies from `uv pip install -r requirements.txt` when the
requirements also
include an editable `-e` install (as every server's `-e nemo-gym[dev] @
../../` does). No error,
no conflict — the package is just omitted.

CI installs uv **unpinned** (`curl -LsSf https://astral.sh/uv/install.sh
| sh`), so it picked up
0.11.20 the day it released — which is exactly when the suite started
failing. The first
(passing) run used an earlier uv.

## Evidence (reproduced locally with the exact CI install command)

For `resources_servers/reasoning_gym` (`source .venv/bin/activate && uv
pip install -r requirements.txt openai==2.7.2`):

| uv version | Resolved | `matplotlib==3.10.6` |
|---|---|---|
| 0.11.19 | 154 packages | ✅ installed → `import matplotlib` OK |
| **0.11.20** | 150 packages | ❌ dropped → `ModuleNotFoundError` |

Bisected 0.10.2 → 0.11.20: every version **through 0.11.19 works**; only
**0.11.20** is broken.

## Fix

Pin the uv installer to **0.11.19** (latest known-good) in
`full-test-suite.yml` (Test + wheel
jobs) and `unit-tests.yml`, with a comment explaining why. Once uv ships
a fix, the pin can be
bumped.

Not caused by — and unblocks — any PR that triggers the full matrix
(e.g. #1561, #1575). Worth
also reporting the regression upstream to astral-sh/uv.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
wprazuch force-pushed the wprazuch/config-missing-values branch from b73795a to b52ac3c Compare June 22, 2026 13:50
@wprazuch
wprazuch requested a review from a team as a code owner June 22, 2026 13:50
wprazuch added a commit that referenced this pull request Jun 22, 2026
…erage

Address review on #1575:
- _recursive_index_dict_using_path now navigates with _get_node and propagates
  MISSING for an unset referenced value, so '${copy:source.model}' where
  source.model is '???' reports the missing value instead of an opaque
  'path does not exist' error.
- Tests: _copy/_inherit_from carry '???' into the target (reported); copy of a
  missing leaf is reported not opaque; plain '${a.b.c}' interpolation still
  resolves through full parse (no regression); e2e get_global_config_dict()
  aggregates multiple missing values.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
wprazuch added a commit that referenced this pull request Jun 23, 2026
…erage

Address review on #1575:
- _recursive_index_dict_using_path now navigates with _get_node and propagates
  MISSING for an unset referenced value, so '${copy:source.model}' where
  source.model is '???' reports the missing value instead of an opaque
  'path does not exist' error.
- Tests: _copy/_inherit_from carry '???' into the target (reported); copy of a
  missing leaf is reported not opaque; plain '${a.b.c}' interpolation still
  resolves through full parse (no regression); e2e get_global_config_dict()
  aggregates multiple missing values.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
wprazuch force-pushed the wprazuch/config-missing-values branch from b52ac3c to 6a4dd8c Compare June 23, 2026 09:11
@wprazuch
wprazuch requested a review from ananthsub June 23, 2026 15:20
wprazuch added a commit that referenced this pull request Jun 24, 2026
…erage

Address review on #1575:
- _recursive_index_dict_using_path now navigates with _get_node and propagates
  MISSING for an unset referenced value, so '${copy:source.model}' where
  source.model is '???' reports the missing value instead of an opaque
  'path does not exist' error.
- Tests: _copy/_inherit_from carry '???' into the target (reported); copy of a
  missing leaf is reported not opaque; plain '${a.b.c}' interpolation still
  resolves through full parse (no regression); e2e get_global_config_dict()
  aggregates multiple missing values.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
wprazuch force-pushed the wprazuch/config-missing-values branch from 6a4dd8c to 3cbe609 Compare June 24, 2026 07:02
@wprazuch

Copy link
Copy Markdown
Contributor Author

/ok to test 3cbe609

Comment thread nemo_gym/global_config.py Resolved
Comment thread nemo_gym/config_types.py Resolved
@github-actions

Copy link
Copy Markdown
Contributor

🌿 Preview your docs: https://nvidia-preview-wprazuch-config-missing-values.docs.buildwithfern.com/nemo/gym

Here are the markdown pages you've updated:

wprazuch added a commit that referenced this pull request Jun 24, 2026
…erage

Address review on #1575:
- _recursive_index_dict_using_path now navigates with _get_node and propagates
  MISSING for an unset referenced value, so '${copy:source.model}' where
  source.model is '???' reports the missing value instead of an opaque
  'path does not exist' error.
- Tests: _copy/_inherit_from carry '???' into the target (reported); copy of a
  missing leaf is reported not opaque; plain '${a.b.c}' interpolation still
  resolves through full parse (no regression); e2e get_global_config_dict()
  aggregates multiple missing values.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
wprazuch force-pushed the wprazuch/config-missing-values branch from 0de4e9f to 03dba7e Compare June 24, 2026 09:04
@wprazuch
wprazuch requested a review from ananthsub June 24, 2026 09:55
wprazuch added 5 commits June 24, 2026 13:00
A required value left as OmegaConf MISSING ('???') currently surfaces as an
opaque MissingMandatoryValue deep in parsing — one field at a time, with no
guidance on how to set it. Add a fast-fail check that runs after all sources
are merged (CLI + env.yaml + config_paths) and before inheritance resolution,
raising a single ConfigMissingValuesError listing every unset value with a
++path=<value> override example.

The scan uses OmegaConf.to_container(resolve=False, throw_on_missing=False) so
it never trips on MISSING values or unresolved interpolations.

Part of #1205 (friction point #10).

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
…anches

Codex flagged a regression: raise_on_missing_values ran before
_recursively_swap_keys, so a '???' inside a branch removed by _delete_key
(or overwritten by _inherit_from/_copy) was wrongly reported as missing.

Moving the check after swap is not sufficient on its own: swap_keys
iterated with .items(), which resolves MISSING and raises
MissingMandatoryValue mid-swap on any genuine '???'. Fix is two parts:

- _recursively_swap_keys_helper: iterate with items_ex(resolve=False) so
  directive strings stay raw (swap detection still matches) and '???'
  leaves are returned as-is instead of crashing.
- parse(): run raise_on_missing_values after _recursively_swap_keys, so
  deleted/overwritten branches are excluded from the missing-value report.

Tests: unit (swap + collect ignores deleted branch) and e2e (full
get_global_config_dict parse with _delete_key removing the only '???').

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
…erage

Address review on #1575:
- _recursive_index_dict_using_path now navigates with _get_node and propagates
  MISSING for an unset referenced value, so '${copy:source.model}' where
  source.model is '???' reports the missing value instead of an opaque
  'path does not exist' error.
- Tests: _copy/_inherit_from carry '???' into the target (reported); copy of a
  missing leaf is reported not opaque; plain '${a.b.c}' interpolation still
  resolves through full parse (no regression); e2e get_global_config_dict()
  aggregates multiple missing values.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
Swap iteration over ListConfig now skips missing ('???') elements without
resolving, so a '???' in a list (or in a dict nested in a list) is reported by
raise_on_missing_values instead of crashing mid-swap with MissingMandatoryValue.
Completes the missing-tolerance started for dict leaves. Found by review.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
…ue crash

Address review: _recursive_index_dict_using_path returned a bare '???' string for a missing
referenced path, which broke the swap callers — .pop() on a str (AttributeError) and
OmegaConf.merge('???', v) (ValueError) — so ${inherit_from:}/${copy:} and the {_inherit_from}/
{_copy} property forms of an unset value hit a *new* opaque error. Return a _MISSING_REF sentinel
instead (and widen the inaccurate -> DictConfig annotation); the swap caller then makes the target
'???' and skips pop/merge/delete, so raise_on_missing_values reports it in the aggregated
ConfigMissingValuesError. Add tests across all four quadrants {string, property} x {missing leaf,
missing parent} for copy and inherit, plus an end-to-end swap -> raise check.

Also refresh fern/versions/latest troubleshooting/configuration.mdx to show the new
ConfigMissingValuesError (aggregated) and ServerRefNotFoundError ('Did you mean') messages instead
of the superseded MissingMandatoryValue / AssertionError.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
wprazuch force-pushed the wprazuch/config-missing-values branch from 03dba7e to c64f332 Compare June 24, 2026 11:00
@wprazuch
wprazuch merged commit 797db29 into main Jun 24, 2026
17 checks passed
@wprazuch
wprazuch deleted the wprazuch/config-missing-values branch June 24, 2026 11:11
@github-project-automation github-project-automation Bot moved this from Dev Todo to Done in NeMo Gym 0.4.0 - July 1 Jun 24, 2026
wprazuch added a commit that referenced this pull request Jun 24, 2026
Reconcile the config-error work with #1575 (aggregated error for unset "???" values), which landed
on main. Both PRs add user-facing config-error types; fold ConfigMissingValuesError into the
ConfigError family (ConfigError, ValueError) so the CLI`s `except ConfigError` handler prints unset-
value errors cleanly (no traceback), consistent with the other config errors. Union the imports in
global_config.py and test_global_config.py.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
wprazuch added a commit that referenced this pull request Jun 25, 2026
…ction #12 / FEP-1016)

Add `gym env validate` (+ ng_validate/nemo_gym_validate deprecated shims): run the full config parse
with no Ray and no server subprocesses, exit 0 (valid) / 1 (invalid) with a clean, traceback-free
message — config errors otherwise only surface ~30-60s later after Ray bootstrap. It reuses
get_global_config_dict so the checks stay in sync: config_paths resolution (#1488/#1490), server
cross-references (#1561), mandatory ??? values (#1575), and schema. A dummy policy_model (NO_MODEL)
is injected so model interpolations resolve without real creds — the model is supplied by --model*
at run time. Registered in the env group with the same config-selection flags as env start; wrapped
in exit_cleanly_on_config_error. Tests: env validate routing + validate() valid/invalid behavior.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
wprazuch added a commit that referenced this pull request Jun 25, 2026
…ction #12 / FEP-1016)

Add `gym env validate` (+ ng_validate/nemo_gym_validate deprecated shims): run the full config parse
with no Ray and no server subprocesses, exit 0 (valid) / 1 (invalid) with a clean, traceback-free
message — config errors otherwise only surface ~30-60s later after Ray bootstrap. It reuses
get_global_config_dict so the checks stay in sync: config_paths resolution (#1488/#1490), server
cross-references (#1561), mandatory ??? values (#1575), and schema. A dummy policy_model (NO_MODEL)
is injected so model interpolations resolve without real creds — the model is supplied by --model*
at run time. Registered in the env group with the same config-selection flags as env start; wrapped
in exit_cleanly_on_config_error. Tests: env validate routing + validate() valid/invalid behavior.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@ritaneves ritaneves linked an issue Jun 25, 2026 that may be closed by this pull request
1 task
wprazuch added a commit that referenced this pull request Jun 25, 2026
…tion #12) (#1599)

## What

Adds **`gym env validate`** (+ `ng_validate` / `nemo_gym_validate`
deprecated shims) — runs the full config parse with **no Ray and no
server subprocesses**, then exits **0 (valid) / 1 (invalid)** with a
clean, rich-escaped message (**no traceback**). Returns in well under a
second instead of after a ~30–60s Ray bootstrap.

```bash
gym env validate --config resources_servers/<env>/configs/<env>.yaml --config responses_api_models/<model>/configs/<model>.yaml
gym env validate --benchmark gsm8k --model-type openai_model
```

## How

`validate()` lives in `cli/env.py` and is registered as `env validate`
in the `gym` router (`cli/main.py` COMMANDS) with the same
config-selection flags as `env start` (`--config`, `--benchmark`,
`--environment`, `--resources-server`, `--model-type`, `--search-dir`,
`--model*`). It reuses the same `get_global_config_dict()` parse path
the other commands use, so the validation checks stay in sync:

- **config_paths** resolution — missing/typo'd
([#1488](#1488)) and malformed
([#1490](#1490))
- **server cross-references** — unknown `name:` refs
([#1561](#1561))
- **mandatory `???`** values
([#1575](#1575))
- **schema** (`BaseNeMoGymCLIConfig`)

Wrapped in `exit_cleanly_on_config_error` (from #1609) so any
`ConfigError` becomes a clean message + `exit 1`. A dummy `policy_model`
is injected (the `NO_MODEL` parser config, as in `gym list` / `env
compose`) so model interpolations like `${policy_base_url}` resolve
without real creds — validation is about config **well-formedness**; the
real model is supplied by the `--model*` flags at run time.

## Targets `main`

Originally drafted on the unified-CLI epic branch; rebuilt directly on
`main` now that [#1630](#1630)
(and #1637/#1609/#1635/#1671) have merged. The old branch contents (a
snapshot of the CLI refactor + unrelated CI commits) were superseded and
replaced.

## Scope note

The zero-server check
([#1489](#1489), "nothing
configured to run") is intentionally **not** part of `validate`:
`NO_MODEL` injects a dummy model server (which would defeat the check),
and "is anything configured to run" is a *start*-time concern already
enforced by `gym env start` before Ray init. `validate` focuses on
config well-formedness.

## Why

Epic [#1205](#1205) friction
#12 (no config validation tooling) — the M1 "fast failure triage"
deliverable. Config errors otherwise only surface after Ray starts
(~30–60s).

## Tests

- `test_cli_main.py`: `gym env validate --config X` routes to
`nemo_gym.cli.env:validate` with `+config_paths=[X]` (added to the
parametrized config-command matrix).
- `test_cli.py`: `validate()` prints OK on a valid config; a raised
`ConfigError` becomes `exit 1` (no traceback).
- All `test_cli` + `test_cli_main` + `test_cli_legacy` pass (the only
failures are the pre-existing Python-3.12 `TestDidYouMean` argparse
issue on `main`); ruff + pre-commit clean. Smoke-tested end-to-end: `✓
Config is valid.` on a real benchmark, clean error + `exit 1` on a bad
path, and the `ng_validate` deprecation shim.

---------

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
ritaneves pushed a commit that referenced this pull request Jun 25, 2026
## What

A required config value left unset (OmegaConf MISSING, `???`) currently
surfaces as an opaque
`omegaconf.errors.MissingMandatoryValue` — **one field at a time**,
raised deep in parsing
(while resolving inheritance), with no guidance on how to set it.

This adds a fast-fail check in `parse()` that raises a single
`ConfigMissingValuesError` listing
**every** unset value with a ready-to-use override example.

### Before / after

```
# before
omegaconf.errors.MissingMandatoryValue: Missing mandatory value:
swe_agents.responses_api_agents.swe_agents.container_formatter

# after
2 required config value(s) are unset (still '???') after merging:
  - swe_agents.responses_api_agents.swe_agents.container_formatter
  - swe_agents.responses_api_agents.swe_agents.dataset_path

Provide each value via a CLI override, in env.yaml, or in a config you pass via config_paths.
For example, on the command line:
  ++swe_agents.responses_api_agents.swe_agents.container_formatter=<value>
  ++swe_agents.responses_api_agents.swe_agents.dataset_path=<value>
```

## How

- New `ConfigMissingValuesError(ValueError)` in `config_types.py`.
- `collect_missing_value_paths()` / `raise_on_missing_values()` in
`global_config.py`. The scan
runs **after** all sources are merged (CLI + env.yaml + config_paths)
**and after**
`_recursively_swap_keys` — so that the `_delete_key` / `_inherit_from` /
`_copy` directives have
been applied first. By that point any remaining `???` is genuinely unset
(not a value that is
about to be deleted, or moved/filled by a swap), so it's reported with
no false positives.
`_recursively_swap_keys` itself is made missing-tolerant
(`items_ex(resolve=False)`) so a real
`???` doesn't trip the opaque `MissingMandatoryValue` before the
aggregated scan reports it.
- The walk uses `OmegaConf.to_container(resolve=False,
throw_on_missing=False)`, so it never
  raises on MISSING values or unresolved `${...}` interpolations.

## Scope / safety

Base configs that intentionally ship `???` (api keys, container paths,
model names — ~33 files)
are unaffected: they're filled at run time via CLI/env, and no test
parses a bare `???` config.
This only changes the *error* a user sees when they forget to supply
one.

## Testing

- `test_collect_missing_value_paths` — nested dict + list, asserts
`["a", "b.d", "e[1]"]`.
- `test_get_global_config_dict_raises_on_missing_values` — asserts the
dotted path and the
  `++...=<value>` hint appear in the message.
- `pytest tests/unit_tests/test_global_config.py` 28/28; related
`test_train_data_utils` /
`test_config_types_help` / `test_rollout_collection` 54/54. ruff clean.
No new dependencies.

Part of #1205 (friction point 10). Companion to #1561 (friction 3).

---------

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
Signed-off-by: Rita Fernandes Neves <rfernandesne@nvidia.com>
khazic pushed a commit to khazic/Gym-lao that referenced this pull request Oct 15, 2026
…EADME (NVIDIA-NeMo#4574)

## What does this PR do?

The math_with_judge README's "Prepare for trajectory collection" command
works again. It loads `configs/dapo17k_trajectory_collection.yaml`,
which no longer exists, so `gym dataset collate` exited with
`ConfigPathNotFoundError` before downloading anything. This PR restores
that config in the current schema, documents the step's prerequisites,
and adds a regression test that replays the README command.

Running the README's other GitLab download, OpenMathReasoning, from
outside the Gym root exposed a second bug: the GitLab downloader did not
create the output directory. The second commit fixes it.

Fixes NVBug [6847517](https://nvbugspro.nvidia.com/bug/6847517).
Closes NVIDIA-NeMo#4498.

### Cause

NVIDIA-NeMo#859 ("Gym 0.2.0 huggingface dataset pointers") deleted three dapo17k
variants under `resources_servers/math_with_judge/configs/`, including
`dapo17k_trajectory_collection.yaml`, and removed their root-README
rows. The math_with_judge README, which has cited that file since NVIDIA-NeMo#291,
was not updated. Config resolution (`nemo_gym/global_config.py`
`load_extra_config_paths`) then raises `ConfigPathNotFoundError`.

`dapo17k.yaml` is not a drop-in replacement. The deleted config put the
DAPO17k train problems in the **validation** split, repeated 16 times,
because NeMo RL with `is_trajectory_collection: true` collects from the
validation dataloader only. `dapo17k.yaml` has DAPO17k as `train` ×1 and
AIME24 ×32 as `validation`, so trajectory collection would run on
AIME24.

Restoring the old file verbatim would not work either. It set
`judge_model_server.name: ???`, and since NVIDIA-NeMo#1575 the parser rejects any
`???` left after merging (`ConfigMissingValuesError`).

The second bug is in `download_jsonl_dataset`
(`nemo_gym/gitlab_utils.py`). It opened `output_fpath` for writing
without creating its directory. A Gym checkout tracks `data/`, so the
README's `--output data/open_math_reasoning_problems.jsonl` works from
the Gym root but fails with `FileNotFoundError` from any other
directory. `gym dataset collate --download` fails the same way for
GitLab-backed datasets. The Hugging Face download already creates the
directory (`hf_utils.py`).

### Fix

- Restore `configs/dapo17k_trajectory_collection.yaml`. The validation
split is the `dapo17k` 0.0.1 `dapo17k_train.jsonl` artifact (the same
GitLab source as `dapo17k.yaml`), `num_repeats: 16`. Grading uses
math-verify only (`should_use_judge: false`), so
`judge_model_server.name` is `policy_model` and the README command needs
no override. The file has `description`/`value` and the
environment-server block like `dapo17k.yaml`. The `update-readme-table`
hook re-adds its root-README row.
- It keeps its own local file,
`data/dapo17k_trajectory_collection_train.jsonl`, as before NVIDIA-NeMo#859.
Collate checks each dataset against `<jsonl stem>_metrics.json`. Sharing
`dapo17k.yaml`'s `dapo17k_train.jsonl` would compare the ×16 validation
stats with that file's train sidecar and fail (`train != validation`,
`17398 != 278368`).
- Add `data/dapo17k_trajectory_collection_train_metrics.json`, written
by a real collate run (278368 rows = 17398 × 16). Its word and turn
statistics match `dapo17k_train_metrics.json` exactly, so the rows are
the same.
- README: the command is unchanged. One note says the GitLab downloads
in this README (the OpenMathReasoning download and this `--download`)
need the `mlflow` extra (optional since NVIDIA-NeMo#3547) and `mlflow_tracking_uri`
/ `mlflow_tracking_token` in `env.yaml`. The section says what it writes
and why the split is validation.

Test:
`tests/unit_tests/test_math_with_judge_readme_trajectory_config.py`
takes the README's `gym dataset collate` block and runs it through the
real CLI with `dispatch` intercepted. It resolves the config offline (no
`env.yaml`), checks the single dataset is `validation` ×16 with
`dapo17k.yaml`'s GitLab identifier, and checks the committed sidecar
describes that split. It then runs `TrainDataProcessor` on a three-row
fixture in a temporary directory, without network access, and checks
that only `validation.jsonl` and its metrics come out, with each
question repeated 16 times in order.

- `download_jsonl_dataset` now creates the output's parent directory, as
the Hugging Face download does. A new test in
`tests/unit_tests/test_gitlab_utils.py` fakes the registry lookup and
the HTTP fetch, then downloads into a missing nested directory. It fails
before this change with the same `FileNotFoundError` and passes after.

These plausible wrong fixes all fail the trajectory test:
- restoring the file verbatim (`???`);
- pointing the README at `dapo17k.yaml` with a judge override;
- `num_repeats: 1`;
- a `train` split;
- sharing `dapo17k_train.jsonl`.

### Before the fix

Gym `977583d3d` (`main`), the README command from the Gym root:

```bash
gym dataset collate \
    --config resources_servers/math_with_judge/configs/dapo17k_trajectory_collection.yaml \
    --config responses_api_models/openai_model/configs/openai_model.yaml \
    --output-dir data/dapo17k_trajectory_collection \
    --mode train_preparation \
    --download
```

```text
nemo_gym.config_types.ConfigPathNotFoundError: config_paths entry 'resources_servers/math_with_judge/configs/dapo17k_trajectory_collection.yaml' was not found. Looked in:
  - <gym>/resources_servers/math_with_judge/configs/dapo17k_trajectory_collection.yaml
Check the path is spelled correctly and is relative to your working directory, an extra root
(NEMO_GYM_EXTRA_ROOTS / --search-dir), or the Gym install root.
EXIT=1
```

The README's OpenMathReasoning GitLab download, from a directory without
`data/`, same base:

```bash
gym dataset download --storage gitlab --name math_open_math_reasoning --revision 0.0.1 \
    --artifact open_math_reasoning_problems.jsonl --output data/open_math_reasoning_problems.jsonl
```

```text
  File ".../nemo_gym/gitlab_utils.py", line 111, in download_jsonl_dataset
    with open(config.output_fpath, "w") as f:
FileNotFoundError: [Errno 2] No such file or directory: '<workspace>/data/open_math_reasoning_problems.jsonl'
```

### After the fix

This branch, same command, with the `mlflow` extra and GitLab registry
credentials in `env.yaml`. No local data beforehand:

```text
Downloading dataset `validation` for `math_with_judge_simple_agent` from gitlab using dataset_name='dapo17k' version='0.0.1' artifact_fpath='dapo17k_train.jsonl' output_fpath='resources_servers/math_with_judge/data/dapo17k_trajectory_collection_train.jsonl'
Dataset validation repeating 16x: each line repeated 16 times (e.g. pattern: abc -> aaaabbbbcccc)
Aggregate metrics for resources_servers/math_with_judge/data/dapo17k_trajectory_collection_train_metrics.json
Aggregate metrics for data/dapo17k_trajectory_collection/validation_metrics.json
EXIT=0
$ wc -l data/dapo17k_trajectory_collection/validation.jsonl
278368 data/dapo17k_trajectory_collection/validation.jsonl
```

There are 17398 runs of identical questions, each 16 rows long, and no
`train.jsonl`. A second run skips the download, reports the sidecar
`already up to date`, and exits 0.

The OpenMathReasoning download, same command and directory: exit 0, with
the file written (the QA case below checks its rows).

### Validation

- QA testcase `math_with_judge`, which runs both README GitLab steps
from a scratch directory with registry credentials:
- base `977583d3d`: 0/2. Trajectory collation fails because the config
is missing; the download fails with `FileNotFoundError`.
- this branch: 2/2. The download passes in 4.4 s; the trajectory
collation passes in 20.3 s with 17398 × 16 rows and the committed
sidecar unchanged.
  - The QA-suite change ships separately.
- New tests: trajectory test before 1 failed
(`ConfigPathNotFoundError`), after 1 passed, and the five wrong fixes
above fail it. Downloader test before 1 failed (`FileNotFoundError`),
after 1 passed, also with the real `mlflow` 3.16.1 installed.
- `test_train_data_utils.py`, `test_cli_main.py`,
`test_global_config.py`, `test_dataset_source.py`,
`test_task_data.py::TestSchemaPresence`, `test_ray_opt_out.py`,
`test_cli_resources_servers.py`, `test_model_registry.py`,
`test_resources_server_registry.py`, `test_environment_scaffold.py`,
`test_gitlab_utils.py`, `test_hf_utils.py`, and the new tests: 592
passed.
- `pre-commit run --all-files`: passed.
- Rollout evidence: N/A. This is a config and docs change, and `gym
dataset collate` runs no model. The full download-and-collate run above
uses the real registry data.

### Compatibility

A new config file. Nothing that loads other configs changes. GitLab
downloads now create a missing output directory instead of failing. The
restored config uses the same GitLab artifact as `dapo17k.yaml`, but a
separate local file, so each keeps its own metrics sidecar.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: qiaochuz <qiaochuz@nvidia.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

10 - Configuration Friction

3 participants