Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional callback execution after function call #58

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

iwamot
Copy link
Contributor

@iwamot iwamot commented Sep 11, 2023

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 seratch added the enhancement New feature or request label Sep 11, 2023
@seratch
Copy link
Owner

seratch commented Sep 11, 2023

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.

@seratch seratch marked this pull request as draft September 11, 2023 06:59
@iwamot
Copy link
Contributor Author

iwamot commented Sep 11, 2023

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
Copy link
Owner

seratch commented Sep 11, 2023

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
Copy link
Contributor Author

iwamot commented Sep 11, 2023

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
Copy link
Contributor Author

iwamot commented Sep 14, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants