refactor(atomic-actions): preserve per-environment runtime lifecycle - #493
refactor(atomic-actions): preserve per-environment runtime lifecycle#493yuecideng wants to merge 1 commit into
Conversation
Greptile SummaryThis PR refactors atomic-action execution to preserve row-local eligibility, retries, effect verification, and completion within a shared vectorized session.
Confidence Score: 4/5This PR should not merge until partial row deactivation during effect verification actively neutralizes the removed controller rows. The new deactivation path can leave persistent controllers executing the last command for removed rows because pending-effect polling emits neither a replacement command nor a hold. Files Needing Attention: embodichain/lab/sim/atomic_actions/runner.py, embodichain/lab/sim/atomic_actions/execution.py
|
| Filename | Overview |
|---|---|
| embodichain/lab/sim/atomic_actions/execution.py | Introduces row-local lifecycle state, correlated partial effect verification, deadlines, retries, deactivation, and expanded terminal events. |
| embodichain/lab/sim/atomic_actions/runner.py | Integrates correlated verification and row deactivation, but partial deactivation at a pending-effect boundary leaves removed rows without an active neutralization write. |
| embodichain/lab/sim/atomic_actions/effects.py | Adds defensively owned effect snapshots while retaining live simulation-entity identity. |
| tests/sim/atomic_actions/test_engine_per_env.py | Adds broad coverage for partial verification, retry, timeout, eligibility, deactivation, and effect ownership. |
| tests/sim/atomic_actions/test_runner.py | Covers runner verification, timeout, cached-request refresh, and safe failure, but not controller neutralization after partial deactivation during an effect wait. |
Sequence Diagram
sequenceDiagram
participant App
participant Runner
participant Session
participant Controller
App->>Runner: deactivate_rows(mask)
Runner->>Session: deactivate_rows(mask)
Session-->>Runner: narrowed pending effect request
Note over Session: Other rows remain unresolved
Runner->>Session: "step(effect_result=None)"
Session-->>Runner: pending effect, no command/hold
Note over Controller: Deactivated rows retain previous command
Prompt To Fix All With AI
### Issue 1
embodichain/lab/sim/atomic_actions/runner.py:445
**Deactivated rows remain commanded**
When a subset of rows is deactivated while other rows remain at an effect-verification boundary, `deactivate_rows()` only narrows the session masks and subsequent pending-effect ticks emit neither a command nor a hold, causing persistent controllers to continue executing the removed rows' previously dispatched commands.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "refactor(atomic-actions): preserve per-e..." | Re-trigger Greptile
| TypeError: If ``env_mask`` is not a tensor. | ||
| ValueError: If the mask or reason is invalid. | ||
| """ | ||
| if self._status is not RunnerStatus.RUNNING: |
There was a problem hiding this comment.
Deactivated rows remain commanded
When a subset of rows is deactivated while other rows remain at an effect-verification boundary, deactivate_rows() only narrows the session masks and subsequent pending-effect ticks emit neither a command nor a hold, causing persistent controllers to continue executing the removed rows' previously dispatched commands.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/lab/sim/atomic_actions/runner.py
Line: 445
Comment:
**Deactivated rows remain commanded**
When a subset of rows is deactivated while other rows remain at an effect-verification boundary, `deactivate_rows()` only narrows the session masks and subsequent pending-effect ticks emit neither a command nor a hold, causing persistent controllers to continue executing the removed rows' previously dispatched commands.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Pull request overview
This PR refactors the atomic-actions execution runtime to preserve per-environment (row-local) lifecycle across a shared vectorized session, so planning, effect verification, retries/timeouts, cancellation, and completion can resolve independently per row while maintaining stable request identity and deterministic events.
Changes:
- Introduces correlated, row-scoped effect verification via
EffectVerificationRequest+EffectVerificationResult, enabling partial effect resolution and stable deadlines/IDs across request shrinkage. - Adds sticky per-session
eligible_maskplus runner/session row deactivation (deactivate_rows) to permanently remove environments while keeping remaining rows progressing under the same session. - Expands runtime event taxonomy (planning failure vs effect failure/timeout vs retry, completed vs failed) and updates docs and tests accordingly.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/sim/atomic_actions/test_runner.py | Updates runner tests for correlated effect results, fresh observations on resume, deactivation refresh, and retry/timeout behavior. |
| tests/sim/atomic_actions/test_engine_per_env.py | Adds extensive per-env lifecycle coverage: sticky eligibility, partial effect commits, retries/timeouts, deactivation interactions, and request/ID invariants. |
| embodichain/lab/sim/atomic_actions/runner.py | Updates runner API to accept EffectVerificationResult, adds deactivate_rows, and adjusts scheduling/terminal messaging. |
| embodichain/lab/sim/atomic_actions/policies.py | Clarifies action_timeout semantics to include terminal effect verification. |
| embodichain/lab/sim/atomic_actions/execution.py | Core refactor: effect verification requests/results with deadlines/IDs, sticky eligibility, row deactivation, richer events, and updated tick semantics. |
| embodichain/lab/sim/atomic_actions/engine.py | Extends engine.start() to accept an initial eligible_mask and passes it into the session. |
| embodichain/lab/sim/atomic_actions/effects.py | Adds StateDelta.snapshot() and deep-copy helpers to ensure effect payloads are owned and safe from external mutation. |
| embodichain/lab/sim/atomic_actions/init.py | Exposes EffectVerificationResult in the public API. |
| docs/source/tutorial/atomic_actions.rst | Updates tutorial to demonstrate eligible_mask, runner.deactivate_rows, and correlated EffectVerificationResult usage and resumption semantics. |
| docs/source/overview/sim/atomic_actions/index.md | Updates overview API table and explains sticky eligibility, deactivation, and effect result correlation/timeout semantics. |
| agent_context/topics/atomic-actions/atomic-actions.md | Updates agent-facing context to match the new per-row lifecycle and effect verification contracts. |
| agent_context/MAP.yaml | Adds new atomic-actions topic keywords/anchors for the updated APIs and semantics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| object.__setattr__( | ||
| self, | ||
| "pending_effect", | ||
| self.pending_effect.snapshot(), | ||
| ) |
Description
Stack
feat/semantic-skill-compilerRefactor execution so planning, commands, effects, retry, cancellation, and completion remain row-local inside a shared vectorized session. Resolved rows commit independently while unresolved rows continue under the same attempt and deadline.
The runner preserves stable request identity across partial progress and emits deterministic per-environment events without introducing a second scheduler.
Refs #471
Refs #474
Type of change
Screenshots
Not applicable.
Validation
tests/sim/atomic_actions/test_engine_per_env.pyandtest_runner.pyChecklist