WordBreakSpellChecker.suggestWordBreaks ignores setMaxChanges(0) and still returns one-break suggestions

WordBreakSpellChecker (lucene-suggest) suggests corrections by breaking one term into indexed words (sunflower -> sun flower) or combining several terms into one. setMaxChanges documents "The maximum numbers of changes (word breaks or combinations) to make on the original term(s). Default=1" and accepts any int. With a maximum of 0, suggestWordCombinations returns nothing, but suggestWordBreaks still returns the one-break suggestions that a maximum of 1 would give, scored 1 (one break): with sun, flower, seed and flowerseed indexed, suggestWordBreaks("sunflower") returns [sun, flower]; a negative value behaves the same. The reason is that suggestWordBreaks always starts its search at one break and reads maxChanges only before trying a second one. The default and every value of 1 or more are not affected by this defect, Solr's stock integration never passes a value below 1, and the only effect is extra suggestions. It dates from the class's first commit (LUCENE-3523, 2012), reproduces on 17 releases from 4.0.0-ALPHA to 10.5.2 and on main (10.6.0 has the same file, by reading), and a one-line early return, like the existing one for maxSuggestions < 1, fixes it.

• WordBreakSpellChecker (lucene-suggest, org.apache.lucene.search.spell) offers two kinds of suggestions: suggestWordBreaks(term, ...) splits one term into words that exist in the index, and suggestWordCombinations(terms, ...) joins adjacent terms into one indexed word.
• maxChanges (default 1): "The maximum numbers of changes (word breaks or combinations) to make on the original term(s)". A suggestion's score is its number of changes: for breaks the javadoc says "The scores returned are equal to the number of word breaks needed".

1. suggestWordBreaks calls generateBreakUpSuggestions(term, ir, 1, ...): the break count starts at 1 whatever maxChanges is (WordBreakSpellChecker.java:109-118).
2. Phase 1 of that method tries every split position and offers each pair of indexed words to the result queue (:268-290). maxChanges is not read here.
3. Only then does it compute newNumberBreaks = numberBreaks + 1 and return if newNumberBreaks > maxChanges (:298-303). That check only stops a second break.
4. suggestWordCombinations instead bounds its loop with j - i <= maxChanges (:186): with 0 the loop never runs, so no combination.

Index sun, flower, seed, flowerseed (each a single token); sunflower and sunflowerseed are not indexed:

Values 1 and 2 cap the breaks as set (rows 1 and 2); every value below 1 returns what 1 returns. (A separate bug in the same check, the full-queue early return, can drop better suggestions at maxChanges 3 or more; it has its own fix.)

• Who. Direct Lucene API users of WordBreakSpellChecker who set maxChanges to 0 or a negative value and still call suggestWordBreaks: for example an application that maps a "no changes" setting onto one instance used for both methods, or that computes the value and can reach 0. The setter accepts it silently.
• Not affected by this defect. The default (1) and every value of 1 or more; upstream tests use only values of 1 or more. Solr's word-break spell checker calls setMaxChanges only for values above 0 and turns breaks off with a separate breakWords option, so it cannot be triggered through the stock Solr setting (from reading Solr's source); custom code that calls Lucene directly can.
• Effect. Extra spelling suggestions: the one-break suggestions that maxChanges 1 would give, each scored 1. The index is unchanged; what the extra suggestions do downstream depends on the caller (for example an application that rewrites queries from them). No exception or hang occurred in any run, and the work is the same as with the default of 1, because phase 1 is bounded by the term length and maxEvaluations.
• Scale. Deterministic at any index size: a call that returns one-break suggestions at maxChanges 1 returns the same ones at 0 or below (the usual mode, frequency, length and result-count limits still apply).
• How likely. Rare: it needs a deliberate value below 1, and no matching public report since 2012 was found.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: suggestWordBreaks(ab) with maxChanges=0 expected:<0> but was:<1>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:647)
       org.apache.lucene.search.spell.TestWordBreakMaxChangesZero.testMaxChangesZeroAllowsNoWordBreak(TestWordBreakMaxChangesZero.java:46)

• Contract. setMaxChanges: "The maximum numbers of changes (word breaks or combinations) to make on the original term(s). Default=1" (WordBreakSpellChecker.java:441-443); getMaxChanges: "Returns the maximum number of changes to perform on the input" (:395-396). No minimum, no exception for breaks, no validation (:447-449).
• Lucene's own score. "The scores returned are equal to the number of word breaks needed" (:65-67); at maxChanges 0 the returned words have score 1.0.
• Sibling method. suggestWordCombinations treats the same field as a true upper bound including 0 (:186), unchanged since 2012.
• Same method, other limits. maxSuggestions below 1 returns nothing (:81-83), and maxEvaluations 0 returns before the first split (:269).
• Upstream test. TestWordBreakSpellChecker.java:294-316: "onethousandonehundredeleven" needs 4 breaks; at setMaxChanges(3) the test asserts 0 suggestions, at 4 one. So maxChanges is meant to be able to leave nothing.
• Explicit floors look different. minBreakWordLength 0 is floored on purpose with Math.max(minBreakWordLength, 1) (:254; in 2012, if (useMinBreakWordLength < 1)). Nothing floors maxChanges; its "floor" is only where the check happens to sit.
• History. The 2012 original already checked if (newNumberBreaks <= maxChanges) only around the recursive call, after offering the split; LUCENE-4587, #12077 ("now correctly respects maxEvaluations") and the #12100 rewrite kept that shape. No issue, PR, CHANGES or MIGRATE entry discusses values below 1.

The strongest works-as-designed case (the cross-model lenses and the devil's advocate rated it moderate): since 2012 maxChanges has acted as a depth limit after a first split that always happens; the class's author also wrote the Solr integration, which never passes a value of 0 or less and has a separate breakWords switch; the class silently floors another knob; and a 2014 solr-user@ mail by the author describes maxChanges loosely as "the maximum number of words it can break 1 word into ... reasonable to set this to 1 or 2". On that reading, values below 1 are unsupported, and a caller who wants no breaks should not call suggestWordBreaks.

Why it fails: no javadoc, test, CHANGES or MIGRATE entry states a minimum; the setter counts word breaks as changes; Lucene scores the returned suggestion as one break; the same field is a true bound at 0 in the sibling method; and the one floor the class has is explicit code. The case does bear on the resolution: documenting and validating maxChanges >= 1 is a reasonable maintainer choice, which is why the bug ranks low. Verdict: not intended; an unvalidated boundary value that one of the two methods ignores.

Two code fixes were built and run in a scratch worktree at a9779ef3c193; a javadoc-only option is listed for completeness. A is recommended because it makes the two methods agree without changing anything for values of 1 or more.

Extend the existing early return at the top of suggestWordBreaks (WordBreakSpellChecker.java:81) and say in the setter's javadoc what a value below 1 means. +3 lines.

if (maxSuggestions < 1 || maxChanges < 1) {
  return new SuggestWord[0][0];
}
...
/**
 * The maximum numbers of changes (word breaks or combinations) to make on the original term(s).
 * Default=1
 *
 * <p>A value below 1 allows no change: neither method then returns a suggestion.
 *
 * @see #getMaxChanges()
 */

Trade-offs

• Values of 1 or more never reach the new condition, so their results are unchanged; values below 1 now do less work than today.
• A caller that relied on 0 still giving one-break suggestions would get none. Nothing upstream or in Solr does.
• For values below 1 it also returns before the suggestMode and sortMethod defaults and before ir.docFreq(term), so the reader is not touched: a closed or failing reader that used to raise an I/O error there would not. The existing maxSuggestions < 1 return already behaves this way.

What the checks validated

Gradle in a scratch worktree at a9779ef3c193, seed DEADBEEF: the whole :lucene:suggest:test with the three repro classes added, 34 classes, 246 tests, 0 failures (7 skipped by the module itself), with assertions on and off; the 5 repro tests that fail on stock pass, and upstream TestWordBreakSpellChecker passes 5 of 5. :lucene:suggest:check -x test with the fix and the primary test added passes (google-java-format, ecj lint, forbidden APIs).

setMaxChanges throws IllegalArgumentException("maxChanges must be at least 1, got " + maxChanges) and its javadoc says "Must be at least 1". A related precedent in the same package: DirectSpellChecker.setMaxEdits documents "This value can be 1 or 2" and range-checks its argument, throwing UnsupportedOperationException("Invalid maxEdits") below 1 or above 2 (DirectSpellChecker.java:107-118). Fix B uses IllegalArgumentException, so it does not copy that exception type.

What was run (not a full validation). Gradle, same worktree and seed, assertions on only: all 31 upstream suggest test classes (236 tests) pass, so no upstream test sets a value below 1. The run as a whole failed (240 tests, 3 failures): in the unchanged primary repro the 3 tests that set 0 or -1, the combinations control included, now get the exception. No test that asserts the new exception was written, and B was not run with assertions off. :lucene:suggest:check -x test passes.

public void setMaxChanges(int maxChanges) {
  if (maxChanges < 1) {
    throw new IllegalArgumentException("maxChanges must be at least 1, got " + maxChanges);
  }
  this.maxChanges = maxChanges;
}

Trade-offs

Also changes suggestWordCombinations: setMaxChanges(0) works there today as "no combinations", and such callers would start to fail. That needs a CHANGES entry and suits a major release better than a bug-fix release. Solr is unaffected (it never passes values of 0 or less).

Say in the javadoc that suggestWordBreaks always considers one break and that maxChanges limits only further breaks.

Trade-offs

Keeps the two methods inconsistent under one setting. Not run (no code change).

Method: bounded-exhaustive translation validation (the hunt's "semi-formal" tier): an independent, exact reference of suggestWordBreaks (every split of the term into indexed words with at most maxChanges breaks, score = number of breaks) compared with the stock jar on every query up to a small length, over randomly generated small indexes and configurations.

• Scope: 200 random indexes over the strings of length 1-6 over {a, b}, all 510 queries of length 1-8, and 12 random configurations with maxChanges 0-4: 1,224,000 checks, run on the earlier main ea57360912ed.
• Result: 148,130 mismatches, all at maxChanges = 0 (this bug), plus 984 at maxChanges 3-4 that are a separate bug (the full-queue early return). A mutated reference produced mismatches as well, so the comparison was not vacuous.
• The first unit left the maxChanges 0 lead unpromoted as "degenerate configuration, low value". A later pass over executed leads re-ran it on main and reduced it to a 3-word public-API JUnit test. An independent reproducer wrote its own test from the claim alone.