Skip to content

feat: enable external plugins for all CLI commands - #2061

Merged
marta-sd merged 10 commits into
mainfrom
martas/cli-1582-1-search-roots
Jul 22, 2026
Merged

feat: enable external plugins for all CLI commands#2061
marta-sd merged 10 commits into
mainfrom
martas/cli-1582-1-search-roots

Conversation

@marta-sd

@marta-sd marta-sd commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Extended usage of --search-dir flag and incorporates env vars logic from #1264 by @gwarmstrong

@copy-pr-bot

copy-pr-bot Bot commented Jul 17, 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.

@marta-sd marta-sd changed the title martas/cli 1582 1 search roots feat: enable external plugins for all CLI commands Jul 17, 2026
@marta-sd marta-sd self-assigned this Jul 17, 2026
@marta-sd
marta-sd requested a review from prokotg July 17, 2026 08:10
@marta-sd marta-sd linked an issue Jul 17, 2026 that may be closed by this pull request
7 tasks
@marta-sd
marta-sd force-pushed the martas/cli-1582-1-search-roots branch from 610aa25 to b32c6d0 Compare July 17, 2026 12:51
@marta-sd
marta-sd requested a review from gwarmstrong July 17, 2026 12:53
@marta-sd

Copy link
Copy Markdown
Contributor Author

/ok to test b32c6d0

Comment thread nemo_gym/__init__.py

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.

I know it's an inherited problem, but I'd suggest to move definitions from __init__.py to a dedicated module

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this would be harder / uglier than it seems.

I can't move old things like PARENT_DIR plus I need to call _augment_sys_path in init. George suggests using component_search_roots in _augment_sys_path (comment) which I think makes sense.

So in principle we have a circular dependency between this new dedicated module and __init__.py. It's possible to avoid circular import error (I think), but I prefer the current solution is a lesser evil.

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.

Ok, let's consider this problem separately from this MR but I strongly discourage having anything than declaration, re-imports or variable definition in __init__

Comment thread nemo_gym/cli/main.py

@gwarmstrong gwarmstrong 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.

I think there's a few useful command here that are missing --search-dir, e.g., all of the following I think have valid use-cases, but aren't enabled here:

gym env resolve \
  --search-dir ../my-plugin \
  --config configs/my-config.yaml

gym dataset render \
  --search-dir ../my-plugin \
  --input data.jsonl \
  --prompt-config prompts/my-prompt.yaml \
  --output rendered.jsonl

gym env test \
  --search-dir ../my-plugin \
  --resources-server my-server

gym env packages \
  --search-dir ../my-plugin \
  --resources-server my-server

Comment thread nemo_gym/__init__.py
return roots[0] / p


def _augment_sys_path() -> None:

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.

Could _augment_sys_path() use component_search_roots() in some way instead of rebuilding the root order separately? The current order looks correct, but file lookup and Python imports still get their ordering from different logic. Sharing the same function would make it less likely that they diverge later.

E.g., if we have:

/tmp/my-plugin/
└── benchmarks/
    └── aime24/
        ├── __init__.py
        ├── config.yaml
        └── prepare.py

When running with:

gym eval prepare --search-dir /tmp/my-plugin --benchmark aime24

Gym should load both:

/tmp/my-plugin/benchmarks/aime24/config.yaml
/tmp/my-plugin/benchmarks/aime24/prepare.py

And I think it would be easier to maintain consistency if those were all resolved in a single function, rather than separate functions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good point. it also made me realize that in _augment_sys_path() we don't have Path.cwd() and WORKING_DIR, while we have them in component_search_roots. also I noticed that WORKING_DIR = PARENT_DIR if _is_editable_install else Path.cwd() so this looks redundant.

I will clean this up

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done in 0d58030

Comment thread nemo_gym/cli/main.py
@marta-sd

Copy link
Copy Markdown
Contributor Author

/ok to test 7db8854

@marta-sd

Copy link
Copy Markdown
Contributor Author

I think there's a few useful command here that are missing --search-dir
@gwarmstrong added in 7db8854

@marta-sd
marta-sd requested a review from gwarmstrong July 21, 2026 10:59
marta-sd and others added 3 commits July 22, 2026 11:54
…ir for gym list and search

env var allows to pass extra dirs deep into the code, including rollouts colletion,
prompt templates, config paths and env.yaml loading.

Co-Authored-By: George Armstrong <georgea@nvidia.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
…ogging too

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
marta-sd added 7 commits July 22, 2026 11:54
…of replacing

Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
…age: gym list [-h] {benchmarks,environments,agents} ...

List available components (benchmarks, agents, environments).

positional arguments:
  {benchmarks,environments,agents}
    benchmarks          List available benchmarks.
    environments        List available environments by name.
    agents              List agent harnesses and how each composes (Pattern A
                        vs self-contained B).

options:
  -h, --help            show this help message and exit
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
…rmatting

Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
… paths

Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
…lugins

Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
@marta-sd
marta-sd force-pushed the martas/cli-1582-1-search-roots branch from 7db8854 to 504b7b2 Compare July 22, 2026 09:54
@marta-sd

Copy link
Copy Markdown
Contributor Author

/ok to test 504b7b2

@marta-sd
marta-sd merged commit 5d56f12 into main Jul 22, 2026
16 checks passed
@marta-sd
marta-sd deleted the martas/cli-1582-1-search-roots branch July 22, 2026 11:29
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.

[Epic] Search & Discoverability for Environments and Components from the CLI

3 participants