Skip to content

Fix series sorting for numeric, non-numeric, and range positions#12186

Merged
cdrini merged 7 commits intointernetarchive:masterfrom
Harshul23:12153/hotfix/series-position-sorting
Apr 14, 2026
Merged

Fix series sorting for numeric, non-numeric, and range positions#12186
cdrini merged 7 commits intointernetarchive:masterfrom
Harshul23:12153/hotfix/series-position-sorting

Conversation

@Harshul23
Copy link
Copy Markdown
Contributor

Closes #12153

fix

Technical

This PR updates series sorting in openlibrary/core/lists/model.py to handle mixed position formats more predictably.

Previously, the sorting logic reduced position to a single float, which worked for simple numeric values but did not handle range-based entries like 1-3, 22-23, and 24-25 well.

This change replaces the float-based sort key with a tuple-based sort key so series items are ordered as:

  • numeric positions first
  • non-numeric positions next
  • range-based positions last

For range-based positions, sorting is further stabilized by:

  • range size (max - min)
  • lower bound
  • work key as a deterministic tie-breaker

Testing

Verified with:

pytest openlibrary/tests/core/lists/test_model.py

Result:

  • TestList : test_owner passed
  • TestSeries : test_series_sorting passed

Manual verification:

  • checked the affected series ordering behavior locally
  • confirmed single-number entries appear first
  • confirmed non-numeric entries are handled safely
  • confirmed range-based entries are sorted consistently and deterministically
  • confirmed Refresh stability test

Screenshot

Sorting behavior was manually verified locally on the affected series page after the fix.

Screen.Recording.2026-03-25.at.1.40.39.AM.mov

Stakeholders

@cdrini @mekarpeles

Copilot AI review requested due to automatic review settings March 24, 2026 20:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates series seed ordering in the Lists/Series model so that mixed position formats (numeric, non-numeric, and ranges) sort in a more predictable, deterministic way, addressing incorrect ordering on series pages (e.g., 3-4 appearing before 1-2).

Changes:

  • Replaces float-based series position sorting with a tuple-based sort key that buckets numeric, non-numeric, and range positions.
  • Adds range parsing via regex and additional tie-breakers for deterministic ordering.
  • Adds/extends unit test coverage for series sorting across numeric, non-numeric, range, and None positions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
openlibrary/core/lists/model.py Reworks Series.get_seeds() sorting logic to handle numeric/non-numeric/range positions deterministically.
openlibrary/tests/core/lists/test_model.py Adds a focused test validating the intended ordering across mixed position formats.

Comment thread openlibrary/core/lists/model.py Outdated
Comment thread openlibrary/core/lists/model.py Outdated
Comment thread openlibrary/tests/core/lists/test_model.py
Copy link
Copy Markdown
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice looks great! Two suggestions to fix the type error and making the unit tests a bit simpler.

Comment thread openlibrary/core/lists/model.py Outdated
Comment thread openlibrary/core/lists/model.py Outdated
@RayBB RayBB requested a review from cdrini March 27, 2026 21:48
@RayBB
Copy link
Copy Markdown
Collaborator

RayBB commented Mar 27, 2026

@Harshul23 please request review from drini (as I have done now) or tag him when you're ready for review again (which it seems you are)

@Harshul23
Copy link
Copy Markdown
Contributor Author

Thanks I will make sure to re-request a review after addressing changes next time.

@github-actions github-actions Bot added the Needs: Response Issues which require feedback from lead label Mar 28, 2026
Copy link
Copy Markdown
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! Tested on testing.

@cdrini cdrini merged commit 926931f into internetarchive:master Apr 14, 2026
4 checks passed
jack-wines pushed a commit to jack-wines/openlibrary that referenced this pull request Apr 18, 2026
…series-position-sorting

Fix series sorting for numeric, non-numeric, and range positions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Response Issues which require feedback from lead On Testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Work series - Collections are listed in reverse order

4 participants