feat(scene-engine): add text-guided scene editing - #515
Open
MuziWong wants to merge 20 commits into
Open
Conversation
…mentation client)
…cs and tests may not be updated
…ption, and add check code
…ption in objects' names
…but no real-world scale...
Greptile SummaryThis PR adds text-guided scene editing, including edit-plan and scene-graph models, generated-asset preparation, graph-guided layout updates, export/import support, CLI integration, documentation, and focused tests.
Confidence Score: 2/5The PR is not yet safe to merge because add categories can write generated files outside the scene workspace and chained planar edits can silently lose requested constraints. The current edit path still derives filesystem components directly from VLM-controlled category text before image and mask writes, while planar batch processing still clears relations created earlier in the same update loop. Files Needing Attention: embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_understanding.py, embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_asset_preparation.py, embodichain/gen_sim/scene_engine/clients/image_generation.py, embodichain/gen_sim/scene_engine/core/scene_graph.py
|
| Filename | Overview |
|---|---|
| embodichain/gen_sim/scene_engine/core/scene_edit_plan.py | Introduces normalized edit operations and validates references, conflicts, metadata, placement, and subtree deletion requirements. |
| embodichain/gen_sim/scene_engine/core/scene_graph.py | Adds support-hierarchy and planar-relation modeling together with batched graph updates and constraint derivation. |
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_understanding.py | Converts VLM edit drafts into normalized operations and applies them to the scene graph. |
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_asset_preparation.py | Generates, segments, reconstructs, and prepares newly added scene assets. |
| embodichain/gen_sim/scene_engine/pipeline/edit.py | Coordinates importing, understanding, asset preparation, layout generation, and re-export for scene edits. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_exporter.py | Extends scene export with graph and metadata artifacts while managing copied and deleted assets. |
Sequence Diagram
sequenceDiagram
participant User
participant CLI
participant VLM
participant Editor
participant Services
participant Exporter
User->>CLI: image and/or edit prompt
opt Image supplied
CLI->>Services: Generate and reconstruct scene
Services-->>CLI: Initial scene export
end
CLI->>VLM: Scene state and edit instruction
VLM-->>Editor: Add, move, and delete operations
Editor->>Editor: Validate plan and update scene graph
Editor->>Services: Generate, segment, and reconstruct added assets
Services-->>Editor: SimReady assets
Editor->>Exporter: Updated scene and graph
Exporter-->>User: Rewritten scene_export
Reviews (3): Last reviewed commit: "run black to tests/" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+24
to
+29
| from embodichain.gen_sim.scene_engine.configs.environment import ( | ||
| read_scene_engine_env_values, | ||
| ) | ||
|
|
||
|
|
||
| class ImageGenerationClient: |
There was a problem hiding this comment.
This new public module does not define __all__, leaving its intended API ambiguous and allowing wildcard imports to expose incidental imported names.
Suggested change
| from embodichain.gen_sim.scene_engine.configs.environment import ( | |
| read_scene_engine_env_values, | |
| ) | |
| class ImageGenerationClient: | |
| from embodichain.gen_sim.scene_engine.configs.environment import ( | |
| read_scene_engine_env_values, | |
| ) | |
| __all__ = ["ImageGenerationClient"] | |
| class ImageGenerationClient: |
Context Used: CLAUDE.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/gen_sim/scene_engine/clients/image_generation.py
Line: 24-29
Comment:
**Public client lacks export declaration**
This new public module does not define `__all__`, leaving its intended API ambiguous and allowing wildcard imports to expose incidental imported names.
```suggestion
from embodichain.gen_sim.scene_engine.configs.environment import (
read_scene_engine_env_values,
)
__all__ = ["ImageGenerationClient"]
class ImageGenerationClient:
```
**Context Used:** CLAUDE.md ([source](https://github.com/dexforce/embodichain/blob/main/CLAUDE.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.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!
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a text-guided scene editing workflow to the Scene Engine pipeline, extending exported artifacts to include an explicit SceneGraph representation and edit planning so scenes can be generated, edited, and re-exported in-place.
Changes:
- Introduces edit-time core structures (
SceneGraph,SceneEditPlan) and editing pipeline stages (understanding → asset prep → layout optimization → export). - Adds an image-generation client and updates segmentation config semantics, plus CLI support for
--edit_prompt(edit-only, generate-only, or generate-then-edit). - Persists additional spatial metadata (table support contour + conservative optimization rectangle, object
center_xy) into exports and import flows; updates docs and expands test coverage.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/gen_sim/scene_engine/test_simready_processor_utils.py | Adds regression coverage for VLM-footprint-driven uniform scaling. |
| tests/gen_sim/scene_engine/test_scene_understanding.py | Updates imports and adds validation tests for location words in descriptions + initial scene graph initialization. |
| tests/gen_sim/scene_engine/test_scene_layout_optimizer.py | Adds tests for table 9-grid mapping and stacked parent/child placement. |
| tests/gen_sim/scene_engine/test_scene_graph.py | Adds validation/normalization tests for graph layers, planar relations, and serialization. |
| tests/gen_sim/scene_engine/test_scene_engine_config.py | Adds CLI tests for edit-only and generate-then-edit behavior. |
| tests/gen_sim/scene_engine/test_scene_edit.py | Adds tests for importing editable exports and validating edit prerequisites. |
| tests/gen_sim/scene_engine/test_scene_edit_plan.py | Adds comprehensive tests for edit operation parsing, validation, graph updates, and asset prep. |
| tests/gen_sim/scene_engine/test_scene_core_and_export.py | Extends export tests to cover scene_graph.json, scene.json, center_xy, and import round-trips. |
| tests/gen_sim/scene_engine/test_clients.py | Adds image-generation client tests and updates segmentation endpoint/config keys. |
| embodichain/gen_sim/scene_engine/pipeline/utils/table_support_surface.py | Persists a conservative optimization rectangle derived from the detected tabletop contour. |
| embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py | Reworks SimReady processing to optionally query a VLM for rotation/scale and to persist support geometry. |
| embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor_utils.py | Adds rendering + VLM query helpers and GLB rotation/scale utilities for VLM-based transforms. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_layout_optimizer.py | Adds numerical optimization utilities for graph-constrained tabletop/stacked layouts. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_layout_constructor.py | Adds orchestration for applying layout optimization across table-root and stacked groups. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_importer.py | Adds importer for existing Scene Engine exports (scene + graph) to enable edit-only flows. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_exporter.py | Extends export to write scene_graph.json + scene.json and cleans up stale mesh assets. |
| embodichain/gen_sim/scene_engine/pipeline/utils/image_segmentation_utils.py | Adds heuristic mask inversion for generated single-object images and related utilities. |
| embodichain/gen_sim/scene_engine/pipeline/generation/scene_understanding.py | Returns (Scene, SceneGraph) and tightens semantic constraints to exclude location words in descriptions. |
| embodichain/gen_sim/scene_engine/pipeline/generation/scene_generation.py | Wires graph validation through generation/refinement and reuses persisted support geometry for refinement. |
| embodichain/gen_sim/scene_engine/pipeline/generation/init.py | Initializes generation subpackage. |
| embodichain/gen_sim/scene_engine/pipeline/generate.py | Updates generation entrypoint to pass segmentation client and export the scene graph. |
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_understanding.py | Adds VLM-to-edit-plan parsing and graph update application utilities. |
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_layout_generation.py | Adds layout optimization stage for applying edit plan results. |
| embodichain/gen_sim/scene_engine/pipeline/editing/scene_edit_asset_preparation.py | Adds image→mask→geometry→SimReady asset generation for add operations. |
| embodichain/gen_sim/scene_engine/pipeline/editing/init.py | Initializes editing subpackage. |
| embodichain/gen_sim/scene_engine/pipeline/edit.py | Adds edit pipeline entrypoint and re-export of edited outputs. |
| embodichain/gen_sim/scene_engine/core/scene_object.py | Extends SceneObject with center_xy and table support geometry fields for edits/layout. |
| embodichain/gen_sim/scene_engine/core/scene_graph.py | Adds SceneGraph data model with validation, normalization, and update application. |
| embodichain/gen_sim/scene_engine/core/scene_edit_plan.py | Adds validated edit operation and plan data structures. |
| embodichain/gen_sim/scene_engine/clients/image_segmentation.py | Renames segmentation endpoint config to “by prompt” and updates request path usage. |
| embodichain/gen_sim/scene_engine/clients/image_generation.py | Adds image-generation service client and dotenv configuration loader. |
| embodichain/gen_sim/scene_engine/cli/start.py | Adds --edit_prompt and supports edit-only / generate-then-edit invocation. |
| docs/source/features/generative_sim/scene_engine.md | Documents edit workflow, new dotenv keys, and new export artifacts. |
Suppressed comments (1)
embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py:251
_vlm_transform_for_objectwrites debug artifacts undersimready_geometry_root.parent / "debug"even when the caller provideddebug_output_root. This makes outputs harder to control/relocate and can create unexpected files during batch runs.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+385
to
+406
| best_rectangle: Polygon | None = None | ||
| best_area = 0.0 | ||
| for x_index, minimum_x in enumerate(x_values[:-1]): | ||
| for maximum_x in x_values[x_index + 1 :]: | ||
| if maximum_x <= minimum_x: | ||
| continue | ||
| for y_index, minimum_y in enumerate(y_values[:-1]): | ||
| for maximum_y in y_values[y_index + 1 :]: | ||
| if maximum_y <= minimum_y: | ||
| continue | ||
| rectangle = Polygon( | ||
| [ | ||
| (minimum_x, minimum_y), | ||
| (maximum_x, minimum_y), | ||
| (maximum_x, maximum_y), | ||
| (minimum_x, maximum_y), | ||
| ] | ||
| ) | ||
| area = rectangle.area | ||
| if area > best_area and polygon.covers(rectangle): | ||
| best_rectangle = rectangle | ||
| best_area = area |
…tate for assets who need to be calibrated
…libration in image-conditioned scene engine pipeline (but only calibrated the upright bottle-like assets currently)
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.
Suppressed comments (5)
embodichain/gen_sim/scene_engine/pipeline/utils/image_segmentation_utils.py:340
- Mask files are opened without being closed (
Image.open(mask_path)); since the convertedImageobjects are retained indecoded_masks, this can keep file handles open longer than intended. Open masks with a context manager and.copy()the converted image before storing it.
mask = Image.open(mask_path).convert("L")
_require_image_size(mask, image.size)
decoded_masks.append((asset_id, mask))
embodichain/gen_sim/scene_engine/pipeline/utils/scene_layout_constructor.py:368
- Using
list.pop(0)for the BFS queue makes group construction O(n^2) in the number of nodes. Switchingpendingto acollections.dequekeeps this O(n).
pending = [TABLE_OBJECT_ID]
while pending:
parent_id = pending.pop(0)
embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py:247
- This prompt string has a grammatical error ("needs to be place" → "needs to be placed"), which can reduce clarity for VLM prompting and for future maintainers reading the code.
embodichain/gen_sim/scene_engine/pipeline/utils/image_segmentation_utils.py:327 Image.open(...)is used without a context manager here; the underlying file handle may remain open until the image object is GC'd, which can leak descriptors in long-running pipelines. Usewith Image.open(...)and.copy()after.convert()to fully detach from the file.
This issue also appears on line 338 of the same file.
image = Image.open(image_path).convert("RGBA")
overlay = Image.new("RGBA", image.size, (0, 0, 0, 0))
embodichain/gen_sim/scene_engine/pipeline/utils/scene_layout_constructor.py:215
- Using
list.pop(0)in a loop is O(n) per iteration and can make descendant propagation quadratic as the number of nodes grows. Using acollections.dequemakes the BFS queue operations O(1).
This issue also appears on line 366 of the same file.
pending = list(children_by_parent.get(root_id, []))
while pending:
descendant_id = pending.pop(0)
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.
Suppressed comments (4)
embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py:247
- The
needed_layoutprompt string has grammatical issues ("needs to be place") and theuse_scale/use_rotationparameters are immediately deleted, which obscures intent.
embodichain/gen_sim/scene_engine/pipeline/utils/table_support_surface.py:405 _largest_inscribed_rectangledoes up to ~5Mpolygon.covers(rectangle)checks (48^4 worst-case), and each covers test can be expensive. Preparing the polygon once can significantly reduce the per-rectangle predicate cost.
area = rectangle.area
if area > best_area and polygon.covers(rectangle):
best_rectangle = rectangle
embodichain/gen_sim/scene_engine/pipeline/utils/simready_processor.py:215
- VLM transform flags are effectively ignored here:
rotate_about_xand the baked GLB rotation should be conditioned onuse_vlm_rotation, and the code currently hard-fails when Blenderbpyis unavailable (common in non-Blender runtimes), which breaks scene-edit add flows.
embodichain/gen_sim/scene_engine/pipeline/utils/image_segmentation_utils.py:339 render_asset_mask_id_overlayopens images/masks without a context manager. On some platforms this can leave file descriptors open longer than needed (and can prevent deleting temp directories on Windows).
image = Image.open(image_path).convert("RGBA")
overlay = Image.new("RGBA", image.size, (0, 0, 0, 0))
colors = (
(239, 83, 80, 255),
(66, 165, 245, 255),
(102, 187, 106, 255),
(255, 202, 40, 255),
(171, 71, 188, 255),
(38, 198, 218, 255),
)
decoded_masks: list[tuple[str, Image.Image]] = []
for index, (asset_id, mask_path) in enumerate(asset_masks):
mask = Image.open(mask_path).convert("L")
_require_image_size(mask, image.size)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds a text-guided scene editing workflow to Scene Engine.
Main changes
SceneGraphandSceneEditPlancore data structures for tabletop support hierarchies, 9-grid tableregions, planar relations, and edit validation.
--edit_promptsupport:--imageonly: generate a scene.--edit_promptonly: edit an existing valid Scene Engine export.add,move, anddeleteoperations.scene_graph.json,scene.json,category, andname.scene_exportdirectory, copy new assets, and remove deleted assetdirectories.
y, back is smallery.Dependencies: No new Python package dependencies.
Issue reference: N/A
Type of change
Validation
black --check --diff --color .pytest tests/gen_sim/scene_engine -q(73 passed)Screenshots
Not applicable.
Checklist