diff --git a/SPEC.md b/SPEC.md index ac6ba0c..751ea6f 100644 --- a/SPEC.md +++ b/SPEC.md @@ -398,10 +398,30 @@ episode ended by the environment's own rules, `truncated` means it was cut short (time limit, external stop). A chunk is also flushed early when either flag is set, so a terminal transition is never buried inside a chunk. -`info` is a free-form map. When present, its values are expected to be -batched to `m` along a leading axis, and the server slices them per -environment; a value that is not `m`-shaped is passed through to every -environment unchanged. `{}` is valid and is what the reference clients send. +`info` is a free-form map. `{}` is valid and is what both reference clients +send. Anything else needs care, and the rule is narrower than it looks. + +> **Gap - a non-empty `info` can close the connection.** The server unbatches +> it by looking for the first value that is an ndarray and taking `m` from +> that array's leading axis, then slicing every ndarray value by it; a +> scalar or string sitting alongside is broadcast to all `m` environments, +> which is the documented behaviour. But if *no* value is an ndarray, the +> map is returned whole as a single per-environment entry regardless of `m` +> - `{"task": "pick"}` with `m` = 2 yields one entry, and so does a +> correctly batched msgpack *list* of length `m`, because a list is not an +> ndarray. The handler then asserts that the per-environment count equals +> the observation count, and an `AssertionError` there becomes a close with +> **1011** and the reason `Internal server error.` (the +> `assert len(info_list) == len(next_obs_list) or len(info_list) == 0` in +> `websocket_agent_server.py`'s connection handler, and `unbatch_aggregate` +> in `plugrl-server`'s `common/data_utils.py`). +> +> So a non-empty `info` is safe only when at least one of its values is an +> ndarray of length `m`, or when `m` is 1, where the mismatch cannot arise. +> The previous wording of this paragraph - that a value which is not +> `m`-shaped is passed through to every environment unchanged - is true only +> in the first of those cases. This has not bitten anyone because the +> reference clients send `{}`. #### Terminal observations @@ -532,6 +552,19 @@ environment it has no step state for. That condition has exactly one cause - the connection was replaced mid-run - and storing the transition silently puts a hole in the training data that nothing downstream can detect. +> **Gap - the reference client breaks this on one path.** +> `plugrl-env-client` run with `--reconnect-on-server-stop` resends the +> `feedback` it was holding instead of dropping it, when the close it hit was +> the server's `plugrl-server-stop`. In `websocket_env_client_agent.py`, +> `feedback()`'s `SERVER_STOP_REASON` branch `continue`s its retry loop, +> which opens a new connection, reads fresh metadata, and re-sends the same +> payload - the exact resend the historical note below says this section +> exists to prevent. Every other close path in that method returns and drops +> the transition, and `tests/test_reconnect_drops_feedback.py` covers three +> of them (a keepalive timeout, a plain 1000 close, a resync) but not this +> one. It is the only path in the reference client that violates the **MUST** +> above. + > **Where reconnects come from.** Nothing in this protocol causes them and > nothing in it can prevent them: a suspended laptop, a flaky link, an > operator restarting the server. The rule above is written in terms of the @@ -574,16 +607,38 @@ A client conforms to version 1 if it: `feedback` for an `action` that arrived on an earlier connection; - [ ] treats a text frame as a fatal error. -`examples/conformance_server.py` checks every clause above that is visible -from the server's side of the wire, and reports what it cannot enforce as a -note rather than a failure. Both reference clients pass it with one note: -they send `text` as a msgpack string array rather than a ` **Correction, 2026-09-11.** This paragraph used to say version 1 "has no +> version field on the wire" and attributed to section 5.1 a remark that the +> metadata message was the obvious place for one. Both halves went stale on +> 2026-09-10, when the metadata message gained contents: `protocol_version` +> has been in it on every connection since, in `plugrl-server` and in +> `examples/conformance_server.py` alike, and section 5.1 lists it as a key +> the server sends rather than as a suggestion. What survives is the absence +> of negotiation, which is what the sentence was reaching for. Changing any of the four `message_type` strings, the two close reasons, the ndarray key names, the action array's axis order, or the meaning of -`rewards` is a **breaking** change. `tests/test_wire_format.py` and -`tests/test_spec_conformance.py` exist so that such a change fails a test -rather than a deployment. +`rewards` is a **breaking** change. + +The first three are pinned in this repository, and a change to them fails a +test rather than a deployment: `tests/test_wire_format.py` holds the four +`message_type` strings and the two close reasons, +`tests/test_spec_conformance.py` the four ndarray key names. The last two +are not, and cannot be - this is the codec repository, and the axis order +and the chunk-sum rule are properties of how the two sides behave, not of +what the packer emits. Transposing the action chunk or redefining `rewards` +as the last step's reward passes every test here. They are covered instead +by `plugrl-env-client`'s +`tests/test_protocol_alternation.py::TestChunkSemantics`, which is the same +file section 8 points at. diff --git a/examples/README.md b/examples/README.md index b62f592..bcb57ab 100644 --- a/examples/README.md +++ b/examples/README.md @@ -16,11 +16,21 @@ broke. `plugrl-server` is deliberately forgiving — it validates the trade for a training run and the wrong one for someone bringing up a client in a new language. +The two Python files here need `websockets>=13`, which is **not** a +dependency of `plugrl-protocol` - installing the package does not get you +one. `conformance_server.py` imports `websockets.asyncio.server` and +`raw_client.py` imports `websockets.sync.client`; the `asyncio` layout +arrived in 13.0, so websockets 12 fails on the import. Ask for it +explicitly, from this directory: + ```bash -python conformance_server.py --port 8000 --steps 20 & +uv run --with 'websockets>=13' conformance_server.py --port 8000 --steps 20 & ./plugrl_client 127.0.0.1 8000 20 ``` +`pip install 'websockets>=13'` and then `python conformance_server.py ...` +works just as well. The C++ client needs nothing. + It exits non-zero on a violation, so it can sit in a CI job. Both clients here pass it with one note: they send `text` as a msgpack string array rather than the `=13' raw_client.py --host 127.0.0.1 --port 8000 --steps 120 ``` ## What the protocol asks of a client