fix: batch list_people's per-person queries into one

backend/src/find_api/routers/people.py

@@ -3,10 +3,10 @@
 """
 
 from fastapi import APIRouter, Depends, HTTPException
-from sqlalchemy.orm import Session
 from sqlalchemy import func
+from sqlalchemy.orm import Session
 from pydantic import BaseModel, ConfigDict
-from typing import List, Optional
+from typing import Dict, List, Optional
 
 from find_api.core.database import get_db
 from find_api.core.config import settings
@@ -22,6 +22,9 @@
 logger = logging.getLogger(__name__)
 router = APIRouter()
 
+# Thumbnails shown per person group on the People page.
+SAMPLE_MEDIA_LIMIT = 4
+
 
 # ─── Pydantic schemas (what the API returns) ──────────────────────────────────
 
@@ -79,29 +82,70 @@ def list_people(
 
     persons = db.query(Person).order_by(Person.created_at.desc()).all()
 
-    result = []
-    for person in persons:
-        # Count how many faces belong to this person
-        count_query = (
-            db.query(func.count(Face.id))
-            .join(Media, Media.id == Face.media_id)
-            .filter(Face.person_id == person.id, Media.is_hidden.is_(False))
-        )
+    def visible_faces():
+        """Faces the caller may see, with the shared-mode scope applied.
+
+        Both aggregates below must filter identically — a count that counts
+        rows the samples exclude would report a person the page cannot show.
+        """
+        query = db.query(
+            Face.person_id.label("person_id"),
+            Face.media_id.label("media_id"),
+            Face.id.label("face_id"),
+        ).join(Media, Media.id == Face.media_id)
+        query = query.filter(Media.is_hidden.is_(False))
         if scope_user_id is not None:
-            count_query = count_query.filter(Media.uploader_user_id == scope_user_id)
-        face_count = count_query.scalar()
+            query = query.filter(Media.uploader_user_id == scope_user_id)
+        return query
+
+    # Two aggregate queries, not one row per face. Deriving these in Python
+    # would mean reading every face in the library into memory on every request
+    # — constant queries, but unbounded transfer, which is the same scaling
+    # cliff in a different place.
+    counted = visible_faces().subquery()
+    face_counts: Dict[int, int] = dict(
+        db.query(counted.c.person_id, func.count(counted.c.face_id))
+        .group_by(counted.c.person_id)
+        .all()
+    )
 
-        # Get up to 4 sample media IDs for thumbnail preview
-        sample_query = (
-            db.query(Face.media_id)
-            .join(Media, Media.id == Face.media_id)
-            .filter(Face.person_id == person.id)
-            .filter(Media.is_hidden.is_(False))
+    # Distinct media per person, then the first few of each, ranked in SQL so
+    # the database returns at most SAMPLE_MEDIA_LIMIT rows per person instead
+    # of every match. Ordering by media_id keeps the chosen thumbnail stable
+    # across requests; the previous `.distinct().limit(4)` had no ORDER BY and
+    # so could return a different sample each time.
+    distinct_pairs = (
+        visible_faces()
+        .with_entities(
+            Face.person_id.label("person_id"), Face.media_id.label("media_id")
         )
-        if scope_user_id is not None:
-            sample_query = sample_query.filter(Media.uploader_user_id == scope_user_id)
-        sample_faces = sample_query.distinct().limit(4).all()
-        sample_media_ids = [f.media_id for f in sample_faces]
+        .distinct()
+        .subquery()
+    )
+    ranked = db.query(
+        distinct_pairs.c.person_id,
+        distinct_pairs.c.media_id,
+        func.row_number()
+        .over(
+            partition_by=distinct_pairs.c.person_id,
+            order_by=distinct_pairs.c.media_id,
+        )
+        .label("rank"),
+    ).subquery()
+
+    sample_media_ids_by_person: Dict[int, List[int]] = {}
+    for person_id, media_id in (
+        db.query(ranked.c.person_id, ranked.c.media_id)
+        .filter(ranked.c.rank <= SAMPLE_MEDIA_LIMIT)
+        .order_by(ranked.c.person_id, ranked.c.media_id)
+        .all()
+    ):
+        sample_media_ids_by_person.setdefault(person_id, []).append(media_id)
+
+    result = []
+    for person in persons:
+        face_count = face_counts.get(person.id, 0)
+        sample_media_ids = sample_media_ids_by_person.get(person.id, [])
         # Skip groups with no visible faces. face_count/sample_media_ids are
         # already scoped to the caller in shared mode, so this also hides
         # person groups the user has none of their own media in.

backend/tests/test_people.py

@@ -108,3 +108,94 @@ def test_people_images_omit_hidden_media_faces(client, db):
     media_ids = [item["media_id"] for item in body["images"]]
 
     assert hidden_media.id not in media_ids
+
+
+def test_people_list_counts_and_samples_are_not_mixed_between_people(client, db):
+    person_a, media_1 = _seed_person_group(db, name="A")
+    person_b, media_3 = _seed_person_group(db, name="B")
+
+    # A second media for person A, plus a second face of A's in media_1 —
+    # face_count should total all faces (3), sample_media_ids only the
+    # 2 distinct media.
+    media_2 = Media(
+        file_hash=hashlib.sha256("a-second".encode()).hexdigest(),
+        minio_key="images/test/a-second.jpg",
+        filename="a-second.jpg",
+        content_type="image/jpeg",
+        file_size=1024,
+        status="indexed",
+        width=800,
+        height=600,
+        is_hidden=False,
+        vault_state="visible",
+        created_at=datetime.now(timezone.utc),
+    )
+    db.add(media_2)
+    db.commit()
+    db.refresh(media_2)
+
+    db.add_all(
+        [
+            Face(
+                media_id=media_1.id,
+                person_id=person_a.id,
+                bounding_box={"x1": 20, "y1": 20, "x2": 30, "y2": 30},
+                confidence=0.9,
+            ),
+            Face(
+                media_id=media_2.id,
+                person_id=person_a.id,
+                bounding_box={"x1": 0, "y1": 0, "x2": 10, "y2": 10},
+                confidence=0.9,
+            ),
+        ]
+    )
+    db.commit()
+
+    body = client.get("/api/people").json()
+    by_id = {item["id"]: item for item in body}
+
+    assert by_id[person_a.id]["face_count"] == 3
+    assert set(by_id[person_a.id]["sample_media_ids"]) == {media_1.id, media_2.id}
+    assert by_id[person_b.id]["face_count"] == 1
+    assert by_id[person_b.id]["sample_media_ids"] == [media_3.id]
+
+
+def test_people_list_query_count_does_not_grow_with_people(client, db):
+    """The point of the batching: cost must not scale with N.
+
+    Asserting the response shape alone would not have caught the original
+    1 + 2N pattern, since it returned correct data — it was only slow. This
+    fails if anyone reintroduces a per-person query.
+    """
+    from sqlalchemy import event
+
+    engine = db.get_bind()
+
+    def count_selects_for(batch: str, person_count: int) -> int:
+        # Names are hashed into file_hash, which is unique, so each batch needs
+        # its own prefix rather than restarting the counter.
+        for index in range(person_count):
+            _seed_person_group(db, name=f"{batch}{index}")
+
+        statements: list[str] = []
+
+        def record(conn, cursor, statement, params, context, executemany):
+            if statement.lstrip().upper().startswith("SELECT"):
+                statements.append(statement)
+
+        event.listen(engine, "before_cursor_execute", record)
+        try:
+            assert client.get("/api/people").status_code == 200
+        finally:
+            event.remove(engine, "before_cursor_execute", record)
+        return len(statements)
+
+    few = count_selects_for("Few", 3)
+    many = count_selects_for("Many", 20)
+
+    # Constant, not merely "fewer": 23 people must cost the same as 3.
+    assert few == many, (
+        f"query count grew with the number of people: {few} -> {many}; "
+        "a per-person query has been reintroduced"
+    )