[johnml1135]

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 slots cannot find their material, in order and at the word edges #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

[codecov-commenter]

Codecov Report

❌ Patch coverage is 92.92929% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.79%. Comparing base (c245476) to head (07d7dca).
⚠️ Report is 13 commits behind head on master.

Files with missing lines	Patch %	Lines
...hine.Morphology.HermitCrab/RequiredSlotMaterial.cs	94.92%	4 Missing and 12 partials ⚠️
...Morphology.HermitCrab/AnalysisAffixTemplateRule.cs	85.52%	7 Missing and 4 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.79%   +0.22%     
==========================================
  Files         453      454       +1     
  Lines       38342    38729     +387     
  Branches     5284     5372      +88     
==========================================
+ Hits        28593    28968     +375     
  Misses       8583     8583              
- Partials     1166     1178      +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.

[claude]

Major: F10. This test fails once merged with master: on the merge commit 724c90f, both cases fail at this line because the parse with pruning off is empty, so the prune is not the cause. Since master's 98d902a, partial lexical entries no longer set HasPartialRules. The deletion rule leaves the word NonTemplate, and TemplateOf's template defaults to IsFinal, so it is now blocked. CI runs on push, so it tests the head, not the merge. Rebase, then set IsFinal = false here.

[claude]

Verdict: request changes. F8 and F9 are fixed in 07d7dca, but the branch fails a test once merged with master.

Most important: F10 (Critical), tests/SIL.Machine.Morphology.HermitCrab.Tests/AnalysisAffixTemplateOrderTests.cs:288. Apply_DeletedOptionalSegmentBeforePrefix_DoesNotCountAgainstReach returns no parse with pruning off on the merge commit, because master's HasPartialRules change (98d902a) now blocks the final template it builds. Rebase onto master and fix the fixture.

Counts: 1 new Critical (F10), 0 Important, 0 new Low.

Ran: dotnet test tests/SIL.Machine.Morphology.HermitCrab.Tests -c Release on merge commit 724c90f: 222 passed, 2 failed (both parallelism cases of F10). Also read the 91ee3da..07d7dca diff. gh pr checks 575: both builds and claude-review are pending; the advisory comment hygiene reports passed. Those builds test the pushed head, not the merge.

Not verified: I could not run the tests at 07d7dca alone, since a separate worktree was not approved here. That the test passes there rests on the PR body (199/199 at 91ee3da). The cause in F10 comes from reading HermitCrabTestBase.AddEntry (an empty syntactic FS makes an entry partial) and the merged Morpher; I did not patch the fixture to confirm the fix.

Public API: Morpher.PruneRequiredSlots (new, default on) is unchanged since the last round. Target framework, package, and sillsdev/machine.py parity: None verified.

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

Reviewed at 07d7dca