• 
      

    Fix errors handling interdiff chunks missing line numbers on a side.

    Review Request #15181 — Created July 21, 2026 and updated

    Information

    Review Board
    release-8.x

    Reviewers

    Expanding a collapsed chunk in an interdiff could fail with a
    TypeError comparing an int to a str. The same failure could occur
    when computing the line ranges shown for a comment in review e-mail.

    Diff lines store a missing line number as an empty string. Interdiffs
    filter out unrelated opcodes and merge the results back into "equal"
    chunks, which can leave one side of a chunk with fewer lines than the
    other. The lines at the end of such a chunk have an empty line number on
    the shorter side.

    Two places assumed the last line of a chunk always had a line number for
    both sides. DiffRenderer.make_context() used it to filter the
    function/class headers down to the newly expanded range, and
    get_displayed_diff_line_ranges() used it to cap the end of a comment's
    displayed range. Both compared or added it to an int, which failed
    whenever the last line was missing that side.

    _get_last_header_in_chunks_before_line() already had a nested
    find_last_line_numbers() that scanned backwards for the last line with
    a number on each side, which described this exact case. I've moved that
    into a module-level function in diffutils, and both of the above call
    it.

    • Reproduced the chunk expansion error from a production traceback in a
      unit test, and verified the fix.
    • Verified the new get_displayed_diff_line_ranges() tests failed
      before the fix, with the same error.
    • Ran all unit tests.
    Summary ID
    Fix errors handling interdiff chunks missing line numbers on a side.
    Expanding a collapsed chunk in an interdiff could fail with a `TypeError` comparing an `int` to a `str`. The same failure could occur when computing the line ranges shown for a comment in review e-mail. Diff lines store a missing line number as an empty string. Interdiffs filter out unrelated opcodes and merge the results back into "equal" chunks, which can leave one side of a chunk with fewer lines than the other. The lines at the end of such a chunk have an empty line number on the shorter side. Two places assumed the last line of a chunk always had a line number for both sides. `DiffRenderer.make_context()` used it to filter the function/class headers down to the newly expanded range, and `get_displayed_diff_line_ranges()` used it to cap the end of a comment's displayed range. Both compared or added it to an int, which failed whenever the last line was missing that side. `_get_last_header_in_chunks_before_line()` already had a nested `find_last_line_numbers()` that scanned backwards for the last line with a number on each side, which described this exact case. I've moved that into a module-level function in `diffutils`, and both of the above call it. Testing Done: - Reproduced the chunk expansion error from a production traceback in a unit test, and verified the fix. - Verified the new `get_displayed_diff_line_ranges()` tests failed before the fix, with the same error. - Ran all unit tests.
    tnkxsumwozqvltnnwsuzskvwvkymszoo
    Description From Last Updated

    This is new module-level API, so I think we can do Version Added here.

    chipx86 chipx86

    Can you update this to use the standard Tuple: directive?

    chipx86 chipx86

    Can you update this to use the standard multi-line list forms?

    chipx86 chipx86

    Here, too.

    chipx86 chipx86
    chipx86
    1. 
        
    2. reviewboard/diffviewer/diffutils.py (Diff revision 1)
       
       
       
      Show all issues

      This is new module-level API, so I think we can do Version Added here.

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

      Can you update this to use the standard Tuple: directive?

    4. Show all issues

      Can you update this to use the standard multi-line list forms?

    5. Show all issues

      Here, too.

    6. 
        
    david
    Review request changed
    Commits:
    Summary ID
    Fix errors handling interdiff chunks missing line numbers on a side.
    Expanding a collapsed chunk in an interdiff could fail with a `TypeError` comparing an `int` to a `str`. The same failure could occur when computing the line ranges shown for a comment in review e-mail. Diff lines store a missing line number as an empty string. Interdiffs filter out unrelated opcodes and merge the results back into "equal" chunks, which can leave one side of a chunk with fewer lines than the other. The lines at the end of such a chunk have an empty line number on the shorter side. Two places assumed the last line of a chunk always had a line number for both sides. `DiffRenderer.make_context()` used it to filter the function/class headers down to the newly expanded range, and `get_displayed_diff_line_ranges()` used it to cap the end of a comment's displayed range. Both compared or added it to an int, which failed whenever the last line was missing that side. `_get_last_header_in_chunks_before_line()` already had a nested `find_last_line_numbers()` that scanned backwards for the last line with a number on each side, which described this exact case. I've moved that into a module-level function in `diffutils`, and both of the above call it. Testing Done: - Reproduced the chunk expansion error from a production traceback in a unit test, and verified the fix. - Verified the new `get_displayed_diff_line_ranges()` tests failed before the fix, with the same error. - Ran all unit tests.
    tnkxsumwozqvltnnwsuzskvwvkymszoo
    Fix errors handling interdiff chunks missing line numbers on a side.
    Expanding a collapsed chunk in an interdiff could fail with a `TypeError` comparing an `int` to a `str`. The same failure could occur when computing the line ranges shown for a comment in review e-mail. Diff lines store a missing line number as an empty string. Interdiffs filter out unrelated opcodes and merge the results back into "equal" chunks, which can leave one side of a chunk with fewer lines than the other. The lines at the end of such a chunk have an empty line number on the shorter side. Two places assumed the last line of a chunk always had a line number for both sides. `DiffRenderer.make_context()` used it to filter the function/class headers down to the newly expanded range, and `get_displayed_diff_line_ranges()` used it to cap the end of a comment's displayed range. Both compared or added it to an int, which failed whenever the last line was missing that side. `_get_last_header_in_chunks_before_line()` already had a nested `find_last_line_numbers()` that scanned backwards for the last line with a number on each side, which described this exact case. I've moved that into a module-level function in `diffutils`, and both of the above call it. Testing Done: - Reproduced the chunk expansion error from a production traceback in a unit test, and verified the fix. - Verified the new `get_displayed_diff_line_ranges()` tests failed before the fix, with the same error. - Ran all unit tests.
    tnkxsumwozqvltnnwsuzskvwvkymszoo

    Checks run (2 succeeded)

    flake8 passed.
    JSHint passed.