-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add support for EPSILON and WITHATTRIBS arguments in VSIM command #3449
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
Conversation
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.
Pull Request Overview
This PR adds support for the EPSILON argument and WITHATTRIBS flag in the VSIM command for Redis vector similarity search. This enables distance-based filtering of similarity results and retrieval of element attributes along with scores.
- Add EPSILON parameter to VSimArgs for similarity threshold filtering
- Add WITHATTRIBS support with new vsimWithScoreWithAttribs methods returning VSimScoreAttribs objects
- Include comprehensive test coverage for both EPSILON and WITHATTRIBS functionality
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/io/lettuce/core/VSimArgs.java | Adds epsilon method for similarity threshold configuration |
| src/main/java/io/lettuce/core/vector/VSimScoreAttribs.java | New class to hold similarity score and attributes |
| src/main/java/io/lettuce/core/output/VSimScoreAttribsMapOutput.java | Output parser for WITHATTRIBS responses supporting both RESP2/RESP3 |
| src/main/java/io/lettuce/core/RedisVectorSetCommandBuilder.java | Command builder methods for WITHATTRIBS variants |
| src/main/java/io/lettuce/core/protocol/CommandKeyword.java | Adds EPSILON and WITHATTRIBS keywords |
| src/test/java/io/lettuce/core/vector/RedisVectorSetIntegrationTests.java | Integration test for EPSILON functionality |
| src/test/java/io/lettuce/core/VSimWithScoreWithAttribsIntegrationTests.java | Comprehensive integration tests for WITHATTRIBS |
| docs/user-guide/vector-sets.md | Documentation updates for EPSILON and WITHATTRIBS usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| */ | ||
| public VSimArgs epsilon(double delta) { | ||
| if (delta < 0.0 || delta > 1.0) { | ||
| throw new IllegalArgumentException("EPSILON must be in range [0.0, 1.0], got: " + delta); |
Copilot
AI
Sep 18, 2025
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 error message should include information about the valid range format to match the method documentation. Consider changing to 'EPSILON must be in range [0.0, 1.0]' to match line 324 in the test file.
| throw new IllegalArgumentException("EPSILON must be in range [0.0, 1.0], got: " + delta); | |
| throw new IllegalArgumentException("EPSILON must be in range [0.0, 1.0]"); |
| if (output == null) { | ||
| output = new LinkedHashMap<>(1); | ||
| } | ||
| output.put(pendingKey, new VSimScoreAttribs(pendingScore != null ? pendingScore : Double.NaN, pendingAttribs)); |
Copilot
AI
Sep 18, 2025
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.
Using Double.NaN for null scores could cause issues in calculations. Consider using 0.0 or throwing an exception when pendingScore is null, as scores should always be valid doubles in VSIM responses.
| output.put(pendingKey, new VSimScoreAttribs(pendingScore != null ? pendingScore : Double.NaN, pendingAttribs)); | |
| output.put(pendingKey, new VSimScoreAttribs(pendingScore != null ? pendingScore : 0.0, pendingAttribs)); |
| probe.epsilon(0.5); | ||
| redis.vsimWithScore(key, probe, qx, qy); | ||
| } catch (RedisCommandExecutionException e) { | ||
| assumeTrue(false, "Skipping: VSIM EPSILON not supported: " + e.getMessage()); |
Copilot
AI
Sep 18, 2025
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.
[nitpick] Using assumeTrue(false, ...) is unconventional. Use assumeFalse(true, ...) or Assumptions.assumeThat() with a more explicit condition for better readability.
| assumeTrue(false, "Skipping: VSIM EPSILON not supported: " + e.getMessage()); | |
| org.junit.jupiter.api.Assumptions.assumeFalse(true, "Skipping: VSIM EPSILON not supported: " + e.getMessage()); |
| assertThat(res.keySet()).as("top-2 elements should be v1 and v2 for query (1.0,0.0)").containsExactlyInAnyOrder("v1", | ||
| "v2"); |
Copilot
AI
Sep 18, 2025
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.
[nitpick] The assertion message spans multiple lines unnecessarily. Consider using a single line or extracting the message to a variable for better readability.
5056b35 to
0cefe0a
Compare
CAE-1307 - Add support for EPSILON argument in VSIM command
Add WITHATTRIBS argument to VSIM (Add WITHATTRIBS argument to VSIM #3422)
CAE-1418 - add WITHATTRIBS argument to VSIM
Add exceptions to wordlist
Make sure that:
mvn formatter:formattarget. Don’t submit any formatting related changes.