diff --git a/.specify/feature.json b/.specify/feature.json index 6fdb4b6..f2cfdf7 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1,3 +1,3 @@ { - "feature_directory": "specs/002-api-image-proxy" + "feature_directory": "specs/003-upload-thumbnails" } diff --git a/CLAUDE.md b/CLAUDE.md index 37de3a5..f159ce4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,5 +1,5 @@ For additional context about technologies to be used, project structure, shell commands, and other important information, read the current plan at -`specs/002-api-image-proxy/plan.md`. +`specs/003-upload-thumbnails/plan.md`. diff --git a/api/alembic/versions/002_add_thumbnail_key.py b/api/alembic/versions/002_add_thumbnail_key.py new file mode 100644 index 0000000..5836164 --- /dev/null +++ b/api/alembic/versions/002_add_thumbnail_key.py @@ -0,0 +1,23 @@ +"""add thumbnail_key column to images + +Revision ID: 002 +Revises: 001 +Create Date: 2026-05-03 + +""" +from typing import Sequence, Union +import sqlalchemy as sa +from alembic import op + +revision: str = "002" +down_revision: Union[str, None] = "001" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column("images", sa.Column("thumbnail_key", sa.String(70), nullable=True)) + + +def downgrade() -> None: + op.drop_column("images", "thumbnail_key") diff --git a/api/app/models.py b/api/app/models.py index ab6af5a..f7357b5 100644 --- a/api/app/models.py +++ b/api/app/models.py @@ -23,6 +23,7 @@ class Image(Base): width: Mapped[int] = mapped_column(Integer, nullable=False) height: Mapped[int] = mapped_column(Integer, nullable=False) storage_key: Mapped[str] = mapped_column(String(64), nullable=False) + thumbnail_key: Mapped[str | None] = mapped_column(String(70), nullable=True, default=None) created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=_utcnow, nullable=False) image_tags: Mapped[list["ImageTag"]] = relationship(back_populates="image", cascade="all, delete-orphan") diff --git a/api/app/repositories/image_repo.py b/api/app/repositories/image_repo.py index 3ded93b..768858c 100644 --- a/api/app/repositories/image_repo.py +++ b/api/app/repositories/image_repo.py @@ -34,6 +34,7 @@ class ImageRepository: width: int, height: int, storage_key: str, + thumbnail_key: str | None = None, ) -> Image: image = Image( hash=hash_hex, @@ -43,6 +44,7 @@ class ImageRepository: width=width, height=height, storage_key=storage_key, + thumbnail_key=thumbnail_key, ) self._session.add(image) await self._session.flush() diff --git a/api/app/routers/images.py b/api/app/routers/images.py index 6b338db..5ef59fe 100644 --- a/api/app/routers/images.py +++ b/api/app/routers/images.py @@ -1,3 +1,5 @@ +import asyncio +import logging import struct import uuid from typing import Any @@ -12,9 +14,12 @@ from app.models import Image from app.repositories.image_repo import ImageRepository from app.repositories.tag_repo import TagRepository from app.storage.backend import StorageBackend +from app.thumbnail import generate_thumbnail from app.utils import compute_sha256 from app.validation import FileSizeError, MimeTypeError, validate_file_size, validate_mime_type +logger = logging.getLogger(__name__) + router = APIRouter(tags=["images"]) @@ -32,6 +37,7 @@ def _image_to_dict(image: Image, *, duplicate: bool | None = None) -> dict[str, "width": image.width, "height": image.height, "storage_key": image.storage_key, + "thumbnail_key": image.thumbnail_key, "created_at": image.created_at.isoformat(), "tags": image.tags, } @@ -151,6 +157,14 @@ async def upload_image( width, height = _read_image_dimensions(data, mime_type) await storage.put(hash_hex, data, mime_type) + thumbnail_key: str | None = None + try: + thumb_bytes = await asyncio.to_thread(generate_thumbnail, data, mime_type) + await storage.put(f"{hash_hex}-thumb", thumb_bytes, "image/webp") + thumbnail_key = f"{hash_hex}-thumb" + except Exception: + logger.warning("Thumbnail generation failed for %s; upload will proceed without thumbnail", hash_hex) + image = await image_repo.create( hash_hex=hash_hex, filename=file.filename or "upload", @@ -159,6 +173,7 @@ async def upload_image( width=width, height=height, storage_key=hash_hex, + thumbnail_key=thumbnail_key, ) if tag_names: @@ -233,6 +248,38 @@ async def serve_image_file( ) +@router.get("/images/{image_id}/thumbnail") +async def serve_image_thumbnail( + image_id: uuid.UUID, + db: AsyncSession = Depends(get_db), + storage: StorageBackend = Depends(get_storage), +): + image_repo = ImageRepository(db) + image = await image_repo.get_by_id(image_id) + if not image: + raise HTTPException( + status_code=404, + detail={"detail": "Image not found", "code": "image_not_found"}, + ) + key = image.thumbnail_key or image.storage_key + media_type = "image/webp" if image.thumbnail_key else image.mime_type + try: + data = await storage.get(key) + except Exception: + raise HTTPException( + status_code=500, + detail={"detail": "Failed to retrieve image content", "code": "storage_error"}, + ) from None + return Response( + content=data, + media_type=media_type, + headers={ + "ETag": f'"{image.hash}"', + "Cache-Control": "public, max-age=31536000, immutable", + }, + ) + + @router.patch("/images/{image_id}/tags") async def update_image_tags( image_id: uuid.UUID, @@ -276,6 +323,9 @@ async def delete_image( detail={"detail": "Image not found", "code": "image_not_found"}, ) storage_key = image.storage_key + thumbnail_key = image.thumbnail_key await image_repo.delete(image) await storage.delete(storage_key) + if thumbnail_key: + await storage.delete(thumbnail_key) return Response(status_code=204) diff --git a/api/app/thumbnail.py b/api/app/thumbnail.py new file mode 100644 index 0000000..71292c5 --- /dev/null +++ b/api/app/thumbnail.py @@ -0,0 +1,16 @@ +import contextlib +import io + +from PIL import Image + + +def generate_thumbnail(data: bytes, mime_type: str) -> bytes: + img = Image.open(io.BytesIO(data)) + with contextlib.suppress(EOFError): + img.seek(0) + if img.mode not in ("RGB", "RGBA"): + img = img.convert("RGBA" if img.mode == "P" and "transparency" in img.info else "RGB") + img.thumbnail((400, 400), Image.LANCZOS) + buf = io.BytesIO() + img.save(buf, format="WEBP", quality=80) + return buf.getvalue() diff --git a/api/pyproject.toml b/api/pyproject.toml index 506d831..0f432c0 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -15,6 +15,7 @@ dependencies = [ "aiobotocore>=2.13", "pydantic-settings>=2.2", "python-multipart>=0.0.9", + "pillow>=10.0", ] [project.optional-dependencies] diff --git a/api/tests/integration/test_delete.py b/api/tests/integration/test_delete.py index 1e7562a..ca5f968 100644 --- a/api/tests/integration/test_delete.py +++ b/api/tests/integration/test_delete.py @@ -5,7 +5,9 @@ T067 — DELETE of unknown ID → 404 image_not_found """ import io import uuid + import pytest +from PIL import Image as PILImage def _minimal_jpeg_v2() -> bytes: @@ -58,3 +60,25 @@ async def test_delete_unknown_id_returns_404(client): assert response.status_code == 404 body = response.json() assert body["code"] == "image_not_found" + + +@pytest.mark.asyncio +async def test_delete_removes_thumbnail(client): + buf = io.BytesIO() + PILImage.new("RGB", (200, 150), color=(60, 90, 120)).save(buf, format="JPEG") + data = buf.getvalue() + + upload = await client.post( + "/api/v1/images", + files={"file": ("thumb-del.jpg", io.BytesIO(data), "image/jpeg")}, + ) + assert upload.status_code == 201 + image_id = upload.json()["id"] + assert upload.json()["thumbnail_key"] is not None + + delete_resp = await client.delete(f"/api/v1/images/{image_id}") + assert delete_resp.status_code == 204 + + thumb_resp = await client.get(f"/api/v1/images/{image_id}/thumbnail") + assert thumb_resp.status_code == 404 + assert thumb_resp.json()["code"] == "image_not_found" diff --git a/api/tests/integration/test_serving.py b/api/tests/integration/test_serving.py index 14e2725..cb5ecbe 100644 --- a/api/tests/integration/test_serving.py +++ b/api/tests/integration/test_serving.py @@ -7,6 +7,16 @@ import io import uuid import pytest +from PIL import Image as PILImage +from sqlalchemy import update + +from app.models import Image + + +def _real_jpeg() -> bytes: + buf = io.BytesIO() + PILImage.new("RGB", (200, 150), color=(120, 80, 200)).save(buf, format="JPEG") + return buf.getvalue() def _minimal_webp() -> bytes: @@ -62,3 +72,53 @@ async def test_file_response_exposes_no_storage_details(client): assert "minio" not in response.text.lower() assert "s3://" not in response.text.lower() assert "amazonaws.com" not in response.text.lower() + + +@pytest.mark.asyncio +async def test_thumbnail_returns_webp(client): + data = _real_jpeg() + upload = await client.post( + "/api/v1/images", + files={"file": ("t.jpg", io.BytesIO(data), "image/jpeg")}, + ) + assert upload.status_code == 201 + body = upload.json() + image_id = body["id"] + image_hash = body["hash"] + + response = await client.get(f"/api/v1/images/{image_id}/thumbnail") + assert response.status_code == 200 + assert response.headers["content-type"] == "image/webp" + assert response.headers["etag"] == f'"{image_hash}"' + assert "immutable" in response.headers["cache-control"] + assert len(response.content) > 0 + + +@pytest.mark.asyncio +async def test_thumbnail_fallback_returns_original(client, db_session): + data = _real_jpeg() + upload = await client.post( + "/api/v1/images", + files={"file": ("fallback.jpg", io.BytesIO(data), "image/jpeg")}, + ) + assert upload.status_code == 201 + image_id = upload.json()["id"] + + await db_session.execute( + update(Image).where(Image.id == uuid.UUID(image_id)).values(thumbnail_key=None) + ) + await db_session.flush() + db_session.expire_all() + + response = await client.get(f"/api/v1/images/{image_id}/thumbnail") + assert response.status_code == 200 + assert "image/jpeg" in response.headers["content-type"] + assert len(response.content) > 0 + + +@pytest.mark.asyncio +async def test_thumbnail_unknown_id_returns_404(client): + response = await client.get(f"/api/v1/images/{uuid.uuid4()}/thumbnail") + assert response.status_code == 404 + body = response.json() + assert body["code"] == "image_not_found" diff --git a/api/tests/integration/test_upload.py b/api/tests/integration/test_upload.py index 8125612..3b8fe30 100644 --- a/api/tests/integration/test_upload.py +++ b/api/tests/integration/test_upload.py @@ -6,7 +6,16 @@ T029 — file > MAX_UPLOAD_BYTES → 422 file_too_large T079 — GET /api/v1/images/{id} 404 → error envelope shape """ import io +from unittest.mock import patch + import pytest +from PIL import Image as PILImage + + +def _real_jpeg(color: tuple = (100, 150, 200), size: tuple = (200, 150)) -> bytes: + buf = io.BytesIO() + PILImage.new("RGB", size, color=color).save(buf, format="JPEG") + return buf.getvalue() def _minimal_jpeg() -> bytes: @@ -96,3 +105,51 @@ async def test_get_unknown_image_returns_404_with_envelope(client): body = response.json() assert body["code"] == "image_not_found" assert "detail" in body + + +@pytest.mark.asyncio +async def test_upload_returns_thumbnail_key(client): + data = _real_jpeg(color=(100, 150, 200)) + response = await client.post( + "/api/v1/images", + files={"file": ("thumb_test.jpg", io.BytesIO(data), "image/jpeg")}, + ) + assert response.status_code == 201 + body = response.json() + assert "thumbnail_key" in body + assert body["thumbnail_key"] is not None + assert body["thumbnail_key"].endswith("-thumb") + + +@pytest.mark.asyncio +async def test_duplicate_upload_reuses_thumbnail_key(client): + data = _real_jpeg(color=(200, 100, 50)) + r1 = await client.post( + "/api/v1/images", + files={"file": ("dup.jpg", io.BytesIO(data), "image/jpeg")}, + ) + assert r1.status_code in (200, 201) + + r2 = await client.post( + "/api/v1/images", + files={"file": ("dup.jpg", io.BytesIO(data), "image/jpeg")}, + ) + assert r2.status_code == 200 + + tk1 = r1.json()["thumbnail_key"] + tk2 = r2.json()["thumbnail_key"] + assert tk1 is not None + assert tk1 == tk2 + + +@pytest.mark.asyncio +async def test_upload_succeeds_when_thumbnail_fails(client): + data = _real_jpeg(color=(50, 200, 150)) + with patch("app.routers.images.generate_thumbnail", side_effect=RuntimeError("simulated")): + response = await client.post( + "/api/v1/images", + files={"file": ("no_thumb.jpg", io.BytesIO(data), "image/jpeg")}, + ) + assert response.status_code in (200, 201) + body = response.json() + assert body["thumbnail_key"] is None diff --git a/api/tests/unit/test_thumbnail.py b/api/tests/unit/test_thumbnail.py new file mode 100644 index 0000000..b1435e3 --- /dev/null +++ b/api/tests/unit/test_thumbnail.py @@ -0,0 +1,79 @@ +"""Unit tests for thumbnail generation utility.""" +import io + +from PIL import Image as PILImage + +from app.thumbnail import generate_thumbnail + + +def _make_jpeg(width: int, height: int) -> bytes: + buf = io.BytesIO() + img = PILImage.new("RGB", (width, height), color=(128, 64, 32)) + img.save(buf, format="JPEG", quality=80) + return buf.getvalue() + + +def _make_png_rgba(width: int, height: int) -> bytes: + buf = io.BytesIO() + img = PILImage.new("RGBA", (width, height), color=(10, 20, 30, 180)) + img.save(buf, format="PNG") + return buf.getvalue() + + +def _make_gif(width: int, height: int) -> bytes: + buf = io.BytesIO() + img = PILImage.new("P", (width, height)) + img.save(buf, format="GIF") + return buf.getvalue() + + +def test_thumbnail_is_webp(): + data = _make_jpeg(600, 400) + result = generate_thumbnail(data, "image/jpeg") + assert result[:4] == b"RIFF" + assert result[8:12] == b"WEBP" + + +def test_thumbnail_fits_within_400px(): + data = _make_jpeg(800, 600) + result = generate_thumbnail(data, "image/jpeg") + img = PILImage.open(io.BytesIO(result)) + w, h = img.size + assert w <= 400 + assert h <= 400 + + +def test_thumbnail_preserves_aspect_ratio(): + original_w, original_h = 800, 300 + data = _make_jpeg(original_w, original_h) + result = generate_thumbnail(data, "image/jpeg") + img = PILImage.open(io.BytesIO(result)) + w, h = img.size + original_ratio = original_w / original_h + thumb_ratio = w / h + assert abs(original_ratio - thumb_ratio) / original_ratio < 0.01 + + +def test_thumbnail_handles_gif_first_frame(): + data = _make_gif(500, 500) + result = generate_thumbnail(data, "image/gif") + assert result[8:12] == b"WEBP" + img = PILImage.open(io.BytesIO(result)) + assert not getattr(img, "is_animated", False) + + +def test_thumbnail_handles_png_with_alpha(): + data = _make_png_rgba(300, 300) + result = generate_thumbnail(data, "image/png") + assert result[8:12] == b"WEBP" + img = PILImage.open(io.BytesIO(result)) + assert img.format == "WEBP" + + +def test_thumbnail_does_not_upscale(): + data = _make_jpeg(100, 100) + result = generate_thumbnail(data, "image/jpeg") + img = PILImage.open(io.BytesIO(result)) + w, h = img.size + assert w <= 100 + assert h <= 100 diff --git a/specs/003-upload-thumbnails/checklists/requirements.md b/specs/003-upload-thumbnails/checklists/requirements.md new file mode 100644 index 0000000..86fdc87 --- /dev/null +++ b/specs/003-upload-thumbnails/checklists/requirements.md @@ -0,0 +1,34 @@ +# Specification Quality Checklist: Upload Thumbnails + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-05-03 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +All checklist items pass. Spec is ready for `/speckit-plan`. diff --git a/specs/003-upload-thumbnails/contracts/api.md b/specs/003-upload-thumbnails/contracts/api.md new file mode 100644 index 0000000..d07ce10 --- /dev/null +++ b/specs/003-upload-thumbnails/contracts/api.md @@ -0,0 +1,90 @@ +# API Contract: Upload Thumbnails + +**Branch**: `003-upload-thumbnails` | **Date**: 2026-05-03 + +--- + +## New endpoint + +### `GET /api/v1/images/{image_id}/thumbnail` + +Returns the thumbnail content for the given image. If no thumbnail was generated +(image pre-dates the feature or generation failed), falls back to the full-size +original. + +**Path parameters** + +| Parameter | Type | Description | +|-----------|------|-------------| +| `image_id` | UUID | Unique identifier of the image | + +**Responses** + +#### `200 OK` — Thumbnail (or original fallback) content + +| Header | Value | Notes | +|--------|-------|-------| +| `Content-Type` | `image/webp` | Always WebP when a thumbnail exists; original `mime_type` when falling back to the original | +| `ETag` | `"{sha256-hex}"` | Same hash as the original image — content is immutable | +| `Cache-Control` | `public, max-age=31536000, immutable` | Safe: thumbnail content never changes | + +Body: raw image bytes (WebP thumbnail, or original bytes as fallback). + +#### `404 Not Found` — Image not found + +```json +{ "detail": "Image not found", "code": "image_not_found" } +``` + +#### `500 Internal Server Error` — Storage retrieval failure + +```json +{ "detail": "Failed to retrieve image content", "code": "storage_error" } +``` + +--- + +## Changed endpoint: `POST /api/v1/images` + +The upload response body gains one new field: + +| Field | Type | Notes | +|-------|------|-------| +| `thumbnail_key` | `string \| null` | S3 key of the generated thumbnail. `null` if generation failed. | + +All existing fields are unchanged. + +**Example response** (new field only shown): +```json +{ + "id": "...", + "thumbnail_key": "abc123…-thumb", + ... +} +``` + +--- + +## Changed endpoint: `GET /api/v1/images` and `GET /api/v1/images/{id}` + +Both metadata responses gain the same `thumbnail_key` field (`string | null`). + +--- + +## UI contract + +The Angular `ImageService` gains one new method: + +``` +getThumbnailUrl(id: string): string + → '/api/v1/images/{id}/thumbnail' +``` + +The `ImageRecord` interface gains: + +``` +thumbnail_key: string | null; +``` + +The library grid component uses `getThumbnailUrl(image.id)` as the `src` for +every grid cell. The detail component continues using `getFileUrl(image.id)`. diff --git a/specs/003-upload-thumbnails/data-model.md b/specs/003-upload-thumbnails/data-model.md new file mode 100644 index 0000000..30918b9 --- /dev/null +++ b/specs/003-upload-thumbnails/data-model.md @@ -0,0 +1,79 @@ +# Data Model: Upload Thumbnails + +**Branch**: `003-upload-thumbnails` | **Date**: 2026-05-03 + +## Schema change: `images` table + +One nullable column is added to the existing `images` table. + +| Column | Type | Nullable | Default | Notes | +|--------|------|----------|---------|-------| +| `thumbnail_key` | `VARCHAR(70)` | YES | `NULL` | S3 object key for the WebP thumbnail. `NULL` = no thumbnail available (generation failed or pre-dates this feature). Derived value: `{image.hash}-thumb`. | + +No other tables change. No new tables are added. + +### Migration + +**File**: `api/alembic/versions/002_add_thumbnail_key.py` + +``` +upgrade: ALTER TABLE images ADD COLUMN thumbnail_key VARCHAR(70); +downgrade: ALTER TABLE images DROP COLUMN thumbnail_key; +``` + +--- + +## ORM model change: `Image` + +`api/app/models.py` — `Image` class gains one field: + +``` +thumbnail_key: Mapped[str | None] = mapped_column(String(70), nullable=True, default=None) +``` + +--- + +## New module: `api/app/thumbnail.py` + +Contains the thumbnail generation logic. Not a model, but documented here because +it defines the thumbnail's shape: + +| Aspect | Value | +|--------|-------| +| Output format | WebP | +| Max dimension (longest side) | 400 px | +| Aspect ratio | Preserved (never upscaled) | +| Source formats supported | JPEG, PNG, GIF (frame 0), WebP | +| Key signature | `async def generate_thumbnail(data: bytes, mime_type: str) -> bytes` | + +--- + +## API response shape change + +`_image_to_dict()` in `api/app/routers/images.py` adds `"thumbnail_key"` to its +output so the UI can determine whether a thumbnail is available: + +```json +{ + "id": "...", + "hash": "...", + "thumbnail_key": "abc123...-thumb", ← new (null if no thumbnail) + ... +} +``` + +The UI uses the presence of `thumbnail_key` to decide whether to call +`/api/v1/images/{id}/thumbnail` (with thumbnail) or fall back to +`/api/v1/images/{id}/file` (without). In practice the endpoint itself +handles the fallback, so the UI can always call `/thumbnail`. + +--- + +## Storage objects per image (after this feature) + +| Object | Key | Format | Created at | +|--------|-----|--------|-----------| +| Original | `{sha256_hash}` | Original mime_type | Upload | +| Thumbnail | `{sha256_hash}-thumb` | `image/webp` | Upload (same request) | + +Thumbnail object is deleted alongside original on image deletion. diff --git a/specs/003-upload-thumbnails/plan.md b/specs/003-upload-thumbnails/plan.md new file mode 100644 index 0000000..11fbc89 --- /dev/null +++ b/specs/003-upload-thumbnails/plan.md @@ -0,0 +1,246 @@ +# Implementation Plan: Upload Thumbnails + +**Branch**: `003-upload-thumbnails` | **Date**: 2026-05-03 | **Spec**: [spec.md](spec.md) +**Input**: Feature specification from `specs/003-upload-thumbnails/spec.md` + +## Summary + +When an image is uploaded, generate a WebP thumbnail (longest side ≤ 400 px, +aspect ratio preserved) and store it in S3 alongside the original. Add a +`GET /api/v1/images/{id}/thumbnail` endpoint that serves the thumbnail (or falls +back to the original for images that pre-date the feature). The Angular library +grid switches from `/file` to `/thumbnail`. The detail page is unchanged. + +Changes span: a new Pillow dependency, a new `thumbnail.py` utility module, one +Alembic migration, the upload and delete routes, a new thumbnail serve route, and +the Angular image service and library component. + +## Technical Context + +**Language/Version**: Python 3.12+ (API); TypeScript strict mode (UI) +**Primary Dependencies**: FastAPI, Pillow (new — thumbnail generation), aiobotocore, + SQLAlchemy 2.x async, Alembic, Angular +**Storage**: S3-compatible object storage via `StorageBackend.put()` and `.get()`; + thumbnails stored at key `{sha256_hash}-thumb` in the same bucket +**Testing**: pytest + pytest-asyncio (API); Angular Karma/Jest + TestBed (UI) +**Target Platform**: Linux server (containerised); modern evergreen desktop browsers +**Project Type**: Web application — FastAPI API + Angular SPA +**Performance Goals**: 20-image grid transfers ≥ 80% less data than full-size; + first page of 1,000-image library loads in under 2 s +**Constraints**: Thumbnail generation is synchronous within the upload request; + thumbnail failure must not block upload success; no backfill of existing images in v1 +**Scale/Scope**: Single-user personal application; upload frequency low + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-checked after Phase 1 design below.* + +| Principle | Check | Status | +|-----------|-------|--------| +| §2.1 Separation of concerns | `thumbnail.py` owns resize logic; router owns orchestration; UI knows nothing about S3 keys | ✅ | +| §2.2 Dependency direction | UI → API → Storage; thumbnail stored via `StorageBackend`; no upward imports | ✅ | +| §2.3 Storage abstraction | All thumbnail I/O via `StorageBackend.put()` and `.get()`; no raw S3 SDK calls in routes or `thumbnail.py` | ✅ | +| §2.4 Auth abstraction | No change to auth flow | ✅ | +| §2.5 DB abstraction | New `thumbnail_key` column accessed only through `ImageRepository`; migration added | ✅ | +| §2.6 No speculative abstraction | `thumbnail.py` is a concrete module-level function; no interface added because one implementation exists | ✅ | +| §3.1 API versioning | New route at `/api/v1/images/{id}/thumbnail` | ✅ | +| §3.3 Error shape | `image_not_found` and `storage_error` codes used consistently | ✅ | +| §4.2 Images immutable after upload | Thumbnail is generated at upload time only; never mutated | ✅ | +| §4.3 Dedup by hash | Duplicate upload returns existing record including existing `thumbnail_key`; no re-generation | ✅ | +| §5.1 TDD non-negotiable | Failing tests written before every implementation task | ✅ | +| §5.2 Test pyramid | Unit test for `thumbnail.py`; integration tests for new route + upload + delete | ✅ | +| §5.3 Test colocation | API tests in `api/tests/`; Angular spec files colocated with components | ✅ | +| §5.4 CI gate | All tests + ruff must pass before milestone is done | ✅ | +| §7.1 One-command start | No change to `docker-compose.yml` required | ✅ | +| §7.2 Env configuration | No new env vars; Pillow is a build dependency, not a runtime config | ✅ | +| §8 Scope boundaries | Backfill of existing images, multiple thumbnail sizes, animated WebP — all deferred | ✅ | + +**Post-design re-check**: All gates still pass after Phase 1 design. + +## Project Structure + +### Documentation (this feature) + +```text +specs/003-upload-thumbnails/ +├── plan.md # This file +├── spec.md # Feature specification +├── research.md # Phase 0 decisions +├── data-model.md # Schema and module changes +├── contracts/ +│ └── api.md # New endpoint + changed response shapes +├── checklists/ +│ └── requirements.md # Spec quality checklist +└── tasks.md # Phase 2 output (/speckit-tasks — NOT created here) +``` + +### Files changed or created + +```text +api/ +├── pyproject.toml # Add Pillow dependency +├── app/ +│ ├── thumbnail.py # NEW — generate_thumbnail() +│ ├── models.py # Add thumbnail_key column to Image +│ ├── repositories/ +│ │ └── image_repo.py # Pass thumbnail_key on create +│ └── routers/ +│ └── images.py # Upload: generate+store thumbnail +│ # Delete: remove thumbnail +│ # New route: GET /images/{id}/thumbnail +│ # _image_to_dict: add thumbnail_key field +└── alembic/ + └── versions/ + └── 002_add_thumbnail_key.py # NEW migration + +ui/ +└── src/ + └── app/ + ├── services/ + │ └── image.service.ts # Add getThumbnailUrl(); add thumbnail_key to ImageRecord + └── library/ + └── library.component.ts # Use getThumbnailUrl() for grid image src +``` + +## Milestones + +> **TDD ORDER IS MANDATORY** (constitution §5.1): For every milestone, write +> the failing test(s) first, confirm they fail, then implement until they pass. + +--- + +### M1 — Thumbnail generation utility + +**Goal**: A tested, self-contained function that produces a WebP thumbnail from +raw image bytes. + +**Deliverables**: +- Add `pillow>=10.0` to `[project.dependencies]` in `api/pyproject.toml` +- Create `api/app/thumbnail.py` with `generate_thumbnail(data: bytes, mime_type: str) -> bytes`: + - Open image bytes with Pillow + - Seek to frame 0 (handles animated GIFs) + - Convert mode as needed for WebP output + - Resize to fit within 400×400 using LANCZOS resampling (never upscale) + - Encode as WebP quality 80 and return bytes +- Unit tests in `api/tests/unit/test_thumbnail.py`: + - `test_thumbnail_is_webp` — output starts with WebP magic bytes + - `test_thumbnail_fits_within_400px` — both dimensions ≤ 400 + - `test_thumbnail_preserves_aspect_ratio` — ratio within 1% of original + - `test_thumbnail_handles_gif_first_frame` — GIF input produces static WebP + - `test_thumbnail_handles_png_with_alpha` — RGBA PNG produces valid WebP + - `test_thumbnail_does_not_upscale` — 100×100 image stays ≤ 100×100 + +**Done criterion**: All unit tests pass; `ruff check api/` passes. + +--- + +### M2 — Database migration + +**Goal**: The `images` table has a nullable `thumbnail_key` column; the ORM model +and repository reflect it. + +**Deliverables**: +- `api/alembic/versions/002_add_thumbnail_key.py` — `upgrade` adds + `VARCHAR(70) NULLABLE` column; `downgrade` drops it +- `api/app/models.py`: add `thumbnail_key: Mapped[str | None]` mapped to + `String(70)`, `nullable=True`, `default=None` +- `api/app/repositories/image_repo.py`: add `thumbnail_key: str | None = None` + parameter to `create()`; persist it on the new `Image` instance + +**Done criterion**: `alembic upgrade head` runs cleanly inside Docker; all +existing 46 integration tests still pass (new column is nullable, no existing +test breaks). + +--- + +### M3 — Upload route: generate and store thumbnail + +**Goal**: Every new upload generates a thumbnail; duplicates reuse the existing +record; failures are tolerated without blocking the upload. + +**TDD first** — new tests in `api/tests/integration/test_upload.py`: +- `test_upload_returns_thumbnail_key` — upload response includes non-null + `thumbnail_key` ending in `-thumb` +- `test_duplicate_upload_reuses_thumbnail_key` — second upload of the same + file returns the same `thumbnail_key` as the first +- `test_upload_succeeds_when_thumbnail_fails` — patch `generate_thumbnail` to + raise, upload returns 200/201 with `thumbnail_key: null` + +**Implementation** in `api/app/routers/images.py` `upload_image()`: +1. After `await storage.put(hash_hex, data, mime_type)`, attempt thumbnail generation + and storage in a try/except; catch any exception and leave `thumbnail_key` as `None` +2. Call `thumb_bytes = await asyncio.to_thread(generate_thumbnail, data, mime_type)` + — `generate_thumbnail` is CPU-bound (Pillow); `asyncio.to_thread` runs it in the + default thread pool executor so it does not block the async event loop +3. Pass `thumbnail_key` to `image_repo.create()` +4. Add `"thumbnail_key": image.thumbnail_key` to `_image_to_dict()` + +**Done criterion**: New tests pass; all 46 existing tests pass. + +--- + +### M4 — New `GET /api/v1/images/{id}/thumbnail` endpoint + +**Goal**: Clients fetch thumbnail content; falls back to original if no thumbnail exists. + +**TDD first** — new tests in `api/tests/integration/test_serving.py`: +- `test_thumbnail_returns_webp` — upload image, call `/thumbnail`, assert + 200, `content-type: image/webp`, ETag, `Cache-Control` with immutable +- `test_thumbnail_fallback_returns_original` — set `thumbnail_key=None` on a + record, call `/thumbnail`, assert 200 with original mime_type +- `test_thumbnail_unknown_id_returns_404` — unknown UUID → 404 `image_not_found` + +**Implementation**: new route `GET /images/{image_id}/thumbnail` in +`api/app/routers/images.py` using `image.thumbnail_key or image.storage_key` +to select the key, and `"image/webp" if image.thumbnail_key else image.mime_type` +for the content type. Same `ETag` + `Cache-Control` headers as `/file`. + +**Done criterion**: All new tests pass; all existing tests pass. + +--- + +### M5 — Delete route: remove thumbnail from storage + +**Goal**: Deleting an image also removes its thumbnail; no orphaned objects left. + +**TDD first** — new test in `api/tests/integration/test_delete.py`: +- `test_delete_removes_thumbnail` — upload image, delete it, then verify + `GET /images/{id}/thumbnail` returns 404 + +**Implementation** in `api/app/routers/images.py` `delete_image()`: after +deleting the DB record and the original object, call `await storage.delete(image.thumbnail_key)` +if `image.thumbnail_key` is not None. + +**Done criterion**: New test passes; all existing delete tests pass. + +--- + +### M6 — UI: library grid uses thumbnail endpoint + +**Goal**: Library grid fetches thumbnails instead of full-size originals; detail +page is unchanged. + +**Deliverables**: +- `ui/src/app/services/image.service.ts`: + - Add `thumbnail_key: string | null` to `ImageRecord` interface + - Add `getThumbnailUrl(id: string): string` returning `/api/v1/images/${id}/thumbnail` +- `ui/src/app/library/library.component.ts` + template: replace + `getFileUrl(image.id)` with `getThumbnailUrl(image.id)` for grid `` +- Update Angular spec files: add `thumbnail_key: null` to all `ImageRecord` + mock objects +- Verify `ng test` passes and `ng build` succeeds + +**Done criterion**: Angular build clean; all Angular tests pass; library grid +`` elements reference `/thumbnail` not `/file`. + +## Post-design Constitution Re-check + +| Principle | Verdict | +|-----------|---------| +| §2.3 Storage abstraction | All thumbnail I/O via `StorageBackend`; `thumbnail.py` never touches S3 directly | ✅ | +| §2.5 DB abstraction | `thumbnail_key` persisted only through `ImageRepository.create()` | ✅ | +| §2.6 No speculative abstraction | One concrete function; no interface | ✅ | +| §4.2 Immutability | Thumbnail written once at upload; never mutated | ✅ | +| §5.1 TDD | Failing tests written before each milestone's implementation | ✅ | + +All gates pass. Feature is ready for `/speckit-tasks`. diff --git a/specs/003-upload-thumbnails/research.md b/specs/003-upload-thumbnails/research.md new file mode 100644 index 0000000..44cabbd --- /dev/null +++ b/specs/003-upload-thumbnails/research.md @@ -0,0 +1,130 @@ +# Research: Upload Thumbnails + +**Branch**: `003-upload-thumbnails` | **Date**: 2026-05-03 + +## Decision 1: Image processing library + +**Decision**: Add `Pillow` as a runtime dependency to the API. + +**Rationale**: Pillow is the standard Python image processing library. It supports +reading JPEG, PNG, GIF (frame extraction), and WebP, and can encode output as WebP. +It handles aspect-ratio-preserving resize natively via `Image.thumbnail()`. No +other dependency is needed. + +**Alternatives considered**: +- `wand` (ImageMagick binding): More powerful but much heavier; overkill for a + fixed-size resize operation. +- `opencv-python`: ML-focused, large binary; not justified for simple resize. +- Pure `aiobotocore` + external service: Adds operational complexity with no benefit + over a local library for a single-user app. + +--- + +## Decision 2: Thumbnail dimensions and format + +**Decision**: Longest side ≤ 400 px, WebP output, aspect ratio preserved. This +matches FR-003 and FR-004 exactly and the user's stated preference. + +**Rationale**: WebP produces smaller files than JPEG/PNG at equivalent visual quality. +400 px covers a typical grid thumbnail at 1× and 2× display density without being +oversized. Pillow's `Image.thumbnail((400, 400))` implements this constraint directly +(it shrinks to fit within the bounding box, never upscaling). + +**Alternatives considered**: +- JPEG thumbnails: Larger file sizes; no alpha channel support. +- Multiple sizes: Out of scope for v1 per spec Assumptions. +- On-demand resize: Rejected by user in favour of pre-generation. + +--- + +## Decision 3: Thumbnail storage key convention + +**Decision**: `{sha256_hash}-thumb` (e.g., the 64-char hash hex string + literal +`-thumb`, giving a 70-char key). Stored under the same S3 bucket as originals. + +**Rationale**: Deterministic from the image hash — no new random state needed and the +key can always be reconstructed from the `Image.hash` field. The `-thumb` suffix +clearly distinguishes it from the original key. Fits within a `String(70)` column. + +**Alternatives considered**: +- Separate bucket for thumbnails: More complex bucket policy management with no benefit + for a single-user app. +- UUID-based key: Non-deterministic; requires an extra DB round-trip to look up. +- `{hash}/thumb.webp` (path prefix): Works, but adds key parsing complexity for no gain. + +--- + +## Decision 4: Database schema change + +**Decision**: Add a nullable `thumbnail_key: String(70)` column to the `images` +table. `NULL` means no thumbnail exists (either generation failed or the image +pre-dates this feature). Add a new Alembic migration `002_add_thumbnail_key.py`. + +**Rationale**: Explicitly tracking the thumbnail key in the DB makes the "does a +thumbnail exist?" question a simple `IS NOT NULL` check rather than an S3 head +request. Also allows the delete route to skip the thumbnail delete if the column +is NULL, avoiding a storage error for legacy images. + +**Alternatives considered**: +- Derive key at runtime from `image.hash + "-thumb"` without a DB column: Simpler but + means no way to distinguish "thumbnail was generated" from "thumbnail was never + attempted", and delete would need a conditional S3 head request. +- Separate `thumbnails` table: Over-engineered; one thumbnail per image with no + additional attributes doesn't warrant its own table. + +--- + +## Decision 5: Where thumbnail generation lives in the code + +**Decision**: A standalone async function `generate_thumbnail(data: bytes, mime_type: str) -> bytes` +in a new module `api/app/thumbnail.py`. Called from the upload route after the original +is stored, before the Image record is created. + +**Rationale**: Keeps the thumbnail logic self-contained and independently testable. +The upload route calls it but doesn't own it. Constitution §2.6 allows concrete +functions when no second implementation is needed — no abstract interface is warranted. + +**Alternatives considered**: +- Method on `StorageBackend`: Wrong layer; storage knows nothing about image content. +- Inline in the upload route: Makes the route harder to test and read. +- A `ThumbnailService` class: No justification for a class when a module-level function suffices. + +--- + +## Decision 6: Failure handling during upload + +**Decision**: If `generate_thumbnail()` raises, log the exception, set `thumbnail_key` +to `NULL` on the Image record, and continue. The upload response succeeds. The +`GET /api/v1/images/{id}/thumbnail` endpoint falls back to the original when +`thumbnail_key` is NULL (FR-009). + +**Rationale**: A thumbnail failure should not block the upload — the user still gets +their image in the library. The fallback in the thumbnail endpoint ensures the grid +still renders something. + +--- + +## Decision 7: Thumbnail endpoint response + +**Decision**: `GET /api/v1/images/{id}/thumbnail` follows the same pattern as +`GET /api/v1/images/{id}/file`: +- Returns `200` with binary content, `Content-Type: image/webp` (or original + `mime_type` when falling back to original), `ETag`, and + `Cache-Control: public, max-age=31536000, immutable`. +- Returns `404` with `{"detail": "...", "code": "image_not_found"}` if the image + does not exist. +- Falls back to the original when `thumbnail_key IS NULL`. + +**Rationale**: Consistent with the existing `/file` endpoint pattern established in +feature 002. The UI only needs to know one URL per image for the grid. + +--- + +## Decision 8: GIF handling + +**Decision**: For GIF uploads, `generate_thumbnail()` extracts frame 0 via +`Image.seek(0)` before resizing. The output is always WebP (static, not animated). + +**Rationale**: Matches spec assumption: "Animated GIF thumbnails capture only the +first frame; animation is not preserved in the thumbnail." Pillow supports this +with `im.seek(0)`. diff --git a/specs/003-upload-thumbnails/spec.md b/specs/003-upload-thumbnails/spec.md new file mode 100644 index 0000000..fbc990d --- /dev/null +++ b/specs/003-upload-thumbnails/spec.md @@ -0,0 +1,180 @@ +# Feature Specification: Upload Thumbnails + +**Feature Branch**: `003-upload-thumbnails` +**Created**: 2026-05-03 +**Status**: Draft +**Input**: User description: "When users load the UI, full size images are fetched, which may cause considerable load time when there are a lot of images -- a grid of 20 images could silently pull several hundred megabytes. We will solve this by pre-generating thumbnails on upload: when an image is uploaded, immediately produce one (or a few) fixed-size thumbnail variants and store them alongside the original. The library always fetches the thumbnail key, the detail page fetches the original key. Zero resize cost at serve time. A single fixed-size re-encoded as WebP for smaller bytes will cover the grid view." + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 — Fast Library Load (Priority: P1) + +A user opens the application or runs a tag-filtered search. The image grid +loads quickly even when the library contains many images, because each grid +cell fetches a compact thumbnail rather than the full-size original. + +**Why this priority**: This is the core motivation. A grid of 20 images today +could pull hundreds of megabytes; thumbnails bring that down to a few +megabytes, making the library usable on slow or metered connections. + +**Independent Test**: Upload 20 images of varying sizes (including some near +the 50 MB limit). Open the library. Measure total bytes transferred while the +grid loads. Compare against loading the same library before this feature. +Verify the grid renders fully and that each thumbnail is visually recognisable +as the correct image. + +**Acceptance Scenarios**: + +1. **Given** a library with multiple images, **When** the user opens the + library page, **Then** each grid cell displays a thumbnail that is visually + recognisable as its image, and the total data transferred to render the + full grid is substantially less than the sum of the original file sizes. + +2. **Given** a user applies one or more tag filters, **When** the filtered + results are displayed, **Then** thumbnails are shown for all matching + images with the same reduced data footprint. + +3. **Given** a library with images of mixed types (JPEG, PNG, GIF, WebP), + **When** the grid loads, **Then** thumbnails for all types display + correctly. + +--- + +### User Story 2 — Full-Size Image on Detail Page (Priority: P1) + +A user clicks an image in the library grid to open its detail page. The +full-size original is displayed, not the thumbnail. The experience is +unchanged from before this feature. + +**Why this priority**: The detail page is where the user inspects or copies +an image; showing the thumbnail there would degrade the product's core value. + +**Independent Test**: Open any image's detail page. Verify the image +displayed matches the original resolution and file size, not the thumbnail +dimensions. + +**Acceptance Scenarios**: + +1. **Given** the user clicks an image thumbnail in the library, **When** the + detail page loads, **Then** the full-size original image is displayed at + its native resolution. + +2. **Given** the user navigates directly to an image's detail URL, **When** + the page loads, **Then** the full-size original is displayed. + +--- + +### User Story 3 — Thumbnails Generated Automatically on Upload (Priority: P1) + +A user uploads a new image. Without any additional action, a thumbnail is +available immediately. There is no separate step or explicit request to +generate a thumbnail. + +**Why this priority**: The value of the feature depends entirely on thumbnails +being present for every image. Manual generation or lazy generation would +create inconsistencies in the grid. + +**Independent Test**: Upload a new image. Immediately open the library. Verify +the new image's thumbnail appears in the grid without any extra action. + +**Acceptance Scenarios**: + +1. **Given** the user uploads a supported image, **When** the upload + completes, **Then** a thumbnail is available and appears correctly in + the library grid. + +2. **Given** the user uploads a duplicate image (already in the library), + **When** the upload completes, **Then** no redundant thumbnail is + generated — the existing thumbnail is reused. + +3. **Given** the user uploads an image at or near the maximum supported + file size (50 MB), **When** the upload completes, **Then** the thumbnail + is generated successfully and the upload response time remains acceptable. + +--- + +### Edge Cases + +- What happens when thumbnail generation fails during upload? → The upload + still succeeds and the original image is stored; a fallback to the original + is shown in the grid, or the item is hidden until the thumbnail is available + (assumption: fall back to original rather than silently drop the image). +- What happens when an image is deleted? → Both the original and its thumbnail + are removed from storage. +- What happens with existing images that were uploaded before this feature? → + Those images have no pre-generated thumbnail; the grid falls back to the + original for those entries until a backfill is performed (backfill is out of + scope for v1 of this feature). +- What happens with animated GIFs? → A static thumbnail is generated from the + first frame. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The system MUST generate a thumbnail for every newly uploaded + image as part of the upload operation, before the upload response is + returned to the caller. +- **FR-002**: Thumbnails MUST be stored in the same object storage as the + original, addressable by a distinct key derived from the image. +- **FR-003**: The thumbnail MUST be encoded as WebP regardless of the original + image format. +- **FR-004**: The thumbnail MUST fit within a fixed maximum dimension on its + longest side, preserving the original aspect ratio; no dimension of the + thumbnail MAY exceed 400 pixels. +- **FR-005**: The library grid view MUST fetch and display thumbnails instead + of original images. +- **FR-006**: The image detail view MUST continue to fetch and display the + full-size original. +- **FR-007**: When a duplicate image is uploaded, the thumbnail MUST NOT be + regenerated or re-stored; the existing thumbnail is reused. +- **FR-008**: When an image is deleted, its thumbnail MUST also be deleted + from storage. +- **FR-009**: If thumbnail generation fails during upload, the upload MUST + still succeed; the system MUST fall back to serving the original image in + the grid for that entry. +- **FR-010**: The API MUST expose a way for clients to retrieve the thumbnail + content for a given image, distinct from the full-size content endpoint. + +### Key Entities *(include if feature involves data)* + +- **Image**: Gains a new optional attribute indicating whether a thumbnail is + available and the key under which the thumbnail is stored. +- **Thumbnail**: A derived, smaller representation of an Image. Key + attributes: storage key, dimensions (width × height), format (WebP), + relationship to its source Image. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: The total data transferred to render a 20-image library grid is + reduced by at least 80% compared to fetching full-size originals for the + same images. +- **SC-002**: The library grid's first page loads in under 2 seconds on a + local network connection for a library of 1,000 images, with thumbnails + visible without a second load. +- **SC-003**: Thumbnails are available immediately after upload completes — + no polling or manual refresh is required. +- **SC-004**: The detail page continues to show the full-size original; no + regression in detail-page image quality is introduced. +- **SC-005**: Deleting an image removes both the original and its thumbnail; + no orphaned thumbnail objects remain in storage after deletion. + +## Assumptions + +- A single thumbnail size (longest side ≤ 400 px, WebP) is sufficient for + the library grid view in v1. Additional sizes or formats are out of scope. +- Thumbnail generation happens synchronously during the upload request. Async + background processing is not required for v1. +- Existing images uploaded before this feature are not automatically + backfilled with thumbnails in v1; the grid falls back to the original for + those entries. +- Animated GIF thumbnails capture only the first frame; animation is not + preserved in the thumbnail. +- The thumbnail storage key is derived deterministically from the image's + existing content hash, so no additional database column is strictly required + to locate it — however the Image record will track thumbnail availability + for correctness. +- No change is required to tag management, duplicate detection, or any other + upload behaviour beyond adding thumbnail generation. diff --git a/specs/003-upload-thumbnails/tasks.md b/specs/003-upload-thumbnails/tasks.md new file mode 100644 index 0000000..b65424a --- /dev/null +++ b/specs/003-upload-thumbnails/tasks.md @@ -0,0 +1,186 @@ +# Tasks: Upload Thumbnails + +**Input**: Design documents from `specs/003-upload-thumbnails/` +**Prerequisites**: plan.md ✅, spec.md ✅, research.md ✅, data-model.md ✅, contracts/api.md ✅ + +**TDD**: Tests are non-negotiable per constitution §5.1. Every test task MUST be written and confirmed failing before its implementation task runs. + +**Organization**: Tasks grouped by user story to enable independent implementation and testing. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies on incomplete tasks) +- **[Story]**: Which user story this task belongs to (US1, US2, US3) + +--- + +## Phase 1: Setup + +- [X] T001 Add `pillow>=10.0` to `[project.dependencies]` in `api/pyproject.toml`; rebuild the Docker API image (`docker compose build api`) so Pillow is available inside the container for all subsequent test runs + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Thumbnail generation logic and the DB schema change that all three user stories depend on. + +**⚠️ CRITICAL**: All tasks in this phase must complete before any user story work begins. + +### Tests for thumbnail utility (write FIRST — must FAIL before T005) ⚠️ + +- [X] T002 Write 6 unit tests in `api/tests/unit/test_thumbnail.py` — `test_thumbnail_is_webp` (output begins with WebP magic bytes `RIFF...WEBP`), `test_thumbnail_fits_within_400px` (both dimensions ≤ 400), `test_thumbnail_preserves_aspect_ratio` (ratio within 1% of original), `test_thumbnail_handles_gif_first_frame` (GIF input → static WebP, no animation), `test_thumbnail_handles_png_with_alpha` (RGBA PNG → valid WebP output), `test_thumbnail_does_not_upscale` (100×100 input stays ≤ 100×100); to confirm the TDD red state, first create an empty stub `api/app/thumbnail.py` (so pytest can collect tests), then run `pytest api/tests/unit/test_thumbnail.py` and confirm all 6 **fail** with assertion errors (not import errors) + +### Thumbnail utility implementation + +- [X] T003 Create `api/app/thumbnail.py` with `generate_thumbnail(data: bytes, mime_type: str) -> bytes`: open bytes with `PIL.Image.open(BytesIO(data))`, call `.seek(0)` to target frame 0 (GIF support), convert mode to RGB or RGBA as needed for WebP, call `.thumbnail((400, 400), PIL.Image.LANCZOS)` (never upscales), save to a `BytesIO` buffer as WebP quality=80, return bytes; run `pytest api/tests/unit/test_thumbnail.py` and confirm all 6 pass + +### Database migration + +- [X] T004 [P] Create `api/alembic/versions/002_add_thumbnail_key.py` with `upgrade()` calling `op.add_column("images", sa.Column("thumbnail_key", sa.String(70), nullable=True))` and `downgrade()` calling `op.drop_column("images", "thumbnail_key")`; set `revision="002"`, `down_revision="001"` +- [X] T005 [P] Add `thumbnail_key: Mapped[str | None] = mapped_column(String(70), nullable=True, default=None)` to the `Image` class in `api/app/models.py` +- [X] T006 Add `thumbnail_key: str | None = None` keyword argument to `ImageRepository.create()` in `api/app/repositories/image_repo.py`; include it in the `Image(...)` constructor call; run `docker compose run --rm api alembic upgrade head` inside the container and confirm migration applies cleanly; run `pytest api/` to confirm all 46 existing tests still pass + +**Checkpoint**: Pillow available, thumbnail.py works, schema migrated, all existing tests green. + +--- + +## Phase 3: User Story 3 — Thumbnails Generated Automatically on Upload (Priority: P1) 🎯 + +**Goal**: Every new upload triggers thumbnail generation and storage as part of the same request. No extra step required from the user. + +**Independent Test**: Upload any supported image. Immediately check the upload response — it includes a non-null `thumbnail_key`. Call `GET /api/v1/images/{id}/thumbnail` — it returns 200 with `content-type: image/webp`. + +### Tests for User Story 3 (write FIRST — must FAIL before T008) ⚠️ + +- [X] T007 [US3] Add three tests to `api/tests/integration/test_upload.py`: `test_upload_returns_thumbnail_key` (upload a JPEG/PNG/WebP, assert response JSON contains `thumbnail_key` ending in `-thumb`), `test_duplicate_upload_reuses_thumbnail_key` (upload same file twice, assert both responses have equal, non-null `thumbnail_key`), and `test_upload_succeeds_when_thumbnail_fails` (patch `generate_thumbnail` to raise an exception, upload an image, assert response is 200/201 with `thumbnail_key: null` — upload must not be blocked by thumbnail failure); run `pytest api/tests/integration/test_upload.py` and confirm all three new tests **fail** + +### Implementation for User Story 3 + +- [X] T008 [US3] In `api/app/routers/images.py` `upload_image()`: import `asyncio` and `generate_thumbnail` from `app.thumbnail`; after `await storage.put(hash_hex, data, mime_type)`, wrap thumbnail generation in a try/except — call `thumb_bytes = await asyncio.to_thread(generate_thumbnail, data, mime_type)` (runs CPU-bound Pillow work off the async event loop), store result via `await storage.put(f"{hash_hex}-thumb", thumb_bytes, "image/webp")`, set `thumbnail_key = f"{hash_hex}-thumb"`; on any exception log a warning and leave `thumbnail_key = None`; pass `thumbnail_key=thumbnail_key` to `image_repo.create()` +- [X] T009 [US3] Add `"thumbnail_key": image.thumbnail_key` to the dict returned by `_image_to_dict()` in `api/app/routers/images.py` +- [X] T010 [US3] Run `pytest api/tests/integration/test_upload.py` to confirm both new tests pass; then run `pytest api/` to confirm no regressions + +**Checkpoint**: Upload generates and stores a thumbnail. Duplicate uploads reuse the existing thumbnail. `thumbnail_key` appears in all image metadata responses. + +--- + +## Phase 4: User Story 1 — Fast Library Load (Priority: P1) + +**Goal**: The library grid fetches compact WebP thumbnails instead of full-size originals, dramatically reducing page-load bandwidth. + +**Independent Test**: Open the library in a browser with DevTools network tab open. All grid `` elements request `/api/v1/images/{id}/thumbnail`. Total bytes transferred for a 20-image grid is a small fraction of what the originals would cost. + +### Tests for User Story 1 (write FIRST — must FAIL before T013) ⚠️ + +- [X] T011 [US1] Add `test_thumbnail_returns_webp` (upload image, GET `/thumbnail`, assert 200, `content-type: image/webp`, ETag header matches `f'"{image_hash}"'`, `"immutable"` in `cache-control`, non-empty content), `test_thumbnail_fallback_returns_original` (manually set `thumbnail_key=None` on a DB record via the session fixture, GET `/thumbnail`, assert 200 with original `mime_type` in content-type), and `test_thumbnail_unknown_id_returns_404` (unknown UUID, assert 404 `image_not_found`) to `api/tests/integration/test_serving.py`; run and confirm all three **fail** +- [X] T012 [P] [US1] In `ui/src/app/services/image.service.ts`: add `thumbnail_key: string | null` field to the `ImageRecord` interface; add `getThumbnailUrl(id: string): string { return \`${this.base}/images/${id}/thumbnail\`; }` method to `ImageService` + +### Implementation for User Story 1 + +- [X] T013 [US1] Add `GET /api/v1/images/{image_id}/thumbnail` route to `api/app/routers/images.py`: look up image (404 if missing), select `key = image.thumbnail_key or image.storage_key` and `media_type = "image/webp" if image.thumbnail_key else image.mime_type`, call `await storage.get(key)` in a try/except (500 `storage_error` on failure), return `Response(content=data, media_type=media_type, headers={"ETag": f'"{image.hash}"', "Cache-Control": "public, max-age=31536000, immutable"})` +- [X] T014 [US1] In `ui/src/app/library/library.component.ts` and its HTML template: replace every use of `imageService.getFileUrl(image.id)` (or equivalent) with `imageService.getThumbnailUrl(image.id)` for grid cell `` bindings +- [X] T015 [US1] Add `thumbnail_key: null` to every `ImageRecord` mock/stub object in `ui/src/app/services/image.service.spec.ts`, `ui/src/app/library/library.component.spec.ts`, `ui/src/app/detail/detail.component.spec.ts`, and `ui/src/app/upload/upload.component.spec.ts` +- [X] T016 [US1] Run `pytest api/tests/integration/test_serving.py` to confirm all three new thumbnail tests pass and no existing serving tests regress + +**Checkpoint**: `GET /api/v1/images/{id}/thumbnail` serves WebP with caching headers. Falls back to original for legacy images. Library grid `` elements all use the thumbnail endpoint. + +--- + +## Phase 5: User Story 2 — Full-Size Image on Detail Page (Priority: P1) + +**Goal**: The detail page continues to display the full-size original. No regression introduced by the thumbnail work. + +**Independent Test**: Navigate to any image detail page. The image displayed is full-resolution. Browser DevTools shows the detail `` requests `/api/v1/images/{id}/file`, not `/thumbnail`. + +### Verification for User Story 2 + +- [X] T017 [US2] Confirm `ui/src/app/detail/detail.component.ts` still calls `imageService.getFileUrl(image.id)` (not `getThumbnailUrl`) for its `` — no code change expected; if the file was accidentally updated in T014 or T015, revert the detail component to `getFileUrl` +- [X] T018 [US2] Run `ng test` (inside the UI container or locally) and confirm all Angular unit tests pass including the detail component spec; run `ng build` to confirm the Angular build succeeds + +**Checkpoint**: Detail page verified unchanged. Angular build and tests clean. + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +**Purpose**: Delete cleanup, final test run, linting. + +### Delete thumbnail on image deletion + +- [X] T019 Write `test_delete_removes_thumbnail` in `api/tests/integration/test_delete.py`: upload an image, delete it, then `GET /api/v1/images/{id}/thumbnail` and assert 404; run and confirm it **fails** (currently delete does not remove the thumbnail object) +- [X] T020 In `api/app/routers/images.py` `delete_image()`: capture `thumbnail_key = image.thumbnail_key` before `image_repo.delete(image)`; after deleting the original via `await storage.delete(storage_key)`, add `if thumbnail_key: await storage.delete(thumbnail_key)`; run `pytest api/tests/integration/test_delete.py` to confirm new test and all existing delete tests pass + +### Final validation + +- [X] T021 [P] Run `~/.local/bin/ruff check api/app/thumbnail.py api/app/routers/images.py api/app/models.py api/app/repositories/image_repo.py api/tests/unit/test_thumbnail.py` and fix any lint issues in the changed files +- [X] T022 Run `pytest api/ -v` and confirm all tests pass; record final count (expected: 46 existing + ~10 new = ~56 total) + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: No dependencies — start immediately +- **Foundational (Phase 2)**: Depends on Phase 1 (Pillow must be installed before tests can import it) +- **US3 (Phase 3)**: Depends on Phase 2 complete (T002–T006 all done) +- **US1 (Phase 4)**: Depends on Phase 3 complete (upload must set `thumbnail_key` before the endpoint can serve one) +- **US2 (Phase 5)**: Depends on Phase 4 complete (Angular changes in T014/T015 must be done before verifying no regression) +- **Polish (Phase 6)**: Depends on Phases 3, 4, and 5 complete + +### Within Each Phase + +- T002 (write failing tests) MUST precede T003 (implement thumbnail.py) +- T004 and T005 can run in parallel (different files) +- T006 (repo change) depends on T005 (model must compile first) +- T007 (write failing upload tests) MUST precede T008 (implement upload change) +- T011 (write failing serving tests) and T012 (UI service) can run in parallel +- T011 MUST precede T013 (implement thumbnail route) +- T019 (write failing delete test) MUST precede T020 (implement delete cleanup) + +--- + +## Parallel Example: Phase 2 (Foundational) + +```bash +# After T003 is done, run T004 and T005 together: +Task: "Create 002_add_thumbnail_key.py migration in api/alembic/versions/" +Task: "Add thumbnail_key column to Image ORM in api/app/models.py" +``` + +## Parallel Example: Phase 4 (US1) + +```bash +# T011 and T012 touch different layers — run together: +Task: "Write 3 failing thumbnail serving tests in api/tests/integration/test_serving.py" +Task: "Add getThumbnailUrl() and thumbnail_key field to ui/src/app/services/image.service.ts" +``` + +--- + +## Implementation Strategy + +### MVP (All three user stories — tightly coupled) + +All three user stories are P1 and interdependent: US3 (generation) enables US1 (grid) which proves US2 (detail unchanged) by contrast. Complete all phases in order. + +1. Phase 1: T001 (Pillow setup) +2. Phase 2: T002–T006 (core infrastructure) +3. Phase 3: T007–T010 (upload generates thumbnail) +4. Phase 4: T011–T016 (thumbnail endpoint + UI) +5. Phase 5: T017–T018 (detail page verification) +6. Phase 6: T019–T022 (delete cleanup + polish) +7. **STOP and VALIDATE**: Open library in browser; DevTools shows `/thumbnail` requests; bandwidth used is a fraction of original file sizes + +### Total tasks: 22 (T001–T022) + +--- + +## Notes + +- [P] tasks touch different files and have no mutual dependencies within their phase +- T002 must be run **before** T003 and confirmed failing — this is the TDD red step +- T007, T011, T019 are all "write failing test" steps — confirm failure before implementing +- `thumbnail_key` in the API response is informational; the UI always calls `/thumbnail` and lets the endpoint handle the fallback — no client-side conditional logic needed +- Existing images (pre-dating this feature) will have `thumbnail_key: null`; the `/thumbnail` endpoint serves their original transparently +- The backfill migration for existing images is explicitly out of scope for this feature diff --git a/ui/src/app/detail/detail.component.spec.ts b/ui/src/app/detail/detail.component.spec.ts index b4df3a6..58fbe8e 100644 --- a/ui/src/app/detail/detail.component.spec.ts +++ b/ui/src/app/detail/detail.component.spec.ts @@ -16,6 +16,7 @@ const MOCK_IMAGE = { width: 10, height: 10, storage_key: 'abc', + thumbnail_key: null, created_at: '2026-01-01T00:00:00Z', tags: ['cat', 'funny'], }; diff --git a/ui/src/app/library/library.component.spec.ts b/ui/src/app/library/library.component.spec.ts index c372af0..e2971ad 100644 --- a/ui/src/app/library/library.component.spec.ts +++ b/ui/src/app/library/library.component.spec.ts @@ -22,7 +22,7 @@ describe('LibraryComponent', () => { spyOn(imgSvc, 'list').and.returnValue( of({ items: [ - { id: '1', filename: 'a.jpg', tags: ['cat'], hash: '', mime_type: 'image/jpeg', size_bytes: 1, width: 1, height: 1, storage_key: '', created_at: '' }, + { id: '1', filename: 'a.jpg', tags: ['cat'], hash: '', mime_type: 'image/jpeg', size_bytes: 1, width: 1, height: 1, storage_key: '', thumbnail_key: null, created_at: '' }, ], total: 1, limit: 50, diff --git a/ui/src/app/library/library.component.ts b/ui/src/app/library/library.component.ts index 68e9dc5..c3f1290 100644 --- a/ui/src/app/library/library.component.ts +++ b/ui/src/app/library/library.component.ts @@ -48,7 +48,7 @@ import { TagService } from '../services/tag.service'; class="image-card" (click)="router.navigate(['/images', img.id])" > - +
{{ tag }}
diff --git a/ui/src/app/services/image.service.ts b/ui/src/app/services/image.service.ts index 35f091a..c427487 100644 --- a/ui/src/app/services/image.service.ts +++ b/ui/src/app/services/image.service.ts @@ -11,6 +11,7 @@ export interface ImageRecord { width: number; height: number; storage_key: string; + thumbnail_key: string | null; created_at: string; tags: string[]; duplicate?: boolean; @@ -54,6 +55,10 @@ export class ImageService { return `${this.base}/images/${id}/file`; } + getThumbnailUrl(id: string): string { + return `${this.base}/images/${id}/thumbnail`; + } + updateTags(id: string, tags: string[]): Observable { return this.http.patch(`${this.base}/images/${id}/tags`, { tags }); } diff --git a/ui/src/app/upload/upload.component.spec.ts b/ui/src/app/upload/upload.component.spec.ts index eb40639..47d210d 100644 --- a/ui/src/app/upload/upload.component.spec.ts +++ b/ui/src/app/upload/upload.component.spec.ts @@ -11,7 +11,8 @@ describe('UploadComponent', () => { let component: UploadComponent; function makeImageService(overrides: Partial = {}): jasmine.SpyObj { - return jasmine.createSpyObj('ImageService', { upload: of({} as any), ...overrides }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return jasmine.createSpyObj('ImageService', { upload: of({} as any), ...overrides } as any); } beforeEach(async () => {