Skip to content
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

Support async nested chats #3289

Closed
wants to merge 10 commits into from
Closed

Support async nested chats #3289

wants to merge 10 commits into from

Conversation

heyitsaamir
Copy link
Collaborator

@heyitsaamir heyitsaamir commented Aug 3, 2024

In this PR:

  1. We add a new use_async flag to register_nested_chats function. This allows the nested chats to internally run a_initiate_chats instead of initiate_chats.
  2. Added a number of non-open-ai dependent tests for nested chats.

Why are these changes needed?

We support nested chats and async initiate_chats. However, if a chat is started asynchronously, nested chats still currently run in-process.

Related issue number

Closes #3286

Checks

@qingyun-wu
Copy link
Contributor

@heyitsaamir, thanks for the contribution! I know this is still a draft PR, but several reminders:

  • Could you first fix the code format issues?
  • Tests should be added to cover the added code.

@heyitsaamir
Copy link
Collaborator Author

@heyitsaamir, thanks for the contribution! I know this is still a draft PR, but several reminders:

  • Could you first fix the code format issues?
  • Tests should be added to cover the added code.

Yep. Working on tests. I am surprised there are no existing tests for nested chats.

@heyitsaamir heyitsaamir changed the title Async nested chat Support async nested chats Aug 5, 2024
@heyitsaamir heyitsaamir marked this pull request as ready for review August 5, 2024 19:32
@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2024

Codecov Report

Attention: Patch coverage is 5.00000% with 38 lines in your changes missing coverage. Please review.

Project coverage is 18.48%. Comparing base (6279247) to head (cd454e4).
Report is 69 commits behind head on main.

Files Patch % Lines
autogen/agentchat/conversable_agent.py 5.00% 38 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3289       +/-   ##
===========================================
- Coverage   32.90%   18.48%   -14.42%     
===========================================
  Files          94      103        +9     
  Lines       10235    10932      +697     
  Branches     2193     2498      +305     
===========================================
- Hits         3368     2021     -1347     
- Misses       6580     8709     +2129     
+ Partials      287      202       -85     
Flag Coverage Δ
unittests 18.45% <5.00%> (-14.46%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@heyitsaamir
Copy link
Collaborator Author

heyitsaamir commented Aug 6, 2024

Closing in favor of #3309

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Nested chats should be able to run asynchronously
3 participants