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 3354082a..f47c5b1a 100644 --- a/photomap/frontend/static/javascript/album-manager.js +++ b/photomap/frontend/static/javascript/album-manager.js @@ -126,6 +126,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(); } @@ -1025,6 +1029,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"; @@ -1039,6 +1044,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", () => { @@ -1638,6 +1659,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 { @@ -1651,6 +1674,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); } @@ -1773,8 +1797,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 7562572b..6ba13db9 100644 --- a/tests/frontend/invokeai-album-source.test.js +++ b/tests/frontend/invokeai-album-source.test.js @@ -356,6 +356,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(),