From 6a5d886962b0a6c4fcefc7fd89bd0b74efa12e36 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 07:15:24 +0700 Subject: [PATCH 01/15] feat: analyze foreach control regions --- src/wf_core/analysis/__init__.py | 12 + src/wf_core/analysis/control_regions.py | 255 +++++++++++++++ tests/core/test_foreach_control_regions.py | 347 +++++++++++++++++++++ 3 files changed, 614 insertions(+) create mode 100644 src/wf_core/analysis/control_regions.py create mode 100644 tests/core/test_foreach_control_regions.py diff --git a/src/wf_core/analysis/__init__.py b/src/wf_core/analysis/__init__.py index 44fab703..63815931 100644 --- a/src/wf_core/analysis/__init__.py +++ b/src/wf_core/analysis/__init__.py @@ -5,9 +5,21 @@ from .context_scopes import ( context_analysis_warnings, context_fields_by_node, ) +from .control_regions import ( + ControlRegionAnalysis, + ControlRegionIssue, + ControlRegionIssueKind, + ForeachOwnerStack, + analyze_control_regions, +) __all__ = [ "ContextFieldAvailability", + "ControlRegionAnalysis", + "ControlRegionIssue", + "ControlRegionIssueKind", + "ForeachOwnerStack", + "analyze_control_regions", "context_analysis_warnings", "context_fields_by_node", ] diff --git a/src/wf_core/analysis/control_regions.py b/src/wf_core/analysis/control_regions.py new file mode 100644 index 00000000..81193aad --- /dev/null +++ b/src/wf_core/analysis/control_regions.py @@ -0,0 +1,255 @@ +from __future__ import annotations + +from collections import deque +from dataclasses import dataclass +from enum import StrEnum + +from wf_core.models.steps import EndNode, ForeachNode +from wf_core.models.workflow import Workflow +from wf_core.tokens import END + +type ForeachOwnerStack = tuple[str, ...] + + +class ControlRegionIssueKind(StrEnum): + UNREACHABLE_NODE = "unreachable_node" + FOREACH_REGION_CONFLICT = "foreach_region_conflict" + INVALID_FOREACH_RETURN = "invalid_foreach_return" + INVALID_FOREACH_TERMINAL = "invalid_foreach_terminal" + EMPTY_FOREACH_BODY = "empty_foreach_body" + FOREACH_BODY_NO_RETURN = "foreach_body_no_return" + + +@dataclass(frozen=True, slots=True) +class ControlRegionIssue: + kind: ControlRegionIssueKind + path: str + message: str + + +@dataclass(frozen=True, slots=True) +class ControlRegionAnalysis: + owner_stack_by_node: dict[str, ForeachOwnerStack] + issues: tuple[ControlRegionIssue, ...] + + +def analyze_control_regions(workflow: Workflow) -> ControlRegionAnalysis: + """Derive one static foreach-owner stack per reachable node use. + + Traversal is over ``(node_id, owner_stack)`` states. A ``loop`` edge from + a foreach pushes that controller; an edge targeting the immediate owner is + an item return that resumes the owner in the popped stack; targeting an + older ancestor is a non-local return; targeting ``END``/``EndNode`` inside + a body is an invalid terminal. Reaching the same node under two stacks is + a region conflict. After traversal every unreached node is unreachable and + every reached body state must have a structural path back to its top owner. + """ + nodes_by_id = {node.id: node for node in workflow.nodes} + if workflow.start not in nodes_by_id: + return ControlRegionAnalysis(owner_stack_by_node={}, issues=()) + + edges_by_node: dict[str, list[tuple[int, object]]] = {} + for index, edge in enumerate(workflow.edges): + edges_by_node.setdefault(edge.from_, []).append((index, edge)) + + owner_stack_by_node: dict[str, ForeachOwnerStack] = {} + conflicted: set[str] = set() + issues: list[ControlRegionIssue] = [] + # Semantic state adjacency for the structural-return check. Return edges + # also link to the resumed owner state so deeper nested returns are part + # of the path search. + adjacency: dict[ + tuple[str, ForeachOwnerStack], list[tuple[str, ForeachOwnerStack]] + ] = {} + return_owner_by_source: dict[tuple[str, ForeachOwnerStack], str] = {} + ambiguous_tops: set[str] = set() + + def mark_ambiguous(stack: ForeachOwnerStack) -> None: + if stack: + ambiguous_tops.add(stack[-1]) + + def add_adjacency( + source: tuple[str, ForeachOwnerStack], + target: tuple[str, ForeachOwnerStack], + ) -> None: + adjacency.setdefault(source, []).append(target) + + pending: deque[tuple[str, ForeachOwnerStack]] = deque([(workflow.start, ())]) + visited: set[tuple[str, ForeachOwnerStack]] = set() + visited_nodes: set[str] = set() + + while pending: + node_id, stack = pending.popleft() + state = (node_id, stack) + if state in visited: + continue + visited.add(state) + node = nodes_by_id.get(node_id) + if node is None: + continue + visited_nodes.add(node_id) + + if node_id in conflicted: + continue + recorded = owner_stack_by_node.get(node_id) + if recorded is None: + owner_stack_by_node[node_id] = stack + elif recorded != stack: + # Same node use reached under two control regions: it has no + # single static owner stack. Drop it so later context analysis + # grants no foreach fields, and stop expanding this ambiguous + # state so the conflict does not cascade. + del owner_stack_by_node[node_id] + conflicted.add(node_id) + issues.append( + ControlRegionIssue( + kind=ControlRegionIssueKind.FOREACH_REGION_CONFLICT, + path=f"nodes[{node_id}]", + message=( + f"node {node_id!r} is reachable under two foreach " + "control regions" + ), + ) + ) + for prior_stack in (recorded, stack): + mark_ambiguous(prior_stack) + continue + + for edge_index, edge in edges_by_node.get(node_id, []): # type: ignore[attr-defined] + target_id: str = edge.to # type: ignore[attr-defined] + source_is_loop = isinstance(node, ForeachNode) and edge.outcome == "loop" # type: ignore[attr-defined] + if source_is_loop: + if target_id == node_id: + issues.append( + ControlRegionIssue( + kind=ControlRegionIssueKind.EMPTY_FOREACH_BODY, + path=f"edges[{edge_index}]", + message=( + f"foreach {node_id!r} loop targets itself; " + "an iteration body needs a distinct node use" + ), + ) + ) + mark_ambiguous(stack) + continue + target_stack: ForeachOwnerStack = (*stack, node_id) + else: + target_stack = stack + + target_node = None if target_id == END else nodes_by_id.get(target_id) + if target_id != END and target_node is None: + # Unknown destinations are owned by ordinary edge validation. + continue + is_terminal = target_id == END or isinstance(target_node, EndNode) + if is_terminal: + if target_stack: + issues.append( + ControlRegionIssue( + kind=ControlRegionIssueKind.INVALID_FOREACH_TERMINAL, + path=f"edges[{edge_index}]", + message=( + f"foreach item path {node_id!r} -> " + f"{target_id!r} targets a workflow terminal " + "from inside a foreach body" + ), + ) + ) + mark_ambiguous(target_stack) + continue + + # At this point target_id is a known non-terminal node id. + if target_stack and target_id == target_stack[-1]: + # Immediate-owner back-edge: the item frame completes at its + # owner without executing the controller again. Resume the + # owner in the popped stack for structural analysis. + resumed: tuple[str, ForeachOwnerStack] = ( + target_id, + target_stack[:-1], + ) + return_owner_by_source[state] = target_id + add_adjacency(state, resumed) + if resumed not in visited: + pending.append(resumed) + continue + if target_id in target_stack: + issues.append( + ControlRegionIssue( + kind=ControlRegionIssueKind.INVALID_FOREACH_RETURN, + path=f"edges[{edge_index}]", + message=( + f"edge {node_id!r} -> {target_id!r} skips the " + "immediate foreach owner" + ), + ) + ) + mark_ambiguous(target_stack) + continue + successor: tuple[str, ForeachOwnerStack] = (target_id, target_stack) + add_adjacency(state, successor) + pending.append(successor) + + for node in workflow.nodes: + if node.id not in visited_nodes: + issues.append( + ControlRegionIssue( + kind=ControlRegionIssueKind.UNREACHABLE_NODE, + path=f"nodes[{node.id}]", + message=f"node {node.id!r} is unreachable from start", + ) + ) + + # Structural returnability: every reached body state needs some graph path + # back to its immediate owner. Data decides whether the exit is taken, so + # one possible path is enough. Skip bodies already made ambiguous by a + # region conflict, invalid return/terminal, or empty body. + tops_with_no_return: set[str] = set() + for node_id, stack in list(visited): + if not stack: + continue + if node_id in conflicted: + continue + top = stack[-1] + if top in ambiguous_tops: + continue + if top in tops_with_no_return: + continue + # Breadth-first search over semantic states for a return to `top`. + seen: set[tuple[str, ForeachOwnerStack]] = set() + queue: deque[tuple[str, ForeachOwnerStack]] = deque([(node_id, stack)]) + found = False + while queue: + current = queue.popleft() + if current in seen: + continue + seen.add(current) + if return_owner_by_source.get(current) == top: + found = True + break + for successor in adjacency.get(current, []): + if successor not in seen: + queue.append(successor) + if not found: + tops_with_no_return.add(top) + + for top in sorted(tops_with_no_return): + # Only report when the owner itself is unambiguous; a conflicted + # owner has no single region to return to. + if top in conflicted: + continue + if top not in owner_stack_by_node and top not in visited_nodes: + continue + issues.append( + ControlRegionIssue( + kind=ControlRegionIssueKind.FOREACH_BODY_NO_RETURN, + path=f"nodes[{top}]", + message=( + f"foreach {top!r} body has no structural path back to " + "its immediate owner" + ), + ) + ) + + return ControlRegionAnalysis( + owner_stack_by_node=dict(owner_stack_by_node), + issues=tuple(issues), + ) diff --git a/tests/core/test_foreach_control_regions.py b/tests/core/test_foreach_control_regions.py new file mode 100644 index 00000000..b3844f2f --- /dev/null +++ b/tests/core/test_foreach_control_regions.py @@ -0,0 +1,347 @@ +from __future__ import annotations + +from wf_core import END, Workflow +from wf_core.analysis.control_regions import ( + ControlRegionAnalysis, + ControlRegionIssueKind, + analyze_control_regions, +) + + +def _workflow( + *, + start: str, + nodes: list[dict[str, object]], + edges: list[dict[str, str]], +) -> Workflow: + return Workflow.model_validate( + { + "name": "control-regions", + "input_schema": {"type": "object", "properties": {}}, + "state_schema": { + "type": "object", + "properties": { + "items": {"type": "array", "items": {"type": "string"}}, + "inner_items": {"type": "array", "items": {"type": "integer"}}, + }, + }, + "output_schema": {"type": "object", "properties": {}}, + "start": start, + "nodes": nodes, + "edges": edges, + "node_defs": [], + } + ) + + +def _node(node_id: str) -> dict[str, object]: + return {"id": node_id, "type": "node", "node": "noop"} + + +def _foreach(node_id: str, *, alias: str = "item") -> dict[str, object]: + return { + "id": node_id, + "type": "foreach", + "over": "state.items", + "as": alias, + "mode": "serial", + } + + +def _condition(node_id: str) -> dict[str, object]: + return { + "id": node_id, + "type": "condition", + "check": {"op": "exists", "path": "state.items"}, + } + + +def test_closed_root_cycle_has_one_empty_control_region() -> None: + workflow = _workflow( + start="a", + nodes=[_node("a"), _node("b")], + edges=[ + {"from": "a", "outcome": "ok", "to": "b"}, + {"from": "b", "outcome": "ok", "to": "a"}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert analysis.issues == () + assert analysis.owner_stack_by_node == {"a": (), "b": ()} + + +def test_foreach_cycle_with_possible_return_is_valid() -> None: + workflow = _workflow( + start="f", + nodes=[_foreach("f"), _node("a")], + edges=[ + {"from": "f", "outcome": "loop", "to": "a"}, + {"from": "a", "outcome": "again", "to": "a"}, + {"from": "a", "outcome": "done", "to": "f"}, + {"from": "f", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert analysis.issues == () + assert analysis.owner_stack_by_node["a"] == ("f",) + assert analysis.owner_stack_by_node["f"] == () + + +def test_conditional_foreach_paths_can_both_return() -> None: + workflow = _workflow( + start="f", + nodes=[_foreach("f"), _condition("condition"), _node("work")], + edges=[ + {"from": "f", "outcome": "loop", "to": "condition"}, + {"from": "condition", "outcome": "true", "to": "work"}, + {"from": "condition", "outcome": "false", "to": "f"}, + {"from": "work", "outcome": "ok", "to": "f"}, + {"from": "f", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert analysis.issues == () + assert analysis.owner_stack_by_node["condition"] == ("f",) + assert analysis.owner_stack_by_node["work"] == ("f",) + + +def test_nested_foreach_assigns_static_owner_stacks() -> None: + workflow = _workflow( + start="f1", + nodes=[ + _foreach("f1"), + _foreach("f2"), + _node("work"), + _node("tail"), + _node("after"), + ], + edges=[ + {"from": "f1", "outcome": "loop", "to": "f2"}, + {"from": "f2", "outcome": "loop", "to": "work"}, + {"from": "work", "outcome": "ok", "to": "f2"}, + {"from": "f2", "outcome": "done", "to": "tail"}, + {"from": "tail", "outcome": "ok", "to": "f1"}, + {"from": "f1", "outcome": "done", "to": "after"}, + {"from": "after", "outcome": "ok", "to": END}, + ], + ) + + analysis: ControlRegionAnalysis = analyze_control_regions(workflow) + + assert analysis.owner_stack_by_node == { + "f1": (), + "f2": ("f1",), + "work": ("f1", "f2"), + "tail": ("f1",), + "after": (), + } + assert analysis.issues == () + + +def test_reentering_completed_foreach_keeps_one_static_region() -> None: + workflow = _workflow( + start="again", + nodes=[_condition("again"), _foreach("f"), _node("work")], + edges=[ + {"from": "again", "outcome": "true", "to": "f"}, + {"from": "f", "outcome": "loop", "to": "work"}, + {"from": "work", "outcome": "ok", "to": "f"}, + {"from": "f", "outcome": "done", "to": "again"}, + {"from": "again", "outcome": "false", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert analysis.issues == () + assert analysis.owner_stack_by_node["f"] == () + assert analysis.owner_stack_by_node["work"] == ("f",) + assert analysis.owner_stack_by_node["again"] == () + + +def test_external_entry_into_foreach_body_is_region_conflict() -> None: + workflow = _workflow( + start="start", + nodes=[_condition("start"), _foreach("f"), _node("b")], + edges=[ + {"from": "start", "outcome": "true", "to": "f"}, + {"from": "start", "outcome": "false", "to": "b"}, + {"from": "f", "outcome": "loop", "to": "b"}, + {"from": "b", "outcome": "ok", "to": "f"}, + {"from": "f", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert (ControlRegionIssueKind.FOREACH_REGION_CONFLICT, "nodes[b]") in [ + (issue.kind, issue.path) for issue in analysis.issues + ] + assert "b" not in analysis.owner_stack_by_node + + +def test_foreach_body_escape_is_region_conflict() -> None: + workflow = _workflow( + start="f", + nodes=[_foreach("f"), _node("b"), _node("after")], + edges=[ + {"from": "f", "outcome": "loop", "to": "b"}, + {"from": "b", "outcome": "ok", "to": "after"}, + {"from": "f", "outcome": "done", "to": "after"}, + {"from": "after", "outcome": "ok", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert (ControlRegionIssueKind.FOREACH_REGION_CONFLICT, "nodes[after]") in [ + (issue.kind, issue.path) for issue in analysis.issues + ] + assert "after" not in analysis.owner_stack_by_node + + +def test_skipping_inner_foreach_owner_is_invalid_return() -> None: + workflow = _workflow( + start="f1", + nodes=[_foreach("f1"), _foreach("f2"), _node("work")], + edges=[ + {"from": "f1", "outcome": "loop", "to": "f2"}, + {"from": "f2", "outcome": "loop", "to": "work"}, + {"from": "work", "outcome": "ok", "to": "f1"}, + {"from": "f1", "outcome": "done", "to": END}, + {"from": "f2", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert (ControlRegionIssueKind.INVALID_FOREACH_RETURN, "edges[2]") in [ + (issue.kind, issue.path) for issue in analysis.issues + ] + + +def test_entering_sibling_foreach_body_is_region_conflict() -> None: + workflow = _workflow( + start="f1", + nodes=[_foreach("f1"), _foreach("f2"), _node("b1"), _node("b2")], + edges=[ + {"from": "f1", "outcome": "loop", "to": "b1"}, + {"from": "b1", "outcome": "ok", "to": "b2"}, + {"from": "f2", "outcome": "loop", "to": "b2"}, + {"from": "b2", "outcome": "ok", "to": "f1"}, + {"from": "f1", "outcome": "done", "to": "f2"}, + {"from": "f2", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert (ControlRegionIssueKind.FOREACH_REGION_CONFLICT, "nodes[b2]") in [ + (issue.kind, issue.path) for issue in analysis.issues + ] + + +def test_empty_foreach_body_is_rejected() -> None: + workflow = _workflow( + start="f", + nodes=[_foreach("f")], + edges=[ + {"from": "f", "outcome": "loop", "to": "f"}, + {"from": "f", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert (ControlRegionIssueKind.EMPTY_FOREACH_BODY, "edges[0]") in [ + (issue.kind, issue.path) for issue in analysis.issues + ] + + +def test_closed_foreach_body_cycle_has_no_return() -> None: + workflow = _workflow( + start="f", + nodes=[_foreach("f"), _node("a"), _node("b")], + edges=[ + {"from": "f", "outcome": "loop", "to": "a"}, + {"from": "a", "outcome": "ok", "to": "b"}, + {"from": "b", "outcome": "ok", "to": "a"}, + {"from": "f", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert (ControlRegionIssueKind.FOREACH_BODY_NO_RETURN, "nodes[f]") in [ + (issue.kind, issue.path) for issue in analysis.issues + ] + + +def test_foreach_body_cannot_target_end_token() -> None: + workflow = _workflow( + start="f", + nodes=[_foreach("f"), _node("body")], + edges=[ + {"from": "f", "outcome": "loop", "to": "body"}, + {"from": "body", "outcome": "ok", "to": END}, + {"from": "f", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert (ControlRegionIssueKind.INVALID_FOREACH_TERMINAL, "edges[1]") in [ + (issue.kind, issue.path) for issue in analysis.issues + ] + + +def test_foreach_body_cannot_target_explicit_end_node() -> None: + workflow = _workflow( + start="f", + nodes=[ + _foreach("f"), + _node("body"), + {"id": "stop", "type": "end", "outcome": "ok"}, + ], + edges=[ + {"from": "f", "outcome": "loop", "to": "body"}, + {"from": "body", "outcome": "ok", "to": "stop"}, + {"from": "f", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert (ControlRegionIssueKind.INVALID_FOREACH_TERMINAL, "edges[1]") in [ + (issue.kind, issue.path) for issue in analysis.issues + ] + + +def test_every_unreachable_node_is_reported() -> None: + workflow = _workflow( + start="work", + nodes=[_node("work"), _node("detached_a"), _node("detached_b")], + edges=[ + {"from": "work", "outcome": "ok", "to": END}, + {"from": "detached_a", "outcome": "ok", "to": "detached_b"}, + {"from": "detached_b", "outcome": "ok", "to": "detached_a"}, + ], + ) + + analysis = analyze_control_regions(workflow) + by_kind_path = [(issue.kind, issue.path) for issue in analysis.issues] + + assert ( + ControlRegionIssueKind.UNREACHABLE_NODE, + "nodes[detached_a]", + ) in by_kind_path + assert ( + ControlRegionIssueKind.UNREACHABLE_NODE, + "nodes[detached_b]", + ) in by_kind_path From 4bbd9f96505ef1fa13ff47854d7ac05a80657ec9 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 07:18:03 +0700 Subject: [PATCH 02/15] refactor: derive context from foreach control regions --- src/wf_core/analysis/context_scopes.py | 169 +++++++++++-------------- tests/core/test_context_scopes.py | 34 +++-- 2 files changed, 95 insertions(+), 108 deletions(-) diff --git a/src/wf_core/analysis/context_scopes.py b/src/wf_core/analysis/context_scopes.py index 84d114e8..87bc8529 100644 --- a/src/wf_core/analysis/context_scopes.py +++ b/src/wf_core/analysis/context_scopes.py @@ -1,11 +1,14 @@ from __future__ import annotations -from collections import deque from collections.abc import Mapping from copy import deepcopy from dataclasses import dataclass from typing import Literal +from wf_core.analysis.control_regions import ( + ForeachOwnerStack, + analyze_control_regions, +) from wf_core.context_contracts import ( STANDARD_CONTEXT_FIELDS, ContextFieldContract, @@ -80,6 +83,13 @@ def context_analysis_warnings(workflow: Workflow) -> tuple[str, ...]: def _analyze(workflow: Workflow) -> _ContextAnalysis: + """Derive context contracts from static foreach control regions. + + Each unambiguous node use has exactly one owner stack; its active foreach + is the final stack item. A canonical return edge pops the stack, so the + controller itself stays in the outer context. Conflicted nodes receive no + foreach fields. + """ nodes = {node.id: node for node in workflow.nodes} foreach_nodes = { node.id: node for node in workflow.nodes if isinstance(node, ForeachNode) @@ -107,35 +117,20 @@ def _analyze(workflow: Workflow) -> _ContextAnalysis: warnings.add(f"workflow start targets missing node {workflow.start!r}") return _ContextAnalysis({}, tuple(warnings.values)) - scopes_by_node: dict[str, set[FrameScope]] = {} - pending: deque[tuple[str, FrameScope]] = deque([(workflow.start, None)]) - visited: set[tuple[str, FrameScope]] = set() - while pending: - node_id, active_scope = pending.popleft() - state = (node_id, active_scope) - if state in visited: - continue - visited.add(state) - node = nodes.get(node_id) - if node is None: - continue - scopes_by_node.setdefault(node_id, set()).add(active_scope) - - for edge in edges_by_node.get(node_id, []): - if edge.to == END or edge.to not in nodes: - continue - next_scope = active_scope - if isinstance(node, ForeachNode) and edge.outcome == "loop": - next_scope = node.id - pending.append((edge.to, next_scope)) + analysis = analyze_control_regions(workflow) + for issue in analysis.issues: + warnings.add( + f"control region {issue.kind.value} at {issue.path}: {issue.message}" + ) fields_by_node: dict[str, tuple[ContextFieldAvailability, ...]] = {} - for node_id, scopes in scopes_by_node.items(): + for node_id, stack in analysis.owner_stack_by_node.items(): + active_foreach_id = stack[-1] if stack else None fields_by_node[node_id] = _available_fields( workflow, foreach_nodes, - scopes, - scopes_by_node, + analysis.owner_stack_by_node, + active_foreach_id, ) return _ContextAnalysis(fields_by_node, tuple(warnings.values)) @@ -143,83 +138,66 @@ def _analyze(workflow: Workflow) -> _ContextAnalysis: def _available_fields( workflow: Workflow, foreach_nodes: Mapping[str, ForeachNode], - scopes: set[FrameScope], - scopes_by_node: Mapping[str, set[FrameScope]], + owner_stack_by_node: Mapping[str, ForeachOwnerStack], + active_scope: FrameScope, ) -> tuple[ContextFieldAvailability, ...]: - fields_by_name: dict[str, ContextFieldContract] = {} - scopes_by_field: dict[str, set[FrameScope]] = {} - for scope in sorted(scopes, key=lambda value: value or ""): - contracts = STANDARD_CONTEXT_FIELDS - if scope is not None: - foreach = foreach_nodes.get(scope) - if foreach is not None: - contracts = ( - *contracts, - *foreach_context_fields( - foreach.as_, - _foreach_item_schema( - workflow, - foreach, - scopes_by_node.get(foreach.id, {None}), - foreach_nodes, - scopes_by_node, - ), + """Return contracts for one static owner stack; all are guaranteed. + + A single node use has one control region, so foreach fields are either + present (inside a body) or absent (outside). Conditional availability is + not used to represent multiple owner stacks. + """ + contracts = list(STANDARD_CONTEXT_FIELDS) + if active_scope is not None: + foreach = foreach_nodes.get(active_scope) + if foreach is not None: + contracts.extend( + foreach_context_fields( + foreach.as_, + _foreach_item_schema( + workflow, + foreach, + foreach_nodes, + owner_stack_by_node, ), ) - for contract in contracts: - fields_by_name.setdefault( + ) + return tuple( + ContextFieldAvailability( + contract=ContextFieldContract( contract.name, - ContextFieldContract( - contract.name, - deepcopy(contract.schema), - contract.description, - ), - ) - scopes_by_field.setdefault(contract.name, set()).add(scope) - - field_count = len(scopes) - result: list[ContextFieldAvailability] = [] - for contract in fields_by_name.values(): - field_scopes = scopes_by_field[contract.name] - availability: ContextAvailability = ( - "available" if len(field_scopes) == field_count else "conditional" + deepcopy(contract.schema), + contract.description, + ), + availability="available", ) - reason = None - if availability == "conditional": - reason = "Available only in some reachable execution frames." - result.append( - ContextFieldAvailability( - contract=contract, - availability=availability, - reason=reason, - ) - ) - return tuple(result) + for contract in contracts + ) def _foreach_item_schema( workflow: Workflow, foreach: ForeachNode, - source_scopes: set[FrameScope], foreach_nodes: Mapping[str, ForeachNode], - scopes_by_node: Mapping[str, set[FrameScope]], + owner_stack_by_node: Mapping[str, ForeachOwnerStack], ) -> ContextSchema: - source_schemas = [ - _schema_at_path( - workflow, - foreach.over.root, - foreach.over.parts, - source_scope, - foreach_nodes, - scopes_by_node, - ) - for source_scope in sorted(source_scopes, key=lambda value: value or "") - ] - if not source_schemas or any( - schema != source_schemas[0] for schema in source_schemas - ): + """Resolve one controller's item schema in its own static context. + + An inner foreach may declare ``over="context.outer_item"``; the lookup + uses the controller's own owner stack, not the inner body stack. + """ + controller_stack = owner_stack_by_node.get(foreach.id) + if controller_stack is None: return {} - source_schema = source_schemas[0] + controller_scope: FrameScope = controller_stack[-1] if controller_stack else None + source_schema = _schema_at_path( + workflow, + foreach.over.root, + foreach.over.parts, + controller_scope, + foreach_nodes, + owner_stack_by_node, + ) if not isinstance(source_schema, Mapping): return {} source_type = source_schema.get("type") @@ -235,7 +213,7 @@ def _foreach_item_schema( workflow, foreach.over.root, foreach_nodes=foreach_nodes, - scopes_by_node=scopes_by_node, + owner_stack_by_node=owner_stack_by_node, ), items, ) @@ -250,7 +228,7 @@ def _schema_at_path( parts: tuple[str, ...], active_scope: FrameScope, foreach_nodes: Mapping[str, ForeachNode], - scopes_by_node: Mapping[str, set[FrameScope]], + owner_stack_by_node: Mapping[str, ForeachOwnerStack], ) -> Mapping[str, object] | None: try: schema_document = _schema_document( @@ -258,7 +236,7 @@ def _schema_at_path( root, active_scope=active_scope, foreach_nodes=foreach_nodes, - scopes_by_node=scopes_by_node, + owner_stack_by_node=owner_stack_by_node, ) current: object = schema_document for part in parts: @@ -282,7 +260,7 @@ def _schema_document( *, active_scope: FrameScope = None, foreach_nodes: Mapping[str, ForeachNode] | None = None, - scopes_by_node: Mapping[str, set[FrameScope]] | None = None, + owner_stack_by_node: Mapping[str, ForeachOwnerStack] | None = None, ) -> Mapping[str, object]: if root == "input": return workflow.input_schema.model_dump(mode="json", exclude_none=True) @@ -295,7 +273,7 @@ def _schema_document( if ( active_scope is not None and foreach_nodes is not None - and scopes_by_node is not None + and owner_stack_by_node is not None ): foreach = foreach_nodes.get(active_scope) if foreach is not None: @@ -307,9 +285,8 @@ def _schema_document( _foreach_item_schema( workflow, foreach, - scopes_by_node.get(foreach.id, {None}), foreach_nodes, - scopes_by_node, + owner_stack_by_node, ), ) } diff --git a/tests/core/test_context_scopes.py b/tests/core/test_context_scopes.py index 645dc286..5eaa9c28 100644 --- a/tests/core/test_context_scopes.py +++ b/tests/core/test_context_scopes.py @@ -146,7 +146,7 @@ def test_serial_and_concurrent_foreach_expose_the_same_scoped_context() -> None: edges=[ {"from": "each", "outcome": "loop", "to": "body"}, {"from": "each", "outcome": "done", "to": "tail"}, - {"from": "body", "outcome": "ok", "to": END}, + {"from": "body", "outcome": "ok", "to": "each"}, {"from": "tail", "outcome": "ok", "to": END}, ], ) @@ -164,7 +164,7 @@ def test_foreach_item_schema_and_configured_alias_are_reported() -> None: nodes=[_foreach("each", alias="record"), _node("body")], edges=[ {"from": "each", "outcome": "loop", "to": "body"}, - {"from": "body", "outcome": "ok", "to": END}, + {"from": "body", "outcome": "ok", "to": "each"}, {"from": "each", "outcome": "done", "to": END}, ], ) @@ -181,7 +181,7 @@ def test_foreach_item_schema_resolves_bounded_local_array_reference() -> None: nodes=[_foreach("each", alias="record"), _node("body")], edges=[ {"from": "each", "outcome": "loop", "to": "body"}, - {"from": "body", "outcome": "ok", "to": END}, + {"from": "body", "outcome": "ok", "to": "each"}, {"from": "each", "outcome": "done", "to": END}, ], state_schema={ @@ -209,7 +209,7 @@ def test_foreach_item_schema_resolves_bounded_local_array_reference() -> None: assert fields["record"].contract.schema["properties"] == {"id": {"type": "string"}} -def test_only_foreach_reachable_node_has_available_context() -> None: +def test_region_conflicted_node_receives_no_guaranteed_foreach_fields() -> None: workflow = _workflow( start="start", nodes=[_node("start"), _foreach("each", alias="item"), _node("body")], @@ -218,12 +218,18 @@ def test_only_foreach_reachable_node_has_available_context() -> None: {"from": "start", "outcome": "loop", "to": "each"}, {"from": "each", "outcome": "loop", "to": "body"}, {"from": "each", "outcome": "done", "to": END}, - {"from": "body", "outcome": "ok", "to": END}, + {"from": "body", "outcome": "ok", "to": "each"}, ], ) - assert _field_map(workflow, "body")["item"].availability == "conditional" - assert _field_map(workflow, "body")["item"].reason + fields = context_fields_by_node(workflow) + assert "body" not in fields or "item" not in { + field.contract.name for field in fields.get("body", ()) + } + warnings = context_analysis_warnings(workflow) + assert any( + "control region" in warning or "conflict" in warning for warning in warnings + ) def test_nested_foreach_replaces_inner_scope_and_restores_outer_scope() -> None: @@ -239,8 +245,8 @@ def test_nested_foreach_replaces_inner_scope_and_restores_outer_scope() -> None: {"from": "outer", "outcome": "loop", "to": "inner"}, {"from": "inner", "outcome": "loop", "to": "inner_body"}, {"from": "inner", "outcome": "done", "to": "after_inner"}, - {"from": "inner_body", "outcome": "ok", "to": END}, - {"from": "after_inner", "outcome": "ok", "to": END}, + {"from": "inner_body", "outcome": "ok", "to": "inner"}, + {"from": "after_inner", "outcome": "ok", "to": "outer"}, {"from": "outer", "outcome": "done", "to": END}, ], state_schema={ @@ -267,12 +273,14 @@ def test_nested_foreach_preserves_context_backed_item_schema() -> None: _foreach("outer", alias="outer_item"), _foreach("inner", alias="inner_item", over="context.outer_item"), _node("inner_body"), + _node("after_inner"), ], edges=[ {"from": "outer", "outcome": "loop", "to": "inner"}, {"from": "inner", "outcome": "loop", "to": "inner_body"}, - {"from": "inner", "outcome": "done", "to": END}, - {"from": "inner_body", "outcome": "ok", "to": END}, + {"from": "inner", "outcome": "done", "to": "after_inner"}, + {"from": "inner_body", "outcome": "ok", "to": "inner"}, + {"from": "after_inner", "outcome": "ok", "to": "outer"}, {"from": "outer", "outcome": "done", "to": END}, ], state_schema={ @@ -350,4 +358,6 @@ def test_scoped_cycle_terminates_and_preserves_scoped_field_availability() -> No fields = context_fields_by_node(workflow) assert fields["body"] assert _field_map(workflow, "body")["item"].availability == "available" - assert _field_map(workflow, "each")["item"].availability == "conditional" + # A canonical back-edge pops the item stack, so the controller itself + # stays in the outer region and exposes no item alias. + assert "item" not in _field_map(workflow, "each") From f69c4502cf981766b7f044721230803a8e33cf35 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 07:25:55 +0700 Subject: [PATCH 03/15] feat: identify dynamic foreach activations --- src/wf_core/runtime/foreach_state.py | 205 +++++++++++++++++- src/wf_core/runtime/lineage.py | 17 +- src/wf_core/runtime/ops/foreach.py | 44 ++-- src/wf_core/runtime/ops/nodes.py | 22 +- src/wf_core/runtime/scheduler.py | 41 +++- src/wf_core/runtime/step.py | 24 +- tests/core/test_concurrent_foreach.py | 50 +++-- tests/core/test_concurrent_foreach_errors.py | 4 +- .../test_concurrent_foreach_interrupts.py | 6 +- tests/core/test_foreach_activations.py | 102 +++++++++ tests/core/test_foreach_barrier_state.py | 38 ++-- tests/core/test_scheduler.py | 22 ++ 12 files changed, 499 insertions(+), 76 deletions(-) create mode 100644 tests/core/test_foreach_activations.py diff --git a/src/wf_core/runtime/foreach_state.py b/src/wf_core/runtime/foreach_state.py index 1da9065f..6aa8236a 100644 --- a/src/wf_core/runtime/foreach_state.py +++ b/src/wf_core/runtime/foreach_state.py @@ -11,6 +11,31 @@ from wf_core.runtime.ops.state import StatePatch from wf_core.runtime.scheduler import ForeachIterationMetadata _BARRIER_METADATA_KEY = "foreach_barriers" +_ACTIVATION_METADATA_KEY = "foreach_activations" + + +@dataclass(slots=True) +class ForeachActivationState: + """Persisted state for one dynamic visit to a foreach controller. + + A parent frame creates a fresh activation on first entry, reuses it while + admitting items, and closes it before emitting ``done``. The id is opaque: + callers compare it by name and never parse it. + """ + + id: str + foreach_node_id: str + barrier: ForeachBarrierState + + +@dataclass(frozen=True, slots=True) +class ForeachItemOwner: + """Named ownership record for one foreach item frame.""" + + parent_frame_id: str + foreach_node_id: str + activation_id: str + item_index: int @dataclass(slots=True) @@ -320,14 +345,188 @@ class ForeachBarrierState: ) -def item_frame_owner(frame: ExecutionFrame) -> tuple[str, str, int] | None: - """Return parent frame id, foreach node id, and item index for item frames.""" +def load_or_begin_foreach_activation( + frame: ExecutionFrame, + foreach_node_id: str, + *, + mode: Literal["serial", "concurrent"], +) -> ForeachActivationState: + """Load the active activation or begin a fresh visit. + + The first entry for one visit allocates an opaque id from the parent frame + id, foreach node id, and a persisted per-frame sequence. Later calls reuse + the active activation; closing it makes the next visit allocate a new id + with fresh barrier state. Mode mismatches and malformed tables fail fast. + """ + table = _activation_table(frame) + entry = table.get(foreach_node_id) + if entry is None: + entry = {"next_sequence": 0, "active": None} + table[foreach_node_id] = entry + if not isinstance(entry, dict): + raise WorkflowExecutionError( + f"malformed foreach activation entry for frame {frame.id!r}" + ) + next_sequence = entry.get("next_sequence", 0) + if not isinstance(next_sequence, int) or next_sequence < 0: + raise WorkflowExecutionError( + f"malformed foreach activation sequence for frame {frame.id!r}" + ) + active = entry.get("active") + if active is not None: + activation = _activation_from_metadata( + active, frame_id=frame.id, foreach_node_id=foreach_node_id + ) + if activation.barrier.mode != mode: + raise WorkflowExecutionError( + f"foreach {foreach_node_id!r} activation {activation.id!r} " + f"has mode {activation.barrier.mode!r}, got {mode!r}" + ) + return activation + activation_id = f"{frame.id}:{foreach_node_id}#{next_sequence}" + activation = ForeachActivationState( + id=activation_id, + foreach_node_id=foreach_node_id, + barrier=ForeachBarrierState(mode=mode), + ) + entry["next_sequence"] = next_sequence + 1 + entry["active"] = { + "id": activation.id, + "barrier": activation.barrier.to_metadata(), + } + return activation + + +def save_foreach_activation( + frame: ExecutionFrame, activation: ForeachActivationState +) -> None: + """Persist barrier progress for the named active activation.""" + table = _activation_table(frame) + entry = table.get(activation.foreach_node_id) + if not isinstance(entry, dict): + raise WorkflowExecutionError( + f"malformed foreach activation entry for frame {frame.id!r}" + ) + active = entry.get("active") + if not isinstance(active, dict) or active.get("id") != activation.id: + raise WorkflowExecutionError( + f"cannot save stale foreach activation {activation.id!r} " + f"for frame {frame.id!r}" + ) + active["barrier"] = activation.barrier.to_metadata() + + +def close_foreach_activation( + frame: ExecutionFrame, activation: ForeachActivationState +) -> None: + """Close the named active activation, preserving the visit sequence. + + The barrier is removed so a later visit starts fresh; the sequence keeps + increasing so child and lineage ids cannot collide across visits. + """ + table = _activation_table(frame) + entry = table.get(activation.foreach_node_id) + if not isinstance(entry, dict): + raise WorkflowExecutionError( + f"malformed foreach activation entry for frame {frame.id!r}" + ) + active = entry.get("active") + if not isinstance(active, dict) or active.get("id") != activation.id: + raise WorkflowExecutionError( + f"cannot close stale foreach activation {activation.id!r} " + f"for frame {frame.id!r}" + ) + entry["active"] = None + + +def load_foreach_activation( + frame: ExecutionFrame, foreach_node_id: str, activation_id: str +) -> ForeachActivationState | None: + """Return the active activation only when its id matches the child. + + A child result naming a closed or different activation must fail closed in + the caller rather than buffering into the wrong barrier. + """ + table = _activation_table(frame) + entry = table.get(foreach_node_id) + if not isinstance(entry, dict): + raise WorkflowExecutionError( + f"malformed foreach activation entry for frame {frame.id!r}" + ) + active = entry.get("active") + if active is None: + return None + activation = _activation_from_metadata( + active, frame_id=frame.id, foreach_node_id=foreach_node_id + ) + if activation.id != activation_id: + return None + return activation + + +def require_foreach_activation( + frame: ExecutionFrame, foreach_node_id: str, activation_id: str +) -> ForeachActivationState: + """Load the named activation or raise when it is closed or superseded.""" + activation = load_foreach_activation(frame, foreach_node_id, activation_id) + if activation is None: + raise WorkflowExecutionError( + f"foreach item activation {activation_id!r} for node " + f"{foreach_node_id!r} is closed or superseded" + ) + return activation + + +def item_frame_owner(frame: ExecutionFrame) -> ForeachItemOwner | None: + """Return the named foreach ownership record for item frames. + + Malformed item metadata fails closed via ``ForeachIterationMetadata``; + only non-item frames return ``None``. + """ if frame.kind != "foreach_iteration" or frame.parent_frame_id is None: return None metadata = ForeachIterationMetadata.from_frame(frame) if metadata is None: return None - return frame.parent_frame_id, metadata.foreach_node_id, metadata.loop_index + return ForeachItemOwner( + parent_frame_id=frame.parent_frame_id, + foreach_node_id=metadata.foreach_node_id, + activation_id=metadata.activation_id, + item_index=metadata.loop_index, + ) + + +def _activation_table(frame: ExecutionFrame) -> dict[str, Any]: + raw = frame.metadata.get(_ACTIVATION_METADATA_KEY) + if raw is None: + table: dict[str, Any] = {} + frame.metadata[_ACTIVATION_METADATA_KEY] = table + return table + if not isinstance(raw, dict): + raise WorkflowExecutionError( + f"malformed foreach activation table for frame {frame.id!r}" + ) + return raw + + +def _activation_from_metadata( + raw: object, *, frame_id: str, foreach_node_id: str +) -> ForeachActivationState: + if not isinstance(raw, dict): + raise WorkflowExecutionError( + f"malformed foreach activation for frame {frame_id!r}" + ) + activation_id = raw.get("id") + barrier_raw = raw.get("barrier") + if not isinstance(activation_id, str) or not activation_id: + raise WorkflowExecutionError( + f"malformed foreach activation id for frame {frame_id!r}" + ) + return ForeachActivationState( + id=activation_id, + foreach_node_id=foreach_node_id, + barrier=ForeachBarrierState.from_metadata(barrier_raw), + ) def _string_tuple(raw: object) -> tuple[str, ...]: diff --git a/src/wf_core/runtime/lineage.py b/src/wf_core/runtime/lineage.py index d2d3d97e..800d82a9 100644 --- a/src/wf_core/runtime/lineage.py +++ b/src/wf_core/runtime/lineage.py @@ -7,7 +7,7 @@ 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 ForeachBarrierState, item_frame_owner +from wf_core.runtime.foreach_state import item_frame_owner, load_foreach_activation from wf_core.runtime.ops.state import ( StatePatch, commit_state_patch, @@ -61,21 +61,24 @@ def lineage_writes_for_frame( # 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_id, foreach_node_id, item_index = owner - parent_frame = run.frames.get(parent_frame_id) + 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"{parent_frame_id!r} for child frame {frame.id!r}" + f"{owner.parent_frame_id!r} for child frame {frame.id!r}" ) - barrier = ForeachBarrierState.from_frame(parent_frame, foreach_node_id) - if barrier is None or barrier.mode != "concurrent": + activation = load_foreach_activation( + parent_frame, owner.foreach_node_id, owner.activation_id + ) + if activation is None or activation.barrier.mode != "concurrent": return () - pending = barrier.pending_results.get(item_index) + pending = activation.barrier.pending_results.get(owner.item_index) if pending is None: return () return pending.patch.writes diff --git a/src/wf_core/runtime/ops/foreach.py b/src/wf_core/runtime/ops/foreach.py index db38d71c..b45b15ef 100644 --- a/src/wf_core/runtime/ops/foreach.py +++ b/src/wf_core/runtime/ops/foreach.py @@ -8,9 +8,12 @@ from wf_core.models.steps import ForeachNode from wf_core.models.workflow import Workflow from wf_core.run_state import ExecutionFrame, FrameStatus, RunState, StepExecutionResult from wf_core.runtime.foreach_state import ( + ForeachActivationState, ForeachBarrierState, ItemErrorRecord, PendingItemResult, + load_or_begin_foreach_activation, + save_foreach_activation, ) from wf_core.runtime.lineage import ( add_lineage, @@ -63,7 +66,8 @@ def _step_foreach_serial( raise WorkflowExecutionError("serial foreach helper received non-serial mode") frame = run.current_frame() - barrier = ForeachBarrierState.from_frame(frame, step.id) or ForeachBarrierState() + activation = load_or_begin_foreach_activation(frame, step.id, mode="serial") + barrier = activation.barrier iterable = _resolve_foreach_iterable(run, frame, step) loop_index = barrier.next_index @@ -89,9 +93,9 @@ def _step_foreach_serial( loop_start = index.next_node_id(frame.node_id, "loop") item = iterable[loop_index] barrier.next_index = loop_index + 1 - barrier.save_to_frame(frame, step.id) - child_id = f"{frame.id}:{step.id}:{loop_index}" - child_lineage_id = _child_lineage_id(frame, step, loop_index) + save_foreach_activation(frame, activation) + child_id = _child_frame_id(activation, loop_index) + child_lineage_id = _child_lineage_id(activation, loop_index) add_frame( run, ExecutionFrame( @@ -105,6 +109,7 @@ def _step_foreach_serial( 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_, @@ -141,11 +146,8 @@ def _step_foreach_concurrent( if step.concurrent is None: raise WorkflowExecutionError("concurrent foreach requires concurrent policy") frame = run.current_frame() - barrier = ForeachBarrierState.from_frame(frame, step.id) - if barrier is None: - barrier = ForeachBarrierState(mode="concurrent") - elif barrier.mode != "concurrent": - raise WorkflowExecutionError("malformed concurrent foreach barrier mode") + activation = load_or_begin_foreach_activation(frame, step.id, mode="concurrent") + barrier = activation.barrier _finish_completed_children(run, step, barrier) iterable = _resolve_foreach_iterable(run, frame, step) @@ -154,6 +156,7 @@ def _step_foreach_concurrent( frame=frame, step=step, index=index, + activation=activation, barrier=barrier, iterable=iterable, ) @@ -169,7 +172,7 @@ def _step_foreach_concurrent( reducers=reducers, ) - barrier.save_to_frame(frame, step.id) + save_foreach_activation(frame, activation) block_frame_on_children(run, frame.id, barrier.outstanding_frame_ids) run.sync_from_current_frame() return run @@ -242,6 +245,7 @@ def _admit_concurrent_children( frame: ExecutionFrame, step: ForeachNode, index: WorkflowIndex, + activation: ForeachActivationState, barrier: ForeachBarrierState, iterable: list[object], ) -> None: @@ -256,8 +260,8 @@ def _admit_concurrent_children( ): loop_index = barrier.next_index item = iterable[loop_index] - child_id = f"{frame.id}:{step.id}:{loop_index}" - child_lineage_id = _child_lineage_id(frame, step, 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, @@ -280,6 +284,7 @@ def _admit_concurrent_children( 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_, @@ -372,13 +377,22 @@ def _finish_concurrent_foreach( return run -def _child_lineage_id(frame: ExecutionFrame, step: ForeachNode, loop_index: int) -> str: - """Return a deterministic opaque lineage id for one foreach child frame. +def _child_frame_id(activation: ForeachActivationState, loop_index: int) -> str: + """Return a deterministic opaque child frame id for one activation item. + + The id embeds the activation so a later visit at item zero cannot collide + with the first visit. Compare full ids; never parse them. + """ + return f"{activation.id}:{loop_index}" + + +def _child_lineage_id(activation: ForeachActivationState, loop_index: int) -> str: + """Return a deterministic opaque lineage id for one activation item. The readable shape is only for diagnostics. Runtime code should compare the full id, not parse it; future structured lineage refs can replace this. """ - return f"{frame.lineage_id}/{step.id}[{loop_index}]" + return f"{activation.id}[{loop_index}]" def _patch_for_successful_item( diff --git a/src/wf_core/runtime/ops/nodes.py b/src/wf_core/runtime/ops/nodes.py index 4df383bf..81d6165e 100644 --- a/src/wf_core/runtime/ops/nodes.py +++ b/src/wf_core/runtime/ops/nodes.py @@ -15,7 +15,11 @@ from wf_core.run_state import ( RuntimeContext, StepExecutionResult, ) -from wf_core.runtime.foreach_state import ForeachBarrierState, item_frame_owner +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 ( append_lineage_writes, @@ -116,10 +120,14 @@ def _finalize_node_execution( if owner is None: state_changes = commit_patch_for_frame(run, frame, patch) else: - parent_frame_id, foreach_node_id, item_index = owner - parent_frame = run.frames[parent_frame_id] - barrier = ForeachBarrierState.from_frame(parent_frame, foreach_node_id) - if barrier is not None and barrier.mode == "concurrent": + parent_frame = run.frames[owner.parent_frame_id] + # 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. append_lineage_writes( @@ -129,12 +137,12 @@ def _finalize_node_execution( writes=patch.writes, ) barrier.add_success_patch( - index=item_index, + index=owner.item_index, frame_id=frame.id, patch=StatePatch(), lineage_id=frame.lineage_id, ) - barrier.save_to_frame(parent_frame, foreach_node_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/scheduler.py b/src/wf_core/runtime/scheduler.py index 002c15d7..a8d2b896 100644 --- a/src/wf_core/runtime/scheduler.py +++ b/src/wf_core/runtime/scheduler.py @@ -38,9 +38,15 @@ class BlockedOnChildren: @dataclass(slots=True, frozen=True) class ForeachIterationMetadata: - """Typed metadata for a foreach iteration frame.""" + """Typed metadata for a foreach iteration frame. + + ``activation_id`` names the dynamic foreach visit that owns this item. + It separates fresh barrier state from earlier visits to the same node use + and must survive checkpoint serialization. + """ foreach_node_id: str + activation_id: str loop_index: int loop_item: Any loop_alias: str @@ -51,12 +57,17 @@ class ForeachIterationMetadata: return None metadata = frame.metadata foreach_node_id = metadata.get("foreach_node_id") + activation_id = metadata.get("activation_id") loop_index = metadata.get("loop_index") loop_alias = metadata.get("loop_alias") if not isinstance(foreach_node_id, str) or not foreach_node_id: raise WorkflowExecutionError( f"malformed foreach node id for frame {frame.id!r}" ) + if not isinstance(activation_id, str) or not activation_id: + raise WorkflowExecutionError( + f"malformed foreach activation id for frame {frame.id!r}" + ) if not isinstance(loop_index, int): raise WorkflowExecutionError( f"malformed foreach loop index for frame {frame.id!r}" @@ -71,6 +82,7 @@ class ForeachIterationMetadata: ) return cls( foreach_node_id=foreach_node_id, + activation_id=activation_id, loop_index=loop_index, loop_item=metadata["loop_item"], loop_alias=loop_alias, @@ -79,6 +91,7 @@ class ForeachIterationMetadata: def to_metadata(self) -> dict[str, object]: return { "foreach_node_id": self.foreach_node_id, + "activation_id": self.activation_id, "loop_index": self.loop_index, "loop_item": self.loop_item, "loop_alias": self.loop_alias, @@ -178,7 +191,11 @@ def wake_parent_if_children_complete(run: RunState, child_frame_id: str) -> None def wake_parent_for_child_progress(run: RunState, child_frame_id: str) -> None: - """Wake a blocked parent after one child finishes so it can refill slots.""" + """Wake a blocked parent after one child finishes so it can refill slots. + + The wake-up includes the foreach activation: a child naming a closed or + superseded activation cannot wake a parent waiting on a later visit. + """ child = _frame(run, child_frame_id) parent_id = child.parent_frame_id if parent_id is None: @@ -189,6 +206,26 @@ def wake_parent_for_child_progress(run: RunState, child_frame_id: str) -> None: block = BlockedOnChildren.from_frame(parent) if block is None or child_frame_id not in block.child_frame_ids: return + # Lazy import avoids a cycle: foreach_state owns activation persistence on + # top of this scheduler's frame metadata types. + from wf_core.runtime.foreach_state import ( + item_frame_owner, + load_foreach_activation, + ) + + try: + owner = item_frame_owner(child) + except WorkflowExecutionError: + raise + if owner is not None: + activation = load_foreach_activation( + parent, owner.foreach_node_id, owner.activation_id + ) + if activation is None: + raise WorkflowExecutionError( + f"foreach item frame {child_frame_id!r} names closed activation " + f"{owner.activation_id!r} and cannot wake parent {parent_id!r}" + ) wake_frame(run, parent_id) diff --git a/src/wf_core/runtime/step.py b/src/wf_core/runtime/step.py index 730ebb1d..ddea7f87 100644 --- a/src/wf_core/runtime/step.py +++ b/src/wf_core/runtime/step.py @@ -16,7 +16,7 @@ from wf_core.models.steps import ( ) from wf_core.models.workflow import Workflow from wf_core.run_state import ExecutionFrame, FrameStatus, RunState, StepExecutionResult -from wf_core.runtime.foreach_state import ForeachBarrierState, item_frame_owner +from wf_core.runtime.foreach_state import item_frame_owner, load_foreach_activation from wf_core.runtime.ops.flow import advance_frame, append_step_result_trace from wf_core.runtime.ops.foreach import step_foreach from wf_core.runtime.ops.handlers import ( @@ -361,10 +361,15 @@ def _claim_matching_async_item_frames( index: WorkflowIndex, first_frame: ExecutionFrame, ) -> list[ExecutionFrame]: + """Claim sibling item frames from the same activation for async batching. + + Batching never mixes activations: only frames naming the same parent, + foreach, and activation id run together, preserving deterministic barrier + commits across revisits. + """ owner = item_frame_owner(first_frame) if owner is None: return [] - parent_frame_id, foreach_node_id, _item_index = owner claimed: list[ExecutionFrame] = [] remaining_ready: list[str] = [] for frame_id in run.ready_frame_ids: @@ -373,7 +378,9 @@ def _claim_matching_async_item_frames( if ( frame.status == FrameStatus.PENDING and frame_owner is not None - and frame_owner[:2] == (parent_frame_id, foreach_node_id) + and frame_owner.parent_frame_id == owner.parent_frame_id + and frame_owner.foreach_node_id == owner.foreach_node_id + and frame_owner.activation_id == owner.activation_id and isinstance(index.nodes_by_id.get(frame.node_id), NodeUse) ): frame.status = FrameStatus.RUNNING @@ -392,14 +399,15 @@ def _can_batch_async_foreach_item( owner = item_frame_owner(frame) if owner is None: return False - parent_frame_id, foreach_node_id, _item_index = owner - parent_frame = run.frames.get(parent_frame_id) + parent_frame = run.frames.get(owner.parent_frame_id) if parent_frame is None: return False - barrier = ForeachBarrierState.from_frame(parent_frame, foreach_node_id) + activation = load_foreach_activation( + parent_frame, owner.foreach_node_id, owner.activation_id + ) return ( - barrier is not None - and barrier.mode == "concurrent" + activation is not None + and activation.barrier.mode == "concurrent" and isinstance(index.nodes_by_id.get(frame.node_id), NodeUse) ) diff --git a/tests/core/test_concurrent_foreach.py b/tests/core/test_concurrent_foreach.py index 2ba18c59..c73afdb6 100644 --- a/tests/core/test_concurrent_foreach.py +++ b/tests/core/test_concurrent_foreach.py @@ -19,7 +19,11 @@ from wf_core import ( execute_workflow, ) from wf_core.run_state import ExecutionFrame, RunState, RuntimeContext -from wf_core.runtime.foreach_state import ForeachBarrierState +from wf_core.runtime.foreach_state import ( + ForeachItemOwner, + item_frame_owner, + load_or_begin_foreach_activation, +) from wf_core.runtime.scheduler import ForeachIterationMetadata @@ -143,7 +147,11 @@ def test_concurrent_foreach_item_frames_use_distinct_lineages() -> None: assert run.frames["root"].lineage_id == "root" assert run.frames["root"].parent_lineage_id is None assert len(item_frames) == 2 - assert item_lineage_ids == {"root/each[0]", "root/each[1]"} + assert item_lineage_ids == {"root:each#0[0]", "root:each#0[1]"} + for frame in item_frames: + owner = item_frame_owner(frame) + assert isinstance(owner, ForeachItemOwner) + assert owner.activation_id == "root:each#0" assert set(context_lineage_ids) == item_lineage_ids assert all(frame.scope_id == "root" for frame in item_frames) assert all(frame.parent_lineage_id == "root" for frame in item_frames) @@ -162,15 +170,27 @@ def test_nested_concurrent_foreach_records_parent_child_lineages() -> None: inner_frames = _foreach_frames(run, "inner_each") assert {frame.lineage_id for frame in outer_frames} == { - "root/outer_each[0]", - "root/outer_each[1]", + "root:outer_each#0[0]", + "root:outer_each#0[1]", } assert all(frame.parent_lineage_id == "root" for frame in outer_frames) assert {(frame.parent_lineage_id, frame.lineage_id) for frame in inner_frames} == { - ("root/outer_each[0]", "root/outer_each[0]/inner_each[0]"), - ("root/outer_each[0]", "root/outer_each[0]/inner_each[1]"), - ("root/outer_each[1]", "root/outer_each[1]/inner_each[0]"), - ("root/outer_each[1]", "root/outer_each[1]/inner_each[1]"), + ( + "root:outer_each#0[0]", + "root:outer_each#0:0:inner_each#0[0]", + ), + ( + "root:outer_each#0[0]", + "root:outer_each#0:0:inner_each#0[1]", + ), + ( + "root:outer_each#0[1]", + "root:outer_each#0:1:inner_each#0[0]", + ), + ( + "root:outer_each#0[1]", + "root:outer_each#0:1:inner_each#0[1]", + ), } @@ -264,12 +284,14 @@ def test_sync_concurrent_foreach_barrier_replays_add_reducer_inputs() -> None: assert run.state["number"] == 6 assert run.output["number"] == 6 - assert run.lineages["root/each[0]"].writes[0].incoming_value == 3 - assert run.lineages["root/each[1]"].writes[0].incoming_value == 1 - barrier = ForeachBarrierState.from_frame(run.frames["root"], "each") - assert barrier is not None - assert barrier.pending_results[0].lineage_id == "root/each[0]" - assert barrier.pending_results[0].patch.writes == [] + assert run.lineages["root:each#0[0]"].writes[0].incoming_value == 3 + assert run.lineages["root:each#0[1]"].writes[0].incoming_value == 1 + active = load_or_begin_foreach_activation( + run.frames["root"], "each", mode="concurrent" + ) + assert active.id == "root:each#0" + assert active.barrier.pending_results[0].lineage_id == "root:each#0[0]" + assert active.barrier.pending_results[0].patch.writes == [] foreach_entries = [entry for entry in run.trace if entry.step_type == "foreach"] assert foreach_entries[-1].state_changes["state.number"] == 6 diff --git a/tests/core/test_concurrent_foreach_errors.py b/tests/core/test_concurrent_foreach_errors.py index 609f9508..18c1e3a4 100644 --- a/tests/core/test_concurrent_foreach_errors.py +++ b/tests/core/test_concurrent_foreach_errors.py @@ -27,7 +27,7 @@ def test_concurrent_foreach_skip_emits_completed_with_errors() -> None: ) assert run.state["seen"] == ["a", "c"] - assert run.frames["root:each:1"].status == "failed" + assert run.frames["root:each#0:1"].status == "failed" foreach_entries = [entry for entry in run.trace if entry.step_type == "foreach"] assert foreach_entries[-1].outcome == "completed_with_errors" assert foreach_entries[-1].resolved_input["failed_items"] == 1 @@ -47,7 +47,7 @@ def test_concurrent_foreach_collect_writes_ordered_error_records() -> None: assert len(run.state["errors"]) == 1 error = run.state["errors"][0] assert error["index"] == 1 - assert error["frame_id"] == "root:each:1" + assert error["frame_id"] == "root:each#0:1" assert error["node_id"] == "record" assert error["error_type"] == "ValueError" assert error["message"] == "bad item" diff --git a/tests/core/test_concurrent_foreach_interrupts.py b/tests/core/test_concurrent_foreach_interrupts.py index f171e700..3463e473 100644 --- a/tests/core/test_concurrent_foreach_interrupts.py +++ b/tests/core/test_concurrent_foreach_interrupts.py @@ -31,8 +31,8 @@ async def test_concurrent_foreach_interrupt_returns_before_refill() -> None: assert run.status is RunStatus.INTERRUPTED assert run.interrupt is not None assert run.interrupt.payload["item"] == "b" - assert run.frames["root:each:1"].status == "interrupted" - assert "root:each:2" not in run.frames + assert run.frames["root:each#0:1"].status == "interrupted" + assert "root:each#0:2" not in run.frames assert "seen" not in run.state @@ -54,7 +54,7 @@ async def test_resume_prioritizes_interrupted_item_before_siblings() -> None: assert resumed.status is RunStatus.COMPLETED assert resumed.state["seen"] == ["a", "b", "c"] - assert resumed.trace[interrupted_trace_len].frame_id == "root:each:1" + assert resumed.trace[interrupted_trace_len].frame_id == "root:each#0:1" assert resumed.trace[interrupted_trace_len].step_type == "interrupt" assert resumed.trace[interrupted_trace_len].outcome == "submitted" foreach_entries = [entry for entry in resumed.trace if entry.step_type == "foreach"] diff --git a/tests/core/test_foreach_activations.py b/tests/core/test_foreach_activations.py new file mode 100644 index 00000000..cb94d0f6 --- /dev/null +++ b/tests/core/test_foreach_activations.py @@ -0,0 +1,102 @@ +from __future__ import annotations + +import pytest + +from wf_core.errors import WorkflowExecutionError +from wf_core.run_state import ExecutionFrame +from wf_core.runtime.foreach_state import ( + close_foreach_activation, + item_frame_owner, + load_or_begin_foreach_activation, + save_foreach_activation, +) +from wf_core.runtime.scheduler import ForeachIterationMetadata + + +def _frame() -> ExecutionFrame: + return ExecutionFrame(id="root", kind="workflow", node_id="each") + + +def test_activation_lifecycle_reuses_active_then_fresh_after_close() -> None: + frame = _frame() + + first = load_or_begin_foreach_activation(frame, "each", mode="serial") + save_foreach_activation(frame, first) + restored = load_or_begin_foreach_activation(frame, "each", mode="serial") + + assert restored.id == first.id + + close_foreach_activation(frame, restored) + second = load_or_begin_foreach_activation(frame, "each", mode="serial") + + assert second.id != first.id + assert second.barrier.next_index == 0 + + +def test_activation_rejects_malformed_metadata() -> None: + frame = ExecutionFrame( + id="root", + kind="workflow", + node_id="each", + metadata={"foreach_activations": "corrupt"}, + ) + + with pytest.raises(WorkflowExecutionError, match="activation"): + load_or_begin_foreach_activation(frame, "each", mode="serial") + + +def test_activation_rejects_mode_mismatch() -> None: + frame = _frame() + activation = load_or_begin_foreach_activation(frame, "each", mode="serial") + save_foreach_activation(frame, activation) + + with pytest.raises(WorkflowExecutionError, match="mode"): + load_or_begin_foreach_activation(frame, "each", mode="concurrent") + + +def test_closing_stale_activation_fails_closed() -> None: + frame = _frame() + first = load_or_begin_foreach_activation(frame, "each", mode="serial") + save_foreach_activation(frame, first) + close_foreach_activation(frame, first) + second = load_or_begin_foreach_activation(frame, "each", mode="serial") + save_foreach_activation(frame, second) + + with pytest.raises(WorkflowExecutionError, match="stale|closed|active"): + close_foreach_activation(frame, first) + + +def test_activation_json_round_trip_through_frame_metadata() -> None: + frame = _frame() + activation = load_or_begin_foreach_activation(frame, "each", mode="serial") + activation.barrier.next_index = 2 + save_foreach_activation(frame, activation) + + dumped = dict(frame.metadata) + restored_frame = ExecutionFrame( + id="root", kind="workflow", node_id="each", metadata=dumped + ) + restored = load_or_begin_foreach_activation(restored_frame, "each", mode="serial") + + assert restored.id == activation.id + assert restored.barrier.next_index == 2 + + +def test_item_metadata_requires_activation_identity() -> None: + frame = ExecutionFrame( + id="root:each#0:0", + kind="foreach_iteration", + node_id="work", + parent_frame_id="root", + metadata={ + "foreach_node_id": "each", + "loop_index": 0, + "loop_item": "a", + "loop_alias": "item", + }, + ) + + with pytest.raises(WorkflowExecutionError, match="activation"): + ForeachIterationMetadata.from_frame(frame) + with pytest.raises(WorkflowExecutionError, match="activation"): + item_frame_owner(frame) diff --git a/tests/core/test_foreach_barrier_state.py b/tests/core/test_foreach_barrier_state.py index a428648d..9bf44abf 100644 --- a/tests/core/test_foreach_barrier_state.py +++ b/tests/core/test_foreach_barrier_state.py @@ -8,9 +8,13 @@ from wf_core.paths import StatePath from wf_core.run_state import ExecutionFrame, RunState, RunStatus, StateWrite from wf_core.runtime.foreach_state import ( ForeachBarrierState, + ForeachItemOwner, ItemErrorRecord, PendingItemResult, _state_write_from_metadata, + item_frame_owner, + 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 @@ -134,20 +138,28 @@ def test_lineage_state_view_materializes_visible_values_without_mutating_base() def test_lineage_writes_for_frame_reads_current_foreach_pending_result() -> 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]" child = ExecutionFrame( - id="root:each:0", + id=f"{activation.id}:0", kind="foreach_iteration", node_id="work", parent_frame_id="root", - lineage_id="root/each[0]", + lineage_id=child_lineage_id, parent_lineage_id="root", metadata={ "foreach_node_id": "each", + "activation_id": activation.id, "loop_index": 0, "loop_item": "a", "loop_alias": "item", }, ) + # Ownership is named, not positional. + owner = item_frame_owner(child) + assert isinstance(owner, ForeachItemOwner) + assert owner.activation_id == activation.id + assert owner.item_index == 0 patch = StatePatch( writes=[ StateWrite( @@ -158,19 +170,14 @@ def test_lineage_writes_for_frame_reads_current_foreach_pending_result() -> None ) ] ) - barrier = ForeachBarrierState( - mode="concurrent", - pending_results={ - 0: PendingItemResult( - index=0, - frame_id=child.id, - status="succeeded", - lineage_id=child.lineage_id, - patch=patch, - ) - }, + activation.barrier.pending_results[0] = PendingItemResult( + index=0, + frame_id=child.id, + status="succeeded", + lineage_id=child.lineage_id, + patch=patch, ) - barrier.save_to_frame(parent, "each") + save_foreach_activation(parent, activation) run = RunState( workflow_name="lineage", status=RunStatus.PENDING, @@ -188,12 +195,13 @@ def test_lineage_writes_for_frame_reads_current_foreach_pending_result() -> None def test_lineage_writes_for_frame_rejects_missing_compatibility_parent_frame() -> None: child = ExecutionFrame( - id="missing:each:0", + 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", diff --git a/tests/core/test_scheduler.py b/tests/core/test_scheduler.py index 741d24e7..b88689aa 100644 --- a/tests/core/test_scheduler.py +++ b/tests/core/test_scheduler.py @@ -163,8 +163,19 @@ def test_child_completion_wakes_blocked_parent() -> None: def test_wake_parent_when_child_finishes_for_refill() -> None: + from wf_core.runtime.foreach_state import ( + ForeachItemOwner, + item_frame_owner, + load_or_begin_foreach_activation, + save_foreach_activation, + ) + run = _run() add_frame(run, ExecutionFrame(id="parent", kind="root", node_id="foreach")) + activation = load_or_begin_foreach_activation( + run.frames["parent"], "foreach", mode="serial" + ) + save_foreach_activation(run.frames["parent"], activation) add_frame( run, ExecutionFrame( @@ -172,11 +183,22 @@ def test_wake_parent_when_child_finishes_for_refill() -> None: kind="foreach_iteration", node_id="__end__", parent_frame_id="parent", + metadata={ + "foreach_node_id": "foreach", + "activation_id": activation.id, + "loop_index": 0, + "loop_item": "a", + "loop_alias": "item", + }, ), ) block_frame_on_children(run, "parent", ("child", "other")) run.frames["child"].status = FrameStatus.COMPLETED + owner = item_frame_owner(run.frames["child"]) + assert isinstance(owner, ForeachItemOwner) + assert owner.activation_id == activation.id + wake_parent_for_child_progress(run, "child") assert run.frames["parent"].status == FrameStatus.PENDING From f23e760b5fbdeb8426907be9514d3dffb69d54db Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 07:40:49 +0700 Subject: [PATCH 04/15] feat: return foreach items through owner back-edges --- examples/authoring_concurrent_foreach.py | 4 +- examples/demo_workflow.py | 2 +- examples/raw_concurrent_foreach.py | 2 +- src/wf_core/runtime/ops/flow.py | 58 ++ src/wf_core/runtime/ops/foreach.py | 17 + src/wf_core/runtime/step.py | 5 + src/wf_core/runtime/subgraphs.py | 26 +- tests/authoring/test_demo_workflow.py | 2 +- tests/core/test_concurrent_foreach.py | 42 +- tests/core/test_concurrent_foreach_async.py | 2 +- tests/core/test_concurrent_foreach_errors.py | 2 +- .../test_concurrent_foreach_interrupts.py | 11 +- tests/core/test_foreach_back_edges.py | 750 ++++++++++++++++++ 13 files changed, 902 insertions(+), 21 deletions(-) create mode 100644 tests/core/test_foreach_back_edges.py diff --git a/examples/authoring_concurrent_foreach.py b/examples/authoring_concurrent_foreach.py index cb2f701d..46959f59 100644 --- a/examples/authoring_concurrent_foreach.py +++ b/examples/authoring_concurrent_foreach.py @@ -114,7 +114,7 @@ def build_concurrent_foreach_workflow( ) builder.set_entry_point(each) builder.connect(each, "loop", record) - builder.connect(record, "ok", END) + builder.connect(record, "ok", each) builder.connect(each, "done", END) if _item_error_action(item_error) in {"collect", "skip"}: builder.connect(each, "completed_with_errors", END) @@ -165,7 +165,7 @@ def run_replace_conflict_example() -> None: ) builder.set_entry_point(each) builder.connect(each, "loop", record) - builder.connect(record, "ok", END) + builder.connect(record, "ok", each) builder.connect(each, "done", END) try: builder.execute({"items": ["a", "b"]}) diff --git a/examples/demo_workflow.py b/examples/demo_workflow.py index d94a49a1..58f8343d 100644 --- a/examples/demo_workflow.py +++ b/examples/demo_workflow.py @@ -255,7 +255,7 @@ def build_demo_workflow() -> Workflow: "outcome": "done", "to": "combine_summaries", }, - {"from": "summarize_one", "outcome": "ok", "to": END}, + {"from": "summarize_one", "outcome": "ok", "to": "summarize_each"}, {"from": "combine_summaries", "outcome": "ok", "to": "should_email"}, {"from": "should_email", "outcome": "true", "to": "approve_email"}, {"from": "should_email", "outcome": "false", "to": "skip_email"}, diff --git a/examples/raw_concurrent_foreach.py b/examples/raw_concurrent_foreach.py index 15cb2265..50e744a2 100644 --- a/examples/raw_concurrent_foreach.py +++ b/examples/raw_concurrent_foreach.py @@ -97,7 +97,7 @@ def build_raw_concurrent_foreach_workflow() -> Workflow: ], "edges": [ {"from": "each", "outcome": "loop", "to": "record"}, - {"from": "record", "outcome": "ok", "to": END}, + {"from": "record", "outcome": "ok", "to": "each"}, {"from": "each", "outcome": "done", "to": END}, {"from": "each", "outcome": "completed_with_errors", "to": END}, ], diff --git a/src/wf_core/runtime/ops/flow.py b/src/wf_core/runtime/ops/flow.py index 7950aebb..2f50a70c 100644 --- a/src/wf_core/runtime/ops/flow.py +++ b/src/wf_core/runtime/ops/flow.py @@ -2,6 +2,7 @@ from __future__ import annotations from typing import Any +from wf_core.errors import WorkflowExecutionError from wf_core.models.workflow import Workflow from wf_core.run_state import ( ExecutionFrame, @@ -76,6 +77,39 @@ def advance_frame( next_node_id: str, front: bool = False, ) -> None: + # 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 + + owner = item_frame_owner(frame) + if owner is not None: + if next_node_id == END: + raise WorkflowExecutionError( + f"foreach item frame {frame.id!r} cannot target workflow END; " + f"return to owning foreach {owner.foreach_node_id!r}" + ) + if next_node_id == owner.foreach_node_id: + source_node_id = frame.node_id + frame.prior_outcome = outcome + frame.activated_incoming_edge = source_node_id + frame.node_id = owner.foreach_node_id + frame.status = FrameStatus.COMPLETED + frame.finished_at_node_id = owner.foreach_node_id + # The child does not execute the controller again; the blocked + # parent activation consumes the result and admits the next item + # or emits done. The owner location stays inspectable in trace + # and checkpoint state. + wake_parent_for_child_progress(run, frame.id) + run.sync_from_current_frame() + return + ancestors = _foreach_ancestor_ids(run, frame) + if next_node_id in ancestors[1:]: + raise WorkflowExecutionError( + f"foreach item frame {frame.id!r} targets non-immediate " + f"ancestor {next_node_id!r}; only {owner.foreach_node_id!r} " + "can complete this item" + ) frame.prior_outcome = outcome frame.activated_incoming_edge = frame.node_id frame.node_id = next_node_id @@ -93,6 +127,30 @@ def advance_frame( run.sync_from_current_frame() +def _foreach_ancestor_ids(run: RunState, frame: ExecutionFrame) -> list[str]: + """Derive active foreach owners from frame ancestry for fail-closed checks. + + The first entry is the frame's immediate owner; later entries are older + ancestors. A target naming an older ancestor is a non-local return, while + a target naming an inactive foreach is an ordinary nested entry. + """ + from wf_core.runtime.foreach_state import item_frame_owner + + ancestors: list[str] = [] + cursor: ExecutionFrame | None = frame + seen: set[str] = set() + while cursor is not None: + owner = item_frame_owner(cursor) + if owner is not None: + if owner.foreach_node_id in seen: + break + seen.add(owner.foreach_node_id) + ancestors.append(owner.foreach_node_id) + parent_id = cursor.parent_frame_id + cursor = run.frames.get(parent_id) if parent_id is not None else None + return ancestors + + def finalize_run(workflow: Workflow, run: RunState) -> RunState: if run.outcome is None: run.outcome = "ok" diff --git a/src/wf_core/runtime/ops/foreach.py b/src/wf_core/runtime/ops/foreach.py index b45b15ef..dec9178b 100644 --- a/src/wf_core/runtime/ops/foreach.py +++ b/src/wf_core/runtime/ops/foreach.py @@ -12,6 +12,7 @@ from wf_core.runtime.foreach_state import ( ForeachBarrierState, ItemErrorRecord, PendingItemResult, + close_foreach_activation, load_or_begin_foreach_activation, save_foreach_activation, ) @@ -87,6 +88,9 @@ def _step_foreach_serial( state_changes={}, ), ) + # Close the visit before following `done` so a self-looping completion + # edge or a later revisit starts a fresh activation. + close_foreach_activation(frame, activation) advance_frame(run, frame, outcome=outcome, next_node_id=next_node_id) return run @@ -96,6 +100,15 @@ def _step_foreach_serial( 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( @@ -168,6 +181,7 @@ def _step_foreach_concurrent( frame=frame, step=step, index=index, + activation=activation, barrier=barrier, reducers=reducers, ) @@ -318,6 +332,7 @@ def _finish_concurrent_foreach( frame: ExecutionFrame, step: ForeachNode, index: WorkflowIndex, + activation: ForeachActivationState, barrier: ForeachBarrierState, reducers: Mapping[str, ReducerDefinition] | None = None, ) -> RunState: @@ -373,6 +388,8 @@ def _finish_concurrent_foreach( state_changes=state_changes, ), ) + # Close the visit before following completion so later revisits start fresh. + close_foreach_activation(frame, activation) advance_frame(run, frame, outcome=outcome, next_node_id=next_node_id) return run diff --git a/src/wf_core/runtime/step.py b/src/wf_core/runtime/step.py index ddea7f87..1da1491c 100644 --- a/src/wf_core/runtime/step.py +++ b/src/wf_core/runtime/step.py @@ -87,6 +87,11 @@ def complete_end_step( """Record an explicit workflow terminal and complete the active frame.""" result = StepExecutionResult(outcome=outcome) frame = run.frames[frame_id] + if item_frame_owner(frame) is not None: + raise WorkflowExecutionError( + f"foreach item frame {frame.id!r} cannot target explicit end node " + f"{node_id!r}; return to its owning foreach" + ) frame.metadata["workflow_outcome"] = outcome if frame.parent_frame_id is None: run.outcome = outcome diff --git a/src/wf_core/runtime/subgraphs.py b/src/wf_core/runtime/subgraphs.py index 3292586e..013950c0 100644 --- a/src/wf_core/runtime/subgraphs.py +++ b/src/wf_core/runtime/subgraphs.py @@ -236,7 +236,31 @@ def _finish_subgraph( reducers=reducers, missing_field_message="subgraph output did not include required field {field}", ) - state_changes = commit_patch_for_frame(run, frame, patch) + # Match node execution: serial item writes commit through the parent + # scope so top-level serial subgraphs land in root state; concurrent + # item writes stay buffered in the item lineage for barrier merge. + from wf_core.runtime.foreach_state import ( + item_frame_owner, + load_foreach_activation, + ) + + commit_frame = frame + try: + owner = item_frame_owner(frame) + except Exception: + owner = None + 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 ( + foreach_activation is not None + and foreach_activation.barrier.mode == "serial" + ): + commit_frame = parent_frame + state_changes = commit_patch_for_frame(run, commit_frame, patch) return StepExecutionResult( outcome=child_outcome, resolved_input=activation.child_input, diff --git a/tests/authoring/test_demo_workflow.py b/tests/authoring/test_demo_workflow.py index 79c725ce..a14de31c 100644 --- a/tests/authoring/test_demo_workflow.py +++ b/tests/authoring/test_demo_workflow.py @@ -210,7 +210,7 @@ def build_authoring_demo_workflow(): builder.connect(list_files, "ok", summarize_each) builder.connect(summarize_each, "loop", summarize_one) builder.connect(summarize_each, "done", combine_summaries) - builder.connect(summarize_one, "ok", END) + builder.connect(summarize_one, "ok", summarize_each) builder.connect(combine_summaries, "ok", should_email) builder.connect(should_email, "true", approve_email) builder.connect(should_email, "false", skip_email) diff --git a/tests/core/test_concurrent_foreach.py b/tests/core/test_concurrent_foreach.py index c73afdb6..936f8ad0 100644 --- a/tests/core/test_concurrent_foreach.py +++ b/tests/core/test_concurrent_foreach.py @@ -286,12 +286,14 @@ def test_sync_concurrent_foreach_barrier_replays_add_reducer_inputs() -> None: assert run.output["number"] == 6 assert run.lineages["root:each#0[0]"].writes[0].incoming_value == 3 assert run.lineages["root:each#0[1]"].writes[0].incoming_value == 1 - active = load_or_begin_foreach_activation( + # The visit closed before `done`; lineage history remains while a new + # load starts fresh barrier state with a new activation id. + fresh = load_or_begin_foreach_activation( run.frames["root"], "each", mode="concurrent" ) - assert active.id == "root:each#0" - assert active.barrier.pending_results[0].lineage_id == "root:each#0[0]" - assert active.barrier.pending_results[0].patch.writes == [] + assert fresh.id == "root:each#1" + assert fresh.barrier.next_index == 0 + assert fresh.barrier.pending_results == {} foreach_entries = [entry for entry in run.trace if entry.step_type == "foreach"] assert foreach_entries[-1].state_changes["state.number"] == 6 @@ -407,7 +409,7 @@ def _sum_items_workflow() -> Workflow: ], edges=[ Edge.model_validate({"from": "each", "outcome": "loop", "to": "add_item"}), - Edge.model_validate({"from": "add_item", "outcome": "ok", "to": END}), + Edge.model_validate({"from": "add_item", "outcome": "ok", "to": "each"}), Edge.model_validate({"from": "each", "outcome": "done", "to": END}), ], ) @@ -517,7 +519,7 @@ def _same_item_reducer_visibility_workflow() -> Workflow: "to": "read_number", } ), - Edge.model_validate({"from": "read_number", "outcome": "ok", "to": END}), + Edge.model_validate({"from": "read_number", "outcome": "ok", "to": "each"}), Edge.model_validate({"from": "each", "outcome": "done", "to": END}), ], ) @@ -593,6 +595,15 @@ def _nested_foreach_lineage_workflow() -> Workflow: "output": [{"source": "seen", "target": "state.seen"}], } ), + NodeUse.model_validate( + { + "id": "tail", + "type": "node", + "node": "record", + "input": [{"target": "seen", "path": "context.outer"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), ], edges=[ Edge.model_validate( @@ -609,8 +620,13 @@ def _nested_foreach_lineage_workflow() -> Workflow: "to": "record", } ), - Edge.model_validate({"from": "record", "outcome": "ok", "to": END}), - Edge.model_validate({"from": "inner_each", "outcome": "done", "to": END}), + Edge.model_validate( + {"from": "record", "outcome": "ok", "to": "inner_each"} + ), + Edge.model_validate( + {"from": "inner_each", "outcome": "done", "to": "tail"} + ), + Edge.model_validate({"from": "tail", "outcome": "ok", "to": "outer_each"}), Edge.model_validate({"from": "outer_each", "outcome": "done", "to": END}), ], ) @@ -624,7 +640,7 @@ def _workflow( ) -> Workflow: edges = [ Edge.model_validate({"from": "each", "outcome": "loop", "to": "record"}), - Edge.model_validate({"from": "record", "outcome": "ok", "to": END}), + Edge.model_validate({"from": "record", "outcome": "ok", "to": "each"}), Edge.model_validate({"from": "each", "outcome": "done", "to": END}), ] if include_completed_with_errors: @@ -794,7 +810,9 @@ def _multi_step_overlay_workflow() -> Workflow: "to": "read_scratch", } ), - Edge.model_validate({"from": "read_scratch", "outcome": "ok", "to": END}), + Edge.model_validate( + {"from": "read_scratch", "outcome": "ok", "to": "each"} + ), Edge.model_validate({"from": "each", "outcome": "done", "to": END}), ], ) @@ -861,7 +879,9 @@ def _same_path_replace_workflow() -> Workflow: "to": "write_winner", } ), - Edge.model_validate({"from": "write_winner", "outcome": "ok", "to": END}), + Edge.model_validate( + {"from": "write_winner", "outcome": "ok", "to": "each"} + ), Edge.model_validate({"from": "each", "outcome": "done", "to": END}), ], ) diff --git a/tests/core/test_concurrent_foreach_async.py b/tests/core/test_concurrent_foreach_async.py index 473bfdbd..f07abdb5 100644 --- a/tests/core/test_concurrent_foreach_async.py +++ b/tests/core/test_concurrent_foreach_async.py @@ -123,7 +123,7 @@ def _workflow(*, max_active: int) -> Workflow: ], edges=[ Edge.model_validate({"from": "each", "outcome": "loop", "to": "record"}), - Edge.model_validate({"from": "record", "outcome": "ok", "to": END}), + Edge.model_validate({"from": "record", "outcome": "ok", "to": "each"}), Edge.model_validate({"from": "each", "outcome": "done", "to": END}), ], ) diff --git a/tests/core/test_concurrent_foreach_errors.py b/tests/core/test_concurrent_foreach_errors.py index 18c1e3a4..a8d087e2 100644 --- a/tests/core/test_concurrent_foreach_errors.py +++ b/tests/core/test_concurrent_foreach_errors.py @@ -144,7 +144,7 @@ def _workflow(*, item_error: dict[str, object]) -> Workflow: ], edges=[ Edge.model_validate({"from": "each", "outcome": "loop", "to": "record"}), - Edge.model_validate({"from": "record", "outcome": "ok", "to": END}), + Edge.model_validate({"from": "record", "outcome": "ok", "to": "each"}), Edge.model_validate({"from": "each", "outcome": "done", "to": END}), Edge.model_validate( { diff --git a/tests/core/test_concurrent_foreach_interrupts.py b/tests/core/test_concurrent_foreach_interrupts.py index 3463e473..009f488f 100644 --- a/tests/core/test_concurrent_foreach_interrupts.py +++ b/tests/core/test_concurrent_foreach_interrupts.py @@ -52,8 +52,15 @@ async def test_resume_prioritizes_interrupted_item_before_siblings() -> None: resume_payload={}, ) + from wf_core.runtime.foreach_state import item_frame_owner + + interrupted_owner = item_frame_owner(run.frames["root:each#0:1"]) + assert interrupted_owner is not None assert resumed.status is RunStatus.COMPLETED assert resumed.state["seen"] == ["a", "b", "c"] + resumed_owner = item_frame_owner(resumed.frames["root:each#0:1"]) + assert resumed_owner is not None + assert resumed_owner.activation_id == interrupted_owner.activation_id assert resumed.trace[interrupted_trace_len].frame_id == "root:each#0:1" assert resumed.trace[interrupted_trace_len].step_type == "interrupt" assert resumed.trace[interrupted_trace_len].outcome == "submitted" @@ -132,7 +139,7 @@ def _workflow() -> Workflow: ], edges=[ Edge.model_validate({"from": "each", "outcome": "loop", "to": "route"}), - Edge.model_validate({"from": "route", "outcome": "ok", "to": END}), + Edge.model_validate({"from": "route", "outcome": "ok", "to": "each"}), Edge.model_validate( { "from": "route", @@ -140,7 +147,7 @@ def _workflow() -> Workflow: "to": "ask", } ), - Edge.model_validate({"from": "ask", "outcome": "submitted", "to": END}), + Edge.model_validate({"from": "ask", "outcome": "submitted", "to": "each"}), Edge.model_validate({"from": "each", "outcome": "done", "to": END}), ], ) diff --git a/tests/core/test_foreach_back_edges.py b/tests/core/test_foreach_back_edges.py new file mode 100644 index 00000000..8a2b969e --- /dev/null +++ b/tests/core/test_foreach_back_edges.py @@ -0,0 +1,750 @@ +from __future__ import annotations + +from typing import Any + +import pytest + +from wf_core import ( + END, + ConditionNode, + Edge, + ForeachNode, + NodeDef, + NodeUse, + ReducerRef, + SchemaRef, + StateField, + StateSchema, + SubgraphNode, + Workflow, + WorkflowExecutionError, + execute_workflow, +) +from wf_core.run_state import ExecutionFrame, FrameStatus, RunState, RunStatus +from wf_core.runtime.foreach_state import item_frame_owner +from wf_core.runtime.scheduler import add_frame + + +def _node_use(node_id: str, *, node: str = "record") -> NodeUse: + return NodeUse.model_validate( + { + "id": node_id, + "type": "node", + "node": node, + "input": [{"target": "value", "path": "context.item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ) + + +def _serial_workflow() -> Workflow: + foreach = ForeachNode.model_validate( + { + "id": "each", + "type": "foreach", + "over": "state.items", + "as": "item", + "mode": "serial", + } + ) + return Workflow( + name="foreach_back_edge", + 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=[ + NodeDef( + name="record", + input_schema=SchemaRef( + type="object", properties={"value": {}}, required=["value"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["ok"], + ) + ], + start="each", + nodes=[ + foreach, + NodeUse.model_validate( + { + "id": "work", + "type": "node", + "node": "record", + "input": [{"target": "value", "path": "context.item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + ], + edges=[ + Edge.model_validate({"from": "each", "outcome": "loop", "to": "work"}), + Edge.model_validate({"from": "work", "outcome": "ok", "to": "each"}), + Edge.model_validate({"from": "each", "outcome": "done", "to": END}), + ], + ) + + +def test_serial_item_return_wakes_parent_and_admits_next_item() -> None: + workflow = _serial_workflow() + + run = execute_workflow( + workflow, + {"items": ["a", "b"]}, + { + "record": lambda payload, _ctx: { + "outcome": "ok", + "output": {"seen": payload["value"]}, + } + }, + ) + + assert run.status == RunStatus.COMPLETED + assert run.state["seen"] == ["a", "b"] + item_frames = [ + frame for frame in run.frames.values() if frame.kind == "foreach_iteration" + ] + assert len(item_frames) == 2 + assert all(frame.status == FrameStatus.COMPLETED for frame in item_frames) + assert all(frame.finished_at_node_id == "each" for frame in item_frames) + assert run.output["seen"] == ["a", "b"] + + +def test_foreach_body_cycle_can_repeat_then_return() -> None: + foreach = ForeachNode.model_validate( + { + "id": "each", + "type": "foreach", + "over": "state.items", + "as": "item", + "mode": "serial", + } + ) + workflow = Workflow( + name="foreach_body_cycle", + 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=[ + NodeDef( + name="step_a", + input_schema=SchemaRef( + type="object", properties={"value": {}}, required=["value"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["again", "done"], + ), + NodeDef( + name="step_b", + input_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["ok"], + ), + ], + start="each", + nodes=[ + foreach, + NodeUse.model_validate( + { + "id": "a", + "type": "node", + "node": "step_a", + "input": [{"target": "value", "path": "context.item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + NodeUse.model_validate( + { + "id": "b", + "type": "node", + "node": "step_b", + "input": [{"target": "seen", "path": "context.item"}], + "output": [], + } + ), + ], + edges=[ + Edge.model_validate({"from": "each", "outcome": "loop", "to": "a"}), + Edge.model_validate({"from": "a", "outcome": "again", "to": "b"}), + Edge.model_validate({"from": "b", "outcome": "ok", "to": "a"}), + Edge.model_validate({"from": "a", "outcome": "done", "to": "each"}), + Edge.model_validate({"from": "each", "outcome": "done", "to": END}), + ], + ) + calls = {"count": 0} + + def step_a(payload: dict[str, Any], _ctx: object) -> dict[str, Any]: + calls["count"] += 1 + outcome = "again" if calls["count"] == 1 else "done" + return {"outcome": outcome, "output": {"seen": payload["value"]}} + + run = execute_workflow( + workflow, + {"items": ["x"]}, + { + "step_a": step_a, + "step_b": lambda payload, _ctx: { + "outcome": "ok", + "output": {"seen": payload["seen"]}, + }, + }, + ) + + assert run.status == RunStatus.COMPLETED + # `b` writes nothing; `a` writes once per visit (again + done). + assert run.state["seen"] == ["x", "x"] + + +def test_conditional_body_can_return_on_either_outcome() -> None: + foreach = ForeachNode.model_validate( + { + "id": "each", + "type": "foreach", + "over": "state.items", + "as": "item", + "mode": "serial", + } + ) + workflow = Workflow( + name="foreach_conditional_return", + 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=[ + NodeDef( + name="decide", + input_schema=SchemaRef( + type="object", properties={"value": {}}, required=["value"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["true", "false"], + ), + NodeDef( + name="work", + input_schema=SchemaRef( + type="object", properties={"value": {}}, required=["value"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["ok"], + ), + ], + start="each", + nodes=[ + foreach, + NodeUse.model_validate( + { + "id": "condition", + "type": "node", + "node": "decide", + "input": [{"target": "value", "path": "context.item"}], + "output": [], + } + ), + NodeUse.model_validate( + { + "id": "work", + "type": "node", + "node": "work", + "input": [{"target": "value", "path": "context.item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + NodeUse.model_validate( + { + "id": "work_false", + "type": "node", + "node": "work", + "input": [{"target": "value", "path": "context.item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + ], + edges=[ + Edge.model_validate({"from": "each", "outcome": "loop", "to": "condition"}), + Edge.model_validate({"from": "condition", "outcome": "true", "to": "work"}), + Edge.model_validate( + {"from": "condition", "outcome": "false", "to": "work_false"} + ), + Edge.model_validate({"from": "work", "outcome": "ok", "to": "each"}), + Edge.model_validate({"from": "work_false", "outcome": "ok", "to": "each"}), + Edge.model_validate({"from": "each", "outcome": "done", "to": END}), + ], + ) + + def decide(payload: dict[str, Any], _ctx: object) -> dict[str, Any]: + outcome = "true" if payload["value"] == "a" else "false" + return {"outcome": outcome, "output": {"seen": payload["value"]}} + + def _work(payload: dict[str, Any], _ctx: object) -> dict[str, Any]: + return {"outcome": "ok", "output": {"seen": payload["value"]}} + + run = execute_workflow( + workflow, + {"items": ["a", "b"]}, + {"decide": decide, "work": _work}, + ) + + assert run.status == RunStatus.COMPLETED + assert sorted(run.state["seen"]) == ["a", "b"] + + +def test_nested_foreach_returns_inner_then_outer() -> None: + outer = ForeachNode.model_validate( + { + "id": "outer", + "type": "foreach", + "over": "state.items", + "as": "outer_item", + "mode": "concurrent", + "concurrent": {"max_active": 2, "max_outstanding": 2}, + } + ) + inner = ForeachNode.model_validate( + { + "id": "inner", + "type": "foreach", + "over": "state.inner_items", + "as": "inner_item", + "mode": "concurrent", + "concurrent": {"max_active": 2, "max_outstanding": 2}, + } + ) + workflow = Workflow( + name="nested_foreach_return", + input_schema=SchemaRef(type="object", properties={}), + state_schema=StateSchema.from_field_map( + { + "items": StateField(type="array"), + "inner_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=[ + NodeDef( + name="work", + input_schema=SchemaRef( + type="object", properties={"value": {}}, required=["value"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["ok"], + ) + ], + start="outer", + nodes=[ + outer, + inner, + NodeUse.model_validate( + { + "id": "work", + "type": "node", + "node": "work", + "input": [{"target": "value", "path": "context.inner_item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + NodeUse.model_validate( + { + "id": "tail", + "type": "node", + "node": "work", + "input": [{"target": "value", "path": "context.outer_item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + NodeUse.model_validate( + { + "id": "after", + "type": "node", + "node": "work", + "input": [{"target": "value", "path": "state.seen"}], + "output": [], + } + ), + ], + edges=[ + Edge.model_validate({"from": "outer", "outcome": "loop", "to": "inner"}), + Edge.model_validate({"from": "inner", "outcome": "loop", "to": "work"}), + Edge.model_validate({"from": "work", "outcome": "ok", "to": "inner"}), + Edge.model_validate({"from": "inner", "outcome": "done", "to": "tail"}), + Edge.model_validate({"from": "tail", "outcome": "ok", "to": "outer"}), + Edge.model_validate({"from": "outer", "outcome": "done", "to": "after"}), + Edge.model_validate({"from": "after", "outcome": "ok", "to": END}), + ], + ) + + run = execute_workflow( + workflow, + {"items": ["a"], "inner_items": [1, 2]}, + { + "work": lambda payload, _ctx: { + "outcome": "ok", + "output": {"seen": payload["value"]}, + } + }, + ) + + assert run.status == RunStatus.COMPLETED + # Inner items 1, 2 plus outer tail "a" plus after echo. + assert run.state["seen"][:3] == [1, 2, "a"] + + +def test_reentering_foreach_uses_fresh_activation_and_item_frames() -> None: + workflow = Workflow( + name="foreach_reentry", + input_schema=SchemaRef(type="object", properties={}), + state_schema=StateSchema.from_field_map( + { + "items": StateField(type="array"), + "count": StateField(type="integer", default=0), + "seen": StateField( + type="array", reducer=ReducerRef(name="wf.std.append") + ), + } + ), + output_schema=SchemaRef(type="object", properties={"seen": {"type": "array"}}), + node_defs=[ + NodeDef( + name="record", + input_schema=SchemaRef( + type="object", properties={"value": {}}, required=["value"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["ok"], + ), + NodeDef( + name="bump", + input_schema=SchemaRef( + type="object", properties={"count": {"type": "integer"}} + ), + output_schema=SchemaRef( + type="object", properties={"count": {"type": "integer"}} + ), + outcomes=["ok"], + ), + ], + start="again", + nodes=[ + ConditionNode.model_validate( + { + "id": "again", + "type": "condition", + "check": { + "op": "lt", + "left": {"path": "state.count"}, + "right": {"value": 2}, + }, + } + ), + ForeachNode.model_validate( + { + "id": "each", + "type": "foreach", + "over": "state.items", + "as": "item", + "mode": "serial", + } + ), + NodeUse.model_validate( + { + "id": "work", + "type": "node", + "node": "record", + "input": [{"target": "value", "path": "context.item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + NodeUse.model_validate( + { + "id": "bump", + "type": "node", + "node": "bump", + "input": [{"target": "count", "path": "state.count"}], + "output": [{"source": "count", "target": "state.count"}], + } + ), + ], + edges=[ + Edge.model_validate({"from": "again", "outcome": "true", "to": "each"}), + Edge.model_validate({"from": "each", "outcome": "loop", "to": "work"}), + Edge.model_validate({"from": "work", "outcome": "ok", "to": "each"}), + Edge.model_validate({"from": "each", "outcome": "done", "to": "bump"}), + Edge.model_validate({"from": "bump", "outcome": "ok", "to": "again"}), + Edge.model_validate({"from": "again", "outcome": "false", "to": END}), + ], + ) + + def bump(payload: dict[str, Any], _ctx: object) -> dict[str, Any]: + count = payload.get("count", 0) + assert isinstance(count, int) + return {"outcome": "ok", "output": {"count": count + 1}} + + run = execute_workflow( + workflow, + {"items": ["a"]}, + { + "record": lambda payload, _ctx: { + "outcome": "ok", + "output": {"seen": payload["value"]}, + }, + "bump": bump, + }, + ) + + assert run.status == RunStatus.COMPLETED + assert run.state["seen"] == ["a", "a"] + item_frames = [ + frame for frame in run.frames.values() if frame.kind == "foreach_iteration" + ] + assert len(item_frames) == 2 + owners = [item_frame_owner(frame) for frame in item_frames] + assert all(owner is not None for owner in owners) + assert owners[0] is not None and owners[1] is not None + assert owners[0].activation_id != owners[1].activation_id + assert item_frames[0].id != item_frames[1].id + # Both visits run item zero, but activation-qualified frame ids differ. + assert all(frame.id.endswith(":0") for frame in item_frames) + + +def test_subgraph_end_returns_to_subgraph_node_then_foreach_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": "serial", + } + ) + workflow = Workflow( + name="foreach_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 run.state["seen"] == ["a", "b"] + + +def test_nonlocal_runtime_return_fails_closed_when_validation_is_bypassed() -> None: + run = RunState( + workflow_name="nonlocal", + status=RunStatus.RUNNING, + workflow_input={}, + state={}, + frames={}, + ) + add_frame( + run, + ExecutionFrame(id="root", kind="workflow", node_id="outer"), + ) + add_frame( + run, + ExecutionFrame( + id="outer-item", + kind="foreach_iteration", + node_id="inner", + parent_frame_id="root", + metadata={ + "foreach_node_id": "outer", + "activation_id": "root:outer#0", + "loop_index": 0, + "loop_item": "a", + "loop_alias": "outer_item", + }, + ), + ) + add_frame( + run, + ExecutionFrame( + id="inner-item", + kind="foreach_iteration", + node_id="work", + parent_frame_id="outer-item", + metadata={ + "foreach_node_id": "inner", + "activation_id": "outer-item:inner#0", + "loop_index": 0, + "loop_item": 1, + "loop_alias": "inner_item", + }, + ), + ) + run.current_frame_id = "inner-item" + run.sync_from_current_frame() + + from wf_core.runtime.ops.flow import advance_frame + + with pytest.raises(WorkflowExecutionError, match="non-local|ancestor|immediate"): + advance_frame(run, run.frames["inner-item"], outcome="ok", next_node_id="outer") + + +def test_completed_activation_cannot_consume_later_activation_result_or_wake() -> None: + """A closed visit rejects buffered results and wake-ups from other visits.""" + from wf_core.runtime.foreach_state import ( + close_foreach_activation, + load_or_begin_foreach_activation, + require_foreach_activation, + ) + from wf_core.runtime.scheduler import ( + block_frame_on_children, + wake_parent_for_child_progress, + ) + + parent = ExecutionFrame(id="root", kind="workflow", node_id="each") + first = load_or_begin_foreach_activation(parent, "each", mode="concurrent") + first.barrier.next_index = 1 + first.barrier.start_child(f"{first.id}:0") + from wf_core.runtime.foreach_state import save_foreach_activation + + save_foreach_activation(parent, first) + close_foreach_activation(parent, first) + second = load_or_begin_foreach_activation(parent, "each", mode="concurrent") + save_foreach_activation(parent, second) + + assert second.id != first.id + + with pytest.raises(WorkflowExecutionError, match="closed|superseded"): + require_foreach_activation(parent, "each", first.id) + + run = RunState( + workflow_name="activation_isolation", + status=RunStatus.RUNNING, + workflow_input={}, + state={}, + frames={parent.id: parent}, + ) + stale_child = ExecutionFrame( + id=f"{first.id}:0", + kind="foreach_iteration", + node_id="work", + parent_frame_id="root", + metadata={ + "foreach_node_id": "each", + "activation_id": first.id, + "loop_index": 0, + "loop_item": "a", + "loop_alias": "item", + }, + ) + run.frames[stale_child.id] = stale_child + block_frame_on_children(run, "root", (stale_child.id,)) + stale_child.status = FrameStatus.COMPLETED + with pytest.raises(WorkflowExecutionError, match="closed activation"): + wake_parent_for_child_progress(run, stale_child.id) From b073407441e73b05e551a54deda9f21bc216f138 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 07:46:29 +0700 Subject: [PATCH 05/15] feat: validate foreach control regions --- src/wf_core/validation/core.py | 14 ++++ src/wf_core/validation/issues.py | 6 ++ tests/artifacts/test_draft_adapter.py | 8 ++- tests/artifacts/test_draft_models.py | 18 ++++-- tests/core/test_foreach_control_regions.py | 74 ++++++++++++++++++++++ tests/core/test_foreach_policy.py | 17 +++-- 6 files changed, 123 insertions(+), 14 deletions(-) diff --git a/src/wf_core/validation/core.py b/src/wf_core/validation/core.py index fb5462fa..2dfdce88 100644 --- a/src/wf_core/validation/core.py +++ b/src/wf_core/validation/core.py @@ -25,6 +25,14 @@ from wf_core.validation.steps import ( def validate_workflow(workflow: Workflow) -> ValidationReport: + """Coordinate structural validation including foreach control regions. + + Ordinary node/edge checks run first; the pure control-region analysis runs + once afterwards and its diagnostics are translated verbatim. No second + graph traversal lives inside validation. + """ + from wf_core.analysis.control_regions import analyze_control_regions + report = ValidationReport() node_defs = _collect_node_defs(workflow, report) @@ -33,6 +41,12 @@ def validate_workflow(workflow: Workflow) -> ValidationReport: _validate_start(workflow, nodes_by_id, report) outgoing = _validate_edges(workflow, nodes_by_id, node_defs, report) _validate_reachable_outcomes(workflow, nodes_by_id, node_defs, outgoing, report) + for issue in analyze_control_regions(workflow).issues: + report.add( + ValidationIssueCode(issue.kind.value), + issue.path, + issue.message, + ) return report diff --git a/src/wf_core/validation/issues.py b/src/wf_core/validation/issues.py index ae4fc3a9..c94afffc 100644 --- a/src/wf_core/validation/issues.py +++ b/src/wf_core/validation/issues.py @@ -26,6 +26,12 @@ class ValidationIssueCode(StrEnum): INVALID_FOREACH_COLLECT_DESTINATION = "invalid_foreach_collect_destination" INVALID_INTERRUPT_SOURCE = "invalid_interrupt_source" INVALID_INTERRUPT_DESTINATION = "invalid_interrupt_destination" + UNREACHABLE_NODE = "unreachable_node" + FOREACH_REGION_CONFLICT = "foreach_region_conflict" + INVALID_FOREACH_RETURN = "invalid_foreach_return" + INVALID_FOREACH_TERMINAL = "invalid_foreach_terminal" + EMPTY_FOREACH_BODY = "empty_foreach_body" + FOREACH_BODY_NO_RETURN = "foreach_body_no_return" @dataclass(slots=True) diff --git a/tests/artifacts/test_draft_adapter.py b/tests/artifacts/test_draft_adapter.py index 3f9d3b63..9422929a 100644 --- a/tests/artifacts/test_draft_adapter.py +++ b/tests/artifacts/test_draft_adapter.py @@ -505,14 +505,16 @@ def test_adapter_lowers_foreach_policy_through_builder() -> None: "collect_to": "state.item_errors", }, } - } + }, + "echo": {"use": "demo.echo"}, }, "routes": { "each_item": { - "loop": "__end__", + "loop": "echo", "done": "__end__", "completed_with_errors": "__end__", - } + }, + "echo": {"ok": "each_item"}, }, } ) diff --git a/tests/artifacts/test_draft_models.py b/tests/artifacts/test_draft_models.py index b839d5dd..22d2b5c5 100644 --- a/tests/artifacts/test_draft_models.py +++ b/tests/artifacts/test_draft_models.py @@ -322,7 +322,7 @@ def test_workflow_draft_foreach_over_dumps_structural_path() -> None: }, "routes": { "each_item": {"loop": "echo", "done": "__end__"}, - "echo": {"ok": "__end__"}, + "echo": {"ok": "each_item"}, }, } ) @@ -338,6 +338,7 @@ def test_workflow_draft_foreach_accepts_canonical_item_error_policy() -> None: **_keyed_echo_draft(), "start": "each_item", "steps": { + **_keyed_echo_draft()["steps"], "each_item": { "foreach": { "over": "state.items", @@ -349,9 +350,12 @@ def test_workflow_draft_foreach_accepts_canonical_item_error_policy() -> None: "collect_to": "state.item_errors", }, } - } + }, + }, + "routes": { + "each_item": {"loop": "echo", "done": "__end__"}, + "echo": {"ok": "each_item"}, }, - "routes": {"each_item": {"loop": "__end__", "done": "__end__"}}, } ) @@ -372,15 +376,19 @@ def test_workflow_draft_foreach_accepts_item_error_action_string() -> None: **_keyed_echo_draft(), "start": "each_item", "steps": { + **_keyed_echo_draft()["steps"], "each_item": { "foreach": { "over": "state.items", "as": "item", "item_error": "skip", } - } + }, + }, + "routes": { + "each_item": {"loop": "echo", "done": "__end__"}, + "echo": {"ok": "each_item"}, }, - "routes": {"each_item": {"loop": "__end__", "done": "__end__"}}, } ) diff --git a/tests/core/test_foreach_control_regions.py b/tests/core/test_foreach_control_regions.py index b3844f2f..e81110e8 100644 --- a/tests/core/test_foreach_control_regions.py +++ b/tests/core/test_foreach_control_regions.py @@ -6,6 +6,7 @@ from wf_core.analysis.control_regions import ( ControlRegionIssueKind, analyze_control_regions, ) +from wf_core.validation.issues import ValidationIssueCode def _workflow( @@ -56,6 +57,21 @@ def _condition(node_id: str) -> dict[str, object]: } +_CONTROL_REGION_CODES = {code.value for code in ControlRegionIssueKind} + + +def _public_control_errors(workflow: Workflow) -> list[tuple[str, str]]: + return [ + (issue.code.value, issue.path) + for issue in workflow.validate_structure().errors + if issue.code.value in _CONTROL_REGION_CODES + ] + + +def _assert_no_public_control_errors(workflow: Workflow) -> None: + assert _public_control_errors(workflow) == [] + + def test_closed_root_cycle_has_one_empty_control_region() -> None: workflow = _workflow( start="a", @@ -70,6 +86,7 @@ def test_closed_root_cycle_has_one_empty_control_region() -> None: assert analysis.issues == () assert analysis.owner_stack_by_node == {"a": (), "b": ()} + _assert_no_public_control_errors(workflow) def test_foreach_cycle_with_possible_return_is_valid() -> None: @@ -89,6 +106,7 @@ def test_foreach_cycle_with_possible_return_is_valid() -> None: assert analysis.issues == () assert analysis.owner_stack_by_node["a"] == ("f",) assert analysis.owner_stack_by_node["f"] == () + _assert_no_public_control_errors(workflow) def test_conditional_foreach_paths_can_both_return() -> None: @@ -109,6 +127,7 @@ def test_conditional_foreach_paths_can_both_return() -> None: assert analysis.issues == () assert analysis.owner_stack_by_node["condition"] == ("f",) assert analysis.owner_stack_by_node["work"] == ("f",) + _assert_no_public_control_errors(workflow) def test_nested_foreach_assigns_static_owner_stacks() -> None: @@ -142,6 +161,7 @@ def test_nested_foreach_assigns_static_owner_stacks() -> None: "after": (), } assert analysis.issues == () + _assert_no_public_control_errors(workflow) def test_reentering_completed_foreach_keeps_one_static_region() -> None: @@ -163,6 +183,7 @@ def test_reentering_completed_foreach_keeps_one_static_region() -> None: assert analysis.owner_stack_by_node["f"] == () assert analysis.owner_stack_by_node["work"] == ("f",) assert analysis.owner_stack_by_node["again"] == () + _assert_no_public_control_errors(workflow) def test_external_entry_into_foreach_body_is_region_conflict() -> None: @@ -184,6 +205,12 @@ def test_external_entry_into_foreach_body_is_region_conflict() -> None: (issue.kind, issue.path) for issue in analysis.issues ] assert "b" not in analysis.owner_stack_by_node + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.FOREACH_REGION_CONFLICT + ] + assert matching[0].path == "nodes[b]" def test_foreach_body_escape_is_region_conflict() -> None: @@ -204,6 +231,12 @@ def test_foreach_body_escape_is_region_conflict() -> None: (issue.kind, issue.path) for issue in analysis.issues ] assert "after" not in analysis.owner_stack_by_node + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.FOREACH_REGION_CONFLICT + ] + assert matching[0].path == "nodes[after]" def test_skipping_inner_foreach_owner_is_invalid_return() -> None: @@ -224,6 +257,12 @@ def test_skipping_inner_foreach_owner_is_invalid_return() -> None: assert (ControlRegionIssueKind.INVALID_FOREACH_RETURN, "edges[2]") in [ (issue.kind, issue.path) for issue in analysis.issues ] + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.INVALID_FOREACH_RETURN + ] + assert matching[0].path == "edges[2]" def test_entering_sibling_foreach_body_is_region_conflict() -> None: @@ -245,6 +284,12 @@ def test_entering_sibling_foreach_body_is_region_conflict() -> None: assert (ControlRegionIssueKind.FOREACH_REGION_CONFLICT, "nodes[b2]") in [ (issue.kind, issue.path) for issue in analysis.issues ] + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.FOREACH_REGION_CONFLICT + ] + assert matching[0].path == "nodes[b2]" def test_empty_foreach_body_is_rejected() -> None: @@ -262,6 +307,12 @@ def test_empty_foreach_body_is_rejected() -> None: assert (ControlRegionIssueKind.EMPTY_FOREACH_BODY, "edges[0]") in [ (issue.kind, issue.path) for issue in analysis.issues ] + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.EMPTY_FOREACH_BODY + ] + assert matching[0].path == "edges[0]" def test_closed_foreach_body_cycle_has_no_return() -> None: @@ -281,6 +332,12 @@ def test_closed_foreach_body_cycle_has_no_return() -> None: assert (ControlRegionIssueKind.FOREACH_BODY_NO_RETURN, "nodes[f]") in [ (issue.kind, issue.path) for issue in analysis.issues ] + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.FOREACH_BODY_NO_RETURN + ] + assert matching[0].path == "nodes[f]" def test_foreach_body_cannot_target_end_token() -> None: @@ -299,6 +356,12 @@ def test_foreach_body_cannot_target_end_token() -> None: assert (ControlRegionIssueKind.INVALID_FOREACH_TERMINAL, "edges[1]") in [ (issue.kind, issue.path) for issue in analysis.issues ] + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.INVALID_FOREACH_TERMINAL + ] + assert matching[0].path == "edges[1]" def test_foreach_body_cannot_target_explicit_end_node() -> None: @@ -321,6 +384,12 @@ def test_foreach_body_cannot_target_explicit_end_node() -> None: assert (ControlRegionIssueKind.INVALID_FOREACH_TERMINAL, "edges[1]") in [ (issue.kind, issue.path) for issue in analysis.issues ] + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.INVALID_FOREACH_TERMINAL + ] + assert matching[0].path == "edges[1]" def test_every_unreachable_node_is_reported() -> None: @@ -345,3 +414,8 @@ def test_every_unreachable_node_is_reported() -> None: ControlRegionIssueKind.UNREACHABLE_NODE, "nodes[detached_b]", ) in by_kind_path + public_by_code_path = [ + (issue.code.value, issue.path) for issue in workflow.validate_structure().errors + ] + assert ("unreachable_node", "nodes[detached_a]") in public_by_code_path + assert ("unreachable_node", "nodes[detached_b]") in public_by_code_path diff --git a/tests/core/test_foreach_policy.py b/tests/core/test_foreach_policy.py index ff752612..ac396605 100644 --- a/tests/core/test_foreach_policy.py +++ b/tests/core/test_foreach_policy.py @@ -137,16 +137,19 @@ def test_collect_policy_requires_completed_with_errors_edge() -> None: workflow = _workflow( item_error={"action": "collect", "collect_to": "state.item_errors"}, edges=[ - {"from": "each", "outcome": "loop", "to": END}, + {"from": "each", "outcome": "loop", "to": "body"}, + {"from": "body", "outcome": "ok", "to": "each"}, {"from": "each", "outcome": "done", "to": END}, ], ) report = validate_workflow(workflow) - assert report.errors - assert report.errors[0].code == "missing_outcome_edge" - assert "completed_with_errors" in report.errors[0].message + matching = [ + issue for issue in report.errors if issue.code == "missing_outcome_edge" + ] + assert matching + assert "completed_with_errors" in matching[0].message def test_collect_policy_destination_must_be_declared_array_field() -> None: @@ -199,11 +202,13 @@ def _workflow( "over": "state.items", "as": "item", "item_error": item_error or {"action": "fail"}, - } + }, + {"id": "body", "type": "node", "node": "noop"}, ], "edges": edges or [ - {"from": "each", "outcome": "loop", "to": END}, + {"from": "each", "outcome": "loop", "to": "body"}, + {"from": "body", "outcome": "ok", "to": "each"}, {"from": "each", "outcome": "done", "to": END}, ], "node_defs": [], From 7a5635b1a2a91507dcbce6f54aa0f3cf9ae4e819 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 08:04:47 +0700 Subject: [PATCH 06/15] fix: keep validation contracts truthful for new control regions --- src/wf_core/analysis/control_regions.py | 24 ++++++++++++++++++++++++ tests/wf_client/test_authoring.py | 8 +++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/wf_core/analysis/control_regions.py b/src/wf_core/analysis/control_regions.py index 81193aad..05bf4795 100644 --- a/src/wf_core/analysis/control_regions.py +++ b/src/wf_core/analysis/control_regions.py @@ -142,6 +142,30 @@ def analyze_control_regions(workflow: Workflow) -> ControlRegionAnalysis: continue is_terminal = target_id == END or isinstance(target_node, EndNode) if is_terminal: + # Explicit end nodes are still program locations with one + # static region; record them so they are not also reported as + # unreachable. The `END` token has no node to record. + if isinstance(target_node, EndNode): + visited_nodes.add(target_id) + recorded_target = owner_stack_by_node.get(target_id) + if recorded_target is None: + owner_stack_by_node[target_id] = target_stack + elif recorded_target != target_stack: + del owner_stack_by_node[target_id] + if target_id not in conflicted: + conflicted.add(target_id) + issues.append( + ControlRegionIssue( + kind=ControlRegionIssueKind.FOREACH_REGION_CONFLICT, + path=f"nodes[{target_id}]", + message=( + f"node {target_id!r} is reachable under " + "two foreach control regions" + ), + ) + ) + for prior_stack in (recorded_target, target_stack): + mark_ambiguous(prior_stack) if target_stack: issues.append( ControlRegionIssue( diff --git a/tests/wf_client/test_authoring.py b/tests/wf_client/test_authoring.py index 19980231..c4b70466 100644 --- a/tests/wf_client/test_authoring.py +++ b/tests/wf_client/test_authoring.py @@ -272,7 +272,13 @@ async def test_snapshotless_remote_node_upgrades_to_real_capability_contract() - replacement = graph.use(capability, id="replacement", input=[], output=[]) graph.connect(replacement, "ok", "done") - assert graph.validate_local().ok is True + # A disconnected replacement has no derivable control region; new + # validation reports it as unreachable rather than silently accepting it. + report = graph.validate_local() + assert any( + issue.code == "unreachable_node" and issue.path == "nodes[replacement]" + for issue in report.errors + ) assert graph.seeded_node_defs["app.default.remote"].input_schema.properties == { "query": {"type": "string"} } From 8a550f7c69f25eaaa5d6905fe0fef06ac3f0f26b Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 08:13:24 +0700 Subject: [PATCH 07/15] docs: publish foreach back-edge semantics --- docs/current_roadmap.md | 11 +++++------ .../specs/2026-09-04-foreach-back-edge-design.md | 6 +++--- docs/wf_authoring_control_flow.md | 11 +++++++++++ docs/wf_core_architecture.md | 10 +++++++--- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/docs/current_roadmap.md b/docs/current_roadmap.md index 27578115..68d9cff3 100644 --- a/docs/current_roadmap.md +++ b/docs/current_roadmap.md @@ -795,12 +795,6 @@ stable. - Native subgraph polish: optional per-use-site child deployment overrides and clearer child trace inspection. -- Active concurrent foreach correction: replace item-body `END` routes with - canonical back-edges to the owning foreach. The approved semantics are in the - [`foreach back-edge design`](superpowers/specs/2026-09-04-foreach-back-edge-design.md). - This slice also makes foreach control regions fail-closed, rejects unreachable - workflow nodes, and gives repeated visits to one foreach node distinct - persisted activation identities. - After that correction, reuse the foreach barrier/lineage machinery for fork/gather. The proposed control semantics are recorded in [`ADR-0006`](adr/0006-explicit-fork-and-topology-driven-gather.md). @@ -942,6 +936,11 @@ stable. routes, or outputs. Python, JSON-RPC, MCP, and local/remote CLI surfaces are aligned. Implementation plan: [`capability step updates`](historical/superpowers/plans/2026-07-26-capability-step-update.md). +- Completed: foreach bodies now return through validated back-edges to their + immediate owner, with unique static control regions and fresh persisted + activation identities for every dynamic visit. Design: + [`foreach back-edge design`](superpowers/specs/2026-09-04-foreach-back-edge-design.md). + Fork/gather remains explicitly deferred. Agent evaluation cohort status and policy: 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 aeeb58da..f6673a96 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 @@ -2,9 +2,9 @@ ## Status -Approved in conversation on 2026-09-04. This document specifies canonical -foreach body-return semantics. It does not include the separately planned -ergonomic Python DSL or authorize fork/gather implementation. +Implemented on 2026-09-04. This document specifies canonical foreach +body-return semantics. It does not include the separately planned ergonomic +Python DSL or authorize fork/gather implementation. ## Purpose diff --git a/docs/wf_authoring_control_flow.md b/docs/wf_authoring_control_flow.md index d3ff1586..881453a5 100644 --- a/docs/wf_authoring_control_flow.md +++ b/docs/wf_authoring_control_flow.md @@ -228,6 +228,17 @@ step with item-local child lineages: In async execution, admitted async item node handlers may run at the same time. Run-state mutation, tracing, and barrier commits remain deterministic. +An iteration body returns through its immediate owning foreach: + +```python +g.connect(each, "loop", record) +g.connect(record, "ok", each) +g.connect(each, "done", END) +``` + +Region conflicts, unreachable nodes, body terminals, non-local returns, empty +bodies, and bodies without possible returns fail validation. + See `examples/authoring_concurrent_foreach.py` for a runnable example covering: - sync concurrent foreach with `item_error={"action": "collect", ...}`; diff --git a/docs/wf_core_architecture.md b/docs/wf_core_architecture.md index 92ac9761..156efd01 100644 --- a/docs/wf_core_architecture.md +++ b/docs/wf_core_architecture.md @@ -74,9 +74,10 @@ interrupted, failed, or deadlocked. This replaces the older assumption that ## Foreach Serial foreach creates one iteration child frame, records typed -`ForeachIterationMetadata`, blocks on that child, and enqueues the child. When -the child reaches `END`, `wake_parent_if_children_complete` wakes the blocked -parent so it can create the next iteration or emit `done`. +`ForeachIterationMetadata`, blocks on that child, and enqueues the child. An +item child returns by targeting its immediate owning foreach. The child +finishes at that owner location without executing the controller; the parent +activation consumes the result and continues or completes its barrier. Concurrent foreach uses the same frame machinery but admits multiple item lineages according to `ForeachConcurrentPolicy`. Each item lineage reads through @@ -98,6 +99,9 @@ See `examples/raw_concurrent_foreach.py` for the canonical raw workflow shape an - validate edge sources, destinations, duplicate outcomes, and declared outcomes - validate reachable nodes have all required outcome edges - validate explicit `EndNode` outcomes against `Workflow.outcomes` +- validate foreach control regions once: region conflicts, unreachable nodes, + body terminals, non-local returns, empty bodies, and bodies without possible + returns fail validation Validation reports multiple issues through `ValidationReport` instead of raising at the first failure. From be583661cfd50a133721cfc715bc8e1130dc3b15 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 08:14:28 +0700 Subject: [PATCH 08/15] docs: archive foreach back-edge plan --- .../superpowers/plans/2026-09-04-foreach-back-edges.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/{ => historical}/superpowers/plans/2026-09-04-foreach-back-edges.md (100%) diff --git a/docs/superpowers/plans/2026-09-04-foreach-back-edges.md b/docs/historical/superpowers/plans/2026-09-04-foreach-back-edges.md similarity index 100% rename from docs/superpowers/plans/2026-09-04-foreach-back-edges.md rename to docs/historical/superpowers/plans/2026-09-04-foreach-back-edges.md From 79ce0d3effe32012864b6abc4c238cc9857de4f4 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 10:48:29 +0700 Subject: [PATCH 09/15] audit: close review gaps, dedupe control-region and activation helpers --- src/wf_core/analysis/context_scopes.py | 3 +- src/wf_core/analysis/control_regions.py | 53 ++++++++++------------ src/wf_core/runtime/foreach_state.py | 32 ++++++++----- src/wf_core/runtime/ops/foreach.py | 6 +-- tests/core/test_foreach_back_edges.py | 26 +++++++++++ tests/core/test_foreach_control_regions.py | 25 ++++++++++ 6 files changed, 100 insertions(+), 45 deletions(-) diff --git a/src/wf_core/analysis/context_scopes.py b/src/wf_core/analysis/context_scopes.py index 87bc8529..1d2b23ce 100644 --- a/src/wf_core/analysis/context_scopes.py +++ b/src/wf_core/analysis/context_scopes.py @@ -145,7 +145,8 @@ def _available_fields( A single node use has one control region, so foreach fields are either present (inside a body) or absent (outside). Conditional availability is - not used to represent multiple owner stacks. + not used to represent multiple owner stacks: a node reached under two + stacks is a region conflict and receives no foreach fields at all. """ contracts = list(STANDARD_CONTEXT_FIELDS) if active_scope is not None: diff --git a/src/wf_core/analysis/control_regions.py b/src/wf_core/analysis/control_regions.py index 05bf4795..8b8063ba 100644 --- a/src/wf_core/analysis/control_regions.py +++ b/src/wf_core/analysis/control_regions.py @@ -68,6 +68,26 @@ def analyze_control_regions(workflow: Workflow) -> ControlRegionAnalysis: if stack: ambiguous_tops.add(stack[-1]) + def record_region_conflict( + node_id: str, stacks: tuple[ForeachOwnerStack, ...] + ) -> None: + """Drop one node use reached under two regions and report it once.""" + del owner_stack_by_node[node_id] + if node_id not in conflicted: + conflicted.add(node_id) + issues.append( + ControlRegionIssue( + kind=ControlRegionIssueKind.FOREACH_REGION_CONFLICT, + path=f"nodes[{node_id}]", + message=( + f"node {node_id!r} is reachable under two foreach " + "control regions" + ), + ) + ) + for prior_stack in stacks: + mark_ambiguous(prior_stack) + def add_adjacency( source: tuple[str, ForeachOwnerStack], target: tuple[str, ForeachOwnerStack], @@ -99,20 +119,7 @@ def analyze_control_regions(workflow: Workflow) -> ControlRegionAnalysis: # single static owner stack. Drop it so later context analysis # grants no foreach fields, and stop expanding this ambiguous # state so the conflict does not cascade. - del owner_stack_by_node[node_id] - conflicted.add(node_id) - issues.append( - ControlRegionIssue( - kind=ControlRegionIssueKind.FOREACH_REGION_CONFLICT, - path=f"nodes[{node_id}]", - message=( - f"node {node_id!r} is reachable under two foreach " - "control regions" - ), - ) - ) - for prior_stack in (recorded, stack): - mark_ambiguous(prior_stack) + record_region_conflict(node_id, (recorded, stack)) continue for edge_index, edge in edges_by_node.get(node_id, []): # type: ignore[attr-defined] @@ -151,21 +158,9 @@ def analyze_control_regions(workflow: Workflow) -> ControlRegionAnalysis: if recorded_target is None: owner_stack_by_node[target_id] = target_stack elif recorded_target != target_stack: - del owner_stack_by_node[target_id] - if target_id not in conflicted: - conflicted.add(target_id) - issues.append( - ControlRegionIssue( - kind=ControlRegionIssueKind.FOREACH_REGION_CONFLICT, - path=f"nodes[{target_id}]", - message=( - f"node {target_id!r} is reachable under " - "two foreach control regions" - ), - ) - ) - for prior_stack in (recorded_target, target_stack): - mark_ambiguous(prior_stack) + record_region_conflict( + target_id, (recorded_target, target_stack) + ) if target_stack: issues.append( ControlRegionIssue( diff --git a/src/wf_core/runtime/foreach_state.py b/src/wf_core/runtime/foreach_state.py index 6aa8236a..eed386db 100644 --- a/src/wf_core/runtime/foreach_state.py +++ b/src/wf_core/runtime/foreach_state.py @@ -345,6 +345,20 @@ class ForeachBarrierState: ) +def _activation_entry( + frame: ExecutionFrame, table: dict[str, Any], foreach_node_id: str +) -> dict[str, Any] | None: + """Return the mutable activation entry or fail fast on corrupt state.""" + entry = table.get(foreach_node_id) + if entry is None: + return None + if not isinstance(entry, dict): + raise WorkflowExecutionError( + f"malformed foreach activation entry for frame {frame.id!r}" + ) + return entry + + def load_or_begin_foreach_activation( frame: ExecutionFrame, foreach_node_id: str, @@ -359,14 +373,10 @@ def load_or_begin_foreach_activation( with fresh barrier state. Mode mismatches and malformed tables fail fast. """ table = _activation_table(frame) - entry = table.get(foreach_node_id) + entry = _activation_entry(frame, table, foreach_node_id) if entry is None: entry = {"next_sequence": 0, "active": None} table[foreach_node_id] = entry - if not isinstance(entry, dict): - raise WorkflowExecutionError( - f"malformed foreach activation entry for frame {frame.id!r}" - ) next_sequence = entry.get("next_sequence", 0) if not isinstance(next_sequence, int) or next_sequence < 0: raise WorkflowExecutionError( @@ -402,8 +412,8 @@ def save_foreach_activation( ) -> None: """Persist barrier progress for the named active activation.""" table = _activation_table(frame) - entry = table.get(activation.foreach_node_id) - if not isinstance(entry, dict): + entry = _activation_entry(frame, table, activation.foreach_node_id) + if entry is None: raise WorkflowExecutionError( f"malformed foreach activation entry for frame {frame.id!r}" ) @@ -425,8 +435,8 @@ def close_foreach_activation( increasing so child and lineage ids cannot collide across visits. """ table = _activation_table(frame) - entry = table.get(activation.foreach_node_id) - if not isinstance(entry, dict): + entry = _activation_entry(frame, table, activation.foreach_node_id) + if entry is None: raise WorkflowExecutionError( f"malformed foreach activation entry for frame {frame.id!r}" ) @@ -448,8 +458,8 @@ def load_foreach_activation( the caller rather than buffering into the wrong barrier. """ table = _activation_table(frame) - entry = table.get(foreach_node_id) - if not isinstance(entry, dict): + entry = _activation_entry(frame, table, foreach_node_id) + if entry is None: raise WorkflowExecutionError( f"malformed foreach activation entry for frame {frame.id!r}" ) diff --git a/src/wf_core/runtime/ops/foreach.py b/src/wf_core/runtime/ops/foreach.py index dec9178b..99c0c972 100644 --- a/src/wf_core/runtime/ops/foreach.py +++ b/src/wf_core/runtime/ops/foreach.py @@ -170,7 +170,6 @@ def _step_foreach_concurrent( step=step, index=index, activation=activation, - barrier=barrier, iterable=iterable, ) @@ -182,7 +181,6 @@ def _step_foreach_concurrent( step=step, index=index, activation=activation, - barrier=barrier, reducers=reducers, ) @@ -260,12 +258,12 @@ def _admit_concurrent_children( step: ForeachNode, index: WorkflowIndex, activation: ForeachActivationState, - barrier: ForeachBarrierState, iterable: list[object], ) -> None: if step.concurrent is None: raise WorkflowExecutionError("concurrent foreach requires concurrent policy") + barrier = activation.barrier loop_start = index.next_node_id(frame.node_id, "loop") while ( barrier.next_index < len(iterable) @@ -333,9 +331,9 @@ def _finish_concurrent_foreach( step: ForeachNode, index: WorkflowIndex, activation: ForeachActivationState, - barrier: ForeachBarrierState, reducers: Mapping[str, ReducerDefinition] | None = None, ) -> RunState: + barrier = activation.barrier error_records = [ result.error.to_metadata() for result in sorted( diff --git a/tests/core/test_foreach_back_edges.py b/tests/core/test_foreach_back_edges.py index 8a2b969e..5512c60b 100644 --- a/tests/core/test_foreach_back_edges.py +++ b/tests/core/test_foreach_back_edges.py @@ -695,6 +695,32 @@ def test_nonlocal_runtime_return_fails_closed_when_validation_is_bypassed() -> N advance_frame(run, run.frames["inner-item"], outcome="ok", next_node_id="outer") +def test_root_frame_targeting_foreach_enters_normally() -> None: + """A root frame naming a foreach enters it; only an item frame returns.""" + from wf_core.runtime.ops.flow import advance_frame + + run = RunState( + workflow_name="root_entry", + status=RunStatus.RUNNING, + workflow_input={}, + state={}, + frames={}, + ) + add_frame( + run, + ExecutionFrame(id="root", kind="workflow", node_id="start"), + ) + run.current_frame_id = "root" + run.sync_from_current_frame() + + advance_frame(run, run.frames["root"], outcome="ok", next_node_id="each") + + entered = run.frames["root"] + assert entered.node_id == "each" + assert entered.status == FrameStatus.PENDING + assert entered.finished_at_node_id is None + + def test_completed_activation_cannot_consume_later_activation_result_or_wake() -> None: """A closed visit rejects buffered results and wake-ups from other visits.""" from wf_core.runtime.foreach_state import ( diff --git a/tests/core/test_foreach_control_regions.py b/tests/core/test_foreach_control_regions.py index e81110e8..c48469e5 100644 --- a/tests/core/test_foreach_control_regions.py +++ b/tests/core/test_foreach_control_regions.py @@ -265,6 +265,31 @@ def test_skipping_inner_foreach_owner_is_invalid_return() -> None: assert matching[0].path == "edges[2]" +def test_reentering_active_ancestor_foreach_as_nested_controller_is_invalid() -> None: + workflow = _workflow( + start="f1", + nodes=[_foreach("f1"), _foreach("f2")], + edges=[ + {"from": "f1", "outcome": "loop", "to": "f2"}, + {"from": "f2", "outcome": "loop", "to": "f1"}, + {"from": "f2", "outcome": "done", "to": "f1"}, + {"from": "f1", "outcome": "done", "to": END}, + ], + ) + + analysis = analyze_control_regions(workflow) + + assert (ControlRegionIssueKind.INVALID_FOREACH_RETURN, "edges[1]") in [ + (issue.kind, issue.path) for issue in analysis.issues + ] + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.INVALID_FOREACH_RETURN + ] + assert matching[0].path == "edges[1]" + + def test_entering_sibling_foreach_body_is_region_conflict() -> None: workflow = _workflow( start="f1", From f155e6651ac44f75511c1af09a4dad8319651de8 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 10:58:13 +0700 Subject: [PATCH 10/15] 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, + } + ) From 8a0737fe8b456284b79820b8e200b57ec23d1991 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 11:20:15 +0700 Subject: [PATCH 11/15] audit: unify item write routing, fix serial interrupt loss, enforce result coherence --- .../plans/2026-09-04-foreach-back-edges.md | 2 +- src/wf_core/runtime/foreach_state.py | 32 ++++++++-- src/wf_core/runtime/lineage.py | 39 +++++++++++ src/wf_core/runtime/ops/foreach.py | 19 ++++-- src/wf_core/runtime/ops/interrupts.py | 6 +- src/wf_core/runtime/ops/nodes.py | 38 ++--------- src/wf_core/runtime/subgraphs.py | 31 ++------- tests/core/test_foreach_back_edges.py | 64 +++++++++++++++++++ tests/core/test_foreach_barrier_state.py | 50 +++++++++++++++ 9 files changed, 206 insertions(+), 75 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 44831ced..a019c421 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 @@ -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`](../../superpowers/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 diff --git a/src/wf_core/runtime/foreach_state.py b/src/wf_core/runtime/foreach_state.py index 41fc86d6..21fc75d8 100644 --- a/src/wf_core/runtime/foreach_state.py +++ b/src/wf_core/runtime/foreach_state.py @@ -112,17 +112,32 @@ class PendingItemResult: f"malformed pending foreach result missing {exc.args[0]!r}" ) from exc lineage_id = raw.get("lineage_id") + raw_error = raw.get("error") 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") - raw_error = raw.get("error") + if status == "succeeded": + if not isinstance(lineage_id, str): + raise WorkflowExecutionError( + "malformed pending foreach result lineage id" + ) + if raw_error is not None: + raise WorkflowExecutionError( + "malformed pending foreach result: succeeded result must not " + "carry an error" + ) + else: + if raw_error is None: + raise WorkflowExecutionError( + "malformed pending foreach result: failed result requires an error" + ) + if lineage_id is not None and not isinstance(lineage_id, str): + raise WorkflowExecutionError( + "malformed pending foreach result lineage id" + ) return cls( index=index, frame_id=frame_id, @@ -178,7 +193,12 @@ class ForeachBarrierState: raise WorkflowExecutionError( "malformed foreach barrier pending result index" ) from exc - parsed_results[index] = PendingItemResult.from_metadata(raw_result) + parsed = PendingItemResult.from_metadata(raw_result) + if parsed.index != index: + raise WorkflowExecutionError( + "malformed foreach barrier pending result index mismatch" + ) + parsed_results[index] = parsed return cls( next_index=next_index, mode=mode, diff --git a/src/wf_core/runtime/lineage.py b/src/wf_core/runtime/lineage.py index 77c762aa..e9ade5b1 100644 --- a/src/wf_core/runtime/lineage.py +++ b/src/wf_core/runtime/lineage.py @@ -89,6 +89,45 @@ def commit_patch_for_frame( return {} +def commit_foreach_aware_patch( + run: RunState, frame: ExecutionFrame, patch: StatePatch +) -> dict[str, Any]: + """Commit one write patch with foreach-aware routing. + + Ordinary frames commit (or buffer) through their own lineage. Serial + item writes commit through the parent scope so they land in root state; + concurrent item writes stay buffered in the item lineage for the barrier + to merge. Malformed ownership, missing parents, and closed or + superseded activations fail closed. + """ + from wf_core.runtime.foreach_state import ( + item_frame_owner, + require_foreach_activation, + ) + + owner = item_frame_owner(frame) + if owner is None: + return commit_patch_for_frame(run, frame, patch) + 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}" + ) + activation = require_foreach_activation( + parent_frame, owner.foreach_node_id, owner.activation_id + ) + if activation.barrier.mode == "concurrent": + append_lineage_writes( + run, + scope_id=frame.scope_id, + lineage_id=frame.lineage_id, + writes=patch.writes, + ) + return {} + return commit_patch_for_frame(run, parent_frame, patch) + + def scope_state_for_frame(run: RunState, frame: ExecutionFrame) -> dict[str, Any]: """Return the committed state root for the frame's runtime scope.""" scope = run.scopes.get(frame.scope_id) diff --git a/src/wf_core/runtime/ops/foreach.py b/src/wf_core/runtime/ops/foreach.py index 87739b64..e94961a5 100644 --- a/src/wf_core/runtime/ops/foreach.py +++ b/src/wf_core/runtime/ops/foreach.py @@ -341,13 +341,18 @@ def _finish_concurrent_foreach( reducers: Mapping[str, ReducerDefinition] | None = None, ) -> RunState: barrier = activation.barrier - error_records = [ - result.error.to_metadata() - for result in sorted( - barrier.pending_results.values(), key=lambda item: item.index - ) - if result.status == "failed" and result.error is not None - ] + # Coherence is enforced at load, but re-check here: a failed result + # without an error must never silent-commit as `done`. + error_records = [] + for result in sorted(barrier.pending_results.values(), key=lambda item: item.index): + if result.status != "failed": + continue + if result.error is None: + raise WorkflowExecutionError( + f"foreach item result for index {result.index!r} is failed " + "but carries no error" + ) + error_records.append(result.error.to_metadata()) outcome = "completed_with_errors" if error_records else "done" next_node_id = index.next_node_id(frame.node_id, outcome) success_patches = [ diff --git a/src/wf_core/runtime/ops/interrupts.py b/src/wf_core/runtime/ops/interrupts.py index 3fd11c8b..d56c1e9c 100644 --- a/src/wf_core/runtime/ops/interrupts.py +++ b/src/wf_core/runtime/ops/interrupts.py @@ -14,7 +14,7 @@ from wf_core.run_state import ( StepExecutionResult, ) from wf_core.runtime.input_bindings import resolve_step_input_bindings -from wf_core.runtime.lineage import commit_patch_for_frame +from wf_core.runtime.lineage import commit_foreach_aware_patch from wf_core.runtime.ops.flow import advance_frame, append_step_result_trace from wf_core.runtime.ops.index import WorkflowIndex from wf_core.runtime.ops.merges import ReducerDefinition @@ -115,7 +115,9 @@ def resume_interrupt( reducers=reducers, missing_field_message="interrupt resume payload is missing required field {field}", ) - state_changes = commit_patch_for_frame(run, frame, patch) + # Foreach-aware routing: a serial item resume commits through the parent + # scope, a concurrent one buffers in the item lineage for barrier merge. + state_changes = commit_foreach_aware_patch(run, frame, patch) next_node_id = index.next_node_id(frame.node_id, resume_outcome) append_step_result_trace( run, diff --git a/src/wf_core/runtime/ops/nodes.py b/src/wf_core/runtime/ops/nodes.py index 6c73976b..199b4df8 100644 --- a/src/wf_core/runtime/ops/nodes.py +++ b/src/wf_core/runtime/ops/nodes.py @@ -15,14 +15,9 @@ from wf_core.run_state import ( RuntimeContext, StepExecutionResult, ) -from wf_core.runtime.foreach_state import ( - item_frame_owner, - require_foreach_activation, -) from wf_core.runtime.input_bindings import resolve_step_input_bindings from wf_core.runtime.lineage import ( - append_lineage_writes, - commit_patch_for_frame, + commit_foreach_aware_patch, scope_input_for_frame, ) from wf_core.runtime.ops.frames import frame_context_values @@ -115,33 +110,10 @@ def _finalize_node_execution( state_view, reducers=reducers, ) - owner = item_frame_owner(frame) - if owner is None: - state_changes = commit_patch_for_frame(run, frame, patch) - else: - 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 - ) - 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, - ) - state_changes = {} - else: - state_changes = commit_patch_for_frame(run, parent_frame, patch) + # Foreach-aware routing (root, serial parent, concurrent lineage) is + # owned by the shared helper so every operation commits the same way. + # Closed or superseded activations fail closed inside. + state_changes = commit_foreach_aware_patch(run, frame, patch) return StepExecutionResult( outcome=result.outcome, resolved_input=resolved_input, diff --git a/src/wf_core/runtime/subgraphs.py b/src/wf_core/runtime/subgraphs.py index 89870993..49daabb1 100644 --- a/src/wf_core/runtime/subgraphs.py +++ b/src/wf_core/runtime/subgraphs.py @@ -17,7 +17,7 @@ from wf_core.run_state import ( StepExecutionResult, ) from wf_core.runtime.input_bindings import resolve_step_input_bindings -from wf_core.runtime.lineage import commit_patch_for_frame +from wf_core.runtime.lineage import commit_foreach_aware_patch 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 @@ -236,31 +236,10 @@ def _finish_subgraph( reducers=reducers, missing_field_message="subgraph output did not include required field {field}", ) - # Match node execution: serial item writes commit through the parent - # scope so top-level serial subgraphs land in root state; concurrent - # item writes stay buffered in the item lineage for barrier merge. - from wf_core.runtime.foreach_state import ( - item_frame_owner, - require_foreach_activation, - ) - - commit_frame = frame - owner = item_frame_owner(frame) - if owner is not None: - parent_frame = run.frames.get(owner.parent_frame_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}" - ) - # 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) + # Foreach-aware routing (root, serial parent, concurrent lineage) is + # owned by the shared helper so subgraph output commits exactly like + # node output. Closed or superseded activations fail closed inside. + state_changes = commit_foreach_aware_patch(run, frame, patch) return StepExecutionResult( outcome=child_outcome, resolved_input=activation.child_input, diff --git a/tests/core/test_foreach_back_edges.py b/tests/core/test_foreach_back_edges.py index 800c53d1..fb6ba2ce 100644 --- a/tests/core/test_foreach_back_edges.py +++ b/tests/core/test_foreach_back_edges.py @@ -9,6 +9,7 @@ from wf_core import ( ConditionNode, Edge, ForeachNode, + InterruptNode, NodeDef, NodeUse, ReducerRef, @@ -19,6 +20,8 @@ from wf_core import ( Workflow, WorkflowExecutionError, execute_workflow, + execute_workflow_async, + resume_workflow_async, ) from wf_core.run_state import ExecutionFrame, FrameStatus, RunState, RunStatus from wf_core.runtime.foreach_state import item_frame_owner @@ -742,6 +745,67 @@ def test_concurrent_subgraph_item_returns_through_owner() -> None: assert sorted(run.state["seen"]) == ["a", "b"] +async def test_serial_interrupt_resume_commits_answer_to_parent_state() -> None: + """A serial item resume must land in parent state, not the child lineage.""" + foreach = ForeachNode.model_validate( + { + "id": "each", + "type": "foreach", + "over": "state.items", + "as": "item", + "mode": "serial", + } + ) + workflow = Workflow( + name="foreach_serial_interrupt", + input_schema=SchemaRef(type="object", properties={"items": {"type": "array"}}), + state_schema=StateSchema.from_field_map( + { + "items": StateField(type="array"), + "answers": StateField( + type="array", reducer=ReducerRef(name="wf.std.append") + ), + } + ), + output_schema=SchemaRef( + type="object", properties={"answers": {"type": "array"}} + ), + node_defs=[], + start="each", + nodes=[ + foreach, + InterruptNode.model_validate( + { + "id": "ask", + "type": "interrupt", + "kind": "approval", + "request": [{"target": "item", "path": "context.item"}], + "resume": [{"source": "answer", "target": "state.answers"}], + } + ), + ], + edges=[ + Edge.model_validate({"from": "each", "outcome": "loop", "to": "ask"}), + Edge.model_validate({"from": "ask", "outcome": "submitted", "to": "each"}), + Edge.model_validate({"from": "each", "outcome": "done", "to": END}), + ], + ) + + run = await execute_workflow_async(workflow, {"items": ["a", "b"]}, {}) + assert run.status == RunStatus.INTERRUPTED + + resumed = await resume_workflow_async( + workflow, run, {}, resume_payload={"answer": "a"} + ) + assert resumed.status == RunStatus.INTERRUPTED + + finished = await resume_workflow_async( + workflow, resumed, {}, resume_payload={"answer": "b"} + ) + assert finished.status == RunStatus.COMPLETED + assert finished.state["answers"] == ["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 1ac3a439..6df3e5ca 100644 --- a/tests/core/test_foreach_barrier_state.py +++ b/tests/core/test_foreach_barrier_state.py @@ -1,5 +1,7 @@ from __future__ import annotations +from copy import deepcopy + import pytest from wf_core.errors import WorkflowExecutionError @@ -290,3 +292,51 @@ def test_pending_item_result_requires_lineage_for_success() -> None: "lineage_id": None, } ) + + +def test_pending_item_result_rejects_error_on_success() -> None: + with pytest.raises(WorkflowExecutionError, match="must not carry an error"): + PendingItemResult.from_metadata( + { + "index": 0, + "frame_id": "child", + "status": "succeeded", + "lineage_id": "root:each#0[0]", + "error": { + "index": 0, + "frame_id": "child", + "node_id": "work", + "error_type": "ValueError", + "message": "bad", + }, + } + ) + + +def test_pending_item_result_requires_error_for_failure() -> None: + with pytest.raises(WorkflowExecutionError, match="requires an error"): + PendingItemResult.from_metadata( + { + "index": 0, + "frame_id": "child", + "status": "failed", + "lineage_id": None, + "error": None, + } + ) + + +def test_pending_item_result_rejects_index_key_mismatch() -> 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) + raw = deepcopy(frame.metadata["foreach_activations"]) + raw["each"]["active"]["barrier"]["pending_results"] = { + "7": raw["each"]["active"]["barrier"]["pending_results"]["0"] + } + + with pytest.raises(WorkflowExecutionError, match="index mismatch"): + ForeachBarrierState.from_metadata(raw["each"]["active"]["barrier"]) From 08cd4e1e5ef9db9b51177bc5730772ab415a2219 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 11:43:41 +0700 Subject: [PATCH 12/15] audit: walk serial owners in write routing, harden result identity --- .../2026-09-04-foreach-back-edge-design.md | 9 +- src/wf_core/runtime/foreach_state.py | 25 ++- src/wf_core/runtime/lineage.py | 59 +++--- src/wf_core/runtime/ops/foreach.py | 4 +- tests/core/test_foreach_back_edges.py | 168 ++++++++++++++++++ tests/core/test_foreach_barrier_state.py | 48 +++++ 6 files changed, 278 insertions(+), 35 deletions(-) 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 10180cca..1eb8b0ca 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 @@ -434,7 +434,14 @@ 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. The completed item is +accepted concurrent-foreach ADR and declared reducers. One shared helper +routes every item write: it climbs through each serial owner to the scope +root, where it commits, or stops at the first concurrent item boundary, +where it buffers for that barrier to merge (the concurrent barrier finish +routes its combined patch through the same helper, so nested serial owners +cannot strand it). Parent cycles, missing parents, and orphaned item frames +fail closed. Buffered failure records must carry an error whose index and +frame match the enclosing result. 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 diff --git a/src/wf_core/runtime/foreach_state.py b/src/wf_core/runtime/foreach_state.py index 21fc75d8..89dd5c54 100644 --- a/src/wf_core/runtime/foreach_state.py +++ b/src/wf_core/runtime/foreach_state.py @@ -138,16 +138,21 @@ class PendingItemResult: raise WorkflowExecutionError( "malformed pending foreach result lineage id" ) + error = ( + ItemErrorRecord.from_metadata(raw_error) if raw_error is not None else None + ) + if error is not None and (error.index != index or error.frame_id != frame_id): + raise WorkflowExecutionError( + "malformed pending foreach result: error identity " + f"(index {error.index!r}, frame {error.frame_id!r}) does not " + f"match enclosing result (index {index!r}, frame {frame_id!r})" + ) return cls( index=index, frame_id=frame_id, status=status, lineage_id=lineage_id, - error=( - ItemErrorRecord.from_metadata(raw_error) - if raw_error is not None - else None - ), + error=error, ) def to_metadata(self) -> dict[str, Any]: @@ -443,10 +448,16 @@ def item_frame_owner(frame: ExecutionFrame) -> ForeachItemOwner | None: """Return the named foreach ownership record for item frames. Malformed item metadata fails closed via ``ForeachIterationMetadata``; - only non-item frames return ``None``. + only genuinely non-item frames return ``None``. An item frame without + a parent is corrupt state and raises rather than masquerading as an + ordinary frame. """ - if frame.kind != "foreach_iteration" or frame.parent_frame_id is None: + if frame.kind != "foreach_iteration": return None + if frame.parent_frame_id is None: + raise WorkflowExecutionError( + f"foreach item frame {frame.id!r} is missing its parent frame" + ) metadata = ForeachIterationMetadata.from_frame(frame) if metadata is None: return None diff --git a/src/wf_core/runtime/lineage.py b/src/wf_core/runtime/lineage.py index e9ade5b1..fb49ebe3 100644 --- a/src/wf_core/runtime/lineage.py +++ b/src/wf_core/runtime/lineage.py @@ -94,38 +94,47 @@ def commit_foreach_aware_patch( ) -> dict[str, Any]: """Commit one write patch with foreach-aware routing. - Ordinary frames commit (or buffer) through their own lineage. Serial - item writes commit through the parent scope so they land in root state; - concurrent item writes stay buffered in the item lineage for the barrier - to merge. Malformed ownership, missing parents, and closed or - superseded activations fail closed. + Ordinary frames commit (or buffer) through their own lineage. The walk + climbs through every serial item owner until it reaches either the + workflow/subgraph scope root, where it commits, or a concurrent item + boundary, where it buffers in that item lineage for the barrier to + merge. Malformed ownership, missing parents, parent cycles, and closed + or superseded activations fail closed. """ from wf_core.runtime.foreach_state import ( item_frame_owner, require_foreach_activation, ) - owner = item_frame_owner(frame) - if owner is None: - return commit_patch_for_frame(run, frame, patch) - 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}" + current = frame + seen: set[str] = set() + while True: + owner = item_frame_owner(current) + if owner is None: + return commit_patch_for_frame(run, current, patch) + if current.id in seen: + raise WorkflowExecutionError( + f"cycle detected in foreach parent chain at frame {current.id!r}" + ) + seen.add(current.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 {current.id!r}" + ) + activation = require_foreach_activation( + parent_frame, owner.foreach_node_id, owner.activation_id ) - activation = require_foreach_activation( - parent_frame, owner.foreach_node_id, owner.activation_id - ) - if activation.barrier.mode == "concurrent": - append_lineage_writes( - run, - scope_id=frame.scope_id, - lineage_id=frame.lineage_id, - writes=patch.writes, - ) - return {} - return commit_patch_for_frame(run, parent_frame, patch) + if activation.barrier.mode == "concurrent": + append_lineage_writes( + run, + scope_id=current.scope_id, + lineage_id=current.lineage_id, + writes=patch.writes, + ) + return {} + current = parent_frame def scope_state_for_frame(run: RunState, frame: ExecutionFrame) -> dict[str, Any]: diff --git a/src/wf_core/runtime/ops/foreach.py b/src/wf_core/runtime/ops/foreach.py index e94961a5..52989dcf 100644 --- a/src/wf_core/runtime/ops/foreach.py +++ b/src/wf_core/runtime/ops/foreach.py @@ -18,7 +18,7 @@ from wf_core.runtime.foreach_state import ( ) from wf_core.runtime.lineage import ( add_lineage, - commit_patch_for_frame, + commit_foreach_aware_patch, lineage_patch, scope_input_for_frame, ) @@ -379,7 +379,7 @@ def _finish_concurrent_foreach( state_view_for_frame(run, frame), reducers=reducers, ) - state_changes = commit_patch_for_frame(run, frame, combined) + state_changes = commit_foreach_aware_patch(run, frame, combined) append_step_result_trace( run, frame_id=frame.id, diff --git a/tests/core/test_foreach_back_edges.py b/tests/core/test_foreach_back_edges.py index fb6ba2ce..eff749c0 100644 --- a/tests/core/test_foreach_back_edges.py +++ b/tests/core/test_foreach_back_edges.py @@ -427,6 +427,174 @@ def test_nested_foreach_returns_inner_then_outer() -> None: assert run.state["seen"][:3] == [1, 2, "a"] +def _nested_mode_workflow(*, outer_mode: str, inner_mode: str) -> Workflow: + def _foreach(node_id: str, *, over: str, alias: str, mode: str) -> ForeachNode: + payload: dict[str, Any] = { + "id": node_id, + "type": "foreach", + "over": over, + "as": alias, + "mode": mode, + } + if mode == "concurrent": + payload["concurrent"] = {"max_active": 2, "max_outstanding": 2} + return ForeachNode.model_validate(payload) + + return Workflow( + name="nested_foreach_modes", + input_schema=SchemaRef(type="object", properties={}), + state_schema=StateSchema.from_field_map( + { + "items": StateField(type="array"), + "inner_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=[ + NodeDef( + name="work", + input_schema=SchemaRef( + type="object", properties={"value": {}}, required=["value"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["ok"], + ) + ], + start="outer", + nodes=[ + _foreach("outer", over="state.items", alias="outer_item", mode=outer_mode), + _foreach( + "inner", + over="state.inner_items", + alias="inner_item", + mode=inner_mode, + ), + NodeUse.model_validate( + { + "id": "work", + "type": "node", + "node": "work", + "input": [{"target": "value", "path": "context.inner_item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + ], + edges=[ + Edge.model_validate({"from": "outer", "outcome": "loop", "to": "inner"}), + Edge.model_validate({"from": "inner", "outcome": "loop", "to": "work"}), + Edge.model_validate({"from": "work", "outcome": "ok", "to": "inner"}), + # No intermediate writer: the inner barrier (or serial return) + # must route inner writes to the scope root on its own. + Edge.model_validate({"from": "inner", "outcome": "done", "to": "outer"}), + Edge.model_validate({"from": "outer", "outcome": "done", "to": END}), + ], + ) + + +@pytest.mark.parametrize( + ("outer_mode", "inner_mode"), + [ + ("serial", "serial"), + ("serial", "concurrent"), + ("concurrent", "serial"), + ("concurrent", "concurrent"), + ], +) +def test_nested_foreach_preserves_inner_writes_in_all_modes( + outer_mode: str, inner_mode: str +) -> None: + """Inner writes must reach root state whatever the nesting modes are. + + Serial owners commit through the scope root; concurrent owners buffer + for their barrier. Every inner write (1, 2 per outer item) must survive + even with no intermediate writer to replay-rescue stranded lineages. + """ + workflow = _nested_mode_workflow(outer_mode=outer_mode, inner_mode=inner_mode) + + run = execute_workflow( + workflow, + {"items": ["a", "b"], "inner_items": [1, 2]}, + { + "work": lambda payload, _ctx: { + "outcome": "ok", + "output": {"seen": payload["value"]}, + } + }, + ) + + assert run.status == RunStatus.COMPLETED + assert sorted(run.state.get("seen") or [], key=repr) == sorted( + [1, 2, 1, 2], key=repr + ) + + +def test_item_frame_owner_rejects_missing_parent_frame() -> None: + """A foreach_iteration frame without a parent is malformed, not ordinary.""" + frame = ExecutionFrame( + id="orphan", + kind="foreach_iteration", + node_id="work", + parent_frame_id=None, + metadata={ + "foreach_node_id": "each", + "activation_id": "root:each#0", + "loop_index": 0, + "loop_item": "a", + "loop_alias": "item", + }, + ) + + with pytest.raises(WorkflowExecutionError, match="parent"): + item_frame_owner(frame) + + +def test_foreach_aware_patch_rejects_parent_cycle() -> None: + """A cyclic item-parent chain fails closed instead of looping forever.""" + from wf_core.runtime.foreach_state import load_or_begin_foreach_activation + from wf_core.runtime.lineage import commit_foreach_aware_patch + from wf_core.runtime.ops.state import StatePatch + + frame_a = ExecutionFrame(id="frame-a", kind="foreach_iteration", node_id="work") + frame_b = ExecutionFrame(id="frame-b", kind="foreach_iteration", node_id="work") + activation_on_b = load_or_begin_foreach_activation(frame_b, "each", mode="serial") + activation_on_a = load_or_begin_foreach_activation(frame_a, "each", mode="serial") + frame_a.parent_frame_id = "frame-b" + frame_a.metadata.update( + { + "foreach_node_id": "each", + "activation_id": activation_on_b.id, + "loop_index": 0, + "loop_item": "a", + "loop_alias": "item", + } + ) + frame_b.parent_frame_id = "frame-a" + frame_b.metadata.update( + { + "foreach_node_id": "each", + "activation_id": activation_on_a.id, + "loop_index": 0, + "loop_item": "a", + "loop_alias": "item", + } + ) + run = RunState( + workflow_name="parent_cycle", + status=RunStatus.RUNNING, + workflow_input={}, + state={}, + frames={"frame-a": frame_a, "frame-b": frame_b}, + ) + + with pytest.raises(WorkflowExecutionError, match="cycle"): + commit_foreach_aware_patch(run, frame_a, StatePatch(changes={})) + + def test_reentering_foreach_uses_fresh_activation_and_item_frames() -> None: workflow = Workflow( name="foreach_reentry", diff --git a/tests/core/test_foreach_barrier_state.py b/tests/core/test_foreach_barrier_state.py index 6df3e5ca..23dd9a8c 100644 --- a/tests/core/test_foreach_barrier_state.py +++ b/tests/core/test_foreach_barrier_state.py @@ -340,3 +340,51 @@ def test_pending_item_result_rejects_index_key_mismatch() -> None: with pytest.raises(WorkflowExecutionError, match="index mismatch"): ForeachBarrierState.from_metadata(raw["each"]["active"]["barrier"]) + + +def _failed_result( + *, index: int, frame_id: str, error_index: int, error_frame: str +) -> dict: + return { + "index": index, + "frame_id": frame_id, + "status": "failed", + "lineage_id": None, + "error": { + "index": error_index, + "frame_id": error_frame, + "node_id": "work", + "error_type": "ValueError", + "message": "bad", + }, + } + + +def test_pending_item_result_rejects_error_index_mismatch() -> None: + with pytest.raises(WorkflowExecutionError, match="error.*index|index.*error"): + PendingItemResult.from_metadata( + _failed_result( + index=0, frame_id="child-0", error_index=7, error_frame="child-0" + ) + ) + + +def test_pending_item_result_rejects_error_frame_mismatch() -> None: + with pytest.raises(WorkflowExecutionError, match="error.*frame|frame.*error"): + PendingItemResult.from_metadata( + _failed_result( + index=0, frame_id="child-0", error_index=0, error_frame="other" + ) + ) + + +def test_pending_item_result_accepts_matching_error_identity() -> None: + result = PendingItemResult.from_metadata( + _failed_result( + index=0, frame_id="child-0", error_index=0, error_frame="child-0" + ) + ) + + assert result.error is not None + assert result.error.index == 0 + assert result.error.frame_id == "child-0" From 47e57ce598c3981ea4c18eeccea9c05172194144 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 11:59:35 +0700 Subject: [PATCH 13/15] audit: validate ancestry before buffering, tighten nested ordering test --- .../2026-09-04-foreach-back-edge-design.md | 7 +- src/wf_core/runtime/lineage.py | 29 +++-- tests/core/test_foreach_back_edges.py | 116 ++++++++++++++++-- 3 files changed, 129 insertions(+), 23 deletions(-) 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 1eb8b0ca..ea1906a8 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 @@ -432,9 +432,10 @@ may independently return and complete the item. ## State and Failure Behavior 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. One shared helper +Concurrent iteration writes remain buffered in the item lineage for the +barrier to merge, while serial owners pass writes outward to the scope +root, which commits them according to the accepted concurrent-foreach ADR +and declared reducers. One shared helper routes every item write: it climbs through each serial owner to the scope root, where it commits, or stops at the first concurrent item boundary, where it buffers for that barrier to merge (the concurrent barrier finish diff --git a/src/wf_core/runtime/lineage.py b/src/wf_core/runtime/lineage.py index fb49ebe3..dcbe077a 100644 --- a/src/wf_core/runtime/lineage.py +++ b/src/wf_core/runtime/lineage.py @@ -98,8 +98,11 @@ def commit_foreach_aware_patch( climbs through every serial item owner until it reaches either the workflow/subgraph scope root, where it commits, or a concurrent item boundary, where it buffers in that item lineage for the barrier to - merge. Malformed ownership, missing parents, parent cycles, and closed - or superseded activations fail closed. + merge. The whole ancestry is validated first: the write lands only + after the chain reaches an acyclic non-item ancestor, so a parent + cycle fails closed even when it passes through a concurrent + boundary. Malformed ownership, missing parents, parent cycles, and + closed or superseded activations fail closed. """ from wf_core.runtime.foreach_state import ( item_frame_owner, @@ -108,10 +111,11 @@ def commit_foreach_aware_patch( current = frame seen: set[str] = set() + buffer_in: ExecutionFrame | None = None while True: owner = item_frame_owner(current) if owner is None: - return commit_patch_for_frame(run, current, patch) + break if current.id in seen: raise WorkflowExecutionError( f"cycle detected in foreach parent chain at frame {current.id!r}" @@ -126,15 +130,18 @@ def commit_foreach_aware_patch( activation = require_foreach_activation( parent_frame, owner.foreach_node_id, owner.activation_id ) - if activation.barrier.mode == "concurrent": - append_lineage_writes( - run, - scope_id=current.scope_id, - lineage_id=current.lineage_id, - writes=patch.writes, - ) - return {} + if buffer_in is None and activation.barrier.mode == "concurrent": + buffer_in = current current = parent_frame + if buffer_in is not None: + append_lineage_writes( + run, + scope_id=buffer_in.scope_id, + lineage_id=buffer_in.lineage_id, + writes=patch.writes, + ) + return {} + return commit_patch_for_frame(run, current, patch) def scope_state_for_frame(run: RunState, frame: ExecutionFrame) -> dict[str, Any]: diff --git a/tests/core/test_foreach_back_edges.py b/tests/core/test_foreach_back_edges.py index eff749c0..e3f75d5e 100644 --- a/tests/core/test_foreach_back_edges.py +++ b/tests/core/test_foreach_back_edges.py @@ -497,22 +497,26 @@ def _nested_mode_workflow(*, outer_mode: str, inner_mode: str) -> Workflow: @pytest.mark.parametrize( - ("outer_mode", "inner_mode"), + ("outer_mode", "inner_mode", "exact_order"), [ - ("serial", "serial"), - ("serial", "concurrent"), - ("concurrent", "serial"), - ("concurrent", "concurrent"), + ("serial", "serial", True), + ("serial", "concurrent", True), + ("concurrent", "serial", False), + ("concurrent", "concurrent", False), ], ) def test_nested_foreach_preserves_inner_writes_in_all_modes( - outer_mode: str, inner_mode: str + outer_mode: str, inner_mode: str, exact_order: bool ) -> None: """Inner writes must reach root state whatever the nesting modes are. Serial owners commit through the scope root; concurrent owners buffer for their barrier. Every inner write (1, 2 per outer item) must survive even with no intermediate writer to replay-rescue stranded lineages. + + Serial outer admission is strictly ordered, so the sequence is exactly + [1, 2, 1, 2]. Concurrent outer completion order depends on scheduling, + so only the multiset is contractual there. """ workflow = _nested_mode_workflow(outer_mode=outer_mode, inner_mode=inner_mode) @@ -528,9 +532,11 @@ def test_nested_foreach_preserves_inner_writes_in_all_modes( ) assert run.status == RunStatus.COMPLETED - assert sorted(run.state.get("seen") or [], key=repr) == sorted( - [1, 2, 1, 2], key=repr - ) + seen = run.state.get("seen") or [] + if exact_order: + assert seen == [1, 2, 1, 2] + else: + assert sorted(seen, key=repr) == sorted([1, 2, 1, 2], key=repr) def test_item_frame_owner_rejects_missing_parent_frame() -> None: @@ -595,6 +601,98 @@ def test_foreach_aware_patch_rejects_parent_cycle() -> None: commit_foreach_aware_patch(run, frame_a, StatePatch(changes={})) +def test_foreach_aware_patch_rejects_concurrent_self_cycle() -> None: + """A self-parented item with a concurrent owner must fail, not buffer.""" + from wf_core.run_state import LineageState + from wf_core.runtime.foreach_state import load_or_begin_foreach_activation + from wf_core.runtime.lineage import commit_foreach_aware_patch + from wf_core.runtime.ops.state import StatePatch + + frame = ExecutionFrame( + id="self", + kind="foreach_iteration", + node_id="work", + scope_id="root", + lineage_id="root", + ) + activation = load_or_begin_foreach_activation(frame, "each", mode="concurrent") + frame.parent_frame_id = "self" + frame.metadata.update( + { + "foreach_node_id": "each", + "activation_id": activation.id, + "loop_index": 0, + "loop_item": "a", + "loop_alias": "item", + } + ) + run = RunState( + workflow_name="concurrent_self_cycle", + status=RunStatus.RUNNING, + workflow_input={}, + state={}, + frames={"self": frame}, + lineages={"root": LineageState(id="root", scope_id="root")}, + ) + + with pytest.raises(WorkflowExecutionError, match="cycle"): + commit_foreach_aware_patch(run, frame, StatePatch(changes={})) + assert run.lineages["root"].writes == [] + + +def test_foreach_aware_patch_rejects_cycle_through_concurrent_boundary() -> None: + """A parent cycle spanning a concurrent boundary must fail, not buffer.""" + from wf_core.run_state import LineageState + from wf_core.runtime.foreach_state import load_or_begin_foreach_activation + from wf_core.runtime.lineage import commit_foreach_aware_patch + from wf_core.runtime.ops.state import StatePatch + + frame_a = ExecutionFrame( + id="frame-a", + kind="foreach_iteration", + node_id="work", + scope_id="root", + lineage_id="root", + ) + frame_b = ExecutionFrame(id="frame-b", kind="foreach_iteration", node_id="work") + activation_on_b = load_or_begin_foreach_activation( + frame_b, "each", mode="concurrent" + ) + activation_on_a = load_or_begin_foreach_activation(frame_a, "each", mode="serial") + frame_a.parent_frame_id = "frame-b" + frame_a.metadata.update( + { + "foreach_node_id": "each", + "activation_id": activation_on_b.id, + "loop_index": 0, + "loop_item": "a", + "loop_alias": "item", + } + ) + frame_b.parent_frame_id = "frame-a" + frame_b.metadata.update( + { + "foreach_node_id": "each", + "activation_id": activation_on_a.id, + "loop_index": 0, + "loop_item": "a", + "loop_alias": "item", + } + ) + run = RunState( + workflow_name="mixed_mode_cycle", + status=RunStatus.RUNNING, + workflow_input={}, + state={}, + frames={"frame-a": frame_a, "frame-b": frame_b}, + lineages={"root": LineageState(id="root", scope_id="root")}, + ) + + with pytest.raises(WorkflowExecutionError, match="cycle"): + commit_foreach_aware_patch(run, frame_a, StatePatch(changes={})) + assert run.lineages["root"].writes == [] + + def test_reentering_foreach_uses_fresh_activation_and_item_frames() -> None: workflow = Workflow( name="foreach_reentry", From 7b2f718ad7aed39f4d8f5774efa2dd666c4ce867 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 12:48:01 +0700 Subject: [PATCH 14/15] audit: validate-first routing, read-only activation lookup, delta-log barrier patches --- .../2026-09-04-foreach-back-edge-design.md | 7 +- src/wf_core/runtime/foreach_state.py | 40 +++++- src/wf_core/runtime/ops/state.py | 32 ++++- tests/core/test_atomic_state_patches.py | 40 ++++++ tests/core/test_foreach_activations.py | 28 ++++ tests/core/test_foreach_back_edges.py | 124 ++++++++++++++++++ 6 files changed, 256 insertions(+), 15 deletions(-) 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 ea1906a8..9a3e40d9 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 @@ -437,8 +437,11 @@ barrier to merge, while serial owners pass writes outward to the scope root, which commits them according to the accepted concurrent-foreach ADR and declared reducers. One shared helper routes every item write: it climbs through each serial owner to the scope -root, where it commits, or stops at the first concurrent item boundary, -where it buffers for that barrier to merge (the concurrent barrier finish +root, where it commits, or selects the first concurrent boundary as the +buffer target, where it buffers for that barrier to merge (the walk +continues past the selected boundary to validate the full ancestry, so +parent cycles fail closed even when they pass through a concurrent +owner; the concurrent barrier finish routes its combined patch through the same helper, so nested serial owners cannot strand it). Parent cycles, missing parents, and orphaned item frames fail closed. Buffered failure records must carry an error whose index and diff --git a/src/wf_core/runtime/foreach_state.py b/src/wf_core/runtime/foreach_state.py index 89dd5c54..f45c71e8 100644 --- a/src/wf_core/runtime/foreach_state.py +++ b/src/wf_core/runtime/foreach_state.py @@ -1,7 +1,7 @@ from __future__ import annotations from dataclasses import dataclass, field -from typing import Any, Literal +from typing import Any, Literal, overload from wf_core.errors import WorkflowExecutionError from wf_core.run_state import ExecutionFrame, RunState @@ -303,9 +303,13 @@ class ForeachBarrierState: def _activation_entry( - frame: ExecutionFrame, table: dict[str, Any], foreach_node_id: str + frame: ExecutionFrame, + table: dict[str, Any] | None, + foreach_node_id: str, ) -> dict[str, Any] | None: """Return the mutable activation entry or fail fast on corrupt state.""" + if table is None: + return None entry = table.get(foreach_node_id) if entry is None: return None @@ -368,7 +372,7 @@ def save_foreach_activation( frame: ExecutionFrame, activation: ForeachActivationState ) -> None: """Persist barrier progress for the named active activation.""" - table = _activation_table(frame) + table = _activation_table(frame, create=False) entry = _activation_entry(frame, table, activation.foreach_node_id) if entry is None: raise WorkflowExecutionError( @@ -391,7 +395,7 @@ def close_foreach_activation( The barrier is removed so a later visit starts fresh; the sequence keeps increasing so child and lineage ids cannot collide across visits. """ - table = _activation_table(frame) + table = _activation_table(frame, create=False) entry = _activation_entry(frame, table, activation.foreach_node_id) if entry is None: raise WorkflowExecutionError( @@ -413,8 +417,11 @@ def load_foreach_activation( A child result naming a closed or different activation must fail closed in the caller rather than buffering into the wrong barrier. + + This is a read-only lookup: a missing table or entry raises without + mutating frame metadata. """ - table = _activation_table(frame) + table = _activation_table(frame, create=False) entry = _activation_entry(frame, table, foreach_node_id) if entry is None: raise WorkflowExecutionError( @@ -469,9 +476,30 @@ def item_frame_owner(frame: ExecutionFrame) -> ForeachItemOwner | None: ) -def _activation_table(frame: ExecutionFrame) -> dict[str, Any]: +@overload +def _activation_table( + frame: ExecutionFrame, *, create: Literal[True] = True +) -> dict[str, Any]: ... + + +@overload +def _activation_table( + frame: ExecutionFrame, *, create: Literal[False] +) -> dict[str, Any] | None: ... + + +def _activation_table( + frame: ExecutionFrame, *, create: bool = True +) -> dict[str, Any] | None: + """Return the activation table, optionally creating it. + + Read-only lookups pass ``create=False`` so a failed lookup leaves + frame metadata untouched. Only ``load_or_begin`` creates the table. + """ raw = frame.metadata.get(_ACTIVATION_METADATA_KEY) if raw is None: + if not create: + return None table: dict[str, Any] = {} frame.metadata[_ACTIVATION_METADATA_KEY] = table return table diff --git a/src/wf_core/runtime/ops/state.py b/src/wf_core/runtime/ops/state.py index 8faff8e6..2a5bc187 100644 --- a/src/wf_core/runtime/ops/state.py +++ b/src/wf_core/runtime/ops/state.py @@ -249,6 +249,15 @@ def build_barrier_patch( committed aggregate values. A barrier trace is the single visible state commit for all buffered item patches, so showing raw per-item incoming values would hide what actually landed in `RunState.state`. + + The emitted `writes` log keeps every constituent item write in order + instead of one merged write per path. A combined patch buffered in a + lineage can itself be re-merged by an outer barrier, and replaying merged + cumulative values would duplicate whatever was already committed when the + constituents were built. Replaying the original per-item deltas stays + correct at any nesting depth. Each kept write still carries the merged + aggregate as its `visible_value`, so overlay reads and `visible_values` + keep showing the final value. """ state_fields = workflow.state_schema.field_index() validate_barrier_writes(item_patches, state_fields, reducers=reducers) @@ -269,22 +278,31 @@ def build_barrier_patch( safe_set_nested_value(staged_state, key_path, merged_value) prepared_patch[destination_path] = (key_path, merged_value) committed_changes[str(destination_path)] = merged_value + merged_visible = { + destination_path: merged_value + for destination_path, (_key_path, merged_value) in prepared_patch.items() + } writes = [ StateWrite( - path=destination_path, - incoming_value=merged_value, - visible_value=merged_value, - reducer=reducer_for_state_path(destination_path, state_fields), + path=write.path, + incoming_value=write.incoming_value, + visible_value=merged_visible[write.path], + reducer=write.reducer, ) - for destination_path, (_key_path, merged_value) in prepared_patch.items() + for item_patch in item_patches + for write in item_patch.writes ] validate_staged_state_patch(staged_state, prepared_patch, state_fields) - return StatePatch( - changes=committed_changes, + combined = StatePatch( writes=writes, _prepared_writes=prepared_patch, _staged_state=staged_state, ) + # The trace-facing view reports the aggregate, while the replay log above + # intentionally carries per-item deltas (see docstring). Assign it after + # construction: passing both to the constructor requires them to agree. + combined.changes = committed_changes + return combined def validate_barrier_writes( diff --git a/tests/core/test_atomic_state_patches.py b/tests/core/test_atomic_state_patches.py index f9fa5f23..d8c10244 100644 --- a/tests/core/test_atomic_state_patches.py +++ b/tests/core/test_atomic_state_patches.py @@ -284,6 +284,46 @@ def test_barrier_replays_incoming_values_not_lineage_visible_values() -> None: assert patch.visible_values["state.number"] == 6 +def test_barrier_combined_patch_remerges_without_duplicating_prefix() -> None: + """A combined patch re-merged by an outer barrier must not duplicate. + + The second barrier is computed after the first aggregate was committed, + so its constituents were built against that prefix. Re-merging must + replay the original per-item deltas, not the cumulative aggregates. + """ + workflow = _workflow( + fields={ + "seen": StateField( + type="array", + reducer=ReducerRef(name="wf.std.append"), + ) + } + ) + + first = build_barrier_patch( + workflow, + [ + StatePatch(changes={"state.seen": "a"}), + StatePatch(changes={"state.seen": "b"}), + ], + {}, + ) + second = build_barrier_patch( + workflow, + [ + StatePatch(changes={"state.seen": "c"}), + StatePatch(changes={"state.seen": "d"}), + ], + {"seen": ["a", "b"]}, + ) + + assert second.changes["state.seen"] == ["a", "b", "c", "d"] + remerged = build_barrier_patch(workflow, [first, second], {}) + + assert remerged.changes["state.seen"] == ["a", "b", "c", "d"] + assert remerged.visible_values["state.seen"] == ["a", "b", "c", "d"] + + def test_build_and_commit_patch_matches_apply_output_bindings() -> None: workflow = _workflow(fields={"person.name": StateField(type="string")}) state_from_apply = {"person": {"name": "old"}} diff --git a/tests/core/test_foreach_activations.py b/tests/core/test_foreach_activations.py index cb94d0f6..0bdea28a 100644 --- a/tests/core/test_foreach_activations.py +++ b/tests/core/test_foreach_activations.py @@ -5,8 +5,11 @@ import pytest from wf_core.errors import WorkflowExecutionError from wf_core.run_state import ExecutionFrame from wf_core.runtime.foreach_state import ( + ForeachActivationState, + ForeachBarrierState, close_foreach_activation, item_frame_owner, + load_foreach_activation, load_or_begin_foreach_activation, save_foreach_activation, ) @@ -100,3 +103,28 @@ def test_item_metadata_requires_activation_identity() -> None: ForeachIterationMetadata.from_frame(frame) with pytest.raises(WorkflowExecutionError, match="activation"): item_frame_owner(frame) + + +def test_failed_activation_lookup_leaves_metadata_untouched() -> None: + """Read-only lookups must not create the activation table on failure.""" + frame = _frame() + stale = ForeachActivationState( + id="root:each#0", + foreach_node_id="each", + barrier=ForeachBarrierState(mode="serial"), + ) + + with pytest.raises(WorkflowExecutionError, match="activation"): + load_foreach_activation(frame, "each", "root:each#0") + with pytest.raises(WorkflowExecutionError, match="activation"): + save_foreach_activation(frame, stale) + with pytest.raises(WorkflowExecutionError, match="activation"): + close_foreach_activation(frame, stale) + + assert frame.metadata == {} + + # The write path still creates the table exactly once. + activation = load_or_begin_foreach_activation(frame, "each", mode="serial") + assert frame.metadata["foreach_activations"]["each"]["active"]["id"] == ( + activation.id + ) diff --git a/tests/core/test_foreach_back_edges.py b/tests/core/test_foreach_back_edges.py index e3f75d5e..bdc368f5 100644 --- a/tests/core/test_foreach_back_edges.py +++ b/tests/core/test_foreach_back_edges.py @@ -539,6 +539,130 @@ def test_nested_foreach_preserves_inner_writes_in_all_modes( assert sorted(seen, key=repr) == sorted([1, 2, 1, 2], key=repr) +def _three_level_workflow( + *, outer_mode: str, middle_mode: str, inner_mode: str +) -> Workflow: + def _foreach(node_id: str, *, over: str, alias: str, mode: str) -> ForeachNode: + payload: dict[str, Any] = { + "id": node_id, + "type": "foreach", + "over": over, + "as": alias, + "mode": mode, + } + if mode == "concurrent": + payload["concurrent"] = {"max_active": 2, "max_outstanding": 2} + return ForeachNode.model_validate(payload) + + return Workflow( + name="three_level_nested_foreach", + input_schema=SchemaRef(type="object", properties={}), + state_schema=StateSchema.from_field_map( + { + "items": StateField(type="array"), + "mid_items": StateField(type="array"), + "inner_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=[ + NodeDef( + name="work", + input_schema=SchemaRef( + type="object", properties={"value": {}}, required=["value"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["ok"], + ) + ], + start="outer", + nodes=[ + _foreach("outer", over="state.items", alias="outer_item", mode=outer_mode), + _foreach( + "middle", + over="state.mid_items", + alias="mid_item", + mode=middle_mode, + ), + _foreach( + "inner", + over="state.inner_items", + alias="inner_item", + mode=inner_mode, + ), + NodeUse.model_validate( + { + "id": "work", + "type": "node", + "node": "work", + "input": [{"target": "value", "path": "context.inner_item"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + ], + edges=[ + Edge.model_validate({"from": "outer", "outcome": "loop", "to": "middle"}), + Edge.model_validate({"from": "middle", "outcome": "loop", "to": "inner"}), + Edge.model_validate({"from": "inner", "outcome": "loop", "to": "work"}), + Edge.model_validate({"from": "work", "outcome": "ok", "to": "inner"}), + Edge.model_validate({"from": "inner", "outcome": "done", "to": "middle"}), + Edge.model_validate({"from": "middle", "outcome": "done", "to": "outer"}), + Edge.model_validate({"from": "outer", "outcome": "done", "to": END}), + ], + ) + + +@pytest.mark.parametrize( + ("outer_mode", "middle_mode", "inner_mode"), + [ + ("serial", "serial", "serial"), + ("serial", "serial", "concurrent"), + ("serial", "concurrent", "serial"), + ("serial", "concurrent", "concurrent"), + ("concurrent", "serial", "serial"), + ("concurrent", "serial", "concurrent"), + ("concurrent", "concurrent", "serial"), + ("concurrent", "concurrent", "concurrent"), + ], +) +def test_three_level_nested_foreach_preserves_writes( + outer_mode: str, middle_mode: str, inner_mode: str +) -> None: + """Write routing holds through three nesting levels in every mode mix. + + Barriers merge items in index order, so each middle visit yields exactly + [1, 2]; only the outer completion order varies. A serial outer admits + in order, making [1, 2, 1, 2] exact, while a concurrent outer leaves + only the multiset contractual. + """ + workflow = _three_level_workflow( + outer_mode=outer_mode, middle_mode=middle_mode, inner_mode=inner_mode + ) + + run = execute_workflow( + workflow, + {"items": ["a", "b"], "mid_items": ["m"], "inner_items": [1, 2]}, + { + "work": lambda payload, _ctx: { + "outcome": "ok", + "output": {"seen": payload["value"]}, + } + }, + ) + + assert run.status == RunStatus.COMPLETED + seen = run.state.get("seen") or [] + if outer_mode == "serial": + assert seen == [1, 2, 1, 2] + else: + assert sorted(seen, key=repr) == sorted([1, 2, 1, 2], key=repr) + + def test_item_frame_owner_rejects_missing_parent_frame() -> None: """A foreach_iteration frame without a parent is malformed, not ordinary.""" frame = ExecutionFrame( From f28e1cc6b4865de54ca936d2dfac8a74977d4206 Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 4 Sep 2026 18:32:36 +0700 Subject: [PATCH 15/15] fix: inherit ancestor writes in nested foreach --- src/wf_core/runtime/lineage.py | 11 +-- tests/core/test_foreach_back_edges.py | 115 ++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 5 deletions(-) diff --git a/src/wf_core/runtime/lineage.py b/src/wf_core/runtime/lineage.py index dcbe077a..e5657af3 100644 --- a/src/wf_core/runtime/lineage.py +++ b/src/wf_core/runtime/lineage.py @@ -43,14 +43,15 @@ class LineageStateView: def lineage_writes_for_frame( run: RunState, frame: ExecutionFrame ) -> Sequence[StateWrite]: - """Return writes visible to this frame's current lineage. + """Return ancestor and current-lineage writes visible to this frame. - This is still backed by concurrent foreach barrier metadata. Keeping the - lookup here gives future `RunState.lineages` or subgraph scopes one place to - plug in without making node execution understand foreach internals. + An empty child lineage still inherits writes buffered by its ancestors, as + happens when an outer concurrent foreach writes before entering an inner + foreach. Lineage existence and scope therefore control traversal; the + current lineage having its own writes does not. """ lineage = run.lineages.get(frame.lineage_id) - if lineage is not None and lineage.scope_id == frame.scope_id and lineage.writes: + if lineage is not None and lineage.scope_id == frame.scope_id: return tuple( lineage_state_writes( run, scope_id=frame.scope_id, lineage_id=frame.lineage_id diff --git a/tests/core/test_foreach_back_edges.py b/tests/core/test_foreach_back_edges.py index bdc368f5..ca19e327 100644 --- a/tests/core/test_foreach_back_edges.py +++ b/tests/core/test_foreach_back_edges.py @@ -539,6 +539,121 @@ def test_nested_foreach_preserves_inner_writes_in_all_modes( assert sorted(seen, key=repr) == sorted([1, 2, 1, 2], key=repr) +@pytest.mark.parametrize("inner_mode", ["serial", "concurrent"]) +def test_nested_item_reads_buffered_ancestor_state(inner_mode: str) -> None: + """An inner item inherits the enclosing concurrent item's state view.""" + inner_payload: dict[str, Any] = { + "id": "inner", + "type": "foreach", + "over": "state.inner_items", + "as": "inner_item", + "mode": inner_mode, + } + if inner_mode == "concurrent": + inner_payload["concurrent"] = {"max_active": 1, "max_outstanding": 1} + workflow = Workflow( + name="nested_foreach_reads_ancestor_state", + input_schema=SchemaRef(type="object", properties={}), + state_schema=StateSchema.from_field_map( + { + "items": StateField(type="array"), + "inner_items": StateField(type="array"), + "marker": StateField(type="string", default="root"), + "seen": StateField( + type="array", reducer=ReducerRef(name="wf.std.append") + ), + } + ), + output_schema=SchemaRef(type="object", properties={"seen": {"type": "array"}}), + node_defs=[ + NodeDef( + name="write_marker", + input_schema=SchemaRef( + type="object", properties={"marker": {}}, required=["marker"] + ), + output_schema=SchemaRef( + type="object", properties={"marker": {}}, required=["marker"] + ), + outcomes=["ok"], + ), + NodeDef( + name="observe_marker", + input_schema=SchemaRef( + type="object", properties={"marker": {}}, required=["marker"] + ), + output_schema=SchemaRef( + type="object", properties={"seen": {}}, required=["seen"] + ), + outcomes=["ok"], + ), + ], + start="outer", + nodes=[ + ForeachNode.model_validate( + { + "id": "outer", + "type": "foreach", + "over": "state.items", + "as": "outer_item", + "mode": "concurrent", + "concurrent": {"max_active": 1, "max_outstanding": 1}, + } + ), + NodeUse.model_validate( + { + "id": "write_outer", + "type": "node", + "node": "write_marker", + "input": [{"target": "marker", "path": "context.outer_item"}], + "output": [{"source": "marker", "target": "state.marker"}], + } + ), + ForeachNode.model_validate(inner_payload), + NodeUse.model_validate( + { + "id": "read_inner", + "type": "node", + "node": "observe_marker", + "input": [{"target": "marker", "path": "state.marker"}], + "output": [{"source": "seen", "target": "state.seen"}], + } + ), + ], + edges=[ + Edge.model_validate( + {"from": "outer", "outcome": "loop", "to": "write_outer"} + ), + Edge.model_validate( + {"from": "write_outer", "outcome": "ok", "to": "inner"} + ), + Edge.model_validate( + {"from": "inner", "outcome": "loop", "to": "read_inner"} + ), + Edge.model_validate({"from": "read_inner", "outcome": "ok", "to": "inner"}), + Edge.model_validate({"from": "inner", "outcome": "done", "to": "outer"}), + Edge.model_validate({"from": "outer", "outcome": "done", "to": END}), + ], + ) + + run = execute_workflow( + workflow, + {"items": ["outer"], "inner_items": [1]}, + { + "write_marker": lambda payload, _ctx: { + "outcome": "ok", + "output": {"marker": payload["marker"]}, + }, + "observe_marker": lambda payload, _ctx: { + "outcome": "ok", + "output": {"seen": payload["marker"]}, + }, + }, + ) + + assert run.status == RunStatus.COMPLETED + assert run.state["seen"] == ["outer"] + + def _three_level_workflow( *, outer_mode: str, middle_mode: str, inner_mode: str ) -> Workflow: