fix: update post process logic for claude instant #1440

abeatrix · 2023-10-19T16:33:43Z

fix: use full infill block and adjust prefix

fix prompt structure order, context should be shared after the role assigning
empty prefix if it contains the infill block for issue where first line is duplicated
Simplify prompt messages to focus on providing clear context
Add CLOSING tag for Assistant's infill block to preserve ending whitespace
This also fixed issues where multi-line autocomplete gets cut off.
remove tags for context code as that might cause confusion for claude

Test plan

Fix: show multi-line completion after {:

Fix: show multi-line completions the line after {

valerybugakov

Could we separate the bug fix from the prompt changes and put the prompt changes behind the feature flag so that we can test how they affect CAR in production before making it a default?

The prompt updates look logical, and I expect them to perform well, but with LLMs, we never know how newly added tokens will affect the output. We have quite a few changes here, from the sentence structure updates to changing the code block tags. Splitting this PR in two would make it easier to reason about CAR changes later:

Minimal prompt change required to fix a bug + logic updates.
Prompt updates behind the feature flag.

abeatrix · 2023-10-20T02:17:16Z

@valerybugakov that's fair, I can do that next!

Have you tried the change in this PR yet? Is it working for you?

philipp-spiess · 2023-10-20T13:47:20Z

vscode/src/completions/providers/anthropic.ts

-            },
-            {
-                speaker: 'human',
-                text: `Below is the code from file path ${relativeFilePath}. Review the code outside the XML tags to detect the functionality, formats, style, patterns, and logics in use. Then, use what you detect and reuse methods/libraries to complete and enclose completed code only inside XML tags precisely without duplicating existing implementations. Here is the code: \n\`\`\`\n${infillPrefix}${OPENING_CODE_TAG}${infillBlock}${CLOSING_CODE_TAG}${infillSuffix}\n\`\`\``,


If we don't split the prompt anymore between "head" and "tail", should we remove the whole getHeadAndTail function?

philipp-spiess · 2023-10-20T14:05:58Z

TBH I’m happy to move fast with this given how big our regression is right now. My only thinking is that:

we should run our completion test suite to ensure we don't add known issues
we delete old code that is no longer needed (like the head/tail splitting of the prefix) to avoid introducing code dept

once you're happy with the results we should push out a patch release.

philipp-spiess

preemptive stamp but lets make sure the tests are good

abeatrix · 2023-10-20T21:31:05Z

@philipp-spiess @valerybugakov I tried to make minimal prompt change as possible but the one we had just doesn't work anymore because of the lines and whitespaces removed on claude side. I added <cursor> to the end of the infill block to make sure the spaces are not removed, and it seems to work fine on my side, but i wasn't able to get the eval tests to work to confirm if there is other regression, but definately better than what is in prod right now. If this is good for now, please feel free to merge and make a patch release on monday to make sure our users are not getting affected, and I can do any required follow up works

philipp-spiess · 2023-10-23T09:39:33Z

@abeatrix I pushed a fix for the local completions test and there seem to be some issues with the newly used XML leaking into the completion:

Also the test case with a comment followed by a \n isn't working well either :/

abeatrix · 2023-10-23T18:43:28Z

@valerybugakov After taking a look at the issue with @philipp-spiess , it looks like the root cause of the issue was the new parseAndTruncateCompletion that doesn't work well with the this.postProcess() for anthropic. I tried added some workaround in my lastest commits and that seems to work (see the updated infill test result). Can you take a look and see if this change makes sense to you? Since this is a P1 issue, do you mind taking over of this PR while I'm away to make sure we can have this issue resolved asap?

Update

looks like the issue (at current main) is when the current line ends with non-bracket, we will display the suggestion after trimming start()

It doesn't happen if prefix ends with bracket (or if we remove the postProcess logic)

@valerybugakov the change in my latest commit where I remove the space after the \n in new suggestions to prevent normalizeStartLine from removing the \n seems to work. can you take a look and see if this looks ok to you or something we can update in normalizeStartLine without causing regression for other provider?

abeatrix · 2023-10-23T21:11:03Z

vscode/src/completions/providers/anthropic.ts

@@ -67,7 +67,7 @@ export class AnthropicProvider extends Provider {
        const { head, tail, overlap } = getHeadAndTail(this.options.docContext.prefix)

        // Infill block represents the code we want the model to complete
-        const infillBlock = tail.trimmed
+        const infillBlock = tail.trimmed.endsWith('{\n') ? tail.trimmed.trimEnd() : tail.trimmed


this allows claude to response with multi-line completion on new line after the { bracket

abeatrix · 2023-10-23T21:13:59Z

vscode/src/completions/providers/anthropic.ts

@@ -216,6 +216,10 @@ export class AnthropicProvider extends Provider {
            // leading `\n` followed by whitespace that Claude might add.
            completion = completion.replace(/^\s*\n\s*/, '')
        } else {
+            // prevent normalizeStartLine from removing the starting new line
+            if (completion.startsWith('\n')) {
+                completion = '\n' + completion.trimStart()


normalizeStartLine in parseAndTruncateCompletion would remove the \n if it follows by indentation when prefix didn't end with closing bracket (Need Valery to confirm if this is intended ). This solves the aforementioned issue.

Updated the logic to normalizeStartLine only for multiline completions like before my changes in this PR. It fixes the issue without adding the additional logic. Thank you for narrowing down this regression!

valerybugakov · 2023-10-24T03:44:23Z

@abeatrix, thank you for sharing the context! I'm debugging it locally and will merge this if I can confirm your findings 👍

valerybugakov

After my change, I verified that the cases mentioned here still work locally as expected. Regenerated completions using the generate:completions to ensure we do not have any leaking XML tags.

valerybugakov · 2023-10-24T05:12:27Z

The PR is blocked by eslint errors on main. Fixing it here: #1471

Patch release for p1 bugs - #1477 - #1440 - Update default prompt mixin ## Test plan  version bump

fix: use full infill block and adjust prefix

ca57122

abeatrix requested review from philipp-spiess and valerybugakov October 19, 2023 16:34

abeatrix added 2 commits October 19, 2023 09:55

Preserve trailing whitespace

6b329f5

move infillBlock out of tags

5abdca4

valerybugakov reviewed Oct 20, 2023

View reviewed changes

philipp-spiess reviewed Oct 20, 2023

View reviewed changes

philipp-spiess approved these changes Oct 20, 2023

View reviewed changes

abeatrix added 2 commits October 20, 2023 14:14

update prompt

139351c

fix tests

d288444

fix quotes

3c01553

philipp-spiess and others added 3 commits October 23, 2023 11:40

Fix completions review tests

8b859d9

simplify postProcess

3e8d932

update infill test

68dacfb

abeatrix added 3 commits October 23, 2023 13:06

fix test

4f1cafd

Revert postprocess

5d93943

update postProcess with test results

7101aa3

abeatrix commented Oct 23, 2023

View reviewed changes

abeatrix changed the title ~~fix: use full infill block and adjust prefix~~ fix: update post process logic for claude instant Oct 23, 2023

Autocomplete: normalizeStartLine only for multiline completions

21ce9ef

valerybugakov assigned valerybugakov and abeatrix Oct 24, 2023

valerybugakov added the autocomplete label Oct 24, 2023

valerybugakov approved these changes Oct 24, 2023

View reviewed changes

Merge branch 'main' into bee/fix-infill

e68d231

valerybugakov merged commit 05e9039 into main Oct 24, 2023
13 of 14 checks passed

valerybugakov deleted the bee/fix-infill branch October 24, 2023 05:26

abeatrix mentioned this pull request Oct 24, 2023

Cody Patch Release v0.14.2 #1478

Merged

abeatrix mentioned this pull request Oct 27, 2023

Autocomplete: bug - regression in Claude Instant #1438

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update post process logic for claude instant #1440

fix: update post process logic for claude instant #1440

abeatrix commented Oct 19, 2023 •

edited

Loading

valerybugakov left a comment

abeatrix commented Oct 20, 2023

philipp-spiess Oct 20, 2023

philipp-spiess commented Oct 20, 2023

philipp-spiess left a comment

abeatrix commented Oct 20, 2023

philipp-spiess commented Oct 23, 2023

abeatrix commented Oct 23, 2023 •

edited

Loading

abeatrix Oct 23, 2023

abeatrix Oct 23, 2023 •

edited

Loading

valerybugakov Oct 24, 2023

valerybugakov commented Oct 24, 2023

valerybugakov left a comment

valerybugakov commented Oct 24, 2023

fix: update post process logic for claude instant #1440

fix: update post process logic for claude instant #1440

Conversation

abeatrix commented Oct 19, 2023 • edited Loading

Test plan

valerybugakov left a comment

Choose a reason for hiding this comment

abeatrix commented Oct 20, 2023

philipp-spiess Oct 20, 2023

Choose a reason for hiding this comment

philipp-spiess commented Oct 20, 2023

philipp-spiess left a comment

Choose a reason for hiding this comment

abeatrix commented Oct 20, 2023

philipp-spiess commented Oct 23, 2023

abeatrix commented Oct 23, 2023 • edited Loading

Update

abeatrix Oct 23, 2023

Choose a reason for hiding this comment

abeatrix Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

valerybugakov Oct 24, 2023

Choose a reason for hiding this comment

valerybugakov commented Oct 24, 2023

valerybugakov left a comment

Choose a reason for hiding this comment

valerybugakov commented Oct 24, 2023

abeatrix commented Oct 19, 2023 •

edited

Loading

abeatrix commented Oct 23, 2023 •

edited

Loading

abeatrix Oct 23, 2023 •

edited

Loading