fix: Continue summarizer on error and add more logging (#2220)

The fix will allow the summarizer to continue even if some content files fail to process. It will also add more logging output for successful and failed content files. In case of failed content files, the error log will also be printed.

Author: arslanashraf7

docker-compose.services.yml

@@ -101,6 +101,7 @@ services:
     environment:
       - KEYCLOAK_ADMIN=${KEYCLOAK_SVC_ADMIN:-admin}
       - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_SVC_ADMIN_PASSWORD:-admin}
+      - "_JAVA_OPTIONS=${JAVA_OPTIONS:-}" # Load _JAVA_OPTIONS from env, fallback to empty string
     networks:
       default:
         aliases:

learning_resources/content_summarizer.py

@@ -7,6 +7,10 @@
 from langchain_community.chat_models import ChatLiteLLM
 from typing_extensions import TypedDict
 
+from learning_resources.exceptions import (
+    FlashcardsGenerationError,
+    SummaryGenerationError,
+)
 from learning_resources.models import (
     ContentFile,
     ContentSummarizerConfiguration,
@@ -124,31 +128,36 @@ def get_unprocessed_content_file_ids(
 
     def summarize_content_files_by_ids(
         self, content_file_ids: list[int], overwrite
-    ) -> None:
+    ) -> list[str]:
         """Process multiple content files by id.
 
         Args:
             - ids (list[int]): List of content file ids to process
             - overwrite (bool): Whether to overwrite existing summary and flashcards
 
         Returns:
-            - None
+            - list[str]: List of status messages for each content file
         """
+        status_messages = []
         for content_file_id in content_file_ids:
-            self.summarize_single_content_file(content_file_id, overwrite=overwrite)
+            status_msg = self.summarize_single_content_file(
+                content_file_id, overwrite=overwrite
+            )
+            status_messages.append(status_msg)
+        return status_messages
 
     def summarize_single_content_file(
         self,
         content_file_id: int,
         overwrite,
-    ) -> None:
+    ) -> str:
         """Process a single content file
         Args:
             - content_file_id (int): Id of the content file to process
             - overwrite (bool): Whether to overwrite existing summary and flashcards
 
         Returns:
-            - None
+            - str: A string message indicating the status of the summarization
         """
         try:
             with transaction.atomic():
@@ -175,10 +184,24 @@ def summarize_single_content_file(
 
                     if updated:
                         content_file.save()
-
-        except Exception:
+                    return f"Summarization succeeded for CONTENT_FILE_ID: {content_file_id}"  # noqa: E501
+                return f"Summarization skipped for CONTENT_FILE_ID: {content_file_id}"
+        except SummaryGenerationError as exc:
+            # Log and return a specific readable error message when summary
+            # generation fails.
+            logger.exception("Error processing content: %d", content_file.id)
+            return f"Summary generation failed for CONTENT_FILE_ID: {content_file_id}\nError: {exc.args[0]}\n\n"  # noqa: E501
+        except FlashcardsGenerationError as exc:
+            # Log and return a specific readable error message when flashcards
+            # generation fails.
+            return f"Flashcards generation failed for CONTENT_FILE_ID: {content_file_id}\nError: {exc.args[0]}\n\n"  # noqa: E501
+        except Exception as exc:
+            # Log and return a specific readable error message when an  unknown
+            # error occurs.
             logger.exception("Error processing content: %d", content_file.id)
-            raise
+            return (
+                f"Summarization failed for CONTENT_FILE_ID: {content_file_id}\nError: {exc.args[0]}\n\n",  # noqa: E501
+            )
 
     def _get_llm(self, model=None, temperature=0.0, max_tokens=1000) -> ChatLiteLLM:
         """Get the ChatLiteLLM instance"""
@@ -216,11 +239,15 @@ def _generate_summary(self, content: str, llm_model: str) -> str:
             generated_summary = response.content
             logger.info("Generated summary: %s", generated_summary)
 
-        except Exception:
+        except Exception as exc:
+            # We do not want to raise the exception as is, we will log the exception and
+            # raise SummaryGenerationError that will be used to make further decisions
+            # in the code.
             logger.exception(
                 "An error occurred while generating summary using model: %s", llm_model
             )
-            raise
+            raise SummaryGenerationError(exc) from exc
+
         else:
             return generated_summary
 
@@ -243,12 +270,14 @@ def _generate_flashcards(
             )
             generated_flashcards = response.get("flashcards")
             logger.info("Generated flashcards: %s", generated_flashcards)
-
-        except Exception:
+        except Exception as exc:
+            # We do not want to raise the exception as is, we will log the exception and
+            # raise FlashcardsGenerationError that will be used to make further
+            # decisions in the code.
             logger.exception(
                 "An error occurred while generating flashcards using model: %s",
                 llm_model,
             )
-            raise
+            raise FlashcardsGenerationError(exc) from exc
         else:
             return generated_flashcards

learning_resources/content_summarizer_test.py

@@ -6,6 +6,10 @@
     PlatformType,
 )
 from learning_resources.content_summarizer import ContentSummarizer
+from learning_resources.exceptions import (
+    FlashcardsGenerationError,
+    SummaryGenerationError,
+)
 from learning_resources.factories import (
     ContentFileFactory,
     ContentSummarizerConfigurationFactory,
@@ -242,9 +246,9 @@ def test_get_unprocessed_content_files_with_platform_and_config(
 def test_summarize_content_files_by_ids(
     processable_content_files, mock_summarize_single_content_file
 ):
-    """The summarizer should process content files that are processable"""
+    """The summarizer should process content files that are processable and return the status results"""
     summarizer = ContentSummarizer()
-    summarizer.summarize_content_files_by_ids(
+    results = summarizer.summarize_content_files_by_ids(
         overwrite=False,
         content_file_ids=[
             content_file.id for content_file in processable_content_files
@@ -253,6 +257,8 @@ def test_summarize_content_files_by_ids(
     assert mock_summarize_single_content_file.call_count == len(
         processable_content_files
     )
+    assert isinstance(results, list)
+    assert len(results) == len(processable_content_files)
 
 
 def test_summarize_single_content_file(mocker, processable_content_files):
@@ -332,3 +338,77 @@ def test_process_single_file_calls_llm_summary(
         assert mock_instance.with_structured_output.call_count == 1
     elif has_flashcards:
         assert mock_instance.invoke.call_count == 1
+
+
+@pytest.mark.parametrize(
+    ("process_type", "expected_exception"),
+    [("summary", SummaryGenerationError), ("flashcards", FlashcardsGenerationError)],
+)
+def test_generate_summary_flashcards_exception(
+    mocker, processable_content_files, settings, process_type, expected_exception
+):
+    """Test the exception handling in the generate_summary and generate_flashcards methods"""
+    settings.OPENAI_API_KEY = "test"
+    summarizer = ContentSummarizer()
+    content_file = processable_content_files[0]
+    content_file.save()
+
+    # Mock the ChatLiteLLM class and its methods
+    mock_chat_llm = mocker.patch(
+        "learning_resources.content_summarizer.ChatLiteLLM", autospec=True
+    )
+    mock_instance = mock_chat_llm.return_value
+
+    # Mock the response for _generate_summary to raise an exception
+    mock_instance.invoke.side_effect = Exception("Test exception")
+    # Mock the response for _generate_flashcards to raise an exception
+    mock_instance.with_structured_output.return_value.invoke.side_effect = Exception(
+        "INVALID_FORMAT"
+    )
+
+    if process_type == "summary":
+        with pytest.raises(expected_exception):
+            summarizer._generate_summary(  # noqa: SLF001
+                llm_model="llm_model", content=content_file.content
+            )
+    else:
+        with pytest.raises(expected_exception):
+            summarizer._generate_flashcards(  # noqa: SLF001
+                llm_model="llm_model", content=content_file.content
+            )
+
+
+def test_summarize_single_content_file_with_exception(
+    mocker, processable_content_files, settings
+):
+    """Test the exception handling in the summarize_single_content_file method"""
+    settings.OPENAI_API_KEY = "test"
+    summarizer = ContentSummarizer()
+    content_file = processable_content_files[0]
+
+    # Mock the ChatLiteLLM class and its methods
+    mock_chat_llm = mocker.patch(
+        "learning_resources.content_summarizer.ChatLiteLLM", autospec=True
+    )
+    mock_instance = mock_chat_llm.return_value
+
+    # Mock the response for _generate_summary to raise an exception
+    mock_instance.invoke.side_effect = Exception("Test exception")
+    # Mock the response for _generate_flashcards to raise an exception
+    mock_instance.with_structured_output.return_value.invoke.side_effect = Exception(
+        "INVALID_FORMAT"
+    )
+
+    error = summarizer.summarize_single_content_file(content_file.id, overwrite=False)
+    assert (
+        error
+        == f"Summary generation failed for CONTENT_FILE_ID: {content_file.id}\nError: Test exception\n\n"
+    )
+    content_file.summary = "Test summary"
+    content_file.save()
+    content_file.refresh_from_db()
+    error = summarizer.summarize_single_content_file(content_file.id, overwrite=False)
+    assert (
+        error
+        == f"Flashcards generation failed for CONTENT_FILE_ID: {content_file.id}\nError: INVALID_FORMAT\n\n"
+    )

learning_resources/exceptions.py

@@ -13,3 +13,11 @@ class PostHogAuthenticationError(Exception):
 
 class PostHogQueryError(Exception):
     """Raised if the PostHog query API returns a non-authentication error."""
+
+
+class SummaryGenerationError(Exception):
+    """Raised if the summary generation fails for a content file."""
+
+
+class FlashcardsGenerationError(Exception):
+    """Raised if the flashcards generation fails for a content file."""

learning_resources/management/commands/generate_summary_flashcards.py

@@ -1,5 +1,7 @@
 """Management command to run the content summarizer"""
 
+import itertools
+
 from django.conf import settings
 from django.core.management import BaseCommand
 
@@ -91,9 +93,16 @@ def handle(self, *args, **options):  # noqa: ARG002
             self.stdout.write("Waiting on task...")
 
             start = now_in_utc()
-            summarizer_task.get()
+            results = summarizer_task.get()
+
+            # Log the summarization stats
+            flat_results = list(itertools.chain(*results))
+            for result in flat_results:
+                self.stdout.write(f"{result}")
 
             total_seconds = (now_in_utc() - start).total_seconds()
             self.stdout.write(
-                f"Content file summarizer finished, took {total_seconds} seconds"
+                self.style.SUCCESS(
+                    f"Content file summarizer finished, took {total_seconds} seconds"
+                )
             )

learning_resources/tasks.py

@@ -424,7 +424,7 @@ def summarize_content_files_task(
         - None
     """
     summarizer = ContentSummarizer()
-    summarizer.summarize_content_files_by_ids(content_file_ids, overwrite)
+    return summarizer.summarize_content_files_by_ids(content_file_ids, overwrite)
 
 
 @app.task(bind=True, acks_late=True)