Merge pull request 'Person merge API + Modify People UI' (#95) from feature/person-merge into master

ROADMAP.md

@@ -111,7 +111,7 @@ Living plan for product quality, auth/email reliability, and automation.
 - [x] **Phase 4 — multi-ref matching** — up to 3 trusted refs per person; best (min) distance wins; rolling-mean calibration fitter; status in `docs/FACE_ACCURACY_STATUS.md`
 - [x] **Immich precision gates** — max recognition distance, next-best person margin, detection floor 0.55, auto-accept distance cap; admin Chabad Blue theme (shell + login)
 - [ ] **Phase 5 — harder reject of junk detections (tiny/blur/pose)** — partial via detection floor; blur/pose still open
-- [ ] **Phase 6 — multi-embedding / person merge / cluster-name Identify**
+- [ ] **Phase 6 — multi-embedding / cluster-name Identify** (person merge shipped in #95)
 
 ## Later
 

admin-frontend/src/api/people.ts

@@ -46,6 +46,14 @@ export interface PersonUpdateRequest {
   phone?: string | null
 }
 
+export interface PersonMergeResponse {
+  person: Person
+  merged_ids: number[]
+  faces_moved: number
+  videos_moved: number
+  encodings_rebuilt: number
+}
+
 export const peopleApi = {
   list: async (lastName?: string): Promise<PeopleListResponse> => {
     const params = lastName ? { last_name: lastName } : {}
@@ -98,6 +106,13 @@ export const peopleApi = {
     )
     return res.data
   },
+  merge: async (keepId: number, mergeIds: number[]): Promise<PersonMergeResponse> => {
+    const res = await apiClient.post<PersonMergeResponse>(
+      `/api/v1/people/${keepId}/merge`,
+      { merge_ids: mergeIds },
+    )
+    return res.data
+  },
   delete: async (personId: number): Promise<void> => {
     await apiClient.delete(`/api/v1/people/${personId}`)
   },

admin-frontend/src/pages/Modify.tsx

@@ -192,6 +192,8 @@ export default function Modify() {
   const [selectedVideos, setSelectedVideos] = useState<Set<number>>(new Set())
   const [editDialogPerson, setEditDialogPerson] = useState<PersonWithFaces | null>(null)
   const [deleteDialogPerson, setDeleteDialogPerson] = useState<PersonWithFaces | null>(null)
+  const [mergeSourceIds, setMergeSourceIds] = useState<Set<number>>(new Set())
+  const [mergeConfirmOpen, setMergeConfirmOpen] = useState(false)
   const [facesExpanded, setFacesExpanded] = useState(true)
   const [videosExpanded, setVideosExpanded] = useState(false)
   const [peoplePanelWidth, setPeoplePanelWidth] = useState(450) // Default width in pixels (will be set to 50% on mount)
@@ -502,10 +504,50 @@ export default function Modify() {
   const handlePersonClick = (person: PersonWithFaces) => {
     setSelectedPersonId(person.id)
     setSelectedPersonName(formatPersonName(person))
+    setMergeSourceIds((prev) => {
+      const next = new Set(prev)
+      next.delete(person.id)
+      return next
+    })
     loadPersonFaces(person.id)
     loadPersonVideos(person.id)
   }
 
+  const toggleMergeSource = (personId: number) => {
+    setMergeSourceIds((prev) => {
+      const next = new Set(prev)
+      if (next.has(personId)) {
+        next.delete(personId)
+      } else {
+        next.add(personId)
+      }
+      return next
+    })
+  }
+
+  const handleMergePeople = async () => {
+    if (!selectedPersonId || mergeSourceIds.size === 0) return
+
+    try {
+      setBusy(true)
+      setError(null)
+      const result = await peopleApi.merge(selectedPersonId, Array.from(mergeSourceIds))
+      setMergeConfirmOpen(false)
+      setMergeSourceIds(new Set())
+      await loadPeople()
+      await loadPersonFaces(selectedPersonId)
+      await loadPersonVideos(selectedPersonId)
+      setSuccess(
+        `Merged ${result.merged_ids.length} person(s): ${result.faces_moved} face(s), ${result.videos_moved} video(s)`,
+      )
+      setTimeout(() => setSuccess(null), 4000)
+    } catch (err: any) {
+      setError(err.response?.data?.detail || err.message || 'Failed to merge people')
+    } finally {
+      setBusy(false)
+    }
+  }
+
   const handleEditPerson = (person: PersonWithFaces) => {
     setEditDialogPerson(person)
   }
@@ -537,6 +579,13 @@ export default function Modify() {
         setSelectedPersonName('')
         setFaces([])
         setVideos([])
+        setMergeSourceIds(new Set())
+      } else {
+        setMergeSourceIds((prev) => {
+          const next = new Set(prev)
+          next.delete(deleteDialogPerson.id)
+          return next
+        })
       }
       
       // Reload people list
@@ -772,6 +821,20 @@ export default function Modify() {
                 </button>
               </div>
               <p className="text-xs text-gray-500">Search by First, Middle, Last, or Maiden Name</p>
+              {selectedPersonId && mergeSourceIds.size > 0 && (
+                <button
+                  onClick={() => setMergeConfirmOpen(true)}
+                  disabled={busy}
+                  className="mt-2 w-full px-3 py-2 text-sm bg-indigo-600 text-white rounded-md hover:bg-indigo-700 disabled:opacity-50"
+                >
+                  Merge {mergeSourceIds.size} into selected
+                </button>
+              )}
+              {selectedPersonId && mergeSourceIds.size === 0 && (
+                <p className="mt-2 text-xs text-gray-500">
+                  Check boxes next to other people, then merge them into the selected person.
+                </p>
+              )}
             </div>
 
             {/* People list */}
@@ -785,13 +848,29 @@ export default function Modify() {
                   {people.map((person) => {
                     const isSelected = selectedPersonId === person.id
                     const name = formatPersonName(person)
+                    const isMergeSource = mergeSourceIds.has(person.id)
                     return (
                       <div
                         key={person.id}
                         className={`flex items-center gap-2 p-2 rounded hover:bg-gray-50 cursor-pointer ${
                           isSelected ? 'bg-blue-50 font-semibold' : ''
-                        }`}
+                        } ${isMergeSource ? 'ring-1 ring-indigo-300' : ''}`}
                       >
+                        <input
+                          type="checkbox"
+                          checked={isMergeSource}
+                          disabled={!selectedPersonId || isSelected || busy}
+                          onChange={() => toggleMergeSource(person.id)}
+                          onClick={(e) => e.stopPropagation()}
+                          title={
+                            isSelected
+                              ? 'Selected person is the merge target'
+                              : selectedPersonId
+                                ? 'Mark to merge into selected'
+                                : 'Select a keep person first'
+                          }
+                          className="rounded border-gray-300"
+                        />
                         <button
                           onClick={(e) => {
                             e.stopPropagation()
@@ -1113,6 +1192,40 @@ export default function Modify() {
         </div>
       )}
 
+      {/* Merge confirmation dialog */}
+      {mergeConfirmOpen && selectedPersonId && (
+        <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50">
+          <div className="bg-white rounded-lg shadow-xl w-full max-w-md p-6">
+            <h2 className="text-xl font-bold mb-4 text-indigo-700">Merge People</h2>
+            <p className="mb-4">
+              Merge {mergeSourceIds.size} person(s) into{' '}
+              <strong>{selectedPersonName || 'the selected person'}</strong>?
+            </p>
+            <p className="mb-4 text-sm text-gray-600">
+              Faces, video links, and match history move to the keep person. Source people are
+              deleted. Duplicate video links on the same file are dropped.
+            </p>
+            <p className="mb-6 text-sm font-semibold text-red-600">This action cannot be undone.</p>
+            <div className="flex justify-end gap-3">
+              <button
+                onClick={() => setMergeConfirmOpen(false)}
+                disabled={busy}
+                className="px-4 py-2 text-gray-700 bg-gray-100 rounded-md hover:bg-gray-200 disabled:opacity-50"
+              >
+                Cancel
+              </button>
+              <button
+                onClick={handleMergePeople}
+                disabled={busy}
+                className="px-4 py-2 bg-indigo-600 text-white rounded-md hover:bg-indigo-700 disabled:opacity-50 disabled:cursor-not-allowed"
+              >
+                {busy ? 'Merging...' : 'Merge'}
+              </button>
+            </div>
+          </div>
+        </div>
+      )}
+
       {/* Unmatch confirmation dialog */}
       {unmatchConfirmDialog && (
         <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50">

backend/api/people.py

@@ -10,13 +10,15 @@ from sqlalchemy import func, or_
 from sqlalchemy.orm import Session
 
 from backend.api.auth import get_current_user_with_id
-from backend.db.models import Face, Person, PersonEncoding, Photo, PhotoPersonLinkage
+from backend.db.models import Face, MatchDecision, Person, PersonEncoding, Photo, PhotoPersonLinkage
 from backend.db.session import get_db
 from backend.schemas.faces import AcceptMatchesRequest, IdentifyFaceResponse, PersonFaceItem, PersonFacesResponse
 from backend.schemas.people import (
     PeopleListResponse,
     PeopleWithFacesListResponse,
     PersonCreateRequest,
+    PersonMergeRequest,
+    PersonMergeResponse,
     PersonResponse,
     PersonUpdateRequest,
     PersonWithFacesResponse,
@@ -350,6 +352,131 @@ def accept_matches(
     )
 
 
+@router.post("/{person_id}/merge", response_model=PersonMergeResponse)
+def merge_people(
+    person_id: int,
+    request: PersonMergeRequest,
+    db: Session = Depends(get_db),
+) -> PersonMergeResponse:
+    """Merge other people into this person (keep).
+
+    Reassigns faces, person encodings, video linkages, and match decisions
+    from each source onto keep, then deletes the source people.
+    """
+    keep = db.query(Person).filter(Person.id == person_id).first()
+    if not keep:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail=f"Person {person_id} not found",
+        )
+
+    merge_ids = list(dict.fromkeys(request.merge_ids))  # preserve order, dedupe
+    if person_id in merge_ids:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="Cannot merge a person into themselves",
+        )
+    if not merge_ids:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="merge_ids must not be empty",
+        )
+
+    sources = db.query(Person).filter(Person.id.in_(merge_ids)).all()
+    found_ids = {p.id for p in sources}
+    missing = [mid for mid in merge_ids if mid not in found_ids]
+    if missing:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail=f"Person(s) not found: {missing}",
+        )
+
+    try:
+        faces_moved = 0
+        videos_moved = 0
+
+        keep_video_photo_ids = {
+            row.photo_id
+            for row in db.query(PhotoPersonLinkage.photo_id)
+            .filter(PhotoPersonLinkage.person_id == person_id)
+            .all()
+        }
+
+        for source_id in merge_ids:
+            moved = (
+                db.query(Face)
+                .filter(Face.person_id == source_id)
+                .update({"person_id": person_id}, synchronize_session=False)
+            )
+            faces_moved += moved or 0
+
+            # Drop source encodings; rebuild keep refs after all moves
+            db.query(PersonEncoding).filter(
+                PersonEncoding.person_id == source_id
+            ).delete(synchronize_session=False)
+
+            source_links = (
+                db.query(PhotoPersonLinkage)
+                .filter(PhotoPersonLinkage.person_id == source_id)
+                .all()
+            )
+            for link in source_links:
+                if link.photo_id in keep_video_photo_ids:
+                    db.delete(link)
+                else:
+                    link.person_id = person_id
+                    keep_video_photo_ids.add(link.photo_id)
+                    videos_moved += 1
+                    db.add(link)
+
+            db.query(MatchDecision).filter(
+                MatchDecision.person_id == source_id
+            ).update({"person_id": person_id}, synchronize_session=False)
+
+            source_person = next(p for p in sources if p.id == source_id)
+            db.delete(source_person)
+
+        # Rebuild trusted encodings for keep (same quality gate as Auto-Match accept)
+        db.query(PersonEncoding).filter(
+            PersonEncoding.person_id == person_id
+        ).delete(synchronize_session=False)
+        current_faces = (
+            db.query(Face)
+            .filter(Face.person_id == person_id)
+            .filter(Face.quality_score >= 0.3)
+            .all()
+        )
+        for face in current_faces:
+            db.add(
+                PersonEncoding(
+                    person_id=person_id,
+                    face_id=face.id,
+                    encoding=face.encoding,
+                    quality_score=face.quality_score,
+                    detector_backend=face.detector_backend,
+                    model_name=face.model_name,
+                )
+            )
+
+        db.commit()
+        db.refresh(keep)
+        return PersonMergeResponse(
+            person=PersonResponse.model_validate(keep),
+            merged_ids=merge_ids,
+            faces_moved=faces_moved,
+            videos_moved=videos_moved,
+            encodings_rebuilt=len(current_faces),
+        )
+    except HTTPException:
+        raise
+    except Exception as e:
+        db.rollback()
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail=f"Failed to merge people: {str(e)}",
+        )
+
+
 @router.delete("/{person_id}")
 def delete_person(person_id: int, db: Session = Depends(get_db)) -> Response:
     """Delete a person and all their linkages.

backend/schemas/people.py

@@ -86,4 +86,24 @@ class PeopleWithFacesListResponse(BaseModel):
     total: int
 
 
+class PersonMergeRequest(BaseModel):
+    """Merge other people into the keep person (Immich-style)."""
+
+    model_config = ConfigDict(protected_namespaces=())
+
+    merge_ids: list[int] = Field(..., min_length=1, description="Person IDs to merge into keep")
+
+
+class PersonMergeResponse(BaseModel):
+    """Result of merging people into keep."""
+
+    model_config = ConfigDict(protected_namespaces=())
+
+    person: PersonResponse
+    merged_ids: list[int]
+    faces_moved: int
+    videos_moved: int
+    encodings_rebuilt: int
+
+
 

docs/FACE_ACCURACY_STATUS.md

@@ -32,7 +32,7 @@ plus Immich-inspired precision gates.
 | Under-merge + margin | **Done** |
 | Min detection score | **Done** (new Process jobs) |
 | Cluster-first / name cluster | Later |
-| Person merge UI | Later |
+| Person merge UI | Done (`POST /people/{id}/merge` + Modify People checkboxes) |
 | InsightFace buffalo | Skip — ArcFace OK |
 
 Immich source is public (`github.com/immich-app/immich`); no local clone required. You already run Immich at `photos.levkin.ca`.

tests/test_api_people.py

@@ -263,3 +263,150 @@ class TestPeopleFaces:
         
         assert response.status_code == 404
 
+
+class TestPeopleMerge:
+    """Test Immich-style person merge."""
+
+    def test_merge_people_moves_faces_and_deletes_source(
+        self,
+        test_client: TestClient,
+        test_db_session: "Session",
+        test_person: "Person",
+        test_photo,
+    ):
+        """Faces move to keep; source person is deleted; encodings rebuild."""
+        from datetime import datetime
+
+        from backend.db.models import Face, Person, PersonEncoding
+        import numpy as np
+
+        source = Person(
+            first_name="Jane",
+            last_name="Duplicate",
+            created_date=datetime.utcnow(),
+        )
+        test_db_session.add(source)
+        test_db_session.commit()
+        test_db_session.refresh(source)
+
+        encoding = np.random.rand(128).astype(np.float32).tobytes()
+        source_face = Face(
+            photo_id=test_photo.id,
+            person_id=source.id,
+            encoding=encoding,
+            location='{"x": 10, "y": 10, "w": 50, "h": 50}',
+            quality_score=0.85,
+            face_confidence=0.9,
+            detector_backend="retinaface",
+            model_name="ArcFace",
+        )
+        test_db_session.add(source_face)
+        test_db_session.flush()
+        test_db_session.add(
+            PersonEncoding(
+                person_id=source.id,
+                face_id=source_face.id,
+                encoding=encoding,
+                quality_score=0.85,
+                detector_backend="retinaface",
+                model_name="ArcFace",
+            )
+        )
+        test_db_session.commit()
+        source_id = source.id
+        source_face_id = source_face.id
+
+        response = test_client.post(
+            f"/api/v1/people/{test_person.id}/merge",
+            json={"merge_ids": [source_id]},
+        )
+        assert response.status_code == 200, response.text
+        data = response.json()
+        assert data["merged_ids"] == [source_id]
+        assert data["faces_moved"] >= 1
+        assert data["person"]["id"] == test_person.id
+
+        test_db_session.expire_all()
+        assert test_db_session.get(Person, source_id) is None
+        moved = (
+            test_db_session.query(Face)
+            .filter(Face.id == source_face_id)
+            .one()
+        )
+        assert moved.person_id == test_person.id
+        enc_count = (
+            test_db_session.query(PersonEncoding)
+            .filter(PersonEncoding.person_id == test_person.id)
+            .count()
+        )
+        assert enc_count >= 1
+
+    def test_merge_rejects_self(
+        self,
+        test_client: TestClient,
+        test_person: "Person",
+    ):
+        response = test_client.post(
+            f"/api/v1/people/{test_person.id}/merge",
+            json={"merge_ids": [test_person.id]},
+        )
+        assert response.status_code == 400
+
+    def test_merge_source_not_found(
+        self,
+        test_client: TestClient,
+        test_person: "Person",
+    ):
+        response = test_client.post(
+            f"/api/v1/people/{test_person.id}/merge",
+            json={"merge_ids": [99999]},
+        )
+        assert response.status_code == 404
+
+    def test_merge_video_unique_conflict_drops_duplicate(
+        self,
+        test_client: TestClient,
+        test_db_session: "Session",
+        test_person: "Person",
+        test_photo,
+    ):
+        """If keep and source both link the same video photo, drop source link."""
+        from datetime import datetime
+
+        from backend.db.models import Person, PhotoPersonLinkage
+
+        source = Person(
+            first_name="Vid",
+            last_name="Dup",
+            created_date=datetime.utcnow(),
+        )
+        test_db_session.add(source)
+        test_db_session.commit()
+        test_db_session.refresh(source)
+
+        test_db_session.add(
+            PhotoPersonLinkage(photo_id=test_photo.id, person_id=test_person.id)
+        )
+        test_db_session.add(
+            PhotoPersonLinkage(photo_id=test_photo.id, person_id=source.id)
+        )
+        test_db_session.commit()
+        source_id = source.id
+
+        response = test_client.post(
+            f"/api/v1/people/{test_person.id}/merge",
+            json={"merge_ids": [source_id]},
+        )
+        assert response.status_code == 200, response.text
+        data = response.json()
+        assert data["videos_moved"] == 0
+
+        links = (
+            test_db_session.query(PhotoPersonLinkage)
+            .filter(PhotoPersonLinkage.photo_id == test_photo.id)
+            .all()
+        )
+        assert len(links) == 1
+        assert links[0].person_id == test_person.id
+        assert test_db_session.get(Person, source_id) is None
+