Alias as Penguin - #4
Merged
Merged
Conversation
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>
chtruong814
approved these changes
Sep 5, 2025
kbhardwaj-nvidia
pushed a commit
that referenced
this pull request
Sep 5, 2025
Signed-off-by: Brian Yu <bxyu@nvidia.com> Signed-off-by: Khushi Bhardwaj <kbhardwaj@nvidia.com>
soares-f
pushed a commit
that referenced
this pull request
Sep 8, 2025
Signed-off-by: Brian Yu <bxyu@nvidia.com> Signed-off-by: soares-f <soarescmsa@gmail.com>
soares-f
pushed a commit
that referenced
this pull request
Sep 15, 2025
Signed-off-by: Brian Yu <bxyu@nvidia.com> Signed-off-by: soares-f <soarescmsa@gmail.com>
abubakaria56
pushed a commit
to abubakaria56/Gym
that referenced
this pull request
Feb 28, 2026
…validator Fixes/lang validator
abubakaria56
pushed a commit
to abubakaria56/Gym
that referenced
this pull request
Mar 2, 2026
Signed-off-by: Brian Yu <bxyu@nvidia.com>
abubakaria56
pushed a commit
to abubakaria56/Gym
that referenced
this pull request
Mar 2, 2026
Signed-off-by: Brian Yu <bxyu@nvidia.com>
Glorf
referenced
this pull request
in Glorf/Gym
May 21, 2026
Drop multi-paragraph docstrings, "Ported from NEL", "pre-fix #4", "Phase-1.5", "CLAUDE.md:358" and similar backstory that belongs in the PR description or commit log rather than in code. Keep one-line WHY notes only where the reason is non-obvious (Set-Cookie list preservation, content-encoding stripping, max_concurrent shape compat). No behavior change; 49 ray-free unit tests still pass and the local adapter test battery is 40/40. Signed-off-by: Michal Bien <mbien@nvidia.com>
4 tasks
This was referenced Jun 25, 2026
copy-pr-bot Bot
pushed a commit
that referenced
this pull request
Jun 25, 2026
…audit) - (#4) Cache the static mcp_config (read once, reuse) instead of reading it from disk every rollout. - (#5) Document that the per-rollout Gym entry overwrites a same-named static mcp_config server. - (#6) Warn when seed MCP metadata has no headers (tool calls would otherwise be unauthenticated). - (#7) Add an agent test asserting the session cookie is threaded seed_session -> verify. - (#8a) Move the example task ground truth (expected_city) under verifier_metadata, per convention. - (#8b) Use the message-list input format in the example dataset, like the other examples. - (#9) Remove the dead 'url' branch in the rollout mcp_config (MCPServerMetadata has no 'url'); audited that every metadata field accessed (server_name/url_path/transport/headers) actually exists. - (#2v) Add a test that a token-less MCP tool call surfaces as a clean isError (HTTP 200), not a 401. Also patch only ensure_claude_code (not all of model_post_init) in the claude_code_agent test helper, so the model's private attributes initialize. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Codex <codex@openai.com>
This was referenced Jun 28, 2026
adil-a
added a commit
that referenced
this pull request
Jun 29, 2026
…ine) (#1802) Addresses @ffrujeri's review on #1572 (all 13 inline comments) plus the reward-profiling baseline. Targets `cmunley1/anyswe`. > Note: the swe_env `README.md` / `ng_test_all` CI-unblock landed separately via #1796 (merged). This PR is the review-comment fixes + docs + gold baseline on top of that. ## Review-comment fixes (@ffrujeri) | # | Comment | Fix | |---|---|---| | 1 | truncation mask never fires | inner agents (hermes/claude_code) now emit top-level `status="incomplete"` on internal max-turns/context/timeout; anyswe masks `resolved AND incomplete` (verified the status round-trips through `response.json`). openclaw exposes no internal signal → documented. | | 2 | openclaw config not runnable | select the apptainer provider for its `.sif` formatter; drop dead `apptainer_memory_limit_mb`/`skip_eval` fields. | | 3 | docker timeout gaps | configurable default `exec` timeout + bounded `close`/`cp` so a hung in-container command can't block a rollout. | | 4 | reward-profiling evidence | gold-patch baseline in the swe_env README (below). | | 5 | `_setup_params` assumes str `instance_dict` | accept str **or** dict (mirror `_build_swetask`). | | 6 | r2egym docstring | missing `eval_script` is unmasked reward-0, not an eval-error mask. | | 7 | missing docker binary | wrap `FileNotFoundError` → clear `SandboxCreateError`. | | 8 | unquoted shell interpolation | `shlex.quote` dataset values in `bash -c` (nv_internal/harness/swe_bench_ext). | | 9 | rc 125/126/127 infra-classify | narrow to **rc 125** only (126/127 are legit user-command codes). | | 10 | swe_rebench inflates on empty-required | empty-required guard (consistent with `compute_resolved`). | | 11 | unmasked infra failure on docker grade | mask `SandboxCreateError`/image-pull (`error_kind="sandbox"`). | | 12 | per-framework parsers untested | add `test_parsing_frameworks.py` (the 7 reviewer cases). | | 13 | docs discoverability | add a fern SWE-Environment page. | ## Gold-patch baseline (review #4) — at parity with the nested reference A full 500-instance SWE-bench Verified **gold-patch census on docker** resolves **493/500** (`patch_exists` 500/500, 0 infra errors), matching the apptainer/`.sif` *nested* reference **492/500** to within environment noise; empty patch **0/500**. (docker and apptainer both use the host-side flat grader here, so they're 1-1; the `.sif` figure is swebench's nested `run_evaluation`.) The census surfaced + fixed two real flat↔nested **reconstruction** gaps (445 → 486 → 493), verified with **0 regressions**: - **`PYTEST_ADDOPTS=-rA`**: swebench 4.1.0's eval for some families (sphinx via `tox`, several sklearn) runs pytest without `-rA`, so passing tests print only as dots and the host-side parser saw zero passes → ~45 unresolved even for gold (445→486). - **drop `GIT_CONFIG_GLOBAL=/dev/null`**: older images' git can't parse `/dev/null`, so the eval's `git checkout` + test-patch `git apply` failed → required tests "absent" (486→493). The remaining 7 misses are a small symmetric difference with `.sif` (4 shared genuine env-flaky: astropy-7606/8707/8872, django-10097; 3 docker-only sphinx instance-specific quirks; docker also resolves 4 that `.sif` misses). ## Validation - 258 unit tests pass (swe_env + docker provider + anyswe + claude_code/openclaw); ruff clean; `fern check` clean. - The `#1` truncation logic was adversarially verified end-to-end (top-level `status` survives the container→host round-trip; the mask is live). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Codex <codex@openai.com> Co-authored-by: Codex <codex@openai.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bxyu-nvidia
pushed a commit
that referenced
this pull request
Jul 31, 2026
…esource requests/limits (#2212) ## Summary Hardens the OpenSandbox provider and the `mini_swe_agent_2` eval path so that large sandboxed agent evals (SWE-bench Verified scale: hundreds of concurrent rollouts against a Kubernetes-backed OpenSandbox deployment) run end-to-end reliably: every finished rollout is delivered and recorded, and sandbox-infrastructure noise is bounded instead of silently zeroing rewards. Diagnosed on multi-node runs at concurrency 300–1500; the same failure signatures appear at concurrency 8, just less often. ### 1. `Server disconnected without sending a response` The SDK's default httpx pool keeps idle connections for 30s (`opensandbox.config.connection.with_transport_if_missing`), but the OpenSandbox server's uvicorn keep-alive reaper closes idle sockets after ~5s. Agent workloads idle between sandbox commands (model think time), so commands routinely reuse a socket the server already closed — surfacing as `httpx.RemoteProtocolError` → `SandboxInternalException`, which permanently kills the rollout. **Fix:** the provider injects a transport whose `keepalive_expiry` sits *below* the server's keep-alive timeout (default 3s, `connection.keepalive_expiry_s`): - `connection.transport_backend: httpx` (default) — stock `httpx.AsyncHTTPTransport(limits=..., retries=connect_retries)`. No new required dependencies. - `connection.transport_backend: aiohttp` (opt-in) — aiohttp pool under the SDK's httpx surface via [`httpx-aiohttp`](https://github.com/karpetrosyan/httpx-aiohttp); falls back to the httpx transport with a warning when absent. - `keepalive_expiry_s: null` disables injection entirely (SDK default transport). ### 2. 502 `could not connect to the backend sandbox endpoint='<podIP>:44772'` on first command With `create.skip_health_check: true`, `Sandbox.create` returns before the pod's exec daemon is listening; the first command races pod startup and the server proxy 502s. Under a create burst this killed 7–12 rollouts per run. **Fix:** `create.skip_health_check: false` (create waits for readiness, bounded by the spec's `ready_timeout_s`; `create.timeout_s` kept above the ready timeout). **`command_retries` stays at 0.** Retrying a command the server may have already started would execute it twice, and agent commands are frequently mutating. The keepalive bound removes the stale-connection failures retries were compensating for. Raise it only for idempotent workloads. ### 3. Separate resource requests and limits `Sandbox.create` accepts distinct `resource` (limits) and `resource_requests` maps; a lone `resource` map is applied by the server as requests=limits. This exposes the requests side via `sandbox_spec.provider_options.resource_requests` (same keys as `SandboxSpec.resources`). Motivation: SWE-bench test suites OOM-killed sandbox pods at 2Gi, but raising a single combined map to 8Gi would 4× the cluster reservation; with the split, limits rise while requests stay small and pods pack densely. Requires `opensandbox>=0.1.15` (lower bound raised here). ### 4. Sandbox teardown off the rollout's critical path `env.cleanup()` ran in the rollout Ray task's `finally` block, so a finished result only became fetchable after the sandbox DELETE returned — and a teardown failure re-raised *over* the successful eval, degrading it to a reward-0 error row. Teardown is also effectively one-shot (`Sandbox.stop` latches `_closed` before dispatch), so a single failed DELETE leaked the sandbox while still costing the rollout. **Fix:** cleanup runs on a best-effort daemon thread; results return immediately, teardown errors are logged rather than raised, and orphans remain covered by the provider's sandbox TTL. ### 5. High-concurrency rollout delivery (`mini_swe_agent_2`) Two coupled fixes so every finished rollout is delivered at high concurrency (note: at very high concurrency the single-process policy-model proxy remains a separate bottleneck — connection-refused storms confirmed by a controlled A/B — with its scaling fix tracked separately): - **await the Ray ObjectRef** instead of `asyncio.to_thread(ray.get, ...)`: the default executor caps at `min(32, cpu+4)` threads, each pinned for a full rollout, so delivery stalls at ~32 concurrent rollouts and finished tasks queue behind blocked `ray.get` calls. - **bounded litellm retries (`num_retries=5`, config-overridable)** — no retry means one transient LLM-call failure kills a whole rollout; unbounded retries make failures look like hangs — and **`num_cpus=0.25`** on the rollout Ray task so concurrency is not capped at cluster core count (rollouts are I/O-bound). ### 6. `datasets` declared in the base agent config The struct-mode config merge rejects keys absent from the base config, so a benchmark config using `_inherit_from` could not add its dataset list (`ConfigKeyError: Key 'datasets' is not in struct`). Declared empty in the base server config, matching how other agents (e.g. `swe_agents`) expose it. ## Validation - Requests/limits split (paired multi-node runs, identical except the variable under test): infra-failed rollouts 41/316 (**13.0%**) at 2Gi requests=limits → 19/288 (**6.6%**) at 8Gi limits / 2Gi requests; pass@1 58.0% → **67.1%**. - Keepalive bound + health-checked create: zero `Server disconnected` events and zero create-race 502s across all subsequent runs (previously 2–12 per run). - Full stack, end-to-end from this branch: single-pass SWE-bench Verified (500 instances, concurrency 500+) delivered **>98% of rollouts** with ~1.6% residual sandbox-infra failures (server-side proxy 502s on established sandboxes, tracked separately) and zero client-transport failures. Before the delivery fixes (#4/#5), an identically shaped run lost the majority of *completed* evaluations at the wall — hundreds of finished evals, only tens recorded. ## Relationship to #2020 Complementary, no overlap; #2020 (job attribution metadata) has since merged and this branch is updated on top of it. Field-validated together — the attribution labels are what make post-cancellation sandbox garbage collection safely scoped to a single job. ## Testing - `tests/unit_tests/test_opensandbox_provider.py`: transport-backend coverage (httpx default with keepalive expiry, custom pool settings, fallback when `httpx_aiohttp` is unavailable, `null` disables injection), aiohttp opt-in (importorskip-guarded), and requests/limits plumbing tests. 16 passed with and without `httpx-aiohttp`. - `responses_api_agents/mini_swe_agent_2/tests/test_app.py`: updated for the awaited ObjectRef (awaitable `FakeObjectRef`); assertions strengthened to check the Ray call's params. - CI green except a pre-existing `tau2` failure on `main` (reproduces on unrelated PRs). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Hemil Desai <hemild@nvidia.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
OlegSudakov
pushed a commit
to OlegSudakov/Gym
that referenced
this pull request
Aug 7, 2026
…esource requests/limits (NVIDIA-NeMo#2212) ## Summary Hardens the OpenSandbox provider and the `mini_swe_agent_2` eval path so that large sandboxed agent evals (SWE-bench Verified scale: hundreds of concurrent rollouts against a Kubernetes-backed OpenSandbox deployment) run end-to-end reliably: every finished rollout is delivered and recorded, and sandbox-infrastructure noise is bounded instead of silently zeroing rewards. Diagnosed on multi-node runs at concurrency 300–1500; the same failure signatures appear at concurrency 8, just less often. ### 1. `Server disconnected without sending a response` The SDK's default httpx pool keeps idle connections for 30s (`opensandbox.config.connection.with_transport_if_missing`), but the OpenSandbox server's uvicorn keep-alive reaper closes idle sockets after ~5s. Agent workloads idle between sandbox commands (model think time), so commands routinely reuse a socket the server already closed — surfacing as `httpx.RemoteProtocolError` → `SandboxInternalException`, which permanently kills the rollout. **Fix:** the provider injects a transport whose `keepalive_expiry` sits *below* the server's keep-alive timeout (default 3s, `connection.keepalive_expiry_s`): - `connection.transport_backend: httpx` (default) — stock `httpx.AsyncHTTPTransport(limits=..., retries=connect_retries)`. No new required dependencies. - `connection.transport_backend: aiohttp` (opt-in) — aiohttp pool under the SDK's httpx surface via [`httpx-aiohttp`](https://github.com/karpetrosyan/httpx-aiohttp); falls back to the httpx transport with a warning when absent. - `keepalive_expiry_s: null` disables injection entirely (SDK default transport). ### 2. 502 `could not connect to the backend sandbox endpoint='<podIP>:44772'` on first command With `create.skip_health_check: true`, `Sandbox.create` returns before the pod's exec daemon is listening; the first command races pod startup and the server proxy 502s. Under a create burst this killed 7–12 rollouts per run. **Fix:** `create.skip_health_check: false` (create waits for readiness, bounded by the spec's `ready_timeout_s`; `create.timeout_s` kept above the ready timeout). **`command_retries` stays at 0.** Retrying a command the server may have already started would execute it twice, and agent commands are frequently mutating. The keepalive bound removes the stale-connection failures retries were compensating for. Raise it only for idempotent workloads. ### 3. Separate resource requests and limits `Sandbox.create` accepts distinct `resource` (limits) and `resource_requests` maps; a lone `resource` map is applied by the server as requests=limits. This exposes the requests side via `sandbox_spec.provider_options.resource_requests` (same keys as `SandboxSpec.resources`). Motivation: SWE-bench test suites OOM-killed sandbox pods at 2Gi, but raising a single combined map to 8Gi would 4× the cluster reservation; with the split, limits rise while requests stay small and pods pack densely. Requires `opensandbox>=0.1.15` (lower bound raised here). ### 4. Sandbox teardown off the rollout's critical path `env.cleanup()` ran in the rollout Ray task's `finally` block, so a finished result only became fetchable after the sandbox DELETE returned — and a teardown failure re-raised *over* the successful eval, degrading it to a reward-0 error row. Teardown is also effectively one-shot (`Sandbox.stop` latches `_closed` before dispatch), so a single failed DELETE leaked the sandbox while still costing the rollout. **Fix:** cleanup runs on a best-effort daemon thread; results return immediately, teardown errors are logged rather than raised, and orphans remain covered by the provider's sandbox TTL. ### 5. High-concurrency rollout delivery (`mini_swe_agent_2`) Two coupled fixes so every finished rollout is delivered at high concurrency (note: at very high concurrency the single-process policy-model proxy remains a separate bottleneck — connection-refused storms confirmed by a controlled A/B — with its scaling fix tracked separately): - **await the Ray ObjectRef** instead of `asyncio.to_thread(ray.get, ...)`: the default executor caps at `min(32, cpu+4)` threads, each pinned for a full rollout, so delivery stalls at ~32 concurrent rollouts and finished tasks queue behind blocked `ray.get` calls. - **bounded litellm retries (`num_retries=5`, config-overridable)** — no retry means one transient LLM-call failure kills a whole rollout; unbounded retries make failures look like hangs — and **`num_cpus=0.25`** on the rollout Ray task so concurrency is not capped at cluster core count (rollouts are I/O-bound). ### 6. `datasets` declared in the base agent config The struct-mode config merge rejects keys absent from the base config, so a benchmark config using `_inherit_from` could not add its dataset list (`ConfigKeyError: Key 'datasets' is not in struct`). Declared empty in the base server config, matching how other agents (e.g. `swe_agents`) expose it. ## Validation - Requests/limits split (paired multi-node runs, identical except the variable under test): infra-failed rollouts 41/316 (**13.0%**) at 2Gi requests=limits → 19/288 (**6.6%**) at 8Gi limits / 2Gi requests; pass@1 58.0% → **67.1%**. - Keepalive bound + health-checked create: zero `Server disconnected` events and zero create-race 502s across all subsequent runs (previously 2–12 per run). - Full stack, end-to-end from this branch: single-pass SWE-bench Verified (500 instances, concurrency 500+) delivered **>98% of rollouts** with ~1.6% residual sandbox-infra failures (server-side proxy 502s on established sandboxes, tracked separately) and zero client-transport failures. Before the delivery fixes (NVIDIA-NeMo#4/NVIDIA-NeMo#5), an identically shaped run lost the majority of *completed* evaluations at the wall — hundreds of finished evals, only tens recorded. ## Relationship to NVIDIA-NeMo#2020 Complementary, no overlap; NVIDIA-NeMo#2020 (job attribution metadata) has since merged and this branch is updated on top of it. Field-validated together — the attribution labels are what make post-cancellation sandbox garbage collection safely scoped to a single job. ## Testing - `tests/unit_tests/test_opensandbox_provider.py`: transport-backend coverage (httpx default with keepalive expiry, custom pool settings, fallback when `httpx_aiohttp` is unavailable, `null` disables injection), aiohttp opt-in (importorskip-guarded), and requests/limits plumbing tests. 16 passed with and without `httpx-aiohttp`. - `responses_api_agents/mini_swe_agent_2/tests/test_app.py`: updated for the awaited ObjectRef (awaitable `FakeObjectRef`); assertions strengthened to check the Ray call's params. - CI green except a pre-existing `tau2` failure on `main` (reproduces on unrelated PRs). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Hemil Desai <hemild@nvidia.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.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.
No description provided.