Implement multimodal( FTS, vector, hybrid) search capability#463
Implement multimodal( FTS, vector, hybrid) search capability#463subin-chella wants to merge 1 commit into
Conversation
- Add new multiModalSearch function in lanceTableWrapper - Support vector, text, and hybrid search modes - Integrate with MainSidebar component for user interface - Update DBResultPreview and DBSearchPreview to handle new result format - Improve search result display with conditional rendering fixes 86
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements multimodal search capability, adding support for vector, text, and hybrid search modes across several components. The changes address the issue of providing a literal text search option alongside the existing AI-based similarity search.
- Added
multiModalSearchfunction inLanceDBTableWrapperclass, supporting vector, text, and hybrid search types - Introduced new IPC handler for multimodal search in
ipcHandlers.ts - Updated
SearchComponentwith a dropdown to select search type (vector, text, or hybrid) - Modified
DBSearchPreviewcomponent to conditionally render similarity scores for vector search results - Improved search results display in
SearchComponent, separating vector and text results
7 file(s) reviewed, 14 comment(s)
Edit PR Review Bot Settings | Greptile
| return searchResults | ||
| }) | ||
|
|
||
| ipcMain.handle( |
There was a problem hiding this comment.
style: The multi-modal search handler is implemented correctly, but consider adding error handling for invalid searchType values.
| ? `${filter} AND content LIKE '%${sanitizedTextQuery}%'` | ||
| : `content LIKE '%${sanitizedTextQuery}%'` |
There was a problem hiding this comment.
style: LIKE with wildcards on both sides can be slow for large datasets. Consider using full-text search capabilities if available in LanceDB
| vectorResults = rawVectorResults | ||
| .map(convertRecordToDBType<DBQueryResult>) | ||
| .filter((r): r is DBQueryResult => r !== null) |
There was a problem hiding this comment.
logic: Filtering out null results here may lead to fewer results than the specified limit. Consider adjusting the limit to account for potential null results
| limit: number, | ||
| searchType: 'vector' | 'text' | 'hybrid', | ||
| filter?: string, | ||
| ) => Promise<{ vectorResults: DBQueryResult[]; textResults: DBQueryResult[] }> |
There was a problem hiding this comment.
style: Consider adding a type alias for the return type of multiModalSearch for better readability and maintainability.
| {entry._distance != null && ( | ||
| <> | ||
| {/* eslint-disable-next-line no-underscore-dangle */}| Similarity:{' '} | ||
| {cosineDistanceToPercentage(entry._distance)}%{' '} | ||
| </> | ||
| )}{' '} |
There was a problem hiding this comment.
style: Consider moving this logic to a separate function for better readability and reusability
| {cosineDistanceToPercentage(entry._distance)}%{' '} | ||
| </> | ||
| )}{' '} | ||
| | {modified && <span className="text-xs text-gray-400">Modified {modified}</span>} |
There was a problem hiding this comment.
style: Add a space before the pipe character for consistent formatting
| const [searchResults, setSearchResults] = useState<{ vectorResults: DBQueryResult[]; textResults: DBQueryResult[] }>({ | ||
| vectorResults: [], | ||
| textResults: [], | ||
| }) |
There was a problem hiding this comment.
style: Consider using a more specific type for searchResults instead of an inline object type
| {searchResults?.textResults?.length > 0 && ( | ||
| <div className="mt-4 w-full"> | ||
| <h3 className="mb-2 text-white">Text Search Results</h3> | ||
| {searchResults.textResults.map((result, index) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <DBSearchPreview key={`text-${index}`} dbResult={result} onSelect={openFileSelectSearch} /> | ||
| ))} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
style: Add error handling for cases where searchResults is undefined
|
thanking you for attempting this @subin-chella imo this should be solved with a full text search index not sql expression like you have implemented in pr. @milaiwi has been working on this exact implementation so would be good to hear thoughts |
2 participants
fixes Is there a search for text option? #86
Test:
https://github.com/user-attachments/assets/ca162f51-e005-4840-a3be-3ce72a7438f2