From bbde436a54ea1ee86cec71896070abcf67e33d6f Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Mon, 3 Aug 2026 16:49:33 -0500 Subject: [PATCH 1/3] Make analyst, experimentalist, and eval-author CLI agents-only. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop legacy top-level aliases and document only `nemo agents …` paths so the optimizer plugins match the platform agent CLI naming. Signed-off-by: Alec Khoury --- plugins/nemo-eval-author/README.md | 7 ++- plugins/nemo-eval-author/pyproject.toml | 3 -- .../src/nemo_eval_author_plugin/cli.py | 8 ++- plugins/nemo-eval-author/tests/test_cli.py | 35 +++++-------- plugins/nemo-experimentalist/AGENTS.md | 24 +++++---- plugins/nemo-experimentalist/README.md | 20 ++++---- plugins/nemo-experimentalist/pyproject.toml | 3 -- .../src/nemo_experimentalist_plugin/cli.py | 5 +- plugins/nemo-insights/README.md | 10 ++-- .../research-agent/tests/test_analyst_e2e.py | 7 +-- .../src/nemo_insights_plugin/analyst/cli.py | 7 +-- .../src/nemo_insights_plugin/cli.py | 11 ++--- plugins/nemo-insights/testbed/README.md | 2 +- plugins/nemo-insights/testbed/cli.py | 2 +- .../nemo-insights/tests/test_cli_profile.py | 49 ++++++++++--------- .../optimizer/InsightOpenModal/command.ts | 2 +- .../optimizer/InsightOpenModal/index.test.ts | 4 +- 17 files changed, 92 insertions(+), 107 deletions(-) diff --git a/plugins/nemo-eval-author/README.md b/plugins/nemo-eval-author/README.md index 7f22176cac..9414a77ff0 100644 --- a/plugins/nemo-eval-author/README.md +++ b/plugins/nemo-eval-author/README.md @@ -63,4 +63,9 @@ Experimentalist import, both tagged `TODO(eval-author-standalone)`: ahead of any Experimentalist agent, because those agents read the environment when their class body executes. -A `nemo eval-author` CLI that auto-loads this `.env` is not wired yet. +A `nemo agents eval-author` CLI is registered under `nemo.cli.agents` and +mounted by the agents plugin. Verb scaffolding is in place +(`discover`, `audit`, `propose`, `run`, `doctor`); bodies are still +placeholders until ASE-673–678 land. The CLI does not auto-load this `.env` +yet — set credentials in the environment (or rely on the transitional +`EXPERIMENTALIST_*` fallback) before invoking it. diff --git a/plugins/nemo-eval-author/pyproject.toml b/plugins/nemo-eval-author/pyproject.toml index 65dedcd6c3..33212fcb09 100644 --- a/plugins/nemo-eval-author/pyproject.toml +++ b/plugins/nemo-eval-author/pyproject.toml @@ -14,9 +14,6 @@ dependencies = [ "tomlkit>=0.13.3", ] -[project.entry-points."nemo.cli"] -eval-author = "nemo_eval_author_plugin.cli:EvalAuthorCLI" - [project.entry-points."nemo.cli.agents"] eval-author = "nemo_eval_author_plugin.cli:EvalAuthorCLI" diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/cli.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/cli.py index 7a4d4401b0..8e9b2d3f79 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/cli.py +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/cli.py @@ -3,9 +3,8 @@ """Eval Author plugin CLI — ``nemo agents eval-author ...`` subcommands. -The same class is registered under both ``nemo.cli.agents`` and ``nemo.cli``, so every -verb is reachable as ``nemo agents eval-author `` (canonical) and as ``nemo -eval-author `` (retained for backward compatibility). +Registered under ``nemo.cli.agents`` and mounted by ``AgentsCLI`` as +``nemo agents eval-author ``. Scaffolding. Every verb is registered under its final name so the command tree is discoverable and the child tickets have a landing spot, and each body exits non-zero @@ -28,8 +27,7 @@ def _not_implemented(ctx: typer.Context, ticket: str) -> NoReturn: """Fail loudly, so a placeholder verb can never be mistaken for a successful run. - The message quotes ``ctx.command_path`` rather than a hardcoded path, so it names - whichever of the two mount points the caller actually used. + The message quotes ``ctx.command_path`` rather than a hardcoded path. """ typer.echo(f"`{ctx.command_path}` is not implemented yet ({ticket}).", err=True) raise typer.Exit(code=1) diff --git a/plugins/nemo-eval-author/tests/test_cli.py b/plugins/nemo-eval-author/tests/test_cli.py index 7f15ddff5a..da9cb03c5d 100644 --- a/plugins/nemo-eval-author/tests/test_cli.py +++ b/plugins/nemo-eval-author/tests/test_cli.py @@ -6,8 +6,7 @@ The entry-point cases cover the ``pyproject.toml`` wiring that nothing else exercises. A typo in the key or the import path does not fail an import; it just makes ``nemo agents eval-author`` quietly missing from the CLI, which no unit test of this module would catch. -The class is registered twice — canonically under ``nemo.cli.agents`` and, for backward -compatibility, under ``nemo.cli`` — so both groups are asserted. +The class is registered only under ``nemo.cli.agents``. """ from importlib.metadata import EntryPoint, entry_points @@ -34,9 +33,9 @@ def app() -> typer.Typer: return cli.EvalAuthorCLI().get_cli() -def _eval_author_entry_point(group: str = "nemo.cli") -> EntryPoint: - matches = [entry for entry in entry_points(group=group) if entry.name == "eval-author"] - assert matches, f"no {group} entry point named 'eval-author'; reinstall the plugin with uv sync" +def _eval_author_entry_point() -> EntryPoint: + matches = [entry for entry in entry_points(group="nemo.cli.agents") if entry.name == "eval-author"] + assert matches, "no nemo.cli.agents entry point named 'eval-author'; reinstall the plugin with uv sync" return matches[0] @@ -56,16 +55,19 @@ def test_verb_refuses_to_run_and_names_its_ticket(app: typer.Typer, command: str assert ticket in result.output -@pytest.mark.parametrize("group", ["nemo.cli.agents", "nemo.cli"]) -def test_entry_point_key_matches_the_cli_name(group: str) -> None: +def test_entry_point_key_matches_the_cli_name() -> None: """Discovery rejects a plugin whose entry-point key differs from its ``name``.""" - assert _eval_author_entry_point(group).value == "nemo_eval_author_plugin.cli:EvalAuthorCLI" + assert _eval_author_entry_point().value == "nemo_eval_author_plugin.cli:EvalAuthorCLI" assert cli.EvalAuthorCLI.name == "eval-author" -@pytest.mark.parametrize("group", ["nemo.cli.agents", "nemo.cli"]) -def test_entry_point_loads_the_cli_class(group: str) -> None: - assert _eval_author_entry_point(group).load() is cli.EvalAuthorCLI +def test_entry_point_loads_the_cli_class() -> None: + assert _eval_author_entry_point().load() is cli.EvalAuthorCLI + + +def test_no_top_level_nemo_cli_entry_point() -> None: + matches = [entry for entry in entry_points(group="nemo.cli") if entry.name == "eval-author"] + assert matches == [] def _mounted_under_agents() -> typer.Typer: @@ -84,14 +86,3 @@ def test_verb_is_reachable_under_agents_and_names_that_path(command: str, ticket assert result.exit_code == 1, result.output assert f"`nemo agents eval-author {command}` is not implemented yet ({ticket})." in result.output - - -@pytest.mark.parametrize(("command", "ticket"), _PLACEHOLDER_VERBS) -def test_legacy_top_level_path_still_works(command: str, ticket: str) -> None: - root = typer.Typer() - root.add_typer(cli.EvalAuthorCLI().get_cli(), name="eval-author") - - result = runner.invoke(root, ["eval-author", command], prog_name="nemo") - - assert result.exit_code == 1, result.output - assert f"`nemo eval-author {command}` is not implemented yet ({ticket})." in result.output diff --git a/plugins/nemo-experimentalist/AGENTS.md b/plugins/nemo-experimentalist/AGENTS.md index 3b70c51f4b..c0df4af3f6 100644 --- a/plugins/nemo-experimentalist/AGENTS.md +++ b/plugins/nemo-experimentalist/AGENTS.md @@ -15,18 +15,15 @@ Inherited from the NeMo Platform monorepo that now hosts this plugin: ### 2026-07-31: Command group nested under `nemo agents` -The canonical path is `nemo agents experimentalist `. `ExperimentalistCLI` is +The only path is `nemo agents experimentalist `. `ExperimentalistCLI` is registered under the `nemo.cli.agents` entry-point group, which the `nemo-agents` -plugin's `AgentsCLI` discovers and mounts. +plugin's `AgentsCLI` discovers and mounts. There is no top-level +`nemo experimentalist` alias. -The `nemo.cli` registration stays, so `nemo experimentalist ` keeps working. Both -groups point at the same class, so a new verb is written once and appears under both — -do not add a second implementation for the legacy path. Docs, help text, and error -messages should name the `nemo agents` form; prefer `ctx.command_path` over a hardcoded -path when a message quotes the command back to the user. - -The analyst and Eval Author moved in the same change: `nemo agents analyst run` (was -`nemo insights analyze`) and `nemo agents eval-author `. +Analyst and Eval Author follow the same rule: `nemo agents analyst run` (was +`nemo insights analyze`) and `nemo agents eval-author `. Prefer +`ctx.command_path` over a hardcoded path when a message quotes the command back +to the user. ### 2026-07-28: Eval Author extracted to its own plugin, heading for standalone @@ -71,10 +68,11 @@ breaking rename with no compatibility aliases: - distribution `nemo-optimizer-plugin` → `nemo-experimentalist-plugin`, source path `src/nemo_optimizer_plugin` → `src/nemo_experimentalist_plugin` -- `OptimizerCLI` → `ExperimentalistCLI`, and both the `nemo.cli` and +- `OptimizerCLI` → `ExperimentalistCLI`, and the `nemo.cli.agents` and `nemo.skills` entry-point keys are now `experimentalist`, so the command is - `nemo experimentalist ...` -- the `experiment` verb is now `run`: `nemo experimentalist run` + `nemo agents experimentalist ...` (historically also briefly exposed as a + top-level `nemo experimentalist` alias; that alias is gone) +- the `experiment` verb is now `run`: `nemo agents experimentalist run` - `OPTIMIZER_API_BASE`, `OPTIMIZER_API_KEY`, `OPTIMIZER_{SMART,MID,FAST}_MODEL_NAME`, `OPTIMIZER_MODEL`, `NEMO_OPTIMIZER_E2E`, and `NEMO_OPTIMIZER_RUNTIME_CACHE` are now `EXPERIMENTALIST_*` / `NEMO_EXPERIMENTALIST_*` diff --git a/plugins/nemo-experimentalist/README.md b/plugins/nemo-experimentalist/README.md index 4ccd8c7f72..e38d8a8bb3 100644 --- a/plugins/nemo-experimentalist/README.md +++ b/plugins/nemo-experimentalist/README.md @@ -36,12 +36,12 @@ commit, currently one past `v0.0.6` that carries an MCP transport-timeout fix. The supported handoff is: ```text -nemo insights analyze → .nemo-optimizer/insights.yaml or Platform Insight ID - → nemo experimentalist doctor - → nemo experimentalist run +nemo agents analyst run → .nemo-optimizer/insights.yaml or Platform Insight ID + → nemo agents experimentalist doctor + → nemo agents experimentalist run ``` -Run `nemo insights analyze` using the Platform Insights plugin and its +Run `nemo agents analyst run` using the Platform Insights plugin and its documented trace, workspace, and output options. The producer may write the local profile default, `.nemo-optimizer/insights.yaml`, or persist an Insight on Platform and report its ID. The Experimentalist does not analyze traces, @@ -51,12 +51,12 @@ From an agent directory with an `optimizer.yaml` profile, validate the effective inputs: ```bash -$NEMO experimentalist doctor +$NEMO agents experimentalist doctor ``` ## Run the Experimentalist locally -`nemo experimentalist run` runs the local Experimentalist loop. It evaluates +`nemo agents experimentalist run` runs the local Experimentalist loop. It evaluates a baseline agent on Harbor-compatible train and validation datasets, proposes candidate mutations, and records its artifacts under the selected experiment directory. @@ -76,7 +76,7 @@ A single local Insight in `.nemo-optimizer/insights.yaml` is selected by default: ```bash -$NEMO experimentalist run +$NEMO agents experimentalist run ``` Use `--insight` to name another local file. Local files can contain one @@ -84,7 +84,7 @@ Insight object or an `insights` list. A list with multiple entries requires `--insight-id`, which accepts an exact ID, exact title, or zero-based index: ```bash -$NEMO experimentalist run \ +$NEMO agents experimentalist run \ --insight path/to/insights.yaml \ --insight-id 0 ``` @@ -92,7 +92,7 @@ $NEMO experimentalist run \ For an Insight persisted by Platform, pass its ID with its Platform location: ```bash -$NEMO experimentalist run \ +$NEMO agents experimentalist run \ --insight \ --workspace \ --base-url https:// @@ -108,7 +108,7 @@ Use `--no-insight` to bypass both an explicit Insight and the profile-local default. Supply a baseline agent when the profile does not provide one: ```bash -$NEMO experimentalist run \ +$NEMO agents experimentalist run \ --no-insight \ --agent path/to/agent \ --train-dataset path/to/train \ diff --git a/plugins/nemo-experimentalist/pyproject.toml b/plugins/nemo-experimentalist/pyproject.toml index 4b1eb0033e..842f9ff161 100644 --- a/plugins/nemo-experimentalist/pyproject.toml +++ b/plugins/nemo-experimentalist/pyproject.toml @@ -18,9 +18,6 @@ dependencies = [ "tomlkit>=0.13.3", ] -[project.entry-points."nemo.cli"] -experimentalist = "nemo_experimentalist_plugin.cli:ExperimentalistCLI" - [project.entry-points."nemo.cli.agents"] experimentalist = "nemo_experimentalist_plugin.cli:ExperimentalistCLI" diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/cli.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/cli.py index 3e324d6a77..72e384faeb 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/cli.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/cli.py @@ -3,9 +3,8 @@ """Experimentalist plugin CLI — ``nemo agents experimentalist ...`` subcommands. -The same class is registered under both ``nemo.cli.agents`` and ``nemo.cli``, so every -verb is reachable as ``nemo agents experimentalist `` (canonical) and as ``nemo -experimentalist `` (retained for backward compatibility). +Registered under ``nemo.cli.agents`` and mounted by ``AgentsCLI`` as +``nemo agents experimentalist ``. """ import asyncio diff --git a/plugins/nemo-insights/README.md b/plugins/nemo-insights/README.md index 5ca1ccf5fc..1a90e66cee 100644 --- a/plugins/nemo-insights/README.md +++ b/plugins/nemo-insights/README.md @@ -14,12 +14,12 @@ The plugin is installed by default through the root workspace's `enabled-plugins From an agent directory, Insights discovers `optimizer.yaml` in the current directory or its parents. Start by checking the profile and its environment, -then run analysis: +then run the analyst: ```bash cd -uv run nemo insights doctor -uv run nemo insights analyze +uv run nemo agents analyst doctor +uv run nemo agents analyst run ``` The profile contract consumed by Insights is deliberately small: @@ -48,7 +48,7 @@ With a discovered profile, analysis reads and writes the shared local output at `--insights-file-output` to use a different file explicitly. ```bash -uv run nemo insights analyze \ +uv run nemo agents analyst run \ --agent research-agent \ --workspace default \ --base-url http://localhost:8080 @@ -93,4 +93,4 @@ uv run ruff check plugins/nemo-insights ## Testbed The analyst-only testbed is in [`testbed/`](testbed/). It can replay pinned -Intake traces or run Tau2 benchmarks before invoking `nemo insights analyze`. +Intake traces or run Tau2 benchmarks before invoking `nemo agents analyst run`. diff --git a/plugins/nemo-insights/examples/research-agent/tests/test_analyst_e2e.py b/plugins/nemo-insights/examples/research-agent/tests/test_analyst_e2e.py index d0d21e285f..8519d31397 100644 --- a/plugins/nemo-insights/examples/research-agent/tests/test_analyst_e2e.py +++ b/plugins/nemo-insights/examples/research-agent/tests/test_analyst_e2e.py @@ -10,7 +10,7 @@ 3. Clear all spans for the test project. 4. Run the research agent on three questions (concurrently) so it logs traces to Intake. -5. Run the analyst agent (``nemo insights analyze``). +5. Run the analyst agent (``nemo agents analyst run``). 6. Assert the analyst created at least one Insight. Required setup (the test is **opt-in** because it costs real tokens and needs @@ -439,8 +439,9 @@ def test_analyst_creates_insight_end_to_end(platform_server: str) -> None: # no result = subprocess.run( _cli_cmd( "nemo", - "insights", - "analyze", + "agents", + "analyst", + "run", "--agent", TEST_AGENT, "--workspace", diff --git a/plugins/nemo-insights/src/nemo_insights_plugin/analyst/cli.py b/plugins/nemo-insights/src/nemo_insights_plugin/analyst/cli.py index 8196eeb2b1..454f83ced0 100644 --- a/plugins/nemo-insights/src/nemo_insights_plugin/analyst/cli.py +++ b/plugins/nemo-insights/src/nemo_insights_plugin/analyst/cli.py @@ -3,9 +3,10 @@ """Analyst CLI — ``nemo agents analyst ...`` subcommands. -The verbs are the module-level callbacks that back ``nemo insights analyze`` -and ``nemo insights doctor``, so the two command trees cannot drift. The -periodic-analysis and job surfaces stay on ``nemo insights``: they manage the +Registered under ``nemo.cli.agents`` and mounted by ``AgentsCLI`` as +``nemo agents analyst ``. Verb bodies live as module-level callbacks in +``nemo_insights_plugin.cli`` so the analyst implementation stays in one place. +Periodic-analysis and job surfaces stay on ``nemo insights``: they manage the plugin's scheduled runs rather than driving the analyst itself. """ diff --git a/plugins/nemo-insights/src/nemo_insights_plugin/cli.py b/plugins/nemo-insights/src/nemo_insights_plugin/cli.py index cb110bd628..83ae80da1f 100644 --- a/plugins/nemo-insights/src/nemo_insights_plugin/cli.py +++ b/plugins/nemo-insights/src/nemo_insights_plugin/cli.py @@ -3,10 +3,10 @@ """Insights CLI and contributed subcommands. -The module-level :func:`analyze` and :func:`doctor` callbacks are shared with -:class:`nemo_insights_plugin.analyst.cli.AnalystCLI`, which mounts them as the canonical -``nemo agents analyst run`` and ``nemo agents analyst doctor``. They stay registered here -as ``nemo insights analyze`` / ``nemo insights doctor`` for backward compatibility. +The module-level :func:`analyze` and :func:`doctor` callbacks are the verb bodies for +:class:`nemo_insights_plugin.analyst.cli.AnalystCLI` (``nemo agents analyst run`` / +``nemo agents analyst doctor``). This module's ``InsightsCLI`` keeps the periodic +``analysis`` surface and does not mount those agent verbs. """ import asyncio @@ -338,9 +338,6 @@ def _root() -> None: ) app.add_typer(analysis_app, name="analysis") - app.command("analyze")(analyze) - app.command("doctor")(doctor) - @analysis_app.command("enable") def enable_analysis( agent: str = typer.Option( diff --git a/plugins/nemo-insights/testbed/README.md b/plugins/nemo-insights/testbed/README.md index a547fbc625..07758f9aaa 100644 --- a/plugins/nemo-insights/testbed/README.md +++ b/plugins/nemo-insights/testbed/README.md @@ -4,7 +4,7 @@ # testbed — insights analyst test runner (maintainer tooling) Runs the Insights analyst against registered **subjects** and emits Insights. Think -"pytest for the analysis loop." This is dev tooling — it *drives* `nemo insights`; +"pytest for the analysis loop." This is dev tooling — it *drives* `nemo agents analyst`; it is not the product CLI and is not shipped in the wheel. ```bash diff --git a/plugins/nemo-insights/testbed/cli.py b/plugins/nemo-insights/testbed/cli.py index d781155dfc..fc3c6a9699 100644 --- a/plugins/nemo-insights/testbed/cli.py +++ b/plugins/nemo-insights/testbed/cli.py @@ -29,7 +29,7 @@ bundles (state-v1..v5) are restorable only from a pre-migration checkout; see testbed/README.md. -This drives the analyst (`nemo insights analyze`) against registered subjects; it is not +This drives the analyst (`nemo agents analyst run`) against registered subjects; it is not the product CLI and is not shipped in the wheel. """ diff --git a/plugins/nemo-insights/tests/test_cli_profile.py b/plugins/nemo-insights/tests/test_cli_profile.py index 418fcece24..82141b85fd 100644 --- a/plugins/nemo-insights/tests/test_cli_profile.py +++ b/plugins/nemo-insights/tests/test_cli_profile.py @@ -9,6 +9,7 @@ import pytest import typer from nemo_insights_plugin import cli +from nemo_insights_plugin.analyst.cli import AnalystCLI from nemo_insights_plugin.contracts.profile import DEFAULT_BASE_URL from nemo_insights_plugin.preflight import AnalysisProbes from nemo_platform import NeMoPlatformError @@ -30,7 +31,7 @@ async def __call__(self, **kwargs: object) -> str: @pytest.fixture def app() -> typer.Typer: - return cli.InsightsCLI().get_cli() + return AnalystCLI().get_cli() @pytest.fixture(autouse=True) @@ -82,7 +83,7 @@ def test_analyze_runs_flag_free_from_profile(app: typer.Typer, profile_tree: Pat monkeypatch.setattr(cli, "run_analyst", recorder) monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze"]) + result = runner.invoke(app, ["run"]) assert result.exit_code == 0, result.output assert recorder.kwargs is not None @@ -97,7 +98,7 @@ def test_analyze_flags_override_profile(app: typer.Typer, profile_tree: Path, mo monkeypatch.setattr(cli, "run_analyst", recorder) monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze", "--agent", "other", "--workspace", "other-ws"]) + result = runner.invoke(app, ["run", "--agent", "other", "--workspace", "other-ws"]) assert result.exit_code == 0, result.output assert recorder.kwargs is not None @@ -112,7 +113,7 @@ def test_profile_env_is_loaded_before_base_url_resolution(app: typer.Typer, prof (profile_tree / ".env").write_text("NMP_BASE_URL=https://platform.example\n", encoding="utf-8") monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze"]) + result = runner.invoke(app, ["run"]) assert result.exit_code == 0, result.output assert recorder.kwargs is not None @@ -145,7 +146,7 @@ async def record_workspace_probe(base_url: str, workspace: str, agent: str) -> b env_file.write_bytes(b"KEY=\xff") monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze"]) + result = runner.invoke(app, ["run"]) assert result.exit_code == 1 error_lines = [line for line in result.stderr.splitlines() if line.startswith("Error:")] @@ -287,7 +288,7 @@ def test_base_url_precedence_uses_only_nmp_base_url( monkeypatch.setenv(name, value) monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze", *arguments]) + result = runner.invoke(app, ["run", *arguments]) assert result.exit_code == 0, result.output assert recorder.kwargs is not None @@ -301,7 +302,7 @@ def test_explicit_profile_is_used_outside_profile_directory( monkeypatch.setattr(cli, "run_analyst", recorder) monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["analyze", "--profile", str(profile_tree / "optimizer.yaml")]) + result = runner.invoke(app, ["run", "--profile", str(profile_tree / "optimizer.yaml")]) assert result.exit_code == 0, result.output assert recorder.kwargs is not None @@ -313,7 +314,7 @@ def test_malformed_explicit_profile_errors(app: typer.Typer, tmp_path: Path, mon profile.write_text("agent: ''\n", encoding="utf-8") monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["analyze", "--profile", str(profile)]) + result = runner.invoke(app, ["run", "--profile", str(profile)]) assert result.exit_code != 0 assert "Invalid profile" in result.output @@ -327,7 +328,7 @@ def test_malformed_discovered_profile_warns_when_flags_are_complete( (tmp_path / "optimizer.yaml").write_text("agent: ''\n", encoding="utf-8") monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["analyze", "--agent", "other", "--workspace", "other-ws"]) + result = runner.invoke(app, ["run", "--agent", "other", "--workspace", "other-ws"]) assert result.exit_code == 0, result.output assert "warning:" in result.output @@ -340,7 +341,7 @@ def test_malformed_discovered_profile_warns_with_agent_only(app: typer.Typer, tm (tmp_path / "optimizer.yaml").write_text("agent: ''\n", encoding="utf-8") monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["analyze", "--agent", "other"]) + result = runner.invoke(app, ["run", "--agent", "other"]) assert result.exit_code == 0, result.output assert "warning:" in result.output @@ -357,7 +358,7 @@ def test_malformed_discovered_profile_still_loads_env_file(app: typer.Typer, tmp (tmp_path / ".env").write_text("INFERENCE_API_KEY=from-env-file\n", encoding="utf-8") monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["analyze", "--agent", "other"]) + result = runner.invoke(app, ["run", "--agent", "other"]) assert result.exit_code == 0, result.output assert os.environ["INFERENCE_API_KEY"] == "from-env-file" @@ -370,7 +371,7 @@ def test_malformed_discovered_profile_errors_without_agent(app: typer.Typer, tmp (tmp_path / "optimizer.yaml").write_text("agent: ''\n", encoding="utf-8") monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["analyze", "--workspace", "other-ws"]) + result = runner.invoke(app, ["run", "--workspace", "other-ws"]) assert result.exit_code != 0 assert "Invalid profile" in result.output @@ -385,7 +386,7 @@ def test_explicit_output_overrides_profile_default( monkeypatch.setattr(cli, "run_analyst", recorder) monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze", "--insights-file-output", str(output)]) + result = runner.invoke(app, ["run", "--insights-file-output", str(output)]) assert result.exit_code == 0, result.output assert recorder.kwargs is not None @@ -395,7 +396,7 @@ def test_explicit_output_overrides_profile_default( def test_missing_profile_and_agent_errors(app: typer.Typer, tmp_path: Path, monkeypatch) -> None: monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["analyze"]) + result = runner.invoke(app, ["run"]) assert result.exit_code != 0 assert "No --agent given and no optimizer.yaml profile found" in result.output @@ -417,7 +418,7 @@ def test_analyze_blocks_before_runner_when_preflight_fails( ) monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze"]) + result = runner.invoke(app, ["run"]) assert result.exit_code == 1 assert "INFERENCE_API_KEY not set" in result.output @@ -446,7 +447,7 @@ async def record_workspace_probe(base_url: str, workspace: str, agent: str) -> b ) monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze"]) + result = runner.invoke(app, ["run"]) assert result.exit_code == 0, result.output assert recorder.kwargs is not None @@ -472,7 +473,7 @@ async def fail_analysis(**kwargs: object) -> str: monkeypatch.setattr(cli, "run_analyst", fail_analysis) monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze"]) + result = runner.invoke(app, ["run"]) assert result.exit_code == 1 error_lines = [line for line in result.stderr.splitlines() if line.startswith("Error:")] @@ -493,7 +494,7 @@ async def fail_analysis(**kwargs: object) -> str: monkeypatch.setattr(cli, "run_analyst", fail_analysis) monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze"]) + result = runner.invoke(app, ["run"]) assert result.exit_code == 1 error_lines = [line for line in result.stderr.splitlines() if line.startswith("Error:")] @@ -523,7 +524,7 @@ def fail_to_construct(base_url: str | None) -> object: monkeypatch.setattr(cli, "make_client", fail_to_construct) monkeypatch.chdir(profile_tree) - result = runner.invoke(app, ["analyze"]) + result = runner.invoke(app, ["run"]) assert attempts == 1 assert result.exit_code == 1 @@ -563,7 +564,7 @@ def test_analyze_rejects_invalid_existing_insights_file_before_runner( output.write_bytes(payload) monkeypatch.setattr(cli, "run_analyst", recorder) monkeypatch.chdir(profile_tree) - arguments = ["analyze", "--insights-file-output", str(output)] if explicit else ["analyze"] + arguments = ["run", "--insights-file-output", str(output)] if explicit else ["run"] result = runner.invoke(app, arguments) @@ -606,7 +607,7 @@ def test_analyze_rejects_invalid_insights_records_before_runner( output.write_bytes(payload) monkeypatch.setattr(cli, "run_analyst", recorder) monkeypatch.chdir(profile_tree) - arguments = ["analyze", "--insights-file-output", str(output)] if explicit else ["analyze"] + arguments = ["run", "--insights-file-output", str(output)] if explicit else ["run"] result = runner.invoke(app, arguments) @@ -633,7 +634,7 @@ def test_analyze_accepts_existing_insights_file_without_insights_key( output.write_text("metadata: retained\n", encoding="utf-8") monkeypatch.setattr(cli, "run_analyst", recorder) monkeypatch.chdir(profile_tree) - arguments = ["analyze", "--insights-file-output", str(output)] if explicit else ["analyze"] + arguments = ["run", "--insights-file-output", str(output)] if explicit else ["run"] result = runner.invoke(app, arguments) @@ -641,7 +642,7 @@ def test_analyze_accepts_existing_insights_file_without_insights_key( assert recorder.kwargs is not None -@pytest.mark.parametrize("command", ["doctor", "analyze"]) +@pytest.mark.parametrize("command", ["doctor", "run"]) def test_commands_reject_invalid_utf8_agent_spec( app: typer.Typer, profile_tree: Path, @@ -659,7 +660,7 @@ def test_commands_reject_invalid_utf8_agent_spec( assert "Traceback" not in result.output -@pytest.mark.parametrize("command", ["doctor", "analyze"]) +@pytest.mark.parametrize("command", ["doctor", "run"]) def test_commands_reject_unreadable_agent_spec( app: typer.Typer, profile_tree: Path, diff --git a/web/packages/studio/src/routes/optimizer/InsightOpenModal/command.ts b/web/packages/studio/src/routes/optimizer/InsightOpenModal/command.ts index 72fdade41c..f14bc29b8b 100644 --- a/web/packages/studio/src/routes/optimizer/InsightOpenModal/command.ts +++ b/web/packages/studio/src/routes/optimizer/InsightOpenModal/command.ts @@ -4,7 +4,7 @@ const shellQuote = (value: string): string => value.replace(/'/g, "'\\''"); export const buildOptimizerExperimentCommand = (insightId: string, workspace: string): string => - `nemo optimizer experiment \\\n` + + `nemo agents experimentalist run \\\n` + ` --insight '${shellQuote(insightId)}' \\\n` + ` --train-dataset "" \\\n` + ` --validation-dataset "" \\\n` + diff --git a/web/packages/studio/src/routes/optimizer/InsightOpenModal/index.test.ts b/web/packages/studio/src/routes/optimizer/InsightOpenModal/index.test.ts index 94576a5711..38458ef53f 100644 --- a/web/packages/studio/src/routes/optimizer/InsightOpenModal/index.test.ts +++ b/web/packages/studio/src/routes/optimizer/InsightOpenModal/index.test.ts @@ -4,11 +4,11 @@ import { buildOptimizerExperimentCommand } from '@studio/routes/optimizer/InsightOpenModal/command'; describe('buildOptimizerExperimentCommand', () => { - it('uses the optimizer experiment contract and quotes the insight and workspace', () => { + it('uses the experimentalist run contract and quotes the insight and workspace', () => { const command = buildOptimizerExperimentCommand("insight-'quoted", "workspace-'quoted"); expect(command).toBe( - 'nemo optimizer experiment \\\n' + + 'nemo agents experimentalist run \\\n' + " --insight 'insight-'\\''quoted' \\\n" + ' --train-dataset "" \\\n' + ' --validation-dataset "" \\\n' + From d66a2dfda8ee7c9dca838835840ce4d6c02c18e4 Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Mon, 3 Aug 2026 17:11:23 -0500 Subject: [PATCH 2/3] Slim eval-author CLI tests to scaffold behavior only. Drop entry-point and fake AgentsCLI mount coverage; keep verb placeholders and a single ctx.command_path check. Signed-off-by: Alec Khoury --- plugins/nemo-eval-author/tests/test_cli.py | 53 +++++----------------- 1 file changed, 12 insertions(+), 41 deletions(-) diff --git a/plugins/nemo-eval-author/tests/test_cli.py b/plugins/nemo-eval-author/tests/test_cli.py index da9cb03c5d..ae6e71218b 100644 --- a/plugins/nemo-eval-author/tests/test_cli.py +++ b/plugins/nemo-eval-author/tests/test_cli.py @@ -1,15 +1,7 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -"""Scaffolding tests: the command tree exists, and every verb still refuses to run. - -The entry-point cases cover the ``pyproject.toml`` wiring that nothing else exercises. A -typo in the key or the import path does not fail an import; it just makes ``nemo agents -eval-author`` quietly missing from the CLI, which no unit test of this module would catch. -The class is registered only under ``nemo.cli.agents``. -""" - -from importlib.metadata import EntryPoint, entry_points +"""Scaffolding tests: the command tree exists, and every verb still refuses to run.""" import pytest import typer @@ -33,12 +25,6 @@ def app() -> typer.Typer: return cli.EvalAuthorCLI().get_cli() -def _eval_author_entry_point() -> EntryPoint: - matches = [entry for entry in entry_points(group="nemo.cli.agents") if entry.name == "eval-author"] - assert matches, "no nemo.cli.agents entry point named 'eval-author'; reinstall the plugin with uv sync" - return matches[0] - - def test_help_lists_every_verb(app: typer.Typer) -> None: result = runner.invoke(app, ["--help"]) @@ -55,34 +41,19 @@ def test_verb_refuses_to_run_and_names_its_ticket(app: typer.Typer, command: str assert ticket in result.output -def test_entry_point_key_matches_the_cli_name() -> None: - """Discovery rejects a plugin whose entry-point key differs from its ``name``.""" - assert _eval_author_entry_point().value == "nemo_eval_author_plugin.cli:EvalAuthorCLI" - assert cli.EvalAuthorCLI.name == "eval-author" - +def test_not_implemented_quotes_the_invoked_command_path() -> None: + """Placeholder messages use ``ctx.command_path``, not a hardcoded CLI string.""" + app = typer.Typer() -def test_entry_point_loads_the_cli_class() -> None: - assert _eval_author_entry_point().load() is cli.EvalAuthorCLI + @app.callback() + def _root() -> None: + """Force subcommand dispatch.""" + @app.command("probe") + def probe(ctx: typer.Context) -> None: + cli._not_implemented(ctx, "ASE-000") -def test_no_top_level_nemo_cli_entry_point() -> None: - matches = [entry for entry in entry_points(group="nemo.cli") if entry.name == "eval-author"] - assert matches == [] - - -def _mounted_under_agents() -> typer.Typer: - """The mount `AgentsCLI` performs, without importing the agents plugin.""" - agents = typer.Typer() - agents.add_typer(cli.EvalAuthorCLI().get_cli(), name="eval-author") - root = typer.Typer() - root.add_typer(agents, name="agents") - return root - - -@pytest.mark.parametrize(("command", "ticket"), _PLACEHOLDER_VERBS) -def test_verb_is_reachable_under_agents_and_names_that_path(command: str, ticket: str) -> None: - """The placeholder message must quote the path the caller typed, not a hardcoded one.""" - result = runner.invoke(_mounted_under_agents(), ["agents", "eval-author", command], prog_name="nemo") + result = runner.invoke(app, ["probe"], prog_name="nemo") assert result.exit_code == 1, result.output - assert f"`nemo agents eval-author {command}` is not implemented yet ({ticket})." in result.output + assert "`nemo probe` is not implemented yet (ASE-000)." in result.output From cfc7b3e8a6915d710c77a4624fa3e570a4eb8708 Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Tue, 4 Aug 2026 16:49:18 -0500 Subject: [PATCH 3/3] feat(experimentalist): hold out half the Insight suite for independent scoring The Eval Author produced a single Insight suite that served as both the optimizer's development feedback and its scoring evidence, so nothing measured whether an agent generalized to the production failures it had not already been tuned against. Split the finalized suite down the middle, giving the odd task to train, and materialize each half with its own content provenance so a candidate records which suite it was scored against. The validation half is hidden through the existing path-based holdout, so the coder cannot read it. Feed the train half's trials to the analyzer for trace-level diagnosis, and merge the validation half into Pareto selection as insight/-prefixed dimensions. Survivor selection, convergence, and winner choice now rank on the same merged axes, so a candidate whose only gain is on the held-out half still moves the front instead of looking stagnant. Author one shared metric key set across the Insight suite and the user's train and validation datasets, because comparing aggregates across splits requires identical keys. Two guards catch violations early: a verifier content-hash comparison fails authoring that left a task untouched, and a baseline key uniformity check fails at round 0 rather than crashing aggregation mid-run. Signed-off-by: Alec Khoury --- .../eval_author/REFERENCE.md | 34 +- .../eval_author/agent.py | 186 +++++++- .../eval_author/materialization.py | 217 ++++++++- .../eval_author/models.py | 16 +- .../tests/test_eval_author_agent.py | 197 +++++++- .../tests/test_eval_author_materialization.py | 137 +++++- .../tests/test_eval_author_repair_e2e.py | 40 +- .../nemo_experimentalist_plugin/entities.py | 116 ++++- .../experimentalist/components/analyzer.py | 31 +- .../components/holdout_utils.py | 13 +- .../components/insight_promotion.py | 84 ++-- .../experimentalist/components/loop.py | 371 +++++++++++---- .../experimentalist/components/models.py | 62 ++- .../experimentalist/components/terminator.py | 10 +- .../experimentalist/experiment_mirror.py | 17 +- .../experimentalist/test_holdout_utils.py | 99 ++++ .../test_insight_split_contract.py | 124 +++++ .../test_loop_insight_signal_invariants.py | 448 ++++++++++++++++++ .../test_loop_insight_suite.py | 303 +++++++++--- .../experimentalist/test_loop_reporting.py | 185 +++++--- .../experimentalist/test_selection_rewards.py | 146 ++++++ .../tests/experimentalist/test_terminator.py | 22 + .../tests/experimentalist/test_tools.py | 32 +- .../tests/test_experiment_mirror.py | 15 +- .../tests/test_experiment_mirror_mapping.py | 6 +- .../tests/test_experimentalist_analyzer.py | 78 ++- 26 files changed, 2624 insertions(+), 365 deletions(-) create mode 100644 plugins/nemo-experimentalist/tests/experimentalist/test_holdout_utils.py create mode 100644 plugins/nemo-experimentalist/tests/experimentalist/test_insight_split_contract.py create mode 100644 plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_signal_invariants.py create mode 100644 plugins/nemo-experimentalist/tests/experimentalist/test_selection_rewards.py diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md index aa3c584557..41aac3d0da 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md @@ -11,13 +11,31 @@ SPDX-License-Identifier: Apache-2.0 | Field | Type | Description | | --- | --- | --- | -| `train_dataset` | `Dataset` | Training dataset supplied to the run. Eval Author does not mutate it. | -| `validation_dataset` | `Dataset` | Validation dataset supplied to the run. Eval Author does not mutate it. | -| `insight_suite` | `Dataset \| None` | Finalized experiment-local Insight dataset for immediate evaluation by the optimization loop. | -| `insight_suite_identity` | `str \| None` | SHA-256 identity of the finalized Insight task and verifier content. | +| `train_dataset` | `Dataset` | Training dataset supplied to the run. Eval Author adds Insight metric keys to its verifiers; it changes no agent-visible input. | +| `validation_dataset` | `Dataset` | Validation dataset supplied to the run, augmented the same way. | +| `insight_train_suite` | `Dataset \| None` | Insight suite train half, at `dataset/insight-train`. Visible to the optimization loop. | +| `insight_train_suite_identity` | `str \| None` | SHA-256 identity of the train half's task and verifier content. | +| `insight_validation_suite` | `Dataset \| None` | Insight suite validation half, at `dataset/insight-validation`. Held out from optimization. | +| `insight_validation_suite_identity` | `str \| None` | SHA-256 identity of the validation half's task and verifier content. | | `summary` | `str` | Eval Author's analysis summary. | -When an Insight suite is materialized successfully, `insight_suite` and -`insight_suite_identity` are both populated. Callers can persist the identity -with candidate results and reuse those results only while the suite identity -continues to match. +## The two Insight halves + +The authored suite is materialized into two physically separate directories, +alternating tasks so any ordering bias in the source traces is spread across both. +The odd task goes to train. A single-task suite therefore leaves +`insight_validation_suite` as `None`. + +The halves carry different evidentiary weight. The train half is visible to the +optimizing agent, so its scores are adaptive/development feedback only. The +validation half is relocated to hidden storage and blocked from shell access, so +its scores are independent evidence and enter candidate ranking. + +Each half has its own identity computed over exactly its own tasks. Callers can +persist a half's identity with candidate results and reuse those results only +while that identity continues to match. The canonical authored suite stays under +`eval-and-optimize/eval_author//insight-suite/` as the provenance home. + +Both halves inherit one identical metric key set, shared with `train_dataset` and +`validation_dataset`, because the scores are compared against each other +downstream. A key present in one dataset but missing from another fails the run. diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/agent.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/agent.py index 780a372c10..bada52e258 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/agent.py +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/agent.py @@ -9,13 +9,21 @@ import asyncio import logging +from collections.abc import Mapping, Sequence +from dataclasses import dataclass from pathlib import Path from typing import Any # Populates EXPERIMENTALIST_* from AUTHOR_*, which the Experimentalist agent imports below # read when their class bodies execute. Must stay ahead of them; isort keeps it there. import nemo_eval_author_plugin._env_bridge # noqa: F401 -from nemo_eval_author_plugin.eval_author.materialization import InsightSuite +from nemo_eval_author_plugin.eval_author.materialization import ( + INSIGHT_TRAIN_SPLIT, + INSIGHT_VALIDATION_SPLIT, + InsightSuite, + materialize_insight_split, + verifier_hashes, +) from nemo_eval_author_plugin.eval_author.models import EvalAuthorConfig, EvalAuthorResult from nemo_eval_author_plugin.model_config import get_fast_model, get_smart_model from nemo_experimentalist_plugin.experimentalist.components import cache @@ -51,6 +59,97 @@ logger = logging.getLogger(__name__) +@dataclass(frozen=True) +class EvalAuthorDatasetValidationFailure: + """Validation failure for one Eval Author dataset split.""" + + split: str + error: DatasetValidationError + + +class EvalAuthorDatasetValidationError(DatasetValidationError): + """Aggregated validation failures across every dataset the Eval Author authored.""" + + def __init__(self, failures: list[EvalAuthorDatasetValidationFailure]) -> None: + self.failures = tuple(failures) + details = "\n".join(f"{failure.split} dataset:\n{failure.error}" for failure in failures) + super().__init__(f"Eval Author dataset validation failed:\n{details}") + + +@dataclass(frozen=True) +class EvalAuthorUnauthoredTasks: + """Tasks in one split whose verifier metric authoring never touched.""" + + split: str + task_ids: tuple[str, ...] + + +class EvalAuthorUnauthoredTasksError(DatasetValidationError): + """Raised when metric authoring skipped tasks, leaving their verifiers unchanged. + + Subclasses ``DatasetValidationError`` so the caller's repair loop treats a skipped + task like any other authoring defect and feeds this message back as feedback. + """ + + def __init__(self, unauthored: Sequence[EvalAuthorUnauthoredTasks]) -> None: + self.unauthored = tuple(unauthored) + details = "\n".join(f"{entry.split} dataset: {', '.join(entry.task_ids)}" for entry in unauthored) + super().__init__( + "Insight metric authoring left these task verifiers byte-identical, so they emit " + "none of the new metric keys and every downstream comparison against them fails:\n" + f"{details}\n" + "Add the same metric key set to each listed task's verifier. Every task in every " + "dataset must emit the identical key set." + ) + + +async def _validate_authored_datasets(splits: Sequence[tuple[str, Dataset]]) -> None: + """Validate every authored split, reporting which splits failed rather than only the first.""" + failures: list[EvalAuthorDatasetValidationFailure] = [] + for split, dataset in splits: + try: + await dataset.validate() + except DatasetValidationError as exc: + failures.append(EvalAuthorDatasetValidationFailure(split=split, error=exc)) + + if failures: + raise EvalAuthorDatasetValidationError(failures) from failures[0].error + + +def _assert_every_task_authored( + splits: Sequence[tuple[str, Dataset]], + baseline: Mapping[str, Mapping[str, str]], +) -> None: + """Fail when authoring left any task's verifier identical to its pre-authoring state. + + ``dataset.validate()`` only checks that each task is structurally sound, and a task + nobody touched always is. Comparing verifier hashes is what distinguishes "authored" + from "skipped", and it needs no knowledge of the metric's name. + + Without this the shared metric contract is enforced for the first time by + ``validate_insight_evaluation_result`` at baseline, one full evaluation later: a + skipped task costs a round of trials before anyone learns it was skipped, and a + skipped task in a single-task Insight half fails the run outright. + + Only pass splits whose tasks are rebuilt from source on every run. Re-running into an + existing experiment directory reuses already-staged copies of the user's datasets, so + their verifiers legitimately start out authored and an unchanged hash proves nothing. + """ + unauthored: list[EvalAuthorUnauthoredTasks] = [] + for split, dataset in splits: + before = baseline.get(split, {}) + unchanged = tuple( + task_id + for task_id, digest in verifier_hashes(dataset.list_tasks()).items() + if before.get(task_id) == digest + ) + if unchanged: + unauthored.append(EvalAuthorUnauthoredTasks(split=split, task_ids=unchanged)) + + if unauthored: + raise EvalAuthorUnauthoredTasksError(unauthored) + + class EvalAuthor(Agent, llm=get_smart_model()): """Insights are failure modes of an agent in production. @@ -107,15 +206,19 @@ async def author_insight_metrics( insight: Insight, diagnostics: list[tuple[str, Diagnostic]], insight_suite: Dataset, + train_dataset: Dataset, + validation_dataset: Dataset, runner_conventions: str, validation_feedback: str | None = None, ) -> str: - """Author verifier metrics for the materialized tasks that capture the insight. + """Author verifier metrics that capture the insight across every evaluated dataset. Args: insight: The insight whose failure mode the tasks should detect. diagnostics: Per-trace ``(trace_ref, Diagnostic)`` pairs for concrete evidence. insight_suite: The materialized tasks recreated from the Insight's production traces. + train_dataset: The user's train dataset, augmented with the same metric keys. + validation_dataset: The user's validation dataset, augmented with the same metric keys. runner_conventions: Summary of how this dataset's runner works (from ``discover_runner``). Use this as the authoritative reference for what artifacts exist at evaluation runtime, how tasks are structured, and how to add metrics. @@ -126,18 +229,23 @@ async def author_insight_metrics( Refer to ``self.context["dataset_documentation"]`` for the dataset-specific API and metric authoring conventions (file layout, how to add/remove/modify a metric). - **Scope: new grades on the materialized Insight tasks** + **Scope: every task in all three datasets** - Add at least one new Insight-specific metric key to every task in ``insight_suite``. - Use the same new metric key set and shared scoring semantics across the entire - suite. Preserve every existing verifier metric, including the task's ordinary - ``reward`` or ``score``; append the Insight signal instead of replacing the - task's original notion of success. + Add at least one new Insight-specific metric key to every task in ``insight_suite``, + ``train_dataset``, and ``validation_dataset``. A metric is only useful as a + suite-wide signal, not a per-sample patch. Preserve every existing verifier + metric, including the task's ordinary ``reward`` or ``score``; append the Insight + signal instead of replacing the task's original notion of success. - Only edit verifier files in the materialized Insight suite. Do not modify the - user's train or validation datasets, and do not change task instructions, - environments, solutions, or other agent-visible inputs. This work adds new - grades to the new rows; it does not add new agent output to old benchmark rows. + **The metric key set must be identical across all three datasets.** Every task + everywhere emits exactly the same new key names with the same scoring semantics. + This is a hard requirement, not a preference: the scores are compared against each + other downstream, and a key present in one dataset but missing from another fails + the run. + + Only add grades. Do not change task instructions, environments, solutions, or any + other agent-visible input in any of the three datasets. Adding a verifier metric is + additive; changing an instruction changes what the benchmark asks. Name each new metric after the root-cause behavior, not a trace id or surface symptom. Measure the current Harbor run from runtime artifacts such as OTLP @@ -146,13 +254,14 @@ async def author_insight_metrics( **Validate while authoring** - After every verifier edit, call ``await insight_suite.validate()``. This performs - evaluator-specific static checks without launching trials or executing verifier - code. If it raises ``DatasetValidationError``, use its task, path, and source - location diagnostics to repair the files, then call it again. Do not return until - the suite passes validation. If ``validation_feedback`` is provided, the caller's - mandatory validation found errors in the previous attempt; fix every reported - failure and revalidate the suite. + After every verifier edit, call ``await insight_suite.validate()``, + ``await train_dataset.validate()``, and ``await validation_dataset.validate()``. + These perform evaluator-specific static checks without launching trials or + executing verifier code. If any raises ``DatasetValidationError``, use its task, + path, and source location diagnostics to repair the files, then call it again. Do + not return until all three pass validation. If ``validation_feedback`` is provided, + the caller's mandatory validation found errors in the previous attempt; it names + which dataset failed. Fix every reported failure and revalidate all three. **Metric quality** @@ -179,8 +288,7 @@ async def author_insight_metrics( all required objects, not merely whether X appears in the final answer. Return a concise summary naming the new metric key(s), what they measure, and - which runtime evidence they score. The caller retains the materialized suite and - the user's unchanged train and validation datasets. + which runtime evidence they score. The caller retains all three datasets. """ # noqa: D413 ... @@ -343,15 +451,34 @@ async def _run( self.context["dataset_documentation"] = doc(type(materialized_dataset), inline_depth=1) runner_conventions = await self.discover_runner(materialized_dataset) + authored_splits = ( + ("insight", materialized_dataset), + ("train", train_dataset), + ("validation", validation_dataset), + ) + # Only the Insight suite: promote_local rebuilds it from the task template on every + # run, so an unchanged verifier there really does mean authoring skipped the task. + # The user's datasets are staged once and reused, so on a re-run they start out + # already authored and an unchanged hash would be a false accusation. + # + # Snapshot before the first pass and keep it: a repair attempt is still measured + # against the pre-authoring state, so a task every attempt skips stays flagged. + insight_verifiers_before_authoring = {"insight": verifier_hashes(materialized_dataset.list_tasks())} summary = await self.author_insight_metrics( insight, diagnostics, materialized_dataset, + train_dataset, + validation_dataset, runner_conventions, ) for repair_attempt in range(self._config.max_validation_repair_attempts + 1): try: - await materialized_dataset.validate() + await _validate_authored_datasets(authored_splits) + _assert_every_task_authored( + (("insight", materialized_dataset),), + insight_verifiers_before_authoring, + ) except DatasetValidationError as exc: if repair_attempt >= self._config.max_validation_repair_attempts: raise @@ -365,16 +492,27 @@ async def _run( insight, diagnostics, materialized_dataset, + train_dataset, + validation_dataset, runner_conventions, validation_feedback=str(exc), ) else: + # Split after authoring so both halves inherit the same metric keys. finalized_suite = insight_suite.finalize() + dataset_dir = self.experiment_dir / "dataset" + split = materialize_insight_split( + finalized_suite, + train_dir=dataset_dir / INSIGHT_TRAIN_SPLIT, + validation_dir=dataset_dir / INSIGHT_VALIDATION_SPLIT, + ) return EvalAuthorResult( train_dataset=train_dataset, validation_dataset=validation_dataset, - insight_suite=finalized_suite.dataset, - insight_suite_identity=finalized_suite.identity, + insight_train_suite=split.train.dataset if split.train else None, + insight_train_suite_identity=split.train.identity if split.train else None, + insight_validation_suite=split.validation.dataset if split.validation else None, + insight_validation_suite_identity=split.validation.identity if split.validation else None, summary=summary, ) diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py index 4e084dccb8..d7bbbb4dbc 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py @@ -11,6 +11,7 @@ import re import shutil import tomllib +from collections.abc import Iterable, Sequence from dataclasses import dataclass from pathlib import Path from typing import cast @@ -26,6 +27,13 @@ _METRIC_CONTRACT_VERSION = 1 _SLUG_RE = re.compile(r"[^a-z0-9]+") +# Directory names for the two halves. Experimentalist declares the same names in its +# holdout_utils to decide which half to hide; they are duplicated rather than imported +# to keep Eval Author's dependency on Experimentalist shrinking, and pinned together by +# test_insight_split_names_match_eval_author in the Experimentalist suite. +INSIGHT_TRAIN_SPLIT = "insight-train" +INSIGHT_VALIDATION_SPLIT = "insight-validation" + def _slug(value: str, *, fallback: str, max_length: int = 48) -> str: slug = _SLUG_RE.sub("-", value.lower()).strip("-")[:max_length].rstrip("-") @@ -67,6 +75,44 @@ def _verifier_dir(task_dir: Path) -> Path: raise ValueError(f"Materialized task has no verifier directory: {task_dir}") +def _scoring_dir(task_dir: Path) -> Path: + """Return the directory whose contents decide which metrics a task emits. + + Falls back to the whole task directory for datasets that do not follow Harbor's + verifier layout. A coarser hash still detects a task nobody touched, which is all + :func:`verifier_hashes` needs. + """ + try: + return _verifier_dir(task_dir) + except (OSError, ValueError, tomllib.TOMLDecodeError): + return task_dir + + +def verifier_hashes(tasks: Iterable[Task]) -> dict[str, str]: + """Return each task's verifier content hash, keyed by task id. + + Snapshot this before metric authoring and compare after: a verifier whose hash did + not move cannot have gained a metric key, so the comparison names exactly the tasks + authoring skipped without needing to know the metric's name. + + Tasks with no readable files on disk are omitted rather than hashed as empty. A task + this cannot inspect is not evidence that nobody authored it, and hashing them all to + the same empty digest would accuse every one of them. + """ + hashes: dict[str, str] = {} + for task in tasks: + if not task.uri: + continue + try: + task_dir = local_path_from_uri(task.uri, context="Authored task").resolve() + except ValueError: + continue + files = _file_hashes(_scoring_dir(task_dir)) + if files: + hashes[task.id] = f"sha256:{_canonical_digest(files)}" + return hashes + + def _content_provenance(suite_dir: Path, manifest: dict[str, object]) -> tuple[list[dict[str, object]], str, str]: raw_tasks = manifest.get("tasks") if not isinstance(raw_tasks, list): @@ -135,6 +181,30 @@ def _content_provenance(suite_dir: Path, manifest: dict[str, object]) -> tuple[l return tasks, scorer_identity, suite_identity +def _write_manifest(manifest_path: Path, manifest: dict[str, object]) -> None: + """Write a suite manifest atomically.""" + pending_path = manifest_path.with_suffix(".json.pending") + pending_path.write_text(json.dumps(manifest, indent=2, sort_keys=True) + "\n", encoding="utf-8") + os.replace(pending_path, manifest_path) + + +def _task_hashes(tasks: list[dict[str, object]]) -> dict[str, dict[str, str]]: + """Return per-task content and verifier hashes keyed by relative task path.""" + task_hashes: dict[str, dict[str, str]] = {} + for task in tasks: + task_path = task.get("path") + content_hash = task.get("content_hash") + verifier = task.get("verifier") + verifier_hash = verifier.get("content_hash") if isinstance(verifier, dict) else None + if not isinstance(task_path, str) or not isinstance(content_hash, str) or not isinstance(verifier_hash, str): + raise ValueError(f"Finalized Insight suite has invalid task provenance: {task!r}") + task_hashes[task_path] = { + "content_hash": content_hash, + "verifier_hash": verifier_hash, + } + return task_hashes + + @dataclass(frozen=True, slots=True) class StagedInsightTask: """One copied task template waiting to be filled and validated.""" @@ -311,9 +381,7 @@ def record_analysis(self, statuses: dict[str, tuple[str, str | None]]) -> None: task["analysis"] = {"status": status} if error is not None: task["analysis"]["error"] = error - pending_path = manifest_path.with_suffix(".json.pending") - pending_path.write_text(json.dumps(manifest, indent=2, sort_keys=True) + "\n", encoding="utf-8") - os.replace(pending_path, manifest_path) + _write_manifest(manifest_path, manifest) def finalize(self) -> FinalizedInsightSuite: """Persist content identities on the experiment-local authored suite.""" @@ -335,35 +403,17 @@ def finalize(self) -> FinalizedInsightSuite: "tasks": tasks, } ) - pending_path = manifest_path.with_suffix(".json.pending") - pending_path.write_text(json.dumps(manifest, indent=2, sort_keys=True) + "\n", encoding="utf-8") - os.replace(pending_path, manifest_path) + _write_manifest(manifest_path, manifest) dataset = HarborDataset.from_path( self.suite_dir, dataset_id=f"insight-{digest[:12]}", ) - task_hashes: dict[str, dict[str, str]] = {} - for task in tasks: - task_path = task.get("path") - content_hash = task.get("content_hash") - verifier = task.get("verifier") - verifier_hash = verifier.get("content_hash") if isinstance(verifier, dict) else None - if ( - not isinstance(task_path, str) - or not isinstance(content_hash, str) - or not isinstance(verifier_hash, str) - ): - raise ValueError(f"Finalized Insight suite has invalid task provenance: {task!r}") - task_hashes[task_path] = { - "content_hash": content_hash, - "verifier_hash": verifier_hash, - } dataset.metadata.update( { "insight_suite_identity": suite_identity, "insight_suite_scorer_identity": scorer_identity, - "insight_suite_task_hashes": task_hashes, + "insight_suite_task_hashes": _task_hashes(tasks), } ) return FinalizedInsightSuite( @@ -372,3 +422,124 @@ def finalize(self) -> FinalizedInsightSuite: path=self.suite_dir, dataset=dataset, ) + + +@dataclass(frozen=True, slots=True) +class InsightSuiteSplit: + """Train and validation halves materialized from one finalized Insight suite.""" + + train: FinalizedInsightSuite | None + validation: FinalizedInsightSuite | None + + +def split_insight_task_paths(task_paths: Sequence[str]) -> tuple[list[str], list[str]]: + """Alternate task paths into train and validation, giving the odd task to train. + + Alternating instead of cutting the ordered list in half keeps any ordering bias + in the source traces (recency, severity) spread across both halves. + """ + return list(task_paths[0::2]), list(task_paths[1::2]) + + +def _materialize_half( + *, + source_dir: Path, + manifest: dict[str, object], + task_paths: list[str], + destination: Path, + split: str, +) -> FinalizedInsightSuite | None: + """Copy one half's tasks to ``destination`` and stamp its own content identity.""" + if not task_paths: + return None + + raw_tasks = manifest.get("tasks") + if not isinstance(raw_tasks, list): + raise ValueError(f"Insight suite manifest has invalid tasks: {source_dir / 'manifest.json'}") + entries_by_path: dict[str, dict[str, object]] = {} + for raw_task in raw_tasks: + if not isinstance(raw_task, dict): + continue + task_entry = cast(dict[str, object], raw_task) + relative_path = task_entry.get("path") + if isinstance(relative_path, str): + entries_by_path[relative_path] = task_entry + if missing := [path for path in task_paths if path not in entries_by_path]: + raise ValueError(f"Insight suite split references unknown tasks: {missing}") + selected = [entries_by_path[path] for path in task_paths] + + if destination.exists(): + shutil.rmtree(destination) + destination.parent.mkdir(parents=True, exist_ok=True) + destination.mkdir() + for relative_path in task_paths: + shutil.copytree(source_dir / relative_path, destination / relative_path) + + half_manifest: dict[str, object] = {**manifest, "split": split, "tasks": selected} + tasks, scorer_identity, suite_identity = _content_provenance(destination, half_manifest) + digest = suite_identity.removeprefix("sha256:") + half_manifest.update( + { + "schema_version": _MANIFEST_SCHEMA_VERSION, + "content_hash_schema_version": _CONTENT_HASH_SCHEMA_VERSION, + "metric_contract_version": _METRIC_CONTRACT_VERSION, + "suite_identity": suite_identity, + "scorer": { + "identity": scorer_identity, + "metric_contract_version": _METRIC_CONTRACT_VERSION, + }, + "tasks": tasks, + } + ) + _write_manifest(destination / "manifest.json", half_manifest) + + dataset = HarborDataset.from_path(destination, dataset_id=f"{split}-{digest[:12]}") + dataset.metadata.update( + { + "insight_suite_identity": suite_identity, + "insight_suite_scorer_identity": scorer_identity, + "insight_suite_task_hashes": _task_hashes(tasks), + } + ) + return FinalizedInsightSuite( + identity=suite_identity, + scorer_identity=scorer_identity, + path=destination, + dataset=dataset, + ) + + +def materialize_insight_split( + finalized: FinalizedInsightSuite, + *, + train_dir: Path, + validation_dir: Path, +) -> InsightSuiteSplit: + """Materialize a finalized suite into two physically separate halves. + + Physical separation is required because holdout relocates whole directories; a + logical subset view would leave both halves interleaved in one directory where + the validation half could not be hidden from the optimizing agent. + + Either half is ``None`` when the split assigns it no tasks, which happens for + the validation half of a single-task suite. + """ + manifest = json.loads((finalized.path / "manifest.json").read_text(encoding="utf-8")) + task_paths = [task.id for task in finalized.dataset.list_tasks()] + train_paths, validation_paths = split_insight_task_paths(task_paths) + return InsightSuiteSplit( + train=_materialize_half( + source_dir=finalized.path, + manifest=manifest, + task_paths=train_paths, + destination=train_dir, + split=INSIGHT_TRAIN_SPLIT, + ), + validation=_materialize_half( + source_dir=finalized.path, + manifest=manifest, + task_paths=validation_paths, + destination=validation_dir, + split=INSIGHT_VALIDATION_SPLIT, + ), + ) diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/models.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/models.py index 83f5e6f86b..092f4aa6a2 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/models.py +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/models.py @@ -33,12 +33,20 @@ class EvalAuthorResult(BaseModel): train_dataset: Dataset validation_dataset: Dataset - insight_suite: Dataset | None = Field( + insight_train_suite: Dataset | None = Field( default=None, - description="Finalized experiment-local Insight dataset for use by the optimization loop.", + description="Insight suite train half, visible to the optimization loop as development feedback.", ) - insight_suite_identity: str | None = Field( + insight_train_suite_identity: str | None = Field( default=None, - description="SHA-256 identity of the finalized Insight task and verifier content.", + description="SHA-256 identity of the Insight train half's task and verifier content.", + ) + insight_validation_suite: Dataset | None = Field( + default=None, + description="Insight suite validation half, held out so its score is independent scoring evidence.", + ) + insight_validation_suite_identity: str | None = Field( + default=None, + description="SHA-256 identity of the Insight validation half's task and verifier content.", ) summary: str diff --git a/plugins/nemo-eval-author/tests/test_eval_author_agent.py b/plugins/nemo-eval-author/tests/test_eval_author_agent.py index e6177c7318..3c3e39e3a6 100644 --- a/plugins/nemo-eval-author/tests/test_eval_author_agent.py +++ b/plugins/nemo-eval-author/tests/test_eval_author_agent.py @@ -13,7 +13,7 @@ import pytest from nemo_eval_author_plugin.eval_author import agent as eval_author_module -from nemo_eval_author_plugin.eval_author.agent import EvalAuthor +from nemo_eval_author_plugin.eval_author.agent import EvalAuthor, EvalAuthorDatasetValidationError from nemo_eval_author_plugin.eval_author.models import EvalAuthorConfig from nemo_experimentalist_plugin.experimentalist.components.evaluator import ( Dataset, @@ -59,8 +59,9 @@ class _PipelineCalls: analyzer_init: list[_AnalyzerInitCall] analyzer_run: list[_AnalyzerRunCall] discovered_datasets: list[Dataset] - author_args: list[tuple[Insight, list[tuple[str, Diagnostic]], Dataset, str, str | None]] + author_args: list[tuple[Insight, list[tuple[str, Diagnostic]], Dataset, Dataset, Dataset, str, str | None]] suite_discards: int + split_dirs: list[tuple[Path, Path]] @dataclass @@ -112,7 +113,7 @@ def _prompt(method: Any) -> str: return " ".join(prompt.split()) -def test_eval_author_prompts_scope_metrics_to_materialized_insight_suite() -> None: +def test_eval_author_prompts_scope_metrics_across_all_three_datasets() -> None: discover_prompt = _prompt(EvalAuthor.discover_runner) author_prompt = _prompt(EvalAuthor.author_insight_metrics) @@ -120,11 +121,22 @@ def test_eval_author_prompts_scope_metrics_to_materialized_insight_suite() -> No assert "inspect the actual files" in discover_prompt assert "authoritative reference for what artifacts exist at evaluation runtime" in author_prompt assert "how tasks are structured, and how to add metrics" in author_prompt - assert "Add at least one new Insight-specific metric key to every task in ``insight_suite``" in author_prompt + assert ( + "Add at least one new Insight-specific metric key to every task in ``insight_suite``, " + "``train_dataset``, and ``validation_dataset``" in author_prompt + ) + assert "A metric is only useful as a suite-wide signal, not a per-sample patch." in author_prompt assert "Preserve every existing verifier metric" in author_prompt - assert "Do not modify the user's train or validation datasets" in author_prompt + assert "The metric key set must be identical across all three datasets." in author_prompt + assert "This is a hard requirement, not a preference" in author_prompt + assert ( + "Do not change task instructions, environments, solutions, or any other agent-visible input " + "in any of the three datasets." in author_prompt + ) assert "call ``await insight_suite.validate()``" in author_prompt - assert "fix every reported failure and revalidate the suite" in author_prompt + assert "``await train_dataset.validate()``" in author_prompt + assert "``await validation_dataset.validate()``" in author_prompt + assert "Fix every reported failure and revalidate all three." in author_prompt def test_eval_author_prompts_retain_root_cause_and_normalized_scoring_guidance() -> None: @@ -162,6 +174,7 @@ def _install_pipeline( discovered_datasets=[], author_args=[], suite_discards=0, + split_dirs=[], ) next_analyzer = 0 @@ -285,6 +298,8 @@ async def __call__( insight: Insight, diagnostics: list[tuple[str, Diagnostic]], insight_suite: Dataset, + train_dataset: Dataset, + validation_dataset: Dataset, runner_conventions: str, validation_feedback: str | None = None, ) -> str: @@ -293,17 +308,38 @@ async def __call__( insight, diagnostics, insight_suite, + train_dataset, + validation_dataset, runner_conventions, validation_feedback, ) ) return "authored insight metrics" + def fake_materialize_split(finalized: Any, *, train_dir: Path, validation_dir: Path) -> SimpleNamespace: + calls.split_dirs.append((train_dir, validation_dir)) + tasks = list(finalized.dataset.list_tasks()) + return SimpleNamespace( + train=SimpleNamespace( + dataset=Dataset(id="insight-train", tasks=tasks[0::2]), + identity="sha256:" + "1" * 64, + ), + validation=( + SimpleNamespace( + dataset=Dataset(id="insight-validation", tasks=tasks[1::2]), + identity="sha256:" + "2" * 64, + ) + if tasks[1::2] + else None + ), + ) + eval_author.fill_task_template = cast(Any, FillTaskTemplate()) eval_author.discover_runner = cast(Any, DiscoverRunner()) eval_author.author_insight_metrics = cast(Any, AuthorInsightMetrics()) monkeypatch.setattr(eval_author_module, "TraceAnalyzer", FakeTraceAnalyzer) monkeypatch.setattr(eval_author_module, "InsightSuite", FakeInsightSuite) + monkeypatch.setattr(eval_author_module, "materialize_insight_split", fake_materialize_split) return calls @@ -560,22 +596,34 @@ def fake_doc(dataset_type: type[Dataset], *, inline_depth: int) -> object: assert eval_author.context["dataset_documentation"] is documentation assert len(calls.discovered_datasets) == 1 materialized_dataset = calls.discovered_datasets[0] - assert result.insight_suite is materialized_dataset - assert result.insight_suite_identity == f"sha256:{'a' * 64}" assert materialized_dataset.id == "insight-suite" assert materialized_dataset is not train_dataset assert materialized_dataset is not validation_dataset + + # The loop consumes the halves, not the authored suite, which stays the provenance home. + assert calls.split_dirs == [(tmp_path / "dataset" / "insight-train", tmp_path / "dataset" / "insight-validation")] + assert result.insight_train_suite is not None + assert result.insight_train_suite.id == "insight-train" + assert result.insight_train_suite_identity == f"sha256:{'1' * 64}" + # A single trace leaves the validation half empty. + assert result.insight_validation_suite is None + assert result.insight_validation_suite_identity is None + assert len(calls.author_args) == 1 ( authored_insight, diagnostics, authored_suite, + authored_train, + authored_validation, runner_conventions, validation_feedback, ) = calls.author_args[0] assert authored_insight is insight assert diagnostics == [("trace-1", diagnostic)] assert authored_suite is materialized_dataset + assert authored_train is train_dataset + assert authored_validation is validation_dataset assert runner_conventions == "runner conventions" assert validation_feedback is None assert result.train_dataset is train_dataset @@ -619,6 +667,8 @@ async def __call__( insight: Insight, diagnostics: list[tuple[str, Diagnostic]], insight_suite: Dataset, + train_dataset: Dataset, + validation_dataset: Dataset, runner_conventions: str, validation_feedback: str | None = None, ) -> str: @@ -646,6 +696,41 @@ async def __call__( assert insight_dataset.validate_calls == 2 +@pytest.mark.asyncio +async def test_validation_failure_names_every_failing_split_not_just_the_first( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + eval_author = _eval_author(tmp_path, max_validation_repair_attempts=0) + insight_dataset = _RepairableDataset("insight-suite", "insight verifier is missing uses_required_tool") + train_dataset = _RepairableDataset("train", "train verifier is missing uses_required_tool") + validation_dataset = _RepairableDataset("validation", None) + _install_pipeline( + monkeypatch, + [_diagnostic("diagnostic")], + eval_author, + materialized_dataset=insight_dataset, + ) + monkeypatch.setattr(eval_author_module.cache, "store", lambda *args: None) + + with pytest.raises(EvalAuthorDatasetValidationError) as exc_info: + await eval_author.run( + _insight(["trace-1"]), + Path("agent"), + Task(id="template"), + train_dataset, + validation_dataset, + client=cast(Any, object()), + ) + + assert [failure.split for failure in exc_info.value.failures] == ["insight", "train"] + message = str(exc_info.value) + assert "insight dataset:\ninsight verifier is missing uses_required_tool" in message + assert "train dataset:\ntrain verifier is missing uses_required_tool" in message + # Every split is validated even after one fails, so the repair prompt sees the whole picture. + assert validation_dataset.validate_calls == 1 + + @pytest.mark.asyncio async def test_run_raises_after_validation_repair_budget_is_exhausted( tmp_path: Path, @@ -678,6 +763,102 @@ async def test_run_raises_after_validation_repair_budget_is_exhausted( assert insight_dataset.validate_calls == 2 +def _authored_task(dataset_dir: Path, task_id: str) -> Task: + """Write a minimal Harbor task whose verifier emits only the task's own reward.""" + task_dir = dataset_dir / task_id + verifier_dir = task_dir / "tests" + verifier_dir.mkdir(parents=True) + (task_dir / "task.toml").write_text(f'[task]\nname = "local/{task_id}"\n', encoding="utf-8") + (verifier_dir / "test.sh").write_text("#!/bin/sh\npython3 /tests/evaluate.py\n", encoding="utf-8") + return Task(id=task_id, uri=task_dir.as_uri()) + + +def _add_metric(dataset_dir: Path, task_id: str) -> None: + """Augment one task's verifier the way metric authoring is supposed to.""" + verifier_dir = dataset_dir / task_id / "tests" + (verifier_dir / "check_escalation_restraint.py").write_text("print('escalation_restraint=1.0')\n", encoding="utf-8") + (verifier_dir / "test.sh").write_text( + "#!/bin/sh\npython3 /tests/evaluate.py\npython3 /tests/check_escalation_restraint.py\n", + encoding="utf-8", + ) + + +def test_a_task_authoring_skipped_is_named_rather_than_silently_unscored(tmp_path: Path) -> None: + # The failure this prevents: authoring augments most tasks and misses one, every + # structural check still passes because a skipped task is a perfectly valid task, and + # nobody finds out until that task evaluates without the shared metric key. When the + # skipped task is the only one in an Insight half, the half scores nothing at all. + insight_dir = tmp_path / "insight" + train_dir = tmp_path / "train" + insight_tasks = [_authored_task(insight_dir, "001-alpha"), _authored_task(insight_dir, "002-beta")] + train_tasks = [_authored_task(train_dir, "train-1")] + splits = ( + ("insight", Dataset(id="insight-suite", tasks=insight_tasks)), + ("train", Dataset(id="train", tasks=train_tasks)), + ) + before = {split: eval_author_module.verifier_hashes(dataset.list_tasks()) for split, dataset in splits} + + _add_metric(insight_dir, "001-alpha") + _add_metric(train_dir, "train-1") + + with pytest.raises(eval_author_module.EvalAuthorUnauthoredTasksError) as exc_info: + eval_author_module._assert_every_task_authored(splits, before) + + assert [entry.split for entry in exc_info.value.unauthored] == ["insight"] + assert exc_info.value.unauthored[0].task_ids == ("002-beta",) + # The message becomes the repair prompt, so it has to name the split and the task. + assert "insight dataset: 002-beta" in str(exc_info.value) + assert "001-alpha" not in str(exc_info.value) + + _add_metric(insight_dir, "002-beta") + eval_author_module._assert_every_task_authored(splits, before) + + +def test_unauthored_task_check_is_a_dataset_validation_error_so_repair_retries_it() -> None: + # The repair loop catches DatasetValidationError. Raising anything else would turn a + # recoverable authoring miss into a hard run failure with no repair attempt. + assert issubclass(eval_author_module.EvalAuthorUnauthoredTasksError, DatasetValidationError) + + +def test_the_check_detects_change_only_so_it_must_not_see_already_authored_splits(tmp_path: Path) -> None: + # This is a pure change detector: a task that already carries the metric but did not + # move this pass is still reported. That is correct for the Insight suite, which + # promote_local rebuilds from the task template every run, and wrong for the user's + # datasets, which dataset_staging._stage leaves in place once staged. Re-running into + # an existing experiment directory therefore hands back train/validation copies the + # previous run already authored, which is why run() scopes this to the Insight suite. + dataset_dir = tmp_path / "already-authored" + task = _authored_task(dataset_dir, "train-1") + _add_metric(dataset_dir, "train-1") + splits = (("train", Dataset(id="train", tasks=[task])),) + baseline = {"train": eval_author_module.verifier_hashes([task])} + + with pytest.raises(eval_author_module.EvalAuthorUnauthoredTasksError): + eval_author_module._assert_every_task_authored(splits, baseline) + + +def test_tasks_with_no_files_on_disk_are_not_accused_of_being_unauthored(tmp_path: Path) -> None: + # Remote or synthetic tasks hash to nothing. Treating "cannot inspect" as "identical" + # would fail every run whose datasets this cannot read off local disk. + splits = ( + ( + "train", + Dataset( + id="train", + tasks=[ + Task(id="missing", uri=(tmp_path / "absent").as_uri()), + Task(id="remote", uri="s3://bucket/task"), + Task(id="no-uri"), + ], + ), + ), + ) + before = {split: eval_author_module.verifier_hashes(dataset.list_tasks()) for split, dataset in splits} + + assert before == {"train": {}} + eval_author_module._assert_every_task_authored(splits, before) + + def test_eval_author_config_defaults_and_bounds_validation_repair_attempts() -> None: # Experimentalist's loop config asserts this default too, but from the other side of # the plugin boundary; owning it here is what keeps the default a plugin contract. diff --git a/plugins/nemo-eval-author/tests/test_eval_author_materialization.py b/plugins/nemo-eval-author/tests/test_eval_author_materialization.py index a110fcfcf0..6d85fc7fb0 100644 --- a/plugins/nemo-eval-author/tests/test_eval_author_materialization.py +++ b/plugins/nemo-eval-author/tests/test_eval_author_materialization.py @@ -11,7 +11,14 @@ import pytest from nemo_eval_author_plugin.eval_author import materialization as materialization_module -from nemo_eval_author_plugin.eval_author.materialization import InsightSuite +from nemo_eval_author_plugin.eval_author.materialization import ( + INSIGHT_TRAIN_SPLIT, + INSIGHT_VALIDATION_SPLIT, + FinalizedInsightSuite, + InsightSuite, + materialize_insight_split, + verifier_hashes, +) from nemo_experimentalist_plugin.experimentalist.components.evaluator import Task from nemo_experimentalist_plugin.experimentalist.components.evaluator.harbor import HarborDataset @@ -276,3 +283,131 @@ def build(instruction: str, verifier_suffix: str = "") -> tuple[str, str]: assert changed_verifier_identity != first_identity assert changed_scorer_identity != first_scorer_identity assert list((tmp_path / "eval-and-optimize" / "eval_author").glob("*/artifacts")) == [] + + +def _finalize_suite(tmp_path: Path, trace_count: int) -> FinalizedInsightSuite: + template = _write_template(tmp_path / "template") + refs = [f"trace-{index}" for index in range(1, trace_count + 1)] + suite = InsightSuite(experiment_dir=tmp_path, insight_id="insight-1", task_template=template) + staged = suite.stage(refs) + for task in staged: + (task.path / "instruction.md").write_text(f"Reproduce {task.trace_ref}.\n", encoding="utf-8") + suite.validate(task) + suite.promote_local(refs, staged) + return suite.finalize() + + +@pytest.mark.parametrize( + ("trace_count", "expected_train", "expected_validation"), + [(1, 1, 0), (2, 1, 1), (5, 3, 2), (6, 3, 3)], +) +def test_split_alternates_and_gives_the_odd_task_to_train( + tmp_path: Path, + trace_count: int, + expected_train: int, + expected_validation: int, +) -> None: + finalized = _finalize_suite(tmp_path, trace_count) + all_task_ids = [task.id for task in finalized.dataset.list_tasks()] + + split = materialize_insight_split( + finalized, + train_dir=tmp_path / "dataset" / INSIGHT_TRAIN_SPLIT, + validation_dir=tmp_path / "dataset" / INSIGHT_VALIDATION_SPLIT, + ) + + train_ids = [task.id for task in split.train.dataset.list_tasks()] if split.train else [] + validation_ids = [task.id for task in split.validation.dataset.list_tasks()] if split.validation else [] + assert (len(train_ids), len(validation_ids)) == (expected_train, expected_validation) + assert train_ids == all_task_ids[0::2] + assert validation_ids == all_task_ids[1::2] + # An empty half is returned as None rather than an unscoreable zero-task dataset. + assert (split.validation is None) == (expected_validation == 0) + + +def test_split_is_deterministic_across_repeated_materialization(tmp_path: Path) -> None: + finalized = _finalize_suite(tmp_path, 5) + dataset_dir = tmp_path / "dataset" + + def materialize() -> tuple[list[str], list[str], str, str]: + split = materialize_insight_split( + finalized, + train_dir=dataset_dir / INSIGHT_TRAIN_SPLIT, + validation_dir=dataset_dir / INSIGHT_VALIDATION_SPLIT, + ) + assert split.train is not None and split.validation is not None + return ( + [task.id for task in split.train.dataset.list_tasks()], + [task.id for task in split.validation.dataset.list_tasks()], + split.train.identity, + split.validation.identity, + ) + + assert materialize() == materialize() + + +def test_each_half_carries_its_own_provenance(tmp_path: Path) -> None: + finalized = _finalize_suite(tmp_path, 4) + dataset_dir = tmp_path / "dataset" + + split = materialize_insight_split( + finalized, + train_dir=dataset_dir / INSIGHT_TRAIN_SPLIT, + validation_dir=dataset_dir / INSIGHT_VALIDATION_SPLIT, + ) + + assert split.train is not None and split.validation is not None + # Distinct identities are what keeps the per-half evaluation cache from confusing them. + assert split.train.identity != split.validation.identity + assert split.train.identity != finalized.identity + assert split.train.scorer_identity != split.validation.scorer_identity + for half, expected_path in ( + (split.train, dataset_dir / INSIGHT_TRAIN_SPLIT), + (split.validation, dataset_dir / INSIGHT_VALIDATION_SPLIT), + ): + assert half.path == expected_path + manifest = json.loads((half.path / "manifest.json").read_text(encoding="utf-8")) + assert manifest["suite_identity"] == half.identity + assert manifest["scorer"]["identity"] == half.scorer_identity + assert manifest["insight_id"] == "insight-1" + task_ids = [task.id for task in half.dataset.list_tasks()] + assert [task["path"] for task in manifest["tasks"]] == task_ids + assert half.dataset.metadata["insight_suite_identity"] == half.identity + task_hashes = half.dataset.metadata["insight_suite_task_hashes"] + assert isinstance(task_hashes, dict) + assert set(task_hashes) == set(task_ids) + assert all(task.uri.startswith(half.path.as_uri()) for task in half.dataset.list_tasks()) + + +def test_verifier_hash_tracks_the_verifier_so_edits_elsewhere_cannot_mask_a_skipped_task( + tmp_path: Path, +) -> None: + # Hashing the whole task directory would let any incidental edit — a rewritten + # instruction, a re-stamped task.toml — make a task with an untouched verifier look + # authored, which is exactly the case the hash exists to catch. + finalized = _finalize_suite(tmp_path, 2) + tasks = list(finalized.dataset.list_tasks()) + before = verifier_hashes(tasks) + assert len(before) == 2 + + untouched, augmented = (Path(task.uri.removeprefix("file://")) for task in tasks) + (untouched / "instruction.md").write_text("Rewritten instruction, same verifier.\n", encoding="utf-8") + (augmented / "tests" / "check_restraint.py").write_text("print('restraint=1.0')\n", encoding="utf-8") + + after = verifier_hashes(tasks) + assert after[tasks[0].id] == before[tasks[0].id] + assert after[tasks[1].id] != before[tasks[1].id] + + +def test_split_leaves_the_authored_suite_as_the_provenance_home(tmp_path: Path) -> None: + finalized = _finalize_suite(tmp_path, 3) + before = json.loads((finalized.path / "manifest.json").read_text(encoding="utf-8")) + + materialize_insight_split( + finalized, + train_dir=tmp_path / "dataset" / INSIGHT_TRAIN_SPLIT, + validation_dir=tmp_path / "dataset" / INSIGHT_VALIDATION_SPLIT, + ) + + assert json.loads((finalized.path / "manifest.json").read_text(encoding="utf-8")) == before + assert len(HarborDataset.from_path(finalized.path).list_tasks()) == 3 diff --git a/plugins/nemo-eval-author/tests/test_eval_author_repair_e2e.py b/plugins/nemo-eval-author/tests/test_eval_author_repair_e2e.py index 3dfbf4cfb0..2ded0a6441 100644 --- a/plugins/nemo-eval-author/tests/test_eval_author_repair_e2e.py +++ b/plugins/nemo-eval-author/tests/test_eval_author_repair_e2e.py @@ -210,11 +210,18 @@ async def test_gpt5_mini_repairs_malformed_harbor_verifiers( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """GPT-5 mini repairs try-without-except failures across an Insight suite.""" + """GPT-5 mini repairs try-without-except failures across every dataset it authored.""" insight_suite_dir = tmp_path / "insight-suite" _write_malformed_task(insight_suite_dir, "task-a") _write_malformed_task(insight_suite_dir, "task-b") insight_suite = HarborDataset.from_path(insight_suite_dir) + # Authoring now spans the user's datasets too, so the repair loop has to reach them. + train_dir = tmp_path / "dataset" / "train" + validation_dir = tmp_path / "dataset" / "validation" + _write_malformed_task(train_dir, "train-task") + _write_malformed_task(validation_dir, "validation-task") + train_dataset = HarborDataset.from_path(train_dir) + validation_dataset = HarborDataset.from_path(validation_dir) with pytest.raises(DatasetValidationError) as exc_info: await insight_suite.validate() @@ -253,17 +260,22 @@ async def test_gpt5_mini_repairs_malformed_harbor_verifiers( insight, [], insight_suite, + train_dataset, + validation_dataset, runner_conventions, validation_feedback=validation_feedback, ), - timeout=300, + timeout=600, ) assert summary - await insight_suite.validate() + for dataset in (insight_suite, train_dataset, validation_dataset): + await dataset.validate() for verifier_path in ( insight_suite_dir / "task-a" / "tests" / "check_tool_hallucination.py", insight_suite_dir / "task-b" / "tests" / "check_tool_hallucination.py", + train_dir / "train-task" / "tests" / "check_tool_hallucination.py", + validation_dir / "validation-task" / "tests" / "check_tool_hallucination.py", ): repaired_source = verifier_path.read_text(encoding="utf-8") assert "def check_tool_hallucination" in repaired_source @@ -281,10 +293,17 @@ async def test_eval_author_metric_scores_known_failing_harbor_baseline_low( """An authored root-cause metric scores a known-failing Harbor baseline low.""" insight_suite_dir = tmp_path / "insight-suite" agent_dir = tmp_path / "known-failing-agent" + train_dir = tmp_path / "dataset" / "train" + validation_dir = tmp_path / "dataset" / "validation" _write_known_failing_task(insight_suite_dir) + _write_known_failing_task(train_dir) + _write_known_failing_task(validation_dir) _write_known_failing_agent(agent_dir) insight_suite = HarborDataset.from_path(insight_suite_dir) - await insight_suite.validate() + train_dataset = HarborDataset.from_path(train_dir) + validation_dataset = HarborDataset.from_path(validation_dir) + for dataset in (insight_suite, train_dataset, validation_dataset): + await dataset.validate() llm = get_fast_model() llm.config["temperature"] = 0.0 @@ -322,9 +341,9 @@ async def test_eval_author_metric_scores_known_failing_harbor_baseline_low( "/logs/verifier/reward.json, where higher is better and values are bounded to [0.0, 1.0]. Make the minimal " "verifier-only edit. Missing tool evidence is the expected failing case: it must score 0.0 while still " "writing reward.json and exiting successfully. Do not use an unguarded grep pipeline whose no-match status " - "can abort a set -e script; prefer a small Python standard-library checker. Call await " - "insight_suite.validate() once after editing, then return the metric summary as soon as validation passes; " - "do not inspect unrelated files." + "can abort a set -e script; prefer a small Python standard-library checker. The three datasets share one " + "task layout, so make the same verifier edit in each. Validate each dataset once after editing, then " + "return the metric summary as soon as validation passes; do not inspect unrelated files." ) summary = await asyncio.wait_for( @@ -332,13 +351,16 @@ async def test_eval_author_metric_scores_known_failing_harbor_baseline_low( insight, [("known-failing-trace", diagnostic)], insight_suite, + train_dataset, + validation_dataset, runner_conventions, ), - timeout=600, + timeout=900, ) assert summary - await insight_suite.validate() + for dataset in (insight_suite, train_dataset, validation_dataset): + await dataset.validate() evaluator = HarborEvaluator(experiment_dir=tmp_path) result = await asyncio.wait_for( evaluator.run( diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py index 139a3a1e75..67e0ff116b 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py @@ -3,9 +3,14 @@ """Experimentalist plugin entity definitions — stored in the NeMo Platform entity store.""" +from dataclasses import dataclass from typing import Any, Literal, Sequence from nemo_experimentalist_plugin.experimentalist.components.evaluator.models import TrialResult +from nemo_experimentalist_plugin.experimentalist.components.holdout_utils import ( + INSIGHT_TRAIN_SPLIT, + INSIGHT_VALIDATION_SPLIT, +) from nemo_platform_plugin.entity import NemoEntity from pydantic import ConfigDict, Field, model_validator @@ -149,21 +154,37 @@ class Candidate(NemoEntity, entity_type="candidate"): default=None, description="Validation split trial results from the last evaluation run.", ) - insight_reward: dict[str, float] | None = Field( + insight_train_reward: dict[str, float] | None = Field( default=None, - description="Multi-dimensional reward on the materialized Insight suite.", + description="Multi-dimensional reward on the Insight suite train half (development feedback).", ) - insight_reward_details: Sequence[TrialResult] | None = Field( + insight_train_reward_details: Sequence[TrialResult] | None = Field( default=None, - description="Insight-suite trial results from the last evaluation run.", + description="Insight train half trial results from the last evaluation run.", ) - insight_suite_identity: str | None = Field( + insight_train_suite_identity: str | None = Field( default=None, - description="Content identity of the Insight suite associated with insight_reward.", + description="Content identity of the Insight half associated with insight_train_reward.", ) - insight_metric_keys: list[str] | None = Field( + insight_train_metric_keys: list[str] | None = Field( default=None, - description="Validated runtime metric keys associated with insight_reward.", + description="Validated runtime metric keys associated with insight_train_reward.", + ) + insight_validation_reward: dict[str, float] | None = Field( + default=None, + description="Multi-dimensional reward on the held-out Insight suite validation half.", + ) + insight_validation_reward_details: Sequence[TrialResult] | None = Field( + default=None, + description="Insight validation half trial results from the last evaluation run.", + ) + insight_validation_suite_identity: str | None = Field( + default=None, + description="Content identity of the Insight half associated with insight_validation_reward.", + ) + insight_validation_metric_keys: list[str] | None = Field( + default=None, + description="Validated runtime metric keys associated with insight_validation_reward.", ) validation_trajectory_reward: dict[str, float] | None = Field( default=None, @@ -196,9 +217,12 @@ def __repr__(self) -> str: if self.validation_reward: scores = ", ".join(f"{k}={v:.3f}" for k, v in self.validation_reward.items()) parts.append(f", validation_reward={{{scores}}}") - if self.insight_reward: - scores = ", ".join(f"{k}={v:.3f}" for k, v in self.insight_reward.items()) - parts.append(f", insight_reward={{{scores}}}") + if self.insight_train_reward: + scores = ", ".join(f"{k}={v:.3f}" for k, v in self.insight_train_reward.items()) + parts.append(f", insight_train_reward={{{scores}}}") + if self.insight_validation_reward: + scores = ", ".join(f"{k}={v:.3f}" for k, v in self.insight_validation_reward.items()) + parts.append(f", insight_validation_reward={{{scores}}}") if self.killed_round is not None: parts.append(f", killed_round={self.killed_round}") parts.append(")") @@ -210,7 +234,75 @@ def slim(self) -> "Candidate": update={ "train_reward_details": None, "validation_reward_details": None, - "insight_reward_details": None, + "insight_train_reward_details": None, + "insight_validation_reward_details": None, "validation_trajectory_reward_details": None, } ) + + +@dataclass(frozen=True, slots=True) +class InsightSplitFields: + """Names of the ``Candidate`` fields holding one Insight half's evaluation results. + + Lets evaluation, promotion, and reporting address either half without branching + on the split name at every access. + """ + + split: str + reward: str + reward_details: str + suite_identity: str + metric_keys: str + held_out: bool + + def reward_of(self, candidate: Candidate) -> dict[str, float] | None: + """Return this half's aggregate reward for ``candidate``.""" + return getattr(candidate, self.reward) + + def reward_details_of(self, candidate: Candidate) -> Sequence[TrialResult] | None: + """Return this half's trial results for ``candidate``.""" + return getattr(candidate, self.reward_details) + + def suite_identity_of(self, candidate: Candidate) -> str | None: + """Return the suite identity ``candidate``'s reward for this half was scored against.""" + return getattr(candidate, self.suite_identity) + + def metric_keys_of(self, candidate: Candidate) -> list[str] | None: + """Return the validated metric keys for this half's reward on ``candidate``.""" + return getattr(candidate, self.metric_keys) + + def updates( + self, + *, + reward: dict[str, float], + reward_details: Sequence[TrialResult], + suite_identity: str, + metric_keys: list[str], + ) -> dict[str, Any]: + """Return a ``Candidate`` update mapping for this half.""" + return { + self.reward: reward, + self.reward_details: reward_details, + self.suite_identity: suite_identity, + self.metric_keys: metric_keys, + } + + +INSIGHT_TRAIN_FIELDS = InsightSplitFields( + split=INSIGHT_TRAIN_SPLIT, + reward="insight_train_reward", + reward_details="insight_train_reward_details", + suite_identity="insight_train_suite_identity", + metric_keys="insight_train_metric_keys", + held_out=False, +) + +INSIGHT_VALIDATION_FIELDS = InsightSplitFields( + split=INSIGHT_VALIDATION_SPLIT, + reward="insight_validation_reward", + reward_details="insight_validation_reward_details", + suite_identity="insight_validation_suite_identity", + metric_keys="insight_validation_metric_keys", + held_out=True, +) diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/analyzer.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/analyzer.py index 335441e571..67628a8425 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/analyzer.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/analyzer.py @@ -263,6 +263,8 @@ async def select_trials( agent_id: str, dataset: Dataset, evaluation: EvaluationResult, + insight_dataset: Dataset | None = None, + insight_trials: Sequence[TrialResult] | None = None, ) -> Sequence[TrialResult]: """Pick which trials to analyze in depth. Return their TrialResult objects. @@ -270,6 +272,8 @@ async def select_trials( agent_id: The agent to analyze. dataset: The dataset to analyze. evaluation: The evaluation result to analyze. + insight_dataset: The Insight suite train half, when this run has one. + insight_trials: This agent's trials on ``insight_dataset``. Returns: Sequence[TrialResult]: The selected trials. @@ -281,6 +285,17 @@ async def select_trials( trials = list(evaluation.trials) ``` + ## Step 1b: Always include every Insight trial + + ```python + insight = list(insight_trials or ()) + ``` + + Each Insight task reconstructs a real production failure, and there are only a + handful of them. Return **all** of them in addition to your triaged selection; + they do not count against the triage budget below. When ``insight_trials`` is + empty or ``None`` there is nothing to add. + ## Step 2: Inspect metrics, outputs, resources, and errors ```python @@ -305,7 +320,7 @@ async def select_trials( ## Step 4: Return TrialResult objects ```python - return selected_trials + return insight + selected_trials ``` """ ... @@ -573,6 +588,8 @@ async def run( client: AsyncNeMoPlatform | None = None, nmp_workspace: str | None = None, agent_spec: Path | None = None, + insight_dataset: Dataset | None = None, + insight_trials: Sequence[TrialResult] | None = None, ) -> AgentAnalysis: """Run the full analysis pipeline for one agent in one optimization round. @@ -587,6 +604,11 @@ async def run( nmp_workspace: NeMo Platform (Intake) workspace *name* — the request context for ``intake://`` trace lookups. Distinct from the constructor's ``workspace: Path`` (the filesystem eval dir). + agent_spec: Optional agent spec used to rationalize task intent. + insight_dataset: The Insight suite train half. Must never be the held-out + validation half, whose contents this analysis would leak into the + goal tree and proposer. + insight_trials: This agent's trials on ``insight_dataset``. Returns: AgentAnalysis: per-trial diagnostics, failure classification, and peer @@ -600,13 +622,16 @@ async def run( # trace-starved. Keying on availability prevents such a degraded result # from being replayed on a later run that *can* load those traces. intake_key = ":intake:1" if client is not None and nmp_workspace is not None else ":intake:0" - cache_key = cache.agent_hash(f"{agent_id}:evaluation:{evaluation.id}{round_key}{intake_key}") + insight_key = ":insight:" + ",".join(sorted(trial.id for trial in insight_trials or ())) + cache_key = cache.agent_hash(f"{agent_id}:evaluation:{evaluation.id}{round_key}{intake_key}{insight_key}") cached = cache.load(self._workspace_path, cache_key, AgentAnalysis) if cached is not None: return cached - trials = await self.select_trials(agent_id, dataset, evaluation) + trials = await self.select_trials(agent_id, dataset, evaluation, insight_dataset, insight_trials) tasks_by_id = self._tasks_by_id(dataset) + if insight_dataset is not None: + tasks_by_id |= self._tasks_by_id(insight_dataset) missing_task_diagnostics: dict[str, Diagnostic] = {} trial_tasks: list[tuple[TrialResult, Task]] = [] diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/holdout_utils.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/holdout_utils.py index 0fdce43f5d..11d2eb137f 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/holdout_utils.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/holdout_utils.py @@ -5,7 +5,14 @@ import shutil from pathlib import Path -HELD_OUT_SPLITS = frozenset({"validation"}) +VALIDATION_SPLIT = "validation" + +# Eval Author materializes the Insight halves into these directories and declares the +# same names itself; the pairing is pinned by test_insight_split_names_match_eval_author. +INSIGHT_TRAIN_SPLIT = "insight-train" +INSIGHT_VALIDATION_SPLIT = "insight-validation" + +HELD_OUT_SPLITS = frozenset({VALIDATION_SPLIT, INSIGHT_VALIDATION_SPLIT}) HELD_OUT_STORAGE_DIR = ".aad-heldout" # Path tokens GuardedShellTools refuses: a tripwire for direct shell access while a @@ -16,8 +23,8 @@ ) BLOCKED_MESSAGE = ( - "blocked: the validation split is held out for scoring only. " - "Its contents are off-limits; diagnose and fix using the train split." + "blocked: the validation splits are held out for scoring only. " + "Their contents are off-limits; diagnose and fix using the train splits." ) diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py index 660aa94b5a..0eddded029 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py @@ -6,11 +6,11 @@ from __future__ import annotations import math -from collections.abc import Sequence +from collections.abc import Iterable, Sequence from dataclasses import dataclass, replace from pathlib import Path -from nemo_experimentalist_plugin.entities import Candidate +from nemo_experimentalist_plugin.entities import Candidate, InsightSplitFields from nemo_experimentalist_plugin.experimentalist.components.evaluator import ( Dataset, EvaluationResult, @@ -22,10 +22,18 @@ _GENERIC_METRIC_NAMES = frozenset({"reward", "score"}) _MAX_REPEAT_SPREAD = 0.1 _MIN_DISCRIMINATION = 1e-9 -_REPORT_SECTION_START = "" -_REPORT_SECTION_END = "" -_COMPARISON_SECTION_START = "" -_COMPARISON_SECTION_END = "" +_PROMOTION_SECTION_MARKER = "insight-suite-promotion-suggestions" +_COMPARISON_SECTION_MARKER = "insight-suite-comparison" + + +def _section_markers(marker: str, split: str) -> tuple[str, str]: + """Return per-split section markers so both Insight halves can coexist in one report.""" + return f"", f"" + + +def insight_metric_names(metric_keys: Iterable[str]) -> set[str]: + """Return the Insight-specific subset of ``metric_keys``, dropping generic reward keys.""" + return set(metric_keys) - _GENERIC_METRIC_NAMES @dataclass(frozen=True, slots=True) @@ -191,11 +199,12 @@ def _task_evidence( baseline: Candidate, winner: Candidate, provenance: InsightSuiteProvenance, + fields: InsightSplitFields, ) -> _TaskEvidence | None: suite_candidates = [ - candidate for candidate in candidates if candidate.insight_suite_identity == provenance.identity + candidate for candidate in candidates if fields.suite_identity_of(candidate) == provenance.identity ] - metric_key_sets = {tuple(sorted(candidate.insight_metric_keys or ())) for candidate in suite_candidates} + metric_key_sets = {tuple(sorted(fields.metric_keys_of(candidate) or ())) for candidate in suite_candidates} if len(metric_key_sets) != 1: return None required_metrics = set(next(iter(metric_key_sets), ())) @@ -204,7 +213,7 @@ def _task_evidence( return None trials_by_candidate = { - candidate.label: [trial for trial in candidate.insight_reward_details or () if trial.task_id == task.id] + candidate.label: [trial for trial in fields.reward_details_of(candidate) or () if trial.task_id == task.id] for candidate in suite_candidates } values_by_candidate: dict[str, dict[str, list[float]]] = {} @@ -286,6 +295,7 @@ def select_insight_promotion_suggestions( dataset: Dataset, candidates: Sequence[Candidate], *, + fields: InsightSplitFields, winner: Candidate | None = None, limit: int = 3, ) -> list[InsightPromotionSuggestion]: @@ -296,7 +306,8 @@ def select_insight_promotion_suggestions( evaluated_candidates = [ candidate for candidate in candidates - if candidate.insight_reward_details is not None and candidate.insight_suite_identity == provenance.identity + if fields.reward_details_of(candidate) is not None + and fields.suite_identity_of(candidate) == provenance.identity ] if len(evaluated_candidates) < 2: return [] @@ -314,6 +325,7 @@ def select_insight_promotion_suggestions( baseline=baseline, winner=winner, provenance=provenance, + fields=fields, ) ) is not None @@ -368,10 +380,11 @@ def _markdown_cell(value: str) -> str: def render_insight_promotion_section( suggestions: Sequence[InsightPromotionSuggestion], + split: str, ) -> str: - """Render an advisory-only final-report section.""" + """Render an advisory-only final-report section for one Insight half.""" lines = [ - "## Insight Suite Promotion Suggestions", + f"## Insight Suite Promotion Suggestions ({split})", "", ( "Advisory adaptive/development evidence only, not independent validation evidence. " @@ -436,13 +449,15 @@ def _write_marked_section( def write_insight_promotion_section( report_path: Path, suggestions: Sequence[InsightPromotionSuggestion], + split: str, ) -> None: - """Append or replace the advisory promotion section in the final report.""" + """Append or replace one half's advisory promotion section in the final report.""" + start_marker, end_marker = _section_markers(_PROMOTION_SECTION_MARKER, split) _write_marked_section( report_path, - rendered=render_insight_promotion_section(suggestions), - start_marker=_REPORT_SECTION_START, - end_marker=_REPORT_SECTION_END, + rendered=render_insight_promotion_section(suggestions, split), + start_marker=start_marker, + end_marker=end_marker, ) @@ -450,23 +465,32 @@ def render_insight_comparison_section( baseline: Candidate, winner: Candidate, provenance: InsightSuiteProvenance, + fields: InsightSplitFields, ) -> str: - """Render the deterministic baseline-versus-winner Insight comparison.""" + """Render the deterministic baseline-versus-winner Insight comparison for one half.""" for candidate in (baseline, winner): - if candidate.insight_suite_identity != provenance.identity: + if fields.suite_identity_of(candidate) != provenance.identity: raise ValueError( f"Candidate {candidate.label!r} Insight evidence does not match finalized suite {provenance.identity}" ) - baseline_reward = baseline.insight_reward or {} - winner_reward = winner.insight_reward or {} + baseline_reward = fields.reward_of(baseline) or {} + winner_reward = fields.reward_of(winner) or {} metric_names = sorted(set(baseline_reward) | set(winner_reward)) + provenance_note = ( + ( + "Held out from optimization, so these dimensions are independent scoring evidence " + "and participate in Pareto and winner selection under the `insight/` prefix." + ) + if fields.held_out + else ( + "Adaptive/development evidence only; this half is visible to the optimizer and " + "does not affect Pareto or winner selection." + ) + ) lines = [ - "## Deterministic Insight Suite Comparison", + f"## Deterministic Insight Suite Comparison ({fields.split})", "", - ( - "Adaptive/development evidence only; canonical validation remains the direct " - "Pareto and winner-selection criterion." - ), + provenance_note, "", (f"Suite: `{provenance.suite_path}` (suite `{provenance.identity}`; scorer `{provenance.scorer_identity}`)"), "", @@ -493,11 +517,13 @@ def write_insight_comparison_section( baseline: Candidate, winner: Candidate, provenance: InsightSuiteProvenance, + fields: InsightSplitFields, ) -> None: - """Append or replace the deterministic baseline-versus-winner section.""" + """Append or replace one half's deterministic baseline-versus-winner section.""" + start_marker, end_marker = _section_markers(_COMPARISON_SECTION_MARKER, fields.split) _write_marked_section( report_path, - rendered=render_insight_comparison_section(baseline, winner, provenance), - start_marker=_COMPARISON_SECTION_START, - end_marker=_COMPARISON_SECTION_END, + rendered=render_insight_comparison_section(baseline, winner, provenance, fields), + start_marker=start_marker, + end_marker=end_marker, ) diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py index 6c8854547d..72e559b4d5 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py @@ -15,11 +15,18 @@ import random import shutil from collections import defaultdict +from collections.abc import Sequence from pathlib import Path from typing import Any, Literal, cast, get_args from nemo_eval_author_plugin.eval_author.agent import EvalAuthor -from nemo_experimentalist_plugin.entities import Candidate, ExperimentRun +from nemo_experimentalist_plugin.entities import ( + INSIGHT_TRAIN_FIELDS, + INSIGHT_VALIDATION_FIELDS, + Candidate, + ExperimentRun, + InsightSplitFields, +) from nemo_experimentalist_plugin.experimentalist.components.analyzer import AgentAnalyzer, AnalyzerConfig from nemo_experimentalist_plugin.experimentalist.components.coder import Coder, CoderConfig from nemo_experimentalist_plugin.experimentalist.components.dataset_staging import stage_eval_author_inputs @@ -38,10 +45,13 @@ traverse_tree, ) from nemo_experimentalist_plugin.experimentalist.components.holdout_utils import ( + INSIGHT_VALIDATION_SPLIT, + VALIDATION_SPLIT, ensure_heldout_hidden, restore_heldout_splits, ) from nemo_experimentalist_plugin.experimentalist.components.insight_promotion import ( + insight_metric_names, insight_suite_provenance, select_insight_promotion_suggestions, stamp_insight_evaluation_result, @@ -56,8 +66,10 @@ from nemo_experimentalist_plugin.experimentalist.components.models import ( EvolutionTree, OptimizationType, + node_selection_rewards, pareto_front, pareto_sort, + selection_rewards, ) from nemo_experimentalist_plugin.experimentalist.components.proposer import Improvement, Proposer, ProposerConfig from nemo_experimentalist_plugin.experimentalist.components.terminator import Terminator @@ -192,22 +204,34 @@ class AnalysisSkill(Skill): | agent-1 | 0.48 | 0.58 | 0.41 | ... | agent-0 | -0.10 | | agent-0 | 0.45 | 0.55 | 0.40 | ... | --- | baseline | - Insight Suite Reward: + Insight Train Reward: | Agent | | | ... | vs. Baseline | | ----- | -------------- | -------------- | --- | ------------ | | agent-3 | 0.80 | 0.67 | ... | +0.40 | | agent-1 | 0.60 | 0.50 | ... | +0.20 | | agent-0 | 0.40 | 0.33 | ... | baseline | + Insight Validation Reward: + | Agent | | | ... | vs. Baseline | + | ----- | -------------- | -------------- | --- | ------------ | + | agent-3 | 0.75 | 0.60 | ... | +0.35 | + | agent-1 | 0.55 | 0.45 | ... | +0.15 | + | agent-0 | 0.40 | 0.30 | ... | baseline | + [Columns are the actual reward dimension keys from metadata. Order by any dimension that - helps comparison — no dimension is privileged. Read Insight Suite Reward from - `candidate.insight_reward`. Omit that table when `insight_reward` is absent or empty - for every agent. Keep Insight Suite Reward separate from train and validation rewards: - it reports performance on scenarios authored for the motivating Insight and is not a - ranking or Pareto-selection input. Insight Suite metrics may steer round analysis, - goal-tree updates, and the proposer only as adaptive/development feedback. Label any - resulting claim accordingly; never present this adaptive evidence as independent - validation evidence.] + helps comparison — no dimension is privileged. Read the two tables from + `candidate.insight_train_reward` and `candidate.insight_validation_reward`. Omit either + table when its field is absent or empty for every agent. Keep both separate from train + and validation rewards; they score scenarios authored for the motivating Insight. + + The two halves carry different evidentiary weight and must be labeled differently: + + - Insight **train** is visible to the optimizer. It may steer round analysis, goal-tree + updates, and the proposer as adaptive/development feedback, but it does not affect + ranking or Pareto selection. Never present it as independent validation evidence. + - Insight **validation** is held out from optimization. It is independent scoring + evidence and does affect ranking: its dimensions enter Pareto selection prefixed with + `insight/` alongside the validation reward. Report it as a selection input.] ## Trajectory Rewards @@ -416,7 +440,8 @@ async def _run(self, deps: ExperimentalistDeps) -> ExperimentalistResult: train_dataset_ref = deps.train_dataset validation_dataset_ref = deps.validation_dataset task_template_ref = deps.task_template - insight_eval_dataset: Dataset | None = None + insight_train_dataset: Dataset | None = None + insight_validation_dataset: Dataset | None = None if deps.insight is not None: if task_template_ref is None: raise ValueError("Task template is required for insight trace analysis") @@ -491,7 +516,11 @@ async def _run(self, deps: ExperimentalistDeps) -> ExperimentalistResult: ) train_eval_dataset = eval_author_result.train_dataset validation_eval_dataset = eval_author_result.validation_dataset - insight_eval_dataset = eval_author_result.insight_suite + insight_train_dataset = eval_author_result.insight_train_suite + insight_validation_dataset = eval_author_result.insight_validation_suite + # Hide the Insight validation half before any optimizing agent runs, rather + # than waiting for the first scoring pass to hide it on the way out. + ensure_heldout_hidden(self.working_dir, splits=frozenset({INSIGHT_VALIDATION_SPLIT})) else: # Mode 2: local agent directory as baseline, no insight required. insight = None @@ -591,16 +620,32 @@ async def _run(self, deps: ExperimentalistDeps) -> ExperimentalistResult: run_id = run_entity.id or "" - if insight_eval_dataset is not None: + insight_halves: list[tuple[InsightSplitFields, Dataset]] = [ + (fields, dataset) + for fields, dataset in ( + (INSIGHT_TRAIN_FIELDS, insight_train_dataset), + (INSIGHT_VALIDATION_FIELDS, insight_validation_dataset), + ) + if dataset is not None + ] + required_insight_metrics: set[str] = set() + if insight_halves: try: - await self._evaluate_and_persist_insight_candidates( - dataset=insight_eval_dataset, + required_insight_metrics = await self._evaluate_and_persist_insight_halves( + halves=insight_halves, evaluator=evaluator, candidates=candidates, workspace=workspace, backend=backend, - run_id=run_entity.id or "", + run_id=run_id, ) + for candidate in candidates: + if candidate.validation_reward is not None: + self._assert_insight_metrics_present( + required=required_insight_metrics, + aggregate_metrics=candidate.validation_reward, + split=VALIDATION_SPLIT, + ) except Exception: run_entity.status = "failed" await backend.update_run(workspace=workspace, run=run_entity) @@ -635,7 +680,7 @@ async def _run(self, deps: ExperimentalistDeps) -> ExperimentalistResult: break survivors = ( - await self._select_survivors([c.slim() for c in candidates], k=config.max_survivors) + await self._select_survivors(candidates, k=config.max_survivors) if len(candidates) > 1 else list(candidates) ) @@ -660,6 +705,11 @@ async def _run(self, deps: ExperimentalistDeps) -> ExperimentalistResult: ) for survivor in survivors: if survivor.label in train_candidate_results: + self._assert_insight_metrics_present( + required=required_insight_metrics, + aggregate_metrics=train_candidate_results[survivor.label].aggregate_metrics, + split="train", + ) await backend.persist_evaluation( workspace=workspace, result=train_candidate_results[survivor.label], @@ -693,6 +743,12 @@ async def _run(self, deps: ExperimentalistDeps) -> ExperimentalistResult: client=backend.client, nmp_workspace=workspace, agent_spec_path=agent_spec_path, + insight_dataset=insight_train_dataset, + insight_trials={ + survivor.label: trials + for survivor in survivors + if (trials := INSIGHT_TRAIN_FIELDS.reward_details_of(survivor)) + }, ) await self._update_goal_tree( analysis_dir=analysis_dir, @@ -744,9 +800,9 @@ async def _run(self, deps: ExperimentalistDeps) -> ExperimentalistResult: backend=backend, run_id=run_id, ) - if insight_eval_dataset is not None: - await self._evaluate_and_persist_insight_candidates( - dataset=insight_eval_dataset, + if insight_halves: + await self._evaluate_and_persist_insight_halves( + halves=insight_halves, evaluator=evaluator, candidates=new_candidates, workspace=workspace, @@ -786,6 +842,11 @@ async def _run(self, deps: ExperimentalistDeps) -> ExperimentalistResult: ) for candidate in candidates: if candidate.label in validation_candidate_results: + self._assert_insight_metrics_present( + required=required_insight_metrics, + aggregate_metrics=validation_candidate_results[candidate.label].aggregate_metrics, + split=VALIDATION_SPLIT, + ) await backend.persist_evaluation( workspace=workspace, result=validation_candidate_results[candidate.label], @@ -856,7 +917,7 @@ async def _run(self, deps: ExperimentalistDeps) -> ExperimentalistResult: run_entity=run_entity, evolution_tree=evolution_tree, agent_name=agent_name, - insight_dataset=insight_eval_dataset, + insight_halves=insight_halves, ) baseline_entity = next( @@ -933,8 +994,11 @@ async def merge_analysis( ```python rewards = {c.id: self.workspace.get_metadata(c.name).train_reward or {} for c in agent_ids} - insight_rewards = { - c.id: self.workspace.get_metadata(c.name).insight_reward or {} for c in agent_ids + insight_train_rewards = { + c.id: self.workspace.get_metadata(c.name).insight_train_reward or {} for c in agent_ids + } + insight_validation_rewards = { + c.id: self.workspace.get_metadata(c.name).insight_validation_reward or {} for c in agent_ids } all_candidates = [ self.workspace.get_metadata(agent_id).slim() for agent_id in self.workspace.list_agents() @@ -944,8 +1008,9 @@ async def merge_analysis( - Compare siblings: which optimization strategy worked better this round? - Compare to ancestors: did the change actually fix the targeted root cause? - - When any Insight Suite rewards are present, compare those dimensions to the - round-zero baseline separately from train and validation rewards. + - When any Insight rewards are present, compare those dimensions to the round-zero + baseline separately from train and validation rewards, and keep the two Insight + halves in separate tables. ## Step 2: Analyze divergent and complementary patterns @@ -960,21 +1025,36 @@ async def merge_analysis( candidate = self.workspace.get_metadata(agent_ids[0].name).slim() train_reward = candidate.train_reward or {} dim_keys = sorted(train_reward.keys()) - insight_dim_keys = sorted({key for reward in insight_rewards.values() for key in reward}) + insight_train_dim_keys = sorted( + {key for reward in insight_train_rewards.values() for key in reward} + ) + insight_validation_dim_keys = sorted( + {key for reward in insight_validation_rewards.values() for key in reward} + ) ``` Follow the `ext.analysis_skill` format exactly for every section (Rewards tables, - including the conditional Insight Suite Reward table; Trajectory Rewards; Divergent - Trial Analysis; Complementary Failures; Failure Patterns; Root Causes; - Mechanical/Infrastructure Errors). - - If at least one agent has a non-empty `insight_reward`, the round analysis must name - every available Insight Suite dimension and show its values in the separate Insight - Suite Reward table. Never blend those metrics into train/validation rewards or imply - that they affected ranking. These metrics may steer this analysis, the goal tree, and - the proposer only as adaptive/development feedback; label claims accordingly and never - present them as independent validation evidence. Fill in every included section with - real data. No placeholders. + including the conditional Insight Train Reward and Insight Validation Reward tables; + Trajectory Rewards; Divergent Trial Analysis; Complementary Failures; Failure + Patterns; Root Causes; Mechanical/Infrastructure Errors). + + If at least one agent has a non-empty `insight_train_reward` or + `insight_validation_reward`, the round analysis must name every available dimension + of that half and show its values in that half's own table. Never blend either half's + metrics into the train or validation reward tables. + + The two halves differ in what you may claim from them: + + - `insight_train_reward` is visible to the optimizer, so it is adaptive/development + feedback. It may steer this analysis, the goal tree, and the proposer, but it did + not affect ranking. Label claims accordingly and never present it as independent + validation evidence. + - `insight_validation_reward` is held out from optimization, so it is independent + scoring evidence and it did affect ranking: its dimensions enter Pareto selection + prefixed with `insight/` alongside the validation reward. Say so when a selection + outcome turned on them. + + Fill in every included section with real data. No placeholders. Return the complete markdown content as a string. """ ... @@ -988,7 +1068,8 @@ async def write_final_report(self, best_agent_id: str) -> None: # pyright: igno ```python agent_ids = self.workspace.list_agents() candidate = self.workspace.get_metadata(agent_id).slim() - insight_reward = candidate.insight_reward or {} + insight_train_reward = candidate.insight_train_reward or {} + insight_validation_reward = candidate.insight_validation_reward or {} analysis = self.workspace.read_analysis_file(n) ``` @@ -999,16 +1080,18 @@ async def write_final_report(self, best_agent_id: str) -> None: # pyright: igno 4. Write eval-and-optimize/OPTIMIZATION.md with format: - Summary (baseline vs best rewards, rounds completed, total agents) - Reward Breakdown table (one row per agent, per-dimension columns) - - Insight Suite Metrics table when available + - Insight Train Metrics and Insight Validation Metrics tables when available - Lineage Tree (ASCII tree with rewards and optimization type) - Round-by-Round Analysis - Optimization Insights - When both the round-zero baseline and best agent have non-empty `insight_reward`, - the Summary must state whether the Insight-specific scenarios improved and the - Insight Suite Metrics table must show every available dimension with baseline, - winner, and signed delta columns. Keep this table separate from generic train and - validation rewards. Omit it only when Insight Suite rewards are unavailable. + For each Insight half where both the round-zero baseline and best agent have a + non-empty reward, the Summary must state whether the Insight-specific scenarios + improved, and that half's table must show every available dimension with baseline, + winner, and signed delta columns. Keep these tables separate from generic train and + validation rewards, and label the train half as adaptive/development feedback that + did not affect ranking and the validation half as held-out evidence that did. + Omit a table only when that half's rewards are unavailable. Fill in every section with real data. Every agent must appear in the lineage tree. Mark the best agent with * BEST. @@ -1395,22 +1478,21 @@ async def _evaluate_validation_candidates( pending = [c for c in candidates if c.validation_reward is None] if not pending: return {} - if pending: - splits = frozenset({"validation"}) - restore_heldout_splits(self.working_dir, splits=splits) - try: - candidate_results = await asyncio.gather( - *[ - self._evaluate_agent( - c, - dataset, - evaluator, - ) - for c in pending - ] - ) - finally: - ensure_heldout_hidden(self.working_dir, splits=splits) + splits = frozenset({VALIDATION_SPLIT}) + restore_heldout_splits(self.working_dir, splits=splits) + try: + candidate_results = await asyncio.gather( + *[ + self._evaluate_agent( + c, + dataset, + evaluator, + ) + for c in pending + ] + ) + finally: + ensure_heldout_hidden(self.working_dir, splits=splits) return { candidate_result[0].label: candidate_result[1] for candidate_result in candidate_results @@ -1421,48 +1503,64 @@ async def _evaluate_insight_candidates( self, *, dataset: Dataset, + fields: InsightSplitFields, evaluator: Evaluator, candidates: list[Candidate], ) -> dict[str, EvaluationResult]: - """Evaluate candidates that do not yet have metrics for this Insight suite.""" + """Evaluate candidates that do not yet have metrics for this Insight half. + + Every task in the half runs for every pending candidate; unlike the train + split, the Insight halves are never sampled down to a batch. + """ if not list(dataset.list_tasks()): return {} provenance = insight_suite_provenance(dataset) pending = [ candidate for candidate in candidates - if candidate.insight_reward is None - or candidate.insight_reward_details is None - or candidate.insight_suite_identity != provenance.identity - or not candidate.insight_metric_keys + if fields.reward_of(candidate) is None + or fields.reward_details_of(candidate) is None + or fields.suite_identity_of(candidate) != provenance.identity + or not fields.metric_keys_of(candidate) ] - evaluated = await asyncio.gather( - *[ - self._evaluate_agent( - candidate, - dataset, - evaluator, - minimum_attempts=2, - ) - for candidate in pending - ] - ) + if not pending: + return {} + splits = frozenset({fields.split}) if fields.held_out else frozenset() + if splits: + restore_heldout_splits(self.working_dir, splits=splits) + try: + evaluated = await asyncio.gather( + *[ + self._evaluate_agent( + candidate, + dataset, + evaluator, + minimum_attempts=2, + ) + for candidate in pending + ] + ) + finally: + if splits: + ensure_heldout_hidden(self.working_dir, splits=splits) return {candidate.label: result for candidate, result in evaluated} async def _evaluate_and_persist_insight_candidates( self, *, dataset: Dataset, + fields: InsightSplitFields, evaluator: Evaluator, candidates: list[Candidate], workspace: str, backend: ExperimentalistBackend, run_id: str, - ) -> None: - """Evaluate and persist Insight-suite metrics for the supplied candidates.""" + ) -> tuple[str, ...] | None: + """Evaluate and persist one Insight half's metrics, returning its metric keys.""" provenance = insight_suite_provenance(dataset) results = await self._evaluate_insight_candidates( dataset=dataset, + fields=fields, evaluator=evaluator, candidates=candidates, ) @@ -1472,9 +1570,9 @@ async def _evaluate_and_persist_insight_candidates( ): raise ValueError("Insight suite runtime metric keys have invalid metadata") cached_metric_key_sets = { - tuple(sorted(candidate.insight_metric_keys or ())) + tuple(sorted(fields.metric_keys_of(candidate) or ())) for candidate in candidates - if candidate.insight_suite_identity == provenance.identity and candidate.insight_metric_keys + if fields.suite_identity_of(candidate) == provenance.identity and fields.metric_keys_of(candidate) } if isinstance(dataset_metric_keys, list): cached_metric_key_sets.add(tuple(sorted(dataset_metric_keys))) @@ -1498,22 +1596,70 @@ async def _evaluate_and_persist_insight_candidates( workspace=workspace, result=result, candidate=candidate, - split="insight", + split=fields.split, ) await self._update_candidate( candidate, - updates={ - "insight_reward": result.aggregate_metrics, - "insight_reward_details": result.trials, - "insight_suite_identity": provenance.identity, - "insight_metric_keys": list(metric_keys), - }, + updates=fields.updates( + reward=result.aggregate_metrics, + reward_details=result.trials, + suite_identity=provenance.identity, + metric_keys=list(metric_keys), + ), workspace=workspace, backend=backend, run_id=run_id, ) if expected_metric_keys is not None: dataset.metadata["insight_metric_keys"] = list(expected_metric_keys) + return expected_metric_keys + + async def _evaluate_and_persist_insight_halves( + self, + *, + halves: Sequence[tuple[InsightSplitFields, Dataset]], + evaluator: Evaluator, + candidates: list[Candidate], + workspace: str, + backend: ExperimentalistBackend, + run_id: str, + ) -> set[str]: + """Evaluate every materialized Insight half and return their Insight metric names.""" + metric_names: set[str] = set() + for fields, dataset in halves: + metric_keys = await self._evaluate_and_persist_insight_candidates( + dataset=dataset, + fields=fields, + evaluator=evaluator, + candidates=candidates, + workspace=workspace, + backend=backend, + run_id=run_id, + ) + metric_names |= insight_metric_names(metric_keys or ()) + return metric_names + + @staticmethod + def _assert_insight_metrics_present( + *, + required: set[str], + aggregate_metrics: dict[str, float], + split: str, + ) -> None: + """Fail fast when an authored Insight metric is missing from a scored split. + + ``dataset.validate()`` is static and cannot see which metric names a verifier + emits at runtime, so a key the Eval Author added to the Insight suite but missed + on train or validation would otherwise surface much later as an + ``aggregate_results`` crash mid-run. + """ + missing = sorted(required - set(aggregate_metrics)) + if missing: + raise ValueError( + f"Insight metrics {missing} are missing from the {split!r} split. The Eval Author " + "must author one identical metric key set across the Insight suite and the train " + "and validation datasets." + ) async def _generate_initial_goal_tree( self, @@ -1546,9 +1692,26 @@ async def _select_survivors( candidates: list[Candidate], k: int, ) -> list[Candidate]: - """Return the top-k Pareto-optimal and architecturally diverse candidates.""" - ranked = pareto_sort(candidates, lambda c: c.validation_reward or {}) - return await self.select_diverse_survivors(ranked, k) + """Return the top-k Pareto-optimal and architecturally diverse candidates. + + Ranking is done on slimmed copies because ``select_diverse_survivors`` is an + LLM strategy and per-trial detail does not belong in its context. The winners + are then mapped back to the caller's objects: survivors are carried into the + next round and persisted, so handing back the slimmed copies would strip the + trial detail the analyzer diagnoses and blank it out in ``metadata.json``. + """ + rewards = selection_rewards(candidates) + ranked = pareto_sort([c.slim() for c in candidates], lambda c: rewards[c.label]) + selected = await self.select_diverse_survivors(ranked, k) + by_label = {c.label: c for c in candidates} + survivors: list[Candidate] = [] + for candidate in selected: + original = by_label.get(candidate.label) + if original is None: + logger.warning(f"[SELECT] ignoring survivor {candidate.label!r}, which was not a ranked candidate") + continue + survivors.append(original) + return survivors async def _evaluate_train_candidates( self, @@ -1616,6 +1779,8 @@ async def _analyze_round( client: AsyncNeMoPlatform | None = None, nmp_workspace: str | None = None, agent_spec_path: Path | None = None, + insight_dataset: Dataset | None = None, + insight_trials: dict[str, Sequence[TrialResult]] | None = None, ) -> str: """Run AgentAnalyzer per survivor, merge analyses, persist to disk. @@ -1623,12 +1788,17 @@ async def _analyze_round( so its ``TraceAnalyzer`` can load ``intake://`` trial traces; when ``None`` those traces are skipped (local ``file://`` traces still load). + ``insight_dataset`` and ``insight_trials`` must come from the Insight train half + only. Analyzer output steers the goal tree and proposer, so passing the held-out + half would leak it into the optimization signal. + Returns the merged analysis markdown string. """ analysis_path = analysis_dir / f"round-{round_num}.md" if analysis_path.exists(): return analysis_path.read_text() + trials_by_label = insight_trials or {} per_agent = await asyncio.gather( *[ AgentAnalyzer( @@ -1644,6 +1814,8 @@ async def _analyze_round( client=client, nmp_workspace=nmp_workspace, agent_spec=agent_spec_path, + insight_dataset=insight_dataset, + insight_trials=trials_by_label.get(s.label) or None, ) for s in survivors ] @@ -1871,12 +2043,13 @@ async def _finalize( run_entity: ExperimentRun, evolution_tree: EvolutionTree, agent_name: str, - insight_dataset: Dataset | None, + insight_halves: Sequence[tuple[InsightSplitFields, Dataset]], ) -> Candidate | None: """Select the winner, copy to workspace root, write final report.""" # Only survivors that actually have a validation reward are eligible winners. scored = [n for n in evolution_tree.nodes.values() if n.is_survivor and n.val_reward] - front = pareto_front(scored, lambda n: n.val_reward) if scored else [] + rewards = node_selection_rewards(scored) + front = pareto_front(scored, lambda n: rewards[n.label]) if scored else [] best_id = front[0].label if front else None restore_heldout_splits(self.working_dir) @@ -1912,7 +2085,7 @@ async def _finalize( ) report_path.write_text(f"# Optimization Report\n\n## Compact Run Summary\n\n{summary}\n") - if insight_dataset is not None: + for fields, insight_dataset in insight_halves: try: provenance = insight_suite_provenance(insight_dataset) if baseline is not None: @@ -1921,18 +2094,21 @@ async def _finalize( baseline, winner, provenance, + fields, ) suggestions = select_insight_promotion_suggestions( insight_dataset, [node.candidate for node in evolution_tree.nodes.values()], + fields=fields, winner=winner, ) write_insight_promotion_section( report_path, suggestions, + fields.split, ) except ValueError as exc: - logger.warning(f"[FINAL] Skipping Insight Suite report sections: {exc}") + logger.warning(f"[FINAL] Skipping {fields.split} Insight report sections: {exc}") run_entity.status = "completed" run_entity.winner_agent = best_id @@ -1952,7 +2128,10 @@ def _render_summary( if winner: if winner.validation_reward: details.append(f"validation_reward={winner.validation_reward}") - if baseline is not None and baseline.insight_reward and winner.insight_reward: - details.append(f"insight_suite=(baseline={baseline.insight_reward}, winner={winner.insight_reward})") + if baseline is not None and baseline.insight_validation_reward and winner.insight_validation_reward: + details.append( + f"insight_validation=(baseline={baseline.insight_validation_reward}, " + f"winner={winner.insight_validation_reward})" + ) suffix = f", {', '.join(details)}" if details else "" return f"Optimization complete: {rounds_completed} round(s) completed, winner={winner_str}{suffix}" diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/models.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/models.py index 0ed2493ff5..af4b83184b 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/models.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/models.py @@ -11,7 +11,7 @@ from __future__ import annotations import json -from collections.abc import Callable, Iterable +from collections.abc import Callable, Iterable, Sequence from pathlib import Path from typing import Literal, TypeVar @@ -65,6 +65,55 @@ def pareto_sort( return out +INSIGHT_REWARD_PREFIX = "insight/" + + +def _merge_selection_rewards( + entries: Sequence[tuple[str, dict[str, float], dict[str, float]]], +) -> dict[str, dict[str, float]]: + """Merge ``(label, validation, insight_validation)`` triples into Pareto rewards. + + Insight validation metrics are namespaced under ``insight/`` so they add their own + axes rather than colliding with same-named validation keys. The union of Insight keys + is zero-filled across the set because :func:`_dominates` treats candidates whose key + sets differ as incomparable, which would otherwise leave a candidate missing an + Insight score silently undominated. + + Entries without a validation reward keep an empty mapping, preserving the existing + contract that unscored candidates are incomparable rather than dominated by everything. + """ + insight_keys = sorted({key for _, _, insight in entries for key in insight}) + rewards: dict[str, dict[str, float]] = {} + for label, validation, insight in entries: + if not validation: + rewards[label] = {} + continue + rewards[label] = { + **validation, + **{f"{INSIGHT_REWARD_PREFIX}{key}": insight.get(key, 0.0) for key in insight_keys}, + } + return rewards + + +def selection_rewards(candidates: Sequence[Candidate]) -> dict[str, dict[str, float]]: + """Return per-label Pareto rewards for candidates. See :func:`_merge_selection_rewards`.""" + return _merge_selection_rewards( + [ + (candidate.label, candidate.validation_reward or {}, candidate.insight_validation_reward or {}) + for candidate in candidates + ] + ) + + +def node_selection_rewards(nodes: Sequence[EvolutionNode]) -> dict[str, dict[str, float]]: + """Return per-label Pareto rewards for evolution nodes. + + The node-shaped twin of :func:`selection_rewards`, so every ranking path — survivor + selection, convergence, and winner choice — scores on the same dimensions. + """ + return _merge_selection_rewards([(node.label, node.val_reward, node.insight_val_reward) for node in nodes]) + + # --------------------------------------------------------------------------- # OptimizationType # --------------------------------------------------------------------------- @@ -151,6 +200,10 @@ def train_reward(self) -> dict[str, float]: def val_reward(self) -> dict[str, float]: return self.candidate.validation_reward or {} + @property + def insight_val_reward(self) -> dict[str, float]: + return self.candidate.insight_validation_reward or {} + @property def trajectory_reward(self) -> dict[str, float]: return self.candidate.validation_trajectory_reward or {} @@ -166,6 +219,8 @@ def reward_str(self) -> str: parts.append(f"tr[{_format_reward(self.train_reward)}]") if self.val_reward: parts.append(f"val[{_format_reward(self.val_reward)}]") + if self.insight_val_reward: + parts.append(f"insight-val[{_format_reward(self.insight_val_reward)}]") if self.trajectory_reward: parts.append(f"traj[{_format_reward(self.trajectory_reward)}]") return " ".join(parts) if parts else "no rewards" @@ -230,11 +285,12 @@ def mark_best(self, label: str) -> None: self.nodes[label].is_best = True def get_best(self) -> list[EvolutionNode]: - """Return the Pareto-optimal nodes by validation reward.""" + """Return the Pareto-optimal nodes by validation and held-out Insight reward.""" scored = [n for n in self.nodes.values() if n.val_reward] if not scored: return [] - return pareto_front(scored, lambda n: n.val_reward) + rewards = node_selection_rewards(scored) + return pareto_front(scored, lambda n: rewards[n.label]) def to_markdown_table(self) -> str: """Export as a markdown table with all score dimensions as columns.""" diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/terminator.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/terminator.py index 60ea30a31d..2f508d586a 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/terminator.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/terminator.py @@ -24,7 +24,7 @@ from pydantic import BaseModel from .model_config import get_fast_model -from .models import EvolutionTree, pareto_front +from .models import EvolutionTree, node_selection_rewards, pareto_front logger = logging.getLogger(__name__) @@ -156,8 +156,12 @@ async def _has_converged( old = [n for n in scored if n.round <= cutoff_round] if not old: return False - old_front_ids = {n.label for n in pareto_front(old, lambda n: n.val_reward)} - full_front_ids = {n.label for n in pareto_front(scored, lambda n: n.val_reward)} + # Rank on the same merged dimensions as survivor and winner selection: a candidate + # whose only gain is on the held-out Insight half still moves the front, so the run + # is not called converged while it is still improving on the Insight tasks. + rewards = node_selection_rewards(scored) + old_front_ids = {n.label for n in pareto_front(old, lambda n: rewards[n.label])} + full_front_ids = {n.label for n in pareto_front(scored, lambda n: rewards[n.label])} if full_front_ids.issubset(old_front_ids): return True return await self.qualitative_stop_check(prior_analysis) diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py index 75dccfe136..bed9a50af5 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py @@ -21,11 +21,16 @@ from typing import Any from nemo_experimentalist_plugin.entities import Candidate, ExperimentRun +from nemo_experimentalist_plugin.experimentalist.components.holdout_utils import ( + INSIGHT_TRAIN_SPLIT, + INSIGHT_VALIDATION_SPLIT, + VALIDATION_SPLIT, +) from nemo_platform import AsyncNeMoPlatform, ConflictError, NotFoundError, omit logger = logging.getLogger(__name__) -SPLITS: tuple[str, ...] = ("train", "validation", "insight") +SPLITS: tuple[str, ...] = ("train", VALIDATION_SPLIT, INSIGHT_TRAIN_SPLIT, INSIGHT_VALIDATION_SPLIT) _NAME_RE = re.compile(r"[^a-z0-9-]+") @@ -95,13 +100,13 @@ def experiment_metadata(candidate: Candidate, split: str) -> dict[str, str]: def _split_reward(candidate: Candidate, split: str) -> Any: """The candidate's reward object for *split* — an explicit lookup over the known - split fields (``train_reward``/``validation_reward``/``insight_reward``) rather - than a dynamic attribute read. Used only as a presence check: the reward value - itself is never projected.""" + split fields rather than a dynamic attribute read. Used only as a presence check: + the reward value itself is never projected.""" return { "train": candidate.train_reward, - "validation": candidate.validation_reward, - "insight": candidate.insight_reward, + VALIDATION_SPLIT: candidate.validation_reward, + INSIGHT_TRAIN_SPLIT: candidate.insight_train_reward, + INSIGHT_VALIDATION_SPLIT: candidate.insight_validation_reward, }[split] diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_holdout_utils.py b/plugins/nemo-experimentalist/tests/experimentalist/test_holdout_utils.py new file mode 100644 index 0000000000..3dac228178 --- /dev/null +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_holdout_utils.py @@ -0,0 +1,99 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from pathlib import Path + +import pytest +from nemo_eval_author_plugin.eval_author import materialization +from nemo_experimentalist_plugin.experimentalist.components.holdout_utils import ( + DEFAULT_BLOCKED_PATHS, + HELD_OUT_SPLITS, + HELD_OUT_STORAGE_DIR, + INSIGHT_TRAIN_SPLIT, + INSIGHT_VALIDATION_SPLIT, + VALIDATION_SPLIT, + ensure_heldout_hidden, + restore_heldout_splits, +) + + +def _stage(workspace: Path, split: str) -> Path: + task_dir = workspace / "dataset" / split / "000-task" + task_dir.mkdir(parents=True) + (task_dir / "solution.sh").write_text(f"{split} answer\n") + return workspace / "dataset" / split + + +def test_insight_split_names_match_eval_author() -> None: + """Eval Author names the materialized directories; holdout derives blocked tokens from them.""" + assert INSIGHT_TRAIN_SPLIT == materialization.INSIGHT_TRAIN_SPLIT + assert INSIGHT_VALIDATION_SPLIT == materialization.INSIGHT_VALIDATION_SPLIT + + +def test_insight_validation_is_held_out_and_insight_train_is_not() -> None: + assert INSIGHT_VALIDATION_SPLIT in HELD_OUT_SPLITS + assert INSIGHT_TRAIN_SPLIT not in HELD_OUT_SPLITS + assert f"dataset/{INSIGHT_VALIDATION_SPLIT}" in DEFAULT_BLOCKED_PATHS + assert f"dataset/{INSIGHT_TRAIN_SPLIT}" not in DEFAULT_BLOCKED_PATHS + assert HELD_OUT_STORAGE_DIR in DEFAULT_BLOCKED_PATHS + + +def test_hiding_relocates_insight_validation_and_leaves_insight_train_visible(tmp_path: Path) -> None: + insight_validation = _stage(tmp_path, INSIGHT_VALIDATION_SPLIT) + insight_train = _stage(tmp_path, INSIGHT_TRAIN_SPLIT) + + ensure_heldout_hidden(tmp_path) + + hidden = tmp_path / HELD_OUT_STORAGE_DIR / INSIGHT_VALIDATION_SPLIT + assert not insight_validation.exists() + assert (hidden / "000-task" / "solution.sh").read_text() == f"{INSIGHT_VALIDATION_SPLIT} answer\n" + assert (insight_train / "000-task" / "solution.sh").exists() + + +@pytest.mark.parametrize("split", [VALIDATION_SPLIT, INSIGHT_VALIDATION_SPLIT]) +def test_scoring_round_trip_restores_then_re_hides_each_held_out_split(tmp_path: Path, split: str) -> None: + visible = _stage(tmp_path, split) + splits = frozenset({split}) + + ensure_heldout_hidden(tmp_path, splits=splits) + assert not visible.exists() + + restore_heldout_splits(tmp_path, splits=splits) + assert (visible / "000-task" / "solution.sh").read_text() == f"{split} answer\n" + + ensure_heldout_hidden(tmp_path, splits=splits) + assert not visible.exists() + assert (tmp_path / HELD_OUT_STORAGE_DIR / split / "000-task" / "solution.sh").exists() + + +def test_restoring_one_half_leaves_the_other_hidden(tmp_path: Path) -> None: + _stage(tmp_path, VALIDATION_SPLIT) + _stage(tmp_path, INSIGHT_VALIDATION_SPLIT) + ensure_heldout_hidden(tmp_path) + + restore_heldout_splits(tmp_path, splits=frozenset({INSIGHT_VALIDATION_SPLIT})) + + assert (tmp_path / "dataset" / INSIGHT_VALIDATION_SPLIT).exists() + assert not (tmp_path / "dataset" / VALIDATION_SPLIT).exists() + assert (tmp_path / HELD_OUT_STORAGE_DIR / VALIDATION_SPLIT).exists() + + +def test_hiding_is_idempotent_so_it_can_run_before_every_phase(tmp_path: Path) -> None: + visible = _stage(tmp_path, INSIGHT_VALIDATION_SPLIT) + hidden = tmp_path / HELD_OUT_STORAGE_DIR / INSIGHT_VALIDATION_SPLIT + + for _ in range(3): + ensure_heldout_hidden(tmp_path) + assert not visible.exists() + assert (hidden / "000-task" / "solution.sh").read_text() == f"{INSIGHT_VALIDATION_SPLIT} answer\n" + + +def test_restoring_replaces_a_path_a_candidate_re_created_while_it_was_hidden(tmp_path: Path) -> None: + visible = _stage(tmp_path, INSIGHT_VALIDATION_SPLIT) + ensure_heldout_hidden(tmp_path) + (visible / "000-task").mkdir(parents=True) + (visible / "000-task" / "solution.sh").write_text("forged\n") + + restore_heldout_splits(tmp_path, splits=frozenset({INSIGHT_VALIDATION_SPLIT})) + + assert (visible / "000-task" / "solution.sh").read_text() == f"{INSIGHT_VALIDATION_SPLIT} answer\n" diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_insight_split_contract.py b/plugins/nemo-experimentalist/tests/experimentalist/test_insight_split_contract.py new file mode 100644 index 0000000000..37b8260286 --- /dev/null +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_insight_split_contract.py @@ -0,0 +1,124 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The handoff contract between Eval Author's split and Experimentalist's consumption of it. + +Eval Author materializes the two Insight halves; the loop then reads provenance off each +one, hides the validation half, and blocks shell access to it. Those live on opposite sides +of a plugin boundary, so the pieces are pinned together here against a real materialized +suite rather than a hand-built ``Dataset``. +""" + +from pathlib import Path + +import pytest +from nemo_eval_author_plugin.eval_author.materialization import ( + InsightSuite, + InsightSuiteSplit, + materialize_insight_split, +) +from nemo_experimentalist_plugin.experimentalist.components.evaluator import Task +from nemo_experimentalist_plugin.experimentalist.components.holdout_utils import ( + HELD_OUT_STORAGE_DIR, + INSIGHT_TRAIN_SPLIT, + INSIGHT_VALIDATION_SPLIT, + ensure_heldout_hidden, +) +from nemo_experimentalist_plugin.experimentalist.components.insight_promotion import insight_suite_provenance +from nemo_experimentalist_plugin.experimentalist.components.tools import GuardedShellTools + +_TASK_TOML = """ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +schema_version = "1.1" + +[task] +name = "example/template__placeholder" + +[metadata] +difficulty = "easy" + +[environment] +build_timeout_sec = 60.0 +""".lstrip() + + +def _write_template(root: Path) -> Task: + root.mkdir(parents=True) + (root / "task.toml").write_text(_TASK_TOML, encoding="utf-8") + (root / "instruction.md").write_text("{{ instruction }}\n", encoding="utf-8") + (root / "environment").mkdir() + (root / "environment" / "Dockerfile").write_text("FROM ubuntu:24.04\n", encoding="utf-8") + (root / "tests").mkdir() + (root / "tests" / "test.sh").write_text("#!/bin/sh\nmkdir -p /logs/verifier\necho 1 > /logs/verifier/reward.txt\n") + return Task(id="task-template", uri=root.as_uri()) + + +@pytest.fixture +def split(tmp_path: Path) -> InsightSuiteSplit: + template = _write_template(tmp_path / "template") + refs = [f"trace-{index}" for index in range(1, 5)] + suite = InsightSuite(experiment_dir=tmp_path, insight_id="insight-1", task_template=template) + staged = suite.stage(refs) + for task in staged: + (task.path / "instruction.md").write_text(f"Reproduce {task.trace_ref}.\n", encoding="utf-8") + suite.validate(task) + suite.promote_local(refs, staged) + return materialize_insight_split( + suite.finalize(), + train_dir=tmp_path / "dataset" / INSIGHT_TRAIN_SPLIT, + validation_dir=tmp_path / "dataset" / INSIGHT_VALIDATION_SPLIT, + ) + + +def test_each_half_satisfies_the_provenance_the_loop_requires(split: InsightSuiteSplit) -> None: + assert split.train is not None and split.validation is not None + provenances = [insight_suite_provenance(half.dataset) for half in (split.train, split.validation)] + + train_provenance, validation_provenance = provenances + assert train_provenance.identity != validation_provenance.identity + for provenance, half in zip(provenances, (split.train, split.validation), strict=True): + assert provenance.identity == half.identity + assert provenance.scorer_identity == half.scorer_identity + assert provenance.suite_path == half.path.resolve() + assert set(provenance.task_hashes) == {task.id for task in half.dataset.list_tasks()} + + +def test_the_halves_partition_the_authored_suite(split: InsightSuiteSplit) -> None: + assert split.train is not None and split.validation is not None + train_tasks = {task.id for task in split.train.dataset.list_tasks()} + validation_tasks = {task.id for task in split.validation.dataset.list_tasks()} + + assert not train_tasks & validation_tasks + assert len(train_tasks) == len(validation_tasks) == 2 + + +def test_the_validation_half_lands_where_the_holdout_mechanism_looks_for_it( + split: InsightSuiteSplit, + tmp_path: Path, +) -> None: + assert split.validation is not None + assert split.validation.path == tmp_path / "dataset" / INSIGHT_VALIDATION_SPLIT + + ensure_heldout_hidden(tmp_path) + + assert not split.validation.path.exists() + hidden = tmp_path / HELD_OUT_STORAGE_DIR / INSIGHT_VALIDATION_SPLIT + assert {path.name for path in hidden.iterdir()} >= {task.id for task in split.validation.dataset.list_tasks()} + + +async def test_the_shell_refuses_the_validation_half_by_its_materialized_path( + split: InsightSuiteSplit, + tmp_path: Path, +) -> None: + assert split.validation is not None + task_id = next(task.id for task in split.validation.dataset.list_tasks()) + shell = GuardedShellTools(cwd=tmp_path) + try: + result = await shell.run(f"cat dataset/{INSIGHT_VALIDATION_SPLIT}/{task_id}/instruction.md") + finally: + await shell.close() + + assert not result.success + assert result.stdout == "" diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_signal_invariants.py b/plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_signal_invariants.py new file mode 100644 index 0000000000..4dcab0593a --- /dev/null +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_signal_invariants.py @@ -0,0 +1,448 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Multi-round guards that every living candidate keeps full Insight signal. + +The single-round harness in ``test_loop_insight_suite.py`` never exercises the +multi-candidate path: round 0 has only the baseline, so survivor selection takes +the ``list(candidates)`` branch and no candidate is ever replaced by a slimmed +copy. These tests run the loop for several rounds with the real evaluation, +selection, and caching code so the per-round signal contract is checked where it +can actually break. +""" + +from dataclasses import dataclass, field +from pathlib import Path +from types import SimpleNamespace +from typing import Any, cast +from unittest.mock import AsyncMock + +import pytest +from nemo_experimentalist_plugin.entities import ( + INSIGHT_TRAIN_FIELDS, + INSIGHT_VALIDATION_FIELDS, + Candidate, + InsightSplitFields, +) +from nemo_experimentalist_plugin.experimentalist.components import loop as loop_module +from nemo_experimentalist_plugin.experimentalist.components.evaluator import ( + Dataset, + EvaluationResult, + MetricResult, + Task, + TrialResult, +) +from nemo_experimentalist_plugin.experimentalist.components.evaluator.models import ( + DatasetRef, + DataValue, + ResourceRef, +) +from nemo_experimentalist_plugin.experimentalist.components.holdout_utils import ( + INSIGHT_TRAIN_SPLIT, + INSIGHT_VALIDATION_SPLIT, +) +from nemo_experimentalist_plugin.experimentalist.components.loop import EvolutionaryOptimizer +from nemo_experimentalist_plugin.experimentalist.components.models import ( + INSIGHT_REWARD_PREFIX, + EvolutionTree, + selection_rewards, +) +from nemo_experimentalist_plugin.experimentalist.deps import ExperimentalistDeps +from nemo_experimentalist_plugin.resolve import EvolutionaryOptimizerConfig + +_ROUNDS = 3 +_METRIC = "uses_required_tool" +_TRAIN_TASK = "insight-train-task" +_VALIDATION_TASK = "insight-validation-task" + + +class _StopAfterRounds(Exception): + pass + + +@pytest.fixture(autouse=True) +def _keep_all_ranked_survivors() -> Any: + """Take the Pareto order as-is instead of calling the LLM diversity strategy. + + ``select_diverse_survivors`` is a public strategy method, so the agent + metaclass rejects ``monkeypatch.setattr``; swap it the way the framework's + own class construction does. + """ + + async def keep_top_k(self: EvolutionaryOptimizer, ranked: list[Candidate], k: int) -> list[Candidate]: + return list(ranked[:k]) + + original = EvolutionaryOptimizer.select_diverse_survivors + type.__setattr__(EvolutionaryOptimizer, "select_diverse_survivors", keep_top_k) + try: + yield + finally: + type.__setattr__(EvolutionaryOptimizer, "select_diverse_survivors", original) + + +def _suite_metadata(identity_char: str, task_id: str) -> dict[str, DataValue]: + return { + "insight_suite_identity": f"sha256:{identity_char * 64}", + "insight_suite_scorer_identity": f"sha256:{'b' * 64}", + "insight_suite_task_hashes": { + task_id: { + "content_hash": f"sha256:{'c' * 64}", + "verifier_hash": f"sha256:{'d' * 64}", + } + }, + } + + +def _result(*, result_id: str, label: str, task_id: str, score: float, attempts: int = 1) -> EvaluationResult: + return EvaluationResult( + id=result_id, + aggregate_metrics={_METRIC: score}, + trials=[ + TrialResult( + id=f"{label}-{task_id}-{attempt}", + task_id=task_id, + attempt=attempt, + status="completed", + metrics={_METRIC: MetricResult(name=_METRIC, value=score)}, + ) + for attempt in range(1, attempts + 1) + ], + ) + + +@dataclass +class _RoundRecord: + """What the loop handed the analyzer, and what the candidates carried, in one round.""" + + round_num: int + survivor_labels: list[str] + analyzer_insight_trials: dict[str, list[str]] + analyzer_train_trials: dict[str, list[str]] + + +@dataclass +class _Harness: + optimizer: EvolutionaryOptimizer + deps: ExperimentalistDeps + tree: EvolutionTree + insight_train_dataset: Dataset + insight_validation_dataset: Dataset + rounds: list[_RoundRecord] = field(default_factory=list) + insight_evaluated: list[tuple[str, str]] = field(default_factory=list) + + def living(self) -> list[Candidate]: + return [node.candidate for node in self.tree.nodes.values() if node.is_survivor] + + +def _install(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> _Harness: + insight_train_dataset = Dataset( + id="insight-train", + source=ResourceRef(uri="file:///experiment/dataset/insight-train"), + tasks=[Task(id=_TRAIN_TASK)], + metadata=_suite_metadata("a", _TRAIN_TASK), + ) + insight_validation_dataset = Dataset( + id="insight-validation", + source=ResourceRef(uri="file:///experiment/dataset/insight-validation"), + tasks=[Task(id=_VALIDATION_TASK)], + metadata=_suite_metadata("e", _VALIDATION_TASK), + ) + train_dataset = Dataset(id="train", tasks=[Task(id="train-task")]) + validation_dataset = Dataset(id="validation", tasks=[Task(id="validation-task")]) + datasets = {"train": train_dataset, "validation": validation_dataset} + + class _DatasetFactory: + def build_dataset(self, evaluator_type: str, ref: DatasetRef) -> Dataset: + return datasets[ref.uri] + + def build_task_template(self, evaluator_type: str, ref: DatasetRef) -> Task: + return Task(id="template", uri=ref.uri) + + class _EvalAuthor: + def __init__(self, **kwargs: object) -> None: + pass + + async def run(self, **kwargs: Any) -> SimpleNamespace: + return SimpleNamespace( + train_dataset=kwargs["train_dataset"], + validation_dataset=kwargs["validation_dataset"], + insight_train_suite=insight_train_dataset, + insight_validation_suite=insight_validation_dataset, + ) + + baseline = Candidate(run_id="run-1", label="agent-0", round=0, optimization="baseline") + tree = EvolutionTree() + tree.add(baseline) + + # Later candidates score higher on every split, so selection always has a + # reason to keep both the newcomer and an older candidate alive. + def _score(label: str) -> float: + return min(1.0, 0.1 * (int(label.split("-")[1]) + 1)) + + harness = _Harness( + optimizer=cast(EvolutionaryOptimizer, None), + deps=cast(ExperimentalistDeps, None), + tree=tree, + insight_train_dataset=insight_train_dataset, + insight_validation_dataset=insight_validation_dataset, + ) + + async def evaluate_agent( + self: EvolutionaryOptimizer, + candidate: Candidate, + dataset: Dataset, + evaluator: object, + task_ids: list[str] | None = None, + minimum_attempts: int | None = None, + ) -> tuple[Candidate, EvaluationResult]: + task_id = { + "insight-train": _TRAIN_TASK, + "insight-validation": _VALIDATION_TASK, + "train": "train-task", + "validation": "validation-task", + }[dataset.id] + if dataset.id.startswith("insight-"): + harness.insight_evaluated.append((dataset.id, candidate.label)) + return candidate, _result( + result_id=f"{candidate.label}-{dataset.id}", + label=candidate.label, + task_id=task_id, + score=_score(candidate.label), + attempts=minimum_attempts or 1, + ) + + async def update_candidate( + self: EvolutionaryOptimizer, + candidate: Candidate, + *, + updates: dict[str, object] | None = None, + **kwargs: object, + ) -> None: + for key, value in (updates or {}).items(): + setattr(candidate, key, value) + + counter = {"n": 0} + + def create_agent(self: EvolutionaryOptimizer, **kwargs: Any) -> Candidate: + counter["n"] += 1 + return Candidate( + run_id="run-1", + label=f"agent-{counter['n']}", + ancestor="agent-0", + round=kwargs["round_num"], + optimization=f"improvement {counter['n']}", + ) + + async def analyze_round(self: EvolutionaryOptimizer, **kwargs: Any) -> str: + insight_trials = kwargs.get("insight_trials") or {} + evaluations = kwargs["evaluations"] + harness.rounds.append( + _RoundRecord( + round_num=kwargs["round_num"], + survivor_labels=[c.label for c in kwargs["survivors"]], + analyzer_insight_trials={ + label: [t.task_id for t in trials] for label, trials in insight_trials.items() + }, + analyzer_train_trials={ + label: [t.task_id for t in result.trials] for label, result in evaluations.items() + }, + ) + ) + return "round analysis" + + class _Terminator: + calls = 0 + + async def run(self, **kwargs: object) -> SimpleNamespace: + self.calls += 1 + if self.calls > _ROUNDS: + raise _StopAfterRounds + return SimpleNamespace(stop=False, reason="continue") + + run_entity = SimpleNamespace(id="run-1", status="running", rounds_completed=0) + backend = SimpleNamespace( + client=object(), + get_insight=AsyncMock(return_value=SimpleNamespace(agent="agent-source")), + get_agent_code=AsyncMock(), + persist_evaluation=AsyncMock(), + update_run=AsyncMock(), + ) + + monkeypatch.setattr(loop_module, "DatasetFactory", _DatasetFactory) + monkeypatch.setattr( + loop_module, + "EvaluatorFactory", + lambda: SimpleNamespace(build_evaluator=lambda *args, **kwargs: object()), + ) + monkeypatch.setattr(loop_module, "EvalAuthor", _EvalAuthor) + monkeypatch.setattr( + loop_module, + "stage_eval_author_inputs", + AsyncMock(side_effect=lambda _, **refs: SimpleNamespace(**refs)), + ) + monkeypatch.setattr(loop_module.EvolutionTree, "from_dir", lambda path: tree) + monkeypatch.setattr(EvolutionaryOptimizer, "_detect_last_round", lambda self: None) + monkeypatch.setattr(EvolutionaryOptimizer, "_create_experiment_run", AsyncMock(return_value=run_entity)) + monkeypatch.setattr(EvolutionaryOptimizer, "_create_baseline_agent", AsyncMock(return_value=baseline)) + monkeypatch.setattr(EvolutionaryOptimizer, "_update_candidate", update_candidate) + monkeypatch.setattr(EvolutionaryOptimizer, "_evaluate_agent", evaluate_agent) + monkeypatch.setattr(EvolutionaryOptimizer, "_generate_initial_goal_tree", AsyncMock()) + monkeypatch.setattr(EvolutionaryOptimizer, "_analyze_round", analyze_round) + monkeypatch.setattr(EvolutionaryOptimizer, "_update_goal_tree", AsyncMock()) + monkeypatch.setattr(EvolutionaryOptimizer, "_propose_improvements", AsyncMock(return_value=[object()])) + monkeypatch.setattr(EvolutionaryOptimizer, "_create_agent", create_agent) + monkeypatch.setattr( + EvolutionaryOptimizer, + "_implement_candidates", + AsyncMock(side_effect=lambda **kwargs: kwargs["candidates"]), + ) + + config = EvolutionaryOptimizerConfig(disable_trajectory_scoring=True, max_survivors=3) + optimizer = object.__new__(EvolutionaryOptimizer) + optimizer.working_dir = tmp_path + optimizer.config = config + optimizer.shell = SimpleNamespace(close=AsyncMock()) + optimizer.terminator = _Terminator() + deps = SimpleNamespace( + backend=backend, + workspace="default", + config=config, + evaluator_type="harbor", + train_dataset=DatasetRef(uri="train"), + validation_dataset=DatasetRef(uri="validation"), + task_template=DatasetRef(uri="template"), + insight="insight-1", + agent=None, + agent_spec=None, + ) + harness.optimizer = optimizer + harness.deps = cast(ExperimentalistDeps, deps) + return harness + + +async def _run(harness: _Harness) -> None: + with pytest.raises(_StopAfterRounds): + await harness.optimizer.run(harness.deps) + + +@pytest.mark.asyncio +async def test_the_analyzer_gets_insight_trials_in_every_round_not_just_the_baseline( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + harness = _install(monkeypatch, tmp_path) + await _run(harness) + + assert len(harness.rounds) == _ROUNDS + starved = [ + record.round_num + for record in harness.rounds + if set(record.analyzer_insight_trials) != set(record.survivor_labels) + ] + assert not starved, ( + f"rounds {starved} sent the analyzer no Insight trials for some survivor; " + f"per round: {[(r.round_num, r.survivor_labels, sorted(r.analyzer_insight_trials)) for r in harness.rounds]}" + ) + for record in harness.rounds: + for label, task_ids in record.analyzer_insight_trials.items(): + assert set(task_ids) == {_TRAIN_TASK}, f"round {record.round_num} {label} got {task_ids}" + assert len(task_ids) == 2, ( + f"round {record.round_num} {label} carried {len(task_ids)} attempts; " + "Insight scoring runs with minimum_attempts=2 so a single flaky run cannot set the score" + ) + + +@pytest.mark.asyncio +async def test_the_analyzer_gets_train_trials_for_survivors_it_did_not_re_evaluate( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + harness = _install(monkeypatch, tmp_path) + await _run(harness) + + starved = [ + (record.round_num, label) + for record in harness.rounds + for label, task_ids in record.analyzer_train_trials.items() + if not task_ids + ] + assert not starved, f"the analyzer received an evaluation with no trials for {starved}" + + +@pytest.mark.asyncio +async def test_every_living_candidate_carries_both_insight_halves( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + harness = _install(monkeypatch, tmp_path) + await _run(harness) + + for candidate in harness.living(): + for fields in (INSIGHT_TRAIN_FIELDS, INSIGHT_VALIDATION_FIELDS): + assert fields.reward_of(candidate) is not None, f"{candidate.label} has no {fields.split} reward" + assert fields.reward_details_of(candidate), f"{candidate.label} has no {fields.split} trials" + assert fields.metric_keys_of(candidate) == [_METRIC] + assert {c.label for c in harness.living()} >= {"agent-0", "agent-1"} + + +@pytest.mark.asyncio +async def test_each_candidate_is_scored_once_per_insight_half( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + harness = _install(monkeypatch, tmp_path) + await _run(harness) + + seen: dict[tuple[str, str], int] = {} + for entry in harness.insight_evaluated: + seen[entry] = seen.get(entry, 0) + 1 + repeats = {entry: count for entry, count in seen.items() if count > 1} + assert not repeats, f"re-scored against an unchanged suite: {repeats}" + assert {label for _, label in harness.insight_evaluated} == {c.label for c in harness.living()} | { + node.candidate.label for node in harness.tree.nodes.values() + } + + +@pytest.mark.asyncio +async def test_insight_validation_reaches_pareto_selection_for_every_ranked_candidate( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + harness = _install(monkeypatch, tmp_path) + await _run(harness) + + living = harness.living() + rewards = selection_rewards(living) + for candidate in living: + merged = rewards[candidate.label] + assert merged, f"{candidate.label} contributed no selection reward" + assert f"{INSIGHT_REWARD_PREFIX}{_METRIC}" in merged, ( + f"{candidate.label} reached selection without an Insight dimension: {sorted(merged)}" + ) + + +@pytest.mark.asyncio +async def test_every_candidate_is_pinned_to_the_suite_it_was_scored_against( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + harness = _install(monkeypatch, tmp_path) + await _run(harness) + + expectations = { + INSIGHT_TRAIN_FIELDS: f"sha256:{'a' * 64}", + INSIGHT_VALIDATION_FIELDS: f"sha256:{'e' * 64}", + } + for candidate in harness.living(): + for fields, identity in expectations.items(): + assert fields.suite_identity_of(candidate) == identity, f"{candidate.label} {fields.split} identity drifted" + + +def _split_of(fields: InsightSplitFields) -> str: + return fields.split + + +def test_the_two_halves_are_distinct_splits() -> None: + assert _split_of(INSIGHT_TRAIN_FIELDS) == INSIGHT_TRAIN_SPLIT + assert _split_of(INSIGHT_VALIDATION_FIELDS) == INSIGHT_VALIDATION_SPLIT + assert INSIGHT_TRAIN_SPLIT != INSIGHT_VALIDATION_SPLIT diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py b/plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py index e959de65be..f3ef605244 100644 --- a/plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py @@ -1,13 +1,18 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +from dataclasses import dataclass from pathlib import Path from types import SimpleNamespace -from typing import Any +from typing import Any, cast from unittest.mock import AsyncMock import pytest -from nemo_experimentalist_plugin.entities import Candidate +from nemo_experimentalist_plugin.entities import ( + INSIGHT_TRAIN_FIELDS, + Candidate, + InsightSplitFields, +) from nemo_experimentalist_plugin.experimentalist.components import loop as loop_module from nemo_experimentalist_plugin.experimentalist.components.evaluator import ( Dataset, @@ -22,7 +27,12 @@ DataValue, ResourceRef, ) +from nemo_experimentalist_plugin.experimentalist.components.holdout_utils import ( + INSIGHT_TRAIN_SPLIT, + INSIGHT_VALIDATION_SPLIT, +) from nemo_experimentalist_plugin.experimentalist.components.loop import EvolutionaryOptimizer +from nemo_experimentalist_plugin.experimentalist.deps import ExperimentalistDeps from nemo_experimentalist_plugin.resolve import EvolutionaryOptimizerConfig @@ -30,13 +40,13 @@ class _StopAfterOneRound(Exception): pass -def _suite_metadata(identity_char: str = "a") -> dict[str, DataValue]: +def _suite_metadata(identity_char: str = "a", task_id: str = "insight-task") -> dict[str, DataValue]: identity = f"sha256:{identity_char * 64}" return { "insight_suite_identity": identity, "insight_suite_scorer_identity": f"sha256:{'b' * 64}", "insight_suite_task_hashes": { - "insight-task": { + task_id: { "content_hash": f"sha256:{'c' * 64}", "verifier_hash": f"sha256:{'d' * 64}", } @@ -44,14 +54,16 @@ def _suite_metadata(identity_char: str = "a") -> dict[str, DataValue]: } -def _insight_result(label: str, score: float) -> EvaluationResult: +def _insight_result( + label: str, score: float, split: str = "insight", task_id: str = "insight-task" +) -> EvaluationResult: return EvaluationResult( - id=f"{label}-insight", + id=f"{label}-{split}", aggregate_metrics={"uses_required_tool": score}, trials=[ TrialResult( - id=f"{label}-insight-task-1", - task_id="insight-task", + id=f"{label}-{task_id}-1", + task_id=task_id, attempt=1, status="completed", metrics={ @@ -65,18 +77,41 @@ def _insight_result(label: str, score: float) -> EvaluationResult: ) -@pytest.mark.asyncio -async def test_insight_run_evaluates_and_persists_baseline_and_new_candidate_metrics( +@dataclass +class _Harness: + """A loop run with everything but the Insight bookkeeping stubbed out.""" + + optimizer: EvolutionaryOptimizer + deps: ExperimentalistDeps + backend: SimpleNamespace + baseline: Candidate + new_candidate: Candidate + insight_train_dataset: Dataset + insight_validation_dataset: Dataset + insight_evaluations: list[tuple[str, Dataset, list[Candidate]]] + analyze_round: AsyncMock + + +def _install_loop_harness( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, -) -> None: + *, + validation_metrics: dict[str, float] | None = None, + train_metrics: dict[str, float] | None = None, +) -> _Harness: train_dataset = Dataset(id="train") validation_dataset = Dataset(id="validation") - insight_dataset = Dataset( - id="insight-suite", - source=ResourceRef(uri="file:///experiment/eval-and-optimize/eval_author/insight-1/insight-suite"), + insight_train_dataset = Dataset( + id="insight-train", + source=ResourceRef(uri="file:///experiment/dataset/insight-train"), tasks=[Task(id="insight-task")], - metadata=_suite_metadata(), + metadata=_suite_metadata("a"), + ) + insight_validation_dataset = Dataset( + id="insight-validation", + source=ResourceRef(uri="file:///experiment/dataset/insight-validation"), + tasks=[Task(id="insight-task-2")], + metadata=_suite_metadata("e", task_id="insight-task-2"), ) datasets = { "train": train_dataset, @@ -98,7 +133,8 @@ async def run(self, **kwargs: Any) -> SimpleNamespace: return SimpleNamespace( train_dataset=kwargs["train_dataset"], validation_dataset=kwargs["validation_dataset"], - insight_suite=insight_dataset, + insight_train_suite=insight_train_dataset, + insight_validation_suite=insight_validation_dataset, ) baseline = Candidate(run_id="run-1", label="agent-0", round=0, optimization="baseline") @@ -110,20 +146,27 @@ async def run(self, **kwargs: Any) -> SimpleNamespace: optimization="use the required tool", ) insight_results = { - "agent-0": _insight_result("agent-0", 0.0), - "agent-1": _insight_result("agent-1", 1.0), + (INSIGHT_TRAIN_SPLIT, "agent-0"): _insight_result("agent-0", 0.0, INSIGHT_TRAIN_SPLIT), + (INSIGHT_TRAIN_SPLIT, "agent-1"): _insight_result("agent-1", 1.0, INSIGHT_TRAIN_SPLIT), + (INSIGHT_VALIDATION_SPLIT, "agent-0"): _insight_result( + "agent-0", 0.25, INSIGHT_VALIDATION_SPLIT, "insight-task-2" + ), + (INSIGHT_VALIDATION_SPLIT, "agent-1"): _insight_result( + "agent-1", 0.75, INSIGHT_VALIDATION_SPLIT, "insight-task-2" + ), } - insight_evaluations: list[tuple[Dataset, list[Candidate]]] = [] + insight_evaluations: list[tuple[str, Dataset, list[Candidate]]] = [] async def evaluate_insight_candidates( self: EvolutionaryOptimizer, *, dataset: Dataset, + fields: InsightSplitFields, evaluator: object, candidates: list[Candidate], ) -> dict[str, EvaluationResult]: - insight_evaluations.append((dataset, candidates)) - return {candidate.label: insight_results[candidate.label] for candidate in candidates} + insight_evaluations.append((fields.split, dataset, candidates)) + return {candidate.label: insight_results[(fields.split, candidate.label)] for candidate in candidates} async def evaluate_validation_candidates( self: EvolutionaryOptimizer, @@ -134,7 +177,7 @@ async def evaluate_validation_candidates( return { candidate.label: EvaluationResult( id=f"{candidate.label}-validation", - aggregate_metrics={"reward": 0.5}, + aggregate_metrics=dict(validation_metrics or {"reward": 0.5, "uses_required_tool": 0.5}), ) for candidate in candidates if candidate.validation_reward is None @@ -169,6 +212,8 @@ async def run(self, **kwargs: object) -> SimpleNamespace: raise _StopAfterOneRound return SimpleNamespace(stop=False, reason="continue") + analyze_round = AsyncMock(return_value="round analysis") + monkeypatch.setattr(loop_module, "DatasetFactory", RecordingDatasetFactory) monkeypatch.setattr( loop_module, @@ -203,11 +248,14 @@ async def run(self, **kwargs: object) -> SimpleNamespace: "_evaluate_train_candidates", AsyncMock( return_value={ - "agent-0": EvaluationResult(id="agent-0-train", aggregate_metrics={"reward": 0.5}), + "agent-0": EvaluationResult( + id="agent-0-train", + aggregate_metrics=dict(train_metrics or {"reward": 0.5, "uses_required_tool": 0.5}), + ), } ), ) - monkeypatch.setattr(EvolutionaryOptimizer, "_analyze_round", AsyncMock(return_value="round analysis")) + monkeypatch.setattr(EvolutionaryOptimizer, "_analyze_round", analyze_round) monkeypatch.setattr(EvolutionaryOptimizer, "_update_goal_tree", AsyncMock()) monkeypatch.setattr(EvolutionaryOptimizer, "_propose_improvements", AsyncMock(return_value=[object()])) monkeypatch.setattr(EvolutionaryOptimizer, "_create_agent", lambda self, **kwargs: new_candidate) @@ -235,30 +283,66 @@ async def run(self, **kwargs: object) -> SimpleNamespace: agent=None, agent_spec=None, ) + return _Harness( + optimizer=optimizer, + deps=cast(ExperimentalistDeps, deps), + backend=backend, + baseline=baseline, + new_candidate=new_candidate, + insight_train_dataset=insight_train_dataset, + insight_validation_dataset=insight_validation_dataset, + insight_evaluations=insight_evaluations, + analyze_round=analyze_round, + ) + + +@pytest.mark.asyncio +async def test_insight_run_evaluates_and_persists_baseline_and_new_candidate_metrics( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + harness = _install_loop_harness(monkeypatch, tmp_path) + baseline = harness.baseline + new_candidate = harness.new_candidate + insight_train_dataset = harness.insight_train_dataset + insight_validation_dataset = harness.insight_validation_dataset + insight_evaluations = harness.insight_evaluations + backend = harness.backend with pytest.raises(_StopAfterOneRound): - await optimizer.run(deps) + await harness.optimizer.run(harness.deps) assert insight_evaluations == [ - (insight_dataset, [baseline]), - (insight_dataset, [new_candidate]), + (INSIGHT_TRAIN_SPLIT, insight_train_dataset, [baseline]), + (INSIGHT_VALIDATION_SPLIT, insight_validation_dataset, [baseline]), + (INSIGHT_TRAIN_SPLIT, insight_train_dataset, [new_candidate]), + (INSIGHT_VALIDATION_SPLIT, insight_validation_dataset, [new_candidate]), ] - assert baseline.insight_reward == {"uses_required_tool": 0.0} - assert new_candidate.insight_reward == {"uses_required_tool": 1.0} - assert baseline.insight_suite_identity == f"sha256:{'a' * 64}" - assert new_candidate.insight_suite_identity == f"sha256:{'a' * 64}" - assert baseline.insight_metric_keys == ["uses_required_tool"] + assert baseline.insight_train_reward == {"uses_required_tool": 0.0} + assert new_candidate.insight_train_reward == {"uses_required_tool": 1.0} + assert baseline.insight_validation_reward == {"uses_required_tool": 0.25} + assert new_candidate.insight_validation_reward == {"uses_required_tool": 0.75} + assert baseline.insight_train_suite_identity == f"sha256:{'a' * 64}" + assert baseline.insight_validation_suite_identity == f"sha256:{'e' * 64}" + assert baseline.insight_train_metric_keys == ["uses_required_tool"] + assert baseline.insight_validation_metric_keys == ["uses_required_tool"] insight_persistence = [ - call.kwargs for call in backend.persist_evaluation.await_args_list if call.kwargs["split"] == "insight" + call.kwargs + for call in backend.persist_evaluation.await_args_list + if call.kwargs["split"] in {INSIGHT_TRAIN_SPLIT, INSIGHT_VALIDATION_SPLIT} ] - assert [call["candidate"] for call in insight_persistence] == [baseline, new_candidate] - assert [call["result"].id for call in insight_persistence] == [ - insight_results["agent-0"].id, - insight_results["agent-1"].id, + assert [(call["split"], call["candidate"]) for call in insight_persistence] == [ + (INSIGHT_TRAIN_SPLIT, baseline), + (INSIGHT_VALIDATION_SPLIT, baseline), + (INSIGHT_TRAIN_SPLIT, new_candidate), + (INSIGHT_VALIDATION_SPLIT, new_candidate), + ] + assert [call["result"].metadata["insight_suite_identity"] for call in insight_persistence] == [ + f"sha256:{'a' * 64}", + f"sha256:{'e' * 64}", + f"sha256:{'a' * 64}", + f"sha256:{'e' * 64}", ] - assert all( - call["result"].metadata["insight_suite_identity"] == f"sha256:{'a' * 64}" for call in insight_persistence - ) assert all( trial.metadata["insight_suite_scorer_identity"] == f"sha256:{'b' * 64}" for call in insight_persistence @@ -266,6 +350,92 @@ async def run(self, **kwargs: object) -> SimpleNamespace: ) +@pytest.mark.asyncio +async def test_the_validation_half_is_hidden_before_any_optimizing_agent_runs( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Hiding must not wait for the first scoring pass to hide it on the way out.""" + harness = _install_loop_harness(monkeypatch, tmp_path) + hidden_splits: list[frozenset[str]] = [] + monkeypatch.setattr( + loop_module, + "ensure_heldout_hidden", + lambda workspace, *, splits=None: hidden_splits.append(frozenset(splits or ())), + ) + monkeypatch.setattr(loop_module, "restore_heldout_splits", lambda workspace, **kwargs: None) + baseline_agent = AsyncMock(return_value=harness.baseline) + monkeypatch.setattr(EvolutionaryOptimizer, "_create_baseline_agent", baseline_agent) + + with pytest.raises(_StopAfterOneRound): + await harness.optimizer.run(harness.deps) + + assert hidden_splits[0] == frozenset({INSIGHT_VALIDATION_SPLIT}) + assert baseline_agent.await_count == 1 + + +@pytest.mark.asyncio +async def test_only_the_train_half_reaches_the_analyzer( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Analyzer output steers the goal tree and proposer, so the held-out half must not reach it.""" + harness = _install_loop_harness(monkeypatch, tmp_path) + + with pytest.raises(_StopAfterOneRound): + await harness.optimizer.run(harness.deps) + + assert harness.analyze_round.await_args is not None + kwargs = harness.analyze_round.await_args.kwargs + assert kwargs["insight_dataset"] is harness.insight_train_dataset + assert list(kwargs["insight_trials"]) == [harness.baseline.label] + trials = kwargs["insight_trials"][harness.baseline.label] + assert [trial.task_id for trial in trials] == ["insight-task"] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("missing_on", ["validation", "train"]) +async def test_an_insight_metric_missing_from_a_user_split_fails_in_round_zero( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + missing_on: str, +) -> None: + """The Eval Author authors one key set across three datasets; a gap must not surface mid-run.""" + complete = {"reward": 0.5, "uses_required_tool": 0.5} + harness = _install_loop_harness( + monkeypatch, + tmp_path, + validation_metrics={"reward": 0.5} if missing_on == "validation" else complete, + train_metrics={"reward": 0.5} if missing_on == "train" else complete, + ) + + with pytest.raises(ValueError) as exc_info: + await harness.optimizer.run(harness.deps) + + message = str(exc_info.value) + assert "Insight metrics ['uses_required_tool'] are missing" in message + assert f"from the {missing_on!r} split" in message + # The run stops before round 0 produces any optimization signal. + assert harness.analyze_round.await_count == 0 + + +@pytest.mark.asyncio +async def test_generic_reward_keys_are_not_required_of_the_user_splits( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Only Insight-specific metrics are the shared contract; `reward` is each suite's own.""" + harness = _install_loop_harness( + monkeypatch, + tmp_path, + validation_metrics={"uses_required_tool": 0.5}, + train_metrics={"uses_required_tool": 0.5}, + ) + + with pytest.raises(_StopAfterOneRound): + await harness.optimizer.run(harness.deps) + + @pytest.mark.asyncio async def test_insight_evaluation_skips_cached_candidates_and_empty_suites( monkeypatch: pytest.MonkeyPatch, @@ -275,10 +445,10 @@ async def test_insight_evaluation_skips_cached_candidates_and_empty_suites( label="agent-0", round=0, optimization="baseline", - insight_reward={"uses_required_tool": 0.0}, - insight_reward_details=[], - insight_suite_identity=f"sha256:{'a' * 64}", - insight_metric_keys=["uses_required_tool"], + insight_train_reward={"uses_required_tool": 0.0}, + insight_train_reward_details=[], + insight_train_suite_identity=f"sha256:{'a' * 64}", + insight_train_metric_keys=["uses_required_tool"], ) pending = Candidate( run_id="run-1", @@ -293,11 +463,12 @@ async def test_insight_evaluation_skips_cached_candidates_and_empty_suites( evaluated = await optimizer._evaluate_insight_candidates( dataset=Dataset( - id="insight-suite", - source=ResourceRef(uri="file:///experiment/eval-and-optimize/eval_author/insight-1/insight-suite"), + id="insight-train", + source=ResourceRef(uri="file:///experiment/dataset/insight-train"), tasks=[Task(id="insight-task")], metadata=_suite_metadata(), ), + fields=INSIGHT_TRAIN_FIELDS, evaluator=object(), # type: ignore[arg-type] candidates=[cached, pending], ) @@ -309,6 +480,7 @@ async def test_insight_evaluation_skips_cached_candidates_and_empty_suites( empty = await optimizer._evaluate_insight_candidates( dataset=Dataset(id="empty-insight-suite"), + fields=INSIGHT_TRAIN_FIELDS, evaluator=object(), # type: ignore[arg-type] candidates=[pending], ) @@ -325,10 +497,10 @@ async def test_insight_evaluation_reuses_only_matching_suite_identity( label="agent-0", round=0, optimization="baseline", - insight_reward={"uses_required_tool": 0.0}, - insight_reward_details=[], - insight_suite_identity=f"sha256:{'a' * 64}", - insight_metric_keys=["uses_required_tool"], + insight_train_reward={"uses_required_tool": 0.0}, + insight_train_reward_details=[], + insight_train_suite_identity=f"sha256:{'a' * 64}", + insight_train_metric_keys=["uses_required_tool"], ) result = _insight_result("agent-0", 0.5) evaluate_agent = AsyncMock(return_value=(cached, result)) @@ -336,14 +508,14 @@ async def test_insight_evaluation_reuses_only_matching_suite_identity( optimizer = object.__new__(EvolutionaryOptimizer) matching = Dataset( - id="insight-suite", - source=ResourceRef(uri="file:///experiment/eval-and-optimize/eval_author/insight-1/insight-suite"), + id="insight-train", + source=ResourceRef(uri="file:///experiment/dataset/insight-train"), tasks=[Task(id="insight-task")], metadata=_suite_metadata("a"), ) changed = Dataset( - id="insight-suite", - source=ResourceRef(uri="file:///experiment/eval-and-optimize/eval_author/insight-1/insight-suite"), + id="insight-train", + source=ResourceRef(uri="file:///experiment/dataset/insight-train"), tasks=[Task(id="insight-task")], metadata=_suite_metadata("e"), ) @@ -351,6 +523,7 @@ async def test_insight_evaluation_reuses_only_matching_suite_identity( assert ( await optimizer._evaluate_insight_candidates( dataset=matching, + fields=INSIGHT_TRAIN_FIELDS, evaluator=object(), # type: ignore[arg-type] candidates=[cached], ) @@ -358,6 +531,7 @@ async def test_insight_evaluation_reuses_only_matching_suite_identity( ) assert await optimizer._evaluate_insight_candidates( dataset=changed, + fields=INSIGHT_TRAIN_FIELDS, evaluator=object(), # type: ignore[arg-type] candidates=[cached], ) == {"agent-0": result} @@ -375,25 +549,25 @@ async def test_cached_insight_metric_keys_are_order_independent( label="agent-0", round=0, optimization="baseline", - insight_reward={"reward": 0.5, "uses_required_tool": 0.0}, - insight_reward_details=[], - insight_suite_identity=identity, - insight_metric_keys=["uses_required_tool", "reward"], + insight_train_reward={"reward": 0.5, "uses_required_tool": 0.0}, + insight_train_reward_details=[], + insight_train_suite_identity=identity, + insight_train_metric_keys=["uses_required_tool", "reward"], ), Candidate( run_id="run-1", label="agent-1", round=1, optimization="improve tool use", - insight_reward={"reward": 0.75, "uses_required_tool": 1.0}, - insight_reward_details=[], - insight_suite_identity=identity, - insight_metric_keys=["reward", "uses_required_tool"], + insight_train_reward={"reward": 0.75, "uses_required_tool": 1.0}, + insight_train_reward_details=[], + insight_train_suite_identity=identity, + insight_train_metric_keys=["reward", "uses_required_tool"], ), ] dataset = Dataset( - id="insight-suite", - source=ResourceRef(uri="file:///experiment/eval-and-optimize/eval_author/insight-1/insight-suite"), + id="insight-train", + source=ResourceRef(uri="file:///experiment/dataset/insight-train"), tasks=[Task(id="insight-task")], metadata={ **_suite_metadata(), @@ -409,6 +583,7 @@ async def test_cached_insight_metric_keys_are_order_independent( await optimizer._evaluate_and_persist_insight_candidates( dataset=dataset, + fields=INSIGHT_TRAIN_FIELDS, evaluator=object(), # type: ignore[arg-type] candidates=candidates, workspace="default", diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py b/plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py index b5cbef7d0d..cfdd89f4ef 100644 --- a/plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py @@ -7,7 +7,11 @@ from unittest.mock import AsyncMock import pytest -from nemo_experimentalist_plugin.entities import Candidate +from nemo_experimentalist_plugin.entities import ( + INSIGHT_TRAIN_FIELDS, + INSIGHT_VALIDATION_FIELDS, + Candidate, +) from nemo_experimentalist_plugin.experimentalist.components.evaluator import ( Dataset, EvaluationResult, @@ -37,7 +41,8 @@ def _candidate( label: str, *, round_num: int, - insight_reward: dict[str, float] | None = None, + insight_train_reward: dict[str, float] | None = None, + insight_validation_reward: dict[str, float] | None = None, validation_reward: dict[str, float] | None = None, ) -> Candidate: return Candidate( @@ -45,10 +50,13 @@ def _candidate( label=label, round=round_num, optimization="baseline" if round_num == 0 else "improve required tool use", - insight_reward=insight_reward, + insight_train_reward=insight_train_reward, validation_reward=validation_reward, - insight_suite_identity=_SUITE_IDENTITY, - insight_metric_keys=["reward", "uses_required_tool"], + insight_train_suite_identity=_SUITE_IDENTITY, + insight_train_metric_keys=["reward", "uses_required_tool"], + insight_validation_reward=insight_validation_reward, + insight_validation_suite_identity=_SUITE_IDENTITY, + insight_validation_metric_keys=["reward", "uses_required_tool"], ) @@ -74,39 +82,54 @@ def _insight_dataset(tasks: list[Task]) -> Dataset: ) -def test_round_analysis_contract_requires_separate_insight_suite_dimensions() -> None: +def test_round_analysis_contract_keeps_the_two_insight_halves_in_separate_tables() -> None: skill_prompt = " ".join((AnalysisSkill.__doc__ or "").split()) merge_prompt = " ".join((EvolutionaryOptimizer.merge_analysis.__doc__ or "").split()) - assert "Insight Suite Reward" in skill_prompt - assert "candidate.insight_reward" in skill_prompt - assert "separate from train and validation rewards" in skill_prompt - assert "insight_dim_keys" in merge_prompt - assert "candidate.round == 0" in merge_prompt - assert "must name every available Insight Suite dimension" in merge_prompt - assert "Never blend those metrics into train/validation rewards" in merge_prompt - assert "adaptive/development feedback" in merge_prompt - assert "never present them as independent validation evidence" in merge_prompt + assert "Insight Train Reward:" in skill_prompt + assert "Insight Validation Reward:" in skill_prompt + assert "candidate.insight_train_reward" in skill_prompt + assert "candidate.insight_validation_reward" in skill_prompt + assert "Keep both separate from train and validation rewards" in skill_prompt + assert "insight_train_dim_keys" in merge_prompt + assert "insight_validation_dim_keys" in merge_prompt + assert "Never blend either half's metrics into the train or validation reward tables" in merge_prompt + + +def test_round_analysis_contract_distinguishes_development_feedback_from_ranking_evidence() -> None: + merge_prompt = " ".join((EvolutionaryOptimizer.merge_analysis.__doc__ or "").split()) + + train_rule, _, validation_rule = merge_prompt.partition("`insight_validation_reward` is held out") + + assert "adaptive/development feedback" in train_rule + assert "it did not affect ranking" in train_rule + assert "never present it as independent validation evidence" in train_rule + assert "independent scoring evidence and it did affect ranking" in validation_rule + assert "prefixed with `insight/`" in validation_rule -def test_final_report_contract_requires_baseline_winner_insight_comparison() -> None: +def test_final_report_contract_requires_a_baseline_winner_table_per_insight_half() -> None: report_prompt = " ".join((EvolutionaryOptimizer.write_final_report.__doc__ or "").split()) - assert "Insight Suite Metrics table" in report_prompt + assert "Insight Train Metrics and Insight Validation Metrics tables" in report_prompt assert "baseline, winner, and signed delta columns" in report_prompt - assert "Keep this table separate from generic train and validation rewards" in report_prompt + assert "Keep these tables separate from generic train and validation rewards" in report_prompt + assert "label the train half as adaptive/development feedback that did not affect ranking" in report_prompt + assert "the validation half as held-out evidence that did" in report_prompt -def test_terminal_summary_includes_baseline_and_winner_insight_metrics() -> None: +def test_terminal_summary_reports_the_held_out_insight_half() -> None: baseline = _candidate( "agent-0", round_num=0, - insight_reward={"uses_required_tool": 0.0}, + insight_train_reward={"uses_required_tool": 0.5}, + insight_validation_reward={"uses_required_tool": 0.0}, ) winner = _candidate( "agent-1", round_num=1, - insight_reward={"uses_required_tool": 1.0}, + insight_train_reward={"uses_required_tool": 0.5}, + insight_validation_reward={"uses_required_tool": 1.0}, validation_reward={"reward": 0.75}, ) optimizer = object.__new__(EvolutionaryOptimizer) @@ -114,8 +137,10 @@ def test_terminal_summary_includes_baseline_and_winner_insight_metrics() -> None summary = optimizer._render_summary(rounds_completed=1, baseline=baseline, winner=winner) assert "validation_reward={'reward': 0.75}" in summary - assert "insight_suite=(baseline={'uses_required_tool': 0.0}" in summary + assert "insight_validation=(baseline={'uses_required_tool': 0.0}" in summary assert "winner={'uses_required_tool': 1.0})" in summary + # The train half steers development only, so it never appears as an outcome number. + assert "0.5" not in summary def test_terminal_summary_omits_insight_comparison_when_unavailable() -> None: @@ -125,7 +150,7 @@ def test_terminal_summary_omits_insight_comparison_when_unavailable() -> None: summary = optimizer._render_summary(rounds_completed=1, baseline=baseline, winner=winner) - assert "insight_suite" not in summary + assert "insight" not in summary def _insight_trial( @@ -158,7 +183,7 @@ def test_insight_promotion_suggestions_are_stable_discriminative_and_diverse( Task(id="task-flat", uri=(tmp_path / "task-flat").as_uri()), ] baseline = _candidate("agent-0", round_num=0) - baseline.insight_reward_details = [ + baseline.insight_train_reward_details = [ _insight_trial("task-a", 0.0, attempt=1), _insight_trial("task-a", 0.0, attempt=2), _insight_trial("task-b", 0.0, attempt=1), @@ -171,8 +196,8 @@ def test_insight_promotion_suggestions_are_stable_discriminative_and_diverse( _insight_trial("task-flat", 0.5, attempt=2), ] winner = _candidate("agent-1", round_num=1) - winner.insight_metric_keys = ["uses_required_tool", "reward"] - winner.insight_reward_details = [ + winner.insight_train_metric_keys = ["uses_required_tool", "reward"] + winner.insight_train_reward_details = [ _insight_trial("task-a", 1.0, attempt=1), _insight_trial("task-a", 1.0, attempt=2), _insight_trial("task-b", 1.0, attempt=1), @@ -188,6 +213,7 @@ def test_insight_promotion_suggestions_are_stable_discriminative_and_diverse( suggestions = select_insight_promotion_suggestions( _insight_dataset(tasks), [baseline, winner], + fields=INSIGHT_TRAIN_FIELDS, winner=winner, ) @@ -197,7 +223,7 @@ def test_insight_promotion_suggestions_are_stable_discriminative_and_diverse( assert suggestions[0].diversity_score is None assert suggestions[1].diversity_score == pytest.approx(0.45) - section = render_insight_promotion_section(suggestions) + section = render_insight_promotion_section(suggestions, INSIGHT_TRAIN_FIELDS.split) assert "Advisory adaptive/development evidence only" in section assert "`task-a`" in section assert str(tmp_path / "task-a") in section @@ -215,9 +241,9 @@ def test_task_evidence_excludes_candidates_from_other_suites(tmp_path: Path) -> baseline = _candidate("agent-0", round_num=0) winner = _candidate("agent-1", round_num=1) stale = _candidate("agent-stale", round_num=1) - stale.insight_suite_identity = f"sha256:{'e' * 64}" + stale.insight_train_suite_identity = f"sha256:{'e' * 64}" for candidate, score in ((baseline, 0.0), (winner, 1.0), (stale, 0.5)): - candidate.insight_reward_details = [ + candidate.insight_train_reward_details = [ _insight_trial(task.id, score, attempt=1), _insight_trial(task.id, score, attempt=2), ] @@ -228,6 +254,7 @@ def test_task_evidence_excludes_candidates_from_other_suites(tmp_path: Path) -> baseline=baseline, winner=winner, provenance=insight_suite_provenance(dataset), + fields=INSIGHT_TRAIN_FIELDS, ) assert evidence is not None @@ -240,9 +267,9 @@ def test_task_evidence_excludes_candidates_from_other_suites(tmp_path: Path) -> def test_insight_promotion_section_explains_when_no_task_qualifies() -> None: - section = render_insight_promotion_section([]) + section = render_insight_promotion_section([], INSIGHT_TRAIN_FIELDS.split) - assert "## Insight Suite Promotion Suggestions" in section + assert "## Insight Suite Promotion Suggestions (insight-train)" in section assert "No task had complete repeated evidence" in section @@ -253,13 +280,24 @@ def test_insight_promotion_section_is_appended_without_rewriting_report( report_path.parent.mkdir(parents=True) report_path.write_text("# Optimization\n\nExisting analysis.\n") - write_insight_promotion_section(report_path, []) + write_insight_promotion_section(report_path, [], INSIGHT_TRAIN_FIELDS.split) first_report = report_path.read_text() - write_insight_promotion_section(report_path, []) + write_insight_promotion_section(report_path, [], INSIGHT_TRAIN_FIELDS.split) assert report_path.read_text() == first_report assert first_report.startswith("# Optimization\n\nExisting analysis.") - assert first_report.count("## Insight Suite Promotion Suggestions") == 1 + assert first_report.count("## Insight Suite Promotion Suggestions (insight-train)") == 1 + + +def test_each_insight_half_gets_its_own_promotion_section(tmp_path: Path) -> None: + report_path = tmp_path / "OPTIMIZATION.md" + + write_insight_promotion_section(report_path, [], INSIGHT_TRAIN_FIELDS.split) + write_insight_promotion_section(report_path, [], INSIGHT_VALIDATION_FIELDS.split) + report = report_path.read_text() + + assert "## Insight Suite Promotion Suggestions (insight-train)" in report + assert "## Insight Suite Promotion Suggestions (insight-validation)" in report @pytest.mark.parametrize("score", [1.1, -0.1, math.inf, -math.inf, math.nan]) @@ -323,14 +361,14 @@ def test_invalid_runtime_metrics_cannot_be_promotion_evidence( task = Task(id="task-a") baseline = _candidate("agent-0", round_num=0) winner = _candidate("agent-1", round_num=1) - baseline.insight_reward_details = [ + baseline.insight_train_reward_details = [ _insight_trial("task-a", 0.0, attempt=1), _insight_trial("task-a", 0.0, attempt=2), ] invalid_trial = _insight_trial("task-a", invalid_score, attempt=1) if missing_key: invalid_trial.metrics.pop("uses_required_tool") - winner.insight_reward_details = [ + winner.insight_train_reward_details = [ invalid_trial, _insight_trial("task-a", 1.0, attempt=2), ] @@ -339,6 +377,7 @@ def test_invalid_runtime_metrics_cannot_be_promotion_evidence( select_insight_promotion_suggestions( _insight_dataset([task]), [baseline, winner], + fields=INSIGHT_TRAIN_FIELDS, winner=winner, ) == [] @@ -349,34 +388,37 @@ def test_one_attempt_failed_and_incomplete_evidence_do_not_qualify_as_stable() - task = Task(id="task-a") baseline = _candidate("agent-0", round_num=0) winner = _candidate("agent-1", round_num=1) - baseline.insight_reward_details = [_insight_trial("task-a", 0.0)] - winner.insight_reward_details = [_insight_trial("task-a", 1.0)] + baseline.insight_train_reward_details = [_insight_trial("task-a", 0.0)] + winner.insight_train_reward_details = [_insight_trial("task-a", 1.0)] assert ( select_insight_promotion_suggestions( _insight_dataset([task]), [baseline, winner], + fields=INSIGHT_TRAIN_FIELDS, winner=winner, ) == [] ) - baseline.insight_reward_details.append(_insight_trial("task-a", 0.0, attempt=2)) - winner.insight_reward_details.append(_insight_trial("task-a", 1.0, attempt=2, status="failed")) + baseline.insight_train_reward_details.append(_insight_trial("task-a", 0.0, attempt=2)) + winner.insight_train_reward_details.append(_insight_trial("task-a", 1.0, attempt=2, status="failed")) assert ( select_insight_promotion_suggestions( _insight_dataset([task]), [baseline, winner], + fields=INSIGHT_TRAIN_FIELDS, winner=winner, ) == [] ) - winner.insight_reward_details = [] + winner.insight_train_reward_details = [] assert ( select_insight_promotion_suggestions( _insight_dataset([task]), [baseline, winner], + fields=INSIGHT_TRAIN_FIELDS, winner=winner, ) == [] @@ -400,17 +442,17 @@ def test_promotion_requires_baseline_to_winner_improvement( baseline = _candidate("agent-0", round_num=0) winner = _candidate("agent-1", round_num=1) candidates = [baseline, winner] - baseline.insight_reward_details = [ + baseline.insight_train_reward_details = [ _insight_trial("task-a", baseline_score, attempt=1), _insight_trial("task-a", baseline_score, attempt=2), ] - winner.insight_reward_details = [ + winner.insight_train_reward_details = [ _insight_trial("task-a", winner_score, attempt=1), _insight_trial("task-a", winner_score, attempt=2), ] if bad_score is not None: bad = _candidate("agent-bad", round_num=1) - bad.insight_reward_details = [ + bad.insight_train_reward_details = [ _insight_trial("task-a", bad_score, attempt=1), _insight_trial("task-a", bad_score, attempt=2), ] @@ -420,6 +462,7 @@ def test_promotion_requires_baseline_to_winner_improvement( select_insight_promotion_suggestions( _insight_dataset([task]), candidates, + fields=INSIGHT_TRAIN_FIELDS, winner=winner, ) == [] @@ -432,25 +475,55 @@ def test_deterministic_insight_comparison_section_uses_local_suite_identity( baseline = _candidate( "agent-0", round_num=0, - insight_reward={"reward": 0.5, "uses_required_tool": 0.0}, + insight_train_reward={"reward": 0.5, "uses_required_tool": 0.0}, ) winner = _candidate( "agent-1", round_num=1, - insight_reward={"reward": 0.75, "uses_required_tool": 1.0}, + insight_train_reward={"reward": 0.75, "uses_required_tool": 1.0}, ) report_path = tmp_path / "OPTIMIZATION.md" provenance = insight_suite_provenance(_insight_dataset([Task(id="task-a")])) - write_insight_comparison_section(report_path, baseline, winner, provenance) + write_insight_comparison_section(report_path, baseline, winner, provenance, INSIGHT_TRAIN_FIELDS) report = report_path.read_text() - assert "## Deterministic Insight Suite Comparison" in report + assert "## Deterministic Insight Suite Comparison (insight-train)" in report assert str(_SUITE_PATH) in report assert _SUITE_IDENTITY in report assert "| `uses_required_tool` | 0.000 | 1.000 | +1.000 |" in report +def test_comparison_section_labels_each_half_by_its_evidentiary_role(tmp_path: Path) -> None: + baseline = _candidate( + "agent-0", + round_num=0, + insight_train_reward={"reward": 0.5, "uses_required_tool": 0.0}, + insight_validation_reward={"reward": 0.5, "uses_required_tool": 0.0}, + ) + winner = _candidate( + "agent-1", + round_num=1, + insight_train_reward={"reward": 0.75, "uses_required_tool": 1.0}, + insight_validation_reward={"reward": 0.75, "uses_required_tool": 1.0}, + ) + report_path = tmp_path / "OPTIMIZATION.md" + provenance = insight_suite_provenance(_insight_dataset([Task(id="task-a")])) + + for fields in (INSIGHT_TRAIN_FIELDS, INSIGHT_VALIDATION_FIELDS): + write_insight_comparison_section(report_path, baseline, winner, provenance, fields) + report = report_path.read_text() + + train_section = report.split("## Deterministic Insight Suite Comparison (insight-train)")[1] + train_section = train_section.split("## Deterministic Insight Suite Comparison (insight-validation)")[0] + validation_section = report.split("## Deterministic Insight Suite Comparison (insight-validation)")[1] + + assert "Adaptive/development evidence only" in train_section + assert "does not affect Pareto or winner selection" in train_section + assert "Held out from optimization" in validation_section + assert "participate in Pareto and winner selection" in validation_section + + @pytest.mark.asyncio async def test_final_report_failure_preserves_compact_summary_and_deterministic_sections( tmp_path: Path, @@ -459,13 +532,13 @@ async def test_final_report_failure_preserves_compact_summary_and_deterministic_ baseline = _candidate( "agent-0", round_num=0, - insight_reward={"reward": 0.5, "uses_required_tool": 0.0}, + insight_train_reward={"reward": 0.5, "uses_required_tool": 0.0}, validation_reward={"reward": 0.5}, ) winner = _candidate( "agent-1", round_num=1, - insight_reward={"reward": 0.75, "uses_required_tool": 1.0}, + insight_train_reward={"reward": 0.75, "uses_required_tool": 1.0}, validation_reward={"reward": 0.75}, ) tree = EvolutionTree() @@ -492,7 +565,7 @@ async def test_final_report_failure_preserves_compact_summary_and_deterministic_ run_entity=run, evolution_tree=tree, agent_name="agent", - insight_dataset=_insight_dataset([Task(id="task-a")]), + insight_halves=[(INSIGHT_TRAIN_FIELDS, _insight_dataset([Task(id="task-a")]))], ) finally: type.__setattr__( @@ -518,16 +591,16 @@ async def test_insight_report_mismatch_does_not_fail_completed_run( baseline = _candidate( "agent-0", round_num=0, - insight_reward={"reward": 0.5, "uses_required_tool": 0.0}, + insight_train_reward={"reward": 0.5, "uses_required_tool": 0.0}, validation_reward={"reward": 0.5}, ) winner = _candidate( "agent-1", round_num=1, - insight_reward={"reward": 0.75, "uses_required_tool": 1.0}, + insight_train_reward={"reward": 0.75, "uses_required_tool": 1.0}, validation_reward={"reward": 0.75}, ) - winner.insight_suite_identity = f"sha256:{'e' * 64}" + winner.insight_train_suite_identity = f"sha256:{'e' * 64}" tree = EvolutionTree() tree.add(baseline) tree.add(winner) @@ -549,7 +622,7 @@ async def test_insight_report_mismatch_does_not_fail_completed_run( run_entity=run, evolution_tree=tree, agent_name="agent", - insight_dataset=_insight_dataset([Task(id="task-a")]), + insight_halves=[(INSIGHT_TRAIN_FIELDS, _insight_dataset([Task(id="task-a")]))], ) finally: type.__setattr__( @@ -562,4 +635,4 @@ async def test_insight_report_mismatch_does_not_fail_completed_run( assert run.status == "completed" assert run.winner_agent == winner.label backend.update_run.assert_awaited_once() - assert "Skipping Insight Suite report sections" in caplog.text + assert "Skipping insight-train Insight report sections" in caplog.text diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_selection_rewards.py b/plugins/nemo-experimentalist/tests/experimentalist/test_selection_rewards.py new file mode 100644 index 0000000000..f425ece2f4 --- /dev/null +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_selection_rewards.py @@ -0,0 +1,146 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from nemo_experimentalist_plugin.entities import Candidate +from nemo_experimentalist_plugin.experimentalist.components.models import ( + INSIGHT_REWARD_PREFIX, + pareto_front, + selection_rewards, +) + + +def _candidate( + label: str, + *, + validation_reward: dict[str, float] | None = None, + insight_validation_reward: dict[str, float] | None = None, + insight_train_reward: dict[str, float] | None = None, +) -> Candidate: + return Candidate( + run_id="run-1", + label=label, + round=1, + optimization="baseline", + validation_reward=validation_reward, + insight_validation_reward=insight_validation_reward, + insight_train_reward=insight_train_reward, + ) + + +def _front(candidates: list[Candidate]) -> set[str]: + rewards = selection_rewards(candidates) + return {candidate.label for candidate in pareto_front(candidates, lambda c: rewards[c.label])} + + +def test_insight_validation_dimensions_are_namespaced_beside_validation_reward() -> None: + candidate = _candidate( + "agent-0", + validation_reward={"reward": 0.5, "uses_required_tool": 0.4}, + insight_validation_reward={"uses_required_tool": 0.9}, + ) + + assert selection_rewards([candidate])["agent-0"] == { + "reward": 0.5, + "uses_required_tool": 0.4, + f"{INSIGHT_REWARD_PREFIX}uses_required_tool": 0.9, + } + + +def test_the_train_half_never_reaches_selection() -> None: + candidate = _candidate( + "agent-0", + validation_reward={"reward": 0.5}, + insight_train_reward={"uses_required_tool": 1.0}, + ) + + assert selection_rewards([candidate])["agent-0"] == {"reward": 0.5} + + +def test_missing_insight_scores_are_zero_filled_across_the_ranked_set() -> None: + scored = _candidate( + "agent-scored", + validation_reward={"reward": 0.5}, + insight_validation_reward={"uses_required_tool": 0.8}, + ) + unscored = _candidate("agent-unscored", validation_reward={"reward": 0.5}) + + rewards = selection_rewards([scored, unscored]) + + assert rewards["agent-unscored"] == {"reward": 0.5, f"{INSIGHT_REWARD_PREFIX}uses_required_tool": 0.0} + # Without the zero-fill the two key sets would differ and _dominates would call them + # incomparable, leaving the strictly worse candidate on the front. + assert _front([scored, unscored]) == {"agent-scored"} + + +def test_unscored_candidates_stay_incomparable_rather_than_dominated() -> None: + scored = _candidate( + "agent-scored", + validation_reward={"reward": 0.9}, + insight_validation_reward={"uses_required_tool": 0.9}, + ) + pending = _candidate("agent-pending", insight_validation_reward={"uses_required_tool": 0.1}) + + assert selection_rewards([scored, pending])["agent-pending"] == {} + assert _front([scored, pending]) == {"agent-scored", "agent-pending"} + + +def test_a_candidate_strong_only_on_insight_dimensions_survives() -> None: + generalist = _candidate( + "agent-generalist", + validation_reward={"reward": 0.9}, + insight_validation_reward={"uses_required_tool": 0.1}, + ) + specialist = _candidate( + "agent-specialist", + validation_reward={"reward": 0.4}, + insight_validation_reward={"uses_required_tool": 1.0}, + ) + + assert _front([generalist, specialist]) == {"agent-generalist", "agent-specialist"} + + +def test_a_candidate_worse_on_every_merged_dimension_is_dominated() -> None: + better = _candidate( + "agent-better", + validation_reward={"reward": 0.9}, + insight_validation_reward={"uses_required_tool": 0.9}, + ) + worse = _candidate( + "agent-worse", + validation_reward={"reward": 0.4}, + insight_validation_reward={"uses_required_tool": 0.1}, + ) + + assert _front([better, worse]) == {"agent-better"} + + +def test_an_insight_only_regression_is_enough_to_be_dominated() -> None: + """Insight validation is a real selection axis, not a tie-break on validation reward.""" + better = _candidate( + "agent-better", + validation_reward={"reward": 0.5}, + insight_validation_reward={"uses_required_tool": 0.9}, + ) + worse = _candidate( + "agent-worse", + validation_reward={"reward": 0.5}, + insight_validation_reward={"uses_required_tool": 0.2}, + ) + + assert _front([better, worse]) == {"agent-better"} + + +def test_a_same_named_metric_on_both_splits_stays_on_two_axes() -> None: + """The prefix keeps an Insight score from overwriting the validation score it shares a name with.""" + validation_specialist = _candidate( + "agent-validation", + validation_reward={"uses_required_tool": 0.9}, + insight_validation_reward={"uses_required_tool": 0.1}, + ) + insight_specialist = _candidate( + "agent-insight", + validation_reward={"uses_required_tool": 0.1}, + insight_validation_reward={"uses_required_tool": 0.9}, + ) + + assert _front([validation_specialist, insight_specialist]) == {"agent-validation", "agent-insight"} diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_terminator.py b/plugins/nemo-experimentalist/tests/experimentalist/test_terminator.py index 2fe0289c9c..f66d0deeb1 100644 --- a/plugins/nemo-experimentalist/tests/experimentalist/test_terminator.py +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_terminator.py @@ -29,6 +29,7 @@ class _FakeNode: label: str round: int val_reward: dict[str, float] = field(default_factory=dict) + insight_val_reward: dict[str, float] = field(default_factory=dict) def _tree(*nodes: _FakeNode) -> SimpleNamespace: @@ -219,6 +220,27 @@ async def test_has_converged_true_when_front_stagnates() -> None: ) +async def test_gains_confined_to_the_held_out_insight_half_keep_the_loop_running() -> None: + # a2 gives up a little validation reward but is the only candidate scoring on the + # held-out Insight half. On validation alone it is dominated and the front looks + # stagnant, so convergence would end the run while the agent is still getting + # better at the production failures the Insight suite reconstructs. + term = _terminator(stop_verdict=False) + tree = _tree( + _FakeNode("a0", 0, {"score": 0.7}), + _FakeNode("a1", 1, {"score": 0.7}), + _FakeNode("a2", 2, {"score": 0.6}, {"uses_required_tool": 0.9}), + ) + assert ( + await term._has_converged( + evolution_tree=tree, + prior_analysis="ignored", + min_rounds_before_stopping=2, + ) + is False + ) + + # --------------------------------------------------------------------------- # Qualitative fallback (deterministic check inconclusive -> LLM decides) # --------------------------------------------------------------------------- diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_tools.py b/plugins/nemo-experimentalist/tests/experimentalist/test_tools.py index 7a0ea7af33..9cd1bd8696 100644 --- a/plugins/nemo-experimentalist/tests/experimentalist/test_tools.py +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_tools.py @@ -3,8 +3,12 @@ from pathlib import Path +import pytest from nemo_experimentalist_plugin.experimentalist.components.coder import Coder -from nemo_experimentalist_plugin.experimentalist.components.holdout_utils import BLOCKED_MESSAGE +from nemo_experimentalist_plugin.experimentalist.components.holdout_utils import ( + BLOCKED_MESSAGE, + HELD_OUT_STORAGE_DIR, +) from nemo_experimentalist_plugin.experimentalist.components.tools import GuardedShellTools from nooa.agentdoc import pformat from nooa.tools import ShellResult @@ -22,10 +26,18 @@ async def test_guarded_shell_tools_runs_allowed_commands(tmp_path): assert result.success -async def test_guarded_shell_tools_returns_failure_for_blocked_paths(tmp_path): +@pytest.mark.parametrize( + "command", + [ + "cat dataset/validation/secret", + "cat dataset/insight-validation/000-task/solution.sh", + f"ls {HELD_OUT_STORAGE_DIR}", + ], +) +async def test_guarded_shell_tools_returns_failure_for_blocked_paths(tmp_path, command: str): shell = GuardedShellTools(cwd=tmp_path) try: - result = await shell.run("cat dataset/validation/secret") + result = await shell.run(command) finally: await shell.close() @@ -36,6 +48,20 @@ async def test_guarded_shell_tools_returns_failure_for_blocked_paths(tmp_path): assert not result.success +async def test_guarded_shell_tools_allows_the_visible_insight_train_half(tmp_path): + task_dir = tmp_path / "dataset" / "insight-train" / "000-task" + task_dir.mkdir(parents=True) + (task_dir / "task.md").write_text("visible\n") + shell = GuardedShellTools(cwd=tmp_path) + try: + result = await shell.run("cat dataset/insight-train/000-task/task.md") + finally: + await shell.close() + + assert result.stdout.strip() == "visible" + assert result.success + + def test_coder_hides_skill_registry_that_can_replace_guarded_shell(tmp_path): nooa_skill = Path(__file__).resolve().parents[2] / "framework-skills" / "nooa" coder = Coder(workspace=tmp_path, framework_skills_dirs=[nooa_skill]) diff --git a/plugins/nemo-experimentalist/tests/test_experiment_mirror.py b/plugins/nemo-experimentalist/tests/test_experiment_mirror.py index f3af6f1fa5..6292903c5b 100644 --- a/plugins/nemo-experimentalist/tests/test_experiment_mirror.py +++ b/plugins/nemo-experimentalist/tests/test_experiment_mirror.py @@ -117,18 +117,21 @@ async def test_project_candidate_skips_when_no_reward(): experiments.create.assert_not_awaited() -async def test_project_candidate_creates_insight_experiment_when_evaluated(): +@pytest.mark.parametrize("split", ["insight-train", "insight-validation"]) +async def test_project_candidate_creates_an_experiment_per_evaluated_insight_half(split: str) -> None: experiments = AsyncMock() - experiments.create.return_value = SimpleNamespace(id="exp-insight") + experiments.create.return_value = SimpleNamespace(id=f"exp-{split}") mirror = ExperimentMirror(_client(AsyncMock(), experiments), workspace="default") - candidate = _cand(insight_reward={"uses_required_tool": 0.5}, insight_reward_details=[]) + prefix = split.replace("-", "_") + candidate = _cand(**{f"{prefix}_reward": {"uses_required_tool": 0.5}, f"{prefix}_reward_details": []}) await mirror.project_candidate(candidate) kwargs = experiments.create.await_args.kwargs - assert kwargs["name"] == "opt-run-1-agent-0-insight" - assert kwargs["dataset_name"] == "insight" - assert kwargs["metadata"] == {"round": "0", "candidate_id": "agent-0", "split": "insight"} + assert kwargs["name"] == f"opt-run-1-agent-0-{split}" + assert kwargs["dataset_name"] == split + assert kwargs["metadata"] == {"round": "0", "candidate_id": "agent-0", "split": split} + assert experiments.create.await_count == 1 async def test_project_candidate_conflict_updates_experiment(): diff --git a/plugins/nemo-experimentalist/tests/test_experiment_mirror_mapping.py b/plugins/nemo-experimentalist/tests/test_experiment_mirror_mapping.py index 7add67fe5e..564ff0e9be 100644 --- a/plugins/nemo-experimentalist/tests/test_experiment_mirror_mapping.py +++ b/plugins/nemo-experimentalist/tests/test_experiment_mirror_mapping.py @@ -1,11 +1,13 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +from typing import Any + from nemo_experimentalist_plugin.entities import Candidate, ExperimentRun from nemo_experimentalist_plugin.experimentalist import experiment_mirror as m -def _cand(**kw): - base = dict(run_id="run-1", label="agent-0", round=0, optimization="baseline") +def _cand(**kw: Any) -> Candidate: + base: dict[str, Any] = dict(run_id="run-1", label="agent-0", round=0, optimization="baseline") base.update(kw) return Candidate(**base) diff --git a/plugins/nemo-experimentalist/tests/test_experimentalist_analyzer.py b/plugins/nemo-experimentalist/tests/test_experimentalist_analyzer.py index 48ab2c4f44..6738f2de31 100644 --- a/plugins/nemo-experimentalist/tests/test_experimentalist_analyzer.py +++ b/plugins/nemo-experimentalist/tests/test_experimentalist_analyzer.py @@ -107,9 +107,18 @@ async def run(self, task: Any, agent_spec: Any = None) -> Rationale: class _SelectTrials: def __init__(self, trials: list[Any]) -> None: self._trials = trials + self.calls: list[tuple[Any, Any]] = [] - async def __call__(self, agent_id: str, dataset: Any, evaluation: Any) -> list[Any]: - return self._trials + async def __call__( + self, + agent_id: str, + dataset: Any, + evaluation: Any, + insight_dataset: Any = None, + insight_trials: Any = None, + ) -> list[Any]: + self.calls.append((insight_dataset, insight_trials)) + return self._trials + list(insight_trials or ()) class _ClassifyFailures: @@ -192,6 +201,71 @@ async def test_run_defaults_client_and_workspace_to_none(tmp_path: Path, monkeyp assert calls[0]["workspace"] is None +@pytest.mark.asyncio +async def test_insight_train_trials_reach_trial_selection_and_get_diagnosed( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Insight failures only get trace-level diagnosis if both the trials and their tasks arrive.""" + calls: list[dict[str, Any]] = [] + _install_fakes(monkeypatch, calls) + trial, dataset, evaluation = _fixtures() + insight_trial = _FakeTrial( + id="insight-trial-1", + task_id="insight-task-1", + trace=object(), + metrics={"uses_required_tool": _FakeMetric(0.0)}, + ) + insight_dataset = _FakeDataset(tasks=[_FakeTask(id="insight-task-1")]) + analyzer = _make_analyzer(tmp_path, [trial]) + + result = await analyzer.run( + agent="agent-a", + dataset=cast(Any, dataset), + evaluation=cast(Any, evaluation), + round=0, + insight_dataset=cast(Any, insight_dataset), + insight_trials=cast(Any, [insight_trial]), + ) + + select_trials = cast(_SelectTrials, analyzer.select_trials) + assert select_trials.calls == [(insight_dataset, [insight_trial])] + # Both trials are diagnosed: the Insight task resolved, so it was not dropped as unknown. + assert len(calls) == 2 + analyses = {analysis.trial_id: analysis for analysis in result.trial_analyses} + assert set(analyses) == {"trial-1", "insight-trial-1"} + assert analyses["insight-trial-1"].task_id == "insight-task-1" + assert analyses["insight-trial-1"].diagnostic.root_cause != "evaluation_result_references_unknown_task" + + +@pytest.mark.asyncio +async def test_insight_trials_are_part_of_the_cache_key(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A run analyzed before the Insight suite existed must not be replayed once it does.""" + calls: list[dict[str, Any]] = [] + _install_fakes(monkeypatch, calls) + trial, dataset, evaluation = _fixtures() + insight_trial = _FakeTrial( + id="insight-trial-1", + task_id="insight-task-1", + trace=object(), + metrics={"uses_required_tool": _FakeMetric(0.0)}, + ) + insight_dataset = _FakeDataset(tasks=[_FakeTask(id="insight-task-1")]) + + await _make_analyzer(tmp_path, [trial]).run( + agent="agent-a", dataset=cast(Any, dataset), evaluation=cast(Any, evaluation), round=0 + ) + await _make_analyzer(tmp_path, [trial]).run( + agent="agent-a", + dataset=cast(Any, dataset), + evaluation=cast(Any, evaluation), + round=0, + insight_dataset=cast(Any, insight_dataset), + insight_trials=cast(Any, [insight_trial]), + ) + + assert len(calls) == 3 + + @pytest.mark.asyncio async def test_intake_availability_is_part_of_cache_key(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: """A trace-skipped (no client) result must not be replayed once a client is available."""