Skip to content

fix: search every stored face in a photo, not just the first - #1590

Merged
rohan-pandeyy merged 2 commits into
AOSSIE-Org:devfrom
rohan-pandeyy:fix/face-search-multi-face
Oct 6, 2026
Merged

rohan-pandeyy merged 2 commits into
AOSSIE-Org:devfrom
rohan-pandeyy:fix/face-search-multi-face

Conversation

@rohan-pandeyy

@rohan-pandeyy rohan-pandeyy commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

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_embeddings keyed its result by image, so a photo with N faces collapsed to one row and the rest were dropped.
  • Tags were appended once per face/tag join row, so 3 faces and 3 tags came back as 9 tag entries.
  • A face with a null bbox raised TypeError in json.loads, which only JSONDecodeError was caught for, and that failed the whole search.

What changed

  • db_get_all_image_face_embeddings returns one row per face.
  • perform_face_search keeps the best-matching face per photo, frames that face, and ranks photos by it. This is the same approach the video half already uses.
  • Matched images are loaded by id through db_get_images_by_ids, which already de-duplicates tags.
  • A face with a null bbox still matches (bboxes is optional), and a face with null or corrupt embeddings is skipped instead of failing the search.
  • A few existing type errors in faces.py were 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

    • Face search now finds matches across multiple faces in a photo and shows each matching photo only once, using its strongest match to determine ranking.
    • Photos can appear in results even when a matching face has no bounding box.
  • Bug Fixes

    • Face search skips malformed or unavailable photo records instead of letting them disrupt results.
    • Search results retain photo tags and display the matching face’s bounding box when available.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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
  • Configuration used: Repository: AOSSIE-Org/PictoPy/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 07b5ebf7-f69a-4333-94fe-7288bf6f5fcb
📥 Commits

Reviewing files that changed from the base of the PR and between 8645431 and 496f072.

📒 Files selected for processing (3)
  • backend/app/database/faces.py
  • backend/app/utils/faceSearch.py
  • backend/tests/test_face_search.py

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


Walkthrough

Face 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.

Changes

Per-face face search

Layer / File(s) Summary
Face retrieval records and database handling
backend/app/database/faces.py
Typed face records support image- and video-face retrieval. Image-face retrieval returns one record per face with an optional bounding box and skips malformed JSON data. Database insertion and update paths include added type and cursor checks.
Match faces and construct image results
backend/app/utils/faceSearch.py, backend/tests/test_face_search.py
Face search groups qualifying face matches by image and ranks images by their best match. Tests cover matches on later faces, optional bounding boxes, tags, result ordering, and missing image records.

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
Loading

Suggested labels: Python

Merge Risk: ⚪ Minimal · up to 496f0

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 Review

Security architecture risk: ⚪ Minimal · up to 496f0

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The reviewed change affects matching and returned metadata within the same configured photo library. More people in group photos become searchable, but the reviewed flow introduces no additional data store, tenant domain, or privileged operation.

Trust Boundaries and Controls

  • observed — The route accepts a local image path or size-limited base64 media and invokes face search. No per-user authorization is visible in the route or its application mounting. That behavior predates this PR; the default localhost binding is a network constraint, not per-user authorization.

Resilience and Maintainability Implications

  • observed — Face insertion retains exclusive image-or-frame ownership validation and cascading parent references. Batch insertion remains sequential with per-face commits, and cluster updates retain caller-owned versus locally owned transaction handling. The typing casts and cursor assertion do not change valid SQLite transitions.
🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The change to db_insert_face_embeddings rejects calls where both image_id and frame_id are set or both are missing. Issue #1583 concerns retrieval of existing image-face records and does not req… Remove the new exactly-one-ID rejection from db_insert_face_embeddings, or provide a linked coding requirement that requires this insertion validation change.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the primary change: face search now checks every stored face in a photo.
Linked Issues check ✅ Passed Issue #1583 requires solo-portrait searches to find a person stored as a non-first face in a group photo, without duplicate image tags or failure on a null bounding box. `db_get_all_image_face_embeddi…
Full details: Out of Scope Changes check

Explanation

The change to db_insert_face_embeddings rejects calls where both image_id and frame_id are set or both are missing. Issue #1583 concerns retrieval of existing image-face records and does not require changing face-insertion validation. This adds unrelated behavior to a shared database API.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

A rabbit checks each face in view,
And finds the match the first one knew.
The group photo joins the line,
Its best match sets the place in time.
No box? The result still can show.
The rabbit hops through rows below.

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added GSoC 2026 bug Something isn't working labels Oct 5, 2026
@rohan-pandeyy
rohan-pandeyy merged commit c8f0c3f into AOSSIE-Org:dev Oct 6, 2026
14 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working GSoC 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: face search misses everyone but the first face in a group photo

1 participant