Repository navigation
harbor_env: forward purpose, provider and eval_sampling from HarborSessionFactory - #1285
Conversation
…ssionFactory HarborSessionFactory is where the opencode_env deprecation (openenv#1276) sends people, but it can't pass `purpose`, `provider` or `eval_sampling`, even though `HarborEnv.run_rollout` and the server's `run_rollout` tool both take them. So a TRL loop-owning run can't ask for `purpose="train"` (which the provider qualification guide recommends), and can't point at a native Anthropic endpoint. The train case is the one that bites: a vLLM started without `--return-tokens-as-token-ids` still runs every rollout, just as eval, and `fetch_proxy_trace()` hands the trainer an empty list per rollout with a warning. With `purpose="train"` the server refuses the rollout before a sandbox is created ("training requires exact engine token capture"). Fix: add the three kwargs to the factory and session and pass them through. The factory checks them at construction the same way it already checks `sampling`, mirroring `SessionRegistry.create`, so a bad value fails once instead of on every rollout. Defaults are unchanged, and the client already leaves defaults off the wire for older servers. Tests: 6 new cases in tests/envs/test_harbor_session_factory.py fail on main (TypeError on the new kwargs) and pass with this change. tests/envs/test_harbor_*.py + test_capture_*.py: 606 passed, 3 skipped. usort, ruff format and ruff check clean on the touched files. Prepared with AI assistance (Claude Code); I reviewed the change and ran the tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ssionFactory `HarborSessionFactory` (where openenv#1276 points opencode_env users) can't pass `purpose`, `provider` or `eval_sampling`, although `HarborEnv.run_rollout` and the server's `run_rollout` tool accept all three. So a TRL loop-owning run can't ask for `purpose="train"`, which the provider qualification guide recommends. That matters most for training: a vLLM started without `--return-tokens-as-token-ids` still runs every rollout, as eval, and `fetch_proxy_trace()` hands the trainer an empty list per rollout with only a warning. With `purpose="train"` the server refuses the rollout before creating a sandbox. The factory now takes the three kwargs and passes them through. It validates them at construction, like `sampling`, so a bad value fails once rather than on every rollout. Defaults are unchanged. Tests: new cases in `tests/envs/test_harbor_session_factory.py` fail on main (TypeError on the new kwargs) and pass with this change; the harbor and capture suites pass (606 passed, 3 skipped). ruff and usort clean on the touched files. Prepared with AI assistance (Claude Code); I reviewed the change and ran the tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@sergiopaniego @adithya-s-k re-checked this against current main: it merges cleanly, and tests/envs/test_harbor_.py plus test_capture_.py pass on the merged tree (566 passed vs 559 on main; the 7 extra are the new tests in test_harbor_session_factory.py). With opencode_env removed in #1346, this is how a TRL loop-owning run gets |
burtenshaw
left a comment
There was a problem hiding this comment.
Reviewed factory-to-session-to-rollout forwarding and validation against the existing server contract. Focused factory tests pass (34 passed, 1 skipped), with clean changed-file lint. No blocking findings.
|
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. |
HarborSessionFactory(where #1276 points opencode_env users) can't passpurpose,provideroreval_sampling, althoughHarborEnv.run_rolloutand the server'srun_rollouttool accept all three. So a TRL loop-owning run can't ask forpurpose="train", which the provider qualification guide recommends.That matters most for training: a vLLM started without
--return-tokens-as-token-idsstill runs every rollout, as eval, andfetch_proxy_trace()hands the trainer an empty list per rollout with only a warning. Withpurpose="train"the server refuses the rollout before creating a sandbox.The factory now takes the three kwargs and passes them through. It validates them at construction, like
sampling, so a bad value fails once rather than on every rollout. Defaults are unchanged.Tests: new cases in
tests/envs/test_harbor_session_factory.pyfail on main (TypeError on the new kwargs) and pass with this change; the harbor and capture suites pass (606 passed, 3 skipped). ruff and usort clean on the touched files.Prepared with AI assistance (Claude Code); I reviewed the change and ran the tests.
Note
Low Risk
Harness plumbing and constructor validation only; rollout semantics align with existing server APIs without touching auth or data paths.
Overview
HarborSessionFactoryandHarborSessionnow acceptprovider,purpose, andeval_samplingand pass them through toHarborEnv.run_rollout, matching what the harbor server already supports.Training loops (e.g. TRL loop-owning mode) can set
purpose="train"so rollouts fail fast when the engine cannot return token ids, instead of running as eval and yielding emptyfetch_proxy_trace()results.providerselects the LLM wire protocol (e.g. Anthropic), andeval_samplingconfigures eval-only temperature/top-p/top-k whenpurpose="eval".The factory validates
purposeand thesampling/eval_samplingcombination at construction time (same pattern as trainingsampling), so misconfiguration fails once rather than on every rollout. Defaults remainopenai,auto, and no eval sampling.Tests cover forwarding to
run_rolloutand the newValueErrorcases for invalid purpose or mixed train/eval sampling.Reviewed by Cursor Bugbot for commit 25a7a13. Bugbot is set up for automated code reviews on this repo. Configure here.