LT-22697: Normalize the range-element ids written to LIFT ranges

[imnasnainaec]

Fixes LT-22697.

Downstream analysis, byte-level evidence, and the full list of affected writes: python-sil-lift#27.

Companion to #1064. The two diffs do not overlap.

Quick Summary

FieldWorks holds strings as NFD and normalizes to NFC on LIFT export, but several writes in
the .lift-ranges output bypass MakeSafeAndNormalizedAttribute and emit the NFD unchanged.
Two defects fall out of that:

1. One name, two encodings, inside a single element. In WritePartOfSpeechRangeElement
   the range-element @id is written with XmlUtils.MakeSafeXmlAttribute while the @parent
   of that same element, two lines below, is normalized. A consumer that resolves a range
   value by raw string comparison then fails to match: the .lift's NFC
   <grammatical-info value="…"/> reads as undefined against the companion's NFD @id, and a
   @parent naming an NFD sibling id looks dangling.
2. The morph-type id had no XML escaping at all. WriteMorphTypeRange wrote the name
   straight into the attribute, so a morph type renamed to something containing & or <
   produces a .lift-ranges no conforming parser can read.

Routing the affected writes through the existing normalizing helper fixes both — the same
helper escapes as well as normalizes.

Observed in real exports from SIL.FLEx 8.3.12.43172 and SIL.FLEx 9.1.15.658. In the
9.1.15 file, 12 range-element ids (7 distinct) are NFD while every label, abbrev,
description, @parent, and .lift value naming them is NFC.

The change

Six writes swap XmlUtils.MakeSafeXmlAttribute — or, for the morph-type id, nothing at all —
for MakeSafeAndNormalizedAttribute:

• WritePartOfSpeechRangeElement — the range-element @id, in both branches
• WriteLexRefType — the lexical-relation range-element @id, in both branches
• WriteMorphTypeRange — the morph-type range-element @id
• WriteMorphTypeRange — the leading-symbol and trailing-symbol traits
• WriteStemNameRanges — the stem-name feature-set trait

Scope

Deliberately left alone. Two writes in the ranges output are writing-system tags, where
normalizing is a no-op: the reversal-type id, and the feature writing-system trait.

The three trait hunks are not equal. Two are cosmetic, one is not:

• leading-symbol and trailing-symbol are genuine no-ops. For ordinary affix markers
  (-, =, ~) NFC changes nothing, and even a combining-mark marker is unchanged, since a
  mark with no base has nothing to compose with. They are in the diff purely for consistency
  and can be dropped if a tighter diff is preferred.
• The stem-name feature-set is a real change. Its value is region.LiftName, a composed
  feature-structure name built from user-defined feature and value abbreviations, which can
  carry diacritics.
  • Normalizing it is correct — the .lift side already normalizes LiftName.
  • But Chorus keys <trait> on name+value, so a respelled value is a delete-plus-add
    there too.
  • The trait has no stable identity attribute to key on instead, so chorus#394 (below) does
    not help it. The damage is smaller because a trait is a leaf that both sides regenerate.
  • I would keep this hunk.

Not addressed here. The possibility maps are NFC by convention, but nothing enforces it:

• TryGetPossibilityMatchingTrait looks up with the raw value.
• FindOrCreatePartOfSpeech inserts with dict.Add(val, pos).
• The ProcessX range-element handlers insert the LIFT id verbatim.

For FLEx-produced files this is invisible, since both sides are NFC; a LIFT file written in
NFD by another exporter can create duplicate list items. An equality comparer on those
dictionaries would enforce the documented convention in one place. Separate defect, separate
review — glad to open it if that is wanted.

Effects worth knowing

FLEx's own importer needs no change; the ids move toward the convention it already uses.

• AddToPossibilityMap keys every possibility map with tss.Text.Normalize(), i.e. NFC, and
  the map declarations in LiftMergerRanges.cs state that outright.
• ProcessPartOfSpeech seeded an NFD key from the ranges @id alongside those NFC keys; now
  the id agrees with them.
• So there are no duplicate list items on the first re-import after upgrading, and no
  import-side change is needed for this one.

LIFT Send/Receive sees a delete plus an add, not an attribute edit. .lift-ranges is
under S/R and gets a 3-way XML merge. Chorus matches range-element across revisions by its
@id, not its @guid
(LiftRangesElementStrategiesMethod.cs),
comparing ordinally, and normalizes nothing anywhere. Three cases follow:

• Nobody else touched that range-element — harmless. The old element is dropped silently
  and the new one lands as an unmatched addition, with no conflict. range-element is
  registered order-irrelevant, so no reordering conflict either, and LiftSorter sorts
  range-elements under StringComparer.InvariantCultureIgnoreCase, which is
  canonical-equivalence-aware, so the element does not move in the file. The hg diff really is
  one attribute on one line.
• The other user edited the same range-element in the same sync — not harmless. Chorus
  raises a RemovedVsEditedElementConflict and keeps their element while ours is still added,
  leaving two <range-element> with the same @guid, ids differing only in the
  normalization, plus a conflict note nobody earned. This degrades rather than corrupts:
  FLEx's import is guid-first (FindExistingPossibility →
  GetPossibilityForGuidIfExisting), so both resolve to the same POS and the second is a
  no-op, and the next export regenerates the file from the model.
• A mixed-version team — repeats every sync. The id is derived from the in-memory name at
  export; it is not stored. A client that has not upgraded re-exports NFD on every sync, so
  between an upgraded and an un-upgraded client this flip-flops per sync — each flip a
  delete-plus-add that can land in the case above — until every client on the repo is
  upgraded. Same for any non-FLEx writer of the ranges file sharing the repo.

This wants a release note telling teams to upgrade together. The change still converges
and is still the right target, but that third case is not a one-time diff.

The durable fix is in Chorus, and is now proposed.
sillsdev/chorus#394 prefers the guid for
matching range-element, with id fallback, which makes ranges merges immune to any id
respelling — normalization, rename, recapitalization — and collapses the duplicated pair in
repos that already have one. This PR does not depend on it, but they are better together.

A third-party tool that stored the old NFD spelling as its own key will see the new
spelling as a new value once. NFC is the right target — it is what the rest of the export
already uses — so this is a one-time correction, not an ongoing incompatibility.

A residual hazard, for the record

Chorus's RemoveAmbiguousChildren drops all but the first sibling its finder cannot tell
apart, emitting only a merge warning. NFC is injective over NFD-normalized input and FLEx
stores NFD, so this should not newly collide. But a project holding two possibilities whose
names differ only by normalization — plausible by way of a non-FLEx LIFT import — would after
this change collapse to one id, and one element would be dropped from the merge output. Not a
reason to hold this PR; worth knowing that is the failure mode.

Testing

Two regression tests in LiftExportTests.cs, each covering one of the two defects. Neither
existed before.

1. LiftExportRanges_PartOfSpeechIdIsNormalizedLikeItsParentAndLabel — a part of speech whose
   name carries a combining diacritic, asserting the range-element @id, the @parent naming
   it from a child, and its own <label> are one and the same string.
2. LiftExportRanges_MorphTypeIdWithMarkupCharacterIsEscaped — a morph type whose name holds
   an &, asserting the ranges document parses and the id reads back as the name stored.

Not run. This was prepared without a local FieldWorks build environment, so it needs
.\build.ps1 / .\test.ps1 (or this PR's CI) before it leaves draft. Please treat both the
exporter edits and the new tests as unverified.

Third-party confirmation. The sil-lift Python library validates real FLEx exports; it
had to normalize both sides of every range comparison to stop reporting sound exports as
broken. Its issue thread, linked at the top, has the byte-level evidence, the corpus counts,
and the full list of bypassed writes with line numbers.

CI-ready checklist

• Commit messages follow .github/commit-guidelines.md (subject ≤ 72 chars, no trailing punctuation; if body present, blank line then ≤ 80-char lines).
• No whitespace warnings locally (git log --check and git diff --check are clean).
• Builds/tests pass locally — not run; see Testing above.
• Docs/workflows/ai-pr-workflow.md — not applicable; this is an outside contribution rather than core-developer work. AI assistance is disclosed in the commit trailers.
• No AGENTS.md under Src/LexText/ describes the exporter's normalization, so none needed updating.

---

This change is

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   7m 40s ⏱️ - 3m 56s
5 763 tests +2  5 682 ✅ +2  81 💤 ±0  0 ❌ ±0 
5 772 runs  +2  5 691 ✅ +2  81 💤 ±0  0 ❌ ±0 

Results for commit 8f72ebf. ± Comparison against base commit aec7a69.

♻️ This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.04%. Comparing base (aec7a69) to head (8f72ebf).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/LexText/LexTextControls/LiftExporter.cs	62.50%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
- Coverage   38.05%   38.04%   -0.01%     
==========================================
  Files        1499     1499              
  Lines      350117   350117              
  Branches    40233    40233              
==========================================
- Hits       133225   133218       -7     
- Misses     187607   187614       +7     
  Partials    29285    29285              

Files with missing lines	Coverage Δ
Src/LexText/LexTextControls/LiftExporter.cs	64.28% <62.50%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.