Skip to content

refactor(grasping): isolate GraspGenX dependencies - #4102

Merged
TomCC7 merged 14 commits into
mainfrom
cc/chore/isolated-graspgenx
Sep 17, 2026
Merged

TomCC7 merged 14 commits into
mainfrom
cc/chore/isolated-graspgenx

Conversation

@TomCC7

@TomCC7 TomCC7 commented Sep 12, 2026

Copy link
Copy Markdown
Member

Contribution path

User-requested dependency isolation, with the design agreed before implementation. No linked issue or discussion; opening as a draft for review.

Problem

GraspGenX contributes its dependency graph and seven overrides to the main uv project even when its extra is not installed. Its dedicated worker separates execution but still uses the host Python environment.

Solution

  • Run GraspGenX in its own locked Python 3.12 environment through IsolatedPythonModule, preserving its proposal RPC and blueprints. Remove its root extra and seven overrides.
  • Keep isolated projects under native/python/, outside the dimOS package. Declarations select a repository-relative project_dir.
  • Reuse the LFS checkout helper for runtime sources and child dimOS: the development checkout, or a cached clone initially fetched from main. No runtime sources or lockfiles are bundled in wheel/sdist artifacts.
  • Keep runtime tests and strict type checking in the isolated project. Dedicated CI integration remains separate.

An installed host and the shared checkout can differ in revision. Existing clones are not updated automatically and must contain the declared runtime project.

How to Test

Verify through the existing visible MuJoCo blueprint:

MUJOCO_GL=glfw dimos --viewer none run xarm-grasp-graspgenx \
  --simulation mujoco --headless false

In a second terminal, run dimos shell, then:

scan = app.PickAndPlaceModule.scan_objects(["gray can"])
cloud = app.ObjectSceneRegistrationModule.get_object_pointcloud_by_object_id(
    scan.metadata["objects"][0]["object_id"]
)
candidates = app.GraspGenXModule.propose_grasps(cloud)
print(len(candidates.candidates), [c.score for c in candidates.candidates[:5]])

Exit the shell and run dimos stop afterward.

Run runtime tests and type checking from the repository root:

cd native/python/graspgenx
export UV_PROJECT_ENVIRONMENT="${XDG_CACHE_HOME:-$HOME/.cache}/dimos/graspgenx-tests"
uv run --frozen --group tests --with-editable ../../.. python -m pytest
uv run --frozen --group lint --with-editable ../../.. python -m mypy

Validation:

  • 43 host/framework tests and 13 runtime tests passed. Root mypy passed across 1,119 source files; runtime mypy passed across its two source files.
  • A wheel-installed host launched the existing example from a provisioned shared checkout and completed RPC calls and restart.
  • Visible MuJoCo plus dimos shell: a 417-point cloud produced 100 ranked grasps with finite scores and preserved frame/timestamp. The GraspGenX child exited with code 0; overall blueprint shutdown required the CLI SIGKILL escalation.
  • Inspected wheel/sdist: host contracts and bootstrap included; isolated projects, implementations, and lockfiles absent. Built with DIMOS_ALLOW_MISSING_COCKPIT=1 because compiled cockpit assets were unavailable.
  • Ruff and commit hooks passed. No coordinator or CI configuration changes in this revision. The previously observed concurrent-shutdown race remains outside this change.

The PR also guards unavailable yourdfpy imports on Linux ARM and clears inherited xdist identity in the nested collection test, addressing earlier CI failures.

AI assistance

Codex (GPT-6) performed repository inspection, implementation, testing, and PR preparation from the user-approved plan.

Checklist

  • I have read and approved the CLA.

Agent: Codex (GPT-6).

Comment thread native/python/example/pyproject.toml
@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.80702% with 5 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/manipulation/grasping/grasp_gen_x/module.py 90.19% 3 Missing and 2 partials ⚠️
@@           Coverage Diff           @@
##             main    #4102   +/-   ##
=======================================
  Coverage   79.06%   79.06%           
=======================================
  Files        1466     1466           
  Lines      138931   138851   -80     
  Branches    11962    11956    -6     
=======================================
- Hits       109842   109781   -61     
+ Misses      25687    25673   -14     
+ Partials     3402     3397    -5     
Components Coverage Δ
Tests 94.99% <100.00%> (-0.01%) ⬇️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.86% <96.03%> (-0.16%) ⬇️
OS-ubuntu-latest 75.71% <94.29%> (-0.01%) ⬇️
Py-3.10 75.48% <94.29%> (-0.02%) ⬇️
Py-3.11 75.69% <94.29%> (-0.01%) ⬇️
Py-3.12 75.70% <96.05%> (-0.01%) ⬇️
SelfHosted-Large 30.77% <44.73%> (+0.02%) ⬆️
SelfHosted-Linux 36.22% <44.73%> (+0.01%) ⬆️
SelfHosted-macOS 35.56% <44.73%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/conftest.py 83.24% <100.00%> (+1.42%) ⬆️
...s/experimental/isolated_python/example/contract.py 88.46% <100.00%> (+0.46%) ⬆️
dimos/experimental/isolated_python/module.py 83.33% <100.00%> (+2.68%) ⬆️
...os/experimental/isolated_python/test_end_to_end.py 100.00% <ø> (ø)
dimos/experimental/isolated_python/test_module.py 99.23% <100.00%> (+0.26%) ⬆️
...s/manipulation/grasping/grasp_gen_x/test_module.py 100.00% <100.00%> (ø)
...anipulation/planning/spec/test_model_validation.py 100.00% <100.00%> (ø)
...ion/planning/utils/test_point_cloud_self_filter.py 97.36% <100.00%> (+0.14%) ⬆️
dimos/robot/all_blueprints.py 100.00% <ø> (ø)
...obot/manipulators/dual_openyam/test_integration.py 100.00% <100.00%> (ø)
... and 4 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread dimos/manipulation/grasping/grasp_gen_x/test_module.py Outdated
Comment thread dimos/manipulation/grasping/grasp_gen_x/fixtures/object_cloud.npy Outdated
Comment thread dimos/manipulation/grasping/grasp_gen_x/python/graspgenx_runtime/backend.py Outdated
Comment thread dimos/experimental/isolated_python/module.py Outdated
Comment thread dimos/experimental/isolated_python/module.py Outdated
Comment thread docs/capabilities/manipulation/xarm-grasp.md
@TomCC7 TomCC7 added the backport:skip Skip creating a backport to any release branches label Sep 13, 2026
@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

Do not merge until an undeploy failure either preserves the deployment record or explicitly terminates and removes the affected worker.

Findings

  1. P1 Preserve failed deployments

Summary

  • This PR packages GraspGenX in an isolated Python runtime, updates runtime provisioning and nested-project discovery, and changes coordinator restart behavior.
  • The coordinator can remove all tracking for a module even when its worker-side undeploy fails before stopping the module. A later restart can then create a replacement while the original worker remains active and untracked.
  • T-Rex validation blocked

  • The focused lifecycle check could not start because the repository environment was unavailable. The system environment lacked dimos, and project dependency setup failed because building pyaudio==0.2.14 requires the unavailable portaudio.h header.

Reviews (1) · Last reviewed commit: "fix: wait for module shutdown before red..."

Comment thread dimos/core/coordination/module_coordinator.py
Comment thread dimos/experimental/isolated_python/module.py Outdated
@TomCC7 TomCC7 added backport release/0.0.14 and removed backport:skip Skip creating a backport to any release branches labels Sep 14, 2026
Comment thread pyproject.toml Outdated
Comment thread MANIFEST.in Outdated
…spgenx

# Conflicts:
#	dimos/experimental/isolated_python/README.md
#	dimos/experimental/isolated_python/test_module.py
@TomCC7
TomCC7 added this pull request to stack #4176 September 16, 2026 00:15
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 17, 2026
@TomCC7
TomCC7 added this pull request to the merge queue Sep 17, 2026
Merged via the queue into main with commit 4bfecf6 Sep 17, 2026
73 of 87 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Backport failed for release/0.0.14, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release/0.0.14
git worktree add -d .worktree/backport/4102-to-release/0.0.14 origin/release/0.0.14
cd .worktree/backport/4102-to-release/0.0.14
git switch --create backport/4102-to-release/0.0.14
git cherry-pick -x 4bfecf65090c9738abef892c1f6b5b0a7f36a489

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport release/0.0.14 ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants