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

openlibrary/core/lists/model.py

@@ -2,10 +2,11 @@
 
 import contextlib
 import logging
+import re
 import typing
 from collections.abc import Iterable
 from functools import cached_property
-from typing import NotRequired, TypedDict, cast
+from typing import Any, NotRequired, TypedDict, cast
 
 import web
 
@@ -681,15 +682,24 @@ def get_seeds(self, sort=False, resolve_redirects=False) -> list['Seed']:
             (work, edge) for work in works if (edge := work.find_series_edge(self.key))
         ]
 
-        def get_work_position(position: str | None) -> float:
-            position_float = 9999.0
+        def get_work_sort_key(
+            tpl: tuple[Work, dict[Any, Any]],
+        ) -> tuple[str, int, float, str]:
+            work, edge = tpl
+            position = edge.get('position')
+            pos_str = str(position or "").strip()
+
             with contextlib.suppress(ValueError):
-                position_float = float(position or 9999)
-            return position_float
+                return ("A: Numeric", 1, float(pos_str), work.key)
 
-        sorted_edges = sorted(
-            series_edges, key=lambda tpl: get_work_position(tpl[1].get('position'))
-        )
+            if match := re.fullmatch(r'(\d+)\s*-\s*(\d+)', pos_str):
+                lower = int(match.group(1))
+                upper = int(match.group(2))
+                return ("C: Range", upper - lower, float(lower), work.key)
+
+            return ("B: Non-numeric", 0, 0.0, work.key)
+
+        sorted_edges = sorted(series_edges, key=get_work_sort_key)  # type: ignore[arg-type]
 
         seeds: list[Seed] = []
         for work, edge in sorted_edges:

openlibrary/tests/core/lists/test_model.py

@@ -1,3 +1,5 @@
+import web
+
 import openlibrary.core.lists.model as list_model
 from openlibrary.mocks.mock_infobase import MockSite
 
@@ -27,3 +29,63 @@ def save_doc(self, site, type, key, **fields):
         d = {"key": key, "type": {"key": type}}
         d.update(fields)
         site.save(d)
+
+
+class MockWork:
+    def __init__(self, key, position):
+        self.key = key
+        self._position = position
+
+    def find_series_edge(self, series_key):
+        return {"position": self._position}
+
+
+class MockSeriesSite:
+    def __init__(self, works):
+        self._works = works
+
+    def things(self, query):
+        return [work.key for work in self._works]
+
+    def get_many(self, keys):
+        return self._works
+
+
+class TestSeries:
+    def test_series_sorting(self, monkeypatch):
+        works = [
+            MockWork("/works/OL5W", "2"),
+            MockWork("/works/OL1W", "22-23"),
+            MockWork("/works/OL2W", "1"),
+            MockWork("/works/OL3W", "3"),
+            MockWork("/works/OL4W", "24-25"),
+            MockWork("/works/OL8W", "Non-numeric"),
+            MockWork("/works/OL7W", "4-5"),
+            MockWork("/works/OL6W", "1-3"),
+            MockWork("/works/OL9W", "1-9"),
+            MockWork("/works/OL10W", None),
+            MockWork("/works/OL11W", "3"),
+        ]
+
+        mock_site = MockSeriesSite(works)
+        monkeypatch.setattr(web, "ctx", web.storage(site=mock_site))
+
+        series = list_model.Series(mock_site, "/series/OL1L")
+        seeds = series.get_seeds()
+
+        result_keys = [seed.key for seed in seeds]
+        expected_keys = [
+            "/works/OL2W",  # 1
+            "/works/OL5W",  # 2
+            "/works/OL11W",  # 3 (tie-break by work key)
+            "/works/OL3W",  # 3
+            "/works/OL10W",  # None -> non-numeric bucket
+            "/works/OL8W",  # Non-numeric
+            "/works/OL7W",  # 4-5   (range size 1, lower 4)
+            "/works/OL1W",  # 22-23 (range size 1, lower 22)
+            "/works/OL4W",  # 24-25 (range size 1, lower 24)
+            "/works/OL6W",  # 1-3   (range size 2, lower 1)
+            "/works/OL9W",  # 1-9   (range size 8, lower 1)
+        ]
+
+        assert result_keys == expected_keys