[rwiltz]

Description

Moves teleop_se3_agent.py, record_demos.py and teleop_replay_agent.py into isaaclab_teleop.entrypoints, following the isaaclab_rl.entrypoints pattern (#6553, #6793), and untangles
record_demos.py from isaaclab_mimic, which is slated for deprecation.

• Entrypoints: run_teleop_se3_agent_cli, run_record_demos_cli and run_teleop_replay_agent_cli take the command line without the program name. The function bodies are unchanged; the argument
  parsing that ran at import time becomes _parse_args(argv). Git tracks the modules as renames, so review with rename detection.
• Scripts kept as thin wrappers at their current paths (same shape as random_agent.py), so documented commands and the wheel staging list keep working.
• isaaclab teleop run|record call the entrypoints in process, like isaaclab train, instead of launching the scripts. replay still runs scripts/tools/replay_demos.py, which is not moved.
• Out-of-tree tasks: each entrypoint calls load_external_tasks() before parsing, so tasks registered through the isaaclab.tasks entry point work with the scripts and isaaclab teleop.
• Mimic untangled from record_demos.py:
  • Mimic task variants (*-Mimic-v0) are registered only when isaaclab_mimic is installed.
  • The instruction display, whose only user was record_demos.py, moves from isaaclab_mimic.ui into the entrypoints package as a private module.
  • Without Kit the display has no labels and shows nothing, replacing the SimpleNamespace stand-ins from Run the teleop scripts without Kit when nothing needs it #8828.

Notes for review:

• The entrypoints import isaaclab_tasks without isaaclab_teleop declaring it. Declaring it would create a cycle, since isaaclab_tasks imports isaaclab_teleop; the import only happens when a CLI
  function runs.

• The instruction display moves from isaaclab_mimic with its original Apache-2.0 and BSD-3-Clause notices;
  .pre-commit-config.yaml licenses it like the mimic files so the header hooks keep them.

• The modules keep a module-level args_cli, so the moved functions stay unchanged. Passing it explicitly, as the RL backends do, is left for a follow-up.

  Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (isaaclab_mimic.ui.instruction_display is removed; its only user was record_demos.py)

Checklist

Docker and GPU tests run on demand. Push the commits you want tested, then
comment run-ci on the pull request.

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with uv run isaaclab --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have validated the changed behavior with focused coverage and applied the test-audit criteria to any test changes
• I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump package versions — CI handles that)
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the remaining test coverage suggestion is non-blocking.

Findings

1.  Public entrypoints lack focused tests ▶

Summary

Moves the teleoperation, recording, and capture-replay scripts into isaaclab_teleop.entrypoints, while keeping their old paths as thin wrappers.

• Live teleoperation and replay now run through package entrypoints.
• Demonstration recording gains a package entrypoint and no longer needs Mimic.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Existing script path] --> B[Public run_*_cli function]
    B --> C[Load installed downstream tasks]
    C --> D[Parse arguments]
    D --> E[Run main]
    E --> F[Restore caller arguments]

Loading

_{Reviews (2) · Last reviewed commit: "Test that the teleop entrypoints load ou..." · Reviewed by Greptile}

[rwiltz]

@greptile

[rwiltz]

run-ci

[isaaclab-review-bot]

Isaac Lab Review Bot

Needs attention: Design and architecture (1), API (1), Compatibility and deprecation (1), Implementation (1), Style consistency (1). Other reviewed categories look good.

• PR description: Replace the inaccurate rename and unchanged-body claims with external-task and optional-Mimic changes, document license provenance, and list focused wrapper, argv, and Mimic-present/absent validation.
• Design and architecture: Documented direct entrypoint calls skip the wrappers’ Warp bootstrap, allowing imported kernels to enable unnecessary adjoint generation; configure Warp before implementation imports.
• Scope and over-engineering: No over-engineering issues.
• API: The three public run functions do not document argv ownership, None behavior, or integer results; add complete Google-style Args and Returns sections.
• Compatibility and deprecation: Breaking changes: isaaclab_mimic.ui.instruction_display is removed without a functional deprecated forwarding module, targeted warning, migration guidance, removal schedule, or transition coverage.
• Implementation: All three entrypoint modules eagerly import undeclared isaaclab_tasks, so lazy API access fails without that cyclic dependency; move task-specific imports into execution paths.
• Style consistency: Replay docstrings reference helpers under the obsolete scripts.tools.record_demos location; update them to isaaclab_teleop.entrypoints.record_demos.
• Test quality: Test quality looks good.

Significant concerns. Posted 5 inline findings.

Automated review; human maintainers own approval decisions.

[rwiltz]

run-ci

[AntoineRichard]

Experimental review tooling 🤖

Thanks for opening this PR to IsaacLab! We're currently experimenting with automated review tooling, and we'd appreciate a little human time to review the review! The reviewer is under review, too.

Please reply with feedback on useful findings, false positives, and anything the tool missed.

Open Code Review v1.13.3 with Sol6.1 (azure/openai/gpt-6.1-sol, via NVIDIA Inference Hub) reviewed 116616efc54fc98bb14e5f9c884553ec878c34fd.

Result: No findings reported across 18 selected text files.

Scope: 18 of 18 changed files selected, including Python tests and .rst files. This was a static review; simulation tests were not run.

Review time: 1m3s. Token usage: 398,358.

[AntoineRichard]

Please preserve the existing license notices in the moved _instruction_display.py.

Isaac Lab uses BSD-3-Clause by default, including isaaclab_teleop, while isaaclab_mimic and its corresponding standalone scripts use Apache-2.0 (documented license split). However, the original display module contains both Apache-2.0 and BSD-3-Clause notices, and the moved file removes the Apache notice and rewrites the copyright header.

Both notices were already present in the initial version I traced, 311d76e201, so I could not confirm the description's claim of a BSD-only header preceding the Apache header. Please retain the original notices during this move; if the existing header is believed to be incorrect, document the provenance supporting that correction.

[rwiltz]

Please preserve the existing license notices in the moved _instruction_display.py.

Isaac Lab uses BSD-3-Clause by default, including isaaclab_teleop, while isaaclab_mimic and its corresponding standalone scripts use Apache-2.0 (documented license split). However, the original display module contains both Apache-2.0 and BSD-3-Clause notices, and the moved file removes the Apache notice and rewrites the copyright header.

Both notices were already present in the initial version I traced, 311d76e201, so I could not confirm the description's claim of a BSD-only header preceding the Apache header. Please retain the original notices during this move; if the existing header is believed to be incorrect, document the provenance supporting that correction.

Thanks for catching that, you're right. I've restored the original Apache-2.0 and BSD-3-Clause notices verbatim and excluded this file from the BSD header hook (it's now handled like the mimic files), so the hooks keep them.

[rwiltz]

run-ci

[hougantc-nvda]

Do we still support teleoperation on Linux aarch64 with pip-installed Isaac Sim? If so, the switch to in-process dispatch may introduce a startup regression.

Previously, isaaclab teleop run and record went through run_python_command(), which uses _aarch64_libgomp_env() to preload the system OpenMP library. The new direct calls bypass that setup; replay still gets it. On ARM without a manually configured LD_PRELOAD, this could cause Isaac Sim startup to fail after Torch loads its bundled OpenMP runtime.

A focused base/head dispatch probe confirms the preload is omitted, but I have not reproduced an actual simulator startup failure on ARM. If this remains supported, could we verify an ARM launch or preserve the subprocess/re-exec bootstrap before importing Torch/Isaac Sim?

[rwiltz]

Do we still support teleoperation on Linux aarch64 with pip-installed Isaac Sim? If so, the switch to in-process dispatch may introduce a startup regression.

Previously, isaaclab teleop run and record went through run_python_command(), which uses _aarch64_libgomp_env() to preload the system OpenMP library. The new direct calls bypass that setup; replay still gets it. On ARM without a manually configured LD_PRELOAD, this could cause Isaac Sim startup to fail after Torch loads its bundled OpenMP runtime.

A focused base/head dispatch probe confirms the preload is omitted, but I have not reproduced an actual simulator startup failure on ARM. If this remains supported, could we verify an ARM launch or preserve the subprocess/re-exec bootstrap before importing Torch/Isaac Sim?

Good catch, thanks. Digging in, this isn't teleop-specific: _aarch64_libgomp_env() (#6458) only applies through run_python_command(), and since #6553 moved train/play in process they've had the same gap on pip installs (install CI sets LD_PRELOAD itself, so it doesn't show up there). Teleop now matches RL here. I'd rather fix it once in cli() for every in process command on aarch64, re-execing through run_python_command() when the system libgomp isn't preloaded, verified on an ARM machine, in a follow-up PR. @StafaH thoughts?

[StafaH]

This is just a copy of the scripts into the submodule? Will a cleanup/refactor happen afterwards to split the large files, consolidate the common code, and make it less script-like and more module-like logic? Also will the tests move and get refactored as well into the submodule?

[rwiltz]

This is just a copy of the scripts into the submodule? Will a cleanup/refactor happen afterwards to split the large files, consolidate the common code, and make it less script-like and more module-like logic? Also will the tests move and get refactored as well into the submodule?

Yes, intentionally a move for this PR: function bodies unchanged so the diff stays reviewable as renames. Follow-ups I have planned:

• pass parsed args explicitly instead of the module-level args_cli (like the rl backends)
• split teleop_replay_agent (~1.6k lines) and consolidate what the entrypoints duplicate (parser options, CloudXR profile resolution, device selection, RTX setup)
• drop the task-name special cases ("Lift"/"Reach") in favour of task config
• replay_demos: stop forcing Kit (its keys come from the visualizer windows now) and use the dataset's task when --task is omitted (gym.make currently gets None; same on main)

[rwiltz]

run-ci