karthink / gptel

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

Support for privateGPT, including support for RAG functions like rspecting context and parsing of sources #312

Closed Aquan1412 closed 1 week ago

Aquan1412 commented 1 month ago

Hi, as suggested in our recent discussion #305, here's a PR that adds support for interacting with privateGPT. Its based on the openai-backend, with some additions. The focus was to enable the use of two specific key words, as described in the API reference: 1) use_context: directs the LLM to use the context of documents that have been "ingested" 2) include_sources: directs the LLM to also return information about the sources of the context (so far: file name and page number, if appropriate).

I changed the code of gptel-openai.elto send the additional keywords to the llm server, and to correctly parse the answer if sources are provided.

I tested the code with different queries, and so far it works for my use case. However, there is at least one missing feature: currently, the two new keywords use_context and include_sources are hardcoded to t. It would probably be better to make them configurable when creating the backend. Unfortunately, I'm not sure how to do that correctly.

karthink commented 1 month ago

@Aquan1412 Thanks for the PR.

I had a look at the code, It uses free/global variables and uses some cl-lib functions that aren't declared at compile-time. I can clean up the code but I need some sample privategpt output to be sure I don't break anything.

Could you run (setq gptel-log-level 'info), use privategpt so it generates some sources (more than one), and paste the results of the *gptel-log* buffer here?

Aquan1412 commented 1 month ago

Thanks for cleaning up the code, I'm unfortunately not very experienced writing elisp code.

I ran the command you asked for, however, the output is quite large, which is why I appended it as a file to this comment.

The user query and the expected response are as follows:

*** What are the Navier-Stokes equations?

The Navier-Stokes equations are a set of partial differential equations that describe the conservation of mass, momentum, and energy for a viscous fluid. They are given by equation (2.1) for mass conservation, equation (2.2) for momentum conservation, and equation (2.3) for energy conservation in compressible form. The pressure p, density ρ, velocity components ui, vi, wi, specific internal energy e, specific enthalpy h, temperature T, effective stress tensor τij, and convective heat flux qj are involved in these equations. For gases, the ideal gas law (2.4) relates pressure p to density ρ and specific volume RT/γ, where R is the gas constant and γ is the ratio of specific heats. The specific internal energy e and enthalpy h can be expressed as functions of temperature T using equations (2.5).

Sources:

  • Beneddine-2017-Characterization_of_unsteady_flow_behavior_by_linear_stability_analysis.pdf (page 62)
  • Iorio-2015-Global_Stability_Analysis_of_Turbulent_Transonic_Flows_on_Airfoil_geometries.pdf (page 33)

gptel-privategpt-output.txt

karthink commented 1 month ago

From what I can see, you're not actually buffering the sources from each response. persistent-sources in your code is reset with every chunk, so when it's finally processed it's set to the sources included in the last-but-one response chunk. Is this intentional?

Aquan1412 commented 1 month ago

As far as I'm aware,the list of sources is the same across all chunks. I didn't find an explicit confirmation in the privateGPT API description, but in all my tests it was like that. Therefore I only always reset the chunk, to avoid parsing the list of sources for each chunk.

karthink commented 1 month ago

Wow, that's really wasteful API design. It's sending about 200-1000x as much JSON as required then.

On Sun, May 19, 2024, 1:43 AM Aquan1412 @.***> wrote:

As far as I'm aware,the list of sources is the same across all chunks. I didn't find an explicit confirmation in the privateGPT API description, but in all my tests it was like that. Therefore I only always reset the chunk, to avoid parsing the list of sources for each chunk.

— Reply to this email directly, view it on GitHub https://github.com/karthink/gptel/pull/312#issuecomment-2119154912, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBVOLFKQJT7U5NVMPR7YQ3ZDBQ4JAVCNFSM6AAAAABHSVGPXWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZGE2TIOJRGI . You are receiving this because you commented.Message ID: @.***>

karthink commented 1 month ago

I cleaned up the code and pushed a couple of commits to your branch. I also made "use_context" and "include_sources" configurable. Please check if this introduced any bugs, as I can't test it.

Aquan1412 commented 1 month ago

Great, thanks! Unfortunately, I'm currently travelling and won't have access to my computer for the next two weeks. But afterwards I'll test it as soon as possible.

Aquan1412 commented 3 weeks ago

I finally found some time to test your changes. There are currently two issues: 1) If :stream is set to t, there is an error with concatenating the sources to the response (it works for :stream nil):

error in process filter: apply: Wrong type argument: characterp, "

This should be straight forward enough to fix,I think, as soon as I find a bit of time. 2) If either :context or :sources is set to nil, I get the following error (independent of the value of :stream):

gptel response error: ((HTTP/1.1 422 Unprocessable Entity) Could not parse HTTP response.) Could not parse HTTP response.

Here, I'm not sure what the reason is. Any idea?

karthink commented 3 weeks ago

2) If either :context or :sources is set to nil, I get the following error (independent of the value of :stream):

gptel response error: ((HTTP/1.1 422 Unprocessable Entity) Could not parse HTTP response.) Could not parse HTTP response.

Here, I'm not sure what the reason is. Any idea?

Can you generate a log for this case? (setq gptel-log-level 'info), then produce this error and check *gptel-log*.

karthink commented 3 weeks ago

Made a couple of changes based on guesses. Please test again and try to use the gptel log buffer to get more details on the errors.

Aquan1412 commented 2 weeks ago

Made a couple of changes based on guesses. Please test again and try to use the gptel log buffer to get more details on the errors.

With these new changes I get an "empty response". I attached the output of the gptel-log-buffer, as it is once again quite lengthy. gptel-privategpt-output_latest-commit.txt

Aquan1412 commented 2 weeks ago
  1. If either :context or :sources is set to nil, I get the following error (independent of the value of :stream):

gptel response error: ((HTTP/1.1 422 Unprocessable Entity) Could not parse HTTP response.) Could not parse HTTP response.

Here, I'm not sure what the reason is. Any idea?

Can you generate a log for this case? (setq gptel-log-level 'info), then produce this error and check *gptel-log*.

I also went back and reran the command while generating a log. Here the output was quite short:

{ "gptel": "request body", "timestamp": "2024-06-10 20:36:33" } { "model": "private-gpt", "messages": [ { "role": "system", "content": "You are a large language model living in Emacs and a helpful assistant. Respond concisely." }, { "role": "user", "content": "What are the Navier-Stokes equations?" } ], "use_context": true, "include_sources": null, "stream": true, "temperature": 1.0 } { "gptel": "response body", "timestamp": "2024-06-10 20:36:33" } { "detail": [ { "type": "bool_type", "loc": [ "body", "include_sources" ], "msg": "Input should be a valid boolean", "input": null, "url": "https://errors.pydantic.dev/2.5/v/bool_type" } ] }

karthink commented 2 weeks ago

Are you sure the changes were applied?

{
    "model": "private-gpt",
    "messages": [
        {
            "role": "system",
            "content": "You are a large language model living in Emacs and a helpful assistant. Respond concisely."
        },
        {
            "role": "user",
            "content": "What are the Navier-Stokes equations?"
        }
    ],
    "use_context": true,
    "include_sources": null,
    "stream": true,
    "temperature": 1.0
}

In this request, "include_sources" should be set to false, not null, if the changes are applied.

karthink commented 2 weeks ago

I made a dummy privategpt backend and tested it, and "include_sources" is indeed set to false.

Aquan1412 commented 2 weeks ago

Are you sure the changes were applied?

{
    "model": "private-gpt",
    "messages": [
        {
            "role": "system",
            "content": "You are a large language model living in Emacs and a helpful assistant. Respond concisely."
        },
        {
            "role": "user",
            "content": "What are the Navier-Stokes equations?"
        }
    ],
    "use_context": true,
    "include_sources": null,
    "stream": true,
    "temperature": 1.0
}

In this request, "include_sources" should be set to false, not null, if the changes are applied.

Sorry, I wasn't clear enough: I went back to the previous commit and ran the command with logging turned on, in case it would help.

Aquan1412 commented 2 weeks ago

Also, I found (and fixed, I hope) an error in your latest commit. Now it is possible to correctly toggle :context and :sources.

karthink commented 2 weeks ago

Also, I found (and fixed, I hope) an error in your latest commit. Now it is possible to correctly toggle :context and :sources.

It looks like you reverted a change I made, to use the backend that is passed to the parsing functions. You are using gptel-backend instead -- this is an error.

gptel-backend can be set buffer-locally, and the parsing code runs in the HTTP request buffer, so it can have the wrong value. This is why the backend is explicitly passed to the parsing functions.

karthink commented 2 weeks ago

Sorry, I wasn't clear enough: I went back to the previous commit and ran the command with logging turned on, in case it would help.

I'm confused about "previous commit" now. As per the latest commit, when :sources is set to nil in the privategpt backend definition, is "include_sources" set to null or false when sending the request?

Aquan1412 commented 2 weeks ago

Sorry, I wasn't clear enough: I went back to the previous commit and ran the command with logging turned on, in case it would help.

I'm confused about "previous commit" now. As per the latest commit, when :sources is set to nil in the privategpt backend definition, is "include_sources" set to null or false when sending the request?

It is set to false.

Aquan1412 commented 2 weeks ago

Now I also fixed the problem that the sources-string was not properly appended to the response if :stream t. So now everything seems to work as it should, all backend-settings (:stream, :context and :sources) can be toggled on and off without problems.

Aquan1412 commented 2 weeks ago

Also, I found (and fixed, I hope) an error in your latest commit. Now it is possible to correctly toggle :context and :sources.

It looks like you reverted a change I made, to use the backend that is passed to the parsing functions. You are using gptel-backend instead -- this is an error.

gptel-backend can be set buffer-locally, and the parsing code runs in the HTTP request buffer, so it can have the wrong value. This is why the backend is explicitly passed to the parsing functions.

The problem with using backend (or _backend) is that I get the following error:

error in process sentinel: Wrong type argument: gptel-privategpt, nil

This is why I changed it back to gptel-backend. I also looked at some of the other implementations (specifically gptel-openai and gptel-ollama, and there you also still use gptel-backend, so I thought it was OK.

The full log output is as follows:

{ "gptel": "request body", "timestamp": "2024-06-16 21:26:55" } { "model": "private-gpt", "messages": [ { "role": "system", "content": "You are a large language model living in Emacs and a helpful assistant. Respond concisely." }, { "role": "user", "content": "What are the Navier-Stokes equations?" } ], "use_context": true, "include_sources": true, "stream": false, "temperature": 1.0 } { "gptel": "response body", "timestamp": "2024-06-16 21:26:59" } { "id": "fe28e561-95e0-4e37-91c0-1bf2defc1173", "object": "completion", "created": 1718566019, "model": "private-gpt", "choices": [ { "finish_reason": "stop", "delta": null, "message": { "role": "assistant", "content": " The Navier-Stokes equations are a set of partial differential equations that describe the motion of viscous fluid substances, including air and water. They were formulated by Claude Louis Marie Henri Navier and George Gabriel Stokes in the early 19th century. These equations are fundamental to fluid dynamics and are widely used in various fields such as meteorology, aeronautics, hydrodynamics, and many more." }, "sources": [ { "object": "context.chunk", "score": 0.5979778911410051, "document": { "object": "ingest.document", "doc_id": "f0fa7ff2-9c83-46f7-9412-0752af9d96ac", "doc_metadata": { "window": "Treaty Org.\n ˚Akervik E, Brandt L, Henningson DS, Hœpffner J, Marxen O, Schlatter P. 2006. Steady solutions of the\nNavier-Stokes equations by selective frequency damping. Phys. Fluids 18:068102\n˚Akervik E, Hœpffner J, Ehrenstein U, Henningson DS. 2007. ", "original_text": "Steady solutions of the\nNavier-Stokes equations by selective frequency damping. ", "page_label": "28", "file_name": "Theofilis-2011-Global_Linear_Instability.pdf", "doc_id": "f0fa7ff2-9c83-46f7-9412-0752af9d96ac" } }, "text": "Treaty Org.\n ˚Akervik E, Brandt L, Henningson DS, Hœpffner J, Marxen O, Schlatter P. 2006. Steady solutions of the\nNavier-Stokes equations by selective frequency damping. Phys. Fluids 18:068102\n˚Akervik E, Hœpffner J, Ehrenstein U, Henningson DS. 2007. ", "previous_texts": null, "next_texts": null }, { "object": "context.chunk", "score": 0.573735513098284, "document": { "object": "ingest.document", "doc_id": "080a7865-1a7f-49d8-8781-e991f04563c0", "doc_metadata": { "window": "J. Fluid Mech. 493:31–58\nOosterlee CW, Wesseling P, Segal A, Brakkee E. 1993. Benchmark solutions for the incompressible Navier-\nStokes equations in general coordinates on staggered grids. Int. J. Numer. Methods Fluids 17:301–21\nPawlowski RP, Salinger AG, Shadid JN, Mountziaris TJ. ", "original_text": "Benchmark solutions for the incompressible Navier-\nStokes equations in general coordinates on staggered grids. ", "page_label": "32", "file_name": "Theofilis-2011-Global_Linear_Instability.pdf", "doc_id": "080a7865-1a7f-49d8-8781-e991f04563c0" } }, "text": "J. Fluid Mech. 493:31–58\nOosterlee CW, Wesseling P, Segal A, Brakkee E. 1993. Benchmark solutions for the incompressible Navier-\nStokes equations in general coordinates on staggered grids. Int. J. Numer. Methods Fluids 17:301–21\nPawlowski RP, Salinger AG, Shadid JN, Mountziaris TJ. ", "previous_texts": null, "next_texts": null } ], "index": 0 } ] }

karthink commented 2 weeks ago

The problem with using backend (or _backend) is that I get the following error:

error in process sentinel: Wrong type argument: gptel-privategpt, nil

This is why I changed it back to gptel-backend. I also looked at some of the other implementations (specifically gptel-openai and gptel-ollama, and there you also still use gptel-backend, so I thought it was OK.

Those are errors too, actually, I need to fix that. However, you'll notice that the backend is not used (as either backend or gptel-backend) when parsing responses for any of the other APIs, in gptel--parse-response or gptel--curl-parse-stream. This is where things can go wrong if the wrong backend is used, and it's why this bug has gone unnoticed. It's only used when creating the request, in gptel--request-data, where backend and gptel-backend typically have the same value.

error in process sentinel: Wrong type argument: gptel-privategpt, nil

Anyway, I'm trying to figure out why you're getting that error :thinking:

karthink commented 2 weeks ago

I fixed the bug that was causing backend to be nil, rewrote that part of the response handling (gptel--parse-buffer and gptel-curl--parse-stream), and rebased on master. Could you update and check if it works as intended now?

Aquan1412 commented 1 week ago

I fixed the bug that was causing backend to be nil, rewrote that part of the response handling (gptel--parse-buffer and gptel-curl--parse-stream), and rebased on master. Could you update and check if it works as intended now?

I just checked it, and everything works as intended. Thanks for the effort!

karthink commented 1 week ago

That's great, we can merge it then. Can you add an entry to the README with instructions for using PrivateGPT? You can copy the text from one of the other sections (like Perplexity or OpenRouter) and modify it, with additional notes on using :sources and :context.

Aquan1412 commented 1 week ago

Done. I added the usage instructions and also included a link to the privateGPT repository for reference.

karthink commented 1 week ago

Cheers @Aquan1412! Thanks for the PR.