[RFC 005] 1/4: split openenv.core.harness into a package - #1097
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
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 module split with explicit back-compat tests; rollout logic is relocated unchanged and public import paths are preserved.
Overview
Refactors
openenv.core.harnessby moving the trainer-side rollout implementation out of the package__init__into a newrolloutmodule, with no intended behavior change.__init__.pyis now a thin re-export layer sofrom openenv.core.harness import ...still resolves the same public symbols (and the same objects) as before, including a back-compat re-export of private_resolve_env_rewardfor existing test/script imports.collect.pyis updated to import rollout types from.rolloutinstead of the package root, avoiding a circular self-import through__init__.Adds
tests/core/test_harness_rollout_backcompat.pyto lock the contract:rollout.__all__, package-root re-exports, and collect module import paths.Reviewed by Cursor Bugbot for commit 153ba87. Bugbot is set up for automated code reviews on this repo. Configure here.