Parse QTI items to rewrite them at publish instead of matching regexes by rtibblesbot · Pull Request #6241 · learningequality/studio

rtibblesbot · 2026-09-30T18:54:53Z

Publish-time QTI item rewrites in utils/assessment/qti/media.py now parse the item with lxml and edit the tree, replacing the regex text substitutions.

set_qti_item_language sets xml:lang on the real root, past prolog comments ([QTI] A QTI item with a comment quoting its root tag is published without the node's language #6235) and on a namespace-prefixed root (Published QTI item with a namespace-prefixed root gets no xml:lang #6236).
rewrite_qti_media_paths remaps a src written as an entity or character reference ([QTI] A QTI item with an entity in a media src attribute is published with an unrewritten path #6240).
rewrite_qti_sized_image_paths and strip_studio_attributes move to the same parse/serialize path.
Published items are lxml's serialization, not the authored bytes; byte-for-byte tests now compare canonical XML.
Malformed XML now raises; publish validates items against the schema before these run.
Rewrites and legacy conversion parse with the validator's secure_parser, so publish reads an item exactly as validation did.

🤖 Generated with Claude Code

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran pre-flight CI checks (lint, format, tests) and verified all pass
Rebased onto the target branch and resolved any conflicts
Reorganized commit history into clean, logical commits
Audited the diff to ensure only issue-relevant files are changed
Built PR body from the repository's PR template with evidence blocks

	⚪ Updating PR Last updated: 2026-10-07 13:35 UTC

rtibbles

One question about continuing with a regex approach here.

rtibbles · 2026-10-02T16:28:56Z

 STUDIO_ATTRIBUTE_OR_QUOTED_VALUE_REGEX = re.compile(
    r"""(?P<studio>\s+data-studio-[\w.-]+\s*=\s*(?:"[^"]*"|'[^']*'))|"[^"]*"|'[^']*'"""
 )
+_WHITESPACE_REGEX = re.compile(r"\s*")


Would it be better to just parse the XML at this point? Creating more and more sophisticated regex for all of these XML parsing operations just adds brittleness - how much does this really save us performance wise?

Agreed — switched to parsing in 3c453e2. The savings were negligible: ~80µs per item for all three rewrites, each parsing separately.

Searched utils/assessment/ for regexes and string ops that read XML structure; 4 places matched in qti/media.py, all now parse with lxml: set_qti_item_language, strip_studio_attributes, rewrite_qti_media_paths, rewrite_qti_sized_image_paths.

Regexes left only match attribute values (checksum filename, pixel length, srcset entries).

Output is now re-serialized, so the author's formatting isn't kept byte-for-byte; tests assert on the parsed tree instead.

rtibblesbot · 2026-10-03T03:19:08Z

Outside this PR's scope — tracked in:

[QTI] A QTI item declaring a non-UTF-8 encoding is published with garbled text #6283 [QTI] A QTI item declaring a non-UTF-8 encoding is published with garbled text

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran pre-flight CI checks (lint, format, tests) and verified all pass
Rebased onto the target branch and resolved any conflicts
Reorganized commit history into clean, logical commits
Audited the diff to ensure only issue-relevant files are changed
Built PR body from the repository's PR template with evidence blocks

rtibbles

Everything makes sense, just one question about parser consistency.

rtibbles · 2026-10-04T23:53:08Z

    return int(width), int(height)


+def _parse(raw_data):


Why aren't we reusing the secure parser here?

studio/contentcuration/contentcuration/utils/assessment/qti/validation.py

Line 42 in ec6eb52

def secure_parser() -> etree.XMLParser:

Feels like it would be better for all of our parsing configuration to be identical?

Agreed — _parse now calls parse_qti_xml (so secure_parser) in 7783b83.

Searched non-test code for XMLParser, etree.fromstring, etree.parse: 2 parsed with their own config — media._parse and convert._adapt_unsupported_markup (default parser). Both now use secure_parser; the rest already did.

Publish now reads an item exactly as validation does: a non-UTF-8 encoding declaration is honoured again, so the UTF-8 reading this branch had added is dropped and stays with [QTI] A QTI item declaring a non-UTF-8 encoding is published with garbled text #6283.

New test: an item that validates only under its declared encoding publishes.

rtibblesbot · 2026-10-05T14:22:08Z

Outside this PR's scope — tracked in:

[QTI] A legacy input question with a control character in an answer is published as malformed XML #6286 [QTI] A legacy input question with a control character in an answer is published as malformed XML

Written by rtibblesbot, an LLM-based coding agent.

rtibbles

Just one more question to make sure I'm understanding.

rtibbles · 2026-10-05T17:35:19Z



+def _parse(raw_data):
+    return parse_qti_xml(raw_data.encode("utf-8"))


If the encoding step is important, shouldn't we add that to the parse_qti_xml function too? Or is it only needed here?

The step only turns text into bytes — lxml refuses a str carrying an encoding declaration — so yes, it belongs in parse_qti_xml. Moved there in 6a9d16a; parse_qti_xml now takes str or bytes.

Searched non-test code for .encode("utf-8") ahead of parse_qti_xml/validate_qti_item: 5 matches — media._parse, media.get_qti_media_references, ingest.find_perseus_custom_interaction_path, perseus_derive._parse, archive._create_native_qti_item. All now pass the text; media._parse is deleted.

validate_qti_item dropped its own copy too.

…egexes - set_qti_item_language finds the real root past prolog comments (learningequality#6235) and on a namespace-prefixed root (learningequality#6236). - rewrite_qti_media_paths rewrites a src written as an entity or character reference (learningequality#6240). - Items are parsed with the validator's parser, so a validated item publishes. - Published items are lxml's serialization, not the authored bytes; canonical XML is unchanged for valid items. - A converted item that is not well-formed XML is logged and left out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

rtibbles

All concerns addressed, good to go.

rtibblesbot · 2026-10-07T22:26:16Z

Quick check on rtibblesbot's work on this PR: react 👍 helpful · 😕 mixed · 👎 not helpful. Reply with anything specific.

learningequality#6241 made publish reserialize QTI item XML through lxml, so restored raw_data and republished packages no longer match the source byte for byte. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Restore now rewrites media paths through lxml (learningequality#6241), which changes the XML declaration and empty-element spelling. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

rtibblesbot marked this pull request as ready for review September 30, 2026 19:06

rtibblesbot requested review from a team and rtibbles September 30, 2026 19:06

rtibblesbot force-pushed the issue-6235-136df6 branch from 805f224 to 41fc509 Compare September 30, 2026 19:13

rtibbles self-assigned this Sep 30, 2026

rtibblesbot force-pushed the issue-6235-136df6 branch 3 times, most recently from ffdcad0 to 8a71d91 Compare October 1, 2026 15:27

rtibbles reviewed Oct 2, 2026

View reviewed changes

rtibblesbot force-pushed the issue-6235-136df6 branch 3 times, most recently from 20318f2 to 3c453e2 Compare October 2, 2026 23:27

rtibblesbot force-pushed the issue-6235-136df6 branch from 3c453e2 to 40c82a3 Compare October 3, 2026 03:54

rtibblesbot changed the title ~~Set the QTI item language on the real root past prolog comments~~ Parse QTI items to rewrite them at publish instead of matching regexes Oct 3, 2026

rtibblesbot force-pushed the issue-6235-136df6 branch from 40c82a3 to d9352ec Compare October 3, 2026 04:59

rtibbles reviewed Oct 4, 2026

View reviewed changes

rtibblesbot mentioned this pull request Oct 5, 2026

[QTI] A legacy input question with a control character in an answer is published as malformed XML #6286

Open

rtibbles reviewed Oct 5, 2026

View reviewed changes

rtibblesbot and others added 3 commits October 7, 2026 05:33

fix: parse converted legacy markup with the validator's secure parser

65c30fd

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

refactor: let parse_qti_xml take text and encode it itself

6a9d16a

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

rtibblesbot force-pushed the issue-6235-136df6 branch from 7783b83 to 6a9d16a Compare October 7, 2026 07:01

rtibbles approved these changes Oct 7, 2026

View reviewed changes

rtibbles merged commit 9a5848f into learningequality:unstable Oct 7, 2026
13 checks passed

rtibblesbot deleted the issue-6235-136df6 branch October 7, 2026 22:26

rtibblesbot mentioned this pull request Oct 8, 2026

[QTI] Perseus archive of an exercise with raw Perseus questions keeps legacy input questions with no answer #6210

Open

2 tasks

This was referenced Oct 8, 2026

Leave answerless legacy input questions out of Perseus archives #6262

Open

Restoring a published QTI item stores lxml's reserialization instead of the published XML #6305

Open



		def _parse(raw_data):
		return parse_qti_xml(raw_data.encode("utf-8"))

Conversation

rtibblesbot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚪ Updating PR

Uh oh!

rtibbles left a comment

Choose a reason for hiding this comment

Uh oh!

rtibbles Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot commented Oct 3, 2026

Uh oh!

rtibbles left a comment

Choose a reason for hiding this comment

Uh oh!

rtibbles Oct 4, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot commented Oct 5, 2026

Uh oh!

rtibbles left a comment

Choose a reason for hiding this comment

Uh oh!

rtibbles Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

rtibbles left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rtibblesbot commented Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rtibblesbot commented Sep 30, 2026 •

edited

Loading