Skip to content

Fix RAG retrieval silent failure and question key mismatch (#130) - #161

Open
rahulkate173 wants to merge 1 commit into
sugarlabs:mainfrom
rahulkate173:fix-rag-retrieval-130
Open

Fix RAG retrieval silent failure and question key mismatch (#130)#161
rahulkate173 wants to merge 1 commit into
sugarlabs:mainfrom
rahulkate173:fix-rag-retrieval-130

Conversation

@rahulkate173

Copy link
Copy Markdown

Fix for Defect 1 (Core Retrieval Bug)

  • Renamed Variable: Changed self.retriever to self.vector_store to store the raw FAISS index instead of the LangChain VectorStoreRetriever wrapper.
  • Updated setup_vectorstore(): Removed the .as_retriever() call and now assigns the FAISS object directly to self.vector_store.
  • Rewrote get_relevant_document():
    • Added a None guard to handle cases where the vector store hasn't been initialized.
    • Now calls self.vector_store.similarity_search_with_score(query, k=1) directly to retrieve (Document, float) tuples (where the float is the L2 distance).
    • Converted the L2 distance to a similarity score using 1.0 / (1.0 + distance). This ensures the threshold comparison works as expected (higher score = more similar).

Note Regarding Defect 2

The reported wrong-key bug ("query" vs "question") appears to have been resolved in a prior refactor. The run() method no longer uses a LangChain chain with RunnablePassthrough. Instead, it already formats the prompt directly via self.prompt_template.format(question=..., context=...) and calls self.provider.generate(). No further changes were required for this defect.

Closes #130

@Noaman-Akhtar

Copy link
Copy Markdown
Contributor

i went through both PR 160 and PR 161, and they look like two different solutions for the same current retrieval issue. am i missing some difference between them, or is PR 161 meant to replace PR 160?

@rahulkate173

Copy link
Copy Markdown
Author

Yes pr 161 is meant to replace pr 160

@Noaman-Akhtar

Copy link
Copy Markdown
Contributor

Then you should close pr 160 , so it doesn't create confusion

@rahulkate173

Copy link
Copy Markdown
Author

Done

Pr 160 closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The RAG retrieval path in run() is non-functional due to two compounding defects

2 participants