Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edit: Fix leaky XML tags #1789

Merged
merged 5 commits into from
Nov 17, 2023
Merged

Edit: Fix leaky XML tags #1789

merged 5 commits into from
Nov 17, 2023

Conversation

umpox
Copy link
Contributor

@umpox umpox commented Nov 17, 2023

Description

Reported here: https://sourcegraph.slack.com/archives/C05AGQYD528/p1700227846587329

Fixups can sometimes leak tags:

  • When little context is provided, so Cody gets confused about the task
  • Due to multiple tags being used, Cody sometimes thinks they are to be used in the response.

Test plan

Test creating fixups.

Primarily test creating fixups on code actions, where we provide the least context

@umpox umpox marked this pull request as ready for review November 17, 2023 14:59
@umpox umpox requested a review from abeatrix November 17, 2023 15:04
Copy link
Contributor

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to run a code action and it's stuck in cody is working even when we get the response back 🤔

image

@abeatrix
Copy link
Contributor

Restarted work. The job still contains the leaked tags though:

image

This is the file I run the Code Action on, just removed a few lines of code to
get an error if that helps: https://sourcegraph.com/github.com/sourcegraph/cody/-/blob/vscode/src/repository/repositoryHelpers.ts?L13-19

@umpox
Copy link
Contributor Author

umpox commented Nov 17, 2023

FixupTest.mov

Can't replicate 🤔

Please can you double check this is the latest branch and not the previous Cody installation, @abeatrix
There's some things there that are very worrying as I have fixed all those in the past week

  • Fix taking over 5s, judging from the log in your screenshot
  • <selectedCode> at the start, but we just included a change in the last release that removes <selectedCode> from the start, so idk how that could happen
  • The "Cody is working" message, which should be fixed as it was related to formatting issues before

@abeatrix
Copy link
Contributor

So sorry @umpox, I was in the wrong branch 😭

@umpox
Copy link
Contributor Author

umpox commented Nov 17, 2023

@abeatrix Np, I was worried the dreaded "Cody is working" was back 😂

Copy link
Contributor

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea i just tried it a couple times and I'm not seeing any leaked tags and the Cody is working forever issue 😆 Thanks for the quick fix!

@umpox umpox merged commit ec1def1 into main Nov 17, 2023
13 checks passed
@umpox umpox deleted the tr/fix-leaky-xml-tags branch November 17, 2023 15:50
@umpox umpox self-assigned this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants