-
-
Notifications
You must be signed in to change notification settings - Fork 0
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(autofix): Reproduction in root causes #1067
Conversation
e23ac2f
to
8e49488
Compare
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.
Just some thoughts on code organization/cleanliness
def clean_tool_call_assistant_messages(self, messages: list[Message]) -> list[Message]: | ||
new_messages = [] | ||
for message in messages: | ||
if message.role == "assistant" and message.tool_calls: | ||
new_messages.append( | ||
Message(role="assistant", content=message.content, tool_calls=[]) | ||
) | ||
elif message.role == "tool": | ||
new_messages.append(Message(role="user", content=message.content, tool_calls=[])) | ||
else: | ||
new_messages.append(message) | ||
return new_messages | ||
|
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.
On this I meant not just as a helper function, but if any messages passed into the client for any completion can be cleaned behind-the-scenes to prevent the errors. That way external code doesn't have to choose whether or not to clean.
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 don't think that's a good idea, given that openai can expect a tool call and its response to be used in a specific way for the LLM input. We should only use this workaround sparingly, in this case, only for the formatter.
Eval results:
(Before is above, after is below)