Skip to content

Refactor get_metadata to return None when create=False #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AlanRockefeller merged 2 commits into from
Mar 12, 2026
Merged

Refactor get_metadata to return None when create=False #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de42838
Refactor get_metadata to return None when create=False
AlanRockefeller Mar 12, 2026
4700ea6
update changelog
AlanRockefeller Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ Todo: More testing Linux / Mac. Create Windows .exe. Write better document
- Improved prefetch behavior when zooming or resizing to reduce stale background work.
- Improved thumbnail lookup speed by adding a faster path-to-row mapping.
- Reduced chances of UI state getting out of sync after external file changes.
- Added `@overload` type hints to `SidecarManager.get_metadata` to provide strict static typing based on the state of the `create` parameter.
- Modified `SidecarManager.get_metadata` to accept a `create` boolean parameter. When `create=False`, the method now returns `None` instead of instantiating and saving an empty metadata entry.
- Updated `AppController` read-only operations (such as thumbnail dictionary generation, status checks, and batching) to request metadata with `create=False`.
- Refactored `AppController` flag extraction (e.g., `uploaded`, `favorite`) to explicitly handle `None` values, replacing older, bulky type-checking logic that looked for both `dict` and `object` structures.

## 1.5.9 (2026-02-16)

Expand Down
51 changes: 30 additions & 21 deletions faststack/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1795,7 +1795,7 @@ def jump_to_last_uploaded(self):
# Dynamic look-up of self.sidecar as requested (important for mocks in tests)
meta = self.sidecar.get_metadata(img.path.stem, create=False)

uploaded = meta.uploaded
uploaded = meta.uploaded if meta else False

if uploaded is True:
last_uploaded_index = idx
Expand Down Expand Up @@ -2115,13 +2115,22 @@ def _get_metadata_dict(self, stem: str) -> dict:
"""Get metadata for a file stem as a dict for thumbnail model."""
try:
meta = self.sidecar.get_metadata(stem, create=False)
if meta is None:
return {
"stacked": False,
"uploaded": False,
"edited": False,
"restacked": False,
"favorite": False,
"todo": False,
}
return {
"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),
"stacked": meta.stacked,
"uploaded": meta.uploaded,
"edited": meta.edited,
"restacked": meta.restacked,
"favorite": meta.favorite,
"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)
Expand Down Expand Up @@ -2372,17 +2381,17 @@ def get_current_metadata(self) -> Dict:
self._metadata_cache = {
"filename": filename,
"exif_brief": exif_brief,
"stacked": meta.stacked,
"stacked_date": meta.stacked_date or "",
"uploaded": meta.uploaded,
"uploaded_date": meta.uploaded_date or "",
"edited": meta.edited,
"edited_date": meta.edited_date or "",
"restacked": meta.restacked,
"restacked_date": meta.restacked_date or "",
"favorite": meta.favorite,
"todo": getattr(meta, "todo", False),
"todo_date": getattr(meta, "todo_date", "") or "",
"stacked": meta.stacked if meta else False,
"stacked_date": (meta.stacked_date or "") if meta else "",
"uploaded": meta.uploaded if meta else False,
"uploaded_date": (meta.uploaded_date or "") if meta else "",
"edited": meta.edited if meta else False,
"edited_date": (meta.edited_date or "") if meta else "",
"restacked": meta.restacked if meta else False,
"restacked_date": (meta.restacked_date or "") if meta else "",
"favorite": meta.favorite if meta else False,
"todo": meta.todo if meta else False,
"todo_date": (meta.todo_date or "") if meta else "",
"stack_info_text": stack_info,
"batch_info_text": batch_info,
}
Expand Down Expand Up @@ -2607,7 +2616,7 @@ def add_favorites_to_batch(self):
indices_to_add = []
for i, img in enumerate(self.image_files):
meta = self.sidecar.get_metadata(img.path.stem, create=False)
if meta.favorite:
if meta and meta.favorite:
indices_to_add.append(i)

if not indices_to_add:
Expand Down Expand Up @@ -2662,7 +2671,7 @@ def add_uploaded_to_batch(self):
indices_to_add = []
for i, img in enumerate(self.image_files):
meta = self.sidecar.get_metadata(img.path.stem, create=False)
if meta.uploaded:
if meta and meta.uploaded:
indices_to_add.append(i)

if not indices_to_add:
Expand Down Expand Up @@ -7362,7 +7371,7 @@ def is_stacked(self) -> bool:
return False
stem = self.image_files[self.current_index].path.stem
meta = self.sidecar.get_metadata(stem, create=False)
return meta.stacked
return meta.stacked if meta else False

def _update_cache_stats(self):
if self.debug_cache:
Expand Down
21 changes: 18 additions & 3 deletions faststack/io/sidecar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import logging
import time
from pathlib import Path
from typing import Literal, Optional, overload

from faststack.models import Sidecar, EntryMetadata

Expand Down Expand Up @@ -118,13 +119,27 @@ def save(self):
if was_watcher_running:
self.start_watcher()

def get_metadata(self, image_stem: str, *, create: bool = True) -> EntryMetadata:
"""Get metadata for an image, optionally creating a persistent entry."""
@overload
def get_metadata(self, image_stem: str, *, create: Literal[True] = True) -> EntryMetadata: ...

@overload
def get_metadata(self, image_stem: str, *, create: Literal[False]) -> Optional[EntryMetadata]: ...

@overload
def get_metadata(self, image_stem: str, *, create: bool) -> Optional[EntryMetadata]: ...

def get_metadata(self, image_stem: str, *, create: bool = True) -> Optional[EntryMetadata]:
"""Get metadata for an image, optionally creating a persistent entry.

When create=True (default), always returns an EntryMetadata (creating
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)
if meta is None and create:
meta = EntryMetadata()
self.data.entries[image_stem] = meta
return meta if meta is not None else EntryMetadata()
return meta

def set_last_index(self, index: int):
self.data.last_index = index
4 changes: 2 additions & 2 deletions faststack/tests/test_jump_to_last_uploaded.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def test_jump_to_last_uploaded_success(mock_controller):
meta2 = EntryMetadata(uploaded=False)
meta3 = EntryMetadata(uploaded=True)

def side_effect(stem):
def side_effect(stem, **kwargs):
return {"img1": meta1, "img2": meta2, "img3": meta3}.get(stem, EntryMetadata())
Comment on lines +69 to 70
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Honor create=False in these get_metadata test doubles.

These mocks still fall back to EntryMetadata() for unknown stems, which preserves the old contract and can hide the regression this PR is meant to catch. If jump_to_last_uploaded() looks up a missing stem with create=False, this test would never exercise the new None path.

Suggested fix 
-    def side_effect(stem, **kwargs):
-        return {"img1": meta1, "img2": meta2, "img3": meta3}.get(stem, EntryMetadata())
+    def side_effect(stem, **kwargs):
+        metadata = {"img1": meta1, "img2": meta2, "img3": meta3}
+        if stem in metadata:
+            return metadata[stem]
+        return EntryMetadata() if kwargs.get("create", True) else None

Also applies to: 140-141

🧰 Tools
🪛 Ruff (0.15.5)

[warning] 69-69: Unused function argument: kwargs

(ARG001)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@faststack/tests/test_jump_to_last_uploaded.py` around lines 69 - 70, The test
double function side_effect used for get_metadata must honor the create flag:
update the side_effect(s) in test_jump_to_last_uploaded.py (the mock used for
get_metadata) to check kwargs.get("create", True) and return EntryMetadata()
only when create is True and the stem is unknown; if create is False and the
stem is not in {"img1","img2","img3"} return None so the test exercises the
missing-stem path (apply same change to the other occurrence around lines
140-141).


mock_controller.sidecar.get_metadata.side_effect = side_effect
Expand Down Expand Up @@ -137,7 +137,7 @@ 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):
def side_effect(stem, **kwargs):
return {"img1": meta1, "img2": meta2, "img3": meta3}.get(stem, EntryMetadata())

mock_controller.sidecar.get_metadata.side_effect = side_effect
Expand Down
Loading