From 349a451500e0fa9fd03f22c285d9f41eb71d1a14 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Mon, 15 Jun 2026 21:20:16 -0400 Subject: [PATCH] feat: surface skipped board images after indexing When an InvokeAI board album indexes fewer images than the board reports (commonly because some names listed in InvokeAI's DB have no file on disk), the discrepancy was invisible: _resolve_board_album_files logged a server-side warning, but nothing reached the UI, so the album silently showed e.g. 240 of 242 images. Surface it as a non-fatal completion notice: - ProgressInfo gains a warning_message, set via a pending-warning channel (set_completion_warning) that survives the per-phase start_operation resets and is folded into the COMPLETED status atomically by complete_operation. Exposed through ProgressResponse. - _resolve_board_album_files now returns the missing count; the background task records "N of M image(s) listed by InvokeAI were not found on disk and were skipped" (and clears it on a clean re-run). - The frontend shows the notice on completion (orange) and persists it under the card's index-status line via a per-album indexWarnings map. Also close a related blind spot: get_image_files' explicit-list branch (used by board albums) now counts and logs sub-threshold/unreadable images, mirroring the directory-scan path instead of dropping them silently. Co-Authored-By: Claude Opus 4.8 (1M context) --- photomap/backend/embeddings.py | 16 +++++- photomap/backend/progress.py | 31 +++++++++++ photomap/backend/routers/index.py | 24 +++++++-- .../static/javascript/album-manager.js | 33 +++++++++++- tests/backend/test_invokeai_board_index.py | 28 ++++++++++ tests/backend/test_progress.py | 41 ++++++++++++++ tests/frontend/album-manager-progress.test.js | 53 +++++++++++++++++++ tests/frontend/invokeai-album-source.test.js | 1 + 8 files changed, 220 insertions(+), 7 deletions(-) diff --git a/photomap/backend/embeddings.py b/photomap/backend/embeddings.py index 1d3e64fe..3918d9ca 100644 --- a/photomap/backend/embeddings.py +++ b/photomap/backend/embeddings.py @@ -534,13 +534,25 @@ def get_image_files( ) elif isinstance(image_paths_or_dir, list): images = [] + skipped_too_small = 0 for p in image_paths_or_dir: if p.is_dir(): images.extend( self.get_image_files_from_directory(p, exts, progress_callback) ) - elif p.suffix.lower() in exts and self._passes_dimension_gate(p): - images.append(p) + elif p.suffix.lower() in exts: + if self._passes_dimension_gate(p): + images.append(p) + else: + skipped_too_small += 1 + # Mirror the directory-scan path: an explicit file list (e.g. an + # InvokeAI board album) silently dropped sub-threshold/unreadable + # images, which made indexed-vs-source count mismatches a mystery. + if skipped_too_small: + logger.info( + f"Skipped {skipped_too_small} image(s) under " + f"{self.min_image_dimension}px in either dimension." + ) else: raise ValueError("Input must be a Path object or a list of Paths.") return images diff --git a/photomap/backend/progress.py b/photomap/backend/progress.py index 2fb17494..3da4ebc2 100644 --- a/photomap/backend/progress.py +++ b/photomap/backend/progress.py @@ -39,6 +39,10 @@ class ProgressInfo: total_images: int start_time: float error_message: str | None = None + # Non-fatal notice shown alongside a COMPLETED status (e.g. "2 images + # listed by InvokeAI were not found on disk and were skipped"). Distinct + # from ``error_message``, which marks the run as failed. + warning_message: str | None = None @property def progress_percentage(self) -> float: @@ -77,6 +81,12 @@ def __init__(self): # pass, instead of running to completion only to have the status # flipped to ERROR after. self._cancel_requested: set[str] = set() + # Pending non-fatal notices, set before/while a run is in flight and + # folded into the ProgressInfo when the run completes (see + # ``complete_operation``). Kept separate from ``_progress`` because the + # per-phase ``start_operation`` calls recreate ProgressInfo and would + # otherwise wipe a warning recorded earlier in the same run. + self._completion_warnings: dict[str, str] = {} self._lock = threading.Lock() def start_operation(self, album_key: str, total_images: int, operation_type: str): @@ -168,6 +178,21 @@ def set_error(self, album_key: str, error_message: str): progress.status = IndexStatus.ERROR progress.error_message = error_message + def set_completion_warning(self, album_key: str, message: str | None) -> None: + """Record (or clear) a non-fatal notice to attach when the run completes. + + Called before indexing starts — e.g. once a board album's missing-on-disk + count is known — so it survives the per-phase ``start_operation`` resets + and is folded in atomically by ``complete_operation``. A falsy + ``message`` clears any pending notice so a clean re-run doesn't inherit + a stale one. + """ + with self._lock: + if message: + self._completion_warnings[album_key] = message + else: + self._completion_warnings.pop(album_key, None) + def get_progress(self, album_key: str) -> ProgressInfo | None: """Get progress info for an album.""" with self._lock: @@ -178,6 +203,7 @@ def remove_progress(self, album_key: str): with self._lock: self._progress.pop(album_key, None) self._cancel_requested.discard(album_key) + self._completion_warnings.pop(album_key, None) def request_cancel(self, album_key: str) -> None: """Signal the indexing loop to stop on its next batch boundary. @@ -217,6 +243,11 @@ def complete_operation( progress.status = IndexStatus.COMPLETED progress.current_step = message progress.images_processed = progress.total_images + # Fold in (and consume) any pending non-fatal notice so it + # lands atomically with the COMPLETED status the poller reads. + progress.warning_message = self._completion_warnings.pop( + album_key, None + ) # Global instance diff --git a/photomap/backend/routers/index.py b/photomap/backend/routers/index.py index a56a2e4c..e4ed35c4 100644 --- a/photomap/backend/routers/index.py +++ b/photomap/backend/routers/index.py @@ -42,6 +42,7 @@ class ProgressResponse(BaseModel): elapsed_time: float estimated_time_remaining: float | None error_message: str | None = None + warning_message: str | None = None class UpdateIndexRequest(BaseModel): @@ -188,6 +189,7 @@ async def get_index_progress( elapsed_time=progress.elapsed_time, estimated_time_remaining=progress.estimated_time_remaining, error_message=progress.error_message, + warning_message=progress.warning_message, ) except HTTPException: @@ -500,7 +502,7 @@ 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) -> list[Path]: +async def _resolve_board_album_files(album_config) -> tuple[list[Path], int]: """Resolve an InvokeAI-board album's images to local file paths. Fetches the selected boards' image names from the InvokeAI API and maps @@ -508,6 +510,10 @@ async def _resolve_board_album_files(album_config) -> list[Path]: 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". + + 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). """ names = await invokeai_client.fetch_board_image_names( album_config.invokeai_url, @@ -531,7 +537,7 @@ async def _resolve_board_album_files(album_config) -> list[Path]: logger.warning( f"{missing} of {len(paths)} board images not found under {images_dir}; skipping them." ) - return existing + return existing, missing # Background Tasks @@ -540,7 +546,7 @@ async def _update_index_background_async(album_key: str, album_config): try: if getattr(album_config, "source_type", "directory") == "invokeai_board": try: - image_paths = await _resolve_board_album_files(album_config) + image_paths, missing = await _resolve_board_album_files(album_config) except HTTPException as e: progress_tracker.set_error( album_key, @@ -553,6 +559,18 @@ async def _update_index_background_async(album_key: str, album_config): album_key, "Selected InvokeAI board(s) contain no images" ) return + # Surface the gallery-vs-indexed discrepancy (always set so a clean + # re-run clears any stale notice). Folded into the COMPLETED status + # by progress_tracker.complete_operation. + if missing: + total = len(image_paths) + missing + progress_tracker.set_completion_warning( + album_key, + f"{missing} of {total} image(s) listed by InvokeAI were not " + f"found on disk and were skipped.", + ) + else: + progress_tracker.set_completion_warning(album_key, None) else: image_paths = [Path(path) for path in album_config.image_paths] index_path = Path(album_config.index) diff --git a/photomap/frontend/static/javascript/album-manager.js b/photomap/frontend/static/javascript/album-manager.js index c9eed328..a71e863a 100644 --- a/photomap/frontend/static/javascript/album-manager.js +++ b/photomap/frontend/static/javascript/album-manager.js @@ -125,6 +125,10 @@ export class AlbumManager { this.progressPollers = new Map(); this.isSetupMode = false; this.autoIndexingAlbums = new Set(); + // Per-album non-fatal indexing notice (e.g. board images missing on disk), + // captured from the completing progress poll so it can persist on the card + // status after the transient progress UI is gone. + this.indexWarnings = new Map(); this.initializeEventListeners(); } @@ -994,6 +998,7 @@ export class AlbumManager { status.textContent = `Index updated ${modDate} (${fileCount} images)`; status.style.color = "green"; createBtn.textContent = "Update Index"; + this._appendIndexWarningNote(status, album.key); } else { status.className = "index-status"; status.textContent = "No index present"; @@ -1008,6 +1013,22 @@ export class AlbumManager { } } + // Append the most recent non-fatal indexing notice (if any) under the card's + // index-status line, so a "N images skipped" warning persists after the + // transient progress UI is gone. + _appendIndexWarningNote(statusElement, albumKey) { + const warning = this.indexWarnings.get(albumKey); + if (!warning) { + return; + } + const note = document.createElement("div"); + note.className = "index-status-warning"; + note.textContent = warning; + note.style.color = "#ff9800"; + note.style.fontSize = "0.9em"; + statusElement.appendChild(note); + } + attachCardEventListeners(card, cardElement, album) { // Edit button card.querySelector(".edit-album-btn").addEventListener("click", () => { @@ -1593,6 +1614,8 @@ export class AlbumManager { console.log(`Already polling progress for album: ${albumKey}`); return; } + // Drop any notice from a previous run before this one can record its own. + this.indexWarnings.delete(albumKey); const interval = setInterval(async () => { try { @@ -1606,6 +1629,7 @@ export class AlbumManager { this.progressPollers.delete(albumKey); if (progress.status === "completed") { + this.indexWarnings.set(albumKey, progress.warning_message || null); await this.handleIndexingCompletion(albumKey, liveCard); } @@ -1728,8 +1752,13 @@ export class AlbumManager { updateProgressStatus(status, progress, estimatedTime) { if (progress.status === "completed") { status.className = AlbumManager.STATUS_CLASSES.COMPLETED; - status.textContent = "Indexing completed successfully"; - status.style.color = "green"; + if (progress.warning_message) { + status.textContent = `Indexing completed — ${progress.warning_message}`; + status.style.color = "#ff9800"; // Orange: completed, but with a caveat + } else { + status.textContent = "Indexing completed successfully"; + status.style.color = "green"; + } estimatedTime.textContent = ""; } else if (progress.status === "error") { status.className = AlbumManager.STATUS_CLASSES.ERROR; diff --git a/tests/backend/test_invokeai_board_index.py b/tests/backend/test_invokeai_board_index.py index 9335f6ce..1b6a50c3 100644 --- a/tests/backend/test_invokeai_board_index.py +++ b/tests/backend/test_invokeai_board_index.py @@ -108,6 +108,34 @@ def test_board_album_index_contains_board_images(client, board_album): ) +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 + InvokeAI gallery shows a higher count than the album indexes).""" + # 4 real images + 1 ghost that has no file under outputs/images. + ghost = f"{uuid.uuid4()}.png" + board_album["boards"]["b1"].append(ghost) + + _build_index(client) + + progress = client.get(f"/index_progress/{ALBUM_KEY}").json() + assert progress["status"] == "completed" + assert progress["warning_message"] + assert "1 of 5" in progress["warning_message"] + # The real images still index; only the ghost is dropped. + metadata = client.get(f"/index_metadata/{ALBUM_KEY}").json() + assert metadata["filename_count"] == 4 + + +def test_complete_board_index_has_no_warning(client, board_album): + """When every listed image exists, the completed run carries no warning.""" + _build_index(client) + + progress = client.get(f"/index_progress/{ALBUM_KEY}").json() + assert progress["status"] == "completed" + assert progress["warning_message"] is None + + def test_board_membership_changes_flow_through_update(client, board_album): _build_index(client) diff --git a/tests/backend/test_progress.py b/tests/backend/test_progress.py index 69ab329e..8920d554 100644 --- a/tests/backend/test_progress.py +++ b/tests/backend/test_progress.py @@ -95,3 +95,44 @@ def test_is_running_includes_downloading(): # A download is an active operation, so a duplicate index must stay blocked. assert tracker.is_running("alb") is True + + +def test_completion_warning_folds_into_completed_status(): + tracker = ProgressTracker() + # Recorded before the run starts (as the board resolver does). + tracker.set_completion_warning("alb", "2 images skipped") + tracker.start_operation("alb", total_images=5, operation_type="indexing") + + tracker.complete_operation("alb", "done") + + progress = tracker.get_progress("alb") + assert progress is not None + assert progress.status is IndexStatus.COMPLETED + assert progress.warning_message == "2 images skipped" + + +def test_completion_warning_survives_phase_restarts_then_clears_on_reuse(): + tracker = ProgressTracker() + tracker.set_completion_warning("alb", "2 images skipped") + # The per-phase scanning -> indexing -> mapping restarts must not wipe it. + tracker.start_operation("alb", 0, "scanning") + tracker.start_operation("alb", 5, "indexing") + tracker.start_operation("alb", 5, "mapping") + tracker.complete_operation("alb") + assert tracker.get_progress("alb").warning_message == "2 images skipped" + + # The notice was consumed at completion, so a clean re-run doesn't inherit it. + tracker.start_operation("alb", 5, "indexing") + tracker.complete_operation("alb") + assert tracker.get_progress("alb").warning_message is None + + +def test_set_completion_warning_none_clears_pending(): + tracker = ProgressTracker() + tracker.set_completion_warning("alb", "stale") + tracker.set_completion_warning("alb", None) + tracker.start_operation("alb", 5, "indexing") + + tracker.complete_operation("alb") + + assert tracker.get_progress("alb").warning_message is None diff --git a/tests/frontend/album-manager-progress.test.js b/tests/frontend/album-manager-progress.test.js index c83faa8b..153d2ea6 100644 --- a/tests/frontend/album-manager-progress.test.js +++ b/tests/frontend/album-manager-progress.test.js @@ -109,3 +109,56 @@ describe("AlbumManager downloading phase", () => { expect(status.textContent).not.toContain("(1024/4096)"); }); }); + +describe("AlbumManager completed status", () => { + test("shows a plain success message when there is no warning", () => { + const { status, estimatedTime } = makeElements(); + + callUpdate(status, { status: "completed" }, estimatedTime); + + expect(status.textContent).toBe("Indexing completed successfully"); + expect(status.className).toBe(AlbumManager.STATUS_CLASSES.COMPLETED); + expect(status.style.color).toBe("green"); + }); + + test("surfaces a non-fatal warning_message alongside completion", () => { + const { status, estimatedTime } = makeElements(); + + callUpdate( + status, + { + status: "completed", + warning_message: "2 of 242 image(s) listed by InvokeAI were not found on disk and were skipped.", + }, + estimatedTime + ); + + expect(status.textContent).toContain("Indexing completed"); + expect(status.textContent).toContain("not found on disk"); + expect(status.className).toBe(AlbumManager.STATUS_CLASSES.COMPLETED); + // Completion-with-a-caveat is coloured differently from a clean success. + expect(status.style.color).not.toBe("green"); + }); +}); + +describe("AlbumManager persistent index warning note", () => { + test("appends a note element when a warning is stored for the album", () => { + const manager = { indexWarnings: new Map([["alb", "2 images skipped"]]) }; + const status = document.createElement("span"); + + AlbumManager.prototype._appendIndexWarningNote.call(manager, status, "alb"); + + const note = status.querySelector(".index-status-warning"); + expect(note).not.toBeNull(); + expect(note.textContent).toBe("2 images skipped"); + }); + + test("does nothing when no warning is stored", () => { + const manager = { indexWarnings: new Map() }; + const status = document.createElement("span"); + + AlbumManager.prototype._appendIndexWarningNote.call(manager, status, "alb"); + + expect(status.querySelector(".index-status-warning")).toBeNull(); + }); +}); diff --git a/tests/frontend/invokeai-album-source.test.js b/tests/frontend/invokeai-album-source.test.js index 2e30feaf..fe3a0654 100644 --- a/tests/frontend/invokeai-album-source.test.js +++ b/tests/frontend/invokeai-album-source.test.js @@ -325,6 +325,7 @@ describe("AlbumManager indexing-progress robustness", () => { const detachedCard = document.createElement("div"); const manager = { progressPollers: new Map(), + indexWarnings: new Map(), _liveCardFor: AlbumManager.prototype._liveCardFor, updateProgress: jest.fn(), handleIndexingCompletion: jest.fn(),