Add AsyncGRPO Harbor example: any harness, any sandbox, any Harbor dataset, served through OpenEnv - #6947
Add AsyncGRPO Harbor example: any harness, any sandbox, any Harbor dataset, served through OpenEnv#6947adithya-s-k wants to merge 12 commits into
Conversation
…hrough OpenEnv Trains against a Harbor task through mini-swe-agent running in an E2B sandbox. The agent owns its own loop; TRL stands up an endpoint, lets it drive, and reads back the captured token ids and logprobs. That is what makes an installed harness trainable without reimplementing it, and it is the difference from examples/grpo_harbor, which runs Harbor tasks against harnesses written inside TRL with TRL owning the loop. mini-swe-agent is the default on measured grounds rather than taste: across a 15-harness sweep on the same 50 tasks it was the most accurate and the most turn-efficient, its prompt re-render is byte-exact against the engine's prompt_token_ids, and it is the only harness that can express a step limit. The re-render matters because TRL rebuilds each prompt locally, and for three of twelve harnesses measured that drifts (claude-code +2 tokens, gemini-cli +2, kimi-cli -10 per tool call) -- invisible for eval, forking the trajectory every turn when training. The step limit is not a cost control. Every turn re-sends the whole conversation, so a rollout's packed length grows with the SQUARE of its turn count; unbounded 58-turn rollouts were enough to OOM the loss step on an 80 GiB card. The docstring states one caveat rather than hiding it: on this path HarnessRolloutOutcome carries a single verifier scalar, not the component dict, so a 'submission' term giving partial credit is unavailable and the reward is all-or-nothing. On a suite the model solves ~16% of the time that means most groups score identically and those steps teach nothing, so the example says to shape component rewards where the suite emits them and otherwise to pick tasks the model solves sometimes.
…d note The grpo_harbor reference would dangle once that example is deprecated, and a docstring should not point at something being removed. The reward paragraph is cut to what it is -- correctness plus a correctness-gated efficiency term, with --reward-key for suites that emit a dict -- rather than a discussion of what the single-scalar path cannot express.
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 025f3309ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| processing_class=tokenizer, | ||
| # Must match how the engine was served, or every prompt is re-rendered under a different template | ||
| # than the rollout was generated with — silent skew, not an error. | ||
| chat_template_kwargs={"enable_thinking": False}, |
There was a problem hiding this comment.
In the loop-owning mode selected here, HarnessRolloutWorker rebuilds prompts through openenv_harness._turns_from_trace, which calls tokenizer.apply_chat_template without the worker's chat_template_kwargs. Consequently this argument is a no-op: the documented vLLM command generates with enable_thinking=false, while the captured turns are locally re-rendered using the tokenizer default, silently pairing generated tokens and old logprobs with different prompt IDs. Pass these kwargs through the loop-owning trace reconstruction before relying on this setting.
Useful? React with 👍 / 👎.
| max_inflight_tasks=args.max_inflight, | ||
| vllm_server_url=args.vllm_url, | ||
| max_tokens=args.max_completion_length, | ||
| temperature=args.temperature, |
There was a problem hiding this comment.
When harness_adapter=None, the external agent submits its own requests and this worker never calls _generate_one_turn; the worker temperature is only included in HarnessRunLimits, which _run_session passes to the white-box branch. Thus --temperature does not control these rollouts, while AsyncGRPOTrainer still recomputes policy logprobs using that value. For any harness whose request temperature differs, the captured processed logprobs and trainer logprobs represent different distributions, corrupting the importance ratios even before weights become stale.
Useful? React with 👍 / 👎.
| # Trackio keys a run by name inside a project, so two relaunches of the same config land on top of | ||
| # each other and the earlier metrics read as part of the later run's history — worst exactly when | ||
| # relaunching after a crash. Stamping the name keeps them apart. | ||
| stamp = os.environ.get("SLURM_JOB_ID", "local") |
There was a problem hiding this comment.
For every non-SLURM invocation that omits --run-name, this always produces the same ...-local run name for a given configuration. That is exactly the relaunch case the preceding comment intends to prevent: Trackio histories are merged and the default output directory is reused after a local crash or repeated experiment. Use a per-invocation timestamp or unique identifier when SLURM_JOB_ID is absent.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| processing_class=tokenizer, | ||
| # Must match how the engine was served, or every prompt is re-rendered under a different template | ||
| # than the rollout was generated with — silent skew, not an error. | ||
| chat_template_kwargs={"enable_thinking": False}, |
There was a problem hiding this comment.
Chat template kwargs never applied
Medium Severity
chat_template_kwargs is passed into HarnessRolloutWorker as a load-bearing match against the served engine, but the loop-owning re-render in _turns_from_trace never forwards those kwargs to apply_chat_template. Prompts are rebuilt from tokenizer defaults instead, so any model whose default thinking mode differs from the vLLM serve flags silently skews every turn and forks the trajectory.
Reviewed by Cursor Bugbot for commit 025f330. Configure here.
Brings the whole stack up inside one container: openenv harbor serve on CPU, vllm serve on GPU 0, the trainer on GPU 1. Self-contained rather than importing its sibling, because hf jobs uv run uploads a single script. Everything lives in one job because AsyncGRPO syncs weights into vLLM over NCCL, which needs both on the same host's GPUs. That leaves only the OpenEnv server placeable, and keeping it here puts the proxy's hop to vLLM on localhost; hosting it on a Space via `openenv harbor push` works but adds a public hop to every model call. The tunnel is not a workaround for missing infrastructure. Jobs can publish a port at <job_id>--<port>.hf.jobs, but access needs an HF token, and the agent's Authorization header already carries its rollout session key -- that key IS how the proxy routes concurrent rollouts, so it cannot carry a second credential. An unauthenticated outbound tunnel needs no ingress at all. The readiness check verifies the tunnel SERVES THE PROXY, not that a port answers. A tunnel whose forwarding process dies keeps resolving and returns the provider's error page; agents then get HTML where an OpenAI endpoint should be, make zero model calls, and every rollout comes back unscorable while the server's own /health still reports healthy. The URL is only accepted after a request through it returns our health document. The parser was replayed over 32 real server logs and matches all 12 published URLs, gradio and cloudflare alike. The mounted bucket holds checkpoints and HF_HOME, and says so loudly when absent rather than losing them silently. Deliberately not the sandbox templates: those are keyed by image hash on the provider's side and already persist across jobs, which is the expensive warm step.
…itten
--split was required, so the reproduction command carried a placeholder nobody could paste. It now
defaults to a public Harbor suite (AdithyaSK/data_agent_rl_environment_train, verified public), which
means the whole thing is one copy-pasteable line with no <angle brackets> in it:
hf jobs uv run --flavor h200x2 --image huggingface/trl \
--secrets HF_TOKEN --secrets E2B_API_KEY \
https://raw.githubusercontent.com/.../async_grpo_harbor_hf_jobs.py
The docstring shows that form first and the bucket form second, since the bucket is optional -- without
it the job still trains, it just loses its checkpoints when the container goes away, which the script
already warns about at startup.
Also fixes a flag that could not do anything: --no-enable-thinking was declared with store_false over a
default of False, so it could only set what was already set. Thinking is off by default because the
trainer's chat_template_kwargs must agree with how the engine was served, and --enable-thinking is now
the switch that changes it.
Found by auditing the script against the Jobs environment rather than the cluster it was written on.
1. vllm was never declared. `uv run --script` builds an isolated environment, so the base image's vllm
is not guaranteed importable -- and the script's own PATH guard would then have blamed "the PEP 723
dependencies did not install", which would have been true but unhelpful.
2. No sandbox template warm. This is the one that would have looked like a harness bug. Harbor decides
whether to build from alias_exists(), which flips true when a build STARTS, so num_generations
rollouts racing their first visit to a task all see "exists" and fail against a half-built image with
404: tag 'default' does not exist. On a cold template with 8 concurrent generations that is the
common case. One serial `openenv harbor rollout` first makes the build serial and every later rollout
finds a finished image; the provider keys images by content hash, so later jobs pay nothing.
Non-fatal on failure: the warm rollout can fail for reasons that say nothing about training, and the
build it triggered still happened.
3. No GPU-count guard. On a one-GPU flavor the trainer was pointed at device 1 and would have failed
deep inside CUDA instead of at startup with a sentence naming --flavor h200x2.
4. The run name fell back to a constant ("local") when HF_JOB_ID was absent -- and that variable's name
is not something this script can verify. Trackio keys a run by name inside a project, so the constant
would fold separate jobs into one history, worst exactly when relaunching after a failure. It now
falls back to a timestamp.
sergiopaniego
left a comment
sergiopaniego
left a comment
There was a problem hiding this comment.
quick review, first pass, we need to add the example to docs/source/example_overview.md
Restructured to match the pattern the opencode recipe already uses for Hugging Face Jobs (described in sergiopaniego's TRL x OpenEnv x Harbor post, with the launcher published as a gist): a small launcher wraps the processes a Job cannot start on its own, and the training script is DOWNLOADED rather than duplicated. That last part is the reason for the change. The previous commit added a second 489-line file that restated the whole training path, so the two could drift and a reader had to diff them to see what was actually different. Now async_grpo_harbor.py is the single canonical script, byte-identical whether it runs locally or in a Job, and launcher.py holds only what is genuinely Jobs-specific. Ours needs three processes where the opencode launcher needs two, because the Harbor dataset and the capture proxy are served by openenv harbor serve. It also tunnels a different hop: opencode tunnels vLLM, since its in-sandbox proxy calls the engine directly, whereas here the sandboxed agent calls the capture proxy and the engine stays entirely private on localhost. --train-script-url exists because the equivalent opencode launcher still points at examples/scripts/openenv/opencode_hf_sandbox.py, a pre-reorg path that now 404s. A stale URL surfaces as a download failure minutes into a paid job, so this one is overridable and can be pointed at a branch before the example is merged. Carried over from the audit of the deleted file: vllm is declared, the sandbox template is warmed by one serial rollout before any group runs concurrently, there is a GPU-count guard, the tunnel readiness check verifies a request THROUGH the tunnel reaches the proxy, and the run name never falls back to a constant.
Verifying once at startup is not enough, and this is measured rather than defensive. Over 27 hours on our own cluster the published tunnel stopped serving the capture proxy 69 times -- roughly once every 24 minutes -- so a run of any length loses it mid-flight. When that happens the sandboxed agent gets the tunnel provider's error page instead of an OpenAI endpoint, makes zero model calls, and every rollout comes back unscorable, with nothing in the trainer's logs to say why. A daemon thread re-probes the published URL and restarts the Harbor server after two consecutive failures, so one flaky request cannot bounce a healthy server mid-step. The restart changes the published URL, which is fine: the trainer talks to the server over localhost and the server hands its current URL to each new sandbox, so only in-flight rollouts are lost. Which check does the work is worth recording. The failure signature originally debugged -- the provider's "no interface is running" placeholder -- was 2 of those 69. The other 137 probe failures were plain 502s. So the test is positive and generic: the URL must return OUR health document. Enumerating known failure modes would have caught almost none of them.
All four test suites on huggingface#6947 fail on one assertion, and it is not a code failure: tests/test_examples_index.py::test_examples_index_matches_folders AssertionError: Example folders missing from the Index table in example_overview.md: {'async_grpo_harbor'} `test_examples_index_matches_folders` requires every directory under `examples/` to have a matching row in the index, so adding the example without the row turns all of "latest", "dev", "minimum versions" and "without optional dependencies" red at once. One row fixes all four. Placed before `async_grpo_math` to keep the table alphabetical, matching the neighbouring `async_grpo_opencode` entry's format.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3ca92ca. Configure here.
| _children.remove(proc) | ||
| time.sleep(5) | ||
| try: | ||
| new_url = start_harbor_server(args, log) |
There was a problem hiding this comment.
Restart rebinds ports too early
Medium Severity
Tunnel recovery sends SIGTERM to the Harbor server, drops it from _children, sleeps a fixed 5 seconds, and starts a replacement on the same ports without waiting for the old process to exit. A slow shutdown leaves 8200/8300 bound, the republish times out, and later sandboxes keep hitting a dead tunnel.
Reviewed by Cursor Bugbot for commit 3ca92ca. Configure here.
| # 1. the Harbor dataset + the capture proxy, published for the sandboxed agent | ||
| server_log = logs / "openenv-server.log" | ||
| public_proxy = start_harbor_server(args, server_log) | ||
| threading.Thread(target=supervise_tunnel, args=(args, server_log, args.tunnel_check_s), daemon=True).start() |
There was a problem hiding this comment.
Zero interval never disables supervisor
Low Severity
--tunnel-check-s is documented as disabling the supervisor at 0, but the supervisor thread is always started. A zero interval then busy-loops probes and can restart a healthy Harbor server after two immediate failures.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3ca92ca. Configure here.
2 participants
Adds an AsyncGRPO example that trains against any Harbor task suite served through OpenEnv.
The shape is: pick a Harbor dataset, pick a sandbox, pick a harness, and train. All three are per-rollout choices against one long-lived OpenEnv server — so switching harness or sandbox is an argument, not a rebuild, and the same server serves training and evaluation at the same time.
This is the case #6018 explicitly left out. That PR supported Harbor's external agents only, because "RL needs the trainer to drive generation turn-by-turn and capture the policy's tokens/log-probs + env mask — which an opaque in-container agent can't expose." OpenEnv's capture proxy exposes exactly that, so installed agents that own their own loop become trainable without reimplementing them.
flowchart LR A["harness<br/>(any sandbox)"] -->|OpenAI-compatible calls| P["OpenEnv<br/>capture proxy"] P -->|forwards| V["vLLM"] P -.->|"token_ids + processed logprobs"| T["AsyncGRPOTrainer"] T -->|NCCL weight sync| V A -->|writes workspace| G["Harbor verifier"] G -.->|reward| TThe harness owns its loop; TRL never calls
step(). It stands up an endpoint, lets the agent drive, and reads back what happened. Because the agent's calls and the trainer's weight updates go to the same vLLM, rollouts stay on-policy — and OpenEnv decides the tier by probing that engine: token ids plus processed logprobs meantrain; anything less meanseval, and the session yields no trainable turns rather than rows of zeros.Nothing is added to TRL. Everything Harbor-specific lives in OpenEnv (
harbor_env.harness), so the example file is the whole integration.Usage
Defaults, and why they are the defaults
--harness mini-swe-agent --sandbox e2b. Any harness the server reports works, but two properties decide which one to train on, and they were measured across a 15-harness sweep on the same 50 tasks:prompt_token_ids. TRL rebuilds each prompt locally becauseTraceEntrycarries no prompt ids, and for three of twelve harnesses measured that drifts —claude-code+2 tokens,gemini-cli+2,kimi-cli−10 per tool call. Invisible for eval; forks the trajectory every turn when training.mini-swe-agentis the one harness that honours a limit.Depends on
huggingface/OpenEnv#1036, which adds
envs/harbor_envand the capture layer this example is built on. The PEP 723 header references it by git subdirectory, so the example is not installable until that lands.Note
Low Risk
Documentation and new example/launcher scripts only; no changes to TRL trainers or library APIs.
Overview
Adds
async_grpo_harbor, an example that trains withAsyncGRPOTraineron Harbor task suites via OpenEnv: a loop-owning coding agent (defaultmini-swe-agent) runs in a sandbox while the capture proxy records token ids and logprobs from the same vLLM the trainer updates over NCCL.The training script wires
HarborSessionFactory(OpenEnv) toHarnessRolloutWorker, defines a verifier-basedharbor_reward(correctness plus gated tool efficiency), and exposes CLI knobs for harness, sandbox, dataset split, concurrency, and agent limits—without changing TRL itself.A
launcher.pyfor Hugging Face Jobs startsopenenv harbor serve(public capture proxy tunnel), vLLM on one GPU, and downloads/runs the canonical training script on the other; it includes tunnel health checks with server restart, serial sandbox template warmup, and process-group cleanup.docs/source/example_overview.mdgains an index row for the new example.Reviewed by Cursor Bugbot for commit 3ca92ca. Bugbot is set up for automated code reviews on this repo. Configure here.