Add support for Assistant APIs #464

paulcoghlan · 2024-10-07T12:26:46Z

Relates to: #421

It would be great if the LLM plugin could support some Assistant features, so I've raised this PR to add support for Assistants, Threads, Runs and Messages.

I've removed the go.mod file from the llmclient so we support the latest version of http://github.com/sashabaranov/go-openai.

Any feedback welcome as it is my first PR! 🙏

Health Check Scenarios

Models OK, Assistant Supported

GET /api/plugins/grafana-llm-app/health
{
    "details": {
        "openAI": {
            "assistant": {
                "ok": true
            },
            "configured": true,
            "models": {
                "base": {
                    "ok": true
                },
                "large": {
                    "ok": true
                }
            },
            "ok": true
        },
   ...
      "version": "0.11.0"
    },
    "message": "",
    "status": "OK"
}

Models OK, Assistant Not Supported

GET /api/plugins/grafana-llm-app/health
{
    "details": {
        "openAI": {
            "assistant": {
                "ok": fail,
                "error": "Assistant not present"
            },
            "configured": true,
            "models": {
                "base": {
                    "ok": true
                },
                "large": {
                    "ok": true
                }
            },
            "ok": true
        },
   ...
      "version": "0.11.0"
    },
    "message": "",
    "status": "OK"
}

Models Fail
As previously - failure

sd2k · 2024-10-09T09:29:25Z

Thanks Paul! In general this looks good, our main concern is how to handle LLM providers which don't support these APIs (e.g. if the user has configured a custom OpenAI-compatible LLM backend other than OpenAI). We may need to add something to the health check response which checks whether these things exist, so that clients can degrade gracefully in those cases.

paulcoghlan · 2024-10-09T13:04:54Z

Thanks Paul! In general this looks good, our main concern is how to handle LLM providers which don't support these APIs (e.g. if the user has configured a custom OpenAI-compatible LLM backend other than OpenAI). We may need to add something to the health check response which checks whether these things exist, so that clients can degrade gracefully in those cases.

I see @sd2k, that makes sense. Maybe I could extend the health API to add an AssistantHealthDetails here?

grafana-llm-app/packages/grafana-llm-frontend/src/types.ts

Lines 6 to 10 in d3ac328

    
           export interface HealthCheckDetails { 
        
             openAI: OpenAIHealthDetails | boolean; 
        
             vector: VectorHealthDetails | boolean; 
        
             version: string; 
        
           }

sd2k · 2024-10-09T13:09:05Z

Yep sounds like a plan. It could maybe go inside the OpenAIHealthDetails type though since it's related to the level of OpenAI support 👍

paulcoghlan · 2024-10-09T13:12:54Z

Yep sounds like a plan. It could maybe go inside the OpenAIHealthDetails type though since it's related to the level of OpenAI support 👍

Got it - thanks, will do 😄.

sd2k

This is looking good to me, thanks Paul! I've added a few suggestions inline but would like to get a second opinion on a few things, mostly:

the go.mod change (I think this should at least be in a separate PR, and need to learn more about it first)
the now-large OpenAI interface in llmclient; perhaps it's worth splitting it

@csmarchbanks could you take a look at this when you have time?

sd2k · 2024-11-07T09:55:22Z

packages/grafana-llm-app/llmclient/go.mod

Removing this completely seems a bit scary, my knowledge of Go dependencies isn't strong enough to know how it would handle a breaking change (e.g. v2) in sashabaranov/go-openai though 🤔

Perhaps we should consider this in a separate PR, unless it's really needed here?

Yeah, what's the reason for this removal? I like specifying that we depend on >=1.15.3, <2.0.0. Are you running into import issues somewhere?

@sd2k @csmarchbanks I got confused about why there were two go.mod files - I didn't quite realise the client was a separate build product! I'll revert, sorry for the confusion.

sd2k · 2024-11-07T09:58:43Z

packages/grafana-llm-app/llmclient/llmclient.go

+	// CreateAssistant creates an assistant using the given request.
+	CreateAssistant(ctx context.Context, req AssistantRequest) (openai.Assistant, error)
+	// RetrieveAssistant retrieves an assistant by ID.
+	RetrieveAssistant(ctx context.Context, assistantID string) (openai.Assistant, error)
+	// ListAssistants lists assistants.
+	ListAssistants(ctx context.Context, limit *int, order *string, after *string, before *string) (openai.AssistantsList, error)
+	// DeleteAssistant deletes an assistant by ID.
+	DeleteAssistant(ctx context.Context, assistantID string) (openai.AssistantDeleteResponse, error)
+	// CreateThread creates a new thread.
+	CreateThread(ctx context.Context, req openai.ThreadRequest) (openai.Thread, error)
+	// RetrieveThread retrieves a thread by ID.
+	RetrieveThread(ctx context.Context, threadID string) (openai.Thread, error)
+	// DeleteThread deletes a thread by ID.
+	DeleteThread(ctx context.Context, threadID string) (openai.ThreadDeleteResponse, error)
+	// CreateMessage creates a new message in a thread.
+	CreateMessage(ctx context.Context, threadID string, request openai.MessageRequest) (msg openai.Message, err error)
+	// ListMessages lists messages in a thread.
+	ListMessages(ctx context.Context, threadID string, limit *int, order *string, after *string, before *string, runID *string) (openai.MessagesList, error)
+	// RetrieveMessage retrieves a message in a thread.
+	RetrieveMessage(ctx context.Context, threadID string, messageID string) (msg openai.Message, err error)
+	// DeleteMessage deletes a message in a thread.
+	DeleteMessage(ctx context.Context, threadID string, messageID string) (msg openai.MessageDeletionStatus, err error)
+	// CreateRun creates a new run in a thread.
+	CreateRun(ctx context.Context, threadID string, request openai.RunRequest) (run openai.Run, err error)
+	// RetrieveRun retrieves a run in a thread.
+	RetrieveRun(ctx context.Context, threadID string, runID string) (run openai.Run, err error)
+	// CancelRun cancels a run in a thread.
+	CancelRun(ctx context.Context, threadID string, runID string) (run openai.Run, err error)
+	// SubmitToolOutputs submits tool outputs for a run in a thread.
+	SubmitToolOutputs(ctx context.Context, threadID string, runID string, request openai.SubmitToolOutputsRequest) (response openai.Run, err error)


A niggling part of my brain thinks this should be a separate OpenAIAssistant interface to avoid the OpenAI interface becoming too big (and make it easier to mock for users in tests), particularly if users are importing and using this in their code already. WDYT?

I guess people would need to type switch though so maybe it's not worth it?

I also think having a second interface could be nice. It would also allow us to check for features based on if the interface is implemented for a connection or not (in the case we add more first class implementations of this interface).

Good point, will add this interface.

sd2k · 2024-11-07T10:01:12Z

packages/grafana-llm-app/pkg/plugin/health.go

@@ -86,6 +98,7 @@ func (a *App) openAIHealth(ctx context.Context, req *backend.CheckHealthRequest)
 		OK:         true,
 		Configured: a.settings.OpenAI.Configured(),
 		Models:     map[Model]openAIModelHealth{},
+		Assistant:  openAIModelHealth{OK: false, Error: "Assistant not present"},


Suggested change

Assistant: openAIModelHealth{OK: false, Error: "Assistant not present"},

Assistant: openAIModelHealth{OK: false, Error: "Assistant not available"},

I kinda like available more but this is very much personal preference!

sd2k · 2024-11-07T10:01:54Z

packages/grafana-llm-app/pkg/plugin/health.go

+		if err == nil {
+			d.Assistant.OK = true
+			d.Assistant.Error = ""
+		}


Perhaps we should append the error to the d.Assistant.Error if it's non-nil, to make it easier for users to debug why the assistant isn't available.

Good point, will do.

sd2k · 2024-11-07T10:03:09Z

packages/grafana-llm-app/pkg/plugin/llm_provider.go

@@ -133,4 +133,6 @@ type LLMProvider interface {
 	// ChatCompletionStream provides text completion in a chat-like interface with
 	// tokens being sent as they are ready.
 	ChatCompletionStream(context.Context, ChatCompletionRequest) (<-chan ChatCompletionStreamResponse, error)
+	// ListAssistants lists assistants.


Suggested change

// ListAssistants lists assistants.

// ListAssistants lists assistants.

//

// This is used by the health check to determine whether the configured provider

// supports assistant APIs.

sd2k · 2024-11-07T10:04:53Z

packages/grafana-llm-app/src/components/AppConfig/HealthCheck.tsx

@@ -128,6 +137,8 @@ function ShowOpenAIHealth({ openAI }: { openAI: OpenAIHealthDetails | boolean })
          </li>
        ))}
      </div>
+      <b>Assistant: </b>
+      {openAI.assistant.ok ? 'OK' : `Error: ${openAI.assistant.error}`}


Suggested change

{openAI.assistant.ok ? 'OK' : `Error: ${openAI.assistant.error}`}

{openAI.assistant.ok ? 'OK' : `Error: ${openAI.assistant.error}. The configured OpenAI provider may not offer assistants APIs.`}

Agree, this is a more precise description.

sd2k · 2024-11-07T10:05:04Z

packages/grafana-llm-frontend/src/types.ts

@@ -21,6 +21,8 @@ export interface OpenAIHealthDetails {
  // The health check attempts to call the OpenAI API with each
  // of a few models and records the result of each call here.
  models?: Record<string, OpenAIModelHealthDetails>;
+  // Health details for the OpenAI assistant model.


Suggested change

// Health details for the OpenAI assistant model.

// Health details for the OpenAI assistant APIs.

paulcoghlan self-assigned this Oct 7, 2024

paulcoghlan added 4 commits November 4, 2024 17:16

Use latest go-openai library, add assistant api calls

ace59f1

Added some tests

940cc70

Added some missing API calls

c0f9070

Add Assistant healthcheck

39ce3c2

paulcoghlan force-pushed the paulcoghlan/add-assistant-api branch from fc3a43d to 39ce3c2 Compare November 4, 2024 17:18

paulcoghlan added 4 commits November 4, 2024 18:05

Fix azure_provider.go

e8944fa

Fix rebase errors

bb2dc8c

Show assistant health in UI

8af2626

Update health tests

08fa187

paulcoghlan marked this pull request as ready for review November 5, 2024 13:42

paulcoghlan requested a review from sd2k November 5, 2024 13:42

sd2k requested a review from a team November 5, 2024 13:44

sd2k reviewed Nov 7, 2024

View reviewed changes

sd2k self-assigned this Nov 7, 2024

sd2k requested a review from csmarchbanks November 7, 2024 20:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Assistant APIs #464

Add support for Assistant APIs #464

paulcoghlan commented Oct 7, 2024 •

edited

Loading

sd2k commented Oct 9, 2024

paulcoghlan commented Oct 9, 2024

sd2k commented Oct 9, 2024

paulcoghlan commented Oct 9, 2024

sd2k left a comment

sd2k Nov 7, 2024

sd2k Nov 7, 2024

csmarchbanks Nov 8, 2024

paulcoghlan Nov 13, 2024

sd2k Nov 7, 2024

csmarchbanks Nov 8, 2024

paulcoghlan Nov 13, 2024

sd2k Nov 7, 2024

sd2k Nov 7, 2024

paulcoghlan Nov 13, 2024

sd2k Nov 7, 2024

sd2k Nov 7, 2024

paulcoghlan Nov 13, 2024

sd2k Nov 7, 2024

	Assistant: openAIModelHealth{OK: false, Error: "Assistant not present"},
	Assistant: openAIModelHealth{OK: false, Error: "Assistant not available"},

	{openAI.assistant.ok ? 'OK' : `Error: ${openAI.assistant.error}`}
	{openAI.assistant.ok ? 'OK' : `Error: ${openAI.assistant.error}. The configured OpenAI provider may not offer assistants APIs.`}

	// Health details for the OpenAI assistant model.
	// Health details for the OpenAI assistant APIs.

Add support for Assistant APIs #464

Are you sure you want to change the base?

Add support for Assistant APIs #464

Conversation

paulcoghlan commented Oct 7, 2024 • edited Loading

Health Check Scenarios

sd2k commented Oct 9, 2024

paulcoghlan commented Oct 9, 2024

sd2k commented Oct 9, 2024

paulcoghlan commented Oct 9, 2024

sd2k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulcoghlan commented Oct 7, 2024 •

edited

Loading