Repository navigation
refactor: report conformance checks independently - #4041
Merged
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
lafrancef
reviewed
Oct 5, 2026
giuliolovisotto
changed the title
giuliolovisotto
force-pushed
the
codex/harness-check-registry
branch
from
October 5, 2026 16:58
a7a73e9 to
e68a0ee
Compare
giuliolovisotto
added a commit
that referenced
this pull request
Oct 6, 2026
Stacked on #4041. Validate each piece of evidence once at its designated saved location, with JSON Schema for fields and explicit semantic checks for relationships. Missing collections block dependent checks without invalidating unrelated evidence. - Keep call, step, tool, grading and sandbox requirements; resolve invocation/call references against canonical targets. All supplied reference identifiers must agree, and ambiguous targets fail. - Replace blanket Python-model validation and cross-copy reconciliation. Do not repeat Gym-owned uniqueness, clock-order or provider-usage normalization checks. - Retain independent comparisons with scripted exchanges, response metadata, token counts and tool outcomes. A behavioral failure does not erase artifact passes. - Emit one table per harness, with individual check rows grouped as Schema, Semantic and Behavioral, scenario columns, and compact status symbols. Process status stays separate. Missing normal rollouts leave artifact checks unassessed. The profile is `gym-p0/v3`. Check IDs, report format and authoritative locations change; regenerate existing reports. TE labels only group results. `ng_trajectory` supplies calls, steps, invocations and tool executions; sandbox/compaction observations retain their explicitly designated source. Capture sidecars are provenance only. Benchmark execution and scoring code are unchanged. Validation: - Focused checker/runner suite: **253 passed, 2 skipped** (`jj` unavailable). - `pre-commit run --all-files`: passed. - Fern check: zero errors, one warning. - Fresh scripted runs on main `b0e3fa6cf` plus this checker stack for simple_agent, SciCode, OpenCode, Terminus2, mini-SWE, Hermes, NOOA, Codex, Pi and Stirrup. OpenCode is also tested separately with observability stack tip `ccf9f997b` (#3872). Failure sidecars leave artifact checks unassessed. - Local additional-driver/checker suite: **272 passed, 2 skipped**. Combined current OpenCode stack, producer/capture and checker tests: **625 passed, 2 skipped**. Controlled-endpoint tests execute shell commands and inspect actual model/tool/verifier exchanges. These scripted runs do not qualify production benchmarks. Stirrup also ran all seven scenarios in Lima/Apptainer with a small shell-test image: six normal rollouts and one terminal-error failure record. The six normal rollouts retain model-call evidence but lack invocation, step and tool records. This uses a local native-tool driver; it does not qualify the full GDPval environment. Full server CI was not run locally. Follow-up to #3766 and #4041; no separate issue is needed for this focused integration. --------- Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
giuliolovisotto
marked this pull request as ready for review
October 6, 2026 14:59
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-4041-da612c4207c1.docs.buildwithfern.com/nemo/gym |
yaoyu-33
added
area:core
Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
Keep JSONL and hashing helpers with the CLI. Remove the copy-only hydration wrapper and keep harness unit tests focused on native evidence and health. Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
giuliolovisotto
force-pushed
the
codex/harness-check-registry
branch
from
October 7, 2026 12:35
60f8351 to
b4f30dc
Compare
Remove conformance runner tests from default discovery. Dedicated CI for the runner is deferred. Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
Keep model and tool witness comparisons in default core discovery. Share independent synthetic evidence with runner integration tests and retain protocol normalization coverage in the runner suite. Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
lafrancef
approved these changes
Oct 7, 2026
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/37640288114 |
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Oct 7, 2026
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/37696271660 |
wdykas
pushed a commit
to wdykas/Gym
that referenced
this pull request
Oct 8, 2026
Adds health expectations to the conformance checker introduced by NVIDIA-NeMo#4041. Rebased onto main after NVIDIA-NeMo#4041 merged; this PR contains only the health-check addition. A harness can satisfy artifact checks while Gym health still lacks required evidence or finds incorrect aggregate usage. Add P0 `HealthCheck` objects generated from Gym's rollout health registry. The conformance runner calls the public health engine once on saved rollout and failure artifacts, then compares each individual health result with the scenario's expectation. Scenarios default to `healthy` and manually declare exceptions. Deliberately omitted usage and injected HTTP errors expect missing-token findings and unobserved token reconciliation; terminal HTTP errors also expect an ended-on-failed-call finding. Missing health results fail. New registered rollout checks default to healthy; unknown scenario override IDs are configuration errors. Explicit step scope remains respected, and task-level checks requiring repeats remain outside this suite. Results appear in a separate Health group in the existing scenario tables. A pass means the expected outcome occurred, including expected unhealthy or unobserved outcomes. Gym's health-check logic and production harness behavior are unchanged. Tracking: [FEA-402](https://linear.app/nvidia/issue/FEA-402). Validation after rebasing onto main (`cc2046c84`), 8 October 2026: - 395 checker, runner and rollout-health tests passed; 2 skipped (Python 3.13.14). - `pre-commit run --all-files` passed. - Fern check: 0 errors, 1 warning. - Preserved main’s moved reader helpers and shared evidence/witness fixture. Updated the step-scope regression test: invocation-owned token reconciliation can pass without turns, and a real aggregate mismatch still fails when step checks are excluded. Original pre-rebase runtime validation: - 293 checker/runner tests passed; 2 skipped. - `pre-commit run --all-files` passed. - Fern check: 0 errors, 1 warning. - Seven fresh scripted-model Pi scenarios completed. Four match every health expectation; retry and terminal-error scenarios expose existing ownership gaps. - Seven saved sandboxed OpenCode scenarios from NVIDIA-NeMo#4106 reinspected using their compatible native Gym health version: all health expectations match. These artifacts use a newer trajectory schema than this branch's base, so the matching health reader is required. No new harness/model execution was performed for this conflict resolution; full server CI was not run locally. The changes resolve imports and test-fixture compatibility without changing the health expectation implementation. --------- Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
This branch was previously deployed
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
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.
Conformance reporting previously coupled check dispatch and pass/fail gates to TE groups, and behavioral failures could obscure valid artifact evidence. Define schema, semantic and behavioral checks as typed objects with explicit IDs, tiers, evidence labels, locations, failure reasons and evaluation logic. All check definitions live under
nemo_gym/harness_capabilities/; scripts supply scenarios, observations and expectations.Each check reports pass, fail, or not applicable, with reasons and blocking prerequisites. Missing required input, blocked checks and absent results display a cross. The P0 gate requires every applicable individual P0 check to pass; all current checks are P0, including ownership and call-to-step checks. TE labels only group results. Artifact, behavioral and execution results remain separate.
The permanent first-request HTTP 400 scenario explicitly excludes step evidence; retry-and-recovery scenarios still require it. Previous-response history is resolved within its model server, avoiding false failures when servers reuse response IDs. Removes unused wrappers, internal record copies and redundant result conversions; consolidates overlapping tests.
Includes the focused evidence checks previously stacked in #4042. Removes string-based dispatch, whole-object validation and cross-copy fallback/consistency checks. Retains pre-PR profile/schema version identifiers. Schema helpers and model-call reference grammar remain in checker.py; schemas.py is removed. Follow-up to #3766; no separate issue is needed.
Harness unit tests assert native observability and health behavior without importing the conformance checker. Removes the copy-only
hydrate_record()wrapper andreader.py; strict JSONL reading and hashing live in the conformance CLI, shared by the scripts.Validation:
b4f30dc96(onmainate76a5c629): 276 checker/runner tests passed, 2 skipped; mini-SWE harness suite: 124 passed, 1 skipped. Tests use the current checkout viaPYTHONPATHwith existing dependency environments.pre-commit run --all-filespassed.Default pytest discovery is limited to
tests. Conformance runner tests remain available through an explicitpytest scripts/harness_conformance/testsinvocation; dedicated CI for that module is deferred. The core coverage threshold remains 95%. This avoids adding incidentally imported harness implementations to the core coverage measurement without their separate server suites.At
da612c420, core behavioral-check tests live intests/unit_tests/harness_capabilities/test_behavior.py. Fifteen cases exercise model exchanges, response IDs, token counts and tool witnesses without importing harness implementations. A shared synthetic helper supplies independent saved evidence and witness records; scenario integration and protocol-normalization tests remain with the runner.Validation for this test relocation:
behavior.pycoverage 96.74% (89 of 92 statements). Verified noresponses_api_agentsmodules were imported.pre-commit run --all-filespassed.Production harness and environment behavior is unchanged. This validation reuses saved controlled-run artifacts; no new live-model benchmark evaluation was performed. Full server CI was not run locally.