Adds abstract LCEL base class #3
Conversation
dialog_lib/agents/abstract.py
Outdated
| ("human", "{input}"), | ||
| ] | ||
| ) | ||
| return input_text |
dialog_lib/agents/abstract.py
Outdated
| ) | ||
|
|
||
|
|
||
| class AbstractLCELDialog(AbstractLCEL): |
There was a problem hiding this comment.
why do we need an abstraction of an abstraction with LCEL here? It looks more like an rag example which used DialogRetriever, i.e., it'd live in another file like dialog.py where some DialogAgent (more coherent with the parent folder agents) or DialogLLM would inherent from AbstractLCEL. Since AbstractLCEL is abstract, would not need another abstract over it, seems more complicated to understand (there are 5 abstract classes already).
There was a problem hiding this comment.
We don't need it, I'm solving this right now!
| return self.memory.messages | ||
|
|
||
|
|
||
| class AbstractLCEL(AbstractLLM): |
There was a problem hiding this comment.
just a note for new ideas: I know AbstractLLM is a convention from the very beginning of dialog, but since this now lives under agents, maybe it'd be good to start naming classes here as XPTOAgent, which would still be chain wrappers, but making ground to implement real Agents in the future (with LangGraph). Of course, always keeping compatibility.
There was a problem hiding this comment.
@lgabs good question. Can we solve the naming after this PR? So we can make the change with more people's opinions
pyproject.toml
Outdated
| [tool.poetry] | ||
| name = "dialog-lib" | ||
| version = "0.0.1.19" | ||
| version = "0.0.1.20" |
There was a problem hiding this comment.
wouldn't this be a minor increment, adding a new functionality?
There was a problem hiding this comment.
@lgabs good question... Honestly, I don't know. We could bump it for sure
lgabs
left a comment
lgabs
left a comment
There was a problem hiding this comment.
Nice! I've made some reviews about new files introduced and if it's expected in this PR, but the abstract class is now a lot more LCEL adherent.
dialog_lib/agents/abstract.py
Outdated
| threshold=0.3, | ||
| top_k=3 |
dialog_lib/agents/abstract.py
Outdated
| def parse_fallback(ai_message): | ||
| return ai_message.content |
There was a problem hiding this comment.
is this used? I think it'd be used after
fallback_prompt | RunnableLambda(lambda x: x.messages[-1])
to parse the ai message content out to the user.
There was a problem hiding this comment.
Totally not used. We are solving this in the post-processing function, so no need for it!
| from dialog_lib.db.models import CompanyContent | ||
| from dialog_lib.embeddings.generate import generate_embedding | ||
|
|
||
| from langchain_openai import OpenAIEmbeddings | ||
| from langchain_community.document_loaders.csv_loader import CSVLoader | ||
|
|
||
|
|
||
| def load_csv( | ||
| file_path, dbsession, embeddings_model_instance=None, | ||
| embedding_llm_model=None, embedding_llm_api_key=None, company_id=None | ||
| ): | ||
| loader = CSVLoader(file_path=file_path) | ||
| contents = loader.load() | ||
|
|
||
| if not embeddings_model_instance: | ||
| if embedding_llm_model.lower() == "openai": | ||
| embeddings_model_instance = OpenAIEmbeddings(openai_api_key=embedding_llm_api_key) | ||
| else: | ||
| raise ValueError("Invalid embeddings model") | ||
|
|
||
| for csv_content in contents: | ||
| content = {} | ||
|
|
||
| for line in csv_content.page_content.split("\n"): | ||
| values = line.split(": ") | ||
| content[values[0]] = values[1] | ||
|
|
||
| company_content = CompanyContent( | ||
| category="csv", | ||
| subcategory="csv-content", | ||
| question=content["question"], | ||
| content=content["content"], | ||
| dataset=company_id, | ||
| embedding=generate_embedding(csv_content.page_content, embeddings_model_instance) | ||
| ) | ||
| dbsession.add(company_content) | ||
|
|
||
| dbsession.commit() | ||
| return company_content |
There was a problem hiding this comment.
Is this new file here expected in this PR?
There was a problem hiding this comment.
Not in this PR, but its a functionality. I can leave it out of this PR
There was a problem hiding this comment.
I think it's probably better to include in a new PR, with some discussion. I did not understand why it's here and its relation to the load_csv.py in dialog as well.
Here is the Abstract LCEL Class to allow us the support of LCEL using our class calling structure.