[Task Clean-up][Manager] Dexterous Part 6/9: Add the handover and camera manager counterparts

[hujc7]

Review Map

• Exact changes: a stacked PR's page shows the cumulative diff of its dependency chain; the link pins the commit range that is the PR's own contribution.
• Links pin specific SHAs and go stale after a branch update. Parts 7–9 are pending a rebase onto the current stack, so their ranges are omitted rather than shown wrong.

PR	Status	Depends on	Exact changes
#6411 Part 1/9: Newton cloner/cubric/visualizer fixes		—	merged
#6412 Part 2/9: OVPhysX articulation + manager runtime		—	merged
#6413 Part 3/9: Reorient Direct, torch		—	merged
#6414 Part 4/9: MARL-to-single-agent fix + handover/camera Direct		—	merged
#6418 Part 5/9: Reorient manager counterparts		—	—
📌 #6421 Part 6/9: Handover + camera manager counterparts		#6418	changes
#6410 Part 7/9: Environment overview docs		#6421	pending rebase
#6415 Part 8/9: Benchmark success-rate utilities		#6421	pending rebase
#6582 Part 9/9: Warp variants → experimental (draft; merges last)		—	pending rebase
#6324 [DO-NOT-MERGE] Lumped validation reference		ALL	—

Summary

Completes manager coverage of the dexterous task families by adding manager-based counterparts
for the Shadow handover and Shadow camera reorientation tasks. Stacks on
#6418 — [Task Clean-up][Manager] Dexterous Part 5/9: Add the reorientation manager counterparts

• 2 manager tasks added: Isaac-Handover-Shadow and Isaac-Reorient-Cube-Shadow-Camera.
• The last 2 -Play registrations in isaaclab_tasks are removed.

1. Handover manager counterpart

The fused handover reward became a plain reward term, with success and goal-distance
bookkeeping moved to HandoverCommand, which owns the goal. It reports Metrics/success_rate as
a per-episode bit, matching the handover Direct environment. A value-parity test covers the
handover pair alongside the reorientation ones.

HandoverCommand.command builds its pose per call rather than returning a persistent buffer, so
consumers that store it across steps are not aliased to live data.

2. Camera manager counterpart

The camera task runs on PhysX by default: the RTX render modalities require Fabric cloning,
which Newton does not support, so the inherited default could not render. Newton remains
selectable with physics=newton_mjwarp for the state-only observation groups.

The feature-extractor observation term ignored its declared feature_extractor_cfg parameter
and read the environment configuration instead; it now honours the parameter.

3. Camera playback moves to play_mode

Isaac-Reorient-Cube-Shadow-Camera-Play and -Camera-Direct-Play were the last -Play
registrations left in isaaclab_tasks; every other task moved to play_mode in
#6676 — Remove the _PLAY configs. Their playback
environment count and inference feature extractor now live in play_mode overrides on the
manager and Direct camera configurations.

The override mutates the feature extractor rather than replacing it. Replacing the object
resets every field the caller does not name, which silently re-enabled the CNN in
ShadowHandCameraBenchmarkEnvCfg — a configuration whose entire purpose is to disable it.

4. Reorientation action configuration

The action term is named through a module path, so loading a task configuration no longer
imports the USD bindings.

Validation

Handover manager reaches a success rate of 0.859 against the Direct task's 0.667 over 1500
iterations at seed 42. The camera task's registered-rendering matrix and golden images are
tracked separately.

[greptile-apps]

Greptile Summary

This PR adds the manager-based counterpart for the two-hand Shadow Hand handover task (Isaac-Shadow-Handover), completing Part 11 of the dexterous task clean-up series. It introduces a new MDP term set (commands, events, observations, rewards, terminations), wires them into a HandoverManagerEnvCfg, and lands the family-wide OVPhysX acceptance test and handover configuration tests that depend on all prior parts being merged.

• The new manager env faithfully mirrors the Direct MARL adapter: scene/physics configs are reused from HandoverEnvCfg via a module-level _DIRECT_CFG sentinel, reward scaling compensates for the RewardManager's step_dt multiplication, and the _handover_reset_actions side-channel threads pre-reset actions into observations at episode boundaries.
• The mdp/__init__.py uses lazy_export() with a .pyi stub that declares from isaaclab.envs.mdp import *; this correctly sets up a lazy fallback for base MDP terms like EMAJointPositionToLimitsActionCfg, consistent with the reorient task's established pattern.
• CI is expected to stay red until the parent PRs ([Task Clean-up] Dexterous Part 3/9: Add success-rate metrics to the reorientation Direct tasks #6413, [Task Clean-up] Dexterous Part 4/9: Fix MARL-to-single-agent training and enable handover Direct RSL-RL #6414, [Task Clean-up][Manager] Dexterous Part 5/9: Add the reorientation manager counterparts #6418, [Task Clean-up][OVPhysX] Dexterous Part 7/11: Add OVPhysX presets to the dexterous tasks #6417, [Task Clean-up][Manager] Dexterous Part 9/11: Add Shadow state and OpenAI manager counterparts #6419, [Task Clean-up][Manager][Camera] Dexterous Part 10/11: Add the Shadow camera manager counterpart #6420) merge.

Confidence Score: 4/5

The change adds new files only (plus a two-line extension to init.py); no existing behavior is modified. The new env config correctly delegates scene/physics to the Direct base, and the MDP terms are consistent in frame conventions.

All newly added files follow the established lazy_export + PresetCfg pattern from the reorient task family. Frame conventions (local vs world) are consistent across commands, events, observations, and rewards. The single finding — storing two log values as zero-dim tensors while adjacent keys use .item() — is an inconsistency that could silently break a strict logging backend but does not affect training correctness.

source/isaaclab_tasks/isaaclab_tasks/core/handover/mdp/rewards.py — the dist_reward/dist_goal logging inconsistency noted above.

Important Files Changed

Filename	Overview
source/isaaclab_tasks/isaaclab_tasks/core/handover/handover_manager_env_cfg.py	New manager-based handover env config; delegates all scene/physics/action settings to the Direct base config via module-level _DIRECT_CFG and PresetCfg presets. Clean structure with no logic bugs found.
source/isaaclab_tasks/isaaclab_tasks/core/handover/mdp/rewards.py	HandoverReward class computes sticky episode success and logs diagnostics; dist_reward and dist_goal are stored as zero-dim tensors without .item(), unlike the Metrics/* keys. Reward scaling via /step_dt is intentional and explained in comments.
source/isaaclab_tasks/isaaclab_tasks/core/handover/mdp/commands.py	HandoverCommand samples fixed goal position from object default_root_pose plus offset and randomizes X/Y orientation per episode; consistent with Direct env's distribution. Goal position is correctly in local/env frame throughout.
source/isaaclab_tasks/isaaclab_tasks/core/handover/mdp/events.py	reset_handover_state mirrors Direct env's reset distribution for object pose/velocity and both hands; uses env._handover_reset_actions side-channel to thread pre-reset actions into observations cleanly.
source/isaaclab_tasks/isaaclab_tasks/core/handover/mdp/observations.py	All seven observation terms are frame-consistent (local/env frame for positions, world frame for velocities which are origin-invariant). object_goal returns a 24-dim block matching the Direct env layout.
source/isaaclab_tasks/isaaclab_tasks/core/handover/mdp/terminations.py	Two simple termination terms: height-based fall detection and direct_timeout that exactly matches the Direct MARL boundary (>= max_episode_length - 1).
source/isaaclab_tasks/isaaclab_tasks/core/handover/init.py	Registers new Isaac-Shadow-Handover manager env and adds rsl_rl_cfg_entry_point to the existing Direct registration.
source/isaaclab_tasks/test/core/test_handover_env_cfg.py	New config tests verify asset reuse, pose consistency across backends, and Newton distal-joint override. Tests look correct and cover the key structural invariants.
source/isaaclab_tasks/test/core/test_dexterous_ovphysx_presets.py	Family-wide OVPhysX acceptance test covering 15 dexterous variants including the new HandoverManagerEnvCfg; also tests Newton presets for allegro/handover variants.
source/isaaclab_tasks/test/benchmarking/configs.yaml	Adds Isaac-Shadow-Handover benchmark row with same thresholds as the existing Direct variant (max_iterations=3000, reward>=1000, episode_length>=150).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ManagerBasedRLEnv\nIsaac-Shadow-Handover] --> B[HandoverManagerEnvCfg]

    B --> C[HandoverManagerSceneCfg\nPresetCfg: physx / newton_mjwarp / ovphysx]
    C --> C1[_HandoverManagerSceneCfg\nright_hand / left_hand / object]
    C1 --> C2[_DIRECT_CFG.right_robot_cfg\n_DIRECT_CFG.left_robot_cfg]

    B --> D[ActionsCfg\nEMAJointPositionToLimitsActionCfg\nright_hand + left_hand]
    B --> E[CommandsCfg\nHandoverCommand\nfixed pos + random X/Y orientation]
    B --> F[ObservationsCfg\nPolicyCfg: right 157-dim + left 157-dim]
    B --> G[EventCfg\nreset_handover_state]
    B --> H[RewardsCfg\nHandoverReward\n2x exp reward / step_dt]
    B --> I[TerminationsCfg\nobject_below_height + direct_timeout]

    G -->|env._handover_reset_actions| J[hand_action obs term]
    H -->|sticky success flag| K[Metrics/success_rate at reset]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[ManagerBasedRLEnv\nIsaac-Shadow-Handover] --> B[HandoverManagerEnvCfg]

    B --> C[HandoverManagerSceneCfg\nPresetCfg: physx / newton_mjwarp / ovphysx]
    C --> C1[_HandoverManagerSceneCfg\nright_hand / left_hand / object]
    C1 --> C2[_DIRECT_CFG.right_robot_cfg\n_DIRECT_CFG.left_robot_cfg]

    B --> D[ActionsCfg\nEMAJointPositionToLimitsActionCfg\nright_hand + left_hand]
    B --> E[CommandsCfg\nHandoverCommand\nfixed pos + random X/Y orientation]
    B --> F[ObservationsCfg\nPolicyCfg: right 157-dim + left 157-dim]
    B --> G[EventCfg\nreset_handover_state]
    B --> H[RewardsCfg\nHandoverReward\n2x exp reward / step_dt]
    B --> I[TerminationsCfg\nobject_below_height + direct_timeout]

    G -->|env._handover_reset_actions| J[hand_action obs term]
    H -->|sticky success flag| K[Metrics/success_rate at reset]

Loading

_{Reviews (1): Last reviewed commit: "Add the Shadow handover manager counterp..." | Re-trigger Greptile}

[greptile-apps]

Mixed tensor/scalar types in extras log dict. dist_reward and dist_goal are stored as zero-dim PyTorch tensors (.mean() without .item()), while the Metrics/* keys on the very next lines use .item() to produce Python floats. A logging backend that serializes extras["log"] to JSON or passes values to a scalar summarizer will silently fail or produce unexpected output for those two keys.

Suggested change

	env.extras.setdefault("log", {})["dist_reward"] = per_agent_reward.mean()
	env.extras["log"]["dist_goal"] = goal_distance.mean()
	env.extras["log"]["Metrics/goal_distance"] = goal_distance.mean().item()
	env.extras.setdefault("log", {})["dist_reward"] = per_agent_reward.mean().item()
	env.extras["log"]["dist_goal"] = goal_distance.mean().item()
	env.extras["log"]["Metrics/goal_distance"] = goal_distance.mean().item()

[AntoineRichard]

[AI Review][Important] The Manager camera task still defaults to an unsupported backend. Resolving ShadowHandCameraManagerEnvCfg() selects NewtonCfg with clone_in_fabric=False. The latest commit documents that the default RTX modalities require Fabric cloning, but its PhysX-default override was applied only to the Direct config. The new Manager and Play registrations therefore remain broken by default. Please default the Manager scene plus its sim-physics, robot, and object presets to PhysX, keep explicit newton_mjwarp selection available, and add the Manager task to the registered-camera rendering matrix so this configuration is exercised.

[AntoineRichard]

[AI Review][Important] This timeout is one control step later than the Direct task. The generic Manager time_out returns episode_length_buf >= max_episode_length, while HandoverEnv._get_dones() uses >= max_episode_length - 1. For max_episode_length == 450, buffer value 449 terminates Direct but not Manager, changing rollout boundaries and reset observations despite this config promising Direct parity. Please add a Direct-compatible handover timeout term (or share the boundary implementation) and cover 448/449/450 explicitly.

[AntoineRichard]

[AI Review][Important] The Direct camera module rename needs a deprecation shim. This PR changes the registered entry point to shadow_hand_direct_camera_env, but the previously documented shadow_hand_camera_env and shadow_hand_camera_env_cfg modules are deleted and no longer importable. That is a breaking public import-path change without the deprecation required by the repository guidelines; docs/source/overview/environments.rst also still links the old path. Please retain compatibility modules that warn and re-export the renamed symbols, and add migration guidance to the changelog before removing them in a later release.

[AntoineRichard]

[AI Review][Moderate] This comment claims a safeguard that is not present on this branch. There is no checked-in Direct-vs-Manager value-parity test for handover; the available dexterous test covers math helpers only. The duplicated task values have already drifted at the timeout boundary. Please either centralize the shared task constants/configuration or land the claimed parity tests in this PR. The source comment itself should describe the functional setting, not the review/test process.

[AntoineRichard]

[AI Review][Moderate] The declared feature_extractor_cfg term parameter is silently ignored. CameraPolicyCfg supplies and rewires this parameter, but this constructor reads env.cfg.feature_extractor instead and __call__ deletes the declared argument. Consequently, configuring the observation term directly has no effect, and the later docstring claim that the term owns its copy is inaccurate. Please construct from cfg.params["feature_extractor_cfg"], or remove the parameter and all associated rewiring so there is only one explicit source of truth.

[AntoineRichard]

[AI Review][Moderate] Please reduce this to durable functional documentation. This docstring is largely a tuning diary: comparisons with another backend, specific MAPPO rewards, the search history for gains, and historical asset names. Those details will age quickly and obscure the actual contract. Keep a concise description of what each actuator override controls, its units, and the backend constraint it addresses; move experiment results and rejected values to the PR or a design note. The same standard should be applied to the num_substeps benchmark anecdote below.

[AntoineRichard]

[AI Review] Review summary — changes requested before showroom-quality merge.

I left six inline findings: three Important correctness/compatibility issues and three Moderate duplication/configuration/comment-quality issues. The principal blockers are the unsupported Newton default on the new Manager camera task, the one-step Direct/Manager handover timeout mismatch, and removal of documented camera module paths without a deprecation shim.

Verification performed on head ba3e66c: ./isaaclab.sh -f passed all hooks; focused dexterous-math and MARL-adapter tests passed (24 tests). I also reproduced the Manager camera default as NewtonCfg with clone_in_fabric=False, the 449/450 timeout boundary mismatch, and failure to resolve both old camera module paths. Full Isaac Sim/GPU training was not run, and the PR currently has no substantive CI result beyond the labeler.

The existing Greptile suggestion to call .item() on per-step log tensors should not be applied: zero-dimensional device tensors follow the Manager logging convention, while .item() would force a host synchronization on every environment step.