Repository navigation
cp: fix(opencode): replay reasoning as reasoning_content on Gym model servers (4180) into r0.7.0 - #4191
Merged
Conversation
…vers (#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: #4046 makes the same change, using `setdefault` so a user's own `interleaved` setting wins, as part of a larger conformance change stacked on #4045. This PR fixes the bug on its own so it can land first. #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> Signed-off-by: NeMo Bot <nemo-bot@nvidia.com>
svcnvidia-nemo-ci
added
area:agent
Contributor
Author
|
/ok to test 7365776 |
ananthsub
approved these changes
Oct 7, 2026
yaoyu-33
pushed a commit
to wefstratis-nv/Gym
that referenced
this pull request
Oct 8, 2026
…DIA-NeMo#3513) ## Overview This PR lets a training framework store large per-call payloads (today: the processed image/video tensors a VLM inference worker ran on) in the **same durable write** as the call's token record, so token capture for multimodal rollouts is acknowledged atomically. Gym stays engine- and tensor-blind: it forwards the payload, never inspects or digests it, and leaves storage, validation, and read-back to the framework (paired change: [NVIDIA-NeMo/RL NVIDIA-NeMo#4191](NVIDIA-NeMo/RL#4191)). Without this, a framework capturing VLM media had to write the token row through Gym and then write the pixels in a second store call, and reconcile the two at training time. With this, `staged` coordinates mean "tokens and media are both durable", and a failed combined write surfaces as `capture_failed` at call time. Net change against `main`: 7 files, +255/-14. No new dependencies. No Megatron adapter or serving changes. ## What is added **1. `attachments` on the staging capture core** (`staging/capture.py`) - `RolloutTokenCapture.complete_call(..., attachments: Mapping[str, Any] | None = None)` and `complete_call_from_response(call, response_payload, *, attachments=None)` accept a keyword-only mapping and forward it to the sink. - Attachments never enter `StagedCallRecord`, `extras`, or any digest. `digest`, `extras_digest`, and `CommitCoords` are byte-identical with or without them. - The keyword is passed to the sink **only when the caller supplied a mapping** (an explicit empty mapping is forwarded). Absent attachments use the existing `stage(record)` call shape, so every sink that exists today keeps working unchanged for text-only calls. - If a sink cannot accept the keyword (`TypeError`) or rejects the bundle, the existing capture boundary reports `capture_failed`. Gym never retries the write with the attachments dropped. **2. `StagingSink` protocol** (`staging/protocols.py`) - `stage(self, record, *, attachments=None) -> StageResult`, with the contract documented on the protocol: a sink that accepts the keyword must store every supplied attachment before returning success or reject the call; silently discarding attachments violates the protocol. Validation of tensor contents, layout, and read-back belong to the framework. **3. vLLM adapter** (`adapters/vllm.py`) - `VLLMCaptureAdapter.extract_extras` now also passes through a `media_spans` list from the response payload (validated as a list of objects) alongside the existing `routed_experts`. These are small, JSON-native placeholder positions and token hashes the framework's worker needs to remap image/video placeholders when it reuses a captured prefix on the next turn. They ride the digest-covered extras; pixels do not. **4. Docs and tests** - `model-call-capture.mdx`: new "Framework-owned media staging" section describing the attachment contract. - `test_token_capture_staging_worker.py`: legacy `stage(record)` sink succeeds without the keyword; attachments reach the sink by identity; an explicit empty mapping is forwarded; absent attachments use the legacy call shape; `TypeError` or explicit rejection yields `capture_failed` with no fallback write; digests and coordinates are equal with and without attachments; `complete_call_from_response` forwards attachments; adapter extras carry `media_spans` and `routed_experts` and never a `media` key; malformed `media_spans` are rejected; the staging package exports no media-summary helpers. - Sink test doubles in the vLLM model tests (`test_token_capture_integration.py`, `test_external_capture_streaming.py`) migrated to the attachment-aware signature and assert no attachments arrive on text-only paths. ## Why this shape An earlier iteration of this branch carried a digest-covered `media` geometry summary in `extras` (a `staging/media.py` module) so the framework could cross-check a separately written pixel store. Writing both in one sink call removes the divergence that summary guarded against, so that module and its exports are gone and the branch now adds only the attachment plumbing above. The framework validates the tensors it writes and reads. ## Compatibility - Text-only callers and existing sinks are unaffected; the keyword is optional and only sent when attachments are present. - Frameworks capturing media must deploy the paired RL change with this Gym change. Rows and checkpoints written by earlier media-capture prototypes are not compatible. ## Validation - Gym token-capture suites in the NeMo-RL nightly container (Slurm job `3884145`, before the `main` merge): **184 tests passing** (`test_token_capture_staging_worker`, `_core`, `_conformance`, `_rebuild`, `_routes`, vLLM `test_token_capture_integration`). After merging `main` (conflicts only in `staging/capture.py` and the worker test): `test_token_capture_staging_worker` and `_core` pass locally (102 tests); a container re-run against the merged branch is in flight. - Paired RL suites exercising this path end to end (87 capture tests on live TransferQueue, including the real vLLM video processor; 129 hosting/finalizer/setup tests) pass; see RL NVIDIA-NeMo#4191. - Rollout evidence: 8-node GRPO A/B on Nemotron-3-Nano-Omni-30B-A3B (CLEVR through NeMo-Gym `string_match`, vLLM generation, Megatron learner, 100 steps) with capture on via this branch (job 3884304) matches the capture-off baseline within run-to-run noise: reward 0.552 vs 0.546, step-wise reward correlation 0.943 (repeatability floor 0.952), zero staging failures, zero invalid rows. Details and W&B links in RL NVIDIA-NeMo#4191. - No separate issue: this is the Gym half of the paired RL feature linked above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.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.
beep boop [🤖]: Hi @ananthsub 👋,