From f155e6651ac44f75511c1af09a4dad8319651de8 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 10:58:13 +0700 Subject: [PATCH] audit: fix concurrent subgraph loss, fail-closed ownership, drop barrier compat --- .../plans/2026-09-04-foreach-back-edges.md | 90 +++--- .../2026-09-04-foreach-back-edge-design.md | 15 +- src/wf_core/analysis/context_scopes.py | 11 +- src/wf_core/runtime/foreach_state.py | 159 +++------- src/wf_core/runtime/lineage.py | 27 +- src/wf_core/runtime/ops/flow.py | 9 +- src/wf_core/runtime/ops/foreach.py | 153 +++++----- src/wf_core/runtime/ops/nodes.py | 24 +- src/wf_core/runtime/subgraphs.py | 26 +- tests/core/test_foreach_back_edges.py | 100 ++++++ tests/core/test_foreach_barrier_state.py | 288 +++++++----------- 11 files changed, 424 insertions(+), 478 deletions(-) diff --git a/docs/historical/superpowers/plans/2026-09-04-foreach-back-edges.md b/docs/historical/superpowers/plans/2026-09-04-foreach-back-edges.md index 8833ed82..44831ced 100644 --- a/docs/historical/superpowers/plans/2026-09-04-foreach-back-edges.md +++ b/docs/historical/superpowers/plans/2026-09-04-foreach-back-edges.md @@ -3,7 +3,7 @@ > **For agentic workers:** REQUIRED SUB-SKILL: Use > superpowers:subagent-driven-development (recommended) or > superpowers:executing-plans to implement this plan task-by-task. Steps use -> checkbox (`- [ ]`) syntax for tracking. +> checkbox (`- [x]`) syntax for tracking. **Goal:** Make foreach bodies return through validated back-edges to their immediate owner, with unique static control regions and fresh persisted state @@ -19,7 +19,7 @@ back-edge as item completion. pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. **Spec:** -[`docs/superpowers/specs/2026-09-04-foreach-back-edge-design.md`](../specs/2026-09-04-foreach-back-edge-design.md) +[`docs/superpowers/specs/2026-09-04-foreach-back-edge-design.md`](../../superpowers/specs/2026-09-04-foreach-back-edge-design.md) ## Global Constraints @@ -85,7 +85,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. - `owner_stack_by_node` contains only nodes whose region is unambiguous. Later context analysis must not grant foreach fields to a conflicted node. -- [ ] **Step 1: Write failing acceptance tests for legal regions** +- [x] **Step 1: Write failing acceptance tests for legal regions** Add explicit tests named: @@ -108,7 +108,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. assert analysis.issues == () ``` -- [ ] **Step 2: Run legal-region tests and confirm the missing module fails** +- [x] **Step 2: Run legal-region tests and confirm the missing module fails** Run: @@ -119,7 +119,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: collection fails because `wf_core.analysis.control_regions` does not exist. -- [ ] **Step 3: Implement semantic traversal over node and owner stack** +- [x] **Step 3: Implement semantic traversal over node and owner stack** Use a bounded worklist of `(node_id, owner_stack)` states. The special edge handling must follow this order: @@ -149,7 +149,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Ignore unknown sources and targets here because ordinary edge validation already owns those diagnostics. -- [ ] **Step 4: Write failing tests for every invalid pressure case** +- [x] **Step 4: Write failing tests for every invalid pressure case** Add one explicit test per topology: @@ -172,7 +172,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. ) ``` -- [ ] **Step 5: Implement conflicts, reachability, and returnability** +- [x] **Step 5: Implement conflicts, reachability, and returnability** Record the first stack for each node. If a second distinct stack reaches the same node, remove it from `owner_stack_by_node` and emit one region conflict. @@ -185,7 +185,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Suppress cascading no-return diagnostics when a region conflict, invalid return, invalid terminal, or empty body already makes that state ambiguous. -- [ ] **Step 6: Run analyzer tests** +- [x] **Step 6: Run analyzer tests** Run: @@ -198,7 +198,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: all commands pass. -- [ ] **Step 7: Commit the analyzer** +- [x] **Step 7: Commit the analyzer** ```bash git add src/wf_core/analysis/control_regions.py \ @@ -223,7 +223,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. `loop_item`, `loop_index`, and its alias; completing it restores the outer context. It does not expose all enclosing aliases. -- [ ] **Step 1: Rewrite context tests to canonical back-edges** +- [x] **Step 1: Rewrite context tests to canonical back-edges** Replace successful item routes such as: @@ -244,7 +244,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. {"from": "after_inner", "outcome": "ok", "to": "outer"} ``` -- [ ] **Step 2: Replace the mixed-reachability expectation** +- [x] **Step 2: Replace the mixed-reachability expectation** Delete the test that expects one node to receive conditional loop fields when reached both inside and outside a foreach. Add a test proving conflicted nodes @@ -260,7 +260,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. assert "inner_item" not in after_inner ``` -- [ ] **Step 3: Run the context tests and confirm old traversal fails** +- [x] **Step 3: Run the context tests and confirm old traversal fails** Run: @@ -271,7 +271,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: failures show that the single `FrameScope` traversal neither pops canonical return edges nor consumes region-conflict diagnostics. -- [ ] **Step 4: Replace duplicate traversal with the analyzer result** +- [x] **Step 4: Replace duplicate traversal with the analyzer result** Remove the local breadth-first scope traversal. For each unambiguous node, derive its active context from the final stack item: @@ -286,7 +286,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. diagnostics to bounded context warnings. Do not reintroduce multiple scopes or conditional fields for a single node use. -- [ ] **Step 5: Run context and authoring-contract tests** +- [x] **Step 5: Run context and authoring-contract tests** Run: @@ -300,7 +300,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: all commands pass. -- [ ] **Step 6: Commit context integration** +- [x] **Step 6: Commit context integration** ```bash git add src/wf_core/analysis/context_scopes.py \ @@ -360,7 +360,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. - Callers compare owner fields by name; remove tuple slicing and positional unpacking. -- [ ] **Step 1: Write failing activation-lifecycle tests** +- [x] **Step 1: Write failing activation-lifecycle tests** Add tests proving: @@ -379,7 +379,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Also test malformed metadata, mode mismatch, closing a stale activation, and JSON round-trip through `ExecutionFrame.metadata`. -- [ ] **Step 2: Run activation tests and confirm failure** +- [x] **Step 2: Run activation tests and confirm failure** Run: @@ -389,7 +389,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: imports fail because activation lifecycle helpers do not exist. -- [ ] **Step 3: Implement the activation metadata seam** +- [x] **Step 3: Implement the activation metadata seam** Hide the JSON dictionary shape inside `foreach_state.py`. Persist, per parent frame and foreach node id, a monotonically increasing visit sequence plus at @@ -400,7 +400,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. retain a compatibility reader for the old barrier-only shape because the spec found no real persisted foreach data. -- [ ] **Step 4: Add activation identity to item metadata and helpers** +- [x] **Step 4: Add activation identity to item metadata and helpers** Require this shape: @@ -417,14 +417,14 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Child frame and lineage ids must include `activation.id`, so a later visit at item index zero cannot collide with the first visit. -- [ ] **Step 5: Move runtime callers onto named owner and activation state** +- [x] **Step 5: Move runtime callers onto named owner and activation state** Update lineage reads, node-result buffering, async batching, failure collection, refill, and barrier commit to load the activation named by the child. Fail closed when a child result names a closed or different active activation. -- [ ] **Step 6: Update focused metadata tests** +- [x] **Step 6: Update focused metadata tests** Replace hand-written item metadata in `test_foreach_barrier_state.py` and `test_scheduler.py` with required activation ids. Update hard-coded child @@ -432,7 +432,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. include the activation identity. Assert `item_frame_owner` returns `ForeachItemOwner`, not a tuple. -- [ ] **Step 7: Run runtime-state tests** +- [x] **Step 7: Run runtime-state tests** Run: @@ -449,7 +449,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: all commands pass. -- [ ] **Step 8: Commit activation identity** +- [x] **Step 8: Commit activation identity** ```bash git add src/wf_core/runtime/foreach_state.py \ @@ -488,7 +488,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. - Produces an internal helper that derives ancestor foreach owners from frame ancestry for defensive non-local-return rejection. -- [ ] **Step 1: Write failing serial return tests** +- [x] **Step 1: Write failing serial return tests** Add tests with canonical edges: @@ -501,7 +501,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Prove two items execute, the child finishes at `each`, the parent wakes, and the final workflow outcome remains `ok`. -- [ ] **Step 2: Write failing cycle, nested, and re-entry runtime tests** +- [x] **Step 2: Write failing cycle, nested, and re-entry runtime tests** Add explicit tests named: @@ -518,7 +518,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. direct defensive test constructs an invalid frame chain without running workflow preparation and asserts `WorkflowExecutionError`. -- [ ] **Step 3: Implement immediate-owner return in frame advancement** +- [x] **Step 3: Implement immediate-owner return in frame advancement** Before `END` handling or ordinary enqueue: @@ -540,20 +540,20 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. an item targets `END`, or targets a foreach found below its immediate owner in the active ancestor chain, raise `WorkflowExecutionError` defensively. -- [ ] **Step 4: Close activations before controller completion edges** +- [x] **Step 4: Close activations before controller completion edges** In both serial and concurrent completion paths, close the active activation before calling `advance_frame` for `done` or `completed_with_errors`. This makes a self-looping or later returning completion edge start a fresh visit. -- [ ] **Step 5: Migrate executable foreach fixtures** +- [x] **Step 5: Migrate executable foreach fixtures** Change item-success routes from `END` to their owner in every file listed for this task. Keep controller completion routes to `END` or their real outer continuation. For nested fixtures, return inner bodies to the inner foreach and outer-tail nodes to the outer foreach. -- [ ] **Step 6: Prove concurrent, async, error, and interrupt behavior** +- [x] **Step 6: Prove concurrent, async, error, and interrupt behavior** Run: @@ -572,7 +572,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. fail-closed test showing a completed activation cannot accept a result or wake-up from another activation. -- [ ] **Step 7: Run runtime static checks** +- [x] **Step 7: Run runtime static checks** Run: @@ -588,7 +588,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: all commands pass. -- [ ] **Step 8: Commit runtime back-edges** +- [x] **Step 8: Commit runtime back-edges** ```bash git add src/wf_core/runtime/ops/flow.py \ @@ -624,7 +624,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. - Keeps `Workflow.validate_structure()` and `ValidationReport` signatures unchanged. -- [ ] **Step 1: Add failing public-validation assertions** +- [x] **Step 1: Add failing public-validation assertions** For every pressure-case test, call both the pure analyzer and `workflow.validate_structure()`. Invalid cases must assert the public code and @@ -642,7 +642,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Legal cases assert `report.ok`. Add a test proving every unreachable node in one component receives its own `UNREACHABLE_NODE` issue. -- [ ] **Step 2: Run public-validation tests and confirm failure** +- [x] **Step 2: Run public-validation tests and confirm failure** Run: @@ -652,7 +652,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: analyzer tests pass, but public reports lack the new issue codes. -- [ ] **Step 3: Wire analysis into validation once** +- [x] **Step 3: Wire analysis into validation once** Add enum members with exactly the analyzer values. Call `analyze_control_regions(workflow)` after ordinary node and edge validation, @@ -668,7 +668,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Do not add a second graph traversal inside validation. -- [ ] **Step 4: Canonicalize policy and draft fixtures** +- [x] **Step 4: Canonicalize policy and draft fixtures** Policy-only workflow helpers must include a distinct body node and route it back to the foreach owner. Draft fixtures with: @@ -688,7 +688,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Parse-only policy fixtures still use a distinct body; do not preserve an invalid `loop -> __end__` shortcut just because the test does not execute it. -- [ ] **Step 5: Run validation and draft suites** +- [x] **Step 5: Run validation and draft suites** Run: @@ -703,7 +703,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. `UNREACHABLE_NODE` when disconnection is the behavior under test. Do not add an allow-unreachable flag. -- [ ] **Step 6: Run core validation static checks** +- [x] **Step 6: Run core validation static checks** Run: @@ -717,7 +717,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: all commands pass. -- [ ] **Step 7: Commit fail-closed validation** +- [x] **Step 7: Commit fail-closed validation** ```bash git add src/wf_core/validation tests/core/test_foreach_control_regions.py \ @@ -744,7 +744,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. surface. - Preserves historical reports and recorded agent-challenge outputs verbatim. -- [ ] **Step 1: Search for stale canonical foreach returns** +- [x] **Step 1: Search for stale canonical foreach returns** Run targeted searches: @@ -760,7 +760,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. completion changes to the owner. Do not rewrite unrelated ordinary terminal routes or immutable historical evidence. -- [ ] **Step 2: Update live architecture and authoring docs** +- [x] **Step 2: Update live architecture and authoring docs** Replace the old architecture statement that item children reach `END` with: @@ -781,13 +781,13 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Document that region conflicts, unreachable nodes, body terminals, non-local returns, empty bodies, and bodies without possible returns fail validation. -- [ ] **Step 3: Mark the design implemented and roadmap item complete** +- [x] **Step 3: Mark the design implemented and roadmap item complete** Set the spec status to `Implemented on 2026-09-04`. Move the roadmap bullet from active correction to recently completed runtime work. Keep fork/gather explicitly deferred. -- [ ] **Step 4: Run the focused acceptance matrix** +- [x] **Step 4: Run the focused acceptance matrix** Run: @@ -806,7 +806,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. Expected: every current pressure-case row passes. The future-fork row remains documented and unimplemented because no fork node exists. -- [ ] **Step 5: Run repository verification** +- [x] **Step 5: Run repository verification** Run: @@ -826,7 +826,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. unrelated lint debt, do not run an unsafe global auto-fix; report it and keep this slice's edited documents clean. -- [ ] **Step 6: Commit implementation documentation** +- [x] **Step 6: Commit implementation documentation** ```bash git add docs/wf_core_architecture.md docs/wf_authoring_control_flow.md \ @@ -835,7 +835,7 @@ pytest, pytest-asyncio, Ruff, basedpyright, markdownlint-cli2. git commit -m "docs: publish foreach back-edge semantics" ``` -- [ ] **Step 7: Archive the completed plan** +- [x] **Step 7: Archive the completed plan** After every prior task is complete and committed: diff --git a/docs/superpowers/specs/2026-09-04-foreach-back-edge-design.md b/docs/superpowers/specs/2026-09-04-foreach-back-edge-design.md index f6673a96..10180cca 100644 --- a/docs/superpowers/specs/2026-09-04-foreach-back-edge-design.md +++ b/docs/superpowers/specs/2026-09-04-foreach-back-edge-design.md @@ -203,9 +203,10 @@ foreach_owner_stack)` rather than merely looking for graph cycles: A node use therefore belongs to one static control region, while remaining free to execute in any number of dynamic frames, lineages, or items. When the same capability is needed at two program locations, authoring creates two node uses -with distinct identifiers. Existing context-contract analysis may still report -fields as conditional because multiple paths can reach a node within its one -region; it must not use multiple owner stacks to represent that case. +with distinct identifiers. Context-contract analysis reports one field set per +static region: fields are available when the region is inside a foreach body +and absent outside it. A node reached under two stacks is a region conflict +and receives no foreach fields rather than a conditional union. The unique-owner rule rejects both ways of crossing a foreach boundary. An outside edge into a body node reaches that node under both the outer and item @@ -433,7 +434,11 @@ may independently return and complete the item. Back-edge return changes control representation, not state semantics. Iteration writes remain buffered in the item lineage. Serial behavior and the concurrent barrier continue to commit or merge those writes according to the -accepted concurrent-foreach ADR and declared reducers. +accepted concurrent-foreach ADR and declared reducers. The completed item is +registered with its barrier at the owner back-edge, keyed by the returning +frame rather than by whichever operation ran last, so node, subgraph, and +nested-control endings all count. A return naming a closed or superseded +activation fails closed instead of buffering into the wrong visit. An ordinary node outcome named `error` remains domain control. An exception remains a runtime item failure handled by `fail`, `skip`, or `collect`. Neither @@ -490,7 +495,7 @@ semantics also run through the runtime. | Closed body cycle | Reject missing owner return | N/A | | Unreachable nodes | Reject each node | N/A | | Re-enter foreach after `done` | Accept | Fresh activation and children | -| Subgraph inside foreach | Accept | Child `END`, then item return | +| Subgraph inside foreach | Accept | Child `END`, then item return (serial and concurrent) | | Interrupt inside foreach | Accept | Resume the same item activation | | Future fork in foreach | Deferred with fork/gather | Gather before return | diff --git a/src/wf_core/analysis/context_scopes.py b/src/wf_core/analysis/context_scopes.py index 1d2b23ce..86f3d020 100644 --- a/src/wf_core/analysis/context_scopes.py +++ b/src/wf_core/analysis/context_scopes.py @@ -68,11 +68,12 @@ def context_fields_by_node( ) -> dict[str, tuple[ContextFieldAvailability, ...]]: """Return runtime context contracts for every reachable graph node. - This is an abstract execution-frame analysis rather than ordinary graph - reachability: the same node can execute in the root frame and in a - foreach child frame, and those frames expose different context keys. - The traversal memoizes both node id and active frame scope so cyclic - graphs terminate without granting aliases from an impossible scope. + This is an abstract execution-frame analysis keyed by static control + region: each node use belongs to exactly one foreach-owner stack, and + that stack decides which foreach aliases the node exposes. A node + reachable under two stacks is a region conflict and receives no foreach + fields. The traversal still memoizes node id and owner stack so cyclic + graphs terminate. """ return _analyze(workflow).fields_by_node diff --git a/src/wf_core/runtime/foreach_state.py b/src/wf_core/runtime/foreach_state.py index eed386db..41fc86d6 100644 --- a/src/wf_core/runtime/foreach_state.py +++ b/src/wf_core/runtime/foreach_state.py @@ -4,13 +4,9 @@ from dataclasses import dataclass, field from typing import Any, Literal from wf_core.errors import WorkflowExecutionError -from wf_core.models.reducers import ReducerRef -from wf_core.paths import StatePath -from wf_core.run_state import ExecutionFrame, StateWrite -from wf_core.runtime.ops.state import StatePatch +from wf_core.run_state import ExecutionFrame, RunState from wf_core.runtime.scheduler import ForeachIterationMetadata -_BARRIER_METADATA_KEY = "foreach_barriers" _ACTIVATION_METADATA_KEY = "foreach_activations" @@ -93,16 +89,14 @@ class ItemErrorRecord: class PendingItemResult: """Buffered item result waiting for a future foreach barrier commit. - New concurrent foreach execution stores item writes in `RunState.lineages` - and records `lineage_id` here. `patch` remains for old serialized barrier - metadata and direct unit tests that still construct pending patches. + Concurrent item writes live in `RunState.lineages`; the barrier keeps + only the lineage identity per item index. """ index: int frame_id: str status: Literal["succeeded", "failed"] lineage_id: str | None = None - patch: StatePatch = field(default_factory=StatePatch) error: ItemErrorRecord | None = None @classmethod @@ -117,34 +111,23 @@ class PendingItemResult: raise WorkflowExecutionError( f"malformed pending foreach result missing {exc.args[0]!r}" ) from exc - patch_changes = raw.get("patch_changes", {}) - patch_writes = raw.get("patch_writes") lineage_id = raw.get("lineage_id") if not isinstance(index, int) or index < 0: raise WorkflowExecutionError("malformed pending foreach result index") if not isinstance(frame_id, str): raise WorkflowExecutionError("malformed pending foreach result frame id") + if status == "succeeded" and not isinstance(lineage_id, str): + raise WorkflowExecutionError("malformed pending foreach result lineage id") if lineage_id is not None and not isinstance(lineage_id, str): raise WorkflowExecutionError("malformed pending foreach result lineage id") if status not in {"succeeded", "failed"}: raise WorkflowExecutionError("malformed pending foreach result status") - if not isinstance(patch_changes, dict): - raise WorkflowExecutionError("malformed pending foreach result patch") - if patch_writes is not None and not isinstance(patch_writes, list): - raise WorkflowExecutionError("malformed pending foreach result writes") raw_error = raw.get("error") return cls( index=index, frame_id=frame_id, status=status, lineage_id=lineage_id, - patch=( - StatePatch( - writes=[_state_write_from_metadata(item) for item in patch_writes] - ) - if patch_writes is not None - else StatePatch(changes=patch_changes) - ), error=( ItemErrorRecord.from_metadata(raw_error) if raw_error is not None @@ -158,10 +141,6 @@ class PendingItemResult: "frame_id": self.frame_id, "status": self.status, "lineage_id": self.lineage_id, - "patch_changes": dict(self.patch.changes), - "patch_writes": [ - _state_write_to_metadata(write) for write in self.patch.writes - ], "error": self.error.to_metadata() if self.error is not None else None, } @@ -176,33 +155,6 @@ class ForeachBarrierState: outstanding_frame_ids: tuple[str, ...] = () pending_results: dict[int, PendingItemResult] = field(default_factory=dict) - @classmethod - def from_frame( - cls, - frame: ExecutionFrame, - foreach_node_id: str, - ) -> ForeachBarrierState | None: - """Load one foreach barrier state from frame metadata. - - Missing metadata means the foreach has not started on this frame yet. - Malformed metadata means runtime state is corrupt and should fail fast. - """ - all_barriers = frame.metadata.get(_BARRIER_METADATA_KEY) - if all_barriers is None: - return None - if not isinstance(all_barriers, dict): - raise WorkflowExecutionError( - f"malformed foreach barrier table for frame {frame.id!r}" - ) - raw = all_barriers.get(foreach_node_id) - if raw is None: - return None - if not isinstance(raw, dict): - raise WorkflowExecutionError( - f"malformed foreach barrier state for frame {frame.id!r}" - ) - return cls.from_metadata(raw) - @classmethod def from_metadata(cls, raw: object) -> ForeachBarrierState: if not isinstance(raw, dict): @@ -235,20 +187,6 @@ class ForeachBarrierState: pending_results=parsed_results, ) - def save_to_frame(self, frame: ExecutionFrame, foreach_node_id: str) -> None: - """Store this barrier state in frame metadata under its foreach node id.""" - existing = frame.metadata.get(_BARRIER_METADATA_KEY) - if existing is None: - frame.metadata[_BARRIER_METADATA_KEY] = { - foreach_node_id: self.to_metadata() - } - return - if not isinstance(existing, dict): - raise WorkflowExecutionError( - f"malformed foreach barrier table for frame {frame.id!r}" - ) - existing[foreach_node_id] = self.to_metadata() - def to_metadata(self) -> dict[str, Any]: return { "next_index": self.next_index, @@ -291,15 +229,13 @@ class ForeachBarrierState: *, index: int, frame_id: str, - patch: StatePatch, - lineage_id: str | None = None, + lineage_id: str, ) -> None: - """Buffer or extend successful item patches by item index. + """Record one completed concurrent item by lineage identity. - New runtime paths pass an empty patch and use `lineage_id`; legacy - callers may still accumulate patches here and replay them at the - barrier. Do not merge `_prepared_writes`: the barrier replays public - write records against one staged parent state. + Registration is idempotent for the same frame and lineage so the + owner back-edge can own it regardless of which operation ran last. + Any conflicting identity fails closed. """ existing = self.pending_results.get(index) if existing is None: @@ -308,7 +244,6 @@ class ForeachBarrierState: frame_id=frame_id, status="succeeded", lineage_id=lineage_id, - patch=patch, ) return if existing.frame_id != frame_id: @@ -316,14 +251,11 @@ class ForeachBarrierState: f"foreach item result for index {index!r} belongs to frame " f"{existing.frame_id!r}, got {frame_id!r}" ) - if lineage_id is not None and existing.lineage_id not in {None, lineage_id}: + if existing.lineage_id != lineage_id: raise WorkflowExecutionError( f"foreach item result for index {index!r} belongs to lineage " f"{existing.lineage_id!r}, got {lineage_id!r}" ) - if existing.lineage_id is None: - existing.lineage_id = lineage_id - existing.patch.extend(patch) def add_failure(self, *, error: ItemErrorRecord) -> None: """Buffer one handled item failure for the foreach barrier. @@ -547,53 +479,30 @@ def _string_tuple(raw: object) -> tuple[str, ...]: raise WorkflowExecutionError("malformed foreach barrier frame id list") -def _state_write_from_metadata(raw: object) -> StateWrite: - """Parse one persisted item-lineage write record. +def register_foreach_item_success( + run: RunState, frame: ExecutionFrame, owner: ForeachItemOwner +) -> None: + """Record one completed concurrent item at its owner back-edge. - Barrier metadata must keep reducer-visible values across interrupt/resume; - reconstructing from `patch_changes` would downgrade reducer writes to - replace-style incoming values. + Registration keys off the returning frame, so it works regardless of + which operation ran last in the item (node, subgraph, or nested + control). Serial items commit through the parent at operation time and + need no barrier entry. A closed or superseded activation fails closed. """ - if not isinstance(raw, dict): - raise WorkflowExecutionError("malformed pending foreach write") - try: - path = raw["path"] - incoming_value = raw["incoming_value"] - visible_value = raw["visible_value"] - reducer = raw["reducer"] - except KeyError as exc: + parent_frame = run.frames.get(owner.parent_frame_id) + if parent_frame is None: raise WorkflowExecutionError( - f"malformed pending foreach write missing {exc.args[0]!r}" - ) from exc - try: - return StateWrite( - path=_state_path_from_metadata(path), - incoming_value=incoming_value, - visible_value=visible_value, - reducer=ReducerRef.model_validate(reducer), + "foreach lineage state references missing parent frame " + f"{owner.parent_frame_id!r} for child frame {frame.id!r}" ) - except WorkflowExecutionError: - raise - except (TypeError, ValueError) as exc: - raise WorkflowExecutionError(f"malformed pending foreach write: {exc}") from exc - - -def _state_write_to_metadata(write: StateWrite) -> dict[str, Any]: - """Serialize one item-lineage write without relying on dotted display paths.""" - return { - "path": {"root": "state", "parts": list(write.path.parts)}, - "incoming_value": write.incoming_value, - "visible_value": write.visible_value, - "reducer": write.reducer.model_dump(mode="json"), - } - - -def _state_path_from_metadata(raw: object) -> StatePath: - if isinstance(raw, str): - return StatePath.parse(raw) - if not isinstance(raw, dict) or raw.get("root") != "state": - raise WorkflowExecutionError("malformed pending foreach write path") - parts = raw.get("parts") - if not isinstance(parts, list) or not all(isinstance(part, str) for part in parts): - raise WorkflowExecutionError("malformed pending foreach write path") - return StatePath(tuple(parts)) + activation = require_foreach_activation( + parent_frame, owner.foreach_node_id, owner.activation_id + ) + if activation.barrier.mode != "concurrent": + return + activation.barrier.add_success_patch( + index=owner.item_index, + frame_id=frame.id, + lineage_id=frame.lineage_id, + ) + save_foreach_activation(parent_frame, activation) diff --git a/src/wf_core/runtime/lineage.py b/src/wf_core/runtime/lineage.py index 800d82a9..77c762aa 100644 --- a/src/wf_core/runtime/lineage.py +++ b/src/wf_core/runtime/lineage.py @@ -7,7 +7,6 @@ from typing import Any from wf_core.errors import WorkflowExecutionError from wf_core.run_state import ExecutionFrame, LineageState, RunState, StateWrite -from wf_core.runtime.foreach_state import item_frame_owner, load_foreach_activation from wf_core.runtime.ops.state import ( StatePatch, commit_state_patch, @@ -57,31 +56,7 @@ def lineage_writes_for_frame( run, scope_id=frame.scope_id, lineage_id=frame.lineage_id ) ) - - # Compatibility fallback: concurrent foreach used barrier-local patches - # before `RunState.lineages` became the primary write store. Keep reading - # those patches so old serialized runs and direct barrier tests still work. - # Barrier lookup includes the activation so a stale visit cannot read a - # later activation's buffered writes. - owner = item_frame_owner(frame) - if owner is None: - return () - parent_frame = run.frames.get(owner.parent_frame_id) - if parent_frame is None: - raise WorkflowExecutionError( - "foreach lineage compatibility state references missing parent frame " - f"{owner.parent_frame_id!r} for child frame {frame.id!r}" - ) - activation = load_foreach_activation( - parent_frame, owner.foreach_node_id, owner.activation_id - ) - if activation is None or activation.barrier.mode != "concurrent": - return () - - pending = activation.barrier.pending_results.get(owner.item_index) - if pending is None: - return () - return pending.patch.writes + return () def is_scope_root_lineage_frame(run: RunState, frame: ExecutionFrame) -> bool: diff --git a/src/wf_core/runtime/ops/flow.py b/src/wf_core/runtime/ops/flow.py index 2f50a70c..237d498d 100644 --- a/src/wf_core/runtime/ops/flow.py +++ b/src/wf_core/runtime/ops/flow.py @@ -80,7 +80,10 @@ def advance_frame( # Foreach back-edge return is an ownership check, not generic cycle # detection. Only the frame's immediate recorded owner completes the item; # a root frame targeting the same foreach enters it normally. - from wf_core.runtime.foreach_state import item_frame_owner + from wf_core.runtime.foreach_state import ( + item_frame_owner, + register_foreach_item_success, + ) owner = item_frame_owner(frame) if owner is not None: @@ -91,6 +94,10 @@ def advance_frame( ) if next_node_id == owner.foreach_node_id: source_node_id = frame.node_id + # Register the completed item with its barrier before completing + # the child, so every final operation (node, subgraph, nested + # control) counts. Closed or superseded activations fail closed. + register_foreach_item_success(run, frame, owner) frame.prior_outcome = outcome frame.activated_incoming_edge = source_node_id frame.node_id = owner.foreach_node_id diff --git a/src/wf_core/runtime/ops/foreach.py b/src/wf_core/runtime/ops/foreach.py index 99c0c972..87739b64 100644 --- a/src/wf_core/runtime/ops/foreach.py +++ b/src/wf_core/runtime/ops/foreach.py @@ -96,40 +96,16 @@ def _step_foreach_serial( loop_start = index.next_node_id(frame.node_id, "loop") item = iterable[loop_index] - barrier.next_index = loop_index + 1 + loop_start, child_id = _admit_item_frame( + run=run, + frame=frame, + step=step, + index=index, + activation=activation, + loop_index=loop_index, + item=item, + ) save_foreach_activation(frame, activation) - child_id = _child_frame_id(activation, loop_index) - child_lineage_id = _child_lineage_id(activation, loop_index) - # Serial items still own a lineage so nested subgraph/boundary commits have - # a parent lineage to buffer into; top-level serial writes commit through - # the parent scope root. - add_lineage( - run, - scope_id=frame.scope_id, - lineage_id=child_lineage_id, - parent_id=frame.lineage_id, - ) - add_frame( - run, - ExecutionFrame( - id=child_id, - kind="foreach_iteration", - node_id=loop_start, - status=FrameStatus.PENDING, - parent_frame_id=frame.id, - scope_id=frame.scope_id, - lineage_id=child_lineage_id, - parent_lineage_id=frame.lineage_id, - metadata=ForeachIterationMetadata( - foreach_node_id=step.id, - activation_id=activation.id, - loop_index=loop_index, - loop_item=item, - loop_alias=step.as_, - ).to_metadata(), - ), - ready=True, - ) block_frame_on_children(run, frame.id, (child_id,)) append_step_result_trace( run, @@ -251,6 +227,58 @@ def _item_error_record(child: ExecutionFrame) -> ItemErrorRecord: ) +def _admit_item_frame( + *, + run: RunState, + frame: ExecutionFrame, + step: ForeachNode, + index: WorkflowIndex, + activation: ForeachActivationState, + loop_index: int, + item: object, +) -> tuple[str, str]: + """Create one activation-qualified child frame and lineage. + + Every item owns a lineage so nested subgraph/boundary commits have a + parent lineage to buffer into; top-level serial writes still commit + through the parent scope root. Returns the loop start node and child id; + barrier child bookkeeping stays with the caller. Compare ids by name; + never parse them. + """ + loop_start = index.next_node_id(frame.node_id, "loop") + child_id = _child_frame_id(activation, loop_index) + child_lineage_id = _child_lineage_id(activation, loop_index) + add_lineage( + run, + scope_id=frame.scope_id, + lineage_id=child_lineage_id, + parent_id=frame.lineage_id, + ) + activation.barrier.next_index = loop_index + 1 + add_frame( + run, + ExecutionFrame( + id=child_id, + kind="foreach_iteration", + node_id=loop_start, + status=FrameStatus.PENDING, + parent_frame_id=frame.id, + scope_id=frame.scope_id, + lineage_id=child_lineage_id, + parent_lineage_id=frame.lineage_id, + metadata=ForeachIterationMetadata( + foreach_node_id=step.id, + activation_id=activation.id, + loop_index=loop_index, + loop_item=item, + loop_alias=step.as_, + ).to_metadata(), + ), + ready=True, + ) + return loop_start, child_id + + def _admit_concurrent_children( *, run: RunState, @@ -272,38 +300,17 @@ def _admit_concurrent_children( ): loop_index = barrier.next_index item = iterable[loop_index] - child_id = _child_frame_id(activation, loop_index) - child_lineage_id = _child_lineage_id(activation, loop_index) - add_lineage( - run, - scope_id=frame.scope_id, - lineage_id=child_lineage_id, - parent_id=frame.lineage_id, - ) active_count = len(barrier.active_frame_ids) - barrier.next_index = loop_index + 1 - barrier.start_child(child_id) - add_frame( - run, - ExecutionFrame( - id=child_id, - kind="foreach_iteration", - node_id=loop_start, - status=FrameStatus.PENDING, - parent_frame_id=frame.id, - scope_id=frame.scope_id, - lineage_id=child_lineage_id, - parent_lineage_id=frame.lineage_id, - metadata=ForeachIterationMetadata( - foreach_node_id=step.id, - activation_id=activation.id, - loop_index=loop_index, - loop_item=item, - loop_alias=step.as_, - ).to_metadata(), - ), - ready=True, + loop_start, child_id = _admit_item_frame( + run=run, + frame=frame, + step=step, + index=index, + activation=activation, + loop_index=loop_index, + item=item, ) + barrier.start_child(child_id) append_step_result_trace( run, frame_id=frame.id, @@ -417,14 +424,16 @@ def _patch_for_successful_item( ) -> StatePatch: """Return the replayable patch for a completed foreach item. - New concurrent foreach results store writes in `RunState.lineages` and keep - only lineage metadata in the barrier. Old serialized barrier metadata may - still carry `result.patch`, so keep that as the compatibility fallback. + Item writes live in `RunState.lineages`; a success without a known + lineage is corrupt state and fails closed. """ - if result.lineage_id is not None and result.lineage_id in run.lineages: - return lineage_patch( - run, - scope_id=frame.scope_id, - lineage_id=result.lineage_id, + if result.lineage_id is None or result.lineage_id not in run.lineages: + raise WorkflowExecutionError( + f"foreach item result for index {result.index!r} references " + f"unknown lineage {result.lineage_id!r}" ) - return result.patch + return lineage_patch( + run, + scope_id=frame.scope_id, + lineage_id=result.lineage_id, + ) diff --git a/src/wf_core/runtime/ops/nodes.py b/src/wf_core/runtime/ops/nodes.py index 81d6165e..6c73976b 100644 --- a/src/wf_core/runtime/ops/nodes.py +++ b/src/wf_core/runtime/ops/nodes.py @@ -18,7 +18,6 @@ from wf_core.run_state import ( from wf_core.runtime.foreach_state import ( item_frame_owner, require_foreach_activation, - save_foreach_activation, ) from wf_core.runtime.input_bindings import resolve_step_input_bindings from wf_core.runtime.lineage import ( @@ -30,7 +29,7 @@ from wf_core.runtime.ops.frames import frame_context_values from wf_core.runtime.ops.merges import ReducerDefinition from wf_core.runtime.ops.overlays import state_view_for_frame from wf_core.runtime.ops.schemas import validate_payload_against_schema -from wf_core.runtime.ops.state import StatePatch, build_output_patch +from wf_core.runtime.ops.state import build_output_patch NodeHandler = Callable[[dict[str, Any], RuntimeContext], NodeResult | dict[str, Any]] AsyncNodeHandler = Callable[ @@ -120,29 +119,26 @@ def _finalize_node_execution( if owner is None: state_changes = commit_patch_for_frame(run, frame, patch) else: - parent_frame = run.frames[owner.parent_frame_id] + parent_frame = run.frames.get(owner.parent_frame_id) + if parent_frame is None: + raise WorkflowExecutionError( + "foreach item state references missing parent frame " + f"{owner.parent_frame_id!r} for child frame {frame.id!r}" + ) # Fail closed when the child names a closed or superseded activation: # its writes must not land in a later visit's barrier. activation = require_foreach_activation( parent_frame, owner.foreach_node_id, owner.activation_id ) - barrier = activation.barrier - if barrier.mode == "concurrent": - # New concurrent foreach stores writes in the child lineage; the - # barrier keeps only result metadata plus old patch fallback. + if activation.barrier.mode == "concurrent": + # Concurrent writes stay buffered in the child lineage; the owner + # back-edge registers the completed item with the barrier. append_lineage_writes( run, scope_id=frame.scope_id, lineage_id=frame.lineage_id, writes=patch.writes, ) - barrier.add_success_patch( - index=owner.item_index, - frame_id=frame.id, - patch=StatePatch(), - lineage_id=frame.lineage_id, - ) - save_foreach_activation(parent_frame, activation) state_changes = {} else: state_changes = commit_patch_for_frame(run, parent_frame, patch) diff --git a/src/wf_core/runtime/subgraphs.py b/src/wf_core/runtime/subgraphs.py index 013950c0..89870993 100644 --- a/src/wf_core/runtime/subgraphs.py +++ b/src/wf_core/runtime/subgraphs.py @@ -241,25 +241,25 @@ def _finish_subgraph( # item writes stay buffered in the item lineage for barrier merge. from wf_core.runtime.foreach_state import ( item_frame_owner, - load_foreach_activation, + require_foreach_activation, ) commit_frame = frame - try: - owner = item_frame_owner(frame) - except Exception: - owner = None + owner = item_frame_owner(frame) if owner is not None: parent_frame = run.frames.get(owner.parent_frame_id) - if parent_frame is not None: - foreach_activation = load_foreach_activation( - parent_frame, owner.foreach_node_id, owner.activation_id + if parent_frame is None: + raise WorkflowExecutionError( + "subgraph state references missing parent frame " + f"{owner.parent_frame_id!r} for child frame {frame.id!r}" ) - if ( - foreach_activation is not None - and foreach_activation.barrier.mode == "serial" - ): - commit_frame = parent_frame + # Fail closed when the child names a closed or superseded + # activation: its output must not land in a later visit's state. + foreach_activation = require_foreach_activation( + parent_frame, owner.foreach_node_id, owner.activation_id + ) + if foreach_activation.barrier.mode == "serial": + commit_frame = parent_frame state_changes = commit_patch_for_frame(run, commit_frame, patch) return StepExecutionResult( outcome=child_outcome, diff --git a/tests/core/test_foreach_back_edges.py b/tests/core/test_foreach_back_edges.py index 5512c60b..800c53d1 100644 --- a/tests/core/test_foreach_back_edges.py +++ b/tests/core/test_foreach_back_edges.py @@ -642,6 +642,106 @@ def test_subgraph_end_returns_to_subgraph_node_then_foreach_owner() -> None: assert run.state["seen"] == ["a", "b"] +def test_concurrent_subgraph_item_returns_through_owner() -> None: + from wf_core import PreparedSubgraph + + child = Workflow( + name="child", + input_schema=SchemaRef(type="object", properties={"value": {}}), + state_schema=StateSchema.from_field_map({"seen": StateField(type="string")}), + output_schema=SchemaRef(type="object", properties={"seen": {}}), + node_defs=[ + NodeDef( + name="inner_record", + input_schema=SchemaRef( + type="object", properties={"value": {}}, required=["value"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["ok"], + ) + ], + start="inner_record", + nodes=[ + NodeUse.model_validate( + { + "id": "inner_record", + "type": "node", + "node": "inner_record", + "input": [{"target": "value", "path": "input.value"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ) + ], + edges=[ + Edge.model_validate({"from": "inner_record", "outcome": "ok", "to": END}) + ], + ) + foreach = ForeachNode.model_validate( + { + "id": "each", + "type": "foreach", + "over": "state.items", + "as": "item", + "mode": "concurrent", + "concurrent": {"max_active": 2, "max_outstanding": 2}, + } + ) + workflow = Workflow( + name="foreach_concurrent_subgraph", + input_schema=SchemaRef(type="object", properties={"items": {"type": "array"}}), + state_schema=StateSchema.from_field_map( + { + "items": StateField(type="array"), + "seen": StateField( + type="array", reducer=ReducerRef(name="wf.std.append") + ), + } + ), + output_schema=SchemaRef(type="object", properties={"seen": {"type": "array"}}), + node_defs=[], + start="each", + nodes=[ + foreach, + SubgraphNode.model_validate( + { + "id": "child", + "type": "subgraph", + "workflow": "child.workflow", + "input_schema": {"type": "object", "properties": {"value": {}}}, + "output_schema": {"type": "object", "properties": {"seen": {}}}, + "input": [{"target": "value", "path": "context.item"}], + "output": [{"source": "seen", "target": "state.seen"}], + "outcomes": ["ok"], + } + ), + ], + edges=[ + Edge.model_validate({"from": "each", "outcome": "loop", "to": "child"}), + Edge.model_validate({"from": "child", "outcome": "ok", "to": "each"}), + Edge.model_validate({"from": "each", "outcome": "done", "to": END}), + ], + ) + + run = execute_workflow( + workflow, + {"items": ["a", "b"]}, + {}, + subgraphs={ + "child.workflow": PreparedSubgraph( + workflow=child, + registry={ + "inner_record": lambda payload, _ctx: {"seen": payload["value"]} + }, + ) + }, + ) + + assert run.status == RunStatus.COMPLETED + assert sorted(run.state["seen"]) == ["a", "b"] + + def test_nonlocal_runtime_return_fails_closed_when_validation_is_bypassed() -> None: run = RunState( workflow_name="nonlocal", diff --git a/tests/core/test_foreach_barrier_state.py b/tests/core/test_foreach_barrier_state.py index 9bf44abf..1ac3a439 100644 --- a/tests/core/test_foreach_barrier_state.py +++ b/tests/core/test_foreach_barrier_state.py @@ -5,107 +5,77 @@ import pytest from wf_core.errors import WorkflowExecutionError from wf_core.models.reducers import ReducerRef from wf_core.paths import StatePath -from wf_core.run_state import ExecutionFrame, RunState, RunStatus, StateWrite +from wf_core.run_state import ( + ExecutionFrame, + LineageState, + RunState, + RunStatus, + RuntimeScope, + StateWrite, +) from wf_core.runtime.foreach_state import ( ForeachBarrierState, ForeachItemOwner, ItemErrorRecord, PendingItemResult, - _state_write_from_metadata, item_frame_owner, + load_foreach_activation, load_or_begin_foreach_activation, save_foreach_activation, ) -from wf_core.runtime.lineage import LineageStateView, lineage_writes_for_frame -from wf_core.runtime.ops.state import StatePatch +from wf_core.runtime.lineage import ( + LineageStateView, + add_lineage, + append_lineage_writes, + lineage_writes_for_frame, +) -def test_foreach_barrier_state_round_trips_through_frame_metadata() -> None: +def test_foreach_barrier_state_round_trips_through_activation_metadata() -> None: frame = ExecutionFrame(id="root", kind="root", node_id="each") - barrier = ForeachBarrierState( - next_index=2, - active_frame_ids=("child-1",), - outstanding_frame_ids=("child-1", "child-2"), - pending_results={ - 1: PendingItemResult( - index=1, - frame_id="child-1", - status="failed", - patch=StatePatch(changes={"state.count": 1}), - error=ItemErrorRecord( - index=1, - frame_id="child-1", - node_id="work", - error_type="ValueError", - message="bad item", - item={"id": "a"}, - ), - ) - }, - ) - - barrier.save_to_frame(frame, "each") - loaded = ForeachBarrierState.from_frame(frame, "each") - - assert loaded is not None - assert loaded.next_index == 2 - assert loaded.active_frame_ids == ("child-1",) - assert loaded.outstanding_frame_ids == ("child-1", "child-2") - assert loaded.pending_results[1].patch.changes["state.count"] == 1 - assert loaded.pending_results[1].error is not None - assert loaded.pending_results[1].error.message == "bad item" - - -def test_foreach_barrier_state_round_trips_reducer_write_records() -> None: - frame = ExecutionFrame(id="root", kind="root", node_id="each") - barrier = ForeachBarrierState( - next_index=1, - mode="concurrent", - pending_results={ - 0: PendingItemResult( - index=0, - frame_id="child-0", - status="succeeded", - lineage_id="root/each[0]", - patch=StatePatch( - writes=[ - StateWrite( - path=StatePath(("count",)), - incoming_value=3, - visible_value=5, - reducer=ReducerRef(name="wf.std.add"), - ) - ] - ), - ) - }, - ) - - barrier.save_to_frame(frame, "each") - loaded = ForeachBarrierState.from_frame(frame, "each") - - assert loaded is not None - write = loaded.pending_results[0].patch.writes[0] - assert loaded.pending_results[0].lineage_id == "root/each[0]" - assert write.path == StatePath(("count",)) - assert write.incoming_value == 3 - assert write.visible_value == 5 - assert write.reducer.name == "wf.std.add" - - -def test_pending_write_metadata_preserves_invalid_reducer_detail() -> None: - with pytest.raises(WorkflowExecutionError, match="mutually exclusive"): - _state_write_from_metadata( - { - "path": {"root": "state", "parts": ["count"]}, - "incoming_value": 1, - "visible_value": 1, - "reducer": { - "name": "wf.std.add", - "ref": {"source": "wf.std", "capability_key": "add"}, - }, - } + activation = load_or_begin_foreach_activation(frame, "each", mode="serial") + activation.barrier.next_index = 2 + activation.barrier.start_child("child-1") + activation.barrier.start_child("child-2") + activation.barrier.finish_child("child-2") + activation.barrier.add_failure( + error=ItemErrorRecord( + index=1, + frame_id="child-1", + node_id="work", + error_type="ValueError", + message="bad item", + item={"id": "a"}, ) + ) + save_foreach_activation(frame, activation) + + loaded = load_foreach_activation(frame, "each", activation.id) + + assert loaded is not None + assert loaded.barrier.next_index == 2 + assert loaded.barrier.active_frame_ids == ("child-1",) + assert loaded.barrier.outstanding_frame_ids == ("child-1",) + assert loaded.barrier.pending_results[1].status == "failed" + assert loaded.barrier.pending_results[1].error is not None + assert loaded.barrier.pending_results[1].error.message == "bad item" + + +def test_concurrent_success_round_trips_lineage_identity() -> None: + frame = ExecutionFrame(id="root", kind="root", node_id="each") + activation = load_or_begin_foreach_activation(frame, "each", mode="concurrent") + activation.barrier.add_success_patch( + index=0, + frame_id="child-0", + lineage_id="root:each#0[0]", + ) + save_foreach_activation(frame, activation) + + loaded = load_foreach_activation(frame, "each", activation.id) + + assert loaded is not None + assert loaded.barrier.pending_results[0].lineage_id == "root:each#0[0]" + assert loaded.barrier.pending_results[0].status == "succeeded" def test_lineage_state_view_materializes_visible_values_without_mutating_base() -> None: @@ -136,7 +106,7 @@ def test_lineage_state_view_materializes_visible_values_without_mutating_base() assert base_state["nested"]["value"] == "old" -def test_lineage_writes_for_frame_reads_current_foreach_pending_result() -> None: +def test_lineage_writes_for_frame_reads_item_lineage_store() -> None: parent = ExecutionFrame(id="root", kind="workflow", node_id="each") activation = load_or_begin_foreach_activation(parent, "each", mode="concurrent") child_lineage_id = f"{activation.id}[0]" @@ -160,23 +130,6 @@ def test_lineage_writes_for_frame_reads_current_foreach_pending_result() -> None assert isinstance(owner, ForeachItemOwner) assert owner.activation_id == activation.id assert owner.item_index == 0 - patch = StatePatch( - writes=[ - StateWrite( - path=StatePath(("count",)), - incoming_value=3, - visible_value=5, - reducer=ReducerRef(name="wf.std.add"), - ) - ] - ) - activation.barrier.pending_results[0] = PendingItemResult( - index=0, - frame_id=child.id, - status="succeeded", - lineage_id=child.lineage_id, - patch=patch, - ) save_foreach_activation(parent, activation) run = RunState( workflow_name="lineage", @@ -185,6 +138,27 @@ def test_lineage_writes_for_frame_reads_current_foreach_pending_result() -> None state={"count": 2}, frames={parent.id: parent, child.id: child}, ) + run.scopes["root"] = RuntimeScope( + id="root", + workflow_name="lineage", + workflow_input={}, + committed_state=run.state, + ) + run.lineages["root"] = LineageState(id="root", scope_id="root") + add_lineage(run, scope_id="root", lineage_id=child_lineage_id, parent_id="root") + append_lineage_writes( + run, + scope_id="root", + lineage_id=child_lineage_id, + writes=[ + StateWrite( + path=StatePath(("count",)), + incoming_value=3, + visible_value=5, + reducer=ReducerRef(name="wf.std.add"), + ) + ], + ) writes = lineage_writes_for_frame(run, child) @@ -193,48 +167,16 @@ def test_lineage_writes_for_frame_reads_current_foreach_pending_result() -> None assert writes[0].visible_value == 5 -def test_lineage_writes_for_frame_rejects_missing_compatibility_parent_frame() -> None: - child = ExecutionFrame( - id="missing:each#0:0", - kind="foreach_iteration", - node_id="work", - parent_frame_id="missing", - metadata={ - "foreach_node_id": "each", - "activation_id": "missing:each#0", - "loop_index": 0, - "loop_item": "a", - "loop_alias": "item", - }, - ) - run = RunState( - workflow_name="lineage", - status=RunStatus.PENDING, - workflow_input={}, - state={}, - frames={child.id: child}, - ) +def test_load_activation_returns_none_when_missing() -> None: + from wf_core.runtime.foreach_state import close_foreach_activation - with pytest.raises(WorkflowExecutionError, match="missing parent frame"): - lineage_writes_for_frame(run, child) - - -def test_foreach_barrier_state_returns_none_when_missing() -> None: frame = ExecutionFrame(id="root", kind="root", node_id="each") + activation = load_or_begin_foreach_activation(frame, "each", mode="serial") + save_foreach_activation(frame, activation) + close_foreach_activation(frame, activation) - assert ForeachBarrierState.from_frame(frame, "each") is None - - -def test_foreach_barrier_state_rejects_malformed_metadata() -> None: - frame = ExecutionFrame( - id="root", - kind="root", - node_id="each", - metadata={"foreach_barriers": {"each": {"next_index": "bad"}}}, - ) - - with pytest.raises(WorkflowExecutionError, match="next_index"): - ForeachBarrierState.from_frame(frame, "each") + assert load_foreach_activation(frame, "each", activation.id) is None + assert load_foreach_activation(frame, "each", "root:each#99") is None def test_foreach_barrier_tracks_active_and_outstanding_children() -> None: @@ -269,39 +211,44 @@ def test_foreach_barrier_rejects_finishing_unknown_child() -> None: barrier.finish_child("child-0") -def test_foreach_barrier_accumulates_multiple_patches_for_one_item() -> None: +def test_foreach_barrier_success_registration_is_idempotent_for_same_lineage() -> None: barrier = ForeachBarrierState(mode="concurrent") - patch = StatePatch(changes={"state.count": 1}) - second_patch = StatePatch(changes={"state.name": "a"}) barrier.add_success_patch( index=0, frame_id="child-0", - patch=patch, - lineage_id="root/each[0]", + lineage_id="root:each#0[0]", ) barrier.add_success_patch( index=0, frame_id="child-0", - patch=second_patch, - lineage_id="root/each[0]", + lineage_id="root:each#0[0]", ) result = barrier.pending_results[0] - assert result.lineage_id == "root/each[0]" - assert result.patch.changes["state.count"] == 1 - assert result.patch.changes["state.name"] == "a" - assert len(result.patch.writes) == 2 + assert result.status == "succeeded" + assert result.lineage_id == "root:each#0[0]" + + +def test_foreach_barrier_rejects_success_lineage_mismatch() -> None: + barrier = ForeachBarrierState(mode="concurrent") + barrier.add_success_patch(index=0, frame_id="child-0", lineage_id="root:each#0[0]") + + with pytest.raises(WorkflowExecutionError, match="belongs to lineage"): + barrier.add_success_patch( + index=0, frame_id="child-0", lineage_id="root:each#0[1]" + ) def test_foreach_barrier_rejects_item_result_frame_mismatch() -> None: barrier = ForeachBarrierState(mode="concurrent") - patch = StatePatch(changes={"state.count": 1}) - barrier.add_success_patch(index=0, frame_id="child-0", patch=patch) + barrier.add_success_patch(index=0, frame_id="child-0", lineage_id="root:each#0[0]") with pytest.raises(WorkflowExecutionError, match="belongs to frame"): - barrier.add_success_patch(index=0, frame_id="child-1", patch=patch) + barrier.add_success_patch( + index=0, frame_id="child-1", lineage_id="root:each#0[0]" + ) def test_item_error_record_rejects_negative_index() -> None: @@ -333,16 +280,13 @@ def test_pending_item_result_rejects_negative_index() -> None: ) -def test_save_to_frame_rejects_corrupt_table_without_mutating() -> None: - frame = ExecutionFrame( - id="root", - kind="root", - node_id="each", - metadata={"foreach_barriers": "corrupt"}, - ) - barrier = ForeachBarrierState(next_index=1) - - with pytest.raises(WorkflowExecutionError, match="barrier table"): - barrier.save_to_frame(frame, "each") - - assert frame.metadata["foreach_barriers"] == "corrupt" +def test_pending_item_result_requires_lineage_for_success() -> None: + with pytest.raises(WorkflowExecutionError, match="lineage"): + PendingItemResult.from_metadata( + { + "index": 0, + "frame_id": "child", + "status": "succeeded", + "lineage_id": None, + } + )