Add script for e2e agentic scene gen

[qianl-nv]

Summary

Add environment_generation_runner for running the agentic environment generation end to end

Detailed description

• What was the reason for the change?
  We need an entry point to run the full agentic env gen pipeline for integration testing.

• What has been changed?
  A new script is added to run the agentic env gen. It does the following steps:

  • Takes user prompt from CLI, pass through EnvGenAgent to generate EnvIntentSpec
  • EnvIntentSpec compiles to UnresolvedArenaEnvGraphSpec (asset match into registry), then ArenaEnvGraphSpec (constraints populated). The final ArenaEnvGraphSpec serialized to yaml.
  • ArenaEnvGraphSpec is loaded back from yaml, and build into an ArenaEnvironment
  • Run zero_action policy simulation for visualizing output (with RelationSolver initializing layouts)

• What is the impact of this change?

  • The env generation agent is harden to reduce server side failure rate from ~30% to <1%.
  • Small fixes are added to the agent: add anchor relation, and add lights, and the generate arena env is valid & can be visualized correctly in sim.
  • Known limitations: no support for ObjectSet or ObjectReference. No user in the loop editing.

To run:
python -m isaaclab_arena.agentic_environment_generation.environment_generation_runner --num_envs 16 --env_spacing 1.5 --num-steps 2000 --viz kit --prompt "franka pick up avocado from the maple table and place it into a bowl on the table. there are other veggies on the table as distractor."

[isaaclab-review-bot]

Code Review: Add script for e2e agentic scene gen

Nice work integrating the full agentic environment generation pipeline! The end-to-end runner is well-structured, and the fixes for anchor relations, dome lighting, and reasoning_content handling address real issues. A few observations below:

---

🔴 High Priority

1. Overly broad exception catch in retry loop (environment_generation_agent.py)

except (json.JSONDecodeError, Exception) as exc:
    last_exc = exc

(json.JSONDecodeError, Exception) is equivalent to just except Exception since every exception is an Exception. This means non-retryable errors (e.g. pydantic.ValidationError indicating a genuine schema mismatch, TypeError, or even KeyboardInterrupt propagating) will be silently swallowed and retried. Consider narrowing to:

except (json.JSONDecodeError, pydantic.ValidationError) as exc:
    last_exc = exc

This way, truly unexpected errors still bubble up immediately rather than wasting retry budget.

2. assert statements are not retried — inconsistent error handling

The assert choices and assert route != "empty" statements inside the retry loop raise AssertionError, which is not caught by the except block. This means:

• Empty choices (content filter / guardrail) → immediate crash (no retry)
• Malformed JSON → retried

If the intent is to also retry on empty/filtered responses (which are often transient), replace the asserts with explicit checks that raise a retriable exception:

if not choices:
    raise RuntimeError("No choices returned — content filter or rate limit")

And catch RuntimeError in the retry handler.

---

🟡 Medium Priority

3. Pose.identity() only applied for is_anchor — potential ordering concern

In arena_env_graph_conversion_utils.py:

if spatial_constraint.kind == "is_anchor" and subject_asset.get_initial_pose() is None:
    subject_asset.set_initial_pose(Pose.identity())

This works when spatial constraints are processed in a single pass, but if the same asset appears in multiple constraints and a later constraint sets a pose, it will overwrite the identity pose. The logic is fine today, but a comment noting that identity is intentionally a fallback-only default (and won't clobber an explicit pose) would prevent future confusion.

4. AssetRegistry() instantiation in fallback path

light_cls = AssetRegistry().get_asset_by_name("light")
scene_assets.append(light_cls())

If AssetRegistry is not a singleton, this creates a fresh registry every time the fallback fires. If it is a singleton, a brief comment confirming that would help readers. Also: get_asset_by_name("light") could return None if the registry doesn't have a "light" entry — a guard/assertion would prevent a confusing TypeError: 'NoneType' object is not callable.

5. Runner has no graceful cleanup on mid-pipeline failure

If the runner fails between environment construction (Step 5) and env.close() (end of Step 6), the Isaac Sim app may leak GPU resources. A try/finally around the env lifecycle would be more robust:

env = builder.make_registered()
try:
    # ... run policy ...
finally:
    env.close()

---

🟢 Low Priority / Suggestions

6. _safe_stem could produce collisions for different prompts

The regex re.sub(r"[^\w.-]+", "_", name) collapses distinct env names into the same stem (e.g. "pick & place" and "pick / place" both → "pick_place"). Adding a short hash suffix (e.g. first 6 chars of the prompt's MD5) would prevent silent overwrites in --out-dir.

7. No tests for the new runner or the retry logic

The new environment_generation_runner.py (217 lines) and the retry mechanism in generate_spec don't have corresponding tests. At minimum, a unit test for the retry path (mocking client.chat.completions.create to return malformed JSON then valid JSON) would prevent regressions. The extract_response_text preference change (JSON-prefixed reasoning_content) also warrants a test given it affects all DeepSeek calls.

8. System prompt reformatting — functional change buried in style commit

The system prompt now includes two new behavioral rules (is_anchor requirement, lighting auto-injection). These are meaningful semantic changes but the commit message says "Reformat system prompt for better readability". Consider documenting these behavioral additions in the PR description or a separate commit for traceability.

---

Summary

The core logic is sound — the retry mechanism, dome light injection, and anchor handling all address real failure modes. The main concern is the exception handling in the retry loop (items 1-2): as written, it catches too broadly while simultaneously not catching the assertion failures it shares scope with. A focused fix there would make the retry behavior predictable and robust.