diff --git a/docs/evaluator/manage-tasks-tasksets.mdx b/docs/evaluator/manage-tasks-tasksets.mdx index 0a3a76ec6b..a8ca305897 100644 --- a/docs/evaluator/manage-tasks-tasksets.mdx +++ b/docs/evaluator/manage-tasks-tasksets.mdx @@ -18,15 +18,21 @@ metrics inline every time. | Concept | What it is | Members | |---------|------------|---------| | **Task** | A reusable agent-eval unit: `intent`, `inputs`, and the `metrics` that score it. | References the metrics that score it. | -| **Taskset** | A flexible grouping of tasks with a description and metadata. | References member tasks by `workspace/name`. Membership is a **set** — order is not significant and duplicate references are rejected. | +| **Taskset** | A flexible grouping of tasks with a description and metadata. | References member tasks by `workspace/name`, each pinned to an exact revision. Membership is a **set** — order is not significant and duplicate references are rejected. | +| **Revision** | An immutable published snapshot of a task's or taskset's content, addressed by a content digest. | Belongs to the task or taskset it snapshots. | Both are addressed by `workspace/name`. Names are unique within a workspace, limited to 255 characters, and must match `^[\w\-\.]+$`. +Every stored task and taskset is **versioned**. Creating one publishes revision 1; replacing its +content publishes the next revision. Earlier revisions stay readable for as long as the task or +taskset exists — deleting it removes its revisions with it — which is what lets an evaluation be +re-run against exactly the content it ran against the first time. + -Tasks and tasksets support **create, retrieve, list, and delete** — there is no update. To change a -stored task or taskset, delete it and create a new one, or store a new version under a different -name. +Publishing is **idempotent**. Replacing a task with content identical to its current revision +publishes nothing and returns the existing revision — so a pipeline can re-submit the same +definition freely without accumulating versions. ## Initialize the SDK @@ -89,6 +95,7 @@ print(stored.id, stored.metrics) | `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. | A stored task holds **metric references only**. Any inline metric bundle you pass on create is stored @@ -99,26 +106,107 @@ why `stored.metrics` always comes back as a list of `MetricRef` references. ### Retrieve, list, and delete ```python -# Retrieve one task by name +# Retrieve one task by name (its current content) task = tasks.retrieve("capital-of-france") +print(task.revision, task.tags) # e.g. 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) -# Delete a task +# Delete a task (this also removes all of its revisions) tasks.delete("capital-of-france") ``` `sort` accepts `name`, `created_at`, or `updated_at`, each optionally prefixed with `-` for descending order. +## Revisions + +### Publish a new revision + +Use `replace` to publish new content. It creates the task if it does not exist, so a publisher needs +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")], + metadata=[MetadataItem(key="suite", value="geography")], +) + +updated = tasks.replace("capital-of-france", task=revised_task) +print(updated.revision) # 2 +``` + +Submitting content identical to the current revision publishes nothing and returns the existing +revision — but any tags in the body are still applied, which is how you tag a revision after the +fact. + +### List revisions + +Each entry carries the `content_hash` used to pin a reference. + +```python +page = tasks.list_revisions("capital-of-france") +for revision in page.data: # newest first + print(revision.revision, revision.content_hash, revision.tags) +``` + +### Read a specific revision + +Pass a content digest or a tag. This returns the content **as published**, not the current content. + +```python +# Revisions come back newest-first and paginated, so index by ordinal rather than by position — +# `data[-1]` is only the oldest entry on the page you happen to have fetched. +page = tasks.list_revisions("capital-of-france") +digest = next(revision.content_hash for revision in page.data if revision.revision == 1) + +original = tasks.retrieve("capital-of-france", revision=digest) # revision 1, as published +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 +``` + +### Tag a revision + +A tag is a mutable pointer to a revision — useful for marking one as reviewed or approved after it +has been evaluated. Read it back with `tag=`, the counterpart to `revision=`: + +```python +tasks.tag("capital-of-france", tag="blessed", revision=digest) + +blessed = tasks.retrieve("capital-of-france", tag="blessed") +``` + +`revision=` takes a content digest and `tag=` takes a tag name. They select the same thing two ways, +so pass one or the other — passing both raises `ValueError`. + + +A tag names exactly one revision. Re-tagging moves the pointer rather than adding a second one, so +`retrieve(tag=...)` always resolves to a single revision — there is no way for two revisions to +share a tag. + +`latest` is managed automatically and always names the most recently published revision; it cannot be +moved by hand. A tag name may not be empty, and may not look like a content digest (64 hexadecimal +characters) — such a tag could be stored but never resolved, because a digest-shaped reference is +looked up as a digest rather than as a tag. + + ## Manage Tasksets A taskset references existing tasks by `workspace/name`. All referenced tasks must already exist when the taskset is created; a missing or duplicate reference is rejected. +Member references are **resolved to an exact revision when the taskset is stored**. You may submit a +bare name (`capital-of-france`), a tag (`capital-of-france#latest`), or a digest — what gets stored +is always `workspace/name#`. This is why a stored taskset keeps naming the same content even +after a member task publishes something new, and it is what makes a suite reproducible. + ```python from nemo_evaluator.api.schemas import TaskRef, TasksetInput @@ -132,6 +220,7 @@ taskset = TasksetInput( stored = tasksets.create("geography-suite", taskset=taskset) print(stored.tasks) +# ['default/capital-of-france#a1b2...', 'default/capital-of-japan#c3d4...'] ``` ### `TasksetInput` fields @@ -139,8 +228,9 @@ print(stored.tasks) | Field | Type | Required | Description | |-------|------|----------|-------------| | `description` | `str` | No | Human-readable description of the grouping. | -| `tasks` | `list[TaskRef]` | No | References to member tasks (`workspace/name`, or bare `name` within the same workspace). Set semantics — duplicates rejected. | +| `tasks` | `list[TaskRef]` | No | References to member tasks (`workspace/name`, or bare `name` within the same workspace), optionally pinned with `#`. Each is resolved to an exact digest when stored. Set semantics — duplicates rejected. | | `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. | ### Retrieve, list, and delete @@ -154,6 +244,26 @@ for item in page.data: tasksets.delete("geography-suite") ``` +Tasksets carry the same revision surface as tasks — `replace`, `list_revisions`, `tag`, and +`retrieve(revision=...)` / `retrieve(tag=...)`: + +```python +# Re-resolving membership after a member task published new content cuts a new revision. +tasksets.replace("geography-suite", taskset=taskset) + +for revision in tasksets.list_revisions("geography-suite").data: + print(revision.revision, revision.content_hash) +``` + + +Re-submitting the *same* member names can still publish a new revision. Members are re-resolved on +every write, so if a member task published in the meantime the grouping now names different content +and genuinely differs. A taskset's identity is the exact revisions it names, not the names alone. + +Member *order*, by contrast, is not part of that identity: membership is a set, so it is stored in a +canonical order and reordering the same members publishes nothing. + + Deleting a taskset does not delete its member tasks — a taskset only holds references. ## Run an evaluation over a taskset @@ -178,15 +288,44 @@ input_spec = AgentEvalInputSpec( ) ``` -When the job runs, the taskset reference is resolved: its member tasks are loaded, and each task's -stored metric references are hydrated into runnable metrics — exactly as if you had inlined them. The -same spec is submitted as the agent-evaluate job input; see +When the job runs, the taskset reference is resolved like this: + +- Each member is loaded at the **revision pinned in the taskset**, not the task's current tip. +- Metric references on those members are hydrated into runnable metrics, the same as for inline tasks. +- Re-running the same taskset therefore evaluates the same content, even if a member has been + republished since. + +```text +# What is stored on the taskset (digests fixed at create/replace time) +geography-suite@1 + └─ tasks: + - default/capital-of-france#aaa111... ← revision 1 content + - default/capital-of-japan#bbb222... + +# Later: the member task publishes new content +capital-of-france tip → revision 2 (#ccc333...) + +# Re-grade still uses the taskset pins, not the tip +JobRun A ──TasksetRef("geography-suite")──► load #aaa111..., #bbb222... +JobRun B ──TasksetRef("geography-suite")──► load #aaa111..., #bbb222... ← same content +``` + +To evaluate the *new* task content, publish a new taskset revision (for example +`tasksets.replace(...)`) so membership is re-resolved to the newer digests. + +Submit this spec as the agent-evaluate job input; see [Agent Evaluation](/documentation/evaluate-models/agent-eval) for the full run, target, and results flow. The inline form remains available for one-off tasks — swap `tasks=TasksetRef(...)` for `tasks=[AgentEvalTaskInput(...), ...]`. + +A `TasksetRef` names the taskset's current revision; it cannot yet carry a `#` +fragment of its own. Member content is pinned, so a re-run always grades the same task content — but +if the taskset itself is replaced, a re-submitted spec expands the new membership. + + 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 @@ -227,17 +366,42 @@ The SDK resources are a thin client over the Evaluator plugin REST API, mounted | Method | Path | Description | |--------|------|-------------| | `GET` | `/tasks` | List tasks (paginated). | -| `POST` | `/tasks/{name}` | Create a task. | -| `GET` | `/tasks/{name}` | Retrieve a task. | -| `DELETE` | `/tasks/{name}` | Delete a task. | +| `POST` | `/tasks/{name}` | Create a task and publish revision 1. | +| `PUT` | `/tasks/{name}` | Replace a task's content and publish; creates it if absent. | +| `GET` | `/tasks/{name}` | Retrieve a task's current content. | +| `GET` | `/tasks/{name}/revisions` | List published revisions (paginated, newest first). | +| `GET` | `/tasks/{name}/revisions/{revision}` | Retrieve content as of a digest or tag. | +| `PUT` | `/tasks/{name}/tags/{tag}?revision=` | Point a tag at an existing revision. | +| `DELETE` | `/tasks/{name}` | Delete a task and all of its revisions. | | `GET` | `/tasksets` | List tasksets (paginated). | -| `POST` | `/tasksets/{name}` | Create a taskset. | -| `GET` | `/tasksets/{name}` | Retrieve a taskset. | -| `DELETE` | `/tasksets/{name}` | Delete a taskset. | - -Creating a name that already exists returns `409`. An invalid metric reference (task) or a missing or -duplicate task reference (taskset) returns `422`. Retrieving or deleting a name that does not exist -returns `404`. +| `POST` | `/tasksets/{name}` | Create a taskset and publish revision 1. | +| `PUT` | `/tasksets/{name}` | Replace a taskset's membership and publish; creates it if absent. | +| `GET` | `/tasksets/{name}` | Retrieve a taskset's current membership. | +| `GET` | `/tasksets/{name}/revisions` | List published revisions (paginated, newest first). | +| `GET` | `/tasksets/{name}/revisions/{revision}` | Retrieve membership as of a digest or tag. | +| `PUT` | `/tasksets/{name}/tags/{tag}?revision=` | Point a tag at an existing revision. | +| `DELETE` | `/tasksets/{name}` | Delete a taskset and all of its revisions. | + +### Edge Cases + +`PUT` distinguishes its two outcomes by status: **`201`** when a new revision was published, and +**`200`** when the submitted content was already the current revision and nothing was cut. The rest: + +| Case | Status | Behavior | +|------|--------|----------| +| `PUT` with new content | `201` | A revision is published and `latest` moves to it. | +| `PUT` with content identical to the current revision | `200` | Nothing is published; any tags in the body are still applied. | +| `PUT` with taskset members reordered | `200` | Membership is a set, so a reordering is not a content change. | +| `PUT` reverting to *older* content | `201` | The record genuinely changed, so it publishes a new ordinal rather than reusing the old one. | +| `PUT` re-resolving a member that has since republished | `201` | A taskset's identity is the exact revisions it names. | +| Two identical `PUT`s racing | `201` + `200` | One publishes; the other adopts its revision rather than cutting a duplicate. | +| `POST` on an existing name | `409` | Create is strict; use `PUT` to upsert. | +| `PUT` losing a race with a concurrent write | `409` | Retry against the current state. | +| Invalid metric reference (task) | `422` | Rejected at validation. | +| Missing or duplicate task reference (taskset) | `422` | Members must exist, and must resolve to distinct tasks. | +| Reserved or malformed tag name | `422` | `latest` cannot be moved by hand; a digest-shaped tag is refused. | +| Retrieving or deleting an unknown name or revision | `404` | Applies to both records and revisions. | +| `DELETE` on a task a taskset pins | `204` | Not prevented; the taskset's reference dangles and fails on read. | ## Related Topics diff --git a/plugins/nemo-evaluator/openapi/openapi.yaml b/plugins/nemo-evaluator/openapi/openapi.yaml index 68ffee6c31..a91e57bc18 100644 --- a/plugins/nemo-evaluator/openapi/openapi.yaml +++ b/plugins/nemo-evaluator/openapi/openapi.yaml @@ -1319,7 +1319,9 @@ paths: tags: - Evaluator Plugin Tasks Routes summary: Create Task - description: Store a new task, addressed by workspace/name. + description: "Store a new task, addressed by workspace/name, and publish it\ + \ as revision 1.\n\nStrict create \u2014 409 if the name is taken. Use ``PUT``\ + \ to publish a further revision of an\nexisting task." operationId: create_task_apis_evaluator_v2_workspaces__workspace__tasks__name__post parameters: - name: workspace @@ -1365,11 +1367,78 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPValidationError' + put: + tags: + - Evaluator Plugin Tasks Routes + summary: Replace Task + description: "Replace a task's content and publish the result, creating the\ + \ task if it does not exist.\n\nUpsert rather than 404-on-missing so a publisher\ + \ can issue one idempotent call without first\nchecking existence \u2014 that\ + \ check is both an extra round trip and a race between two publishers.\n\n\ + Idempotent in the strict sense: submitting content that matches the current\ + \ revision publishes\nnothing and returns **200**, while a genuine change\ + \ returns **201**. Tags in the body are\napplied either way, so re-PUTting\ + \ unchanged content is how you tag an existing revision." + operationId: replace_task_apis_evaluator_v2_workspaces__workspace__tasks__name__put + parameters: + - name: workspace + in: path + required: true + schema: + type: string + title: Workspace + - name: name + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Name + - name: project + in: query + required: false + schema: + description: Optional project to associate with the task. + title: Project + type: string + description: Optional project to associate with the task. + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/TaskInput' + responses: + '200': + description: Content already published; no new revision was cut + content: + application/json: + schema: + $ref: '#/components/schemas/Task' + '201': + description: A new revision was published + content: + application/json: + schema: + $ref: '#/components/schemas/Task' + '409': + description: Concurrent write; refresh and retry + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' get: tags: - Evaluator Plugin Tasks Routes summary: Get Task - description: Get a stored task by workspace and name. + description: 'Get a stored task''s current content. + + + Use ``GET /tasks/{name}/revisions/{revision}`` to read it as of a specific + published revision.' operationId: get_task_apis_evaluator_v2_workspaces__workspace__tasks__name__get parameters: - name: workspace @@ -1429,6 +1498,180 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPValidationError' + /apis/evaluator/v2/workspaces/{workspace}/tasks/{name}/revisions: + get: + tags: + - Evaluator Plugin Tasks Routes + summary: List Task Revisions + description: 'List a task''s published revisions. + + + This is how a caller discovers what it can pin to: each entry carries the + ``content_hash`` that + + goes in a reference''s ``#fragment``.' + operationId: list_task_revisions_apis_evaluator_v2_workspaces__workspace__tasks__name__revisions_get + parameters: + - name: workspace + in: path + required: true + schema: + type: string + title: Workspace + - name: name + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Name + - name: page + in: query + required: false + schema: + type: integer + minimum: 1 + description: Page number. + default: 1 + title: Page + description: Page number. + - name: page_size + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + description: Page size. + default: 100 + title: Page Size + description: Page size. + responses: + '200': + description: Return the task's published revisions, newest first + content: + application/json: + schema: + $ref: '#/components/schemas/RevisionsPage' + '404': + description: Task not found + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /apis/evaluator/v2/workspaces/{workspace}/tasks/{name}/revisions/{revision}: + get: + tags: + - Evaluator Plugin Tasks Routes + summary: Get Task Revision + description: "Get a task's content as of a published revision.\n\n``revision``\ + \ is a content digest or a tag \u2014 exactly what a reference's ``#fragment``\ + \ carries,\nso a consumer holding ``workspace/task-a#`` reads what\ + \ was published rather than\nwhatever is current. Returns the full task DTO;\ + \ the collection route above is a thin index of\nwhat can be pinned to." + operationId: get_task_revision_apis_evaluator_v2_workspaces__workspace__tasks__name__revisions__revision__get + parameters: + - name: workspace + in: path + required: true + schema: + type: string + title: Workspace + - name: name + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Name + - name: revision + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Revision + responses: + '200': + description: Return the task's content as of a published revision + content: + application/json: + schema: + $ref: '#/components/schemas/Task' + '404': + description: Task or revision not found + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /apis/evaluator/v2/workspaces/{workspace}/tasks/{name}/tags/{tag}: + put: + tags: + - Evaluator Plugin Tasks Routes + summary: Tag Task Revision + description: 'Point a tag at an already-published revision. + + + Separate from publishing because blessing a revision usually happens *after* + it has been + + evaluated. ``latest`` is reserved: it is machine-managed, always names the + most recent publish, + + and moving it by hand would break the forward-only guarantee that keeps concurrent + publishes + + consistent.' + operationId: tag_task_revision_apis_evaluator_v2_workspaces__workspace__tasks__name__tags__tag__put + parameters: + - name: workspace + in: path + required: true + schema: + type: string + title: Workspace + - name: name + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Name + - name: tag + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Tag + - name: revision + in: query + required: true + schema: + type: string + description: "Revision to tag \u2014 a content digest, or another tag." + title: Revision + description: "Revision to tag \u2014 a content digest, or another tag." + responses: + '200': + description: Point a tag at an existing revision + content: + application/json: + schema: + $ref: '#/components/schemas/Task' + '404': + description: Task or revision not found + '422': + description: Tag is reserved /apis/evaluator/v2/workspaces/{workspace}/tasksets: get: tags: @@ -1501,7 +1744,9 @@ paths: tags: - Evaluator Plugin Tasksets Routes summary: Create Taskset - description: Store a new taskset, addressed by workspace/name. + description: "Store a new taskset, addressed by workspace/name, and publish\ + \ it as revision 1.\n\nStrict create \u2014 409 if the name is taken. Use\ + \ ``PUT`` to publish a further revision." operationId: create_taskset_apis_evaluator_v2_workspaces__workspace__tasksets__name__post parameters: - name: workspace @@ -1547,11 +1792,76 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPValidationError' + put: + tags: + - Evaluator Plugin Tasksets Routes + summary: Replace Taskset + description: "Replace a taskset's membership and publish the result, creating\ + \ it if it does not exist.\n\nMembers are re-resolved to exact revision digests\ + \ on every write, so re-submitting the *same*\nmember names can still publish\ + \ a new revision \u2014 if a member task published in the meantime,\nthis\ + \ grouping now names different content and genuinely differs. A taskset's\ + \ identity is the\nexact revisions it names, not the names alone." + operationId: replace_taskset_apis_evaluator_v2_workspaces__workspace__tasksets__name__put + parameters: + - name: workspace + in: path + required: true + schema: + type: string + title: Workspace + - name: name + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Name + - name: project + in: query + required: false + schema: + description: Optional project to associate with the taskset. + title: Project + type: string + description: Optional project to associate with the taskset. + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/TasksetInput' + responses: + '200': + description: Content already published; no new revision was cut + content: + application/json: + schema: + $ref: '#/components/schemas/Taskset' + '201': + description: A new revision was published + content: + application/json: + schema: + $ref: '#/components/schemas/Taskset' + '409': + description: Concurrent write; refresh and retry + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' get: tags: - Evaluator Plugin Tasksets Routes summary: Get Taskset - description: Get a stored taskset by workspace and name. + description: 'Get a stored taskset''s current membership. + + + Use ``GET /tasksets/{name}/revisions/{revision}`` to read it as of a published + revision.' operationId: get_taskset_apis_evaluator_v2_workspaces__workspace__tasksets__name__get parameters: - name: workspace @@ -1611,6 +1921,162 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPValidationError' + /apis/evaluator/v2/workspaces/{workspace}/tasksets/{name}/revisions: + get: + tags: + - Evaluator Plugin Tasksets Routes + summary: List Taskset Revisions + description: "List a taskset's published revisions \u2014 each entry's ``content_hash``\ + \ is what a pinned\nreference carries." + operationId: list_taskset_revisions_apis_evaluator_v2_workspaces__workspace__tasksets__name__revisions_get + parameters: + - name: workspace + in: path + required: true + schema: + type: string + title: Workspace + - name: name + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Name + - name: page + in: query + required: false + schema: + type: integer + minimum: 1 + description: Page number. + default: 1 + title: Page + description: Page number. + - name: page_size + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + description: Page size. + default: 100 + title: Page Size + description: Page size. + responses: + '200': + description: Return the taskset's published revisions, newest first + content: + application/json: + schema: + $ref: '#/components/schemas/RevisionsPage' + '404': + description: Taskset not found + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /apis/evaluator/v2/workspaces/{workspace}/tasksets/{name}/revisions/{revision}: + get: + tags: + - Evaluator Plugin Tasksets Routes + summary: Get Taskset Revision + description: "Get a taskset's membership as of a published revision.\n\n``revision``\ + \ is a content digest or a tag \u2014 what a reference's ``#fragment`` carries\ + \ \u2014 so a\npinned consumer reads the exact grouping that was published." + operationId: get_taskset_revision_apis_evaluator_v2_workspaces__workspace__tasksets__name__revisions__revision__get + parameters: + - name: workspace + in: path + required: true + schema: + type: string + title: Workspace + - name: name + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Name + - name: revision + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Revision + responses: + '200': + description: Return the taskset's membership as of a published revision + content: + application/json: + schema: + $ref: '#/components/schemas/Taskset' + '404': + description: Taskset or revision not found + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /apis/evaluator/v2/workspaces/{workspace}/tasksets/{name}/tags/{tag}: + put: + tags: + - Evaluator Plugin Tasksets Routes + summary: Tag Taskset Revision + description: Point a tag at an already-published revision. ``latest`` is reserved + and machine-managed. + operationId: tag_taskset_revision_apis_evaluator_v2_workspaces__workspace__tasksets__name__tags__tag__put + parameters: + - name: workspace + in: path + required: true + schema: + type: string + title: Workspace + - name: name + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Name + - name: tag + in: path + required: true + schema: + type: string + maxLength: 255 + pattern: ^[\w\-\.]+$ + title: Tag + - name: revision + in: query + required: true + schema: + type: string + description: "Revision to tag \u2014 a content digest, or another tag." + title: Revision + description: "Revision to tag \u2014 a content digest, or another tag." + responses: + '200': + description: Point a tag at an existing revision + content: + application/json: + schema: + $ref: '#/components/schemas/Taskset' + '404': + description: Taskset or revision not found + '422': + description: Tag is reserved components: schemas: AgentEvalInputSpec: @@ -4194,6 +4660,64 @@ components: - -updated_at title: ResultSort description: Sort fields for result queries (``-`` prefix sorts descending). + Revision: + properties: + revision: + type: integer + title: Revision + description: Monotonic 1-based ordinal within the record. + content_hash: + type: string + title: Content Hash + description: 'Full 64-char hex SHA-256 of the revision''s content. This + is what a pinned reference carries: ''workspace/name#''.' + tags: + items: + type: string + type: array + title: Tags + description: Tags currently pointing at this revision, if any. + created_at: + type: string + format: date-time + title: Created At + description: Timestamp the revision was published. + type: object + required: + - revision + - content_hash + - created_at + title: Revision + description: "A published revision of a task or taskset.\n\nDeliberately thin:\ + \ it identifies a revision and says when it was cut, without repeating the\n\ + content. Listing a record's history is a \"what can I pin to?\" question,\ + \ and answering it with\nfull content on every entry would make the response\ + \ large for no benefit \u2014 fetch the record at\na specific revision to\ + \ get its content." + RevisionsPage: + properties: + data: + items: + $ref: '#/components/schemas/Revision' + type: array + title: Data + pagination: + allOf: + - $ref: '#/components/schemas/PaginationData' + description: Pagination information. + sort: + title: Sort + description: The field on which the results are sorted. + type: string + filter: + title: Filter + description: Filtering information. + additionalProperties: true + type: object + type: object + required: + - data + title: RevisionsPage RubricScoreStat: properties: label: @@ -4442,6 +4966,21 @@ components: type: array title: Metadata description: Key/value annotations for the task. + revision: + type: integer + title: Revision + description: "Ordinal of the published revision this content corresponds\ + \ to. Every stored task has at least one revision \u2014 creating a task\ + \ publishes revision 1 \u2014 so this is never 0." + tags: + additionalProperties: + type: integer + type: object + title: Tags + description: "Tag \u2192 revision-ordinal pointers. Reading the record's\ + \ current content returns every tag, including 'latest'. Reading a *specific*\ + \ revision returns only the tags pointing at that revision, which may\ + \ be none \u2014 so do not assume 'latest' is present." created_at: type: string format: date-time @@ -4458,6 +4997,7 @@ components: - name - workspace - intent + - revision - created_at - updated_at title: Task @@ -4521,6 +5061,13 @@ components: type: array title: Metadata description: Key/value annotations for the task. + tags: + items: + type: string + type: array + title: Tags + description: Tags to point at the revision this request publishes. 'latest' + is always applied server-side and need not be listed. additionalProperties: false type: object required: @@ -4548,12 +5095,18 @@ components: runtime falls back to the task ``intent`` when it is unset.' TaskRef: type: string - pattern: ^[\w\-.]+(/[\w\-.]+)?$ + pattern: ^[\w\-.]+(/[\w\-.]+)?(#[\w\-.]+)?$ title: TaskRef - description: "Reference to a persisted task (format: ``workspace/name`` or ``name``).\n\ - \nSame shape and charset as :class:`MetricRef` \u2014 a taskset points at\ - \ its member tasks by reference\n(there are no inline tasks), so a stored\ - \ taskset only ever holds refs." + description: "Reference to a persisted task (format: ``workspace/name``, ``name``,\ + \ or either with a\n``#revision`` fragment).\n\nA taskset points at its member\ + \ tasks by reference (there are no inline tasks), so a stored\ntaskset only\ + \ ever holds refs. Unlike :class:`MetricRef`, a task ref may address a specific\n\ + revision via the platform's standard ``#`` sub-entity fragment.\n\nThe fragment\ + \ is optional *on input* and means :data:`LATEST_TAG` when absent \u2014 a\ + \ bare\n``workspace/name`` is \"the current revision\", not \"unpinned\".\ + \ It may name a tag or a content\ndigest. Anything **persisted** as a published\ + \ snapshot must carry a resolved digest: tags move,\nand a stored tag fragment\ + \ would silently re-point published membership." TaskSort: type: string enum: @@ -4623,6 +5176,20 @@ components: type: array title: Metadata description: Key/value annotations for the taskset. + revision: + type: integer + title: Revision + description: Ordinal of the published revision this content corresponds + to. Every stored taskset has at least one revision, so this is never 0. + tags: + additionalProperties: + type: integer + type: object + title: Tags + description: "Tag \u2192 revision-ordinal pointers. Reading the record's\ + \ current content returns every tag, including 'latest'. Reading a *specific*\ + \ revision returns only the tags pointing at that revision, which may\ + \ be none \u2014 so do not assume 'latest' is present." created_at: type: string format: date-time @@ -4638,6 +5205,7 @@ components: - id - name - workspace + - revision - created_at - updated_at title: Taskset @@ -4679,13 +5247,25 @@ components: $ref: '#/components/schemas/TaskRef' type: array title: Tasks - description: References to the member tasks (set semantics; duplicates rejected). + description: 'References to the member tasks (set semantics; duplicates + rejected). Each may be bare, tag-pinned (''task-a#latest''), or digest-pinned; + all are resolved to an exact digest when stored, so the grouping cannot + change underneath you when a member republishes. Because membership is + a set, the stored order is canonical rather than the submitted order: + reordering the same members is not a content change and publishes no revision.' metadata: items: $ref: '#/components/schemas/MetadataItem' type: array title: Metadata description: Key/value annotations for the taskset. + tags: + items: + type: string + type: array + title: Tags + description: Tags to point at the revision this request publishes. 'latest' + is always applied server-side and need not be listed. additionalProperties: false type: object title: TasksetInput diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py index 10c5a62b60..97fb2d81be 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py @@ -5,10 +5,12 @@ from __future__ import annotations +import re from datetime import datetime from enum import StrEnum 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, @@ -138,6 +140,28 @@ class MetricInline(BaseModel): # 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 +# shared by ``MetricRef`` and ``TasksetRef``, neither of which has revisions yet, and admitting a +# fragment there would accept input nothing is built to resolve. ``TasksetRef`` moves onto this +# pattern when taskset revisions are addressable; ``MetricRef`` 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)``. @@ -145,11 +169,27 @@ def parse_entity_ref(root: str, default_workspace: str) -> tuple[str, str]: 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. """ - workspace, separator, name = root.partition("/") + base, _, _ = root.partition(REF_FRAGMENT_SEPARATOR) + workspace, separator, name = base.partition("/") if separator: return workspace, name - return default_workspace, root + 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]): @@ -168,15 +208,23 @@ class MetricRef(RootModel[str]): class TaskRef(RootModel[str]): - """Reference to a persisted task (format: ``workspace/name`` or ``name``). + """Reference to a persisted task (format: ``workspace/name``, ``name``, or either with a + ``#revision`` fragment). - Same shape and charset as :class:`MetricRef` — a taskset points at its member tasks by reference - (there are no inline tasks), so a stored taskset only ever holds refs. + 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=_ENTITY_REF_PATTERN, - description="Reference to a stored task (format: workspace/task-name, or task-name in the taskset workspace).", + 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 '#'.", ) @@ -348,6 +396,16 @@ class Task(BaseModel): default_factory=dict, description="Optional reporting views mapping metric outputs into named semantic scores." ) 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 " + "has at least one revision — creating a task publishes revision 1 — so this is never 0." + ) + tags: dict[str, int] = Field( + default_factory=dict, + description="Tag → revision-ordinal pointers. Reading the record's current content returns " + "every tag, including 'latest'. Reading a *specific* revision returns only the tags pointing " + "at that revision, which may be none — so do not assume 'latest' is present.", + ) created_at: datetime = Field(description="Timestamp the task was created.") updated_at: datetime = Field(description="Timestamp the task was last updated.") @@ -370,6 +428,29 @@ class TaskInput(BaseModel): default_factory=dict, description="Optional reporting views mapping metric outputs into named semantic scores." ) metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the task.") + tags: list[str] = Field( + default_factory=list, + description="Tags to point at the revision this request publishes. 'latest' is always applied " + "server-side and need not be listed.", + ) + + +class Revision(BaseModel): + """A published revision of a task or taskset. + + Deliberately thin: it identifies a revision and says when it was cut, without repeating the + content. Listing a record's history is a "what can I pin to?" question, and answering it with + full content on every entry would make the response large for no benefit — fetch the record at + a specific revision to get its content. + """ + + revision: int = Field(description="Monotonic 1-based ordinal within the record.") + content_hash: str = Field( + description="Full 64-char hex SHA-256 of the revision's content. This is what a pinned " + "reference carries: 'workspace/name#'." + ) + tags: list[str] = Field(default_factory=list, description="Tags currently pointing at this revision, if any.") + created_at: datetime = Field(description="Timestamp the revision was published.") class TaskSort(StrEnum): @@ -406,6 +487,35 @@ def _reject_duplicate_task_refs(refs: list[TaskRef]) -> list[TaskRef]: #: 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. @@ -423,6 +533,16 @@ class Taskset(BaseModel): default_factory=list, description="References to the member tasks (set semantics; duplicates rejected)." ) metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the taskset.") + revision: int = Field( + description="Ordinal of the published revision this content corresponds to. Every stored " + "taskset has at least one revision, so this is never 0." + ) + tags: dict[str, int] = Field( + default_factory=dict, + description="Tag → revision-ordinal pointers. Reading the record's current content returns " + "every tag, including 'latest'. Reading a *specific* revision returns only the tags pointing " + "at that revision, which may be none — so do not assume 'latest' is present.", + ) created_at: datetime = Field(description="Timestamp the taskset was created.") updated_at: datetime = Field(description="Timestamp the taskset was last updated.") @@ -438,9 +558,19 @@ class TasksetInput(BaseModel): description: str | None = Field(default=None, description="Human-readable description of the grouping.") tasks: TaskRefList = Field( - default_factory=list, description="References to the member tasks (set semantics; duplicates rejected)." + default_factory=list, + description="References to the member tasks (set semantics; duplicates rejected). Each may be " + "bare, tag-pinned ('task-a#latest'), or digest-pinned; all are resolved to an exact digest " + "when stored, so the grouping cannot change underneath you when a member republishes. " + "Because membership is a set, the stored order is canonical rather than the submitted order: " + "reordering the same members is not a content change and publishes no revision.", ) metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the taskset.") + tags: list[str] = Field( + default_factory=list, + description="Tags to point at the revision this request publishes. 'latest' is always applied " + "server-side and need not be listed.", + ) class TasksetSort(StrEnum): 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 a64e4ceb58..171bda0fae 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 @@ -13,13 +13,30 @@ from __future__ import annotations import logging -from typing import Protocol +from typing import Protocol, cast -from nemo_evaluator.api.schemas import MetricInline, MetricRef, Task, TaskInput, parse_entity_ref -from nemo_evaluator.entities import TaskEntity -from nemo_platform_plugin.entities import PaginationInfo +from nemo_evaluator.api.schemas import ( + LATEST_TAG, + MetricInline, + MetricRef, + Revision, + Task, + TaskInput, + parse_entity_ref, +) +from nemo_evaluator.entities import TaskEntity, TaskRevisionEntity +from nemo_evaluator.revisions import ( + apply_tag, + get_revision, + list_revisions, + publish_revision, +) +from nemo_platform_plugin.entities import ( + EntityClientProtocol, + EntityUpdateClientProtocol, + PaginationInfo, +) from nemo_platform_plugin.entity_client import ( - NemoEntitiesClientProtocol, NemoEntityConflictError, NemoEntityNotFoundError, ) @@ -56,11 +73,53 @@ def _entity_to_task(entity: TaskEntity) -> Task: metrics=entity.metrics, views=entity.views, metadata=entity.metadata, + revision=entity.latest_revision, + tags=entity.tags, created_at=created_at, updated_at=updated_at, ) +def _revision_to_task(head: TaskEntity, revision: TaskRevisionEntity) -> Task: + """Present a published revision as the ``Task`` DTO. + + Identity (id, name, workspace, project) comes from the head — it is the same task — while every + content field comes from the revision. ``tags`` are the head's, because tags are current state: + "which tags point here now" is a question about the present, not about what was frozen. + """ + created_at = revision.created_at + if created_at is None: + raise ValueError(f"Revision {revision.revision} of '{head.workspace}/{head.name}' has no timestamp") + return Task( + id=head.id, + name=head.name, + workspace=head.workspace, + project=head.project, + intent=revision.intent, + inputs=revision.inputs, + metrics=revision.metrics, + views=revision.views, + metadata=revision.metadata, + revision=revision.revision, + tags={tag: ordinal for tag, ordinal in head.tags.items() if ordinal == revision.revision}, + created_at=created_at, + updated_at=revision.updated_at or created_at, + ) + + +def _entity_to_revision(revision: TaskRevisionEntity, tags: dict[str, int]) -> Revision: + """Map a stored revision to its API DTO, attaching the tags that currently point at it.""" + created_at = revision.created_at + if created_at is None: + raise ValueError(f"Revision {revision.revision} is missing its creation timestamp") + return Revision( + revision=revision.revision, + content_hash=revision.content_hash, + tags=sorted(tag for tag, ordinal in tags.items() if ordinal == revision.revision), + created_at=created_at, + ) + + def _pagination(src: PaginationInfo, current_page_size: int) -> PaginationData: """Carry the entity-store pagination counts into the API ``Page`` envelope.""" return PaginationData( @@ -72,11 +131,25 @@ def _pagination(src: PaginationInfo, current_page_size: int) -> PaginationData: ) +class TaskEntityStoreProtocol(EntityClientProtocol[TaskEntity], EntityUpdateClientProtocol[TaskEntity], Protocol): + """The store surface this service needs for its own task records. + + Composed from the shared entity-client protocols rather than a bespoke one, so the surface a + service depends on cannot drift from the client that satisfies it. + """ + + class TaskService: """Create/get/list/delete for persisted agent-eval task entities, exposed as the ``Task`` DTO.""" - def __init__(self, entity_client: NemoEntitiesClientProtocol[TaskEntity], metric_service: _MetricService): + def __init__(self, entity_client: TaskEntityStoreProtocol, metric_service: _MetricService): self.entity_client = entity_client + #: The same client, viewed at the revision type. Python has no intersection types, so a + #: single annotation cannot say "serves TaskEntity *and* TaskRevisionEntity" — but the + #: concrete client's methods are generic over the entity type and genuinely satisfy both. + self.revision_client: EntityClientProtocol[TaskRevisionEntity] = cast( + EntityClientProtocol[TaskRevisionEntity], entity_client + ) self.metric_service = metric_service async def _normalize_metrics(self, metrics: list[MetricRef | MetricInline], *, workspace: str) -> list[MetricRef]: @@ -97,35 +170,156 @@ 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 _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.metadata = task_input.metadata + return entity + async def create_task( self, name: str, task_input: TaskInput, *, workspace: str, project: str | None = None - ) -> Task: - """Store a new task (addressed by workspace/name). Raises ``ValueError`` if it already exists.""" - entity = TaskEntity( - name=name, + ) -> tuple[Task, bool]: + """Store a new task and publish it as revision 1. + + Strict create: raises ``ValueError`` if the name is taken. Returns ``(task, published)``, + 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, workspace=workspace, - project=project, - intent=task_input.intent, - inputs=task_input.inputs, - metrics=await self._normalize_metrics(task_input.metrics, workspace=workspace), - views=task_input.views, - metadata=task_input.metadata, ) try: created = await self.entity_client.create(entity) except NemoEntityConflictError as exc: raise ValueError(f"Task '{workspace}/{name}' already exists") from exc + try: + head, published = await self._publish(created, tags=set(task_input.tags)) + except Exception: + # A head with no revision would violate the invariant every consumer relies on — that + # `#latest` always resolves and `revision` is never 0. There is no cross-entity + # transaction, so roll the head back by hand rather than leaving a half-created task. + logger.exception("Publishing revision 1 failed; rolling back the task record") + try: + await self.entity_client.delete(TaskEntity, name=name, workspace=workspace) + except Exception: + # Report the rollback failure, but re-raise the *original* error: replacing it + # would hide why the publish failed and leave the caller debugging the cleanup. + logger.exception("Rollback of the orphaned task record also failed") + raise logger.info( "Task created", extra={"workspace": sanitize_for_log(workspace), "task_name": sanitize_for_log(name)} ) - return _entity_to_task(created) + return _entity_to_task(head), published + + async def replace_task( + self, name: str, task_input: TaskInput, *, workspace: str, project: str | None = None + ) -> tuple[Task, bool]: + """Replace a task's content and publish the result, creating the task if absent. + + Upsert rather than 404-on-missing so a publisher can issue one idempotent call without + first checking existence — checking then creating is both an extra round trip and a race + between two publishers of the same task. + + Returns ``(task, published)``. ``published`` is ``False`` when the submitted content matches + the current revision: the request is then a no-op that still applies any new tags, which is + what makes repeated PUTs of the same content cheap and genuinely idempotent. + """ + try: + head = await self.entity_client.get(TaskEntity, name=name, workspace=workspace) + except NemoEntityNotFoundError: + return await self.create_task(name, task_input, workspace=workspace, project=project) + + await self._apply_content(head, task_input, workspace=workspace) + if project is not None: + # Applied rather than ignored. ``project`` is a query parameter, so an omitted value + # means "leave it alone" — only an explicit value moves the record. + head.project = project + # Publish the staged content *without* committing the head first. Publishing already writes + # the head (pointers and content together), so a pre-write would be a second round trip + # whose only distinct effect is a window: if publishing then failed, the head would hold + # content no revision covers and a plain GET would serve it. + published_head, published = await self._publish(head, tags=set(task_input.tags)) + if not published: + # Content matched a revision that is already tagged as requested, so publishing wrote + # nothing. Anything outside the digest — ``project`` — still has to be persisted. + published_head = await self.entity_client.update(published_head) + logger.info( + "Task replaced", + extra={ + "workspace": sanitize_for_log(workspace), + "task_name": sanitize_for_log(name), + "published": published, + }, + ) + return _entity_to_task(published_head), published + + async def _publish(self, head: TaskEntity, *, tags: set[str]) -> tuple[TaskEntity, bool]: + """Freeze the head as a revision. The returned head already carries the new pointers.""" + _, published_head, created = await publish_revision( + self.entity_client, self.revision_client, head, TaskRevisionEntity, tags=tags + ) + return published_head, created + + async def resolve_revision(self, workspace: str, name: str, fragment: str = LATEST_TAG) -> str: + """Return the content digest of the revision a ref fragment names. + + This is what turns a *tag*-pinned member reference into a *digest*-pinned one at taskset + publish time. Raises :class:`RevisionNotFoundError` if the task has no such revision, and + ``NemoEntityNotFoundError`` if the task itself is missing. + """ + head = await self.entity_client.get(TaskEntity, name=name, workspace=workspace) + revision = await get_revision(self.revision_client, TaskRevisionEntity, head, fragment) + return revision.content_hash + + async def list_revisions( + self, workspace: str, name: str, *, page: int = 1, page_size: int = 100 + ) -> Page[Revision] | None: + """List a task's published revisions, newest first; ``None`` if the task is absent. + + Paged rather than returning every revision: a bare list would silently truncate at the + store's page size, making a capped history indistinguishable from a complete one. + """ + try: + head = await self.entity_client.get(TaskEntity, name=name, workspace=workspace) + except NemoEntityNotFoundError: + return None + result = await list_revisions(self.revision_client, TaskRevisionEntity, head, page=page, page_size=page_size) + data = [_entity_to_revision(revision, head.tags) for revision in result.data] + return Page(data=data, pagination=_pagination(result.pagination, len(data)), sort=None, filter=None) + + async def tag_revision(self, workspace: str, name: str, tag: str, fragment: str) -> Task | None: + """Point a tag at an existing revision; ``None`` if the task is absent.""" + try: + head = await self.entity_client.get(TaskEntity, name=name, workspace=workspace) + except NemoEntityNotFoundError: + return None + updated = await apply_tag(self.entity_client, self.revision_client, TaskRevisionEntity, head, tag, fragment) + logger.info( + "Task revision tagged", + extra={"workspace": sanitize_for_log(workspace), "task_name": sanitize_for_log(name)}, + ) + return _entity_to_task(updated) + + async def get_task(self, workspace: str, name: str, revision: str | None = None) -> Task | None: + """Get a stored task; ``None`` if absent. - async def get_task(self, workspace: str, name: str) -> Task | None: + ``revision`` is a tag or a content digest — the same thing a ref's ``#fragment`` carries. + Omitted, the current content is returned. Supplied, the task is returned *as of* that + revision, which is how a consumer holding a pinned reference reads what was published + rather than what happens to be current. + """ try: - entity = await self.entity_client.get(TaskEntity, workspace=workspace, name=name) + head = await self.entity_client.get(TaskEntity, name=name, workspace=workspace) except NemoEntityNotFoundError: return None - return _entity_to_task(entity) + if revision is None: + return _entity_to_task(head) + return _revision_to_task(head, await get_revision(self.revision_client, TaskRevisionEntity, head, revision)) async def list_tasks( self, @@ -150,7 +344,7 @@ async def list_tasks( async def delete_task(self, workspace: str, name: str) -> bool: """Delete a stored task; ``False`` if absent.""" try: - await self.entity_client.delete(TaskEntity, name, workspace=workspace) + await self.entity_client.delete(TaskEntity, name=name, workspace=workspace) except NemoEntityNotFoundError: return False logger.info( 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 adb404c9ea..359fea7a24 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 @@ -9,20 +9,40 @@ ``TaskService`` uses — so the wire contract round-trips cleanly (an ``EntityBase``'s ``id``/ ``created_at`` are computed and don't deserialize from the entity's own serialized form). -Unlike ``TaskService`` there are no inline members to normalize; instead, each referenced task is -validated to exist at create time (a taskset that points at missing tasks is rejected). +Unlike ``TaskService`` there are no inline members to normalize. Instead, every member reference is +resolved on write to the exact revision digest it names, which is also where a missing member is +caught — resolution has to fetch the task anyway, so existence falls out of it rather than costing +a separate round trip per member. """ from __future__ import annotations +import asyncio import logging -from typing import Protocol - -from nemo_evaluator.api.schemas import TaskRef, Taskset, TasksetInput, parse_entity_ref -from nemo_evaluator.entities import TasksetEntity -from nemo_platform_plugin.entities import PaginationInfo +from typing import Protocol, cast + +from nemo_evaluator.api.schemas import ( + Revision, + TaskRef, + Taskset, + TasksetInput, + parse_entity_ref, + parse_subentity_ref, +) +from nemo_evaluator.entities import TasksetEntity, TasksetRevisionEntity +from nemo_evaluator.revisions import ( + RevisionNotFoundError, + apply_tag, + get_revision, + list_revisions, + publish_revision, +) +from nemo_platform_plugin.entities import ( + EntityClientProtocol, + EntityUpdateClientProtocol, + PaginationInfo, +) from nemo_platform_plugin.entity_client import ( - NemoEntitiesClientProtocol, NemoEntityConflictError, NemoEntityNotFoundError, ) @@ -32,10 +52,17 @@ logger = logging.getLogger(__name__) +#: How many member refs resolve at once when publishing a taskset. Bounded so a large grouping +#: cannot flood the entity store with simultaneous requests, but high enough that a hundred-member +#: dataset does not publish at one round trip per member. +_MEMBER_RESOLUTION_CONCURRENCY = 10 + class _TaskService(Protocol): async def get_task(self, workspace: str, name: str) -> object | None: ... + async def resolve_revision(self, workspace: str, name: str, fragment: str = "latest") -> str: ... + class TaskRefNotFoundError(ValueError): """A taskset references a task that does not exist. @@ -75,12 +102,48 @@ def _entity_to_taskset(entity: TasksetEntity) -> Taskset: project=entity.project, description=entity.description, tasks=entity.tasks, + revision=entity.latest_revision, + tags=entity.tags, metadata=entity.metadata, created_at=created_at, updated_at=updated_at, ) +def _revision_to_taskset(head: TasksetEntity, revision: TasksetRevisionEntity) -> Taskset: + """Present a published revision as the ``Taskset`` DTO — identity from the head, content from + the revision. Tags are the head's, since "what points here now" is a question about the present.""" + created_at = revision.created_at + if created_at is None: + raise ValueError(f"Revision {revision.revision} of '{head.workspace}/{head.name}' has no timestamp") + return Taskset( + id=head.id, + name=head.name, + workspace=head.workspace, + project=head.project, + description=revision.description, + tasks=revision.tasks, + metadata=revision.metadata, + revision=revision.revision, + tags={tag: ordinal for tag, ordinal in head.tags.items() if ordinal == revision.revision}, + created_at=created_at, + updated_at=revision.updated_at or created_at, + ) + + +def _entity_to_revision(revision: TasksetRevisionEntity, tags: dict[str, int]) -> Revision: + """Map a stored revision to its API DTO, attaching the tags that currently point at it.""" + created_at = revision.created_at + if created_at is None: + raise ValueError(f"Revision {revision.revision} is missing its creation timestamp") + return Revision( + revision=revision.revision, + content_hash=revision.content_hash, + tags=sorted(tag for tag, ordinal in tags.items() if ordinal == revision.revision), + created_at=created_at, + ) + + def _pagination(src: PaginationInfo, current_page_size: int) -> PaginationData: """Carry the entity-store pagination counts into the API ``Page`` envelope.""" return PaginationData( @@ -92,21 +155,35 @@ def _pagination(src: PaginationInfo, current_page_size: int) -> PaginationData: ) +class TasksetEntityStoreProtocol( + EntityClientProtocol[TasksetEntity], EntityUpdateClientProtocol[TasksetEntity], Protocol +): + """The store surface this service needs for its own taskset records. + + Composed from the shared entity-client protocols rather than a bespoke one, so the surface a + service depends on cannot drift from the client that satisfies it. + """ + + class TasksetService: """Create/get/list/delete for persisted taskset entities, exposed as the ``Taskset`` DTO.""" - def __init__(self, entity_client: NemoEntitiesClientProtocol[TasksetEntity], task_service: _TaskService): + def __init__(self, entity_client: TasksetEntityStoreProtocol, task_service: _TaskService): self.entity_client = entity_client + #: The same client, viewed at the revision type. Python has no intersection types, so a + #: single annotation cannot say "serves TasksetEntity *and* TasksetRevisionEntity" — but the + #: concrete client's methods are generic over the entity type and genuinely satisfy both. + self.revision_client: EntityClientProtocol[TasksetRevisionEntity] = cast( + EntityClientProtocol[TasksetRevisionEntity], entity_client + ) self.task_service = task_service - async def _validate_tasks_exist(self, tasks: list[TaskRef], *, workspace: str) -> None: - """Validate the member refs: each must resolve to a stored task, and no two may resolve to the - same one. + def _reject_duplicate_members(self, tasks: list[TaskRef], *, workspace: str) -> None: + """Reject two refs resolving to the same task. Pure in-memory, so it runs before any I/O. - A bare ``name`` ref resolves against the taskset's own workspace; a ``workspace/name`` ref - resolves against the named workspace. Raises :class:`DuplicateTaskRefError` if two refs point - at the same ``(workspace, name)`` and :class:`TaskRefNotFoundError` if a referenced task is - missing. + 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. """ seen: set[tuple[str, str]] = set() for ref in tasks: @@ -116,41 +193,192 @@ async def _validate_tasks_exist(self, tasks: list[TaskRef], *, workspace: str) - f"Task reference '{ref.root}' resolves to '{resolved[0]}/{resolved[1]}', already in this taskset" ) seen.add(resolved) - if await self.task_service.get_task(*resolved) is None: - raise TaskRefNotFoundError(f"Task reference '{ref.root}' not found in workspace '{resolved[0]}'") + + async def _pin_member(self, ref: TaskRef, *, workspace: str) -> TaskRef: + """Resolve one member ref to ``workspace/name#``. + + Existence and revision resolution are one operation: ``resolve_revision`` already fetches + the task, so a separate existence check would just re-read the same record. + """ + ref_workspace, name, fragment = parse_subentity_ref(ref.root, workspace) + try: + digest = await self.task_service.resolve_revision(ref_workspace, name, fragment) + except NemoEntityNotFoundError as exc: + raise TaskRefNotFoundError(f"Task reference '{ref.root}' not found in workspace '{ref_workspace}'") from exc + except RevisionNotFoundError as exc: + raise TaskRefNotFoundError(f"Task reference '{ref.root}' names no published revision: {exc}") from exc + return TaskRef(f"{ref_workspace}/{name}#{digest}") + + async def _resolved_content(self, taskset_input: TasksetInput, *, workspace: str) -> list[TaskRef]: + """Validate membership and resolve it to digest-pinned refs. + + Members resolve **concurrently**, bounded by :data:`_MEMBER_RESOLUTION_CONCURRENCY`. A + Harbor-scale dataset can name hundreds of tasks, and resolving them one at a time made + publish latency linear in membership. Bounded rather than unbounded so a large taskset + cannot open hundreds of simultaneous connections to the entity store. + + Resolution happens on write because tags move: a stored ``#latest`` would silently re-point + this taskset's membership the next time that task published, and a published grouping that + changes underneath you is not a grouping. Same reason a lockfile records resolved versions, + not ranges. + + The resolved refs are then sorted into a **canonical order**. Membership is a set — the + field documents it as such and duplicates are rejected — but the digest covers the stored + list, so without this ``[a, b]`` and ``[b, a]`` would be the same grouping under two + different digests, and re-submitting a reordered request would cut a revision that changed + nothing. Sorting the stored content rather than only the hash input keeps the digest a hash + of exactly what is stored, which is what verification on read re-checks. + """ + self._reject_duplicate_members(taskset_input.tasks, workspace=workspace) + limit = asyncio.Semaphore(_MEMBER_RESOLUTION_CONCURRENCY) + + async def _bounded(ref: TaskRef) -> TaskRef: + async with limit: + return await self._pin_member(ref, workspace=workspace) + + pending = [asyncio.create_task(_bounded(ref)) for ref in taskset_input.tasks] + try: + return sorted(await asyncio.gather(*pending), key=lambda ref: ref.root) + except BaseException: + # ``gather`` propagates the first failure but leaves its siblings running. One bad + # member in a large grouping would otherwise keep issuing reads long after the request + # failed, and their own errors would surface as unretrieved-task warnings. + for task in pending: + task.cancel() + raise async def create_taskset( self, name: str, taskset_input: TasksetInput, *, workspace: str, project: str | None = None - ) -> Taskset: - """Store a new taskset (addressed by workspace/name). + ) -> tuple[Taskset, bool]: + """Store a new taskset and publish it as revision 1. - Raises ``ValueError`` if it already exists or if any referenced task does not exist. + Strict create: raises :class:`TasksetExistsError` if the name is taken, and + :class:`TaskRefNotFoundError` if a member does not exist or has no such revision. Returns + ``(taskset, published)``; ``published`` is always ``True`` here. """ - await self._validate_tasks_exist(taskset_input.tasks, workspace=workspace) entity = TasksetEntity( name=name, workspace=workspace, project=project, description=taskset_input.description, - tasks=taskset_input.tasks, + tasks=await self._resolved_content(taskset_input, workspace=workspace), metadata=taskset_input.metadata, ) try: created = await self.entity_client.create(entity) except NemoEntityConflictError as exc: raise TasksetExistsError(f"Taskset '{workspace}/{name}' already exists") from exc + try: + head, published = await self._publish(created, tags=set(taskset_input.tags)) + except Exception: + # A head with no revision would break the invariant consumers rely on — `#latest` + # always resolves and `revision` is never 0. No cross-entity transaction exists, so + # roll the head back rather than leave a half-created taskset behind. + logger.exception("Publishing revision 1 failed; rolling back the taskset record") + try: + await self.entity_client.delete(TasksetEntity, name=name, workspace=workspace) + except Exception: + # Report the rollback failure, but re-raise the *original* error: replacing it + # would hide why the publish failed and leave the caller debugging the cleanup. + logger.exception("Rollback of the orphaned taskset record also failed") + raise logger.info( "Taskset created", extra={"workspace": sanitize_for_log(workspace), "taskset_name": sanitize_for_log(name)}, ) - return _entity_to_taskset(created) + return _entity_to_taskset(head), published + + async def replace_taskset( + self, name: str, taskset_input: TasksetInput, *, workspace: str, project: str | None = None + ) -> tuple[Taskset, bool]: + """Replace a taskset's content and publish the result, creating it if absent. + + Note that re-submitting *identical* membership can still publish a new revision: members are + re-resolved on every write, so if a member task published since last time, ``#latest`` now + names a different digest and this grouping genuinely differs. That is the intended + behavior — the taskset's content is the exact revisions it names, not the names alone. + """ + try: + head = await self.entity_client.get(TasksetEntity, name=name, workspace=workspace) + except NemoEntityNotFoundError: + return await self.create_taskset(name, taskset_input, workspace=workspace, project=project) + + if project is not None: + # Applied rather than ignored. ``project`` is a query parameter, so an omitted value + # means "leave it alone" — only an explicit value moves the record. + head.project = project + head.description = taskset_input.description + head.tasks = await self._resolved_content(taskset_input, workspace=workspace) + head.metadata = taskset_input.metadata + # Publish the staged content *without* committing the head first — see the matching comment + # in ``TaskService.replace_task``. Publishing writes the head itself, so a pre-write would + # only open a window where a failed publish leaves the head serving uncovered content. + published_head, published = await self._publish(head, tags=set(taskset_input.tags)) + if not published: + # Publishing wrote nothing (content already published and tagged as requested), so + # persist what sits outside the digest — ``project``. + published_head = await self.entity_client.update(published_head) + logger.info( + "Taskset replaced", + extra={ + "workspace": sanitize_for_log(workspace), + "taskset_name": sanitize_for_log(name), + "published": published, + }, + ) + return _entity_to_taskset(published_head), published + + async def _publish(self, head: TasksetEntity, *, tags: set[str]) -> tuple[TasksetEntity, bool]: + """Freeze the head as a revision. The returned head already carries the new pointers.""" + _, published_head, created = await publish_revision( + self.entity_client, self.revision_client, head, TasksetRevisionEntity, tags=tags + ) + return published_head, created - async def get_taskset(self, workspace: str, name: str) -> Taskset | None: + async def list_revisions( + self, workspace: str, name: str, *, page: int = 1, page_size: int = 100 + ) -> Page[Revision] | None: + """List a taskset's published revisions, newest first; ``None`` if the taskset is absent. + + Paged rather than returning every revision: a bare list would silently truncate at the + store's page size, making a capped history indistinguishable from a complete one. + """ try: - entity = await self.entity_client.get(TasksetEntity, workspace=workspace, name=name) + head = await self.entity_client.get(TasksetEntity, name=name, workspace=workspace) except NemoEntityNotFoundError: return None - return _entity_to_taskset(entity) + result = await list_revisions(self.revision_client, TasksetRevisionEntity, head, page=page, page_size=page_size) + data = [_entity_to_revision(revision, head.tags) for revision in result.data] + return Page(data=data, pagination=_pagination(result.pagination, len(data)), sort=None, filter=None) + + async def tag_revision(self, workspace: str, name: str, tag: str, fragment: str) -> Taskset | None: + """Point a tag at an existing revision; ``None`` if the taskset is absent.""" + try: + head = await self.entity_client.get(TasksetEntity, name=name, workspace=workspace) + except NemoEntityNotFoundError: + return None + updated = await apply_tag(self.entity_client, self.revision_client, TasksetRevisionEntity, head, tag, fragment) + logger.info( + "Taskset revision tagged", + extra={"workspace": sanitize_for_log(workspace), "taskset_name": sanitize_for_log(name)}, + ) + return _entity_to_taskset(updated) + + async def get_taskset(self, workspace: str, name: str, revision: str | None = None) -> Taskset | None: + """Get a stored taskset; ``None`` if absent. + + ``revision`` is a tag or content digest — what a ref's ``#fragment`` carries — so a consumer + holding a pinned reference reads the membership that was published, not what is current. + """ + try: + head = await self.entity_client.get(TasksetEntity, name=name, workspace=workspace) + except NemoEntityNotFoundError: + return None + if revision is None: + return _entity_to_taskset(head) + return _revision_to_taskset( + head, await get_revision(self.revision_client, TasksetRevisionEntity, head, revision) + ) async def list_tasksets( self, @@ -175,7 +403,7 @@ async def list_tasksets( async def delete_taskset(self, workspace: str, name: str) -> bool: """Delete a stored taskset; ``False`` if absent.""" try: - await self.entity_client.delete(TasksetEntity, name, workspace=workspace) + await self.entity_client.delete(TasksetEntity, name=name, workspace=workspace) except NemoEntityNotFoundError: return False logger.info( diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py index a07bd3c4c3..194de74c0d 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py @@ -8,12 +8,13 @@ import logging from typing import Annotated -from fastapi import APIRouter, Depends, HTTPException, Path, Query, status +from fastapi import APIRouter, Depends, HTTPException, Path, Query, Response, status from nemo_evaluator.api.dependencies import get_task_service -from nemo_evaluator.api.schemas import Task, TaskFilter, TaskInput, TaskSort +from nemo_evaluator.api.schemas import Revision, Task, TaskFilter, TaskInput, TaskSort from nemo_evaluator.api.service.task_service import MetricRefNotFoundError, TaskService from nemo_evaluator.authz import scope from nemo_evaluator.entities import MAX_NAME_LENGTH, NAME_PATTERN +from nemo_evaluator.revisions import RevisionConflictError, RevisionNotFoundError from nemo_platform_plugin.api.parsed_filter import ParsedFilter, make_filter_dep from nemo_platform_plugin.authz import CallerKind, PermissionSet, path_rule, perm from nemo_platform_plugin.entities import EntityValidationError @@ -94,12 +95,17 @@ async def create_task( project: str | None = Query(default=None, description="Optional project to associate with the task."), service: TaskService = Depends(get_task_service), ) -> Task: - """Store a new task, addressed by workspace/name.""" + """Store a new task, addressed by workspace/name, and publish it as revision 1. + + Strict create — 409 if the name is taken. Use ``PUT`` to publish a further revision of an + existing task. + """ safe_workspace = sanitize_for_log(workspace) safe_name = sanitize_for_log(name) logger.info(f"Creating task: {safe_workspace}/{safe_name}") try: - return await service.create_task(name, task, workspace=workspace, project=project) + created, _ = await service.create_task(name, task, workspace=workspace, project=project) + return created except EntityValidationError as e: logger.warning(f"Entity store validation error during task creation: {e}") raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) @@ -122,6 +128,169 @@ async def create_task( raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Internal server error") +@router.put( + "/tasks/{name}", + summary="Replace Task", + response_description="Publish a revision of the task", + status_code=status.HTTP_200_OK, + responses={ + status.HTTP_200_OK: {"description": "Content already published; no new revision was cut"}, + status.HTTP_201_CREATED: {"model": Task, "description": "A new revision was published"}, + status.HTTP_409_CONFLICT: {"description": "Concurrent write; refresh and retry"}, + }, +) +@scope.write +@path_rule(callers=[CallerKind.PRINCIPAL], permissions=[TaskPerms.CREATE]) +async def replace_task( + workspace: str, + name: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + task: TaskInput, + response: Response, + project: str | None = Query(default=None, description="Optional project to associate with the task."), + service: TaskService = Depends(get_task_service), +) -> Task: + """Replace a task's content and publish the result, creating the task if it does not exist. + + Upsert rather than 404-on-missing so a publisher can issue one idempotent call without first + checking existence — that check is both an extra round trip and a race between two publishers. + + Idempotent in the strict sense: submitting content that matches the current revision publishes + nothing and returns **200**, while a genuine change returns **201**. Tags in the body are + applied either way, so re-PUTting unchanged content is how you tag an existing revision. + """ + safe_workspace = sanitize_for_log(workspace) + safe_name = sanitize_for_log(name) + logger.info(f"Replacing task: {safe_workspace}/{safe_name}") + try: + replaced, published = await service.replace_task(name, task, workspace=workspace, project=project) + except EntityValidationError as e: + logger.warning(f"Entity store validation error during task replace: {e}") + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) + except MetricRefNotFoundError as e: + logger.warning(f"Task has an invalid metric reference: {e}") + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) + except RevisionConflictError as e: + logger.warning(f"Task revision allocation contended: {e}") + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(e)) + except NemoEntityConflictError as e: + # Another request updated this record between our read and our write. A retry against the + # current state is the caller's move; a 500 would wrongly suggest a server fault. + logger.warning(f"Task modified concurrently during replace: {e}") + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"Task was modified by another request: {workspace}/{name}. Refresh and try again.", + ) + except ValueError as e: + logger.warning(f"Task replace validation error: {e}") + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid task data") + except HTTPException: + raise + except Exception: + logger.exception("Failed to replace task") + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Internal server error") + response.status_code = status.HTTP_201_CREATED if published else status.HTTP_200_OK + return replaced + + +@router.get( + "/tasks/{name}/revisions", + summary="List Task Revisions", + response_description="Return the task's published revisions, newest first", + status_code=status.HTTP_200_OK, + responses={status.HTTP_404_NOT_FOUND: {"description": "Task not found"}}, +) +@scope.read +@path_rule(callers=[CallerKind.PRINCIPAL], permissions=[TaskPerms.READ]) +async def list_task_revisions( + workspace: str, + name: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + page: int = Query(default=1, ge=1, description="Page number."), + page_size: int = Query(default=100, ge=1, le=1000, description="Page size."), + service: TaskService = Depends(get_task_service), +) -> Page[Revision]: + """List a task's published revisions. + + This is how a caller discovers what it can pin to: each entry carries the ``content_hash`` that + goes in a reference's ``#fragment``. + """ + try: + revisions = await service.list_revisions(workspace, name, page=page, page_size=page_size) + except Exception: + logger.exception("Failed to list task revisions") + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Internal server error") + if revisions is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=f"Task '{name}' not found") + return revisions + + +@router.get( + "/tasks/{name}/revisions/{revision}", + summary="Get Task Revision", + response_description="Return the task's content as of a published revision", + status_code=status.HTTP_200_OK, + responses={status.HTTP_404_NOT_FOUND: {"description": "Task or revision not found"}}, +) +@scope.read +@path_rule(callers=[CallerKind.PRINCIPAL], permissions=[TaskPerms.READ]) +async def get_task_revision( + workspace: str, + name: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + revision: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + service: TaskService = Depends(get_task_service), +) -> Task: + """Get a task's content as of a published revision. + + ``revision`` is a content digest or a tag — exactly what a reference's ``#fragment`` carries, + so a consumer holding ``workspace/task-a#`` reads what was published rather than + whatever is current. Returns the full task DTO; the collection route above is a thin index of + what can be pinned to. + """ + try: + task = await service.get_task(workspace, name, revision) + except RevisionNotFoundError as e: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e)) + if task is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=f"Task not found: {workspace}/{name}") + return task + + +@router.put( + "/tasks/{name}/tags/{tag}", + summary="Tag Task Revision", + response_description="Point a tag at an existing revision", + status_code=status.HTTP_200_OK, + responses={ + status.HTTP_404_NOT_FOUND: {"description": "Task or revision not found"}, + status.HTTP_422_UNPROCESSABLE_ENTITY: {"description": "Tag is reserved"}, + }, +) +@scope.write +@path_rule(callers=[CallerKind.PRINCIPAL], permissions=[TaskPerms.CREATE]) +async def tag_task_revision( + workspace: str, + name: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + tag: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + revision: str = Query(description="Revision to tag — a content digest, or another tag."), + service: TaskService = Depends(get_task_service), +) -> Task: + """Point a tag at an already-published revision. + + Separate from publishing because blessing a revision usually happens *after* it has been + evaluated. ``latest`` is reserved: it is machine-managed, always names the most recent publish, + and moving it by hand would break the forward-only guarantee that keeps concurrent publishes + consistent. + """ + try: + tagged = await service.tag_revision(workspace, name, tag, revision) + except ValueError as e: + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) + except RevisionNotFoundError as e: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e)) + if tagged is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=f"Task '{name}' not found") + return tagged + + @router.get( "/tasks/{name}", summary="Get Task", @@ -136,7 +305,10 @@ async def get_task( name: str, service: TaskService = Depends(get_task_service), ) -> Task: - """Get a stored task by workspace and name.""" + """Get a stored task's current content. + + Use ``GET /tasks/{name}/revisions/{revision}`` to read it as of a specific published revision. + """ logger.debug(f"Getting task: {sanitize_for_log(workspace)}/{sanitize_for_log(name)}") try: task = await service.get_task(workspace, name) diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py b/plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py index 697e3dbc43..a697b9cdcc 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py @@ -8,9 +8,9 @@ import logging from typing import Annotated -from fastapi import APIRouter, Depends, HTTPException, Path, Query, status +from fastapi import APIRouter, Depends, HTTPException, Path, Query, Response, status from nemo_evaluator.api.dependencies import get_taskset_service -from nemo_evaluator.api.schemas import Taskset, TasksetFilter, TasksetInput, TasksetSort +from nemo_evaluator.api.schemas import Revision, Taskset, TasksetFilter, TasksetInput, TasksetSort from nemo_evaluator.api.service.taskset_service import ( DuplicateTaskRefError, TaskRefNotFoundError, @@ -19,6 +19,7 @@ ) from nemo_evaluator.authz import scope from nemo_evaluator.entities import MAX_NAME_LENGTH, NAME_PATTERN +from nemo_evaluator.revisions import RevisionConflictError, RevisionNotFoundError from nemo_platform_plugin.api.parsed_filter import ParsedFilter, make_filter_dep from nemo_platform_plugin.authz import CallerKind, PermissionSet, path_rule, perm from nemo_platform_plugin.entities import EntityValidationError @@ -99,12 +100,16 @@ async def create_taskset( project: str | None = Query(default=None, description="Optional project to associate with the taskset."), service: TasksetService = Depends(get_taskset_service), ) -> Taskset: - """Store a new taskset, addressed by workspace/name.""" + """Store a new taskset, addressed by workspace/name, and publish it as revision 1. + + Strict create — 409 if the name is taken. Use ``PUT`` to publish a further revision. + """ safe_workspace = sanitize_for_log(workspace) safe_name = sanitize_for_log(name) logger.info(f"Creating taskset: {safe_workspace}/{safe_name}") try: - return await service.create_taskset(name, taskset, workspace=workspace, project=project) + created, _ = await service.create_taskset(name, taskset, workspace=workspace, project=project) + return created except EntityValidationError as e: logger.warning(f"Entity store validation error during taskset creation: {e}") raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) @@ -128,6 +133,156 @@ async def create_taskset( raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Internal server error") +@router.put( + "/tasksets/{name}", + summary="Replace Taskset", + response_description="Publish a revision of the taskset", + status_code=status.HTTP_200_OK, + responses={ + status.HTTP_200_OK: {"description": "Content already published; no new revision was cut"}, + status.HTTP_201_CREATED: {"model": Taskset, "description": "A new revision was published"}, + status.HTTP_409_CONFLICT: {"description": "Concurrent write; refresh and retry"}, + }, +) +@scope.write +@path_rule(callers=[CallerKind.PRINCIPAL], permissions=[TasksetPerms.CREATE]) +async def replace_taskset( + workspace: str, + name: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + taskset: TasksetInput, + response: Response, + project: str | None = Query(default=None, description="Optional project to associate with the taskset."), + service: TasksetService = Depends(get_taskset_service), +) -> Taskset: + """Replace a taskset's membership and publish the result, creating it if it does not exist. + + Members are re-resolved to exact revision digests on every write, so re-submitting the *same* + member names can still publish a new revision — if a member task published in the meantime, + this grouping now names different content and genuinely differs. A taskset's identity is the + exact revisions it names, not the names alone. + """ + safe_workspace = sanitize_for_log(workspace) + safe_name = sanitize_for_log(name) + logger.info(f"Replacing taskset: {safe_workspace}/{safe_name}") + try: + replaced, published = await service.replace_taskset(name, taskset, workspace=workspace, project=project) + except EntityValidationError as e: + logger.warning(f"Entity store validation error during taskset replace: {e}") + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) + except (TaskRefNotFoundError, DuplicateTaskRefError) as e: + logger.warning(f"Taskset has an invalid task reference: {e}") + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) + except RevisionConflictError as e: + logger.warning(f"Taskset revision allocation contended: {e}") + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(e)) + except NemoEntityConflictError as e: + # Another request updated this record between our read and our write. A retry against the + # current state is the caller's move; a 500 would wrongly suggest a server fault. + logger.warning(f"Taskset modified concurrently during replace: {e}") + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"Taskset was modified by another request: {workspace}/{name}. Refresh and try again.", + ) + except ValueError as e: + logger.warning(f"Taskset replace validation error: {e}") + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid taskset data") + except HTTPException: + raise + except Exception: + logger.exception("Failed to replace taskset") + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Internal server error") + response.status_code = status.HTTP_201_CREATED if published else status.HTTP_200_OK + return replaced + + +@router.get( + "/tasksets/{name}/revisions", + summary="List Taskset Revisions", + response_description="Return the taskset's published revisions, newest first", + status_code=status.HTTP_200_OK, + responses={status.HTTP_404_NOT_FOUND: {"description": "Taskset not found"}}, +) +@scope.read +@path_rule(callers=[CallerKind.PRINCIPAL], permissions=[TasksetPerms.READ]) +async def list_taskset_revisions( + workspace: str, + name: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + page: int = Query(default=1, ge=1, description="Page number."), + page_size: int = Query(default=100, ge=1, le=1000, description="Page size."), + service: TasksetService = Depends(get_taskset_service), +) -> Page[Revision]: + """List a taskset's published revisions — each entry's ``content_hash`` is what a pinned + reference carries.""" + try: + revisions = await service.list_revisions(workspace, name, page=page, page_size=page_size) + except Exception: + logger.exception("Failed to list taskset revisions") + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Internal server error") + if revisions is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=f"Taskset '{name}' not found") + return revisions + + +@router.get( + "/tasksets/{name}/revisions/{revision}", + summary="Get Taskset Revision", + response_description="Return the taskset's membership as of a published revision", + status_code=status.HTTP_200_OK, + responses={status.HTTP_404_NOT_FOUND: {"description": "Taskset or revision not found"}}, +) +@scope.read +@path_rule(callers=[CallerKind.PRINCIPAL], permissions=[TasksetPerms.READ]) +async def get_taskset_revision( + workspace: str, + name: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + revision: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + service: TasksetService = Depends(get_taskset_service), +) -> Taskset: + """Get a taskset's membership as of a published revision. + + ``revision`` is a content digest or a tag — what a reference's ``#fragment`` carries — so a + pinned consumer reads the exact grouping that was published. + """ + try: + taskset = await service.get_taskset(workspace, name, revision) + except RevisionNotFoundError as e: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e)) + if taskset is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=f"Taskset not found: {workspace}/{name}") + return taskset + + +@router.put( + "/tasksets/{name}/tags/{tag}", + summary="Tag Taskset Revision", + response_description="Point a tag at an existing revision", + status_code=status.HTTP_200_OK, + responses={ + status.HTTP_404_NOT_FOUND: {"description": "Taskset or revision not found"}, + status.HTTP_422_UNPROCESSABLE_ENTITY: {"description": "Tag is reserved"}, + }, +) +@scope.write +@path_rule(callers=[CallerKind.PRINCIPAL], permissions=[TasksetPerms.CREATE]) +async def tag_taskset_revision( + workspace: str, + name: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + tag: Annotated[str, Path(max_length=MAX_NAME_LENGTH, pattern=NAME_PATTERN)], + revision: str = Query(description="Revision to tag — a content digest, or another tag."), + service: TasksetService = Depends(get_taskset_service), +) -> Taskset: + """Point a tag at an already-published revision. ``latest`` is reserved and machine-managed.""" + try: + tagged = await service.tag_revision(workspace, name, tag, revision) + except ValueError as e: + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) + except RevisionNotFoundError as e: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e)) + if tagged is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=f"Taskset '{name}' not found") + return tagged + + @router.get( "/tasksets/{name}", summary="Get Taskset", @@ -142,7 +297,10 @@ async def get_taskset( name: str, service: TasksetService = Depends(get_taskset_service), ) -> Taskset: - """Get a stored taskset by workspace and name.""" + """Get a stored taskset's current membership. + + Use ``GET /tasksets/{name}/revisions/{revision}`` to read it as of a published revision. + """ logger.debug(f"Getting taskset: {sanitize_for_log(workspace)}/{sanitize_for_log(name)}") # Only the service call can fail unexpectedly; wrap just that so the 404 below is raised outside # the try (no catching HTTPException only to re-raise it). diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/content_hash.py b/plugins/nemo-evaluator/src/nemo_evaluator/content_hash.py new file mode 100644 index 0000000000..78dd55e9b8 --- /dev/null +++ b/plugins/nemo-evaluator/src/nemo_evaluator/content_hash.py @@ -0,0 +1,91 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Canonical content hashing for revisioned entities (tasks and tasksets). + +A revision is addressed by a digest of its *content* — the same projection the entity store +persists (``model_dump(exclude=__base_fields__)``), serialized canonically and hashed with SHA-256. +Two independent computations of the same content must agree, because a consumer reading a pinned +ref (``workspace/task-name#``) recomputes the digest and compares. + +Three properties this deliberately has: + +**Content only, never identity.** The name, workspace, and revision index are *not* inputs. Salting +the digest with them would break the two things it exists for: republishing identical content would +produce a new digest (defeating publish-time dedup), and verify-on-read would pass whenever +identity matched, regardless of whether the content beneath it had changed. Identity is carried by +the ref, which already names the entity; a cross-entity digest collision can't cause a +misresolution because lookups are scoped by name before the digest is consulted. + +**Full digest, never truncated.** SHA-256's collision resistance is bounded at 2**128 by the +birthday paradox, which is ample. A truncated prefix is not: 12 hex chars is 48 bits, with a +birthday bound near 2**24 — reachable by accident at scale. Short forms belong in display and +prefix *matching* against stored full digests (git's model), never in the stored value. + +**Bare hex, no algorithm prefix.** Matches the existing derived-metric digest idiom in +``metric_service`` and keeps the ``#`` ref fragment free of ``:``, which the entity ref charset +does not admit. See ``docs`` in the backend design for the algorithm-agility tradeoff. + +The weak link is not SHA-256 — it is *canonicalization*. If two semantically different revisions +serialize identically they collide with probability 1 and no hash strength helps. Hence the +explicit rules below, and the near-miss tests that pin them. +""" + +from __future__ import annotations + +import hashlib +import json +from collections.abc import Set + +from nemo_platform_plugin.entities import EntityBase + +#: Length of a SHA-256 digest rendered as lowercase hex. +DIGEST_LENGTH = 64 + +#: Charset/shape of a digest as it appears in a ``#`` ref fragment. +DIGEST_PATTERN = r"^[0-9a-f]{64}$" + + +def canonical_payload(entity: EntityBase, *, exclude: Set[str] | 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 + shape for unchanged content, every stored digest is invalidated. Tests assert on it directly. + + Canonicalization rules: + + - Server-owned fields are dropped via ``EntityBase.__base_fields__`` — the same exclusion the + entity store itself uses when persisting custom fields, so the hash input tracks what is + actually stored rather than a parallel hand-maintained list. + - ``mode="json"`` normalizes rich types (datetimes, enums, sub-models) to JSON primitives. + - ``sort_keys=True`` makes mapping order irrelevant. + - ``separators=(",", ":")`` removes insignificant whitespace. + + Rules this does NOT impose, deliberately: + + - **Sequence order is significant.** A list that is semantically a set must be normalized by + its own model (as ``TaskRefList`` does) before hashing; this function will not reorder it, + because it cannot tell a set from an ordered sequence. + - **``1`` and ``1.0`` hash differently**, per JSON. That is desired: an int and a float are + distinguishable values, not the same value formatted twice. + - **Absent and default-valued fields collapse**, because ``model_dump`` materializes defaults. + A model that needs to distinguish "unset" from "set to the default" must model that + explicitly (e.g. with an optional field defaulting to ``None``). + + Args: + entity: The entity whose content to serialize. + exclude: Extra field names to drop on top of ``__base_fields__``. Revisioned entities pass + 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 ()) + 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: + """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. + """ + return hashlib.sha256(canonical_payload(entity, exclude=exclude).encode("utf-8")).hexdigest() diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/entities.py b/plugins/nemo-evaluator/src/nemo_evaluator/entities.py index c91d5d9f48..cd2404db77 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/entities.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/entities.py @@ -19,7 +19,14 @@ from typing import ClassVar -from nemo_evaluator.api.schemas import MetricRef, TaskInputs, TaskMetadataList, TaskRefList +from nemo_evaluator.api.schemas import ( + MetricRef, + PinnedTaskRefList, + TaskInputs, + 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 @@ -34,6 +41,17 @@ MAX_DESCRIPTION_LENGTH = 1000 NAME_PATTERN = r"^[\w\-\.]+$" +#: Fields on a publishable record that are revision *bookkeeping*, not content. Excluded when +#: digesting a head record so that hashing the head yields the same digest as hashing the +#: corresponding revision. Two things depend on that equality: a publish recognizing "the head +#: already matches the current revision", and a read re-hashing a revision to check it against the +#: digest stored beside it. +REVISION_POINTER_FIELDS = frozenset({"latest_revision", "tags"}) + +#: The mirror of :data:`REVISION_POINTER_FIELDS` on a revision record. A revision's digest covers +#: neither itself nor the ordinal that was assigned because of it. +REVISION_SELF_FIELDS = frozenset({"content_hash", "revision"}) + class MetricBundleEntity(EntityBase): """Persisted index for a stored metric, addressed by workspace/name. @@ -164,7 +182,33 @@ class EvaluateResultEntity(_EvalResultCommon, EntityBase): ) -class TaskEntity(EntityBase): +class _RevisionedCommon(BaseModel): + """Mutable revision bookkeeping carried by a record that can be published. + + Both fields are pointers into the record's immutable revision children, and both are excluded + from the content digest — they describe *which* content is current, not what the content is. + Including them would make the digest change whenever a tag moved. + """ + + latest_revision: int = Field( + default=0, + description="Ordinal of the most recently published revision; 0 before the first publish. " + "The next publish allocates ``latest_revision + 1`` under the record's optimistic lock, so a " + "concurrent publisher that raced loses with a conflict and retries.", + ge=0, + ) + tags: dict[str, int] = Field( + default_factory=dict, + description="Mutable tag → revision-ordinal pointers. ``latest`` is reserved and re-applied " + "on every publish; other tags are user-supplied and may be moved after the fact. Tags are " + "resolution *inputs* only — anything persisted (e.g. a published taskset's membership) " + "stores the resolved content digest, never the tag, so a moved tag cannot re-point it. " + "Ordinals rather than digests because a revision's identity within its parent is its " + "ordinal, which makes tag resolution a direct child lookup with no query.", + ) + + +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 @@ -189,7 +233,7 @@ class TaskEntity(EntityBase): metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the task.") -class TasksetEntity(EntityBase): +class TasksetEntity(_RevisionedCommon, EntityBase): """Persisted, queryable taskset, addressed by workspace/name. A taskset is a flexible grouping of stored tasks: it holds references to its members @@ -209,3 +253,104 @@ class TasksetEntity(EntityBase): description="References to the member tasks (set semantics; duplicates rejected).", ) metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the taskset.") + + +# --- Revisions --------------------------------------------------------------- +# +# A revision is an immutable snapshot of a task's or taskset's content, addressed by a digest of +# that content (see ``nemo_evaluator.content_hash``). Revisions exist so a reference can be *pinned* +# — ``workspace/task-name#`` resolves to exactly the content that was published, and a +# consumer re-derives the digest on read to confirm it. +# +# Persistence shape. A revision is a **child entity** of the record it snapshots, so entity-store +# parent-scoped uniqueness — unique within ``(workspace, entity_type, parent, name)`` — makes an +# ordinal collide rather than silently duplicate. Allocation of the next ordinal rides on the +# parent's ``db_version`` optimistic lock: bump ``latest_revision`` on the parent, and a concurrent +# publisher that raced loses with a conflict and retries. +# +# Naming. Revisions are named ``rev.``, NOT by their digest. Entity names are capped at 63 chars +# and must start with a lowercase letter (``entity_naming.NAME_PATTERN``); a full 64-char hex digest +# violates both. Truncating it to fit — as derived metric names do — would shrink the collision +# bound for no benefit here, since a ref carries the digest in its ``#`` fragment (governed by the +# ref pattern, not the entity-name rules) and the digest lives on the record as an ordinary field +# with no length pressure. Resolving a pinned ref is a filtered lookup on ``content_hash`` scoped to +# the parent. As a bonus, ``rev.7`` is legible in a log line in a way a hex string is not. +# +# Head vs history. The parent record keeps its content fields as the *head* — the current version — +# and revisions accumulate alongside. This keeps the change additive: existing stored records stay +# valid, ``get_task`` and the ``Task`` DTO are unchanged, and nothing needs migrating. The cost is a +# denormalized copy: head and its corresponding revision can drift if a write fails between the two +# (there is no cross-entity transaction). Publishing reconciles — it hashes the head and creates the +# revision only if no revision carries that digest — so a torn write self-heals on the next publish +# rather than persisting a lie. + + +class _RevisionCommon(BaseModel): + """Fields shared by every revision record: its digest and its ordinal. + + A mixin rather than an ``EntityBase`` so each concrete revision type declares its own + ``__entity_type__``, matching the ``_EvalResultCommon`` split above. + + Both fields are excluded from the content digest — a revision's digest cannot cover the ordinal + that was assigned *because of* that digest, nor cover itself. See + ``content_hash.content_hash(..., exclude=...)``. + """ + + content_hash: str = Field( + description="Full 64-char lowercase hex SHA-256 of this revision's content. Never truncated: " + "a shortened digest collapses the birthday bound, and there is no length pressure here " + "because this is a field, not an entity name.", + min_length=DIGEST_LENGTH, + max_length=DIGEST_LENGTH, + pattern=DIGEST_PATTERN, + ) + revision: int = Field( + description="Monotonic 1-based ordinal within the parent record. Matches the ``rev.`` " + "entity name; carried as a field too so it can be sorted and filtered on directly.", + ge=1, + ) + + +class TaskRevisionEntity(_RevisionCommon, EntityBase): + """An immutable published snapshot of a :class:`TaskEntity`'s content. + + Child of the task it snapshots. Content fields mirror ``TaskEntity`` exactly — a revision is + that content frozen, not a different shape. + """ + + __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.", + ) + metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the task.") + + +class TasksetRevisionEntity(_RevisionCommon, EntityBase): + """An immutable published snapshot of a :class:`TasksetEntity`'s content. + + Child of the taskset it snapshots. Its ``tasks`` are **fully pinned** — every member ref carries + a ``#`` fragment resolved at publish time. A stored revision must never retain a tag + fragment (``#latest``, ``#some-tag``): tags move, and a moved tag would silently re-point a + published taskset's membership, which is the failure this whole design exists to prevent. + """ + + __entity_type__: ClassVar[str] = "taskset_revision" + + description: str | None = Field( + default=None, + description="Human-readable description of the grouping.", + max_length=MAX_DESCRIPTION_LENGTH, + ) + tasks: PinnedTaskRefList = Field( + default_factory=list, + description="Digest-pinned references to the member tasks, resolved at publish time.", + ) + metadata: TaskMetadataList = Field(default_factory=list, description="Key/value annotations for the taskset.") diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/revisions.py b/plugins/nemo-evaluator/src/nemo_evaluator/revisions.py new file mode 100644 index 0000000000..5b02566018 --- /dev/null +++ b/plugins/nemo-evaluator/src/nemo_evaluator/revisions.py @@ -0,0 +1,520 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Publishing and resolving revisions of tasks and tasksets. + +A *head* record (``TaskEntity`` / ``TasksetEntity``) holds the current content plus revision +bookkeeping. Publishing freezes that content into an immutable *revision* child +(``TaskRevisionEntity`` / ``TasksetRevisionEntity``) addressed by a digest of the content, and +points the requested tags at it. Resolution goes the other way: a ref's ``#fragment`` — a tag or a +digest — becomes a concrete revision. + +Two ways to address a revision, two lookup costs. A **tag** is a mutable pointer stored on the head +as ``tag → ordinal``, so resolving one is a direct child lookup with no query — which matters +because ``#latest`` is the common path. A **digest** is content-addressed and resolved by querying +the children on ``(parent, content_hash)``. Nothing is denormalized: the head stores only pointers, +never a copy of what the children already say. + +Concurrency. Two publishers racing on the same record both compute ordinal N; the entity store's +parent-scoped uniqueness — unique within ``(workspace, entity_type, parent, name)`` — makes the +second ``rev.N`` create conflict rather than silently duplicate. The loser refreshes its allocation +state and retries against N+1, keeping the content it was asked to publish. The child create, not +the head update, is the serialization point, so a revision is never allocated twice even if the +head update is slow. +""" + +from __future__ import annotations + +import logging +import re +from typing import Protocol, TypeVar + +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_FIELDS, + REVISION_SELF_FIELDS, + TaskEntity, + TaskRevisionEntity, + TasksetEntity, + TasksetRevisionEntity, +) +from nemo_platform_plugin.entities import ( + EntityBase, + EntityClientProtocol, + EntityGetterProtocol, + EntityUpdateClientProtocol, + ListResponse, +) +from nemo_platform_plugin.entity_client import ( + NemoEntityConflictError, + NemoEntityNotFoundError, +) +from nemo_platform_plugin.filter_ops import ComparisonOperation, FilterOperator, LogicalOperation + +logger = logging.getLogger(__name__) + +#: Attempts to allocate an ordinal before giving up. Each retry costs one head re-read; contention +#: on a single task is expected to be rare (publishes are human- or pipeline-paced), so a small +#: bound is enough to absorb a race without masking a genuine, persistent conflict. +_MAX_ALLOCATION_ATTEMPTS = 5 + +#: Prefix for revision entity names. Entity names must start with a lowercase letter and cap at 63 +#: chars, so a revision is ``rev.`` — a bare ordinal is not a legal name, and the full +#: 64-char digest is both too long and usually digit-leading. +_REVISION_NAME_PREFIX = "rev." + +#: Shape of a content digest in a ref fragment — what distinguishes a digest from a tag name. +_DIGEST_FRAGMENT = re.compile(DIGEST_PATTERN) + +#: A tag has to be usable as a ref ``#fragment``, so it is bound by the same charset. Sharing the +#: constant keeps the two from drifting into a state where a tag is mintable but unreferenceable. +_TAG_NAME = re.compile(REF_FRAGMENT_CHARSET) + +HeadT = TypeVar("HeadT", TaskEntity, TasksetEntity) +RevisionT = TypeVar("RevisionT", TaskRevisionEntity, TasksetRevisionEntity) +_EntityT = TypeVar("_EntityT", bound=EntityBase) + + +class HeadStoreProtocol(EntityGetterProtocol[_EntityT], EntityUpdateClientProtocol[_EntityT], Protocol[_EntityT]): + """The head-record surface: read it, then write its pointers under the optimistic lock. + + Composed from the shared protocols rather than restated — ``EntityUpdateClientProtocol`` is + deliberately separate from the CRUD one precisely so callers needing ``update`` can add it. + """ + + +#: The revision-child surface: create a revision, fetch one by ``(name, parent)``, query by digest. +#: The shared CRUD protocol covers all three, parameterised on the revision type rather than erased +#: to "any entity", so a wrong entity type at a call site is a type error rather than a cast. +RevisionStoreProtocol = EntityClientProtocol + + +class RevisionNotFoundError(LookupError): + """A ref names a revision — by tag or digest — that the record does not have.""" + + +class RevisionConflictError(RuntimeError): + """Ordinal allocation lost too many races to concurrent publishers.""" + + +class RevisionContentMismatchError(RuntimeError): + """A stored revision's content no longer hashes to the digest recorded with it. + + Corruption rather than absence: the revision resolved, but serving it would break the promise + a pinned reference makes, so it is refused instead. + """ + + +def revision_name(ordinal: int) -> str: + """Entity name for a revision ordinal.""" + return f"{_REVISION_NAME_PREFIX}{ordinal}" + + +def head_digest(head: EntityBase) -> str: + """Digest of a head record's content, excluding its revision bookkeeping. + + 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) + + +def validate_tag_name(tag: str) -> str: + """Reject tag names that could be stored but never resolved. + + - **Empty**: an absent fragment already means ``latest``, so an empty tag is unaddressable. + - **Outside the fragment charset** (a slash, a space): a ref's ``#fragment`` admits only + ``[\\w\\-.]+``, so such a tag could be applied and listed but never written into a member + reference — the one thing tags exist for. + - **Digest-shaped** (64 lowercase hex): resolution checks the digest form *first*, so such a + tag would be queried as a content hash and the tag map never consulted. + + Each is silently useless rather than obviously wrong, which is why they are refused here + rather than left to surprise someone later. ``latest`` is *not* rejected: whether it is + acceptable depends on the operation, so that check lives with the caller. + """ + if not tag.strip(): + raise ValueError("tag name must not be empty") + if not _TAG_NAME.fullmatch(tag): + raise ValueError( + f"tag {tag!r} contains characters that cannot appear in a reference fragment; such a " + "tag could be stored but never used to pin a member, because a ref's '#fragment' " + "admits only letters, digits, underscores, hyphens and dots" + ) + if is_digest(tag): + raise ValueError( + f"tag {tag!r} looks like a content digest; such a tag could be stored but never " + "resolved, because a digest-shaped reference is looked up as a digest, not as a tag" + ) + return tag + + +def validate_movable_tag(tag: str) -> str: + """Validate a tag a caller is asking to *point somewhere* by hand. + + Adds the ``latest`` restriction on top of :func:`validate_tag_name`: ``latest`` is + machine-managed and only ever advances on publish, so moving it manually would break the + forward-only guarantee that keeps concurrent publishes consistent. + """ + if tag == LATEST_TAG: + raise ValueError( + f"{LATEST_TAG!r} is managed automatically and always names the most recently published " + "revision; it cannot be moved by hand" + ) + return validate_tag_name(tag) + + +def is_digest(fragment: str) -> bool: + """Whether a ref fragment is a content digest rather than a tag. + + Unambiguous by construction: a digest is exactly 64 lowercase hex chars, and tag names are + bound to the same charset a fragment allows, which cannot produce that shape by accident. + + ``fullmatch`` rather than ``match``: Python's ``$`` also matches before a trailing newline, so + ``match`` would classify a 64-hex string with a newline glued on as a digest and send it down + the query path instead of the tag path. + """ + return bool(_DIGEST_FRAGMENT.fullmatch(fragment)) + + +async def find_by_digest( + entity_client: RevisionStoreProtocol[RevisionT], + revision_type: type[RevisionT], + head: TaskEntity | TasksetEntity, + digest: str, +) -> RevisionT | None: + """Find this record's newest revision with a given content digest, or ``None``. + + Scoped by parent as well as digest: identical content published under two different records + yields the same digest, so the digest alone does not identify a revision. + + One record *can* hold two revisions with the same digest — reverting to earlier content + republishes it — so this orders newest-first rather than taking whichever row came back. The + two are interchangeable in content, which is all a digest pin promises, but a pin should not + resolve to a different ordinal from one call to the next. + """ + result = await entity_client.list( + revision_type, + workspace=head.workspace, + filter_operation=LogicalOperation( + operator=FilterOperator.AND, + operations=[ + ComparisonOperation(field="parent", operator=FilterOperator.EQ, value=head.id), + ComparisonOperation(field="data.content_hash", operator=FilterOperator.EQ, value=digest), + ], + ), + sort="-created_at", + page_size=1, + ) + return result.data[0] if result.data else None + + +async def _revision_at( + revision_client: RevisionStoreProtocol[RevisionT], + revision_type: type[RevisionT], + head: TaskEntity | TasksetEntity, + ordinal: int, +) -> RevisionT | None: + """This record's revision ``ordinal``, or ``None`` if no such child exists. + + A direct child lookup rather than a query: a revision's identity within its parent is its + ordinal, so the name is already known. + """ + try: + return await revision_client.get( + revision_type, name=revision_name(ordinal), workspace=head.workspace, parent=head.id + ) + except NemoEntityNotFoundError: + return None + + +async def _current_revision( + revision_client: RevisionStoreProtocol[RevisionT], + revision_type: type[RevisionT], + head: TaskEntity | TasksetEntity, +) -> RevisionT | None: + """The revision ``latest`` names, or ``None`` if this record has none yet. + + ``None`` also covers the record whose ``latest`` names a revision that is missing: publishing + is then the recovery, not an error to propagate. + """ + ordinal = head.tags.get(LATEST_TAG) + if ordinal is None: + return None + return await _revision_at(revision_client, revision_type, head, ordinal) + + +async def get_revision( + entity_client: RevisionStoreProtocol[RevisionT], + revision_type: type[RevisionT], + head: TaskEntity | TasksetEntity, + fragment: str = LATEST_TAG, +) -> RevisionT: + """Fetch the revision a ref fragment names, verifying its content on the way out. + + A tag resolves through ``tags`` to an ordinal and then a direct child lookup — no query. A + digest is queried by ``(parent, content_hash)``. Either way the content that comes back is + re-hashed and compared against the digest stored beside it, because a pinned ref is a claim + about what the consumer will get and this is the one place that claim can be checked. Without + it the digest is only a label: a revision mutated in place would keep resolving and quietly + serve content nobody pinned. + """ + if is_digest(fragment): + revision = await find_by_digest(entity_client, revision_type, head, fragment) + if revision is None: + raise RevisionNotFoundError(f"'{head.workspace}/{head.name}' has no revision with digest {fragment!r}") + _verify_content(head, revision) + return revision + + ordinal = head.tags.get(fragment) + if ordinal is None: + known = ", ".join(sorted(head.tags)) or "none" + raise RevisionNotFoundError( + f"'{head.workspace}/{head.name}' has no revision tagged {fragment!r} (known tags: {known})" + ) + try: + revision = await entity_client.get( + revision_type, name=revision_name(ordinal), workspace=head.workspace, parent=head.id + ) + except NemoEntityNotFoundError as exc: + raise RevisionNotFoundError( + f"'{head.workspace}/{head.name}' tags {fragment!r} as revision {ordinal}, but that " + "revision record is missing" + ) from exc + _verify_content(head, revision) + return revision + + +def _verify_content(head: TaskEntity | TasksetEntity, revision: TaskRevisionEntity | TasksetRevisionEntity) -> None: + """Re-hash a revision's content and check it against the digest stored alongside it. + + A revision is immutable by convention, not by enforcement — the store will happily accept a + 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) + if actual != revision.content_hash: + raise RevisionContentMismatchError( + f"revision {revision.revision} of '{head.workspace}/{head.name}' does not match its " + f"recorded digest (recorded {revision.content_hash}, actual {actual}); the stored " + "content has been modified since it was published" + ) + + +async def list_revisions( + entity_client: RevisionStoreProtocol[RevisionT], + revision_type: type[RevisionT], + head: TaskEntity | TasksetEntity, + *, + page: int = 1, + page_size: int = 100, +) -> ListResponse[RevisionT]: + """Return a page of a record's published revisions, newest first. + + Scoped by parent, so it returns this record's history rather than every revision of every + record in the workspace. + + Ordering is server-side on ``-created_at`` rather than sorted client-side by ordinal: sorting + one page locally would order *within* the page while paging silently returned an arbitrary + slice. Creation order matches ordinal order because ordinal allocation is serialized by the + child create, so the two agree. + + Returns the store's ``ListResponse`` — including pagination counts — so a caller can tell a + complete history from a truncated one. Returning a bare list would make a capped result + indistinguishable from the whole thing. + """ + return await entity_client.list( + revision_type, + workspace=head.workspace, + filter_operation=ComparisonOperation(field="parent", operator=FilterOperator.EQ, value=head.id), + sort="-created_at", + page=page, + page_size=page_size, + ) + + +async def apply_tag( + head_client: HeadStoreProtocol[HeadT], + revision_client: RevisionStoreProtocol[RevisionT], + revision_type: type[RevisionT], + head: HeadT, + tag: str, + fragment: str, +) -> HeadT: + """Point an existing tag at the revision ``fragment`` names. + + Separate from publishing because a tag is not always applied at publish time — blessing a + revision after it has been evaluated is the common case, and Harbor exposes the same operation + (``tag_package_version``) independently of publish. + + ``latest`` is refused: it is machine-managed and moves only forward, on publish. Letting a + caller point it at an arbitrary revision would make it disagree with ``latest_revision`` and + break the forward-only guarantee that keeps concurrent publishes consistent. + """ + validate_movable_tag(tag) + revision = await get_revision(revision_client, revision_type, head, fragment) + return await _point_tags(head_client, head, {tag}, revision.revision) + + +async def publish_revision( + head_client: HeadStoreProtocol[HeadT], + revision_client: RevisionStoreProtocol[RevisionT], + head: HeadT, + revision_type: type[RevisionT], + *, + tags: set[str] | None = None, +) -> tuple[RevisionT, HeadT, bool]: + """Freeze a head record's current content as a revision and point tags at it. + + Returns ``(revision, head, created)``. The head is returned already carrying the new pointers, + so callers do not have to re-read it. ``created`` is ``False`` when the content was already + published — republishing identical content is a no-op that still applies any newly requested + tags, which is what makes a re-publish cheap and idempotent. + + ``latest`` is always applied, on top of any caller-supplied tags. + """ + # ``latest`` is applied regardless, so a caller who lists it explicitly is asking for what + # already happens — tolerated rather than rejected. Only *moving* latest by hand is refused + # (see :func:`validate_movable_tag`). + applied = {LATEST_TAG} | {validate_tag_name(tag) for tag in tags or set() if tag != LATEST_TAG} + + for attempt in range(_MAX_ALLOCATION_ATTEMPTS): + digest = head_digest(head) + + # Dedup against the revision ``latest`` names, *not* against any revision ever published. + # Matching any would make reverting to older content a no-op: the head would be rewritten + # with that content while ``latest`` — which only moves forward — kept naming the newer + # revision, so a plain read and a ``#latest`` read would disagree. A revert is a genuine + # change to what this record currently is, so it publishes. + current = await _current_revision(revision_client, revision_type, head) + if current is not None and current.content_hash == digest: + if any(head.tags.get(tag) != current.revision for tag in applied): + head = await _point_tags(head_client, head, applied, current.revision) + return current, head, False + + ordinal = head.latest_revision + 1 + content = head.model_dump(exclude=set(REVISION_POINTER_FIELDS) | set(head.__base_fields__), mode="json") + revision = revision_type( + name=revision_name(ordinal), + workspace=head.workspace, + project=head.project, + content_hash=digest, + revision=ordinal, + **content, + ) + revision._parent = head.id + + try: + created_revision = await revision_client.create(revision) + except NemoEntityConflictError: + # Another publisher took this ordinal. Refresh only the allocation state — re-reading + # the whole head would replace the content the caller staged for publishing with + # whatever is currently stored, and we'd publish the wrong thing. + logger.info( + "Revision ordinal contended, retrying", + extra={"record": f"{head.workspace}/{head.name}", "ordinal": ordinal, "attempt": attempt + 1}, + ) + _refresh_pointers(head, await head_client.get(type(head), name=head.name, workspace=head.workspace)) + + # The winner may have published exactly what we are publishing. Two identical requests + # that overlap both compute this digest; the dedup check above missed it only because + # the winner's pointer write had not landed when we read ``latest``. Stepping past their + # ordinal here would cut a byte-identical duplicate and report ``created`` to both + # callers, so idempotency has to be re-checked against the revision that actually beat + # us rather than against a head that may still be stale. + contended = await _revision_at(revision_client, revision_type, head, ordinal) + if contended is not None and contended.content_hash == digest: + if any(head.tags.get(tag) != contended.revision for tag in applied): + head = await _point_tags(head_client, head, applied, contended.revision) + return contended, head, False + + # The refresh earlier adopts the *stored* allocation state, which does not necessarily + # account for the ordinal we just lost: if the winner's own pointer write never landed, + # the stored head still names N-1 and we would recompute N and lose again, every attempt + # until the retries run out — and every later publish would do the same, leaving the + # record permanently unpublishable. The failed create is itself proof that N is taken, + # so step past it explicitly rather than trusting the head to say so. + head.latest_revision = max(head.latest_revision, ordinal) + continue + + # If the pointer write below exhausts its retries, the revision child still exists while + # `latest_revision` never advanced. That is safe but wasteful: the next publish computes the + # same ordinal, loses the create, and — via the step-past above — retries onto N+1. It + # self-heals rather than corrupting anything, so it is left alone deliberately. + head = await _point_tags(head_client, head, applied, ordinal) + return created_revision, head, True + + raise RevisionConflictError( + f"could not allocate a revision ordinal for '{head.workspace}/{head.name}' after " + f"{_MAX_ALLOCATION_ATTEMPTS} attempts" + ) + + +def _refresh_pointers(head: HeadT, fresh: HeadT) -> HeadT: + """Adopt another writer's revision bookkeeping without touching our content. + + A publish carries the caller's staged content; only the allocation state (which ordinals exist + and the version we're writing against) can go stale underneath it. So a retry refreshes exactly + that and leaves the content alone. + """ + head.latest_revision = fresh.latest_revision + head.tags = fresh.tags + head._db_version = fresh._db_version + return head + + +def _apply_pointers(head: HeadT, tags: set[str], ordinal: int) -> HeadT: + """Fold one publish's pointers into a head record (pure; no I/O). + + ``latest`` moves **forward only**. Two publishers can create their revisions in one order and + reach the head update in the other; without this guard the second writer's ``latest`` wins even + though it names the older revision, leaving ``latest`` and ``latest_revision`` disagreeing. + ``latest`` is machine-managed, so forward-only is the right rule. User tags are explicit intent + and may legitimately be moved backwards — retagging ``blessed`` onto an older revision is a + rollback, not a race. + """ + moves_forward = ordinal >= head.tags.get(LATEST_TAG, 0) + applied = {tag: ordinal for tag in tags if tag != LATEST_TAG or moves_forward} + head.tags = {**head.tags, **applied} + head.latest_revision = max(head.latest_revision, ordinal) + return head + + +async def _point_tags( + head_client: HeadStoreProtocol[HeadT], + head: HeadT, + tags: set[str], + ordinal: int, +) -> HeadT: + """Point tags at a revision ordinal, under the head's optimistic lock. + + The read-modify-write is retried on conflict rather than propagated: losing this race means + another publisher updated the head between our read and our write, and their pointers must be + preserved — so we re-read *their* head and fold our pointers into it, instead of overwriting + with a copy that predates them. + + Which content survives depends on who is newer. If the revision we are pointing at is the + newest, the head takes our content. If the winner published a *later* revision, the head has to + keep theirs: writing ours would leave the head serving one revision's content while ``latest`` + named another, which is the same divergence a plain read and a ``#latest`` read must never show. + Our non-``latest`` tags still apply either way — pointing ``blessed`` at an older revision is + legitimate; dragging the head's content back with it is not. + """ + for attempt in range(_MAX_ALLOCATION_ATTEMPTS): + try: + return await head_client.update(_apply_pointers(head, tags, ordinal)) + except NemoEntityConflictError: + logger.info( + "Head pointer update contended, re-reading", + extra={"record": f"{head.workspace}/{head.name}", "attempt": attempt + 1}, + ) + fresh = await head_client.get(type(head), name=head.name, workspace=head.workspace) + if ordinal < fresh.tags.get(LATEST_TAG, 0): + head = fresh + else: + _refresh_pointers(head, fresh) + raise RevisionConflictError( + f"could not update revision pointers for '{head.workspace}/{head.name}' after " + f"{_MAX_ALLOCATION_ATTEMPTS} attempts" + ) diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/sdk/http_utils.py b/plugins/nemo-evaluator/src/nemo_evaluator/sdk/http_utils.py index c93c8be7ba..75e6a06c75 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/sdk/http_utils.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/sdk/http_utils.py @@ -42,6 +42,22 @@ def url(platform: PlatformClient, path: str, workspace: str | None = None) -> st return _join_url(str(platform.base_url), f"{_API_PREFIX}/{resolved_path}") +def revision_selector(revision: str | None, tag: str | None) -> str | None: + """Encode a revision selector for a ``/revisions/{selector}`` path segment. + + The route takes one selector that may be either a digest or a tag, but the SDK splits it into + two named arguments. One argument would mean writing ``revision="blessed"`` to read a tag, which + reads as a contradiction at the call site even though the server resolves it happily; two names + make the caller's intent explicit and cost nothing, since both still resolve server-side. + + Returns ``None`` when neither is given — the caller then reads current content instead. + """ + if revision is not None and tag is not None: + raise ValueError("pass either 'revision' (a content digest) or 'tag', not both") + selector = revision if revision is not None else tag + return None if selector is None else quote(selector, safe="") + + def platform_default_headers(platform: PlatformClient) -> dict[str, str]: """Return string-valued default platform headers for direct evaluator HTTP calls.""" return {str(key): value for key, value in platform.default_headers.items() if isinstance(value, str)} diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/sdk/task_resources.py b/plugins/nemo-evaluator/src/nemo_evaluator/sdk/task_resources.py index 1973c0f419..f80556017d 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/sdk/task_resources.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/sdk/task_resources.py @@ -12,7 +12,7 @@ from urllib.parse import quote -from nemo_evaluator.api.schemas import Task, TaskInput +from nemo_evaluator.api.schemas import Revision, Task, TaskInput from nemo_evaluator.sdk import http_utils from nemo_platform import AsyncNeMoPlatform, NeMoPlatform from nemo_platform_plugin.schema import Page @@ -53,14 +53,69 @@ def create(self, name: str, *, task: TaskInput, project: str | None = None, work response.raise_for_status() return Task.model_validate(response.json()) - def retrieve(self, name: str, *, workspace: str | None = None) -> Task: - """Get a stored task by name.""" + def replace(self, name: str, *, task: TaskInput, project: str | None = None, workspace: str | None = None) -> Task: + """Publish a revision of a task, creating it if absent. + + Upsert, so a publisher needs no existence check. Submitting content identical to the current + revision publishes nothing and returns the task unchanged. + + The response body is the same either way, so this does not report whether a revision was + cut — the server signals that with 201 vs 200, which is discarded here. Compare the returned + ``revision`` against a prior read if you need to know.""" + response = self._http_client.put( + self._item_url(name, workspace), + json=task.model_dump(mode="json"), + params={"project": project} if project is not None else None, + headers=self._headers(), + timeout=self._platform.timeout, + ) + response.raise_for_status() + return Task.model_validate(response.json()) + + def list_revisions( + self, name: str, *, page: int = 1, page_size: int = 100, workspace: str | None = None + ) -> Page[Revision]: + """List a task's published revisions, newest first.""" response = self._http_client.get( - self._item_url(name, workspace), headers=self._headers(), timeout=self._platform.timeout + f"{self._item_url(name, workspace)}/revisions", + params={"page": page, "page_size": page_size}, + headers=self._headers(), + timeout=self._platform.timeout, + ) + response.raise_for_status() + return Page[Revision].model_validate(response.json()) + + def tag(self, name: str, *, tag: str, revision: str, workspace: str | None = None) -> Task: + """Point ``tag`` at an existing revision, named by digest or by another tag. + + Both selectors are keyword-only: ``tag`` names the pointer being written and ``revision`` + names what it points at, and two bare strings in a row gave no hint which was which. + """ + response = self._http_client.put( + f"{self._item_url(name, workspace)}/tags/{quote(tag, safe='')}", + params={"revision": revision}, + headers=self._headers(), + timeout=self._platform.timeout, ) response.raise_for_status() return Task.model_validate(response.json()) + def retrieve( + self, name: str, *, revision: str | None = None, tag: str | None = None, workspace: str | None = None + ) -> Task: + """Get a stored task by name, or as of a published revision. + + Pass ``revision`` for a content digest or ``tag`` for a named pointer — not both. With + neither, this returns the task's current content. + """ + url = self._item_url(name, workspace) + selector = http_utils.revision_selector(revision, tag) + if selector is not None: + url = f"{url}/revisions/{selector}" + response = self._http_client.get(url, headers=self._headers(), timeout=self._platform.timeout) + response.raise_for_status() + return Task.model_validate(response.json()) + def list( self, *, workspace: str | None = None, page: int = 1, page_size: int = 100, sort: str | None = None ) -> Page[Task]: @@ -112,14 +167,71 @@ async def create( response.raise_for_status() return Task.model_validate(response.json()) - async def retrieve(self, name: str, *, workspace: str | None = None) -> Task: - """Get a stored task by name.""" + async def replace( + self, name: str, *, task: TaskInput, project: str | None = None, workspace: str | None = None + ) -> Task: + """Publish a revision of a task, creating it if absent. + + Upsert, so a publisher needs no existence check. Submitting content identical to the current + revision publishes nothing and returns the task unchanged. + + The response body is the same either way, so this does not report whether a revision was + cut — the server signals that with 201 vs 200, which is discarded here. Compare the returned + ``revision`` against a prior read if you need to know.""" + response = await self._http_client.put( + self._item_url(name, workspace), + json=task.model_dump(mode="json"), + params={"project": project} if project is not None else None, + headers=self._headers(), + timeout=self._platform.timeout, + ) + response.raise_for_status() + return Task.model_validate(response.json()) + + async def list_revisions( + self, name: str, *, page: int = 1, page_size: int = 100, workspace: str | None = None + ) -> Page[Revision]: + """List a task's published revisions, newest first.""" response = await self._http_client.get( - self._item_url(name, workspace), headers=self._headers(), timeout=self._platform.timeout + f"{self._item_url(name, workspace)}/revisions", + params={"page": page, "page_size": page_size}, + headers=self._headers(), + timeout=self._platform.timeout, + ) + response.raise_for_status() + return Page[Revision].model_validate(response.json()) + + async def tag(self, name: str, *, tag: str, revision: str, workspace: str | None = None) -> Task: + """Point ``tag`` at an existing revision, named by digest or by another tag. + + Both selectors are keyword-only: ``tag`` names the pointer being written and ``revision`` + names what it points at, and two bare strings in a row gave no hint which was which. + """ + response = await self._http_client.put( + f"{self._item_url(name, workspace)}/tags/{quote(tag, safe='')}", + params={"revision": revision}, + headers=self._headers(), + timeout=self._platform.timeout, ) response.raise_for_status() return Task.model_validate(response.json()) + async def retrieve( + self, name: str, *, revision: str | None = None, tag: str | None = None, workspace: str | None = None + ) -> Task: + """Get a stored task by name, or as of a published revision. + + Pass ``revision`` for a content digest or ``tag`` for a named pointer — not both. With + neither, this returns the task's current content. + """ + url = self._item_url(name, workspace) + selector = http_utils.revision_selector(revision, tag) + if selector is not None: + url = f"{url}/revisions/{selector}" + response = await self._http_client.get(url, headers=self._headers(), timeout=self._platform.timeout) + response.raise_for_status() + return Task.model_validate(response.json()) + async def list( self, *, workspace: str | None = None, page: int = 1, page_size: int = 100, sort: str | None = None ) -> Page[Task]: diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/sdk/taskset_resources.py b/plugins/nemo-evaluator/src/nemo_evaluator/sdk/taskset_resources.py index 8526992fd9..37c42e045d 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/sdk/taskset_resources.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/sdk/taskset_resources.py @@ -12,7 +12,7 @@ from urllib.parse import quote -from nemo_evaluator.api.schemas import Taskset, TasksetInput +from nemo_evaluator.api.schemas import Revision, Taskset, TasksetInput from nemo_evaluator.sdk import http_utils from nemo_platform import AsyncNeMoPlatform, NeMoPlatform from nemo_platform_plugin.schema import Page @@ -57,12 +57,69 @@ def create( response.raise_for_status() return Taskset.model_validate(response.json()) - def retrieve(self, name: str, *, workspace: str | None = None) -> Taskset: - """Get a stored taskset by name.""" + def replace( + self, name: str, *, taskset: TasksetInput, project: str | None = None, workspace: str | None = None + ) -> Taskset: + """Publish a revision of a taskset, creating it if absent. + + Members are re-resolved to exact revision digests on every call, so identical member names + can still publish a new revision if a member task published in the meantime. + + The response body is the same either way, so this does not report whether a revision was + cut — the server signals that with 201 vs 200, which is discarded here. Compare the returned + ``revision`` against a prior read if you need to know.""" + response = self._http_client.put( + self._item_url(name, workspace), + json=taskset.model_dump(mode="json"), + params={"project": project} if project is not None else None, + headers=self._headers(), + timeout=self._platform.timeout, + ) + response.raise_for_status() + return Taskset.model_validate(response.json()) + + def list_revisions( + self, name: str, *, page: int = 1, page_size: int = 100, workspace: str | None = None + ) -> Page[Revision]: + """List a taskset's published revisions, newest first.""" response = self._http_client.get( - self._item_url(name, workspace), headers=self._headers(), timeout=self._platform.timeout + f"{self._item_url(name, workspace)}/revisions", + params={"page": page, "page_size": page_size}, + headers=self._headers(), + timeout=self._platform.timeout, ) response.raise_for_status() + return Page[Revision].model_validate(response.json()) + + def tag(self, name: str, *, tag: str, revision: str, workspace: str | None = None) -> Taskset: + """Point ``tag`` at an existing revision, named by digest or by another tag. + + Both selectors are keyword-only: ``tag`` names the pointer being written and ``revision`` + names what it points at, and two bare strings in a row gave no hint which was which. + """ + response = self._http_client.put( + f"{self._item_url(name, workspace)}/tags/{quote(tag, safe='')}", + params={"revision": revision}, + headers=self._headers(), + timeout=self._platform.timeout, + ) + response.raise_for_status() + return Taskset.model_validate(response.json()) + + def retrieve( + self, name: str, *, revision: str | None = None, tag: str | None = None, workspace: str | None = None + ) -> Taskset: + """Get a stored taskset by name, or as of a published revision. + + Pass ``revision`` for a content digest or ``tag`` for a named pointer — not both. With + neither, this returns the taskset's current membership. + """ + url = self._item_url(name, workspace) + selector = http_utils.revision_selector(revision, tag) + if selector is not None: + url = f"{url}/revisions/{selector}" + response = self._http_client.get(url, headers=self._headers(), timeout=self._platform.timeout) + response.raise_for_status() return Taskset.model_validate(response.json()) def list( @@ -118,12 +175,69 @@ async def create( response.raise_for_status() return Taskset.model_validate(response.json()) - async def retrieve(self, name: str, *, workspace: str | None = None) -> Taskset: - """Get a stored taskset by name.""" + async def replace( + self, name: str, *, taskset: TasksetInput, project: str | None = None, workspace: str | None = None + ) -> Taskset: + """Publish a revision of a taskset, creating it if absent. + + Members are re-resolved to exact revision digests on every call, so identical member names + can still publish a new revision if a member task published in the meantime. + + The response body is the same either way, so this does not report whether a revision was + cut — the server signals that with 201 vs 200, which is discarded here. Compare the returned + ``revision`` against a prior read if you need to know.""" + response = await self._http_client.put( + self._item_url(name, workspace), + json=taskset.model_dump(mode="json"), + params={"project": project} if project is not None else None, + headers=self._headers(), + timeout=self._platform.timeout, + ) + response.raise_for_status() + return Taskset.model_validate(response.json()) + + async def list_revisions( + self, name: str, *, page: int = 1, page_size: int = 100, workspace: str | None = None + ) -> Page[Revision]: + """List a taskset's published revisions, newest first.""" response = await self._http_client.get( - self._item_url(name, workspace), headers=self._headers(), timeout=self._platform.timeout + f"{self._item_url(name, workspace)}/revisions", + params={"page": page, "page_size": page_size}, + headers=self._headers(), + timeout=self._platform.timeout, ) response.raise_for_status() + return Page[Revision].model_validate(response.json()) + + async def tag(self, name: str, *, tag: str, revision: str, workspace: str | None = None) -> Taskset: + """Point ``tag`` at an existing revision, named by digest or by another tag. + + Both selectors are keyword-only: ``tag`` names the pointer being written and ``revision`` + names what it points at, and two bare strings in a row gave no hint which was which. + """ + response = await self._http_client.put( + f"{self._item_url(name, workspace)}/tags/{quote(tag, safe='')}", + params={"revision": revision}, + headers=self._headers(), + timeout=self._platform.timeout, + ) + response.raise_for_status() + return Taskset.model_validate(response.json()) + + async def retrieve( + self, name: str, *, revision: str | None = None, tag: str | None = None, workspace: str | None = None + ) -> Taskset: + """Get a stored taskset by name, or as of a published revision. + + Pass ``revision`` for a content digest or ``tag`` for a named pointer — not both. With + neither, this returns the taskset's current membership. + """ + url = self._item_url(name, workspace) + selector = http_utils.revision_selector(revision, tag) + if selector is not None: + url = f"{url}/revisions/{selector}" + response = await self._http_client.get(url, headers=self._headers(), timeout=self._platform.timeout) + response.raise_for_status() return Taskset.model_validate(response.json()) async def list( diff --git a/plugins/nemo-evaluator/src/nemo_evaluator/task_refs.py b/plugins/nemo-evaluator/src/nemo_evaluator/task_refs.py index 0ce535fdc5..589da09313 100644 --- a/plugins/nemo-evaluator/src/nemo_evaluator/task_refs.py +++ b/plugins/nemo-evaluator/src/nemo_evaluator/task_refs.py @@ -16,35 +16,51 @@ from __future__ import annotations -from nemo_evaluator.api.schemas import TasksetRef, parse_entity_ref -from nemo_evaluator.entities import TaskEntity, TasksetEntity +from typing import cast + +from nemo_evaluator.api.schemas import TasksetRef, parse_entity_ref, parse_subentity_ref +from nemo_evaluator.entities import TaskEntity, TaskRevisionEntity, TasksetEntity from nemo_evaluator.jobs.agent_spec import AgentEvalTaskInput -from nemo_platform_plugin.entity_client import NemoAnyEntityGetterProtocol, NemoEntityNotFoundError +from nemo_evaluator.revisions import RevisionNotFoundError, get_revision +from nemo_platform_plugin.entities import EntityClientProtocol +from nemo_platform_plugin.entity_client import NemoEntityNotFoundError + +def _entity_to_task_input(entity: TaskEntity, revision: TaskRevisionEntity) -> AgentEvalTaskInput: + """Project a stored task's *published revision* onto the submitter-facing inline task DTO. -def _entity_to_task_input(entity: TaskEntity) -> AgentEvalTaskInput: - """Project a stored task onto the submitter-facing inline task DTO. + Identity (``id``) comes from the head record — it is the same task — while every content field + comes from the revision the taskset pinned. That split is what makes a taskset-driven evaluation + reproducible: re-running it expands to the same content even if the member task has published + since. - The task's stable ``id`` is its record ``name``. 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. + 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. """ return AgentEvalTaskInput( id=entity.name, - intent=entity.intent, - inputs=entity.inputs, - metrics=list(entity.metrics), - views=entity.views, - metadata=entity.metadata, + intent=revision.intent, + inputs=revision.inputs, + metrics=list(revision.metrics), + views=revision.views, + metadata=revision.metadata, ) +#: Expanding a taskset reads three entity types through one client — the taskset head, each member +#: task's head, and the pinned revision of each member. Python has no intersection types, so the +#: parameter is annotated at one of them and the other two are taken as typed views of the same +#: object; the concrete client's methods are generic over the entity type and satisfy all three. +TasksetStoreProtocol = EntityClientProtocol[TasksetEntity] + + async def resolve_taskset_ref( ref: TasksetRef, *, workspace: str, - entity_client: NemoAnyEntityGetterProtocol | None, + entity_client: TasksetStoreProtocol | None, ) -> list[AgentEvalTaskInput]: """Load a stored taskset and expand its members into inline task DTOs. @@ -56,6 +72,9 @@ async def resolve_taskset_ref( "A TasksetRef requires a platform connection (entity store) to resolve; it cannot be used " "in local execution. Pass an inline task list instead." ) + task_store = cast(EntityClientProtocol[TaskEntity], entity_client) + revision_store = cast(EntityClientProtocol[TaskRevisionEntity], entity_client) + ref_workspace, name = parse_entity_ref(ref.root, workspace) try: taskset = await entity_client.get(TasksetEntity, name=name, workspace=ref_workspace) @@ -72,14 +91,24 @@ async def resolve_taskset_ref( tasks: list[AgentEvalTaskInput] = [] seen_ids: set[str] = set() for task_ref in taskset.tasks: - task_workspace, task_name = parse_entity_ref(task_ref.root, ref_workspace) + task_workspace, task_name, fragment = parse_subentity_ref(task_ref.root, ref_workspace) try: - entity = await entity_client.get(TaskEntity, name=task_name, workspace=task_workspace) + entity = await task_store.get(TaskEntity, name=task_name, workspace=task_workspace) except NemoEntityNotFoundError as exc: raise ValueError( f"Task '{task_ref.root}' referenced by taskset '{ref.root}' was not found; " "the stored task may have been deleted after the taskset was created." ) from exc + # Expand the *pinned* revision, not the task's current content. A published taskset names + # exact revisions; resolving to whatever is current would silently defeat the pinning and + # make an evaluation irreproducible the moment a member republished. + try: + revision = await get_revision(revision_store, TaskRevisionEntity, entity, fragment) + except RevisionNotFoundError as exc: + raise ValueError( + f"Task '{task_ref.root}' referenced by taskset '{ref.root}' names a revision that no " + f"longer resolves: {exc}" + ) from exc # Agent-eval task ids must be unique within a run. Member refs are unique per (workspace, # name), but refs from different workspaces can share a name — surface that as a clear error # rather than letting the SDK evaluator reject duplicate ids deeper in the run. @@ -89,7 +118,7 @@ async def resolve_taskset_ref( "task ids must be unique within an evaluation." ) seen_ids.add(entity.name) - tasks.append(_entity_to_task_input(entity)) + tasks.append(_entity_to_task_input(entity, revision)) return tasks @@ -97,7 +126,7 @@ async def resolve_agent_eval_tasks( tasks: TasksetRef | list[AgentEvalTaskInput], *, workspace: str, - entity_client: NemoAnyEntityGetterProtocol | None, + entity_client: TasksetStoreProtocol | None, ) -> list[AgentEvalTaskInput]: """Normalize an agent-eval ``tasks`` field to an inline task list. 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 6287817f27..10913536a3 100644 --- a/plugins/nemo-evaluator/tests/api/service/test_task_service.py +++ b/plugins/nemo-evaluator/tests/api/service/test_task_service.py @@ -3,16 +3,12 @@ from __future__ import annotations -from datetime import datetime, timezone - import pytest from nemo_evaluator.api.schemas import MetadataItem, MetricInline, MetricRef, Task, TaskInput, TaskInputs from nemo_evaluator.api.service.task_service import MetricRefNotFoundError, TaskService -from nemo_evaluator.entities import TaskEntity 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 -from nemo_platform_plugin.entities import ListResponse, PaginationInfo from nemo_platform_plugin.entity_client import NemoEntityConflictError, NemoEntityNotFoundError @@ -39,68 +35,6 @@ def _inline_metric() -> MetricInline: return MetricInline.model_validate(bundle.model_dump(mode="json")) -class _FakeEntityClient: - def __init__(self) -> None: - self.entities: dict[tuple[str, str, str], TaskEntity] = {} - - async def create(self, entity: TaskEntity) -> TaskEntity: - key = (entity.__entity_type__, entity.workspace, entity.name) - if key in self.entities: - raise NemoEntityConflictError(f"{key} exists") - now = datetime.now(timezone.utc) - entity._id = f"{entity.__entity_type__}-{entity.name}" - entity._created_at = now - entity._updated_at = now - self.entities[key] = entity - return entity - - async def get( - self, entity_type: type[TaskEntity], *, workspace: str, name: str, parent: str | None = None - ) -> TaskEntity: - key = (entity_type.__entity_type__, workspace, name) - if key not in self.entities: - raise NemoEntityNotFoundError(f"{workspace}/{name} not found") - return self.entities[key] - - async def delete( - self, - entity_type: type[TaskEntity], - name: str, - *, - workspace: str, - parent: str | None = None, - expected_db_version: int | None = None, - ) -> None: - key = (entity_type.__entity_type__, workspace, name) - if key not in self.entities: - raise NemoEntityNotFoundError(f"{workspace}/{name} not found") - del self.entities[key] - - async def list( - self, - entity_type: type[TaskEntity], - *, - workspace: str, - filter_operation: object | None = None, - sort: str | None = None, - page: int = 1, - page_size: int = 100, - ) -> ListResponse[TaskEntity]: - items = [ - e for (etype, ws, _), e in self.entities.items() if etype == entity_type.__entity_type__ and ws == workspace - ] - return ListResponse( - data=items, - pagination=PaginationInfo( - page=page, - page_size=page_size, - current_page_size=len(items), - total_pages=1, - total_results=len(items), - ), - ) - - def _task_input() -> TaskInput: return TaskInput( intent="Answer the question.", @@ -116,12 +50,12 @@ def metric_service() -> _FakeMetricService: @pytest.fixture -def service(metric_service: _FakeMetricService) -> TaskService: - return TaskService(_FakeEntityClient(), metric_service) +def service(metric_service: _FakeMetricService, entity_store) -> TaskService: + return TaskService(entity_store, metric_service) async def test_create_then_get(service: TaskService) -> None: - created = await service.create_task("task-1", _task_input(), workspace="default") + created, _ = await service.create_task("task-1", _task_input(), workspace="default") assert isinstance(created, Task) assert created.name == "task-1" @@ -144,7 +78,7 @@ async def test_create_normalizes_inline_metrics_to_refs( metrics=[MetricRef("default/stored-metric"), inline], ) - created = await service.create_task("task-1", task_input, workspace="default") + created, _ = await service.create_task("task-1", task_input, workspace="default") # The inline metric was offloaded to the metric service (stored as a derived metric)... assert metric_service.stored == [inline] @@ -163,7 +97,7 @@ async def test_create_rejects_missing_metric_ref(service: TaskService) -> None: 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")]) - created = await service.create_task("task-1", task_input, workspace="default") + created, _ = await service.create_task("task-1", task_input, workspace="default") assert created.metrics[0].root == "default/stored-metric" @@ -195,3 +129,140 @@ async def test_delete(service: TaskService) -> None: async def test_delete_returns_false_when_missing(service: TaskService) -> None: assert await service.delete_task("default", "nope") is False + + +# --- Failure handling --------------------------------------------------------- + + +async def test_create_rolls_back_the_head_when_publishing_fails( + metric_service: _FakeMetricService, entity_store +) -> None: + """A head with no revision would break the invariant every consumer relies on — `#latest` + always resolves and `revision` is never 0. There is no cross-entity transaction, so create + must undo itself rather than leave a half-created task behind.""" + service = TaskService(entity_store, metric_service) + + real_create = type(entity_store).create + + async def _boom(entity): + if entity.__entity_type__ == "task_revision": + raise RuntimeError("store unavailable") + return await real_create(entity_store, entity) + + entity_store.create = _boom + + with pytest.raises(RuntimeError): + await service.create_task("task-1", _task_input(), workspace="default") + + entity_store.create = real_create.__get__(entity_store) + assert await service.get_task("default", "task-1") is None, "the orphaned head must not survive" + + +async def test_replace_propagates_a_concurrent_write_conflict(metric_service: _FakeMetricService, entity_store) -> None: + """Losing the optimistic lock is a client-retryable conflict, not a server fault — the route + maps this to 409, so the service must let it through rather than swallowing it.""" + service = TaskService(entity_store, metric_service) + await service.create_task("task-1", _task_input(), workspace="default") + + async def _stale(entity, *, original_name=None): + raise NemoEntityConflictError("modified by another request") + + entity_store.update = _stale + + with pytest.raises(NemoEntityConflictError): + await service.replace_task("task-1", _task_input(), workspace="default") + + +async def test_replace_leaves_no_uncovered_head_content_when_publishing_fails( + metric_service: _FakeMetricService, entity_store +) -> None: + """A failed publish must not leave the head serving content no revision covers. + + Replace stages content in memory and lets publishing commit it, so a publish that never + happens leaves nothing behind. Committing the head first would instead make a plain GET — + which reads the head — return content that `#latest` does not resolve to. + """ + service = TaskService(entity_store, metric_service) + await service.create_task("task-1", _task_input(), workspace="default") + + real_create = type(entity_store).create + + async def _boom(entity): + if entity.__entity_type__ == "task_revision": + raise RuntimeError("store unavailable") + return await real_create(entity_store, entity) + + entity_store.create = _boom + + changed = TaskInput( + intent="Rewritten.", inputs=TaskInputs(instruction="?"), metrics=[MetricRef("default/stored-metric")] + ) + with pytest.raises(RuntimeError): + await service.replace_task("task-1", changed, workspace="default") + + 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" + + +async def test_tag_revision_returns_none_for_a_missing_task(service: TaskService) -> None: + assert await service.tag_revision("default", "nope", "blessed", "latest") is None + + +async def test_list_revisions_returns_none_for_a_missing_task(service: TaskService) -> None: + assert await service.list_revisions("default", "nope") is None + + +async def test_replace_applies_project(service: TaskService) -> None: + """`project` used to be accepted on replace and silently discarded.""" + await service.create_task("task-1", _task_input(), workspace="default", project="proj-a") + replaced, _ = await service.replace_task("task-1", _task_input(), workspace="default", project="proj-b") + assert replaced.project == "proj-b" + + +async def test_replace_without_project_leaves_it_unchanged(service: TaskService) -> None: + """An omitted query parameter means "leave it alone", not "clear it".""" + await service.create_task("task-1", _task_input(), workspace="default", project="proj-a") + replaced, _ = await service.replace_task("task-1", _task_input(), workspace="default") + assert replaced.project == "proj-a" + + +async def test_rollback_failure_does_not_mask_the_original_error( + metric_service: _FakeMetricService, entity_store +) -> None: + """If cleanup also fails, the caller must still see *why* the publish failed.""" + service = TaskService(entity_store, metric_service) + real_create = type(entity_store).create + + async def _boom(entity): + if entity.__entity_type__ == "task_revision": + raise RuntimeError("the original failure") + return await real_create(entity_store, entity) + + async def _delete_also_fails(*args, **kwargs): + raise RuntimeError("rollback failed too") + + entity_store.create = _boom + entity_store.delete = _delete_also_fails + + with pytest.raises(RuntimeError, match="the original failure"): + await service.create_task("task-1", _task_input(), workspace="default") + + +async def test_resolve_revision_returns_the_digest_for_a_tag(service: TaskService) -> None: + """The hook taskset publishing uses to turn a member's tag into an exact digest.""" + await service.create_task("task-1", _task_input(), workspace="default") + + digest = await service.resolve_revision("default", "task-1") + + revisions = await service.list_revisions("default", "task-1") + assert revisions is not None + assert digest == revisions.data[0].content_hash + + +async def test_resolve_revision_raises_for_a_missing_task(service: TaskService) -> None: + """Existence surfaces from resolution itself — taskset publishing relies on this to reject a + member that does not exist, now that the separate existence check is gone.""" + with pytest.raises(NemoEntityNotFoundError): + await service.resolve_revision("default", "nope") diff --git a/plugins/nemo-evaluator/tests/api/service/test_taskset_service.py b/plugins/nemo-evaluator/tests/api/service/test_taskset_service.py index cb9c2c2550..ee6679fc2e 100644 --- a/plugins/nemo-evaluator/tests/api/service/test_taskset_service.py +++ b/plugins/nemo-evaluator/tests/api/service/test_taskset_service.py @@ -3,7 +3,8 @@ from __future__ import annotations -from datetime import datetime, timezone +import asyncio +import hashlib import pytest from nemo_evaluator.api.schemas import MetadataItem, TaskRef, Taskset, TasksetInput @@ -13,9 +14,8 @@ TasksetExistsError, TasksetService, ) -from nemo_evaluator.entities import TasksetEntity -from nemo_platform_plugin.entities import ListResponse, PaginationInfo -from nemo_platform_plugin.entity_client import NemoEntityConflictError, NemoEntityNotFoundError +from nemo_evaluator.revisions import RevisionNotFoundError +from nemo_platform_plugin.entity_client import NemoEntityNotFoundError class _FakeTaskService: @@ -27,67 +27,15 @@ def __init__(self, existing: set[tuple[str, str]]) -> None: async def get_task(self, workspace: str, name: str) -> object | None: return object() if (workspace, name) in self.existing else None + async def resolve_revision(self, workspace: str, name: str, fragment: str = "latest") -> str: + """A stable per-task digest, so pinned membership is deterministic across a test. -class _FakeEntityClient: - def __init__(self) -> None: - self.entities: dict[tuple[str, str, str], TasksetEntity] = {} - - async def create(self, entity: TasksetEntity) -> TasksetEntity: - key = (entity.__entity_type__, entity.workspace, entity.name) - if key in self.entities: - raise NemoEntityConflictError(f"{key} exists") - now = datetime.now(timezone.utc) - entity._id = f"{entity.__entity_type__}-{entity.name}" - entity._created_at = now - entity._updated_at = now - self.entities[key] = entity - return entity - - async def get( - self, entity_type: type[TasksetEntity], *, workspace: str, name: str, parent: str | None = None - ) -> TasksetEntity: - key = (entity_type.__entity_type__, workspace, name) - if key not in self.entities: + Raises for an unknown task, matching the real service: resolution fetches the task, so a + missing one surfaces here rather than from a separate existence check. + """ + if (workspace, name) not in self.existing: raise NemoEntityNotFoundError(f"{workspace}/{name} not found") - return self.entities[key] - - async def delete( - self, - entity_type: type[TasksetEntity], - name: str, - *, - workspace: str, - parent: str | None = None, - expected_db_version: int | None = None, - ) -> None: - key = (entity_type.__entity_type__, workspace, name) - if key not in self.entities: - raise NemoEntityNotFoundError(f"{workspace}/{name} not found") - del self.entities[key] - - async def list( - self, - entity_type: type[TasksetEntity], - *, - workspace: str, - filter_operation: object | None = None, - sort: str | None = None, - page: int = 1, - page_size: int = 100, - ) -> ListResponse[TasksetEntity]: - items = [ - e for (etype, ws, _), e in self.entities.items() if etype == entity_type.__entity_type__ and ws == workspace - ] - return ListResponse( - data=items, - pagination=PaginationInfo( - page=page, - page_size=page_size, - current_page_size=len(items), - total_pages=1, - total_results=len(items), - ), - ) + return hashlib.sha256(f"{workspace}/{name}".encode()).hexdigest() def _taskset_input() -> TasksetInput: @@ -104,18 +52,21 @@ def existing_tasks() -> set[tuple[str, str]]: @pytest.fixture -def service(existing_tasks: set[tuple[str, str]]) -> TasksetService: - return TasksetService(_FakeEntityClient(), _FakeTaskService(existing_tasks)) +def service(existing_tasks: set[tuple[str, str]], entity_store) -> TasksetService: + return TasksetService(entity_store, _FakeTaskService(existing_tasks)) async def test_create_then_get(service: TasksetService) -> None: - created = await service.create_taskset("ts-1", _taskset_input(), workspace="default") + created, _ = await service.create_taskset("ts-1", _taskset_input(), workspace="default") assert isinstance(created, Taskset) assert created.name == "ts-1" assert created.id == "taskset-ts-1" assert created.description == "A smoke-test grouping." - assert {t.root for t in created.tasks} == {"task-a", "default/task-b"} + # Members are stored workspace-qualified *and* digest-pinned: a bare "task-a" is resolved on + # write, so the stored grouping names exact revisions rather than moving targets. + assert {t.root.split("#")[0] for t in created.tasks} == {"default/task-a", "default/task-b"} + assert all(len(t.root.split("#")[1]) == 64 for t in created.tasks) assert created.created_at is not None got = await service.get_taskset("default", "ts-1") @@ -128,9 +79,11 @@ async def test_create_validates_missing_task_ref(service: TasksetService) -> Non await service.create_taskset("ts-1", taskset_input, workspace="default") -async def test_create_resolves_bare_ref_against_taskset_workspace(existing_tasks: set[tuple[str, str]]) -> None: +async def test_create_resolves_bare_ref_against_taskset_workspace( + existing_tasks: set[tuple[str, str]], entity_store +) -> None: # A bare "task-a" ref must resolve against the taskset's own workspace ("other"), where it is absent. - service = TasksetService(_FakeEntityClient(), _FakeTaskService(existing_tasks)) + service = TasksetService(entity_store, _FakeTaskService(existing_tasks)) with pytest.raises(ValueError, match="not found in workspace 'other'"): await service.create_taskset("ts-1", TasksetInput(tasks=[TaskRef("task-a")]), workspace="other") @@ -153,8 +106,8 @@ async def test_create_allows_same_name_in_different_workspaces(service: TasksetS # Taskset names are unique per workspace, not globally: the same name in another workspace is a # distinct taskset and must not raise TasksetExistsError (409). Empty task lists keep this focused # on name scoping rather than per-workspace task-ref validation. - first = await service.create_taskset("ts-1", TasksetInput(), workspace="default") - second = await service.create_taskset("ts-1", TasksetInput(), workspace="other") + first, _ = await service.create_taskset("ts-1", TasksetInput(), workspace="default") + second, _ = await service.create_taskset("ts-1", TasksetInput(), workspace="other") assert first.name == second.name == "ts-1" assert first.workspace == "default" @@ -183,3 +136,174 @@ async def test_delete(service: TasksetService) -> None: async def test_delete_returns_false_when_missing(service: TasksetService) -> None: assert await service.delete_taskset("default", "nope") is False + + +# --- Failure handling and membership resolution ------------------------------- + + +async def test_create_rolls_back_the_head_when_publishing_fails( + existing_tasks: set[tuple[str, str]], entity_store +) -> None: + """Mirrors the task-side rollback: a taskset head with no revision would break the invariant + that `#latest` always resolves.""" + service = TasksetService(entity_store, _FakeTaskService(existing_tasks)) + real_create = type(entity_store).create + + async def _boom(entity): + if entity.__entity_type__ == "taskset_revision": + raise RuntimeError("store unavailable") + return await real_create(entity_store, entity) + + entity_store.create = _boom + + with pytest.raises(RuntimeError): + await service.create_taskset("ts-1", _taskset_input(), workspace="default") + + entity_store.create = real_create.__get__(entity_store) + assert await service.get_taskset("default", "ts-1") is None + + +async def test_rollback_failure_does_not_mask_the_original_error( + existing_tasks: set[tuple[str, str]], entity_store +) -> None: + """If cleanup also fails, the caller must still see *why* the publish failed — otherwise they + debug the rollback instead of the actual fault.""" + service = TasksetService(entity_store, _FakeTaskService(existing_tasks)) + real_create = type(entity_store).create + + async def _boom(entity): + if entity.__entity_type__ == "taskset_revision": + raise RuntimeError("the original failure") + return await real_create(entity_store, entity) + + async def _delete_also_fails(*args, **kwargs): + raise RuntimeError("rollback failed too") + + entity_store.create = _boom + entity_store.delete = _delete_also_fails + + with pytest.raises(RuntimeError, match="the original failure"): + await service.create_taskset("ts-1", _taskset_input(), workspace="default") + + +async def test_member_naming_an_unknown_revision_is_rejected( + existing_tasks: set[tuple[str, str]], entity_store +) -> None: + """A member pinned to a revision that does not exist is a client error on the submitted body, + not a server fault — the route maps this to 422.""" + + class _NoSuchRevision(_FakeTaskService): + async def resolve_revision(self, workspace: str, name: str, fragment: str = "latest") -> str: + raise RevisionNotFoundError(f"no revision {fragment!r}") + + service = TasksetService(entity_store, _NoSuchRevision(existing_tasks)) + + with pytest.raises(TaskRefNotFoundError, match="no published revision"): + await service.create_taskset("ts-1", TasksetInput(tasks=[TaskRef(f"task-a#{'c' * 64}")]), workspace="default") + + +async def test_members_resolve_concurrently(existing_tasks: set[tuple[str, str]], entity_store) -> None: + """Membership resolution fans out rather than running one member at a time — a Harbor-scale + dataset names hundreds of tasks, and serial resolution made publish latency linear in size.""" + overlap = {"peak": 0, "current": 0} + + class _Tracking(_FakeTaskService): + async def resolve_revision(self, workspace: str, name: str, fragment: str = "latest") -> str: + overlap["current"] += 1 + overlap["peak"] = max(overlap["peak"], overlap["current"]) + await asyncio.sleep(0) # yield, so overlapping calls can interleave + overlap["current"] -= 1 + return hashlib.sha256(f"{workspace}/{name}".encode()).hexdigest() + + members = {("default", f"task-{i}") for i in range(5)} + service = TasksetService(entity_store, _Tracking(members)) + + await service.create_taskset( + "ts-1", TasksetInput(tasks=[TaskRef(f"task-{i}") for i in range(5)]), workspace="default" + ) + + assert overlap["peak"] > 1, "members must resolve concurrently, not one at a time" + + +async def test_membership_is_stored_in_canonical_order(existing_tasks: set[tuple[str, str]], entity_store) -> None: + """Membership is a set, so it is stored sorted rather than in submission order. + + The digest covers the stored list, so leaving order to the caller would give one grouping two + identities. Sorting also has to survive concurrent resolution, which completes out of order. + """ + members = {("default", f"task-{i}") for i in range(5)} + service = TasksetService(entity_store, _FakeTaskService(members)) + + created, _ = await service.create_taskset( + "ts-1", TasksetInput(tasks=[TaskRef(f"task-{i}") for i in reversed(range(5))]), workspace="default" + ) + + assert [t.root.split("#")[0] for t in created.tasks] == [f"default/task-{i}" for i in range(5)] + + +async def test_reordering_members_publishes_no_revision(existing_tasks: set[tuple[str, str]], entity_store) -> None: + """Re-submitting the same members in a different order is not a content change. + + This is the property canonical ordering exists for: without it the two requests hash + differently, and a caller that merely rebuilt its list from a set would cut a revision whose + content is indistinguishable from its predecessor's. + """ + members = {("default", f"task-{i}") for i in range(3)} + service = TasksetService(entity_store, _FakeTaskService(members)) + + created, _ = await service.create_taskset( + "ts-1", TasksetInput(tasks=[TaskRef(f"task-{i}") for i in range(3)]), workspace="default" + ) + replaced, published = await service.replace_taskset( + "ts-1", TasksetInput(tasks=[TaskRef(f"task-{i}") for i in reversed(range(3))]), workspace="default" + ) + + assert not published, "reordered membership is the same set, so nothing should be published" + assert replaced.revision == created.revision == 1 + + +async def test_tag_revision_returns_none_for_a_missing_taskset(service: TasksetService) -> None: + assert await service.tag_revision("default", "nope", "blessed", "latest") is None + + +async def test_list_revisions_returns_none_for_a_missing_taskset(service: TasksetService) -> None: + assert await service.list_revisions("default", "nope") is None + + +async def test_replace_applies_project(service: TasksetService) -> None: + """`project` used to be accepted and silently discarded on replace.""" + await service.create_taskset("ts-1", _taskset_input(), workspace="default", project="proj-a") + replaced, _ = await service.replace_taskset("ts-1", _taskset_input(), workspace="default", project="proj-b") + assert replaced.project == "proj-b" + + +async def test_replace_without_project_leaves_it_unchanged(service: TasksetService) -> None: + """An omitted query parameter means "leave it alone", not "clear it".""" + await service.create_taskset("ts-1", _taskset_input(), workspace="default", project="proj-a") + replaced, _ = await service.replace_taskset("ts-1", _taskset_input(), workspace="default") + assert replaced.project == "proj-a" + + +async def test_a_failed_member_cancels_its_siblings(existing_tasks: set[tuple[str, str]], entity_store) -> None: + """`gather` propagates the first failure but leaves siblings running; one bad member in a large + grouping would otherwise keep reading long after the request failed.""" + finished: list[str] = [] + + class _SlowExceptOne(_FakeTaskService): + async def resolve_revision(self, workspace: str, name: str, fragment: str = "latest") -> str: + if name == "task-0": + raise NemoEntityNotFoundError("missing") + await asyncio.sleep(0.05) + finished.append(name) + return hashlib.sha256(name.encode()).hexdigest() + + members = {("default", f"task-{i}") for i in range(5)} + service = TasksetService(entity_store, _SlowExceptOne(members)) + + with pytest.raises(TaskRefNotFoundError): + await service.create_taskset( + "ts-1", TasksetInput(tasks=[TaskRef(f"task-{i}") for i in range(5)]), workspace="default" + ) + + await asyncio.sleep(0.1) # give any un-cancelled sibling time to finish + assert finished == [], "siblings must be cancelled once a member has failed" 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 f249c52e7b..d8a71339d7 100644 --- a/plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py +++ b/plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py @@ -9,8 +9,6 @@ from __future__ import annotations -from datetime import datetime, timezone - import pytest from fastapi import FastAPI from fastapi.testclient import TestClient @@ -18,68 +16,7 @@ from nemo_evaluator.api.schemas import 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_evaluator.entities import TaskEntity -from nemo_platform_plugin.entities import ListResponse, PaginationInfo -from nemo_platform_plugin.entity_client import NemoEntityConflictError, NemoEntityNotFoundError -from nemo_platform_plugin.filter_ops import FilterOperation - - -class _FakeEntityClient: - def __init__(self) -> None: - self.entities: dict[tuple[str, str, str], TaskEntity] = {} - - async def create(self, entity: TaskEntity) -> TaskEntity: - key = (entity.__entity_type__, entity.workspace, entity.name) - if key in self.entities: - raise NemoEntityConflictError(f"{key} exists") - now = datetime.now(timezone.utc) - entity._id = f"{entity.__entity_type__}-{entity.name}" - entity._created_at = now - entity._updated_at = now - self.entities[key] = entity - return entity - - async def get( - self, entity_type: type[TaskEntity], *, workspace: str, name: str, parent: str | None = None - ) -> TaskEntity: - key = (entity_type.__entity_type__, workspace, name) - if key not in self.entities: - raise NemoEntityNotFoundError(f"{workspace}/{name} not found") - return self.entities[key] - - async def delete( - self, - entity_type: type[TaskEntity], - name: str, - *, - workspace: str, - parent: str | None = None, - expected_db_version: int | None = None, - ) -> None: - key = (entity_type.__entity_type__, workspace, name) - if key not in self.entities: - raise NemoEntityNotFoundError(f"{workspace}/{name} not found") - del self.entities[key] - - async def list( - self, - entity_type: type[TaskEntity], - *, - workspace: str, - filter_operation: FilterOperation | None = None, - sort: str | None = None, - page: int = 1, - page_size: int = 100, - ) -> ListResponse[TaskEntity]: - items = [ - e for (etype, ws, _), e in self.entities.items() if etype == entity_type.__entity_type__ and ws == workspace - ] - return ListResponse( - data=items, - pagination=PaginationInfo( - page=page, page_size=page_size, current_page_size=len(items), total_pages=1, total_results=len(items) - ), - ) +from nemo_platform_plugin.entity_client import NemoEntityConflictError class _FakeMetricService: @@ -94,19 +31,20 @@ async def get_metric(self, workspace: str, name: str) -> object | None: @pytest.fixture -def client() -> TestClient: +def client(entity_store) -> TestClient: app = FastAPI() app.include_router(tasks_routes.router, prefix="/v2/workspaces/{workspace}") - service = TaskService(_FakeEntityClient(), _FakeMetricService()) + service = TaskService(entity_store, _FakeMetricService()) app.dependency_overrides[get_task_service] = lambda: service return TestClient(app) -def _body() -> dict: +def _body(*, intent: str = "Answer the question.", tags: list[str] | None = None) -> dict: return TaskInput( - intent="Answer the question.", + intent=intent, inputs=TaskInputs(instruction="What is 2+2?"), metrics=[MetricRef("default/stored-metric")], + tags=tags or [], ).model_dump(mode="json") @@ -191,3 +129,145 @@ async def delete_task(self, workspace: str, name: str) -> bool: client = TestClient(app) assert client.delete(f"{_BASE}/task-1").status_code == 409 + + +# --- Publishing revisions (POST creates, PUT replaces) ------------------------ + + +def test_create_publishes_revision_one(client: TestClient) -> None: + """Every stored task has a revision from the moment it exists — there is no unpublished head, + so no consumer has to define what one would mean.""" + created = client.post(f"{_BASE}/task-1", json=_body()).json() + assert created["revision"] == 1 + assert created["tags"] == {"latest": 1} + + +def test_put_on_missing_task_creates_it(client: TestClient) -> None: + """Upsert, so a publisher makes one call without first checking existence — that check is both + a round trip and a race between two publishers of the same task.""" + response = client.put(f"{_BASE}/task-1", json=_body()) + assert response.status_code == 201 + assert response.json()["revision"] == 1 + + +def test_put_with_changed_content_publishes_a_new_revision(client: TestClient) -> None: + client.post(f"{_BASE}/task-1", json=_body()) + response = client.put(f"{_BASE}/task-1", json=_body(intent="Do something else.")) + assert response.status_code == 201 + assert response.json()["revision"] == 2 + assert response.json()["tags"]["latest"] == 2 + + +def test_put_with_identical_content_publishes_nothing(client: TestClient) -> None: + """Strict idempotency: PUT twice leaves exactly the same state, and says so with 200.""" + client.post(f"{_BASE}/task-1", json=_body()) + response = client.put(f"{_BASE}/task-1", json=_body()) + assert response.status_code == 200 + assert response.json()["revision"] == 1 + + +def test_put_applies_tags_without_publishing(client: TestClient) -> None: + """Re-PUTting unchanged content is how an existing revision gets tagged.""" + client.post(f"{_BASE}/task-1", json=_body()) + response = client.put(f"{_BASE}/task-1", json=_body(tags=["blessed"])) + assert response.status_code == 200 + assert response.json()["tags"] == {"latest": 1, "blessed": 1} + + +def test_post_on_existing_task_still_conflicts(client: TestClient) -> None: + """POST keeps its guard: a typo'd name must not silently overwrite someone else's task.""" + client.post(f"{_BASE}/task-1", json=_body()) + assert client.post(f"{_BASE}/task-1", json=_body(intent="Different.")).status_code == 409 + + +def test_get_returns_the_current_revision(client: TestClient) -> None: + client.post(f"{_BASE}/task-1", json=_body()) + 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." + + +# --- Reading and tagging a specific revision --------------------------------- + + +def test_list_revisions_newest_first(client: TestClient) -> None: + """How a caller discovers what it can pin to.""" + client.post(f"{_BASE}/task-1", json=_body()) + client.put(f"{_BASE}/task-1", json=_body(intent="Second.")) + + revisions = client.get(f"{_BASE}/task-1/revisions").json()["data"] + assert [r["revision"] for r in revisions] == [2, 1] + assert all(len(r["content_hash"]) == 64 for r in revisions) + assert revisions[0]["tags"] == ["latest"] + + +def test_list_revisions_missing_task_returns_404(client: TestClient) -> None: + assert client.get(f"{_BASE}/nope/revisions").status_code == 404 + + +def test_get_by_digest_returns_the_published_content(client: TestClient) -> None: + """The point of pinning: a consumer holding a digest reads what was published, not what is + current.""" + first = client.post(f"{_BASE}/task-1", json=_body()).json() + digest = client.get(f"{_BASE}/task-1/revisions").json()["data"][0]["content_hash"] + 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["revision"] == 1 + assert client.get(f"{_BASE}/task-1").json()["intent"] == "Newer." + + +def test_get_by_tag_resolves(client: TestClient) -> None: + client.post(f"{_BASE}/task-1", json=_body(tags=["blessed"])) + client.put(f"{_BASE}/task-1", json=_body(intent="Newer.")) + assert client.get(f"{_BASE}/task-1/revisions/blessed").json()["revision"] == 1 + + +def test_get_by_unknown_revision_returns_404(client: TestClient) -> None: + client.post(f"{_BASE}/task-1", json=_body()) + assert client.get(f"{_BASE}/task-1/revisions/{'c' * 64}").status_code == 404 + + +def test_tag_an_existing_revision(client: TestClient) -> None: + """Blessing a revision usually happens after it has been evaluated, not at publish time.""" + client.post(f"{_BASE}/task-1", json=_body()) + digest = client.get(f"{_BASE}/task-1/revisions").json()["data"][0]["content_hash"] + client.put(f"{_BASE}/task-1", json=_body(intent="Newer.")) + + tagged = client.put(f"{_BASE}/task-1/tags/blessed", params={"revision": digest}) + assert tagged.status_code == 200 + assert tagged.json()["tags"]["blessed"] == 1 + assert tagged.json()["tags"]["latest"] == 2, "tagging must not disturb latest" + + +def test_cannot_move_latest_by_hand(client: TestClient) -> None: + """``latest`` is machine-managed; moving it would break the forward-only guarantee.""" + client.post(f"{_BASE}/task-1", json=_body()) + digest = client.get(f"{_BASE}/task-1/revisions").json()["data"][0]["content_hash"] + assert client.put(f"{_BASE}/task-1/tags/latest", params={"revision": digest}).status_code == 422 + + +def test_tag_unknown_revision_returns_404(client: TestClient) -> None: + client.post(f"{_BASE}/task-1", json=_body()) + assert client.put(f"{_BASE}/task-1/tags/blessed", params={"revision": "c" * 64}).status_code == 404 + + +def test_concurrent_replace_returns_409_not_500(entity_store) -> None: + """A lost optimistic lock is a retryable client conflict. Before this was mapped it fell to the + catch-all and surfaced as a 500, wrongly implying a server fault.""" + + async def _stale(entity, *, original_name=None): + raise NemoEntityConflictError("modified by another request") + + app = FastAPI() + app.include_router(tasks_routes.router, prefix="/v2/workspaces/{workspace}") + service = TaskService(entity_store, _FakeMetricService()) + app.dependency_overrides[get_task_service] = lambda: service + client = TestClient(app) + + client.post(f"{_BASE}/task-1", json=_body()) + entity_store.update = _stale + + assert client.put(f"{_BASE}/task-1", json=_body(intent="Newer.")).status_code == 409 diff --git a/plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py b/plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py index eb743eea10..f62cb38c8c 100644 --- a/plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py +++ b/plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py @@ -9,7 +9,7 @@ from __future__ import annotations -from datetime import datetime, timezone +import hashlib import pytest from fastapi import FastAPI @@ -18,68 +18,7 @@ from nemo_evaluator.api.schemas import TaskRef, TasksetInput from nemo_evaluator.api.service.taskset_service import TasksetService from nemo_evaluator.api.v2 import tasksets as tasksets_routes -from nemo_evaluator.entities import TasksetEntity -from nemo_platform_plugin.entities import ListResponse, PaginationInfo from nemo_platform_plugin.entity_client import NemoEntityConflictError, NemoEntityNotFoundError -from nemo_platform_plugin.filter_ops import FilterOperation - - -class _FakeEntityClient: - def __init__(self) -> None: - self.entities: dict[tuple[str, str, str], TasksetEntity] = {} - - async def create(self, entity: TasksetEntity) -> TasksetEntity: - key = (entity.__entity_type__, entity.workspace, entity.name) - if key in self.entities: - raise NemoEntityConflictError(f"{key} exists") - now = datetime.now(timezone.utc) - entity._id = f"{entity.__entity_type__}-{entity.name}" - entity._created_at = now - entity._updated_at = now - self.entities[key] = entity - return entity - - async def get( - self, entity_type: type[TasksetEntity], *, workspace: str, name: str, parent: str | None = None - ) -> TasksetEntity: - key = (entity_type.__entity_type__, workspace, name) - if key not in self.entities: - raise NemoEntityNotFoundError(f"{workspace}/{name} not found") - return self.entities[key] - - async def delete( - self, - entity_type: type[TasksetEntity], - name: str, - *, - workspace: str, - parent: str | None = None, - expected_db_version: int | None = None, - ) -> None: - key = (entity_type.__entity_type__, workspace, name) - if key not in self.entities: - raise NemoEntityNotFoundError(f"{workspace}/{name} not found") - del self.entities[key] - - async def list( - self, - entity_type: type[TasksetEntity], - *, - workspace: str, - filter_operation: FilterOperation | None = None, - sort: str | None = None, - page: int = 1, - page_size: int = 100, - ) -> ListResponse[TasksetEntity]: - items = [ - e for (etype, ws, _), e in self.entities.items() if etype == entity_type.__entity_type__ and ws == workspace - ] - return ListResponse( - data=items, - pagination=PaginationInfo( - page=page, page_size=page_size, current_page_size=len(items), total_pages=1, total_results=len(items) - ), - ) class _FakeTaskService: @@ -88,20 +27,27 @@ class _FakeTaskService: async def get_task(self, workspace: str, name: str) -> object | None: return object() if name in {"task-a", "task-b"} else None + async def resolve_revision(self, workspace: str, name: str, fragment: str = "latest") -> str: + """A stable per-task digest; raises for an unknown task, as the real service does.""" + if name not in {"task-a", "task-b"}: + raise NemoEntityNotFoundError(f"{workspace}/{name} not found") + return hashlib.sha256(f"{workspace}/{name}".encode()).hexdigest() + @pytest.fixture -def client() -> TestClient: +def client(entity_store) -> TestClient: app = FastAPI() app.include_router(tasksets_routes.router, prefix="/v2/workspaces/{workspace}") - service = TasksetService(_FakeEntityClient(), _FakeTaskService()) + service = TasksetService(entity_store, _FakeTaskService()) app.dependency_overrides[get_taskset_service] = lambda: service return TestClient(app) -def _body() -> dict: +def _body(*, description: str = "A grouping.", members: list[str] | None = None, tags: list[str] | None = None) -> dict: return TasksetInput( - description="A grouping.", - tasks=[TaskRef("task-a"), TaskRef("default/task-b")], + description=description, + tasks=[TaskRef(m) for m in (members or ["task-a", "default/task-b"])], + tags=tags or [], ).model_dump(mode="json") @@ -117,7 +63,9 @@ def test_create_then_get(client: TestClient) -> None: assert got.status_code == 200 body = got.json() assert body["description"] == "A grouping." - assert body["tasks"] == ["task-a", "default/task-b"] # TaskRef serializes to a bare string + # TaskRef serializes to a bare string, and stored membership is workspace-qualified and + # digest-pinned — a bare "task-a" is resolved to an exact revision on write. + assert [t.split("#")[0] for t in body["tasks"]] == ["default/task-a", "default/task-b"] def test_create_rejects_unknown_body_key(client: TestClient) -> None: @@ -199,3 +147,102 @@ async def delete_taskset(self, workspace: str, name: str) -> bool: client = TestClient(app) assert client.delete(f"{_BASE}/ts-1").status_code == 409 + + +# --- Publishing revisions (POST creates, PUT replaces) ------------------------ + + +def test_create_publishes_revision_one(client: TestClient) -> None: + created = client.post(f"{_BASE}/ts-1", json=_body()).json() + assert created["revision"] == 1 + assert created["tags"] == {"latest": 1} + + +def test_members_are_stored_digest_pinned(client: TestClient) -> None: + """A published grouping names exact revisions. Storing '#latest' would let a member republish + silently change what this taskset contains — which is the whole failure this design prevents.""" + body = client.post(f"{_BASE}/ts-1", json=_body()).json() + assert all(len(ref.split("#")[1]) == 64 for ref in body["tasks"]) + + +def test_tag_pinned_member_is_resolved_to_a_digest(client: TestClient) -> None: + """Tags are resolution *inputs*: accepted on the way in, never persisted.""" + body = client.post(f"{_BASE}/ts-1", json=_body(members=["task-a#latest"])).json() + assert "#latest" not in body["tasks"][0] + assert len(body["tasks"][0].split("#")[1]) == 64 + + +def test_put_on_missing_taskset_creates_it(client: TestClient) -> None: + response = client.put(f"{_BASE}/ts-1", json=_body()) + assert response.status_code == 201 + assert response.json()["revision"] == 1 + + +def test_put_with_changed_membership_publishes_a_new_revision(client: TestClient) -> None: + client.post(f"{_BASE}/ts-1", json=_body()) + response = client.put(f"{_BASE}/ts-1", json=_body(members=["task-a"])) + assert response.status_code == 201 + assert response.json()["revision"] == 2 + + +def test_put_with_identical_membership_publishes_nothing(client: TestClient) -> None: + """Idempotent when nothing underneath moved — the member digests resolve the same.""" + client.post(f"{_BASE}/ts-1", json=_body()) + response = client.put(f"{_BASE}/ts-1", json=_body()) + assert response.status_code == 200 + assert response.json()["revision"] == 1 + + +def test_put_applies_tags_without_publishing(client: TestClient) -> None: + client.post(f"{_BASE}/ts-1", json=_body()) + response = client.put(f"{_BASE}/ts-1", json=_body(tags=["blessed"])) + assert response.status_code == 200 + assert response.json()["tags"] == {"latest": 1, "blessed": 1} + + +def test_post_on_existing_taskset_still_conflicts(client: TestClient) -> None: + client.post(f"{_BASE}/ts-1", json=_body()) + assert client.post(f"{_BASE}/ts-1", json=_body(description="Other.")).status_code == 409 + + +# --- Reading and tagging a specific revision --------------------------------- + + +def test_list_revisions_newest_first(client: TestClient) -> None: + client.post(f"{_BASE}/ts-1", json=_body()) + client.put(f"{_BASE}/ts-1", json=_body(members=["task-a"])) + + revisions = client.get(f"{_BASE}/ts-1/revisions").json()["data"] + assert [r["revision"] for r in revisions] == [2, 1] + assert revisions[0]["tags"] == ["latest"] + + +def test_get_by_digest_returns_published_membership(client: TestClient) -> None: + """The reason a dataset is reproducible: a pinned read returns the membership as published.""" + client.post(f"{_BASE}/ts-1", json=_body()) + digest = client.get(f"{_BASE}/ts-1/revisions").json()["data"][0]["content_hash"] + client.put(f"{_BASE}/ts-1", json=_body(members=["task-a"])) + + pinned = client.get(f"{_BASE}/ts-1/revisions/{digest}").json() + assert len(pinned["tasks"]) == 2 + assert len(client.get(f"{_BASE}/ts-1").json()["tasks"]) == 1 + + +def test_tag_an_existing_revision(client: TestClient) -> None: + client.post(f"{_BASE}/ts-1", json=_body()) + digest = client.get(f"{_BASE}/ts-1/revisions").json()["data"][0]["content_hash"] + client.put(f"{_BASE}/ts-1", json=_body(members=["task-a"])) + + tagged = client.put(f"{_BASE}/ts-1/tags/blessed", params={"revision": digest}) + assert tagged.status_code == 200 + assert tagged.json()["tags"] == {"latest": 2, "blessed": 1} + + +def test_cannot_move_latest_by_hand(client: TestClient) -> None: + client.post(f"{_BASE}/ts-1", json=_body()) + digest = client.get(f"{_BASE}/ts-1/revisions").json()["data"][0]["content_hash"] + assert client.put(f"{_BASE}/ts-1/tags/latest", params={"revision": digest}).status_code == 422 + + +def test_list_revisions_missing_taskset_returns_404(client: TestClient) -> None: + assert client.get(f"{_BASE}/nope/revisions").status_code == 404 diff --git a/plugins/nemo-evaluator/tests/conftest.py b/plugins/nemo-evaluator/tests/conftest.py new file mode 100644 index 0000000000..6926410d87 --- /dev/null +++ b/plugins/nemo-evaluator/tests/conftest.py @@ -0,0 +1,163 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Shared test doubles for the evaluator plugin.""" + +from __future__ import annotations + +import math +from datetime import datetime, timedelta, timezone + +import pytest +from nemo_platform_plugin.entities import EntityBase, ListResponse, PaginationInfo +from nemo_platform_plugin.entity_client import NemoEntityConflictError, NemoEntityNotFoundError +from nemo_platform_plugin.filter_ops import LogicalOperation + + +def matches_filter(entity, operation) -> bool: + """Evaluate the AND-of-equality filters services emit. + + Filters are *evaluated*, not ignored: a fake that returned every row would make a revision + lookup that forgot its ``parent`` predicate look correct, and cross-record confusion is exactly + the bug that predicate prevents. + """ + if operation is None: + return True + if isinstance(operation, LogicalOperation): + return all(matches_filter(entity, child) for child in operation.operations) + field = operation.field + actual = entity.parent if field == "parent" else getattr(entity, field.removeprefix("data."), None) + return actual == operation.value + + +class FakeEntityStore: + """In-memory entity store standing in for ``NemoEntitiesClient``. + + Two behaviors are reproduced deliberately, because service logic depends on them: + + - **Parent-scoped uniqueness.** Records key on ``(entity_type, workspace, name, parent)``, which + is what lets revision children of different tasks both be named ``rev.1`` while making a + duplicate ordinal under one parent conflict. + - **Optimistic locking.** ``update`` rejects a write whose ``db_version`` is stale. A fake that + accepted every update would make correct retry logic untestable and broken retry logic look + fine. + - **Copy-on-read and copy-on-write.** Every record crosses the boundary as a deep copy, because + the real client serializes over HTTP and cannot share objects with its caller. A fake that + handed back the stored instance would let a service mutate the store just by touching an + entity it read — so staging changes in memory and writing them later would be + indistinguishable from writing them immediately, and a missing write would still pass. + + Signatures follow the *concrete* client (positional ``name``, optional ``parent``) rather than + the narrower shared protocols, since that is what services actually call. + """ + + def __init__(self) -> None: + self.entities: dict[tuple[str, str, str, str | None], EntityBase] = {} + #: Monotonic tick for creation timestamps. Wall-clock ``now()`` can repeat within a test, + #: which would make ``-created_at`` ordering non-deterministic — the real store's inserts + #: are genuinely ordered, so the fake must be too. + self._tick = 0 + + def _now(self) -> datetime: + self._tick += 1 + return datetime(2026, 1, 1, tzinfo=timezone.utc) + timedelta(seconds=self._tick) + + def _key(self, entity_type, name: str, workspace: str, parent: str | None): + return (entity_type.__entity_type__, workspace, name, parent) + + async def create(self, entity): + key = self._key(type(entity), entity.name, entity.workspace, entity._parent) + if key in self.entities: + raise NemoEntityConflictError(f"{key} exists") + now = self._now() + entity._id = f"{entity.__entity_type__}-{entity.name}" + entity._created_at = now + entity._updated_at = now + entity._db_version = 0 + self.entities[key] = entity.model_copy(deep=True) + return entity.model_copy(deep=True) + + async def get(self, entity_type, name: str, *, workspace: str, parent: str | None = None): + key = self._key(entity_type, name, workspace, parent) + if key not in self.entities: + raise NemoEntityNotFoundError(f"{workspace}/{name} not found") + return self.entities[key].model_copy(deep=True) + + async def update(self, entity, *, original_name: str | None = None): + key = self._key(type(entity), original_name or entity.name, entity.workspace, entity._parent) + stored = self.entities.get(key) + if stored is not None and entity._db_version != stored._db_version: + raise NemoEntityConflictError(f"stale update for {entity.name}") + entity._db_version = (stored._db_version if stored is not None else 0) + 1 + entity._updated_at = self._now() + self.entities[key] = entity.model_copy(deep=True) + return entity.model_copy(deep=True) + + async def delete( + self, + entity_type, + name: str, + *, + workspace: str, + parent: str | None = None, + expected_db_version: int | None = None, + ) -> None: + # ``parent`` is part of the key, not decoration: revision children of different heads share + # the name ``rev.1``, so ignoring it here would delete whichever one was inserted first. + key = self._key(entity_type, name, workspace, parent) + if key not in self.entities: + raise NemoEntityNotFoundError(f"{workspace}/{name} not found") + del self.entities[key] + + async def list( + self, + entity_type, + *, + workspace: str, + filter_operation=None, + sort: str | None = None, + page: int = 1, + page_size: int = 100, + ) -> ListResponse: + items = [ + entity + for (entity_type_name, record_workspace, _, _), entity in self.entities.items() + if entity_type_name == entity_type.__entity_type__ and record_workspace == workspace + ] + items = [entity for entity in items if matches_filter(entity, filter_operation)] + if sort and items: + # Honour ``sort`` rather than ignoring it: services rely on server-side ordering, and a + # fake that returned insertion order would make a wrong ``sort`` argument invisible. + # Nothing to order when the result is empty, and probing the field on a bare ``object`` + # would reject a legitimate sort over an empty page. + field = sort.lstrip("-") + if not hasattr(items[0], field): + raise NotImplementedError(f"fake cannot sort on {field!r}") + items = sorted(items, key=lambda entity: getattr(entity, field), reverse=sort.startswith("-")) + # Totals describe the whole result set, not the page — that distinction is the only way a + # caller can tell a truncated history from a complete one, which is what ``list_revisions`` + # documents. + total_results = len(items) + start = (page - 1) * page_size + items = [entity.model_copy(deep=True) for entity in items[start : start + page_size]] + return ListResponse( + data=items, + pagination=PaginationInfo( + page=page, + page_size=page_size, + current_page_size=len(items), + total_pages=max(1, math.ceil(total_results / page_size)), + total_results=total_results, + ), + ) + + +@pytest.fixture +def entity_store() -> FakeEntityStore: + """A fresh in-memory entity store. + + Exposed as a fixture rather than an importable name because ``from conftest import ...`` is + ambiguous once more than one test root is collected in the same run — the wrong ``conftest`` + module wins and the import fails. + """ + return FakeEntityStore() diff --git a/plugins/nemo-evaluator/tests/integration/test_task_revisions.py b/plugins/nemo-evaluator/tests/integration/test_task_revisions.py new file mode 100644 index 0000000000..4e814251ff --- /dev/null +++ b/plugins/nemo-evaluator/tests/integration/test_task_revisions.py @@ -0,0 +1,249 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Integration tests for task/taskset revisions through the SDK against a real platform. + +Everything else about revisions is covered by unit tests against an in-memory fake. This suite +exists because that fake has repeatedly been *wrong* in ways that hid real behavior — it initially +ignored filters, ignored ``sort``, and aliased stored and in-memory objects so optimistic-lock +conflicts could never fire. Each of those made correct-looking tests pass over broken assumptions. + +So these tests target exactly the things only a real entity store can confirm: + +- child records are addressable under a parent, and ordinals collide per-parent rather than globally; +- the ``(parent, data.content_hash)`` query really resolves a digest to its revision; +- server-side ``-created_at`` ordering actually returns revisions newest-first; +- deleting a task cascades to its revisions; +- a published revision is immutable in practice: reading a pinned digest returns the old content + after the task has moved on. + +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 TaskInput, TasksetInput +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" + + +def _unique(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +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 []) + + +def _client(base_url: str) -> NeMoPlatform: + client = NeMoPlatform(base_url=base_url, max_retries=2) + client.workspaces.create(name=WORKSPACE, exist_ok=True) + return client + + +@pytest.mark.timeout(300) +def test_publish_and_read_a_pinned_revision(subprocess_platform: str) -> None: + """The core promise: a digest-pinned read returns what was published, not what is current.""" + client = _client(subprocess_platform) + name = _unique("task") + try: + created = client.evaluator.tasks.create(name, task=_task_input("First."), workspace=WORKSPACE) + assert created.revision == 1 + assert created.tags["latest"] == 1 + + first_digest = client.evaluator.tasks.list_revisions(name, workspace=WORKSPACE).data[0].content_hash + + replaced = client.evaluator.tasks.replace(name, task=_task_input("Second."), workspace=WORKSPACE) + assert replaced.revision == 2 + + pinned = client.evaluator.tasks.retrieve(name, revision=first_digest, workspace=WORKSPACE) + assert pinned.intent == "First." + assert pinned.revision == 1 + assert client.evaluator.tasks.retrieve(name, workspace=WORKSPACE).intent == "Second." + finally: + client.evaluator.tasks.delete(name, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_republishing_identical_content_cuts_no_revision(subprocess_platform: str) -> None: + """Publish-time dedup against a real store: the ``(parent, content_hash)`` query must find the + existing revision, or every republish would allocate a new ordinal.""" + client = _client(subprocess_platform) + name = _unique("task") + try: + client.evaluator.tasks.create(name, task=_task_input("Same."), workspace=WORKSPACE) + again = client.evaluator.tasks.replace(name, task=_task_input("Same."), workspace=WORKSPACE) + + assert again.revision == 1 + assert client.evaluator.tasks.list_revisions(name, workspace=WORKSPACE).pagination.total_results == 1 + finally: + client.evaluator.tasks.delete(name, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_revisions_come_back_newest_first(subprocess_platform: str) -> None: + """Ordering is server-side (``-created_at``); the fake ignored ``sort`` entirely at first, so + this is the only place the real ordering is confirmed.""" + client = _client(subprocess_platform) + name = _unique("task") + try: + client.evaluator.tasks.create(name, task=_task_input("One."), workspace=WORKSPACE) + client.evaluator.tasks.replace(name, task=_task_input("Two."), workspace=WORKSPACE) + client.evaluator.tasks.replace(name, task=_task_input("Three."), workspace=WORKSPACE) + + page = client.evaluator.tasks.list_revisions(name, workspace=WORKSPACE) + assert [r.revision for r in page.data] == [3, 2, 1] + finally: + client.evaluator.tasks.delete(name, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_ordinals_are_scoped_per_task(subprocess_platform: str) -> None: + """Two tasks each own a ``rev.1``. Parent-scoped uniqueness is what allows that; without it the + second task's first publish would collide on the name.""" + client = _client(subprocess_platform) + first, second = _unique("task-a"), _unique("task-b") + try: + a = client.evaluator.tasks.create(first, task=_task_input("A."), workspace=WORKSPACE) + b = client.evaluator.tasks.create(second, task=_task_input("B."), workspace=WORKSPACE) + assert a.revision == b.revision == 1 + + a_digest = client.evaluator.tasks.list_revisions(first, workspace=WORKSPACE).data[0].content_hash + b_digest = client.evaluator.tasks.list_revisions(second, workspace=WORKSPACE).data[0].content_hash + assert a_digest != b_digest + finally: + client.evaluator.tasks.delete(first, workspace=WORKSPACE) + client.evaluator.tasks.delete(second, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_identical_content_under_two_tasks_does_not_cross_resolve(subprocess_platform: str) -> None: + """Same content under two tasks yields one digest, so the digest alone cannot identify a + revision — resolution must be parent-scoped. Only a real store proves the query is.""" + client = _client(subprocess_platform) + first, second = _unique("task-a"), _unique("task-b") + try: + client.evaluator.tasks.create(first, task=_task_input("Shared."), workspace=WORKSPACE) + client.evaluator.tasks.create(second, task=_task_input("Shared."), workspace=WORKSPACE) + + a_digest = client.evaluator.tasks.list_revisions(first, workspace=WORKSPACE).data[0].content_hash + b_digest = client.evaluator.tasks.list_revisions(second, workspace=WORKSPACE).data[0].content_hash + assert a_digest == b_digest, "identical content must digest identically" + + # Each resolves under its own parent, and neither leaks the other's record. + assert client.evaluator.tasks.retrieve(first, revision=a_digest, workspace=WORKSPACE).name == first + assert client.evaluator.tasks.retrieve(second, revision=b_digest, workspace=WORKSPACE).name == second + finally: + client.evaluator.tasks.delete(first, workspace=WORKSPACE) + client.evaluator.tasks.delete(second, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_tagging_an_older_revision_leaves_latest_alone(subprocess_platform: str) -> None: + client = _client(subprocess_platform) + name = _unique("task") + try: + client.evaluator.tasks.create(name, task=_task_input("First."), workspace=WORKSPACE) + first_digest = client.evaluator.tasks.list_revisions(name, workspace=WORKSPACE).data[0].content_hash + client.evaluator.tasks.replace(name, task=_task_input("Second."), workspace=WORKSPACE) + + tagged = client.evaluator.tasks.tag(name, tag="blessed", revision=first_digest, workspace=WORKSPACE) + + 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." + finally: + client.evaluator.tasks.delete(name, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_deleting_a_task_removes_its_revisions(subprocess_platform: str) -> None: + """Cascade is a DB-level FK behavior, so it can only be confirmed against real persistence.""" + client = _client(subprocess_platform) + name = _unique("task") + client.evaluator.tasks.create(name, task=_task_input("One."), workspace=WORKSPACE) + client.evaluator.tasks.replace(name, task=_task_input("Two."), workspace=WORKSPACE) + + client.evaluator.tasks.delete(name, workspace=WORKSPACE) + + # Recreating under the same name starts from revision 1 — the old children are gone, so the + # ordinal is free. A surviving `rev.1` would make this publish conflict. + recreated = client.evaluator.tasks.create(name, task=_task_input("Fresh."), workspace=WORKSPACE) + try: + assert recreated.revision == 1 + assert client.evaluator.tasks.list_revisions(name, workspace=WORKSPACE).pagination.total_results == 1 + finally: + client.evaluator.tasks.delete(name, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_taskset_membership_is_pinned_and_stays_pinned(subprocess_platform: str) -> None: + """The reproducibility guarantee, end to end: a published taskset keeps naming the member + revision it was created with, even after that member publishes new content.""" + client = _client(subprocess_platform) + task_name, set_name = _unique("task"), _unique("ts") + try: + client.evaluator.tasks.create(task_name, task=_task_input("Original."), workspace=WORKSPACE) + + created = client.evaluator.tasksets.create( + set_name, taskset=TasksetInput(tasks=[task_name]), workspace=WORKSPACE + ) + member = created.tasks[0].root + assert "#" in member, "membership must be stored digest-pinned" + pinned_digest = member.split("#", 1)[1] + # A fragment alone only proves *some* sub-entity was named. Pin it to the member's actual + # content digest, which is what makes the reference content-addressed rather than a label. + task_revisions = client.evaluator.tasks.list_revisions(task_name, workspace=WORKSPACE) + assert pinned_digest == task_revisions.data[0].content_hash + + # The member moves on; the published taskset must not. + 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." + ) + finally: + client.evaluator.tasksets.delete(set_name, workspace=WORKSPACE) + client.evaluator.tasks.delete(task_name, workspace=WORKSPACE) + + +@pytest.mark.timeout(300) +def test_republishing_a_taskset_after_a_member_moves_cuts_a_revision(subprocess_platform: str) -> None: + """Members re-resolve on every write, so identical member *names* can still be a real change. + This is the semantic that is hardest to change later, so it is worth pinning against a real + store rather than only against a fake.""" + client = _client(subprocess_platform) + task_name, set_name = _unique("task"), _unique("ts") + try: + client.evaluator.tasks.create(task_name, task=_task_input("v1."), workspace=WORKSPACE) + created = client.evaluator.tasksets.create( + set_name, taskset=TasksetInput(tasks=[task_name]), workspace=WORKSPACE + ) + assert created.revision == 1 + + client.evaluator.tasks.replace(task_name, task=_task_input("v2."), workspace=WORKSPACE) + republished = client.evaluator.tasksets.replace( + set_name, taskset=TasksetInput(tasks=[task_name]), workspace=WORKSPACE + ) + + assert republished.revision == 2, "the grouping names different content, so it is a new revision" + assert republished.tasks[0].root != created.tasks[0].root + finally: + client.evaluator.tasksets.delete(set_name, workspace=WORKSPACE) + client.evaluator.tasks.delete(task_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 f09f7a99f3..6334460ece 100644 --- a/plugins/nemo-evaluator/tests/sdk/test_task_sdk_resources.py +++ b/plugins/nemo-evaluator/tests/sdk/test_task_sdk_resources.py @@ -9,7 +9,8 @@ from typing import Any from unittest.mock import AsyncMock, MagicMock -from nemo_evaluator.api.schemas import MetricRef, Task, TaskInput +import pytest +from nemo_evaluator.api.schemas import MetricRef, Revision, Task, TaskInput from nemo_evaluator.sdk.task_resources import AsyncEvaluatorTasksResource, EvaluatorTasksResource _BASE = "http://localhost:8080/apis/evaluator/v2/workspaces/default" @@ -24,6 +25,8 @@ def _task_payload(name: str) -> dict[str, Any]: intent="Answer the question.", inputs={"instruction": "What is 2+2?"}, metrics=[MetricRef("default/stored-metric")], + revision=1, + tags={"latest": 1}, created_at=now, updated_at=now, ).model_dump(mode="json") @@ -118,3 +121,171 @@ async def test_async_retrieve_parses_dto() -> None: assert isinstance(result, Task) assert result.name == "task-9" assert http_client.get.call_args[0][0] == f"{_BASE}/tasks/task-9" + + +# --- Revision-aware resources ------------------------------------------------- + + +def _revision_payload(ordinal: int, digest: str) -> dict[str, Any]: + return Revision( + revision=ordinal, + content_hash=digest, + tags=["latest"] if ordinal == 2 else [], + created_at=datetime.now(timezone.utc), + ).model_dump(mode="json") + + +def test_sync_replace_puts_task_input_to_item_url() -> None: + http_client = MagicMock() + http_client.put.return_value = _response(_task_payload("task-1")) + resource = EvaluatorTasksResource(_platform(http_client)) + + 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 isinstance(result, Task) + + +def test_sync_replace_passes_project_through() -> None: + http_client = MagicMock() + http_client.put.return_value = _response(_task_payload("task-1")) + resource = EvaluatorTasksResource(_platform(http_client)) + + resource.replace("task-1", task=_task_input(), project="proj-a") + + assert http_client.put.call_args.kwargs["params"] == {"project": "proj-a"} + + +def test_sync_retrieve_without_revision_targets_the_item_url() -> None: + http_client = MagicMock() + http_client.get.return_value = _response(_task_payload("task-1")) + resource = EvaluatorTasksResource(_platform(http_client)) + + resource.retrieve("task-1") + + assert http_client.get.call_args.args[0] == f"{_BASE}/tasks/task-1" + + +def test_sync_retrieve_with_revision_targets_the_revision_sub_path() -> None: + """The revision is a path segment, matching the tags route rather than a query parameter.""" + http_client = MagicMock() + http_client.get.return_value = _response(_task_payload("task-1")) + resource = EvaluatorTasksResource(_platform(http_client)) + digest = "a" * 64 + + resource.retrieve("task-1", revision=digest) + + assert http_client.get.call_args.args[0] == f"{_BASE}/tasks/task-1/revisions/{digest}" + + +def test_sync_retrieve_with_tag_targets_the_same_sub_path() -> None: + """``tag`` and ``revision`` are two names for one route segment, resolved server-side. + + Splitting them is a call-site readability change only, so a tag must reach exactly the URL a + digest would — a separate query parameter or route would be a behaviour change nobody asked for. + """ + http_client = MagicMock() + http_client.get.return_value = _response(_task_payload("task-1")) + resource = EvaluatorTasksResource(_platform(http_client)) + + resource.retrieve("task-1", tag="blessed") + + assert http_client.get.call_args.args[0] == f"{_BASE}/tasks/task-1/revisions/blessed" + + +def test_sync_retrieve_rejects_both_selectors() -> None: + """Two selectors is ambiguous intent, not a precedence question — refuse rather than pick one.""" + resource = EvaluatorTasksResource(_platform(MagicMock())) + + with pytest.raises(ValueError, match="not both"): + resource.retrieve("task-1", revision="a" * 64, tag="blessed") + + +def test_sync_retrieve_percent_encodes_a_tag() -> None: + """Tags admit ``/`` (``release/v1``), which would otherwise open a path segment.""" + http_client = MagicMock() + http_client.get.return_value = _response(_task_payload("task-1")) + resource = EvaluatorTasksResource(_platform(http_client)) + + resource.retrieve("task-1", tag="release/v1") + + assert http_client.get.call_args.args[0] == f"{_BASE}/tasks/task-1/revisions/release%2Fv1" + + +def test_sync_list_revisions_parses_the_page() -> None: + http_client = MagicMock() + http_client.get.return_value = _response( + { + "data": [_revision_payload(2, "b" * 64), _revision_payload(1, "a" * 64)], + "pagination": { + "page": 1, + "page_size": 100, + "current_page_size": 2, + "total_pages": 1, + "total_results": 2, + }, + } + ) + resource = EvaluatorTasksResource(_platform(http_client)) + + page = resource.list_revisions("task-1") + + assert http_client.get.call_args.args[0] == f"{_BASE}/tasks/task-1/revisions" + assert [r.revision for r in page.data] == [2, 1] + assert page.data[0].content_hash == "b" * 64 + + +def test_sync_tag_puts_to_the_tag_url_with_the_revision() -> None: + http_client = MagicMock() + http_client.put.return_value = _response(_task_payload("task-1")) + resource = EvaluatorTasksResource(_platform(http_client)) + digest = "a" * 64 + + resource.tag("task-1", tag="blessed", revision=digest) + + assert http_client.put.call_args.args[0] == f"{_BASE}/tasks/task-1/tags/blessed" + assert http_client.put.call_args.kwargs["params"] == {"revision": digest} + + +def test_sync_tag_escapes_the_tag_name() -> None: + """Tag names reach the URL as a path segment; anything needing escaping must be escaped.""" + http_client = MagicMock() + http_client.put.return_value = _response(_task_payload("task-1")) + resource = EvaluatorTasksResource(_platform(http_client)) + + resource.tag("task-1", tag="release/v1", revision="a" * 64) + + assert http_client.put.call_args.args[0] == f"{_BASE}/tasks/task-1/tags/release%2Fv1" + + +async def test_async_replace_puts_task_input() -> None: + http_client = MagicMock() + http_client.put = AsyncMock(return_value=_response(_task_payload("task-1"))) + resource = AsyncEvaluatorTasksResource(_platform(http_client)) + + result = await resource.replace("task-1", task=_task_input()) + + assert http_client.put.call_args.args[0] == f"{_BASE}/tasks/task-1" + assert isinstance(result, Task) + + +async def test_async_retrieve_with_revision_targets_the_revision_sub_path() -> None: + http_client = MagicMock() + http_client.get = AsyncMock(return_value=_response(_task_payload("task-1"))) + resource = AsyncEvaluatorTasksResource(_platform(http_client)) + digest = "a" * 64 + + await resource.retrieve("task-1", revision=digest) + + assert http_client.get.call_args.args[0] == f"{_BASE}/tasks/task-1/revisions/{digest}" + + +async def test_async_tag_puts_to_the_tag_url() -> None: + http_client = MagicMock() + http_client.put = AsyncMock(return_value=_response(_task_payload("task-1"))) + resource = AsyncEvaluatorTasksResource(_platform(http_client)) + + await resource.tag("task-1", tag="blessed", revision="a" * 64) + + assert http_client.put.call_args.args[0] == f"{_BASE}/tasks/task-1/tags/blessed" diff --git a/plugins/nemo-evaluator/tests/sdk/test_taskset_sdk_resources.py b/plugins/nemo-evaluator/tests/sdk/test_taskset_sdk_resources.py index ba98ace269..335489761f 100644 --- a/plugins/nemo-evaluator/tests/sdk/test_taskset_sdk_resources.py +++ b/plugins/nemo-evaluator/tests/sdk/test_taskset_sdk_resources.py @@ -9,7 +9,7 @@ from typing import Any from unittest.mock import AsyncMock, MagicMock -from nemo_evaluator.api.schemas import TaskRef, Taskset, TasksetInput +from nemo_evaluator.api.schemas import Revision, TaskRef, Taskset, TasksetInput from nemo_evaluator.sdk.taskset_resources import AsyncEvaluatorTasksetsResource, EvaluatorTasksetsResource _BASE = "http://localhost:8080/apis/evaluator/v2/workspaces/default" @@ -23,6 +23,8 @@ def _taskset_payload(name: str) -> dict[str, Any]: workspace="default", description="A grouping.", tasks=[TaskRef("default/task-a")], + revision=1, + tags={"latest": 1}, created_at=now, updated_at=now, ).model_dump(mode="json") @@ -117,3 +119,104 @@ async def test_async_retrieve_parses_dto() -> None: assert isinstance(result, Taskset) assert result.name == "ts-9" assert http_client.get.call_args[0][0] == f"{_BASE}/tasksets/ts-9" + + +# --- Revision-aware resources ------------------------------------------------- + + +def _revision_payload(ordinal: int, digest: str) -> dict[str, Any]: + return Revision(revision=ordinal, content_hash=digest, tags=[], created_at=datetime.now(timezone.utc)).model_dump( + mode="json" + ) + + +def test_sync_replace_puts_taskset_input_to_item_url() -> None: + http_client = MagicMock() + http_client.put.return_value = _response(_taskset_payload("ts-1")) + resource = EvaluatorTasksetsResource(_platform(http_client)) + + result = resource.replace("ts-1", taskset=_taskset_input()) + + assert http_client.put.call_args.args[0] == f"{_BASE}/tasksets/ts-1" + assert isinstance(result, Taskset) + + +def test_sync_retrieve_with_revision_targets_the_revision_sub_path() -> None: + http_client = MagicMock() + http_client.get.return_value = _response(_taskset_payload("ts-1")) + resource = EvaluatorTasksetsResource(_platform(http_client)) + digest = "a" * 64 + + resource.retrieve("ts-1", revision=digest) + + assert http_client.get.call_args.args[0] == f"{_BASE}/tasksets/ts-1/revisions/{digest}" + + +def test_sync_list_revisions_requests_a_page() -> None: + http_client = MagicMock() + http_client.get.return_value = _response( + { + "data": [_revision_payload(1, "a" * 64)], + "pagination": { + "page": 2, + "page_size": 50, + "current_page_size": 1, + "total_pages": 2, + "total_results": 51, + }, + } + ) + resource = EvaluatorTasksetsResource(_platform(http_client)) + + page = resource.list_revisions("ts-1", page=2, page_size=50) + + assert http_client.get.call_args.args[0] == f"{_BASE}/tasksets/ts-1/revisions" + assert http_client.get.call_args.kwargs["params"] == {"page": 2, "page_size": 50} + # The envelope is carried through so a caller can tell a truncated history from a complete one. + assert page.pagination is not None and page.pagination.total_results == 51 + + +def test_sync_tag_puts_to_the_tag_url_with_the_revision() -> None: + http_client = MagicMock() + http_client.put.return_value = _response(_taskset_payload("ts-1")) + resource = EvaluatorTasksetsResource(_platform(http_client)) + digest = "a" * 64 + + resource.tag("ts-1", tag="blessed", revision=digest) + + assert http_client.put.call_args.args[0] == f"{_BASE}/tasksets/ts-1/tags/blessed" + assert http_client.put.call_args.kwargs["params"] == {"revision": digest} + + +async def test_async_replace_puts_taskset_input() -> None: + http_client = MagicMock() + http_client.put = AsyncMock(return_value=_response(_taskset_payload("ts-1"))) + resource = AsyncEvaluatorTasksetsResource(_platform(http_client)) + + result = await resource.replace("ts-1", taskset=_taskset_input()) + + assert http_client.put.call_args.args[0] == f"{_BASE}/tasksets/ts-1" + assert isinstance(result, Taskset) + + +async def test_async_list_revisions_parses_the_page() -> None: + http_client = MagicMock() + http_client.get = AsyncMock( + return_value=_response( + { + "data": [_revision_payload(1, "a" * 64)], + "pagination": { + "page": 1, + "page_size": 100, + "current_page_size": 1, + "total_pages": 1, + "total_results": 1, + }, + } + ) + ) + resource = AsyncEvaluatorTasksetsResource(_platform(http_client)) + + page = await resource.list_revisions("ts-1") + + assert [r.revision for r in page.data] == [1] diff --git a/plugins/nemo-evaluator/tests/test_content_hash.py b/plugins/nemo-evaluator/tests/test_content_hash.py new file mode 100644 index 0000000000..6dd082ec48 --- /dev/null +++ b/plugins/nemo-evaluator/tests/test_content_hash.py @@ -0,0 +1,225 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Canonical content-hash tests. + +The digest is what a consumer recomputes when reading a pinned ref, so these tests pin the +properties that make that comparison meaningful: determinism, insensitivity to identity and to +server-owned fields, and — the part most likely to break quietly — that near-miss content +variations hash *differently* rather than collapsing onto one digest. +""" + +from __future__ import annotations + +import hashlib +import json +import re +from typing import ClassVar + +from nemo_evaluator.api.schemas import 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_sdk.agent_eval.tasks import SemanticReducer, SemanticView, ViewSignal +from nemo_platform_plugin.entities import EntityBase +from pydantic import Field + +_DEFAULT_VIEWS = { + "correctness": SemanticView( + reducer=SemanticReducer.SINGLE, + signals=[ViewSignal(metric="exact-match", output="score")], + ) +} +_DEFAULT_METADATA = [MetadataItem(key="suite", value="smoke")] + + +def _task( + *, + name: str = "task-1", + workspace: str = "default", + project: str | None = None, + intent: str = "Answer the question.", + inputs: TaskInputs | None = None, + metrics: list[MetricRef] | None = None, + views: dict[str, SemanticView] | None = None, + metadata: list[MetadataItem] | None = None, +) -> TaskEntity: + return TaskEntity( + 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, + ) + + +# --- Shape ------------------------------------------------------------------- + + +def test_digest_is_full_length_lowercase_hex() -> None: + """Never truncated: a shortened prefix would collapse the birthday bound from 2**128.""" + assert re.match(DIGEST_PATTERN, content_hash(_task())) + + +def test_digest_matches_sha256_of_canonical_payload() -> None: + """The payload is the compatibility contract; the digest is just its SHA-256.""" + entity = _task() + expected = hashlib.sha256(canonical_payload(entity).encode("utf-8")).hexdigest() + assert content_hash(entity) == expected + + +def test_canonical_payload_is_compact_and_key_sorted() -> None: + payload = canonical_payload(_task()) + assert ", " not in payload and '": ' not in payload + keys = list(json.loads(payload).keys()) + assert keys == sorted(keys) + + +# --- Determinism ------------------------------------------------------------- + + +def test_identical_content_hashes_identically() -> None: + """The property publish-time dedup depends on: republishing the same content is a no-op.""" + assert content_hash(_task()) == content_hash(_task()) + + +def test_mapping_insertion_order_does_not_affect_digest() -> None: + """Canonicalization sorts mapping keys, so two equal mappings built in different orders are one + piece of content — otherwise republishing an unchanged task would cut a spurious revision. + + ``views`` is the mapping to test this with. ``metadata`` is a *list*, where order is significant + and genuinely changes the digest (see the ordering test below). + """ + first = SemanticView(reducer=SemanticReducer.SINGLE, signals=[ViewSignal(metric="exact-match", output="score")]) + second = SemanticView(reducer=SemanticReducer.SINGLE, signals=[ViewSignal(metric="contains", output="score")]) + + a = _task(views={"correctness": first, "coverage": second}) + b = _task(views={"coverage": second, "correctness": first}) + + assert content_hash(a) == content_hash(b) + + +# --- Identity is not an input ------------------------------------------------ + + +def test_name_and_workspace_do_not_affect_digest() -> None: + """Content-only. Salting with identity would break dedup and defeat verify-on-read: the + recomputed digest would match whenever identity matched, regardless of the content beneath.""" + assert content_hash(_task()) == content_hash(_task(name="task-2", workspace="other")) + + +def test_project_does_not_affect_digest() -> None: + """``project`` is a server-owned base field, not content.""" + assert content_hash(_task()) == content_hash(_task(project="proj-a")) + + +def test_extra_exclude_is_honoured() -> None: + """Revisioned entities exclude their own revision/tag bookkeeping — a revision's digest must + not cover the index that was assigned because of that digest.""" + entity = _task() + assert content_hash(entity, exclude={"metadata"}) != content_hash(entity) + + +# --- Near misses: these must NOT collide ------------------------------------- + + +def test_differing_intent_changes_digest() -> None: + assert content_hash(_task()) != content_hash(_task(intent="Do something else.")) + + +def test_populated_and_empty_metrics_differ() -> None: + """Dropping a task's metrics is a content change, so it must cut a new revision. + + Note this is *not* "absent differs from empty": ``metrics`` defaults to ``[]`` and + ``model_dump`` materializes defaults, so an unset list and an explicitly empty one hash + identically — see :func:`test_absent_field_collapses_onto_its_default`. + """ + assert content_hash(_task(metrics=[])) != content_hash(_task()) + + +def test_metric_ref_order_changes_digest() -> None: + """Sequence order is significant: this function cannot tell a set from an ordered list, so a + set-semantics field must be normalized by its own model before hashing.""" + a = _task(metrics=[MetricRef("default/m-a"), MetricRef("default/m-b")]) + b = _task(metrics=[MetricRef("default/m-b"), MetricRef("default/m-a")]) + assert content_hash(a) != content_hash(b) + + +def test_nested_view_change_changes_digest() -> None: + """Nested sub-models participate; a change buried in a view must not be invisible.""" + changed = _task( + views={ + "correctness": SemanticView( + reducer=SemanticReducer.SINGLE, + signals=[ViewSignal(metric="exact-match", output="other")], + ) + } + ) + assert content_hash(_task()) != content_hash(changed) + + +def test_empty_and_populated_metadata_value_differ() -> None: + a = _task(metadata=[MetadataItem(key="suite", value="smoke")]) + b = _task(metadata=[MetadataItem(key="suite", value="")]) + assert content_hash(a) != content_hash(b) + + +def test_absent_field_collapses_onto_its_default() -> None: + """Documented behavior, asserted so a future change to it is deliberate: ``model_dump`` + materializes defaults, so "unset" and "set to the default" are indistinguishable. A model + needing that distinction must express it (e.g. an optional defaulting to ``None``).""" + assert content_hash(_task(inputs=TaskInputs())) == content_hash(_task(inputs=TaskInputs(instruction=None))) + + +class _NumericEntity(EntityBase): + """Local entity for canonicalization properties the real task schemas can't express. + + ``TaskInputs`` is ``extra="forbid"`` with a single string field, so no current entity carries a + number. The canonicalizer is schema-agnostic, so exercise it directly rather than not at all. + """ + + __entity_type__: ClassVar[str] = "test_numeric" + + value: float | int = Field(description="A number, to pin JSON numeric rendering.") + + +def test_int_and_float_render_distinctly() -> None: + """``1`` and ``1.0`` are distinguishable values, and JSON renders them distinctly.""" + assert content_hash(_NumericEntity(name="n", workspace="default", value=1)) != content_hash( + _NumericEntity(name="n", workspace="default", value=1.0) + ) + + +# --- Tasksets ---------------------------------------------------------------- + + +def _taskset( + *, + name: str = "set-1", + workspace: str = "default", + description: str | None = "A grouping.", + tasks: list[TaskRef] | None = None, +) -> TasksetEntity: + return TasksetEntity( + name=name, + workspace=workspace, + description=description, + tasks=tasks if tasks is not None else [TaskRef("default/task-a"), TaskRef("default/task-b")], + metadata=[], + ) + + +def test_taskset_digest_is_content_only() -> None: + assert content_hash(_taskset()) == content_hash(_taskset(name="set-2", workspace="other")) + + +def test_taskset_membership_change_changes_digest() -> None: + """A dataset's identity is its membership: changing a member must change the digest.""" + changed = _taskset(tasks=[TaskRef("default/task-a"), TaskRef("default/task-c")]) + assert content_hash(_taskset()) != content_hash(changed) + + +def test_taskset_description_change_changes_digest() -> None: + assert content_hash(_taskset()) != content_hash(_taskset(description="Different.")) diff --git a/plugins/nemo-evaluator/tests/test_revision_entity.py b/plugins/nemo-evaluator/tests/test_revision_entity.py new file mode 100644 index 0000000000..ca1dffb4bf --- /dev/null +++ b/plugins/nemo-evaluator/tests/test_revision_entity.py @@ -0,0 +1,205 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Revision entity shape and the head/revision digest invariant. + +The load-bearing property here: hashing a *head* record (excluding its revision pointers) must +yield the same digest as hashing the corresponding *revision* (excluding its own digest and +ordinal). Publish depends on it to recognize "this content is already published" — if the two +digests can't be compared, every publish allocates a new revision and dedup never fires. +""" + +from __future__ import annotations + +import re + +import pytest +from nemo_evaluator.api.schemas import MetadataItem, MetricRef, TaskInputs, TaskRef +from nemo_evaluator.content_hash import content_hash +from nemo_evaluator.entities import ( + REVISION_POINTER_FIELDS, + REVISION_SELF_FIELDS, + TaskEntity, + TaskRevisionEntity, + TasksetEntity, + TasksetRevisionEntity, +) +from nemo_platform_plugin.entity_naming import NAME_MAX_LENGTH, NAME_PATTERN +from pydantic import ValidationError + +_DIGEST = "a" * 64 +_OTHER_DIGEST = "b" * 64 + +_INTENT = "Answer the question." +_INPUTS = TaskInputs(instruction="What is 2+2?") +_METRICS = [MetricRef("default/stored-metric")] +_ANNOTATIONS = [MetadataItem(key="suite", value="smoke")] + +_DESCRIPTION = "A grouping." +_MEMBERS = [TaskRef(f"default/task-a#{_DIGEST}")] + + +def _task_head(*, intent: str = _INTENT, latest_revision: int = 0, tags: dict[str, int] | None = None) -> TaskEntity: + return TaskEntity( + name="task-1", + workspace="default", + intent=intent, + inputs=_INPUTS, + metrics=_METRICS, + metadata=_ANNOTATIONS, + latest_revision=latest_revision, + tags=tags or {}, + ) + + +def _task_revision(*, intent: str = _INTENT, revision: int = 1, digest: str = _DIGEST) -> TaskRevisionEntity: + return TaskRevisionEntity( + name=f"rev.{revision}", + workspace="default", + content_hash=digest, + revision=revision, + intent=intent, + inputs=_INPUTS, + metrics=_METRICS, + metadata=_ANNOTATIONS, + ) + + +def _taskset_head(*, members: list[TaskRef] | None = None) -> TasksetEntity: + return TasksetEntity( + name="set-1", + workspace="default", + description=_DESCRIPTION, + tasks=members if members is not None else _MEMBERS, + ) + + +def _taskset_revision(*, members: list[TaskRef] | None = None) -> TasksetRevisionEntity: + return TasksetRevisionEntity( + name="rev.1", + workspace="default", + content_hash=_DIGEST, + revision=1, + description=_DESCRIPTION, + tasks=members if members is not None else _MEMBERS, + ) + + +# --- The head/revision invariant --------------------------------------------- + + +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 + ) + + +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 + ) + + +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 + ) + + +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 + ) + + +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 + ) + + +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 + ) + + +# --- Published membership must be digest-pinned ------------------------------ + + +@pytest.mark.parametrize( + "unpinned", + [ + "default/task-a", # bare — resolves through `latest` + "task-a", # bare, workspace-relative + "default/task-a#latest", # the reserved moving tag + "default/task-a#candidate", # a user tag, equally mutable + f"default/task-a#{'a' * 63}", # truncated digest is not a digest + f"default/task-a#{'A' * 64}", # uppercase hex + ], +) +def test_published_taskset_rejects_unpinned_members(unpinned: str) -> None: + """Enforced on the field, not in the publish path, so no writer can bypass it. A tag-pinned + member would silently re-point the published revision the moment that tag moved.""" + with pytest.raises(ValidationError): + _taskset_revision(members=[TaskRef(unpinned)]) + + +def test_published_taskset_accepts_digest_pinned_members() -> None: + assert _taskset_revision(members=[TaskRef(f"other/task-a#{_OTHER_DIGEST}")]).tasks[0].root.endswith(_OTHER_DIGEST) + + +def test_head_taskset_still_accepts_unpinned_members() -> None: + """Only *published* membership must be pinned. The head is a working record; a bare ref there + means `latest`, which is exactly what publish resolves.""" + assert _taskset_head(members=[TaskRef("default/task-a")]).tasks[0].root == "default/task-a" + + +# --- Digest field validation ------------------------------------------------- + + +@pytest.mark.parametrize( + "bad", + [ + "a" * 63, # too short + "a" * 65, # too long + "A" * 64, # uppercase + "g" * 64, # non-hex + f"sha256:{'a' * 57}", # algorithm-prefixed + ], +) +def test_revision_rejects_malformed_digest(bad: str) -> None: + """The digest is what a consumer compares against on read; a malformed one must not persist.""" + with pytest.raises(ValidationError): + _task_revision(digest=bad) + + +def test_revision_ordinal_is_one_based() -> None: + with pytest.raises(ValidationError): + _task_revision(revision=0) + + +# --- Naming ------------------------------------------------------------------ + + +def test_ordinal_names_are_legal_entity_names() -> None: + """``rev.`` exists because the entity-name rules reject the alternatives.""" + for ordinal in (1, 9, 10, 12345): + assert re.match(NAME_PATTERN, f"rev.{ordinal}") + + +def test_a_full_digest_is_not_a_legal_entity_name() -> None: + """Why revisions aren't named by digest: 64 chars exceeds the cap, and a hex digest usually + starts with a digit while names must start with a lowercase letter.""" + assert len(_DIGEST) > NAME_MAX_LENGTH + assert not re.match(NAME_PATTERN, "0" + "a" * 63) + + +def test_bare_ordinal_is_not_a_legal_entity_name() -> None: + """Why the ``rev.`` prefix exists rather than naming a revision ``1``.""" + assert not re.match(NAME_PATTERN, "1") diff --git a/plugins/nemo-evaluator/tests/test_revisions.py b/plugins/nemo-evaluator/tests/test_revisions.py new file mode 100644 index 0000000000..b263389dfe --- /dev/null +++ b/plugins/nemo-evaluator/tests/test_revisions.py @@ -0,0 +1,803 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Revision publishing and resolution. + +Exercised against an in-memory store that reproduces the two entity-store behaviors this logic +leans on: parent-scoped name uniqueness (which is what serializes concurrent ordinal allocation) +and conflict-on-duplicate-create. +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable +from datetime import datetime, timedelta, timezone +from types import SimpleNamespace +from typing import TypeVar + +import pytest +from nemo_evaluator.api.schemas import LATEST_TAG, MetricRef, TaskInputs, TaskRef +from nemo_evaluator.entities import TaskEntity, TaskRevisionEntity +from nemo_evaluator.revisions import ( + RevisionConflictError, + RevisionContentMismatchError, + RevisionNotFoundError, + apply_tag, + get_revision, + head_digest, + list_revisions, + publish_revision, + revision_name, +) +from nemo_platform_plugin.entities import EntityBase +from nemo_platform_plugin.entity_client import NemoEntityConflictError, NemoEntityNotFoundError +from nemo_platform_plugin.filter_ops import FilterOperator, LogicalOperation + +_E = TypeVar("_E", bound=EntityBase) + + +class FakeStore: + """In-memory stand-in keyed the way the entity store keys records.""" + + def __init__(self) -> None: + self.records: dict[tuple[str, str, str, str | None], EntityBase] = {} + self._next_id = 0 + #: Monotonic tick for creation timestamps. ``list``/``find_by_digest`` order on + #: ``-created_at``, so leaving it unset would make ordering undefined — and sorting several + #: ``None`` timestamps raises rather than quietly returning insertion order. + self._tick = 0 + #: Ordinals to fail the first create for, simulating a lost allocation race. + self.contend_ordinals: set[int] = set() + #: Ordinals to lose to a publisher of *identical* content whose pointer write has not landed + #: yet — the interleaving that makes two concurrent identical requests each look novel. + self.contend_identically: set[int] = set() + #: One-shot hook fired just before a *head* update, modelling a competing publisher that + #: lands in the window between our child create and our pointer write. Lives on the fake + #: rather than a monkeypatch so the race is expressed the same way ``contend_ordinals`` is. + self.before_head_update: Callable[[], Awaitable[None]] | None = None + + def _key(self, entity_type: type[EntityBase], name: str, workspace: str, parent: str | None): + return (entity_type.__entity_type__, workspace, name, parent) + + async def create(self, entity: _E) -> _E: + key = self._key(type(entity), entity.name, entity.workspace, entity._parent) + if key in self.records: + raise NemoEntityConflictError(f"{entity.name} exists") + ordinal = getattr(entity, "revision", None) + if ordinal in self.contend_ordinals: + self.contend_ordinals.discard(ordinal) + self._win_race(entity, ordinal) + raise NemoEntityConflictError(f"{entity.name} taken by a concurrent publisher") + if ordinal in self.contend_identically: + self.contend_identically.discard(ordinal) + self._win_race(entity, ordinal, digest=entity.content_hash, advance_head=False) + raise NemoEntityConflictError(f"{entity.name} taken by a concurrent publisher") + self._next_id += 1 + self._tick += 1 + entity._id = f"id-{self._next_id}" + entity._created_at = datetime(2026, 1, 1, tzinfo=timezone.utc) + timedelta(seconds=self._tick) + self.records[key] = entity.model_copy(deep=True) + return entity + + def _win_race(self, losing_entity, ordinal: int, *, digest: str | None = None, advance_head: bool = True) -> None: + """Model a *complete* competing publish, not just a failed create. + + By default the winner's record exists under the contended name and the head has advanced. + Without both, the loser re-reads an unchanged head and recomputes the same ordinal — which + no real race would do, and which would make the retry look broken when it isn't. The + competitor publishes *different* content (its own digest), so the loser genuinely needs a + new ordinal rather than discovering its content already published. + + ``digest`` and ``advance_head`` express the opposite interleaving: a winner publishing the + *same* content whose pointer write has not landed yet. The loser then sees a head that still + names the previous revision, so nothing but the contended child itself reveals that its + content is already published. + """ + winner = losing_entity.model_copy(update={"content_hash": digest or f"{ordinal:064x}"}) + winner._parent = losing_entity._parent + self._next_id += 1 + winner._id = f"id-{self._next_id}" + self._tick += 1 + winner._created_at = datetime(2026, 1, 1, tzinfo=timezone.utc) + timedelta(seconds=self._tick) + self.records[self._key(type(winner), winner.name, winner.workspace, winner._parent)] = winner + if not advance_head: + return + head = self.records.get(self._key(TaskEntity, "task-1", winner.workspace, None)) + if head is not None: + head.latest_revision = max(head.latest_revision, ordinal) + head.tags = {**head.tags, LATEST_TAG: ordinal} + + async def get(self, entity_type: type[_E], name, *, workspace=None, parent=None) -> _E: + """Hand back a *copy*, as the real client does — it rebuilds entities from an HTTP response + and cannot share objects with its caller. Returning the stored instance would let code + under test mutate the store just by touching what it read.""" + key = self._key(entity_type, name, workspace or "default", parent) + if key not in self.records: + raise NemoEntityNotFoundError(f"{name} not found") + stored = self.records[key] + copy = stored.model_copy(deep=True) + copy._parent, copy._id, copy._db_version = stored._parent, stored._id, stored._db_version + return copy + + async def update(self, entity: _E, *, original_name=None) -> _E: + """Enforce the ``db_version`` optimistic lock the real store enforces. + + A fake that accepts every update makes correct retry logic untestable and incorrect retry + logic look fine, so this rejects a write whose base version is stale. + """ + if self.before_head_update is not None and isinstance(entity, TaskEntity): + hook, self.before_head_update = self.before_head_update, None + await hook() + key = self._key(type(entity), original_name or entity.name, entity.workspace, entity._parent) + stored = self.records.get(key) + if stored is not None and entity._db_version != stored._db_version: + raise NemoEntityConflictError( + f"stale update for {entity.name}: base version {entity._db_version}, " + f"stored version {stored._db_version}" + ) + saved = entity.model_copy(deep=True) + saved._parent = entity._parent + saved._id = entity._id + saved._db_version = (stored._db_version if stored is not None else 0) + 1 + self.records[key] = saved + return saved + + async def delete(self, entity_type, name, *, workspace, parent=None, expected_db_version=None) -> object: + """Part of the standard client surface, so the fake carries it even though publishing and + resolving never delete — a stand-in that omits it would not be substitutable.""" + key = self._key(entity_type, name, workspace, parent) + if key not in self.records: + raise NemoEntityNotFoundError(f"{name} not found") + return self.records.pop(key) + + async def list(self, entity_type, *, workspace, filter_operation=None, sort=None, page=1, page_size=100): + """Evaluate the filter the way the store does, so the query path is actually exercised. + + Supports exactly what this module emits: an AND of equality comparisons over ``parent`` and + ``data.``. Anything else raises rather than silently matching everything — a fake + that ignores filters would make a broken query look correct. ``sort`` and ``page`` are + honoured for the same reason: accepting them and returning insertion order would make a + caller that ordered wrongly look right. + """ + rows = [ + record + for (entity_type_name, record_workspace, _, _), record in self.records.items() + if entity_type_name == entity_type.__entity_type__ and record_workspace == workspace + ] + for comparison in self._comparisons(filter_operation): + if comparison.operator is not FilterOperator.EQ: + raise NotImplementedError(f"fake supports only EQ, got {comparison.operator}") + if comparison.field == "parent": + rows = [r for r in rows if r.parent == comparison.value] + elif comparison.field.startswith("data."): + attribute = comparison.field.removeprefix("data.") + rows = [r for r in rows if getattr(r, attribute, None) == comparison.value] + else: + raise NotImplementedError(f"fake cannot filter on {comparison.field!r}") + if sort and rows: + field = sort.lstrip("-") + if not hasattr(rows[0], field): + raise NotImplementedError(f"fake cannot sort on {field!r}") + rows = sorted(rows, key=lambda record: getattr(record, field), reverse=sort.startswith("-")) + start = (page - 1) * page_size + return SimpleNamespace(data=rows[start : start + page_size]) + + def _comparisons(self, operation): + if operation is None: + return [] + if isinstance(operation, LogicalOperation): + if operation.operator is not FilterOperator.AND: + raise NotImplementedError(f"fake supports only AND, got {operation.operator}") + return [c for child in operation.operations for c in self._comparisons(child)] + return [operation] + + def concurrent_head_write(self, head: EntityBase, *, tags: dict[str, int]) -> None: + """Simulate another publisher committing to the head between our read and our write.""" + key = self._key(type(head), head.name, head.workspace, None) + stored = self.records[key] + stored.tags = {**stored.tags, **tags} + stored.latest_revision = max([stored.latest_revision, *tags.values()], default=stored.latest_revision) + stored._db_version += 1 + + +def _head(store: FakeStore, *, intent: str = "Answer the question.") -> TaskEntity: + head = TaskEntity( + 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 + # Store a *distinct* copy: the real client returns freshly deserialized objects, so a caller's + # in-memory head and the stored record are never the same object. Aliasing them would make + # every optimistic-lock conflict invisible (bumping one bumps the other). + stored = head.model_copy(deep=True) + stored._id = head.id + stored._db_version = 0 + store.records[store._key(TaskEntity, "task-1", "default", None)] = stored + return head + + +def _head_named(store: FakeStore, name: str) -> TaskEntity: + """A second record with content identical to :func:`_head`'s — same digest, different parent.""" + head = TaskEntity( + 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 + stored = head.model_copy(deep=True) + stored._id = head.id + stored._db_version = 0 + store.records[store._key(TaskEntity, name, "default", None)] = stored + return head + + +async def _publish(store: FakeStore, head: TaskEntity, *, tags: set[str] | None = None): + """Publish and drop the returned head — these tests assert against their own head object.""" + revision, _head, created = await publish_revision(store, store, head, TaskRevisionEntity, tags=tags) + return revision, created + + +# --- First publish ----------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_first_publish_creates_revision_one() -> None: + store = FakeStore() + head = _head(store) + revision, created = await _publish(store, head) + assert created + assert revision.revision == 1 + assert revision.name == revision_name(1) + assert revision.content_hash == head_digest(head) + + +@pytest.mark.asyncio +async def test_first_publish_points_latest_at_revision_one() -> None: + store = FakeStore() + head = _head(store) + revision, _ = await _publish(store, head) + assert head.tags[LATEST_TAG] == revision.revision + assert head.latest_revision == 1 + + +@pytest.mark.asyncio +async def test_revision_is_a_child_of_its_head() -> None: + """Parent scoping is what makes ordinals collide instead of silently duplicating.""" + store = FakeStore() + head = _head(store) + revision, _ = await _publish(store, head) + assert revision.parent == head.id + + +# --- Idempotency ------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_republishing_identical_content_creates_nothing() -> None: + """The property that makes a re-publish cheap: same content, same digest, no new revision.""" + store = FakeStore() + head = _head(store) + first, created_first = await _publish(store, head) + second, created_second = await _publish(store, head) + assert created_first and not created_second + assert second.revision == first.revision + assert head.latest_revision == 1 + + +@pytest.mark.asyncio +async def test_republishing_identical_content_still_applies_new_tags() -> None: + """A no-op publish is not a no-op tag operation — that's how you tag an existing revision.""" + store = FakeStore() + head = _head(store) + revision, _ = await _publish(store, head) + _, created = await _publish(store, head, tags={"blessed"}) + assert not created + assert head.tags["blessed"] == revision.revision + + +@pytest.mark.asyncio +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." + second, created = await _publish(store, head) + assert created + assert second.revision == 2 + assert second.content_hash != first.content_hash + assert head.tags[LATEST_TAG] == second.revision + assert head.latest_revision == 2 + + +# --- Concurrency ------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_contended_ordinal_is_retried() -> None: + """A publisher that loses the race for ordinal N re-reads and takes N+1 rather than failing.""" + store = FakeStore() + head = _head(store) + await _publish(store, head) + head.intent = "Changed." + store.contend_ordinals = {2} + revision, created = await _publish(store, head) + assert created + assert revision.revision == 3 + + +@pytest.mark.asyncio +async def test_identical_contended_publish_adopts_the_winners_revision() -> None: + """Two identical publishes that overlap must still cut exactly one revision. + + The dedup check reads the revision ``latest`` names, so it misses a winner whose pointer write + has not landed: the loser sees a head still naming ``N-1``, computes the same digest, and would + step past the contended ordinal onto a byte-identical ``rev.N+1``. Both callers would then + report a new revision for the same content, which is exactly what publishing is supposed to be + idempotent against. + """ + store = FakeStore() + head = _head(store) + await _publish(store, head) + + head.intent = "Changed." + store.contend_identically = {2} + revision, created = await _publish(store, head) + + assert not created, "the loser must report the winner's revision, not a publish of its own" + assert revision.revision == 2, "adopt the contended ordinal rather than allocating past it" + + page = await list_revisions(store, TaskRevisionEntity, head) + assert [entry.revision for entry in page.data] == [2, 1], "no duplicate-content revision was cut" + + stored = store.records[store._key(TaskEntity, head.name, head.workspace, None)] + assert isinstance(stored, TaskEntity) + assert stored.tags[LATEST_TAG] == 2, "adopting must still point latest at the revision it adopted" + + +@pytest.mark.asyncio +async def test_contended_publish_of_different_content_still_allocates_a_new_ordinal() -> None: + """Adoption is keyed on the digest, not on losing the race. + + The guard added for identical publishes must not swallow a genuine one: a loser whose content + differs from the winner's still needs its own ordinal. + """ + store = FakeStore() + head = _head(store) + await _publish(store, head) + + head.intent = "Changed." + store.contend_ordinals = {2} # winner publishes *different* content + revision, created = await _publish(store, head) + + assert created + assert revision.revision == 3 + + +@pytest.mark.asyncio +async def test_publishing_recovers_when_a_revision_exists_but_the_head_never_advanced() -> None: + """The one case where losing a create does *not* mean someone else advanced the head. + + If a publisher creates ``rev.N`` and then exhausts its pointer-write retries, the child exists + while the head still names ``N-1``. A later publish computes N, loses the create, re-reads the + head — which still says ``N-1`` — and computes N again, every attempt until it gives up. Every + publish after that does the same, so the record would be permanently unpublishable rather than + merely wasteful. The failed create is proof enough that N is taken. + """ + store = FakeStore() + head = _head(store) + await _publish(store, head) + + # Simulate the pointer write never landing: rev.1 exists, the head still names revision 0. + head.latest_revision, head.tags = 0, {} + stored = store.records[store._key(TaskEntity, head.name, head.workspace, None)] + assert isinstance(stored, TaskEntity) + stored.latest_revision, stored.tags = 0, {} + + head.intent = "Changed." + revision, created = await _publish(store, head) + + assert created + assert revision.revision == 2, "must step past the orphaned rev.1 instead of retrying it" + + +@pytest.mark.asyncio +async def test_losing_the_head_race_does_not_leave_the_head_on_an_older_revision() -> None: + """A publisher that loses the *pointer* race must not drag the head's content backwards. + + A creates rev.2 and stalls; B publishes rev.3 and commits first; A's pointer write then loses + the lock and retries. ``latest`` correctly stays at 3 — but if A's staged content still landed + on the head, a plain read would return rev.2's content while ``#latest`` returned rev.3's, and + the record would report itself as revision 3 while serving something else. + """ + store = FakeStore() + head = _head(store) + await publish_revision(store, store, head, TaskRevisionEntity) # rev.1 + + 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." + + async def b_publishes() -> None: + await publish_revision(store, store, b, TaskRevisionEntity) + + store.before_head_update = b_publishes + a_revision, _, _ = await publish_revision(store, store, a, TaskRevisionEntity) + + stored = store.records[store._key(TaskEntity, "task-1", "default", None)] + assert isinstance(stored, TaskEntity) + 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.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." + + +@pytest.mark.asyncio +async def test_persistent_contention_raises_rather_than_looping() -> None: + store = FakeStore() + head = _head(store) + store.contend_ordinals = set(range(1, 50)) + with pytest.raises(RevisionConflictError): + await _publish(store, head) + + +@pytest.mark.asyncio +async def test_latest_revision_never_rewinds() -> None: + """Re-tagging an older revision must not hand the next publish an ordinal already in use. + + ``latest_revision`` is the allocation watermark, not a pointer: pointing a user tag backwards + is legitimate, but if that dragged the watermark back with it, the next publish would compute + an ordinal that already exists and lose its create. + """ + store = FakeStore() + head = _head(store) + first, _ = await _publish(store, head) + head.intent = "Changed." + await _publish(store, head) + + head = await store.get(TaskEntity, "task-1", workspace="default") + await apply_tag(store, store, TaskRevisionEntity, head, "rollback", first.content_hash) + + stored = store.records[store._key(TaskEntity, "task-1", "default", None)] + assert stored.tags["rollback"] == first.revision + assert stored.latest_revision == 2, "the watermark must not follow a backwards tag" + assert stored.tags[LATEST_TAG] == 2 + + +@pytest.mark.asyncio +async def test_head_update_retries_on_optimistic_lock_conflict() -> None: + """A publisher that loses the head write re-reads and folds its pointers into the winner's + record, rather than propagating the conflict or overwriting with a stale copy.""" + store = FakeStore() + head = _head(store) + store.concurrent_head_write(head, tags={"other": 1}) + revision, created = await _publish(store, head) + assert created + stored = store.records[store._key(TaskEntity, "task-1", "default", None)] + assert stored.tags["other"] == 1, "the winner's tag must survive our retry" + assert stored.tags[LATEST_TAG] == revision.revision + + +@pytest.mark.asyncio +async def test_persistent_head_contention_raises() -> None: + """Bounded retries: a head that keeps moving must fail loudly, not loop forever.""" + + store = FakeStore() + head = _head(store) + original_update = store.update + + async def always_stale(entity, *, original_name=None): + store.concurrent_head_write(entity, tags={}) + return await original_update(entity, original_name=original_name) + + store.update = always_stale # type: ignore[method-assign] + with pytest.raises(RevisionConflictError): + await _publish(store, head) + + +@pytest.mark.asyncio +async def test_latest_does_not_regress_when_writes_interleave() -> None: + """The interleaving that motivates forward-only ``latest``: A creates rev.1 and B creates + rev.2, but B commits its head pointers first. A's later write must not drag ``latest`` back + onto rev.1 while ``latest_revision`` says 2.""" + store = FakeStore() + head = _head(store) + + # A has staged content and is about to publish as rev.1. B publishes rev.2 and commits its head + # pointers first, so A's pointer write loses the optimistic lock and retries against B's head. + store.concurrent_head_write(head, tags={LATEST_TAG: 2}) + + revision, created = await _publish(store, head) + + assert created and revision.revision == 1 + stored = store.records[store._key(TaskEntity, "task-1", "default", None)] + assert stored.tags[LATEST_TAG] == 2, "latest must not regress onto A's older revision" + assert stored.latest_revision == 2 + + +@pytest.mark.asyncio +async def test_user_tags_may_be_moved_backwards() -> None: + """Only ``latest`` is forward-only. Retagging onto an older revision is a rollback — explicit + user intent, not a race — and must be honoured.""" + store = FakeStore() + head = _head(store) + older, _ = await _publish(store, head, tags={"blessed"}) + head.intent = "Newer content." + await _publish(store, head) + + head = await store.get(TaskEntity, "task-1", workspace="default") + await apply_tag(store, store, TaskRevisionEntity, head, "blessed", older.content_hash) + + stored = store.records[store._key(TaskEntity, "task-1", "default", None)] + assert stored.tags["blessed"] == older.revision + assert stored.tags[LATEST_TAG] == 2, "only the user tag moves; latest stays put" + + +@pytest.mark.asyncio +async def test_reverting_to_earlier_content_publishes_a_new_revision() -> None: + """Dedup is against the *current* revision, not the whole history. + + Reverting is a real change to what the record is now. If it deduped onto the old revision the + head would hold that content while ``latest`` — forward-only — kept naming the newer one, so a + plain read and a ``#latest`` read would disagree about the same record. + """ + store = FakeStore() + head = _head(store) + await _publish(store, head) # rev.1: "Answer the question." + head.intent = "Changed." + await _publish(store, head) # rev.2 + + head.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" + assert revision.revision == 3 + 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" + + +@pytest.mark.asyncio +async def test_a_digest_shared_by_two_revisions_resolves_to_the_newer_one() -> None: + """Reverting makes one record hold two revisions with the same digest, so a digest lookup has + to pick deterministically. Both carry identical content — all a pin promises — but a pin must + not resolve to a different ordinal from one call to the next.""" + store = FakeStore() + head = _head(store) + first, _ = await _publish(store, head) # rev.1 + head.intent = "Changed." + await _publish(store, head) # rev.2 + head.intent = "Answer the question." + third, _ = await _publish(store, head) # rev.3, same digest as rev.1 + + assert third.content_hash == first.content_hash + + for _ in range(3): + resolved = await get_revision(store, TaskRevisionEntity, head, first.content_hash) + assert resolved.revision == third.revision + + +@pytest.mark.asyncio +async def test_republishing_the_current_content_is_still_a_no_op() -> None: + """The idempotency the narrower dedup must not cost: re-PUTting unchanged content.""" + store = FakeStore() + head = _head(store) + first, _ = await _publish(store, head) + revision, created = await _publish(store, head) + + assert not created + assert revision.revision == first.revision + + +# --- Resolution -------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_resolves_latest_by_default() -> None: + store = FakeStore() + head = _head(store) + await _publish(store, head) + head.intent = "Changed." + second, _ = await _publish(store, head) + assert (await get_revision(store, TaskRevisionEntity, head)).content_hash == second.content_hash + + +@pytest.mark.asyncio +async def test_resolves_a_digest_to_its_revision() -> None: + store = FakeStore() + head = _head(store) + first, _ = await _publish(store, head) + head.intent = "Changed." + await _publish(store, head) + resolved = await get_revision(store, TaskRevisionEntity, head, first.content_hash) + assert resolved.revision == 1 + + +@pytest.mark.asyncio +async def test_resolves_a_user_tag() -> None: + store = FakeStore() + head = _head(store) + revision, _ = await _publish(store, head, tags={"blessed"}) + assert (await get_revision(store, TaskRevisionEntity, head, "blessed")).revision == revision.revision + + +@pytest.mark.asyncio +async def test_unknown_fragment_raises() -> None: + store = FakeStore() + head = _head(store) + await _publish(store, head) + with pytest.raises(RevisionNotFoundError): + await get_revision(store, TaskRevisionEntity, head, "nonexistent") + + +@pytest.mark.asyncio +async def test_index_pointing_at_a_missing_record_raises() -> None: + """A torn write must surface, not resolve to nothing.""" + store = FakeStore() + head = _head(store) + revision, _ = await _publish(store, head) + del store.records[store._key(TaskRevisionEntity, revision.name, "default", head.id)] + with pytest.raises(RevisionNotFoundError): + await get_revision(store, TaskRevisionEntity, head) + + +@pytest.mark.asyncio +async def test_unknown_digest_raises() -> None: + store = FakeStore() + head = _head(store) + await _publish(store, head) + with pytest.raises(RevisionNotFoundError): + await get_revision(store, TaskRevisionEntity, head, "c" * 64) + + +@pytest.mark.asyncio +async def test_digest_resolution_is_scoped_to_the_parent() -> None: + """Identical content under two records yields one digest, so the digest alone does not identify + a revision. Without parent scoping this would resolve to the other record's revision.""" + store = FakeStore() + mine = _head(store) + theirs = _head_named(store, "task-2") + published, _, _ = await publish_revision(store, store, theirs, TaskRevisionEntity) + + assert head_digest(mine) == published.content_hash, "same content, same digest" + with pytest.raises(RevisionNotFoundError): + await get_revision(store, TaskRevisionEntity, mine, published.content_hash) + + +@pytest.mark.asyncio +async def test_identical_content_under_two_records_publishes_independently() -> None: + """The flip side: one record's publish must not make another's look already-published.""" + store = FakeStore() + mine = _head(store) + theirs = _head_named(store, "task-2") + await publish_revision(store, store, theirs, TaskRevisionEntity) + revision, created = await _publish(store, mine) + assert created and revision.revision == 1 + + +@pytest.mark.asyncio +async def test_reading_a_revision_whose_content_was_tampered_with_is_refused() -> None: + """The digest has to be checked on the way out or it is only a label. + + A revision is immutable by convention; the store will still accept a write to one. If reading + trusted the recorded digest, a pinned ref would keep resolving and quietly serve content nobody + pinned — the exact failure content-addressing exists to prevent. + """ + store = FakeStore() + head = _head(store) + revision, _ = await _publish(store, head) + + stored = store.records[store._key(TaskRevisionEntity, revision_name(1), "default", head.id)] + assert isinstance(stored, TaskRevisionEntity) + stored.intent = "Tampered with after publication." + + with pytest.raises(RevisionContentMismatchError, match="does not match its recorded digest"): + await get_revision(store, TaskRevisionEntity, head, LATEST_TAG) + + +@pytest.mark.asyncio +async def test_a_digest_pinned_read_is_verified_too() -> None: + """Both resolution paths verify — a digest lookup filters on the *recorded* digest, so without + re-hashing it would happily return a record whose content no longer matches it.""" + store = FakeStore() + head = _head(store) + revision, _ = await _publish(store, head) + + stored = store.records[store._key(TaskRevisionEntity, revision_name(1), "default", head.id)] + assert isinstance(stored, TaskRevisionEntity) + stored.intent = "Tampered with after publication." + + with pytest.raises(RevisionContentMismatchError): + await get_revision(store, TaskRevisionEntity, head, revision.content_hash) + + +# --- Tag validation ----------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_a_digest_shaped_tag_is_rejected() -> None: + """Such a tag could be stored but never resolved: a digest-shaped reference is looked up as a + digest, so the tag map would never be consulted. Rejected rather than silently useless.""" + store = FakeStore() + head = _head(store) + with pytest.raises(ValueError, match="looks like a content digest"): + await _publish(store, head, tags={"a" * 64}) + + +@pytest.mark.asyncio +async def test_latest_cannot_be_moved_by_hand() -> None: + """Refused where it matters: pointing ``latest`` somewhere would break the forward-only rule.""" + store = FakeStore() + head = _head(store) + revision, _ = await _publish(store, head) + with pytest.raises(ValueError, match="managed automatically"): + await apply_tag(store, store, TaskRevisionEntity, head, LATEST_TAG, revision.content_hash) + + +@pytest.mark.asyncio +async def test_latest_in_publish_tags_is_tolerated() -> None: + """Listing ``latest`` at publish time asks for exactly what the server does anyway, so it is a + no-op rather than an error. Rejecting it would break clients that pass it defensively — which + is what an earlier version of this validation did.""" + store = FakeStore() + head = _head(store) + revision, created = await _publish(store, head, tags={LATEST_TAG, "blessed"}) + assert created + assert head.tags[LATEST_TAG] == revision.revision + assert head.tags["blessed"] == revision.revision + + +@pytest.mark.asyncio +async def test_an_empty_tag_is_rejected() -> None: + """An absent fragment already means ``latest``, so an empty tag could never be addressed.""" + store = FakeStore() + head = _head(store) + with pytest.raises(ValueError, match="must not be empty"): + await _publish(store, head, tags={" "}) + + +@pytest.mark.asyncio +async def test_a_nearly_digest_shaped_tag_is_allowed() -> None: + """The rejection is exact — 64 lowercase hex — so ordinary names stay usable.""" + store = FakeStore() + head = _head(store) + _, created = await _publish(store, head, tags={"a" * 63, "deadbeef"}) + assert created + assert head.tags["deadbeef"] == 1 + + +@pytest.mark.parametrize("tag", ["release/2026", "release candidate"]) +@pytest.mark.asyncio +async def test_a_tag_outside_the_fragment_charset_is_rejected(tag: str) -> None: + """A tag exists to be written after ``#`` in a member reference. ``TaskRef`` admits only + ``[\\w\\-.]+`` there, so a tag with a slash or a space would apply and list cleanly and then be + unusable for the one thing it is for — the same silent dead end as a digest-shaped tag.""" + store = FakeStore() + head = _head(store) + with pytest.raises(ValueError, match="cannot appear in a reference fragment"): + await _publish(store, head, tags={tag}) + + +@pytest.mark.asyncio +async def test_an_accepted_tag_is_usable_as_a_ref_fragment() -> None: + """The other half of the rule: whatever publishing accepts, ``TaskRef`` must also accept. Pins + the two charsets together so neither can drift into rejecting the other's output.""" + store = FakeStore() + head = _head(store) + await _publish(store, head, tags={"blessed", "v1.2.3", "rc_2"}) + + for tag in head.tags: + TaskRef(f"{head.workspace}/{head.name}#{tag}") diff --git a/plugins/nemo-evaluator/tests/test_subentity_refs.py b/plugins/nemo-evaluator/tests/test_subentity_refs.py new file mode 100644 index 0000000000..e26dc7cc71 --- /dev/null +++ b/plugins/nemo-evaluator/tests/test_subentity_refs.py @@ -0,0 +1,99 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Sub-entity (``#fragment``) reference parsing and validation. + +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). +""" + +from __future__ import annotations + +import pytest +from nemo_evaluator.api.schemas import ( + LATEST_TAG, + MetricRef, + TaskRef, + TasksetRef, + parse_entity_ref, + parse_subentity_ref, +) +from pydantic import ValidationError + +_DIGEST = "a" * 64 + + +# --- Parsing ----------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("ref", "expected"), + [ + ("task-a", ("default", "task-a", LATEST_TAG)), + ("other/task-a", ("other", "task-a", LATEST_TAG)), + ("task-a#latest", ("default", "task-a", "latest")), + ("other/task-a#candidate", ("other", "task-a", "candidate")), + (f"other/task-a#{_DIGEST}", ("other", "task-a", _DIGEST)), + ], +) +def test_parse_subentity_ref(ref: str, expected: tuple[str, str, str]) -> None: + assert parse_subentity_ref(ref, "default") == expected + + +def test_absent_fragment_means_latest_not_unpinned() -> None: + """A bare ref is "the current revision", which is what makes `#latest` a real default rather + than a special case the resolver has to guess at.""" + _, _, fragment = parse_subentity_ref("task-a", "default") + assert fragment == LATEST_TAG + + +def test_empty_fragment_falls_back_to_latest() -> None: + """``task-a#`` is degenerate input; treat it as unpinned rather than as a tag named ''.""" + assert parse_subentity_ref("task-a#", "default") == ("default", "task-a", LATEST_TAG) + + +def test_fragment_is_returned_verbatim() -> None: + """Parsing does not decide whether a fragment is a tag or a digest — that's resolution's job.""" + _, _, fragment = parse_subentity_ref(f"task-a#{_DIGEST}", "default") + assert fragment == _DIGEST + + +# --- Backward compatibility -------------------------------------------------- + + +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_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") + + +# --- Field validation -------------------------------------------------------- + + +@pytest.mark.parametrize("ref", ["task-a", "other/task-a", "task-a#latest", f"other/task-a#{_DIGEST}"]) +def test_task_ref_accepts_fragments(ref: str) -> None: + assert TaskRef(ref).root == ref + + +@pytest.mark.parametrize("ref", ["task-a#one#two", "task-a#bad/frag", "#latest", "task a#latest"]) +def test_task_ref_rejects_malformed_fragments(ref: str) -> None: + with pytest.raises(ValidationError): + TaskRef(ref) + + +@pytest.mark.parametrize("ref_type", [MetricRef, TasksetRef]) +def test_sibling_ref_types_still_reject_fragments(ref_type: type) -> None: + """The fragment pattern is a sibling, not a widening of the shared constant: metrics and + tasksets have no revisions yet, so admitting a fragment would accept input nothing resolves. + They move onto it when they gain revisions — deliberately, at that point.""" + with pytest.raises(ValidationError): + ref_type(f"other/thing#{_DIGEST}") diff --git a/plugins/nemo-evaluator/tests/test_task_refs.py b/plugins/nemo-evaluator/tests/test_task_refs.py index 97b1beb62b..9da12bcc2b 100644 --- a/plugins/nemo-evaluator/tests/test_task_refs.py +++ b/plugins/nemo-evaluator/tests/test_task_refs.py @@ -9,35 +9,15 @@ import pytest from nemo_evaluator.api.schemas import MetadataItem, MetricRef, TaskInputs, TaskRef, TasksetRef -from nemo_evaluator.entities import TaskEntity, TasksetEntity +from nemo_evaluator.entities import TaskEntity, TaskRevisionEntity, TasksetEntity from nemo_evaluator.jobs.agent_spec import AgentEvalTaskInput +from nemo_evaluator.revisions import head_digest, publish_revision from nemo_evaluator.task_refs import resolve_agent_eval_tasks, resolve_taskset_ref from nemo_platform_plugin.entities import EntityBase -from nemo_platform_plugin.entity_client import NemoEntityNotFoundError _EntityT = TypeVar("_EntityT", bound=EntityBase) -class _FakeEntityClient: - """Minimal entity store keyed by (type, workspace, name), mirroring EntityClient.get.""" - - def __init__(self) -> None: - self.entities: dict[tuple[str, str, str], EntityBase] = {} - - def add(self, entity: EntityBase) -> None: - self.entities[(entity.__entity_type__, entity.workspace, entity.name)] = entity - - async def get( - self, entity_type: type[_EntityT], *, workspace: str, name: str, parent: str | None = None - ) -> _EntityT: - key = (entity_type.__entity_type__, workspace, name) - if key not in self.entities: - raise NemoEntityNotFoundError(f"{workspace}/{name} not found") - entity = self.entities[key] - assert isinstance(entity, entity_type) - return entity - - def _task(name: str, *, workspace: str = "default", metric: str = "default/m") -> TaskEntity: return TaskEntity( name=name, @@ -53,15 +33,22 @@ def _taskset(name: str, task_refs: list[str], *, workspace: str = "default") -> return TasksetEntity(name=name, workspace=workspace, tasks=[TaskRef(r) for r in task_refs]) -def _store(*entities: EntityBase) -> _FakeEntityClient: - client = _FakeEntityClient() +async def _store(client, *entities: EntityBase): + """Build a store and *publish* every task, so members resolve to a real revision. + + Tasks are published rather than merely inserted because taskset expansion now reads the pinned + revision's content, not the head's — the same thing the service does on create. + """ for entity in entities: - client.add(entity) + await client.create(entity) + if isinstance(entity, TaskEntity): + await publish_revision(client, client, entity, TaskRevisionEntity) return client -async def test_resolves_taskset_members_to_inline_task_inputs() -> None: - client = _store( +async def test_resolves_taskset_members_to_inline_task_inputs(entity_store) -> None: + client = await _store( + entity_store, _task("capital-of-france"), _task("capital-of-japan"), _taskset("geo", ["default/capital-of-france", "default/capital-of-japan"]), @@ -79,34 +66,37 @@ async def test_resolves_taskset_members_to_inline_task_inputs() -> None: assert tasks[0].reference == {} -async def test_bare_member_ref_resolves_against_taskset_workspace() -> None: - client = _store(_task("t1", workspace="team"), _taskset("ts", ["t1"], workspace="team")) +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")) tasks = await resolve_taskset_ref(TasksetRef("team/ts"), workspace="default", entity_client=client) assert [t.id for t in tasks] == ["t1"] -async def test_unknown_taskset_raises_clear_error() -> None: +async def test_unknown_taskset_raises_clear_error(entity_store) -> None: with pytest.raises(ValueError, match="Taskset reference 'default/missing' not found"): - await resolve_taskset_ref(TasksetRef("default/missing"), workspace="default", entity_client=_store()) + await resolve_taskset_ref( + TasksetRef("default/missing"), workspace="default", entity_client=await _store(entity_store) + ) -async def test_missing_member_task_raises_clear_error() -> None: - client = _store(_taskset("geo", ["default/gone"])) +async def test_missing_member_task_raises_clear_error(entity_store) -> None: + client = await _store(entity_store, _taskset("geo", ["default/gone"])) with pytest.raises(ValueError, match="Task 'default/gone' referenced by taskset 'default/geo'"): await resolve_taskset_ref(TasksetRef("default/geo"), workspace="default", entity_client=client) -async def test_empty_taskset_raises_clear_error() -> None: - client = _store(_taskset("empty", [])) +async def test_empty_taskset_raises_clear_error(entity_store) -> None: + client = await _store(entity_store, _taskset("empty", [])) with pytest.raises(ValueError, match="has no member tasks"): await resolve_taskset_ref(TasksetRef("default/empty"), workspace="default", entity_client=client) -async def test_duplicate_expanded_task_ids_rejected() -> None: +async def test_duplicate_expanded_task_ids_rejected(entity_store) -> None: # Two members from different workspaces share the name 'dup' -> ambiguous task id. - client = _store( + client = await _store( + entity_store, _task("dup", workspace="a"), _task("dup", workspace="b"), _taskset("geo", ["a/dup", "b/dup"]), @@ -115,18 +105,52 @@ async def test_duplicate_expanded_task_ids_rejected() -> None: await resolve_taskset_ref(TasksetRef("default/geo"), workspace="default", entity_client=client) -async def test_taskset_ref_requires_entity_client() -> None: +async def test_taskset_ref_requires_entity_client(entity_store) -> None: with pytest.raises(ValueError, match="requires a platform connection"): await resolve_taskset_ref(TasksetRef("default/geo"), workspace="default", entity_client=None) -async def test_resolve_agent_eval_tasks_passes_inline_list_through() -> None: +async def test_resolve_agent_eval_tasks_passes_inline_list_through(entity_store) -> None: inline = [AgentEvalTaskInput(id="t", intent="x", metrics=[])] result = await resolve_agent_eval_tasks(inline, workspace="default", entity_client=None) assert result is inline -async def test_resolve_agent_eval_tasks_expands_a_taskset_ref() -> None: - client = _store(_task("only"), _taskset("geo", ["default/only"])) +async def test_resolve_agent_eval_tasks_expands_a_taskset_ref(entity_store) -> None: + client = await _store(entity_store, _task("only"), _taskset("geo", ["default/only"])) result = await resolve_agent_eval_tasks(TasksetRef("default/geo"), workspace="default", entity_client=client) assert [t.id for t in result] == ["only"] + + +async def test_expansion_uses_the_pinned_revision_not_current_content(entity_store) -> None: + """The property the whole pinning design exists for: an evaluation re-run expands to the same + content even after a member task has published newer content. + + Before this was wired, expansion read the member's *head*, silently defeating the pin — the + taskset looked reproducible and wasn't. + """ + task = _task("capital-of-france") + client = await _store(entity_store, task) + pinned_digest = head_digest(task) + + taskset = _taskset("geo", [f"default/capital-of-france#{pinned_digest}"]) + await client.create(taskset) + + # The member publishes newer content after the taskset was pinned. + task.intent = "Something else entirely." + 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].intent == "Do capital-of-france.", "expansion must return the pinned content" + + +async def test_expansion_fails_loudly_when_a_pin_no_longer_resolves(entity_store) -> None: + """Verify-on-read at the point it matters most: a pin that cannot be honoured must stop the + evaluation rather than quietly substituting whatever is current.""" + client = await _store(entity_store, _task("only")) + await client.create(_taskset("geo", [f"default/only#{'c' * 64}"])) + + with pytest.raises(ValueError, match="no longer resolves"): + await resolve_taskset_ref(TasksetRef("default/geo"), workspace="default", entity_client=client) diff --git a/services/core/entities/tests/repository/test_child_entity_filtering.py b/services/core/entities/tests/repository/test_child_entity_filtering.py new file mode 100644 index 0000000000..6148e347a0 --- /dev/null +++ b/services/core/entities/tests/repository/test_child_entity_filtering.py @@ -0,0 +1,117 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Filtering child entities by parent and by a custom (``data.*``) field. + +``list_entities`` takes no ``parent`` argument, which reads as "you cannot query children". These +tests establish that you can: ``parent`` is a mapped column, so it is reachable through an ordinary +``filter_op`` — no API change needed. Combined with ``data.*`` JSON extraction, that covers +"find the child of this parent whose custom field equals X", which is what a content-addressed +revision lookup needs. +""" + +import pytest +from nmp.common.api.filter import ComparisonOperation, FilterOperator, LogicalOperation +from nmp.core.entities.app.repository import SQLAlchemyEntityRepository + +pytestmark = pytest.mark.asyncio + +_DIGEST_A = "a" * 64 +_DIGEST_B = "b" * 64 + + +async def _seed(entity_repo: SQLAlchemyEntityRepository): + """Two parents, each with two revision children; digests repeat across parents on purpose.""" + parents = {} + for parent_name in ("task-one", "task-two"): + parent = await entity_repo.create_entity(workspace="workspace-1", entity_type="task", name=parent_name, data={}) + parents[parent_name] = parent + for ordinal, digest in ((1, _DIGEST_A), (2, _DIGEST_B)): + await entity_repo.create_entity( + workspace="workspace-1", + entity_type="task_revision", + name=f"rev.{ordinal}", + parent=parent.id, + data={"content_hash": digest, "revision": ordinal}, + ) + return parents + + +async def test_children_can_be_filtered_by_parent(entity_repo: SQLAlchemyEntityRepository, setup_workspaces): + """``parent`` is a real column, so it filters like any other — despite ``list_entities`` + exposing no dedicated argument for it.""" + parents = await _seed(entity_repo) + rows, total = await entity_repo.list_entities( + workspace="workspace-1", + entity_type="task_revision", + filter_op=ComparisonOperation(field="parent", operator=FilterOperator.EQ, value=parents["task-one"].id), + ) + assert total == 2 + assert {row.name for row in rows} == {"rev.1", "rev.2"} + + +async def test_children_can_be_filtered_by_custom_data_field(entity_repo: SQLAlchemyEntityRepository, setup_workspaces): + """``data.*`` paths are JSON-extracted by the filter translator, so a custom field on a stored + entity is queryable without promoting it to a column.""" + await _seed(entity_repo) + rows, total = await entity_repo.list_entities( + workspace="workspace-1", + entity_type="task_revision", + filter_op=ComparisonOperation(field="data.content_hash", operator=FilterOperator.EQ, value=_DIGEST_A), + ) + assert total == 2, "the same content digest exists under both parents" + assert {row.data["content_hash"] for row in rows} == {_DIGEST_A} + + +async def test_parent_and_custom_field_compose(entity_repo: SQLAlchemyEntityRepository, setup_workspaces): + """The query a digest-pinned ref actually needs: this parent's child with this digest. + + Parent scoping is what disambiguates — identical content under two different tasks yields the + same digest, so filtering on the digest alone returns both. + """ + parents = await _seed(entity_repo) + rows, total = await entity_repo.list_entities( + workspace="workspace-1", + entity_type="task_revision", + filter_op=LogicalOperation( + operator=FilterOperator.AND, + operations=[ + ComparisonOperation(field="parent", operator=FilterOperator.EQ, value=parents["task-two"].id), + ComparisonOperation(field="data.content_hash", operator=FilterOperator.EQ, value=_DIGEST_A), + ], + ), + ) + assert total == 1 + assert rows[0].parent == parents["task-two"].id + assert rows[0].data["content_hash"] == _DIGEST_A + + +async def test_unknown_digest_returns_nothing(entity_repo: SQLAlchemyEntityRepository, setup_workspaces): + await _seed(entity_repo) + _, total = await entity_repo.list_entities( + workspace="workspace-1", + entity_type="task_revision", + filter_op=ComparisonOperation(field="data.content_hash", operator=FilterOperator.EQ, value="c" * 64), + ) + assert total == 0 + + +async def test_deleting_a_parent_removes_its_children(entity_repo: SQLAlchemyEntityRepository, setup_workspaces): + """The parent FK is ``ondelete="CASCADE"``, so deleting a task takes its revisions with it — + no orphaned children left addressable by a stale parent id.""" + parents = await _seed(entity_repo) + await entity_repo.delete_entity(entity_id=parents["task-one"].id) + + _, orphaned = await entity_repo.list_entities( + workspace="workspace-1", + entity_type="task_revision", + filter_op=ComparisonOperation(field="parent", operator=FilterOperator.EQ, value=parents["task-one"].id), + ) + assert orphaned == 0 + + _, survivors = await entity_repo.list_entities( + workspace="workspace-1", + entity_type="task_revision", + filter_op=ComparisonOperation(field="parent", operator=FilterOperator.EQ, value=parents["task-two"].id), + ) + assert survivors == 2, "the other task's revisions must be untouched"