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

langchain_core/langchain_community: add ControlMessage to langchain_core and enable passing messages with 'control' role through ChatOllama #30147

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rylativity
Copy link

  • Description: this creates a ControlMessage class in langchain_core for messages with the 'control' role, and allows passing messages with 'control' role through to ollama through ChatOllama. The update to langchain_ollama’s ChatOllama relies on the added ControlMessage class in langchain_core, so updates to both libs are included in this PR

  • Issue: Granite 3.2 Thinking #30122

  • Dependencies: no new dependencies

  • PR title:

  • PR message:

  • Lint and test: format, lint, and test completed and passing in langchain_core and langchain_community.

Ryan Stewart added 2 commits March 6, 2025 22:16
Copy link

vercel bot commented Mar 7, 2025

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 Mar 9, 2025 3:41pm

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. community Related to langchain-community labels Mar 7, 2025
Copy link
Collaborator

@ccurme ccurme left a comment

Choose a reason for hiding this comment

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

Several issues with this:

  1. It's unlikely we are going to add a new message type to core until multiple large providers adopt something similar.
  2. We should be updating langchain-ollama in libs/partners/ollama instead of the community implementation, which is deprecated (you should be seeing warning messages if you are using it).

Are you able to use use ChatMessage with a custom role for this instead?

@rylativity
Copy link
Author

rylativity commented Mar 9, 2025

@ccurme a ChatMessage with a custom role can potentially work, but there is currently an incompatibility between the type of the role attribute in the ChatMessage class (which is a str) and the type of the role variable in ChatOllama's _convert_messages_to_ollama_messages method (which is a Literal)

Also, ChatOllama's _convert_messages_to_ollama_messages does not currently have an isinstance check for ChatMessages, so if you pass a ChatMessage to ChatOllama it raises an exception.

So in order to go that route, 1) the _convert_messages_to_ollama_messages method would need to be updated to handle ChatMessages like so:

...
 elif isinstance(message, ChatMessage):
    role = message.role
...

and 2) either the ChatMessage's role attribute would need to be changed to a literal (probably not ideal because that takes away its flexibility), or the "role" variable's type in _convert_messages_to_ollama_messages would need to be updated to accept the str type role attribute of a ChatMessage instance:

...
role: Literal["user", "assistant", "system", "tool", "control"] | str
...

The ChatMessage can technically work after addressing item 1) above, but the mypy typechecking will fail without implementing something like item2) above.

If you are ok with this proposed route @ccurme (open to any feedback or alternative suggestions), and if that would satisfy your requirements/use-case @lemassykoi, I am happy to make those changes and open a new PR.

@ccurme
Copy link
Collaborator

ccurme commented Mar 9, 2025

Very much appreciate the detailed plan! Let's go for it, we can work around / document the issues with role.

@rylativity
Copy link
Author

New PR opened here - #30191 - much cleaner.

We can go ahead and close this in favor of #30191

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Related to langchain-community size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants