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.

Sign up for GitHub

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

Jump to bottom

fix generate_reply when sender is None. #1186

Merged
merged 9 commits into from
Aug 25, 2023
Merged

fix generate_reply when sender is None. #1186

merged 9 commits into from
Aug 25, 2023

Conversation

yiranwu0
Copy link
Contributor

Why are these changes needed?

generate_reply allows either messages or sender is None. But current code doesn't handle situation when sender is None. This PR fixes this bug.

Related issue number

Checks

sonichi
sonichi reviewed Aug 18, 2023
View reviewed changes
Copy link
Collaborator

@sonichi sonichi left a comment

Choose a reason for hiding this comment

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

It's better to provide a use case for sender=None to justify this change, and a test case.

flaml/autogen/agentchat/responsive_agent.py Outdated Show resolved Hide resolved
@yiranwu0
Copy link
Contributor Author

How about this? _match_trigger accepts None, and we register the default generate replies with trigger [Agent, None].

sonichi
sonichi reviewed Aug 20, 2023
View reviewed changes
Copy link
Collaborator

@sonichi sonichi left a comment

Choose a reason for hiding this comment

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

This way works.

flaml/autogen/agentchat/responsive_agent.py Show resolved Hide resolved
test/autogen/agentchat/test_responsive_agent.py Outdated Show resolved Hide resolved
test/autogen/agentchat/test_responsive_agent.py Outdated Show resolved Hide resolved
@yiranwu0 @sonichi
Update test/autogen/agentchat/test_responsive_agent.py
f609a27 
Co-authored-by: Chi Wang <wang.chi@microsoft.com>
@yiranwu0 @sonichi
Update test/autogen/agentchat/test_responsive_agent.py
49c3d8e 
Co-authored-by: Chi Wang <wang.chi@microsoft.com>
@yiranwu0 yiranwu0 requested a review from sonichi August 22, 2023 00:58
sonichi
sonichi reviewed Aug 22, 2023
View reviewed changes
Copy link
Collaborator

@sonichi sonichi left a comment

Choose a reason for hiding this comment

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

almost ready.

flaml/autogen/agentchat/responsive_agent.py Outdated Show resolved Hide resolved
@yiranwu0 @sonichi
Update flaml/autogen/agentchat/responsive_agent.py
fd1aa18 
Co-authored-by: Chi Wang <wang.chi@microsoft.com>
@yiranwu0 yiranwu0 temporarily deployed to openai August 22, 2023 14:54 — with GitHub Actions Inactive
@yiranwu0 yiranwu0 requested a review from sonichi August 22, 2023 18:27
@yiranwu0 yiranwu0 temporarily deployed to openai August 22, 2023 23:11 — with GitHub Actions Inactive
@yiranwu0 yiranwu0 temporarily deployed to openai August 22, 2023 23:11 — with GitHub Actions Inactive
@yiranwu0 yiranwu0 temporarily deployed to openai August 22, 2023 23:11 — with GitHub Actions Inactive
@sonichi sonichi added this pull request to the merge queue Aug 25, 2023
Merged via the queue into main with commit 87c2361 Aug 25, 2023
16 checks passed
@sonichi sonichi deleted the fixgenreply branch August 25, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sonichi sonichi sonichi approved these changes

@skzhang1 skzhang1 skzhang1 approved these changes

@qingyun-wu qingyun-wu Awaiting requested review from qingyun-wu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@yiranwu0 @sonichi @skzhang1