Pr/optimize sidecar

faststack/app.py

@@ -2608,27 +2608,37 @@ def _get_metadata_dict(self, image_path: Path | str) -> dict:
             }
 
     def _get_bulk_metadata_map(self, images=None) -> Dict[str, dict]:
-        """Get flattened metadata map for the given images (defaults to self.image_files).
+        """Get flattened metadata map for the given images, keyed by str(img.path).
 
-        Used to avoid per-image sidecar lookups on the UI thread during grid refresh.
+        Used to avoid per-image sidecar lookups on the UI thread during grid
+        refresh. Keying by the raw path string lets downstream consumers
+        look up entries directly without re-resolving the stable sidecar key.
+
+        The map is treated as authoritative by the grid (see
+        ``ThumbnailModel.refresh_from_controller``), so per-image errors must
+        be isolated — a single bad path must not collapse flags for the
+        whole folder.
         """
         bulk_map = {}
-        try:
-            for img in (images if images is not None else self.image_files):
-                key = self.sidecar.metadata_key_for_path(img.path)
+        for img in (images if images is not None else self.image_files):
+            try:
                 meta = self.sidecar.get_metadata(img.path, create=False)
                 if meta is None:
                     continue
-                bulk_map[key] = {
+                bulk_map[str(img.path)] = {
                     "stacked": getattr(meta, "stacked", False),
                     "uploaded": getattr(meta, "uploaded", False),
                     "edited": getattr(meta, "edited", False),
                     "restacked": getattr(meta, "restacked", False),
                     "favorite": getattr(meta, "favorite", False),
                     "todo": getattr(meta, "todo", False),
                 }
-        except Exception as e:
-            log.warning("Failed to build bulk metadata map: %s", e)
+            except Exception as e:
+                log.warning(
+                    "Failed to read metadata for %s during bulk build: %s",
+                    img.path,
+                    e,
+                )
         return bulk_map
 
     def _invalidate_batch_cache(self):

faststack/io/sidecar.py

@@ -46,6 +46,17 @@ def __init__(self, directory: Path, watcher, debug: bool = False):
         self.path = directory / "faststack.json"
         self.watcher = watcher
         self.debug = debug
+        # Precomputed once: the case-normalized absolute base dir used by
+        # metadata_key_for_path / _metadata_filename_key on every call.
+        self._base_dir_normcased = Path(
+            os.path.normcase(os.path.abspath(str(directory)))
+        )
+        # Bounded per-instance caches: input str → resolved key. Folder
+        # refresh resolves the same paths repeatedly across bulk-map build,
+        # flag filter, and grid entry construction.
+        self._stable_key_cache: dict[str, str] = {}
+        self._filename_key_cache: dict[str, str] = {}
+        self._key_cache_max = 8192
         self.data = self.load()
 
     def stop_watcher(self):
@@ -187,21 +198,30 @@ def get_metadata(
 
     def metadata_key_for_path(self, image_path: Union[str, Path]) -> str:
         """Return the stable sidecar key for a concrete image path."""
+        cache_key = str(image_path)
+        cached = self._stable_key_cache.get(cache_key)
+        if cached is not None:
+            return cached
+
         path = Path(image_path)
         if not path.name:
             return ""
         if not path.is_absolute():
             path = self.directory / path
-        base_dir = Path(os.path.normcase(os.path.abspath(str(self.directory))))
         abs_path = Path(os.path.normcase(os.path.abspath(str(path))))
 
         try:
-            relative = abs_path.relative_to(base_dir)
+            relative = abs_path.relative_to(self._base_dir_normcased)
             stable_path = relative.parent / relative.stem
-            return str(stable_path).replace("\\", "/")
+            result = str(stable_path).replace("\\", "/")
         except ValueError:
             stable_path = abs_path.parent / abs_path.stem
-            return str(stable_path).replace("\\", "/")
+            result = str(stable_path).replace("\\", "/")
+
+        if len(self._stable_key_cache) >= self._key_cache_max:
+            del self._stable_key_cache[next(iter(self._stable_key_cache))]
+        self._stable_key_cache[cache_key] = result
+        return result
 
     def _lookup_keys(self, image_ref: Union[str, Path]) -> tuple[str, list[str]]:
         """Return (stable_key, migration_candidate_keys) for a metadata lookup."""
@@ -230,19 +250,28 @@ def _lookup_keys(self, image_ref: Union[str, Path]) -> tuple[str, list[str]]:
 
     def _metadata_filename_key(self, image_path: Union[str, Path]) -> str:
         """Return the extension-preserving key used by the regressed patch."""
+        cache_key = str(image_path)
+        cached = self._filename_key_cache.get(cache_key)
+        if cached is not None:
+            return cached
+
         path = Path(image_path)
         if not path.name:
             return ""
         if not path.is_absolute():
             path = self.directory / path
-        base_dir = Path(os.path.normcase(os.path.abspath(str(self.directory))))
         abs_path = Path(os.path.normcase(os.path.abspath(str(path))))
 
         try:
-            relative = abs_path.relative_to(base_dir)
-            return str(relative).replace("\\", "/")
+            relative = abs_path.relative_to(self._base_dir_normcased)
+            result = str(relative).replace("\\", "/")
         except ValueError:
-            return str(abs_path).replace("\\", "/")
+            result = str(abs_path).replace("\\", "/")
+
+        if len(self._filename_key_cache) >= self._key_cache_max:
+            del self._filename_key_cache[next(iter(self._filename_key_cache))]
+        self._filename_key_cache[cache_key] = result
+        return result
 
     def _stable_key_from_key(self, key: str, check_fs: bool = False) -> str:
         """Convert any historical sidecar key form into today's stable key.

faststack/tests/test_bulk_metadata_map.py

@@ -0,0 +1,77 @@
+"""Tests for AppController._get_bulk_metadata_map error isolation.
+
+The grid (ThumbnailModel.refresh_from_controller) treats a non-None metadata
+map as authoritative — it does not fall back to per-image sidecar lookups on
+miss. So per-image errors during bulk build must be isolated; one bad path
+must not silently force every grid flag to false for the whole folder.
+"""
+
+from faststack.models import EntryMetadata, ImageFile
+
+
+def _make_image(tmp_path, name):
+    p = tmp_path / name
+    p.write_bytes(b"\xff\xd8")
+    return ImageFile(path=p, timestamp=1000.0)
+
+
+def test_per_image_error_does_not_invalidate_other_entries(app_controller, tmp_path):
+    """One sidecar.get_metadata raise must not drop other images from the map."""
+    a = _make_image(tmp_path, "a.jpg")
+    bad = _make_image(tmp_path, "b.jpg")
+    c = _make_image(tmp_path, "c.jpg")
+    app_controller.image_files = [a, bad, c]
+
+    def fake_get_metadata(path, *, create):
+        if path == bad.path:
+            raise OSError("simulated sidecar failure for one image")
+        return EntryMetadata(favorite=True, uploaded=True)
+
+    app_controller.sidecar.get_metadata.side_effect = fake_get_metadata
+
+    bulk_map = app_controller._get_bulk_metadata_map()
+
+    assert str(a.path) in bulk_map
+    assert str(c.path) in bulk_map
+    assert str(bad.path) not in bulk_map
+    assert bulk_map[str(a.path)]["favorite"] is True
+    assert bulk_map[str(a.path)]["uploaded"] is True
+    assert bulk_map[str(c.path)]["favorite"] is True
+
+
+def test_all_images_raise_yields_empty_map(app_controller, tmp_path):
+    """Documents the contract: when every lookup fails, the map is empty.
+
+    The grid treats this empty map as authoritative — the resulting all-false
+    flags are intentional, not a silent regression. Any future change to the
+    error-path contract should update this test.
+    """
+    a = _make_image(tmp_path, "a.jpg")
+    b = _make_image(tmp_path, "b.jpg")
+    app_controller.image_files = [a, b]
+
+    app_controller.sidecar.get_metadata.side_effect = OSError("everything is broken")
+
+    bulk_map = app_controller._get_bulk_metadata_map()
+
+    assert bulk_map == {}
+
+
+def test_first_image_raises_subsequent_succeed(app_controller, tmp_path):
+    """Earlier exceptions must not skip later images (regression for outer try)."""
+    bad = _make_image(tmp_path, "a.jpg")
+    good = _make_image(tmp_path, "b.jpg")
+    app_controller.image_files = [bad, good]
+
+    def fake_get_metadata(path, *, create):
+        if path == bad.path:
+            raise RuntimeError("first one fails")
+        return EntryMetadata(stacked=True)
+
+    app_controller.sidecar.get_metadata.side_effect = fake_get_metadata
+
+    bulk_map = app_controller._get_bulk_metadata_map()
+
+    assert str(bad.path) not in bulk_map
+    assert str(good.path) in bulk_map
+    assert bulk_map[str(good.path)]["stacked"] is True

faststack/thumbnail_view/model.py

@@ -572,10 +572,15 @@ def refresh_from_controller(
             if self._active_filter_flags:
                 flags = self._active_filter_flags
                 filtered = []
+                # When metadata_map is provided (even empty) treat it as
+                # authoritative — don't fall through to per-image sidecar
+                # lookups, and key lookups by str(img.path) so we don't
+                # re-resolve a stable key the bulk-map builder already paid for.
+                map_authoritative = metadata_map is not None
                 for img in images:
                     try:
-                        if metadata_map and self._metadata_key_fn:
-                            meta = metadata_map.get(self._metadata_key_fn(img.path), {})
+                        if map_authoritative:
+                            meta = metadata_map.get(str(img.path), {})
                         elif self._get_metadata:
                             meta = self._get_metadata(img.path)
                         else:
@@ -618,6 +623,9 @@ def _add_images_to_entries(
         self, images: List, metadata_map: Optional[Dict[str, dict]] = None
     ):
         """Convert list of objects (ImageFile or similar) to ThumbnailEntry."""
+        # See refresh_from_controller: a non-None metadata_map is authoritative
+        # and keyed by str(img.path).
+        map_authoritative = metadata_map is not None
         for img in images:
             try:
                 # Use mtime from object if available to avoid stat()
@@ -636,8 +644,8 @@ def _add_images_to_entries(
             is_favorite = False
             is_todo = False
 
-            if metadata_map and self._metadata_key_fn:
-                meta = metadata_map.get(self._metadata_key_fn(img.path), {})
+            if map_authoritative:
+                meta = metadata_map.get(str(img.path), {})
                 is_stacked = meta.get("stacked", False)
                 is_uploaded = meta.get("uploaded", False)
                 is_edited = meta.get("edited", False)