From c34d0bdd6b0a9ff36675715898d237d4bd19470e Mon Sep 17 00:00:00 2001 From: Andy Arijs Date: Tue, 17 Mar 2026 19:27:08 +0100 Subject: [PATCH 1/4] Fix sidecar metadata lookup to use stable image path keys --- faststack/app.py | 85 +++++++------- faststack/io/sidecar.py | 105 ++++++++++++++++-- faststack/logging_setup.py | 51 ++++++++- faststack/tests/test_jump_to_last_uploaded.py | 16 ++- faststack/tests/test_sidecar.py | 93 ++++++++++++++-- .../tests/thumbnail_view/test_folder_stats.py | 12 ++ faststack/thumbnail_view/folder_stats.py | 4 +- faststack/thumbnail_view/model.py | 20 ++-- 8 files changed, 304 insertions(+), 82 deletions(-) diff --git a/faststack/app.py b/faststack/app.py index 37efddc..304bd33 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -1040,13 +1040,9 @@ def _apply_filter_to_cached_list(self): # Apply flag-based filtering (AND logic: image must have ALL checked flags) if self._filter_enabled and self._filter_flags: flags = self._filter_flags - # Optimize: access sidecar entries directly to avoid get_metadata overhead - entries = self.sidecar.data.entries result = [] for img in filtered: - # Direct dict lookup is faster than get_metadata() which might create objects - stem = img.path.stem - meta = entries.get(stem) + meta = self.sidecar.get_metadata(img.path, create=False) if not meta: continue @@ -1810,7 +1806,7 @@ def jump_to_last_uploaded(self): for idx in range(len(self.image_files) - 1, -1, -1): img = self.image_files[idx] # Dynamic look-up of self.sidecar as requested (important for mocks in tests) - meta = self.sidecar.get_metadata(img.path.stem, create=False) + meta = self.sidecar.get_metadata(img.path, create=False) uploaded = meta.uploaded if meta else False @@ -2126,10 +2122,10 @@ def _on_thumbnail_ready_gui(self, thumbnail_id: str): if self._thumbnail_model: self._thumbnail_model.thumbnailReady.emit(thumbnail_id) - def _get_metadata_dict(self, stem: str) -> dict: - """Get metadata for a file stem as a dict for thumbnail model.""" + def _get_metadata_dict(self, image_path: Path | str) -> dict: + """Get metadata for an image path as a dict for thumbnail model.""" try: - meta = self.sidecar.get_metadata(stem, create=False) + meta = self.sidecar.get_metadata(image_path, create=False) if meta is None: return { "stacked": False, @@ -2148,7 +2144,7 @@ def _get_metadata_dict(self, stem: str) -> dict: "todo": meta.todo, } except Exception as e: # Broad catch for UI plumbing - don't crash grid view - log.debug("Failed to get metadata for %s: %s", stem, e) + log.debug("Failed to get metadata for %s: %s", image_path, e) return { "stacked": False, "uploaded": False, @@ -2162,9 +2158,11 @@ def _get_bulk_metadata_map(self) -> Dict[str, dict]: """Get flattened metadata map for all images (for efficient grid refresh).""" bulk_map = {} try: - # sidecar.data.entries is a dict of stem -> EntryMetadata - for stem, meta in self.sidecar.data.entries.items(): - bulk_map[stem] = { + for img in self.image_files: + meta = self.sidecar.get_metadata(img.path, create=False) + if meta is None: + continue + bulk_map[normalize_path_key(img.path)] = { "stacked": getattr(meta, "stacked", False), "uploaded": getattr(meta, "uploaded", False), "edited": getattr(meta, "edited", False), @@ -2214,8 +2212,8 @@ def toggle_uploaded(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.uploaded = not meta.uploaded if meta.uploaded: @@ -2229,7 +2227,7 @@ def toggle_uploaded(self): self.sync_ui_state() status = "uploaded" if meta.uploaded else "not uploaded" self.update_status_message(f"Marked as {status}") - log.info("Toggled uploaded flag to %s for %s", meta.uploaded, stem) + log.info("Toggled uploaded flag to %s for %s", meta.uploaded, image_path) def toggle_todo(self): """Toggle todo flag for current image.""" @@ -2237,8 +2235,8 @@ def toggle_todo(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.todo = not getattr(meta, "todo", False) if meta.todo: @@ -2252,7 +2250,7 @@ def toggle_todo(self): self.sync_ui_state() status = "todo" if meta.todo else "not todo" self.update_status_message(f"Marked as {status}") - log.info("Toggled todo flag to %s for %s", meta.todo, stem) + log.info("Toggled todo flag to %s for %s", meta.todo, image_path) def toggle_edited(self): """Toggle edited flag for current image.""" @@ -2260,8 +2258,8 @@ def toggle_edited(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.edited = not meta.edited if meta.edited: @@ -2275,7 +2273,7 @@ def toggle_edited(self): self.sync_ui_state() status = "edited" if meta.edited else "not edited" self.update_status_message(f"Marked as {status}") - log.info("Toggled edited flag to %s for %s", meta.edited, stem) + log.info("Toggled edited flag to %s for %s", meta.edited, image_path) def toggle_restacked(self): """Toggle restacked flag for current image.""" @@ -2283,8 +2281,8 @@ def toggle_restacked(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.restacked = not meta.restacked if meta.restacked: @@ -2298,15 +2296,15 @@ def toggle_restacked(self): self.sync_ui_state() status = "restacked" if meta.restacked else "not restacked" self.update_status_message(f"Marked as {status}") - log.info("Toggled restacked flag to %s for %s", meta.restacked, stem) + log.info("Toggled restacked flag to %s for %s", meta.restacked, image_path) def toggle_favorite(self): """Toggle favorite flag for current image.""" if not self.image_files or self.current_index >= len(self.image_files): return - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.favorite = not meta.favorite @@ -2316,7 +2314,7 @@ def toggle_favorite(self): self.sync_ui_state() status = "Favorited" if meta.favorite else "Unfavorited" self.update_status_message(status) - log.info("Toggled favorite flag to %s for %s", meta.favorite, stem) + log.info("Toggled favorite flag to %s for %s", meta.favorite, image_path) def toggle_stacked(self): """Toggle stacked flag for current image.""" @@ -2324,8 +2322,8 @@ def toggle_stacked(self): return today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path) meta.stacked = not meta.stacked if meta.stacked: @@ -2339,7 +2337,7 @@ def toggle_stacked(self): self.sync_ui_state() status = "stacked" if meta.stacked else "not stacked" self.update_status_message(f"Marked as {status}") - log.info("Toggled stacked flag to %s for %s", meta.stacked, stem) + log.info("Toggled stacked flag to %s for %s", meta.stacked, image_path) def get_current_metadata(self) -> Dict: if not self.image_files or self.current_index >= len(self.image_files): @@ -2357,8 +2355,8 @@ def get_current_metadata(self) -> Dict: return self._metadata_cache # Compute and cache - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem, create=False) + image_path = self.image_files[self.current_index].path + meta = self.sidecar.get_metadata(image_path, create=False) stack_info = self._get_stack_info(self.current_index) batch_info = self._get_batch_info(self.current_index) @@ -2619,7 +2617,7 @@ def add_favorites_to_batch(self): # Find indices of all favorited images indices_to_add = [] for i, img in enumerate(self.image_files): - meta = self.sidecar.get_metadata(img.path.stem, create=False) + meta = self.sidecar.get_metadata(img.path, create=False) if meta and meta.favorite: indices_to_add.append(i) @@ -2674,7 +2672,7 @@ def add_uploaded_to_batch(self): # Find indices of all uploaded images indices_to_add = [] for i, img in enumerate(self.image_files): - meta = self.sidecar.get_metadata(img.path.stem, create=False) + meta = self.sidecar.get_metadata(img.path, create=False) if meta and meta.uploaded: indices_to_add.append(i) @@ -3099,8 +3097,7 @@ def _launch_helicon_with_files(self, files: List[Path]) -> bool: for img_file in self.image_files: # Match by either RAW pair or JPG path if img_file.raw_pair == file_path or img_file.path == file_path: - stem = img_file.path.stem - meta = self.sidecar.get_metadata(stem) + meta = self.sidecar.get_metadata(img_file.path) meta.stacked = True meta.stacked_date = today break @@ -5448,8 +5445,7 @@ def edit_in_photoshop(self): # Mark as edited on successful launch today = datetime.now().strftime("%Y-%m-%d") - stem = image_file.path.stem - meta = self.sidecar.get_metadata(stem) + meta = self.sidecar.get_metadata(image_file.path) meta.edited = True meta.edited_date = today self.sidecar.save() @@ -5583,8 +5579,7 @@ def start_drag_current_image(self): today = datetime.now().strftime("%Y-%m-%d") for idx in existing_indices: - stem = self.image_files[idx].path.stem - meta = self.sidecar.get_metadata(stem) + meta = self.sidecar.get_metadata(self.image_files[idx].path) meta.uploaded = True meta.uploaded_date = today @@ -6456,8 +6451,7 @@ def stack_source_raws(self): if success: # Mark as restacked on success today = datetime.now().strftime("%Y-%m-%d") - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem) + meta = self.sidecar.get_metadata(self.image_files[self.current_index].path) meta.restacked = True meta.restacked_date = today self.sidecar.save() @@ -7363,8 +7357,9 @@ def get_stack_summary(self) -> str: def is_stacked(self) -> bool: if not self.image_files or self.current_index >= len(self.image_files): return False - stem = self.image_files[self.current_index].path.stem - meta = self.sidecar.get_metadata(stem, create=False) + meta = self.sidecar.get_metadata( + self.image_files[self.current_index].path, create=False + ) return meta.stacked if meta else False def _update_cache_stats(self): diff --git a/faststack/io/sidecar.py b/faststack/io/sidecar.py index c1adcf0..7371d5f 100644 --- a/faststack/io/sidecar.py +++ b/faststack/io/sidecar.py @@ -2,9 +2,10 @@ import json import logging +import os import time from pathlib import Path -from typing import Literal, Optional, overload +from typing import Literal, Optional, Union, overload from faststack.models import Sidecar, EntryMetadata @@ -37,6 +38,7 @@ def _entrymetadata_from_json(meta: dict) -> EntryMetadata: class SidecarManager: def __init__(self, directory: Path, watcher, debug: bool = False): + self.directory = directory self.path = directory / "faststack.json" self.watcher = watcher self.debug = debug @@ -71,8 +73,8 @@ def load(self) -> Sidecar: # Reconstruct nested objects entries = { - stem: _entrymetadata_from_json(meta) - for stem, meta in data.get("entries", {}).items() + key: _entrymetadata_from_json(meta) + for key, meta in data.get("entries", {}).items() } return Sidecar( version=data.get("version", 2), @@ -103,7 +105,7 @@ def save(self): "version": self.data.version, "last_index": self.data.last_index, "entries": { - stem: meta.__dict__ for stem, meta in self.data.entries.items() + key: meta.__dict__ for key, meta in self.data.entries.items() }, "stacks": self.data.stacks, } @@ -121,21 +123,21 @@ def save(self): @overload def get_metadata( - self, image_stem: str, *, create: Literal[True] = True + self, image_ref: Union[str, Path], *, create: Literal[True] = True ) -> EntryMetadata: ... @overload def get_metadata( - self, image_stem: str, *, create: Literal[False] + self, image_ref: Union[str, Path], *, create: Literal[False] ) -> Optional[EntryMetadata]: ... @overload def get_metadata( - self, image_stem: str, *, create: bool + self, image_ref: Union[str, Path], *, create: bool ) -> Optional[EntryMetadata]: ... def get_metadata( - self, image_stem: str, *, create: bool = True + self, image_ref: Union[str, Path], *, create: bool = True ) -> Optional[EntryMetadata]: """Get metadata for an image, optionally creating a persistent entry. @@ -143,11 +145,94 @@ def get_metadata( and storing one if it doesn't exist). When create=False, returns None if no entry exists — callers must handle the None case explicitly. """ - meta = self.data.entries.get(image_stem) + stable_key, candidate_keys = self._lookup_keys(image_ref) + + meta = self.data.entries.get(stable_key) + if meta is None: + for candidate_key in candidate_keys: + if candidate_key == stable_key: + continue + candidate_meta = self.data.entries.get(candidate_key) + if candidate_meta is None: + continue + meta = candidate_meta + if stable_key not in self.data.entries: + self.data.entries[stable_key] = candidate_meta + if candidate_key in self.data.entries and candidate_key != stable_key: + del self.data.entries[candidate_key] + break + if meta is None: + for existing_key, existing_meta in list(self.data.entries.items()): + if existing_key == stable_key: + continue + if self._stable_key_from_key(existing_key) != stable_key: + continue + meta = existing_meta + self.data.entries[stable_key] = existing_meta + del self.data.entries[existing_key] + break + if meta is None and create: meta = EntryMetadata() - self.data.entries[image_stem] = meta + self.data.entries[stable_key] = meta return meta + def metadata_key_for_path(self, image_path: Union[str, Path]) -> str: + """Return the stable sidecar key for a concrete image path.""" + path = Path(image_path) + if not path.is_absolute(): + path = self.directory / path + base_dir = Path(os.path.abspath(str(self.directory))) + abs_path = Path(os.path.abspath(str(path))) + + try: + relative = abs_path.relative_to(base_dir) + stable_path = relative.parent / relative.stem + return str(stable_path).replace("\\", "/") + except ValueError: + stable_path = abs_path.parent / abs_path.stem + return str(stable_path).replace("\\", "/") + + def _lookup_keys(self, image_ref: Union[str, Path]) -> tuple[str, list[str]]: + """Return (stable_key, migration_candidate_keys) for a metadata lookup.""" + if isinstance(image_ref, Path): + stable_key = self.metadata_key_for_path(image_ref) + full_name_key = self._metadata_filename_key(image_ref) + return stable_key, [full_name_key, image_ref.stem] + + value = str(image_ref) + if not value: + return "", [] + + if os.path.sep in value or "/" in value or "\\" in value or "." in Path(value).name: + path = Path(value) + stable_key = self.metadata_key_for_path(path) + full_name_key = self._metadata_filename_key(path) + return stable_key, [full_name_key, path.stem] + + return value, [value] + + def _metadata_filename_key(self, image_path: Union[str, Path]) -> str: + """Return the extension-preserving key used by the regressed patch.""" + path = Path(image_path) + if not path.is_absolute(): + path = self.directory / path + base_dir = Path(os.path.abspath(str(self.directory))) + abs_path = Path(os.path.abspath(str(path))) + + try: + relative = abs_path.relative_to(base_dir) + return str(relative).replace("\\", "/") + except ValueError: + return str(abs_path).replace("\\", "/") + + def _stable_key_from_key(self, key: str) -> str: + """Convert any historical sidecar key form into today's stable key.""" + if not key: + return "" + if os.path.sep in key or "/" in key or "\\" in key or "." in Path(key).name: + return self.metadata_key_for_path(Path(key)) + return key + def set_last_index(self, index: int): self.data.last_index = index diff --git a/faststack/logging_setup.py b/faststack/logging_setup.py index ab684a4..e783d94 100644 --- a/faststack/logging_setup.py +++ b/faststack/logging_setup.py @@ -6,12 +6,57 @@ from pathlib import Path +def _is_writable_dir(path: Path) -> bool: + """Return True when an existing directory accepts file writes.""" + if not path.is_dir(): + return False + + try: + probe = path / ".write_test" + with probe.open("w", encoding="utf-8") as f: + f.write("ok") + probe.unlink(missing_ok=True) + return True + except OSError: + return False + + +def _can_create_dir(path: Path) -> bool: + """Return True when the nearest existing parent is writable.""" + parent = path + while not parent.exists(): + next_parent = parent.parent + if next_parent == parent: + return False + parent = next_parent + + return parent.is_dir() and os.access(parent, os.W_OK) def get_app_data_dir() -> Path: - """Returns the application data directory.""" + """Return a writable application data directory, with fallbacks.""" + candidates = [] + app_data = os.getenv("APPDATA") if app_data: - return Path(app_data) / "faststack" - return Path.home() / ".faststack" + candidates.append(Path(app_data) / "faststack") + + local_app_data = os.getenv("LOCALAPPDATA") + if local_app_data: + candidates.append(Path(local_app_data) / "faststack") + + candidates.append(Path.home() / ".faststack") + candidates.append(Path.cwd() / "var" / "appdata") + + for candidate in candidates: + if _is_writable_dir(candidate): + return candidate + + for candidate in candidates: + if _can_create_dir(candidate): + return candidate + + # Final fallback: return the first candidate even if unwritable so callers + # still get a deterministic location for error reporting. + return candidates[0] if candidates else Path.cwd() / "var" / "appdata" def setup_logging(debug: bool = False): diff --git a/faststack/tests/test_jump_to_last_uploaded.py b/faststack/tests/test_jump_to_last_uploaded.py index 524d8a3..80e21da 100644 --- a/faststack/tests/test_jump_to_last_uploaded.py +++ b/faststack/tests/test_jump_to_last_uploaded.py @@ -66,8 +66,12 @@ def test_jump_to_last_uploaded_success(mock_controller): meta2 = EntryMetadata(uploaded=False) meta3 = EntryMetadata(uploaded=True) - def side_effect(stem, **kwargs): - return {"img1": meta1, "img2": meta2, "img3": meta3}.get(stem, EntryMetadata()) + def side_effect(path, **kwargs): + return { + Path("img1.jpg"): meta1, + Path("img2.jpg"): meta2, + Path("img3.jpg"): meta3, + }.get(Path(path), EntryMetadata()) mock_controller.sidecar.get_metadata.side_effect = side_effect @@ -137,8 +141,12 @@ def test_jump_to_last_uploaded_one(mock_controller): mock_controller.image_files = [img1, img2, img3] mock_controller.current_index = 0 - def side_effect(stem, **kwargs): - return {"img1": meta1, "img2": meta2, "img3": meta3}.get(stem, EntryMetadata()) + def side_effect(path, **kwargs): + return { + Path("img1.jpg"): meta1, + Path("img2.jpg"): meta2, + Path("img3.jpg"): meta3, + }.get(Path(path), EntryMetadata()) mock_controller.sidecar.get_metadata.side_effect = side_effect diff --git a/faststack/tests/test_sidecar.py b/faststack/tests/test_sidecar.py index 2c97eae..1334dac 100644 --- a/faststack/tests/test_sidecar.py +++ b/faststack/tests/test_sidecar.py @@ -61,7 +61,7 @@ def test_sidecar_save(mock_sidecar_dir): # Modify data sm.set_last_index(10) - meta = sm.get_metadata("IMG_TEST") + meta = sm.get_metadata(Path("IMG_TEST.jpg")) # Modify a valid field meta.stack_id = 99 @@ -78,8 +78,8 @@ def test_sidecar_get_metadata_creates_new(mock_sidecar_dir): """Tests that get_metadata creates a new entry if one doesn't exist.""" d = mock_sidecar_dir() sm = SidecarManager(d, None) - assert "NEW_IMG" not in sm.data.entries - meta = sm.get_metadata("NEW_IMG") + assert "NEW_IMG.jpg" not in sm.data.entries + meta = sm.get_metadata(Path("NEW_IMG.jpg")) # EntryMetadata may be a runtime class OR a typing alias, depending on refactors. if isinstance(EntryMetadata, type): @@ -96,7 +96,7 @@ def test_favorite_toggle_sets_json(mock_sidecar_dir): """Tests that toggling favorite writes true/false to JSON.""" d = mock_sidecar_dir() sm = SidecarManager(d, None) - meta = sm.get_metadata("IMG_FAV") + meta = sm.get_metadata(Path("IMG_FAV.jpg")) # Initially false assert meta.favorite is False @@ -120,12 +120,12 @@ def test_favorite_loads_from_sidecar(mock_sidecar_dir): "version": 2, "last_index": 0, "entries": { - "IMG_FAV": {"favorite": True}, + "IMG_FAV.jpg": {"favorite": True}, }, } d = mock_sidecar_dir(content) sm = SidecarManager(d, None) - meta = sm.get_metadata("IMG_FAV") + meta = sm.get_metadata(Path("IMG_FAV.jpg")) assert meta.favorite is True @@ -133,7 +133,7 @@ def test_favorite_toggle_roundtrip(mock_sidecar_dir): """Tests that toggling twice restores original JSON (round-trip).""" d = mock_sidecar_dir() sm = SidecarManager(d, None) - meta = sm.get_metadata("IMG_FAV") + meta = sm.get_metadata(Path("IMG_FAV.jpg")) # Capture original state assert meta.favorite is False @@ -145,5 +145,82 @@ def test_favorite_toggle_roundtrip(mock_sidecar_dir): # Reload and verify sm2 = SidecarManager(d, None) - meta2 = sm2.get_metadata("IMG_FAV") + meta2 = sm2.get_metadata(Path("IMG_FAV.jpg")) assert meta2.favorite is False + + +def test_legacy_stem_entry_migrates_to_path_key(mock_sidecar_dir): + """Legacy stem-keyed entries should migrate on first concrete path lookup.""" + content = { + "version": 2, + "entries": { + "IMG_0001": {"uploaded": True}, + }, + } + d = mock_sidecar_dir(content) + sm = SidecarManager(d, None) + + meta = sm.get_metadata(Path("IMG_0001.jpg"), create=False) + + assert meta is not None + assert meta.uploaded is True + assert "IMG_0001" in sm.data.entries + assert "IMG_0001.jpg" not in sm.data.entries + assert sm.metadata_key_for_path(Path("IMG_0001.jpg")) == "IMG_0001" + + +def test_raw_only_entry_survives_transition_to_visible_jpg(mock_sidecar_dir): + """RAW-only metadata must remain visible after a matching JPG appears.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + raw_meta = sm.get_metadata(Path("photo.CR2")) + raw_meta.favorite = True + raw_meta.uploaded = True + + jpg_meta = sm.get_metadata(Path("photo.jpg"), create=False) + + assert jpg_meta is raw_meta + assert jpg_meta.favorite is True + assert jpg_meta.uploaded is True + assert list(sm.data.entries) == ["photo"] + + +def test_regressed_filename_key_migrates_to_stable_stem_key(mock_sidecar_dir): + """Entries created by the filename-key regression should migrate back.""" + content = { + "version": 2, + "entries": { + "photo.CR2": {"favorite": True}, + }, + } + d = mock_sidecar_dir(content) + sm = SidecarManager(d, None) + + meta = sm.get_metadata(Path("photo.jpg"), create=False) + + assert meta is not None + assert meta.favorite is True + assert "photo" in sm.data.entries + assert "photo.CR2" not in sm.data.entries + assert "photo.jpg" not in sm.data.entries + + +def test_same_stem_in_different_subfolders_do_not_collide(mock_sidecar_dir): + """Relative parent path must namespace same-stem files in different folders.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + first = sm.get_metadata(Path("a") / "photo.CR2") + first.favorite = True + + second = sm.get_metadata(Path("b") / "photo.jpg") + second.uploaded = True + + assert first is not second + assert sm.metadata_key_for_path(Path("a") / "photo.CR2") == "a/photo" + assert sm.metadata_key_for_path(Path("b") / "photo.jpg") == "b/photo" + assert sm.data.entries["a/photo"].favorite is True + assert sm.data.entries["a/photo"].uploaded is False + assert sm.data.entries["b/photo"].favorite is False + assert sm.data.entries["b/photo"].uploaded is True diff --git a/faststack/tests/thumbnail_view/test_folder_stats.py b/faststack/tests/thumbnail_view/test_folder_stats.py index 90866e9..fb36a10 100644 --- a/faststack/tests/thumbnail_view/test_folder_stats.py +++ b/faststack/tests/thumbnail_view/test_folder_stats.py @@ -344,6 +344,18 @@ def test_coverage_buckets_in_stats(self, temp_folder): # With 2 files and default 40 buckets, should have 2 buckets assert len(stats.coverage_buckets) == 2 + def test_coverage_buckets_support_path_keys(self, temp_folder): + """Path-aware sidecar keys should still contribute to sparkline coverage.""" + (temp_folder / "a.jpg").touch() + json_path = temp_folder / "faststack.json" + data = {"entries": {"a.jpg": {"uploaded": True}}} + json_path.write_text(json.dumps(data)) + + stats = read_folder_stats(temp_folder) + + assert stats is not None + assert stats.coverage_buckets == [(1.0, 0.0, 0.0)] + class TestCountImagesInFolder: """Tests for count_images_in_folder function.""" diff --git a/faststack/thumbnail_view/folder_stats.py b/faststack/thumbnail_view/folder_stats.py index 8711f06..de19a7e 100644 --- a/faststack/thumbnail_view/folder_stats.py +++ b/faststack/thumbnail_view/folder_stats.py @@ -247,7 +247,9 @@ def _compute_coverage_buckets( # Efficient stem extraction and metadata lookup stem, _ = os.path.splitext(filename) - meta = entries.get(stem) + meta = entries.get(filename) + if meta is None: + meta = entries.get(stem) if isinstance(meta, dict): if meta.get("uploaded", False): diff --git a/faststack/thumbnail_view/model.py b/faststack/thumbnail_view/model.py index fd08620..f26ee02 100644 --- a/faststack/thumbnail_view/model.py +++ b/faststack/thumbnail_view/model.py @@ -128,7 +128,7 @@ def __init__( self, base_directory: Path, current_directory: Path, - get_metadata_callback: Optional[Callable[[str], dict]] = None, + get_metadata_callback: Optional[Callable[[Path | str], dict]] = None, get_batch_indices_callback: Optional[Callable[[], Set[int]]] = None, get_current_index_callback: Optional[Callable[[], int]] = None, thumbnail_size: int = 200, @@ -406,12 +406,10 @@ def refresh(self): filtered = [] for img in images: try: - meta = self._get_metadata(img.path.stem) + meta = self._get_metadata(img.path) if not isinstance(meta, dict): # Ensure it's a dict before .get() - log.debug( - "Metadata for %s is not a dict: %r", img.path.stem, meta - ) + log.debug("Metadata for %s is not a dict: %r", img.path, meta) continue if all(meta.get(flag, False) for flag in flags): @@ -550,15 +548,15 @@ def refresh_from_controller( for img in images: try: if metadata_map: - meta = metadata_map.get(img.path.stem, {}) + meta = metadata_map.get(normalize_path_key(img.path), {}) elif self._get_metadata: - meta = self._get_metadata(img.path.stem) + meta = self._get_metadata(img.path) else: continue if not isinstance(meta, dict): log.debug( - "Metadata for %s is not a dict: %r", img.path.stem, meta + "Metadata for %s is not a dict: %r", img.path, meta ) continue @@ -612,7 +610,7 @@ def _add_images_to_entries( is_todo = False if metadata_map: - meta = metadata_map.get(img.path.stem, {}) + meta = metadata_map.get(normalize_path_key(img.path), {}) is_stacked = meta.get("stacked", False) is_uploaded = meta.get("uploaded", False) is_edited = meta.get("edited", False) @@ -621,7 +619,7 @@ def _add_images_to_entries( is_todo = meta.get("todo", False) elif self._get_metadata: try: - meta = self._get_metadata(img.path.stem) + meta = self._get_metadata(img.path) if isinstance(meta, dict): is_stacked = meta.get("stacked", False) is_uploaded = meta.get("uploaded", False) @@ -631,7 +629,7 @@ def _add_images_to_entries( is_todo = meta.get("todo", False) else: log.debug( - "Metadata for %s is not a dict: %r", img.path.stem, meta + "Metadata for %s is not a dict: %r", img.path, meta ) except Exception as e: log.debug("Error getting metadata for %s: %s", img.path, e) From c5a5abbced4d8d54bd4c9fff7bf1f03818bb7765 Mon Sep 17 00:00:00 2001 From: Andy Arijs Date: Wed, 18 Mar 2026 09:04:27 +0100 Subject: [PATCH 2/4] Fix-sidecar-dotted-key-migration-and-logging-fallbacks --- faststack/io/sidecar.py | 43 +++++++++++++++++--- faststack/logging_setup.py | 20 ++++++---- faststack/tests/test_sidecar.py | 71 +++++++++++++++++++++++++++++---- 3 files changed, 114 insertions(+), 20 deletions(-) diff --git a/faststack/io/sidecar.py b/faststack/io/sidecar.py index 7371d5f..a46f2c2 100644 --- a/faststack/io/sidecar.py +++ b/faststack/io/sidecar.py @@ -7,9 +7,11 @@ from pathlib import Path from typing import Literal, Optional, Union, overload +from faststack.io.indexer import JPG_EXTENSIONS, RAW_EXTENSIONS from faststack.models import Sidecar, EntryMetadata log = logging.getLogger(__name__) +KNOWN_IMAGE_EXTENSIONS = frozenset(ext.lower() for ext in JPG_EXTENSIONS | RAW_EXTENSIONS) def _entrymetadata_from_json(meta: dict) -> EntryMetadata: @@ -146,6 +148,10 @@ def get_metadata( if no entry exists — callers must handle the None case explicitly. """ stable_key, candidate_keys = self._lookup_keys(image_ref) + if not stable_key: + if create: + raise ValueError(f"image_ref must not be empty: {image_ref!r}") + return None meta = self.data.entries.get(stable_key) if meta is None: @@ -180,10 +186,12 @@ 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.""" path = Path(image_path) + if not path.name: + return "" if not path.is_absolute(): path = self.directory / path - base_dir = Path(os.path.abspath(str(self.directory))) - abs_path = Path(os.path.abspath(str(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) @@ -196,6 +204,8 @@ def metadata_key_for_path(self, image_path: Union[str, Path]) -> str: def _lookup_keys(self, image_ref: Union[str, Path]) -> tuple[str, list[str]]: """Return (stable_key, migration_candidate_keys) for a metadata lookup.""" if isinstance(image_ref, Path): + if not image_ref.name: + return "", [] stable_key = self.metadata_key_for_path(image_ref) full_name_key = self._metadata_filename_key(image_ref) return stable_key, [full_name_key, image_ref.stem] @@ -204,21 +214,34 @@ def _lookup_keys(self, image_ref: Union[str, Path]) -> tuple[str, list[str]]: if not value: return "", [] - if os.path.sep in value or "/" in value or "\\" in value or "." in Path(value).name: + if ( + os.path.sep in value + or "/" in value + or "\\" in value + or Path(value).suffix.lower() in KNOWN_IMAGE_EXTENSIONS + ): path = Path(value) stable_key = self.metadata_key_for_path(path) full_name_key = self._metadata_filename_key(path) return stable_key, [full_name_key, path.stem] + candidate_path = self.directory / value + if candidate_path.exists(): + stable_key = self.metadata_key_for_path(candidate_path) + full_name_key = self._metadata_filename_key(candidate_path) + return stable_key, [full_name_key, value] + return value, [value] def _metadata_filename_key(self, image_path: Union[str, Path]) -> str: """Return the extension-preserving key used by the regressed patch.""" path = Path(image_path) + if not path.name: + return "" if not path.is_absolute(): path = self.directory / path - base_dir = Path(os.path.abspath(str(self.directory))) - abs_path = Path(os.path.abspath(str(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) @@ -230,8 +253,16 @@ def _stable_key_from_key(self, key: str) -> str: """Convert any historical sidecar key form into today's stable key.""" if not key: return "" - if os.path.sep in key or "/" in key or "\\" in key or "." in Path(key).name: + if ( + os.path.sep in key + or "/" in key + or "\\" in key + or Path(key).suffix.lower() in KNOWN_IMAGE_EXTENSIONS + ): return self.metadata_key_for_path(Path(key)) + candidate_path = self.directory / key + if candidate_path.exists(): + return self.metadata_key_for_path(candidate_path) return key def set_last_index(self, index: int): diff --git a/faststack/logging_setup.py b/faststack/logging_setup.py index e783d94..6dda08f 100644 --- a/faststack/logging_setup.py +++ b/faststack/logging_setup.py @@ -3,6 +3,7 @@ import logging import logging.handlers import os +import tempfile from pathlib import Path @@ -12,10 +13,10 @@ def _is_writable_dir(path: Path) -> bool: return False try: - probe = path / ".write_test" - with probe.open("w", encoding="utf-8") as f: + with tempfile.NamedTemporaryFile( + mode="w", encoding="utf-8", dir=path, prefix="faststack-write-", delete=True + ) as f: f.write("ok") - probe.unlink(missing_ok=True) return True except OSError: return False @@ -31,6 +32,8 @@ def _can_create_dir(path: Path) -> bool: parent = next_parent return parent.is_dir() and os.access(parent, os.W_OK) + + def get_app_data_dir() -> Path: """Return a writable application data directory, with fallbacks.""" candidates = [] @@ -54,9 +57,8 @@ def get_app_data_dir() -> Path: if _can_create_dir(candidate): return candidate - # Final fallback: return the first candidate even if unwritable so callers - # still get a deterministic location for error reporting. - return candidates[0] if candidates else Path.cwd() / "var" / "appdata" + # Final fallback: system temp is the most reliable writable location. + return Path(tempfile.gettempdir()) / "faststack" def setup_logging(debug: bool = False): @@ -66,7 +68,11 @@ def setup_logging(debug: bool = False): debug: If True, sets log level to DEBUG. Otherwise, sets to WARNING to reduce noise. """ log_dir = get_app_data_dir() / "logs" - log_dir.mkdir(parents=True, exist_ok=True) + try: + log_dir.mkdir(parents=True, exist_ok=True) + except OSError: + log_dir = Path(tempfile.gettempdir()) / "faststack" / "logs" + log_dir.mkdir(parents=True, exist_ok=True) log_file = log_dir / "app.log" # File handler diff --git a/faststack/tests/test_sidecar.py b/faststack/tests/test_sidecar.py index 1334dac..46f696f 100644 --- a/faststack/tests/test_sidecar.py +++ b/faststack/tests/test_sidecar.py @@ -1,6 +1,7 @@ """Tests for the SidecarManager.""" import json +import os from pathlib import Path import pytest @@ -70,15 +71,17 @@ def test_sidecar_save(mock_sidecar_dir): # Verify file content saved_data = json.loads((d / "faststack.json").read_text()) + expected_key = sm.metadata_key_for_path(Path("IMG_TEST.jpg")) assert saved_data["last_index"] == 10 - assert saved_data["entries"]["IMG_TEST"]["stack_id"] == 99 + assert saved_data["entries"][expected_key]["stack_id"] == 99 def test_sidecar_get_metadata_creates_new(mock_sidecar_dir): """Tests that get_metadata creates a new entry if one doesn't exist.""" d = mock_sidecar_dir() sm = SidecarManager(d, None) - assert "NEW_IMG.jpg" not in sm.data.entries + expected_key = sm.metadata_key_for_path(Path("NEW_IMG.jpg")) + assert expected_key not in sm.data.entries meta = sm.get_metadata(Path("NEW_IMG.jpg")) # EntryMetadata may be a runtime class OR a typing alias, depending on refactors. @@ -89,7 +92,7 @@ def test_sidecar_get_metadata_creates_new(mock_sidecar_dir): assert meta.__class__.__name__ == "EntryMetadata" assert hasattr(meta, "stack_id") - assert "NEW_IMG" in sm.data.entries + assert expected_key in sm.data.entries def test_favorite_toggle_sets_json(mock_sidecar_dir): @@ -105,13 +108,14 @@ def test_favorite_toggle_sets_json(mock_sidecar_dir): meta.favorite = True sm.save() saved = json.loads((d / "faststack.json").read_text()) - assert saved["entries"]["IMG_FAV"]["favorite"] is True + expected_key = sm.metadata_key_for_path(Path("IMG_FAV.jpg")) + assert saved["entries"][expected_key]["favorite"] is True # Toggle off meta.favorite = False sm.save() saved = json.loads((d / "faststack.json").read_text()) - assert saved["entries"]["IMG_FAV"]["favorite"] is False + assert saved["entries"][expected_key]["favorite"] is False def test_favorite_loads_from_sidecar(mock_sidecar_dir): @@ -161,12 +165,16 @@ def test_legacy_stem_entry_migrates_to_path_key(mock_sidecar_dir): sm = SidecarManager(d, None) meta = sm.get_metadata(Path("IMG_0001.jpg"), create=False) + expected_key = sm.metadata_key_for_path(Path("IMG_0001.jpg")) assert meta is not None assert meta.uploaded is True - assert "IMG_0001" in sm.data.entries + assert expected_key in sm.data.entries assert "IMG_0001.jpg" not in sm.data.entries - assert sm.metadata_key_for_path(Path("IMG_0001.jpg")) == "IMG_0001" + if os.name == "nt": + assert expected_key == "img_0001" + else: + assert expected_key == "IMG_0001" def test_raw_only_entry_survives_transition_to_visible_jpg(mock_sidecar_dir): @@ -224,3 +232,52 @@ def test_same_stem_in_different_subfolders_do_not_collide(mock_sidecar_dir): assert sm.data.entries["a/photo"].uploaded is False assert sm.data.entries["b/photo"].favorite is False assert sm.data.entries["b/photo"].uploaded is True + + +def test_semantic_dot_key_is_not_treated_as_path(mock_sidecar_dir): + """Semantic keys like sample.v1 must not be truncated as filenames.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + legacy = EntryMetadata(todo=True) + sm.data.entries["sample.v1"] = legacy + + meta = sm.get_metadata("sample.v1", create=False) + + assert meta is legacy + assert "sample.v1" in sm.data.entries + assert "sample" not in sm.data.entries + + +def test_empty_string_create_false_returns_none(mock_sidecar_dir): + """Empty string refs must not create a sidecar entry.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + assert sm.get_metadata("", create=False) is None + assert "" not in sm.data.entries + + +def test_empty_string_create_true_raises(mock_sidecar_dir): + """Empty string refs should fail fast when asked to create metadata.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + with pytest.raises(ValueError, match="image_ref must not be empty"): + sm.get_metadata("", create=True) + + assert "" not in sm.data.entries + + +def test_metadata_key_for_path_normalizes_case_on_windows(mock_sidecar_dir): + """Windows path casing should not create duplicate stable keys.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + key_upper = sm.metadata_key_for_path(Path("Mixed/Case/IMG_0001.JPG")) + key_lower = sm.metadata_key_for_path(Path("mixed/case/img_0001.jpg")) + + if os.name == "nt": + assert key_upper == key_lower + else: + assert key_upper != "" + assert key_lower != "" From bc52c392a760d48cfd08dbc0e20fe32381fb6497 Mon Sep 17 00:00:00 2001 From: Andy Arijs Date: Wed, 18 Mar 2026 09:13:01 +0100 Subject: [PATCH 3/4] Fix-logging-fallback-to-console-only --- faststack/logging_setup.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/faststack/logging_setup.py b/faststack/logging_setup.py index 6dda08f..7a52794 100644 --- a/faststack/logging_setup.py +++ b/faststack/logging_setup.py @@ -68,21 +68,22 @@ def setup_logging(debug: bool = False): debug: If True, sets log level to DEBUG. Otherwise, sets to WARNING to reduce noise. """ log_dir = get_app_data_dir() / "logs" + log_file = None try: log_dir.mkdir(parents=True, exist_ok=True) except OSError: log_dir = Path(tempfile.gettempdir()) / "faststack" / "logs" - log_dir.mkdir(parents=True, exist_ok=True) - log_file = log_dir / "app.log" + try: + log_dir.mkdir(parents=True, exist_ok=True) + except OSError: + log_dir = None + + if log_dir is not None: + log_file = log_dir / "app.log" - # File handler - file_handler = logging.handlers.RotatingFileHandler( - log_file, maxBytes=10 * 1024 * 1024, backupCount=5 - ) formatter = logging.Formatter( "%(asctime)s - %(name)s - %(levelname)s - %(message)s" ) - file_handler.setFormatter(formatter) # Console handler (for seeing logs in terminal) console_handler = logging.StreamHandler() @@ -92,9 +93,15 @@ def setup_logging(debug: bool = False): # Set log level based on debug flag root_logger.setLevel(logging.DEBUG if debug else logging.WARNING) root_logger.handlers.clear() - root_logger.addHandler(file_handler) root_logger.addHandler(console_handler) + if log_file is not None: + file_handler = logging.handlers.RotatingFileHandler( + log_file, maxBytes=10 * 1024 * 1024, backupCount=5 + ) + file_handler.setFormatter(formatter) + root_logger.addHandler(file_handler) + # Configure logging for key modules if debug: logging.getLogger("faststack.imaging.cache").setLevel(logging.DEBUG) From 32614872effb2aa49fd6ea38811dc370c49797e4 Mon Sep 17 00:00:00 2001 From: Andy Arijs Date: Wed, 18 Mar 2026 09:19:55 +0100 Subject: [PATCH 4/4] Add-tempdir-warning-and-empty-Path-tests --- faststack/logging_setup.py | 10 +++++++++- faststack/tests/test_sidecar.py | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/faststack/logging_setup.py b/faststack/logging_setup.py index 7a52794..74122c4 100644 --- a/faststack/logging_setup.py +++ b/faststack/logging_setup.py @@ -6,6 +6,8 @@ import tempfile from pathlib import Path +log = logging.getLogger(__name__) + def _is_writable_dir(path: Path) -> bool: """Return True when an existing directory accepts file writes.""" @@ -58,7 +60,13 @@ def get_app_data_dir() -> Path: return candidate # Final fallback: system temp is the most reliable writable location. - return Path(tempfile.gettempdir()) / "faststack" + fallback = Path(tempfile.gettempdir()) / "faststack" + log.warning( + "No writable app-data directory found; falling back to temp directory %s. " + "Configuration and logs may not persist across restarts.", + fallback, + ) + return fallback def setup_logging(debug: bool = False): diff --git a/faststack/tests/test_sidecar.py b/faststack/tests/test_sidecar.py index 46f696f..dbf358f 100644 --- a/faststack/tests/test_sidecar.py +++ b/faststack/tests/test_sidecar.py @@ -268,6 +268,26 @@ def test_empty_string_create_true_raises(mock_sidecar_dir): assert "" not in sm.data.entries +def test_empty_path_create_false_returns_none(mock_sidecar_dir): + """Path('') should be treated the same as an empty string.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + assert sm.get_metadata(Path(""), create=False) is None + assert "" not in sm.data.entries + + +def test_empty_path_create_true_raises(mock_sidecar_dir): + """Path('') should fail fast when asked to create metadata.""" + d = mock_sidecar_dir() + sm = SidecarManager(d, None) + + with pytest.raises(ValueError, match="image_ref must not be empty"): + sm.get_metadata(Path(""), create=True) + + assert "" not in sm.data.entries + + def test_metadata_key_for_path_normalizes_case_on_windows(mock_sidecar_dir): """Windows path casing should not create duplicate stable keys.""" d = mock_sidecar_dir()