diff --git a/docs/current_roadmap.md b/docs/current_roadmap.md index 000026d1..9ec3a989 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. - Proposed follow-up: replace innermost-only foreach values with same-scope [`structured runtime context`](superpowers/specs/2026-09-04-structured-runtime-context-design.md), including typed Python lookup, graph paths, schema analysis, authoring refs, @@ -808,7 +802,7 @@ stable. - Proposed runtime guard: add a persisted, run-wide [`step budget`](superpowers/specs/2026-09-04-run-step-budget-design.md) for valid graph cycles that cannot be proven terminating during validation. -- After that correction, reuse the foreach barrier/lineage machinery for +- Next, 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). - Protocol-native progress: investigate MCP tasks/progress or WebSocket/SSE only @@ -949,6 +943,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/plans/2026-09-04-foreach-back-edges.md b/docs/historical/superpowers/plans/2026-09-04-foreach-back-edges.md similarity index 91% 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 index 8833ed82..a019c421 100644 --- a/docs/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 aeeb58da..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 @@ -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 @@ -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 @@ -431,9 +432,24 @@ 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. +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 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 +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 +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 +506,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/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. 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/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/context_scopes.py b/src/wf_core/analysis/context_scopes.py index 84d114e8..86f3d020 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, @@ -65,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 @@ -80,6 +84,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 +118,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 +139,67 @@ 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: 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: + 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 +215,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 +230,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 +238,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 +262,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 +275,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 +287,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/src/wf_core/analysis/control_regions.py b/src/wf_core/analysis/control_regions.py new file mode 100644 index 00000000..8b8063ba --- /dev/null +++ b/src/wf_core/analysis/control_regions.py @@ -0,0 +1,274 @@ +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 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], + ) -> 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. + record_region_conflict(node_id, (recorded, 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: + # 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: + record_region_conflict( + target_id, (recorded_target, target_stack) + ) + 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/src/wf_core/runtime/foreach_state.py b/src/wf_core/runtime/foreach_state.py index 1da9065f..f45c71e8 100644 --- a/src/wf_core/runtime/foreach_state.py +++ b/src/wf_core/runtime/foreach_state.py @@ -1,16 +1,37 @@ 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.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" + + +@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) @@ -68,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 @@ -92,39 +111,48 @@ 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") + 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 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") + 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" + ) + 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, - 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 - else None - ), + error=error, ) def to_metadata(self) -> dict[str, Any]: @@ -133,10 +161,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, } @@ -151,33 +175,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): @@ -201,7 +198,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, @@ -210,20 +212,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, @@ -266,15 +254,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: @@ -283,7 +269,6 @@ class ForeachBarrierState: frame_id=frame_id, status="succeeded", lineage_id=lineage_id, - patch=patch, ) return if existing.frame_id != frame_id: @@ -291,14 +276,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. @@ -320,14 +302,232 @@ 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.""" - if frame.kind != "foreach_iteration" or frame.parent_frame_id is None: +def _activation_entry( + 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 + 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, + *, + 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 = _activation_entry(frame, table, foreach_node_id) + if entry is None: + entry = {"next_sequence": 0, "active": None} + table[foreach_node_id] = entry + 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, create=False) + 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}" + ) + 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, create=False) + 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}" + ) + 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. + + This is a read-only lookup: a missing table or entry raises without + mutating frame metadata. + """ + table = _activation_table(frame, create=False) + entry = _activation_entry(frame, table, foreach_node_id) + if entry is None: + 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 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": + 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 - 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, + ) + + +@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 + 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, ...]: @@ -338,53 +538,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 d2d3d97e..e5657af3 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 ForeachBarrierState, item_frame_owner from wf_core.runtime.ops.state import ( StatePatch, commit_state_patch, @@ -44,41 +43,21 @@ 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 ) ) - - # 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. - 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) - 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}" - ) - barrier = ForeachBarrierState.from_frame(parent_frame, foreach_node_id) - if barrier is None or barrier.mode != "concurrent": - return () - - pending = barrier.pending_results.get(item_index) - if pending is None: - return () - return pending.patch.writes + return () def is_scope_root_lineage_frame(run: RunState, frame: ExecutionFrame) -> bool: @@ -111,6 +90,61 @@ 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. 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. 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, + require_foreach_activation, + ) + + current = frame + seen: set[str] = set() + buffer_in: ExecutionFrame | None = None + while True: + owner = item_frame_owner(current) + if owner is None: + break + 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 + ) + 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]: """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/flow.py b/src/wf_core/runtime/ops/flow.py index 7950aebb..237d498d 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,46 @@ 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, + register_foreach_item_success, + ) + + 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 + # 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 + 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 +134,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 db38d71c..52989dcf 100644 --- a/src/wf_core/runtime/ops/foreach.py +++ b/src/wf_core/runtime/ops/foreach.py @@ -8,13 +8,17 @@ 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, + close_foreach_activation, + load_or_begin_foreach_activation, + save_foreach_activation, ) from wf_core.runtime.lineage import ( add_lineage, - commit_patch_for_frame, + commit_foreach_aware_patch, lineage_patch, scope_input_for_frame, ) @@ -63,7 +67,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 @@ -83,35 +88,24 @@ 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 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) - 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, - 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, ) + save_foreach_activation(frame, activation) block_frame_on_children(run, frame.id, (child_id,)) append_step_result_trace( run, @@ -141,11 +135,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,7 +145,7 @@ def _step_foreach_concurrent( frame=frame, step=step, index=index, - barrier=barrier, + activation=activation, iterable=iterable, ) @@ -165,11 +156,11 @@ def _step_foreach_concurrent( frame=frame, step=step, index=index, - barrier=barrier, + activation=activation, 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 @@ -236,18 +227,71 @@ 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, frame: ExecutionFrame, step: ForeachNode, index: WorkflowIndex, - barrier: ForeachBarrierState, + activation: ForeachActivationState, 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) @@ -256,37 +300,17 @@ 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) - 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, - 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, @@ -313,16 +337,22 @@ def _finish_concurrent_foreach( frame: ExecutionFrame, step: ForeachNode, index: WorkflowIndex, - barrier: ForeachBarrierState, + activation: ForeachActivationState, reducers: Mapping[str, ReducerDefinition] | None = None, ) -> RunState: - 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 - ] + barrier = activation.barrier + # 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 = [ @@ -349,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, @@ -368,17 +398,28 @@ 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 -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( @@ -388,14 +429,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/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 4df383bf..199b4df8 100644 --- a/src/wf_core/runtime/ops/nodes.py +++ b/src/wf_core/runtime/ops/nodes.py @@ -15,18 +15,16 @@ from wf_core.run_state import ( RuntimeContext, StepExecutionResult, ) -from wf_core.runtime.foreach_state import ForeachBarrierState, item_frame_owner 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 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[ @@ -112,32 +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_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": - # New concurrent foreach stores writes in the child lineage; the - # barrier keeps only result metadata plus old patch fallback. - append_lineage_writes( - run, - scope_id=frame.scope_id, - lineage_id=frame.lineage_id, - writes=patch.writes, - ) - barrier.add_success_patch( - index=item_index, - frame_id=frame.id, - patch=StatePatch(), - lineage_id=frame.lineage_id, - ) - barrier.save_to_frame(parent_frame, foreach_node_id) - 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/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/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..1da1491c 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 ( @@ -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 @@ -361,10 +366,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 +383,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 +404,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/src/wf_core/runtime/subgraphs.py b/src/wf_core/runtime/subgraphs.py index 3292586e..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,7 +236,10 @@ 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) + # 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/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/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_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_concurrent_foreach.py b/tests/core/test_concurrent_foreach.py index 2ba18c59..936f8ad0 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,16 @@ 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 + # 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 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 @@ -385,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}), ], ) @@ -495,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}), ], ) @@ -571,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( @@ -587,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}), ], ) @@ -602,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: @@ -772,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}), ], ) @@ -839,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 609f9508..a8d087e2 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" @@ -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 f171e700..009f488f 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 @@ -52,9 +52,16 @@ 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"] - assert resumed.trace[interrupted_trace_len].frame_id == "root:each:1" + 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" foreach_entries = [entry for entry in resumed.trace if entry.step_type == "foreach"] @@ -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_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") diff --git a/tests/core/test_foreach_activations.py b/tests/core/test_foreach_activations.py new file mode 100644 index 00000000..0bdea28a --- /dev/null +++ b/tests/core/test_foreach_activations.py @@ -0,0 +1,130 @@ +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 ( + ForeachActivationState, + ForeachBarrierState, + close_foreach_activation, + item_frame_owner, + load_foreach_activation, + 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) + + +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 new file mode 100644 index 00000000..ca19e327 --- /dev/null +++ b/tests/core/test_foreach_back_edges.py @@ -0,0 +1,1445 @@ +from __future__ import annotations + +from typing import Any + +import pytest + +from wf_core import ( + END, + ConditionNode, + Edge, + ForeachNode, + InterruptNode, + NodeDef, + NodeUse, + ReducerRef, + SchemaRef, + StateField, + StateSchema, + SubgraphNode, + 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 +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 _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", "exact_order"), + [ + ("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, 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) + + 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 + 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) + + +@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: + 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( + 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_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", + 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_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"] + + +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", + 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_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 ( + 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) diff --git a/tests/core/test_foreach_barrier_state.py b/tests/core/test_foreach_barrier_state.py index a428648d..23dd9a8c 100644 --- a/tests/core/test_foreach_barrier_state.py +++ b/tests/core/test_foreach_barrier_state.py @@ -1,107 +1,83 @@ from __future__ import annotations +from copy import deepcopy + 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, + add_lineage, + append_lineage_writes, + lineage_writes_for_frame, ) -from wf_core.runtime.lineage import LineageStateView, lineage_writes_for_frame -from wf_core.runtime.ops.state import StatePatch -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: @@ -132,23 +108,50 @@ 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]" 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", }, ) - patch = StatePatch( + # 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 + save_foreach_activation(parent, activation) + run = RunState( + workflow_name="lineage", + status=RunStatus.PENDING, + workflow_input={}, + 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",)), @@ -156,27 +159,7 @@ def test_lineage_writes_for_frame_reads_current_foreach_pending_result() -> None visible_value=5, reducer=ReducerRef(name="wf.std.add"), ) - ] - ) - barrier = ForeachBarrierState( - mode="concurrent", - 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") - run = RunState( - workflow_name="lineage", - status=RunStatus.PENDING, - workflow_input={}, - state={"count": 2}, - frames={parent.id: parent, child.id: child}, + ], ) writes = lineage_writes_for_frame(run, child) @@ -186,47 +169,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", - kind="foreach_iteration", - node_id="work", - parent_frame_id="missing", - metadata={ - "foreach_node_id": "each", - "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: @@ -261,39 +213,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: @@ -325,16 +282,109 @@ 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"}, +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, + } + ) + + +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]" ) - barrier = ForeachBarrierState(next_index=1) + 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="barrier table"): - barrier.save_to_frame(frame, "each") + with pytest.raises(WorkflowExecutionError, match="index mismatch"): + ForeachBarrierState.from_metadata(raw["each"]["active"]["barrier"]) - assert frame.metadata["foreach_barriers"] == "corrupt" + +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" diff --git a/tests/core/test_foreach_control_regions.py b/tests/core/test_foreach_control_regions.py new file mode 100644 index 00000000..c48469e5 --- /dev/null +++ b/tests/core/test_foreach_control_regions.py @@ -0,0 +1,446 @@ +from __future__ import annotations + +from wf_core import END, Workflow +from wf_core.analysis.control_regions import ( + ControlRegionAnalysis, + ControlRegionIssueKind, + analyze_control_regions, +) +from wf_core.validation.issues import ValidationIssueCode + + +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"}, + } + + +_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", + 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": ()} + _assert_no_public_control_errors(workflow) + + +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"] == () + _assert_no_public_control_errors(workflow) + + +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",) + _assert_no_public_control_errors(workflow) + + +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 == () + _assert_no_public_control_errors(workflow) + + +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"] == () + _assert_no_public_control_errors(workflow) + + +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 + 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: + 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 + 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: + 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 + ] + matching = [ + issue + for issue in workflow.validate_structure().errors + if issue.code == ValidationIssueCode.INVALID_FOREACH_RETURN + ] + 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", + 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 + ] + 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: + 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 + ] + 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: + 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 + ] + 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: + 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 + ] + 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: + 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 + ] + 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: + 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 + 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": [], 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 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"} }