Repository navigation
fix(editor): stop the autocomplete list answering for keys it cannot act on - #2932
Merged
datlechin merged 5 commits intoSep 16, 2026
Merged
Conversation
…tion engine tests
datlechin
merged commit Sep 16, 2026
aa168de
into
fix/filter-autocomplete-trigger
9 of 14 checks passed
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
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.
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+Returnran nothing while the autocomplete list was openSuggestionController.handleKeyDownswitched on the key code alone and returnednilin every arm butdefault:, so the panel answered forCmd+Return,Cmd+Shift+Return,Cmd+Option+ReturnandShift+Tabas readily as for bareReturn.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 returnsnilstops the event before the main menu. Measured in a standalone AppKit harness with a menu item bound toCmd+Return: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.showCompletionspresents with no emptiness guard andSuggestionContentViewrenders a "No Completions" row for that state, so the panel can be on screen with nothing in it.MongoCompletionServicereaches it: it guards the unfiltered pool and then returns the filteredshown, which is empty whenever the typed prefix matches nothing. Typedb.users.zzin a MongoDB tab and every letter re-presents an empty panel whereReturn,Tab,UpandDownall did nothing andEscapewas 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.hasSelectionis fed bymodel.selectedItem, which already guards an out-of-range index, and is the same vocabularyFilterValueTextField.suggestionCommandOutcomelanded with in #2927.The empty-list arm dismisses rather than passing through. Pass-through would need
InlineSuggestionManagerchanged in the same commit, because the coordinator loop runs before the panel link in oneclaimKeyDowncall: a passed-throughTabdoes not re-enter the AI inline suggestion, it falls tohandleTaband 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
Returnwhen it has one. That is deliberate and differs from the filter field, where nothing is selected until an arrow key: in a code editorReturninserts a newline thatCmd+Zundoes, in a one-line filter it is the field's only action.The completion engine dropped
comparisonColumnCompletionEngine.getCompletionsrebuilt the analyzer'sSQLContextby hand to moveprefixRangeinto document coordinates, and the copy named twelve of the thirteen fields.comparisonColumnis 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 existingreplacingTableReferences(_:), 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.suppressesEmptyPrefixafter 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.completionSessionsplits intoanalyzedContext(text:cursorPosition:forcedTableReferences:), which is synchronous and builds nothing, andcompletionSession(for:).CompletionEngine.getCompletionsasks the rule between them. Both callers drop their own guard.The rule now takes
SQLCompletionTriggerrather than aBool. The default is.explicit, not.automatic, so a forgotten argument opens the list rather than silently closing it: 25 existingCompletionEngineTestscall sites ask the engine directly at positions the automatic rule suppresses, includingSELECT * FROM users WHERE.isManualTrigger: Boolstays the vocabulary at theQueryCompletionServiceboundary, because it originates inTableProEditorKit'sCodeSuggestionDelegate;SQLCompletionServiceconverts.SQLCompletionServiceEmptyPrefixTestsis the proof the move is outcome-identical: it asserts bothautomatic == nilandmanual != nilfor three suppressed clauses, and it needed no edit.Not taken: a synchronous
shouldOffergate 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 deletedshouldOfferTokenCompletionfor that reason one commit earlier. Since #2927 also madeReturnindependent 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.isInsideStringtoggles on every unescaped", so an opening one makesanalyzereport 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 ownunterminatedDoubleQuoteReadsAsAStringasserts 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: PASSverify.sh testover the ten affected suites: 432/432 PASSswift test --package-path Packages/TableProEditor:SuggestionKeyPolicyTests5/5,EditorKeyMonitorCompositionTests10/10 unchangedverify.sh lintover all 13 changed files: 0 violationsdocs/: no page needs a change, checked.docs/features/autocomplete.mdxdescribes 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.
InlineSuggestionManagerhas no caller-side test.SuggestionController.modelis kit-internal andTableProTestsimports 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 thepackagesjob ofmacos-tests.ymlrunsPackages/TableProEditor, so a greenverify.shon this PR would not have run a single assertion about the key fix. Run by hand, result above.Found and not fixed
CompletionEngine.filterCompletionsmapsreplacementRangeback by-prefixLengthbut forwards a context whoseprefixRangeis still in"WHERE " + fragmentcoordinates.SQLCompletionService.completionshas the same mismatch on its own window, up to 5,000 units. Both are latent because the only reader asksisEmpty, which is shift-invariant.SuggestionViewModel.showCompletionsclearsisPresentedsynchronously while a request ending throughendSessionnever closes the window, so a superseded request can leave a visible panel withisPresented == falseand no key routing at all. A worse version of the bug fixed here.CompareTableScopeEditorbuilds aRawSQLFilterCompletionProviderfor any endpoint with noisSQLDialectgate, unlikeFilterPanelView. It is the one place a MongoDB endpoint reaches the SQL clause rule.