diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf4ddc1..fe47939 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,7 +35,8 @@ jobs: - name: Install system deps run: | sudo apt-get update - sudo apt-get install -y libgl1 libglib2.0-0 + # Base GL/GLib plus EGL for headless PyQt6 usage + sudo apt-get install -y libgl1 libglib2.0-0 libegl1 - name: Cache pip uses: actions/cache@v4 @@ -59,6 +60,8 @@ jobs: - name: Run Tests env: PYTHONWARNINGS: default + # Ensure Qt runs without an X server + QT_QPA_PLATFORM: offscreen run: | pytest -q --cov=src --cov-report=xml:coverage.xml --cov-report=term # Single coverage dataset only; no secondary copying. @@ -77,8 +80,9 @@ jobs: with: name: pytest-artifacts path: | - .pytest_cache + .pytest_cache/ ./**/pytest-*.log + if-no-files-found: ignore retention-days: 5 diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 9a9b629..b0293a3 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -27,6 +27,30 @@ The application is structured into two main packages: `core` and `ui`. - **`app_controller.py`**: The controller that mediates between the UI and the `core` logic. It handles user actions, calls the appropriate `core` services, and updates the UI. - **`app_state.py`**: Holds the application's runtime state, including caches and loaded data. This object is shared across the application. - **`worker_manager.py`**: Manages all background threads and workers, decoupling the UI from long-running tasks. + - **Controller Layer (Encapsulation Refactor)**: Non-trivial UI behaviors previously embedded in `MainWindow` have been extracted into focused controllers under `src/ui/controllers/`: + - `navigation_controller.py`: Linear & group-aware navigation (honors skip-deleted logic, smart up/down traversal). Consumes a minimal protocol for selection & model interrogation. + - `hotkey_controller.py`: Central mapping of key events to actions (allows headless tests to exercise hotkey dispatch logic). + - `similarity_controller.py`: Orchestrates similarity analysis workflow (start, embeddings, clustering) AND (post-refactor) prepares cluster structures via `prepare_clusters()` returning pure data used by the view. Sorting strategies (Default / Time / Similarity then Time) live here; PCA fallback rationale documented inline. Uses only a protocol subset of `AppState` (see `AppStateSimilarityView`) for loose coupling. + - `preview_controller.py`: Handles preview image loading / refresh separation from navigation triggers. + - `metadata_controller.py`: Updates metadata sidebar without polluting MainWindow event handlers. + - `filter_controller.py`: Applies user-entered filter text / rating filters to proxy model. + - `selection_controller.py`: Shared selection operations & multi-select semantics. + - `deletion_mark_controller.py`: Non-destructive mark/unmark & presentation (text color/blur). Distinguished from actual deletion for clarity & test isolation. + - `file_deletion_controller.py`: Destructive operations (move to trash), reverse-order removal, prunes empty headers, restores deterministic selection. + + Extension Pattern: + 1. Identify a cohesive behavior cluster in `MainWindow` (heuristics, branching logic, side-effect orchestration). + 2. Define a minimal Protocol capturing only what the controller needs (attributes + methods). Avoid passing full MainWindow if possible. + 3. Implement controller with pure helpers; return data structures instead of mutating widgets directly where feasible. + 4. Add targeted tests for new controller focusing on logic not easily covered via GUI. + 5. Replace in-place logic with controller delegation. Remove obsolete helpers after tests pass. + 6. Document rationale (fallbacks, sentinels like `date_obj.max`) inline for future maintainers. + + Benefits: + - Smaller `main_window.py` surface area. + - Faster unit tests (controllers testable without QApplication event loop). + - Clear separation of destructive vs non-destructive operations (mark vs delete). + - Easier future feature toggles or re-use in alternate front-ends. - **`dialog_manager.py`**: Manages the creation and display of all dialog boxes. `show_about_dialog(block: bool = True)` supports a non-blocking mode (`block=False`) used by tests; prefer blocking mode in production UI code. - **`menu_manager.py`**: Manages the main menu bar and its actions. - **`left_panel.py`**, **`metadata_sidebar.py`**, **`advanced_image_viewer.py`**: Reusable UI components. @@ -174,6 +198,17 @@ When you: Update the relevant sections instead of appending ad hoc notes at the bottom. +### 8.1 Recent Refactor Summary (Encapsulation Phase) + +Refactors completed: + - Extracted navigation & hotkey handling (legacy behaviors restored: Ctrl includes deleted, cyclic horizontal navigation, smart vertical grouping). + - Split deletion responsibilities: presentation/marking vs filesystem deletion. + - Moved clustering grouping & sorting to `SimilarityController.prepare_clusters()`; `MainWindow` now only renders structure. + - Introduced protocol-driven design (e.g., `AppStateSimilarityView`) for type clarity and decoupling. + - Added deterministic cluster sorting strategies with PCA + timestamp fallback (see docstring in `ClusterUtils.sort_clusters_by_similarity_time`). + +When adding new controllers, follow the listed Extension Pattern to maintain consistency. + ## 9. Common Pitfalls | Scenario | Recommended Action | diff --git a/src/core/caching/exif_cache.py b/src/core/caching/exif_cache.py index 99af98f..790cdae 100644 --- a/src/core/caching/exif_cache.py +++ b/src/core/caching/exif_cache.py @@ -1,7 +1,7 @@ import diskcache import os import logging -import time # Added for startup timing +import time from typing import Optional, Dict, Any # Import the settings functions to get the cache size limit diff --git a/src/core/caching/preview_cache.py b/src/core/caching/preview_cache.py index 2dec41b..0224718 100644 --- a/src/core/caching/preview_cache.py +++ b/src/core/caching/preview_cache.py @@ -1,7 +1,7 @@ import diskcache import os import logging -import time # Added for startup timing +import time from PIL import Image from typing import Optional, Tuple diff --git a/src/core/caching/rating_cache.py b/src/core/caching/rating_cache.py index 4296dd7..df275f6 100644 --- a/src/core/caching/rating_cache.py +++ b/src/core/caching/rating_cache.py @@ -1,7 +1,7 @@ import diskcache import os import logging -import time # Added for startup timing +import time from typing import Optional from src.core.app_settings import DEFAULT_RATING_CACHE_SIZE_LIMIT_MB diff --git a/src/core/caching/thumbnail_cache.py b/src/core/caching/thumbnail_cache.py index 234700a..1d0088e 100644 --- a/src/core/caching/thumbnail_cache.py +++ b/src/core/caching/thumbnail_cache.py @@ -1,7 +1,7 @@ import diskcache import os import logging -import time # Added for startup timing +import time from PIL import Image from typing import Optional, Tuple from src.core.app_settings import ( diff --git a/src/core/file_scanner.py b/src/core/file_scanner.py index 50292f9..d87a196 100644 --- a/src/core/file_scanner.py +++ b/src/core/file_scanner.py @@ -141,7 +141,7 @@ def scan_directory( ) continue - is_blurred = None # Initialize as None + is_blurred = None if perform_blur_detection: # Perform blur detection # Pass the apply_auto_edits flag to control RAW preview generation for blur detection diff --git a/src/core/image_processing/raw_image_processor.py b/src/core/image_processing/raw_image_processor.py index 09feefc..ad407ab 100644 --- a/src/core/image_processing/raw_image_processor.py +++ b/src/core/image_processing/raw_image_processor.py @@ -5,7 +5,7 @@ import logging import time from typing import Optional, Set -from PIL import ImageEnhance # Added for brightness adjustment on PIL images +from PIL import ImageEnhance from src.core.app_settings import ( RAW_AUTO_EDIT_BRIGHTNESS_STANDARD, RAW_AUTO_EDIT_BRIGHTNESS_ENHANCED, diff --git a/src/main.py b/src/main.py index 9c65b9d..3c6b37d 100644 --- a/src/main.py +++ b/src/main.py @@ -1,8 +1,8 @@ import pyexiv2 # noqa: F401 # This must be the first import or else it will cause a silent crash on windows import sys import os -import logging # Added for startup logging -import time # Added for startup timing +import logging +import time import argparse import traceback # For global exception handler from PyQt6.QtWidgets import ( diff --git a/src/ui/app_controller.py b/src/ui/app_controller.py index af78f26..94df867 100644 --- a/src/ui/app_controller.py +++ b/src/ui/app_controller.py @@ -312,7 +312,6 @@ def start_auto_rotation_analysis(self): self.main_window.show_loading_overlay("Starting rotation analysis...") self.main_window.menu_manager.auto_rotate_action.setEnabled(False) - # Initialize the rotation suggestions storage self.main_window.rotation_suggestions.clear() image_paths = [fd["path"] for fd in self.app_state.image_files_data] diff --git a/src/ui/controllers/deletion_mark_controller.py b/src/ui/controllers/deletion_mark_controller.py new file mode 100644 index 0000000..65dcf9a --- /dev/null +++ b/src/ui/controllers/deletion_mark_controller.py @@ -0,0 +1,213 @@ +# Renamed from deletion_controller.py to align with class name DeletionMarkController +from __future__ import annotations +from typing import Optional, List, Callable, Iterable, Tuple +from PyQt6.QtGui import QStandardItem, QColor +from PyQt6.QtWidgets import QApplication +from PyQt6.QtCore import Qt + +from src.ui.helpers.deletion_utils import build_presentation + + +class DeletionMarkController: + """Encapsulates *non-destructive* deletion mark & blur presentation logic. + + Split from FileDeletionController (destructive / filesystem). This class only: + * toggles mark state in AppState + * updates in-model QStandardItem presentation (text + color + blur flag) + Keeping destructive actions separate lowers risk and keeps tests fast & pure. + """ + + ORANGE = QColor("#FFB366") + + def __init__(self, app_state, is_marked_func): + self.app_state = app_state + self._is_marked_func = is_marked_func + + # --- Presentation helpers --- + def apply_presentation( + self, item: QStandardItem, file_path: str, is_blurred: Optional[bool] + ): + pres = build_presentation( + basename=item.text().split(" ")[0] + if item.text() + else file_path.split("\\")[-1], + is_marked=self._is_marked_func(file_path), + is_blurred=is_blurred, + ) + if pres.is_marked: + item.setForeground(self.ORANGE) + elif pres.is_blurred: + item.setForeground(QColor(Qt.GlobalColor.red)) + else: + item.setForeground(QApplication.palette().text().color()) + item.setText(pres.text) + + # --- Mark / Unmark operations --- + def toggle_mark(self, file_path: str): + if self._is_marked_func(file_path): + self.app_state.unmark_for_deletion(file_path) + else: + self.app_state.mark_for_deletion(file_path) + + def mark(self, file_path: str): + if not self._is_marked_func(file_path): + self.app_state.mark_for_deletion(file_path) + + def unmark(self, file_path: str): + if self._is_marked_func(file_path): + self.app_state.unmark_for_deletion(file_path) + + def mark_others(self, keep_path: str, paths: List[str]) -> int: + count = 0 + for p in paths: + if p != keep_path and not self._is_marked_func(p): + self.app_state.mark_for_deletion(p) + count += 1 + return count + + def unmark_others(self, keep_path: str, paths: List[str]) -> int: + count = 0 + for p in paths: + if p != keep_path and self._is_marked_func(p): + self.app_state.unmark_for_deletion(p) + count += 1 + return count + + def clear_all(self) -> int: + marked = list(self.app_state.get_marked_files()) + self.app_state.clear_all_deletion_marks() + return len(marked) + + # --- Batch / UI integrated operations (delegated from MainWindow) --- + def _resolve_item( + self, + file_path: str, + find_proxy_index: Callable[[str], object], + file_system_model, + proxy_model, + ) -> Tuple[Optional[QStandardItem], Optional[bool]]: + proxy_idx = find_proxy_index(file_path) + if proxy_idx and proxy_idx.isValid(): # type: ignore[attr-defined] + source_idx = proxy_model.mapToSource(proxy_idx) + item = file_system_model.itemFromIndex(source_idx) + if item: + data = item.data(Qt.ItemDataRole.UserRole) + if isinstance(data, dict): + return item, data.get("is_blurred") + return item, None + return None, None + + def _update_item_presentation( + self, + item: Optional[QStandardItem], + file_path: str, + is_blurred: Optional[bool], + ): + if item: + self.apply_presentation(item, file_path, is_blurred) + + def toggle_paths( + self, + paths: Iterable[str], + find_proxy_index: Callable[[str], object], + file_system_model, + proxy_model, + ) -> int: + count = 0 + for p in paths: + if self._is_marked_func(p): + self.app_state.unmark_for_deletion(p) + else: + self.app_state.mark_for_deletion(p) + item, is_blurred = self._resolve_item( + p, find_proxy_index, file_system_model, proxy_model + ) + self._update_item_presentation(item, p, is_blurred) + count += 1 + return count + + def mark_others_in_collection( + self, + keep_path: str, + collection_paths: Iterable[str], + find_proxy_index: Callable[[str], object], + file_system_model, + proxy_model, + ) -> int: + count = 0 + for p in collection_paths: + if p == keep_path: + continue + if not self._is_marked_func(p): + self.app_state.mark_for_deletion(p) + count += 1 + item, is_blurred = self._resolve_item( + p, find_proxy_index, file_system_model, proxy_model + ) + self._update_item_presentation(item, p, is_blurred) + return count + + def unmark_others_in_collection( + self, + keep_path: str, + collection_paths: Iterable[str], + find_proxy_index: Callable[[str], object], + file_system_model, + proxy_model, + ) -> int: + count = 0 + for p in collection_paths: + if p == keep_path: + continue + if self._is_marked_func(p): + self.app_state.unmark_for_deletion(p) + count += 1 + item, is_blurred = self._resolve_item( + p, find_proxy_index, file_system_model, proxy_model + ) + self._update_item_presentation(item, p, is_blurred) + return count + + def clear_all_and_update( + self, + find_proxy_index: Callable[[str], object], + file_system_model, + proxy_model, + ) -> int: + marked_files = list(self.app_state.get_marked_files()) + if not marked_files: + return 0 + self.app_state.clear_all_deletion_marks() + for p in marked_files: + item, is_blurred = self._resolve_item( + p, find_proxy_index, file_system_model, proxy_model + ) + self._update_item_presentation(item, p, is_blurred) + return len(marked_files) + + def update_blur_status( + self, + image_path: str, + is_blurred: bool, + find_proxy_index: Callable[[str], object], + file_system_model, + proxy_model, + active_view_getter: Callable[[], object], + selection_changed_callback: Callable[[], None], + ): + item, _prev_blur = self._resolve_item( + image_path, find_proxy_index, file_system_model, proxy_model + ) + if item: + data = item.data(Qt.ItemDataRole.UserRole) + if isinstance(data, dict): + data["is_blurred"] = is_blurred + item.setData(data, Qt.ItemDataRole.UserRole) + self.apply_presentation(item, image_path, is_blurred) + active_view = active_view_getter() + if active_view and active_view.currentIndex().isValid(): # type: ignore[attr-defined] + cur_proxy = active_view.currentIndex() # type: ignore[attr-defined] + cur_source = proxy_model.mapToSource(cur_proxy) + selected_item = file_system_model.itemFromIndex(cur_source) + if selected_item == item: + selection_changed_callback() diff --git a/src/ui/controllers/file_deletion_controller.py b/src/ui/controllers/file_deletion_controller.py new file mode 100644 index 0000000..f218732 --- /dev/null +++ b/src/ui/controllers/file_deletion_controller.py @@ -0,0 +1,237 @@ +from __future__ import annotations +from typing import List +import os +import logging +from PyQt6.QtCore import QModelIndex, QItemSelection, QTimer, QItemSelectionModel +from PyQt6.QtWidgets import QAbstractItemView + +logger = logging.getLogger(__name__) + + +class FileDeletionContextProtocol: + """Protocol-like duck interface used by FileDeletionController. + + We intentionally avoid importing typing.Protocol to keep this lightweight. + The provided context (MainWindow) must supply these attributes/methods. + """ + + # Attributes expected: + file_system_model: object + proxy_model: object + app_controller: object + app_state: object + dialog_manager: object + advanced_image_viewer: object + show_folders_mode: bool + group_by_similarity_mode: bool + + # Methods expected: + def _get_active_file_view(self): ... # returns QAbstractItemView or None + def _get_selected_file_paths_from_view(self) -> List[str]: ... + def _get_all_visible_image_paths(self) -> List[str]: ... + def _find_proxy_index_for_path(self, path: str): ... + def _handle_file_selection_changed(self, override_selected_paths=None): ... + def _update_image_info_label(self): ... + def statusBar(self): ... + + +class FileDeletionController: + """Encapsulates complex multi-step deletion logic from MainWindow. + + Handles: + - Determining which paths to delete (focused vs selection) + - Confirm dialog + - Removing items from the model in correct order + - Cleaning up empty group headers + - Restoring a sensible selection/focus afterwards + """ + + def __init__(self, ctx: FileDeletionContextProtocol): + self.ctx = ctx + # Stateful flags mirrored from previous MainWindow logic + self.original_selection_paths: List[str] = [] + self.was_focused_delete: bool = False + + # Public API + def move_current_image_to_trash(self): + view = self.ctx._get_active_file_view() + if not view: + return + + logger.debug("Initiating file deletion process (controller).") + + self.original_selection_paths = self.ctx._get_selected_file_paths_from_view() + visible_paths_before_delete = self.ctx._get_all_visible_image_paths() + focused_path = self.ctx.advanced_image_viewer.get_focused_image_path_if_any() + + if focused_path: + target_paths = [focused_path] + self.was_focused_delete = True + logger.debug("Deleting focused image: %s", os.path.basename(focused_path)) + else: + target_paths = self.original_selection_paths + self.was_focused_delete = False + + if not target_paths: + self.ctx.statusBar().showMessage("No image(s) selected to delete.", 3000) + return + + if not self.ctx.dialog_manager.show_confirm_delete_dialog(target_paths): + return + + source_indices = self._collect_source_indices(target_paths) + deleted_count, parent_items = self._delete_indices(source_indices) + + if deleted_count > 0: + self._prune_empty_parent_groups(parent_items) + self.ctx.statusBar().showMessage( + f"{deleted_count} image(s) moved to trash.", 5000 + ) + view.selectionModel().clearSelection() + visible_after = self.ctx._get_all_visible_image_paths() + self._restore_selection_after_delete( + visible_paths_before_delete, visible_after, target_paths, view + ) + self.ctx._update_image_info_label() + elif deleted_count == 0 and len(self.original_selection_paths) > 0: + self.ctx.statusBar().showMessage( + "No valid image files were deleted from selection.", 3000 + ) + + # --- Internal helpers --- + def _collect_source_indices(self, paths: List[str]): + indices = [] + for p in paths: + proxy = self.ctx._find_proxy_index_for_path(p) + if proxy.isValid(): # type: ignore[attr-defined] + src = self.ctx.proxy_model.mapToSource(proxy) + if src.isValid() and src not in indices: # type: ignore[attr-defined] + indices.append(src) + indices.sort( + key=lambda idx: (idx.parent().internalId(), idx.row()), reverse=True + ) # type: ignore[attr-defined] + return indices + + def _delete_indices(self, source_indices): + deleted_count = 0 + parent_items = [] + for src_idx in source_indices: + item = self.ctx.file_system_model.itemFromIndex(src_idx) + if not item: + continue + data = item.data(0x0100) # Qt.UserRole + if not isinstance(data, dict) or "path" not in data: + continue + path = data["path"] + if not os.path.isfile(path): + continue + try: + self.ctx.app_controller.move_to_trash(path) + self.ctx.app_state.remove_data_for_path(path) + parent_idx = src_idx.parent() + parent_item = ( + self.ctx.file_system_model.itemFromIndex(parent_idx) + if parent_idx.isValid() + else self.ctx.file_system_model.invisibleRootItem() + ) + if parent_item: + parent_item.takeRow(src_idx.row()) + if parent_item not in parent_items: + parent_items.append(parent_item) + deleted_count += 1 + except Exception as e: # pragma: no cover (rare path) + logger.error("Error moving file to trash: %s", e, exc_info=True) + self.ctx.dialog_manager.show_error_dialog( + "Delete Error", f"Could not move {os.path.basename(path)} to trash." + ) + return deleted_count, parent_items + + def _prune_empty_parent_groups(self, parents): + for parent_item_candidate in list(parents): + if parent_item_candidate == self.ctx.file_system_model.invisibleRootItem(): + continue + if parent_item_candidate.model() is None: + continue + is_eligible = False + user_data = parent_item_candidate.data(0x0100) + if isinstance(user_data, str): + if user_data.startswith("cluster_header_") or user_data.startswith( + "date_header_" + ): + is_eligible = True + elif ( + self.ctx.show_folders_mode + and not self.ctx.group_by_similarity_mode + and os.path.isdir(user_data) + ): + is_eligible = True + if is_eligible and parent_item_candidate.rowCount() == 0: + row = parent_item_candidate.row() + actual_parent_qitem = parent_item_candidate.parent() + if actual_parent_qitem is None: + parent_to_operate = self.ctx.file_system_model.invisibleRootItem() + else: + parent_to_operate = actual_parent_qitem + parent_to_operate.takeRow(row) + + def _restore_selection_after_delete( + self, visible_before, visible_after, deleted_paths, view: QAbstractItemView + ): + handled = False + if self.was_focused_delete: + remaining = [p for p in self.original_selection_paths if p in visible_after] + if remaining: + self.ctx._handle_file_selection_changed( + override_selected_paths=remaining + ) + selection = QItemSelection() + first_idx = QModelIndex() + for p in remaining: + proxy = self.ctx._find_proxy_index_for_path(p) + if proxy.isValid(): # type: ignore[attr-defined] + selection.select(proxy, proxy) + if not first_idx.isValid(): + first_idx = proxy + if not selection.isEmpty(): + sel_model = view.selectionModel() + sel_model.blockSignals(True) + if first_idx.isValid(): + view.setCurrentIndex(first_idx) + sel_model.select( + selection, QItemSelectionModel.SelectionFlag.ClearAndSelect + ) + sel_model.blockSignals(False) + if first_idx.isValid(): + view.scrollTo( + first_idx, QAbstractItemView.ScrollHint.EnsureVisible + ) + QTimer.singleShot(0, self.ctx._handle_file_selection_changed) + handled = True + if handled: + return + if not visible_after: + self.ctx.advanced_image_viewer.clear() + self.ctx.advanced_image_viewer.setText("No images left to display.") + self.ctx.statusBar().showMessage("No images left or visible.") + return + first_deleted_idx = -1 + if visible_before and deleted_paths: + try: + first_deleted_idx = visible_before.index(deleted_paths[0]) + except ValueError: + first_deleted_idx = 0 + elif visible_before: + first_deleted_idx = 0 + target_idx = min(first_deleted_idx, len(visible_after) - 1) + target_idx = max(0, target_idx) + proxy_target = self.ctx._find_proxy_index_for_path(visible_after[target_idx]) + if proxy_target.isValid(): # type: ignore[attr-defined] + view.setCurrentIndex(proxy_target) + view.selectionModel().select( + proxy_target, QItemSelectionModel.SelectionFlag.ClearAndSelect + ) + view.scrollTo(proxy_target, QAbstractItemView.ScrollHint.EnsureVisible) + QTimer.singleShot(0, self.ctx._handle_file_selection_changed) + else: + self.ctx.advanced_image_viewer.clear() + self.ctx.advanced_image_viewer.setText("No valid image to select.") diff --git a/src/ui/controllers/filter_controller.py b/src/ui/controllers/filter_controller.py new file mode 100644 index 0000000..ee87cd4 --- /dev/null +++ b/src/ui/controllers/filter_controller.py @@ -0,0 +1,126 @@ +from __future__ import annotations +from dataclasses import dataclass +from typing import Protocol, Optional + +from PyQt6.QtCore import QSortFilterProxyModel + + +class FilterContext(Protocol): + proxy_model: QSortFilterProxyModel + app_state: object # only passed through to proxy_model for now + + def refresh_filter(self) -> None: ... # triggers view/model refresh + + +@dataclass +class FilterState: + rating_filter: str = "Show All" + cluster_filter_id: int = -1 + search_text: str = "" + + +class FilterController: + """Encapsulates rating/cluster filter manipulation previously handled in MainWindow. + + Responsibilities: + - Track current rating and cluster filter choices + - Apply them to the proxy model + - Provide convenience predicates for tests / external logic + """ + + def __init__(self, ctx: FilterContext): + self.ctx = ctx + self.state = FilterState() + # Track whether we've deferred the initial push because proxy_model was missing + self._initial_push_pending = True # start pending until proxy exists + + # --- Public API --- + def set_rating_filter(self, value: str) -> None: + if value == self.state.rating_filter: + return + self.state.rating_filter = value + self._push_state_to_proxy() + + def set_cluster_filter(self, cluster_id: int) -> None: + if cluster_id == self.state.cluster_filter_id: + return + self.state.cluster_filter_id = cluster_id + self._push_state_to_proxy() + + def clear_filters(self) -> None: + changed = False + if self.state.rating_filter != "Show All": + self.state.rating_filter = "Show All" + changed = True + if self.state.cluster_filter_id != -1: + self.state.cluster_filter_id = -1 + changed = True + if changed: + self._push_state_to_proxy() + + def get_rating_filter(self) -> str: + return self.state.rating_filter + + def get_cluster_filter_id(self) -> int: + return self.state.cluster_filter_id + + def set_search_text(self, text: str) -> None: + t = (text or "").lower() + if t == self.state.search_text: + return + self.state.search_text = t + self._apply_search_text() + + # --- Application hooks --- + def apply_all(self, show_folders: bool, current_view_mode: str) -> None: + """Push filters + search text + view flags to proxy model.""" + # If initial push was deferred, do it now transparently + if self._initial_push_pending: + self._push_state_to_proxy(show_folders, current_view_mode) + return + self._push_state_to_proxy(show_folders, current_view_mode) + + # --- Internal helpers --- + def _apply_search_text(self) -> None: + proxy = getattr(self.ctx, "proxy_model", None) + if proxy is None: + # Context not fully initialized yet; mark for later + self._initial_push_pending = True + return + proxy.setFilterRegularExpression(self.state.search_text) + + def _push_state_to_proxy( + self, + show_folders: Optional[bool] = None, + current_view_mode: Optional[str] = None, + ) -> None: + proxy = getattr(self.ctx, "proxy_model", None) + if proxy is None: + # MainWindow may not have created proxy_model yet; defer application + self._initial_push_pending = True + return + # Attach AppState reference once (idempotent) if attribute exists + if getattr(proxy, "app_state_ref", None) is None: + try: + proxy.app_state_ref = self.ctx.app_state # type: ignore[attr-defined] + except Exception: + pass + # Push current filters + if hasattr(proxy, "current_rating_filter"): + proxy.current_rating_filter = self.state.rating_filter # type: ignore[attr-defined] + if hasattr(proxy, "current_cluster_filter_id"): + proxy.current_cluster_filter_id = self.state.cluster_filter_id # type: ignore[attr-defined] + if show_folders is not None and hasattr(proxy, "show_folders_mode_ref"): + proxy.show_folders_mode_ref = show_folders # type: ignore[attr-defined] + if current_view_mode is not None and hasattr(proxy, "current_view_mode_ref"): + proxy.current_view_mode_ref = current_view_mode # type: ignore[attr-defined] + # search text + proxy.setFilterRegularExpression(self.state.search_text) + # Trigger a refresh + self.ctx.refresh_filter() + self._initial_push_pending = False + + def ensure_initialized(self, show_folders: bool, current_view_mode: str) -> None: + """Apply deferred initial state once proxy exists.""" + if self._initial_push_pending: + self._push_state_to_proxy(show_folders, current_view_mode) diff --git a/src/ui/controllers/hotkey_controller.py b/src/ui/controllers/hotkey_controller.py new file mode 100644 index 0000000..bee654e --- /dev/null +++ b/src/ui/controllers/hotkey_controller.py @@ -0,0 +1,47 @@ +from __future__ import annotations +from typing import Protocol, Dict, Tuple, Callable +from PyQt6.QtCore import Qt + + +class HotkeyContext(Protocol): + def navigate_left_in_group(self) -> None: ... + def navigate_right_in_group(self) -> None: ... + def navigate_up_sequential(self) -> None: ... + def navigate_down_sequential(self) -> None: ... + def navigate_down_smart( + self, + ) -> None: ... # New: group-cyclic if in group else sequential + + +class HotkeyController: + def __init__(self, ctx: HotkeyContext): + self.ctx = ctx + # Bindings map key -> (label, callable accepting skip_deleted) + self.bindings: Dict[int, Tuple[str, Callable[[bool], None]]] = { + Qt.Key.Key_Left: ("LEFT/H", self.ctx.navigate_left_in_group), + Qt.Key.Key_H: ("LEFT/H", self.ctx.navigate_left_in_group), + Qt.Key.Key_Right: ("RIGHT/L", self.ctx.navigate_right_in_group), + Qt.Key.Key_L: ("RIGHT/L", self.ctx.navigate_right_in_group), + # Up/Down arrows are sequential only (no group-cycling) + Qt.Key.Key_Up: ("UP/K", self.ctx.navigate_up_sequential), + Qt.Key.Key_K: ("UP/K", self.ctx.navigate_up_sequential), + Qt.Key.Key_Down: ("DOWN/J", self.ctx.navigate_down_sequential), + Qt.Key.Key_J: ("DOWN/J", self.ctx.navigate_down_sequential), + } + + def handle_key(self, key: int, skip_deleted: bool = True) -> bool: + """Handle a navigation key. + + skip_deleted = True -> skip images marked for deletion (default) + skip_deleted = False -> include images marked for deletion (Ctrl/Cmd modified navigation) + """ + entry = self.bindings.get(key) + if not entry: + return False + _, fn = entry + try: + fn(skip_deleted) # New signature (wrappers updated in MainWindow) + except TypeError: + # Backwards compatibility if wrapper not yet updated + fn() + return True diff --git a/src/ui/controllers/metadata_controller.py b/src/ui/controllers/metadata_controller.py new file mode 100644 index 0000000..a930195 --- /dev/null +++ b/src/ui/controllers/metadata_controller.py @@ -0,0 +1,27 @@ +from __future__ import annotations +from typing import Protocol, List + + +class MetadataContext(Protocol): + metadata_sidebar: object | None + sidebar_visible: bool + + def get_selected_file_paths(self) -> List[str]: ... + def ensure_metadata_sidebar(self) -> None: ... + + +class MetadataController: + def __init__(self, ctx: MetadataContext): + self.ctx = ctx + self._cached_selection: List[str] = [] + + def refresh_for_selection(self): + if not self.ctx.sidebar_visible or not self.ctx.metadata_sidebar: + return + paths = self.ctx.get_selected_file_paths() + if paths == self._cached_selection: + return + self._cached_selection = paths + sidebar = self.ctx.metadata_sidebar + if hasattr(sidebar, "update_selection"): + sidebar.update_selection(paths) diff --git a/src/ui/controllers/navigation_controller.py b/src/ui/controllers/navigation_controller.py new file mode 100644 index 0000000..07481ba --- /dev/null +++ b/src/ui/controllers/navigation_controller.py @@ -0,0 +1,159 @@ +from __future__ import annotations +from typing import List, Optional, Iterable, Protocol, Set +from PyQt6.QtCore import QModelIndex, Qt +from PyQt6.QtWidgets import QAbstractItemView +import logging + +logger = logging.getLogger(__name__) + + +class NavigationContext(Protocol): + def get_active_view(self) -> Optional[QAbstractItemView]: ... + def is_valid_image_index(self, proxy_index: QModelIndex) -> bool: ... + def map_to_source(self, proxy_index: QModelIndex) -> QModelIndex: ... + def item_from_source(self, source_index: QModelIndex): ... + def get_group_sibling_images(self, current_proxy_index: QModelIndex): ... + def find_first_visible_item(self) -> QModelIndex: ... + def find_proxy_index_for_path(self, path: str) -> QModelIndex: ... + def get_all_visible_image_paths(self) -> List[str]: ... + def get_marked_deleted(self) -> Iterable[str]: ... + def validate_and_select_image_candidate( + self, proxy_index: QModelIndex, direction: str, log_skip: bool + ): ... + + +def navigate_group_cyclic( + group_paths: List[str], + current: Optional[str], + direction: str, + skip_deleted: bool, + deleted_set: Set[str], +) -> Optional[str]: + if not group_paths: + return None + if current not in group_paths: + return group_paths[0] if direction == "right" else group_paths[-1] + idx = group_paths.index(current) + step = -1 if direction == "left" else 1 + for _ in range(len(group_paths)): + idx = (idx + step) % len(group_paths) + candidate = group_paths[idx] + if not skip_deleted or candidate not in deleted_set: + return candidate + return None + + +def navigate_linear( + all_visible: List[str], + current: Optional[str], + direction: str, + skip_deleted: bool, + deleted_set: Set[str], +) -> Optional[str]: + if not all_visible: + return None + if current not in all_visible: + return all_visible[0] if direction == "down" else all_visible[-1] + idx = all_visible.index(current) + step = -1 if direction in ("up", "left") else 1 + while True: + idx += step + if idx < 0 or idx >= len(all_visible): + return None + candidate = all_visible[idx] + if not skip_deleted or candidate not in deleted_set: + return candidate + + +class NavigationController: + def __init__(self, ctx: NavigationContext): + self.ctx = ctx + + def navigate_group(self, direction: str, skip_deleted: bool = True): + active_view = self.ctx.get_active_view() + if not active_view: + return + current_proxy_idx = active_view.currentIndex() + current_path = None + if current_proxy_idx.isValid() and self.ctx.is_valid_image_index( + current_proxy_idx + ): + src_idx = self.ctx.map_to_source(current_proxy_idx) + item = self.ctx.item_from_source(src_idx) + if item: + data = item.data(Qt.ItemDataRole.UserRole) + if isinstance(data, dict): + current_path = data.get("path") + group_paths: List[str] = [] + if current_proxy_idx.isValid(): + _parent_group_idx, group_image_indices, cur_local_idx = ( + self.ctx.get_group_sibling_images(current_proxy_idx) + ) + for idx in group_image_indices: + src_idx = self.ctx.map_to_source(idx) + item = self.ctx.item_from_source(src_idx) + if item: + d = item.data(Qt.ItemDataRole.UserRole) + if isinstance(d, dict) and "path" in d: + group_paths.append(d["path"]) + logger.debug( + "Group navigation: dir=%s, group_size=%d, cur_local_idx=%s, current=%s", + direction, + len(group_paths), + cur_local_idx, + current_path, + ) + if not group_paths: + first_item = self.ctx.find_first_visible_item() + if first_item.isValid(): + sel_model = active_view.selectionModel() + if sel_model: + # Access selection flag dynamically to avoid strict import dependency in tests + flag = getattr(sel_model, "SelectionFlag", None) + if flag is not None: + sel_model.setCurrentIndex(first_item, flag.ClearAndSelect) # type: ignore[attr-defined] + else: + sel_model.setCurrentIndex(first_item, 0) + active_view.setFocus(Qt.FocusReason.ShortcutFocusReason) + return + deleted_set = set(self.ctx.get_marked_deleted()) if skip_deleted else set() + target_path = navigate_group_cyclic( + group_paths, current_path, direction, skip_deleted, deleted_set + ) + logger.debug( + "Group navigation: target_path=%s (skip_deleted=%s)", + target_path, + skip_deleted, + ) + if target_path: + proxy_idx = self.ctx.find_proxy_index_for_path(target_path) + if proxy_idx.isValid(): + # Pass skip_deleted through for accurate logging and any downstream filtering + self.ctx.validate_and_select_image_candidate( + proxy_idx, direction, skip_deleted + ) + + def navigate_linear(self, direction: str, skip_deleted: bool = True): + active_view = self.ctx.get_active_view() + if not active_view: + return + all_visible = self.ctx.get_all_visible_image_paths() + current_path = None + cur_idx = active_view.currentIndex() + if cur_idx.isValid() and self.ctx.is_valid_image_index(cur_idx): + src_idx = self.ctx.map_to_source(cur_idx) + item = self.ctx.item_from_source(src_idx) + if item: + d = item.data(Qt.ItemDataRole.UserRole) + if isinstance(d, dict): + current_path = d.get("path") + deleted_set = set(self.ctx.get_marked_deleted()) if skip_deleted else set() + target_path = navigate_linear( + all_visible, current_path, direction, skip_deleted, deleted_set + ) + if target_path: + proxy_idx = self.ctx.find_proxy_index_for_path(target_path) + if proxy_idx.isValid(): + self.ctx.validate_and_select_image_candidate( + proxy_idx, direction, skip_deleted + ) diff --git a/src/ui/controllers/preview_controller.py b/src/ui/controllers/preview_controller.py new file mode 100644 index 0000000..5c67aaf --- /dev/null +++ b/src/ui/controllers/preview_controller.py @@ -0,0 +1,31 @@ +from __future__ import annotations +from typing import Protocol, List, Dict + + +class PreviewContext(Protocol): + worker_manager: object + + def show_loading_overlay(self, text: str) -> None: ... + def hide_loading_overlay(self) -> None: ... + def status_message(self, msg: str, timeout: int = 3000) -> None: ... + + +class PreviewController: + def __init__(self, ctx: PreviewContext): + self.ctx = ctx + + def start_preload(self, image_data_list: List[Dict[str, any]]): + if not image_data_list: + self.ctx.hide_loading_overlay() + self.ctx.status_message("No previews to preload.") + return + paths = [ + d.get("path") + for d in image_data_list + if isinstance(d, dict) and d.get("path") + ] + paths = [p for p in paths if p] + if not paths: + self.ctx.hide_loading_overlay() + return + self.ctx.worker_manager.start_preview_preload(paths) diff --git a/src/ui/controllers/rotation_controller.py b/src/ui/controllers/rotation_controller.py new file mode 100644 index 0000000..4e99f8e --- /dev/null +++ b/src/ui/controllers/rotation_controller.py @@ -0,0 +1,85 @@ +from __future__ import annotations +from typing import Dict, List, Optional, Callable +from src.ui.helpers.rotation_utils import compute_next_after_rotation + + +class RotationController: + """Encapsulates logic for accepting / refusing rotation suggestions. + + Keeps state in a shared rotation_suggestions dict (path -> degrees). + Delegates actual rotation application to provided callback (typically AppController). + """ + + def __init__( + self, + rotation_suggestions: Dict[str, int], + apply_rotations: Callable[[Dict[str, int]], None], + ): + self._rotation_suggestions = rotation_suggestions + self._apply_rotations = apply_rotations + + # --- State access --- + @property + def rotation_suggestions(self) -> Dict[str, int]: + return self._rotation_suggestions + + def has_suggestions(self) -> bool: + return bool(self._rotation_suggestions) + + def get_visible_order(self) -> List[str]: + # Current UI uses dict iteration order as the visual list in rotation view rebuild + return list(self._rotation_suggestions.keys()) + + # --- Accept / Refuse operations --- + def accept_all(self) -> List[str]: + if not self._rotation_suggestions: + return [] + to_apply = dict(self._rotation_suggestions) + self._apply_rotations(to_apply) + accepted = list(to_apply.keys()) + self._rotation_suggestions.clear() + return accepted + + def accept_paths(self, paths: List[str]) -> List[str]: + to_apply = { + p: self._rotation_suggestions[p] + for p in paths + if p in self._rotation_suggestions + } + if not to_apply: + return [] + self._apply_rotations(to_apply) + for p in to_apply: + self._rotation_suggestions.pop(p, None) + return list(to_apply.keys()) + + def refuse_all(self) -> List[str]: + refused = list(self._rotation_suggestions.keys()) + self._rotation_suggestions.clear() + return refused + + def refuse_paths(self, paths: List[str]) -> List[str]: + refused = [] + for p in paths: + if p in self._rotation_suggestions: + self._rotation_suggestions.pop(p) + refused.append(p) + return refused + + # --- Next selection computation --- + def compute_next_after_accept( + self, + visible_before: List[str], + accepted_paths: List[str], + anchor_path: Optional[str] = None, + ) -> Optional[str]: + remaining = self.get_visible_order() + if not accepted_paths: + return None + anchor = anchor_path or accepted_paths[0] + return compute_next_after_rotation( + visible_paths_before=visible_before, + accepted_paths=accepted_paths, + remaining_paths_after=remaining, + anchor_path=anchor, + ) diff --git a/src/ui/controllers/selection_controller.py b/src/ui/controllers/selection_controller.py new file mode 100644 index 0000000..feec917 --- /dev/null +++ b/src/ui/controllers/selection_controller.py @@ -0,0 +1,161 @@ +from __future__ import annotations +from typing import List, Protocol, Any +import os +from PyQt6.QtCore import QModelIndex +from PyQt6.QtGui import QStandardItem +from PyQt6.QtCore import Qt + + +class SelectionContext(Protocol): + """Protocol the SelectionController depends on. + + Relaxed for duck-typing: any model/view pair exposing the minimal methods + used by the controller is acceptable (e.g. test stubs without QWidget). + """ + + def get_active_view(self) -> Any | None: ... # view must expose model() + @property + def proxy_model(self) -> Any: ... # kept for backward compatibility + def is_valid_image_item(self, proxy_index: QModelIndex) -> bool: ... + def file_system_model_item_from_index( + self, source_index: QModelIndex + ) -> QStandardItem | None: ... + def map_to_source(self, proxy_index: QModelIndex) -> QModelIndex: ... + + +class SelectionController: + """Encapsulates selection gathering logic from MainWindow.""" + + def __init__(self, ctx: SelectionContext): + self.ctx = ctx + + def get_selected_file_paths(self) -> List[str]: + view = self.ctx.get_active_view() + if not view: + return [] + sel_model = view.selectionModel() + if not sel_model: + return [] + selected_indexes = sel_model.selectedIndexes() + out: List[str] = [] + for proxy_index in selected_indexes: + if proxy_index.column() != 0: + continue + source_index = self.ctx.map_to_source(proxy_index) + if not source_index.isValid(): + continue + item = self.ctx.file_system_model_item_from_index(source_index) + if not item: + continue + item_user_data = item.data(Qt.ItemDataRole.UserRole) + if isinstance(item_user_data, dict) and "path" in item_user_data: + file_path = item_user_data["path"] + if os.path.isfile(file_path) and file_path not in out: + out.append(file_path) + return out + + # --- Visibility scanning helpers (migrated from MainWindow) --- + def find_first_visible_item(self) -> QModelIndex: + """Return first visible, valid image index or invalid if none. + + Tree vs flat detection is duck-typed via presence of 'isExpanded'. + """ + from PyQt6.QtCore import QModelIndex + + view = self.ctx.get_active_view() + if view is None: + return QModelIndex() + # view must provide model(); tolerate AttributeError + try: + model = view.model() + except Exception: + return QModelIndex() + if model is None: + return QModelIndex() + root_idx = QModelIndex() + row_count = model.rowCount(root_idx) + + is_tree = hasattr(view, "isExpanded") + if is_tree: + queue = [model.index(r, 0, root_idx) for r in range(row_count)] + head = 0 + while head < len(queue): + idx = queue[head] + head += 1 + if not idx.isValid(): + continue + try: + # Optional row hidden support + if hasattr(view, "isRowHidden") and view.isRowHidden( + idx.row(), idx.parent() + ): # type: ignore[attr-defined] + continue + except Exception: + pass + if self.ctx.is_valid_image_item(idx): + return idx + try: + if getattr(view, "isExpanded", lambda _i: False)( + idx + ) and model.hasChildren(idx): + for child_row in range(model.rowCount(idx)): + queue.append(model.index(child_row, 0, idx)) + except Exception: + pass + return QModelIndex() + # Flat scan + for r in range(row_count): + idx = model.index(r, 0, root_idx) + if self.ctx.is_valid_image_item(idx): + return idx + return QModelIndex() + + def find_last_visible_item(self) -> QModelIndex: + """Return last visible, valid image index or invalid if none.""" + from PyQt6.QtCore import QModelIndex + + view = self.ctx.get_active_view() + if view is None: + return QModelIndex() + try: + model = view.model() + except Exception: + return QModelIndex() + if model is None: + return QModelIndex() + root_idx = QModelIndex() + row_count = model.rowCount(root_idx) + + is_tree = hasattr(view, "isExpanded") + if is_tree: + last_valid = QModelIndex() + queue = [model.index(r, 0, root_idx) for r in range(row_count)] + head = 0 + while head < len(queue): + idx = queue[head] + head += 1 + if not idx.isValid(): + continue + try: + if hasattr(view, "isRowHidden") and view.isRowHidden( + idx.row(), idx.parent() + ): # type: ignore[attr-defined] + continue + except Exception: + pass + if self.ctx.is_valid_image_item(idx): + last_valid = idx + try: + if getattr(view, "isExpanded", lambda _i: False)( + idx + ) and model.hasChildren(idx): + for child_row in range(model.rowCount(idx)): + queue.append(model.index(child_row, 0, idx)) + except Exception: + pass + return last_valid + for r in range(row_count - 1, -1, -1): + idx = model.index(r, 0, root_idx) + if self.ctx.is_valid_image_item(idx): + return idx + return QModelIndex() diff --git a/src/ui/controllers/similarity_controller.py b/src/ui/controllers/similarity_controller.py new file mode 100644 index 0000000..0045027 --- /dev/null +++ b/src/ui/controllers/similarity_controller.py @@ -0,0 +1,166 @@ +from __future__ import annotations +from typing import Protocol, Dict, List, Any, Optional +from datetime import date as date_obj + +from src.ui.helpers.cluster_utils import ClusterUtils + + +class SimilarityContext(Protocol): + app_state: object + worker_manager: object + menu_manager: object + + def show_loading_overlay(self, text: str) -> None: ... + def hide_loading_overlay(self) -> None: ... + def update_loading_text(self, text: str) -> None: ... + def status_message(self, msg: str, timeout: int = 3000) -> None: ... + def rebuild_model_view(self) -> None: ... + def enable_group_by_similarity(self, enabled: bool) -> None: ... + def set_group_by_similarity_checked(self, checked: bool) -> None: ... + def set_cluster_sort_visible(self, visible: bool) -> None: ... + def enable_cluster_sort_combo(self, enabled: bool) -> None: ... + def populate_cluster_filter(self, cluster_ids: List[int]) -> None: ... + + +class AppStateSimilarityView(Protocol): + """Protocol subset of AppState attributes used by SimilarityController. + + Keeps controller loosely coupled to the concrete AppState implementation. + """ + + image_files_data: List[Dict[str, Any]] + cluster_results: Dict[str, int] + date_cache: Dict[str, Optional[date_obj]] + embeddings_cache: Dict[str, List[float]] | Dict[str, Any] + + +class SimilarityController: + def __init__(self, ctx: SimilarityContext): + self.ctx = ctx + + def start(self, paths: List[str], auto_edits: bool): + if not paths: + self.ctx.status_message("No valid image paths for similarity analysis.") + return + self.ctx.show_loading_overlay("Starting similarity analysis...") + self.ctx.worker_manager.start_similarity_analysis(paths, auto_edits) + + def embeddings_generated(self, embeddings_dict): + self.ctx.app_state.embeddings_cache = embeddings_dict + self.ctx.update_loading_text("Embeddings generated. Clustering...") + + def clustering_complete(self, cluster_results: Dict[str, int], group_mode: bool): + self.ctx.app_state.cluster_results = cluster_results + if not cluster_results: + self.ctx.hide_loading_overlay() + self.ctx.status_message("Clustering did not produce results.") + return + self.ctx.update_loading_text("Clustering complete. Updating view...") + cluster_ids = sorted(set(cluster_results.values())) + self.ctx.populate_cluster_filter(cluster_ids) + self.ctx.enable_group_by_similarity(True) + self.ctx.set_group_by_similarity_checked(True) + if cluster_results: + self.ctx.set_cluster_sort_visible(True) + self.ctx.enable_cluster_sort_combo(True) + if group_mode: + self.ctx.rebuild_model_view() + self.ctx.hide_loading_overlay() + + def error(self, message: str): + self.ctx.status_message(f"Similarity Error: {message}", 8000) + self.ctx.hide_loading_overlay() + + # --- New extracted logic for cluster grouping / sorting --- + def get_images_by_cluster(self) -> Dict[int, List[Dict[str, Any]]]: + """Return mapping cluster_id -> list of image dicts. + + This excludes any paths absent from app_state.image_files_data ensuring + the UI only renders data with full metadata available. + """ + app_state = getattr(self.ctx, "app_state", None) + if not app_state: + return {} + return ClusterUtils.group_images_by_cluster( + getattr(app_state, "image_files_data", []), + getattr(app_state, "cluster_results", {}) or {}, + ) + + def _get_cluster_timestamps( + self, + images_by_cluster: Dict[int, List[Dict[str, Any]]], + date_cache: Dict[str, Optional[date_obj]], + ) -> Dict[int, date_obj]: + return ClusterUtils.get_cluster_timestamps(images_by_cluster, date_cache) + + def _sort_by_similarity_time( + self, + images_by_cluster: Dict[int, List[Dict[str, Any]]], + embeddings_cache: Dict[str, List[float]], + date_cache: Dict[str, Optional[date_obj]], + ) -> List[int]: + return ClusterUtils.sort_clusters_by_similarity_time( + images_by_cluster, embeddings_cache, date_cache + ) + + def sort_cluster_ids( + self, + images_by_cluster: Dict[int, List[Dict[str, Any]]], + sort_method: str, + ) -> List[int]: + """Return ordered cluster ids based on sort method. + + Methods: + Default -> numeric ascending + Time -> earliest timestamp among items in cluster + Similarity then Time -> PCA order of centroids then earliest timestamp + + Falls back to Time ordering if embeddings are missing or insufficient + for PCA (handled inside ClusterUtils). + """ + cluster_ids = list(images_by_cluster.keys()) + if not cluster_ids: + return [] + app_state = getattr(self.ctx, "app_state", None) + date_cache = getattr(app_state, "date_cache", {}) if app_state else {} + embeddings_cache = ( + getattr(app_state, "embeddings_cache", {}) if app_state else {} + ) + + if sort_method == "Time": + timestamps = self._get_cluster_timestamps(images_by_cluster, date_cache) + cluster_ids.sort(key=lambda cid: timestamps.get(cid, date_obj.max)) + elif sort_method == "Similarity then Time": + if not embeddings_cache: + timestamps = self._get_cluster_timestamps(images_by_cluster, date_cache) + cluster_ids.sort(key=lambda cid: timestamps.get(cid, date_obj.max)) + else: + cluster_ids = self._sort_by_similarity_time( + images_by_cluster, embeddings_cache, date_cache + ) + else: # Default or unknown + cluster_ids.sort() + return cluster_ids + + def prepare_clusters(self, sort_method: str) -> Dict[str, Any]: + """Return structured cluster info for view construction. + + Returns keys: + images_by_cluster -> cluster_id -> list[image dict] + sorted_cluster_ids -> display order + total_images -> total images assigned to clusters + """ + images_by_cluster = self.get_images_by_cluster() + if not images_by_cluster: + return { + "images_by_cluster": {}, + "sorted_cluster_ids": [], + "total_images": 0, + } + sorted_cluster_ids = self.sort_cluster_ids(images_by_cluster, sort_method) + total_images = sum(len(v) for v in images_by_cluster.values()) + return { + "images_by_cluster": images_by_cluster, + "sorted_cluster_ids": sorted_cluster_ids, + "total_images": total_images, + } diff --git a/src/ui/helpers/cluster_utils.py b/src/ui/helpers/cluster_utils.py new file mode 100644 index 0000000..5901b3c --- /dev/null +++ b/src/ui/helpers/cluster_utils.py @@ -0,0 +1,170 @@ +import logging +from datetime import date as date_obj +from typing import Dict, List, Optional, Any + +import numpy as np +from sklearn.decomposition import PCA + +logger = logging.getLogger(__name__) + + +class ClusterUtils: + """Utility helpers for grouping images into clusters and ordering them. + + These are extracted from the previous monolithic MainWindow implementation + to enable unit testing without a running Qt event loop. + """ + + @staticmethod + def group_images_by_cluster( + image_files_data: List[Dict[str, Any]], + cluster_results: Dict[str, int], + ) -> Dict[int, List[Dict[str, Any]]]: + images_by_cluster: Dict[int, List[Dict[str, Any]]] = {} + if not image_files_data or not cluster_results: + return images_by_cluster + + image_data_map = { + img_data.get("path"): img_data + for img_data in image_files_data + if isinstance(img_data, dict) + } + for file_path, cluster_id in cluster_results.items(): + if file_path in image_data_map: + images_by_cluster.setdefault(cluster_id, []).append( + image_data_map[file_path] + ) + return images_by_cluster + + @staticmethod + def get_cluster_timestamps( + images_by_cluster: Dict[int, List[Dict[str, Any]]], + date_cache: Dict[str, Optional[date_obj]], + ) -> Dict[int, date_obj]: + cluster_timestamps: Dict[int, date_obj] = {} + for cluster_id, file_data_list in images_by_cluster.items(): + earliest_date = date_obj.max + found_date = False + for file_data in file_data_list: + path = file_data.get("path") if isinstance(file_data, dict) else None + if not path: + continue + img_date = date_cache.get(path) + if img_date and img_date < earliest_date: + earliest_date = img_date + found_date = True + cluster_timestamps[cluster_id] = ( + earliest_date if found_date else date_obj.max + ) + return cluster_timestamps + + @staticmethod + def calculate_cluster_centroids( + images_by_cluster: Dict[int, List[Dict[str, Any]]], + embeddings_cache: Dict[str, List[float]], + ) -> Dict[int, np.ndarray]: + centroids: Dict[int, np.ndarray] = {} + if not embeddings_cache: + return centroids + for cluster_id, file_data_list in images_by_cluster.items(): + cluster_embeddings = [] + for file_data in file_data_list: + path = file_data.get("path") if isinstance(file_data, dict) else None + if not path: + continue + embedding = embeddings_cache.get(path) + if embedding is None: + continue + if isinstance(embedding, np.ndarray): + cluster_embeddings.append(embedding) + elif isinstance(embedding, list): + try: + cluster_embeddings.append(np.array(embedding, dtype=np.float32)) + except Exception: # pragma: no cover - defensive + pass + if cluster_embeddings: + try: + centroids[cluster_id] = np.mean( + np.stack(cluster_embeddings), axis=0 + ) + except Exception as e: # pragma: no cover - defensive + logger.error( + f"Error calculating centroid for cluster {cluster_id}: {e}" + ) + return centroids + + @staticmethod + def sort_clusters_by_similarity_time( + images_by_cluster: Dict[int, List[Dict[str, Any]]], + embeddings_cache: Dict[str, List[float]], + date_cache: Dict[str, Optional[date_obj]], + ) -> List[int]: + """Sort clusters using PCA of centroids, falling back to time. + + Strategy: + 1. Compute centroid per cluster (mean embedding). + 2. If at least 2 valid centroids exist, run 1D PCA and order by the + projected component then earliest timestamp as tie-breaker. + 3. If PCA not feasible (<2 centroids or errors) fallback to purely + earliest timestamp ordering. + + Rationale: + Using PCA normalizes embedding variance and yields a stable scalar + ordering without computing a full distance matrix (O(n^2)). The + earliest timestamp is used as deterministic secondary ordering. + + date_obj.max is used as a sentinel for missing timestamps so that + clusters lacking dates naturally sink to the end of time-based + sorts without additional conditionals. + """ + cluster_ids = list(images_by_cluster.keys()) + if not cluster_ids: + return [] + centroids = ClusterUtils.calculate_cluster_centroids( + images_by_cluster, embeddings_cache + ) + valid_cluster_ids_for_pca = [ + cid + for cid in cluster_ids + if isinstance(centroids.get(cid), np.ndarray) and centroids[cid].size > 0 + ] + if len(valid_cluster_ids_for_pca) < 2: + # Fallback to time sort + cluster_timestamps_fb = ClusterUtils.get_cluster_timestamps( + images_by_cluster, date_cache + ) + return sorted( + cluster_ids, + key=lambda cid: cluster_timestamps_fb.get(cid, date_obj.max), + ) + + centroid_matrix = np.stack( + [centroids[cid] for cid in valid_cluster_ids_for_pca] + ) + pca_scores = {} + if ( + centroid_matrix.ndim == 2 + and centroid_matrix.shape[0] > 1 + and centroid_matrix.shape[1] > 0 + ): + try: + n_components = 1 if centroid_matrix.shape[0] > 1 else 0 + if n_components > 0: + pca = PCA(n_components=n_components) + transformed = pca.fit_transform(centroid_matrix) + for i, cid in enumerate(valid_cluster_ids_for_pca): + pca_scores[cid] = transformed[i, 0] + except Exception as e: # pragma: no cover - defensive + logger.error(f"Error during PCA sorting: {e}") + + # Build timestamp map + cluster_timestamps = ClusterUtils.get_cluster_timestamps( + images_by_cluster, date_cache + ) + sortable = [] + for cid in cluster_ids: + pca_val = pca_scores.get(cid, float("inf")) + ts_val = cluster_timestamps.get(cid, date_obj.max) + sortable.append((cid, pca_val, ts_val)) + sortable.sort(key=lambda x: (x[1], x[2])) + return [cid for cid, _, _ in sortable] diff --git a/src/ui/helpers/deletion_utils.py b/src/ui/helpers/deletion_utils.py new file mode 100644 index 0000000..d368d3b --- /dev/null +++ b/src/ui/helpers/deletion_utils.py @@ -0,0 +1,39 @@ +from __future__ import annotations +from dataclasses import dataclass +from typing import Optional + +# Lightweight UI-agnostic description of how a marked/unmarked item should look. +# The actual QColor / palette lookup happens in MainWindow; we only decide suffix logic here. + + +@dataclass(frozen=True) +class DeletionPresentation: + text: str + is_marked: bool + is_blurred: Optional[bool] + + +def build_item_text(basename: str, is_marked: bool, is_blurred: Optional[bool]) -> str: + """Return the display text given mark + blur states. + + Rules (mirrors legacy inline logic): + - Append (DELETED) when marked. + - Append (Blurred) when blurred. + - Order: filename (DELETED) (Blurred) + """ + parts = [basename] + if is_marked: + parts.append("(DELETED)") + if is_blurred: + parts.append("(Blurred)") + return " ".join(parts) + + +def build_presentation( + basename: str, is_marked: bool, is_blurred: Optional[bool] +) -> DeletionPresentation: + return DeletionPresentation( + text=build_item_text(basename, is_marked, is_blurred), + is_marked=is_marked, + is_blurred=is_blurred, + ) diff --git a/src/ui/helpers/index_lookup_utils.py b/src/ui/helpers/index_lookup_utils.py new file mode 100644 index 0000000..fc27587 --- /dev/null +++ b/src/ui/helpers/index_lookup_utils.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +from typing import Callable, List, Optional +from PyQt6.QtCore import QModelIndex, Qt, QSortFilterProxyModel +from PyQt6.QtGui import QStandardItemModel + + +def find_proxy_index_for_path( + target_path: str, + proxy_model: QSortFilterProxyModel, + source_model: QStandardItemModel, + is_valid_image_item: Callable[[QModelIndex], bool], + is_expanded: Optional[Callable[[QModelIndex], bool]] = None, +) -> QModelIndex: + """Pure traversal of proxy model to locate index for a path. + + Parameters: + target_path: file path to find. + proxy_model: the active proxy model (must map to source_model). + source_model: underlying source model containing QStandardItems with UserRole data dict including 'path'. + is_valid_image_item: predicate to determine if an index points to an image item. + is_expanded: optional function telling if a given proxy index is expanded (for tree traversal). When None, assumes expanded. + Returns: + QModelIndex for the proxy model if found else invalid QModelIndex(). + """ + if not isinstance(proxy_model, QSortFilterProxyModel): # Safety + return QModelIndex() + + queue: List[QModelIndex] = [] + root_parent = QModelIndex() + for r in range(proxy_model.rowCount(root_parent)): + queue.append(proxy_model.index(r, 0, root_parent)) + + head = 0 + while head < len(queue): + current_proxy_idx = queue[head] + head += 1 + if not current_proxy_idx.isValid(): + continue + if is_valid_image_item(current_proxy_idx): + source_idx = proxy_model.mapToSource(current_proxy_idx) + item = source_model.itemFromIndex(source_idx) + if item: + data = item.data(Qt.ItemDataRole.UserRole) + if isinstance(data, dict) and data.get("path") == target_path: + return current_proxy_idx + # traverse children if tree-like + if is_expanded is None or is_expanded(current_proxy_idx): + for child_row in range(proxy_model.rowCount(current_proxy_idx)): + queue.append(proxy_model.index(child_row, 0, current_proxy_idx)) + return QModelIndex() + + +def classify_selection(selected_paths: List[str]): + """Return a simple classification tuple for selection state. + + ('none' | 'single' | 'multi', count) + """ + if not selected_paths: + return ("none", 0) + if len(selected_paths) == 1: + return ("single", 1) + return ("multi", len(selected_paths)) diff --git a/src/ui/helpers/navigation_utils.py b/src/ui/helpers/navigation_utils.py new file mode 100644 index 0000000..3002273 --- /dev/null +++ b/src/ui/helpers/navigation_utils.py @@ -0,0 +1,101 @@ +from __future__ import annotations +from typing import Optional, Sequence, Iterable, Set + +# Navigation helpers extracted from MainWindow. These are UI-agnostic and operate on +# ordered path lists plus simple state flags. MainWindow is responsible for mapping +# between QModelIndex <-> paths and invoking these helpers. + + +def navigate_group_cyclic( + sibling_paths: Sequence[str], + current_path: Optional[str], + direction: str, + skip_deleted: bool, + deleted_paths: Set[str] | Iterable[str], +) -> Optional[str]: + """Return the next path within a logical sibling group (left/right cyclic). + + direction: 'left' or 'right' + skip_deleted: if True skip any path in deleted_paths + Wraps around (cyclic) matching original behavior. + """ + if not sibling_paths: + return None + deleted_set = set(deleted_paths) + + # Build the candidate list respecting skip_deleted + if skip_deleted: + candidates = [p for p in sibling_paths if p not in deleted_set] + else: + candidates = list(sibling_paths) + if not candidates: + return None + + if current_path not in candidates: + # Default to first element for deterministic behavior + return candidates[0] + + idx = candidates.index(current_path) + if direction == "left": + return candidates[(idx - 1) % len(candidates)] + elif direction == "right": + return candidates[(idx + 1) % len(candidates)] + else: # Unknown direction + return current_path + + +def navigate_linear( + ordered_paths: Sequence[str], + current_path: Optional[str], + direction: str, + skip_deleted: bool, + deleted_paths: Set[str] | Iterable[str], +) -> Optional[str]: + """Return next path in a flat ordering (up/down semantics). + + direction: 'up' or 'down' + Behavior matches legacy: + - If no current_path: 'down' => first, 'up' => last + - No wrap-around + - If skip_deleted, deleted items are skipped. + """ + if not ordered_paths: + return None + deleted_set = set(deleted_paths) + + def is_ok(p: str) -> bool: + return (p not in deleted_set) if skip_deleted else True + + # Establish starting index + if current_path in ordered_paths: + start_idx = ordered_paths.index(current_path) + else: + start_idx = -1 # Force selection rule below + + if direction == "down": + if start_idx == -1: # No current selection + # pick first acceptable + for p in ordered_paths: + if is_ok(p): + return p + return None + # iterate forward + for i in range(start_idx + 1, len(ordered_paths)): + p = ordered_paths[i] + if is_ok(p): + return p + return None # No further candidate + elif direction == "up": + if start_idx == -1: + # pick last acceptable + for p in reversed(ordered_paths): + if is_ok(p): + return p + return None + for i in range(start_idx - 1, -1, -1): + p = ordered_paths[i] + if is_ok(p): + return p + return None + else: + return current_path diff --git a/src/ui/helpers/rotation_utils.py b/src/ui/helpers/rotation_utils.py new file mode 100644 index 0000000..17cacb7 --- /dev/null +++ b/src/ui/helpers/rotation_utils.py @@ -0,0 +1,34 @@ +from __future__ import annotations +from typing import Optional, Sequence, Iterable + +from src.ui.selection_utils import select_next_surviving_path + + +def compute_next_after_rotation( + visible_paths_before: Sequence[str], + accepted_paths: Iterable[str], + remaining_paths_after: Sequence[str], + anchor_path: Optional[str] = None, +) -> Optional[str]: + """Decide which path should be selected after applying rotation(s). + + Delegates core heuristic to ``select_next_surviving_path`` while providing a + safe default if that returns None. The anchor defaults to the first accepted + path (stable & deterministic) if not explicitly provided. + """ + accepted_list = list(dict.fromkeys(accepted_paths)) # de-dupe preserving order + if not accepted_list: + return None + anchor = anchor_path or accepted_list[0] + candidate = select_next_surviving_path( + visible_paths_before=visible_paths_before, + removed_paths=accepted_list, + anchor_path_before=anchor, + visible_paths_after=remaining_paths_after, + ) + if candidate: + return candidate + # Fallbacks: try the last remaining, else None + if remaining_paths_after: + return remaining_paths_after[min(len(remaining_paths_after) - 1, 0)] + return None diff --git a/src/ui/helpers/statusbar_utils.py b/src/ui/helpers/statusbar_utils.py new file mode 100644 index 0000000..69534a3 --- /dev/null +++ b/src/ui/helpers/statusbar_utils.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +import os +from dataclasses import dataclass +from typing import Any, Optional + + +@dataclass +class StatusBarInfo: + filename: str + rating: int + date_text: str + cluster_id: Optional[int] + size_kb: Optional[int] + width: int + height: int + is_blurred: Optional[bool] + + def to_message(self) -> str: + cluster_part = f" | C: {self.cluster_id}" if self.cluster_id is not None else "" + size_part = ( + f" | Size: {self.size_kb} KB" + if self.size_kb is not None + else " | Size: N/A" + ) + blur_part = ( + " | Blurred: Yes" + if self.is_blurred is True + else (" | Blurred: No" if self.is_blurred is False else "") + ) + return ( + f"{self.filename} | R: {self.rating} | {self.date_text}" + f"{cluster_part}{size_part} | {self.width}x{self.height}{blur_part}" + ) + + +def build_status_bar_info( + file_path: str, + metadata: dict[str, Any], + width: int, + height: int, + cluster_lookup: dict[str, int] | None = None, + file_data_from_model: Optional[dict[str, Any]] = None, +) -> StatusBarInfo: + filename = os.path.basename(file_path) + rating = int(metadata.get("rating", 0) or 0) + date_obj = metadata.get("date") + date_text = f"D: {date_obj.strftime('%Y-%m-%d')}" if date_obj else "D: Unknown" + cluster_id = None + if cluster_lookup and file_path in cluster_lookup: + cluster_id = cluster_lookup[file_path] + try: + size_kb = os.path.getsize(file_path) // 1024 + except OSError: + size_kb = None + is_blurred = ( + file_data_from_model.get("is_blurred") if file_data_from_model else None + ) + return StatusBarInfo( + filename=filename, + rating=rating, + date_text=date_text, + cluster_id=cluster_id, + size_kb=size_kb, + width=width, + height=height, + is_blurred=is_blurred, + ) diff --git a/src/ui/left_panel.py b/src/ui/left_panel.py index eec8cad..93b1222 100644 --- a/src/ui/left_panel.py +++ b/src/ui/left_panel.py @@ -38,7 +38,7 @@ def __init__(self, proxy_model, app_state, main_window, parent=None): self.app_state = app_state self.main_window = main_window self.current_view_mode = "list" - self.thumbnail_delegate = None # This will be set from main_window + self.thumbnail_delegate = None self._create_widgets() self._create_layout() diff --git a/src/ui/main_window.py b/src/ui/main_window.py index 233c6f8..ef5f5dc 100644 --- a/src/ui/main_window.py +++ b/src/ui/main_window.py @@ -10,7 +10,6 @@ QFileDialog, QTreeView, QPushButton, - QListView, QComboBox, QStyle, # For standard icons QAbstractItemView, @@ -44,9 +43,7 @@ QStandardItem, QResizeEvent, ) -import numpy as np -from sklearn.decomposition import PCA -from sklearn.metrics.pairwise import cosine_similarity # Add cosine_similarity import +from sklearn.metrics.pairwise import cosine_similarity import sys from src.core.image_pipeline import ImagePipeline @@ -60,8 +57,6 @@ get_auto_edit_photos, set_auto_edit_photos, DEFAULT_BLUR_DETECTION_THRESHOLD, - DEFAULT_MAX_ITERATIONS, - DEFAULT_SAFETY_ITERATION_MULTIPLIER, LEFT_PANEL_STRETCH, CENTER_PANEL_STRETCH, RIGHT_PANEL_STRETCH, @@ -75,6 +70,20 @@ from src.ui.app_controller import AppController from src.ui.menu_manager import MenuManager from src.ui.selection_utils import select_next_surviving_path +from src.ui.helpers.statusbar_utils import build_status_bar_info +from src.ui.helpers.index_lookup_utils import find_proxy_index_for_path + +# build_presentation now used only inside DeletionMarkController +from src.ui.controllers.deletion_mark_controller import DeletionMarkController +from src.ui.controllers.file_deletion_controller import FileDeletionController +from src.ui.controllers.rotation_controller import RotationController +from src.ui.controllers.filter_controller import FilterController +from src.ui.controllers.hotkey_controller import HotkeyController +from src.ui.controllers.navigation_controller import NavigationController +from src.ui.controllers.selection_controller import SelectionController +from src.ui.controllers.similarity_controller import SimilarityController +from src.ui.controllers.preview_controller import PreviewController +from src.ui.controllers.metadata_controller import MetadataController logger = logging.getLogger(__name__) @@ -192,17 +201,28 @@ def __init__(self, initial_folder=None): self.apply_auto_edits_enabled = get_auto_edit_photos() self.blur_detection_threshold = DEFAULT_BLUR_DETECTION_THRESHOLD self.rotation_suggestions = {} + # Controllers (always created – treat as invariants for simpler code paths) + self.deletion_controller = DeletionMarkController( + app_state=self.app_state, + is_marked_func=lambda p: self.app_state.is_marked_for_deletion(p), + ) + self.file_deletion_controller = FileDeletionController(self) + self.rotation_controller = RotationController( + rotation_suggestions=self.rotation_suggestions, + apply_rotations=lambda mapping: self.app_controller._apply_approved_rotations( + mapping + ), + ) + # Navigation & selection controllers use this MainWindow as context + self.navigation_controller = NavigationController(self) + self.selection_controller = SelectionController(self) + self.filter_controller = FilterController(self) + self.similarity_controller = SimilarityController(self) + self.preview_controller = PreviewController(self) + self.metadata_controller = MetadataController(self) - self.navigation_keys = { - Qt.Key.Key_Left: ("LEFT/H", self._navigate_left_in_group), - Qt.Key.Key_H: ("LEFT/H", self._navigate_left_in_group), - Qt.Key.Key_Right: ("RIGHT/L", self._navigate_right_in_group), - Qt.Key.Key_L: ("RIGHT/L", self._navigate_right_in_group), - Qt.Key.Key_Up: ("UP/K", self._navigate_up_sequential), - Qt.Key.Key_K: ("UP/K", self._navigate_up_sequential), - Qt.Key.Key_Down: ("DOWN/J", self._navigate_down_sequential), - Qt.Key.Key_J: ("DOWN/J", self._navigate_down_sequential), - } + # Hotkey controller wraps navigation key handling + self.hotkey_controller = HotkeyController(self) self.filter_combo = QComboBox() self.filter_combo.addItems( @@ -231,6 +251,16 @@ def __init__(self, initial_folder=None): self.menu_manager = MenuManager(self) self.menu_manager.create_menus(self.menuBar()) self._create_widgets() + + # At this point _create_widgets() built file_system_model + proxy_model and wired it; + # it's now safe to apply any deferred FilterController initialization. + try: + self.filter_controller.ensure_initialized( + self.show_folders_mode, self._determine_current_view_mode() + ) + except Exception as e: + logger.debug(f"FilterController ensure_initialized skipped: {e}") + self._create_layout() self._create_loading_overlay() self.left_panel.thumbnail_delegate = self.thumbnail_delegate @@ -262,7 +292,7 @@ def _update_image_info_label(self, status_message_override: Optional[str] = None folder_name_display = "N/A" # Default text if no folder is loaded yet status_text = "No folder loaded. Open a folder to begin." - scan_logically_active = False # Initialize to avoid UnboundLocalError + scan_logically_active = False if self.app_state.current_folder_path: folder_name_display = os.path.basename(self.app_state.current_folder_path) @@ -309,6 +339,23 @@ def _update_image_info_label(self, status_message_override: Optional[str] = None self.statusBar().showMessage(status_text) + # --- Helper for controllers --- + def _determine_current_view_mode(self) -> str: + """Return a simple string for current primary view mode. + + Existing code historically used 'grid' vs 'list' naming in filtering logic. + We infer from which left-panel view is visible. Defaults to 'grid'. + """ + try: + if hasattr(self, "left_panel"): + if self.left_panel.grid_display_view.isVisible(): + return "grid" + if self.left_panel.tree_display_view.isVisible(): + return "list" + except Exception: + pass + return "grid" + def _create_loading_overlay(self): start_time = time.perf_counter() logger.debug("Creating loading overlay...") @@ -418,7 +465,7 @@ def _clear_exif_cache_action(self): self._update_cache_dialog_labels() # No direct visual refresh needed for EXIF data itself in list/grid, # but metadata display for current image might need update - self._refresh_current_selection_preview() # This will re-fetch metadata + self._refresh_current_selection_preview() def _apply_exif_cache_limit_action(self): selected_index = self.exif_cache_size_combo.currentIndex() @@ -462,13 +509,14 @@ def _create_widgets(self): """Create the UI widgets.""" start_time = time.perf_counter() logger.debug("Creating widgets...") - self.file_system_model = QStandardItemModel() + # Models + self.file_system_model = QStandardItemModel() # Model for file system self.proxy_model = CustomFilterProxyModel(self) self.proxy_model.setSourceModel(self.file_system_model) self.proxy_model.app_state_ref = self.app_state # Link AppState to proxy model + # Left panel (views list/tree/rotation) self.left_panel = LeftPanel(self.proxy_model, self.app_state, self) - self._left_panel_views = { self.left_panel.tree_display_view, self.left_panel.grid_display_view, @@ -493,10 +541,6 @@ def _create_widgets(self): self._image_viewer_views = { v.image_view for v in self.advanced_image_viewer.image_viewers } - - # The rating and color controls are now part of the IndividualViewer - # widgets inside SynchronizedImageViewer, so they are no longer created here. - self.accept_all_button = QPushButton("Accept All") self.accept_all_button.setObjectName("acceptAllButton") self.accept_all_button.setVisible(False) @@ -703,40 +747,20 @@ def _rebuild_model_view( root_item.appendRow(no_cluster_item) return - images_by_cluster = self._group_images_by_cluster() + current_sort_method = self.cluster_sort_combo.currentText() + cluster_info = self.similarity_controller.prepare_clusters( + current_sort_method + ) + images_by_cluster = self.similarity_controller.get_images_by_cluster() + sorted_cluster_ids = cluster_info.get("sorted_cluster_ids", []) + total_clustered_images = cluster_info.get("total_images", 0) + if not images_by_cluster: no_images_in_clusters = QStandardItem("No images assigned to clusters.") no_images_in_clusters.setEditable(False) root_item.appendRow(no_images_in_clusters) return - sorted_cluster_ids = list(images_by_cluster.keys()) - current_sort_method = self.cluster_sort_combo.currentText() - if current_sort_method == "Time": - cluster_timestamps = self._get_cluster_timestamps( - images_by_cluster, self.app_state.date_cache - ) - sorted_cluster_ids.sort( - key=lambda cid: cluster_timestamps.get(cid, date_obj.max) - ) - elif current_sort_method == "Similarity then Time": - if not self.app_state.embeddings_cache: - cluster_timestamps = self._get_cluster_timestamps( - images_by_cluster, self.app_state.date_cache - ) - sorted_cluster_ids.sort( - key=lambda cid: cluster_timestamps.get(cid, date_obj.max) - ) - else: - sorted_cluster_ids = self._sort_clusters_by_similarity_time( - images_by_cluster, - self.app_state.embeddings_cache, - self.app_state.date_cache, - ) - else: # Default sort - sorted_cluster_ids.sort() - - total_clustered_images = 0 for cluster_id in sorted_cluster_ids: cluster_item = QStandardItem(f"Group {cluster_id}") cluster_item.setEditable(False) @@ -745,8 +769,7 @@ def _rebuild_model_view( ) cluster_item.setForeground(QColor(Qt.GlobalColor.gray)) root_item.appendRow(cluster_item) - files_in_cluster = images_by_cluster[cluster_id] - total_clustered_images += len(files_in_cluster) + files_in_cluster = images_by_cluster.get(cluster_id, []) self._populate_model_standard(cluster_item, files_in_cluster) self.statusBar().showMessage( f"Grouped {total_clustered_images} images into {len(sorted_cluster_ids)} clusters.", @@ -814,7 +837,8 @@ def _rebuild_model_view( ) else: # If no selection to restore, fall back to selecting the first visible item - first_index = self._find_first_visible_item() + # selection_controller is always initialized in __init__; simplify fallback logic + first_index = self.selection_controller.find_first_visible_item() if first_index.isValid(): active_view.setCurrentIndex(first_index) active_view.scrollTo( @@ -838,8 +862,126 @@ def _rebuild_model_view( for idx_to_expand in reversed(expand_list): active_view.expand(idx_to_expand) - def _reload_current_folder(self): - self.app_controller.reload_current_folder() + # --- Controller adapter helpers (SimilarityContext / PreviewContext / MetadataContext) --- + def status_message(self, msg: str, timeout: int = 3000) -> None: + self.statusBar().showMessage(msg, timeout) + + def rebuild_model_view(self) -> None: + self._rebuild_model_view() + + def enable_group_by_similarity(self, enabled: bool) -> None: + self.menu_manager.group_by_similarity_action.setEnabled(enabled) + + def set_group_by_similarity_checked(self, checked: bool) -> None: + self.group_by_similarity_mode = checked + self.menu_manager.group_by_similarity_action.setChecked(checked) + + def set_cluster_sort_visible(self, visible: bool) -> None: + self.menu_manager.cluster_sort_action.setVisible(visible) + + def enable_cluster_sort_combo(self, enabled: bool) -> None: + self.cluster_sort_combo.setEnabled(enabled) + + def populate_cluster_filter(self, cluster_ids: List[int]) -> None: + self.cluster_filter_combo.clear() + self.cluster_filter_combo.addItems( + ["All Clusters"] + [f"Cluster {cid}" for cid in cluster_ids] + ) + self.cluster_filter_combo.setEnabled(bool(cluster_ids)) + + def get_selected_file_paths(self) -> List[str]: # For MetadataController + # Prefer SelectionController; fall back only if something unexpected occurs + try: + return self.selection_controller.get_selected_file_paths() + except Exception: + return self._get_selected_file_paths_from_view() + + def ensure_metadata_sidebar(self) -> None: + if not self.metadata_sidebar: + try: + self.metadata_sidebar = MetadataSidebar(self) + except Exception: + return + + # --- NavigationContext adapter wrappers --- + # These expose expected public method names for NavigationController while + # delegating to existing internal implementations that use leading underscores. + def get_all_visible_image_paths( + self, + ) -> List[str]: # NavigationController expects this name + return self._get_all_visible_image_paths() + + def get_active_view(self): # QAbstractItemView | None + return self._get_active_file_view() + + def is_valid_image_index(self, proxy_index): # bool + return self._is_valid_image_item(proxy_index) + + def map_to_source(self, proxy_index): # QModelIndex + active_view = self._get_active_file_view() + if not active_view: + from PyQt6.QtCore import QModelIndex + + return QModelIndex() + model = active_view.model() + try: + return model.mapToSource(proxy_index) # type: ignore[attr-defined] + except Exception: # Fallback + from PyQt6.QtCore import QModelIndex + + return QModelIndex() + + def item_from_source(self, source_index): + try: + return self.file_system_model.itemFromIndex(source_index) + except Exception: + return None + + def get_group_sibling_images(self, current_proxy_index): + # Defer to existing internal method if present + internal = getattr(self, "_get_group_sibling_images", None) + if callable(internal): + return internal(current_proxy_index) + # Fallback shape: (parent_idx, [current_proxy_index], []) + return None, [current_proxy_index], [] + + def find_first_visible_item(self): # Expected by NavigationController + # Delegate to SelectionController (let exceptions surface during development) + return self.selection_controller.find_first_visible_item() + + def find_proxy_index_for_path(self, path: str): # Expected public name + try: + return self._find_proxy_index_for_path(path) + except Exception: + from PyQt6.QtCore import QModelIndex + + return QModelIndex() + + def validate_and_select_image_candidate( + self, proxy_index, direction: str, log_skip: bool + ): + validator = getattr(self, "_validate_and_select_image_candidate", None) + if callable(validator): + return validator(proxy_index, direction, log_skip) + # Minimal fallback: set current selection + active_view = self._get_active_file_view() + if active_view and proxy_index.isValid(): + sel_model = active_view.selectionModel() + if sel_model: + try: + flag = getattr(sel_model, "SelectionFlag", None) + if flag is not None: + sel_model.setCurrentIndex(proxy_index, flag.ClearAndSelect) # type: ignore[attr-defined] + else: + sel_model.setCurrentIndex(proxy_index, 0) + except Exception: + pass + + def get_marked_deleted(self): # Iterable[str] expected by NavigationController + try: + return self.app_state.get_marked_files() + except Exception: + return [] def _rebuild_rotation_view(self): self.file_system_model.clear() @@ -856,19 +998,6 @@ def _rebuild_rotation_view(self): item.setData({"path": path, "rotation": rotation}, Qt.ItemDataRole.UserRole) root_item.appendRow(item) - def _group_images_by_cluster(self) -> Dict[int, List[Dict[str, any]]]: - images_by_cluster: Dict[int, List[Dict[str, any]]] = {} - image_data_map = { - img_data["path"]: img_data for img_data in self.app_state.image_files_data - } - - for file_path, cluster_id in self.app_state.cluster_results.items(): - if file_path in image_data_map: - if cluster_id not in images_by_cluster: - images_by_cluster[cluster_id] = [] - images_by_cluster[cluster_id].append(image_data_map[file_path]) - return images_by_cluster - def _populate_model_standard( self, parent_item: QStandardItem, image_data_list: List[Dict[str, any]] ): @@ -1035,9 +1164,6 @@ def _delete_multiple_images(self, file_paths: List[str]): self.statusBar().showMessage("Failed to delete any images", 5000) def _log_qmodelindex(self, index: QModelIndex, prefix: str = "") -> str: - if not hasattr(self, "proxy_model") or not hasattr(self, "file_system_model"): - return f"{prefix} Invalid QModelIndex (models not initialized)" - if not index.isValid(): return f"{prefix} Invalid QModelIndex" @@ -1075,15 +1201,37 @@ def _is_valid_image_item(self, proxy_index: QModelIndex) -> bool: def _get_active_file_view(self): return self.left_panel.get_active_view() if self.left_panel else None + # --- SelectionController protocol adapter methods --- + # These lightweight wrappers let SelectionController interact with the + # existing MainWindow API without renaming legacy methods yet. + + def is_valid_image_item(self, proxy_index: QModelIndex) -> bool: # protocol alias + return self._is_valid_image_item(proxy_index) + + def file_system_model_item_from_index( + self, source_index: QModelIndex + ): # protocol alias + return ( + self.file_system_model.itemFromIndex(source_index) + if self.file_system_model + else None + ) + + def refresh_filter(self): + # Invalidate proxy model to re-run filtering and sorting. + try: + self.proxy_model.invalidate() + except Exception: + logger.exception("refresh_filter: failed to invalidate proxy model") + def resizeEvent(self, event: QResizeEvent): super().resizeEvent(event) if ( - hasattr(self, "left_panel") + self.left_panel and self.left_panel.current_view_mode == "grid" and not self.group_by_similarity_mode ): - if hasattr(self, "left_panel"): - self.left_panel.update_grid_view_layout() + self.left_panel.update_grid_view_layout() if self.loading_overlay: self.loading_overlay.update_position() @@ -1097,6 +1245,16 @@ def keyPressEvent(self, event: QKeyEvent): # or fallbacks if focus is not on the views. key = event.key() + modifiers = event.modifiers() + is_macos = sys.platform == "darwin" + if is_macos: + has_special = bool(modifiers & Qt.KeyboardModifier.MetaModifier) + else: + has_special = bool(modifiers & Qt.KeyboardModifier.ControlModifier) + skip_deleted = not has_special # Holding Ctrl/Cmd includes deleted + if self.hotkey_controller.handle_key(key, skip_deleted=skip_deleted): + event.accept() + return # Escape key to clear focus from search input (if it has focus) if key == Qt.Key.Key_Escape: @@ -1139,8 +1297,6 @@ def _handle_image_focus_shortcut(self): num_images = sum( 1 for v in self.advanced_image_viewer.image_viewers if v.has_image() ) - # It is considered multi-image if more than one image is loaded into the viewer, - # even if only one is currently visible (focused mode). if num_images > 1 and index < num_images: self.advanced_image_viewer.set_focused_viewer(index) return # Handled @@ -1153,327 +1309,8 @@ def _handle_delete_action(self): self._move_current_image_to_trash() def _move_current_image_to_trash(self): - active_view = self._get_active_file_view() - if not active_view: - return - - # This function is complex. Let's add more targeted debug logs. - logger.debug("Initiating file deletion process.") - - # --- Pre-deletion information gathering --- - self.original_selection_paths = self._get_selected_file_paths_from_view() - visible_paths_before_delete = self._get_all_visible_image_paths() - focused_path_to_delete = ( - self.advanced_image_viewer.get_focused_image_path_if_any() - ) - - # If a single image is focused in the viewer from a multi-selection, - # we prioritize deleting only that focused image. - if focused_path_to_delete: - deleted_file_paths = [focused_path_to_delete] - self.was_focused_delete = True - logger.debug( - f"Deleting focused image: {os.path.basename(focused_path_to_delete)}" - ) - else: - # Otherwise, delete the entire selection from the list/grid view. - deleted_file_paths = self.original_selection_paths - self.was_focused_delete = False - - if not deleted_file_paths: - self.statusBar().showMessage("No image(s) selected to delete.", 3000) - return - - if not self.dialog_manager.show_confirm_delete_dialog(deleted_file_paths): - return - - # Store the proxy index of the initially focused item to try and select something near it later. - # This is tricky with multiple selections across different parents, so we'll simplify. - # We'll try to select the item that was *next* to the *first* item in the original selection, - # or the first visible item if that fails. - - active_selection = active_view.selectionModel().selectedIndexes() - if active_selection: - pass - - # This part now finds the source indices for the file paths we've decided to delete. - # This correctly handles deleting a single focused file or a whole selection. - source_indices_to_delete = [] - for path_to_delete in deleted_file_paths: - proxy_idx_to_delete = self._find_proxy_index_for_path(path_to_delete) - if proxy_idx_to_delete.isValid(): - source_idx = self.proxy_model.mapToSource(proxy_idx_to_delete) - if source_idx.isValid() and source_idx not in source_indices_to_delete: - source_indices_to_delete.append(source_idx) - - # Sort source indices in reverse order by row, then by parent. - # This ensures that when we remove items from a parent, the row numbers - # of subsequent items in that same parent are not affected. - source_indices_to_delete.sort( - key=lambda idx: (idx.parent().internalId(), idx.row()), reverse=True - ) - - deleted_count = 0 - affected_source_parent_items = [] # Store unique QStandardItem objects of parents - - for source_idx_to_delete in source_indices_to_delete: - item_to_delete = self.file_system_model.itemFromIndex(source_idx_to_delete) - if not item_to_delete: - continue - - item_data = item_to_delete.data(Qt.ItemDataRole.UserRole) - if not isinstance(item_data, dict) or "path" not in item_data: - continue - - file_path_to_delete = item_data["path"] - if not os.path.isfile(file_path_to_delete): - continue - - file_name_to_delete = os.path.basename(file_path_to_delete) - try: - self.app_controller.move_to_trash(file_path_to_delete) - self.app_state.remove_data_for_path(file_path_to_delete) - - source_parent_idx = source_idx_to_delete.parent() - source_parent_item = ( - self.file_system_model.itemFromIndex(source_parent_idx) - if source_parent_idx.isValid() - else self.file_system_model.invisibleRootItem() - ) - - if source_parent_item: - source_parent_item.takeRow(source_idx_to_delete.row()) - if source_parent_item not in affected_source_parent_items: - affected_source_parent_items.append( - source_parent_item - ) # Add if unique - deleted_count += 1 - except Exception as e: - # Log the error to terminal as well for easier debugging - logger.error(f"Error moving file to trash: {e}", exc_info=True) - self.dialog_manager.show_error_dialog( - "Delete Error", f"Could not move {file_name_to_delete} to trash." - ) - # Optionally, break or continue if one file fails - - if deleted_count > 0: - # Check and remove empty group headers - # Iterate over a list copy as we might modify the underlying structure - parents_to_check_for_emptiness = list(affected_source_parent_items) - logger.debug( - f"Checking {len(parents_to_check_for_emptiness)} parent groups for emptiness after deletion." - ) - - for parent_item_candidate in parents_to_check_for_emptiness: - if ( - parent_item_candidate == self.file_system_model.invisibleRootItem() - ): # Skip root - continue - if ( - parent_item_candidate.model() is None - ): # Already removed (e.g. child of another removed empty group) - logger.debug( - f"Parent candidate '{parent_item_candidate.text()}' is no longer in the model, skipping." - ) - continue - - is_eligible_group_header = False - parent_user_data = parent_item_candidate.data(Qt.ItemDataRole.UserRole) - - if isinstance(parent_user_data, str): - if parent_user_data.startswith( - "cluster_header_" - ) or parent_user_data.startswith("date_header_"): - is_eligible_group_header = True - elif ( - self.show_folders_mode - and not self.group_by_similarity_mode - and os.path.isdir(parent_user_data) - ): # Folder item - is_eligible_group_header = True - - if is_eligible_group_header and parent_item_candidate.rowCount() == 0: - item_row = ( - parent_item_candidate.row() - ) # Get row before potential parent() call alters context - # parent() of a QStandardItem returns its QStandardItem parent, or None if it's a top-level item. - actual_parent_qstandarditem = parent_item_candidate.parent() - - parent_to_operate_on = None - parent_display_name_for_log = "" - - if ( - actual_parent_qstandarditem is None - ): # It's a top-level item in the model - parent_to_operate_on = ( - self.file_system_model.invisibleRootItem() - ) - parent_display_name_for_log = "invisibleRootItem" - else: # It's a child of another QStandardItem - parent_to_operate_on = actual_parent_qstandarditem - parent_display_name_for_log = ( - f"'{actual_parent_qstandarditem.text()}'" - ) - - logger.debug( - f"Removing empty group header '{parent_item_candidate.text()}' (row {item_row}) from parent {parent_display_name_for_log}" - ) - - # Use takeRow on the QStandardItem that is the actual parent in the model hierarchy - removed_items_list = parent_to_operate_on.takeRow(item_row) - - if ( - removed_items_list - ): # takeRow returns a list of QStandardItems removed - logger.debug( - f"Removed empty group: '{parent_item_candidate.text()}'." - ) - else: - logger.warning( - f"Failed to remove empty group '{parent_item_candidate.text()}' from parent {parent_display_name_for_log} at row {item_row}." - ) - - self.statusBar().showMessage( - f"{deleted_count} image(s) moved to trash.", 5000 - ) - active_view.selectionModel().clearSelection() # Clear old selection to avoid issues - - # Get the state of the view AFTER deletions have occurred. - visible_paths_after_delete = self._get_all_visible_image_paths() - logger.debug( - f"{len(visible_paths_after_delete)} visible paths remaining after deletion." - ) - - # This flag will be used to determine if our special focused-delete logic handled the selection. - selection_handled_by_focus_logic = False - - if self.was_focused_delete: - remaining_selection_paths = [ - p - for p in self.original_selection_paths - if p in visible_paths_after_delete - ] - logger.debug( - f"Found {len(remaining_selection_paths)} remaining items from original selection." - ) - - if remaining_selection_paths: - logger.debug( - f"Handling focused delete with {len(remaining_selection_paths)} remaining paths: {remaining_selection_paths}" - ) - self._handle_file_selection_changed( - override_selected_paths=remaining_selection_paths - ) - - selection = QItemSelection() - first_proxy_idx_to_select = QModelIndex() - - for path in remaining_selection_paths: - proxy_idx = self._find_proxy_index_for_path(path) - if proxy_idx.isValid(): - selection.select(proxy_idx, proxy_idx) - if not first_proxy_idx_to_select.isValid(): - first_proxy_idx_to_select = proxy_idx - - if not selection.isEmpty(): - logger.debug( - f"Setting selection with {len(selection.indexes())} indexes" - ) - selection_model = active_view.selectionModel() - selection_model.blockSignals(True) - # Set the current index first - if first_proxy_idx_to_select.isValid(): - active_view.setCurrentIndex(first_proxy_idx_to_select) - selection_model.select( - selection, QItemSelectionModel.SelectionFlag.ClearAndSelect - ) - selection_model.blockSignals(False) - - if first_proxy_idx_to_select.isValid(): - logger.debug( - f"Scrolling to first selected item: {first_proxy_idx_to_select}" - ) - active_view.scrollTo( - first_proxy_idx_to_select, - QAbstractItemView.ScrollHint.EnsureVisible, - ) - - # Explicitly call _handle_file_selection_changed to ensure viewer updates - logger.debug( - "Explicitly calling _handle_file_selection_changed after focused delete selection update" - ) - # Add a small delay to ensure the selection model has time to update - QTimer.singleShot(0, self._handle_file_selection_changed) - - selection_handled_by_focus_logic = True - - # Fallback logic for standard deletion or if focused-delete logic fails to find items - if not selection_handled_by_focus_logic: - logger.debug("Using fallback logic for selection after deletion") - if not visible_paths_after_delete: - logger.debug("No visible image items left after deletion.") - self.advanced_image_viewer.clear() - self.advanced_image_viewer.setText("No images left to display.") - self.statusBar().showMessage("No images left or visible.") - else: - first_deleted_path_idx_in_visible_list = -1 - if visible_paths_before_delete and deleted_file_paths: - try: - first_deleted_path_idx_in_visible_list = ( - visible_paths_before_delete.index(deleted_file_paths[0]) - ) - except ValueError: - first_deleted_path_idx_in_visible_list = 0 - elif visible_paths_before_delete: - first_deleted_path_idx_in_visible_list = 0 - - target_idx_in_new_list = min( - first_deleted_path_idx_in_visible_list, - len(visible_paths_after_delete) - 1, - ) - target_idx_in_new_list = max(0, target_idx_in_new_list) - - logger.debug( - f"Selecting item at index {target_idx_in_new_list} from {len(visible_paths_after_delete)} remaining items" - ) - - next_item_to_select_proxy_idx = self._find_proxy_index_for_path( - visible_paths_after_delete[target_idx_in_new_list] - ) - - if next_item_to_select_proxy_idx.isValid(): - logger.debug( - f"Setting current index and selection for item: {visible_paths_after_delete[target_idx_in_new_list]}" - ) - active_view.setCurrentIndex(next_item_to_select_proxy_idx) - active_view.selectionModel().select( - next_item_to_select_proxy_idx, - QItemSelectionModel.SelectionFlag.ClearAndSelect, - ) - active_view.scrollTo( - next_item_to_select_proxy_idx, - QAbstractItemView.ScrollHint.EnsureVisible, - ) - # Explicitly call _handle_file_selection_changed to ensure viewer updates - logger.debug( - "Explicitly calling _handle_file_selection_changed after selection update" - ) - # Add a small delay to ensure the selection model has time to update - QTimer.singleShot(0, self._handle_file_selection_changed) - else: - logger.debug( - "Fallback failed. No valid item to select. Clearing UI." - ) - self.advanced_image_viewer.clear() - self.advanced_image_viewer.setText("No valid image to select.") - - self._update_image_info_label() - elif ( - deleted_count == 0 and len(self.original_selection_paths) > 0 - ): # No items were actually deleted, but some were selected - self.statusBar().showMessage( - "No valid image files were deleted from selection.", 3000 - ) + # Delegate to controller + self.file_deletion_controller.move_current_image_to_trash() def _is_row_hidden_in_tree_if_applicable( self, active_view, proxy_idx: QModelIndex @@ -1640,8 +1477,21 @@ def _get_current_group_sibling_images( if sibling_idx == current_image_proxy_idx: current_item_local_idx = len(sibling_image_items) - 1 + try: + # Debug information to help verify runtime grouping + logger.debug( + "_get_current_group_sibling_images: group_size=%d, cur_local_idx=%s", + len(sibling_image_items), + current_item_local_idx, + ) + except Exception: + pass return parent_proxy_idx, sibling_image_items, current_item_local_idx + # Backwards-compatible alias used by get_group_sibling_images adapter + def _get_group_sibling_images(self, current_proxy_index: QModelIndex): + return self._get_current_group_sibling_images(current_proxy_index) + def _validate_and_select_image_candidate( self, candidate_idx: QModelIndex, direction: str, skip_deleted: bool ) -> bool: @@ -1703,378 +1553,107 @@ def _validate_and_select_image_candidate( return True - def _navigate_left_in_group(self, skip_deleted=True): - active_view = self._get_active_file_view() - if not active_view: - return - current_proxy_idx = active_view.currentIndex() - if not current_proxy_idx.isValid() or not self._is_valid_image_item( - current_proxy_idx - ): - first_item = self._find_first_visible_item() - if first_item.isValid(): - sel_model = active_view.selectionModel() - if sel_model is not None: - sel_model.select( - first_item, QItemSelectionModel.SelectionFlag.ClearAndSelect - ) - active_view.setCurrentIndex(first_item) - active_view.setFocus(Qt.FocusReason.ShortcutFocusReason) - return - - _parent_group_idx, group_images, local_idx = ( - self._get_current_group_sibling_images(current_proxy_idx) - ) + def _navigate_left_in_group(self, skip_deleted: bool = True): + self.navigation_controller.navigate_group("left", skip_deleted) - if not group_images or local_idx == -1: - logger.debug("Navigate left: No sibling images found in the current group.") - return + def _navigate_right_in_group(self, skip_deleted: bool = True): + self.navigation_controller.navigate_group("right", skip_deleted) - num_items = len(group_images) - if num_items == 0: - return + def _navigate_up_sequential(self, skip_deleted: bool = True): + self.navigation_controller.navigate_linear("up", skip_deleted) - # Since _get_current_group_sibling_images already returns a list of non-deleted items, - # we can simply calculate the next index. The `skip_deleted` parameter is handled by that function. - new_local_idx = (local_idx - 1 + num_items) % num_items - candidate_idx = group_images[new_local_idx] + def _navigate_down_sequential(self, skip_deleted: bool = True): + self.navigation_controller.navigate_linear("down", skip_deleted) - # We call the validator just to perform the selection action. - # Passing skip_deleted=False because we know the item is not deleted. - self._validate_and_select_image_candidate(candidate_idx, "left", False) + # HotkeyController expects context methods without underscores (now accept skip_deleted) + def navigate_left_in_group(self, skip_deleted: bool = True): + self._navigate_left_in_group(skip_deleted) - def _navigate_right_in_group(self, skip_deleted=True): - active_view = self._get_active_file_view() - if not active_view: - return - current_proxy_idx = active_view.currentIndex() - if not current_proxy_idx.isValid() or not self._is_valid_image_item( - current_proxy_idx - ): - first_item = self._find_first_visible_item() - if first_item.isValid(): - sel_model = active_view.selectionModel() - if sel_model is not None: - sel_model.select( - first_item, QItemSelectionModel.SelectionFlag.ClearAndSelect - ) - active_view.setCurrentIndex(first_item) - active_view.setFocus(Qt.FocusReason.ShortcutFocusReason) - return + def navigate_right_in_group(self, skip_deleted: bool = True): + self._navigate_right_in_group(skip_deleted) - _parent_group_idx, group_images, local_idx = ( - self._get_current_group_sibling_images(current_proxy_idx) - ) + def navigate_up_sequential(self, skip_deleted: bool = True): + self._navigate_up_sequential(skip_deleted) - if not group_images or local_idx == -1: - logger.debug( - "Navigate right: No sibling images found in the current group." - ) - return + def navigate_down_sequential(self, skip_deleted: bool = True): + self._navigate_down_sequential(skip_deleted) - num_items = len(group_images) - if num_items == 0: - return + # Smart down navigation: if currently in a similarity group (more than one image in group) + # and not at end (or wrap), move within group like horizontal cycle; else fall back to linear down. + def navigate_down_smart(self, skip_deleted: bool = True): + self._navigate_group_smart("down", skip_deleted) - # Since _get_current_group_sibling_images already returns a list of non-deleted items, - # we can simply calculate the next index. The `skip_deleted` parameter is handled by that function. - new_local_idx = (local_idx + 1) % num_items - candidate_idx = group_images[new_local_idx] + def navigate_up_smart(self, skip_deleted: bool = True): + """Smart up: cycle backwards within a similarity group, else linear up.""" + self._navigate_group_smart("up", skip_deleted) - # We call the validator just to perform the selection action. - # Passing skip_deleted=False because we know the item is not deleted. - self._validate_and_select_image_candidate(candidate_idx, "right", False) + def _navigate_group_smart(self, direction: str, skip_deleted: bool): + """Shared smart navigation inside a similarity group. - def _navigate_up_sequential(self, skip_deleted=True): - active_view = self._get_active_file_view() - if not active_view: - logger.debug("Navigate up: No active view found.") + direction: 'up' or 'down' + Falls back to sequential navigation if any precondition fails. + """ + if direction not in ("up", "down"): return - - current_proxy_idx = active_view.currentIndex() - - if not current_proxy_idx.isValid(): - last_item_index = self._find_last_visible_item() - if last_item_index.isValid(): - sel_model = active_view.selectionModel() - if sel_model is not None: - sel_model.select( - last_item_index, - QItemSelectionModel.SelectionFlag.ClearAndSelect, - ) - active_view.setCurrentIndex(last_item_index) - active_view.scrollTo( - last_item_index, QAbstractItemView.ScrollHint.EnsureVisible + try: + active_view = self._get_active_file_view() + if not active_view: + return ( + self._navigate_up_sequential(skip_deleted) + if direction == "up" + else self._navigate_down_sequential(skip_deleted) ) - active_view.setFocus(Qt.FocusReason.ShortcutFocusReason) - return - - iter_idx = current_proxy_idx - - max_iterations = ( - self.proxy_model.rowCount(QModelIndex()) - + sum( - self.proxy_model.rowCount(self.proxy_model.index(r, 0, QModelIndex())) - for r in range(self.proxy_model.rowCount(QModelIndex())) - ) - ) * 2 - if max_iterations == 0 and self.app_state and self.app_state.image_files_data: - max_iterations = len(self.app_state.image_files_data) * 5 - if max_iterations == 0: - max_iterations = DEFAULT_MAX_ITERATIONS - - for iteration_count in range(max_iterations): - prev_visual_idx = self._index_above(active_view, iter_idx) - - if not prev_visual_idx.isValid(): - break - - if self._validate_and_select_image_candidate( - prev_visual_idx, "up", skip_deleted - ): - return - - if isinstance(active_view, QTreeView) and self._is_expanded_group_header( - active_view, prev_visual_idx - ): - if iter_idx.parent() != prev_visual_idx: - last_in_group = self._find_last_visible_image_item_in_subtree( - prev_visual_idx, skip_deleted=skip_deleted - ) - # Validate the item before selecting it - if ( - last_in_group.isValid() - and self._validate_and_select_image_candidate( - last_in_group, "up", skip_deleted - ) - ): - return - - iter_idx = prev_visual_idx - if iteration_count == max_iterations - 1: # Safety break - logger.warning("Navigate up: Max iterations reached, aborting.") - - logger.debug("Navigate up: No previous image found.") - - def _navigate_down_sequential(self, skip_deleted=True): - active_view = self._get_active_file_view() - if not active_view: - logger.debug("Navigate down: No active view found.") - return - - current_index = active_view.currentIndex() - if not current_index.isValid(): - first_item_index = self._find_first_visible_item() - if first_item_index.isValid(): - sel_model = active_view.selectionModel() - if sel_model is not None: - sel_model.select( - first_item_index, - QItemSelectionModel.SelectionFlag.ClearAndSelect, - ) - active_view.setCurrentIndex(first_item_index) - active_view.scrollTo( - first_item_index, QAbstractItemView.ScrollHint.EnsureVisible + cur_idx = active_view.currentIndex() + if not cur_idx.isValid(): + return ( + self._navigate_up_sequential(skip_deleted) + if direction == "up" + else self._navigate_down_sequential(skip_deleted) ) - active_view.setFocus(Qt.FocusReason.ShortcutFocusReason) - return - - temp_index = current_index - iteration_count = 0 - - # Determine a safe iteration limit to prevent infinite loops in unexpected scenarios - safety_iteration_limit = ( - self.proxy_model.rowCount(QModelIndex()) - * DEFAULT_SAFETY_ITERATION_MULTIPLIER - ) - if self.app_state.image_files_data: - safety_iteration_limit = max( - safety_iteration_limit, - len(self.app_state.image_files_data) - * DEFAULT_SAFETY_ITERATION_MULTIPLIER, - ) - if safety_iteration_limit == 0: - safety_iteration_limit = DEFAULT_MAX_ITERATIONS - - while temp_index.isValid() and iteration_count < safety_iteration_limit: - iteration_count += 1 - # 1. Get the item visually below the current one - temp_index = self._index_below(active_view, temp_index) - - if not temp_index.isValid(): - break - - if self._validate_and_select_image_candidate( - temp_index, "down", skip_deleted - ): - return - - if iteration_count >= safety_iteration_limit: - logger.warning("Navigate down: Max iterations reached, aborting.") - - logger.debug("Navigate down: No next image found.") - - def _index_below(self, view: QAbstractItemView, index: QModelIndex) -> QModelIndex: - """Return the index visually below the given one for both tree and list/grid views.""" - from PyQt6.QtWidgets import QTreeView - - if isinstance(view, QTreeView): - return view.indexBelow(index) - # QListView/grid: advance row by 1 within the same parent - model = view.model() - if not model: - return QModelIndex() - parent = index.parent() - next_row = index.row() + 1 - if 0 <= next_row < model.rowCount(parent): - return model.index(next_row, index.column(), parent) - return QModelIndex() - - def _index_above(self, view: QAbstractItemView, index: QModelIndex) -> QModelIndex: - """Return the index visually above the given one for both tree and list/grid views.""" - from PyQt6.QtWidgets import QTreeView - - if isinstance(view, QTreeView): - return view.indexAbove(index) - # QListView/grid: move row by -1 within the same parent - model = view.model() - if not model: - return QModelIndex() - parent = index.parent() - prev_row = index.row() - 1 - if 0 <= prev_row < model.rowCount(parent): - return model.index(prev_row, index.column(), parent) - return QModelIndex() - - def _find_first_visible_item(self) -> QModelIndex: - active_view = self._get_active_file_view() - if not active_view: - return QModelIndex() - - proxy_model = active_view.model() - if not isinstance(proxy_model, QSortFilterProxyModel): - return QModelIndex() - - root_proxy_index = QModelIndex() - proxy_row_count = proxy_model.rowCount(root_proxy_index) - if isinstance(active_view, QTreeView): - q = [ - proxy_model.index(r, 0, root_proxy_index) - for r in range(proxy_row_count) - ] - - head = 0 - while head < len(q): - current_proxy_idx = q[head] - head += 1 - if not current_proxy_idx.isValid(): - continue - - if not active_view.isRowHidden( - current_proxy_idx.row(), current_proxy_idx.parent() - ): - if self._is_valid_image_item(current_proxy_idx): - return current_proxy_idx - source_idx_for_children_check = proxy_model.mapToSource( - current_proxy_idx - ) - item_for_children_check = None - if source_idx_for_children_check.isValid(): - item_for_children_check = ( - proxy_model.sourceModel().itemFromIndex( - source_idx_for_children_check - ) - ) - if ( - item_for_children_check - and proxy_model.hasChildren(current_proxy_idx) - and active_view.isExpanded(current_proxy_idx) - ): - for child_row in range(proxy_model.rowCount(current_proxy_idx)): - q.append(proxy_model.index(child_row, 0, current_proxy_idx)) - return QModelIndex() - elif isinstance(active_view, QListView): - for r in range(proxy_row_count): - proxy_idx = proxy_model.index(r, 0, root_proxy_index) - if self._is_valid_image_item(proxy_idx): - return proxy_idx - return QModelIndex() - return QModelIndex() - - def _find_last_visible_item(self) -> QModelIndex: - active_view = self._get_active_file_view() - if not active_view: - return QModelIndex() - proxy_model = active_view.model() - if not isinstance(proxy_model, QSortFilterProxyModel): - return QModelIndex() - - root_proxy_index = QModelIndex() - - if isinstance(active_view, QTreeView): - # DFS-like approach, exploring last children first - # Stack stores (index_to_visit, has_been_expanded_and_children_queued) - # This is a bit complex to do purely iteratively backwards for DFS. - # Let's try a simpler reversed BFS-like approach on expanded items. - - # Iterate all items in display order and pick the last valid one. - # This is less efficient but simpler to implement correctly than reverse DFS. - # We can optimize if needed, but correctness first. - - last_found_valid_image = QModelIndex() - - # Queue for BFS-like traversal - q = [ - proxy_model.index(r, 0, root_proxy_index) - for r in range(proxy_model.rowCount(root_proxy_index)) - ] - head = 0 - while head < len(q): - current_proxy_idx = q[head] - head += 1 - if not current_proxy_idx.isValid(): - continue - - if not active_view.isRowHidden( - current_proxy_idx.row(), current_proxy_idx.parent() - ): - if self._is_valid_image_item(current_proxy_idx): - last_found_valid_image = ( - current_proxy_idx # Update if this one is valid - ) - - source_idx_for_children_check = proxy_model.mapToSource( - current_proxy_idx - ) - item_for_children_check = None - if source_idx_for_children_check.isValid(): - item_for_children_check = ( - proxy_model.sourceModel().itemFromIndex( - source_idx_for_children_check - ) - ) - - if ( - item_for_children_check - and proxy_model.hasChildren(current_proxy_idx) - and active_view.isExpanded(current_proxy_idx) - ): - for child_row in range(proxy_model.rowCount(current_proxy_idx)): - q.append(proxy_model.index(child_row, 0, current_proxy_idx)) - return last_found_valid_image - - elif isinstance(active_view, QListView): - for r in range( - proxy_model.rowCount(root_proxy_index) - 1, -1, -1 - ): # Iterate backwards - proxy_idx = proxy_model.index(r, 0, root_proxy_index) - if self._is_valid_image_item(proxy_idx): - return proxy_idx - logger.debug("Find last item (List): No visible image item found.") - return QModelIndex() + if not getattr(self, "group_by_similarity_mode", False): + return ( + self._navigate_up_sequential(skip_deleted) + if direction == "up" + else self._navigate_down_sequential(skip_deleted) + ) + _parent_group_idx, group_indices, _ = self.get_group_sibling_images(cur_idx) + if not group_indices or len(group_indices) <= 1: + return ( + self._navigate_up_sequential(skip_deleted) + if direction == "up" + else self._navigate_down_sequential(skip_deleted) + ) + try: + pos = group_indices.index(cur_idx) + except ValueError: + return ( + self._navigate_up_sequential(skip_deleted) + if direction == "up" + else self._navigate_down_sequential(skip_deleted) + ) + step = 1 if direction == "down" else -1 + target_pos = (pos + step) % len(group_indices) + if target_pos == pos: + return # single element guard + target_proxy_idx = group_indices[target_pos] + if target_proxy_idx.isValid(): + self.validate_and_select_image_candidate( + target_proxy_idx, direction, not skip_deleted + ) + else: + ( + self._navigate_up_sequential + if direction == "up" + else self._navigate_down_sequential + )(skip_deleted) + except Exception: + ( + self._navigate_up_sequential + if direction == "up" + else self._navigate_down_sequential + )(skip_deleted) - logger.debug("Find last item: Unknown view type or scenario.") - return QModelIndex() + # Removed obsolete _index_below/_index_above and last-visible item logic (moved to SelectionController or no longer needed) def _get_all_visible_image_paths(self) -> List[str]: """Gets an ordered list of file paths for all currently visible image items.""" @@ -2144,89 +1723,28 @@ def _get_all_visible_image_paths(self) -> List[str]: return paths def _find_proxy_index_for_path(self, target_path: str) -> QModelIndex: - """Finds the QModelIndex in the current proxy model for a given file path.""" active_view = self._get_active_file_view() if not active_view: return QModelIndex() - proxy_model = active_view.model() if not isinstance(proxy_model, QSortFilterProxyModel): - logger.warning( - "Cannot find proxy index: Active view's model is not a QSortFilterProxyModel." - ) return QModelIndex() - - # Similar traversal as _get_all_visible_image_paths - queue = [] - root_proxy_parent_idx = QModelIndex() - for r in range(proxy_model.rowCount(root_proxy_parent_idx)): - queue.append(proxy_model.index(r, 0, root_proxy_parent_idx)) - - head = 0 - while head < len(queue): - current_proxy_idx = queue[head] - head += 1 - if not current_proxy_idx.isValid(): - continue - - if self._is_valid_image_item(current_proxy_idx): - source_idx = proxy_model.mapToSource(current_proxy_idx) - item = self.file_system_model.itemFromIndex( - source_idx - ) # Use source_model - if item: - item_data = item.data(Qt.ItemDataRole.UserRole) - if ( - isinstance(item_data, dict) - and item_data.get("path") == target_path - ): - return current_proxy_idx # Found it - - if isinstance(active_view, QTreeView): - source_idx_for_children_check = proxy_model.mapToSource( - current_proxy_idx - ) - if source_idx_for_children_check.isValid(): - item_for_children_check = self.file_system_model.itemFromIndex( - source_idx_for_children_check - ) - if ( - item_for_children_check - and item_for_children_check.hasChildren() - and active_view.isExpanded(current_proxy_idx) - ): - for child_row in range( - proxy_model.rowCount(current_proxy_idx) - ): # Children from proxy model - queue.append( - proxy_model.index(child_row, 0, current_proxy_idx) - ) - return QModelIndex() # Not found + return find_proxy_index_for_path( + target_path=target_path, + proxy_model=proxy_model, + source_model=self.file_system_model, + is_valid_image_item=self._is_valid_image_item, + is_expanded=(lambda idx: active_view.isExpanded(idx)) + if isinstance(active_view, QTreeView) + else None, + ) def _get_selected_file_paths_from_view(self) -> List[str]: - """Helper to get valid, unique, existing file paths from the current selection.""" - active_view = self._get_active_file_view() - if not active_view: - return [] + """Return list of selected file paths using SelectionController. - selected_indexes = active_view.selectionModel().selectedIndexes() - selected_file_paths = [] - for proxy_index in selected_indexes: - if proxy_index.column() == 0: - source_index = self.proxy_model.mapToSource(proxy_index) - if source_index.isValid(): - item = self.file_system_model.itemFromIndex(source_index) - if item: - item_user_data = item.data(Qt.ItemDataRole.UserRole) - if ( - isinstance(item_user_data, dict) - and "path" in item_user_data - ): - file_path = item_user_data["path"] - if os.path.isfile(file_path): - if file_path not in selected_file_paths: - selected_file_paths.append(file_path) - return selected_file_paths + Previous inlined implementation lived here; now delegated for clarity. + """ + return self.selection_controller.get_selected_file_paths() def _get_cached_metadata_for_selection( self, file_path: str @@ -2379,29 +1897,15 @@ def _display_rotated_image_preview( def _update_status_bar_for_image( self, file_path, metadata, pixmap, file_data_from_model ): - """Helper to compose and set the status bar message for an image.""" - filename = os.path.basename(file_path) - rating_text = f"R: {metadata.get('rating', 0)}" - date_obj = metadata.get("date") - date_text = f"D: {date_obj.strftime('%Y-%m-%d')}" if date_obj else "D: Unknown" - cluster = self.app_state.cluster_results.get(file_path) - cluster_text = f" | C: {cluster}" if cluster is not None else "" - try: - size_text = f" | Size: {os.path.getsize(file_path) // 1024} KB" - except OSError: - size_text = " | Size: N/A" - dimensions_text = f" | {pixmap.width()}x{pixmap.height()}" - is_blurred = ( - file_data_from_model.get("is_blurred") if file_data_from_model else None - ) - blur_status_text = ( - " | Blurred: Yes" - if is_blurred is True - else (" | Blurred: No" if is_blurred is False else "") + info = build_status_bar_info( + file_path=file_path, + metadata=metadata, + width=pixmap.width() if pixmap else 0, + height=pixmap.height() if pixmap else 0, + cluster_lookup=self.app_state.cluster_results, + file_data_from_model=file_data_from_model, ) - - status_message = f"{filename} | {rating_text} | {date_text}{cluster_text}{size_text}{dimensions_text}{blur_status_text}" - self.statusBar().showMessage(status_message) + self.statusBar().showMessage(info.to_message()) def _display_multi_selection_info(self, selected_paths: List[str]): """Handles UI updates when multiple images are selected.""" @@ -2620,12 +2124,10 @@ def _handle_file_selection_changed( file_data_from_model = self._get_cached_metadata_for_selection(file_path) logger.debug(f"Displaying single image preview for: {file_path}") - # This will force the viewer into single-view mode. self._display_single_image_preview(file_path, file_data_from_model) elif len(selected_file_paths) >= 2: logger.debug(f"Handling multi-selection (count={len(selected_file_paths)})") - # This will force the viewer into side-by-side mode. self._display_multi_selection_info(selected_file_paths) else: # No selection @@ -2633,93 +2135,47 @@ def _handle_file_selection_changed( self._handle_no_selection_or_non_image() if self.sidebar_visible and self.metadata_sidebar: self.metadata_sidebar.show_placeholder() + # Always allow MetadataController to update (it internally caches selection) + try: + self.metadata_controller.refresh_for_selection() + except Exception: + pass def _apply_filter(self): # Guard: Don't apply filters if no images are loaded yet if not self.app_state.image_files_data: logger.debug("Filter skipped: No images loaded.") return - search_text = self.left_panel.search_input.text() logger.info(f"Applying filters. Search term: '{search_text}'") - search_text = search_text.lower() - selected_filter_text = self.filter_combo.currentText() - selected_cluster_text = self.cluster_filter_combo.currentText() - target_cluster_id = -1 - if ( - self.cluster_filter_combo.isEnabled() - and selected_cluster_text != "All Clusters" - ): + self.filter_controller.set_search_text(search_text) + self.filter_controller.set_rating_filter(self.filter_combo.currentText()) + cluster_text = self.cluster_filter_combo.currentText() + cluster_id = -1 + if self.cluster_filter_combo.isEnabled() and cluster_text != "All Clusters": try: - target_cluster_id = int(selected_cluster_text.split(" ")[-1]) + cluster_id = int(cluster_text.split(" ")[-1]) except ValueError: - pass - - self.proxy_model.app_state_ref = self.app_state - self.proxy_model.current_rating_filter = selected_filter_text - self.proxy_model.current_cluster_filter_id = target_cluster_id - self.proxy_model.show_folders_mode_ref = self.show_folders_mode - self.proxy_model.current_view_mode_ref = self.left_panel.current_view_mode - - # Set the search text filter - self.proxy_model.setFilterRegularExpression(search_text) - self.proxy_model.setFilterKeyColumn(-1) # Search all columns - self.proxy_model.setFilterRole( - Qt.ItemDataRole.DisplayRole - ) # ← Changed from UserRole to DisplayRole - + cluster_id = -1 + self.filter_controller.set_cluster_filter(cluster_id) + self.filter_controller.apply_all( + show_folders=self.show_folders_mode, + current_view_mode=self.left_panel.current_view_mode, + ) + # Ensure proxy uses desired roles/columns + self.proxy_model.setFilterKeyColumn(-1) + self.proxy_model.setFilterRole(Qt.ItemDataRole.DisplayRole) self.proxy_model.invalidateFilter() def _start_preview_preloader(self, image_data_list: List[Dict[str, any]]): - logger.info( - f"<<< ENTRY >>> _start_preview_preloader called with {len(image_data_list)} items." - ) - if not image_data_list: - logger.info( - "_start_preview_preloader: image_data_list is empty. Hiding overlay." - ) - self.hide_loading_overlay() - return - - paths_for_preloader = [ - fd["path"] - for fd in image_data_list - if fd and isinstance(fd, dict) and "path" in fd - ] - logger.info( - f"_start_preview_preloader: Extracted {len(paths_for_preloader)} paths for preloader." - ) - - if not paths_for_preloader: - logger.info( - "_start_preview_preloader: No valid paths_for_preloader. Hiding overlay." - ) - self.hide_loading_overlay() - return - - self.update_loading_text( - f"Preloading previews ({len(paths_for_preloader)} images)..." - ) - logger.info( - f"_start_preview_preloader: Calling worker_manager.start_preview_preload for {len(paths_for_preloader)} paths." - ) try: - logger.info( - f"_start_preview_preloader: --- CALLING --- worker_manager.start_preview_preload for {len(paths_for_preloader)} paths." - ) - self.worker_manager.start_preview_preload( - paths_for_preloader, self.apply_auto_edits_enabled - ) - logger.info( - "_start_preview_preloader: --- RETURNED --- worker_manager.start_preview_preload call successful." - ) - except Exception as e_preview_preload: - logger.error( - f"_start_preview_preloader: Error calling worker_manager.start_preview_preload: {e_preview_preload}", - exc_info=True, - ) - self.hide_loading_overlay() # Ensure overlay is hidden on error - logger.info("<<< EXIT >>> _start_preview_preloader.") + if logger.isEnabledFor(logging.DEBUG): + logger.debug( + f"Delegating preview preload: {len(image_data_list)} items" + ) + self.preview_controller.start_preload(image_data_list) + except Exception as e: + logger.error(f"PreviewController error: {e}") # Slot for WorkerManager's file_scan_thumbnail_preload_finished signal # This signal is now deprecated in favor of chaining after rating load. @@ -3021,52 +2477,21 @@ def _create_standard_item(self, file_data: Dict[str, any]): if thumbnail_pixmap: item.setIcon(QIcon(thumbnail_pixmap)) - if self._is_marked_for_deletion(file_path): - item.setForeground( - QColor("#FFB366") - ) # Orange/Amber color to indicate marked status - # Add (DELETED) suffix to indicate it's marked - if is_blurred is True: - item.setText(item_text + " (DELETED) (Blurred)") - else: - item.setText(item_text + " (DELETED)") - elif is_blurred is True: - item.setForeground(QColor(Qt.GlobalColor.red)) - item.setText(item_text + " (Blurred)") - else: # Default - item.setForeground(QApplication.palette().text().color()) - item.setText(item_text) + # Unified presentation (marked / blurred) delegated to deletion controller + self.deletion_controller.apply_presentation(item, file_path, is_blurred) return item - def _start_similarity_analysis(self): - logger.info("_start_similarity_analysis called.") - if self.worker_manager.is_similarity_worker_running(): - self.statusBar().showMessage( - "Similarity analysis is already in progress.", 3000 - ) - return - - if not self.app_state.image_files_data: - self.hide_loading_overlay() - self.statusBar().showMessage( - "No images loaded to analyze similarity.", 3000 - ) - return + # _update_item_deletion_blur_presentation removed (inlined via deletion_controller) - paths_for_similarity = [fd["path"] for fd in self.app_state.image_files_data] - if not paths_for_similarity: - self.hide_loading_overlay() - self.statusBar().showMessage( - "No valid image paths for similarity analysis.", 3000 - ) - return - - self.show_loading_overlay("Starting similarity analysis...") - self.menu_manager.analyze_similarity_action.setEnabled(False) - self.worker_manager.start_similarity_analysis( - paths_for_similarity, self.apply_auto_edits_enabled - ) + def _start_similarity_analysis(self): + logger.info("_start_similarity_analysis delegated to SimilarityController") + paths = [ + fd.get("path") + for fd in (self.app_state.image_files_data or []) + if fd.get("path") + ] + self.similarity_controller.start(paths, self.apply_auto_edits_enabled) # Slot for WorkerManager's similarity_progress signal def _handle_similarity_progress(self, percentage, message): @@ -3074,49 +2499,15 @@ def _handle_similarity_progress(self, percentage, message): # Slot for WorkerManager's similarity_embeddings_generated signal def _handle_embeddings_generated(self, embeddings_dict): - self.app_state.embeddings_cache = embeddings_dict - self.update_loading_text("Embeddings generated. Clustering...") + self.similarity_controller.embeddings_generated(embeddings_dict) # Slot for WorkerManager's similarity_clustering_complete signal def _handle_clustering_complete(self, cluster_results_dict: Dict[str, int]): - self.app_state.cluster_results = cluster_results_dict - self.menu_manager.analyze_similarity_action.setEnabled( - bool(self.app_state.image_files_data) - ) - - if not self.app_state.cluster_results: - self.hide_loading_overlay() - self.statusBar().showMessage("Clustering did not produce results.", 3000) - return - - self.update_loading_text("Clustering complete. Updating view...") - cluster_ids = sorted(list(set(self.app_state.cluster_results.values()))) - self.cluster_filter_combo.clear() - self.cluster_filter_combo.addItems( - ["All Clusters"] + [f"Cluster {cid}" for cid in cluster_ids] - ) - self.cluster_filter_combo.setEnabled(True) - self.menu_manager.group_by_similarity_action.setEnabled(True) - self.menu_manager.group_by_similarity_action.setChecked( - True - ) # Automatically switch to group by similarity view - if ( - self.menu_manager.group_by_similarity_action.isChecked() - and self.app_state.cluster_results - ): - self.menu_manager.cluster_sort_action.setVisible(True) - self.cluster_sort_combo.setEnabled(True) - if self.group_by_similarity_mode: - self._rebuild_model_view() - self.hide_loading_overlay() + self.similarity_controller.clustering_complete(cluster_results_dict, True) # Slot for WorkerManager's similarity_error signal def _handle_similarity_error(self, message): - self.statusBar().showMessage(f"Similarity Error: {message}", 8000) - self.menu_manager.analyze_similarity_action.setEnabled( - bool(self.app_state.image_files_data) - ) - self.hide_loading_overlay() + self.similarity_controller.error(message) def _reload_current_folder(self): if self.app_state.image_files_data: @@ -3148,143 +2539,6 @@ def _cluster_sort_changed(self): if self.group_by_similarity_mode and self.app_state.cluster_results: self._rebuild_model_view() - def _get_cluster_timestamps( - self, - images_by_cluster: Dict[int, List[Dict[str, any]]], - date_cache: Dict[str, Optional[date_obj]], - ) -> Dict[int, date_obj]: - cluster_timestamps = {} - for cluster_id, file_data_list in images_by_cluster.items(): - earliest_date = date_obj.max - found_date = False - for file_data in file_data_list: - img_date = date_cache.get(file_data["path"]) - if img_date and img_date < earliest_date: - earliest_date = img_date - found_date = True - cluster_timestamps[cluster_id] = ( - earliest_date if found_date else date_obj.max - ) - return cluster_timestamps - - def _calculate_cluster_centroids( - self, - images_by_cluster: Dict[int, List[Dict[str, any]]], - embeddings_cache: Dict[str, List[float]], - ) -> Dict[int, np.ndarray]: - centroids = {} - if not embeddings_cache: - return centroids - for cluster_id, file_data_list in images_by_cluster.items(): - cluster_embeddings = [] - for file_data in file_data_list: - embedding = embeddings_cache.get(file_data["path"]) - if embedding is not None: - if isinstance(embedding, np.ndarray): - cluster_embeddings.append(embedding) - elif isinstance(embedding, list): - cluster_embeddings.append(np.array(embedding)) - if cluster_embeddings: - try: - # Ensure all embeddings are numpy arrays before stacking for mean calculation - if all(isinstance(emb, np.ndarray) for emb in cluster_embeddings): - if cluster_embeddings: # Ensure list is not empty - # Explicitly cast to float32 if not already, for consistency - centroids[cluster_id] = np.mean( - np.array(cluster_embeddings, dtype=np.float32), axis=0 - ) - except Exception as e: # Catch potential errors in np.mean, like empty list or dtype issues - logger.error( - f"Error calculating centroid for cluster {cluster_id}: {e}" - ) - pass - return centroids - - def _sort_clusters_by_similarity_time( - self, - images_by_cluster: Dict[int, List[Dict[str, any]]], - embeddings_cache: Dict[str, List[float]], - date_cache: Dict[str, Optional[date_obj]], - ) -> List[int]: - cluster_ids = list(images_by_cluster.keys()) - if not cluster_ids: - return [] - - centroids = self._calculate_cluster_centroids( - images_by_cluster, embeddings_cache - ) - valid_cluster_ids_for_pca = [ - cid - for cid in cluster_ids - if cid in centroids - and centroids[cid] is not None - and centroids[cid].size > 0 - ] - - if not valid_cluster_ids_for_pca or len(valid_cluster_ids_for_pca) < 2: - cluster_timestamps_for_fallback = self._get_cluster_timestamps( - images_by_cluster, date_cache - ) - return sorted( - list(images_by_cluster.keys()), - key=lambda cid_orig: cluster_timestamps_for_fallback.get( - cid_orig, date_obj.max - ), - ) - - valid_centroid_list = [centroids[cid] for cid in valid_cluster_ids_for_pca] - if not valid_centroid_list: - cluster_timestamps_for_fallback = self._get_cluster_timestamps( - images_by_cluster, date_cache - ) - return sorted( - list(images_by_cluster.keys()), - key=lambda cid_orig: cluster_timestamps_for_fallback.get( - cid_orig, date_obj.max - ), - ) - - centroid_matrix = np.array(valid_centroid_list) - - pca_scores = {} - # Ensure matrix is 2D and has enough samples/features for PCA - if ( - centroid_matrix.ndim == 2 - and centroid_matrix.shape[0] > 1 - and centroid_matrix.shape[1] > 0 - ): - try: - # n_components for PCA must be less than min(n_samples, n_features) - n_components_pca = min( - 1, - centroid_matrix.shape[0] - 1 if centroid_matrix.shape[0] > 1 else 1, - centroid_matrix.shape[1], - ) - if n_components_pca > 0: # Ensure n_components is at least 1 - pca = PCA(n_components=n_components_pca) - transformed_centroids = pca.fit_transform(centroid_matrix) - for i, cid in enumerate(valid_cluster_ids_for_pca): - pca_scores[cid] = ( - transformed_centroids[i, 0] - if transformed_centroids.ndim > 1 - else transformed_centroids[i] - ) - except Exception as e: - logger.error(f"Error during PCA for cluster sorting: {e}") - - cluster_timestamps = self._get_cluster_timestamps(images_by_cluster, date_cache) - sortable_clusters = [] - for cid in cluster_ids: - pca_val = pca_scores.get( - cid, float("inf") - ) # Default to inf if PCA score not found - ts_val = cluster_timestamps.get(cid, date_obj.max) - sortable_clusters.append((cid, pca_val, ts_val)) - sortable_clusters.sort( - key=lambda x: (x[1], x[2]) - ) # Sort by PCA score, then timestamp - return [item[0] for item in sortable_clusters] - def _handle_toggle_auto_edits(self, checked: bool): self.apply_auto_edits_enabled = checked set_auto_edit_photos(checked) # Save to persistent settings @@ -3329,7 +2583,7 @@ def _handle_toggle_auto_edits(self, checked: bool): except TypeError: pass else: - first_visible_item = self._find_first_visible_item() + first_visible_item = self.selection_controller.find_first_visible_item() if first_visible_item.isValid(): active_view.setCurrentIndex(first_visible_item) @@ -3512,7 +2766,7 @@ def _perform_group_selection_from_key( search_idx = search_idx.parent() if determined_cluster_id is not None: - images_by_cluster = self._group_images_by_cluster() + images_by_cluster = self.similarity_controller.get_images_by_cluster() images_in_group_data = images_by_cluster.get(determined_cluster_id, []) images_to_consider_paths = [d["path"] for d in images_in_group_data] @@ -3550,32 +2804,6 @@ def sort_key(path): return False - def _dispatch_navigation(self, key: int, is_modified: bool) -> bool: - """ - Dispatches a navigation action based on key and modifier. - Returns True if navigation was handled, False otherwise. - """ - if key in self.navigation_keys: - direction_name, nav_func = self.navigation_keys[key] - skip_deleted = not is_modified - - key_type = ( - "arrow" - if direction_name in ["Up", "Down", "Left", "Right"] - else "navigation" - ) - log_prefix = ( - f"Modified {key_type}" if is_modified else key_type.capitalize() - ) - log_suffix = " (bypass deleted)" if is_modified else "" - logger.debug( - f"{log_prefix} key pressed: {direction_name} - Starting navigation{log_suffix}" - ) - - nav_func(skip_deleted=skip_deleted) - return True - return False - def eventFilter(self, obj: QObject, event: QEvent) -> bool: if event.type() == QEvent.Type.KeyPress: # Ensure the event is for one of our views @@ -3671,10 +2899,17 @@ def eventFilter(self, obj: QObject, event: QEvent) -> bool: # For navigation, the platform's special modifier has precedence for # "modified" navigation (include deleted), even with Shift. if has_special: - if self._dispatch_navigation(key, is_modified=True): + # When the platform special modifier is held, include deleted (skip_deleted=False) + if hasattr( + self, "hotkey_controller" + ) and self.hotkey_controller.handle_key( + key, skip_deleted=False + ): return True elif is_unmodified_or_keypad: - if self._dispatch_navigation(key, is_modified=False): + if hasattr( + self, "hotkey_controller" + ) and self.hotkey_controller.handle_key(key, skip_deleted=True): return True if key == Qt.Key.Key_Delete or key == Qt.Key.Key_Backspace: self._handle_delete_action() @@ -3738,7 +2973,7 @@ def _hide_metadata_sidebar(self): def _set_sidebar_visibility(self, show: bool): """Show or hide the sidebar instantly""" - if not hasattr(self, "main_splitter") or not self.metadata_sidebar: + if not self.main_splitter or not self.metadata_sidebar: return current_sizes = self.main_splitter.sizes() @@ -3908,7 +3143,7 @@ def _handle_successful_rotation( """Handle successful rotation - update caches and UI.""" handle_start_time = time.perf_counter() filename = os.path.basename(file_path) - logger.debug( + logger.info( f"Handling successful rotation for '{filename}' (Lossy: {is_lossy})" ) @@ -4419,51 +3654,12 @@ def _mark_selection_for_deletion(self): ) return - path_index_map = { - path: self._find_proxy_index_for_path(path) for path in paths_to_act_on - } - marked_count = 0 - - for file_path in paths_to_act_on: - is_marked = self._is_marked_for_deletion(file_path) - - # Toggle the mark status - if is_marked: - self.app_state.unmark_for_deletion(file_path) - else: - self.app_state.mark_for_deletion(file_path) - - marked_count += 1 - - proxy_idx = path_index_map.get(file_path) - if proxy_idx and proxy_idx.isValid(): - source_idx = self.proxy_model.mapToSource(proxy_idx) - item = self.file_system_model.itemFromIndex(source_idx) - if item: - item_data = item.data(Qt.ItemDataRole.UserRole) - if is_marked: # Unmarking - is_blurred = item_data.get("is_blurred") - if is_blurred: - item.setForeground(QColor(Qt.GlobalColor.red)) - item.setText(os.path.basename(file_path) + " (Blurred)") - else: - item.setForeground(QApplication.palette().text().color()) - # Reset the text to the original filename - item.setText( - os.path.basename(file_path) - + (" (Blurred)" if is_blurred else "") - ) - else: # Marking - item.setForeground(QColor("#FFB366")) - # Keep the original filename but add (DELETED) suffix to indicate it's marked - # If the file is also blurred, we need to keep the (Blurred) suffix - is_blurred = item_data.get("is_blurred") - if is_blurred: - item.setText( - os.path.basename(file_path) + " (DELETED) (Blurred)" - ) - else: - item.setText(os.path.basename(file_path) + " (DELETED)") + marked_count = self.deletion_controller.toggle_paths( + paths_to_act_on, + self._find_proxy_index_for_path, + self.file_system_model, + self.proxy_model, + ) self.statusBar().showMessage(f"Toggled mark for {marked_count} image(s).", 5000) self.proxy_model.invalidate() @@ -4487,35 +3683,12 @@ def _mark_image_for_deletion(self, file_path: str): self.app_state.mark_for_deletion(file_path) # Update the UI - proxy_idx = self._find_proxy_index_for_path(file_path) - if proxy_idx and proxy_idx.isValid(): - source_idx = self.proxy_model.mapToSource(proxy_idx) - item = self.file_system_model.itemFromIndex(source_idx) - if item: - item_data = item.data(Qt.ItemDataRole.UserRole) - if is_marked: # Unmarking - is_blurred = item_data.get("is_blurred") - if is_blurred: - item.setForeground(QColor(Qt.GlobalColor.red)) - item.setText(os.path.basename(file_path) + " (Blurred)") - else: - item.setForeground(QApplication.palette().text().color()) - # Reset the text to the original filename - item.setText( - os.path.basename(file_path) - + (" (Blurred)" if is_blurred else "") - ) - else: # Marking - item.setForeground(QColor("#FFB366")) - # Keep the original filename but add (DELETED) suffix to indicate it's marked - # If the file is also blurred, we need to keep the (Blurred) suffix - is_blurred = item_data.get("is_blurred") - if is_blurred: - item.setText( - os.path.basename(file_path) + " (DELETED) (Blurred)" - ) - else: - item.setText(os.path.basename(file_path) + " (DELETED)") + self.deletion_controller.toggle_paths( + [file_path], + self._find_proxy_index_for_path, + self.file_system_model, + self.proxy_model, + ) self.statusBar().showMessage("Marked 1 image for deletion.", 5000) self.proxy_model.invalidate() @@ -4539,31 +3712,13 @@ def _mark_others_for_deletion(self, file_path_to_keep: str): self.statusBar().showMessage("No other images to mark for deletion.", 3000) return - # Mark all other images for deletion - marked_count = 0 - for path in paths_to_mark: - # Only mark if not already marked - if not self._is_marked_for_deletion(path): - self.app_state.mark_for_deletion(path) - marked_count += 1 - - # Update the UI for all marked images - for path in paths_to_mark: - proxy_idx = self._find_proxy_index_for_path(path) - if proxy_idx and proxy_idx.isValid(): - source_idx = self.proxy_model.mapToSource(proxy_idx) - item = self.file_system_model.itemFromIndex(source_idx) - if item: - item_data = item.data(Qt.ItemDataRole.UserRole) - # Marking - item.setForeground(QColor("#FFB366")) - # Keep the original filename but add (DELETED) suffix to indicate it's marked - # If the file is also blurred, we need to keep the (Blurred) suffix - is_blurred = item_data.get("is_blurred") - if is_blurred: - item.setText(os.path.basename(path) + " (DELETED) (Blurred)") - else: - item.setText(os.path.basename(path) + " (DELETED)") + marked_count = self.deletion_controller.mark_others_in_collection( + file_path_to_keep, + paths_to_mark, + self._find_proxy_index_for_path, + self.file_system_model, + self.proxy_model, + ) self.statusBar().showMessage( f"Marked {marked_count} other image(s) for deletion.", 5000 @@ -4584,24 +3739,12 @@ def _unmark_image_for_deletion(self, file_path: str): # Unmark the file for deletion in the app state self.app_state.unmark_for_deletion(file_path) - # Update the UI - proxy_idx = self._find_proxy_index_for_path(file_path) - if proxy_idx and proxy_idx.isValid(): - source_idx = self.proxy_model.mapToSource(proxy_idx) - item = self.file_system_model.itemFromIndex(source_idx) - if item: - item_data = item.data(Qt.ItemDataRole.UserRole) - # Unmarking - is_blurred = item_data.get("is_blurred") - if is_blurred: - item.setForeground(QColor(Qt.GlobalColor.red)) - item.setText(os.path.basename(file_path) + " (Blurred)") - else: - item.setForeground(QApplication.palette().text().color()) - # Reset the text to the original filename - item.setText( - os.path.basename(file_path) + (" (Blurred)" if is_blurred else "") - ) + self.deletion_controller.toggle_paths( + [file_path], + self._find_proxy_index_for_path, + self.file_system_model, + self.proxy_model, + ) self.statusBar().showMessage("Unmarked 1 image for deletion.", 5000) self.proxy_model.invalidate() @@ -4629,33 +3772,13 @@ def _unmark_others_for_deletion(self, file_path_to_keep: str): ) return - # Unmark all other images for deletion - unmarked_count = 0 - for path in paths_to_unmark: - # Only unmark if actually marked - if self._is_marked_for_deletion(path): - self.app_state.unmark_for_deletion(path) - unmarked_count += 1 - - # Update the UI for all unmarked images - for path in paths_to_unmark: - proxy_idx = self._find_proxy_index_for_path(path) - if proxy_idx and proxy_idx.isValid(): - source_idx = self.proxy_model.mapToSource(proxy_idx) - item = self.file_system_model.itemFromIndex(source_idx) - if item: - item_data = item.data(Qt.ItemDataRole.UserRole) - # Unmarking - is_blurred = item_data.get("is_blurred") - if is_blurred: - item.setForeground(QColor(Qt.GlobalColor.red)) - item.setText(os.path.basename(path) + " (Blurred)") - else: - item.setForeground(QApplication.palette().text().color()) - # Reset the text to the original filename - item.setText( - os.path.basename(path) + (" (Blurred)" if is_blurred else "") - ) + unmarked_count = self.deletion_controller.unmark_others_in_collection( + file_path_to_keep, + paths_to_unmark, + self._find_proxy_index_for_path, + self.file_system_model, + self.proxy_model, + ) self.statusBar().showMessage( f"Unmarked {unmarked_count} other image(s) for deletion.", 5000 @@ -4674,25 +3797,11 @@ def _clear_all_deletion_marks(self): self.statusBar().showMessage("No images are marked for deletion.", 3000) return - # Clear all marks from the app state - self.app_state.clear_all_deletion_marks() - - # Update the UI for each marked file - for file_path in marked_files: - proxy_idx = self._find_proxy_index_for_path(file_path) - if proxy_idx and proxy_idx.isValid(): - source_idx = self.proxy_model.mapToSource(proxy_idx) - item = self.file_system_model.itemFromIndex(source_idx) - if item: - item_data = item.data(Qt.ItemDataRole.UserRole) - # Reset the color to the original - is_blurred = item_data.get("is_blurred") - if is_blurred: - item.setForeground(QColor(Qt.GlobalColor.red)) - item.setText(os.path.basename(file_path) + " (Blurred)") - else: - item.setForeground(QApplication.palette().text().color()) - item.setText(os.path.basename(file_path)) + self.deletion_controller.clear_all_and_update( + self._find_proxy_index_for_path, + self.file_system_model, + self.proxy_model, + ) self.statusBar().showMessage( f"Cleared deletion marks for {len(marked_files)} image(s).", 5000 @@ -4824,105 +3933,15 @@ def _handle_focused_image_changed(self, index: int, file_path: str): QTimer.singleShot(0, lambda: setattr(self, "_is_syncing_selection", False)) def _update_item_blur_status(self, image_path: str, is_blurred: bool): - """ - Finds the QStandardItem for a given path and updates its visual state - to reflect its blurriness. This is the UI-specific part of the update process. - """ - source_model = self.file_system_model - proxy_model = self.proxy_model - - # Get the active view from the LeftPanel, not from the MainWindow itself. - active_view = self.left_panel.get_active_view() - if not active_view: - logger.warning( - f"Could not get active view to update blur status for {image_path}" - ) - return - - item_to_update = None - # The search logic remains the same, as it operates on the source_model which MainWindow owns. - for r_top in range(source_model.rowCount()): - top_item = source_model.item(r_top) - if not top_item: - continue - - # Check top-level item - top_item_data = top_item.data(Qt.ItemDataRole.UserRole) - if ( - isinstance(top_item_data, dict) - and top_item_data.get("path") == image_path - ): - item_to_update = top_item - break - - if top_item.hasChildren(): - for r_child in range(top_item.rowCount()): - child_item = top_item.child(r_child) - if not child_item: - continue - - child_item_data = child_item.data(Qt.ItemDataRole.UserRole) - if ( - isinstance(child_item_data, dict) - and child_item_data.get("path") == image_path - ): - item_to_update = child_item - break - - if child_item.hasChildren(): - for r_grandchild in range(child_item.rowCount()): - grandchild_item = child_item.child(r_grandchild) - if not grandchild_item: - continue - grandchild_item_data = grandchild_item.data( - Qt.ItemDataRole.UserRole - ) - if ( - isinstance(grandchild_item_data, dict) - and grandchild_item_data.get("path") == image_path - ): - item_to_update = grandchild_item - break - if item_to_update: - break - if item_to_update: - break - - if item_to_update: - original_text = os.path.basename(image_path) - item_user_data = item_to_update.data(Qt.ItemDataRole.UserRole) - - # Update the UserRole data on the item for consistency. - if isinstance(item_user_data, dict): - item_user_data["is_blurred"] = is_blurred - item_to_update.setData(item_user_data, Qt.ItemDataRole.UserRole) - - # Update display text and color, respecting the deletion mark status. - is_marked_deleted = self._is_marked_for_deletion(image_path) - blur_suffix = " (Blurred)" if is_blurred else "" - - if is_marked_deleted: - item_to_update.setForeground(QColor("#FFB366")) - # The file is marked for deletion, so we just need to update the color - item_to_update.setText(original_text) - elif is_blurred: - item_to_update.setForeground(QColor(Qt.GlobalColor.red)) - item_to_update.setText(original_text + blur_suffix) - else: - item_to_update.setForeground(QApplication.palette().text().color()) - item_to_update.setText(original_text) - - # If the updated item is currently selected, refresh the main image viewer and status bar. - if active_view.currentIndex().isValid(): - current_proxy_idx = active_view.currentIndex() - current_source_idx = proxy_model.mapToSource(current_proxy_idx) - selected_item = source_model.itemFromIndex(current_source_idx) - if selected_item == item_to_update: - self._handle_file_selection_changed() - else: - logger.warning( - f"Could not find QStandardItem for {image_path} to update blur status in UI." - ) + self.deletion_controller.update_blur_status( + image_path, + is_blurred, + self._find_proxy_index_for_path, + self.file_system_model, + self.proxy_model, + self.left_panel.get_active_view, + lambda: self._handle_file_selection_changed(), + ) def _display_side_by_side_comparison(self, file_path): """Displays the current image and the rotated suggestion side-by-side.""" @@ -4985,133 +4004,52 @@ def _display_side_by_side_comparison(self, file_path): def _accept_all_rotations(self): """Apply all suggested rotations and exit rotation view.""" - if not self.rotation_suggestions: + if not self.rotation_controller.has_suggestions(): self.statusBar().showMessage("No rotation suggestions to accept.", 3000) return - # Use a shallow copy to avoid mutation during iteration inside controller. - rotations_copy = dict(self.rotation_suggestions) - self.app_controller._apply_approved_rotations(rotations_copy) - self.rotation_suggestions.clear() + self.rotation_controller.accept_all() self._hide_rotation_view() def _accept_current_rotation(self): selected_paths = self._get_selected_file_paths_from_view() if not selected_paths: return - - # Handle multi-selection for applying rotations. This method is triggered by the "Accept (N)" button in the UI. - rotations_to_apply = { - path: self.rotation_suggestions[path] - for path in selected_paths - if path in self.rotation_suggestions - } - - if not rotations_to_apply: + target_paths = [ + p + for p in selected_paths + if p in self.rotation_controller.rotation_suggestions + ] + if not target_paths: return - - # Capture ordering before applying rotations so we can compute next selection - visible_paths_before = list(self.rotation_suggestions.keys()) - - self.app_controller._apply_approved_rotations(rotations_to_apply) - - # Remove the accepted rotations from the main suggestion list - for path in rotations_to_apply: - if path in self.rotation_suggestions: - del self.rotation_suggestions[path] - - # If no suggestions are left, hide the rotation view and return to list view - if not self.rotation_suggestions: + visible_before = self.rotation_controller.get_visible_order() + accepted = self.rotation_controller.accept_paths(target_paths) + if not self.rotation_controller.has_suggestions(): self._hide_rotation_view() return - - # Determine the next path to select using helper (prefers forward progress) - try: - from src.ui.selection_utils import ( - select_next_surviving_path as _next_after_delete, - ) - except Exception: - _next_after_delete = None - - path_to_select = None - if _next_after_delete: - # Use the first of the deleted paths as anchor (stable) - deleted_list = list(rotations_to_apply.keys()) - path_to_select = _next_after_delete( - visible_paths_before=visible_paths_before, - deleted_paths=deleted_list, - anchor_path_before=deleted_list[0] if deleted_list else None, - visible_paths_after=list(self.rotation_suggestions.keys()), - ) - - # Rebuild the rotation view to show the remaining items (must happen before mapping index) + next_path = self.rotation_controller.compute_next_after_accept( + visible_before, accepted, accepted[0] if accepted else None + ) self._rebuild_rotation_view() - - # Attempt selection of next candidate - if path_to_select: - proxy_idx_to_select = self._find_proxy_index_for_path(path_to_select) - if proxy_idx_to_select.isValid(): + if next_path: + proxy_idx = self._find_proxy_index_for_path(next_path) + if proxy_idx.isValid(): active_view = self._get_active_file_view() if active_view: - active_view.setCurrentIndex(proxy_idx_to_select) + active_view.setCurrentIndex(proxy_idx) active_view.selectionModel().select( - proxy_idx_to_select, - QItemSelectionModel.SelectionFlag.ClearAndSelect, + proxy_idx, QItemSelectionModel.SelectionFlag.ClearAndSelect ) active_view.scrollTo( - proxy_idx_to_select, - QAbstractItemView.ScrollHint.EnsureVisible, + proxy_idx, QAbstractItemView.ScrollHint.EnsureVisible ) return - - # Fallback: clear selection & viewer if we couldn't determine a next item active_view = self._get_active_file_view() if active_view: active_view.selectionModel().clear() self.advanced_image_viewer.clear() self.accept_button.setVisible(False) - def _accept_rotation(self, file_path: str): - """Applies a single rotation suggestion and selects the next/previous item.""" - if file_path in self.rotation_suggestions: - # Get the list of items before modification to determine the next selection - current_items = list(self.rotation_suggestions.keys()) - try: - current_index = current_items.index(file_path) - except ValueError: - current_index = -1 - - rotation = self.rotation_suggestions.pop(file_path) - self.app_controller._apply_approved_rotations({file_path: rotation}) - - if not self.rotation_suggestions: - self._hide_rotation_view() - return - - remaining_items = list(self.rotation_suggestions.keys()) - path_to_select = None - if current_index != -1 and remaining_items: - # Select the next item in the list - next_index = min(current_index, len(remaining_items) - 1) - path_to_select = remaining_items[next_index] - - self._rebuild_rotation_view() - - if path_to_select: - proxy_idx_to_select = self._find_proxy_index_for_path( - path_to_select - ) - if proxy_idx_to_select.isValid(): - active_view = self._get_active_file_view() - if active_view: - active_view.setCurrentIndex(proxy_idx_to_select) - active_view.selectionModel().select( - proxy_idx_to_select, - QItemSelectionModel.SelectionFlag.ClearAndSelect, - ) - active_view.scrollTo( - proxy_idx_to_select, - QAbstractItemView.ScrollHint.EnsureVisible, - ) + # (Legacy nested _accept_rotation removed; logic handled by controller methods above.) def _on_accept_button_clicked(self): """Handle accept button click with automatic navigation in rotation view.""" @@ -5131,45 +4069,23 @@ def _accept_single_rotation_and_move_to_next(self): # If not exactly one item selected, fall back to the standard accept behavior self._accept_current_rotation() return - file_path = selected_paths[0] - if file_path not in self.rotation_suggestions: + if file_path not in self.rotation_controller.rotation_suggestions: return - # Capture current visible order to compute the best next candidate try: visible_paths_before = self._get_all_visible_image_paths() except Exception: - visible_paths_before = list(self.rotation_suggestions.keys()) - - # Apply the rotation for the current item - rotation = self.rotation_suggestions.pop(file_path) - self.app_controller._apply_approved_rotations({file_path: rotation}) - - # If nothing remains, exit rotation view - if not self.rotation_suggestions: + visible_paths_before = self.rotation_controller.get_visible_order() + accepted = self.rotation_controller.accept_paths([file_path]) + if not self.rotation_controller.has_suggestions(): self._hide_rotation_view() return - # Rebuild the view, then compute the next selection self._rebuild_rotation_view() - - try: - visible_paths_after = self._get_all_visible_image_paths() - except Exception: - visible_paths_after = list(self.rotation_suggestions.keys()) - - from src.ui.selection_utils import ( - select_next_surviving_path as _next_after_delete, - ) - - path_to_select = _next_after_delete( - visible_paths_before=visible_paths_before, - deleted_paths=[file_path], - anchor_path_before=file_path, - visible_paths_after=visible_paths_after, + path_to_select = self.rotation_controller.compute_next_after_accept( + visible_paths_before, accepted, file_path ) - active_view = self._get_active_file_view() if path_to_select and active_view: proxy_idx_to_select = self._find_proxy_index_for_path(path_to_select) @@ -5184,7 +4100,6 @@ def _accept_single_rotation_and_move_to_next(self): QAbstractItemView.ScrollHint.EnsureVisible, ) return - # Fallback: clear selection and preview if we couldn't determine the next if active_view: active_view.selectionModel().clear() @@ -5194,11 +4109,10 @@ def _accept_single_rotation_and_move_to_next(self): def _refuse_all_rotations(self): """Refuses all remaining rotation suggestions.""" - if not self.rotation_suggestions: + if not self.rotation_controller.has_suggestions(): self.statusBar().showMessage("No rotation suggestions to refuse.", 3000) return - - self.rotation_suggestions.clear() + self.rotation_controller.refuse_all() self.statusBar().showMessage( "All rotation suggestions have been refused.", 5000 ) @@ -5209,15 +4123,17 @@ def _refuse_current_rotation(self): selected_paths = self._get_selected_file_paths_from_view() if not selected_paths: return - - for path in selected_paths: - if path in self.rotation_suggestions: - del self.rotation_suggestions[path] - - if not self.rotation_suggestions: + target_paths = [ + p + for p in selected_paths + if p in self.rotation_controller.rotation_suggestions + ] + if not target_paths: + return + self.rotation_controller.refuse_paths(target_paths) + if not self.rotation_controller.has_suggestions(): self._hide_rotation_view() return - self._rebuild_rotation_view() self.advanced_image_viewer.clear() self.accept_button.setVisible(False) diff --git a/src/ui/ui_components.py b/src/ui/ui_components.py index 172046a..864c1ec 100644 --- a/src/ui/ui_components.py +++ b/src/ui/ui_components.py @@ -18,11 +18,11 @@ ) from typing import List, Optional -from src.core.image_pipeline import ImagePipeline # For PreviewPreloaderWorker +from src.core.image_pipeline import ImagePipeline from src.core.image_features.blur_detector import ( BlurDetector, -) # For BlurDetectionWorker -from src.core.caching.exif_cache import ExifCache # Added for RotationDetectionWorker +) +from src.core.caching.exif_cache import ExifCache import logging logger = logging.getLogger(__name__) diff --git a/src/ui/view_manager.py b/src/ui/view_manager.py index 45fbb48..cf5021e 100644 --- a/src/ui/view_manager.py +++ b/src/ui/view_manager.py @@ -25,7 +25,7 @@ def __init__( self.app_state = app_state self.left_panel = left_panel self.current_view_mode = "list" # Default view mode - self.thumbnail_delegate = None # This will be set from main_window + self.thumbnail_delegate = None def connect_signals(self): """Connects signals for the view manager.""" diff --git a/src/ui/worker_manager.py b/src/ui/worker_manager.py index 6e7c147..8403a27 100644 --- a/src/ui/worker_manager.py +++ b/src/ui/worker_manager.py @@ -5,20 +5,19 @@ # Import worker classes from src.core.file_scanner import FileScanner -# from src.core.similarity_engine import SimilarityEngine # Commented out for lazy loading from src.ui.ui_components import ( PreviewPreloaderWorker, BlurDetectionWorker, RotationDetectionWorker, SimilarityWorker, ) -from src.core.image_pipeline import ImagePipeline # Needed for PreviewPreloaderWorker +from src.core.image_pipeline import ImagePipeline from src.core.rating_loader_worker import ( RatingLoaderWorker, -) # Import RatingLoaderWorker -from src.core.caching.rating_cache import RatingCache # For type hinting -from src.core.caching.exif_cache import ExifCache # Added for RotationDetectionWorker -from src.ui.app_state import AppState # For type hinting +) +from src.core.caching.rating_cache import RatingCache +from src.core.caching.exif_cache import ExifCache +from src.ui.app_state import AppState logger = logging.getLogger(__name__) diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..ec04159 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,8 @@ +# Ensure root path (containing src/) is on sys.path for test imports +import sys +import os + +ROOT = os.path.dirname(os.path.abspath(__file__)) +PROJECT_ROOT = os.path.abspath(os.path.join(ROOT, os.pardir)) +if PROJECT_ROOT not in sys.path: + sys.path.insert(0, PROJECT_ROOT) diff --git a/tests/test_cluster_utils.py b/tests/test_cluster_utils.py new file mode 100644 index 0000000..0b85e67 --- /dev/null +++ b/tests/test_cluster_utils.py @@ -0,0 +1,63 @@ +import datetime +from src.ui.helpers.cluster_utils import ClusterUtils + + +def build_fd(path): + return {"path": path} + + +def test_group_images_by_cluster_basic(): + data = [build_fd("a.jpg"), build_fd("b.jpg"), build_fd("c.jpg")] + clusters = {"a.jpg": 1, "b.jpg": 2, "c.jpg": 1} + grouped = ClusterUtils.group_images_by_cluster(data, clusters) + assert set(grouped.keys()) == {1, 2} + assert sorted([fd["path"] for fd in grouped[1]]) == ["a.jpg", "c.jpg"] + + +def test_cluster_timestamps(): + data = [build_fd("a.jpg"), build_fd("b.jpg"), build_fd("c.jpg")] + clusters = {"a.jpg": 1, "b.jpg": 2, "c.jpg": 1} + grouped = ClusterUtils.group_images_by_cluster(data, clusters) + today = datetime.date.today() + date_cache = {"a.jpg": today, "b.jpg": today + datetime.timedelta(days=1)} + ts = ClusterUtils.get_cluster_timestamps(grouped, date_cache) + assert ts[1] == today + assert ts[2] == today + datetime.timedelta(days=1) + + +def test_sort_clusters_fallback_time(): + # Only one centroid -> fallback to time ordering + today = datetime.date.today() + data = [build_fd("a.jpg"), build_fd("b.jpg"), build_fd("c.jpg")] + clusters = {"a.jpg": 2, "b.jpg": 1, "c.jpg": 2} + grouped = ClusterUtils.group_images_by_cluster(data, clusters) + date_cache = { + "a.jpg": today, + "b.jpg": today - datetime.timedelta(days=2), + "c.jpg": today - datetime.timedelta(days=1), + } + order = ClusterUtils.sort_clusters_by_similarity_time( + grouped, embeddings_cache={}, date_cache=date_cache + ) + # Expect cluster 1 (earliest date) then 2 + assert order == [1, 2] + + +def test_sort_clusters_with_embeddings(): + today = datetime.date.today() + data = [build_fd("a.jpg"), build_fd("b.jpg"), build_fd("c.jpg"), build_fd("d.jpg")] + clusters = {"a.jpg": 1, "b.jpg": 1, "c.jpg": 2, "d.jpg": 2} + grouped = ClusterUtils.group_images_by_cluster(data, clusters) + date_cache = {p: today for p in ["a.jpg", "b.jpg", "c.jpg", "d.jpg"]} + # Distinct embeddings so PCA produces deterministic ordering by first component + embeddings = { + "a.jpg": [0.0, 0.0], + "b.jpg": [0.0, 0.2], + "c.jpg": [5.0, 5.0], + "d.jpg": [5.2, 5.0], + } + order = ClusterUtils.sort_clusters_by_similarity_time( + grouped, embeddings, date_cache + ) + assert set(order) == {1, 2} + assert len(order) == 2 diff --git a/tests/test_deletion_utils.py b/tests/test_deletion_utils.py new file mode 100644 index 0000000..0c44e54 --- /dev/null +++ b/tests/test_deletion_utils.py @@ -0,0 +1,17 @@ +from src.ui.helpers.deletion_utils import build_item_text + + +def test_build_item_text_unmarked_unblurred(): + assert build_item_text("photo.jpg", False, None) == "photo.jpg" + + +def test_build_item_text_marked_only(): + assert build_item_text("photo.jpg", True, None) == "photo.jpg (DELETED)" + + +def test_build_item_text_blurred_only(): + assert build_item_text("photo.jpg", False, True) == "photo.jpg (Blurred)" + + +def test_build_item_text_marked_and_blurred(): + assert build_item_text("photo.jpg", True, True) == "photo.jpg (DELETED) (Blurred)" diff --git a/tests/test_file_deletion_controller.py b/tests/test_file_deletion_controller.py new file mode 100644 index 0000000..5c6fab3 --- /dev/null +++ b/tests/test_file_deletion_controller.py @@ -0,0 +1,368 @@ +import os +import pytest +from PyQt6.QtCore import Qt, QModelIndex +from PyQt6.QtGui import QStandardItemModel, QStandardItem +from PyQt6.QtWidgets import QApplication, QTreeView +from PyQt6.QtCore import QSortFilterProxyModel + +from src.ui.controllers.file_deletion_controller import FileDeletionController + +# Ensure a QApplication exists +app = QApplication.instance() or QApplication([]) + + +@pytest.fixture +def deletion_context(): + """Fixture that provides a fully configured DeletionTestContext.""" + return DeletionTestContext() + + +@pytest.fixture +def deletion_controller(deletion_context): + """Fixture that provides a FileDeletionController with deletion_context.""" + return FileDeletionController(deletion_context) + + +@pytest.fixture +def temp_image_files(tmp_path): + """Fixture that creates temporary image files and returns their paths.""" + + def create_images(*names): + paths = [] + for name in names: + p = tmp_path / name + p.write_text("x") # Create file with some content + paths.append(str(p)) + return paths + + return create_images + + +class IdentityProxy(QSortFilterProxyModel): + def mapToSource(self, idx): + return idx + + def mapFromSource(self, source_idx): + return source_idx + + def rowCount(self, parent): # pragma: no cover - trivial + src = self.sourceModel() + if src is None: + return 0 + return src.rowCount(parent) + + def index(self, row, column, parent=QModelIndex()): # pragma: no cover - trivial + src = self.sourceModel() + if src is None: + return super().index(row, column, parent) + return src.index(row, column, parent) + + +class DummyStatusBar: + def __init__(self): + self.last_message = None + + def showMessage(self, msg, timeout=0): # pragma: no cover - trivial + self.last_message = msg + + +class DummyAppController: + def __init__(self): + self.moved_to_trash = [] + + def move_to_trash(self, path): + # Simulate success + self.moved_to_trash.append(path) + return True + + +class DummyAppState: + def __init__(self): + self._data_by_path = {} + self._marked_for_deletion = set() + + def add_path(self, path): + self._data_by_path[path] = {} + + def remove_data_for_path(self, path): + self._data_by_path.pop(path, None) + + def mark_for_deletion(self, path): + self._marked_for_deletion.add(path) + + def unmark_for_deletion(self, path): + self._marked_for_deletion.discard(path) + + def is_marked_for_deletion(self, path): + return path in self._marked_for_deletion + + def get_marked_files(self): + return list(self._marked_for_deletion) + + def clear_all_deletion_marks(self): + self._marked_for_deletion.clear() + + +class DummyDialogManager: + def __init__(self, confirm=True): + self.confirm = confirm + + def show_confirm_delete_dialog(self, paths): # pragma: no cover - trivial + return self.confirm + + def show_error_dialog(self, *a, **k): # pragma: no cover - trivial + pass + + +class DummyAdvancedViewer: + def __init__(self): + self.focused = None + + def get_focused_image_path_if_any(self): + return self.focused + + def clear(self): # pragma: no cover - trivial + self.focused = None + + def setText(self, *_): # pragma: no cover - trivial + pass + + +class DeletionTestContext: + """Minimal context satisfying FileDeletionContextProtocol for tests.""" + + def __init__(self): + self.file_system_model = QStandardItemModel() + self.proxy_model = IdentityProxy() + self.proxy_model.setSourceModel(self.file_system_model) + self.view = QTreeView() + self.view.setModel(self.proxy_model) + self.app_controller = DummyAppController() + self.app_state = DummyAppState() + self.dialog_manager = DummyDialogManager(confirm=True) + self.advanced_image_viewer = DummyAdvancedViewer() + self._selected_paths = [] + self._status_bar = DummyStatusBar() + self.show_folders_mode = False + self.group_by_similarity_mode = False + + # Protocol methods + def _get_active_file_view(self): + return self.view + + def _get_selected_file_paths_from_view(self): + return list(self._selected_paths) + + def _get_all_visible_image_paths(self): + paths = [] + root = self.file_system_model.invisibleRootItem() + for r in range(root.rowCount()): + child = root.child(r) + if not child: + continue + d = child.data(Qt.ItemDataRole.UserRole) + if isinstance(d, dict) and "path" in d: + paths.append(d["path"]) + # If it's a header (string) include its children + elif isinstance(d, str): + for cr in range(child.rowCount()): + img = child.child(cr) + if not img: + continue + dd = img.data(Qt.ItemDataRole.UserRole) + if isinstance(dd, dict) and "path" in dd: + paths.append(dd["path"]) + return paths + + def _find_proxy_index_for_path(self, target_path: str) -> QModelIndex: + root = self.file_system_model.invisibleRootItem() + # search depth 1 and 2 (headers) + for r in range(root.rowCount()): + child = root.child(r) + if not child: + continue + d = child.data(Qt.ItemDataRole.UserRole) + if isinstance(d, dict) and d.get("path") == target_path: + return self.proxy_model.index(r, 0) + elif isinstance(d, str): # header + for cr in range(child.rowCount()): + img = child.child(cr) + dd = img.data(Qt.ItemDataRole.UserRole) + if isinstance(dd, dict) and dd.get("path") == target_path: + return self.proxy_model.index( + cr, 0, self.proxy_model.index(r, 0) + ) + return QModelIndex() + + def _handle_file_selection_changed( + self, override_selected_paths=None + ): # pragma: no cover - trivial + if override_selected_paths is not None: + self._selected_paths = list(override_selected_paths) + + def _update_image_info_label(self): # pragma: no cover - trivial + pass + + def statusBar(self): # pragma: no cover - trivial + return self._status_bar + + +# Helper to create image item + + +def make_image_item(path: str): + it = QStandardItem(os.path.basename(path)) + it.setData({"path": path}, Qt.ItemDataRole.UserRole) + return it + + +def test_focused_single_image_delete_restores_selection( + deletion_controller, deletion_context, temp_image_files +): + """Test that deleting a focused image restores selection to remaining image.""" + ctx = deletion_context + controller = deletion_controller + + # Create temporary image files and add them to the context + paths = temp_image_files("a.jpg", "b.jpg") + for path in paths: + ctx.app_state.add_path(path) + ctx.file_system_model.appendRow(make_image_item(path)) + + ctx._selected_paths = paths + ctx.advanced_image_viewer.focused = paths[0] # Focused delete scenario + + controller.move_current_image_to_trash() + + # paths[0] should be removed; paths[1] remains and becomes selected/focused candidate + remaining_paths = ctx._get_all_visible_image_paths() + assert paths[0] not in remaining_paths and paths[1] in remaining_paths + assert controller.was_focused_delete is True + assert ctx.app_controller.moved_to_trash == [paths[0]] + + +def test_multi_selection_delete( + deletion_controller, deletion_context, temp_image_files +): + """Test that multi-selection deletion removes all selected images.""" + ctx = deletion_context + controller = deletion_controller + + # Create temporary image files and add them to the context + paths = temp_image_files("a.jpg", "b.jpg", "c.jpg") + for path in paths: + ctx.app_state.add_path(path) + ctx.file_system_model.appendRow(make_image_item(path)) + + ctx._selected_paths = paths[:2] # multi-select first two images; no focused image + + controller.move_current_image_to_trash() + + remaining = ctx._get_all_visible_image_paths() + # paths[0] & paths[1] removed, paths[2] should remain + assert set(remaining) == {paths[2]} + assert set(ctx.app_controller.moved_to_trash) == set(paths[:2]) + assert len(ctx.app_controller.moved_to_trash) == 2 + + +def test_empty_header_pruned(deletion_controller, deletion_context, temp_image_files): + """Test that empty cluster headers are pruned after deleting all images in the cluster.""" + ctx = deletion_context + controller = deletion_controller + + # Create header with one image + header = QStandardItem("Cluster 1") + header.setData("cluster_header_1", Qt.ItemDataRole.UserRole) + paths = temp_image_files("a.jpg") + ctx.app_state.add_path(paths[0]) + header.appendRow(make_image_item(paths[0])) + ctx.file_system_model.appendRow(header) + ctx._selected_paths = paths + + # Sanity: header present + assert ctx.file_system_model.invisibleRootItem().rowCount() == 1 + + controller.move_current_image_to_trash() + + # Header should be pruned (rowCount == 0) + assert ctx.file_system_model.invisibleRootItem().rowCount() == 0 + + +def test_mark_for_deletion_then_commit_preserves_selection( + deletion_controller, deletion_context, temp_image_files +): + """Test that marking images for deletion then committing them preserves selection on non-deleted item. + Scenario: 5 images [a, b, c, d, e] where b and d are marked for deletion, user is selected on c. + After committing deletions, selection should remain on c since it wasn't deleted. + """ + from src.ui.controllers.deletion_mark_controller import DeletionMarkController + + ctx = deletion_context + deletion_controller = deletion_controller + mark_controller = DeletionMarkController( + ctx.app_state, ctx.app_state.is_marked_for_deletion + ) + + # Create 5 test images + image_names = ["a.jpg", "b.jpg", "c.jpg", "d.jpg", "e.jpg"] + images = temp_image_files(*image_names) + + for path in images: + ctx.app_state.add_path(path) + ctx.file_system_model.appendRow(make_image_item(path)) + + visible_before = ctx._get_all_visible_image_paths() + assert set(visible_before) == set(images) + + # Step 1: Mark b and d for deletion (non-destructive) + mark_controller.mark(images[1]) # b.jpg + mark_controller.mark(images[3]) # d.jpg + + # Verify they are marked but not deleted + assert ctx.app_state.is_marked_for_deletion(images[1]) + assert ctx.app_state.is_marked_for_deletion(images[3]) + assert not ctx.app_state.is_marked_for_deletion(images[2]) # c.jpg not marked + + # Step 2: Set current selection to c.jpg + ctx.advanced_image_viewer.focused = images[2] # c.jpg is focused + ctx._handle_file_selection_changed([images[2]]) # Set current selection to c.jpg + + # Step 3: Simulate committing marked deletions by deleting the marked files directly + marked_files = ctx.app_state.get_marked_files() + visible_paths_before_delete = ctx._get_all_visible_image_paths() + + # Manually delete the marked files (simulating commit operation) + for marked_file in marked_files: + ctx.app_controller.move_to_trash(marked_file) + ctx.app_state.remove_data_for_path(marked_file) + # Remove from model + proxy_idx = ctx._find_proxy_index_for_path(marked_file) + if proxy_idx.isValid(): + source_idx = ctx.proxy_model.mapToSource(proxy_idx) + if source_idx.isValid(): + ctx.file_system_model.removeRow(source_idx.row(), source_idx.parent()) + + # Step 4: Restore selection after deletion (simulate what happens after commit) + visible_after = ctx._get_all_visible_image_paths() + view = ctx._get_active_file_view() + if view: + deletion_controller._restore_selection_after_delete( + visible_paths_before_delete, visible_after, marked_files, view + ) + + # Verify deletion - b and d should be deleted + assert set(ctx.app_controller.moved_to_trash) == {images[1], images[3]} + + # Verify remaining images + expected_remaining = {images[0], images[2], images[4]} # a, c, e + assert set(visible_after) == expected_remaining + + # Key test: Selection should remain on c since it wasn't deleted + # The selection restoration should prioritize keeping current selection if it's still valid + current_paths = ctx._get_selected_file_paths_from_view() + assert len(current_paths) == 1, "Should have exactly one selected item" + assert current_paths[0] == images[2], "Selection should remain on c.jpg" + + print( + "✅ Test passed: Selection preserved on c.jpg after deleting marked items b.jpg and d.jpg" + ) diff --git a/tests/test_filter_controller.py b/tests/test_filter_controller.py new file mode 100644 index 0000000..7c452f1 --- /dev/null +++ b/tests/test_filter_controller.py @@ -0,0 +1,81 @@ +from PyQt6.QtCore import QSortFilterProxyModel +from PyQt6.QtGui import QStandardItemModel, QStandardItem +from PyQt6.QtWidgets import QApplication +import pytest + +from src.ui.controllers.filter_controller import FilterController, FilterContext + + +class DummyCtx(FilterContext): # type: ignore[misc] + def __init__(self): + self.model = QStandardItemModel() + for name in ["apple.jpg", "banana.png", "cherry.JPG"]: + item = QStandardItem(name) + self.model.appendRow(item) + self.proxy_model = QSortFilterProxyModel() + self.proxy_model.setSourceModel(self.model) + self.app_state = object() + self.refresh_called = 0 + + def refresh_filter(self) -> None: + self.refresh_called += 1 + + +@pytest.fixture(scope="module") +def qapp(): + app = QApplication.instance() or QApplication([]) + return app + + +@pytest.mark.parametrize( + "search_text,expected_matches", + [ + ("apple", 1), + ("banana", 1), + ("png", 1), + ("nonexistent", 0), + ("", 3), # Empty search should match all + ], +) +def test_filter_controller_search_text_filtering(qapp, search_text, expected_matches): + """Test that search text filtering works correctly for different search terms.""" + ctx = DummyCtx() + fc = FilterController(ctx) + # initial state + assert fc.get_rating_filter() == "Show All" + assert fc.get_cluster_filter_id() == -1 + # apply search + fc.set_search_text(search_text) + fc.apply_all(show_folders=False, current_view_mode="list") + # Check filtering results + proxy = ctx.proxy_model + rows = proxy.rowCount() + assert rows == expected_matches + + +def test_filter_controller_initial_state(qapp): + """Test that filter controller initializes with correct default values.""" + ctx = DummyCtx() + fc = FilterController(ctx) + assert fc.get_rating_filter() == "Show All" + assert fc.get_cluster_filter_id() == -1 + + +def test_filter_controller_cluster_filter_setting(qapp): + """Test setting cluster filter and verifying it persists.""" + ctx = DummyCtx() + fc = FilterController(ctx) + fc.set_cluster_filter(5) + assert fc.get_cluster_filter_id() == 5 + + +def test_filter_controller_cluster_filter_idempotent_refresh(qapp): + """Test that setting the same cluster filter multiple times doesn't trigger extra refreshes.""" + ctx = DummyCtx() + fc = FilterController(ctx) + fc.set_cluster_filter(5) + assert fc.get_cluster_filter_id() == 5 + # Setting again to same value should not trigger extra refresh + before = ctx.refresh_called + fc.set_cluster_filter(5) + assert ctx.refresh_called == before diff --git a/tests/test_filter_controller_deferred.py b/tests/test_filter_controller_deferred.py new file mode 100644 index 0000000..428af65 --- /dev/null +++ b/tests/test_filter_controller_deferred.py @@ -0,0 +1,51 @@ +from src.ui.controllers.filter_controller import FilterController + + +class DummyCtx: + # Intentionally no proxy_model at start + def __init__(self): + self.app_state = object() + self.refresh_called = 0 + + def refresh_filter(self): + self.refresh_called += 1 + + +class DummyProxy: + def __init__(self): + self.current_rating_filter = None + self.current_cluster_filter_id = None + self.show_folders_mode_ref = None + self.current_view_mode_ref = None + self._regex = "" + + def setFilterRegularExpression(self, regex): + self._regex = regex + + +def test_deferred_initialization_then_apply_all(): + ctx = DummyCtx() + fc = FilterController(ctx) + # Initial push should be pending because no proxy yet + assert fc._initial_push_pending is True + # Provide proxy later + proxy = DummyProxy() + ctx.proxy_model = proxy # type: ignore + # Call apply_all which should detect pending and initialize then return + fc.apply_all(show_folders=False, current_view_mode="grid") + assert fc._initial_push_pending is False + assert proxy.current_rating_filter == "Show All" + assert proxy.current_cluster_filter_id == -1 + assert ctx.refresh_called == 1 + + +def test_deferred_initialized_via_ensure(): + ctx = DummyCtx() + fc = FilterController(ctx) + proxy = DummyProxy() + ctx.proxy_model = proxy # type: ignore + fc.ensure_initialized(show_folders=True, current_view_mode="list") + assert fc._initial_push_pending is False + assert proxy.show_folders_mode_ref is True + assert proxy.current_view_mode_ref == "list" + assert ctx.refresh_called == 1 diff --git a/tests/test_find_proxy_index_for_path.py b/tests/test_find_proxy_index_for_path.py new file mode 100644 index 0000000..ecc888a --- /dev/null +++ b/tests/test_find_proxy_index_for_path.py @@ -0,0 +1,104 @@ +import os +from PyQt6.QtCore import Qt +from PyQt6.QtGui import QStandardItemModel, QStandardItem +from PyQt6.QtWidgets import QApplication, QTreeView +from src.ui.main_window import MainWindow +from PyQt6.QtCore import QSortFilterProxyModel +from src.ui.app_state import AppState + +# Minimal stubs/mocks may be needed; if QApplication already exists reuse. +app = QApplication.instance() or QApplication([]) + + +class DummyImagePipeline: + def get_preview_qpixmap(self, *a, **k): + return None + + def get_thumbnail_qpixmap(self, *a, **k): + return None + + +class DummyWorkerManager: + pass + + +class DummyAdvancedViewer: + def set_image_data(self, *a, **k): + pass + + def clear(self): + pass + + +# Because MainWindow expects many attributes, we subclass and override initializer heavy parts. +class IdentityProxy(QSortFilterProxyModel): + def mapToSource(self, idx): + return idx + + def mapFromSource(self, source_idx): + return source_idx + + def rowCount(self, parent): + src = self.sourceModel() + if src is None: + return 0 + return src.rowCount(parent) + + def index(self, row, column, parent): + src = self.sourceModel() + if src is None: + return super().index(row, column, parent) + return src.index(row, column, parent) + + +class ProxyIndexTestWindow(MainWindow): # Renamed to avoid pytest collection warning + def __init__(self): + self.app_state = AppState() + self.file_system_model = QStandardItemModel() + self.proxy_model = IdentityProxy() + self.proxy_model.setSourceModel(self.file_system_model) + tv = QTreeView() + tv.setModel(self.proxy_model) + self._active_file_view = tv + + def _get_active_file_view(self): + return self._active_file_view + + def _is_valid_image_item(self, proxy_idx): + item = self.file_system_model.itemFromIndex(proxy_idx) + if not item: + return False + d = item.data(Qt.ItemDataRole.UserRole) + return isinstance(d, dict) and "path" in d + + +def make_item(path): + it = QStandardItem(os.path.basename(path)) + it.setData({"path": path}, Qt.ItemDataRole.UserRole) + return it + + +def test_find_proxy_index_simple(tmp_path): + mw = ProxyIndexTestWindow() + # Build simple tree: root items with image data + p1 = tmp_path / "a.jpg" + p1.write_text("x") + p2 = tmp_path / "b.jpg" + p2.write_text("y") + mw.file_system_model.appendRow(make_item(str(p1))) + mw.file_system_model.appendRow(make_item(str(p2))) + idx = mw._find_proxy_index_for_path(str(p2)) + assert idx.isValid() + # In real code we map to source; here identity but keep explicit for clarity + item = mw.file_system_model.itemFromIndex(mw.proxy_model.mapToSource(idx)) + assert item.text() == "b.jpg" + + +def test_find_proxy_index_not_found(tmp_path): + mw = ProxyIndexTestWindow() + p1 = tmp_path / "c.jpg" + p1.write_text("x") + mw.file_system_model.appendRow(make_item(str(p1))) + idx = mw._find_proxy_index_for_path(str(tmp_path / "missing.jpg")) + assert not idx.isValid() + assert not idx.isValid() diff --git a/tests/test_group_left_right_navigation.py b/tests/test_group_left_right_navigation.py new file mode 100644 index 0000000..b31f9c7 --- /dev/null +++ b/tests/test_group_left_right_navigation.py @@ -0,0 +1,175 @@ +import os +from PyQt6.QtCore import QModelIndex, Qt +from PyQt6.QtGui import QStandardItemModel, QStandardItem + +from src.ui.controllers.navigation_controller import NavigationController + + +class StubView: + def __init__(self): + self._current = QModelIndex() + + def currentIndex(self): + return self._current + + def setCurrentIndex(self, idx: QModelIndex): + self._current = idx + + +class Ctx: + """Minimal NavigationContext for group navigation tests.""" + + def __init__(self, model: QStandardItemModel): + self.model = model + self.view = StubView() + + # NavigationContext API + def get_active_view(self): + return self.view + + def is_valid_image_index(self, proxy_index: QModelIndex) -> bool: + item = self.model.itemFromIndex(proxy_index) + if not item: + return False + d = item.data(Qt.ItemDataRole.UserRole) + return isinstance(d, dict) and "path" in d + + def map_to_source(self, proxy_index: QModelIndex) -> QModelIndex: + return proxy_index + + def item_from_source(self, source_index: QModelIndex): + return self.model.itemFromIndex(source_index) + + def get_group_sibling_images(self, current_proxy_index: QModelIndex): + parent = current_proxy_index.parent() + count = self.model.rowCount(parent) + group_indices = [self.model.index(r, 0, parent) for r in range(count)] + return parent, group_indices, None + + def find_first_visible_item(self) -> QModelIndex: + # Not used in group tests (we always have a group), but implement for completeness + root = QModelIndex() + for r in range(self.model.rowCount(root)): + idx = self.model.index(r, 0, root) + if self.is_valid_image_index(idx): + return idx + # descend into children + for cr in range(self.model.rowCount(idx)): + cidx = self.model.index(cr, 0, idx) + if self.is_valid_image_index(cidx): + return cidx + return QModelIndex() + + def find_proxy_index_for_path(self, path: str) -> QModelIndex: + # Linear scan through tree (small test models) + root = QModelIndex() + for r in range(self.model.rowCount(root)): + idx = self.model.index(r, 0, root) + for cr in range(self.model.rowCount(idx)): + cidx = self.model.index(cr, 0, idx) + item = self.model.itemFromIndex(cidx) + if not item: + continue + d = item.data(Qt.ItemDataRole.UserRole) + if isinstance(d, dict) and d.get("path") == path: + return cidx + return QModelIndex() + + def get_all_visible_image_paths(self): + # Not used in group navigation tests + return [] + + def get_marked_deleted(self): + return [] + + def validate_and_select_image_candidate( + self, proxy_index: QModelIndex, direction: str, log_skip: bool + ): + self.view.setCurrentIndex(proxy_index) + + +def make_image_item(path: str): + it = QStandardItem(os.path.basename(path)) + it.setData({"path": path}, Qt.ItemDataRole.UserRole) + return it + + +def test_right_wraps_within_group(tmp_path): + model = QStandardItemModel() + header = QStandardItem("Group 1") + header.setData("cluster_header_1", Qt.ItemDataRole.UserRole) + a = tmp_path / "a.jpg" + b = tmp_path / "b.jpg" + a.write_text("x") + b.write_text("x") + header.appendRow(make_image_item(str(a))) + header.appendRow(make_image_item(str(b))) + model.appendRow(header) + + ctx = Ctx(model) + nav = NavigationController(ctx) + # start at last image in group + last_idx = header.child(1).index() + ctx.view.setCurrentIndex(last_idx) + + nav.navigate_group("right", skip_deleted=True) + cur = ctx.view.currentIndex() + assert model.itemFromIndex(cur).text() == "a.jpg" + + +def test_left_wraps_within_group(tmp_path): + model = QStandardItemModel() + header = QStandardItem("Group 1") + header.setData("cluster_header_1", Qt.ItemDataRole.UserRole) + a = tmp_path / "a.jpg" + b = tmp_path / "b.jpg" + c = tmp_path / "c.jpg" + for p in (a, b, c): + p.write_text("x") + header.appendRow(make_image_item(str(a))) + header.appendRow(make_image_item(str(b))) + header.appendRow(make_image_item(str(c))) + model.appendRow(header) + + ctx = Ctx(model) + nav = NavigationController(ctx) + # start at first image + first_idx = header.child(0).index() + ctx.view.setCurrentIndex(first_idx) + + nav.navigate_group("left", skip_deleted=True) + cur = ctx.view.currentIndex() + assert model.itemFromIndex(cur).text() == "c.jpg" + + +def test_right_does_not_move_to_next_group(tmp_path): + model = QStandardItemModel() + g1 = QStandardItem("Group 1") + g1.setData("cluster_header_1", Qt.ItemDataRole.UserRole) + a = tmp_path / "a.jpg" + a.write_text("x") + b = tmp_path / "b.jpg" + b.write_text("x") + g1.appendRow(make_image_item(str(a))) + g1.appendRow(make_image_item(str(b))) + g2 = QStandardItem("Group 2") + g2.setData("cluster_header_2", Qt.ItemDataRole.UserRole) + c = tmp_path / "c.jpg" + c.write_text("x") + d = tmp_path / "d.jpg" + d.write_text("x") + g2.appendRow(make_image_item(str(c))) + g2.appendRow(make_image_item(str(d))) + model.appendRow(g1) + model.appendRow(g2) + + ctx = Ctx(model) + nav = NavigationController(ctx) + # start at last item of first group + last_g1 = g1.child(1).index() + ctx.view.setCurrentIndex(last_g1) + + nav.navigate_group("right", skip_deleted=True) + cur = ctx.view.currentIndex() + # Should wrap within group 1, not move to c.jpg in group 2 + assert model.itemFromIndex(cur).text() == "a.jpg" diff --git a/tests/test_hotkey_controller_integration.py b/tests/test_hotkey_controller_integration.py new file mode 100644 index 0000000..af1f7e0 --- /dev/null +++ b/tests/test_hotkey_controller_integration.py @@ -0,0 +1,38 @@ +from PyQt6.QtCore import Qt +from src.ui.controllers.hotkey_controller import HotkeyController + + +class DummyCtx: + def __init__(self): + self.calls = [] + + # Provide required methods; capture skip_deleted + def navigate_left_in_group(self, skip_deleted=True): + self.calls.append(("left", skip_deleted)) + + def navigate_right_in_group(self, skip_deleted=True): + self.calls.append(("right", skip_deleted)) + + def navigate_up_sequential(self, skip_deleted=True): + self.calls.append(("up", skip_deleted)) + + def navigate_down_sequential(self, skip_deleted=True): + self.calls.append(("down", skip_deleted)) + + # smart variants no longer used for arrow keys + def navigate_down_smart(self, skip_deleted=True): + self.calls.append(("down_smart", skip_deleted)) + + def navigate_up_smart(self, skip_deleted=True): + self.calls.append(("up_smart", skip_deleted)) + + +def test_hotkey_controller_skip_deleted_flag_propagates(): + ctx = DummyCtx() + hk = HotkeyController(ctx) + # Simulate pressing Down with skip_deleted False -> sequential mapping + hk.handle_key(Qt.Key.Key_Down, skip_deleted=False) + assert ctx.calls[-1] == ("down", False) + # Up key propagation + hk.handle_key(Qt.Key.Key_Up, skip_deleted=False) + assert ctx.calls[-1] == ("up", False) diff --git a/tests/test_image_rotation.py b/tests/test_image_rotation.py deleted file mode 100644 index af95fd3..0000000 --- a/tests/test_image_rotation.py +++ /dev/null @@ -1,467 +0,0 @@ -import pyexiv2 # noqa: F401 # Must be first to avoid Windows crash with pyexiv2 -import pytest -import os -import sys -import tempfile -import shutil -from unittest.mock import Mock -import logging - -# Add the project root to Python path so we can import src modules -project_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) -if project_root not in sys.path: - sys.path.insert(0, project_root) - -# Try to import the modules we need -try: - from src.core.image_processing.image_rotator import ImageRotator - from src.core.metadata_processor import MetadataProcessor - from src.core.caching.exif_cache import ExifCache - - IMPORTS_AVAILABLE = True - IMPORT_ERROR = None -except ImportError as e: - IMPORTS_AVAILABLE = False - IMPORT_ERROR = str(e) - - # Create dummy classes to avoid NameError - class ImageRotator: - pass - - class MetadataProcessor: - pass - - class ExifCache: - pass - - -class TestImageRotator: - """Tests for the ImageRotator class.""" - - @classmethod - def setup_class(cls): - """Setup test environment with sample images.""" - cls.test_folder = "tests/samples" - cls.sample_images = [] - - # Find test images - if os.path.exists(cls.test_folder): - for filename in os.listdir(cls.test_folder): - if filename.lower().endswith( - (".png", ".jpg", ".jpeg", ".arw", ".cr2", ".nef") - ): - cls.sample_images.append(os.path.join(cls.test_folder, filename)) - - if len(cls.sample_images) == 0: - pytest.skip( - f"No test images found in {cls.test_folder}", allow_module_level=True - ) - - def setup_method(self): - """Setup for each test method.""" - self.rotator = ImageRotator() - self.temp_files = [] - - def teardown_method(self): - """Clean up temporary files.""" - for temp_file in self.temp_files: - if os.path.exists(temp_file): - try: - os.unlink(temp_file) - except Exception: - pass - - def _create_temp_copy(self, source_path: str) -> str: - """Create a temporary copy of an image for testing.""" - _, ext = os.path.splitext(source_path) - with tempfile.NamedTemporaryFile(suffix=ext, delete=False) as temp_file: - temp_path = temp_file.name - - shutil.copy2(source_path, temp_path) - self.temp_files.append(temp_path) - return temp_path - - def test_rotator_initialization(self): - """Test that ImageRotator initializes correctly.""" - assert isinstance(self.rotator, ImageRotator) - # jpegtran availability might vary by system - assert isinstance(self.rotator.jpegtran_available, bool) - - def test_get_supported_formats(self): - """Test that supported formats are returned correctly.""" - formats = self.rotator.get_supported_formats() - assert isinstance(formats, list) - assert ".jpg" in formats - assert ".png" in formats - assert ".tiff" in formats or ".tif" in formats - - def test_is_rotation_supported(self): - """Test rotation support checking for different formats.""" - # Test supported formats (pixel rotation) - assert self.rotator.is_rotation_supported("test.jpg") - assert self.rotator.is_rotation_supported("test.jpeg") - assert self.rotator.is_rotation_supported("test.png") - assert self.rotator.is_rotation_supported("test.tiff") - - # Test supported RAW formats (metadata-only rotation) - assert self.rotator.is_rotation_supported("test.arw") - assert self.rotator.is_rotation_supported("test.cr2") - assert self.rotator.is_rotation_supported("test.nef") - assert self.rotator.is_rotation_supported("test.dng") - - # Test unsupported formats - assert not self.rotator.is_rotation_supported("test.txt") - assert not self.rotator.is_rotation_supported("test.mp4") - - def test_get_current_orientation(self): - """Test reading current EXIF orientation.""" - if not self.sample_images: - pytest.skip("No sample images available") - - for image_path in self.sample_images: - orientation = self.rotator._get_current_orientation(image_path) - assert isinstance(orientation, int) - assert 1 <= orientation <= 8 # Valid EXIF orientation range - - def test_calculate_new_orientation(self): - """Test orientation calculation for different rotations.""" - # Test clockwise rotation from normal orientation - assert self.rotator._calculate_new_orientation(1, "clockwise") == 6 - assert self.rotator._calculate_new_orientation(1, "counterclockwise") == 8 - assert self.rotator._calculate_new_orientation(1, "180") == 3 - - # Test full rotation cycle (clockwise) - orientation = 1 - for _ in range(4): - orientation = self.rotator._calculate_new_orientation( - orientation, "clockwise" - ) - assert orientation == 1 # Should return to original after 4 rotations - - def test_rotation_with_sample_images(self): - """Test actual rotation on sample images.""" - if not self.sample_images: - pytest.skip("No sample images available") - - # Test with first available image - source_image = self.sample_images[0] - temp_image = self._create_temp_copy(source_image) - - # Test clockwise rotation - success, message = self.rotator.rotate_clockwise(temp_image) - logging.info(f"Clockwise rotation: {success}, {message}") - - if success: - # Verify file still exists and is valid - assert os.path.exists(temp_image) - assert os.path.getsize(temp_image) > 0 - - def test_metadata_only_rotation(self): - """Test metadata-only rotation (no pixel changes).""" - if not self.sample_images: - pytest.skip("No sample images available") - - source_image = self.sample_images[0] - temp_image = self._create_temp_copy(source_image) - - # Get original file size - original_size = os.path.getsize(temp_image) - - # Perform metadata-only rotation - success, message = self.rotator.rotate_image( - temp_image, "clockwise", update_metadata_only=True - ) - logging.info(f"Metadata-only rotation: {success}, {message}") - - # File should still exist with same or very similar size - assert os.path.exists(temp_image) - new_size = os.path.getsize(temp_image) - # Allow small size difference due to metadata changes - assert abs(new_size - original_size) < 1024 # Less than 1KB difference - - def test_rotation_error_handling(self): - """Test error handling for invalid inputs.""" - # Test with non-existent file - success, message = self.rotator.rotate_image( - "/nonexistent/file.jpg", "clockwise" - ) - assert not success - assert "not found" in message.lower() - - # Test with invalid direction - if self.sample_images: - temp_image = self._create_temp_copy(self.sample_images[0]) - success, message = self.rotator.rotate_image( - temp_image, "invalid_direction" - ) - assert not success - - -class TestMetadataProcessorRotation: - """Tests for MetadataProcessor rotation methods.""" - - @classmethod - def setup_class(cls): - """Setup test environment.""" - cls.test_folder = "tests/samples" - cls.sample_images = [] - - if os.path.exists(cls.test_folder): - for filename in os.listdir(cls.test_folder): - if filename.lower().endswith( - (".png", ".jpg", ".jpeg", ".arw", ".cr2", ".nef") - ): - cls.sample_images.append(os.path.join(cls.test_folder, filename)) - - if len(cls.sample_images) == 0: - pytest.skip( - f"No test images found in {cls.test_folder}", allow_module_level=True - ) - - def setup_method(self): - """Setup for each test method.""" - self.exif_cache = Mock(spec=ExifCache) - self.temp_files = [] - - def teardown_method(self): - """Clean up temporary files.""" - for temp_file in self.temp_files: - if os.path.exists(temp_file): - try: - os.unlink(temp_file) - except Exception: - pass - - def _create_temp_copy(self, source_path: str) -> str: - """Create a temporary copy of an image for testing.""" - _, ext = os.path.splitext(source_path) - with tempfile.NamedTemporaryFile(suffix=ext, delete=False) as temp_file: - temp_path = temp_file.name - - shutil.copy2(source_path, temp_path) - self.temp_files.append(temp_path) - return temp_path - - def test_is_rotation_supported(self): - """Test MetadataProcessor.is_rotation_supported method.""" - if not self.sample_images: - pytest.skip("No sample images available") - - for image_path in self.sample_images: - supported = MetadataProcessor.is_rotation_supported(image_path) - assert isinstance(supported, bool) - - # Check based on file extension - ext = os.path.splitext(image_path)[1].lower() - if ext in [".jpg", ".jpeg", ".png", ".tiff", ".tif"]: - assert supported - logging.info( - f"Rotation support for {os.path.basename(image_path)} ({ext}): {supported}" - ) - - def test_rotate_clockwise(self): - """Test MetadataProcessor.rotate_clockwise method.""" - if not self.sample_images: - pytest.skip("No sample images available") - - source_image = self.sample_images[0] - temp_image = self._create_temp_copy(source_image) - - success = MetadataProcessor.rotate_clockwise( - temp_image, exif_disk_cache=self.exif_cache - ) - - # Check that cache was invalidated if rotation was successful - if success: - assert os.path.exists(temp_image) - self.exif_cache.delete.assert_called_once() - - print(f"Clockwise rotation of {os.path.basename(source_image)}: {success}") - - def test_rotate_counterclockwise(self): - """Test MetadataProcessor.rotate_counterclockwise method.""" - if not self.sample_images: - pytest.skip("No sample images available") - - source_image = self.sample_images[0] - temp_image = self._create_temp_copy(source_image) - - success = MetadataProcessor.rotate_counterclockwise( - temp_image, exif_disk_cache=self.exif_cache - ) - - if success: - assert os.path.exists(temp_image) - self.exif_cache.delete.assert_called_once() - - logging.info( - f"Counterclockwise rotation of {os.path.basename(source_image)}: {success}" - ) - - def test_rotate_180(self): - """Test MetadataProcessor.rotate_180 method.""" - if not self.sample_images: - pytest.skip("No sample images available") - - source_image = self.sample_images[0] - temp_image = self._create_temp_copy(source_image) - - success = MetadataProcessor.rotate_180( - temp_image, exif_disk_cache=self.exif_cache - ) - - if success: - assert os.path.exists(temp_image) - self.exif_cache.delete.assert_called_once() - - logging.info(f"180° rotation of {os.path.basename(source_image)}: {success}") - - def test_rotation_with_metadata_only(self): - """Test rotation with metadata_only flag.""" - if not self.sample_images: - pytest.skip("No sample images available") - - source_image = self.sample_images[0] - temp_image = self._create_temp_copy(source_image) - - # Get original file info - original_size = os.path.getsize(temp_image) - - success = MetadataProcessor.rotate_image( - temp_image, - "clockwise", - update_metadata_only=True, - exif_disk_cache=self.exif_cache, - ) - - if success: - # File should exist and cache should be invalidated - assert os.path.exists(temp_image) - self.exif_cache.delete.assert_called_once() - - # File size should be similar (metadata changes only) - new_size = os.path.getsize(temp_image) - assert abs(new_size - original_size) < 1024 # Less than 1KB difference - - logging.info( - f"Metadata-only rotation of {os.path.basename(source_image)}: {success}" - ) - - def test_error_handling(self): - """Test error handling in MetadataProcessor rotation methods.""" - # Test with non-existent file - success = MetadataProcessor.rotate_clockwise("/nonexistent/file.jpg") - assert not success - - # Test with invalid path - success = MetadataProcessor.rotate_counterclockwise("") - assert not success - - -class TestRotationIntegration: - """Integration tests for the complete rotation system.""" - - @classmethod - def setup_class(cls): - """Setup test environment.""" - cls.test_folder = "tests/samples" - cls.sample_images = [] - - if os.path.exists(cls.test_folder): - for filename in os.listdir(cls.test_folder): - if filename.lower().endswith((".png", ".jpg", ".jpeg")): - cls.sample_images.append(os.path.join(cls.test_folder, filename)) - - if len(cls.sample_images) == 0: - pytest.skip( - f"No test images found in {cls.test_folder}", allow_module_level=True - ) - - def setup_method(self): - """Setup for each test method.""" - self.temp_files = [] - - def teardown_method(self): - """Clean up temporary files.""" - for temp_file in self.temp_files: - if os.path.exists(temp_file): - try: - os.unlink(temp_file) - except Exception: - pass - - def _create_temp_copy(self, source_path: str) -> str: - """Create a temporary copy of an image for testing.""" - _, ext = os.path.splitext(source_path) - with tempfile.NamedTemporaryFile(suffix=ext, delete=False) as temp_file: - temp_path = temp_file.name - - shutil.copy2(source_path, temp_path) - self.temp_files.append(temp_path) - return temp_path - - def test_full_rotation_cycle(self): - """Test that 4 clockwise rotations return to original orientation.""" - if not self.sample_images: - pytest.skip("No sample images available") - - source_image = self.sample_images[0] - temp_image = self._create_temp_copy(source_image) - - # Get original metadata - - # Perform 4 clockwise rotations - for i in range(4): - success = MetadataProcessor.rotate_clockwise(temp_image) - assert success, f"Rotation {i + 1} failed" - assert os.path.exists(temp_image), f"File missing after rotation {i + 1}" - - # Check that we're back to original orientation - final_metadata = MetadataProcessor.get_batch_display_metadata([temp_image]) - final_detailed = MetadataProcessor.get_detailed_metadata(temp_image) - - # The image should still be valid - assert final_metadata is not None - assert final_detailed is not None - - logging.info( - f"Full rotation cycle completed for {os.path.basename(source_image)}" - ) - - def test_mixed_rotations(self): - """Test different rotation combinations.""" - if not self.sample_images: - pytest.skip("No sample images available") - - source_image = self.sample_images[0] - temp_image = self._create_temp_copy(source_image) - - # Test sequence: clockwise -> 180 -> counterclockwise -> 180 - rotations = [ - (MetadataProcessor.rotate_clockwise, "clockwise"), - (MetadataProcessor.rotate_180, "180°"), - (MetadataProcessor.rotate_counterclockwise, "counterclockwise"), - (MetadataProcessor.rotate_180, "180°"), - ] - - for rotate_func, description in rotations: - success = rotate_func(temp_image) - assert success, f"{description} rotation failed" - assert os.path.exists(temp_image), ( - f"File missing after {description} rotation" - ) - - # Verify image is still readable - metadata = MetadataProcessor.get_detailed_metadata(temp_image) - assert metadata is not None, ( - f"Cannot read metadata after {description} rotation" - ) - - logging.info( - f"Mixed rotation sequence completed for {os.path.basename(source_image)}" - ) - - -if __name__ == "__main__": - # Run tests with verbose output - pytest.main([__file__, "-v", "-s"]) diff --git a/tests/test_image_rotator.py b/tests/test_image_rotator.py new file mode 100644 index 0000000..6bfce9c --- /dev/null +++ b/tests/test_image_rotator.py @@ -0,0 +1,203 @@ +import pyexiv2 # noqa: F401 # Must be first to avoid Windows crash with pyexiv2 +import pytest +import os +import sys +import tempfile +import shutil +import logging + +# Add the project root to Python path so we can import src modules +project_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +if project_root not in sys.path: + sys.path.insert(0, project_root) + +# Try to import the modules we need +try: + from src.core.image_processing.image_rotator import ImageRotator + + IMPORTS_AVAILABLE = True + IMPORT_ERROR = None +except ImportError as e: + IMPORTS_AVAILABLE = False + IMPORT_ERROR = str(e) + + # Create dummy classes to avoid NameError + class ImageRotator: + pass + + +class TestImageRotator: + """Tests for the ImageRotator class.""" + + @classmethod + def setup_class(cls): + """Setup test environment with sample images.""" + cls.test_folder = "tests/samples" + cls.sample_images = [] + + # Find test images + if os.path.exists(cls.test_folder): + for filename in os.listdir(cls.test_folder): + if filename.lower().endswith( + (".png", ".jpg", ".jpeg", ".arw", ".cr2", ".nef") + ): + cls.sample_images.append(os.path.join(cls.test_folder, filename)) + + if len(cls.sample_images) == 0: + pytest.skip( + f"No test images found in {cls.test_folder}", allow_module_level=True + ) + + def setup_method(self): + """Setup for each test method.""" + self.rotator = ImageRotator() + self.temp_files = [] + + def teardown_method(self): + """Clean up temporary files.""" + for temp_file in self.temp_files: + if os.path.exists(temp_file): + try: + os.unlink(temp_file) + except Exception: + pass + + def _create_temp_copy(self, source_path: str) -> str: + """Create a temporary copy of an image for testing.""" + _, ext = os.path.splitext(source_path) + with tempfile.NamedTemporaryFile(suffix=ext, delete=False) as temp_file: + temp_path = temp_file.name + + shutil.copy2(source_path, temp_path) + self.temp_files.append(temp_path) + return temp_path + + def test_rotator_initialization(self): + """Test that ImageRotator initializes correctly.""" + assert isinstance(self.rotator, ImageRotator) + # jpegtran availability might vary by system + assert isinstance(self.rotator.jpegtran_available, bool) + + def test_get_supported_formats(self): + """Test that supported formats are returned correctly.""" + formats = self.rotator.get_supported_formats() + assert isinstance(formats, list) + assert ".jpg" in formats + assert ".png" in formats + assert ".tiff" in formats or ".tif" in formats + + @pytest.mark.parametrize( + "filename,expected", + [ + ("test.jpg", True), + ("test.jpeg", True), + ("test.png", True), + ("test.tiff", True), + ("test.arw", True), + ("test.cr2", True), + ("test.nef", True), + ("test.dng", True), + ("test.txt", False), + ("test.mp4", False), + ], + ) + def test_is_rotation_supported(self, filename, expected): + """Test rotation support checking for different formats.""" + assert self.rotator.is_rotation_supported(filename) == expected + + def test_get_current_orientation_with_sample_images(self): + """Test reading current EXIF orientation from sample images.""" + if not self.sample_images: + pytest.skip("No sample images available") + + for image_path in self.sample_images: + orientation = self.rotator._get_current_orientation(image_path) + assert isinstance(orientation, int) + assert 1 <= orientation <= 8 # Valid EXIF orientation range + + @pytest.mark.parametrize( + "initial,direction,expected", + [ + (1, "clockwise", 6), + (1, "counterclockwise", 8), + (1, "180", 3), + (3, "clockwise", 8), + (6, "clockwise", 3), + (8, "clockwise", 1), + ], + ) + def test_calculate_new_orientation(self, initial, direction, expected): + """Test orientation calculation for different rotations.""" + assert self.rotator._calculate_new_orientation(initial, direction) == expected + + def test_calculate_new_orientation_full_cycle(self): + """Test that 4 clockwise rotations return to original orientation.""" + orientation = 1 + for _ in range(4): + orientation = self.rotator._calculate_new_orientation( + orientation, "clockwise" + ) + assert orientation == 1 # Should return to original after 4 rotations + + def test_rotation_with_sample_images_clockwise(self): + """Test clockwise rotation on sample images.""" + if not self.sample_images: + pytest.skip("No sample images available") + + # Test with first available image + source_image = self.sample_images[0] + temp_image = self._create_temp_copy(source_image) + + # Test clockwise rotation + success, message = self.rotator.rotate_clockwise(temp_image) + logging.info(f"Clockwise rotation: {success}, {message}") + + if success: + # Verify file still exists and is valid + assert os.path.exists(temp_image) + assert os.path.getsize(temp_image) > 0 + + def test_metadata_only_rotation(self): + """Test metadata-only rotation (no pixel changes).""" + if not self.sample_images: + pytest.skip("No sample images available") + + source_image = self.sample_images[0] + temp_image = self._create_temp_copy(source_image) + + # Get original file size + original_size = os.path.getsize(temp_image) + + # Perform metadata-only rotation + success, message = self.rotator.rotate_image( + temp_image, "clockwise", update_metadata_only=True + ) + logging.info(f"Metadata-only rotation: {success}, {message}") + + # File should still exist with same or very similar size + assert os.path.exists(temp_image) + new_size = os.path.getsize(temp_image) + # Allow small size difference due to metadata changes + assert abs(new_size - original_size) < 1024 # Less than 1KB difference + + def test_rotation_error_handling_nonexistent_file(self): + """Test error handling for non-existent file.""" + success, message = self.rotator.rotate_image( + "/nonexistent/file.jpg", "clockwise" + ) + assert not success + assert "not found" in message.lower() + + def test_rotation_error_handling_invalid_direction(self): + """Test error handling for invalid rotation direction.""" + if self.sample_images: + temp_image = self._create_temp_copy(self.sample_images[0]) + success, message = self.rotator.rotate_image( + temp_image, "invalid_direction" + ) + assert not success + + +if __name__ == "__main__": + # Run tests with verbose output + pytest.main([__file__, "-v", "-s"]) diff --git a/tests/test_index_lookup_utils.py b/tests/test_index_lookup_utils.py new file mode 100644 index 0000000..fd51989 --- /dev/null +++ b/tests/test_index_lookup_utils.py @@ -0,0 +1,86 @@ +from PyQt6.QtGui import QStandardItemModel, QStandardItem +from PyQt6.QtCore import Qt, QSortFilterProxyModel +from PyQt6.QtWidgets import QApplication, QTreeView +from src.ui.helpers.index_lookup_utils import ( + find_proxy_index_for_path, + classify_selection, +) + +app = QApplication.instance() or QApplication([]) + + +class IdentityProxy(QSortFilterProxyModel): + def mapToSource(self, idx): + return idx + + def mapFromSource(self, sidx): + return sidx + + def rowCount(self, parent): + src = self.sourceModel() + if src is None: + return 0 + return src.rowCount(parent) + + def index(self, row, column, parent): + src = self.sourceModel() + if src is None: + return super().index(row, column, parent) + return src.index(row, column, parent) + + +def _is_valid(idx, src_model: QStandardItemModel): + item = src_model.itemFromIndex(idx) + if not item: + return False + data = item.data(Qt.ItemDataRole.UserRole) + return isinstance(data, dict) and "path" in data + + +def make_item(path: str): + it = QStandardItem(path.split("/")[-1]) + it.setData({"path": path}, Qt.ItemDataRole.UserRole) + return it + + +def test_find_proxy_index_basic(tmp_path): + src = QStandardItemModel() + proxy = IdentityProxy() + proxy.setSourceModel(src) + view = QTreeView() + view.setModel(proxy) + p1 = tmp_path / "a.jpg" + p1.write_text("x") + p2 = tmp_path / "b.jpg" + p2.write_text("y") + src.appendRow(make_item(str(p1))) + src.appendRow(make_item(str(p2))) + idx = find_proxy_index_for_path( + str(p2), proxy, src, lambda i: _is_valid(i, src), view.isExpanded + ) + assert idx.isValid() + + +def test_find_proxy_index_missing(tmp_path): + src = QStandardItemModel() + proxy = IdentityProxy() + proxy.setSourceModel(src) + view = QTreeView() + view.setModel(proxy) + p1 = tmp_path / "a.jpg" + p1.write_text("x") + src.appendRow(make_item(str(p1))) + idx = find_proxy_index_for_path( + str(tmp_path / "missing.jpg"), + proxy, + src, + lambda i: _is_valid(i, src), + view.isExpanded, + ) + assert not idx.isValid() + + +def test_classify_selection(): + assert classify_selection([]) == ("none", 0) + assert classify_selection(["a"]) == ("single", 1) + assert classify_selection(["a", "b"]) == ("multi", 2) diff --git a/tests/test_metadata_controller.py b/tests/test_metadata_controller.py new file mode 100644 index 0000000..f595254 --- /dev/null +++ b/tests/test_metadata_controller.py @@ -0,0 +1,89 @@ +import pytest +from src.ui.controllers.metadata_controller import MetadataController + + +class DummySidebar: + def __init__(self): + self.updated = [] + + def update_selection(self, paths): + self.updated.append(paths) + + +class DummyCtx: + def __init__(self): + self.metadata_sidebar = DummySidebar() + self.sidebar_visible = True + self._paths = [] + + def get_selected_file_paths(self): + return self._paths + + def ensure_metadata_sidebar(self): + pass + + +@pytest.mark.parametrize( + "initial_paths,new_paths,expected_final_length", + [ + (["a.jpg", "b.jpg"], ["a.jpg", "b.jpg"], 1), # Same selection, no extra update + (["a.jpg", "b.jpg"], ["b.jpg"], 2), # Changed selection, should update + (["a.jpg"], ["a.jpg", "b.jpg", "c.jpg"], 2), # Multiple files + ( + [], + ["new.jpg"], + 1, + ), # From empty to single - only updates when changing TO non-empty + ], +) +def test_metadata_refresh_updates_on_selection_change( + initial_paths, new_paths, expected_final_length +): + """Test that metadata sidebar updates when selection changes but not when it stays the same.""" + ctx = DummyCtx() + mc = MetadataController(ctx) + + # Set initial selection and refresh + ctx._paths = initial_paths + mc.refresh_for_selection() + + # Verify initial update (only if initial_paths is non-empty) + initial_update_count = 1 if initial_paths else 0 + assert len(ctx.metadata_sidebar.updated) == initial_update_count + if initial_paths: + assert ctx.metadata_sidebar.updated[0] == initial_paths + + # Change selection and refresh again + ctx._paths = new_paths + mc.refresh_for_selection() + + # Should have expected number of total updates + assert len(ctx.metadata_sidebar.updated) == expected_final_length + if expected_final_length > initial_update_count: + assert ctx.metadata_sidebar.updated[-1] == new_paths + + +def test_metadata_refresh_skipped_when_sidebar_hidden(): + """Test that metadata refresh is skipped when sidebar is not visible.""" + ctx = DummyCtx() + ctx.sidebar_visible = False + mc = MetadataController(ctx) + ctx._paths = ["x.jpg"] + mc.refresh_for_selection() + assert ctx.metadata_sidebar.updated == [] + + +@pytest.mark.parametrize("sidebar_visible", [True, False]) +def test_metadata_sidebar_visibility_behavior(sidebar_visible): + """Test metadata controller behavior with different sidebar visibility states.""" + ctx = DummyCtx() + ctx.sidebar_visible = sidebar_visible + mc = MetadataController(ctx) + ctx._paths = ["test.jpg"] + mc.refresh_for_selection() + + if sidebar_visible: + assert len(ctx.metadata_sidebar.updated) == 1 + assert ctx.metadata_sidebar.updated[0] == ["test.jpg"] + else: + assert ctx.metadata_sidebar.updated == [] diff --git a/tests/test_metadata_processor_constants.py b/tests/test_metadata_processor_constants.py new file mode 100644 index 0000000..54c2569 --- /dev/null +++ b/tests/test_metadata_processor_constants.py @@ -0,0 +1,56 @@ +import pyexiv2 # noqa: F401 # This must be the first import or else it will cause a silent crash on windows +import pytest +import os +import sys + +# Add the project root to Python path so we can import src modules +project_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +if project_root not in sys.path: + sys.path.insert(0, project_root) + +# Try to import the modules we need +try: + from src.core.metadata_processor import ( + DATE_TAGS_PREFERENCE, + COMPREHENSIVE_METADATA_TAGS, + ) + + IMPORTS_AVAILABLE = True + IMPORT_ERROR = None +except ImportError as e: + IMPORTS_AVAILABLE = False + IMPORT_ERROR = str(e) + + # Create dummy constants to avoid NameError + DATE_TAGS_PREFERENCE = [] + COMPREHENSIVE_METADATA_TAGS = [] + + +class TestConstants: + """Test that constants are properly defined.""" + + def test_date_tags_preference(self): + """Test DATE_TAGS_PREFERENCE is properly defined.""" + assert isinstance(DATE_TAGS_PREFERENCE, list) + assert len(DATE_TAGS_PREFERENCE) > 0 + assert all(isinstance(tag, str) for tag in DATE_TAGS_PREFERENCE) + + # Should include common date tags + expected_tags = ["Exif.Photo.DateTimeOriginal", "Xmp.xmp.CreateDate"] + for tag in expected_tags: + assert tag in DATE_TAGS_PREFERENCE + + def test_comprehensive_metadata_tags(self): + """Test COMPREHENSIVE_METADATA_TAGS is properly defined.""" + assert isinstance(COMPREHENSIVE_METADATA_TAGS, list) + assert len(COMPREHENSIVE_METADATA_TAGS) > 0 + assert all(isinstance(tag, str) for tag in COMPREHENSIVE_METADATA_TAGS) + + # Should include rating and label tags + assert "Xmp.xmp.Rating" in COMPREHENSIVE_METADATA_TAGS + assert "Xmp.xmp.Label" in COMPREHENSIVE_METADATA_TAGS + + +if __name__ == "__main__": + # Run tests with verbose output + pytest.main([__file__, "-v", "-s"]) diff --git a/tests/test_metadata_processor.py b/tests/test_metadata_processor_core.py similarity index 68% rename from tests/test_metadata_processor.py rename to tests/test_metadata_processor_core.py index 0fb14d1..e930cf3 100644 --- a/tests/test_metadata_processor.py +++ b/tests/test_metadata_processor_core.py @@ -16,14 +16,7 @@ # Try to import the modules we need try: - from src.core.metadata_processor import ( - MetadataProcessor, - _parse_exif_date, - _parse_date_from_filename, - _parse_rating, - DATE_TAGS_PREFERENCE, - COMPREHENSIVE_METADATA_TAGS, - ) + from src.core.metadata_processor import MetadataProcessor from src.core.caching.rating_cache import RatingCache from src.core.caching.exif_cache import ExifCache @@ -43,18 +36,6 @@ class RatingCache: class ExifCache: pass - def _parse_exif_date(*args): - return None - - def _parse_date_from_filename(*args): - return None - - def _parse_rating(*args): - return 0 - - DATE_TAGS_PREFERENCE = [] - COMPREHENSIVE_METADATA_TAGS = [] - class TestMetadataProcessor: """Comprehensive tests for MetadataProcessor using sample images.""" @@ -189,7 +170,8 @@ def test_detailed_metadata_extraction(self): found_fields = [field for field in common_fields if field in metadata] logging.info(f" Common EXIF fields found: {found_fields}") - def test_set_and_get_rating(self): + @pytest.mark.parametrize("rating_value", [0, 3, 5]) + def test_set_and_get_rating(self, rating_value): """Test setting and getting ratings on sample images.""" if not self.sample_images: pytest.skip("No sample images available") @@ -205,25 +187,21 @@ def test_set_and_get_rating(self): temp_image_path = tmp.name try: - # Test setting different ratings - for rating in [0, 3, 5]: - success = MetadataProcessor.set_rating( - temp_image_path, - rating, - rating_disk_cache=self.rating_cache, - exif_disk_cache=self.exif_cache, - ) - - assert success, f"Failed to set rating {rating}" - - # Verify rating was set by reading it back - results = MetadataProcessor.get_batch_display_metadata( - [temp_image_path] - ) - norm_path = os.path.normpath(temp_image_path) - assert results[norm_path]["rating"] == rating - - logging.info(f"Successfully set and verified rating {rating}") + success = MetadataProcessor.set_rating( + temp_image_path, + rating_value, + rating_disk_cache=self.rating_cache, + exif_disk_cache=self.exif_cache, + ) + + assert success, f"Failed to set rating {rating_value}" + + # Verify rating was set by reading it back + results = MetadataProcessor.get_batch_display_metadata([temp_image_path]) + norm_path = os.path.normpath(temp_image_path) + assert results[norm_path]["rating"] == rating_value + + logging.info(f"Successfully set and verified rating {rating_value}") finally: # Clean up temporary file @@ -297,97 +275,16 @@ def test_error_handling_nonexistent_file(self): # Should still return valid structure with defaults assert results[norm_path]["rating"] == 0 - def test_invalid_rating_values(self): + @pytest.mark.parametrize("invalid_rating", [-1, 6, 10, "invalid", None]) + def test_invalid_rating_values(self, invalid_rating): """Test error handling for invalid rating values.""" if not self.sample_images: pytest.skip("No sample images available") test_image = self.sample_images[0] - # Test invalid ratings - invalid_ratings = [-1, 6, 10, "invalid", None] - - for invalid_rating in invalid_ratings: - success = MetadataProcessor.set_rating(test_image, invalid_rating) - assert success is False, f"Should reject invalid rating {invalid_rating}" - - -class TestHelperFunctions: - """Test helper functions used by MetadataProcessor.""" - - def test_parse_exif_date(self): - """Test EXIF date parsing function.""" - test_cases = [ - ("2023:12:25 14:30:45", date(2023, 12, 25)), - ("2023-12-25 14:30:45", date(2023, 12, 25)), - ("2023-12-25T14:30:45", date(2023, 12, 25)), - ("2023:12:25", date(2023, 12, 25)), - ("2023-12-25", date(2023, 12, 25)), - ("invalid", None), - ("", None), - (None, None), - ] - - for date_string, expected in test_cases: - result = _parse_exif_date(date_string) - assert result == expected, f"Failed for '{date_string}'" - - def test_parse_date_from_filename(self): - """Test filename date parsing function.""" - test_cases = [ - ("IMG_20231225_143045.jpg", date(2023, 12, 25)), - ("2023-12-25_photo.jpg", date(2023, 12, 25)), - ("20231225_143045.jpg", date(2023, 12, 25)), - ("photo_2023.12.25.jpg", date(2023, 12, 25)), - ("random_filename.jpg", None), - ("IMG_20991301_invalid.jpg", None), # Invalid date - ] - - for filename, expected in test_cases: - result = _parse_date_from_filename(filename) - assert result == expected, f"Failed for '{filename}'" - - def test_parse_rating(self): - """Test rating parsing function.""" - test_cases = [ - (3, 3), - ("4", 4), - ("5.0", 5), - (0, 0), - (6, 5), # Should clamp to 5 - (-1, 0), # Should clamp to 0 - ("invalid", 0), - (None, 0), - ] - - for value, expected in test_cases: - result = _parse_rating(value) - assert result == expected, f"Failed for {value}" - - -class TestConstants: - """Test that constants are properly defined.""" - - def test_date_tags_preference(self): - """Test DATE_TAGS_PREFERENCE is properly defined.""" - assert isinstance(DATE_TAGS_PREFERENCE, list) - assert len(DATE_TAGS_PREFERENCE) > 0 - assert all(isinstance(tag, str) for tag in DATE_TAGS_PREFERENCE) - - # Should include common date tags - expected_tags = ["Exif.Photo.DateTimeOriginal", "Xmp.xmp.CreateDate"] - for tag in expected_tags: - assert tag in DATE_TAGS_PREFERENCE - - def test_comprehensive_metadata_tags(self): - """Test COMPREHENSIVE_METADATA_TAGS is properly defined.""" - assert isinstance(COMPREHENSIVE_METADATA_TAGS, list) - assert len(COMPREHENSIVE_METADATA_TAGS) > 0 - assert all(isinstance(tag, str) for tag in COMPREHENSIVE_METADATA_TAGS) - - # Should include rating and label tags - assert "Xmp.xmp.Rating" in COMPREHENSIVE_METADATA_TAGS - assert "Xmp.xmp.Label" in COMPREHENSIVE_METADATA_TAGS + success = MetadataProcessor.set_rating(test_image, invalid_rating) + assert success is False, f"Should reject invalid rating {invalid_rating}" if __name__ == "__main__": diff --git a/tests/test_metadata_processor_helpers.py b/tests/test_metadata_processor_helpers.py new file mode 100644 index 0000000..bfeadf2 --- /dev/null +++ b/tests/test_metadata_processor_helpers.py @@ -0,0 +1,95 @@ +import pyexiv2 # noqa: F401 # This must be the first import or else it will cause a silent crash on windows +import pytest +import os +import sys +from datetime import date + +# Add the project root to Python path so we can import src modules +project_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +if project_root not in sys.path: + sys.path.insert(0, project_root) + +# Try to import the modules we need +try: + from src.core.metadata_processor import ( + _parse_exif_date, + _parse_date_from_filename, + _parse_rating, + ) + + IMPORTS_AVAILABLE = True + IMPORT_ERROR = None +except ImportError as e: + IMPORTS_AVAILABLE = False + IMPORT_ERROR = str(e) + + # Create dummy functions to avoid NameError + def _parse_exif_date(*args): + return None + + def _parse_date_from_filename(*args): + return None + + def _parse_rating(*args): + return 0 + + +class TestHelperFunctions: + """Test helper functions used by MetadataProcessor.""" + + @pytest.mark.parametrize( + "date_string,expected", + [ + ("2023:12:25 14:30:45", date(2023, 12, 25)), + ("2023-12-25 14:30:45", date(2023, 12, 25)), + ("2023-12-25T14:30:45", date(2023, 12, 25)), + ("2023:12:25", date(2023, 12, 25)), + ("2023-12-25", date(2023, 12, 25)), + ("invalid", None), + ("", None), + (None, None), + ], + ) + def test_parse_exif_date(self, date_string, expected): + """Test EXIF date parsing function.""" + result = _parse_exif_date(date_string) + assert result == expected, f"Failed for '{date_string}'" + + @pytest.mark.parametrize( + "filename,expected", + [ + ("IMG_20231225_143045.jpg", date(2023, 12, 25)), + ("2023-12-25_photo.jpg", date(2023, 12, 25)), + ("20231225_143045.jpg", date(2023, 12, 25)), + ("photo_2023.12.25.jpg", date(2023, 12, 25)), + ("random_filename.jpg", None), + ("IMG_20991301_invalid.jpg", None), # Invalid date + ], + ) + def test_parse_date_from_filename(self, filename, expected): + """Test filename date parsing function.""" + result = _parse_date_from_filename(filename) + assert result == expected, f"Failed for '{filename}'" + + @pytest.mark.parametrize( + "value,expected", + [ + (3, 3), + ("4", 4), + ("5.0", 5), + (0, 0), + (6, 5), # Should clamp to 5 + (-1, 0), # Should clamp to 0 + ("invalid", 0), + (None, 0), + ], + ) + def test_parse_rating(self, value, expected): + """Test rating parsing function.""" + result = _parse_rating(value) + assert result == expected, f"Failed for {value}" + + +if __name__ == "__main__": + # Run tests with verbose output + pytest.main([__file__, "-v", "-s"]) diff --git a/tests/test_metadata_processor_rotation.py b/tests/test_metadata_processor_rotation.py new file mode 100644 index 0000000..4334730 --- /dev/null +++ b/tests/test_metadata_processor_rotation.py @@ -0,0 +1,197 @@ +import pyexiv2 # noqa: F401 # Must be first to avoid Windows crash with pyexiv2 +import pytest +import os +import sys +import tempfile +import shutil +from unittest.mock import Mock +import logging + +# Add the project root to Python path so we can import src modules +project_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +if project_root not in sys.path: + sys.path.insert(0, project_root) + +# Try to import the modules we need +try: + from src.core.metadata_processor import MetadataProcessor + from src.core.caching.exif_cache import ExifCache + + IMPORTS_AVAILABLE = True + IMPORT_ERROR = None +except ImportError as e: + IMPORTS_AVAILABLE = False + IMPORT_ERROR = str(e) + + # Create dummy classes to avoid NameError + class MetadataProcessor: + pass + + class ExifCache: + pass + + +class TestMetadataProcessorRotation: + """Tests for MetadataProcessor rotation methods.""" + + @classmethod + def setup_class(cls): + """Setup test environment.""" + cls.test_folder = "tests/samples" + cls.sample_images = [] + + if os.path.exists(cls.test_folder): + for filename in os.listdir(cls.test_folder): + if filename.lower().endswith( + (".png", ".jpg", ".jpeg", ".arw", ".cr2", ".nef") + ): + cls.sample_images.append(os.path.join(cls.test_folder, filename)) + + if len(cls.sample_images) == 0: + pytest.skip( + f"No test images found in {cls.test_folder}", allow_module_level=True + ) + + def setup_method(self): + """Setup for each test method.""" + self.exif_cache = Mock(spec=ExifCache) + self.temp_files = [] + + def teardown_method(self): + """Clean up temporary files.""" + for temp_file in self.temp_files: + if os.path.exists(temp_file): + try: + os.unlink(temp_file) + except Exception: + pass + + def _create_temp_copy(self, source_path: str) -> str: + """Create a temporary copy of an image for testing.""" + _, ext = os.path.splitext(source_path) + with tempfile.NamedTemporaryFile(suffix=ext, delete=False) as temp_file: + temp_path = temp_file.name + + shutil.copy2(source_path, temp_path) + self.temp_files.append(temp_path) + return temp_path + + def test_is_rotation_supported_for_sample_images(self): + """Test MetadataProcessor.is_rotation_supported method for sample images.""" + if not self.sample_images: + pytest.skip("No sample images available") + + for image_path in self.sample_images: + supported = MetadataProcessor.is_rotation_supported(image_path) + assert isinstance(supported, bool) + + # Check based on file extension + ext = os.path.splitext(image_path)[1].lower() + if ext in [".jpg", ".jpeg", ".png", ".tiff", ".tif"]: + assert supported + logging.info( + f"Rotation support for {os.path.basename(image_path)} ({ext}): {supported}" + ) + + def test_rotate_clockwise_with_cache_invalidation(self): + """Test MetadataProcessor.rotate_clockwise method with cache invalidation.""" + if not self.sample_images: + pytest.skip("No sample images available") + + source_image = self.sample_images[0] + temp_image = self._create_temp_copy(source_image) + + success = MetadataProcessor.rotate_clockwise( + temp_image, exif_disk_cache=self.exif_cache + ) + + # Check that cache was invalidated if rotation was successful + if success: + assert os.path.exists(temp_image) + self.exif_cache.delete.assert_called_once() + + print(f"Clockwise rotation of {os.path.basename(source_image)}: {success}") + + def test_rotate_counterclockwise_with_cache_invalidation(self): + """Test MetadataProcessor.rotate_counterclockwise method with cache invalidation.""" + if not self.sample_images: + pytest.skip("No sample images available") + + source_image = self.sample_images[0] + temp_image = self._create_temp_copy(source_image) + + success = MetadataProcessor.rotate_counterclockwise( + temp_image, exif_disk_cache=self.exif_cache + ) + + if success: + assert os.path.exists(temp_image) + self.exif_cache.delete.assert_called_once() + + logging.info( + f"Counterclockwise rotation of {os.path.basename(source_image)}: {success}" + ) + + def test_rotate_180_with_cache_invalidation(self): + """Test MetadataProcessor.rotate_180 method with cache invalidation.""" + if not self.sample_images: + pytest.skip("No sample images available") + + source_image = self.sample_images[0] + temp_image = self._create_temp_copy(source_image) + + success = MetadataProcessor.rotate_180( + temp_image, exif_disk_cache=self.exif_cache + ) + + if success: + assert os.path.exists(temp_image) + self.exif_cache.delete.assert_called_once() + + logging.info(f"180° rotation of {os.path.basename(source_image)}: {success}") + + def test_rotation_with_metadata_only_flag(self): + """Test rotation with metadata_only flag and cache invalidation.""" + if not self.sample_images: + pytest.skip("No sample images available") + + source_image = self.sample_images[0] + temp_image = self._create_temp_copy(source_image) + + # Get original file info + original_size = os.path.getsize(temp_image) + + success = MetadataProcessor.rotate_image( + temp_image, + "clockwise", + update_metadata_only=True, + exif_disk_cache=self.exif_cache, + ) + + if success: + # File should exist and cache should be invalidated + assert os.path.exists(temp_image) + self.exif_cache.delete.assert_called_once() + + # File size should be similar (metadata changes only) + new_size = os.path.getsize(temp_image) + assert abs(new_size - original_size) < 1024 # Less than 1KB difference + + logging.info( + f"Metadata-only rotation of {os.path.basename(source_image)}: {success}" + ) + + def test_error_handling_nonexistent_file(self): + """Test error handling for non-existent file.""" + success = MetadataProcessor.rotate_clockwise("/nonexistent/file.jpg") + assert not success + + def test_error_handling_invalid_path(self): + """Test error handling for invalid path.""" + success = MetadataProcessor.rotate_counterclockwise("") + assert not success + + +if __name__ == "__main__": + # Run tests with verbose output + pytest.main([__file__, "-v", "-s"]) diff --git a/tests/test_navigation_controller.py b/tests/test_navigation_controller.py new file mode 100644 index 0000000..20f846c --- /dev/null +++ b/tests/test_navigation_controller.py @@ -0,0 +1,200 @@ +from src.ui.controllers.navigation_controller import NavigationController +from PyQt6.QtCore import Qt, QModelIndex + + +class DummyIndex(QModelIndex): + def __init__(self, row: int): + super().__init__() + self._row = row + + def isValid(self): + return self._row >= 0 + + def row(self): + return self._row + + +class DummyItem: + def __init__(self, path: str): + self._path = path + + def data(self, role): + if role == Qt.ItemDataRole.UserRole: + return {"path": self._path} + return None + + +class DummyView: + def __init__(self, ctx): + self.ctx = ctx + self._current = DummyIndex(-1) + + def currentIndex(self): + return self._current + + def selectionModel(self): + return None + + # Simulate setCurrentIndex used in controller code path (indirectly) + def setCurrentIndex(self, idx): + self._current = idx + + def setFocus(self, *_args, **_kwargs): + pass + + def viewport(self): + class V: + def update(self_inner): + pass + + return V() + + +class Ctx: + def __init__(self, paths, deleted=None): + self.paths = list(paths) + self.deleted = set(deleted or []) + self.last_selected = None + self.view = DummyView(self) + + # Protocol methods + def get_active_view(self): + return self.view + + def is_valid_image_index(self, proxy_index): + return proxy_index.isValid() + + def map_to_source(self, proxy_index): + return proxy_index + + def item_from_source(self, source_index): + row = source_index.row() + if 0 <= row < len(self.paths): + return DummyItem(self.paths[row]) + # fallback to last selected + if self.last_selected and self.last_selected in self.paths: + return DummyItem(self.last_selected) + if self.paths: + return DummyItem(self.paths[0]) + return None + + def get_group_sibling_images(self, current_proxy_index): + indices = [DummyIndex(i) for i in range(len(self.paths))] + return None, indices, None + + def find_first_visible_item(self): + return DummyIndex(0) if self.paths else DummyIndex(-1) + + def find_proxy_index_for_path(self, path): + if path in self.paths: + idx = DummyIndex(self.paths.index(path)) + self.last_selected = path + self.view.setCurrentIndex(idx) + return idx + return DummyIndex(-1) + + def get_all_visible_image_paths(self): + return list(self.paths) + + def get_marked_deleted(self): + return list(self.deleted) + + def validate_and_select_image_candidate(self, proxy_index, direction, log_skip): + # Called after controller finds a candidate; ensure selection updates + row = proxy_index.row() + if 0 <= row < len(self.paths): + self.last_selected = self.paths[row] + + +def test_navigation_group_cyclic_with_deleted_items(): + """Test that group navigation cycles correctly and skips deleted items when skip_deleted=True.""" + ctx = Ctx(["a", "b", "c"], deleted=["b"]) + nav = NavigationController(ctx) + # simulate current path 'a' + ctx.find_proxy_index_for_path("a") + + # Navigate right: should skip deleted 'b' and go to 'c' + nav.navigate_group("right") + assert ctx.last_selected == "c" + + # Navigate right again: should wrap around to 'a' + nav.navigate_group("right") + assert ctx.last_selected == "a" + + # Navigate left: should go to 'c' + nav.navigate_group("left") + assert ctx.last_selected == "c" + + +def test_navigation_linear_sequential_with_boundaries(): + """Test linear navigation handles start/end boundaries and deleted items correctly.""" + ctx = Ctx(["a", "b", "c", "d"], deleted=["c"]) + nav = NavigationController(ctx) + + # Start with no selection + ctx.last_selected = None + + # Navigate down: should select first non-deleted item 'a' + nav.navigate_linear("down") + assert ctx.last_selected == "a" + + # Navigate down: should go to 'b' + nav.navigate_linear("down") + assert ctx.last_selected == "b" + + # Navigate down: should skip deleted 'c' and go to 'd' + nav.navigate_linear("down") + assert ctx.last_selected == "d" + + # Navigate down at end: should stay at 'd' + nav.navigate_linear("down") + assert ctx.last_selected == "d" + + # Navigate up: should skip 'c' and go to 'b' + nav.navigate_linear("up") + assert ctx.last_selected == "b" + + # Navigate up: should go to 'a' + nav.navigate_linear("up") + assert ctx.last_selected == "a" + + +def test_navigation_linear_includes_deleted_when_skip_false(): + """Test that linear navigation includes deleted items when skip_deleted=False.""" + ctx = Ctx(["a", "b", "c"], deleted=["b"]) + nav = NavigationController(ctx) + ctx.find_proxy_index_for_path("a") + nav.navigate_linear("down", skip_deleted=False) # should land on deleted 'b' + assert ctx.last_selected == "b" + + +def test_navigation_group_includes_deleted_when_skip_false(): + """Test that group navigation includes deleted items when skip_deleted=False.""" + ctx = Ctx(["a", "b", "c"], deleted=["b"]) + nav = NavigationController(ctx) + ctx.find_proxy_index_for_path("a") + nav.navigate_group("right", skip_deleted=False) # should land on deleted 'b' + assert ctx.last_selected == "b" + + +def test_navigation_skip_deleted_with_multiple_deleted_items(): + """Test navigation correctly skips multiple deleted items in both directions. + Scenario: 5 images [a, b, c, d, e] where b and d are deleted, user selects c. + - Down navigation should skip d and go to e + - Up navigation should skip b and go to a + """ + ctx = Ctx(["a", "b", "c", "d", "e"], deleted=["b", "d"]) + nav = NavigationController(ctx) + + # Select image c (middle item, surrounded by deleted items) + ctx.find_proxy_index_for_path("c") + assert ctx.last_selected == "c" + + # Navigate right - should skip d (deleted) and go to e + nav.navigate_linear("down", skip_deleted=True) + assert ctx.last_selected == "e", "Should go to e when skipping deleted d" + + # Reset to c and navigate left - should skip b (deleted) and go to a + ctx.find_proxy_index_for_path("c") + nav.navigate_linear("up", skip_deleted=True) + assert ctx.last_selected == "a", "Should go to a when skipping deleted b" diff --git a/tests/test_navigation_direct.py b/tests/test_navigation_direct.py new file mode 100644 index 0000000..6bafb7d --- /dev/null +++ b/tests/test_navigation_direct.py @@ -0,0 +1,164 @@ +import os +from PyQt6.QtCore import Qt, QModelIndex +from PyQt6.QtGui import QStandardItemModel, QStandardItem +from PyQt6.QtWidgets import QApplication, QTreeView +from src.ui.controllers.navigation_controller import NavigationController + +app = QApplication.instance() or QApplication([]) + + +class MockNavigationContext: + def __init__(self): + self.file_system_model = QStandardItemModel() + self._active_file_view = QTreeView() + self._active_file_view.setModel(self.file_system_model) + self.current_selection_path = None + + def get_active_view(self): + return self._active_file_view + + def is_valid_image_index(self, index): + item = self.file_system_model.itemFromIndex(index) + if not item: + return False + d = item.data(Qt.ItemDataRole.UserRole) + return isinstance(d, dict) and "path" in d + + def map_to_source(self, index): + return index + + def item_from_source(self, source_index): + return self.file_system_model.itemFromIndex(source_index) + + def find_first_visible_item(self): + return QModelIndex() # Not used in linear navigation + + def get_marked_deleted(self): + return [] + + def get_all_visible_image_paths(self): + paths = [] + root = self.file_system_model.invisibleRootItem() + for i in range(root.rowCount()): + group = root.child(i) + for j in range(group.rowCount()): + item = group.child(j) + d = item.data(Qt.ItemDataRole.UserRole) + if isinstance(d, dict) and "path" in d: + paths.append(d["path"]) + return paths + + def find_proxy_index_for_path(self, path): + root = self.file_system_model.invisibleRootItem() + for i in range(root.rowCount()): + group = root.child(i) + for j in range(group.rowCount()): + item = group.child(j) + d = item.data(Qt.ItemDataRole.UserRole) + if isinstance(d, dict) and d.get("path") == path: + return item.index() + return QModelIndex() + + def validate_and_select_image_candidate(self, index, direction, log_skip): + # Mock the selection by storing the path + if index.isValid(): + item = self.file_system_model.itemFromIndex(index) + if item: + d = item.data(Qt.ItemDataRole.UserRole) + if isinstance(d, dict): + self.current_selection_path = d.get("path") + + def get_group_sibling_images(self, current_index): + parent = current_index.parent() + count = self.file_system_model.rowCount(parent) + group_indices = [ + self.file_system_model.index(r, 0, parent) for r in range(count) + ] + return parent, group_indices, None + + def set_current_selection(self, path): + """Helper method to simulate current selection""" + self.current_selection_path = path + + # Mock the active view's currentIndex + class MockView: + def __init__(self, ctx, path): + self.ctx = ctx + self.path = path + + def currentIndex(self): + return self.ctx.find_proxy_index_for_path(self.path) + + self._active_file_view = MockView(self, path) + + +def make_image_item(path: str): + it = QStandardItem(os.path.basename(path)) + it.setData({"path": path}, Qt.ItemDataRole.UserRole) + return it + + +def test_navigate_down_is_sequential(tmp_path): + ctx = MockNavigationContext() + nav = NavigationController(ctx) + + # Create a group with two image items + group_header = QStandardItem("Group 1") + group_header.setData("cluster_header_1", Qt.ItemDataRole.UserRole) + + paths = [] + for name in ["a.jpg", "b.jpg"]: + p = tmp_path / name + p.write_text("x") + paths.append(str(p)) + group_header.appendRow(make_image_item(str(p))) + ctx.file_system_model.appendRow(group_header) + + # Set initial selection to first item + ctx.set_current_selection(str(paths[0])) # a.jpg + + # Verify setup + all_paths = ctx.get_all_visible_image_paths() + assert all_paths == [str(paths[0]), str(paths[1])], ( + f"Expected paths, got {all_paths}" + ) + + # Navigate down - should move from a.jpg to b.jpg + nav.navigate_linear("down", skip_deleted=True) + + # Check that selection moved to b.jpg + assert ctx.current_selection_path == str(paths[1]), ( + f"Expected {paths[1]}, got {ctx.current_selection_path}" + ) + + +def test_navigate_up_is_sequential(tmp_path): + ctx = MockNavigationContext() + nav = NavigationController(ctx) + + # Create a group with three image items + group_header = QStandardItem("Group 1") + group_header.setData("cluster_header_1", Qt.ItemDataRole.UserRole) + + paths = [] + for name in ["a.jpg", "b.jpg", "c.jpg"]: + p = tmp_path / name + p.write_text("x") + paths.append(str(p)) + group_header.appendRow(make_image_item(str(p))) + ctx.file_system_model.appendRow(group_header) + + # Set initial selection to middle item (b.jpg) + ctx.set_current_selection(str(paths[1])) # b.jpg + + # Verify setup + all_paths = ctx.get_all_visible_image_paths() + assert all_paths == [str(p) for p in paths], f"Expected paths, got {all_paths}" + + # Navigate up - should move from b.jpg to a.jpg + nav.navigate_linear("up", skip_deleted=True) + + # Check that selection moved to a.jpg + assert ctx.current_selection_path == str(paths[0]), ( + f"Expected {paths[0]}, got {ctx.current_selection_path}" + ) diff --git a/tests/test_navigation_utils.py b/tests/test_navigation_utils.py new file mode 100644 index 0000000..0eab46f --- /dev/null +++ b/tests/test_navigation_utils.py @@ -0,0 +1,36 @@ +from src.ui.helpers.navigation_utils import navigate_group_cyclic, navigate_linear + + +def test_group_cyclic_basic(): + sibs = ["a", "b", "c"] + assert navigate_group_cyclic(sibs, "b", "right", True, set()) == "c" + assert navigate_group_cyclic(sibs, "c", "right", True, set()) == "a" + assert navigate_group_cyclic(sibs, "a", "left", True, set()) == "c" + + +def test_group_cyclic_skip_deleted(): + sibs = ["a", "b", "c", "d"] + deleted = {"b", "c"} + # starting from d left should go to a (skip b,c) + assert navigate_group_cyclic(sibs, "d", "left", True, deleted) == "a" + + +def test_linear_down(): + ordered = ["a", "b", "c"] + assert navigate_linear(ordered, None, "down", True, set()) == "a" + assert navigate_linear(ordered, "a", "down", True, set()) == "b" + assert navigate_linear(ordered, "c", "down", True, set()) is None + + +def test_linear_up(): + ordered = ["a", "b", "c"] + assert navigate_linear(ordered, None, "up", True, set()) == "c" + assert navigate_linear(ordered, "c", "up", True, set()) == "b" + assert navigate_linear(ordered, "a", "up", True, set()) is None + + +def test_linear_skip_deleted(): + ordered = ["a", "b", "c", "d"] + deleted = {"b", "c"} + assert navigate_linear(ordered, "a", "down", True, deleted) == "d" + assert navigate_linear(ordered, "d", "up", True, deleted) == "a" diff --git a/tests/test_preview_controller.py b/tests/test_preview_controller.py new file mode 100644 index 0000000..1a63384 --- /dev/null +++ b/tests/test_preview_controller.py @@ -0,0 +1,37 @@ +from src.ui.controllers.preview_controller import PreviewController + + +class DummyCtx: + def __init__(self): + self.worker_manager = type( + "WM", (), {"calls": [], "start_preview_preload": self._start} + )() + self.loading = [] + self.statuses = [] + + def _start(self, paths): + self.worker_manager.calls.append(paths) + + def show_loading_overlay(self, text): + self.loading.append(("show", text)) + + def hide_loading_overlay(self): + self.loading.append(("hide",)) + + def status_message(self, msg, timeout=3000): + self.statuses.append(msg) + + +def test_preview_preload_filters_and_starts(): + ctx = DummyCtx() + pc = PreviewController(ctx) + pc.start_preload([{"path": "a.jpg"}, {"path": None}, {"no": "key"}]) + assert ctx.worker_manager.calls == [["a.jpg"]] + + +def test_preview_preload_no_items(): + ctx = DummyCtx() + pc = PreviewController(ctx) + pc.start_preload([]) + assert ctx.worker_manager.calls == [] + assert any("No previews" in s for s in ctx.statuses) diff --git a/tests/test_rotation_controller.py b/tests/test_rotation_controller.py new file mode 100644 index 0000000..d7c774c --- /dev/null +++ b/tests/test_rotation_controller.py @@ -0,0 +1,67 @@ +from src.ui.controllers.rotation_controller import RotationController + + +class DummyApplier: + def __init__(self): + self.applied = [] + + def __call__(self, mapping): + self.applied.append(dict(mapping)) + + +def make_controller(suggestions): + applier = DummyApplier() + ctrl = RotationController(suggestions, applier) + return ctrl, applier + + +def test_accept_paths_and_next_basic(): + suggestions = {"a.jpg": 90, "b.jpg": 180, "c.jpg": 270} + ctrl, applier = make_controller(suggestions) + before = ctrl.get_visible_order() + accepted = ctrl.accept_paths(["b.jpg"]) + assert accepted == ["b.jpg"] + assert "b.jpg" not in ctrl.rotation_suggestions + next_path = ctrl.compute_next_after_accept(before, accepted, "b.jpg") + # After removing b, remaining order preserves sequence of a, c; anchor b picks next c + assert next_path == "c.jpg" + assert applier.applied[0] == {"b.jpg": 180} + + +def test_accept_all_clears(): + suggestions = {"a.jpg": 90, "b.jpg": 180} + ctrl, applier = make_controller(suggestions) + accepted = ctrl.accept_all() + assert accepted == ["a.jpg", "b.jpg"] + assert not ctrl.has_suggestions() + assert applier.applied[0] == {"a.jpg": 90, "b.jpg": 180} + + +def test_refuse_paths(): + suggestions = {"a.jpg": 90, "b.jpg": 180, "c.jpg": 270} + ctrl, applier = make_controller(suggestions) + refused = ctrl.refuse_paths(["a.jpg", "x.jpg"]) # x ignored + assert refused == ["a.jpg"] + assert "a.jpg" not in ctrl.rotation_suggestions + assert ctrl.has_suggestions() + # No application when refusing + assert applier.applied == [] + + +def test_compute_next_after_accept_end_boundary(): + suggestions = {"a.jpg": 90, "b.jpg": 180, "c.jpg": 270} + ctrl, applier = make_controller(suggestions) + before = ctrl.get_visible_order() + accepted = ctrl.accept_paths(["c.jpg"]) # last element + next_path = ctrl.compute_next_after_accept(before, accepted, "c.jpg") + # Wraps to previous remaining (b.jpg) given anchor at end + assert next_path == "b.jpg" + + +def test_accept_paths_empty_input(): + suggestions = {"a.jpg": 90} + ctrl, applier = make_controller(suggestions) + accepted = ctrl.accept_paths([]) + assert accepted == [] + # Still present + assert ctrl.has_suggestions() diff --git a/tests/test_rotation_integration.py b/tests/test_rotation_integration.py new file mode 100644 index 0000000..8e5f3db --- /dev/null +++ b/tests/test_rotation_integration.py @@ -0,0 +1,142 @@ +import pyexiv2 # noqa: F401 # Must be first to avoid Windows crash with pyexiv2 +import pytest +import os +import sys +import tempfile +import shutil +import logging + +# Add the project root to Python path so we can import src modules +project_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +if project_root not in sys.path: + sys.path.insert(0, project_root) + +# Try to import the modules we need +try: + from src.core.metadata_processor import MetadataProcessor + + IMPORTS_AVAILABLE = True + IMPORT_ERROR = None +except ImportError as e: + IMPORTS_AVAILABLE = False + IMPORT_ERROR = str(e) + + # Create dummy classes to avoid NameError + class MetadataProcessor: + pass + + +class TestRotationIntegration: + """Integration tests for the complete rotation system.""" + + @classmethod + def setup_class(cls): + """Setup test environment.""" + cls.test_folder = "tests/samples" + cls.sample_images = [] + + if os.path.exists(cls.test_folder): + for filename in os.listdir(cls.test_folder): + if filename.lower().endswith((".png", ".jpg", ".jpeg")): + cls.sample_images.append(os.path.join(cls.test_folder, filename)) + + if len(cls.sample_images) == 0: + pytest.skip( + f"No test images found in {cls.test_folder}", allow_module_level=True + ) + + def setup_method(self): + """Setup for each test method.""" + self.temp_files = [] + + def teardown_method(self): + """Clean up temporary files.""" + for temp_file in self.temp_files: + if os.path.exists(temp_file): + try: + os.unlink(temp_file) + except Exception: + pass + + def _create_temp_copy(self, source_path: str) -> str: + """Create a temporary copy of an image for testing.""" + _, ext = os.path.splitext(source_path) + with tempfile.NamedTemporaryFile(suffix=ext, delete=False) as temp_file: + temp_path = temp_file.name + + shutil.copy2(source_path, temp_path) + self.temp_files.append(temp_path) + return temp_path + + def test_full_rotation_cycle_returns_to_original(self): + """Test that 4 clockwise rotations return to original orientation.""" + if not self.sample_images: + pytest.skip("No sample images available") + + source_image = self.sample_images[0] + temp_image = self._create_temp_copy(source_image) + + # Perform 4 clockwise rotations + for i in range(4): + success = MetadataProcessor.rotate_clockwise(temp_image) + assert success, f"Rotation {i + 1} failed" + assert os.path.exists(temp_image), f"File missing after rotation {i + 1}" + + # Check that we're back to original orientation + final_metadata = MetadataProcessor.get_batch_display_metadata([temp_image]) + final_detailed = MetadataProcessor.get_detailed_metadata(temp_image) + + # The image should still be valid + assert final_metadata is not None + assert final_detailed is not None + + logging.info( + f"Full rotation cycle completed for {os.path.basename(source_image)}" + ) + + @pytest.mark.parametrize( + "rotation_sequence", + [ + ["clockwise", "180", "counterclockwise", "180"], + ["clockwise", "clockwise", "180"], + ["counterclockwise", "clockwise", "180"], + ], + ) + def test_mixed_rotations_sequence(self, rotation_sequence): + """Test different rotation combinations using parametrized sequences.""" + if not self.sample_images: + pytest.skip("No sample images available") + + source_image = self.sample_images[0] + temp_image = self._create_temp_copy(source_image) + + # Map rotation names to functions + rotation_functions = { + "clockwise": MetadataProcessor.rotate_clockwise, + "counterclockwise": MetadataProcessor.rotate_counterclockwise, + "180": MetadataProcessor.rotate_180, + } + + # Execute rotation sequence + for rotation_name in rotation_sequence: + rotate_func = rotation_functions[rotation_name] + success = rotate_func(temp_image) + assert success, f"{rotation_name} rotation failed" + assert os.path.exists(temp_image), ( + f"File missing after {rotation_name} rotation" + ) + + # Verify image is still readable + metadata = MetadataProcessor.get_detailed_metadata(temp_image) + assert metadata is not None, ( + f"Cannot read metadata after {rotation_name} rotation" + ) + + logging.info( + f"Mixed rotation sequence {rotation_sequence} completed for {os.path.basename(source_image)}" + ) + + +if __name__ == "__main__": + # Run tests with verbose output + pytest.main([__file__, "-v", "-s"]) diff --git a/tests/test_rotation_utils.py b/tests/test_rotation_utils.py new file mode 100644 index 0000000..10db16a --- /dev/null +++ b/tests/test_rotation_utils.py @@ -0,0 +1,30 @@ +from src.ui.helpers.rotation_utils import compute_next_after_rotation + + +def test_compute_next_after_rotation_basic_forward(): + before = ["a.jpg", "b.jpg", "c.jpg", "d.jpg"] + accepted = ["b.jpg"] + after = ["a.jpg", "c.jpg", "d.jpg"] + assert compute_next_after_rotation(before, accepted, after) == "c.jpg" + + +def test_compute_next_after_rotation_multi_delete(): + before = ["a.jpg", "b.jpg", "c.jpg", "d.jpg", "e.jpg"] + accepted = ["b.jpg", "c.jpg"] + after = ["a.jpg", "d.jpg", "e.jpg"] + # Expect to advance to first surviving after earliest accepted (c's successor -> d) + assert compute_next_after_rotation(before, accepted, after) == "d.jpg" + + +def test_compute_next_after_rotation_end_boundary(): + before = ["a.jpg", "b.jpg"] + accepted = ["b.jpg"] + after = ["a.jpg"] + assert compute_next_after_rotation(before, accepted, after) == "a.jpg" + + +def test_compute_next_after_rotation_all_removed(): + before = ["a.jpg"] + accepted = ["a.jpg"] + after = [] + assert compute_next_after_rotation(before, accepted, after) is None diff --git a/tests/test_selection_controller.py b/tests/test_selection_controller.py new file mode 100644 index 0000000..d93962b --- /dev/null +++ b/tests/test_selection_controller.py @@ -0,0 +1,86 @@ +import os +from typing import List +from PyQt6.QtCore import QModelIndex, Qt +from PyQt6.QtGui import QStandardItemModel, QStandardItem +from PyQt6.QtWidgets import QTreeView, QApplication +from src.ui.controllers.selection_controller import ( + SelectionController, + SelectionContext, +) + + +class DummyView(QTreeView): + def __init__(self, model): + super().__init__() + self.setModel(model) + + +class DummyCtx(SelectionContext): # type: ignore[misc] + def __init__(self, model): + self._model = model + self._view = DummyView(model) + + def get_active_view(self): + return self._view + + @property + def proxy_model(self): # minimal proxy interface usage in controller + return self._model # not actually a proxy; map_to_source is identity + + def is_valid_image_item(self, proxy_index: QModelIndex) -> bool: + return True + + def file_system_model_item_from_index(self, source_index: QModelIndex): + return self._model.itemFromIndex(source_index) + + def map_to_source(self, proxy_index: QModelIndex) -> QModelIndex: + return proxy_index + + +def build_model_with_files(file_paths: List[str]): + model = QStandardItemModel() + for p in file_paths: + item = QStandardItem(os.path.basename(p)) + item.setData({"path": p}, Qt.ItemDataRole.UserRole) + model.appendRow(item) + return model + + +def test_selection_controller_returns_selected_paths(tmp_path): + # Ensure QApplication exists + QApplication.instance() or QApplication([]) + # Arrange + files = [tmp_path / f"img_{i}.jpg" for i in range(3)] + for fp in files: + fp.write_text("x") + paths = [str(f) for f in files] + model = build_model_with_files(paths) + ctx = DummyCtx(model) + controller = SelectionController(ctx) + + # Simulate selecting first and third + sel_model = ctx.get_active_view().selectionModel() + first_idx = model.index(0, 0) + third_idx = model.index(2, 0) + sel_model.select( + first_idx, sel_model.SelectionFlag.Select | sel_model.SelectionFlag.Rows + ) + sel_model.select( + third_idx, sel_model.SelectionFlag.Select | sel_model.SelectionFlag.Rows + ) + + # Act + result = controller.get_selected_file_paths() + + # Assert + assert set(result) == {paths[0], paths[2]} + + +def test_selection_controller_empty_when_no_view(): + QApplication.instance() or QApplication([]) + model = build_model_with_files([]) + ctx = DummyCtx(model) + controller = SelectionController(ctx) + # Replace active view with None to simulate missing view + ctx._view = None # type: ignore + assert controller.get_selected_file_paths() == [] diff --git a/tests/test_selection_visibility.py b/tests/test_selection_visibility.py new file mode 100644 index 0000000..340300d --- /dev/null +++ b/tests/test_selection_visibility.py @@ -0,0 +1,170 @@ +from PyQt6.QtCore import Qt, QModelIndex +from PyQt6.QtGui import QStandardItemModel, QStandardItem +from src.ui.controllers.selection_controller import ( + SelectionController, + SelectionContext, +) + + +class StubListView: + """Lightweight list view stub exposing only model() as used by controller.""" + + def __init__(self, model): + self._model = model + + def model(self): + return self._model + + +class StubTreeView: + """Lightweight tree view stub supporting expansion tracking (duck-typed).""" + + def __init__(self, model): + self._model = model + self._expanded = set() + + def model(self): + return self._model + + # Methods emulating QTreeView API subset + def expand(self, index: QModelIndex): + if index.isValid(): + self._expanded.add( + (index.row(), index.parent().row() if index.parent().isValid() else -1) + ) + + def isExpanded(self, index: QModelIndex) -> bool: # noqa: N802 (Qt style) + if not index.isValid(): + return False + key = (index.row(), index.parent().row() if index.parent().isValid() else -1) + return key in self._expanded + + +class ListCtx(SelectionContext): # type: ignore[misc] + def __init__(self, model): + self._model = model + self._view = StubListView(model) + + def get_active_view(self): + return self._view + + @property + def proxy_model(self): # type: ignore[override] + return self._model + + def is_valid_image_item(self, proxy_index: QModelIndex) -> bool: + return proxy_index.isValid() + + def file_system_model_item_from_index(self, source_index: QModelIndex): + return self._model.itemFromIndex(source_index) + + def map_to_source(self, proxy_index: QModelIndex) -> QModelIndex: + return proxy_index + + +class TreeCtx(SelectionContext): # type: ignore[misc] + def __init__(self, model, view): + self._model = model + self._view = view + + def get_active_view(self): + return self._view + + @property + def proxy_model(self): # type: ignore[override] + return self._model + + def is_valid_image_item(self, proxy_index: QModelIndex) -> bool: + return proxy_index.isValid() + + def file_system_model_item_from_index(self, source_index: QModelIndex): + return self._model.itemFromIndex(source_index) + + def map_to_source(self, proxy_index: QModelIndex) -> QModelIndex: + return proxy_index + + +def build_model_with_items(names): + model = QStandardItemModel() + for n in names: + it = QStandardItem(n) + it.setData({"path": n}, Qt.ItemDataRole.UserRole) + model.appendRow(it) + return model + + +def test_find_first_visible_item_list(): + model = build_model_with_items(["a.jpg", "b.jpg", "c.jpg"]) + ctx = ListCtx(model) + sc = SelectionController(ctx) + first = sc.find_first_visible_item() + assert first.isValid() + assert model.itemFromIndex(first).text() == "a.jpg" + + +def test_find_last_visible_item_list(): + model = build_model_with_items(["a.jpg", "b.jpg", "c.jpg"]) + ctx = ListCtx(model) + sc = SelectionController(ctx) + last = sc.find_last_visible_item() + assert last.isValid() + assert model.itemFromIndex(last).text() == "c.jpg" + + +def test_find_first_visible_item_tree_expanded(): + # Build simple tree with a top-level that isn't an image (simulate non-image header) and children images + model = QStandardItemModel() + header = QStandardItem("Group 1") + header.setData("cluster_header_1", Qt.ItemDataRole.UserRole) + child1 = QStandardItem("x.jpg") + child1.setData({"path": "x.jpg"}, Qt.ItemDataRole.UserRole) + child2 = QStandardItem("y.jpg") + child2.setData({"path": "y.jpg"}, Qt.ItemDataRole.UserRole) + header.appendRow(child1) + header.appendRow(child2) + model.appendRow(header) + view = StubTreeView(model) + view.expand(model.indexFromItem(header)) + + class TreeCtxImages(TreeCtx): # override validity: only dict with path is image + def is_valid_image_item(self, proxy_index: QModelIndex) -> bool: + item = self._model.itemFromIndex(proxy_index) + if not item: + return False + data = item.data(Qt.ItemDataRole.UserRole) + return isinstance(data, dict) and "path" in data + + ctx = TreeCtxImages(model, view) + sc = SelectionController(ctx) + first = sc.find_first_visible_item() + assert first.isValid() + assert model.itemFromIndex(first).text() == "x.jpg" + + +def test_find_last_visible_item_tree_expanded(): + model = QStandardItemModel() + header = QStandardItem("Group 1") + header.setData("cluster_header_1", Qt.ItemDataRole.UserRole) + child1 = QStandardItem("x.jpg") + child1.setData({"path": "x.jpg"}, Qt.ItemDataRole.UserRole) + child2 = QStandardItem("y.jpg") + child2.setData({"path": "y.jpg"}, Qt.ItemDataRole.UserRole) + header.appendRow(child1) + header.appendRow(child2) + model.appendRow(header) + view = StubTreeView(model) + view.expand(model.indexFromItem(header)) + + class TreeCtxImages(TreeCtx): + def is_valid_image_item(self, proxy_index: QModelIndex) -> bool: + item = self._model.itemFromIndex(proxy_index) + if not item: + return False + data = item.data(Qt.ItemDataRole.UserRole) + return isinstance(data, dict) and "path" in data + + ctx = TreeCtxImages(model, view) + sc = SelectionController(ctx) + last = sc.find_last_visible_item() + assert last.isValid() + assert model.itemFromIndex(last).text() == "y.jpg" diff --git a/tests/test_similarity_controller.py b/tests/test_similarity_controller.py new file mode 100644 index 0000000..d4f41cc --- /dev/null +++ b/tests/test_similarity_controller.py @@ -0,0 +1,93 @@ +from src.ui.controllers.similarity_controller import SimilarityController + + +class DummyCtx: + def __init__(self): + class AppState: + pass + + self.app_state = AppState() + self.app_state.embeddings_cache = None + self.app_state.cluster_results = {} + self.worker_manager = type( + "WM", (), {"started": False, "start_similarity_analysis": self._start} + )() + self.menu_manager = type( + "MM", + (), + { + "group_by_similarity_action": type( + "A", + (), + { + "setEnabled": lambda *a, **k: None, + "setChecked": lambda *a, **k: None, + }, + )(), + "cluster_sort_action": type( + "B", (), {"setVisible": lambda *a, **k: None} + )(), + }, + )() + self.loading = [] + self.statuses = [] + self.rebuilt = 0 + self.cluster_combo_enabled = False + self.cluster_ids = [] + + def _start(self, paths, auto_edits): + self.worker_manager.started = True + self.worker_manager.paths = paths + self.worker_manager.auto_edits = auto_edits + + def show_loading_overlay(self, text): + self.loading.append(("show", text)) + + def hide_loading_overlay(self): + self.loading.append(("hide",)) + + def update_loading_text(self, text): + self.loading.append(("update", text)) + + def status_message(self, msg, timeout=3000): + self.statuses.append(msg) + + def rebuild_model_view(self): + self.rebuilt += 1 + + def enable_group_by_similarity(self, enabled): + pass + + def set_group_by_similarity_checked(self, checked): + pass + + def set_cluster_sort_visible(self, visible): + pass + + def enable_cluster_sort_combo(self, enabled): + self.cluster_combo_enabled = enabled + + def populate_cluster_filter(self, cluster_ids): + self.cluster_ids = cluster_ids + + +def test_similarity_start_and_clustering_flow(): + ctx = DummyCtx() + sc = SimilarityController(ctx) + sc.start(["a.jpg", "b.jpg"], auto_edits=True) + assert ctx.worker_manager.started is True + assert ctx.worker_manager.paths == ["a.jpg", "b.jpg"] + sc.embeddings_generated({"a.jpg": [0, 1], "b.jpg": [1, 0]}) + assert ctx.app_state.embeddings_cache is not None + sc.clustering_complete({"a.jpg": 1, "b.jpg": 2}, group_mode=True) + assert ctx.app_state.cluster_results == {"a.jpg": 1, "b.jpg": 2} + assert ctx.cluster_ids == [1, 2] + assert ctx.rebuilt == 1 + + +def test_similarity_no_paths(): + ctx = DummyCtx() + sc = SimilarityController(ctx) + sc.start([], auto_edits=False) + assert ctx.worker_manager.started is False + assert any("No valid image paths" in s for s in ctx.statuses) diff --git a/tests/test_similarity_prepare_clusters.py b/tests/test_similarity_prepare_clusters.py new file mode 100644 index 0000000..6856472 --- /dev/null +++ b/tests/test_similarity_prepare_clusters.py @@ -0,0 +1,129 @@ +from datetime import date as date_obj + +from src.ui.controllers.similarity_controller import SimilarityController + + +class DummyAppState: + def __init__(self): + self.image_files_data = [] + self.cluster_results = {} + self.date_cache = {} + self.embeddings_cache = {} + + +class DummyCtx: + def __init__(self, app_state): + self.app_state = app_state + # Unused protocol methods for these tests + self.worker_manager = None + self.menu_manager = None + + def show_loading_overlay(self, text): + pass + + def hide_loading_overlay(self): + pass + + def update_loading_text(self, text): + pass + + def status_message(self, msg, timeout=3000): + pass + + def rebuild_model_view(self): + pass + + def enable_group_by_similarity(self, enabled): + pass + + def set_group_by_similarity_checked(self, checked): + pass + + def set_cluster_sort_visible(self, visible): + pass + + def enable_cluster_sort_combo(self, enabled): + pass + + def populate_cluster_filter(self, cluster_ids): + pass + + +def _build_images(paths): + # minimal image file data dicts + return [{"path": p} for p in paths] + + +def test_prepare_clusters_empty(): + app_state = DummyAppState() + ctx = DummyCtx(app_state) + sc = SimilarityController(ctx) + info = sc.prepare_clusters("Default") + assert info["images_by_cluster"] == {} + assert info["sorted_cluster_ids"] == [] + assert info["total_images"] == 0 + + +def test_prepare_clusters_default_numeric(): + app_state = DummyAppState() + app_state.image_files_data = _build_images(["a.jpg", "b.jpg", "c.jpg"]) + app_state.cluster_results = {"a.jpg": 2, "b.jpg": 1, "c.jpg": 3} + ctx = DummyCtx(app_state) + sc = SimilarityController(ctx) + info = sc.prepare_clusters("Default") + assert info["sorted_cluster_ids"] == [1, 2, 3] + assert info["total_images"] == 3 + + +def test_prepare_clusters_time_sort(): + app_state = DummyAppState() + app_state.image_files_data = _build_images(["x.jpg", "y.jpg", "z.jpg"]) + app_state.cluster_results = {"x.jpg": 10, "y.jpg": 5, "z.jpg": 10} + # Provide dates: cluster 10 earliest is 2024-01-01, cluster 5 earliest 2024-06-01 -> 10 should come first + app_state.date_cache = { + "x.jpg": date_obj(2024, 1, 1), + "y.jpg": date_obj(2024, 6, 1), + "z.jpg": date_obj(2024, 2, 1), + } + ctx = DummyCtx(app_state) + sc = SimilarityController(ctx) + info = sc.prepare_clusters("Time") + assert info["sorted_cluster_ids"] == [10, 5] + + +def test_prepare_clusters_similarity_then_time_with_embeddings(): + app_state = DummyAppState() + app_state.image_files_data = _build_images(["p.jpg", "q.jpg", "r.jpg", "s.jpg"]) + app_state.cluster_results = {"p.jpg": 1, "q.jpg": 2, "r.jpg": 1, "s.jpg": 3} + # identical timestamps so PCA/similarity ordering decides before time fallback + app_state.date_cache = { + f: date_obj(2024, 1, 1) for f in ["p.jpg", "q.jpg", "r.jpg", "s.jpg"] + } + # embeddings: construct 2D vectors so PCA produces deterministic ordering by first component + app_state.embeddings_cache = { + "p.jpg": [0.1, 0.0], + "r.jpg": [0.2, 0.0], # cluster 1 centroid ~0.15 + "q.jpg": [0.5, 0.0], # cluster 2 centroid 0.5 + "s.jpg": [0.3, 0.0], # cluster 3 centroid 0.3 + } + ctx = DummyCtx(app_state) + sc = SimilarityController(ctx) + info = sc.prepare_clusters("Similarity then Time") + # Expected centroid ordering by PCA (monotonic with x values here): cluster1(~0.15), cluster3(0.3), cluster2(0.5) + assert info["sorted_cluster_ids"] == [1, 3, 2] + + +def test_prepare_clusters_similarity_then_time_without_embeddings_fallback_to_time(): + app_state = DummyAppState() + app_state.image_files_data = _build_images(["m.jpg", "n.jpg", "o.jpg"]) + app_state.cluster_results = {"m.jpg": 7, "n.jpg": 8, "o.jpg": 7} + app_state.date_cache = { + "m.jpg": date_obj(2024, 5, 1), + "n.jpg": date_obj(2024, 4, 1), + "o.jpg": date_obj(2024, 5, 2), + } + # No embeddings -> should fallback to time ordering: cluster 7 earliest 2024-05-01; cluster 8 earliest 2024-04-01 -> 8 first + ctx = DummyCtx(app_state) + sc = SimilarityController(ctx) + info = sc.prepare_clusters("Similarity then Time") + assert info["sorted_cluster_ids"] == [8, 7] diff --git a/tests/test_statusbar_utils.py b/tests/test_statusbar_utils.py new file mode 100644 index 0000000..14af540 --- /dev/null +++ b/tests/test_statusbar_utils.py @@ -0,0 +1,58 @@ +from src.ui.helpers.statusbar_utils import build_status_bar_info +from datetime import datetime + + +def test_statusbar_basic_no_cluster(): + md = {"rating": 3, "date": datetime(2025, 8, 10)} + info = build_status_bar_info( + file_path=__file__, + metadata=md, + width=1920, + height=1080, + cluster_lookup={}, + file_data_from_model={"is_blurred": False}, + ) + msg = info.to_message() + assert "R: 3" in msg + assert "D: 2025-08-10" in msg + assert "Blurred: No" in msg + + +def test_statusbar_with_cluster_and_blur_yes(tmp_path): + f = tmp_path / "a.jpg" + f.write_text("x") + md = {"rating": 5, "date": None} + info = build_status_bar_info( + file_path=str(f), + metadata=md, + width=100, + height=200, + cluster_lookup={str(f): 7}, + file_data_from_model={"is_blurred": True}, + ) + msg = info.to_message() + assert "C: 7" in msg + assert "R: 5" in msg + assert "D: Unknown" in msg + assert "100x200" in msg + assert "Blurred: Yes" in msg + + +def test_statusbar_size_unavailable(monkeypatch, tmp_path): + f = tmp_path / "b.jpg" + f.write_text("x") + + def broken_getsize(path): + raise OSError + + monkeypatch.setattr("os.path.getsize", broken_getsize) + md = {"rating": 0, "date": None} + info = build_status_bar_info( + file_path=str(f), + metadata=md, + width=10, + height=10, + cluster_lookup=None, + file_data_from_model=None, + ) + assert "Size: N/A" in info.to_message()