• 
      

    Enhance interdiff filtering to avoid filtering swapped lines.

    Review Request #15206 — Created July 29, 2026 and updated

    Information

    Review Board
    release-8.x

    Reviewers

    When generating an interdiff that swaps a line below another, the
    algorithm would generally opt to move the first line below the second,
    representing the second as an "equals" line. The problem is, this second
    line could have been newly-added in the original revision, but due to
    the way the interdiff is generated, the equals line's range would not
    overlap the original's range.

    This change updates the algorithm to consider these cases. Now, if an
    equal op is found, it backtracks, looking to see if a filtered-equal
    precedes it. This will be enough to trigger combining the new range with
    the filtered range into one big range and putting the filtered op and
    current op back in the list of pending opcodes. This will cause the full
    extent of the range to be reconsidered for any overlaps.

    The result of this should be that line swaps should be represented
    correctly in interdiffs, with minimal upstream line leaks (though there
    may be some in practice).

    Further improvements on this would involve peeking ahead to determine if
    there's an insert/replace following the equal, which would be more
    likely to be this situation. That would require a bit more work to how
    we process and re-queue opcodes (we'd need to be able to consume the
    next opcode and then put it back in a buffer that's lower precedence
    than pending_opcodes).

    Unit tests passed, covering all our provided sample datasets.

    Manually tested with some customer-provided datasets, verifying
    they were fixed.

    Summary ID
    Enhance interdiff filtering to avoid filtering swapped lines.
    When generating an interdiff that swaps a line below another, the algorithm would generally opt to move the first line below the second, representing the second as an "equals" line. The problem is, this second line could have been newly-added in the original revision, but due to the way the interdiff is generated, the equals line's range would not overlap the original's range. This change updates the algorithm to consider these cases. Now, if an `equal` op is found, it backtracks, looking to see if a `filtered-equal` precedes it. This will be enough to trigger combining the new range with the filtered range into one big range and putting the filtered op and current op back in the list of pending opcodes. This will cause the full extent of the range to be reconsidered for any overlaps. The result of this should be that line swaps should be represented correctly in interdiffs, with minimal upstream line leaks (though there may be some in practice). Further improvements on this would involve peeking ahead to determine if there's an `insert`/`replace` following the `equal`, which would be more likely to be this situation. That would require a bit more work to how we process and re-queue opcodes (we'd need to be able to consume the next opcode and then put it back in a buffer that's lower precedence than `pending_opcodes`).
    731cbc7bfbe659f0ce57481ab8fb9bf45c75ba10
    Description From Last Updated

    orig_needs_combine can accept replace opcodes, but the unit tests only cover delete. Is replace actually reachable given that this is …

    david david

    Can we add a new Version Changed entry to this file regarding these new changes for 8.1?

    david david

    This is no longer true. If it's an equal, we check to see whether it needs to be combined with …

    david david
    Checks run (2 succeeded)
    flake8 passed.
    JSHint passed.
    david
    1. 
        
    2. Show all issues

      orig_needs_combine can accept replace opcodes, but the unit tests only cover delete.

      Is replace actually reachable given that this is supposed to happen. with delete/equal/insert? If not, we should probably remove that from the set, and if so, does the new-side bookkeeping need rewinding too (and can we get a test)?

      1. It's precautionary and theoretical. I don't have a working test case for this yet. I'm still trying to build test cases around this. I'll circle back to this.

    3. reviewboard/diffviewer/processors.py (Diff revision 1)
       
       
       
      Show all issues

      Can we add a new Version Changed entry to this file regarding these new changes for 8.1?

    4. reviewboard/diffviewer/processors.py (Diff revision 1)
       
       
      Show all issues

      This is no longer true. If it's an equal, we check to see whether it needs to be combined with a preceding filtered range (phase 1b)

    5.