Skip to content

HermitCrab: keep final-template pruning on when only lexical entries are partial - #553

Open
johnml1135 wants to merge 1 commit into
masterfrom
fix-551-partial-entry-prune
Open

johnml1135 wants to merge 1 commit into
masterfrom
fix-551-partial-entry-prune

Conversation

@johnml1135

@johnml1135 johnml1135 commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #551.

Problem

The final-template prune from #491 is skipped whenever Morpher.IsPartial is true, and IsPartial counts partial lexical entries. A grammar with partial entries and no partial rules never prunes.

Why entries can be ignored

#551 suspected this was unsound: a word inherits partiality from its root (Word.cs:150), and synthesis lets a non-partial rule follow a final template when the input word is partial (SynthesisAffixProcessRule.cs:67). That would let a partial root license a derivation the prune rejects.

It does not, because SynthesisAffixTemplatesRule skips every template when input.RootAllomorph.Morpheme.IsPartial. No template, final or not, ever applies to a word with a partial root, so an analysis that unapplies a final template cannot end at a partial root.

Partial rules are different. A partial rule makes the word partial after the root, and the template can still apply, so a non-partial rule can then follow a final template. They keep disabling the prune.

Change

  • Morpher.HasPartialRules: true if any partial morpheme is not a LexEntry. IsPartial is unchanged.
  • AnalysisAffixTemplateRule and AnalysisAffixProcessRule read HasPartialRules instead of IsPartial.
  • AffixTemplateTests.PartialLexicalEntryDoesNotDisableEarlyPruningOfFinalTemplate: with a partial entry and no partial rule the prune fires (trace depth 2) and a partial root parses the same as before (sab, sabv parse; sabd, sabdv do not). With a partial rule the prune stays off and sagdv still parses as 32 PAST NOM. The test fails with the old guard (the prune is reached at depth 6 instead of 2).

Evidence

  • Fixture conformance/edge-cases/final-template-partial-root on Add a conformance suite for HermitCrab, with its adequacy argument #480 (integrate-conformance-framework at fa8d9b2) has partial roots in a lower stratum and in the template's own stratum. Every final-template string on a partial root has no analysis, with non-partial controls.
  • In a scratch copy with the same guard, all 142 conformance runs over 132 fixtures (including pathological) passed, and the prune fired on the new fixture.
  • ./local_check.sh --agent-strict passes on this branch.

Not covered

Inflectional-family stem substitution (ChooseInflectionalStem) replaces the root with a family relative before the template check. I traced it by reading only: the replacement word is non-partial, so a following non-partial rule is blocked either way. No fixture exercises it.

Lexical-pattern guesses were already excluded from IsPartial and pruned.

🤖 Generated with Claude Code


This change is Reviewable

The analysis-side final-template prune (491) was switched off whenever Morpher.IsPartial
was true, and IsPartial counts partial lexical entries. Grammars with partial entries but
no partial rules therefore never pruned.

A partial entry cannot make the prune lose a parse. A word takes its partiality from its
root, but SynthesisAffixTemplatesRule applies no template to a word whose root morpheme is
partial, so a derivation that unapplies a final template can never end at a partial root.
Partial rules differ: they make a word partial after the template has applied, which lets
a non-partial rule follow the final template, so they still disable the prune.

Add Morpher.HasPartialRules and read it in the two analysis guards. IsPartial keeps its
meaning. Lexical-pattern guesses were already excluded from IsPartial and pruned.

Closes #551

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

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.74%. Comparing base (2599204) to head (57a3b7e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #553      +/-   ##
==========================================
+ Coverage   74.57%   74.74%   +0.16%     
==========================================
  Files         453      453              
  Lines       38314    38316       +2     
  Branches     5280     5280              
==========================================
+ Hits        28573    28638      +65     
+ Misses       8576     8525      -51     
+ Partials     1165     1153      -12     

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

/// True if any rule is partial, ignoring lexical entries. Analysis may prune a final template that follows
/// a non-template rule only when this is false. A partial lexical entry does not matter here:
/// SynthesisAffixTemplatesRule applies no template to a word whose root is partial, so a
/// derivation that unapplies a final template can never end at a partial root.

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.

FYI: F1. The summary says an analysis that unapplies a final template "can never end at a partial root", but it can: SynthesisAffixTemplatesRule.Apply calls ChooseInflectionalStem before the RootAllomorph.Morpheme.IsPartial check, so a partial root with a non-partial LexFamily relative still takes the template. The prune stays sound only because that relative's word is non-partial. Consider cutting the summary to the contract: true when any morpheme other than a LexEntry is partial.

Morphophonemic.AffixTemplates.Add(verbTemplate);
var nominalizer = CreateSuffix("nominalizer", "NOM", "v");
Morphophonemic.MorphologicalRules.Add(nominalizer);
AddEntry("partial_root", verb.Clone(), Morphophonemic, "sab").IsPartial = true;

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.

FYI: F2. The partial sab entry is never removed. TestCleanup clears rules and templates but not Entries, and Language is built once per fixture, so later tests in AffixTemplateTests run against a grammar with an extra partial V root. No current test parses a sab… form, so nothing fails today. Removing it in a finally, as PruneDisagreeingCopiesTests does, keeps the fixture independent of test order.

@claude

claude Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor
  1. Verdict: approve. The soundness argument holds: a word gets IsPartial only from its root (Word.cs:150) or a partial rule (SynthesisAffixProcessRule.cs:190). Templates never apply to a partial root (SynthesisAffixTemplatesRule.cs:38), and a family stem swapped in by ChooseInflectionalStem is non-partial, so no partial entry can license a rule after a final template.
  2. Most important: F1, src/SIL.Machine.Morphology.HermitCrab/Morpher.cs:110. The new doc comment states the invariant more strongly than the code guarantees.
  3. Counts: 0 Critical, 0 Important, 2 Low.
  4. Ran: dotnet test tests/SIL.Machine.Morphology.HermitCrab.Tests --filter FullyQualifiedName~AffixTemplateTests, which passed 11 of 11. gh pr checks 553: Build on ubuntu-22.04 and windows-latest pass, Comment hygiene pass (advisory), claude-review pending.
  5. Not verified: no test covers the ChooseInflectionalStem path with a partial root and a non-partial relative, so I checked it by reading the code only, as the PR also says. Merging and parallel runs were not exercised in a grammar that has partial entries.

Public API: new public property Morpher.HasPartialRules. Analysis behavior changes for grammars that have partial lexical entries but no partial rules: the final-template prune now runs, and traces show NonPartialRuleProhibitedAfterFinalTemplate. Target framework, package, and sillsdev/machine.py parity: none verified.

Findings: F1 new, F2 new.

Reviewed at 57a3b7e

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HermitCrab: partial lexical entries switch off final-template pruning for the whole grammar

3 participants