From 7a788aa0b45dab7c9ff74fb6b1fdd8d1bf8fdce8 Mon Sep 17 00:00:00 2001 From: Brad Butler Date: Fri, 2 Oct 2026 09:40:15 +1000 Subject: [PATCH 1/2] fix: keep library records in sync after file conversion The standalone convert utility converts a tracked file, moves the original to trash and leaves the LibraryFile row on the old path. The record then references a file that no longer exists, the converted file is untracked, and later utilities cannot find metadata for it (Mass Convert reports "No metadata available from unknown"). Update the record after a completed conversion, as the mass convert pipeline already does, and point it back on rollback. The shared sync helper now takes the format from the target path so a rollback to a CBR is not recorded as CBZ. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01E9gUFQEryh4gDJRKvjNUZM --- .../services/library_convert_service.py | 7 +- .../utilities/executors/file_converter.py | 86 ++++++++++- tests/utilities/test_file_converter.py | 136 ++++++++++++++++++ 3 files changed, 227 insertions(+), 2 deletions(-) diff --git a/src/pullbox/services/library_convert_service.py b/src/pullbox/services/library_convert_service.py index 893ecf80..44e02554 100644 --- a/src/pullbox/services/library_convert_service.py +++ b/src/pullbox/services/library_convert_service.py @@ -59,7 +59,12 @@ async def _sync_converted_file_record( updated_path = Path(after_path) library_file.file_path = after_path library_file.file_name = updated_path.name - library_file.file_format = FileFormat.CBZ + # Conversions always produce CBZ, but a rollback points the record back at + # the original file, which keeps its own format. + try: + library_file.file_format = FileFormat(updated_path.suffix.lower().lstrip(".")) + except ValueError: + library_file.file_format = FileFormat.CBZ library_file.file_hash = None if metadata_embedded: library_file.has_comicinfo = True diff --git a/src/pullbox/utilities/executors/file_converter.py b/src/pullbox/utilities/executors/file_converter.py index be60023a..27e77db2 100644 --- a/src/pullbox/utilities/executors/file_converter.py +++ b/src/pullbox/utilities/executors/file_converter.py @@ -29,7 +29,15 @@ from pullbox.core.library_root_resolution import resolve_path_inside_roots from pullbox.core.rar_backend import RarBackendUnavailableError, configure_rarfile_backend from pullbox.models.library import LibraryFile, LibraryFileStorageMode -from pullbox.utilities.base_executor import ExecutionMode, ItemResult, JobExecutor, ProcessedItem +from pullbox.utilities.base_executor import ( + ApplyResult, + ExecutionMode, + ItemResult, + JobExecutor, + JobRunSummary, + ProcessedItem, + RuntimeLogEntry, +) from pullbox.utilities.settings import ( move_file_to_utility_trash, resolve_trash_directory, @@ -592,6 +600,35 @@ def build_convert_preview( ) +async def _sync_library_record(*, before_path: str, after_path: str, session: Any) -> ApplyResult: + """Move a tracked library record from one path to another, if one exists.""" + if not before_path or not after_path or before_path == after_path: + return ApplyResult() + + # Imported here: library_convert_service imports convert_file from this module. + from pullbox.services.library_convert_service import _sync_converted_file_record + + tracked = ( + await session.execute(select(LibraryFile.id).where(LibraryFile.file_path == before_path)) + ).scalar_one_or_none() + if tracked is None: + return ApplyResult() + + await _sync_converted_file_record(session, before_path=before_path, after_path=after_path) + return ApplyResult( + extra_logs=[ + RuntimeLogEntry( + level="INFO", + message=( + f"Updated library record: {Path(before_path).name} -> {Path(after_path).name}" + ), + file_path=after_path, + extra={"previous_path": before_path, "updated_path": after_path}, + ) + ] + ) + + # ── FileConverterExecutor ────────────────────────────────────── @@ -808,6 +845,53 @@ def process_item( ], ) + async def apply_item_result( + self, + session: Any, + item: Any, + item_data: dict[str, Any], + processed: ProcessedItem, + job_config: dict[str, Any], + job_context: dict[str, Any] | None, + summary: JobRunSummary, + ) -> ApplyResult: + """Point the tracked library record at the converted file. + + The original is moved to trash, so a record left on the old path would + reference a file that no longer exists and the converted file would be + untracked. + """ + if processed.result != ItemResult.COMPLETED or not processed.after_state: + return ApplyResult() + + before_path = str(item_data.get("file_path", "") or "") + after_path = str(processed.after_state.get("path", "") or "") + return await _sync_library_record( + before_path=before_path, after_path=after_path, session=session + ) + + @staticmethod + async def apply_rollback_result( + session: Any, + item_data: dict[str, Any], + processed: ProcessedItem, + ) -> ApplyResult: + """Point the tracked library record back at the restored original.""" + if processed.result != ItemResult.COMPLETED: + return ApplyResult() + + before_state = item_data.get("before_state", {}) + if isinstance(before_state, str): + before_state = json.loads(before_state or "{}") + after_state = item_data.get("after_state", {}) + if isinstance(after_state, str): + after_state = json.loads(after_state or "{}") + return await _sync_library_record( + before_path=str(after_state.get("path", "") or ""), + after_path=str(before_state.get("path", "") or ""), + session=session, + ) + def rollback_item( self, item_data: dict[str, Any], diff --git a/tests/utilities/test_file_converter.py b/tests/utilities/test_file_converter.py index f3effc81..9a812530 100644 --- a/tests/utilities/test_file_converter.py +++ b/tests/utilities/test_file_converter.py @@ -883,3 +883,139 @@ def test_process_item_passes_pdf_quality(self, tmp_path: Path) -> None: {"target_format": "cbz", "source_format": "pdf", "pdf_quality": "high"} ) assert errors == [] + + +# ── Library Record Sync ──────────────────────────────────────── + + +async def _track_library_file(db_session, file_path: Path) -> int: + from datetime import UTC, datetime + + from pullbox.models.issue import Issue + from pullbox.models.library import FileFormat, LibraryFile, LibraryRoot, MatchConfidence + from pullbox.models.series import Series + + library_root = LibraryRoot(name="Library", path=str(file_path.parent.parent), enabled=True) + series = Series(title="Batman", sort_title="batman", year_start=2016, library_root=library_root) + issue = Issue(series=series, issue_number=1.0) + library_file = LibraryFile( + file_path=str(file_path), + file_name=file_path.name, + file_size=file_path.stat().st_size, + file_format=FileFormat(file_path.suffix.lstrip(".")), + file_modified_at=datetime.now(UTC), + match_confidence=MatchConfidence.HIGH, + issue=issue, + library_root=library_root, + ) + db_session.add(library_file) + await db_session.flush() + return int(library_file.id) + + +def _processed_conversion(source: Path, target: Path): + from pullbox.utilities.base_executor import ProcessedItem + + return ProcessedItem( + item_id="item-1", + result=ItemResult.COMPLETED, + before_state={"path": str(source), "format": "cbr"}, + after_state={"path": str(target), "format": "cbz", "original_path": "/trash/x.cbr"}, + ) + + +class TestLibraryRecordSync: + """A converted file must stay tracked under its new name.""" + + @pytest.mark.asyncio + async def test_completed_conversion_moves_the_library_record( + self, db_session, tmp_path: Path + ) -> None: + from pullbox.models.library import FileFormat, LibraryFile + from pullbox.utilities.base_executor import JobRunSummary + + source = tmp_path / "Batman (2016)" / "Batman 001.cbr" + _create_test_cbz(source) + library_file_id = await _track_library_file(db_session, source) + target = _create_test_cbz(source.with_suffix(".cbz"), page_count=5) + source.unlink() + + applied = await FileConverterExecutor().apply_item_result( + db_session, + item=None, + item_data={"id": "item-1", "file_path": str(source)}, + processed=_processed_conversion(source, target), + job_config={"target_format": "cbz"}, + job_context=None, + summary=JobRunSummary(), + ) + + record = await db_session.get(LibraryFile, library_file_id) + assert record.file_path == str(target) + assert record.file_name == "Batman 001.cbz" + assert record.file_format == FileFormat.CBZ + assert record.file_size == target.stat().st_size + assert [entry.message for entry in applied.extra_logs] == [ + "Updated library record: Batman 001.cbr -> Batman 001.cbz" + ] + + @pytest.mark.asyncio + async def test_untracked_or_failed_conversions_change_nothing( + self, db_session, tmp_path: Path + ) -> None: + from pullbox.utilities.base_executor import JobRunSummary, ProcessedItem + + source = tmp_path / "loose" / "Saga 001.cbr" + target = _create_test_cbz(source.with_suffix(".cbz")) + executor = FileConverterExecutor() + + untracked = await executor.apply_item_result( + db_session, + item=None, + item_data={"id": "item-1", "file_path": str(source)}, + processed=_processed_conversion(source, target), + job_config={"target_format": "cbz"}, + job_context=None, + summary=JobRunSummary(), + ) + failed = await executor.apply_item_result( + db_session, + item=None, + item_data={"id": "item-2", "file_path": str(source)}, + processed=ProcessedItem(item_id="item-2", result=ItemResult.FAILED), + job_config={"target_format": "cbz"}, + job_context=None, + summary=JobRunSummary(), + ) + + assert untracked.extra_logs == [] + assert failed.extra_logs == [] + + @pytest.mark.asyncio + async def test_rollback_points_the_record_back_at_the_original( + self, db_session, tmp_path: Path + ) -> None: + from pullbox.models.library import FileFormat, LibraryFile + from pullbox.utilities.base_executor import ProcessedItem + + converted = tmp_path / "Batman (2016)" / "Batman 001.cbz" + _create_test_cbz(converted) + library_file_id = await _track_library_file(db_session, converted) + original = converted.with_suffix(".cbr") + _create_test_cbz(original, page_count=4) + converted.unlink() + + await FileConverterExecutor.apply_rollback_result( + db_session, + { + "id": "item-1", + "before_state": {"path": str(original), "format": "cbr"}, + "after_state": {"path": str(converted), "format": "cbz"}, + }, + ProcessedItem(item_id="item-1", result=ItemResult.COMPLETED), + ) + + record = await db_session.get(LibraryFile, library_file_id) + assert record.file_path == str(original) + assert record.file_name == "Batman 001.cbr" + assert record.file_format == FileFormat.CBR From 791b7af432a1f9155643ea07bebe855ed48a26c2 Mon Sep 17 00:00:00 2001 From: Brad Butler Date: Sat, 3 Oct 2026 12:13:42 +1000 Subject: [PATCH 2/2] fix: refresh the library record after a same-path repack A CBZ -> CBZ repack keeps the file name but rewrites the archive, and the sync skipped it because the path had not changed, leaving the old size, modified time and cached hash on the record. The record is now refreshed whenever a conversion or its rollback completes, whether or not the path moved; the shared helper already clears the cached hash. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01E9gUFQEryh4gDJRKvjNUZM --- .../utilities/executors/file_converter.py | 16 +++-- tests/utilities/test_file_converter.py | 68 +++++++++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/src/pullbox/utilities/executors/file_converter.py b/src/pullbox/utilities/executors/file_converter.py index 27e77db2..d03bfc98 100644 --- a/src/pullbox/utilities/executors/file_converter.py +++ b/src/pullbox/utilities/executors/file_converter.py @@ -601,8 +601,12 @@ def build_convert_preview( async def _sync_library_record(*, before_path: str, after_path: str, session: Any) -> ApplyResult: - """Move a tracked library record from one path to another, if one exists.""" - if not before_path or not after_path or before_path == after_path: + """Point a tracked library record at the converted file and refresh its attributes. + + A same-path repack (CBZ -> CBZ) keeps the name but changes the contents, so the + record is still refreshed: size, modified time and the cached hash. + """ + if not before_path or not after_path: return ApplyResult() # Imported here: library_convert_service imports convert_file from this module. @@ -615,13 +619,15 @@ async def _sync_library_record(*, before_path: str, after_path: str, session: An return ApplyResult() await _sync_converted_file_record(session, before_path=before_path, after_path=after_path) + if before_path == after_path: + message = f"Refreshed library record: {Path(after_path).name}" + else: + message = f"Updated library record: {Path(before_path).name} -> {Path(after_path).name}" return ApplyResult( extra_logs=[ RuntimeLogEntry( level="INFO", - message=( - f"Updated library record: {Path(before_path).name} -> {Path(after_path).name}" - ), + message=message, file_path=after_path, extra={"previous_path": before_path, "updated_path": after_path}, ) diff --git a/tests/utilities/test_file_converter.py b/tests/utilities/test_file_converter.py index 9a812530..c0e7c8cb 100644 --- a/tests/utilities/test_file_converter.py +++ b/tests/utilities/test_file_converter.py @@ -959,6 +959,74 @@ async def test_completed_conversion_moves_the_library_record( "Updated library record: Batman 001.cbr -> Batman 001.cbz" ] + @pytest.mark.asyncio + async def test_same_path_repack_refreshes_the_library_record( + self, db_session, tmp_path: Path + ) -> None: + from pullbox.models.library import FileFormat, LibraryFile + from pullbox.utilities.base_executor import JobRunSummary + + archive = tmp_path / "Batman (2016)" / "Batman 001.cbz" + _create_test_cbz(archive, page_count=2) + library_file_id = await _track_library_file(db_session, archive) + record = await db_session.get(LibraryFile, library_file_id) + record.file_hash = "hash-of-the-old-archive" + old_size = record.file_size + # A CBZ -> CBZ repack rewrites the archive in place. + _create_test_cbz(archive, page_count=9) + assert archive.stat().st_size != old_size + + applied = await FileConverterExecutor().apply_item_result( + db_session, + item=None, + item_data={"id": "item-1", "file_path": str(archive)}, + processed=_processed_conversion(archive, archive), + job_config={"target_format": "cbz"}, + job_context=None, + summary=JobRunSummary(), + ) + + record = await db_session.get(LibraryFile, library_file_id) + assert record.file_path == str(archive) + assert record.file_format == FileFormat.CBZ + assert record.file_size == archive.stat().st_size + assert record.file_hash is None + assert [entry.message for entry in applied.extra_logs] == [ + "Refreshed library record: Batman 001.cbz" + ] + + @pytest.mark.asyncio + async def test_same_path_repack_rollback_refreshes_the_library_record( + self, db_session, tmp_path: Path + ) -> None: + from pullbox.models.library import LibraryFile + from pullbox.utilities.base_executor import ProcessedItem + + archive = tmp_path / "Batman (2016)" / "Batman 001.cbz" + _create_test_cbz(archive, page_count=9) + library_file_id = await _track_library_file(db_session, archive) + record = await db_session.get(LibraryFile, library_file_id) + record.file_hash = "hash-of-the-repacked-archive" + repacked_size = record.file_size + # Rollback restores the original archive to the same path. + _create_test_cbz(archive, page_count=2) + assert archive.stat().st_size != repacked_size + + await FileConverterExecutor.apply_rollback_result( + db_session, + { + "id": "item-1", + "before_state": {"path": str(archive), "format": "cbz"}, + "after_state": {"path": str(archive), "format": "cbz"}, + }, + ProcessedItem(item_id="item-1", result=ItemResult.COMPLETED), + ) + + record = await db_session.get(LibraryFile, library_file_id) + assert record.file_path == str(archive) + assert record.file_size == archive.stat().st_size + assert record.file_hash is None + @pytest.mark.asyncio async def test_untracked_or_failed_conversions_change_nothing( self, db_session, tmp_path: Path