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

adding tools to MistralChatGenerator - based on OpenAIChatGenerator #1358

Merged
merged 15 commits into from
Feb 10, 2025

Conversation

davidsbatista
Copy link
Contributor

Related Issues

Proposed Changes:

  • mostly based on OpenAIChatGenerator code

How did you test it?

Checklist

@github-actions github-actions bot added integration:mistral type:documentation Improvements or additions to documentation labels Feb 6, 2025
@davidsbatista davidsbatista marked this pull request as ready for review February 7, 2025 11:29
@davidsbatista davidsbatista requested a review from a team as a code owner February 7, 2025 11:29
@davidsbatista davidsbatista requested review from anakin87 and vblagoje and removed request for a team February 7, 2025 11:29
message = results["replies"][0]

assert message.tool_calls
tool_call = message.tool_call
Copy link
Member

Choose a reason for hiding this comment

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

we discussed about the removal of assert not message.text...

Which kind of text does the model produce along with a tool call?
Is it expected? I can't find it mentioned in [Mistral docs], but I haven't studied the topic in depth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm still trying to understand how the OpenAIChatGeneration processes the output of the MistralAPI and builds a ChatMessage object - there are too many details here that I'm out of context.

I can't figure out what kind of texts are supposed to come from the Response Sample on this page

Copy link
Member

Choose a reason for hiding this comment

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

@vblagoje please re-add assert not message.texts here if applicable

@vblagoje
Copy link
Member

vblagoje commented Feb 7, 2025

I asked @davidsbatista to complete this one @anakin87 - he allowed me, should have PR ready by EOD

@@ -54,10 +60,11 @@ class MistralChatGenerator(OpenAIChatGenerator):
def __init__(
self,
api_key: Secret = Secret.from_env_var("MISTRAL_API_KEY"),
model: str = "mistral-tiny",
model: str = "mistral-small-latest",
Copy link
Member

Choose a reason for hiding this comment

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

if there is a good reason to change the default model, let's mark this PR as breaking ("!" in the title).
In any case, the title needs to be aligned with the common format.

Copy link
Member

Choose a reason for hiding this comment

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

ok yeah, will do mark it as breaking, couldn't find this model listed any more nor supporting FC, see https://docs.mistral.ai/capabilities/function_calling/

Copy link
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

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

  • Please address the 2 small comments I've left.
  • Change the title to reflect our conventions.

Then feel free to merge!

message = results["replies"][0]

assert message.tool_calls
tool_call = message.tool_call
Copy link
Member

Choose a reason for hiding this comment

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

@vblagoje please re-add assert not message.texts here if applicable

tool_calls.append(ToolCall(id=payload["id"], tool_name=payload["name"], arguments=arguments))
except json.JSONDecodeError:
logger.warning(
"OpenAI returned a malformed JSON string for tool call arguments. This tool call "
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"OpenAI returned a malformed JSON string for tool call arguments. This tool call "
"Mistral returned a malformed JSON string for tool call arguments. This tool call "

@vblagoje vblagoje merged commit 00e4cc5 into main Feb 10, 2025
11 checks passed
@vblagoje vblagoje deleted the adding-tools-to-mistral branch February 10, 2025 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration:mistral type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mistral ChatGenerator- support for Tool
3 participants