Conversation
huggingface#1174 added state_cls to the app factories, but nothing passes it, so the 28 envs that declare a State subclass still publish a state schema of episode_id and step_count and still strip every subclass field from the /state body. Their WebSocket state frame returns the full object, so the two transports disagree. Pass each env's State subclass at its app factory call site, and add a static regression test that walks envs/ with ast, so a new env cannot quietly skip it. Follow-up to huggingface#1174.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4b39a3d. Configure here.
repl_env and textarena_env pick between two create_app calls by probing inspect.signature(create_app). The first branch is the one that runs against a current openenv; the second exists for a release predating gradio_builder. The previous commit wired only the second, so /schema and /state still served the base State model for these two envs. Pass state_cls in the live branch, guarded by the same signature probe the file already uses for its other newer kwargs, and drop it from the legacy branch, where an openenv old enough to take that path would reject the argument. The test inspected only the last factory call in a module, which is why it did not catch this. It now checks every call, resolves values passed through a splatted kwargs dict, and exempts calls inside a signature-guarded fallback.
|
Both findings were right, thanks. Fixed in The test had the same blind spot, which is why it stayed green through the first commit. It now checks every factory call in a module rather than the last one, resolves values passed through a splatted kwargs dict, and exempts only calls inside a signature-guarded fallback. Removing the guard from |
1 participant
Summary
#1174 added
state_clsto the app factories, but nothing passes it yet, so the fix is not reaching users. All 28 envs that declare aStatesubclass still publish a state schema ofepisode_idandstep_count, and still strip every subclass field from the/statebody, while their WebSocketstateframe returns the full object. This wires each env to its own class and adds a static regression test so a new env cannot forget.Follow-up to #1174, which closed #1155.
Type of Change
Alignment Checklist
.claude/docs/PRINCIPLES.mdand this PR aligns with our principles.claude/docs/INVARIANTS.mdand no invariants are violateduvis not on my shellPATH, so I ran the checks in a Python 3.12 virtualenv with the test dependencies rather than through.claude/hooks/lint.sh.RFC Status
No API changes. Every edit is an argument at an existing call site.
Test Plan
New file
tests/envs/test_env_state_cls_wiring.py: for every env that declares aStatesubclass, assert its app factory passesstate_cls, that the value is a subclass declared in that env, and that the name is imported inserver/app.py. It walks the source withastrather than importing, so playwright, carla, dm_control and the rest of the optional-dependency tail stay out of it and the check runs everywhere. A guard case fails if the discovery ever matches nothing.envs/changes and rerunning it fails 28 of the 29, one per env, which is the point of the test.tests/core: 641 passed, 2 skipped.tests/envs/test_manifest_app_targets.py: 78 passed.ruff format --checkand compiles.usortreports the same 10 pre-existingenvs/**/app.pyfiles before and after this change, so nothing here adds to that drift. I left those alone rather than reformatting files this PR only touches by one line.Notes for review
Three cases needed care rather than a blanket edit:
thinkingbox_envbuildsHTTPEnvServerdirectly and registers routes in production mode, where/stateis not registered at all. It still getsstate_clsbecause/schemais served in production and was publishing the base model.coding_tools_env,finqa_env,jupyter_env,opencode_env,pi_env,terminus_env) takeCallToolObservationfromopenenv.core, so theirStateimport comes from the env's ownmodelsmodule and follows each file's existing in-repo and standalone import pattern.wildfire_envbuilds its state withWildfireState.model_construct, andopencode_env,pi_envandjupyter_envbind the class to an instance attribute first. All four still return their declared subclass, which is what the endpoints now serve.Envs that do not declare a
Statesubclass are untouched and keep the base model.Note
Low Risk
Mechanical wiring at existing factory call sites with no new API; main impact is richer, consistent HTTP state/schema responses aligned with WebSocket behavior.
Overview
Fixes a gap after
state_clswas added to OpenEnv app factories: envs that define aStatesubclass were still serving the baseStateon/schemaand/state, so HTTP clients only sawepisode_idandstep_countwhile WebSocketstateframes returned the full object.Every affected
server/app.pynow imports that env’s*Stateand passesstate_cls=...intocreate_app(orHTTPEnvServerforthinkingbox_env).repl_envandtextarena_envonly setstate_clswheninspect.signature(create_app)supports it, so older OpenEnv installs still boot.Adds
tests/envs/test_env_state_cls_wiring.py, which usesastto assert each env with a declaredStatesubclass wires the matching class through its app factory—without importing optional env dependencies.Reviewed by Cursor Bugbot for commit 1a84a1b. Bugbot is set up for automated code reviews on this repo. Configure here.