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

Implement a gptel-define-tool macro #685

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

swflint
Copy link

@swflint swflint commented Mar 6, 2025

Based on discussion in #514 (#514 (comment) through #514 (comment)), this PR adds a macro to define tools in one single form, simplifying their creation.

As of now, this PR is very much a draft, as noted, I will need to get debug and edebug working, but I also will need to finish the docstring. That said, I would appreciate other feedback on the macro and its developer experience.

@karthink
Copy link
Owner

karthink commented Mar 7, 2025

@swflint I'm busy with other parts of gptel presently but I popped in to say that this is a great idea. I second @psionic-k's suggestion in this comment about the macro accepting keywords directly.

I also think the naming is going to be confusing -- there will be a gptel-make-tool and a gptel-define-tool and users will have to check both out to figure out which one to use. I don't yet know of a good solution to this, the names are ambiguous.

As for this comment:

This could make it easier to write without repeating too much, and avoid the use anonymous functions, making debugging easier.

You don't have to use anonymous functions? You can provide a regular named elisp function to the tool. This design was intentional, so you can reuse regular elisp functions as LLM tools if required. See for instance the yt-dlp tool in my opening comment.

That said a macro to define an elisp function and a gptel tool together like this is welcome.

@psionic-k
Copy link
Contributor

@swflint if you PR the experimental branch of my fork, we can develop this independently.

Just one macro should be able to handle the existing keyword-only implementation while recognizing several "shorthand" forms by their unique content. That is how transient's definers work.

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

Successfully merging this pull request may close these issues.

3 participants