feat: short answer AI XBlock handle attachment encoding and errors

Author: ArturGaspar

ai_eval/shortanswer.py

@@ -1,5 +1,6 @@
 """Short answers Xblock with AI evaluation."""
 
+import chardet
 import logging
 import traceback
 import urllib.parse
@@ -144,6 +145,15 @@ def validate_field_data(self, validation, data):
                 )
             )
 
+        try:
+            self._get_attachments(data.attachment_urls)
+        except Exception:
+            validation.add(
+                ValidationMessage(
+                    ValidationMessage.ERROR, _("Error downloading attachments"),
+                )
+            )
+
     def student_view(self, context=None):
         """
         The primary view of the ShortAnswerAIEvalXBlock, shown to students
@@ -180,15 +190,17 @@ def student_view(self, context=None):
 
     def _download_attachment(self, url):
         with urllib.request.urlopen(url) as f:
-            return f.read().decode('utf-8')
+            data = f.read()
+            encoding = chardet.detect(data)['encoding']
+            return data.decode(encoding)
 
     def _filename_for_url(self, url):
         return urllib.parse.urlparse(url).path.split('/')[-1]
 
-    def _get_attachments(self):
+    def _get_attachments(self, attachment_urls):
         pool = Pool(self.ATTACHMENT_PARALLEL_DOWNLOADS)
-        attachments = pool.map(self._download_attachment, self.attachment_urls)
-        filenames = map(self._filename_for_url, self.attachment_urls)
+        attachments = pool.map(self._download_attachment, attachment_urls)
+        filenames = map(self._filename_for_url, attachment_urls)
         return zip(filenames, attachments)
 
     @XBlock.json_handler
@@ -197,7 +209,7 @@ def get_response(self, data, suffix=""):  # pylint: disable=unused-argument
         user_submission = str(data["user_input"])
 
         attachments = []
-        for filename, contents in self._get_attachments():
+        for filename, contents in self._get_attachments(self.attachment_urls):
             attachments.append(f"""
                 <attachment>
                     <filename>{saxutils.escape(filename)}</filename>

ai_eval/tests/test_ai_eval.py

@@ -3,6 +3,7 @@
 """
 # pylint: disable=redefined-outer-name,protected-access
 
+import urllib.request
 from unittest.mock import Mock, patch
 
 import pytest
@@ -145,6 +146,14 @@ def test_shortanswer_attachments(shortanswer_block_data):
     assert "<contents>file contents &lt;&amp;&gt;</contents>" in prompt
 
 
+def test_shortanswer_attachments_encoding(shortanswer_block_data):
+    """Test attachments for ShortAnswerAIEvalXBlock."""
+    block = ShortAnswerAIEvalXBlock(ToyRuntime(), DictFieldData(shortanswer_block_data), None)
+    urllib.request.urlopen = Mock(return_value=io.BytesIO("á".encode('latin-1')))
+    contents = block._download_attachment("http://example.com/1.txt")
+    assert contents == "á"
+
+
 def test_multiagent_block_finished():
     """Test the MultiAgentAIEvalXBlock for not allowing input after finished."""
     data = {

setup.py

@@ -32,6 +32,7 @@ def package_data(pkg, roots):
     install_requires=[
         "XBlock",
         "celery",
+        "chardet",
         "litellm>=1.42",
     ],
     entry_points={