From 5aa7b3935299c53da4f3bfb5fe57c3d54d9bde46 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 25 Sep 2026 18:48:52 -0400 Subject: [PATCH] Fix a dropped morpheme ID when a zero-width rule wraps one ending at the same node In SynthesisAffixProcessAllomorphRuleSpec.ApplyRhs, a zero-width rule's fallback "no new output morphs" marking claimed the shared final shape node before the loop that re-marks the wrapped rule's own morph, so the wrapped rule's morpheme ID could be overwritten or ordered ahead of the outer rule's. Move the fallback marking after that loop so the wrapped allomorph keeps its ID and the outer rule's ID sorts after it. Adds IdentityRuleSignatureMorphDropTests with two deterministic controls and one regression that retries fresh language loads. Measured over 100 fresh loads: with the fix, the correct signature is reached 41/100 times; with the fix reverted, 0/100 -- confirmed by reverting the production change and rerunning. The remaining gap under the fix is a separate, unfixed ordering defect (tracked in #506); this PR does not make output deterministic for every ordering, only reachable. Co-Authored-By: Claude Opus 5.5 --- .../SynthesisAffixProcessAllomorphRuleSpec.cs | 14 +- .../IdentityRuleSignatureMorphDropTests.cs | 190 ++++++++++++++++++ 2 files changed, 197 insertions(+), 7 deletions(-) create mode 100644 tests/SIL.Machine.Morphology.HermitCrab.Tests/IdentityRuleSignatureMorphDropTests.cs diff --git a/src/SIL.Machine.Morphology.HermitCrab/MorphologicalRules/SynthesisAffixProcessAllomorphRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/MorphologicalRules/SynthesisAffixProcessAllomorphRuleSpec.cs index aab74102d..86f057a06 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/MorphologicalRules/SynthesisAffixProcessAllomorphRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/MorphologicalRules/SynthesisAffixProcessAllomorphRuleSpec.cs @@ -165,13 +165,6 @@ public Word ApplyRhs(PatternRule rule, Match m _allomorph, output.MorphologicalRuleApplicationCount.ToString() ); - if (outputNewMorph == null) - { - // There are no new output morphs in a truncation rule, - // so we add its allomorph to the last output shape. - string morphID = output.MorphologicalRuleApplicationCount.ToString(); - output.MarkMorph(new List() { output.Shape.Last }, _allomorph, morphID); - } var markedAllomorphs = new HashSet(); foreach (Annotation inputMorph in match.Input.Morphs) { @@ -205,6 +198,13 @@ public Word ApplyRhs(PatternRule rule, Match m } markedAllomorphs.Add(allomorph); } + if (outputNewMorph == null) + { + // Truncation and zero-width rules have no new morph. Mark the last node after the + // loop, so a wrapped allomorph ending there keeps it and this rule's ID sorts after. + string morphID = output.MorphologicalRuleApplicationCount.ToString(); + output.MarkMorph(new List() { output.Shape.Last }, _allomorph, morphID); + } output.MprFeatures.AddOutput(_allomorph.OutMprFeatures); diff --git a/tests/SIL.Machine.Morphology.HermitCrab.Tests/IdentityRuleSignatureMorphDropTests.cs b/tests/SIL.Machine.Morphology.HermitCrab.Tests/IdentityRuleSignatureMorphDropTests.cs new file mode 100644 index 000000000..47b43c6fa --- /dev/null +++ b/tests/SIL.Machine.Morphology.HermitCrab.Tests/IdentityRuleSignatureMorphDropTests.cs @@ -0,0 +1,190 @@ +using NUnit.Framework; + +namespace SIL.Machine.Morphology.HermitCrab; + +/// +/// Regression coverage for a dropped morpheme ID when a zero-width rule wraps one ending at the +/// same shape node. +/// +[TestFixture] +public class IdentityRuleSignatureMorphDropTests +{ + private const string GrammarXml = """ + + + + + IdentityRuleSignatureMorphDropTest + + x + + + + grp + + p + q + + + + + Main + + f + e + n + o + s + + + + Any + + + + Main + + + ptoq + + + + o + + + + + PTOQ + PTOQ + + + qtop + + + + s + + + + + QTOP + QTOP + + + identity + + + + + + + + + IDENT + IDENT + + + + + fen + FEN + fen + + + + + + + """; + + private static string WriteTempGrammar() + { + string path = Path.Combine( + Path.GetTempPath(), + "hc-identity-signature-drop-" + Guid.NewGuid().ToString("N") + ".xml" + ); + File.WriteAllText(path, GrammarXml); + return path; + } + + // Mirrors BatchCommand.BuildSignature's format without a dependency on the Tool project. + private static string Signature(IEnumerable results) + { + List signatures = results + .Select(w => + string.Join("+", w.AllomorphsInMorphOrder.Select(a => a.Morpheme.Id)) + + "|" + + w.Shape.ToRegexString(w.Stratum.CharacterDefinitionTable, true) + ) + .OrderBy(s => s, StringComparer.Ordinal) + .ToList(); + return signatures.Count == 0 ? "-" : string.Join(";", signatures); + } + + private static string ParseWithFreshMorpher(Language language, string word) + { + // maxDegreeOfParallelism: 1 matches ConformanceMorpherFactory.Create's default + // (useMemoization: true) -- the construction every conformance self-check run actually + // uses. + var morpher = new Morpher(new TraceManager(), language, maxDegreeOfParallelism: 1); + return Signature(morpher.ParseWord(word, out _, false).ToList()); + } + + [Test] + public void Fen_BareRootControl_IdentityWrapsRootDirectly_StableAndCorrect() + { + // Control: mrIdentity wraps the root directly, so no two rules share an affix boundary. + string path = WriteTempGrammar(); + try + { + Language language = XmlLanguageLoader.Load(path); + for (int trial = 0; trial < 10; trial++) + Assert.That(ParseWithFreshMorpher(language, "fen"), Is.EqualTo("FEN+IDENT|fen;FEN|fen")); + } + finally + { + File.Delete(path); + } + } + + [Test] + public void Feno_NoIdentityRuleApplies_StableAndCorrect() + { + // Control: after mrPtoQ alone the value is q, so mrIdentity (requires p) never applies. + string path = WriteTempGrammar(); + try + { + Language language = XmlLanguageLoader.Load(path); + for (int trial = 0; trial < 10; trial++) + Assert.That(ParseWithFreshMorpher(language, "feno"), Is.EqualTo("FEN+PTOQ|feno")); + } + finally + { + File.Delete(path); + } + } + + [Test] + public void Fenos_IdentityWrapsOutermost_IsFlakyPendingASeparateOrderingDefect() + { + // Measured: fix reaches the correct signature 41/100 fresh loads; reverted, 0/100. The + // remaining gap is a separate ordering defect (#506), not BidirList's random skip-list levels. + string path = WriteTempGrammar(); + try + { + const string CorrectSig = "FEN+PTOQ+QTOP+IDENT|fenos;FEN+PTOQ+QTOP|fenos"; + bool sawCorrect = false; + for (int trial = 0; trial < 100 && !sawCorrect; trial++) + { + Language language = XmlLanguageLoader.Load(path); + sawCorrect = ParseWithFreshMorpher(language, "fenos") == CorrectSig; + } + + Assert.That(sawCorrect, Is.True, "the correct signature was never reached in 100 tries"); + } + finally + { + File.Delete(path); + } + } +}