feat(workflow): add Workflow tool for dynamic multi-agent orchestration - #186
Conversation
agent() checked budget.remaining() before acquiring the concurrency semaphore, so every agent() call dispatched together (e.g. via parallel()) saw the same stale state["spent"] and passed regardless of the configured concurrency, only catching the overrun after the fact. Move the check inside async with semaphore so it re-evaluates spend once a slot is actually granted, bounding worst-case overshoot to one concurrency batch instead of the whole dispatched set.
…pdates Adds the Unreleased changelog bullet for the Workflow tool and a real abort-teardown integration test proving cancellation propagates through AgentTool -> ForegroundSubagentRunner and marks the child instance "killed" (not just the engine's own bookkeeping). Registering Workflow in agent.yaml grew the default agent spec's tools list, which moved pinned tool-list snapshots in test_agent_spec.py (root spec + 5 inheriting child specs) and test_default_agent.py (the loaded toolset's tool names) -- all fixed as a single additive line each. test_workflow_parser.py/test_workflow_registration.py/test_workflow_tool.py also picked up ruff-format reflow from the full-repo gate.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds a built-in ChangesWorkflow Tool Implementation
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Workflow
participant ApprovalSystem
participant run_workflow
participant AgentTool
Caller->>Workflow: __call__(params: script, args, token_budget)
Workflow->>Workflow: enforce root-only role
Workflow->>Workflow: compute script fingerprint
Workflow->>ApprovalSystem: request orchestration approval
ApprovalSystem-->>Workflow: approved or rejected
alt rejected
Workflow-->>Caller: ToolError
else approved
Workflow->>run_workflow: run_workflow(script, agent_runner, concurrency, token_budget, hooks)
loop per agent() call
run_workflow->>AgentTool: dispatch child prompt
AgentTool-->>run_workflow: result or schema retry error
end
run_workflow-->>Workflow: WorkflowRunResult(meta, result, logs, phases)
Workflow-->>Caller: ToolOk(JSON result)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/tools/workflow/__init__.py`:
- Around line 101-123: The workflow progress hook is dropping `log()` output
because `RunWorkflowHooks.on_log` ignores the log message and only calls
`emit()`, so the text never reaches `WorkflowSnapshot` or the final `ToolOk`
result. Update the workflow event handling around `emit`, `on_log`, and
`render_progress` so log messages are either stored in `WorkflowSnapshot` and
rendered in `ProgressNote`, or remove/disable `log()` from the workflow API if
logs are not meant to be surfaced. Ensure the visible progress/result path
includes the actual log text, not just a rerender trigger.
In `@src/pythinker_code/tools/workflow/display.py`:
- Around line 40-50: Track agent completion by stable id rather than label in
the workflow display state. Update the AgentSnapshot lifecycle in display.py so
start_agent() returns and stores a unique identifier for each agent, and
end_agent() uses that identifier instead of walking the agents list by label.
Keep the display label for rendering only, and make sure the running/error/done
status update targets the exact snapshot created by start_agent().
In `@src/pythinker_code/tools/workflow/engine.py`:
- Around line 273-285: _validate_options() currently passes through arbitrary
dict values, which lets bad types reach AgentOptions consumers like the strip()
call on opts.label in agent() and causes internal AttributeError failures.
Tighten normalization in _normalize_options() by validating that label, phase,
model, and agent_type are either strings or None, and that schema is an allowed
schema shape (or None), raising WorkflowRuntimeError with a clear message when
any field is malformed. Keep the existing dict check and use AgentOptions as the
central place to reject invalid agent() option values early so scripts like
agent("x", {"label": 1}) fail with a tool error instead of an internal
exception.
In `@tests/tools/test_workflow_tool.py`:
- Around line 132-176: The current tests in Workflow.__call__ only spy on
run_workflow kwargs, which ties them to an internal implementation detail
instead of observable tool behavior. Update these tests to exercise the public
tool surface by invoking the tool with a script that is sensitive to token
limits, then assert the resulting ToolOk or ToolError outcome for a small
token_budget and the default unbounded case when token_budget is omitted. Keep
the focus on the Workflow tool contract rather than monkeypatching
pythinker_code.tools.workflow.run_workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 65907d0d-9e7f-4b03-90fb-34e78cfe76c9
📒 Files selected for processing (15)
CHANGELOG.mdsrc/pythinker_code/agents/default/agent.yamlsrc/pythinker_code/tools/workflow/__init__.pysrc/pythinker_code/tools/workflow/description.mdsrc/pythinker_code/tools/workflow/display.pysrc/pythinker_code/tools/workflow/engine.pytests/core/test_agent_spec.pytests/core/test_default_agent.pytests/tools/test_workflow_abort_integration.pytests/tools/test_workflow_display.pytests/tools/test_workflow_engine.pytests/tools/test_workflow_parser.pytests/tools/test_workflow_registration.pytests/tools/test_workflow_tool.pytests/utils/test_pyinstaller_utils.py
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary
Workflowtool: the model writes a small Python "workflow script" that orchestrates many subagents viaagent(),parallel(),pipeline(),phase(),log(), and an optionalbudget, AST-validated for determinism and executed in a restricted namespace (ported from the JS reference underblackbox/pi-dynamic-workflows-main, redesigned Python-native since the CLI ships no Node runtime).agent()call routes through the existingAgentTool(not a lower-level runner), so it inherits root-only/no-nesting enforcement, type/policy/capacity checks, and the establishedRunAgentsorchestration-approval pattern (one approval per distinct script).wire_send(ProgressNote(...)); structured output supports JSON-schema validation with one retry; cancellation propagatesasyncio.CancelledErrorthrough to real child teardown (AgentTool→ForegroundSubagentRunner), proven by both an engine-level test and a real-spawn-layer integration test.concurrency; fixed by gating the check inside the semaphore (now bounded to one concurrency batch's worth of overshoot).token_budgetis exposed as a settable, optional per-call tool parameter (mirroring the existingAgentTool.Params.budget_secondsprecedent) so the documentedbudgetprimitive actually enforces when set, rather than being inert in production.Test plan
make check-pythinker-code(ruff check + format + pyright + ty) — cleantests/) — 6475 passed, 7 skipped, 1 xfailedtests_e2e— 65 passed, 4 skippedCHANGELOG.mdupdated under## UnreleasedSummary by CodeRabbit
New Features
Bug Fixes