fix: Enhance chat response handling for v1 and v2 endpoints (#199)

* fix: Enhance chat response handling for v1 and v2 endpoints

* fix: Improve handling of chat message formats for v1 and v2 endpoints

* fix: Add extract_message_content utility for handling chat API responses

* fix: Addressing the lint issues

Author: mraman-2U

CHANGELOG.rst

@@ -13,6 +13,10 @@ Change Log
 
 Unreleased
 **********
+4.10.8 - 2025-07-31
+*******************
+* Chat history to support XPert Chat API V2 response with a list of messages.
+
 4.10.7 - 2025-07-15
 *******************
 * XPert Chat API response will be message object for V1 endpoint and a list of messages for V2 endpoint.

learning_assistant/__init__.py

@@ -2,6 +2,6 @@
 Plugin for a learning assistant backend, intended for use within edx-platform.
 """
 
-__version__ = '4.10.7'
+__version__ = '4.10.8'
 
 default_app_config = 'learning_assistant.apps.LearningAssistantConfig'  # pylint: disable=invalid-name

learning_assistant/utils.py

@@ -208,3 +208,32 @@ def parse_lms_datetime(datetime_string):
         return None
 
     return parsed_datetime
+
+
+def extract_message_content(message):
+    """
+    Extract content from a message response handling both v1 and v2 endpoint formats.
+
+    Args:
+        message: The message response from the chat API. Can be:
+            - v2 format: List of message objects
+            - v1 format: Single message dict with 'content' key
+            - Error format: Plain string or other format
+
+    Returns:
+        str: The extracted message content, or empty string for empty lists
+    """
+    if v2_endpoint_enabled() and isinstance(message, list):
+        # For v2 endpoint, message is an array - get the last message content
+        if len(message) > 0 and isinstance(message[-1], dict):
+            return message[-1].get('content', '')
+        elif len(message) > 0:
+            return str(message[-1])
+        else:
+            return ''  # Fallback for empty list
+    elif isinstance(message, dict) and 'content' in message:
+        # For v1 endpoint, message is a dict with content key
+        return message['content']
+    else:
+        # Fallback for other formats (e.g., error strings)
+        return str(message)

learning_assistant/views.py

@@ -38,6 +38,7 @@
 from learning_assistant.serializers import MessageSerializer
 from learning_assistant.toggles import chat_history_enabled
 from learning_assistant.utils import (
+    extract_message_content,
     get_audit_trial_length_days,
     get_chat_response,
     parse_lms_datetime,
@@ -115,7 +116,8 @@ def _get_next_message(self, request, courserun_key, course_run_id):
         status_code, message = get_chat_response(prompt_template, message_list)
 
         if chat_history_enabled(courserun_key):
-            save_chat_message(courserun_key, user_id, LearningAssistantMessage.ASSISTANT_ROLE, message['content'])
+            content = extract_message_content(message)
+            save_chat_message(courserun_key, user_id, LearningAssistantMessage.ASSISTANT_ROLE, content)
 
         return Response(status=status_code, data=message)
 

test_settings.py

@@ -43,6 +43,10 @@ def root(*args):
 
 SECRET_KEY = 'insecure-secret-key'
 
+# Timezone settings
+USE_TZ = True
+TIME_ZONE = 'UTC'  # Use UTC to avoid timezone conversion issues in tests
+
 MIDDLEWARE = (
     'django.contrib.sessions.middleware.SessionMiddleware',
     'django.contrib.auth.middleware.AuthenticationMiddleware',

tests/test_api.py

@@ -12,6 +12,7 @@
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
 from django.test import TestCase, override_settings
+from django.utils import timezone
 from freezegun import freeze_time
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey, UsageKey
@@ -568,18 +569,18 @@ def setUp(self):
         datetime(2024, 1, 29),
     )
     def test_exists(self, audit_trial_expiration_date):
-        audit_trial_start_date = datetime.now()
+        audit_trial_start_date = timezone.now()
 
         LearningAssistantAuditTrial.objects.create(
             user=self.user,
             start_date=audit_trial_start_date,
-            expiration_date=audit_trial_expiration_date,
+            expiration_date=timezone.make_aware(audit_trial_expiration_date)
         )
 
         expected_return = LearningAssistantAuditTrialData(
             user_id=self.user.id,
             start_date=audit_trial_start_date,
-            expiration_date=audit_trial_expiration_date,
+            expiration_date=timezone.make_aware(audit_trial_expiration_date),
         )
         self.assertEqual(expected_return, get_audit_trial(self.user))
 
@@ -608,19 +609,19 @@ def setUp(self):
     )
     @patch('learning_assistant.api.get_audit_trial_expiration_date_from_start_date')
     def test_exists_get(self, audit_trial_expiration_date, get_audit_trial_expiration_date_mock):
-        audit_trial_start_date = datetime.now()
-        get_audit_trial_expiration_date_mock.return_value = audit_trial_expiration_date
+        audit_trial_start_date = timezone.now()
+        get_audit_trial_expiration_date_mock.return_value = timezone.make_aware(audit_trial_expiration_date)
 
         LearningAssistantAuditTrial.objects.create(
             user=self.user,
             start_date=audit_trial_start_date,
-            expiration_date=audit_trial_expiration_date,
+            expiration_date=timezone.make_aware(audit_trial_expiration_date),
         )
 
         expected_return = LearningAssistantAuditTrialData(
             user_id=self.user.id,
             start_date=audit_trial_start_date,
-            expiration_date=audit_trial_expiration_date,
+            expiration_date=timezone.make_aware(audit_trial_expiration_date),
         )
         self.assertEqual(expected_return, get_or_create_audit_trial(self.user, 'verified'))
 
@@ -634,13 +635,13 @@ def test_not_exists_create(self, audit_trial_expiration_date, get_audit_trial_ex
         other_user = User(username='other-tester', email='other-tester@test.com')
         other_user.save()
 
-        start_date = datetime.now()
-        get_audit_trial_expiration_date_mock.return_value = audit_trial_expiration_date
+        start_date = timezone.now()
+        get_audit_trial_expiration_date_mock.return_value = timezone.make_aware(audit_trial_expiration_date)
 
         expected_return = LearningAssistantAuditTrialData(
             user_id=self.user.id,
             start_date=start_date,
-            expiration_date=audit_trial_expiration_date
+            expiration_date=timezone.make_aware(audit_trial_expiration_date)
         )
 
         self.assertEqual(expected_return, get_or_create_audit_trial(self.user, 'verified'))
@@ -660,7 +661,7 @@ def setUp(self):
         self.user.save()
 
     def test_upgrade_deadline_expired(self):
-        today = datetime.now()
+        today = timezone.now()
         mock_enrollment = MagicMock()
         mock_enrollment.upgrade_deadline = today - timedelta(days=1)  # yesterday
 
@@ -674,7 +675,7 @@ def test_upgrade_deadline_expired(self):
         self.assertEqual(audit_trial_is_expired(mock_enrollment, audit_trial_data), True)
 
     def test_upgrade_deadline_none(self):
-        today = datetime.now()
+        today = timezone.now()
         mock_enrollment = MagicMock()
         mock_enrollment.upgrade_deadline = None
 
@@ -705,10 +706,12 @@ def test_upgrade_deadline_none(self):
         datetime(year=2024, month=1, day=1) - timedelta(days=settings.LEARNING_ASSISTANT_AUDIT_TRIAL_LENGTH_DAYS + 1),
     )
     def test_audit_trial_expired(self, start_date):
-        today = datetime.now()
+        today = timezone.now()
         mock_enrollment = MagicMock()
         mock_enrollment.upgrade_deadline = today + timedelta(days=1)  # tomorrow
 
+        # Convert naive start_date to timezone-aware
+        start_date = timezone.make_aware(start_date)
         audit_trial_data = LearningAssistantAuditTrialData(
             user_id=self.user.id,
             start_date=start_date,
@@ -718,7 +721,7 @@ def test_audit_trial_expired(self, start_date):
         self.assertEqual(audit_trial_is_expired(mock_enrollment, audit_trial_data), True)
 
     def test_audit_trial_unexpired(self):
-        today = datetime.now()
+        today = timezone.now()
         mock_enrollment = MagicMock()
         mock_enrollment.upgrade_deadline = today + timedelta(days=1)  # tomorrow
 

tests/test_utils.py

@@ -13,6 +13,7 @@
 
 from learning_assistant.constants import LMS_DATETIME_FORMAT
 from learning_assistant.utils import (
+    extract_message_content,
     get_audit_trial_length_days,
     get_chat_response,
     get_optimizely_variation,
@@ -148,6 +149,31 @@ def test_post_request_structure_v2_endpoint(self, mock_requests, mock_v2_enabled
             timeout=(connect_timeout, read_timeout)
         )
 
+    @ddt.data(
+        # (v2_enabled, response_data, description)
+        (False, {'role': 'assistant', 'content': 'v1 response'}, 'v1 single dict'),
+        (True, [{'role': 'assistant', 'content': 'v2 response'}], 'v2 array of dicts'),
+        (True, {'role': 'assistant', 'content': 'v2 dict'}, 'v2 unexpected dict format'),
+    )
+    @ddt.unpack
+    @responses.activate
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_endpoint_response_formats(self, v2_enabled, response_data, description, mock_v2_enabled):
+        """Test that both v1 and v2 endpoint response formats are handled correctly."""
+        mock_v2_enabled.return_value = v2_enabled
+
+        endpoint = settings.CHAT_COMPLETION_API_V2 if v2_enabled else settings.CHAT_COMPLETION_API
+        responses.add(
+            responses.POST,
+            endpoint,
+            status=200,
+            body=json.dumps(response_data),
+        )
+
+        status_code, message = self.get_response()
+        self.assertEqual(status_code, 200, f"Failed for {description}")
+        self.assertEqual(message, response_data, f"Response mismatch for {description}")
+
 
 class GetReducedMessageListTests(TestCase):
     """
@@ -225,7 +251,9 @@ def test_get_audit_trial_length_days_no_setting(self):
     )
     @ddt.unpack
     @patch('learning_assistant.utils.get_optimizely_variation')
-    def test_get_audit_trial_length_days_experiment(self, variation_key, expected_value, mock_get_optimizely_variation):
+    def test_get_audit_trial_length_days_experiment(
+        self, variation_key, expected_value, mock_get_optimizely_variation
+    ):
         mock_get_optimizely_variation.return_value = {'enabled': True, 'variation_key': variation_key}
         with patch.object(settings, 'OPTIMIZELY_LEARNING_ASSISTANT_TRIAL_VARIATION_KEY_28', 'variation'):
             self.assertEqual(get_audit_trial_length_days(1, 'verified'), expected_value)
@@ -273,3 +301,107 @@ def test_wrong_date(self):
         response = parse_lms_datetime('when I get my homework done')
 
         self.assertEqual(response, expected_value)
+
+
+@ddt.ddt
+class ExtractMessageContentTests(TestCase):
+    """
+    Tests for the extract_message_content utility function
+    """
+
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_v2_endpoint_with_list_dict_message(self, mock_v2_enabled):
+        """Test v2 endpoint with list containing dict messages"""
+        mock_v2_enabled.return_value = True
+
+        message = [
+            {'role': 'assistant', 'content': 'First message'},
+            {'role': 'assistant', 'content': 'Last message'}
+        ]
+
+        result = extract_message_content(message)
+        self.assertEqual(result, 'Last message')
+
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_v2_endpoint_with_list_non_dict_message(self, mock_v2_enabled):
+        """Test v2 endpoint with list containing non-dict messages"""
+        mock_v2_enabled.return_value = True
+
+        message = ['First message', 'Last message']
+
+        result = extract_message_content(message)
+        self.assertEqual(result, 'Last message')
+
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_v2_endpoint_with_empty_list(self, mock_v2_enabled):
+        """Test v2 endpoint with empty list"""
+        mock_v2_enabled.return_value = True
+
+        message = []
+
+        result = extract_message_content(message)
+        self.assertEqual(result, '')
+
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_v2_endpoint_with_dict_missing_content(self, mock_v2_enabled):
+        """Test v2 endpoint with dict message missing content key"""
+        mock_v2_enabled.return_value = True
+
+        message = [{'role': 'assistant'}]
+
+        result = extract_message_content(message)
+        self.assertEqual(result, '')
+
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_v1_endpoint_with_dict_message(self, mock_v2_enabled):
+        """Test v1 endpoint with dict message containing content"""
+        mock_v2_enabled.return_value = False
+
+        message = {'role': 'assistant', 'content': 'v1 response'}
+
+        result = extract_message_content(message)
+        self.assertEqual(result, 'v1 response')
+
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_v1_endpoint_with_dict_missing_content(self, mock_v2_enabled):
+        """Test v1 endpoint with dict message missing content key"""
+        mock_v2_enabled.return_value = False
+
+        message = {'role': 'assistant'}
+
+        result = extract_message_content(message)
+        self.assertEqual(result, "{'role': 'assistant'}")
+
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_fallback_with_string_message(self, mock_v2_enabled):
+        """Test fallback case with string message"""
+        mock_v2_enabled.return_value = False
+
+        message = 'Error: Something went wrong'
+
+        result = extract_message_content(message)
+        self.assertEqual(result, 'Error: Something went wrong')
+
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_fallback_with_none_message(self, mock_v2_enabled):
+        """Test fallback case with None message"""
+        mock_v2_enabled.return_value = False
+
+        message = None
+
+        result = extract_message_content(message)
+        self.assertEqual(result, 'None')
+
+    @patch('learning_assistant.utils.v2_endpoint_enabled')
+    def test_v2_endpoint_mixed_message_types(self, mock_v2_enabled):
+        """Test v2 endpoint with mixed message types in list"""
+        mock_v2_enabled.return_value = True
+
+        message = [
+            'First string message',
+            {'role': 'assistant', 'content': 'Dict message'},
+            'Last string message'
+        ]
+
+        result = extract_message_content(message)
+        self.assertEqual(result, 'Last string message')