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

community: avoid double templating in langchain_prompty #25777

Merged
merged 6 commits into from
Aug 28, 2024

Conversation

matthieucx
Copy link
Contributor

@matthieucx matthieucx commented Aug 27, 2024

Description

In langchain_prompty, messages are templated by Prompty. However, a call to ChatPromptTemplate was initiating a second templating. We now convert parsed messages to Message objects before calling ChatPromptTemplate, signifying clearly that they are already templated.

We also revert #25739 , which applied to this second templating, which we now avoid, and did not fix the original issue.

Issue

Closes #25703

@efriis efriis added the partner label Aug 27, 2024
Copy link

vercel bot commented Aug 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Aug 27, 2024 5:46pm

@efriis efriis self-assigned this Aug 27, 2024
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. community Related to langchain-community 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs labels Aug 27, 2024
@matthieucx
Copy link
Contributor Author

Not relevant to the core logic, but I hesitated betwen sharing a dict for the mapping between <Role>Message classes and Prompty roles, and wrapping it with a class, since we only need the mapping and nothing else.

I went with the latter, but if anybody has pointers on what would be the cleanest here I'd be happy to learn.

Prompty templates the given `.prompty` file and parse it for messages.
We convert those messages to langchain's Message objects.
This avoids the second templating in the call to ChatPromptTemplate.
from pydantic import BaseModel

from .core import Invoker, Prompty, SimpleModel


class RoleMap:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this cover the full scope of roles? What about "tool"?

Copy link
Contributor Author

@matthieucx matthieucx Aug 28, 2024

Choose a reason for hiding this comment

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

Yes, I simply adapted the roles that were defined when integrating Prompty into LangChain. The latest versions of Prompty removed ai and human, so we could even delete those (but that would break some users' prompts)

Tools are defined in the YAML header of the .prompty files, here's an example


lc_messages.append(
MessagesPlaceholder(
variable_name=input_name_agent_scratchpad, optional=True
) # type: ignore[arg-type]
)
lc_p = ChatPromptTemplate.from_messages(
lc_messages, template_format=template_format
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you clarify why specifying template_format with a default of "f-string" is breaking? It looks like that is the default in ChatPromptTemplate.from_messages:

def from_messages(
cls,
messages: Sequence[MessageLikeRepresentation],
template_format: Literal["f-string", "mustache", "jinja2"] = "f-string",
) -> ChatPromptTemplate:

Or is it just unrelated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right that's it's unrelated.

Here the issue is that before initializing the ChatPromptTemplate, Prompty has already parsed and templated the prompt using the user input.

When invoking ChatPromptTemplate, it tries to template the messages and interprets text inside { } (or {{ }}) as missing variables. The fix proposed in this PR avoids this templating by converting to Messages objects, which are already templated.

Since we skip this second templating entirely, it does not make sense to expose one of its parameters to the user, as it won't have any effect. You can select the template engine used by Prompty directly in the .promptys file YAML header.

@ccurme ccurme assigned ccurme and unassigned efriis Aug 28, 2024
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Aug 28, 2024
@ccurme ccurme merged commit 783397e into langchain-ai:master Aug 28, 2024
16 checks passed
@matthieucx
Copy link
Contributor Author

Thanks for merging and for the time you spent on this!

@ccurme
Copy link
Collaborator

ccurme commented Aug 29, 2024

Thanks for merging and for the time you spent on this!

Thank you for the fix! Released in langchain-prompty 0.0.3, feel free to take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Related to langchain-community lgtm PR looks good. Use to confirm that a PR is ready for merging. 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs partner size:M This PR changes 30-99 lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

prompty: Superfluous f-string templating causes prompt templaing to fail
3 participants