fix: rename ChunksHybridSearchRetriever and add embedding error handling#793
fix: rename ChunksHybridSearchRetriever and add embedding error handling#793Dayang26 wants to merge 2 commits intoMODSetter:mainfrom
Conversation
|
@Dayang26 is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on d970688..cd82e78
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (2)
• surfsense_backend/app/retriever/chunks_hybrid_search.py
• surfsense_backend/app/services/connector_service.py
|
@Dayang26 Thanks. Can you please raise this PR on 'dev' branch |
JiwaniZakir
left a comment
JiwaniZakir
left a comment
There was a problem hiding this comment.
The typo fix in chunks_hybrid_search.py (Chucks → Chunks) is a worthwhile cleanup, and updating the import and instantiation in connector_service.py keeps things consistent.
One concern with the error handling in vector_search: swallowing the exception with return [] after logging silently degrades search quality — the caller in connector_service.py has no way to distinguish "no results found" from "embedding failed entirely." Consider either raising a more specific exception (e.g., EmbeddingError) or returning a sentinel value/None so callers can handle the failure path explicitly rather than treating it as an empty result set.
Also, the log message "fail to call embedding model" should be "failed to call embedding model" for grammatical consistency with the rest of the codebase. It's also worth including the query context (e.g., a truncated query_text) in the log to aid debugging in production.
3 participants
This PR fixes a typo in the
ChunksHybridSearchRetrieverclass name and improves the robustness of the search functionality by adding error handling for embedding generation.Description
ChucksHybridSearchRetrievertoChunksHybridSearchRetrieverto fix the spelling error.embedding_model.embed(query_text)calls intry-exceptblocks withinvector_searchandhybrid_search.[]instead of crashing (or raisingUnboundLocalError) when the embedding model fails to generate embeddings.I didn't add a retry mechanism because I wanted to keep the changes minimal and avoid adding tenacity to pyproject.toml.
Motivation and Context
FIX #783
Screenshots
API Changes
Change Type
Testing Performed
Checklist
Close #783
High-level PR Summary
This PR corrects a typo in the class name from
ChucksHybridSearchRetrievertoChunksHybridSearchRetrieverand adds error handling to thevector_searchmethod. When embedding generation fails, the method now logs the error and returns an empty list instead of crashing, improving the robustness of the search functionality.⏱️ Estimated Review Time: 5-15 minutes
💡 Review Order Suggestion
surfsense_backend/app/retriever/chunks_hybrid_search.pysurfsense_backend/app/services/connector_service.py