Publish mixed Perseus + native QTI exercise nodes as one QTI package - #6047
Conversation
8bc0120 to
056fe67
Compare
rtibbles
left a comment
There was a problem hiding this comment.
Mostly looking good, some questions of things I'm less sure about.
| 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_node_package_lists_both_question_kinds(self): |
There was a problem hiding this comment.
This test seems repetitive of the exercise creation tests?
There was a problem hiding this comment.
Agreed — removed test_mixed_node_package_lists_both_question_kinds.
Its assertions (both item kinds present, Perseus JSON packaged, qti-custom-interaction with data-type="perseus") duplicate the generator-level test_perseus_custom_interaction_embedded_with_native_qti in tests/utils/test_exercise_creation.py, which covers the same package contents more thoroughly. The mixed-node publish path stays covered here by test_mixed_perseus_and_native_qti_routes_to_qti (routing) and test_mixed_qti_file_included_presets (the qti | exercise bit). Done in 3f7a67f.
| if ( | ||
| included_presets is not None | ||
| and preset.pk == format_presets.QTI_ZIP | ||
| and ccnode.assessment_items.filter(type=exercises.PERSEUS_QUESTION).exists() |
There was a problem hiding this comment.
Does this really need an additional query? Also, could there be an edge case where this is true, but the question itself did not publish, wasn't complete? etc?
Wouldn't it be easier just to set this as discovered during the generation process, so that we don't have to look for it again, and we ground it in what was actually generated?
There was a problem hiding this comment.
Good call on both counts. Dropped the extra assessment_items.filter(type=PERSEUS_QUESTION).exists() query.
The mixed-node decision is already made during routing in recurse_nodes as any_perseus_question and any_native_qti — the exact condition that sends the node to QTIExerciseGenerator and embeds the Perseus custom interactions. I now capture it there as qti_embeds_perseus and thread it into create_associated_file_objects, so the included_presets |= exercise bit is grounded in the same decision that drives the embedding rather than re-derived.
That also closes the edge case you raised: the flag and the packaged contents now come from one source and cannot diverge. Done in b47f996.
3f7a67f to
70624f2
Compare
rtibbles
left a comment
There was a problem hiding this comment.
This isn't going to quite work as is - we need to properly handle the response, and also do outcome processing, so that this gets handled completely as a QTI question.
| response_declaration=[ | ||
| ResponseDeclaration( | ||
| identifier="RESPONSE", | ||
| cardinality=Cardinality.SINGLE, |
There was a problem hiding this comment.
The perseus renderer returns an object for its response, of this shape:
{
correct,
answerState,
simpleAnswer,
}
So, the response declaration will have to be of record cardinality, I think? correct is a boolean, simpleAnswer is a string. Unfortunately answerState is an arbitrary Javascript object, so I think we'll have to store it as a stringified JSON.
The outcome declaration will therefore be checking the correct property in order to mark correct/incorrect.
There was a problem hiding this comment.
Done in 232b534. RESPONSE is now declared with record cardinality, and grading reads its correct field. On the field types: QTI record declarations do not enumerate their fields (records are open, each field carries its own base-type at runtime), so the correct (boolean) / simpleAnswer (string) / answerState (stringified JSON) shape is documented as the renderer contract in the builder docstring rather than declared. The item only needs to read correct to grade, which response processing now does.
| """ | ||
| identifier = hex_to_qti_id(assessment_id) | ||
|
|
||
| item = AssessmentItem( |
There was a problem hiding this comment.
As per below, this AssessmentItem needs an outcome declaration and a response processing template associated with it (which can be the default correct/incorrect template - we just need the special outcome declaration for checking the correct property.
There was a problem hiding this comment.
Done in 232b534. The item now has a SCORE outcome declaration plus response processing that grades correct/incorrect.
One deviation worth flagging: I could not use the standard match_correct template. That template compares RESPONSE to a declared correctResponse, and there is no standard response-processing template that can inspect a single field of a record. So instead of a template=... reference I emit the equivalent grading inline:
<qti-response-processing>
<qti-response-condition>
<qti-response-if>
<qti-field-value field-identifier="correct">
<qti-variable identifier="RESPONSE"/>
</qti-field-value>
<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-else>
<qti-set-outcome-value identifier="SCORE">
<qti-base-value base-type="float">0</qti-base-value>
</qti-set-outcome-value>
</qti-response-else>
</qti-response-condition>
</qti-response-processing>Same correct/incorrect (1/0) semantics as the default template, just reading the record's correct field. This required adding the minimal QTI rule/expression models (qti-response-condition, qti-response-if/else, qti-field-value, qti-variable, qti-set-outcome-value, qti-base-value). The full item validates against the QTI 3.0 XSD. Happy to switch to a template reference instead if you would rather the host own this grading step.
rtibbles
left a comment
There was a problem hiding this comment.
Just one small bit of cleanup, reversing something I said in the last review round. Let's also rationalize the commit history to be about where we have got to, not how we got here.
| The host's Perseus renderer owns rendering and grading, but the result is | ||
| handled as a complete QTI question. The renderer reports its outcome through | ||
| the ``RESPONSE`` variable as a record with fields ``correct`` (boolean), | ||
| ``simpleAnswer`` (string) and ``answerState`` (an arbitrary object stored as |
There was a problem hiding this comment.
Actually, because we don't need to specify the schema for the record type, there's no need for answerState to be stringified - it can just be the plain object.
There was a problem hiding this comment.
Done in 94f1a46. The docstring no longer describes answerState as stringified JSON — RESPONSE is a record declared with no schema, so each field carries its own base-type at runtime and answerState is just the plain object. Nothing in the builder ever stringified it (the item only reads correct to grade), so this is a documentation-only change.
Also rationalized the branch history as requested: the iteration commits (grade-completely, ground-the-exercise-bit, drop-redundant-test) are folded into four commits describing the final state rather than how it got there.
232b534 to
93ef97a
Compare
Introduce a `CustomInteraction` element (`qti-custom-interaction`) and `build_perseus_custom_interaction_item`, which wraps a raw Perseus question in a schema-valid `qti-assessment-item` marked `data-type="perseus"`. The Perseus renderer owns rendering and grading, reporting its result through the RESPONSE variable as a record (fields: `correct` boolean, `simpleAnswer` string, `answerState` object). RESPONSE is declared with record cardinality and no schema, so each field carries its own base-type at runtime and `answerState` need not be stringified. A SCORE outcome plus inline response processing reads the record's `correct` field and sets SCORE to 1/0, so the item grades as a complete QTI question. The response- processing rule/expression models this needs are added alongside, and ResponseDeclaration's base-type is made optional for record cardinality. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the raw-Perseus image/graphie writing out of PerseusExerciseGenerator into ExerciseArchiveGenerator._write_raw_perseus_assets, parameterized on the target images directory and returning the package-relative paths it wrote, so the QTI generator can reuse it. Perseus output is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Teach QTIExerciseGenerator to intercept PERSEUS_QUESTION items in process_assessment_item and emit each as a qti-custom-interaction: write the Perseus JSON (content-storage refs rewritten to the packaged asset paths) and its image/graphie assets into the package, and declare them as dependencies of the wrapper item's manifest resource. Removes the old ValueError rejection of Perseus questions in the QTI format. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Route a node holding both raw Perseus and native QTI questions to QTIExerciseGenerator alone (one QTI package, no separate Perseus archive), capturing the mixed fact as qti_embeds_perseus during generator selection. Thread that flag into create_associated_file_objects to OR the exercise bit into the QTI file's included_presets (qti | exercise), so the flag is grounded in the same routing decision that embeds the Perseus custom interactions and cannot diverge from the packaged contents. Tests cover the routing, the included_presets bit, and the end-to-end mixed package contents. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
93ef97a to
203e25e
Compare
rtibbles
left a comment
There was a problem hiding this comment.
Changes make sense, and the Kolibri side issue has been updated with the additional details here, so we should have a secure contract.
Summary
qti-custom-interaction data-type="perseus"(Perseus JSON + assets declared in the manifest) that Kolibri's Perseus renderer owns.RESPONSErecord variable holds the renderer's outcome and inline response processing maps itscorrectfield to a 0/1SCORE(expressed inline because no standard response-processing template can read a record field).QTI_ZIPfile setsincluded_presets = qti | exerciseso older Kolibri also loads the Perseus renderer.References
Closes #6006. Depends on #6004 (
File.included_presets, landed onunstablevia #6046). Renderer counterpart: learningequality/kolibri#14892.Reviewer guidance
utils/publish.pyselection — confirm a Perseus-only node still routes toPerseusExerciseGenerator.utils/publish.pyselection — confirm a native-QTI-only node's routing is unchanged.utils/publish.py— confirm theqti_embeds_perseusflag (raw Perseus and native QTI in the same node) is derived from the sameassessment_mappingthat selects the generator, and thatcreate_associated_file_objectssets theexercisebit only on theQTI_ZIPfile when that flag is set.utils/assessment/qti/archive.py_create_perseus_custom_interaction— check the{☣ LOCALPATH}/perseus/images/...rewrite matches how the Kolibri renderer resolvesdata-perseus-path-relative assets.utils/assessment/qti/convert.pybuild_perseus_custom_interaction_item— confirm theRESPONSErecord and inline response processing (correctfield → 0/1SCORE) match the record fields the renderer reports back.AI usage
Used Claude Code to implement the change against a pre-approved plan with test-driven development. Verified with the affected pytest suites (
test_convert.py,test_exercise_creation.py,test_exportchannel.py), amakemigrations --check, and pre-commit lint.@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
🟡 Waiting for feedback
Last updated: 2026-07-18 17:27 UTC