Skip to content

fix(editor): stop the autocomplete list answering for keys it cannot act on - #2932

Merged
datlechin merged 5 commits into
fix/filter-autocomplete-triggerfrom
fix/autocomplete-collateral
Sep 16, 2026
Merged

datlechin merged 5 commits into
fix/filter-autocomplete-triggerfrom
fix/autocomplete-collateral

Conversation

@datlechin

Copy link
Copy Markdown
Member

Four defects found while rewriting #2927, reported there and deliberately left out of it. Stacked on that branch, so review #2927 first; the diff here is only the five commits on top.

Cmd+Return ran nothing while the autocomplete list was open

SuggestionController.handleKeyDown switched on the key code alone and returned nil in every arm but default:, so the panel answered for Cmd+Return, Cmd+Shift+Return, Cmd+Option+Return and Shift+Tab as readily as for bare Return.

That matters because of where it is consulted. The editor installs one NSEvent.addLocalMonitorForEvents(matching: .keyDown) (TextViewController+Lifecycle.swift:168), and a local monitor that returns nil stops the event before the main menu. Measured in a standalone AppKit harness with a menu item bound to Cmd+Return:

monitor returns nil (consumes): ["MONITOR"]
monitor returns the event:      ["MONITOR", "MENU"]

So Execute Query never fired while the list was up. Shipped in v0.74.0.

The same switch swallowed keys it could not act on

SuggestionViewModel.showCompletions presents with no emptiness guard and SuggestionContentView renders a "No Completions" row for that state, so the panel can be on screen with nothing in it. MongoCompletionService reaches it: it guards the unfiltered pool and then returns the filtered shown, which is empty whenever the typed prefix matches nothing. Type db.users.zz in a MongoDB tab and every letter re-presents an empty panel where Return, Tab, Up and Down all did nothing and Escape was the only way out. Elasticsearch routes to the same service (editorLanguage = .javascript), where Mongo's vocabulary matches almost nothing a user types, so it is more reachable there than on MongoDB.

Both are now one rule. SuggestionKeyPolicy.outcome(forKeyCode:modifiers:hasSelection:) passes a chord carrying a modifier through untouched, and with nothing selected it dismisses rather than swallowing. hasSelection is fed by model.selectedItem, which already guards an out-of-range index, and is the same vocabulary FilterValueTextField.suggestionCommandOutcome landed with in #2927.

The empty-list arm dismisses rather than passing through. Pass-through would need InlineSuggestionManager changed in the same commit, because the coordinator loop runs before the panel link in one claimKeyDown call: a passed-through Tab does not re-enter the AI inline suggestion, it falls to handleTab and inserts a literal tab over live ghost text. Dismissing fixes the complaint without that coupling.

The editor's panel still preselects its first row and still takes Return when it has one. That is deliberate and differs from the filter field, where nothing is selected until an arrow key: in a code editor Return inserts a newline that Cmd+Z undoes, in a one-line filter it is the field's only action.

The completion engine dropped comparisonColumn

CompletionEngine.getCompletions rebuilt the analyzer's SQLContext by hand to move prefixRange into document coordinates, and the copy named twelve of the thirteen fields. comparisonColumn is the last field of the struct and the last parameter of its init, the signature of a field added after the copy was written.

Latent rather than user-visible: the one reader, SQLCompletionProvider.allowedValueItems, runs on the analyzer's own context before the rebuild, and the re-rank path never asks. The fix is structural rather than one added argument: SQLContext.replacingPrefixRange(_:) sits beside the existing replacingTableReferences(_:), which already carried the field correctly, so the engine no longer enumerates fields at all.

The trigger rule ran after the work it could have skipped

Both surfaces applied SQLCompletionTriggerPolicy.suppressesEmptyPrefix after awaiting the engine, so a suppressed keystroke had already built and ranked several hundred candidates, taken an actor hop for the schema, and on a cold column cache could issue a database round trip, all to throw the result away.

SQLCompletionProvider.completionSession splits into analyzedContext(text:cursorPosition:forcedTableReferences:), which is synchronous and builds nothing, and completionSession(for:). CompletionEngine.getCompletions asks the rule between them. Both callers drop their own guard.

The rule now takes SQLCompletionTrigger rather than a Bool. The default is .explicit, not .automatic, so a forgotten argument opens the list rather than silently closing it: 25 existing CompletionEngineTests call sites ask the engine directly at positions the automatic rule suppresses, including SELECT * FROM users WHERE . isManualTrigger: Bool stays the vocabulary at the QueryCompletionService boundary, because it originates in TableProEditorKit's CodeSuggestionDelegate; SQLCompletionService converts.

SQLCompletionServiceEmptyPrefixTests is the proof the move is outcome-identical: it asserts both automatic == nil and manual != nil for three suppressed clauses, and it needed no edit.

Not taken: a synchronous shouldOffer gate on the filter provider, to close the popup before the 50ms debounce instead of after it. It would re-add a second decision path over the same fragment on the one surface whose whole bug history is two decision paths disagreeing, and #2927 deleted shouldOfferTokenCompletion for that reason one commit earlier. Since #2927 also made Return independent of whether a popup is up, what remains is an already-open list staying open for 50ms.

The CHANGELOG over-claimed the double quote

- No autocomplete after an opening backtick or double quote, in the editor and the grid filter field.

The backtick half is true on both surfaces. The double-quote half was false when it was written. SQLContextAnalyzer.isInsideString toggles on every unescaped ", so an opening one makes analyze report the cursor inside a string literal and the provider early-returns. Every test #2923 added uses a backtick; not one uses a double quote, and its own unterminatedDoubleQuoteReadsAsAString asserts the opposite of what the entry claims. Narrowed to the backtick.

Making the double quote work is a real change and not this one: it needs the analyzer to know the dialect's identifier quote, so " opens a string on MySQL and quotes an identifier on PostgreSQL. Worth its own issue.

Validation

  • verify.sh build: PASS
  • verify.sh test over the ten affected suites: 432/432 PASS
  • swift test --package-path Packages/TableProEditor: SuggestionKeyPolicyTests 5/5, EditorKeyMonitorCompositionTests 10/10 unchanged
  • verify.sh lint over all 13 changed files: 0 violations
  • docs/: no page needs a change, checked. docs/features/autocomplete.mdx describes a populated list and stays accurate.

The app tests were run in a detached worktree at this commit, because this checkout is shared with another session whose half-written test file fails to compile and would otherwise read as a failure here.

InlineSuggestionManager has no caller-side test. SuggestionController.model is kit-internal and TableProTests imports the kit non-@testable, so the app target cannot stage a presented empty panel. Nothing in this PR changes that file; the note is here so the gap is not silent.

Package tests are outside verify.sh. Only the packages job of macos-tests.yml runs Packages/TableProEditor, so a green verify.sh on this PR would not have run a single assertion about the key fix. Run by hand, result above.

Found and not fixed

  • CompletionEngine.filterCompletions maps replacementRange back by -prefixLength but forwards a context whose prefixRange is still in "WHERE " + fragment coordinates. SQLCompletionService.completions has the same mismatch on its own window, up to 5,000 units. Both are latent because the only reader asks isEmpty, which is shift-invariant.
  • SuggestionViewModel.showCompletions clears isPresented synchronously while a request ending through endSession never closes the window, so a superseded request can leave a visible panel with isPresented == false and no key routing at all. A worse version of the bug fixed here.
  • CompareTableScopeEditor builds a RawSQLFilterCompletionProvider for any endpoint with no isSQLDialect gate, unlike FilterPanelView. It is the one place a MongoDB endpoint reaches the SQL clause rule.

@datlechin
datlechin merged commit aa168de into fix/filter-autocomplete-trigger Sep 16, 2026
9 of 14 checks passed
@datlechin
datlechin deleted the fix/autocomplete-collateral branch September 16, 2026 20:07
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.

1 participant