FreeTextSuggester: a rebuild that throws resets the unigram token total, so the previous model keeps serving with every unigram scored 1.0

FreeTextSuggester (lucene/suggest, @lucene.experimental) predicts the next word with an n-gram model and stupid backoff; a single word's score is its count divided by totTokens, the number of tokens the model was built from (FreeTextSuggester.java:568, :680-687 on main 645d4fe14a20). build() sets the field totTokens = 0 (:273) before it reads the first entry and adds to it while it builds the new FST (:315-317), but installs fst and count only after success (:328-329, since LUCENE-10292 in 9.2.0). So a rebuild that throws, on an empty or stop-words-only dictionary ("need at least one suggestion"), an IOException from the input, an analyzer exception or build()'s own rejection of a token with the separator byte, keeps the previous FST and getCount() but scores it against a total of 0 or a partial total. With 0 every single-word suggestion scores Long.MAX_VALUE (1.0) and a backed-off word outranks the real bigram predictions: for the model {foo bar baz, foo bar, foo bee, fun}, lookup("f") goes from foo 0.375, fun 0.125 to 1.0 and 1.0, and lookup("foo b") ranks baz 1.0 first instead of 0.05 last. No error; store() writes the zero total (:355), so a reloaded copy is damaged the same way; the next successful build() repairs it. Every release from 4.6.0 through 10.6.0 and main. Accumulating the total in a local and publishing it next to fst and count fixes it (+3/-3).

• FreeTextSuggester: a Lookup that "uses the stupid backoff language model to smooth scores across ngram models" (FreeTextSuggester.java:93-95 on main 645d4fe14a20); default bigrams (:116-117), default separator byte 0x1e between the words of an n-gram (:147-151), @lucene.experimental (:103). "The value is Long.MAX_VALUE * score ... Divide by Long.MAX_VALUE to get the score back, which ranges from 0.0 to 1.0" (:97-99).
• ALPHA: "if a given trigram did not occur, and we backoff to the bigram, the overall score will be 0.4 times what the bigram model would have assigned" (:121-126).
• totTokens: a field (:134); for single-word predictions lookup() uses it as the denominator, long contextCount = totTokens; (:568); for longer n-grams it uses the context's own count from the FST (:572-581).

1. build(InputIterator, double) rejects payloads and contexts first (:247-252; those leave the model intact), then sets totTokens = 0; (:273) and reads the input into a temporary index (:276-284).
2. After that point it can throw: no terms gives "need at least one suggestion" (:288-290), a term with too many grams gives "tokens must not contain separator byte" (:305-314), and an IOException from next() or the analyzer propagates. (The second "need at least one suggestion" check, :325-327, is a defensive guard: once there are terms, every term goes into the FST, and FSTCompiler.compile() returns null only for an FST that accepts nothing, FSTCompiler.java:1002-1005.)
3. While the terms move into the new FST, single words add to the field: totTokens += termsEnum.totalTermFreq(); (:315-317). Only after success: fst = newFst; count = newCount; (:328-329).
4. A failure leaves the old FST and count with a total of 0, or with the part of the new input counted before the failure.
5. The value is (long) (Long.MAX_VALUE * backoff * ((double) decodeWeight(completion.output())) / contextCount) (:680-687): the n-gram's count from the FST over contextCount. With contextCount = 0 the double is Infinity and the cast gives Long.MAX_VALUE, so every single word scores 1.0. The backoff factor no longer helps (0.4 x Infinity is still Infinity), so the backed-off baz scores 1.0 and sorts first for "foo b"; foo and fun tie at 1.0 and come out in key order (:695-705).
6. store() writes totTokens (:355) and load() reads it back (:377), so a copy saved after the failed build keeps the damage. The next successful build() repairs it.

Model {foo bar baz, foo bar, foo bee, fun}: single-word counts foo 3, bar 2, baz 1, bee 1, fun 1 (total 8); pairs foo bar 2, foo bee 1, bar baz 1.

A separator-byte rejection leaves a partial total that depends on the new input. With the hunt's new input "foo<U+001E>bar baz" the rejection comes after "baz" was counted, so the total is 1: foo and fun still score 1.0, and baz scores 0.4, above foo bee. Other inputs leave other partial totals, which can also lower the scores (an independent reproducer's input doubled its total: cat 0.1 and cow 0.05 instead of 0.2 and 0.1).

• Who is hit. Code that rebuilds a serving FreeTextSuggester in place and keeps using it after build() threw. Solr's suggest component keeps serving after a failed rebuild: SuggestComponent.buildSuggesterIndex catches the exception, logs it and keeps serving the same Lookup, which SolrSuggester.build rebuilt in place. Solr stores the lookup only after build() returned normally, so it shows the serving case, not the persistence case. Solr users have posted this exact "need at least one suggestion" failure, one from a rebuild on commit (Stack Overflow 71582458) and one from a build at startup (solr-user 2018-08-03), without looking at the scores.
• Default or opt-in. Opt-in: one Lookup among several, @lucene.experimental. Within it, the default bigrams and separator are affected.
• Effect. Silent wrong scores and ranking: every single word at 1.0 when the total is 0, a backed-off word first, and with a partial total scores off in either direction. No exception; since 9.2.0 getCount() is unchanged, so nothing tells the application. Assertions on or off make no difference.
• Persistence. store() after the failed build writes the damaged total; the next successful build() repairs it.
• Scale. None needed: 4 entries, one thread.
• How likely: rare. It needs a rebuild that throws and continued use of that instance; the recorded 2026-10-29 search found no public report of these scores.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => org.junit.ComparisonFailure: expected:<[foo/[3458764513820540928, fun/1152921504606846976]]> but was:<[foo/[9223372036854775807, fun/9223372036854775807]]>
       org.junit.Assert.assertEquals(Assert.java:117)
       org.junit.Assert.assertEquals(Assert.java:146)
       org.apache.lucene.search.suggest.analyzing.TestFreeTextSuggesterFailedRebuild.testRejectedRebuildKeepsUnigramScores(TestFreeTextSuggesterFailedRebuild.java:37)

• The method keeps the rest of the previous model. fst and count are installed only after success (FreeTextSuggester.java:328-329). LUCENE-10292 made that change: "Fix other FST based suggesters so that getCount() returned results consistent with lookup() during concurrent build()" (lucene/CHANGES.txt:4289-4291, 9.2.0). On #11328 hossman (they) described the other suggesters as having "atomically replaced their internal structures (typically FSTs) at the end of their build() process" and wrote "I evidently overlooked that FreeTextSuggester has this same code pattern" (about count, which their commit then fixed). totTokens belongs to the same model (store() writes it next to fst and count) and was not touched.
• The model's own semantics break. Stupid backoff (:93-95) and ALPHA (:121-126) cannot give two words 1.0 each for "f" or a backed-off word 1.0. The state is neither the old model, a new one, nor empty.
• Public triggers. InputIterator.EMPTY is a public constant (InputIterator.java:57); next() declares IOException (BytesRefIterator.java:32-34); the empty and separator rejections come from build() itself. In the separator case only the new input is invalid, yet the previous, valid model is damaged.
• Tests state the intent but check no scores. SuggestRebuildTestUtil "confirms that assertions which pass after an initial build will continue to pass during a (slow) rebuild" (SuggestRebuildTestUtil.java:33-35); TestFreeTextSuggester.testLookupsDuringReBuild checks only getCount() and keys (:98-119), testIllegalByteDuringBuild only that a fresh instance throws (:121-134). Of the tests that use FreeTextSuggester (TestFreeTextSuggester and the @Ignored TestLookupBenchmark, by grep), none looks up after a failed build().
• History. The early reset is the class's first version (2013), kept through LUCENE-5404, LUCENE-10292 and a 2025 refactor; the commits and discussions inspected record no rationale for it.

The intended-behaviour checker and a reviewer from another model family (OpenAI GPT-5.6) built the strongest case: no javadoc promises failure atomicity (Lookup.build says only "Builds up a new internal {@link Lookup} representation", Lookup.java:178-182) and the class is @lucene.experimental; the documented 0.0-1.0 range is not broken, because the cast saturates at 1.0; a separator byte in a token is a documented precondition violation ("No input tokens should have this byte, otherwise IllegalArgumentException is thrown", FreeTextSuggester.java:176-181) and an empty dictionary is rejected; maintainers said on #11328 that concurrent build and lookup is not guaranteed (dweiss (they): "Concurrency concerns should be handled by the code that uses Lookups"); and the open #5558 favours building a new instance and swapping it in.

Why it fails: the code does keep the previous fst and count, and Lookup.build still rebuilds in place, so the gap is one field LUCENE-10292 did not cover. The precondition explains the exception, not the damage to the previous valid model, and the empty-input and IOException triggers are not misuse. The concurrency remark applies only to the secondary lookups-during-rebuild symptom; the main case is single-threaded. @lucene.experimental concerns API compatibility, not correctness. The contract question stays open, though: Lookup.build promises no failure atomicity, so a maintainer may treat the fix as an improvement rather than a bug fix. Every check that ran the code found a real bug, and every check that rated priority rated it low; of the reading-only cross-model reviews, one said real (medium confidence), the other returned a procedural "not real" because it could not run the code.

One fix was built and validated; three alternatives were considered and not built.

In build(), replace totTokens = 0; with long newTotTokens = 0;, add to the local while the terms move into the FST, and assign totTokens = newTotTokens; between fst = newFst; and count = newCount;. A failed build then leaves all three fields of the previous model as they were.

-      totTokens = 0;
-
+      long newTotTokens = 0;
       long newCount = 0;
 ...
-            totTokens += termsEnum.totalTermFreq();
+            newTotTokens += termsEnum.totalTermFreq();
 ...
         fst = newFst;
+        totTokens = newTotTokens;
         count = newCount;

Trade-offs

• Three plain field writes, as today's fst and count publication: not one atomic snapshot, so a lookup on another thread that races the publication can still pair the new FST with the old total for an instant.
• It also fixes lookups made while a rebuild is still reading its input (the hunt's concurrent test passes with it).
• No cost on the lookup path.

What the checks validated

• Scratch worktree of 645d4fe14a20, gradle, OpenJDK 25: the repro test fails on the stock tree and passes with the fix, with assertions on and off. These gradle test runs used an earlier copy of the test whose one comment line was longer; it was shortened for google-java-format, with no code change, and check and the standalone runs used the final file.
• With the fix and the test, the whole lucene/suggest module passes with assertions on and off: 32 suites, 237 tests, 7 skipped, 0 failures (TestFreeTextSuggester: 11 tests, 1 skipped).
• :lucene:suggest:check -x test passes (google-java-format, ecjLint, forbiddenApis).
• Earlier, the hunt's 5 tests (every trigger, store() + load(), lookups during a rebuild) passed 5 of 5 with the same change, and its bounded-exhaustive fault-injection check found 0 mixed states.

Save totTokens before the reset and put it back in a catch before rethrowing. Not built.

Trade-offs

Same single-threaded result with more code, and lookups during a successful rebuild would still see the reset total.

Hold fst, totTokens and count in one final object published by a single volatile write and read once per lookup() / store(). Not built.

Trade-offs

Also closes the publication race and would cover the load() counterpart, but touches lookup(), store(), load() and getCount(): larger than the bug needs.

Javadoc only. Not recommended.

Trade-offs

Leaves silent wrong scores in place, while the method already keeps the rest of the previous model.

Method: non-formal first signal, then semi-formal: a planning probe followed by a bounded-exhaustive fault-injection check (the hunt's hunt unit for suggest lookups whose failed build or load leaves mixed state, 2026-10-28, main a9779ef3c193).

• Lead. A planning probe for exception safety rebuilt a built FreeTextSuggester from an empty input, a separator-byte input and a failing input on the stock jars (assertions on and off): after the empty and the failing input the single-word values turned to 9223372036854775807, and store() + load(), checked after the failing input, kept them; its separator-byte input was accepted without an exception (a separate bug, LFV-600). The same on the branch_10_6 RC jars.
• Exhaustive check. A translation-validation harness on the stock jar: 25 failure kinds (empty and stop-words-only input, an IOException or RuntimeException from next() at each call, a separator token at each position, an analyzer IOException at each input, and 3 controls that throw before any state changes: payloads, contexts, a negative RAM buffer) x 3 previous states (built, loaded from a stored copy, fresh) = 75 failed builds, plus a deterministic hand-off at every next() call of a concurrent rebuild; 312 checks per assertion mode. All 22 non-control kinds left a mixed state on the built and on the loaded suggester (44 of 75), the controls none, and 4 of 4 hand-off points were mixed; 0 with the fix.
• JUnit. Reduced to a public-API JUnit; this doc reduced it further to one test.