Refactor ArenaEnvGraphSpec

[qianl-nv]

Summary

Unify into one ArenaEnvGraphSpec type cross agentic env gen pipeline; modify its nodes representation for agent compatibility

Detailed description

• Consolidate env graph spec into ArenaEnvGraphSpec and update it to be more agent-friendly.
• Replace the dual initial/linked graph representation with a single ArenaEnvGraphSpec that is the source of truth: it validates the schema, reads/writes YAML, applies CLI overrides, and converts to a live IsaacLabArenaEnvironment. Drop EnvironmentIntentSpec and IntentCompiler and have the generation agent emit ArenaEnvGraphSpec directly. IntentCompile error traces is now replaced with ArenaEnvGraphSpec model validation error traces.
• EnvironmentGenerationAgent returns the json so that if ArenaEnvGraphSpec parsing failures, the user can still manually edit the json to fix the agent output using the model validation error traces as guidance.
• Migrate robolab YAMLs, tests, runners, and the review GUI to the unified spec, and fix default-light injection so runtime-injected assets land in the scene.

Note: this breaks compatibility of existing env yaml. The yamls checked into the repo has all be converted.

[qianl-nv]

self review 1

[qianl-nv]

self review 2

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

This PR collapses the dual EnvironmentIntentSpec / IntentCompiler representation into a single ArenaEnvGraphSpec that owns schema validation, YAML I/O, CLI override application, and conversion to a live env, and has the generation agent emit that spec directly. It is a large but coherent refactor: the node model is flattened into typed AssetSpec / ObjectReferenceSpec / SpatialRelationSpec / TaskSpec, agent-only validation is cleanly split into spec_validation.py, dead modules (asset_matcher, intent_compiler, environment_intent_spec, graph_spec_utils) and their tests are removed, and the robolab YAMLs are migrated. The consolidation into one source-of-truth type reads well and the sim-agnostic layering (lazy sim imports, validators as pure asserts) is respected. Two small nits below; no blocking concerns.

Findings

🔵 arena_env_graph_spec.py:66 — _assert_asset_ids_unique is annotated -> None but returns seen, and the caller depends on the returned set. Fix the annotation to -> set[str].

🔵 arena_env_graph_conversion_utils.py:105 — ObjectReferenceSpec.prim_path is now optional, but conversion passes it directly into ObjectReference, which resolves the prim eagerly. An unresolved prim_path would fail with an opaque USD error; a boundary assert with a clear message would fail fast.

Test Coverage

Good — the invalid-spec/validation-trace path is covered by test_returns_none_with_validation_traces_on_invalid_spec, the rejection matrix in test_arena_env_graph_spec.py exercises the inline validators, and deleted-module tests were removed alongside their code. Minor gap: collect_agent_ready_task_validation_traces (the agent-ready / missing-required-param checks) does not appear to be exercised directly — a small unit test would lock in that behavior.

Verdict

Minor fixes needed

[greptile-apps]

Greptile Summary

This PR replaces the two-phase EnvironmentIntentSpec → ArenaEnvInitialGraphSpec → ArenaEnvGraphSpec pipeline with a single ArenaEnvGraphSpec that the LLM agent emits directly. The spec gains typed top-level fields (embodiment, background, objects, object_references, relations, tasks) instead of the heterogeneous nodes list, self-validates in a Pydantic model_validator, and is the sole source of truth for YAML round-tripping, CLI overrides, and environment building.

• Core model (arena_env_graph_spec.py, arena_env_graph_types.py): ArenaEnvGraphNodeSpec hierarchy and all state-spec/task-wiring structures removed; replaced by flat AssetSpec, ObjectReferenceSpec, SpatialRelationSpec, and TaskSpec. Validation logic from the deleted graph_spec_utils.py is re-implemented inline in ArenaEnvGraphSpec.
• Agent pipeline (environment_generation_agent.py, spec_validation.py, spec_io.py): IntentCompiler and EnvironmentIntentSpec are deleted; the agent targets ArenaEnvGraphSpec directly and returns (spec | None, raw_dict) so callers can write the raw agent output on parse failure.
• Runners and GUI (environment_generation_runner.py, generation_panel.py, robolab YAMLs): updated throughout to use the unified spec; all 15 robolab YAMLs migrated to the new schema.

Confidence Score: 3/5

Two conversion-layer gaps can crash at runtime with unhelpful errors rather than failing at spec-load time with a clear message.

The core unification is well-executed and significantly reduces complexity. prim_path=None reaching ObjectReference and an unordered object_reference list reaching the assets dict are both real defects on the agent-generated spec path, even though existing robolab YAMLs are unlikely to trigger them.

arena_env_graph_spec.py (_assert_asset_ids_unique annotation and object-reference ordering validation), arena_env_graph_conversion_utils.py (prim_path=None guard before ObjectReference instantiation)

Important Files Changed

Filename	Overview
isaaclab_arena/environments/arena_env_graph_spec.py	Core refactor: collapses ArenaEnvInitialGraphSpec + ArenaEnvGraphSpec into one unified model. Two issues: _assert_asset_ids_unique is annotated -> None but returns set[str], and object-reference parent ordering is no longer validated.
isaaclab_arena/environments/arena_env_graph_types.py	Drops ArenaEnvGraphNodeSpec hierarchy in favour of flat AssetSpec / ObjectReferenceSpec / SpatialRelationSpec / TaskSpec. ObjectReferenceSpec.prim_path is str
isaaclab_arena/environments/arena_env_graph_conversion_utils.py	Rewrites asset materialisation to use the new typed spec. Passes prim_path=None to ObjectReference when an object_reference has no resolved prim path, which will crash at runtime.
isaaclab_arena/agentic_environment_generation/environment_generation_agent.py	Agent now targets ArenaEnvGraphSpec directly. Returns (spec
isaaclab_arena/agentic_environment_generation/spec_validation.py	New module: try_parse_env_graph_spec wraps model_validate with friendly error formatting; collect_agent_ready_task_validation_traces checks task agent-readiness and required params.
isaaclab_arena/agentic_environment_generation/spec_io.py	Simplified from dual initial/linked file output to single write_env_graph_spec; adds write_env_graph_dict for invalid-spec fallback.
isaaclab_arena_examples/agentic_environment_generation/environment_generation_runner.py	Runner calls generate_spec once and handles the None spec case with trace output. --env_graph_spec_yaml comes from the base CLI parser. Task-warning traces not printed on success path.
isaaclab_arena_environments/cli.py	Moves add_cli_override_args inline from graph_spec_utils. Updates override apply logic to use registry_name field. Clean and correct.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User
    participant Agent as EnvironmentGenerationAgent
    participant Spec as ArenaEnvGraphSpec
    participant IO as spec_io
    participant Conv as arena_env_graph_conversion_utils
    participant Env as IsaacLabArenaEnvironment

    User->>Agent: generate_spec(prompt)
    Agent->>Agent: LLM call
    Agent->>Spec: model_validate(data)
    alt validation fails
        Spec-->>Agent: ValidationError traces
        Agent-->>User: (None, data) + last_validation_traces
        Agent->>IO: write_env_graph_dict(data)
    else validation passes
        Spec-->>Agent: ArenaEnvGraphSpec
        Agent->>Agent: collect_agent_ready_task_validation_traces
        Agent-->>User: (spec, data)
        User->>IO: write_env_graph_spec(spec)
        IO->>Spec: write_yaml(path)
    end

    User->>Spec: from_yaml(path)
    User->>Spec: to_arena_env()
    Spec->>Conv: build_arena_env_from_graph_spec(spec)
    Conv->>Conv: _instantiate_assets_from_spec
    Conv->>Conv: _ensure_scene_lighting
    Conv->>Conv: _attach_spatial_relations_to_assets
    Conv-->>Env: IsaacLabArenaEnvironment

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"}}}%%
sequenceDiagram
    participant User
    participant Agent as EnvironmentGenerationAgent
    participant Spec as ArenaEnvGraphSpec
    participant IO as spec_io
    participant Conv as arena_env_graph_conversion_utils
    participant Env as IsaacLabArenaEnvironment

    User->>Agent: generate_spec(prompt)
    Agent->>Agent: LLM call
    Agent->>Spec: model_validate(data)
    alt validation fails
        Spec-->>Agent: ValidationError traces
        Agent-->>User: (None, data) + last_validation_traces
        Agent->>IO: write_env_graph_dict(data)
    else validation passes
        Spec-->>Agent: ArenaEnvGraphSpec
        Agent->>Agent: collect_agent_ready_task_validation_traces
        Agent-->>User: (spec, data)
        User->>IO: write_env_graph_spec(spec)
        IO->>Spec: write_yaml(path)
    end

    User->>Spec: from_yaml(path)
    User->>Spec: to_arena_env()
    Spec->>Conv: build_arena_env_from_graph_spec(spec)
    Conv->>Conv: _instantiate_assets_from_spec
    Conv->>Conv: _ensure_scene_lighting
    Conv->>Conv: _attach_spatial_relations_to_assets
    Conv-->>Env: IsaacLabArenaEnvironment

Loading

Comments Outside Diff (1)

1. isaaclab_arena_examples/agentic_environment_generation/environment_generation_runner.py, line 151-155 (link)

   Task-validation warning traces are silently discarded when spec generation succeeds

   agent.last_validation_traces is populated even on success by collect_agent_ready_task_validation_traces (e.g. "Task 'X' is not agent-ready" or "missing required param"). The runner only prints these traces when env_graph_spec is None; when the spec parses successfully the traces are never surfaced. A generated env with a non-agent-ready task will load and run without any visible warning.

_{Reviews (1): Last reviewed commit: "Remove unused asset_matcher module and i..." | Re-trigger Greptile}

[alexmillane]

LGTM. Massive simplification!