diff --git a/ROADMAP.md b/ROADMAP.md index d41777b..f20d9ce 100644 --- a/ROADMAP.md +++ b/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 diff --git a/admin-frontend/src/api/people.ts b/admin-frontend/src/api/people.ts index 7611fc4..d8afee8 100644 --- a/admin-frontend/src/api/people.ts +++ b/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 => { const params = lastName ? { last_name: lastName } : {} @@ -98,6 +106,13 @@ export const peopleApi = { ) return res.data }, + merge: async (keepId: number, mergeIds: number[]): Promise => { + const res = await apiClient.post( + `/api/v1/people/${keepId}/merge`, + { merge_ids: mergeIds }, + ) + return res.data + }, delete: async (personId: number): Promise => { await apiClient.delete(`/api/v1/people/${personId}`) }, diff --git a/admin-frontend/src/pages/Modify.tsx b/admin-frontend/src/pages/Modify.tsx index 586fbb1..4349da3 100644 --- a/admin-frontend/src/pages/Modify.tsx +++ b/admin-frontend/src/pages/Modify.tsx @@ -192,6 +192,8 @@ export default function Modify() { const [selectedVideos, setSelectedVideos] = useState>(new Set()) const [editDialogPerson, setEditDialogPerson] = useState(null) const [deleteDialogPerson, setDeleteDialogPerson] = useState(null) + const [mergeSourceIds, setMergeSourceIds] = useState>(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() {

Search by First, Middle, Last, or Maiden Name

+ {selectedPersonId && mergeSourceIds.size > 0 && ( + + )} + {selectedPersonId && mergeSourceIds.size === 0 && ( +

+ Check boxes next to other people, then merge them into the selected person. +

+ )} {/* 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 (
+ 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" + />
)} + {/* Merge confirmation dialog */} + {mergeConfirmOpen && selectedPersonId && ( +
+
+

Merge People

+

+ Merge {mergeSourceIds.size} person(s) into{' '} + {selectedPersonName || 'the selected person'}? +

+

+ Faces, video links, and match history move to the keep person. Source people are + deleted. Duplicate video links on the same file are dropped. +

+

This action cannot be undone.

+
+ + +
+
+
+ )} + {/* Unmatch confirmation dialog */} {unmatchConfirmDialog && (
diff --git a/backend/api/people.py b/backend/api/people.py index 2ce0d85..72905d9 100644 --- a/backend/api/people.py +++ b/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. diff --git a/backend/schemas/people.py b/backend/schemas/people.py index 8eb7d2a..94b4b13 100644 --- a/backend/schemas/people.py +++ b/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 + + diff --git a/docs/FACE_ACCURACY_STATUS.md b/docs/FACE_ACCURACY_STATUS.md index 8eac7f3..0e5750b 100644 --- a/docs/FACE_ACCURACY_STATUS.md +++ b/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`. diff --git a/tests/test_api_people.py b/tests/test_api_people.py index aafb0e9..a4c83c8 100644 --- a/tests/test_api_people.py +++ b/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 +