Prune affix template partials whose required slot cannot find its material by johnml1135 · Pull Request #575 · sillsdev/machine

johnml1135 · 2026-10-28T01:09:09Z

Start here: src/SIL.Machine.Morphology.HermitCrab/RequiredSlotMaterial.cs -- the class remarks (Presence, Order) give the soundness argument for dropping a partial; the rest is wiring.

Status: open, review required; head 91ee3da. CI was still running when this was written. #576 and #579 are folded into this branch (both show as merged); #580 (draft) is parked on top and #577 (draft) is parked.

HermitCrab's affix-template walk now drops a partial as soon as a required slot can no longer find the segments its affix inserts. Parses are unchanged; doomed partials stop paying for every optional slot between them and the required one. The slot's literals must appear in the order the rule inserts them and, for prefix-only or suffix-only slots, near the word edge; slots whose unmatched parts are unbounded runs keep being tested. Pruning is on by default; Morpher.PruneRequiredSlots = false turns it off (set it before parsing), and tracing never prunes, so failure reasons stay.

The question a reviewer arrives with is "does it lose a parse?". Tests assert identical parses with pruning off and on for each hard case, and a shadow pass found 0 violations (carried over, see Evidence). A slot is tested only while every slot walked since is pure, ordered where order is used, and typed; untyped constraints and boundary-only inserts are handled conservatively. The question worth your time is the cost side: the look-ahead has its own unification cost, so whether it pays on real grammars.

Where to look

Soundness of pruning: Apply_ImpureSlotBetween_DoesNotPruneForTheInnerRequiredSlot (purity gate), AnalysisAffixTemplateOrderTests (order and edge window SlotWalk.Lo/Hi), AnalysisAffixTemplateFusionTests (unbounded runs).
RequiredSlotMaterial.RestoredSegments against AnalysisMorphologicalTransform.Untruncate: an unbounded run must restore nothing, or the purity test is unsound.
Parallel and tracing: Apply_Parallel_ReturnsTheSameParsesAsSequential, Apply_Tracing_RunsTheRulesForDetailedFailureInformation.
Public switch and conservative cases: Apply_PublicPruningSwitch_ControlsRuleAttempts, Compile_UntypedConstraints_DisablesTheSummaryConservatively, Apply_BoundaryOnlyInfix_DoesNotMakeReachUnbounded.

Not here: the performance follow-ups (repeated shape scans, narrowing, keeping shadow mode) are tracked in #598; #580 is parked; the stack was not re-measured on top of #569, which may reduce its value.

Verification at 91ee3da: HermitCrab tests 199/199 pass (Release); dotnet csharpier and comment hygiene clean; the HermitCrab project builds with 0 warnings. The full local_check.sh did not complete (cmake is missing for SentencePiece). The measurements below were not re-run at this head.

Next: review RequiredSlotMaterial.cs, and say whether the cost side justifies merging before #569 is measured.

Reading this a year from now -- start here

This branch is four commits on master c245476: 0bf9e64 (the required-slot prune, issue #561), 28cbee1 (was PR #576: slot literals in order and at reachable edges, issue #567), faa2499 (was PR #579: keep testing behind slots whose unmatched parts are unbounded runs) and 91ee3da (review fixes). #576 and #579 were separate stacked PRs and were folded in so the look-ahead is reviewed once. All of it runs under the internal RequiredSlotLookaheadMode (Off, Shadow, On); Morpher.PruneRequiredSlots is the public on/off view of it. The unification and census counters that produced the numbers below are not in this PR; they live on the research branches research/hc-rsl-census @ 6ad11702 and research/hc-ordered-census @ 4bb687f0.

Decisions, and why

Net unification calls over the whole parse (look-ahead included) is the yardstick for the first commit, not the original plan's kill criterion. That criterion (look-ahead check unifications more than 3x the analysis events saved) tripped on Mbugwe at 3.19x (1,112,761 check unifications against 349,004 events saved) because it compared unlike units; on the same-unit net comparison Mbugwe was break-even.
Shadow mode counts and prunes nothing, so soundness and cost can be checked on real grammars without changing parses. It and RequiredSlotCheckObserver (null by default) are kept for the soundness tests; whether to keep them is part of HermitCrab analysis: measure the required-slot look-ahead's repeated scans, narrowing and shadow diagnostics in steps #598.
An unbounded quantifier counts as restoring zero segments because Untruncate writes nothing for it, so a slot whose unmatched parts are unbounded runs stays pure. Bounded runs, bare restored segments, alternations with an optional branch and ModifyFromInput stay impure.
A part copied twice is unordered, so only the presence test runs behind it.
Review fixes (91ee3da): tracing bypasses pruning so detailed failure information is kept; an allomorph with an untyped constraint is impure and unprunable; boundary-only inserts do not make the edge reach unbounded.

Paths not taken

Skipping slots that insert a single segment was tried on the first commit: it halved Sena's gain and did not reduce Mbugwe's check cost.
A supply-set design (an optional restored node could stand for any required segment, so count distinct nodes) was not built here; it is perf(hermitcrab): keep the required-slot look-ahead testing behind slots that write back deleted stem segments #580, parked.

Evidence (carried over, not re-run at 91ee3da)

All figures are from the bodies of #575, #576 and #579 before the fold, measured on those PRs' own heads (before the review-fix commit; the #579 figures also before #522 removed the analysis memo). They are not from 91ee3da.

Whole stack against master, from the perf(hermitcrab): keep the required-slot look-ahead testing behind slots whose unmatched parts are unbounded runs #579 table: events plus synthesis steps, Mbugwe 40.85% less, Sena 45.92% less, Aweti 11.50% less, Amharic 14.08% less, conformance 1.80% less, Indonesian equal. Unification calls including the look-ahead's own checks: Mbugwe 22.55% less, Sena 5.22% less, Aweti 9.27% less, Amharic 2.75% less, conformance 1.22% less, Indonesian equal. Words measured: Mbugwe 40, Sena 51, Aweti 99, Amharic 24, Indonesian 70, conformance 1,452.
First commit alone (earlier Prune affix template partials whose required slot cannot find its material #575 body), unification calls: Mbugwe and conformance within 0.4% of break-even, Sena 1.6% less, Aweti 4.5% less, stress grammar 37.9% less, Amharic 0.63% more (411,073 to 413,674 calls, 24 words). The order and edge commit turned Amharic into a net saving (3.4 points, per the Require slot literals in order and at reachable edges in the template look-ahead #576 body).
Look-ahead's own check unifications: Mbugwe 995,434 to 1,133,568 (+13.88%) and Aweti 232,302 to 232,639 (+0.15%) from the unbounded-run commit; both are inside the totals above.
Shadow runs over conformance, Indonesian, Aweti, Amharic, Mbugwe, Sena and the Bantu stress grammar: 0 violations, 0 completions under a fired check; per-word parses equal to master in traced and merged modes.
Conformance runner, base vs branch (first and second commits): the same 4 known failures in both (truncate-morphotactic family), per-fixture verdicts identical.
The fusion stress grammar is synthetic (it models vowel-fusion allomorphs in template slots, which none of the other stress grammars do); its percentages are not evidence about a real language. Mbugwe is the real one.

Deferred, and what would unblock it

Mbugwe's remaining impure slots and Aweti's events behind impure slots (bounded runs, bare restored segments, modification): perf(hermitcrab): keep the required-slot look-ahead testing behind slots that write back deleted stem segments #580, parked until it justifies its code on real grammars.
Look-ahead on the shared trailing-slot walk: perf(hermitcrab): run the required-slot look-ahead on the shared trailing-slot template walk #577 (draft), parked until perf(hermitcrab): share trailing analysis template slot batches #546 lands.
Re-measuring on top of Extend MergeEquivalentAnalyses to cover templates and rules #569 (merge equivalent analyses across templates and rules), which shrinks the same search space.
The three performance questions in HermitCrab analysis: measure the required-slot look-ahead's repeated scans, narrowing and shadow diagnostics in steps #598.

🤖 Generated with Claude Code

This change is

…erial An analysis template partial now stops as soon as a required slot can no longer find the segments its affix inserts, instead of paying for every optional slot before the required one. The test is used only while every slot walked since is a pure copy-and-insert rule, so parses do not change. A count-only shadow mode and a null-by-default check observer let a harness verify soundness and cost on real grammars. Closes #561 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

codecov-commenter · 2026-10-28T01:11:56Z

Codecov Report

❌ Patch coverage is 89.51220% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.76%. Comparing base (c245476) to head (91ee3da).

Files with missing lines	Patch %	Lines
...Morphology.HermitCrab/AnalysisAffixTemplateRule.cs	75.28%	15 Missing and 7 partials ⚠️
...hine.Morphology.HermitCrab/RequiredSlotMaterial.cs	93.67%	8 Missing and 12 partials ⚠️
src/SIL.Machine.Morphology.HermitCrab/Morpher.cs	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #575      +/-   ##
==========================================
+ Coverage   74.57%   74.76%   +0.19%     
==========================================
  Files         453      454       +1     
  Lines       38342    38743     +401     
  Branches     5284     5375      +91     
==========================================
+ Hits        28593    28967     +374     
- Misses       8583     8595      +12     
- Partials     1166     1181      +15

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

…lking a template The required-slot look-ahead now needs each rule's inserted segments to match distinct segments in the order the rule inserts them, a slot that inserts only before or after its copied parts to find that material within the reach of the outer slots' own edge material, and the material of a required ordered slot to lie after the prefix and before the suffix of the required slots that unapply first. Slots that can reorder, repeat or remove from the middle switch the order and edge tests off for the slots behind them. Closes #567 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…hed parts are unbounded runs, which restore no segments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…y rules conservatively - Morpher.PruneRequiredSlots turns the prune off (default on); set it before parsing. - Tracing runs the slot rules without pruning, so failure reasons stay. - An allomorph with an untyped constraint is impure and unprunable, so a grammar that compiled before cannot throw. - A boundary-only insertion no longer counts as an infix. - Drop the redundant HasSegmentConstraint guard. - Tests for the optional-node rule, the suffix-side window, empty and deleted unbounded runs, and ordered copies.

claude · 2026-10-30T22:19:57Z

+        private bool RequiredSlotCannotUnapply(Word word, int lastSlot, ref int[] fired)
+        {
+            RequiredSlotLookaheadMode mode = _morpher.RequiredSlotLookahead;
+            if (mode == RequiredSlotLookaheadMode.Off || _morpher.TraceManager.IsTracing)


FYI: F8. Dead trace branches: this method now returns false whenever TraceManager.IsTracing is set, so the IsTracing blocks that trace a prune at lines 69-70, 152-153, and 200-206 can never run. They suggest pruned partials still show up in traces, but they don't. Remove them.

claude · 2026-10-30T22:20:00Z

+                        || allomorph.Rhs.Where(IsInsertion).SelectMany(InsertedConstraints).Any(HasNoType)
+                    )
+                    {
+                        summary.MayAlwaysUnapply = true;


FYI: F9. Duplicated block: these five lines, which make a slot opaque, are the same as lines 306-310 for an unknown rule type. A field added to SlotSummary later has to be set in both places, and missing one makes the test unsound. Move them into one helper.

claude · 2026-10-30T22:20:06Z

Verdict: approve. F1-F3 and F6-F7 are fixed in 91ee3da. F4 and F5 are deferred to #598.

Most important: F8 (Low), AnalysisAffixTemplateRule.cs:91. Turning the prune off while tracing leaves the three trace-a-prune branches unreachable, so they should be removed.

Counts: 0 Critical, 0 Important, 2 new Low (F8, F9).

Ran: gh pr checks 575: both builds passed (ubuntu-22.04, windows-latest), and so did NuGet packaging and the advisory comment hygiene report. claude-review was still pending. I read the faa2499..91ee3da diff and checked each fix against the code and its new test. I did not run --agent-strict.

Not verified: I did not run the new tests locally. I did not re-measure the pruning cost (F5, now #598).

Public API changed: Morpher.PruneRequiredSlots is a new public property, defaulting to on. Target framework, package, and sillsdev/machine.py parity: None verified. There is no Python counterpart.

Findings: F1 addressed, F2 addressed, F3 addressed, F4 accepted (#598), F5 accepted (#598, unverified), F6 addressed, F7 addressed, F8 new, F9 new.

Reviewed at 91ee3da