karthink / gptel

A simple LLM client for Emacs
GNU General Public License v3.0
1.04k stars 113 forks source link

Add multiline prefixes & AI response prefixes #142

Closed daedsidog closed 6 months ago

daedsidog commented 7 months ago

This makes it so that prefixes with newlines are respected s.t. the point is moved to the end of the prefix:

image

Before, the point would be at the end of the first line. This saves two key presses.

daedsidog commented 7 months ago

Would it be better to export those chars to a symbol that can be configurable?

daedsidog commented 7 months ago

I took the time to add support for more complex prefixes for the dedicated gptel-mode. I wanted to have prefixes for the AI responses as well as the user prompts so that it would be easier to organize a document from my end. It looks like this:

image

I made sure the parsing of the buffer works as expected when using either backend:

((:role "system" :content "You are a large language model living in Emacs and a helpful assistant. Respond concisely.") (:role "user" :content "Hello") (:role "assistant" :content "Hello! How can I assist you today?") (:role "user" :content "How are you?"))

Behavior is maintained for normal, inline prefixes:

image

((:role "system" :content "You are a large language model living in Emacs and a helpful assistant. Respond concisely.") (:role "user" :content "Hello") (:role "assistant" :content "Hello! How can I assist you today?") (:role "user" :content "How are you?"))

By default, responses don't have prefixes, and the behavior is unchanged unless configured:

image

((:role "system" :content "You are a large language model living in Emacs and a helpful assistant. Respond concisely.") (:role "user" :content "Hello") (:role "assistant" :content "Hello! How can I assist you today?") (:role "user" :content "How are you?"))

I tested with and without curl, though I did not test the other backends. I expect them to behave the same, since the only thing I changed is how the messages are being trimmed.

karthink commented 7 months ago

Thanks for this PR, it's looking good!

I'm testing it right now. We can merge it soon if you think it's ready.

karthink commented 7 months ago

Is there some reason you insert the response-prefix after the response is fully inserted instead of before?

daedsidog commented 7 months ago

The original reason was that it was mostly a hack from my own configuration file that had that behavior with post hooks. The latest commit changes the behavior to insert the response prefix before the message starts. I tested it and it does not affect the parsing.

Seems like you don't have c6dd3e5ddcc2f737028bb58fc81b72be02854283, then?

karthink commented 7 months ago

Seems like you don't have c6dd3e5, then?

I do. Is this the change you are referring to?

This inserts the response prefix after the response has been fully received. Is there some reason you don't insert it before the response starts?

karthink commented 7 months ago

The latest commit changes the behavior to insert the response prefix before the message starts.

I'm not seeing this -- could you point me to it?

daedsidog commented 6 months ago

The comment on that commit is wrong, I carelessly copied it from the previous location. It puts the prefix before the message has been received, not after. The current behavior is like this:

gptel

daedsidog commented 6 months ago

I was also not sure how to deal with the skip-chars-backward, as I was not sure what's its purpose, so I just exported them to an internal variable without changing the default value. However, for multiline prefixes to be respected on startup, the \n needs to be removed, which I suggest doing, unless it would affect something else I don't know about.

karthink commented 6 months ago

It puts the prefix before the message has been received, not after. The current behavior is like this:

My bad, I had pulled c6dd3e5 but hadn't reloaded the file yet.

It works as expected.

I was also not sure how to deal with the skip-chars-backward, as I was not sure what's its purpose

It's there to clean things up when inserting the output when redirecting it from another buffer, usually from the transient menu.

(goto-char (point-max))
(skip-chars-backward gptel--prefix-chars-to-skip)
(if (bobp) (insert (or initial (gptel-prompt-prefix-string))))
daedsidog commented 6 months ago

Done. I should note that the README needs to be updated to mention the response prefixes.

daedsidog commented 6 months ago

Done

karthink commented 6 months ago

Looks good, thank you for the PR @daedsidog!