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

Fix MistralAIMessage to handle "Tool" Output #850

Conversation

dkaplan88
Copy link
Contributor

Solves: #839

Breaks up hash by specific role following API Conventions

https://docs.mistral.ai/api/#tag/chat/operation/chat_completion_v1_chat_completions_post
Assistant
image

System
image

Tool
image

User
image

@@ -46,29 +46,10 @@ def llm?
# @return [Hash] The message as an MistralAI API-compatible hash
def to_hash
Copy link
Collaborator

@andreibondarev andreibondarev Oct 20, 2024

Choose a reason for hiding this comment

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

What're your thoughts on doing something like this:

def to_hash
  if assistant?
    to_assistant_hash
  elsif tool?
    to_tool_hash
  ...
end

def to_assistant_hash
  h = {
    role: role,
    content: to_content
  }
  h[:tool_calls] = tool_calls if tool_calls.any?
  h
end

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's pretty similar to what you've got just slightly different stylistically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I am very on the fence about it as one hash or hash by role.

I think declaring the separation by role makes sense since it feels like Mistral has declared separate API Contracts by role (as we see with content) 😄

I updated it to do it by role and refactored the content hash so it was just building the array value.

@andreibondarev
Copy link
Collaborator

@dkaplan88 Thank you! Great work!

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