Regenerate course metadata when course information updates (#2419)

* ensuring that changed metadata docs get re-embedded

* adding test

* remove existing metadata documents

* fixing bug with missing key sent in params

* adding types

* adding type

Author: shanbady

learning_resources/models.py

@@ -3,7 +3,6 @@
 import uuid
 from abc import abstractmethod
 from functools import cached_property
-from hashlib import md5
 from typing import TYPE_CHECKING, Optional
 
 from django.conf import settings
@@ -25,6 +24,7 @@
     PrivacyLevel,
 )
 from main.models import TimestampedModel, TimestampedModelQuerySet
+from main.utils import checksum_for_content
 
 if TYPE_CHECKING:
     from django.contrib.auth import get_user_model
@@ -942,15 +942,8 @@ class ContentFile(TimestampedModel):
     summary = models.TextField(blank=True, default="")
     flashcards = models.JSONField(blank=True, default=list)
 
-    def content_checksum(self):
-        hasher = md5()  # noqa: S324
-        if self.content:
-            hasher.update(self.content.encode("utf-8"))
-            return hasher.hexdigest()
-        return None
-
     def save(self, **kwargs):
-        self.checksum = self.content_checksum()
+        self.checksum = checksum_for_content(self.content)
         super().save(**kwargs)
 
     class Meta:

main/utils.py

@@ -5,6 +5,7 @@
 import os
 from enum import Flag, auto
 from functools import wraps
+from hashlib import md5
 from itertools import islice
 from urllib.parse import urljoin
 
@@ -364,3 +365,14 @@ def clear_search_cache():
         search_keys = cache.keys("views.decorators.cache.cache_header.search.*")
         cleared += cache.delete_many(search_keys) or 0
     return cleared
+
+
+def checksum_for_content(content: str) -> str:
+    """
+    Generate a checksum based on the provided content string
+    """
+    hasher = md5()  # noqa: S324
+    if content:
+        hasher.update(content.encode("utf-8"))
+        return hasher.hexdigest()
+    return None

vector_search/utils.py

@@ -1,5 +1,6 @@
 import logging
 import uuid
+from typing import Optional
 
 from django.conf import settings
 from langchain.text_splitter import RecursiveCharacterTextSplitter
@@ -21,6 +22,7 @@
     serialize_bulk_content_files,
     serialize_bulk_learning_resources,
 )
+from main.utils import checksum_for_content
 from vector_search.constants import (
     CONTENT_FILES_COLLECTION_NAME,
     QDRANT_CONTENT_FILE_INDEXES,
@@ -253,11 +255,13 @@ def update_learning_resource_payload(serialized_document):
 
 
 def update_content_file_payload(serialized_document):
-    params = {
-        "resource_readable_id": serialized_document["resource_readable_id"],
-        "key": serialized_document["key"],
-        "run_readable_id": serialized_document["run_readable_id"],
-    }
+    search_keys = ["resource_readable_id", "key", "run_readable_id"]
+    params = {}
+    for key in search_keys:
+        if key in serialized_document:
+            params[key] = serialized_document[key]
+    if not params:
+        return
     points = [
         point.id
         for point in retrieve_points_matching_params(
@@ -319,18 +323,20 @@ def should_generate_resource_embeddings(serialized_document):
     return True
 
 
-def should_generate_content_embeddings(serialized_document):
+def should_generate_content_embeddings(
+    serialized_document: dict, point_id: Optional[str] = None
+) -> bool:
     """
     Determine if we should generate embeddings for a content file
     """
     client = qdrant_client()
-
-    # we just need metadata from the first chunk
-    point_id = vector_point_id(
-        f"{serialized_document['resource_readable_id']}."
-        f"{serialized_document.get('run_readable_id', '')}."
-        f"{serialized_document['key']}.0"
-    )
+    if not point_id:
+        # we just need metadata from the first chunk
+        point_id = vector_point_id(
+            f"{serialized_document['resource_readable_id']}."
+            f"{serialized_document.get('run_readable_id', '')}."
+            f"{serialized_document['key']}.0"
+        )
     response = client.retrieve(
         collection_name=CONTENT_FILES_COLLECTION_NAME,
         ids=[point_id],
@@ -353,19 +359,37 @@ def _embed_course_metadata_as_contentfile(serialized_resources):
     ids = []
     docs = []
     for doc in serialized_resources:
-        if not should_generate_resource_embeddings(doc):
-            continue
         readable_id = doc["readable_id"]
         resource_vector_point_id = str(vector_point_id(readable_id))
         serializer = LearningResourceMetadataDisplaySerializer(doc)
+        serialized_document = serializer.render_document()
+        checksum = checksum_for_content(str(serialized_document))
+        key = f"{doc['readable_id']}.course_metadata"
+        serialized_document["checksum"] = checksum
+        serialized_document["key"] = key
+        document_point_id = vector_point_id(
+            f"{doc['readable_id']}.course_information.0"
+        )
+        if not should_generate_content_embeddings(
+            serialized_document, document_point_id
+        ):
+            continue
+        # remove existing course info docs
+        remove_points_matching_params(
+            {"key": key}, collection_name=CONTENT_FILES_COLLECTION_NAME
+        )
         split_texts = serializer.render_chunks()
         split_metadatas = [
             {
                 "resource_point_id": str(resource_vector_point_id),
                 "chunk_number": chunk_id,
                 "chunk_content": chunk_content,
                 "resource_readable_id": doc["readable_id"],
+                "run_readable_id": doc["readable_id"],
                 "file_extension": ".txt",
+                "file_type": "course_metadata",
+                "key": key,
+                "checksum": checksum,
                 **{key: doc[key] for key in ["offered_by", "platform"]},
             }
             for chunk_id, chunk_content in enumerate(split_texts)

vector_search/utils_test.py

@@ -12,10 +12,12 @@
     LearningResourceRunFactory,
 )
 from learning_resources.models import LearningResource
+from learning_resources.serializers import LearningResourceMetadataDisplaySerializer
 from learning_resources_search.serializers import (
     serialize_bulk_content_files,
     serialize_bulk_learning_resources,
 )
+from main.utils import checksum_for_content
 from vector_search.constants import (
     CONTENT_FILES_COLLECTION_NAME,
     QDRANT_CONTENT_FILE_PARAM_MAP,
@@ -635,3 +637,89 @@ def test_embed_learning_resources_summarizes_only_contentfiles_with_summary(mock
     # Only contentfiles with summary should be passed
     expected_ids = [cf.id for cf in contentfiles_with_summary]
     summarize_mock.assert_called_once_with(expected_ids, True)  # noqa: FBT003
+
+
+@pytest.mark.django_db
+def test_embed_course_metadata_as_contentfile_uploads_points_on_change(mocker):
+    """
+    Test that _embed_course_metadata_as_contentfile uploads points to Qdrant
+    if any property of a serialized_resource has changed
+    """
+
+    mock_client = mocker.patch("vector_search.utils.qdrant_client").return_value
+    mock_encoder = mocker.patch("vector_search.utils.dense_encoder").return_value
+    mock_encoder.model_short_name.return_value = "test-model"
+    mock_encoder.embed_documents.return_value = [[0.1, 0.2, 0.3]]
+    resource = LearningResourceFactory.create()
+    serialized_resource = next(serialize_bulk_learning_resources([resource.id]))
+    serializer = LearningResourceMetadataDisplaySerializer(serialized_resource)
+    rendered_document = serializer.render_document()
+    resource_checksum = checksum_for_content(str(rendered_document))
+
+    """
+    Simulate qdrant returning a checksum for existing
+    record that matches the checksum of metadata doc
+    """
+    mock_point = mocker.Mock()
+    mock_point.payload = {"checksum": "checksum2"}
+    mock_client.retrieve.return_value = [mock_point]
+
+    _embed_course_metadata_as_contentfile([serialized_resource])
+
+    # Assert upload_points was called
+    assert mock_client.upload_points.called
+    args, kwargs = mock_client.upload_points.call_args
+    assert args[0] == CONTENT_FILES_COLLECTION_NAME
+    points = list(kwargs["points"])
+    assert len(points) == 1
+    assert points[0].payload["resource_readable_id"] == resource.readable_id
+    assert points[0].payload["checksum"] == resource_checksum
+
+    # simulate qdrant returning the same checksum for the metadata doc
+    mock_point.payload = {"checksum": resource_checksum}
+    mock_client.upload_points.reset_mock()
+    _embed_course_metadata_as_contentfile([serialized_resource])
+
+    # nothing has changed - no updates to make
+    assert not mock_client.upload_points.called
+
+
+@pytest.mark.parametrize(
+    ("serialized_document", "expected_params"),
+    [
+        (
+            {"resource_readable_id": "r1", "key": "k1", "run_readable_id": "run1"},
+            {"resource_readable_id": "r1", "key": "k1", "run_readable_id": "run1"},
+        ),
+        (
+            {"resource_readable_id": "r2", "key": "k2"},
+            {"resource_readable_id": "r2", "key": "k2"},
+        ),
+        (
+            {"run_readable_id": "run3"},
+            {"run_readable_id": "run3"},
+        ),
+        ({"test": "run3"}, None),
+    ],
+)
+def test_update_content_file_payload_only_includes_existing_keys(
+    mocker, serialized_document, expected_params
+):
+    """
+    Test that params only includes keys
+    that are defined in the input document
+    """
+    mock_retrieve = mocker.patch(
+        "vector_search.utils.retrieve_points_matching_params", return_value=[]
+    )
+    mocker.patch("vector_search.utils._set_payload")
+
+    update_content_file_payload(serialized_document)
+    if expected_params:
+        # Check that retrieve_points_matching_params was called with only the expected keys
+        mock_retrieve.assert_called_once_with(
+            expected_params,
+            collection_name=CONTENT_FILES_COLLECTION_NAME,
+        )
+    else:
+        mock_retrieve.assert_not_called()