From 985290574ea7d78f0a67e01a7fffbd1264b7ab89 Mon Sep 17 00:00:00 2001 From: Sandy Chapman Date: Tue, 4 Aug 2026 12:36:11 -0300 Subject: [PATCH 1/4] feat(evaluator): make a stored task runner-polymorphic via kind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A task is an evaluation unit; how it runs is a property of the task, not a different kind of record. The target side already models this — `AgentRunnerTarget` is a `kind`-discriminated union of codex/fabric/harbor — so the stored side now matches, and a user manages every evaluation unit in one place regardless of which runner executes it. Task content moves under a discriminated `spec`: - `EvaluatorTaskDefinition` (kind="evaluator") — intent, inputs, reference, metrics, views - `HarborTaskDefinition` (kind="harbor") — a reference to the task's packaged directory in the Files service, plus Harbor's own config Nested rather than flattened with nullable per-kind fields, so each variant's required fields stay required and the revision digest covers the spec as a unit; two kinds with coincidentally similar metadata cannot collide on content. `kind` is a `Literal`, matching how the runner targets discriminate. The two definitions live in their own modules under `api/task_definitions/`; the shared field types they need moved to `api/fields.py`, since the definitions are imported *by* `schemas` and cannot import back from it. A single model per kind, rather than a stored/input pair: only `metrics` widens on the way in, and the service narrows it to references when storing. That keeps the API surface small at the cost of making the narrowing a service invariant rather than a type-level one. `EvaluatorTaskDefinition` gains the grader-only `reference` — held-out ground truth, surfaced to metrics but never seeded into the agent's workspace. It has existed on the inline `AgentEvalTaskInput` since #566, where persisting it was deferred because the stored schemas then lived in the root OpenAPI/SDK; they are plugin-owned now, so that reason has lapsed. Until this, a taskset-driven run expanded to an empty reference, so any task needing ground truth the agent cannot edit had to give up stored tasks and tasksets entirely. It is covered by the revision digest. The rule: the digest covers anything that affects a task's execution output or how it is graded, and `reference` decides what a metric grades against — two revisions that score differently must not share a digest, or publish-time dedup would collapse them and a pin would stop fixing the grading. Held out from the *agent*, not from the API: anyone who can read the task can read it. Harbor's `config` is the one exclusion, and it does not contradict that rule. It is a projection of `task.toml`; Harbor reads the real file out of the materialized archive at run time, and `archive_digest` is authoritative over every file in that directory. A config change that genuinely alters execution or grading therefore already moves the digest, while hashing the projection would make revision history sensitive to Harbor's serialization. That makes `archive_digest` load-bearing: a Harbor field ever read from the stored record rather than from the archive would have to be digested. Harbor specifics: - One fileset per task, so a task shared by several tasksets is stored once. - `archive_ref` is shape-validated, so a malformed reference is rejected at publish rather than surfacing as a download failure mid-run. - `config` is stored but excluded from the revision digest, as above. - Which agent runs a task is not stored: that comes from the run's target, so the same stored task can be evaluated against different agents. Taskset expansion rejects a `harbor` member rather than projecting it onto an agent-eval DTO: that content is a directory of files, not fields, so a pure projection would silently produce a task with no intent and no metrics — an evaluation that runs and scores nothing. Mixed tasksets stay storable; the mismatch surfaces at submit as a 422. The rejection is unconditional, not target-dependent. Storage landed ahead of the execution bridge, so no target can run a stored `harbor` task yet and the message says so plainly instead of suggesting the reader find a compatible one. Bridging the two — and encoding runner/task-kind compatibility declaratively rather than as an isinstance check here — is AALGO-481. Note for anyone with existing task rows: this is a breaking schema change with no migration. Rows stored in the previous flat shape fail validation on read, which surfaces as a 500 when listing tasks. Clear them before upgrading. Signed-off-by: Sandy Chapman --- docs/evaluator/manage-tasks-tasksets.mdx | 56 ++- plugins/nemo-evaluator/openapi/openapi.yaml | 185 ++++++--- .../src/nemo_evaluator/api/fields.py | 308 +++++++++++++++ .../src/nemo_evaluator/api/schemas.py | 364 +++--------------- .../api/service/task_service.py | 38 +- .../api/task_definitions/evaluator.py | 49 +++ .../api/task_definitions/harbor.py | 58 +++ .../src/nemo_evaluator/content_hash.py | 21 +- .../src/nemo_evaluator/entities.py | 78 ++-- .../src/nemo_evaluator/revisions.py | 9 +- .../src/nemo_evaluator/task_refs.py | 44 ++- .../tests/api/service/test_task_service.py | 246 +++++++++++- .../tests/api/v2/test_tasks_routes.py | 50 ++- .../integration/test_agent_evaluate_job.py | 9 +- .../integration/test_task_derived_metrics.py | 16 +- .../tests/integration/test_task_revisions.py | 105 ++++- .../tests/sdk/test_task_sdk_resources.py | 29 +- .../nemo-evaluator/tests/test_content_hash.py | 79 +++- .../tests/test_revision_entity.py | 38 +- .../nemo-evaluator/tests/test_revisions.py | 54 +-- .../tests/test_skill_examples.py | 11 +- .../nemo-evaluator/tests/test_task_entity.py | 38 +- .../nemo-evaluator/tests/test_task_refs.py | 72 +++- .../assets/examples/plugin_sdk_examples.py | 11 +- .../references/resources.md | 44 ++- 25 files changed, 1427 insertions(+), 585 deletions(-) create mode 100644 plugins/nemo-evaluator/src/nemo_evaluator/api/fields.py create mode 100644 plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/evaluator.py create mode 100644 plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/harbor.py diff --git a/docs/evaluator/manage-tasks-tasksets.mdx b/docs/evaluator/manage-tasks-tasksets.mdx index 32c1be6ce5..d3ac84215e 100644 --- a/docs/evaluator/manage-tasks-tasksets.mdx +++ b/docs/evaluator/manage-tasks-tasksets.mdx @@ -73,34 +73,63 @@ 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( + 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. Both kinds are stored as the +same record type, so a taskset can group them and you manage every evaluation unit in one place. + +`EvaluatorTaskDefinition` (`kind="evaluator"`) — scored by platform metrics: + | 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"`) — a Harbor task, scored by Harbor's own reward: + +| 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 +137,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") @@ -361,9 +390,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/plugins/nemo-evaluator/openapi/openapi.yaml b/plugins/nemo-evaluator/openapi/openapi.yaml index 366017a676..76280188ee 100644 --- a/plugins/nemo-evaluator/openapi/openapi.yaml +++ b/plugins/nemo-evaluator/openapi/openapi.yaml @@ -3461,6 +3461,63 @@ components: title: EvaluateSpec description: Canonical SDK evaluation spec with platform model and metric references resolved. + EvaluatorTaskDefinition: + properties: + kind: + type: string + const: evaluator + title: Kind + default: evaluator + 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: + - 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 +3821,59 @@ 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 + default: harbor + 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: + - 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 +5136,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 +5184,7 @@ components: - id - name - workspace - - intent + - spec - revision - created_at - updated_at @@ -5120,30 +5219,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 +5247,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..09953fb2bd --- /dev/null +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/fields.py @@ -0,0 +1,308 @@ +# 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. + +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 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.") + + +# 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" +# A *fileset* reference: ``workspace/fileset#path/inside.ext``. The fragment is a file path, so +# unlike a revision fragment it admits ``/`` and ``.``. Validated on the field so a malformed +# reference is rejected at publish rather than surfacing as a download failure mid-run — the same +# reason a metric reference is checked when a task is stored. +FILESET_REF_PATTERN = r"^[\w\-.]+/[\w\-.]+#[\w\-./]+$" + + +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 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..b615761de3 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py @@ -5,23 +5,73 @@ 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 ( + FILESET_REF_PATTERN as FILESET_REF_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 ( + MetadataItem as MetadataItem, +) +from nemo_evaluator.api.fields import ( + MetricInline as MetricInline, +) +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_entity_ref as parse_entity_ref, +) +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.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 +102,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 +196,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 +208,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 +233,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 +280,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..11c0fb3902 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,10 +17,12 @@ from nemo_evaluator.api.schemas import ( LATEST_TAG, + HarborTaskDefinition, MetricInline, MetricRef, Revision, Task, + TaskDefinition, TaskInput, parse_entity_ref, ) @@ -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}, @@ -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/task_definitions/evaluator.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/evaluator.py new file mode 100644 index 0000000000..38c18e5285 --- /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"] = "evaluator" + 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..9f64361338 --- /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.api.fields import FILESET_REF_PATTERN +from nemo_evaluator.content_hash import DIGEST_LENGTH, DIGEST_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"] = "harbor" + 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/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..3c6dce14e3 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,36 @@ 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( + intent="Answer the question.", + inputs=TaskInputs(instruction="What is 2+2?"), + metrics=[MetricRef("default/stored-metric")], + ), metadata=[MetadataItem(key="suite", value="smoke")], ) @@ -60,8 +102,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 +115,11 @@ 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( + 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 +127,54 @@ 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( + 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( + 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( + 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 +271,9 @@ async def _boom(entity): entity_store.create = _boom changed = TaskInput( - intent="Rewritten.", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("default/stored-metric")] + spec=EvaluatorTaskDefinition( + 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 +281,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 +351,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 +367,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 +378,131 @@ 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( + 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( + 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( + 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..34a64d43c8 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,9 @@ 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( + intent=intent, inputs=TaskInputs(instruction="What is 2+2?"), metrics=[MetricRef("default/stored-metric")] + ), tags=tags or [], ).model_dump(mode="json") @@ -59,14 +66,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,7 +86,7 @@ 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 @@ -185,7 +192,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 +221,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 +278,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( + 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..db6bf13fd6 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,11 @@ 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( + 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_task_derived_metrics.py b/plugins/nemo-evaluator/tests/integration/test_task_derived_metrics.py index b377e1efe4..bfce1447fc 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( + 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..4405279bd0 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,9 @@ 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(intent=intent, inputs={"instruction": "What is 2+2?"}), tags=tags or [] + ) def _client(base_url: str) -> NeMoPlatform: @@ -71,9 +78,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 +175,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 +226,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 +258,79 @@ 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( + 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..9ed849216f 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,14 @@ def _task_payload(name: str) -> dict[str, Any]: now = datetime.now(timezone.utc) return Task( + spec=EvaluatorTaskDefinition( + 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 +41,11 @@ 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( + intent="Answer.", inputs={"instruction": "x"}, metrics=[MetricRef("default/stored-metric")] + ) + ) def _response(payload: Any) -> MagicMock: @@ -63,7 +75,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 +86,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 +156,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..d08b292abc 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,38 @@ 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( + 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( + 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 +171,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 +228,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..2c44ad4d2e 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(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(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..e1ed80f666 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,11 @@ 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( + 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 +223,13 @@ 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( + 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 +309,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 +327,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 +348,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 +374,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 +402,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 +424,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 +437,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 +466,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 +536,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 +558,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 +569,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 +580,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 +612,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 +622,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 +704,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 +720,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_task_entity.py b/plugins/nemo-evaluator/tests/test_task_entity.py index 86aa76005e..3e5e68decf 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,18 @@ 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( + 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 +49,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..9ceeacd564 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,11 @@ def _task(name: str, *, workspace: str = "default", metric: str = "default/m") -> TaskEntity: return TaskEntity( + spec=EvaluatorTaskDefinition( + 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 +123,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 +233,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 +258,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 +364,19 @@ 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(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..a8ed319f20 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,16 @@ 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( + 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..9f277a2b73 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,11 @@ 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( + intent="Name the capital of France.", + inputs=TaskInputs(instruction="What is the capital of France?"), + metrics=[MetricRef("answer-exact")], + ), ), ) @@ -53,17 +56,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 +76,27 @@ 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( + 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 From fdd6c679ecfe729e0452e00fd39266e4385882e5 Mon Sep 17 00:00:00 2001 From: Sandy Chapman Date: Mon, 10 Aug 2026 13:23:56 -0300 Subject: [PATCH 2/4] fix(evaluator): require the kind discriminator on a task definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TaskDefinition` is discriminated on `kind`, but both variants defaulted it, so the generated schema left `kind` out of `required` while the validator demanded it. A raw create or replace body without `kind` fails with `union_tag_not_found` — meaning a client generated from that spec would omit the field and 422 on every write. Make `kind` a required field on both definitions, matching how the metric payload DTOs in the same package already declare their discriminator, and regenerate the plugin spec. Tests cover both halves of the mismatch: raw POST and PUT bodies without `kind` are rejected, and the published schema keeps `kind` in `required`. Signed-off-by: Sandy Chapman --- docs/evaluator/manage-tasks-tasksets.mdx | 1 + plugins/nemo-evaluator/openapi/openapi.yaml | 6 +++-- .../api/task_definitions/evaluator.py | 2 +- .../api/task_definitions/harbor.py | 2 +- .../tests/api/service/test_task_service.py | 15 ++++++++--- .../tests/api/v2/test_tasks_routes.py | 26 +++++++++++++++++-- .../integration/test_agent_evaluate_job.py | 1 + .../integration/test_task_derived_metrics.py | 2 +- .../tests/integration/test_task_revisions.py | 4 ++- .../tests/sdk/test_task_sdk_resources.py | 6 ++++- .../nemo-evaluator/tests/test_content_hash.py | 2 ++ .../tests/test_revision_entity.py | 4 +-- .../nemo-evaluator/tests/test_revisions.py | 6 ++++- .../nemo-evaluator/tests/test_task_entity.py | 1 + .../nemo-evaluator/tests/test_task_refs.py | 9 +++++-- .../assets/examples/plugin_sdk_examples.py | 1 + .../references/resources.md | 2 ++ 17 files changed, 73 insertions(+), 17 deletions(-) diff --git a/docs/evaluator/manage-tasks-tasksets.mdx b/docs/evaluator/manage-tasks-tasksets.mdx index d3ac84215e..fec7852116 100644 --- a/docs/evaluator/manage-tasks-tasksets.mdx +++ b/docs/evaluator/manage-tasks-tasksets.mdx @@ -77,6 +77,7 @@ from nemo_evaluator.api.schemas import EvaluatorTaskDefinition, MetadataItem, Me 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("default/answer-exact-match")], diff --git a/plugins/nemo-evaluator/openapi/openapi.yaml b/plugins/nemo-evaluator/openapi/openapi.yaml index 76280188ee..eb4a8a2c21 100644 --- a/plugins/nemo-evaluator/openapi/openapi.yaml +++ b/plugins/nemo-evaluator/openapi/openapi.yaml @@ -3467,7 +3467,7 @@ components: type: string const: evaluator title: Kind - default: evaluator + description: Task kind discriminator. intent: type: string title: Intent @@ -3505,6 +3505,7 @@ components: additionalProperties: false type: object required: + - kind - intent title: EvaluatorTaskDefinition description: "What the agent should do, and how the platform scores it.\n\n\ @@ -3827,7 +3828,7 @@ components: type: string const: harbor title: Kind - default: harbor + description: Task kind discriminator. archive_ref: type: string pattern: ^[\w\-.]+/[\w\-.]+#[\w\-./]+$ @@ -3859,6 +3860,7 @@ components: additionalProperties: false type: object required: + - kind - archive_ref - archive_digest title: HarborTaskDefinition 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 index 38c18e5285..584231cc64 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/evaluator.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/evaluator.py @@ -28,7 +28,7 @@ class EvaluatorTaskDefinition(BaseModel): model_config = ConfigDict(extra="forbid") - kind: Literal["evaluator"] = "evaluator" + 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( 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 index 9f64361338..49aa154de3 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/harbor.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/harbor.py @@ -29,7 +29,7 @@ class HarborTaskDefinition(BaseModel): model_config = ConfigDict(extra="forbid") - kind: Literal["harbor"] = "harbor" + 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).", 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 3c6dce14e3..b8cfa3ebcf 100644 --- a/plugins/nemo-evaluator/tests/api/service/test_task_service.py +++ b/plugins/nemo-evaluator/tests/api/service/test_task_service.py @@ -78,6 +78,7 @@ def _ref(metric: MetricRef | MetricInline) -> MetricRef: def _task_input() -> TaskInput: return TaskInput( spec=EvaluatorTaskDefinition( + kind="evaluator", intent="Answer the question.", inputs=TaskInputs(instruction="What is 2+2?"), metrics=[MetricRef("default/stored-metric")], @@ -116,6 +117,7 @@ async def test_create_normalizes_inline_metrics_to_refs( inline = _inline_metric() task_input = TaskInput( spec=EvaluatorTaskDefinition( + kind="evaluator", intent="Answer the question.", inputs=TaskInputs(instruction="What is 2+2?"), metrics=[MetricRef("default/stored-metric"), inline], @@ -142,6 +144,7 @@ async def test_create_preserves_grader_only_reference(service: TaskService) -> N 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, @@ -159,7 +162,7 @@ async def test_create_preserves_grader_only_reference(service: TaskService) -> N async def test_create_rejects_missing_metric_ref(service: TaskService) -> None: task_input = TaskInput( spec=EvaluatorTaskDefinition( - intent="x", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("default/nope")] + kind="evaluator", intent="x", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("default/nope")] ) ) with pytest.raises(MetricRefNotFoundError, match="not found"): @@ -170,7 +173,7 @@ async def test_create_canonicalizes_bare_metric_ref(service: TaskService) -> Non # A bare "stored-metric" ref resolves against the task workspace and is persisted as "default/stored-metric". task_input = TaskInput( spec=EvaluatorTaskDefinition( - intent="x", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("stored-metric")] + kind="evaluator", intent="x", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("stored-metric")] ) ) created, _ = await service.create_task("task-1", task_input, workspace="default") @@ -272,7 +275,10 @@ async def _boom(entity): changed = TaskInput( spec=EvaluatorTaskDefinition( - intent="Rewritten.", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("default/stored-metric")] + kind="evaluator", + intent="Rewritten.", + inputs=TaskInputs(instruction="?"), + metrics=[MetricRef("default/stored-metric")], ) ) with pytest.raises(RuntimeError): @@ -386,6 +392,7 @@ async def test_resolve_revision_raises_for_a_missing_task(service: TaskService) 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.", @@ -449,6 +456,7 @@ async def test_harbor_config_is_stored_but_not_hashed(service: TaskService) -> N 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.", @@ -483,6 +491,7 @@ async def test_reference_only_change_publishes_a_revision(service: TaskService) 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}, 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 34a64d43c8..c7b3861471 100644 --- a/plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py +++ b/plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py @@ -49,7 +49,10 @@ def client(entity_store) -> TestClient: def _body(*, intent: str = "Answer the question.", tags: list[str] | None = None) -> dict: return TaskInput( spec=EvaluatorTaskDefinition( - intent=intent, inputs=TaskInputs(instruction="What is 2+2?"), metrics=[MetricRef("default/stored-metric")] + kind="evaluator", + intent=intent, + inputs=TaskInputs(instruction="What is 2+2?"), + metrics=[MetricRef("default/stored-metric")], ), tags=tags or [], ).model_dump(mode="json") @@ -90,6 +93,25 @@ def test_create_missing_metric_ref_returns_422(client: TestClient) -> None: 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 @@ -287,7 +309,7 @@ def test_list_includes_harbor_tasks(client: TestClient) -> None: f"{_BASE}/harbor-task", json=TaskInput( spec=HarborTaskDefinition( - archive_ref="default/harbor#packages/o-n/abc/dist.tar.gz", archive_digest="a" * 64 + kind="harbor", archive_ref="default/harbor#packages/o-n/abc/dist.tar.gz", archive_digest="a" * 64 ) ).model_dump(mode="json"), ) 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 db6bf13fd6..05156539d7 100644 --- a/plugins/nemo-evaluator/tests/integration/test_agent_evaluate_job.py +++ b/plugins/nemo-evaluator/tests/integration/test_agent_evaluate_job.py @@ -556,6 +556,7 @@ def test_submit_over_taskset_ref_resolves_and_scores(subprocess_platform: str) - name, task=TaskInput( 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}")], 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 bfce1447fc..8b9d8106e9 100644 --- a/plugins/nemo-evaluator/tests/integration/test_task_derived_metrics.py +++ b/plugins/nemo-evaluator/tests/integration/test_task_derived_metrics.py @@ -57,7 +57,7 @@ def _inline_metric(marker: str) -> MetricInline: def _task_input(metric: MetricInline) -> TaskInput: return TaskInput( spec=EvaluatorTaskDefinition( - intent="Answer the question.", inputs={"instruction": "What is 2+2?"}, metrics=[metric] + kind="evaluator", intent="Answer the question.", inputs={"instruction": "What is 2+2?"}, metrics=[metric] ) ) diff --git a/plugins/nemo-evaluator/tests/integration/test_task_revisions.py b/plugins/nemo-evaluator/tests/integration/test_task_revisions.py index 4405279bd0..bbe7f34d9c 100644 --- a/plugins/nemo-evaluator/tests/integration/test_task_revisions.py +++ b/plugins/nemo-evaluator/tests/integration/test_task_revisions.py @@ -52,7 +52,8 @@ def _unique(prefix: str) -> str: def _task_input(intent: str = "Answer the question.", *, tags: list[str] | None = None) -> TaskInput: return TaskInput( - spec=EvaluatorTaskDefinition(intent=intent, inputs={"instruction": "What is 2+2?"}), tags=tags or [] + spec=EvaluatorTaskDefinition(kind="evaluator", intent=intent, inputs={"instruction": "What is 2+2?"}), + tags=tags or [], ) @@ -266,6 +267,7 @@ def test_republishing_a_taskset_after_a_member_moves_cuts_a_revision(subprocess_ 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.", 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 9ed849216f..e9e80fdc0e 100644 --- a/plugins/nemo-evaluator/tests/sdk/test_task_sdk_resources.py +++ b/plugins/nemo-evaluator/tests/sdk/test_task_sdk_resources.py @@ -26,6 +26,7 @@ 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")], @@ -43,7 +44,10 @@ def _task_payload(name: str) -> dict[str, Any]: def _task_input() -> TaskInput: return TaskInput( spec=EvaluatorTaskDefinition( - intent="Answer.", inputs={"instruction": "x"}, metrics=[MetricRef("default/stored-metric")] + kind="evaluator", + intent="Answer.", + inputs={"instruction": "x"}, + metrics=[MetricRef("default/stored-metric")], ) ) diff --git a/plugins/nemo-evaluator/tests/test_content_hash.py b/plugins/nemo-evaluator/tests/test_content_hash.py index d08b292abc..77311b0638 100644 --- a/plugins/nemo-evaluator/tests/test_content_hash.py +++ b/plugins/nemo-evaluator/tests/test_content_hash.py @@ -54,6 +54,7 @@ def _task( ) -> 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 {}, @@ -70,6 +71,7 @@ def _task( 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 {}, diff --git a/plugins/nemo-evaluator/tests/test_revision_entity.py b/plugins/nemo-evaluator/tests/test_revision_entity.py index 2c44ad4d2e..ed21bb7db5 100644 --- a/plugins/nemo-evaluator/tests/test_revision_entity.py +++ b/plugins/nemo-evaluator/tests/test_revision_entity.py @@ -41,7 +41,7 @@ def _task_head(*, intent: str = _INTENT, latest_revision: int = 0, tags: dict[str, int] | None = None) -> TaskEntity: return TaskEntity( - spec=EvaluatorTaskDefinition(intent=intent, inputs=_INPUTS, metrics=_METRICS), + spec=EvaluatorTaskDefinition(kind="evaluator", intent=intent, inputs=_INPUTS, metrics=_METRICS), name="task-1", workspace="default", metadata=_ANNOTATIONS, @@ -52,7 +52,7 @@ 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(intent=intent, inputs=_INPUTS, metrics=_METRICS), + spec=EvaluatorTaskDefinition(kind="evaluator", intent=intent, inputs=_INPUTS, metrics=_METRICS), name=f"rev.{revision}", workspace="default", content_hash=digest, diff --git a/plugins/nemo-evaluator/tests/test_revisions.py b/plugins/nemo-evaluator/tests/test_revisions.py index e1ed80f666..42d9dcdc7b 100644 --- a/plugins/nemo-evaluator/tests/test_revisions.py +++ b/plugins/nemo-evaluator/tests/test_revisions.py @@ -203,7 +203,10 @@ 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( - intent=intent, inputs=TaskInputs(instruction="What is 2+2?"), metrics=[MetricRef("default/stored-metric")] + kind="evaluator", + intent=intent, + inputs=TaskInputs(instruction="What is 2+2?"), + metrics=[MetricRef("default/stored-metric")], ), name="task-1", workspace="default", @@ -224,6 +227,7 @@ 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")], diff --git a/plugins/nemo-evaluator/tests/test_task_entity.py b/plugins/nemo-evaluator/tests/test_task_entity.py index 3e5e68decf..0a88acf5ca 100644 --- a/plugins/nemo-evaluator/tests/test_task_entity.py +++ b/plugins/nemo-evaluator/tests/test_task_entity.py @@ -23,6 +23,7 @@ def _entity() -> TaskEntity: name="task-1", workspace="default", 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. diff --git a/plugins/nemo-evaluator/tests/test_task_refs.py b/plugins/nemo-evaluator/tests/test_task_refs.py index 9ceeacd564..27fb0a33e2 100644 --- a/plugins/nemo-evaluator/tests/test_task_refs.py +++ b/plugins/nemo-evaluator/tests/test_task_refs.py @@ -36,7 +36,10 @@ def _task(name: str, *, workspace: str = "default", metric: str = "default/m") -> TaskEntity: return TaskEntity( spec=EvaluatorTaskDefinition( - intent=f"Do {name}.", inputs=TaskInputs(instruction=f"instruction for {name}"), metrics=[MetricRef(metric)] + kind="evaluator", + intent=f"Do {name}.", + inputs=TaskInputs(instruction=f"instruction for {name}"), + metrics=[MetricRef(metric)], ), name=name, workspace=workspace, @@ -373,7 +376,9 @@ async def test_expansion_rejects_a_task_whose_runner_the_target_cannot_run(entit harbor_task = TaskEntity( name="fix-test", workspace="default", - spec=HarborTaskDefinition(archive_ref="default/harbor#packages/o-n/abc/dist.tar.gz", archive_digest="a" * 64), + 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)}"])) 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 a8ed319f20..62d4e25022 100644 --- a/skills/nemo-evaluator-plugin/assets/examples/plugin_sdk_examples.py +++ b/skills/nemo-evaluator-plugin/assets/examples/plugin_sdk_examples.py @@ -74,6 +74,7 @@ def store_resources(client: Any) -> None: "capital-france", task=TaskInput( spec=EvaluatorTaskDefinition( + kind="evaluator", intent="Name the capital of France.", inputs=TaskInputs(instruction="What is the capital of France?"), metrics=[MetricRef("answer-exact")], diff --git a/skills/nemo-evaluator-plugin/references/resources.md b/skills/nemo-evaluator-plugin/references/resources.md index 9f277a2b73..0c3465d978 100644 --- a/skills/nemo-evaluator-plugin/references/resources.md +++ b/skills/nemo-evaluator-plugin/references/resources.md @@ -45,6 +45,7 @@ client.evaluator.tasks.create( "capital-france", task=TaskInput( spec=EvaluatorTaskDefinition( + kind="evaluator", intent="Name the capital of France.", inputs=TaskInputs(instruction="What is the capital of France?"), metrics=[MetricRef("answer-exact")], @@ -80,6 +81,7 @@ 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"}, From 44ddaa2c3c2d8f5ede83ef418e225e332d2da99b Mon Sep 17 00:00:00 2001 From: Sandy Chapman Date: Tue, 11 Aug 2026 09:11:51 -0300 Subject: [PATCH 3/4] refactor(evaluator): adopt the platform's entity-ref parser and fix the task docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on #1071. Reference parsing was duplicated. `nmp.common.entities.utils` already re-exports `nemo_platform_plugin.refs.parse_entity_ref`, which ~10 services and three other plugins use; the evaluator was the last place carrying its own copy under the same name. Delete it and delegate: `parse_subentity_ref` now adds only the `#fragment` that a revisioned entity needs on top of the shared split, and `ENTITY_REF_PATTERN` / `FILESET_REF_PATTERN` move next to the parser and the `FilesetRef` type they describe. `_SUBENTITY_REF_PATTERN` is spliced from the shared constant, so widening what counts as a `workspace/name` widens both shapes at once instead of leaving one behind. One behavior detail this makes explicit: taskset duplicate-detection relied on the old parser silently stripping `#fragment`, so `task-a` and `task-a#` deduped as one member. The platform parser does not strip, so that path now discards the fragment deliberately. Restore the `CloudpickleMetricPayload` / `InlineMetricPayload` / `MetricPayload` re-exports from `api.schemas`, which `fields.py` promises in its module docstring and lost when they moved. The `manage-tasks-tasksets` revision snippets still passed the pre-`spec` flat shape. `make docs-check-python-snippets` did not catch it because the snippet linter passes ty a rule name that was renamed upstream, so ty answered with `warning[unknown-rule]` and the check failed for every doc regardless of its content. Fix the rule name, fix the snippets, and name both task kinds before the sentence that refers to "both kinds". Type-checking a snippet would not have caught one that type-checks and then fails at run time, nor a documented output gone stale — which is the shape of what review found here. So add an integration test that walks the doc top to bottom against a real platform and asserts the results it claims. No OpenAPI change: the spec regenerates byte-identical. Co-Authored-By: Claude Opus 5 Signed-off-by: Sandy Chapman --- docs/_scripts/lint_python_snippets.py | 4 +- docs/evaluator/manage-tasks-tasksets.mdx | 24 +- .../src/nemo_platform_plugin/refs.py | 18 ++ .../src/nemo_evaluator/api/fields.py | 61 +++--- .../src/nemo_evaluator/api/schemas.py | 18 +- .../api/service/task_service.py | 4 +- .../api/service/taskset_service.py | 7 +- .../api/task_definitions/harbor.py | 2 +- .../src/nemo_evaluator/metric_refs.py | 12 +- .../test_docs_manage_tasks_tasksets.py | 205 ++++++++++++++++++ .../tests/test_subentity_refs.py | 39 +++- 11 files changed, 324 insertions(+), 70 deletions(-) create mode 100644 plugins/nemo-evaluator/tests/integration/test_docs_manage_tasks_tasksets.py 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 fec7852116..a3f20e4c90 100644 --- a/docs/evaluator/manage-tasks-tasksets.mdx +++ b/docs/evaluator/manage-tasks-tasksets.mdx @@ -99,10 +99,15 @@ print(stored.id, stored.spec.metrics) ### Task kinds -A task is an evaluation unit; its `kind` says which runner executes it. Both kinds are stored as the -same record type, so a taskset can group them and you manage every evaluation unit in one place. +A task is an evaluation unit; its `kind` says which runner executes it. There are two: -`EvaluatorTaskDefinition` (`kind="evaluator"`) — scored by platform metrics: +- `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 | |-------|------|----------|-------------| @@ -112,7 +117,7 @@ same record type, so a taskset can group them and you manage every evaluation un | `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. | -`HarborTaskDefinition` (`kind="harbor"`) — a Harbor task, scored by Harbor's own reward: +`HarborTaskDefinition` (`kind="harbor"`): | Field | Type | Required | Description | |-------|------|----------|-------------| @@ -161,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")], ) @@ -199,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 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/src/nemo_evaluator/api/fields.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/fields.py index 09953fb2bd..2e682c9baa 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/fields.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/fields.py @@ -7,6 +7,12 @@ 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. """ @@ -21,6 +27,7 @@ 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 @@ -107,71 +114,53 @@ class MetricInline(BaseModel): 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. +# 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 +# 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 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" -# A *fileset* reference: ``workspace/fileset#path/inside.ext``. The fragment is a file path, so -# unlike a revision fragment it admits ``/`` and ``.``. Validated on the field so a malformed -# reference is rejected at publish rather than surfacing as a download failure mid-run — the same -# reason a metric reference is checked when a task is stored. -FILESET_REF_PATTERN = r"^[\w\-.]+/[\w\-.]+#[\w\-./]+$" - - -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)``. + 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) - workspace, name = parse_entity_ref(base, default_workspace) - return workspace, name, fragment if separator and fragment else LATEST_TAG + 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, + pattern=ENTITY_REF_PATTERN, description="Reference to a stored metric (format: workspace/metric-name, or metric-name in the job workspace).", ) diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py index b615761de3..b6b3c5017f 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py @@ -9,9 +9,6 @@ from enum import StrEnum from typing import Annotated, TypeAlias -from nemo_evaluator.api.fields import ( - FILESET_REF_PATTERN as FILESET_REF_PATTERN, -) from nemo_evaluator.api.fields import ( LATEST_TAG as LATEST_TAG, ) @@ -21,12 +18,21 @@ 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, ) @@ -51,9 +57,6 @@ from nemo_evaluator.api.fields import ( TasksetRef as TasksetRef, ) -from nemo_evaluator.api.fields import ( - parse_entity_ref as parse_entity_ref, -) from nemo_evaluator.api.fields import ( parse_subentity_ref as parse_subentity_ref, ) @@ -66,6 +69,9 @@ 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 BaseModel, ConfigDict, Field 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 11c0fb3902..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 @@ -24,9 +24,9 @@ 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, @@ -154,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. " 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/harbor.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/harbor.py index 49aa154de3..6bc71e7144 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/harbor.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/task_definitions/harbor.py @@ -7,8 +7,8 @@ from typing import Any, Literal -from nemo_evaluator.api.fields import FILESET_REF_PATTERN 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 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/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/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 -------------------------------------------------------- From 574ae1211038d60943e267ea5a82a8a4f1cd8453 Mon Sep 17 00:00:00 2001 From: Sandy Chapman Date: Tue, 11 Aug 2026 12:09:11 -0300 Subject: [PATCH 4/4] feat(harbor-nemo): add a NeMo registry backend for Harbor Harbor publishes to and downloads from the public Hub. This implements Harbor's pluggable registry-backend interface against NeMo, so export HARBOR_REGISTRY_BACKEND=nemo harbor publish ./my-task harbor run -d nvidia/my-dataset use NeMo instead, with no change to Harbor. Registration is the `harbor.registry_backends` entry point, which is what makes the stock CLI work. A Harbor task becomes a stored task with `kind="harbor"`, its archive a file in a fileset; a Harbor dataset becomes a taskset. The org is folded into the entity name (`nvidia/my-task` -> `nvidia.my-task`) rather than mapped to a workspace: a workspace is a tenancy boundary with its own lifecycle and authorization, while a Harbor org is a cheap namespace that `publish` creates on demand, and mapping one to the other would make publishing a tenancy operation. Two digests are in play and are not interchangeable. NeMo addresses a revision by a digest of the stored spec; Harbor addresses a version by a digest of the task directory's files, kept in `spec.archive_digest`. A `sha256:` reference arriving from Harbor is always the latter and is not a valid revision selector, so a content-pinned lookup scans revisions rather than fetching directly. Two adjacent traps are handled with it: a revision *ordinal* is read by the platform as a tag name, and NeMo digests are bare hex, so Harbor's `sha256:` prefix must be stripped before any digest is used as a selector. Publishing a dataset translates between the two spaces so a manifest's pins survive. 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 rather than what the manifest named. `_create_archive`, `remote_path` and `publish_tasks` are inherited untouched, so a package published here is byte-identical to the same package on the Hub and their content hashes agree. Dataset-level files currently ride in taskset `metadata` as JSON. The taskset `files` field that replaces this is a separate change; this package moves over once it lands. This depends on Harbor's registry-backend abstraction, which is not upstream yet, so it is not usable as it stands and is not wired into the uv workspace. Signed-off-by: Sandy Chapman --- packages/harbor_nemo/README.md | 93 ++++ packages/harbor_nemo/pyproject.toml | 41 ++ .../harbor_nemo/src/harbor_nemo/__init__.py | 34 ++ .../harbor_nemo/src/harbor_nemo/backend.py | 93 ++++ .../harbor_nemo/src/harbor_nemo/client.py | 127 ++++++ .../harbor_nemo/src/harbor_nemo/config.py | 57 +++ .../src/harbor_nemo/dataset_client.py | 203 +++++++++ packages/harbor_nemo/src/harbor_nemo/names.py | 84 ++++ .../harbor_nemo/src/harbor_nemo/publisher.py | 416 ++++++++++++++++++ .../harbor_nemo/src/harbor_nemo/storage.py | 108 +++++ .../src/harbor_nemo/task_resolver.py | 187 ++++++++ packages/harbor_nemo/tests/conftest.py | 55 +++ .../harbor_nemo/tests/test_dataset_pinning.py | 162 +++++++ .../tests/test_error_translation.py | 70 +++ packages/harbor_nemo/tests/test_names.py | 49 +++ packages/harbor_nemo/tests/test_publisher.py | 213 +++++++++ packages/harbor_nemo/tests/test_storage.py | 93 ++++ .../harbor_nemo/tests/test_task_resolver.py | 153 +++++++ 18 files changed, 2238 insertions(+) create mode 100644 packages/harbor_nemo/README.md create mode 100644 packages/harbor_nemo/pyproject.toml create mode 100644 packages/harbor_nemo/src/harbor_nemo/__init__.py create mode 100644 packages/harbor_nemo/src/harbor_nemo/backend.py create mode 100644 packages/harbor_nemo/src/harbor_nemo/client.py create mode 100644 packages/harbor_nemo/src/harbor_nemo/config.py create mode 100644 packages/harbor_nemo/src/harbor_nemo/dataset_client.py create mode 100644 packages/harbor_nemo/src/harbor_nemo/names.py create mode 100644 packages/harbor_nemo/src/harbor_nemo/publisher.py create mode 100644 packages/harbor_nemo/src/harbor_nemo/storage.py create mode 100644 packages/harbor_nemo/src/harbor_nemo/task_resolver.py create mode 100644 packages/harbor_nemo/tests/conftest.py create mode 100644 packages/harbor_nemo/tests/test_dataset_pinning.py create mode 100644 packages/harbor_nemo/tests/test_error_translation.py create mode 100644 packages/harbor_nemo/tests/test_names.py create mode 100644 packages/harbor_nemo/tests/test_publisher.py create mode 100644 packages/harbor_nemo/tests/test_storage.py create mode 100644 packages/harbor_nemo/tests/test_task_resolver.py 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