Support LT-22605: Add ability to limit HermitCrab parses

[jtmaxwell3]

MaxUnapplications was an experimental variable that never got incorporated into FieldWorks. I decided to rename it MaxAlternatives for clarity and to change it to throw a TimeoutException since the MaxUnapplications implementation truncated results without telling the user and since it could produce different results with different parses of the same word because of multi-threading.

MaxAlternatives limits the number of alternatives attempted by each stratum during analysis. This is where most of the blowup occurs since rules and templates within a stratum are unordered and since rules and templates are optional during unapplication (analysis). The stratum checks MaxAlternatives as it iterates over the results produced by ApplyTemplates and ApplyMorphological rules. This terminates without a full enumeration because ApplyTemplates and ApplyMorphologicalRules use lazy evaluation (yield return). However, _mrulesRule and _mTemplatesRule do not use lazy evaluation, so we set MaxAlternatives on them as well. Because of this, the maximum number of alternatives can be some multiple of MaxAlternatives in the worst case. We could also pass in the current number of alternatives to _mrulesRule and _mTemplatesRule, but we probably don't need to be that precise. We are just trying to avoid words taking forever to parse.

MaxAlternatives also limits the number of syntheses attempted.

We could also limit the number of alternatives considered within each stratum during synthesis, but the number of alternatives generated is usually proportionate to the number of syntheses attempted because rules are obligatory in the synthesis direction and there are rarely alternative orderings of the rules that match. The current limits should be enough to bound the amount of time spent parsing a word.

NB: I had to wrap the bodies of Parallel.ForEach statements with try ... catch in ParallelCombinationRuleCascade and ParallelPermutationRuleCascade in order to get the TimeoutException to propagate correctly (noted by Devin).

I tested this against Aweti-opap-no-go from Andy Black by setting MaxAlternatives to 1000 and all of the words finished in a reasonable amount of time and the words that were limited gave a reasonable error message.

---

This change is 

[codecov-commenter]

Codecov Report

❌ Patch coverage is 57.00000% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.29%. Comparing base (d9deb16) to head (3e548c3).

Files with missing lines	Patch %	Lines
...IL.Machine/Rules/ParallelPermutationRuleCascade.cs	0.00%	24 Missing ⚠️
...chine.Morphology.HermitCrab/AnalysisStratumRule.cs	42.85%	7 Missing and 1 partial ⚠️
src/SIL.Machine.Morphology.HermitCrab/Morpher.cs	70.00%	2 Missing and 1 partial ⚠️
src/SIL.Machine/Rules/RuleBatch.cs	25.00%	2 Missing and 1 partial ⚠️
....Machine/Rules/MaxAlternativesExceededException.cs	33.33%	2 Missing ⚠️
...IL.Machine/Rules/ParallelCombinationRuleCascade.cs	93.10%	0 Missing and 2 partials ⚠️
src/SIL.Machine/Rules/RuleCascade.cs	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #452      +/-   ##
==========================================
- Coverage   73.33%   73.29%   -0.04%     
==========================================
  Files         443      444       +1     
  Lines       37214    37281      +67     
  Branches     5110     5115       +5     
==========================================
+ Hits        27290    27326      +36     
- Misses       8801     8830      +29     
- Partials     1123     1125       +2     

☔ 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.

[johnml1135]

Hello John,

I am still trying to get a grapple on how this all works (getting better). I like the approach - having a deterministic "kill the parse if it takes too long" and it seems like a meaningful improvement over what we have. I investigated it with AI and here are some comments that seem reasonable to me:

The robustness gaps are mostly small, contained fixes:

1. Unwrap the parallel exception — either wrap the Parallel.ForEach body in ParallelCombinationRuleCascade with the same try/catch-and-rethrow pattern Morpher.Synthesize already uses (Morpher.cs:330-338), or move the check outside the parallel region. Without this, the feature is broken on the default configuration it's meant to serve.
2. Count with an Interlocked counter instead of output.Count() — fixes both the O(n²) counting on ConcurrentStack and the interleaving-dependent throw-or-not behavior near the boundary. One line of state, restores the determinism claim.
3. Push the cap into _prulesRule too — the CheckMaxAlternatives call is already sitting in LinearRuleCascade; it just never gets armed. One extra SetMaxAlternatives call.
4. Fix the mixed condition — use _maxAlternatives consistently instead of _morpher.MaxAlternatives > 0 && count >= _maxAlternatives, which removes the latent instant-throw on the un-initialized path, and align the >= vs > off-by-one while there.
5. A dedicated exception type (e.g. MaxAlternativesExceededException : Exception) rather than TimeoutException would let callers distinguish "grammar too ambiguous for this word" from actual timeouts, and carries the count for the error message. Cheap, but an API decision the author may prefer to keep as-is if FLEx already handles TimeoutException.

The one thing that isn't a small tweak is that it bounds breadth, not depth — a single slow pattern match with few alternatives sails past the cap. Fixing that properly means threading a real cancellation/deadline through the matcher, which is a much bigger change. But that's arguably fine as a scope cut: the observed 30-minute words are alternative-explosion cases, and this handles those. The design smell of mutable SetMaxAlternatives state on shared compiled rules is similar — worth noting in review, not worth blocking over, since every current caller sets a constant value.

So: sound idea, right layer, and items 1–4 are each a few lines. Item 1 is the only one I'd call a must-fix before merge, since it defeats the feature in the default build.

Looking forward to this landing!

[jtmaxwell3]

@johnml1135:

1: I don’t understand why "the feature is broken on the default configuration". It worked fine for me.
2: I’m not sure what is meant by “Count with an Interlocked counter”.
3: _prulesRule always has exactly one alternative because of how phonological rules work, so there is nothing to bound.
5: I don’t think we need a dedicated exception type. There are no other timeouts, and even though it isn’t technically a timeout, it effectively is.

I’m fine with only bounding the breadth, not the depth. I’m just trying to prevent an exponential explosion, and I don’t think that you can have an exponential depth.

[ddaspit]

@ddaspit reviewed 12 files and all commit messages, and made 11 comments.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on jtmaxwell3).

---

src/SIL.Machine/Rules/ParallelCombinationRuleCascade.cs line 59 at r2 (raw file):

                                        output.PushRange(results);
                                        Interlocked.Add(ref alternativeCount, results.Length);
                                        CheckMaxAlternatives(alternativeCount);

CheckMaxAlternatives(Interlocked.Add(ref alternativeCount, results.Length)) would be better.

---

src/SIL.Machine/Rules/RuleBatch.cs line 47 at r2 (raw file):

        }

        public void SetMaxAlternatives(int maxAlternatives)

This should be a property.

---

src/SIL.Machine/Rules/RuleBatch.cs line 58 at r2 (raw file):

            {
                output.UnionWith(rule.Apply(input));
                if (_maxAlternatives > 0 && output.Count >= _maxAlternatives)

I believe that this should be output.Count > _maxAlternatives.

---

src/SIL.Machine/Rules/RuleCascade.cs line 52 at r2 (raw file):

        }

        public void SetMaxAlternatives(int maxApplications)

This should be a property.

---

src/SIL.Machine/Rules/RuleCascade.cs line 57 at r2 (raw file):

        }

        public void CheckMaxAlternatives(int alternativeCount)

This should be protected.

---

src/SIL.Machine/Rules/RuleCascade.cs line 59 at r2 (raw file):

        public void CheckMaxAlternatives(int alternativeCount)
        {
            if (_maxAlternatives > 0 && alternativeCount >= _maxAlternatives)

I believe that this should be alternativeCount > _maxAlternatives.

---

src/SIL.Machine.Morphology.HermitCrab/AnalysisStratumRule.cs line 143 at r2 (raw file):

            {
                alternativeCount++;
                if (_maxAlternatives > 0 && alternativeCount >= _maxAlternatives)

I believe that this should be alternativeCount > _maxAlternatives.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 75 at r2 (raw file):

        /// <summary>
        /// MaxUnapplications limits the number of unapplications to make it possible

This comment needs to be fixed.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 322 at r2 (raw file):

                                foreach (Word alternative in synthesisWord.ExpandAlternatives())
                                {
                                    alternativeCount++;

You should use Interlocked.Increment here to avoid a race condition.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 323 at r2 (raw file):

                                {
                                    alternativeCount++;
                                    if (MaxAlternatives > 0 && alternativeCount >= MaxAlternatives)

I believe that this should be alternativeCount > MaxAlternatives.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 324 at r2 (raw file):

                                    alternativeCount++;
                                    if (MaxAlternatives > 0 && alternativeCount >= MaxAlternatives)
                                        throw new TimeoutException("MaxAlternatives exceeded");

I would prefer this be a custom exception. TimeoutException has a bit of a different meaning.

[jtmaxwell3]

@jtmaxwell3 made 11 comments.
Reviewable status: 5 of 13 files reviewed, 11 unresolved discussions (waiting on ddaspit).

---

src/SIL.Machine/Rules/ParallelCombinationRuleCascade.cs line 59 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

CheckMaxAlternatives(Interlocked.Add(ref alternativeCount, results.Length)) would be better.

Done.

---

src/SIL.Machine/Rules/RuleBatch.cs line 47 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This should be a property.

Done.

---

src/SIL.Machine/Rules/RuleBatch.cs line 58 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I believe that this should be output.Count > _maxAlternatives.

Done.

---

src/SIL.Machine/Rules/RuleCascade.cs line 52 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This should be a property.

Done.

---

src/SIL.Machine/Rules/RuleCascade.cs line 57 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This should be protected.

Done.

---

src/SIL.Machine/Rules/RuleCascade.cs line 59 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I believe that this should be alternativeCount > _maxAlternatives.

Done.

---

src/SIL.Machine.Morphology.HermitCrab/AnalysisStratumRule.cs line 143 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I believe that this should be alternativeCount > _maxAlternatives.

Done.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 75 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This comment needs to be fixed.

Done.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 322 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

You should use Interlocked.Increment here to avoid a race condition.

Done.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 323 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I believe that this should be alternativeCount > MaxAlternatives.

Done.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 324 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I would prefer this be a custom exception. TimeoutException has a bit of a different meaning.

Done.

[ddaspit]

@ddaspit reviewed 8 files and all commit messages, made 3 comments, and resolved 11 discussions.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on jtmaxwell3).

---

src/SIL.Machine/Rules/MaxAlternativesExceeded.cs at r3 (raw file):
This file should be renamed MaxAlternativesExceededException.cs to match the class name.

---

src/SIL.Machine/Rules/ParallelPermutationRuleCascade.cs line 56 at r3 (raw file):

                                {
                                    output.PushRange(results);
                                    Interlocked.Add(ref alternativeCount, results.Length);

Same pattern as ParallelCombinationRuleCascade, CheckMaxAlternatives(Interlocked.Add(ref alternativeCount, results.Length)) would be better.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 323 at r3 (raw file):

                                {
                                    Interlocked.Increment(ref alternativeCount);
                                    if (MaxAlternatives > 0 && alternativeCount > MaxAlternatives)

Same pattern as the rule cascades, if (MaxAlternatives > 0 && Interlocked.Increment(ref alternativeCount) > MaxAlternatives) would be better.

[jtmaxwell3]

@jtmaxwell3 made 3 comments.
Reviewable status: 10 of 13 files reviewed, 3 unresolved discussions (waiting on ddaspit).

---

src/SIL.Machine/Rules/ParallelPermutationRuleCascade.cs line 56 at r3 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

Same pattern as ParallelCombinationRuleCascade, CheckMaxAlternatives(Interlocked.Add(ref alternativeCount, results.Length)) would be better.

Done.

---

src/SIL.Machine.Morphology.HermitCrab/Morpher.cs line 323 at r3 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

Same pattern as the rule cascades, if (MaxAlternatives > 0 && Interlocked.Increment(ref alternativeCount) > MaxAlternatives) would be better.

Done.

---

src/SIL.Machine/Rules/MaxAlternativesExceeded.cs at r3 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This file should be renamed MaxAlternativesExceededException.cs to match the class name.

Done.

[ddaspit]

The build seems to be failing. It looks like a formatting error.

@ddaspit partially reviewed 3 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on jtmaxwell3).

[ddaspit]

@ddaspit reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on jtmaxwell3).