Skip to content

Conversation

@uglide
Copy link
Collaborator

@uglide uglide commented Sep 18, 2025

  • 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:

  • You have read the contribution guidelines.
  • You have created a feature request first to discuss your contribution intent. Please reference the feature request ticket number in the pull request.
  • You applied code formatting rules using the mvn formatter:format target. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.

@uglide uglide requested review from atakavci, Copilot, ggivo and tishun and removed request for Copilot September 18, 2025 13:13
Copy link
Contributor

Copilot AI left a 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);
Copy link

Copilot AI Sep 18, 2025

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.

Suggested change
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]");

Copilot uses AI. Check for mistakes.
if (output == null) {
output = new LinkedHashMap<>(1);
}
output.put(pendingKey, new VSimScoreAttribs(pendingScore != null ? pendingScore : Double.NaN, pendingAttribs));
Copy link

Copilot AI Sep 18, 2025

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.

Suggested change
output.put(pendingKey, new VSimScoreAttribs(pendingScore != null ? pendingScore : Double.NaN, pendingAttribs));
output.put(pendingKey, new VSimScoreAttribs(pendingScore != null ? pendingScore : 0.0, pendingAttribs));

Copilot uses AI. Check for mistakes.
probe.epsilon(0.5);
redis.vsimWithScore(key, probe, qx, qy);
} catch (RedisCommandExecutionException e) {
assumeTrue(false, "Skipping: VSIM EPSILON not supported: " + e.getMessage());
Copy link

Copilot AI Sep 18, 2025

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.

Suggested change
assumeTrue(false, "Skipping: VSIM EPSILON not supported: " + e.getMessage());
org.junit.jupiter.api.Assumptions.assumeFalse(true, "Skipping: VSIM EPSILON not supported: " + e.getMessage());

Copilot uses AI. Check for mistakes.
Comment on lines +206 to +207
assertThat(res.keySet()).as("top-2 elements should be v1 and v2 for query (1.0,0.0)").containsExactlyInAnyOrder("v1",
"v2");
Copy link

Copilot AI Sep 18, 2025

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.

Copilot uses AI. Check for mistakes.
@a-TODO-rov a-TODO-rov force-pushed the vsim-epsilon-withattribs branch from 5056b35 to 0cefe0a Compare September 18, 2025 13:16
@a-TODO-rov a-TODO-rov changed the title Add support for EPSILON argument in VSIM command (#3421) Add support for EPSILON and WITHATTRIBS arguments in VSIM command Sep 18, 2025
@uglide uglide merged commit 7ce425d into main Sep 18, 2025
14 of 16 checks passed
@uglide uglide deleted the vsim-epsilon-withattribs branch September 18, 2025 14:41
@ggivo ggivo added the type: feature A new feature label Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[vector sets] Add support for EPSILON and WITHATTRIBS arguments in VSIM command

4 participants