Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request introduces an optional Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant Cache as GitHub Cache
participant CLI as GitHub CLI (gh)
participant Repo as jiwenc-nv/v2d
participant FS as Filesystem
participant CMake as CMake Build
GHA->>GHA: Read deps/v2d/version.txt
GHA->>GHA: Extract pinned SHA
GHA->>Cache: Lookup cache key v2d-src-{sha}
alt Cache Hit
Cache-->>GHA: Return cached deps/v2d/src
else Cache Miss
GHA->>CLI: gh repo clone jiwenc-nv/v2d (retargeter)
CLI->>Repo: Clone retargeter branch
Repo-->>CLI: Repository content
GHA->>GHA: git checkout {pinned-sha}
GHA->>FS: Copy robotic_grounding/ to deps/v2d/src/
GHA->>FS: Clean __pycache__ directories
GHA->>Cache: Store in cache with key v2d-src-{sha}
end
GHA->>GHA: Check for __init__.py
GHA->>GHA: Set bundled output true/false
GHA->>CMake: Pass -DBUNDLE_ROBOTIC_GROUNDING={bundled}
CMake->>FS: Copy robotic_grounding into wheel staging
CMake->>FS: Generate wheel with package
sequenceDiagram
participant Build as Build System
participant CMake as CMake
participant Wheel as Wheel File
participant Script as strip_robotic_grounding.py
participant Artifact as Release Artifact
Build->>CMake: Configure with BUNDLE_ROBOTIC_GROUNDING
CMake->>Wheel: Create wheel (robotic_grounding included)
alt Release Branch
Build->>Script: Call strip_robotic_grounding_from_wheel.py
Script->>Wheel: Scan for robotic_grounding/* entries
Script->>Wheel: Create temp wheel without matched entries
Script->>Wheel: Recompute SHA-256 hashes & RECORD
Script->>Wheel: Atomically replace original
Script->>Build: Report modifications made
else Non-Release Branch
Wheel->>Artifact: Use as-is (robotic_grounding included)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/setup-v2d-src/action.yml:
- Around line 46-52: The current check only ensures deps/v2d/version.txt is
non-empty; update the script to validate that the value in SHA is a full git
commit SHA (e.g. match /^[0-9a-f]{40}$/i) before writing to GITHUB_OUTPUT and
proceeding, and fail with an explicit error if it doesn't match; apply the same
SHA regex validation to the counterpart script (scripts/setup_v2d_src.sh) so
both the action and the setup script reject branch/tag names and only accept
commit SHAs.
In `@scripts/strip_robotic_grounding_from_wheel.py`:
- Around line 63-76: The strip step currently only removes files under
robotic_grounding/ and rewrites RECORD; extend it to also rewrite the package
METADATA inside the .dist-info directory to remove the grounding extra: locate
the METADATA entry when iterating zin.infolist() (use the same loop that
references item.filename, kept_records and record_arcname), read and parse its
text, drop any "Provides-Extra: grounding" lines and remove "[grounding]"
suffixes from any Requires-Dist entries (e.g., turn "isaacteleop[grounding]"
into "isaacteleop"), then write the modified METADATA back into zout and add its
new RECORD line via _record_line so RECORD reflects the changed METADATA before
you append the final RECORD row and write RECORD using kept_records and
record_arcname.
In `@src/retargeters/__init__.py`:
- Around line 105-121: The current __init__.py exposes SharpaHandRetargeter,
SharpaBiManualRetargeter, and SharpaHandRetargeterConfig unconditionally which
breaks wheels built without the robotic_grounding bundle; update the module to
only add these three symbols to the export mapping when the same bundling signal
used at build time is present (e.g. a module-level flag like
_HAS_ROBOTIC_GROUNDING or a guarded import check for the robotic_grounding
package), otherwise omit/hide those entries and replace the pip-install error
text with a gated message that appears only when the bundle is available;
specifically modify the export dict population in __init__.py so
SharpaHandRetargeter, SharpaBiManualRetargeter, and SharpaHandRetargeterConfig
are only added when the bundling flag/import check succeeds.
In `@src/retargeters/sharpa_hand_retargeter.py`:
- Around line 284-291: The loop that builds
_output_indices_left/_output_indices_right and _left_indices/_right silently
prefers left when a joint name appears in both left_joint_names and
right_joint_names; update the logic in the loop over target_joint_names (the
block that appends to _output_indices_left/_output_indices_right and
_left_indices/_right) to detect the overlap (jname present in both
left_joint_names and right_joint_names) and reject it instead of choosing one
side — e.g., raise an exception or log an explicit error and skip/abort the
mapping for that jname so duplicate names cannot silently misroute data.
- Around line 237-244: Wrap the call to self._kinematics.compute(...) in a
try-except inside the retargeter method so any exception from compute does not
abort the frame; on exception call self._emit_zeros() and return (or set
new_qpos to zeros) and do not update self._qpos_prev, otherwise continue using
the returned result and update self._qpos_prev = new_qpos.copy(). Locate the
compute call in the method that invokes self._kinematics.compute and ensure the
exception handler mirrors the existing invalid-input pattern (calls
_emit_zeros() as lines handling invalid inputs do) and only updates
self._qpos_prev on successful compute.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 59566bda-8c3d-4d35-8849-9c02c7aae965
📒 Files selected for processing (19)
.github/actions/setup-v2d-src/action.yml.github/workflows/build-ubuntu.yml.gitignoredeps/v2d/version.txtdocs/source/index.rstdocs/source/references/retargeting/index.rstdocs/source/references/retargeting/sharpa.rstexamples/retargeting/python/sharpa_hand_retargeter_demo.pyscripts/setup_v2d_src.shscripts/strip_robotic_grounding_from_wheel.pysrc/core/python/CMakeLists.txtsrc/core/python/pyproject.toml.insrc/core/python/requirements-grounding.txtsrc/core/python/requirements-retargeters.txtsrc/core/retargeting_engine_tests/python/CMakeLists.txtsrc/core/retargeting_engine_tests/python/pyproject.tomlsrc/core/retargeting_engine_tests/python/test_sharpa_hand_retargeter.pysrc/retargeters/__init__.pysrc/retargeters/sharpa_hand_retargeter.py
Collaborator
Author
|
/preview-docs |
Contributor
|
✅ Preview deployed: https://NVIDIA.github.io/IsaacTeleop/preview/pr-449/ |
Note: V2D is still pre-release and gated from public access. The fetch is gated on V2D_RETARGETER_TOKEN and skipped on release branches.
Wraps robotic_grounding.retarget.SharpaHandKinematics with Teleop's BaseRetargeter contract -- OpenXR -> MANO joint mapping, warm-started qpos, output indexing. Lazy-imports cleanly: installs without the [grounding] extra raise a directed ModuleNotFoundError instead of breaking module load. Co-Authored-By: rwiltz <165190220+rwiltz@users.noreply.github.com>
Promotes references/retargeting.rst into a directory and adds sharpa.rst covering [grounding] build, Python usage, the demo, and ctest validation.
sgrizan-nv
approved these changes
May 6, 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
2 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.
Summary by CodeRabbit
New Features
[grounding]extra package with hand retargeting capabilities and dependenciesDocumentation
Tests