sudden coderabbit review fixes
This commit is contained in:
@@ -8,6 +8,11 @@ from pydantic import BaseModel, ConfigDict, Field, model_validator
|
|||||||
from wf_platform.refs import CapabilityRef
|
from wf_platform.refs import CapabilityRef
|
||||||
|
|
||||||
|
|
||||||
|
def _raise_conflicting_ref_and_name() -> None:
|
||||||
|
"""Reject ambiguous reducer references before compatibility coercion."""
|
||||||
|
raise ValueError("reducer ref and name are mutually exclusive")
|
||||||
|
|
||||||
|
|
||||||
class ReducerRef(BaseModel):
|
class ReducerRef(BaseModel):
|
||||||
"""Reference to one reducer capability plus JSON-compatible configuration."""
|
"""Reference to one reducer capability plus JSON-compatible configuration."""
|
||||||
|
|
||||||
@@ -28,6 +33,8 @@ class ReducerRef(BaseModel):
|
|||||||
`ReducerRef(name="wf.std.add")`. That remains source-compatible, but
|
`ReducerRef(name="wf.std.add")`. That remains source-compatible, but
|
||||||
the model state is now the structural capability reference.
|
the model state is now the structural capability reference.
|
||||||
"""
|
"""
|
||||||
|
if ref is not None and name is not None:
|
||||||
|
_raise_conflicting_ref_and_name()
|
||||||
payload: dict[str, object] = {"config": config or {}}
|
payload: dict[str, object] = {"config": config or {}}
|
||||||
if ref is not None:
|
if ref is not None:
|
||||||
payload["ref"] = ref
|
payload["ref"] = ref
|
||||||
@@ -45,6 +52,8 @@ class ReducerRef(BaseModel):
|
|||||||
if not isinstance(value, Mapping):
|
if not isinstance(value, Mapping):
|
||||||
return value
|
return value
|
||||||
data = dict(value)
|
data = dict(value)
|
||||||
|
if "ref" in data and "name" in data:
|
||||||
|
_raise_conflicting_ref_and_name()
|
||||||
if "ref" not in data and "name" in data:
|
if "ref" not in data and "name" in data:
|
||||||
data["ref"] = CapabilityRef.parse(str(data.pop("name")))
|
data["ref"] = CapabilityRef.parse(str(data.pop("name")))
|
||||||
return data
|
return data
|
||||||
|
|||||||
@@ -406,6 +406,8 @@ def get_nested_value(state: Mapping[str, Any], path_parts: list[str]) -> Any:
|
|||||||
def set_nested_value(
|
def set_nested_value(
|
||||||
state: MutableMapping[str, Any], path_parts: list[str], value: Any
|
state: MutableMapping[str, Any], path_parts: list[str], value: Any
|
||||||
) -> None:
|
) -> None:
|
||||||
|
if not path_parts:
|
||||||
|
raise PathResolutionError("cannot set value with empty path")
|
||||||
current: MutableMapping[str, Any] = state
|
current: MutableMapping[str, Any] = state
|
||||||
for part in path_parts[:-1]:
|
for part in path_parts[:-1]:
|
||||||
next_value = current.get(part)
|
next_value = current.get(part)
|
||||||
|
|||||||
@@ -93,7 +93,14 @@ class RunState:
|
|||||||
def current_frame(self) -> ExecutionFrame:
|
def current_frame(self) -> ExecutionFrame:
|
||||||
if self.current_frame_id is None:
|
if self.current_frame_id is None:
|
||||||
raise ValueError("run has no current frame")
|
raise ValueError("run has no current frame")
|
||||||
return self.frames[self.current_frame_id]
|
frame = self.frames.get(self.current_frame_id)
|
||||||
|
if frame is None:
|
||||||
|
raise ValueError(
|
||||||
|
"run current frame id is missing from frames: "
|
||||||
|
f"current_frame_id={self.current_frame_id!r}, "
|
||||||
|
f"frames={sorted(self.frames)!r}"
|
||||||
|
)
|
||||||
|
return frame
|
||||||
|
|
||||||
def sync_from_current_frame(self) -> None:
|
def sync_from_current_frame(self) -> None:
|
||||||
frame = self.current_frame()
|
frame = self.current_frame()
|
||||||
|
|||||||
@@ -27,7 +27,7 @@ def execute_workflow(
|
|||||||
run = create_run_state(workflow, workflow_input)
|
run = create_run_state(workflow, workflow_input)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
run = prepare_new_run(workflow, workflow_input)
|
prepare_new_run(workflow, workflow_input, run)
|
||||||
return resume_workflow(workflow, run, registry, reducers=reducers)
|
return resume_workflow(workflow, run, registry, reducers=reducers)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
run.status = RunStatus.FAILED
|
run.status = RunStatus.FAILED
|
||||||
@@ -46,7 +46,7 @@ async def execute_workflow_async(
|
|||||||
run = create_run_state(workflow, workflow_input)
|
run = create_run_state(workflow, workflow_input)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
run = prepare_new_run(workflow, workflow_input)
|
prepare_new_run(workflow, workflow_input, run)
|
||||||
return await resume_workflow_async(workflow, run, registry, reducers=reducers)
|
return await resume_workflow_async(workflow, run, registry, reducers=reducers)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
run.status = RunStatus.FAILED
|
run.status = RunStatus.FAILED
|
||||||
|
|||||||
@@ -9,15 +9,17 @@ from wf_core.runtime.ops.frames import collapse_completed_frames
|
|||||||
from wf_core.runtime.ops.index import WorkflowIndex, build_workflow_index
|
from wf_core.runtime.ops.index import WorkflowIndex, build_workflow_index
|
||||||
from wf_core.runtime.ops.interrupts import resume_interrupt
|
from wf_core.runtime.ops.interrupts import resume_interrupt
|
||||||
from wf_core.runtime.ops.merges import ReducerDefinition
|
from wf_core.runtime.ops.merges import ReducerDefinition
|
||||||
from wf_core.runtime.ops.runs import create_run_state
|
|
||||||
from wf_core.runtime.ops.schemas import validate_payload_against_schema
|
from wf_core.runtime.ops.schemas import validate_payload_against_schema
|
||||||
from wf_core.run_state import FrameStatus, RunState, RunStatus
|
from wf_core.run_state import FrameStatus, RunState, RunStatus
|
||||||
from wf_core.tokens import END
|
from wf_core.tokens import END
|
||||||
|
|
||||||
|
|
||||||
def prepare_new_run(workflow: Workflow, workflow_input: dict[str, Any]) -> RunState:
|
def prepare_new_run(
|
||||||
|
workflow: Workflow,
|
||||||
|
workflow_input: dict[str, Any],
|
||||||
|
run: RunState,
|
||||||
|
) -> RunState:
|
||||||
"""Create and validate a fresh run state for a workflow invocation."""
|
"""Create and validate a fresh run state for a workflow invocation."""
|
||||||
run = create_run_state(workflow, workflow_input)
|
|
||||||
workflow.validate_structure().raise_for_errors()
|
workflow.validate_structure().raise_for_errors()
|
||||||
validate_payload_against_schema(
|
validate_payload_against_schema(
|
||||||
workflow.input_schema, workflow_input, "workflow input"
|
workflow.input_schema, workflow_input, "workflow input"
|
||||||
|
|||||||
@@ -1,5 +1,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
from wf_core import (
|
from wf_core import (
|
||||||
ReducerRef,
|
ReducerRef,
|
||||||
ReducerSpec,
|
ReducerSpec,
|
||||||
@@ -354,6 +356,22 @@ def test_reducer_ref_accepts_canonical_ref_object() -> None:
|
|||||||
assert reducer.name == "wf.std.append"
|
assert reducer.name == "wf.std.append"
|
||||||
|
|
||||||
|
|
||||||
|
def test_reducer_ref_rejects_conflicting_ref_and_name() -> None:
|
||||||
|
with pytest.raises(ValueError, match="mutually exclusive"):
|
||||||
|
ReducerRef(
|
||||||
|
ref={"source": "wf.std", "capability_key": "append"},
|
||||||
|
name="wf.std.add",
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="mutually exclusive"):
|
||||||
|
ReducerRef.model_validate(
|
||||||
|
{
|
||||||
|
"ref": {"source": "wf.std", "capability_key": "append"},
|
||||||
|
"name": "wf.std.add",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_unknown_state_reducer_fails_clearly() -> None:
|
def test_unknown_state_reducer_fails_clearly() -> None:
|
||||||
workflow = _workflow(
|
workflow = _workflow(
|
||||||
fields={
|
fields={
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ from wf_core.paths import (
|
|||||||
StatePath,
|
StatePath,
|
||||||
is_valid_destination_path,
|
is_valid_destination_path,
|
||||||
is_valid_source_path,
|
is_valid_source_path,
|
||||||
|
set_nested_value,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -254,3 +255,8 @@ def test_existing_source_and_destination_validation_helpers_use_new_parsers() ->
|
|||||||
assert is_valid_destination_path("state") is False
|
assert is_valid_destination_path("state") is False
|
||||||
assert is_valid_destination_path("state.person") is True
|
assert is_valid_destination_path("state.person") is True
|
||||||
assert is_valid_destination_path("input.person") is False
|
assert is_valid_destination_path("input.person") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_nested_value_rejects_empty_path() -> None:
|
||||||
|
with pytest.raises(PathResolutionError, match="empty path"):
|
||||||
|
set_nested_value({}, [], "value")
|
||||||
|
|||||||
@@ -0,0 +1,18 @@
|
|||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from wf_core.run_state import RunState, RunStatus
|
||||||
|
|
||||||
|
|
||||||
|
def test_current_frame_rejects_missing_frame_id_with_clear_error() -> None:
|
||||||
|
run = RunState(
|
||||||
|
workflow_name="demo",
|
||||||
|
status=RunStatus.RUNNING,
|
||||||
|
workflow_input={},
|
||||||
|
state={},
|
||||||
|
current_frame_id="missing",
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="current_frame_id='missing'"):
|
||||||
|
run.current_frame()
|
||||||
Reference in New Issue
Block a user