Skip to content

refactor(vllm_model): let callers own transport-log context fields - #4886

Draft
yaoyu-33 wants to merge 16 commits into
NVIDIA-NeMo:mainfrom
yaoyu-33:refactor/vllm-transport-log-passthrough
Draft

yaoyu-33 wants to merge 16 commits into
NVIDIA-NeMo:mainfrom
yaoyu-33:refactor/vllm-transport-log-passthrough

Conversation

@yaoyu-33

@yaoyu-33 yaoyu-33 commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Note

Stacked on #3591. This branch starts from #3591's head (c004893a9), so until #3591 merges this diff also shows #3591's 11 files. Review only the last commit, 56c4518d5. It changes 5 files (+106 / −106). I will rebase onto main after #3591 lands, then mark this PR ready for review.

What does this PR do?

This moves the OSWorld-only transport-log fields out of the vLLM model server, the follow-up requested in the #3591 review.

responses_api_models/vllm_model/app.py hard-coded thirteen x-nemo-gym-log-* header names: seven on main, six more added by #3591. The only sender of any of them is osworld_agent, which caused four problems:

  • the generic model server carried OSWorld/NeMo-RL field names such as domain, parse_attempt and sampling_event_id;
  • every new OSWorld field needed a vllm_model change;
  • four fields the agent already sends (rollout_purpose, agent_contract_id, history_policy_id, model_protocol_id) were silently dropped;
  • the transport log showed a top-level rollout_id that is not Gym's _ng_rollout_id and is easy to mistake for it.

Changes:

  • osworld_agent sends its normalized log context as one JSON object in the x-nemo-gym-log-context header. The field list, integer normalization and 1,024-character bound stay in _OSWORLD_LOG_CONTEXT_FIELDS / _normalize_log_context, where they already lived. The JSON is ASCII-only, so control characters are escaped instead of stripped.
  • vllm_model reads that header as an opaque object and records it unchanged under one key, log_context, in its opt-in transport log (NEMO_GYM_VLLM_TRANSPORT_LOG). It no longer names any field. The caller's fields can no longer pass for the log's own (event, call_index, payloads, …). Transport events move to schema version 3. A missing, malformed or non-object header is ignored, and the context never reaches the model body or the engine.
  • benchmarks/osworld/README.md names the header and the log_context key.

No issue was opened: this is the follow-up agreed in the #3591 review.

Compatibility

  • Transport log format: changed. Correlation fields move from the top level of each model-io-transport.jsonl record to log_context, with the same names and types (integers stay integers), plus the four fields above that were previously dropped. Records now carry schema_version: 3, so analysis that reads task_id, step and similar fields at the top level must read them from log_context for version 3. The agent log (OSWORLD_MODEL_IO_LOG) is unchanged and still carries the same fields at the top level.
  • Header contract: vllm_model no longer reads the per-field x-nemo-gym-log-* headers. The only in-repo sender (osworld_agent) is updated here. An out-of-tree client that set those headers would lose them from the transport log until it sends x-nemo-gym-log-context instead.
  • Benchmark impact: none. This affects only an opt-in diagnostic log, not requests, responses, token IDs or rewards.

Validation

On macOS ARM64 with Python 3.13.14, using a venv built from this branch (uv sync --extra dev --extra telemetry --extra ray):

  • responses_api_models/vllm_model/tests: 571 passed.

  • responses_api_agents/osworld_agent/tests: 385 passed; the base commit has 384.

  • The new and changed tests fail when only the two app.py files are reverted (the end-to-end test also fails against the earlier flat version of this PR):

    • test_transport_log_context_is_the_callers_json_object
    • test_transport_log_records_the_callers_context_under_one_key, an end-to-end /v1/responses call. It checks that the context lands under log_context (schema version 3) and not at the top level, that a caller-supplied event stays inside it, and that the context does not reach the engine.
    • test_log_context_headers_do_not_change_model_payload
    • test_log_context_headers_drop_unknown_fields_and_bound_values
    • test_messages_model_fn_propagates_task_context_in_headers_and_logs

    The malformed-header cases are guards and also pass on the old code.

  • pre-commit run on the changed files: passed.

Rollouts

N/A. This changes only the opt-in transport log's context plumbing. It does not change the model request body, sampling, responses, token IDs, verification or rewards, so no GPU or OSWorld VM run was made.

🤖 Generated with Claude Code

JeffPengCoder and others added 15 commits September 22, 2026 11:59
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>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Merge upstream main through 64a734e. Preserve environment-based config transport, bounded rollout admission, episode identity and HTTP retry/telemetry controls. Copy legacy dispatch payloads only after admission; keep role-local runtime policy and E2B embedded-worker compatibility. Add regressions for the merged contracts.

Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Merge upstream main 8908cb2. Resolve uv.lock by preserving both e2b>=2.46.0,<3.0.0 and fastapi!=0.142.*. No additional package version changes relative to upstream main.

Validation: 2804 focused regression tests passed with aiohttp 3.14.4, yarl 1.25.1, FastAPI 0.135.2 and real E2B SDK 2.46.0. One known Linux-procfs-specific test was deselected on macOS. uv lock --check --offline and pre-commit run --all-files passed. No live GPU rollout or training run was launched.
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
Merge upstream main d96b337. Keep main's server startup environment, Ray auto-connect guard, reserved control-client pool, archived-audio splicing and requested Gym extras alongside this branch's runtime contracts.

- Setup: main now pins the parent's Ray version as a constraint (head_server_constraints) rather than a dependency. Keep its -c flag and also write the constraints into the per-venv parent-runtime overrides file, so a stale exact Ray pin in a server dependency is replaced instead of making the solve unsatisfiable; a server that does not require Ray still does not install it. Include the constraints in the environment identity.
- HTTP lifecycle: close the reserved control client together with the data client on the owner loop.
- vLLM proxy: keep the purpose marker where it was, before the new audio-splicing helper.

Validation: the core unit suite plus the vLLM, simple/compaction-agent and OSWorld server tests ran against this merge and against main d96b337 in the same environments (Python 3.13.14, aiohttp 3.14.4, yarl 1.25.1, FastAPI 0.135.2, real E2B SDK 2.46.0). On a Linux x86_64 CPU node, 10617 passed (main: 10563); every remaining failure also fails on main (optional mlflow/wandb absent, a symlinked work directory, and the OSWorld requirements assertion left by main NVIDIA-NeMo#4301), except one load-sensitive 10 s subprocess timeout in test_slurm_script that passes in isolation on both trees. On macOS ARM64, no test that passes on main fails here. The focused regression selection passed 3031 of 3034; its 3 failures are identical on main. pre-commit run --all-files, uv lock --check --offline and the main-relative git diff --check passed. No live GPU rollout or training run was launched.
Signed-off-by: Jeff Peng <jepeng@nvidia.com>
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>
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>
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>
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>
…#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>
…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>
@copy-pr-bot

copy-pr-bot Bot commented Oct 21, 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.

@svcnvidia-nemo-ci svcnvidia-nemo-ci added area:model Model servers, inference providers, and model adapters feature New capabilities, enhancements, or enablement work labels Oct 21, 2026
The vLLM model server hard-coded thirteen x-nemo-gym-log-* header names, all of
them OSWorld/NeMo-RL fields sent only by osworld_agent. Each new OSWorld field
needed a vllm_model change, four the agent already sends (rollout_purpose,
agent_contract_id, history_policy_id, model_protocol_id) were dropped, and the
log showed a top-level rollout_id that is not Gym's _ng_rollout_id.

osworld_agent now sends its normalized context as one JSON object in the
x-nemo-gym-log-context header. vllm_model records that object unchanged under
its own log_context key, without interpreting it, and transport events move to
schema version 3. The agent's types are kept (integers stay integers), a missing
or malformed header is ignored, and the context still never reaches the model
body.

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

Copy link
Copy Markdown
Collaborator

I re-checked this at 56c4518d5, the commit we reviewed in #3591 (comment). It hasn't changed since. No problems from our side:

  • No OSWorld field names in vllm_model.
    • It stores the caller's context unchanged under log_context, with transport schema_version: 3. There is no longer a top-level rollout_id that could be confused with _ng_rollout_id.
    • _OSWORLD_LOG_CONTEXT_FIELDS is now the only field list.
    • OSWORLD_MODEL_IO_LOG keeps the same fields at its top level.
  • The header is always valid.
    • It is one compact JSON object written with ensure_ascii=True, which is more reliable than stripping CR/LF.
    • A missing or malformed header becomes {} and does not fail the request.
  • Nothing depends on the moved fields. No other sender or reader on main uses them. Our OSWorld validation scripts only count transport events by the top-level event, which stays where it is.
  • The PR body is accurate. Its compatibility note matches the change.
  • It merges cleanly. The commit applies cleanly on current main (aa53f3bdf). It doesn't conflict with fix(osworld): keep the Nano Omni profile's sampling and document its Omni v3 recipe #4897: both edit benchmarks/osworld/README.md, but in different sections.

The branch still carries #3591's pre-merge commits. Because #3591 was squash-merged, those commits now conflict with main, so the branch needs the planned rebase before it can leave draft. @Glorf, could you approve once it's rebased? Approvals are dismissed on push here, so an approval on the current head wouldn't carry over.

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:model Model servers, inference providers, and model adapters feature New capabilities, enhancements, or enablement work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants