Merge the Alternate Number sort into the Alternate Series sort

.gitignore

@@ -95,6 +95,7 @@ profile_default/
 sdist/
 share/python-wheels/
 target/
+tasks/
 test-results
 TODO.md
 var/

NEWS.md

@@ -14,12 +14,13 @@ border-radius: 128px;
       them keeps each one's sort instead of carrying one sort everywhere. A
       collection you haven't sorted yet keeps whatever sort you arrive with.
     - Clearing a search puts back the sort the search replaced.
-    - Sort by Alternate Number, the issue number a comic carries inside its
-      alternate series (ComicInfo AlternateSeries / AlternateNumber). Filter by
-      an alternate series first to pick which one to sort by. Comics with no
-      alternate number fall back to their own issue number.
-    - The Alternate Series sort is available in cover view, not just the table.
-      Comics with no alternate series sort by their real series name.
+    - Sort by Alternate Series (ComicInfo AlternateSeries / AlternateNumber).
+      Comics group by their alternate series and order by its issue number
+      inside that group. Filter by an alternate series first to sort by that one
+      instead of the first one alphabetically. Comics with no alternate series
+      sort by their real series and issue, so they interleave rather than
+      clumping, and an alternate series with no issue numbers keeps its comics
+      in their own issue order. Available in cover view as well as the table.
     - Read an alternate series as a reading order: pick it in the reader's
       reading-order menu and next/prev follow the alternate numbering. Handy for
       using alternate series tags as durable reading lists.

codex/choices/browser.py

@@ -18,7 +18,6 @@
     {
         "created_at": "Added Time",
         "age_rating": "Age Rating",
-        "alternate_number": "Alternate Number",
         "reprints": "Alternate Series",
         "characters": "Characters",
         "child_count": "Child Count",
@@ -76,7 +75,6 @@
     {
         "created_at",
         "age_rating",
-        "alternate_number",
         "reprints",
         "child_count",
         "community_rating",
@@ -97,15 +95,12 @@
 # They sort fine as the primary, but the per-extra annotation
 # pipeline can't safely produce a value for them on every model
 # / context: ``story_arc_number`` requires StoryArc-context ``pks``
-# to resolve which arc's number to pick, ``alternate_number`` likewise
-# needs the ``reprints`` filter to resolve which alternate series'
-# number to pick, and ``search_score``'s ``ComicFTSRank`` only
-# resolves when an FTS subquery is joined.
+# to resolve which arc's number to pick, and ``search_score``'s
+# ``ComicFTSRank`` only resolves when an FTS subquery is joined.
 # Mirrored on the frontend so the table headers can grey out the
 # affected columns and refuse the shift-click.
 BROWSER_EXTRA_SORT_UNSUPPORTED_KEYS = frozenset(
     {
-        "alternate_number",
         "story_arc_number",
         "search_score",
     }

codex/librarian/scribe/importer/delete/comics.py

@@ -59,7 +59,9 @@ def _populate_deleted_comic_collections(
         """Populate changed collections for cover timestamp updater."""
         comics_deleted_qs = delete_qs.only(
             *ALL_COMIC_COLLECTION_FIELD_NAMES
-        ).prefetch_related("story_arc_numbers__story_arc", *DIRECT_M2M_COLLECTION_FIELD_NAMES)
+        ).prefetch_related(
+            "story_arc_numbers__story_arc", *DIRECT_M2M_COLLECTION_FIELD_NAMES
+        )
         for comic in comics_deleted_qs.iterator(
             chunk_size=IMPORTER_DELETE_MAX_CHUNK_SIZE
         ):

codex/migrations/0053_reprint_issue_number_and_collection_order_memory.py

@@ -1,10 +1,10 @@
 """
 Split Reprint.issue into sortable columns & remember sorts per collection.
 
-The ``alternate_number`` order_by key sorts comics by their
-ComicInfo ``AlternateNumber`` within an alternate series. ``issue``
-is a string, so sorting it directly puts "#10" before "#2"; these
-derived columns mirror ``Comic.issue_number`` / ``issue_suffix``.
+The Alternate Series order_by key sorts comics by their ComicInfo
+``AlternateNumber`` within an alternate series. ``issue`` is a string,
+so sorting it directly puts "#10" before "#2"; these derived columns
+mirror ``Comic.issue_number`` / ``issue_suffix``.
 
 ``SettingsBrowser.collection_order_memory`` remembers the sort each top
 collection was last browsed with. Switching top collections used to drag one
@@ -73,7 +73,6 @@ class Migration(migrations.Migration):
                 choices=[
                     ("created_at", "Added Time"),
                     ("age_rating", "Age Rating"),
-                    ("alternate_number", "Alternate Number"),
                     ("reprints", "Alternate Series"),
                     ("characters", "Characters"),
                     ("child_count", "Child Count"),

codex/migrations/0054_merge_alternate_number_sort.py

@@ -0,0 +1,102 @@
+"""
+Merge the Alternate Number sort into Alternate Series.
+
+The two sorts each did half the job — ``reprints`` ordered by a display
+label (so "#10" preceded "#2") and ``alternate_number`` ordered by the
+parsed number but ignored which alternate series it belonged to. They
+are now one key, ``reprints``.
+
+0053 dropped ``alternate_number`` from the field's choices, but choices
+aren't enforced by SQLite and 0053 has already been applied on every
+database that could hold the value, so the stored settings need this
+separate pass. Left in place, a stale key reaches ORDER BY unvalidated
+(settings load raw) and raises FieldError.
+
+The key lives in three places on SettingsBrowser — the ``order_by``
+column, the ``order_extra_keys`` list, and the per-top-collection
+``collection_order_memory`` map — and saved views are more rows in the
+same table, so every row is remapped regardless of name or client.
+"""
+
+from django.db import migrations
+
+_OLD_KEY = "alternate_number"
+_NEW_KEY = "reprints"
+
+
+def _remap_extra_keys(entries) -> tuple[list, bool]:
+    """Remap the sort key in an extras list, dropping a duplicate."""
+    if not isinstance(entries, list):
+        return entries, False
+    remapped: list = []
+    changed = False
+    seen: set = set()
+    for entry in entries:
+        key = entry.get("key") if isinstance(entry, dict) else None
+        if key == _OLD_KEY:
+            entry = {**entry, "key": _NEW_KEY}  # noqa: PLW2901
+            key = _NEW_KEY
+            changed = True
+        if key in seen:
+            # The row already sorted by the surviving key. Two entries
+            # for one column is not a state the sort can express, so the
+            # first occurrence wins, as the serializer's own extras
+            # cleaner does.
+            changed = True
+            continue
+        seen.add(key)
+        remapped.append(entry)
+    return remapped, changed
+
+
+def _remap_memory(memory) -> tuple[dict, bool]:
+    """Remap the sort key inside a collection_order_memory map."""
+    if not isinstance(memory, dict):
+        return memory, False
+    changed = False
+    for remembered in memory.values():
+        if not isinstance(remembered, dict):
+            continue
+        if remembered.get("order_by") == _OLD_KEY:
+            remembered["order_by"] = _NEW_KEY
+            changed = True
+        extras, extras_changed = _remap_extra_keys(remembered.get("order_extra_keys"))
+        if extras_changed:
+            remembered["order_extra_keys"] = extras
+            changed = True
+    return memory, changed
+
+
+def _remap_browser_settings(apps, _schema_editor) -> None:
+    settings_browser = apps.get_model("codex", "SettingsBrowser")
+    settings_browser.objects.filter(order_by=_OLD_KEY).update(order_by=_NEW_KEY)
+    # JSON payloads need a python pass; JSONField key lookups are
+    # unsupported on SQLite, so scan and rewrite sparsely.
+    rows = []
+    for row in settings_browser.objects.only(
+        "pk", "order_extra_keys", "collection_order_memory"
+    ):
+        extras, extras_changed = _remap_extra_keys(row.order_extra_keys)
+        memory, memory_changed = _remap_memory(row.collection_order_memory)
+        if extras_changed or memory_changed:
+            row.order_extra_keys = extras
+            row.collection_order_memory = memory
+            rows.append(row)
+    if rows:
+        settings_browser.objects.bulk_update(
+            rows, ["order_extra_keys", "collection_order_memory"]
+        )
+
+
+class Migration(migrations.Migration):
+    """Remap the retired alternate_number sort key onto reprints."""
+
+    dependencies = [
+        ("codex", "0053_reprint_issue_number_and_collection_order_memory"),
+    ]
+
+    operations = [
+        # Irreversible by design: both former keys map onto ``reprints``,
+        # so a reverse pass can't know which rows to send back.
+        migrations.RunPython(_remap_browser_settings, migrations.RunPython.noop),
+    ]

codex/models/named.py

@@ -142,7 +142,7 @@ class Reprint(BaseModel):
     identifier = ForeignKey(Identifier, on_delete=SET_NULL, null=True)
     # ``issue`` split into its sortable parts, mirroring
     # ``Comic.issue_number`` / ``issue_suffix``. Without them the
-    # ``alternate_number`` sort would order "#10" before "#2". Derived
+    # Alternate Series sort would order "#10" before "#2". Derived
     # in ``presave``, never imported directly; unindexed because they're
     # only read after an indexed join on pk or series_name.
     issue_number = CoercingDecimalField(decimal_places=2, max_digits=10, null=True)

codex/user_data/restore.py

@@ -553,8 +553,19 @@ def _restore_settings_browser(
 
 # Filter/sort keys renamed across codex versions; sidecar backups from
 # older versions still carry the old name (0048: critical -> community).
+# This map also resolves *filter* columns by their legacy name, so only
+# renames that applied to a filter column belong here.
 _LEGACY_KEY_RENAMES: Final[dict[str, str]] = {"critical_rating": "community_rating"}
 
+# Sort keys retired into another key. Sort-only, because there was never
+# an ``alternate_number`` filter column for ``_resolve_filter_column``
+# to look for (0054: the Alternate Number sort merged into Alternate
+# Series).
+_SORT_KEY_RENAMES: Final[dict[str, str]] = {
+    **_LEGACY_KEY_RENAMES,
+    "alternate_number": "reprints",
+}
+
 
 def _resolve_filter_column(row_keys, column: str) -> str | None:
     """Sidecar column holding ``column``'s value: itself, its legacy name, or None."""
@@ -577,10 +588,49 @@ def _row_column(row, column: str):
         return None
 
 
+def _rename_sort_key(key) -> str:
+    """Rename one retired sort key. Sidecar JSON can hold anything."""
+    key = key if isinstance(key, str) else ""
+    return _SORT_KEY_RENAMES.get(key, key)
+
+
+def _rename_extra_keys(entries) -> list:
+    """Rename retired sort keys in an extras list, dropping duplicates."""
+    if not isinstance(entries, list):
+        return []
+    renamed: list = []
+    seen: set[str] = set()
+    for entry in entries:
+        if not isinstance(entry, dict):
+            continue
+        key = _rename_sort_key(entry.get("key"))
+        if key in seen:
+            # Both a retired key and its replacement were stored; one
+            # column can only carry one sort, so the first one wins.
+            continue
+        seen.add(key)
+        renamed.append({**entry, "key": key})
+    return renamed
+
+
+def _rename_memory_keys(memory) -> dict:
+    """Rename retired sort keys inside a collection_order_memory map."""
+    if not isinstance(memory, dict):
+        return {}
+    for remembered in memory.values():
+        if not isinstance(remembered, dict):
+            continue
+        remembered["order_by"] = _rename_sort_key(remembered.get("order_by"))
+        remembered["order_extra_keys"] = _rename_extra_keys(
+            remembered.get("order_extra_keys")
+        )
+    return memory
+
+
 def _build_browser_defaults(row, show) -> dict[str, Any]:
     """Map a sidecar settings_browser row to ``update_or_create`` defaults."""
     order_by = row["order_by"] or ""
-    order_by = _LEGACY_KEY_RENAMES.get(order_by, order_by)
+    order_by = _SORT_KEY_RENAMES.get(order_by, order_by)
     table_columns = json.loads(row["table_columns"] or "{}")
     for old, new in _LEGACY_KEY_RENAMES.items():
         if old in table_columns:
@@ -590,9 +640,11 @@ def _build_browser_defaults(row, show) -> dict[str, Any]:
         "top_collection": row["top_collection"] or "",
         "order_by": order_by,
         "order_reverse": bool(row["order_reverse"]),
-        "order_extra_keys": json.loads(row["order_extra_keys"] or "[]"),
-        "collection_order_memory": json.loads(
-            _row_column(row, "collection_order_memory") or "{}"
+        "order_extra_keys": _rename_extra_keys(
+            json.loads(row["order_extra_keys"] or "[]")
+        ),
+        "collection_order_memory": _rename_memory_keys(
+            json.loads(_row_column(row, "collection_order_memory") or "{}")
         ),
         "search": row["search"] or "",
         "custom_covers": bool(row["custom_covers"]),

codex/views/browser/annotate/order.py

@@ -12,7 +12,7 @@
 )
 from django.db.models.aggregates import Avg, Count, Max, Min, Sum
 from django.db.models.fields import CharField
-from django.db.models.functions import Coalesce, Reverse, Right, StrIndex
+from django.db.models.functions import Reverse, Right, StrIndex
 
 from codex.choices.browser import BROWSER_EXTRA_SORT_UNSUPPORTED_KEYS
 from codex.models import (
@@ -84,7 +84,6 @@
 _ANNOTATED_ORDER_FIELDS = frozenset(
     # These are annotated with their own functions
     {
-        "alternate_number",
         "bookmark_updated_at",
         "child_count",
         "favorite",
@@ -227,46 +226,6 @@ def _alias_story_arc_number(self, qs):
 
         return qs.alias(story_arc_number=story_arc_number)
 
-    def _alias_alternate_number(self, qs):
-        """Alias the alternate series issue number & suffix for ordering."""
-        if self.order_key != "alternate_number":
-            return qs
-
-        # Unlike ``story_arc_number`` there's no alternate series browse
-        # collection, so the ``reprints`` filter is the only thing that can
-        # say *which* alternate series' number to sort by.
-        reprint_pks = self.params.get("filters", {}).get("reprints", ())
-        # ``self.rel_prefix`` is memoized off the *view's* model, but this
-        # runs for the book queryset too — take the prefix from the
-        # queryset being annotated, as ``_alias_story_arc_number`` does.
-        rel_prefix = self.get_rel_prefix(qs.model)
-        own_number = rel_prefix + "issue_number"
-        own_suffix = rel_prefix + "issue_suffix"
-
-        if reprint_pks:
-            rel = rel_prefix + "reprints"
-            condition = Q(**{f"{rel}__pk__in": reprint_pks})
-            qs = qs.alias(selected_reprint=FilteredRelation(rel, condition=condition))
-            # Comics carrying no alternate number fall back to their own
-            # issue number so a mixed listing stays readable instead of
-            # collapsing every untagged comic to NULL. Coalescing *inside*
-            # the aggregate lets each comic contribute its own effective
-            # value; coalescing outside would compare one series' minimum
-            # alternate number against another's minimum issue number.
-            number = self.order_agg_func(
-                Coalesce("selected_reprint__issue_number", own_number)
-            )
-            suffix = self.order_agg_func(
-                Coalesce("selected_reprint__issue_suffix", own_suffix)
-            )
-        else:
-            # No alternate series selected: degrade to the plain issue sort
-            # rather than ordering everything by NULL.
-            number = self.order_agg_func(own_number)
-            suffix = self.order_agg_func(own_suffix)
-
-        return qs.alias(alternate_number=number, alternate_number_suffix=suffix)
-
     def _annotate_page_count(self, qs):
         """Hoist up total page_count of children."""
         # Used for sorting and progress
@@ -604,7 +563,6 @@ def annotate_order_aggregates(self, qs: QuerySet, *, for_cover: bool = False):
         qs = self._alias_sort_names(qs)
         qs = self._alias_filename(qs)
         qs = self._alias_story_arc_number(qs)
-        qs = self._alias_alternate_number(qs)
         if not for_cover:
             qs = self._annotate_page_count(qs)
         qs = self._annotate_bookmark_updated_at(qs)

codex/views/browser/browser.py

@@ -192,16 +192,20 @@ def _add_table_view_sort_annotations(self, qs):
             return qs
         fk_anns = fk_name_annotations_for(sort_keys)
         m2m_anns = m2m_annotations_for(sort_keys)
-        # Every key that sorts through a fallback alias needs it annotated,
-        # extras included — ``_comic_extra_fields`` resolves an extra to the
-        # same alias the primary uses.
-        m2m_sort_anns = m2m_sort_annotations_for(sort_keys)
+        # Every key that sorts through elected-value aliases needs them
+        # annotated, extras included — ``_comic_extra_fields`` resolves an
+        # extra to the same aliases the primary uses. An active reprints
+        # filter names which alternate series the user is looking at, so
+        # the election is narrowed to it instead of picking the
+        # alphabetically first one.
+        reprint_pks = self.params.get("filters", {}).get("reprints", ())
+        m2m_sort_anns = m2m_sort_annotations_for(sort_keys, reprint_pks)
         if fk_anns:
             qs = qs.annotate(**fk_anns)
         if m2m_anns:
             qs = qs.annotate(**m2m_anns)
         if m2m_sort_anns:
-            qs = qs.annotate(**m2m_sort_anns)
+            qs = qs.alias(**m2m_sort_anns)
         return qs
 
     def _add_table_view_display_annotations(self, qs):