From 94bc7ad006a12de2d3cb702f4db0b6f02eda1b40 Mon Sep 17 00:00:00 2001 From: AJ Slater Date: Mon, 19 Oct 2026 20:47:20 -0700 Subject: [PATCH] Online tagging saves series years, status and summary A Metron match brings the series' end year, status and summary, and Comic Vine its start year and summary. ComicInfo holds none of them and MetronInfo only the start year, so the re-read after the write lost them. The tag writer now pulls those series attributes out of each comic's patch and hands them to the re-read ImportTask, and the importer lays them over the series the file names. Doing it during the import, rather than updating the comic's Series row when the write is queued, puts the values on the row the comic lands on: a match that names a new publisher or series (the usual first tag of a comic whose series came from its filename) moves the comic to a different row on re-import. Comics of one series that disagree within a batch settle by the importer's existing rules: earliest start year, latest end year, first comic's status and summary. A later write replaces a stored row's values. Empty values are dropped, so online data never clears one, and a file-only re-import leaves them standing. Co-Authored-By: Claude Opus 5.5 --- NEWS.md | 1 + .../scribe/importer/read/aggregate_path.py | 3 + codex/librarian/scribe/importer/tasks.py | 9 + codex/librarian/scribe/tag_writer.py | 21 +- .../librarian/scribe/tagwrite_series_attrs.py | 57 +++++ tests/test_onlinetag_series_attrs.py | 231 ++++++++++++++++++ 6 files changed, 318 insertions(+), 4 deletions(-) create mode 100644 codex/librarian/scribe/tagwrite_series_attrs.py create mode 100644 tests/test_onlinetag_series_attrs.py diff --git a/NEWS.md b/NEWS.md index 3cd1de184..8bcafbc67 100644 --- a/NEWS.md +++ b/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. diff --git a/codex/librarian/scribe/importer/read/aggregate_path.py b/codex/librarian/scribe/importer/read/aggregate_path.py index 031697057..4cb63c4c6 100644 --- a/codex/librarian/scribe/importer/read/aggregate_path.py +++ b/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 diff --git a/codex/librarian/scribe/importer/tasks.py b/codex/librarian/scribe/importer/tasks.py index c8e6a8bc9..b52b8a807 100644 --- a/codex/librarian/scribe/importer/tasks.py +++ b/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, diff --git a/codex/librarian/scribe/tag_writer.py b/codex/librarian/scribe/tag_writer.py index 3c66a69fe..6b99eaf72 100644 --- a/codex/librarian/scribe/tag_writer.py +++ b/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) diff --git a/codex/librarian/scribe/tagwrite_series_attrs.py b/codex/librarian/scribe/tagwrite_series_attrs.py new file mode 100644 index 000000000..fbe693f13 --- /dev/null +++ b/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, "")} diff --git a/tests/test_onlinetag_series_attrs.py b/tests/test_onlinetag_series_attrs.py new file mode 100644 index 000000000..222c6dd96 --- /dev/null +++ b/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