Fix unhandled InvalidCastException when a rewrite/metathesis rule has a top-level Quantifier

[johnml1135]

The bug

Constraint<TData, TOffset> (src/SIL.Machine/Matching/Constraint.cs:12-13) and Quantifier<TData, TOffset> (src/SIL.Machine/Matching/Quantifier.cs:13-14) are both direct subclasses of PatternNode<TData, TOffset> — siblings, with no inheritance relationship between them.

The HermitCrab rule specs assume every top-level child of a rule's target (Lhs) or replacement (Rhs) pattern — or a metathesis switch group — is a Constraint, and cast accordingly with Children.Cast<Constraint<Word, ShapeNode>>() or a direct (Constraint<Word, ShapeNode>) cast:

• PhonologicalRules/SynthesisRewriteRuleSpec.cs:33 (and :93, lazily, via Pattern.Acceptable)
• PhonologicalRules/FeatureAnalysisRewriteRuleSpec.cs:42-43
• PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs:45 (lazily, in Unapply)
• PhonologicalRules/EpenthesisAnalysisRewriteRuleSpec.cs:18
• PhonologicalRules/SynthesisMetathesisRuleSpec.cs:31
• PhonologicalRules/AnalysisMetathesisRuleSpec.cs:53

But the DTD's PhoneticSequence element (HermitCrabInput.dtd:515) allows an OptionalSegmentSequence in exactly the same positions as a plain Segment/SimpleContext — and OptionalSegmentSequence itself (HermitCrabInput.dtd:560) carries an id attribute explicitly documented as "only needed for a metathesis rule", so it's legal there too. XmlLanguageLoader.LoadPatternNodes (~1405-1415, ~1493-1505) builds a real Quantifier for an OptionalSegmentSequence with no LHS-vs-environment distinction.

So a DTD-legal grammar — a rewrite rule whose target or replacement contains a top-level optional/repeated segment sequence — crashes with an unhandled InvalidCastException during Morpher construction. Depending on which of the sites above runs first, it either propagates raw straight out of new Morpher(...) (the synthesis side has no try/catch around rule compilation), or gets caught by a pre-existing, unrelated catch-all in AnalysisStratumRule.CompilePhonologicalRule — but even then the message is uninformative (just names the rule; the real cause is buried in an opaque InnerException).

Quantifiers already work correctly in a rule's environment (LeftEnvironment/RightEnvironment — see the existing QuantifierRules test) since environment matching doesn't go through this cast. The bug is specific to the target/replacement/switch-group position.

Reproduction

Verified directly (not hypothetical) — found while building an independent FST-based morphological analyzer/proposer (PanGloss) against this library; a grammar under construction hit this.

Two new tests in RewriteRuleTests.cs build a RewriteRule directly (bypassing the XML loader, matching the rest of that file) with a top-level Quantifier in the Lhs and, separately, the Rhs, then construct a Morpher:

• QuantifierInTargetIsRejectedWithClearError — before the fix, throws a raw, unhandled InvalidCastException straight out of new Morpher(...) (confirmed against unmodified master).
• QuantifierInReplacementIsRejectedWithClearError — before the fix, throws a CompileException (via the pre-existing AnalysisStratumRule wrapper) whose InnerException is the same raw, uninformative InvalidCastException (also confirmed against unmodified master).

The fix

I considered two approaches:

1. Genuinely support a Quantifier in this position — the DTD permits it, so the library arguably should handle it.
2. Convert the crash into a clear, typed, load-time diagnostic naming the unsupported construct and where it appeared.

I went with (2). Actually supporting a variable-width quantifier as the target of a rewrite rule (matched-and-replaced, or matched-and-deleted) is a real semantic feature, not a mechanical fix — it changes what "the same number of Lhs and Rhs children" even means for the whole rewrite-subrule-shape dispatch (Feature/Narrow/Epenthesis selection) these classes are built around. That's too large and semantically risky a change to make confidently in an external repo I don't own, especially with no existing test/grammar exercising it to pin the intended behavior. A clear, fail-fast diagnostic is the conservative, reviewable option, and is a strict improvement over an opaque crash either way.

Added PatternNodeCastExtensions (CastToConstraints / AsConstraint) and used it at every site above instead of the unchecked cast. For any grammar built entirely from Constraints — i.e. every currently-working grammar — behavior is byte-for-byte unchanged: the helper returns the exact same Constraint the cast would have produced. When a Quantifier (or any other non-Constraint PatternNode) shows up in one of these positions, it now raises a CompileException such as:

The target (left-hand side) of a rewrite rule contains a 'Quantifier`2', which is not supported there. Only simple segments, natural classes, and boundary markers are supported; optional or repeated segment sequences (quantifiers) are only supported within a rule's left/right environment.

Test evidence

Full solution, before vs. after (temporarily reverted just the 6 fixed src/ files to master's content to confirm red state, then restored):

Before the fix (unmodified master + the two new tests): both new tests fail —

• QuantifierInTargetIsRejectedWithClearError: raw System.InvalidCastException thrown directly from new Morpher(...), at SynthesisRewriteRuleSpec.cs:33.
• QuantifierInReplacementIsRejectedWithClearError: CompileException wrapping a raw InvalidCastException at FeatureAnalysisRewriteRuleSpec.cs:43.

After the fix: full solution build clean (0 errors, 0 new warnings), dotnet test on the whole solution:

SIL.Machine.Tokenization.SentencePiece.Tests: Passed: 4, Failed: 0
SIL.Machine.Morphology.HermitCrab.Tests:      Passed: 68, Failed: 0   (66 pre-existing + 2 new)
SIL.Machine.Translation.Thot.Tests:           Passed: 83, Failed: 0
SIL.Machine.Tests:                            Passed: 804, Failed: 0, Skipped: 3 (pre-existing, unrelated)

All pre-existing tests pass unchanged; only the two new tests are added.

Ran dotnet csharpier format on the touched files only (no wholesale reformatting).

Test plan

• Reproducing tests added, confirmed failing against unmodified master
• Fix applied, same tests now pass
• Full solution dotnet build: clean
• Full solution dotnet test: all pass (962 total, 3 pre-existing unrelated skips)
• dotnet csharpier check clean on touched files

🤖 Generated with Claude Code

---

This change is 

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.33%. Comparing base (c1811b4) to head (521334b).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #464      +/-   ##
==========================================
+ Coverage   73.29%   73.33%   +0.03%     
==========================================
  Files         444      445       +1     
  Lines       37283    37313      +30     
  Branches     5116     5118       +2     
==========================================
+ Hits        27328    27363      +35     
+ Misses       8830     8825       -5     
  Partials     1125     1125              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.