Skip to content

Parse QTI items to rewrite them at publish instead of matching regexes - #6241

Open
rtibblesbot wants to merge 1 commit into
learningequality:unstablefrom
rtibblesbot:issue-6235-136df6
Open

rtibblesbot wants to merge 1 commit into
learningequality:unstablefrom
rtibblesbot:issue-6235-136df6

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Closes #6235
Refs #6236
Refs #6240

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.

🤖 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
@rtibblesbot

🟡 Waiting for feedback

Last updated: 2026-10-03 05:07 UTC

@rtibblesbot
rtibblesbot marked this pull request as ready for review September 30, 2026 19:06
@rtibblesbot
rtibblesbot requested review from a team and rtibbles September 30, 2026 19:06
@rtibbles rtibbles self-assigned this Sep 30, 2026
@rtibblesbot
rtibblesbot force-pushed the issue-6235-136df6 branch 3 times, most recently from ffdcad0 to 8a71d91 Compare October 1, 2026 15:27

@rtibbles rtibbles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about continuing with a regex approach here.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
rtibblesbot force-pushed the issue-6235-136df6 branch 3 times, most recently from 20318f2 to 3c453e2 Compare October 2, 2026 23:27
@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Outside this PR's scope — tracked in:


@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

@rtibblesbot 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
…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).
- Published items are lxml's serialization, not the authored bytes;
  canonical XML is unchanged for valid items. Invalid XML now raises.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[QTI] A QTI item with a comment quoting its root tag is published without the node's language

2 participants