fix: raise ValueError for reversed page ranges in expand_page_range#11492
Open
basilalshukaili wants to merge 1 commit into
Open
fix: raise ValueError for reversed page ranges in expand_page_range#11492basilalshukaili wants to merge 1 commit into
basilalshukaili wants to merge 1 commit into
Conversation
A reversed page range like '5-3' would either raise a confusing 'No valid page numbers or ranges found' error (when it was the only entry) or silently drop the pages when mixed with valid ranges (e.g. ['1-3', '7-5', '8'] returned [1, 2, 3, 8] instead of raising). Add an explicit check that start <= end and raise a clear ValueError with a descriptive message identifying the problematic range. Also adds three new tests covering: - reversed range alone raises ValueError - reversed range mixed with valid ranges raises ValueError (no silent drop) - equal start and end (e.g. '3-3') is valid and returns [3]
|
@dhofaroffice365 is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
No issue filed; found while reviewing the code.
Proposed Changes
expand_page_rangesilently drops pages when a reversed range (e.g.'7-5') appears alongside valid entries.For example:
When the reversed range is the only entry, the error message is misleading:
The fix adds an explicit
start <= endcheck immediately after parsing the two parts, raising a clearValueErrorthat identifies the offending range. All other code paths are unchanged.How did you test it?
Added three new unit tests to
TestExpandPageRange:test_reversed_range_alone_raises_value_error— single reversed range raises a clear, specific errortest_reversed_range_mixed_raises_value_error— reversed range mixed with valid entries now raises instead of silently dropping pagestest_equal_start_end_is_valid— confirms'3-3'(valid single-page range notation) still returns[3]All existing tests continue to pass.
Notes for the reviewer
Change is limited to
haystack/utils/misc.py(3 lines added, 2 refactored) and the corresponding test file. No behaviour change for valid inputs.Checklist
fix:) for the PR title.