Skip to content

Cover non-BMP characters, and refuse lone surrogates with LiftWriteError - #36

Merged
imnasnainaec merged 3 commits into
mainfrom
tests/nonbmp-surrogate-regression
Aug 21, 2026
Merged

Cover non-BMP characters, and refuse lone surrogates with LiftWriteError#36
imnasnainaec merged 3 commits into
mainfrom
tests/nonbmp-surrogate-regression

Conversation

@imnasnainaec

@imnasnainaec imnasnainaec commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

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:

  • byte-identical round trips with non-BMP text in an attribute value, in element text, and inside a <span>
  • exact byte-region scanner boundaries across a 4-byte UTF-8 sequence — the input a scanner that guessed at character boundaries would break on
  • an untouched non-BMP entry emitted verbatim while its neighbour is edited (raw UTF-8 out, not character references)
  • streaming read and write; clean validation
  • a &#xD800; character reference and CESU-8/WTF-8 surrogate halves both rejected as LiftParseError, through the full and the streaming reader — so no lone surrogate can reach 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, 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 UnicodeEncodeError from wherever the text or attribute was set, naming no node, out of four public entry points: save(), changes(), changed_entries(), and validation.

  • New 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 a str cannot encode as UTF-8 — nothing, today — is re-raised rather than mislabelled.
  • The digest functions delegate to the canonical byte functions they duplicated, so snapshots are covered too; canonical_range_bytes and _ranges_root_open_bytes replace three copies of those bodies (including a # noqa: E731 lambda).
  • Validation renders the document before checking it, so it cannot check an unrenderable one: it reports the refusal as a single lone-surrogate error instead of propagating the encode error — addressed to the companion, and without stopping the .lift's own checks, when a .lift-ranges is 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.md gains a "Content XML cannot represent" section; docs/en/guides/validate.md cross-references it; LiftWriteError reaches the API reference through mkdocstrings.
  • CHANGELOG: folded into the existing [0.1.0] writer bullet rather than [Unreleased], since 0.1.0 has not shipped.
  • The CLI's inability to print non-BMP findings on a legacy codepage stdout is Redirected CLI output crashes on characters the locale encoding cannot represent, truncating export #32, untouched here. One coupling: reconfiguring those streams to UTF-8 is enough for non-BMP but not for a lone surrogate, which needs an explicit errors= handler.
  • python scripts/check.py green (557 passed, 97.8% coverage); mkdocs build --strict clean.

🤖 Generated with Claude Code


Devin review: https://app.devin.ai/review/sillsdev/python-sil-lift/pull/36


This change is Reviewable

imnasnainaec and others added 2 commits August 20, 2026 13:53
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 &#xD800; 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
imnasnainaec force-pushed the tests/nonbmp-surrogate-regression branch from f78e82a to b13553d Compare August 20, 2026 17:59
@imnasnainaec
imnasnainaec marked this pull request as ready for review August 21, 2026 15:17
@imnasnainaec imnasnainaec self-assigned this Aug 21, 2026
@imnasnainaec imnasnainaec added the 🟨Medium Medium-priority PR label Aug 21, 2026

@jasonleenaylor jasonleenaylor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

:lgtm:

@jasonleenaylor reviewed 10 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).

@imnasnainaec
imnasnainaec merged commit d7e2166 into main Aug 21, 2026
13 checks passed
@imnasnainaec
imnasnainaec deleted the tests/nonbmp-surrogate-regression branch August 21, 2026 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🟨Medium Medium-priority PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Verify that surrogate pairs in LIFT files don't break the tooling

2 participants