Generate html from EnvGraphSpec yaml for user review

[qianl-nv]

Summary

First part of the LLM env gen pipeline: add static html view of the LLM-generated initial scene graph proposal

Detailed description

• Reason: Provide user with an intuitive way to visualize the env graph yaml
• Changed: Add a review_graph which takes yaml input and generate html with graph viz and node snapshots
• Impact: Adds the user facing component for the agentic env gen part 1

To run:
python -m isaaclab_arena.llm_env_gen.review_graph --yaml isaaclab_arena/tests/test_data/pick_and_place_maple_table_env_graph.yaml --render-thumbnails

[isaaclab-review-bot]

Code Review: Generate HTML from EnvGraphSpec YAML

Thanks for this contribution! This is a well-structured visualization tool for reviewing LLM-generated environment graph specifications. The dark dashboard layout with three panels (graph, tasks, nodes) provides a clean interface for users to inspect the YAML specs.

✅ Strengths

1. Clean architecture — Good separation of concerns between HTML generation, Mermaid graph rendering, task tables, and node cards.

2. Smart caching — The SHA1-keyed thumbnail cache under .cache/llm_env_gen_thumbnails/ avoids redundant USD renders across runs and environments.

3. Graceful degradation — The tool works without Isaac Sim (placeholder thumbnails) and gracefully handles missing assets or render failures without breaking the entire output.

4. Self-contained output — Inlining thumbnails as base64 data URIs makes the HTML fully portable.

💡 Suggestions

1. Consider adding a --quiet flag — The script prints diagnostic messages to stderr which is useful for debugging, but a quiet mode would be helpful for scripted usage.

2. Type hints on internal functions — Functions like _wait_for_stage_load, _wait_for_capture, and _ensure_default_lighting accept app and stage parameters without type hints. Adding SimulationApp and pxr.Usd.Stage type hints (even as strings for lazy imports) would improve readability.

3. Magic number constants — Consider extracting hardcoded values like the 30 Hydra settle frames (line 553), max_updates=600, and lighting intensities (800.0, 2500.0) into named constants at the module level for easier tuning.

4. Output path validation — Currently out_path.parent.mkdir(parents=True, exist_ok=True) silently creates directories. Consider logging when directories are created, especially for cases where the user might have made a typo in --out.

⚠️ Minor Issues

1. Line 161: HTML template f-string — The inline conditional {html_lib.escape(initial_state.id if initial_state else "<none>")} is correct but hard to read. Consider extracting to a variable for clarity.

2. Line 249: success_state_spec_id handling — The or '<em>unset</em>' fallback renders HTML, but html_lib.escape() is applied to it, which would double-escape if success_state_spec_id is actually the literal string <em>unset</em>. This is unlikely but worth noting. A cleaner pattern:

   success_cell = (f"<code>{html_lib.escape(t.success_state_spec_id)}</code>" 
                   if t.success_state_spec_id else "<em>unset</em>")

3. Potential issue with mermaid labels — The _mermaid_label function escapes " and |, but node IDs containing [, ], (, ) could break mermaid syntax if they appear in labels. Consider escaping these as well or wrapping labels in quotes.

🧪 Testing

No tests are included in this PR. Given this is a visualization tool, manual testing may be acceptable for the initial implementation. For future maintainability, consider adding:

• Unit tests for _render_mermaid() with various constraint configurations
• Tests for _pick_initial_state() edge cases (no tasks, no state specs)
• Integration tests that verify HTML output structure

Overall

This is a solid first implementation of the user-facing review component for the agentic env gen pipeline. The code is well-documented with helpful docstrings explaining the design decisions (especially around USD rendering quirks). Approve with the minor suggestions above.

---

Automated review by isaaclab-review-bot

[greptile-apps]

Greptile Summary

This PR adds review_graph.py, a new developer utility that reads an ArenaEnvGraphSpec YAML and writes a self-contained dark-themed HTML page with three panels: a mermaid spatial-constraint graph, a tasks table, and a node-card grid with optional USD viewport thumbnails (cached on disk, inlined as base64).

• HTML generation: node/task data is HTML-escaped throughout, but the mermaid graph block is inserted verbatim into <pre class=\"mermaid\"> without escaping <, >, or </pre> sequences in node labels — an LLM-generated node ID containing those characters would inject raw HTML into the page.
• USD thumbnail pipeline: lazily boots a single SimulationApp, resolves USD paths via AssetRegistry, deduplicates renders, caches PNGs by SHA-1 of the path, and fans results back to all nodes sharing the same asset.
• Minor issues: _THUMBNAIL_SIZE = 256 is defined but unused; _mermaid_label does not escape [ or ], which are syntactically significant in mermaid node definitions.

Confidence Score: 3/5

The core HTML generation is largely well-implemented, but mermaid node labels are embedded in the pre block without HTML escaping, leaving the generated page open to script injection from LLM-authored YAML node IDs.

The HTML injection in the mermaid block is a real present defect: any YAML with a node ID containing would break the page structure and allow arbitrary HTML/JS execution in the reviewer's browser. Given that this tool is explicitly part of the LLM pipeline and the YAML content is machine-generated, adversarial or accidentally malformed node IDs are a plausible input.

isaaclab_arena/llm_env_gen/review_graph.py — specifically the _render_html f-string where mermaid content is inserted and the _mermaid_label helper.

Security Review

• HTML injection / XSS (review_graph.py, _render_html line 141 / _mermaid_label): mermaid graph definition is inserted verbatim into a <pre class="mermaid"> element. Node IDs and labels are sanitized only for mermaid-specific characters but not for HTML special characters. A YAML with a node ID containing </pre><script> would break out of the <pre> element and execute arbitrary script in the reviewer's browser. Because the YAML is LLM-generated, adversarial content reaching this path is a realistic concern.

Important Files Changed

Filename	Overview
isaaclab_arena/llm_env_gen/review_graph.py	New module that converts an ArenaEnvGraphSpec YAML to a self-contained dark-themed HTML review page. Contains an HTML injection issue where LLM-generated node IDs are embedded unescaped in a mermaid pre block, plus a dead _THUMBNAIL_SIZE constant and missing bracket escaping in mermaid labels.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([yaml input]) --> B[ArenaEnvGraphSpec.from_yaml]
    B --> C{render-thumbnails?}
    C -- No --> D[_render_html]
    C -- Yes --> E[_launch_simulation_app]
    E -- success --> F[_render_thumbnails_with_app]
    E -- failure --> D
    F --> G[_resolve_node_usd_paths]
    G --> H{cache hit?}
    H -- Yes --> I[read cached PNG]
    H -- No --> J[_capture_usd_thumbnails]
    J --> K[write PNG to cache]
    I --> L[thumbnails dict]
    K --> L
    L --> D
    D --> M[_render_mermaid]
    D --> N[_render_tasks_table]
    D --> O[_render_node_cards]
    M --> P[write HTML file]
    N --> P
    O --> P
    P --> Q([html output])

Loading

_{Reviews (1): Last reviewed commit: "Add robot snapshot" | Re-trigger Greptile}

[greptile-apps]

HTML injection via unescaped mermaid content

The output of _render_mermaid() is inserted verbatim into a <pre class="mermaid"> HTML element. _mermaid_label() only escapes " and |, leaving <, >, and </pre> sequences unescaped. Because the browser parses HTML before mermaid.js reads the text content, a node ID in the YAML containing </pre><script>alert(1)</script> would close the <pre> element early and execute arbitrary script in the reviewer's browser. This is especially relevant here because node IDs are LLM-generated strings with no guaranteed character-set restriction.

[greptile-apps]

_mermaid_label does not escape ] or [

Mermaid's node-definition syntax is id[label], so a ] inside the label text prematurely closes the bracket and corrupts the graph definition. For example, a node ID like "robot[0]" would produce robot_0_[robot[0]], causing a mermaid parse error (or silent mis-render) for the entire diagram. Only " and | are currently escaped; [ and ] need the same treatment.

[greptile-apps]

Unescaped raw HTML in success_state_spec_id fallback

html_lib.escape(t.success_state_spec_id) returns a plain string. If it were ever falsy (empty string), the or branch would inject the literal raw <em>unset</em> HTML tag into the table cell without escaping. While required_str currently prevents an empty value, the pattern is fragile. The fallback should be HTML-escaped too.

Suggested change

	f"<td><code>{html_lib.escape(t.success_state_spec_id) or '<em>unset</em>'}</code></td>"
	f"<td><code>{html_lib.escape(t.success_state_spec_id) or '&lt;em&gt;unset&lt;/em&gt;'}</code></td>"

[greptile-apps]

_THUMBNAIL_SIZE is defined but never used

The constant is referenced nowhere in the file; the actual captured image size is determined solely by the viewport configuration. Remove it to avoid confusion about whether thumbnail dimensions are controlled here.

Suggested change

_THUMBNAIL_SIZE = 256