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

Added ability to specify 'role' field for select speaker messages for Group Chats (Replaces PR #2167) #2199

Merged
merged 8 commits into from
Mar 31, 2024

Conversation

marklysze
Copy link
Collaborator

Note: This replaces #2167 due to it being based on an older version of main.

Why are these changes needed?

As per feature request #1861 ("[Feature Request]: Allowing user to specify the "role" field for select speaker messages"), this PR includes changes to the GroupChat constructor to allow the user to specify the role for the select speaker messages. This addresses the issue of role names when using Mistral through Mistral.AI's API.

Changes:

  • Added role_for_select_speaker_messages to GroupChat constructor in groupchat.py
  • Added @ekzhu's Mistral.AI notebook to documentation under Non-OpenAI models section
  • Added @ekzhu's updates to the Using Non-OpenAI about page
  • Added a Tips for Non-OpenAI models page to documentation (should be a basis for future tips)
  • Added a sub-section on the Conversation Patterns tutorial explaining the new attribute and provided an example

Related issue number

Addresses #1861

Checks

Copy link
Collaborator

@ekzhu ekzhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests to check if the select speaker messages are indeed following the role specified

marklysze and others added 4 commits March 30, 2024 06:23
…models.md

Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
Updated to include note about being applicable when auto.
@marklysze
Copy link
Collaborator Author

Unit tests to check if the select speaker messages are indeed following the role specified

Okay, I'll add to tests

@marklysze
Copy link
Collaborator Author

Unit tests to check if the select speaker messages are indeed following the role specified

I've added a test for this. I've also updated groupchat.py (sorry @jackgerrits!) to include a check that role_for_select_speaker_messages isn't blank or None.

@marklysze
Copy link
Collaborator Author

I've updated the tests to use pytest instead of try-except (as per my other PR).

autogen/agentchat/groupchat.py Show resolved Hide resolved
@sonichi
Copy link
Contributor

sonichi commented Mar 31, 2024

Could you please fix the conflicts? Thanks.

@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.73%. Comparing base (989c182) to head (0a2b4d7).

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2199       +/-   ##
===========================================
+ Coverage   37.83%   49.73%   +11.90%     
===========================================
  Files          77       77               
  Lines        7766     7769        +3     
  Branches     1663     1801      +138     
===========================================
+ Hits         2938     3864      +926     
+ Misses       4579     3583      -996     
- Partials      249      322       +73     
Flag Coverage Δ
unittest 14.36% <33.33%> (?)
unittests 48.66% <100.00%> (+10.84%) ⬆️

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.

@sonichi sonichi added this pull request to the merge queue Mar 31, 2024
Merged via the queue into microsoft:main with commit 3f63db3 Mar 31, 2024
60 of 72 checks passed
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
… Group Chats (Replaces PR microsoft#2167) (microsoft#2199)

* Re-commit of code from PR (microsoft#2167) addressing microsoft#1861, due to wrong basing

* Update website/docs/topics/non-openai-models/best-tips-for-nonopenai-models.md

Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>

* Removed unnecessary notebook images

* Update conversation-patterns.ipynb

Updated to include note about being applicable when auto.

* Updated to include checks that the role is not blank/None. Added tests.

* Changed try-except to use pytest

---------

Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
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.

5 participants