Fix: UI streaming issue #391
Conversation
🦋 Changeset detectedLatest commit: dad3c98 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe pull request introduces a patch named "create-llama" aimed at resolving a streaming issue within the application. Key modifications include enhancements to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
.changeset/popular-pears-repeat.md (1)
5-5: Enhance the changeset description with more details.The current description is quite brief. Consider expanding it to include:
- What was causing the UI to hang
- How the streaming parameter in
run_agentmethod fixes the issue- Any potential considerations for users upgrading to this version
Example:
-Fix the streaming issue to prevent the UI from hanging. +Fix UI streaming issue by adding streaming parameter to run_agent method. This prevents the UI from hanging during BlogPostWorkflow and FinancialReportWorkflow operations by improving control over streaming behavior and error handling during agent execution.templates/components/agents/python/financial_report/app/agents/workflow.py (1)
Line range hint
172-186: Fix return type annotation and clarify streaming behavior.The method has several typing and implementation inconsistencies:
- The
AsyncGeneratortype hint is missing its type parameters- Despite the union return type, the method always returns
AgentRunResult- The relationship between the
streamingparameter and return type isn't clearApply these fixes:
async def run_agent( self, ctx: Context, agent: FunctionCallingAgent, input: str, streaming: bool = False, - ) -> AgentRunResult | AsyncGenerator: + ) -> AgentRunResult: handler = agent.run(input=input, streaming=streaming) # bubble all events while running the executor to the planner async for event in handler.stream_events(): # Don't write the StopEvent from sub task to the stream if type(event) is not StopEvent: ctx.write_event_to_stream(event) return await handlerConsider documenting the streaming behavior in the method's docstring to clarify how the
streamingparameter affects the execution.templates/components/agents/python/blog/app/agents/workflow.py (1)
241-243: Consider enhancing error handling with more specific error messages.The addition of streaming support and error handling is good. However, the error message could be more informative.
Consider this enhancement:
except Exception as e: ctx.write_event_to_stream( AgentRunEvent( name=publisher.name, - msg=f"Error publishing: {e}", + msg=f"Error publishing: {e.__class__.__name__}: {str(e)}", ) )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- .changeset/popular-pears-repeat.md (1 hunks)
- templates/components/agents/python/blog/app/agents/workflow.py (1 hunks)
- templates/components/agents/python/financial_report/app/agents/workflow.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
templates/components/agents/python/blog/app/agents/workflow.py (1)
Pattern
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.templates/components/agents/python/financial_report/app/agents/workflow.py (1)
Pattern
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
🔇 Additional comments (4)
.changeset/popular-pears-repeat.md (1)
1-3: LGTM! Appropriate version bump for a bug fix.The patch version bump is correct for this type of change as it fixes a bug without introducing breaking changes.
templates/components/agents/python/financial_report/app/agents/workflow.py (1)
151-153: Verify consistent streaming implementation across workflows.The streaming changes are part of a broader update affecting multiple workflow classes. Let's verify the consistency of the implementation.
Also applies to: 172-186
✅ Verification successful
Let me gather more specific information about the streaming implementation in these files to verify consistency.
Based on the gathered information, I can now provide the final response:
Streaming implementation is consistent across workflows
The verification shows that:
- Both
BlogPostWorkflowandFinancialReportWorkflowhave identicalrun_agentmethod signatures with consistent streaming parameter handling- All workflows consistently access streaming configuration via
ctx.data["streaming"]- The streaming parameter is properly propagated to the agent's run method in all implementations
- The planner workflow has additional logic to handle streaming only for last tasks and when plan refining is disabled
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check streaming implementation across workflow classes # Expected: Similar streaming parameter handling in other workflow classes # Find all workflow files echo "Searching for workflow files..." fd -e py -p "workflow.py$" # Check for streaming parameter usage echo -e "\nChecking streaming parameter usage..." rg -l "streaming.*=.*ctx\.data\[.streaming.\]" # Check for run_agent method implementations echo -e "\nChecking run_agent implementations..." ast-grep --pattern 'def run_agent($$$, streaming: bool = False)'Length of output: 739
Script:
#!/bin/bash # Check the actual implementation of run_agent and streaming usage in each file echo "Financial Report Workflow Implementation:" rg -A 10 "def run_agent" templates/components/agents/python/financial_report/app/agents/workflow.py echo -e "\nBlog Workflow Implementation:" rg -A 10 "def run_agent" templates/components/agents/python/blog/app/agents/workflow.py echo -e "\nPlanner Workflow Implementation:" rg -A 10 "def run_agent" templates/components/multiagent/python/app/workflows/planner.py echo -e "\nStreaming Context Usage:" rg -B 2 -A 2 "streaming.*=.*ctx\.data\[.streaming.\]"Length of output: 3850
templates/components/agents/python/blog/app/agents/workflow.py (2)
Line range hint
244-262: LGTM! The streaming implementation looks solid.The addition of the streaming parameter with a default value of
Falseis well-implemented and consistent with the PR's objective of fixing UI streaming issues. The method properly handles both streaming and non-streaming cases.
Line range hint
1-262: Verify consistency with FinancialReportWorkflow changes.The AI summary mentions similar changes in
FinancialReportWorkflow, but this file isn't provided for review. Let's verify the consistency of streaming implementation across workflows.
templates/components/agents/python/financial_report/app/agents/workflow.py
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
streamingparameter, allowing for better control of agent execution in streaming mode.Bug Fixes
Documentation