-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: add streaming chunk events #3403
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: master
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
|
@JINO-ROHIT thanks for contribution! |
fengju0213
left a comment
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.
Thanks for working on this feature! The overall direction is correct, but there are some critical issues that need to be addressed before this can work properly.
| chunk=chunk.msg.content, | ||
| chunk_index=chunk_index, | ||
| ) | ||
| if ( |
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.
here assumes self.callback exists in SingleAgentWorker, but it doesn't. Workers have no way to access the callbacks registered in Workforce.
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.
maybe we need add a mechanism to propagate callbacks from Workforce to Worker instances.
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.
For the worker callback, I suggest implementing callback support at the BaseNode level, since all node types (both Worker and Workforce) could benefit from this change.
However, it might be good to get some feedback from @Wendong-Fan on this idea.
| ): | ||
| try: | ||
| await ( | ||
| self.callback.log_task_streaming_chunk( |
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.
log_task_streaming_chunk is defined as a synchronous method in WorkforceCallback
|
yeah thanks, i suppose we wait till wendong gives some comments then, ill also take some time to come up w alts, |
im unaware if this works correctly but figured it might be a good idea to get early feedback -