Online tagging saves series years, status and summary

NEWS.md

@@ -10,6 +10,7 @@ border-radius: 128px;
 
 - Features
     - Series years, status and summary in the metadata pane.
+    - Online tagging saves series years, status and summary.
     - Browser title shows series years and a Tags button.
     - Tag editor edits the MetronInfo series start year.
 

codex/librarian/scribe/importer/read/aggregate_path.py

@@ -123,6 +123,9 @@ def _aggregate_path(self, path, status) -> None:
         """Aggregate metadata for one path."""
         # Prepare
         md = self.metadata[EXTRACTED].pop(path)
+        if series_attrs := self.task.series_attrs.get(path):
+            # A tag write's values the file couldn't hold. See ImportTask.
+            md[SERIES_KEY] = {**(md.get(SERIES_KEY) or {}), **series_attrs}
         self._transform_metadata(md)
 
         # Aggregate

codex/librarian/scribe/importer/tasks.py

@@ -2,6 +2,7 @@
 
 from collections.abc import Mapping
 from dataclasses import dataclass, field
+from typing import Any
 
 from codex.librarian.scribe.tasks import ScribeTask
 
@@ -31,6 +32,14 @@ class ImportTask(ScribeTask):
 
     force_import_metadata: bool = False
     check_metadata_mtime: bool = True
+    # path -> series attributes laid over the series that file names.
+    #
+    # Set only by the TagWriter's re-read, for values a tag write was
+    # given but no format it writes can hold. Applying them during the
+    # import, rather than to the comic's series row beforehand, lands them
+    # on the row the comic ends up in: a write that names a new publisher
+    # or series moves the comic to a different row.
+    series_attrs: Mapping[str, Mapping[str, Any]] = field(default_factory=dict)
     # Keep vanished rows instead of deleting them.
     #
     # Set by the two filesystem scanners, the poller and the watcher,

codex/librarian/scribe/tag_writer.py

@@ -35,6 +35,7 @@
     predict_name,
     will_convert,
 )
+from codex.librarian.scribe.tagwrite_series_attrs import patch_series_attrs
 from codex.librarian.scribe.timestamp_update import TimestampUpdater
 from codex.librarian.worker import WorkerStatusAbortableBase
 from codex.models.comic import Comic
@@ -297,7 +298,7 @@ def write_tags(self, task: BulkTagWriteTask) -> None:
         )
 
         self._enqueue_rereads(
-            current_paths, written_paths, converted_paths, post_renamed, lib_of
+            task, current_paths, written_paths, converted_paths, post_renamed, lib_of
         )
         num_renamed = len(renamed_paths) + len(post_renamed)
         reason = (
@@ -675,6 +676,7 @@ def _sync_conversions(
 
     def _enqueue_rereads(
         self,
+        task: BulkTagWriteTask,
         current_paths: dict[int, Path],
         written_paths: dict[int, Path],
         converted_paths: dict[int, Path],
@@ -690,19 +692,29 @@ def _enqueue_rereads(
         eventually re-read from the watcher's own modify event, but only
         with the import-metadata flag on, so the re-read is requested
         either way.
+
+        Each comic's patch may carry series attributes the written formats
+        can't hold. They ride along for the importer to apply (see
+        ``tagwrite_series_attrs``).
         """
         modified: defaultdict[int, set[str]] = defaultdict(set)
         created: defaultdict[int, set[str]] = defaultdict(set)
+        series_attrs: defaultdict[int, dict[str, dict]] = defaultdict(dict)
         for pk, written_path in written_paths.items():
             library_id = lib_of[pk]
             if pk in converted_paths:
-                modified[library_id].add(str(converted_paths[pk]))
+                path = str(converted_paths[pk])
+                modified[library_id].add(path)
             elif written_path != current_paths[pk]:
                 # A kept original's new CBZ is a file the database has
                 # never seen.
-                created[library_id].add(str(post_renamed.get(pk, written_path)))
+                path = str(post_renamed.get(pk, written_path))
+                created[library_id].add(path)
             else:
-                modified[library_id].add(str(written_path))
+                path = str(written_path)
+                modified[library_id].add(path)
+            if attrs := patch_series_attrs(self._patch_for(task, pk)):
+                series_attrs[library_id][path] = attrs
         # A rename with no write changed no metadata, so nothing to re-read.
         for library_id in modified.keys() | created.keys():
             import_task = ImportTask(
@@ -711,5 +723,6 @@ def _enqueue_rereads(
                 files_created=frozenset(created.get(library_id, ())),
                 force_import_metadata=True,
                 check_metadata_mtime=False,
+                series_attrs=series_attrs.get(library_id, {}),
             )
             self.librarian_queue.put(import_task)

codex/librarian/scribe/tagwrite_series_attrs.py

@@ -0,0 +1,57 @@
+"""
+Series attributes a tag write saves to the database.
+
+An online match brings the series' years, status and summary, but no
+format codex writes holds the end year, the status or the summary, and
+ComicInfo has no start year either. The re-read that follows a write
+only sees the file, so on their own these values would be lost.
+
+The tag writer hands them to that re-read instead, and the importer lays
+them over the series the file names. That puts them on the row the comic
+lands on, which is a different row whenever the write names a new
+publisher or series. The importer's batch rules settle comics of one
+series that disagree: the earliest start year, the latest end year and
+the first comic's status and summary. A later write's values replace a
+stored row's. Empty values are left out, so a write never clears one.
+"""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING, Any, Final
+
+from comicbox.enums.comicbox import SeriesStatusEnum
+from comicbox.formats.comicbox.schema import SERIES_KEY
+
+from codex.librarian.scribe.importer.const import (
+    END_YEAR_FIELD_NAME,
+    START_YEAR_FIELD_NAME,
+    STATUS_FIELD_NAME,
+    SUMMARY_FIELD_NAME,
+)
+
+if TYPE_CHECKING:
+    from collections.abc import Mapping
+
+_SERIES_ATTR_KEYS: Final = (
+    START_YEAR_FIELD_NAME,
+    END_YEAR_FIELD_NAME,
+    STATUS_FIELD_NAME,
+    SUMMARY_FIELD_NAME,
+)
+
+
+def _status_value(status: Any) -> str | None:
+    """Return a status as ``SeriesStatusChoices`` stores it, or None."""
+    try:
+        return SeriesStatusEnum(status).value
+    except ValueError:
+        return None
+
+
+def patch_series_attrs(patch: Mapping[str, Any] | None) -> dict[str, Any]:
+    """Return the non-empty series attributes a tag write patch carries."""
+    series = (patch or {}).get(SERIES_KEY) or {}
+    attrs = {key: series.get(key) for key in _SERIES_ATTR_KEYS}
+    if attrs[STATUS_FIELD_NAME] is not None:
+        attrs[STATUS_FIELD_NAME] = _status_value(attrs[STATUS_FIELD_NAME])
+    return {key: value for key, value in attrs.items() if value not in (None, "")}

tests/test_onlinetag_series_attrs.py

@@ -0,0 +1,231 @@
+"""
+Online tagging keeps the series attributes no written format can hold.
+
+A Metron match brings the series' end year, status and summary, and
+ComicInfo has no start year either. The tag write can't put them in the
+file, so they ride on the re-read and the importer applies them to the
+series the comic lands on. Here the match also names a new publisher and
+series, which moves the comic off its filename-derived series row: the
+values have to reach the new row, not the one the comic left.
+"""
+
+import shutil
+import threading
+import zipfile
+from pathlib import Path
+from threading import Event, Lock
+from types import SimpleNamespace
+from typing import Any, Final, override
+from unittest.mock import MagicMock
+
+from comicbox.enums.comicbox import SeriesStatusEnum
+from django.core.cache import cache
+from django.test import SimpleTestCase, TestCase
+from loguru import logger
+
+from codex.librarian.mp_queue import LIBRARIAN_QUEUE
+from codex.librarian.onlinetag.tag_pass_runner import TagPassRunner
+from codex.librarian.scribe.importer.importer import ComicImporter
+from codex.librarian.scribe.importer.tasks import ImportTask
+from codex.librarian.scribe.tag_writer import TagWriter
+from codex.librarian.scribe.tagwrite_series_attrs import patch_series_attrs
+from codex.librarian.scribe.tasks import BulkTagWriteTask
+from codex.models import Comic, Library, Series
+from codex.startup import codex_init
+from tests.importer.test_basic import run_full_import
+from tests.onlinetag_session_fakes import FakeQueue, double
+from tests.tmp_dirs import tmp_dir
+
+_TMP_DIR: Final = tmp_dir("codex.tests.onlinetag.series_attrs")
+_FILENAME_SERIES: Final = "Spidey"
+_SERIES: Final = "The Amazing Spider-Man"
+_START_YEAR: Final = 1963
+_END_YEAR: Final = 1998
+_STATUS: Final = "Completed"
+_SUMMARY: Final = "The first Spider-Man series."
+#: The tags a Metron match hands back, trimmed to what this test reads.
+_MATCH_TAGS: Final = {
+    "publisher": {"name": "Marvel"},
+    "series": {
+        "name": _SERIES,
+        "start_year": _START_YEAR,
+        "end_year": _END_YEAR,
+        "status": _STATUS,
+        "summary": _SUMMARY,
+    },
+    "issue": {"number": "1"},
+}
+
+
+class _NoopStatusController:
+    def start(self, _status, **_kwargs) -> None:
+        pass
+
+    def update(self, _status, **_kwargs) -> None:
+        pass
+
+    def finish(self, _status, **_kwargs) -> None:
+        pass
+
+
+class _MatchedSession:
+    """Matches every comic to ``_MATCH_TAGS``."""
+
+    def tag_many(self, paths):
+        for path in paths:
+            yield SimpleNamespace(
+                path=path, matched=True, tags=_MATCH_TAGS, error=None, cancelled=False
+            )
+
+
+def _untagged_cbz(path: Path) -> None:
+    """Build an archive with no metadata, so its series comes from its name."""
+    with zipfile.ZipFile(path, "w") as zf:
+        zf.writestr("page1.jpg", b"\xff\xd8\xff\xe0not-a-real-jpeg")
+
+
+def _make_writer(queue: FakeQueue) -> TagWriter:
+    """Build a TagWriter without the threading machinery."""
+    writer = TagWriter.__new__(TagWriter)
+    writer.log = double(logger)
+    writer.librarian_queue = double(queue)
+    writer.status_controller = double(MagicMock())
+    writer.abort_event = threading.Event()
+    return writer
+
+
+def _queued(queue: FakeQueue, task_type: type) -> list[Any]:
+    return [item for item in queue.items if isinstance(item, task_type)]
+
+
+class PatchSeriesAttrsTestCase(SimpleTestCase):
+    """Only non-empty attributes leave the patch, status as its choice value."""
+
+    def test_picks_the_attrs(self) -> None:
+        assert patch_series_attrs(_MATCH_TAGS) == {
+            "start_year": _START_YEAR,
+            "end_year": _END_YEAR,
+            "status": _STATUS,
+            "summary": _SUMMARY,
+        }
+
+    def test_status_enum_member_stores_its_value(self) -> None:
+        patch = {"series": {"status": SeriesStatusEnum.HIATUS}}
+        assert patch_series_attrs(patch) == {"status": "Hiatus"}
+
+    def test_drops_empty_and_unknown_values(self) -> None:
+        patch = {"series": {"end_year": None, "status": "Paused", "summary": ""}}
+        assert patch_series_attrs(patch) == {}
+
+    def test_no_series(self) -> None:
+        assert patch_series_attrs({"issue": {"number": "1"}}) == {}
+        assert patch_series_attrs(None) == {}
+
+
+class OnlineTagSeriesAttrsTestCase(TestCase):
+    """An online match's series attributes survive its write and re-read."""
+
+    @override
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+        cache.clear()
+        codex_init()
+
+    @override
+    def setUp(self) -> None:
+        cache.clear()
+        _TMP_DIR.mkdir(exist_ok=True, parents=True)
+        self.path = _TMP_DIR / f"{_FILENAME_SERIES} 001.cbz"  # pyright: ignore[reportUninitializedInstanceVariable]
+        _untagged_cbz(self.path)
+        self.library = Library.objects.create(path=str(_TMP_DIR), events=False)  # pyright: ignore[reportUninitializedInstanceVariable]
+        self._import(
+            ImportTask(self.library.pk, files_created=frozenset({str(self.path)}))
+        )
+        self.comic = Comic.objects.select_related("series").get(path=str(self.path))  # pyright: ignore[reportUninitializedInstanceVariable]
+
+    @override
+    def tearDown(self) -> None:
+        shutil.rmtree(_TMP_DIR, ignore_errors=True)
+
+    @staticmethod
+    def _import(task: ImportTask) -> None:
+        run_full_import(ComicImporter(task, logger, LIBRARIAN_QUEUE, Lock(), Event()))
+
+    def _online_tag(self) -> BulkTagWriteTask:
+        """Run a pass over the comic and return the write it flushed."""
+        queue = FakeQueue()
+        runner = TagPassRunner(
+            double(logger),
+            double(queue),
+            double(_NoopStatusController()),
+            lambda _state: None,
+            lambda _state: None,
+        )
+        state = double(
+            SimpleNamespace(
+                session=_MatchedSession(),
+                cancelled=False,
+                pending_paths=[],
+                total_comics=0,
+                completed_comics=0,
+                path_to_pk={self.path: self.comic.pk},
+                collected_tags={},
+                effort="balanced",
+                sources=("metron",),
+                merge_all_sources=False,
+                mode="update",
+                formats=("COMIC_INFO",),
+                delete_original=False,
+                rename=False,
+            )
+        )
+        runner.collect_results(state, [self.path], flush_writes=True)
+        (write_task,) = _queued(queue, BulkTagWriteTask)
+        return write_task
+
+    def _write_and_reread(self, write_task: BulkTagWriteTask) -> ImportTask:
+        """Write the tags and run the re-read the writer queued."""
+        queue = FakeQueue()
+        _make_writer(queue).write_tags(write_task)
+        (import_task,) = _queued(queue, ImportTask)
+        self._import(import_task)
+        return import_task
+
+    def test_series_attrs_land_on_the_matched_series(self) -> None:
+        assert self.comic.series.name == _FILENAME_SERIES
+        old_series_pk = self.comic.series.pk
+
+        import_task = self._write_and_reread(self._online_tag())
+
+        assert import_task.series_attrs == {
+            str(self.path): patch_series_attrs(_MATCH_TAGS)
+        }
+        self.comic.refresh_from_db()
+        series = self.comic.series
+        assert series.pk != old_series_pk
+        assert series.name == _SERIES
+        assert series.publisher.name == "Marvel"
+        # ComicInfo holds none of them, the start year included.
+        assert series.start_year == _START_YEAR
+        assert series.end_year == _END_YEAR
+        assert series.status == _STATUS
+        assert series.summary == _SUMMARY
+
+    def test_a_file_only_reimport_keeps_them(self) -> None:
+        self._write_and_reread(self._online_tag())
+
+        # The watcher's re-read of the same file carries no attributes.
+        self._import(
+            ImportTask(
+                self.library.pk,
+                files_modified=frozenset({str(self.path)}),
+                force_import_metadata=True,
+                check_metadata_mtime=False,
+            )
+        )
+
+        series = Series.objects.get(name=_SERIES)
+        assert series.end_year == _END_YEAR
+        assert series.status == _STATUS
+        assert series.summary == _SUMMARY