test: address Task 3 RPC review minors
This commit is contained in:
@@ -0,0 +1,47 @@
|
||||
# Task 3 Report: Expose Generic Insertion Through Python JSON-RPC
|
||||
|
||||
## Status
|
||||
|
||||
Validated the existing five-file Task 3 implementation without redesigning it.
|
||||
No CLI or ordinary documentation files were changed.
|
||||
|
||||
## Validation
|
||||
|
||||
- Focused pytest with xdist disabled and `C:\tmp\task-3-pytest`: `48 passed, 101 warnings in 29.24s`.
|
||||
- Ruff check on the five changed files: `All checks passed!`.
|
||||
- `git diff --check`: passed; only LF/CRLF normalization warnings were emitted.
|
||||
- basedpyright: `0 errors, 0 warnings, 0 notes` within the 120-second bound.
|
||||
|
||||
The pytest warnings are dependency deprecations from `fastapi_jsonrpc`.
|
||||
The repository `uv` shim was not executable, so validation used the installed
|
||||
uv binary directly. The task-owned pytest temp directory was removed.
|
||||
|
||||
## Scope Review
|
||||
|
||||
The diff contains exactly the five files named by the brief. It adds typed RPC
|
||||
parameter models, the `workflow.draft_workspaces.add_step` server method, the
|
||||
remote client method with alias-preserving JSON serialization, and coverage for
|
||||
all nine typed variants, malformed requests, and a typed interrupt round-trip.
|
||||
|
||||
## Commit
|
||||
|
||||
The existing Task 3 five-file diff is being committed on `main`.
|
||||
|
||||
## Concerns
|
||||
|
||||
No implementation concerns found in the focused validation. Full-repository
|
||||
tests were not run because the brief requested scoped validation.
|
||||
|
||||
## Task 3 Minor Review Fix Evidence
|
||||
|
||||
- Added RPC coverage for an interrupt whose `request_schema` and `resume_schema`
|
||||
are explicitly `null`; the draft round trip asserts both values remain null.
|
||||
- Strengthened the nine-variant client serialization test with independent
|
||||
expected wire fields, including the `as` and `if` aliases and default keys;
|
||||
corrected fixture indentation.
|
||||
- Focused pytest was run with `-n0` and `C:\tmp\task-3-pytest-minors` as the
|
||||
basetemp: the first run completed with `46 passed, 3 failed`; the failures
|
||||
were test expectations that omitted intentional serialized defaults for
|
||||
`use`, `foreach`, and `subgraph`. The corrected expectations were not rerun
|
||||
because verification was stopped at the user's request.
|
||||
- Ruff format/check for the touched tests was not run for the same reason.
|
||||
@@ -1063,6 +1063,53 @@ async def test_rpc_draft_workspace_add_typed_step_round_trip(tmp_path) -> None:
|
||||
}
|
||||
|
||||
|
||||
async def test_rpc_draft_workspace_add_untyped_interrupt_preserves_null_schemas(
|
||||
tmp_path,
|
||||
) -> None:
|
||||
server = build_local_static_workflow_server(tmp_path / "store")
|
||||
app = create_rpc_app(server)
|
||||
transport = httpx.ASGITransport(app=app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
created = await _rpc(
|
||||
client,
|
||||
"workflow.draft_workspaces.create_from_capability",
|
||||
{
|
||||
"workspace_id": "untyped_interrupt_ws",
|
||||
"capability_name": "wf.std.constant",
|
||||
"name": "untyped_interrupt",
|
||||
},
|
||||
)
|
||||
added = await _rpc(
|
||||
client,
|
||||
"workflow.draft_workspaces.add_step",
|
||||
{
|
||||
"workspace_id": "untyped_interrupt_ws",
|
||||
"revision": created["result"]["revision"],
|
||||
"step_id": "pause",
|
||||
"step": {
|
||||
"interrupt": {
|
||||
"kind": "approval",
|
||||
"request_schema": None,
|
||||
"resume_schema": None,
|
||||
}
|
||||
},
|
||||
},
|
||||
)
|
||||
fetched = await _rpc(
|
||||
client,
|
||||
"workflow.draft_workspaces.get",
|
||||
{"workspace_id": "untyped_interrupt_ws", "include_draft": True},
|
||||
)
|
||||
|
||||
assert added["result"]["revision"] == created["result"]["revision"] + 1
|
||||
interrupt = fetched["result"]["draft"]["steps"]["pause"]["interrupt"]
|
||||
assert interrupt["request_schema"] is None
|
||||
assert interrupt["resume_schema"] is None
|
||||
assert fetched["result"]["draft"]["steps"]["pause"] == {
|
||||
"interrupt": interrupt
|
||||
}
|
||||
|
||||
|
||||
async def test_rpc_diagnoses_source(tmp_path) -> None:
|
||||
server = build_local_static_workflow_server(tmp_path / "store")
|
||||
app = create_rpc_app(server)
|
||||
|
||||
@@ -629,62 +629,94 @@ async def test_rpc_client_draft_workspace_add_step_from_capability(tmp_path) ->
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("step_id", "step"),
|
||||
("step_id", "step", "expected_wire"),
|
||||
[
|
||||
("use", TypeAdapter(DraftStep).validate_python({"use": "demo.echo"})),
|
||||
(
|
||||
"use",
|
||||
TypeAdapter(DraftStep).validate_python({"use": "demo.echo"}),
|
||||
{"use": "demo.echo"},
|
||||
),
|
||||
(
|
||||
"foreach",
|
||||
TypeAdapter(DraftStep).validate_python(
|
||||
{"foreach": {"over": "state.items", "as": "item"}}
|
||||
),
|
||||
{"over": "state.items", "as": "item"},
|
||||
),
|
||||
(
|
||||
"interrupt",
|
||||
TypeAdapter(DraftStep).validate_python(
|
||||
{
|
||||
"interrupt": {
|
||||
"kind": "approval",
|
||||
"request_schema": {"type": "object"},
|
||||
"resume_schema": {"type": "object"},
|
||||
"kind": "approval",
|
||||
"request_schema": {"type": "object"},
|
||||
"resume_schema": {"type": "object"},
|
||||
}
|
||||
}
|
||||
),
|
||||
{
|
||||
"kind": "approval",
|
||||
"request": [],
|
||||
"resume": [],
|
||||
"request_schema": {"type": "object", "properties": {}, "required": []},
|
||||
"resume_schema": {"type": "object", "properties": {}, "required": []},
|
||||
"outcomes": ["submitted"],
|
||||
},
|
||||
),
|
||||
("join", TypeAdapter(DraftStep).validate_python({"join": {}})),
|
||||
("end", TypeAdapter(DraftStep).validate_python({"end": {}})),
|
||||
("join", TypeAdapter(DraftStep).validate_python({"join": {}}), {}),
|
||||
("end", TypeAdapter(DraftStep).validate_python({"end": {}}), {"outcome": "ok"}),
|
||||
(
|
||||
"when",
|
||||
TypeAdapter(DraftStep).validate_python(
|
||||
{
|
||||
"when": {
|
||||
"if": {"op": "exists", "path": "state.ready"},
|
||||
"then": "next",
|
||||
"if": {"op": "exists", "path": "state.ready"},
|
||||
"then": "next",
|
||||
}
|
||||
}
|
||||
),
|
||||
{
|
||||
"if": {"op": "exists", "path": "state.ready"},
|
||||
"then": "next",
|
||||
"otherwise": "__end__",
|
||||
},
|
||||
),
|
||||
(
|
||||
"choose",
|
||||
TypeAdapter(DraftStep).validate_python(
|
||||
{
|
||||
"choose": {
|
||||
"clauses": [
|
||||
{"if": {"op": "exists", "path": "state.ready"}, "then": "next"}
|
||||
]
|
||||
"clauses": [
|
||||
{
|
||||
"if": {"op": "exists", "path": "state.ready"},
|
||||
"then": "next",
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
),
|
||||
{
|
||||
"clauses": [
|
||||
{"if": {"op": "exists", "path": "state.ready"}, "then": "next"}
|
||||
],
|
||||
"default": "__end__",
|
||||
},
|
||||
),
|
||||
(
|
||||
"match",
|
||||
TypeAdapter(DraftStep).validate_python(
|
||||
{
|
||||
"match": {
|
||||
"value": "state.status",
|
||||
"cases": [{"equals": "ready", "then": "next"}],
|
||||
"value": "state.status",
|
||||
"cases": [{"equals": "ready", "then": "next"}],
|
||||
}
|
||||
}
|
||||
),
|
||||
{
|
||||
"value": "state.status",
|
||||
"cases": [{"equals": "ready", "then": "next"}],
|
||||
"default": "__end__",
|
||||
},
|
||||
),
|
||||
(
|
||||
"subgraph",
|
||||
@@ -693,11 +725,12 @@ async def test_rpc_client_draft_workspace_add_step_from_capability(tmp_path) ->
|
||||
"subgraph": {"workflow": {"artifact_id": "child", "version": 2}}
|
||||
}
|
||||
),
|
||||
{"workflow": {"artifact_id": "child", "version": 2}},
|
||||
),
|
||||
],
|
||||
)
|
||||
async def test_rpc_client_add_step_preserves_all_typed_variants(
|
||||
step_id: str, step: DraftStep
|
||||
step_id: str, step: DraftStep, expected_wire: dict[str, Any]
|
||||
) -> None:
|
||||
calls: list[dict[str, Any]] = []
|
||||
|
||||
@@ -718,7 +751,10 @@ async def test_rpc_client_add_step_preserves_all_typed_variants(
|
||||
assert result == {"revision": 2}
|
||||
request = calls[0]
|
||||
assert request["method"] == "workflow.draft_workspaces.add_step"
|
||||
assert request["params"]["step"] == step.model_dump(mode="json", by_alias=True)
|
||||
wire_step = request["params"]["step"]
|
||||
assert set(wire_step) == {step_id}
|
||||
for field, expected_value in expected_wire.items():
|
||||
assert wire_step[step_id][field] == expected_value
|
||||
assert request["params"]["incoming"] == {
|
||||
"step_id": "lookup",
|
||||
"outcome": "ok",
|
||||
|
||||
Reference in New Issue
Block a user