diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/reranking/ScoredDocument.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/reranking/ScoredDocument.java index de61c343a2..2c9cc8626c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/reranking/ScoredDocument.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/reranking/ScoredDocument.java @@ -123,26 +123,9 @@ static ScoredDocument create( ? DocumentScores.fromBm25Read(rank) : DocumentScores.EMPTY; + // only text can be a passage, missing, null and other types are dropped var passageNode = passageLocator.findValueIn(document); - var passage = - switch (passageNode) { - case MissingNode ignored -> { - // undefined in the document, default to null so it is dropped - yield null; - } - case NullNode ignored -> { - // explicit {$vectorize : null} treat same as undefined, empty passage to rerank on - yield null; - } - case ValueNode valueNode -> { - // could be text, number, boolean, but not array or object - yield valueNode.asText(); - } - default -> - throw new IllegalArgumentException( - "Passage field %s is present but not null or a valueNode _id=%s , passageField=%s" - .formatted(passageLocator.path(), documentId, passageNode)); - }; + var passage = passageNode.isTextual() ? passageNode.textValue() : null; // we will have one or the other of the vector or bm25 scores, merging handles this. // normalise passage to null, to make it easier to use optional. diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndRerankCollectionSuccessIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndRerankCollectionSuccessIntegrationTest.java index 0d5c1ff380..6c3df7807f 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndRerankCollectionSuccessIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindAndRerankCollectionSuccessIntegrationTest.java @@ -185,6 +185,27 @@ void missingPassagesAvoidProviderCalls() { reranker.verify(0, postRequestedFor(urlEqualTo(RerankingTestResource.PATH))); } + @Test + void nonStringPassagesAreExcluded() { + insertDocuments( + collectionName, + List.of( + Map.of("_id", "text", "$vector", VECTOR, "content", "score:1 text passage"), + Map.of("_id", "number", "$vector", VECTOR, "content", 42), + Map.of("_id", "boolean", "$vector", VECTOR, "content", true), + Map.of("_id", "array", "$vector", VECTOR, "content", List.of("score:2 array")), + Map.of("_id", "object", "$vector", VECTOR, "content", Map.of("text", "score:3")))); + + findAndRerank( + collectionName, + Map.of( + "sort", Map.of("$hybrid", Map.of("$vector", VECTOR)), + "options", Map.of("rerankQuery", "query", "rerankOn", "content"))) + .body("data.documents._id", contains("text")); + + assertThat(requestedPassages()).containsExactly("score:1 text passage"); + } + @Test @DisabledIfSystemProperty(named = TEST_PROP_LEXICAL_DISABLED, matches = "true") void hybridCandidatesAreDeduplicatedBeforeReranking() { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/reranking/ScoredDocumentTests.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/reranking/ScoredDocumentTests.java index 3c63398a6f..d48fc33568 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/reranking/ScoredDocumentTests.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/reranking/ScoredDocumentTests.java @@ -270,47 +270,23 @@ public void testCreatePassageField() { var docWithNumber = baseDocument().put(VECTOR_EMBEDDING_TEXT_FIELD, 1); var numberField = ScoredDocument.create(1, Rank.RankSource.VECTOR, docWithNumber, VECTORIZE_LOCATOR); - assertThat(numberField.passage()) - .as("passage is present and correct when field is a number") - .isPresent() - .hasValue("1"); + assertThat(numberField.passage()).as("passage missing when field is a number").isEmpty(); var docWithBoolean = baseDocument().put(VECTOR_EMBEDDING_TEXT_FIELD, true); var booleanField = ScoredDocument.create(1, Rank.RankSource.VECTOR, docWithBoolean, VECTORIZE_LOCATOR); - assertThat(booleanField.passage()) - .as("passage is present and correct when field is a boolean") - .isPresent() - .hasValue("true"); - - var docWithObjectPassage = baseDocument(); - docWithObjectPassage.putObject(VECTOR_EMBEDDING_TEXT_FIELD); - assertThatThrownBy( - () -> - ScoredDocument.create( - 1, Rank.RankSource.VECTOR, docWithObjectPassage, VECTORIZE_LOCATOR)) - .as("Cannot use object as passage node") - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Passage field %s is present but not null or a valueNode _id=%s , passageField=%s" - .formatted( - VECTOR_EMBEDDING_TEXT_FIELD, - ID_1, - docWithObjectPassage.get(VECTOR_EMBEDDING_TEXT_FIELD))); - - var docWithArrayPassage = baseDocument(); - docWithArrayPassage.putArray(VECTOR_EMBEDDING_TEXT_FIELD); - assertThatThrownBy( - () -> - ScoredDocument.create( - 1, Rank.RankSource.VECTOR, docWithArrayPassage, VECTORIZE_LOCATOR)) - .as("Cannot use array as passage node") - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Passage field %s is present but not null or a valueNode _id=%s , passageField=%s" - .formatted( - VECTOR_EMBEDDING_TEXT_FIELD, - ID_1, - docWithArrayPassage.get(VECTOR_EMBEDDING_TEXT_FIELD))); + assertThat(booleanField.passage()).as("passage missing when field is a boolean").isEmpty(); + + var docWithObject = baseDocument(); + docWithObject.putObject(VECTOR_EMBEDDING_TEXT_FIELD).put("text", PASSAGE); + var objectField = + ScoredDocument.create(1, Rank.RankSource.VECTOR, docWithObject, VECTORIZE_LOCATOR); + assertThat(objectField.passage()).as("passage missing when field is an object").isEmpty(); + + var docWithArray = baseDocument(); + docWithArray.putArray(VECTOR_EMBEDDING_TEXT_FIELD).add(PASSAGE); + var arrayField = + ScoredDocument.create(1, Rank.RankSource.VECTOR, docWithArray, VECTORIZE_LOCATOR); + assertThat(arrayField.passage()).as("passage missing when field is an array").isEmpty(); } }