C-Loftus / talon-ai-tools

Query LLMs and AI tools with voice commands
http://colton.place/talon-ai-tools/
MIT License
46 stars 17 forks source link

Support other endpoints and generalize settings #24

Closed C-Loftus closed 6 months ago

C-Loftus commented 6 months ago

This pull request makes it so the endpoint and general settings used for the model are easier to customize. Addresses https://github.com/C-Loftus/talon-ai-tools/issues/11

Also made some the documentation in the readme better and more reflective of the current state of the repository.

Only a thing I'm not entirely sure on as if we want a different default system prompt. For OpenAI, you do not have to specify the system prompt and then there's some sort of implicit system prompt used in the background. I don't think there's a way to know what that is. But for some models like llamafiles, you need to explicitly state that so i picked a system prompt that generally seems fine.

C-Loftus commented 6 months ago

Blocked until Douglas has a chance to look at this (or anyone else with an Azure endpoint)

douglasg14b commented 6 months ago

Waiting on access to endpoints to test it out!

C-Loftus commented 6 months ago

I think I am going to merge this, @jaresty do you have any opinions? It doesn't break anything so it is not a regression. I basically add a new setting for the endpoint

C-Loftus commented 6 months ago

https://github.com/C-Loftus/talon-ai-tools/blob/f6833cf0cfefe5d4c533e5d4e4e112d44b8fcfd8/GPT/beta-commands/gpt-function-calling.py#L45 I'm continuing to refactor some things as well to reduce code duplication. @jaresty is there a reason we have the code language within a system prompt? I believe that should be passed in within the user prompt, unless I'm mistaking something with how it wants the language specification be passed in. If we always have just the language in the system prompt, I don't the model has enough meta contextual info about the request. I think the language should be passed in only if it needs it, ie in the callable functions that insert code. We can dynamically get this at action-time so don't think this needs to be in the system prompt at all so we limit it to just the necessary fns

But if I am overlooking lmk

jaresty commented 6 months ago

I chose to put the language in the system prompt because I considered it to be a kind of metadata about the context in which the command was being called. I think that more information really should be available as part of the system prompt too. Putting everything into the user prompt requires the LLM to parse the metadata-information it needs to respond to the query from the prompt itself. I think using the system prompt to do so makes it easier on the LLM.

Also, with please the user can specify natural language that results in insert being called, so it really helps to not have to specify the language in your natural language request.

jaresty commented 6 months ago

I believe you can have multiple system prompts, fwiw. Any particular reason you wanted to remove the language?

jaresty commented 6 months ago

Maybe a good way to phrase the question is, what evidence have you seen that the language is causing issues?

C-Loftus commented 6 months ago

I agree that we want to keep this somehow. Unless I'm overlooking something we can't have multiple system prompts https://platform.openai.com/docs/api-reference/chat/create

Essentially my problem was the fact that the language is often going to be none if we're not in a code editor and even if we are in the code editor a lot of our commands could feasibly be unrelated to the language. I want to be only sending necessary information in the system prompt since sending irrelevant metadata can cause issues.

That being said you unfortunately Python fn docstrings need to be string literals and can't be interpolated so I think what I will do is just have a ternary operator to add the code language into the system prompt at the end, but only if it is defined. This isn't perfect and we will still be sending the code language a lot of times when it isn't really needed, but I suppose it's not likely to be a big deal.

>>> talon.actions.code.language()
''
C-Loftus commented 6 months ago

added a ternary expression for the language context. I think I am happy with that. So we can add the contextual info like the language you want but it's not always going to add it. in general there's probably a lot of potential to put contextual info within the system prompt but keeping it pretty simple for now with just the language. let me know if that last commit above looks good

C-Loftus commented 6 months ago

@jaresty if this looks good to you, feel free to approve and merge, I have done a fair bit of testing with both the function calling and standard behavior so there shouldn't be any regressions here

jaresty commented 6 months ago

I haven't had too much time to try it out but I'm ok with rolling forward. We can always change it again.