diff --git a/docs/_scripts/lint_python_snippets.py b/docs/_scripts/lint_python_snippets.py index 73fae2263e..a8e4b6e798 100644 --- a/docs/_scripts/lint_python_snippets.py +++ b/docs/_scripts/lint_python_snippets.py @@ -51,7 +51,9 @@ "", "", } -DEFAULT_IGNORED_TY_RULES = ("possibly-unbound-attribute",) +# ``possibly-unbound-attribute`` was renamed upstream; passing the old name makes ty emit +# ``warning[unknown-rule]``, which fails this check for every doc regardless of its snippets. +DEFAULT_IGNORED_TY_RULES = ("possibly-missing-attribute",) @dataclass(frozen=True) diff --git a/docs/evaluator/manage-tasks-tasksets.mdx b/docs/evaluator/manage-tasks-tasksets.mdx index 32c1be6ce5..a3f20e4c90 100644 --- a/docs/evaluator/manage-tasks-tasksets.mdx +++ b/docs/evaluator/manage-tasks-tasksets.mdx @@ -73,34 +73,69 @@ Reference the stored metric with a `MetricRef` (`workspace/name`, or a bare `nam the task's workspace). The service returns the stored `Task`. ```python -from nemo_evaluator.api.schemas import MetadataItem, MetricRef, TaskInput, TaskInputs +from nemo_evaluator.api.schemas import EvaluatorTaskDefinition, MetadataItem, MetricRef, TaskInput, TaskInputs task = TaskInput( - intent="Answer the user's geography question with the capital city.", - inputs=TaskInputs(instruction="What is the capital of France?"), - metrics=[MetricRef("default/answer-exact-match")], + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the user's geography question with the capital city.", + inputs=TaskInputs(instruction="What is the capital of France?"), + metrics=[MetricRef("default/answer-exact-match")], + ), metadata=[MetadataItem(key="suite", value="geography")], ) stored = tasks.create("capital-of-france", task=task) -print(stored.id, stored.metrics) +print(stored.id, stored.spec.metrics) ``` ### `TaskInput` fields +| Field | Type | Required | Description | +|-------|------|----------|-------------| +| `spec` | `TaskDefinition` | Yes | The task's content, discriminated by `kind` — see below. | +| `metadata` | `list[MetadataItem]` | No | Key/value annotations. Keys must be unique. | +| `tags` | `list[str]` | No | Tags to point at the revision this request publishes. `latest` is always applied server-side. | + +### Task kinds + +A task is an evaluation unit; its `kind` says which runner executes it. There are two: + +- `evaluator` — the task's content is fields you author, scored by platform metrics. +- `harbor` — the task's content is a packaged directory of files, scored by Harbor's own reward. + +Both are stored as the same record type, so a taskset can group them and you manage every evaluation +unit in one place regardless of which runner executes it. + +`EvaluatorTaskDefinition` (`kind="evaluator"`): + | Field | Type | Required | Description | |-------|------|----------|-------------| | `intent` | `str` | Yes | Human-readable description of the desired agent behavior. | | `inputs` | `TaskInputs` | No | The task's recognized input fields. `instruction` is the agent's prompt; it falls back to `intent` when unset. | +| `reference` | `dict[str, Any]` | No | Grader-only ground truth (held-out tests, expected outputs, rubric data). Surfaced to metrics but never seeded into the agent's workspace or shown to the agent. Held out from the *agent*, not from the API. | | `metrics` | `list[MetricRefOrInline]` | No | The metrics that score the task, as `MetricRef` references (`workspace/name`) to stored metrics. Pre-built inline metric bundles (`MetricInline`) are also accepted and are normalized to stored metrics on create. | | `views` | `dict[str, SemanticView]` | No | Optional reporting views mapping metric outputs into named semantic scores. | -| `metadata` | `list[MetadataItem]` | No | Key/value annotations. Keys must be unique. | -| `tags` | `list[str]` | No | Tags to point at the revision this request publishes. `latest` is always applied server-side. | + +`HarborTaskDefinition` (`kind="harbor"`): + +| Field | Type | Required | Description | +|-------|------|----------|-------------| +| `archive_ref` | `str` | Yes | Files reference to the task's packaged directory (`workspace/fileset#path`). One fileset per task, so a task shared by several tasksets is stored once. | +| `archive_digest` | `str` | Yes | Content hash Harbor computed over the task directory. | +| `instruction` | `str` | No | The task's instruction text, when it has one. | +| `config` | `dict` | No | Harbor's own task configuration (verifier, agent, environment, steps), stored as published. | + + +Storing a Harbor task is supported; **running one from storage is not yet**. A taskset may group both +kinds, but expanding a `harbor` member is rejected with `422` before the run starts, whatever target +you submit against. Harbor evaluations continue to run through the existing dataset-driven path. + A stored task holds **metric references only**. Any inline metric bundle you pass on create is stored as a content-addressed *derived* metric, and the task record is normalized to reference it. This is -why `stored.metrics` always comes back as a list of `MetricRef` references. +why `stored.spec.metrics` always comes back as a list of `MetricRef` references. ### Retrieve, list, and delete @@ -108,12 +143,12 @@ why `stored.metrics` always comes back as a list of `MetricRef` references. ```python # Retrieve one task by name (its current content) task = tasks.retrieve("capital-of-france") -print(task.revision, task.tags) # e.g. 1 {'latest': 1} +print(task.spec.kind, task.revision, task.tags) # e.g. evaluator 1 {'latest': 1} # List tasks in the workspace (paginated) page = tasks.list(page=1, page_size=100, sort="-created_at") for item in page.data: - print(item.name, item.intent) + print(item.name, item.spec.kind) # Delete a task (this also removes all of its revisions) tasks.delete("capital-of-france") @@ -131,9 +166,12 @@ no existence check. ```python revised_task = TaskInput( - intent="Answer the user's geography question with the capital city.", - inputs=TaskInputs(instruction="Name the capital city of France."), - metrics=[MetricRef("default/answer-exact-match")], + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the user's geography question with the capital city.", + inputs=TaskInputs(instruction="Name the capital city of France."), + metrics=[MetricRef("default/answer-exact-match")], + ), metadata=[MetadataItem(key="suite", value="geography")], ) @@ -169,7 +207,7 @@ original = tasks.retrieve("capital-of-france", revision=digest) # revision 1, a current = tasks.retrieve("capital-of-france") # revision 2, the current content assert original.revision == 1 and current.revision == 2 -assert original.inputs.instruction != current.inputs.instruction +assert original.spec.inputs.instruction != current.spec.inputs.instruction ``` ### Tag a revision @@ -361,9 +399,10 @@ A fragment that no longer resolves fails the evaluation rather than falling back revision. -Stored tasks carry no grader-only `reference` (held-out ground truth): that field lives only on inline -`AgentEvalTaskInput`. Taskset-driven tasks therefore run with an empty `reference`, so use a taskset -when your metrics score the agent's output directly rather than against per-task held-out data. +A member's grader-only `reference` (held-out ground truth) is loaded from the pinned revision along +with the rest of its content, so a taskset-driven run grades against the ground truth that revision +fixed. Because `reference` is covered by the revision digest, changing it publishes a new revision — +a pin fixes the grading, not just the prompt. ## Async usage diff --git a/packages/harbor_nemo/README.md b/packages/harbor_nemo/README.md new file mode 100644 index 0000000000..0ad12931c6 --- /dev/null +++ b/packages/harbor_nemo/README.md @@ -0,0 +1,93 @@ +# harbor-nemo + +A [Harbor](https://github.com/harbor-framework/harbor) registry backend that publishes to and +runs from **NeMo Platform** instead of the public Harbor Hub, with no changes to Harbor. + +```bash +pip install -e packages/harbor_nemo + +export HARBOR_REGISTRY_BACKEND=nemo +export NMP_BASE_URL=http://localhost:8080 + +harbor publish ./my-task +harbor download nvidia/my-task -o ./out +harbor run -t nvidia/my-task --agent nop +``` + +Installing the package registers `nemo` under the `harbor.registry_backends` entry point. +That is the whole integration: the stock CLI resolves the backend by name at call time. + +## How Harbor concepts map onto NeMo + +| Harbor | NeMo | +|---|---| +| task package `org/name` | task entity `org.name` (`kind="harbor"`), one workspace | +| task version | a published *revision* of that entity | +| task archive (`dist.tar.gz`) | a file in the `harbor-packages` fileset | +| content hash | `spec.archive_digest` | +| dataset `org/name` | taskset entity `org.name` | +| dataset-level files | a JSON blob in taskset `metadata` (see *Known gaps*) | +| tags (`latest`, …) | revision tags | + +**The org is folded into the entity name.** A NeMo workspace is a tenancy boundary with its +own lifecycle and authorization; a Harbor org is a cheap, self-serve namespace that +`harbor publish` creates on demand. Mapping org to workspace would make publishing a tenancy +operation. The cost is that the org prefix is a convention, not an enforced boundary. + +## Configuration + +| Variable | Default | Meaning | +|---|---|---| +| `NMP_BASE_URL` | `http://localhost:8080` | platform to publish to / read from | +| `HARBOR_NEMO_WORKSPACE` / `NMP_WORKSPACE` | `default` | workspace holding tasks and tasksets | +| `HARBOR_NEMO_FILESET` | `harbor-packages` | fileset holding package archives | +| `NMP_TOKEN` / `NMP_API_KEY` | — | bearer token, when the platform has auth enabled | +| `HARBOR_NEMO_TIMEOUT_SEC` | `120` | HTTP timeout | + +Set `HARBOR_REGISTRY_WEBSITE_URL` too: `harbor publish` prints a hub URL from a Harbor-side +constant, so without it the CLI advertises `hub.harborframework.com` for NeMo packages. + +## Two digests, and why it matters + +NeMo addresses a revision by a digest of the revision's *content* (canonical JSON of the +stored spec). Harbor addresses a version by a digest of the task *directory's files*. They are +different hashes of different things, and both are live: + +- `ResolvedTaskVersion.content_hash` carries **Harbor's**, because Harbor's download cache is + keyed on it. +- A `sha256:` reference reaching `resolve_version` is always **Harbor's**, and is *not* a + valid NeMo revision selector — the platform returns 404 for it. Resolving one is a scan + over revisions comparing `spec.archive_digest`, not a direct fetch. +- A **revision ordinal** is not a valid selector either: the platform reads any non-digest + fragment as a *tag name*, so `/revisions/2` looks for a tag called `"2"`. Ordinals are + translated to that revision's content hash first. +- NeMo digests are **bare hex**, deliberately, so a `#` fragment stays free of `:` — which the + entity-ref charset does not admit and the route's path pattern rejects with a 422. Harbor's + `sha256:` prefix is stripped before any digest is used as a selector. + +Publishing a dataset translates between the two spaces: a Harbor manifest pins members by +archive digest, a taskset pins by revision digest, so each member costs one lookup. This is +not optional — the taskset service re-resolves bare member refs at write time, so an +unpinned member would silently pin whatever was `latest` at publish, not what the manifest +named. + +## Known gaps + +- **Dataset-level files ride in taskset `metadata`** as a JSON string, because a taskset has + no file-reference field. A taskset-level file reference would replace this. +- **No yank support.** `ResolvedTaskVersion.yanked_at` is always `None`; NeMo has no + equivalent. +- **`record_download` is a deliberate no-op.** NeMo has no counter primitive, so implementing + it would mean a read-modify-write on the hottest entity per package for best-effort + telemetry. +- **`harbor version list|show|tag` is Supabase-pinned** in Harbor itself and will show Hub + data regardless of `HARBOR_REGISTRY_BACKEND`. + +## Requirements + +Needs a NeMo Platform with the `entities`, `files`, and `evaluator` services, and the +`kind="harbor"` task definition from nemo-platform PR #1071. + +```bash +uv run nemo services run --services entities,files,evaluator --port 8080 +``` diff --git a/packages/harbor_nemo/pyproject.toml b/packages/harbor_nemo/pyproject.toml new file mode 100644 index 0000000000..b4bb7c6a0c --- /dev/null +++ b/packages/harbor_nemo/pyproject.toml @@ -0,0 +1,41 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# Deliberately NOT a member of the root `[tool.uv.workspace]`. This package depends on +# `harbor`, which the platform keeps as a marker-gated optional extra rather than a default +# dependency; listing it as a workspace member would pull harbor into every bare +# `uv sync --all-packages`. Install it explicitly instead: +# +# uv pip install -e packages/harbor_nemo +[project] +name = "harbor-nemo" +version = "0.1.0" +description = "NeMo Platform registry backend for Harbor: publish and run Harbor packages against NeMo." +readme = "README.md" +requires-python = ">=3.12" +license = { text = "Apache-2.0" } + +dependencies = [ + "harbor>=0.20.0", + "httpx>=0.27", + "pydantic>=2.7", +] + +# This is what makes the stock `harbor` CLI find the backend: with the package installed, +# `HARBOR_REGISTRY_BACKEND=nemo` resolves through here. The value is a zero-argument callable +# returning a BaseRegistryBackend, loaded lazily so importing harbor stays cheap. +[project.entry-points."harbor.registry_backends"] +nemo = "harbor_nemo:load_backend" + +[project.optional-dependencies] +dev = ["pytest>=8", "pytest-asyncio>=0.23", "respx>=0.21"] + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +packages = ["src/harbor_nemo"] + +[tool.pytest.ini_options] +asyncio_mode = "auto" diff --git a/packages/harbor_nemo/src/harbor_nemo/__init__.py b/packages/harbor_nemo/src/harbor_nemo/__init__.py new file mode 100644 index 0000000000..4a6cd5f28f --- /dev/null +++ b/packages/harbor_nemo/src/harbor_nemo/__init__.py @@ -0,0 +1,34 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""A NeMo Platform registry backend for Harbor. + +Installing this package registers ``nemo`` under the ``harbor.registry_backends`` entry +point, so the stock Harbor CLI publishes to and runs from NeMo with no change to Harbor:: + + export HARBOR_REGISTRY_BACKEND=nemo + harbor publish ./my-task + harbor run -d nvidia/my-dataset +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from harbor_nemo.backend import NemoRegistryBackend + +__all__ = ["load_backend"] + + +def load_backend() -> "NemoRegistryBackend": + """Entry point target: build the backend. + + Imports inside the function rather than at module scope because Harbor resolves entry + points lazily and only for a backend that was actually selected. A module-level import + would pull httpx and every Harbor publisher model into any process that merely *lists* + installed backends — including one using the default Supabase backend. + """ + from harbor_nemo.backend import NemoRegistryBackend + + return NemoRegistryBackend() diff --git a/packages/harbor_nemo/src/harbor_nemo/backend.py b/packages/harbor_nemo/src/harbor_nemo/backend.py new file mode 100644 index 0000000000..146a5950b2 --- /dev/null +++ b/packages/harbor_nemo/src/harbor_nemo/backend.py @@ -0,0 +1,93 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The NeMo registry backend: one host, four collaborators.""" + +from __future__ import annotations + +from typing import override + +from harbor.publisher.base import BasePublisher +from harbor.registry.backend import BaseRegistryBackend +from harbor.registry.client.base import BaseRegistryClient +from harbor.registry.task_resolver import BaseTaskResolver +from harbor.storage.base import BaseStorage + +from harbor_nemo.client import NemoClient +from harbor_nemo.config import NemoConfig +from harbor_nemo.dataset_client import NemoDatasetClient +from harbor_nemo.publisher import NemoPublisher +from harbor_nemo.storage import NemoStorage +from harbor_nemo.task_resolver import NemoTaskResolver + + +class NemoRegistryBackend(BaseRegistryBackend): + """Publishes to and reads from a NeMo Platform. + + Every collaborator is built once and memoized. That is the interface's requirement, and it + is load bearing here for a second reason: all four share one ``NemoClient``, so a single + HTTP connection pool serves a 50-wide ``publish_tasks`` instead of 50 pools. + + ``package_type`` is left as the inherited default, which probes the dataset client and + then the task resolver. NeMo has no single endpoint that answers "is this a task or a + taskset", so overriding it would mean making the same two requests with more code. + """ + + def __init__(self, config: NemoConfig | None = None) -> None: + self._config = config or NemoConfig.from_env() + self._client = NemoClient(self._config) + self._storage_instance: NemoStorage | None = None + self._publisher_instance: NemoPublisher | None = None + self._dataset_client_instance: NemoDatasetClient | None = None + self._task_resolver_instance: NemoTaskResolver | None = None + + @property + def config(self) -> NemoConfig: + return self._config + + def _nemo_storage(self) -> NemoStorage: + """The concrete storage. ``storage()`` narrows to the interface for callers; the + publisher and dataset client need the NeMo-specific helpers (``exists``, + ``to_fileset_ref``), and must get *this* instance, not another one.""" + if self._storage_instance is None: + self._storage_instance = NemoStorage(self._client, self._config) + return self._storage_instance + + @override + def storage(self) -> BaseStorage: + return self._nemo_storage() + + @override + def publisher(self) -> BasePublisher: + if self._publisher_instance is None: + # The publisher writes blobs where the resolver will read them: same fileset, + # same host, because it is handed this backend's storage rather than opening its + # own client. + self._publisher_instance = NemoPublisher( + self._client, self._config, self._nemo_storage(), self._nemo_resolver() + ) + return self._publisher_instance + + @override + def dataset_client(self) -> BaseRegistryClient: + if self._dataset_client_instance is None: + self._dataset_client_instance = NemoDatasetClient( + self._client, self._config, self._nemo_storage() + ) + return self._dataset_client_instance + + def _nemo_resolver(self) -> NemoTaskResolver: + """The concrete resolver, for the same reason as ``_nemo_storage``: the publisher + needs ``revision_digest_for_archive`` to pin a dataset's members, and it must be the + same instance the download side resolves through.""" + if self._task_resolver_instance is None: + self._task_resolver_instance = NemoTaskResolver(self._client, self._config) + return self._task_resolver_instance + + @override + def task_resolver(self) -> BaseTaskResolver: + return self._nemo_resolver() + + async def aclose(self) -> None: + """Release the shared HTTP client. The CLI is short lived and does not call this.""" + await self._client.aclose() diff --git a/packages/harbor_nemo/src/harbor_nemo/client.py b/packages/harbor_nemo/src/harbor_nemo/client.py new file mode 100644 index 0000000000..e7bb76254b --- /dev/null +++ b/packages/harbor_nemo/src/harbor_nemo/client.py @@ -0,0 +1,127 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""HTTP transport and error translation for the NeMo registry backend. + +Talks to the platform's documented REST API with ``httpx`` rather than importing the NeMo +Platform SDK. The SDK would drag the whole platform into any environment that installs +``harbor``, and a registry backend needs three endpoint families (filesets, tasks, tasksets), +not a platform client. + +**Error translation is the point of this module.** A caller holds a ``BasePublisher`` and +cannot be expected to catch transport exceptions, so every response passes through +:meth:`NemoClient.request`, which maps status codes onto ``harbor.publisher.errors``. The one +rule that is load bearing: a 404 becomes :class:`NotFound`, which read paths convert to +``ValueError`` and *nothing else does*. ``BaseRegistryBackend.package_type`` distinguishes +"absent" from "broken" by catching exactly ``ValueError``, so translating an auth or transport +failure into a not-found signal would report "package not found" to a user whose real problem +is that they are logged out. +""" + +from __future__ import annotations + +from types import TracebackType +from typing import Any, Self + +import httpx +from harbor.publisher.errors import ( + PublishAuthError, + PublishBackendError, + PublishPermissionError, +) + +from harbor_nemo.config import NemoConfig + + +class NotFound(Exception): + """The platform returned 404. + + Not a ``ValueError`` itself: whether a miss means "no such package" (read paths, where + ``ValueError`` is the contract) or a genuine backend failure (a publish whose fileset + vanished mid-flight) depends on the caller, so the decision is left to them. + """ + + +def _detail(response: httpx.Response) -> str: + """Pull the platform's own error text out of a response, falling back to the status line.""" + try: + body = response.json() + except Exception: + return response.text.strip() or f"HTTP {response.status_code}" + if isinstance(body, dict): + detail = body.get("detail") or body.get("message") + if isinstance(detail, str): + return detail + if detail is not None: + return str(detail) + return str(body) + + +class NemoClient: + """A thin async HTTP client whose failures are already Harbor's error types.""" + + def __init__(self, config: NemoConfig) -> None: + self.config = config + headers = {"Accept": "application/json"} + if config.token: + headers["Authorization"] = f"Bearer {config.token}" + self._client = httpx.AsyncClient(headers=headers, timeout=config.timeout_sec) + + async def __aenter__(self) -> Self: + return self + + async def __aexit__( + self, + exc_type: type[BaseException] | None, + exc: BaseException | None, + tb: TracebackType | None, + ) -> None: + await self.aclose() + + async def aclose(self) -> None: + await self._client.aclose() + + async def request( + self, + method: str, + url: str, + *, + json: Any = None, + content: bytes | None = None, + params: dict[str, Any] | None = None, + headers: dict[str, str] | None = None, + ) -> httpx.Response: + """Perform a request, translating every failure into a Harbor-visible error. + + Returns the raw response on success so callers can read the *status code* — the + evaluator's ``PUT`` distinguishes 200 ("content already published, no new revision") + from 201 ("new revision"), which is exactly Harbor's ``skipped`` signal and is + available nowhere else in the response. + """ + try: + response = await self._client.request( + method, url, json=json, content=content, params=params, headers=headers + ) + except httpx.HTTPError as exc: + # Connection refused, DNS failure, timeout. Emphatically *not* a not-found: a + # platform that is down must not be reported as a package that does not exist. + raise PublishBackendError(f"Could not reach the NeMo platform at {url}: {exc}") from exc + + if response.status_code == 401: + raise PublishAuthError( + "Not authenticated with the NeMo platform. Set NMP_TOKEN (or NMP_API_KEY) to a " + f"valid token for {self.config.base_url}." + ) + if response.status_code == 403: + raise PublishPermissionError( + f"You don't have permission to write to workspace " + f"{self.config.workspace!r} on {self.config.base_url}." + ) + if response.status_code == 404: + raise NotFound(_detail(response)) + if response.status_code >= 400: + raise PublishBackendError(_detail(response)) + return response + + async def get_json(self, url: str, *, params: dict[str, Any] | None = None) -> Any: + return (await self.request("GET", url, params=params)).json() diff --git a/packages/harbor_nemo/src/harbor_nemo/config.py b/packages/harbor_nemo/src/harbor_nemo/config.py new file mode 100644 index 0000000000..f0ea075db5 --- /dev/null +++ b/packages/harbor_nemo/src/harbor_nemo/config.py @@ -0,0 +1,57 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Where this backend points, and how it authenticates.""" + +from __future__ import annotations + +import os +from dataclasses import dataclass + +#: Default fileset holding every published package archive in a workspace. One fileset rather +#: than one per task: `BaseStorage.download_file` receives only a path string, so the fewer +#: places a blob can live, the fewer ways metadata and blobs can end up on different hosts. +DEFAULT_FILESET = "harbor-packages" +DEFAULT_BASE_URL = "http://localhost:8080" +DEFAULT_WORKSPACE = "default" + + +@dataclass(frozen=True) +class NemoConfig: + """Resolved connection settings for the NeMo registry backend. + + Read from the environment at construction rather than import time, so a process can point + at a different platform between backend instances (which is also what makes the tests able + to run without patching module globals). + """ + + base_url: str + workspace: str + fileset: str + token: str | None + timeout_sec: float + + @classmethod + def from_env(cls) -> "NemoConfig": + return cls( + base_url=os.environ.get("NMP_BASE_URL", DEFAULT_BASE_URL).rstrip("/"), + workspace=os.environ.get("HARBOR_NEMO_WORKSPACE") + or os.environ.get("NMP_WORKSPACE", DEFAULT_WORKSPACE), + fileset=os.environ.get("HARBOR_NEMO_FILESET", DEFAULT_FILESET), + # NMP_TOKEN first: a token is what the platform actually accepts, and an API key + # env var is the more common thing to have set for an unrelated service. + token=os.environ.get("NMP_TOKEN") or os.environ.get("NMP_API_KEY"), + timeout_sec=float(os.environ.get("HARBOR_NEMO_TIMEOUT_SEC", "120")), + ) + + @property + def files_url(self) -> str: + return f"{self.base_url}/apis/files/v2/workspaces/{self.workspace}/filesets" + + @property + def tasks_url(self) -> str: + return f"{self.base_url}/apis/evaluator/v2/workspaces/{self.workspace}/tasks" + + @property + def tasksets_url(self) -> str: + return f"{self.base_url}/apis/evaluator/v2/workspaces/{self.workspace}/tasksets" diff --git a/packages/harbor_nemo/src/harbor_nemo/dataset_client.py b/packages/harbor_nemo/src/harbor_nemo/dataset_client.py new file mode 100644 index 0000000000..fa3b753f3c --- /dev/null +++ b/packages/harbor_nemo/src/harbor_nemo/dataset_client.py @@ -0,0 +1,203 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Reading Harbor datasets out of NeMo tasksets.""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any, override + +from harbor.models.package.reference import PackageReference +from harbor.models.registry import DatasetFileInfo, DatasetMetadata, DatasetSummary +from harbor.models.task.id import GitTaskId, LocalTaskId, PackageTaskId +from harbor.registry.client.base import BaseRegistryClient + +from harbor_nemo.client import NemoClient, NotFound +from harbor_nemo.config import NemoConfig +from harbor_nemo.names import NameMappingError, from_entity_name, to_entity_name +from harbor_nemo.publisher import ( + DATASET_FILES_METADATA_KEY, + PACKAGE_NAME_METADATA_KEY, +) +from harbor_nemo.storage import NemoStorage + + +#: Harbor writes content references as ``sha256:``; NeMo revision fragments are bare hex. +_SHA256_PREFIX = "sha256:" + + +def _strip_digest_prefix(ref: str) -> str: + return ref[len(_SHA256_PREFIX) :] if ref.startswith(_SHA256_PREFIX) else ref + + +def _metadata_value(record: dict[str, Any], key: str) -> str | None: + for item in record.get("metadata") or []: + if item.get("key") == key: + return item.get("value") + return None + + +class NemoDatasetClient(BaseRegistryClient): + """Resolves ``org/name@ref`` to dataset metadata backed by a NeMo taskset. + + Storage comes from the owning backend rather than being constructed here: a + ``DatasetFileInfo.storage_path`` is a literal path issued by one fileset, so reading it + from a different one would be a lookup for a blob that fileset never stored. + """ + + def __init__(self, client: NemoClient, config: NemoConfig, storage: NemoStorage) -> None: + # BaseRegistryClient.__init__ builds the TaskClient that the inherited + # `download_dataset` drives. Skipping it leaves that path broken. + super().__init__() + self._client = client + self._config = config + self._storage = storage + + def _taskset_url(self, entity_name: str) -> str: + return f"{self._config.tasksets_url}/{entity_name}" + + async def _member_archive_digest(self, ref: str) -> tuple[str, str, str]: + """Resolve a taskset member reference to ``(org, name, archive_digest)``. + + A taskset pins members by NeMo *revision* digest, while a Harbor dataset pins tasks by + the *archive* hash, so every member needs a fetch to read ``spec.archive_digest`` out + of the revision the taskset actually named. There is no way to answer this from the + taskset alone. + """ + location, _, fragment = ref.partition("#") + workspace, _, entity_name = location.partition("/") + if not entity_name: + workspace, entity_name = self._config.workspace, workspace + + url = f"{self._config.base_url}/apis/evaluator/v2/workspaces/{workspace}/tasks/{entity_name}" + if fragment: + url = f"{url}/revisions/{_strip_digest_prefix(fragment)}" + task = await self._client.get_json(url) + + spec = task.get("spec") or {} + if spec.get("kind") != "harbor": + raise ValueError( + f"Taskset member {ref!r} is a {spec.get('kind')!r} task, not a Harbor package." + ) + org, name = from_entity_name(entity_name) + return org, name, spec["archive_digest"] + + @override + async def _get_dataset_metadata(self, name: str) -> DatasetMetadata: + reference = PackageReference.parse(name) + try: + entity_name = to_entity_name(reference.org, reference.short_name) + except NameMappingError as exc: + raise ValueError(str(exc)) from exc + + url = self._taskset_url(entity_name) + if reference.ref: + # Strip Harbor's `sha256:` prefix. NeMo writes a revision digest as bare hex + # precisely so a `#` fragment stays free of ':', which the entity-ref charset does + # not admit — and the route's own path pattern rejects it with a 422, not a 404. + # This matters beyond hand-typed refs: `version` on the metadata we return carries + # the prefix (Harbor's convention), and Harbor feeds it straight back in when it + # re-resolves a dataset, which is how `harbor run -d` hit it. + url = f"{url}/revisions/{_strip_digest_prefix(reference.ref)}" + + try: + taskset = await self._client.get_json(url) + except NotFound as exc: + # The not-found contract: `package_type` distinguishes absent from broken on + # exactly this, and the dataset probe runs first, so a wrong exception type here + # would stop a task from ever being found. + raise ValueError(f"Dataset not found: {name}") from exc + + task_ids: list[GitTaskId | LocalTaskId | PackageTaskId] = [] + for member in taskset.get("tasks") or []: + org, short_name, digest = await self._member_archive_digest(member) + task_ids.append(PackageTaskId(org=org, name=short_name, ref=f"sha256:{digest}")) + + files: list[DatasetFileInfo] = [] + raw_files = _metadata_value(taskset, DATASET_FILES_METADATA_KEY) + if raw_files: + for entry in json.loads(raw_files): + files.append( + DatasetFileInfo( + path=entry["path"], + storage_path=entry["storage_path"], + content_hash=entry["content_hash"], + ) + ) + + revisions = await self._revision_hash(entity_name, taskset.get("revision")) + return DatasetMetadata( + name=_metadata_value(taskset, PACKAGE_NAME_METADATA_KEY) or reference.name, + version=f"sha256:{revisions}" if revisions else None, + description=taskset.get("description") or "", + task_ids=task_ids, + metrics=[], + files=files, + dataset_version_id=f"{self._config.workspace}/{entity_name}#{taskset.get('revision')}", + dataset_version_content_hash=revisions or None, + ) + + async def _revision_hash(self, entity_name: str, revision: int | None) -> str: + if revision is None: + return "" + try: + listing = await self._client.get_json(f"{self._taskset_url(entity_name)}/revisions") + except NotFound: + return "" + for entry in listing.get("data", []): + if entry.get("revision") == revision: + return entry.get("content_hash", "") + return "" + + @override + async def list_datasets(self) -> list[DatasetSummary]: + listing = await self._client.get_json( + self._config.tasksets_url, params={"page_size": 100} + ) + summaries: list[DatasetSummary] = [] + for taskset in listing.get("data", []): + entity_name = taskset.get("name", "") + try: + org, short_name = from_entity_name(entity_name) + harbor_name = f"{org}/{short_name}" + except NameMappingError: + # A taskset created outside Harbor has no org prefix. Listing it under its raw + # name is more useful than hiding it or failing the whole listing. + harbor_name = entity_name + summaries.append( + DatasetSummary( + name=_metadata_value(taskset, PACKAGE_NAME_METADATA_KEY) or harbor_name, + description=taskset.get("description") or "", + task_count=len(taskset.get("tasks") or []), + ) + ) + return summaries + + @override + async def download_dataset_files( + self, + metadata: DatasetMetadata, + overwrite: bool = False, + output_dir: Path | None = None, + ) -> dict[str, Path]: + from harbor.constants import DATASET_CACHE_DIR + + if not metadata.files: + return {} + + if output_dir is not None: + cache_dir = output_dir + else: + org, _, short_name = metadata.name.partition("/") + version = metadata.dataset_version_content_hash or "unversioned" + cache_dir = DATASET_CACHE_DIR / org / short_name / version + + result: dict[str, Path] = {} + for file_info in metadata.files: + local_path = cache_dir / file_info.path + if not local_path.exists() or overwrite: + await self._storage.download_file(file_info.storage_path, local_path) + result[file_info.path] = local_path + return result diff --git a/packages/harbor_nemo/src/harbor_nemo/names.py b/packages/harbor_nemo/src/harbor_nemo/names.py new file mode 100644 index 0000000000..ec5c09b1fb --- /dev/null +++ b/packages/harbor_nemo/src/harbor_nemo/names.py @@ -0,0 +1,84 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Mapping between Harbor package references and NeMo entity names. + +Harbor addresses a package as ``org/short-name``. NeMo addresses a task as +``workspace/name``, where *workspace* is a tenancy boundary with its own lifecycle and +authorization, not a cheap self-serve namespace like a Harbor org. Creating one per org on +publish would make ``harbor publish`` a tenancy operation, so the org is folded into the +entity name instead: ``nvidia/my-task`` becomes ``nvidia.my-task`` in a single workspace. + +The cost is that the org is a naming convention rather than an enforced boundary. Anyone who +can publish to the workspace can publish under any org prefix. +""" + +from __future__ import annotations + +import re + +#: The entity *store's* name rule, which is stricter than the evaluator route's own +#: ``^[\w\-\.]+$``/255. A name that passes the route can still be rejected by the store, so this +#: is the one worth validating against — it is the one that actually fails, and it fails late. +_ENTITY_NAME_PATTERN = re.compile(r"^[a-z](?!.*--)[a-z0-9\-@.+_]{1,62}$") + +#: Names may not end with a hyphen (the store's rule carries a trailing negative lookbehind). +_TRAILING_HYPHEN = re.compile(r"-$") + +MAX_ENTITY_NAME_LENGTH = 63 + + +class NameMappingError(ValueError): + """A Harbor reference cannot be represented as a NeMo entity name. + + Deliberately a ``ValueError``: on the read path this is indistinguishable from "no such + package", because a reference NeMo could never have stored is a reference NeMo does not + have. The publish path catches it and re-raises as a backend error, where the caller can + act on it. + """ + + +def to_entity_name(org: str, name: str) -> str: + """Map ``org``/``name`` to the NeMo entity name that holds it. + + Raises :class:`NameMappingError` when the result could not be stored. Validating here + rather than letting the store reject it turns a late, opaque 422 into a message that names + the actual constraint. + """ + if "." in org: + # The decode splits on the first dot, so a dotted org would be ambiguous with a dotted + # package name. Rejecting is better than a silent mis-split on the way back out. + raise NameMappingError( + f"Harbor org {org!r} contains a '.', which cannot be represented: the org and " + f"package name are joined with '.' and split on the first one." + ) + + entity_name = f"{org}.{name}" + + if len(entity_name) > MAX_ENTITY_NAME_LENGTH: + raise NameMappingError( + f"{org}/{name} maps to {entity_name!r} ({len(entity_name)} chars), over the " + f"{MAX_ENTITY_NAME_LENGTH}-character entity-name limit." + ) + if not _ENTITY_NAME_PATTERN.match(entity_name) or _TRAILING_HYPHEN.search(entity_name): + raise NameMappingError( + f"{org}/{name} maps to {entity_name!r}, which is not a valid entity name. Names " + f"must start with a lowercase letter, use only [a-z0-9-@.+_], contain no " + f"consecutive hyphens, and not end with a hyphen." + ) + return entity_name + + +def from_entity_name(entity_name: str) -> tuple[str, str]: + """Recover ``(org, name)`` from a NeMo entity name. + + Splits on the *first* dot, which is why :func:`to_entity_name` refuses a dotted org: a + package name may contain dots (``nvidia.my.task`` -> ``nvidia``, ``my.task``), but an org + containing one would make the split ambiguous. + """ + org, separator, name = entity_name.partition(".") + if not separator: + raise NameMappingError( + f"Entity name {entity_name!r} has no '.' separating org from package name." + ) + return org, name diff --git a/packages/harbor_nemo/src/harbor_nemo/publisher.py b/packages/harbor_nemo/src/harbor_nemo/publisher.py new file mode 100644 index 0000000000..ce47db8c2b --- /dev/null +++ b/packages/harbor_nemo/src/harbor_nemo/publisher.py @@ -0,0 +1,416 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The publish path: Harbor packages into NeMo tasks and tasksets.""" + +from __future__ import annotations + +import json +import tempfile +import time +from pathlib import Path +from typing import Any, override + +from harbor.constants import ARCHIVE_FILENAME +from harbor.models.dataset.manifest import DatasetManifest +from harbor.models.dataset.paths import DatasetPaths +from harbor.models.task.config import TaskConfig +from harbor.models.task.paths import TaskPaths +from harbor.models.task.task import Task +from harbor.publisher.base import BasePublisher +from harbor.publisher.errors import PublishBackendError +from harbor.publisher.packager import Packager +from harbor.publisher.results import ( + DatasetPublishResult, + FilePublishResult, + PublishResult, +) + +from harbor_nemo.client import NemoClient, NotFound +from harbor_nemo.config import NemoConfig +from harbor_nemo.names import NameMappingError, to_entity_name +from harbor_nemo.storage import NemoStorage +from harbor_nemo.task_resolver import NemoTaskResolver + +LATEST_TAG = "latest" + +#: Metadata key under which a dataset's dataset-level files are recorded. A taskset has no +#: field for them, so they ride as a JSON string here. This is the stopgap: the durable fix is +#: a file-reference field on the taskset entity, at which point this key becomes legacy. +DATASET_FILES_METADATA_KEY = "harbor.files" + +#: Harbor's package name (``org/short-name``) as published, kept alongside the folded entity +#: name so the original reference survives the mapping and can be shown back to users. +PACKAGE_NAME_METADATA_KEY = "harbor.package_name" + +#: Harbor's ``visibility``. NeMo has no per-entity visibility — access is a workspace-level +#: question — so recording it keeps the publisher's argument from being silently discarded, +#: but it is documentation, not enforcement. +VISIBILITY_METADATA_KEY = "harbor.visibility" + + +def _metadata(items: dict[str, str]) -> list[dict[str, str]]: + return [{"key": key, "value": value} for key, value in items.items()] + + +class NemoPublisher(BasePublisher): + """Publishes Harbor packages to a NeMo platform. + + Storage is injected rather than constructed here: the archive path recorded on a task is a + literal path issued by one fileset, so the publisher must write blobs to the same place + the backend's resolver will later read them from. + + ``_create_archive`` and ``remote_path`` are inherited untouched, which is what keeps an + archive a pure function of file contents — a package published here is byte-identical to + the same package on the public Hub, so a content hash computed against one is still valid + against the other. + """ + + def __init__( + self, + client: NemoClient, + config: NemoConfig, + storage: NemoStorage, + resolver: "NemoTaskResolver", + ) -> None: + self._client = client + self._config = config + self.storage = storage + # Publishing a dataset means resolving its members' pins, so the publisher needs the + # same resolver the download side uses — not a second one that could point elsewhere. + self._resolver = resolver + + def _task_url(self, entity_name: str) -> str: + return f"{self._config.tasks_url}/{entity_name}" + + def _taskset_url(self, entity_name: str) -> str: + return f"{self._config.tasksets_url}/{entity_name}" + + @override + async def check_auth(self) -> None: + """Confirm the platform is reachable and the caller may read the workspace. + + Any 401/403 is already a ``PublishAuthError``/``PublishPermissionError`` by the time + the client returns, so this only has to make a cheap authenticated request and let a + missing workspace surface as a backend error rather than an auth one. + """ + try: + await self._client.get_json(self._config.tasks_url, params={"page_size": 1}) + except NotFound as exc: + raise PublishBackendError( + f"Workspace {self._config.workspace!r} does not exist on " + f"{self._config.base_url}." + ) from exc + + async def _get_task(self, entity_name: str) -> dict[str, Any] | None: + try: + return await self._client.get_json(self._task_url(entity_name)) + except NotFound: + return None + + @override + async def publish_file(self, package_name: str, file_path: Path) -> FilePublishResult: + content_hash = Packager.compute_file_hash(file_path) + remote_path = self.remote_path(package_name, content_hash, file_path.name) + file_size = file_path.stat().st_size + + upload_start = time.monotonic() + # Check-then-act: two concurrent publishes of identical content can both miss here and + # both upload. That is safe *because* the path is content addressed — they write + # byte-identical bytes to the same key — but it does mean `skipped` is a report of + # what this call observed, not a distributed lock. + skipped = await self.storage.exists(remote_path) + if not skipped: + await self.storage.upload_file(file_path, remote_path) + upload_time = time.monotonic() - upload_start + + return FilePublishResult( + content_hash=content_hash, + # The self-describing reference, not the bare path: this becomes + # `DatasetFileInfo.storage_path` and is later handed back to `download_file`. + remote_path=self.storage.to_fileset_ref(remote_path), + file_size_bytes=file_size, + upload_time_sec=round(upload_time, 3), + skipped=skipped, + ) + + async def _put_task(self, entity_name: str, body: dict[str, Any]) -> tuple[dict[str, Any], bool]: + """Publish a revision. Returns ``(task, created)``. + + The platform answers 201 when it cut a new revision and 200 when the content was + already published — that status code is the only place the distinction appears, and it + is exactly Harbor's ``skipped``/``db_skipped`` signal. + """ + response = await self._client.request("PUT", self._task_url(entity_name), json=body) + return response.json(), response.status_code == 201 + + @override + async def publish_task( + self, + task_dir: Path, + tags: set[str] | None = None, + visibility: str = "public", + ) -> PublishResult: + paths = TaskPaths(task_dir) + if not paths.config_path.exists(): + raise FileNotFoundError(f"task.toml not found in {task_dir}") + + config = TaskConfig.model_validate_toml(paths.config_path.read_text()) + if config.task is None: + raise ValueError("task.toml must contain a [task] section with a name") + if not paths.environment_dir.exists(): + raise ValueError(f"Task directory {task_dir} is missing environment/.") + try: + Task(task_dir) + except FileNotFoundError as exc: + raise ValueError(str(exc)) from exc + + try: + entity_name = to_entity_name(config.task.org, config.task.short_name) + except NameMappingError as exc: + # On the publish path this is a real, actionable failure rather than a miss, so it + # is reported as one instead of riding out as the read path's "not found". + raise PublishBackendError(str(exc)) from exc + + applied_tags = {LATEST_TAG} | (tags or set()) + build_start = time.monotonic() + content_hash, files = Packager.compute_content_hash(task_dir) + + # Preflight *before* building the archive: if this exact content is already the task's + # current spec, there is nothing to package or upload. Tags may still need to move, so + # this decides whether to skip the build — not whether to skip the request. + existing = await self._get_task(entity_name) + existing_spec = (existing or {}).get("spec") or {} + content_already_published = ( + existing_spec.get("kind") == "harbor" + and existing_spec.get("archive_digest") == content_hash + ) + + archive_size = 0 + upload_time = 0.0 + if content_already_published: + archive_ref = existing_spec["archive_ref"] + build_time = time.monotonic() - build_start + existing_tags = (existing or {}).get("tags") or {} + existing_revision = (existing or {}).get("revision") + if all(existing_tags.get(tag) == existing_revision for tag in applied_tags): + # Nothing at all to do: same content, same tags. No request, no upload. + return PublishResult( + name=config.task.name, + content_hash=content_hash, + archive_path=archive_ref, + file_count=len(files), + archive_size_bytes=0, + build_time_sec=round(build_time, 3), + upload_time_sec=0.0, + rpc_time_sec=0.0, + skipped=True, + revision=None, + tags=sorted(applied_tags), + db_skipped=True, + ) + else: + remote_path = self.remote_path(config.task.name, content_hash, ARCHIVE_FILENAME) + with tempfile.TemporaryDirectory() as tmp: + archive_path = Path(tmp) / ARCHIVE_FILENAME + self._create_archive(task_dir, files, archive_path) + archive_size = archive_path.stat().st_size + build_time = time.monotonic() - build_start + + upload_start = time.monotonic() + # Upload the blob *before* registering the task. The reverse order would let a + # crash in between leave a task pointing at an archive that was never written, + # and every later publish would then see matching content and report "skipped" + # forever. An orphaned blob is inert by comparison. + await self.storage.upload_file(archive_path, remote_path) + upload_time = time.monotonic() - upload_start + archive_ref = self.storage.to_fileset_ref(remote_path) + + instruction: str | None + if paths.instruction_path.exists(): + instruction = paths.instruction_path.read_text() + elif config.steps: + instruction = None + else: + instruction = "" + + body = { + "spec": { + "kind": "harbor", + "archive_ref": archive_ref, + "archive_digest": content_hash, + "instruction": instruction, + # Stored whole rather than shredded into per-field columns. Nothing on the + # read path needs it — `resolve_version` returns a path and a hash — so + # modelling Harbor's schema here would be cost without a consumer, and it + # would go stale the first time Harbor added a field. + "config": config.model_dump(mode="json"), + }, + "metadata": _metadata( + { + PACKAGE_NAME_METADATA_KEY: config.task.name, + VISIBILITY_METADATA_KEY: visibility, + } + ), + "tags": sorted(applied_tags - {LATEST_TAG}), + } + + rpc_start = time.monotonic() + if existing is None: + try: + response = await self._client.request( + "POST", self._task_url(entity_name), json=body + ) + task, created = response.json(), True + except PublishBackendError as exc: + # A concurrent publisher created it between our preflight and here. `publish_tasks` + # runs 50-wide, so this is a live race, not a theoretical one. + if "already exists" not in str(exc).lower(): + raise + task, created = await self._put_task(entity_name, body) + else: + task, created = await self._put_task(entity_name, body) + rpc_time = time.monotonic() - rpc_start + + return PublishResult( + name=config.task.name, + content_hash=content_hash, + archive_path=archive_ref, + file_count=len(files), + archive_size_bytes=archive_size, + build_time_sec=round(build_time, 3), + upload_time_sec=round(upload_time, 3), + rpc_time_sec=round(rpc_time, 3), + skipped=not created, + revision=task.get("revision") if created else None, + tags=sorted(applied_tags), + db_skipped=not created, + ) + + @override + async def publish_dataset( + self, + dataset_dir: Path, + tags: set[str] | None = None, + visibility: str = "public", + promote_tasks: bool = False, + ) -> DatasetPublishResult: + paths = DatasetPaths(dataset_dir) + if not paths.manifest_path.exists(): + raise FileNotFoundError(f"dataset.toml not found in {dataset_dir}") + + manifest = DatasetManifest.from_toml_file(paths.manifest_path) + try: + entity_name = to_entity_name(manifest.dataset.org, manifest.dataset.short_name) + except NameMappingError as exc: + raise PublishBackendError(str(exc)) from exc + + applied_tags = {LATEST_TAG} | (tags or set()) + + file_infos: list[dict[str, Any]] = [] + for file_ref in manifest.files: + file_path = dataset_dir / file_ref.path + if not file_path.exists(): + raise FileNotFoundError( + f"Dataset file '{file_ref.path}' not found in {dataset_dir}" + ) + result = await self.publish_file(manifest.dataset.name, file_path) + file_infos.append( + { + "path": file_ref.path, + "content_hash": result.content_hash, + "size_bytes": result.file_size_bytes, + "storage_path": result.remote_path, + } + ) + + task_refs: list[str] = [] + for ref in manifest.tasks: + try: + member = to_entity_name(ref.org, ref.short_name) + except NameMappingError as exc: + raise PublishBackendError(str(exc)) from exc + + # A Harbor manifest always carries a `sha256:` pin (the field is required), and + # dropping it would make a published dataset resolve to whatever its members' + # `latest` happens to be later — silently changing what a dataset means. Translate + # the archive digest into the revision digest a taskset pins by. + try: + revision_digest = await self._resolver.revision_digest_for_archive( + ref.org, ref.short_name, ref.digest + ) + except ValueError as exc: + raise PublishBackendError( + f"Dataset {manifest.dataset.name} pins {ref.name} at {ref.digest}, " + f"which is not published here: {exc}" + ) from exc + task_refs.append(f"{self._config.workspace}/{member}#{revision_digest}") + + body = { + "description": manifest.dataset.description or None, + "tasks": task_refs, + "metadata": _metadata( + { + PACKAGE_NAME_METADATA_KEY: manifest.dataset.name, + VISIBILITY_METADATA_KEY: visibility, + DATASET_FILES_METADATA_KEY: json.dumps(file_infos), + } + ), + "tags": sorted(applied_tags - {LATEST_TAG}), + } + + rpc_start = time.monotonic() + try: + existing = await self._client.get_json(self._taskset_url(entity_name)) + except NotFound: + existing = None + + if existing is None: + try: + response = await self._client.request( + "POST", self._taskset_url(entity_name), json=body + ) + created = True + except PublishBackendError as exc: + if "already exists" not in str(exc).lower(): + raise + response = await self._client.request( + "PUT", self._taskset_url(entity_name), json=body + ) + created = response.status_code == 201 + else: + response = await self._client.request( + "PUT", self._taskset_url(entity_name), json=body + ) + created = response.status_code == 201 + rpc_time = time.monotonic() - rpc_start + taskset = response.json() + + return DatasetPublishResult( + name=manifest.dataset.name, + content_hash=await self._revision_hash(entity_name, taskset.get("revision")), + revision=taskset.get("revision") or 0, + task_count=manifest.task_count, + file_count=len(file_infos), + skipped=not created, + db_skipped=not created, + rpc_time_sec=round(rpc_time, 3), + tags=sorted(applied_tags), + ) + + async def _revision_hash(self, entity_name: str, revision: int | None) -> str: + """The taskset revision's content digest, for the publish report. + + Best effort: this is display data on a result that has already succeeded, so a failure + to read it back must not turn a completed publish into an error. + """ + if revision is None: + return "" + try: + listing = await self._client.get_json(f"{self._taskset_url(entity_name)}/revisions") + except Exception: # noqa: BLE001 - see docstring + return "" + for entry in listing.get("data", []): + if entry.get("revision") == revision: + return entry.get("content_hash", "") + return "" diff --git a/packages/harbor_nemo/src/harbor_nemo/storage.py b/packages/harbor_nemo/src/harbor_nemo/storage.py new file mode 100644 index 0000000000..0f6cbf19a3 --- /dev/null +++ b/packages/harbor_nemo/src/harbor_nemo/storage.py @@ -0,0 +1,108 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Blob storage for published archives, backed by the NeMo Files service.""" + +from __future__ import annotations + +from pathlib import Path +from typing import override + +from harbor.storage.base import BaseStorage + +from harbor_nemo.client import NemoClient, NotFound +from harbor_nemo.config import NemoConfig + +#: Separator in a fileset reference: ``workspace/fileset#path-within-fileset``. +FILESET_REF_SEPARATOR = "#" + + +class NemoStorage(BaseStorage): + """Reads and writes package blobs in a single NeMo fileset. + + Accepts two shapes of ``remote_path``, which is not an accident: + + * A **bare path** (``packages/nvidia.my-task//dist.tar.gz``) is resolved against the + configured workspace and fileset. ``BasePublisher.remote_path`` — which backends must not + override — produces exactly this, so uploads always arrive in this form. + * A **full fileset reference** (``default/harbor-packages#packages/...``) is used as + given. This is what gets *stored* on a task, and what comes back out of + ``ResolvedTaskVersion.archive_path`` on the download side. + + Storing the full reference rather than the bare path is deliberate. ``download_file`` + receives only a string, with no workspace or fileset alongside it, so a bare path + published against one workspace would silently resolve against whichever workspace the + environment happens to name at download time — asking a fileset for a blob it never + stored. A self-describing reference cannot be pointed at the wrong host by a changed + environment variable. + """ + + def __init__(self, client: NemoClient, config: NemoConfig) -> None: + self._client = client + self._config = config + self._ensured_filesets: set[tuple[str, str]] = set() + + def _resolve(self, remote_path: str) -> tuple[str, str, str]: + """Split ``remote_path`` into ``(workspace, fileset, path)``.""" + if FILESET_REF_SEPARATOR in remote_path: + location, _, path = remote_path.partition(FILESET_REF_SEPARATOR) + workspace, _, fileset = location.partition("/") + if not fileset: + # "fileset#path" with no workspace: legal in the platform's own path parser, + # so accept it rather than failing on a form users will reasonably write. + return self._config.workspace, workspace, path + return workspace, fileset, path + return self._config.workspace, self._config.fileset, remote_path + + def _file_url(self, workspace: str, fileset: str, path: str) -> str: + base = f"{self._config.base_url}/apis/files/v2/workspaces/{workspace}/filesets" + return f"{base}/{fileset}/-/{path}" + + def to_fileset_ref(self, remote_path: str) -> str: + """Render ``remote_path`` as the self-describing reference to store on a task.""" + workspace, fileset, path = self._resolve(remote_path) + return f"{workspace}/{fileset}{FILESET_REF_SEPARATOR}{path}" + + async def _ensure_fileset(self, workspace: str, fileset: str) -> None: + """Create the fileset if it does not exist, tolerating a concurrent creator. + + ``publish_tasks`` runs up to 50 publishes at once against an empty workspace, so this + races with itself on the very first publish. A 409 means someone else won, which is + the outcome we wanted anyway. + """ + if (workspace, fileset) in self._ensured_filesets: + return + base = f"{self._config.base_url}/apis/files/v2/workspaces/{workspace}/filesets" + try: + await self._client.request("POST", base, json={"name": fileset}) + except Exception as exc: # noqa: BLE001 - re-raised below unless it is a benign conflict + if "already exists" not in str(exc).lower() and "conflict" not in str(exc).lower(): + raise + self._ensured_filesets.add((workspace, fileset)) + + async def exists(self, remote_path: str) -> bool: + """Whether a blob is already present, via HEAD rather than a full download.""" + workspace, fileset, path = self._resolve(remote_path) + try: + await self._client.request("HEAD", self._file_url(workspace, fileset, path)) + except NotFound: + return False + return True + + @override + async def upload_file(self, file_path: Path, remote_path: str) -> None: + workspace, fileset, path = self._resolve(remote_path) + await self._ensure_fileset(workspace, fileset) + await self._client.request( + "PUT", + self._file_url(workspace, fileset, path), + content=file_path.read_bytes(), + headers={"Content-Type": "application/octet-stream"}, + ) + + @override + async def download_file(self, remote_path: str, file_path: Path) -> None: + workspace, fileset, path = self._resolve(remote_path) + response = await self._client.request("GET", self._file_url(workspace, fileset, path)) + file_path.parent.mkdir(parents=True, exist_ok=True) + file_path.write_bytes(response.content) diff --git a/packages/harbor_nemo/src/harbor_nemo/task_resolver.py b/packages/harbor_nemo/src/harbor_nemo/task_resolver.py new file mode 100644 index 0000000000..c06c0d0978 --- /dev/null +++ b/packages/harbor_nemo/src/harbor_nemo/task_resolver.py @@ -0,0 +1,187 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Resolving ``org/name@ref`` to an archive on the NeMo platform.""" + +from __future__ import annotations + +import re +from typing import Any, override + +from harbor.models.registry import ResolvedTaskVersion +from harbor.registry.task_resolver import BaseTaskResolver + +from harbor_nemo.client import NemoClient, NotFound +from harbor_nemo.config import NemoConfig +from harbor_nemo.names import NameMappingError, to_entity_name + +#: Harbor writes a pinned reference as ``sha256:`` (see ``PackageTaskId.ref``). +_SHA256_PREFIX = "sha256:" +_HEX_DIGEST = re.compile(r"^[0-9a-f]{64}$") + +#: How many revisions to walk when resolving a Harbor content hash. Generous: the answer is +#: almost always the head or one of the last few revisions, and the alternative to a bound is +#: an unbounded scan of a task republished thousands of times. +_MAX_REVISION_SCAN = 200 + + +class NemoTaskResolver(BaseTaskResolver): + """Resolves Harbor task references against NeMo's stored tasks. + + **Two digests are in play and they are not interchangeable.** NeMo addresses a revision by + a digest of the revision's *content* (canonical JSON of the stored spec). Harbor addresses + a version by a digest of the task *directory's files*, which NeMo stores as a field, + ``spec.archive_digest``. A ``sha256:`` reference arriving here is always Harbor's, and + passing it to NeMo's revision selector returns 404 — verified against a live platform. + So a content-pinned lookup is a scan over revisions comparing ``archive_digest``, not a + direct fetch. + + ``record_download`` is deliberately left as the inherited no-op. The platform has no + counter primitive, so implementing it would mean a read-modify-write against the single + hottest entity per package on every download — a poor trade for best-effort telemetry. + """ + + def __init__(self, client: NemoClient, config: NemoConfig) -> None: + self._client = client + self._config = config + + def _task_url(self, entity_name: str) -> str: + return f"{self._config.tasks_url}/{entity_name}" + + @staticmethod + def _to_resolved(entity_name: str, workspace: str, task: dict[str, Any]) -> ResolvedTaskVersion: + spec = task.get("spec") or {} + if spec.get("kind") != "harbor": + # An agent-eval task stored under a name Harbor asked for. Not a Harbor package, + # so from Harbor's point of view it does not exist — and saying so as ValueError + # keeps `package_type` able to fall through to the dataset probe. + raise ValueError( + f"Task {workspace}/{entity_name} is a {spec.get('kind')!r} task, not a Harbor package." + ) + revision = task.get("revision") + return ResolvedTaskVersion( + id=f"{workspace}/{entity_name}#{revision}", + archive_path=spec["archive_ref"], + content_hash=spec["archive_digest"], + revision=revision, + ) + + async def _revisions(self, entity_name: str) -> list[dict[str, Any]]: + """The task's revisions, newest first. + + Sorted here rather than trusting a query parameter: the ordering this scan depends on + is worth owning, and the listing is already bounded by ``page_size``. + """ + listing = await self._client.get_json( + f"{self._task_url(entity_name)}/revisions", + params={"page_size": _MAX_REVISION_SCAN}, + ) + entries = listing.get("data", []) + return sorted(entries, key=lambda entry: entry.get("revision", 0), reverse=True) + + async def _resolve_by_archive_digest( + self, entity_name: str, digest: str + ) -> ResolvedTaskVersion: + """Find the revision whose task directory hashed to ``digest``. + + Checks the head first — republishing the same content is the common case, so the + current revision is the likely answer and costs one request. + """ + head = await self._client.get_json(self._task_url(entity_name)) + if (head.get("spec") or {}).get("archive_digest") == digest: + return self._to_resolved(entity_name, self._config.workspace, head) + + entries = await self._revisions(entity_name) + for entry in entries: + # Fetch by the revision's *content hash*, never its ordinal. The platform reads a + # non-digest fragment as a tag name, so `/revisions/2` is a lookup for a tag called + # "2" and 404s — which surfaced as a bogus "task version not found" for any + # digest-pinned download that was not the head. + revision = await self._client.get_json( + f"{self._task_url(entity_name)}/revisions/{entry['content_hash']}" + ) + if (revision.get("spec") or {}).get("archive_digest") == digest: + return self._to_resolved(entity_name, self._config.workspace, revision) + + scanned = len(entries) + hint = ( + f" (scanned the most recent {scanned}; a match older than that would not be found)" + if scanned >= _MAX_REVISION_SCAN + else "" + ) + raise ValueError(f"No revision of {entity_name} has content hash {digest}{hint}") + + async def revision_digest_for_archive(self, org: str, name: str, archive_digest: str) -> str: + """The NeMo *revision* digest of the revision whose archive hashed to ``archive_digest``. + + Exists to pin a taskset member. A Harbor manifest pins a task by the archive digest, + while a taskset pins by NeMo's revision digest, so publishing a dataset with its pins + intact means translating between the two hash spaces — one lookup per member, which is + why an unpinned member ref is the cheaper (and lossier) alternative. + """ + entity_name = to_entity_name(org, name) + digest = ( + archive_digest[len(_SHA256_PREFIX) :] + if archive_digest.startswith(_SHA256_PREFIX) + else archive_digest + ) + entries = await self._revisions(entity_name) + by_ordinal = {entry.get("revision"): entry.get("content_hash", "") for entry in entries} + + head = await self._client.get_json(self._task_url(entity_name)) + if (head.get("spec") or {}).get("archive_digest") == digest: + current = by_ordinal.get(head.get("revision")) + if current: + return current + + for entry in entries: + revision = await self._client.get_json( + f"{self._task_url(entity_name)}/revisions/{entry['content_hash']}" + ) + if (revision.get("spec") or {}).get("archive_digest") == digest: + return entry["content_hash"] + + raise ValueError(f"No revision of {org}/{name} has content hash {digest}") + + @override + async def resolve_version( + self, org: str, name: str, ref: str = "latest" + ) -> ResolvedTaskVersion: + # A reference NeMo could never have stored is a reference NeMo does not have. + # NameMappingError is a ValueError, so this already reads as "not found". + entity_name = to_entity_name(org, name) + + selector = ref[len(_SHA256_PREFIX) :] if ref.startswith(_SHA256_PREFIX) else ref + try: + if ref.startswith(_SHA256_PREFIX) and _HEX_DIGEST.match(selector): + return await self._resolve_by_archive_digest(entity_name, selector) + + if selector.isdigit(): + # Harbor's `ref` may be a revision ordinal, but the platform reads any + # non-digest fragment as a *tag*, so asking for `/revisions/2` looks for a tag + # named "2". Translate the ordinal to that revision's content hash first. + ordinal = int(selector) + for entry in await self._revisions(entity_name): + if entry.get("revision") == ordinal: + selector = entry["content_hash"] + break + else: + raise ValueError( + f"Task version not found: {org}/{name}@{ref} " + f"(no revision {ordinal})" + ) + + # A tag, or a NeMo revision digest — both of which the platform's own revision + # selector understands directly. + task = await self._client.get_json( + f"{self._task_url(entity_name)}/revisions/{selector}" + ) + return self._to_resolved(entity_name, self._config.workspace, task) + except NotFound as exc: + # The load-bearing translation. `BaseRegistryBackend.package_type` catches exactly + # ValueError to tell "absent" from "broken"; auth and transport failures are + # already separate exception types by the time they reach here, so they propagate. + raise ValueError(f"Task version not found: {org}/{name}@{ref}") from exc + + +__all__ = ["NemoTaskResolver", "NameMappingError"] diff --git a/packages/harbor_nemo/tests/conftest.py b/packages/harbor_nemo/tests/conftest.py new file mode 100644 index 0000000000..0c52069474 --- /dev/null +++ b/packages/harbor_nemo/tests/conftest.py @@ -0,0 +1,55 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +import pytest +from harbor_nemo.client import NemoClient +from harbor_nemo.config import NemoConfig + +BASE_URL = "http://nemo.test" +WORKSPACE = "default" +FILESET = "harbor-packages" + +TASKS_URL = f"{BASE_URL}/apis/evaluator/v2/workspaces/{WORKSPACE}/tasks" +TASKSETS_URL = f"{BASE_URL}/apis/evaluator/v2/workspaces/{WORKSPACE}/tasksets" +FILES_URL = f"{BASE_URL}/apis/files/v2/workspaces/{WORKSPACE}/filesets" + + +@pytest.fixture +def config() -> NemoConfig: + return NemoConfig( + base_url=BASE_URL, + workspace=WORKSPACE, + fileset=FILESET, + token=None, + timeout_sec=5.0, + ) + + +@pytest.fixture +async def client(config: NemoConfig): + nemo_client = NemoClient(config) + yield nemo_client + await nemo_client.aclose() + + +def harbor_task( + *, + archive_digest: str = "a" * 64, + revision: int = 1, + tags: dict[str, int] | None = None, + kind: str = "harbor", +) -> dict: + return { + "id": "task-1", + "name": "nvidia.my-task", + "workspace": WORKSPACE, + "revision": revision, + "tags": tags if tags is not None else {"latest": revision}, + "spec": { + "kind": kind, + "archive_ref": f"{WORKSPACE}/{FILESET}#packages/nvidia/my-task/{archive_digest}/dist.tar.gz", + "archive_digest": archive_digest, + "instruction": "", + "config": {}, + }, + } diff --git a/packages/harbor_nemo/tests/test_dataset_pinning.py b/packages/harbor_nemo/tests/test_dataset_pinning.py new file mode 100644 index 0000000000..7a2f7bfdc2 --- /dev/null +++ b/packages/harbor_nemo/tests/test_dataset_pinning.py @@ -0,0 +1,162 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""A dataset's members must stay pinned to the exact task content the manifest named.""" + +from pathlib import Path + +import httpx +import pytest +import respx +from harbor.publisher.errors import PublishBackendError +from harbor_nemo.dataset_client import NemoDatasetClient +from harbor_nemo.publisher import NemoPublisher +from harbor_nemo.storage import NemoStorage +from harbor_nemo.task_resolver import NemoTaskResolver + +from conftest import TASKS_URL, TASKSETS_URL, WORKSPACE, harbor_task + +TASK_URL = f"{TASKS_URL}/nvidia.my-task" +TASKSET_URL = f"{TASKSETS_URL}/nvidia.my-dataset" + +ARCHIVE_1 = "a" * 64 +ARCHIVE_2 = "b" * 64 +REV_1_HASH = "1" * 64 +REV_2_HASH = "2" * 64 + +DATASET_TOML = f"""\ +[dataset] +name = "nvidia/my-dataset" +version = "0.1.0" +description = "fixture" + +[[tasks]] +name = "nvidia/my-task" +digest = "sha256:{ARCHIVE_1}" +""" + + +@pytest.fixture +def dataset_dir(tmp_path: Path) -> Path: + directory = tmp_path / "my-dataset" + directory.mkdir() + (directory / "dataset.toml").write_text(DATASET_TOML) + return directory + + +def _publisher(client, config) -> NemoPublisher: + return NemoPublisher( + client, config, NemoStorage(client, config), NemoTaskResolver(client, config) + ) + + +def _mock_two_revisions() -> None: + respx.get(f"{TASK_URL}/revisions").mock( + return_value=httpx.Response( + 200, + json={ + "data": [ + {"revision": 1, "content_hash": REV_1_HASH, "tags": []}, + {"revision": 2, "content_hash": REV_2_HASH, "tags": ["latest"]}, + ] + }, + ) + ) + # The head is revision 2 — so pinning revision 1 must NOT be answered from the head. + respx.get(TASK_URL).mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_2, revision=2)) + ) + respx.get(f"{TASK_URL}/revisions/{REV_2_HASH}").mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_2, revision=2)) + ) + respx.get(f"{TASK_URL}/revisions/{REV_1_HASH}").mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_1, revision=1)) + ) + + +@respx.mock +async def test_a_manifest_pin_becomes_a_pinned_taskset_member(client, config, dataset_dir): + """The manifest names an *older* revision by archive digest. The published taskset must + pin that revision's NeMo digest — not the member's current `latest`, which is a different + task directory entirely.""" + _mock_two_revisions() + respx.get(TASKSET_URL).mock(return_value=httpx.Response(404)) + create = respx.post(TASKSET_URL).mock( + return_value=httpx.Response(201, json={"name": "nvidia.my-dataset", "revision": 1}) + ) + respx.get(f"{TASKSET_URL}/revisions").mock(return_value=httpx.Response(200, json={"data": []})) + + await _publisher(client, config).publish_dataset(dataset_dir) + + body = create.calls.last.request.read().decode() + assert f"{WORKSPACE}/nvidia.my-task#{REV_1_HASH}" in body + assert REV_2_HASH not in body, "must not pin the head when the manifest named revision 1" + + +@respx.mock +async def test_a_pin_that_is_not_published_here_fails_loudly(client, config, dataset_dir): + """Silently dropping an unresolvable pin would publish a dataset that means something + different from the one the manifest describes.""" + respx.get(f"{TASK_URL}/revisions").mock(return_value=httpx.Response(200, json={"data": []})) + respx.get(TASK_URL).mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_2, revision=2)) + ) + respx.get(TASKSET_URL).mock(return_value=httpx.Response(404)) + + with pytest.raises(PublishBackendError, match="not published here"): + await _publisher(client, config).publish_dataset(dataset_dir) + + +@respx.mock +async def test_a_pinned_member_reads_back_as_that_archive_digest(client, config): + """Round trip: the dataset client must report the pinned revision's archive digest, which + is what Harbor turns into `PackageTaskId(ref="sha256:...")`.""" + _mock_two_revisions() + taskset = { + "name": "nvidia.my-dataset", + "revision": 1, + "description": "fixture", + "tasks": [f"{WORKSPACE}/nvidia.my-task#{REV_1_HASH}"], + "metadata": [], + } + # A bare `org/name` parses with ref "latest", so the lookup goes through the revision + # selector rather than the head. + respx.get(f"{TASKSET_URL}/revisions/latest").mock(return_value=httpx.Response(200, json=taskset)) + respx.get(TASKSET_URL).mock(return_value=httpx.Response(200, json=taskset)) + respx.get(f"{TASKSET_URL}/revisions").mock( + return_value=httpx.Response(200, json={"data": [{"revision": 1, "content_hash": "d" * 64}]}) + ) + + metadata = await NemoDatasetClient( + client, config, NemoStorage(client, config) + )._get_dataset_metadata("nvidia/my-dataset") + + assert [task.ref for task in metadata.task_ids] == [f"sha256:{ARCHIVE_1}"] + + +@respx.mock +async def test_a_sha256_prefixed_dataset_ref_is_normalised(client, config): + """`version` on the metadata we return carries Harbor's `sha256:` prefix, and Harbor feeds + it straight back when re-resolving a dataset. NeMo fragments are bare hex, and the route's + path pattern rejects the ':' with a 422 — which is how `harbor run -d` broke.""" + revision_hash = "d" * 64 + route = respx.get(f"{TASKSET_URL}/revisions/{revision_hash}").mock( + return_value=httpx.Response( + 200, + json={ + "name": "nvidia.my-dataset", + "revision": 1, + "description": "", + "tasks": [], + "metadata": [], + }, + ) + ) + respx.get(f"{TASKSET_URL}/revisions").mock( + return_value=httpx.Response(200, json={"data": [{"revision": 1, "content_hash": revision_hash}]}) + ) + + await NemoDatasetClient(client, config, NemoStorage(client, config))._get_dataset_metadata( + f"nvidia/my-dataset@sha256:{revision_hash}" + ) + assert route.called diff --git a/packages/harbor_nemo/tests/test_error_translation.py b/packages/harbor_nemo/tests/test_error_translation.py new file mode 100644 index 0000000000..45284e53d4 --- /dev/null +++ b/packages/harbor_nemo/tests/test_error_translation.py @@ -0,0 +1,70 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Every backend-native failure must arrive as a type a Harbor caller can handle.""" + +import httpx +import pytest +import respx +from harbor.publisher.errors import ( + PublishAuthError, + PublishBackendError, + PublishError, + PublishPermissionError, +) +from harbor_nemo.client import NemoClient, NotFound + +from conftest import TASKS_URL + + +@respx.mock +async def test_401_becomes_an_auth_error(client: NemoClient): + respx.get(TASKS_URL).mock(return_value=httpx.Response(401)) + with pytest.raises(PublishAuthError, match="NMP_TOKEN"): + await client.get_json(TASKS_URL) + + +@respx.mock +async def test_403_becomes_a_permission_error(client: NemoClient): + respx.get(TASKS_URL).mock(return_value=httpx.Response(403)) + with pytest.raises(PublishPermissionError, match="permission"): + await client.get_json(TASKS_URL) + + +@respx.mock +async def test_404_becomes_not_found_not_a_publish_error(client: NemoClient): + """`NotFound` is deliberately not a PublishError: only read paths turn it into the + ValueError that `package_type` keys on, and a publish must not silently treat it as one.""" + respx.get(TASKS_URL).mock(return_value=httpx.Response(404, json={"detail": "nope"})) + with pytest.raises(NotFound): + await client.get_json(TASKS_URL) + + +@respx.mock +async def test_500_carries_the_platforms_own_message(client: NemoClient): + respx.get(TASKS_URL).mock(return_value=httpx.Response(500, json={"detail": "boom"})) + with pytest.raises(PublishBackendError, match="boom") as exc_info: + await client.get_json(TASKS_URL) + assert exc_info.value.message == "boom" + + +@respx.mock +async def test_a_transport_failure_is_never_reported_as_not_found(client: NemoClient): + """The failure mode this whole module exists to prevent: a platform that is down being + reported to the user as a package that does not exist.""" + respx.get(TASKS_URL).mock(side_effect=httpx.ConnectError("refused")) + with pytest.raises(PublishBackendError, match="Could not reach"): + await client.get_json(TASKS_URL) + + +@respx.mock +async def test_a_non_json_error_body_still_produces_a_message(client: NemoClient): + respx.get(TASKS_URL).mock(return_value=httpx.Response(502, text="bad gateway")) + with pytest.raises(PublishBackendError, match="bad gateway"): + await client.get_json(TASKS_URL) + + +def test_permission_error_remains_catchable_as_the_builtin(): + """Harbor's retry predicate classifies the builtin PermissionError as non-retryable.""" + assert issubclass(PublishPermissionError, PermissionError) + assert issubclass(PublishAuthError, PublishError) diff --git a/packages/harbor_nemo/tests/test_names.py b/packages/harbor_nemo/tests/test_names.py new file mode 100644 index 0000000000..82eb756487 --- /dev/null +++ b/packages/harbor_nemo/tests/test_names.py @@ -0,0 +1,49 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +import pytest +from harbor_nemo.names import NameMappingError, from_entity_name, to_entity_name + + +def test_org_is_folded_into_the_entity_name(): + assert to_entity_name("nvidia", "my-task") == "nvidia.my-task" + + +def test_round_trips_a_package_name_containing_dots(): + """The decode splits on the *first* dot, so dots in the package name survive.""" + assert from_entity_name(to_entity_name("nvidia", "my.task")) == ("nvidia", "my.task") + + +def test_rejects_a_dotted_org_rather_than_mis_splitting_it(): + with pytest.raises(NameMappingError, match="contains a '.'"): + to_entity_name("nvidia.labs", "my-task") + + +def test_rejects_a_name_over_the_entity_store_limit(): + # 63 is the store's cap, and it is stricter than the evaluator route's own 255 — a name + # that passes the route can still be rejected by the store, late and opaquely. + with pytest.raises(NameMappingError, match="63-character"): + to_entity_name("nvidia", "x" * 60) + + +def test_rejects_names_the_entity_store_charset_forbids(): + for org, name in [ + ("NVIDIA", "my-task"), # must start lowercase + ("9nvidia", "my-task"), # must start with a letter + ("nvidia", "my--task"), # no consecutive hyphens + ("nvidia", "my-task-"), # no trailing hyphen + ("nvidia", "my task"), # charset + ]: + with pytest.raises(NameMappingError): + to_entity_name(org, name) + + +def test_name_mapping_error_is_a_value_error(): + """The read path relies on this: a reference NeMo could never have stored is a + reference NeMo does not have, and `package_type` tells absent from broken by ValueError.""" + assert issubclass(NameMappingError, ValueError) + + +def test_entity_name_without_a_separator_is_rejected(): + with pytest.raises(NameMappingError, match="no '.'"): + from_entity_name("plain-name") diff --git a/packages/harbor_nemo/tests/test_publisher.py b/packages/harbor_nemo/tests/test_publisher.py new file mode 100644 index 0000000000..9da674bd10 --- /dev/null +++ b/packages/harbor_nemo/tests/test_publisher.py @@ -0,0 +1,213 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The publish contracts: idempotency reported not raised, and preflight before packaging.""" + +from pathlib import Path + +import httpx +import pytest +import respx +from harbor.publisher.base import BasePublisher +from harbor.publisher.packager import Packager +from harbor_nemo.publisher import NemoPublisher +from harbor_nemo.storage import NemoStorage +from harbor_nemo.task_resolver import NemoTaskResolver + +from conftest import FILES_URL, FILESET, TASKS_URL, WORKSPACE, harbor_task + +TASK_URL = f"{TASKS_URL}/nvidia.my-task" + +TASK_TOML = """\ +schema_version = "1.1" + +[task] +name = "nvidia/my-task" +description = "fixture" + +[verifier] +timeout_sec = 60.0 + +[agent] +timeout_sec = 60.0 +""" + + +@pytest.fixture +def task_dir(tmp_path: Path) -> Path: + directory = tmp_path / "my-task" + (directory / "environment").mkdir(parents=True) + (directory / "tests").mkdir() + (directory / "task.toml").write_text(TASK_TOML) + (directory / "instruction.md").write_text("do it") + (directory / "environment" / "Dockerfile").write_text("FROM alpine:3.22\n") + (directory / "tests" / "test.sh").write_text("#!/bin/bash\ntrue\n") + return directory + + +def _publisher(client, config) -> NemoPublisher: + return NemoPublisher(client, config, NemoStorage(client, config), NemoTaskResolver(client, config)) + + +@respx.mock +async def test_first_publish_uploads_and_creates(client, config, task_dir): + content_hash, _ = Packager.compute_content_hash(task_dir) + respx.get(TASK_URL).mock(return_value=httpx.Response(404)) + respx.post(FILES_URL).mock(return_value=httpx.Response(201, json={})) + upload = respx.put(url__startswith=f"{FILES_URL}/{FILESET}/-/").mock( + return_value=httpx.Response(200, json={}) + ) + create = respx.post(TASK_URL).mock( + return_value=httpx.Response(201, json=harbor_task(archive_digest=content_hash)) + ) + + result = await _publisher(client, config).publish_task(task_dir) + + assert upload.called + assert create.called + assert result.skipped is False + assert result.revision == 1 + assert result.content_hash == content_hash + assert result.tags == ["latest"] + + +@respx.mock +async def test_republishing_identical_content_reports_skipped_and_never_packages( + client, config, task_dir +): + """Contract: idempotency is reported, not raised — and the existence check happens + *before* an archive is built, so a no-op publish does no packaging and no upload.""" + content_hash, _ = Packager.compute_content_hash(task_dir) + respx.get(TASK_URL).mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=content_hash)) + ) + upload = respx.put(url__startswith=f"{FILES_URL}/{FILESET}/-/").mock( + return_value=httpx.Response(200, json={}) + ) + replace = respx.put(TASK_URL).mock(return_value=httpx.Response(200, json=harbor_task())) + + result = await _publisher(client, config).publish_task(task_dir) + + assert result.skipped is True + assert result.db_skipped is True + assert result.revision is None + assert result.archive_size_bytes == 0 + assert not upload.called, "identical content must not be re-uploaded" + assert not replace.called, "identical content with identical tags needs no request at all" + + +@respx.mock +async def test_identical_content_with_a_new_tag_still_moves_the_tag(client, config, task_dir): + """Skipping the *package* must not mean skipping the tag the user asked for.""" + content_hash, _ = Packager.compute_content_hash(task_dir) + respx.get(TASK_URL).mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=content_hash)) + ) + upload = respx.put(url__startswith=f"{FILES_URL}/{FILESET}/-/").mock( + return_value=httpx.Response(200, json={}) + ) + replace = respx.put(TASK_URL).mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=content_hash)) + ) + + result = await _publisher(client, config).publish_task(task_dir, tags={"stable"}) + + assert replace.called, "a requested tag that is not applied must still be published" + assert not upload.called, "the archive is unchanged, so it must not be re-uploaded" + assert result.skipped is True + assert result.tags == ["latest", "stable"] + + +@respx.mock +async def test_the_platforms_200_versus_201_is_the_skipped_signal(client, config, task_dir): + """A PUT that dedups server-side answers 200; a new revision answers 201. That status + code is the only place the distinction appears.""" + respx.get(TASK_URL).mock(return_value=httpx.Response(200, json=harbor_task(archive_digest="f" * 64))) + respx.post(FILES_URL).mock(return_value=httpx.Response(201, json={})) + respx.put(url__startswith=f"{FILES_URL}/{FILESET}/-/").mock(return_value=httpx.Response(200, json={})) + respx.put(TASK_URL).mock(return_value=httpx.Response(200, json=harbor_task())) + + result = await _publisher(client, config).publish_task(task_dir) + assert result.skipped is True + assert result.db_skipped is True + + +@respx.mock +async def test_a_concurrent_creator_falls_back_to_publishing_a_revision(client, config, task_dir): + """`publish_tasks` runs 50-wide, so another publisher can create the task between our + preflight and our POST. Losing that race must not fail the publish.""" + respx.get(TASK_URL).mock(return_value=httpx.Response(404)) + respx.post(FILES_URL).mock(return_value=httpx.Response(201, json={})) + respx.put(url__startswith=f"{FILES_URL}/{FILESET}/-/").mock(return_value=httpx.Response(200, json={})) + respx.post(TASK_URL).mock( + return_value=httpx.Response(409, json={"detail": "Task 'nvidia.my-task' already exists."}) + ) + replace = respx.put(TASK_URL).mock(return_value=httpx.Response(201, json=harbor_task(revision=2))) + + result = await _publisher(client, config).publish_task(task_dir) + + assert replace.called + assert result.revision == 2 + + +@respx.mock +async def test_the_blob_is_uploaded_before_the_task_is_registered(client, config, task_dir): + """Order matters: registering first would let a crash leave a task pointing at an archive + that was never written, and every later publish would then report "skipped" forever.""" + calls: list[str] = [] + respx.get(TASK_URL).mock(return_value=httpx.Response(404)) + respx.post(FILES_URL).mock(return_value=httpx.Response(201, json={})) + respx.put(url__startswith=f"{FILES_URL}/{FILESET}/-/").mock( + side_effect=lambda request: calls.append("upload") or httpx.Response(200, json={}) + ) + respx.post(TASK_URL).mock( + side_effect=lambda request: calls.append("register") or httpx.Response(201, json=harbor_task()) + ) + + await _publisher(client, config).publish_task(task_dir) + assert calls == ["upload", "register"] + + +@respx.mock +async def test_publish_file_skips_an_upload_when_the_blob_is_present(client, config, tmp_path): + blob = tmp_path / "metric.py" + blob.write_text("def score(): return 1.0\n") + content_hash = Packager.compute_file_hash(blob) + remote = BasePublisher.remote_path("nvidia/my-dataset", content_hash, "metric.py") + + respx.head(f"{FILES_URL}/{FILESET}/-/{remote}").mock(return_value=httpx.Response(200)) + upload = respx.put(f"{FILES_URL}/{FILESET}/-/{remote}").mock(return_value=httpx.Response(200, json={})) + + result = await _publisher(client, config).publish_file("nvidia/my-dataset", blob) + + assert result.skipped is True + assert not upload.called + # The self-describing reference, because this becomes DatasetFileInfo.storage_path and is + # later handed straight back to download_file. + assert result.remote_path == f"{WORKSPACE}/{FILESET}#{remote}" + + +@respx.mock +async def test_an_unrepresentable_name_fails_the_publish_loudly(client, config, tmp_path): + """On the read path a bad name reads as "absent"; on the publish path it is a real, + actionable failure and must be reported as one.""" + from harbor.publisher.errors import PublishBackendError + + directory = tmp_path / "my-task" + (directory / "environment").mkdir(parents=True) + (directory / "tests").mkdir() + (directory / "task.toml").write_text(TASK_TOML.replace("nvidia/my-task", "nvidia.labs/my-task")) + (directory / "instruction.md").write_text("do it") + (directory / "environment" / "Dockerfile").write_text("FROM alpine:3.22\n") + (directory / "tests" / "test.sh").write_text("#!/bin/bash\ntrue\n") + + with pytest.raises(PublishBackendError, match="contains a '.'"): + await _publisher(client, config).publish_task(directory) + + +def test_archive_construction_is_inherited_untouched(): + """Overriding either of these would break byte-identity with the public Hub, and with it + the comparability of historical eval results across a migration.""" + assert "_create_archive" not in NemoPublisher.__dict__ + assert "remote_path" not in NemoPublisher.__dict__ + assert "publish_tasks" not in NemoPublisher.__dict__ diff --git a/packages/harbor_nemo/tests/test_storage.py b/packages/harbor_nemo/tests/test_storage.py new file mode 100644 index 0000000000..ec267c18bf --- /dev/null +++ b/packages/harbor_nemo/tests/test_storage.py @@ -0,0 +1,93 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +import httpx +import respx +from harbor_nemo.storage import NemoStorage + +from conftest import FILES_URL, FILESET, WORKSPACE + +BARE_PATH = "packages/nvidia.my-task/abc/dist.tar.gz" +FULL_REF = f"{WORKSPACE}/{FILESET}#{BARE_PATH}" + + +def test_a_bare_path_resolves_against_the_configured_fileset(client, config): + storage = NemoStorage(client, config) + assert storage._resolve(BARE_PATH) == (WORKSPACE, FILESET, BARE_PATH) + + +def test_a_full_reference_is_used_as_given(client, config): + """This is what makes a stored archive path immune to a changed environment: the + reference names its own workspace and fileset, so it cannot be pointed elsewhere.""" + storage = NemoStorage(client, config) + assert storage._resolve(f"other-ws/other-fs#{BARE_PATH}") == ("other-ws", "other-fs", BARE_PATH) + + +def test_a_fileset_only_reference_falls_back_to_the_configured_workspace(client, config): + storage = NemoStorage(client, config) + assert storage._resolve(f"just-a-fileset#{BARE_PATH}") == (WORKSPACE, "just-a-fileset", BARE_PATH) + + +def test_bare_paths_are_rendered_as_self_describing_references(client, config): + assert NemoStorage(client, config).to_fileset_ref(BARE_PATH) == FULL_REF + + +@respx.mock +async def test_upload_creates_the_fileset_once_then_reuses_it(client, config): + """`publish_tasks` runs 50-wide against a possibly empty workspace, so the first publishes + race to create the fileset. Creating it once per process keeps that to one request.""" + create = respx.post(FILES_URL).mock(return_value=httpx.Response(201, json={"name": FILESET})) + put = respx.put(f"{FILES_URL}/{FILESET}/-/{BARE_PATH}").mock(return_value=httpx.Response(200, json={})) + + storage = NemoStorage(client, config) + import tempfile + from pathlib import Path + + with tempfile.TemporaryDirectory() as tmp: + blob = Path(tmp) / "dist.tar.gz" + blob.write_bytes(b"payload") + await storage.upload_file(blob, BARE_PATH) + await storage.upload_file(blob, BARE_PATH) + + assert create.call_count == 1 + assert put.call_count == 2 + + +@respx.mock +async def test_a_concurrent_fileset_creation_is_not_an_error(client, config): + """Losing the create race means someone else made it, which is the outcome we wanted.""" + respx.post(FILES_URL).mock( + return_value=httpx.Response(409, json={"detail": "fileset already exists"}) + ) + respx.put(f"{FILES_URL}/{FILESET}/-/{BARE_PATH}").mock(return_value=httpx.Response(200, json={})) + + import tempfile + from pathlib import Path + + with tempfile.TemporaryDirectory() as tmp: + blob = Path(tmp) / "dist.tar.gz" + blob.write_bytes(b"payload") + await NemoStorage(client, config).upload_file(blob, BARE_PATH) + + +@respx.mock +async def test_exists_uses_head_rather_than_downloading(client, config): + head = respx.head(f"{FILES_URL}/{FILESET}/-/{BARE_PATH}").mock(return_value=httpx.Response(200)) + assert await NemoStorage(client, config).exists(BARE_PATH) is True + assert head.called + + +@respx.mock +async def test_exists_is_false_on_404(client, config): + respx.head(f"{FILES_URL}/{FILESET}/-/{BARE_PATH}").mock(return_value=httpx.Response(404)) + assert await NemoStorage(client, config).exists(BARE_PATH) is False + + +@respx.mock +async def test_download_writes_the_bytes_and_creates_parent_directories(client, config, tmp_path): + respx.get(f"{FILES_URL}/{FILESET}/-/{BARE_PATH}").mock( + return_value=httpx.Response(200, content=b"archive-bytes") + ) + target = tmp_path / "nested" / "deeper" / "dist.tar.gz" + await NemoStorage(client, config).download_file(FULL_REF, target) + assert target.read_bytes() == b"archive-bytes" diff --git a/packages/harbor_nemo/tests/test_task_resolver.py b/packages/harbor_nemo/tests/test_task_resolver.py new file mode 100644 index 0000000000..993f7eb2bc --- /dev/null +++ b/packages/harbor_nemo/tests/test_task_resolver.py @@ -0,0 +1,153 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +import httpx +import pytest +import respx +from harbor.publisher.errors import PublishAuthError +from harbor_nemo.task_resolver import NemoTaskResolver + +from conftest import TASKS_URL, harbor_task + +TASK_URL = f"{TASKS_URL}/nvidia.my-task" + +REV_1_HASH = "1" * 64 +REV_2_HASH = "2" * 64 +ARCHIVE_1 = "a" * 64 +ARCHIVE_2 = "b" * 64 + + +def _revisions_payload() -> dict: + return { + "data": [ + {"revision": 1, "content_hash": REV_1_HASH, "tags": []}, + {"revision": 2, "content_hash": REV_2_HASH, "tags": ["latest"]}, + ] + } + + +@respx.mock +async def test_resolves_a_tag_and_reports_harbors_hash_not_nemos(client, config): + """`content_hash` must carry the *archive* digest: Harbor keys its download cache on it.""" + respx.get(f"{TASK_URL}/revisions/latest").mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_2, revision=2)) + ) + resolved = await NemoTaskResolver(client, config).resolve_version("nvidia", "my-task") + assert resolved.content_hash == ARCHIVE_2 + assert resolved.archive_path.startswith("default/harbor-packages#") + assert resolved.revision == 2 + + +@respx.mock +async def test_a_missing_task_is_a_value_error(client, config): + """Load bearing: `package_type` tells absent from broken by catching exactly ValueError.""" + respx.get(f"{TASK_URL}/revisions/latest").mock(return_value=httpx.Response(404)) + with pytest.raises(ValueError, match="not found"): + await NemoTaskResolver(client, config).resolve_version("nvidia", "my-task") + + +@respx.mock +async def test_an_auth_failure_is_not_downgraded_to_not_found(client, config): + """If this leaked as ValueError, a logged-out user would be told the package is missing.""" + respx.get(f"{TASK_URL}/revisions/latest").mock(return_value=httpx.Response(401)) + with pytest.raises(PublishAuthError): + await NemoTaskResolver(client, config).resolve_version("nvidia", "my-task") + + +@respx.mock +async def test_an_agent_eval_task_is_not_a_harbor_package(client, config): + """A name collision with a non-Harbor task must read as absent, so `package_type` can + fall through to the dataset probe rather than exploding.""" + respx.get(f"{TASK_URL}/revisions/latest").mock( + return_value=httpx.Response(200, json=harbor_task(kind="evaluator")) + ) + with pytest.raises(ValueError, match="not a Harbor package"): + await NemoTaskResolver(client, config).resolve_version("nvidia", "my-task") + + +@respx.mock +async def test_a_content_pinned_ref_hits_the_head_without_scanning(client, config): + """Re-resolving the current content is the common case and must cost one request.""" + head = respx.get(TASK_URL).mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_2, revision=2)) + ) + listing = respx.get(f"{TASK_URL}/revisions").mock(return_value=httpx.Response(200, json={"data": []})) + + resolved = await NemoTaskResolver(client, config).resolve_version( + "nvidia", "my-task", f"sha256:{ARCHIVE_2}" + ) + assert resolved.content_hash == ARCHIVE_2 + assert head.called + assert not listing.called + + +@respx.mock +async def test_a_content_pinned_ref_scans_revisions_by_content_hash_not_ordinal(client, config): + """The regression this pins: the platform reads a non-digest fragment as a *tag*, so + fetching `/revisions/1` looks for a tag named "1" and 404s. Every digest-pinned download + that was not the head failed with a bogus "task version not found".""" + respx.get(TASK_URL).mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_2, revision=2)) + ) + respx.get(f"{TASK_URL}/revisions").mock( + return_value=httpx.Response(200, json=_revisions_payload()) + ) + by_ordinal = respx.get(f"{TASK_URL}/revisions/1").mock(return_value=httpx.Response(404)) + respx.get(f"{TASK_URL}/revisions/{REV_2_HASH}").mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_2, revision=2)) + ) + respx.get(f"{TASK_URL}/revisions/{REV_1_HASH}").mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_1, revision=1)) + ) + + resolved = await NemoTaskResolver(client, config).resolve_version( + "nvidia", "my-task", f"sha256:{ARCHIVE_1}" + ) + assert resolved.content_hash == ARCHIVE_1 + assert resolved.revision == 1 + assert not by_ordinal.called + + +@respx.mock +async def test_a_revision_ordinal_ref_is_translated_to_a_content_hash(client, config): + """Harbor documents `ref` as "a tag, a revision, or a digest". A bare ordinal is not a + valid platform selector, so it has to be looked up rather than passed through.""" + respx.get(f"{TASK_URL}/revisions").mock( + return_value=httpx.Response(200, json=_revisions_payload()) + ) + respx.get(f"{TASK_URL}/revisions/{REV_1_HASH}").mock( + return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_1, revision=1)) + ) + resolved = await NemoTaskResolver(client, config).resolve_version("nvidia", "my-task", "1") + assert resolved.revision == 1 + + +@respx.mock +async def test_an_unknown_ordinal_is_a_value_error(client, config): + respx.get(f"{TASK_URL}/revisions").mock( + return_value=httpx.Response(200, json=_revisions_payload()) + ) + with pytest.raises(ValueError, match="no revision 7"): + await NemoTaskResolver(client, config).resolve_version("nvidia", "my-task", "7") + + +@respx.mock +async def test_a_content_hash_that_no_revision_carries_is_a_value_error(client, config): + respx.get(TASK_URL).mock(return_value=httpx.Response(200, json=harbor_task(archive_digest=ARCHIVE_2))) + respx.get(f"{TASK_URL}/revisions").mock(return_value=httpx.Response(200, json={"data": []})) + with pytest.raises(ValueError, match="No revision"): + await NemoTaskResolver(client, config).resolve_version( + "nvidia", "my-task", f"sha256:{'c' * 64}" + ) + + +async def test_an_unrepresentable_name_reads_as_absent(client, config): + """No request is made: a name NeMo could never have stored is one NeMo does not have.""" + with pytest.raises(ValueError): + await NemoTaskResolver(client, config).resolve_version("nvidia", "X" * 80) + + +async def test_record_download_is_a_no_op(client, config): + """Deliberate: no counter primitive exists, so implementing it would mean a + read-modify-write on the hottest entity per package for best-effort telemetry.""" + assert await NemoTaskResolver(client, config).record_download("anything") is None diff --git a/packages/nemo_platform_plugin/src/nemo_platform_plugin/refs.py b/packages/nemo_platform_plugin/src/nemo_platform_plugin/refs.py index 2c7c555f15..f43586ab29 100644 --- a/packages/nemo_platform_plugin/src/nemo_platform_plugin/refs.py +++ b/packages/nemo_platform_plugin/src/nemo_platform_plugin/refs.py @@ -93,6 +93,14 @@ class LocalDir(StrRef): __cli_metavar__: ClassVar[str | None] = "PATH" +#: Regex form of the shape :func:`parse_entity_ref` accepts: ``name`` or ``workspace/name``, each +#: segment using the platform name charset. Pydantic fields that hold a reference declare +#: ``pattern=ENTITY_REF_PATTERN`` so a malformed ref is rejected at validation rather than surfacing +#: as a confusing failure during parsing; :func:`parse_entity_ref` then only has to split. Kept +#: beside the parser so the two cannot drift apart. +ENTITY_REF_PATTERN = r"^[\w\-.]+(/[\w\-.]+)?$" + + class FilesetRef(StrRef): """A NeMo Platform fileset reference (``"name"`` or ``"workspace/name"``). @@ -104,6 +112,14 @@ class FilesetRef(StrRef): __cli_metavar__: ClassVar[str | None] = "FILESET_REF" +#: A reference to a *file inside* a fileset: ``workspace/fileset#path/inside.ext``. Unlike +#: :data:`ENTITY_REF_PATTERN` the workspace is mandatory (a stored reference must be unambiguous +#: wherever it is later read from), and the ``#`` fragment is a file path, so it admits ``/`` and +#: ``.``. Declared as a field pattern so a malformed reference is rejected when it is stored rather +#: than surfacing as a download failure mid-run. +FILESET_REF_PATTERN = r"^[\w\-.]+/[\w\-.]+#[\w\-./]+$" + + # Documentary union alias — the wire shape is still ``str``. The # ``_spec_flags`` generator collapses this to a single ``--output`` flag # of type ``str``; the disambiguation between the two arms happens in @@ -182,6 +198,8 @@ def parse_entity_ref(identifier: str, default_workspace: str | None = None) -> P __all__ = [ + "ENTITY_REF_PATTERN", + "FILESET_REF_PATTERN", "EndpointURL", "FilesetRef", "LocalDir", diff --git a/plugins/nemo-evaluator/openapi/openapi.yaml b/plugins/nemo-evaluator/openapi/openapi.yaml index 366017a676..eb4a8a2c21 100644 --- a/plugins/nemo-evaluator/openapi/openapi.yaml +++ b/plugins/nemo-evaluator/openapi/openapi.yaml @@ -3461,6 +3461,64 @@ components: title: EvaluateSpec description: Canonical SDK evaluation spec with platform model and metric references resolved. + EvaluatorTaskDefinition: + properties: + kind: + type: string + const: evaluator + title: Kind + description: Task kind discriminator. + intent: + type: string + title: Intent + description: Human-readable description of the desired agent behavior. + inputs: + allOf: + - $ref: '#/components/schemas/TaskInputs' + description: The task's recognized input fields. + metrics: + items: + anyOf: + - $ref: '#/components/schemas/MetricInline' + - $ref: '#/components/schemas/MetricRef' + type: array + title: Metrics + description: "Metrics that score this task \u2014 stored-metric references,\ + \ and inline bundles on create (normalized to derived stored metrics before\ + \ the task is persisted)." + reference: + additionalProperties: true + type: object + title: Reference + description: 'Grader-only ground truth (held-out tests, expected outputs, + rubric data). Surfaced to metrics but never seeded into the agent''s workspace + or shown to the agent, so a metric can grade against artifacts the agent + cannot influence. Held out from the *agent*, not from the API: anyone + who can read the task can read this.' + views: + additionalProperties: + $ref: '#/components/schemas/SemanticView' + type: object + title: Views + description: Optional reporting views mapping metric outputs into named + semantic scores. + additionalProperties: false + type: object + required: + - kind + - intent + title: EvaluatorTaskDefinition + description: "What the agent should do, and how the platform scores it.\n\n\ + ``metrics`` accepts inline bundles on the way in and holds references once\ + \ stored: the service\noffloads an inline metric to a content-addressed *derived*\ + \ metric on create, so a persisted task\nonly ever names metrics it does not\ + \ own. That narrowing is a service invariant rather than a\ntype-level one\ + \ \u2014 a single model keeps the API surface small, at the cost of this field\ + \ being\nwider than what a stored task actually contains.\n\nEvery field here\ + \ is covered by the revision digest, ``reference`` included: it decides what\ + \ a\nmetric grades against, so two revisions that score differently must not\ + \ share a digest. Pinning\na revision therefore fixes the grading, not just\ + \ the prompt." EvidenceDescriptor: anyOf: - required: @@ -3764,6 +3822,60 @@ components: injected from the job''s storage at run time; only the harness-selection and run knobs live here.' + HarborTaskDefinition: + properties: + kind: + type: string + const: harbor + title: Kind + description: Task kind discriminator. + archive_ref: + type: string + pattern: ^[\w\-.]+/[\w\-.]+#[\w\-./]+$ + title: Archive Ref + description: 'Files reference to the task''s packaged directory (format: + workspace/fileset#path).' + archive_digest: + type: string + maxLength: 64 + minLength: 64 + pattern: ^[0-9a-f]{64}$ + title: Archive Digest + description: "Content hash Harbor computed over the task directory. This\ + \ is the authoritative identity of a Harbor task's content \u2014 every\ + \ file, including task.toml." + instruction: + title: Instruction + description: The task's instruction text, when it has one (multi-step tasks + may not). + type: string + config: + additionalProperties: true + type: object + title: Config + description: "Harbor's own task configuration (verifier, agent, environment,\ + \ steps), as published. A queryable projection of task.toml \u2014 inspect\ + \ a task's verifier without downloading the archive. Opaque here: Harbor\ + \ owns this schema." + additionalProperties: false + type: object + required: + - kind + - archive_ref + - archive_digest + title: HarborTaskDefinition + description: "A reference to the task's packaged files, plus a projection of\ + \ Harbor's own config.\n\nHarbor identifies a task by a *directory* \u2014\ + \ ``task.toml``, an instruction, an environment \u2014 so\nwhat is stored\ + \ is a reference to that directory's archive in the Files service, not the\ + \ files\nthemselves. One fileset per task, so a task shared by several tasksets\ + \ is stored once. The\narchive is materialized back into ``//``\ + \ at run time, which is the layout\nHarbor's own discovery expects.\n\nWhich\ + \ agent runs the task is *not* stored here. That comes from the run's target\n\ + (``HarborRunnerTarget``), so the same stored task can be evaluated against\ + \ different agents.\nHarbor's own ``[agent]`` block \u2014 carried inside\ + \ ``config`` \u2014 configures how the agent *phase*\nruns (timeout, user,\ + \ network policy), not which agent it is." HelloResponse: properties: message: @@ -5026,29 +5138,18 @@ components: title: Project description: The project associated with this task. type: string - intent: - type: string - title: Intent - description: Human-readable description of the desired agent behavior. - inputs: - allOf: - - $ref: '#/components/schemas/TaskInputs' - description: The task's recognized input fields. - metrics: - items: - $ref: '#/components/schemas/MetricRef' - type: array - title: Metrics - description: References to the metrics that score this task; inline metrics - submitted on create are normalized to (derived) stored metrics, so a stored - task holds refs only. - views: - additionalProperties: - $ref: '#/components/schemas/SemanticView' - type: object - title: Views - description: Optional reporting views mapping metric outputs into named - semantic scores. + spec: + oneOf: + - $ref: '#/components/schemas/EvaluatorTaskDefinition' + - $ref: '#/components/schemas/HarborTaskDefinition' + title: Spec + description: The task's content, discriminated by which runner executes + it. + discriminator: + propertyName: kind + mapping: + evaluator: '#/components/schemas/EvaluatorTaskDefinition' + harbor: '#/components/schemas/HarborTaskDefinition' metadata: items: $ref: '#/components/schemas/MetadataItem' @@ -5085,7 +5186,7 @@ components: - id - name - workspace - - intent + - spec - revision - created_at - updated_at @@ -5120,30 +5221,18 @@ components: type: object TaskInput: properties: - intent: - type: string - title: Intent - description: Human-readable description of the desired agent behavior. - inputs: - allOf: - - $ref: '#/components/schemas/TaskInputs' - description: The task's recognized input fields. - metrics: - items: - anyOf: - - $ref: '#/components/schemas/MetricInline' - - $ref: '#/components/schemas/MetricRef' - type: array - title: Metrics - description: "Metrics that score this task \u2014 inline bundles and/or\ - \ stored-metric refs." - views: - additionalProperties: - $ref: '#/components/schemas/SemanticView' - type: object - title: Views - description: Optional reporting views mapping metric outputs into named - semantic scores. + spec: + oneOf: + - $ref: '#/components/schemas/EvaluatorTaskDefinition' + - $ref: '#/components/schemas/HarborTaskDefinition' + title: Spec + description: The task's content, discriminated by which runner executes + it. + discriminator: + propertyName: kind + mapping: + evaluator: '#/components/schemas/EvaluatorTaskDefinition' + harbor: '#/components/schemas/HarborTaskDefinition' metadata: items: $ref: '#/components/schemas/MetadataItem' @@ -5160,7 +5249,7 @@ components: additionalProperties: false type: object required: - - intent + - spec title: TaskInput description: "Create/replace body for a stored task (the name comes from the\ \ path).\n\nThe authorable subset of :class:`Task` \u2014 the SDK ``AgentEvalTask``\ diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/fields.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/fields.py new file mode 100644 index 0000000000..2e682c9baa --- /dev/null +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/fields.py @@ -0,0 +1,297 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Shared field types for the evaluator API: entity references and metric payloads. + +Split out of :mod:`nemo_evaluator.api.schemas` so the per-kind task definitions can use these +without importing the module that composes them into DTOs — the definitions are imported *by* +``schemas``, so they cannot import from it. + +What counts as a ``workspace/name`` reference is **not** decided here: the shape +(:data:`~nemo_platform_plugin.refs.ENTITY_REF_PATTERN`) and the parser +(:func:`~nemo_platform_plugin.refs.parse_entity_ref`) are the platform's, shared with every other +plugin. This module only adds what is specific to a *revisioned* evaluator entity — the ``#fragment`` +that selects a revision. + +Everything here is re-exported from ``schemas`` for callers that already import it from there. +""" + +from __future__ import annotations + +import re +from typing import Annotated, Any, Literal, TypeAlias + +from nemo_evaluator.content_hash import DIGEST_PATTERN +from nemo_evaluator.shared.metric_bundles.bundles import ( + BundledMetricOutputSpec, + MetricMetadata, +) +from nemo_evaluator_sdk.values.common import SecretRef +from nemo_platform_plugin.refs import ENTITY_REF_PATTERN, parse_entity_ref +from pydantic import AfterValidator, BaseModel, ConfigDict, Field, RootModel, field_validator + + +class CloudpickleMetricPayload(BaseModel): + """Wire schema for a cloudpickle-serialized metric payload. + + Mirrors the runtime ``CloudpickleMetricPayload`` so the API contract is + explicit in the OpenAPI spec. The runtime bundle model serializes payloads + polymorphically (typed as an abstract base), which renders as an opaque + object in the spec; this concrete DTO documents the actual fields. + """ + + model_config = ConfigDict(extra="forbid", ser_json_bytes="base64", val_json_bytes="base64") + + kind: Literal["cloudpickle"] = Field(description="Payload format discriminator.") + python_version: str = Field(description="Python version the metric was pickled with (must match at execution).") + cloudpickle_version: str = Field(description="cloudpickle version used to serialize the metric.") + pickle_protocol: int = Field(description="Pickle protocol used.") + blob: bytes = Field(description="Base64-encoded cloudpickled metric object.") + digest: str | None = Field( + default=None, + description="SHA-256 digest of the payload bytes. Informational; recomputed server-side.", + ) + + +class InlineMetricPayload(BaseModel): + """Wire schema for an inline (config-serialized) metric payload. + + Mirrors the runtime ``InlineMetricPayload``. The metric is stored as its own + JSON configuration and reconstructed from the metric type union at execution, + so no code is shipped or executed on load. Used for platform-recognized + built-in metric types. + """ + + model_config = ConfigDict(extra="forbid") + + kind: Literal["inline"] = Field(description="Payload format discriminator.") + metric: dict[str, Any] = Field( + description="JSON-serialized built-in metric configuration, discriminated by its own `type`." + ) + digest: str | None = Field( + default=None, + description="SHA-256 digest of the canonical metric JSON. Informational; recomputed server-side.", + ) + + @field_validator("metric") + @classmethod + def _metric_must_declare_type(cls, value: dict[str, Any]) -> dict[str, Any]: + """Reject payloads without a metric ``type`` discriminator at the API boundary. + + The metric body stays an open object (the concrete shape is validated when + the bundle is hydrated against the metric type union), but a non-empty + ``type`` is required so malformed payloads fail fast rather than at execution. + """ + metric_type = value.get("type") + if not isinstance(metric_type, str) or not metric_type: + raise ValueError("inline metric payload must include a non-empty 'type'") + return value + + +# Discriminated on ``kind`` so additional payload formats can join the union +# without changing the field type. +MetricPayload = Annotated[CloudpickleMetricPayload | InlineMetricPayload, Field(discriminator="kind")] + + +class MetricInline(BaseModel): + """An executable metric submitted to the platform. + + Carries the bundled metric — type, metadata, output contracts, secret + references, and a format-specific payload — used both as the create-request + body and as an inline metric in an evaluation job. + """ + + model_config = ConfigDict(extra="forbid") + + bundle_kind: Literal["metric-bundle"] = "metric-bundle" + bundle_format_version: Literal["v1"] = "v1" + metric_type: str = Field(min_length=1, description="Runtime metric type name.") + metadata: MetricMetadata = Field(default_factory=MetricMetadata, description="User-facing metric metadata.") + outputs: list[BundledMetricOutputSpec] = Field(min_length=1, description="The metric's output contracts.") + secrets: dict[str, SecretRef] = Field( + default_factory=dict, description="Secret references required to execute the metric." + ) + payload: MetricPayload = Field(description="Format-specific serialized metric.") + + +#: The charset a ``#fragment`` may use. Exported because anything that *mints* a fragment — notably +#: revision tag names — has to be constrained by it: a value outside this set can be stored happily +#: and then never appear in a reference, which is a silent dead end rather than an error. +REF_FRAGMENT_CHARSET = r"[\w\-.]+" + +# A *sub-entity* reference adds an optional ``#fragment`` to the platform's ``ENTITY_REF_PATTERN``, +# which is the standard way of addressing something contained within an entity (filesets address a +# contained file the same way: ``workspace/fileset#path``). For a revisioned entity the fragment +# selects a revision — either a tag (``#latest``, ``#candidate``) or a full 64-char content digest. +# +# Deliberately a sibling of ``ENTITY_REF_PATTERN`` rather than a widening of it: that constant is +# still shared by ``MetricRef``, which has no revisions, and admitting a fragment there would accept +# input nothing is built to resolve. ``TaskRef`` and ``TasksetRef`` both use this pattern, since both +# name revisioned records; ``MetricRef`` joins them when (if) metrics gain revisions. +# +# The base alternation is spliced in from the shared constant (minus its anchors) so the two shapes +# cannot drift: widening what counts as a ``workspace/name`` widens both at once. +_SUBENTITY_REF_PATTERN = rf"^{ENTITY_REF_PATTERN.removeprefix('^').removesuffix('$')}(#{REF_FRAGMENT_CHARSET})?$" +#: The fragment separator for sub-entity references. Matches the fileset/job ref convention. +REF_FRAGMENT_SEPARATOR = "#" +#: The tag applied to every publish and used when a ref carries no fragment. +LATEST_TAG = "latest" + + +def parse_subentity_ref(root: str, default_workspace: str) -> tuple[str, str, str]: + """Split a reference into ``(workspace, name, fragment)``. + + The ``workspace/name`` split is delegated to the platform's :func:`~nemo_platform_plugin.refs. + parse_entity_ref`; this only adds the revision fragment on top, so evaluator refs and every other + plugin's refs agree on what a ``workspace/name`` is. Callers that don't care about revisions + discard the third element — that, rather than a second parser, is how a pinned ref is read + unpinned. + + An absent fragment resolves to :data:`LATEST_TAG` — a bare ``workspace/name`` means "the current + revision", never "unpinned". The fragment is returned verbatim: it may be a tag or a content + digest, and telling them apart is resolution's job, not parsing's. + """ + base, separator, fragment = root.partition(REF_FRAGMENT_SEPARATOR) + parsed = parse_entity_ref(base, default_workspace) + return parsed.workspace, parsed.name, fragment if separator and fragment else LATEST_TAG + + +class MetricRef(RootModel[str]): + """Reference to a persisted metric (format: ``workspace/name`` or ``name``).""" + + root: str = Field( + pattern=ENTITY_REF_PATTERN, + description="Reference to a stored metric (format: workspace/metric-name, or metric-name in the job workspace).", + ) + + +#: A wire metric is either an inline bundle DTO or a reference to a stored metric. Lives here (next to +#: ``MetricInline``) rather than in ``metric_refs`` so entity/DTO modules can use it without importing +#: the ref-resolution logic (which depends on ``entities`` and would cycle); ``metric_refs`` re-exports. +MetricRefOrInline: TypeAlias = MetricInline | MetricRef + + +class TaskRef(RootModel[str]): + """Reference to a persisted task (format: ``workspace/name``, ``name``, or either with a + ``#revision`` fragment). + + A taskset points at its member tasks by reference (there are no inline tasks), so a stored + taskset only ever holds refs. Unlike :class:`MetricRef`, a task ref may address a specific + revision via the platform's standard ``#`` sub-entity fragment. + + The fragment is optional *on input* and means :data:`LATEST_TAG` when absent — a bare + ``workspace/name`` is "the current revision", not "unpinned". It may name a tag or a content + digest. Anything **persisted** as a published snapshot must carry a resolved digest: tags move, + and a stored tag fragment would silently re-point published membership. + """ + + root: str = Field( + pattern=_SUBENTITY_REF_PATTERN, + description="Reference to a stored task (format: workspace/task-name, or task-name in the " + "taskset workspace), optionally pinned to a revision with '#'.", + ) + + +class TasksetRef(RootModel[str]): + """Reference to a persisted taskset (format: ``workspace/name`` or ``name``, optionally ``#rev``). + + Same shape and charset as :class:`TaskRef`. Lets an evaluation reference a stored taskset in place + of an inline task list; the taskset's member tasks are loaded and expanded during spec resolution. + + An optional ``#`` fragment pins the taskset revision to expand — a tag or a full content digest, + with an absent fragment meaning ``latest``. + + What each form guarantees, precisely. A taskset revision pins its members by digest, so a member + task publishing new content never changes what *any* ref expands to. A **bare** ref still tracks + the taskset's own revisions, and republishing the taskset re-resolves its members on write — so a + ``replace`` can change both which members are named and the content they resolve to, even if the + submitted member names were identical. A **pinned** ref is fixed against that too, and is what an + evaluation needs to stay comparable across a ``replace``. + """ + + root: str = Field( + pattern=_SUBENTITY_REF_PATTERN, + description="Reference to a stored taskset (format: workspace/taskset-name, or taskset-name in the " + "job workspace), optionally pinned to a revision with '#'.", + ) + + +class TaskInputs(BaseModel): + """A task's recognized input fields. + + ``extra="forbid"``: only the field below is accepted. ``instruction`` is the agent's prompt; the + runtime falls back to the task ``intent`` when it is unset. + """ + + model_config = ConfigDict(extra="forbid") + + instruction: str | None = Field( + default=None, description="The agent's instruction (its prompt). Falls back to the task `intent` when unset." + ) + + +class MetadataItem(BaseModel): + """A single key/value annotation on a task.""" + + model_config = ConfigDict(extra="forbid") + + key: str = Field(description="Annotation key.") + value: str = Field(description="Annotation value.") + + +def _reject_duplicate_metadata_keys(items: list[MetadataItem]) -> list[MetadataItem]: + """Metadata is a key→value map expressed as a list; duplicate keys would silently collapse (e.g. + when folded into a mapping for the runtime), so reject them at validation rather than lose data.""" + seen: set[str] = set() + for item in items: + if item.key in seen: + raise ValueError(f"duplicate metadata key: {item.key!r}") + seen.add(item.key) + return items + + +#: A task's metadata: key/value annotations with unique keys (duplicates rejected at validation). +TaskMetadataList: TypeAlias = Annotated[list[MetadataItem], AfterValidator(_reject_duplicate_metadata_keys)] + + +def _reject_duplicate_task_refs(refs: list[TaskRef]) -> list[TaskRef]: + """A taskset's members are an unordered set expressed as a list; a repeated ref is ambiguous + (it can't mean anything more than membership), so reject duplicates at validation.""" + seen: set[str] = set() + for ref in refs: + if ref.root in seen: + raise ValueError(f"duplicate task reference: {ref.root!r}") + seen.add(ref.root) + return refs + + +#: A list of task references with set semantics (order not significant, duplicates rejected). +TaskRefList: TypeAlias = Annotated[list[TaskRef], AfterValidator(_reject_duplicate_task_refs)] +#: Shape of a content digest in a ref fragment: full-length lowercase hex, never truncated. +_DIGEST_FRAGMENT_PATTERN = re.compile(DIGEST_PATTERN) + + +def _require_pinned_task_refs(refs: list[TaskRef]) -> list[TaskRef]: + """Every member of a *published* taskset revision must name an exact content digest. + + Enforced on the field rather than in the publish path so it cannot be bypassed by any other + writer. A ref that is bare (``workspace/name``) or tag-pinned (``#latest``, ``#candidate``) + resolves through a mutable pointer: the moment that tag moves, the published revision's + membership silently changes under it, and a "reproducible" dataset stops being reproducible. + Tags are resolution *inputs*, resolved to digests at publish time; only digests persist. + """ + for ref in refs: + _, _, fragment = parse_subentity_ref(ref.root, "") + if not _DIGEST_FRAGMENT_PATTERN.match(fragment): + raise ValueError( + f"task reference {ref.root!r} is not pinned to a content digest: a published taskset " + f"revision must reference an exact revision (got fragment {fragment!r}). Tags move; " + "resolve them to a digest before persisting." + ) + return refs + + +#: Member refs of a published taskset revision: set semantics *and* every ref digest-pinned. +PinnedTaskRefList: TypeAlias = Annotated[ + list[TaskRef], AfterValidator(_reject_duplicate_task_refs), AfterValidator(_require_pinned_task_refs) +] diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py index 22c894bf77..b6b3c5017f 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py @@ -5,23 +5,79 @@ from __future__ import annotations -import re from datetime import datetime from enum import StrEnum -from typing import Annotated, Any, Literal, TypeAlias +from typing import Annotated, TypeAlias -from nemo_evaluator.content_hash import DIGEST_PATTERN +from nemo_evaluator.api.fields import ( + LATEST_TAG as LATEST_TAG, +) +from nemo_evaluator.api.fields import ( + REF_FRAGMENT_CHARSET as REF_FRAGMENT_CHARSET, +) +from nemo_evaluator.api.fields import ( + REF_FRAGMENT_SEPARATOR as REF_FRAGMENT_SEPARATOR, +) +from nemo_evaluator.api.fields import ( + CloudpickleMetricPayload as CloudpickleMetricPayload, +) +from nemo_evaluator.api.fields import ( + InlineMetricPayload as InlineMetricPayload, +) +from nemo_evaluator.api.fields import ( + MetadataItem as MetadataItem, +) +from nemo_evaluator.api.fields import ( + MetricInline as MetricInline, +) +from nemo_evaluator.api.fields import ( + MetricPayload as MetricPayload, +) +from nemo_evaluator.api.fields import ( + MetricRef as MetricRef, +) +from nemo_evaluator.api.fields import ( + MetricRefOrInline as MetricRefOrInline, +) +from nemo_evaluator.api.fields import ( + PinnedTaskRefList as PinnedTaskRefList, +) +from nemo_evaluator.api.fields import ( + TaskInputs as TaskInputs, +) +from nemo_evaluator.api.fields import ( + TaskMetadataList as TaskMetadataList, +) +from nemo_evaluator.api.fields import ( + TaskRef as TaskRef, +) +from nemo_evaluator.api.fields import ( + TaskRefList as TaskRefList, +) +from nemo_evaluator.api.fields import ( + TasksetRef as TasksetRef, +) +from nemo_evaluator.api.fields import ( + parse_subentity_ref as parse_subentity_ref, +) +from nemo_evaluator.api.task_definitions.evaluator import EvaluatorTaskDefinition as EvaluatorTaskDefinition +from nemo_evaluator.api.task_definitions.harbor import HarborTaskDefinition as HarborTaskDefinition from nemo_evaluator.shared.metric_bundles.bundles import ( BundledMetricOutputSpec, - MetricMetadata, ) -from nemo_evaluator_sdk.agent_eval.tasks import SemanticView from nemo_evaluator_sdk.values.common import SecretRef from nemo_evaluator_sdk.values.results import AggregatedMetricResult from nemo_platform_plugin.api.filter import ComparisonOperation, FilterOperation, LogicalOperation from nemo_platform_plugin.api.parsed_filter import ENTITY_BASE_FIELDS +from nemo_platform_plugin.refs import ( + FILESET_REF_PATTERN as FILESET_REF_PATTERN, +) from nemo_platform_plugin.schema import DatetimeFilter, Filter -from pydantic import AfterValidator, BaseModel, ConfigDict, Field, RootModel, field_validator +from pydantic import BaseModel, ConfigDict, Field + +#: A stored task's content, discriminated by which runner executes it. Widen with more members as +#: runners land — the same way ``AgentRunnerTarget`` does on the target side. +TaskDefinition: TypeAlias = Annotated[EvaluatorTaskDefinition | HarborTaskDefinition, Field(discriminator="kind")] class DataFilter(Filter): @@ -52,206 +108,6 @@ def _walk(op: FilterOperation) -> FilterOperation: return _walk(operation) -class CloudpickleMetricPayload(BaseModel): - """Wire schema for a cloudpickle-serialized metric payload. - - Mirrors the runtime ``CloudpickleMetricPayload`` so the API contract is - explicit in the OpenAPI spec. The runtime bundle model serializes payloads - polymorphically (typed as an abstract base), which renders as an opaque - object in the spec; this concrete DTO documents the actual fields. - """ - - model_config = ConfigDict(extra="forbid", ser_json_bytes="base64", val_json_bytes="base64") - - kind: Literal["cloudpickle"] = Field(description="Payload format discriminator.") - python_version: str = Field(description="Python version the metric was pickled with (must match at execution).") - cloudpickle_version: str = Field(description="cloudpickle version used to serialize the metric.") - pickle_protocol: int = Field(description="Pickle protocol used.") - blob: bytes = Field(description="Base64-encoded cloudpickled metric object.") - digest: str | None = Field( - default=None, - description="SHA-256 digest of the payload bytes. Informational; recomputed server-side.", - ) - - -class InlineMetricPayload(BaseModel): - """Wire schema for an inline (config-serialized) metric payload. - - Mirrors the runtime ``InlineMetricPayload``. The metric is stored as its own - JSON configuration and reconstructed from the metric type union at execution, - so no code is shipped or executed on load. Used for platform-recognized - built-in metric types. - """ - - model_config = ConfigDict(extra="forbid") - - kind: Literal["inline"] = Field(description="Payload format discriminator.") - metric: dict[str, Any] = Field( - description="JSON-serialized built-in metric configuration, discriminated by its own `type`." - ) - digest: str | None = Field( - default=None, - description="SHA-256 digest of the canonical metric JSON. Informational; recomputed server-side.", - ) - - @field_validator("metric") - @classmethod - def _metric_must_declare_type(cls, value: dict[str, Any]) -> dict[str, Any]: - """Reject payloads without a metric ``type`` discriminator at the API boundary. - - The metric body stays an open object (the concrete shape is validated when - the bundle is hydrated against the metric type union), but a non-empty - ``type`` is required so malformed payloads fail fast rather than at execution. - """ - metric_type = value.get("type") - if not isinstance(metric_type, str) or not metric_type: - raise ValueError("inline metric payload must include a non-empty 'type'") - return value - - -# Discriminated on ``kind`` so additional payload formats can join the union -# without changing the field type. -MetricPayload = Annotated[CloudpickleMetricPayload | InlineMetricPayload, Field(discriminator="kind")] - - -class MetricInline(BaseModel): - """An executable metric submitted to the platform. - - Carries the bundled metric — type, metadata, output contracts, secret - references, and a format-specific payload — used both as the create-request - body and as an inline metric in an evaluation job. - """ - - model_config = ConfigDict(extra="forbid") - - bundle_kind: Literal["metric-bundle"] = "metric-bundle" - bundle_format_version: Literal["v1"] = "v1" - metric_type: str = Field(min_length=1, description="Runtime metric type name.") - metadata: MetricMetadata = Field(default_factory=MetricMetadata, description="User-facing metric metadata.") - outputs: list[BundledMetricOutputSpec] = Field(min_length=1, description="The metric's output contracts.") - secrets: dict[str, SecretRef] = Field( - default_factory=dict, description="Secret references required to execute the metric." - ) - payload: MetricPayload = Field(description="Format-specific serialized metric.") - - -# An entity reference is ``name`` or ``workspace/name``, each segment using the platform name charset. -# Shared by every ``workspace/name`` reference type (metrics, tasks). Enforced on the field so -# empty/malformed refs are rejected at validation rather than during parsing. -_ENTITY_REF_PATTERN = r"^[\w\-.]+(/[\w\-.]+)?$" - -#: The charset a ``#fragment`` may use. Exported because anything that *mints* a fragment — notably -#: revision tag names — has to be constrained by it: a value outside this set can be stored happily -#: and then never appear in a reference, which is a silent dead end rather than an error. -REF_FRAGMENT_CHARSET = r"[\w\-.]+" - -# A *sub-entity* reference adds an optional ``#fragment``, the platform's standard way of addressing -# something contained within an entity (filesets address a contained file the same way: -# ``workspace/fileset#path``). For a revisioned entity the fragment selects a revision — either a tag -# (``#latest``, ``#candidate``) or a full 64-char content digest. -# -# Deliberately a sibling of ``_ENTITY_REF_PATTERN`` rather than a widening of it: that constant is -# still shared by ``MetricRef``, which has no revisions, and admitting a fragment there would accept -# input nothing is built to resolve. ``TaskRef`` and ``TasksetRef`` both use this pattern, since both -# name revisioned records; ``MetricRef`` joins them when (if) metrics gain revisions. -_SUBENTITY_REF_PATTERN = rf"^[\w\-.]+(/[\w\-.]+)?(#{REF_FRAGMENT_CHARSET})?$" - -#: The fragment separator for sub-entity references. Matches the fileset/job ref convention. -REF_FRAGMENT_SEPARATOR = "#" - -#: The tag applied to every publish and used when a ref carries no fragment. -LATEST_TAG = "latest" - - -def parse_entity_ref(root: str, default_workspace: str) -> tuple[str, str]: - """Split a validated ``workspace/name`` (or bare ``name``) reference into ``(workspace, name)``. - - The ``workspace/name`` vs bare-``name`` shape is guaranteed by the field's ``_ENTITY_REF_PATTERN``, - so this only needs to split. Shared by every reference type (metrics, tasks); lives here — next to - the pattern, with no entity dependency — so ref-owning modules can reuse it without cycling. - - Any ``#fragment`` is stripped before splitting, so callers that don't care about revisions keep - working unchanged against a pinned ref. Use :func:`parse_subentity_ref` to read the fragment. - """ - base, _, _ = root.partition(REF_FRAGMENT_SEPARATOR) - workspace, separator, name = base.partition("/") - if separator: - return workspace, name - return default_workspace, base - - -def parse_subentity_ref(root: str, default_workspace: str) -> tuple[str, str, str]: - """Split a reference into ``(workspace, name, fragment)``. - - An absent fragment resolves to :data:`LATEST_TAG` — a bare ``workspace/name`` means "the current - revision", never "unpinned". The fragment is returned verbatim: it may be a tag or a content - digest, and telling them apart is resolution's job, not parsing's. - """ - base, separator, fragment = root.partition(REF_FRAGMENT_SEPARATOR) - workspace, name = parse_entity_ref(base, default_workspace) - return workspace, name, fragment if separator and fragment else LATEST_TAG - - -class MetricRef(RootModel[str]): - """Reference to a persisted metric (format: ``workspace/name`` or ``name``).""" - - root: str = Field( - pattern=_ENTITY_REF_PATTERN, - description="Reference to a stored metric (format: workspace/metric-name, or metric-name in the job workspace).", - ) - - -#: A wire metric is either an inline bundle DTO or a reference to a stored metric. Lives here (next to -#: ``MetricInline``) rather than in ``metric_refs`` so entity/DTO modules can use it without importing -#: the ref-resolution logic (which depends on ``entities`` and would cycle); ``metric_refs`` re-exports. -MetricRefOrInline: TypeAlias = MetricInline | MetricRef - - -class TaskRef(RootModel[str]): - """Reference to a persisted task (format: ``workspace/name``, ``name``, or either with a - ``#revision`` fragment). - - A taskset points at its member tasks by reference (there are no inline tasks), so a stored - taskset only ever holds refs. Unlike :class:`MetricRef`, a task ref may address a specific - revision via the platform's standard ``#`` sub-entity fragment. - - The fragment is optional *on input* and means :data:`LATEST_TAG` when absent — a bare - ``workspace/name`` is "the current revision", not "unpinned". It may name a tag or a content - digest. Anything **persisted** as a published snapshot must carry a resolved digest: tags move, - and a stored tag fragment would silently re-point published membership. - """ - - root: str = Field( - pattern=_SUBENTITY_REF_PATTERN, - description="Reference to a stored task (format: workspace/task-name, or task-name in the " - "taskset workspace), optionally pinned to a revision with '#'.", - ) - - -class TasksetRef(RootModel[str]): - """Reference to a persisted taskset (format: ``workspace/name`` or ``name``, optionally ``#rev``). - - Same shape and charset as :class:`TaskRef`. Lets an evaluation reference a stored taskset in place - of an inline task list; the taskset's member tasks are loaded and expanded during spec resolution. - - An optional ``#`` fragment pins the taskset revision to expand — a tag or a full content digest, - with an absent fragment meaning ``latest``. - - What each form guarantees, precisely. A taskset revision pins its members by digest, so a member - task publishing new content never changes what *any* ref expands to. A **bare** ref still tracks - the taskset's own revisions, and republishing the taskset re-resolves its members on write — so a - ``replace`` can change both which members are named and the content they resolve to, even if the - submitted member names were identical. A **pinned** ref is fixed against that too, and is what an - evaluation needs to stay comparable across a ``replace``. - """ - - root: str = Field( - pattern=_SUBENTITY_REF_PATTERN, - description="Reference to a stored taskset (format: workspace/taskset-name, or taskset-name in the " - "job workspace), optionally pinned to a revision with '#'.", - ) - - class Metric(BaseModel): """API representation of a stored metric. @@ -346,44 +202,6 @@ class EvaluateResult(_ResultBase): metric_types: list[str] = Field(description="Runtime metric type names applied in the run.") -class TaskInputs(BaseModel): - """A task's recognized input fields. - - ``extra="forbid"``: only the field below is accepted. ``instruction`` is the agent's prompt; the - runtime falls back to the task ``intent`` when it is unset. - """ - - model_config = ConfigDict(extra="forbid") - - instruction: str | None = Field( - default=None, description="The agent's instruction (its prompt). Falls back to the task `intent` when unset." - ) - - -class MetadataItem(BaseModel): - """A single key/value annotation on a task.""" - - model_config = ConfigDict(extra="forbid") - - key: str = Field(description="Annotation key.") - value: str = Field(description="Annotation value.") - - -def _reject_duplicate_metadata_keys(items: list[MetadataItem]) -> list[MetadataItem]: - """Metadata is a key→value map expressed as a list; duplicate keys would silently collapse (e.g. - when folded into a mapping for the runtime), so reject them at validation rather than lose data.""" - seen: set[str] = set() - for item in items: - if item.key in seen: - raise ValueError(f"duplicate metadata key: {item.key!r}") - seen.add(item.key) - return items - - -#: A task's metadata: key/value annotations with unique keys (duplicates rejected at validation). -TaskMetadataList: TypeAlias = Annotated[list[MetadataItem], AfterValidator(_reject_duplicate_metadata_keys)] - - class Task(BaseModel): """API representation of a stored agent-eval task. @@ -396,16 +214,7 @@ class Task(BaseModel): name: str = Field(description="Task name — the stable task id, unique within its workspace.") workspace: str = Field(description="Workspace the task belongs to.") project: str | None = Field(default=None, description="The project associated with this task.") - intent: str = Field(description="Human-readable description of the desired agent behavior.") - inputs: TaskInputs = Field(default_factory=TaskInputs, description="The task's recognized input fields.") - metrics: list[MetricRef] = Field( - default_factory=list, - description="References to the metrics that score this task; inline metrics submitted on create " - "are normalized to (derived) stored metrics, so a stored task holds refs only.", - ) - views: dict[str, SemanticView] = Field( - default_factory=dict, description="Optional reporting views mapping metric outputs into named semantic scores." - ) + spec: TaskDefinition = Field(description="The task's content, discriminated by which runner executes it.") metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the task.") revision: int = Field( description="Ordinal of the published revision this content corresponds to. Every stored task " @@ -430,14 +239,7 @@ class TaskInput(BaseModel): model_config = ConfigDict(extra="forbid") - intent: str = Field(description="Human-readable description of the desired agent behavior.") - inputs: TaskInputs = Field(default_factory=TaskInputs, description="The task's recognized input fields.") - metrics: list[MetricRefOrInline] = Field( - default_factory=list, description="Metrics that score this task — inline bundles and/or stored-metric refs." - ) - views: dict[str, SemanticView] = Field( - default_factory=dict, description="Optional reporting views mapping metric outputs into named semantic scores." - ) + spec: TaskDefinition = Field(description="The task's content, discriminated by which runner executes it.") metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the task.") tags: list[str] = Field( default_factory=list, @@ -484,50 +286,6 @@ class TaskFilter(Filter): updated_at: DatetimeFilter | None = Field(None, description="Filter by update date.") -def _reject_duplicate_task_refs(refs: list[TaskRef]) -> list[TaskRef]: - """A taskset's members are an unordered set expressed as a list; a repeated ref is ambiguous - (it can't mean anything more than membership), so reject duplicates at validation.""" - seen: set[str] = set() - for ref in refs: - if ref.root in seen: - raise ValueError(f"duplicate task reference: {ref.root!r}") - seen.add(ref.root) - return refs - - -#: A list of task references with set semantics (order not significant, duplicates rejected). -TaskRefList: TypeAlias = Annotated[list[TaskRef], AfterValidator(_reject_duplicate_task_refs)] - -#: Shape of a content digest in a ref fragment: full-length lowercase hex, never truncated. -_DIGEST_FRAGMENT_PATTERN = re.compile(DIGEST_PATTERN) - - -def _require_pinned_task_refs(refs: list[TaskRef]) -> list[TaskRef]: - """Every member of a *published* taskset revision must name an exact content digest. - - Enforced on the field rather than in the publish path so it cannot be bypassed by any other - writer. A ref that is bare (``workspace/name``) or tag-pinned (``#latest``, ``#candidate``) - resolves through a mutable pointer: the moment that tag moves, the published revision's - membership silently changes under it, and a "reproducible" dataset stops being reproducible. - Tags are resolution *inputs*, resolved to digests at publish time; only digests persist. - """ - for ref in refs: - _, _, fragment = parse_subentity_ref(ref.root, "") - if not _DIGEST_FRAGMENT_PATTERN.match(fragment): - raise ValueError( - f"task reference {ref.root!r} is not pinned to a content digest: a published taskset " - f"revision must reference an exact revision (got fragment {fragment!r}). Tags move; " - "resolve them to a digest before persisting." - ) - return refs - - -#: Member refs of a published taskset revision: set semantics *and* every ref digest-pinned. -PinnedTaskRefList: TypeAlias = Annotated[ - list[TaskRef], AfterValidator(_reject_duplicate_task_refs), AfterValidator(_require_pinned_task_refs) -] - - class Taskset(BaseModel): """API representation of a stored taskset — a flexible grouping of tasks with metadata. diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py index 171bda0fae..2b44f76778 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py @@ -17,14 +17,16 @@ from nemo_evaluator.api.schemas import ( LATEST_TAG, + HarborTaskDefinition, MetricInline, MetricRef, Revision, Task, + TaskDefinition, TaskInput, - parse_entity_ref, ) from nemo_evaluator.entities import TaskEntity, TaskRevisionEntity +from nemo_evaluator.metric_refs import parse_metric_ref from nemo_evaluator.revisions import ( apply_tag, get_revision, @@ -68,10 +70,7 @@ def _entity_to_task(entity: TaskEntity) -> Task: name=entity.name, workspace=entity.workspace, project=entity.project, - intent=entity.intent, - inputs=entity.inputs, - metrics=entity.metrics, - views=entity.views, + spec=entity.spec, metadata=entity.metadata, revision=entity.latest_revision, tags=entity.tags, @@ -95,10 +94,7 @@ def _revision_to_task(head: TaskEntity, revision: TaskRevisionEntity) -> Task: name=head.name, workspace=head.workspace, project=head.project, - intent=revision.intent, - inputs=revision.inputs, - metrics=revision.metrics, - views=revision.views, + spec=revision.spec, metadata=revision.metadata, revision=revision.revision, tags={tag: ordinal for tag, ordinal in head.tags.items() if ordinal == revision.revision}, @@ -158,7 +154,7 @@ async def _normalize_metrics(self, metrics: list[MetricRef | MetricInline], *, w refs: list[MetricRef] = [] for metric in metrics: if isinstance(metric, MetricRef): - ref_workspace, name = parse_entity_ref(metric.root, workspace) + ref_workspace, name = parse_metric_ref(metric.root, workspace) if await self.metric_service.get_metric(ref_workspace, name) is None: raise MetricRefNotFoundError( f"Metric reference '{metric.root}' not found. " @@ -170,12 +166,21 @@ async def _normalize_metrics(self, metrics: list[MetricRef | MetricInline], *, w refs.append(await self.metric_service.store_derived_metric(metric, workspace=workspace)) return refs + async def _normalize_spec(self, spec: TaskDefinition, *, workspace: str) -> TaskDefinition: + """Narrow a submitted spec to its stored form. + + Only the agent-eval variant changes: its inline metrics are offloaded to derived stored + metrics so a persisted task holds references only. A Harbor spec is already in stored form — + its archive was uploaded before the task was submitted. + """ + if isinstance(spec, HarborTaskDefinition): + return spec + # Same model in and out — only ``metrics`` narrows, from possibly-inline to references. + return spec.model_copy(update={"metrics": await self._normalize_metrics(spec.metrics, workspace=workspace)}) + async def _apply_content(self, entity: TaskEntity, task_input: TaskInput, *, workspace: str) -> TaskEntity: """Overwrite a head record's content from a request body (leaving revision pointers alone).""" - entity.intent = task_input.intent - entity.inputs = task_input.inputs - entity.metrics = await self._normalize_metrics(task_input.metrics, workspace=workspace) - entity.views = task_input.views + entity.spec = await self._normalize_spec(task_input.spec, workspace=workspace) entity.metadata = task_input.metadata return entity @@ -188,10 +193,13 @@ async def create_task( where ``published`` is always ``True`` here — a fresh task always cuts a revision. Use :meth:`replace_task` to publish a further revision of an existing task. """ - entity = await self._apply_content( - TaskEntity(name=name, workspace=workspace, project=project, intent=task_input.intent), - task_input, + # Normalize once: ``_apply_content`` would offload the same inline metrics a second time. + entity = TaskEntity( + name=name, workspace=workspace, + project=project, + spec=await self._normalize_spec(task_input.spec, workspace=workspace), + metadata=task_input.metadata, ) try: created = await self.entity_client.create(entity) diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py index 359fea7a24..b6fe457ca1 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py @@ -26,7 +26,6 @@ TaskRef, Taskset, TasksetInput, - parse_entity_ref, parse_subentity_ref, ) from nemo_evaluator.entities import TasksetEntity, TasksetRevisionEntity @@ -184,10 +183,14 @@ def _reject_duplicate_members(self, tasks: list[TaskRef], *, workspace: str) -> The field validator only catches byte-identical refs; this catches refs that differ in form but resolve to the same ``(workspace, name)`` — e.g. ``task-a`` and ``default/task-a`` in the ``default`` workspace. + + The revision fragment is deliberately discarded: two refs naming the same task at different + revisions are still the same member, and a taskset holding both would expand that task twice. """ seen: set[tuple[str, str]] = set() for ref in tasks: - resolved = parse_entity_ref(ref.root, workspace) + ref_workspace, name, _ = parse_subentity_ref(ref.root, workspace) + resolved = (ref_workspace, name) if resolved in seen: raise DuplicateTaskRefError( f"Task reference '{ref.root}' resolves to '{resolved[0]}/{resolved[1]}', already in this taskset" diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/evaluator.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/evaluator.py new file mode 100644 index 0000000000..584231cc64 --- /dev/null +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/evaluator.py @@ -0,0 +1,49 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The built-in task kind: an agent scored by platform metrics.""" + +from __future__ import annotations + +from typing import Any, Literal + +from nemo_evaluator.api.fields import MetricRefOrInline, TaskInputs +from nemo_evaluator_sdk.agent_eval.tasks import SemanticView +from pydantic import BaseModel, ConfigDict, Field + + +class EvaluatorTaskDefinition(BaseModel): + """What the agent should do, and how the platform scores it. + + ``metrics`` accepts inline bundles on the way in and holds references once stored: the service + offloads an inline metric to a content-addressed *derived* metric on create, so a persisted task + only ever names metrics it does not own. That narrowing is a service invariant rather than a + type-level one — a single model keeps the API surface small, at the cost of this field being + wider than what a stored task actually contains. + + Every field here is covered by the revision digest, ``reference`` included: it decides what a + metric grades against, so two revisions that score differently must not share a digest. Pinning + a revision therefore fixes the grading, not just the prompt. + """ + + model_config = ConfigDict(extra="forbid") + + kind: Literal["evaluator"] = Field(description="Task kind discriminator.") + intent: str = Field(description="Human-readable description of the desired agent behavior.") + inputs: TaskInputs = Field(default_factory=TaskInputs, description="The task's recognized input fields.") + metrics: list[MetricRefOrInline] = Field( + default_factory=list, + description="Metrics that score this task — stored-metric references, and inline bundles on " + "create (normalized to derived stored metrics before the task is persisted).", + ) + reference: dict[str, Any] = Field( + default_factory=dict, + description="Grader-only ground truth (held-out tests, expected outputs, rubric data). Surfaced to " + "metrics but never seeded into the agent's workspace or shown to the agent, so a metric can grade " + "against artifacts the agent cannot influence. Held out from the *agent*, not from the API: anyone " + "who can read the task can read this.", + ) + views: dict[str, SemanticView] = Field( + default_factory=dict, + description="Optional reporting views mapping metric outputs into named semantic scores.", + ) diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/harbor.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/harbor.py new file mode 100644 index 0000000000..6bc71e7144 --- /dev/null +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/harbor.py @@ -0,0 +1,58 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The Harbor task kind: a packaged task directory, run and scored by Harbor.""" + +from __future__ import annotations + +from typing import Any, Literal + +from nemo_evaluator.content_hash import DIGEST_LENGTH, DIGEST_PATTERN +from nemo_platform_plugin.refs import FILESET_REF_PATTERN +from pydantic import BaseModel, ConfigDict, Field + + +class HarborTaskDefinition(BaseModel): + """A reference to the task's packaged files, plus a projection of Harbor's own config. + + Harbor identifies a task by a *directory* — ``task.toml``, an instruction, an environment — so + what is stored is a reference to that directory's archive in the Files service, not the files + themselves. One fileset per task, so a task shared by several tasksets is stored once. The + archive is materialized back into ``//`` at run time, which is the layout + Harbor's own discovery expects. + + Which agent runs the task is *not* stored here. That comes from the run's target + (``HarborRunnerTarget``), so the same stored task can be evaluated against different agents. + Harbor's own ``[agent]`` block — carried inside ``config`` — configures how the agent *phase* + runs (timeout, user, network policy), not which agent it is. + """ + + model_config = ConfigDict(extra="forbid") + + kind: Literal["harbor"] = Field(description="Task kind discriminator.") + archive_ref: str = Field( + pattern=FILESET_REF_PATTERN, + description="Files reference to the task's packaged directory (format: workspace/fileset#path).", + ) + archive_digest: str = Field( + description="Content hash Harbor computed over the task directory. This is the authoritative " + "identity of a Harbor task's content — every file, including task.toml.", + min_length=DIGEST_LENGTH, + max_length=DIGEST_LENGTH, + pattern=DIGEST_PATTERN, + ) + instruction: str | None = Field( + default=None, description="The task's instruction text, when it has one (multi-step tasks may not)." + ) + # Excluded from the revision digest (see ``_DERIVED_SPEC_FIELDS`` in ``entities``). Safe only + # because this is never an execution input: Harbor reads the real ``task.toml`` out of the + # materialized archive, and ``archive_digest`` already covers every file in that directory. + # Hashing the projection too would add no coverage, and would make revision history sensitive to + # Harbor's serialization — a release that reordered keys would cut a revision for byte-identical + # files. Anything here that becomes a genuine execution or grading input must be digested. + config: dict[str, Any] = Field( + default_factory=dict, + description="Harbor's own task configuration (verifier, agent, environment, steps), as published. " + "A queryable projection of task.toml — inspect a task's verifier without downloading the " + "archive. Opaque here: Harbor owns this schema.", + ) diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/content_hash.py b/plugins/nemo-evaluator/src/nemo_evaluator/content_hash.py index 78dd55e9b8..df245c0470 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/content_hash.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/content_hash.py @@ -35,7 +35,8 @@ import hashlib import json -from collections.abc import Set +from collections.abc import Mapping, Set +from typing import Any from nemo_platform_plugin.entities import EntityBase @@ -46,7 +47,16 @@ DIGEST_PATTERN = r"^[0-9a-f]{64}$" -def canonical_payload(entity: EntityBase, *, exclude: Set[str] | None = None) -> str: +def _as_exclude_map(exclude: Set[str] | Mapping[str, Any] | None) -> dict[str, Any]: + """Normalize either accepted ``exclude`` form to pydantic's dict form.""" + if exclude is None: + return {} + if isinstance(exclude, Mapping): + return {str(name): nested for name, nested in exclude.items()} + return {str(name): True for name in exclude} + + +def canonical_payload(entity: EntityBase, *, exclude: Set[str] | Mapping[str, Any] | None = None) -> str: """Return the canonical JSON serialization that :func:`content_hash` digests. Exposed separately because it is the actual compatibility contract: if this string changes @@ -78,12 +88,15 @@ def canonical_payload(entity: EntityBase, *, exclude: Set[str] | None = None) -> their own revision/tag bookkeeping here — a revision's digest must not cover the revision index that was assigned *because of* that digest. """ - excluded = set(entity.__base_fields__) | set(exclude or ()) + # Pydantic's dict form lets a caller exclude a *nested* field (``{"spec": {"config"}}``), which + # a flat set cannot express. Both forms are accepted so simple cases stay simple. + excluded: dict[str, Any] = {str(name): True for name in entity.__base_fields__} + excluded.update(_as_exclude_map(exclude)) payload = entity.model_dump(exclude=excluded, exclude_computed_fields=True, mode="json") return json.dumps(payload, sort_keys=True, separators=(",", ":")) -def content_hash(entity: EntityBase, *, exclude: Set[str] | None = None) -> str: +def content_hash(entity: EntityBase, *, exclude: Set[str] | Mapping[str, Any] | None = None) -> str: """Return the full 64-char lowercase hex SHA-256 digest of an entity's content. See :func:`canonical_payload` for what is and is not included. diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/entities.py b/plugins/nemo-evaluator/src/nemo_evaluator/entities.py index cd2404db77..a32c66ec1f 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/entities.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/entities.py @@ -20,15 +20,13 @@ from typing import ClassVar from nemo_evaluator.api.schemas import ( - MetricRef, PinnedTaskRefList, - TaskInputs, + TaskDefinition, TaskMetadataList, TaskRefList, ) from nemo_evaluator.content_hash import DIGEST_LENGTH, DIGEST_PATTERN from nemo_evaluator.shared.metric_bundles.bundles import BundledMetricOutputSpec -from nemo_evaluator_sdk.agent_eval.tasks import SemanticView from nemo_evaluator_sdk.values.common import SecretRef from nemo_evaluator_sdk.values.results import AggregatedMetricResult from nemo_platform_plugin.entities import EntityBase @@ -52,6 +50,39 @@ #: neither itself nor the ordinal that was assigned because of it. REVISION_SELF_FIELDS = frozenset({"content_hash", "revision"}) +#: Spec fields excluded from the revision digest. +#: +#: The rule for what belongs *in* the digest: any field that affects the output of a task's +#: execution or the mechanism used to grade it. A field may only be excluded if it is a derived +#: view of content the digest already covers by another route. +#: +#: ``HarborTaskDefinition.config`` qualifies. It is a projection of ``task.toml``, which lives +#: inside the archive, and Harbor reads the real ``task.toml`` out of the materialized archive at +#: run time — this copy is never an execution input, only a queryable convenience. ``archive_digest`` +#: is authoritative over every file in that directory including ``task.toml``, so a config change +#: that actually alters execution or grading already moves the digest. Hashing the projection too +#: would add no coverage and would make revision history sensitive to Harbor's serialization: a +#: release that reordered keys or emitted a new defaulted field would cut a revision for +#: byte-identical files. +#: +#: That makes ``archive_digest`` load-bearing. If a Harbor field ever becomes an execution input in +#: its own right — read from the stored record rather than from the archive — it must be digested. +_DERIVED_SPEC_FIELDS = {"config"} + +#: What a *head* record excludes when digesting: its revision pointers, plus derived spec fields. +#: Nested form, because the derived fields live inside ``spec``. +REVISION_POINTER_EXCLUDE: dict[str, object] = { + **dict.fromkeys(REVISION_POINTER_FIELDS, True), + "spec": set(_DERIVED_SPEC_FIELDS), +} + +#: The mirror for a *revision* record. Both must exclude the same derived fields, or the head and +#: its revision would digest differently and publish-time dedup would never fire. +REVISION_SELF_EXCLUDE: dict[str, object] = { + **dict.fromkeys(REVISION_SELF_FIELDS, True), + "spec": set(_DERIVED_SPEC_FIELDS), +} + class MetricBundleEntity(EntityBase): """Persisted index for a stored metric, addressed by workspace/name. @@ -209,27 +240,23 @@ class _RevisionedCommon(BaseModel): class TaskEntity(_RevisionedCommon, EntityBase): - """Persisted, queryable agent-eval task, addressed by workspace/name. - - Maps to the SDK :class:`~nemo_evaluator_sdk.agent_eval.tasks.AgentEvalTask`: the task's stable - ``id`` is the record ``name``, and ``metrics`` are stored in their wire form (inline bundles - and/or references to stored metrics) so a task can reference curated metrics or carry its own; - references resolve to inline runtime metrics when the task is run. + """Persisted, queryable task, addressed by workspace/name. + + A task is an evaluation unit; ``spec`` says what it is and which runner executes it. Both kinds + live in one record type so a user manages every evaluation unit in one place, and so a taskset + can group them without caring how each one runs — the same way ``AgentRunnerTarget`` already + treats codex/fabric/harbor as members of one union on the target side. + + Content is nested under ``spec`` rather than flattened with nullable per-kind fields, so each + variant's required fields stay genuinely required and the revision digest covers the spec as one + unit. An agent-eval task's ``metrics`` are stored as references (inline metrics submitted on + create are normalized to derived stored metrics); a Harbor task's files live in a fileset, and + the spec holds a reference to them. """ __entity_type__: ClassVar[str] = "task" - intent: str = Field(description="Human-readable description of the desired agent behavior.") - inputs: TaskInputs = Field(default_factory=TaskInputs, description="The task's recognized input fields.") - metrics: list[MetricRef] = Field( - default_factory=list, - description="References to the metrics that score this task. Inline metrics submitted with the " - "task are normalized to (derived) stored metrics, so a persisted task only ever holds refs.", - ) - views: dict[str, SemanticView] = Field( - default_factory=dict, - description="Optional reporting views mapping this task's metric outputs into named semantic scores.", - ) + spec: TaskDefinition = Field(description="The task's content, discriminated by which runner executes it.") metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the task.") @@ -320,16 +347,7 @@ class TaskRevisionEntity(_RevisionCommon, EntityBase): __entity_type__: ClassVar[str] = "task_revision" - intent: str = Field(description="Human-readable description of the desired agent behavior.") - inputs: TaskInputs = Field(default_factory=TaskInputs, description="The task's recognized input fields.") - metrics: list[MetricRef] = Field( - default_factory=list, - description="References to the metrics that score this task, as of this revision.", - ) - views: dict[str, SemanticView] = Field( - default_factory=dict, - description="Reporting views mapping this task's metric outputs into named semantic scores.", - ) + spec: TaskDefinition = Field(description="The task's content as of this revision.") metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the task.") diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/metric_refs.py b/plugins/nemo-evaluator/src/nemo_evaluator/metric_refs.py index e46f6901f7..b9d922b75c 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/metric_refs.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/metric_refs.py @@ -16,21 +16,25 @@ # entity/DTO modules can reference them without importing this module's entities-dependent resolution # logic (which would create an import cycle). Imported here for use below and re-exported for the # existing ``nemo_evaluator.metric_refs`` import sites. -from nemo_evaluator.api.schemas import MetricRef, MetricRefOrInline, parse_entity_ref +from nemo_evaluator.api.schemas import MetricRef, MetricRefOrInline from nemo_evaluator.entities import MetricBundleEntity from nemo_evaluator.metric_storage import load_bundle from nemo_evaluator.shared.metric_bundles.bundles import MetricBundle from nemo_platform import AsyncNeMoPlatform from nemo_platform_plugin.entity_client import NemoEntityGetterProtocol, NemoEntityNotFoundError +from nemo_platform_plugin.refs import parse_entity_ref def parse_metric_ref(root: str, default_workspace: str) -> tuple[str, str]: """Split a validated metric reference into ``(workspace, name)``. - Thin alias over the shared :func:`~nemo_evaluator.api.schemas.parse_entity_ref` (all - ``workspace/name`` refs split identically); kept for the existing ``metric_refs`` call sites. + Thin alias over the platform's :func:`~nemo_platform_plugin.refs.parse_entity_ref` (all + ``workspace/name`` refs split identically); kept for the existing ``metric_refs`` call sites, + which want a tuple. A metric ref carries no ``#fragment`` — metrics are not revisioned — so the + plain entity parser is the right one here. """ - return parse_entity_ref(root, default_workspace) + parsed = parse_entity_ref(root, default_workspace) + return parsed.workspace, parsed.name async def resolve_metric_ref( diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/revisions.py b/plugins/nemo-evaluator/src/nemo_evaluator/revisions.py index 5b02566018..40555bc483 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/revisions.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/revisions.py @@ -32,8 +32,9 @@ from nemo_evaluator.api.schemas import LATEST_TAG, REF_FRAGMENT_CHARSET from nemo_evaluator.content_hash import DIGEST_PATTERN, content_hash from nemo_evaluator.entities import ( + REVISION_POINTER_EXCLUDE, REVISION_POINTER_FIELDS, - REVISION_SELF_FIELDS, + REVISION_SELF_EXCLUDE, TaskEntity, TaskRevisionEntity, TasksetEntity, @@ -117,7 +118,7 @@ def head_digest(head: EntityBase) -> str: The exclusion is what makes this comparable to a revision's own digest: pointers describe *which* content is current, not what the content is. """ - return content_hash(head, exclude=REVISION_POINTER_FIELDS) + return content_hash(head, exclude=REVISION_POINTER_EXCLUDE) def validate_tag_name(tag: str) -> str: @@ -293,7 +294,7 @@ def _verify_content(head: TaskEntity | TasksetEntity, revision: TaskRevisionEnti write to one. This turns that convention into something detectable rather than something the reader has to assume. """ - actual = content_hash(revision, exclude=REVISION_SELF_FIELDS) + actual = content_hash(revision, exclude=REVISION_SELF_EXCLUDE) if actual != revision.content_hash: raise RevisionContentMismatchError( f"revision {revision.revision} of '{head.workspace}/{head.name}' does not match its " @@ -394,6 +395,8 @@ async def publish_revision( return current, head, False ordinal = head.latest_revision + 1 + # Copy *all* content, including fields the digest excludes: a revision stores the full + # published spec, and only its hash ignores the derived parts. content = head.model_dump(exclude=set(REVISION_POINTER_FIELDS) | set(head.__base_fields__), mode="json") revision = revision_type( name=revision_name(ordinal), diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/task_refs.py b/plugins/nemo-evaluator/src/nemo_evaluator/task_refs.py index d7788799bd..1fccec90e8 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/task_refs.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/task_refs.py @@ -18,7 +18,7 @@ from typing import cast -from nemo_evaluator.api.schemas import TasksetRef, parse_subentity_ref +from nemo_evaluator.api.schemas import EvaluatorTaskDefinition, TasksetRef, parse_subentity_ref from nemo_evaluator.entities import TaskEntity, TaskRevisionEntity, TasksetEntity, TasksetRevisionEntity from nemo_evaluator.jobs.agent_spec import AgentEvalTaskInput from nemo_evaluator.revisions import RevisionNotFoundError, get_revision @@ -26,6 +26,19 @@ from nemo_platform_plugin.entity_client import NemoEntityNotFoundError +class UnsupportedTaskKindError(ValueError): + """A taskset member's runner kind cannot be executed by the requested target. + + A taskset may group tasks of different kinds — that is the point of managing every evaluation + unit in one place — but a single run has one target, so expansion is where a mismatch surfaces. + + Raised during ``to_spec``, which the job submit path wraps: any exception there becomes a 422 + carrying this message (``_apply_transformer`` in ``nemo_platform_plugin.jobs.api_factory``). It + subclasses ``ValueError`` for local callers that catch it deliberately, not to obtain that + mapping — the mapping is a catch-all and would apply to any exception type. + """ + + def _entity_to_task_input(entity: TaskEntity, revision: TaskRevisionEntity) -> AgentEvalTaskInput: """Project a stored task's *published revision* onto the submitter-facing inline task DTO. @@ -36,15 +49,32 @@ def _entity_to_task_input(entity: TaskEntity, revision: TaskRevisionEntity) -> A A stored task holds metric *references* (inline metrics were normalized to derived stored metrics on create); those resolve to inline bundles in the shared metric-ref pass that runs - after expansion. A stored task carries no grader-only ``reference`` (the entity has no such - field), so taskset-driven tasks run with an empty one. + after expansion. The grader-only ``reference`` comes from the revision too, so a taskset-driven + run grades against the ground truth that revision pinned — held-out data is not the privilege of + inline submissions. """ + spec = revision.spec + if not isinstance(spec, EvaluatorTaskDefinition): + # A Harbor task's content is a *directory of files*, not fields — the runner needs the + # archive materialized on disk, which this pure projection cannot do. Rejecting here means a + # mismatched taskset fails before the run rather than silently evaluating an empty task. + # + # Deliberately does *not* suggest picking a different target: no target can run a stored + # task of this kind yet, so pointing at one would send the reader in circles. Storing the + # kind landed ahead of the execution bridge (AALGO-481). + raise UnsupportedTaskKindError( + f"Task '{entity.workspace}/{entity.name}' is a {spec.kind!r} task. Running a stored " + f"{spec.kind!r} task is not supported yet — no target can execute one, so this taskset " + "cannot be evaluated until that lands. Remove the member, or submit an " + "'evaluator'-kind taskset." + ) return AgentEvalTaskInput( id=entity.name, - intent=revision.intent, - inputs=revision.inputs, - metrics=list(revision.metrics), - views=revision.views, + intent=spec.intent, + inputs=spec.inputs, + reference=dict(spec.reference), + metrics=list(spec.metrics), + views=spec.views, metadata=revision.metadata, ) diff --git a/plugins/nemo-evaluator/tests/api/service/test_task_service.py b/plugins/nemo-evaluator/tests/api/service/test_task_service.py index 46b117f50d..b8cfa3ebcf 100644 --- a/plugins/nemo-evaluator/tests/api/service/test_task_service.py +++ b/plugins/nemo-evaluator/tests/api/service/test_task_service.py @@ -4,7 +4,16 @@ from __future__ import annotations import pytest -from nemo_evaluator.api.schemas import MetadataItem, MetricInline, MetricRef, Task, TaskInput, TaskInputs +from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, + HarborTaskDefinition, + MetadataItem, + MetricInline, + MetricRef, + Task, + TaskInput, + TaskInputs, +) from nemo_evaluator.api.service.task_service import MetricRefNotFoundError, TaskService from nemo_evaluator.shared.metric_bundles.bundles import bundle_metric from nemo_evaluator.shared.metric_bundles.cloudpickle import CloudpickleMetricBundlePackager @@ -13,10 +22,17 @@ class _FakeMetricService: - """Records inline-metric normalization so we can assert a task stores refs, not bundles.""" + """Records both metric-service entry points. + + ``stored`` covers inline-metric normalization, so a test can assert a task stores refs rather + than bundles. ``looked_up`` covers ref validation — recorded separately because "this task never + touched the metric service" is a claim about *both* calls, and asserting only on ``stored`` + would leave a lookup-only path silently passing. + """ def __init__(self, existing: set[tuple[str, str]] | None = None) -> None: self.stored: list[MetricInline] = [] + self.looked_up: list[tuple[str, str]] = [] self.existing = existing if existing is not None else {("default", "stored-metric")} async def store_derived_metric(self, metric: MetricInline, *, workspace: str) -> MetricRef: @@ -24,6 +40,7 @@ async def store_derived_metric(self, metric: MetricInline, *, workspace: str) -> return MetricRef(f"{workspace}/derived.{metric.payload.digest}") async def get_metric(self, workspace: str, name: str) -> object | None: + self.looked_up.append((workspace, name)) return object() if (workspace, name) in self.existing else None @@ -35,11 +52,37 @@ def _inline_metric() -> MetricInline: return MetricInline.model_validate(bundle.model_dump(mode="json")) +def _evaluator_spec(task: Task) -> EvaluatorTaskDefinition: + """Narrow ``Task.spec`` to the evaluator variant before reading a variant-specific field. + + ``spec`` is a discriminated union, so a test that reads ``intent``/``metrics``/``reference`` has + to say which kind it expects. Asserting it rather than assuming it means a change that routed + the wrong variant here fails on the kind, not with an ``AttributeError`` mid-assertion. + """ + assert isinstance(task.spec, EvaluatorTaskDefinition) + return task.spec + + +def _harbor_spec(task: Task) -> HarborTaskDefinition: + """The Harbor half of :func:`_evaluator_spec`.""" + assert isinstance(task.spec, HarborTaskDefinition) + return task.spec + + +def _ref(metric: MetricRef | MetricInline) -> MetricRef: + """Narrow a stored task's metric to a reference — inline bundles are offloaded on create.""" + assert isinstance(metric, MetricRef) + return metric + + def _task_input() -> TaskInput: return TaskInput( - intent="Answer the question.", - inputs=TaskInputs(instruction="What is 2+2?"), - metrics=[MetricRef("default/stored-metric")], + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the question.", + inputs=TaskInputs(instruction="What is 2+2?"), + metrics=[MetricRef("default/stored-metric")], + ), metadata=[MetadataItem(key="suite", value="smoke")], ) @@ -60,8 +103,8 @@ async def test_create_then_get(service: TaskService) -> None: assert isinstance(created, Task) assert created.name == "task-1" assert created.id == "task-task-1" - assert created.intent == "Answer the question." - assert isinstance(created.metrics[0], MetricRef) + assert _evaluator_spec(created).intent == "Answer the question." + assert isinstance(_evaluator_spec(created).metrics[0], MetricRef) assert created.created_at is not None got = await service.get_task("default", "task-1") @@ -73,9 +116,12 @@ async def test_create_normalizes_inline_metrics_to_refs( ) -> None: inline = _inline_metric() task_input = TaskInput( - intent="Answer the question.", - inputs=TaskInputs(instruction="What is 2+2?"), - metrics=[MetricRef("default/stored-metric"), inline], + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the question.", + inputs=TaskInputs(instruction="What is 2+2?"), + metrics=[MetricRef("default/stored-metric"), inline], + ) ) created, _ = await service.create_task("task-1", task_input, workspace="default") @@ -83,22 +129,55 @@ async def test_create_normalizes_inline_metrics_to_refs( # The inline metric was offloaded to the metric service (stored as a derived metric)... assert metric_service.stored == [inline] # ...and the persisted task holds only refs — the passthrough ref plus the derived one. - assert all(isinstance(m, MetricRef) for m in created.metrics) - assert created.metrics[0].root == "default/stored-metric" - assert created.metrics[1].root == f"default/derived.{inline.payload.digest}" + assert all(isinstance(m, MetricRef) for m in _evaluator_spec(created).metrics) + assert _ref(_evaluator_spec(created).metrics[0]).root == "default/stored-metric" + assert _ref(_evaluator_spec(created).metrics[1]).root == f"default/derived.{inline.payload.digest}" + + +async def test_create_preserves_grader_only_reference(service: TaskService) -> None: + """Normalization narrows ``metrics`` and must leave the rest of the spec alone. + + ``_normalize_spec`` rebuilds the spec with ``model_copy(update=...)``, so a field it does not + name rides along untouched — this pins that, since silently dropping ground truth would leave + metrics grading against nothing while the run still reported a score. + """ + reference = {"expected": "Paris", "held_out_tests": ["test_capital.py"]} + task_input = TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the question.", + inputs=TaskInputs(instruction="What is the capital of France?"), + reference=reference, + metrics=[_inline_metric()], + ) + ) + + created, _ = await service.create_task("task-1", task_input, workspace="default") + + assert _evaluator_spec(created).reference == reference, "normalizing metrics must not disturb the reference" + got = await service.get_task("default", "task-1") + assert got is not None and _evaluator_spec(got).reference == reference async def test_create_rejects_missing_metric_ref(service: TaskService) -> None: - task_input = TaskInput(intent="x", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("default/nope")]) + task_input = TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", intent="x", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("default/nope")] + ) + ) with pytest.raises(MetricRefNotFoundError, match="not found"): await service.create_task("task-1", task_input, workspace="default") async def test_create_canonicalizes_bare_metric_ref(service: TaskService) -> None: # A bare "stored-metric" ref resolves against the task workspace and is persisted as "default/stored-metric". - task_input = TaskInput(intent="x", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("stored-metric")]) + task_input = TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", intent="x", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("stored-metric")] + ) + ) created, _ = await service.create_task("task-1", task_input, workspace="default") - assert created.metrics[0].root == "default/stored-metric" + assert _ref(_evaluator_spec(created).metrics[0]).root == "default/stored-metric" async def test_create_rejects_duplicate(service: TaskService) -> None: @@ -195,7 +274,12 @@ async def _boom(entity): entity_store.create = _boom changed = TaskInput( - intent="Rewritten.", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("default/stored-metric")] + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Rewritten.", + inputs=TaskInputs(instruction="?"), + metrics=[MetricRef("default/stored-metric")], + ) ) with pytest.raises(RuntimeError): await service.replace_task("task-1", changed, workspace="default") @@ -203,7 +287,7 @@ async def _boom(entity): entity_store.create = real_create.__get__(entity_store) head = await service.get_task("default", "task-1") assert head is not None - assert head.intent == "Answer the question.", "the head must still hold the last published content" + assert _evaluator_spec(head).intent == "Answer the question.", "the head must still hold the last published content" async def test_tag_revision_returns_none_for_a_missing_task(service: TaskService) -> None: @@ -273,7 +357,7 @@ async def test_resolve_revision_honours_a_tag_naming_an_older_revision(service: await service.tag_revision("default", "task-1", "blessed", "latest") revised = _task_input() - revised.intent = "Answer differently." + revised.spec.intent = "Answer differently." await service.replace_task("task-1", revised, workspace="default") latest = await service.resolve_revision("default", "task-1") @@ -289,7 +373,7 @@ async def test_resolve_revision_round_trips_a_digest_fragment(service: TaskServi first = (await service.list_revisions("default", "task-1")).data[0].content_hash revised = _task_input() - revised.intent = "Answer differently." + revised.spec.intent = "Answer differently." await service.replace_task("task-1", revised, workspace="default") assert await service.resolve_revision("default", "task-1", first) == first @@ -300,3 +384,134 @@ async def test_resolve_revision_raises_for_a_missing_task(service: TaskService) member that does not exist, now that the separate existence check is gone.""" with pytest.raises(NemoEntityNotFoundError): await service.resolve_revision("default", "nope") + + +# --- Harbor-kind tasks -------------------------------------------------------- + + +def _harbor_input(digest: str = "a" * 64) -> TaskInput: + return TaskInput( + spec=HarborTaskDefinition( + kind="harbor", + archive_ref="default/harbor-tasks#packages/org-name/abc/dist.tar.gz", + archive_digest=digest, + instruction="Fix the failing test.", + config={"verifier": {"type": "pytest"}}, + ), + metadata=[MetadataItem(key="suite", value="swe")], + ) + + +async def test_stores_a_harbor_task(service: TaskService) -> None: + """Both kinds live in one record type, so a user manages every evaluation unit in one place.""" + created, published = await service.create_task("fix-test", _harbor_input(), workspace="default") + + assert published + assert created.spec.kind == "harbor" + assert created.spec.archive_ref.endswith("dist.tar.gz") + assert created.spec.config == {"verifier": {"type": "pytest"}} + + +async def test_harbor_task_publishes_revisions_like_any_other(service: TaskService) -> None: + await service.create_task("fix-test", _harbor_input(), workspace="default") + + same, published_again = await service.replace_task("fix-test", _harbor_input(), workspace="default") + assert not published_again, "identical content must not cut a revision" + + changed, published = await service.replace_task("fix-test", _harbor_input(digest="b" * 64), workspace="default") + assert published and changed.revision == 2 + + +async def test_a_harbor_task_never_reaches_the_metric_service( + service: TaskService, metric_service: _FakeMetricService +) -> None: + """Metric normalization is agent-eval-specific: a Harbor task is scored by Harbor's own reward, + and its spec arrives already in stored form. + + Both entry points, not just the write: a Harbor spec must not be validated against stored + metrics either, so ``_normalize_spec`` has to short-circuit before ref resolution rather than + merely find nothing to offload. + """ + await service.create_task("fix-test", _harbor_input(), workspace="default") + assert metric_service.stored == [] + assert metric_service.looked_up == [] + + +async def test_kinds_with_matching_metadata_do_not_share_a_digest(service: TaskService) -> None: + """The revision digest covers the whole spec, so two kinds cannot collide on content.""" + harbor, _ = await service.create_task("a", _harbor_input(), workspace="default") + agent, _ = await service.create_task("b", _task_input(), workspace="default") + + harbor_revisions = await service.list_revisions("default", "a") + agent_revisions = await service.list_revisions("default", "b") + assert harbor_revisions is not None and agent_revisions is not None + assert harbor_revisions.data[0].content_hash != agent_revisions.data[0].content_hash + + +async def test_harbor_config_is_stored_but_not_hashed(service: TaskService) -> None: + """`config` is a projection of task.toml, which lives inside the archive — a real change moves + `archive_digest`. Hashing the projection too would make our history sensitive to Harbor's + serialization: a release that reordered keys would cut a revision for byte-identical files.""" + await service.create_task("fix-test", _harbor_input(), workspace="default") + + reserialized = TaskInput( + spec=HarborTaskDefinition( + kind="harbor", + archive_ref="default/harbor-tasks#packages/org-name/abc/dist.tar.gz", + archive_digest="a" * 64, + instruction="Fix the failing test.", + config={"verifier": {"type": "pytest"}, "added_by_a_new_harbor_release": True}, + ), + metadata=[MetadataItem(key="suite", value="swe")], + ) + same, published = await service.replace_task("fix-test", reserialized, workspace="default") + + assert not published, "a config-only change must not cut a revision" + assert same.revision == 1 + + # ...but the new config is *persisted*, so the queryable projection stays current. Re-read + # rather than trusting the returned object: `replace_task` builds its result from the head it + # already mutated in memory, so asserting on `same` would pass even if nothing were written. + # On this path the write is a lone `entity_client.update` whose comment justifies it by + # `project` alone — drop it as a redundant round trip and only a re-read notices. + refetched = await service.get_task("default", "fix-test") + assert refetched is not None + assert _harbor_spec(refetched).config["added_by_a_new_harbor_release"] is True + + +async def test_reference_only_change_publishes_a_revision(service: TaskService) -> None: + """The mirror of the Harbor ``config`` case, and the reason the two differ. + + ``config`` is excluded because it is a projection of content ``archive_digest`` already covers. + ``reference`` is nothing of the sort: it is the ground truth a metric grades against, so a task + whose reference changed scores differently and must be a distinct revision. Deduping it onto the + old digest would let a pinned taskset silently re-grade. + """ + + def _graded(expected: str) -> TaskInput: + return TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the question.", + inputs=TaskInputs(instruction="What is the capital of France?"), + reference={"expected": expected}, + metrics=[MetricRef("default/stored-metric")], + ) + ) + + await service.create_task("capital", _graded("Paris"), workspace="default") + + same, published_again = await service.replace_task("capital", _graded("Paris"), workspace="default") + assert not published_again and same.revision == 1, "identical content must still dedup" + + changed, published = await service.replace_task("capital", _graded("Lyon"), workspace="default") + assert published, "changing the ground truth must cut a new revision" + assert changed.revision == 2 + assert _evaluator_spec(changed).reference == {"expected": "Lyon"} + + +async def test_a_real_archive_change_does_cut_a_revision(service: TaskService) -> None: + """The flip side: `archive_digest` is the authoritative identity, so it must still move.""" + await service.create_task("fix-test", _harbor_input(), workspace="default") + changed, published = await service.replace_task("fix-test", _harbor_input(digest="b" * 64), workspace="default") + assert published and changed.revision == 2 diff --git a/plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py b/plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py index d8a71339d7..c7b3861471 100644 --- a/plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py +++ b/plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py @@ -13,7 +13,14 @@ from fastapi import FastAPI from fastapi.testclient import TestClient from nemo_evaluator.api.dependencies import get_task_service -from nemo_evaluator.api.schemas import MetricInline, MetricRef, TaskInput, TaskInputs +from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, + HarborTaskDefinition, + MetricInline, + MetricRef, + TaskInput, + TaskInputs, +) from nemo_evaluator.api.service.task_service import TaskService from nemo_evaluator.api.v2 import tasks as tasks_routes from nemo_platform_plugin.entity_client import NemoEntityConflictError @@ -41,9 +48,12 @@ def client(entity_store) -> TestClient: def _body(*, intent: str = "Answer the question.", tags: list[str] | None = None) -> dict: return TaskInput( - intent=intent, - inputs=TaskInputs(instruction="What is 2+2?"), - metrics=[MetricRef("default/stored-metric")], + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent=intent, + inputs=TaskInputs(instruction="What is 2+2?"), + metrics=[MetricRef("default/stored-metric")], + ), tags=tags or [], ).model_dump(mode="json") @@ -59,14 +69,14 @@ def test_create_then_get(client: TestClient) -> None: got = client.get(f"{_BASE}/task-1") assert got.status_code == 200 body = got.json() - assert body["intent"] == "Answer the question." - assert body["metrics"] == ["default/stored-metric"] # MetricRef serializes to a bare string + assert body["spec"]["intent"] == "Answer the question." + assert body["spec"]["metrics"] == ["default/stored-metric"] # MetricRef serializes to a bare string def test_create_rejects_unrecognized_input_key(client: TestClient) -> None: # inputs is a strict TaskInputs (extra="forbid") — an unknown key is a 422, not silently stored. body = _body() - body["inputs"]["expected"] = "4" + body["spec"]["inputs"]["expected"] = "4" assert client.post(f"{_BASE}/task-1", json=body).status_code == 422 @@ -79,10 +89,29 @@ def test_create_rejects_duplicate_metadata_keys(client: TestClient) -> None: def test_create_missing_metric_ref_returns_422(client: TestClient) -> None: body = _body() - body["metrics"] = ["default/missing-metric"] + body["spec"]["metrics"] = ["default/missing-metric"] assert client.post(f"{_BASE}/task-1", json=body).status_code == 422 +@pytest.mark.parametrize("method", ["post", "put"]) +def test_write_without_a_spec_kind_returns_422(client: TestClient, method: str) -> None: + """``spec`` is a discriminated union, so a raw body that omits ``kind`` has no variant to + validate against. ``kind`` is therefore required in both definitions — a schema that defaulted + it would tell a generated client it may be omitted, and every such request would 422.""" + body = _body() + del body["spec"]["kind"] + response = getattr(client, method)(f"{_BASE}/task-1", json=body) + assert response.status_code == 422 + assert response.json()["detail"][0]["type"] == "union_tag_not_found" + + +@pytest.mark.parametrize("definition", [EvaluatorTaskDefinition, HarborTaskDefinition]) +def test_published_schema_requires_the_kind_discriminator(definition: type) -> None: + """The generated spec has to agree with the validator above: a default on ``kind`` would leave + it out of ``required``, and a client generated from that spec would omit it.""" + assert "kind" in definition.model_json_schema()["required"] + + def test_create_duplicate_returns_409(client: TestClient) -> None: assert client.post(f"{_BASE}/task-1", json=_body()).status_code == 201 assert client.post(f"{_BASE}/task-1", json=_body()).status_code == 409 @@ -185,7 +214,7 @@ def test_get_returns_the_current_revision(client: TestClient) -> None: client.put(f"{_BASE}/task-1", json=_body(intent="Do something else.")) got = client.get(f"{_BASE}/task-1").json() assert got["revision"] == 2 - assert got["intent"] == "Do something else." + assert got["spec"]["intent"] == "Do something else." # --- Reading and tagging a specific revision --------------------------------- @@ -214,9 +243,9 @@ def test_get_by_digest_returns_the_published_content(client: TestClient) -> None client.put(f"{_BASE}/task-1", json=_body(intent="Newer.")) pinned = client.get(f"{_BASE}/task-1/revisions/{digest}").json() - assert pinned["intent"] == first["intent"] + assert pinned["spec"]["intent"] == first["spec"]["intent"] assert pinned["revision"] == 1 - assert client.get(f"{_BASE}/task-1").json()["intent"] == "Newer." + assert client.get(f"{_BASE}/task-1").json()["spec"]["intent"] == "Newer." def test_get_by_tag_resolves(client: TestClient) -> None: @@ -271,3 +300,24 @@ async def _stale(entity, *, original_name=None): entity_store.update = _stale assert client.put(f"{_BASE}/task-1", json=_body(intent="Newer.")).status_code == 409 + + +def test_list_includes_harbor_tasks(client: TestClient) -> None: + """Both kinds are one record type, so the listing must serialize either.""" + client.post(f"{_BASE}/evaluator-task", json=_body()) + client.post( + f"{_BASE}/harbor-task", + json=TaskInput( + spec=HarborTaskDefinition( + kind="harbor", archive_ref="default/harbor#packages/o-n/abc/dist.tar.gz", archive_digest="a" * 64 + ) + ).model_dump(mode="json"), + ) + + response = client.get(_BASE) + + assert response.status_code == 200 + assert {t["name"]: t["spec"]["kind"] for t in response.json()["data"]} == { + "evaluator-task": "evaluator", + "harbor-task": "harbor", + } diff --git a/plugins/nemo-evaluator/tests/integration/test_agent_evaluate_job.py b/plugins/nemo-evaluator/tests/integration/test_agent_evaluate_job.py index 0bb2892a33..05156539d7 100644 --- a/plugins/nemo-evaluator/tests/integration/test_agent_evaluate_job.py +++ b/plugins/nemo-evaluator/tests/integration/test_agent_evaluate_job.py @@ -37,6 +37,7 @@ import httpx import pytest from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, MetricInline, TaskInput, TaskInputs, @@ -554,9 +555,12 @@ def test_submit_over_taskset_ref_resolves_and_scores(subprocess_platform: str) - client.evaluator.tasks.create( name, task=TaskInput( - intent="Obtain a one-word reply from the model.", - inputs=TaskInputs(instruction="Reply with the single word DONE and nothing else."), - metrics=[MetricRef(f"{WORKSPACE}/{metric_name}")], + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Obtain a one-word reply from the model.", + inputs=TaskInputs(instruction="Reply with the single word DONE and nothing else."), + metrics=[MetricRef(f"{WORKSPACE}/{metric_name}")], + ) ), ) taskset_name = _unique("done-suite") diff --git a/plugins/nemo-evaluator/tests/integration/test_docs_manage_tasks_tasksets.py b/plugins/nemo-evaluator/tests/integration/test_docs_manage_tasks_tasksets.py new file mode 100644 index 0000000000..e4b5ca842f --- /dev/null +++ b/plugins/nemo-evaluator/tests/integration/test_docs_manage_tasks_tasksets.py @@ -0,0 +1,205 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The ``Manage Tasks & Tasksets`` doc walkthrough, executed against a real platform. + +``make docs-check-python-snippets`` type-checks the doc's snippets, which catches a snippet that +names a field that no longer exists — but not one that type-checks and then fails at run time, and +not a documented *output* that no longer matches. Both happened: the task model moved its content +under a discriminated ``spec``, and the revision snippets in this doc kept the old flat shape +through review because nothing executed them. + +So this walks the doc top to bottom, in order, doing what it says and asserting the results it +claims. It deliberately mirrors the doc's own code rather than being written as an idiomatic test — +when it fails, the fix is usually the doc. + +Pure CRUD (no codex/IGW), so it only needs the host subprocess backend. Shares the evaluator-plugin +integration opt-in (``RUN_AGENT_EVAL_INTEGRATION``) and the session-scoped ``subprocess_platform``. +""" + +from __future__ import annotations + +import os +import uuid + +import pytest +from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, + MetadataItem, + MetricRef, + TaskInput, + TaskInputs, + TaskRef, + TasksetInput, + TasksetRef, +) +from nemo_evaluator_sdk import ExactMatchMetric +from nemo_platform import NeMoPlatform + +pytestmark = [ + pytest.mark.integration, + pytest.mark.skipif( + not os.environ.get("RUN_AGENT_EVAL_INTEGRATION"), + reason="opt-in; set RUN_AGENT_EVAL_INTEGRATION=1 to run (spins real nemo services platforms)", + ), +] + +WORKSPACE = "default" + + +@pytest.fixture +def doc_client(subprocess_platform: str) -> NeMoPlatform: + """The doc's own ``Initialize the SDK`` snippet, with the base URL the fixture provides. + + ``workspace=`` on the constructor is part of what is being checked: every later snippet omits a + per-call workspace and relies on this default. + """ + client = NeMoPlatform(base_url=subprocess_platform, workspace=WORKSPACE, max_retries=2) + client.workspaces.create(name=WORKSPACE, exist_ok=True) + return client + + +def _unique(prefix: str) -> str: + """Names are per-test so a reused platform can't leak state between them.""" + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +@pytest.mark.timeout(300) +def test_the_manage_tasks_walkthrough(doc_client: NeMoPlatform) -> None: + """``Manage Tasks`` through ``Tag a revision`` — create, read, publish, pin, tag.""" + client = doc_client + tasks = client.evaluator.tasks + task_name = _unique("capital-of-france") + metric_name = _unique("answer-exact-match") + + client.evaluator.metrics.create( + metric_name, + metric=ExactMatchMetric(reference="{{item.expected}}", candidate="{{item.output}}"), + ) + + task = TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the user's geography question with the capital city.", + inputs=TaskInputs(instruction="What is the capital of France?"), + metrics=[MetricRef(f"{WORKSPACE}/{metric_name}")], + ), + metadata=[MetadataItem(key="suite", value="geography")], + ) + + stored = tasks.create(task_name, task=task) + # The doc prints `stored.id, stored.spec.metrics` and states that a stored task holds metric + # *references* only. + assert stored.id + assert [ref.root for ref in stored.spec.metrics] == [f"{WORKSPACE}/{metric_name}"] + + # "Retrieve, list, and delete" — the doc's comment claims `evaluator 1 {'latest': 1}`. + retrieved = tasks.retrieve(task_name) + assert (retrieved.spec.kind, retrieved.revision, retrieved.tags) == ("evaluator", 1, {"latest": 1}) + + page = tasks.list(page=1, page_size=100, sort="-created_at") + assert (task_name, "evaluator") in [(item.name, item.spec.kind) for item in page.data] + + # "Publish a new revision" — the doc's comment claims revision 2. + revised_task = TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the user's geography question with the capital city.", + inputs=TaskInputs(instruction="Name the capital city of France."), + metrics=[MetricRef(f"{WORKSPACE}/{metric_name}")], + ), + metadata=[MetadataItem(key="suite", value="geography")], + ) + updated = tasks.replace(task_name, task=revised_task) + assert updated.revision == 2 + + # The doc's idempotence Note: re-submitting identical content publishes nothing. + assert tasks.replace(task_name, task=revised_task).revision == 2 + + # "Read a specific revision" — a pinned read returns what was published, not what is current. + revisions = tasks.list_revisions(task_name) + digest = next(revision.content_hash for revision in revisions.data if revision.revision == 1) + + original = tasks.retrieve(task_name, revision=digest) + current = tasks.retrieve(task_name) + assert original.revision == 1 and current.revision == 2 + assert original.spec.inputs.instruction != current.spec.inputs.instruction + + # "Tag a revision", including the documented `ValueError` when both selectors are passed. + tasks.tag(task_name, tag="blessed", revision=digest) + blessed = tasks.retrieve(task_name, tag="blessed") + assert blessed.revision == 1 + with pytest.raises(ValueError): + tasks.retrieve(task_name, revision=digest, tag="blessed") + + tasks.delete(task_name) + + +@pytest.mark.timeout(300) +def test_the_manage_tasksets_walkthrough(doc_client: NeMoPlatform) -> None: + """``Manage Tasksets`` and ``Pin the taskset itself`` — membership pinning is the claim.""" + client = doc_client + tasks = client.evaluator.tasks + tasksets = client.evaluator.tasksets + france, japan = _unique("capital-of-france"), _unique("capital-of-japan") + suite = _unique("geography-suite") + + for name, city in ((france, "France"), (japan, "Japan")): + tasks.create( + name, + task=TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the user's geography question with the capital city.", + inputs=TaskInputs(instruction=f"What is the capital of {city}?"), + ) + ), + ) + + taskset = TasksetInput( + description="Geography questions for smoke-testing the agent.", + tasks=[TaskRef(f"{WORKSPACE}/{france}"), TaskRef(f"{WORKSPACE}/{japan}")], + ) + stored = tasksets.create(suite, taskset=taskset) + + # The doc's central claim: a bare member ref is stored resolved to `workspace/name#`. + assert all("#" in ref.root for ref in stored.tasks) + assert {ref.root.split("#")[0] for ref in stored.tasks} == {f"{WORKSPACE}/{france}", f"{WORKSPACE}/{japan}"} + + page = tasksets.list(page=1, page_size=100, sort="name") + assert suite in [item.name for item in page.data] + assert tasksets.retrieve(suite).description == "Geography questions for smoke-testing the agent." + + # The doc's Note: member *order* is not part of a taskset's identity, so reordering the same + # members publishes nothing. + reordered = TasksetInput( + description="Geography questions for smoke-testing the agent.", + tasks=[TaskRef(f"{WORKSPACE}/{japan}"), TaskRef(f"{WORKSPACE}/{france}")], + ) + assert tasksets.replace(suite, taskset=reordered).revision == 1 + + # ...but re-resolving after a member republishes genuinely differs, so it does cut a revision. + tasks.replace( + france, + task=TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the user's geography question with the capital city.", + inputs=TaskInputs(instruction="Name the capital city of France."), + ) + ), + ) + assert tasksets.replace(suite, taskset=taskset).revision == 2 + + # "Pin the taskset itself" — both ref forms are accepted by the field. + current = tasksets.list_revisions(suite).data[0] + assert current.revision == 2 # revisions come back newest-first, as the doc's comment says + assert TasksetRef(f"{WORKSPACE}/{suite}").root + assert TasksetRef(f"{WORKSPACE}/{suite}#{current.content_hash}").root + + # Deleting a taskset does not delete its member tasks. + tasksets.delete(suite) + assert tasks.retrieve(france).name == france + + for name in (france, japan): + tasks.delete(name) diff --git a/plugins/nemo-evaluator/tests/integration/test_task_derived_metrics.py b/plugins/nemo-evaluator/tests/integration/test_task_derived_metrics.py index b377e1efe4..8b9d8106e9 100644 --- a/plugins/nemo-evaluator/tests/integration/test_task_derived_metrics.py +++ b/plugins/nemo-evaluator/tests/integration/test_task_derived_metrics.py @@ -22,7 +22,7 @@ import uuid import pytest -from nemo_evaluator.api.schemas import MetricInline, TaskInput +from nemo_evaluator.api.schemas import EvaluatorTaskDefinition, MetricInline, TaskInput from nemo_evaluator.shared.metric_bundles.bundles import bundle_metric from nemo_evaluator.shared.metric_bundles.cloudpickle import CloudpickleMetricBundlePackager from nemo_evaluator_sdk.metrics.exact_match import ExactMatchMetric @@ -55,7 +55,11 @@ def _inline_metric(marker: str) -> MetricInline: def _task_input(metric: MetricInline) -> TaskInput: - return TaskInput(intent="Answer the question.", inputs={"instruction": "What is 2+2?"}, metrics=[metric]) + return TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", intent="Answer the question.", inputs={"instruction": "What is 2+2?"}, metrics=[metric] + ) + ) @pytest.mark.timeout(300) @@ -70,14 +74,16 @@ def test_inline_task_metric_normalizes_to_derived_metric(subprocess_platform: st try: # The inline metric is offloaded: the stored task holds a single derived reference, not a bundle. created_a = client.evaluator.tasks.create(task_a, task=_task_input(inline), workspace=WORKSPACE) - assert len(created_a.metrics) == 1 - derived_ref = created_a.metrics[0].root + assert isinstance(created_a.spec, EvaluatorTaskDefinition) + assert len(created_a.spec.metrics) == 1 + derived_ref = created_a.spec.metrics[0].root assert derived_ref.startswith(f"{WORKSPACE}/derived.") derived_name = derived_ref.split("/", 1)[1] # A second task with byte-identical inline content dedupes to the same derived metric. created_b = client.evaluator.tasks.create(task_b, task=_task_input(inline), workspace=WORKSPACE) - assert created_b.metrics[0].root == derived_ref + assert isinstance(created_b.spec, EvaluatorTaskDefinition) + assert created_b.spec.metrics[0].root == derived_ref # The derived metric is a real, Files-backed, flagged metric. fetched = client.evaluator.metrics.retrieve(derived_name, workspace=WORKSPACE) diff --git a/plugins/nemo-evaluator/tests/integration/test_task_revisions.py b/plugins/nemo-evaluator/tests/integration/test_task_revisions.py index 4e814251ff..bbe7f34d9c 100644 --- a/plugins/nemo-evaluator/tests/integration/test_task_revisions.py +++ b/plugins/nemo-evaluator/tests/integration/test_task_revisions.py @@ -27,7 +27,12 @@ import uuid import pytest -from nemo_evaluator.api.schemas import TaskInput, TasksetInput +from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, + HarborTaskDefinition, + TaskInput, + TasksetInput, +) from nemo_platform import NeMoPlatform pytestmark = [ @@ -46,7 +51,10 @@ def _unique(prefix: str) -> str: def _task_input(intent: str = "Answer the question.", *, tags: list[str] | None = None) -> TaskInput: - return TaskInput(intent=intent, inputs={"instruction": "What is 2+2?"}, tags=tags or []) + return TaskInput( + spec=EvaluatorTaskDefinition(kind="evaluator", intent=intent, inputs={"instruction": "What is 2+2?"}), + tags=tags or [], + ) def _client(base_url: str) -> NeMoPlatform: @@ -71,9 +79,12 @@ def test_publish_and_read_a_pinned_revision(subprocess_platform: str) -> None: assert replaced.revision == 2 pinned = client.evaluator.tasks.retrieve(name, revision=first_digest, workspace=WORKSPACE) - assert pinned.intent == "First." + assert isinstance(pinned.spec, EvaluatorTaskDefinition) + assert pinned.spec.intent == "First." assert pinned.revision == 1 - assert client.evaluator.tasks.retrieve(name, workspace=WORKSPACE).intent == "Second." + current = client.evaluator.tasks.retrieve(name, workspace=WORKSPACE) + assert isinstance(current.spec, EvaluatorTaskDefinition) + assert current.spec.intent == "Second." finally: client.evaluator.tasks.delete(name, workspace=WORKSPACE) @@ -165,7 +176,9 @@ def test_tagging_an_older_revision_leaves_latest_alone(subprocess_platform: str) assert tagged.tags["blessed"] == 1 assert tagged.tags["latest"] == 2, "latest is machine-managed and must not follow a manual tag" - assert client.evaluator.tasks.retrieve(name, tag="blessed", workspace=WORKSPACE).intent == "First." + blessed = client.evaluator.tasks.retrieve(name, tag="blessed", workspace=WORKSPACE) + assert isinstance(blessed.spec, EvaluatorTaskDefinition) + assert blessed.spec.intent == "First." finally: client.evaluator.tasks.delete(name, workspace=WORKSPACE) @@ -214,10 +227,9 @@ def test_taskset_membership_is_pinned_and_stays_pinned(subprocess_platform: str) client.evaluator.tasks.replace(task_name, task=_task_input("Updated."), workspace=WORKSPACE) assert client.evaluator.tasksets.retrieve(set_name, workspace=WORKSPACE).tasks[0].root == member - assert ( - client.evaluator.tasks.retrieve(task_name, revision=pinned_digest, workspace=WORKSPACE).intent - == "Original." - ) + pinned_task = client.evaluator.tasks.retrieve(task_name, revision=pinned_digest, workspace=WORKSPACE) + assert isinstance(pinned_task.spec, EvaluatorTaskDefinition) + assert pinned_task.spec.intent == "Original." finally: client.evaluator.tasksets.delete(set_name, workspace=WORKSPACE) client.evaluator.tasks.delete(task_name, workspace=WORKSPACE) @@ -247,3 +259,80 @@ def test_republishing_a_taskset_after_a_member_moves_cuts_a_revision(subprocess_ finally: client.evaluator.tasksets.delete(set_name, workspace=WORKSPACE) client.evaluator.tasks.delete(task_name, workspace=WORKSPACE) + + +# --- Harbor-kind tasks -------------------------------------------------------- + + +def _harbor_input(digest: str = "a" * 64, *, config: dict | None = None) -> TaskInput: + return TaskInput( + spec=HarborTaskDefinition( + kind="harbor", + archive_ref="default/harbor-tasks#packages/org-name/abc/dist.tar.gz", + archive_digest=digest, + instruction="Fix the failing test.", + config=config if config is not None else {"verifier": {"type": "pytest"}}, + ) + ) + + +@pytest.mark.timeout(300) +def test_harbor_and_evaluator_tasks_coexist(subprocess_platform: str) -> None: + """Both kinds are one record type, so they list together and a taskset can group them — the + point of managing every evaluation unit in one place.""" + client = _client(subprocess_platform) + harbor_name, evaluator_name = _unique("harbor"), _unique("evaluator") + try: + harbor = client.evaluator.tasks.create(harbor_name, task=_harbor_input(), workspace=WORKSPACE) + evaluator = client.evaluator.tasks.create(evaluator_name, task=_task_input(), workspace=WORKSPACE) + + assert harbor.spec.kind == "harbor" + assert evaluator.spec.kind == "evaluator" + + listed = {t.name: t.spec.kind for t in client.evaluator.tasks.list(workspace=WORKSPACE, page_size=1000).data} + assert listed[harbor_name] == "harbor" + assert listed[evaluator_name] == "evaluator" + finally: + client.evaluator.tasks.delete(harbor_name, workspace=WORKSPACE) + client.evaluator.tasks.delete(evaluator_name, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_harbor_task_round_trips_through_the_store(subprocess_platform: str) -> None: + """The discriminated union has to survive the entity store's JSON column, which is the one + thing a unit test against an in-memory fake cannot confirm.""" + client = _client(subprocess_platform) + name = _unique("harbor") + try: + client.evaluator.tasks.create(name, task=_harbor_input(), workspace=WORKSPACE) + + fetched = client.evaluator.tasks.retrieve(name, workspace=WORKSPACE) + assert isinstance(fetched.spec, HarborTaskDefinition) + assert fetched.spec.kind == "harbor" + assert fetched.spec.archive_digest == "a" * 64 + assert fetched.spec.config == {"verifier": {"type": "pytest"}} + assert fetched.spec.instruction == "Fix the failing test." + finally: + client.evaluator.tasks.delete(name, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_harbor_config_changes_do_not_cut_a_revision(subprocess_platform: str) -> None: + """`config` is excluded from the digest because it is a projection of task.toml inside the + archive. Confirmed end-to-end, since the exclusion is applied where the digest is computed.""" + client = _client(subprocess_platform) + name = _unique("harbor") + try: + client.evaluator.tasks.create(name, task=_harbor_input(), workspace=WORKSPACE) + + same = client.evaluator.tasks.replace( + name, task=_harbor_input(config={"verifier": {"type": "pytest"}, "new_field": 1}), workspace=WORKSPACE + ) + assert same.revision == 1, "a config-only change must not publish" + assert isinstance(same.spec, HarborTaskDefinition) + assert same.spec.config["new_field"] == 1 + + moved = client.evaluator.tasks.replace(name, task=_harbor_input(digest="b" * 64), workspace=WORKSPACE) + assert moved.revision == 2, "an archive change must publish" + finally: + client.evaluator.tasks.delete(name, workspace=WORKSPACE) diff --git a/plugins/nemo-evaluator/tests/sdk/test_task_sdk_resources.py b/plugins/nemo-evaluator/tests/sdk/test_task_sdk_resources.py index 6334460ece..e9e80fdc0e 100644 --- a/plugins/nemo-evaluator/tests/sdk/test_task_sdk_resources.py +++ b/plugins/nemo-evaluator/tests/sdk/test_task_sdk_resources.py @@ -10,7 +10,13 @@ from unittest.mock import AsyncMock, MagicMock import pytest -from nemo_evaluator.api.schemas import MetricRef, Revision, Task, TaskInput +from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, + MetricRef, + Revision, + Task, + TaskInput, +) from nemo_evaluator.sdk.task_resources import AsyncEvaluatorTasksResource, EvaluatorTasksResource _BASE = "http://localhost:8080/apis/evaluator/v2/workspaces/default" @@ -19,12 +25,15 @@ def _task_payload(name: str) -> dict[str, Any]: now = datetime.now(timezone.utc) return Task( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the question.", + inputs={"instruction": "What is 2+2?"}, + metrics=[MetricRef("default/stored-metric")], + ), id=f"task-{name}", name=name, workspace="default", - intent="Answer the question.", - inputs={"instruction": "What is 2+2?"}, - metrics=[MetricRef("default/stored-metric")], revision=1, tags={"latest": 1}, created_at=now, @@ -33,7 +42,14 @@ def _task_payload(name: str) -> dict[str, Any]: def _task_input() -> TaskInput: - return TaskInput(intent="Answer.", inputs={"instruction": "x"}, metrics=[MetricRef("default/stored-metric")]) + return TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer.", + inputs={"instruction": "x"}, + metrics=[MetricRef("default/stored-metric")], + ) + ) def _response(payload: Any) -> MagicMock: @@ -63,7 +79,7 @@ def test_sync_create_posts_task_input_to_item_url() -> None: assert isinstance(result, Task) assert result.name == "task-1" assert http_client.post.call_args[0][0] == f"{_BASE}/tasks/task-1" - assert http_client.post.call_args.kwargs["json"]["intent"] == "Answer." + assert http_client.post.call_args.kwargs["json"]["spec"]["intent"] == "Answer." def test_sync_retrieve_targets_item_url_and_parses_dto() -> None: @@ -74,7 +90,8 @@ def test_sync_retrieve_targets_item_url_and_parses_dto() -> None: result = resource.retrieve("task-1") assert isinstance(result, Task) - assert isinstance(result.metrics[0], MetricRef) + assert isinstance(result.spec, EvaluatorTaskDefinition) + assert isinstance(result.spec.metrics[0], MetricRef) assert http_client.get.call_args[0][0] == f"{_BASE}/tasks/task-1" @@ -143,7 +160,7 @@ def test_sync_replace_puts_task_input_to_item_url() -> None: result = resource.replace("task-1", task=_task_input()) assert http_client.put.call_args.args[0] == f"{_BASE}/tasks/task-1" - assert http_client.put.call_args.kwargs["json"]["intent"] == "Answer." + assert http_client.put.call_args.kwargs["json"]["spec"]["intent"] == "Answer." assert isinstance(result, Task) diff --git a/plugins/nemo-evaluator/tests/test_content_hash.py b/plugins/nemo-evaluator/tests/test_content_hash.py index 6dd082ec48..77311b0638 100644 --- a/plugins/nemo-evaluator/tests/test_content_hash.py +++ b/plugins/nemo-evaluator/tests/test_content_hash.py @@ -14,11 +14,19 @@ import hashlib import json import re -from typing import ClassVar - -from nemo_evaluator.api.schemas import MetadataItem, MetricRef, TaskInputs, TaskRef +from typing import Any, ClassVar + +from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, + HarborTaskDefinition, + MetadataItem, + MetricRef, + TaskInputs, + TaskRef, +) from nemo_evaluator.content_hash import DIGEST_PATTERN, canonical_payload, content_hash from nemo_evaluator.entities import TaskEntity, TasksetEntity +from nemo_evaluator.revisions import head_digest from nemo_evaluator_sdk.agent_eval.tasks import SemanticReducer, SemanticView, ViewSignal from nemo_platform_plugin.entities import EntityBase from pydantic import Field @@ -39,22 +47,40 @@ def _task( project: str | None = None, intent: str = "Answer the question.", inputs: TaskInputs | None = None, + reference: dict[str, Any] | None = None, metrics: list[MetricRef] | None = None, views: dict[str, SemanticView] | None = None, metadata: list[MetadataItem] | None = None, ) -> TaskEntity: return TaskEntity( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent=intent, + inputs=inputs if inputs is not None else TaskInputs(instruction="What is 2+2?"), + reference=reference if reference is not None else {}, + metrics=metrics if metrics is not None else [MetricRef("default/stored-metric")], + views=views if views is not None else _DEFAULT_VIEWS, + ), name=name, workspace=workspace, project=project, - intent=intent, - inputs=inputs if inputs is not None else TaskInputs(instruction="What is 2+2?"), - metrics=metrics if metrics is not None else [MetricRef("default/stored-metric")], - views=views if views is not None else _DEFAULT_VIEWS, metadata=metadata if metadata is not None else _DEFAULT_METADATA, ) +def _harbor_task(*, config: dict[str, Any] | None = None, archive_digest: str = "a" * 64) -> TaskEntity: + return TaskEntity( + spec=HarborTaskDefinition( + kind="harbor", + archive_ref="default/harbor#packages/o-n/abc/dist.tar.gz", + archive_digest=archive_digest, + config=config if config is not None else {}, + ), + name="harbor-1", + workspace="default", + ) + + # --- Shape ------------------------------------------------------------------- @@ -147,6 +173,18 @@ def test_metric_ref_order_changes_digest() -> None: assert content_hash(a) != content_hash(b) +def test_grader_only_reference_changes_digest() -> None: + """``reference`` decides what a metric grades *against*, so it is task content. + + Two revisions that score the same output differently must not share a digest — otherwise + publish-time dedup would collapse them and a pin would no longer fix the grading. This is the + general rule for the digest: it covers anything affecting a task's execution output or the + mechanism used to grade it. + """ + assert content_hash(_task(reference={"expected": "Paris"})) != content_hash(_task()) + assert content_hash(_task(reference={"expected": "Paris"})) != content_hash(_task(reference={"expected": "Lyon"})) + + def test_nested_view_change_changes_digest() -> None: """Nested sub-models participate; a change buried in a view must not be invisible.""" changed = _task( @@ -192,6 +230,35 @@ def test_int_and_float_render_distinctly() -> None: ) +# --- Harbor: the one deliberate exclusion ------------------------------------ + + +def test_harbor_config_does_not_change_digest() -> None: + """``config`` is a *projection* of ``task.toml``, never an execution input. + + Harbor reads the real ``task.toml`` out of the materialized archive at run time, so this copy + affects neither execution nor grading. Hashing it would buy no coverage and would make revision + history sensitive to Harbor's serialization — a release that reordered keys or emitted a new + defaulted field would cut a revision for byte-identical files. + + Exercised through ``head_digest`` rather than ``content_hash``: the exclusion lives in + ``REVISION_POINTER_EXCLUDE``, not in the hashing primitive. + """ + plain = _harbor_task() + configured = _harbor_task(config={"verifier": {"type": "pytest"}, "agent": {"timeout": 600}}) + assert head_digest(plain) == head_digest(configured) + + +def test_harbor_archive_digest_changes_digest() -> None: + """The invariant that makes excluding ``config`` safe. + + ``archive_digest`` is authoritative over every file in the task directory, ``task.toml`` + included — so a config change that genuinely alters execution or grading moves *this* field and + is covered. If this ever stopped holding, excluding ``config`` would become a real gap. + """ + assert head_digest(_harbor_task()) != head_digest(_harbor_task(archive_digest="b" * 64)) + + # --- Tasksets ---------------------------------------------------------------- diff --git a/plugins/nemo-evaluator/tests/test_revision_entity.py b/plugins/nemo-evaluator/tests/test_revision_entity.py index ca1dffb4bf..ed21bb7db5 100644 --- a/plugins/nemo-evaluator/tests/test_revision_entity.py +++ b/plugins/nemo-evaluator/tests/test_revision_entity.py @@ -14,11 +14,11 @@ import re import pytest -from nemo_evaluator.api.schemas import MetadataItem, MetricRef, TaskInputs, TaskRef +from nemo_evaluator.api.schemas import EvaluatorTaskDefinition, MetadataItem, MetricRef, TaskInputs, TaskRef from nemo_evaluator.content_hash import content_hash from nemo_evaluator.entities import ( - REVISION_POINTER_FIELDS, - REVISION_SELF_FIELDS, + REVISION_POINTER_EXCLUDE, + REVISION_SELF_EXCLUDE, TaskEntity, TaskRevisionEntity, TasksetEntity, @@ -41,11 +41,9 @@ def _task_head(*, intent: str = _INTENT, latest_revision: int = 0, tags: dict[str, int] | None = None) -> TaskEntity: return TaskEntity( + spec=EvaluatorTaskDefinition(kind="evaluator", intent=intent, inputs=_INPUTS, metrics=_METRICS), name="task-1", workspace="default", - intent=intent, - inputs=_INPUTS, - metrics=_METRICS, metadata=_ANNOTATIONS, latest_revision=latest_revision, tags=tags or {}, @@ -54,13 +52,11 @@ def _task_head(*, intent: str = _INTENT, latest_revision: int = 0, tags: dict[st def _task_revision(*, intent: str = _INTENT, revision: int = 1, digest: str = _DIGEST) -> TaskRevisionEntity: return TaskRevisionEntity( + spec=EvaluatorTaskDefinition(kind="evaluator", intent=intent, inputs=_INPUTS, metrics=_METRICS), name=f"rev.{revision}", workspace="default", content_hash=digest, revision=revision, - intent=intent, - inputs=_INPUTS, - metrics=_METRICS, metadata=_ANNOTATIONS, ) @@ -89,43 +85,43 @@ def _taskset_revision(*, members: list[TaskRef] | None = None) -> TasksetRevisio def test_task_head_and_revision_digests_agree() -> None: - assert content_hash(_task_head(), exclude=REVISION_POINTER_FIELDS) == content_hash( - _task_revision(), exclude=REVISION_SELF_FIELDS + assert content_hash(_task_head(), exclude=REVISION_POINTER_EXCLUDE) == content_hash( + _task_revision(), exclude=REVISION_SELF_EXCLUDE ) def test_taskset_head_and_revision_digests_agree() -> None: - assert content_hash(_taskset_head(), exclude=REVISION_POINTER_FIELDS) == content_hash( - _taskset_revision(), exclude=REVISION_SELF_FIELDS + assert content_hash(_taskset_head(), exclude=REVISION_POINTER_EXCLUDE) == content_hash( + _taskset_revision(), exclude=REVISION_SELF_EXCLUDE ) def test_moving_a_tag_does_not_change_the_head_digest() -> None: """Tags are pointers, not content. If they were digested, every retag would fork history.""" tagged = _task_head(latest_revision=7, tags={"latest": 7, "candidate": 3}) - assert content_hash(_task_head(), exclude=REVISION_POINTER_FIELDS) == content_hash( - tagged, exclude=REVISION_POINTER_FIELDS + assert content_hash(_task_head(), exclude=REVISION_POINTER_EXCLUDE) == content_hash( + tagged, exclude=REVISION_POINTER_EXCLUDE ) def test_ordinal_does_not_change_the_revision_digest() -> None: """Two revisions of identical content digest identically regardless of when they were cut.""" - assert content_hash(_task_revision(revision=1), exclude=REVISION_SELF_FIELDS) == content_hash( - _task_revision(revision=9), exclude=REVISION_SELF_FIELDS + assert content_hash(_task_revision(revision=1), exclude=REVISION_SELF_EXCLUDE) == content_hash( + _task_revision(revision=9), exclude=REVISION_SELF_EXCLUDE ) def test_content_change_changes_the_revision_digest() -> None: - assert content_hash(_task_revision(), exclude=REVISION_SELF_FIELDS) != content_hash( - _task_revision(intent="Do something else."), exclude=REVISION_SELF_FIELDS + assert content_hash(_task_revision(), exclude=REVISION_SELF_EXCLUDE) != content_hash( + _task_revision(intent="Do something else."), exclude=REVISION_SELF_EXCLUDE ) def test_membership_change_changes_the_taskset_revision_digest() -> None: """A published dataset's identity is its membership — including which revision of each member.""" repinned = _taskset_revision(members=[TaskRef(f"default/task-a#{_OTHER_DIGEST}")]) - assert content_hash(_taskset_revision(), exclude=REVISION_SELF_FIELDS) != content_hash( - repinned, exclude=REVISION_SELF_FIELDS + assert content_hash(_taskset_revision(), exclude=REVISION_SELF_EXCLUDE) != content_hash( + repinned, exclude=REVISION_SELF_EXCLUDE ) diff --git a/plugins/nemo-evaluator/tests/test_revisions.py b/plugins/nemo-evaluator/tests/test_revisions.py index b263389dfe..42d9dcdc7b 100644 --- a/plugins/nemo-evaluator/tests/test_revisions.py +++ b/plugins/nemo-evaluator/tests/test_revisions.py @@ -16,7 +16,7 @@ from typing import TypeVar import pytest -from nemo_evaluator.api.schemas import LATEST_TAG, MetricRef, TaskInputs, TaskRef +from nemo_evaluator.api.schemas import LATEST_TAG, EvaluatorTaskDefinition, MetricRef, TaskInputs, TaskRef from nemo_evaluator.entities import TaskEntity, TaskRevisionEntity from nemo_evaluator.revisions import ( RevisionConflictError, @@ -202,11 +202,14 @@ def concurrent_head_write(self, head: EntityBase, *, tags: dict[str, int]) -> No def _head(store: FakeStore, *, intent: str = "Answer the question.") -> TaskEntity: head = TaskEntity( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent=intent, + inputs=TaskInputs(instruction="What is 2+2?"), + metrics=[MetricRef("default/stored-metric")], + ), name="task-1", workspace="default", - intent=intent, - inputs=TaskInputs(instruction="What is 2+2?"), - metrics=[MetricRef("default/stored-metric")], ) head._id = "head-1" head._db_version = 0 @@ -223,11 +226,14 @@ def _head(store: FakeStore, *, intent: str = "Answer the question.") -> TaskEnti def _head_named(store: FakeStore, name: str) -> TaskEntity: """A second record with content identical to :func:`_head`'s — same digest, different parent.""" head = TaskEntity( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the question.", + inputs=TaskInputs(instruction="What is 2+2?"), + metrics=[MetricRef("default/stored-metric")], + ), name=name, workspace="default", - intent="Answer the question.", - inputs=TaskInputs(instruction="What is 2+2?"), - metrics=[MetricRef("default/stored-metric")], ) head._id = f"head-{name}" head._db_version = 0 @@ -307,7 +313,7 @@ async def test_changed_content_allocates_the_next_ordinal() -> None: store = FakeStore() head = _head(store) first, _ = await _publish(store, head) - head.intent = "Do something else." + head.spec.intent = "Do something else." second, created = await _publish(store, head) assert created assert second.revision == 2 @@ -325,7 +331,7 @@ async def test_contended_ordinal_is_retried() -> None: store = FakeStore() head = _head(store) await _publish(store, head) - head.intent = "Changed." + head.spec.intent = "Changed." store.contend_ordinals = {2} revision, created = await _publish(store, head) assert created @@ -346,7 +352,7 @@ async def test_identical_contended_publish_adopts_the_winners_revision() -> None head = _head(store) await _publish(store, head) - head.intent = "Changed." + head.spec.intent = "Changed." store.contend_identically = {2} revision, created = await _publish(store, head) @@ -372,7 +378,7 @@ async def test_contended_publish_of_different_content_still_allocates_a_new_ordi head = _head(store) await _publish(store, head) - head.intent = "Changed." + head.spec.intent = "Changed." store.contend_ordinals = {2} # winner publishes *different* content revision, created = await _publish(store, head) @@ -400,7 +406,7 @@ async def test_publishing_recovers_when_a_revision_exists_but_the_head_never_adv assert isinstance(stored, TaskEntity) stored.latest_revision, stored.tags = 0, {} - head.intent = "Changed." + head.spec.intent = "Changed." revision, created = await _publish(store, head) assert created @@ -422,7 +428,7 @@ async def test_losing_the_head_race_does_not_leave_the_head_on_an_older_revision a = await store.get(TaskEntity, name="task-1", workspace="default") b = await store.get(TaskEntity, name="task-1", workspace="default") - a.intent, b.intent = "A's content.", "B's content." + a.spec.intent, b.spec.intent = "A's content.", "B's content." async def b_publishes() -> None: await publish_revision(store, store, b, TaskRevisionEntity) @@ -435,13 +441,13 @@ async def b_publishes() -> None: latest = await get_revision(store, TaskRevisionEntity, stored, LATEST_TAG) assert stored.tags[LATEST_TAG] == 3 - assert stored.intent == latest.intent == "B's content." + assert stored.spec.intent == latest.spec.intent == "B's content." assert stored.latest_revision == latest.revision, "the reported revision must describe the content served" # A's publish is not lost — it is a real revision, still resolvable by digest. assert a_revision.revision == 2 pinned = await get_revision(store, TaskRevisionEntity, stored, a_revision.content_hash) - assert pinned.intent == "A's content." + assert pinned.spec.intent == "A's content." @pytest.mark.asyncio @@ -464,7 +470,7 @@ async def test_latest_revision_never_rewinds() -> None: store = FakeStore() head = _head(store) first, _ = await _publish(store, head) - head.intent = "Changed." + head.spec.intent = "Changed." await _publish(store, head) head = await store.get(TaskEntity, "task-1", workspace="default") @@ -534,7 +540,7 @@ async def test_user_tags_may_be_moved_backwards() -> None: store = FakeStore() head = _head(store) older, _ = await _publish(store, head, tags={"blessed"}) - head.intent = "Newer content." + head.spec.intent = "Newer content." await _publish(store, head) head = await store.get(TaskEntity, "task-1", workspace="default") @@ -556,10 +562,10 @@ async def test_reverting_to_earlier_content_publishes_a_new_revision() -> None: store = FakeStore() head = _head(store) await _publish(store, head) # rev.1: "Answer the question." - head.intent = "Changed." + head.spec.intent = "Changed." await _publish(store, head) # rev.2 - head.intent = "Answer the question." # back to rev.1's content + head.spec.intent = "Answer the question." # back to rev.1's content revision, created = await _publish(store, head) assert created, "a revert is a publish, not a no-op" @@ -567,7 +573,7 @@ async def test_reverting_to_earlier_content_publishes_a_new_revision() -> None: assert head.tags[LATEST_TAG] == 3 latest = await get_revision(store, TaskRevisionEntity, head, LATEST_TAG) - assert latest.intent == head.intent, "the head and #latest must describe the same content" + assert latest.spec.intent == head.spec.intent, "the head and #latest must describe the same content" @pytest.mark.asyncio @@ -578,9 +584,9 @@ async def test_a_digest_shared_by_two_revisions_resolves_to_the_newer_one() -> N store = FakeStore() head = _head(store) first, _ = await _publish(store, head) # rev.1 - head.intent = "Changed." + head.spec.intent = "Changed." await _publish(store, head) # rev.2 - head.intent = "Answer the question." + head.spec.intent = "Answer the question." third, _ = await _publish(store, head) # rev.3, same digest as rev.1 assert third.content_hash == first.content_hash @@ -610,7 +616,7 @@ async def test_resolves_latest_by_default() -> None: store = FakeStore() head = _head(store) await _publish(store, head) - head.intent = "Changed." + head.spec.intent = "Changed." second, _ = await _publish(store, head) assert (await get_revision(store, TaskRevisionEntity, head)).content_hash == second.content_hash @@ -620,7 +626,7 @@ async def test_resolves_a_digest_to_its_revision() -> None: store = FakeStore() head = _head(store) first, _ = await _publish(store, head) - head.intent = "Changed." + head.spec.intent = "Changed." await _publish(store, head) resolved = await get_revision(store, TaskRevisionEntity, head, first.content_hash) assert resolved.revision == 1 @@ -702,7 +708,7 @@ async def test_reading_a_revision_whose_content_was_tampered_with_is_refused() - stored = store.records[store._key(TaskRevisionEntity, revision_name(1), "default", head.id)] assert isinstance(stored, TaskRevisionEntity) - stored.intent = "Tampered with after publication." + stored.spec.intent = "Tampered with after publication." with pytest.raises(RevisionContentMismatchError, match="does not match its recorded digest"): await get_revision(store, TaskRevisionEntity, head, LATEST_TAG) @@ -718,7 +724,7 @@ async def test_a_digest_pinned_read_is_verified_too() -> None: stored = store.records[store._key(TaskRevisionEntity, revision_name(1), "default", head.id)] assert isinstance(stored, TaskRevisionEntity) - stored.intent = "Tampered with after publication." + stored.spec.intent = "Tampered with after publication." with pytest.raises(RevisionContentMismatchError): await get_revision(store, TaskRevisionEntity, head, revision.content_hash) diff --git a/plugins/nemo-evaluator/tests/test_skill_examples.py b/plugins/nemo-evaluator/tests/test_skill_examples.py index f59e67bc1b..cbb464d9fe 100644 --- a/plugins/nemo-evaluator/tests/test_skill_examples.py +++ b/plugins/nemo-evaluator/tests/test_skill_examples.py @@ -445,13 +445,20 @@ def test_multiple_metric_platform_submission_uses_cli() -> None: assert "nemo evaluator evaluate submit --spec-file multi-metric.json" in section -def test_resources_show_inline_task_before_held_out_reference_guidance() -> None: +def test_resources_show_a_stored_task_carrying_held_out_reference() -> None: + """Held-out ground truth belongs on a *stored* task, so it survives taskset expansion. + + The skill used to steer users to an inline ``AgentEvalTaskInput`` because the stored spec had no + ``reference`` field. It has one now, and routing them back to inline would cost them tasksets + and revision pinning for no reason. + """ reference = (_repo_root() / "skills/nemo-evaluator-plugin/references/resources.md").read_text(encoding="utf-8") - example_position = reference.index("inline_task = AgentEvalTaskInput(") + example_position = reference.index('"capital-france-graded"') guidance_position = reference.index("Stored tasks keep metric references.") assert example_position < guidance_position assert 'reference={"expected": "Paris"}' in reference + assert "EvaluatorTaskDefinition(" in reference def test_agent_evaluation_shows_how_to_retrieve_stored_trials() -> None: diff --git a/plugins/nemo-evaluator/tests/test_subentity_refs.py b/plugins/nemo-evaluator/tests/test_subentity_refs.py index 5beab88b1d..b4bc02fd05 100644 --- a/plugins/nemo-evaluator/tests/test_subentity_refs.py +++ b/plugins/nemo-evaluator/tests/test_subentity_refs.py @@ -5,21 +5,23 @@ A revision is addressed with the platform's standard ``#`` fragment — the same convention filesets use for a contained file (``workspace/fileset#path``). These tests pin two things: that an absent -fragment means ``latest`` rather than "unpinned", and that existing fragment-unaware callers keep -working against a pinned ref (``parse_entity_ref`` strips it). +fragment means ``latest`` rather than "unpinned", and that a fragment-unaware caller reading a +pinned ref still lands on the right task rather than on one literally named ``task-a#``. """ from __future__ import annotations +import re + import pytest from nemo_evaluator.api.schemas import ( LATEST_TAG, MetricRef, TaskRef, TasksetRef, - parse_entity_ref, parse_subentity_ref, ) +from nemo_platform_plugin.refs import ENTITY_REF_PATTERN, parse_entity_ref from pydantic import ValidationError _DIGEST = "a" * 64 @@ -60,20 +62,37 @@ def test_fragment_is_returned_verbatim() -> None: assert fragment == _DIGEST -# --- Backward compatibility -------------------------------------------------- +# --- Composition with the platform's entity parser --------------------------- -def test_parse_entity_ref_strips_the_fragment() -> None: - """Fragment-unaware callers (metric resolution, taskset member existence checks) keep working - against a pinned ref instead of trying to look up a task literally named 'task-a#'.""" - assert parse_entity_ref(f"other/task-a#{_DIGEST}", "default") == ("other", "task-a") - assert parse_entity_ref("task-a#latest", "default") == ("default", "task-a") +def test_dropping_the_fragment_recovers_the_plain_entity_ref() -> None: + """Fragment-unaware callers (taskset member existence checks) read a pinned ref by discarding the + third element, rather than through a second parser that strips ``#`` itself. Keeping one parser + is what stops evaluator refs and platform refs drifting on what a ``workspace/name`` is.""" + assert parse_subentity_ref(f"other/task-a#{_DIGEST}", "default")[:2] == ("other", "task-a") + assert parse_subentity_ref("task-a#latest", "default")[:2] == ("default", "task-a") def test_pinned_and_bare_refs_resolve_to_the_same_task() -> None: """The property taskset duplicate-detection relies on: two refs differing only by fragment are the same member, and must not both be admitted.""" - assert parse_entity_ref(f"task-a#{_DIGEST}", "default") == parse_entity_ref("task-a", "default") + assert parse_subentity_ref(f"task-a#{_DIGEST}", "default")[:2] == parse_subentity_ref("task-a", "default")[:2] + + +def test_the_base_split_is_the_platform_parser() -> None: + """Not an implementation detail worth pinning for its own sake — it is the guarantee that a + reference means the same thing to the evaluator as it does to every other plugin.""" + parsed = parse_entity_ref("other/task-a", "default") + assert parse_subentity_ref("other/task-a", "default")[:2] == (parsed.workspace, parsed.name) + + +def test_subentity_pattern_is_the_entity_pattern_plus_a_fragment() -> None: + """The evaluator's ref shape is derived from the platform constant, so widening what counts as a + ``workspace/name`` widens both at once instead of leaving one behind.""" + assert TaskRef.model_fields["root"].metadata # the pattern is declared on the field + for bare in ("task-a", "other/task-a"): + assert re.fullmatch(ENTITY_REF_PATTERN, bare) + assert TaskRef(bare).root == bare # --- Field validation -------------------------------------------------------- diff --git a/plugins/nemo-evaluator/tests/test_task_entity.py b/plugins/nemo-evaluator/tests/test_task_entity.py index 86aa76005e..0a88acf5ca 100644 --- a/plugins/nemo-evaluator/tests/test_task_entity.py +++ b/plugins/nemo-evaluator/tests/test_task_entity.py @@ -13,7 +13,7 @@ import json -from nemo_evaluator.api.schemas import MetricRef +from nemo_evaluator.api.schemas import EvaluatorTaskDefinition, MetricRef from nemo_evaluator.entities import TaskEntity from nemo_evaluator_sdk.agent_eval.tasks import SemanticReducer, SemanticView, ViewSignal @@ -22,16 +22,19 @@ def _entity() -> TaskEntity: return TaskEntity( name="task-1", workspace="default", - intent="Answer the question.", - inputs={"instruction": "What is 2+2?"}, - # A persisted task holds metric references only — a workspace-qualified ref and a bare name. - metrics=[MetricRef("default/stored-metric"), MetricRef("derived.abc123")], - views={ - "correctness": SemanticView( - reducer=SemanticReducer.SINGLE, - signals=[ViewSignal(metric="exact-match", output="score")], - ) - }, + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Answer the question.", + inputs={"instruction": "What is 2+2?"}, + # A persisted task holds metric references only — a workspace-qualified ref and a bare name. + metrics=[MetricRef("default/stored-metric"), MetricRef("derived.abc123")], + views={ + "correctness": SemanticView( + reducer=SemanticReducer.SINGLE, + signals=[ViewSignal(metric="exact-match", output="score")], + ) + }, + ), metadata=[{"key": "suite", "value": "smoke"}], ) @@ -47,16 +50,16 @@ def test_roundtrip_preserves_task_fields() -> None: restored = _roundtrip(entity) - assert restored.intent == "Answer the question." - assert restored.inputs.instruction == "What is 2+2?" + assert restored.spec.intent == "Answer the question." + assert restored.spec.inputs.instruction == "What is 2+2?" assert [(m.key, m.value) for m in restored.metadata] == [("suite", "smoke")] # Metric refs survive as RootModel strings. - assert isinstance(restored.metrics[0], MetricRef) - assert restored.metrics[0].root == "default/stored-metric" - assert isinstance(restored.metrics[1], MetricRef) - assert restored.metrics[1].root == "derived.abc123" + assert isinstance(restored.spec.metrics[0], MetricRef) + assert restored.spec.metrics[0].root == "default/stored-metric" + assert isinstance(restored.spec.metrics[1], MetricRef) + assert restored.spec.metrics[1].root == "derived.abc123" # Nested SemanticView survives the JSON column. - assert restored.views == entity.views + assert restored.spec.views == entity.spec.views def test_entity_type_is_task() -> None: diff --git a/plugins/nemo-evaluator/tests/test_task_refs.py b/plugins/nemo-evaluator/tests/test_task_refs.py index 1b794c9749..27fb0a33e2 100644 --- a/plugins/nemo-evaluator/tests/test_task_refs.py +++ b/plugins/nemo-evaluator/tests/test_task_refs.py @@ -9,6 +9,8 @@ import pytest from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, + HarborTaskDefinition, MetadataItem, MetricRef, TaskInputs, @@ -19,7 +21,11 @@ from nemo_evaluator.entities import TaskEntity, TaskRevisionEntity, TasksetEntity, TasksetRevisionEntity from nemo_evaluator.jobs.agent_spec import AgentEvalTaskInput from nemo_evaluator.revisions import apply_tag, get_revision, head_digest, is_digest, publish_revision -from nemo_evaluator.task_refs import resolve_agent_eval_tasks, resolve_taskset_ref +from nemo_evaluator.task_refs import ( + UnsupportedTaskKindError, + resolve_agent_eval_tasks, + resolve_taskset_ref, +) from nemo_platform_plugin.entities import EntityBase from nemo_platform_plugin.entity_client import NemoEntityNotFoundError from pydantic import ValidationError @@ -29,11 +35,14 @@ def _task(name: str, *, workspace: str = "default", metric: str = "default/m") -> TaskEntity: return TaskEntity( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent=f"Do {name}.", + inputs=TaskInputs(instruction=f"instruction for {name}"), + metrics=[MetricRef(metric)], + ), name=name, workspace=workspace, - intent=f"Do {name}.", - inputs=TaskInputs(instruction=f"instruction for {name}"), - metrics=[MetricRef(metric)], metadata=[MetadataItem(key="suite", value="geo")], ) @@ -117,10 +126,46 @@ async def test_resolves_taskset_members_to_inline_task_inputs(entity_store) -> N assert tasks[0].metrics == [MetricRef("default/m")] assert tasks[0].intent == "Do capital-of-france." assert tasks[0].inputs.instruction == "instruction for capital-of-france" - # A stored task carries no grader-only reference. + # A task stored without ground truth expands to an empty reference, not a missing one. assert tasks[0].reference == {} +async def test_grader_only_reference_survives_taskset_expansion(entity_store) -> None: + """Held-out ground truth must not be the privilege of inline submissions. + + Expansion projects a stored task onto the inline DTO field by field, so a field added to the + stored spec and forgotten here silently becomes empty at run time — the agent is then graded + against nothing, and the run still reports a score. That is the failure this guards. + """ + task = _task("fix-bug") + task.spec.reference = {"expected": "Paris", "held_out_tests": ["test_capital.py"]} + client = await _store(entity_store, task, _taskset("geo", ["default/fix-bug"])) + + tasks = await resolve_taskset_ref(TasksetRef("default/geo"), workspace="default", entity_client=client) + + assert tasks[0].reference == {"expected": "Paris", "held_out_tests": ["test_capital.py"]} + + +async def test_expansion_returns_the_pinned_reference_not_the_current_one(entity_store) -> None: + """``reference`` is digest-covered, so republishing it cuts a revision the old pin excludes. + + A pin that honoured new ground truth would silently re-grade a "reproducible" dataset. + """ + task = _task("fix-bug") + task.spec.reference = {"expected": "Paris"} + client = await _store(entity_store, task) + pinned_digest = head_digest(task) + await _create_published(client, _taskset("geo", [f"default/fix-bug#{pinned_digest}"])) + + task.spec.reference = {"expected": "Lyon"} + await client.update(task) + await publish_revision(client, client, task, TaskRevisionEntity) + + tasks = await resolve_taskset_ref(TasksetRef("default/geo"), workspace="default", entity_client=client) + + assert tasks[0].reference == {"expected": "Paris"}, "expansion must return the pinned ground truth" + + async def test_bare_member_ref_resolves_against_taskset_workspace(entity_store) -> None: client = await _store(entity_store, _task("t1", workspace="team"), _taskset("ts", ["t1"], workspace="team")) @@ -191,7 +236,7 @@ async def test_expansion_uses_the_pinned_revision_not_current_content(entity_sto await _create_published(client, _taskset("geo", [f"default/capital-of-france#{pinned_digest}"])) # The member publishes newer content after the taskset was pinned. - task.intent = "Something else entirely." + task.spec.intent = "Something else entirely." await client.update(task) await publish_revision(client, client, task, TaskRevisionEntity) @@ -216,7 +261,7 @@ async def test_a_tag_pinned_member_stores_the_tagged_revision_not_the_head(entit # ``apply_tag`` hands back — tagging bumps the record's version, so the original object is stale. stored_task = await client.get(TaskEntity, name="capital-of-france", workspace="default") tagged = await apply_tag(client, client, TaskRevisionEntity, stored_task, "blessed", "latest") - tagged.intent = "Something else entirely." + tagged.spec.intent = "Something else entirely." await client.update(tagged) await publish_revision(client, client, tagged, TaskRevisionEntity) @@ -322,3 +367,21 @@ async def test_expansion_fails_loudly_when_a_pin_no_longer_resolves(entity_store with pytest.raises(ValueError, match="no longer resolves"): await resolve_taskset_ref(TasksetRef("default/geo"), workspace="default", entity_client=client) + + +async def test_expansion_rejects_a_task_whose_runner_the_target_cannot_run(entity_store) -> None: + """A Harbor task's content is a directory of files, not fields. Projecting it onto an inline + agent-eval task would silently produce a task with no intent and no metrics — an evaluation that + runs and scores nothing. Refused instead, before the run starts.""" + harbor_task = TaskEntity( + name="fix-test", + workspace="default", + spec=HarborTaskDefinition( + kind="harbor", archive_ref="default/harbor#packages/o-n/abc/dist.tar.gz", archive_digest="a" * 64 + ), + ) + client = await _store(entity_store, harbor_task) + await _create_published(client, _taskset("mixed", [f"default/fix-test#{head_digest(harbor_task)}"])) + + with pytest.raises(UnsupportedTaskKindError, match="harbor"): + await resolve_taskset_ref(TasksetRef("default/mixed"), workspace="default", entity_client=client) diff --git a/skills/nemo-evaluator-plugin/assets/examples/plugin_sdk_examples.py b/skills/nemo-evaluator-plugin/assets/examples/plugin_sdk_examples.py index b5e127d60d..62d4e25022 100644 --- a/skills/nemo-evaluator-plugin/assets/examples/plugin_sdk_examples.py +++ b/skills/nemo-evaluator-plugin/assets/examples/plugin_sdk_examples.py @@ -58,6 +58,7 @@ def submit_and_collect(client: Any, output_dir: Path) -> tuple[Any, Path]: def store_resources(client: Any) -> None: """Store one metric, task, and taskset.""" from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, MetricRef, TaskInput, TaskInputs, @@ -72,14 +73,17 @@ def store_resources(client: Any) -> None: client.evaluator.tasks.create( "capital-france", task=TaskInput( - intent="Name the capital of France.", - inputs=TaskInputs(instruction="What is the capital of France?"), - metrics=[MetricRef("default/answer-exact")], + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Name the capital of France.", + inputs=TaskInputs(instruction="What is the capital of France?"), + metrics=[MetricRef("answer-exact")], + ), ), ) client.evaluator.tasksets.create( "geography", - taskset=TasksetInput(tasks=[TaskRef("default/capital-france")]), + taskset=TasksetInput(tasks=[TaskRef("capital-france")]), ) diff --git a/skills/nemo-evaluator-plugin/references/resources.md b/skills/nemo-evaluator-plugin/references/resources.md index 826b02794e..0c3465d978 100644 --- a/skills/nemo-evaluator-plugin/references/resources.md +++ b/skills/nemo-evaluator-plugin/references/resources.md @@ -20,6 +20,7 @@ new versioned name. ```python from nemo_evaluator.api.schemas import ( + EvaluatorTaskDefinition, MetricRef, TaskInput, TaskInputs, @@ -43,9 +44,12 @@ client.evaluator.metrics.create( client.evaluator.tasks.create( "capital-france", task=TaskInput( - intent="Name the capital of France.", - inputs=TaskInputs(instruction="What is the capital of France?"), - metrics=[MetricRef("default/answer-exact")], + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Name the capital of France.", + inputs=TaskInputs(instruction="What is the capital of France?"), + metrics=[MetricRef("answer-exact")], + ), ), ) @@ -53,17 +57,16 @@ client.evaluator.tasksets.create( "geography", taskset=TasksetInput( description="Geography smoke tasks.", - tasks=[TaskRef("default/capital-france")], + tasks=[TaskRef("capital-france")], ), ) ``` -For a task that needs held-out ground truth invisible to the agent, keep the reference on an -inline `AgentEvalTaskInput` and use a metric that reads it: +For a task that needs held-out ground truth invisible to the agent, put it in `reference` and +use a metric that reads it. This works on a stored task, so it survives into taskset-driven runs: ```python -from nemo_evaluator.api.schemas import MetricRef, TaskInputs -from nemo_evaluator.jobs.agent_spec import AgentEvalTaskInput +from nemo_evaluator.api.schemas import EvaluatorTaskDefinition, MetricRef, TaskInput, TaskInputs from nemo_evaluator_sdk import ExactMatchMetric client.evaluator.metrics.create( @@ -74,19 +77,28 @@ client.evaluator.metrics.create( ), ) -inline_task = AgentEvalTaskInput( - id="capital-france", - intent="Name the capital of France.", - inputs=TaskInputs(instruction="What is the capital of France?"), - reference={"expected": "Paris"}, - metrics=[MetricRef("default/answer-from-reference")], +client.evaluator.tasks.create( + "capital-france-graded", + task=TaskInput( + spec=EvaluatorTaskDefinition( + kind="evaluator", + intent="Name the capital of France.", + inputs=TaskInputs(instruction="What is the capital of France?"), + reference={"expected": "Paris"}, + metrics=[MetricRef("answer-from-reference")], + ), + ), ) ``` +`reference` is surfaced to metrics but never seeded into the agent's workspace or shown to the +agent, so a metric can grade against artifacts the agent cannot edit. It is held out from the +*agent*, not from the API — anyone who can read the task can read it. It is covered by the revision +digest, so changing ground truth publishes a new revision. + Stored tasks keep metric references. Inline task metrics are normalized into -content-addressed derived metrics. The stored-task example uses an output-only -metric because stored tasks do not carry the grader-only `reference` field; use -an inline `AgentEvalTaskInput` when held-out per-task data is required. +content-addressed derived metrics. The same `reference` field is available on an inline +`AgentEvalTaskInput` for one-off submissions. ## Retrieve, list, and delete