diff --git a/docs/user-guide/invokeai-integration.md b/docs/user-guide/invokeai-integration.md index 7a92415c..2a6c289c 100644 --- a/docs/user-guide/invokeai-integration.md +++ b/docs/user-guide/invokeai-integration.md @@ -111,6 +111,13 @@ present on the select board(s) will be displayed, and deleting an image within PhotoMapAI will trigger a deletion event in InvokeAI so that the gallery and albumm remain in sync. +Videos on the selected board(s) are indexed too, alongside the +images. As in any other album, a video is represented by a still frame +taken shortly after its start, so it takes part in search and +clustering just like a photo, and clicking its play button opens it in +the embedded player. Deleting a video routes through InvokeAI as well, +so the gallery stays in sync the same way it does for images. + !!! warning PhotoMapAI does not automatically watch the InvokeAI gallery for new or deleted files. Whenever you have generated a batch of images you want to diff --git a/photomap/backend/config.py b/photomap/backend/config.py index cb297700..3da6508a 100644 --- a/photomap/backend/config.py +++ b/photomap/backend/config.py @@ -143,17 +143,28 @@ class Album(BaseModel): def _derive_board_album_fields(cls, data: Any) -> Any: """Fill in `image_paths` and `index` for InvokeAI-board albums. - Board albums have no user-chosen image directory: their images live - under `/outputs/images` and their index in the user - data directory. Both must be derived *before* field validation - because `image_paths` has `min_length=1` and `index` is required. + Board albums have no user-chosen image directory: their media live + under `/outputs/images` and `/outputs/videos`, + and their index in the user data directory. Both must be derived + *before* field validation because `image_paths` has `min_length=1` + and `index` is required. + + The paths are recomputed on every construction rather than only when + absent. They are derived, not user-chosen, so whatever was persisted + must never win over the current root — and albums written before + video support stored only the images directory, which is how those + pick up the videos directory with no migration step. (`image_paths` + is what gates file access and relative-path resolution, so a board + video would otherwise be indexed but refused by `/videos/…`.) """ if not isinstance(data, dict) or data.get("source_type") != "invokeai_board": return data root = data.get("invokeai_root") - if root and not data.get("image_paths"): + if root: + outputs = Path(root).expanduser() / "outputs" data["image_paths"] = [ - str(Path(root).expanduser() / "outputs" / "images") + str(outputs / "images"), + str(outputs / "videos"), ] if not data.get("index") and data.get("key"): data["index"] = default_board_index_path(str(data["key"])).as_posix() @@ -183,13 +194,29 @@ def _validate_board_fields(self) -> "Album": @field_validator("image_paths") @classmethod - def expand_and_validate_image_paths(cls, v: list[str]) -> list[str]: - """Expand ~ and warn if image paths do not exist.""" - expanded = [str(Path(path).expanduser()) for path in v] - for path in expanded: + def expand_image_paths(cls, v: list[str]) -> list[str]: + """Expand ``~`` in image paths.""" + return [str(Path(path).expanduser()) for path in v] + + @model_validator(mode="after") + def _warn_about_missing_image_paths(self) -> "Album": + """Warn about configured directories that are not on disk. + + Runs after the model is built rather than as a field validator so it + can see ``source_type``: a board album's ``outputs/videos`` is derived + from the InvokeAI root, and InvokeAI does not create that directory + until it first writes a video. Warning about it would fire on every + config load for a perfectly correct setup, so a board album's derived + directories are exempt — for those, a wrong root surfaces as the + pointed "none of the board files were found" error at index time, + which is a better diagnosis than this warning could give. + """ + if self.source_type == "invokeai_board": + return self + for path in self.image_paths: if not Path(path).exists(): logger.warning(f"Image path does not exist: {path}") - return expanded + return self @field_validator("index") @classmethod diff --git a/photomap/backend/invokeai_client.py b/photomap/backend/invokeai_client.py index bb2a428a..4ca06a62 100644 --- a/photomap/backend/invokeai_client.py +++ b/photomap/backend/invokeai_client.py @@ -4,7 +4,11 @@ running InvokeAI instance: URL validation, the JWT token cache with its single-user/multi-user fallback logic, and thin wrappers around the InvokeAI REST endpoints PhotoMap consumes (version probe, board listing, -board image names, image deletion). +board image and video names, image and video deletion). + +Images and videos are distinct resources on the InvokeAI side — separate +routers, separate listing shapes, separate output directories — so each has +its own wrapper here rather than one parameterized by media type. It deliberately lives outside ``routers/`` so that non-router code (the indexing pipeline, curation) can use it without importing a FastAPI router @@ -23,6 +27,7 @@ import logging import time from collections.abc import Awaitable, Callable +from typing import NamedTuple from urllib.parse import urlsplit import httpx @@ -346,6 +351,121 @@ async def _do( return list(dict.fromkeys(all_names)) +class BoardVideoNames(NamedTuple): + """What :func:`fetch_board_video_names` learned about a board's videos. + + ``api_available`` is False when the backend answered 404 for the video + router. ``names`` may still be non-empty in that case (earlier boards in + the same call succeeded); what the flag says is that the listing is + incomplete, so an empty or short list must not be read as "these videos + were removed from the board". + """ + + names: list[str] + api_available: bool + + +async def fetch_board_video_names( + base_url: str, + board_ids: list[str], + username: str | None, + password: str | None, +) -> BoardVideoNames: + """Return the video names belonging to ``board_ids``, deduplicated. + + Videos are a separate resource from images in InvokeAI, with their own + router: this calls ``GET /api/v1/videos/names?board_id=...`` once per + board (the board is a *query* parameter here, unlike the images + endpoint's path parameter) and reads the ``video_names`` array out of the + returned ``VideoNamesResult`` object. Returned names include their + extension (``{uuid}.mp4``) and resolve under + ``/outputs/videos``. + + The same ``is_intermediate``/``categories`` filters as + :func:`fetch_board_image_names` are applied, and they matter just as much + here: a Wan pipeline writes its intermediate clips to the board too, so + an unfiltered listing returns videos the InvokeAI gallery itself hides. + + A backend predating video support has no ``/api/v1/videos`` router at + all and answers **404**, which must not fail the whole index run — board + albums have to keep indexing their images against an older InvokeAI. + That case is reported as ``api_available=False`` rather than as a bare + empty list, because a 404 is *not* unambiguous: InvokeAI's own + ``get_video_names`` calls ``assert_board_read_access``, which answers 404 + "Board not found" for a non-admin caller whose board id no longer + resolves, and a reverse proxy in front of InvokeAI can route + ``/api/v1/images`` while 404ing ``/api/v1/videos``. An empty listing and + an absent listing are different facts to the caller: the first means the + board has no videos, the second means we do not know what it has, and + only the caller can decide whether dropping previously indexed videos is + warranted. + + Names already collected from earlier boards are kept when a later board + 404s, for the same reason: they were fetched successfully and are not + made wrong by a subsequent failure. + """ + filter_params = { + "is_intermediate": "false", + "categories": ["general", "user"], + } + names_url = f"{base_url.rstrip('/')}/api/v1/videos/names" + all_names: list[str] = [] + try: + async with httpx.AsyncClient(timeout=_BOARD_FETCH_TIMEOUT) as client: + for board_id in board_ids: + params = {**filter_params, "board_id": board_id} + + async def _do( + headers: dict[str, str], params: dict = params + ) -> httpx.Response: + return await client.get(names_url, params=params, headers=headers) + + response = await _request_with_auth_fallback( + base_url, username, password, _do + ) + if response.status_code == 404: + logger.info( + "InvokeAI backend at %s did not answer the video-names " + "endpoint for board %r (no video API, or the board is " + "not readable); board videos were not listed.", + base_url, + board_id, + ) + return BoardVideoNames(list(dict.fromkeys(all_names)), False) + if response.status_code >= 400: + raise HTTPException( + status_code=502, + detail=( + f"InvokeAI backend returned {response.status_code} for " + f"videos on board {board_id!r}: {response.text[:200]}" + ), + ) + try: + payload = response.json() + except ValueError as exc: + raise HTTPException( + status_code=502, + detail=f"Video-names endpoint for board {board_id!r} did not return JSON", + ) from exc + names = payload.get("video_names") if isinstance(payload, dict) else None + if not isinstance(names, list): + raise HTTPException( + status_code=502, + detail=f"Video-names endpoint for board {board_id!r} returned an unexpected shape", + ) + all_names.extend(str(name) for name in names) + except httpx.RequestError as exc: + logger.warning("InvokeAI video-names request failed: %s", exc) + raise HTTPException( + status_code=502, + detail=f"Could not reach InvokeAI backend at {base_url}: {exc}", + ) from exc + + # A video belongs to one board, but overlapping selections (a board plus + # "none") must not index the same file twice — dedupe preserving order. + return BoardVideoNames(list(dict.fromkeys(all_names)), True) + + async def delete_image( base_url: str, image_name: str, @@ -389,3 +509,76 @@ async def _do(headers: dict[str, str]) -> httpx.Response: f"{response.text[:200]}" ), ) + + +async def delete_video( + base_url: str, + video_name: str, + username: str | None, + password: str | None, +) -> None: + """Delete ``video_name`` on the InvokeAI backend. + + The video counterpart of :func:`delete_image`, and a separate call rather + than a different path handed to that one: videos live behind their own + router and answer with a ``DeleteVideosResult`` body instead of the + images route's ``DeleteImagesResult``. + + Today's single-video route reports failure as a 500 and always sends an + empty ``failed_videos``; a populated one comes from the batch + ``POST /videos/delete``. The list is still checked, because it is the + one way this endpoint can report a failure *with* HTTP 200, and taking + that for success would drop the row from the local index while the file + stayed in InvokeAI — the video would silently reappear on the next + re-index. + + As with images, a 404 means InvokeAI no longer knows the video: log and + return so the caller can still drop it locally. + """ + url = f"{base_url.rstrip('/')}/api/v1/videos/i/{video_name}" + try: + async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT) as client: + + async def _do(headers: dict[str, str]) -> httpx.Response: + return await client.delete(url, headers=headers) + + response = await _request_with_auth_fallback( + base_url, username, password, _do + ) + except httpx.RequestError as exc: + logger.warning("InvokeAI video delete request failed: %s", exc) + raise HTTPException( + status_code=502, + detail=f"Could not reach InvokeAI backend at {base_url}: {exc}", + ) from exc + + if response.status_code == 404: + logger.warning( + "InvokeAI no longer has video %s; removing from index anyway", + video_name, + ) + return + if response.status_code >= 400: + raise HTTPException( + status_code=502, + detail=( + f"InvokeAI video delete returned {response.status_code}: " + f"{response.text[:200]}" + ), + ) + + try: + payload = response.json() + except ValueError: + payload = None + # Guarded on the shape, not just on the parse: a proxy (or a future + # response model) can answer 200 with a JSON array or string, and + # ``.get`` on one of those would escape as an AttributeError *after* + # InvokeAI had already deleted the video — leaving the row in the local + # index pointing at a file that is gone. + failed = payload.get("failed_videos") if isinstance(payload, dict) else None + if isinstance(failed, list) and video_name in failed: + raise HTTPException( + status_code=502, + detail=f"InvokeAI reported that {video_name} could not be deleted", + ) diff --git a/photomap/backend/routers/index.py b/photomap/backend/routers/index.py index cb958508..a7fee279 100644 --- a/photomap/backend/routers/index.py +++ b/photomap/backend/routers/index.py @@ -8,7 +8,9 @@ import os import shutil from pathlib import Path +from typing import NamedTuple +import numpy as np from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException from fastapi.responses import JSONResponse from pydantic import BaseModel @@ -296,6 +298,34 @@ async def index_metadata(album_config: AlbumDep) -> EmbeddingsIndexMetadata: ) +async def _delete_board_media(album_config, media_path: Path) -> None: + """Delete one file of an InvokeAI-board album through InvokeAI's API. + + Images and videos are distinct resources over there, with their own + delete endpoints, and a mis-dispatch fails *quietly*: InvokeAI's + ``DELETE /images/i/{name}`` wraps its lookup in a bare ``except`` and + answers 200 with an empty ``deleted_images`` for a name it does not know, + so handing it a video name would read as success. The row would vanish + from the local index while the video stayed on the board, then reappear + on the next re-index. Dispatch on the suffix so each name reaches its own + endpoint. + """ + if is_video(media_path): + await invokeai_client.delete_video( + album_config.invokeai_url, + media_path.name, + album_config.invokeai_username, + album_config.invokeai_password, + ) + else: + await invokeai_client.delete_image( + album_config.invokeai_url, + media_path.name, + album_config.invokeai_username, + album_config.invokeai_password, + ) + + def _remove_image_file(image_path: Path, move_to_trash: bool) -> None: """Trash or unlink ``image_path``, translating OS failures into HTTP errors whose ``detail`` tells the user what to fix. @@ -376,12 +406,11 @@ async def delete_image( # would leave a dangling row in InvokeAI's database, so route # the deletion through its API (which also removes the file). # ``move_to_trash`` has no meaning here and is ignored. - await invokeai_client.delete_image( - album_config.invokeai_url, - image_path.name, - album_config.invokeai_username, - album_config.invokeai_password, - ) + await _delete_board_media(album_config, image_path) + # Same reason as the non-board branch below: a board album can + # hold videos, and their extracted stills outlive the row unless + # they are discarded here. + _discard_cached_frame(album_key, image_path) embeddings.remove_image_from_embeddings(index) return JSONResponse( content={ @@ -465,12 +494,7 @@ async def delete_images( # database, so route each deletion through its API # (which also removes the file). ``move_to_trash`` has # no meaning here and is ignored. - await invokeai_client.delete_image( - album_config.invokeai_url, - image_path.name, - album_config.invokeai_username, - album_config.invokeai_password, - ) + await _delete_board_media(album_config, image_path) else: if not image_path.exists() or not image_path.is_file(): errors.append(f"Index {index}: File not found") @@ -687,47 +711,110 @@ async def copy_images( raise HTTPException(status_code=500, detail=f"Failed to copy images: {str(e)}") from e -async def _resolve_board_album_files(album_config) -> tuple[list[Path], int]: - """Resolve an InvokeAI-board album's images to local file paths. +class BoardAlbumFiles(NamedTuple): + """What :func:`_resolve_board_album_files` found for a board album.""" - Fetches the selected boards' image names from the InvokeAI API and maps - them to ``/outputs/images/``. Names the API lists - but that don't exist locally are skipped with a warning; if *none* of - them exist the InvokeAI root is almost certainly wrong, which deserves - a pointed error instead of a generic "no images found". + paths: list[Path] + missing: int + video_api_available: bool - Returns the existing files plus the count of listed-but-missing ones, so - the caller can surface that discrepancy to the user (the InvokeAI gallery - will show a higher total than the album indexes). + +async def _resolve_board_album_files(album_config) -> BoardAlbumFiles: + """Resolve an InvokeAI-board album's images and videos to local paths. + + Fetches the selected boards' image names *and* video names from the + InvokeAI API and maps them to ``/outputs/images/`` + and ``/outputs/videos/`` respectively — the two are + separate resources on the InvokeAI side, listed by separate endpoints and + stored in separate directories, so both have to be asked for by name. + Names the API lists but that don't exist locally are skipped with a + warning; if *none* of them exist the InvokeAI root is almost certainly + wrong, which deserves a pointed error instead of a generic "no images + found". The error names the directory that actually came up empty, since + a board holding only videos would otherwise blame ``outputs`` as a whole. + + Returns the existing files, the count of listed-but-missing ones (so the + caller can surface that discrepancy — the InvokeAI gallery will show a + higher total than the album indexes), and whether the video listing could + be obtained at all. That last flag is not cosmetic: an index update drops + rows for every file the resolver did not return, so a video listing that + failed rather than came back empty would silently prune videos that are + still on the board. """ - names = await invokeai_client.fetch_board_image_names( + outputs = Path(album_config.invokeai_root).expanduser() / "outputs" + image_names = await invokeai_client.fetch_board_image_names( album_config.invokeai_url, album_config.invokeai_board_ids, album_config.invokeai_username, album_config.invokeai_password, ) - images_dir = Path(album_config.invokeai_root).expanduser() / "outputs" / "images" - # 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))] + # ``api_available`` is False against an InvokeAI with no video API, so a + # board album on an older backend keeps indexing exactly as it did before + # — the caller only has to say so when it costs the album something. + video_names, video_api_available = await invokeai_client.fetch_board_video_names( + album_config.invokeai_url, + album_config.invokeai_board_ids, + album_config.invokeai_username, + album_config.invokeai_password, + ) + + image_paths = [outputs / "images" / name for name in image_names] + video_paths = [outputs / "videos" / name for name in video_names] + paths = image_paths + video_paths existing = [p for p in paths if p.is_file()] missing = len(paths) - len(existing) if missing and not existing: + if not video_paths: + what, where = f"{len(paths)} board image(s)", outputs / "images" + elif not image_paths: + what, where = f"{len(paths)} board video(s)", outputs / "videos" + else: + what, where = f"{len(paths)} board image(s) and video(s)", outputs raise HTTPException( status_code=502, detail=( - f"None of the {len(paths)} board images were found under " - f"{images_dir} — check the InvokeAI root directory." + f"None of the {what} listed by InvokeAI were found under " + f"{where} — check the InvokeAI root directory." ), ) if missing: logger.warning( - f"{missing} of {len(paths)} board images not found under {images_dir}; skipping them." + f"{missing} of {len(paths)} board files not found under {outputs}; skipping them." ) - return existing, missing + return BoardAlbumFiles(existing, missing, video_api_available) + + +def _indexed_videos_dropped(index_path: Path, listed: list[Path]) -> int: + """How many indexed video rows are absent from ``listed``. + + Counting *all* indexed videos would be wrong: a video listing can fail + part-way (an unreadable board among several) and still return names, and + the update only prunes rows for files the resolver did not return. The + keys come from :meth:`Embeddings._path_compare_key` applied to the same + paths the update itself will diff, so this count is exactly what that + diff will drop. + + The .npz is read directly rather than through + ``Embeddings.open_cached_embeddings``: this runs *before* the index is + rewritten (and before an encoder-mismatch rebuild may unlink it), and + that opener is an lru_cache keyed on the path alone, so priming it here + would hand the pre-update contents to any later reader. + """ + if not index_path.exists(): + return 0 + keep = {Embeddings._path_compare_key(p) for p in listed if is_video(p)} + try: + with np.load(index_path, allow_pickle=True) as data: + filenames = [str(f) for f in data["filenames"]] + except Exception as e: # pragma: no cover - unreadable index + logger.warning(f"Could not read {index_path} to count indexed videos: {e}") + return 0 + return sum( + 1 + for name in filenames + if is_video(Path(name)) + and Embeddings._path_compare_key(Path(name)) not in keep + ) # Background Tasks @@ -739,7 +826,7 @@ async def _update_index_background_async(album_key: str, album_config): album_key, 0, "Fetching board contents from InvokeAI..." ) try: - image_paths, missing = await _resolve_board_album_files(album_config) + board_files = await _resolve_board_album_files(album_config) except HTTPException as e: progress_tracker.set_error( album_key, @@ -747,9 +834,18 @@ async def _update_index_background_async(album_key: str, album_config): f"{album_config.invokeai_url}: {e.detail}", ) return + image_paths, missing = board_files.paths, board_files.missing if not image_paths: + # "No videos" is something we only know when the listing + # answered. Saying it after an outage would state as fact the + # very thing that could not be checked. progress_tracker.set_error( - album_key, "Selected InvokeAI board(s) contain no images" + album_key, + "Selected InvokeAI board(s) contain no images, and InvokeAI " + "did not answer the video listing, so any videos on them " + "could not be indexed." + if not board_files.video_api_available + else "Selected InvokeAI board(s) contain no images or videos", ) return # Surface the gallery-vs-indexed discrepancy (always set so a clean @@ -759,11 +855,30 @@ async def _update_index_background_async(album_key: str, album_config): total = len(image_paths) + missing progress_tracker.set_completion_warning( album_key, - f"{missing} of {total} image(s) listed by InvokeAI were not " + f"{missing} of {total} file(s) listed by InvokeAI were not " f"found on disk and were skipped.", ) else: progress_tracker.set_completion_warning(album_key, None) + # An unavailable video API is silent when the album has no videos + # to lose (an InvokeAI predating video support, which is a + # supported setup). When rows *are* about to be pruned for a + # reason that has nothing to do with the board's contents, say so + # — otherwise the run reports a clean success while the album + # quietly shrinks. + if not board_files.video_api_available: + at_risk = _indexed_videos_dropped( + Path(album_config.index), board_files.paths + ) + if at_risk: + progress_tracker.add_completion_warning( + album_key, + f"InvokeAI did not answer the video listing, so this " + f"album's videos could not be checked; {at_risk} " + f"previously indexed video(s) were dropped. They are " + f"restored by the next update that reaches the video " + f"API.", + ) else: image_paths = [Path(path) for path in album_config.image_paths] index_path = Path(album_config.index) diff --git a/tests/backend/test_albums.py b/tests/backend/test_albums.py index 155159ae..96157a47 100644 --- a/tests/backend/test_albums.py +++ b/tests/backend/test_albums.py @@ -413,8 +413,11 @@ def test_add_board_album_derives_paths_and_index(client): album = manager.get_album("board_album") assert album is not None assert album.source_type == "invokeai_board" + # Both output directories: a board's videos are indexed alongside its + # images, and ``image_paths`` is what grants access to them. assert album.image_paths == [ - str(Path("/srv/invokeai") / "outputs" / "images") + str(Path("/srv/invokeai") / "outputs" / "images"), + str(Path("/srv/invokeai") / "outputs" / "videos"), ] assert album.index == default_board_index_path("board_album").as_posix() assert album.invokeai_board_ids == ["b1", "none"] @@ -423,6 +426,55 @@ def test_add_board_album_derives_paths_and_index(client): client.delete("/delete_album/board_album") +def test_board_album_written_before_video_support_gains_the_videos_path(): + """Albums persisted when only ``outputs/images`` was derived must pick up + the videos directory on load, with no migration step: ``image_paths`` is + what gates file access, so without it a board video indexes and then 403s + at playback.""" + from photomap.backend.config import Album + + album = Album( + **_board_album_payload(image_paths=[str(Path("/srv/invokeai/outputs/images"))]) + ) + + assert album.image_paths == [ + str(Path("/srv/invokeai") / "outputs" / "images"), + str(Path("/srv/invokeai") / "outputs" / "videos"), + ] + + +def test_board_album_does_not_warn_about_an_absent_videos_directory(caplog): + """InvokeAI creates ``outputs/videos`` only once it writes a video, and the + directory is derived rather than user-chosen — warning about it would fire + on every config load for a correct setup.""" + import logging + + from photomap.backend.config import Album + + with caplog.at_level(logging.WARNING, logger="photomap.backend.config"): + Album(**_board_album_payload()) + + assert "Image path does not exist" not in caplog.text + + +def test_directory_album_still_warns_about_a_missing_path(caplog, tmp_path): + """The warning itself stays: it is the only notice a directory album gets + for a path that is not there.""" + import logging + + from photomap.backend.config import Album + + with caplog.at_level(logging.WARNING, logger="photomap.backend.config"): + Album( + key="dir_album", + name="Dir Album", + image_paths=[str(tmp_path / "not-there")], + index=str(tmp_path / "idx.npz"), + ) + + assert "Image path does not exist" in caplog.text + + def test_board_album_yaml_round_trip(client): """All board fields survive a save/reload cycle of the YAML config.""" response = client.post("/add_album/", json=_board_album_payload()) diff --git a/tests/backend/test_invokeai_board_index.py b/tests/backend/test_invokeai_board_index.py index 312d0275..d87fe1b1 100644 --- a/tests/backend/test_invokeai_board_index.py +++ b/tests/backend/test_invokeai_board_index.py @@ -1,9 +1,15 @@ """Tests for indexing and curating InvokeAI board-backed albums. -The InvokeAI HTTP API is stubbed at the ``invokeai_client`` layer: a fake -``fetch_board_image_names`` serves a mutable board → image-name mapping, and -the images themselves are UUID-named copies of the bundled test images laid -out under a fake ``/outputs/images`` directory. +The InvokeAI HTTP API is stubbed at the ``invokeai_client`` layer: fake +``fetch_board_image_names`` / ``fetch_board_video_names`` serve mutable +board → name mappings, and the files themselves are UUID-named copies of the +bundled fixtures laid out under fake ``/outputs/images`` and +``/outputs/videos`` directories. + +The video mapping starts out empty, so the image-only tests below run the +same path as a board that simply has no videos. An InvokeAI too old to have +a video API is a *different* shape — an unavailable listing rather than an +empty one — and is exercised by flipping ``video_api_available``. """ import shutil @@ -14,11 +20,19 @@ import numpy as np import pytest from fastapi import HTTPException +from fixtures import media_fixture_path from photomap.backend import invokeai_client +from photomap.backend.invokeai_client import BoardVideoNames +from photomap.backend.video import VIDEO_METADATA_KEY, ffmpeg_exe +from photomap.backend.video_cache import VideoFrameCache ALBUM_KEY = "board_index_album" +requires_ffmpeg = pytest.mark.skipif( + ffmpeg_exe() is None, reason="no bundled ffmpeg binary on this platform" +) + def _index_filenames(index_path: Path) -> set[str]: data = np.load(index_path, allow_pickle=True) @@ -46,6 +60,8 @@ def board_album(client, tmp_path, monkeypatch): """ images_dir = tmp_path / "invokeai" / "outputs" / "images" images_dir.mkdir(parents=True) + videos_dir = tmp_path / "invokeai" / "outputs" / "videos" + videos_dir.mkdir(parents=True) src_images = sorted( p for p in (Path(__file__).parent / "test_images").iterdir() if p.is_file() )[:4] @@ -57,14 +73,29 @@ def board_album(client, tmp_path, monkeypatch): names.append(name) boards = {"b1": list(names)} - - async def fake_fetch(base_url, board_ids, username, password): + video_boards: dict[str, list[str]] = {} + # Flipped by tests that need the "InvokeAI did not answer the video + # listing" case, which is not the same as a board with no videos. + # ``names_on_outage`` covers the partial variant: some boards answered + # before a later one 404'd, so names come back *with* the failure flag. + video_api: dict = {"available": True, "names_on_outage": []} + + def _merged(mapping, board_ids): merged = [] for board_id in board_ids: - merged.extend(boards.get(board_id, [])) + merged.extend(mapping.get(board_id, [])) return list(dict.fromkeys(merged)) + async def fake_fetch(base_url, board_ids, username, password): + return _merged(boards, board_ids) + + async def fake_fetch_videos(base_url, board_ids, username, password): + if not video_api["available"]: + return BoardVideoNames(list(video_api["names_on_outage"]), False) + return BoardVideoNames(_merged(video_boards, board_ids), True) + monkeypatch.setattr(invokeai_client, "fetch_board_image_names", fake_fetch) + monkeypatch.setattr(invokeai_client, "fetch_board_video_names", fake_fetch_videos) index_path = tmp_path / "index" / "embeddings.npz" album = { @@ -83,7 +114,10 @@ async def fake_fetch(base_url, board_ids, username, password): yield { "album": album, "boards": boards, + "video_boards": video_boards, + "video_api": video_api, "images_dir": images_dir, + "videos_dir": videos_dir, "index_path": index_path, "src_images": src_images, } @@ -108,6 +142,273 @@ def test_board_album_index_contains_board_images(client, board_album): ) +def _add_board_video(board_album, fixture="clip.mp4"): + """Copy a video fixture into the fake outputs/videos and list it on b1.""" + name = f"{uuid.uuid4()}.mp4" + shutil.copy(media_fixture_path(fixture), board_album["videos_dir"] / name) + board_album["video_boards"].setdefault("b1", []).append(name) + return name + + +def _index_of(client, name, count): + """The sorted-index position holding ``name``, or None.""" + for idx in range(count): + filename = client.get(f"/retrieve_image/{ALBUM_KEY}/{idx}").json()["filename"] + if Path(filename).name == name: + return idx + return None + + +def test_board_album_covers_both_output_directories(client, board_album): + """Board albums derive their paths from the InvokeAI root, and the videos + directory has to be one of them: ``image_paths`` is what gates file access + and relative-path resolution, so a board video that is indexed but not + covered here would be refused by ``/videos/…`` at playback time.""" + album = client.get(f"/album/{ALBUM_KEY}/").json() + assert [Path(p).name for p in album["image_paths"]] == ["images", "videos"] + + +@requires_ffmpeg +def test_board_videos_are_indexed_alongside_images(client, board_album): + video_name = _add_board_video(board_album) + + _build_index(client) + + metadata = client.get(f"/index_metadata/{ALBUM_KEY}").json() + assert metadata["filename_count"] == 5 + assert metadata["image_count"] == 4 + assert metadata["video_count"] == 1 + assert video_name in _index_filenames(board_album["index_path"]) + + +@requires_ffmpeg +def test_indexed_board_video_carries_its_probe_facts(client, board_album): + """The video rides through the encoder as a still frame, but the facts + ffmpeg reported have to survive into the index — they are what make the + slide render as a video rather than as a photo.""" + video_name = _add_board_video(board_album) + _build_index(client) + + idx = _index_of(client, video_name, 5) + assert idx is not None, "indexed video not found by name" + slide = client.get(f"/retrieve_image/{ALBUM_KEY}/{idx}").json() + + assert slide["media_type"] == "video" + assert slide["video_info"]["duration"] > 0 + # The still, not the raw container, is what `image_url` points at. + assert slide["image_url"].startswith("video_frame/") + assert slide["video_url"].endswith(video_name) + + data = np.load(board_album["index_path"], allow_pickle=True) + stored = { + Path(str(f)).name: m + for f, m in zip(data["filenames"], data["metadata"], strict=True) + } + assert VIDEO_METADATA_KEY in stored[video_name] + + +@requires_ffmpeg +def test_deleting_a_board_video_uses_the_video_endpoint( + client, board_album, monkeypatch +): + """Videos are a separate resource on the InvokeAI side, and a mis-dispatch + is silent: the *image* delete endpoint answers 200 with an empty + ``deleted_images`` for a name it does not know, so the row would leave the + index while the video stayed on the board.""" + video_name = _add_board_video(board_album) + _build_index(client) + + deleted_videos = [] + deleted_images = [] + + async def fake_delete_video(base_url, name, username, password): + deleted_videos.append(name) + + async def fake_delete_image(base_url, name, username, password): + deleted_images.append(name) + + monkeypatch.setattr(invokeai_client, "delete_video", fake_delete_video) + monkeypatch.setattr(invokeai_client, "delete_image", fake_delete_image) + + idx = _index_of(client, video_name, 5) + assert idx is not None + response = client.delete(f"/delete_image/{ALBUM_KEY}/{idx}") + assert response.status_code == 200, response.text + + assert deleted_videos == [video_name] + assert deleted_images == [] + # InvokeAI owns the file; PhotoMap only drops the index row. + assert (board_album["videos_dir"] / video_name).exists() + assert video_name not in _index_filenames(board_album["index_path"]) + + +@requires_ffmpeg +def test_batch_delete_splits_videos_from_images(client, board_album, monkeypatch): + video_name = _add_board_video(board_album) + _build_index(client) + + deleted_videos = [] + deleted_images = [] + + async def fake_delete_video(base_url, name, username, password): + deleted_videos.append(name) + + async def fake_delete_image(base_url, name, username, password): + deleted_images.append(name) + + monkeypatch.setattr(invokeai_client, "delete_video", fake_delete_video) + monkeypatch.setattr(invokeai_client, "delete_image", fake_delete_image) + + video_idx = _index_of(client, video_name, 5) + image_idx = next(i for i in range(5) if i != video_idx) + image_name = Path( + client.get(f"/retrieve_image/{ALBUM_KEY}/{image_idx}").json()["filename"] + ).name + + response = client.post( + f"/delete_images/{ALBUM_KEY}", json={"indices": [video_idx, image_idx]} + ) + assert response.status_code == 200, response.text + assert response.json()["deleted_count"] == 2 + + assert deleted_videos == [video_name] + assert deleted_images == [image_name] + + +def test_missing_board_video_is_skipped_not_fatal(client, board_album): + """A video InvokeAI lists but that is absent on disk is counted in the + same discrepancy warning as a missing image.""" + board_album["video_boards"].setdefault("b1", []).append(f"{uuid.uuid4()}.mp4") + + _build_index(client) + + progress = client.get(f"/index_progress/{ALBUM_KEY}").json() + assert progress["status"] == "completed" + assert "1 of 5" in progress["warning_message"] + metadata = client.get(f"/index_metadata/{ALBUM_KEY}").json() + assert metadata["filename_count"] == 4 + + +@requires_ffmpeg +def test_board_video_is_served_through_the_video_routes(client, board_album): + """The whole point of covering ``outputs/videos`` in ``image_paths``: an + indexed board video must actually be fetchable, both the container and + its extracted still, instead of being refused by the access gate.""" + video_name = _add_board_video(board_album) + _build_index(client) + + idx = _index_of(client, video_name, 5) + slide = client.get(f"/retrieve_image/{ALBUM_KEY}/{idx}").json() + + assert client.get(f"/{slide['video_url']}").status_code == 200 + assert client.get(f"/{slide['image_url']}").status_code == 200 + + +@requires_ffmpeg +def test_deleting_a_board_video_discards_its_cached_still( + client, board_album, monkeypatch +): + """InvokeAI owns the container, but the extracted still is ours: it has to + go with the row, or it lingers until the next index-time sweep and can be + served for a slide that no longer exists.""" + video_name = _add_board_video(board_album) + _build_index(client) + + async def fake_delete_video(base_url, name, username, password): + return None + + monkeypatch.setattr(invokeai_client, "delete_video", fake_delete_video) + + video_path = board_album["videos_dir"] / video_name + cache = VideoFrameCache(ALBUM_KEY) + assert cache.get(video_path) is not None, "indexing should have cached a still" + + idx = _index_of(client, video_name, 5) + assert client.delete(f"/delete_image/{ALBUM_KEY}/{idx}").status_code == 200 + + assert cache.get(video_path) is None + + +@requires_ffmpeg +def test_video_api_outage_warns_when_indexed_videos_are_dropped(client, board_album): + """A video listing that could not be fetched looks exactly like a board + whose videos were all deleted, and the update prunes rows for everything + the listing omits. Losing them silently would report a clean success while + the album quietly shrank, so the run has to say what happened.""" + _add_board_video(board_album) + _build_index(client) + assert client.get(f"/index_metadata/{ALBUM_KEY}").json()["video_count"] == 1 + + board_album["video_api"]["available"] = False + _build_index(client) + + progress = client.get(f"/index_progress/{ALBUM_KEY}").json() + assert "1 previously indexed video(s) were dropped" in progress["warning_message"] + assert client.get(f"/index_metadata/{ALBUM_KEY}").json()["video_count"] == 0 + + +@requires_ffmpeg +def test_partial_video_listing_does_not_claim_kept_videos_were_dropped( + client, board_album +): + """A listing can fail on one board after another answered, so names come + back *with* the failure flag. Those videos are still indexed, and saying + they were dropped would be plainly false.""" + video_name = _add_board_video(board_album) + _build_index(client) + + board_album["video_api"]["available"] = False + board_album["video_api"]["names_on_outage"] = [video_name] + _build_index(client) + + progress = client.get(f"/index_progress/{ALBUM_KEY}").json() + assert not progress["warning_message"] + assert client.get(f"/index_metadata/{ALBUM_KEY}").json()["video_count"] == 1 + + +def test_video_outage_on_an_image_free_board_does_not_claim_there_are_no_videos( + client, board_album +): + """With no images on the board and no answer about its videos, the album + is empty for a reason worth naming — "contains no videos" is exactly what + could not be checked.""" + board_album["boards"]["b1"].clear() + board_album["video_api"]["available"] = False + + response = client.post("/update_index_async", json={"album_key": ALBUM_KEY}) + assert response.status_code == 202 + progress = _poll_until(client, ALBUM_KEY, {"completed", "error"}) + assert progress["status"] == "error" + assert "did not answer the video listing" in progress["error_message"] + + +def test_video_api_outage_is_silent_with_no_indexed_videos(client, board_album): + """The same outage against an InvokeAI predating video support costs the + album nothing, and that is the common case — it must not nag.""" + board_album["video_api"]["available"] = False + + _build_index(client) + + progress = client.get(f"/index_progress/{ALBUM_KEY}").json() + assert progress["status"] == "completed" + assert not progress["warning_message"] + assert client.get(f"/index_metadata/{ALBUM_KEY}").json()["filename_count"] == 4 + + +def test_videos_only_board_error_names_the_videos_directory(client, board_album): + """A board holding only videos, none of them on disk, must not be blamed + on ``outputs`` as a whole — the images directory is fine and untouched.""" + board_album["boards"]["b1"].clear() + board_album["video_boards"]["b1"] = [f"{uuid.uuid4()}.mp4" for _ in range(2)] + + response = client.post("/update_index_async", json={"album_key": ALBUM_KEY}) + assert response.status_code == 202 + progress = _poll_until(client, ALBUM_KEY, {"completed", "error"}) + assert progress["status"] == "error" + assert "2 board video(s)" in progress["error_message"] + assert str(board_album["videos_dir"]) in progress["error_message"] + + def test_missing_board_images_surface_completion_warning(client, board_album): """A board name InvokeAI lists but that's absent on disk is skipped, and the discrepancy is surfaced as a non-fatal warning on the completed run (the diff --git a/tests/backend/test_invokeai_client.py b/tests/backend/test_invokeai_client.py index c0c69776..b6b37a47 100644 --- a/tests/backend/test_invokeai_client.py +++ b/tests/backend/test_invokeai_client.py @@ -1,13 +1,14 @@ """Tests for the raw HTTP behaviour of ``photomap.backend.invokeai_client``. The board-index tests (``test_invokeai_board_index.py``) monkeypatch -``fetch_board_image_names`` wholesale, so the request-building details are -covered here instead — most importantly that board fetches ask InvokeAI to -exclude canvas intermediates and mask/control assets, mirroring what the -InvokeAI gallery itself displays. +``fetch_board_image_names`` / ``fetch_board_video_names`` wholesale, so the +request-building details are covered here instead — most importantly that +board fetches ask InvokeAI to exclude canvas intermediates and mask/control +assets, mirroring what the InvokeAI gallery itself displays. """ import pytest +from fastapi import HTTPException from photomap.backend import invokeai_client @@ -42,6 +43,10 @@ async def get(self, url, **kwargs): self.calls.append({"url": url, "params": kwargs.get("params")}) return self._script.pop(0) + async def delete(self, url, **kwargs): + self.calls.append({"url": url, "method": "DELETE"}) + return self._script.pop(0) + @pytest.fixture(autouse=True) def _clear_token_cache(): @@ -75,3 +80,142 @@ async def test_fetch_board_image_names_filters_out_intermediates(monkeypatch): "is_intermediate": "false", "categories": ["general", "user"], } + + +@pytest.mark.asyncio +async def test_fetch_board_video_names_queries_each_board_with_filters(monkeypatch): + """Videos are listed by a query parameter, not a path segment, and get the + same intermediate/category filtering as images (a Wan pipeline writes its + intermediate clips to the board too).""" + stub = _RecordingClient( + [ + _Resp(json_body={"video_names": ["a.mp4", "b.mp4"], "total_count": 2}), + _Resp(json_body={"video_names": ["b.mp4", "c.mp4"], "total_count": 2}), + ] + ) + monkeypatch.setattr(invokeai_client.httpx, "AsyncClient", stub) + + result = await invokeai_client.fetch_board_video_names( + "http://localhost:9090", ["board-1", "none"], None, None + ) + + assert result.names == ["a.mp4", "b.mp4", "c.mp4"] + assert result.api_available is True + assert [call["url"] for call in stub.calls] == [ + "http://localhost:9090/api/v1/videos/names", + "http://localhost:9090/api/v1/videos/names", + ] + assert [call["params"]["board_id"] for call in stub.calls] == ["board-1", "none"] + for call in stub.calls: + assert call["params"]["is_intermediate"] == "false" + assert call["params"]["categories"] == ["general", "user"] + + +@pytest.mark.asyncio +async def test_fetch_board_video_names_tolerates_backend_without_videos(monkeypatch): + """An InvokeAI predating the video API 404s the whole router; that must + not fail the index run, and it must be distinguishable from a board that + genuinely holds no videos — the caller prunes rows for anything absent + from the listing.""" + stub = _RecordingClient([_Resp(status_code=404, text="Not Found")]) + monkeypatch.setattr(invokeai_client.httpx, "AsyncClient", stub) + + result = await invokeai_client.fetch_board_video_names( + "http://localhost:9090", ["board-1"], None, None + ) + + assert result.names == [] + assert result.api_available is False + + +@pytest.mark.asyncio +async def test_fetch_board_video_names_keeps_names_from_earlier_boards(monkeypatch): + """A 404 on a later board must not discard what earlier boards returned. + InvokeAI answers 404 for a board a non-admin cannot read, not only for a + missing video router, so the successful listings are still good data.""" + stub = _RecordingClient( + [ + _Resp(json_body={"video_names": ["a.mp4"], "total_count": 1}), + _Resp(status_code=404, text="Board not found"), + ] + ) + monkeypatch.setattr(invokeai_client.httpx, "AsyncClient", stub) + + result = await invokeai_client.fetch_board_video_names( + "http://localhost:9090", ["board-1", "board-gone"], None, None + ) + + assert result.names == ["a.mp4"] + assert result.api_available is False + + +@pytest.mark.asyncio +async def test_fetch_board_video_names_rejects_unexpected_shape(monkeypatch): + """A bare list (the *images* endpoint's shape) is not silently accepted — + reading no names off a successful response would quietly drop every video + from the album.""" + stub = _RecordingClient([_Resp(json_body=["a.mp4"])]) + monkeypatch.setattr(invokeai_client.httpx, "AsyncClient", stub) + + with pytest.raises(HTTPException) as excinfo: + await invokeai_client.fetch_board_video_names( + "http://localhost:9090", ["board-1"], None, None + ) + assert excinfo.value.status_code == 502 + + +@pytest.mark.asyncio +async def test_delete_video_uses_the_video_endpoint(monkeypatch): + stub = _RecordingClient( + [_Resp(json_body={"deleted_videos": ["a.mp4"], "failed_videos": []})] + ) + monkeypatch.setattr(invokeai_client.httpx, "AsyncClient", stub) + + await invokeai_client.delete_video( + "http://localhost:9090", "a.mp4", None, None + ) + + assert stub.calls == [ + {"url": "http://localhost:9090/api/v1/videos/i/a.mp4", "method": "DELETE"} + ] + + +@pytest.mark.asyncio +async def test_delete_video_raises_when_reported_as_failed(monkeypatch): + """InvokeAI can report a failure *inside* a 200 response. Treating that as + success would drop the local index row while the video stayed on the + board, so it has to surface as an error.""" + stub = _RecordingClient( + [_Resp(json_body={"deleted_videos": [], "failed_videos": ["a.mp4"]})] + ) + monkeypatch.setattr(invokeai_client.httpx, "AsyncClient", stub) + + with pytest.raises(HTTPException) as excinfo: + await invokeai_client.delete_video( + "http://localhost:9090", "a.mp4", None, None + ) + assert excinfo.value.status_code == 502 + + +@pytest.mark.asyncio +async def test_delete_video_tolerates_already_gone(monkeypatch): + """A 404 means InvokeAI has forgotten the video; the caller still needs to + drop its index row, so this returns rather than raising.""" + stub = _RecordingClient([_Resp(status_code=404, text="Video not found")]) + monkeypatch.setattr(invokeai_client.httpx, "AsyncClient", stub) + + await invokeai_client.delete_video( + "http://localhost:9090", "a.mp4", None, None + ) + + +@pytest.mark.asyncio +async def test_delete_video_accepts_a_200_that_is_not_an_object(monkeypatch): + """The success check reads ``failed_videos`` off the body, so a 200 + carrying anything but a JSON object must not blow up: InvokeAI has + already deleted the video at that point, and an exception here would keep + the index row pointing at a file that is gone.""" + stub = _RecordingClient([_Resp(json_body=["a.mp4"])]) + monkeypatch.setattr(invokeai_client.httpx, "AsyncClient", stub) + + await invokeai_client.delete_video("http://localhost:9090", "a.mp4", None, None)