[gwarmstrong]

What does this PR do?

When opencode_agent routes OpenCode through a Gym Model Server, a text part that starts with a <think>...</think> block is split. The session output, observations and trajectory turns carry the block as reasoning and the rest of the part as the answer. The host-path response leaves the block out, as it already leaves out OpenCode's own reasoning parts. Any other think tag stays the model's literal text, so the literal-tag behavior from #3639 and its test are unchanged. Without a Gym Model Server, nothing changes.

A message whose Gym reply was only its reasoning block, with no tool call after it and no answer text in another text part, keeps an empty answer at that reply. A verifier that reads the last assistant message then sees an empty answer instead of an earlier turn's text. A tool-call turn keeps only its reasoning. Trajectory turns split only for opencode_agent: the deprecated opencode_sandboxed_agent and vibench_agent, which share the turn builder, keep the block as answer text, as their conversations do.

Why. A Gym Model Server's chat completions reply carries the model's reasoning ahead of its text as one <think>...</think> block (vllm_model with a reasoning parser). opencode_agent configures OpenCode to read reasoning from a separate reasoning_content field. That field never arrives, so OpenCode stores the block as the start of a text part, and every OpenCode answer graded through a Gym Model Server began with the model's full reasoning. The consequences:

• the judge read reasoning and answer together;
• a long reasoning block could exceed a verifier's answer-length limit, scoring 0 (final_answer_too_long with HLE's max_answer_chars);
• trajectories recorded the reasoning as answer text.

The deprecated opencode_sandboxed_agent never had this problem: it passes each text part through ResponsesConverter.postprocess_assistant_message_dict, which moves every <think> block into a reasoning item. The session harness kept the block as content, so this is a regression from the deprecated agent. This change restores the split for the Gym-routed case, narrowed to a reply's leading block so that #3639's literal-tag contract holds. The other chat-completions harnesses also keep the block out of their answers (OpenClaw, Hermes), and Pi and Codex use the Responses API, where reasoning is a separate item.

Part of #5552. Found while validating OpenCode on HLE Diamond. In 5 of 5 OpenCode rollouts, the final message started with <think> (up to 218,573 characters); the final messages of Pi, Codex, OpenClaw and Hermes contained no think tags (0 of 20).

Validation

• gym env test +entrypoint=responses_api_agents/opencode_agent: 677 passed, 1 skipped.
• The opencode_agent, opencode_sandboxed_agent, vibench_agent and kilocode_agent suites in the repo venv: 1029 passed, 8 skipped, 1 failed. The failure, opencode_sandboxed_agent's test_required_mcp_extension, runs node --test and fails on this host's Node 19 before reaching code this PR touches.
• Each new test fails with its fix disabled: the split helper, host response, observations and trajectory, and HTTP session path; the empty final answer on the host, export and HTTP session paths, where the judge returns empty_final_answer without calling its model; the compaction boundary from an earlier activation; the deprecated agent's turns; and the local routing gate set and unset. The tests that check text stays literal still pass.
• pre-commit run on the changed files: clean.

Rollouts

Internal HLE Diamond text run with this change (Nemotron 3.5 Super on vLLM, 5 tasks, OpenCode in OpenSandbox): 5/5 rollouts completed and healthy. 0 of 5 final answers contain <think>, and the reasoning comes back as reasoning items. Both non-empty answers were graded on the answer alone. The other 3 rollouts ended with an empty answer: the model closed its reasoning after 78k to 115k tokens in one call and wrote nothing after it. That is model behavior, independent of the split; Pi showed it once in the same run. Each of those 3 was a single policy call with no earlier assistant message, so the empty-answer change in the review commit does not change their scores; A 5-task HLE smoke on the review commit (merged with #5550 for the profile) had 0/5 final answers and 0/5 judge requests with <think>, against 5/5 without this PR; its one reasoning-only final reply ended in an empty answer scored empty_final_answer without a judge call. Before this change, all 5 OpenCode final answers began with the full reasoning, up to 218,573 characters.

Compatibility and benchmark impact

Behavior change for OpenCode runs through a Gym Model Server: final answers no longer include the reasoning block, which now appears as a reasoning item. Benchmark results can change. OpenCode answers that were cut off by an answer-length limit, or that confused the judge, are graded on the answer alone. Results recorded before this change are not comparable.

With training token capture enabled, the split answer no longer matches the captured Chat output, which keeps the gateway's <think> block, so terminal attribution reports no_content_match. A rollout with several chains (subagents or compaction) then falls back to the masked multi-chain policy instead of attributing the scored chain; a single clean chain keeps its existing fallback. Capture is off by default and the deprecated opencode_sandboxed_agent behaves the same. A token_capture follow-up could ignore a gateway-added leading block.

Checklist

• I have read the contributing guidelines.
• The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs.
• Tests added or updated and pass locally, or N/A for docs-only / non-code changes.
• Documentation added or updated, or N/A with justification. N/A: no setting changes; the fix restores the documented reasoning/answer split.
• Pre-commit checks pass locally (pre-commit run --all-files).
• New source files include the required Apache-2.0 SPDX header. N/A: no new source files.
• All commits have DCO sign-off (git commit -s).

[copy-pr-bot]

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.

[yaoyu-33]

SHIP WITH CARE. The fix is right for what a Gym Model Server sends, and it closes a real scoring bug. The four inline notes are not blocking. Two of them are worth handling before merge: the compaction ID (artifacts.py) and the deprecated agent's turns (observability.py).

What we checked

• vllm_model (and inference_provider) puts exactly one closed, non-empty <think>…</think> at offset 0 of content (vllm_model/app.py:1595-1612). So the leading-block split catches every Gym reply. For that shape it gives the same result as the deprecated agent's _parse_think_tags (chat_completions_converter.py:297-300), apart from the lstrip. The two differ only on shapes the server never emits: leading whitespace, or several blocks.
• Reasoning is kept as reasoning items in the session output, the observations and the trajectory turns. Model-call correlation joins on IDs, so the change does not affect it. Token capture is affected in one case, noted inline at artifacts.py:471.
• No unclosed <think> reaches OpenCode from these servers: vLLM's parsers treat a reply with no end tag as all reasoning, and vllm_model then wraps it. A reply split by the model server arrives as an OpenCode reasoning part, so the split leaves it alone.
• A text part that starts with a closed <think> block is now split even through a pass-through server such as openai_model. So "#3639's literal-tag contract holds" is slightly broader than the code. In practice that block is reasoning, so this needs no change.

Validation we ran: the opencode_agent, opencode_sandboxed_agent, vibench_agent and kilocode_agent test suites at this head: 934 passed and 77 skipped. The skipped tests need Linux process supervision or the OpenCode binary. With the split disabled, exactly the 5 new tests fail. Not run: real rollouts and pre-commit.

[yaoyu-33]

artifacts.py:124-137 NOTE: On a continuation, the pre-pass still records an earlier activation's text part as row["id"]. This loop now emits {row_id}-reasoning first, and for a part that is only a <think> block (a tool-call turn) it emits no row["id"] item at all. So when a compaction's tail falls in an earlier activation, its first_kept_item_id differs from a single-pass parse or points at an item that was never emitted, and no compaction_first_kept_item_unavailable gap is raised. test_compaction_boundary_uses_only_emitted_assistant_items passes only because it has no model_ref case. Could the pre-pass use the same split, and could that test get a model_ref + <think> case?

            if part.get("type") in {"text", "reasoning"} and isinstance(text, str) and text.strip():
                item_id = row["id"]
                if part["type"] == "text" and model_ref is not None and split_gym_reasoning(text)[0] is not None:
                    item_id = f"{row['id']}-reasoning"

[gwarmstrong]

Done in ca40230: the pre-pass now records {part id}-reasoning for a split reply, the item the conversation emits first. test_compaction_boundary_uses_only_emitted_assistant_items now has model_ref cases for a reasoning-only reply and for a reply with an answer, parsed from the current activation and from a later one. Both resolve to the reasoning item with no compaction_first_kept_item_unavailable gap. Without model_ref, the same text keeps the raw part ID. With the pre-pass change reverted, the two earlier-activation cases fail.

[yaoyu-33]

observability.py:172-178 NOTE: The deprecated opencode_sandboxed_agent and its subclass vibench_agent also call append_opencode_turns, and they always pass model_ref (app.py:516). So their trajectory turns now split as well, while their own conversation (app.py:301-312) keeps <think>…</think> in the message text, and the two records in one bundle disagree. Could this use an explicit keyword-only gym_reasoning flag that only opencode_agent passes, like parse_opencode_session/parse_opencode_export? Or else split the deprecated conversation too and say so in the PR body. Those two suites pass locally at this head.

[gwarmstrong]

Done in ca40230 with the explicit flag: append_opencode_turns takes keyword-only gym_reasoning: bool = False, and only opencode_agent's observation parser passes gym_reasoning=model_ref is not None. model_ref still only labels the turns and compactions for model-call correlation. opencode_sandboxed_agent and vibench_agent keep the block as answer text in their turns, matching their conversation. A new test in the opencode_sandboxed_agent suite parses a Gym reply with model_ref set and checks that both records keep the raw text; it fails if the split goes back to keying on model_ref.

[yaoyu-33]

app.py:499 NOTE: No test reaches this gate. Forcing it to True or False leaves the suite green, because every _run_opencode test either patches parse_opencode_session out or needs the binary. The session-path gate at L1017 is covered by test_http_splits_the_reasoning_block_a_gym_reply_starts_with. Could you add a _run_opencode test parametrized on model_server set and unset? It could copy a prepared DB holding a Gym reply into XDG_DATA_HOME/opencode/opencode.db and check both the scored output and the observations.

[gwarmstrong]

Added test_local_run_splits_a_reply_only_when_opencode_calls_a_gym_model_server, parametrized on model_server set and unset. A fake subprocess copies a prepared database into XDG_DATA_HOME/opencode/opencode.db, and both artifact parsers run unpatched. With a model server, the scored output is 4, and the observed conversation and trajectory turn carry reasoning r plus answer 4. Without one, all three keep the original text and there is no reasoning item. Forcing the gate to True or to False fails one case each.

[yaoyu-33]

artifacts.py:471 NOTE: With token_id_capture on, terminal attribution (resolve_terminal) matches the scored response against the captured Chat output, which keeps the gateway's <think>…</think> in the message text (completion.py:17-21). After the split the answer no longer matches (no_content_match, here and in parse_opencode_export). Attribution then falls back to the builder's single-chain policy, which matters only for rollouts with several chains (subagents, compaction). Capture is off by default and the deprecated agent behaves the same, so this needn't block. Could you mention it in the compatibility section? A token_capture follow-up could then ignore a gateway-added leading block, or OpenCode could declare its terminal response.

[gwarmstrong]

Agreed, and it's now in the PR's Compatibility section. With training token capture on, the split answer no longer matches the captured Chat output, which keeps the gateway's <think> block (no_content_match, on both the host and export paths). Attribution then falls back to the builder's single-chain policy, so a rollout with several chains (subagents, compaction) is masked rather than attributed. Capture is off by default, and the deprecated agent behaves the same. I didn't change capture itself here; a token_capture follow-up could ignore a leading block the gateway added.

[yaoyu-33]

/ok to test 242129f

[svcnvidia-nemo-ci]

HOLD — dropping a reasoning-only terminal text part can make the verifier grade an earlier answer instead.

I found one BLOCKER, one RISK, and three non-blocking NOTEs. The main split matches the inspected Gym gateway output, but terminal-answer handling and cross-activation compaction identities need correction.

I reviewed all five changed files and traced the affected response paths, continuation handling, shared deprecated callers, and token attribution. I couldn't execute tests or rollouts in this read-only environment; the PR's validation results remain author-reported.

Note: formal approval is not available for this request, so this review is posted as a comment only.

---

Inline comments (escaped v1):

• responses_api_agents/opencode_agent/artifacts.py line 554: BLOCKER — Keep an empty terminal answer from falling back to an earlier answer.

  With Gym reasoning enabled, consider an earlier assistant answer 4, a tool call/result, and a terminal assistant text `` with finish="stop". This branch emits reasoning but no terminal message. The host converter drops that terminal part entirely. The judge's backward search then selects `4`, so an empty final answer can receive credit for earlier text instead of returning `empty_final_answer`. Local padding doesn't help because an earlier assistant message exists. Before this change, the final text remained the last message.

  Could you make final-answer conversion message-aware in parse_opencode_session and parse_opencode_export/_session_output? When this split removes all answer text from the terminal assistant message, retain an empty final assistant message. _session_output currently converts one part at a time, so the decision needs whole-message context. The invariant is that removing reasoning must never promote an earlier turn into the scored final answer. A blanket empty message for every split isn't safe: reasoning-only tool-call turns must remain reasoning followed by tool items, and a terminal message with another non-empty text part must retain that answer without a trailing empty message.

  Proposed checks, not executed: build SQLite and export transcripts containing earlier 4, a tool call/result, and terminal ``. Both response paths must extract an empty final answer; the judge must return reward 0, `failure_reason="empty_final_answer"`, and make no judge call, as required by its empty-answer branch. Retain the HTTP test's reasoning-only tool turn without an empty message. Also check a terminal message with reasoning-only text followed by answer `4`, non-Gym literal text, and existing typed reasoning. I haven't supplied replacement code because the current per-part conversion needs a message-level decision.

• responses_api_agents/opencode_agent/artifacts.py line 130: RISK — Resolve earlier-activation compaction boundaries to the emitted split item.

  With model_ref set, assistant message m0 containing text part p0 = "" emits only p0-reasoning in the new emission branch. A later activation containing a compaction with tail_start_id="m0" instead records p0 in this pre-pass when message_ids excludes m0. Its first_kept_item_id points to an item that was never emitted, without an unavailable-boundary gap. With an answer suffix, p0 exists but still isn't the first emitted item. This affects continuation compaction observations, not scored answers.

  Could you mirror the emission branch here? Immediately after item_id = row["id"], add:

                  if (
                      part.get("type") == "text"
                      and model_ref is not None
                      and split_gym_reasoning(text)[0] is not None
                  ):
                      item_id = f"{row['id']}-reasoning"

  Full and activation-filtered parsing must resolve the same first emitted item for a retained message. This preserves raw IDs for plain text, typed reasoning, and text without model_ref; it also leaves tool callID boundaries, user-message exclusion, and empty/whitespace skipping unchanged.

  Proposed checks, not executed: extend the existing boundary regression with model_ref and text ``, both with and without an answer suffix. Current and earlier-activation tails must return the reasoning item's ID, with no unavailable-boundary gap. Without model_ref, retain the raw text ID. Retain the existing plain-assistant `p2`, user-tail `None`, and cross-activation tool boundary `spawn-child`. I traced the suggested code through these branches; please verify it before adopting.

• responses_api_agents/opencode_agent/observability.py line 173: NOTE — Suggestion: make the shared trajectory split an explicit opt-in.

  model_ref isn't specific to this harness. The deprecated agent's observation parser also passes it here, and ViBench inherits that execution path. For answer, their trajectory now reports reasoning r and answer answer, while their unchanged observation conversation still reports the entire string as answer text. This isn't a scoring failure, but it changes those adapters outside the stated scope and leaves two records of the same turn using different classifications.

  Could you add keyword-only gym_reasoning: bool = False to append_opencode_turns, use that flag for this conditional, and opt in only from the current harness's observation parser? The corresponding replacements are:

  # append_opencode_turns: add after the model_ref parameter
      gym_reasoning: bool = False,
  
  # Replace the conditional expression at this location
  split_gym_reasoning(part["text"]) if gym_reasoning else (None, part["text"])
  
  # _parse_opencode_session: replace its append_opencode_turns call
  append_opencode_turns(
      trajectory, session_ids, message_rows, part_rows,
      model_ref=model_ref, gym_reasoning=model_ref is not None,
  )

  Keep model_ref independently available for model-call correlation. Preserve typed reasoning, tool items, synthetic/ignored-part exclusion, and the current harness's Gym split.

  Proposed checks, not executed: parse the same database with step-start/text/step-finish parts through both observation parsers with model_ref set. The current harness should split conversation and trajectory; the deprecated parser should retain its previous raw text in both records. Without model_ref, the current harness must retain raw text. Keep the existing model-scope and call-join assertions. Please verify the suggested edits before adopting them.

• responses_api_agents/opencode_agent/artifacts.py line 471: NOTE — Suggestion: document the token-attribution limitation.

  When training token capture is enabled, completion normalization deliberately retains Chat content such as \n\n4 as message text. This converter now returns 4, and the export converter separates the reasoning. Terminal attribution compares those texts without reversing the transformation, so a previously unique content match is lost. If capture contains two independent chains, the consumer's fallback masks the sample rather than delivering the attributed scored chain.

  The README already calls the session path eval-only, so this needn't expand into a token-capture implementation change here. Could you add the concrete limitation to the PR's compatibility notes: enabled capture can lose terminal content attribution and mask multi-chain rollouts after the split? Preserve the scoring fix; don't globally strip tags from captured Chat text, which intentionally preserves literal content for other harnesses.

  Acceptance: inspect that the note identifies enabled training capture and multi-chain masking without implying capture is enabled by default. A proposed regression setup is two clean captured roots with distinct outputs, one ending in \n\n4: the unsplit response uniquely attributes that call, while the split response reports no_content_match and uses the masked multi-chain fallback. A single clean chain retains its existing fallback behavior. These outcomes follow from source inspection; I haven't executed the check.

• responses_api_agents/opencode_agent/app.py line 499: NOTE — Suggestion: test the local routing gate with a populated artifact.

  The new host conversion test passes gym_reasoning directly to the parser, so it doesn't protect this app-level decision. The inspected local-run tests replace the parser, use no populated database, or require the optional binary without asserting this split. This is a coverage suggestion, not a defect in the current conditional.

  Could you add a binary-free _run_opencode test parametrized on configured/unset model_server? Have the fake subprocess copy a prepared database into its supplied XDG_DATA_HOME/opencode/opencode.db, leaving both artifact parsers intact. The scored output and observations must use the same routing decision.

  Proposed assertions, not executed: persist \n\n4. With model_server configured, assert scored text 4 and observed reasoning r plus answer 4. With it unset, assert the original text in both outputs and no invented reasoning item. Retain typed reasoning and nonleading literal-tag behavior. These cases should detect either unconditional value of this gate.

Review generated by codex. This was a full review. Request a full review with /review reset=true.

[gwarmstrong]

Thanks, both of you. ca40230 addresses every item.

Empty final answer (the bot's BLOCKER). Confirmed: both response paths dropped a final message whose Gym reply was only its reasoning block, so equivalence_llm_judge graded an earlier assistant message. The decision is now made per message. A message whose reply was only its reasoning block, with no tool call after it and no answer text in another text part, keeps an empty answer at that reply. A tool-call turn still emits only its reasoning followed by its tool items. A message that has answer text in another part keeps that answer with no extra empty message. Non-Gym literal text and typed reasoning parts are unchanged. The host response (parse_opencode_session), the session output (_session_output, which still converts each part on its own so a bad part costs only that part) and the observed conversation all follow the same rule. Tests build an earlier answer 4, a reasoning-only tool turn and five final-message shapes on the host path and the export parser, and run the reasoning-only shape through the HTTP session path. For that shape, the judge returns reward 0 with failure_reason="empty_final_answer" and never calls its model. Removing the empty answer from either path fails its test. The existing reasoning-only tool-turn HTTP test is unchanged and passes.

The three empty answers in the PR's HLE Diamond evidence were not affected: each was a single policy call with no earlier assistant message, so the judge already scored them empty_final_answer.

The compaction ID, the gym_reasoning opt-in, the local gate test and the token-capture note are in the threads below. The PR body now lists the token-capture limitation under Compatibility.

Validation at ca40230: gym env test +entrypoint=responses_api_agents/opencode_agent: 677 passed, 1 skipped. The opencode_agent, opencode_sandboxed_agent, vibench_agent and kilocode_agent suites together in the repo venv: 1029 passed, 8 skipped, 1 failed. The failure is opencode_sandboxed_agent's test_required_mcp_extension, which runs node --test and fails on this host's Node 19 before reaching code this PR touches. Each new test fails with its fix reverted. pre-commit is clean. I haven't rerun real rollouts on this commit.

[gwarmstrong]

/ok to test ca40230

[gwarmstrong]

HLE smoke with the review fix (ca40230). This PR alone has no session-harness profile, so the run used a merge of this head and #5550's head (c394bcb) with no other changes: 5 HLE tasks, Nemotron 3.5 Super on vLLM, OpenSandbox, opencode_agent through a Gym Model Server.

Slurm 8611407: 5/5 completed and healthy, failures sidecar empty, observations on every rollout, no cleanup or sandbox errors. Compared with the same smoke on #5550's head alone, without this PR:

	With this PR	Without
Final answers containing <think> or </think>	0/5 (591 to 1,050 chars)	5/5 (1.8k to 47k chars)
Judge requests containing <think>	0/5	5/5
Reasoning items per rollout	1 to 2	0

One rollout ended with a reply that was only its reasoning block. Its output ends with an empty assistant message, scored empty_final_answer with reward 0, and the judge was never called. That rollout had no earlier assistant text, so the case where an earlier answer could be graded is covered only by the unit tests.

[giuliolovisotto]

At ca40230, the empty-final-answer handling and emitted compaction-boundary IDs address the earlier concerns in the inspected code and regression tests. Please also exercise the split through the harness-conformance compaction scenario, checking retained reasoning, answer-only verifier input and valid retained-item references.

Non-blocking follow-up: please track the documented token_id_capture terminal-attribution gap explicitly, with a multi-chain regression for no_content_match and masking. Keep that separate from this scoring fix. Source review only; I did not rerun the suites or rollouts.

— @codex

[gwarmstrong]

Thanks. On the conformance compaction scenario: the harness-conformance suite has no compaction scenario. Its README lists compaction as outside its qualification scope, and the eight scenarios are tool_success, tool_failure, usage_omitted, retry_429, retry_500, model_error, judge_verifier and verifier_failure. The suite also can't exercise this split. Its scripted provider puts the reasoning in a separate reasoning_content field and the answer as plain text, and never sends a leading <think> block the way vllm_model does, so OpenCode stores a typed reasoning part and the split has nothing to act on.

I ran the OpenCode probes on ca40230 anyway. They do route OpenCode through a Gym Model Server (model_server: policy_model), so the split's gate is on. Verdict: fulfilled, all eight scenarios fulfilled, and no <think> in any captured reply. That shows the gate leaves plain answers and typed reasoning alone, but it doesn't test the split.

The split with compaction is covered in the unit tests. test_compaction_boundary_uses_only_emitted_assistant_items resolves a compaction whose kept tail starts at a split reply to that reply's reasoning item, from both the current and an earlier activation. The session and host tests check that the verifier input holds only the answer. The real-model evidence is the HLE smoke in the comment above: 0/5 answers and 0/5 judge requests with <think>, reasoning kept as reasoning items. That run did not compact.

If you want the compaction path covered end to end, a probe scenario would need a provider mode that answers like vllm_model, with a leading <think> block in content, plus a compaction-triggering script. That's a change to the conformance suite, so I'd do it in its own PR rather than here.

Token capture: I'll file the attribution gap as its own issue, with the multi-chain no_content_match and masking regression you describe (draft ready), and link it here.

[gwarmstrong]

Filed the token-capture attribution gap as #5609, with the multi-chain no_content_match and masking regression.

[giuliolovisotto]

Agreed: my earlier request assumed compaction coverage available on this revision. The shared scenario has since landed in #5439, but exercising this split still needs the provider extension you describe. That belongs in a separate conformance change and should not block this scoring fix.

The boundary/empty-answer regressions and reported smoke evidence address the scoped concerns; #5609 now tracks token attribution separately. No remaining blocker from my previous review. I inspected the follow-up evidence but did not independently rerun the probes.

— @codex

[giuliolovisotto]

Approved following the re-review of ca40230. Additional conformance coverage and token attribution remain separate follow-ups.

[github-actions]

🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/42065094876