fix: allow submitting comments on completely deleted files#14183
Conversation
## Description Fixes a bug in code review comment submission where comments on completely deleted files would fail validation or relocation and could not be submitted. ### Root causes 1. **Missing Base Content**: When applying a diff to a deleted file, the code editor's base content (`set_base`) was never initialized. This caused line relocation to fail and mark comments as `outdated = true`. 2. **Missing File Path Metadata**: Deleted files have no backing on-disk file model and are initialized with no file path metadata. `LocalCodeEditorEvent::CommentSaved` was querying the editor's path metadata, which returned `None` and aborted comment saving. 3. **Empty Target Range Bounds**: When commenting on a deleted line in an empty/deleted file, the line range was hardcoded to `line_number..line_number + 1` (evaluating to `0..1`). Because the file has 0 lines, this range was out of bounds, failing backend validation. ### Fixes 1. **Reinitialize Base Content**: Call `set_base` with the old base content for deleted files during diff application. 2. **Direct Path Propagation**: Used the `file_location` parameter already supplied to the event handler closure directly instead of attempting to query the empty editor metadata. 3. **Dynamic Range Calculation**: Compute target ranges as `line_number..line_number + span`. We ensure pure deletions in normal files receive a minimum 1-line range span, while completely deleted files receive the correct `0..0` range. ## Linked Issue Fixes warpdotdev#14182 ## Testing Added unit tests: - `test_relocate_comments_on_deleted_file`: Verifies comment relocation on deleted files. - `test_save_comment_on_deleted_file`: Verifies that `CommentSaved` events attach comments successfully to deleted file editors. - `test_api_review_comment_line_range_deleted_file`, `test_api_review_comment_line_range_normal_file`, `test_api_review_comment_line_range_multiline`, and `test_api_review_comment_line_range_pure_deletion`: Verifies range boundaries. <!-- CHANGELOG-BUG-FIX: Fixed bug where code review comments could not be submitted on completely deleted files -->
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @menil on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
Every PR must be linked to a same-repo issue before Oz can review it. This PR is linked to #14182, but no linked issue is marked See the contribution guidelines for the full readiness model. Powered by Oz |
There was a problem hiding this comment.
Every PR must be linked to a same-repo issue before Oz can review it.
This PR is linked to #14182, but no linked issue is marked ready-to-implement yet. Only repository maintainers apply that label, so please wait for a maintainer to mark the issue. Once it is marked, push a new commit or comment /oz-review to re-trigger review.
See the contribution guidelines for the full readiness model.
Powered by Oz
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
Description
Fixes a bug in code review comment submission where comments on completely deleted files would fail validation or relocation and could not be submitted.
Root causes
set_base) was never initialized. This caused line relocation to fail and mark comments asoutdated = true.LocalCodeEditorEvent::CommentSavedwas querying the editor's path metadata, which returnedNoneand aborted comment saving.line_number..line_number + 1(evaluating to0..1). Because the file has 0 lines, this range was out of bounds, failing backend validation.Fixes
set_basewith the old base content for deleted files during diff application.file_locationparameter already supplied to the event handler closure directly instead of attempting to query the empty editor metadata.line_number..line_number + span. We ensure pure deletions in normal files receive a minimum 1-line range span, while completely deleted files receive the correct0..0range.Linked Issue
Fixes #14182
Testing
Added unit tests:
test_relocate_comments_on_deleted_file: Verifies comment relocation on deleted files.test_save_comment_on_deleted_file: Verifies thatCommentSavedevents attach comments successfully to deleted file editors.test_api_review_comment_line_range_deleted_file,test_api_review_comment_line_range_normal_file,test_api_review_comment_line_range_multiline, andtest_api_review_comment_line_range_pure_deletion: Verifies range boundaries.I have manually tested my changes locally with
./script/runScreenshots / Videos
Before
Screen.Recording.2026-07-22.at.21.33.56.mov
After
Screen.Recording.2026-07-22.at.22.48.41.mov
Agent Mode
CHANGELOG-BUG-FIX: Fixed bug where code review comments could not be submitted on completely deleted files