Serialize spell checks and drop results the document outran

[Wavesonics]

Follow-up to the spell-check investigation in #93. That PR fixed why a right-click on a squiggle opened the wrong menu; this one fixes the other half, why spell check sometimes doesn't run at all after navigating out and back in.

What was wrong

SpellCheckState had no serialization between checks. Each individual check was already careful about cancellation (compute under suspension, then swap spans with no suspension points), but nothing stopped two checks being alive at once, and nothing noticed the document changing while a check was suspended in its lookups.

Three consequences:

1. Two full checks run concurrently at init. rememberSpellCheckState runs one when the checker resolves, and a host with its own re-check effect (the sample app has one, for an Android import race) runs another. Both interleave suspending isCorrectWord calls into a single spell-checker session, and every word gets looked up twice.

2. A stale full check wipes a newer partial check. A full check snapshots its candidates, suspends across N lookups, and on resume clears misspelledWords and swaps in spans computed against the pre-edit document. Anything the debounced partial check installed in the meantime is discarded, the reinstalled ranges are stale, and misspelledWords ends up out of step with the spans on the document, so handleSpanClick finds no segment and the right-click falls back to the plain menu. On a large document behind an IPC spell checker, N lookups comfortably exceeds the 500ms debounce.

3. The atomic swap held by convention only. "No suspension points between removal and re-add" is true on the main dispatcher but nothing stated or enforced it.

What changed

• A Mutex serializes runFullSpellCheck, runPartialSpellCheck and checkWordSegment.
• Every check records the document hash it computed over and re-checks it before installing spans. A full check recomputes (up to MAX_FULL_CHECK_ATTEMPTS, then leaves the edited ranges to the debounced partial checks); a partial check drops its result, since the edit that invalidated it schedules its own.
• A full check that queued behind an equivalent one (same text, same checker instance, completed while this caller waited) is skipped. Keyed on "completed after I asked" so a later deliberate refresh over identical text still runs.
• Main-dispatcher confinement is now stated on the class, since the non-suspending entry points can't take a suspending lock and the spans are Compose state.

Tests

Four cases in SpellCheckStateTest; three fail against the pre-change code:

• a full check that outlived an edit re-runs against the new text (2 stale spans vs 3 correct ones)
• two checks never run against the spell checker at once
• a check queued behind an identical one does not scan the document again
• a later check over the same text still runs (guards the skip from becoming a footgun; passes either way by design)

./gradlew check is green.

Not addressed here

The sample app's extra LaunchedEffect is now harmless rather than actively racing, but it's still a workaround for setText not emitting an edit operation, so the library never learns its document was replaced. Fixing that at the source is a separate change.

getSuggestions still hits the spell checker outside the lock. Locking it would block the context menu behind a full document check, which is a worse trade for a hazard that is so far unconfirmed.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 16 complexity · 2 duplication

Metric	Results
Complexity	16
Duplication	2

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[Wavesonics]

Code review findings

High-effort review of this branch against main. 34 candidates, 8 refuted, 10 distinct defects confirmed or plausible. Drafting this PR until the guard design is reworked.

They collapse to two root causes.

The de-dup key is under-specified

spellCheckMode is not part of the key (SpellCheckState.kt:203, CONFIRMED)
A host that sets spellCheckMode = Sentence and calls runFullSpellCheck() while the init Word check is in flight gets its request swallowed: the Word check completes over the same text, bumps completedFullChecks, and the queued sentence request sees satisfiedWhileWaiting. sentenceCorrections stays empty until the document is edited.

The bookkeeping stamps the current checker, not the one that ran (SpellCheckState.kt:214, CONFIRMED)
runFullWordCheck captures sp at entry and does every lookup through it, but the completion stamp re-reads the mutable spellChecker field. A check started with C1 that finishes after a swap to C2 credits C2 with C1's results, so C2's own full check is skipped and the document keeps the previous language's squiggles.

The guards are whole-document; the work they gate is range-local or queued

Partial checks snapshot the revision after the mutex wait (SpellCheckState.kt:329, CONFIRMED)
The range argument is computed before the call blocks on checkMutex, but revision is read after the lock is granted. Edits that land during the wait are invisible to the guard, so the stale range is used: spans are stripped from a region the user did not edit and wordSegmentsInRange clamps silently and squiggles the wrong words. This re-admits the exact corruption the PR set out to prevent, through the queuing the mutex introduces.

A whole-document hash discards results that were still valid (SpellCheckState.kt:342, also :377, CONFIRMED)
An edit in paragraph 5 drops paragraph 1's computed misspellings even though its ranges were untouched. invalidateSpellCheckSpans already stripped paragraph 1's spans, and the next debounce batch only covers paragraph 5, so the misspelling is left with no squiggle and no scheduled re-check. The comment's claim that "the edit that invalidated it schedules its own check" holds only for the newly edited range.

checkWordSegment returns "misspelled" without installing the span (SpellCheckState.kt:411, CONFIRMED)
On a hash mismatch the guard skips the span swap while the return value stays unconditional. The caller is told the word is misspelled, no SpellCheckStyle span is added, nothing is appended to misspelledWords, and a right-click there gets no suggestions. Nothing re-checks that word.

The retry loop abandons the document (SpellCheckState.kt:279, CONFIRMED)
After MAX_FULL_CHECK_ATTEMPTS the check returns null and no code path ever calls runFullSpellCheck again: its callers are the one-shot LaunchedEffect(spellChecker) and setSpellCheckingEnabled. Typing into a long document against a slow checker during the init check leaves the untouched bulk of the document with no squiggles for the rest of the session. The previous code always installed a result, so whole-document coverage was guaranteed even when the result was stale.

requestedHash is captured before the lock (SpellCheckState.kt:196, PLAUSIBLE)
setText emits no edit operation, so the debounced collector never sees a programmatic load and only an explicit runFullSpellCheck covers it. If importMarkdown's setText lands while such a request is queued, the request wakes comparing a hash the document no longer has, sees satisfiedWhileWaiting, and returns. The freshly loaded document gets zero squiggles.

Remaining

Partial checks queue behind up to three full re-scans (SpellCheckState.kt:224, PLAUSIBLE)
The retry loop holds checkMutex across complete re-scans, so squiggles for freshly typed text arrive seconds late, and if the user keeps typing during the wait the partial check is then dropped by its own guard.

getSuggestions calls the checker outside the mutex (SpellCheckState.kt:97, PLAUSIBLE)
Known and noted in the PR description. It means the "lookups never interleave against a single session" invariant the mutex KDoc claims is not actually enforced: a right-click during a full check runs suggestions/isCorrectWord concurrently with the scan.

The full-check log line sits outside the retry loop (SpellCheckState.kt:242, also :291, cleanup)
The document can be scanned three times per call while the line prints once, so the log understates the work done.

Direction

The de-dup key fixes are mechanical: add the mode, record sp. The rest wants a different guard: a monotonic revision counter on TextEditorState, captured at request time and carried through the mutex wait, with partial checks validating their own range rather than the whole document, and the exhausted-retry path scheduling a retry rather than returning. Tests for mode switch, checker swap, and a partial check queued behind a full one should come first.

A smaller alternative: keep the Mutex alone and drop every revision guard and retry loop. That fixes the duplicate init check and the interleaving, carries none of the dropped-work findings above, and leaves the stale-swap race for a later change.