Docs/migration 1x to 2x

[ayushiahjolia]

Issue #, if available: N/A

Description of changes:
Migration guide for python v2

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

Claude AI review

Documentation-only PR: a new docs/migration-1.x-to-2.x.md migration guide and an expanded WaitForConditionConfig docstring in packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/waits.py.

I verified the guide's technical claims against the source rather than reviewing runtime behavior, since inaccurate migration docs would misdirect users:

• Error hierarchy / exports — DurableOperationError base, StepError/InvokeError/ChildContextError/WaitForConditionError, graded CallbackError subtypes, SerDesError (child of DurableExecutionsError), RetryableSerDesError (retryable InvocationError); all root exports match __init__.py __all__.
• __cause__ — raise_as_operation_error reconstructs a DurableOperationError stand-in (registry fallback to base for non-SDK types) on both first run and replay; the guide's claim is accurate.
• First-run round trip — confirmed in step.py, child.py, wait_for_condition.py; invoke.py does not round-trip, matching "invoke unaffected."
• wait_for_condition — initial_state round-tripped before first check; empty-string payload treated as no stored state, restarting from initial_state.
• wait_for_callback serdes — enclosing child context uses default extended-type serdes, distinct from WaitForCallbackConfig.serdes.
• Completion predicate / BatchResult — should_complete, complete_batch/continue_batch, BatchCompletionError (only CUSTOM_COMPLETION_FAILED), throw_if_error typed raises, and fail-fast map/parallel defaults all match.
• attempt field — required no-default field on StepContext and WaitForConditionCheckContext.
• Removed names — CallableRuntimeError, WaitDecision, ItemBatcher, WaitStrategyConfig.timeout*, etc. confirmed absent.

No inaccurate statements or broken examples found. No actionable findings.

Residual test risk: the guide's code snippets and rg helpers are not covered by doctests/CI, so future API drift wouldn't be caught automatically — acceptable for a docs-only change. Separately (out of scope, not in this diff): the pre-existing DurableOperationError docstring at packages/aws-durable-execution-sdk-python/src/aws_durable_execution_sdk_python/exceptions.py:268-269 states the escaping error "is preserved as __cause__ on the first run," which contradicts the actual reconstruction behavior the new guide correctly documents.

Reviewed commit 6c1c23fe6206a4982795f437c0e251941292116b. Workflow run

[yaythomas]

picking up this PR to incorporate #698

[yaythomas]

Addressed in 25de7cc. The section now states validation catches drift in the four compared fields, cannot catch swaps of identical identities, and narrows rather than closes the silent-mismatch window; the deployment guidance prohibits reordering under in-flight executions regardless.

[yaythomas]

Addressed in 25de7cc. The paragraph now distinguishes phases: pre-SUCCEED first-run failures re-run an at-least-once step body, at-most-once follows interrupted-step handling, and a retryable error while reading an already-succeeded checkpoint retries only the deserialization.

[ParidelPooya]

The guide is unusually accurate — I checked ~25 factual claims and confirmed the large majority, including several that are easy to get wrong (cause holding a reconstructed stand-in, empty-string payload preservation, BatchResult.all dropping PENDING branches, wait_for_callback's child context ignoring WaitForCallbackConfig.serdes, the
empty-payload-means-no-stored-state quirk in wait_for_condition). Ruff passes on the waits.py docstring change.

But there is one blocking error, and it's in the section the guide itself promotes as one of the top three changes.

Blocking: the all_completed() section gets 1.x backwards

The guide says:

│ the default map and parallel completion config runs fail-fast … Pass all_completed() explicitly to keep the 1.x process-all behavior.

Both halves are wrong about 1.x:

The fail-fast default is not new. In 1.x, MapConfig.completion_config already defaulted to a bare CompletionConfig() and ParallelConfig to all_successful() — identical defaults to 2.0. And 1.x explicitly fail-fast on an empty config (concurrency/models.py at sdk-v1.7.0):

if not has_any_completion_criteria:
# Empty completion config - fail fast on any failure
if failure_count > 0:
return CompletionReason.FAILURE_TOLERANCE_EXCEEDED

with the scheduler agreeing via should_continue() → return self.failure_count == 0. 2.0's is_tolerance_exceeded reduces to the same failed > 0. Nothing changed here, yet it is listed as a breaking change in both the opening paragraph and the "Porting to 2.0" summary.

There was no 1.x process-all behavior to keep. 1.x's all_completed() returned all-None, which hit the fail-fast branch above. 2.0's returns tolerated_failure_percentage=100. So the change is a bug fix in the opposite direction: all_completed() used to fail fast and now actually tolerates every failure. Anyone who wrote all_completed() in 1.x and got fail-fast
will silently start processing every item after upgrading — the real migration risk, and the guide's advice would lead them straight into it.

Suggested reframing: retitle to something like "all_completed() now actually tolerates all failures", drop the default-completion claim from the intro and the porting summary, and tell readers who relied on 1.x all_completed() to switch to all_successful() or a bare CompletionConfig().

Should fix

should_complete invocation timing is wrong. The guide says the predicate "runs before the SDK schedules any branch (completed_count == 0) and on each suspension state change." In concurrency/executor.py, needs_snapshot_rebuild = True is set on branch submission and on COMPLETED, FAILED, SUSPENDED, and SUSPENDED_UNTIL events. Terminal completion and failure
are the primary triggers and the whole point of a quorum predicate, but the guide's wording implies they don't fire it. CompletionStatus's own docstring says "on each branch terminal event". Should read "on every branch state change (scheduling, completion, failure, and suspension)".

Wrong link for the plugin section. It points at /durable-execution/sdk-reference/observability/logging/, which documents the replay-aware logger and says nothing about the plugin interface. There is a dedicated page at /durable-execution/sdk-reference/observability/plugins/ that covers exactly the hooks and InvocationEndInfo changes @yaythomas asked about.

The SerDesError section's stated reason is wrong, though its advice is right. The guide says SerDesError "descends directly from DurableExecutionsError, not from ExecutionError" — but that was already its base class in 1.x; the hierarchy did not change. What changed is the wrapping in serdes.py: 1.x's serialize/deserialize helpers caught everything and raised
ExecutionError, while 2.0 raises SerDesError. The except ExecutionError → except SerDesError migration is correct; only the explanation misleads. (This misconception originated in the review thread, not the author's draft.)

Worth doing while you're here

Two SDK docstrings now contradict the guide, and the guide is right in both cases:

• MapConfig.completion_config still says "Default allows any number of failures." A bare CompletionConfig() fail-fast. This is the docstring that would make a reader think the guide's fail-fast claim is wrong.
• DurableOperationError's class docstring says "the escaping error is preserved as cause on the first run and reconstructed on replay." ErrorObject.raise_as_operation_error sets cause to a DurableOperationError rebuilt from the wire fields on both paths — deliberately, for replay determinism. The guide's "cause does not hold the original
  exception" is the accurate statement.

Since the guide's credibility depends on those docstrings agreeing, fixing them here is cheap.

[yaythomas]

Thank you for the thorough check, all four confirmed against the sdk-v1.7.0 tag and fixed in 25de7cc, along with both docstrings:

• all_completed(): section rewritten per your reframing. The factory change is the breaking change, the default is unchanged, and 1.x all_completed() users are told to switch to all_successful() or a bare CompletionConfig() to keep fail-fast. Claim dropped from the intro and porting summary; replay identity validation takes the headline slot.
• should_complete timing: now "on every branch state change: scheduling, completion, failure, and suspension".
• Plugin link now points at /observability/plugins/.
• SerDesError: explanation rewritten around the raise-site change (1.x wrapped in ExecutionError); you are right the hierarchy never changed, and right about where the misconception started.
• MapConfig.completion_config and DurableOperationError docstrings fixed in the same commit.

[ParidelPooya]

Re-reviewed at 25de7cc against 1.x (sdk-v1.7.0, 075b65a). All four points are fixed correctly, and the two claims you added beyond my feedback also check out.

Verified fixed

• all_completed(): the section now says what the code says. 1.x's all-None config hit if not has_any_completion_criteria: if failure_count > 0 in concurrency/models.py, and should_continue() returned failure_count == 0, so 1.x all_completed() did fail fast. Telling those callers to switch to all_successful() is the right advice.
• Default completion dropped from the intro and porting summary. Both defaults are unchanged between versions (MapConfig → bare CompletionConfig(), ParallelConfig → all_successful()), so replacing it with replay identity validation in the headline is the right call.
• SerDesError: correctly reframed as a raise-site change. 1.x serdes.py wrapped into ExecutionError; the hierarchy was always DurableExecutionsError.
• should_complete timing: matches needs_snapshot_rebuild, which is set on submit and on COMPLETED, FAILED, SUSPENDED, and SUSPENDED_UNTIL.
• Plugin link now points at the page that documents the hooks.
• Both SDK docstrings now agree with the guide.

New claims, both correct

• Identity validation can't catch same-identity swaps. Operation ids are positional (create_id_for_step), so two unnamed steps of the same type under the same parent produce identical type/sub_type/name/parent_id at the same ids and validate_checkpoint sees no mismatch. Good caveat to add.
• RetryableSerDesError phase split. In step.py, the SUCCEEDED branch of check_result_status deserializes with no try, so the error escapes process(), no checkpoint is written, and the next invocation re-reads the same SUCCEEDED checkpoint and retries only the deserialize. The body is not re-run.

Anchors all resolve; ruff clean on the three touched source files. LGTM.

[github-actions]

Codex AI review · Finding arf_v1_jqxzb55os3vqawerdjj46josgx

P1 — Do not promise per-submission predicate evaluation. ConcurrentExecutor.execute() submits the entire available scheduling wave before evaluating should_complete again; scheduling itself does not trigger the predicate. Code relying on this text to stop after one branch becomes STARTED can therefore start up to max_concurrency branches—all branches by default—and initiate side effects it intended to avoid. Document the event-driven evaluations and require max_concurrency to bound admissions.

Suggested change

	- It runs on every branch state change: scheduling, completion, failure, and
	suspension. The first evaluation happens before the SDK schedules any branch
	(`completed_count == 0`), and an unscheduled item has status `None` at that
	point, so handle a missing status explicitly, or the predicate can fail the
	batch or complete it before useful work starts.
	- It is evaluated once before any branch is scheduled and after each worker
	event (completion, failure, or suspension). Scheduling fills all currently
	available `max_concurrency` slots before the next evaluation, so the predicate
	does not observe each individual branch admission; set `max_concurrency` to
	bound how many branches can start. The initial snapshot has
	`completed_count == 0`, and unscheduled items have status `None`, so handle
	missing statuses explicitly.

[github-actions]

Codex AI review

Found one correctness issue in the documented should_complete scheduling semantics. Runtime behavior is otherwise unchanged by this documentation-focused PR.

Reviewed commit 25de7cc4de8dae664f8f561985d5d4e3aabc9125. Workflow run