fix(multi-agent-router): record every selected agent's response (#2044) - #2076
fix(multi-agent-router): record every selected agent's response (#2044)#2076Steve-Dusty wants to merge 1 commit into
Conversation
|
Verified this independently rather than taking the write-up on faith — it all holds. Approving. Two things your description understates, both of which strengthen the case: The discarded answers never reached the caller at all. It fired on every invocation. Details you got right that a smaller patch would have missed: attributing each message to its own agent rather than recording all N under Verification on my side, from detached worktrees, run offline: Identical failure sets — zero introduced. The 4/3 split on the unpatched base is the useful part: the four multi-agent tests fail and the single-handoff and unknown-agent guards pass, which is what demonstrates those paths are untouched. Two non-blocking nits, either here or as a follow-up:
Scoping out the raw-transcript issue at |
9054a66 to
5783d86
Compare
…omez#2044) handle_multiple_handoffs runs every agent the boss selected, then writes only the first one's answer into the conversation: agent_responses = list( executor.map( lambda pair: pair[0].run(pair[1]), zip(selected_agents, final_tasks), ) ) self.conversation.add( role=selected_agents[0].agent_name, content=agent_responses[0], ) agent_responses[1:] is never read. Whenever the boss selects more than one agent, the caller pays for N completions and receives one, and nothing logs or warns that the rest were dropped -- it looks like the router simply chose a single agent. The dangling `# return agent_responses` on the next line was the remnant of the refactor that left it this way. Every response is now recorded under its own agent's name, in the order the agents were dispatched. The docstring said "record the first response" and is corrected to match. Removed the commented-out return, which described the discarded values. Behavior note: callers reading self.conversation after a multi-agent handoff now see one message per agent instead of one message total. Single-agent handoffs, skipped null tasks, and the up-front unknown-agent validation are unchanged. Closes kyegomez#2044
5783d86 to
ddf9306
Compare
Closes #2044.
MultiAgentRouter.handle_multiple_handoffsran every agent the boss selected and then recorded only the first one's answer. Whenever the boss selected more than one agent, the caller paid for N completions and received one, with nothing logged or warned about the rest.Problem
swarms/structs/multi_agent_router.py:371-386:agent_responses[1:]is never read. Two consequences:The dangling
# return agent_responseson the following line is the remnant of the refactor that left it this way; the method's own docstring said "record the first response", so the code and its documentation agreed on the wrong behavior.Fix
Every response is recorded under its own agent's name, in the order the agents were run. The docstring is corrected to match, and the commented-out return — which described exactly the discarded values — is removed.
Files
swarms/structs/multi_agent_router.pytests/structs/test_multi_agent_router.pyDesign decisions
handoffsis a list and the boss prompt actively invites multiple entries — so the conservative reading is that running all of them was intended and the recording was the half left unfinished.selected_agents[0].agent_namewould have been a smaller diff and is wrong: a downstream reader filtering the conversation by agent would attribute three agents' work to the first.zip(selected_agents, agent_responses)reuses the list built alongsidefinal_tasks, so the recorded order matches the order the agents actually ran, not the boss's original list — which can differ onceskip_null_tasksdrops entries.Deliberately not in this PR
The issue lists two neighbours in the same file. Both are left alone, each its own concern:
:301-305has the same shape) —Agent.runhonoursoutput_type, defaulting to"str-all-except-first". Fixing that means routing throughcontext_utils.agent_answerat both sites and changes what every existing caller reads out of the conversation.concurrent_batch_runraces on one sharedConversation(:485) — that is the per-task conversation isolation work, not this.Behavior-change note
Callers reading
self.conversationafter a multi-agent handoff now see one message per agent instead of one message total. Anything that indexedconversation_history[-1]expecting the sole handoff result will now get the last agent's answer rather than the first's. Single-agent handoffs,skip_null_tasksskipping, and the up-front unknown-agent validation are all unchanged — each is covered by a test below.Verification
Unit: 7 tests appended to the existing
tests/structs/test_multi_agent_router.py. No new test file.Covered: all three answers kept; one message per agent run; each recorded under its own name; a single handoff unchanged; skipped null-task agents contribute nothing; a handoff with no rewritten task falls back to the original; an unknown agent raises before anything runs and leaves the conversation empty.
The tests fail on the unfixed base. Same tests against
upstream/master'smulti_agent_router.py: 4 failed, 3 passed. The four that fail are exactly the multi-agent ones; the three that pass are the single-agent and validation guards, which is the point — they prove the fix did not change those paths.The test file, before and after:
upstream/master@ff8a60e0(git worktree)Full
tests/structssuite, both runs on this machine, same interpreter:upstream/master(git worktree)Identical failure and error sets; the +7 is exactly this PR's tests. The 112 pre-existing failures — including the 22 in this very file — are live-LLM tests that construct real
Agents and need an API key; they fail identically on the clean base.Lint:
black --checkclean on both files;ruff checkwith the CI rule set (ruff==0.2.1defaults, per.github/workflows/lint.yml) reports no findings on either.Not verified here: no live-LLM path was exercised. The new tests build the router with
MultiAgentRouter.__new__and agents viaAgent.__new__, setting only the attributeshandle_multiple_handoffsreads (agents,skip_null_tasks,print_on,conversation), so no model is contacted and no API key is needed. The boss call itself is not exercised — these tests drivehandle_multiple_handoffswith a boss response directly, which is the method the defect lives in.