seratch / ChatGPT-in-Slack

Swift demonstration of how to build a Slack app that enables end-users to interact with a ChatGPT bot
MIT License
437 stars 162 forks source link

Add optional callback execution after function call #58

Open iwamot opened 1 year ago

iwamot commented 1 year ago

This PR introduces the concept of optional callbacks, named {original_function_name}_called, executed directly after a corresponding function call.

I envisage the following two use cases:

  1. File Upload: Pairing a create_image function with a create_image_called callback, we could handle the task of uploading the generated image to Slack.
  2. Retrieval-augmented Generation (RAG): If we pair a search_faq function with a search_faq_called callback, we could incorporate the search results into the user prompt.

I would appreciate your review and feedback on this implementation. Thank you for all your efforts in making this wonderful software available.

seratch commented 1 year ago

Hi @iwamot, thanks for the suggestion. I understand this could be valuable in certain use cases, however, I hesitate to add a custom property to OpenAI's function module data structure. Additionally, passing wip_reply to the callback could result in unexpected behavior. If this app allows external code to modify the wip_reply message text, blocks, or attachments, there may be conflicts between the modifications made by the callback and those made by this app's core logic. I'm happy to explore ways to offer more customization to developers, but this proposal seems to have a few issues that need to be resolved.

iwamot commented 1 year ago

Hi @seratch, thank you for your review. I fully concur with your feedback.

To be frank, the use cases I've outlined are the features I'm particularly eager to utilise myself. However, I also believe that their incorporation could significantly broaden the range of applications for this software.

At present, one improvement idea that comes to mind is passing thread_ts instead of wip_reply. If my idea is off the mark, my apologies.

seratch commented 1 year ago

Still thinking further about this... 🤔

Firstly, I believe that this project should avoid polluting OpenAI's functions data structure. Based on that premise, it could be still beneficial to explore introducing this app's custom configuration. This is just a random idea, but perhaps introducing an additional object like function_callbacks into the module file could be feasible:

function_callbacks = [
    {
        "function_name": "get_current_weather",
        "callback_type": "before",
        "callback_function_name": "before_get_current_weather",
        # parameters: client, context, messages, wip_reply
    },
    {
        "function_name": "get_current_weather",
        "callback_type": "after",
        "callback_function_name": "after_get_current_weather",
        # parameters: function_response, client, context, messages, wip_reply
    },
]

With this concept, developers are able to define callback functions in the module code in an even more flexible way. If we add something like this, the README, of course, should provide clear guidance on it, as it's entirely specific to this app.

However, I am yet to be confident that this would be a sufficiently good design in terms of the maintainability of an app. Modifying wip_reply (or calling chat.update, chat.postMessage in the thread, whatever), performing Slack Web API calls using client, and setting additional values in context - doing these actions in the module file could make the app's maintenance more challenging.

In conclusion, I am not yet convinced about adding what we're discussing here. If others have thoughts on this topic, please feel free to jump in!

iwamot commented 1 year ago

The introduction of function_callbacks strikes me as a very smart idea! On the other hand, I fully agree with your concern that this feature might complicate the maintenance of the application.

File upload, while really handy, might not be absolutely necessary. We can simply choose to post the URLs of the generated images.

However, at the moment, there's no method available to implement RAG. It would become possible if we could handle messages, at least, in the callback.

iwamot commented 1 year ago

In light of our previous discussions, I have reexamined and revised the implementation of the function call callbacks.