feat(gen_sim): add Scene Engine and Gradio workspace - #457
Conversation
…ain into muzi/feat_scene_engine
# Conflicts: # .github/workflows/main.yml # embodichain/gen_sim/scene_engine/pipeline/utils/scene_exporter.py # pyproject.toml # tests/gen_sim/scene_engine/test_scene_core_and_export.py
Greptile SummaryThe PR adds an image-to-scene generation workflow and a Gradio workspace for running, managing, and previewing Scene Engine, SimReady, Articraft, and Action Engine pipelines.
Confidence Score: 4/5The PR is not yet safe to merge because Articraft preview readiness can still accept an unrelated listener and Codex still has write access to other sessions’ records. The current preview check proves only that some service is listening on the selected port, while the released probe socket leaves the ownership race reachable. Codex is also granted workspace-write access to the entire shared Articraft output directory, so prompt instructions do not prevent cross-session record modification. Files Needing Attention: embodichain/gen_sim/gradio_ui/app_articraft.py
|
| Filename | Overview |
|---|---|
| embodichain/gen_sim/env.py | Adds optional shared dotenv discovery and loading; the missing-file startup failure is now guarded and tested. |
| embodichain/gen_sim/gradio_ui/app_processes.py | Adds per-session subprocess ownership, termination helpers, output redaction, and credential-minimized Codex environments. |
| embodichain/gen_sim/gradio_ui/app_articraft.py | Adds Codex-backed articulation generation and Viser previews, but previously reported listener-ownership and shared-output write boundaries remain unresolved. |
| embodichain/gen_sim/gradio_ui/app_asset_engine.py | Adds session-scoped SimReady execution and Articraft integration; the prior missing import and cross-session reset failures are fixed. |
| embodichain/gen_sim/gradio_ui/app_workflows.py | Implements Scene and Action Engine execution, generated-scene management, and Viser preview orchestration. |
| pyproject.toml | Adds Scene Engine dependencies and extends the GenSim optional dependency group. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
U[Gradio workspace] --> S[Scene Engine]
U --> A[Asset Engine]
U --> X[Action Engine]
A --> R[SimReady]
A --> C[Articraft and Codex]
S --> E[Scene exports]
R --> M[Processed assets]
C --> M
E --> V[Viser previews]
M --> V
X --> V
Reviews (4): Last reviewed commit: "delete gradio test" | Re-trigger Greptile
| target="embodichain.workspace_cache_cli:main", | ||
| help="Inspect and clean workspace analyzer caches.", | ||
| ), | ||
| Command( |
yuecideng
left a comment
There was a problem hiding this comment.
Review focused on the new Gradio workspace. The inline comments cover security, data-loss, runtime, packaging, and CI blockers. Local validation also found that pytest tests/test_main.py fails because analyze-workspace is no longer registered, while pytest tests/gen_sim fails during collection because of the duplicate test_config.py module name. The existing inline thread on embodichain/__main__.py already calls out restoring that command, so I did not duplicate it.
| server_name=SERVER_NAME, | ||
| server_port=SERVER_PORT, | ||
| allowed_paths=[ | ||
| str(EMBODICHAIN_ROOT), |
There was a problem hiding this comment.
Blocking security issue: this exposes the entire repository through Gradio while the server defaults to 0.0.0.0 and no authentication is configured. Gradio treats every file below an allowed directory as publicly servable, so this includes embodichain/gen_sim/.env and its API credentials. Please whitelist only the generated artifact/assets directories, explicitly block secret paths, and either bind to localhost by default or require authentication.
| key=lambda item: len(item.parts), | ||
| reverse=True, | ||
| ): | ||
| if path.is_file() and path.suffix.lower() not in VIDEO_SUFFIXES: |
There was a problem hiding this comment.
OUTPUTS_DIR is the repository-wide outputs/ directory, so this loop deletes every non-video file below it whenever Reset or Auto cleanup runs. That includes unrelated RL checkpoints, debug reports, trajectories, and datasets. Please track and remove only artifacts created by the current Gradio run instead of recursively cleaning this shared directory.
|
|
||
| def configured_lerobot_roots() -> list[Path]: | ||
| roots: list[Path] = [] | ||
| env_root = os.environ.get("EMBODICHAIN_DATASET_ROOT") |
There was a problem hiding this comment.
This function cannot run: os is not imported, and CURRENT_PATHS referenced below is not defined or imported by this module either. monitor_simulation() calls this path after DexSim exits but before clearing runtime.sim_process, so every completion raises NameError and leaves the UI/Auto loop stuck in a running state. Please import or pass these dependencies explicitly and make the runtime cleanup execute in a finally path.
| f"Port {self._port} is unavailable without a visible listener." | ||
| ) | ||
|
|
||
| self._signal_listeners(listener_pids, signal.SIGTERM, "stop") |
There was a problem hiding this comment.
This treats every process listening on the configured port as a stale Articraft preview and sends it SIGTERM, later escalating to SIGKILL, without verifying ownership. A normal port collision can therefore terminate an unrelated user service. Please terminate only self._process (or another registered child owned by this app); otherwise report that the port is already in use.
| ValueError: If the file contains an invalid ``KEY=VALUE`` entry. | ||
| """ | ||
| target_env = os.environ if env is None else env | ||
| env_path = find_gen_sim_env_file() |
There was a problem hiding this comment.
When neither EMBODICHAIN_ENV_FILE nor embodichain/gen_sim/.env exists, find_gen_sim_env_file() falls through and returns None, so this immediately raises AttributeError: 'NoneType' object has no attribute 'is_file'. A clean checkout only contains .env.example. Please return Path | None and guard None here, or always return the documented fallback path.
| def build_pipeline_env() -> dict[str, str]: | ||
| env = os.environ.copy() | ||
| configure_direct_network_env(env) | ||
| configure_simready_llm_env(env) |
There was a problem hiding this comment.
build_pipeline_env() is used by Scene Engine, DexSim, Viser, and Articraft preview processes as well as SimReady, but this unconditionally maps SIMREADY_OPENAI_* over OPENAI_*. If separate endpoints are configured as supported by .env.example, Scene Engine will receive the SimReady model, URL, and key. Please apply this mapping only to the SimReady command.
| return "stopped" | ||
|
|
||
| with runtime_lock: | ||
| simulation_completed = ( |
There was a problem hiding this comment.
sim_returncode is recorded by the monitor but never checked here. A non-zero DexSim exit still satisfies sim_started && sim_finished && sim_process is None, so Auto records the round as completed and continues to later phases. Please require runtime.sim_returncode == 0 and also propagate a non-zero exit to the failed phase/last_error.
|
|
||
| import pytest | ||
|
|
||
| from embodichain.gen_sim.scene_engine.cli import start |
There was a problem hiding this comment.
This new test module has the same basename as tests/gen_sim/simready_pipeline/test_config.py, and neither parent directory is a Python package. With the repository's default pytest import mode, pytest tests/gen_sim fails during collection with an import-file-mismatch error. Please rename this file (for example, test_scene_engine_cli_config.py) or make the test directories packages/use importlib mode.
| from pathlib import Path | ||
| from typing import Any, Iterable | ||
|
|
||
| import gradio as gr |
There was a problem hiding this comment.
Gradio is now a direct runtime dependency of this feature, but neither the core dependency list nor the gensim extra declares it. A clean pip install .[gensim] therefore cannot reliably launch this UI. Please add a supported Gradio version to an appropriate optional extra and document that installation path.
| /gym_project/ | ||
| .debug_engine/ | ||
|
|
||
| # Local Gradio UI dependencies, generated Articraft records, and bytecode |
Description
This PR introduces an image-to-scene generation workflow for EmbodiChain, together with a Gradio-based workspace for running and previewing generative-simulation workflows.
Key changes:
Fixes # (issue)
Type of change
Checklist
black .command to format the code base.