-
Notifications
You must be signed in to change notification settings - Fork 5
feat: implement vector search UI #23
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.
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
feat: implement vector search UI #23
Conversation
- Replace List.of() with Arrays.asList() to handle null values in MongoDB aggregation pipeline - Add safe type conversion for year field to handle dirty data (e.g., '1994è1998') - Improve error handling with specific catch blocks for IOException and InterruptedException - Add validation for Voyage AI API response structure - Remove debug logging statements
- Add missing import for voyage_ai_available function - Add conditional projection to handle non-integer year values (e.g., '1994è1998') - Return None for invalid year data instead of causing validation errors
dacharyc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At a high level, this looks fine. I only tried it with the Java backend - I didn't run the Python backend - LMK if you'd like me to test it with Python, too.
I did spot a couple of things with Java that I'd love to see us handle. Presumably it's out of scope of MVP at this point given current timeframe, but could we capture these in a doc for future improvements?
| } | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The frontend doesn't seem to be handling a case gracefully where the backend is missing the Voyage API key - at least, for Java. Didn't try it with Python. If I omit the Voyage API key from my env file, I don't get any feedback during startup, but when I try to use Vector Search, I see this in the console:
2025-11-06T09:53:28.258-05:00 ERROR 40074 --- [sample-app-java-mflix] [nio-3001-exec-6] c.m.s.exception.GlobalExceptionHandler : Validation error: Vector search unavailable: VOYAGE_API_KEY not configured. Please add your API key to the application.properties file
And the frontend shows this error:
We should probably be handling this with an HTTP status code - I guess a 401 Unauthorized - which we could display in the frontend.
Presumably we don't have time to scope this for MVP, but as a fast follow, we should scope unifying the error handling across the server backends to throw the appropriate HTTP error codes and handle those in the frontend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good callout, I agree, I think we need some error handling here and probably a general error page. Added to the list of fast follows
| results.add(result); | ||
| ObjectId movieId = doc.getObjectId("_id"); | ||
| movieIds.add(movieId); | ||
| scoreMap.put(movieId.toString(), doc.getDouble("score")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we're returning the score, but I don't see anything in the frontend where we expose this nor do I see it when the server is running.
I got some results that don't make sense to me when I did a few different searches, so it would be great to see the scores somewhere - either the UI, or as a console log from the server. Presumably this is OOS for MVP but I'd love to see it as a fast follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add it to the UI, I could see it being part of the movie card potentially. Added to the fast follows list
Adds UI for vector search and adjusts the backends slightly so search results can show on the UI