From 228829d7739f6e1359562e783d208a912689a34b Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Wed, 19 Aug 2026 16:39:40 -0500 Subject: [PATCH 1/8] feat(eval-author): add Eval Author skills for Harbor eval discovery Customers are wary of deploying an agent into their codebase to act on their code, so package Eval Author's discovery pass as a skill their own agent can run instead. Two skills, following a core-plus-sub-flow shape: - eval-author: the standard that governs every sub-flow, which is that a provider's own validators judge each recorded fact rather than the agent inferring it from file layout. Also owns the shared vocabulary, the boundaries, and the routing. - eval-author-discover: the discovery sub-flow. Probes for Harbor, inventories the repository with the standard library, then runs Harbor's full validation ladder in-process when Harbor is importable, and reports an unproven inventory when it is not. The skill ships no dependency of its own. Harbor is its only import beyond the standard library, and a repository holding Harbor evaluations has Harbor by construction; PyYAML, pydantic, and toml arrive with it. Provider code sits under scripts/providers/harbor/ rather than scripts/harbor/: a directory named harbor on sys.path satisfies find_spec("harbor") on a machine without Harbor, which would make the probe claim an install that is not there. Signed-off-by: Alec Khoury --- .../skills/eval-author-discover/SKILL.md | 164 +++++++ .../eval-author-discover/scripts/_checks.py | 101 ++++ .../eval-author-discover/scripts/discover.py | 246 ++++++++++ .../scripts/providers/harbor/_inventory.py | 290 ++++++++++++ .../scripts/providers/harbor/_ladder.py | 358 ++++++++++++++ .../scripts/providers/harbor/_probe.py | 91 ++++ .../skills/eval-author/SKILL.md | 114 +++++ .../tests/test_skill_contract.py | 447 ++++++++++++++++++ 8 files changed, 1811 insertions(+) create mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md create mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/_checks.py create mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py create mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_inventory.py create mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_ladder.py create mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_probe.py create mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md create mode 100644 plugins/nemo-eval-author/tests/test_skill_contract.py diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md new file mode 100644 index 0000000000..e047fe124d --- /dev/null +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md @@ -0,0 +1,164 @@ +--- +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +name: eval-author-discover +description: >- + Record whether a repository's Harbor evaluations are ready to run, and prove it + with Harbor's own validators instead of guessing. Finds every repository-owned + job config, dataset, and task directory, then makes Harbor judge each config: + schema, job resolution, agent, environment backend, per-task validity, tasks + Harbor silently dropped, and required host variables. Use when the user wants + to run an eval suite they did not write, hand a suite to a cheaper model, or + asks "can I run these evals?", "why won't my Harbor config resolve?", "which + env vars does this suite need?", "where are the evals in this repo?", or "why + did Harbor skip my task?". Reads the repository and writes nothing to it. +triggers: + - can I run the evals in this repo + - where are the Harbor evals in this repository + - why won't my Harbor job config resolve + - which environment variables does this eval suite need + - why did Harbor skip one of my tasks + - check whether this eval suite is ready to run +not-for: + - eval-author (use for the standard, the boundaries, and to pick a sub-flow) + - nemo-experimentalist (use to run insight-driven optimization end to end, which drives the Eval Author agent itself) + - nemo-evaluator (use to run an existing benchmark rather than establish that a Harbor suite is runnable) +compatibility: >- + Python 3.11 or later. Harbor must be importable by the interpreter that runs the + script for any finding to be proven; without it the script reports an unproven + inventory and exits 1. Docker is needed only for the environment backend check. +maturity: alpha +license: Apache-2.0 +user-invocable: true +allowed-tools: [Bash, Read, Grep, Glob] +--- + +# Eval Author: discover + +The Eval Author discovery pass. Read `eval-author` for the standard this follows, +the shared vocabulary, and the boundaries that apply throughout. In short: Harbor +judges every fact recorded here, and anything Harbor did not judge is marked +unproven and is not evidence. + +Three phases, in order. The bundled script runs all three in one invocation. + +1. **Probe.** Is Harbor importable by this interpreter? +2. **Inventory.** Which config files, datasets, and task directories does the + repository own? +3. **Judge.** Only when Harbor is importable: run Harbor's full validation ladder. + +Without Harbor, phase 3 cannot run and no claim about runnability is possible. The +report still comes back, every finding marked unproven, with a required failure +naming what to install. + +## Before you start + +Run the script with the interpreter that has Harbor installed. This is the one step +people get wrong, and getting it wrong voids the whole report. + +A `harbor` command on your `PATH` does not mean Harbor is importable by the Python +you are about to run. A repository with its own virtual environment usually needs +that environment's interpreter. Try these in order until one prints a version: + +```bash +for py in .venv/bin/python ./venv/bin/python python3; do + "$py" -c "import harbor, sys; print(sys.executable, harbor.__version__)" 2>/dev/null && break +done +``` + +Nothing prints a version when Harbor is not installed anywhere. Do not install it +yourself; in the user's repository the missing environment is the finding. Tell +them what you found and ask how they want to proceed. + +The report records which mode produced it either way, in `runtime.harbor_importable` +and the top-level `proven` field. + +## Step 1: run discovery + +Point the script at the repository root, not at a suite directory. It searches for +configs to a depth of four directories and finds datasets at any depth. + +```bash +.venv/bin/python /scripts/discover.py --repo . +``` + +One JSON object goes to stdout. Add `--out discovery.md` to also write a Markdown +report, and `--compact` for single-line JSON. + +The exit code carries the verdict, so check it: + +- `0` — every repository-owned config passed every required check +- `1` — a required check failed, Harbor was unavailable, or the path was unusable + +**Only run this against a repository you trust.** Validating a config that names an +agent `import_path` imports that module, which executes its top-level code. + +## Step 2: read the verdict + +Read these four fields before any others. + +| Field | What it settles | +|---|---| +| `proven` | Whether Harbor judged this report. When `false`, nothing below is evidence | +| `runnable` | Whether every config passed every required check | +| `run_command` | The exact command to run the suite. Present only when the repository has exactly one config and it is runnable | +| `configs[].runnable` | The per-config verdict, when the repository owns several | + +`run_command` is deliberately absent when several configs exist. Picking one for the +user guesses at intent, so ask which suite they mean and build the command from that +config's `path`. + +## Step 3: fix what failed + +Each check names one rung of Harbor's ladder. Work top to bottom, because a lower +rung's failure often disappears once you fix a higher one. + +| Check | What it means and what to do | +|---|---| +| `harbor` | Harbor is not importable by this interpreter. Re-run with the interpreter from **Before you start** | +| `config` | No config file declares a nonempty `datasets` or `tasks` list. Confirm with the user where their suite lives | +| `config-parse` | The file could not be read, because PyYAML is missing. Harbor ships PyYAML, so this means the wrong interpreter | +| `schema` | Harbor rejected the config's shape. The message carries the offending field path | +| `resolution` | Harbor could not turn the config into a job. Usually a `datasets[].path` that does not exist. This fails before any container starts | +| `tasks` | Some resolved directories are not valid Harbor tasks. A task directory needs a parseable `task.toml` and an `environment/` directory, even when the image is prebuilt | +| `coverage` | Harbor silently dropped task directories that exist on disk. Harbor skips unparseable tasks without raising, so treat this as a real defect, not noise | +| `credentials` | Reports the host variables the suite needs. Confirm each one is set before running; a missing key surfaces as a failed trial, not a clear error | +| `agent` | The named built-in agent does not exist, or the `import_path` does not import. Check the message for which | +| `backend` | The environment backend failed preflight. For `docker`, confirm the daemon is running with `docker info` | +| `round-trip` | The Harbor CLI rejected the config file's bytes. This is the weakest rung: it round-trips the schema only, so it can pass while `resolution` fails | +| `ethos` | Advisory. `ETHOS.md` is absent, so no agent doctrine is defined for this repository | +| `tasks-on-disk` | Advisory, and always unproven. A count of directories holding a `task.toml` | + +## Step 4: verify before you report + +Discovery writes nothing to the repository, so verification means confirming the +report describes the repository the user meant: + +1. `proven` is `true`. When it is `false`, report only that Harbor is missing. +2. `repo_root` is the repository they named. +3. `configs` lists the suite they care about. An empty list on a repository they + described as having evals means the configs sit deeper than four directories, or + declare no `datasets` or `tasks` list. +4. `task_count` is in the range they expect. A count of zero with a passing `tasks` + check means the config resolves tasks from a registry, not from disk. + +Report `proven`, `runnable`, and the failing check names. Never describe a suite as +ready to run while `runnable` is `false`. + +## Files in this skill + +Provider-specific code sits under `scripts/providers/`, so support for a second +evaluation provider is an added directory rather than a change to the entry point. + +| Path | Purpose | +|---|---| +| `scripts/discover.py` | Entry point. Owns phase order, report assembly, and the exit code, and nothing provider-specific | +| `scripts/_checks.py` | The check result contract, ported from the platform so both sides read alike | +| `scripts/providers/harbor/_probe.py` | Detects whether Harbor can judge this repository. Standard library only | +| `scripts/providers/harbor/_inventory.py` | Finds configs, datasets, and task directories. Standard library only | +| `scripts/providers/harbor/_ladder.py` | Runs Harbor's validators. Imported only after the probe reports Harbor available | + +The provider directory deliberately sits one level down. A `scripts/harbor/` +directory would be importable as `harbor`, which on a machine without Harbor makes +`find_spec("harbor")` succeed and the probe report an install that is not there. diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/_checks.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/_checks.py new file mode 100644 index 0000000000..d744b9420a --- /dev/null +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/_checks.py @@ -0,0 +1,101 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Readiness result construction and presentation. + +A standard-library port of ``nemo_insights_plugin.contracts.checks``, which the +platform-side discovery command uses. The field names, statuses, severities, and +rendered symbols match, so a report produced by the skill reads the same as one +produced by the CLI. Keep them aligned when either side changes. + +Uses ``dataclass`` rather than ``pydantic.BaseModel`` so this module carries no +dependency of its own. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + +PASS = "pass" +WARN = "warn" +FAIL = "fail" + +REQUIRED = "required" +ADVISORY = "advisory" + +_MARKS = {PASS: "\u2713", WARN: "\u26a0", FAIL: "\u2717"} + + +@dataclass +class CheckResult: + """One required or advisory readiness check. + + ``proven`` records whether Harbor judged this result or the skill merely + observed it. An observed result never counts as evidence that a suite runs, + so the report keeps the distinction rather than flattening it. + """ + + name: str + group: str + status: str + severity: str + message: str + hint: str | None = None + proven: bool = field(default=True) + + def as_dict(self) -> dict: + """Return the JSON-serializable form.""" + return { + "name": self.name, + "group": self.group, + "status": self.status, + "severity": self.severity, + "message": self.message, + "hint": self.hint, + "proven": self.proven, + } + + +def check( + name: str, + group: str, + status: str, + message: str, + *, + severity: str = REQUIRED, + hint: str | None = None, + proven: bool = True, +) -> CheckResult: + """Build one check result.""" + return CheckResult( + name=name, + group=group, + status=status, + severity=severity, + message=message, + hint=hint, + proven=proven, + ) + + +def format_report(results: list[CheckResult]) -> str: + """Format checks into deterministic grouped terminal output.""" + lines: list[str] = [] + for group in sorted({result.group for result in results}): + lines.append(group.capitalize()) + for result in (item for item in results if item.group == group): + suffix = "" if result.proven else " (observed, not proven)" + lines.append(" {} {}{}".format(_MARKS[result.status], result.message, suffix)) + if result.hint and result.status != PASS: + lines.append(" hint: {}".format(result.hint)) + return "\n".join(lines) + + +def required_failures(results: list[CheckResult]) -> list[CheckResult]: + """Return required failures that block a command.""" + return [result for result in results if result.status == FAIL and result.severity == REQUIRED] + + +def advisories(results: list[CheckResult]) -> list[CheckResult]: + """Return non-blocking warnings.""" + return [result for result in results if result.status == WARN] diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py new file mode 100644 index 0000000000..0832ddad87 --- /dev/null +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py @@ -0,0 +1,246 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Record whether a repository's Harbor evaluations are ready to run. + +Discovery exists so a later, cheaper model can run a repository's Harbor evals +without re-deriving how. That only works when every recorded fact was proved +rather than observed, so this command finds the repository's Harbor artifacts and +then makes Harbor's own validators judge them. + +Three phases, in order: + +1. **Probe.** Standard library only. Is Harbor importable, and is its CLI on PATH? +2. **Inventory.** Standard library only. Which config files, dataset directories, + and task directories does the repository own? +3. **Judge.** Only when Harbor is importable. Run the full validation ladder: + schema, job resolution, agent, environment backend, CLI round trip, per-task + validity, dropped-task coverage, and required host variables. + +All three phases are provider-specific and live under ``providers/harbor/``. This +module owns only the parts no provider changes: argument parsing, phase order, +report assembly, and the exit code. + +Without Harbor, phase 3 cannot run and no claim about runnability is possible. +The report is still emitted, every finding is marked ``"proven": false``, and the +verdict is a required failure naming what to install. An unproven inventory is +useful for orienting in an unfamiliar repository; it is never evidence. + +This skill carries no dependency of its own. Harbor is the one import beyond the +standard library, and a repository holding Harbor evaluations has Harbor by +construction. PyYAML, pydantic, and toml arrive with it. + +Usage: + discover.py [--repo PATH] [--out PATH] [--compact] + + --repo PATH Repository to inspect. Defaults to the working directory. + --out PATH Also write the Markdown report to PATH. + --compact Emit single-line JSON. + +Prints a JSON report on stdout. + +Exit codes: + 0 every repository-owned config passed every required check + 1 a required check failed, Harbor is unavailable, or the path is unusable + +WARNING: Run this only against a trusted repository. Validating an agent's +import path executes module top-level code. +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import sys +from datetime import datetime, timezone +from pathlib import Path + +# Every bundled module resolves against this directory, so put it on the path +# before importing one. Note that it holds no directory named after a provider +# package: a `harbor/` directory here would satisfy `find_spec("harbor")` on a +# machine without Harbor and make the probe claim an install that is not there. +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from _checks import CheckResult, format_report, required_failures # noqa: E402 +from providers.harbor import _probe # noqa: E402 +from providers.harbor._inventory import RepositoryScan, scan_repository # noqa: E402 + +_SCHEMA_VERSION = 1 +_MIN_PYTHON = (3, 11) + + +def _unproven(checks: list[CheckResult]) -> list[CheckResult]: + """Mark observed findings so they cannot read as evidence.""" + for result in checks: + result.proven = False + return checks + + +async def _judge(scan: RepositoryScan, repo_root: Path) -> list[dict]: + """Run the validation ladder against each config Harbor can read. + + Imported here rather than at module scope because the ladder imports Harbor, + which a repository without Harbor does not have. + """ + from providers.harbor import _ladder + + configs: list[dict] = [] + for candidate in scan.configs: + outcome = await _ladder.run_ladder(candidate, repo_root) + configs.append( + { + "name": candidate.name, + "path": _display(candidate.path, repo_root), + "runnable": not required_failures(outcome.checks), + "required_env_vars": [ + { + "name": item.name, + "default": item.default, + "declared_in": _display(item.declared_in, repo_root), + } + for item in outcome.required_env_vars + ], + "checks": [result.as_dict() for result in outcome.checks], + "_checks": outcome.checks, + } + ) + return configs + + +def _unjudged(scan: RepositoryScan, repo_root: Path) -> list[dict]: + """Describe each config without claiming anything about it.""" + return [ + { + "name": candidate.name, + "path": _display(candidate.path, repo_root), + "runnable": False, + "required_env_vars": [], + "checks": [], + "_checks": [], + } + for candidate in scan.configs + ] + + +def _display(path: Path, repo_root: Path) -> str: + if not path.is_absolute(): + return path.as_posix() + try: + return path.resolve().relative_to(repo_root.resolve()).as_posix() + except ValueError: + return path.as_posix() + + +def _run_command(repo_root: Path, configs: list[dict]) -> str | None: + """Return the Harbor command, only when exactly one config is runnable.""" + runnable = [config for config in configs if config["runnable"]] + if len(configs) != 1 or len(runnable) != 1: + return None + return "cd {} && harbor job start -c {}".format(repo_root, runnable[0]["path"]) + + +def _render_markdown(report: dict, grouped: list[CheckResult]) -> str: + """Render the report as Markdown with a status block per config.""" + lines = ["# Discovery report for `{}`".format(report["repo_root"])] + lines.extend(["", "Proven by Harbor: {}".format("yes" if report["proven"] else "no")]) + # Validation checks belong to a config, so they render under that config. + status = format_report([result for result in grouped if result.group != "validation"]) + if status: + lines.extend(["", "```text", status, "```"]) + if report["configs"]: + lines.extend(["", "## Harbor entrypoints"]) + for config in report["configs"]: + lines.extend( + [ + "", + "### `{}` (`{}`)".format(config["name"], config["path"]), + "", + "Runnable: {}".format("true" if config["runnable"] else "false"), + ] + ) + if config["checks"]: + rendered = format_report([CheckResult(**item) for item in config["checks"]]) + lines.extend(["", "```text", rendered, "```"]) + if report["run_command"]: + lines.extend(["", "```bash", report["run_command"], "```"]) + return "\n".join(lines) + "\n" + + +def _fail(message: str, hint: str) -> int: + json.dump({"error": message, "hint": hint}, sys.stdout, indent=2) + sys.stdout.write("\n") + return 1 + + +async def _main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Record whether a repository's Harbor evaluations are ready to run.") + parser.add_argument("--repo", type=Path, default=Path(), help="Repository to inspect.") + parser.add_argument("--out", type=Path, default=None, help="Also write the Markdown report to this path.") + parser.add_argument("--compact", action="store_true", help="Emit single-line JSON.") + args = parser.parse_args(argv) + + if sys.version_info < _MIN_PYTHON: + return _fail( + "Discovery needs Python {}.{} or later; this is {}.".format( + _MIN_PYTHON[0], _MIN_PYTHON[1], ".".join(str(part) for part in sys.version_info[:3]) + ), + "Re-run with a newer interpreter, for example `python3.12 discover.py --repo .`.", + ) + + repo_root = args.repo.expanduser() + if not repo_root.is_dir(): + return _fail( + "Not a directory: {}".format(repo_root), + "Pass the repository that holds your Harbor configs and task directories.", + ) + repo_root = repo_root.resolve() + + runtime = _probe.probe() + runtime_checks = _probe.probe_checks(runtime) + scan = scan_repository(repo_root) + proven = _probe.is_available(runtime) + configs = await _judge(scan, repo_root) if proven else _unjudged(scan, repo_root) + + repository_checks = scan.checks if proven else _unproven(scan.checks) + grouped = [*runtime_checks, *repository_checks, *(item for config in configs for item in config["_checks"])] + for config in configs: + config.pop("_checks") + + runnable = proven and bool(configs) and all(config["runnable"] for config in configs) + report = { + "schema_version": _SCHEMA_VERSION, + "repo_root": repo_root.as_posix(), + "provider": _probe.PROVIDER, + "proven": proven, + "runnable": runnable, + "runtime": runtime, + "configs": configs, + "dataset_paths": [_display(path, repo_root) for path in scan.dataset_paths], + "task_count": len(scan.task_paths), + "ethos_path": scan.ethos_path, + "fingerprint": "sha256:{}".format(scan.fingerprint), + "input_file_count": scan.input_file_count, + "discovered_at": datetime.now(timezone.utc).isoformat(), + "checks": [result.as_dict() for result in grouped], + } + report["run_command"] = _run_command(repo_root, configs) + + if args.out is not None: + args.out.parent.mkdir(parents=True, exist_ok=True) + args.out.write_text(_render_markdown(report, grouped), encoding="utf-8") + report["report_path"] = args.out.as_posix() + + json.dump(report, sys.stdout, indent=None if args.compact else 2) + sys.stdout.write("\n") + return 0 if runnable else 1 + + +def main(argv: list[str] | None = None) -> int: + """Run discovery and print the JSON report.""" + return asyncio.run(_main(argv)) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_inventory.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_inventory.py new file mode 100644 index 0000000000..8984c9c07d --- /dev/null +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_inventory.py @@ -0,0 +1,290 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Find the Harbor artifacts a repository owns. + +Standard library only, and safe to import when Harbor is absent, so an inventory +survives to orient in a repository the ladder cannot judge. + +A standard-library port of the repository scan in +``nemo_eval_author_plugin/discovery/scan.py``, with the platform reads removed: +no client, no workspace, no Intake trace probe, and the agent doctrine comes from +a local ``ETHOS.md`` rather than a downloaded ``AGENT-SPEC.md``. + +Everything here observes rather than proves. Finding a config file says nothing +about whether Harbor accepts it, which is why the ladder in ``_ladder.py`` runs +next whenever Harbor is importable. + +``yaml`` is used when available and is not a dependency of this skill: Harbor +depends on PyYAML, so a repository with Harbor installed always has it. Without +it, config detection falls back to a top-level key scan and every candidate is +marked unparsed. +""" + +from __future__ import annotations + +import hashlib +import json +import os +import re +from collections.abc import Iterator +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +from _checks import ADVISORY, FAIL, PASS, WARN, CheckResult, check + +try: + import yaml +except ModuleNotFoundError: # ships with Harbor; absent only when Harbor is + yaml = None # ty: ignore[invalid-assignment] + +_CONFIG_SUFFIXES = (".yaml", ".yml", ".json") +_MAX_CONFIG_DEPTH = 4 +_WORK_KEYS = ("datasets", "tasks") +# Matches a top-level `datasets:` or `tasks:` key, for the no-PyYAML fallback. +_WORK_KEY_PATTERN = re.compile(r"^(?:{}):".format("|".join(_WORK_KEYS)), re.MULTILINE) +_PRUNE_DIR_NAMES = frozenset( + { + ".git", + ".venv", + "venv", + "node_modules", + "__pycache__", + ".ruff_cache", + ".pytest_cache", + ".mypy_cache", + ".tox", + ".eggs", + ".cache", + "site-packages", + "vendor", + "cache", + "dist", + "build", + "eval-and-optimize", + ".nemo-optimizer", + "jobs", + } +) + + +@dataclass(frozen=True) +class ConfigCandidate: + """A repository-owned Harbor config file. + + ``data`` is empty when PyYAML is absent and the file is YAML. The ladder + needs parsed data, so an unparsed candidate is reported and skipped. + """ + + path: Path + data: dict[str, Any] + parsed: bool + + @property + def name(self) -> str: + """Return the declared job name or the file name.""" + job_name = self.data.get("job_name") + return job_name.strip() if isinstance(job_name, str) and job_name.strip() else self.path.name + + +@dataclass +class RepositoryScan: + """The repository facts the validation ladder needs.""" + + configs: list[ConfigCandidate] + dataset_paths: list[Path] + task_paths: list[Path] + ethos_path: str | None + fingerprint: str + input_file_count: int + checks: list[CheckResult] + + +def _check(name: str, status: str, message: str, **kwargs: Any) -> CheckResult: + return check(name, "repository", status, message, **kwargs) + + +def walk_dirs(root: Path, *, max_depth: int | None = None) -> Iterator[Path]: + """Yield repository directories and skip generated trees.""" + for current, dir_names, _ in os.walk(root): + directory = Path(current) + depth = len(directory.relative_to(root).parts) + dir_names[:] = sorted( + name for name in dir_names if name not in _PRUNE_DIR_NAMES and (max_depth is None or depth < max_depth) + ) + yield directory + + +def scan_repository(repo_root: Path) -> RepositoryScan: + """Find repo-owned configs, Harbor datasets, and task directories.""" + repo_root = repo_root.resolve() + configs = _config_candidates(repo_root) + checks: list[CheckResult] = [] + + if not configs: + checks.append( + _check( + "config", + FAIL, + "No repository-owned Harbor config file exists.", + hint="Add a YAML, YML, or JSON config with a nonempty datasets or tasks list.", + ) + ) + else: + count = len(configs) + plural = "s" if count != 1 else "" + checks.append(_check("config", PASS, "Found {} repository-owned Harbor config file{}.".format(count, plural))) + + unparsed = [candidate for candidate in configs if not candidate.parsed] + if unparsed: + names = ", ".join(candidate.path.name for candidate in unparsed) + checks.append( + _check( + "config-parse", + FAIL, + "Cannot read {} config file{}: {}.".format(len(unparsed), "s" if len(unparsed) != 1 else "", names), + hint="Install PyYAML, which arrives with Harbor, to read YAML configs.", + ) + ) + + ethos: tuple[str, bytes] | None = None + if (repo_root / "ETHOS.md").is_file(): + ethos = ("ETHOS.md", (repo_root / "ETHOS.md").read_bytes()) + checks.append(_check("ethos", PASS, "ETHOS.md defines the agent doctrine.", severity=ADVISORY)) + else: + checks.append( + _check( + "ethos", + WARN, + "ETHOS.md does not exist at the repository root.", + severity=ADVISORY, + hint="Add ETHOS.md to define the agent doctrine.", + ) + ) + + datasets = _dataset_paths(repo_root) + tasks = _task_paths(repo_root) + if tasks: + checks.append( + _check( + "tasks-on-disk", + PASS, + "Found {} task {} in {} dataset {}.".format( + len(tasks), + "directory" if len(tasks) == 1 else "directories", + len(datasets), + "directory" if len(datasets) == 1 else "directories", + ), + severity=ADVISORY, + proven=False, + ) + ) + else: + checks.append( + _check( + "tasks-on-disk", + WARN, + "No task directories exist. A Harbor task directory holds a task.toml.", + severity=ADVISORY, + proven=False, + ) + ) + + fingerprint, count = _fingerprint(repo_root, [config.path for config in configs], ethos, datasets) + return RepositoryScan( + configs=configs, + dataset_paths=datasets, + task_paths=tasks, + ethos_path=ethos[0] if ethos else None, + fingerprint=fingerprint, + input_file_count=count, + checks=checks, + ) + + +def _config_candidates(repo_root: Path) -> list[ConfigCandidate]: + candidates: list[ConfigCandidate] = [] + for directory in walk_dirs(repo_root, max_depth=_MAX_CONFIG_DEPTH): + for path in sorted(directory.iterdir()): + if path.is_symlink() or not path.is_file() or path.suffix.lower() not in _CONFIG_SUFFIXES: + continue + candidate = _candidate(path) + if candidate is not None: + candidates.append(candidate) + return sorted( + candidates, + key=lambda candidate: ( + len(candidate.path.relative_to(repo_root).parts) - 1, + candidate.path.relative_to(repo_root).as_posix(), + ), + ) + + +def _candidate(path: Path) -> ConfigCandidate | None: + """Return a candidate when the file declares Harbor work, else None.""" + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeError): + return None + + is_json = path.suffix.lower() == ".json" + if is_json or yaml is not None: + try: + data = json.loads(text) if is_json else yaml.safe_load(text) + except (json.JSONDecodeError, ValueError): + return None + if not isinstance(data, dict) or not _has_work(data): + return None + return ConfigCandidate(path=path, data=data, parsed=True) + + if not _WORK_KEY_PATTERN.search(text): + return None + return ConfigCandidate(path=path, data={}, parsed=False) + + +def _has_work(data: dict[str, Any]) -> bool: + return any(isinstance(data.get(name), list) and data[name] for name in _WORK_KEYS) + + +def _dataset_paths(repo_root: Path) -> list[Path]: + datasets: set[Path] = set() + for directory in walk_dirs(repo_root): + if directory != repo_root and directory.name != "task_template" and (directory / "task.toml").is_file(): + datasets.add(directory.parent) + return sorted(datasets) + + +def _task_paths(repo_root: Path) -> list[Path]: + return sorted( + directory + for directory in walk_dirs(repo_root) + if directory != repo_root and directory.name != "task_template" and (directory / "task.toml").is_file() + ) + + +def _fingerprint( + repo_root: Path, + config_paths: list[Path], + ethos: tuple[str, bytes] | None, + datasets: list[Path], +) -> tuple[str, int]: + files = {path for path in [*config_paths, repo_root / "optimizer.yaml"] if path.is_file()} + for dataset in datasets: + if not dataset.is_relative_to(repo_root): + continue + for directory in walk_dirs(dataset): + files.update( + path for path in directory.iterdir() if path.is_file() and path.resolve().is_relative_to(repo_root) + ) + files.discard(repo_root / "ETHOS.md") + + digest = hashlib.sha256() + for path in sorted(files): + digest.update(str(path.relative_to(repo_root)).encode()) + digest.update(b"\0") + digest.update(path.read_bytes()) + digest.update(b"\0") + if ethos is not None: + digest.update(ethos[0].encode() + b"\0" + ethos[1] + b"\0") + return digest.hexdigest(), len(files) + (ethos is not None) diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_ladder.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_ladder.py new file mode 100644 index 0000000000..51b1b3adbc --- /dev/null +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_ladder.py @@ -0,0 +1,358 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Make Harbor judge a repository-owned config. + +A port of ``nemo_eval_author_plugin/discovery/validate.py``. Every rung asks +Harbor's own validators for a verdict, so each recorded fact is proved rather +than observed. Nothing here reimplements a Harbor rule. + +This module imports Harbor at module scope. Import it only after ``_probe`` reports +Harbor available, so that a repository without Harbor still gets an inventory +instead of an ImportError. + +``from harbor... import`` here reaches the installed Harbor library, not the +directory holding this file. Python 3 imports are absolute, and ``scripts/`` holds +no top-level ``harbor`` name for the enclosing directory to be found under. +""" + +from __future__ import annotations + +import contextlib +import shutil +import subprocess +import sys +import tomllib +from collections.abc import Iterator +from dataclasses import dataclass, field +from fnmatch import fnmatchcase +from pathlib import Path + +from _checks import ADVISORY, FAIL, PASS, REQUIRED, WARN, CheckResult, check +from harbor.agents.factory import AgentFactory +from harbor.environments.factory import EnvironmentFactory +from harbor.job import Job +from harbor.models.agent.name import AgentName +from harbor.models.job.config import JobConfig +from harbor.models.task.config import TaskConfig +from harbor.models.task.paths import TaskPaths +from harbor.models.task.task import Task +from harbor.utils.env import get_required_host_vars +from harbor.utils.import_path import import_class +from providers.harbor._inventory import ConfigCandidate +from pydantic import ValidationError + +_ROUND_TRIP_TIMEOUT_SEC = 120 + + +@dataclass +class RequiredEnvVar: + """A host variable required by a Harbor config.""" + + name: str + default: str | None + declared_in: Path + + +@dataclass +class ValidationOutcome: + """Results from one Harbor preflight.""" + + checks: list[CheckResult] = field(default_factory=list) + required_env_vars: list[RequiredEnvVar] = field(default_factory=list) + + +def _check(name: str, status: str, message: str, **kwargs: object) -> CheckResult: + return check(name, "validation", status, message, **kwargs) # ty: ignore[invalid-argument-type] + + +async def run_ladder(candidate: ConfigCandidate, repo_root: Path) -> ValidationOutcome: + """Run the complete preflight without caching or skipping any check.""" + outcome = ValidationOutcome() + if not candidate.parsed: + outcome.checks.append( + _check( + "schema", + FAIL, + "Cannot read {} to validate it.".format(candidate.path.name), + hint="Install PyYAML, which arrives with Harbor, to read YAML configs.", + ) + ) + return outcome + + with contextlib.chdir(repo_root): + try: + config = JobConfig.model_validate(candidate.data) + config.validate_agent_concurrency_limits() + except ValidationError as exc: + errors = exc.errors(include_url=False, include_input=False) + outcome.checks.append(_check("schema", FAIL, "Harbor rejected the job config: {}".format(errors))) + return outcome + except ValueError as exc: + outcome.checks.append(_check("schema", FAIL, "Harbor rejected the job config: {}".format(exc))) + return outcome + outcome.checks.append(_check("schema", PASS, "Harbor accepts the job config schema.")) + + job = await _resolve(config, outcome) + _check_agent(config, outcome) + _check_backend(config, outcome) + outcome.checks.append(check_config_file(candidate.path, repo_root)) + if job is None: + return outcome + + resolved = _resolved_task_paths(job) + if resolved is None: + outcome.checks.append( + _check( + "compatibility", + FAIL, + "This Harbor version does not expose Job._task_configs.", + hint="Install a Harbor version that exposes the resolved task list.", + ) + ) + return outcome + task_dirs = _check_tasks(resolved, outcome) + _check_coverage(config, resolved, outcome) + _check_required_env_vars(config, task_dirs, outcome) + return outcome + + +async def _resolve(config: JobConfig, outcome: ValidationOutcome) -> Job | None: + import tempfile + + try: + with tempfile.TemporaryDirectory(prefix="eval-author-jobs-") as scratch: + job = await Job.create(config.model_copy(update={"jobs_dir": Path(scratch)})) + job._close_logger_handlers() + except Exception as exc: + outcome.checks.append( + _check( + "resolution", + FAIL, + "Harbor could not resolve the job: {}: {}".format(type(exc).__name__, exc), + hint="This error occurs before Harbor starts a container.", + ) + ) + return None + outcome.checks.append(_check("resolution", PASS, "Harbor resolved the job.")) + return job + + +def _resolved_task_paths(job: Job) -> list[Path] | None: + task_configs = getattr(job, "_task_configs", None) + if task_configs is None: + return None + paths: list[Path] = [] + for task_config in task_configs: + try: + paths.append(task_config.get_local_path().resolve()) + except ValueError: + continue + return paths + + +def _check_tasks(resolved: list[Path], outcome: ValidationOutcome) -> list[Path]: + valid = [path for path in resolved if Task.is_valid_dir(path)] + if not resolved: + outcome.checks.append(_check("tasks", FAIL, "The config resolves to zero tasks.")) + return [] + outcome.checks.append( + _check( + "tasks", + FAIL if len(valid) != len(resolved) else PASS, + "{} of {} task dirs are valid Harbor tasks.".format(len(valid), len(resolved)), + ) + ) + return valid + + +def _check_coverage(config: JobConfig, resolved: list[Path], outcome: ValidationOutcome) -> None: + """Report task dirs Harbor dropped. + + Harbor skips task directories it cannot parse without raising, so a per-task + check alone reports every survivor valid while one silently vanishes. + """ + resolved_set = {path.resolve() for path in resolved} + dropped_any = False + for dataset in config.datasets: + if dataset.path is None or not dataset.path.is_dir(): + continue + on_disk = [ + child + for child in sorted(dataset.path.iterdir()) + if child.is_dir() and child.name != "task_template" and (child / "task.toml").is_file() + ] + dropped = [child for child in on_disk if child.resolve() not in resolved_set] + if not dropped: + continue + dropped_any = True + selected_dropped = [ + path + for path in dropped + if (not dataset.task_names or any(fnmatchcase(path.name, pattern) for pattern in dataset.task_names)) + and not any(fnmatchcase(path.name, pattern) for pattern in dataset.exclude_task_names or []) + ] + required = bool(selected_dropped) and dataset.n_tasks is None + filtered = bool(dataset.task_names or dataset.exclude_task_names) + reported = selected_dropped if required else dropped + names = ", ".join(path.name for path in reported) + outcome.checks.append( + _check( + "coverage", + FAIL if required else WARN, + "Harbor did not resolve {} task dirs: {}.".format(len(reported), names), + severity=REQUIRED if required else ADVISORY, + hint=( + "Harbor skipped a task selected by the dataset filters." + if required and filtered + else "Harbor skips these task dirs silently." + if required + else "The dataset filters or n_tasks select a task subset." + ), + ) + ) + if not dropped_any: + outcome.checks.append(_check("coverage", PASS, "Harbor dropped no local task dirs.")) + + +def _check_required_env_vars(config: JobConfig, task_dirs: list[Path], outcome: ValidationOutcome) -> None: + required: dict[str, RequiredEnvVar] = {} + + def collect(env: dict[str, str], declared_in: Path) -> None: + for name, default in get_required_host_vars(env): + required.setdefault(name, RequiredEnvVar(name, default, declared_in)) + + for task_dir in task_dirs: + task_config = _task_config(task_dir) + if task_config is None: + continue + path = TaskPaths(task_dir).config_path + collect(task_config.environment.env, path) + collect(task_config.verifier.env, path) + collect(task_config.solution.env, path) + collect(config.environment.env, Path("")) + collect(config.verifier.env, Path("")) + for agent in config.agents: + collect(agent.env, Path("")) + outcome.required_env_vars = sorted(required.values(), key=lambda item: item.name) + names = ", ".join(item.name for item in outcome.required_env_vars) + outcome.checks.append( + _check( + "credentials", + PASS, + "{} host variables required".format(len(required)) + (": {}.".format(names) if names else "."), + ) + ) + + +def _task_config(task_dir: Path) -> TaskConfig | None: + try: + return TaskConfig.model_validate_toml(TaskPaths(task_dir).config_path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, tomllib.TOMLDecodeError, ValidationError): + return None + + +def _check_agent(config: JobConfig, outcome: ValidationOutcome) -> None: + for agent in config.agents: + if agent.import_path is not None: + with _evict_module_tree(agent.import_path): + try: + imported = import_class(agent.import_path, label="agent") + except (Exception, SystemExit) as exc: + outcome.checks.append( + _check( + "agent", + FAIL, + "Cannot import agent {}: {}: {}".format(agent.import_path, type(exc).__name__, exc), + ) + ) + else: + outcome.checks.append( + _check("agent", PASS, "Agent {} imports as a class.".format(imported.__name__)) + ) + elif agent.name is not None: + try: + AgentFactory.get_agent_class(AgentName(agent.name)) + except Exception as exc: + outcome.checks.append( + _check( + "agent", + FAIL, + "Cannot load Harbor agent {}: {}: {}".format(agent.name, type(exc).__name__, exc), + ) + ) + else: + outcome.checks.append(_check("agent", PASS, "Built-in agent {} is available.".format(agent.name))) + + +def _check_backend(config: JobConfig, outcome: ValidationOutcome) -> None: + label = config.environment.import_path or (config.environment.type.value if config.environment.type else "docker") + try: + EnvironmentFactory.run_preflight(config.environment.type, config.environment.import_path) + except (Exception, SystemExit) as exc: + outcome.checks.append( + _check( + "backend", + FAIL, + "Environment backend {} is not ready: {}: {}".format(label, type(exc).__name__, exc), + ) + ) + else: + outcome.checks.append(_check("backend", PASS, "Environment backend {} passed preflight.".format(label))) + + +def check_config_file(config_path: Path, repo_root: Path) -> CheckResult: + """Check the bytes that Harbor receives from its CLI.""" + harbor = _harbor_executable() + if harbor is None: + return _check( + "round-trip", + WARN, + "The Harbor CLI round trip did not run.", + severity=ADVISORY, + hint="No harbor executable exists on PATH.", + ) + try: + completed = subprocess.run( + [harbor, "job", "start", "--print-config", "-c", str(config_path)], + cwd=repo_root, + capture_output=True, + text=True, + timeout=_ROUND_TRIP_TIMEOUT_SEC, + check=False, + ) + except (OSError, subprocess.SubprocessError) as exc: + return _check("round-trip", WARN, "The Harbor CLI round trip failed: {}".format(exc), severity=ADVISORY) + if completed.returncode: + detail = (completed.stderr or completed.stdout).strip().splitlines() + return _check( + "round-trip", + FAIL, + "The Harbor CLI rejected the config: {}.".format(detail[-1] if detail else "no output"), + ) + return _check("round-trip", PASS, "The config file loads through the Harbor CLI.") + + +def _harbor_executable() -> str | None: + local = Path(sys.executable).parent / "harbor" + return str(local) if local.is_file() else shutil.which("harbor") + + +@contextlib.contextmanager +def _evict_module_tree(import_path: str) -> Iterator[None]: + """Import without cached modules from another repository.""" + module = import_path.split(":", 1)[0].split(".", 1)[0] + previous = { + name: cached + for name, cached in list(sys.modules.items()) + if name == module or name.startswith("{}.".format(module)) + } + for name in previous: + sys.modules.pop(name) + try: + yield + finally: + for name in list(sys.modules): + if name == module or name.startswith("{}.".format(module)): + sys.modules.pop(name) + sys.modules.update(previous) diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_probe.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_probe.py new file mode 100644 index 0000000000..6042856454 --- /dev/null +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_probe.py @@ -0,0 +1,91 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Detect whether Harbor can judge this repository. + +Standard library only, and safe to import when Harbor is absent. Everything that +touches Harbor itself lives in ``_ladder.py``, which ``discover.py`` imports only +after this module reports Harbor available. + +This is the provider gate. A second provider adds its own probe next to this one +rather than changing ``discover.py``. +""" + +from __future__ import annotations + +import importlib.metadata +import importlib.util +import shutil +import sys +from pathlib import Path + +from _checks import ADVISORY, FAIL, PASS, WARN, CheckResult, check + +PROVIDER = "harbor" + + +def probe() -> dict: + """Report the runtime without importing Harbor. + + ``find_spec`` answers whether Harbor is importable without paying the import + or running its top-level code, which matters when the answer is no. + """ + importable = False + version = None + try: + importable = importlib.util.find_spec("harbor") is not None + except (ImportError, ValueError): + importable = False + if importable: + try: + version = importlib.metadata.version("harbor") + except importlib.metadata.PackageNotFoundError: + version = None + + executable = Path(sys.executable).parent / "harbor" + return { + "python": ".".join(str(part) for part in sys.version_info[:3]), + "harbor_importable": importable, + "harbor_version": version, + "harbor_cli": str(executable) if executable.is_file() else shutil.which("harbor"), + } + + +def probe_checks(runtime: dict) -> list[CheckResult]: + """Turn the probe into the checks that gate the validation ladder.""" + checks: list[CheckResult] = [] + if runtime["harbor_importable"]: + checks.append( + check( + "harbor", + "runtime", + PASS, + "Harbor {} is importable, so Harbor judges this report.".format(runtime["harbor_version"] or "?"), + ) + ) + else: + checks.append( + check( + "harbor", + "runtime", + FAIL, + "Harbor is not importable, so nothing in this report is proven.", + hint="Install Harbor into the interpreter running this script, then run discovery again.", + ) + ) + if runtime["harbor_cli"] is None: + checks.append( + check( + "harbor-cli", + "runtime", + WARN, + "No harbor executable exists on PATH, so the CLI round trip cannot run.", + severity=ADVISORY, + ) + ) + return checks + + +def is_available(runtime: dict) -> bool: + """Return whether the ladder can run against this runtime.""" + return bool(runtime["harbor_importable"]) diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md new file mode 100644 index 0000000000..fc3387cf48 --- /dev/null +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md @@ -0,0 +1,114 @@ +--- +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +name: eval-author +description: >- + Work on the evaluation suites that live in a user's own repository: establish + whether they run, explain why one fails, and report what it needs. Owns the + standard every sub-flow follows, which is that a provider's own validators + judge every recorded fact rather than the agent guessing from file layout. Use + when the user asks about the evals in their repository without naming a step: + "help me with my evals", "what's the state of the eval suite here?", "are these + evals any good?", "I inherited this repo and there are Harbor tasks in it", or + when you need to pick between the Eval Author sub-flows. Routes to a sub-flow; + reads the repository and changes nothing in it. +triggers: + - help me with the evals in this repo + - what is the state of the eval suite here + - I inherited a repo with Harbor tasks in it + - work on my evaluation suite + - which eval author step do I need +not-for: + - eval-author-discover (use to run the discovery pass and get a runnable verdict) + - nemo-experimentalist (use to run insight-driven optimization end to end, which drives the Eval Author agent itself) + - nemo-evaluator (use to run an existing benchmark rather than work on a repository's own suite) +compatibility: Reading only. Each sub-flow states its own runtime needs. +maturity: alpha +license: Apache-2.0 +user-invocable: true +allowed-tools: [Read, Grep, Glob] +--- + +# Eval Author + +Work on the evaluation suites that live in a user's own repository, so a later and +cheaper model can run them without working out how from scratch. + +That last clause is the whole point, and it sets a standard the sub-flows have to +meet. A report that a downstream model trusts has to be right. A report that is +merely plausible is worse than no report, because it gets acted on. + +## The standard + +**Every fact you record is one a provider's validators confirmed, not one you +observed.** + +The two are easy to confuse and not close to equivalent: + +- **Observed.** A `harbor-job.yaml` exists, and a directory holds a `task.toml`. + You read the filesystem and described it. +- **Proven.** Harbor parsed that config, resolved it into tasks, confirmed each + task directory is valid, and named the host variables it needs. + +Observation cannot substitute for proof. A config file that exists can still name +a dataset path that is absent, an agent that does not exist, or tasks the provider +silently drops. Each of those passes every structural check you could invent and +fails the moment the suite runs. + +So no sub-flow reimplements a provider's rules. It asks the provider. Where a +sub-flow can only observe, it says so and marks the finding unproven. + +## Vocabulary + +The sub-flows share this language, and reports use it verbatim. + +| Term | Meaning | +|---|---| +| Check | One named result: `pass`, `warn`, or `fail`. Carries a message and, when it fails, a hint | +| Required | A failing required check blocks the suite. Report the suite as not ready | +| Advisory | A warning worth surfacing that blocks nothing | +| Rung | One step of a provider's validation ladder, ordered so a lower rung's failure often clears once a higher one is fixed | +| Proven | A provider judged this check. An unproven check is an observation and never evidence | +| Provider | The evaluation framework that owns the rules. Harbor today | + +## Sub-flows + +Read the sub-flow's own `SKILL.md` and follow it. This file carries the standard +and the boundaries; the sub-flow carries the steps. + +| Sub-flow | Use it to | +|---|---| +| `eval-author-discover` | Establish whether a repository's evaluations run, name the rung that fails, and get the exact command to run them | + +Authoring new tasks and verifier metrics is not built yet. When a user asks for +that, say so plainly rather than improvising a task layout by hand. A task written +against a guessed convention scores nothing and costs a full evaluation run to +discover. + +## Boundaries + +These hold for every sub-flow. They exist because the repository belongs to the +user, not to you. + +- **Propose, never mutate.** Read the repository and report. Do not create, edit, + move, or reformat anything in it. A sub-flow that writes a report file writes it + only where the user pointed. +- **A missing tool is a finding, not a task.** When the provider is not installed, + report that and stop. Do not install it into the user's environment. +- **Do not run the suite.** Prove it can run and hand over the command. Starting a + job spends the user's compute and credentials on a decision they did not make. +- **Trusted repositories only.** Validating a config can execute repository code, + because an agent named by import path gets imported. If the repository is not + trusted, say so and stop. +- **No platform services.** Eval Author sub-flows talk to no NeMo service, resolve + no workspace, and upload nothing. Everything happens against the local checkout. + +## Reporting + +Lead with the verdict, then the evidence. + +State whether the findings are proven, whether the suite is ready, and the names +of the checks that failed. Never describe a suite as ready while a required check +fails, and never present an observation as proof. When a sub-flow could not reach +its provider, the only honest headline is that nothing was proven. diff --git a/plugins/nemo-eval-author/tests/test_skill_contract.py b/plugins/nemo-eval-author/tests/test_skill_contract.py new file mode 100644 index 0000000000..273760055c --- /dev/null +++ b/plugins/nemo-eval-author/tests/test_skill_contract.py @@ -0,0 +1,447 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Contract tests for the bundled Eval Author skills. + +``eval-author`` is the core skill: it owns the standard, the vocabulary, the +boundaries, and the routing. ``eval-author-discover`` is a sub-flow that carries +the steps and defers the standard to the core. Both ship as directories customers +copy into their own repository, so nothing at runtime enforces their promises. +These tests are that enforcement: + +- The frontmatter of each skill carries every field ``docs/contributing/skills-spec.mdx`` requires. +- The core routes to every sub-flow, and each sub-flow points back at the core + rather than restating the standard, which is how the two would drift. +- Only the sub-flow can execute. The core routes, so it gets no Bash. +- The bundled scripts depend on Harbor and nothing else. Harbor is acceptable + because a repository holding Harbor evaluations has Harbor by construction; a + NeMo import would not be, and that is the boundary these tests defend. +- ``_ladder.py`` stays out of module scope in ``discover.py``, so a repository + without Harbor gets an inventory instead of an ImportError. +- No bundled directory is named after a provider package. ``scripts/harbor/`` + would be importable as ``harbor``, which makes ``find_spec`` succeed on a + machine with no Harbor and the probe claim an install that is not there. +- The check contract matches ``nemo_insights_plugin.contracts.checks``, so a + report from the skill reads the same as one from the platform command. +- Discovery reports a valid suite runnable and names the rung a broken one fails. + +These tests compare the skill against this repository, never against Harbor's +rules. The skill reimplements no Harbor rule: it asks Harbor for every verdict, +so a Harbor change that tightens a rule flows through without a test change here. +""" + +import ast +import importlib.util +import json +import re +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml +from nemo_insights_plugin.contracts import checks as platform_checks + +_SKILLS_DIR = Path(__file__).resolve().parents[1] / "src" / "nemo_eval_author_plugin" / "skills" +_CORE_DIR = _SKILLS_DIR / "eval-author" +_FLOW_DIR = _SKILLS_DIR / "eval-author-discover" +_SKILL_DIRS = (_CORE_DIR, _FLOW_DIR) +_SUB_FLOW_DIRS = (_FLOW_DIR,) +_SCRIPTS_DIR = _FLOW_DIR / "scripts" +_DISCOVER = _SCRIPTS_DIR / "discover.py" +_LADDER = _SCRIPTS_DIR / "providers" / "harbor" / "_ladder.py" + +_REQUIRED_FRONTMATTER = ( + "name", + "description", + "triggers", + "not-for", + "compatibility", + "maturity", + "license", + "user-invocable", + "allowed-tools", +) +_MAX_BODY_LINES = 500 + +# Matches the core skill name but not a longer name that starts with it, so a +# reference to eval-author-discover cannot pass for a reference to eval-author. +_CORE_REFERENCE = re.compile(rf"\b{re.escape(_CORE_DIR.name)}\b(?!-)") + +# Harbor brings these in, so a bundled script may name them. Nothing else outside +# the standard library may appear. +_PERMITTED_THIRD_PARTY = frozenset({"harbor", "pydantic", "yaml"}) + + +def _not_for_names(frontmatter: dict) -> set[str]: + """Return the skill names in not-for, dropping the parenthetical reason.""" + return {entry.split("(", 1)[0].strip() for entry in frontmatter["not-for"]} + + +def _frontmatter_and_body(skill_dir: Path) -> tuple[dict, str]: + text = (skill_dir / "SKILL.md").read_text(encoding="utf-8") + assert text.startswith("---\n"), f"{skill_dir.name}/SKILL.md must open with YAML frontmatter" + _, frontmatter, body = text.split("---\n", 2) + return yaml.safe_load(frontmatter), body + + +def _bundled_scripts() -> list[Path]: + """Return every bundled module, including the ones under providers/.""" + return sorted(path for path in _SCRIPTS_DIR.rglob("*.py") if "__pycache__" not in path.parts) + + +def _local_roots() -> set[str]: + """Return the top-level names a bundled module can import from scripts/.""" + return {path.stem if path.is_file() else path.name for path in _SCRIPTS_DIR.iterdir()} | { + path.stem for path in _bundled_scripts() + } + + +def _imported_roots(path: Path) -> set[str]: + """Return the root package of every import in one module.""" + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + roots: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + names = [alias.name for alias in node.names] + elif isinstance(node, ast.ImportFrom): + names = [node.module] if node.module and not node.level else [] + else: + continue + roots.update(name.split(".")[0] for name in names) + return roots + + +def _run_discover(repo: Path, *args: str, with_harbor: bool = True) -> tuple[int, dict]: + """Run discover.py as the skill documents it, and parse its JSON. + + ``with_harbor=False`` passes ``-S``, which drops site-packages from the path + so Harbor and PyYAML are both unimportable. That reproduces a customer + repository with no Harbor install without needing a second interpreter. + """ + command = [sys.executable, *([] if with_harbor else ["-S"]), str(_DISCOVER), "--repo", str(repo), *args] + result = subprocess.run(command, capture_output=True, text=True, check=False) + assert result.stdout, f"discover.py printed nothing; stderr:\n{result.stderr}" + return result.returncode, json.loads(result.stdout) + + +def _write_task(task_dir: Path, *, name: str = "smoke/generated") -> None: + """Write a Harbor task that mirrors the canonical prebuilt-image layout. + + ``environment/`` has to exist even though the image is prebuilt, because + ``Task.is_valid_dir`` checks for the directory before reading the config. + """ + (task_dir / "tests").mkdir(parents=True) + (task_dir / "environment").mkdir() + (task_dir / "task.toml").write_text( + 'schema_version = "1.1"\n' + "\n[task]\n" + f'name = "{name}"\n' + 'authors = [{ name = "NVIDIA" }]\n' + "\n[agent]\ntimeout_sec = 120.0\n" + "\n[verifier]\ntimeout_sec = 60.0\n" + '\n[environment]\ndocker_image = "smoke-agent-env:latest"\ncpus = 1\nmemory_mb = 1024\n', + encoding="utf-8", + ) + (task_dir / "instruction.md").write_text("Look up the total hours for Ada.\n", encoding="utf-8") + (task_dir / "tests" / "test.sh").write_text("#!/usr/bin/env bash\nexit 0\n", encoding="utf-8") + + +@pytest.fixture +def suite(tmp_path: Path) -> Path: + """Build a repository with one valid task and one Harbor job config.""" + _write_task(tmp_path / "dataset" / "task-one") + (tmp_path / "harbor-job.yaml").write_text( + "job_name: fixture\ndatasets:\n - path: ./dataset\nagents:\n - name: oracle\n", + encoding="utf-8", + ) + return tmp_path + + +def _named(report: dict, name: str) -> dict: + """Return one check by name, failing loudly when the ladder never ran it.""" + found = next((check for check in report["checks"] if check["name"] == name), None) + assert found is not None, f"no {name!r} check in report; ran {[c['name'] for c in report['checks']]}" + return found + + +@pytest.mark.parametrize("skill_dir", _SKILL_DIRS, ids=lambda path: path.name) +def test_frontmatter_carries_every_required_field(skill_dir: Path) -> None: + frontmatter, _ = _frontmatter_and_body(skill_dir) + missing = [field for field in _REQUIRED_FRONTMATTER if field not in frontmatter] + assert not missing, f"{skill_dir.name} frontmatter is missing {missing}" + assert frontmatter["name"] == skill_dir.name, "frontmatter name must match the skill directory name" + assert frontmatter["maturity"] in {"alpha", "beta", "active", "deprecated"} + assert isinstance(frontmatter["user-invocable"], bool) + assert len(frontmatter["triggers"]) >= 3, "the routing audit needs at least three trigger phrases" + assert len(frontmatter["not-for"]) >= 2, "not-for needs at least two sibling skills to prevent collisions" + + +@pytest.mark.parametrize("skill_dir", _SKILL_DIRS, ids=lambda path: path.name) +def test_no_skill_grants_write_access(skill_dir: Path) -> None: + """Eval Author proposes and never mutates, so a Write grant would contradict the core.""" + frontmatter, _ = _frontmatter_and_body(skill_dir) + tools = set(frontmatter["allowed-tools"]) + assert not {"Write", "Edit", "MultiEdit", "NotebookEdit"} & tools, ( + f"{skill_dir.name} writes nothing, so {sorted(tools)} is too broad" + ) + + +def test_the_core_routes_and_the_sub_flow_executes() -> None: + """The core only picks a sub-flow, so it has no reason to run anything.""" + core_tools = set(_frontmatter_and_body(_CORE_DIR)[0]["allowed-tools"]) + assert "Bash" not in core_tools, f"the core routes and explains; {sorted(core_tools)} lets it execute" + for skill_dir in _SUB_FLOW_DIRS: + tools = set(_frontmatter_and_body(skill_dir)[0]["allowed-tools"]) + assert "Bash" in tools, f"{skill_dir.name} runs a bundled script, which needs Bash" + + +def test_the_core_names_every_sub_flow() -> None: + """A sub-flow the core never mentions cannot be routed to.""" + _, body = _frontmatter_and_body(_CORE_DIR) + for skill_dir in _SUB_FLOW_DIRS: + assert skill_dir.name in body, f"the core does not route to {skill_dir.name}" + + +@pytest.mark.parametrize("skill_dir", _SUB_FLOW_DIRS, ids=lambda path: path.name) +def test_each_sub_flow_defers_to_the_core(skill_dir: Path) -> None: + """A sub-flow points at the core rather than restating the standard itself. + + Restating it is how the two drift: the copy in the sub-flow gets edited and the + core keeps saying something else. The match rejects a bare prefix, so naming + ``eval-author-discover`` does not count as pointing at ``eval-author``. + """ + frontmatter, body = _frontmatter_and_body(skill_dir) + assert _CORE_REFERENCE.search(body), f"{skill_dir.name} must point at {_CORE_DIR.name} for the standard" + assert _CORE_DIR.name in _not_for_names(frontmatter), ( + f"{skill_dir.name} must name {_CORE_DIR.name} in not-for so the router can tell them apart" + ) + + +@pytest.mark.parametrize("skill_dir", _SKILL_DIRS, ids=lambda path: path.name) +def test_skill_body_stays_within_the_line_budget(skill_dir: Path) -> None: + _, body = _frontmatter_and_body(skill_dir) + line_count = len(body.splitlines()) + assert line_count <= _MAX_BODY_LINES, ( + f"{skill_dir.name} body is {line_count} lines, over the {_MAX_BODY_LINES} budget" + ) + + +def test_every_bundled_path_the_skill_names_exists() -> None: + _, body = _frontmatter_and_body(_FLOW_DIR) + for relative in ( + "scripts/discover.py", + "scripts/_checks.py", + "scripts/providers/harbor/_probe.py", + "scripts/providers/harbor/_inventory.py", + "scripts/providers/harbor/_ladder.py", + ): + assert relative in body, f"SKILL.md no longer documents {relative}" + assert (_FLOW_DIR / relative).exists(), f"SKILL.md names {relative}, which is missing on disk" + + +def test_bundled_scripts_never_import_the_platform() -> None: + """The boundary that makes the skill copyable: Harbor is fine, NeMo is not.""" + permitted = _local_roots() | sys.stdlib_module_names | _PERMITTED_THIRD_PARTY + offenders: dict[str, set[str]] = {} + for path in _bundled_scripts(): + found = {root for root in _imported_roots(path) if root not in permitted} + if found: + offenders[path.name] = found + assert not offenders, ( + "Bundled scripts may import the standard library, a sibling, or " + f"{sorted(_PERMITTED_THIRD_PARTY)}, found: " + + "; ".join(f"{filename} imports {sorted(names)}" for filename, names in sorted(offenders.items())) + ) + + +def test_no_bundled_directory_is_named_after_a_provider_package() -> None: + """The trap that makes a literal ``scripts/harbor/`` unusable. + + A directory on the path named ``harbor`` is importable as a namespace package. + On a machine with no Harbor installed that makes ``find_spec("harbor")`` + succeed, so the probe reports an install that is not there and the ladder then + fails on import. Provider code sits one level down for exactly this reason. + """ + collisions = _local_roots() & _PERMITTED_THIRD_PARTY + assert not collisions, ( + f"{sorted(collisions)} shadows a package the skill imports; " + "keep it under scripts/providers/ rather than directly in scripts/" + ) + + +def test_only_the_ladder_imports_harbor() -> None: + """Every other module must keep working when Harbor is absent.""" + assert "harbor" in _imported_roots(_LADDER), "the ladder is the Harbor boundary and must import Harbor" + for path in _bundled_scripts(): + if path == _LADDER: + continue + assert "harbor" not in _imported_roots(path), ( + f"{path.relative_to(_SCRIPTS_DIR)} imports Harbor; move that call behind the probe" + ) + + +def test_discover_defers_the_ladder_import_to_call_time() -> None: + """A module-scope ladder import would break every Harbor-free repository.""" + tree = ast.parse(_DISCOVER.read_text(encoding="utf-8"), filename=str(_DISCOVER)) + module_scope: set[str] = set() + for node in tree.body: + if isinstance(node, ast.Import): + module_scope.update(alias.name for alias in node.names) + elif isinstance(node, ast.ImportFrom): + module_scope.update(alias.name for alias in node.names) + if node.module: + module_scope.add(node.module) + named = sorted(name for name in module_scope if "_ladder" in name) + assert not named, f"discover.py imports {named} at module scope; move it inside a function, after the probe" + + +def test_check_contract_matches_the_platform() -> None: + """Drift guard. Both sides must render the same statuses and severities.""" + spec = importlib.util.spec_from_file_location("_skill_checks", _SCRIPTS_DIR / "_checks.py") + assert spec is not None and spec.loader is not None + skill_checks = importlib.util.module_from_spec(spec) + # ``dataclass`` resolves its module through ``sys.modules``, so register the + # module before executing it. + sys.modules[spec.name] = skill_checks + try: + spec.loader.exec_module(skill_checks) + finally: + sys.modules.pop(spec.name, None) + + assert {skill_checks.PASS, skill_checks.WARN, skill_checks.FAIL} == set(platform_checks.CheckStatus.__args__) + assert {skill_checks.REQUIRED, skill_checks.ADVISORY} == set(platform_checks.CheckSeverity.__args__) + platform_fields = set(platform_checks.CheckResult.model_fields) + skill_fields = set(skill_checks.CheckResult.__dataclass_fields__) + assert platform_fields <= skill_fields, f"the skill dropped platform check fields {platform_fields - skill_fields}" + + +def test_discover_proves_a_valid_suite_runnable(suite: Path) -> None: + code, report = _run_discover(suite) + + assert report["proven"] is True, f"Harbor must judge this report; runtime was {report['runtime']}" + assert report["task_count"] == 1 + assert report["dataset_paths"] == ["dataset"] + assert len(report["configs"]) == 1 + + for name in ("schema", "resolution", "tasks", "coverage", "credentials", "agent"): + check = _named(report, name) + assert check["status"] == "pass", f"{name} failed: {check['message']}" + assert check["proven"] is True + + backend = _named(report, "backend") + if backend["status"] != "pass": + pytest.skip(f"no environment backend available: {backend['message']}") + assert code == 0 + assert report["runnable"] is True + assert report["run_command"] == f"cd {suite} && harbor job start -c harbor-job.yaml" + + +def test_discover_names_the_rung_a_broken_config_fails(suite: Path) -> None: + """A dataset path that does not exist must fail resolution, not schema.""" + (suite / "harbor-job.yaml").write_text( + "job_name: fixture\ndatasets:\n - path: ./no-such-dataset\nagents:\n - name: oracle\n", + encoding="utf-8", + ) + + code, report = _run_discover(suite) + + assert code == 1 + assert report["runnable"] is False + assert _named(report, "schema")["status"] == "pass", "the shape is valid; only the path is wrong" + resolution = _named(report, "resolution") + assert resolution["status"] == "fail" + assert "no-such-dataset" in resolution["message"] + + +def test_discover_names_an_unknown_agent(suite: Path) -> None: + (suite / "harbor-job.yaml").write_text( + "job_name: fixture\ndatasets:\n - path: ./dataset\nagents:\n - name: no-such-agent\n", + encoding="utf-8", + ) + + code, report = _run_discover(suite) + + assert code == 1 + agent = _named(report, "agent") + assert agent["status"] == "fail" + assert "no-such-agent" in agent["message"] + + +def test_discover_reports_required_host_variables(suite: Path) -> None: + task_toml = suite / "dataset" / "task-one" / "task.toml" + task_toml.write_text( + task_toml.read_text(encoding="utf-8") + '\n[environment.env]\nACME_API_KEY = "${ACME_API_KEY}"\n', + encoding="utf-8", + ) + + _, report = _run_discover(suite) + + credentials = _named(report, "credentials") + assert credentials["status"] == "pass" + assert "ACME_API_KEY" in credentials["message"] + assert [item["name"] for item in report["configs"][0]["required_env_vars"]] == ["ACME_API_KEY"] + + +def test_discover_reports_a_task_harbor_silently_dropped(suite: Path) -> None: + """Harbor skips an unparseable task without raising, which coverage must catch.""" + broken = suite / "dataset" / "task-two" + _write_task(broken) + (broken / "task.toml").write_text("this is not valid toml = = =\n", encoding="utf-8") + + code, report = _run_discover(suite) + + assert code == 1 + coverage = _named(report, "coverage") + assert coverage["status"] == "fail" + assert "task-two" in coverage["message"] + + +def test_discover_marks_every_finding_unproven_without_harbor(suite: Path) -> None: + """The promise that keeps an inventory from reading as evidence.""" + code, report = _run_discover(suite, with_harbor=False) + + assert code == 1, "no Harbor means no proof, so discovery cannot report success" + assert report["proven"] is False + assert report["runnable"] is False + assert report["run_command"] is None + assert report["runtime"]["harbor_importable"] is False + + harbor_check = _named(report, "harbor") + assert harbor_check["status"] == "fail" + assert harbor_check["severity"] == "required" + assert harbor_check["hint"], "a missing Harbor must tell the user what to do" + + observed = [check for check in report["checks"] if check["name"] != "harbor"] + assert observed, "an unproven inventory is still worth reporting" + assert all(check["proven"] is False for check in observed), ( + f"these findings claim proof without Harbor: {[c['name'] for c in observed if c['proven']]}" + ) + + +def test_discover_finds_configs_without_pyyaml(suite: Path) -> None: + """Without PyYAML the fallback still finds YAML configs, and says it cannot read them.""" + _, report = _run_discover(suite, with_harbor=False) + + assert [config["path"] for config in report["configs"]] == ["harbor-job.yaml"] + parse = _named(report, "config-parse") + assert parse["status"] == "fail" + assert "harbor-job.yaml" in parse["message"] + + +def test_discover_writes_a_markdown_report_only_where_asked(suite: Path, tmp_path: Path) -> None: + out = tmp_path / "reports" / "discovery.md" + + _run_discover(suite, "--out", str(out)) + + assert out.is_file(), "--out must create the report and its parent directory" + text = out.read_text(encoding="utf-8") + assert text.startswith("# Discovery report for") + assert "Proven by Harbor: yes" in text + + +def test_discover_fails_with_a_hint_when_the_path_is_missing(tmp_path: Path) -> None: + code, report = _run_discover(tmp_path / "nowhere") + assert code == 1 + assert "error" in report + assert report["hint"] From 71ed1034fa06d20ded03262e9409ab62f816bc89 Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Thu, 20 Aug 2026 14:01:56 -0500 Subject: [PATCH 2/8] refactor(eval-author)!: delete the CLI and discovery, leaving only skills Harbor tasks live in the customer's repository, so a CLI that proposes changes has to write to that repository, and customers would not grant that however it was sandboxed. The skills are the replacement: the customer's own agent does the work and nothing gets installed. Removes the nemo agents eval-author command group, its entry point, and the discovery/ package behind discover, along with their tests. The eval-author-discover skill covers the same ground: it probes for an installed Harbor, finds the repository's configs and tasks with the standard library, then has Harbor's own validators judge each one. Dependencies drop to pyyaml and nemo-insights-plugin, both for the contract test, because the bundled scripts import the standard library only. The package still resolves as a namespace package, so root test discovery keeps finding its tests, and nemo agents now lists only analyst and experimentalist. Vendor left the entry point behind pointing at the deleted module, so that line comes out by hand; vendor then reclaims the table and stops rewriting it. Signed-off-by: Alec Khoury --- docs/agents/insight-driven-optimization.mdx | 17 +- packages/nemo_platform/pyproject.toml | 5 - plugins/nemo-eval-author/.env.example | 10 - plugins/nemo-eval-author/README.md | 44 +- plugins/nemo-eval-author/pyproject.toml | 15 +- .../src/nemo_eval_author_plugin/cli.py | 118 ------ .../discovery/report.py | 193 --------- .../nemo_eval_author_plugin/discovery/run.py | 121 ------ .../nemo_eval_author_plugin/discovery/scan.py | 220 ---------- .../discovery/validate.py | 309 -------------- plugins/nemo-eval-author/tests/conftest.py | 20 - .../tests/discover/test_command.py | 285 ------------- .../tests/discover/test_report.py | 211 ---------- .../tests/discover/test_scan.py | 171 -------- .../tests/discover/test_validate.py | 389 ------------------ .../nemo-eval-author/tests/harbor_fixtures.py | 80 ---- plugins/nemo-eval-author/tests/test_cli.py | 36 -- plugins/nemo-experimentalist/AGENTS.md | 13 +- plugins/nemo-experimentalist/README.md | 6 +- .../eval_author/README.md | 5 +- pyproject.toml | 8 +- uv.lock | 27 -- 22 files changed, 52 insertions(+), 2251 deletions(-) delete mode 100644 plugins/nemo-eval-author/.env.example delete mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/cli.py delete mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/report.py delete mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/run.py delete mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/scan.py delete mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/validate.py delete mode 100644 plugins/nemo-eval-author/tests/conftest.py delete mode 100644 plugins/nemo-eval-author/tests/discover/test_command.py delete mode 100644 plugins/nemo-eval-author/tests/discover/test_report.py delete mode 100644 plugins/nemo-eval-author/tests/discover/test_scan.py delete mode 100644 plugins/nemo-eval-author/tests/discover/test_validate.py delete mode 100644 plugins/nemo-eval-author/tests/harbor_fixtures.py delete mode 100644 plugins/nemo-eval-author/tests/test_cli.py diff --git a/docs/agents/insight-driven-optimization.mdx b/docs/agents/insight-driven-optimization.mdx index db17b7c794..f30fea0f24 100644 --- a/docs/agents/insight-driven-optimization.mdx +++ b/docs/agents/insight-driven-optimization.mdx @@ -285,10 +285,9 @@ evaluation suite for validating candidates that address the Insight. The Experimenter invokes the Eval Author workflow in Insight mode and reads the `eval_author` section of the experiment configuration. -The plugin also exposes the canonical `nemo agents eval-author` command -namespace with `discover`, `audit`, `propose`, `run`, and `doctor` verbs. These -standalone verbs are currently scaffolding and exit with a nonzero status until -their implementations are available. +The Eval Author has no command namespace. Work on the evaluation suites in your +own repository runs through the `eval-author` skills, which your agent reads and +follows. See the [Eval Author README](https://github.com/NVIDIA-NeMo/nemo-platform/tree/main/plugins/nemo-eval-author). ## Get Started @@ -325,7 +324,6 @@ Confirm the plugins are installed and discoverable: ```bash nemo agents analyst --help nemo agents experimentalist --help -nemo agents eval-author --help ``` From an agent directory with an `optimizer.yaml` profile, check that the @@ -529,8 +527,7 @@ seconds and submits a run after the scheduled daily or weekly window is reached. ## Command Reference The Analyst lives under `nemo agents analyst`, and the Experimenter lives under -`nemo agents experimentalist`. The Eval Author command surface lives under -`nemo agents eval-author`. These command namespaces share the `optimizer.yaml` +`nemo agents experimentalist`. These command namespaces share the `optimizer.yaml` profile, and each validates its own configuration. Scheduled analysis remains under `nemo insights analysis`. @@ -586,12 +583,6 @@ Run the local Experimenter loop. Diagnose the Experimenter setup: profile, configured models, Insight resolution, datasets, and the experiment plan. -### `nemo agents eval-author` - -Discover the current Eval Author command surface with `--help`. The -`discover`, `audit`, `propose`, `run`, and `doctor` verbs are placeholders and -exit with a nonzero status until their implementations are available. - ### Models The optimization agents use the active Platform CLI context: diff --git a/packages/nemo_platform/pyproject.toml b/packages/nemo_platform/pyproject.toml index bfedc69722..b6de1dd347 100644 --- a/packages/nemo_platform/pyproject.toml +++ b/packages/nemo_platform/pyproject.toml @@ -293,11 +293,7 @@ nemo-deployments-plugin = [ # Generated from [tool.bundle-package]; do not edit by hand. nemo-eval-author-plugin = [ - "pydantic>=2", - "harbor>=0.18", - "nemo-experimentalist-plugin", "nemo-insights-plugin", - "nemo-platform-plugin", "pyyaml>=6.0.3", ] @@ -569,7 +565,6 @@ insights = "nemo_insights_plugin.cli:InsightsCLI" # Generated from [tool.bundle-package]; do not edit this table by hand. [project.entry-points."nemo.cli.agents"] -eval-author = "nemo_eval_author_plugin.cli:EvalAuthorCLI" experimentalist = "nemo_experimentalist_plugin.cli:ExperimentalistCLI" analyst = "nemo_insights_plugin.analyst.cli:AnalystCLI" diff --git a/plugins/nemo-eval-author/.env.example b/plugins/nemo-eval-author/.env.example deleted file mode 100644 index c656ffefab..0000000000 --- a/plugins/nemo-eval-author/.env.example +++ /dev/null @@ -1,10 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -# -# Run `nemo setup` to register a provider and select default and fast models. -# The stored active context is used by default. These environment variables are -# only needed to override it in a non-interactive or isolated environment. -# NMP_BASE_URL=http://localhost:8080 -# NEMO_DEFAULT_MODEL=default/gpt-4-1 -# NEMO_FAST_MODEL=default/gpt-4-1-mini diff --git a/plugins/nemo-eval-author/README.md b/plugins/nemo-eval-author/README.md index f37349fde1..69235ae5db 100644 --- a/plugins/nemo-eval-author/README.md +++ b/plugins/nemo-eval-author/README.md @@ -1,26 +1,36 @@ -# NeMo Eval Author Plugin +# NeMo Eval Author -Owns the `nemo agents eval-author` command group, registered under `nemo.cli.agents` and -mounted by the agents plugin. `discover` is implemented; `audit`, `propose`, `run`, and -`doctor` are placeholders. +Two skills that an agent reads to work on the evaluation suites in a user's own +repository. There is no CLI and no service. A customer points their agent at +`skills/` and nothing gets installed. -Use `discover` only with a trusted repository, because importing an agent runs -module top-level code. +| Skill | Role | +| --- | --- | +| [`eval-author`](src/nemo_eval_author_plugin/skills/eval-author/SKILL.md) | Core. Owns the standard every sub-flow follows and routes to one. | +| [`eval-author-discover`](src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md) | Sub-flow. Records whether a repository's Harbor evals are ready to run. | -The Eval Author agent moved into the Experimentalist plugin, at -[`nemo_experimentalist_plugin.eval_author`](../nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md). -Experimentalist insight mode is its only caller, so the agent sits beside the evaluator, -staging, and trace helpers it depends on. +## Why skills instead of an agent -## Direction of travel +Harbor tasks live in the customer's repository, so an agent that proposes changes +has to write to that repository. Customers were unwilling to grant that, sandboxed +or not. A skill inverts the arrangement: the customer's own agent does the work, +and this package only supplies the instructions and the deterministic scripts. -The dependency is one arrow. `discovery/run.py` borrows `make_client` from Experimentalist, -and Experimentalist imports nothing from here, so there is no package cycle for `uv` to -resolve. Install both plugins with: +The Eval Author agent that Experimentalist insight mode still uses lives in +[the Experimentalist plugin](../nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md). -```bash -uv sync --group experimentalist -``` +## Dependencies + +The scripts under `skills/*/scripts/` import the standard library only, so they run +on whatever Python the customer already has. Where a real answer needs a provider, +the skill defers to the provider's own validators rather than guessing from file +layout, which is why `eval-author-discover` probes for an installed Harbor and asks +Harbor to judge each config. + +The two declared dependencies serve `tests/test_skill_contract.py`, which reads the +skills with `pyyaml` and checks them against the platform's check helpers. Adding a +runtime dependency to a bundled script is a breaking change for anyone who copied +the skill, so the contract test guards against it. diff --git a/plugins/nemo-eval-author/pyproject.toml b/plugins/nemo-eval-author/pyproject.toml index 10644face2..b5a5055722 100644 --- a/plugins/nemo-eval-author/pyproject.toml +++ b/plugins/nemo-eval-author/pyproject.toml @@ -4,22 +4,16 @@ [project] name = "nemo-eval-author-plugin" version = "0.1.0" -description = "Eval Author commands for NeMo Platform (borrows the Experimentalist platform client)." +description = "Eval Author skills that an agent reads to discover a repository's Harbor eval setup." requires-python = ">=3.12,<3.14" +# The bundled skill scripts run on the standard library alone, so a customer needs +# no install to use them. These two are for the contract test: it reads the skill +# with pyyaml and checks it against the platform's own check helpers. dependencies = [ - "pydantic>=2", - # Harbor 0.18 provides the discovery APIs used by this plugin. - "harbor>=0.18", - "nemo-experimentalist-plugin", "nemo-insights-plugin", - "nemo-platform", - "nemo-platform-plugin", "pyyaml>=6.0.3", ] -[project.entry-points."nemo.cli.agents"] -eval-author = "nemo_eval_author_plugin.cli:EvalAuthorCLI" - [build-system] requires = ["hatchling"] build-backend = "hatchling.build" @@ -28,6 +22,5 @@ build-backend = "hatchling.build" packages = ["src/nemo_eval_author_plugin"] [tool.pytest.ini_options] -asyncio_mode = "auto" pythonpath = ["src"] testpaths = ["tests"] 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 deleted file mode 100644 index 5f5db9b429..0000000000 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/cli.py +++ /dev/null @@ -1,118 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Eval Author commands under ``nemo agents eval-author``.""" - -import asyncio -from pathlib import Path -from typing import Annotated, ClassVar, NoReturn - -import typer -from nemo_eval_author_plugin.discovery import run as discovery -from nemo_insights_plugin.contracts.checks import format_report -from nemo_platform_plugin.cli import NemoCLI - - -def _not_implemented(ctx: typer.Context, ticket: str) -> NoReturn: - typer.echo(f"`{ctx.command_path}` is not implemented yet ({ticket}).", err=True) - raise typer.Exit(code=1) - - -def _report_discovery(result: discovery.DiscoverResult) -> None: - status = format_report(result.report.checks) - if status: - typer.echo(status) - if result.report.run_command: - typer.echo("") - typer.echo(f"Run: {result.report.run_command}") - - typer.echo("") - if result.dry_run: - typer.echo("Dry run: no files were uploaded.") - typer.echo("") - typer.echo(result.markdown, nl=False) - elif result.uploaded: - remote_path = f"{result.report.agent}/{discovery.REPORT_FILENAME}" - typer.echo(f"Uploaded {remote_path} to fileset '{discovery.FILESET_NAME}'.") - else: - typer.echo(f"Upload failed: {result.upload_error or 'unknown error'}", err=True) - - failures = sum(check.status == "fail" for check in result.report.checks) - failures += not result.dry_run and not result.uploaded - warnings = sum(check.status == "warn" for check in result.report.checks) - failure_label = "failure" if failures == 1 else "failures" - warning_label = "warning" if warnings == 1 else "warnings" - status = "passed" if result.ok else "failed" - typer.echo(f"Final overview: Discovery {status} with {failures} {failure_label} and {warnings} {warning_label}.") - - -class EvalAuthorCLI(NemoCLI): - """``nemo agents eval-author ...`` subcommands.""" - - name: ClassVar[str] = "eval-author" - description: ClassVar[str] = "NeMo Eval Author commands." - - def get_cli(self) -> typer.Typer: - app = typer.Typer(help=self.description, no_args_is_help=True) - - @app.callback() - def _root() -> None: - """Select an Eval Author command.""" - - @app.command("discover") - def discover( - repo: Annotated[ - Path, - typer.Option("--repo", help="Repository that contains the agent.", exists=True, file_okay=False), - ] = Path(), - agent: Annotated[ - str | None, - typer.Option("--agent", help="Agent name. The default comes from optimizer.yaml or the directory."), - ] = None, - dry_run: Annotated[ - bool, - typer.Option("--dry-run", help="Print discovery.md without an upload."), - ] = False, - ) -> None: - """Inspect the repository and record its Harbor preflight. - - WARNING: Use this command only with a trusted repository. - Agent imports execute module top-level code. - """ - result = asyncio.run( - discovery.discover( - discovery.DiscoverOptions( - repo_root=repo, - agent=agent, - dry_run=dry_run, - ) - ) - ) - _report_discovery(result) - raise typer.Exit(code=0 if result.ok else 1) - - @app.command("audit") - def audit(ctx: typer.Context) -> None: - """Report coverage gaps in an existing eval suite.""" - # TODO(ASE-676): declare flags and wire the audit. - _not_implemented(ctx, "ASE-676") - - @app.command("propose") - def propose(ctx: typer.Context) -> None: - """Propose eval suite additions for review.""" - # TODO(ASE-675): declare flags and wire the proposal. - _not_implemented(ctx, "ASE-675") - - @app.command("run") - def run(ctx: typer.Context) -> None: - """Run the Eval Author pipeline.""" - # TODO(ASE-673): declare flags and wire the pipeline to run_eval_author. - _not_implemented(ctx, "ASE-673") - - @app.command("doctor") - def doctor(ctx: typer.Context) -> None: - """Diagnose credentials, platform access, and the runtime.""" - # TODO(ASE-678): report the prerequisites the other verbs gate on. - _not_implemented(ctx, "ASE-678") - - return app diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/report.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/report.py deleted file mode 100644 index a241c89101..0000000000 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/report.py +++ /dev/null @@ -1,193 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Discovery report contract and Markdown renderer.""" - -import shlex -from dataclasses import dataclass, field -from datetime import UTC, datetime -from importlib.metadata import version -from pathlib import Path -from typing import Any - -import yaml -from nemo_eval_author_plugin.discovery.scan import RepositoryScan -from nemo_eval_author_plugin.discovery.validate import RequiredEnvVar, ValidationOutcome -from nemo_insights_plugin.contracts.checks import CheckResult, format_report, required_failures - - -@dataclass -class ConfigReport: - """Preflight results for one Harbor config.""" - - name: str - path: Path - required_env_vars: list[RequiredEnvVar] - checks: list[CheckResult] - - @property - def runnable(self) -> bool: - """Return whether the config passed all required checks.""" - return not required_failures(self.checks) - - -@dataclass -class DiscoveryReport: - """Repository facts and current Harbor preflight results.""" - - agent: str - workspace: str - repo_root: Path - configs: list[ConfigReport] - dataset_paths: list[Path] - ethos_path: str | None - harbor_version: str - discovered_at: datetime - fingerprint: str - input_file_count: int - repository_checks: list[CheckResult] - trace_check: CheckResult - schema_version: int = field(init=False, default=1) - - @property - def runnable(self) -> bool: - """Return whether all repository-owned configs passed required checks.""" - return bool(self.configs) and all(config.runnable for config in self.configs) - - @property - def checks(self) -> list[CheckResult]: - """Return repository, config, and trace checks in execution order.""" - checks = list(self.repository_checks) - for config in self.configs: - checks.extend(config.checks) - checks.append(self.trace_check) - return checks - - @property - def run_command(self) -> str | None: - """Return the Harbor command only for one runnable config.""" - if len(self.configs) != 1: - return None - return self.run_command_for(self.configs[0]) - - def run_command_for(self, config: ConfigReport) -> str | None: - """Return the Harbor command for one runnable config.""" - if not config.runnable: - return None - config_path = config.path.resolve().relative_to(self.repo_root.resolve()).as_posix() - cd_command = shlex.join(["cd", str(self.repo_root.resolve())]) - harbor_command = shlex.join(["harbor", "job", "start", "-c", config_path]) - return f"{cd_command} && {harbor_command}" - - -def harbor_version() -> str: - """Return the installed Harbor version.""" - return version("harbor") - - -def build_report( - *, - agent: str, - workspace: str, - repo_root: Path, - scan_result: RepositoryScan, - validations: list[ValidationOutcome], - trace_check: CheckResult, - discovered_at: datetime | None = None, -) -> DiscoveryReport: - """Build one report from the repository scan and config preflights.""" - configs = [ - ConfigReport( - name=candidate.name, - path=candidate.path, - required_env_vars=validation.required_env_vars, - checks=validation.checks, - ) - for candidate, validation in zip(scan_result.configs, validations, strict=True) - ] - return DiscoveryReport( - agent=agent, - workspace=workspace, - repo_root=repo_root.resolve(), - configs=configs, - dataset_paths=scan_result.dataset_paths, - ethos_path=scan_result.ethos_path, - harbor_version=harbor_version(), - discovered_at=discovered_at or datetime.now(UTC), - fingerprint=f"sha256:{scan_result.fingerprint}", - input_file_count=scan_result.input_file_count, - repository_checks=scan_result.checks, - trace_check=trace_check, - ) - - -def render_markdown(report: DiscoveryReport) -> str: - """Render YAML front matter and a concise status report.""" - front = yaml.safe_dump(_front_matter(report), sort_keys=False, default_flow_style=False).rstrip() - lines = [f"# Discovery report for `{report.agent}`"] - status = format_report([*report.repository_checks, report.trace_check]) - if status: - lines.extend(["", "```text", status, "```"]) - if report.configs: - lines.extend(["", "## Harbor entrypoints"]) - for config in report.configs: - path = _display_path(config.path, report.repo_root) - lines.extend( - [ - "", - f"### `{config.name}` (`{path}`)", - "", - f"Runnable: {'true' if config.runnable else 'false'}", - "", - "```text", - format_report(config.checks), - "```", - ] - ) - if command := report.run_command_for(config): - lines.extend(["", "```bash", command, "```"]) - body = "\n".join(lines) - return f"---\n{front}\n---\n\n{body}\n" - - -def _front_matter(report: DiscoveryReport) -> dict[str, Any]: - config = report.configs[0] if len(report.configs) == 1 else None - return { - "schema_version": report.schema_version, - "agent": report.agent, - "workspace": report.workspace, - "repo_root": str(report.repo_root), - "runnable": report.runnable, - "configs": [ - {"name": config.name, "path": _display_path(config.path, report.repo_root)} for config in report.configs - ], - "config_path": _display_path(config.path if config is not None else None, report.repo_root), - "dataset_paths": [_display_path(path, report.repo_root) for path in report.dataset_paths], - "run_command": report.run_command, - "ethos_path": report.ethos_path, - "harbor_version": report.harbor_version, - "required_env_vars": [ - { - "name": item.name, - "default": item.default, - "declared_in": _display_path(item.declared_in, report.repo_root), - } - for config in report.configs - for item in config.required_env_vars - ], - "discovered_at": report.discovered_at.isoformat(), - "fingerprint": report.fingerprint, - "input_file_count": report.input_file_count, - "checks": [check.model_dump(mode="json") for check in report.checks], - } - - -def _display_path(path: Path | None, repo_root: Path) -> str | None: - if path is None: - return None - if not path.is_absolute(): - return path.as_posix() - try: - return path.resolve().relative_to(repo_root.resolve()).as_posix() - except ValueError: - return path.as_posix() diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/run.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/run.py deleted file mode 100644 index 016c1e0401..0000000000 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/run.py +++ /dev/null @@ -1,121 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Orchestration for ``nemo agents eval-author discover``.""" - -import re -from dataclasses import dataclass -from pathlib import Path - -import yaml -from nemo_eval_author_plugin.discovery import report, scan, validate -from nemo_experimentalist_plugin.client import make_client -from nemo_platform import AsyncNeMoPlatform -from nemo_platform.config.config import Config - -FILESET_NAME = "nemo-eval-author" -REPORT_FILENAME = "discovery.md" -_SLUG_PATTERN = re.compile(r"[^a-z0-9]+") - - -@dataclass -class DiscoverOptions: - """Resolved command options.""" - - repo_root: Path - agent: str | None = None - dry_run: bool = False - - -@dataclass -class DiscoverResult: - """The report and upload result for one invocation.""" - - report: report.DiscoveryReport - markdown: str - uploaded: bool = False - dry_run: bool = False - upload_error: str | None = None - - @property - def ok(self) -> bool: - """Return the command exit condition.""" - return self.report.runnable and (self.dry_run or self.uploaded) - - -async def discover(options: DiscoverOptions) -> DiscoverResult: - """Scan, validate, report, and optionally upload one repository.""" - repo_root = options.repo_root.resolve() - agent = _slug(options.agent) if options.agent is not None else _infer_agent_name(repo_root) - workspace = _active_workspace() - client = make_client(None) - try: - return await _discover(client, options, repo_root=repo_root, agent=agent, workspace=workspace) - finally: - await client.close() - - -async def _discover( - client: AsyncNeMoPlatform, - options: DiscoverOptions, - *, - repo_root: Path, - agent: str, - workspace: str, -) -> DiscoverResult: - ref = f"{workspace}/{agent}-spec#AGENT-SPEC.md" - try: - platform_ethos = (ref, await client.files.download_content(remote_path=ref)) - except Exception: - platform_ethos = None - scan_result = scan.scan_repository(repo_root, platform_ethos=platform_ethos) - validations = [await validate.run_ladder(config, repo_root) for config in scan_result.configs] - trace_check = await scan.probe_traces(client, agent=agent, workspace=workspace) - record = report.build_report( - agent=agent, - workspace=workspace, - repo_root=repo_root, - scan_result=scan_result, - validations=validations, - trace_check=trace_check, - ) - markdown = report.render_markdown(record) - result = DiscoverResult(report=record, markdown=markdown, dry_run=options.dry_run) - if options.dry_run: - return result - - try: - await client.files.upload_content( - content=markdown.encode("utf-8"), - remote_path=f"{agent}/{REPORT_FILENAME}", - fileset=FILESET_NAME, - workspace=workspace, - fileset_auto_create=True, - ) - except Exception as exc: - result.upload_error = f"{type(exc).__name__}: {exc}" - else: - result.uploaded = True - return result - - -def _active_workspace() -> str: - return Config.load().resolve().workspace - - -def _infer_agent_name(repo_root: Path) -> str: - """Read a root profile name, or use the repository directory.""" - try: - data = yaml.safe_load((repo_root / "optimizer.yaml").read_text(encoding="utf-8")) - except (OSError, UnicodeError, yaml.YAMLError): - data = None - declared = data.get("agent") if isinstance(data, dict) else None - return _slug(declared) if isinstance(declared, str) and declared.strip() else _slug(repo_root.name) - - -def _slug(value: str) -> str: - slug = _SLUG_PATTERN.sub("-", value.strip().lower()).strip("-") - return slug or "agent" - - -__all__ = ["DiscoverOptions", "DiscoverResult", "discover"] diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/scan.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/scan.py deleted file mode 100644 index be90dcc310..0000000000 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/scan.py +++ /dev/null @@ -1,220 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Scan repository-owned Harbor inputs.""" - -import hashlib -import json -import os -from collections.abc import Iterator -from dataclasses import dataclass -from pathlib import Path -from typing import Any - -import yaml -from nemo_insights_plugin.contracts.checks import CheckResult, CheckSeverity, CheckStatus - -_CONFIG_SUFFIXES = (".yaml", ".yml", ".json") -_MAX_CONFIG_DEPTH = 4 -_PRUNE_DIR_NAMES = frozenset( - { - ".git", - ".venv", - "venv", - "node_modules", - "__pycache__", - ".ruff_cache", - ".pytest_cache", - ".mypy_cache", - ".tox", - ".eggs", - ".cache", - "site-packages", - "vendor", - "cache", - "dist", - "build", - "eval-and-optimize", - ".nemo-optimizer", - "jobs", - } -) - - -@dataclass(frozen=True) -class ConfigCandidate: - """A repository-owned Harbor config file.""" - - path: Path - data: dict[str, Any] - - @property - def name(self) -> str: - """Return the declared job name or the file name.""" - job_name = self.data.get("job_name") - return job_name.strip() if isinstance(job_name, str) and job_name.strip() else self.path.name - - -@dataclass -class RepositoryScan: - """The repository facts that the validation ladder needs.""" - - configs: list[ConfigCandidate] - dataset_paths: list[Path] - ethos_path: str | None - fingerprint: str - input_file_count: int - checks: list[CheckResult] - - -def _check( - name: str, - status: CheckStatus, - message: str, - *, - severity: CheckSeverity = "required", - hint: str | None = None, -) -> CheckResult: - return CheckResult(name=name, group="repository", status=status, severity=severity, message=message, hint=hint) - - -def walk_dirs(root: Path, *, max_depth: int | None = None) -> Iterator[Path]: - """Yield repository directories and skip generated trees.""" - for current, dir_names, _ in os.walk(root): - directory = Path(current) - depth = len(directory.relative_to(root).parts) - dir_names[:] = sorted( - name for name in dir_names if name not in _PRUNE_DIR_NAMES and (max_depth is None or depth < max_depth) - ) - yield directory - - -def scan_repository(repo_root: Path, *, platform_ethos: tuple[str, bytes] | None = None) -> RepositoryScan: - """Find repo-owned configs and local Harbor datasets.""" - repo_root = repo_root.resolve() - configs = _config_candidates(repo_root) - checks: list[CheckResult] = [] - if not configs: - checks.append( - _check( - "config", - "fail", - "No repository-owned Harbor config file exists.", - hint="Add a YAML, YML, or JSON config with a nonempty datasets or tasks list.", - ) - ) - else: - count = len(configs) - checks.append( - _check("config", "pass", f"Found {count} repository-owned Harbor config file{'s' if count != 1 else ''}.") - ) - - ethos = platform_ethos - if ethos is None and (repo_root / "ETHOS.md").is_file(): - ethos = ("ETHOS.md", (repo_root / "ETHOS.md").read_bytes()) - if ethos is not None: - checks.append(_check("ethos", "pass", f"{ethos[0]} defines the agent doctrine.", severity="advisory")) - else: - checks.append( - _check( - "ethos", - "warn", - "ETHOS.md does not exist at the repository root.", - severity="advisory", - hint="Add ETHOS.md to define the agent doctrine.", - ) - ) - - datasets = _dataset_paths(repo_root) - fingerprint, count = _fingerprint(repo_root, [config.path for config in configs], ethos, datasets) - return RepositoryScan(configs, datasets, ethos[0] if ethos else None, fingerprint, count, checks) - - -def _config_candidates(repo_root: Path) -> list[ConfigCandidate]: - candidates: list[ConfigCandidate] = [] - for directory in walk_dirs(repo_root, max_depth=_MAX_CONFIG_DEPTH): - for path in sorted(directory.iterdir()): - if path.is_symlink() or not path.is_file() or path.suffix.lower() not in _CONFIG_SUFFIXES: - continue - data = _load_mapping(path) - if data is not None and _has_work(data): - candidates.append(ConfigCandidate(path=path, data=data)) - return sorted( - candidates, - key=lambda candidate: ( - len(candidate.path.relative_to(repo_root).parts) - 1, - candidate.path.relative_to(repo_root).as_posix(), - ), - ) - - -def _load_mapping(path: Path) -> dict[str, Any] | None: - try: - text = path.read_text(encoding="utf-8") - data = json.loads(text) if path.suffix.lower() == ".json" else yaml.safe_load(text) - except (OSError, UnicodeError, json.JSONDecodeError, yaml.YAMLError): - return None - return data if isinstance(data, dict) else None - - -def _has_work(data: dict[str, Any]) -> bool: - return any(isinstance(data.get(name), list) and data[name] for name in ("datasets", "tasks")) - - -def _dataset_paths(repo_root: Path) -> list[Path]: - datasets: set[Path] = set() - for directory in walk_dirs(repo_root): - if directory != repo_root and directory.name != "task_template" and (directory / "task.toml").is_file(): - datasets.add(directory.parent) - return sorted(datasets) - - -def _fingerprint( - repo_root: Path, - config_paths: list[Path], - ethos: tuple[str, bytes] | None, - datasets: list[Path], -) -> tuple[str, int]: - files = {path for path in [*config_paths, repo_root / "optimizer.yaml"] if path.is_file()} - for dataset in datasets: - if not dataset.is_relative_to(repo_root): - continue - for directory in walk_dirs(dataset): - files.update( - path for path in directory.iterdir() if path.is_file() and path.resolve().is_relative_to(repo_root) - ) - files.discard(repo_root / "ETHOS.md") - - digest = hashlib.sha256() - for path in sorted(files): - digest.update(str(path.relative_to(repo_root)).encode()) - digest.update(b"\0") - digest.update(path.read_bytes()) - digest.update(b"\0") - if ethos is not None: - digest.update(ethos[0].encode() + b"\0" + ethos[1] + b"\0") - return digest.hexdigest(), len(files) + (ethos is not None) - - -async def probe_traces(client: Any, *, agent: str, workspace: str) -> CheckResult: - """Check whether Intake has traces for later authoring steps.""" - try: - page = await client.intake.spans.groups.list( - workspace=workspace, - by="session_id", - page=1, - page_size=1, - filter={"agent_name": agent}, - sort="-span_count", - ) - except Exception as exc: - return _check( - "traces", - "warn", - f"Cannot read traces for {agent}: {type(exc).__name__}: {exc}", - severity="advisory", - ) - total = page.pagination.total_results if page.pagination is not None else len(page.data) - if not total: - return _check("traces", "warn", f"No traces exist for {agent}.", severity="advisory") - return _check("traces", "pass", f"{total} trace sessions exist for {agent}.", severity="advisory") diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/validate.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/validate.py deleted file mode 100644 index 3761c8e907..0000000000 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/discovery/validate.py +++ /dev/null @@ -1,309 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Run Harbor preflight checks against a repository-owned config.""" - -import contextlib -import shutil -import subprocess -import sys -import tempfile -import tomllib -from collections.abc import Iterator -from dataclasses import dataclass, field -from fnmatch import fnmatchcase -from pathlib import Path - -from harbor.agents.factory import AgentFactory -from harbor.environments.factory import EnvironmentFactory -from harbor.job import Job -from harbor.models.agent.name import AgentName -from harbor.models.job.config import JobConfig -from harbor.models.task.config import TaskConfig -from harbor.models.task.paths import TaskPaths -from harbor.models.task.task import Task -from harbor.utils.env import get_required_host_vars -from harbor.utils.import_path import import_class -from nemo_eval_author_plugin.discovery.scan import ConfigCandidate -from nemo_insights_plugin.contracts.checks import CheckResult, CheckSeverity, CheckStatus -from pydantic import ValidationError - - -@dataclass -class RequiredEnvVar: - """A host variable required by a Harbor config.""" - - name: str - default: str | None - declared_in: Path - - -@dataclass -class ValidationOutcome: - """Results from one Harbor preflight.""" - - checks: list[CheckResult] = field(default_factory=list) - required_env_vars: list[RequiredEnvVar] = field(default_factory=list) - - -def _check( - name: str, - status: CheckStatus, - message: str, - *, - severity: CheckSeverity = "required", - hint: str | None = None, -) -> CheckResult: - return CheckResult(name=name, group="validation", status=status, severity=severity, message=message, hint=hint) - - -async def run_ladder(candidate: ConfigCandidate, repo_root: Path) -> ValidationOutcome: - """Run the complete preflight without caching or skipping any check.""" - outcome = ValidationOutcome() - with contextlib.chdir(repo_root): - try: - config = JobConfig.model_validate(candidate.data) - config.validate_agent_concurrency_limits() - except ValidationError as exc: - errors = exc.errors(include_url=False, include_input=False) - outcome.checks.append(_check("schema", "fail", f"Harbor rejected the job config: {errors}")) - return outcome - except ValueError as exc: - outcome.checks.append(_check("schema", "fail", f"Harbor rejected the job config: {exc}")) - return outcome - outcome.checks.append(_check("schema", "pass", "Harbor accepts the job config schema.")) - - job = await _resolve(config, outcome) - _check_agent(config, outcome) - _check_backend(config, outcome) - outcome.checks.append(check_config_file(candidate.path, repo_root)) - if job is None: - return outcome - - resolved = _resolved_task_paths(job) - if resolved is None: - outcome.checks.append( - _check( - "compatibility", - "fail", - "This Harbor version does not expose Job._task_configs.", - hint="Install a Harbor version that exposes the resolved task list.", - ) - ) - return outcome - task_dirs = _check_tasks(resolved, outcome) - _check_coverage(config, resolved, outcome) - _check_required_env_vars(config, task_dirs, outcome) - return outcome - - -async def _resolve(config: JobConfig, outcome: ValidationOutcome) -> Job | None: - try: - with tempfile.TemporaryDirectory(prefix="eval-author-jobs-") as scratch: - job = await Job.create(config.model_copy(update={"jobs_dir": Path(scratch)})) - job._close_logger_handlers() - except Exception as exc: - outcome.checks.append( - _check( - "resolution", - "fail", - f"Harbor could not resolve the job: {type(exc).__name__}: {exc}", - hint="This error occurs before Harbor starts a container.", - ) - ) - return None - outcome.checks.append(_check("resolution", "pass", "Harbor resolved the job.")) - return job - - -def _resolved_task_paths(job: Job) -> list[Path] | None: - task_configs = getattr(job, "_task_configs", None) - if task_configs is None: - return None - paths: list[Path] = [] - for task_config in task_configs: - try: - paths.append(task_config.get_local_path().resolve()) - except ValueError: - continue - return paths - - -def _check_tasks(resolved: list[Path], outcome: ValidationOutcome) -> list[Path]: - valid = [path for path in resolved if Task.is_valid_dir(path)] - if not resolved: - outcome.checks.append(_check("tasks", "fail", "The config resolves to zero tasks.")) - return [] - outcome.checks.append( - _check( - "tasks", - "fail" if len(valid) != len(resolved) else "pass", - f"{len(valid)} of {len(resolved)} task dirs are valid Harbor tasks.", - ) - ) - return valid - - -def _check_coverage(config: JobConfig, resolved: list[Path], outcome: ValidationOutcome) -> None: - resolved_set = {path.resolve() for path in resolved} - dropped_any = False - for dataset in config.datasets: - if dataset.path is None or not dataset.path.is_dir(): - continue - on_disk = [ - child - for child in sorted(dataset.path.iterdir()) - if child.is_dir() and child.name != "task_template" and (child / "task.toml").is_file() - ] - dropped = [child for child in on_disk if child.resolve() not in resolved_set] - if not dropped: - continue - dropped_any = True - selected_dropped = [ - path - for path in dropped - if (not dataset.task_names or any(fnmatchcase(path.name, pattern) for pattern in dataset.task_names)) - and not any(fnmatchcase(path.name, pattern) for pattern in dataset.exclude_task_names or []) - ] - required = bool(selected_dropped) and dataset.n_tasks is None - filtered = bool(dataset.task_names or dataset.exclude_task_names) - reported = selected_dropped if required else dropped - names = ", ".join(path.name for path in reported) - outcome.checks.append( - _check( - "coverage", - "fail" if required else "warn", - f"Harbor did not resolve {len(reported)} task dirs: {names}.", - severity="required" if required else "advisory", - hint=( - "Harbor skipped a task selected by the dataset filters." - if required and filtered - else "Harbor skips these task dirs silently." - if required - else "The dataset filters or n_tasks select a task subset." - ), - ) - ) - if not dropped_any: - outcome.checks.append(_check("coverage", "pass", "Harbor dropped no local task dirs.")) - - -def _check_required_env_vars(config: JobConfig, task_dirs: list[Path], outcome: ValidationOutcome) -> None: - required: dict[str, RequiredEnvVar] = {} - - def collect(env: dict[str, str], declared_in: Path) -> None: - for name, default in get_required_host_vars(env): - required.setdefault(name, RequiredEnvVar(name, default, declared_in)) - - for task_dir in task_dirs: - task_config = _task_config(task_dir) - if task_config is None: - continue - path = TaskPaths(task_dir).config_path - collect(task_config.environment.env, path) - collect(task_config.verifier.env, path) - collect(task_config.solution.env, path) - collect(config.environment.env, Path("")) - collect(config.verifier.env, Path("")) - for agent in config.agents: - collect(agent.env, Path("")) - outcome.required_env_vars = sorted(required.values(), key=lambda item: item.name) - names = ", ".join(item.name for item in outcome.required_env_vars) - outcome.checks.append( - _check("credentials", "pass", f"{len(required)} host variables required" + (f": {names}." if names else ".")) - ) - - -def _task_config(task_dir: Path) -> TaskConfig | None: - try: - return TaskConfig.model_validate_toml(TaskPaths(task_dir).config_path.read_text(encoding="utf-8")) - except (OSError, UnicodeError, tomllib.TOMLDecodeError, ValidationError): - return None - - -def _check_agent(config: JobConfig, outcome: ValidationOutcome) -> None: - for agent in config.agents: - if agent.import_path is not None: - with _evict_module_tree(agent.import_path): - try: - imported = import_class(agent.import_path, label="agent") - except (Exception, SystemExit) as exc: - outcome.checks.append( - _check("agent", "fail", f"Cannot import agent {agent.import_path}: {type(exc).__name__}: {exc}") - ) - else: - outcome.checks.append(_check("agent", "pass", f"Agent {imported.__name__} imports as a class.")) - elif agent.name is not None: - try: - AgentFactory.get_agent_class(AgentName(agent.name)) - except Exception as exc: - outcome.checks.append( - _check("agent", "fail", f"Cannot load Harbor agent {agent.name}: {type(exc).__name__}: {exc}") - ) - else: - outcome.checks.append(_check("agent", "pass", f"Built-in agent {agent.name} is available.")) - - -def _check_backend(config: JobConfig, outcome: ValidationOutcome) -> None: - label = config.environment.import_path or (config.environment.type.value if config.environment.type else "docker") - try: - EnvironmentFactory.run_preflight(config.environment.type, config.environment.import_path) - except (Exception, SystemExit) as exc: - outcome.checks.append( - _check("backend", "fail", f"Environment backend {label} is not ready: {type(exc).__name__}: {exc}") - ) - else: - outcome.checks.append(_check("backend", "pass", f"Environment backend {label} passed preflight.")) - - -def check_config_file(config_path: Path, repo_root: Path) -> CheckResult: - """Check the bytes that Harbor receives from its CLI.""" - harbor = _harbor_executable() - if harbor is None: - return _check( - "round-trip", - "warn", - "The Harbor CLI round trip did not run.", - severity="advisory", - hint="No harbor executable exists on PATH.", - ) - try: - completed = subprocess.run( - [harbor, "job", "start", "--print-config", "-c", str(config_path)], - cwd=repo_root, - capture_output=True, - text=True, - timeout=120, - check=False, - ) - except (OSError, subprocess.SubprocessError) as exc: - return _check("round-trip", "warn", f"The Harbor CLI round trip failed: {exc}", severity="advisory") - if completed.returncode: - detail = (completed.stderr or completed.stdout).strip().splitlines() - return _check( - "round-trip", "fail", f"The Harbor CLI rejected the config: {detail[-1] if detail else 'no output'}." - ) - return _check("round-trip", "pass", "The config file loads through the Harbor CLI.") - - -def _harbor_executable() -> str | None: - local = Path(sys.executable).parent / "harbor" - return str(local) if local.is_file() else shutil.which("harbor") - - -@contextlib.contextmanager -def _evict_module_tree(import_path: str) -> Iterator[None]: - """Import without cached modules from another repository.""" - module = import_path.split(":", 1)[0].split(".", 1)[0] - previous = { - name: cached for name, cached in list(sys.modules.items()) if name == module or name.startswith(f"{module}.") - } - for name in previous: - sys.modules.pop(name) - try: - yield - finally: - for name in list(sys.modules): - if name == module or name.startswith(f"{module}."): - sys.modules.pop(name) - sys.modules.update(previous) diff --git a/plugins/nemo-eval-author/tests/conftest.py b/plugins/nemo-eval-author/tests/conftest.py deleted file mode 100644 index 029bbf49b1..0000000000 --- a/plugins/nemo-eval-author/tests/conftest.py +++ /dev/null @@ -1,20 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Eval Author test-wide state isolation.""" - -import os - -import litellm -import pytest - -litellm.drop_params = True - - -@pytest.fixture(autouse=True) -def _restore_environ(): - """Undo environment changes that monkeypatch cannot restore.""" - snapshot = os.environ.copy() - yield - os.environ.clear() - os.environ.update(snapshot) diff --git a/plugins/nemo-eval-author/tests/discover/test_command.py b/plugins/nemo-eval-author/tests/discover/test_command.py deleted file mode 100644 index 74f8abea1e..0000000000 --- a/plugins/nemo-eval-author/tests/discover/test_command.py +++ /dev/null @@ -1,285 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""End-to-end tests for the discovery command.""" - -import asyncio -from pathlib import Path -from unittest.mock import AsyncMock - -import pytest -import typer -from harbor_fixtures import StubClient, StubFiles, read_front_matter, write_dataset, write_job_config -from nemo_eval_author_plugin import cli -from nemo_eval_author_plugin.discovery import run as discovery -from nemo_eval_author_plugin.discovery import validate -from typer.testing import CliRunner - -runner = CliRunner() -AGENT = "ticket-triage" - - -@pytest.fixture -def app() -> typer.Typer: - return cli.EvalAuthorCLI().get_cli() - - -@pytest.fixture -def client(monkeypatch) -> StubClient: - stub = StubClient() - monkeypatch.setattr(discovery, "make_client", lambda base_url: stub) - return stub - - -@pytest.fixture(autouse=True) -def workspace(monkeypatch, tmp_path): - config = tmp_path / "nmp-config.yaml" - config.touch() - monkeypatch.setenv("NMP_WORKSPACE", "default") - monkeypatch.setenv("NMP_CONFIG_FILE", str(config)) - - -@pytest.fixture(autouse=True) -def successful_external_preflight(monkeypatch): - monkeypatch.setattr(validate.EnvironmentFactory, "run_preflight", lambda *args: None) - monkeypatch.setattr( - validate, - "check_config_file", - lambda path, root: validate._check("round-trip", "pass", "The config file loads through the Harbor CLI."), - ) - - -def _invoke(app: typer.Typer, repo: Path, *extra: str): - return runner.invoke(app, ["discover", "--repo", str(repo), *extra]) - - -def _healthy_repo(root: Path) -> Path: - write_dataset(root / "evals" / "validation") - write_job_config(root / "configs" / "eval.yaml", dataset="evals/validation") - return root - - -def _snapshot(root: Path) -> list[tuple[str, bytes | None]]: - return [ - (path.relative_to(root).as_posix(), path.read_bytes() if path.is_file() else None) - for path in sorted(root.rglob("*")) - ] - - -def test_command_reads_the_canonical_platform_agent_spec(app, client, monkeypatch, tmp_path): - repo = _healthy_repo(tmp_path / "agent-repo") - (repo / "ETHOS.md").write_text("# Local\n", encoding="utf-8") - ref = f"default/{AGENT}-spec#AGENT-SPEC.md" - download = AsyncMock(side_effect=[b"# Platform one\n", b"# Platform two\n"]) - monkeypatch.setattr(client.files, "download_content", download) - - first = _invoke(app, repo, "--agent", AGENT) - first_front = read_front_matter(client.files.stored[f"{AGENT}/discovery.md"]) - second = _invoke(app, repo, "--agent", AGENT) - second_front = read_front_matter(client.files.stored[f"{AGENT}/discovery.md"]) - - assert (first.exit_code, second.exit_code) == (0, 0) - assert first_front["ethos_path"] == ref - assert first_front["fingerprint"] != second_front["fingerprint"] - assert [awaited.kwargs for awaited in download.await_args_list] == [{"remote_path": ref}] * 2 - - -def test_healthy_config_uploads_only_discovery_report_and_does_not_write_to_repo(app, client, tmp_path): - repo = _healthy_repo(tmp_path / "agent-repo") - before = _snapshot(repo) - - result = _invoke(app, repo, "--agent", AGENT) - - assert result.exit_code == 0, result.output - assert "Repository\n ✓ Found 1 repository-owned Harbor config file." in result.output - assert "harbor job start -c configs/eval.yaml" in result.output - assert "Uploaded ticket-triage/discovery.md to fileset 'nemo-eval-author'." in result.output - assert client.files.stored.keys() == {f"{AGENT}/discovery.md"} - assert len(client.files.uploads) == 1 - assert client.files.uploads[0]["fileset"] == "nemo-eval-author" - assert client.files.uploads[0]["workspace"] == "default" - assert client.files.uploads[0]["fileset_auto_create"] is True - assert read_front_matter(client.files.stored[f"{AGENT}/discovery.md"])["runnable"] is True - assert _snapshot(repo) == before - assert client.closed is True - assert result.output.rstrip().endswith("Final overview: Discovery passed with 0 failures and 2 warnings.") - - -def test_missing_config_exits_one_but_uploads_the_report(app, client, tmp_path): - repo = tmp_path / "empty-repo" - repo.mkdir() - (repo / "README.md").write_text("# Empty\n", encoding="utf-8") - - result = _invoke(app, repo, "--agent", AGENT) - - assert result.exit_code == 1, result.output - assert client.files.stored.keys() == {f"{AGENT}/discovery.md"} - front = read_front_matter(client.files.stored[f"{AGENT}/discovery.md"]) - assert front["runnable"] is False - assert front["config_path"] is None - assert front["run_command"] is None - assert "harbor job start" not in result.output - assert result.output.rstrip().endswith("Final overview: Discovery failed with 1 failure and 2 warnings.") - - -def test_rejected_config_exits_one_and_uploads_its_report(app, client, tmp_path): - repo = tmp_path / "rejected-repo" - config = repo / "configs" / "eval.yaml" - config.parent.mkdir(parents=True) - config.write_text("datasets:\n - invalid\n", encoding="utf-8") - - result = _invoke(app, repo, "--agent", AGENT) - - assert result.exit_code == 1, result.output - front = read_front_matter(client.files.stored[f"{AGENT}/discovery.md"]) - assert front["runnable"] is False - assert any(check["name"] == "schema" and check["status"] == "fail" for check in front["checks"]) - assert front["run_command"] is None - assert result.output.rstrip().endswith("Final overview: Discovery failed with 1 failure and 2 warnings.") - - -def test_preflights_every_config_sequentially_and_uploads_all_results_after_a_failure( - app, client, monkeypatch, tmp_path -): - repo = tmp_path / "multi-config-repo" - first = repo / "first.yaml" - second = repo / "nested" / "second.yml" - for path, text in ( - (first, "job_name: first-entry\ndatasets:\n- path: first\n"), - (second, "datasets:\n- path: second\n"), - ): - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(text, encoding="utf-8") - - calls: list[str] = [] - active = False - - async def preflight(candidate, root): - nonlocal active - assert root == repo - assert not active - active = True - await asyncio.sleep(0) - relative = candidate.path.relative_to(repo).as_posix() - calls.append(relative) - active = False - if candidate.path == first: - return validate.ValidationOutcome( - checks=[validate._check("schema", "fail", "Harbor rejected the first config.")] - ) - return validate.ValidationOutcome( - checks=[validate._check("schema", "pass", "Harbor accepted the second config.")] - ) - - monkeypatch.setattr(validate, "run_ladder", preflight) - - result = _invoke(app, repo, "--agent", AGENT) - - assert result.exit_code == 1, result.output - assert calls == ["first.yaml", "nested/second.yml"] - uploaded = client.files.stored[f"{AGENT}/discovery.md"].decode() - assert "Harbor rejected the first config." in uploaded - assert "Harbor accepted the second config." in uploaded - - -def test_schema_failure_does_not_upload_the_rejected_input_value(app, client, tmp_path): - secret = "nvapi-secret-value-123456789" - repo = tmp_path / "secret-rejected-repo" - config = repo / "configs" / "eval.yaml" - config.parent.mkdir(parents=True) - config.write_text(f"datasets:\n - {secret}\n", encoding="utf-8") - - result = _invoke(app, repo, "--agent", AGENT) - - assert result.exit_code == 1, result.output - uploaded = client.files.stored[f"{AGENT}/discovery.md"] - schema_check = next(check for check in read_front_matter(uploaded)["checks"] if check["name"] == "schema") - assert secret not in schema_check["message"] - assert secret.encode() not in uploaded - assert "errors.pydantic.dev" not in schema_check["message"] - - -def test_dry_run_uploads_nothing_and_prints_the_report(app, client, tmp_path): - repo = _healthy_repo(tmp_path / "dry-repo") - - result = _invoke(app, repo, "--agent", AGENT, "--dry-run") - - assert result.exit_code == 0, result.output - assert "Dry run: no files were uploaded." in result.output - assert "---\nschema_version: 1" in result.output - assert "runnable: true" in result.output - assert client.files.uploads == [] - assert result.output.rstrip().endswith("Final overview: Discovery passed with 0 failures and 2 warnings.") - - -def test_upload_error_exits_one(app, monkeypatch, tmp_path): - repo = _healthy_repo(tmp_path / "upload-error-repo") - client = StubClient(files=StubFiles(fail=True)) - monkeypatch.setattr(discovery, "make_client", lambda base_url: client) - - result = _invoke(app, repo, "--agent", AGENT) - - assert result.exit_code == 1, result.output - assert "Upload failed: RuntimeError: fileset unavailable" in result.output - assert "harbor job start -c configs/eval.yaml" in result.output - assert client.files.stored == {} - assert result.output.rstrip().endswith("Final overview: Discovery failed with 1 failure and 2 warnings.") - - -def test_agent_name_precedence_uses_explicit_profile_then_directory(app, client, tmp_path): - explicit = _healthy_repo(tmp_path / "explicit-checkout") - (explicit / "optimizer.yaml").write_text("agent: ignored\n", encoding="utf-8") - profiled = _healthy_repo(tmp_path / "profile-checkout") - (profiled / "optimizer.yaml").write_text("agent: Profile Agent\n", encoding="utf-8") - defaulted = _healthy_repo(tmp_path / "Directory Agent") - - results = [ - _invoke(app, explicit, "--agent", "explicit-agent"), - _invoke(app, profiled), - _invoke(app, defaulted), - ] - - assert all(result.exit_code == 0 for result in results), [result.output for result in results] - assert client.files.stored.keys() == { - "explicit-agent/discovery.md", - "profile-agent/discovery.md", - "directory-agent/discovery.md", - } - - -def test_explicit_agent_is_slugged_for_traces_and_remote_paths(app, client, tmp_path, monkeypatch): - repo = _healthy_repo(tmp_path / "explicit-agent-repo") - trace_probe = AsyncMock( - return_value=discovery.scan._check("traces", "warn", "No traces exist.", severity="advisory") - ) - monkeypatch.setattr(discovery.scan, "probe_traces", trace_probe) - - result = _invoke(app, repo, "--agent", "../Ticket Agent#production") - - assert result.exit_code == 0, result.output - trace_probe.assert_awaited_once_with(client, agent="ticket-agent-production", workspace="default") - assert client.files.stored.keys() == {"ticket-agent-production/discovery.md"} - assert ( - read_front_matter(client.files.stored["ticket-agent-production/discovery.md"])["agent"] - == "ticket-agent-production" - ) - - -def test_every_invocation_revalidates_the_repository_config(app, client, tmp_path): - repo = _healthy_repo(tmp_path / "changing-repo") - config = repo / "configs" / "eval.yaml" - - first = _invoke(app, repo, "--agent", AGENT) - first_front = read_front_matter(client.files.stored[f"{AGENT}/discovery.md"]) - config.write_text("datasets:\n - invalid\n", encoding="utf-8") - second = _invoke(app, repo, "--agent", AGENT) - - assert first.exit_code == 0, first.output - assert second.exit_code == 1, second.output - second_front = read_front_matter(client.files.stored[f"{AGENT}/discovery.md"]) - assert second_front["runnable"] is False - assert any(check["name"] == "schema" and check["status"] == "fail" for check in second_front["checks"]) - assert { - "discovered_at": second_front["discovered_at"] != first_front["discovered_at"], - "fingerprint": second_front["fingerprint"] != first_front["fingerprint"], - } == {"discovered_at": True, "fingerprint": True} diff --git a/plugins/nemo-eval-author/tests/discover/test_report.py b/plugins/nemo-eval-author/tests/discover/test_report.py deleted file mode 100644 index caf999e1ea..0000000000 --- a/plugins/nemo-eval-author/tests/discover/test_report.py +++ /dev/null @@ -1,211 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Discovery report contract tests.""" - -import shlex -from datetime import UTC, datetime - -from harbor_fixtures import read_front_matter -from nemo_eval_author_plugin.discovery import report, scan, validate -from nemo_eval_author_plugin.discovery.validate import RequiredEnvVar -from nemo_insights_plugin.contracts.checks import CheckResult, CheckStatus, format_report - - -def _check(name: str = "config", status: CheckStatus = "pass", message: str = "Found the config.") -> CheckResult: - return CheckResult( - name=name, - group="repository" if name == "config" else "validation", - status=status, - severity="required", - message=message, - ) - - -_TRACE_CHECK = scan._check("traces", "pass", "Trace sessions exist.", severity="advisory") - - -def _config( - root, - *, - path: str = "configs/eval.yaml", - name: str = "evaluation", - checks: list[CheckResult] | None = None, -): - root = root.resolve() - return report.ConfigReport( - name=name, - path=root / path, - required_env_vars=[ - RequiredEnvVar(name="HF_TOKEN", default=None, declared_in=root / "evals" / "validation" / "task.toml") - ], - checks=checks if checks is not None else [_check()], - ) - - -def _record(tmp_path, *, configs=None, repository_checks: list[CheckResult] | None = None): - root = tmp_path.resolve() - return report.DiscoveryReport( - agent="ticket-triage", - workspace="default", - repo_root=root, - configs=[_config(root)] if configs is None else configs, - dataset_paths=[root / "evals" / "validation"], - ethos_path="ETHOS.md", - harbor_version="0.18.0", - discovered_at=datetime(2026, 8, 10, 15, tzinfo=UTC), - fingerprint="sha256:abc123", - input_file_count=4, - repository_checks=repository_checks or [], - trace_check=_TRACE_CHECK, - ) - - -def test_front_matter_records_the_complete_repository_contract(tmp_path): - check = _check() - record = _record(tmp_path, configs=[_config(tmp_path, checks=[check])]) - - markdown = report.render_markdown(record) - front = read_front_matter(markdown) - - assert front == { - "schema_version": 1, - "agent": "ticket-triage", - "workspace": "default", - "repo_root": str(tmp_path.resolve()), - "runnable": True, - "configs": [{"name": "evaluation", "path": "configs/eval.yaml"}], - "config_path": "configs/eval.yaml", - "dataset_paths": ["evals/validation"], - "run_command": f"cd {shlex.quote(str(tmp_path.resolve()))} && harbor job start -c configs/eval.yaml", - "ethos_path": "ETHOS.md", - "harbor_version": "0.18.0", - "required_env_vars": [ - { - "name": "HF_TOKEN", - "default": None, - "declared_in": "evals/validation/task.toml", - } - ], - "discovered_at": "2026-08-10T15:00:00+00:00", - "fingerprint": "sha256:abc123", - "input_file_count": 4, - "checks": [check.model_dump(mode="json"), _TRACE_CHECK.model_dump(mode="json")], - } - assert format_report([check]) in markdown - - -def test_a_rejected_config_is_blocked_and_has_no_command(tmp_path): - failure = _check("resolution", "fail", "Harbor could not resolve the job.") - record = _record(tmp_path, configs=[_config(tmp_path, checks=[_check(), failure])]) - - markdown = report.render_markdown(record) - front = read_front_matter(markdown) - - assert front["runnable"] is False - assert front["run_command"] is None - assert "harbor job start" not in markdown - assert failure.message in markdown - - -def test_a_report_without_a_repository_config_is_not_runnable(tmp_path): - record = _record( - tmp_path, - configs=[], - repository_checks=[_check("config", "fail", "No repository-owned Harbor config file exists.")], - ) - - front = read_front_matter(report.render_markdown(record)) - - assert front["runnable"] is False - assert front["config_path"] is None - assert front["run_command"] is None - - -def test_the_run_command_changes_to_the_repo_and_quotes_shell_paths(tmp_path): - repo = tmp_path / "repo $(touch unsafe); name" - record = _record( - repo, - configs=[_config(repo, path="configs/eval $(touch unsafe); suite.yaml")], - ) - - cd_command, harbor_command = record.run_command.split(" && ") - assert shlex.split(cd_command) == ["cd", str(repo.resolve())] - assert shlex.split(harbor_command) == [ - "harbor", - "job", - "start", - "-c", - "configs/eval $(touch unsafe); suite.yaml", - ] - - -def test_multi_config_report_uses_names_paths_stable_sections_and_one_command_each(tmp_path, monkeypatch): - root = tmp_path.resolve() - candidates = [ - scan.ConfigCandidate(root / "a.yaml", {"job_name": "shared", "tasks": ["a"]}), - scan.ConfigCandidate(root / "nested" / "b.yml", {"job_name": "shared", "tasks": ["b"]}), - scan.ConfigCandidate(root / "nested" / "fallback.json", {"job_name": " ", "tasks": ["c"]}), - ] - scan_result = scan.RepositoryScan( - configs=candidates, - dataset_paths=[], - ethos_path=None, - fingerprint="abc123", - input_file_count=3, - checks=[_check("config", "pass", "Found 3 repository-owned Harbor config files.")], - ) - validations = [ - validate.ValidationOutcome(checks=[_check("schema", "pass", f"Schema {index} passed.")]) for index in range(3) - ] - trace_check = scan._check("traces", "warn", "No traces exist.", severity="advisory") - monkeypatch.setattr(report, "harbor_version", lambda: "0.18.0") - - record = report.build_report( - agent="ticket-triage", - workspace="default", - repo_root=root, - scan_result=scan_result, - validations=validations, - trace_check=trace_check, - ) - markdown = report.render_markdown(record) - front = read_front_matter(markdown) - - assert front["configs"] == [ - {"name": "shared", "path": "a.yaml"}, - {"name": "shared", "path": "nested/b.yml"}, - {"name": "fallback.json", "path": "nested/fallback.json"}, - ] - assert (front["runnable"], front["config_path"], front["run_command"]) == (True, None, None) - headings = [ - "### `shared` (`a.yaml`)", - "### `shared` (`nested/b.yml`)", - "### `fallback.json` (`nested/fallback.json`)", - ] - positions = [markdown.index(heading) for heading in headings] - assert positions == sorted(positions) - for index, (position, candidate) in enumerate(zip(positions, candidates, strict=True)): - end = positions[index + 1] if index + 1 < len(positions) else len(markdown) - section = markdown[position:end] - assert f"Schema {index} passed." in section - assert f"harbor job start -c {candidate.path.relative_to(root).as_posix()}" in section - assert markdown.count("harbor job start -c") == 3 - - -def test_multi_config_report_is_not_runnable_when_one_config_fails(tmp_path): - failure = _check("schema", "fail", "Harbor rejected one config.") - record = _record( - tmp_path, - configs=[ - _config(tmp_path, path="first.yaml", name="first", checks=[failure]), - _config(tmp_path, path="second.yaml", name="second"), - ], - ) - - markdown = report.render_markdown(record) - front = read_front_matter(markdown) - - assert (front["runnable"], front["config_path"], front["run_command"]) == (False, None, None) - assert "harbor job start -c first.yaml" not in markdown - assert "harbor job start -c second.yaml" in markdown diff --git a/plugins/nemo-eval-author/tests/discover/test_scan.py b/plugins/nemo-eval-author/tests/discover/test_scan.py deleted file mode 100644 index 7337367b73..0000000000 --- a/plugins/nemo-eval-author/tests/discover/test_scan.py +++ /dev/null @@ -1,171 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Repository scan contract tests.""" - -from pathlib import Path -from types import SimpleNamespace -from unittest.mock import AsyncMock - -from nemo_eval_author_plugin.discovery import scan - - -def _config(path: Path, text: str = "datasets:\n- path: evals\n") -> Path: - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(text, encoding="utf-8") - return path - - -def _task(path: Path) -> None: - path.mkdir(parents=True, exist_ok=True) - (path / "task.toml").write_text('version = "1.0"\n', encoding="utf-8") - - -def test_finds_configs_through_depth_four_in_stable_order_and_prunes_jobs(tmp_path): - expected = [ - _config(tmp_path / "root.yaml"), - _config(tmp_path / "z" / "depth-one.yml"), - _config(tmp_path / "a" / "a" / "depth-two.json", '{"tasks": ["task"]}'), - _config(tmp_path / "b" / "b" / "b" / "depth-three.yaml"), - _config(tmp_path / "c" / "c" / "c" / "c" / "depth-four.yaml"), - ] - _config(tmp_path / "d" / "d" / "d" / "d" / "d" / "depth-five.yaml") - - result = scan.scan_repository(tmp_path) - - assert [candidate.path for candidate in result.configs] == expected - assert {check.status for check in result.checks if check.name == "config"} == {"pass"} - assert any(check.name == "ethos" and check.status == "warn" for check in result.checks) - - -def test_reads_yaml_yml_and_json_configs(tmp_path): - for suffix, text in ( - (".yaml", "datasets:\n- path: evals\n"), - (".yml", "datasets:\n- path: evals\n"), - (".json", '{"datasets": [{"path": "evals"}]}'), - ): - repo = tmp_path / suffix[1:] - expected = _config(repo / "configs" / f"job{suffix}", text) - - result = scan.scan_repository(repo) - - assert [candidate.path for candidate in result.configs] == [expected] - - -def test_rejects_a_config_symlink_that_resolves_outside_the_repository(tmp_path): - repo = tmp_path / "repo" - outside = _config(tmp_path / "outside.yaml") - link = repo / "configs" / "eval.yaml" - link.parent.mkdir(parents=True) - link.symlink_to(outside) - - result = scan.scan_repository(repo) - - assert result.configs == [] - assert next(check for check in result.checks if check.name == "config").status == "fail" - - -def test_rejects_a_config_file_symlink_inside_the_repository(tmp_path): - config = _config(tmp_path / "configs" / "eval.yaml") - link = config.with_name("alias.yaml") - link.symlink_to(config.name) - - result = scan.scan_repository(tmp_path) - - assert [candidate.path for candidate in result.configs] == [config] - - -def test_profile_prior_job_and_task_layout_never_become_config_candidates(tmp_path): - (tmp_path / "optimizer.yaml").write_text( - "agent: ticket-triage\ndatasets:\n validation: evals/validation\n", - encoding="utf-8", - ) - prior_job = tmp_path / "jobs" / "run-1" - _config(prior_job / "config.json", '{"datasets": [{"path": "evals/validation"}]}') - (prior_job / "lock.json").write_text('{"harbor_version": "0.18.0"}\n', encoding="utf-8") - dataset = tmp_path / "evals" / "validation" - _task(dataset / "task-0") - - result = scan.scan_repository(tmp_path) - - assert (result.configs, result.dataset_paths) == ([], [dataset]) - - -def test_uses_only_ethos_for_the_doctrine_contract(tmp_path): - _config(tmp_path / "harbor-job.yaml") - (tmp_path / "README.md").write_text("# Readme\n", encoding="utf-8") - (tmp_path / "AGENT-SPEC.md").write_text("# Old\n", encoding="utf-8") - - without_ethos = scan.scan_repository(tmp_path) - assert without_ethos.ethos_path is None - assert any(check.name == "ethos" and check.status == "warn" for check in without_ethos.checks) - - ethos = tmp_path / "ETHOS.md" - ethos.write_text("# Agent doctrine\n", encoding="utf-8") - with_ethos = scan.scan_repository(tmp_path) - - assert with_ethos.ethos_path == "ETHOS.md" - assert any(check.name == "ethos" and check.status == "pass" for check in with_ethos.checks) - - -def test_discovers_local_datasets_and_prunes_generated_trees(tmp_path): - _config(tmp_path / "harbor-job.yaml") - _task(tmp_path) - _task(tmp_path / "evals" / "suite" / "task-one") - _task(tmp_path / "evals" / "suite" / "task_template") - _task(tmp_path / ".nemo-optimizer" / "output" / "task-two") - _task(tmp_path / "node_modules" / "package" / "task-three") - _task(tmp_path / "vendor" / "package" / "task-four") - _task(tmp_path / "cache" / "package" / "task-five") - _task(tmp_path / "jobs" / "prior-run" / "task-six") - - result = scan.scan_repository(tmp_path) - - assert result.dataset_paths == [tmp_path / "evals" / "suite"] - assert result.input_file_count == 3 - - -def test_fingerprint_covers_config_ethos_optimizer_and_dataset_files(tmp_path): - config = _config(tmp_path / "harbor-job.yaml") - other_config = _config(tmp_path / "nested" / "other.json", '{"tasks": ["task"]}') - ethos = tmp_path / "ETHOS.md" - optimizer = tmp_path / "optimizer.yaml" - ethos.write_text("# One\n", encoding="utf-8") - optimizer.write_text("model: one\n", encoding="utf-8") - task = tmp_path / "evals" / "suite" / "task-one" - _task(task) - dataset_file = task / "notes.txt" - dataset_file.write_text("one\n", encoding="utf-8") - - first = scan.scan_repository(tmp_path) - - assert first.input_file_count == 6 - assert [candidate.path for candidate in first.configs] == [config, other_config] - for path, replacement in ( - (config, "datasets:\n- path: another-evals\n"), - (other_config, '{"tasks": ["another-task"]}'), - (ethos, "# Two\n"), - (optimizer, "model: two\n"), - (task / "task.toml", 'version = "2.0"\n'), - (dataset_file, "two\n"), - ): - original = path.read_text(encoding="utf-8") - path.write_text(replacement, encoding="utf-8") - assert scan.scan_repository(tmp_path).fingerprint != first.fingerprint - path.write_text(original, encoding="utf-8") - - -async def test_trace_probe_handles_exception_empty_and_positive_totals(): - for total, status in ((None, "warn"), (0, "warn"), (2, "pass")): - list_groups = ( - AsyncMock(side_effect=RuntimeError("intake unavailable")) - if total is None - else AsyncMock(return_value=SimpleNamespace(pagination=SimpleNamespace(total_results=total))) - ) - client = SimpleNamespace( - intake=SimpleNamespace(spans=SimpleNamespace(groups=SimpleNamespace(list=list_groups))) - ) - - finding = await scan.probe_traces(client, agent="ticket-triage", workspace="default") - - assert (finding.status, finding.severity) == (status, "advisory") diff --git a/plugins/nemo-eval-author/tests/discover/test_validate.py b/plugins/nemo-eval-author/tests/discover/test_validate.py deleted file mode 100644 index 2c2acffe30..0000000000 --- a/plugins/nemo-eval-author/tests/discover/test_validate.py +++ /dev/null @@ -1,389 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Focused Harbor preflight contract tests.""" - -import subprocess -import sys -from pathlib import Path - -import pytest -from harbor.utils.logger import logger as harbor_logger -from harbor_fixtures import write_dataset, write_task, write_wrapper -from nemo_eval_author_plugin.discovery import scan, validate - - -def _candidate(path: Path, data: dict) -> scan.ConfigCandidate: - return scan.ConfigCandidate(path=path, data=data) - - -def _check(outcome: validate.ValidationOutcome, name: str): - matches = [item for item in outcome.checks if item.name == name] - assert matches, f"no {name!r} check in {[item.name for item in outcome.checks]}" - return matches[0] - - -def _patch_external_preflight(monkeypatch) -> None: - monkeypatch.setattr(validate.EnvironmentFactory, "run_preflight", lambda *args: None) - monkeypatch.setattr( - validate, - "check_config_file", - lambda path, root: validate._check("round-trip", "pass", "The config file loads through the Harbor CLI"), - ) - - -async def test_a_well_formed_repo_passes_the_ladder(tmp_path, monkeypatch): - dataset = write_dataset(tmp_path / "evals" / "validation") - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate( - tmp_path / "harbor-job.yaml", {"agents": [{"name": "oracle"}], "datasets": [{"path": str(dataset)}]} - ), - tmp_path, - ) - - assert not [check for check in outcome.checks if check.status == "fail"] - assert {check.name for check in outcome.checks} == { - "schema", - "resolution", - "agent", - "backend", - "round-trip", - "tasks", - "coverage", - "credentials", - } - assert _check(outcome, "tasks").message.startswith("2 of 2") - assert _check(outcome, "coverage").status == "pass" - - -async def test_ladder_closes_added_harbor_logger_handlers(tmp_path, monkeypatch): - dataset = write_dataset(tmp_path / "evals" / "validation") - _patch_external_preflight(monkeypatch) - handlers_before = tuple(harbor_logger.handlers) - - await validate.run_ladder( - _candidate( - tmp_path / "harbor-job.yaml", - {"agents": [{"name": "oracle"}], "datasets": [{"path": str(dataset)}]}, - ), - tmp_path, - ) - - assert tuple(harbor_logger.handlers) == handlers_before - - -async def test_schema_failure_stops_the_ladder(tmp_path): - outcome = await validate.run_ladder(_candidate(tmp_path / "harbor-job.yaml", {"datasets": "not-a-list"}), tmp_path) - - assert _check(outcome, "schema").status == "fail" - assert [item.name for item in outcome.checks] == ["schema"] - - -async def test_resolution_failure_reports_the_real_error(tmp_path, monkeypatch): - _patch_external_preflight(monkeypatch) - outcome = await validate.run_ladder( - _candidate(tmp_path / "harbor-job.yaml", {"datasets": [{"path": str(tmp_path / "nope")}]}), tmp_path - ) - - finding = _check(outcome, "resolution") - assert finding.status == "fail" - assert "nope" in finding.message - - -async def test_missing_resolved_task_attribute_is_a_compatibility_failure(tmp_path, monkeypatch): - dataset = write_dataset(tmp_path / "evals" / "validation", count=1) - _patch_external_preflight(monkeypatch) - - class _JobWithoutTaskConfigs: - @classmethod - async def create(cls, _config): - return cls() - - def _close_logger_handlers(self): - pass - - monkeypatch.setattr(validate, "Job", _JobWithoutTaskConfigs) - outcome = await validate.run_ladder( - _candidate(tmp_path / "harbor-job.yaml", {"datasets": [{"path": str(dataset)}]}), tmp_path - ) - - assert _check(outcome, "compatibility").status == "fail" - assert "Job._task_configs" in _check(outcome, "compatibility").message - - -async def test_invalid_resolved_task_fails_tasks_check(tmp_path, monkeypatch): - dataset = write_dataset(tmp_path / "evals" / "validation", count=1) - _patch_external_preflight(monkeypatch) - create_job = validate.Job.create - - async def resolve_then_invalidate(config): - job = await create_job(config) - monkeypatch.setattr(validate.Task, "is_valid_dir", lambda _path: False) - return job - - monkeypatch.setattr(validate.Job, "create", staticmethod(resolve_then_invalidate)) - - outcome = await validate.run_ladder( - _candidate(tmp_path / "harbor-job.yaml", {"datasets": [{"path": str(dataset)}]}), tmp_path - ) - - assert _check(outcome, "tasks").status == "fail" - - -async def test_silent_task_drop_fails_concrete_coverage(tmp_path, monkeypatch): - dataset = tmp_path / "evals" / "validation" - write_task(dataset / "task-0") - write_task(dataset / "task-1", instruction=None) - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate(tmp_path / "harbor-job.yaml", {"datasets": [{"path": str(dataset)}]}), tmp_path - ) - - assert _check(outcome, "coverage").status == "fail" - assert "task-1" in _check(outcome, "coverage").message - - -async def test_empty_task_names_does_not_make_a_dropped_task_advisory(tmp_path, monkeypatch): - dataset = tmp_path / "evals" / "validation" - write_task(dataset / "task-0") - write_task(dataset / "task-1", instruction=None) - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate( - tmp_path / "harbor-job.yaml", - {"datasets": [{"path": str(dataset), "task_names": []}]}, - ), - tmp_path, - ) - - coverage = _check(outcome, "coverage") - assert (coverage.status, coverage.severity) == ("fail", "required") - assert "task-1" in coverage.message - assert coverage.hint == "Harbor skips these task dirs silently." - - -async def test_malformed_explicit_task_name_fails_coverage(tmp_path, monkeypatch): - dataset = tmp_path / "evals" / "validation" - write_task(dataset / "task-0") - write_task(dataset / "task-1", instruction=None) - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate( - tmp_path / "harbor-job.yaml", - {"datasets": [{"path": str(dataset), "task_names": ["task-0", "task-1"]}]}, - ), - tmp_path, - ) - - assert (_check(outcome, "coverage").status, _check(outcome, "coverage").severity) == ("fail", "required") - assert "task-1" in _check(outcome, "coverage").message - - -async def test_excluded_task_drop_is_advisory(tmp_path, monkeypatch): - dataset = tmp_path / "evals" / "validation" - write_task(dataset / "task-0") - write_task(dataset / "task-1", instruction=None) - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate( - tmp_path / "harbor-job.yaml", - {"datasets": [{"path": str(dataset), "exclude_task_names": ["task-1"]}]}, - ), - tmp_path, - ) - - assert (_check(outcome, "coverage").status, _check(outcome, "coverage").severity) == ("warn", "advisory") - - -async def test_non_excluded_invalid_task_fails_coverage_with_exclude_filter(tmp_path, monkeypatch): - dataset = tmp_path / "evals" / "validation" - write_task(dataset / "task-0") - write_task(dataset / "task-excluded") - write_task(dataset / "task-invalid", instruction=None) - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate( - tmp_path / "harbor-job.yaml", - {"datasets": [{"path": str(dataset), "exclude_task_names": ["task-excluded"]}]}, - ), - tmp_path, - ) - - coverage = _check(outcome, "coverage") - assert (coverage.status, coverage.severity) == ("fail", "required") - assert "task-invalid" in coverage.message - assert "task-excluded" not in coverage.message - assert coverage.hint == "Harbor skipped a task selected by the dataset filters." - - -async def test_n_tasks_subset_drop_is_advisory(tmp_path, monkeypatch): - dataset = write_dataset(tmp_path / "evals" / "validation") - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate( - tmp_path / "harbor-job.yaml", - {"datasets": [{"path": str(dataset), "n_tasks": 1}]}, - ), - tmp_path, - ) - - assert (_check(outcome, "coverage").status, _check(outcome, "coverage").severity) == ("warn", "advisory") - - -async def test_required_host_variables_are_recorded(tmp_path, monkeypatch): - dataset = tmp_path / "evals" / "validation" - write_task( - dataset / "task-0", - task_toml='\n[environment.env]\nHF_TOKEN = "${HF_TOKEN}"\nREGION = "${AWS_REGION:-us-west-2}"\n', - ) - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate(tmp_path / "harbor-job.yaml", {"datasets": [{"path": str(dataset)}]}), tmp_path - ) - - assert {item.name: item.default for item in outcome.required_env_vars} == { - "HF_TOKEN": None, - "AWS_REGION": "us-west-2", - } - assert _check(outcome, "credentials").status == "pass" - - -async def test_missing_custom_agent_import_is_recorded(tmp_path, monkeypatch): - dataset = write_dataset(tmp_path / "evals" / "validation", count=1) - write_wrapper(tmp_path) - monkeypatch.setattr(sys, "path", [path for path in sys.path if path not in {"", str(tmp_path)}]) - monkeypatch.delitem(sys.modules, "harbor_wrapper", raising=False) - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate( - tmp_path / "harbor-job.yaml", - {"agents": [{"import_path": "harbor_wrapper:WrappedAgent"}], "datasets": [{"path": str(dataset)}]}, - ), - tmp_path, - ) - - assert _check(outcome, "agent").status == "fail" - - -async def test_non_class_custom_agent_import_is_recorded(tmp_path, monkeypatch): - dataset = write_dataset(tmp_path / "evals" / "validation", count=1) - (tmp_path / "invalid_wrapper.py").write_text("def not_a_class():\n return None\n", encoding="utf-8") - monkeypatch.syspath_prepend(str(tmp_path)) - _patch_external_preflight(monkeypatch) - - outcome = await validate.run_ladder( - _candidate( - tmp_path / "harbor-job.yaml", - {"agents": [{"import_path": "invalid_wrapper:not_a_class"}], "datasets": [{"path": str(dataset)}]}, - ), - tmp_path, - ) - - assert _check(outcome, "agent").status == "fail" - - -def test_custom_agent_import_system_exit_is_recorded(monkeypatch): - config = validate.JobConfig.model_validate({"agents": [{"import_path": "agent_module:Agent"}]}) - outcome = validate.ValidationOutcome() - - def exit_import(*_args, **_kwargs): - raise SystemExit("agent stopped") - - monkeypatch.setattr(validate, "import_class", exit_import) - - validate._check_agent(config, outcome) - - assert (_check(outcome, "agent").status, _check(outcome, "agent").message) == ( - "fail", - "Cannot import agent agent_module:Agent: SystemExit: agent stopped", - ) - - -def test_custom_agent_import_keyboard_interrupt_propagates(monkeypatch): - config = validate.JobConfig.model_validate({"agents": [{"import_path": "agent_module:Agent"}]}) - outcome = validate.ValidationOutcome() - - def interrupt_import(*_args, **_kwargs): - raise KeyboardInterrupt - - monkeypatch.setattr(validate, "import_class", interrupt_import) - - with pytest.raises(KeyboardInterrupt): - validate._check_agent(config, outcome) - - -async def test_custom_agent_import_does_not_reuse_another_repositorys_module(tmp_path, monkeypatch): - first, second = tmp_path / "first", tmp_path / "second" - _patch_external_preflight(monkeypatch) - for repo, class_name in ((first, "FirstAgent"), (second, "SecondAgent")): - dataset = write_dataset(repo / "evals" / "validation", count=1) - write_wrapper(repo, class_name=class_name) - monkeypatch.syspath_prepend(str(repo)) - outcome = await validate.run_ladder( - _candidate( - repo / "harbor-job.yaml", - { - "agents": [{"import_path": f"harbor_wrapper:{class_name}"}], - "datasets": [{"path": str(dataset)}], - }, - ), - repo, - ) - assert _check(outcome, "agent").status == "pass" - - -async def test_backend_failure_is_recorded(tmp_path, monkeypatch): - dataset = write_dataset(tmp_path / "evals" / "validation", count=1) - _patch_external_preflight(monkeypatch) - monkeypatch.setattr( - validate.EnvironmentFactory, - "run_preflight", - lambda *args: (_ for _ in ()).throw(RuntimeError("no Docker")), - ) - - outcome = await validate.run_ladder( - _candidate(tmp_path / "harbor-job.yaml", {"datasets": [{"path": str(dataset)}]}), tmp_path - ) - - assert _check(outcome, "backend").status == "fail" - - -def test_config_file_round_trip_runs_harbor_cli(tmp_path, monkeypatch): - config_path = tmp_path / "harbor-job.yaml" - config_path.write_text("datasets: []\n", encoding="utf-8") - monkeypatch.setattr(validate, "_harbor_executable", lambda: "harbor") - calls = [] - - def run(command, **kwargs): - calls.append((command, kwargs["cwd"])) - return subprocess.CompletedProcess(command, 0, "", "") - - monkeypatch.setattr(subprocess, "run", run) - - assert validate.check_config_file(config_path, tmp_path).status == "pass" - assert calls == [(["harbor", "job", "start", "--print-config", "-c", str(config_path)], tmp_path)] - - -def test_config_file_round_trip_reports_harbor_rejection(tmp_path, monkeypatch): - config_path = tmp_path / "harbor-job.yaml" - config_path.write_text("datasets: []\n", encoding="utf-8") - monkeypatch.setattr(validate, "_harbor_executable", lambda: "harbor") - - def reject(command, **_kwargs): - return subprocess.CompletedProcess(command, 1, "", "invalid config\n") - - monkeypatch.setattr(subprocess, "run", reject) - - finding = validate.check_config_file(config_path, tmp_path) - - assert (finding.status, finding.message) == ("fail", "The Harbor CLI rejected the config: invalid config.") diff --git a/plugins/nemo-eval-author/tests/harbor_fixtures.py b/plugins/nemo-eval-author/tests/harbor_fixtures.py deleted file mode 100644 index c3f21b92e2..0000000000 --- a/plugins/nemo-eval-author/tests/harbor_fixtures.py +++ /dev/null @@ -1,80 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Harbor task builders and platform stubs.""" - -from pathlib import Path -from typing import Any - -import yaml - - -def read_front_matter(text: str | bytes) -> dict[str, Any]: - if isinstance(text, bytes): - text = text.decode("utf-8") - assert text.startswith("---\n"), f"no front matter in {text[:40]!r}" - payload = yaml.safe_load(text[4:].partition("\n---\n")[0]) - assert isinstance(payload, dict) - return payload - - -def write_task( - task_dir: Path, - *, - task_toml: str = "", - instruction: str | None = "Do the thing.\n", -) -> None: - task_dir.mkdir(parents=True, exist_ok=True) - (task_dir / "task.toml").write_text(f'version = "1.0"\n{task_toml}', encoding="utf-8") - - if instruction is not None: - (task_dir / "instruction.md").write_text(instruction, encoding="utf-8") - (task_dir / "environment").mkdir(exist_ok=True) - (task_dir / "environment" / "Dockerfile").write_text("FROM ubuntu:24.04\n\nWORKDIR /app\n", encoding="utf-8") - (task_dir / "tests").mkdir(exist_ok=True) - (task_dir / "tests" / "test.sh").write_text("#!/bin/bash\necho 1 > /logs/verifier/reward.txt\n", encoding="utf-8") - - -def write_dataset(root: Path, *, count: int = 2) -> Path: - for index in range(count): - write_task(root / f"task-{index}") - return root - - -def write_wrapper(wrapper_dir: Path, *, class_name: str = "WrappedAgent") -> None: - wrapper_dir.mkdir(parents=True, exist_ok=True) - (wrapper_dir / "harbor_wrapper.py").write_text( - f"from harbor.agents.base import BaseAgent\n\n\nclass {class_name}(BaseAgent):\n pass\n", - encoding="utf-8", - ) - - -def write_job_config(path: Path, *, dataset: str) -> None: - path.parent.mkdir(parents=True, exist_ok=True) - payload = {"agents": [{"name": "oracle"}], "datasets": [{"path": dataset}]} - path.write_text(yaml.safe_dump(payload), encoding="utf-8") - - -class StubFiles: - def __init__(self, fail: bool = False) -> None: - self.stored: dict[str, bytes] = {} - self.uploads: list[dict[str, Any]] = [] - self.fail = fail - - async def download_content(self, *, remote_path: str) -> bytes: - raise FileNotFoundError(remote_path) - - async def upload_content(self, *, content: bytes, remote_path: str, **kwargs: Any) -> None: - if self.fail: - raise RuntimeError("fileset unavailable") - self.uploads.append({"remote_path": remote_path, "content": content, **kwargs}) - self.stored[remote_path] = content - - -class StubClient: - def __init__(self, files: StubFiles | None = None) -> None: - self.files = files or StubFiles() - self.closed = False - - async def close(self) -> None: - self.closed = True diff --git a/plugins/nemo-eval-author/tests/test_cli.py b/plugins/nemo-eval-author/tests/test_cli.py deleted file mode 100644 index d42d8b0f79..0000000000 --- a/plugins/nemo-eval-author/tests/test_cli.py +++ /dev/null @@ -1,36 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Command-tree and placeholder tests.""" - -import pytest -import typer -from nemo_eval_author_plugin import cli -from typer.testing import CliRunner - -runner = CliRunner() - - -@pytest.fixture -def app() -> typer.Typer: - return cli.EvalAuthorCLI().get_cli() - - -def test_help_lists_discover(app: typer.Typer) -> None: - result = runner.invoke(app, ["--help"]) - - assert result.exit_code == 0, result.output - assert "discover" in result.output - - -def test_placeholder_verbs_refuse_to_run_and_name_their_tickets(app: typer.Typer) -> None: - for command, ticket in ( - ("audit", "ASE-676"), - ("propose", "ASE-675"), - ("run", "ASE-673"), - ("doctor", "ASE-678"), - ): - result = runner.invoke(app, [command]) - - assert result.exit_code == 1, result.output - assert ticket in result.output diff --git a/plugins/nemo-experimentalist/AGENTS.md b/plugins/nemo-experimentalist/AGENTS.md index 85d87f60eb..b3004f7823 100644 --- a/plugins/nemo-experimentalist/AGENTS.md +++ b/plugins/nemo-experimentalist/AGENTS.md @@ -29,8 +29,9 @@ Experimentalist imports nothing from `nemo-eval-author-plugin`, so the package c ten borrows to zero, so it went away with them. - `tests/test_contract_dependency.py` asserts that this plugin never declares `nemo-eval-author-plugin` as a dependency, which is what keeps the cycle broken. -- `plugins/nemo-eval-author/` keeps the `nemo agents eval-author` command group and its - `discovery/` package, whose one remaining borrow is `make_client`. +- `plugins/nemo-eval-author/` ships the customer-facing skills and nothing else. Its CLI + and `discovery/` package went away with the pivot to skills, and with them the last + borrow from this plugin, so neither side imports the other now. - Agent tests live in `tests/eval_author/`. This plugin's `conftest.py` already covers the isolation those tests need, so the Eval Author copy went away. @@ -41,10 +42,10 @@ registered under the `nemo.cli.agents` entry-point group, which the `nemo-agents plugin's `AgentsCLI` discovers and mounts. There is no top-level `nemo experimentalist` alias. -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. +The Analyst follows the same rule: `nemo agents analyst run` (was `nemo insights +analyze`). Prefer `ctx.command_path` over a hardcoded path when a message quotes the +command back to the user. Eval Author had a command group under this rule and no +longer does; it ships skills instead. ### 2026-07-28: Eval Author extracted to its own plugin, heading for standalone (superseded) diff --git a/plugins/nemo-experimentalist/README.md b/plugins/nemo-experimentalist/README.md index 8420234161..f198cb0511 100644 --- a/plugins/nemo-experimentalist/README.md +++ b/plugins/nemo-experimentalist/README.md @@ -78,9 +78,9 @@ single leader, so complementary strengths stay alive across rounds. analyze traces or host an Insight API. - [Eval Author](src/nemo_experimentalist_plugin/eval_author/README.md) builds the Insight-specific evaluation suite, and Insight mode invokes it automatically. It - ships inside this plugin; the separate - [Eval Author plugin](../nemo-eval-author/README.md) owns the - `nemo agents eval-author` commands. + ships inside this plugin. The separate + [Eval Author package](../nemo-eval-author/README.md) is the customer-facing path, + and it ships skills rather than an agent or a CLI. - **Harbor** runs the task containers that score every candidate. - **NeMo Experiments** mirrors each run and its candidates as an experiment group, so the lineage is visible in Studio. Structure only — rewards and diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md index f1678087fa..016224a8cf 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md @@ -17,8 +17,9 @@ This package lives inside the Experimentalist plugin and uses its evaluator, sta and trace helpers directly. Experimentalist insight mode imports and runs Eval Author before optimization begins. -The `nemo agents eval-author` command group lives in the separate -[Eval Author plugin](../../../../nemo-eval-author/README.md). +The customer-facing path is a skill rather than an agent, and the +[Eval Author package](../../../../nemo-eval-author/README.md) holds those skills. It +has no CLI and shares no code with this agent. ## Current Files diff --git a/pyproject.toml b/pyproject.toml index 62676be44b..526993027b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,10 +71,10 @@ version = "0.0.0" [dependency-groups] # Insights analyst plugin convenience group. insights = ["nemo-insights-plugin"] -# Experimentalist and Eval Author support Python 3.12 and 3.13. Both are listed so the -# `nemo agents eval-author` commands are installed alongside Experimentalist. The -# dependency is one arrow: Eval Author borrows the Experimentalist platform client -# (see plugins/nemo-eval-author/README.md). +# Experimentalist and Eval Author support Python 3.12 and 3.13. Eval Author ships skills +# rather than code, and imports nothing from Experimentalist. It is grouped here so that +# installing Experimentalist also makes the Eval Author tests visible to root test +# discovery (see tests/discovery_exclusions.py). experimentalist = [ "nemo-experimentalist-plugin ; python_full_version < '3.14'", "nemo-eval-author-plugin ; python_full_version < '3.14'", diff --git a/uv.lock b/uv.lock index 68d50aea41..29383d3d69 100644 --- a/uv.lock +++ b/uv.lock @@ -4352,23 +4352,13 @@ name = "nemo-eval-author-plugin" version = "0.1.0" source = { editable = "plugins/nemo-eval-author" } dependencies = [ - { name = "harbor", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-experimentalist-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-insights-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-platform", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-platform-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "pydantic", extra = ["email"], marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "pyyaml", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, ] [package.metadata] requires-dist = [ - { name = "harbor", specifier = ">=0.18" }, - { name = "nemo-experimentalist-plugin", editable = "plugins/nemo-experimentalist" }, { name = "nemo-insights-plugin", editable = "plugins/nemo-insights" }, - { name = "nemo-platform", editable = "packages/nemo_platform" }, - { name = "nemo-platform-plugin", editable = "packages/nemo_platform_plugin" }, - { name = "pydantic", specifier = ">=2" }, { name = "pyyaml", specifier = ">=6.0.3" }, ] @@ -4849,7 +4839,6 @@ all = [ { name = "nemo-auditor-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-deployments-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-evaluator-sdk", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-experimentalist-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-fabric", extra = ["claude", "codex", "deepagents", "relay"], marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-fabric-adapters-hermes", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-insights-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, @@ -5132,11 +5121,7 @@ nemo-deployments-plugin = [ { name = "pyyaml", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, ] nemo-eval-author-plugin = [ - { name = "harbor", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-experimentalist-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-insights-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-platform-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "pydantic", extra = ["email"], marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "pyyaml", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, ] nemo-evaluator-plugin = [ @@ -5318,7 +5303,6 @@ plugins = [ { name = "nemo-anonymizer", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-deployments-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-evaluator-sdk", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-experimentalist-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-fabric", extra = ["claude", "codex", "deepagents", "relay"], marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-fabric-adapters-hermes", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-insights-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, @@ -5399,7 +5383,6 @@ services = [ { name = "nemo-auditor-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-deployments-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-evaluator-sdk", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-experimentalist-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-fabric", extra = ["claude", "codex", "deepagents", "relay"], marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-fabric-adapters-hermes", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-insights-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, @@ -5626,13 +5609,9 @@ requires-dist = [ { name = "gunicorn", marker = "extra == 'plugins'", specifier = ">=23.0.0" }, { name = "gunicorn", marker = "extra == 'services'", specifier = ">=23.0.0" }, { name = "harbor", marker = "extra == 'all'", specifier = ">=0.16" }, - { name = "harbor", marker = "extra == 'all'", specifier = ">=0.18" }, - { name = "harbor", marker = "extra == 'nemo-eval-author-plugin'", specifier = ">=0.18" }, { name = "harbor", marker = "extra == 'nemo-experimentalist-plugin'", specifier = ">=0.16" }, { name = "harbor", marker = "extra == 'plugins'", specifier = ">=0.16" }, - { name = "harbor", marker = "extra == 'plugins'", specifier = ">=0.18" }, { name = "harbor", marker = "extra == 'services'", specifier = ">=0.16" }, - { name = "harbor", marker = "extra == 'services'", specifier = ">=0.18" }, { name = "httpx", marker = "extra == 'all'" }, { name = "httpx", marker = "extra == 'all'", specifier = ">=0.27.0" }, { name = "httpx", marker = "extra == 'all'", specifier = ">=0.27.2" }, @@ -5732,10 +5711,6 @@ requires-dist = [ { name = "nemo-evaluator-sdk", marker = "extra == 'nemo-optimization-plugin'", editable = "packages/nemo_evaluator_sdk" }, { name = "nemo-evaluator-sdk", marker = "extra == 'plugins'", editable = "packages/nemo_evaluator_sdk" }, { name = "nemo-evaluator-sdk", marker = "extra == 'services'", editable = "packages/nemo_evaluator_sdk" }, - { name = "nemo-experimentalist-plugin", marker = "extra == 'all'", editable = "plugins/nemo-experimentalist" }, - { name = "nemo-experimentalist-plugin", marker = "extra == 'nemo-eval-author-plugin'", editable = "plugins/nemo-experimentalist" }, - { name = "nemo-experimentalist-plugin", marker = "extra == 'plugins'", editable = "plugins/nemo-experimentalist" }, - { name = "nemo-experimentalist-plugin", marker = "extra == 'services'", editable = "plugins/nemo-experimentalist" }, { name = "nemo-fabric", marker = "extra == 'nemo-evaluator-sdk'", git = "https://github.com/NVIDIA/NeMo-Fabric.git?rev=e7353383024523179be6a009ef16dea223bea8c0" }, { name = "nemo-fabric", extras = ["claude", "codex", "deepagents", "relay"], marker = "extra == 'all'", git = "https://github.com/NVIDIA/NeMo-Fabric.git?rev=e7353383024523179be6a009ef16dea223bea8c0" }, { name = "nemo-fabric", extras = ["claude", "codex", "deepagents", "relay"], marker = "extra == 'nemo-agents-plugin'", git = "https://github.com/NVIDIA/NeMo-Fabric.git?rev=e7353383024523179be6a009ef16dea223bea8c0" }, @@ -5765,7 +5740,6 @@ requires-dist = [ { name = "nemo-platform-plugin", marker = "extra == 'nemo-auditor-plugin'", editable = "packages/nemo_platform_plugin" }, { name = "nemo-platform-plugin", marker = "extra == 'nemo-data-designer-plugin'", editable = "packages/nemo_platform_plugin" }, { name = "nemo-platform-plugin", marker = "extra == 'nemo-deployments-plugin'", editable = "packages/nemo_platform_plugin" }, - { name = "nemo-platform-plugin", marker = "extra == 'nemo-eval-author-plugin'", editable = "packages/nemo_platform_plugin" }, { name = "nemo-platform-plugin", marker = "extra == 'nemo-evaluator-plugin'", editable = "packages/nemo_platform_plugin" }, { name = "nemo-platform-plugin", marker = "extra == 'nemo-experimentalist-plugin'", editable = "packages/nemo_platform_plugin" }, { name = "nemo-platform-plugin", marker = "extra == 'nemo-guardrails-plugin'", editable = "packages/nemo_platform_plugin" }, @@ -5960,7 +5934,6 @@ requires-dist = [ { name = "pydantic", marker = "extra == 'models-service'", specifier = ">=2.10.6" }, { name = "pydantic", marker = "extra == 'nemo-auditor-plugin'", specifier = ">=2.10.6" }, { name = "pydantic", marker = "extra == 'nemo-deployments-plugin'", specifier = ">=2.10.6" }, - { name = "pydantic", marker = "extra == 'nemo-eval-author-plugin'", specifier = ">=2" }, { name = "pydantic", marker = "extra == 'nemo-evaluator-plugin'", specifier = ">=2.12.0" }, { name = "pydantic", marker = "extra == 'nemo-evaluator-sdk'", specifier = ">=2.10.6" }, { name = "pydantic", marker = "extra == 'nemo-experimentalist-plugin'", specifier = ">=2" }, From 2d5c272a70a76fd1d508e81a0fd32fd841dc9e5c Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Thu, 20 Aug 2026 14:11:20 -0500 Subject: [PATCH 3/8] docs(packaging): correct the Eval Author bundling rationale The comment justified bundling Eval Author with Experimentalist and Insights by a dependency cycle that no longer exists: Experimentalist no longer imports EvalAuthor, and Eval Author no longer borrows Experimentalist helpers. Only the shared Insights profile contract remains, and that alone would not require co-bundling. Records why the entry stays anyway, which is that bundling is how the skills reach a customer through nemo-platform[all], and notes that the entry-point inherit is now a no-op so nobody reads the empty clause as a bug. Signed-off-by: Alec Khoury --- packages/nemo_platform/pyproject.toml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/nemo_platform/pyproject.toml b/packages/nemo_platform/pyproject.toml index b6de1dd347..216c51bcfd 100644 --- a/packages/nemo_platform/pyproject.toml +++ b/packages/nemo_platform/pyproject.toml @@ -723,10 +723,11 @@ nemo-anonymizer-plugin = { source = "../../plugins/nemo-anonymizer/src/nemo_anon nemo-auditor-plugin = { source = "../../plugins/nemo-auditor/src/nemo_auditor", module = "nemo_auditor", inherit = { "entry-points" = ["nemo.*"] } } nemo-data-designer-plugin = { source = "../../plugins/nemo-data-designer/src/nemo_data_designer_plugin", module = "nemo_data_designer_plugin", inherit = { "entry-points" = ["nemo.*"] } } nemo-deployments-plugin = { source = "../../plugins/nemo-deployments/src/nemo_deployments_plugin", module = "nemo_deployments_plugin", inherit = { "entry-points" = ["nemo.*"] } } -# Eval Author, Experimentalist, and Insights ship together: Experimentalist imports -# EvalAuthor and EvalAuthorConfig at module scope, Eval Author still borrows -# Experimentalist helpers, and both read the Insights profile contract. Bundling all -# three into one distribution collapses that dependency cycle into intra-wheel extras. +# Eval Author ships skills and no code, so it declares no entry points and the inherit +# below is a no-op kept for uniformity. It stays bundled because that is how the skills +# reach a customer through `nemo-platform[all]`. The cycle this entry once collapsed +# (Experimentalist importing EvalAuthor, Eval Author borrowing Experimentalist helpers) +# is gone; only the shared Insights profile contract remains. nemo-eval-author-plugin = { source = "../../plugins/nemo-eval-author/src/nemo_eval_author_plugin", module = "nemo_eval_author_plugin", inherit = { "entry-points" = ["nemo.*"] } } nemo-evaluator-plugin = { source = "../../plugins/nemo-evaluator/src/nemo_evaluator", module = "nemo_evaluator", inherit = { "entry-points" = ["nemo.*"] } } nemo-experimentalist-plugin = { source = "../../plugins/nemo-experimentalist/src/nemo_experimentalist_plugin", module = "nemo_experimentalist_plugin", inherit = { "entry-points" = ["nemo.*"] } } From 30b0d72fbb3f9f8d4736c7dd64d389112b256446 Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Thu, 20 Aug 2026 14:17:18 -0500 Subject: [PATCH 4/8] build(eval-author)!: stop shipping the skills in nemo-platform[all] The package has no entry points and no importable code, so bundling it into the platform distribution shipped files that nothing can discover. `nemo skills list` reads the `nemo.skills` registry and these skills are not registered there yet, so a customer installing nemo-platform[all] received two SKILL.md files reachable only by knowing a path inside the wheel. Installing them into service images through enabled-plugins had the same problem. Removes the [tool.bundle-package] entry, which is what generated the nemo-eval-author-plugin extra along with its membership in the plugins and all extras, and drops the package from enabled-plugins. Regenerating cleared every eval-author reference out of the published wrapper. The package stays a uv workspace member, so uv sync --all-packages still installs it for development and the contract test still runs. Bundle it again when the skills register under nemo.skills and a distribution has something to expose. Signed-off-by: Alec Khoury --- packages/nemo_platform/pyproject.toml | 17 ++++------------- pyproject.toml | 1 - uv.lock | 14 +------------- 3 files changed, 5 insertions(+), 27 deletions(-) diff --git a/packages/nemo_platform/pyproject.toml b/packages/nemo_platform/pyproject.toml index 216c51bcfd..b5639e9da0 100644 --- a/packages/nemo_platform/pyproject.toml +++ b/packages/nemo_platform/pyproject.toml @@ -291,12 +291,6 @@ nemo-deployments-plugin = [ "pyyaml>=6.0", ] -# Generated from [tool.bundle-package]; do not edit by hand. -nemo-eval-author-plugin = [ - "nemo-insights-plugin", - "pyyaml>=6.0.3", -] - # Generated from [tool.bundle-package]; do not edit by hand. nemo-evaluator-plugin = [ "cloudpickle>=3.1.1", @@ -484,7 +478,6 @@ plugins = [ "nemo-platform[nemo-auditor-plugin]", "nemo-platform[nemo-data-designer-plugin]", "nemo-platform[nemo-deployments-plugin]", - "nemo-platform[nemo-eval-author-plugin]", "nemo-platform[nemo-evaluator-plugin]", "nemo-platform[nemo-experimentalist-plugin]", "nemo-platform[nemo-guardrails-plugin]", @@ -723,12 +716,10 @@ nemo-anonymizer-plugin = { source = "../../plugins/nemo-anonymizer/src/nemo_anon nemo-auditor-plugin = { source = "../../plugins/nemo-auditor/src/nemo_auditor", module = "nemo_auditor", inherit = { "entry-points" = ["nemo.*"] } } nemo-data-designer-plugin = { source = "../../plugins/nemo-data-designer/src/nemo_data_designer_plugin", module = "nemo_data_designer_plugin", inherit = { "entry-points" = ["nemo.*"] } } nemo-deployments-plugin = { source = "../../plugins/nemo-deployments/src/nemo_deployments_plugin", module = "nemo_deployments_plugin", inherit = { "entry-points" = ["nemo.*"] } } -# Eval Author ships skills and no code, so it declares no entry points and the inherit -# below is a no-op kept for uniformity. It stays bundled because that is how the skills -# reach a customer through `nemo-platform[all]`. The cycle this entry once collapsed -# (Experimentalist importing EvalAuthor, Eval Author borrowing Experimentalist helpers) -# is gone; only the shared Insights profile contract remains. -nemo-eval-author-plugin = { source = "../../plugins/nemo-eval-author/src/nemo_eval_author_plugin", module = "nemo_eval_author_plugin", inherit = { "entry-points" = ["nemo.*"] } } +# Eval Author is deliberately absent. It ships skills and no code, declares no entry +# points, and has nothing a distribution can expose, so bundling it into +# `nemo-platform[all]` would ship files no mechanism can find. Add it back when the +# skills register under `nemo.skills`. nemo-evaluator-plugin = { source = "../../plugins/nemo-evaluator/src/nemo_evaluator", module = "nemo_evaluator", inherit = { "entry-points" = ["nemo.*"] } } nemo-experimentalist-plugin = { source = "../../plugins/nemo-experimentalist/src/nemo_experimentalist_plugin", module = "nemo_experimentalist_plugin", inherit = { "entry-points" = ["nemo.*"] } } nemo-insights-plugin = { source = "../../plugins/nemo-insights/src/nemo_insights_plugin", module = "nemo_insights_plugin", inherit = { "entry-points" = ["nemo.*"] } } diff --git a/pyproject.toml b/pyproject.toml index 526993027b..a9f1dfe338 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -187,7 +187,6 @@ enabled-plugins = [ "nemo-evaluator-plugin", "nemo-insights-plugin", "nemo-experimentalist-plugin ; python_full_version < '3.14'", - "nemo-eval-author-plugin ; python_full_version < '3.14'", "nemo-guardrails-plugin", "nemo-auditor-plugin", "nemo-safe-synthesizer-plugin", diff --git a/uv.lock b/uv.lock index 29383d3d69..1ed14f86e9 100644 --- a/uv.lock +++ b/uv.lock @@ -5120,10 +5120,6 @@ nemo-deployments-plugin = [ { name = "pydantic", extra = ["email"], marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "pyyaml", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, ] -nemo-eval-author-plugin = [ - { name = "nemo-insights-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "pyyaml", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, -] nemo-evaluator-plugin = [ { name = "cloudpickle", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-evaluator-sdk", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, @@ -5721,7 +5717,6 @@ requires-dist = [ { name = "nemo-fabric-adapters-hermes", marker = "python_full_version < '3.14' and extra == 'plugins'", git = "https://github.com/NVIDIA/NeMo-Fabric.git?subdirectory=adapters%2Fhermes&rev=e7353383024523179be6a009ef16dea223bea8c0" }, { name = "nemo-fabric-adapters-hermes", marker = "python_full_version < '3.14' and extra == 'services'", git = "https://github.com/NVIDIA/NeMo-Fabric.git?subdirectory=adapters%2Fhermes&rev=e7353383024523179be6a009ef16dea223bea8c0" }, { name = "nemo-insights-plugin", marker = "extra == 'all'", editable = "plugins/nemo-insights" }, - { name = "nemo-insights-plugin", marker = "extra == 'nemo-eval-author-plugin'", editable = "plugins/nemo-insights" }, { name = "nemo-insights-plugin", marker = "extra == 'nemo-experimentalist-plugin'", editable = "plugins/nemo-insights" }, { name = "nemo-insights-plugin", marker = "extra == 'plugins'", editable = "plugins/nemo-insights" }, { name = "nemo-insights-plugin", marker = "extra == 'services'", editable = "plugins/nemo-insights" }, @@ -6005,7 +6000,6 @@ requires-dist = [ { name = "pyyaml", marker = "extra == 'nemo-anonymizer-plugin'", specifier = ">=6.0.2" }, { name = "pyyaml", marker = "extra == 'nemo-auditor-plugin'", specifier = ">=6.0.2" }, { name = "pyyaml", marker = "extra == 'nemo-deployments-plugin'", specifier = ">=6.0" }, - { name = "pyyaml", marker = "extra == 'nemo-eval-author-plugin'", specifier = ">=6.0.3" }, { name = "pyyaml", marker = "extra == 'nemo-experimentalist-plugin'", specifier = ">=6.0.3" }, { name = "pyyaml", marker = "extra == 'nemo-insights-plugin'", specifier = ">=6.0.3" }, { name = "pyyaml", marker = "extra == 'nemo-optimization-plugin'", specifier = ">=6.0" }, @@ -6127,7 +6121,7 @@ requires-dist = [ { name = "yara-python", marker = "extra == 'guardrails-service'", specifier = "==4.5.1" }, { name = "yara-python", marker = "extra == 'services'", specifier = "==4.5.1" }, ] -provides-extras = ["aiohttp", "all", "auditor-service", "auth-service", "core-service", "data-designer-nemo", "entities-service", "files-service", "guardrails-service", "hello-world-service", "inference-gateway-service", "intake-service", "jobs-service", "models-service", "nemo-agents-example-calculator", "nemo-agents-plugin", "nemo-anonymizer-plugin", "nemo-auditor-plugin", "nemo-data-designer-plugin", "nemo-deployments-plugin", "nemo-eval-author-plugin", "nemo-evaluator-plugin", "nemo-evaluator-sdk", "nemo-experimentalist-plugin", "nemo-guardrails-plugin", "nemo-insights-plugin", "nemo-optimization-plugin", "nemo-platform-plugin", "nemo-platform-sdk", "nemo-safe-synthesizer-plugin", "nemo-switchyard", "nmp-common", "platform-seed-service", "plugins", "secrets-service", "services", "studio-service", "switchyard-vendored"] +provides-extras = ["aiohttp", "all", "auditor-service", "auth-service", "core-service", "data-designer-nemo", "entities-service", "files-service", "guardrails-service", "hello-world-service", "inference-gateway-service", "intake-service", "jobs-service", "models-service", "nemo-agents-example-calculator", "nemo-agents-plugin", "nemo-anonymizer-plugin", "nemo-auditor-plugin", "nemo-data-designer-plugin", "nemo-deployments-plugin", "nemo-evaluator-plugin", "nemo-evaluator-sdk", "nemo-experimentalist-plugin", "nemo-guardrails-plugin", "nemo-insights-plugin", "nemo-optimization-plugin", "nemo-platform-plugin", "nemo-platform-sdk", "nemo-safe-synthesizer-plugin", "nemo-switchyard", "nmp-common", "platform-seed-service", "plugins", "secrets-service", "services", "studio-service", "switchyard-vendored"] [[package]] name = "nemo-platform-ext" @@ -6734,7 +6728,6 @@ core-services = [ { name = "nemo-customizer-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-data-designer-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-deployments-plugin", extra = ["docker", "k8s"], marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-eval-author-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-evaluator-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-experimentalist-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-guardrails-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, @@ -6835,7 +6828,6 @@ enabled-plugins = [ { name = "nemo-customizer-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-data-designer-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-deployments-plugin", extra = ["docker", "k8s"], marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-eval-author-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-evaluator-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-experimentalist-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-guardrails-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, @@ -6858,7 +6850,6 @@ functional-services = [ { name = "nemo-customizer-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-data-designer-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-deployments-plugin", extra = ["docker", "k8s", "openshell"], marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "nemo-eval-author-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-evaluator-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-experimentalist-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-guardrails-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, @@ -6956,7 +6947,6 @@ core-services = [ { name = "nemo-customizer-plugin", editable = "plugins/nemo-customizer" }, { name = "nemo-data-designer-plugin", editable = "plugins/nemo-data-designer" }, { name = "nemo-deployments-plugin", extras = ["docker", "k8s"], editable = "plugins/nemo-deployments" }, - { name = "nemo-eval-author-plugin", marker = "python_full_version < '3.14'", editable = "plugins/nemo-eval-author" }, { name = "nemo-evaluator-plugin", editable = "plugins/nemo-evaluator" }, { name = "nemo-experimentalist-plugin", marker = "python_full_version < '3.14'", editable = "plugins/nemo-experimentalist" }, { name = "nemo-guardrails-plugin", editable = "plugins/nemo-guardrails" }, @@ -7060,7 +7050,6 @@ enabled-plugins = [ { name = "nemo-customizer-plugin", editable = "plugins/nemo-customizer" }, { name = "nemo-data-designer-plugin", editable = "plugins/nemo-data-designer" }, { name = "nemo-deployments-plugin", extras = ["docker", "k8s"], editable = "plugins/nemo-deployments" }, - { name = "nemo-eval-author-plugin", marker = "python_full_version < '3.14'", editable = "plugins/nemo-eval-author" }, { name = "nemo-evaluator-plugin", editable = "plugins/nemo-evaluator" }, { name = "nemo-experimentalist-plugin", marker = "python_full_version < '3.14'", editable = "plugins/nemo-experimentalist" }, { name = "nemo-guardrails-plugin", editable = "plugins/nemo-guardrails" }, @@ -7084,7 +7073,6 @@ functional-services = [ { name = "nemo-data-designer-plugin", editable = "plugins/nemo-data-designer" }, { name = "nemo-deployments-plugin", extras = ["docker", "k8s"], editable = "plugins/nemo-deployments" }, { name = "nemo-deployments-plugin", extras = ["openshell"], editable = "plugins/nemo-deployments" }, - { name = "nemo-eval-author-plugin", marker = "python_full_version < '3.14'", editable = "plugins/nemo-eval-author" }, { name = "nemo-evaluator-plugin", editable = "plugins/nemo-evaluator" }, { name = "nemo-experimentalist-plugin", marker = "python_full_version < '3.14'", editable = "plugins/nemo-experimentalist" }, { name = "nemo-guardrails-plugin", editable = "plugins/nemo-guardrails" }, From 435341f1123fe8b64b5af541f93c2212576883fb Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Thu, 20 Aug 2026 16:47:10 -0500 Subject: [PATCH 5/8] feat(eval-author): have the agent save a discovery report The CLI uploaded a discovery.md to a fileset, which the skills cannot do and should not: they talk to no platform service. Without a replacement, findings died with the run that produced them and the next reader had to redo discovery using the Harbor install the report exists to describe. The skill now tells the agent to save the report to .eval-author/discovery.md, leading with the JSON as front matter so a later model reads the verdict, the run command, and the required host variables from the file alone. The report stays visible and uncommitted: it is worth committing so a teammate skips the discovery pass, but that is the user's call, and the repository's .gitignore is not ours to edit. Saving is guidance rather than plumbing. The scripts write no files at all, so deciding where a file belongs in someone's repository stays a judgement made in the open. That let discover.py drop --out and its Markdown renderer, and the sub-flow trade its blanket no-writes grant for Write without Edit: create your own report, never rewrite anything that predates you. Signed-off-by: Alec Khoury --- plugins/nemo-eval-author/README.md | 9 ++++ .../skills/eval-author-discover/SKILL.md | 26 ++++++++---- .../eval-author-discover/scripts/discover.py | 41 ++----------------- .../skills/eval-author/SKILL.md | 9 ++-- .../tests/test_skill_contract.py | 34 ++++++++------- 5 files changed, 57 insertions(+), 62 deletions(-) diff --git a/plugins/nemo-eval-author/README.md b/plugins/nemo-eval-author/README.md index 69235ae5db..59df50cf95 100644 --- a/plugins/nemo-eval-author/README.md +++ b/plugins/nemo-eval-author/README.md @@ -12,6 +12,15 @@ repository. There is no CLI and no service. A customer points their agent at | [`eval-author`](src/nemo_eval_author_plugin/skills/eval-author/SKILL.md) | Core. Owns the standard every sub-flow follows and routes to one. | | [`eval-author-discover`](src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md) | Sub-flow. Records whether a repository's Harbor evals are ready to run. | +## Where findings go + +`eval-author-discover` leaves a report at `.eval-author/discovery.md`, carrying the +JSON as front matter so a later model reads the verdict without Harbor. It is +visible and worth committing: a teammate who reads it skips the discovery pass. + +The scripts write no files. They report to stdout and the skill tells the agent +where to save, because that is a judgement about someone's repository. + ## Why skills instead of an agent Harbor tasks live in the customer's repository, so an agent that proposes changes diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md index e047fe124d..c021c48704 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md @@ -12,7 +12,9 @@ description: >- to run an eval suite they did not write, hand a suite to a cheaper model, or asks "can I run these evals?", "why won't my Harbor config resolve?", "which env vars does this suite need?", "where are the evals in this repo?", or "why - did Harbor skip my task?". Reads the repository and writes nothing to it. + did Harbor skip my task?". Changes none of your source, and leaves behind + `.eval-author/discovery.md` so your team and the next model read the verdict + without Harbor and without discovering again. triggers: - can I run the evals in this repo - where are the Harbor evals in this repository @@ -31,7 +33,7 @@ compatibility: >- maturity: alpha license: Apache-2.0 user-invocable: true -allowed-tools: [Bash, Read, Grep, Glob] +allowed-tools: [Bash, Read, Write, Grep, Glob] --- # Eval Author: discover @@ -83,8 +85,8 @@ configs to a depth of four directories and finds datasets at any depth. .venv/bin/python /scripts/discover.py --repo . ``` -One JSON object goes to stdout. Add `--out discovery.md` to also write a Markdown -report, and `--compact` for single-line JSON. +One JSON object goes to stdout, and `--compact` puts it on one line. The script +writes no files; you save the report in **Step 5**. The exit code carries the verdict, so check it: @@ -132,8 +134,8 @@ rung's failure often disappears once you fix a higher one. ## Step 4: verify before you report -Discovery writes nothing to the repository, so verification means confirming the -report describes the repository the user meant: +Discovery changes none of the user's source, so verification means confirming the +report describes the repository they meant: 1. `proven` is `true`. When it is `false`, report only that Harbor is missing. 2. `repo_root` is the repository they named. @@ -142,10 +144,20 @@ report describes the repository the user meant: declare no `datasets` or `tasks` list. 4. `task_count` is in the range they expect. A count of zero with a passing `tasks` check means the config resolves tasks from a registry, not from disk. - Report `proven`, `runnable`, and the failing check names. Never describe a suite as ready to run while `runnable` is `false`. +## Step 5: save the report + +Write the report to `.eval-author/discovery.md`, so the next model and the user's +teammates inherit the findings instead of rerunning discovery to get them back. +Lead with the JSON as front matter, verbatim, then the verdict, the failing checks +by name, and the run command. Never paraphrase a check; its wording is the evidence. + +Leave the file in the working tree and say where it is. Committing it is the user's +call, and worth suggesting. Do not touch their `.gitignore`. A rerun replaces the +file rather than merging into it. + ## Files in this skill Provider-specific code sits under `scripts/providers/`, so support for a second diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py index 0832ddad87..16caa75fc4 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py @@ -32,13 +32,13 @@ construction. PyYAML, pydantic, and toml arrive with it. Usage: - discover.py [--repo PATH] [--out PATH] [--compact] + discover.py [--repo PATH] [--compact] --repo PATH Repository to inspect. Defaults to the working directory. - --out PATH Also write the Markdown report to PATH. --compact Emit single-line JSON. -Prints a JSON report on stdout. +Prints a JSON report on stdout and writes nothing. ``SKILL.md`` tells the agent +where to save it. Exit codes: 0 every repository-owned config passed every required check @@ -63,7 +63,7 @@ # machine without Harbor and make the probe claim an install that is not there. sys.path.insert(0, str(Path(__file__).resolve().parent)) -from _checks import CheckResult, format_report, required_failures # noqa: E402 +from _checks import CheckResult, required_failures # noqa: E402 from providers.harbor import _probe # noqa: E402 from providers.harbor._inventory import RepositoryScan, scan_repository # noqa: E402 @@ -141,33 +141,6 @@ def _run_command(repo_root: Path, configs: list[dict]) -> str | None: return "cd {} && harbor job start -c {}".format(repo_root, runnable[0]["path"]) -def _render_markdown(report: dict, grouped: list[CheckResult]) -> str: - """Render the report as Markdown with a status block per config.""" - lines = ["# Discovery report for `{}`".format(report["repo_root"])] - lines.extend(["", "Proven by Harbor: {}".format("yes" if report["proven"] else "no")]) - # Validation checks belong to a config, so they render under that config. - status = format_report([result for result in grouped if result.group != "validation"]) - if status: - lines.extend(["", "```text", status, "```"]) - if report["configs"]: - lines.extend(["", "## Harbor entrypoints"]) - for config in report["configs"]: - lines.extend( - [ - "", - "### `{}` (`{}`)".format(config["name"], config["path"]), - "", - "Runnable: {}".format("true" if config["runnable"] else "false"), - ] - ) - if config["checks"]: - rendered = format_report([CheckResult(**item) for item in config["checks"]]) - lines.extend(["", "```text", rendered, "```"]) - if report["run_command"]: - lines.extend(["", "```bash", report["run_command"], "```"]) - return "\n".join(lines) + "\n" - - def _fail(message: str, hint: str) -> int: json.dump({"error": message, "hint": hint}, sys.stdout, indent=2) sys.stdout.write("\n") @@ -177,7 +150,6 @@ def _fail(message: str, hint: str) -> int: async def _main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser(description="Record whether a repository's Harbor evaluations are ready to run.") parser.add_argument("--repo", type=Path, default=Path(), help="Repository to inspect.") - parser.add_argument("--out", type=Path, default=None, help="Also write the Markdown report to this path.") parser.add_argument("--compact", action="store_true", help="Emit single-line JSON.") args = parser.parse_args(argv) @@ -227,11 +199,6 @@ async def _main(argv: list[str] | None = None) -> int: } report["run_command"] = _run_command(repo_root, configs) - if args.out is not None: - args.out.parent.mkdir(parents=True, exist_ok=True) - args.out.write_text(_render_markdown(report, grouped), encoding="utf-8") - report["report_path"] = args.out.as_posix() - json.dump(report, sys.stdout, indent=None if args.compact else 2) sys.stdout.write("\n") return 0 if runnable else 1 diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md index fc3387cf48..f464273b96 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md +++ b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md @@ -12,7 +12,7 @@ description: >- "help me with my evals", "what's the state of the eval suite here?", "are these evals any good?", "I inherited this repo and there are Harbor tasks in it", or when you need to pick between the Eval Author sub-flows. Routes to a sub-flow; - reads the repository and changes nothing in it. + changes none of your source, and saves what it finds under `.eval-author/`. triggers: - help me with the evals in this repo - what is the state of the eval suite here @@ -91,9 +91,10 @@ discover. These hold for every sub-flow. They exist because the repository belongs to the user, not to you. -- **Propose, never mutate.** Read the repository and report. Do not create, edit, - move, or reformat anything in it. A sub-flow that writes a report file writes it - only where the user pointed. +- **Propose, never mutate.** Read the user's source and report on it. Do not edit, + move, or reformat any of it, including its `.gitignore`. The one thing you add is + your own report under `.eval-author/`, which is theirs to commit or ignore. The + bundled scripts write nothing at all; saving is your job, not theirs. - **A missing tool is a finding, not a task.** When the provider is not installed, report that and stop. Do not install it into the user's environment. - **Do not run the suite.** Prove it can run and hand over the command. Starting a diff --git a/plugins/nemo-eval-author/tests/test_skill_contract.py b/plugins/nemo-eval-author/tests/test_skill_contract.py index 273760055c..db10a30739 100644 --- a/plugins/nemo-eval-author/tests/test_skill_contract.py +++ b/plugins/nemo-eval-author/tests/test_skill_contract.py @@ -24,6 +24,9 @@ - The check contract matches ``nemo_insights_plugin.contracts.checks``, so a report from the skill reads the same as one from the platform command. - Discovery reports a valid suite runnable and names the rung a broken one fails. +- The bundled scripts write no files. ``SKILL.md`` tells the agent where to save + the report, because where a file belongs in someone's repository is a judgement + rather than a fact about their evals. These tests compare the skill against this repository, never against Harbor's rules. The skill reimplements no Harbor rule: it asks Harbor for every verdict, @@ -178,22 +181,27 @@ def test_frontmatter_carries_every_required_field(skill_dir: Path) -> None: @pytest.mark.parametrize("skill_dir", _SKILL_DIRS, ids=lambda path: path.name) -def test_no_skill_grants_write_access(skill_dir: Path) -> None: - """Eval Author proposes and never mutates, so a Write grant would contradict the core.""" +def test_no_skill_can_edit_what_it_did_not_write(skill_dir: Path) -> None: + """Eval Author creates its own report and changes nothing that was already there. + + ``Write`` covers the discovery report, which is the one artifact a sub-flow + leaves behind. ``Edit`` would let it rewrite files that predate it, which is the + permission customers declined to grant and the reason these ship as skills. + """ frontmatter, _ = _frontmatter_and_body(skill_dir) tools = set(frontmatter["allowed-tools"]) - assert not {"Write", "Edit", "MultiEdit", "NotebookEdit"} & tools, ( - f"{skill_dir.name} writes nothing, so {sorted(tools)} is too broad" + assert not {"Edit", "MultiEdit", "NotebookEdit"} & tools, ( + f"{skill_dir.name} edits nothing that predates it, so {sorted(tools)} is too broad" ) def test_the_core_routes_and_the_sub_flow_executes() -> None: - """The core only picks a sub-flow, so it has no reason to run anything.""" + """The core only picks a sub-flow, so it neither runs nor saves anything.""" core_tools = set(_frontmatter_and_body(_CORE_DIR)[0]["allowed-tools"]) - assert "Bash" not in core_tools, f"the core routes and explains; {sorted(core_tools)} lets it execute" + assert not {"Bash", "Write"} & core_tools, f"the core routes and explains; {sorted(core_tools)} is too broad" for skill_dir in _SUB_FLOW_DIRS: tools = set(_frontmatter_and_body(skill_dir)[0]["allowed-tools"]) - assert "Bash" in tools, f"{skill_dir.name} runs a bundled script, which needs Bash" + assert {"Bash", "Write"} <= tools, f"{skill_dir.name} runs a script and saves a report; it has {sorted(tools)}" def test_the_core_names_every_sub_flow() -> None: @@ -429,15 +437,13 @@ def test_discover_finds_configs_without_pyyaml(suite: Path) -> None: assert "harbor-job.yaml" in parse["message"] -def test_discover_writes_a_markdown_report_only_where_asked(suite: Path, tmp_path: Path) -> None: - out = tmp_path / "reports" / "discovery.md" +def test_discovery_writes_no_files(suite: Path) -> None: + """The scripts report and the agent saves, which is what makes the skill safe to run.""" + before = {path.relative_to(suite).as_posix() for path in suite.rglob("*")} - _run_discover(suite, "--out", str(out)) + _run_discover(suite) - assert out.is_file(), "--out must create the report and its parent directory" - text = out.read_text(encoding="utf-8") - assert text.startswith("# Discovery report for") - assert "Proven by Harbor: yes" in text + assert {path.relative_to(suite).as_posix() for path in suite.rglob("*")} == before def test_discover_fails_with_a_hint_when_the_path_is_missing(tmp_path: Path) -> None: From c2e453869365eb053af59ab286eeea99f3c1534a Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Fri, 21 Aug 2026 13:52:53 -0500 Subject: [PATCH 6/8] refactor(eval-author)!: move the skills up and cut the platform import The `src//skills/` layout exists so a plugin's skills can be imported and shipped through the `nemo.skills` entry point, which Eval Author deliberately does not use. Once the agent code moved to Experimentalist, that tree held a single `py.typed` marker for a package with no modules, so the skills move to the plugin root and the directory stops building a package at all. Being a non-package drops it from the experimentalist dependency group and from `uv.sources`, and it takes two pieces of now-dead config with it: a `ty` source path that no longer exists and an `empty-body` override scoped to nooa agent classes that left in an earlier commit. The contract test also imported `nemo_insights_plugin`, for a drift guard that compared the bundled check contract against the platform's. That guard existed so a skill report would read like one from the Eval Author CLI, which this branch deletes, so it goes as well. The five tests that make Harbor judge a fixture suite now skip when Harbor is absent rather than failing, which retires the root test-discovery exclusion: the suite runs on pytest and PyYAML alone, matching the boundary the bundled scripts already hold. Signed-off-by: Alec Khoury --- plugins/README.md | 1 - plugins/nemo-eval-author/README.md | 23 ++++++---- plugins/nemo-eval-author/pyproject.toml | 21 +++------ .../skills/eval-author-discover/SKILL.md | 0 .../eval-author-discover/scripts/_checks.py | 7 ++- .../eval-author-discover/scripts/discover.py | 0 .../scripts/providers/harbor/_inventory.py | 6 +-- .../scripts/providers/harbor/_ladder.py | 5 +-- .../scripts/providers/harbor/_probe.py | 0 .../skills/eval-author/SKILL.md | 0 .../src/nemo_eval_author_plugin/py.typed | 0 .../tests/test_skill_contract.py | 43 ++++++++----------- pyproject.toml | 22 +++------- tests/discovery_exclusions.py | 6 --- uv.lock | 18 +------- 15 files changed, 54 insertions(+), 98 deletions(-) rename plugins/nemo-eval-author/{src/nemo_eval_author_plugin => }/skills/eval-author-discover/SKILL.md (100%) rename plugins/nemo-eval-author/{src/nemo_eval_author_plugin => }/skills/eval-author-discover/scripts/_checks.py (90%) rename plugins/nemo-eval-author/{src/nemo_eval_author_plugin => }/skills/eval-author-discover/scripts/discover.py (100%) rename plugins/nemo-eval-author/{src/nemo_eval_author_plugin => }/skills/eval-author-discover/scripts/providers/harbor/_inventory.py (97%) rename plugins/nemo-eval-author/{src/nemo_eval_author_plugin => }/skills/eval-author-discover/scripts/providers/harbor/_ladder.py (98%) rename plugins/nemo-eval-author/{src/nemo_eval_author_plugin => }/skills/eval-author-discover/scripts/providers/harbor/_probe.py (100%) rename plugins/nemo-eval-author/{src/nemo_eval_author_plugin => }/skills/eval-author/SKILL.md (100%) delete mode 100644 plugins/nemo-eval-author/src/nemo_eval_author_plugin/py.typed diff --git a/plugins/README.md b/plugins/README.md index ac74f9a4a3..cbfd561dae 100644 --- a/plugins/README.md +++ b/plugins/README.md @@ -64,7 +64,6 @@ The package name is the `name` field in the plugin's `pyproject.toml`, not the d | `nemo-agents/` | `nemo-agents-plugin` | | `nemo-anonymizer/` | `nemo-anonymizer-plugin` | | `nemo-data-designer/` | `nemo-data-designer-plugin` | -| `nemo-eval-author/` | `nemo-eval-author-plugin` | | `nemo-evaluator/` | `nemo-evaluator-plugin` | | `nemo-experimentalist/` | `nemo-experimentalist-plugin` | | `nemo-guardrails/` | `nemo-guardrails-plugin` | diff --git a/plugins/nemo-eval-author/README.md b/plugins/nemo-eval-author/README.md index 59df50cf95..c0c3706ff3 100644 --- a/plugins/nemo-eval-author/README.md +++ b/plugins/nemo-eval-author/README.md @@ -4,13 +4,14 @@ # NeMo Eval Author Two skills that an agent reads to work on the evaluation suites in a user's own -repository. There is no CLI and no service. A customer points their agent at -`skills/` and nothing gets installed. +repository. There is no CLI, no service, and no importable code, so this directory +builds no package at all. A customer points their agent at `skills/` and nothing +gets installed. | Skill | Role | | --- | --- | -| [`eval-author`](src/nemo_eval_author_plugin/skills/eval-author/SKILL.md) | Core. Owns the standard every sub-flow follows and routes to one. | -| [`eval-author-discover`](src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md) | Sub-flow. Records whether a repository's Harbor evals are ready to run. | +| [`eval-author`](skills/eval-author/SKILL.md) | Core. Owns the standard every sub-flow follows and routes to one. | +| [`eval-author-discover`](skills/eval-author-discover/SKILL.md) | Sub-flow. Records whether a repository's Harbor evals are ready to run. | ## Where findings go @@ -26,7 +27,7 @@ where to save, because that is a judgement about someone's repository. Harbor tasks live in the customer's repository, so an agent that proposes changes has to write to that repository. Customers were unwilling to grant that, sandboxed or not. A skill inverts the arrangement: the customer's own agent does the work, -and this package only supplies the instructions and the deterministic scripts. +and this directory only supplies the instructions and the deterministic scripts. The Eval Author agent that Experimentalist insight mode still uses lives in [the Experimentalist plugin](../nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md). @@ -39,7 +40,11 @@ the skill defers to the provider's own validators rather than guessing from file layout, which is why `eval-author-discover` probes for an installed Harbor and asks Harbor to judge each config. -The two declared dependencies serve `tests/test_skill_contract.py`, which reads the -skills with `pyyaml` and checks them against the platform's check helpers. Adding a -runtime dependency to a bundled script is a breaking change for anyone who copied -the skill, so the contract test guards against it. +`tests/test_skill_contract.py` holds to the same boundary and imports nothing from +the platform, so `pytest` and `pyyaml` are enough to run it. The five tests that +make Harbor judge a fixture suite skip when Harbor is absent, which is why this +directory declares no dependencies and appears in no dependency group. + +Adding a runtime dependency to a bundled script is a breaking change for anyone who +copied the skill, so the contract test walks each script's imports and fails on +anything outside the standard library, a sibling module, or Harbor. diff --git a/plugins/nemo-eval-author/pyproject.toml b/plugins/nemo-eval-author/pyproject.toml index b5a5055722..a32ab7fa65 100644 --- a/plugins/nemo-eval-author/pyproject.toml +++ b/plugins/nemo-eval-author/pyproject.toml @@ -6,21 +6,14 @@ name = "nemo-eval-author-plugin" version = "0.1.0" description = "Eval Author skills that an agent reads to discover a repository's Harbor eval setup." requires-python = ">=3.12,<3.14" -# The bundled skill scripts run on the standard library alone, so a customer needs -# no install to use them. These two are for the contract test: it reads the skill -# with pyyaml and checks it against the platform's own check helpers. -dependencies = [ - "nemo-insights-plugin", - "pyyaml>=6.0.3", -] -[build-system] -requires = ["hatchling"] -build-backend = "hatchling.build" - -[tool.hatch.build.targets.wheel] -packages = ["src/nemo_eval_author_plugin"] +[tool.uv] +# Nothing here is importable. The directory holds two skills and the test that guards +# them, so there is no package to build and nothing to install. That is also why the +# skills sit at the top level instead of under `src//skills/`: that layout exists +# to make skills importable and shippable through the `nemo.skills` entry point, and this +# plugin deliberately does not plug into skill discovery yet. +package = false [tool.pytest.ini_options] -pythonpath = ["src"] testpaths = ["tests"] diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md b/plugins/nemo-eval-author/skills/eval-author-discover/SKILL.md similarity index 100% rename from plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/SKILL.md rename to plugins/nemo-eval-author/skills/eval-author-discover/SKILL.md diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/_checks.py b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/_checks.py similarity index 90% rename from plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/_checks.py rename to plugins/nemo-eval-author/skills/eval-author-discover/scripts/_checks.py index d744b9420a..eb492340ff 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/_checks.py +++ b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/_checks.py @@ -3,10 +3,9 @@ """Readiness result construction and presentation. -A standard-library port of ``nemo_insights_plugin.contracts.checks``, which the -platform-side discovery command uses. The field names, statuses, severities, and -rendered symbols match, so a report produced by the skill reads the same as one -produced by the CLI. Keep them aligned when either side changes. +One check is one named verdict, and a report is a list of them. Statuses and +severities are plain strings rather than enums, so the JSON a report emits needs no +conversion step and stays readable to whatever reads it next. Uses ``dataclass`` rather than ``pydantic.BaseModel`` so this module carries no dependency of its own. diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/discover.py similarity index 100% rename from plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/discover.py rename to plugins/nemo-eval-author/skills/eval-author-discover/scripts/discover.py diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_inventory.py b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_inventory.py similarity index 97% rename from plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_inventory.py rename to plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_inventory.py index 8984c9c07d..2d9b773830 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_inventory.py +++ b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_inventory.py @@ -6,10 +6,8 @@ Standard library only, and safe to import when Harbor is absent, so an inventory survives to orient in a repository the ladder cannot judge. -A standard-library port of the repository scan in -``nemo_eval_author_plugin/discovery/scan.py``, with the platform reads removed: -no client, no workspace, no Intake trace probe, and the agent doctrine comes from -a local ``ETHOS.md`` rather than a downloaded ``AGENT-SPEC.md``. +Everything is read from the local checkout: no client, no workspace, no trace +probe, and the agent doctrine comes from a local ``ETHOS.md``. Everything here observes rather than proves. Finding a config file says nothing about whether Harbor accepts it, which is why the ladder in ``_ladder.py`` runs diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_ladder.py b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_ladder.py similarity index 98% rename from plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_ladder.py rename to plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_ladder.py index 51b1b3adbc..d5e50521c0 100644 --- a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_ladder.py +++ b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_ladder.py @@ -3,9 +3,8 @@ """Make Harbor judge a repository-owned config. -A port of ``nemo_eval_author_plugin/discovery/validate.py``. Every rung asks -Harbor's own validators for a verdict, so each recorded fact is proved rather -than observed. Nothing here reimplements a Harbor rule. +Every rung asks Harbor's own validators for a verdict, so each recorded fact is +proved rather than observed. Nothing here reimplements a Harbor rule. This module imports Harbor at module scope. Import it only after ``_probe`` reports Harbor available, so that a repository without Harbor still gets an inventory diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_probe.py b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_probe.py similarity index 100% rename from plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author-discover/scripts/providers/harbor/_probe.py rename to plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_probe.py diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md b/plugins/nemo-eval-author/skills/eval-author/SKILL.md similarity index 100% rename from plugins/nemo-eval-author/src/nemo_eval_author_plugin/skills/eval-author/SKILL.md rename to plugins/nemo-eval-author/skills/eval-author/SKILL.md diff --git a/plugins/nemo-eval-author/src/nemo_eval_author_plugin/py.typed b/plugins/nemo-eval-author/src/nemo_eval_author_plugin/py.typed deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/plugins/nemo-eval-author/tests/test_skill_contract.py b/plugins/nemo-eval-author/tests/test_skill_contract.py index db10a30739..c795f23e49 100644 --- a/plugins/nemo-eval-author/tests/test_skill_contract.py +++ b/plugins/nemo-eval-author/tests/test_skill_contract.py @@ -21,8 +21,6 @@ - No bundled directory is named after a provider package. ``scripts/harbor/`` would be importable as ``harbor``, which makes ``find_spec`` succeed on a machine with no Harbor and the probe claim an install that is not there. -- The check contract matches ``nemo_insights_plugin.contracts.checks``, so a - report from the skill reads the same as one from the platform command. - Discovery reports a valid suite runnable and names the rung a broken one fails. - The bundled scripts write no files. ``SKILL.md`` tells the agent where to save the report, because where a file belongs in someone's repository is a judgement @@ -31,21 +29,25 @@ These tests compare the skill against this repository, never against Harbor's rules. The skill reimplements no Harbor rule: it asks Harbor for every verdict, so a Harbor change that tightens a rule flows through without a test change here. + +Nothing here imports the platform, for the same reason the bundled scripts do not: +these skills are copied into someone else's repository and have to stand alone. The +tests that make Harbor judge a fixture suite skip when Harbor is absent, so the whole +file runs against nothing but pytest and PyYAML. """ import ast -import importlib.util import json import re import subprocess import sys +from importlib.util import find_spec from pathlib import Path import pytest import yaml -from nemo_insights_plugin.contracts import checks as platform_checks -_SKILLS_DIR = Path(__file__).resolve().parents[1] / "src" / "nemo_eval_author_plugin" / "skills" +_SKILLS_DIR = Path(__file__).resolve().parents[1] / "skills" _CORE_DIR = _SKILLS_DIR / "eval-author" _FLOW_DIR = _SKILLS_DIR / "eval-author-discover" _SKILL_DIRS = (_CORE_DIR, _FLOW_DIR) @@ -71,6 +73,12 @@ # reference to eval-author-discover cannot pass for a reference to eval-author. _CORE_REFERENCE = re.compile(rf"\b{re.escape(_CORE_DIR.name)}\b(?!-)") +# Only the tests that make Harbor judge a fixture suite need Harbor. Skipping rather +# than failing is what lets this file run wherever the skills themselves run. +_needs_harbor = pytest.mark.skipif( + find_spec("harbor") is None, reason="Harbor is not installed, so it can judge nothing" +) + # Harbor brings these in, so a bundled script may name them. Nothing else outside # the standard library may appear. _PERMITTED_THIRD_PARTY = frozenset({"harbor", "pydantic", "yaml"}) @@ -304,26 +312,7 @@ def test_discover_defers_the_ladder_import_to_call_time() -> None: assert not named, f"discover.py imports {named} at module scope; move it inside a function, after the probe" -def test_check_contract_matches_the_platform() -> None: - """Drift guard. Both sides must render the same statuses and severities.""" - spec = importlib.util.spec_from_file_location("_skill_checks", _SCRIPTS_DIR / "_checks.py") - assert spec is not None and spec.loader is not None - skill_checks = importlib.util.module_from_spec(spec) - # ``dataclass`` resolves its module through ``sys.modules``, so register the - # module before executing it. - sys.modules[spec.name] = skill_checks - try: - spec.loader.exec_module(skill_checks) - finally: - sys.modules.pop(spec.name, None) - - assert {skill_checks.PASS, skill_checks.WARN, skill_checks.FAIL} == set(platform_checks.CheckStatus.__args__) - assert {skill_checks.REQUIRED, skill_checks.ADVISORY} == set(platform_checks.CheckSeverity.__args__) - platform_fields = set(platform_checks.CheckResult.model_fields) - skill_fields = set(skill_checks.CheckResult.__dataclass_fields__) - assert platform_fields <= skill_fields, f"the skill dropped platform check fields {platform_fields - skill_fields}" - - +@_needs_harbor def test_discover_proves_a_valid_suite_runnable(suite: Path) -> None: code, report = _run_discover(suite) @@ -345,6 +334,7 @@ def test_discover_proves_a_valid_suite_runnable(suite: Path) -> None: assert report["run_command"] == f"cd {suite} && harbor job start -c harbor-job.yaml" +@_needs_harbor def test_discover_names_the_rung_a_broken_config_fails(suite: Path) -> None: """A dataset path that does not exist must fail resolution, not schema.""" (suite / "harbor-job.yaml").write_text( @@ -362,6 +352,7 @@ def test_discover_names_the_rung_a_broken_config_fails(suite: Path) -> None: assert "no-such-dataset" in resolution["message"] +@_needs_harbor def test_discover_names_an_unknown_agent(suite: Path) -> None: (suite / "harbor-job.yaml").write_text( "job_name: fixture\ndatasets:\n - path: ./dataset\nagents:\n - name: no-such-agent\n", @@ -376,6 +367,7 @@ def test_discover_names_an_unknown_agent(suite: Path) -> None: assert "no-such-agent" in agent["message"] +@_needs_harbor def test_discover_reports_required_host_variables(suite: Path) -> None: task_toml = suite / "dataset" / "task-one" / "task.toml" task_toml.write_text( @@ -391,6 +383,7 @@ def test_discover_reports_required_host_variables(suite: Path) -> None: assert [item["name"] for item in report["configs"][0]["required_env_vars"]] == ["ACME_API_KEY"] +@_needs_harbor def test_discover_reports_a_task_harbor_silently_dropped(suite: Path) -> None: """Harbor skips an unparseable task without raising, which coverage must catch.""" broken = suite / "dataset" / "task-two" diff --git a/pyproject.toml b/pyproject.toml index a9f1dfe338..d8293deb62 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,14 +71,10 @@ version = "0.0.0" [dependency-groups] # Insights analyst plugin convenience group. insights = ["nemo-insights-plugin"] -# Experimentalist and Eval Author support Python 3.12 and 3.13. Eval Author ships skills -# rather than code, and imports nothing from Experimentalist. It is grouped here so that -# installing Experimentalist also makes the Eval Author tests visible to root test -# discovery (see tests/discovery_exclusions.py). -experimentalist = [ - "nemo-experimentalist-plugin ; python_full_version < '3.14'", - "nemo-eval-author-plugin ; python_full_version < '3.14'", -] +# Experimentalist supports Python 3.12 and 3.13. Eval Author is absent on purpose: it +# ships isolated skills rather than code, builds no package, and its tests need nothing +# beyond pytest and PyYAML. +experimentalist = ["nemo-experimentalist-plugin ; python_full_version < '3.14'"] dev = [ "pre-commit>=3.8.0", "pydantic-settings>=2.6.1", @@ -393,7 +389,6 @@ nmp-testing = { workspace = true } nmp-build-tools = { workspace = true } nemo-platform-plugin = { workspace = true } nemo-insights-plugin = { workspace = true } -nemo-eval-author-plugin = { workspace = true } nemo-experimentalist-plugin = { workspace = true } # Temporary until Fabric 0.2.0 publishes (mid-Aug): pin the monorepo SHA that lands @@ -616,8 +611,6 @@ extra-paths = [ "plugins/nemo-evaluator/src", # Source path for the Insights analyst plugin. "plugins/nemo-insights/src", - # Source path for the Eval Author plugin. - "plugins/nemo-eval-author/src", # Source path for Experimentalist; its tests also import the benchmark runner. "plugins/nemo-experimentalist/src", # Agentic-use tests place both tests/agentic-use and tests/agentic-use/shared @@ -687,12 +680,9 @@ exclude = [ # nooa agents declare their LLM-generated methods as annotated signatures with an # ellipsis body; the framework fills them in at runtime. ty reads that as a missing -# return, so the rule is off for the Experimentalist and Eval Author agent classes only. +# return, so the rule is off for the Experimentalist agent classes only. [[tool.ty.overrides]] -include = [ - "plugins/nemo-experimentalist/src/**", - "plugins/nemo-eval-author/src/**", -] +include = ["plugins/nemo-experimentalist/src/**"] [tool.ty.overrides.rules] empty-body = "ignore" diff --git a/tests/discovery_exclusions.py b/tests/discovery_exclusions.py index 7e2e609b76..bb594388d6 100644 --- a/tests/discovery_exclusions.py +++ b/tests/discovery_exclusions.py @@ -19,9 +19,3 @@ TEST_DISCOVERY_EXCLUSIONS[Path("plugins/nemo-experimentalist/tests")] = ( "The optional experimentalist dependency group is not installed in the root test environment." ) - -if find_spec("nemo_eval_author_plugin") is None: - TEST_DISCOVERY_EXCLUSIONS[Path("plugins/nemo-eval-author/tests")] = ( - "Eval Author ships in the optional experimentalist dependency group, which is not installed " - "in the root test environment." - ) diff --git a/uv.lock b/uv.lock index 1ed14f86e9..c3a5cd6159 100644 --- a/uv.lock +++ b/uv.lock @@ -4350,17 +4350,7 @@ dev = [ [[package]] name = "nemo-eval-author-plugin" version = "0.1.0" -source = { editable = "plugins/nemo-eval-author" } -dependencies = [ - { name = "nemo-insights-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, - { name = "pyyaml", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, -] - -[package.metadata] -requires-dist = [ - { name = "nemo-insights-plugin", editable = "plugins/nemo-insights" }, - { name = "pyyaml", specifier = ">=6.0.3" }, -] +source = { virtual = "plugins/nemo-eval-author" } [[package]] name = "nemo-evaluator-plugin" @@ -6839,7 +6829,6 @@ enabled-plugins = [ { name = "nemo-unsloth-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, ] experimentalist = [ - { name = "nemo-eval-author-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, { name = "nemo-experimentalist-plugin", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, ] functional-services = [ @@ -7060,10 +7049,7 @@ enabled-plugins = [ { name = "nemo-switchyard", editable = "plugins/nemo-switchyard" }, { name = "nemo-unsloth-plugin", editable = "plugins/nemo-unsloth" }, ] -experimentalist = [ - { name = "nemo-eval-author-plugin", marker = "python_full_version < '3.14'", editable = "plugins/nemo-eval-author" }, - { name = "nemo-experimentalist-plugin", marker = "python_full_version < '3.14'", editable = "plugins/nemo-experimentalist" }, -] +experimentalist = [{ name = "nemo-experimentalist-plugin", marker = "python_full_version < '3.14'", editable = "plugins/nemo-experimentalist" }] functional-services = [ { name = "nemo-agents-plugin", editable = "plugins/nemo-agents" }, { name = "nemo-anonymizer-plugin", editable = "plugins/nemo-anonymizer" }, From 451f81db018233b37a21749ca6bdbd45016cf411 Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Fri, 21 Aug 2026 14:36:38 -0500 Subject: [PATCH 7/8] fix(eval-author): keep discovery alive on files it cannot read Discovery crashed on three inputs a customer repository can hold, and each one took the whole run down with a traceback and no JSON at all, so the agent got nothing rather than the unproven report the skill promises. A malformed YAML file was the worst of them. Every .yaml file within four directories reaches yaml.safe_load, and PyYAML raises yaml.YAMLError, which is not a ValueError and so escaped the handler in _candidate: one broken template anywhere in the tree ended the run. Catching it and skipping the file would trade the crash for a misleading report, because a harbor-job.yaml with a syntax error would then be reported as no config at all, hinting that the user add one they already have. The handler instead falls through to the top-level key scan that already runs when PyYAML is absent, so a broken config surfaces through config-parse while a broken file naming no Harbor work is still ignored. An unreadable ETHOS.md raised OSError from read_bytes. It is advisory input, so it now warns and stays out of the fingerprint rather than costing the report. An unreadable file inside a dataset directory aborted the fingerprint after every check had already been built; each file now hashes on its own through hashlib.file_digest, which skips an unreadable one atomically instead of contributing the bytes read before the failure, and keeps a large repository-owned dataset out of memory. The resolution rung could also report a failure that did not happen. job._close_logger_handlers is cleanup on a private API and shared the try that guards Job.create, so a rename in Harbor would have claimed Harbor could not resolve the job after resolution succeeded. It is suppressed now, and stays ahead of the scratch directory removal it exists to precede. Two hints named PyYAML unconditionally, which is the wrong instruction once a malformed file can reach them: with Harbor installed PyYAML is present and the syntax is at fault. The check table gained harbor-cli and compatibility, which the probe and the ladder emit but no row explained, and dataset paths now come from the task list rather than from a second walk of the repository under the same predicate. Each crash has a test that fails on the previous code with the crash it predicts. The no-write test compares file contents now, because comparing path names cannot catch an in-place rewrite, which is the thing it promises does not happen. Found by CodeRabbit review on this branch. Signed-off-by: Alec Khoury --- plugins/nemo-eval-author/README.md | 10 +- .../skills/eval-author-discover/SKILL.md | 6 +- .../scripts/providers/harbor/_inventory.py | 95 ++++++++++++++----- .../scripts/providers/harbor/_ladder.py | 9 +- .../skills/eval-author/SKILL.md | 3 +- .../tests/test_skill_contract.py | 87 ++++++++++++++++- 6 files changed, 172 insertions(+), 38 deletions(-) diff --git a/plugins/nemo-eval-author/README.md b/plugins/nemo-eval-author/README.md index c0c3706ff3..913cdbde86 100644 --- a/plugins/nemo-eval-author/README.md +++ b/plugins/nemo-eval-author/README.md @@ -34,11 +34,11 @@ The Eval Author agent that Experimentalist insight mode still uses lives in ## Dependencies -The scripts under `skills/*/scripts/` import the standard library only, so they run -on whatever Python the customer already has. Where a real answer needs a provider, -the skill defers to the provider's own validators rather than guessing from file -layout, which is why `eval-author-discover` probes for an installed Harbor and asks -Harbor to judge each config. +The scripts under `skills/*/scripts/` import nothing beyond the standard library and +Harbor itself, so they run on whatever Python the customer already has. Where a real +answer needs a provider, the skill defers to the provider's own validators rather +than guessing from file layout, which is why `eval-author-discover` probes for an +installed Harbor and asks Harbor to judge each config. `tests/test_skill_contract.py` holds to the same boundary and imports nothing from the platform, so `pytest` and `pyyaml` are enough to run it. The five tests that diff --git a/plugins/nemo-eval-author/skills/eval-author-discover/SKILL.md b/plugins/nemo-eval-author/skills/eval-author-discover/SKILL.md index c021c48704..37fa9591b4 100644 --- a/plugins/nemo-eval-author/skills/eval-author-discover/SKILL.md +++ b/plugins/nemo-eval-author/skills/eval-author-discover/SKILL.md @@ -120,7 +120,7 @@ rung's failure often disappears once you fix a higher one. |---|---| | `harbor` | Harbor is not importable by this interpreter. Re-run with the interpreter from **Before you start** | | `config` | No config file declares a nonempty `datasets` or `tasks` list. Confirm with the user where their suite lives | -| `config-parse` | The file could not be read, because PyYAML is missing. Harbor ships PyYAML, so this means the wrong interpreter | +| `config-parse` | A config file did not parse. Either PyYAML is missing, which means the wrong interpreter, or the file's YAML is broken. The hint says which | | `schema` | Harbor rejected the config's shape. The message carries the offending field path | | `resolution` | Harbor could not turn the config into a job. Usually a `datasets[].path` that does not exist. This fails before any container starts | | `tasks` | Some resolved directories are not valid Harbor tasks. A task directory needs a parseable `task.toml` and an `environment/` directory, even when the image is prebuilt | @@ -129,7 +129,9 @@ rung's failure often disappears once you fix a higher one. | `agent` | The named built-in agent does not exist, or the `import_path` does not import. Check the message for which | | `backend` | The environment backend failed preflight. For `docker`, confirm the daemon is running with `docker info` | | `round-trip` | The Harbor CLI rejected the config file's bytes. This is the weakest rung: it round-trips the schema only, so it can pass while `resolution` fails | -| `ethos` | Advisory. `ETHOS.md` is absent, so no agent doctrine is defined for this repository | +| `harbor-cli` | Advisory. No `harbor` executable exists on `PATH`, so the `round-trip` rung cannot run | +| `compatibility` | The installed Harbor does not expose the resolved task list, so `tasks`, `coverage`, and `credentials` cannot run. Install a Harbor version that exposes it | +| `ethos` | Advisory. `ETHOS.md` is absent or unreadable, so no agent doctrine is defined for this repository | | `tasks-on-disk` | Advisory, and always unproven. A count of directories holding a `task.toml` | ## Step 4: verify before you report diff --git a/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_inventory.py b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_inventory.py index 2d9b773830..9aaf769a01 100644 --- a/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_inventory.py +++ b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_inventory.py @@ -16,7 +16,8 @@ ``yaml`` is used when available and is not a dependency of this skill: Harbor depends on PyYAML, so a repository with Harbor installed always has it. Without it, config detection falls back to a top-level key scan and every candidate is -marked unparsed. +marked unparsed. A file PyYAML rejects falls back to that same scan, so a config +with broken syntax is reported rather than silently missing. """ from __future__ import annotations @@ -37,6 +38,10 @@ except ModuleNotFoundError: # ships with Harbor; absent only when Harbor is yaml = None # ty: ignore[invalid-assignment] +# Malformed YAML raises yaml.YAMLError, which is not a ValueError. Empty without +# PyYAML, so the except clause naming these stays valid either way. +_PARSE_ERRORS: tuple[type[BaseException], ...] = () if yaml is None else (yaml.YAMLError,) + _CONFIG_SUFFIXES = (".yaml", ".yml", ".json") _MAX_CONFIG_DEPTH = 4 _WORK_KEYS = ("datasets", "tasks") @@ -71,8 +76,9 @@ class ConfigCandidate: """A repository-owned Harbor config file. - ``data`` is empty when PyYAML is absent and the file is YAML. The ladder - needs parsed data, so an unparsed candidate is reported and skipped. + ``data`` is empty when the file could not be parsed, either because PyYAML is + absent or because the syntax is broken. The ladder needs parsed data, so an + unparsed candidate is reported and skipped. """ path: Path @@ -142,14 +148,31 @@ def scan_repository(repo_root: Path) -> RepositoryScan: "config-parse", FAIL, "Cannot read {} config file{}: {}.".format(len(unparsed), "s" if len(unparsed) != 1 else "", names), - hint="Install PyYAML, which arrives with Harbor, to read YAML configs.", + hint=( + "Install PyYAML, which arrives with Harbor, to read YAML configs." + if yaml is None + else "Fix the YAML syntax in each file this message names." + ), ) ) ethos: tuple[str, bytes] | None = None - if (repo_root / "ETHOS.md").is_file(): - ethos = ("ETHOS.md", (repo_root / "ETHOS.md").read_bytes()) - checks.append(_check("ethos", PASS, "ETHOS.md defines the agent doctrine.", severity=ADVISORY)) + ethos_file = repo_root / "ETHOS.md" + if ethos_file.is_file(): + try: + ethos = ("ETHOS.md", ethos_file.read_bytes()) + except OSError as exc: + checks.append( + _check( + "ethos", + WARN, + "ETHOS.md exists but cannot be read: {}.".format(exc.strerror or exc), + severity=ADVISORY, + hint="Make ETHOS.md readable to record the agent doctrine.", + ) + ) + else: + checks.append(_check("ethos", PASS, "ETHOS.md defines the agent doctrine.", severity=ADVISORY)) else: checks.append( _check( @@ -161,8 +184,8 @@ def scan_repository(repo_root: Path) -> RepositoryScan: ) ) - datasets = _dataset_paths(repo_root) tasks = _task_paths(repo_root) + datasets = _dataset_paths(tasks) if tasks: checks.append( _check( @@ -230,11 +253,12 @@ def _candidate(path: Path) -> ConfigCandidate | None: if is_json or yaml is not None: try: data = json.loads(text) if is_json else yaml.safe_load(text) - except (json.JSONDecodeError, ValueError): - return None - if not isinstance(data, dict) or not _has_work(data): - return None - return ConfigCandidate(path=path, data=data, parsed=True) + except (json.JSONDecodeError, ValueError, *_PARSE_ERRORS): + pass # unparseable, so fall back to the key scan + else: + if not isinstance(data, dict) or not _has_work(data): + return None + return ConfigCandidate(path=path, data=data, parsed=True) if not _WORK_KEY_PATTERN.search(text): return None @@ -245,14 +269,6 @@ def _has_work(data: dict[str, Any]) -> bool: return any(isinstance(data.get(name), list) and data[name] for name in _WORK_KEYS) -def _dataset_paths(repo_root: Path) -> list[Path]: - datasets: set[Path] = set() - for directory in walk_dirs(repo_root): - if directory != repo_root and directory.name != "task_template" and (directory / "task.toml").is_file(): - datasets.add(directory.parent) - return sorted(datasets) - - def _task_paths(repo_root: Path) -> list[Path]: return sorted( directory @@ -261,6 +277,11 @@ def _task_paths(repo_root: Path) -> list[Path]: ) +def _dataset_paths(tasks: list[Path]) -> list[Path]: + """Return the directories holding the tasks, which is what Harbor calls a dataset.""" + return sorted({task.parent for task in tasks}) + + def _fingerprint( repo_root: Path, config_paths: list[Path], @@ -272,17 +293,39 @@ def _fingerprint( if not dataset.is_relative_to(repo_root): continue for directory in walk_dirs(dataset): - files.update( - path for path in directory.iterdir() if path.is_file() and path.resolve().is_relative_to(repo_root) - ) + try: + entries = list(directory.iterdir()) + except OSError: + continue + files.update(path for path in entries if path.is_file() and path.resolve().is_relative_to(repo_root)) files.discard(repo_root / "ETHOS.md") digest = hashlib.sha256() + counted = 0 for path in sorted(files): + body = _file_digest(path) + if body is None: + continue digest.update(str(path.relative_to(repo_root)).encode()) digest.update(b"\0") - digest.update(path.read_bytes()) + digest.update(body) digest.update(b"\0") + counted += 1 if ethos is not None: digest.update(ethos[0].encode() + b"\0" + ethos[1] + b"\0") - return digest.hexdigest(), len(files) + (ethos is not None) + return digest.hexdigest(), counted + (ethos is not None) + + +def _file_digest(path: Path) -> bytes | None: + """Return the file's digest, or None when it cannot be read. + + Hashing each file separately keeps a file the fingerprint cannot read out of + the digest entirely, rather than contributing the bytes read before the + failure. ``file_digest`` reads in chunks, so a large repository-owned dataset + never lands in memory whole. + """ + try: + with path.open("rb") as source: + return hashlib.file_digest(source, "sha256").digest() + except OSError: + return None diff --git a/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_ladder.py b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_ladder.py index d5e50521c0..f915d89506 100644 --- a/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_ladder.py +++ b/plugins/nemo-eval-author/skills/eval-author-discover/scripts/providers/harbor/_ladder.py @@ -74,7 +74,8 @@ async def run_ladder(candidate: ConfigCandidate, repo_root: Path) -> ValidationO "schema", FAIL, "Cannot read {} to validate it.".format(candidate.path.name), - hint="Install PyYAML, which arrives with Harbor, to read YAML configs.", + # Harbor is importable here, so it brought PyYAML: the syntax is the fault. + hint="Fix the file's YAML syntax, which PyYAML could not parse.", ) ) return outcome @@ -122,7 +123,11 @@ async def _resolve(config: JobConfig, outcome: ValidationOutcome) -> Job | None: try: with tempfile.TemporaryDirectory(prefix="eval-author-jobs-") as scratch: job = await Job.create(config.model_copy(update={"jobs_dir": Path(scratch)})) - job._close_logger_handlers() + # Cleanup on a private API, and it has to precede the scratch removal. + # Resolution already succeeded, so a Harbor rename here is not a + # resolution failure and must not be reported as one. + with contextlib.suppress(Exception): + job._close_logger_handlers() except Exception as exc: outcome.checks.append( _check( diff --git a/plugins/nemo-eval-author/skills/eval-author/SKILL.md b/plugins/nemo-eval-author/skills/eval-author/SKILL.md index f464273b96..b51e03ae57 100644 --- a/plugins/nemo-eval-author/skills/eval-author/SKILL.md +++ b/plugins/nemo-eval-author/skills/eval-author/SKILL.md @@ -96,7 +96,8 @@ user, not to you. your own report under `.eval-author/`, which is theirs to commit or ignore. The bundled scripts write nothing at all; saving is your job, not theirs. - **A missing tool is a finding, not a task.** When the provider is not installed, - report that and stop. Do not install it into the user's environment. + say so and stop short of proving anything. Report what you found regardless, and + do not install the provider into the user's environment. - **Do not run the suite.** Prove it can run and hand over the command. Starting a job spends the user's compute and credentials on a decision they did not make. - **Trusted repositories only.** Validating a config can execute repository code, diff --git a/plugins/nemo-eval-author/tests/test_skill_contract.py b/plugins/nemo-eval-author/tests/test_skill_contract.py index c795f23e49..3e5d43041d 100644 --- a/plugins/nemo-eval-author/tests/test_skill_contract.py +++ b/plugins/nemo-eval-author/tests/test_skill_contract.py @@ -38,6 +38,7 @@ import ast import json +import os import re import subprocess import sys @@ -79,6 +80,13 @@ find_spec("harbor") is None, reason="Harbor is not installed, so it can judge nothing" ) +# Root reads a mode-000 file regardless, so the failure these tests stage cannot +# happen there and the tests would pass without proving anything. +_needs_unreadable_files = pytest.mark.skipif( + not hasattr(os, "geteuid") or os.geteuid() == 0, + reason="this user can read a file whatever its mode, so unreadability cannot be staged", +) + # Harbor brings these in, so a bundled script may name them. Nothing else outside # the standard library may appear. _PERMITTED_THIRD_PARTY = frozenset({"harbor", "pydantic", "yaml"}) @@ -169,6 +177,17 @@ def suite(tmp_path: Path) -> Path: return tmp_path +def _tree_state(root: Path) -> dict[str, bytes | None]: + """Map every path under root to its bytes, so an in-place edit is visible. + + Comparing names alone cannot catch a script that rewrote a file that was + already there, which is the promise this is here to hold. + """ + return { + path.relative_to(root).as_posix(): path.read_bytes() if path.is_file() else None for path in root.rglob("*") + } + + def _named(report: dict, name: str) -> dict: """Return one check by name, failing loudly when the ladder never ran it.""" found = next((check for check in report["checks"] if check["name"] == name), None) @@ -432,11 +451,75 @@ def test_discover_finds_configs_without_pyyaml(suite: Path) -> None: def test_discovery_writes_no_files(suite: Path) -> None: """The scripts report and the agent saves, which is what makes the skill safe to run.""" - before = {path.relative_to(suite).as_posix() for path in suite.rglob("*")} + before = _tree_state(suite) _run_discover(suite) - assert {path.relative_to(suite).as_posix() for path in suite.rglob("*")} == before + assert _tree_state(suite) == before + + +def test_discover_reports_a_config_pyyaml_cannot_parse(suite: Path) -> None: + """Broken YAML is a finding, not a crash. + + A tab cannot start a YAML token, so PyYAML raises ``yaml.YAMLError``, which is + not a ``ValueError``. Left uncaught it escaped the scan and took the whole run + down with a traceback and no report at all. The file still names Harbor work, + so it is reported through the same fallback that runs when PyYAML is absent. + """ + (suite / "broken.yaml").write_text("datasets:\n\t- path: ./tabbed\n", encoding="utf-8") + + _, report = _run_discover(suite) + + assert "harbor-job.yaml" in [config["path"] for config in report["configs"]], ( + "one broken file must not hide the configs around it" + ) + parse = _named(report, "config-parse") + assert parse["status"] == "fail" + assert "broken.yaml" in parse["message"] + assert parse["hint"], "an unreadable config must tell the user what to do" + + +def test_discover_ignores_a_yaml_file_that_declares_no_harbor_work(suite: Path) -> None: + """Broken YAML that names no datasets or tasks is somebody else's file.""" + (suite / "docker-compose.yaml").write_text("services:\n\t- broken\n", encoding="utf-8") + + _, report = _run_discover(suite) + + assert [config["path"] for config in report["configs"]] == ["harbor-job.yaml"] + + +@_needs_unreadable_files +def test_discover_reports_an_unreadable_ethos(suite: Path) -> None: + """ETHOS.md is advisory, so failing to read it must not cost the whole report.""" + ethos = suite / "ETHOS.md" + ethos.write_text("# doctrine\n", encoding="utf-8") + ethos.chmod(0o000) + try: + _, report = _run_discover(suite, with_harbor=False) + finally: + ethos.chmod(0o644) + + assert report["ethos_path"] is None, "an unread file defines no doctrine" + check = _named(report, "ethos") + assert check["status"] == "warn" + assert check["severity"] == "advisory" + + +@_needs_unreadable_files +def test_discover_keeps_an_unreadable_dataset_file_out_of_the_fingerprint(suite: Path) -> None: + """One unreadable file must neither abort the fingerprint nor silently join it.""" + _, baseline = _run_discover(suite, with_harbor=False) + + blocked = suite / "dataset" / "task-one" / "blocked.bin" + blocked.write_bytes(b"payload") + blocked.chmod(0o000) + try: + _, report = _run_discover(suite, with_harbor=False) + finally: + blocked.chmod(0o644) + + assert report["input_file_count"] == baseline["input_file_count"] + assert report["fingerprint"] == baseline["fingerprint"] def test_discover_fails_with_a_hint_when_the_path_is_missing(tmp_path: Path) -> None: From 566b4d7727f5b57d4dc4aa408cffc96f6b2bc52e Mon Sep 17 00:00:00 2001 From: Alec Khoury Date: Fri, 21 Aug 2026 15:03:33 -0500 Subject: [PATCH 8/8] docs(eval-author): cut the prose explaining the removed CLI Review asked for deletions rather than replacements: a passage saying a command namespace is absent is only useful to someone who remembers it existed, and that memory expires. Signed-off-by: Alec Khoury --- docs/agents/insight-driven-optimization.mdx | 4 ---- packages/nemo_platform/pyproject.toml | 4 ---- plugins/nemo-experimentalist/AGENTS.md | 6 +----- plugins/nemo-experimentalist/README.md | 4 +--- .../src/nemo_experimentalist_plugin/eval_author/README.md | 4 ---- 5 files changed, 2 insertions(+), 20 deletions(-) diff --git a/docs/agents/insight-driven-optimization.mdx b/docs/agents/insight-driven-optimization.mdx index f30fea0f24..fb48f923b8 100644 --- a/docs/agents/insight-driven-optimization.mdx +++ b/docs/agents/insight-driven-optimization.mdx @@ -285,10 +285,6 @@ evaluation suite for validating candidates that address the Insight. The Experimenter invokes the Eval Author workflow in Insight mode and reads the `eval_author` section of the experiment configuration. -The Eval Author has no command namespace. Work on the evaluation suites in your -own repository runs through the `eval-author` skills, which your agent reads and -follows. See the [Eval Author README](https://github.com/NVIDIA-NeMo/nemo-platform/tree/main/plugins/nemo-eval-author). - ## Get Started ### Set Up diff --git a/packages/nemo_platform/pyproject.toml b/packages/nemo_platform/pyproject.toml index f50a933a00..80af4f772d 100644 --- a/packages/nemo_platform/pyproject.toml +++ b/packages/nemo_platform/pyproject.toml @@ -716,10 +716,6 @@ nemo-anonymizer-plugin = { source = "../../plugins/nemo-anonymizer/src/nemo_anon nemo-auditor-plugin = { source = "../../plugins/nemo-auditor/src/nemo_auditor", module = "nemo_auditor", inherit = { "entry-points" = ["nemo.*"] } } nemo-data-designer-plugin = { source = "../../plugins/nemo-data-designer/src/nemo_data_designer_plugin", module = "nemo_data_designer_plugin", inherit = { "entry-points" = ["nemo.*"] } } nemo-deployments-plugin = { source = "../../plugins/nemo-deployments/src/nemo_deployments_plugin", module = "nemo_deployments_plugin", inherit = { "entry-points" = ["nemo.*"] } } -# Eval Author is deliberately absent. It ships skills and no code, declares no entry -# points, and has nothing a distribution can expose, so bundling it into -# `nemo-platform[all]` would ship files no mechanism can find. Add it back when the -# skills register under `nemo.skills`. nemo-evaluator-plugin = { source = "../../plugins/nemo-evaluator/src/nemo_evaluator", module = "nemo_evaluator", inherit = { "entry-points" = ["nemo.*"] } } nemo-experimentalist-plugin = { source = "../../plugins/nemo-experimentalist/src/nemo_experimentalist_plugin", module = "nemo_experimentalist_plugin", inherit = { "entry-points" = ["nemo.*"] } } nemo-insights-plugin = { source = "../../plugins/nemo-insights/src/nemo_insights_plugin", module = "nemo_insights_plugin", inherit = { "entry-points" = ["nemo.*"] } } diff --git a/plugins/nemo-experimentalist/AGENTS.md b/plugins/nemo-experimentalist/AGENTS.md index b3004f7823..fef7c340df 100644 --- a/plugins/nemo-experimentalist/AGENTS.md +++ b/plugins/nemo-experimentalist/AGENTS.md @@ -29,9 +29,6 @@ Experimentalist imports nothing from `nemo-eval-author-plugin`, so the package c ten borrows to zero, so it went away with them. - `tests/test_contract_dependency.py` asserts that this plugin never declares `nemo-eval-author-plugin` as a dependency, which is what keeps the cycle broken. -- `plugins/nemo-eval-author/` ships the customer-facing skills and nothing else. Its CLI - and `discovery/` package went away with the pivot to skills, and with them the last - borrow from this plugin, so neither side imports the other now. - Agent tests live in `tests/eval_author/`. This plugin's `conftest.py` already covers the isolation those tests need, so the Eval Author copy went away. @@ -44,8 +41,7 @@ plugin's `AgentsCLI` discovers and mounts. There is no top-level The Analyst follows the same rule: `nemo agents analyst run` (was `nemo insights analyze`). Prefer `ctx.command_path` over a hardcoded path when a message quotes the -command back to the user. Eval Author had a command group under this rule and no -longer does; it ships skills instead. +command back to the user. ### 2026-07-28: Eval Author extracted to its own plugin, heading for standalone (superseded) diff --git a/plugins/nemo-experimentalist/README.md b/plugins/nemo-experimentalist/README.md index f198cb0511..68632ee236 100644 --- a/plugins/nemo-experimentalist/README.md +++ b/plugins/nemo-experimentalist/README.md @@ -78,9 +78,7 @@ single leader, so complementary strengths stay alive across rounds. analyze traces or host an Insight API. - [Eval Author](src/nemo_experimentalist_plugin/eval_author/README.md) builds the Insight-specific evaluation suite, and Insight mode invokes it automatically. It - ships inside this plugin. The separate - [Eval Author package](../nemo-eval-author/README.md) is the customer-facing path, - and it ships skills rather than an agent or a CLI. + ships inside this plugin. - **Harbor** runs the task containers that score every candidate. - **NeMo Experiments** mirrors each run and its candidates as an experiment group, so the lineage is visible in Studio. Structure only — rewards and diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md index 016224a8cf..29e144234c 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md @@ -17,10 +17,6 @@ This package lives inside the Experimentalist plugin and uses its evaluator, sta and trace helpers directly. Experimentalist insight mode imports and runs Eval Author before optimization begins. -The customer-facing path is a skill rather than an agent, and the -[Eval Author package](../../../../nemo-eval-author/README.md) holds those skills. It -has no CLI and shares no code with this agent. - ## Current Files - `agent.py` defines the canonical `EvalAuthor` agent.