Repository navigation
fix: search every stored face in a photo, not just the first - #1590
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. WalkthroughFace search now retrieves image-face embeddings separately from image metadata. It groups qualifying matches by image, ranks each image by its best match, and supports faces without bounding boxes. Invalid metadata records are logged and skipped. ChangesPer-face face search
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant FaceSearch
participant db_get_all_image_face_embeddings
participant ImageLookup
participant ImageData
FaceSearch->>db_get_all_image_face_embeddings: Fetch per-face image embeddings
db_get_all_image_face_embeddings-->>FaceSearch: Return face records with optional bounding boxes
FaceSearch->>FaceSearch: Group qualifying matches and rank image IDs
FaceSearch->>ImageLookup: Fetch image records in match order
ImageLookup-->>FaceSearch: Return image metadata
FaceSearch->>ImageData: Parse metadata and validate image results
Suggested labels: Merge Risk: ⚪ Minimal · up to Face search now finds people who are not the first face in a group photo, returns each photo once, and handles faces with no bounding box. No merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The change searches additional faces within the existing photo library without adding access privileges or exposing a new library. Reviewed access and storage behavior remain unchanged, and no material security risk introduced by this PR was identified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation The change to
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks each face in view, Comment |
Fixes #1583
Face search only compared the first stored face of each photo, so anyone who wasn't face 0 in a group shot could never be found.
What was wrong
get_all_face_embeddingskeyed its result by image, so a photo with N faces collapsed to one row and the rest were dropped.TypeErrorinjson.loads, which onlyJSONDecodeErrorwas caught for, and that failed the whole search.What changed
db_get_all_image_face_embeddingsreturns one row per face.perform_face_searchkeeps the best-matching face per photo, frames that face, and ranks photos by it. This is the same approach the video half already uses.db_get_images_by_ids, which already de-duplicates tags.bboxesis optional), and a face with null or corrupt embeddings is skipped instead of failing the search.faces.pywere fixed so the mypy hook passes on the touched files.Testing
6 new tests in
tests/test_face_search.py; full backend suite passes (1353).I also rebuilt a fresh DB from 128 images (335 faces, 65 multi-face photos) and searched with each of the 296 faces in those photos. Every query found its own photo, framed on the right face. The photo with 32 faces and 5 tags now returns 5 tags instead of 160.
Searching with a group photo as the query (only its first face is used) is a separate issue and not touched here.
Summary by CodeRabbit
New Features
Bug Fixes