Skip to content

fix: write a file truncated to empty instead of deleting it - #94

Merged
2 commits merged into
mainfrom
fix/emptied-file-not-deleted
Sep 27, 2026
Merged

2 commits merged into
mainfrom
fix/emptied-file-not-deleted

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Problem

materialize_group_files used unidiff's is_removed_file to decide whether to delete a path. That property is a heuristic: it is also true for any single hunk whose target length is 0 — i.e. a file that dev truncated to empty (@@ -1,20 +0,0 @@, target still b/a.txt). The sub-PR therefore deleted the file: its tree differed from dev (a.txt | 0) and merging the sub-PR back conflicted (modify/delete). Found by the end-to-end hunt.

Fix

Treat a path as deleted only when patch_file.target_file == "/dev/null" (the first clause of unidiff's own check). Verified in a real repo with a file emptied, a file deleted, an empty file filled, and an ordinary edit: the sub-PR git ls-tree -r now equals dev; on base the emptied file is missing.

Tests

Emptied file materialises as "" (fails on base); real /dev/null deletion still yields None.

Local review: 5/5. 452 tests pass, ruff clean. Stacked on #68 (same function). Follow-up: #90's target_file_modes uses the same heuristic and will get the same one-line change.

unidiff's is_removed_file is true for any single hunk whose target
length is 0, which also matches a file emptied on the dev branch. The
sub-PR then deleted the file, its tree differed from dev, and merging
it conflicted (modify/delete). Treat only a /dev/null target as a
deletion.
@coderabbitai

coderabbitai Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 51 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5115b61e-2197-478b-bfc9-4b8205aed3f5

📥 Commits

Reviewing files that changed from the base of the PR and between 671a081 and 8a7f17e.

📒 Files selected for processing (2)
  • pr_split/diff_ops/reconstructor.py
  • tests/test_reconstructor.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Base automatically changed from fix/duplicate-file-assignments to main August 31, 2026 01:16
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
@vitali87 vitali87 closed this pull request by merging all changes into main in 8ca9e3b Sep 27, 2026
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.

2 participants