Conversation
Moves the trainer-side rollout API out of the package __init__ and into `openenv.core.harness.rollout`, leaving __init__ as a re-export shim. No behavior change: every name previously importable from `openenv.core.harness` still is, and is the same object. The module was ~730 lines living directly in __init__ with a docstring noting it sat outside the stable surface "while RFC 005 is still under review". Splitting it now makes room for the RFC 005 turn-based agentic harness layer to land in sibling modules instead of growing the __init__ further. Also re-exports the private `_resolve_env_reward`, which tests/scripts/test_browsergym_harness_eval_examples.py imports from the package root, and points `collect.py` at `.rollout` directly rather than importing from its own package. Consumers left untouched and verified: `openenv collect`, pi_env, opencode_env, browsergym_env, reasoning_gym_env, openspiel_env. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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. |
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
APPROVE at 0fb24bef
Pure mechanical RFC 005 stack 1/4 split. Verified: rollout.py matches the prior __init__.py body; __all__ re-exports are intact (including private _resolve_env_reward); external from openenv.core.harness import … sites keep working. Backcompat tests 4/4 green locally; lint clean on changed files.
Wheel cargo: yes (src/openenv/core/harness/). Zero behavior change — safe foundational split for stacks 2–4; fine for 0.6.0 if you want the package layout landed early.
No Tier 1 / alignment blockers. LGTM.
Sent by Cursor Automation: Release
There was a problem hiding this comment.
APPROVE at e3a69713 (merge-main only)
Rebased onto main after #1178. Feature tip unchanged from prior APPROVE at 0fb24bef (RFC 005 harness package split; behavior-preserving relocation). 0.6.0 package cargo if merged — consider cut churn; refresh #1211 after green exact-head CI.
Still needs exact-head repository CI / maintainer merge.
Sent by Cursor Automation: Release
There was a problem hiding this comment.
Merge-main re-APPROVE at ae6f13f5
Ben merged main into this fork tip. Feature diff unchanged: mechanical harness package split (rollout.py / collect.py + re-exports) with backcompat tests.
Fork PR: repository CI still needs Approve-and-run. OpenEnv wheel cargo if merged before Thursday — would require refreshing draft #1211 + TestPyPI. Safe to defer past 0.6.0.
Sent by Cursor Automation: Release
There was a problem hiding this comment.
Re-APPROVE at 260015b9 (merge-main only)
Ben merged main (26c9465e / #1179) into this fork PR. Stable feature patch-id is unchanged vs prior approved tip ae6f13f5 (d125511b…); harness package-split blobs identical.
Prior technical approval stands. Still needs maintainer Approve and run. Package cargo if merged before Thursday — would require refreshing draft #1211 / TestPyPI again. Prefer hold through 0.6.0.
Sent by Cursor Automation: Release
This branch has not been deployed
2 participants
Stack for RFC 005: Agentic Harness Integration — this is 1 of 4. Each PR is one commit; later PRs target
main(cross-fork PRs cannot chain bases), so review them in order and read the top commit of each.HarnessEnvironment+ subprocess + tool bridge/harnessroute + mode wiringWhat
Moves the trainer-side rollout API out of the package
__init__and intoopenenv.core.harness.rollout, leaving__init__as a re-export shim.No behavior change. Every name previously importable from
openenv.core.harnessstill is, and is the identical object.Why now
openenv/core/harness/__init__.pyis ~730 lines of implementation living directly in the package__init__, with a docstring saying it sits outside the stable surface "while RFC 005 is still under review". The rest of RFC 005 adds a second, differently-shaped layer to the same package. Splitting first means that layer lands in sibling modules instead of growing the__init__to ~2000 lines, and it keeps the refactor reviewable on its own instead of buried inside a feature diff.Two details worth a look
_resolve_env_rewardis re-exported too, with anoqaand a comment. It is private, buttests/scripts/test_browsergym_harness_eval_examples.pyimports it from the package root. I found this by running the consumer tests, not by reading — worth knowing it is load-bearing.collect.pynow imports from.rolloutrather thanfrom . import ..., removing a self-referential package import.Verification
tests/core/test_harness_rollout_backcompat.pyis added as the contract: every name in the old__all__resolves from the package root andisthe same object as onrollout.All in-repo consumers pass unchanged —
openenv collect,pi_env,opencode_env,browsergym_env,reasoning_gym_env,openspiel_env:(
test_harness_runtime,test_harness_collect,test_opencode_factory_lifecycle,test_openspiel_collect,test_browsergym_harness,test_browsergym_harness_eval_examples, plus the new back-compat file.) Lint clean.Question for reviewers
Longer term the naming is muddled:
HarnessAdapterhere drives rollouts, while RFC 005's adapter adapts an external process, and both want the plain name. A rationalization would beRolloutDriver/MCPRolloutDriver/BlackBoxRolloutDriver/RolloutResult/RolloutLimitsfor this layer, freeingHarnessAdapterfor the RFC 005 meaning. Not done here — it would break the "no behavior change" property of this PR — but this is the natural moment to decide, before the names calcify. Happy to do it as a follow-up with deprecation aliases.Note
Low Risk
Mechanical move with explicit back-compat tests; no rollout or reward logic changes.
Overview
Refactors
openenv.core.harnessso trainer rollout code lives inrollout.py, with the package root acting only as re-exports.The ~800 lines that implemented
ResourceSession,MCPHarnessAdapter,build_harness_rollout_func, and related types move unchanged intoopenenv.core.harness.rollout.__init__.pyshrinks to documentation plus the same public (and_resolve_env_reward) names re-exported fromrollout, so existingfrom openenv.core.harness import ...call sites keep identical objects.collect.pynow imports rollout symbols from.rolloutinstead of the package__init__, avoiding a circular self-import through the shim.tests/core/test_harness_rollout_backcompat.pylocks the contract:rollout.__all__, package-root__all__, identity of re-exports, and that collect CLI modules still import.Reviewed by Cursor Bugbot for commit aa5f4d9. Bugbot is set up for automated code reviews on this repo. Configure here.