From 0c47ed19530ef339bd5fe62261784a91b4626d71 Mon Sep 17 00:00:00 2001 From: Ioana Tagirta Date: Sun, 11 May 2025 16:17:57 +0200 Subject: [PATCH 1/4] Change error message for negative rank_window_size --- .../retriever/CompoundRetrieverBuilder.java | 8 ++++ .../search/retriever/RetrieversFeatures.java | 6 +++ .../test/linear/10_linear_retriever.yml | 38 +++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java b/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java index 0bb5fd849bbcf..03b041fb8cc6d 100644 --- a/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java @@ -247,6 +247,14 @@ public ActionRequestValidationException validate( if (isScroll) { validationException = addValidationError("cannot specify [" + getName() + "] and [scroll]", validationException); } + if (rankWindowSize < 0) { + validationException = addValidationError( + "[" + getRankWindowSizeField().getPreferredName() + + "] parameter cannot be negative, found [" + rankWindowSize + "]", + validationException + ); + } + for (RetrieverSource innerRetriever : innerRetrievers) { validationException = innerRetriever.retriever().validate(source, validationException, isScroll, allowPartialSearchResults); if (innerRetriever.retriever() instanceof CompoundRetrieverBuilder compoundChild) { diff --git a/server/src/main/java/org/elasticsearch/search/retriever/RetrieversFeatures.java b/server/src/main/java/org/elasticsearch/search/retriever/RetrieversFeatures.java index bfd6f572a9e65..46ebef50d47d8 100644 --- a/server/src/main/java/org/elasticsearch/search/retriever/RetrieversFeatures.java +++ b/server/src/main/java/org/elasticsearch/search/retriever/RetrieversFeatures.java @@ -19,9 +19,15 @@ * retrievers can be added individually with additional functionality. */ public class RetrieversFeatures implements FeatureSpecification { + public static final NodeFeature NEGATIVE_RANK_WINDOW_SIZE_FIX = new NodeFeature("retriever.negative_rank_window_size_fix"); @Override public Set getFeatures() { return Set.of(); } + + @Override + public Set getTestFeatures() { + return Set.of(NEGATIVE_RANK_WINDOW_SIZE_FIX); + } } diff --git a/x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml b/x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml index 70db6c1543365..b86213abecb20 100644 --- a/x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml +++ b/x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml @@ -1063,3 +1063,41 @@ setup: - close_to: { hits.hits.0._score: { value: 10.5, error: 0.001 } } - match: { hits.hits.1._id: "1" } - match: { hits.hits.1._score: 10 } + +--- +"should throw when rank_window_size is negative": + - requires: + cluster_features: [ "retriever.negative_rank_window_size_fix" ] + reason: "Fix for negative rank_window_size error message" + - do: + catch: /\[rank_window_size\] parameter cannot be negative, found \[-10\]/ + search: + index: test + body: + retriever: + linear: + retrievers: [ + { + retriever: { + standard: { + query: { + match_all: { } + } + } + }, + weight: 10.0, + normalizer: "minmax" + }, + { + retriever: { + knn: { + field: "vector", + query_vector: [ 4 ], + k: 1, + num_candidates: 1 + } + }, + weight: 2.0 + } + ] + rank_window_size: -10 From 2cd5a0fa984f3a107f9cdc3035e975d38abfe8fc Mon Sep 17 00:00:00 2001 From: Ioana Tagirta Date: Fri, 16 May 2025 22:32:16 +0200 Subject: [PATCH 2/4] Add rankWindowSize method to base class --- .../search/retriever/CompoundRetrieverBuilder.java | 4 ++++ .../rules/retriever/QueryRuleRetrieverBuilder.java | 4 ---- .../textsimilarity/TextSimilarityRankRetrieverBuilder.java | 4 ---- .../searchbusinessrules/retriever/PinnedRetrieverBuilder.java | 4 ---- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java b/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java index 03b041fb8cc6d..8e7597ab47144 100644 --- a/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java @@ -287,6 +287,10 @@ public int doHashCode() { return Objects.hash(innerRetrievers); } + public int rankWindowSize() { + return rankWindowSize; + } + protected final SearchSourceBuilder createSearchSourceBuilder(PointInTimeBuilder pit, RetrieverBuilder retrieverBuilder) { var sourceBuilder = new SearchSourceBuilder().pointInTimeBuilder(pit) .trackTotalHits(false) diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java index ac359bc4446f3..b724cd133f0c5 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java @@ -119,10 +119,6 @@ public String getName() { return NAME; } - public int rankWindowSize() { - return rankWindowSize; - } - @Override protected SearchSourceBuilder finalizeSourceBuilder(SearchSourceBuilder source) { checkValidSort(source.sorts()); diff --git a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java index d6883d3743a1d..4f913e28539ab 100644 --- a/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java +++ b/x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/rank/textsimilarity/TextSimilarityRankRetrieverBuilder.java @@ -193,10 +193,6 @@ public String inferenceId() { return inferenceId; } - public int rankWindowSize() { - return rankWindowSize; - } - public boolean failuresAllowed() { return failuresAllowed; } diff --git a/x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java b/x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java index 2e7c18e6a85b9..60ac7019d6a90 100644 --- a/x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java +++ b/x-pack/plugin/search-business-rules/src/main/java/org/elasticsearch/xpack/searchbusinessrules/retriever/PinnedRetrieverBuilder.java @@ -151,10 +151,6 @@ public String getName() { return NAME; } - public int rankWindowSize() { - return rankWindowSize; - } - /** * Creates a PinnedQueryBuilder with the appropriate pinned documents. * From 79ba14e1019157eaa7873a313b1a88ea4f8218b7 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Fri, 16 May 2025 20:53:10 +0000 Subject: [PATCH 3/4] [CI] Auto commit changes from spotless --- .../search/retriever/CompoundRetrieverBuilder.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java b/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java index 8e7597ab47144..6cf0af0ef1541 100644 --- a/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java @@ -249,8 +249,7 @@ public ActionRequestValidationException validate( } if (rankWindowSize < 0) { validationException = addValidationError( - "[" + getRankWindowSizeField().getPreferredName() + - "] parameter cannot be negative, found [" + rankWindowSize + "]", + "[" + getRankWindowSizeField().getPreferredName() + "] parameter cannot be negative, found [" + rankWindowSize + "]", validationException ); } From 626ea09cfef8e67327b90520155e9ad3ac39f6d7 Mon Sep 17 00:00:00 2001 From: Ioana Tagirta Date: Mon, 19 May 2025 17:32:36 +0200 Subject: [PATCH 4/4] Check response status --- .../resources/rest-api-spec/test/linear/10_linear_retriever.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml b/x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml index b86213abecb20..2d644a5746479 100644 --- a/x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml +++ b/x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/10_linear_retriever.yml @@ -1101,3 +1101,4 @@ setup: } ] rank_window_size: -10 + - match: { status: 400 }