Route QTI exercises by what each item can express, with one numeric answer reader

contentcuration/contentcuration/tests/test_restore_channel.py

@@ -317,7 +317,7 @@ def _add_perseus_item(self, order, image):
         image.save()
         return item
 
-    def _publish(self, generator_class):
+    def _publish(self, generator_class, **options):
         items = self.source.assessment_items.order_by("order")
         generator_class(
             self.source,
@@ -332,6 +332,7 @@ def _publish(self, generator_class):
             self.channel.id,
             "en-US",
             user_id=self.user.id,
+            **options,
         ).create_exercise_archive()
 
     def _restore(self, *presets, package=None, download_url="http://studio"):
@@ -464,7 +465,7 @@ def test_dual_published_node_restores_each_question_once(self):
         self._add_legacy_item(1)
         self._add_editor_item_with_encoded_id(2)
         self._publish(QTIExerciseGenerator)
-        self._publish(PerseusExerciseGenerator)
+        self._publish(PerseusExerciseGenerator, qti_item_ids=True)
 
         items = self._restore(format_presets.QTI_ZIP, format_presets.EXERCISE)
 
@@ -548,7 +549,7 @@ def test_unusable_package_restores_no_items(self):
         # The dual node's exercise archive holds QTI ids, which cannot republish.
         self._add_editor_item(0)
         self._publish(QTIExerciseGenerator)
-        self._publish(PerseusExerciseGenerator)
+        self._publish(PerseusExerciseGenerator, qti_item_ids=True)
 
         empty_package = BytesIO()
         with zipfile.ZipFile(empty_package, "w") as zf:
@@ -635,8 +636,7 @@ def _published_presets(self, node):
         os.remove(create_content_database(self.channel, True, self.user.id, False))
         return set(node.files.values_list("preset_id", flat=True))
 
-    def test_restored_legacy_only_node_also_publishes_derived_exercise_archive(self):
-        # Legacy items restore as QTI, which publish can also derive Perseus from.
+    def test_restored_legacy_only_node_publishes_both_archives(self):
         self._add_legacy_item(0)
         self._add_item(
             exercises.MULTIPLE_SELECTION,
@@ -649,11 +649,9 @@ def test_restored_legacy_only_node_also_publishes_derived_exercise_archive(self)
                 ]
             ),
         )
-        self.assertEqual(self._published_presets(self.source), {format_presets.QTI_ZIP})
+        both_archives = {format_presets.QTI_ZIP, format_presets.EXERCISE}
+        self.assertEqual(self._published_presets(self.source), both_archives)
 
         self._restore(format_presets.QTI_ZIP)
 
-        self.assertEqual(
-            self._published_presets(self.restored),
-            {format_presets.QTI_ZIP, format_presets.EXERCISE},
-        )
+        self.assertEqual(self._published_presets(self.restored), both_archives)

contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py

@@ -6,6 +6,7 @@
 from contentcuration.tests.utils.qti.test_validation import _item_xml
 from contentcuration.tests.utils.qti.test_validation import ENTITY_CHOICE_ITEM
 from contentcuration.tests.utils.qti.test_validation import HINTED_EDITOR_ITEM
+from contentcuration.utils.assessment.qti.archive import qti_package_identifier
 from contentcuration.utils.assessment.qti.perseus_derive import derive_perseus_item
 from contentcuration.utils.assessment.qti.perseus_derive import (
     is_answerless_numeric_entry,
@@ -20,6 +21,7 @@ def __init__(self, raw_data, randomize=False, assessment_id="a" * 32):
         self.raw_data = raw_data
         self.randomize = randomize
         self.assessment_id = assessment_id
+        self.type = exercises.QTI
 
 
 def _choice_item(
@@ -63,6 +65,7 @@ def _text_item(
     prompt="<p>What is 6 times 7?</p>",
     base_type="float",
     map_entries=(),
+    identifier="item_text",
 ):
     correct = ""
     if correct_values:
@@ -78,7 +81,7 @@ def _text_item(
             )
         )
     return _item_xml(
-        "item_text",
+        identifier,
         "Text Item",
         '<qti-response-declaration identifier="RESPONSE" cardinality="{}" '
         'base-type="{}">{}{}</qti-response-declaration>'.format(
@@ -199,23 +202,53 @@ def test_choice_padded_correct_value_matches_identifier():
     assert [a["correct"] for a in answers] == [True, False]
 
 
-def test_overflowing_correct_value_is_dropped():
-    raw_data = _text_item("single", ["42", "1e400"])
+@pytest.mark.parametrize(
+    "correct_values",
+    [
+        pytest.param(["42", "1e400"], id="overflowing_value"),
+        pytest.param(["42", "1/2"], id="fraction_value"),
+        pytest.param(["42", "False"], id="non_number_value"),
+    ],
+)
+def test_unreadable_correct_value_is_left_out_of_package(correct_values):
+    raw_data = _text_item("single", correct_values)
     assert is_perseus_derivable(raw_data) is True
-    answers = json.loads(derive_perseus_item(_Item(raw_data)).answers)
-    assert [a["answer"] for a in answers] == ["42"]
+    assert qti_package_identifier(_Item(raw_data)) is None
+
+
+@pytest.mark.parametrize(
+    "raw_data",
+    [
+        pytest.param(_text_item("single", ["42", "+3", "1E3"]), id="numbers"),
+        pytest.param(_text_item("single", ["cat"], base_type="string"), id="string"),
+        pytest.param(
+            _text_item("single", ["42"], map_entries=[("1/2", "1.0")]),
+            id="unreadable_map_key",
+        ),
+    ],
+)
+def test_readable_declarations_ship_in_package(raw_data):
+    assert qti_package_identifier(_Item(raw_data)) is not None
 
 
-def test_answerless_float_entry_is_skipped_but_node_stays_derivable(caplog):
+def test_answerless_float_entry_is_left_out_but_node_stays_derivable():
     raw_data = _text_item("single", [])
-    assert derive_perseus_item(_Item(raw_data)) is None
-    assert "no correct answer" in caplog.text
+    assert qti_package_identifier(_Item(raw_data)) is None
+    assert is_perseus_derivable(raw_data) is True
+
+
+def test_schema_invalid_item_is_left_out_but_node_stays_derivable():
+    raw_data = _text_item("single", ["4"]).replace(
+        'expected-length="50"', 'expected-length="abc"'
+    )
+    assert qti_package_identifier(_Item(raw_data)) is None
     assert is_perseus_derivable(raw_data) is True
 
 
 def test_math_prompt_survives_into_question():
     prompt = (
-        "<p>Solve <math><semantics><mrow><mi>x</mi></mrow>"
+        '<p>Solve <math xmlns="http://www.w3.org/1998/Math/MathML">'
+        "<semantics><mrow><mi>x</mi></mrow>"
         '<annotation encoding="application/x-tex">x^2</annotation>'
         "</semantics></math></p>"
     )
@@ -356,16 +389,19 @@ def test_response_identifier_mismatch_degrades_to_qti_only():
     'rptemplates/match_correct.xml" />'
 )
 
-TOLERANCE_ITEM = _text_item("single", []).replace(
-    MATCH_CORRECT_PROCESSING,
+TOLERANCE_PROCESSING = (
     "<qti-response-processing><qti-response-condition><qti-response-if>"
     '<qti-equal tolerance-mode="absolute" tolerance="0.5">'
     '<qti-variable identifier="RESPONSE" />'
     '<qti-base-value base-type="float">42</qti-base-value></qti-equal>'
     '<qti-set-outcome-value identifier="SCORE">'
     '<qti-base-value base-type="float">1</qti-base-value>'
     "</qti-set-outcome-value></qti-response-if></qti-response-condition>"
-    "</qti-response-processing>",
+    "</qti-response-processing>"
+)
+
+TOLERANCE_ITEM = _text_item("single", []).replace(
+    MATCH_CORRECT_PROCESSING, TOLERANCE_PROCESSING
 )
 
 
@@ -510,6 +546,17 @@ def test_comment_inside_prompt_markup_does_not_change_question(prompt):
         pytest.param(
             _text_item("single", ["1" + "0" * 400]), True, id="float_overflow_integer"
         ),
+        pytest.param(_text_item("single", ["1_000"]), True, id="float_underscore"),
+        pytest.param(
+            _text_item("single", ["1_0E3"]), True, id="float_underscore_exponent"
+        ),
+        pytest.param(_text_item("single", ["٥"]), True, id="float_arabic_indic"),
+        pytest.param(_text_item("single", ["５"]), True, id="float_fullwidth"),
+        pytest.param(_text_item("single", ["1/2"]), True, id="float_fraction"),
+        pytest.param(_text_item("single", ["1,000"]), True, id="float_comma"),
+        pytest.param(_text_item("single", ["50%"]), True, id="float_percent"),
+        pytest.param(_text_item("single", ["+3"]), False, id="float_plus_sign"),
+        pytest.param(_text_item("single", ["1E3"]), False, id="float_exponent"),
         pytest.param(
             _text_item("single", [], map_entries=[("13", "1.0"), ("12", "1.0")]),
             False,

contentcuration/contentcuration/tests/utils/test_exercise_creation.py

@@ -943,7 +943,7 @@ def test_bad_checksum_image_input_answer_is_answerless_outside_master(self):
             answerless_input_ids(self.exercise_node), {image_only.assessment_id}
         )
 
-    def test_unit_keeps_answerless_input_question(self):
+    def test_unit_leaves_out_answerless_input_question(self):
         self.exercise_node.kind_id = content_kinds.TOPIC
         self.exercise_node.save()
         perseus_item, _ = self._create_perseus_item()
@@ -955,10 +955,9 @@ def test_unit_keeps_answerless_input_question(self):
         self._create_perseus_zip(_exercise_data([perseus_item, answerless]))
         exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE)
         zip_file, exercise_data = self._validate_perseus_zip(exercise_file)
-        self.assertIn(f"{answerless.assessment_id}.json", zip_file.namelist())
+        self.assertNotIn(f"{answerless.assessment_id}.json", zip_file.namelist())
         self.assertEqual(
-            exercise_data["all_assessment_items"],
-            [perseus_item.assessment_id, answerless.assessment_id],
+            exercise_data["all_assessment_items"], [perseus_item.assessment_id]
         )
 
     def test_numeric_input_answers_are_read(self):

contentcuration/contentcuration/utils/assessment/perseus.py

@@ -6,14 +6,14 @@
 
 from django.template.loader import render_to_string
 from django.utils.functional import cached_property
-from le_utils.constants import content_kinds
 from le_utils.constants import exercises
 from le_utils.constants import file_formats
 from le_utils.constants import format_presets
 
 from contentcuration.utils.assessment.base import ExerciseArchiveGenerator
 from contentcuration.utils.assessment.base import image_filenames
 from contentcuration.utils.assessment.base import packageable_answers
+from contentcuration.utils.assessment.qti.archive import qti_package_identifier
 from contentcuration.utils.assessment.qti.convert import accepted_answers
 from contentcuration.utils.assessment.qti.convert import format_number
 from contentcuration.utils.assessment.qti.convert import hex_to_qti_id
@@ -41,8 +41,6 @@ def is_perseus_readable_input_item(assessment_item):
 
 
 def answerless_input_ids(ccnode):
-    """Input questions an exercise's Perseus archive leaves out, judged from
-    stored answers, as exercise.json is written before any item is processed."""
     return {
         item.assessment_id
         for item in ccnode.assessment_items.filter(type=exercises.INPUT_QUESTION)
@@ -70,8 +68,9 @@ class PerseusExerciseGenerator(ExerciseArchiveGenerator):
         "true_false": "perseus/multiple_selection.json",
     }
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, qti_item_ids=False, **kwargs):
         super().__init__(*args, **kwargs)
+        self.qti_item_ids = qti_item_ids
         # Derived proxies for native QTI items, keyed by source assessment_id.
         # Computed once and reused by both exercise.json and item processing.
         self._derived_cache = None
@@ -82,17 +81,16 @@ def _derived_items(self):
         not be derived (already logged) and so is skipped from the archive."""
         if self._derived_cache is None:
             self._derived_cache = {
-                item.assessment_id: derive_perseus_item(item)
+                item.assessment_id: (
+                    derive_perseus_item(item) if qti_package_identifier(item) else None
+                )
                 for item in self.ccnode.assessment_items.all()
                 if item.type == exercises.QTI
             }
         return self._derived_cache
 
     @cached_property
     def _answerless_input_ids(self):
-        """Units keep them: their pre/post test lists every id."""
-        if self.ccnode.kind_id != content_kinds.EXERCISE:
-            return set()
         answerless_ids = answerless_input_ids(self.ccnode)
         for item in self.ccnode.assessment_items.filter(
             assessment_id__in=answerless_ids
@@ -121,12 +119,11 @@ def process_assessment_item(self, assessment_item):
         if assessment_item.type == exercises.QTI:
             derived = self._derived_items().get(assessment_item.assessment_id)
             if derived is None:
-                # derive_perseus_item already logs why it could not derive.
                 return
             return super().process_assessment_item(derived)
         if assessment_item.type == exercises.PERSEUS_QUESTION:
             self._write_raw_perseus_assets(assessment_item, self.get_image_file_path())
-        if self._derived_items():
+        if self.qti_item_ids:
             # Dual-published: AssessmentMetaData records the QTI manifest id.
             assessment_item = copy.copy(assessment_item)
             assessment_item.assessment_id = hex_to_qti_id(assessment_item.assessment_id)
@@ -158,14 +155,6 @@ def get_image_ref_prefix(self):
         return f"${exercises.IMG_PLACEHOLDER}/images"
 
     def _exercise_data_for_archive(self):
-        """``exercise.json`` must list the same item ids and types as the item
-        JSON files written into this archive. When the node has native QTI
-        items, every item is written by its QTI manifest id, and native items
-        carry their derived proxy's legacy Perseus ``type``. On a mismatch,
-        ``restore_channel``'s ``extract_assessment_items`` opens ``{hex}.json``
-        and raises ``FileNotFoundError``, and ``generate_assessment_item``
-        receives a ``qti`` type it cannot map.
-        """
         assessment_ids = [
             assessment_id
             for assessment_id in self.exercise_data.get("all_assessment_items", [])
@@ -185,7 +174,9 @@ def _exercise_data_for_archive(self):
                 new_ids.append(proxy.assessment_id)
                 new_mapping[proxy.assessment_id] = proxy.type
             else:
-                item_id = hex_to_qti_id(assessment_id) if derived else assessment_id
+                item_id = (
+                    hex_to_qti_id(assessment_id) if self.qti_item_ids else assessment_id
+                )
                 new_ids.append(item_id)
                 if assessment_id in original_mapping:
                     new_mapping[item_id] = original_mapping[assessment_id]