fix(agent): route parallel/council fan-out to one concurrent spawn_parallel_agents call (#4754)#4757
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e8d1a2c93
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…rallel_agents call (tinyhumansai#4754) An efficiency eval found the orchestrator never fanning out workers concurrently: parallel / "separate researcher for each" / council prompts either single-spawned or issued serial `spawn_subagent` calls 145-200s apart (each sub-agent finishing before the next started), defeating the request. Root cause is routing, not harness concurrency. `spawn_parallel_agents` already fans out concurrently (tinyagents `map_reduce` / `buffer_unordered`) as a single tool call, but the orchestrator reached for serial `spawn_subagent` instead — and the "only 1 spawn" council case proves the model simply never fanned out, which no harness change can fix. Steer fan-out to one `spawn_parallel_agents` call in the two places the model actually decides: - orchestrator prompt: map "in parallel" / "a separate X for each" / council / "fan out over N" to a single `spawn_parallel_agents` call, and warn that looping `spawn_subagent` runs strictly one-at-a-time. - `spawn_subagent` tool description: redirect concurrent fan-out to `spawn_parallel_agents` (the description is what the model reads at tool-selection time). Note: tinyagents also serializes multi-tool-call turns when tool-wrap middleware is present (openhuman always registers several) because `ToolMiddleware::wrap_tool` holds `&mut RunContext`; that is a separate, deliberate harness limitation and out of scope here. Regression: tests/orchestrator_parallel_fanout_routing.rs.
6e8d1a2 to
8a78f18
Compare
…g blocking (addresses @chatgpt-codex-connector on spawn_subagent.rs:79) The new tool description opened with "Runs ONE sub-agent and blocks until it returns", but the default path (blocking unset) routes to SpawnAsyncSubagentTool and returns immediately. Reword to state the async default and point at `blocking: true` for inline output, while keeping the fan-out redirect to `spawn_parallel_agents` intact.
|
Approve. The fix delivers what #4754 needs, and the root-cause call is correct. The issue guessed the bug was in the tinyagents harness. Verified it is not: Notes:
Comment resolution: Codex P2 (description promising synchronous behavior) is genuinely addressed in |
YellowSnnowmann
left a comment
There was a problem hiding this comment.
Approving — feature satisfied, no blocker/major issues, review comments addressed in code, CI green. Detailed review in the PR comment above.
What
An agent-efficiency eval found the orchestrator never fanning out sub-agents concurrently (
PARALLEL=0across the whole run):Root cause (verified in code) — routing, not harness concurrency
I traced the concurrency path end to end:
tinyagentsspawn_parallel_agents→map_reduceis genuinely concurrent —futures::stream::iter(...).buffer_unordered(N)with eagerly-constructed per-worker futures. openhuman's adapter passesmax_concurrency = NandFailurePolicy::CollectAll. As a single tool call it also sidesteps the harness's serial-multi-call path.spawn_subagent(single or repeated) instead of the one concurrentspawn_parallel_agentscall. The "only 1 spawn" council/parallel cases are decisive: the model never fanned out at all, which no harness change can fix.This is an openhuman routing defect: the model reaches for
spawn_subagenteven when the user explicitly asks for parallel/council work.Fix (minimal) — steer at the two decision points the model actually uses
orchestrator/prompt.md— explicit trigger→tool mapping: "in parallel" / "a separate researcher for each X" / "convene a council / multiple independent opinions" / "fan out over N" ⇒ onespawn_parallel_agentscall (one task per worker), with a warning that loopingspawn_subagentruns strictly one-at-a-time (~145s+ each) and defeats the parallel intent.spawn_subagent.rstool description — the string the model reads at tool-selection time now redirects concurrent fan-out tospawn_parallel_agentsand calls out that repeatedspawn_subagentserializes.tests/orchestrator_parallel_fanout_routing.rs— regression test pinning the routing guidance in both places (mirrors the existingorchestrator_*_wiring.rsinclude-str tests).Secondary finding (reported, out of scope)
The tinyagents harness does serialize multi-tool-call turns whenever tool-wrap middleware is registered (openhuman always registers 5: schema-guard, approval, cli-rpc, tool-policy, credential-scrub) — because
ToolMiddleware::wrap_tool(&mut RunContext)holds an exclusive borrow across the call (agent_loop/tools.rs:tool_calls.len() > 1 && tool_middleware_len() == 0). That's a real latency limitation for legitimate in-turn multi-tool fan-out, but it's documented as deliberate, its fix is architectural (not minimal), and it would not address the confirmed single-spawn cases here. Flagging for separate tracking rather than changing the harness in this PR.Testing
No local build (disk constrained); CI will compile/run. Failing-before/passing-after is deterministic: on
mainthe prompt containscouncil0× and the serial-loop warning 0×, andspawn_subagent's description mentionsspawn_parallel_agents0×; with the fix all three are present. (Note: the repo'scargo test --libtarget has a pre-existing, unrelated compile break onmain—web_tests.rsimportssuper::compose_system_prompt_suffix, whichweb/mod.rsdoesn't re-export — so this regression is written as an integration test that compiles against the clean lib, per the existingorchestrator_*_wiring.rsconvention.)Closes #4754