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/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_cli.py b/plugins/nemo-eval-author/tests/test_cli.py index 7f15ddff5a..ae6e71218b 100644 --- a/plugins/nemo-eval-author/tests/test_cli.py +++ b/plugins/nemo-eval-author/tests/test_cli.py @@ -1,16 +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 twice — canonically under ``nemo.cli.agents`` and, for backward -compatibility, under ``nemo.cli`` — so both groups are asserted. -""" - -from importlib.metadata import EntryPoint, entry_points +"""Scaffolding tests: the command tree exists, and every verb still refuses to run.""" import pytest import typer @@ -34,12 +25,6 @@ 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" - return matches[0] - - def test_help_lists_every_verb(app: typer.Typer) -> None: result = runner.invoke(app, ["--help"]) @@ -56,42 +41,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: - """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 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_not_implemented_quotes_the_invoked_command_path() -> None: + """Placeholder messages use ``ctx.command_path``, not a hardcoded CLI string.""" + app = typer.Typer() -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 + @app.callback() + def _root() -> None: + """Force subcommand dispatch.""" + @app.command("probe") + def probe(ctx: typer.Context) -> None: + cli._not_implemented(ctx, "ASE-000") -@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") - - 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") + result = runner.invoke(app, ["probe"], prog_name="nemo") assert result.exit_code == 1, result.output - assert f"`nemo eval-author {command}` is not implemented yet ({ticket})." in result.output + assert "`nemo probe` is not implemented yet (ASE-000)." in result.output 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/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-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.""" 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' +