Skip to content

fix(policies/lerobot_async): rename_map renames the camera it declares - #3496

Merged
cagataycali merged 3 commits into
strands-labs:mainfrom
shipitfast:fix/732-lerobot-async-rename-map-client-side
Sep 17, 2026
Merged

cagataycali merged 3 commits into
strands-labs:mainfrom
shipitfast:fix/732-lerobot-async-rename-map-client-side

Conversation

@shipitfast

@shipitfast shipitfast commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

What

lerobot_async applies a rename_map camera entry on the client, so the handshake and the raw observation carry the checkpoint's own camera name.

Why

lerobot's PolicyServer resizes every declared observation.images.<key> by policy_image_features[key] in prepare_raw_observation, before the RenameObservationsProcessorStep runs (0.6.1 and main). A camera declared under the robot's name is a KeyError there, and the client raised "server returned no actions". On main:

assert "laptop" in raw and "front" not in raw
E  AssertionError: assert ('laptop' in {'j0': 0.0, 'j1': 0.0, 'front': array(...)})

Against a real policy_server on CPU (smolvla_base, front -> camera1), the same call now returns a 10-action chunk.

Tests

tests/policies/lerobot_async/test_rename_map_renames_the_camera_the_server_resizes.py; ruff, mypy clean on touched files.


Review rounds

Round Concern Fix commit Pin test
R1 Args: entry + docs table still said "forwarded to the server" for image keys (now client-side); test missing importorskip("lerobot") f21c572 existing test + importorskip gate

The client declared observation.images.<robot cam> and forwarded rename_map to the server, whose prepare_raw_observation resizes each declared image by the checkpoint's image features before the rename step, so a renamed camera was a KeyError and every observation returned no actions.
The client now applies an image rename itself: the handshake and the raw observation carry the model's camera name.

@cagataycali cagataycali left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is sound and I verified the mechanism against lerobot 0.6.1 rather than the description: PolicyServer sizes every declared image by policy_image_features (policy_server.py:345) ahead of the rename step, and RenameObservationsProcessorStep.observation passes an absent key through (rename_processor.py:45-48), so the image entry that is still forwarded to the server is a harmless no-op once the client has renamed. Lazy from lerobot... inside the method is this module's existing shape (lines 348, 373, 437), so no concern there.

Two things, both one commit and a plain push:

  1. The rename_map contract is now described in two places and they disagree - see the thread on _camera_items.
  2. The new test module needs the importorskip its sibling carries - see the thread on the test.

The fork runs were held at action_required; released this cycle, so CI is running on this head.

Comment thread strands_robots/policies/lerobot_async/policy.py
…ient-side image rename; test gates on importorskip (addresses thread policy.py:411 + test:17)

@cagataycali cagataycali left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R1 landed in f21c572 and answers both threads exactly: the Args: entry and the docs table row now say which side applies which kind of rename_map entry and why (the server resizes by policy_image_features before its rename step), and the test module gates on pytest.importorskip("lerobot"). Both threads resolved. Superseding the request for changes.

The fork runs on this head were held at action_required and are released. Note for the merge: this head was pushed under the reviewing account, so require_last_push_approval needs a second approver; this approval clears the review decision but cannot satisfy that rule on its own.

@cagataycali
cagataycali enabled auto-merge (squash) September 11, 2026 17:21

@cagataycali cagataycali left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the mechanism against lerobot 0.6.2 rather than taking the description on trust, and it holds as stated:

  • async_inference/policy_server.py:340-351 runs raw_observation_to_observation (step 1) before self.preprocessor(observation) (step 2), and the rename step lives in the preprocessor (policy_server.py:162). Resize precedes rename.
  • async_inference/helpers.py:166 subscripts policy_image_features[key] directly on the declared key, so a camera declared under the robot's name is a KeyError there, not a rename.
  • RenameObservationsProcessorStep.observation iterates the observation's own keys, so the camera entry still forwarded to the server is a harmless no-op once the client has applied it.

Both cells fail on the merge base 0759ececd with the pre-fix keys (observation.images.front declared, front on the wire); tests/policies/lerobot_async/ is 28 passed on this head. ruff check, ruff format --check and mypy strands_robots tests tests_integ clean (Success, 1958 files).

The Args: entry and the docs row now say which side applies which kind of entry and why, and the module-level importorskip("lerobot") matches the sibling roundtrip test. Both threads addressed.

@cagataycali

Copy link
Copy Markdown
Member

Green and ready: rollup SUCCESS on f21c572, MERGEABLE, both review threads resolved, auto-merge armed (squash). My two approvals cannot clear it — main sets require_extra_approval_for_unattributed_changes on top of one required approval, so a second approving account is the only gate left. @yinsong1986 over to you.

@yinsong1986 yinsong1986 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Fixes rename_map camera entries in the lerobot_async provider by applying the image rename client-side: _camera_items now returns the model's feature name as the wire key (via rename_map lookup keyed on observation.images.<key>), so both the SendPolicyInstructions handshake and the raw observation carry the checkpoint's own camera name. This matches the server's actual pipeline, where prepare_raw_observation resizes every declared image by policy_image_features before the RenameObservationsProcessorStep runs, turning a robot-named camera into a server-side KeyError. State entries are still forwarded to the server's rename_map as before. The docstring, docs table, and changelog were updated in R1 to describe the client-side/server-side split, and the pin test gates on pytest.importorskip("lerobot").

What's good

  • Pin tests assert on outputs (_build_lerobot_features keys, _to_raw_observation keys) and fail on pre-fix code, per AGENTS.md testing conventions.
  • Scope discipline: the diff touches exactly the rename path plus its docs and changelog; the forwarded server-side map entry was verified a harmless no-op rather than speculatively removed.
  • Docstrings and the docs table were kept in sync with the new semantics in the same PR.

@cagataycali
cagataycali merged commit b6d9b4a into strands-labs:main Sep 17, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 Next policies Policy inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants