early review: feat: MuJoCo Simulation Backend + Build System Modernization#80
Closed
cagataycali wants to merge 2 commits into
Closed
early review: feat: MuJoCo Simulation Backend + Build System Modernization#80cagataycali wants to merge 2 commits into
cagataycali wants to merge 2 commits into
Conversation
cagataycali
pushed a commit
to cagataycali/robots
that referenced
this pull request
Mar 31, 2026
…trands-labs#56 Distilled @awsarron's recurring review feedback and applied to PR strands-labs#80: 1. Narrow except Exception blocks (13 instances fixed): - simulation.py: object/camera injection now raises RuntimeError instead of silently falling back to 'metadata only' - simulation.py: auto-download failure returns error instead of warning - scene_ops.py: narrowed to ValueError/KeyError/RuntimeError - mjcf_builder.py: narrowed to FileNotFoundError/OSError - rendering.py: narrowed camera render failure + added explanatory comment - base.py: narrowed __del__ cleanup to specific types 2. Use require_optional() for MuJoCo lazy import (backend.py): - Consistent with package-wide pattern from strands_robots/utils.py - Same error messages, same caching behavior 3. Dependency upper bounds (pyproject.toml): - robot_descriptions>=1.0.0,<2.0.0 (was missing upper bound) 4. Added REVIEW_TASKS.md tracking all issues from PR review distillation Conventions enforced per AGENTS.md: - Raise on fatal errors, never warn-and-continue - No silent defaults on error - Use require_optional() for optional deps
cagataycali
force-pushed
the
feat/mujoco-simulation
branch
2 times, most recently
from
March 31, 2026 11:41
be04ab2 to
298dbfe
Compare
Add a complete MuJoCo-based simulation backend that enables agent-driven robot simulation via natural language. 35 simulation actions exposed as a single AgentTool, supporting physics stepping, rendering, policy inference, dataset recording, domain randomization, and scene manipulation. ## New Modules ### strands_robots/simulation/ - base.py: SimulationBackend ABC defining the interface all backends implement - factory.py: create_simulation() dispatch — currently MuJoCo, extensible - models.py: SimWorld, SimRobot, SimObject, SimCamera, TrajectoryStep dataclasses - model_registry.py: URDF/MJCF path resolution via robot_descriptions + registry ### strands_robots/simulation/mujoco/ - simulation.py: Simulation(AgentTool) — 35 actions callable by NL agents - physics.py: PhysicsMixin — step, reset, joint control, contact detection - rendering.py: RenderingMixin — RGB + depth offscreen rendering - recording.py: RecordingMixin — start/stop recording → LeRobot v3 datasets - policy_runner.py: PolicyRunnerMixin — run_policy, eval_policy, replay_episode - randomization.py: RandomizationMixin — domain randomization (colors, physics) - scene_ops.py: Inject/eject objects & cameras into live MuJoCo scenes - mjcf_builder.py: Procedural MJCF XML generation with URDF conversion - backend.py: Lazy MuJoCo import with GL auto-detection + headless support - tool_spec.json: AgentTool JSON schema (35 actions) ### strands_robots/ - factory.py: Robot() factory — resolves name → sim or hardware dispatch - _async_utils.py: Coroutine resolution helpers - assets/: Robot asset manager with auto-download from MuJoCo Menagerie - dataset_recorder.py: DatasetRecorder → LeRobot v3 parquet + video format ## Build System - Migrated from hatch to uv (pyproject.toml + uv.lock) - .python-version pinned to 3.12 - CI workflow uses uv for install + test + lint - OSMesa installed in CI for headless MuJoCo rendering ## Testing (34 tests) - test_factory.py: 22 tests — Robot factory, registry, mode detection, aliases - test_mujoco_e2e.py: 12 tests — physics, rendering, mock policy loop, domain randomization, shared dataclass contracts - @requires_gl skip marker for tests needing OpenGL context ## Error Handling (review pattern compliance) - Narrowed 13 broad except-Exception blocks to specific types (ValueError, RuntimeError, FileNotFoundError, OSError, KeyError) - Object/camera injection raises RuntimeError on failure instead of silently falling back to metadata-only tracking - Auto-download failures return explicit error dicts - Uses require_optional() from strands_robots/utils.py for MuJoCo lazy import - Dependency upper bounds: robot_descriptions>=1.0.0,<2.0.0 ## Headless Rendering - _can_render() probe in backend.py tests OpenGL once, caches result - render()/render_depth() return clear error when GL unavailable - _get_sim_observation() skips cameras gracefully, still returns joint state - Policy runner silently skips video frames when rendering unavailable - Physics/stepping/joints work without any GL context
cagataycali
force-pushed
the
feat/mujoco-simulation
branch
from
March 31, 2026 11:42
298dbfe to
f2a6b52
Compare
- Remove test.py (used pre-factory Robot API) - Add 6 examples covering sim + real factory workflows: 01_sim_quickstart: 5-line sim quickstart 02_sim_agent: Agent + Robot tool (NL control) 03_sim_recording: Mock policy → LeRobot dataset 04_real_hardware: Real robot with cameras 05_real_groot_policy: GR00T on real SO-101 06_list_robots: Registry discovery
cagataycali
changed the title
cagataycali
changed the title
Member
Author
📦 PR Decomposition CompleteThis monolithic PR has been split into 6 sequential PRs for easier review: Merge order
How to proceed
All PRs are on the project board with Status=In review, Priority=P1. cc @awsarron |
Contributor
|
No concerns from me |
github-project-automation
Bot
moved this from In review
to Done
in Strands Labs - Robots
max-rattray-aws
approved these changes
Apr 2, 2026
github-project-automation
Bot
moved this from In review
to Done
in Strands Labs - Robots
Member
Author
|
Closing — superseded by the decomposed PR stack: #83 (build system) → #84 (sim foundation) → #85 (MuJoCo backend). This monolithic PR was split for smaller, reviewable chunks. 🤖 AI agent response. Strands Agents. Feedback welcome! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This was the original monolithic PR (~4,200 lines). It has been split into a reviewable chain:
Merge Order (sequential — each depends on the previous)
[sim]extraSimulationAgentTool with 35 actions, 47 testsRobot()factory — auto-detect sim/real,list_robots(), lazy importsHow to review
Each PR has its own test suite and passes lint independently.
After all 6 are merged
Close this PR — it will be fully superseded.
Original description preserved below for reference.
Original: MuJoCo Simulation Backend + Build System Modernization
Adds MuJoCo-based simulation with 35 agent tool actions, Robot() factory with auto-detect, comprehensive test suite (357 tests), build system modernization, and full documentation rewrite.