Skip to content

Compare range-element parents under NFC - #28

Merged
imnasnainaec merged 13 commits into
mainfrom
range-parent-nfc
Aug 18, 2026
Merged

Compare range-element parents under NFC#28
imnasnainaec merged 13 commits into
mainfrom
range-parent-nfc

Conversation

@imnasnainaec

@imnasnainaec imnasnainaec commented Aug 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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

Fixes #14. Fixes #27.

range-parent compared a range-element's parent against its siblings' ids as
exact strings. FLEx wrote the two with different normalization — the range-element
ids skipped its normalizing helper and came out NFD, the parent on that same
element went through it and came out NFC — so sound parent links in real exports (Sena
in #14, Sango in #27) were reported as errors. Fixed upstream since (LT-22697);
that changes nothing here, as those files are permanent and sil-lift never rewrites
ids.

What changes

  • One policy for resolving a name to an id: exact spelling first, then NFC.
    • Covers parent links, range values, and — new — the name that keys a range at all
      (a trait name, a header range id).
    • Only undefined-range-value normalized before, so one encoding split was an error
      in one check and invisible in another.
    • A range whose own id was NFD went unchecked entirely: the NFC trait names keying
      it never found it, and an unresolved name looks like a trait no range defines.
  • A match that needed normalizing is reported: new normalization-mismatch
    warning, once per id however many references differ.
    • The ids are never rewritten, so a consumer comparing raw strings still fails to
      resolve those references, and nothing else would say so.
    • Both spellings are named by code point ('Comple\u0301ments' vs
      'Compl\u00e9ments'), since they render identically.
    • range-parent stays an error for a parent no sibling defines in any normalization.
  • Range findings are addressed to the companion that defines the element, not to
    the .lift that merely references it. Header ids resolve before the dangling-href
    check, so a header id against a companion's own id is answered in memory too.
  • Each range's id lookup is built once, not per trait: Sango's semantic pass drops
    1.54s → 0.84s over the same findings.
  • Prose fixed: three sites and the changelog called the .lift NFC and the
    .lift-ranges NFD, but the normalizer ran per write, not per file.

Coverage

  • negative/nfd-range-ids.lift + .lift-ranges — NFD ids, NFC references, one parent
    dangling in every normalization: one error, one deduplicated warning, and both files
    still byte-identical on save.
  • In-memory: the range-keying name, the header id, and a range holding one name in two
    normalizations, where each reference must resolve to the sibling it matches exactly.
  • The test names are \uXXXX escapes — a tool that normalizes on save would weaken
    the assertions rather than fail them.

Effect on the corpus

before after
sango.lift range-parent 2 errors none
sango.lift normalization-mismatch — 6 warnings (82 references → 5 ids)
sango.lift undefined-range-value 1 (prenom) 1 (prenom)
all other fixtures — no new findings

Not changed

  • dangling-ref — all 7 corpus findings are ASCII and none resolve under NFC.
  • duplicate-guid — guids, ASCII by construction.

Verification

ruff check, ruff format --check, mypy, full suite (553 passed).

Upstream, all merged: sillsdev/FieldWorks#1063, sillsdev/FieldWorks#1064 (the exporter
writes, plus an escaping gap on the morph-type id), sillsdev/chorus#394.

This change is Reviewable

@imnasnainaec imnasnainaec self-assigned this Aug 12, 2026
@imnasnainaec imnasnainaec mentioned this pull request Aug 13, 2026
Open
imnasnainaec and others added 9 commits August 17, 2026 14:13
@imnasnainaec @claude
Compare a range-element's parent against sibling ids under NFC
3a7943d 
FLEx normalizes strings to NFC on export, but a few writes emit the NFD
it holds in memory -- among them the grammatical-info and lexical-relation
range-element ids, while the parent attribute on those same elements is
normalized. Comparing the two as exact strings reported a sound parent
link as dangling: both range-parent errors in the sango fixture are that
encoding difference, not defects.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec @claude
Say which FLEx writes skip normalization, not which file
a157286 
The validation notes described the .lift as NFC and the .lift-ranges as
NFD. It is narrower than that: the normalizer is applied per write, so a
range-element id can be NFD beside a label, a parent, and a .lift value
that are all NFC.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec @claude
Warn when a name resolves only after NFC normalization
eca6e8d 
Normalizing both sides of the range comparisons hid a real property of
FLEx output: an id and the names referring to it can differ in encoding,
and any consumer comparing raw strings -- a Send/Receive merge, say --
fails to resolve them. Report that as a warning, once per range-element
id however many references differ (6 rather than 82 in the sango
fixture), naming both spellings by code point because they render
identically, and addressing it to the file the id lives in.

The new negative pair carries the shape end to end: NFD ids, NFC parents
and grammatical-info value, one parent that dangles in every
normalization, and bytes that must survive the round trip unchanged --
normalization belongs to the comparison, not to what gets written.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec @claude
Resolve a range name by its exact spelling before its NFC form
056e1c5 
Keying each range's lookup by both the ids as written and their NFC forms,
with an exact id mapping to itself, makes a reference that matches a
sibling exactly stay an exact match. Before, the first spelling of an
NFC-equal pair won, so a range carrying one name in both normalizations --
distinct ids as strings, and FLEx normalizes some writes and not others --
would resolve the second element's references to the first and report them
as mismatched.

The lookup is now built once per range and read by the value checks, which
were rebuilding it per trait: thousands of traits over a few ranges of
hundreds of elements each. The sango fixture's semantic pass drops from
1.54s to 0.84s.

The module docstring's semantic checks fed the API reference as one
sentence running nine checks and their parentheticals together. They are a
bullet each now, named by the Problem code the check reports, matching how
layer 1 lays out its deviations; the comments around the normalization
helpers drop the mechanism they were re-telling from that list.

Also escapes the list markers in the new PROVENANCE sentence, per that
file's convention for hard-wrapped prose, and drops a Markdown escape that
had no business in an XML comment.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec @claude
Address a range finding to the companion that defines the range
6acaf71 
The file the mismatch warning reports against is worked out for that
warning alone, three lines under a range-parent error about the very same
range-element that still reports against the .lift. Both belong to the
document the element lives in, so the lookup moves above the check that
walks the ranges and answers for both.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec @claude
Resolve the name keying a range under NFC too
b7fa496 
The comparison of a name against a range-element id was normalized while
the comparison that reaches the range in the first place -- a trait name,
a header range id -- stayed exact. A range whose id FLEx wrote in NFD is
therefore never found by the NFC trait names keyed on it, and its values
go unchecked: an unresolved name is indistinguishable from a trait no
range defines, which is silently accepted, so the whole range drops out
of validation with nothing to show for it.

Both lookups are now built by one helper over a collection of ids, and a
range reached only by normalizing is a normalization-mismatch warning
like an element is. The value checks name their range through that
resolution, which also drops the allowed-ids set from the per-check
tuples: they carry the resolved range id, and the lookup kept for it
answers by that key.

The validate guide's normalization entry, five sentences beside four
one-liners once this landed, keeps the mechanism on the bullet and gives
the rest a sub-bullet each. It moves last as well, so the list runs
schema deviations first and the semantic check after them, as the layers
above are ordered.

The Sango notes counted its 6 warnings as 6 ids. They are 5: FLEx aliases
the part-of-speech list under two range ids, and the id under both is
warned about under each, since each range defines its own element.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec @claude
Settle a header range id's spelling outside the file check
7a3a319 
Resolving the header's range id against the companion's own id was done
inside the check for a dangling href, which runs only for a lexicon read
from disk. Whether those two spellings agree is a property of the
document, so a lexicon held in memory reported nothing where the same
document on disk warned -- and validation is supposed to answer for what
save() would write, wherever the document came from.

Every header range id resolves before that check now, and the check reads
the answer. Ranges that carry their own elements resolve to themselves and
record nothing, so covering them all costs a lookup each.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec
Add order-justifying comment
139c4cf
@imnasnainaec @claude
Pin the mismatch dedup to the id, not the reference spelling
5c560f0 
The existing coverage for reporting a normalization mismatch once per id
uses references that are all spelled the same way, so it pins only the
collapse of identical references. Add a case where two references differ
from the id and from each other -- a parent link in canonical mark order
and a grammatical-info value with the same marks reversed -- and assert
one warning. Which spelling the message names is deliberately left
unasserted, so the order references are reached in stays free.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 17, 2026
Open
Open
imnasnainaec and others added 2 commits August 17, 2026 17:08
@imnasnainaec @claude
Tabulate the problem codes in the validate guide
c102936 
The roster of Problem codes was stated in three places -- the changelog's
validation bullet, the module docstring, and the guide's layers list -- and
none of them gave a code's level, though --strict promotes every warning to
an error and validate's exit code is a supported interface.

The guide gains a Problem codes section: all eleven codes, alphabetical,
each with its level and what it flags, with the two that come from the
schema layers named in the intro rather than in a column that would read
"semantic" for nine of eleven rows and could not hold one value for schema.
The layers list goes back to describing the three passes, and the module
docstring points at the table instead of repeating it, leaving Problem.code
as the one place the names are listed in the source.

The changelog's validation bullet keeps only the codes whose behavior a
reader would not guess -- missing-id because it is opt-in, and
normalization-mismatch because resolving names under NFC is surprising --
and names the two RELAX NG departures by what a user sees rather than by how
they are implemented. It goes from 18 lines to 10, in range with the bullets
around it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec @claude
State the Problem address per field rather than per code
e9e747a 
The addressing paragraph named range-parent, normalization-mismatch and
dangling-ranges-href as the findings that carry a file and nothing else,
which sorted every code into one of two buckets. Four cases fall outside
that split: a companion schema error has no entry and sometimes no line;
duplicate-guid raised for a range-element carries a guid without an entry;
missing-media names an entry but has no line; and file itself is None
whenever the lexicon has no path. duplicate-guid and schema also land in
either bucket depending on where the finding is, so extending the list
would mean naming most of the codes and splitting two of them by context.

Describe instead when each of file, entry_id, guid, and line is set, and
keep the companion addressing as its own sentence. Nothing enumerated, so
nothing to revisit as codes are added.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec
imnasnainaec marked this pull request as ready for review August 17, 2026 21:49
imnasnainaec and others added 2 commits August 18, 2026 15:07
@imnasnainaec @claude
Describe FLEx's NFD range ids as past exporter behavior
e3ab30b 
The writes that emitted NFD range-element ids have been fixed upstream
(LT-22697), so the docs, the fixture comments, and the validator's own
comment now present the split as something existing exports carry rather
than something FLEx still does. The NFC comparison and the
normalization-mismatch warning are unchanged: sil-lift never rewrites
ids, and the Sango pair is a pre-fix export.

The warning's description also loses a Send/Receive example that no
longer holds, and PROVENANCE.md pins the asymmetry to the FLEx version
that wrote the Sango pair.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec @claude
Write the normalization test names as code-point escapes
171a042 
The remaining non-ASCII literals in the validation tests are spelled
\uXXXX, so no test depends on an editor or a tool preserving a
composition it cannot show on screen. The third spelling in the dedup
test is the one that mattered: it is dot-above before dot-below, which
canonical ordering would swap, making it equal to the parent link and
quietly retiring the two-differing-references case rather than failing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imnasnainaec imnasnainaec added the 🟨Medium Medium-priority PR label Aug 18, 2026
jasonleenaylor
jasonleenaylor approved these changes Aug 18, 2026
View reviewed changes

@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 9 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 1d55858 into main Aug 18, 2026
13 checks passed
@imnasnainaec
imnasnainaec deleted the range-parent-nfc branch August 18, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasonleenaylor jasonleenaylor jasonleenaylor approved these changes

Assignees

@imnasnainaec imnasnainaec

Labels

🟨Medium Medium-priority PR

Projects

None yet

Milestone

No milestone

2 participants

@imnasnainaec @jasonleenaylor