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: Use a tracked originalRange when retrying a task #1926

Merged
merged 5 commits into from
Nov 30, 2023
Merged

Conversation

umpox
Copy link
Contributor

@umpox umpox commented Nov 28, 2023

Description

RetryLogic.mov

Follow up on: #1892 (comment)

We were retrying fixup tasks with the selectionRange - this is not suitable as this range may shrink or expand depending on the code that the LLM produces.

Instead we should use the originalRange - the range that is provided when the fixup task is created. We still need to track the positions of the range, as it could shift due to code being added above.

Test plan

  1. Create fixups
  2. Press "retry"
  3. Check retry range is correct (code is not unnecessarily deleted, unless caused by the LLM)

@umpox umpox changed the title Improve fixup consistency Edit: Use original range for retry logic Nov 28, 2023
Base automatically changed from tr/fixup-consistency to main November 28, 2023 10:55
@umpox umpox changed the title Edit: Use original range for retry logic Edit: Use a tracked originalRange when retrying a task Nov 28, 2023
@umpox umpox marked this pull request as ready for review November 28, 2023 11:00
@umpox umpox requested review from abeatrix and a team November 28, 2023 11:00
* Only handles edits that are outside of the range, as it is purely focused on shifting a fixed range in a document.
* Does not expand or shrink the original rank.
*/
export function updateFixedRange(range: vscode.Range, change: TextChange): vscode.Range {
Copy link
Member

Choose a reason for hiding this comment

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

Seems like a good candidate for a unit test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, have added a test!

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Nice!

@umpox umpox merged commit e6753d3 into main Nov 30, 2023
14 checks passed
@umpox umpox deleted the tr/retry-range branch November 30, 2023 08:57
@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

3 participants