Agentic env gen supports replicator kitchen#942
Conversation
Discover Replicator/seed_* USDA floorplans and register them as replicator_kitchen_<seed> background assets with a framing viewer cfg. Signed-off-by: Qian Lin <qianl@nvidia.com>
- Add a "Background 360° panorama" toggle that renders the background asset as a raw fisheyeSpherical panorama, cached in a separate panorama cache directory. - Thread the background_panorama flag through the SimApp client/server and the thumbnail capture path, and flag the resulting card as a panorama. Signed-off-by: Qian Lin <qianl@nvidia.com>
Show the background USD path and prim tree above the YAML editor, resolving from the background block even when the full spec fails validation. Signed-off-by: Qian Lin <qianl@nvidia.com>
List default-prim-relative paths verbatim in BACKGROUND PRIM TREE so the LLM returns values like Kitchen/Floor instead of leaf-only Floor on Replicator kitchens. Signed-off-by: Qian Lin <qianl@nvidia.com>
| _REPLICATOR_ROOT = Path(__file__).resolve().parents[2] / "Replicator" | ||
|
|
||
|
|
||
| def _discover_replicator_kitchen_usda_paths() -> dict[str, Path]: |
There was a problem hiding this comment.
🟡 Background set depends on untracked local files
_discover_replicator_kitchen_usda_paths() globs <repo>/Replicator/seed_*/*.usda at import time, but Replicator/ is neither committed nor in .gitignore — so which replicator_kitchen_<seed> backgrounds get registered varies per machine. A spec that references one resolves on the box that generated it and silently fails elsewhere, while every other background here points to Nucleus or downloads on demand. For an eval library that's a reproducibility gap. Where are these USDAs meant to come from — could the seed→path mapping be explicit (and the chosen paths logged) rather than discovered from a local dir at import time?
| """Return a single world-frame pose for viewer targeting.""" | ||
| initial_pose = lookat_object.get_initial_pose() | ||
| if initial_pose is None: | ||
| object_cfg = getattr(lookat_object, "object_cfg", None) |
There was a problem hiding this comment.
🔵 getattr/hasattr chain over known attributes
object_cfg, init_state, and pos are all declared on ObjectBase/the Isaac Lab cfg, so if lookat_object is always an ObjectBase this defensive getattr/hasattr chain can be plain attribute access. It also reads the pose from a second place (object_cfg.init_state.pos) when get_initial_pose() is None — is that case real? If so, could it be exposed through get_initial_pose() so there's a single source of truth? This fallback branch is also currently untested (the new test only covers the PosePerEnv path).
| st.session_state["edited_text"] = new_text | ||
|
|
||
| validation = validate_yaml_text(st.session_state["edited_text"]) | ||
| from isaaclab_arena_examples.agentic_environment_generation.review_gui.spec_visualization.visualization_widgets import ( |
There was a problem hiding this comment.
🔵 Imports inside the function
These two review_gui imports don't need the sim running — is this deferring to break an import cycle? If not, they'd read better at module top.
🤖 Isaac Lab-Arena Review BotSummaryThis PR adds Replicator-kitchen support to the agentic-env-gen review GUI: it registers Design, Boundaries & ScopeThe one concern that reaches beyond the GUI: Findings🟡 Warning: background_library.py:205 — registered backgrounds depend on untracked local Test CoverageGood additions: full-path prim-tree formatting, the VerdictMinor fixes needed |
Greptile SummaryThis PR adds Replicator kitchen support to the agentic environment generation review GUI: dynamically discovering and registering
Confidence Score: 4/5Safe to merge; the core panorama pipeline and camera restore logic are well-structured, and the prim-path inference fix is correct and tested. The changes introduce a non-trivial rendering pipeline (fisheye camera, stage mutation, cache management) alongside dynamic asset registration. The panorama capture finally block cleanly restores viewport state before subsequent ref captures. The main concern is that
Important Files Changed
|
| from isaaclab_arena_examples.agentic_environment_generation.review_gui.spec_visualization.visualization_widgets import ( | ||
| render_background_prim_tree, | ||
| ) | ||
| from isaaclab_arena_examples.agentic_environment_generation.review_gui.visualization_service import ( | ||
| resolve_background_preview, | ||
| ) |
There was a problem hiding this comment.
Deferred imports inside function body
render_background_prim_tree and resolve_background_preview are imported inside render_editor_panel on every Streamlit render pass. There is no circular dependency between editor_panel, visualization_widgets, and visualization_service — none of those modules import from editor_panel. Moving these to module-level imports at the top of the file would make the dependency graph easier to read and consistent with the other imports in this module.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| viewport.camera_path = PANORAMA_CAMERA_PRIM_PATH | ||
| pump_app(app, count=PRE_CAPTURE_UPDATES) | ||
|
|
||
| try: | ||
| png_bytes = capture_viewport_png(app, cache_path, pre_capture_updates=PRE_CAPTURE_UPDATES) |
There was a problem hiding this comment.
Double pre-capture pump for panorama
_capture_background_panorama calls pump_app(app, count=PRE_CAPTURE_UPDATES) explicitly before the try block, and then passes pre_capture_updates=PRE_CAPTURE_UPDATES to capture_viewport_png, which calls pump_app again internally (kit_viewport.py line 135). The fisheye camera therefore settles for 2 × PRE_CAPTURE_UPDATES = 10 frames before the capture fires.
If the double pump is intentional (fisheye projection needs more settle time), a clarifying comment here would help; otherwise the outer pump_app call is redundant since capture_viewport_png already handles pre-capture pumping.
Summary
Add Replicator kitchen review previews
Detailed description