From d63a85528f0e736e957f11d0260294a0e3c0a394 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Wed, 30 Sep 2026 21:08:51 -0700 Subject: [PATCH 1/3] fix(qti): leave answerless input questions out of Perseus archives Exercises only; units keep them, as their pre/post test lists every id. Assessment metadata drops them too when there is no QTI package. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tests/test_exportchannel.py | 170 +++++++++++++++++- .../tests/utils/test_exercise_creation.py | 99 ++++++++-- .../utils/assessment/perseus.py | 42 ++++- .../utils/assessment/qti/archive.py | 6 +- .../utils/assessment/qti/convert.py | 4 + .../contentcuration/utils/publish.py | 11 +- 6 files changed, 294 insertions(+), 38 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index a296d08967..cadbc97793 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -1,3 +1,4 @@ +import io import json import os import random @@ -53,6 +54,7 @@ from contentcuration.utils.publish import convert_channel_thumbnail from contentcuration.utils.publish import create_content_database from contentcuration.utils.publish import create_draft_channel_version +from contentcuration.utils.publish import create_kolibri_assessment_metadata from contentcuration.utils.publish import create_slideshow_manifest from contentcuration.utils.publish import fill_published_fields from contentcuration.utils.publish import map_prerequisites @@ -455,6 +457,85 @@ def setUp(self): randomize=False, ) + # Perseus packaging with an input question that has no accepted answer + perseus_answerless_input_exercise = create_node( + { + "kind_id": "exercise", + "title": "Perseus Answerless Input Exercise", + "extra_fields": qti_extra_fields, + } + ) + perseus_answerless_input_exercise.complete = True + perseus_answerless_input_exercise.parent = current_exercise.parent + perseus_answerless_input_exercise.save() + cc.AssessmentItem.objects.create( + contentnode=perseus_answerless_input_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.PERSEUS_QUESTION, + question="", + answers="[]", + hints="[]", + raw_data="{}", + order=1, + randomize=False, + ) + cc.AssessmentItem.objects.create( + contentnode=perseus_answerless_input_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.INPUT_QUESTION, + question="What is 2+3?", + answers=json.dumps([{"answer": "", "correct": True, "order": 1}]), + hints="[]", + raw_data="{}", + order=2, + randomize=False, + ) + + # Dual-published: derivable native QTI item plus an answerless input question + native_qti_answerless_input_exercise = create_node( + { + "kind_id": "exercise", + "title": "Native QTI Answerless Input Exercise", + "extra_fields": qti_extra_fields, + } + ) + native_qti_answerless_input_exercise.complete = True + native_qti_answerless_input_exercise.parent = current_exercise.parent + native_qti_answerless_input_exercise.save() + cc.AssessmentItem.objects.create( + contentnode=native_qti_answerless_input_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.QTI, + question="", + answers="[]", + hints="[]", + raw_data=VALID_CHOICE_ITEM, + order=1, + randomize=False, + ) + cc.AssessmentItem.objects.create( + contentnode=native_qti_answerless_input_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.INPUT_QUESTION, + question="What is 2+3?", + answers=json.dumps([{"answer": "", "correct": True, "order": 1}]), + hints="[]", + raw_data="{}", + order=2, + randomize=False, + ) + cc.AssessmentItem.objects.create( + contentnode=native_qti_answerless_input_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.INPUT_QUESTION, + question="What is 2+3?", + answers=json.dumps([{"answer": "5", "correct": True, "order": 1}]), + hints="[]", + raw_data="{}", + order=3, + randomize=False, + ) + # A node mixing a native QTI item with a raw perseus_question item -> # must route to a single QTI package (Perseus embedded as custom # interactions), not a separate Perseus archive. @@ -1104,18 +1185,24 @@ def item_contents(title): self.assertTrue(commented) self.assertEqual(commented, item_contents("Native QTI Exercise")) - def test_native_qti_perseus_ids_match_assessment_metadata(self): - """The derived Perseus item JSON filenames must equal the ids recorded - in the published node's ``AssessmentMetaData.assessment_item_ids`` (the - QTI manifest ``K``-ids), so older Kolibri resolves the derived items.""" - node = cc.ContentNode.objects.get(title="Native QTI Exercise") + def _read_perseus_archive(self, node): exercise_file = node.files.get(preset_id=format_presets.EXERCISE) - with exercise_file.file_on_disk.open("rb") as file_handle: + with exercise_file.file_on_disk.open("rb") as file_handle, zipfile.ZipFile( + file_handle + ) as archive: item_stems = { name[: -len(".json")] - for name in zipfile.ZipFile(file_handle).namelist() + for name in archive.namelist() if name.endswith(".json") and name != "exercise.json" } + return item_stems, json.loads(archive.read("exercise.json")) + + def test_native_qti_perseus_ids_match_assessment_metadata(self): + """The derived Perseus item JSON filenames must equal the ids recorded + in the published node's ``AssessmentMetaData.assessment_item_ids`` (the + QTI manifest ``K``-ids), so older Kolibri resolves the derived items.""" + node = cc.ContentNode.objects.get(title="Native QTI Exercise") + item_stems, _ = self._read_perseus_archive(node) published_node = kolibri_models.ContentNode.objects.get( title="Native QTI Exercise" @@ -1206,6 +1293,75 @@ def test_perseus_question_item_routes_to_perseus_packaging(self): self.assertTrue(node.files.filter(preset_id=format_presets.EXERCISE).exists()) self.assertFalse(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) + def test_answerless_input_left_out_of_perseus_archive_and_metadata(self): + node = cc.ContentNode.objects.get(title="Perseus Answerless Input Exercise") + perseus_id = node.assessment_items.get( + type=exercises.PERSEUS_QUESTION + ).assessment_id + self.assertFalse(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) + item_stems, exercise_data = self._read_perseus_archive(node) + self.assertEqual(item_stems, {perseus_id}) + self.assertEqual(exercise_data["all_assessment_items"], [perseus_id]) + + published_node = kolibri_models.ContentNode.objects.get( + title="Perseus Answerless Input Exercise" + ) + self.assertEqual( + published_node.assessmentmetadata.first().assessment_item_ids, + [perseus_id], + ) + + def test_unchanged_republish_metadata_leaves_out_answerless_input(self): + # An unchanged node keeps its archive, which may predate the skip. + node = cc.ContentNode.objects.get(title="Perseus Answerless Input Exercise") + all_ids = list( + node.assessment_items.order_by("order").values_list( + "assessment_id", flat=True + ) + ) + stale_archive = io.BytesIO() + with zipfile.ZipFile(stale_archive, "w") as archive: + archive.writestr( + "exercise.json", json.dumps({"all_assessment_items": all_ids}) + ) + for assessment_id in all_ids: + archive.writestr(f"{assessment_id}.json", "{}") + node.files.filter(preset_id=format_presets.EXERCISE).delete() + stale_file = create_studio_file( + stale_archive.getvalue(), preset=format_presets.EXERCISE, ext="perseus" + )["db_file"] + stale_file.contentnode = node + stale_file.save() + published_node = kolibri_models.ContentNode.objects.get( + title="Perseus Answerless Input Exercise" + ) + published_node.assessmentmetadata.all().delete() + + create_kolibri_assessment_metadata(node, published_node) + + self.assertEqual( + published_node.assessmentmetadata.get().assessment_item_ids, + [all_ids[0]], + ) + + def test_answerless_input_left_out_of_dual_published_archives(self): + node = cc.ContentNode.objects.get(title="Native QTI Answerless Input Exercise") + answerless_id = node.assessment_items.get( + type=exercises.INPUT_QUESTION, order=2 + ).assessment_id + item_stems, exercise_data = self._read_perseus_archive(node) + self.assertEqual(len(item_stems), 2) + self.assertNotIn(hex_to_qti_id(answerless_id), item_stems) + self.assertEqual(set(exercise_data["all_assessment_items"]), item_stems) + + published_node = kolibri_models.ContentNode.objects.get( + title="Native QTI Answerless Input Exercise" + ) + self.assertEqual( + set(published_node.assessmentmetadata.first().assessment_item_ids), + item_stems, + ) + def test_mixed_perseus_and_native_qti_routes_to_qti(self): node = cc.ContentNode.objects.get(title="Perseus + Native QTI Mixed Exercise") self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) diff --git a/contentcuration/contentcuration/tests/utils/test_exercise_creation.py b/contentcuration/contentcuration/tests/utils/test_exercise_creation.py index 5ffd864422..8dab079411 100644 --- a/contentcuration/contentcuration/tests/utils/test_exercise_creation.py +++ b/contentcuration/contentcuration/tests/utils/test_exercise_creation.py @@ -179,6 +179,17 @@ def _assert_upright_150x200(test, content): test.assertGreater(blue, red) +def _exercise_data(items): + return { + "mastery_model": exercises.M_OF_N, + "randomize": True, + "n": len(items), + "m": 1, + "all_assessment_items": [item.assessment_id for item in items], + "assessment_mapping": {item.assessment_id: item.type for item in items}, + } + + class TestPerseusExerciseCreation(StudioTestCase): """ Tests for the create_perseus_exercise function which handles exercise file generation. @@ -719,6 +730,70 @@ def test_exercise_with_graphie(self): processed_perseus_json, ) + def _assert_only_perseus_item_packaged(self, perseus_item, answerless): + with self.assertLogs(level="WARNING") as logs: + self._create_perseus_zip(_exercise_data([perseus_item, answerless])) + self.assertTrue(any("no correct answer" in message for message in logs.output)) + exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE) + zip_file, exercise_data = self._validate_perseus_zip(exercise_file) + self.assertNotIn(f"{answerless.assessment_id}.json", zip_file.namelist()) + self.assertEqual( + exercise_data["all_assessment_items"], [perseus_item.assessment_id] + ) + self.assertEqual( + exercise_data["assessment_mapping"], + {perseus_item.assessment_id: exercises.PERSEUS_QUESTION}, + ) + return zip_file + + def test_answerless_input_question_is_left_out(self): + perseus_item, _ = self._create_perseus_item() + for answers in ( + [{"answer": "", "correct": True, "order": 1}], + [{"answer": False, "correct": True, "order": 1}], + [{"answer": None, "order": 1}], + [{"answer": "5", "correct": False, "order": 1}], + [], + ): + with self.subTest(answers=answers): + answerless = self._create_assessment_item( + exercises.INPUT_QUESTION, "What is 2+3?", answers + ) + self._assert_only_perseus_item_packaged(perseus_item, answerless) + answerless.delete() + + def test_answerless_input_question_images_are_not_packaged(self): + perseus_item, _ = self._create_perseus_item() + image_file = fileobj_exercise_image() + image_url = exercises.CONTENT_STORAGE_FORMAT.format(image_file.filename()) + answerless = self._create_assessment_item( + exercises.INPUT_QUESTION, + f"How many sides? ![shape]({image_url})", + [{"answer": "", "correct": True, "order": 1}], + ) + image_file.assessment_item = answerless + image_file.save() + zip_file = self._assert_only_perseus_item_packaged(perseus_item, answerless) + self.assertNotIn(f"images/{image_file.filename()}", zip_file.namelist()) + + def test_unit_keeps_answerless_input_question(self): + self.exercise_node.kind_id = content_kinds.TOPIC + self.exercise_node.save() + perseus_item, _ = self._create_perseus_item() + answerless = self._create_assessment_item( + exercises.INPUT_QUESTION, + "What is 2+3?", + [{"answer": "", "correct": True, "order": 1}], + ) + 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.assertEqual( + exercise_data["all_assessment_items"], + [perseus_item.assessment_id, answerless.assessment_id], + ) + def test_formula_processing(self): """Test that formulas are properly processed in exercises""" # Create a question with LaTeX formulas @@ -2259,7 +2334,7 @@ def _publish_legacy_sized_image(self, content, ext, width, height, ref_ext=None) image_file.assessment_item = item image_file.save() - self._create_qti_zip(self._exercise_data([item])) + self._create_qti_zip(_exercise_data([item])) exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP) zip_file = self._validate_qti_zip_structure(exercise_file) @@ -2635,19 +2710,9 @@ def test_native_qti_item_invalid_raw_data_is_skipped(self): [name for name in zip_file.namelist() if name.startswith("items/")], [] ) - def _exercise_data(self, items): - return { - "mastery_model": exercises.M_OF_N, - "randomize": True, - "n": len(items), - "m": 1, - "all_assessment_items": [item.assessment_id for item in items], - "assessment_mapping": {item.assessment_id: item.type for item in items}, - } - def _assert_only_item_packaged(self, items, identifier): with self.assertLogs(level="WARNING") as logs: - self._create_qti_zip(self._exercise_data(items)) + self._create_qti_zip(_exercise_data(items)) self.assertTrue(any("no correct answer" in message for message in logs.output)) exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP) zip_file = self._validate_qti_zip_structure(exercise_file) @@ -2699,7 +2764,7 @@ def test_legacy_input_question_zero_answer_is_packaged(self): "What is 5 minus 5?", [{"answer": 0, "correct": True, "order": 1}], ) - self._create_qti_zip(self._exercise_data([item])) + self._create_qti_zip(_exercise_data([item])) exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP) zip_file = self._validate_qti_zip_structure(exercise_file) item_xml = zip_file.read( @@ -2714,7 +2779,7 @@ def test_native_answerless_float_entry_is_skipped(self): def test_native_custom_processed_float_entry_is_packaged(self): item = self._create_native_qti_item(TOLERANCE_ITEM) - self._create_qti_zip(self._exercise_data([item])) + self._create_qti_zip(_exercise_data([item])) exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP) zip_file = self._validate_qti_zip_structure(exercise_file) self.assertIn("items/item_text.xml", zip_file.namelist()) @@ -2722,7 +2787,7 @@ def test_native_custom_processed_float_entry_is_packaged(self): def _assert_missing_media_is_logged_and_omitted(self, raw_data): item = self._create_native_qti_item(raw_data) # no File row linked with self.assertLogs(level="ERROR") as logs: - self._create_qti_zip(self._exercise_data([item])) + self._create_qti_zip(_exercise_data([item])) self.assertTrue( any("no matching File record linked" in message for message in logs.output) ) @@ -2755,7 +2820,7 @@ def _create_native_qti_image_item(self, image_file, imgs): def _publish_native_qti_images(self, image_file, imgs): item = self._create_native_qti_image_item(image_file, imgs) - self._create_qti_zip(self._exercise_data([item])) + self._create_qti_zip(_exercise_data([item])) exercise_file = self.exercise_node.files.get(preset_id=format_presets.QTI_ZIP) zip_file = self._validate_qti_zip_structure(exercise_file) item_doc = parse_qti_xml(zip_file.read("items/native_item_1.xml")) @@ -2979,7 +3044,7 @@ def test_native_qti_perseus_derivation_resizes_sized_images(self): image_file, self.TWO_SIZE_IMGS.format(filename=filename) ) - self._create_perseus_zip(self._exercise_data([item])) + self._create_perseus_zip(_exercise_data([item])) exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE) with storage.open(exercise_file.file_on_disk.name, "rb") as f: diff --git a/contentcuration/contentcuration/utils/assessment/perseus.py b/contentcuration/contentcuration/utils/assessment/perseus.py index adf0c4fb25..e8fafff83a 100644 --- a/contentcuration/contentcuration/utils/assessment/perseus.py +++ b/contentcuration/contentcuration/utils/assessment/perseus.py @@ -1,20 +1,26 @@ import copy import json +import logging import math import re import zipfile 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.qti.convert import hex_to_qti_id +from contentcuration.utils.assessment.qti.convert import is_answerless_input from contentcuration.utils.assessment.qti.perseus_derive import derive_perseus_item from contentcuration.utils.parser import extract_value +logger = logging.getLogger(__name__) + _DOUBLE_DOLLAR_RE = re.compile(r"\$\$(.+?)\$\$", flags=re.DOTALL) @@ -56,6 +62,22 @@ def _derived_items(self): } return self._derived_cache + @cached_property + def _answerless_input_ids(self): + """Judged from stored answers, as exercise.json is written before any + item is processed. Units keep them: their pre/post test lists every id.""" + answerless_ids = set() + if self.ccnode.kind_id != content_kinds.EXERCISE: + return answerless_ids + for item in self.ccnode.assessment_items.filter(type=exercises.INPUT_QUESTION): + if is_answerless_input(item.type, json.loads(item.answers)): + logger.warning( + f"Input question {item.assessment_id} on node {self.ccnode.pk} " + f"has no correct answer and will be excluded from the Perseus archive" + ) + answerless_ids.add(item.assessment_id) + return answerless_ids + def _process_formulas(self, content): return _DOUBLE_DOLLAR_RE.sub(r"$\1$", content) @@ -64,6 +86,8 @@ def _process_content(self, content): return super()._process_content(content) def process_assessment_item(self, assessment_item): + if assessment_item.assessment_id in self._answerless_input_ids: + return if assessment_item.type == exercises.QTI: derived = self._derived_items().get(assessment_item.assessment_id) if derived is None: @@ -118,14 +142,16 @@ def _exercise_data_for_archive(self): and raises ``FileNotFoundError``, and ``generate_assessment_item`` receives a ``qti`` type it cannot map. """ - derived = self._derived_items() - if not derived: - return self.exercise_data - + assessment_ids = [ + assessment_id + for assessment_id in self.exercise_data.get("all_assessment_items", []) + if assessment_id not in self._answerless_input_ids + ] original_mapping = self.exercise_data.get("assessment_mapping", {}) + derived = self._derived_items() new_ids = [] new_mapping = {} - for assessment_id in self.exercise_data.get("all_assessment_items", []): + for assessment_id in assessment_ids: if assessment_id in derived: proxy = derived[assessment_id] if proxy is None: @@ -135,10 +161,10 @@ def _exercise_data_for_archive(self): new_ids.append(proxy.assessment_id) new_mapping[proxy.assessment_id] = proxy.type else: - qti_id = hex_to_qti_id(assessment_id) - new_ids.append(qti_id) + item_id = hex_to_qti_id(assessment_id) if derived else assessment_id + new_ids.append(item_id) if assessment_id in original_mapping: - new_mapping[qti_id] = original_mapping[assessment_id] + new_mapping[item_id] = original_mapping[assessment_id] return { **self.exercise_data, "all_assessment_items": new_ids, diff --git a/contentcuration/contentcuration/utils/assessment/qti/archive.py b/contentcuration/contentcuration/utils/assessment/qti/archive.py index af77f65d21..e76291dc64 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/archive.py +++ b/contentcuration/contentcuration/utils/assessment/qti/archive.py @@ -13,7 +13,6 @@ from contentcuration.utils.assessment.base import ExerciseArchiveGenerator from contentcuration.utils.assessment.qti.constants import ResourceType -from contentcuration.utils.assessment.qti.convert import accepted_answers from contentcuration.utils.assessment.qti.convert import ( build_perseus_custom_interaction_item, ) @@ -21,6 +20,7 @@ convert_legacy_assessment_item_to_qti, ) from contentcuration.utils.assessment.qti.convert import hex_to_qti_id +from contentcuration.utils.assessment.qti.convert import is_answerless_input from contentcuration.utils.assessment.qti.convert import LegacyAssessmentItem from contentcuration.utils.assessment.qti.imsmanifest import File as ManifestFile from contentcuration.utils.assessment.qti.imsmanifest import Manifest @@ -102,9 +102,7 @@ def _warn_answerless(self, assessment_item): def _skip_answerless_input(self, assessment_item, answers) -> bool: # Conversion makes an input question with no accepted answer an # answerless float entry. - is_answerless = assessment_item.type == exercises.INPUT_QUESTION and not ( - accepted_answers(answers) - ) + is_answerless = is_answerless_input(assessment_item.type, answers) if is_answerless: self._warn_answerless(assessment_item) return is_answerless diff --git a/contentcuration/contentcuration/utils/assessment/qti/convert.py b/contentcuration/contentcuration/utils/assessment/qti/convert.py index 311334b02f..4daed6ae70 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/convert.py +++ b/contentcuration/contentcuration/utils/assessment/qti/convert.py @@ -253,6 +253,10 @@ def accepted_answers(answers: List[Dict[str, Any]]) -> List[str]: return accepted +def is_answerless_input(item_type: str, answers: List[Dict[str, Any]]) -> bool: + return item_type == exercises.INPUT_QUESTION and not accepted_answers(answers) + + # Decimal literals Kolibri's Number() reads that extract_value does not, e.g. +3, 1E3. _JS_NUMBER = re.compile(r"[+-]?(\d+\.?\d*|\.\d+)([eE][+-]?\d+)?", re.ASCII) diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 060bf63304..8664a52693 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -48,6 +48,7 @@ from contentcuration.decorators import delay_user_storage_calculation from contentcuration.utils.assessment.perseus import PerseusExerciseGenerator from contentcuration.utils.assessment.qti.archive import QTIExerciseGenerator +from contentcuration.utils.assessment.qti.convert import is_answerless_input from contentcuration.utils.assessment.qti.imsmanifest import ( get_assessment_ids_from_manifest, ) @@ -838,15 +839,21 @@ def process_assessment_metadata(ccnode): def create_kolibri_assessment_metadata(ccnode, kolibrinode): assessment_items = ccnode.assessment_items.all().order_by("order") - assessment_item_ids = [a.assessment_id for a in assessment_items] randomize, _, mastery_model = _get_exercise_data_from_ccnode( - ccnode, len(assessment_item_ids) + ccnode, len(assessment_items) ) qti_file = ccnode.files.filter(preset_id=format_presets.QTI_ZIP).first() if qti_file: # Open the zip file from Django storage with qti_file.file_on_disk.open("rb") as file_handle: assessment_item_ids = get_assessment_ids_from_manifest(file_handle) + else: + # The Perseus archive leaves out answerless input questions. + assessment_item_ids = [ + a.assessment_id + for a in assessment_items + if not is_answerless_input(a.type, json.loads(a.answers)) + ] kolibrimodels.AssessmentMetaData.objects.create( id=uuid.uuid4(), From 94463a8631eb568d9274fc460d88e792e5d22f95 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Thu, 1 Oct 2026 15:14:52 -0700 Subject: [PATCH 2/3] fix(perseus): read numeric input answers as Perseus renders them - JSON-number answers (4, 0, 4.5) were published as answers: [] - Answers marked "correct": false were published as correct Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tests/utils/test_exercise_creation.py | 77 +++++++++++-------- .../utils/assessment/perseus.py | 35 +++++---- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/contentcuration/contentcuration/tests/utils/test_exercise_creation.py b/contentcuration/contentcuration/tests/utils/test_exercise_creation.py index 8dab079411..e2f7515151 100644 --- a/contentcuration/contentcuration/tests/utils/test_exercise_creation.py +++ b/contentcuration/contentcuration/tests/utils/test_exercise_creation.py @@ -259,6 +259,18 @@ def _create_perseus_zip(self, exercise_data): ) return generator.create_exercise_archive() + def _perseus_input_values(self, answers): + """Values the Perseus numeric-input renders for an input question.""" + item = self._create_assessment_item( + exercises.INPUT_QUESTION, "What is the answer?", answers + ) + self._create_perseus_zip(_exercise_data([item])) + exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE) + zip_file, _ = self._validate_perseus_zip(exercise_file) + item_json = json.loads(zip_file.read(f"{item.assessment_id}.json")) + widget = item_json["question"]["widgets"]["numeric-input 1"] + return [answer["value"] for answer in widget["options"]["answers"]] + def _validate_perseus_zip(self, exercise_file): """Helper to validate the structure of the Perseus zip file""" # Use Django's storage backend to read the file @@ -794,6 +806,36 @@ def test_unit_keeps_answerless_input_question(self): [perseus_item.assessment_id, answerless.assessment_id], ) + def test_json_number_input_answers_are_read(self): + for number in (4, 0, 4.5): + with self.subTest(number=number): + self.assertEqual( + self._perseus_input_values( + [{"answer": number, "correct": True, "order": 1}] + ), + [number], + ) + self.exercise_node.assessment_items.all().delete() + + def test_incorrect_input_answers_are_dropped(self): + values = self._perseus_input_values( + [ + {"answer": "4", "correct": True, "order": 1}, + {"answer": 5, "correct": False, "order": 2}, + {"answer": "6", "correct": False, "order": 3}, + ] + ) + self.assertEqual(values, [4]) + + def test_input_answer_read_after_unpackageable_image_is_stripped(self): + image_url = exercises.CONTENT_STORAGE_FORMAT.format( + f"{'a' * 32}.{file_formats.PNG}" + ) + values = self._perseus_input_values( + [{"answer": f"4![]({image_url} =0x0)", "correct": True, "order": 1}] + ) + self.assertEqual(values, [4]) + def test_formula_processing(self): """Test that formulas are properly processed in exercises""" # Create a question with LaTeX formulas @@ -829,41 +871,12 @@ def test_formula_processing(self): ) self.assertIn("$\\frac{x}{2} = 3$", item_json["question"]["content"]) - def _numeric_answer_values(self, answers): - item = self._create_assessment_item( - exercises.INPUT_QUESTION, - "What is the answer?", + def test_input_question_drops_non_finite_answers(self): + answers = ["6", "1e400", "-1e400", "1" * 401, "1" * 401 + "/" + "1" * 401] + values = self._perseus_input_values( [ {"answer": answer, "correct": True, "order": order} for order, answer in enumerate(answers, start=1) - ], - ) - self._create_perseus_zip( - { - "mastery_model": exercises.M_OF_N, - "randomize": True, - "n": 1, - "m": 1, - "all_assessment_items": [item.assessment_id], - "assessment_mapping": {item.assessment_id: exercises.INPUT_QUESTION}, - } - ) - exercise_file = self.exercise_node.files.get(preset_id=format_presets.EXERCISE) - zip_file, _ = self._validate_perseus_zip(exercise_file) - item_json = json.loads( - zip_file.read(f"{item.assessment_id}.json").decode("utf-8") - ) - options = item_json["question"]["widgets"]["numeric-input 1"]["options"] - return [answer["value"] for answer in options["answers"]] - - def test_input_question_drops_non_finite_answers(self): - values = self._numeric_answer_values( - [ - "6", - "1e400", - "-1e400", - "1" * 401, - "1" * 401 + "/" + "1" * 401, ] ) self.assertEqual(values, [6.0]) diff --git a/contentcuration/contentcuration/utils/assessment/perseus.py b/contentcuration/contentcuration/utils/assessment/perseus.py index e8fafff83a..8757f718ce 100644 --- a/contentcuration/contentcuration/utils/assessment/perseus.py +++ b/contentcuration/contentcuration/utils/assessment/perseus.py @@ -24,6 +24,13 @@ _DOUBLE_DOLLAR_RE = re.compile(r"\$\$(.+?)\$\$", flags=re.DOTALL) +def perseus_input_value(answer): + """The number a Perseus numeric-input renders for a legacy input answer, or None.""" + value = extract_value(str(answer)) + # JSON has no infinity, so the rendered item would not parse. + return value if value is not None and math.isfinite(value) else None + + class PerseusExerciseGenerator(ExerciseArchiveGenerator): """ Exercise zip generator for Perseus format exercises. @@ -102,15 +109,21 @@ def process_assessment_item(self, assessment_item): assessment_item.assessment_id = hex_to_qti_id(assessment_item.assessment_id) return super().process_assessment_item(assessment_item) - def _process_input_answers(self, processed_data): - """Extract input answer processing logic""" - numeric_answers = [] - for answer in processed_data["answers"]: - answer["answer"] = extract_value(answer["answer"]) - if answer["answer"] is not None and math.isfinite(answer["answer"]): - numeric_answers.append(answer) - - return {**processed_data, "answers": numeric_answers} + def _process_answers(self, assessment_item): + # The base drops a JSON 0 as falsy. + if assessment_item.type != exercises.INPUT_QUESTION: + return super()._process_answers(assessment_item) + answers = [] + for answer in json.loads(assessment_item.answers): + # The template marks every rendered answer correct. + if not answer.get("correct", True): + continue + if isinstance(answer["answer"], str): + answer["answer"], _ = self._process_content(answer["answer"]) + value = perseus_input_value(answer["answer"]) + if value is not None: + answers.append({**answer, "answer": value}) + return self._sort_by_order(answers, "answers") def create_assessment_item(self, assessment_item, processed_data): template = self.TEMPLATE_MAP.get(assessment_item.type) @@ -119,10 +132,6 @@ def create_assessment_item(self, assessment_item, processed_data): f"Unrecognized question type on item {assessment_item.assessment_id}: {assessment_item.type}" ) - # Handle input question special case - if assessment_item.type == exercises.INPUT_QUESTION: - processed_data = self._process_input_answers(processed_data) - filename = f"{assessment_item.assessment_id}.json" content = render_to_string(template, processed_data).encode("utf-8", "ignore") return filename, content From 97bcf22dc6deaa047bc28d5625f0345c1ef42097 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Thu, 1 Oct 2026 15:14:55 -0700 Subject: [PATCH 3/3] fix(qti): publish mixed exercises QTI only when Perseus can't read an input A legacy input answer such as "Sphere", "+3" or "1e400" left the derived Perseus numeric-input with no answers; such nodes now skip the Perseus archive. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tests/test_exportchannel.py | 78 +++++++++++++++++++ .../contentcuration/utils/publish.py | 7 ++ 2 files changed, 85 insertions(+) diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index cadbc97793..1e25d1711a 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -87,6 +87,9 @@ # Perseus input questions are numeric-only, so this publishes QTI only. STRING_ENTRY_QTI_ITEM = _text_item("single", ["cat"], base_type="string") +# Legacy input answers Perseus can't render as a finite number. +UNREADABLE_PERSEUS_INPUT_ANSWERS = ["Sphere", "+3", "1e400"] + # Larger than the signed 32-bit maximum (2_147_483_647); ~3 GB. LARGE_FILE_SIZE = 3 * 1024 ** 3 @@ -606,6 +609,41 @@ def setUp(self): randomize=False, ) + # Mixed nodes whose legacy input answer Perseus cannot read -> QTI only + for answer in UNREADABLE_PERSEUS_INPUT_ANSWERS: + unreadable_input_exercise = create_node( + { + "kind_id": "exercise", + "title": f"Native QTI + Input {answer!r} Mixed Exercise", + "extra_fields": qti_extra_fields, + } + ) + unreadable_input_exercise.complete = True + unreadable_input_exercise.parent = current_exercise.parent + unreadable_input_exercise.save() + cc.AssessmentItem.objects.create( + contentnode=unreadable_input_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.QTI, + question="", + answers="[]", + hints="[]", + raw_data=VALID_CHOICE_ITEM, + order=1, + randomize=False, + ) + cc.AssessmentItem.objects.create( + contentnode=unreadable_input_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.INPUT_QUESTION, + question="What is the answer?", + answers=json.dumps([{"answer": answer, "correct": True, "order": 1}]), + hints=json.dumps([]), + raw_data="{}", + order=2, + randomize=False, + ) + first_topic = self.content_channel.main_tree.get_descendants().first() # Add a publishable topic to ensure it does not inherit but that its children do @@ -1241,6 +1279,33 @@ def test_mixed_native_legacy_perseus_ids_match_assessment_metadata(self): set(exercise_data["assessment_mapping"]), set(assessment_item_ids) ) + def test_dual_published_numeric_inputs_have_answers(self): + dual_published = cc.ContentNode.objects.filter( + files__preset_id=format_presets.QTI_ZIP + ).filter(files__preset_id=format_presets.EXERCISE) + mixed_answers = [] + for node in dual_published: + exercise_file = node.files.get(preset_id=format_presets.EXERCISE) + with exercise_file.file_on_disk.open("rb") as file_handle, zipfile.ZipFile( + file_handle + ) as archive: + for name in archive.namelist(): + if not name.endswith(".json") or name == "exercise.json": + continue + widgets = json.loads(archive.read(name))["question"]["widgets"] + for widget in widgets.values(): + if widget["type"] != "numeric-input": + continue + answers = [ + answer["value"] for answer in widget["options"]["answers"] + ] + with self.subTest(node=node.title, item=name): + self.assertTrue(answers) + if node.title == "Native QTI + Legacy Mixed Exercise": + mixed_answers.append(answers) + + self.assertEqual(mixed_answers, [[4]]) + def test_native_qti_item_declares_the_node_language(self): """The editor has no language of its own to write, so publishing supplies it. @@ -1283,6 +1348,19 @@ def test_native_qti_string_entry_publishes_qti_only(self): self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) self.assertFalse(node.files.filter(preset_id=format_presets.EXERCISE).exists()) + def test_mixed_native_unreadable_input_publishes_qti_only(self): + for answer in UNREADABLE_PERSEUS_INPUT_ANSWERS: + with self.subTest(answer=answer): + node = cc.ContentNode.objects.get( + title=f"Native QTI + Input {answer!r} Mixed Exercise" + ) + self.assertTrue( + node.files.filter(preset_id=format_presets.QTI_ZIP).exists() + ) + self.assertFalse( + node.files.filter(preset_id=format_presets.EXERCISE).exists() + ) + def test_legacy_items_without_perseus_question_route_to_qti_packaging(self): node = cc.ContentNode.objects.get(title="Legacy No Perseus Exercise") self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py index 8664a52693..4d29ed75b4 100644 --- a/contentcuration/contentcuration/utils/publish.py +++ b/contentcuration/contentcuration/utils/publish.py @@ -46,8 +46,10 @@ from contentcuration import models as ccmodels from contentcuration.decorators import delay_user_storage_calculation +from contentcuration.utils.assessment.perseus import perseus_input_value from contentcuration.utils.assessment.perseus import PerseusExerciseGenerator from contentcuration.utils.assessment.qti.archive import QTIExerciseGenerator +from contentcuration.utils.assessment.qti.convert import accepted_answers from contentcuration.utils.assessment.qti.convert import is_answerless_input from contentcuration.utils.assessment.qti.imsmanifest import ( get_assessment_ids_from_manifest, @@ -272,6 +274,11 @@ def _node_is_perseus_derivable(node): return False elif item.type not in PERSEUS_EXPRESSIBLE_LEGACY_TYPES: return False + elif item.type == exercises.INPUT_QUESTION and not all( + perseus_input_value(answer) is not None + for answer in accepted_answers(json.loads(item.answers)) + ): + return False return has_native_qti