Add EnvGraphSpec to ArenaEnv method

[xyao-nv]

Summary

Convert EnvGraph to ArenaEnv

Detailed description

• Adds ArenaEnvGraphSpec.to_arena_env() so YAML graph specs can be converted into IsaacLabArenaEnvironment.
• Resolves task classes at runtime and converts multi-task specs into a sequential composite task using all task entries.
• Includes nodes, state, task validation in to_yaml().

[isaaclab-review-bot]

Code Review

Thanks for adding the to_arena_env() method and the comprehensive conversion utilities! This is a substantial addition that bridges the gap between declarative graph specs and runtime environment instances. Below are my observations:

✅ Strengths

1. Clean API Design: The to_arena_env() method on ArenaEnvGraphSpec provides a natural, discoverable entry point. The TYPE_CHECKING guard for the import keeps the module dependency graph clean.

2. Extensible Task Resolution: The task factories mechanism allows custom task types while still supporting class name and module discovery fallbacks. The resolution order (import path → class name → module stem) is intuitive.

3. Well-Structured Utilities: The conversion logic is broken into focused helper functions (_select_state_spec, _instantiate_node_assets, _apply_state_spatial_constraints, etc.), making the code readable and testable.

4. Good Test Coverage: The new test case exercises sequential task composition and validates the full conversion pipeline through a realistic YAML fixture.

---

🔍 Suggestions & Questions

1. Consider documenting the task_args aliasing behavior
The _align_task_kwargs function does clever fuzzy matching of YAML keys to constructor parameters (lines 357-390). While useful for YAML ergonomics, this implicit behavior could surprise users when debugging. Consider either:

• Adding a note in docstrings about which aliases are supported, or
• Logging/warning when a fuzzy match is used

2. _discover_task_classes does package-wide scanning
If the direct module candidates fail, the fallback scans all modules under isaaclab_arena.tasks. This could become slow as the task library grows. Is caching viable here, or is this path rarely hit in practice?

3. Potential edge case in _match_task_arg_to_parameter
The substring matching (normalized_key in _normalize_identifier(name)) could yield unexpected results for short keys. For example, a task arg "x" would match any parameter containing the letter "x". The narrowing to required params helps, but it might be worth documenting or tightening this.

4. The IN spatial constraint raises NotImplementedError
Noticed that ArenaEnvGraphSpatialConstraintType.IN is defined but raises at conversion time. The TODO comment references May 21—is there a tracking issue for this, or should one be created?

5. Minor import ordering
In arena_env_graph_spec.py, the diff shows yaml import moved below from collections.abc which deviates from the typical stdlib-first ordering in this codebase. Not blocking, just a style nit.

---

🧪 Testing

The new test test_arena_env_graph_conversion_builds_sequential_pick_and_place_task is well-structured. One suggestion: consider adding a negative test for the IN constraint NotImplementedError to lock in the expected behavior until solver support lands.

---

Overall, this is a solid PR that adds valuable functionality. The suggestions above are minor refinements. Happy to discuss any of these points!

---

Update (53a0a44)

Thanks for the follow-up commit! This update addresses several important areas:

✅ What's Improved

1. API Simplification: Removed the task_factories parameter from to_arena_env() and build_arena_env_from_graph_spec(). This simplifies the public API—users now rely on task class discovery rather than custom factories.

2. Validation Before Instantiation: Added comprehensive pre-conversion validation via _validate_graph_spec_for_conversion():

   • _validate_spatial_constraint_shapes() ensures proper child node arity for each constraint type
   • _validate_task_arg_node_references() catches dangling node references in task args early
   • _validate_initial_state_supported_for_conversion() prevents unsupported IN constraints from reaching the runtime

3. Expanded Test Coverage: Three new validation tests added:

   • test_arena_env_graph_conversion_validates_references_before_instantiation
   • test_arena_env_graph_conversion_validates_relationship_shape_before_instantiation
   • test_arena_env_graph_conversion_validates_task_arg_node_references_before_instantiation

4. Code Consolidation:

   • Unified _normalize_task_class_name and _normalize_task_module_name into single _normalize_task_name()
   • Cleaned up the test data by consolidating two_pick_and_place_maple_table_env_graph.yaml into the existing test fixture
   • Tightened error messages for better debugging

5. TYPE_CHECKING Guards: Expanded use of TYPE_CHECKING imports in the conversion utils to keep the module lean at runtime.

🔍 Minor Observations

• The validation now catches my earlier suggestion #4 (the IN constraint issue) with a clear assertion before hitting the runtime NotImplementedError.
• Good use of early-exit validation pattern—fail fast with descriptive errors rather than cryptic runtime failures during asset instantiation.

🧪 Test Fixture Updates

The pick_and_place_maple_table_env_graph.yaml now includes the second task and third state spec inline, which makes the sequential task test self-contained. The removed two_pick_and_place_maple_table_env_graph.yaml was redundant.

---

No blocking concerns. This is a nice cleanup that improves both usability and debuggability. 👍

---

Update (f28226a): Reviewed 4 new commits (03410c8 → f28226a).

Changes Since Last Review

These commits are a significant cleanup and simplification pass:

1. API Simplification: Removed state_spec_id parameter from to_arena_env() — now always uses the first state spec. This is cleaner but removes flexibility; the TODO notes a future aggregation approach.

2. Refactored is_unary() in Relations: Moved is_unary() from RelationBase to individual subclasses (UnaryRelation, IsAnchor, RandomAroundSolution, RotateAroundSolution). ⚠️ Note: If external code subclasses RelationBase directly without implementing is_unary(), it will now fail with AttributeError. Since these relations are likely internal, this is probably fine, but worth a note in release/migration docs if public.

3. Removed Helper Indirection: _select_state_spec(), _select_embodiment(), and _relation_params_for_constraint() are gone — logic is inlined or moved upstream. The upstream contract now expects constraint.params to arrive in constructor-ready form (no more Side enum conversion or position_xyz unpacking).

4. Hoisted Imports: Several imports moved from inside functions to module-level. Trades laziness for clarity; should be fine given these modules are already in use when conversion runs.

5. Extensive Docstring Improvements: arena_env_graph_task_conversion_utils.py and graph_spec_utils.py now have detailed docstrings with examples. Nice for discoverability.

6. Test Adjustment: Removed test_arena_env_graph_conversion_validates_selected_initial_state_before_instantiation since the state_spec_id param no longer exists.

Assessment

No new bugs spotted. The simplification is reasonable — removing state_spec_id reduces API surface while the TODO signals the intended future direction. The is_unary() change is the most notable from a compatibility standpoint but appears safe for internal usage.

✅ New commits look good.

---

Update (4d10198): Reviewed incremental changes.

Changes

• Inlined helper functions: Removed _child_asset(), _add_relation(), _set_initial_pose() — logic now directly calls asset methods. Cleaner code, relies on upstream validation to catch issues early.
• Single-pass OBJECT_REFERENCE instantiation: Consolidated into main loop with documented upstream contract (nodes must be ordered parent-first). Avoids second iteration.
• Module-level imports: Hoisted isaaclab_arena.tasks, SequentialTaskBase, TaskBase imports.
• Minor: Removed unused ArenaEnvGraphSpatialConstraintSpec import and redundant tuple() wrapping.

Assessment

Continuation of the cleanup theme. No new issues introduced. ✅

---

Update (b0ae704): Reviewed incremental changes.

Changes

This commit is a major API simplification that removes the fuzzy/loose matching behavior:

1. Removed fuzzy task_args matching — _align_task_kwargs() and related helpers are gone. Task arg keys must now exactly match constructor parameter names.

2. Removed task type loose resolution — _normalize_task_name(), _task_module_candidates(), and module-stem matching removed. Task type: in YAML must now be the exact class name (e.g., PickAndPlaceTask) or a qualified import path.

3. Removed auto-injection of instance_name — upstream must now emit instance_name explicitly in node.params when needed.

4. Removed assert_task_arg_node_references_exist() validation — node reference validation now happens at runtime rather than spec construction.

5. Test fixtures updated to use exact names (pick_up_object instead of object, PickAndPlaceTask instead of pick_and_place).

Assessment

This directly addresses my earlier suggestions (#1-#3 from the original review) by removing the complexity entirely rather than documenting it. The strict upstream contract approach is cleaner and more predictable.

⚠️ Breaking change for existing YAML files that relied on loose matching — they'll need to use exact class/parameter names.

No new issues. ✅

---

Update (ced545a): Reviewed incremental changes.

Changes

This commit is a refactoring to break circular imports by extracting types into a dedicated module:

1. New arena_env_graph_types.py module: Contains all the pure-data schema types (enums and dataclasses) that were previously in arena_env_graph_spec.py:

   • Enums: ArenaEnvGraphNodeType, ArenaEnvGraphSpatialConstraintType, ArenaEnvGraphTaskConstraintType
   • Dataclasses: ArenaEnvGraphNodeSpec, ArenaEnvGraphObjectReferenceNodeSpec, ArenaEnvGraphSpatialConstraintSpec, ArenaEnvGraphTaskConstraintSpec, ArenaEnvGraphStateSpec, ArenaEnvGraphTaskSpec

2. arena_env_graph_spec.py slimmed down: Now imports/re-exports types via __all__ for backward compatibility. The build_arena_env_from_graph_spec import is hoisted to module level (removing the lazy import in to_arena_env()).

3. Import path updates: arena_env_graph_conversion_utils.py, arena_env_graph_task_conversion_utils.py, and graph_spec_utils.py now import types from the new arena_env_graph_types module.

4. Test fixture adjustments: Node indices updated in validation tests due to node reordering (background now at index 1, object_reference at index 2). Task type updated to exact name PickAndPlaceTask.

Assessment

Clean separation of concerns:

• Types module: Pure data (no behavior, no dependencies on conversion logic)
• Spec module: Parsing, validation, and conversion entry point

This breaks the potential circular import chain (conversion_utils → spec → conversion_utils) while maintaining backward compatibility via re-exports. No new issues. ✅

---

Update (f020c7f): Reviewed 1 new commit (ced545a → f020c7f).

Changes

This is a substantial infrastructure commit that introduces relation and task registries with discovery-time registration:

1. New Registry Infrastructure

• ObjectRelationLibraryRegistry and TaskRegistry added to registries.py
• Both extend the base Registry pattern with typed accessor methods
• ensure_assets_registered() now imports relations.relations and tasks modules to trigger decorator registration

2. Relation Registration System

• @register_object_relation decorator in register.py
• All relation classes (NextTo, On, IsAnchor, AtPosition, PositionLimits, RandomAroundSolution, RotateAroundSolution) now have a name class attribute and are registered
• is_unary() method added as @staticmethod on each relation class for proper polymorphic dispatch

3. Task Registration System

• @register_task decorator in register.py
• All task classes (PickAndPlaceTask, LiftObjectTask, NoTask, etc.) are now decorated with @register_task
• Registry keys use class __name__ directly (e.g., "PickAndPlaceTask")

4. Graph Spec Validation Enhancement

• assert_spatial_constraint_shapes() added to graph_spec_utils.py — validates that each spatial constraint has the correct parent/child arity based on the relation's is_unary() method
• relation_class_for_spatial_constraint_type() helper resolves enum to relation class via the new registry
• Validation is now called in ArenaEnvGraphSpec.validate() and invoked automatically in from_dict()

5. Relation Solver Interface Extraction

• New relation_solver_interface.py module — extracts placement orchestration logic from ArenaEnvBuilder._solve_relations()
• solve_and_apply_relation_placement() is the new public API entry point
• PooledObjectPlacer property pool_size added for introspection

6. Lightwheel Timeout/Retry Wrapper

• New lightwheel_utils.py module with acquire_lightwheel_asset() helper
• Wraps Lightwheel SDK calls with configurable timeout, retry attempts, and delay between retries
• Applied to RobocasaKitchenBackground and several object_library.py assets

7. Documentation Updates

• Static apple workflow docs significantly expanded with dual-viewport monitoring, pre-trained model downloads, and single-GPU training config
• Links updated to pinned GR00T commit SHAs
• gh-pages.yml simplified to only build from main branch

Assessment

Strengths:

• The registry approach enables the graph-spec-to-env conversion to resolve task/relation types at runtime without import-time coupling
• assert_spatial_constraint_shapes() provides excellent fail-fast validation — catches arity errors at YAML load time
• Lightwheel retry wrapper is a pragmatic addition for flaky network calls
• Documentation improvements are substantial and user-friendly

Minor Observations:

• The _assets_registered flag is now set before imports (line 281) to handle re-entrant decorator calls — good defensive programming, but the rollback-on-exception pattern adds complexity. Consider adding a comment explaining when this can actually trigger.
• relation_class_for_spatial_constraint_type() returns None for AT_POSE and IN — the caller must handle these special cases. The TODO notes are helpful here.

Tests:

• New tests for TaskRegistry, relation_solver_interface, and Lightwheel retry are comprehensive
• test_arena_env_graph_spec.py updated to validate the new relation_class_for_spatial_constraint_type() behavior

No blocking issues. This is excellent infrastructure work that sets up clean extensibility for future task and relation types. ✅

---

Update (48f0be3): Reviewed incremental changes (4b56480 → 48f0be3).

Changes

This is a substantial commit adding several important features:

1. CompositeTaskBase — Order-Independent Composite Tasks

A new base class CompositeTaskBase is introduced alongside the existing SequentialTaskBase:

• Order-independent completion: Unlike SequentialTaskBase which requires subtasks to complete in sequence (state machine), CompositeTaskBase allows subtasks to complete in any order
• SequentialTaskBase now inherits from CompositeTaskBase, overriding only composite_task_success_func and reset_subtask_success_state to add ordering logic
• Both support desired_subtask_success_state with None entries for "don't care" positions
• Comprehensive test coverage in test_composite_task_base.py and test_composite_open_door.py

2. MetricsManager — Manager-Based Metrics Infrastructure

A significant refactor of the metrics system:

• New MetricsManager class in metrics_manager.py — follows the IsaacLab manager pattern
• New MetricTermCfg configclass — decouples metric configuration from recorder terms
• MetricBase.get_metric_term_cfg() replaces compute_metric_from_recording() — metrics now return a config rather than computing directly
• IsaacLabArenaManagerBasedRLEnv split into env + cfg modules:
  • isaaclab_arena_manager_based_env.py → env class with load_managers() and compute_metrics()
  • isaaclab_arena_manager_based_env_cfg.py → config class with ArenaPhysicsCfg
• All metrics (SuccessRateMetric, ObjectMovedRateMetric, RevoluteJointMovedRateMetric, SubtaskSuccessRateMetric) updated to the new pattern

3. Task Registry Infrastructure Completion

• task_library.py added — mirrors object_library.py pattern for lazy task class registration
• All task classes now decorated with @register_task
• TaskRegistry().get_task_by_name() enables graph-spec task resolution by class name
• New test test_task_registry.py validates all expected tasks are discoverable

4. Graph Spec Enhancements

• ArenaEnvGraphSpec.validate() extracted as public method — allows post-mutation validation
• Parent node ordering validation — nodes must be ordered parent-before-child (matches instantiation order)
• assert_spatial_constraint_shapes() now validates arity at parse time
• Tests expanded to cover validation after mutation scenarios

5. Task Conversion Utilities

• New arena_env_graph_task_conversion_utils.py — dedicated module for task spec → task instance conversion
• find_node_ref_params_in_signature() introspects task __init__ for Asset / AffordanceBase typed params
• _resolve_node_refs_in_task_args() swaps node-id strings for live assets at instantiation time
• Comprehensive test coverage in test_arena_env_graph_task_conversion.py

6. Minor Fixes & Cleanup

• Pose rotation field: rotation_wxyz → rotation_xyzw in compile_env_notebook.py
• PickAndPlaceTask.force_threshold: Default changed from 1.0 → 0.1
• Import path fixes: IsaacLabArenaManagerBasedRLEnvCfg imports updated across codebase
• Removed compute_metrics() function from metrics.py — replaced by manager method
• relation_class_for_spatial_constraint_type() now exposed in graph_spec_utils.py
• Sequential task tests refactored to use direct mocks rather than full env simulation

Assessment

Strengths:

• The CompositeTaskBase / SequentialTaskBase inheritance hierarchy is elegant — shared logic in base, ordering constraint in subclass
• The metrics refactor brings Arena into alignment with IsaacLab's manager-based architecture
• Task registry enables fully declarative task instantiation from YAML specs
• Excellent test coverage for the new composite/sequential success logic, including edge cases with None in desired_subtask_success_state

Minor Observations:

• The force_threshold default change (1.0 → 0.1) could affect existing pick-and-place evaluations — may warrant a note
• The rotation_wxyz → rotation_xyzw fix in the notebook suggests there may be other places using the wrong convention (worth a grep to verify)

No blocking issues. This is a substantial and well-structured addition to the Arena infrastructure. ✅

---

Update (bcb3b2e): Reviewed incremental changes (48f0be3 → bcb3b2e).

Changes

This commit is a final refactoring pass with significant code reorganization:

1. Module Extraction — Types and Conversion Utilities Split

• arena_env_graph_types.py — Now a standalone module containing all pure-data schema classes. The docstring explicitly states this is to break circular imports between spec and conversion modules.
• arena_env_graph_task_conversion_utils.py — New module dedicated to task-spec → live-task resolution. Contains build_task_from_specs(), find_node_ref_params_in_signature(), and the node-id resolution logic.
• arena_env_graph_conversion_utils.py — Slimmed down to focus on env/scene/asset instantiation; task conversion delegated to the new module.

2. Relation is_unary() as @staticmethod

All relation classes now declare is_unary() as a @staticmethod returning a literal bool:

• UnaryRelation, IsAnchor, RandomAroundSolution, RotateAroundSolution → return True
• Relation (binary base) → return False

This enables assert_spatial_constraint_shapes() to call relation_class.is_unary() without instantiating the class.

3. Enhanced Validation in graph_spec_utils.py

• assert_references_exist() now enforces parent-before-child ordering — nodes must be declared in topological order so the conversion single-pass instantiation works.
• assert_spatial_constraint_shapes() validates parent/child arity for each constraint using the new is_unary() dispatch, with special-case handling for AT_POSE and IN.

4. Lazy Import in relations.py

RotateAroundSolution.get_rotation_xyzw() now imports torch inside the method body instead of at module level. This keeps the schema modules torch-free for lightweight consumers.

5. Comprehensive Test Additions

• test_arena_env_graph_conversion.py — End-to-end subprocess test for SimulationApp-requiring conversion. Deliberately isolated from test_arena_env_graph_spec.py to avoid pxr import conflicts.
• test_arena_env_graph_task_conversion.py — Unit tests for the new task conversion utilities: signature introspection, node-id resolution, sequential task wrapping.
• test_arena_env_graph_spec.py — Two new tests for post-mutation validate() calls, ensuring programmatically modified specs can be re-validated.

6. Documentation Updates

• step_2_teleoperation.rst and step_3_policy_training.rst — Simplified language instruction from verbose ("Pick up the apple from the shelf...") to concise ("move the apple to the plate").
• g1_static_apple_config.yaml and g1_static_apple_gr00t_closedloop_config.yaml — Updated to match the new instruction.

Assessment

This is a clean wrap-up commit that:

1. Breaks circular imports with proper module separation
2. Makes is_unary() a class-level (not instance-level) property
3. Adds defensive validation for node ordering
4. Includes comprehensive test coverage for the new task conversion logic
5. Aligns documentation/config with simpler language instructions

No blocking issues. ✅

[qianl-nv]

Thanks @xyao-nv for the draft. Overall looks ok, but isaaclab_arena/environments/arena_env_graph_conversion_utils.py is a long file and could potentially be clean up to make it simpler. Especially in terms of spec validation, many checks should happen at the spec construction step, not at the spec->arena env conversion step.

[xyao-nv]

Changes made:

• Add is_unary() to Relation and RelationBase
• Add a spatial constraints lookup table to auto populate Relation class objects
• Move some validation related steps in graph_spec_utils.py

[alexmillane]

A couple of nits, but my main comment is that we should add registries for tasks and constraints to avoid some stuff in here that makes the graph decoding not extensible out-of-source.

[alexmillane]

Could of nits.

LGTM!

[greptile-apps]

Greptile Summary

This PR adds ArenaEnvGraphSpec.to_arena_env() to convert YAML env-graph specs into live IsaacLabArenaEnvironment objects. It extracts the spec dataclasses into a new arena_env_graph_types.py, introduces two conversion utility modules, adds is_unary() to relation classes, and extends validation with assert_spatial_constraint_shapes.

• New conversion pipeline: to_arena_env() calls build_arena_env_from_graph_spec, which materialises assets, attaches spatial-constraint relations, partitions embodiment from scene assets, and builds either a single task or a SequentialTaskBase for multi-task specs.
• Multi-task support: build_task_from_specs wraps multiple task instances in a SequentialTaskBase with all subtasks required to succeed.
• Validation improvements: assert_spatial_constraint_shapes checks parent/child shape correctness at parse time; assert_references_exist now enforces parent-before-child ordering.

Confidence Score: 2/5

Not safe to merge: moving PoseRange to TYPE_CHECKING breaks RandomAroundSolution.to_pose_range_centered_at at runtime.

The PoseRange import in relations.py was moved into the TYPE_CHECKING block, but the class is instantiated as return PoseRange(...) inside RandomAroundSolution.to_pose_range_centered_at. At runtime TYPE_CHECKING is False, so PoseRange is never defined and every call to that method raises NameError. The rest of the conversion pipeline is well-structured, but this regression in a commonly-used relation utility needs to be fixed before merging.

isaaclab_arena/relations/relations.py — the PoseRange import regression. isaaclab_arena/environments/arena_env_graph_conversion_utils.py — missing zero-embodiment guard in _partition_nodes_into_embodiment_and_scene.

Important Files Changed

Filename	Overview
isaaclab_arena/relations/relations.py	PoseRange moved to TYPE_CHECKING-only import but still used as a runtime constructor in RandomAroundSolution.to_pose_range_centered_at — causes NameError when the method is called.
isaaclab_arena/environments/arena_env_graph_conversion_utils.py	New conversion module; _partition_nodes_into_embodiment_and_scene asserts at-most-one embodiment but never asserts at-least-one, so a graph with no embodiment silently passes embodiment=None to IsaacLabArenaEnvironment.
isaaclab_arena/environments/arena_env_graph_task_conversion_utils.py	New task-conversion module; signature introspection and node-ref resolution look correct; null task-arg values for optional Asset params raise an unhelpful AssertionError rather than passing None through.
isaaclab_arena/environments/arena_env_graph_spec.py	Refactored to import types from the new arena_env_graph_types module; validate() extracted from from_dict; to_arena_env() added with lazy import; re-exports all for backward compatibility.
isaaclab_arena/environments/graph_spec_utils.py	Adds assert_spatial_constraint_shapes, ordering check in assert_references_exist, and utility helpers; logic looks correct.
isaaclab_arena/environments/arena_env_graph_types.py	Pure data-class extraction from arena_env_graph_spec.py; code is identical to what was removed from that module, no logic changes.
isaaclab_arena/tests/test_arena_env_graph_conversion.py	New end-to-end test for the full graph to env conversion; correctly spawned in a subprocess to avoid SimulationApp/pxr ordering issues.
isaaclab_arena/tests/test_arena_env_graph_task_conversion.py	Comprehensive unit tests for task-conversion utilities covering all annotation shapes, resolution, and error cases.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["ArenaEnvGraphSpec.from_yaml / from_dict"] --> B["validate()"]
    B --> C["assert_unique_ids"]
    B --> D["assert_references_exist + parent-before-child ordering"]
    B --> E["assert_spatial_constraint_shapes (new)"]

    F["ArenaEnvGraphSpec.to_arena_env()"] --> G["build_arena_env_from_graph_spec"]
    G --> H["_instantiate_assets_from_nodes"]
    G --> I["_attach_spatial_constraints_to_assets"]
    G --> J["_partition_nodes_into_embodiment_and_scene"]
    J --> J1{"EMBODIMENT count"}
    J1 -->|"2 or more"| J2["AssertionError"]
    J1 -->|"0 - silent bug"| J3["embodiment = None"]
    J1 -->|"exactly 1"| J4["OK"]
    G --> K["build_task_from_specs"]
    K --> L{"task count"}
    L -->|"0"| M["None"]
    L -->|"1"| N["single task instance"]
    L -->|"more than 1"| O["SequentialTaskBase"]
    G --> P["IsaacLabArenaEnvironment"]

Loading

Comments Outside Diff (1)

1. isaaclab_arena/relations/relations.py, line 30-38 (link)

   RelationBase does not define is_unary(), but both assert_spatial_constraint_shapes and _attach_spatial_constraints_to_assets call relation_cls.is_unary() on any registered class whose static type is type[RelationBase]. A third-party or future class that inherits RelationBase directly without implementing is_unary() will raise AttributeError at conversion time. Defining an abstract version on RelationBase enforces the contract at the hierarchy root.

_{Reviews (1): Last reviewed commit: "bug fix" | Re-trigger Greptile}

[greptile-apps]

Runtime NameError on to_pose_range_centered_at: PoseRange was moved to the TYPE_CHECKING block, but it is used as a constructor in the function body (return PoseRange(...)). Because TYPE_CHECKING is False at runtime, PoseRange is never bound as a name, so any call to this method will raise NameError: name 'PoseRange' is not defined. The from __future__ import annotations only makes the -> PoseRange return annotation lazy — it does not protect the runtime constructor call.

Suggested change

	if TYPE_CHECKING:
	from isaaclab_arena.assets.object_base import ObjectBase
	from isaaclab_arena.utils.pose import PoseRange
	from isaaclab_arena.utils.pose import PoseRange
	if TYPE_CHECKING:
	from isaaclab_arena.assets.object_base import ObjectBase

[greptile-apps]

Missing assertion for zero-embodiment graph specs: the docstring states "Asserts exactly one EMBODIMENT node," but the code only asserts at-most-one. When no EMBODIMENT node is present, embodiment remains None and is silently passed to IsaacLabArenaEnvironment, which will fail downstream with an opaque error.

Suggested change

	return embodiment, scene_assets
	assert embodiment is not None, "Exactly one embodiment node is required to convert a graph spec to an IsaacLabArenaEnvironment"
	return embodiment, scene_assets

[greptile-apps]

Explicit null task-arg value for an optional node-ref param is not handled: if a YAML spec contains optional_object: null for an Asset | None-typed parameter, raw_param_value is None, param_name in raw_task_args is True, and _lookup_asset_by_node_id(None, ...) raises AssertionError: unknown node id None instead of passing None through.

Suggested change

	else:
	# Asset-typed param: resolve the single node id to its live asset.
	# e.g. "pick_up_object": "cube" -> "pick_up_object": <Object: cube>
	resolved_task_kwargs[param_name] = _lookup_asset_by_node_id(
	raw_param_value, assets_by_node_id, task_class, param_name
	)
	else:
	# Asset-typed param: resolve the single node id to its live asset.
	# e.g. "pick_up_object": "cube" -> "pick_up_object": <Object: cube>
	# None is a valid explicit value for optional (Asset | None) params.
	if raw_param_value is None:
	resolved_task_kwargs[param_name] = None
	else:
	resolved_task_kwargs[param_name] = _lookup_asset_by_node_id(
	raw_param_value, assets_by_node_id, task_class, param_name
	)