From ed40595234790e762e916be102c688518a668d10 Mon Sep 17 00:00:00 2001
From: Lincoln Stein <lincoln.stein@gmail.com>
Date: Sat, 15 Aug 2026 14:36:28 -0400
Subject: [PATCH 1/2] feat: index video files as their extracted still frame
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is the commit that turns video support on. Everything it activates was
merged and tested in the preceding PRs, so it is also the single revertible
commit that turns it back off.

The directory walk now defaults to INDEXABLE_EXTENSIONS, and _load_image
dispatches videos to a new _load_video, which extracts a frame, caches it, and
returns the same (frame, modtime, metadata) shape. Everything downstream is
therefore media-agnostic: a video is CLIP-encoded, clustered, searchable and
curatable exactly like a photo. Video facts ride inside the existing per-image
metadata dict, so every .npz rewrite path carries them for free and indexes
predating this need no migration.

_passes_dimension_gate returns early for videos, and that early return is the
point rather than an accident of the byte bands. The gate's middle band opens
the file with PIL, which raises on a video, and the caller memoizes that
rejection into scan_rejects.npz keyed by (size, mtime) — which only changes if
the file does. A video rejected once would stay invisible forever with no UI
to clear it. Most real videos exceed the 500 KB probe ceiling and pass on size
alone, so this would never have reproduced in manual testing with real
footage, only with small clips. scan_rejects.npz gains a cache_version, bumped
here, so anyone who ran an intermediate build has their cache discarded once.

flush() now retries a failed batch one item at a time. Extracted frames are
the first realistic source of a PIL object the encoder chokes on, and losing
seven unrelated photos to one bad video would be confusing and hard to
attribute.

Cached stills are swept at save time, when the index is authoritative. One
sweep covers what would otherwise need seven hooks: mtime changes, moves,
copies, single and batch deletes, and files removed outside the app. Delete
also discards immediately, since a stale frame for a deleted file is exactly
what users notice, and album deletion clears the album's cache.

bad_files were collected but reported nowhere — the user just saw a smaller
count than expected. They now surface as a completion warning, composed with
rather than clobbering the board album's missing-on-disk notice.

Board albums stay image-only: deletion there routes through
invokeai_client.delete_image, which is unverified against video assets, and
indexing something we could not then delete would be worse than skipping it.

Tests: 19 end-to-end in test_video_indexing.py, against a new_media_album
fixture that copies test_images/ AND test_media/ so existing suites keep their
exact counts. Includes the regression that matters most — running the update
twice and asserting a stable count, which is what catches the frame cache
being re-indexed as photos. Backend 609 passed, frontend 501 passed, ruff
clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 photomap/backend/embeddings.py       | 153 ++++++++++-
 photomap/backend/routers/album.py    |  16 ++
 photomap/backend/routers/index.py    |  51 +++-
 tests/backend/conftest.py            |   2 +-
 tests/backend/fixtures.py            |  38 +++
 tests/backend/test_video_indexing.py | 381 +++++++++++++++++++++++++++
 6 files changed, 624 insertions(+), 17 deletions(-)
 create mode 100644 tests/backend/test_video_indexing.py

diff --git a/photomap/backend/embeddings.py b/photomap/backend/embeddings.py
index a1984af3..d01e7e91 100644
--- a/photomap/backend/embeddings.py
+++ b/photomap/backend/embeddings.py
@@ -41,12 +41,14 @@
     capture_download_progress,
     get_cached_encoder,
 )
-from .media_types import IMAGE_EXTENSIONS, is_video
+from .media_types import IMAGE_EXTENSIONS, INDEXABLE_EXTENSIONS, is_video
 from .metadata_extraction import MetadataExtractor
 from .metadata_formatting import format_metadata
 from .metadata_modules import SlideSummary
 from .progress import IndexingCancelled, progress_tracker
 from .util import atomic_savez
+from .video import VIDEO_METADATA_KEY, extract_video_frame
+from .video_cache import VideoFrameCache
 
 logger = logging.getLogger(__name__)
 
@@ -105,6 +107,11 @@
 # on every single update.
 SCAN_REJECTS_FILENAME = "scan_rejects.npz"
 
+# Bump to discard every existing scan-reject cache once. Version 1 retires
+# caches written while videos still went through the pixel gate, which could
+# hold permanent rejections for perfectly good video files.
+SCAN_REJECTS_VERSION = 1
+
 # Process-wide gate around the GPU-using portion of indexing. Two concurrent
 # albums each spinning up a CLIP/SigLIP encoder will OOM a typical 8-12 GiB
 # card; this serializes them so the second album waits its turn. Created
@@ -461,6 +468,11 @@ class Embeddings(BaseModel):
     # heavy upscaling. Default mirrors the Album field default in config.py.
     min_image_dimension: int = 256
     min_image_bytes: int = DIMENSION_REJECT_MIN_BYTES
+    # Album this index belongs to, used to address the per-album video-frame
+    # cache. Optional because the CLI entry points build an Embeddings from a
+    # bare .npz path with no album behind it; videos still index there, their
+    # stills just aren't persisted and get re-extracted on demand.
+    album_key: str | None = None
 
     def __init__(self, **data):
         """Ensure embeddings_path is always resolved to prevent cache key mismatches."""
@@ -536,6 +548,21 @@ def _passes_dimension_gate(self, path: Path, st: os.stat_result | None = None) -
         surfaced side by side in the album editor; either can be disabled.
         Callers that already stat'ed the file pass the result as ``st``.
         """
+        # Videos bypass the gate entirely. Its bands are image heuristics: the
+        # middle band opens the file with ``Image.open(path).size``, which
+        # raises on a video, and the caller memoizes that ``False`` into
+        # scan_rejects.npz keyed by (size, mtime) — so a video rejected once
+        # would stay invisible until the file itself changed, with no UI to
+        # clear it. An explicit early return is the point, not an accident of
+        # the byte bands: most real videos exceed the probe ceiling and would
+        # pass on size alone, so the bug would never reproduce in manual
+        # testing with real footage, only with small clips.
+        #
+        # Nor is the pixel gate applied after extraction: a 240p home video is
+        # legitimately worth indexing.
+        if is_video(path):
+            return True
+
         min_dim = self.min_image_dimension
         byte_floor = self.min_image_bytes
         pixel_gate_active = min_dim > 1
@@ -592,6 +619,18 @@ def _load_scan_rejects(self) -> dict[str, tuple[int, float]]:
                     or int(data["min_bytes"]) != self.min_image_bytes
                 ):
                     return {}
+                # A cache written before videos bypassed the gate can hold
+                # permanent rejections for them: the gate opened the file with
+                # PIL, that raised, and the verdict was memoized against
+                # (size, mtime). Since those only change if the file itself
+                # does, such a video would stay invisible forever with no UI
+                # to clear it. Discarding the whole cache once on version
+                # change is cheap — it only costs one re-probe per file.
+                version = (
+                    int(data["cache_version"]) if "cache_version" in data.files else 0
+                )
+                if version != SCAN_REJECTS_VERSION:
+                    return {}
                 return {
                     str(key): (int(size), float(mtime))
                     for key, size, mtime in zip(
@@ -615,6 +654,7 @@ def _save_scan_rejects(self, rejects: dict[str, tuple[int, float]]) -> None:
                 mtimes=np.array([v[1] for v in rejects.values()], dtype=np.float64),
                 min_dim=np.int64(self.min_image_dimension),
                 min_bytes=np.int64(self.min_image_bytes),
+                cache_version=np.int64(SCAN_REJECTS_VERSION),
             )
         except OSError as e:
             logger.warning(f"Could not save scan-reject cache {path}: {e}")
@@ -622,7 +662,7 @@ def _save_scan_rejects(self, rejects: dict[str, tuple[int, float]]) -> None:
     def get_image_files_from_directory(
         self,
         directory: Path,
-        exts: AbstractSet[str] = SUPPORTED_EXTENSIONS,
+        exts: AbstractSet[str] = INDEXABLE_EXTENSIONS,
         progress_callback: Callable | None = None,
         update_interval: int = 100,
         apply_dimension_gate: bool = True,
@@ -713,7 +753,7 @@ def get_image_files_from_directory(
     def get_image_files(
         self,
         image_paths_or_dir: list[Path] | Path,
-        exts: AbstractSet[str] = SUPPORTED_EXTENSIONS,
+        exts: AbstractSet[str] = INDEXABLE_EXTENSIONS,
         progress_callback: Callable | None = None,
         apply_dimension_gate: bool = True,
         reject_sink: dict[str, tuple[int, float]] | None = None,
@@ -827,14 +867,53 @@ def _clip_root(self) -> str | None:
         """Root directory for CLIP model caching (None = use the default cache)."""
         return None
 
-    def _load_image(
-        self, image_path: Path
-    ) -> tuple[Image.Image, float, dict] | None:
+    def _load_video(self, video_path: Path) -> tuple[Image.Image, float, dict] | None:
+        """Extract a video's still frame, cache it, and describe the video.
+
+        Returns the same shape as :meth:`_load_image` so the batch encoder
+        treats a video exactly like a photo from here on. ``None`` on any
+        failure — the caller already records that as a ``bad_file`` and moves
+        on, so "skip with a warning, never abort" is the pre-existing
+        contract rather than something new.
+
+        Thread-safe: each call spawns and owns its own ffmpeg process.
+        """
+        try:
+            extracted = extract_video_frame(video_path)
+            if extracted is None:
+                return None
+            frame, info = extracted
+
+            # Persist the still so the grid, slideshow poster and UMAP
+            # thumbnails have something to show without re-running ffmpeg.
+            # Extraction happens here, at index time, and never in a request
+            # handler — a lazy path would spawn dozens of concurrent ffmpeg
+            # processes on the first grid paint, with no timeout, no cancel
+            # and nowhere to surface a warning.
+            if self.album_key:
+                VideoFrameCache(self.album_key).store(video_path, frame)
+
+            # Video facts ride inside the existing per-image metadata dict, so
+            # every .npz rewrite path carries them for free and indexes
+            # predating video support need no migration.
+            metadata = {VIDEO_METADATA_KEY: info.model_dump()}
+            # Videos carry no EXIF for _get_modification_time to read.
+            return frame, video_path.stat().st_mtime, metadata
+        except Exception as e:
+            logger.error(f"Error processing video {video_path}: {e}")
+            return None
+
+    def _load_image(self, image_path: Path) -> tuple[Image.Image, float, dict] | None:
         """Open an image and extract modtime + metadata. Returns None on failure.
 
+        Videos are dispatched to :meth:`_load_video`, which returns the same
+        shape, so everything downstream of here is media-agnostic.
+
         Thread-safe: PIL decoders release the GIL during native I/O and the
         helpers used here don't share mutable state.
         """
+        if is_video(image_path):
+            return self._load_video(image_path)
         try:
             pil = Image.open(image_path)
             pil = ImageOps.exif_transpose(pil)
@@ -895,20 +974,38 @@ def _process_images_batch(
         buf_modtimes: list[float] = []
         buf_metadatas: list[dict] = []
 
+        def keep(j: int, path: Path, embedding) -> None:
+            embeddings.append(embedding)
+            filenames.append(path.resolve().as_posix())
+            modification_times.append(buf_modtimes[j])
+            metadatas.append(buf_metadatas[j])
+
         def flush() -> None:
             if not buf_images:
                 return
             try:
                 batch_emb = encoder.encode_images(buf_images)
             except Exception as e:
-                logger.error(f"Error encoding batch of {len(buf_images)} images: {e}")
-                bad_files.extend(buf_paths)
+                # Retry the batch one item at a time so a single hostile
+                # image costs only itself. Extracted video frames are the
+                # first realistic source of such an image, and losing a whole
+                # batch of unrelated photos to one of them would be a
+                # confusing, hard-to-attribute data loss.
+                logger.warning(
+                    f"Error encoding batch of {len(buf_images)} images ({e}); "
+                    "retrying them individually."
+                )
+                for j, path in enumerate(buf_paths):
+                    try:
+                        single = encoder.encode_images([buf_images[j]])
+                    except Exception as inner:
+                        logger.error(f"Error encoding {path}: {inner}")
+                        bad_files.append(path)
+                    else:
+                        keep(j, path, single[0])
             else:
                 for j, path in enumerate(buf_paths):
-                    embeddings.append(batch_emb[j])
-                    filenames.append(path.resolve().as_posix())
-                    modification_times.append(buf_modtimes[j])
-                    metadatas.append(buf_metadatas[j])
+                    keep(j, path, batch_emb[j])
             buf_paths.clear()
             buf_images.clear()
             buf_modtimes.clear()
@@ -1041,6 +1138,38 @@ def _save_embeddings(self, index_result: IndexResult) -> None:
         # Clear cache after saving
         _open_npz_file.cache_clear()
 
+        self._prune_video_frame_cache(
+            index_result.filenames, index_result.modification_times
+        )
+
+    def _prune_video_frame_cache(self, filenames, modification_times) -> None:
+        """Drop cached stills that the just-written index no longer refers to.
+
+        One sweep here replaces what would otherwise be seven separate
+        cleanups — mtime changes, moves, copies, single and batch deletes, and
+        files removed outside the app all leave orphans behind, and each would
+        need its own hook. Running at save time means it runs exactly when the
+        index is authoritative.
+
+        Failures only waste disk, so they are logged and swallowed.
+        """
+        if not self.album_key:
+            return
+        try:
+            cache = VideoFrameCache(self.album_key)
+            if not cache.directory.is_dir():
+                return
+            keep = {
+                cache.key_for(Path(str(name)), float(mtime))
+                for name, mtime in zip(filenames, modification_times, strict=False)
+                if is_video(Path(str(name)))
+            }
+            removed = cache.prune(keep)
+            if removed:
+                logger.info(f"Removed {removed} stale video frame(s) from the cache")
+        except Exception as e:
+            logger.warning(f"Could not prune the video frame cache: {e}")
+
     @staticmethod
     def _path_compare_key(p: Path) -> str:
         """Canonical key for the new-vs-missing diff in
diff --git a/photomap/backend/routers/album.py b/photomap/backend/routers/album.py
index cb93f0b8..cfb23290 100644
--- a/photomap/backend/routers/album.py
+++ b/photomap/backend/routers/album.py
@@ -11,6 +11,7 @@
 from ..config import Album, create_album, default_board_index_path, get_config_manager
 from ..embeddings import Embeddings
 from ..encoders import default_encoder_spec
+from ..video_cache import VideoFrameCache
 
 
 class UmapEpsSetRequest(BaseModel):
@@ -105,6 +106,7 @@ def get_embeddings_for_album(album_key: str) -> Embeddings:
         encoder_spec=album_config.encoder_spec,
         min_image_dimension=album_config.min_image_dimension,
         min_image_bytes=album_config.min_image_bytes,
+        album_key=album_key,
     )
 
 
@@ -192,6 +194,19 @@ def _cleanup_derived_index(album: Album | None) -> None:
         logger.warning(f"Could not remove index directory {derived_dir}: {e}")
 
 
+def _cleanup_video_frames(album_key: str) -> None:
+    """Remove an album's extracted video stills when the album goes away.
+
+    The frame cache lives in the per-user cache directory, keyed by album, so
+    nothing else would ever reclaim it. Never raises: a failure here costs
+    disk space, not correctness.
+    """
+    try:
+        VideoFrameCache(album_key).clear()
+    except Exception as e:
+        logger.warning(f"Could not clear video frame cache for '{album_key}': {e}")
+
+
 def _album_public_dict(album: Album) -> dict[str, Any]:
     """Album fields as exposed to the frontend.
 
@@ -349,6 +364,7 @@ async def delete_album(album_key: str) -> JSONResponse:
         album = config_manager.get_album(album_key)
         if config_manager.delete_album(album_key):
             _cleanup_derived_index(album)
+            _cleanup_video_frames(album_key)
             return JSONResponse(
                 content={
                     "success": True,
diff --git a/photomap/backend/routers/index.py b/photomap/backend/routers/index.py
index fd5bcc72..693346bb 100644
--- a/photomap/backend/routers/index.py
+++ b/photomap/backend/routers/index.py
@@ -20,6 +20,7 @@
 from ..embeddings import LAST_UPDATED_FILENAME, Embeddings, peek_encoder_spec
 from ..media_types import is_video
 from ..progress import IndexingCancelled, progress_tracker
+from ..video_cache import VideoFrameCache
 from .album import (
     AlbumDep,
     EmbeddingsDep,
@@ -341,6 +342,16 @@ def _remove_image_file(image_path: Path, move_to_trash: bool) -> None:
         ) from e
 
 
+def _discard_cached_frame(album_key: str, path: Path) -> None:
+    """Remove a deleted video's cached still. Never raises."""
+    if not is_video(path):
+        return
+    try:
+        VideoFrameCache(album_key).discard(path)
+    except Exception as e:
+        logger.debug(f"Could not discard cached frame for {path}: {e}")
+
+
 @index_router.delete(
     "/delete_image/{album_key}/{index}",
     tags=["Index"],
@@ -386,6 +397,11 @@ async def delete_image(
         print(f"{'Trashing' if move_to_trash else 'Deleting'} image: {image_path}")
         _remove_image_file(image_path, move_to_trash)
 
+        # Drop the extracted still too. The index-time sweep would collect it
+        # eventually, but not until the next update — and a stale frame for a
+        # deleted file is exactly the kind of thing users notice.
+        _discard_cached_frame(album_key, image_path)
+
         # Remove from embeddings
         embeddings.remove_image_from_embeddings(index)
 
@@ -464,6 +480,7 @@ async def delete_images(
                     else:
                         image_path.unlink()
 
+                _discard_cached_frame(album_key, image_path)
                 deleted_indices.append(index)
                 deleted_files.append(image_path.name)
             except Exception as e:
@@ -690,7 +707,12 @@ async def _resolve_board_album_files(album_config) -> tuple[list[Path], int]:
         album_config.invokeai_password,
     )
     images_dir = Path(album_config.invokeai_root).expanduser() / "outputs" / "images"
-    paths = [images_dir / name for name in names]
+    # Board albums stay image-only for now. Newer InvokeAI versions can hold
+    # video assets, but deletion for board albums routes through
+    # invokeai_client.delete_image, which has not been verified against them —
+    # indexing a video we could not then delete would be worse than skipping
+    # it. Directory albums are unaffected.
+    paths = [images_dir / name for name in names if not is_video(Path(name))]
     existing = [p for p in paths if p.is_file()]
     missing = len(paths) - len(existing)
     if missing and not existing:
@@ -751,6 +773,7 @@ async def _update_index_background_async(album_key: str, album_config):
             encoder_spec=album_config.encoder_spec,
             min_image_dimension=album_config.min_image_dimension,
             min_image_bytes=getattr(album_config, "min_image_bytes", 8192),
+            album_key=album_key,
         )
 
         if index_path.exists():
@@ -763,6 +786,7 @@ async def _update_index_background_async(album_key: str, album_config):
                 )
                 stored_spec = None
 
+            result = None
             if stored_spec is not None and stored_spec != album_config.encoder_spec:
                 logger.warning(
                     f"Encoder mismatch for album '{album_key}': existing index was built "
@@ -777,18 +801,37 @@ async def _update_index_background_async(album_key: str, album_config):
                 )
                 index_path.unlink()
                 logger.info(f"Creating new index for album '{album_key}'...")
-                await embeddings.create_index_async(
+                result = await embeddings.create_index_async(
                     image_paths, album_key, create_index=True
                 )
             else:
                 logger.info(f"Updating existing index for album '{album_key}'...")
-                await embeddings.update_index_async(image_paths, album_key)
+                result = await embeddings.update_index_async(image_paths, album_key)
         else:
             logger.info(f"Creating new index for album '{album_key}'...")
-            await embeddings.create_index_async(
+            result = await embeddings.create_index_async(
                 image_paths, album_key, create_index=True
             )
 
+        # Files that couldn't be read at all are collected but were, until
+        # now, reported nowhere — the user just saw a smaller count than
+        # expected. Videos make that much more likely (a truncated download, a
+        # codec ffmpeg can't handle), so surface it. ``add_`` rather than
+        # ``set_`` so this composes with the board album's
+        # "N of M missing on disk" notice instead of discarding it.
+        if result is not None and result.bad_files:
+            count = len(result.bad_files)
+            noun = "file" if count == 1 else "files"
+            progress_tracker.add_completion_warning(
+                album_key,
+                f"{count} {noun} could not be read and were skipped.",
+            )
+            logger.warning(
+                f"Skipped {count} unreadable {noun} in album '{album_key}': "
+                + ", ".join(p.name for p in result.bad_files[:5])
+                + ("…" if count > 5 else "")
+            )
+
         logger.info(f"Index update completed for album '{album_key}'")
 
     except IndexingCancelled as e:
diff --git a/tests/backend/conftest.py b/tests/backend/conftest.py
index dcbce405..cc3b7193 100644
--- a/tests/backend/conftest.py
+++ b/tests/backend/conftest.py
@@ -4,7 +4,7 @@
 import yaml
 
 # Import fixtures so they're available to all tests
-from fixtures import client, new_album  # noqa: F401
+from fixtures import client, new_album, new_media_album  # noqa: F401
 
 
 @pytest.fixture(autouse=True)
diff --git a/tests/backend/fixtures.py b/tests/backend/fixtures.py
index 066ce0a8..1ed81224 100644
--- a/tests/backend/fixtures.py
+++ b/tests/backend/fixtures.py
@@ -52,6 +52,44 @@ def new_album(client, tmp_path) -> dict:
     client.delete(f"/delete_album/{album_data['key']}")
 
 
+@pytest.fixture
+def new_media_album(client, tmp_path) -> dict:
+    """A temp album holding both the test images and the test videos.
+
+    Kept separate from ``new_album`` so existing suites see zero churn: they
+    keep their exact image counts, and only video tests pay the ffmpeg cost.
+    """
+    src_images = Path(__file__).parent / "test_images"
+    temp_dir = tmp_path / "media"
+    temp_dir.mkdir(parents=True, exist_ok=True)
+
+    for f in src_images.iterdir():
+        if f.is_file():
+            shutil.copy(f, temp_dir / f.name)
+    for f in TEST_MEDIA_DIR.iterdir():
+        if f.is_file():
+            shutil.copy(f, temp_dir / f.name)
+
+    album_data = {
+        "key": "test_media_album",
+        "name": "Test Media Album",
+        "image_paths": [temp_dir.as_posix()],
+        # The index deliberately lives inside the media directory, mirroring
+        # ``new_album`` — that is the layout that would let a frame cache
+        # placed next to the index get re-indexed as photos.
+        "index": (temp_dir / "embeddings.npz").as_posix(),
+        "umap_eps": 0.1,
+        "description": "A test album with videos",
+        "encoder_spec": "openai-clip:ViT-B/32",
+    }
+    response = client.post("/add_album/", json=album_data)
+    assert response.status_code == 201
+
+    yield {**album_data, "media_dir": temp_dir}
+
+    client.delete(f"/delete_album/{album_data['key']}")
+
+
 def poll_during_indexing(client, album_key, timeout=60):
     """Poll the index progress until it completes or times out."""
     start_time = time.time()
diff --git a/tests/backend/test_video_indexing.py b/tests/backend/test_video_indexing.py
new file mode 100644
index 00000000..2b712cf6
--- /dev/null
+++ b/tests/backend/test_video_indexing.py
@@ -0,0 +1,381 @@
+"""End-to-end: the directory walk now collects videos.
+
+This is the PR that turns the feature on, so these are the tests that prove a
+video actually makes it from disk into the index, gets a still, and behaves
+like a photo everywhere downstream.
+"""
+
+from __future__ import annotations
+
+import shutil
+
+import numpy as np
+import pytest
+from fixtures import (
+    build_index,
+    count_test_images,
+    count_test_media,
+    media_fixture_path,
+    poll_during_indexing,
+)
+
+from photomap.backend.embeddings import Embeddings, _open_npz_file
+from photomap.backend.video import VIDEO_METADATA_KEY, ffmpeg_exe
+from photomap.backend.video_cache import VideoFrameCache
+
+TEST_IMAGE_COUNT = count_test_images()
+TEST_MEDIA_COUNT = count_test_media()
+
+# broken.mp4 is deliberately truncated, so it is skipped rather than indexed.
+EXPECTED_VIDEO_COUNT = TEST_MEDIA_COUNT - 1
+EXPECTED_TOTAL = TEST_IMAGE_COUNT + EXPECTED_VIDEO_COUNT
+
+pytestmark = pytest.mark.skipif(
+    ffmpeg_exe() is None, reason="no bundled ffmpeg binary on this platform"
+)
+
+
+@pytest.fixture(autouse=True)
+def _clear_frame_cache():
+    yield
+    VideoFrameCache("test_media_album").clear()
+
+
+def _index_filenames(album) -> list[str]:
+    data = Embeddings.open_cached_embeddings(album["index"])
+    return [str(f) for f in data["filenames"]]
+
+
+# --------------------------------------------------------------------------
+# The walk
+# --------------------------------------------------------------------------
+
+
+def test_directory_scan_indexes_videos_alongside_images(client, new_media_album):
+    build_index(client, new_media_album)
+
+    names = {f.rsplit("/", 1)[-1] for f in _index_filenames(new_media_album)}
+
+    assert "clip.mp4" in names
+    assert "clip.webm" in names
+    assert "building1.jpeg" in names
+    assert len(names) == EXPECTED_TOTAL
+
+
+def test_unreadable_video_is_skipped_without_aborting(client, new_media_album):
+    """A truncated file must not take the rest of the album down with it."""
+    build_index(client, new_media_album)
+
+    names = {f.rsplit("/", 1)[-1] for f in _index_filenames(new_media_album)}
+
+    assert "broken.mp4" not in names
+    # ...and everything else still indexed.
+    assert len(names) == EXPECTED_TOTAL
+
+
+def test_skipped_files_are_reported_to_the_user(client, new_media_album):
+    """bad_files used to be collected and surfaced nowhere."""
+    build_index(client, new_media_album)
+
+    progress = client.get(f"/index_progress/{new_media_album['key']}").json()
+
+    assert progress["warning_message"]
+    assert "could not be read" in progress["warning_message"]
+
+
+def test_video_metadata_records_duration_fps_and_resolution(client, new_media_album):
+    build_index(client, new_media_album)
+
+    data = Embeddings.open_cached_embeddings(new_media_album["index"])
+    by_name = {
+        str(f).rsplit("/", 1)[-1]: m
+        for f, m in zip(data["filenames"], data["metadata"], strict=True)
+    }
+
+    info = by_name["clip.mp4"][VIDEO_METADATA_KEY]
+    assert info["duration"] == pytest.approx(2.0, abs=0.2)
+    assert info["fps"] == pytest.approx(10.0)
+    assert (info["width"], info["height"]) == (64, 64)
+    assert info["codec"] == "h264"
+    assert info["playable"] is True
+
+
+def test_images_carry_no_video_metadata(client, new_media_album):
+    build_index(client, new_media_album)
+
+    data = Embeddings.open_cached_embeddings(new_media_album["index"])
+    by_name = {
+        str(f).rsplit("/", 1)[-1]: m
+        for f, m in zip(data["filenames"], data["metadata"], strict=True)
+    }
+
+    assert VIDEO_METADATA_KEY not in by_name["building1.jpeg"]
+
+
+# --------------------------------------------------------------------------
+# The frame cache
+# --------------------------------------------------------------------------
+
+
+def test_still_frames_are_cached_at_index_time(client, new_media_album):
+    """Extraction must happen here, never in a request handler."""
+    build_index(client, new_media_album)
+
+    cache = VideoFrameCache(new_media_album["key"])
+    video = new_media_album["media_dir"] / "clip.mp4"
+    assert cache.get(video) is not None
+
+
+def test_cached_frames_are_not_reindexed_as_photos(client, new_media_album):
+    """The single sharpest trap in the feature.
+
+    Stills are full-resolution, so they sail through both gates. If the cache
+    ever lands inside a scanned tree, every still becomes a photo — and each
+    of those gets a still of its own, forever. Running the update twice and
+    asserting a stable count is what catches that.
+    """
+    build_index(client, new_media_album)
+    first = len(_index_filenames(new_media_album))
+
+    response = client.post(
+        "/update_index_async", json={"album_key": new_media_album["key"]}
+    )
+    assert response.status_code == 202
+    poll_during_indexing(client, new_media_album["key"])
+    _open_npz_file.cache_clear()
+
+    assert len(_index_filenames(new_media_album)) == first
+
+
+def test_deleting_a_video_removes_its_cached_frame(client, new_media_album):
+    build_index(client, new_media_album)
+
+    video = new_media_album["media_dir"] / "clip.mp4"
+    cache = VideoFrameCache(new_media_album["key"])
+    assert cache.get(video) is not None
+
+    # API indices are into the sorted order, not the stored order.
+    sorted_names = Embeddings.open_cached_embeddings(new_media_album["index"])[
+        "sorted_filenames"
+    ]
+    api_index = list(sorted_names).index(video.resolve().as_posix())
+
+    response = client.delete(
+        f"/delete_image/{new_media_album['key']}/{api_index}?move_to_trash=false"
+    )
+    assert response.status_code == 200
+    assert cache.get(video) is None
+
+
+def test_index_update_prunes_orphaned_frames(client, new_media_album):
+    """The one sweep that stands in for seven separate cleanups."""
+    build_index(client, new_media_album)
+
+    cache = VideoFrameCache(new_media_album["key"])
+    orphan = cache.directory / ("0" * 32 + ".jpg")
+    orphan.write_bytes(b"stale")
+    assert orphan.exists()
+
+    # Touch a file so the update has something to do and reaches a save.
+    shutil.copy(
+        media_fixture_path("clip.mp4"), new_media_album["media_dir"] / "extra.mp4"
+    )
+    response = client.post(
+        "/update_index_async", json={"album_key": new_media_album["key"]}
+    )
+    assert response.status_code == 202
+    poll_during_indexing(client, new_media_album["key"])
+
+    assert not orphan.exists()
+    # The real frames survived.
+    assert cache.get(new_media_album["media_dir"] / "clip.mp4") is not None
+
+
+# --------------------------------------------------------------------------
+# The dimension gate
+# --------------------------------------------------------------------------
+
+
+def test_videos_bypass_the_dimension_gate(client, tmp_path):
+    """A gate tuned to exclude thumbnails must not exclude clips.
+
+    The gate opens files with PIL, which raises on a video — and the caller
+    memoizes that rejection against (size, mtime), so a video rejected once
+    would stay invisible until the file itself changed.
+    """
+    media = tmp_path / "gated"
+    media.mkdir()
+    shutil.copy(media_fixture_path("clip.mp4"), media / "clip.mp4")
+    shutil.copy(
+        media_fixture_path("../test_images/building1.jpeg").resolve(),
+        media / "building1.jpeg",
+    )
+
+    embeddings = Embeddings(
+        embeddings_path=media / "index" / "embeddings.npz",
+        # Far above anything the fixtures could satisfy.
+        min_image_dimension=100_000,
+        min_image_bytes=0,
+    )
+    found = embeddings.get_image_files(media)
+    names = {p.name for p in found}
+
+    assert "clip.mp4" in names, "the video must survive an aggressive pixel gate"
+    assert "building1.jpeg" not in names, "the gate must still reject small images"
+
+
+def test_videos_never_enter_the_scan_reject_cache(client, tmp_path):
+    media = tmp_path / "rejects"
+    media.mkdir()
+    shutil.copy(media_fixture_path("clip.mp4"), media / "clip.mp4")
+
+    embeddings = Embeddings(
+        embeddings_path=media / "index" / "embeddings.npz",
+        min_image_dimension=100_000,
+        min_image_bytes=0,
+    )
+    sink: dict = {}
+    embeddings.get_image_files_from_directory(media, reject_sink=sink)
+
+    assert sink == {}
+
+
+def test_a_stale_scan_reject_cache_is_discarded_once(tmp_path):
+    """Users who ran an intermediate build must not stay poisoned.
+
+    A cache written while videos still went through the pixel gate can hold
+    permanent rejections for good files, and (size, mtime) only changes if the
+    file does.
+    """
+    from photomap.backend.util import atomic_savez
+
+    index_dir = tmp_path / "index"
+    index_dir.mkdir()
+    embeddings = Embeddings(embeddings_path=index_dir / "embeddings.npz")
+
+    atomic_savez(
+        index_dir / "scan_rejects.npz",
+        keys=np.array(["/some/clip.mp4"], dtype=str),
+        sizes=np.array([123], dtype=np.int64),
+        mtimes=np.array([1.0], dtype=np.float64),
+        min_dim=np.int64(embeddings.min_image_dimension),
+        min_bytes=np.int64(embeddings.min_image_bytes),
+        # No cache_version: written by a build that predates the bump.
+    )
+
+    assert embeddings._load_scan_rejects() == {}
+
+
+def test_a_current_scan_reject_cache_is_kept(tmp_path):
+    index_dir = tmp_path / "index2"
+    index_dir.mkdir()
+    embeddings = Embeddings(embeddings_path=index_dir / "embeddings.npz")
+
+    embeddings._save_scan_rejects({"/some/tiny.jpg": (10, 1.0)})
+
+    assert embeddings._load_scan_rejects() == {"/some/tiny.jpg": (10, 1.0)}
+
+
+# --------------------------------------------------------------------------
+# Downstream behaviour
+# --------------------------------------------------------------------------
+
+
+def test_retrieve_image_serves_videos_as_videos(client, new_media_album):
+    build_index(client, new_media_album)
+
+    sorted_names = list(
+        Embeddings.open_cached_embeddings(new_media_album["index"])["sorted_filenames"]
+    )
+    video_index = next(
+        i for i, name in enumerate(sorted_names) if str(name).endswith("clip.mp4")
+    )
+
+    payload = client.get(
+        f"/retrieve_image/{new_media_album['key']}/{video_index}"
+    ).json()
+
+    assert payload["media_type"] == "video"
+    assert payload["image_url"].startswith("video_frame/")
+    assert payload["video_url"].endswith("clip.mp4")
+    assert payload["video_info"]["codec"] == "h264"
+
+
+def test_thumbnails_render_for_indexed_videos(client, new_media_album):
+    build_index(client, new_media_album)
+
+    sorted_names = list(
+        Embeddings.open_cached_embeddings(new_media_album["index"])["sorted_filenames"]
+    )
+    video_index = next(
+        i for i, name in enumerate(sorted_names) if str(name).endswith("clip.mp4")
+    )
+
+    response = client.get(
+        f"/thumbnails/{new_media_album['key']}/{video_index}?size=64"
+    )
+    assert response.status_code == 200
+
+
+def test_umap_includes_videos(client, new_media_album):
+    build_index(client, new_media_album)
+
+    points = client.get(f"umap_data/{new_media_album['key']}").json()
+
+    assert len(points) == EXPECTED_TOTAL
+
+
+def test_index_metadata_counts_the_videos(client, new_media_album):
+    build_index(client, new_media_album)
+
+    payload = client.get(f"/index_metadata/{new_media_album['key']}").json()
+
+    assert payload["filename_count"] == EXPECTED_TOTAL
+    assert payload["video_count"] == EXPECTED_VIDEO_COUNT
+    assert payload["image_count"] == TEST_IMAGE_COUNT
+
+
+def test_image_search_matches_a_video_against_itself(client, new_media_album):
+    """Deterministic: a video's own frame is its own top match.
+
+    Asserts the embedding is real and reachable without claiming anything
+    semantic about CLIP's view of a test pattern.
+    """
+    import base64
+
+    build_index(client, new_media_album)
+
+    frame_path = VideoFrameCache(new_media_album["key"]).get(
+        new_media_album["media_dir"] / "clip.mp4"
+    )
+    payload = base64.b64encode(frame_path.read_bytes()).decode()
+
+    response = client.post(
+        f"/search_with_text_and_image/{new_media_album['key']}",
+        json={
+            "image_data": f"data:image/jpeg;base64,{payload}",
+            "positive_query": "",
+            "negative_query": "",
+            "image_weight": 1.0,
+        },
+    )
+    assert response.status_code == 200
+    results = response.json()["results"]
+    assert results, "the video's own frame should match something"
+
+    top = results[0]
+    sorted_names = list(
+        Embeddings.open_cached_embeddings(new_media_album["index"])["sorted_filenames"]
+    )
+    assert str(sorted_names[top["index"]]).endswith("clip.mp4")
+    assert top["score"] > 0.9
+
+
+def test_a_legacy_image_only_index_still_reports_images(client, new_album):
+    """Indexes predating video support need no migration."""
+    build_index(client, new_album)
+
+    payload = client.get(f"/retrieve_image/{new_album['key']}/0").json()
+
+    assert payload["media_type"] == "image"
+    assert payload["video_url"] == ""

From a8b575254bab5899ce379a375a2d45aa3049c09a Mon Sep 17 00:00:00 2001
From: Lincoln Stein <lincoln.stein@gmail.com>
Date: Mon, 17 Aug 2026 07:38:23 -0400
Subject: [PATCH 2/2] fix: address adversarial review of video indexing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The "N files were skipped" notice never reached the user. It was
registered from the router after create_index_async/update_index_async
returned — but complete_operation runs *inside* those calls, and it is
what folds pending notices into the ProgressInfo the poller reads and then
clears the queue. So the notice was stranded: invisible for its own run,
and silently attached to whichever run completed next. Verified directly:

    PENDING DICT: {'test_media_album': ['1 file could not be read...']}
    PROGRESS STATUS: completed
    WARNING FIELD: None

Registration moved into the indexing calls, ahead of every
complete_operation (including the no-new-images path, which can still
have skipped files). Doing it in the router could not be fixed by
re-folding after the fact: the frontend stops polling once it sees
COMPLETED, so a notice attached later races the last poll.

Its test passed only through cross-test pollution. It read the previous
test's stranded warning — it fails when run on its own, and passes again
as soon as any other indexing test precedes it in the same process. The
suite now clears the shared progress_tracker queue around each test, the
assertion pins the exact message, and a clean album is asserted to report
no notice at all as a control.

Also "1 file could not be read and were skipped" — the noun was
pluralised, the verb was not.

The dimension-gate test only exercised one of the two bands. It set
min_image_bytes=0, disabling the byte floor, so it would have passed with
the early return moved below the floor check. That matters: clip.mp4 is
~2 KB and clip.webm ~940 bytes, both under the *default* min_image_bytes
of 8192, so small real videos would be rejected on size alone. The test
now raises both bands, and a separate case pins the default floor.

Confirmed all of these fail without the fix: removing the early return
breaks 10 tests in the file.

Test-only: index into "results" in test_search_still_accepts_an_image_
query. The response is a SearchResultsResponse, so len() over the whole
payload counted its single key and was always truthy.
---
 photomap/backend/embeddings.py       | 37 +++++++++++++++
 photomap/backend/routers/index.py    | 29 +++---------
 tests/backend/test_video_guards.py   |  4 +-
 tests/backend/test_video_indexing.py | 69 +++++++++++++++++++++++++---
 4 files changed, 110 insertions(+), 29 deletions(-)

diff --git a/photomap/backend/embeddings.py b/photomap/backend/embeddings.py
index d01e7e91..54dd63c2 100644
--- a/photomap/backend/embeddings.py
+++ b/photomap/backend/embeddings.py
@@ -1142,6 +1142,37 @@ def _save_embeddings(self, index_result: IndexResult) -> None:
             index_result.filenames, index_result.modification_times
         )
 
+    @staticmethod
+    def _register_unreadable_files_warning(
+        album_key: str, result: "IndexResult | None"
+    ) -> None:
+        """Queue the "N files were skipped" notice for this run's completion.
+
+        Files that could not be read at all were collected in ``bad_files`` and
+        reported nowhere — the user just saw a smaller count than expected.
+        Videos make it far more likely (a truncated download, a codec ffmpeg
+        cannot handle), so it needs surfacing.
+
+        This has to run *before* ``complete_operation``, which is what folds
+        pending notices into the ProgressInfo the poller reads and clears the
+        queue. Registering it afterwards — from the router, once the index call
+        has returned — left the notice stranded in the queue: never shown for
+        this run, and silently attached to whichever run completed next.
+        """
+        if result is None or not result.bad_files:
+            return
+        count = len(result.bad_files)
+        noun, verb = ("file", "was") if count == 1 else ("files", "were")
+        progress_tracker.add_completion_warning(
+            album_key,
+            f"{count} {noun} could not be read and {verb} skipped.",
+        )
+        logger.warning(
+            f"Skipped {count} unreadable {noun} in album '{album_key}': "
+            + ", ".join(p.name for p in result.bad_files[:5])
+            + ("…" if count > 5 else "")
+        )
+
     def _prune_video_frame_cache(self, filenames, modification_times) -> None:
         """Drop cached stills that the just-written index no longer refers to.
 
@@ -1459,6 +1490,7 @@ def traversal_callback(count, message):
                 self.create_umap_index, result.embeddings
             )
             result.umap_embeddings = umap_embeddings
+            self._register_unreadable_files_warning(album_key, result)
             progress_tracker.complete_operation(
                 album_key, "Indexing completed successfully"
             )
@@ -1728,6 +1760,11 @@ def _on_save_start() -> None:
                 len(missing_image_paths),
                 on_save_start=_on_save_start,
             )
+            # Before either completion path below: a run that indexed nothing
+            # new can still have skipped files, and complete_operation is what
+            # consumes the queue.
+            self._register_unreadable_files_warning(album_key, result)
+
             if not did_rebuild:
                 logger.info(
                     "No new images needed to be indexed. Will not regenerate umap"
diff --git a/photomap/backend/routers/index.py b/photomap/backend/routers/index.py
index 693346bb..cb958508 100644
--- a/photomap/backend/routers/index.py
+++ b/photomap/backend/routers/index.py
@@ -786,7 +786,6 @@ async def _update_index_background_async(album_key: str, album_config):
                 )
                 stored_spec = None
 
-            result = None
             if stored_spec is not None and stored_spec != album_config.encoder_spec:
                 logger.warning(
                     f"Encoder mismatch for album '{album_key}': existing index was built "
@@ -801,36 +800,22 @@ async def _update_index_background_async(album_key: str, album_config):
                 )
                 index_path.unlink()
                 logger.info(f"Creating new index for album '{album_key}'...")
-                result = await embeddings.create_index_async(
+                await embeddings.create_index_async(
                     image_paths, album_key, create_index=True
                 )
             else:
                 logger.info(f"Updating existing index for album '{album_key}'...")
-                result = await embeddings.update_index_async(image_paths, album_key)
+                await embeddings.update_index_async(image_paths, album_key)
         else:
             logger.info(f"Creating new index for album '{album_key}'...")
-            result = await embeddings.create_index_async(
+            await embeddings.create_index_async(
                 image_paths, album_key, create_index=True
             )
 
-        # Files that couldn't be read at all are collected but were, until
-        # now, reported nowhere — the user just saw a smaller count than
-        # expected. Videos make that much more likely (a truncated download, a
-        # codec ffmpeg can't handle), so surface it. ``add_`` rather than
-        # ``set_`` so this composes with the board album's
-        # "N of M missing on disk" notice instead of discarding it.
-        if result is not None and result.bad_files:
-            count = len(result.bad_files)
-            noun = "file" if count == 1 else "files"
-            progress_tracker.add_completion_warning(
-                album_key,
-                f"{count} {noun} could not be read and were skipped.",
-            )
-            logger.warning(
-                f"Skipped {count} unreadable {noun} in album '{album_key}': "
-                + ", ".join(p.name for p in result.bad_files[:5])
-                + ("…" if count > 5 else "")
-            )
+        # The "N files were skipped" notice is registered inside the indexing
+        # calls above, not here: ``complete_operation`` runs before they
+        # return, and it is what folds pending notices into the ProgressInfo
+        # the poller reads. Anything added at this point is stranded.
 
         logger.info(f"Index update completed for album '{album_key}'")
 
diff --git a/tests/backend/test_video_guards.py b/tests/backend/test_video_guards.py
index f8406eb0..7a1f1398 100644
--- a/tests/backend/test_video_guards.py
+++ b/tests/backend/test_video_guards.py
@@ -139,7 +139,9 @@ def test_search_still_accepts_an_image_query(client, new_album):
     )
 
     assert response.status_code == 200, response.text
-    assert len(response.json()) > 0
+    # Index into "results": the response is a SearchResultsResponse object, so
+    # len() over the whole payload counts its one key and is always truthy.
+    assert len(response.json()["results"]) > 0
 
 
 # --------------------------------------------------------------------------
diff --git a/tests/backend/test_video_indexing.py b/tests/backend/test_video_indexing.py
index 2b712cf6..9cc63043 100644
--- a/tests/backend/test_video_indexing.py
+++ b/tests/backend/test_video_indexing.py
@@ -20,6 +20,7 @@
 )
 
 from photomap.backend.embeddings import Embeddings, _open_npz_file
+from photomap.backend.progress import progress_tracker
 from photomap.backend.video import VIDEO_METADATA_KEY, ffmpeg_exe
 from photomap.backend.video_cache import VideoFrameCache
 
@@ -41,6 +42,21 @@ def _clear_frame_cache():
     VideoFrameCache("test_media_album").clear()
 
 
+@pytest.fixture(autouse=True)
+def _isolate_completion_warnings():
+    """``progress_tracker`` is a module-level singleton shared by every test.
+
+    A notice queued but never consumed leaks into whichever run completes
+    next, which is how the "skipped files are reported" test came to pass:
+    it was reading the *previous* test's stranded warning, and failed as soon
+    as it ran on its own. Clearing on both sides makes each test's assertion
+    about its own run.
+    """
+    progress_tracker._completion_warnings.clear()
+    yield
+    progress_tracker._completion_warnings.clear()
+
+
 def _index_filenames(album) -> list[str]:
     data = Embeddings.open_cached_embeddings(album["index"])
     return [str(f) for f in data["filenames"]]
@@ -74,13 +90,33 @@ def test_unreadable_video_is_skipped_without_aborting(client, new_media_album):
 
 
 def test_skipped_files_are_reported_to_the_user(client, new_media_album):
-    """bad_files used to be collected and surfaced nowhere."""
+    """bad_files used to be collected and surfaced nowhere.
+
+    The notice has to be queued before ``complete_operation``, which is what
+    folds it into the ProgressInfo the poller reads and then clears the queue.
+    Queue it afterwards and it is stranded: invisible for this run, and
+    attached to whichever run finishes next.
+    """
     build_index(client, new_media_album)
 
     progress = client.get(f"/index_progress/{new_media_album['key']}").json()
 
-    assert progress["warning_message"]
-    assert "could not be read" in progress["warning_message"]
+    # Exactly one fixture (broken.mp4) is unreadable, so the wording is
+    # pinnable — including the verb agreement.
+    assert progress["warning_message"] == "1 file could not be read and was skipped."
+
+
+def test_a_clean_album_reports_no_warning(client, new_album):
+    """Nothing to skip must mean no notice at all, not an empty string.
+
+    Also the control for the test above: it fails if a notice from an earlier
+    run is still queued when this album completes.
+    """
+    build_index(client, new_album)
+
+    progress = client.get(f"/index_progress/{new_album['key']}").json()
+
+    assert progress["warning_message"] is None
 
 
 def test_video_metadata_records_duration_fps_and_resolution(client, new_media_album):
@@ -213,17 +249,38 @@ def test_videos_bypass_the_dimension_gate(client, tmp_path):
 
     embeddings = Embeddings(
         embeddings_path=media / "index" / "embeddings.npz",
-        # Far above anything the fixtures could satisfy.
+        # Both bands set far above anything the fixtures could satisfy. The
+        # byte floor matters as much as the pixel gate here: clip.mp4 is ~2 KB,
+        # under even the *default* min_image_bytes of 8192, so a small real
+        # video would be rejected on size alone without the early return.
         min_image_dimension=100_000,
-        min_image_bytes=0,
+        min_image_bytes=1_000_000,
     )
     found = embeddings.get_image_files(media)
     names = {p.name for p in found}
 
-    assert "clip.mp4" in names, "the video must survive an aggressive pixel gate"
+    assert "clip.mp4" in names, "the video must survive both gate bands"
     assert "building1.jpeg" not in names, "the gate must still reject small images"
 
 
+def test_a_small_video_survives_the_default_byte_floor(client, tmp_path):
+    """The fixtures are all smaller than the 8 KB default floor.
+
+    Worth its own case because it is the configuration users actually run,
+    and because the byte floor is checked before the pixel probe — so a
+    regression there would not show up in the aggressive-gate test above if
+    that test only raised min_image_dimension.
+    """
+    media = tmp_path / "smallvid"
+    media.mkdir()
+    shutil.copy(media_fixture_path("clip.webm"), media / "clip.webm")  # ~940 bytes
+
+    embeddings = Embeddings(embeddings_path=media / "index" / "embeddings.npz")
+    assert embeddings.min_image_bytes > (media / "clip.webm").stat().st_size
+
+    assert {p.name for p in embeddings.get_image_files(media)} == {"clip.webm"}
+
+
 def test_videos_never_enter_the_scan_reject_cache(client, tmp_path):
     media = tmp_path / "rejects"
     media.mkdir()