fix(core): serve the environment's own State subclass on /schema and /state - #1174
Conversation
…/state /schema returned the base State model's JSON schema and /state was registered with response_model=State, so every field declared on an environment's State subclass was missing from the published schema and stripped from the response body. The WebSocket state frame calls model_dump() on the live environment and keeps those fields, so the two transports disagreed about the same object. Thread an optional state_cls through HTTPEnvServer, create_fastapi_app, create_app and create_web_interface_app, and use it for the /state response model and the state entry of /schema. It defaults to State and is the last parameter on the public factories, so no existing call site changes behaviour or shifts a positional argument. Fixes huggingface#1155
|
@burtenshaw the workflows here are sitting at Maintainer edits are enabled on this branch, it is current with |
|
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. |
|
thanks! |
There was a problem hiding this comment.
Release-manager triage at head 66ccdb83. The bug is real and I confirmed it on main: http_server.py:1421 declares response_model=State and http_server.py:1480 publishes State.model_json_schema(), so any field an environment adds on its own State subclass is both absent from /schema and stripped out of the /state body — while the WebSocket state frame calls model_dump() on the live environment and keeps those fields. Two transports disagreeing about the same object is worth fixing, the approach (thread an optional state_cls, default State) matches what #1155 proposed, and putting the parameter last so no existing positional argument shifts is the right call.
I am not merging this, and the reason is not the code. It adds a parameter to create_app and create_web_interface_app, which are public core factories, so it needs a maintainer decision rather than a release-manager merge. Your own instinct in the description is the right one: additive-with-a-default is normally not RFC territory, but the signature is public surface and that call is not mine to make. Two questions for whoever takes it:
- Is
state_clsthe interface you want long-term, or shouldEnvironment[ActT, ObsT, StateT]become the single source of truth? You note only five env packages parameterize that generic today, which is a fair argument for the explicit parameter now — but it is worth deciding whether the parameter is the destination or a bridge, because removing it later is a breaking change. - The 28 environments that declare a
Statesubclass each still need a one-line change to actually benefit. Shipping the core fix without them means/schemastays wrong for all of them, so the follow-up should be tracked rather than implied.
The practical gate: repository CI has never run here — only Bugbot has, and the branch is behind main. It needs a maintainer to approve the workflow run, and a refresh onto current main (4a1fa2a8). Not in the 0.5.0 candidate (#1190) and not holding the release.
Sent by Cursor Automation: Release


Summary
/schemaand/statewere wired to the baseStatemodel, so every field an environment declared on its ownStatesubclass was missing from the published schema and stripped from the response body. The WebSocketstateframe callsmodel_dump()on the live session environment and keeps those fields, so the two transports disagreed about the same object. This threads an optionalstate_clsthrough the app factories and uses it for the/stateresponse model and thestateentry of/schema.Fixes #1155.
Type of Change
Alignment Checklist
.claude/docs/PRINCIPLES.mdand this PR aligns with our principles.claude/docs/INVARIANTS.mdand no invariants are violatedOn the last box:
uvis not on my shellPATH, so I ran the checks in a Python 3.11 virtualenv with the test dependencies rather than through.claude/hooks/lint.sh. Details in the test plan.RFC Status
Test Plan
New file
tests/core/test_state_schema_subclass.py, eight cases across two groups: an environment that declaresEchoStategetscounterandhistoryin both/schemaand/state, including throughcreate_appwith the web interface enabled or disabled. Action and observation schemas are unaffected, and an app that declares nothing keeps the exact base-model behaviour it has today.Before adding the two
create_appmode cases, reverting the source files and rerunning the original six tests gave 3 errors, 1 failure and 2 passes. The errors are the fixtures rejecting the unknownstate_clsargument, the failure is the missingstate_clsattribute, and the 2 that still pass are the default-path tests, which is the intended result: they exist to catch a regression in the untouched path, so they should pass both before and after.tests/core: 621 passed, 1 skipped.usort check,ruff format --check,ruff checkon the three changed files: clean.test_grid_world.pyandtest_julia_env.py; no changed file is among them.PYTHONPATH=src:envs python -m pytest.Two notes for review:
state_clsis the last parameter oncreate_appandcreate_web_interface_apprather than sitting next toaction_clsandobservation_cls, so no existing positional argument shifts. I checked everycreate_app,create_fastapi_appandcreate_web_interface_appcall insrc/andenvs/with an AST scan: none passes more than three positional arguments today, but the ordering keeps out-of-tree callers safe too.Deriving the class from
Environment[ActT, ObsT, StateT]would avoid the parameter entirely, but only five environment packages currently parameterize that generic (maze_env,tbench2_env,pelican_svg_env,thinkingbox_env, andsophistry_bench_sprint_env); the last uses the baseState. Some other environments annotate thestateproperty as returning the baseState, so generic inference would miss most of the affected environments.The 28 environments that declare a
Statesubclass each need a one-line change to pass it. I left that out to keep this diff to the core fix, and I am happy to add it here or in a follow-up, whichever you prefer.Claude Code Review
Checked against
PRINCIPLES.mdandINVARIANTS.md: no Tier 1 issues in this diff, no principle conflicts, and no RFC conflicts. The change stays inside the HTTP transport surface and does not touch the agent and orchestration boundary or client and server separation.Note
Low Risk
Additive API defaulting to base State; behavior unchanged for existing callers until environments pass state_cls.
Overview
Fixes a transport mismatch where HTTP
/schemaand/statealways used the baseStatemodel, so subclass fields were dropped from the published schema and JSON body even though WebSocketstateframes kept them.Adds an optional
state_clsparameter (defaultState) onHTTPEnvServer,create_app,create_fastapi_app, andcreate_web_interface_app, and wires it into the/stateOpenAPI response model and thestatesection of/schema. Callers that pass their environment’sStatesubclass get those fields published and serialized; omittingstate_clspreserves prior behavior.New regression tests in
tests/core/test_state_schema_subclass.pycover declared vs defaultstate_cls, includingcreate_appwith the web interface on or off.Reviewed by Cursor Bugbot for commit 0cef418. Bugbot is set up for automated code reviews on this repo. Configure here.