Autocomplete: do adjust a completion range if it does not match the current line suffix

[valerybugakov]

Context

• Fixes the issue where completions were needlessly hidden: myFunction(█) with the insert text being "one", false because the current line suffix ) was not present in the generated completion. However, we still adjusted the completion insertion range to include it.
• Fixes the issue where all the generated completions were hidden, even if only one needed to be ignored. Now, we filter out invisible completions individually and keep visible completions if we have any.

Test plan

Added unit tests.

[abeatrix]

Fixes the issue where completions were needlessly hidden: myFunction(█) with the insert text being "one", false because the current line suffix ) was not present in the generated completion. However, we still adjusted the completion insertion range to include it.

I found cases where completion is still not showing up because of current line suffix 🤔

Is this something else that's unrelated to the current issue?

[valerybugakov]

@abeatrix, great catch! I debugged your examples locally and found out that we change completion insert ranges in two places:

1. cody/vscode/src/completions/text-processing/process-inline-completions.ts

   Line 67 in b1f78ac

   completion.range = getRangeAdjustedForOverlappingCharacters(completion, { position, currentLineSuffix })

2. cody/vscode/src/completions/inline-completion-item-provider.ts

   Line 506 in b1f78ac

   const vscodeInsertRange = new vscode.Range(start, end)

The second part doesn't care if the completion range already exists, so the logic in the post-processing call (1) does not affect the result.

I feel that this is one of these cases we bumped into frequently in the last month: we have multiple places implementing parts of the same logic, and they are not explicitly connected, which causes customer-facing bugs because it's impossible to keep all these pieces in our heads. The codebase snowballs. We should spend time streamlining the completion processing logic to clarify where specific changes are expected on the codebase architecture level (e.g., having one place where we operate on completion insert ranges or having one place for multiline truncation). Setting clear boundaries based on the current implementation would help us move faster with less time spent debugging such cases. WDYT?

cc @philipp-spiess @beyang

[abeatrix]

What you said makes a lot of sense to me but I'll defer to you and Philipp since you both know the code better than I do 😃 it sounds like my examples require additional follow-up work but shouldn't block what you fixed here?

[valerybugakov]

it sounds like my examples require additional follow-up work but shouldn't block what you fixed here?

Yeah, I'm working on it now and will add them to the diff in this PR.

[valerybugakov]

@abeatrix, I pushed the update: 3f64635 to account for cases you shared above. Could you take another look, please?

[abeatrix]

@valerybugakov sorry I spent too much time on the chat redesign pr and forgot to review this. Will do it first thing on Monday 🙇🏻‍♀️

[philipp-spiess]

Fix looks reasonable! TY! It's indeed concerning that we have two places where we expand the range, we should make the range required with the completion item, that should help!