core[patch]: On Chain Start Fix for Chain Class
#26593
Conversation
keenborder786
commented
Sep 17, 2024
keenborder786
commented
- Issue: Issue with callback manager for verbose output #26588
dosubot
bot
added
the
size:XS
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
@eyurtsev please review |
dosubot
bot
added
langchain
There was a problem hiding this comment.
HI @keenborder786 thanks for the PR! this is not the correct fix. We removed the serialization representation on purpose. The tracer itself should be fixed to deal with no representation being passed.
|
@keenborder786 we'll also need a unit test to be added for the PR to be merged |
|
@eyurtsev okay understood, let me do it. |
dosubot
bot
added
size:S
dosubot
bot
added
size:M
|
@eyurtsev review please |
|
@eyurtsev I have implemented the changes |
eyurtsev
changed the title
Chain ClassChain Class
eyurtsev
changed the title
Chain ClassChain Class
| run_manager = callback_manager.on_chain_start( | ||
| None, | ||
| inputs, | ||
| run_id, | ||
| class_name=self.name, |
There was a problem hiding this comment.
This is a big decision!
on_chain_start here is an event dispatcher. By updating the information sent here the code sets the shape of dispatched on chain start events.
i.e., we should probably not be including class_name and lc_id in here, instead everything can be placed in the name logic if any additional attributes are needed
There was a problem hiding this comment.
I'm going to remove these additions since they're not necessary or desirable
| @@ -32,8 +32,14 @@ def on_chain_start( | |||
| inputs (Dict[str, Any]): The inputs to the chain. | |||
| **kwargs (Any): Additional keyword arguments. | |||
| """ | |||
| class_name = serialized.get("name", serialized.get("id", ["<unknown>"])[-1]) | |||
| print(f"\n\n\033[1m> Entering new {class_name} chain...\033[0m") # noqa: T201 | |||
| if "name" in kwargs: | |||
There was a problem hiding this comment.
We can fetch the name from kwargs here if present and otherwise fallback to serialized representation
dosubot
bot
added
the
lgtm
| @@ -0,0 +1,44 @@ | |||
| from typing import Any, Dict, List, Optional | |||
There was a problem hiding this comment.
awesome thank you for including the unit test
|
Should resolve: #26773 |
|
@eyurtsev sorry to ask, so which version should I use to have this fix in place? How do I figure out based on this PR? |
- **Issue:** langchain-ai#26588 --------- Co-authored-by: Eugene Yurtsev <eyurtsev@gmail.com>
