Repository navigation
fix(pi): retain failed model-attempt ownership - #4045
Merged
Merged
Conversation
lafrancef
added
bug
This was referenced Oct 5, 2026
yaoyu-33
added
the
complexity:low
yaoyu-33
approved these changes
Oct 5, 2026
yaoyu-33
added
ready-to-merge
Contributor
|
/ok to test e5c1ae2 |
Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
lafrancef
force-pushed
the
flafrance/pi-harness-conformance
branch
from
October 6, 2026 20:44
e5c1ae2 to
5dc3d2e
Compare
Contributor
Author
|
/ok to test 5dc3d2e |
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/37530804225 |
7 tasks done
jeffwillette
pushed a commit
to jeffwillette/Gym
that referenced
this pull request
Oct 7, 2026
…vers (NVIDIA-NeMo#4180) ## What does this PR do? When `opencode_agent` runs through a Gym model server (`model_server` set), the OpenCode config it generates sets `interleaved.field` to `reasoning`. OpenCode then sends earlier assistant turns back with a `reasoning` key. Gym's Chat Completions request schema accepts `reasoning_content` on assistant messages, not `reasoning`, so the first request after any tool call fails with a 422 (`extra_forbidden` at `body.messages[i]...reasoning`). The rollout ends at that point, but `gym eval run` still exits 0 and the failures file stays empty. Single-turn answers are unaffected, which makes this easy to miss. This PR sets the field to `reasoning_content`, the key Gym model servers send and accept. `opencode_sandboxed_agent` doesn't set `interleaved` and is unaffected. No GitHub issue: the bug was reported through internal QA, and this PR includes the reproduction below. Overlap: NVIDIA-NeMo#4046 makes the same change, using `setdefault` so a user's own `interleaved` setting wins, as part of a larger conformance change stacked on NVIDIA-NeMo#4045. This PR fixes the bug on its own so it can land first. NVIDIA-NeMo#4046 can drop its copy of the line or rebase over it. ## Validation - Added `test_model_server_replayed_reasoning_is_accepted_by_gym_chat_completions`. It builds the assistant message OpenCode sends after a tool call, using the field from the generated config, and validates it against `NeMoGymChatCompletionCreateParamsNonStreaming`. Without the fix it fails with the same `extra_forbidden` error. - `pytest responses_api_agents/opencode_agent/tests` in a fresh environment (`uv venv && uv sync --extra dev`). - `pre-commit run --all-files`. ## Rollouts All runs use OpenCode 1.17.11, `environments/opencode_reasoning_gym`, and `--model-type inference_provider`, with `model_server` pointing at `policy_model` and model-call capture enabled. Status codes come from the capture store. | Scenario | Model | Code | Call after the tool call | Result | |---|---|---|---|---| | Subtask command from the bug reproduction, 1 task | `eccn-gpt-oss-20b` | before | 422 | Stopped after 1 turn | | Subtask command from the bug reproduction, 1 task | `eccn-gpt-oss-20b` | after | 200 | 2 turns, reward 1.0 | | Model's own `bash` call, 3 tasks | `eccn-nemotron-3-nano-30b-a3b` | before | 422 on all 3 | Rewards 0, 0, 0 (empty answers); exit 0; failures file empty | | Model's own `bash` call, 3 tasks | `eccn-nemotron-3-nano-30b-a3b` | after | 200 | Rewards 1.0, 1.0, 1.0 | With the fix, OpenCode sends the model's reasoning back under `reasoning_content` (5,203 and 7,631 characters on the two tasks that used a tool). In the subtask scenario the replayed reasoning is empty, because OpenCode creates that tool call itself without calling the model. The Nemotron runs add a system prompt that requires one bash call before answering, so every task reaches the post-tool-call turn. ## Compatibility and benchmark impact OpenCode rollouts through a Gym model server no longer stop after their first tool call. Scores from multi-turn OpenCode runs on this route will change, since earlier runs were cut short. Nothing else in the configuration or API changes. ## Checklist - [x] I have read the [contributing guidelines](https://github.com/NVIDIA-NeMo/Gym/blob/main/CONTRIBUTING.md). - [x] The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs. - [x] Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable). - [x] Documentation added or updated, or N/A with justification. N/A: the README doesn't describe the `interleaved` setting. - [x] Pre-commit checks pass locally (`pre-commit run --all-files`) (so CI lint/format checks pass). - [x] New source files include the required Apache-2.0 SPDX header. N/A: no new files. - [x] All commits have DCO sign-off (`git commit -s`) (so the DCO check passes). Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
giuliolovisotto
pushed a commit
to yaoyu-33/Gym
that referenced
this pull request
Oct 9, 2026
…Mo#4048) Hermes retries lose model-attempt ownership when no response ID exists, and local terminal provider failures previously aborted before collection and verification. Propagate invocation headers through SDK retries and retain local provider failures so the verifier and collector receive their evidence. Provider-failed rollouts are always marked `mask_sample: true` and `failure_kind: agent_request_failed`, even if the verifier awards a positive reward. They remain excluded from scores. This intentionally changes the local failure behavior established in NVIDIA-NeMo#3554; sandbox sessions retain their retryable failure path. Depends on NVIDIA-NeMo#4045 for the shared collector correction; this PR is based on that branch to keep its diff harness-specific. No separate issue is needed: the failures reproduce with the conformance suite from NVIDIA-NeMo#3766. All seven conformance scenarios pass with Hermes commit 26bb847a88493342ca1b194e0455b479073ae21d (previously 4/7). Reproduce with `python scripts/run_harness_conformance.py --harness hermes --output /tmp/hermes-conformance` using a fresh output directory. Error/retry ownership satisfies TE-8, with TE-9 as the alternative response-ID contract. `pre-commit run --all-files` is clean. A real gpt-4.1-mini rollout computed 17 × 23, wrote and read 391 through a shell tool, and returned CONFORMANCE_DONE; the independent verifier checked both the file and final answer and awarded 1.0 (two provider calls). The live smoke disabled `chat_template_kwargs_enabled` for OpenAI compatibility. Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
yaoyu-33
added a commit
to yaoyu-33/Gym
that referenced
this pull request
Oct 9, 2026
…DIA-NeMo#4251) ## What does this PR do? Align conformance with NVIDIA-NeMo#4045: retained HTTP errors belong to an invocation and may have zero or one step reference. Successful policy responses still require a step; compaction calls remain separate. This corrects the P0 check introduced in NVIDIA-NeMo#4192 without inventing turns for failed HTTP exchanges. Also pass the configured assistant-message header to the scripted Model Server's capture middleware, matching normal Model Server startup. Tests cover header retention on HTTP success/failure and reject duplicate links or missing ownership. The OpenCode plugin-loading commit was removed and replaced by the separate shared-runtime PR NVIDIA-NeMo#4288. This PR only changes conformance checks, probe wiring, their tests and documentation. It is rebased on main `77509af61`. ## Validation After removing the plugin-loading commit: ```bash PYTHONPATH=$PWD python -m pytest \ tests/unit_tests/harness_capabilities \ scripts/harness_conformance/tests \ tests/unit_tests/test_harness_conformance_checks.py -q ``` **342 passed, 2 skipped.** Full core and all-server suites were not run locally. The all-files pre-commit check passed on the rebased branch before removing the isolated plugin-loading commit. The fresh four-harness probe run on the rebased branch, before that split, gave OpenCode 5/7, Hermes 7/7, Pi 6/7, Codex 0/7. These are not results for this final split: OpenCode additionally needs the separate plugin setup. Hermes benefits from NVIDIA-NeMo#4048 now on main. Pi's two provider-error classification unit failures remain separate (NVIDIA-NeMo#4285); its HTTP-400 health expectation also remains unresolved. Codex retains independent evidence gaps. No fresh generative-model benchmark is needed for this diagnostic/probe correction. No API migration or reward/scoring change. No new source files; existing licenses retained. All commits have DCO sign-off. --------- Signed-off-by: yaoyu-33 <yaoyu.094@gmail.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.
Pi loses ownership of model attempts that fail before returning a response ID, causing the retry and terminal-error scenarios from #3766 to fail. Carry the invocation ID in provider headers through retries and keep HTTP error payloads as captured attempts without creating empty model turns.
The collector correction is shared by the other harness conformance fixes. No separate issue is needed: these failures were reproduced with the existing conformance runner.
All seven conformance scenarios pass with Pi 0.80.2 (previously 4/7). Reproduce with
python scripts/run_harness_conformance.py --harness pi --output /tmp/pi-conformanceusing a fresh output directory. Error/retry ownership satisfies TE-8; TE-9 remains the alternative response-ID contract.pre-commit run --all-filesis clean. A real gpt-4.1-mini rollout computed 17 × 23, wrote and read 391 through a shell tool, and returned CONFORMANCE_DONE; the independent verifier awarded 1.0 after checking the file and final answer (two provider calls).