Repository navigation
GlobalConfig resolves overrides from recursive config_paths correctly - #2478
Merged
Merged
Conversation
5 tasks done
bxyu-nvidia
pushed a commit
that referenced
this pull request
Aug 17, 2026
…rride inner ones (#2572) ## What does this PR do? Nesting depth and sibling order are two independent precedence axes in `config_paths`, and a single list reversal cannot express both. `load_extra_config_paths` walks `config_paths` breadth-first and appends nested entries to the same flat list, so it ends up as `[caller entries..., their children..., grandchildren...]`. Merging that left-to-right let children override their parents, so #2478 (0051ad8) reversed the whole list. That correctly made outer configs win, but it also reversed the caller's own entries against each other, inverting sibling precedence: given `--config base.yaml --config override.yaml`, `base.yaml` now wins. The failure is silent. The override still loads, still resolves, and still appears in the final `config_paths`; it just loses every key it sets. For an eval config that means the run completes and reports plausible numbers scored against the wrong judge prompt and reward mode. This PR tracks each entry's nesting depth as it is discovered and orders the configs deepest-first with a stable sort. Deepest-first keeps #2478's intent at any depth: an outer config merges last and overrides anything it pulled in. Stability restores sibling precedence, so same-level configs keep the order they were listed in and a later one overrides an earlier one. The blanket `extra_configs.reverse()` at the call site is no longer needed. Nothing in `docs/` describes this precedence, so the added tests are the spec. ## Tests Three additive cases in `TestGlobalConfig`; no existing assertion changed. - `..._config_paths_later_sibling_wins` — two caller entries, neither referencing the other; the later one overrides the earlier. - `..._config_paths_outer_beats_inner_across_siblings` — both axes at once: the later sibling wins its key while the parent still beats the config it pulled in. - `..._config_paths_outer_beats_inner_two_levels_deep` — outer beats middle beats inner. Negative control: with `global_config.py` reverted to `main`, the two sibling tests fail with the production symptom (`a: base` where `a: override` is expected) while the three-level nesting test passes either way, confirming the property #2478 added is preserved rather than traded away. Full unit suite: 2435 passed. One unrelated pre-existing failure (`test_cli_utils.py::TestPrintRichTable::test_not_truncated_regardless_of_ambient_width`), which fails identically on unmodified `main`. ## Checklist - [x] I have read the [contributing guidelines](https://docs.nvidia.com/nemo/gym/latest/contribute/development-setup). - [x] The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs. - [x] Tests added or updated and pass locally, or N/A for docs-only / non-code changes. - [x] Pre-commit checks pass locally (`pre-commit run --all-files`). - [x] All commits have DCO sign-off (`git commit -s`). Signed-off-by: Ushnish De <ude@nvidia.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.