Cover non-BMP characters, and refuse lone surrogates with LiftWriteError - #36
Merged
Merged
Conversation
A non-BMP codepoint is one UTF-16 surrogate pair in a UTF-16 source and one 4-byte UTF-8 sequence on disk — exactly the input a byte-region scanner that guessed at character boundaries would break on. Nothing pinned that down. tests/test_unicode.py adds the deterministic cases: byte-identical round trips with non-BMP text in an attribute value, in element text, and inside a <span>; exact scanner regions across a 4-byte sequence; an untouched non-BMP entry emitted verbatim while its neighbour is edited; streaming read and write; clean validation. On the refusal side, a numeric character reference to a lone surrogate and CESU-8/WTF-8 surrogate halves both raise LiftParseError through the full and the streaming reader, so no lone surrogate reaches the model from a file. A UTF-16 source carrying non-BMP content loads, re-serializes canonically as UTF-8 (the documented byte-identity exception for a non-ASCII-compatible encoding), and is byte-stable from there on. The property suite drew non-BMP codepoints from st.characters() too rarely to count as coverage, so text and attribute-value alphabets now sample them explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A Python string can hold a lone surrogate (U+D800-U+DFFF); an XML document cannot, in any encoding. No LIFT file can introduce one — the parser rejects both a � character reference and CESU-8/WTF-8 bytes — so it only ever arrives as a string assigned through the API, and then lxml raised a bare UnicodeEncodeError from wherever the text or attribute was set, naming no node, out of four different public entry points: save(), changes(), changed_entries(), and validation. Every writer path that builds and serializes a node now goes through _guarded(), which reports it as LiftWriteError naming the node and the codepoint. Anything else a str cannot encode as UTF-8 — nothing, today — is re-raised untouched rather than mislabelled. The digest functions delegate to the canonical byte functions they duplicated, so the guard covers snapshots too, and canonical_range_bytes / _ranges_root_open_bytes replace the copies of those two bodies. Validation renders the document before checking it, so it cannot check an unrenderable one: it now reports the refusal as a single lone-surrogate error (addressed to the companion, and without stopping the rest, when a .lift-ranges is the unwritable part) instead of propagating the encode error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
imnasnainaec
force-pushed
the
tests/nonbmp-surrogate-regression
branch
from
August 20, 2026 17:59
f78e82a to
b13553d
Compare
imnasnainaec
marked this pull request as ready for review
August 21, 2026 15:17
jasonleenaylor
approved these changes
Aug 21, 2026
jasonleenaylor
left a comment
There was a problem hiding this comment.
@jasonleenaylor reviewed 10 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #21.
Regression coverage (the gap #21 identified)
Non-BMP characters and UTF-16 sources already behaved correctly; nothing pinned it down. New
tests/test_unicode.py:<span>�character reference and CESU-8/WTF-8 surrogate halves both rejected asLiftParseError, through the full and the streaming reader — so no lone surrogate can reach the model from a fileThe property suite drew non-BMP codepoints from
st.characters()too rarely to count as coverage, so text and attribute-value alphabets now sample them explicitly, across five planes.Lone surrogates through the API
A Python string can hold a lone surrogate (U+D800-U+DFFF); an XML document cannot, in any encoding. Since no file can introduce one, it only ever arrives as a string assigned through the API — and then lxml raised a bare
UnicodeEncodeErrorfrom wherever the text or attribute was set, naming no node, out of four public entry points:save(),changes(),changed_entries(), and validation.LiftWriteError. Every writer path that builds and serializes a node passes through_guarded(), which names the node and the codepoint:entry 'abat': U+D800 is a lone surrogate, which XML cannot represent in any encoding (in 'a\ud800b'). Nothing is written; the model is untouched. Anything else astrcannot encode as UTF-8 — nothing, today — is re-raised rather than mislabelled.canonical_range_bytesand_ranges_root_open_bytesreplace three copies of those bodies (including a# noqa: E731lambda).lone-surrogateerror instead of propagating the encode error — addressed to the companion, and without stopping the.lift's own checks, when a.lift-rangesis the unwritable part.Not built: substituting or dropping the surrogate on save (silently rewriting lexical data), and rejecting at assignment time (the model is plain slotted dataclasses, so partial coverage would be a promise the library could not keep).
Notes
docs/en/fidelity.mdgains a "Content XML cannot represent" section;docs/en/guides/validate.mdcross-references it;LiftWriteErrorreaches the API reference through mkdocstrings.[0.1.0]writer bullet rather than[Unreleased], since 0.1.0 has not shipped.errors=handler.python scripts/check.pygreen (557 passed, 97.8% coverage);mkdocs build --strictclean.🤖 Generated with Claude Code
Devin review: https://app.devin.ai/review/sillsdev/python-sil-lift/pull/36
This change is