Open B-Step62 opened 2 months ago
I can see the reason for the difference
branch for list of handlers here both the handlers and the inheritable handlers are set to be the list of handlers
branch for callback manager here the inheritable handlers are set to be the inheritable handlers of the callback manager, which is nothing
step invoke when the step (prompt, llm, etc) is invoked, only the inheritable handlers are passed to them.
Having said that, I don't know what is the proper fix for it
In fact, maybe there is nothing to fix here. the guide here pass in a list of handlers and it is outputting "chain start" when every step (prompt, llm, etc) is invoked
Thank you for the investigation, @wulifu2hao.
Yes the a list of handlers works fine, but I don't see why the callback manger branch is implemented in the way. The logic ignores the inheritable_callbacks
passed to _configure
function at all, which doesn't seem to be correct, because it is no longer "inheritable" then. The L2149 should merge the inheritable_callbacks
and callback_manager.inheritable_callbacks
instead. Or was there any particular reason not doing so?
Not sure if I understand it correctly though...
My understanding is that the constructor of CallbackManager takes in the parameter inheritable_callbacks
which seems to mean "the callback manager or list of callbacks to inherit from when constructing this new callback manager".
In this sense, line 2148,2149 seems to be doing the right thing: it copies the handlers
as well as the inheritable_handlers
when constructing the new callback manager
@wulifu2hao Sorry it seems I misunderstood a bit. So what _configure
does now is:
inheritable_callbacks
=> Inheritedcallback_manager.inheritable_handlers
passed via inheritable_callbacks
=> Inheritedcallback_manager.handlers
passed via inheritable_callbacks
=> Not inheritedI think what confuses me the most is that 3rd case. The guide mentions that the callbacks passed via callbacks
kwargs will be propagated to all nested objects. It doesn't explain the difference of handlers
and inheritable_handlers
field of callback manager.
But before concluding it to documentation problem, I'm curious if BaseCallbackManager
is not designed for users to passing their callbacks? In other words, is it only used internally to pass around callbacks, and users should only pass list of callbacks in their code?
If that's the case, I agree that we don't need any fix. I just used BaseCallbackManager
follows some internet example, but it might not be intended usage.
I doubt that BaseCallbackManager is for internal only because the callbacks in RunnableConfig is defined to be union of list of handlers and BaseCallbackManager , so it should be legitimate for the user to use it as the callbacks of RunnableConfig
Maybe what is lacking is a clear explanation of what the fields mean in the doc ?
Makes sense. Ok then the action here is to add a note for the behavior in the callback guide. I can make the update in a couple of days. How does it sound? @wulifu2hao
Checked other resources
Example Code
Setup:
Invoking with a list of callbacks => chain events print three times per each.
Invoking with a callback manager => chain events print only once
Error Message and Stack Trace (if applicable)
NA
Description
When passing callbacks to the runnable's
.invoke
method, there are two ways to do that:model.invoke("Hi", config={"callbacks": [CustomCallbackHandler()]})
model.invoke("Hi", config={"callbacks": BaseCallbackManager([CustomCallbackHandler()])})
However, the behavior is different between two. The former triggers the handler more times then the latter.
System Info
System Information
Package Information
Packages not installed (Not Necessarily a Problem)
The following packages were not found: