Repository navigation
fix(multiagent): add DISPATCHED status to prevent fan-in double-execution on resume (#3633) - #3724
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Assessment: Approve Clean, minimal, and well-targeted fix. The Review Notes
Nicely written docstrings that explain the why behind the change — makes the resume-frontier reasoning easy to follow. |
6459ee9 to
88bd093
Compare
|
Hi @liramon2 — same situation here as #3723: named checks green since the 10-15 rebase, but the integration-test jobs and |
|
@SuperMarioYL Thanks for the contribution! I left some comments on this PR. The implementation looks sound, but the comments can be shortened. |
|
Thanks @liramon2! All four spots trimmed in edb9c80: the |
Description
When a graph node is dispatched (
asyncio.create_task) but the process crashes before the node's generator body setsStatus.EXECUTING(e.g., Lambda timeout, container restart,os._exit), the node is leftPENDING. On session resume,_compute_pending_sourcesseeds only fromEXECUTINGnodes — so a dispatched-but-not-started node with no completed ancestor is indistinguishable from a bypassed dead branch. Its fan-in children are marked ready off the completed sibling alone, and the fan-in is double-executed on resume.This adds a
DISPATCHEDstatus to theStatusenum, set at the dispatch site immediately beforeasyncio.create_task._compute_pending_sourcesnow seeds from bothEXECUTINGandDISPATCHED, closing the window between dispatch and generator-body start. This completes the partial fix from #3390, which added theEXECUTINGseed but left the dispatched-but-not-started gap.Changes:
base.py: AddDISPATCHED = "dispatched"toStatusenum (additive, non-breaking)graph.py: SetDISPATCHEDat the dispatch site in_execute_nodes_parallel; update_compute_pending_sourcesto seed fromEXECUTINGandDISPATCHEDtest_graph.py: AddTestResumeDispatchedSiblingwith two tests verifying the fan-in is not double-executed when a sibling was dispatched but not startedRelated Issues
Fixes #3633
Documentation PR
No documentation changes needed — this is an internal status enum addition with no new public API surface.
Type of Change
Bug fix
Testing
Added
TestResumeDispatchedSiblingwith two tests:test_dispatched_sibling_excluded_from_resume_frontier: focused unit test that injectsDISPATCHEDstate directly (right.execution_status = DISPATCHED) and asserts the fan-injoinis excluded fromnext_nodes_to_executewhile the dispatched siblingrightis included. It pins the resume-frontier computation; on raw master it raisesAttributeErroronStatus.DISPATCHED.test_resume_runs_fan_in_once_for_dispatched_sibling: real-dispatch end-to-end reproduction — a fast-completingleftentry point and arightentry point held in the dispatched window (dispatched via the real_execute_nodes_parallelpath, blocked before_execute_nodesetsEXECUTING). The snapshot is captured fromleft'sAfterNodeCallEvent, deserialized into a fresh graph, and resumed;joinmust run exactly once with both parents' outputs present.Both tests are red on master via distinct, valid signals: the end-to-end test fails with an assertion failure (
joinlands innext_nodes_to_execute→ double-execution on resume, becauserightstaysPENDINGand is treated as a bypassed dead branch), while the unit test raisesAttributeErroronStatus.DISPATCHED. Both are green on the branch.All 83 existing
test_graph.pytests passAll 181 broader multiagent tests pass (excluding
a2awhich has a pre-existing missing-dependency issue)ruff checkandruff format --checkpassmypypasses with no issuesI ran
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.