From 21e3f242336e34dcd93f68b841f17c4b1dfe27dd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 14 Aug 2026 18:10:44 +0000 Subject: [PATCH 1/5] fix(experimentalist): preflight the evaluator entrypoint in doctor and run nemo agents experimentalist doctor passed every check for an agent directory with no harbor_wrapper.py, the most common first-run blocker, and the run only failed once Harbor tried to import the wrapper. The entrypoint contract now lives in one module that both the Harbor evaluator and preflight read, so the shared check_artifacts suite resolves the effective evaluator.import_path against the local agent source. Doctor and run report the same required failure before anything is downloaded or evaluated. Signed-off-by: Cursor Agent --- .../src/nemo_experimentalist_plugin/cli.py | 2 + .../components/evaluator/entrypoint.py | 42 ++++++++++++++++ .../components/evaluator/harbor.py | 14 +++--- .../nemo_experimentalist_plugin/preflight.py | 48 ++++++++++++++++++- .../tests/test_cli_profile.py | 33 +++++++++++++ .../tests/test_experiment_cli.py | 9 +++- .../tests/test_preflight.py | 45 +++++++++++++++++ 7 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/cli.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/cli.py index ff5d6a4c3f..105b05d700 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/cli.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/cli.py @@ -270,6 +270,7 @@ async def _flow() -> str: task_template=plan.task_template, agent_source=plan.agent, storage=plan.config.storage.model_dump(exclude_unset=True), + evaluator=plan.config.evaluator, require_template=plan.insight is not None, probes=_PREFLIGHT_PROBES, ) @@ -412,6 +413,7 @@ def doctor( task_template=plan.task_template, agent_source=plan.agent, storage=plan.config.storage.model_dump(), + evaluator=plan.config.evaluator, require_template=plan.insight is not None, probes=_PREFLIGHT_PROBES, ) diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py new file mode 100644 index 0000000000..ec8387110d --- /dev/null +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py @@ -0,0 +1,42 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The agent entrypoint contract: one definition of what the evaluator imports. + +The evaluator imports ``import_path`` from the candidate agent directory and +nothing else. Preflight resolves the same reference against the agent source so +a missing wrapper is reported before a run instead of at the first trial. Both +sides read this module, so they cannot disagree. + +Importable in the CLI hot path: standard library only, no ``harbor``. +""" + +from importlib.machinery import ModuleSpec, PathFinder +from pathlib import Path + +DEFAULT_AGENT_IMPORT_PATH = "harbor_wrapper:WrappedAgent" + + +def split_import_path(import_path: str) -> tuple[str, str]: + """Split ``module[:attribute]`` into its normalized module and attribute.""" + module_name, _, attribute = import_path.partition(":") + module_name = module_name.strip().lstrip(".") + if not module_name: + raise ValueError("import_path module is required") + return module_name, attribute.strip() + + +def find_entrypoint_module(agent_dir: Path, module_name: str) -> Path | None: + """Return the file the evaluator would import for *module_name*, or None. + + Searches *agent_dir* alone, as the evaluator's scoped import does, and never + executes agent code. + """ + search = [str(agent_dir)] + spec: ModuleSpec | None = None + for part in module_name.split("."): + spec = PathFinder.find_spec(part, search) + if spec is None: + return None + search = list(spec.submodule_search_locations or ()) + return Path(spec.origin) if spec is not None and spec.origin is not None else None diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py index cfcbf609fe..9a42337c79 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py @@ -52,6 +52,10 @@ EvaluatorConfig, EvaluatorType, ) +from nemo_experimentalist_plugin.experimentalist.components.evaluator.entrypoint import ( + DEFAULT_AGENT_IMPORT_PATH, + split_import_path, +) from pydantic import Field @@ -187,7 +191,7 @@ class HarborEvaluatorConfig(EvaluatorConfig): environment_build_timeout_multiplier: float | None = Field(default=1.0) artifacts: list[str] = Field(default=[]) retry: RetryConfig = Field(default=RetryConfig(exclude_exceptions=set())) - import_path: str = Field(default="harbor_wrapper:WrappedAgent") + import_path: str = Field(default=DEFAULT_AGENT_IMPORT_PATH) trace_dir: str = Field(default=_TRACE_ARTIFACT_SOURCE) trace_format: Literal["otlp", "atif"] = Field( default="otlp", @@ -392,15 +396,11 @@ def _ensure_package(name: str, search_path: Path | None = None) -> None: def _scoped_import_path(agent_path: Path, import_path: str) -> tuple[str, str]: - module_name, separator, attribute = import_path.partition(":") - module_name = module_name.strip().lstrip(".") - if not module_name: - raise ValueError("import_path module is required") - + module_name, attribute = split_import_path(import_path) package_name = _agent_import_package(agent_path) _ensure_package(package_name, search_path=agent_path) scoped = f"{package_name}.{module_name}" - if separator: + if ":" in import_path: scoped = f"{scoped}:{attribute}" return scoped, package_name diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py index 29d636a66b..8b4151b0d7 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py @@ -15,6 +15,11 @@ from pathlib import Path import httpx +from nemo_experimentalist_plugin.experimentalist.components.evaluator.entrypoint import ( + DEFAULT_AGENT_IMPORT_PATH, + find_entrypoint_module, + split_import_path, +) from nemo_experimentalist_plugin.experimentalist.components.repository import ( _redact_url, looks_like_git, @@ -200,6 +205,7 @@ def check_artifacts( task_template: str | None = None, agent_source: str | None = None, storage: dict | None = None, + evaluator: dict | None = None, require_template: bool = True, probes: Probes | None = None, ) -> list[CheckResult]: @@ -221,7 +227,7 @@ def check_artifacts( effective_template = task_template or (profile.task_template if profile is not None else None) if require_template and effective_template is not None: results += _check_task_template(resolve_profile_path(effective_template, base_dir)) - results += _check_agent_source(profile, p, agent_source=agent_source, storage=storage) + results += _check_agent_source(profile, p, agent_source=agent_source, storage=storage, evaluator=evaluator) return results @@ -334,6 +340,7 @@ def _check_agent_source( *, agent_source: str | None = None, storage: dict | None = None, + evaluator: dict | None = None, ) -> list[CheckResult]: results: list[CheckResult] = [] source = agent_source or (profile.agent_source if profile is not None else None) @@ -366,6 +373,8 @@ def _check_agent_source( f"agent source dir missing: {path}", ) ) + if path.is_dir(): + results.append(_check_agent_entrypoint(path, evaluator)) # Effective storage flags from the auto path (resolved config, which may come # from --config); doctor falls back to reading the profile's inline dict or # path-form experiment_config without importing the loop chain. @@ -451,6 +460,43 @@ def _check_agent_source( return results +def _check_agent_entrypoint(agent_dir: Path, evaluator: dict | None) -> CheckResult: + """Resolve the evaluator's entrypoint module inside a local agent directory. + + The evaluator imports it from that directory alone, so a module it cannot + find there fails every trial of every candidate. A git source is unchecked: + the tree only exists after the clone. Git sources aside, this is the most + common first-run blocker, so it is required rather than advisory. + """ + configured = (evaluator or {}).get("import_path") + import_path = str(configured) if configured else DEFAULT_AGENT_IMPORT_PATH + try: + module_name, attribute = split_import_path(import_path) + except ValueError as exc: + return CheckResult( + name="agent-entrypoint", + group="agent-source", + status="fail", + severity="required", + message=f"evaluator import_path {import_path!r} names no module: {exc}", + hint="set evaluator.import_path to [:] in the experiment config", + ) + found = find_entrypoint_module(agent_dir, module_name) + expected = Path(*module_name.split(".")) + return make_check_result( + "agent-entrypoint", + "agent-source", + found is not None, + "required", + f"evaluator entrypoint {import_path} at {found}", + f"evaluator cannot import {module_name!r} from the agent source: {agent_dir}", + hint=( + f"add {expected}.py{f' defining {attribute}' if attribute else ''} to the agent directory, " + "or point evaluator.import_path in the experiment config at the module you have" + ), + ) + + def _check_insight_file( insight: str | None, selector: str | None, diff --git a/plugins/nemo-experimentalist/tests/test_cli_profile.py b/plugins/nemo-experimentalist/tests/test_cli_profile.py index 8c093780d8..fc41082841 100644 --- a/plugins/nemo-experimentalist/tests/test_cli_profile.py +++ b/plugins/nemo-experimentalist/tests/test_cli_profile.py @@ -56,10 +56,16 @@ def app(): return cli.ExperimentalistCLI().get_cli() +def write_harbor_wrapper(agent_dir: Path) -> None: + """Make *agent_dir* importable by the evaluator's default entrypoint.""" + (agent_dir / "harbor_wrapper.py").write_text("class WrappedAgent:\n pass\n", encoding="utf-8") + + @pytest.fixture() def profile_tree(tmp_path: Path) -> Path: for sub in ("evals/task_template", "evals/train", "evals/val"): (tmp_path / sub).mkdir(parents=True) + write_harbor_wrapper(tmp_path) (tmp_path / "optimizer.yaml").write_text( "agent: flight-planner\n" "task_template: ./evals/task_template\n" @@ -134,6 +140,7 @@ def test_experiment_all_flags_no_profile_still_works(app, tmp_path: Path, monkey monkeypatch.setattr(cli, "_PREFLIGHT_PROBES", quiet_probes()) for sub in ("agent", "t", "v"): (tmp_path / sub).mkdir() + write_harbor_wrapper(tmp_path / "agent") monkeypatch.chdir(tmp_path) result = runner.invoke( app, @@ -246,6 +253,7 @@ def now(*, tz): monkeypatch.setattr(cli, "run_experimentalist", recorder) for sub in ("agent", "t", "v"): (tmp_path / sub).mkdir() + write_harbor_wrapper(tmp_path / "agent") monkeypatch.chdir(tmp_path) args = [ "run", @@ -1045,6 +1053,31 @@ def test_doctor_validates_full_effective_experiment_config( assert "Traceback" not in result.output +@pytest.mark.parametrize("command", ["run", "doctor"]) +def test_missing_evaluator_entrypoint_blocks_both_commands( + app, + profile_tree: Path, + monkeypatch: pytest.MonkeyPatch, + command: str, +) -> None: + """Doctor reports the same missing wrapper that stops a run, not a clean bill.""" + write_task_toml(profile_tree) + (profile_tree / "harbor_wrapper.py").unlink() + recorder = RunRecorder() + monkeypatch.setattr(cli, "run_experimentalist", recorder) + monkeypatch.chdir(profile_tree) + args = [command] + if command == "run": + args += ["--no-insight", "-o", str(profile_tree / "out")] + + result = runner.invoke(app, args) + + assert result.exit_code == 1 + assert recorder.kwargs is None + assert "evaluator cannot import 'harbor_wrapper'" in result.output + assert "Traceback" not in result.output + + @pytest.mark.parametrize("command", ["run", "doctor"]) def test_env_file_encoding_failure_is_clean_and_actionable( app, diff --git a/plugins/nemo-experimentalist/tests/test_experiment_cli.py b/plugins/nemo-experimentalist/tests/test_experiment_cli.py index 6a37ad0ebb..936d12e5c9 100644 --- a/plugins/nemo-experimentalist/tests/test_experiment_cli.py +++ b/plugins/nemo-experimentalist/tests/test_experiment_cli.py @@ -133,12 +133,19 @@ def _make_dir(path: Path) -> Path: return path +def _make_agent_dir(path: Path) -> Path: + """An agent directory the evaluator can import its entrypoint from.""" + _make_dir(path) + (path / "harbor_wrapper.py").write_text("class WrappedAgent:\n pass\n", encoding="utf-8") + return path + + def _make_paths(tmp_path: Path) -> ExperimentCliPaths: template = _make_dir(tmp_path / "template") (template / "task.toml").write_text('[task]\nname = "org/test-template"\n', encoding="utf-8") (template / "instruction.md").write_text("Test task instructions.\n", encoding="utf-8") return ExperimentCliPaths( - agent=_make_dir(tmp_path / "agent"), + agent=_make_agent_dir(tmp_path / "agent"), train=_make_dir(tmp_path / "train"), validation=_make_dir(tmp_path / "validation"), template=template, diff --git a/plugins/nemo-experimentalist/tests/test_preflight.py b/plugins/nemo-experimentalist/tests/test_preflight.py index 8835edbdde..9debe7a36c 100644 --- a/plugins/nemo-experimentalist/tests/test_preflight.py +++ b/plugins/nemo-experimentalist/tests/test_preflight.py @@ -29,6 +29,7 @@ def make_probes(*, cmd_ok: bool = True, http: bool = True) -> Probes: def full_profile(tmp_path: Path, *, agent_source: str | None = None): tt = tmp_path / "evals" / "task_template" tt.mkdir(parents=True) + (tmp_path / "harbor_wrapper.py").write_text("class WrappedAgent:\n pass\n", encoding="utf-8") (tt / "task.toml").write_text('[task]\nname = "org/agent__template"\n', encoding="utf-8") (tt / "instruction.md").write_text("do the thing", encoding="utf-8") for sub in ("evals/train", "evals/val"): @@ -212,6 +213,50 @@ def test_local_agent_source_dir_stays_required(tmp_path: Path) -> None: assert src.status == "fail" +def test_missing_evaluator_entrypoint_is_required_failure(tmp_path: Path) -> None: + (tmp_path / "agent").mkdir() + + results = check_artifacts(full_profile(tmp_path, agent_source="./agent"), probes=make_probes()) + + entrypoint = next(r for r in results if r.name == "agent-entrypoint") + assert entrypoint.status == "fail" + assert entrypoint.severity == "required" + assert "harbor_wrapper.py" in (entrypoint.hint or "") + assert "WrappedAgent" in (entrypoint.hint or "") + + +@pytest.mark.parametrize( + ("import_path", "expected_status"), + [ + ("pkg.wrapper:Agent", "pass"), + ("pkg.missing:Agent", "fail"), + (":Agent", "fail"), + ], + ids=["configured", "configured-missing", "no-module"], +) +def test_configured_entrypoint_replaces_the_default(tmp_path: Path, import_path: str, expected_status: str) -> None: + (tmp_path / "agent" / "pkg").mkdir(parents=True) + (tmp_path / "agent" / "pkg" / "wrapper.py").write_text("class Agent:\n pass\n", encoding="utf-8") + + results = check_artifacts( + full_profile(tmp_path, agent_source="./agent"), + probes=make_probes(), + evaluator={"import_path": import_path}, + ) + + entrypoint = next(r for r in results if r.name == "agent-entrypoint") + assert entrypoint.status == expected_status + + +def test_git_agent_source_has_no_entrypoint_check(tmp_path: Path) -> None: + results = check_artifacts( + full_profile(tmp_path, agent_source="https://host/g/repo.git@main"), + probes=make_probes(), + ) + + assert not any(r.name == "agent-entrypoint" for r in results) + + @pytest.mark.parametrize( ("source", "expected_repo"), [ From 587c6b5e1196c1493a1bfbdf888f32ef5169b6e0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 14 Aug 2026 18:14:03 +0000 Subject: [PATCH 2/5] refactor(experimentalist): report an explicitly empty import_path instead of defaulting An empty evaluator.import_path is a configuration error the run raises, so preflight must not read it as 'unset' and check the default module instead. Signed-off-by: Cursor Agent --- .../src/nemo_experimentalist_plugin/preflight.py | 13 ++++++------- .../nemo-experimentalist/tests/test_preflight.py | 3 ++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py index 8b4151b0d7..e1f092115a 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py @@ -463,22 +463,21 @@ def _check_agent_source( def _check_agent_entrypoint(agent_dir: Path, evaluator: dict | None) -> CheckResult: """Resolve the evaluator's entrypoint module inside a local agent directory. - The evaluator imports it from that directory alone, so a module it cannot - find there fails every trial of every candidate. A git source is unchecked: - the tree only exists after the clone. Git sources aside, this is the most - common first-run blocker, so it is required rather than advisory. + The evaluator imports the module from that directory alone, so a module that + is absent there fails every trial of every candidate. Only local sources are + checked, because a git tree exists after the clone. """ configured = (evaluator or {}).get("import_path") - import_path = str(configured) if configured else DEFAULT_AGENT_IMPORT_PATH + import_path = DEFAULT_AGENT_IMPORT_PATH if configured is None else str(configured) try: module_name, attribute = split_import_path(import_path) - except ValueError as exc: + except ValueError: return CheckResult( name="agent-entrypoint", group="agent-source", status="fail", severity="required", - message=f"evaluator import_path {import_path!r} names no module: {exc}", + message=f"evaluator import_path {import_path!r} names no module", hint="set evaluator.import_path to [:] in the experiment config", ) found = find_entrypoint_module(agent_dir, module_name) diff --git a/plugins/nemo-experimentalist/tests/test_preflight.py b/plugins/nemo-experimentalist/tests/test_preflight.py index 9debe7a36c..629c6ae29b 100644 --- a/plugins/nemo-experimentalist/tests/test_preflight.py +++ b/plugins/nemo-experimentalist/tests/test_preflight.py @@ -231,8 +231,9 @@ def test_missing_evaluator_entrypoint_is_required_failure(tmp_path: Path) -> Non ("pkg.wrapper:Agent", "pass"), ("pkg.missing:Agent", "fail"), (":Agent", "fail"), + ("", "fail"), ], - ids=["configured", "configured-missing", "no-module"], + ids=["configured", "configured-missing", "no-module", "empty"], ) def test_configured_entrypoint_replaces_the_default(tmp_path: Path, import_path: str, expected_status: str) -> None: (tmp_path / "agent" / "pkg").mkdir(parents=True) From 621741ad5f2a69d3710d46d481ceede56524ba8a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 14 Aug 2026 18:30:02 +0000 Subject: [PATCH 3/5] fix(experimentalist): require : in one shared splitter Harbor's import_symbol rejects a path without both halves, so a colon-less evaluator.import_path passed doctor and then failed at the first trial. Moving the whole format rule into split_import_path closes that gap and drops the attribute branch _scoped_import_path carried. Signed-off-by: Cursor Agent --- .../components/evaluator/entrypoint.py | 25 +++++++++---------- .../components/evaluator/harbor.py | 5 +--- .../nemo_experimentalist_plugin/preflight.py | 15 ++++++----- .../experimentalist/test_evaluator_harbor.py | 7 +++--- .../tests/test_cli_profile.py | 5 +--- .../tests/test_experiment_cli.py | 11 +++----- .../tests/test_preflight.py | 5 ++-- 7 files changed, 30 insertions(+), 43 deletions(-) diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py index ec8387110d..999653a16b 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py @@ -3,27 +3,25 @@ """The agent entrypoint contract: one definition of what the evaluator imports. -The evaluator imports ``import_path`` from the candidate agent directory and -nothing else. Preflight resolves the same reference against the agent source so -a missing wrapper is reported before a run instead of at the first trial. Both -sides read this module, so they cannot disagree. - -Importable in the CLI hot path: standard library only, no ``harbor``. +Preflight reports a wrapper the evaluator could not import before a run instead +of at the first trial, so it needs this contract without importing ``harbor``: +whether harbor is importable is itself one of the checks. """ -from importlib.machinery import ModuleSpec, PathFinder +from importlib.machinery import PathFinder from pathlib import Path DEFAULT_AGENT_IMPORT_PATH = "harbor_wrapper:WrappedAgent" def split_import_path(import_path: str) -> tuple[str, str]: - """Split ``module[:attribute]`` into its normalized module and attribute.""" + """Split ``module:attribute``, the only form the evaluator can import.""" module_name, _, attribute = import_path.partition(":") module_name = module_name.strip().lstrip(".") - if not module_name: - raise ValueError("import_path module is required") - return module_name, attribute.strip() + attribute = attribute.strip() + if not module_name or not attribute: + raise ValueError("import_path must be :") + return module_name, attribute def find_entrypoint_module(agent_dir: Path, module_name: str) -> Path | None: @@ -33,10 +31,11 @@ def find_entrypoint_module(agent_dir: Path, module_name: str) -> Path | None: executes agent code. """ search = [str(agent_dir)] - spec: ModuleSpec | None = None + origin: str | None = None for part in module_name.split("."): spec = PathFinder.find_spec(part, search) if spec is None: return None search = list(spec.submodule_search_locations or ()) - return Path(spec.origin) if spec is not None and spec.origin is not None else None + origin = spec.origin + return Path(origin) if origin is not None else None diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py index 9a42337c79..2a984f6922 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py @@ -399,10 +399,7 @@ def _scoped_import_path(agent_path: Path, import_path: str) -> tuple[str, str]: module_name, attribute = split_import_path(import_path) package_name = _agent_import_package(agent_path) _ensure_package(package_name, search_path=agent_path) - scoped = f"{package_name}.{module_name}" - if ":" in import_path: - scoped = f"{scoped}:{attribute}" - return scoped, package_name + return f"{package_name}.{module_name}:{attribute}", package_name def _cleanup_scoped_imports(package_name: str) -> None: diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py index e1f092115a..2ae2f5d2cc 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py @@ -461,24 +461,23 @@ def _check_agent_source( def _check_agent_entrypoint(agent_dir: Path, evaluator: dict | None) -> CheckResult: - """Resolve the evaluator's entrypoint module inside a local agent directory. + """Resolve the evaluator's entrypoint inside a local agent directory. - The evaluator imports the module from that directory alone, so a module that - is absent there fails every trial of every candidate. Only local sources are - checked, because a git tree exists after the clone. + The evaluator imports it from that directory alone, so an entrypoint it + cannot find there fails every trial of every candidate. """ configured = (evaluator or {}).get("import_path") import_path = DEFAULT_AGENT_IMPORT_PATH if configured is None else str(configured) try: module_name, attribute = split_import_path(import_path) - except ValueError: + except ValueError as exc: return CheckResult( name="agent-entrypoint", group="agent-source", status="fail", severity="required", - message=f"evaluator import_path {import_path!r} names no module", - hint="set evaluator.import_path to [:] in the experiment config", + message=f"evaluator import_path {import_path!r} is unusable: {exc}", + hint="set evaluator.import_path in the experiment config to :", ) found = find_entrypoint_module(agent_dir, module_name) expected = Path(*module_name.split(".")) @@ -490,7 +489,7 @@ def _check_agent_entrypoint(agent_dir: Path, evaluator: dict | None) -> CheckRes f"evaluator entrypoint {import_path} at {found}", f"evaluator cannot import {module_name!r} from the agent source: {agent_dir}", hint=( - f"add {expected}.py{f' defining {attribute}' if attribute else ''} to the agent directory, " + f"add {expected}.py defining {attribute} to the agent directory, " "or point evaluator.import_path in the experiment config at the module you have" ), ) diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py b/plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py index c4c3b40dc8..468d407ba0 100644 --- a/plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py @@ -1292,9 +1292,10 @@ def test_safe_identifier_normal(): assert _safe_identifier("hello world") == "hello_world" -def test_scoped_import_path_empty_module(tmp_path): - with pytest.raises(ValueError, match="import_path module is required"): - _scoped_import_path(tmp_path, ":SomeClass") +@pytest.mark.parametrize("import_path", [":SomeClass", "harbor_wrapper", "harbor_wrapper:"]) +def test_scoped_import_path_requires_module_and_attribute(tmp_path, import_path): + with pytest.raises(ValueError, match="import_path must be :"): + _scoped_import_path(tmp_path, import_path) def test_cleanup_scoped_imports_removes_parent_attr(): diff --git a/plugins/nemo-experimentalist/tests/test_cli_profile.py b/plugins/nemo-experimentalist/tests/test_cli_profile.py index fc41082841..d9246aec7d 100644 --- a/plugins/nemo-experimentalist/tests/test_cli_profile.py +++ b/plugins/nemo-experimentalist/tests/test_cli_profile.py @@ -57,7 +57,6 @@ def app(): def write_harbor_wrapper(agent_dir: Path) -> None: - """Make *agent_dir* importable by the evaluator's default entrypoint.""" (agent_dir / "harbor_wrapper.py").write_text("class WrappedAgent:\n pass\n", encoding="utf-8") @@ -1063,8 +1062,7 @@ def test_missing_evaluator_entrypoint_blocks_both_commands( """Doctor reports the same missing wrapper that stops a run, not a clean bill.""" write_task_toml(profile_tree) (profile_tree / "harbor_wrapper.py").unlink() - recorder = RunRecorder() - monkeypatch.setattr(cli, "run_experimentalist", recorder) + monkeypatch.setattr(cli, "run_experimentalist", RunRecorder()) monkeypatch.chdir(profile_tree) args = [command] if command == "run": @@ -1073,7 +1071,6 @@ def test_missing_evaluator_entrypoint_blocks_both_commands( result = runner.invoke(app, args) assert result.exit_code == 1 - assert recorder.kwargs is None assert "evaluator cannot import 'harbor_wrapper'" in result.output assert "Traceback" not in result.output diff --git a/plugins/nemo-experimentalist/tests/test_experiment_cli.py b/plugins/nemo-experimentalist/tests/test_experiment_cli.py index 936d12e5c9..83a0ec8e4a 100644 --- a/plugins/nemo-experimentalist/tests/test_experiment_cli.py +++ b/plugins/nemo-experimentalist/tests/test_experiment_cli.py @@ -133,19 +133,14 @@ def _make_dir(path: Path) -> Path: return path -def _make_agent_dir(path: Path) -> Path: - """An agent directory the evaluator can import its entrypoint from.""" - _make_dir(path) - (path / "harbor_wrapper.py").write_text("class WrappedAgent:\n pass\n", encoding="utf-8") - return path - - def _make_paths(tmp_path: Path) -> ExperimentCliPaths: template = _make_dir(tmp_path / "template") (template / "task.toml").write_text('[task]\nname = "org/test-template"\n', encoding="utf-8") (template / "instruction.md").write_text("Test task instructions.\n", encoding="utf-8") + agent = _make_dir(tmp_path / "agent") + (agent / "harbor_wrapper.py").write_text("class WrappedAgent:\n pass\n", encoding="utf-8") return ExperimentCliPaths( - agent=_make_agent_dir(tmp_path / "agent"), + agent=agent, train=_make_dir(tmp_path / "train"), validation=_make_dir(tmp_path / "validation"), template=template, diff --git a/plugins/nemo-experimentalist/tests/test_preflight.py b/plugins/nemo-experimentalist/tests/test_preflight.py index 629c6ae29b..d97dd4286e 100644 --- a/plugins/nemo-experimentalist/tests/test_preflight.py +++ b/plugins/nemo-experimentalist/tests/test_preflight.py @@ -230,10 +230,9 @@ def test_missing_evaluator_entrypoint_is_required_failure(tmp_path: Path) -> Non [ ("pkg.wrapper:Agent", "pass"), ("pkg.missing:Agent", "fail"), - (":Agent", "fail"), - ("", "fail"), + ("pkg.wrapper", "fail"), ], - ids=["configured", "configured-missing", "no-module", "empty"], + ids=["configured", "configured-missing", "no-attribute"], ) def test_configured_entrypoint_replaces_the_default(tmp_path: Path, import_path: str, expected_status: str) -> None: (tmp_path / "agent" / "pkg").mkdir(parents=True) From 9d64d8d4eb433c0f7311854da6f4a96d8f4b04d4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 14 Aug 2026 18:33:17 +0000 Subject: [PATCH 4/5] docs(experimentalist): state plainly why the entrypoint contract has its own module Signed-off-by: Cursor Agent --- .../experimentalist/components/evaluator/entrypoint.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py index 999653a16b..4bb2b6671b 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py @@ -3,9 +3,9 @@ """The agent entrypoint contract: one definition of what the evaluator imports. -Preflight reports a wrapper the evaluator could not import before a run instead -of at the first trial, so it needs this contract without importing ``harbor``: -whether harbor is importable is itself one of the checks. +Preflight resolves the same reference the evaluator imports, so it reports a +missing wrapper before a run instead of at the first trial. It must not import +``harbor`` to do so, because whether harbor is importable is itself a check. """ from importlib.machinery import PathFinder From 7dd6db1f502b37eaa934b641ef9b802c5a06ec17 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 14 Aug 2026 19:39:18 +0000 Subject: [PATCH 5/5] fix(experimentalist): require the entrypoint attribute to be an identifier The evaluator reaches the attribute with getattr, so 'mod:Agent:extra' and 'mod:Outer.Inner' can never resolve. partition() let both through preflight and Harbor then failed at the first trial on the attribute name. Signed-off-by: Cursor Agent --- .../experimentalist/components/evaluator/entrypoint.py | 9 +++++++-- .../tests/experimentalist/test_evaluator_harbor.py | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py index 4bb2b6671b..0a7039b966 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/entrypoint.py @@ -15,11 +15,16 @@ def split_import_path(import_path: str) -> tuple[str, str]: - """Split ``module:attribute``, the only form the evaluator can import.""" + """Split ``module:attribute``, the only form the evaluator can import. + + The evaluator reaches the attribute with ``getattr``, so a name that is not + an identifier — absent, dotted, or holding a second ``:`` — can never + resolve. + """ module_name, _, attribute = import_path.partition(":") module_name = module_name.strip().lstrip(".") attribute = attribute.strip() - if not module_name or not attribute: + if not module_name or not attribute.isidentifier(): raise ValueError("import_path must be :") return module_name, attribute diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py b/plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py index 468d407ba0..57eb49ae30 100644 --- a/plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py @@ -1292,7 +1292,10 @@ def test_safe_identifier_normal(): assert _safe_identifier("hello world") == "hello_world" -@pytest.mark.parametrize("import_path", [":SomeClass", "harbor_wrapper", "harbor_wrapper:"]) +@pytest.mark.parametrize( + "import_path", + [":SomeClass", "harbor_wrapper", "harbor_wrapper:", "harbor_wrapper:WrappedAgent:extra"], +) def test_scoped_import_path_requires_module_and_attribute(tmp_path, import_path): with pytest.raises(ValueError, match="import_path must be :"): _scoped_import_path(tmp_path, import_path)