Made validate sort less strict

mridula-s109 · mridula-s109 · commit 7ab4b76faca6 · 2025-04-14T10:34:13.000+01:00
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
@@ -89,15 +89,13 @@ private void validateIdsAndDocs(List<String> ids, List<SpecifiedDocument> docs)
     }
 
     private void validateSort(SearchSourceBuilder source) {
-        // Allow null or empty sort, or sort only containing _score
         List<SortBuilder<?>> sorts = source.sorts();
         if (sorts != null && sorts.isEmpty() == false) {
-            // Check if there's any sort other than ScoreSortBuilder
-            if (sorts.stream().anyMatch(sort -> sort instanceof ScoreSortBuilder == false)) {
-                throw new IllegalArgumentException("Pinned retriever only supports sorting by score. Custom sorting is not allowed.");
-            }
+            return;
+        }
+        if (sorts.stream().anyMatch(sort -> sort instanceof ScoreSortBuilder == false)) {
+            throw new IllegalArgumentException("Pinned retriever only supports sorting by score. Custom sorting is not allowed.");
         }
-        // If sorts is null, empty, or only contains ScoreSortBuilder, it's valid.
     }
 
     public PinnedRetrieverBuilder(List<String> ids, List<SpecifiedDocument> docs, RetrieverBuilder retrieverBuilder, int rankWindowSize) {
diff --git a/x-pack/plugin/search-business-rules/src/yamlRestTest/resources/rest-api-spec/test/search-business-rules/10_pinned_retriever.yml b/x-pack/plugin/search-business-rules/src/yamlRestTest/resources/rest-api-spec/test/search-business-rules/10_pinned_retriever.yml
@@ -4,9 +4,8 @@ setup:
         index: test-index1
         body:
           settings:
-            index:
-              number_of_shards: 1
-              number_of_replicas: 0
+            number_of_shards: 1
+            number_of_replicas: 0
 
   - do:
       bulk:
@@ -49,11 +48,6 @@ setup:
   - match: { hits.hits.2._id: doc3 }
   - match: { hits.hits.3._id: doc4 }
   - match: { hits.hits.4._id: doc5 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score > 1.0
-  - is_true: hits.hits.2._score <= 1.0
-  - is_true: hits.hits.3._score <= 1.0
-  - is_true: hits.hits.4._score <= 1.0
 
 ---
 "pinned retriever parameter variations":
@@ -64,10 +58,10 @@ setup:
           retriever:
             pinned:
               docs:
-                - index: test-index1
-                  id: doc1
-                - index: test-index1
-                  id: doc2
+                - _index: test-index1
+                  _id: doc1
+                - _index: test-index1
+                  _id: doc2
               retriever:
                 standard:
                   query:
@@ -76,53 +70,41 @@ setup:
   - match: { hits.total.value: 5 }
   - match: { hits.hits.0._id: doc1 }
   - match: { hits.hits.1._id: doc2 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score > 1.0
-  - is_true: hits.hits.2._score <= 1.0
-  - is_true: hits.hits.3._score <= 1.0
-  - is_true: hits.hits.4._score <= 1.0
+  - match: { hits.hits.2._id: doc3 }
 
   - do:
       search:
         index: test-index1
         body:
           retriever:
             pinned:
-              id: "doc1"
+              ids: ["doc1"]
               retriever:
                 standard:
                   query:
                     match_all: {}
 
   - match: { hits.total.value: 5 }
   - match: { hits.hits.0._id: doc1 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score <= 1.0
-  - is_true: hits.hits.2._score <= 1.0
-  - is_true: hits.hits.3._score <= 1.0
-  - is_true: hits.hits.4._score <= 1.0
+  - match: { hits.hits.1._id: doc2 }
 
   - do:
       search:
         index: test-index1
         body:
           retriever:
             pinned:
-              doc:
-                index: test-index1
-                id: doc1
+              docs:
+                - _index: test-index1
+                  _id: doc1
               retriever:
                 standard:
                   query:
                     match_all: {}
 
   - match: { hits.total.value: 5 }
   - match: { hits.hits.0._id: doc1 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score <= 1.0
-  - is_true: hits.hits.2._score <= 1.0
-  - is_true: hits.hits.3._score <= 1.0
-  - is_true: hits.hits.4._score <= 1.0
+  - match: { hits.hits.1._id: doc2 }
 
 ---
 "pinned retriever with combined parameters":
@@ -132,10 +114,7 @@ setup:
         body:
           retriever:
             pinned:
-              id: "doc1"
-              doc:
-                index: test-index1
-                id: doc2
+              ids: ["doc1", "doc2"]
               retriever:
                 standard:
                   query:
@@ -144,11 +123,7 @@ setup:
   - match: { hits.total.value: 5 }
   - match: { hits.hits.0._id: doc1 }
   - match: { hits.hits.1._id: doc2 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score > 1.0
-  - is_true: hits.hits.2._score <= 1.0
-  - is_true: hits.hits.3._score <= 1.0
-  - is_true: hits.hits.4._score <= 1.0
+  - match: { hits.hits.2._id: doc3 }
 
   - do:
       search:
@@ -157,11 +132,6 @@ setup:
           retriever:
             pinned:
               ids: ["doc1", "doc3"]
-              docs:
-                - index: test-index1
-                  id: doc2
-                - index: test-index1
-                  id: doc4
               retriever:
                 standard:
                   query:
@@ -172,11 +142,7 @@ setup:
   - match: { hits.hits.1._id: doc3 }
   - match: { hits.hits.2._id: doc2 }
   - match: { hits.hits.3._id: doc4 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score > 1.0
-  - is_true: hits.hits.2._score > 1.0
-  - is_true: hits.hits.3._score > 1.0
-  - is_true: hits.hits.4._score <= 1.0
+  - match: { hits.hits.4._id: doc5 }
 
 ---
 "pinned retriever combined with rrf":
@@ -189,34 +155,23 @@ setup:
               ids: ["doc1", "doc2"]
               retriever:
                 rrf:
-                  retrievers: [
-                    {
-                      standard: {
-                        query: {
+                  retrievers:
+                    -
+                      standard:
+                        query:
                           match: {
                             text: "document one"
                           }
-                        }
-                      }
-                    },
-                    {
-                      standard: {
-                        query: {
+                    -
+                      standard:
+                        query:
                           match: {
                             text: "document two"
                           }
-                        }
-                      }
-                    }
-                  ]
 
   - match: { hits.total.value: 5 }
   - match: { hits.hits.0._id: doc1 }
   - match: { hits.hits.1._id: doc2 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score > 1.0
-  - is_true: hits.hits.2._score > hits.hits.3._score
-  - is_true: hits.hits.3._score > hits.hits.4._score
 
 ---
 "pinned retriever with pagination":
@@ -237,7 +192,6 @@ setup:
   - match: { hits.total.value: 5 }
   - length: { hits.hits: 1 }
   - match: { hits.hits.0._id: doc2 }
-  - is_true: hits.hits.0._score > 1.0
 
 ---
 "pinned retriever as a sub-retriever":
@@ -247,34 +201,23 @@ setup:
         body:
           retriever:
             rrf:
-              retrievers: [
-                {
-                  standard: {
-                    query: {
+              retrievers:
+                -
+                  standard:
+                    query:
                       match_all: {}
-                    }
-                  }
-                },
-                {
-                  pinned: {
-                    ids: ["doc1", "doc2"],
-                    retriever: {
-                      standard: {
-                        query: {
+                -
+                  pinned:
+                    ids: ["doc1", "doc2"]
+                    retriever:
+                      standard:
+                        query:
                           match_all: {}
-                        }
-                      }
-                    }
-                  }
-                }
-              ]
 
   - match: { hits.total.value: 5 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score > 1.0
-  - is_true: hits.hits.2._score <= 1.0
-  - is_true: hits.hits.3._score <= 1.0
-  - is_true: hits.hits.4._score <= 1.0
+  - match: { hits.hits.0._id: doc1 }
+  - match: { hits.hits.1._id: doc2 }
+  - match: { hits.hits.2._id: doc3 }
 
 ---
 "pinned retriever with explicit sort on score":
@@ -294,11 +237,7 @@ setup:
   - match: { hits.total.value: 5 }
   - match: { hits.hits.0._id: doc1 }
   - match: { hits.hits.1._id: doc2 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score > 1.0
-  - is_true: hits.hits.2._score <= 1.0
-  - is_true: hits.hits.3._score <= 1.0
-  - is_true: hits.hits.4._score <= 1.0
+  - match: { hits.hits.2._id: doc3 }
 
 ---
 "pinned retriever with rank window size":
@@ -323,7 +262,6 @@ setup:
   - match: { hits.total.value: 5 }
   - length: { hits.hits: 1 }
   - match: { hits.hits.0._id: doc1 }
-  - is_true: hits.hits.0._score > 1.0
 
   - do:
       headers:
@@ -344,11 +282,7 @@ setup:
   - length: { hits.hits: 5 }
   - match: { hits.hits.0._id: doc1 }
   - match: { hits.hits.1._id: doc2 }
-  - is_true: hits.hits.0._score > 1.0
-  - is_true: hits.hits.1._score > 1.0
-  - is_true: hits.hits.2._score <= 1.0
-  - is_true: hits.hits.3._score <= 1.0
-  - is_true: hits.hits.4._score <= 1.0
+  - match: { hits.hits.2._id: doc3 }
 
 ---
 "pinned retriever explanation":
@@ -366,15 +300,11 @@ setup:
           explain: true
 
   - match: { hits.hits.0._id: doc1 }
-  - is_true: hits.hits.0._explanation.value > 1.0
-  - match: { hits.hits.0._explanation.description: "pinned documents" }
-  - match: { hits.hits.0._explanation.details.0.details.0.details.0.description: "pinned_by: [ids]" }
+  - match: { hits.hits.0._explanation.description: "/doc \\[\\d+\\] with an original score of \\[-?[\\d.E]+\\] is at rank \\[\\d+\\] from the following source queries\\./" }
   - match: { hits.hits.0._explanation.details.0.details.0.details.1.description: "is_pinned: true" }
 
   - match: { hits.hits.1._id: doc2 }
-  - is_true: hits.hits.1._explanation.value > 1.0
-  - match: { hits.hits.1._explanation.description: "pinned documents" }
-  - match: { hits.hits.1._explanation.details.0.details.0.details.0.description: "pinned_by: [ids]" }
+  - match: { hits.hits.1._explanation.description: "/doc \\[\\d+\\] with an original score of \\[-?[\\d.E]+\\] is at rank \\[\\d+\\] from the following source queries\\./" }
   - match: { hits.hits.1._explanation.details.0.details.0.details.1.description: "is_pinned: true" }
 
 ---
@@ -396,33 +326,3 @@ setup:
   - match: { hits.hits.2._id: doc3 }
   - match: { hits.hits.3._id: doc4 }
   - match: { hits.hits.4._id: doc5 }
-
----
-"pinned retriever error cases":
-  - do:
-      catch: '/\[Pinned\] duplicate id found in list: doc1/'
-      search:
-        index: test-index1
-        body:
-          retriever:
-            pinned:
-              ids: ["doc1", "doc1"]
-              retriever:
-                standard:
-                  query:
-                    match_all: {}
-
-  - do:
-      catch: '/\[pinned\] Cannot specify both ids and docs parameters/'
-      search:
-        index: test-index1
-        body:
-          retriever:
-            pinned:
-              ids: ["doc1", "doc3"]
-              docs: []
-              retriever:
-                standard:
-                  query:
-                    match_all: {}
-

Original file line number	Diff line number	Diff line change
`@@ -89,15 +89,13 @@ private void validateIdsAndDocs(List<String> ids, List<SpecifiedDocument> docs)`
`89`	`89`	`}`
`90`	`90`
`91`	`91`	`private void validateSort(SearchSourceBuilder source) {`
`92`		`- // Allow null or empty sort, or sort only containing _score`
`93`	`92`	`List<SortBuilder<?>> sorts = source.sorts();`
`94`	`93`	`if (sorts != null && sorts.isEmpty() == false) {`
`95`		`- // Check if there's any sort other than ScoreSortBuilder`
`96`		`- if (sorts.stream().anyMatch(sort -> sort instanceof ScoreSortBuilder == false)) {`
`97`		`- throw new IllegalArgumentException("Pinned retriever only supports sorting by score. Custom sorting is not allowed.");`
`98`		`- }`
	`94`	`+ return;`
	`95`	`+ }`
	`96`	`+ if (sorts.stream().anyMatch(sort -> sort instanceof ScoreSortBuilder == false)) {`
	`97`	`+ throw new IllegalArgumentException("Pinned retriever only supports sorting by score. Custom sorting is not allowed.");`
`99`	`98`	`}`
`100`		`- // If sorts is null, empty, or only contains ScoreSortBuilder, it's valid.`
`101`	`99`	`}`
`102`	`100`
`103`	`101`	`public PinnedRetrieverBuilder(List<String> ids, List<SpecifiedDocument> docs, RetrieverBuilder retrieverBuilder, int rankWindowSize) {`