[ty] Add some completion ranking improvements#20807
Merged
MichaReiser merged 15 commits intomainfrom Oct 15, 2025
Merged
Conversation
Contributor
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
Contributor
|
9415542 to
a788c47
Compare
Contributor
|
AlexWaygood
reviewed
Oct 10, 2025
a788c47 to
94d68c4
Compare
Contributor
|
I planned to take a look at this, but it looks like we have two reviews already. |
This means that when new characters are typed, the LSP client should send new completion requests to the LSP server. Previously, only one request would be emitted and then the client would filter them. The primary reason to do this is so the server can take what has been typed into account when returning completions. ty is already doing this with auto-import, because it isn't really feasible to do otherwise: always returning *every* symbol throughout an environment isn't a good way to do business. But, we likely also want to take what is typed into account for all completion requests so that we can do our own ranking. Moreover, if we take what is typed into account when doing a completion request, then consider a request that was initiated with `fubar` already typed. If the user hits backspace and gets `fub`, then since the LSP client isn't sending another request, the user won't see all matching completions. This is why we need to mark the response as "incomplete." This will instruct the LSP client to re-send completion requests. The trade-off is that the client will make more completion requests. And if completions are slow, this will magnify that effect for users.
Instead of only extracting this for scoped completions using the AST, we now extract the typed text for all requests in a uniform way directly from the token stream. This doesn't otherwise change any behavior.
This doesn't do any ranking or sorting, but it does filter our completions by the text the user has typed so far. We were already doing this for auto-import. For non-auto-import completions, the number of symbols tends to be much smaller, so I don't really expect this to result in any meaningful performance improvements. This does mean our eval score gets a lot better, and indeed, the eval reflects the real world more accurate since LSP clients do their own filtering.
I had the logic flipped here, which resulted in `show-one` not matching expected tasks in some cases. Basically, when the `--file-name` and `--index` flags aren't given, then the CLI should behave as if all file names and indices are matched.
This is mostly a basic sanity test that we can return completions inside of the interpolated segments of f-strings and t-strings.
This had been annoying me for a while: completions shouldn't be offered inside of comments. We detect this by looking for a `Comment` token before the offset (with only `NonLogicalNewline` being allowed as an intermediate token).
... except when we're in the interpolated segments of an f-string or a t-string. Previously, we would show completions for interpolated segments of an f-string or a t-string. Which is good. But we did this by virtue of never trying to suppress completions when in the non-interpolated part of an f-string/t-string, or any other kind of string. Whcih is bad. Here, we do our best to detect whether the cursor is within a string or not. This seems somewhat straight-forward in the case of a string that is properly opened/closed. But for a string that isn't closed yet---which is likely common while typing---this is less straight-forward. For the most part, I looked at some examples and tried to reverse engineer the kinds of patterns we see in the token stream when there is an unclosed string. It's likely not fully correct, but we can improve it as we go.
Since `None` is a keyword, it isn't currently included in completions. But this is a common papercut that is really annoying, and actually makes typing `None` into the playground nearly impossible. So let's try to fix this.
This doesn't implement fully context aware keyword completions, but it does let us include the 3 keywords that are also values in completions. This helps improve the completion experience when trying to write one of these values in the ty playground. Ranking could still be improved here. We haven't yet devised a mechanism for ranking "builtin" symbols above others. It's even plausible that these _particular_ symbols should get special treatment over and above others.
94d68c4 to
7bd9ee2
Compare
Member
|
Hmm, I think the evaluation results differ between mac and linux? |
AlexWaygood
reviewed
Oct 15, 2025
Member
|
Sorry @BurntSushi, I'll squash-merge the commits because @AlexWaygood and I don't have as good a commit hygiene as you. |
dcreager
added a commit
that referenced
this pull request
Oct 15, 2025
…rable * origin/main: [ty] Add (unused) `inferable` parameter to type property methods (#20865) Run macos tests on macos (#20889) Remove `release` CI job (#20887) [ty] CI: Faster ecosystem analysis (#20886) Remove `strip` from release profile (#20885) [ty] Sync vendored typeshed stubs (#20876) [ty] Add some completion ranking improvements (#20807) Improved error recovery for unclosed strings (including f- and t-strings) (#20848) Enable lto=fat (#20863) [`pyupgrade`] Extend `UP019` to detect `typing_extensions.Text` (`UP019`) (#20825) [`flake8-bugbear`] Omit annotation in preview fix for `B006` (#20877) fix(docs): Fix typo in `RUF015` description (#20873) [ty] Improve and extend tests for instance attributes redeclared in subclasses (#20866) [ty] Ignore slow seeds as a temporary measure (#20870) Remove parentheses around multiple exception types on Python 3.14+ (#20768) Update Black tests (#20794)
dcreager
added a commit
that referenced
this pull request
Oct 15, 2025
…nt-sets * dcreager/non-non-inferable: (174 commits) [ty] Add (unused) `inferable` parameter to type property methods (#20865) Run macos tests on macos (#20889) Remove `release` CI job (#20887) [ty] CI: Faster ecosystem analysis (#20886) Remove `strip` from release profile (#20885) [ty] Sync vendored typeshed stubs (#20876) [ty] Add some completion ranking improvements (#20807) Improved error recovery for unclosed strings (including f- and t-strings) (#20848) Enable lto=fat (#20863) [`pyupgrade`] Extend `UP019` to detect `typing_extensions.Text` (`UP019`) (#20825) [`flake8-bugbear`] Omit annotation in preview fix for `B006` (#20877) fix(docs): Fix typo in `RUF015` description (#20873) [ty] Improve and extend tests for instance attributes redeclared in subclasses (#20866) [ty] Ignore slow seeds as a temporary measure (#20870) use existing method Remove parentheses around multiple exception types on Python 3.14+ (#20768) Update Black tests (#20794) just the api parts [ty] Fix further issues in `super()` inference logic (#20843) [ty] Document when a rule was added (#20859) ...
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.
This PR contains a few improvements for completions:
generating completions in all cases. Previously, we only did this for
auto-import completions.
None,TrueandFalseare now included inscoped completions.
f-strings and t-strings.
We add some new evaluation tasks to capture some of these cases. (And
lots of tests for the string detection logic.)
Note: I did make some changes to the
TokenFlagsrepresentation.I'm open to other ideas. But I wanted to flag this since
TokenFlagsis used within the representation of a
Token, which is a fundamentaldata type inside of Ruff. Thankfully, even though
TokenFlagsgetsbigger,
Tokenitself does not (because of padding/alignment).