fix: rename extensions, watcher path prefixes and scribe queue ordering

NEWS.md

@@ -24,6 +24,17 @@ border-radius: 128px;
     - Comics keep their bookmarks and read progress through renames, conversions
       and long tag write batches. PDFs lost them every time, other formats
       occasionally.
+    - Deleting or renaming a watched folder no longer deletes comics from
+      sibling folders whose names begin the same way, like "Batman" and "Batman
+      Beyond".
+    - Renaming keeps each archive's own file extension. PDFs and unconverted
+      CBRs were renamed to .cbz names.
+    - Libraries with paths like /comics and /comics-kids no longer claim each
+      other's file changes.
+    - A second online tagging scan no longer re-queues comics the running scan
+      already has.
+    - The librarian shuts down cleanly with work queued, and tasks queued in the
+      same instant no longer collide and lose one.
 
 ## v2.2.10
 

codex/librarian/fs/watcher/dirs.py

@@ -57,23 +57,27 @@ def expand_dir_deleted(dir_path: str, library_pk: int, batch: ChangeBatch) -> No
         )
     )
 
+    # Only true children. A bare prefix match also claims every sibling whose
+    # name merely starts with this one ("/c/Batman" would delete "/c/Batman
+    # Beyond" and all its comics), so terminate the prefix with a separator.
+    child_prefix = dir_path.rstrip(os.sep) + os.sep
+
     # Child folders
     child_folder_paths = Folder.objects.filter(
-        library_id=library_pk, path__startswith=dir_path
+        library_id=library_pk, path__startswith=child_prefix
     ).values_list("path", flat=True)
     for path in child_folder_paths:
-        if path != dir_path:
-            batch.dir_deleted.append(
-                (
-                    library_pk,
-                    FSEvent(src_path=path, change=FSChange.deleted, is_directory=True),
-                )
+        batch.dir_deleted.append(
+            (
+                library_pk,
+                FSEvent(src_path=path, change=FSChange.deleted, is_directory=True),
             )
+        )
 
     # Child comics and failed imports
     for model in (Comic, FailedImport):
         child_paths = model.objects.filter(
-            library_id=library_pk, path__startswith=dir_path
+            library_id=library_pk, path__startswith=child_prefix
         ).values_list("path", flat=True)
         for path in child_paths:
             batch.deleted.append(

codex/librarian/fs/watcher/events.py

@@ -7,6 +7,7 @@
 3. Move detection -> match delete+add pairs by inode within a batch
 """
 
+import os
 from pathlib import Path
 
 from watchfiles import Change
@@ -67,7 +68,14 @@ def _process_change(
 def _find_library(library_paths: dict[str, int], file_path: str) -> int | None:
     """Find which library a changed path belongs to."""
     for lib_path, pk in library_paths.items():
-        if file_path.startswith(lib_path):
+        # Terminate the root with a separator before matching. Libraries may
+        # legitimately be siblings sharing a name prefix ("/c/comics" and
+        # "/c/comics-kids" — the admin serializer only rejects nesting), and a
+        # bare prefix match would file the second library's events under the
+        # first, importing its comics at paths outside their own library.
+        if file_path == lib_path or file_path.startswith(
+            lib_path.rstrip(os.sep) + os.sep
+        ):
             return pk
     return None
 

codex/librarian/onlinetag/session_manager.py

@@ -354,7 +354,9 @@ def _merge_task(self, state: SessionState, task: Any) -> None:
         new_paths = {}
         for comic in comics:
             path = Path(comic.path)
-            if path not in state.path_to_pk.values():
+            # ``path_to_pk`` is keyed by path; its *values* are pks, which a
+            # Path never equals, so testing them admitted every comic twice.
+            if path not in state.path_to_pk:
                 new_paths[path] = comic.pk
         if not new_paths:
             return

codex/librarian/scribe/importer/init.py

@@ -425,14 +425,16 @@ def init_apply(self) -> None:
         self.start_time = now()
         self._defer_pending_tag_write_moves()
         self.library.start_update()
-        too_long = self._wait_for_filesystem_ops_to_finish()
-        if too_long:
+        if self._wait_for_filesystem_ops_to_finish():
+            # The import runs anyway: abandoning the task would drop these
+            # events entirely on a watched library that isn't also polled.
+            # Files still mid-copy fail to import and are retried by a later
+            # scan, so say that rather than implying nothing was imported.
             reason = (
                 "Import apply waited for the filesystem to stop changing too long. "
-                "Try polling again once files have finished copying"
-                f" in library: {self.library.path}"
+                "Importing anyway; files still copying may fail and be retried"
+                f" on a later scan in library: {self.library.path}"
             )
             self.log.warning(reason)
-            return
         self._log_task()
         self._init_librarian_status(self.library.path)

codex/librarian/scribe/priority.py

@@ -1,6 +1,7 @@
 """Priority for Scribe tasks in the PriorityQueue."""
 
 from datetime import UTC, datetime
+from itertools import count
 
 from codex.librarian.scribe.importer.tasks import (
     ImportTask,
@@ -77,9 +78,18 @@
     JanitorDumpUserDataTask,
 )
 
+# Final element of every priority tuple. Tasks are pushed onto a heap as
+# ``(priority, task)``; when two priorities compare equal the heap falls
+# through to comparing the tasks themselves, and ScribeTask dataclasses
+# define no ordering — a ``TypeError`` in the routing thread that loses the
+# task. Timestamps tie more often than they look like they would (they are
+# truncated, and a clock can step backwards), so carry a strictly monotonic
+# counter that can never tie. ``next`` on an ``itertools.count`` is atomic.
+_TIE_BREAKER = count()
 
-def get_task_priority(task: ScribeTask) -> tuple[int, float]:
+
+def get_task_priority(task: ScribeTask) -> tuple[int, float, int]:
     """Get task priority by index."""
     now = datetime.now(tz=UTC).timestamp()
     priority = _SCRIBE_TASK_PRIORITY.index(type(task))
-    return priority, now
+    return priority, now, next(_TIE_BREAKER)

codex/librarian/scribe/scribed.py

@@ -48,7 +48,13 @@
 class ScribeThread(QueuedThread):
     """A worker to handle all bulk database updates."""
 
-    SHUTDOWN_MSG = (0, QueuedThread.SHUTDOWN_MSG)
+    # Shaped like a real queue item — ``(get_task_priority(...), task)`` —
+    # because a PriorityQueue orders the shutdown message against whatever
+    # is already queued. A bare int here raises TypeError comparing int to
+    # tuple, so stopping the thread with a task pending would abort the
+    # daemon's shutdown loop. The negative index sorts it ahead of every
+    # task so a stop is honored promptly.
+    SHUTDOWN_MSG = ((-1, 0.0, -1), QueuedThread.SHUTDOWN_MSG)
     # Importer / janitor / search bursts are minutes-to-hours apart on
     # a typical install. Releasing the conn between bursts saves an
     # open file handle + ~50 KiB pinned for the entire idle gap; the

codex/librarian/scribe/tag_writer.py

@@ -26,7 +26,7 @@
 from codex.librarian.scribe.tagwrite_moves import register_tag_write_move
 from codex.librarian.worker import WorkerStatusAbortableBase
 from codex.models.comic import Comic
-from codex.settings import COMICBOX_CONFIG
+from codex.settings import COMICBOX_RENAME_CONFIG
 
 if TYPE_CHECKING:
     from comicbox.events import Event
@@ -217,13 +217,28 @@ def _rename_one(self, old_path: Path) -> Path | None:
         could be built. Raises ``FileExistsError`` on a collision with a
         *different* file so the caller reports it without clobbering anything
         (comicbox's ``rename_file`` does a bare ``Path.rename``).
+
+        The rendered name ends in ``ext``, which is a *metadata* field rather
+        than the file's suffix, so it is stated here from the archive on disk
+        — the authority. Left to the merge it would be missing (the read
+        config deletes it) and comicfn2dict would fall back to its "cbz"
+        default, renaming every PDF or unconverted CBR to a name claiming to
+        be a zip; or it would be whatever a third-party tagger embedded in
+        the archive. Stating it as metadata outranks both.
         """
-        with Comicbox(old_path, config=COMICBOX_CONFIG) as car:
+        ext = old_path.suffix.lstrip(".")
+        with Comicbox(
+            old_path,
+            config=COMICBOX_RENAME_CONFIG,
+            metadata={"comicbox": {"ext": ext}} if ext else None,
+        ) as car:
             # to_string(FILENAME) is exactly what rename_file() derives the
             # name from (schema.dumps(_to_dict(FILENAME))), so this pre-check
             # targets the precise destination rename_file() will use.
             target = car.to_string(MetadataFormats.FILENAME)
-            if not target:
+            # A name that is nothing but an extension (nothing parsed at all)
+            # would make a hidden file, so treat it as no name.
+            if not target or target.startswith("."):
                 self.log.warning(f"Rename skipped; no filename built for {old_path}")
                 return None
             new_path = old_path.parent / target

codex/settings/__init__.py

@@ -1188,3 +1188,22 @@ def _get_middleware(features: FeatureFlags) -> tuple[str, ...]:
         }
     }
 )
+
+# Renaming to the comicbox filename scheme needs one field the read config
+# deletes: ``ext``, which the rendered name ends in. Deleted, comicfn2dict
+# falls back to its "cbz" default and every PDF or unconverted CBR is
+# renamed to a name claiming to be a zip. Callers must still *supply* the
+# extension (the archive's real suffix) as metadata — un-deleting the key
+# alone leaves it unset — but the delete runs after the merge, so it would
+# strip a supplied value too. Read paths keep ``COMICBOX_CONFIG``: this
+# only widens what the rename pass parses.
+COMICBOX_RENAME_CONFIG: ComicboxSettings = get_config(
+    {
+        "comicbox": {
+            "general": {
+                "loglevel": LOGLEVEL,
+                "delete_keys": tuple(sorted(_COMICBOX_DELETE_KEYS - {"ext"})),
+            }
+        }
+    }
+)

codex/views/admin/tagwrite.py

@@ -18,7 +18,7 @@
 from codex.models.admin import ComicboxTaggingDefaults
 from codex.models.comic import Comic
 from codex.serializers.admin.tagging import TagWriteRequestSerializer
-from codex.settings import COMICBOX_CONFIG
+from codex.settings import COMICBOX_RENAME_CONFIG
 from codex.views.admin.auth import AdminAPIView
 from codex.views.admin.identifier_parse import parse_identifier_url
 from codex.views.browser.filters.filter import BrowserFilterView
@@ -104,20 +104,30 @@ class AdminTagWritePreflightView(FilteredComicPksView):
     """Check how many comics need conversion before writing."""
 
     @staticmethod
-    def _preview_one(old_path: Path, metadata: dict | None, config) -> str:
+    def _preview_one(old_path: Path, patch: dict | None, config) -> str:
         """
         Return the comicbox-scheme name the given patch produces for one comic.
 
         Overlays the pending (unsaved) patch onto the archive's metadata in
         memory and serializes the FILENAME format — the same construction
         ``rename_file`` uses — so the dialog can show the would-be name. Opens
         the archive (I/O). Returns "" when no name could be built.
+
+        States ``ext`` from the file's real suffix exactly as
+        ``TagWriter._rename_one`` does, so the preview cannot promise a name
+        the rename won't produce.
         """
+        metadata = dict(patch) if patch else {}
+        if ext := old_path.suffix.lstrip("."):
+            metadata["ext"] = ext
         try:
-            with Comicbox(old_path, config=config, metadata=metadata) as car:
-                return car.to_string(MetadataFormats.FILENAME) or ""
+            with Comicbox(
+                old_path, config=config, metadata={"comicbox": metadata}
+            ) as car:
+                target = car.to_string(MetadataFormats.FILENAME) or ""
         except Exception:
             return ""
+        return "" if target.startswith(".") else target
 
     def _filename_previews(
         self,
@@ -127,10 +137,9 @@ def _filename_previews(
     ) -> list[dict[str, str]]:
         """Preview the rename (old → new) for each selected comic, capped."""
         patch = json.loads(patch_str or "null")
-        metadata = {"comicbox": patch} if patch else None
         # Pending cleared fields must vanish from the previewed name exactly
         # as the real write (BulkWriteItem.delete_keys) will clear them.
-        config = COMICBOX_CONFIG
+        config = COMICBOX_RENAME_CONFIG
         if delete_keys:
             config = replace(
                 config,
@@ -150,7 +159,7 @@ def _filename_previews(
             previews.append(
                 {
                     "old": old_path.name,
-                    "new": self._preview_one(old_path, metadata, config),
+                    "new": self._preview_one(old_path, patch, config),
                 }
             )
         return previews

tests/test_onlinetag_merge_task.py

@@ -0,0 +1,82 @@
+"""
+Merging a second scan into a running online-tag session.
+
+A second ``BulkOnlineTagTask`` enqueued mid-scan is merged into the live
+session. Comics the session already holds must be recognized, or a scan
+whose selection overlaps (re-picking a folder to catch additions is the
+natural gesture) queues them twice: inflated totals and duplicate lookups
+against rate-limited sources.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+from typing import Final
+
+from codex.librarian.onlinetag.session_state import SessionState
+from codex.librarian.onlinetag.tasks import BulkOnlineTagTask
+from codex.models import Comic
+from tests.onlinetag_session_fakes import (
+    FakeSession,
+    OnlineTagSessionTestCase,
+    double,
+    make_comic,
+)
+
+#: The comic the session already holds, plus the one merged into it.
+_HELD_PLUS_NEW: Final = 2
+
+
+class MergeTaskTests(OnlineTagSessionTestCase):
+    """_merge_task adds only comics the session isn't already tagging."""
+
+    def _state_holding(self, comic) -> SessionState:
+        """Build a session state that already carries ``comic``."""
+        return SessionState(
+            session=double(FakeSession()),
+            path_to_pk={Path(comic.path): comic.pk},
+            total_comics=1,
+        )
+
+    def test_already_queued_comic_is_not_merged_again(self) -> None:
+        """An overlapping selection must not re-queue a comic."""
+        comic = make_comic()
+        state = self._state_holding(comic)
+        task = BulkOnlineTagTask(comic_pks=frozenset({comic.pk}), session_id="s")
+
+        self.manager._merge_task(state, task)  # noqa: SLF001
+
+        assert state.total_comics == 1
+        assert state.pending_paths == []
+        assert state.path_to_pk == {Path(comic.path): comic.pk}
+
+    @staticmethod
+    def _sibling_of(comic: Comic) -> Comic:
+        """Create a second comic in the same library as ``comic``."""
+        path = Path(comic.path).parent / "d.cbz"
+        path.touch()
+        return Comic.objects.create(
+            library=comic.library,
+            path=path,
+            issue_number=2,
+            name="d",
+            publisher=comic.publisher,
+            imprint=comic.imprint,
+            series=comic.series,
+            volume=comic.volume,
+            size=1,
+            file_type="CBZ",
+        )
+
+    def test_new_comic_is_merged(self) -> None:
+        """A comic the session doesn't hold is queued and counted."""
+        held = make_comic()
+        state = self._state_holding(held)
+        new_comic = self._sibling_of(held)
+        task = BulkOnlineTagTask(comic_pks=frozenset({new_comic.pk}), session_id="s")
+
+        self.manager._merge_task(state, task)  # noqa: SLF001
+
+        assert state.total_comics == _HELD_PLUS_NEW
+        assert state.pending_paths == [Path(new_comic.path)]
+        assert state.path_to_pk[Path(new_comic.path)] == new_comic.pk