search

wagtail / wagtail-vector-index

Store Wagtail pages & Django models as embeddings in vector databases
https://wagtail-vector-index.readthedocs.io/en/latest/
MIT License
15 stars 10 forks source link

Bug in index.query() prevents content from similar documents to be included in LLM query #69

Closed berry closed 2 months ago

berry commented 2 months ago

Index.query() does not use the content of 'similar documents'. The variable 'merged_context' in def query() is always empty. Effectively, this means that the RAG functionality of index.query() is not working.

The reason is that 'similar_documents' is of type iterator. After 'similar_documents' is used in 'sources', the iterator cannot be used again in 'merged_context'. The solution is to convert the iterator to a list at an earlier stage.

berry commented 2 months ago

I've given it some more thought. This solution is only a temporary fix and not future-proof, as it breaks the iterator chain downstream. A better approach might be to create the iterator twice, assuming that creating iterators is relatively inexpensive.

similar_documents_dedup = self.backend_index.similarity_search(query_embedding)
sources = self._deduplicate_list(
            self.get_converter().bulk_from_documents(similar_documents_dedup)
        )

similar_documents_merge = self.backend_index.similarity_search(query_embedding)
merged_context = "\n".join(doc.metadata["content"] for doc in similar_documents_merge)
tomusher commented 2 months ago

Good catch thank you @berry!

I don't think we need to be too concerned about persisting the iterator in the QueryResponse - it's going to be cheaper to convert to a list than regenerate that similarity response.

It would be great to have some tests to catch this issue in the future though - if you have time to add them that would be great.

berry commented 2 months ago

Thanks! I will try to write a test. I don’t write Python tests very often. When I follow the instruction in the README, the installation process is difficult. I get a lot of ‘INFO: pip is looking at multiple versions of […] to determine which version is compatible with other requirements. This could take a while.’ The installation takes forever. Anyway, I will give it another try.

zerolab commented 2 months ago

When I follow the instruction in the README, the installation process is difficult. I get a lot of ‘INFO: pip is looking at multiple versions of […] to determine which version is compatible with other requirements. This could take a while.’ The installation takes forever.

@berry could you open a separate issue with log of pip is looking at multiple versions of ? And any other relevant information that may help us speed things up

tomusher commented 2 months ago

Thanks @berry - I've rebased, tweaked your change to use the new method name and added a test for now so we can get this fix in ASAP. Feel free to add more in another PR if you're interested.

Thanks again for your help with this!