reducer policy
This commit is contained in:
@@ -104,17 +104,19 @@ the foreach barrier commits.
|
||||
## Merge and Reducer Rules
|
||||
|
||||
At a barrier, missing reducer means default replace only for single-writer
|
||||
paths. Multiple sibling lineages writing the same state path require an explicit
|
||||
reducer. Ancestor/descendant overlapping writes across lineages are conflicts
|
||||
unless an explicit merge strategy covers them.
|
||||
paths. Multiple sibling lineages writing the same state path require a reducer
|
||||
whose metadata declares it `mergeable`. `replace` is an explicit reducer, but it
|
||||
is `exclusive`, so it is rejected for same-path sibling writes. Ancestor/
|
||||
descendant overlapping writes across lineages are conflicts unless an explicit
|
||||
future merge strategy covers them.
|
||||
|
||||
Reducers apply incrementally in deterministic lineage order. For foreach, that
|
||||
means item index order.
|
||||
|
||||
Current barrier validation enforces this policy for sibling foreach item
|
||||
lineages. Same-path sibling writes require an explicit non-`replace` reducer on
|
||||
the exact destination state path. Ancestor/descendant sibling writes are
|
||||
rejected until a future explicit deep merge policy exists.
|
||||
lineages. Same-path sibling writes require a `mergeable` reducer on the exact
|
||||
destination state path. Ancestor/descendant sibling writes are rejected until a
|
||||
future explicit deep merge policy exists.
|
||||
|
||||
## Interrupt and Failure Quiescence
|
||||
|
||||
|
||||
@@ -288,6 +288,16 @@ to `dict.update` or `operator.or_`. It is not a recursive deep merge.
|
||||
If recursive merge is ever needed, it should be explicit rather than hidden
|
||||
inside `merge_object`.
|
||||
|
||||
Reducers also declare a sibling-write policy for concurrent barriers:
|
||||
|
||||
- `exclusive`: valid for ordinary writes, rejected when multiple sibling
|
||||
lineages write the same state path at one barrier.
|
||||
- `mergeable`: allowed for same-path sibling writes; the barrier still replays
|
||||
writes in deterministic item-index order.
|
||||
|
||||
`wf.std.replace` is `exclusive`. The other built-in reducers are currently
|
||||
`mergeable`.
|
||||
|
||||
## Future Reducers
|
||||
|
||||
Reducers are a capability family, similar to reusable node specs:
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
|
||||
**Goal:** Enforce deterministic and explicit write semantics when concurrent foreach sibling item lineages commit at the barrier.
|
||||
|
||||
**Architecture:** Keep per-node patch building unchanged. Add a barrier-only validation step before replaying item patches: inspect all item patch destination paths, reject ambiguous sibling writes, and allow multi-writer paths only when the exact declared state path has an explicit non-replace reducer. Barrier replay still happens in item-index order through existing reducer logic.
|
||||
**Architecture:** Keep per-node patch building unchanged. Add a barrier-only validation step before replaying item patches: inspect all item patch destination paths, reject ambiguous sibling writes, and allow multi-writer paths only when the exact declared state path has a reducer whose metadata is `mergeable`. Barrier replay still happens in item-index order through existing reducer logic.
|
||||
|
||||
**Tech Stack:** Python 3.14, dataclasses, Pydantic v2 models, pytest, `StatePath`, `StateSchema.field_index()`, `StatePatch`, `ForeachBarrierState`, and existing reducer definitions.
|
||||
|
||||
@@ -17,14 +17,14 @@ This slice owns sibling write policy at a foreach barrier.
|
||||
Allowed:
|
||||
|
||||
- A destination path written by exactly one item lineage uses normal state rules.
|
||||
- A destination path written by multiple item lineages is allowed only if that exact declared state path has an explicit reducer other than `wf.std.replace`.
|
||||
- A destination path written by multiple item lineages is allowed only if that exact declared state path has a `mergeable` reducer.
|
||||
- Multi-writer reducer replay is deterministic item-index order.
|
||||
- Multiple nodes inside the same item lineage may write multiple paths; that is item-local overlay behavior from Slice 2.
|
||||
|
||||
Rejected:
|
||||
|
||||
- Multiple sibling item lineages writing the same destination path with missing reducer/default replace.
|
||||
- Multiple sibling item lineages writing the same destination path with explicit `wf.std.replace`.
|
||||
- Multiple sibling item lineages writing the same destination path with an `exclusive` reducer such as `wf.std.replace`.
|
||||
- Sibling item lineages writing ancestor/descendant paths such as `state.person` and `state.person.name`.
|
||||
|
||||
Important distinction:
|
||||
@@ -508,9 +508,9 @@ In `docs/adr/0002-concurrent-foreach-policy-and-barrier-commits.md`, under
|
||||
|
||||
```markdown
|
||||
Current barrier validation enforces this policy for sibling foreach item
|
||||
lineages. Same-path sibling writes require an explicit non-`replace` reducer on
|
||||
the exact destination state path. Ancestor/descendant sibling writes are
|
||||
rejected until a future explicit deep merge policy exists.
|
||||
lineages. Same-path sibling writes require a `mergeable` reducer on the exact
|
||||
destination state path. Ancestor/descendant sibling writes are rejected until a
|
||||
future explicit deep merge policy exists.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Update roadmap Slice 3**
|
||||
|
||||
@@ -28,7 +28,7 @@ Already implemented:
|
||||
commits.
|
||||
- Multi-step concurrent item bodies are supported for fail-only item policy.
|
||||
- Barrier write validation rejects ambiguous sibling writes: same-path sibling
|
||||
writes require an explicit non-`replace` reducer, and ancestor/descendant
|
||||
writes require a `mergeable` reducer, and ancestor/descendant
|
||||
sibling writes are rejected.
|
||||
|
||||
## Non-Goals For Phase 4
|
||||
@@ -88,7 +88,8 @@ Scope:
|
||||
|
||||
- Detect sibling lineage writes to the same state path.
|
||||
- If exactly one lineage writes a destination path, default replace is allowed.
|
||||
- If multiple sibling lineages write the same destination path, a declared reducer is required.
|
||||
- If multiple sibling lineages write the same destination path, a declared
|
||||
`mergeable` reducer is required.
|
||||
- Ancestor/descendant writes across sibling lineages are conflicts unless an explicit future merge strategy covers them.
|
||||
- Commit order is item index order, never completion order.
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ from typing import Any, TypeVar, cast, overload
|
||||
|
||||
from pydantic import BaseModel
|
||||
|
||||
from wf_core import ReducerSpec
|
||||
from wf_core import ReducerSpec, SiblingWritePolicy
|
||||
from wf_core.runtime.ops.merges import ReducerDefinition
|
||||
|
||||
from .callables import ConfigReducerCallable, ConfigT, PlainReducerCallable
|
||||
@@ -28,6 +28,7 @@ def reducer(
|
||||
*,
|
||||
name: str | None = None,
|
||||
description: str | None = None,
|
||||
sibling_write_policy: SiblingWritePolicy = SiblingWritePolicy.MERGEABLE,
|
||||
) -> AuthoredReducer: ...
|
||||
|
||||
|
||||
@@ -36,6 +37,7 @@ def reducer(
|
||||
*,
|
||||
name: str,
|
||||
description: str | None = None,
|
||||
sibling_write_policy: SiblingWritePolicy = SiblingWritePolicy.MERGEABLE,
|
||||
) -> Callable[[Callable[..., Any]], AuthoredReducer]: ...
|
||||
|
||||
|
||||
@@ -45,6 +47,7 @@ def reducer(
|
||||
name: str,
|
||||
config_model: type[ConfigT],
|
||||
description: str | None = None,
|
||||
sibling_write_policy: SiblingWritePolicy = SiblingWritePolicy.MERGEABLE,
|
||||
) -> Callable[[Callable[..., Any]], AuthoredReducer]: ...
|
||||
|
||||
|
||||
@@ -55,6 +58,7 @@ def reducer(
|
||||
name: str | None = None,
|
||||
config_model: type[BaseModel] | None = None,
|
||||
description: str | None = None,
|
||||
sibling_write_policy: SiblingWritePolicy = SiblingWritePolicy.MERGEABLE,
|
||||
) -> AuthoredReducer | Callable[[Callable[..., Any]], AuthoredReducer]:
|
||||
"""Wrap a Python reducer function as a runtime reducer definition."""
|
||||
|
||||
@@ -67,6 +71,7 @@ def reducer(
|
||||
spec=ReducerSpec(
|
||||
name=reducer_name,
|
||||
description=_clean_doc(reducer_description),
|
||||
sibling_write_policy=sibling_write_policy,
|
||||
),
|
||||
fn=raw,
|
||||
)
|
||||
@@ -92,6 +97,7 @@ def reducer(
|
||||
name=reducer_name,
|
||||
description=_clean_doc(reducer_description),
|
||||
config_schema=config_model.model_json_schema(),
|
||||
sibling_write_policy=sibling_write_policy,
|
||||
),
|
||||
fn=runtime_fn,
|
||||
accepts_config=True,
|
||||
|
||||
@@ -12,6 +12,7 @@ from .models import (
|
||||
ReducerRef,
|
||||
ReducerSpec,
|
||||
SchemaRef,
|
||||
SiblingWritePolicy,
|
||||
StateField,
|
||||
StateSchema,
|
||||
Workflow,
|
||||
@@ -60,6 +61,7 @@ __all__ = [
|
||||
"ReducerRef",
|
||||
"ReducerSpec",
|
||||
"SchemaRef",
|
||||
"SiblingWritePolicy",
|
||||
"StateField",
|
||||
"StateSchema",
|
||||
"AsyncNodeHandler",
|
||||
|
||||
@@ -9,7 +9,7 @@ from wf_core.models.conditions import (
|
||||
VariadicCondition,
|
||||
)
|
||||
from wf_core.models.results import NodeResult
|
||||
from wf_core.models.reducers import ReducerRef, ReducerSpec
|
||||
from wf_core.models.reducers import ReducerRef, ReducerSpec, SiblingWritePolicy
|
||||
from wf_core.models.schemas import NodeDef, SchemaRef, StateField, StateSchema
|
||||
from wf_core.models.steps import (
|
||||
ConditionNode,
|
||||
@@ -44,6 +44,7 @@ __all__ = [
|
||||
"Operand",
|
||||
"PathOperand",
|
||||
"SchemaRef",
|
||||
"SiblingWritePolicy",
|
||||
"StateField",
|
||||
"StateSchema",
|
||||
"Step",
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Mapping
|
||||
from enum import StrEnum
|
||||
from typing import Any
|
||||
|
||||
from pydantic import BaseModel, ConfigDict, Field, model_validator
|
||||
@@ -64,6 +65,13 @@ class ReducerRef(BaseModel):
|
||||
return str(self.ref)
|
||||
|
||||
|
||||
class SiblingWritePolicy(StrEnum):
|
||||
"""Whether a reducer is safe for sibling foreach lineages at a barrier."""
|
||||
|
||||
EXCLUSIVE = "exclusive"
|
||||
MERGEABLE = "mergeable"
|
||||
|
||||
|
||||
class ReducerSpec(BaseModel):
|
||||
"""Inspectable metadata for one named pure state reducer."""
|
||||
|
||||
@@ -78,3 +86,11 @@ class ReducerSpec(BaseModel):
|
||||
"additionalProperties": False,
|
||||
}
|
||||
)
|
||||
sibling_write_policy: SiblingWritePolicy = Field(
|
||||
default=SiblingWritePolicy.MERGEABLE,
|
||||
description=(
|
||||
"Whether sibling foreach item lineages may write this state path "
|
||||
"at one barrier. Exclusive reducers are valid for ordinary writes "
|
||||
"but ambiguous for sibling barrier commits."
|
||||
),
|
||||
)
|
||||
|
||||
@@ -5,7 +5,7 @@ from dataclasses import dataclass
|
||||
from typing import Any, cast
|
||||
|
||||
from wf_core.errors import WorkflowExecutionError
|
||||
from wf_core.models.reducers import ReducerRef, ReducerSpec
|
||||
from wf_core.models.reducers import ReducerRef, ReducerSpec, SiblingWritePolicy
|
||||
from wf_core.runtime.ops.schemas import validate_payload_against_schema
|
||||
|
||||
PlainReducer = Callable[[Any, Any], Any]
|
||||
@@ -116,6 +116,7 @@ DEFAULT_REDUCER_DEFINITIONS: Mapping[str, ReducerDefinition] = {
|
||||
spec=ReducerSpec(
|
||||
name="wf.std.replace",
|
||||
description="Replace the current state value with the incoming value.",
|
||||
sibling_write_policy=SiblingWritePolicy.EXCLUSIVE,
|
||||
),
|
||||
fn=replace_reducer,
|
||||
),
|
||||
@@ -157,6 +158,33 @@ DEFAULT_REDUCER_DEFINITIONS: Mapping[str, ReducerDefinition] = {
|
||||
}
|
||||
|
||||
|
||||
def get_reducer_definition(
|
||||
reducer: ReducerRef,
|
||||
reducers: Mapping[str, ReducerDefinition] | None = None,
|
||||
) -> ReducerDefinition:
|
||||
"""Resolve a reducer from injected definitions plus built-ins."""
|
||||
definition = None if reducers is None else reducers.get(reducer.name)
|
||||
if definition is None:
|
||||
definition = DEFAULT_REDUCER_DEFINITIONS.get(reducer.name)
|
||||
if definition is None:
|
||||
raise WorkflowExecutionError(f"unknown reducer {reducer.name!r}")
|
||||
return definition
|
||||
|
||||
|
||||
def reducer_allows_sibling_writes(
|
||||
reducer: ReducerRef,
|
||||
reducers: Mapping[str, ReducerDefinition] | None = None,
|
||||
) -> bool:
|
||||
"""Return whether a reducer can merge sibling foreach item writes.
|
||||
|
||||
Reducers remain pure value functions. Barrier conflict rules live here as
|
||||
metadata so `replace` can be exclusive without teaching the reducer about
|
||||
foreach, frames, or item ordering.
|
||||
"""
|
||||
definition = get_reducer_definition(reducer, reducers)
|
||||
return definition.spec.sibling_write_policy is SiblingWritePolicy.MERGEABLE
|
||||
|
||||
|
||||
def apply_reducer(
|
||||
*,
|
||||
reducer: ReducerRef,
|
||||
@@ -171,11 +199,7 @@ def apply_reducer(
|
||||
tests and local packages can provide custom reducers without re-registering
|
||||
every `wf.std.*` reducer.
|
||||
"""
|
||||
definition = None if reducers is None else reducers.get(reducer.name)
|
||||
if definition is None:
|
||||
definition = DEFAULT_REDUCER_DEFINITIONS.get(reducer.name)
|
||||
if definition is None:
|
||||
raise WorkflowExecutionError(f"unknown reducer {reducer.name!r}")
|
||||
definition = get_reducer_definition(reducer, reducers)
|
||||
return definition.apply(
|
||||
reducer=reducer,
|
||||
current_value=current_value,
|
||||
|
||||
@@ -19,7 +19,11 @@ from wf_core.paths import (
|
||||
set_nested_value,
|
||||
split_graph_path,
|
||||
)
|
||||
from wf_core.runtime.ops.merges import ReducerDefinition, apply_reducer
|
||||
from wf_core.runtime.ops.merges import (
|
||||
ReducerDefinition,
|
||||
apply_reducer,
|
||||
reducer_allows_sibling_writes,
|
||||
)
|
||||
from wf_core.runtime.ops.schemas import validate_payload_against_schema
|
||||
|
||||
_MISSING = object()
|
||||
@@ -175,7 +179,7 @@ def build_barrier_patch(
|
||||
values would hide what actually landed in `RunState.state`.
|
||||
"""
|
||||
state_fields = workflow.state_schema.field_index()
|
||||
validate_barrier_writes(item_patches, state_fields)
|
||||
validate_barrier_writes(item_patches, state_fields, reducers=reducers)
|
||||
staged_state = deepcopy(state)
|
||||
prepared_patch: dict[StatePath, tuple[list[str], Any]] = {}
|
||||
committed_changes: dict[str, Any] = {}
|
||||
@@ -204,6 +208,8 @@ def build_barrier_patch(
|
||||
def validate_barrier_writes(
|
||||
item_patches: Sequence[StatePatch],
|
||||
state_fields: Mapping[StatePath, StateFieldDecl],
|
||||
*,
|
||||
reducers: Mapping[str, ReducerDefinition] | None = None,
|
||||
) -> None:
|
||||
"""Reject ambiguous sibling writes before replaying a foreach barrier.
|
||||
|
||||
@@ -216,11 +222,11 @@ def validate_barrier_writes(
|
||||
if left.item_index == right.item_index:
|
||||
continue
|
||||
if left.path == right.path:
|
||||
if _has_explicit_non_replace_reducer(left.path, state_fields):
|
||||
if _allows_sibling_writes(left.path, state_fields, reducers):
|
||||
continue
|
||||
raise WorkflowExecutionError(
|
||||
"multiple sibling writes to "
|
||||
f"{left.source_key!r} require an explicit reducer"
|
||||
f"{left.source_key!r} require a mergeable reducer"
|
||||
)
|
||||
if _state_paths_overlap(left.path, right.path):
|
||||
raise WorkflowExecutionError(
|
||||
@@ -244,14 +250,15 @@ def _barrier_writes(item_patches: Sequence[StatePatch]) -> list[_BarrierWrite]:
|
||||
return writes
|
||||
|
||||
|
||||
def _has_explicit_non_replace_reducer(
|
||||
def _allows_sibling_writes(
|
||||
path: StatePath,
|
||||
state_fields: Mapping[StatePath, StateFieldDecl],
|
||||
reducers: Mapping[str, ReducerDefinition] | None,
|
||||
) -> bool:
|
||||
field = state_fields.get(path)
|
||||
if field is None or field.reducer is None:
|
||||
return False
|
||||
return field.reducer.name != "wf.std.replace"
|
||||
return reducer_allows_sibling_writes(field.reducer, reducers)
|
||||
|
||||
|
||||
def _state_paths_overlap(left: StatePath, right: StatePath) -> bool:
|
||||
|
||||
@@ -8,7 +8,9 @@ from wf_core import (
|
||||
NodeDef,
|
||||
NodeUse,
|
||||
ReducerRef,
|
||||
ReducerSpec,
|
||||
SchemaRef,
|
||||
SiblingWritePolicy,
|
||||
StateField,
|
||||
StateSchema,
|
||||
Workflow,
|
||||
@@ -16,6 +18,7 @@ from wf_core import (
|
||||
)
|
||||
from wf_core.models.steps import OutputBinding
|
||||
from wf_core.runtime.engine import resume_workflow
|
||||
from wf_core.runtime.ops.merges import ReducerDefinition
|
||||
from wf_core.runtime.ops.runs import create_run_state
|
||||
from wf_core.runtime.ops.state import (
|
||||
StatePatch,
|
||||
@@ -215,7 +218,7 @@ def test_build_and_commit_patch_matches_apply_output_bindings() -> None:
|
||||
def test_barrier_rejects_sibling_same_path_writes_without_reducer() -> None:
|
||||
workflow = _workflow(fields={"value": StateField(type="string")})
|
||||
|
||||
with pytest.raises(WorkflowExecutionError, match="explicit reducer"):
|
||||
with pytest.raises(WorkflowExecutionError, match="mergeable reducer"):
|
||||
build_barrier_patch(
|
||||
workflow,
|
||||
[
|
||||
@@ -236,7 +239,7 @@ def test_barrier_rejects_sibling_same_path_writes_with_explicit_replace() -> Non
|
||||
}
|
||||
)
|
||||
|
||||
with pytest.raises(WorkflowExecutionError, match="explicit reducer"):
|
||||
with pytest.raises(WorkflowExecutionError, match="mergeable reducer"):
|
||||
build_barrier_patch(
|
||||
workflow,
|
||||
[
|
||||
@@ -269,6 +272,62 @@ def test_barrier_allows_sibling_same_path_writes_with_non_replace_reducer() -> N
|
||||
assert patch.changes["state.seen"] == ["a", "b"]
|
||||
|
||||
|
||||
def test_barrier_uses_reducer_policy_instead_of_reducer_name() -> None:
|
||||
workflow = _workflow(
|
||||
fields={
|
||||
"value": StateField(
|
||||
type="integer",
|
||||
reducer=ReducerRef(name="test.keep_latest"),
|
||||
)
|
||||
}
|
||||
)
|
||||
reducer = ReducerDefinition(
|
||||
spec=ReducerSpec(name="test.keep_latest"),
|
||||
fn=lambda _current, incoming: incoming,
|
||||
)
|
||||
|
||||
patch = build_barrier_patch(
|
||||
workflow,
|
||||
[
|
||||
StatePatch(changes={"state.value": 1}),
|
||||
StatePatch(changes={"state.value": 2}),
|
||||
],
|
||||
{},
|
||||
reducers={"test.keep_latest": reducer},
|
||||
)
|
||||
|
||||
assert patch.changes["state.value"] == 2
|
||||
|
||||
|
||||
def test_barrier_rejects_custom_exclusive_reducer() -> None:
|
||||
workflow = _workflow(
|
||||
fields={
|
||||
"value": StateField(
|
||||
type="integer",
|
||||
reducer=ReducerRef(name="test.last"),
|
||||
)
|
||||
}
|
||||
)
|
||||
reducer = ReducerDefinition(
|
||||
spec=ReducerSpec(
|
||||
name="test.last",
|
||||
sibling_write_policy=SiblingWritePolicy.EXCLUSIVE,
|
||||
),
|
||||
fn=lambda _current, incoming: incoming,
|
||||
)
|
||||
|
||||
with pytest.raises(WorkflowExecutionError, match="mergeable reducer"):
|
||||
build_barrier_patch(
|
||||
workflow,
|
||||
[
|
||||
StatePatch(changes={"state.value": 1}),
|
||||
StatePatch(changes={"state.value": 2}),
|
||||
],
|
||||
{},
|
||||
reducers={"test.last": reducer},
|
||||
)
|
||||
|
||||
|
||||
def test_barrier_rejects_sibling_ancestor_descendant_writes() -> None:
|
||||
workflow = _workflow_from_state_schema(
|
||||
StateSchema.model_validate(
|
||||
|
||||
@@ -210,7 +210,7 @@ def test_sync_concurrent_foreach_sibling_overlays_do_not_leak() -> None:
|
||||
def test_sync_concurrent_foreach_rejects_sibling_replace_writes() -> None:
|
||||
workflow = _same_path_replace_workflow()
|
||||
|
||||
with pytest.raises(WorkflowExecutionError, match="explicit reducer"):
|
||||
with pytest.raises(WorkflowExecutionError, match="mergeable reducer"):
|
||||
execute_workflow(
|
||||
workflow,
|
||||
{"items": ["a", "b"]},
|
||||
|
||||
@@ -6,6 +6,7 @@ from wf_core import (
|
||||
ReducerRef,
|
||||
ReducerSpec,
|
||||
SchemaRef,
|
||||
SiblingWritePolicy,
|
||||
StateField,
|
||||
StateSchema,
|
||||
Workflow,
|
||||
@@ -468,6 +469,12 @@ def test_reducer_definition_can_wrap_plain_two_arg_callable() -> None:
|
||||
assert result == 5
|
||||
|
||||
|
||||
def test_reducer_spec_defaults_to_mergeable_sibling_write_policy() -> None:
|
||||
spec = ReducerSpec(name="test.reducer")
|
||||
|
||||
assert spec.sibling_write_policy is SiblingWritePolicy.MERGEABLE
|
||||
|
||||
|
||||
def test_reducer_definition_can_wrap_config_aware_callable() -> None:
|
||||
definition = ReducerDefinition(
|
||||
spec=ReducerSpec(
|
||||
|
||||
Reference in New Issue
Block a user