-
Notifications
You must be signed in to change notification settings - Fork 16
feat: add Jinja2 ChatMessage extension #297
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
base: main
Are you sure you want to change the base?
Conversation
709e608
to
697e3e1
Compare
Pull Request Test Coverage Report for Build 14929353715Details
💛 - Coveralls |
### How it works | ||
1. The {% message %} tag is used to define a chat message. | ||
2. The message can contain text and other structured content parts. | ||
3. To include a structured content part in the message, the `| for_template` filter is used. |
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.
Also more of just a question. Why the name for_template
? I'm not yet quite understanding what it does based on just the name.
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.
Good point. I would like a short but descriptive name.
Other ideas: content_part
/prepare_content_part
/mark_content_part
/format_content_part
; something related to "including".
Do you have any other ideas?
(We can also involve some other team members if necessary)
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.
Yeah I'm not entirely sure either. I'd be happy to have other team members give their input
@anakin87 Overall looks really good! I have some some comments mostly surrounding tests and error handling but the templating language itself looks very solid! |
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.
Everything looks good! I don't think the for_template
naming needs to block this. Will leave it up to you
Related Issues
ChatPromptBuilder
adaptation for multimodality haystack#9260: I'm not yet modifying theChatPromptBuilder
but introducing the foundations to make it workProposed Changes:
{% message %}
tag is used to define a chat message.| for_template
filter is used.The filter serializes the content part into a JSON string and wraps it in a
<haystack_content_part>
tag._build_chat_message_json
method of the extension parses the message content parts,converts them into a
ChatMessage
object and serializes it to a JSON string.ChatPromptBuilder
component, where templates are rendered to actualChatMessage
objects.How did you test it?
CI; several tests for the extension
Notes for the reviewer
ChatPromptBuilder
using this basis)Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
.