Async grpo OpenEnv harness rollout#6420
Conversation
dabfebb to
1903378
Compare
|
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. |
|
Thanks I think it looks good overall. I didn't try myself, but from reading the edits, nothing much to say. Check the cursor review, it's likely the reason of the CI failure. Once the CI green I approve and we merge |
sergiopaniego
left a comment
There was a problem hiding this comment.
thanks!!
approving but with 2 ideas:
- we could add a short harness-rollout section to
openenv.mdin this PR: white-box vs loop-owning,--return-tokens-as-token-ids, reward viasession.verify()→harness_reward. - we should add an example script. I think this could be added once we add support for HF sandbox + harnesses. wdyt? it's in the roadmap here -> huggingface/OpenEnv#940
|
Ran this branch loop-owning with OpenCode on HF Jobs; two things surfaced (both line up with the Cursor comments above, now with a concrete repro): 1. Minimal guard: choices = last["response"].get("choices") or []
content = choices[0]["message"].get("content", "") if choices else ""(plus rebuilding the final assistant 2. Rollout child doesn't exit cleanly. Confirmed the |
|
Heads-up on another rough edge hit while training a loop-owning harness (Pi coding agent) on this branch, separate from the empty-
Repro-side workaround: coerce structured content to text before append, e.g. c = msg["content"]
t.append(c if isinstance(c, str) else "".join(
p.get("text", "") if isinstance(p, dict) else str(p) for p in c))Happy to open a small PR if useful. |
|
@sergiopaniego can you check #5309, would it solve the issue? I can revamp it you think it makes sense |
@sergiopaniego thanks for catching this, but |
| last = entries[-1] | ||
| choices = (last.get("response") or {}).get("choices") or [] | ||
| content = choices[0]["message"].get("content", "") if choices else "" | ||
| return list(last["request"].get("messages") or []) + [{"role": "assistant", "content": content}] |
There was a problem hiding this comment.
Structured content crashes logging
High Severity
Loop-owning completions keep agent message content as-is, including structured lists. With log_completions=True, print_prompt_completions_sample appends that value to rich Text and raises TypeError, which kills the score loop and stops the rollout worker.
Reviewed by Cursor Bugbot for commit 06b6083. Configure here.
|
In this commit : 06b6083 fixed @sergiopaniego you can rerun if possible :
OpenEnv Workarounds @burtenshaw @adithya-s-k @sergiopaniegoI had to work around some OpenEnv fundamental type /API limitation. I noted them as
So what I just pushed is
I also added this : cf #6349 @qgallouedec
|
qgallouedec
left a comment
There was a problem hiding this comment.
lgtm, just a bug (not sure) on group id
| def _tool_failure_count(entries: list[TraceEntry]) -> int: | ||
| """Best-effort tool-failure count across the real agent turns (`entries`). The agent (not TRL) executed the tools, | ||
| so we only see their free-text RESULTS, which come back as `role="tool"` messages in a later request; a failure can | ||
| only be inferred from error-looking result text. The last agent request holds the fullest message list; dedupe by | ||
| (name, content) so a result echoed across requests is not counted twice.""" | ||
| seen, failures = set(), 0 | ||
| for msg in (entries[-1]["request"] if entries else {}).get("messages") or []: | ||
| if msg.get("role") != "tool": | ||
| continue | ||
| key = (msg.get("name"), str(msg.get("content"))[:200]) | ||
| if key in seen: | ||
| continue | ||
| seen.add(key) | ||
| text = str(msg.get("content") or "").lower() | ||
| if any(w in text for w in ("error", "failed", "traceback", "exception")): | ||
| failures += 1 | ||
| return failures |
There was a problem hiding this comment.
quite fragile, consider dropping it.
if you want to keep it, let's keep in mind that we should re-visit later when openenv exposes real tool-result status
There was a problem hiding this comment.
100% agree, it was needed for tracking and reward, but this needs to be exposed from the trace directly
| return [] | ||
| last = entries[-1] | ||
| choices = (last.get("response") or {}).get("choices") or [] | ||
| content = choices[0]["message"].get("content", "") if choices else "" |
There was a problem hiding this comment.
Fragile final-choice message access
Medium Severity
Empty choices is handled, but a non-empty choice missing message still does choices[0]["message"] and raises. That happens after turns are already built, so the outer handler discards an otherwise trainable rollout as unscorable.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9015404. Configure here.
Co-authored-by: Quentin Gallouédec <45557362+qgallouedec@users.noreply.github.com>
| ) | ||
| completion = result.messages | ||
| tool_call_count = int(result.metrics.get("tool_calls", len(result.tool_trace))) | ||
| tool_failure_count = sum(1 for entry in result.tool_trace if entry.result.error is not None) |
There was a problem hiding this comment.
White-box hooks incomplete
Medium Severity
On the white-box path, train_turn_fn is never applied and tool_calls_by_name stays an empty dict. rollout_reward_fn therefore sees zero per-tool counts, and every sampled turn is trained even when the caller passed a turn filter.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 789acec. Configure here.
| `_start_proxy` exactly except for the port.""" | ||
|
|
||
| def _start_proxy(self, sandbox): | ||
| port = _free_port() |
There was a problem hiding this comment.
Proxy port race
Medium Severity
_free_port binds a port, closes the socket, then later starts the proxy on that port. Concurrent rollouts can each observe the same free port between release and bind, so proxy startup fails and those rollouts become unscorable.
Reviewed by Cursor Bugbot for commit 789acec. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
There are 7 total unresolved issues (including 5 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 444ca7d. Configure here.
| tokenize=True, | ||
| return_dict=False, | ||
| ) | ||
| turns.append(TurnRecord(prompt_ids, _trace_output_ids(entry), entry.get("per_token_logps") or [])) |
There was a problem hiding this comment.
Mismatched logprobs break sequences
Medium Severity
_turns_from_trace builds a TurnRecord from proxy completion_token_ids and per_token_logps without checking equal length. _SampleBuilder._append then extends tokens/mask by the id length and logprobs by the logprob list length, so a partial capture silently misaligns training tensors.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 444ca7d. Configure here.
| d = os.path.join(sdir, sub) | ||
| shutil.rmtree(d, ignore_errors=True) | ||
| os.makedirs(d, exist_ok=True) | ||
| return LocalSandboxHandle(sdir, home_alias=self._alias, base_env=envs, cleanup=True) |
There was a problem hiding this comment.
Hardlink sandboxes share install files
Medium Severity
create hardlink-clones the template with cp -al and only replaces a few subdirs. The .opencode install tree stays shared across sandboxes, so in-place writes (cache, autoupdate, or package mutation) can corrupt the template and concurrent rollouts despite the comment claiming isolation.
Reviewed by Cursor Bugbot for commit 444ca7d. Configure here.


WIP
Note
Medium Risk
Changes touch the experimental async GRPO rollout/scoring path and spawn external agent processes; default worker behavior stays backward compatible, but harness mode adds new failure modes (session create, trace parsing) that are mostly swallowed as unscorable rollouts.
Overview
AsyncGRPO can now train on rollouts produced by OpenEnv sessions (external agents that own their tool loop) instead of only TRL’s built-in vLLM tool loop.
A new
HarnessRolloutWorker/_HarnessRolloutLoopruns each rollout via aResourceSessionFactory: either white-box (OpenEnvHarnessAdapter+ TRL sampling per turn) or loop-owning (wait for the agent, read the in-sandbox proxy trace for token ids/logprobs, thensession.verify). Rewards can come from arollout_reward_fnonHarnessRolloutOutcomewithoutreward_funcs; hookstrain_turn_fn/agent_turn_fnfilter which trace turns become training rows.The default
AsyncRolloutWorkeris extended so_generate_onereturns a sixthrollout_reward(alwaysNonetoday),RolloutGroupcarriesrollout_rewards, and_score_groupsums them when_provides_rollout_rewardis set. The child process accepts a pluggable_loop_cls.RolloutWorkerProtocolallowsmultiprocessing.Queueas well asqueue.Queue.examples/scripts/openenv/opencode.pydemonstrates end-to-end GRPO on DeepCoder with the real opencode CLI, a local subprocess sandbox (remapped/home/user), per-session proxy ports, held-out stdin/stdout verification, and custom reward/turn filtering.Tests are updated for the new return tuple and
rollout_rewardson groups.Reviewed by Cursor Bugbot for commit 444ca7d. Bugbot is set up for automated code reviews on this repo. Configure here.