Skip to content

fix(config): actionable error for unknown server cross-references - #1561

Merged
wprazuch merged 2 commits into
mainfrom
wprazuch/config-friction-quickwins
Jun 15, 2026
Merged

fix(config): actionable error for unknown server cross-references#1561
wprazuch merged 2 commits into
mainfrom
wprazuch/config-friction-quickwins

Conversation

@wprazuch

@wprazuch wprazuch commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

What

When a server config references another server by {type, name} and the name doesn't exist in the merged config, replace the bare assert in validate_and_populate_defaults with a dedicated ServerRefNotFoundError that:

  • names the offending instance and the field that holds the bad reference,
  • fuzzy-matches the missing name against same-type instances (via difflib) and suggests the closest correct name,
  • falls back to listing the available same-type instances when there's no close match.

It also iterates .items() (to recover the field name) and raises instead of asserting, so validation is not stripped under python -O.

Why

Today a typo in a cross-reference produces an opaque AssertionError that dumps the full list of every server ref, only after Ray has initialized (~30–60s). This is friction point 3 in #1205.

Before / after

# before
AssertionError: Could not find type='resources_servers' name='workplace_assitant' in the list
of available servers: [type='resources_servers' name='workplace_assistant', ...]

# after
ServerRefNotFoundError: In server instance 'workplace_assistant_simple_agent', field
'resources_server' references resources_servers/'workplace_assitant', which is not defined in
the merged config.
Did you mean: 'workplace_assistant'?

Scope

This is a message-quality fix only — behavior is unchanged for valid configs, and invalid configs still fail at the same point. The deeper recursive scan for refs nested inside sub-dicts is intentionally deferred (it would surface previously-silent misses, a behavior change).

Testing

  • Updated the two existing error tests (errors_on_missing, errors_on_wrong_type) to expect ServerRefNotFoundError; the first now asserts the message names the instance, field, and ref.
  • Added test_get_global_config_dict_server_refs_suggests_close_match covering the fuzzy "Did you mean?" branch (typo resource → suggests resources, scoped to same type).
  • pytest tests/unit_tests/test_global_config.py — 27/27 pass. ruff clean.

Part of #1205 (friction point 3). No new dependencies (difflib is stdlib).

@copy-pr-bot

copy-pr-bot Bot commented Jun 10, 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.

Comment thread nemo_gym/global_config.py Outdated
Comment thread nemo_gym/global_config.py Outdated
wprazuch added a commit that referenced this pull request Jun 10, 2026
…uote error

Per review on #1561: import get_close_matches directly instead of the
difflib module, and use a triple-quoted block for the ServerRefNotFoundError
message instead of concatenated single-quote f-strings. Output is unchanged.

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

Per review on #1561: import get_close_matches directly instead of the
difflib module, and use a triple-quoted block for the ServerRefNotFoundError
message instead of concatenated single-quote f-strings. Output is unchanged.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
wprazuch force-pushed the wprazuch/config-friction-quickwins branch from 5a85a53 to 186a762 Compare June 10, 2026 18:18
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 added 2 commits June 15, 2026 15:06
Replace the bare assert in validate_and_populate_defaults with a
ServerRefNotFoundError that names the offending instance and field, and
fuzzy-matches the missing name against same-type instances to suggest a
correction. Iterate items() to recover the field name; raise instead of
assert so validation is not stripped under python -O.

Closes part of #1205 (friction point #3).

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

Per review on #1561: import get_close_matches directly instead of the
difflib module, and use a triple-quoted block for the ServerRefNotFoundError
message instead of concatenated single-quote f-strings. Output is unchanged.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
wprazuch force-pushed the wprazuch/config-friction-quickwins branch from 186a762 to 84bce25 Compare June 15, 2026 13:06

@marta-sd marta-sd 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.

Approving since Brian's suggestions were addressed and he's ooto

@wprazuch
wprazuch enabled auto-merge (squash) June 15, 2026 13:29
@wprazuch

Copy link
Copy Markdown
Contributor Author

/ok to test 84bce25

@wprazuch
wprazuch dismissed bxyu-nvidia’s stale review June 15, 2026 13:51

Applied changes, and brian is currently unavailable

@wprazuch
wprazuch merged commit 9f3192b into main Jun 15, 2026
30 checks passed
@wprazuch
wprazuch deleted the wprazuch/config-friction-quickwins branch June 15, 2026 13:51
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 added a commit that referenced this pull request Jun 22, 2026
)

## What

When a server config references another server by `{type, name}` and the
name doesn't exist in the merged config, replace the bare `assert` in
`validate_and_populate_defaults` with a dedicated
`ServerRefNotFoundError` that:

- names the **offending instance** and the **field** that holds the bad
reference,
- fuzzy-matches the missing name against **same-type** instances (via
`difflib`) and suggests the closest correct name,
- falls back to listing the available same-type instances when there's
no close match.

It also iterates `.items()` (to recover the field name) and `raise`s
instead of `assert`ing, so validation is not stripped under `python -O`.

## Why

Today a typo in a cross-reference produces an opaque `AssertionError`
that dumps the full list of every server ref, only after Ray has
initialized (~30–60s). This is friction point 3 in #1205.

## Before / after

```
# before
AssertionError: Could not find type='resources_servers' name='workplace_assitant' in the list
of available servers: [type='resources_servers' name='workplace_assistant', ...]

# after
ServerRefNotFoundError: In server instance 'workplace_assistant_simple_agent', field
'resources_server' references resources_servers/'workplace_assitant', which is not defined in
the merged config.
Did you mean: 'workplace_assistant'?
```

## Scope

This is a message-quality fix only — behavior is unchanged for valid
configs, and invalid configs still fail at the same point. The deeper
recursive scan for refs nested inside sub-dicts is intentionally
deferred (it would surface previously-silent misses, a behavior change).

## Testing

- Updated the two existing error tests (`errors_on_missing`,
`errors_on_wrong_type`) to expect `ServerRefNotFoundError`; the first
now asserts the message names the instance, field, and ref.
- Added `test_get_global_config_dict_server_refs_suggests_close_match`
covering the fuzzy "Did you mean?" branch (typo `resource` → suggests
`resources`, scoped to same type).
- `pytest tests/unit_tests/test_global_config.py` — 27/27 pass. `ruff`
clean.

Part of #1205 (friction point 3). No new dependencies (`difflib` is
stdlib).

---------

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
wprazuch added a commit that referenced this pull request Jun 24, 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>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 - Configuration Friction

3 participants