Migrates to langgraph #165
Conversation
…'s not a tool call
… Tests run with as_graph.
| @@ -73,13 +77,17 @@ def add_messages(self, messages: Sequence[BaseMessage]) -> None: | |||
| messages: A list of BaseMessage objects to store. | |||
| """ | |||
| with transaction.atomic(): | |||
| existing_messages = self.get_messages() | |||
|
|
|||
| messages_to_create = [m for m in messages if m not in existing_messages] | |||
There was a problem hiding this comment.
how is the not in making the equality for messages? do you know?
There was a problem hiding this comment.
Good point, it worked but I didn't check why it was working, perhaps we should force comparing by id to make this safer and more explicit?
There was a problem hiding this comment.
I think so, but does the messages Langchain passes as params contain any ids?
Also, we can optimize this a bit by querying with ids.
There was a problem hiding this comment.
The langchain ones receive a uuid id that is added in the built in add_messages function that is called before calling this one and they are update here to the db ids a few lines bellow this.
There was a problem hiding this comment.
I've made the change to compare ids and optimized the query to only return the ids
| assert ( | ||
| response_message.content == "The current temperature in Recife today is 32 degrees Celsius." | ||
| ) | ||
| assert response_message.id == str(messages_ids[1]) |
There was a problem hiding this comment.
I preferred the old approach for readability. If you had trouble with the diffs, it's possible to change pytest to include all the diff.
There was a problem hiding this comment.
Yes, it was better, the problem is that in the new version of langchain-core there's a bunch of metadata (see example bellow) that is not relevant to tests added to messages:
There was a problem hiding this comment.
Got it! We could use something like this: https://kalnytskyi.com/posts/assert-str-matches-regex-in-pytest/
In this case, it would be a comparator that always returns true.
But that's a nitpick.
This PR sunsets the previous use of chains in favor of the more modern, more readable and simpler to use langgraph approach. The migration was done trying to keep the changes as minimal as possible and the both methods
as_chainandas_graphare still available. For nowas_chainstill the default approach but it can be changed by changing the settingAI_ASSISTANT_USE_LANGGRAPHtoTrue. After this changes have been field tested we will sunset the oldas_chainapproach. Tests are already running withAI_ASSISTANT_USE_LANGGRAPHset to true. During this migration only 5 tests broke because it was required rerunning the cassettes due to small changes in the setup of how messages are organized in the prompt.