Add AsyncGRPO Harbor example: any harness, any sandbox, any Harbor dataset, served through OpenEnv - #6947
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}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 025f330. Configure here.
| 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.
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
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
Example-only addition with no library or training-core changes; risk is limited to users running the script against external OpenEnv/vLLM/sandbox services.
Overview
Adds
examples/async_grpo_harbor/async_grpo_harbor.py, a standalone recipe for training AsyncGRPO on Harbor task suites through an OpenEnv Harbor server—without new TRL APIs.The script wires
HarborSessionFactory(OpenEnv) toHarnessRolloutWorkerwithharness_adapter=Noneso loop-owning agents (defaultmini-swe-agenton E2B) run in sandboxes while model traffic is proxied to the same vLLM used for weight sync. Training usesharbor_reward(verifier correctness plus gated tool-efficiency),has_tool_callto keep action turns, and CLI knobs for harness/sandbox, concurrency, staleness, agent timeouts/step limits, and optional--reward-keyfor dict rewards.A PEP 723 header pulls
openenv-harbor-envfrom OpenEnv; the docstring documents the two-terminal vLLM + train flow and operational defaults (e.g. SLURM-safe@pathtask indices, Trackio run naming).Reviewed by Cursor Bugbot for commit 437e7be. Bugbot is set up for automated code reviews on this repo. Configure here.