feat: macOS 15.x spell-check fallback — engine-driven underlines#66
Draft
Ender-Wang wants to merge 6 commits into
Draft
feat: macOS 15.x spell-check fallback — engine-driven underlines#66Ender-Wang wants to merge 6 commits into
Ender-Wang wants to merge 6 commits into
Conversation
## Summary - On macOS 15.x the system's own TextKit 2 continuous-spell-check pass neither writes nor paints `.spellingState` rendering attributes on custom `NSTextLayoutFragment` subclasses (confirmed by the harness attached to nodes-app#59: red=0 on both arms, byte-identical PNGs). This left `isContinuousSpellCheckingEnabled` silently no-op in the editor on 15.x. - Drive `NSSpellChecker.requestChecking(of:…)` asynchronously on the coordinator (400 ms debounce, no main-thread XPC stalls). Hits are filtered through the existing zone helpers (`isInsideCode`, `isInsideLatex`, `isInsideSpellcheckSuppressedToken`) so code, LaTeX, links, and image embeds stay clean. - Paint dotted-red underlines in `MarkdownTextLayoutFragment.draw(at:in:)` as step 7 of the draw pipeline. The cache is cleared **synchronously** on every `textDidChange` so no stale-offset underlines paint during the debounce window. - Version-gated: `#unavailable(macOS 26)` at the coordinator entry points keeps the driver a no-op on 26+ where AppKit's native pass works. No double underlines. - Uses `textView.spellCheckerDocumentTag` (not a private tag) so the system "Ignore Spelling" action clears the underline. - Print/PDF exclusion via `NSPrintOperation.current` — underlines are for on-screen editing only. - RTL-safe: computes absolute x1/x2, swaps when the text direction flips the order. - Theme integration: `MarkdownEditorTheme.misspellingUnderlineColor` (default `.systemRed`) so apps with a custom palette can override. - `MarkdownASTStyler` stamps `.spellingState: 0` on fenced code blocks and inline `code` spans, completing the existing suppression convention (belt-and-suspenders with the coordinator-side filter). Behavior toggles from nodes-app#36 (`SpellCheckingPolicy`, context-menu overrides, `userPrefersContinuousSpellChecking`) are unchanged — this PR only adds the missing render pass and routes through the same preference. Supersedes the render-pass portion of nodes-app#59.
NSSpellChecker.requestChecking calls back on NSTextCheckingOperationQueue (a background queue), not the main thread. The completion handler was setting textView.needsDisplay = true directly, which: 1. Triggered Main Thread Checker (UI API on background thread) 2. Silently failed to redraw, so underlines never reappeared after edits Wrap updateSpellMisspelledRanges in DispatchQueue.main.async. The filtering (pure computation over results) stays on the background queue; only the cache assignment + needsDisplay hop to main.
Two issues causing underlines to disappear after edits and reappear inconsistently: 1. requestChecking(of:…) completion handler unreliable on macOS 15.x — after the first call, subsequent completions on NSTextCheckingOperationQueue sometimes never fire, leaving the cache permanently empty after edits. Switch to synchronous checkSpelling(of:startingAt:) walk on a background queue — fast for typical notes and deterministically returns results. 2. System's own continuous spell-check pass races with the engine's driver — on macOS 15.7.7 the system paints .spellingState unreliably (sometimes after mount, sometimes after selection, rarely after edits), producing the inconsistent underlines seen in testing. Disable textView.isContinuousSpellCheckingEnabled when scheduling the engine's pass on 15.x so the engine is the sole painter.
Background-queue approaches had two fatal flaws on macOS 15.x: 1. requestChecking(of:…) completion handler sometimes never fired after the first call, leaving cache permanently empty. 2. DispatchQueue.global approach read textView.string (not thread- safe) and called shouldSuppressSpellMark which accesses cachedParsedDocument — a property the main thread mutates during restyles. This data race silently produced empty results after edits (underlines disappeared and never returned on insert). Run the entire checkSpelling(of:startingAt:) walk synchronously on the main thread. For typical notes (hundreds of words, a handful of misspellings) this takes single-digit milliseconds — well within the 400ms debounce window. Eliminates all threading issues.
This commit folds two iterations together:
1. Remove `textView.isContinuousSpellCheckingEnabled = false` toggle
that caused a recursive textDidChange cascade.
2. Add `[SC]` diagnostic prints + try four fragment-redraw strategies
to fix the on-insert underline disappearance — none worked.
--- Part 1: recursive cascade fix (kept) ---
Setting `textView.isContinuousSpellCheckingEnabled = false` in
`scheduleSpellCheck` caused a recursive cascade on macOS 15.x:
1. AppKit synchronously clears `.spellingState` attributes
2. Attribute change triggers `textDidChange` delegate
3. `textDidChange` calls `clearSpellMisspellings` (cache empty) +
`scheduleSpellCheck` (cancels the current debounced pass)
4. Original work item is cancelled before it fires.
Deletion masked the bug because by the time the user deletes, the
flag is already false, so re-setting is a no-op — no recursive
trigger. Insert reliably reproduced it on the first keystroke after
mount.
The system's own continuous spell-check pass is now left enabled.
On macOS 15.x it paints on the default fragment (which our custom
`MarkdownTextLayoutFragment` replaces), so its `.spellingState`
underlines are invisible. Our cache-based drawing in
`drawSpellMisspellings` is the sole visible painter.
This change is independent of the redraw issue described below;
even after removing it, the on-insert disappearance still occurs.
--- Part 2: redraw issue (still open) ---
Symptom on macOS 15.7.7:
- Open a note with misspellings -> underlines drawn correctly on
initial layout.
- Type any character -> all underlines disappear and never come back
until something forces a full layout re-run (panel show/hide,
select-all, big scroll, etc.).
- Delete a character -> underlines blink and reappear after the
400 ms debounce (works correctly).
- The 400 ms-debounced spell-check pipeline itself is verified
correct via `[SC]` diagnostic prints (left in for maintainer
review):
textDidChange -> update was=N now=0 -> schedule -> run -> done -> update was=0 now=N
The cache `spellMisspelledRanges` repopulates with the right count
after every keystroke. NSSpellChecker returns the same ranges as
before the edit.
Root-cause hypothesis:
TextKit 2 maintains a per-fragment rendering cache. After an
insert, only the edited paragraph's fragments redraw on the next
display pass; the other fragments containing misspellings reuse
their pre-update imagery (no underlines) even though
`coordinator.spellMisspelledRanges` is correct.
Attempted fixes (all rebuilt and tested — none worked for insert):
1. `textView.needsDisplay = true` (original) -> only edited
fragment redraws.
2. `setNeedsDisplay(fragment.layoutFragmentFrame)` per
overlapping fragment -> rect goes stale during typing bursts
as fragments shift.
3. `tlm.invalidateRenderingAttributes(for: textRange)` per
misspelled range -> silently no-ops; no `draw(at:in:)`
follows.
4. `tlm.invalidateLayout(for: textRange)` per misspelled range
-> only the range overlapping the edited fragment redraws
(e.g. {349,7} drawing 54 times in our log); other fragments
still reuse cached imagery.
In all four attempts, the diagnostic confirms
`coordinator.spellMisspelledRanges` is correctly repopulated; the
failure is purely in the redraw pipeline.
Diagnostics added (kept for maintainer review):
- NativeTextViewCoordinator+TextDelegate.swift:
`[SC] textDidChange:` entry
- NativeTextViewCoordinator+SpellCheck.swift:
`[SC] schedule:` (queued / toggle-off-clearing)
`[SC] run: start | empty | done rawHits=N kept=N tag=T`
`[SC] update: was=N now=N`
- MarkdownTextLayoutFragment.swift:
`[SC] draw frag={loc,len} cacheCount=N`
Repro recipe:
1. Build EdgeMark (or any TextKit 2 host) against this branch on
macOS 15.x.
2. Run with Console filtered by `[SC]`.
3. Open a note with several misspellings spread across paragraphs.
4. Insert one character; observe `[SC] update: was=0 now=N`
followed by draws covering only the edited fragment (and
possibly the prev paragraph from restyle scope), not the
fragments holding the other misspellings. Visually all
underlines vanish.
5. Select-all (or hide/show side panel). Logs show all relevant
fragments redraw; underlines reappear.
Open question for engine authors:
Is there a TextKit 2 invalidation API that reliably forces
fragments overlapping an arbitrary document range to re-execute
`draw(at:in:)` without re-laying out (which would clobber the
styler's `.spellingState: 0` stamps the styler just put down)? Or
is the recommended pattern to drive fragment redraw via
`setRenderingAttributes` (and have the spell-check underline live
as a rendering attribute rather than custom drawing)?
Local SPM dependency note:
This branch is being iterated against EdgeMark via a local SPM
path (`XCLocalSwiftPackageReference`); no Package.resolved pin
needed.
Ender-Wang
force-pushed
the
feature/spellcheck-15x-fallback
branch
from
017c9f9 to
bb10a96
Compare
Contributor
Author
🐛 Open issue on this branch: TextKit 2 fragment redraw on text insertHi @luca-chen198 @Nicolas-Py — before this branch is ready for review, I'm stuck on one rendering issue and would appreciate a hint on the right TextKit 2 invalidation API. The 400 ms-debounced spell-check pipeline itself is fully verified; the failure is purely in the redraw path. Repro environment: macOS 15.7.7, EdgeMark host app, this branch's Symptom
Pipeline is verified correctI added
What's actually brokenAfter Delete works because the engine's restyle scope happens to overlap more fragments, including the ones holding misspellings. Strategies I tried (all built, all failed for the insert path)I rebuilt and tested each one against the same repro:
In all four, Commits on this branch
Open question 🙏Is there a TextKit 2 invalidation API that reliably forces every fragment overlapping an arbitrary document range to re-execute Or is the recommended pattern to drive spell underlines via Happy to capture more diagnostics or try anything you'd like to see. The full Thanks for the engine — it's been a pleasure to build on. 🙇 |
Member
|
thx for the kind words means a lot. I will definitely take a look in the next days. Sorry for the wair we currently have much going on in the nodes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Close #58
On macOS 15.x the system's own TextKit 2 continuous-spell-check pass neither writes nor paints
.spellingStaterendering attributes on customNSTextLayoutFragmentsubclasses. The harness attached to #59 confirmed this is a system-wide TextKit 2 issue (red=0 on both arms, byte-identical PNGs for vanilla TK2NSTextViewand the engine's custom fragment), not something the subclass causes.This PR adds a version-gated engine-side fallback:
NSSpellChecker.checkSpelling(of:startingAt:…)walked on the main thread with a 400 ms debounce. Replaced the original asyncrequestChecking(of:…)after testing surfaced data races between the completion handler and the AST cache. Scheduled on text change, initial mount, node switch, and toggle-on.drawSpellMisspellings(at:in:)as step 7 ofMarkdownTextLayoutFragment.draw(at:in:)— dotted red, RTL-safe, print-excluded, theme-colored.textDidChangeBEFORE the debounced pass is scheduled, eliminating the bug class from Fix: Misspelling underlines never appear in the editor #59.isContinuousSpellCheckingEnabledtoggle: the system pass is left enabled. On 15.x it paints on the default fragment (which our custom fragment replaces), so its.spellingStateunderlines are invisible. Disabling it was tried and caused a recursivetextDidChangecascade (AppKit clears.spellingStatesynchronously → triggers delegate → cancels the debounced pass).#unavailable(macOS 26)at coordinator entry points. On 26+ the driver is a no-op (AppKit's native pass handles everything) — no double underlines.textView.spellCheckerDocumentTagso "Ignore Spelling" works; respectsSpellCheckingPolicytoggles from Persist user's spell/grammar toggle across caret movement #36.isInsideCode/isInsideLatex/isInsideSpellcheckSuppressedToken+.spellingState: 0styler stamps on fenced code blocks and inlinecodespans (completing the convention from feat: stamp.spellingState: 0on fenced code blocks and inlinecode#64).TextKit 2 fragment redraw on text insert. The 400 ms pipeline itself is verified correct via
[SC]diagnostic prints (kept on thebb10a96tip for review): on every keystroke the cachespellMisspelledRangesis correctly cleared and repopulated byNSSpellChecker. But after an insert, only the edited paragraph's fragments re-executedraw(at:in:). Fragments containing the other misspellings reuse their pre-update cached imagery (no underlines), so all visible underlines vanish until something forces a full layout pass (panel show/hide, select-all, large scroll). Delete works because the engine's restyle scope happens to overlap the misspelling fragments.Tried four fragment-redraw strategies, all on
bb10a96, all fail for the insert path:textView.needsDisplay = truesetNeedsDisplay(fragment.layoutFragmentFrame)per overlapping fragmenttlm.invalidateRenderingAttributes(for: textRange)per misspelled rangedraw(at:in:)followstlm.invalidateLayout(for: textRange)per misspelled range{349,7}drew 54× in the log); other fragments still serve cached imageryOpen question for maintainers: Is there a TextKit 2 invalidation API that reliably forces every fragment overlapping an arbitrary document range to re-execute
draw(at:in:)without re-laying out (which would clobber the styler's.spellingState: 0stamps and trigger an unwanted restyle)? Or should spell underlines live as a rendering attribute viasetRenderingAttributes(_:for:)rather than custom drawing — letting AppKit own the dirty-tracking? Any guidance on how the rendering-attribute path interacts withMarkdownTextLayoutFragmentoverrides would be very helpful.Files
Sources/MarkdownEngine/Configuration/MarkdownEditorTheme.swift— newmisspellingUnderlineColor: NSColor(default.systemRed).Sources/MarkdownEngine/TextView/Coordinator/NativeTextViewCoordinator+SpellCheck.swift— new file.scheduleSpellCheck(400 ms debounce, version gate) +runSpellCheckPass(synchronous main-threadcheckSpelling(of:startingAt:…)walk, zone filter).clearSpellMisspellingsresets the cache. Currently includes[SC]diagnostic prints + four-strategyupdateSpellMisspelledRangesfor maintainer review (will be stripped before final merge).Sources/MarkdownEngine/TextView/Coordinator/NativeTextViewCoordinator.swift—spellMisspelledRangescache,spellCheckWorkItem,didToggleSpellCheckingPolicycancel/schedule hook.Sources/MarkdownEngine/TextView/Coordinator/NativeTextViewCoordinator+TextDelegate.swift— synchronous cache-clear at top oftextDidChange,scheduleSpellCheckat end.[SC] textDidChange:diagnostic print at the top.Sources/MarkdownEngine/Renderer/MarkdownTextLayoutFragment.swift—drawSpellMisspellings(at:in:)as draw step 7.[SC] draw frag={loc,len} cacheCount=Ndiagnostic print.Sources/MarkdownEngine/TextView/NativeTextViewWrapper.swift— initialscheduleSpellCheckinmakeNSView, re-schedule on node switch inupdateNSView.Sources/MarkdownEngine/Styling/MarkdownASTStyler.swift—.spellingState: 0on fenced code blocks and inlinecodespans.Tests/MarkdownEngineTests/MarkdownASTStylerTests.swift—codeRegionsSuppressSpellCheck@test.CHANGELOG.md— 3 bullets under[Unreleased]/Added.Commits
08bcffafeat: macOS 15.x spell-check fallback — engine-driven underlines— initial fallbackaf2dc56fix: dispatch spell-check completion to main queue6ddf1bcchore: exclude .tmp/ from tracking (diagnostic harness artifacts)3dea6c8fix: switch to synchronous checkSpelling + disable system pass on 15.x083ec31fix: run spell-check pass on main thread to eliminate data racesbb10a96wip: TextKit 2 fragment redraw issue on insert (diagnostics included)— recursive-cascade fix + four invalidation attempts +[SC]diagnostic prints. Latest tip; needs maintainer input on the open issue above.Test plan
swift build— clean.swift test— 70 tests in 7 suites pass, includingcodeRegionsSuppressSpellCheck.paragrahh). A dotted red underline appears within ~400 ms. ✅code,$latex$,[link](url),![[image]]are NOT marked. ✅[SC]log) but the underline doesn't paint until the next layout-forcing event. See the open-issue section above. ❌Out of scope
checkSpelling(of:startingAt:…)passeslanguage: nil, soNSSpellCheckeruses the user's preferred languages.Supersedes the render-pass approach in #59 (abandoned after the harness confirmed the issue is system-wide).
Closes #58.