fix(autoedit): fix shrink prediction logic

[valerybugakov]

• Follow up for autoedit: shrink prediction repro issue #6386

Test plan

1. CI
2. New unit tests
3. Manually tested on cody-chat-eval examples

[hitesh-1997]

can we add a comment why do we want to strip the last empty line

[valerybugakov]

Added 👍

[hitesh-1997]

Why are we only checking if predictionSlice[j].startsWith(suffixSlice[j]). If we want to see that prediction lines matches with suffix, shouldn't we check === condition

[valerybugakov]

See the test case with self.email = email in vscode/src/autoedits/shrink-prediction.test.ts. I want to catch cases where something is added to the end of the suffix line, too.

[hitesh-1997]

I don't think that should be the expectation since suffix self.email = and not self.email = email which is in the prediction. So, we can't match it suffix. There is risk of prediction getting trimmed just because suffix prefixed with prediction.

I added a test case here which will have issues because .startsWith logic just because the suffix had bunch of empty lines below and prediction added lines at the same indentation level, so prediction.startwith(suffix) becomes true because suffix is only the spaces with same indentation level, and all the prediction lines gets trimmed.

(please note that I had to turn off trim_trailing_whitespace = false in .editorconfig in our code to add trailing spaces in the code, so if we git pull and save, the trailing space would get trimmed and test case might not work, so please turn off this settting before git pull)

[valerybugakov]

The partial match logic is giving us too many false positives. Even after fixing the test case you pushed (thanks for that!), I found this behavior popping up in several places during manual testing. So, I reverted the logic back to an exact match.

Could we handle this at the model layer? Is there a way to nudge the model to only make changes within the code-to-rewrite block? I guess this issue comes from the training dataset, where suffix changes sometimes get duplicated in the updated code section. Could this be the case?

You can check the test case with self.email = email to see an example. Even though this line is part of the suffix, the model still modifies it and includes it in the response. Ideally, it should only return new line insertions.

[valerybugakov]

The goal of the shrink-until-suffix logic was to ensure we still show parts of suggestions that are relevant, even if they only partially match the suffix. Previously, we had logic using startsWith() that would hide the entire suggestion if its last lines were partially present in the document. Let’s chat about the best way to solve this over Zoom.

[valerybugakov]

I traced the addition of this logic to the feature, which comes from this PR, which does not give much context for this specific change. I wonder how critical was the starsWith() bit back then when it was introduced and why didn't we opt-in for the exact match instead.

[hitesh-1997]

The reasoning behind the old logic was that, if model includes the suffix code, than they can only be new added lines from the code to rewrite, and I used startsWith there because we were not doing the line level comparison, but the whole suffix is the whole suffix in the file.
But the logic earlier also had its flaws and not very well though, but it mitigated some issues I encountered while manual testing.
I think it would be okay to just let the model output wrong predictions and we can figure out the severity from the logs.

[hitesh-1997]

I had confusion in some statements, added comments to clarify

[hitesh-1997]

Added test case which would have issue with the current logic, please check this commit (comment)