Conversation
This branch has not been deployed
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.
Typing a word quickly while the first completion request of a session is still out presents a list ranked for the letters typed before the request was read, so Return commits the wrong keyword:
selthen Return gaveset. This is whatEditorAutocompleteFocusUITests/testCommittedKeywordTakesTheTypedCasehas been catching.Evidence
testCommittedKeywordTakesTheTypedCasefailed its first run and passed on retry in 7 of 127 executions, twice on main (35812418402 on 3f827b0, 36121207600 on c4833e6) and on five unrelated PRs (35494222457, 35654778508, 35709500014, 35836071911, 36135765290). Every failure is the first iteration right after launch: "Typing 'sel' and accepting should commit 'select'; got 'set'".testPopupReturnsAfterARequestWithNoMatchesfailed the same way once (35532237312, "got 'set'").First Runfailed andRetry 1passed for that case.["set", "select"]withsetselected.Cause
SuggestionViewModelwithholds cursor moves while a request is in flight and never replays them.sopens a session:showCompletionsasksQueryCompletionAdapter.completionSuggestionsRequested, which waits its 50 ms debounce, reads the cursor and text, and awaitsservice.completions. The first request after launch is the slow one.eandlarrive throughcursorsUpdated. Nothing is presented yet, so the re-rank is skipped (isPresentedis false, since fix(editor): end a completion session that presented nothing so the popup can open again #2921), andguard itemsRequestTask == nil else { return }drops the move.sorse), andshowCompletionsassigns it toitemswith row 0 selected, without looking at where the cursor is now.completionWindowApplyCompletiontakes its replacement range from the live cursor, soselis replaced withset.The adapter's own comment on
completionOnCursorMovesays a list ordered for an earlier prefix commits the wrong item on Return; this path presented one without going through it. It shipped: the unreconciled presentation is in v0.75.0 and every release before it (git tag --containson #1611 and #2921).Change
SuggestionViewModel.showCompletionsreconciles the answer with the live cursor before presenting it. If the cursor is still where the delegate answered (windowPosition), the list is shown as returned. If it moved, the list comes fromcompletionOnCursorMovefor the live cursor, the same call an open popup makes on every keystroke, and when that has nothing the session ends instead of showing a stale list. Comparing againstwindowPositionrather than the trigger position matters: the adapter reads the cursor after its debounce, so an empty-prefix list after.stays as it is.QueryCompletionAdaptersessions remember the token they were opened for, andcompletionOnCursorMovedeclines a cursor in another token. Before, it re-ranked the old context's candidates by whatever word the cursor was in, so a request forsthat resolved afterse uwould have offered keywords foru. With the popup open this was unreachable by typing, because a space or.empties the token and closes it on the way; the in-flight path skips those intermediate moves, so it needs the check. It also closes the popup on Option-Right into the next word and on Backspace over the.that opened a column list, where it used to re-rank in the wrong context.Tests
SuggestionLiveCursorTests(TableProEditorKit, headless,swift test): a delegate that parks on a continuation, so the ordering is deterministic. The window reports itself key, the patternSuggestionSessionLifetimeTestsalready runs on CI.test_showCompletions_ranksForTheKeysTypedWhileTheRequestWasOut: the request is opened fors,eandlgo throughcursorsUpdatedwhile it is out, then it answers["set", "select"]. Expects["select"]presented and selected. On the old code it presents["set", "select"]withsetselected (4 assertions fail). Reverting the reconciliation inshowCompletionsturns it red.test_showCompletions_endsTheSessionWhenNothingMatchesWhereTheCursorMoved: nothing ranks for the live cursor. Expects no presentation and the session ended. On the old code the stale list is presented (5 assertions fail).test_showCompletions_presentsTheAnswerAsIsWhenTheCursorStayedPut: the cursor did not move, so the answer is shown untouched andcompletionOnCursorMoveis never asked. Passes on old and new code; it turns red if the reconciliation is made unconditional, which would end every empty-prefix session after..QueryCompletionAdapterLifecycleTests/sessionDeclinesACursorOnAnotherToken(TableProTests): a session opened onsere-ranksselto["select"]and returns nil forse u. Removing thetokenStartguard incompletionOnCursorMoveturns it red (run locally: 17 executed, 1 failed).Verification
swift test --build-system native --package-path Packages/TableProEditor: 174 XCTest cases, 0 failures, 2 skipped; the 504 Swift Testing tests pass exceptGutterNumberOffsetTests"A window's gutter keeps its margin and its room to grow", which fails the same way on this machine with the change stashed and passes on CI (Package Tests for fix(datagrid): report a query tab's first run in the status bar, and put the six quarantined UI cases back on the gate #3130 on 2026-09-25), so it is local rendering, not this change.verify.sh test QueryCompletionAdapterLifecycleTests: 17 executed, 17 passed.swiftlint lint --stricton the four Swift files: 0 violations.