Conversation
Greptile SummaryThis PR restructures Key issues found:
Confidence Score: 2/5Not safe to merge — multiple P1 defects including broken macOS installs for the manipulation extra, a stale dds reference that may abort developer setup, and stripped CI tooling config that will silently break automated checks. Three P1 issues remain: missing platform guards in manipulation will cause install failures on macOS (the exact target platform of the accompanying doc), the stale --no-extra dds reference in a widely-copied setup command, and duplicated/conflicting dimos-viewer version specs between core and the visualization extra. Combined with the stripped pytest/mypy/ruff config removing CI safety nets, the risk surface is significant enough to warrant a score of 2. pyproject.toml requires the most attention — platform guards, deduplication, and restored tooling config. docs/installation/osx.md line 129 needs the --no-extra dds flag removed. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
core_deps["Core dependencies\n(always installed)"]
sim["sim\nmujoco, pygame"]
visualization["visualization\nrerun-sdk, dimos-viewer"]
agents["agents\nlangchain, openai, anthropic, whisper"]
cpu["cpu\nonnxruntime, ctransformers"]
web["web\nfastapi, uvicorn, sse-starlette"]
perception["perception\nultralytics, transformers, Pillow"]
manipulation["manipulation\n⚠️ drake (no aarch64 guard)\n⚠️ pyrealsense2 (no macOS guard)"]
dev["dev\npytest, mypy, ruff, pre-commit"]
unitree["unitree\nunitree-webrtc-connect-leshy"]
full["full"]
recommended["recommended"]
ai["ai"]
core_extra["core"]
full --> sim
full --> agents
full --> web
full --> perception
full --> visualization
unitree --> full
recommended --> sim
recommended --> agents
recommended --> web
recommended --> perception
recommended --> visualization
recommended --> cpu
ai --> agents
ai --> cpu
core_extra --> sim
core_extra --> visualization
core_deps -->|"⚠️ also declares\nrerun-sdk & dimos-viewer"| visualization
Reviews (1): Last reviewed commit: "optimize & update osx.md" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| manipulation = [ | ||
| # Planning (Drake) | ||
| "drake==1.45.0; sys_platform == 'darwin' and platform_machine != 'aarch64'", | ||
| "drake>=1.40.0; sys_platform != 'darwin' and platform_machine != 'aarch64'", | ||
|
|
||
| # Hardware SDKs | ||
| "piper-sdk", | ||
| "pyrealsense2; sys_platform != 'darwin'", | ||
| "xarm-python-sdk>=1.17.0", | ||
|
|
||
| # Visualization (Optional) | ||
| "kaleido>=0.2.1", | ||
| "plotly>=5.9.0", | ||
| "xacro", | ||
|
|
||
| # Other | ||
| "drake>=1.40.0", | ||
| "pyrealsense2", | ||
| "matplotlib>=3.7.1", | ||
| "pyyaml>=6.0", | ||
| ] |
There was a problem hiding this comment.
manipulation extra will break macOS installs
Two packages in the new manipulation extra are missing platform conditions that existed in the previous version:
-
pyrealsense2— Intel RealSense SDK has no macOS support. The old definition included; sys_platform != 'darwin'to guard against this. Without it, any macOS user who runsuv pip install 'dimos[manipulation]'will get a build/install failure. -
drake— The new specdrake>=1.40.0has no platform condition. The original version explicitly excluded Apple Silicon (platform_machine != 'aarch64') because no Drake wheel exists fordarwin/aarch64. M1/M2/M3/M4 Mac users will fail here.
Suggested fix:
manipulation = [
"drake>=1.40.0; platform_machine != 'aarch64'",
"pyrealsense2; sys_platform != 'darwin'",
"matplotlib>=3.7.1",
]
docs/installation/osx.md
Outdated
| cd dimos | ||
|
|
||
| # Project lockfile + all optional groups except DDS (DDS needs separate uv extra). | ||
| uv sync --all-extras --no-extra dds |
There was a problem hiding this comment.
dds extra no longer exists — --no-extra dds may fail
The dds extra was removed from pyproject.toml in this PR, but this command still passes --no-extra dds. Depending on the uv version in use, referencing an unknown extra name with --no-extra may produce an error (e.g., "Unknown extra: dds") and abort the sync entirely, breaking the developer onboarding workflow documented here.
The same stale reference also appears verbatim in docs/installation/ubuntu.md:32, docs/development/testing.md:6, AGENTS.md:13, and README.md:316, and the DDS transport doc at docs/usage/transports/dds.md still instructs users to install .[dds]. All of these need to be updated consistently.
| uv sync --all-extras --no-extra dds | |
| uv sync --all-extras |
| # Runtime tools | ||
| "rerun-sdk>=0.20.0", | ||
| "dimos-viewer>=0.30.0a2", | ||
| "toolz>=1.1.0", | ||
| "protobuf>=6.33.5,<7", | ||
| "psutil>=7.0.0", | ||
| "sqlite-vec>=0.1.6", | ||
| "lz4>=4.4.5", |
There was a problem hiding this comment.
rerun-sdk and dimos-viewer duplicated in core and visualization with mismatched version floors
rerun-sdk>=0.20.0 and dimos-viewer>=0.30.0a2 are already in the dependencies block (lines 80–81), and then rerun-sdk>=0.20.0 and dimos-viewer>=0.30.0a4 appear again in the visualization extra (lines 113–116).
The duplicate dimos-viewer entries carry different lower-bound versions (a2 vs a4). While a resolver will satisfy both by picking >=a4, it means the core package silently requires a lower version than the optional extra, which is misleading and could cause unexpected downgrades if the visualization extra is not installed. The packages should be declared once in the appropriate place.
| # 🔹 Developer tools | ||
| dev = [ | ||
| "ruff==0.14.3", | ||
| "mypy==1.19.0", | ||
| "pre_commit==4.2.0", | ||
| "pytest==8.3.5", | ||
| "pytest-asyncio==0.26.0", | ||
| "pytest-mock==3.15.0", | ||
| "pytest-env==1.1.5", | ||
| "pytest-timeout==2.4.0", | ||
| "coverage>=7.0", | ||
| "requests-mock==1.12.1", | ||
| "terminaltexteffects==0.12.2", | ||
| "watchdog>=3.0.0", | ||
|
|
||
| # docs | ||
| "md-babel-py==1.1.1", | ||
|
|
||
| # LSP | ||
| "python-lsp-server[all]==1.14.0", | ||
| "python-lsp-ruff==2.3.0", | ||
|
|
||
| # Types | ||
| "lxml-stubs>=0.5.1,<1", | ||
| "pandas-stubs>=2.3.2.250926,<3", | ||
| "scipy-stubs>=1.15.0", | ||
| "types-PySocks>=1.7.1.20251001,<2", | ||
| "types-PyYAML>=6.0.12.20250915,<7", | ||
| "types-colorama>=0.4.15.20250801,<1", | ||
| "types-defusedxml>=0.7.0.20250822,<1", | ||
| "types-gevent>=25.4.0.20250915,<26", | ||
| "types-greenlet>=3.2.0.20250915,<4", | ||
| "types-jmespath>=1.0.2.20250809,<2", | ||
| "types-requests>=2.32.4.20260107,<3", | ||
| "types-jsonschema>=4.25.1.20251009,<5", | ||
| "types-networkx>=3.5.0.20251001,<4", | ||
| "types-protobuf>=6.32.1.20250918,<7", | ||
| "types-psutil>=7.2.2.20260130,<8", | ||
| "types-psycopg2>=2.9.21.20251012", | ||
| "types-pytz>=2025.2.0.20250809,<2026", | ||
| "types-simplejson>=3.20.0.20250822,<4", | ||
| "types-tabulate>=0.9.0.20241207,<1", | ||
| "types-tensorflow>=2.18.0.20251008,<3", | ||
| "types-tqdm>=4.67.0.20250809,<5", | ||
|
|
||
| # Tools | ||
| "py-spy", | ||
| "pytest", | ||
| "mypy", | ||
| "ruff", | ||
| "pre-commit", | ||
| ] |
There was a problem hiding this comment.
dev extra and [tool.*] sections
The dev extra previously pinned specific, tested versions of ruff, mypy, pytest, and related packages, plus the full [tool.ruff.lint], [tool.mypy.overrides], [tool.pytest.ini_options] (env vars, addopts, asyncio settings), and [tool.coverage] configurations. All of these have been removed.
Immediate practical side-effects:
[tool.pytest.ini_options]no longer setsasyncio_mode = "auto"— any async test usingpytest-asynciowill now fail unless that fixture is set up individually.[tool.pytest.ini_options]no longer setsGOOGLE_MAPS_API_KEYenv var — tests depending on it will fail or need live credentials.[tool.mypy]no longer hasignore_missing_importsoverrides for third-party stubs (mujoco, scipy, ultralytics, …) —mypyCI will produce new errors on unchanged code.rufflint rule configuration (extend-select,ignore) is gone — the CI linting profile has changed silently.
If this is intentional (moving config to separate files), those files should be part of this PR.
| classifiers = [ | ||
| "Development Status :: 3 - Alpha", | ||
| "Intended Audience :: Developers", | ||
| "Intended Audience :: Science/Research", | ||
| "License :: OSI Approved :: Apache Software License", | ||
| "Programming Language :: Python :: 3", | ||
| "Programming Language :: Python :: 3 :: Only", | ||
| "Programming Language :: Python :: 3.10", | ||
| "Programming Language :: Python :: 3.11", | ||
| "Programming Language :: Python :: 3.12", | ||
| "Topic :: Scientific/Engineering :: Artificial Intelligence", | ||
| "Topic :: Software Development :: Libraries :: Application Frameworks", | ||
| "Topic :: Scientific/Engineering :: Image Processing", | ||
| "Topic :: Scientific/Engineering :: Visualization", | ||
| "Operating System :: POSIX :: Linux", | ||
| "Operating System :: MacOS", | ||
| "Operating System :: POSIX :: Linux", | ||
| ] |
There was a problem hiding this comment.
requires-python = ">=3.10"
The Programming Language :: Python :: 3.10 and Programming Language :: Python :: 3.11 classifiers were dropped, but requires-python = ">=3.10" (line 25) still allows those versions. PyPI uses these classifiers to filter packages in search results — users on 3.10 or 3.11 may not discover the package, even though it claims to support them. Either update requires-python to ">=3.12" (if support is intentionally dropping), or restore the missing classifiers.
Also: "Programming Language :: Python :: 3 :: Only" was removed, which means the PyPI page will no longer explicitly signal that this is a Python 3-only package.
| # ----------------------- | ||
| # CLI Entry Points | ||
| # ----------------------- | ||
| [project.scripts] | ||
| lcmspy = "dimos.utils.cli.lcmspy.run_lcmspy:main" | ||
| foxglove-bridge = "dimos.utils.cli.foxglove_bridge.run_foxglove_bridge:main" | ||
| agentspy = "dimos.utils.cli.agentspy.agentspy:main" | ||
| humancli = "dimos.utils.cli.human.humanclianim:main" | ||
| dimos = "dimos.robot.cli.dimos:main" | ||
| agentspy = "dimos.utils.cli.agentspy.agentspy:main" | ||
| foxglove-bridge = "dimos.utils.cli.foxglove_bridge.run_foxglove_bridge:main" | ||
| rerun-bridge = "dimos.visualization.rerun.bridge:app" |
There was a problem hiding this comment.
The following [project.scripts] entries were removed, but their implementation files still exist in the repository:
lcmspy→dimos/utils/cli/lcmspy/run_lcmspy.pyhumancli→dimos/utils/cli/human/humanclianim.pydoclinks→dimos/utils/docs/doclinks.pydtop→dimos/utils/cli/dtop.py
Removing these from entry points without deleting (or explicitly deprecating) the source files leaves dead code and breaks any workflows or documentation that reference those commands. If the intent is to deprecate them, the source files should be removed or a migration note added.
|
Sorry but closing until you've done the CLA and given some details |
2 participants
Problem
Closes DIM-XXX
Solution
Breaking Changes
How to Test
Contributor License Agreement