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: make llama.cpp Chat Generator compatible with new ChatMessage #1254

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

anakin87
Copy link
Member

@anakin87 anakin87 commented Dec 18, 2024

Related Issues

Proposed Changes:

  • fixes to ensure that the llama.cpp Chat Generator is compatible with both old and new ChatMessage

How did you test it?

CI; local tests with Haystack main branch

Checklist

Comment on lines +24 to +26
if formatted_msg["role"] == "tool":
formatted_msg["name"] = message.tool_call_result.origin.tool_name
formatted_msg["content"] = message.tool_call_result.result
Copy link
Member Author

Choose a reason for hiding this comment

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

starting from 2.9.0, Haystack function messages will be automatically converted to tool messages. This change ensures compatibility.

Comment on lines +141 to +145
if name:
if hasattr(reply, "_name"):
reply._name = name # new ChatMessage
elif hasattr(reply, "name"):
reply.name = name # legacy ChatMessage
Copy link
Member Author

Choose a reason for hiding this comment

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

once we release 2.9.0, we can get rid of this ugly check

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 after 2.9.0 release, we may consider to refactor for's code in a _create_reply method maybe.

@anakin87 anakin87 marked this pull request as ready for review December 18, 2024 16:46
@anakin87 anakin87 requested a review from a team as a code owner December 18, 2024 16:46
@anakin87 anakin87 requested review from mpangrazzi and removed request for a team December 18, 2024 16:46
@anakin87 anakin87 merged commit 1aba307 into main Dec 19, 2024
10 checks passed
@anakin87 anakin87 deleted the fix-llamacpp-new-chat-message branch December 19, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants