Letter reordering creates false positives for diagnostics

[jc00ke]

I don't know if this is an Expert issue or a neovim (nightly) issue. I never saw this with ElixirLS or Nextls, and have not seen it with other LSPs.

vim.lsp: Active Clients ~
- expert (id: 1)
  - Version: 0.0.1
  - Root directory: ~/projects/website
  - Command: { "expert" }
  - Settings: {}
  - Attached buffers: 3, 5, 7

with neovim v0.12.0-dev-1078+g088ba835ed.

Please let me know if there's any other helpful information I can add, or if it's an upstream issue.

[mhanberg]

Can you elaborate on the issue a bit more? I see the typo in the module reported by the diagnostic but I'm not entirely sure what the steps you took to achieve that screenshot.

[jc00ke]

cd
mix new foo
cd foo
nvim lib/foo.ex
# def bar(name) do
# return
# tab (I think)
# String.upcase(name
# close paren
# diagnostic error appears with misspelled variable name

Just to make sure...

[throttle2k]

Hi, I'm experiencing the same issue on stable Neovim (0.11.4-1).

The problem occurs when typing quickly: although the words are entered correctly, the diagnostics sometimes display characters in the wrong order and mark them as errors.

If I type more slowly, the issue does not occur. This problem happens only with expert, and does not occur with elixirls.

[andyleclair]

Hey all, I'm also having this, neovim 0.11.3. Seems to happen when I type quickly. Is there a debounce timeout? Increasing that slightly might just make it ok

[vstegen]

I've tried to set update_in_insert to false for the nvim diagnostics and it does not help, so it seems that it's something internal to expert

[badosu]

Just a note that this issue existed in lexical for a while, so maybe the fix might be similar to the one that was applied to lexical at the time?

[heywhy]

Just a note that this issue existed in lexical for a while, so maybe the fix might be similar to the one that was applied to lexical at the time?

Can you point to the fix applied to Lexical?

[heywhy]

@mhanberg @doorgan So, I made a change to the cloned expert repo. I added a couple of log statements that show the content changes received from the client (Neovim) and the state of the document after the changes have been applied. Here is the log file expert.log. Below is a recording showing how to reproduce the issue.

If you look at the log file, you will find out that Neovim seems to use the same range for all content changes when you select a completion item, but Expert didn't handle the situation correctly. One of the ways I tried to fix it was by batching consecutive changes that share the same range into a single change map. But I worry the solution is not sophisticated, so I'm still testing with various interaction patterns to make sure no edge case is missed.

It's tricky to tell if the problem is from Neovim or Expert.

Excerpt:

# Current state of document held in memory
defmodule Proxy do
  String.a
end

# Notification received when navigating in the completion popup
%XPGenLSP.Structures.DidChangeTextDocumentParams{
  content_changes: [
    %{text: "", range: XpRange[<<2, 10>>...<<2, 11>>], range_length: 1},
    %{text: "b", range: XpRange[<<2, 10>>...<<2, 10>>], range_length: 0},
    %{text: "a", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "g", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "_", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "d", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "i", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "s", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "t", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "a", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "n", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "c", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0},
    %{text: "e", range: XpRange[<<2, 11>>...<<2, 11>>], range_length: 0}
  ],
  text_document: %XPGenLSP.Structures.VersionedTextDocumentIdentifier{uri: "file:///Users/home/Dev/me/proxy/lib/proxy.ex", version: 27}
}

# After Expert Applied the above content changes, the document state in memory is now the below.
defmodule Proxy do
  String.becnatsid_ga
end

[mrluc]

Re: the recent report of "typing very fast" #119 causing letter order issues -- I can report that I definitely get it more consistently with things I've pasted, which could be related since that's like typing very fast.

[heywhy]

So, I was able to trace the whole flow, and I got to find out that Neovim has this concept of incremental updates, and that's the reason for different edits having the same range values, which Expert does not handle correctly. I've been able to identify a few new patterns, and I already have a fix that handles a lot of them, but I'm currently unwell, so I can't tackle it right now (hopefully, I should be better in the next couple of days).

To mitigate temporarily, you should turn off incremental updates for Expert using the below config:

vim.lsp.config('expert', {
  flags = {
    allow_incremental_sync = false,
    -- When I set the below option to nil and have incremental sync on, I got substantial improvements with the fixes
    -- I had in place until a weird edge case popped up
    -- debounce_text_changes = nil
  }
})

The downside of turning off incremental updates is that the whole file content is sent for every keystroke.

And I also tried looking in the Zed repo (per this https://github.com/elixir-lang/expert/issues/83#issuecomment-3263278440) to see if they share the same concept, but I can't find anything substantial yet.

[badosu]

To mitigate temporarily, you should turn off incremental updates for Expert using the below config:

vim.lsp.config('expert', { flags = { allow_incremental_sync = false, -- When I set the below option to nil and have incremental sync on, I got substantial improvements with the fixes -- I had in place until a weird edge case popped up -- debounce_text_changes = nil } })

Thank you so much! I can confirm this workaround works, I was almost giving up on expert due to having to reload the document everytime it failed ingesting text changes correctly.

[heywhy]

Guess what?! I think I've resolved the following issues:

1. Letter reordering problem
2. Formatting wrangling the inner representation of the document
3. I don't need to turn off incremental sync in Neovim anymore

I would love it if anyone could build this #123 and test (with incremental sync turned back on, in case you turned yours off). I would love it if people using other editors also give it a try; hopefully, it solves the problem across the board.

[badosu]

I would love it if anyone could build this #123 and test (with incremental sync turned back on, in case you turned). I would love it if people using other editors also give it a try; hopefully, it solves the problem across the board.

I'll give it a spin tomorrow, thank you for your effort!

[ogomezba]

@heywhy Thank you for the effort! I'll be trying it, mainly in Neovim v0.11.4. Just to confirm, is the fix also solving the "typing fast causes reordering problem"? For now, I'm still getting fake reordering when typing fast.

https://github.com/user-attachments/assets/0c99184f-dfa4-4e4b-a4d4-fb3a02352522

[heywhy]

Just to confirm, is the fix also solving the "typing fast causes reordering problem"

Yes, it does.

But I've seen cases where I generate a new release, but the changes do not reflect while testing, so I would advise you to delete the .burrito folder (as it is mostly stored in the user's "users data" directory) and restart your editor. Let me know what you find afterwards.

I think you would find the folder in the path below.

• ~/Library/Application Support/.burrito on macOS
• ~/.local/share/.burrito on Linux
• %appdata%/Expert on Windows

NB: I'm sure of the macOS because I use one.

[heywhy]

Just a quick update, while doing some other work, I started experiencing the same issue, but at reduced frequency. The next place I would be looking is Neovim, this file to be precise. I believe a lot of the issue is coming from the incremental sync implementation.

EDIT: At the same time, I'm also worried about the fact that I don't need to turn off increment sync for other language servers, and they work just fine. So, it's difficult to accept turning it off for Expert (maybe it's just me 😅 ).

[mhanberg]

If you produce a local prod build for the same version you already have, you need to run expert maintenance uninstall. Easier than manually deleting folders.

But you can build it mix env dev and you won't have that problem, but it'll be slower to start.

[vstegen]

Just a quick update, while doing some other work, I started experiencing the same issue, but at reduced frequency. The next place I would be looking is Neovim, this file to be precise. I believe a lot of the issue is coming from the incremental sync implementation.

EDIT: At the same time, I'm also worried about the fact that I don't need to turn off increment sync for other language servers, and they work just fine. So, it's difficult to accept turning it off for Expert (maybe it's just me 😅 ).

I think this is interesting because I don't experience this with lexical or elixirls. So I assume their implementation of this feature is different somehow?

[mhanberg]

The lexical implementation is the same, but expert has GenLSP in front of it (from NextLS).

This is why I'm going to investigate what I said above.

[ogomezba]

@mhanberg Just in case it helps, I've been unable to reproduce the problem in VSCode. With the same expert binary being used as LSP in both cases and typing at the same speed, I easily reproduce the "reordering" problem in Neovim but not in VSCode.

[andyleclair]

If this is helpful, this is what I'm seeing in Neovim right now. I didn't make any mistakes typing this out, but Expert thinks the word is all wrong

[iurimateus]

A couple of thoughts:

neovim's debounce_text_changes is more of a ratelimiter/throttle than a debounce (so it may send more smaller changes, such as the one in heywhy's example, in that each character of bag_distance turns to be single/different edit).

After Expert Applied the above content changes, the document state in memory is now the below.

 defmodule Proxy do
  String.becnatsid_ga
end

Note that String.becnatsid_ga is b <> ag_distance, with the latter reversed.

--

There's a TODO in Expert.State's def apply(%__MODULE__{} = state, %GenLSP.Notifications.TextDocumentDidChange{params: params}) https://github.com/elixir-lang/expert/blob/e9d69dd34c49eef4061c6cc010930a99c616e908/apps/expert/lib/expert/state.ex#L110

Although it does seem that's handled by Convertible.to_native in Forge.Document.apply_change.

[heywhy]

The source of this issue has been identified and fixed. Anyone interested can build #123 to test; the description has been updated.

@mhanberg The PR is ready for review.

[vstegen]

I'm not sure if this is related to this particular issue, will this also fix an issue with auto completion?

I.e. when accepting a suggestion, it might not remove the initial characters that led to the suggestion. As an example (I can record something later and open another issue if it's unrelated):

• open a liveview file
• type han to get auto completion for handle_event
• accept the suggestion
• output:

han@impl true
def handle_event(..., ..., ...) do
end

instead of

@impl true
def handle_event(..., ..., ...) do
end

So instead of overwriting, it simply appends the suggestion. If it's unrelated, I'll open another issue

[heywhy]

Yup, it fixes the autocomplete issue too.

EDIT: You can build the changes locally to confirm your case, but I believe it does.

[vstegen]

Yup, it fixes the autocomplete issue too.

EDIT: You can build the changes locally to confirm your case, but I believe it does.

Thanks! I'll check it out locally and report back!

[vstegen]

Yup, it fixes the autocomplete issue too.

EDIT: You can build the changes locally to confirm your case, but I believe it does.

I've tested it now, and it fixes this autocomplete issue too! Thanks a lot for your work on this 🙏

[ogomezba]

Hello! I've also been testing a bit, and it looks indeed much better in Neovim. Thank you for the effort!

Still, I'm not sure if it's only me, but I still reach some inconsistent states. I'm sharing two videos that illustrate two ways in which I consistently get problems:

https://github.com/user-attachments/assets/9caa450a-8ed9-4595-813b-27a894fc2580

https://github.com/user-attachments/assets/8edbb0ee-6d58-46a0-80b4-74158b9644f0

[heywhy]

Did you run expert maintenance uninstall after building? I would advise you to do that, and kill every beam.smp process, then restart your editor.

[ogomezba]

Yes, I did. It might be just me if nobody else can reproduce it.