-
Notifications
You must be signed in to change notification settings - Fork 41
Fixes #231
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
Fixes #231
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated executed_state handling to set status of states created for additional outputs (beyond the first) to EXECUTED at creation time, without altering other fields or background task scheduling. Single-output and zero-output paths remain unchanged apart from this status behavior for subsequent outputs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant StateStore as State Store
participant Scheduler
Client->>Controller: executed_state(body with outputs)
alt No outputs
Controller->>StateStore: Record executed state
else One output
Controller->>StateStore: Create state for first output (EXECUTED)
Controller->>Scheduler: Schedule next-state task
else Multiple outputs
Controller->>StateStore: Create state for first output (EXECUTED)
Controller->>Scheduler: Schedule next-state task
loop For each output in outputs[1:]
Controller->>StateStore: Create state (status=EXECUTED)
Controller->>Scheduler: Schedule next-state task
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)state-manager/app/controller/executed_state.py (4)
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Summary of Changes
Hello @NiveditJain, 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 bug where a state, upon execution, was incorrectly assigned a CREATED status instead of an EXECUTED status. The change ensures that the system accurately reflects the true operational status of a state once its execution process is complete.
Highlights
- State Management Fix: Corrected the status assignment in
state-manager/app/controller/executed_state.pyfromStateStatusEnum.CREATEDtoStateStatusEnum.EXECUTEDwhen processing an executed state.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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 an issue in the state management logic for nodes that produce multiple outputs. The change correctly sets the status of newly created states for additional outputs to EXECUTED instead of CREATED, which aligns with the intended state machine flow. My review includes a suggestion to enhance the unit tests to cover this specific case and prevent future regressions. For better maintainability, I recommend using more descriptive titles and providing a summary in the pull request body for future changes.
| graph_name=state.graph_name, | ||
| run_id=state.run_id, | ||
| status=StateStatusEnum.CREATED, | ||
| status=StateStatusEnum.EXECUTED, |
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.
This change correctly sets the status to EXECUTED. To prevent future regressions and ensure the correctness of this fix, it's important to add an assertion to the test_executed_state_success_multiple_outputs unit test. The test should verify that the newly created states for multiple outputs are indeed created with the EXECUTED status.
No description provided.