improve completions sorting #798

vezwork · 2025-08-22T22:31:46Z

I was originally planning to copy completions sorting code from VSCode, but it is incredibly complex and cannot be directly copied&pasted (because we probably don't want to rewrite CodeMirror's Autocompletion from scratch). If I was going to copy VSCode's completion code, at the very least I would first have to understand how CodeMirror Autocompletion API works...

So, in this PR I have

refactored the existing VE's completion sorting and filtering code for readability,
manually improved the VE's completions sorting in Positron and VSCode with the Python Jedi language server, and
accept completion with Tab.

Upon initial testing, the completions are good in both Python and R in Positron and VSCode with the Python Jedi language server. It may be best to go with this simple fix for now. In the future it would be good if we can understand more what is happening in VSCode completions, but I have realized that could be quite a bit of work outside of my domain.

juliasilge

Wow, this is turning out GREAT! 🤩

I am very supportive of going this direction and iterating to finetune the behavior in the future.

I do notice a few difference in R completions, but I am not sure with my current level of review whether we should change what happens in ark or here in this logic. Just in case you might already know, an immediate noticeable difference I am seeing is that R packages show up in the visual editor but not the source editor:

It's the bslib and jquerylib entries there.

vezwork · 2025-08-28T14:45:58Z

I do notice a few difference in R completions, but I am not sure with my current level of review whether we should change what happens in ark or here in this logic.

I am not sure either. If we can identify issues with R completions, I would now say it shouldn't be difficult to address them here.

Tangential discussion, but: note that since ark always gives like 2500+ completions, and since sorting and filtering is done in the UI process, processing completions is consistently slightly slow in R code and makes the UI feel slightly less responsive. I might try moving a bunch of the filtering to the LSP so its not in the UI process. I feel like ideally ark would do some basic filtering on their side(such as inclusion of what the user is typing in the proposed completions) for performance.

Just in case you might already know, an immediate noticeable difference I am seeing is that R packages show up in the visual editor but not the source editor:
[..]
It's the bslib and jquerylib entries there.

I noticed these! I didn't know what they were but I played around with them and they seem useful?

vezwork · 2025-08-28T14:47:31Z

I wonder if I should just do another quick pass over this then go to merge it. I can't think of another step forward. Maybe I should get someone who uses R in Positron to try it out.

juliasilge · 2025-08-28T14:57:12Z

I will have the ark team outline some changes we could make to the completions offered, but I think that slightly slow completions in R is totally fine for an initial PR. 👍

I would love to see if we could address those package/namespace completions that appear (i.e. bslib and jquerylib in this example). Right now, in Python, the completions look exactly the same in source and visual mode, but in R, bslib and jquerylib do not show up in source but do show up in the visual editor.

One other note, I would keep the completions turned off (i.e. that code that is commented out right now) until we can get Tab fixed up to accept a suggestion. It is pretty surprising not to be able to accept with Tab! 😅

I am totally fine to merge this in, if completions stay turned off! I think the things I would like to see before we turn completions on are:

address remaining big difference between source and visual mode (i.e. like bslib completion for lib)
accept completion with Tab

vezwork · 2025-08-28T14:59:41Z

Thank you @juliasilge, totally clarifying.

vezwork · 2025-08-29T19:59:02Z

I also have code based on this PR that fixes position miscalculations that result in snippets often being filtered out of completions. After I fixed that, I found out that snippets with nested fields are somewhat broken in codemirror's autocomplete extension. I raised an issue in their repo here: codemirror/dev#1607

vezwork · 2025-08-29T20:00:51Z

@juliasilge I have added

filtering for namespaces in completions (bslib and the like no longer show up), and
Tab-to-accept-completion support.

The PR is ready!

juliasilge · 2025-08-30T16:09:27Z

It looks like you may want to merge #801 into this branch. Do you want to do that before I review? Or an additional review afterwards? Just let me know!

vezwork · 2025-09-02T15:52:38Z

@juliasilge I feel that #801 is blocked on being able to upgrade to the last codemirror/autocomplete version. This version fixes snippets with nested fields (which seem to be common in R). I attempted to upgrade but failed due to Quarto monorepo things that I don't understand.

@juliasilge please review this one and we can merge it, then rebase #801 on main. I don't want #801 to block this one (they solve separate problems).

DavisVaughan · 2025-09-03T13:03:20Z

@juliasilge asked me to summarise a potential change we may make on the ark side regarding completions

It's mostly this comment: posit-dev/positron#4489 (comment)

With the TLDR being that completions can be thought of in two ways:

The initial set that ark sends over after a user types 3 characters or something like dplyr::. This results in an initial completion list pop up box. The frontend has a little control over this because it will fuzzy filter this initial set a bit before showing it to the user using the initial characters they typed.
The filtering behavior that is applied on that pop up box by the frontend when the user continues to type after the box has already appeared. Ark no longer has anything to do with this, and is not called again after it provides that initial set.

Right now Ark's initial set sends everything relevant over and we rely fully on the frontend for all filtering.

This results in a model where:

The initial set is fuzzy.
The filtering is fuzzy.

RStudio's model is:

The initial set is a (fairly) strict subset of what the user typed, i.e. typing data only brings up an initial set of things that start with data.
- The caveat being that trigger characters like the : of dplyr:: pop up the entire list of dplyr functions with no strict subsetting applied, because the user hasn't typed a function name yet. This ends up working well.
The filtering is fuzzy.

I think people would enjoy if Positron moved towards this model as well. We'd accomplish this by making Ark send over an initial set of completions that have been filtered to only a (roughly) strict subset of what the user has typed. So if the user has typed datae we'd send over dataentry() and data.entry(), allowing for some special non-alphanumeric characters like . to be fuzzed over in the initial set, but we'd no longer send over everything else in scope and rely on the frontend for all filtering.

The benefits are that we get a model that works a bit more like RStudio, which I do like.

The downsides are that ark itself is muddying the waters a bit about how completions should be filtered in Positron. There are a lot of existing options in Positron related to sorting and filtering completions, and if ark pre-filters completions then it might make the experience feel a bit inconsistent. But I don't think that is a huge worry.

juliasilge

This is such an incredible improvement! Having Tab set up to accept completions is such a relief. 😅 I think we should merge and move forward!

I do think we're going to get reports of differences in the completions in visual vs. editor mode, but we can log those and address them moving forward. Maybe we can even set up some testing infrastructure so we can test completions for the visual editor, to specify the expected behavior via tests? Like unit tests for the editor-codemirror package here?

@DavisVaughan Thank you for outlining those thoughts. Should we log any of that additional detail at posit-dev/ark#900?

juliasilge · 2025-09-03T14:14:43Z

apps/vscode/src/providers/editor/codeview.ts

      // if this is Positron, no visual editor completions
      // TODO: fix LSP issues for visual editor in Positron:
      // https://github.com/posit-dev/positron/issues/1805
-      if (hasHooks()) {
-        return {
-          items: [],
-          isIncomplete: false
-        };
-      }
+      // if (hasHooks()) {
+      //   return {
+      //     items: [],
+      //     isIncomplete: false
+      //   };
+      // }


Let's remove this code before we merge!

DavisVaughan · 2025-09-03T14:34:07Z

packages/editor-codemirror/src/behaviors/completion.ts

+    // no namespaces
+    if (vsKindToType(item.kind) === 'namespace') return false;


I would strongly recommend not wholesale removing namespaces

Namespaces are included in both RStudio and Positron's console and it is very important to be able to complete these

@juliasilge had made a comment about bslib not showing up when she typed lib in source mode, but I think that just has to do with how VS Code itself filters based on the text and doesn't have anything to do with the type of completion in question.

For example, if I do the same thing i see lightgbm pop up in the positron console, which suggests that namespace completions are definitely still there

So I vote we just remove this because keeping it would make for a worse experience overall

I guess I am OK with that, and we can log a new issue to address the specific filtering that happens in this type of case. I do think we need to get some unit tests set up for completions so we can specify the behavior we expect in a fairly low-burden way, and then can iterate moving forward without regressing on various cases.

@juliasilge FWIW if we do #798 (comment) then your experience here would be massively improved because only things starting with dpl or lib would be provided by ark in the initial set of completions

(I think that's a better way to improve the experience over turning off namespaces here)

I'll remove the filtering of namespaces for now!

@juliasilge I made an issue to capture the differences in filtering in the case of namespaces #808

thanks @vezwork!

Try to manually improve completions sorting

c3a0a49

vezwork requested a review from juliasilge August 22, 2025 22:31

juliasilge reviewed Aug 26, 2025

View reviewed changes

vezwork added 2 commits August 29, 2025 15:53

filter namespaces in completions

dba6d7e

add support for tab for accepting completions

4383020

vezwork marked this pull request as ready for review August 29, 2025 19:54

vezwork requested a review from juliasilge September 2, 2025 15:53

vezwork changed the title ~~Try to manually improve completions sorting~~ improve completions sorting Sep 2, 2025

juliasilge approved these changes Sep 3, 2025

View reviewed changes

DavisVaughan reviewed Sep 3, 2025

View reviewed changes

vezwork added 2 commits September 3, 2025 13:45

Remove namespace filtering

a61fcc9

remove code that disabled completions in Positron

85cb789

vezwork merged commit 2349d21 into main Sep 3, 2025
2 checks passed

vezwork mentioned this pull request Sep 3, 2025

completions include unexpected namespaces #808

Open

		// no namespaces
		if (vsKindToType(item.kind) === 'namespace') return false;

improve completions sorting #798

improve completions sorting #798

Uh oh!

Conversation

vezwork commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juliasilge left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vezwork commented Aug 28, 2025

Uh oh!

vezwork commented Aug 28, 2025

Uh oh!

juliasilge commented Aug 28, 2025

Uh oh!

vezwork commented Aug 28, 2025

Uh oh!

vezwork commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vezwork commented Aug 29, 2025

Uh oh!

juliasilge commented Aug 30, 2025

Uh oh!

vezwork commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DavisVaughan commented Sep 3, 2025

Uh oh!

juliasilge left a comment

Choose a reason for hiding this comment

Uh oh!

juliasilge Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

vezwork Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

DavisVaughan Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juliasilge Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DavisVaughan Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vezwork Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

vezwork Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

vezwork Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

DavisVaughan Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vezwork commented Aug 22, 2025 •

edited

Loading

juliasilge left a comment •

edited

Loading

vezwork commented Aug 29, 2025 •

edited

Loading

vezwork commented Sep 2, 2025 •

edited

Loading

DavisVaughan Sep 3, 2025 •

edited

Loading

juliasilge Sep 3, 2025 •

edited

Loading

DavisVaughan Sep 3, 2025 •

edited

Loading