Repository navigation
docs: Environment Builder client uses .sync() handle (from #1129) - #1166
Conversation
… handle (#1118) `from_docker_image()` / `from_env()` return a lazy bootstrap handle (`_BootstrapResult`) that must be awaited or chained with `.sync()`; it is not a context manager. The "Use Your Environment" example in the Environment Builder guide predates the async client migration and used the handle directly in a `with` block, and also referenced a `MyEnv.from_hub()` method that does not exist on `EnvClient`. - Use `.sync()` on all constructor lines and in the manual-cleanup block - Replace `from_hub()` with `from_env(...).sync()`, including the Quick Reference Card row at the top of the page - Explain the lazy handle and show the async equivalent - Apply the same `.sync()` fix to the Task API guide example - Sync the generate-openenv-env skill's reference copy of this page so the environment generator stops reproducing the stale API - Add unit tests that exercise the documented sync and async bootstrap/context-manager patterns through the real dispatch path Fixes #1118
burtenshaw
added
documentation
Cursor
Co-authored-by: benjamin.burtenshaw <benjamin.burtenshaw@huggingface.co>
|
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. |
Co-authored-by: benjamin.burtenshaw <benjamin.burtenshaw@huggingface.co>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Alignment Review Report
Scope: docs + tests only — 3 Markdown files (docs/source/getting_started/environment-builder.md, docs/source/guides/task-api.md, .claude/skills/generate-openenv-env/references/openenv-docs-environment-builder.md) and 1 test file (tests/test_core/test_generic_client.py). No src/ changes. Base 81ef28d is an ancestor of head, so the 3-dot diff equals the net change (nothing already-on-main).
Automated Checks
- Lint: PASS (for this PR). The only changed
.py(tests/test_core/test_generic_client.py) passesruff format --check,ruff check, andusort checkstandalone. CI's lint job onlyruff formatssrc/ tests/and fails only on.pydiffs, so the Markdown edits aren't lint-gated.bash .claude/hooks/lint.shexits 1, but every flagged file is pre-existingenvs/**drift unrelated to this PR (the hook also reverts only.py, leaving ~30README.mddirtied — a known hook quirk; restored the tree afterward). - Debug code: CLEAN (for this PR).
check-debug.shscanssrc/only, which this PR doesn't touch.
Verification (I checked every claim against the code and ran the examples)
EnvClient.from_env()exists (env_client.py:814); with the defaultuse_docker=Trueit pullsregistry.hf.space/…and runs it viaLocalDockerProvider→ "run the image of a Hugging Face Space locally" is accurate.- The old
from_hubexample was actually broken:EnvClienthas nofrom_hub(onlyAutoEnv/AutoActiondo), soMyEnv.from_hub(...)would raiseAttributeError. Replacing it withfrom_envis a genuine correctness fix. - Bootstrap-handle behavior is accurate:
from_docker_image()/from_env()return a lazy_BootstrapResult;.sync()→ connectedSyncEnvClient,await→ connected async client. Empirically confirmed on Python 3.12 thatwith handle:raises exactlyTypeError: '_BootstrapResult' object does not support the context manager protocol. - Async example is correct —
reset()/step()/state()dispatch to awaitables on the async client (await client.state()✓). - New cross-reference
../guides/async-syncresolves (docs/source/guides/async-sync.mdexists and is in_toctree.yml). - Both new tests pass; full
test_generic_client.py= 106 passed / 5 skipped.
Open RFCs Context
- RFC 002 – env-spec (In Review) — covers env/client usage examples (relevant, see flag below).
- RFC 011 – ARD catalog discovery (Draft) — covers Hub resolution /
from_env(adjacent, no conflict). - Others (000/001/003/005/008 In Review, 010 Draft, 004 no status header) don't touch this surface.
Tier 1: Fixes Required
- None. No lint/debug/type/import/security issues introduced; documented behavior is accurate and test-backed.
Tier 2: Alignment Discussion
Principle Conflicts
None identified. The examples reinforce the Gymnasium-style reset/step/state API and keep the orchestration client (WebSocket boundary) separate from the agent/MCP boundary — consistent with PRINCIPLES "Simple Gymnasium-style API" and the dual-API-boundary invariant.
RFC Conflicts
ALIGNMENT FLAG (non-blocking): shipped .sync()/bootstrap-handle client API diverges from RFC 002's example
- RFC at stake: RFC 002 (env-spec, In Review)
- The concern:
rfcs/002-env-spec.md:332still showsclient = CodingEnv.from_docker_image("coding-env:latest")used directly (then.step()/.close()) — the old eager-client pattern. This PR correctly documents the current shipped API wherefrom_docker_image()returns a lazy_BootstrapResultthat must be resolved with.sync()orawait. The docs are accurate to the code; the In-Review RFC example is now stale. Worth reconciling RFC 002's example so the RFC and shipped API agree (flag the RFC, not the docs). - Suggested reviewer: @Darktex (RFC 002 author; also @pankit-eng / @jspisak / @zkwentz)
RFC 011 (Draft) references AutoEnv.from_env("owner/repo") for Hub resolution; this PR's MyEnv.from_env(...) is the distinct EnvClient.from_env, so no conflict — the from_hub→from_env wording is consistent with that direction.
Minor non-blocking notes
- Residual broken
from_hubelsewhere:docs/source/tutorials/rl-training-2048.md:147and:469still callOpenSpielEnv.from_hub("openenv/openspiel-env")— the same nonexistent-from_hub-on-EnvClientpattern this PR fixes here. Out of this PR's stated scope, but a good follow-up to fully retire it (→from_env(...).sync()). - Version-specific error text: the documented
TypeError: … does not support the context manager protocolmatches Python 3.11/3.12 (CI-tested); on 3.10 (allowed byrequires-python >=3.10) CPython emits a different string. Illustrative-only; optional to caveat. - Test class name: the new async test lives in
TestSyncBootstrapConstructors— cosmetic naming mismatch.
Summary
- 0 mechanical issues to fix
- 0 principle conflicts (the change actually reinforces them)
- 1 non-blocking RFC-context flag (RFC 002's example is stale vs the shipped
.sync()API) + 3 minor non-blocking notes
Accurate, well-tested docs fix: it retires a broken from_hub example in favor of the real from_env/.sync() API and backs the documented sync + async flows with passing tests.
Sent by Cursor Automation: Pre-review
| # Create environment from Docker image (starts a container) | ||
| client = MyEnv.from_docker_image("my-env:latest").sync() | ||
| # Or, run the image of a Hugging Face Space locally | ||
| client = MyEnv.from_env("my-org/my-env").sync() |
There was a problem hiding this comment.
Good fix. EnvClient has no from_hub (it only exists on AutoEnv/AutoAction), so the previous MyEnv.from_hub(...) example would have raised AttributeError. from_env(...) (env_client.py:814) with the default use_docker=True pulls registry.hf.space/… and runs it locally via LocalDockerProvider, which matches the "run the image … locally" comment.
Follow-up (out of scope for this PR): docs/source/tutorials/rl-training-2048.md:147 and :469 still use the same broken OpenSpielEnv.from_hub(...) pattern — worth migrating to from_env(...) in a later pass.
| return a lazy bootstrap handle, and nothing starts until you resolve it: chain | ||
| `.sync()` for a synchronous client (as above), or `await` the handle from async | ||
| code. Using the handle directly in a `with` block raises | ||
| `TypeError: '_BootstrapResult' object does not support the context manager protocol`. |
There was a problem hiding this comment.
Verified end-to-end: on Python 3.12, with handle: raises exactly this TypeError, and .sync() / await both resolve as documented — the two new tests cover the sync + async flows and pass. One minor caveat: this exact wording is a Python 3.11+ message; on 3.10 (allowed by requires-python >=3.10) CPython emits a different string, so treat it as illustrative rather than a guaranteed literal.
Co-authored-by: benjamin.burtenshaw <benjamin.burtenshaw@huggingface.co>
Co-authored-by: benjamin.burtenshaw <benjamin.burtenshaw@huggingface.co>
Co-authored-by: benjamin.burtenshaw <benjamin.burtenshaw@huggingface.co>
Summary
Replacement for #1129 (fork workflows stuck at
action_required). Same change: fix Environment Builder / Task API client examples to use the.sync()bootstrap handle, with a focused generic-client regression.Please close #1129 once this lands.
Type of Change
Alignment Checklist
mainso required CI can run on-repoRFC Status
Test Plan
tests/test_core/test_generic_client.pycoverage for the bootstrap handleClaude Code Review
N/A — release automation replacement for Ben's watch/merge ask on #1129.
Note
Low Risk
Docs and test-only changes; no runtime or API behavior modified in this diff.
Overview
Documentation now matches the lazy bootstrap client API: examples chain
.sync()afterfrom_docker_image(),from_env(), andbase_urlconstruction, replacefrom_hubwithfrom_envfor Hugging Face Spaces, and add prose plus an async snippet explaining that factories return a handle (not a connected client) until resolved—using the handle inwithwithout.sync()/awaitraises aTypeError.The same pattern is applied in the Task API training loop (
from_docker_image(...).sync()), the quick-reference share step, and the Claude skill reference doc.Tests add two
GenericEnvClientregressions (issue #1118) that mirror the documented sync (with client: reset()) and async (await+async with) flows, asserting concreteStepResult, single WebSocket, and provider start/stop.Reviewed by Cursor Bugbot for commit 0dfebdf. Bugbot is set up for automated code reviews on this repo. Configure here.