use gpt functions to handle different needs

jaresty commented 7 months ago

Adds function calling to the ChatGPT can interact with the user's system intentionally.

C-Loftus commented 7 months ago

For pokey's context. We reviewed this in a call. I plan to look this over in the coming days and tweak a few things. I may change the function calling to be more explicit and reduce some duplication in the HTML help menu.

Lots of potential with function calling generally though.

C-Loftus commented 7 months ago

Other thing I forgot to mention. It seems we have some changes coming just from different python formatters (changes of " to ' or vice versa, spacing etc). I might set up a pre-commit yml to have a standardized autoformat so these aren't treated as changes. Let me know if you have a preference or if this is due to other reasons. (I have to look into how this is actually set up since I haven't done this logistically before, can't be too hard unless Im misunderstanding something)

jaresty commented 7 months ago

Oh yeah I don't have strong opinions on formatting in python. I think I had run the formatter once because I had some trouble with pasting a properly indented chunk of Json.

C-Loftus commented 7 months ago

Checked this out in more detail. Here is what I would prefer if you wouldn't mind refactoring:

I think we should have a separate gpt-fuction-calling.py file and a separate public function just for function calling. The gpt.py file is getting a bit verbose and I would prefer not passing a function object. I think to start we should see how the function calling does, and slowly but surely expand things. I am worried about any implicit behavior as we expand this.
- Instead of having actions.user.search_for_command() be global we can just scope it privately to that file since it doesn't need to be globally exposed and having a static function instead of an overloadable talon function also reduces error potential.
Instead of using model ask, I was thinking of having an explicit separate command for function calls like
```
model intuit <user.text>: 
    instruction = user.text
    user.gpt_function_call(instruction)
```
This separate function will allow us to keep the talon file commands in the talon files like user.paste so it is more clear to the user what is occurring with each model command. Anything with keyboard input is important to be explicit in my opinion
I don't think we need display_response. I believe we can just use gpt_help since it is the same code? If we need a natural language and cleaner function name for function calling, we can just call wrap actions.user.gpt_help

jaresty commented 7 months ago

I am mostly fine with the suggestions above but i have two main areas I disagree:

with respect to the function calling for input - the LLM does a much more targeted response when given a function to insert a response. If you compare the difference between "model ask insert a while loop" versus "model ask how do I do a while loop" I think you will see what I mean. I think the usability difference warrants the change.
for the display command - it's not exactly the same code as got_help. The body of the resulting page is the contents of the LLM response. I think this is useful when the LLM is responding to a user and you don't want to paste the text into the window since it is more conversational. Are you suggesting that gpt_help be changed to take that text and display it instead?

jaresty commented 7 months ago

The one issue with making a separate command for function calls is that it will require a different request be sent so there could be some duplication there.

As to a name, how about "model do" since we're asking the model to do what we ask and letting it decide?

jaresty commented 7 months ago

OK! Responded to your feedback. There is now a model go command that will execute function calls if they are requested.

C-Loftus commented 7 months ago

Thanks for your work. I added some comments. relatively simple refactoring stuff.

I'm thinking as well that we might want to rename gpt go to gpt call since that is also a one syllable verb and it makes a little bit more sense semantically. So a user could say model call a function that opens the file panel for vscode. But you have more familiarity with how the prompts are constructed so maybe go makes more sense with how the wording would be passed in

jaresty commented 7 months ago

I could be persuaded that 'call' works, but I think it exposes some implementation details. "Go" is a bit more volition-oriented: "go do it for me, I don't care how", which is why I like it.

C-Loftus commented 7 months ago

Essentially my thought is that until we have high confidence in the fact this function calling is working well, we want to be as explicit as possible. I was presuming but am not entirely sure if we will have better performance if the user crafts the text in such a way that they are aware of the underlying function calling.

The only real issue I have with model go is that it appears a bit semantically ambiguous and not entirely clear how it differs from the please command.

You definitely have more experience than I do within function calling so I will let you make the final decision, but that was my rationale

jaresty commented 7 months ago

I've been using it like this:

"model go insert a while loop"
"model go please split this window"

Swapping in call is a bit awkward:

"model call insert a whole loop"
"model call please split this window"

I think the similarity to please is a feature; the function calling is an implementation detail that I don't really want to expose since you are using it to solve a task (not for the purpose of calling a function)

C-Loftus commented 7 months ago

Ok let's go with that. And if we have any other ideas we can obviously edit before merge. I will probably ask for Pokey's feedback before merge.

jaresty commented 7 months ago

I updated this to use the new builder and squashed the commits.

pokey commented 7 months ago

Other thing I forgot to mention. It seems we have some changes coming just from different python formatters (changes of " to ' or vice versa, spacing etc). I might set up a pre-commit yml to have a standardized autoformat so these aren't treated as changes. Let me know if you have a preference or if this is due to other reasons. (I have to look into how this is actually set up since I haven't done this logistically before, can't be too hard unless Im misunderstanding something)

yeah let's set up an autoformatter. we should be able to just steal the pre-commit setup from community

C-Loftus commented 7 months ago

Got a git issue figured out so I can commit to this. (messing with pre-commit if you see errors)

Generally as I have trying this out, I don't think that someone who is new to using the repo will understand what this command can do and have it be practical for them without actually reading the source, since it is very dependent on the underlying function call options. We don't want users to have to experiment to see what will work. I think function calling is a cool idea and I am all for iterating and starting small but I think the amount of iteration this would take to be practical would be a lot. (ie we need to prevent this from causing errors, have a more scalable way of generating json function specs, have a plan for how to make a large amount of potential function calls maintainable if the underlying talon fn changes in args, etc.)

@jaresty I am curious, have you found this to be practically useful so far? For me, the code generation via pilot make <phrase> is much better since it has repo context and we don't need to pass in anything / prompt engineer on our end.

Pokey, any opinions on this? As mentioned, I really appreciate the work Joshua has done on this, but I just am not sure if I see function calling as the best fit for us. Although perhaps I am overlooking something.

I just want to make sure that anything behind the beta tag has a plan for ways it could be feasibly stabilized and used practically.

jaresty commented 7 months ago

I have found this to be useful. I think it can be expanded on as well. I don't have access to copilot presently so I am leaning on ChatGPT for all of my AI needs, and letting the AI differentiate between the kinds of things it can do is more functional for me than simply having it dump a response into the buffer directly. Also, function calling was improved in gpt-4 so I think OpenAI is leaning into this as one of their main directions going forward so I think it's good for us to experiment with it as well. As I mentioned before, I see a strong argument to make this the general use case instead of a separate entry point. I'm pretty sure that copilot is also built on top of function calling under the hood-so we ought to leverage the same IMO.

jaresty commented 7 months ago

I'm trying to not get too attached to it until it is merged, however my expectation is that I will use this for almost all of my model usage. I find it useful to be able to use chatGPT to understand code and to insert syntax when I do not have a snippet and am not aware how to do it in the language I'm working in. I've been using this in python a lot. The experience of using chatGPT when it can display a response in a web browser is nicer than when it pastes into my buffer. I do think there is a world in which we might want to split out what functions are available for what sub commands; perhaps running a VSCode command should be its own thing. Because my experience is not the general case, I don't think we'll really know until more people try this out.

C-Loftus commented 7 months ago

For context: aligned on a call. Seems to be in a good spot now, we changed the command name and we will just scope the command under the beta tag for the time being. I think there is a path forward in terms of maintainability as well by splitting the functions into their own file and automatically generating the JSON.

pokey commented 7 months ago

Strangely, I can't mark my own comments as resolved 🤔

jaresty commented 6 months ago

@C-Loftus is there anything left to do on this one?

C-Loftus commented 6 months ago

Other than resolving the merge conflict, I think it is pretty much fine. It would probably be good to have some sort of basic tests for this repo as we do the json schematization, but that is probably out of scope for this PR.

I think we are just waiting for any feedback from Pokey. ( It is possible that this PR review system might be adding unnecessary friction, and if so I can do this merge without extra review. Currently the repository is setup such that it requires a review to merge. But whatever is best for everyone )

jaresty commented 6 months ago

Ah, I think you can review but not me-authors can't review their own PRs.

C-Loftus commented 6 months ago

@pokey I think we are nearly good to merge this but do you why/how to fix the failing pre-commit CI? I thought it would just reformat and then commit changes to fix it if there were any issues. Wasn't sure if this is due to a setting issue in the repo

jaresty commented 6 months ago

@pokey I think the build is required to merge, so it would be helpful if you could take a look! Thanks.

jaresty commented 6 months ago

@pokey it seems that the pre-commit.ci configuration and isort configuration are in conflict. No matter which one you conform to, the gpt-function-calling.py file doesn't pass CI.

C-Loftus commented 6 months ago

Looking good! See my pushed changes in 50aed65. Also, it looks like #13 (comment) got lost in all the comments

Awesome thanks for checking on this Pokey. Much appreciated. I personally sort of prefer model can you in reference to https://github.com/C-Loftus/talon-ai-tools/pull/13#discussion_r1484314812 since I still find please quite useful and prefer to have keep this command as a separate one without function calling, even as we experiment with function calling. (ie I think there are scenarios where you want the determinism of knowing what will be called explicitly instead of many potential options). But at the same time I could see these eventually being condensed once the function calling is ironed out a bit

But if you feel strongly otherwise I am fine changing it to please. Should be an easy change either way

EDIT: And just to clarify, otherwise I think I am good to merge

C-Loftus commented 6 months ago

I made some fairly significant changes to this. Previously, it pretty much never inserted text given the fact that insertion wasn't being passed to the API for function calling and as a result was only a backup. I turned insertion into another callable function and then made it so the insertion option is just passed as an enum.

For some reason previously we also had it pop up a new window every time. I do not think that this is proper behavior so I changed it. If you want to have a pop up window in a new tab I think you should specify that in the prompt since otherwise it can get kind of annoying.

With this functionality, it is now pretty much a superset of please so I'm okay with using the word please now and have changed it as such. I moved the typing into the lib folder and changed the function name to include the word "dynamic" in it, to signify something more precise than can you which I think is too vague and apt to change if we use please instead

C-Loftus commented 6 months ago

Cursorless functionality works without a destination, but only issue is that it seems like

^model please <user.text> <user.cursorless_target> [<user.cursorless_destination>]$:
    text_list = user.cursorless_get_text_list(cursorless_target)
    user.gpt_dynamic_request_cursorless(user.text, text_list, cursorless_destination or 0)

doesn't work when a destination is defined. I get a weird ipc error that might be a signifier or a type issue outside this repo.

2024-02-17 14:38:41.562 ERROR    25:                                talon/scripting/talon_script.py:721| 
   24:                                talon/scripting/talon_script.py:314| 
   23:                                     talon/scripting/actions.py:88 | 
   22:  user/talon-ai-tools/GPT/beta-commands/gpt-function-calling.py:121| gpt_function_query(utterance, str(sele..
   21:                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   20:  user/talon-ai-tools/GPT/beta-commands/gpt-function-calling.py:60 | process_function_calls(result, insert_..
   19:                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   18:  user/talon-ai-tools/GPT/beta-commands/gpt-function-calling.py:97 | actions.user.cursorless_insert(first_a..
   17:                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   16:                                     talon/scripting/actions.py:88 | 
   15:                   user/cursorless-talon/src/actions/actions.py:128| cursorless_replace_action(destination,..
   14:                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   13:                   user/cursorless-talon/src/actions/replace.py:10 | actions.user.private_cursorless_comman..
   12:                                                                     ^
   11:                                     talon/scripting/actions.py:88 | 
   10:                           user/cursorless-talon/src/command.py:33 | actions.user.private_cursorless_run_rp..
    9:                                                                     ^
    8:                                     talon/scripting/actions.py:88 | 
    7:         user/cursorless-talon/src/cursorless_command_server.py:15 | actions.user.run_rpc_command_and_wait(..
    6:                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    5:                                     talon/scripting/actions.py:88 | 
    4: user/knausj_talon/apps/vscode/command_client/command_client.py:307| run_command(
    3:                                                                     ^
    2: user/knausj_talon/apps/vscode/command_client/command_client.py:193| raise Exception(decoded_contents["error"])
    1:                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Exception: Cannot read properties of undefined (reading 'flatMap')

jaresty commented 6 months ago

Great! I'll kick the tires and then click merge when I'm back at my desk.

jaresty commented 6 months ago

Quick question-it looks like insert response will always paste now; I wanted it to insert to the cursorless destination if it was provided. Am I misreading that?

jaresty commented 6 months ago

I see it now-the switch is happening before the function calling.

jaresty commented 6 months ago

Fwiw - the reason I had it defaulted to the pop up was that I prefer the textual responses from the LLM to display (versus editing). I think I'm ok with either for now since I'd like to get this merged sooner. We can always iterate on making behavior smoother later.

C-Loftus / talon-ai-tools

use gpt functions to handle different needs #13