Enhance Adaptive Face Clustering and Improve Face Collection Showcase

[rohan-pandeyy]

Fixes #1271

Summary

An investigation into two long-standing face clustering symptoms — faces never being detected, and detected people missing many of their photos — traced them to specific, measurable defects in the detection pipeline and the clustering logic. This PR fixes the detection losses, adds a physical-consistency constraint to clustering, and improves how clusters are ordered in the People view.

Measured on a real 205-image library, stored faces went from 161 to 230 (+43%), and person-tagged images with zero stored faces dropped from 17.9% to 6.3% (the remainder are genuine no-face cases: masked/rear views and sub-threshold tiny faces).

Detection fixes

• Letterbox YOLO preprocessing — Input was previously resized directly to 640×640, distorting the aspect ratio and shrinking small faces below detectability in large photos. Inputs are now letterboxed (aspect-preserving resize + gray padding), with detections unpadded and unscaled back to the original image before being clipped to image bounds. On one test group photo, this alone increased detected faces from 2 to 4.
• Removed the group-photo cap — Face detection was previously skipped entirely for images where the object classifier found 7+ people, permanently excluding exactly the group shots where face tagging matters most.
• Retuned the quality gate — The blur threshold (Laplacian variance 80) rejected clean studio portraits, which typically measure 40–75 because smooth lighting and bokeh produce little high-frequency texture. The default threshold has been lowered to 20, and the minimum face area reduced from 1600 → 1000 px² to recover group-photo faces that fell just below the old cutoff. Both thresholds remain environment-variable overridable.

Clustering fixes

• Cannot-link constraint — Faces that co-occur in the same photo must represent different people, but degraded embeddings from tiny faces could previously cluster together, causing a single image to appear up to four times within one cluster. After DBSCAN and merge, each cluster now retains at most one face per image (keeping the face closest to the centroid). Incremental assignment applies the same constraint, and cluster image listings now deduplicate per image as an additional display-level safeguard.
• Empty-library crash — Fixed cluster_util_cluster_all_face_embeddings returning a bare list instead of the expected tuple when no faces exist, preventing a caller crash.

People view ordering

Clusters were previously ordered by UUID, making the People view effectively random. They are now ranked by avg_confidence × log2(1 + face_count), causing frequently photographed people (typically the device owner) to surface first while logarithmic damping prevents large low-quality clusters from outranking smaller, cleaner ones.

Testing

• All backend tests pass (57 after merging main), including the existing clustering regression suite.
• End-to-end verification on a real image library confirmed that previously missed portraits and group photos now store faces, same-image duplicates within clusters dropped from 6 affected clusters to 0, and cluster ordering is monotonic by prominence score.

Summary by CodeRabbit

• New Features

  • Improved face clustering to prevent multiple faces from the same image being grouped into one person.
  • Added smarter cluster ranking based on face confidence and cluster size.
  • Face results now include associated image information.

• Bug Fixes

  • Improved image detection accuracy by preserving aspect ratios during processing.
  • Selected the highest-confidence face when returning one face per image.
  • Expanded face detection for images containing multiple detected objects.
  • Improved clustering of lower-quality and smaller faces.

[coderabbitai]

Walkthrough

Clustering defaults and YOLO preprocessing were revised, face queries now preserve image identifiers, clusters receive confidence-based ranking, and full or incremental clustering prevents multiple faces from one image entering the same cluster.

Changes

Face detection and clustering refinements

Layer / File(s)	Summary
Detection preprocessing and thresholds backend/app/config/settings.py, backend/app/models/YOLO.py, backend/app/utils/images.py	Clustering quality defaults changed, YOLO inputs now use letterboxing with inverse box scaling, and face detection runs whenever class 0 is present.
Face metadata and cluster retrieval backend/app/database/faces.py, backend/app/database/face_clusters.py	Face query results include image_id, cluster-image pairs are available, clusters use confidence-weighted ranking, and cluster image results retain one highest-confidence face per image.
One-face-per-image clustering backend/app/utils/face_clusters.py	Full reclustering removes same-image duplicates using centroid similarity, while incremental assignment skips occupied cluster-image pairs.
Database reset failure output backend/scripts/reset_database.py	The reset script’s stderr failure message was reformatted without changing its exit behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant FaceClustering
  participant Database
  participant DBSCAN
  participant ConstraintEnforcer
  FaceClustering->>Database: load embeddings and image identifiers
  FaceClustering->>DBSCAN: cluster validated embeddings
  DBSCAN-->>FaceClustering: return cluster assignments
  FaceClustering->>ConstraintEnforcer: enforce one face per image
  ConstraintEnforcer-->>FaceClustering: return filtered assignments

Loading

Possibly related PRs

• AOSSIE-Org/PictoPy#1290: Introduces the clustering quality settings adjusted here.
• AOSSIE-Org/PictoPy#1330: Also changes clustering behavior in face_clusters.py.
• AOSSIE-Org/PictoPy#1392: Also modifies grouped cluster retrieval logic.

Suggested labels: Python

Suggested reviewers: rahulharpal1603

Poem

I’m a rabbit who clusters with care,
One face per photo, neatly there.
Boxes keep their shape in flight,
Sharp faces rise by confidence bright.
Hop, hop—the portraits align!

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR improves clustering robustness, but it does not implement #1271's adaptive eps or skipped-face reporting requirements.	Add adaptive eps estimation, surface skipped-face counts in the response/UI, and centralize clustering parameters as specified.
Out of Scope Changes check	⚠️ Warning	The reset_database.py print formatting change is unrelated to face clustering and appears out of scope.	Move the database reset script cleanup to a separate housekeeping PR unless it is needed for this clustering fix.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title captures the main changes to adaptive face clustering and the People-view showcase, even if it is slightly broad.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (4)

backend/scripts/reset_database.py (1)

50-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep the error message within PEP 8 line length.

Line 51 is 84 characters including indentation and may trigger E501 under strict linting. Split the adjacent string literals:

         print(
-            "Error: Database reset failed because some files could not be deleted.",
+            "Error: Database reset failed because some files "
+            "could not be deleted.",
             file=sys.stderr,
         )

As per path instructions, Python files must be checked for PEP 8 violations.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/scripts/reset_database.py` around lines 50 - 53, Update the error
message in the database reset failure print statement to split the adjacent
string literals so the source line stays within PEP 8’s maximum length, while
preserving the existing message and stderr output.

Source: Path instructions

backend/app/database/face_clusters.py (1)

343-347: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Move the import statement outside the loop.

Importing json inside the loop causes unnecessary overhead on every iteration. Please move it to the top of the file or outside the loop.

♻️ Proposed fix

-            seen_image_ids.add(image_id)
-
-            import json
-
+            seen_image_ids.add(image_id)
+
             metadata_dict = json.loads(metadata) if metadata else None

(Ensure import json is added at the top of the file if not already present).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/database/face_clusters.py` around lines 343 - 347, Move the json
import out of the loop containing the seen_image_ids check, placing it with the
module-level imports at the top of the file; remove the inner-loop import while
leaving the surrounding face-cluster processing unchanged.

backend/app/database/faces.py (2)

222-228: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Update return type hints to include ImageId.

The return dictionaries for these functions were updated to include the image_id field, but the return type hints were not updated to reflect this addition. As per the path instructions, ensure proper use of type hints.

• backend/app/database/faces.py#L222-L228: Add ImageId to the Union for db_get_faces_unassigned_clusters (e.g., Union[FaceId, ImageId, FaceEmbedding]).
• backend/app/database/faces.py#L253-L255: Add ImageId to the Union for db_get_all_faces_with_cluster_names (e.g., Union[FaceId, ImageId, FaceEmbedding, Optional[str]]).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/database/faces.py` around lines 222 - 228, Update the return type
hints for db_get_faces_unassigned_clusters and
db_get_all_faces_with_cluster_names to include ImageId in their Union types,
preserving the existing FaceId, FaceEmbedding, and Optional[str] members as
applicable. Apply this in backend/app/database/faces.py at lines 222-228 and
253-255.

Source: Path instructions

---

360-362: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Specify the types for the returned set.

As per the path instructions, ensure proper use of type hints. The set return type can be made more specific to indicate its contents, such as Set[Tuple[str, str]] (or set[tuple[str, str]] in Python 3.9+).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/database/faces.py` around lines 360 - 362, Update the return
annotation of db_get_cluster_image_pairs to specify a set of two-string tuples,
using the project’s supported typing syntax (Set[Tuple[str, str]] or
set[tuple[str, str]]), while preserving the function’s existing behavior.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@backend/app/database/face_clusters.py`:
- Around line 343-347: Move the json import out of the loop containing the
seen_image_ids check, placing it with the module-level imports at the top of the
file; remove the inner-loop import while leaving the surrounding face-cluster
processing unchanged.

In `@backend/app/database/faces.py`:
- Around line 222-228: Update the return type hints for
db_get_faces_unassigned_clusters and db_get_all_faces_with_cluster_names to
include ImageId in their Union types, preserving the existing FaceId,
FaceEmbedding, and Optional[str] members as applicable. Apply this in
backend/app/database/faces.py at lines 222-228 and 253-255.
- Around line 360-362: Update the return annotation of
db_get_cluster_image_pairs to specify a set of two-string tuples, using the
project’s supported typing syntax (Set[Tuple[str, str]] or set[tuple[str,
str]]), while preserving the function’s existing behavior.

In `@backend/scripts/reset_database.py`:
- Around line 50-53: Update the error message in the database reset failure
print statement to split the adjacent string literals so the source line stays
within PEP 8’s maximum length, while preserving the existing message and stderr
output.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 77ec36f8-bd88-4c3f-a7f3-25cd0a163b3f

📥 Commits

Reviewing files that changed from the base of the PR and between f465155 and e921ea2.

📒 Files selected for processing (7)

• backend/app/config/settings.py
• backend/app/database/face_clusters.py
• backend/app/database/faces.py
• backend/app/models/YOLO.py
• backend/app/utils/face_clusters.py
• backend/app/utils/images.py
• backend/scripts/reset_database.py