[DO NOT MERGE] bxyu dev - #2175
Draft
bxyu-nvidia wants to merge 934 commits into
Draft
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-bxyu-harness-benchmark-agnostic.docs.buildwithfern.com/nemo/gym Here are the markdown pages you've updated: |
bxyu-nvidia
changed the base branch from
ansubramania/sandbox-server
to
bxyu/harness-benchmark-agnostic-base
July 29, 2026 03:57
5 tasks
5 tasks
This was
linked to
issues
Jul 31, 2026
Open
bxyu-nvidia
changed the base branch from
bxyu/harness-benchmark-agnostic-base
to
main
August 5, 2026 15:55
bxyu-nvidia
added a commit
that referenced
this pull request
Aug 7, 2026
Taken from #2175 --------- Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
…arness-benchmark-agnostic
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Sugam Devare <sdevare@nvidia.com> Co-authored-by: Brian Yu <bxyu@nvidia.com>
…arness-benchmark-agnostic
…NVIDIA-NeMo/Gym into bxyu/harness-benchmark-agnostic
…arness-benchmark-agnostic Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
…-NeMo/Gym into bxyu/harness-benchmark-agnostic
…arness-benchmark-agnostic
Signed-off-by: Brian Yu <bxyu@nvidia.com>
bxyu-nvidia
added a commit
that referenced
this pull request
Aug 28, 2026
## Problem On terminal-bench 2.1 runs at 1k-sandbox scale, verify-phase re-attaches intermittently fail with: ``` nemo_gym.sandbox.providers.base.SandboxPtyError: PTY session already has an attached client ``` even though the attach passes `takeover=True`. ## Root cause The tb2.1 flow passes one PTY session through three holders — resources server (create) → opencode agent → resources server (verify) — and every handoff relies on takeover-evicting the previous client. execd implements takeover as a cooperative eviction with a timeout (its binary carries `takeover timed out for pty session`). Eviction is only cheap while the previous client is alive: a socket idled through the whole agent phase is exactly the one the NLB/server-proxy path drops silently, so the next takeover waits out execd's eviction timeout and surfaces as a policy-violation close, which the client maps to "already has an attached client". The stale peer is torn down in the background, so a fresh attempt lands. ## Changes 1. **Retry the takeover attach** (`provider.attach_pty`): up to three re-dials (2s/5s/10s) on exactly that signature, fresh HTTP client per attempt. Non-takeover rejections and other errors stay definitive. 2. **Detach the baton instead of dangling it** (`tb2.1 seed_session`): the creator detaches right after creating the session, so the agent's attach finds nothing to evict; the server-side session keeps running and replays on reattach. Takeover+retry becomes the recovery path for crashed holders, not the routine handoff. 3. **Websocket heartbeats on every PTY dial** (30s): intermediaries stop idle-reaping healthy sockets, and a dead socket surfaces as a failed ping and re-dials via the existing reattach path within ~a minute instead of dangling half-open until the next takeover times out against it. ### For the tb2.1 dev branch (#2175) — same contract, two snippets The agent-attach and verify-reattach code paths live on the dev branch, so two spots there complete the pattern: - opencode agent server, when the rollout finishes: `await pty_session.detach()` — whoever is done with the terminal detaches; takeover is for crashes. - verify, restoring a client: keep `attach(session_id=..., takeover=True)` (it now retries), or reattach the stored session when it is merely detached. ## Testing - Unit: `tests/unit_tests/test_opensandbox_pty.py` — 61 passed (new: takeover-timeout retry lands on re-dial with the failed client released; no retry without takeover; every dial passes the heartbeat). - E2E against the cell-2 OpenSandbox cluster (real execd through the server proxy), full three-holder flow with this branch's code: - creator opened a session, ran a command, **detached**; - agent takeover-attached (nothing to evict), set shell state, **detached**; - verify takeover-attached and ran the check **detached across multiple reattach polls** (12s command, 4s poll); - shell state set by the agent (`$AGENT_MARK`) was visible in verify — one server-side session across all three handoffs; exit code 0; sandbox deleted cleanly. - Separately verified live takeover of an attached client (evictee sees close 4001 "taken over") and that a no-takeover attach against a held session returns exactly the retried signature. Companion platform ask (separate): execd should force-close the stale socket instead of waiting for a half-open peer, and use a distinct close code for takeover-timeout vs genuinely-attached. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Hemil Desai <hemild@nvidia.com> Signed-off-by: Brian Yu <bxyu@nvidia.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Brian Yu <bxyu@nvidia.com>
…arness-benchmark-agnostic Signed-off-by: Brian Yu <bxyu@nvidia.com>
Returning a bare model or dict from responses_dispatch, chat_completions_dispatch, or messages makes FastAPI serialize it with jsonable_encoder — a pure-Python recursive walk with one function call per element — followed by stdlib json.dumps. On training responses the prompt/generation token-id and logprob arrays make that walk the dominant CPU cost of the model server. Build the response bytes in the dispatch instead: model_dump(mode="json") (the exact conversion jsonable_encoder performs first for a model) plus orjson.dumps, handed to FastAPI as a finished Response, which it passes through untouched. Responses that are already a Response (server overrides) pass through unchanged, and the streaming branches and 422 validation path are untouched. Measured on a SimpleResponsesAPIModel serving training-shaped bodies (64k generated tokens, token ids + full-precision logprobs, 1.97 MB) through the production middleware stack, per uvicorn worker: - 1.97 MB body: 8.1 -> 157 req/s (19x); p50 at concurrency 32: 3.9 s -> 199 ms - 37 KB tool-call body: 225 -> 1398 req/s (6x) Parsed response payloads are identical before and after in every cell. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
Gym depends on plain uvicorn rather than uvicorn[standard], so httptools was never installed and every server silently fell back to uvicorn's pure-Python h11 protocol implementation for HTTP parsing. Add httptools (C bindings to the llhttp parser) as a dependency and name it explicitly in the uvicorn config: uvicorn's "auto" selection only picks httptools when the wheel is importable and falls back to h11 silently, so an explicit http="httptools" makes a missing wheel fail loudly at startup instead of silently degrading. Measured on a SimpleResponsesAPIModel serving training-shaped bodies through the production middleware stack: +1-4% alone (response serialization dominates the server today), rising to +5-11% at high request rates once the orjson dispatch-serialization change lands. Wheels cover manylinux/musllinux aarch64 and x86_64 for cp310-cp314. No behavior change for well-formed traffic; llhttp is stricter than h11 on malformed requests. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
Explain the serialization sequence with short, reader-focused sentences so the helper's performance purpose and response handling are clear. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
Use short sentences to explain why Gym explicitly selects httptools and how startup behaves when the dependency is unavailable. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
Verify that dictionaries and Pydantic models produce encoded JSON responses and that model-server response overrides pass through unchanged. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…ub.com/ananthsub/Gym into bxyu/test-ananth
…thsub/Gym into bxyu/test-ananth
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Signed-off-by: Brian Yu <bxyu@nvidia.com>
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.
Internal RFC under section "Harness x Benchmark decoupling RFC"