feat(sim): add declarative scene affordances - #489
Conversation
Greptile SummaryAdds a declarative semantic scene layer with typed metadata, capability-scoped affordance lookup, explicit defaults, and revision validation.
Confidence Score: 4/5The PR appears safe to merge, with the non-blocking requirement that its newly exported public scene-affordance API be documented. The affordance validation and resolution paths are internally consistent, and the only accepted issue is missing documentation for newly public symbols and contracts. Files Needing Attention: embodichain/lab/sim/skills/init.py, embodichain/lab/sim/skills/scene.py
|
| Filename | Overview |
|---|---|
| embodichain/lab/sim/skills/scene.py | Introduces semantic metadata and validated affordance indexing/resolution; implementation is coherent, but its new public API lacks accompanying documentation in this PR. |
| embodichain/lab/sim/skills/init.py | Re-exports the new scene-affordance API, making the missing public documentation independently actionable. |
| tests/sim/skills/test_scene.py | Adds focused coverage for payload and revision validation, scoped defaults, ambiguity, unsupported capabilities, and metadata projection. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Parent and required capability] --> B[Resolve canonical parent]
B --> C[Index direct-child affordances by capability]
C --> D{Explicit affordance supplied?}
D -->|Yes| E{Registered child with capability?}
E -->|Yes| F[Return explicit affordance]
E -->|No| G[UnsupportedSceneAffordanceError]
D -->|No| H{Candidate count}
H -->|0| G
H -->|1| I[Return sole candidate]
H -->|Multiple| J{Scoped default configured?}
J -->|Yes| K[Return default affordance]
J -->|No| L[AmbiguousSceneAffordanceError]
Prompt To Fix All With AI
### Issue 1
embodichain/lab/sim/skills/__init__.py:41-52
**Public affordance API lacks documentation**
The package now publicly exports the affordance capability constants, resolution exceptions, and `SceneEntityMetadata`, but this PR adds no user-facing documentation for their construction and validation contracts, making the new API harder to adopt correctly.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(sim): add declarative scene afforda..." | Re-trigger Greptile
| AmbiguousSceneAffordanceError, | ||
| GRASP_AFFORDANCE_CAPABILITY, | ||
| PLACE_IN_AFFORDANCE_CAPABILITY, | ||
| PLACE_ON_AFFORDANCE_CAPABILITY, | ||
| RegistrySceneProvider, | ||
| SceneAffordanceRef, | ||
| SceneArticulationRef, | ||
| SceneCollisionRole, | ||
| SceneCollisionWorldMode, | ||
| SceneDynamics, | ||
| SceneEntityRef, | ||
| SceneEntityMetadata, |
There was a problem hiding this comment.
Public affordance API lacks documentation
The package now publicly exports the affordance capability constants, resolution exceptions, and SceneEntityMetadata, but this PR adds no user-facing documentation for their construction and validation contracts, making the new API harder to adopt correctly.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/lab/sim/skills/__init__.py
Line: 41-52
Comment:
**Public affordance API lacks documentation**
The package now publicly exports the affordance capability constants, resolution exceptions, and `SceneEntityMetadata`, but this PR adds no user-facing documentation for their construction and validation contracts, making the new API harder to adopt correctly.
**Context Used:** AGENTS.md ([source](https://github.com/dexforce/embodichain/blob/main/AGENTS.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!
There was a problem hiding this comment.
Pull request overview
Introduces a semantic “scene affordance” layer in embodichain.lab.sim.skills.scene so higher-level Expert Programs can reference typed affordances (with capabilities and revisions) without coupling to simulator handles, while also enabling provider-free metadata projection for registries.
Changes:
- Added capability-scoped affordance selection (
affordances()/resolve_affordance()), including explicit-default disambiguation and dedicated error types. - Introduced
SceneEntityMetadataprojection to expose registry metadata without copying live affordance payloads. - Expanded scene tests to cover capability validation, default selection behavior, and payload-copy avoidance for metadata projection.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
embodichain/lab/sim/skills/scene.py |
Adds affordance capability/revision validation, metadata projection, and new registry affordance resolution APIs. |
embodichain/lab/sim/skills/__init__.py |
Re-exports new scene affordance constants, metadata, and error types. |
tests/sim/skills/test_scene.py |
Adds regression tests for capability validation, default affordance selection, and metadata projection behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| self._validate_reference_contract() | ||
| SceneEntityMetadata.from_registration(self) |
Description
Stack
feat/atomic-action-pr2c-runtime-endpointsAdd the typed semantic scene layer consumed by Expert Programs: object, articulation, link, pose, affordance, and relation references with strict capability and revision validation.
The API separates semantic intent from simulation handles and preserves registry-backed identity, allowing later compilers to resolve fresh poses without exposing transform math in task configuration.
Refs #471
Refs #474
Type of change
Screenshots
Not applicable.
Validation
tests/sim/skills/test_scene.pyChecklist