refactor: Refactor main window

.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
 
 

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 |

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

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
 

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
 

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 (

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

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,

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 (

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]

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()