Added ability to specify 'role' field for select speaker messages for Group Chats (addresses #1861) #2167
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hat in conversation patterns
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2167 +/- ##
===========================================
+ Coverage 37.98% 49.22% +11.23%
===========================================
Files 75 75
Lines 7589 7580 -9
Branches 1636 1766 +130
===========================================
+ Hits 2883 3731 +848
+ Misses 4463 3543 -920
- Partials 243 306 +63
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ekzhu
requested changes
Mar 28, 2024
| @@ -716,7 +716,7 @@ def _print_received_message(self, message: Union[Dict, str], sender: Agent): | |||
| if "function_call" in message and message["function_call"]: | |||
| function_call = dict(message["function_call"]) | |||
| func_print = ( | |||
| f"***** Suggested function call: {function_call.get('name', '(No function name found)')} *****" | |||
| f"***** Suggested function Call: {function_call.get('name', '(No function name found)')} *****" | |||
There was a problem hiding this comment.
Suggested change
| f"***** Suggested function Call: {function_call.get('name', '(No function name found)')} *****" | |
| f"***** Suggested function call: {function_call.get('name', '(No function name found)')} *****" |
| @@ -728,7 +728,7 @@ def _print_received_message(self, message: Union[Dict, str], sender: Agent): | |||
| iostream.print(colored("*" * len(func_print), "green"), flush=True) | |||
| if "tool_calls" in message and message["tool_calls"]: | |||
| for tool_call in message["tool_calls"]: | |||
| id = tool_call.get("id", "No tool call id found") | |||
| id = tool_call.get("id", "(No id found)") | |||
There was a problem hiding this comment.
Suggested change
| id = tool_call.get("id", "(No id found)") | |
| id = tool_call.get("id", "No tool_call_id found") |
| @@ -1311,12 +1311,6 @@ def _generate_oai_reply_from_client(self, llm_client, messages, cache) -> Union[ | |||
| ) | |||
| for tool_call in extracted_response.get("tool_calls") or []: | |||
| tool_call["function"]["name"] = self._normalize_name(tool_call["function"]["name"]) | |||
| # Remove id and type if they are not present. | |||
There was a problem hiding this comment.
These were for Mistral AI compatibility. Are these not needed now?
3 tasks
|
Closing this PR as it was based on an older version of main. New PR #2199 |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 31, 2024
… Group Chats (Replaces PR #2167) (#2199) * Re-commit of code from PR (#2167) addressing #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>
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
role_for_select_speaker_messagestoGroupChatconstructor ingroupchat.pyRelated issue number
Addresses #1861
Checks