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

Skip adding empty content from gemini #768

Merged
merged 5 commits into from
Nov 19, 2024

Conversation

gabrielibagon
Copy link
Contributor

This PR fixes errors where the user adds an empty turn in a multi-turn conversation request, causing the following API error message:

InvalidArgument: 400 Please use a valid role: user, model.

Gemini sometimes returns an empty content during multi-turn conversations, particularly in response to a function result in the previous turn.

As a workaround, we can just skip adding this turn and proceed with the next user turn, which should not affect the correctness of the example unless the missing turn was a function call. But generally, it seems that the missing turn is the optional natural language response to the function response.

Copy link
Collaborator

@HuanzhiMao HuanzhiMao left a comment

Choose a reason for hiding this comment

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

Hey @gabrielibagon ,
Thanks for the PR.
I think it might be better if we handle it similar to how we do it in the prompting mode?
Because if the model didn't output any response, then in your implementation, the chat history would be like user, model, tool, user, model..., which is not ideal because each tool message should have a model response before the next user query to make it complete; it should be like user, model, tool, model, user, model. I will submit a PR to your branch with my proposed implementation. Let me know what you think.

@gabrielibagon
Copy link
Contributor Author

@HuanzhiMao That approach looks good, and I agree that it is good to align with the prompting strategy. I've merged your addition to this PR. Thank you!

Copy link
Collaborator

@HuanzhiMao HuanzhiMao left a comment

Choose a reason for hiding this comment

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

LGTM

@HuanzhiMao HuanzhiMao merged commit 5df6372 into ShishirPatil:main Nov 19, 2024
HuanzhiMao added a commit that referenced this pull request Dec 7, 2024
This PR updates the leaderboard to reflect the change in score due to
the following PR merge:

1. #747 
2. #770 
3. #768 
4. #750 
5. #763 
6. #772 
7. #777 
8. #778 
9. #786 
10. #787 
11. #697 
12. #718 
13. #755 
14. #796 
15. #789 
16. #804 
17. #808 
18. #809
19. #811 
20. #810 

Models were evaluated using checkpoint commit d7e52e5.
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.

2 participants