fix: stop browsing from re-importing tagless comics and reloading the page (#992)

NEWS.md

@@ -16,6 +16,8 @@ border-radius: 128px;
     - Online tag match picker shows series and issue details to help
       disambiguate similar covers and titles.
 - Fixes
+    - Browsing comics without embedded tags, or re-importing unchanged comics,
+      no longer rebuilds their covers and reloads the page.
     - Searches for a universe designation no longer miss imported comics.
 
 ## v2.5.5

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

@@ -12,9 +12,11 @@
     BULK_UPDATE_COMIC_FIELDS_SET,
     COLLECTION_FIELD_NAMES,
     CREATE_COMICS,
+    DELETE_M2MS,
     FTS_CREATE,
     FTS_UPDATE,
     LINK_FKS,
+    LINK_M2MS,
     NON_FTS_FIELDS,
     PATH_FIELD_NAME,
     UPDATE_COMICS,
@@ -33,6 +35,10 @@
 if TYPE_CHECKING:
     from codex.models.collections import BrowserCollectionModel
 
+# What ``presave`` derives from the file and the row. An update that would
+# change none of them, the timestamp aside, writes nothing new.
+_NOOP_CHECK_FIELDS = tuple(sorted(ALWAYS_UPDATE_COMIC_FIELDS - {"updated_at"}))
+
 
 class CreateComicsImporter(CreateForeignKeyLinksImporter):
     """Create comics methods."""
@@ -66,10 +72,55 @@ def _record_moved_source_collections(
             )
             self.moved_source_collections.setdefault(model, set()).add(old_pk)
 
+    def _is_noop_update(
+        self,
+        comic: Comic,
+        md: dict,
+        link_md: dict,
+        m2m_deleted_pks: frozenset[int],
+    ) -> bool:
+        """
+        Return whether updating this comic would write nothing new.
+
+        A forced or lazy import of an unchanged comic, typically one with no
+        embedded tags, used to rewrite the row anyway: that purged and rebuilt
+        its cover and broadcast LIBRARY_CHANGED, reloading every browser. The
+        query phase has already pruned every link that is already correct, so
+        no scalar, FK or m2m change remains. If ``presave`` then derives the
+        same stat and the same values from the file on disk, the row is
+        current. A chunk that changed tag or collection rows keeps every comic
+        (see ``chunk_fks_updated``).
+        """
+        if (
+            self.chunk_fks_updated
+            or md
+            or link_md
+            or comic.path in self.task.series_attrs
+        ):
+            return False
+        if (
+            comic.path in self.metadata.get(LINK_M2MS, {})
+            or comic.pk in m2m_deleted_pks
+        ):
+            return False
+        before = tuple(getattr(comic, field_name) for field_name in _NOOP_CHECK_FIELDS)
+        comic.presave()
+        return before == tuple(
+            getattr(comic, field_name) for field_name in _NOOP_CHECK_FIELDS
+        )
+
     def _update_comic_values(
-        self, comic: Comic, update_comics: list, comic_pks: list, changed_fields: set
+        self,
+        comic: Comic,
+        update_comics: list,
+        comic_pks: list,
+        changed_fields: set,
+        m2m_deleted_pks: frozenset[int],
     ) -> None:
         md = self.metadata[UPDATE_COMICS].pop(comic.pk, {})
+        link_md = self.get_comic_fk_links(comic.pk, comic.path, for_create=False)
+        if self._is_noop_update(comic, md, link_md, m2m_deleted_pks):
+            return
         for field_name, value in md.items():
             setattr(comic, field_name, value)
         changed_fields.update(md)
@@ -82,7 +133,6 @@ def _update_comic_values(
             field_name: getattr(comic, f"{field_name}_id")
             for field_name in COLLECTION_FIELD_NAMES
         }
-        link_md = self.get_comic_fk_links(comic.pk, comic.path, for_create=False)
         changed_fields.update(link_md)
         for field_name, value in link_md.items():
             set_value = value
@@ -125,12 +175,17 @@ def update_comics(self) -> int:
             update_comics = []
             comic_pks = []
             changed_fields: set[str] = set()
+            m2m_deleted_pks = frozenset(
+                comic_pk
+                for rows in self.metadata.get(DELETE_M2MS, {}).values()
+                for comic_pk, _ in rows
+            )
             for comic in comics:
                 if self.abort_event.is_set():
                     return count
                 try:
                     self._update_comic_values(
-                        comic, update_comics, comic_pks, changed_fields
+                        comic, update_comics, comic_pks, changed_fields, m2m_deleted_pks
                     )
                 except Exception:
                     self.log.exception(f"Error preparing {comic} for update.")

codex/librarian/scribe/importer/create/foreign_keys.py

@@ -309,6 +309,7 @@ def update_all_fks(self) -> int:
             )
             count += self.bulk_update_all_models(update_status)
         finally:
+            self.chunk_fks_updated = bool(count)
             self.metadata.pop(UPDATE_FKS, None)
             self.status_controller.finish(update_status)
         return count

codex/librarian/scribe/importer/finish.py

@@ -1,5 +1,6 @@
 """The main importer class."""
 
+from itertools import batched
 from operator import itemgetter
 from time import perf_counter
 from types import MappingProxyType
@@ -8,6 +9,7 @@
 from django.db.models.functions import Now
 from humanize import intcomma, naturaldelta
 
+from codex.choices.admin import AdminFlagChoices
 from codex.librarian.notifier.tasks import (
     FAILED_IMPORTS_CHANGED_TASK,
     LIBRARY_CHANGED_TASK,
@@ -18,7 +20,9 @@
 from codex.librarian.scribe.importer.statii import IMPORTER_STATII
 from codex.librarian.scribe.search.status import SEARCH_INDEX_STATII
 from codex.librarian.scribe.status import SCRIBE_STATII
+from codex.models.admin import AdminFlag
 from codex.models.comic import Comic
+from codex.settings import IMPORTER_LINK_FK_BATCH_SIZE
 
 _REPORT_MAP = MappingProxyType(
     {
@@ -91,24 +95,34 @@ def _log_finish(self) -> None:
         self.log.success(log_txt)
         self._log_phase_times()
 
+    def is_import_metadata(self) -> bool:
+        """Return whether this pass reads tags, forced or by admin flag."""
+        if self.task.force_import_metadata:
+            return True
+        key = AdminFlagChoices.IMPORT_METADATA.value
+        return AdminFlag.objects.only("on").get(key=key).on
+
     def _stamp_metadata_imported(self) -> None:
         """
-        Mark every comic this forced import pass touched as import-complete.
+        Mark every comic this tag-reading pass touched as import-complete.
 
         Path-scoped so it covers SKIPPED no-metadata comics that never reach
-        the per-comic write path. Idempotent (``__isnull=True``) and gated on
-        ``force_import_metadata`` so ordinary watcher-driven imports never
-        rewrite the column.
+        the per-comic write path. Idempotent (``__isnull=True``). A scan with
+        the import-tags flag off reads no tags, so it leaves the column NULL
+        for the lazy importer to pick up.
         """
-        if not self.task.force_import_metadata:
-            return
         if not (paths := self.metadata_import_paths):
             return
-        Comic.objects.filter(
-            library=self.library,
-            path__in=paths,
-            metadata_imported_at__isnull=True,
-        ).update(metadata_imported_at=Now())
+        if not self.is_import_metadata():
+            return
+        # A first scan reaches here with every path in the library, so
+        # batch path__in to stay under SQLite's variable limit.
+        for batch_paths in batched(paths, IMPORTER_LINK_FK_BATCH_SIZE):
+            Comic.objects.filter(
+                library=self.library,
+                path__in=batch_paths,
+                metadata_imported_at__isnull=True,
+            ).update(metadata_imported_at=Now())
 
     def finish(self) -> None:
         """Perform final tasks when the apply is done."""

codex/librarian/scribe/importer/init.py

@@ -156,6 +156,11 @@ def __init__(
         self.moved_source_collections: dict[
             type[BrowserCollectionModel | Reprint], set[int]
         ] = {}
+        # Whether this chunk's ``update_all_fks`` changed any rows. Those
+        # rows reach the browser's refresh gate and the search index only
+        # through the comics this import rewrites, so ``update_comics`` keeps
+        # every comic of such a chunk, changed or not.
+        self.chunk_fks_updated = False
         # Full set of paths this import touched, captured before the chunking
         # loop and extract phase zero out ``task.files_*``. Consumed at finish
         # to stamp ``Comic.metadata_imported_at`` on every comic a forced/lazy

codex/librarian/scribe/importer/query/update_comics.py

@@ -33,7 +33,8 @@ def _query_update_comic(self, comic: Comic, status: Status) -> None:
             for key, value in proposed_comic_dict.items()
             if getattr(comic, key) != value
         }
-        # Update even if update_comic_dict is empty to set stat
+        # Keep it even if update_comic_dict is empty: the update phase still
+        # writes a changed stat, and skips the comic only if nothing changed.
         self.metadata[UPDATE_COMICS][comic.pk] = update_comic_dict
         status.increment_complete()
         self.status_controller.update(status)

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

@@ -9,7 +9,6 @@
 from comicbox.formats.comicbox.schema import PAGE_COUNT_KEY
 from comicbox.process import ReadResult, iter_process_files
 
-from codex.choices.admin import AdminFlagChoices
 from codex.librarian.scribe.importer.const import (
     EXTRACTED,
     EXTRACTED_STAT_ONLY,
@@ -20,7 +19,6 @@
     AggregateMetadataImporter,
 )
 from codex.librarian.scribe.importer.statii.read import ImporterReadComicsStatus
-from codex.models.admin import AdminFlag
 from codex.models.comic import Comic
 from codex.settings import COMICBOX_CONFIG, LOGLEVEL
 from codex.startup.loguru import CODEX_LOG_FORMAT
@@ -102,11 +100,7 @@ def _filesystem_mtime_prefilter(
 
     def _get_import_metadata_flag(self) -> bool:
         """Set import_metadata flag."""
-        if self.task.force_import_metadata:
-            import_metadata = True
-        else:
-            key = AdminFlagChoices.IMPORT_METADATA.value
-            import_metadata = AdminFlag.objects.only("on").get(key=key).on
+        import_metadata = self.is_import_metadata()
         if not import_metadata:
             self.log.warning("Admin flag set to NOT import tags.")
         return import_metadata

codex/librarian/scribe/importer/search/update.py

@@ -10,6 +10,7 @@
     FTS_CREATE,
     FTS_EXISTING_M2MS,
     FTS_UPDATE,
+    FTS_UPDATED_M2MS,
     UNIVERSES_FIELD_NAME,
 )
 from codex.librarian.scribe.importer.search.sync_m2m import (
@@ -270,7 +271,10 @@ def import_search_index(self, cleaned_count: int) -> None:
             if self.abort_event.is_set():
                 self.log.info("Search Index update aborted early.")
             self.abort_event.clear()
-            self.metadata.pop(FTS_EXISTING_M2MS, None)
+            # An operate with no entries returns before it consumes its
+            # inputs; an import that rewrote no comics leaves them empty.
+            for key in (FTS_CREATE, FTS_UPDATE, FTS_UPDATED_M2MS, FTS_EXISTING_M2MS):
+                self.metadata.pop(key, None)
             self.status_controller.finish_many(
                 (ImporterFTSCreateStatus, ImporterFTSUpdateStatus)
             )

codex/migrations/0060_backfill_metadata_imported_at.py

@@ -0,0 +1,44 @@
+"""
+Mark comics scanned with tag import on as import-complete.
+
+Before this release, only a forced or lazy import stamped
+``Comic.metadata_imported_at``. A library scan with the import-tags flag on
+read every comic but stamped none, so a comic with no embedded metadata file
+read as un-imported forever. Hovering its tag button re-imported it, purged
+and rebuilt its cover and reloaded the browser. 0043 backfilled the rows that
+existed then; this backfills the ones scanned since, with the same seed.
+
+Only when the import-tags flag is on: a scan with it off read no tags, and
+its NULL rows are what the lazy importer is for. A missing flag row counts
+as on, its default.
+"""
+
+from django.db import migrations
+from django.db.models.functions import Coalesce
+
+# AdminFlagChoices.IMPORT_METADATA, frozen at this migration's point in time.
+_IMPORT_METADATA_KEY = "IM"
+
+
+def _backfill_metadata_imported_at(apps, _schema_editor) -> None:
+    """Stamp un-stamped comics if library scans import tags."""
+    admin_flag_model = apps.get_model("codex", "adminflag")
+    flag = admin_flag_model.objects.filter(key=_IMPORT_METADATA_KEY).first()
+    if flag is not None and not flag.on:
+        return
+    comic_model = apps.get_model("codex", "comic")
+    comic_model.objects.filter(metadata_imported_at__isnull=True).update(
+        metadata_imported_at=Coalesce("metadata_mtime", "updated_at")
+    )
+
+
+class Migration(migrations.Migration):
+    """Backfill Comic.metadata_imported_at for comics scanned since 0043."""
+
+    dependencies = [
+        ("codex", "0059_series_filters_and_fts_summary"),
+    ]
+
+    operations = [
+        migrations.RunPython(_backfill_metadata_imported_at, migrations.RunPython.noop),
+    ]