Feature/redbox 200 move sources to a separate field for chat endpoint responses #298
Conversation
| models.ManyToManyField( | ||
| blank=True, related_name="chat_histories", to="redbox_core.file" | ||
| ), | ||
| models.ManyToManyField(blank=True, related_name="chat_histories", to="redbox_core.file"), |
There was a problem hiding this comment.
this is ruff modifying some unrelated files
| processing_status = models.CharField( | ||
| choices=ProcessingStatusEnum.choices, null=False, blank=False | ||
| ) | ||
| processing_status = models.CharField(choices=ProcessingStatusEnum.choices, null=False, blank=False) |
| @@ -112,7 +112,7 @@ def simple_chat(chat_request: ChatRequest) -> ChatResponse: | |||
| return ChatResponse(response_message=ChatMessage(text=response.text, role="ai")) | |||
|
|
|||
|
|
|||
| @chat_app.post("/rag", tags=["chat"], response_model=ChatResponse) | |||
| @chat_app.post("/rag", tags=["chat"]) | |||
There was a problem hiding this comment.
Does the response model need to be returned?
There was a problem hiding this comment.
not if it is in the annotation (below). ofc its not doing any harm either, happy to put it back?
redbox/models/chat.py
Outdated
| question: str = Field( | ||
| description="original question", | ||
| examples=["Who is the prime minister?"], | ||
| ) |
There was a problem hiding this comment.
Why do you need this? I think it makes things more complicated -- the frontend already has this information because it submitted it.
There was a problem hiding this comment.
the frontend knows all of the documents that could have been used in RAG, but not the documents actually used in RAG. The point of this change is that the document references come back in a helpful format rather-than/as-well-as the Sources: <Doc37518e8f-f6af-4f5f-bdcd-49e2f1b65fc1> <Docf36e683f-0f9e-4573-b122-1f90b65d5ad1> text format
There was a problem hiding this comment.
I think I highlighted too much -- this is a point about the question param. The front end submitted that.
There was a problem hiding this comment.
ahh, good point. yes lets drop it, its unhelpful
redbox/models/chat.py
Outdated
| description="original question", | ||
| examples=["Who is the prime minister?"], | ||
| ) | ||
| input_documents: Optional[list[InputDocuments]] = Field( |
There was a problem hiding this comment.
I've been using LangChain's own Documents class. Is there a reason to roll our own? Pydantic v1 stuff?
ALSO: "input" might be misleading -- down the line we might have lots of file inputs, but the LLM only chooses certain chunks as sources for this response.
There was a problem hiding this comment.
we absolutely could use the LangChain Document class but it is very underspecified, i.e. the Matadata is just a dictionary. but... im in two minds here. The point of this PR is to return structured data, however we cant be sure that LangChain is going to do this 🤔 . What do you think @lmwilkigov ?
ALSO: "input" might be misleading -- down the line we might have lots of file inputs, but the LLM only chooses certain chunks as sources for this response.
This is what i was trying to say here but put much better! we could rename the field to input_documents_used ?
There was a problem hiding this comment.
Fair play on the Document point! I think Metadata is underspecified because it can contain things like what tools were called by an agent -- it's deliberately a bit open-ended and extendable. If we want to be stricter, cool, it's helpful for the frontend.
On name I'd been going with sources to conceptually align with what it is to the user. I'm not married to it but input* as a word is very unaligned with the frontend cause it'll have passed a bunch of inputs, and this attribute is quite loosely coupled to what they'd have been.
There was a problem hiding this comment.
sources sounds good, it is what appears in the text afterall
gecBurton
force-pushed
the
feature/REDBOX-200-move-sources-to-a-separate-field-for-chat-endpoint-responses
branch
from
f1dc9c7 to
55f5685
Compare
gecBurton
deleted the
feature/REDBOX-200-move-sources-to-a-separate-field-for-chat-endpoint-responses
branch
Context
As a frontend I want the backed to return as much information as possible., i.e. the uuid of the original file.
Changes proposed in this pull request
ChatResponseto return the full langchain responseGuidance to review
Relevant links
https://technologyprogramme.atlassian.net/browse/REDBOX-200
Things to check