⚰️(backend) remove sorting
sorting is not supported with hybrid search. I should have removed it long ago. It is not supported also with rerank.
This commit is contained in:
@@ -43,3 +43,6 @@ and this project adheres to
|
||||
|
||||
- 🐛(backend) fix missing index creation in 'index/' view
|
||||
- 🐛(backend) fix parallel test execution issues
|
||||
|
||||
## Removed
|
||||
- 🗑️(backend) remove sorting
|
||||
@@ -38,9 +38,6 @@ UPDATED_AT = "updated_at"
|
||||
USERS = "users"
|
||||
GROUPS = "groups"
|
||||
|
||||
RELEVANCE = "relevance"
|
||||
|
||||
ORDER_BY_OPTIONS = (RELEVANCE, TITLE, CREATED_AT, UPDATED_AT, SIZE, REACH)
|
||||
SOURCE_FIELDS = (
|
||||
TITLE,
|
||||
CONTENT,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"""Pydantic model to validate documents before indexation."""
|
||||
|
||||
from typing import Annotated, List, Literal, Optional
|
||||
from typing import Annotated, List, Optional
|
||||
|
||||
from django.utils import timezone
|
||||
from django.utils.text import slugify
|
||||
@@ -116,8 +116,6 @@ class SearchQueryParametersSchema(BaseModel):
|
||||
reach: Optional[enums.ReachEnum] = None
|
||||
tags: StringListParameter = Field(default_factory=list)
|
||||
path: Optional[str] = None
|
||||
order_by: Optional[Literal[enums.ORDER_BY_OPTIONS]] = Field(default=enums.RELEVANCE)
|
||||
order_direction: Optional[Literal["asc", "desc"]] = Field(default="desc")
|
||||
nb_results: Optional[conint(ge=1, le=300)] = Field(default=50)
|
||||
search_type: Optional[SearchTypeEnum] = Field(default=None)
|
||||
rerank: Optional[bool] = Field(default=None)
|
||||
|
||||
@@ -18,8 +18,6 @@ logger = logging.getLogger(__name__)
|
||||
def search( # noqa : PLR0913
|
||||
q,
|
||||
nb_results,
|
||||
order_by,
|
||||
order_direction,
|
||||
search_indices,
|
||||
reach,
|
||||
visited,
|
||||
@@ -50,11 +48,6 @@ def search( # noqa : PLR0913
|
||||
"number_of_users": {"script": {"source": "doc['users'].size()"}},
|
||||
"number_of_groups": {"script": {"source": "doc['groups'].size()"}},
|
||||
},
|
||||
"sort": get_sort(
|
||||
query_keys=query.keys(),
|
||||
order_by=order_by,
|
||||
order_direction=order_direction,
|
||||
),
|
||||
"size": nb_results,
|
||||
"query": query,
|
||||
},
|
||||
@@ -237,19 +230,6 @@ def get_filter( # noqa : PLR0913
|
||||
return filters
|
||||
|
||||
|
||||
def get_sort(query_keys, order_by, order_direction):
|
||||
"""Build OpenSearch sort clause"""
|
||||
# Add sorting logic based on relevance or specified field
|
||||
if "hybrid" in query_keys:
|
||||
# sorting by other field than "_score" is not supported in hybrid search
|
||||
# see: https://github.com/opensearch-project/neural-search/issues/866
|
||||
return {"_score": {"order": order_direction}}
|
||||
if order_by == enums.RELEVANCE:
|
||||
return {"_score": {"order": order_direction}}
|
||||
|
||||
return {order_by: {"order": order_direction}}
|
||||
|
||||
|
||||
def get_params(query_keys):
|
||||
"""Build OpenSearch search parameters"""
|
||||
if "hybrid" in query_keys:
|
||||
|
||||
@@ -6,7 +6,6 @@ of documents is slow and better done only once.
|
||||
"""
|
||||
|
||||
import logging
|
||||
import operator
|
||||
import random
|
||||
from unittest import mock
|
||||
|
||||
@@ -500,181 +499,6 @@ def test_api_documents_hybrid_search(settings):
|
||||
}
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_search_ordering_by_fields(settings):
|
||||
"""It should be possible to order by several fields"""
|
||||
setup_oicd_resource_server(responses, settings, sub="user_sub")
|
||||
responses.add(
|
||||
responses.POST,
|
||||
settings.EMBEDDING_API_PATH,
|
||||
json=albert_embedding_response.response,
|
||||
status=200,
|
||||
)
|
||||
service = factories.ServiceFactory()
|
||||
documents = factories.DocumentSchemaFactory.build_batch(
|
||||
4, reach=random.choice(["public", "authenticated"])
|
||||
)
|
||||
prepare_index(service.index_name, documents)
|
||||
|
||||
parameters = [
|
||||
(enums.CREATED_AT, "asc"),
|
||||
(enums.CREATED_AT, "desc"),
|
||||
(enums.UPDATED_AT, "asc"),
|
||||
(enums.UPDATED_AT, "desc"),
|
||||
(enums.SIZE, "asc"),
|
||||
(enums.SIZE, "desc"),
|
||||
(enums.REACH, "asc"),
|
||||
(enums.REACH, "desc"),
|
||||
]
|
||||
|
||||
for field, direction in parameters:
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/search/",
|
||||
{
|
||||
"q": "*",
|
||||
"order_by": field,
|
||||
"order_direction": direction,
|
||||
"visited": [doc["id"] for doc in documents],
|
||||
},
|
||||
format="json",
|
||||
HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}",
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert len(data) == 4
|
||||
|
||||
# Check that results are sorted by the field as expected
|
||||
compare = operator.le if direction == "asc" else operator.ge
|
||||
for i in range(len(data) - 1):
|
||||
assert compare(data[i]["_source"][field], data[i + 1]["_source"][field])
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_search_ordering_by_relevance(settings):
|
||||
"""It should be possible to order by relevance (score)"""
|
||||
setup_oicd_resource_server(responses, settings, sub="user_sub")
|
||||
responses.add(
|
||||
responses.POST,
|
||||
settings.EMBEDDING_API_PATH,
|
||||
json=albert_embedding_response.response,
|
||||
status=200,
|
||||
)
|
||||
service = factories.ServiceFactory()
|
||||
documents = factories.DocumentSchemaFactory.build_batch(
|
||||
4, reach=random.choice(["public", "authenticated"])
|
||||
)
|
||||
prepare_index(service.index_name, documents)
|
||||
|
||||
for direction in ["asc", "desc"]:
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/search/",
|
||||
{
|
||||
"q": "*",
|
||||
"order_by": "relevance",
|
||||
"order_direction": direction,
|
||||
"visited": [doc["id"] for doc in documents],
|
||||
},
|
||||
format="json",
|
||||
HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}",
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert len(data) == 4
|
||||
|
||||
# Check that results are sorted by score as expected
|
||||
compare = operator.le if direction == "asc" else operator.ge
|
||||
for i in range(len(data) - 1):
|
||||
assert compare(data[i]["_score"], data[i + 1]["_score"])
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_search_ordering_by_unknown_field(settings):
|
||||
"""Trying to sort by an unknown field should return a 400 error"""
|
||||
setup_oicd_resource_server(responses, settings, sub="user_sub")
|
||||
responses.add(
|
||||
responses.POST,
|
||||
settings.EMBEDDING_API_PATH,
|
||||
json=albert_embedding_response.response,
|
||||
status=200,
|
||||
)
|
||||
# Setup: Initialize the service and documents only once
|
||||
service = factories.ServiceFactory()
|
||||
documents = factories.DocumentSchemaFactory.build_batch(
|
||||
2, reach=random.choice(["public", "authenticated"])
|
||||
)
|
||||
prepare_index(service.index_name, documents)
|
||||
|
||||
# Define the parameters manually
|
||||
directions = ["asc", "desc"]
|
||||
|
||||
# Perform the parameterized tests
|
||||
for direction in directions:
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/search/",
|
||||
{
|
||||
"q": "*",
|
||||
"order_by": "unknown",
|
||||
"order_direction": direction,
|
||||
"visited": [doc["id"] for doc in documents],
|
||||
},
|
||||
format="json",
|
||||
HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}",
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert response.json() == [
|
||||
{
|
||||
"loc": ["order_by"],
|
||||
"msg": (
|
||||
"Input should be 'relevance', 'title', 'created_at', "
|
||||
"'updated_at', 'size' or 'reach'"
|
||||
),
|
||||
"type": "literal_error",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_search_ordering_by_unknown_direction(settings):
|
||||
"""Trying to sort with an unknown direction should return a 400 error"""
|
||||
setup_oicd_resource_server(responses, settings, sub="user_sub")
|
||||
responses.add(
|
||||
responses.POST,
|
||||
settings.EMBEDDING_API_PATH,
|
||||
json=albert_embedding_response.response,
|
||||
status=200,
|
||||
)
|
||||
service = factories.ServiceFactory()
|
||||
documents = factories.DocumentSchemaFactory.build_batch(
|
||||
2, reach=random.choice(["public", "authenticated"])
|
||||
)
|
||||
prepare_index(service.index_name, documents)
|
||||
|
||||
for field in enums.ORDER_BY_OPTIONS:
|
||||
response = APIClient().post(
|
||||
"/api/v1.0/documents/search/",
|
||||
{
|
||||
"q": "*",
|
||||
"order_by": field,
|
||||
"order_direction": "unknown",
|
||||
"visited": [doc["id"] for doc in documents],
|
||||
},
|
||||
format="json",
|
||||
HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}",
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert response.json() == [
|
||||
{
|
||||
"loc": ["order_direction"],
|
||||
"msg": "Input should be 'asc' or 'desc'",
|
||||
"type": "literal_error",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_search_filtering_by_reach(settings):
|
||||
"""It should be possible to filter results by their reach"""
|
||||
@@ -756,7 +580,7 @@ def test_api_documents_search_with_nb_results(settings):
|
||||
)
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert [r["_id"] for r in data] == ids[0:nb_results]
|
||||
assert {r["_id"] for r in data} == set(ids[0:nb_results])
|
||||
|
||||
nb_results = 10
|
||||
response = APIClient().post(
|
||||
@@ -772,7 +596,7 @@ def test_api_documents_search_with_nb_results(settings):
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
# nb_results > total number of documents => returns all documents
|
||||
assert [r["_id"] for r in data] == ids[0:9]
|
||||
assert {r["_id"] for r in data} == set(ids[0:9])
|
||||
|
||||
|
||||
@responses.activate
|
||||
@@ -846,7 +670,7 @@ def test_api_documents_search_nb_results_with_filtering(settings):
|
||||
HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert [r["_id"] for r in response.json()] == public_ids[0:nb_results]
|
||||
assert {r["_id"] for r in response.json()} == set(public_ids[0:nb_results])
|
||||
|
||||
|
||||
@responses.activate
|
||||
|
||||
@@ -3,7 +3,6 @@ Test suite for opensearch search service
|
||||
"""
|
||||
|
||||
import logging
|
||||
import operator
|
||||
from json import dumps as json_dumps
|
||||
from unittest.mock import patch
|
||||
|
||||
@@ -33,8 +32,6 @@ def search_params(service):
|
||||
"""Build opensearch.search() parameters for tests using the service index name"""
|
||||
return {
|
||||
"nb_results": 20,
|
||||
"order_by": "relevance",
|
||||
"order_direction": "desc",
|
||||
"search_indices": {service.index_name},
|
||||
"reach": None,
|
||||
"user_sub": "user_sub",
|
||||
@@ -408,41 +405,6 @@ def test_match_all(settings, caplog):
|
||||
assert len(result["hits"]["hits"]) == 3
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_search_ordering_by_relevance(settings, caplog):
|
||||
"""Test the hybrid supports ordering by relevance asc and desc"""
|
||||
enable_hybrid_search(settings)
|
||||
responses.add(
|
||||
responses.POST,
|
||||
settings.EMBEDDING_API_PATH,
|
||||
json=albert_embedding_response.response,
|
||||
status=200,
|
||||
)
|
||||
|
||||
documents = bulk_create_documents(
|
||||
[
|
||||
{"title": "wolf", "content": "wolves live in packs and hunt together"},
|
||||
{"title": "dog", "content": "dogs are loyal domestic animals"},
|
||||
{"title": "cat", "content": "cats are curious and independent pets"},
|
||||
]
|
||||
)
|
||||
q = "canine pet"
|
||||
service = factories.ServiceFactory(name=SERVICE_NAME)
|
||||
prepare_index(service.index_name, documents)
|
||||
|
||||
for direction in ["asc", "desc"]:
|
||||
with caplog.at_level(logging.INFO):
|
||||
result = search(
|
||||
q=q, **{**search_params(service), "order_direction": direction}
|
||||
)
|
||||
|
||||
# Check that results are sorted by score as expected
|
||||
hits = result["hits"]["hits"]
|
||||
compare = operator.le if direction == "asc" else operator.ge
|
||||
for i in range(len(hits) - 1):
|
||||
assert compare(hits[i]["_score"], hits[i + 1]["_score"])
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_hybrid_search_number_of_matches(settings):
|
||||
"""
|
||||
|
||||
@@ -332,12 +332,6 @@ class SearchDocumentView(ResourceServerMixin, views.APIView):
|
||||
path : str, optional
|
||||
Filter results based on the 'path' field. Only documents whose path
|
||||
starts with the provided value will be returned.
|
||||
order_by : str, optional
|
||||
Order results by 'relevance', 'created_at', 'updated_at', or 'size'.
|
||||
Defaults to 'relevance' if not specified.
|
||||
order_direction : str, optional
|
||||
Order direction, 'asc' for ascending or 'desc' for descending.
|
||||
Defaults to 'desc'.
|
||||
nb_results : int, optional
|
||||
The number of results to return.
|
||||
Defaults to 50 if not specified.
|
||||
@@ -383,8 +377,6 @@ class SearchDocumentView(ResourceServerMixin, views.APIView):
|
||||
result = search(
|
||||
q=params.q,
|
||||
nb_results=params.nb_results,
|
||||
order_by=params.order_by,
|
||||
order_direction=params.order_direction,
|
||||
search_indices=search_indices,
|
||||
reach=params.reach,
|
||||
visited=params.visited,
|
||||
|
||||
@@ -38,8 +38,6 @@ class Command(BaseCommand):
|
||||
index_name = "evaluation-index"
|
||||
search_params = {
|
||||
"nb_results": 20,
|
||||
"order_by": "relevance",
|
||||
"order_direction": "desc",
|
||||
"search_indices": {index_name},
|
||||
"reach": None,
|
||||
"user_sub": "user_sub",
|
||||
|
||||
Reference in New Issue
Block a user