Skip to content

feat(data): task_data validation gating: auto mode, misplaced-field check, repo drift test - #2827

Merged
kajalj22 merged 4 commits into
mainfrom
data-prep/task-data-b3
Aug 28, 2026
Merged

feat(data): task_data validation gating: auto mode, misplaced-field check, repo drift test#2827
kajalj22 merged 4 commits into
mainfrom
data-prep/task-data-b3

Conversation

@adil-a

@adil-a adil-a commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What this does

This PR turns the schema validation from #2800 into a gate.

Auto validation mode. task_data_validation gains an auto setting. Auto is now the default. Auto resolves to error in example_validation mode and to warn in train_preparation mode. Committed example data is known clean, so a bad row should fail the PR gate. User training data must not crash long collate jobs, so it warns instead.

Before: the default was warn everywhere. A bad committed example row only printed a warning.
After: gym dataset collate +mode=example_validation fails on a bad row with a per-row error report. Training collate still warns and completes.

Misplaced-field check. Some schema fields carry a legacy_location: verifier_metadata marker. The marker means the server reads that field only from inside verifier_metadata. If a row puts the field at the top level instead, the server never sees it. The validator now counts these rows and reports them as misplaced_keys. The instruction_following schema drops its markers because that server accepts both placements.

Schema presence test. Every resources server with an app.py must ship a task_data.py. New servers can no longer skip the schema.

Repo drift test. A new unit test walks every committed dataset in the repo and validates each row against its server's schema. If a server's wire model changes and the schema does not, this test fails. This is the drift gate requested in the #2795 review.

The presence test already paid for itself. terminal_bench_2_1 landed on main after the schema sweep and had no schema. The test caught it, and this PR adds its schema.

Review fixes. Three additions from the first review round. Undeclared row keys now fail the gate instead of only being reported; no committed row carries one, and the stricter gate immediately caught two missing fields in a new schema. The drift test now also validates datasets owned by self-contained agents (agents with no resources server), using the same schema fallback collate uses; schemas for the nine such agents with committed data are included. normalize_task_fields now unpacks a migrated task_data container the same way it unpacks verifier_metadata, so nested fields cannot dodge validation.

Verification

  • All 302 tests in tests/unit_tests/test_task_data.py pass.
  • Live smoke: a bad row (expected_city as an int) fails gym dataset collate in example_validation mode with a per-row pydantic report. The same row only warns in train_preparation mode and the run completes.
  • Golden routing gate: base capture vs branch capture, strict compare, 0 differences across 233 closures.
  • ruff and pre-commit are clean.

@copy-pr-bot

copy-pr-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@adil-a

adil-a commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test ac0ac2b

@adil-a

adil-a commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 76fc4f0

Comment thread tests/unit_tests/test_task_data.py
Comment thread tests/unit_tests/test_task_data.py
Comment thread nemo_gym/task_data.py Outdated
@adil-a

adil-a commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 6ea653a

ananthsub
ananthsub previously approved these changes Aug 27, 2026
@adil-a

adil-a commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 8e92d77

@adil-a
adil-a force-pushed the data-prep/task-data-schemas-b2 branch from 47ad660 to b12f47e Compare August 28, 2026 09:18
@adil-a
adil-a force-pushed the data-prep/task-data-b3 branch from 8e92d77 to f904602 Compare August 28, 2026 09:19
@adil-a

adil-a commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test f904602

ananthsub
ananthsub previously approved these changes Aug 28, 2026
@adil-a
adil-a force-pushed the data-prep/task-data-schemas-b2 branch from 3a75506 to 1188068 Compare August 28, 2026 17:03
@adil-a
adil-a force-pushed the data-prep/task-data-b3 branch from 15ccaed to 3a4b217 Compare August 28, 2026 17:04
@adil-a

adil-a commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 9176cc6

Base automatically changed from data-prep/task-data-schemas-b2 to main August 28, 2026 17:21
@kajalj22
kajalj22 dismissed ananthsub’s stale review August 28, 2026 17:21

The base branch was changed.

adil-a and others added 4 commits August 28, 2026 10:25
…heck, repo drift test

task_data_validation gains an auto mode and becomes the default. Auto resolves to error in example_validation mode and warn in train_preparation mode. The validator now reports misplaced legacy fields: a field marked legacy_location=verifier_metadata that appears at the row top level is flagged, because the wire reads it exclusively from verifier_metadata. instruction_following drops its legacy_location markers since its before-validator accepts both placements. New tests: every resources server with an app.py must ship task_data.py, and a repo-wide drift test validates all committed dataset rows against their server schemas.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Adil Asif <adasif@nvidia.com>
terminal_bench_2_1 landed on main after the schema sweep in the previous PR. The new schema-presence test caught it. Rows carry task_name, docker_image, and task_folder top-level, all wire-required strings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Adil Asif <adasif@nvidia.com>
…e, task_data splice

Three fixes from the PR review. unknown_keys now dirties the validation report, so the drift gate and error-mode collate fail on undeclared row keys; no committed row carries one today. The drift test now maps datasets on agent blocks without a resources_server reference to the agent's own schema, the same fallback collate uses, and schemas are added for the nine self-contained agents with committed data. normalize_task_fields now splices a migrated task_data container the same way it splices verifier_metadata, with conflict detection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Adil Asif <adasif@nvidia.com>
agentif landed on main after the schema sweep. The schema-presence and drift tests caught it. Rows nest all task fields in an untyped verifier_metadata bucket; the schema follows the longmemeval pattern: flat, all Optional, legacy_location markers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Adil Asif <adasif@nvidia.com>
@adil-a

adil-a commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 4df2449

@adil-a
adil-a force-pushed the data-prep/task-data-b3 branch from 9176cc6 to 4df2449 Compare August 28, 2026 17:26
@kajalj22
kajalj22 self-requested a review August 28, 2026 17:37
@kajalj22
kajalj22 merged commit b369322 into main Aug 28, 2026
38 checks passed
@kajalj22
kajalj22 deleted the data-prep/task-data-b3 branch August 28, 2026 17:37
ananthsub pushed a commit that referenced this pull request Aug 28, 2026
…heck, repo drift test (#2827)

## What this does

This PR turns the schema validation from #2800 into a gate.

**Auto validation mode.** `task_data_validation` gains an `auto`
setting. Auto is now the default. Auto resolves to `error` in
`example_validation` mode and to `warn` in `train_preparation` mode.
Committed example data is known clean, so a bad row should fail the PR
gate. User training data must not crash long collate jobs, so it warns
instead.

Before: the default was `warn` everywhere. A bad committed example row
only printed a warning.
After: `gym dataset collate +mode=example_validation` fails on a bad row
with a per-row error report. Training collate still warns and completes.

**Misplaced-field check.** Some schema fields carry a `legacy_location:
verifier_metadata` marker. The marker means the server reads that field
only from inside `verifier_metadata`. If a row puts the field at the top
level instead, the server never sees it. The validator now counts these
rows and reports them as `misplaced_keys`. The instruction_following
schema drops its markers because that server accepts both placements.

**Schema presence test.** Every resources server with an `app.py` must
ship a `task_data.py`. New servers can no longer skip the schema.

**Repo drift test.** A new unit test walks every committed dataset in
the repo and validates each row against its server's schema. If a
server's wire model changes and the schema does not, this test fails.
This is the drift gate requested in the #2795 review.

The presence test already paid for itself. terminal_bench_2_1 landed on
main after the schema sweep and had no schema. The test caught it, and
this PR adds its schema.

**Review fixes.** Three additions from the first review round.
Undeclared row keys now fail the gate instead of only being reported; no
committed row carries one, and the stricter gate immediately caught two
missing fields in a new schema. The drift test now also validates
datasets owned by self-contained agents (agents with no resources
server), using the same schema fallback collate uses; schemas for the
nine such agents with committed data are included.
`normalize_task_fields` now unpacks a migrated `task_data` container the
same way it unpacks `verifier_metadata`, so nested fields cannot dodge
validation.

## Verification

- All 302 tests in `tests/unit_tests/test_task_data.py` pass.
- Live smoke: a bad row (`expected_city` as an int) fails `gym dataset
collate` in `example_validation` mode with a per-row pydantic report.
The same row only warns in `train_preparation` mode and the run
completes.
- Golden routing gate: base capture vs branch capture, strict compare, 0
differences across 233 closures.
- ruff and pre-commit are clean.

---------

Signed-off-by: Adil Asif <adasif@nvidia.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit b369322)
mcuevas-nvidia added a commit that referenced this pull request Aug 31, 2026
#2827 (2026-08-28) made every resources server ship a task_data.py describing
its dataset rows; it landed after this PR branched, so the merge of main left
two repo tests failing.

Rows nest everything in an untyped verifier_metadata bucket, so the schema is
flat with legacy_location annotations. Every field is Optional because the
server reads the bucket only via .get(...) or <default> and never 422s on its
contents. Shapes derived from all 655 committed and generated rows: 744
gym_servers_config entries and 3,532 verifiers, with user_info the one variable
field (absent from 44 entries, str or dict where present).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Marc Cuevas <mcuevas@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants