[Core]: On Tool End Obersvation Casting Fix

[keenborder786]

Fixes the following issue #18760 raised by @eyurtsev

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
langchain	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 11, 2024 2:55pm

[eyurtsev]

This might break a bunch of user code -- I'm going to scope out how to do this without introducing breaking changes

[keenborder786]

okay, please let me know about the scope so I can integrate that within this PR. Thank you. @eyurtsev

[eyurtsev]

Keep PR open for now in case we decide to make a breaking here

[eyurtsev]

OK I think we can actually do this! It's a breaking change, but not one that I think will affect many users, and likely better to do this than to implement a work-around.

The remaining changes we need to make:

1. git grep on_tool_end across the code base to find all tracer/callback handlers
2. update the type signature from str to Any
3. Add a cast to string inside the handler itself

    def on_tool_end(self, output: str, *, run_id: UUID, **kwargs: Any) -> Run:
        """End a trace for a tool run."""

to

    def on_tool_end(self, output: Any, *, run_id: UUID, **kwargs: Any) -> Run:
        """End a trace for a tool run."""
        output = str(output) # Cast to maintain logic unchanged

--

We can then follow up with another PR to actually fix astream_events:

1. Add a unit test that includes the snippet of code I posted in the original issue -- goes here:

   langchain/libs/core/tests/unit_tests/runnables/test_runnable_events.py

   Line 48 in bc6249c

   for event in events_:

2. Remove the cast to str in BaseTracer on_tool_end (that this PR will introduce)

   langchain/libs/core/langchain_core/tracers/base.py

   Line 507 in bc6249c

   def on_tool_end(self, output: str, *, run_id: UUID, **kwargs: Any) -> Run:

3. Celebrate!

[adreo00]

@keenborder786 @eyurtsev looks like the same str casting appears elsewhere (in the arun method). Should that be lumped into this fix as well?

langchain/libs/core/langchain_core/tools.py

Line 501 in 62ba5e1

str(observation), color="red", name=self.name, **kwargs

and

langchain/libs/core/langchain_core/tools.py

Line 509 in 62ba5e1

str(observation), color=color, name=self.name, **kwargs

[eyurtsev]

Yeah we should get both sync and async methods good catch @adreo00

[eyurtsev]

@keenborder786 let me know if you're OK taking this on or want me to commandeer! Would be nice to merge a fix next week :)

[keenborder786]

Ok. Yes I am taking this. Get Outlook for Android<https://aka.ms/AAb9ysg>

…

________________________________ From: Eugene Yurtsev ***@***.***> Sent: Friday, March 8, 2024 11:38:06 PM To: langchain-ai/langchain ***@***.***> Cc: Mohammad Mohtashim Khan ***@***.***>; Mention ***@***.***> Subject: Re: [langchain-ai/langchain] [Core]: On Tool End Obersvation Casting Fix (PR #18798) CAUTION: This email has originated from outside of LUMS. Do not click links or open attachments unless you recognize sender’s email address and know the content is safe. @keenborder786<https://github.com/keenborder786> let me know if you're OK taking this on or want me to commandeer! Would be nice to merge a fix next week :) — Reply to this email directly, view it on GitHub<#18798 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKZFN6YXOFG7FVU6N7D2J3DYXIAQ5AVCNFSM6AAAAABEM3QLNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBWGIYTGMRWG4>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[keenborder786]

@eyurtsev please review.

[keenborder786]

I will create the second PR for tests that you have mentioned after you have given a green light for this one..

[eyurtsev]

Looks good! On mobile right now, so won't merge until at my laptop later tonight.

[eyurtsev]

Had no time over the weekend, taking a look now