diff --git a/frontends/main/src/app/(site)/search/page.test.tsx b/frontends/main/src/app/(site)/search/page.test.tsx new file mode 100644 index 0000000000..d99a03042a --- /dev/null +++ b/frontends/main/src/app/(site)/search/page.test.tsx @@ -0,0 +1,77 @@ +import { factories, makeRequest, setMockResponse, urls } from "api/test-utils" +import Page from "./page" + +jest.mock("@/app/getQueryClient", () => { + const { makeBrowserQueryClient } = jest.requireActual("@/app/getQueryClient") + return { getQueryClient: () => makeBrowserQueryClient({ maxRetries: 0 }) } +}) + +const SEARCH_RESPONSE = { + count: 0, + next: null, + previous: null, + results: [], + metadata: { + aggregations: {}, + suggestions: [], + }, +} + +beforeEach(() => { + setMockResponse.get( + urls.offerors.list(), + factories.learningResources.offerors({ count: 0 }), + ) + setMockResponse.get(expect.stringContaining(urls.search.resources()), { + ...SEARCH_RESPONSE, + }) + setMockResponse.get(expect.stringContaining(urls.search.vectorResources()), { + ...SEARCH_RESPONSE, + }) +}) + +test("prefetches OpenSearch results by default", async () => { + await Page({ + params: Promise.resolve({}), + searchParams: Promise.resolve({ q: "test" }), + }) + + expect( + makeRequest.mock.calls.some(([args]) => + args.url.startsWith(urls.search.resources()), + ), + ).toBe(true) + expect( + makeRequest.mock.calls.some(([args]) => + args.url.startsWith(urls.search.vectorResources()), + ), + ).toBe(false) +}) + +test("prefetches vector results when vector_search is enabled", async () => { + await Page({ + params: Promise.resolve({}), + searchParams: Promise.resolve({ + q: "test", + vector_search: "true", + topic: "Physics", + }), + }) + + const vectorCall = makeRequest.mock.calls.find(([args]) => + args.url.startsWith(urls.search.vectorResources()), + ) + expect(vectorCall).toBeDefined() + expect( + makeRequest.mock.calls.some(([args]) => + args.url.startsWith(urls.search.resources()), + ), + ).toBe(false) + + const searchParams = new URL(vectorCall?.[0].url ?? "").searchParams + expect(searchParams.get("hybrid_search")).toBe("true") + expect(searchParams.get("q")).toBe("test") + expect(searchParams.has("topic")).toBe(false) + expect(searchParams.has("limit")).toBe(false) + expect(searchParams.has("offset")).toBe(false) +}) diff --git a/frontends/main/src/app/(site)/search/page.tsx b/frontends/main/src/app/(site)/search/page.tsx index f222ddc07f..c2861c44c9 100644 --- a/frontends/main/src/app/(site)/search/page.tsx +++ b/frontends/main/src/app/(site)/search/page.tsx @@ -11,6 +11,10 @@ import { getExtraFacetNames, } from "@/app-pages/SearchPage/searchRequests" import getSearchParams from "@/page-components/SearchDisplay/getSearchParams" +import { + toUnfacetedVectorSearchParams, + toVectorSearchParams, +} from "@/page-components/SearchDisplay/vectorSearchParams" import validateRequestParams from "@/page-components/SearchDisplay/validateRequestParams" import type { ResourceSearchRequest } from "@/page-components/SearchDisplay/validateRequestParams" import { LearningResourcesSearchApiLearningResourcesSearchRetrieveRequest as LRSearchRequest } from "api" @@ -50,13 +54,28 @@ const Page: React.FC> = async ({ searchParams }) => { }) const queryClient = getQueryClient() + const isVectorSearch = urlParams.get("vector_search") === "true" + const hasSearchTerm = typeof params.q === "string" && params.q.trim() !== "" - await Promise.all([ - queryClient.prefetchQuery(offerorQueries.list({})), - queryClient.prefetchQuery( - learningResourceQueries.search(params as LRSearchRequest), - ), - ]) + if (isVectorSearch) { + await Promise.all([ + queryClient.prefetchQuery(offerorQueries.list({})), + queryClient.prefetchQuery( + learningResourceQueries.vectorSearch( + hasSearchTerm + ? toUnfacetedVectorSearchParams(params) + : toVectorSearchParams(params), + ), + ), + ]) + } else { + await Promise.all([ + queryClient.prefetchQuery(offerorQueries.list({})), + queryClient.prefetchQuery( + learningResourceQueries.search(params as LRSearchRequest), + ), + ]) + } return ( diff --git a/frontends/main/src/page-components/SearchDisplay/HybridSearchDisplay.tsx b/frontends/main/src/page-components/SearchDisplay/HybridSearchDisplay.tsx index 49857e5ab8..075f1b705e 100644 --- a/frontends/main/src/page-components/SearchDisplay/HybridSearchDisplay.tsx +++ b/frontends/main/src/page-components/SearchDisplay/HybridSearchDisplay.tsx @@ -1,107 +1,14 @@ import React, { useMemo } from "react" import { learningResourceQueries } from "api/hooks/learningResources" import type { LearningResource } from "api" -import type { Facets, BooleanFacets } from "@mitodl/course-search-utils" -import type { - LearningResourcesVectorSearchResponse, - VectorLearningResourcesSearchApiVectorLearningResourcesSearchRetrieveRequest as VectorSearchRequest, -} from "api/v0" +import type { LearningResourcesVectorSearchResponse } from "api/v0" import getSearchParams from "./getSearchParams" import SearchDisplay, { SearchDisplayProps } from "./SearchDisplay" - -const mapVectorSortby = ( - sortby?: string, -): VectorSearchRequest["sortby"] | undefined => { - switch (sortby) { - case "-views": - case "popular": - return "-views" - case "upcoming": - return "next_start_date" - case "new": - return "-created_on" - default: - return undefined - } -} - -/** - * Extracts only the fields supported by the vector search API from a broader - * search params object, dropping admin-only params (e.g., content_file_score_weight) - * that the vector endpoint does not accept. - * - * The `as` casts for enum arrays are safe because the v0 and v1 generated - * clients define separate (but structurally identical) enum types for the same - * string-literal values (e.g., delivery: 'online' | 'hybrid' | ...). - */ -const toVectorSearchParams = ( - params: ReturnType & { sortby?: string }, - cutoffScore?: number, -): VectorSearchRequest => ({ - aggregations: params.aggregations as VectorSearchRequest["aggregations"], - certification: params.certification, - certification_type: - params.certification_type as VectorSearchRequest["certification_type"], - course_feature: params.course_feature, - delivery: params.delivery as VectorSearchRequest["delivery"], - department: params.department as VectorSearchRequest["department"], - free: params.free, - level: params.level as VectorSearchRequest["level"], - limit: params.limit, - ocw_topic: params.ocw_topic, - offered_by: params.offered_by as VectorSearchRequest["offered_by"], - offset: params.offset, - platform: params.platform as VectorSearchRequest["platform"], - professional: params.professional, - q: params.q, - resource_category: - params.resource_category as VectorSearchRequest["resource_category"], - resource_type: params.resource_type as VectorSearchRequest["resource_type"], - resource_type_group: - params.resource_type_group as VectorSearchRequest["resource_type_group"], - score_cutoff: cutoffScore, - sortby: mapVectorSortby(params.sortby), - topic: params.topic, - hybrid_search: true, -}) - -const VECTOR_CLIENT_FILTER_FACETS = [ - "resource_type", - "certification_type", - "delivery", - "department", - "topic", - "offered_by", - "free", - "professional", - "resource_category", - "resource_type_group", - "level", - "platform", - "course_feature", -] as const - -type VectorClientFilterFacet = (typeof VECTOR_CLIENT_FILTER_FACETS)[number] - -const toUnfacetedVectorSearchParams = ( - params: ReturnType & { sortby?: string }, - constantSearchParams: Facets & BooleanFacets = {}, - cutoffScore?: number, -): VectorSearchRequest => { - const { - offset: _offset, - limit: _limit, - ...vectorParams - } = toVectorSearchParams(params, cutoffScore) - - return Object.fromEntries( - Object.entries(vectorParams).filter( - ([key]) => - !VECTOR_CLIENT_FILTER_FACETS.includes(key as VectorClientFilterFacet) || - key in constantSearchParams, - ), - ) as VectorSearchRequest -} +import { + VECTOR_CLIENT_FILTER_FACETS, + toUnfacetedVectorSearchParams, + toVectorSearchParams, +} from "./vectorSearchParams" const normalizeParamValues = (value: unknown): string[] => { if (Array.isArray(value)) { diff --git a/frontends/main/src/page-components/SearchDisplay/vectorSearchParams.ts b/frontends/main/src/page-components/SearchDisplay/vectorSearchParams.ts new file mode 100644 index 0000000000..edf587f283 --- /dev/null +++ b/frontends/main/src/page-components/SearchDisplay/vectorSearchParams.ts @@ -0,0 +1,97 @@ +import type { Facets, BooleanFacets } from "@mitodl/course-search-utils" +import type { VectorLearningResourcesSearchApiVectorLearningResourcesSearchRetrieveRequest as VectorSearchRequest } from "api/v0" +import getSearchParams from "./getSearchParams" + +const mapVectorSortby = ( + sortby?: string, +): VectorSearchRequest["sortby"] | undefined => { + switch (sortby) { + case "-views": + case "popular": + return "-views" + case "upcoming": + return "next_start_date" + case "new": + return "-created_on" + default: + return undefined + } +} + +/** + * Extracts only the fields supported by the vector search API from a broader + * search params object, dropping admin-only params (e.g., content_file_score_weight) + * that the vector endpoint does not accept. + * + * The `as` casts for enum arrays are safe because the v0 and v1 generated + * clients define separate (but structurally identical) enum types for the same + * string-literal values (e.g., delivery: 'online' | 'hybrid' | ...). + */ +export const toVectorSearchParams = ( + params: ReturnType & { sortby?: string }, + cutoffScore?: number, +): VectorSearchRequest => ({ + aggregations: params.aggregations as VectorSearchRequest["aggregations"], + certification: params.certification, + certification_type: + params.certification_type as VectorSearchRequest["certification_type"], + course_feature: params.course_feature, + delivery: params.delivery as VectorSearchRequest["delivery"], + department: params.department as VectorSearchRequest["department"], + free: params.free, + level: params.level as VectorSearchRequest["level"], + limit: params.limit, + ocw_topic: params.ocw_topic, + offered_by: params.offered_by as VectorSearchRequest["offered_by"], + offset: params.offset, + platform: params.platform as VectorSearchRequest["platform"], + professional: params.professional, + q: params.q, + resource_category: + params.resource_category as VectorSearchRequest["resource_category"], + resource_type: params.resource_type as VectorSearchRequest["resource_type"], + resource_type_group: + params.resource_type_group as VectorSearchRequest["resource_type_group"], + score_cutoff: cutoffScore, + sortby: mapVectorSortby(params.sortby), + topic: params.topic, + hybrid_search: true, +}) + +export const VECTOR_CLIENT_FILTER_FACETS = [ + "resource_type", + "certification_type", + "delivery", + "department", + "topic", + "offered_by", + "free", + "professional", + "resource_category", + "resource_type_group", + "level", + "platform", + "course_feature", +] as const + +type VectorClientFilterFacet = (typeof VECTOR_CLIENT_FILTER_FACETS)[number] + +export const toUnfacetedVectorSearchParams = ( + params: ReturnType & { sortby?: string }, + constantSearchParams: Facets & BooleanFacets = {}, + cutoffScore?: number, +): VectorSearchRequest => { + const { + offset: _offset, + limit: _limit, + ...vectorParams + } = toVectorSearchParams(params, cutoffScore) + + return Object.fromEntries( + Object.entries(vectorParams).filter( + ([key]) => + !VECTOR_CLIENT_FILTER_FACETS.includes(key as VectorClientFilterFacet) || + key in constantSearchParams, + ), + ) as VectorSearchRequest +} diff --git a/main/settings.py b/main/settings.py index 2d16eab3e6..d5756130fc 100644 --- a/main/settings.py +++ b/main/settings.py @@ -829,6 +829,13 @@ def get_all_config_keys(): # hard limit for special cases where we need to return all results without pagination VECTOR_SEARCH_PAGE_MAX_LIMIT = get_int("VECTOR_SEARCH_PAGE_MAX_LIMIT", 200) +# serve learning resource search hits from the Qdrant payload instead of +# re-hydrating them from the database. Set to False to fall back to database +# hydration without a deploy. +VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD = get_bool( + name="VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD", default=True +) + # toggle to use requests (default for local) or webdriver which renders js elements EMBEDDINGS_EXTERNAL_FETCH_USE_WEBDRIVER = get_bool( "EMBEDDINGS_EXTERNAL_FETCH_USE_WEBDRIVER", default=False diff --git a/vector_search/constants.py b/vector_search/constants.py index 85ef1f42dc..95d73b6888 100644 --- a/vector_search/constants.py +++ b/vector_search/constants.py @@ -149,6 +149,33 @@ CONTENT_FILES_RETRIEVE_PAYLOAD = True RESOURCES_RETRIEVE_PAYLOAD = ["readable_id", "platform"] +# Payload keys dropped when resource hits are served straight from the Qdrant +# payload (VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD): what the indexing serializer +# adds on top of the LearningResourceSerializer shape the API returns, plus +# video.transcript, which the response never renders. +# +# content_files is NOT excluded. Document and video responses declare it +# (NestedContentFileSerializer), and search cards fall back to +# content_files[0].image_src for the thumbnail when the resource has no image. +# The indexing serializer re-serializes it with the *full* ContentFileSerializer, +# so its large text fields are trimmed in Python instead -- see +# _trim_indexing_only_list_fields. +RESOURCES_PAYLOAD_EXCLUDE = [ + "_id", + "resource_relations", + "is_learning_material", + "resource_age_date", + "featured_rank", + "is_incomplete_or_stale", + "vector_embedding", + "video.transcript", +] + +# Qdrant payload selectors descend into objects but not into lists of objects, +# so the extra fields SearchCourseNumberSerializer puts on each course number +# cannot be named in RESOURCES_PAYLOAD_EXCLUDE and are trimmed in Python. +COURSE_NUMBER_INDEXING_ONLY_FIELDS = frozenset({"sort_coursenum", "primary"}) + COLLECTION_PARAM_MAP = { RESOURCES_COLLECTION_NAME: QDRANT_RESOURCE_PARAM_MAP, diff --git a/vector_search/utils.py b/vector_search/utils.py index accd72d1ca..ba3d574944 100644 --- a/vector_search/utils.py +++ b/vector_search/utils.py @@ -12,6 +12,7 @@ from qdrant_client import AsyncQdrantClient, QdrantClient, models from learning_resources.constants import ( + CONTENT_FILE_LARGE_FIELDS, PROGRAM_COURSE_CACHE_KEY_TEST_MODE, ) from learning_resources.models import ( @@ -42,6 +43,7 @@ from vector_search.constants import ( COLLECTION_PARAM_MAP, CONTENT_FILES_COLLECTION_NAME, + COURSE_NUMBER_INDEXING_ONLY_FIELDS, QDRANT_CONTENT_FILE_INDEXES, QDRANT_CONTENT_FILE_PARAM_MAP, QDRANT_LEARNING_RESOURCE_INDEXES, @@ -60,6 +62,8 @@ QDRANT_RESOURCE_PARAM_MAP, QDRANT_TOPIC_INDEXES, RESOURCES_COLLECTION_NAME, + RESOURCES_PAYLOAD_EXCLUDE, + RESOURCES_RETRIEVE_PAYLOAD, TOPICS_COLLECTION_NAME, VECTOR_SEARCH_SCORE_BOOST, ) @@ -1150,6 +1154,93 @@ def process_batch(docs_batch): ) +def resources_payload_selector(): + """ + Return the `with_payload` value to use for the resources collection. + + When hits are served from the payload we want everything the API response + needs, minus the indexing-only keys. Otherwise we only need the two fields + the database hydration path looks resources up by. + """ + if settings.VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD: + return models.PayloadSelectorExclude(exclude=RESOURCES_PAYLOAD_EXCLUDE) + return RESOURCES_RETRIEVE_PAYLOAD + + +def _without_keys(items, drop_keys): + """Drop drop_keys from every dict in a list, leaving non-dicts alone""" + return [ + {key: value for key, value in item.items() if key not in drop_keys} + if isinstance(item, dict) + else item + for item in items + ] + + +def _trim_indexing_only_list_fields(payload): + """ + Drop indexing-only keys the Qdrant payload selector cannot reach. + + Selectors descend into objects but not into lists of objects, so anything + the indexing serializer adds *inside* a list survives the exclude and has + to be removed here: + + - course.course_numbers[] carries sort_coursenum and primary, which + SearchCourseNumberSerializer adds on top of CourseNumberSerializer. + - content_files[] is re-serialized with the full ContentFileSerializer for + nested search, so it carries the large text fields that the API's + NestedContentFileSerializer omits. The rest of the field must survive: + document and video responses declare it, and search cards use + content_files[0].image_src as the thumbnail fallback. + """ + trimmed = payload + + course = payload.get("course") + if isinstance(course, dict) and isinstance(course.get("course_numbers"), list): + trimmed = { + **trimmed, + "course": { + **course, + "course_numbers": _without_keys( + course["course_numbers"], COURSE_NUMBER_INDEXING_ONLY_FIELDS + ), + }, + } + + content_files = payload.get("content_files") + if isinstance(content_files, list): + trimmed = { + **trimmed, + "content_files": _without_keys(content_files, CONTENT_FILE_LARGE_FIELDS), + } + + return trimmed + + +def _resource_payload_hits(search_result): + """ + Build resource hits from the Qdrant payloads themselves. + + The payload is the resource as the indexing serializer wrote it, so it + already carries every field the API response needs -- no database + hydration required. Dedupes on platform:readable_id and preserves the + Qdrant ranking, the same way the hydrated path does. + """ + hits = [] + seen = set() + for hit in search_result: + payload = hit.payload or {} + readable_id = payload.get("readable_id") + if not readable_id: + continue + key = f"{(payload.get('platform') or {}).get('code', '')}:{readable_id}" + if key in seen: + continue + seen.add(key) + hits.append(_trim_indexing_only_list_fields(payload)) + return hits + + def _resource_vector_hits(search_result): readable_ids = [ hit.payload.get("readable_id") diff --git a/vector_search/utils_test.py b/vector_search/utils_test.py index faf88c7796..e0b10faded 100644 --- a/vector_search/utils_test.py +++ b/vector_search/utils_test.py @@ -15,7 +15,9 @@ import vector_search.utils as vs_utils from learning_resources.constants import ( + CONTENT_FILE_LARGE_FIELDS, GROUP_CONTENT_FILE_CONTENT_VIEWERS, + LearningResourceType, ) from learning_resources.factories import ( ContentFileFactory, @@ -25,7 +27,7 @@ LearningResourceRunFactory, LearningResourceTopicFactory, ) -from learning_resources.models import LearningResource +from learning_resources.models import ContentFile, LearningResource from learning_resources.serializers import LearningResourceMetadataDisplaySerializer from learning_resources_search.constants import ( CONTENT_FILE_TYPE, @@ -55,6 +57,8 @@ QDRANT_OPTIMIZER_THRESHOLD_SMALL, QDRANT_RESOURCE_PARAM_MAP, RESOURCES_COLLECTION_NAME, + RESOURCES_PAYLOAD_EXCLUDE, + RESOURCES_RETRIEVE_PAYLOAD, ) from vector_search.encoders.utils import dense_encoder, sparse_encoder from vector_search.utils import ( @@ -65,6 +69,7 @@ _generate_content_file_points, _get_text_splitter, _is_markdown_content, + _resource_payload_hits, _resource_vector_hits, _set_payload, async_qdrant_aggregations, @@ -77,6 +82,7 @@ filter_existing_qdrant_points, qdrant_query_conditions, remove_qdrant_records, + resources_payload_selector, should_generate_content_embeddings, should_generate_resource_embeddings, update_content_file_payload, @@ -2343,6 +2349,259 @@ def test_resource_vector_hits_duplicate_readable_ids_different_platforms(): assert result_2[1]["platform"]["code"] == "xpro" +def test_resources_payload_selector_excludes_indexing_fields(settings): + """The selector should ask for the whole payload minus indexing-only keys""" + settings.VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD = True + selector = resources_payload_selector() + assert isinstance(selector, models.PayloadSelectorExclude) + assert selector.exclude == RESOURCES_PAYLOAD_EXCLUDE + + +def test_resources_payload_selector_kill_switch(settings): + """With payload hits disabled we only fetch the DB hydration lookup fields""" + settings.VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD = False + assert resources_payload_selector() == RESOURCES_RETRIEVE_PAYLOAD + + +def test_resource_payload_hits_preserves_order_and_dedupes(): + """Hits come straight from the payloads, in Qdrant order, deduped by platform:id""" + search_result = [ + MagicMock( + payload={ + "readable_id": "course-2", + "platform": {"code": "ocw"}, + "title": "Second", + } + ), + MagicMock( + payload={ + "readable_id": "course-1", + "platform": {"code": "ocw"}, + "title": "First", + } + ), + # same readable_id as the first hit, different platform: kept + MagicMock( + payload={ + "readable_id": "course-2", + "platform": {"code": "xpro"}, + "title": "Second on xpro", + } + ), + # exact duplicate of the first hit: dropped + MagicMock( + payload={ + "readable_id": "course-2", + "platform": {"code": "ocw"}, + "title": "Second", + } + ), + # unusable without a readable_id: dropped + MagicMock(payload={"platform": {"code": "ocw"}, "title": "No readable id"}), + ] + + hits = _resource_payload_hits(search_result) + + assert [(hit["readable_id"], hit["platform"]["code"]) for hit in hits] == [ + ("course-2", "ocw"), + ("course-1", "ocw"), + ("course-2", "xpro"), + ] + assert hits[0]["title"] == "Second" + + +def test_resource_payload_hits_handles_null_platform(): + """A resource indexed without a platform should still produce a hit""" + hits = _resource_payload_hits( + [MagicMock(payload={"readable_id": "course-1", "platform": None})] + ) + assert [hit["readable_id"] for hit in hits] == ["course-1"] + + +def test_resource_payload_hits_trims_indexing_only_course_number_fields(): + """ + Qdrant payload selectors cannot descend into lists of objects, so the extra + course number fields the indexing serializer adds are trimmed in Python. + """ + payload = { + "readable_id": "course-1", + "platform": {"code": "ocw"}, + "course": { + "course_numbers": [ + { + "value": "6.006", + "listing_type": "Primary", + "department": {"department_id": "6"}, + "primary": True, + "sort_coursenum": "06.006", + } + ] + }, + } + + hits = _resource_payload_hits([MagicMock(payload=payload)]) + + assert hits[0]["course"]["course_numbers"] == [ + { + "value": "6.006", + "listing_type": "Primary", + "department": {"department_id": "6"}, + } + ] + # the payload dict Qdrant handed us is not mutated + assert "sort_coursenum" in payload["course"]["course_numbers"][0] + + +@pytest.mark.parametrize( + "course", + [None, {}, {"course_numbers": None}], +) +def test_resource_payload_hits_tolerates_missing_course_numbers(course): + """Non-course resources pass through the course number trim untouched""" + hits = _resource_payload_hits( + [MagicMock(payload={"readable_id": "video-1", "course": course})] + ) + assert hits[0]["course"] == course + + +def _add_direct_content_files(resource, count=2, **kwargs): + """ + Attach the direct content files that video/document responses nest. + + ContentFileFactory._create always fills in run or learning_resource, but the + model's check constraint requires a direct content file to have neither, so + the foreign key is moved after creation. + """ + content_files = ContentFileFactory.create_batch( + count, learning_resource=resource, **kwargs + ) + ContentFile.objects.filter(id__in=[cf.id for cf in content_files]).update( + learning_resource=None, direct_learning_resource=resource + ) + return content_files + + +def _payload_as_search_sees_it(resource_id): + """ + Return the indexed payload minus what PayloadSelectorExclude strips, + i.e. exactly what _resource_payload_hits receives from a search. + """ + payload = next(iter(serialize_bulk_learning_resources([resource_id]))) + for excluded in RESOURCES_PAYLOAD_EXCLUDE: + top_level, _, nested = excluded.partition(".") + if nested: + if isinstance(payload.get(top_level), dict): + payload[top_level].pop(nested, None) + else: + payload.pop(top_level, None) + return payload + + +def test_content_files_is_not_excluded_from_the_payload(): + """ + content_files must stay in the payload: document and video responses declare + it, and search cards fall back to content_files[0].image_src for the + thumbnail. Its large text fields are trimmed in Python instead, because a + Qdrant payload selector cannot descend into a list of objects. + """ + assert "content_files" not in RESOURCES_PAYLOAD_EXCLUDE + + +@pytest.mark.parametrize( + ("factory_kwargs", "has_content_files"), + [ + ({"is_course": True}, False), + ({"is_video": True}, True), + ({"resource_type": LearningResourceType.document.name}, True), + ], +) +def test_resource_payload_hits_matches_hydrated_hits(factory_kwargs, has_content_files): + """ + The payload path should return what the database hydration path returns, + modulo the fields the indexing serializer adds on top of the API shape -- + including the nested content_files that document and video responses + declare. + """ + resource = LearningResourceFactory.create(**factory_kwargs) + if has_content_files: + _add_direct_content_files( + resource, image_src="https://img.youtube.com/thumb.jpg" + ) + + payload = _payload_as_search_sees_it(resource.id) + hydrated = _resource_vector_hits( + [ + MagicMock( + payload={ + "readable_id": resource.readable_id, + "platform": { + "code": resource.platform.code if resource.platform else "" + }, + } + ) + ] + ) + from_payload = _resource_payload_hits([MagicMock(payload=payload)]) + + assert len(from_payload) == 1 + assert set(from_payload[0]) == set(hydrated[0]) + + if has_content_files: + # the nested field must carry the API's shape, not the indexing shape + assert from_payload[0]["content_files"] + assert {frozenset(cf) for cf in from_payload[0]["content_files"]} == { + frozenset(cf) for cf in hydrated[0]["content_files"] + } + + +@pytest.mark.parametrize( + "resource_type", + [LearningResourceType.video.name, LearningResourceType.document.name], +) +def test_resource_payload_hits_keeps_content_files_thumbnail_fallback(resource_type): + """ + Search cards use content_files[0].image_src as the thumbnail when the + resource has no image, so the payload path must keep the nested content + files -- minus the large text the indexing serializer re-adds. + """ + payload = { + "readable_id": f"{resource_type}-1", + "platform": {"code": "youtube"}, + "resource_type": resource_type, + "image": None, + "content_files": [ + { + "id": 1, + "key": "lecture.pdf", + "title": "Lecture", + "image_src": "https://img.youtube.com/thumb.jpg", + "content": "the full extracted text, many kilobytes of it", + "summary": "a generated summary", + "flashcards": [{"question": "q", "answer": "a"}], + } + ], + } + + hits = _resource_payload_hits([MagicMock(payload=payload)]) + content_file = hits[0]["content_files"][0] + + assert content_file["image_src"] == "https://img.youtube.com/thumb.jpg" + assert content_file["key"] == "lecture.pdf" + assert content_file["title"] == "Lecture" + assert set(CONTENT_FILE_LARGE_FIELDS).isdisjoint(content_file) + # the payload dict Qdrant handed us is not mutated + assert "content" in payload["content_files"][0] + + +@pytest.mark.parametrize("content_files", [None, [], "not-a-list"]) +def test_resource_payload_hits_tolerates_odd_content_files(content_files): + """Resources without nested content files pass through untouched""" + hits = _resource_payload_hits( + [MagicMock(payload={"readable_id": "c-1", "content_files": content_files})] + ) + assert hits[0]["content_files"] == content_files + + def _make_facet_hit(count=0, value="test"): """Build a minimal mock that looks like a Qdrant FacetHit.""" hit = MagicMock() diff --git a/vector_search/views.py b/vector_search/views.py index 92d140bffd..bb8c747356 100644 --- a/vector_search/views.py +++ b/vector_search/views.py @@ -23,7 +23,6 @@ CONTENT_FILES_RETRIEVE_PAYLOAD, QDRANT_RESOURCE_PARAM_MAP, RESOURCES_COLLECTION_NAME, - RESOURCES_RETRIEVE_PAYLOAD, ) from vector_search.serializers import ( ContentFileVectorSearchRequestSerializer, @@ -34,6 +33,7 @@ from vector_search.utils import ( _content_file_vector_hits, _merge_dicts, + _resource_payload_hits, _resource_vector_hits, async_qdrant_aggregations, async_qdrant_client, @@ -43,6 +43,7 @@ db_sync_to_async, dense_encoder, qdrant_query_conditions, + resources_payload_selector, sparse_encoder, ) @@ -150,7 +151,7 @@ async def _build_search_params( # noqa: PLR0913 "collection_name": search_collection, "query_filter": search_filter, "with_vectors": False, - "with_payload": RESOURCES_RETRIEVE_PAYLOAD + "with_payload": resources_payload_selector() if search_collection == RESOURCES_COLLECTION_NAME else CONTENT_FILES_RETRIEVE_PAYLOAD, "search_params": models.SearchParams( @@ -276,6 +277,11 @@ async def _execute_scroll_search( # noqa: PLR0913 "collection_name": search_collection, "scroll_filter": search_filter, "with_vectors": False, + # Scroll otherwise defaults to the entire payload, transcripts and + # all -- ask for the same fields the query path does. + "with_payload": resources_payload_selector() + if search_collection == RESOURCES_COLLECTION_NAME + else CONTENT_FILES_RETRIEVE_PAYLOAD, } if order_by: @@ -389,6 +395,10 @@ async def _async_vector_hits( # noqa: PLR0913 ) if search_collection == RESOURCES_COLLECTION_NAME: + if settings.VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD: + # Payloads are already the serialized resources -- no database + # round trip, so no thread hop either. + return _resource_payload_hits(search_result) return await db_sync_to_async(_resource_vector_hits)(search_result) else: return await db_sync_to_async(_content_file_vector_hits)(search_result) diff --git a/vector_search/views_test.py b/vector_search/views_test.py index 1d1ec2e239..50038d1784 100644 --- a/vector_search/views_test.py +++ b/vector_search/views_test.py @@ -14,6 +14,12 @@ LearningResourceFactory, LearningResourceRunFactory, ) +from learning_resources_search.serializers import serialize_bulk_learning_resources +from vector_search.constants import ( + CONTENT_FILES_RETRIEVE_PAYLOAD, + RESOURCES_PAYLOAD_EXCLUDE, + RESOURCES_RETRIEVE_PAYLOAD, +) from vector_search.encoders.utils import dense_encoder, sparse_encoder from vector_search.views import QdrantView @@ -749,11 +755,11 @@ def test_vector_search_sortby_with_score_cutoff_manually_sorted(mocker, client): mock_result = mocker.MagicMock() mock_point_1 = mocker.MagicMock() - mock_point_1.payload = {"readable_id": "course-1"} + mock_point_1.payload = {"readable_id": "course-1", "views": 100} mock_point_2 = mocker.MagicMock() - mock_point_2.payload = {"readable_id": "course-2"} + mock_point_2.payload = {"readable_id": "course-2", "views": 50} mock_point_3 = mocker.MagicMock() - mock_point_3.payload = {"readable_id": "course-3"} + mock_point_3.payload = {"readable_id": "course-3", "views": 200} mock_result.points = [mock_point_1, mock_point_2, mock_point_3] mock_qdrant.query_points = mocker.AsyncMock(return_value=mock_result) @@ -763,16 +769,6 @@ def test_vector_search_sortby_with_score_cutoff_manually_sorted(mocker, client): return_value=mock_qdrant, ) - mock_hits = [ - {"readable_id": "course-1", "views": 100}, - {"readable_id": "course-2", "views": 50}, - {"readable_id": "course-3", "views": 200}, - ] - mocker.patch( - "vector_search.views._resource_vector_hits", - return_value=mock_hits, - ) - # Test descending sort: sortby=-views params = { "hybrid_search": "true", @@ -1256,3 +1252,148 @@ def test_content_file_vector_search_count_is_approximate( mock_qdrant.count.assert_awaited() assert mock_qdrant.count.await_args.kwargs["exact"] is False + + +@pytest.mark.parametrize("from_payload", [True, False]) +def test_vector_search_payload_selector(mocker, client, settings, from_payload): + """ + Resource searches request the trimmed full payload when payload hits are + enabled, and only the two hydration lookup fields when they are not. + """ + settings.VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD = from_payload + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + empty = mocker.MagicMock() + empty.points = [] + mock_qdrant.query_points = mocker.AsyncMock(return_value=empty) + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.count = mocker.AsyncMock(return_value=CountResult(count=0)) + mocker.patch("vector_search.views.async_qdrant_client", return_value=mock_qdrant) + + client.get( + reverse("vector_search:v0:vector_learning_resources_search"), + data={"q": "test"}, + ) + + with_payload = mock_qdrant.query_points.mock_calls[0].kwargs["with_payload"] + if from_payload: + assert with_payload == models.PayloadSelectorExclude( + exclude=RESOURCES_PAYLOAD_EXCLUDE + ) + else: + assert with_payload == RESOURCES_RETRIEVE_PAYLOAD + + +@pytest.mark.parametrize("from_payload", [True, False]) +def test_vector_search_scroll_payload_selector(mocker, client, settings, from_payload): + """ + The scroll path (no query string) must use the same selector; it otherwise + defaults to the entire payload, transcripts included. + """ + settings.VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD = from_payload + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.count = mocker.AsyncMock(return_value=CountResult(count=0)) + mocker.patch("vector_search.views.async_qdrant_client", return_value=mock_qdrant) + + client.get( + reverse("vector_search:v0:vector_learning_resources_search"), + data={"q": ""}, + ) + + with_payload = mock_qdrant.scroll.mock_calls[0].kwargs["with_payload"] + if from_payload: + assert with_payload == models.PayloadSelectorExclude( + exclude=RESOURCES_PAYLOAD_EXCLUDE + ) + else: + assert with_payload == RESOURCES_RETRIEVE_PAYLOAD + + +@pytest.mark.django_db(transaction=True) +def test_content_file_vector_search_scroll_keeps_full_payload( + mocker, client, content_file_viewer +): + """Content file search is unaffected: it still scrolls the whole payload""" + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.count = mocker.AsyncMock(return_value=CountResult(count=0)) + mocker.patch("vector_search.views.async_qdrant_client", return_value=mock_qdrant) + + client.get(reverse("vector_search:v0:vector_content_files_search"), data={"q": ""}) + + assert ( + mock_qdrant.scroll.mock_calls[0].kwargs["with_payload"] + == CONTENT_FILES_RETRIEVE_PAYLOAD + ) + + +@pytest.mark.django_db(transaction=True) +def test_vector_search_returns_payload_is_not_hydrated(mocker, client): + """ + With payload hits enabled the response is built from the Qdrant payload + rather than re-fetched from the database. + """ + resource = LearningResourceFactory.create(is_course=True) + payload = next(iter(serialize_bulk_learning_resources([resource.id]))) + + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + mock_result = mocker.MagicMock() + point = mocker.MagicMock() + point.payload = payload + mock_result.points = [point] + mock_qdrant.query_points = mocker.AsyncMock(return_value=mock_result) + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.count = mocker.AsyncMock(return_value=CountResult(count=1)) + mocker.patch("vector_search.views.async_qdrant_client", return_value=mock_qdrant) + hydrate = mocker.patch("vector_search.views._resource_vector_hits") + + response = client.get( + reverse("vector_search:v0:vector_learning_resources_search"), + data={"q": "test"}, + ) + + assert response.status_code == 200 + results = response.json()["results"] + assert [result["readable_id"] for result in results] == [resource.readable_id] + assert results[0]["title"] == resource.title + hydrate.assert_not_called() + + +@pytest.mark.django_db(transaction=True) +def test_vector_search_kill_switch_hydrates_from_database(mocker, client, settings): + """Turning the setting off restores database hydration""" + settings.VECTOR_SEARCH_RESOURCES_FROM_PAYLOAD = False + resource = LearningResourceFactory.create(is_course=True) + + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + mock_result = mocker.MagicMock() + point = mocker.MagicMock() + point.payload = { + "readable_id": resource.readable_id, + "platform": {"code": resource.platform.code}, + } + mock_result.points = [point] + mock_qdrant.query_points = mocker.AsyncMock(return_value=mock_result) + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.count = mocker.AsyncMock(return_value=CountResult(count=1)) + mocker.patch("vector_search.views.async_qdrant_client", return_value=mock_qdrant) + payload_hits = mocker.patch("vector_search.views._resource_payload_hits") + + response = client.get( + reverse("vector_search:v0:vector_learning_resources_search"), + data={"q": "test"}, + ) + + assert response.status_code == 200 + assert [result["id"] for result in response.json()["results"]] == [resource.id] + payload_hits.assert_not_called()