Fix LT-22612: LIFT import duplicated inflection classes

[mark-sil]

Summary

An inflection class with a name but no abbreviation was duplicated on every LIFT export/import round trip, doubling the count each time (one user reached 8643 MoInflClass objects where there should have been 8).

Root cause: On import, ProcessInflectionClassDefinition reuses an existing inflection class only when both its name and abbreviation match the imported range element. That reuse test relies on HasMatchingUnicodeAlternative treating "no abbreviation data" as a match — but it only did so when the LiftMultiText was null. The LIFT parser supplies an empty (non-null) LiftMultiText for an absent <abbrev>, so the abbreviation check returned false and a duplicate class was created on each import.

Fix: Treat an empty text (no usable value) the same as a null one in HasMatchingUnicodeAlternative, so an absent abbreviation no longer blocks a match.

Test

Adds TestLiftImport_InflectionClassWithNoAbbrevIsNotDuplicatedOnReimport, which imports the same data (inflection class with a label but no abbrev) twice and asserts the class is reused rather than duplicated. Fails before the fix (count → 2), passes after.

Acceptance test (from the ticket)

1. Restore the LT-22612 project.
2. File > Export > Full Lexicon LIFT.
3. File > Import > LIFT Lexicon, choosing the file just exported.
4. Grammar > Category Edit > Noun: there should be only one PL inflection class (was two, doubling on each round trip).

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   7m 41s ⏱️ - 3m 7s
4 307 tests +1  4 234 ✅ +1  73 💤 ±0  0 ❌ ±0 
4 316 runs  +1  4 243 ✅ +1  73 💤 ±0  0 ❌ ±0 

Results for commit b906e7e. ± Comparison against base commit 9b7d537.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.02%. Comparing base (9b7d537) to head (b906e7e).

Files with missing lines	Patch %	Lines
...LexTextControls/LiftMergerSupportCodeAndClasses.cs	50.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1017      +/-   ##
==========================================
+ Coverage   32.99%   33.02%   +0.03%     
==========================================
  Files        1202     1202              
  Lines      278209   278213       +4     
  Branches    37160    37161       +1     
==========================================
+ Hits        91789    91884      +95     
+ Misses     158563   158487      -76     
+ Partials    27857    27842      -15     

Files with missing lines	Coverage Δ
...LexTextControls/LiftMergerSupportCodeAndClasses.cs	57.86% <50.00%> (+1.09%)	⬆️

... and 5 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.

[mark-sil]

The Devin review results are below. The only "flagged concern" was intentional, to prevent other similar bugs.

Review Summary

The fix is small, correct, and well-targeted.

The fix itself — correct

HasMatchingUnicodeAlternative(ITsMultiString, LiftMultiText) (LiftMergerSupportCodeAndClasses.cs:2230-2253) is used as an AND condition in ProcessInflectionClassDefinition (LiftMergerSupportCodeAndClasses.cs:2383-2384): a class is reused only if both name and abbrev match.

The old code (per the diff) returned false when it fell through the loop without a positive match — which is exactly what happened for an empty (non-null) abbrev: the loop body found an empty sValue, the String.IsNullOrEmpty(sValue) || tssAlt==null branch returned false, so the whole method returned false, blocking reuse → duplicate created each import. The author's root-cause description matches the code.

The new version:

• skips empty values (:2242-2243) instead of failing on them,
• tracks whether any usable data existed (textHasData),
• returns !textHasData at the end (:2252) — so an all-empty/absent text is treated as a match.

This restores the intended "no data to compare = match" semantics for the non-null-but-empty case. Correct.

The flagged concern (Bug catcher) — real but low risk

HasMatchingUnicodeAlternative is also called from FindMatchingPossibility (:2212-2213) for all possibility types (POS, morph types, semantic domains, etc.), not just inflection classes. The behavior change: previously an empty-but-present LiftMultiText returned false; now it returns true.

So the theoretical regression is: if an import supplies an empty label (name) alongside a real abbrev, the name check now matches any candidate, and the first candidate whose abbrev matches would be reused. In practice, labels/names are effectively always populated in LIFT, so this is an edge case. I agree with the flag's assessment: real semantic broadening, benign in practice.

Worth noting the previous behavior was already inconsistent — a null text returned match, an empty one didn't. The fix makes them consistent, which is arguably more correct.

Test — good

The regression test (LiftMergerTests.cs) imports twice and asserts count stays 1 and the same GUID is reused — directly exercises the bug. Solid.

Minor observations

• No test covers the broadened path for other possibility types (the flagged edge case). Not required, but a targeted test asserting "empty label + real abbrev doesn't false-match" would document the intended semantics.
• The GUID-based path (GetPossibilityForGuidIfExisting, :2181) is worth noting: FLEx-exported LIFT normally does include guids on entries. The bug only bites when reuse falls through to the label/abbrev matching path (FindMatchingPossibility). It'd be worth confirming why the round-trip in the ticket hits the name/abbrev path rather than the guid path — but that's a diagnostic question, not a blocker for the fix.

Overall: a clean, minimal fix with a matching test. LGTM with the caveat that the semantic change applies to all possibility matching, not just inflection classes.

@mark-sil made 1 comment.
Reviewable status: 0 of 2 files reviewed, all discussions resolved.