Merged
Conversation
spomichter
previously approved these changes
Jan 9, 2026
Contributor
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR fixes an issue where the new version of mujoco_playground no longer automatically downloads the mujoco_menagerie package on import. The fix explicitly calls mjx_env.ensure_menagerie_exists() during initialization to pre-download the menagerie before spawning the MuJoCo subprocess, preventing timeout issues.
Issues found:
- Typo in comment: "mujoco_menajerie" should be "mujoco_menagerie"
- Missing error handling around the new
ensure_menagerie_exists()call (could fail due to network issues, missing package, etc.) - Using private API (
_srcmodule) which could be fragile if the library changes its internal structure
Confidence Score: 4/5
- This PR is safe to merge with minor improvements recommended
- The change correctly addresses the issue with the new mujoco_playground version. The logic is sound - downloading the menagerie before spawning the subprocess prevents timeouts. However, there's a typo in the comment and missing error handling that should be addressed for robustness
- The single changed file has minor issues (typo and missing error handling) but the core logic is correct
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| dimos/robot/unitree_webrtc/mujoco_connection.py | 4/5 | Updates menagerie download trigger to use explicit function call. Found typo in comment and missing error handling |
Sequence Diagram
sequenceDiagram
participant User
participant MujocoConnection
participant mjx_env
participant subprocess
participant mujoco_process
participant model_py
User->>MujocoConnection: __init__(global_config)
MujocoConnection->>MujocoConnection: import mujoco (check)
MujocoConnection->>MujocoConnection: get_data("mujoco_sim")
Note over MujocoConnection,mjx_env: NEW: Explicit menagerie download
MujocoConnection->>mjx_env: ensure_menagerie_exists()
mjx_env-->>MujocoConnection: menagerie downloaded
User->>MujocoConnection: start()
MujocoConnection->>subprocess: spawn mujoco_process.py
subprocess->>mujoco_process: __main__
mujoco_process->>model_py: load_model()
Note over model_py,mjx_env: Uses MENAGERIE_PATH (already downloaded)
model_py->>mjx_env: MENAGERIE_PATH / "unitree_go1"
mjx_env-->>model_py: path returned
model_py-->>mujoco_process: model loaded
mujoco_process-->>MujocoConnection: ready signal via shared memory
spomichter
requested changes
Jan 9, 2026
b6506ae to
ab55cff
Compare
spomichter
approved these changes
Jan 9, 2026
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
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.
With the new version of
playgroundyou have to trigger downloading of the mujoco_menagerie yourself. It's not done automatically when the playground is imported.