Play the policy's trajectory in the harness, one command per channel per round

.claude/skills/address-review/SKILL.md

@@ -111,8 +111,11 @@ For each open comment, decide and note severity if the bot tagged one (e.g. Code
   resolve only if you land a concrete change, else leave open.
 - **Decline** — wrong, not applicable, or contradicts a deliberate decision → reasoned
   reply, **leave open**.
-- **Defer** — valid but out of scope for this PR → reply (note where it's tracked),
-  **leave open**.
+- **Defer** — valid but out of scope for this PR → reply saying so, **leave open**, and name it in
+  the Step 6 report as wanting a tracking issue. **Never open an issue without asking first.** An
+  issue is filed under the user's own account, so it reads to their team as their judgement about
+  what matters; that call is theirs. Propose the title and the substance and wait for a yes. If an
+  existing issue already covers it, cite that instead — citing is not filing.
 - **Discuss** — the reviewer is asking a question or opening a design discussion, not
   requesting a change → answer it, **leave open** for them to respond.
 
@@ -122,8 +125,15 @@ Present the triage as a short numbered list: comment → verdict → planned fix
 declines without prompting. "Unclear" is a verdict you reach after the research above, never
 before it — a question you could have answered by reading the code costs the user a round trip
 and comes back as *go and find out*, which is the same work plus a delay. Pause and confirm
-only for what research cannot settle: a preference, a cost, a scope call, or a change whose
-blast radius is genuinely the user's to accept. The user may drop or override any item.
+only for what research cannot settle: a preference, a cost, or a change whose blast radius is
+genuinely the user's to accept. The user may drop or override any item.
+
+**Whether a finding belongs in this PR is your call.** "Fix it here or track it for later" is
+the verdict this skill exists to reach, and by the time you can pose it the research is done,
+so the human has nothing to add that you don't already have. Reach it, file the issue, reply,
+and put it in the Step 6 report where they can overrule it. Bring a deferral back only when
+leaving it undone changes what the PR ships — a gap in the behaviour the user asked for, not a
+follow-up carrying an issue number.
 
 When you do bring something back, explain it in full and in plain words: what the thing is and
 how it works, what the reviewer asked for, what you did or did not do and why, and what each
@@ -256,10 +266,62 @@ Summarize:
 - which threads you resolved (fixes only) vs left open (declines / defers / discussion),
 - any follow-ups the user should track.
 
+Then **offer** the walk below, in one line, and go to Step 7 without waiting:
+
+> 3 threads are open on my call: <title>, <title>, <title>. Want to go through them?
+
 A bot will re-review on push and may add comments. **Don't hand the watch back to the
 user** — go to Step 7, which watches for that re-review in the background and loops you
 through another pass automatically until the reviewer converges.
 
+## Walking your declines — only on an explicit go
+
+Every thread you declined or deferred is a decision the human has not seen. They are entitled to
+review each one, during the cycle or after it converges. **Offer it; never start it uninvited.** A
+proposal is one line (above). Presenting a thread before they say go is the failure this section
+exists to prevent — as is presenting all of them at once.
+
+On their go, **one thread per message, no section headings**, covering four things:
+
+1. what the thing is,
+2. what the reviewer wanted,
+3. why you declined it,
+4. the decision you need — bolded, on its own line, with the options named.
+
+**Simple beats short.** The failure mode is density, not length: four sentences carrying three ideas
+each are worse than twelve carrying one each. Unpack — one idea per sentence, plain words, and
+describe the thing before arguing about it. Length is the budget for being understood; spend it on
+unpacking and on nothing else.
+
+**A short code snippet is welcome** where code says it faster than prose — the branch that decides,
+the line that was removed. A few lines, and say what it shows. Long listings, file tours and full
+diffs are not.
+
+Then **stop**. Do not append the next thread, a summary, or what you plan to do after.
+
+Withheld until asked: the measurements, the alternative designs, the options table, the wider diff.
+Fine to *have* — `git show`, instrumentation, a longer argument — none of it goes in unasked.
+
+- **"too long" means cut it in half** — not rewrite it at the same length.
+- **"too complicated" / "I don't understand" means unpack it** — more sentences, fewer ideas per
+  sentence, and a snippet if one exists. Cutting further is the wrong reflex and makes it worse.
+- **A follow-up question gets that answer alone.** "why declined?" is not an invitation to re-present
+  the thread; "which three?" wants three names and nothing else.
+- Assume they have not read the code and will not. No "see the call site".
+
+Their verdicts are terse. What each one means:
+
+| They say | You do |
+|---|---|
+| "next" / "resolve, next" | reply, resolve, present the next thread — same shape, no re-asking |
+| "leave it" | reply that it is accepted as a known limit, resolve |
+| "fix it" | fix, run the gates, commit, push, reply with the SHA, resolve |
+| "file it" / "open a bug" | file the issue, reply linking it, resolve — this verdict is the only thing that authorizes filing one |
+| "why declined?" | the reasoning only |
+
+This is the one path on which a declined thread gets resolved: the human closed it, thread by thread.
+Absent that, Step 5 stands and it stays open.
+
 ## Step 7: Watch for convergence in the background (so the user doesn't have to)
 
 Two signals decide whether a push actually lands the PR, and **both are asynchronous**: CI

ARCHITECTURE.md

@@ -36,9 +36,9 @@ library must supply the tools that make late binding possible — codecs, per-tr
 projections over raw recordings.
 
 **Every decision lives with the party that has the information.** Only a driver knows its motion
-capabilities, so drivers plan through waypoints. Only a sensor knows its own cadence, so sensors
-run at their own rate instead of a rate the loop imposes. Only a policy knows what its model was
-trained on, so translation to model I/O ships with the policy.
+capabilities, so drivers own how they reach a commanded setpoint. Only a sensor knows its own
+cadence, so sensors run at their own rate instead of a rate the loop imposes. Only a policy knows
+what its model was trained on, so translation to model I/O ships with the policy.
 
 **Components are functions over flowing data.** A component sees nothing but its inputs and touches
 nothing but its outputs. Whatever varies enters as data — time is an observation field, hardware
@@ -116,21 +116,30 @@ control system its clock, and no component reads time at point of use. Trajector
 the same time frame the observations carry, so a virtual clock, a slowed sim, or a replayed episode
 changes nothing downstream.
 
-**Trajectory is the command.** Ownership puts execution with the driver: the policy emits a
-trajectory of waypoints with absolute timestamps, and the driver plays it at its own control rate,
-planning through the waypoints as well as it knows how. Signals are last-value-wins, so a new
-trajectory overwrites the current one — the previous command is merely context for the next.
-Continuous-update schemes (RTC, temporal ensembling) therefore need no special mechanism: they are
-wrappers that rewrite the command more often. An empty trajectory cancels the channel and the
-device holds.
+**The wrapper owns the plan, the harness plays it, the driver executes.** A policy speaks in
+trajectories — waypoints with absolute timestamps — because a model predicts a horizon, not an
+instant. But a trajectory on the wire makes every driver buffer the future, and makes the recording
+guess which prefix of that buffer actually ran. So the plan stops at the harness: a command channel
+carries the single command due at the moment it is emitted, the driver executes the latest one and
+holds otherwise, and emission time *is* execution time. Continuous-update schemes (RTC, temporal
+ensembling) therefore need no special mechanism: they are wrappers that hand back a new trajectory
+more often, and the harness keeps playing the old one until they do.
 
 **The harness stays thin.** It is the one layer standing between any policy and any embodiment, so
 anything it encodes about either side breaks the any-to-any goal. It assembles the observation
-dict, calls the session, demuxes the returned waypoints per command channel, and runs episode
-lifecycle — nothing else. Scheduling, blending, history stacking and error recovery live in the
-wrapper stack around the policy; a session returning `None` means "keep executing the current
+dict, calls the session, plays the returned trajectory one command per channel per round, and runs
+episode lifecycle — nothing else. Scheduling, blending, history stacking and error recovery live in
+the wrapper stack around the policy; a session returning `None` means "keep executing the current
 trajectory".
 
+**Inference cost is a fact of the trial, owned by the harness.** The trial context carries
+`inference_latency` — a constant charge in seconds, or the call's own wall duration — and the harness
+alone reads it: it withholds a returned trajectory, and the world clock, until the charge is paid,
+and the clock it hands the policy stack (`now`) reads the instant the in-flight call's output takes
+effect. An answer with no waypoints to place — a skip, or the empty trajectory that stops what is
+executing — lands at once. A scheduling wrapper stamps its chunk at `now()` and never learns the
+mode, so the charge is deterministic wherever a constant one is asked for.
+
 **Recordings are canonical; codecs bind the dialect late.** The dataset records every run in the
 canonical conventions (frames, key names, absolute time) — never in a model's dialect. Every
 model-facing view — action space, control frame, vendor format — is a codec's projection.

docs/connect-your-model.md

@@ -101,14 +101,15 @@ The client sends the full raw robot state as a dict. Keys are flat strings (the
 | `robot_state.ee_pose` | float32 | (7,) | End-effector pose: `x, y, z, qw, qx, qy, qz` (quaternion is **wxyz**, scalar first) |
 | `robot_state.q` | float32 | (7,) | Joint positions (radians) |
 | `robot_state.dq` | float32 | (7,) | Joint velocities (radians/s) |
+| `robot_state.fault` | bool | scalar | Whether the arm is faulted. A faulted arm sends no `robot_state.ee_pose`/`q`/`dq` at all — it has no sample to give — so a stack that reads them needs `StopOnFault` ahead of it |
 | `grip` | float32 | scalar | Gripper closure in `[0, 1]`: 0 = open, 1 = closed |
 | `image.<name>` | uint8 | (H, W, 3) | Camera RGB. Every eval target — PhAIL and each sim — sends `image.exterior` and `image.wrist`, whatever the underlying benchmark calls those cameras, so one codec reads them all; a target with more views adds its own names beside them (the MuJoCo sim adds `image.agent_view`) |
 | `obs_time_ns` | int | scalar | Harness-clock timestamp of this observation (ns) |
 | `wall_time_ns` | int | scalar | Wall-clock timestamp (ns) |
 | `task` | str | — | Language instruction for the episode |
 | `descriptor` | str | — | Embodiment the observation came from (e.g. `mujoco.franka`); empty string when unset. Lets a multi-embodiment policy adapt to the current robot |
 
-Your server receives every key each step. Use what your model needs and ignore the rest. Image stream names are configuration-driven, so key off the names your deployment uses rather than assuming fixed ones. The table above is a single-arm rig; a multi-arm one names its state and grip channels per arm.
+Your server receives every key each step, except that an arm with no sound sample to give — faulted, or mid-reset — omits its `robot_state.*` measurements and sends only `robot_state.fault`. The standard stack puts `StopOnFault` ahead of the model, which answers such a step itself and never forwards it; a stack without it reaches the model with those keys missing. Use what your model needs and ignore the rest. Image stream names are configuration-driven, so key off the names your deployment uses rather than assuming fixed ones. The table above is a single-arm rig; a multi-arm one names its state and grip channels per arm.
 
 ### Actions (server → client)
 
@@ -169,7 +170,7 @@ from positronic.drivers.roboarm import command
 from positronic.offboard import PolicyServer
 from positronic.policy import Policy, Session
 from positronic.policy.spec import PolicySource, remote
-from positronic.policy.wrappers import ChunkedSchedule
+from positronic.policy.wrappers import ChunkedSchedule, StopOnFault
 
 
 class MySession(Session):
@@ -200,20 +201,20 @@ class MyPolicy(Policy):
         return {'type': 'my_model'}
 
 
-pipeline = ChunkedSchedule() | remote | PolicySource(MyPolicy(load_my_model()))
+pipeline = StopOnFault() | ChunkedSchedule() | remote | PolicySource(MyPolicy(load_my_model()))
 PolicyServer(pipeline, host='0.0.0.0', port=8000).serve()
 ```
 
-The pipeline reads left to right: everything left of the `remote` marker is the client-side stack the server declares in its handshake (here the standard `ChunkedSchedule`); everything right of it runs on the server. `PolicySource` is the pipeline's terminal — a model source that serves one already-built policy.
+The pipeline reads left to right: everything left of the `remote` marker is the client-side stack the server declares in its handshake (here the standard `StopOnFault` and `ChunkedSchedule`); everything right of it runs on the server. `PolicySource` is the pipeline's terminal — a model source that serves one already-built policy.
 
-The left side is not optional: a pipeline with nothing there is refused when the server starts, and a rig refuses a handshake that declares nothing. It needs a scheduler in particular. Actions come back timestamped relative to their chunk, and `ChunkedSchedule` is what turns those into times on the rig's clock; a stack that leaves them relative — or anchors them twice — makes the harness reject the chunk at the first inference, since it schedules nothing more than `MAX_ACTION_SKEW_SEC` from now.
+The left side is not optional: a pipeline with nothing there is refused when the server starts, and a rig refuses a handshake that declares nothing. It needs a scheduler in particular, and `StopOnFault` outside that scheduler — a faulted arm is not tracking the plan it was given, so the wrapper answers the empty trajectory and the rig stops rather than resuming a chunk stamped before the fault. It is what makes the session above safe to write: a faulted observation arrives without `robot_state.ee_pose`/`q`/`dq`, so a session reading them by name needs the wrapper ahead of it. Actions come back timestamped relative to their chunk, and `ChunkedSchedule` is what turns those into times on the rig's clock; a stack that leaves them relative — or anchors them twice — makes the harness reject the chunk at the first inference, since it schedules nothing more than `MAX_ACTION_SKEW_SEC` from now.
 
 `new_session`'s `now` argument is the runtime clock that wrappers scheduling against live time read; a policy that does no scheduling of its own just accepts and ignores it (server-side it is `None`).
 
 If you put a `Codec` right of the marker (`ChunkedSchedule() | remote | codec | PolicySource(...)`), your session works entirely in *model space* — it receives encoded observations and returns model-native actions, and the codec handles the wire format. A codec that encodes images should also bound them on the rig, so full-resolution frames never cross the wire — that is what the built-in vendor pipelines do:
 
 ```python
-ChunkedSchedule() | RestrictImageSize() | remote | codec | source
+StopOnFault() | ChunkedSchedule() | RestrictImageSize() | remote | codec | source
 ```
 
 Give it the geometry your codec encodes to — `RestrictImageSize(224, 224)` for a 224x224 model — so a frame is shrunk once, on the rig. The default is a loose 640x640, for a codec that resizes to nothing in particular. Leaving it out costs bandwidth, not correctness.

pimm/core.py

@@ -11,7 +11,11 @@ class NoValueException(Exception):
     pass
 
 
-NODEFAULT = object()
+class _NoDefault:
+    """No default at all, which `T | None` cannot express: `None` is itself a value a receiver may default to."""
+
+
+NODEFAULT = _NoDefault()
 
 
 @dataclass
@@ -164,7 +168,7 @@ def emit(self, data: T, ts: int = -1):
 class ControlSystemReceiver(SignalReceiver[T]):
     """Receiver adaptor bound to a single upstream signal on behalf of a system."""
 
-    def __init__(self, owner: ControlSystem, default: T | None = NODEFAULT, maxsize: int | None = None):
+    def __init__(self, owner: ControlSystem, default: T | None | _NoDefault = NODEFAULT, maxsize: int | None = None):
         self._owner = owner
         self._default = default
         self._internal: SignalReceiver[T] | None = None
@@ -187,8 +191,8 @@ def read(self) -> Message[T] | None:
             value = self._internal.read()
             if value is not None:
                 return value
-        if self._default is not NODEFAULT:
-            # Always not-updated; the check above excludes the sentinel, which `T | None` cannot express.
+        if not isinstance(self._default, _NoDefault):
+            # Always not-updated; `None` reaches here as the value it was given, not as an absent default.
             return Message(cast(T, self._default), -1, False)
         return None
 
@@ -227,7 +231,9 @@ class ReceiverDict(dict[str, ControlSystemReceiver[U]]):
     Pass fake=True for all fake receivers, or fake={'key1', 'key2'} for specific keys.
     """
 
-    def __init__(self, owner: ControlSystem, *, default: U | None = NODEFAULT, fake: bool | Iterable[str] = False):
+    def __init__(
+        self, owner: ControlSystem, *, default: U | None | _NoDefault = NODEFAULT, fake: bool | Iterable[str] = False
+    ):
         super().__init__()
         self._owner = owner
         self._default = default