Skip building an observation nothing below will read - #724
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 566fc5da7b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
A rig runs its control loop far faster than it re-queries a policy, and two layers were doing their most expensive work on every tick regardless. `TemporalStack` stacked a whole history window per call, and a codec resized every frame of it, and the scheduling layer below then answered `None` without looking at either. Measured on a real rollout: the window is stacked 57 times per chunk for 1886 ms of CPU, on the thread that drives the arm, in a chunk period of 3394 ms. 41 of those samples run while the arm is playing, with no round trip open at all. `Session.reads_observation(time_ns)` says whether a call would read what it is given. A scheduling layer answers False while its own chunk plays; a remote session answers False while a round trip is in flight. The call happens either way — a session waiting on a function in flight is answered by being called, not by the observation it carries — so a layer told no hands on what it already has. The default is True and a delegating session keeps it, because the answer is about the `__call__` that gives it: a recording tap wraps another session and still reads every observation, and a forwarded no would have let the layers above hand it something other than what went to the server. The two layers that forward are the two that genuinely pass the observation through. It reads the caller's clock rather than the observation, because a served pipeline runs these layers over observations the harness never stamped. That clock runs at or past the observation's, so the gate can only answer True where the call then answers None: a window built and unused, never one skipped and wanted. Ticket: Positronic-Robotics/internal#1168 #refs
`ChunkedSchedule` judged expiry at the observation's own instant in the call and at the caller's clock in the gate. Where the two straddle the chunk's end — an observation ahead of the clock, which the schedule's own test permits — the gate declined and the call went through, so the layer above handed the inner session a raw, unstacked observation. The query now takes what the call takes, and the schedule makes the same comparison in both. A test pins the case where the two readings differ. Ticket: none - a review finding on an unmerged branch
50320b9 to
baf7416
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baf741665d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| reads = self._inner.reads_observation(obs, time_ns) | ||
| encoded = self._codec.encode(obs) if reads else obs | ||
| action = self._inner(encoded, time_ns) |
There was a problem hiding this comment.
Rule hidden-dependency violated:
_CodecSession queries the inner session with raw obs, but when the answer is true it calls that session with codec.encode(obs). For any codec that transforms a field consulted by an inner gate, the gate therefore decides for a different observation than __call__ receives, contrary to the new Session contract; this silently depends on every inner gate ignoring every encoded field. Query with the value that will actually be passed, or redesign the gate so its decision inputs are explicit and invariant across codecs.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
1 participant
What
Session.reads_observation(obs, time_ns)says whether the call it precedes would read the observation it is given. A layer about to build an expensive one asks first, and hands on what it already has when the answer is no.ChunkedScheduleanswers False while its own chunk plays;RemoteSessionanswers False while a round trip is in flight; the default is True.Two layers act on it:
TemporalStackskips sampling its window, and a codec skips encoding. Both still call inward — a session waiting on a function in flight is answered by being called, not by the observation it carries.Why
A rig runs its control loop far faster than it re-queries a policy, so both layers were doing their most expensive work on every tick and the scheduling layer below was answering
Nonewithout looking at either.Measured on a real rollout (
blind_20260908-135658, 663 chunks, from this repo's own spans): the history window is stacked 57 times per chunk for 1886 ms of CPU — on the thread that drives the arm — inside a chunk period of 3394 ms. 41 of those 57 samples run while the arm is playing, with no round trip open at all. The resize is 21 calls and 1261 ms, of which 17 fall inside the wait.The two costs scale differently, which decides what this is worth. The resize scales with the round trip, so a faster server removes most of it anyway. The stack does not: it is a function of tick count alone and stays whatever the wire does. It is the floor a chunk period cannot go below, and it is the reason for the change.
Three decisions worth arguing with:
__call__does.ChunkedSchedulemakes one comparison,_obs_time(obs) < self._trajectory_end, in both places. Judging the gate on the caller's clock instead reads cheaper and is wrong wherever the two readings straddle the chunk's end: the gate declines, the call goes through, and the layer above hands the inner session a raw window. Nothing but the observation's own instant makes the two agree. A session that needs no field of it —RemoteSession, a codec — takes the argument and ignores it.__call__that gives it. A recording tap wraps another session and still reads every observation; a forwarded no would have let the layers above hand it something other than what went to the server, and nothing would have raised — the recording code accepts both stacked and unstacked frames. That failure is pinned by a test that fails against the forwarding version. Only the two layers that genuinely pass the observation through forward the question.reads_observationand once in__call__. One predicate would have to answer before the call, which is the whole point of the query.test_the_gate_agrees_with_the_callandtest_the_gate_reads_the_observation_instant_the_call_readshold the two together, the second with the observation ahead of the caller's clock.Verification
uv run pytest positronic pimm— 1488 passed, 8 skipped.pre-commit runclean on every touched file.test_a_delegating_session_answers_for_itselfwas watched failing against the forwarding default and passing after it.Refs
Positronic-Robotics/internal#1168.