Skip to content

fix: raise ValueError for reversed page ranges in expand_page_range#11492

Open
basilalshukaili wants to merge 1 commit into
deepset-ai:mainfrom
basilalshukaili:fix/expand-page-range-reversed-range
Open

fix: raise ValueError for reversed page ranges in expand_page_range#11492
basilalshukaili wants to merge 1 commit into
deepset-ai:mainfrom
basilalshukaili:fix/expand-page-range-reversed-range

Conversation

@basilalshukaili
Copy link
Copy Markdown

Related Issues

No issue filed; found while reviewing the code.

Proposed Changes

expand_page_range silently drops pages when a reversed range (e.g. '7-5') appears alongside valid entries.

For example:

expand_page_range(["1-3", "7-5", "8"])
# Returns [1, 2, 3, 8]  — pages 5, 6, 7 silently lost, no error raised

When the reversed range is the only entry, the error message is misleading:

expand_page_range(["5-3"])
# Raises: ValueError: No valid page numbers or ranges found in the input list
# (says nothing about which range caused the problem or why)

The fix adds an explicit start <= end check immediately after parsing the two parts, raising a clear ValueError that 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 error
  • test_reversed_range_mixed_raises_value_error — reversed range mixed with valid entries now raises instead of silently dropping pages
  • test_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

  • I have read the contributors guidelines and the code of conduct.
  • I have added unit tests and updated the docstring is not needed (the docstring already says it raises ValueError for invalid ranges).
  • I have added a release note.
  • I used a conventional commit type (fix:) for the PR title.

AI-generated contribution: This PR was prepared with AI assistance. The bug was identified by code inspection and verified by running the function with the affected inputs.

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]
@basilalshukaili basilalshukaili requested a review from a team as a code owner June 2, 2026 20:38
@basilalshukaili basilalshukaili requested review from bogdankostic and removed request for a team June 2, 2026 20:38
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 2, 2026

@dhofaroffice365 is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants