Skip invalid recipe images

docs/exif_mapping.md

@@ -102,9 +102,14 @@ if color is a numeric label (e.g. "0 (normal)", "+2 (high)"):
     → saturation adjustment; decode using table below
 
 elif color is a non-numeric string (e.g. "Acros", "None (B&W)", "Film Simulation"):
-    → film simulation name or special case; recipe returns "N/A" for the Color field
+    → film simulation name or special case; recipe returns None for the Color field
 ```
 
+Note that Color returns `None`, not the string `"N/A"` that Sharpness uses for its
+equivalent case. For a monochromatic simulation that absence is correct and the
+recipe is stored. For a colour simulation it is not: a colour recipe with no Color
+value fails `validate_recipe_data`, and the image is skipped at ingest.
+
 ### Numeric saturation mapping table
 
 | Camera display value | EXIF stored value | Recipe output |
@@ -136,8 +141,9 @@ elif color is a non-numeric string (e.g. "Acros", "None (B&W)", "Film Simulation
 
 ### Key observations
 
-- `Film Simulation` appears for some film simulations (e.g. Eterna, Astia, Pro Neg. Std); it means the film profile controls saturation internally and the user cannot override it.
-- For B&W/Acros/Sepia simulations the `Film Mode` EXIF field is absent; `Saturation` encodes the simulation name instead.  In those cases the Color recipe field is `"N/A"` — there is no separate saturation adjustment.
+- `Film Simulation` means the camera, not the user, drove saturation for that shot.  It is not a property of the film simulation: it has been observed on Eterna, Astia and Pro Neg. Std images, but the same simulations carry numeric values on the great majority of shots.  Nothing else in the EXIF flags the condition, so this value is the only signal.
+- A colour simulation whose `Saturation` is `Film Simulation` records no recipe of the user's, so the image is skipped at ingest rather than stored with an empty Color.  See [management_commands.md](management_commands.md) for how skips are reported.
+- For B&W/Acros/Sepia simulations the `Film Mode` EXIF field is absent; `Saturation` encodes the simulation name instead.  In those cases the Color recipe field is `None` — there is no separate saturation adjustment, and the recipe is stored normally.
 - Recipe output for numeric values is a signed integer string: `"-2"`, `"+3"`, `"0"`.
 
 ---

docs/management_commands.md

@@ -63,6 +63,13 @@ Behaviour depends on your install mode:
 - **Full install** (`USE_ASYNC_TASKS=True`): one Celery task is enqueued per image and
   processed in parallel by the worker (start it first with `make worker`).
 
+Files that cannot produce a recipe are skipped, never aborting the run. That covers
+images carrying no Fujifilm metadata, and images whose EXIF fails recipe validation,
+such as a colour simulation whose Color the camera set rather than the user. In lite
+mode the command reports how many were skipped; add `--verbosity 2` to list their
+paths. In full install mode each skip is recorded as an `image.import.skipped` event
+in the worker log, carrying the reason and the offending recipe field.
+
 ---
 
 ## Rating images in bulk

src/application/usecases/images/process_images.py

@@ -1,19 +1,32 @@
+import attrs
 from django.conf import settings
 
 from src.domain.images import events, operations, queries
+from src.domain.recipes import validation as recipe_validation
 from src.services import workertasks
 
 
-def import_images_from_folder(*, folder: str) -> int:
+@attrs.frozen
+class FolderImportSummary:
     """
-    Process all JPG images in *folder*, dispatching async or sync based on settings.
+    Counts produced by an :func:`import_images_from_folder` run.
+
+    ``skipped`` holds the files that cannot produce a recipe. It is always empty
+    in async mode, where each file's outcome is only known inside the worker.
+    """
+
+    total: int
+    processed: int = 0
+    skipped: tuple[str, ...] = ()
 
-    Returns the total number of images found.
+
+def import_images_from_folder(*, folder: str) -> FolderImportSummary:
+    """
+    Process all JPG images in *folder*, dispatching async or sync based on settings.
     """
     if settings.USE_ASYNC_TASKS:
-        return _enqueue_images_in_folder(folder=folder)
-    total, _ = _process_images_in_folder(folder=folder)
-    return total
+        return FolderImportSummary(total=_enqueue_images_in_folder(folder=folder))
+    return _process_images_in_folder(folder=folder)
 
 
 def _enqueue_images_in_folder(*, folder: str) -> int:
@@ -33,18 +46,38 @@ def _enqueue_images_in_folder(*, folder: str) -> int:
     return len(paths)
 
 
-def _process_images_in_folder(*, folder: str) -> tuple[int, list[str]]:
+def _process_images_in_folder(*, folder: str) -> FolderImportSummary:
     """
-    Process all JPG images in *folder* sequentially, skipping those without Fujifilm metadata.
+    Process all JPG images in *folder* sequentially, one file at a time.
 
-    Returns:
-        A tuple of (total_found, skipped_paths).
+    A file that carries no Fujifilm metadata, or whose EXIF cannot produce a
+    valid recipe, is recorded as skipped so it never aborts the rest of the run.
     """
     paths = queries.collect_image_paths(folder=folder)
-    skipped = []
+    processed = 0
+    skipped: list[str] = []
     for path in paths:
         try:
             operations.process_image(image_path=path)
         except operations.NoFilmSimulationError:
             skipped.append(path)
-    return len(paths), skipped
+            events.publish_event(
+                event_type=events.IMAGE_IMPORT_SKIPPED,
+                image_path=path,
+                reason=events.SKIP_REASON_NO_FILM_SIMULATION,
+            )
+        except recipe_validation.InvalidFujifilmRecipeData as exc:
+            skipped.append(path)
+            events.publish_event(
+                event_type=events.IMAGE_IMPORT_SKIPPED,
+                image_path=path,
+                reason=events.SKIP_REASON_INVALID_RECIPE_DATA,
+                recipe_field=exc.field,
+            )
+        else:
+            processed += 1
+    return FolderImportSummary(
+        total=len(paths),
+        processed=processed,
+        skipped=tuple(skipped),
+    )

src/application/usecases/library/process_synced_image.py

@@ -1,9 +1,11 @@
 import structlog
 
+from src.domain.images import events as image_events
 from src.domain.images import operations as image_operations
 from src.domain.images.queries import NoFilmSimulationError
 from src.domain.library import operations as library_operations
 from src.domain.library import queries as library_queries
+from src.domain.recipes import validation as recipe_validation
 
 logger = structlog.get_logger("application.library.process_synced_image")
 
@@ -13,10 +15,11 @@ def process_synced_image(*, image_path: str, sync_run_id: int) -> None:
     Process a single image as part of sync run *sync_run_id* and record progress.
 
     Composes the pure image-processing operation with sync-run bookkeeping so the
-    processing operation stays unaware of sync: non-Fujifilm files count as
-    skipped, unexpected failures are logged and counted as errors (the run
-    continues), and successful imports count as processed. When every image in the
-    run has a terminal outcome, the run is completed.
+    processing operation stays unaware of sync: files that cannot produce a recipe
+    count as skipped, whether because they carry no Fujifilm metadata or because
+    their EXIF fails recipe validation; unexpected failures are logged and counted
+    as errors (the run continues); and successful imports count as processed. When
+    every image in the run has a terminal outcome, the run is completed.
 
     If the run no longer exists (its folder was removed while this work was
     queued), the call returns without processing.
@@ -30,6 +33,19 @@ def process_synced_image(*, image_path: str, sync_run_id: int) -> None:
         image_operations.process_image(image_path=image_path)
     except NoFilmSimulationError:
         run.record_skipped()
+        image_events.publish_event(
+            event_type=image_events.IMAGE_IMPORT_SKIPPED,
+            image_path=image_path,
+            reason=image_events.SKIP_REASON_NO_FILM_SIMULATION,
+        )
+    except recipe_validation.InvalidFujifilmRecipeData as exc:
+        run.record_skipped()
+        image_events.publish_event(
+            event_type=image_events.IMAGE_IMPORT_SKIPPED,
+            image_path=image_path,
+            reason=image_events.SKIP_REASON_INVALID_RECIPE_DATA,
+            recipe_field=exc.field,
+        )
     except Exception:
         logger.exception("Failed to process image during sync")
         run.record_error()

src/application/usecases/recipes/import_recipes_from_uploaded_files.py

@@ -5,6 +5,7 @@
 from src.domain.images.queries import NoFilmSimulationError
 from src.domain.recipes import dataclasses as recipe_dataclasses
 from src.domain.recipes import operations
+from src.domain.recipes import validation as recipe_validation
 
 
 def import_recipes_from_uploaded_files(
@@ -17,8 +18,9 @@ def import_recipes_from_uploaded_files(
     the recipe is extracted, and the temporary file is deleted immediately
     afterwards — whether the operation succeeds or fails.
 
-    Files that do not contain Fujifilm recipe EXIF data are recorded as
-    failures; processing continues with the remaining files.
+    Files that do not contain Fujifilm recipe EXIF data, and files whose EXIF
+    cannot produce a valid recipe, are recorded as failures; processing
+    continues with the remaining files.
 
     Returns an ImportRecipesResult describing which files were imported
     and which failed.
@@ -34,7 +36,7 @@ def import_recipes_from_uploaded_files(
                 tmp_path = tmp.name
             recipe, _ = operations.get_or_create_recipe_from_filepath(filepath=tmp_path)
             imported.append(recipe)
-        except NoFilmSimulationError:
+        except (NoFilmSimulationError, recipe_validation.InvalidFujifilmRecipeData):
             failed.append(file.name)
         finally:
             if tmp_path is not None:

src/domain/images/events.py

@@ -22,10 +22,15 @@
 IMAGE_RATING_SET = "image.rating.set"
 IMAGE_RATING_FAILED = "image.rating.failed"
 IMAGE_DEDUP_FILE_MISSING = "image.dedup.file.missing"
+IMAGE_IMPORT_SKIPPED = "image.import.skipped"
 TASK_IMAGE_ENQUEUED = "task.image.enqueued"
 TASK_IMAGE_STARTED = "task.image.started"
 TASK_IMAGE_COMPLETED = "task.image.completed"
 
+# Values carried on the `reason` field of IMAGE_IMPORT_SKIPPED.
+SKIP_REASON_NO_FILM_SIMULATION = "no_film_simulation"
+SKIP_REASON_INVALID_RECIPE_DATA = "invalid_recipe_data"
+
 
 def publish_event(*, event_type: str, **kwargs: object) -> None:
     """

src/domain/images/operations.py

@@ -126,6 +126,8 @@ def process_image(*, image_path: str) -> models.Image:
 
     Raises:
         NoFilmSimulationError: If the image has no film simulation EXIF data.
+        InvalidFujifilmRecipeData: If the image's EXIF cannot produce a valid
+            recipe. Nothing is persisted: the whole call is one transaction.
     """
     metadata = queries.read_image_exif(image_path=image_path)
 

src/domain/recipes/operations.py

@@ -387,6 +387,9 @@ def get_or_create_recipe_from_metadata(
     Create or retrieve a FujifilmRecipe for the given parsed EXIF data.
 
     :raises NoFilmSimulationError: If the EXIF data contains no known film simulation.
+    :raises InvalidFujifilmRecipeData: If the EXIF data cannot produce a valid
+        recipe, e.g. a colour simulation whose Color the camera set rather than
+        the user, leaving no value to store.
     """
     try:
         recipe_data = image_queries.exif_to_recipe(exif=metadata)
@@ -402,6 +405,7 @@ def get_or_create_recipe_from_filepath(
     Read EXIF from *filepath* and return the matching FujifilmRecipe, creating it if needed.
 
     :raises NoFilmSimulationError: If the file is not a Fujifilm image or has no film simulation.
+    :raises InvalidFujifilmRecipeData: If the file's EXIF cannot produce a valid recipe.
     """
     metadata = image_queries.read_image_exif(image_path=filepath)
     if metadata.camera_make.upper() != "FUJIFILM":

src/interfaces/management/commands/process_images.py

@@ -16,9 +16,18 @@ def handle(self, *args: object, **options: Any) -> None:
         folder = options["folder"]
         self.stdout.write(f"Scanning {folder} for JPG files…")
 
-        total = process_images.import_images_from_folder(folder=folder)
+        summary = process_images.import_images_from_folder(folder=folder)
 
         if settings.USE_ASYNC_TASKS:
-            self.stdout.write(self.style.SUCCESS(f"Successfully enqueued {total} tasks."))
-        else:
-            self.stdout.write(self.style.SUCCESS(f"Successfully processed {total} images."))
+            self.stdout.write(self.style.SUCCESS(f"Successfully enqueued {summary.total} tasks."))
+            return
+
+        self.stdout.write(self.style.SUCCESS(
+            f"Successfully processed {summary.processed} of {summary.total} images."
+        ))
+        if summary.skipped:
+            self.stdout.write(f"Skipped {len(summary.skipped)} image(s) that cannot produce a recipe.")
+            # The full list is behind -v 2 so a large skip run does not swamp the output.
+            if options["verbosity"] >= 2:
+                for path in summary.skipped:
+                    self.stdout.write(f"  skipped: {path}")

src/interfaces/tasks.py

@@ -7,6 +7,7 @@
 from src.application.usecases.library.process_synced_image import process_synced_image
 from src.domain.images import events, operations
 from src.domain.images.thumbnails import operations as thumbnail_operations
+from src.domain.recipes import validation as recipe_validation
 
 
 @shared_task(name="domain.process_image", bind=True, queue=settings.PROCESS_IMAGE_QUEUE)
@@ -22,7 +23,20 @@ def process_image_task(self: Any, /, *, image_path: str, **kwargs: object) -> st
     try:
         recipe = operations.process_image(image_path=image_path)
     except operations.NoFilmSimulationError:
+        events.publish_event(
+            event_type=events.IMAGE_IMPORT_SKIPPED,
+            image_path=image_path,
+            reason=events.SKIP_REASON_NO_FILM_SIMULATION,
+        )
         return f"Skipped {image_path} (no film simulation)"
+    except recipe_validation.InvalidFujifilmRecipeData as exc:
+        events.publish_event(
+            event_type=events.IMAGE_IMPORT_SKIPPED,
+            image_path=image_path,
+            reason=events.SKIP_REASON_INVALID_RECIPE_DATA,
+            recipe_field=exc.field,
+        )
+        return f"Skipped {image_path} (invalid recipe data)"
     events.publish_event(
         event_type=events.TASK_IMAGE_COMPLETED,
         image_path=image_path,

tests/functional/test_process_image_task.py

@@ -0,0 +1,64 @@
+import shutil
+from pathlib import Path
+
+import pytest
+
+from src.data import models
+from src.domain.images import events
+from src.interfaces.tasks import process_image_task
+
+FIXTURES_DIR = Path(__file__).resolve().parent.parent / "fixtures" / "images"
+FUJIFILM_FIXTURE = FIXTURES_DIR / "XS107114.JPG"
+NON_FUJIFILM_FIXTURE = FIXTURES_DIR / "sub-folder" / "img_4968_dng_embedded.jpg"
+# The camera set this file's Saturation, so its EXIF cannot produce a valid recipe.
+CAMERA_CONTROLLED_FIXTURE = (
+    Path(__file__).resolve().parent.parent / "fixtures" / "recipe" / "film_simulation_eterna.jpg"
+)
+
+
+def _skip_events(captured_logs):
+    return [e for e in captured_logs if e.get("event_type") == events.IMAGE_IMPORT_SKIPPED]
+
+
+def _run_task(fixture: Path, tmp_path: Path) -> str:
+    image_path = tmp_path / fixture.name
+    shutil.copy(fixture, image_path)
+    return process_image_task.apply(kwargs={"image_path": str(image_path)}).get()
+
+
+@pytest.mark.django_db
+class TestProcessImageTask:
+    def test_processes_a_valid_image(self, tmp_path):
+        result = _run_task(FUJIFILM_FIXTURE, tmp_path)
+
+        assert "Processed" in result
+        assert models.Image.objects.count() == 1
+
+    def test_skips_an_image_whose_recipe_data_is_invalid(self, tmp_path):
+        result = _run_task(CAMERA_CONTROLLED_FIXTURE, tmp_path)
+
+        assert "Skipped" in result
+        assert "invalid recipe data" in result
+        assert models.Image.objects.count() == 0
+
+    def test_skips_an_image_with_no_film_simulation(self, tmp_path):
+        result = _run_task(NON_FUJIFILM_FIXTURE, tmp_path)
+
+        assert "Skipped" in result
+        assert "no film simulation" in result
+        assert models.Image.objects.count() == 0
+
+    def test_publishes_an_import_skipped_event_for_invalid_recipe_data(self, tmp_path, captured_logs):
+        _run_task(CAMERA_CONTROLLED_FIXTURE, tmp_path)
+
+        skipped = _skip_events(captured_logs)
+        assert len(skipped) == 1
+        assert skipped[0]["reason"] == events.SKIP_REASON_INVALID_RECIPE_DATA
+        assert skipped[0]["recipe_field"] == "color"
+
+    def test_publishes_an_import_skipped_event_for_no_film_simulation(self, tmp_path, captured_logs):
+        _run_task(NON_FUJIFILM_FIXTURE, tmp_path)
+
+        skipped = _skip_events(captured_logs)
+        assert len(skipped) == 1
+        assert skipped[0]["reason"] == events.SKIP_REASON_NO_FILM_SIMULATION