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.
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
feat: add web search #274
feat: add web search #274
Changes from 33 commits
fd98180
a19aba2
ce8a4a6
1ce288b
c593ad3
7f8450d
5193c01
1f75055
591fa47
a882e26
5f16411
b224c1d
9ecb3e4
51d9ca2
1cf40ff
f3f65dc
e8ba969
48515fc
eb9521c
f48f46c
25590ba
6444d85
13a3bb8
70c6ef6
685e7d4
a7d8dbe
a9435b2
5c826ba
db92799
4fc33f7
84f032c
a14213f
3e2514c
2f0dc19
6d9b1eb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Since we had this bug in the code, please add to the test of
ai_society\role_playing.py
a check of the sequence of roles:or whatever it must be. I expect added checks in
test/agents/test_role_playing.py
.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.
Wait a second. I recall figuring out this complex behavior of changing the roles. I even put an explicit comment:
also see the doctoring above:
Please let me know if you think this is in the scope of this change or not. If not, make a separate bug ticket and a separate PR with the proper fix and the test, and revert the irrelevant changes in thin PR.
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.
Wait. I don't think this is a bug.
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.
I am not sure why this change is needed. @Obs01ete @zhiyu-01 can you explain?
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.
The
assistant_msg
is the message to be sent to the user by the assistant agent. So I guess theself.user_sys_msg
is needed to be sent. Do you confirm this change is correct?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.
I have some doubts about this test. Should it be
['system', 'user', 'assistant', 'user', 'assistant', ...]
normally?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.
This is very important to have tested! I request this change in #298.