Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add HybridCypherRetriever, refactors Retriever directory, refactor query construction #22

Merged
merged 5 commits into from
Apr 30, 2024
Merged

Add HybridCypherRetriever, refactors Retriever directory, refactor query construction #22

merged 5 commits into from
Apr 30, 2024

Conversation

willtai
Copy link
Contributor

@willtai willtai commented Apr 26, 2024

Summary

  • Created retrievers/ that has the vector and hybrid retrievers in their respective files
  • Created HybridCypherRetriever that is similar to HybridRetriever but requires an additional retrieval query
  • Created get_search_query that constructs the Cypher query based on its SearchType. Inspired by this code in langchain.
  • Adds example for HybridCypherRetriever
  • Adds tests for HybridCypherRetriever
  • Adds tests for get_search_query

@willtai willtai marked this pull request as ready for review April 26, 2024 15:34
@willtai willtai requested a review from a team April 26, 2024 15:34
stellasia
stellasia reviewed Apr 26, 2024
View reviewed changes
Copy link
Contributor

@stellasia stellasia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new naming definitely sounds better :)

I feel like we are missing some e2e tests, was this already discussed/planned?

src/neo4j_genai/queries.py Outdated Show resolved Hide resolved
src/neo4j_genai/retrievers/hybrid.py Outdated Show resolved Hide resolved
@willtai willtai force-pushed the willtai/hybrid-cypher-retriever branch from b612f0b to d2b5e72 Compare April 29, 2024 09:13
@willtai
Copy link
Contributor Author

willtai commented Apr 29, 2024

@stellasia It's not planned atm. I think we can work on that but it will be outside the scope of this PR

@willtai willtai force-pushed the willtai/hybrid-cypher-retriever branch from 5d01573 to 9cca2a3 Compare April 29, 2024 09:20
@willtai willtai force-pushed the willtai/hybrid-cypher-retriever branch from 9cca2a3 to 5a7e139 Compare April 29, 2024 09:25
@willtai willtai force-pushed the willtai/hybrid-cypher-retriever branch from 7ab5cbf to e04c078 Compare April 30, 2024 09:27
@willtai willtai requested a review from oskarhane April 30, 2024 09:30
oskarhane
oskarhane approved these changes Apr 30, 2024
View reviewed changes
Copy link
Member

@oskarhane oskarhane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚋

@willtai willtai merged commit 4e840d0 into main Apr 30, 2024
7 checks passed
@willtai willtai deleted the willtai/hybrid-cypher-retriever branch April 30, 2024 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stellasia stellasia stellasia left review comments

@oskarhane oskarhane oskarhane approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@willtai @oskarhane @stellasia