Repository navigation
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
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| quantifier = str(items[0]["quantifier"]) | ||
| count = int(items[0]["count"]) |
There was a problem hiding this comment.
When two steps refer to the same scene object but request different quantities—for example, one item first and a counted set later—the references are grouped together, and these lines keep only the first step’s quantity. The generated TaskTemplate loses the later requirement, so it can be grounded against too few objects.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/gen_sim/task_engine/task_spec.py
Line: 239-240
Comment:
**Later object counts disappear**
When two steps refer to the same scene object but request different quantities—for example, one item first and a counted set later—the references are grouped together, and these lines keep only the first step’s quantity. The generated TaskTemplate loses the later requirement, so it can be grounded against too few objects.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| if ( | ||
| raw.get("semantic_hash") is not None | ||
| and raw["semantic_hash"] != spec.semantic_hash | ||
| ): | ||
| raise ValueError( | ||
| "TaskSpec.semantic_hash does not match its semantic payload." |
There was a problem hiding this comment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/task_spec/spec.py
Line: 500-505
Comment:
**Legacy Cache Entries Fail** When an existing cache entry uses the previous template schema, its stored semantic hash reflects the old payload. The new decoder converts the schema, calculates a different hash, and rejects the entry. Reading a previously populated TaskSpec cache therefore raises instead of returning the stored template.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| I[Gym Lifecycle] | ||
| J[Observed State / Trajectory / Runtime Result] | ||
| K[Shared Predicate Evaluator] | ||
| L[Existing runtime evidence] |
There was a problem hiding this comment.
D but retains D --> K. It therefore shows an unexplained input to the evaluator, making the scene-binding and measurement flow harder to understand.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/architecture/gen-sim-taskspec-design.md
Line: 163
Comment:
**Diagram Has Undefined Node** The revised diagram removes the definition of `D` but retains `D --> K`. It therefore shows an unexplained input to the evaluator, making the scene-binding and measurement flow harder to understand.
---
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!
| "urdf-assembly", | ||
| "convex-decomposition" | ||
| "convex-decomposition", | ||
| "task-spec" |
There was a problem hiding this comment.
TaskSpec is added to the overview and specialist architecture views without a relationship edge. It therefore appears disconnected from the scene and Task Program components that the new module description says consume it, making the diagrams less useful for understanding the integration.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/architecture/curated.json
Line: 5467
Comment:
**TaskSpec appears disconnected**
TaskSpec is added to the overview and specialist architecture views without a relationship edge. It therefore appears disconnected from the scene and Task Program components that the new module description says consume it, making the diagrams less useful for understanding the integration.
---
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!
Comments Outside DiffThese findings could not be posted inline.
|
| from embodichain.gen_sim.task_spec import * # noqa: F401,F403 | ||
| from embodichain.gen_sim.task_spec import __all__ |
There was a problem hiding this comment.
This compatibility package forwards names such as TaskSpec, but it no longer provides the former embodichain.task_spec.spec, .registry, and other submodules. Callers importing directly from those modules now get ModuleNotFoundError, so the compatibility package does not preserve those import paths.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/task_spec/__init__.py
Line: 25-26
Comment:
**TaskSpec submodule imports break**
This compatibility package forwards names such as `TaskSpec`, but it no longer provides the former `embodichain.task_spec.spec`, `.registry`, and other submodules. Callers importing directly from those modules now get `ModuleNotFoundError`, so the compatibility package does not preserve those import paths.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Correct the regressions identified in the task-engine PR review: - Preserve scene poses, coordinate frames, native scale, and locked asset metadata across legacy conversion and scene revisions. - Enforce temporal and milestone evidence, deterministic task identities, configuration overrides, and semantic acceptance reporting. - Correct signed pouring, gripper release checks, planner instance binding, and staged motion fallback behavior. - Guard endpoint annotation paths, recording preservation, and asynchronous workflow publication; reject the unsupported continue failure policy. Add focused regression coverage and update the affected project context. Validation: - 698 affected CPU/mock tests and 48 context/docs tests passed. - Black 26.3.1, diff checks, and project context checks passed. - API documentation coverage: 2829/2829 exports; deployment inspector passed. - The /root/task2_1 simulation accepted 9/10 task groups. The final stack was rejected with 0.204 m position error against a 0.05 m tolerance; full-flow success remains unresolved.
Existing-hold HandOver now defaults to commanding measured hand angles. For the generated Robotiq task2_1 deployment, that loses the established closing drive and changes the subsequent grasp poses, ending in a failed stack. The pre-review PR head reproduces the same failure frame for frame. Expose source_hold_mode on HandOverOptions, retaining the observed default and existing positional arguments. Decode the mode through configured profiles and have generated Robotiq handovers select grasp_command so the bound closing target remains active until release. Update the owning context. Validation: - 109 focused action, decoder, and bundle/service tests passed. - 48 context/docs tests passed; Black 26.3.1 and API coverage checks passed. - Official HandOver-v1 deployment inspection passed. - A fresh run-all on /root/task2_1 with seed 0 accepted all 10 task groups and 31 calls. Final stack error was 0.006605 m against the unchanged 0.05 m tolerance, with both stable windows accepted. - The complete successful actions and states exactly match the historical successful recording; the source scene fingerprint is unchanged.
The Place policy tests passed a bare object as their simulation, so both cartesian_calls variants failed after planner factories began using the owning simulation's instance_id. Supply a named simulation mock, capture the actual planner configuration, and assert that the instance ID is forwarded. Retain the existing Place recovery and control-timing assertions. Validation: - Reproduced both CI AttributeError failures locally before the fix. - All 4 Place policy tests passed after the fix. - 79 neighboring policy/action/service tests passed with normal conftest. - Black 26.3.1, diff/syntax/header checks, and API documentation coverage passed. - The broader local CPU run passed 6941 tests but was interrupted after asset mirror downloads and a renderer test failed; it is not a full-suite pass. Context review: no update; this repairs a test fixture to the existing planner-instance contract without changing runtime behavior.
This branch has not been deployed
4 participants
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.Semantic graph visualization
The current
semantic_task_graph/v1bundle can be rendered without starting a simulator. The defaultgroupsview uses a horizontal timeline with separate left/right control-part lanes, declared dependency edges, handover arrows, full task instruction text, object-highlighted nodes, and theE*to Atomic Skill mapping table:Use
--view callsfor one card per Semantic Call.--reportis optional; when supplied, successful and failed calls are overlaid from the Task Program execution report.