fix: workflow incorrectly marked as completed while nodes are still executing#48
Conversation
🤖 Augment PR SummarySummary: Fixes a workflow-graph execution edge case where the overall run could be marked complete while downstream nodes are still expected to run. Changes:
Technical Notes: Completion is determined by an empty ready-queue and an empty executing-nodes set, so marking nodes as executing earlier helps prevent premature completion checks. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| if edge_states["has_taken"]: | ||
| # Enqueue node | ||
| # Start execution and enqueue node | ||
| print(f"Starting execution for node: {downstream_node_id}") |
There was a problem hiding this comment.
The print(...) here will write to stdout from core workflow execution, which is likely noisy in production and can interfere with test output capture. Consider using the project logger (or removing this) if you need diagnostics.
🤖 Was this useful? React with 👍 or 👎
| for edge in outgoing_edges: | ||
| self._state_manager.mark_edge_skipped(edge.id) | ||
| # Recursively propagate skip | ||
| self.propagate_skip_from_edge(edge.id) |
There was a problem hiding this comment.
propagate_skip_from_edge(edge.id) is invoked before the edge is marked as skipped, so the downstream node will typically see this incoming edge as UNKNOWN and stop propagation. Since mark_edge_skipped happens after, there’s no follow-up propagation call, which can break recursive skip propagation.
🤖 Was this useful? React with 👍 or 👎
| mock_graph.get_outgoing_edges.return_value = [edge1, edge2] | ||
|
|
||
| # Setup get_incoming_edges to return empty list to stop recursion | ||
| mock_graph.get_incoming_edges.return_value = [] |
There was a problem hiding this comment.
This test sets get_incoming_edges to [], which makes analyze_edge_states() treat the downstream node as all_skipped=True regardless of whether the outgoing edge was actually marked skipped. That setup can mask regressions around the ordering between mark_edge_skipped() and propagate_skip_from_edge().
🤖 Was this useful? React with 👍 or 👎
1 participant
Benchmark PR from qodo-benchmark#441