Fix single file diff collapsed bug

What does this MR do and why?

Problem

  • In a previous MR (!44629 (merged)), a functionality was added to automatically expand single diff files even when they exceed normal auto-expansion limits.
  • However, when a MR contains a single large file diff, it may incorrectly appear collapsed due to inconsistent handling of diff counts between Gitaly and Array iterators. This especially happens when both Show whitespace changes and Show one file at a time settings are disabled.

Potential Root Cause

  • This occurs because @iterator.size returns nil in the Gitaly case (due to streaming response), causing the expansion logic to fail, while it works correctly for Array iterators. Screenshot_2025-02-10_at_11.19.18_AM
  • With Gitaly's DiffStitcher, a single diff can be split across multiple messages. The current @iterator.size approach fails because:
    • It delegates to the gRPC streaming response's size, which returns nil (size cannot be determined until the stream is consumed I believe)
    • Even if size was available, it would count messages rather than complete diffs (for example: one diff split into 3 messages would incorrectly trigger collapse when it should be expanded)

Solution

  • Modify DiffStitcher to track complete diffs internally by:
    • Adding a @diff_count counter in DiffStitcher that increments only when a complete diff is assembled (end_of_patch)
    • Implementing a size method that returns the actual number of complete diffs
    • Removing delegation to the gRPC response's size method

An alternative approach would be to increment the diff counter in each_gitaly_patch and each_serialized_patch methods. However, encapsulating the counting logic within DiffStitcher provides a cleaner and more maintainable solution.

Testing

Note: make sure both Show whitespace changes and Show one file at a time settings are disabled

  • Checkout to master branch and create a MR with 1 large file. It should be collapsed
  • Checkout to 516100-fix-single-diff-file-collapsed-bug and create a MR with 1 large file. It shouldn't be collapsed
  • Checkout to 516100-fix-single-diff-file-collapsed-bug and create a MR with 1 large file and 1 normal file. Large file should be collapsed and normal file shouldn't be collapsed

Screenshots

Before After
master_branch this_branch

References

Please include cross links to any resources that are relevant to this MR. This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #516100 (closed)

Edited by Kinshuk Singh

Merge request reports

Loading
Loading