diff --git a/fern/versions/latest/pages/about/release-notes.mdx b/fern/versions/latest/pages/about/release-notes.mdx index f960c4762f..8d9df17b45 100644 --- a/fern/versions/latest/pages/about/release-notes.mdx +++ b/fern/versions/latest/pages/about/release-notes.mdx @@ -131,7 +131,7 @@ Added 5 new agent servers: Aviary agent, proof refinement agent, SWE agents, too ### Infrastructure & Developer Experience - PyPI compatibility: install via `pip install nemo-gym` -- Dry run mode: `ng_run +dryrun=true` to validate configs and install environments without starting servers +- Dry run mode: `ng_run +dry_run=true` to validate configs and install environments without starting servers - `ng_status` command to list running servers and their health - FastAPI worker support for higher throughput across multiple workers - Server stdout/stderr redirection with server name prefixes diff --git a/fern/versions/v0.3.0/pages/about/release-notes.mdx b/fern/versions/v0.3.0/pages/about/release-notes.mdx index f960c4762f..8d9df17b45 100644 --- a/fern/versions/v0.3.0/pages/about/release-notes.mdx +++ b/fern/versions/v0.3.0/pages/about/release-notes.mdx @@ -131,7 +131,7 @@ Added 5 new agent servers: Aviary agent, proof refinement agent, SWE agents, too ### Infrastructure & Developer Experience - PyPI compatibility: install via `pip install nemo-gym` -- Dry run mode: `ng_run +dryrun=true` to validate configs and install environments without starting servers +- Dry run mode: `ng_run +dry_run=true` to validate configs and install environments without starting servers - `ng_status` command to list running servers and their health - FastAPI worker support for higher throughput across multiple workers - Server stdout/stderr redirection with server name prefixes diff --git a/nemo_gym/cli/env.py b/nemo_gym/cli/env.py index 5513f9e621..2b4bcd910d 100644 --- a/nemo_gym/cli/env.py +++ b/nemo_gym/cli/env.py @@ -89,6 +89,19 @@ def wrapper(*args, **kwargs): return wrapper +def _resolve_server_dir(rel_path: Path) -> Path: + """Resolve a relative server dir (e.g. ``resources_servers/``) to an absolute path. + + Checks the current working directory first (a user's local server), then falls back to the Gym + install root (``PARENT_DIR``) where built-in servers live in both editable and wheel installs. + This lets ``gym env test`` find and run built-in servers from any cwd, not just a repo checkout. + """ + cwd_path = Path.cwd() / rel_path + if (cwd_path / "requirements.txt").exists() or (cwd_path / "pyproject.toml").exists(): + return cwd_path + return PARENT_DIR / rel_path + + class RunConfig(BaseNeMoGymCLIConfig): """ Start NeMo Gym servers for agents, models, and resources. @@ -135,6 +148,15 @@ def model_post_init(self, context): # pragma: no cover def dir_path(self) -> Path: return self._dir_path + @property + def resolved_dir_path(self) -> Path: + """Absolute server dir resolved against the cwd, then the Gym install root. + + Use this for filesystem access (reading data, running the suite); use ``dir_path`` (the + relative entrypoint) for display and example commands shown to the user. + """ + return _resolve_server_dir(self._dir_path) + class RunHelper: # pragma: no cover _head_server: uvicorn.Server @@ -191,11 +213,8 @@ def start(self, global_config_dict_parser_config: GlobalConfigDictParserConfig) entrypoint_fpath = Path(server_config_dict.entrypoint) assert not entrypoint_fpath.is_absolute() - # Check cwd first for a local server, fall back to the install location for built-ins. - _server_rel_path = Path(first_key, second_key) - _cwd_path = Path.cwd() / _server_rel_path - _cwd_is_server = (_cwd_path / "requirements.txt").exists() or (_cwd_path / "pyproject.toml").exists() - dir_path = _cwd_path if _cwd_is_server else PARENT_DIR / _server_rel_path + # Resolve cwd-first (a local server), else the install location for built-ins. + dir_path = _resolve_server_dir(Path(first_key, second_key)) command = f"""{setup_env_command(dir_path, global_config_dict, top_level_path)} \\ && {NEMO_GYM_CONFIG_DICT_ENV_VAR_NAME}={escaped_config_dict_yaml_str} \\ @@ -459,8 +478,9 @@ def _validate_data_single(test_config: TestConfig) -> None: # pragma: no cover if test_config.dir_path.parts[0] != "resources_servers": return - # Check that the required examples and example metrics are present. - example_fpath = test_config.dir_path / "data/example.jsonl" + # Check that the required examples and example metrics are present. Read from the resolved dir + # (built-ins live under the install root) while messages reference the relative entrypoint. + example_fpath = test_config.resolved_dir_path / "data/example.jsonl" assert example_fpath.exists(), ( f"A jsonl file containing 5 examples is required for the {test_config.dir_path} resources server. The file must be found at {example_fpath}. Usually this example data is just the first 5 examples of your train dataset." ) @@ -469,7 +489,7 @@ def _validate_data_single(test_config: TestConfig) -> None: # pragma: no cover assert count == 5, f"Expected 5 examples at {example_fpath} but got {count}." server_type_name = test_config.dir_path.parts[-1] - example_metrics_fpath = test_config.dir_path / "data/example_metrics.json" + example_metrics_fpath = test_config.resolved_dir_path / "data/example_metrics.json" assert ( example_metrics_fpath.exists() ), f"""You must run the example data validation for the example data found at {example_fpath}. @@ -499,11 +519,11 @@ def _validate_data_single(test_config: TestConfig) -> None: # pragma: no cover f"Expected 5 examples in the metrics at {example_metrics_fpath}, but got {example_metrics['Number of examples']}" ) - conflict_paths = glob(str(test_config.dir_path / "data/*conflict*")) + conflict_paths = glob(str(test_config.resolved_dir_path / "data/*conflict*")) conflict_paths_str = "\n- ".join([""] + conflict_paths) assert not conflict_paths, f"Found {len(conflict_paths)} conflicting paths: {conflict_paths_str}" - example_rollouts_fpath = test_config.dir_path / "data/example_rollouts.jsonl" + example_rollouts_fpath = test_config.resolved_dir_path / "data/example_rollouts.jsonl" assert example_rollouts_fpath.exists(), f"""You must run the example data through your agent and provide the example rollouts at `{example_rollouts_fpath}`. Your commands should look something like: @@ -533,8 +553,11 @@ def _validate_data_single(test_config: TestConfig) -> None: # pragma: no cover def _test_single(test_config: TestConfig, global_config_dict: DictConfig) -> Popen: # pragma: no cover # Eventually we may want more sophisticated testing here, but this is sufficient for now. prefix = test_config.entrypoint.replace("/", "\\/") - command = f"""{setup_env_command(test_config.dir_path, global_config_dict, prefix)} && pytest""" - return run_command(command, test_config.dir_path) + resolved_dir = test_config.resolved_dir_path + command = f"""{setup_env_command(resolved_dir, global_config_dict, prefix)} && pytest""" + # Generated server tests import `resources_servers....`, so the project root (the dir + # holding the server-type dirs) must be on PYTHONPATH when running from outside a repo checkout. + return run_command(command, resolved_dir, project_root=resolved_dir.parent.parent) def test(): # pragma: no cover @@ -616,15 +639,26 @@ def test_all(): # pragma: no cover global_config_dict = get_global_config_dict() test_all_config = TestAllConfig.model_validate(global_config_dict) - candidate_dir_paths = [ - *glob("resources_servers/*"), - *glob("responses_api_agents/*"), - *glob("responses_api_models/*"), - ] - candidate_dir_paths = [p for p in candidate_dir_paths if "pycache" not in p] + # Discover server modules under both the cwd (a user's project) and the Gym install root + # (built-ins, which live under PARENT_DIR in editable and wheel installs). Entrypoints are kept + # relative; the cwd shadows the install root for same-named modules. This lets `gym env test` + # discover and run built-in servers from any cwd, not only a repo checkout. + server_type_dirs = ("resources_servers", "responses_api_agents", "responses_api_models") + seen_rel_paths: set[str] = set() + candidate_dir_paths: List[str] = [] + for root in (Path.cwd(), PARENT_DIR): + for server_type_dir in server_type_dirs: + for module_path in sorted((root / server_type_dir).glob("*")): + if "pycache" in module_path.name or not module_path.is_dir(): + continue + rel_path = f"{server_type_dir}/{module_path.name}" + if rel_path in seen_rel_paths: + continue + seen_rel_paths.add(rel_path) + candidate_dir_paths.append(rel_path) print(f"Found {len(candidate_dir_paths)} total modules:{_display_list_of_paths(candidate_dir_paths)}\n") dir_paths: List[Path] = list(map(Path, candidate_dir_paths)) - dir_paths = [p for p in dir_paths if (p / "README.md").exists()] + dir_paths = [p for p in dir_paths if (_resolve_server_dir(p) / "README.md").exists()] print(f"Found {len(dir_paths)} modules to test:{_display_list_of_paths(dir_paths)}\n") # Keep the full list for the total-vs-tested mismatch check below, then narrow to this shard. @@ -668,7 +702,7 @@ def test_all(): # pragma: no cover data_validation_failed.append(dir_path) if test_all_config.delete_venvs_after_each_test: - venv_path = dir_path / ".venv" + venv_path = _resolve_server_dir(dir_path) / ".venv" print(f"Deleting {venv_path} since `delete_venvs_after_each_test=true`") rmtree(venv_path, ignore_errors=True) @@ -978,7 +1012,7 @@ def pip_list(): # pragma: no cover global_config_dict = get_global_config_dict() config = PipListConfig.model_validate(global_config_dict) - dir_path = Path(config.entrypoint) + dir_path = _resolve_server_dir(Path(config.entrypoint)) venv_path = dir_path / ".venv" if not venv_path.exists(): diff --git a/nemo_gym/cli/setup_command.py b/nemo_gym/cli/setup_command.py index c1940d4933..4741f25457 100644 --- a/nemo_gym/cli/setup_command.py +++ b/nemo_gym/cli/setup_command.py @@ -172,16 +172,24 @@ def setup_env_command(dir_path: Path, global_config_dict: DictConfig, prefix: st return f"cd {dir_path} && {env_setup_cmd}" -def run_command(command: str, working_dir_path: Path, server_name: str = "") -> Popen: +def run_command( + command: str, working_dir_path: Path, server_name: str = "", project_root: Path | None = None +) -> Popen: global_config_dict = get_global_config_dict() work_dir = f"{working_dir_path.absolute()}" custom_env = environ.copy() - py_path = custom_env.get("PYTHONPATH", None) - if py_path is not None: - custom_env["PYTHONPATH"] = f"{work_dir}:{py_path}" - else: - custom_env["PYTHONPATH"] = work_dir + # The server dir on PYTHONPATH lets `import app` work. When a caller passes `project_root` (the + # dir containing resources_servers/, responses_api_agents/, ...), it's added so generated + # `resources_servers..app`-style imports resolve from outside a repo checkout — opt-in, so + # this generic helper doesn't bake a layout assumption in for its other callers. + py_path_entries = [work_dir] + if project_root is not None: + py_path_entries.append(f"{project_root.absolute()}") + existing_py_path = custom_env.get("PYTHONPATH") + if existing_py_path: + py_path_entries.append(existing_py_path) + custom_env["PYTHONPATH"] = ":".join(py_path_entries) custom_env["UV_CACHE_DIR"] = global_config_dict[UV_CACHE_DIR_KEY_NAME] diff --git a/nemo_gym/config_types.py b/nemo_gym/config_types.py index 4f694dd873..02842b6b08 100644 --- a/nemo_gym/config_types.py +++ b/nemo_gym/config_types.py @@ -167,6 +167,15 @@ class ServerRefNotFoundError(ConfigError, ValueError): """A server cross-reference points to an instance that is not defined in the merged config.""" +class InheritPathNotFoundError(ConfigError, ValueError): + """An `_inherit_from` / swap / copy directive references a config path that does not exist.""" + + +class AlmostServerError(ConfigError, ValueError): + """One or more server blocks are almost-servers (right shape, failed validation) and + `error_on_almost_servers` is set, so the run is aborted.""" + + ######################################## # Dataset configs for handling and upload/download ######################################## diff --git a/nemo_gym/global_config.py b/nemo_gym/global_config.py index 2942223164..8043ec47f8 100644 --- a/nemo_gym/global_config.py +++ b/nemo_gym/global_config.py @@ -38,8 +38,10 @@ from nemo_gym import CACHE_DIR, PARENT_DIR, RESULTS_DIR, WORKING_DIR from nemo_gym.config_types import ( + AlmostServerError, ConfigMissingValuesError, ConfigPathNotFoundError, + InheritPathNotFoundError, MalformedConfigPathsError, NoServerInstancesError, ServerInstanceConfig, @@ -509,7 +511,7 @@ def _recursive_index_dict_using_path(self, dict_config: DictConfig, path: List[s # absent key still errors clearly. node = dict_config._get_node(k) if isinstance(dict_config, DictConfig) else None if node is None: - raise ValueError(f"Path specified does not exist in config: {path}") + raise InheritPathNotFoundError(f"Path specified does not exist in config: {path}") # The referenced value (or an ancestor of it) is unset. Return the _MISSING_REF sentinel # so the caller makes the swap/copy/inherit target '???' too (instead of calling .pop()/ @@ -621,7 +623,7 @@ def parse(self, parse_config: Optional[GlobalConfigDictParserConfig] = None) -> Found global config dict yaml: {config_to_log_yaml}""" - raise ValueError(error_msg) + raise AlmostServerError(error_msg) server_instance_configs = self.filter_for_server_instance_configs(global_config_dict) diff --git a/tests/unit_tests/test_cli.py b/tests/unit_tests/test_cli.py index d65e860de1..82df178a7a 100644 --- a/tests/unit_tests/test_cli.py +++ b/tests/unit_tests/test_cli.py @@ -32,6 +32,8 @@ _GRACEFUL_SHUTDOWN_TIMEOUT_SEC, RunConfig, RunHelper, + TestConfig, + _resolve_server_dir, _select_shard, exit_cleanly_on_config_error, init_resources_server, @@ -192,6 +194,28 @@ def test_run_helper_falls_back_to_install_when_not_in_cwd(self, tmp_path: Path) assert dir_path == PARENT_DIR / "resources_servers" / "arc_agi" +class TestResolveServerDir: + """`_resolve_server_dir` resolves a relative server dir against cwd first, then the install root.""" + + def test_prefers_local_server_in_cwd(self, tmp_path: Path, monkeypatch) -> None: + local = tmp_path / "resources_servers" / "my_server" + local.mkdir(parents=True) + (local / "requirements.txt").write_text("nemo-gym\n") + monkeypatch.chdir(tmp_path) + assert _resolve_server_dir(Path("resources_servers/my_server")) == local + + def test_falls_back_to_install_root(self, tmp_path: Path, monkeypatch) -> None: + # Empty cwd (no local server) -> the built-in resolves under the install root. + monkeypatch.chdir(tmp_path) + rel = Path("resources_servers/arc_agi") + assert _resolve_server_dir(rel) == PARENT_DIR / rel + + def test_test_config_resolved_dir_path_uses_install_root(self, tmp_path: Path, monkeypatch) -> None: + monkeypatch.chdir(tmp_path) + cfg = TestConfig(entrypoint="resources_servers/arc_agi") + assert cfg.resolved_dir_path == PARENT_DIR / "resources_servers" / "arc_agi" + + class TestRunHelperShutdownReap: """RunHelper.shutdown must reap every server subprocess on every exit path.""" diff --git a/tests/unit_tests/test_cli_setup_command.py b/tests/unit_tests/test_cli_setup_command.py index 4f531eedc1..26d3a83075 100644 --- a/tests/unit_tests/test_cli_setup_command.py +++ b/tests/unit_tests/test_cli_setup_command.py @@ -267,6 +267,7 @@ def test_sanity(self, monkeypatch: MonkeyPatch) -> None: "my command", executable="/bin/bash", shell=True, + # Default (no project_root): only the server dir is on PYTHONPATH. env={"PYTHONPATH": "/my path", "UV_CACHE_DIR": "default uv cache dir"}, stdout="stdout", stderr="stderr", @@ -294,6 +295,28 @@ def test_custom_pythonpath(self, monkeypatch: MonkeyPatch) -> None: actual_args = Popen_mock.call_args assert expected_args == actual_args + def test_project_root_added_to_pythonpath(self, monkeypatch: MonkeyPatch) -> None: + # Opt-in: callers that need `resources_servers.`-style imports (e.g. gym env test) pass + # the project root, which is appended after the server dir. + Popen_mock, get_global_config_dict_mock = self._setup(monkeypatch) + + run_command( + command="my command", + working_dir_path=Path("/root/resources_servers/my_server"), + project_root=Path("/root"), + ) + + expected_args = call( + "my command", + executable="/bin/bash", + shell=True, + env={"PYTHONPATH": "/root/resources_servers/my_server:/root", "UV_CACHE_DIR": "default uv cache dir"}, + stdout="stdout", + stderr="stderr", + ) + actual_args = Popen_mock.call_args + assert expected_args == actual_args + def test_custom_uv_cache_dir(self, monkeypatch: MonkeyPatch) -> None: Popen_mock, get_global_config_dict_mock = self._setup(monkeypatch) @@ -430,7 +453,7 @@ def test_tee_logs_with_server_name(self, monkeypatch: MonkeyPatch) -> None: run_command( command="my command", - working_dir_path=Path("/my path"), + working_dir_path=Path("/root/resources_servers/my_server"), server_name="my_resources/my_server", ) @@ -438,7 +461,7 @@ def test_tee_logs_with_server_name(self, monkeypatch: MonkeyPatch) -> None: "set -o pipefail; (my command) 2>&1 | tee -a /tmp/gym_logs/my_resources_my_server.log", executable="/bin/bash", shell=True, - env={"PYTHONPATH": "/my path", "UV_CACHE_DIR": "default uv cache dir"}, + env={"PYTHONPATH": "/root/resources_servers/my_server", "UV_CACHE_DIR": "default uv cache dir"}, stdout="stdout", stderr="stderr", ) @@ -455,14 +478,14 @@ def test_tee_logs_falls_back_to_dir_name(self, monkeypatch: MonkeyPatch) -> None run_command( command="my command", - working_dir_path=Path("/my path"), + working_dir_path=Path("/root/resources_servers/my_server"), ) expected_args = call( - "set -o pipefail; (my command) 2>&1 | tee -a /tmp/gym_logs/my path.log", + "set -o pipefail; (my command) 2>&1 | tee -a /tmp/gym_logs/my_server.log", executable="/bin/bash", shell=True, - env={"PYTHONPATH": "/my path", "UV_CACHE_DIR": "default uv cache dir"}, + env={"PYTHONPATH": "/root/resources_servers/my_server", "UV_CACHE_DIR": "default uv cache dir"}, stdout="stdout", stderr="stderr", ) diff --git a/tests/unit_tests/test_global_config.py b/tests/unit_tests/test_global_config.py index 49f913d1f4..51ad5e8403 100644 --- a/tests/unit_tests/test_global_config.py +++ b/tests/unit_tests/test_global_config.py @@ -24,6 +24,8 @@ import nemo_gym.server_utils from nemo_gym import CACHE_DIR, WORKING_DIR from nemo_gym.config_types import ( + AlmostServerError, + ConfigError, ConfigMissingValuesError, ConfigPathNotFoundError, MalformedConfigPathsError, @@ -895,8 +897,10 @@ def hydra_main_wrapper(fn): hydra_main_mock.return_value = hydra_main_wrapper monkeypatch.setattr(nemo_gym.global_config.hydra, "main", hydra_main_mock) - with raises(ValueError, match="almost-server.*validation errors"): + # AlmostServerError is a ConfigError, so the CLI reports it cleanly (no traceback). + with raises(AlmostServerError, match="almost-server.*validation errors") as exc_info: get_global_config_dict() + assert isinstance(exc_info.value, ConfigError) def test_almost_servers_error_flag_bypasses_value_error(self, monkeypatch: MonkeyPatch) -> None: """