gptel icon indicating copy to clipboard operation
gptel copied to clipboard

send parameter in request when args is nil

Open longlene opened this issue 11 months ago • 4 comments

longlene avatar Apr 30 '25 22:04 longlene

As you can see from the comment in the original code, OpenAI complains if the function takes no arguments but the tool spec contains a :parameters field. From #688, it looks like some OpenAI-compatible APIs (like Grok?) complain if the tool spec does not contain a :parameters field.

So this needs to be tested carefully against all OpenAI compatible APIs.

karthink avatar May 01 '25 07:05 karthink

@karthink Thank you for the explanation. If so, maybe we can add a defcustom variable, gptel will still not cast parameters field by default, then we can customize the value to change the behavior to meet the different inference servers. What do you think?

longlene avatar May 01 '25 08:05 longlene

At least, llama-server expect the parameters field: https://github.com/ggml-org/llama.cpp/blob/13c9a3319b65469e883c49dd1c478abedc410157/common/chat.cpp#L232

longlene avatar May 01 '25 09:05 longlene

If so, maybe we can add a defcustom variable, gptel will still not cast parameters field by default, then we can customize the value to change the behavior to meet the different inference servers.

This will not work -- consider the case where you switch your backend from OpenAI to llama.cpp.

karthink avatar May 01 '25 09:05 karthink

@karthink I updated the code, the default behavior will be still the same as before, then for some user who knows what they are doing, there is still a way for them to always send parameters when needed.

longlene avatar May 01 '25 20:05 longlene

@longlene As I mentioned above, a user option is not the solution here.

Could you check if the OpenAI API works correctly after your original change? It did not use to, but things might be different now. If it works with :parameters specified, we merge the change without the user option.

Also :parameters should probably be set to :null, not nil.

karthink avatar May 05 '25 20:05 karthink

@karthink ok, let me see if I can test it with OpenAI API.

longlene avatar May 06 '25 06:05 longlene

@karthink Tested with gpt-4.1-nano Looks the parameters: null is ok now, I use nil just because it can also work. Will update the pr then. gptel.log

longlene avatar May 06 '25 18:05 longlene

(list :paramters nil) would cause gptel send paramters: {} to the API.

longlene avatar May 06 '25 18:05 longlene

(list :paramters nil) would cause gptel send paramters: {} to the API.

Yes. In your testing, do null and {} both work?

karthink avatar May 06 '25 19:05 karthink

@karthink Yes, both OpenAI and llama-server can work with paramters: {} and null. I updated pr as null.

longlene avatar May 06 '25 21:05 longlene

In the log file I uploaded, the value is null. gpt can call git_status tool correctly.

longlene avatar May 06 '25 21:05 longlene

@longlene I'm merging this PR now, but please stick around in case OpenAI tool calls for functions without args start failing.

karthink avatar May 22 '25 07:05 karthink