Conversation
…mentation client)
…cs and tests may not be updated
…ption, and add check code
…ption in objects' names
…but no real-world scale...
…tate for assets who need to be calibrated
…libration in image-conditioned scene engine pipeline (but only calibrated the upright bottle-like assets currently)
… follow the assets' orientation_states
yuecideng
added
bug
5 of 13 tasks
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+312
to
+313
| if first_error is not None: | ||
| raise first_error |
There was a problem hiding this comment.
If an attachment manager fails to detach after drawer transport planning succeeds, close() raises from the finally block in DrawerMotionGenerator.generate(). That replaces the usable plan with an exception and fails the drawer call, even though attachment cleanup is meant to be best-effort.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/gen_sim/task_engine/_task_program/drawer_curobo.py
Line: 312-313
Comment:
**Cleanup discards successful plans**
If an attachment manager fails to detach after drawer transport planning succeeds, `close()` raises from the `finally` block in `DrawerMotionGenerator.generate()`. That replaces the usable plan with an exception and fails the drawer call, even though attachment cleanup is meant to be best-effort.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+92
to
+103
| def _pose(config: dict[str, Any], *, proxy: bool) -> np.ndarray: | ||
| if not proxy and config.get("init_local_pose") is not None: | ||
| return np.asarray(config["init_local_pose"], dtype=float) | ||
| matrix = np.eye(4) | ||
| matrix[:3, :3] = Rotation.from_euler( | ||
| "XYZ", config.get("init_rot", [0.0, 0.0, 0.0]), degrees=True | ||
| ).as_matrix() | ||
| matrix[:3, 3] = np.asarray( | ||
| config.get("proxy_init_pos" if proxy else "init_pos", config.get("init_pos")), | ||
| dtype=float, | ||
| ) | ||
| return matrix |
There was a problem hiding this comment.
When a scene has an articulation proxy and a nonzero scene rotation or XY translation, scene preparation transforms init_pos and init_rot but leaves proxy_init_pos unchanged. This renderer combines the unchanged proxy position with the transformed rotation, so the articulation's label and mask appear away from its prepared scene position. Visual grounding can then use misleading evidence even for a task that does not operate the articulation.
Knowledge Base Used: Generative simulation pipelines
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/gen_sim/task_engine/orchestration/visual_evidence.py
Line: 92-103
Comment:
**Articulation proxies render misplaced**
When a scene has an articulation proxy and a nonzero scene rotation or XY translation, scene preparation transforms `init_pos` and `init_rot` but leaves `proxy_init_pos` unchanged. This renderer combines the unchanged proxy position with the transformed rotation, so the articulation's label and mask appear away from its prepared scene position. Visual grounding can then use misleading evidence even for a task that does not operate the articulation.
**Knowledge Base Used:** [Generative simulation pipelines](https://app.greptile.com/dexforce/-/custom-context/knowledge-base/dexforce/embodichain/-/docs/generative-simulation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.…refactor_v38 Integrate standalone contact-verified E9 pressing while retaining v38 invocation policies and E6 recovery and terminal validation. Align container landing tests with the confirmed 35 mm wall margin. Validation: 1100 tests passed, 1 skipped; task1154/task1155 trajectories matched their baselines, and task1107 passed the contact-press contract. Existing API documentation gaps are deferred.
skywhite1024
force-pushed
the
codex/action-engine-v36-main-sync
branch
from
September 30, 2026 04:13
cfea1d6 to
58e5690
Compare
| return root | ||
|
|
||
|
|
||
| def test_visual_evidence_renders_current_assets_with_uid_masks(tmp_path: Path) -> None: |
There was a problem hiding this comment.
If a CPU-only test host has no usable EGL renderer, this test now runs its EGL-dependent rendering subprocess unconditionally. Rendering fails and the test fails instead of skipping as it did with the removed capability check.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/gen_sim/task_engine/orchestration/test_visual_evidence.py
Line: 67
Comment:
**Visual test loses EGL gate**
If a CPU-only test host has no usable EGL renderer, this test now runs its EGL-dependent rendering subprocess unconditionally. Rendering fails and the test fails instead of skipping as it did with the removed capability check.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Preserve v38 E6/E9 policies while integrating main planner, plan-transform, rigidized object, and scene USD APIs. Restore scoped drawer cleanup and arm-routing fixes, keep IK tolerances GenSim-local, and document visual grounding exports. Validated API documentation coverage, a clean-snapshot Sphinx build, affected CPU tests, and three fixed E6/E9 physical replays. The configured Drawer asset mirror still fails checksum validation.
skywhite1024
changed the title
skywhite1024
changed the title
Comment on lines
+513
to
+515
| "UIDs from the supplied candidate inventory. The previous " | ||
| f"validation failed with {type(first_error).__name__}; return a " | ||
| "fresh object that satisfies the schema." |
There was a problem hiding this comment.
When a grounding response selects an unknown UID or omits a reference, the one repair prompt now says only ValueError. It no longer tells the model which binding failed validation, making that retry less useful and increasing the chance that an otherwise groundable candidate is rejected.
Suggested change
| "UIDs from the supplied candidate inventory. The previous " | |
| f"validation failed with {type(first_error).__name__}; return a " | |
| "fresh object that satisfies the schema." | |
| "UIDs from the supplied candidate inventory. Validation error: " | |
| f"{first_error}" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/gen_sim/task_engine/orchestration/grounding.py
Line: 513-515
Comment:
**Repair loses error details**
When a grounding response selects an unknown UID or omits a reference, the one repair prompt now says only `ValueError`. It no longer tells the model which binding failed validation, making that retry less useful and increasing the chance that an otherwise groundable candidate is rejected.
```suggestion
"UIDs from the supplied candidate inventory. Validation error: "
f"{first_error}"
```
---
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!
This branch has not been deployed
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
4 participants
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
Synchronize the current Task Engine v38 line, including E6 recovery and standalone E9 pressing, with
mainat7e56e321. This resolves the conflicts introduced when PR #729 was updated to the newer Task Engine branch.render_scene_visual_evidence,make_visual_grounding_caller, andvisual_grounding_available, including their evidence limits and failure behavior. Repair short RST headings and gate the visual rendering test on isolated EGL availability.E9 remains a standalone, single-button, single-environment contact-press workflow. Its existing generated unit-scale/calibration policy, 10 mm extra commanded travel, contact-supported 0.05 mm event criterion, and retreat checks are unchanged. This does not certify device activation, self-latching, or success in the original unadapted scene.
Refs #531
Dependencies: the GenSim optional extra includes
shapely,yourdfpy>=0.0.60, andpython-fcl>=0.7.0.11. Documentation was built with the project's pinned documentation requirements in an isolated Python 3.11 environment.Type of change
Validation
black .with Black 26.3.1: 1297 files unchanged after formatting the targeted edits.git diff --cached --checkand agent context checks: passed.python docs/scripts/check_api_docs.py: 2695/2695 exports documented.activation_verifiedremains false.The physical replays used existing local scenes and deterministic/provider-free preparation; no external language-model or scene-generation API was called. Source scene and asset hashes were unchanged. These results do not claim all-scene or multi-seed qualification.
Remaining environment limitation
test_all_examples_register_plain_embodied_env_under_config_selected_ids[open_drawer]cannot complete because the configured mirror returns aDrawer.zipwith an invalid MD5. The asset checksum was not changed and the check was not disabled to hide this failure. Other affected integration tests passed; live cuRobo and the full native/GPU test matrix were not rerun.Screenshots
No UI change. Local simulation videos, execution reports, contact traces, and trajectory comparisons were retained with the validation artifacts; generated scene assets and recordings are not included in this PR.
Checklist
black .to format the code base.