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: Improve Rejection Tracker. Handle file deletions and formatters #4097

Merged
merged 8 commits into from May 13, 2024

Conversation

umpox
Copy link
Contributor

@umpox umpox commented May 8, 2024

Description

This PR:

  • Improves how we track rejection for edit tasks
    • We now track when the file is deleted, before a change is made (important for cases like test or generate)
    • We now filter out multi-character changes, as these are likely to come from formatters or other commands. They are not a good signal that the user has made an implicit choice to accept this Edit.
    • We now filter out empty (whitespace) changes, as these are not a good signal that the user has made an implicit choice to continue coding and thus accept the edit
  • Added lots of unit tests for the rejection tracker, so we can ensure this logic doesn't break over time

Test plan

Tested with unit tests, manual plan below:

Normal Edit - Rejected:

  1. Make an Edit
  2. Undo it
  3. Make another change
  4. Expected rejected

Normal Edit - Accepted

  1. Make an edit
  2. Make another change, type something
  3. Expected accepted

Normal Edit - Empty changes - Accepted

  1. Make an edit
  2. Enter an empty change (e.g. new lines)
  3. Expect not accepted or rejected
  4. Make a single character change
  5. Expect accepted

Normal Edit - Empty changes - Rejected

  1. Make an edit
  2. Make another change
  3. Enter an empty change (e.g. new lines)
  4. Expect not accepted or rejected
  5. Undo the change
  6. Expect nothing happened (undone empty change)
  7. Undo the change again
  8. Expect rejected

Normal Edit - Large changes - Accepted

  1. Make an edit
  2. Paste multiple characters of text (simulate something like a formatter)
  3. Expect not accepted or rejected
  4. Make a single character change
  5. Expect accepted

Normal Edit - Large changes - Rejected

  1. Make an edit
  2. Paste multiple characters of text (simulate something like a formatter)
  3. Expect not accepted or rejected
  4. Undo the change
  5. Expect nothing happened (undone empty change)
  6. Undo the change again
  7. Expect rejected

@umpox umpox changed the title Edit: Track file deletions as task rejections Edit: Improve Rejection Tracker. Handle file deletions and formatters May 10, 2024
@umpox umpox marked this pull request as ready for review May 10, 2024 09:27
@umpox umpox requested review from a team May 10, 2024 09:29
@umpox
Copy link
Contributor Author

umpox commented May 10, 2024

@abeatrix For the unit test one in particular, here's some additional testing:

  • If the file is unsaved, and then closed we log rejected
  • If the file is saved, we don't log anything
  • If the user makes a change to the file, we log accepted
  • If the user clicks "Undo" on the test change in the file, we log rejected

@umpox umpox requested a review from chenkc805 May 10, 2024 11:23
@umpox umpox requested a review from philipp-spiess May 13, 2024 10:43
Copy link
Member

@philipp-spiess philipp-spiess left a comment

Choose a reason for hiding this comment

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

Love this! Especially the tests are :chef-kiss:

@umpox umpox merged commit b49b5c5 into main May 13, 2024
18 of 19 checks passed
@umpox umpox deleted the tr/edit-stop-tracking branch May 13, 2024 12:54
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