Skip to content

fix(cli): fix dimos --help (both bad imports and speed)#1571

Merged
paul-nechifor merged 7 commits intodevfrom
jeff/fix/help
Mar 20, 2026
Merged

fix(cli): fix dimos --help (both bad imports and speed)#1571
paul-nechifor merged 7 commits intodevfrom
jeff/fix/help

Conversation

@jeff-hykin
Copy link
Member

@jeff-hykin jeff-hykin commented Mar 15, 2026

Problem

uv pip install dimos
dimos --help  # throws error, missing langchain

dimos --help takes ~5s because GlobalConfig imports NavigationStrategy from path_map.py (which pulls in matplotlib/scipy/numpy) and VlModelName from create.py (which pulls in torch/langchain). These are just Literal type aliases but they drag in the entire ML stack.

Solution

Move the type aliases into lightweight files in their respective packages:

  • dimos/mapping/occupancy/types.pyNavigationStrategy
  • dimos/models/vl/types.pyVlModelName

global_config.py imports from these instead. Original modules re-export so existing imports still work.

Also adds a regression test that fails if dimos --help takes >8s or if heavy deps leak into the CLI entrypoint.

Breaking Changes

None. All existing imports of NavigationStrategy and VlModelName still work.

How to Test

time dimos --help
# Should be ~2s, was ~5s before

python -m pytest dimos/robot/cli/test_cli_startup.py -v
# Both tests should pass

Contributor License Agreement

  • I have read and approved the CLA.

Move NavigationStrategy and VlModelName type aliases into dimos/core/types.py
so that global_config.py no longer pulls in matplotlib/scipy (via path_map.py)
or torch/langchain (via create.py) at import time. Original modules re-export
from the new file so existing imports continue to work.

`dimos --help` drops from ~3-4s to ~1.9s.
NavigationStrategy → dimos/mapping/occupancy/types.py
VlModelName → dimos/models/vl/types.py
Remove dimos/core/types.py
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 15, 2026

Greptile Summary

Extracts lightweight Literal type aliases (NavigationStrategy, VlModelName) into dedicated types.py modules so that GlobalConfig no longer transitively imports heavy ML/viz libraries (matplotlib, scipy, torch) at CLI startup, cutting dimos --help time from ~5s to ~2s.

  • dimos/mapping/occupancy/types.py and dimos/models/vl/types.py are new zero-dependency modules housing the extracted type aliases
  • global_config.py now imports from these lightweight modules instead of path_map.py (scipy/numpy) and create.py (torch/langchain)
  • Original modules re-import from types.py, preserving backward compatibility for all existing consumers
  • Adds two regression tests: one verifying no heavy deps leak into GlobalConfig, another ensuring dimos --help finishes within 8s
  • .venv added to .gitignore

Confidence Score: 5/5

  • This PR is safe to merge — it only moves type aliases to leaf modules with no behavioral changes.
  • The changes are minimal and mechanical: two Literal type aliases are extracted into standalone files, import paths are updated, and original modules re-export for backward compatibility. No logic changes, no new dependencies, and the regression tests provide a good safety net. Verified that no other files in the repo use the old import paths directly.
  • No files require special attention.

Important Files Changed

Filename Overview
.gitignore Adds .venv to gitignore — straightforward and safe.
dimos/core/global_config.py Switches imports of NavigationStrategy and VlModelName from heavy modules to lightweight types.py files — the core change that breaks the heavy import chain.
dimos/mapping/occupancy/path_map.py Moves NavigationStrategy definition to types.py and re-imports it, maintaining backward compatibility. Typing-only imports removed cleanly.
dimos/mapping/occupancy/types.py New lightweight module containing only the NavigationStrategy type alias with no heavy dependencies.
dimos/models/vl/types.py New lightweight module containing only the VlModelName Literal type with no heavy dependencies.
dimos/models/vl/create.py Moves VlModelName definition to types.py and re-imports it. Unused Literal import removed cleanly.
dimos/robot/cli/test_cli_startup.py New regression tests for CLI startup time and heavy-dep leakage. Unused pytest import. Tests are well-structured with reasonable timeouts.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Before["Before (slow path)"]
        CLI_OLD["dimos CLI"] --> GC_OLD["global_config.py"]
        GC_OLD --> PM_OLD["path_map.py"]
        PM_OLD --> SCIPY["scipy / numpy / matplotlib"]
        GC_OLD --> CR_OLD["create.py"]
        CR_OLD --> TORCH["torch / langchain"]
    end

    subgraph After["After (fast path)"]
        CLI_NEW["dimos CLI"] --> GC_NEW["global_config.py"]
        GC_NEW --> OT["occupancy/types.py\n(NavigationStrategy)"]
        GC_NEW --> VT["vl/types.py\n(VlModelName)"]
        OT -. "only typing stdlib" .-> STDLIB["typing (stdlib)"]
        VT -. "only typing stdlib" .-> STDLIB
    end

    style SCIPY fill:#f88,stroke:#a00
    style TORCH fill:#f88,stroke:#a00
    style STDLIB fill:#8f8,stroke:#0a0
    style OT fill:#8f8,stroke:#0a0
    style VT fill:#8f8,stroke:#0a0
Loading

Last reviewed commit: 0bd3622

import sys
import time

# CI runners are slower — give generous headroom but still catch gross regressions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused pytest import

pytest is imported on this line but never used in the module — no pytest.mark, pytest.raises, pytest.fixture, etc. appear anywhere. This will trigger linter warnings (e.g. F401 in flake8/ruff).

Suggested change
# CI runners are slower — give generous headroom but still catch gross regressions.

@jeff-hykin jeff-hykin changed the title fix(cli): speed up dimos --help (~5s → ~2s) fix(cli): fix dimos --help (both bad imports and speed) Mar 15, 2026
Guards against heavy imports (matplotlib, torch, scipy) leaking into
the CLI entrypoint via GlobalConfig. Fails if dimos --help takes >8s.
@jeff-hykin jeff-hykin mentioned this pull request Mar 15, 2026
1 task
paul-nechifor
paul-nechifor previously approved these changes Mar 17, 2026
leshy
leshy previously approved these changes Mar 19, 2026
@jeff-hykin jeff-hykin dismissed stale reviews from leshy and paul-nechifor via 88b1ed4 March 19, 2026 21:17
Conflicts resolved:
- global_config.py: keep PR's refactored imports (VlModelName from types, NavigationStrategy from occupancy/types)
- path_map.py: take dev's expanded imports (GradientStrategy, gradient, voronoi_gradient) needed by the function
build_file_index now skips paths rooted in .venv, node_modules,
__pycache__, or .git. Fixes test_excludes_venv failure when .venv
is a symlink (not matched by gitignore trailing-slash patterns).
@paul-nechifor paul-nechifor merged commit aed901f into dev Mar 20, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants