-
Notifications
You must be signed in to change notification settings - Fork 18.6k
Fix: prevent workflow early termination when Answer node completes before parallel nodes in IF/ELSE branch #27602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
check pending session
Optional response stream coordinator for checking pending sessions
add check pending sesstion in executuion
Summary of ChangesHello @runzhezhang11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in workflow execution where workflows with parallel branches, particularly those involving an Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a premature workflow termination issue that occurs when an Answer node in one branch of an IF/ELSE statement completes before parallel nodes in another branch. The fix ensures workflows properly wait for all parallel nodes to complete by tracking pending response sessions.
Key Changes:
- Added response session tracking to prevent early termination when Answer nodes complete before parallel nodes
- Enhanced completion check to consider pending response sessions alongside queue and execution state
- Introduced comprehensive unit tests to validate the fix
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| api/core/workflow/graph_engine/response_coordinator/coordinator.py | Adds has_pending_sessions() method to check for active, waiting, or pending response sessions |
| api/core/workflow/graph_engine/graph_state_manager.py | Updates completion logic to consider pending response sessions and accepts response coordinator as constructor parameter |
| api/core/workflow/graph_engine/graph_engine.py | Passes response coordinator to GraphStateManager during initialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
api/core/workflow/graph_engine/response_coordinator/coordinator.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses a critical bug where workflows were prematurely terminating when an Answer node completed before parallel nodes in an IF/ELSE branch. The changes include adding a has_pending_sessions() method to ResponseStreamCoordinator, updating GraphStateManager.is_execution_complete() to consider pending response sessions, and passing the response_coordinator to GraphStateManager in GraphEngine. Comprehensive unit tests have also been added to validate the fix.
| # === State Management === | ||
| # Unified state manager handles all node state transitions and queue operations | ||
| self._state_manager = GraphStateManager(self._graph, self._ready_queue) | ||
| self._state_manager = GraphStateManager(self._graph, self._ready_queue, self._response_coordinator) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self, graph: Graph, ready_queue: ReadyQueue, response_coordinator: "ResponseStreamCoordinator | None" = None | ||
| ) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
api/core/workflow/graph_engine/response_coordinator/coordinator.py
Outdated
Show resolved
Hide resolved
|
@copilot open a new pull request to apply changes based on the comments in this thread |
…r.py Co-authored-by: Copilot <[email protected]>
Important
Fixes #<issue number>.Summary
Details
This fixes an issue where workflows would terminate prematurely when:
The workflow now correctly waits for all parallel nodes to complete before terminating.
Checklist
dev/reformat(backend) andcd web && npx lint-staged(frontend) to appease the lint gods