Skip to content

Cap RealizationalRule synthesis application at once per word - #474

Open
johnml1135 wants to merge 1 commit into
masterfrom
fix/realizational-rule-unbounded-reapplication
Open

Cap RealizationalRule synthesis application at once per word#474
johnml1135 wants to merge 1 commit into
masterfrom
fix/realizational-rule-unbounded-reapplication

Conversation

@johnml1135

@johnml1135 johnml1135 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

SynthesisRealizationalAffixProcessRule.Apply (the synthesis side of a
RealizationalRule) never checked a per-word application-count cap the
way SynthesisAffixProcessRule and SynthesisCompoundingRule already
do. That omission let a Blockable RealizationalRule reapply to its
own output forever whenever CheckBlocking couldn't rescue it, hanging
ParseWord/AnalyzeWord.

Reproduction

Minimal case (no AffixTemplate involved — this only reproduces when the
rule is a direct Stratum morphological rule with morphologicalRuleOrder="linear"):

  • One PartOfSpeech, one MorphologicalPhonologicalRuleFeature (mprX).
  • One RealizationalRule whose single subrule requires mprX and appends
    a fixed suffix to whatever it's given.
  • One LexicalEntry on that part of speech carrying ruleFeatures="mprX",
    with no family attribute.

Parsing that entry's own bare surface form hangs indefinitely. A stemName
on the entry is not required to reproduce it (an earlier report described
a fixture where the triggering entry happened to carry one) — a bare
ruleFeatures="mprX" entry with no family reproduces it on its own.

I found this while investigating a report against a conformance fixture
that has two pre-existing mprRRealTest-tagged roots and hangs when a
third is added. Each of the two pre-existing roots happens to dodge the
bug for a different, unrelated reason: one has AssignedHeadFeatures
that conflict with the rule's RequiredHeadFeatures (so the rule never
applies at all), and the other has a family whose other member doesn't
carry the same MPR feature (so CheckBlocking substitutes a word that
fails the retry and the recursion stops after one step). A third root
that is rule-eligible and not rescued either way exposes the underlying
defect regardless of stemName.

Root cause

SynthesisStratumRule builds its morphological-rule cascade as
new LinearRuleCascade<Word, ShapeNode>(mrules, true, ...). The true
("multiple application") tells LinearRuleCascade.ApplyRules to retry
the same rule index against a rule's own output rather than only
advancing forward, guarded only by "stop once the rule's output equals
its input" (LinearRuleCascade.cs, the "avoid infinite loop" comment).

For an ordinary AffixProcessRule/CompoundingRule, SynthesisAffixProcessRule/
SynthesisCompoundingRule check input.GetApplicationCount(_rule) >= _rule.MaxApplicationCount
before doing anything else, so the second attempt at the same rule index
returns nothing and the recursion terminates. RealizationalRule has no
multipleApplication attribute in the DTD and no MaxApplicationCount
property at all, and SynthesisRealizationalAffixProcessRule.Apply never
consulted GetApplicationCount — even though it already records the
application via the existing outWord.MorphologicalRuleApplied(_rule, ...)
call. So a root that is realization-eligible and not blocking-rescued keeps
appending the same affix to its own growing output, forever: each pass makes
the shape strictly longer, so the cascade's only other guard never fires
either, and Morpher.MaxAlternatives (the general escape valve for this
class of runaway) is wired into AnalysisStratumRule but never into
SynthesisStratumRule, so it can't catch this path even when configured.

This surfaces during analysis because Morpher.ParseWord confirms every
analysis candidate by re-synthesizing it (Synthesize), including the
trivial "bare root, no affixes" candidate — so a root eligible for an
unguarded realizational rule hangs confirming even its own bare-word parse.

Fix

Add the same application-count guard SynthesisAffixProcessRule already
has, hardcoded to a cap of 1 since RealizationalRule has no configurable
multipleApplication attribute — a realizational rule realizes a feature
value once per word by definition.

Test plan

  • Added ParseWord_UnblockedRealizationalRuleMatchesOwnOutput_DoesNotHang
    to MorpherTests.cs: a Blockable RealizationalRule (no family) on
    a stratum with MorphologicalRuleOrder.Linear, gated by an MPR
    feature the root carries. Wraps ParseWord in a 10s timeout and
    asserts it completes.
    • Verified the test fails (times out) with the fix reverted, and
      passes with the fix in place.
  • Full SIL.Machine.Morphology.HermitCrab.Tests suite: 69/69 passing
    (68 pre-existing + the new test), including the pre-existing
    template-based RealizationalRule coverage in AffixTemplateTests.cs.
  • SIL.Machine.Morphology.HermitCrab is referenced only by its own
    test project and the hc CLI tool, so no other test project is
    affected by this change.

🤖 Generated with Claude Code


This change is Reviewable

A Blockable RealizationalRule with no family (or no family member
matching the word's current features) reapplied to its own output
forever during synthesis: LinearRuleCascade retries a matching rule
against its own result whenever that result differs from the input,
and SynthesisRealizationalAffixProcessRule never checked an
application-count cap the way SynthesisAffixProcessRule and
SynthesisCompoundingRule already do. Since RealizationalRule has no
multipleApplication attribute, cap it at one application per word.

This hung ParseWord/AnalyzeWord confirmation for any root eligible for
such a rule, including confirmation of the root's own bare-word parse.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 23.07692% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.32%. Comparing base (0b58830) to head (e82f6b2).

Files with missing lines Patch % Lines
...calRules/SynthesisRealizationalAffixProcessRule.cs 23.07% 9 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #474      +/-   ##
==========================================
- Coverage   73.33%   73.32%   -0.02%     
==========================================
  Files         445      445              
  Lines       37317    37330      +13     
  Branches     5118     5120       +2     
==========================================
+ Hits        27367    27371       +4     
- Misses       8825     8834       +9     
  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.

@ddaspit ddaspit left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants