Skip to content

Add native NSSpellCheck and WinSpellCheck spellchecking support#330

Merged
RyeMutt merged 5 commits into
developfrom
feature/native-spellcheck-backends
Jun 27, 2026
Merged

Add native NSSpellCheck and WinSpellCheck spellchecking support#330
RyeMutt merged 5 commits into
developfrom
feature/native-spellcheck-backends

Conversation

@RyeMutt

@RyeMutt RyeMutt commented Jun 27, 2026

Copy link
Copy Markdown
Member

Description

Related Issues

  • Please link to a relevant GitHub issue for additional context.
    • Bug Fix: Link to an issue that includes reproduction steps and testing guidance.
    • Feature/Enhancement: Link to an issue with a write-up, rationale, and requirements.

Issue Link:


Checklist

Please ensure the following before requesting review:

  • I have provided a clear title and detailed description for this pull request.
  • If useful, I have included media such as screenshots and video to show off my changes.
  • I have tested the changes locally and verified they work as intended.
  • All new and existing tests pass.
  • Code follows the project's style guidelines.
  • Documentation has been updated if needed.
  • Any dependent changes have been merged and published in downstream modules
  • I have reviewed the contributing guidelines.

Additional Notes

RyeMutt and others added 3 commits June 26, 2026 23:03
Split the monolithic Hunspell-backed LLSpellChecker into a backend-agnostic core
plus a pluggable engine, so additional spell-check backends can be added without
duplicating the dictionary-metadata, persistence, and orchestration logic.

- New llspellcheckengine.h: a small abstract engine interface
  (setLanguage(name)/isActive/checkWord/getSuggestions/getInstalledLanguages)
  plus a build-time-selected create() factory and a shared matchLanguage() helper
  for the OS-list backends. An engine knows only the active primary language,
  addressed by the dictionaries.xml basename; the custom/ignore/glossary
  "accepted words", metadata, persistence, and the change signal stay in
  LLSpellChecker.
- llspellcheck.cpp holds the one copy of the shared logic and a lowercased
  mAcceptedWords set (an unordered_set) consulted case-insensitively in
  checkSpelling. refreshDictionaryMap resolves all primary dictionaries in a
  single getInstalledLanguages() batch (one OS query). The engine is held behind
  a unique_ptr<LLSpellCheckEngine>; ~LLSpellChecker is defined out-of-line.
- llspellcheck.h is platform-clean: it only forward-declares the engine.
- llspellcheckengine_hunspell.cpp is the default engine (wraps Hunspell). CMake
  always compiles the core and selects exactly one engine.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add USE_NSSPELLCHECKER (DARWIN-only, default OFF) which compiles
llspellcheckengine_mac.mm instead of the Hunspell engine and links AppKit +
Foundation. The engine maps the viewer's dictionary names to NSSpellChecker
language codes (via the shared LLSpellCheckEngine::matchLanguage helper) and
answers checkWord/getSuggestions via the shared system checker. The SL glossary
and user custom/ignore words are handled by LLSpellChecker's accepted-word set,
so they never touch the user's global system dictionary. include(Hunspell) is
skipped when this engine is selected, so a macOS-native build no longer requires
Hunspell.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add USE_WINSPELLCHECK (WIN32-only, default OFF) which compiles
llspellcheckengine_win32.cpp instead of the Hunspell engine and links ole32. The
engine owns an apartment-threaded COM apartment for its lifetime, creates an
ISpellCheckerFactory, maps the viewer's dictionary names to BCP-47 language tags
(via the shared LLSpellCheckEngine::matchLanguage helper), and answers
checkWord/getSuggestions via ISpellChecker (Check + IEnumSpellingError, Suggest
with S_FALSE treated as no suggestions). As with the macOS engine, the SL glossary
and user words live in LLSpellChecker's accepted-word set, and include(Hunspell)
is skipped for a Windows-native build.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@RyeMutt, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 5 minutes and 23 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c38d16fe-dd45-4671-9832-fad0ae82a916

📥 Commits

Reviewing files that changed from the base of the PR and between a8483f5 and c27faac.

📒 Files selected for processing (3)
  • indra/llui/llspellcheck.cpp
  • indra/llui/llspellcheckengine_hunspell.cpp
  • indra/llui/llspellcheckengine_win32.cpp
📝 Walkthrough

Walkthrough

The build now selects Hunspell, macOS NSSpellChecker, or Windows spell checking through CMake options and backend modules. LLSpellChecker now uses a shared engine interface, with platform-specific engine implementations and updated llui wiring for sources, links, and tests.

Changes

Spellcheck backend selection

Layer / File(s) Summary
Build options and feature gating
indra/CMakeLists.txt, indra/vcpkg.json
Adds macOS and Windows spell-check build options, conditionally enables Hunspell feature wiring, and moves hunspell into an optional vcpkg feature.
Native backend module targets
indra/cmake/Copy3rdPartyLibs.cmake, indra/cmake/Hunspell.cmake, indra/cmake/NSSpellChecker.cmake, indra/cmake/WinSpellCheck.cmake
Wraps Hunspell staging/discovery in native-backend checks and adds imported CMake targets for the macOS and Windows spellcheck backends.
Engine interface and Hunspell backend
indra/llui/llspellcheckengine.h, indra/llui/llspellcheckengine_hunspell.cpp
Adds the LLSpellCheckEngine interface and the Hunspell-backed implementation that loads dictionaries, checks words, and returns suggestions.
Native macOS and Windows engines
indra/llui/llspellcheckengine_mac.mm, indra/llui/llspellcheckengine_win32.cpp
Adds the NSSpellChecker and Windows COM spellcheck engines, including language matching, word checks, suggestions, and installed-language queries.
llui spellcheck wiring
indra/llui/CMakeLists.txt
Moves llspellcheck.cpp into a dedicated spellcheck source block, exports llspellcheckengine.h, and selects the backend source and link libraries from the active option.
LLSpellChecker engine-backed flow
indra/llui/llspellcheck.h, indra/llui/llspellcheck.cpp
Replaces direct Hunspell state with LLSpellCheckEngine, rebuilds accepted words, and routes spell-check activation, checks, and suggestions through the engine.

Sequence Diagram(s)

sequenceDiagram
  participant LLSpellChecker
  participant LLSpellCheckEngine
  participant LLHunspellEngine
  participant LLNSSpellEngine
  participant LLWinSpellEngine

  LLSpellChecker->>LLSpellCheckEngine: create()
  alt USE_NSSPELLCHECKER
    LLSpellCheckEngine-->>LLSpellChecker: LLNSSpellEngine
  else USE_WINSPELLCHECK
    LLSpellCheckEngine-->>LLSpellChecker: LLWinSpellEngine
  else Hunspell
    LLSpellCheckEngine-->>LLSpellChecker: LLHunspellEngine
  end
  LLSpellChecker->>LLSpellCheckEngine: setLanguage(dict_language)
  LLSpellChecker->>LLSpellCheckEngine: checkWord(word)
  LLSpellChecker->>LLSpellCheckEngine: getSuggestions(word, suggestions)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I hop through engines, light and keen,
Mac, Win, and Hunspell in between.
New words I sniff, new paths I test,
A twitchy nose approves the quest.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is still the template placeholder and does not describe the changes or include a real issue link. Replace the template text with a real summary of the change, add a related issue link, and note any testing or caveats.
Docstring Coverage ⚠️ Warning Docstring coverage is 26.32% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding native macOS and Windows spellchecking support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@RyeMutt RyeMutt force-pushed the feature/native-spellcheck-backends branch from 9e9efd8 to 8b7d8ca Compare June 27, 2026 04:31
@RyeMutt RyeMutt force-pushed the feature/native-spellcheck-backends branch from 8b7d8ca to a8483f5 Compare June 27, 2026 05:10

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
indra/llui/llspellcheck.cpp (1)

298-336: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

addToCustomDictionary can append duplicate words to the on-disk .dic.

addToIgnoreList dedups against mIgnoreList before writing, but addToCustomDictionary writes to the custom .dic and inserts into mAcceptedWords unconditionally. Re-adding the same word across calls/sessions grows the file with duplicates (the set dedups in memory, the file does not). A guard against re-adding already-accepted words keeps the file consistent with the ignore-list behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/llui/llspellcheck.cpp` around lines 298 - 336, addToCustomDictionary
currently writes every word straight to the custom dictionary file and
mAcceptedWords without checking for existing entries, so repeated calls can
duplicate entries on disk. Update LLSpellChecker::addToCustomDictionary to
mirror the dedup behavior used in addToIgnoreList by checking whether the
lowercased word is already present in mAcceptedWords or the custom dictionary
before calling addToDictFile, then only append and signal settings when it is
new.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@indra/llui/llspellcheckengine_hunspell.cpp`:
- Around line 92-106: `LLHunspellEngine::getInstalledLanguages` currently
reports a language as installed when only the `.dic` file exists, but
`LLHunspellEngine::setLanguage` requires both `.dic` and `.aff` to successfully
create `Hunspell`. Update the availability check in `getInstalledLanguages` to
verify both files for each candidate name in both the user and app dictionary
paths, so only languages that can actually be activated are returned.

In `@indra/llui/llspellcheckengine_win32.cpp`:
- Around line 73-107: Add the same main-thread thread-affinity check used
elsewhere in LLWinSpellEngine so the constructor and destructor only run on the
main thread; update LLWinSpellEngine::LLWinSpellEngine and
LLWinSpellEngine::~LLWinSpellEngine to assert on_main_thread() before any COM
lifetime work, keeping CoInitializeEx/CoUninitialize paired on the same
apartment and making the singleton teardown path explicit.

---

Nitpick comments:
In `@indra/llui/llspellcheck.cpp`:
- Around line 298-336: addToCustomDictionary currently writes every word
straight to the custom dictionary file and mAcceptedWords without checking for
existing entries, so repeated calls can duplicate entries on disk. Update
LLSpellChecker::addToCustomDictionary to mirror the dedup behavior used in
addToIgnoreList by checking whether the lowercased word is already present in
mAcceptedWords or the custom dictionary before calling addToDictFile, then only
append and signal settings when it is new.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8a24ad1e-37f8-4dce-aa3c-867c51e978a0

📥 Commits

Reviewing files that changed from the base of the PR and between 7963260 and a8483f5.

📒 Files selected for processing (13)
  • indra/CMakeLists.txt
  • indra/cmake/Copy3rdPartyLibs.cmake
  • indra/cmake/Hunspell.cmake
  • indra/cmake/NSSpellChecker.cmake
  • indra/cmake/WinSpellCheck.cmake
  • indra/llui/CMakeLists.txt
  • indra/llui/llspellcheck.cpp
  • indra/llui/llspellcheck.h
  • indra/llui/llspellcheckengine.h
  • indra/llui/llspellcheckengine_hunspell.cpp
  • indra/llui/llspellcheckengine_mac.mm
  • indra/llui/llspellcheckengine_win32.cpp
  • indra/vcpkg.json

Comment thread indra/llui/llspellcheckengine_hunspell.cpp
Comment thread indra/llui/llspellcheckengine_win32.cpp
@RyeMutt RyeMutt merged commit 325137c into develop Jun 27, 2026
17 checks passed
@RyeMutt RyeMutt deleted the feature/native-spellcheck-backends branch June 27, 2026 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant