From 3f30327fa55c3f362cbcf9ac3da49634aa32ce4f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 3 Jun 2026 23:50:19 -0400 Subject: [PATCH] feat(gallery): composable scroll filter (multi-tag AND, media, sort) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 backend for the gallery filter bar. Extends scroll/timeline/jump from a single mutually-exclusive filter to a composable one: - tag_ids: image must carry ALL of them (one correlated EXISTS per tag — AND, no row multiplication), replacing the single-tag JOIN. - artist_id composes with tags; media_type ('image'|'video') narrows by mime; post_id stays the exclusive post-detail path. - sort ('newest'|'oldest') flips the effective_date/id cursor comparison and ordering; the cursor value is unchanged (direction comes from the request). jump_cursor honors sort too. - Shared _apply_scope helper applied across scroll/timeline/jump so the timeline sidebar reflects the filtered set. API _parse_filters parses tag_id (comma list), artist_id, media, sort. Tests: multi-tag AND, media filter, sort reversal (service + API); post_id-excludes-others; single tag_id back-compat. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/scheduled_tasks.lock | 1 + backend/app/api/gallery.py | 55 ++++++---- backend/app/services/gallery_service.py | 127 ++++++++++++++++-------- tests/test_api_gallery.py | 18 ++++ tests/test_gallery_provenance_filter.py | 4 +- tests/test_gallery_service.py | 52 +++++++++- 6 files changed, 188 insertions(+), 69 deletions(-) create mode 100644 .claude/scheduled_tasks.lock diff --git a/.claude/scheduled_tasks.lock b/.claude/scheduled_tasks.lock new file mode 100644 index 0000000..c64db71 --- /dev/null +++ b/.claude/scheduled_tasks.lock @@ -0,0 +1 @@ +{"sessionId":"168e9de3-9f44-4fca-8fa6-09c87523c11e","pid":1470750,"procStart":"11499472","acquiredAt":1780543996001} \ No newline at end of file diff --git a/backend/app/api/gallery.py b/backend/app/api/gallery.py index 41b561c..1533532 100644 --- a/backend/app/api/gallery.py +++ b/backend/app/api/gallery.py @@ -8,26 +8,42 @@ from ..services.gallery_service import GalleryService gallery_bp = Blueprint("gallery", __name__, url_prefix="/api/gallery") +def _parse_filters(): + """Parse the composable gallery filters from query args. Raises + ValueError (→ 400) on malformed ids. `tag_id` accepts a single id or a + comma-separated list (AND); `media` is image|video; `sort` is + newest|oldest.""" + tag_raw = request.args.get("tag_id") + tag_ids = ( + [int(x) for x in tag_raw.split(",") if x.strip()] if tag_raw else None + ) or None + post_id_raw = request.args.get("post_id") + post_id = int(post_id_raw) if post_id_raw else None + artist_id_raw = request.args.get("artist_id") + artist_id = int(artist_id_raw) if artist_id_raw else None + media = request.args.get("media") + media_type = media if media in ("image", "video") else None + sort = request.args.get("sort") + sort = sort if sort in ("newest", "oldest") else "newest" + return tag_ids, post_id, artist_id, media_type, sort + + @gallery_bp.route("/scroll", methods=["GET"]) async def scroll(): cursor = request.args.get("cursor") or None try: limit = int(request.args.get("limit", "50")) + tag_ids, post_id, artist_id, media_type, sort = _parse_filters() except ValueError: - return jsonify({"error": "limit must be an integer"}), 400 - tag_id_raw = request.args.get("tag_id") - tag_id = int(tag_id_raw) if tag_id_raw else None - post_id_raw = request.args.get("post_id") - post_id = int(post_id_raw) if post_id_raw else None - artist_id_raw = request.args.get("artist_id") - artist_id = int(artist_id_raw) if artist_id_raw else None + return jsonify({"error": "invalid filter or limit parameter"}), 400 async with get_session() as session: svc = GalleryService(session) try: page = await svc.scroll( - cursor=cursor, limit=limit, tag_id=tag_id, + cursor=cursor, limit=limit, tag_ids=tag_ids, post_id=post_id, artist_id=artist_id, + media_type=media_type, sort=sort, ) except ValueError as exc: return jsonify({"error": str(exc)}), 400 @@ -58,17 +74,16 @@ async def scroll(): @gallery_bp.route("/timeline", methods=["GET"]) async def timeline(): - tag_id_raw = request.args.get("tag_id") - tag_id = int(tag_id_raw) if tag_id_raw else None - post_id_raw = request.args.get("post_id") - post_id = int(post_id_raw) if post_id_raw else None - artist_id_raw = request.args.get("artist_id") - artist_id = int(artist_id_raw) if artist_id_raw else None + try: + tag_ids, post_id, artist_id, media_type, _sort = _parse_filters() + except ValueError: + return jsonify({"error": "invalid filter parameter"}), 400 async with get_session() as session: svc = GalleryService(session) try: buckets = await svc.timeline( - tag_id=tag_id, post_id=post_id, artist_id=artist_id + tag_ids=tag_ids, post_id=post_id, artist_id=artist_id, + media_type=media_type, ) except ValueError as exc: return jsonify({"error": str(exc)}), 400 @@ -82,20 +97,16 @@ async def jump(): try: year = int(request.args["year"]) month = int(request.args["month"]) + tag_ids, post_id, artist_id, media_type, sort = _parse_filters() except (KeyError, ValueError): return jsonify({"error": "year and month query params required"}), 400 - tag_id_raw = request.args.get("tag_id") - tag_id = int(tag_id_raw) if tag_id_raw else None - post_id_raw = request.args.get("post_id") - post_id = int(post_id_raw) if post_id_raw else None - artist_id_raw = request.args.get("artist_id") - artist_id = int(artist_id_raw) if artist_id_raw else None async with get_session() as session: svc = GalleryService(session) try: cursor = await svc.jump_cursor( - year=year, month=month, tag_id=tag_id, + year=year, month=month, tag_ids=tag_ids, post_id=post_id, artist_id=artist_id, + media_type=media_type, sort=sort, ) except ValueError as exc: return jsonify({"error": str(exc)}), 400 diff --git a/backend/app/services/gallery_service.py b/backend/app/services/gallery_service.py index d0eb09b..7690875 100644 --- a/backend/app/services/gallery_service.py +++ b/backend/app/services/gallery_service.py @@ -118,13 +118,43 @@ def thumbnail_url(thumbnail_path: str | None, sha256_hex: str, mime: str) -> str return f"/images/thumbs/{bucket}/{sha256_hex}{ext}" -def _require_single_filter(tag_id, post_id, artist_id) -> None: - if sum(x is not None for x in (tag_id, post_id, artist_id)) > 1: +def _require_single_filter(tag_ids, post_id, artist_id) -> None: + """post_id is the post-detail view — it can't combine with the + composable filters. tag_ids + artist_id (+ media_type) compose freely + (AND).""" + if post_id is not None and (tag_ids or artist_id is not None): raise ValueError( - "tag_id, post_id, artist_id are mutually exclusive" + "post_id cannot be combined with tag or artist filters" ) +def _apply_scope(stmt, *, tag_ids, post_id, artist_id, media_type): + """Apply the composable gallery filters to a statement already joined + to Post via _outer_join_primary_post. + + - tag_ids: image must carry ALL of them — one correlated EXISTS per tag + (AND), which avoids the row-multiplication a multi-join would cause. + - post_id / artist_id: provenance EXISTS (post_id is exclusive, guarded + by _require_single_filter). + - media_type: 'image' | 'video' narrows by mime prefix. + """ + for tid in tag_ids or []: + stmt = stmt.where( + exists().where( + image_tag.c.image_record_id == ImageRecord.id, + image_tag.c.tag_id == tid, + ) + ) + prov = _provenance_clause(post_id, artist_id) + if prov is not None: + stmt = stmt.where(prov) + if media_type == "image": + stmt = stmt.where(ImageRecord.mime.like("image/%")) + elif media_type == "video": + stmt = stmt.where(ImageRecord.mime.like("video/%")) + return stmt + + def _provenance_clause(post_id, artist_id): """Correlated EXISTS clause (NOT a join) so an image with multiple matching provenance rows is returned exactly once and the @@ -180,35 +210,43 @@ class GalleryService: self, cursor: str | None, limit: int = 50, - tag_id: int | None = None, + tag_ids: list[int] | None = None, post_id: int | None = None, artist_id: int | None = None, + media_type: str | None = None, + sort: str = "newest", ) -> GalleryPage: if limit < 1 or limit > 200: raise ValueError("limit must be between 1 and 200") - _require_single_filter(tag_id, post_id, artist_id) + _require_single_filter(tag_ids, post_id, artist_id) eff = _effective_date_col() stmt = select(ImageRecord, Post.post_date, eff.label("eff")) stmt = _outer_join_primary_post(stmt) - if tag_id is not None: - stmt = stmt.join(image_tag, image_tag.c.image_record_id == ImageRecord.id).where( - image_tag.c.tag_id == tag_id - ) - prov = _provenance_clause(post_id, artist_id) - if prov is not None: - stmt = stmt.where(prov) + stmt = _apply_scope( + stmt, tag_ids=tag_ids, post_id=post_id, + artist_id=artist_id, media_type=media_type, + ) + descending = sort != "oldest" if cursor: cur_ts, cur_id = decode_cursor(cursor) - stmt = stmt.where( - or_( - eff < cur_ts, - and_(eff == cur_ts, ImageRecord.id < cur_id), + # The cursor is just (last eff, last id); the request's sort + # decides which side of it the next page lies on. + if descending: + stmt = stmt.where( + or_(eff < cur_ts, and_(eff == cur_ts, ImageRecord.id < cur_id)) + ) + else: + stmt = stmt.where( + or_(eff > cur_ts, and_(eff == cur_ts, ImageRecord.id > cur_id)) ) - ) - stmt = stmt.order_by(eff.desc(), ImageRecord.id.desc()).limit(limit + 1) + if descending: + stmt = stmt.order_by(eff.desc(), ImageRecord.id.desc()) + else: + stmt = stmt.order_by(eff.asc(), ImageRecord.id.asc()) + stmt = stmt.limit(limit + 1) rows = (await self.session.execute(stmt)).all() next_cursor = None @@ -244,9 +282,10 @@ class GalleryService: async def timeline( self, - tag_id: int | None = None, + tag_ids: list[int] | None = None, post_id: int | None = None, artist_id: int | None = None, + media_type: str | None = None, ) -> list[TimelineBucket]: eff = _effective_date_col() year_col = func.date_part("year", eff).label("yr") @@ -255,25 +294,23 @@ class GalleryService: year_col, month_col, func.count(ImageRecord.id).label("cnt") ) stmt = _outer_join_primary_post(stmt) - _require_single_filter(tag_id, post_id, artist_id) - if tag_id is not None: - stmt = stmt.join(image_tag, image_tag.c.image_record_id == ImageRecord.id).where( - image_tag.c.tag_id == tag_id - ) - prov = _provenance_clause(post_id, artist_id) - if prov is not None: - stmt = stmt.where(prov) + _require_single_filter(tag_ids, post_id, artist_id) + stmt = _apply_scope( + stmt, tag_ids=tag_ids, post_id=post_id, + artist_id=artist_id, media_type=media_type, + ) stmt = stmt.group_by(year_col, month_col).order_by(year_col.desc(), month_col.desc()) rows = (await self.session.execute(stmt)).all() return [TimelineBucket(year=int(r.yr), month=int(r.mo), count=int(r.cnt)) for r in rows] async def jump_cursor( - self, year: int, month: int, tag_id: int | None = None, + self, year: int, month: int, tag_ids: list[int] | None = None, post_id: int | None = None, artist_id: int | None = None, + media_type: str | None = None, sort: str = "newest", ) -> str | None: - """Returns a cursor that, when passed to scroll(), positions at the - first image of the given year-month (by effective_date, not - created_at). None if the bucket is empty. + """Returns a cursor that, when passed to scroll() with the same sort, + positions at the first image of the given year-month. None if the + bucket is empty. """ from sqlalchemy import extract @@ -283,22 +320,24 @@ class GalleryService: extract("month", eff) == month, ) stmt = _outer_join_primary_post(stmt) - _require_single_filter(tag_id, post_id, artist_id) - if tag_id is not None: - stmt = stmt.join(image_tag, image_tag.c.image_record_id == ImageRecord.id).where( - image_tag.c.tag_id == tag_id - ) - prov = _provenance_clause(post_id, artist_id) - if prov is not None: - stmt = stmt.where(prov) - stmt = stmt.order_by(eff.desc(), ImageRecord.id.desc()).limit(1) - first = (await self.session.execute(stmt)).first() + _require_single_filter(tag_ids, post_id, artist_id) + stmt = _apply_scope( + stmt, tag_ids=tag_ids, post_id=post_id, + artist_id=artist_id, media_type=media_type, + ) + descending = sort != "oldest" + if descending: + stmt = stmt.order_by(eff.desc(), ImageRecord.id.desc()) + else: + stmt = stmt.order_by(eff.asc(), ImageRecord.id.asc()) + first = (await self.session.execute(stmt.limit(1))).first() if first is None: return None record, eff_date = first - # Cursor is exclusive; we encode a cursor with id+1 so the row itself - # is the first result in the next scroll(). - return encode_cursor(eff_date, record.id + 1) + # Cursor is exclusive; nudge the id one past the boundary row (in the + # scan direction) so the row itself is the first result of scroll(). + boundary = record.id + 1 if descending else record.id - 1 + return encode_cursor(eff_date, boundary) async def get_image_with_tags(self, image_id: int) -> dict | None: record = await self.session.get(ImageRecord, image_id) diff --git a/tests/test_api_gallery.py b/tests/test_api_gallery.py index b15a690..cf62d9b 100644 --- a/tests/test_api_gallery.py +++ b/tests/test_api_gallery.py @@ -59,3 +59,21 @@ async def test_jump_requires_year_month(client): async def test_image_detail_404_when_missing(client): resp = await client.get("/api/gallery/image/99999") assert resp.status_code == 404 + + +@pytest.mark.asyncio +async def test_scroll_sort_param_reverses(client, db): + await _seed(db, 3) + newest = await (await client.get("/api/gallery/scroll?limit=10&sort=newest")).get_json() + oldest = await (await client.get("/api/gallery/scroll?limit=10&sort=oldest")).get_json() + ids_new = [i["id"] for i in newest["images"]] + ids_old = [i["id"] for i in oldest["images"]] + assert ids_old == list(reversed(ids_new)) + + +@pytest.mark.asyncio +async def test_scroll_media_param(client, db): + await _seed(db, 2) # only image/jpeg seeded + resp = await client.get("/api/gallery/scroll?limit=10&media=video") + assert resp.status_code == 200 + assert (await resp.get_json())["images"] == [] diff --git a/tests/test_gallery_provenance_filter.py b/tests/test_gallery_provenance_filter.py index 1cda34a..c345808 100644 --- a/tests/test_gallery_provenance_filter.py +++ b/tests/test_gallery_provenance_filter.py @@ -81,10 +81,10 @@ async def test_scroll_artist_id_filter(db): @pytest.mark.asyncio -async def test_mutually_exclusive_filters_raise(db): +async def test_post_id_excludes_other_filters(db): svc = GalleryService(db) with pytest.raises(ValueError): - await svc.scroll(cursor=None, limit=10, tag_id=1, post_id=2) + await svc.scroll(cursor=None, limit=10, tag_ids=[1], post_id=2) @pytest.mark.asyncio diff --git a/tests/test_gallery_service.py b/tests/test_gallery_service.py index f1e553d..13405d8 100644 --- a/tests/test_gallery_service.py +++ b/tests/test_gallery_service.py @@ -87,7 +87,7 @@ async def test_scroll_with_tag_filter(db): ) svc = GalleryService(db) - page = await svc.scroll(cursor=None, limit=10, tag_id=tag.id) + page = await svc.scroll(cursor=None, limit=10, tag_ids=[tag.id]) assert len(page.images) == 1 assert page.images[0].id == images[0].id @@ -272,3 +272,53 @@ async def test_get_image_with_tags_includes_posted_at_when_present(db): assert payload["posted_at"] is not None # Image's own created_at is still surfaced separately. assert payload["created_at"] != payload["posted_at"] + + +@pytest.mark.asyncio +async def test_scroll_multi_tag_and(db): + images = await _seed_images(db, 5) + a = Tag(name="aa", kind=TagKind.general) + b = Tag(name="bb", kind=TagKind.general) + db.add_all([a, b]) + await db.flush() + # images[0] carries BOTH tags; images[1] carries only a. + await db.execute(image_tag.insert().values([ + {"image_record_id": images[0].id, "tag_id": a.id, "source": "manual"}, + {"image_record_id": images[0].id, "tag_id": b.id, "source": "manual"}, + {"image_record_id": images[1].id, "tag_id": a.id, "source": "manual"}, + ])) + svc = GalleryService(db) + both = await svc.scroll(cursor=None, limit=10, tag_ids=[a.id, b.id]) + assert [i.id for i in both.images] == [images[0].id] # AND: only the both-tagged + just_a = await svc.scroll(cursor=None, limit=10, tag_ids=[a.id]) + assert {i.id for i in just_a.images} == {images[0].id, images[1].id} + + +@pytest.mark.asyncio +async def test_scroll_media_filter(db): + imgs = await _seed_images(db, 2) # image/jpeg + vid = ImageRecord( + path="/images/test/v.mp4", sha256="v" * 64, size_bytes=1, + mime="video/mp4", width=1, height=1, + origin="imported_filesystem", integrity_status="unknown", + ) + vid.created_at = _now() + vid.effective_date = vid.created_at + db.add(vid) + await db.flush() + svc = GalleryService(db) + vids = await svc.scroll(cursor=None, limit=10, media_type="video") + assert [i.id for i in vids.images] == [vid.id] + pics = await svc.scroll(cursor=None, limit=10, media_type="image") + assert {i.id for i in pics.images} == {i.id for i in imgs} + + +@pytest.mark.asyncio +async def test_scroll_sort_oldest_reverses_order(db): + images = await _seed_images(db, 4) # images[0] newest ... images[3] oldest + svc = GalleryService(db) + newest = await svc.scroll(cursor=None, limit=10) + oldest = await svc.scroll(cursor=None, limit=10, sort="oldest") + newest_ids = [i.id for i in newest.images] + assert newest_ids[0] == images[0].id + assert [i.id for i in oldest.images] == list(reversed(newest_ids))