Skip to content

fix(editor): stop keystrokes being erased and filter the completion popup in place (#1608)#1611

Merged
datlechin merged 2 commits into
mainfrom
fix/1608-editor-typing-clobber
Jun 7, 2026
Merged

fix(editor): stop keystrokes being erased and filter the completion popup in place (#1608)#1611
datlechin merged 2 commits into
mainfrom
fix/1608-editor-typing-clobber

Conversation

@datlechin
Copy link
Copy Markdown
Member

Fixes #1608.

Problem

On 0.49.x, typing in a new query tab could erase each character right after it was typed, drop focus on every keystroke, and leave the completion popup stuck open without filtering. The reporter sees it on macOS 15.7.7 (M1); it rarely reproduces on macOS 26 because the trigger is a SwiftUI render-ordering race.

Root causes

1. Stale binding snapshot clobbering the editor (regression from #1592, shipped in 0.49.0).
SourceEditor.updateNSViewController pushes the binding value into the editor via syncBindingText -> TextView.setText. The query tab binding getter captured a value-type QueryTab snapshot, so a render that runs after a keystroke but before the parent rebuilds the binding hands syncBindingText the pre-edit text. setText then wipes the typed character, removes the text view's subviews (the per-keystroke focus loss), and clears the undo stack. TablePro also still had its own older push-down in SQLEditorView (.onChange(of: text) -> replaceCharacters), so two sync paths competed.

2. Completion popup restart loop (pre-existing).
Every keystroke cancelled the in-flight completion request and closed/reopened the popup, because the filterable context is only set after the async fetch completes. Type faster than the 50ms debounce and the context never exists, so the popup never filters.

Fix

  • syncBindingText skips while isUpdateFromTextView is set: a render carrying a stale binding snapshot can never overwrite an edit the text view just made.
  • Deleted the duplicate .onChange(of: text) push-down in SQLEditorView; the package coordinator is the single owner of binding-to-editor sync.
  • queryTextBinding getter reads live from tabManager by tab id instead of a captured struct, removing the staleness at its source.
  • SuggestionViewModel.cursorsUpdated keeps the popup open while a fetch is in flight and updates items in place; it only closes when there is nothing to filter and nothing pending.
  • SQLCompletionAdapter seeds a keyword-only context synchronously before the debounce (new CompletionEngine.keywordCompletions(), no schema I/O), so the first keystroke already filters, and uses the live cursor position after the debounce instead of the stale trigger offset.

Tests

  • 2 new regression tests in SourceEditorBindingSyncTests (the stale-snapshot test fails on the old code).
  • New SuggestionCursorsUpdatedTests with 4 cases covering keep-open-while-fetching, in-place filtering, close-when-idle, and text view changes.
  • All 6 TablePro autocomplete suites pass (366 cases). swiftlint lint --strict clean on changed files.

Notes

  • Known trade-off: Clear and Format now apply through setText, so they are not undoable with Cmd+Z.
  • Manual check on macOS 15 hardware is worth doing before release since the race does not reproduce reliably on macOS 26.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7658f74bfc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

/// whole-document replacement. `setText` clearing the undo stack matches the
/// new-document semantics of that replacement.
func syncBindingText(_ newValue: String, controller: TextViewController) {
guard !isUpdateFromTextView else { return }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Cancel pending writeback before skipping sync

When a large document edit is still in the 150 ms debounced writeback window, isUpdateFromTextView remains true and this guard skips any external binding update such as Clear/Format. Because the pending textBindingTask is not cancelled on this path, that task can later write the pre-clear text back into the binding, so the programmatic change is dropped or reverted; the old duplicate SQLEditorView push-down was removed in this commit, so syncBindingText is now the only path that can apply the external change.

Useful? React with 👍 / 👎.

@datlechin datlechin merged commit d0153ce into main Jun 7, 2026
4 checks passed
@datlechin datlechin deleted the fix/1608-editor-typing-clobber branch June 7, 2026 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

App unresponsive when typing a new query

1 participant