feat(evaluator): give a taskset its own files - #1234
Open
SandyChapman wants to merge 1 commit into
Open
Conversation
8 tasks
SandyChapman
force-pushed
the
taskset-file-refs/schapman
branch
from
August 11, 2026 16:28
e746c6e to
9e342b0
Compare
A grouping can need files that every member uses and none owns — a shared metric script, a fixture, a schema. A Harbor dataset ships exactly this beside its tasks. There was nowhere to put them, so the only option was to encode a JSON blob in `metadata`, which is unqueryable, untyped, and invisible to anything that does not know the convention. `files_ref` is one Files reference for the whole grouping, the same shape as `bundle_ref` and `archive_ref`. A taskset's files are a directory, not a set of independently addressed blobs: the fileset already knows what it contains, so a list of per-file references would carry no information it does not hold and would let the two disagree about what the taskset ships. It is a first-class field rather than an annotation because it is content — the revision digest covers it, so repointing or clearing it publishes a revision and a pinned revision keeps naming the files it was published with. Pinning the bytes is arranged by referencing a location that is not rewritten: a fileset written once, or a content-addressed prefix inside a shared one. A reference into a location that is later rewritten resolves to whatever it holds when read, which is the contract the Files service gives every other consumer. The alternative — a per-file digest stored beside each reference — buys pinning the entity store cannot enforce anyway, and duplicates state the Files service owns. The revision entity carries the reference too, but needs no resolution step: unlike a member ref, which is resolved from a possibly-tag-pinned form to an exact digest at publish time, a Files reference already names an exact location. The field is additive and defaults to unset, so an existing taskset is unchanged and its revision digest does not move. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman
force-pushed
the
taskset-file-refs/schapman
branch
from
August 12, 2026 19:38
9e342b0 to
34ee878
Compare
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughChangesTaskset files reference
Sequence Diagram(s)sequenceDiagram
participant TasksetInput
participant TasksetService
participant TasksetEntity
participant TasksetRevisionEntity
TasksetInput->>TasksetService: Submit files_ref
TasksetService->>TasksetEntity: Persist files_ref
TasksetService->>TasksetRevisionEntity: Publish changed files_ref
TasksetService-->>TasksetInput: Return files_ref
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A taskset can need files that every member uses and none owns — a shared metric script, a fixture, a schema. A Harbor dataset ships exactly this beside its tasks. There was nowhere to put them, so the only option was encoding a JSON blob into
metadata: unqueryable, untyped, and invisible to anything that does not already know the convention.This adds
files_refto the taskset: one Files reference for the whole grouping, the same shape asbundle_refandarchive_ref.Changes
TasksetFilesRefinapi/fields.py— astrconstrained toFILESET_REF_PATTERN.files_refonTasksetEntity,TasksetRevisionEntity,Taskset, andTasksetInput; threaded through create, replace, and both DTO mappings.plugins/nemo-evaluator/openapi/openapi.yaml— two string fields, no new schema.tests/api/service/test_taskset_files.py.Three decisions worth review:
One consequence to flag:
FILESET_REF_PATTERNrequires the#fragment, so a bareworkspace/filesetis not accepted — a taskset reference isworkspace/fileset#prefix, where the fragment names the prefix the files sit under rather than a single file. I kept the shared pattern rather than loosening it for one field; making a bare fileset legal would be a deliberate change to the pattern every ref uses.The field is additive and defaults to unset, so existing tasksets are unchanged and their revision digests do not move.
Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
uv run --frozen pytest plugins/nemo-evaluator/tests --ignore=plugins/nemo-evaluator/tests/integration— 804 passed (796 before this change on the same base; 8 new). No existing test needed changing, which is the additive-field claim holding rather than asserted.uv run --frozen pytest plugins/nemo-evaluator/tests/api/service/test_taskset_files.py— 8 passed: round-trip, revision-on-repoint, revision-on-clear, no-revision-on-identical, a pinned revision keeping its original reference after the head moves, unset default, and reference-shape validation (including that a bareworkspace/filesetis rejected).make refresh-openapi— regeneratesplugins/nemo-evaluator/openapi/openapi.yamlonly, idempotent on a second run, no unrelated plugin spec drifted.tools/lint/lint-python-types.sh(CI's exact type gate) — exit 0.uv run ruff check/ruff format --checkon the changed tree — clean.uv run pre-commit run -a— all substantive hooks pass:ruff,ruff format,Run ty typechecks,Check config reference doc is up to date,Check for uv.lock drift,Check CI and Flox uv versions,Check Make and Flox Python versions,Check Node.js and pnpm versions,Fix copyright headers,Plugins must not import from nmp-common,check for merge conflicts. Three hooks could not run in my local environment, which is why the gate above is left unchecked — neither is reported as passing:Run uv lock with platform uv— requires uv 0.9.14; local uv is 0.9.30. Nopyproject.tomlis touched, andCheck for uv.lock driftpasses.Run UI lint-staged—pnpmunavailable locally (untrustedmise.toml). Noweb/files are touched.Helm Docs— thehelm-docsbinary is not installed locally (the hook moved off a container onmain). It modifies nothing here; no Helm files are touched.Re-ran the full suite, the type gate, ruff, and
make refresh-openapiafter the rebase ontomain; the committed spec needed no regeneration against it.Note for reviewers:
pytest plugins/nemo-evaluator/testswithout--ignorefails to collect, becausetests/test_evaluate_job.pyandtests/integration/test_evaluate_job.pyshare a basename and test directories are not modules. That is pre-existing on the base branch and unrelated to this change.Summary by CodeRabbit
New Features
Bug Fixes
Tests