[BUG] Nasty side effect in hook before_cat_sends_message

[pieroit]

If you call cat.llm from within the before_cat_sends_message hook, model interactions are not saved in the final CatMessage.why.model_interactions. Example:

@hook
def before_cat_sends_message(mex, cat):

    joke = cat.llm("Tell me a joke about da police")
    mex.text += joke

    return mex

The LLM is actually used and the joke added to the reply, but since MadHatter.execute_hook stores the original CatMessage and passes to each hook a deepcopy of it, when the second is returned and overwritten on the first, some info gets lost.

This only happens in hooks having side effects on the same data structure passed by argument to the hook.
The example above does not incur in the same bug with other hooks.

A possible solution is to avoid deepcopying the hook argument?
It was there to avoid side effects in the first place XD.

Also, this kind of stuff should be heavily tested, at the MadHatter class level tests.

Nasty one

[lucagobbi]

This is not related to deepcopy luckily, it is just a matter of order in the execution of the __build_why method in the __call__ method.

Are there any downside of having it at the end of __call__, after the execution of all hooks and other methods? Just like:

# ... rest of the call method code

# prepare final cat message
final_output = CatMessage(
    user_id=self.user_id, text=str(agent_output.output)
)

# run message through plugins
final_output = self.mad_hatter.execute_hook(
    "before_cat_sends_message", final_output, cat=self
)

# why this response?
why = self.__build_why()
# TODO: should these assignations be included in self.__build_why ?
why.intermediate_steps = agent_output.intermediate_steps
why.agent_output = agent_output.model_dump()

final_output.why = why

# update conversation history (AI turn)
self.working_memory.update_history(
    final_output
)

return final_output

[pieroit]

Thanks @lucagobbi I think you're right on the root cause.

The downside of building the why afterwards is that you cannot access the full CatMessage just before it goes out, which is the main purpose of this hook. For example, you may want to delete the why altogether or modifying it.

Maybe building the why directly when CatMessage is initialized, and populate it along the main pipeline? That also is problematic because any change to working memory should be reflected on the CatMessage

Still a very nasty issue
Unless we accept that recalls and model calls in before_cat_sends_message are not reported in why