From c7de53f734b34cc01747bef5687637deea5f7b37 Mon Sep 17 00:00:00 2001 From: Mathias Scheider Date: Sun, 18 Oct 2026 09:04:36 +0200 Subject: [PATCH 1/7] Add batch photo processing with mode-specific LLM sessions and metadata writes --- src/photools3/llm_session.py | 43 +++++++++++ src/photools3/metadata_writer.py | 33 ++++++-- src/photools3/models.py | 36 +++++++++ src/photools3/photo.py | 14 +++- src/photools3/processing.py | 128 ++++++++++++++++++++++++++++++ test/test_metadata_writer.py | 30 +++++++ test/test_photo.py | 9 +++ test/test_processing.py | 129 +++++++++++++++++++++++++++++++ 8 files changed, 415 insertions(+), 7 deletions(-) create mode 100644 src/photools3/processing.py create mode 100644 test/test_processing.py diff --git a/src/photools3/llm_session.py b/src/photools3/llm_session.py index c79a19f..14922af 100644 --- a/src/photools3/llm_session.py +++ b/src/photools3/llm_session.py @@ -12,8 +12,10 @@ from photools3.models import ( LlmMessageType, LlmRequest, LlmRequestMessage, + LlmResponseCaption, LlmResponseCaptionsAndKeywords, LlmResponseGeneral, + LlmResponseKeywords, LlmResponseQuality, ) from photools3.photo import Photo @@ -63,6 +65,15 @@ class LlmSession: api_key=self.api_key, ) + def close(self): + self.client.close() + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.close() + def client_chat_completion_create(self, request: LlmRequest, choice_no: int = 0): response = self.client.chat.completions.create(**request.model_dump()) return response.choices[choice_no].message.content @@ -171,6 +182,38 @@ class LlmSessionCaptionsAndKeywords(LlmSession): ) +class LlmSessionCaption(LlmSession): + response_model = LlmResponseCaption + prompt = "Write a caption for the following photo" + + def __init__(self, temperature: float = settings.llm_default_temperature): + super().__init__( + system_prompt=( + "You are a helpful photographer's assistant that writes captions " + "for photos. Answer in German.\n" + "Output must be a JSON of the following schema:\n" + f"{self.response_model.model_json_schema()}" + ), + temperature=temperature, + ) + + +class LlmSessionKeywords(LlmSession): + response_model = LlmResponseKeywords + prompt = "Choose appropriate keywords for the following photo" + + def __init__(self, temperature: float = settings.llm_default_temperature): + super().__init__( + system_prompt=( + "You are a helpful photographer's assistant that chooses keywords " + "for photos. Answer in German.\n" + "Output must be a JSON of the following schema:\n" + f"{self.response_model.model_json_schema()}" + ), + temperature=temperature, + ) + + class LlmSessionQuality(LlmSession): response_model = LlmResponseQuality prompt = "Assess the quality of the following photos to help me decide which ones to sort out" diff --git a/src/photools3/metadata_writer.py b/src/photools3/metadata_writer.py index 7370838..4df2a87 100644 --- a/src/photools3/metadata_writer.py +++ b/src/photools3/metadata_writer.py @@ -87,12 +87,12 @@ class MetadataWriter: return metadata def get_caption(self) -> str: - result = self.metadata.get(XMP_KEYS_CAPTION[0], "") + result = (self.metadata or {}).get(XMP_KEYS_CAPTION[0], "") assert isinstance(result, str) return result def get_keywords(self) -> list[str]: - result = self.metadata.get(XMP_KEYS_KEYWORDS[0], []) or [] + result = (self.metadata or {}).get(XMP_KEYS_KEYWORDS[0], []) or [] if isinstance(result, str): result = [result] assert isinstance(result, list) @@ -108,8 +108,11 @@ class MetadataWriter: tags=metadata_tags, params=["-overwrite_original"], ) - except Exception as exc: # noqa: BLE001 + except Exception as exc: log.error(f"Could not write metadata to file {self.sidecar_path}: {exc}") + raise RuntimeError( + f"Could not write metadata to file {self.sidecar_path}" + ) from exc def write_caption( self, caption: Caption, include_prefix: bool = True, override: bool = True @@ -151,9 +154,27 @@ class MetadataWriter: keywords: Keywords | None = None, color: Color | None = None, ): + metadata_tags: XmpMetadata = {} if caption is not None: - self.write_caption(caption) + existing_no_prefix, _ = _separate_prefixed_caption(self.get_caption()) + if existing_no_prefix: + existing_no_prefix = ( + f"{existing_no_prefix}." + if not existing_no_prefix.endswith(".") + else existing_no_prefix + ) + existing_no_prefix = f"{existing_no_prefix} " + caption = f"{existing_no_prefix}{_prefixed(caption)}" + metadata_tags.update({key: caption for key in XMP_KEYS_CAPTION}) + if keywords is not None: - self.write_keywords(keywords) + keywords = list(map(_prefixed, keywords)) + existing = self.get_keywords() + keywords = list(set(existing + keywords)) + metadata_tags.update({tag: keywords for tag in XMP_KEYS_KEYWORDS}) + if color is not None: - self.write_color(color) + metadata_tags["XMP-darktable:Colorlabels"] = color + + if metadata_tags: + self.set_metadata(metadata_tags) diff --git a/src/photools3/models.py b/src/photools3/models.py index f3b27da..d545c00 100644 --- a/src/photools3/models.py +++ b/src/photools3/models.py @@ -86,6 +86,42 @@ class LlmResponseGeneral(BaseModel): text: str +class LlmResponseCaption(BaseModel): + caption: Annotated[ + str, + Field( + description=( + "A brief but precise description of what the picture shows. " + "No interpretation; just what is visible" + ) + ), + ] + + +class LlmResponseKeywords(BaseModel): + tags: list[str] = Field( + description=( + "A list of keywords that properly tag the photo. " + "Allowed keywords: " + + str(load_valid_keywords()) + .replace("'", '"') + .replace("{", "[") + .replace("}", "]") + ) + ) + + @field_validator("tags") + @classmethod + def validate_tags(cls, v: list[str]) -> list[str]: + valid_keywords = load_valid_keywords() + for tag in v: + if tag not in valid_keywords: + raise ValueError( + f"'{tag}' is not a valid keyword. Must be one from the configuration." + ) + return v + + class LlmResponseCaptionsAndKeywords(BaseModel): caption: Annotated[ str, diff --git a/src/photools3/photo.py b/src/photools3/photo.py index 738254f..f57b0ce 100644 --- a/src/photools3/photo.py +++ b/src/photools3/photo.py @@ -1,6 +1,7 @@ import base64 from io import BytesIO from pathlib import Path +from typing import Self from PIL import Image @@ -17,6 +18,15 @@ class Photo: self.path = path self.image = Image.open(path) + def close(self): + self.image.close() + + def __enter__(self) -> Self: + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.close() + def resize(self, image_max_size=1024) -> Photo: """Limiting max dimensions while maintaining aspect ratio""" self.image.thumbnail((image_max_size, image_max_size), Image.Resampling.LANCZOS) @@ -24,7 +34,9 @@ class Photo: def to_rgb(self): if self.image.mode != "RGB": - self.image = self.image.convert("RGB") + original = self.image + self.image = original.convert("RGB") + original.close() return self def get_encoded(self): diff --git a/src/photools3/processing.py b/src/photools3/processing.py new file mode 100644 index 0000000..5e71ee1 --- /dev/null +++ b/src/photools3/processing.py @@ -0,0 +1,128 @@ +import logging +from collections.abc import Iterable +from dataclasses import dataclass +from enum import StrEnum +from pathlib import Path + +from photools3.llm_session import ( + LlmSession, + LlmSessionCaption, + LlmSessionCaptionsAndKeywords, + LlmSessionKeywords, +) +from photools3.metadata_writer import MetadataWriter +from photools3.models import ( + LlmResponseCaption, + LlmResponseCaptionsAndKeywords, + LlmResponseKeywords, +) +from photools3.photo import Photo + +logger = logging.getLogger(__name__) + +type ProcessingResponse = ( + LlmResponseCaption | LlmResponseKeywords | LlmResponseCaptionsAndKeywords +) + + +class ProcessingMode(StrEnum): + KEYWORDS = "keywords" + CAPTIONS = "captions" + BOTH = "both" + + +@dataclass(frozen=True) +class ProcessingOutcome: + path: Path + result: ProcessingResponse | None = None + error: str | None = None + + @property + def succeeded(self) -> bool: + return self.error is None and self.result is not None + + +def _create_session(mode: ProcessingMode) -> LlmSession: + if mode == ProcessingMode.KEYWORDS: + return LlmSessionKeywords() + if mode == ProcessingMode.CAPTIONS: + return LlmSessionCaption() + return LlmSessionCaptionsAndKeywords() + + +def process_photo( + path: Path | str, + mode: ProcessingMode | str = ProcessingMode.BOTH, + *, + session: LlmSession | None = None, +) -> ProcessingResponse: + """Process one photo and write the requested LLM result to its metadata. + + If a session is supplied, its lifecycle remains the caller's responsibility. + """ + path = Path(path) + mode = ProcessingMode(mode) + owns_session = session is None + active_session = session if session is not None else _create_session(mode) + + try: + with Photo(path) as photo: + result = active_session.call([photo]) + + caption: str | None = None + keywords: list[str] | None = None + if mode == ProcessingMode.CAPTIONS: + if not isinstance(result, LlmResponseCaption): + raise TypeError("Caption mode returned an unexpected response type") + caption = result.caption + elif mode == ProcessingMode.KEYWORDS: + if not isinstance(result, LlmResponseKeywords): + raise TypeError("Keywords mode returned an unexpected response type") + keywords = result.tags + else: + if not isinstance(result, LlmResponseCaptionsAndKeywords): + raise TypeError("Both mode returned an unexpected response type") + caption = result.caption + keywords = result.tags + + MetadataWriter(path).write(caption=caption, keywords=keywords) + return result + finally: + if owns_session: + active_session.close() + + +def process_batch( + paths: Iterable[Path | str], + mode: ProcessingMode | str = ProcessingMode.BOTH, +) -> list[ProcessingOutcome]: + """Process photos sequentially, reusing one LLM client for the whole batch. + + A failed photo is recorded in its outcome and does not stop later photos. + Only one decoded image is held at a time. + """ + mode = ProcessingMode(mode) + session = _create_session(mode) + outcomes: list[ProcessingOutcome] = [] + + try: + for item in paths: + path = Path(item) + try: + result = process_photo(path, mode, session=session) + except Exception as exc: + logger.exception("Failed to process %s", path) + # Keep only a string, not the exception/traceback, which can + # retain references to the just-processed image and stack. + outcomes.append( + ProcessingOutcome( + path=path, + error=f"{type(exc).__name__}: {exc}", + ) + ) + else: + outcomes.append(ProcessingOutcome(path=path, result=result)) + finally: + session.close() + + return outcomes diff --git a/test/test_metadata_writer.py b/test/test_metadata_writer.py index b2b96a6..0c8aed1 100644 --- a/test/test_metadata_writer.py +++ b/test/test_metadata_writer.py @@ -1,5 +1,6 @@ import shutil from pathlib import Path +from unittest.mock import patch import pytest @@ -211,3 +212,32 @@ def test_set_keywords(make_dng_file, pic, expected_keywords): def test_set_keywords_on_existing(): # TODO ... + + +def test_write_caption_and_keywords_in_one_metadata_update(tmp_path): + writer = MetadataWriter(tmp_path / "photo.jpg") + with ( + patch.object(writer, "get_caption", return_value="Original caption"), + patch.object(writer, "get_keywords", return_value=["existing"]), + patch.object(writer, "set_metadata") as set_metadata, + ): + writer.write(caption="Generated caption", keywords=["architektur"]) + + set_metadata.assert_called_once() + metadata_tags = set_metadata.call_args.args[0] + assert metadata_tags["XMP:Description"] == ( + f"Original caption. {PREFIX}Generated caption" + ) + assert set(metadata_tags["XMP:Subject"]) == { + "existing", + f"{PREFIX}architektur", + } + + +def test_set_metadata_surfaces_write_failures(tmp_path): + writer = MetadataWriter(tmp_path / "photo.jpg") + with patch("photools3.metadata_writer.exiftool.ExifToolHelper") as exif: + exif_instance = exif.return_value.__enter__.return_value + exif_instance.execute.side_effect = OSError("ExifTool failed") + with pytest.raises(RuntimeError, match="Could not write metadata"): + writer.set_metadata({"XMP:Description": "caption"}) diff --git a/test/test_photo.py b/test/test_photo.py index 0c924fe..7d8a61f 100644 --- a/test/test_photo.py +++ b/test/test_photo.py @@ -48,6 +48,15 @@ def test_load_photo(): assert isinstance(photo.image, Image.Image) +def test_photo_context_manager_closes_image(tmp_path): + path = _make_image_file(tmp_path, "RGB") + with Photo(path) as photo: + image = photo.image + assert image.fp is not None + + assert image.fp is None + + def test_resize_method_exists(): """Test that resize method exists and returns a Photo instance""" photo = Photo(PATHS_DNG_EXAMPLES[0]) diff --git a/test/test_processing.py b/test/test_processing.py new file mode 100644 index 0000000..c1a27d3 --- /dev/null +++ b/test/test_processing.py @@ -0,0 +1,129 @@ +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest +from PIL import Image + +from photools3.models import ( + LlmResponseCaption, + LlmResponseCaptionsAndKeywords, + LlmResponseKeywords, +) +from photools3.processing import ( + ProcessingMode, + _create_session, + process_batch, + process_photo, +) + + +@pytest.fixture +def image_path(tmp_path): + path = tmp_path / "photo.png" + Image.new("RGB", (20, 20)).save(path) + return path + + +@pytest.mark.parametrize( + ("mode", "response", "expected_metadata"), + [ + ( + ProcessingMode.KEYWORDS, + LlmResponseKeywords(tags=["architektur"]), + {"caption": None, "keywords": ["architektur"]}, + ), + ( + ProcessingMode.CAPTIONS, + LlmResponseCaption(caption="Ein Haus"), + {"caption": "Ein Haus", "keywords": None}, + ), + ( + ProcessingMode.BOTH, + LlmResponseCaptionsAndKeywords(caption="Ein Haus", tags=["architektur"]), + {"caption": "Ein Haus", "keywords": ["architektur"]}, + ), + ], +) +def test_process_photo_writes_only_requested_metadata( + image_path, mode, response, expected_metadata +): + session = MagicMock() + session.call.return_value = response + + with ( + patch("photools3.processing._create_session", return_value=session), + patch("photools3.processing.MetadataWriter") as writer_class, + ): + result = process_photo(image_path, mode) + + assert result == response + session.call.assert_called_once() + session.close.assert_called_once() + writer_class.assert_called_once_with(image_path) + writer_class.return_value.write.assert_called_once_with(**expected_metadata) + + +def test_process_photo_does_not_close_a_caller_owned_session(image_path): + session = MagicMock() + session.call.return_value = LlmResponseCaption(caption="Ein Haus") + + with patch("photools3.processing.MetadataWriter"): + process_photo(image_path, ProcessingMode.CAPTIONS, session=session) + + session.close.assert_not_called() + + +@pytest.mark.parametrize( + ("mode", "constructor"), + [ + (ProcessingMode.KEYWORDS, "LlmSessionKeywords"), + (ProcessingMode.CAPTIONS, "LlmSessionCaption"), + (ProcessingMode.BOTH, "LlmSessionCaptionsAndKeywords"), + ], +) +def test_create_session_selects_mode_specific_response_model(mode, constructor): + with patch(f"photools3.processing.{constructor}") as session_class: + assert _create_session(mode) is session_class.return_value + + session_class.assert_called_once_with() + + +def test_process_batch_is_sequential_reuses_session_and_continues_after_error(): + active_photos = 0 + max_active_photos = 0 + + class TrackingPhoto: + def __init__(self, path): + self.path = path + + def __enter__(self): + nonlocal active_photos, max_active_photos + active_photos += 1 + max_active_photos = max(max_active_photos, active_photos) + return self + + def __exit__(self, exc_type, exc_value, traceback): + nonlocal active_photos + active_photos -= 1 + + session = MagicMock() + response = LlmResponseCaptionsAndKeywords(caption="Ein Haus", tags=["architektur"]) + session.call.side_effect = [response, RuntimeError("temporary error"), response] + paths = [Path("one.jpg"), Path("two.jpg"), Path("three.jpg")] + + with ( + patch("photools3.processing._create_session", return_value=session), + patch("photools3.processing.Photo", TrackingPhoto), + patch("photools3.processing.MetadataWriter") as writer_class, + ): + outcomes = process_batch(paths, ProcessingMode.BOTH) + + assert [outcome.succeeded for outcome in outcomes] == [True, False, True] + assert [outcome.path for outcome in outcomes] == paths + assert outcomes[1].error == "RuntimeError: temporary error" + assert max_active_photos == 1 + assert active_photos == 0 + assert session.call.call_count == 3 + assert all(len(call.args[0]) == 1 for call in session.call.call_args_list) + assert writer_class.return_value.write.call_count == 2 + session.close.assert_called_once() -- 2.54.0 From e818e0f31e9d9c46c1633d76f60bd901505e4284 Mon Sep 17 00:00:00 2001 From: Mathias Scheider Date: Sun, 18 Oct 2026 09:23:54 +0200 Subject: [PATCH 2/7] Add metadata-magic CLI for photo metadata generation and document usage. --- README.md | 86 ++++++++++++++++ src/photools3/cli.py | 135 ++++++++++++++++++++----- test/test_cli.py | 233 ++++++++++++++++++++++++++++++------------- 3 files changed, 360 insertions(+), 94 deletions(-) diff --git a/README.md b/README.md index e69de29..971625d 100644 --- a/README.md +++ b/README.md @@ -0,0 +1,86 @@ +# PhotoTools3 + +PhotoTools3 uses an OpenAI-compatible vision API to generate photo captions and +keywords, then writes the results to photo metadata. It can also assess photo +quality. + +## Setup + +Install the Python dependencies with `uv` and make sure ExifTool is installed and +available on `PATH` (it is a system dependency, not a Python package): + +```bash +uv sync +``` + +Create a `.env` file in the project root with the LLM connection details and the +prefix used to identify metadata written by PhotoTools3: + +```dotenv +llm_api_base_url=https://your-openai-compatible-api/v1 +llm_api_key=your-api-key +llm_default_temperature=0.2 +metadata_prefix=photools3: +``` + +Keyword suggestions are restricted to the vocabulary in `config/keywords.txt`. + +## Generate metadata + +The `metadata-magic` command accepts one or more image paths and/or directories. +The default mode is `both`, which generates and writes a caption and keywords: + +```bash +uv run photools3 metadata-magic photo.dng +``` + +Choose a mode with `--mode`: + +```bash +# Generate only keywords +uv run photools3 metadata-magic photo.dng --mode keywords + +# Generate only a caption +uv run photools3 metadata-magic photo.dng --mode captions + +# Explicitly request both (the default) +uv run photools3 metadata-magic photo.dng --mode both +``` + +Pass multiple files to process them sequentially as a batch: + +```bash +uv run photools3 metadata-magic photo1.dng photo2.jpg --mode both +``` + +Directories are scanned at the top level by default. Add `--recursive` to +include supported images in subdirectories. You can combine directories and +individual file paths: + +```bash +uv run photools3 metadata-magic ./photos --recursive --mode keywords +uv run photools3 metadata-magic ./photos photo-to-include.dng --mode captions +``` + +The command reports each processed file and exits with a non-zero status if any +file fails. In a batch, it continues processing the remaining files after an +individual failure. Generated metadata is written using ExifTool and XMP +sidecars. + +## Assess photo quality + +The `quality` command reports an assessment without changing photo metadata: + +```bash +uv run photools3 quality photo.dng +``` + +## Help + +Show the available commands and options with: + +```bash +uv run photools3 --help +uv run photools3 metadata-magic --help +uv run photools3 quality --help +``` diff --git a/src/photools3/cli.py b/src/photools3/cli.py index 11a7590..971b7c0 100644 --- a/src/photools3/cli.py +++ b/src/photools3/cli.py @@ -3,13 +3,16 @@ from pathlib import Path from typing import Annotated import typer +from PIL import Image from utils.logging_config import LoggingSettings -from photools3.llm_session import ( - LlmSessionCaptionsAndKeywords, - LlmSessionQuality, -) +from photools3.llm_session import LlmSessionQuality from photools3.photo import Photo +from photools3.processing import ( + ProcessingMode, + process_batch, + process_photo, +) app = typer.Typer(help="PhotoTools3: A CLI for image processing.") LoggingSettings() @@ -21,9 +24,7 @@ def main( ctx: typer.Context, version: bool | None = typer.Option(None, "--version", is_eager=True), ): - """ - Main entry point for PhotoTools3 CLI. - """ + """Main entry point for PhotoTools3 CLI.""" if version: typer.echo("photools3 version 0.1.0") raise typer.Exit() @@ -33,36 +34,118 @@ def main( raise typer.Exit() -@app.command("keywords") -def keywords( - file_path: Annotated[Path, typer.Argument(..., help="Path to the image file")], +@app.command("metadata-magic") +def metadata_magic( + paths: Annotated[ + list[Path], typer.Argument(..., help="Photo file(s) and/or directory paths") + ], + mode: Annotated[ + ProcessingMode, + typer.Option( + "--mode", + case_sensitive=False, + help="Metadata to generate and write", + ), + ] = ProcessingMode.BOTH, + recursive: Annotated[ + bool, + typer.Option( + "--recursive/--no-recursive", + help="Recursively discover photos inside input directories", + ), + ] = False, ): - session = LlmSessionCaptionsAndKeywords() - _run_llm_task(file_path, session) + """Generate and write keywords, captions, or both for photos.""" + _run_processing(paths, mode, recursive) @app.command("quality") def quality( file_path: Annotated[Path, typer.Argument(..., help="Path to the image file")], ): - session = LlmSessionQuality() - _run_llm_task(file_path, session) - - -def _run_llm_task( - file_path: Path, session: LlmSessionCaptionsAndKeywords | LlmSessionQuality -): + """Assess a photo's quality without changing its metadata.""" try: - logger.info(f"Processing: {file_path}") - photo = Photo(file_path) - result = session.call([photo]) - logger.info(result) + with LlmSessionQuality() as session, Photo(file_path) as photo: + result = session.call([photo]) + logger.info("Quality assessment for %s: %s", file_path, result) except FileNotFoundError: - logger.error(f"Error: File '{file_path}' does not exist.") + logger.error("Photo not found: %s", file_path) + raise typer.Exit(code=1) from None + except Exception as exc: + logger.error("Error assessing photo %s: %s", file_path, exc) + raise typer.Exit(code=1) from exc + + +def _run_processing(inputs: list[Path], mode: ProcessingMode, recursive: bool) -> None: + photo_paths = _expand_photo_paths(inputs, recursive) + if not photo_paths: + logger.error("No supported photo files found in the given input paths.") raise typer.Exit(code=1) - except Exception as e: # noqa: BLE001 + + logger.info("Processing %d photo(s) in %s mode", len(photo_paths), mode.value) + if len(photo_paths) == 1: + path = photo_paths[0] + try: + process_photo(path, mode) + except FileNotFoundError: + logger.error("Photo not found: %s", path) + raise typer.Exit(code=1) from None + except Exception as exc: + logger.error("Failed to process %s: %s", path, exc) + raise typer.Exit(code=1) from exc + logger.info("Processed: %s", path) + return + + try: + outcomes = process_batch(photo_paths, mode) + except Exception as exc: + logger.error("Batch processing failed: %s", exc) + raise typer.Exit(code=1) from exc + + failures = 0 + for outcome in outcomes: + if outcome.succeeded: + logger.info("Processed: %s", outcome.path) + else: + failures += 1 + logger.error("Failed to process %s: %s", outcome.path, outcome.error) + + logger.info( + "Batch complete: %d succeeded, %d failed", len(outcomes) - failures, failures + ) + if failures: raise typer.Exit(code=1) - logger.error(f"Error processing image: {e}") + + +def _expand_photo_paths(inputs: list[Path], recursive: bool) -> list[Path]: + """Expand directories to supported image files, preserving input order.""" + supported_extensions = { + ".dng", + *(extension.lower() for extension in Image.registered_extensions()), + } + discovered: list[Path] = [] + + for path in inputs: + if not path.is_dir(): + discovered.append(path) + continue + + try: + candidates = path.rglob("*") if recursive else path.iterdir() + discovered.extend( + sorted( + candidate + for candidate in candidates + if candidate.is_file() + and candidate.suffix.lower() in supported_extensions + ) + ) + except OSError as exc: + logger.error("Could not read directory %s: %s", path, exc) + raise typer.Exit(code=1) from exc + + # Avoid processing the same exact path more than once while retaining order. + return list(dict.fromkeys(discovered)) if __name__ == "__main__": diff --git a/test/test_cli.py b/test/test_cli.py index 643a1e7..8bab510 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -1,100 +1,197 @@ -import logging from pathlib import Path +from types import SimpleNamespace from unittest.mock import patch -import pytest from typer.testing import CliRunner from photools3.cli import app +from photools3.processing import ProcessingMode runner = CliRunner() -CLI_LOGGER = "photools3.cli" + +def _success_outcome(path: Path): + return SimpleNamespace(path=path, succeeded=True, error=None) -@pytest.fixture -def mock_photo_logic(tmp_path): - """Fixture to mock the core logic of keywords command.""" +def test_metadata_magic_command_defaults_to_both_for_single_photo(tmp_path): + image_path = tmp_path / "photo.jpg" + image_path.write_bytes(b"test image") + with ( - patch("photools3.cli.LlmSessionCaptionsAndKeywords") as mocked_session, - patch("photools3.cli.Photo") as mocked_photo, + patch("photools3.cli.process_photo") as process_photo, + patch("photools3.cli.process_batch") as process_batch, ): - - # Configure session mock - session_instance = mocked_session.return_value - session_instance.call.return_value = { - "caption": "A beautiful test image", - "args": ["test", "mock", "image"], # Wait, the code uses 'tags' - } - # Let me re-check the code in cli.py - # result.get("tags", []) - session_instance.call.return_value = { - "caption": "A beautiful test image", - "tags": ["test", "mock", "image"], - } - - # Configure photo mock - photo_instance = mocked_photo.return_value - photo_instance.path = tmp_path / "test_image.jpg" - - yield { - "session": session_instance, - "photo_class": mocked_photo, - "photo_instance": photo_instance, - } - - -@pytest.fixture -def dummy_file(tmp_path): - """Fixture to create a dummy file for CLI testing.""" - dummy_path = tmp_path / "test_image.jpg" - dummy_path.write_text("dummy content") - return dummy_path - - -def test_keywords_command_success(mock_photo_logic, dummy_file, caplog): - """Test the 'keywords' command with a valid file path.""" - # Ensure the mock photo instance points to our dummy file - mock_photo_logic["photo_class"].return_value.path = dummy_file - - with caplog.at_level(logging.INFO, logger=CLI_LOGGER): - result = runner.invoke(app, ["keywords", str(dummy_file)]) + result = runner.invoke(app, ["metadata-magic", str(image_path)]) assert result.exit_code == 0 - assert "Processing: " in caplog.text - # The result is logged as a raw dictionary string - assert ( - "{'caption': 'A beautiful test image', 'tags': ['test', 'mock', 'image']}" - in caplog.text + process_photo.assert_called_once_with(image_path, ProcessingMode.BOTH) + process_batch.assert_not_called() + + +def test_metadata_magic_command_selects_caption_mode(tmp_path): + image_path = tmp_path / "photo.jpg" + image_path.write_bytes(b"test image") + + with patch("photools3.cli.process_photo") as process_photo: + result = runner.invoke( + app, ["metadata-magic", str(image_path), "--mode", "captions"] + ) + + assert result.exit_code == 0 + process_photo.assert_called_once_with(image_path, ProcessingMode.CAPTIONS) + + +def test_metadata_magic_command_batches_multiple_files(tmp_path): + paths = [tmp_path / "one.jpg", tmp_path / "two.jpg"] + for path in paths: + path.write_bytes(b"test image") + + with ( + patch("photools3.cli.process_photo") as process_photo, + patch("photools3.cli.process_batch") as process_batch, + ): + process_batch.return_value = [_success_outcome(path) for path in paths] + result = runner.invoke( + app, + ["metadata-magic", *(str(path) for path in paths), "--mode", "keywords"], + ) + + assert result.exit_code == 0 + process_photo.assert_not_called() + process_batch.assert_called_once_with(paths, ProcessingMode.KEYWORDS) + + +def test_metadata_magic_command_expands_directories_recursively(tmp_path): + root = tmp_path / "photos" + nested = root / "nested" + nested.mkdir(parents=True) + top_level_photo = root / "one.jpg" + nested_photo = nested / "two.png" + sidecar = root / "one.jpg.xmp" + for path in (top_level_photo, nested_photo, sidecar): + path.write_bytes(b"placeholder") + + with ( + patch("photools3.cli.process_photo") as process_photo, + patch("photools3.cli.process_batch") as process_batch, + ): + process_batch.return_value = [ + _success_outcome(nested_photo), + _success_outcome(top_level_photo), + ] + result = runner.invoke( + app, ["metadata-magic", str(root), "--recursive", "--mode", "both"] + ) + + assert result.exit_code == 0 + process_photo.assert_not_called() + process_batch.assert_called_once_with( + [nested_photo, top_level_photo], ProcessingMode.BOTH ) -def test_keywords_command_file_not_found(caplog): - """Test the 'keywords' command with a non-existent file path.""" - non_existent_file = Path("non_existent_file.jpg") +def test_metadata_magic_command_does_not_recurse_by_default(tmp_path): + root = tmp_path / "photos" + nested = root / "nested" + nested.mkdir(parents=True) + top_level_photo = root / "one.jpg" + nested_photo = nested / "two.jpg" + top_level_photo.write_bytes(b"placeholder") + nested_photo.write_bytes(b"placeholder") - with caplog.at_level(logging.ERROR, logger=CLI_LOGGER): - result = runner.invoke(app, ["keywords", str(non_existent_file)]) + with ( + patch("photools3.cli.process_photo") as process_photo, + patch("photools3.cli.process_batch") as process_batch, + ): + result = runner.invoke(app, ["metadata-magic", str(root)]) + + assert result.exit_code == 0 + process_photo.assert_called_once_with(top_level_photo, ProcessingMode.BOTH) + process_batch.assert_not_called() + + +def test_metadata_magic_command_returns_failure_if_any_batch_item_fails(tmp_path): + paths = [tmp_path / "one.jpg", tmp_path / "two.jpg"] + for path in paths: + path.write_bytes(b"test image") + + with patch("photools3.cli.process_batch") as process_batch: + process_batch.return_value = [ + _success_outcome(paths[0]), + SimpleNamespace( + path=paths[1], succeeded=False, error="RuntimeError: write failed" + ), + ] + result = runner.invoke(app, ["metadata-magic", *(str(path) for path in paths)]) assert result.exit_code == 1 - assert f"Error: File '{non_existent_file}' does not exist." in caplog.text -def test_keywords_command_error_during_processing(mock_photo_logic, dummy_file): - """Test the 'keywords' command when an exception occurs during processing.""" - # Configure the session mock to raise an exception - mock_photo_logic["session"].call.side_effect = Exception("LLM error") +def test_metadata_magic_command_returns_failure_for_single_photo_error(tmp_path): + image_path = tmp_path / "photo.jpg" + image_path.write_bytes(b"test image") - result = runner.invoke(app, ["keywords", str(dummy_file)]) + with patch("photools3.cli.process_photo", side_effect=RuntimeError("LLM error")): + result = runner.invoke(app, ["metadata-magic", str(image_path)]) + + assert result.exit_code == 1 + + +def test_keywords_alias_is_not_registered(): + result = runner.invoke(app, ["keywords", "photo.jpg"]) + + assert result.exit_code == 2 + assert "No such command 'keywords'" in result.output + + +def test_old_process_command_name_is_not_registered(): + result = runner.invoke(app, ["process", "photo.jpg"]) + + assert result.exit_code == 2 + assert "No such command 'process'" in result.output + + +def test_metadata_magic_command_rejects_empty_directory(tmp_path): + with ( + patch("photools3.cli.process_photo") as process_photo, + patch("photools3.cli.process_batch") as process_batch, + ): + result = runner.invoke(app, ["metadata-magic", str(tmp_path)]) + + assert result.exit_code == 1 + process_photo.assert_not_called() + process_batch.assert_not_called() + + +def test_quality_command_closes_photo_and_session(tmp_path): + image_path = tmp_path / "photo.jpg" + image_path.write_bytes(b"test image") + + with ( + patch("photools3.cli.LlmSessionQuality") as session_class, + patch("photools3.cli.Photo") as photo_class, + ): + result = runner.invoke(app, ["quality", str(image_path)]) + + assert result.exit_code == 0 + session_class.return_value.__enter__.return_value.call.assert_called_once_with( + [photo_class.return_value.__enter__.return_value] + ) + session_class.return_value.__exit__.assert_called_once() + photo_class.return_value.__exit__.assert_called_once() + + +def test_quality_command_reports_error_and_exits(tmp_path): + image_path = tmp_path / "photo.jpg" + + with patch("photools3.cli.Photo", side_effect=FileNotFoundError(image_path)): + result = runner.invoke(app, ["quality", str(image_path)]) - # Note: cli.py raises typer.Exit(code=1) before logging the - # "Error processing image: ..." message (the logger.error call is dead - # code after the raise), so only the exit code can be asserted here. assert result.exit_code == 1 def test_version_command(): - """Test the --version option.""" result = runner.invoke(app, ["--version"]) assert result.exit_code == 0 -- 2.54.0 From 0ad504e4c966be08ddc30c4951b1263e7ac0590c Mon Sep 17 00:00:00 2001 From: Mathias Scheider Date: Sun, 18 Oct 2026 12:01:46 +0200 Subject: [PATCH 3/7] Add configurable LM Studio flushing and LLM retries. --- .env.template | 3 + README.md | 12 ++- pyproject.toml | 2 +- src/photools3/cli.py | 3 +- src/photools3/llm_session.py | 78 ++++++++++++------ src/photools3/lm_studio.py | 135 ++++++++++++++++++++++++++++++++ src/photools3/processing.py | 21 ++++- src/photools3/settings.py | 6 +- test/test_llm_session_mocked.py | 69 ++++++++++++++++ test/test_lm_studio.py | 102 ++++++++++++++++++++++++ test/test_processing.py | 36 +++++++++ uv.lock | 8 +- 12 files changed, 444 insertions(+), 31 deletions(-) create mode 100644 src/photools3/lm_studio.py create mode 100644 test/test_lm_studio.py diff --git a/.env.template b/.env.template index 39cc3c7..43397fe 100644 --- a/.env.template +++ b/.env.template @@ -3,3 +3,6 @@ LLM_API_KEY = "not-relevant" LLM_DEFAULT_TEMPERATURE = 1.0 METADATA_PREFIX = "photools|" +LOG_LEVEL=INFO +LLM_FLUSH_EVERY_PHOTOS=50 +LLM_MAX_RETRIES=5 diff --git a/README.md b/README.md index 971625d..f8bd8b6 100644 --- a/README.md +++ b/README.md @@ -17,13 +17,23 @@ Create a `.env` file in the project root with the LLM connection details and the prefix used to identify metadata written by PhotoTools3: ```dotenv -llm_api_base_url=https://your-openai-compatible-api/v1 +llm_api_base_url=http://localhost:1234/v1 llm_api_key=your-api-key llm_default_temperature=0.2 metadata_prefix=photools3: +LOG_LEVEL=DEBUG +LLM_FLUSH_EVERY_PHOTOS=50 +LLM_MAX_RETRIES=5 ``` Keyword suggestions are restricted to the vocabulary in `config/keywords.txt`. +`LOG_LEVEL=DEBUG` enables detailed per-request and per-photo timings. The flush +interval uses LM Studio's native model-management API to unload and reload the +model after every 50 attempted batch photos. This requires LM Studio's native +`/api/v1` model-management API. Set `LLM_FLUSH_EVERY_PHOTOS=0` to disable it; +use `0` for non-LM-Studio OpenAI-compatible providers. +`LLM_MAX_RETRIES` controls how many corrective retry requests are allowed after +the initial LLM response; `5` means up to 6 requests for a photo. ## Generate metadata diff --git a/pyproject.toml b/pyproject.toml index 80ade47..34537d4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ dependencies = [ "pytest>=9.1.1", "ruff>=0.16.9", "typer>=0.12.0", - "utils>=0.1.2", + "utils>=0.1.3", ] [project.scripts] diff --git a/src/photools3/cli.py b/src/photools3/cli.py index 971b7c0..5adef79 100644 --- a/src/photools3/cli.py +++ b/src/photools3/cli.py @@ -13,9 +13,10 @@ from photools3.processing import ( process_batch, process_photo, ) +from photools3.settings import settings app = typer.Typer(help="PhotoTools3: A CLI for image processing.") -LoggingSettings() +LoggingSettings(LOG_LEVEL=settings.log_level) logger = logging.getLogger(__name__) diff --git a/src/photools3/llm_session.py b/src/photools3/llm_session.py index 14922af..68da969 100644 --- a/src/photools3/llm_session.py +++ b/src/photools3/llm_session.py @@ -1,11 +1,13 @@ import json import logging import re +import time import openai from openai import OpenAI from pydantic import BaseModel, ValidationError +from photools3.lm_studio import LMStudioModelManager from photools3.models import ( LlmMessageContent, LlmMessageRole, @@ -30,19 +32,15 @@ def _clean_llm_response(txt: str): return txt -def _correction_prompt( - original_request: LlmRequest, exception: Exception, origial_json -): +def _correction_prompt(exception: Exception, original_json: str) -> LlmRequestMessage: correction_prompt = ( f"The previous response was invalid due to: {exception}. " f"Please provide the corrected JSON based on this content: " - f"{origial_json}" + f"{original_json}" ) - original_request.messages.append( - LlmRequestMessage( - role=LlmMessageRole.user, - content=correction_prompt, - ) + return LlmRequestMessage( + role=LlmMessageRole.user, + content=correction_prompt, ) @@ -56,6 +54,7 @@ class LlmSession: self, system_prompt: str, temperature: float = settings.llm_default_temperature ): self._models: list = [] + self._model_id: str | None = None self.system_prompt = system_prompt self.temperature = temperature @@ -64,6 +63,7 @@ class LlmSession: base_url=self.base_url, api_key=self.api_key, ) + self._model_manager = LMStudioModelManager(self.base_url, self.api_key) def close(self): self.client.close() @@ -74,6 +74,12 @@ class LlmSession: def __exit__(self, exc_type, exc_value, traceback): self.close() + def flush_model(self): + """Unload and reload this session's model through LM Studio.""" + if self._model_id is None: + self._model_id = self.models[0].id + self._model_manager.reload_model(self._model_id) + def client_chat_completion_create(self, request: LlmRequest, choice_no: int = 0): response = self.client.chat.completions.create(**request.model_dump()) return response.choices[choice_no].message.content @@ -95,34 +101,62 @@ class LlmSession: self, photos: list[Photo], model_id=None, - max_retries: int = 2, + max_retries: int | None = None, prompt: str | None = None, ): - if not model_id: - model_id = self.models[0].id + retry_limit = settings.llm_max_retries if max_retries is None else max_retries + if retry_limit < 0: + raise ValueError("max_retries must be greater than or equal to zero") - request = self._build_request(prompt or self.prompt, photos, model_id) + if model_id is None: + if self._model_id is None: + self._model_id = self.models[0].id + model_id = self._model_id + else: + self._model_id = model_id - last_error = None - for attempt in range(max_retries + 1): + base_request = self._build_request(prompt or self.prompt, photos, model_id) + request = base_request + + for attempt in range(retry_limit + 1): + request_started = time.perf_counter() try: result_raw = self.client_chat_completion_create(request) + except Exception: + logger.debug( + "LLM request attempt %d/%d failed after %.2f seconds", + attempt + 1, + retry_limit + 1, + time.perf_counter() - request_started, + ) + raise + + logger.debug( + "LLM request attempt %d/%d completed in %.2f seconds", + attempt + 1, + retry_limit + 1, + time.perf_counter() - request_started, + ) + try: json_str = _clean_llm_response(result_raw) return self.response_model.model_validate_json(json_str) except (json.JSONDecodeError, ValidationError) as exc: - last_error = exc - if attempt < max_retries: - logger.info("Chatbot response is invalid; promting to correct ...") - _correction_prompt( - request, + if attempt < retry_limit: + logger.info("Chatbot response is invalid; prompting to correct ...") + correction = _correction_prompt( exc, - json_str if "json_str" in locals() else "origial json", + json_str if "json_str" in locals() else "unparseable response", + ) + # Start each retry from the initial prompt and only the latest + # correction; do not grow the conversation with all past retries. + request = base_request.model_copy( + update={"messages": [*base_request.messages, correction]} ) continue else: raise ValueError( - f"Failed to parse or validate LLM response after {max_retries} retries: {exc}" + f"Failed to parse or validate LLM response after {retry_limit} retries: {exc}" ) except Exception: raise diff --git a/src/photools3/lm_studio.py b/src/photools3/lm_studio.py new file mode 100644 index 0000000..cb5c1e0 --- /dev/null +++ b/src/photools3/lm_studio.py @@ -0,0 +1,135 @@ +import json +import logging +import time +from urllib.error import HTTPError, URLError +from urllib.parse import urlsplit, urlunsplit +from urllib.request import Request, urlopen + +logger = logging.getLogger(__name__) + +_LOAD_CONFIG_KEYS = ( + "context_length", + "eval_batch_size", + "flash_attention", + "num_experts", + "offload_kv_cache_to_gpu", +) + + +class LMStudioError(RuntimeError): + """Raised when LM Studio model management fails.""" + + +class LMStudioModelManager: + """Unload and reload a model through LM Studio's native REST API.""" + + def __init__(self, openai_base_url: str, api_key: str, timeout: float = 600): + parsed_url = urlsplit(openai_base_url) + path = parsed_url.path.rstrip("/") + path = path.removesuffix("/v1") + root_url = urlunsplit( + (parsed_url.scheme, parsed_url.netloc, path, "", "") + ).rstrip("/") + self.api_base_url = f"{root_url}/api/v1" + self.api_key = api_key + self.timeout = timeout + + def reload_model(self, model_id: str) -> None: + """Unload the matching loaded instance and reload it with its config.""" + started = time.perf_counter() + models_response = self._request("GET", "/models") + model, loaded_instance = self._find_model(models_response, model_id) + model_key = model["key"] + + if loaded_instance is not None: + self._request( + "POST", + "/models/unload", + {"instance_id": loaded_instance["id"]}, + ) + load_config = loaded_instance.get("config", {}) + else: + load_config = {} + + load_body = {"model": model_key} + load_body.update( + { + key: load_config[key] + for key in _LOAD_CONFIG_KEYS + if key in load_config and load_config[key] is not None + } + ) + self._request("POST", "/models/load", load_body) + logger.info( + "Reloaded LM Studio model %s in %.1f seconds", + model_key, + time.perf_counter() - started, + ) + + @staticmethod + def _find_model(models_response: dict, model_id: str) -> tuple[dict, dict | None]: + models = models_response.get("models") + if not isinstance(models, list): + raise LMStudioError("LM Studio model list response has no models array") + + for model in models: + loaded_instances = model.get("loaded_instances", []) + for instance in loaded_instances: + if instance.get("id") == model_id: + return model, instance + + matching_models = [model for model in models if model.get("key") == model_id] + if len(matching_models) != 1: + raise LMStudioError( + f"Could not uniquely identify LM Studio model {model_id!r}" + ) + + model = matching_models[0] + loaded_instances = model.get("loaded_instances", []) + if len(loaded_instances) > 1: + raise LMStudioError( + f"Multiple loaded instances found for LM Studio model {model_id!r}" + ) + loaded_instance = loaded_instances[0] if loaded_instances else None + return model, loaded_instance + + def _request(self, method: str, path: str, body: dict | None = None) -> dict: + data = None if body is None else json.dumps(body).encode("utf-8") + headers = {"Accept": "application/json"} + if data is not None: + headers["Content-Type"] = "application/json" + if self.api_key: + headers["Authorization"] = f"Bearer {self.api_key}" + + request = Request( + f"{self.api_base_url}{path}", + data=data, + headers=headers, + method=method, + ) + try: + with urlopen(request, timeout=self.timeout) as response: + response_body = response.read() + except HTTPError as exc: + detail = exc.read().decode("utf-8", errors="replace") + raise LMStudioError( + f"LM Studio {method} {path} returned HTTP {exc.code}: {detail}" + ) from exc + except (URLError, TimeoutError) as exc: + raise LMStudioError( + f"Could not reach LM Studio at {self.api_base_url}: {exc}" + ) from exc + + if not response_body: + return {} + try: + result = json.loads(response_body) + except json.JSONDecodeError as exc: + raise LMStudioError( + f"LM Studio {method} {path} returned invalid JSON" + ) from exc + if not isinstance(result, dict): + raise LMStudioError( + f"LM Studio {method} {path} returned an unexpected response" + ) + return result diff --git a/src/photools3/processing.py b/src/photools3/processing.py index 5e71ee1..747ae28 100644 --- a/src/photools3/processing.py +++ b/src/photools3/processing.py @@ -1,4 +1,5 @@ import logging +import time from collections.abc import Iterable from dataclasses import dataclass from enum import StrEnum @@ -17,6 +18,7 @@ from photools3.models import ( LlmResponseKeywords, ) from photools3.photo import Photo +from photools3.settings import settings logger = logging.getLogger(__name__) @@ -65,6 +67,8 @@ def process_photo( owns_session = session is None active_session = session if session is not None else _create_session(mode) + logger.info(f"Processing photo: {path!s}") + try: with Photo(path) as photo: result = active_session.call([photo]) @@ -104,10 +108,19 @@ def process_batch( mode = ProcessingMode(mode) session = _create_session(mode) outcomes: list[ProcessingOutcome] = [] + flush_every = settings.llm_flush_every_photos try: - for item in paths: + for attempted_photos, item in enumerate(paths): + if flush_every and attempted_photos and attempted_photos % flush_every == 0: + logger.info( + "Flushing LM Studio model after %d attempted photos", + attempted_photos, + ) + session.flush_model() + path = Path(item) + photo_started = time.perf_counter() try: result = process_photo(path, mode, session=session) except Exception as exc: @@ -122,6 +135,12 @@ def process_batch( ) else: outcomes.append(ProcessingOutcome(path=path, result=result)) + finally: + logger.debug( + "Photo processing for %s took %.2f seconds", + path, + time.perf_counter() - photo_started, + ) finally: session.close() diff --git a/src/photools3/settings.py b/src/photools3/settings.py index 2ceae41..7503a2b 100644 --- a/src/photools3/settings.py +++ b/src/photools3/settings.py @@ -1,5 +1,6 @@ from pathlib import Path +from pydantic import Field from pydantic_core import Url from pydantic_settings import BaseSettings, SettingsConfigDict @@ -13,11 +14,14 @@ KEYWORDS_LIST_PATH = CONFIG_DIR / "keywords.txt" class Settings(BaseSettings): - model_config = SettingsConfigDict(env_file=".env") + model_config = SettingsConfigDict(env_file=".env", extra="ignore") llm_api_base_url: Url llm_api_key: str llm_default_temperature: float + llm_flush_every_photos: int = Field(default=0, ge=0) + llm_max_retries: int = Field(default=5, ge=0) + log_level: str = "INFO" metadata_prefix: str diff --git a/test/test_llm_session_mocked.py b/test/test_llm_session_mocked.py index c6b9bb0..f7e1bc0 100644 --- a/test/test_llm_session_mocked.py +++ b/test/test_llm_session_mocked.py @@ -1,5 +1,7 @@ from unittest.mock import MagicMock, patch +import pytest + from photools3.llm_session import LlmSession @@ -84,3 +86,70 @@ def test_llm_session_captions_and_keywords_mocked(test_photo): # Verify that the mock was called assert mock_client.chat.completions.create.called + + +def test_llm_session_retries_from_original_request_without_accumulating_messages(): + with patch("photools3.llm_session.OpenAI") as mocked_openai: + mock_client = MagicMock() + mocked_openai.return_value = mock_client + mock_model = MagicMock() + mock_model.id = "gpt-4o" + mock_client.models.list.return_value.data = [mock_model] + + def response(content): + message = MagicMock() + message.content = content + choice = MagicMock() + choice.message = message + result = MagicMock() + result.choices = [choice] + return result + + mock_client.chat.completions.create.side_effect = [ + response('{"wrong": "first"}'), + response('{"wrong": "second"}'), + response('{"text": "correct"}'), + ] + + session = LlmSession(system_prompt="You are a helper.") + result = session.call(photos=[], prompt="Return JSON") + + assert result.text == "correct" + requests = mock_client.chat.completions.create.call_args_list + messages_per_attempt = [call.kwargs["messages"] for call in requests] + assert [len(messages) for messages in messages_per_attempt] == [2, 3, 3] + final_correction = messages_per_attempt[2][-1]["content"] + assert '"wrong": "second"' in final_correction + assert '"wrong": "first"' not in final_correction + + +def test_llm_session_uses_configured_retry_count(monkeypatch): + monkeypatch.setattr("photools3.llm_session.settings.llm_max_retries", 1) + + with patch("photools3.llm_session.OpenAI") as mocked_openai: + mock_client = MagicMock() + mocked_openai.return_value = mock_client + mock_model = MagicMock() + mock_model.id = "gpt-4o" + mock_client.models.list.return_value.data = [mock_model] + + def response(content): + message = MagicMock() + message.content = content + choice = MagicMock() + choice.message = message + result = MagicMock() + result.choices = [choice] + return result + + mock_client.chat.completions.create.side_effect = [ + response('{"wrong": "first"}'), + response('{"wrong": "second"}'), + response('{"text": "would be a third call"}'), + ] + session = LlmSession(system_prompt="You are a helper.") + + with pytest.raises(ValueError, match="after 1 retries"): + session.call(photos=[]) + + assert mock_client.chat.completions.create.call_count == 2 diff --git a/test/test_lm_studio.py b/test/test_lm_studio.py new file mode 100644 index 0000000..254244b --- /dev/null +++ b/test/test_lm_studio.py @@ -0,0 +1,102 @@ +import json +from unittest.mock import MagicMock, patch + +import pytest + +from photools3.lm_studio import LMStudioError, LMStudioModelManager + + +def _http_response(payload: dict): + response = MagicMock() + response.__enter__.return_value = response + response.read.return_value = json.dumps(payload).encode("utf-8") + return response + + +def test_reload_model_unloads_and_reloads_preserving_context_config(): + model_response = { + "models": [ + { + "key": "qwen/qwen3-vl-4b", + "loaded_instances": [ + { + "id": "qwen/qwen3-vl-4b-instance", + "config": { + "context_length": 16384, + "eval_batch_size": 256, + "flash_attention": True, + "offload_kv_cache_to_gpu": False, + "parallel": 2, + }, + } + ], + } + ] + } + with patch( + "photools3.lm_studio.urlopen", + side_effect=[ + _http_response(model_response), + _http_response({"instance_id": "qwen/qwen3-vl-4b-instance"}), + _http_response({"instance_id": "qwen/qwen3-vl-4b-instance"}), + ], + ) as mocked_urlopen: + manager = LMStudioModelManager( + "http://localhost:1234/v1", "test-token", timeout=12 + ) + manager.reload_model("qwen/qwen3-vl-4b") + + requests = [call.args[0] for call in mocked_urlopen.call_args_list] + assert requests[0].full_url == "http://localhost:1234/api/v1/models" + assert requests[0].get_method() == "GET" + assert requests[0].get_header("Authorization") == "Bearer test-token" + + assert requests[1].full_url.endswith("/models/unload") + assert json.loads(requests[1].data) == {"instance_id": "qwen/qwen3-vl-4b-instance"} + + assert requests[2].full_url.endswith("/models/load") + assert json.loads(requests[2].data) == { + "model": "qwen/qwen3-vl-4b", + "context_length": 16384, + "eval_batch_size": 256, + "flash_attention": True, + "offload_kv_cache_to_gpu": False, + } + + +def test_reload_model_loads_by_key_if_no_instance_is_loaded(): + model_response = {"models": [{"key": "qwen/model", "loaded_instances": []}]} + with patch( + "photools3.lm_studio.urlopen", + side_effect=[ + _http_response(model_response), + _http_response({"status": "loaded"}), + ], + ) as mocked_urlopen: + manager = LMStudioModelManager("http://localhost:1234/v1", "token") + manager.reload_model("qwen/model") + + assert mocked_urlopen.call_count == 2 + load_request = mocked_urlopen.call_args.args[0] + assert load_request.full_url.endswith("/models/load") + assert json.loads(load_request.data) == {"model": "qwen/model"} + + +def test_reload_model_rejects_ambiguous_model_selection(): + model_response = { + "models": [ + { + "key": "qwen/model", + "loaded_instances": [ + {"id": "instance-1", "config": {}}, + {"id": "instance-2", "config": {}}, + ], + } + ] + } + with patch( + "photools3.lm_studio.urlopen", return_value=_http_response(model_response) + ): + manager = LMStudioModelManager("http://localhost:1234/v1", "token") + with pytest.raises(LMStudioError, match="Multiple loaded instances"): + manager.reload_model("qwen/model") diff --git a/test/test_processing.py b/test/test_processing.py index c1a27d3..a0069da 100644 --- a/test/test_processing.py +++ b/test/test_processing.py @@ -127,3 +127,39 @@ def test_process_batch_is_sequential_reuses_session_and_continues_after_error(): assert all(len(call.args[0]) == 1 for call in session.call.call_args_list) assert writer_class.return_value.write.call_count == 2 session.close.assert_called_once() + + +def test_process_batch_flushes_after_configured_attempt_count(monkeypatch): + monkeypatch.setattr("photools3.processing.settings.llm_flush_every_photos", 2) + + class FakePhoto: + def __init__(self, path): + self.path = path + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + return None + + response = LlmResponseCaptionsAndKeywords(caption="Ein Haus", tags=["architektur"]) + session = MagicMock() + session.call.side_effect = [ + response, + RuntimeError("temporary error"), + response, + response, + response, + ] + paths = [Path(f"{i}.jpg") for i in range(5)] + + with ( + patch("photools3.processing._create_session", return_value=session), + patch("photools3.processing.Photo", FakePhoto), + patch("photools3.processing.MetadataWriter"), + ): + outcomes = process_batch(paths, ProcessingMode.BOTH) + + assert len(outcomes) == len(paths) + assert session.flush_model.call_count == 2 + session.close.assert_called_once() diff --git a/uv.lock b/uv.lock index b88df17..56a3bc3 100644 --- a/uv.lock +++ b/uv.lock @@ -780,7 +780,7 @@ requires-dist = [ { name = "pytest", specifier = ">=9.1.1" }, { name = "ruff", specifier = ">=0.16.9" }, { name = "typer", specifier = ">=0.12.0" }, - { name = "utils", specifier = ">=0.1.2", index = "https://gitea.schneiderserver.site/api/packages/mathias2784/pypi/simple" }, + { name = "utils", specifier = ">=0.1.3", index = "https://gitea.schneiderserver.site/api/packages/mathias2784/pypi/simple" }, ] [[package]] @@ -1205,7 +1205,7 @@ wheels = [ [[package]] name = "utils" -version = "0.1.2" +version = "0.1.3" source = { registry = "https://gitea.schneiderserver.site/api/packages/mathias2784/pypi/simple" } dependencies = [ { name = "black" }, @@ -1217,9 +1217,9 @@ dependencies = [ { name = "twine" }, { name = "uvicorn" }, ] -sdist = { url = "https://gitea.schneiderserver.site/api/packages/mathias2784/pypi/files/utils/0.1.2/utils-0.1.2.tar.gz", hash = "sha256:c711755c3e2fabd9cd380e25e45bc19da9b901868a6eb67028968803c725b118" } +sdist = { url = "https://gitea.schneiderserver.site/api/packages/mathias2784/pypi/files/utils/0.1.3/utils-0.1.3.tar.gz", hash = "sha256:8970a179573a63ab4c546cfd1da8ceca0d915bc4357b49ccffaba9b3d4a0195e" } wheels = [ - { url = "https://gitea.schneiderserver.site/api/packages/mathias2784/pypi/files/utils/0.1.2/utils-0.1.2-py3-none-any.whl", hash = "sha256:f3335ba10bd8eb9f5c000287a6ee41987b188372f3dca2783d99e6e70513a5d7" }, + { url = "https://gitea.schneiderserver.site/api/packages/mathias2784/pypi/files/utils/0.1.3/utils-0.1.3-py3-none-any.whl", hash = "sha256:c24078ba05f9f5b41fc3909598084b93c6df59f4ca60001e1e0b4fa55797e642" }, ] [[package]] -- 2.54.0 From 68bb2dc28c2b938516b5573fade686b0fc1591f7 Mon Sep 17 00:00:00 2001 From: Mathias Scheider Date: Sun, 18 Oct 2026 13:11:13 +0200 Subject: [PATCH 4/7] Cap generated keywords and simplify LLM correction retries. --- .env.template | 4 +- AGENTS.md | 15 +++ README.md | 21 +++-- src/photools3/llm_session.py | 146 ++++++++++++++++++++++++------ src/photools3/models.py | 56 +++++------- src/photools3/processing.py | 11 +-- src/photools3/settings.py | 4 +- test/test_llm_session_mocked.py | 156 +++++++++++++++++++++++++++++++- test/test_processing.py | 36 -------- 9 files changed, 329 insertions(+), 120 deletions(-) diff --git a/.env.template b/.env.template index 43397fe..4e2a77a 100644 --- a/.env.template +++ b/.env.template @@ -4,5 +4,7 @@ LLM_DEFAULT_TEMPERATURE = 1.0 METADATA_PREFIX = "photools|" LOG_LEVEL=INFO -LLM_FLUSH_EVERY_PHOTOS=50 +LLM_REQUEST_TIMEOUT_SECONDS=10 +LLM_MAX_COMPLETION_TOKENS=1024 LLM_MAX_RETRIES=5 +LLM_MAX_KEYWORDS=15 diff --git a/AGENTS.md b/AGENTS.md index eb30161..0c954a2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,27 +1,42 @@ # Agent Instructions: PhotoTools3 ## Environment Setup + Requires a `.env` file with the following variables: + - `llm_api_key`: API key for the LLM provider. - `llm_api_base_url`: Base URL for the LLM API. - `llm_default_temperature`: Float value for temperature. ## System Dependencies + - `exiftool` binary: required by `pyexiftool` at runtime, not installable via pip/uv. - Arch: `perl-image-exiftool` - Debian/Ubuntu: `libimage-exiftool-perl` ## Development Toolchain + - **Dependency Management**: Use `uv`. - **Python Version**: Requires Python `>=3.14`. - **Testing**: Run `pytest` to execute the test suite in `test/`. - **Linting & Formatting**: Use `ruff` and `black`. ## CLI Usage + The primary entry point is the `photools3` command: + - `photools3 keywords `: Extracts keywords and captions from a single image. ## Architecture & Logic + - **`src/photools3/photo.py`**: Core image processing logic, including resizing, RGB conversion, and generating base64 JPEG encodings specifically for LLM vision compatibility. - **`src/photools3/llm_session.py`**: Manages the workflow of sending processed images to the LLM and parsing results. - **Configuration**: The list of keywords to extract is maintained in `config/keywords.txt`. + +## Code style + +- Avoid overly verbose docstrings or comments. Be precise! + +## Comunication + +- When done with code modifications, give a one-liner for the currently unstaged changes that can be used as a commit message. diff --git a/README.md b/README.md index f8bd8b6..c1905f5 100644 --- a/README.md +++ b/README.md @@ -22,18 +22,23 @@ llm_api_key=your-api-key llm_default_temperature=0.2 metadata_prefix=photools3: LOG_LEVEL=DEBUG -LLM_FLUSH_EVERY_PHOTOS=50 +LLM_REQUEST_TIMEOUT_SECONDS=10 +LLM_MAX_COMPLETION_TOKENS=1024 LLM_MAX_RETRIES=5 +LLM_MAX_KEYWORDS=15 ``` Keyword suggestions are restricted to the vocabulary in `config/keywords.txt`. -`LOG_LEVEL=DEBUG` enables detailed per-request and per-photo timings. The flush -interval uses LM Studio's native model-management API to unload and reload the -model after every 50 attempted batch photos. This requires LM Studio's native -`/api/v1` model-management API. Set `LLM_FLUSH_EVERY_PHOTOS=0` to disable it; -use `0` for non-LM-Studio OpenAI-compatible providers. -`LLM_MAX_RETRIES` controls how many corrective retry requests are allowed after -the initial LLM response; `5` means up to 6 requests for a photo. +`LOG_LEVEL=DEBUG` enables detailed per-request and per-photo timings. +`LLM_MAX_RETRIES` controls retries for malformed or structurally invalid +responses; `5` means up to 6 requests for a photo. A valid response with +unsupported keywords gets at most one targeted correction retry, then remaining +invalid keywords are discarded. +Each request is limited by `LLM_REQUEST_TIMEOUT_SECONDS`; a timeout reloads the +LM Studio model before retrying. `LLM_MAX_COMPLETION_TOKENS` caps the generated +response length. When all retries are exhausted, the model is reloaded before +the failed photo is reported. `LLM_MAX_KEYWORDS` sets the maximum number of +distinct keywords requested and stored per photo; excess keywords are discarded. ## Generate metadata diff --git a/src/photools3/llm_session.py b/src/photools3/llm_session.py index 68da969..a2ba265 100644 --- a/src/photools3/llm_session.py +++ b/src/photools3/llm_session.py @@ -19,6 +19,7 @@ from photools3.models import ( LlmResponseGeneral, LlmResponseKeywords, LlmResponseQuality, + load_valid_keywords, ) from photools3.photo import Photo from photools3.settings import settings @@ -32,15 +33,23 @@ def _clean_llm_response(txt: str): return txt -def _correction_prompt(exception: Exception, original_json: str) -> LlmRequestMessage: - correction_prompt = ( - f"The previous response was invalid due to: {exception}. " - f"Please provide the corrected JSON based on this content: " - f"{original_json}" - ) +def _correction_prompt(instruction: str) -> LlmRequestMessage: return LlmRequestMessage( role=LlmMessageRole.user, - content=correction_prompt, + content=instruction, + ) + + +def _request_with_correction(base_request: LlmRequest, instruction: str) -> LlmRequest: + return base_request.model_copy( + update={"messages": [*base_request.messages, _correction_prompt(instruction)]} + ) + + +def _validation_summary(exception: ValidationError) -> str: + return "; ".join( + f"{'.'.join(map(str, error['loc']))}: {error['msg']}" + for error in exception.errors(include_input=False) ) @@ -62,6 +71,7 @@ class LlmSession: self.client = OpenAI( base_url=self.base_url, api_key=self.api_key, + max_retries=0, ) self._model_manager = LMStudioModelManager(self.base_url, self.api_key) @@ -81,7 +91,10 @@ class LlmSession: self._model_manager.reload_model(self._model_id) def client_chat_completion_create(self, request: LlmRequest, choice_no: int = 0): - response = self.client.chat.completions.create(**request.model_dump()) + response = self.client.chat.completions.create( + **request.model_dump(), + timeout=settings.llm_request_timeout_seconds, + ) return response.choices[choice_no].message.content def is_api_reachable(self) -> bool: @@ -117,11 +130,25 @@ class LlmSession: base_request = self._build_request(prompt or self.prompt, photos, model_id) request = base_request + tag_correction_used = False + valid_keywords = load_valid_keywords() + max_keywords = settings.llm_max_keywords for attempt in range(retry_limit + 1): request_started = time.perf_counter() try: result_raw = self.client_chat_completion_create(request) + except openai.APITimeoutError as exc: + logger.warning( + "LLM request timed out after %.2f seconds; reloading model", + time.perf_counter() - request_started, + ) + self.flush_model() + if attempt < retry_limit: + continue + raise ValueError( + f"LLM request timed out after {retry_limit + 1} attempts" + ) from exc except Exception: logger.debug( "LLM request attempt %d/%d failed after %.2f seconds", @@ -138,28 +165,88 @@ class LlmSession: time.perf_counter() - request_started, ) try: - json_str = _clean_llm_response(result_raw) - return self.response_model.model_validate_json(json_str) - - except (json.JSONDecodeError, ValidationError) as exc: + response_data = json.loads(_clean_llm_response(result_raw)) + except (json.JSONDecodeError, TypeError) as exc: if attempt < retry_limit: - logger.info("Chatbot response is invalid; prompting to correct ...") - correction = _correction_prompt( - exc, - json_str if "json_str" in locals() else "unparseable response", - ) - # Start each retry from the initial prompt and only the latest - # correction; do not grow the conversation with all past retries. - request = base_request.model_copy( - update={"messages": [*base_request.messages, correction]} + logger.info("Chatbot response was not valid JSON; retrying") + request = _request_with_correction( + base_request, + "Return one complete JSON object matching the schema. " + f"The previous response was invalid: {exc}", ) continue - else: - raise ValueError( - f"Failed to parse or validate LLM response after {retry_limit} retries: {exc}" + + logger.warning( + "LLM retry limit reached; reloading model before failing" + ) + self.flush_model() + raise ValueError( + f"Failed to parse LLM response after {retry_limit} retries: {exc}" + ) from exc + + if ( + isinstance(response_data, dict) + and "tags" in self.response_model.model_fields + and isinstance(response_data.get("tags"), list) + ): + tags = response_data["tags"] + considered_tags = tags[:max_keywords] + invalid_tags = [ + tag + for tag in considered_tags + if not isinstance(tag, str) or tag not in valid_keywords + ] + + if invalid_tags and not tag_correction_used and attempt < retry_limit: + tag_correction_used = True + logger.info( + "LLM returned unsupported keywords; requesting one correction" ) - except Exception: - raise + request = _request_with_correction( + base_request, + f"These keywords are not allowed: {invalid_tags!r}. " + "Replace them with exact allowed keywords when sensible; " + f"otherwise omit them. Return at most {max_keywords} " + "distinct keywords and do not list the vocabulary.", + ) + continue + + if len(tags) > max_keywords: + logger.info( + "Discarding keywords beyond configured limit of %d", + max_keywords, + ) + if invalid_tags: + logger.warning("Discarding unsupported keywords: %s", invalid_tags) + + response_data = dict(response_data) + valid_tags = [ + tag + for tag in tags + if isinstance(tag, str) and tag in valid_keywords + ] + response_data["tags"] = list(dict.fromkeys(valid_tags))[:max_keywords] + + try: + return self.response_model.model_validate(response_data) + except ValidationError as exc: + if attempt < retry_limit: + logger.info("LLM response did not match the schema; retrying") + request = _request_with_correction( + base_request, + "Return one complete JSON object matching the schema. " + f"Validation issues: {_validation_summary(exc)}", + ) + continue + + logger.warning( + "LLM retry limit reached; reloading model before failing" + ) + self.flush_model() + raise ValueError( + f"Failed to validate LLM response after {retry_limit} retries: " + f"{_validation_summary(exc)}" + ) from exc def _build_request( self, @@ -171,6 +258,7 @@ class LlmSession: return LlmRequest( model=model_id, temperature=self.temperature, + max_tokens=settings.llm_max_completion_tokens, messages=[ # System prompt LlmRequestMessage( @@ -202,13 +290,15 @@ class LlmSession: class LlmSessionCaptionsAndKeywords(LlmSession): response_model = LlmResponseCaptionsAndKeywords - prompt = "Give me a caption and keyword for the following photo(s)" + prompt = "Give me a caption and keywords for the following photo" def __init__(self, temperature: float = settings.llm_default_temperature): super().__init__( system_prompt=( "You are a helpful photographer's assistant that helps to write **captions** and **keywords** to photos:\n" "Answer in German.\n" + f"Choose at most {settings.llm_max_keywords} distinct, relevant keywords. " + "Do not repeat or enumerate the allowed-keyword list.\n" "Output must be a JSON of the following schema:\n" f"{self.response_model.model_json_schema()}" ), @@ -241,6 +331,8 @@ class LlmSessionKeywords(LlmSession): system_prompt=( "You are a helpful photographer's assistant that chooses keywords " "for photos. Answer in German.\n" + f"Choose at most {settings.llm_max_keywords} distinct, relevant keywords. " + "Do not repeat or enumerate the allowed-keyword list.\n" "Output must be a JSON of the following schema:\n" f"{self.response_model.model_json_schema()}" ), diff --git a/src/photools3/models.py b/src/photools3/models.py index d545c00..97197c7 100644 --- a/src/photools3/models.py +++ b/src/photools3/models.py @@ -1,3 +1,4 @@ +import json from enum import StrEnum from functools import lru_cache from typing import Annotated, Any, Literal @@ -10,7 +11,7 @@ from pydantic import ( model_validator, ) -from .settings import KEYWORDS_LIST_PATH +from .settings import KEYWORDS_LIST_PATH, settings @lru_cache @@ -42,6 +43,7 @@ class LlmRequest(BaseModel): model: str = "gpt-4o" messages: list[LlmRequestMessage] temperature: float | None = None + max_tokens: int class LlmMessageRole(StrEnum): @@ -98,17 +100,22 @@ class LlmResponseCaption(BaseModel): ] -class LlmResponseKeywords(BaseModel): - tags: list[str] = Field( - description=( - "A list of keywords that properly tag the photo. " - "Allowed keywords: " - + str(load_valid_keywords()) - .replace("'", '"') - .replace("{", "[") - .replace("}", "]") - ) +def _keyword_tags_field(): + allowed_keywords = json.dumps( + sorted(load_valid_keywords()), ensure_ascii=False, separators=(",", ":") ) + return Field( + max_length=settings.llm_max_keywords, + description=( + f"Choose at most {settings.llm_max_keywords} distinct, relevant keywords. " + "Do not enumerate the vocabulary. Allowed keywords: " + f"{allowed_keywords}" + ), + ) + + +class _LlmResponseWithTags(BaseModel): + tags: list[str] = _keyword_tags_field() @field_validator("tags") @classmethod @@ -122,7 +129,11 @@ class LlmResponseKeywords(BaseModel): return v -class LlmResponseCaptionsAndKeywords(BaseModel): +class LlmResponseKeywords(_LlmResponseWithTags): + pass + + +class LlmResponseCaptionsAndKeywords(_LlmResponseWithTags): caption: Annotated[ str, Field( @@ -132,27 +143,6 @@ class LlmResponseCaptionsAndKeywords(BaseModel): ) ), ] - tags: list[str] = Field( - description=( - "A list of keywords that properly tag the photo. " - "Allowed keywords: " - + str(load_valid_keywords()) - .replace("'", '"') - .replace("{", "[") - .replace("}", "]") - ) - ) - - @field_validator("tags") - @classmethod - def validate_tags(cls, v: list[str]) -> list[str]: - valid_keywords = load_valid_keywords() - for tag in v: - if tag not in valid_keywords: - raise ValueError( - f"'{tag}' is not a valid keyword. Must be one from the configuration." - ) - return v class LlmResponseQuality(BaseModel): diff --git a/src/photools3/processing.py b/src/photools3/processing.py index 747ae28..fe7d14c 100644 --- a/src/photools3/processing.py +++ b/src/photools3/processing.py @@ -18,7 +18,6 @@ from photools3.models import ( LlmResponseKeywords, ) from photools3.photo import Photo -from photools3.settings import settings logger = logging.getLogger(__name__) @@ -108,17 +107,9 @@ def process_batch( mode = ProcessingMode(mode) session = _create_session(mode) outcomes: list[ProcessingOutcome] = [] - flush_every = settings.llm_flush_every_photos try: - for attempted_photos, item in enumerate(paths): - if flush_every and attempted_photos and attempted_photos % flush_every == 0: - logger.info( - "Flushing LM Studio model after %d attempted photos", - attempted_photos, - ) - session.flush_model() - + for item in paths: path = Path(item) photo_started = time.perf_counter() try: diff --git a/src/photools3/settings.py b/src/photools3/settings.py index 7503a2b..8f7092a 100644 --- a/src/photools3/settings.py +++ b/src/photools3/settings.py @@ -19,8 +19,10 @@ class Settings(BaseSettings): llm_api_base_url: Url llm_api_key: str llm_default_temperature: float - llm_flush_every_photos: int = Field(default=0, ge=0) + llm_request_timeout_seconds: float = Field(default=10, gt=0) + llm_max_completion_tokens: int = Field(default=1024, gt=0) llm_max_retries: int = Field(default=5, ge=0) + llm_max_keywords: int = Field(default=15, ge=1) log_level: str = "INFO" metadata_prefix: str diff --git a/test/test_llm_session_mocked.py b/test/test_llm_session_mocked.py index f7e1bc0..31160ee 100644 --- a/test/test_llm_session_mocked.py +++ b/test/test_llm_session_mocked.py @@ -1,8 +1,10 @@ from unittest.mock import MagicMock, patch +import openai import pytest -from photools3.llm_session import LlmSession +from photools3.llm_session import LlmSession, LlmSessionCaptionsAndKeywords +from photools3.models import LlmResponseCaptionsAndKeywords def test_llm_session_mocked(test_photo): @@ -119,8 +121,8 @@ def test_llm_session_retries_from_original_request_without_accumulating_messages messages_per_attempt = [call.kwargs["messages"] for call in requests] assert [len(messages) for messages in messages_per_attempt] == [2, 3, 3] final_correction = messages_per_attempt[2][-1]["content"] - assert '"wrong": "second"' in final_correction - assert '"wrong": "first"' not in final_correction + assert "Field required" in final_correction + assert '"wrong"' not in final_correction def test_llm_session_uses_configured_retry_count(monkeypatch): @@ -149,7 +151,153 @@ def test_llm_session_uses_configured_retry_count(monkeypatch): ] session = LlmSession(system_prompt="You are a helper.") - with pytest.raises(ValueError, match="after 1 retries"): + with ( + patch.object(session, "flush_model") as flush_model, + pytest.raises(ValueError, match="after 1 retries"), + ): session.call(photos=[]) assert mock_client.chat.completions.create.call_count == 2 + flush_model.assert_called_once() + + +def test_llm_session_reloads_after_timeout_and_retries(monkeypatch): + monkeypatch.setattr( + "photools3.llm_session.settings.llm_request_timeout_seconds", 10 + ) + monkeypatch.setattr("photools3.llm_session.settings.llm_max_completion_tokens", 768) + monkeypatch.setattr("photools3.llm_session.settings.llm_max_retries", 1) + + with patch("photools3.llm_session.OpenAI") as mocked_openai: + mock_client = MagicMock() + mocked_openai.return_value = mock_client + mock_model = MagicMock() + mock_model.id = "gpt-4o" + mock_client.models.list.return_value.data = [mock_model] + + response_message = MagicMock(content='{"text": "recovered"}') + response_choice = MagicMock(message=response_message) + success_response = MagicMock(choices=[response_choice]) + mock_client.chat.completions.create.side_effect = [ + openai.APITimeoutError(request=MagicMock()), + success_response, + ] + + session = LlmSession(system_prompt="You are a helper.") + with patch.object(session, "flush_model") as flush_model: + result = session.call(photos=[]) + + assert result.text == "recovered" + flush_model.assert_called_once() + calls = mock_client.chat.completions.create.call_args_list + assert len(calls) == 2 + assert all(call.kwargs["timeout"] == 10 for call in calls) + assert all(call.kwargs["max_tokens"] == 768 for call in calls) + mocked_openai.assert_called_once_with( + base_url=session.base_url, + api_key=session.api_key, + max_retries=0, + ) + + +@pytest.mark.parametrize("failure_kind", ["invalid_response", "timeout"]) +def test_llm_session_reloads_when_retry_limit_is_exhausted(monkeypatch, failure_kind): + monkeypatch.setattr("photools3.llm_session.settings.llm_max_retries", 0) + + with patch("photools3.llm_session.OpenAI") as mocked_openai: + mock_client = MagicMock() + mocked_openai.return_value = mock_client + mock_model = MagicMock() + mock_model.id = "gpt-4o" + mock_client.models.list.return_value.data = [mock_model] + + if failure_kind == "timeout": + mock_client.chat.completions.create.side_effect = openai.APITimeoutError( + request=MagicMock() + ) + else: + response_message = MagicMock(content='{"wrong": "shape"}') + response_choice = MagicMock(message=response_message) + mock_client.chat.completions.create.return_value = MagicMock( + choices=[response_choice] + ) + + session = LlmSession(system_prompt="You are a helper.") + with ( + patch.object(session, "flush_model") as flush_model, + pytest.raises(ValueError), + ): + session.call(photos=[]) + + flush_model.assert_called_once() + assert mock_client.chat.completions.create.call_count == 1 + + +def test_invalid_keywords_get_one_correction_then_invalid_tags_are_dropped(monkeypatch): + monkeypatch.setattr("photools3.llm_session.settings.llm_max_keywords", 3) + monkeypatch.setattr("photools3.llm_session.settings.llm_max_retries", 5) + + with patch("photools3.llm_session.OpenAI") as mocked_openai: + mock_client = MagicMock() + mocked_openai.return_value = mock_client + mock_model = MagicMock() + mock_model.id = "gpt-4o" + mock_client.models.list.return_value.data = [mock_model] + + def response(content): + message = MagicMock(content=content) + choice = MagicMock(message=message) + return MagicMock(choices=[choice]) + + mock_client.chat.completions.create.side_effect = [ + response( + '{"caption": "A caption", "tags": ["not_allowed", "architektur"]}' + ), + response( + '{"caption": "A caption", ' + '"tags": ["still_invalid", "architektur", "personen"]}' + ), + ] + session = LlmSessionCaptionsAndKeywords() + with patch.object(session, "flush_model") as flush_model: + result = session.call(photos=[]) + + assert result.tags == ["architektur", "personen"] + assert mock_client.chat.completions.create.call_count == 2 + flush_model.assert_not_called() + correction = mock_client.chat.completions.create.call_args_list[1].kwargs[ + "messages" + ][-1]["content"] + assert "not_allowed" in correction + assert '"caption"' not in correction + + +def test_excess_keywords_are_truncated_to_configured_limit(monkeypatch): + monkeypatch.setattr("photools3.llm_session.settings.llm_max_keywords", 2) + + with patch("photools3.llm_session.OpenAI") as mocked_openai: + mock_client = MagicMock() + mocked_openai.return_value = mock_client + mock_model = MagicMock() + mock_model.id = "gpt-4o" + mock_client.models.list.return_value.data = [mock_model] + message = MagicMock( + content=( + '{"caption": "A caption", ' + '"tags": ["architektur", "personen", "landschaft"]}' + ) + ) + mock_client.chat.completions.create.return_value = MagicMock( + choices=[MagicMock(message=message)] + ) + + result = LlmSessionCaptionsAndKeywords().call(photos=[]) + + assert result.tags == ["architektur", "personen"] + assert mock_client.chat.completions.create.call_count == 1 + + +def test_keyword_response_schema_includes_configured_max_items(): + schema = LlmResponseCaptionsAndKeywords.model_json_schema() + + assert schema["properties"]["tags"]["maxItems"] == 15 diff --git a/test/test_processing.py b/test/test_processing.py index a0069da..c1a27d3 100644 --- a/test/test_processing.py +++ b/test/test_processing.py @@ -127,39 +127,3 @@ def test_process_batch_is_sequential_reuses_session_and_continues_after_error(): assert all(len(call.args[0]) == 1 for call in session.call.call_args_list) assert writer_class.return_value.write.call_count == 2 session.close.assert_called_once() - - -def test_process_batch_flushes_after_configured_attempt_count(monkeypatch): - monkeypatch.setattr("photools3.processing.settings.llm_flush_every_photos", 2) - - class FakePhoto: - def __init__(self, path): - self.path = path - - def __enter__(self): - return self - - def __exit__(self, exc_type, exc_value, traceback): - return None - - response = LlmResponseCaptionsAndKeywords(caption="Ein Haus", tags=["architektur"]) - session = MagicMock() - session.call.side_effect = [ - response, - RuntimeError("temporary error"), - response, - response, - response, - ] - paths = [Path(f"{i}.jpg") for i in range(5)] - - with ( - patch("photools3.processing._create_session", return_value=session), - patch("photools3.processing.Photo", FakePhoto), - patch("photools3.processing.MetadataWriter"), - ): - outcomes = process_batch(paths, ProcessingMode.BOTH) - - assert len(outcomes) == len(paths) - assert session.flush_model.call_count == 2 - session.close.assert_called_once() -- 2.54.0 From a478713f5cb34e0f66c74e1325a8b430cbb732b5 Mon Sep 17 00:00:00 2001 From: Mathias Scheider Date: Sun, 18 Oct 2026 13:47:22 +0200 Subject: [PATCH 5/7] exclude non-app loggers --- src/photools3/cli.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/photools3/cli.py b/src/photools3/cli.py index 5adef79..d8f725d 100644 --- a/src/photools3/cli.py +++ b/src/photools3/cli.py @@ -17,6 +17,8 @@ from photools3.settings import settings app = typer.Typer(help="PhotoTools3: A CLI for image processing.") LoggingSettings(LOG_LEVEL=settings.log_level) +for name in ("uvicorn", "uvicorn.error", "uvicorn.access", "PIL", "openai", "httpx2", "httpcore2"): + logging.getLogger(name).setLevel(logging.WARNING) logger = logging.getLogger(__name__) -- 2.54.0 From 05007d0720320154e91b26ff6e3b128567d39268 Mon Sep 17 00:00:00 2001 From: Mathias Scheider Date: Sun, 18 Oct 2026 13:48:35 +0200 Subject: [PATCH 6/7] black --- src/photools3/cli.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/photools3/cli.py b/src/photools3/cli.py index d8f725d..1d7a94f 100644 --- a/src/photools3/cli.py +++ b/src/photools3/cli.py @@ -17,7 +17,15 @@ from photools3.settings import settings app = typer.Typer(help="PhotoTools3: A CLI for image processing.") LoggingSettings(LOG_LEVEL=settings.log_level) -for name in ("uvicorn", "uvicorn.error", "uvicorn.access", "PIL", "openai", "httpx2", "httpcore2"): +for name in ( + "uvicorn", + "uvicorn.error", + "uvicorn.access", + "PIL", + "openai", + "httpx2", + "httpcore2", +): logging.getLogger(name).setLevel(logging.WARNING) logger = logging.getLogger(__name__) -- 2.54.0 From 426a3f69973678e67f3dc0e2c2d565e64bfd0230 Mon Sep 17 00:00:00 2001 From: Mathias Scheider Date: Sun, 18 Oct 2026 13:49:52 +0200 Subject: [PATCH 7/7] removed one excess black in the pipeline definition --- .gitea/workflows/ci.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index eb71684..4242e21 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -35,5 +35,3 @@ jobs: run: uv run mypy . - name: Check formatting (black) run: uv run black --check . --extend-exclude .venv - - name: Check formatting (black) - run: uv run black --check . --extend-exclude .venv -- 2.54.0