fix: handle newline issues in patch generation

[mrT23]

User description

addresses #1438

---

PR Type

Bug fix, Tests

---

Description

• Fixed newline handling in load_large_diff to prevent incorrect patch generation.

• Added tests for scenarios with missing newlines and empty inputs in load_large_diff.

• Updated test cases to include trailing newline in expected patch strings.

• Introduced a new test class TestLoadLargeDiff for targeted patch generation testing.

---

Changes walkthrough 📝

	Relevant files
Bug fix	utils.py Fix newline handling in `load_large_diff` function              pr_agent/algo/utils.py Ensured both original and new file content strings end with a newline. Prevented issues with patch generation when files lack trailing newlines. +2/-0
Tests	test_bitbucket_provider.py Update test cases for trailing newline handling                    tests/unittest/test_bitbucket_provider.py Updated expected patch strings to include trailing newlines. Improved test accuracy for multi-line file diffs. +3/-3      test_extend_patch.py Add tests for `load_large_diff` function                                  tests/unittest/test_extend_patch.py Added TestLoadLargeDiff class for patch generation tests. Included tests for missing newlines and empty input scenarios. +32/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro-for-open-source]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 1438 - Fully compliant Compliant requirements: Fix bug in load_large_diff where incorrect patch is generated when old file doesn't end in newline Prevent mashing of two lines together in the generated patch

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 95

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Edge Cases Verify that rstrip() + newline approach handles all edge cases correctly, including empty strings and strings with only whitespace original_file_content_str = original_file_content_str.rstrip() + "\n" new_file_content_str = new_file_content_str.rstrip() + "\n"

[mrT23]

/describe

[mrT23]

/improve

[qodo-merge-pro-for-open-source]

PR Code Suggestions ✨

Latest suggestions up to 793f76f

Category	Suggestion	Score
Possible issue	Normalize test input string indentation to prevent misleading test results The test case test_no_newline contains inconsistent indentation in the input strings which could lead to misleading test results. Normalize the indentation in the test input strings. tests/unittest/test_extend_patch.py [165-173] patch = load_large_diff("test.py", - """\ - old content 1 - some new content - another line - """, - """ - old content 1 - old content 2""") + """old content 1 + some new content + another line + """, + """old content 1 + old content 2""") Apply this suggestion Suggestion importance[1-10]: 7 Why: The inconsistent indentation in test input strings could lead to incorrect test behavior since whitespace is significant in diff comparisons. Normalizing the indentation would make the tests more reliable and accurate.	7
General	Improve whitespace handling consistency by using strip() instead of rstrip() The rstrip() operation followed by adding a newline could be simplified by using strip() followed by newline. This ensures consistent handling of both leading and trailing whitespace. pr_agent/algo/utils.py [591-592] -original_file_content_str = (original_file_content_str or "").rstrip() + "\n" -new_file_content_str = (new_file_content_str or "").rstrip() + "\n" +original_file_content_str = (original_file_content_str or "").strip() + "\n" +new_file_content_str = (new_file_content_str or "").strip() + "\n" Apply this suggestion Suggestion importance[1-10]: 3 Why: While the suggestion to use strip() instead of rstrip() is technically valid, the current rstrip() usage is intentional as it only removes trailing whitespace while preserving leading whitespace, which is often important in diff generation.	3

• Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

---

Previous suggestions

✅ Suggestions up to commit e431979

Category	Suggestion	Score
Possible issue	✅ Add null-safety checks to prevent potential runtime errors when handling string operations Consider handling empty strings and None values explicitly before applying string operations to prevent potential AttributeError exceptions. pr_agent/algo/utils.py [591-592] -original_file_content_str = original_file_content_str.rstrip() + "\n" -new_file_content_str = new_file_content_str.rstrip() + "\n" +original_file_content_str = (original_file_content_str or "").rstrip() + "\n" +new_file_content_str = (new_file_content_str or "").rstrip() + "\n" [Suggestion has been applied] Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential runtime vulnerability by adding null-safety checks, which is important since the code already has a condition checking for empty strings but doesn't handle None values explicitly.	8
General	Enhance test coverage by including edge cases for empty and null inputs Add test cases to verify the behavior of load_large_diff with None inputs and empty strings to ensure robust error handling. tests/unittest/test_extend_patch.py [164-173] def test_no_newline(self): patch = load_large_diff("test.py", """\ old content 1 some new content another line """, """ old content 1 old content 2""") +def test_empty_inputs(self): + assert load_large_diff("test.py", "", "") == "" + assert load_large_diff("test.py", None, None) == "" + assert load_large_diff("test.py", "content\n", "") == "..." # Assert expected diff + Suggestion importance[1-10]: 7 Why: The suggestion improves test coverage by adding important edge cases that verify the function's behavior with empty and null inputs, which complements the existing test and helps ensure robust error handling.	7