Unable to use llm.Chat (OpenAI) with only one existing embedder for OpenAI

[zivkovicn]

Description

• this PR embeddings: remove unnecessary mostly-duplicated implementations #362 removed the openai and vertexai chat embedders, with that we cannot anymore pass in the LLM created with the WithClient option to the embedder init method, because openai.LLM (https://github.com/tmc/langchaingo/blob/main/llms/openai/llm.go) and openai.Chat LLM (https://github.com/tmc/langchaingo/blob/main/llms/openai/openaillm_chat.go) are two different LLMs.

• when we want to use openai.Chat LLM, we need to have proper embedder for it.

code example (chat LLM through Azure) with vectorstore:

package main

import (
	"context"
	"os"

	openAIChatEmbedder "github.com/tmc/langchaingo/embeddings/openai/openaichat"
	"github.com/tmc/langchaingo/llms/openai"
	"github.com/tmc/langchaingo/vectorstores"
	"github.com/tmc/langchaingo/vectorstores/pinecone"
)

type Config struct {
	ApiKey          string // Azure OpenAI key 1 or (key 2)
	BaseUrl         string // Azure OpenAI base url
	Model           string // Azure OpenAI model
	EmbeddingsModel string // Azure OpenAI embeddings model
}

func main() {
	ctx := context.Background()
	config := Config{
		ApiKey:          "",
		BaseUrl:         "",
		Model:           "",
		EmbeddingsModel: "",
	}

	chatLLM, err := openai.NewChat(
		openai.WithAPIType(openai.APITypeAzure),
		openai.WithBaseURL(config.BaseUrl),
		openai.WithToken(config.ApiKey),
		openai.WithModel(config.Model),
		openai.WithEmbeddingModel(config.EmbeddingsModel),
	)
	if err != nil {
		print(err)
		os.Exit(1)
	}

	emb, err := openAIChatEmbedder.NewChatOpenAI(openAIChatEmbedder.WithClient(chatLLM)) // we cannot do this anymore
	if err != nil {
		print(err)
		os.Exit(1)
	}

	vectorDB, err := pinecone.New(ctx, pinecone.WithEmbedder(emb))
	if err != nil {
		print(err)
		os.Exit(1)
	}

	retriever := vectorstores.ToRetriever(
		vectorDB,
		1,
		vectorstores.WithNameSpace("namespace"),
		vectorstores.WithEmbedder(emb), // even here we could pass in the embedder separately
	)
	
        all other logic here not that important
	....
}

this code above shows how we can use llm.Chat with it's embedder.

The whole handicap is that now, without "github.com/tmc/langchaingo/embeddings/openai/openaichat" or the same for vertexai we cannot create the the embedder needed to be used in the llm.Chat or to pass it via WithClient llm option.

@tmc @eliben, continuing the discussion from the merged PR above.

This is the main blocker for us/me and whoever wants to use the chat llm.

[eliben]

A chat just defers to its underlying client to do an embedding: https://github.com/tmc/langchaingo/blob/51a3a0a0f54a/llms/openai/openaillm_chat.go#L127

There's a huge amount of duplication between Chat and LLM already.

There are multiple approaches to solve this without adding even more duplication. For example, we could add an interface for types that provide CreateEmbedding, and then the code in embeddings could use that. How does that sound? I can try to prototype something later today or tomorrow.

[eliben]

... another option is just to let each Chat expose its underlying Client, and then that can be used for the embeddings package.

[zivkovicn]

yeah, that could resolve the issue, we should revert back the PR #362, then once we have better approach to satisfy the needs, we could do the clean up (and remove the duplication), b/c at the moment all people that were using llm.Chat are blocked for updates for the newer changes from the lib. cc @tmc

[tmc]

I can help with this a bit later today, if we don't have a simple enough forward fix I'm open to reverting for now.

[eliben]

PTAL at #374 - this is a sketch of what I mean. It makes the EmbedderClient interface public (we can still keep it in internal later) and adds NewEmbedder which takes EmbedderClient and returns a type that itself implements Embedder.

Both LLM and Chat implement EmbedderClient implicitly, so @zivkovicn you can take a Chat you created with openai.NewChat and pass it to NewEmbedder. Its return value can then be passed into pinecone.WithEmbedder.

Please try it and let me know if it works for you.

---

This approach can help reduce a lot of duplication that currently exists in the embeddings package; happy to get to that once we hash out the basic design, which is using interfaces to decouple things. What the embeddings package does is really take lower level clients that have CreateEmbedding and adorns them with some additional options (like stripping newlines and batching) to produce implementations of the more full-featured Embedder interface. #374 implements this with interfaces instead of duplicating the same code over and over and over.

[zivkovicn]

@eliben yeah this is great! thanks for putting this together! @tmc do you mind reviewing #374 and merging! IMO this abstraction solves bot problems (usage of Chat llm and duplication).

[tmc]

Merged! Can we consider this resolved?

[zivkovicn]

I think that we are good. Will resolve this issue. Thanks both!

[eliben]

SGTM, I created #379 to track the followup cleanup and refactoring work.