Skip to content

refactor(agent): decouple simple-agent compaction from model serving - #3368

Open
aroshanghias-nvd wants to merge 28 commits into
NVIDIA-NeMo:mainfrom
aroshanghias-nvd:cc-capture-redesign-20260914
Open

aroshanghias-nvd wants to merge 28 commits into
NVIDIA-NeMo:mainfrom
aroshanghias-nvd:cc-capture-redesign-20260914

Conversation

@aroshanghias-nvd

@aroshanghias-nvd aroshanghias-nvd commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

The native compaction agent returns ordinary accepted rollout history and uses shared capture, allowing NeMo-RL to determine training segments. Semantic history, recency policies, guards, and the context client move into the optional nemo_gym.context_management package, and the agent shares termination semantics with simple_agent. The dedicated legacy compaction model/session are replaced by the ordinary vLLM model and context client, including removal of the associated ASR model entry.

Depends on shared infrastructure #4128, which should merge first. External harnesses that own compaction and NeMo-RL #4124 need the infrastructure layer; they do not need this optional native-agent migration.

Review scope: GitHub requires a base branch in the upstream repository, where this fork cannot create branches. This PR therefore remains based on main and its Files changed tab still includes #4128 until that PR merges. Review the agent-only diff for this layer. Once #4128 merges, update/rebase this PR so only the native-agent changes remain against main.

The original split preserved the entire source tree from head cf908566. Both layers are now rebased onto Gym main 366a9218; the agent commit remains patch-identical by git range-diff. Fresh upper-layer tests passed 95 cases on Mac, and pre-commit run --all-files passed on both layers. The rebased lower layer passed 1,761 Mac CPU cases. No dependency or lockfile changes are included. The lower-only infrastructure run independently passed two optimizer steps and exact capture/media audits with the compaction client package absent (W&B, maximum TMPE 1.000021). The GPU evidence predates this latest rebase and has not been rerun. Repository CI remains required before merge.

Previously documented combined-stack validation

The evidence below predates this source-preserving split. The new lower-only check is a two-step integration check, not a repeat of these 20-optimizer-step runs.

  • 727 affected Gym tests passed after the main merge; pre-commit run --all-files passed.
  • 152 paired RL compatibility/historical-consumer/exporter tests and 165 native capture/media/template tests passed; native tests had no skips. Startup using the actual TQ reassembler passed.
  • Four scripted 20-turn before/after cases (compaction on/off, rejected attempts on/off) preserved requests, parent hints, accepted outputs, histories, and policy records.
  • The paired live runs below each completed 20 optimizer steps and 40 rollouts. Saved-evidence audits verified exact training tokens, masks, rollout logprobs, rewards, and accepted-call membership; every selected generation was trained exactly once. The manufactured image run also verified image tensors and retained-image counts.
Run Training rows Selected calls
Manufactured, no compaction 40 800
Manufactured, image K2 400 800
Custom OpenCode, identity 40 850
Custom OpenCode, compaction 120 880

Qualification used paired source in a private aligned runtime (Ray 2.58.0, vLLM 0.29.0), not final CI against a merged dependency. The custom fork used a disclosed temporary collector ordering correction and a bounded tool task; stock OpenCode and SWE-bench are not qualified.

The fork's overlapping title/agent calls encountered transient admission refusals and retried successfully. All 80 final ledgers had zero pending/failed calls. Two Mac CPU ASGI reproductions confirmed that the rejected overlap never starts generation or poisons the ledger. The sequential guard is retained; arbitrary concurrent/branching capture remains unsupported by this qualification. The generic HTTP 500 and ambiguous pending-capture error text remain a diagnostic limitation.

The paired training TMPE investigation remains unresolved and deferred, with maximum OpenCode identity/compaction values of 1.0015546 / 1.0094431. Exact reconstruction does not prove numerical equivalence between generation and learner forward passes; no experimental router-padding changes are included.

@copy-pr-bot

copy-pr-bot Bot commented Sep 14, 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.

lauradang pushed a commit to lauradang/Gym that referenced this pull request Sep 23, 2026
Pass immutable local tensor attachments through worker capture and preserve image descriptors in vLLM extras. Retain stage-before-ack ordering and token-only sink compatibility. Builds on the served-response ancestry fix from NVIDIA-NeMo#3301 and the VLM capture work in NVIDIA-NeMo#3368.

Signed-off-by: Pranav Prashant Thombre <pthombre@nvidia.com>
@aroshanghias-nvd
aroshanghias-nvd force-pushed the cc-capture-redesign-20260914 branch from 3e8a828 to 21965f1 Compare September 30, 2026 01:04
@aroshanghias-nvd aroshanghias-nvd changed the title feat(cc): shared context compaction over worker-owned capture feat(capture): support framework-owned context reconstruction Sep 30, 2026
@aroshanghias-nvd
aroshanghias-nvd force-pushed the cc-capture-redesign-20260914 branch from 21965f1 to 4769525 Compare September 30, 2026 01:13
@aroshanghias-nvd

aroshanghias-nvd commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor Author

Please authorize the standard Gym CI for draft PR #3368 at 4870eb240475e078c4ea96af54ca2393298f4170. This updates the existing CC draft with the reviewed synthetic-digest baseline correction; the changed-file secrets scan passes locally. The paired RL PR is rebased onto merged #4191. Native/runtime qualification remains pending, and this PR remains a draft.

@aroshanghias-nvd
aroshanghias-nvd marked this pull request as ready for review September 30, 2026 21:35
@aroshanghias-nvd
aroshanghias-nvd requested a review from a team as a code owner September 30, 2026 21:35
@yaoyu-33 yaoyu-33 added area:training Training framework integrations and training-data interfaces complexity:high Cross-component or shared-contract change with a large review surface feature New capabilities, enhancements, or enablement work needs-review PR is ready for code review and waiting on a reviewer labels Sep 30, 2026
@github-actions github-actions Bot added the sla:triage-overdue Review assignment is over the one-business-day SLA label Oct 1, 2026
@aroshanghias-nvd
aroshanghias-nvd force-pushed the cc-capture-redesign-20260914 branch from cf90856 to 115f2db Compare October 6, 2026 19:43
@aroshanghias-nvd aroshanghias-nvd changed the title feat(capture): support framework-owned context reconstruction refactor(agent): build compaction agent on shared capture Oct 6, 2026
Let training frameworks own capture roots and continuation decisions using request replay evidence and committed coordinates. Preserve completion identity, buffered Responses capture, explicit retry controls, and exact measurement through the ordinary vLLM server.

Keep the existing compaction agent and model intact in this infrastructure layer. The native agent migration is a separate stacked change.

Validation: 618 core/model cases and 56 retained legacy agent/model cases passed on Mac CPU across the documented runs. Combined-head GPU smoke evidence is recorded separately.

Co-authored-by: alexandery <alexandery@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Preserve concurrent framework capture alongside upstream rollout timing, outbound telemetry, tool-call attribution, and canonical finish reasons. Keep retry suppression effective with metrics and tracing enabled.

Validation: 2,475 affected Gym CPU tests and 184 paired RL compatibility tests passed; two optional custom OpenCode exporter cases skipped. Full pre-commit passed. Fern check passed with the unauthenticated redirect check skipped.
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Move semantic history, policies and the context client into the reusable context_management package. Return ordinary accepted rollout history from simple_agent_with_compaction and use the ordinary vLLM capture server.

Preserve shared termination semantics and remove the replaced legacy compaction session/model with its paired ASR configuration entry. Context management stays optional for external harnesses.

The combined source tree exactly matches the qualified Gym PR head cf90856; no dependency or lockfile changes.

Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
@aroshanghias-nvd
aroshanghias-nvd force-pushed the cc-capture-redesign-20260914 branch from 8eec6bf to d5be5af Compare October 20, 2026 18:58
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
@aroshanghias-nvd

Copy link
Copy Markdown
Contributor Author

@yaoyu-33 could you review this PR? In particular, should the compaction logic stay in simple_agent_with_compaction, or would it make sense to extract it into a shared library that other agents can reuse?

JeffPengCoder added a commit to JeffPengCoder/Gym that referenced this pull request Oct 21, 2026
Publishing the final policy decision only after guard admission changes nothing observable today: a guard rejection raises before build_response, the only place the decision is published. NVIDIA-NeMo#3368 deletes this session, and its replacement client checks guards before generation and acknowledges history only after accepting a response, so the invariant belongs there. Restore session.py and its test to main's versions; this also removes the only conflict with NVIDIA-NeMo#3368.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
@JeffPengCoder

Copy link
Copy Markdown
Collaborator

@aroshanghias-nvd, following up on what I said in #3591: here are regression tests for the guard invariant on the new client. A view rejected by a context guard must never be sent to the model or become acknowledged history.

#3591 no longer touches the legacy ContextCompactionSession, so this invariant now lives only in this PR. The tests pass on 8cdaf672 (3 passed). Take, adapt or drop them as you see fit. They reach into a few private attributes (_completed_view_digest, _last_accepted_response_id, _step), so a public accessor may read better.

  • A first action rejected by a guard is neither generated nor acknowledged, and the client closes.
  • In turn-chunked mode, a rejection keeps only previously accepted history; the chunk record names the guard.
  • Positive control: with a recency policy, the same guard admits after recompaction.
test_guard_invariant.py
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
"""A view rejected by a context guard is never sent to the model and never becomes acknowledged history."""

import json
from copy import deepcopy
from unittest.mock import AsyncMock, MagicMock

import pytest

from nemo_gym.config_types import ModelServerRef
from nemo_gym.context_management import ContextHistoryConfig, ContextManagedResponsesClient
from nemo_gym.context_management.client import ContextGuardRejected
from nemo_gym.openai_utils import NeMoGymResponse, NeMoGymResponseCreateParamsNonStreaming
from nemo_gym.server_utils import ServerClient


def answer(index):
    return NeMoGymResponse(
        id=f"response-{index}",
        created_at=0,
        error=None,
        incomplete_details=None,
        instructions=None,
        metadata=None,
        model="test",
        object="response",
        parallel_tool_calls=True,
        temperature=None,
        tool_choice="auto",
        tools=[],
        top_p=None,
        output=[
            {
                "id": f"message-{index}",
                "type": "message",
                "role": "assistant",
                "status": "completed",
                "content": [{"type": "output_text", "text": f"answer {index}", "annotations": []}],
            }
        ],
    )


def http_response(payload):
    response = MagicMock(ok=True, status=200, cookies={})
    response.read = AsyncMock(return_value=json.dumps(payload).encode())
    return response


def image_observation(name):
    return {
        "role": "user",
        "type": "message",
        "content": [
            {"type": "input_image", "image_url": f"https://example.invalid/{name}.png", "detail": "auto"},
            {"type": "input_text", "text": f"observation {name}"},
        ],
    }


def make_client(config, initial_input="task"):
    transport = MagicMock(spec=ServerClient)
    queue = [answer(index) for index in range(1, 20)]
    calls = []

    async def post(**kwargs):
        calls.append(deepcopy(kwargs))
        if kwargs["url_path"].endswith("/measure"):
            return http_response({"prompt_token_count": 10})
        return http_response(queue.pop(0).model_dump(mode="json"))

    transport.post = AsyncMock(side_effect=post)
    client = ContextManagedResponsesClient(
        server_client=transport,
        model_server=ModelServerRef(name="policy", type="responses_api_models"),
        logical_rollout_id="guard_g0",
        config=config,
        initial_request=NeMoGymResponseCreateParamsNonStreaming(input=initial_input, max_output_tokens=8),
    )
    return client, calls


def generation_calls(calls):
    return [call for call in calls if call["url_path"].endswith("/v1/responses")]


async def test_first_action_rejected_by_guard_is_never_generated_or_acknowledged():
    # Same scenario as #3591's legacy regression: an image in the initial context, no images allowed.
    config = ContextHistoryConfig.model_validate({"enabled": True, "guards": {"max_active_images": 0}})
    client, calls = make_client(config, initial_input=[image_observation("initial")])
    events_before = len(client.history.events)

    with pytest.raises(ContextGuardRejected):
        await client.create()

    assert generation_calls(calls) == []
    assert client.controller._completed_view_digest is None
    assert client._last_accepted_response_id is None
    assert client._step == 1
    assert len(client.history.events) == events_before
    with pytest.raises(RuntimeError, match="closed"):
        await client.create()
    with pytest.raises(RuntimeError, match="closed"):
        client.finish(answer(1))


async def test_chunked_guard_rejection_keeps_only_previously_accepted_history():
    config = ContextHistoryConfig.model_validate(
        {
            "enabled": True,
            "schedule": {"type": "turn_chunked_recency", "actions_per_chunk": 3},
            "guards": {"max_active_images": 1},
        }
    )
    client, calls = make_client(config)
    first = await client.create()
    client.append_observation([image_observation("a")])
    second = await client.create()
    accepted_digest = client.controller._completed_view_digest
    events_before = len(client.history.events)
    client.append_observation([image_observation("b")])
    events_with_observation = len(client.history.events)

    # Identity policy keeps both images, so closing the chunk cannot satisfy the guard.
    with pytest.raises(ContextGuardRejected):
        await client.create()

    assert len(generation_calls(calls)) == 2
    assert client._last_accepted_response_id == second.id
    assert client.controller._completed_view_digest == accepted_digest
    assert events_with_observation == events_before + 1
    assert len(client.history.events) == events_with_observation
    records = client.controller.chunk_records
    assert len(records) == 1
    assert records[0].eligible_action_ids == (first.id, second.id)
    assert records[0].early_close_reason == "guard:active_images"
    with pytest.raises(RuntimeError, match="closed"):
        await client.create()


async def test_chunked_guard_admits_after_recompaction_positive_control():
    config = ContextHistoryConfig.model_validate(
        {
            "enabled": True,
            "policy": {"type": "recency", "config": {"images": {"enabled": True, "keep_last_groups": 1}}},
            "schedule": {"type": "turn_chunked_recency", "actions_per_chunk": 3},
            "guards": {"max_active_images": 1},
        }
    )
    client, calls = make_client(config)
    await client.create()
    client.append_observation([image_observation("a")])
    await client.create()
    client.append_observation([image_observation("b")])
    third = await client.create()

    assert len(generation_calls(calls)) == 3
    assert client._last_accepted_response_id == third.id
    assert client.controller.chunk_records[0].early_close_reason == "guard:active_images"

Merge main while preserving rollout dispatch tracing. Validate Responses
conversion before intent registration, shield cancellation failure writes,
reuse connection-safe replay controls, and root ambiguous source candidates
without selecting a parent.

Validated with 2,279 Gym CPU cases, four paired Gym/RL selection and
reconstruction cases, and pre-commit --all-files.

Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
zyzhou5 pushed a commit to zyzhou5/Gym that referenced this pull request Oct 21, 2026
…VIDIA-NeMo#3591)

* fix(core): isolate runtime, sandbox and model transport contracts

Extract shared support from JeffPengCoder/Gym 360b0df. Include E2B endpoint/transport and vLLM model changes; retain upstream GymAiohttpTransport while preserving the no-config Ray worker fallback. Keep OSWorld tasks, agents and resources out of this PR.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(e2b): align SDK installation guidance with gateway requirements

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(cli): keep site-packages off server import paths in wheel installs

In a wheel install the built-in servers live in the parent interpreter's site-packages, so dir_path.parent.parent is that site-packages. Passing it as project_root put it first on each server's PYTHONPATH and NEMO_GYM_EXTRA_ROOTS, ahead of the server venv's own pinned dependencies. Apply the source-checkout guard the verifier runner already uses, and test both layouts; the wheel-install case fails without the guard.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(rollout): keep index-derived capture ids and gate dispatch logging

Rows without a caller-supplied _ng_rollout_id no longer receive a random rollout-<uuid4>. The capture key stays main's {task}-{rollout}, so a resumed attempt decodes to the same rollout as attempt 0 and existing capture files keep their names. A caller-supplied id is still forwarded unchanged. Rows are no longer mutated, so the per-dispatch deep copy is removed as well.

The per-dispatch summary is printed only when request debugging is enabled, like the reverification summary. The summary drops the NeMo-RL purpose and trajectory-identity fields and adds only the core _ng_rollout_id.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(vllm): drop the per-request purpose marker

The marker printed on every chat-completion request and ran before sampling overrides and capture/prefix preparation, so it could report temperature and top_p values the engine never received. The opt-in transport log (NEMO_GYM_VLLM_TRANSPORT_LOG) already records the final request body together with the rollout context headers. A test now checks that the log holds the post-override values and that nothing is printed per request.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* chore(agent): drop the legacy compaction session change

Publishing the final policy decision only after guard admission changes nothing observable today: a guard rejection raises before build_response, the only place the decision is published. NVIDIA-NeMo#3368 deletes this session, and its replacement client checks guards before generation and acknowledges history only after accepting a response, so the invariant belongs there. Restore session.py and its test to main's versions; this also removes the only conflict with NVIDIA-NeMo#3368.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* chore(core): leave provider and HTTP lifecycle changes to NVIDIA-NeMo#4822 and NVIDIA-NeMo#4823

The sandbox-provider changes (Docker create reconciliation, E2B service endpoints and transport fallback, OpenSandbox endpoint compatibility, the e2b>=2.46.0 range) now live in NVIDIA-NeMo#4822, and the owner-loop close of the shared aiohttp clients with wildcard-host handling lives in NVIDIA-NeMo#4823. Restore those 16 files to this branch's main base so each change is reviewed in exactly one PR. Nothing else in this PR uses them; it keeps the component runtime policy, rollout identity and the vLLM transport-log context.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>

* fix(core): keep server venvs on the parent Python and fail loudly on pin conflicts

- A server's .python-version now selects its interpreter only when it excludes the
  parent's Python. A request that admits it (such as >=3.13.14,<3.14, 3.13 or
  cpython@3.13) builds the venv with the parent's exact version, because Ray rejects
  processes on different patch releases. Variant and pre-release requests
  (3.13+freethreaded, 3.13.14+debug, 3.13.14rc1) and requests for other implementations
  are passed to uv unchanged. Like uv, the first non-comment line is the request.
- Only the parent's constraints (the Ray pin) are passed as uv overrides, through process
  substitution like the existing -c flag. head_server_deps such as openai stay ordinary
  requirements, so a server pin that conflicts with them fails setup instead of being
  replaced silently.
- Drop the environment-identity file: nothing read it, so it could not detect a stale venv,
  and its tests passed whatever it contained.
- Replace the vacuous vllm_model marker-file test with one that checks the real policy, and
  document per-server .python-version/uv.toml and NEMO_GYM_ALLOWED_COMPONENT_ROOTS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>

---------

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Co-authored-by: Onur Yilmaz <35306097+oyilmaz-nvidia@users.noreply.github.com>
Co-authored-by: yaoyu-33 <yaoyu.094@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Replace removed _retry arguments in context measurement, generation, and resources calls. Exercise the real ServerClient retry stack against aiohttp's accepted arguments, preserving safe connection retries and refusing ambiguous resends.

Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
…ements

Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
@aroshanghias-nvd

Copy link
Copy Markdown
Contributor Author

/ok to test 4d72925

Merge Gym main at 4945282. Retain upstream output-budget classification in both backend routes and source-history worker-acknowledged refusal handling.

Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>
…20261026

Signed-off-by: Ali Roshan Ghias <aroshanghias@nvidia.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:training Training framework integrations and training-data interfaces complexity:high Cross-component or shared-contract change with a large review surface feature New capabilities, enhancements, or enablement work needs-review PR is ready for code review and waiting on a reviewer sla:triage-overdue Review assignment is over the one-business-day SLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants