refactor(atomic-actions): verify effects on due observations - #494
refactor(atomic-actions): verify effects on due observations#494yuecideng wants to merge 1 commit into
Conversation
Greptile SummaryThis PR aligns physical-effect verification with due simulator observations and correlates each result with the active request and plan generation.
Confidence Score: 5/5The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issues identified. The changed runner consumes synchronous verification against a fresh due observation and the current request in the same step, while existing guards prevent revision installation from racing pending effect resolution and the added tests cover the principal lifecycle transitions.
|
| Filename | Overview |
|---|---|
| embodichain/lab/sim/atomic_actions/runner.py | Moves verification into the fresh due-cycle step, validates callback behavior, and preserves safe timeout and failure handling. |
| embodichain/lab/sim/atomic_actions/execution.py | Adds session-local attempt generations to effect requests and advances them whenever a replacement plan is installed. |
| tests/sim/atomic_actions/test_runner.py | Covers same-step fresh observations, deadline behavior, invalid results, repeated unresolved polling, and request refresh after deactivation. |
| tests/sim/atomic_actions/test_engine_per_env.py | Verifies generation stability during request shrinkage and advancement across retries and tracking replans. |
| scripts/tutorials/atomic_action/moving_target_recovery.py | Updates the tutorial verifier to return an exactly correlated per-environment verification result. |
Sequence Diagram
sequenceDiagram
participant App
participant Runner
participant Observer
participant Verifier
participant Session
App->>Runner: step(effect_verifier)
Runner->>Observer: observe(current task state)
Observer-->>Runner: fresh due PlanningContext
Runner->>Session: pending_effect snapshot
alt "request pending and timestamp <= deadline"
Runner->>Verifier: verify(context, request)
Verifier-->>Runner: correlated EffectVerificationResult
end
Runner->>Session: tick(context, effect_result)
Session-->>Runner: ExecutionTick
Runner-->>App: RunnerStep
Reviews (1): Last reviewed commit: "refactor(atomic-actions): verify effects..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
Aligns atomic-action physical-effect verification with the runner’s due-observation cadence so effect evidence is only consumed when a fresh observation is due, correlated to the active request, and ignored after its deadline—preventing repeated polling from producing synthetic consecutive samples.
Changes:
- Introduced a synchronous
effect_verifier(context, request)pathway inExecutionRunner.step()/run_until_blocked(), mutually exclusive with supplyingeffect_result. - Added
attempt_generationtoEffectVerificationRequestand advanced it on newly installed plans to robustly correlate effect requests across replans/retries vs. mask shrink/deactivation. - Expanded test coverage and updated tutorial/docs to reflect the new verifier contract and request correlation semantics.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
embodichain/lab/sim/atomic_actions/runner.py |
Adds synchronous due-observation effect verification and enforces exclusivity with async effect_result. |
embodichain/lab/sim/atomic_actions/execution.py |
Adds attempt_generation to effect requests and increments it on plan installation for request correlation. |
tests/sim/atomic_actions/test_runner.py |
Updates verifier signature and adds regression tests for due-observation consumption, deadlines, retries, and invalid verifier behavior. |
tests/sim/atomic_actions/test_engine_per_env.py |
Extends per-env engine/session assertions around effect-request generation across replans/retries. |
scripts/tutorials/atomic_action/moving_target_recovery.py |
Updates tutorial verifier to return EffectVerificationResult correlated to the request. |
docs/source/tutorial/atomic_actions.rst |
Updates tutorial documentation to the new verifier signature and synchronous semantics. |
docs/source/overview/sim/atomic_actions/index.md |
Updates runner API overview to include effect_verifier support and exclusivity rule. |
agent_context/topics/atomic-actions/atomic-actions.md |
Updates internal agent-facing context on effect verification semantics and synchronous verification behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Description
Stack
refactor/atomic-runtime-row-lifecycleAlign physical-effect verification with the environment's observation cadence. Evidence is consumed only when due, correlated to the active request, and rejected when stale; repeated polling no longer manufactures consecutive samples.
This preserves real simulator timing and keeps effect monitors observational rather than turning them into state-mutating completion hooks.
Refs #471
Refs #474
Type of change
Screenshots
Not applicable.
Validation
tests/sim/atomic_actions/test_engine_per_env.pyandtest_runner.pyChecklist