From 0eaf27d2d3805faec44294f296455809af64952c Mon Sep 17 00:00:00 2001
From: Cameron Cordes <cameronc.dev@gmail.com>
Date: Wed, 29 Apr 2026 23:41:52 +0000
Subject: [PATCH] =?UTF-8?q?faces:=20cover=20hydrate=5Fface=5Fwith=5Fperson?=
 =?UTF-8?q?=20=E2=80=94=20assigned=20+=20unassigned=20branches?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two unit tests pinning the response shape that PATCH/POST /image/faces
relies on. They use the existing in-memory SQLite harness and exercise
the helper directly:

- assigned: person_name resolves through the persons join and bbox /
  source / person_id round-trip cleanly.
- unassigned: person_name is None (not stale, not omitted), person_id
  is None.

These would have caught the prior regression — when the handlers
returned a bare FaceDetectionRow, person_name was structurally absent
from the response shape. A test that asserts person_name is populated
when person_id is set forces the join (or any equivalent) to exist.

A dangling-person_id case isn't covered: the FK on face_detections
makes that state structurally impossible at rest (ON DELETE SET NULL
zeroes the column when a person is removed), so there's nothing to
defend against.
---
 src/faces.rs | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/src/faces.rs b/src/faces.rs
index 66ddfca..0c6c007 100644
--- a/src/faces.rs
+++ b/src/faces.rs
@@ -3153,4 +3153,80 @@ mod tests {
         assert!(img.height() <= 100);
         assert!(img.width() > 0 && img.height() > 0);
     }
+
+    // ── hydrate_face_with_person — PATCH/POST /image/faces response shape ──
+
+    fn seed_library_and_face(dao: &mut SqliteFaceDao, person_id: Option<i32>) -> FaceDetectionRow {
+        diesel::sql_query(
+            "INSERT OR IGNORE INTO libraries (id, name, root_path, created_at) \
+             VALUES (1, 'main', '/tmp', 0)",
+        )
+        .execute(dao.connection.lock().unwrap().deref_mut())
+        .expect("seed libraries");
+        dao.store_detection(
+            &ctx(),
+            InsertFaceDetectionInput {
+                library_id: 1,
+                content_hash: "h-hydrate".into(),
+                rel_path: "p.jpg".into(),
+                bbox: Some((0.1, 0.2, 0.3, 0.4)),
+                embedding: Some(vec![0u8; 2048]),
+                confidence: Some(0.9),
+                source: "manual".into(),
+                person_id,
+                status: "detected".into(),
+                model_version: "buffalo_l".into(),
+            },
+        )
+        .unwrap()
+    }
+
+    #[test]
+    fn hydrate_face_carries_person_name_when_assigned() {
+        // Regression guard for the bug where PATCH /image/faces/{id}
+        // returned a bare FaceDetectionRow (no person_name), causing
+        // the carousel overlay's optimistic replace to drop the VFD
+        // label off the bbox after every save. The handler hydrates
+        // via this helper; if anyone refactors the helper to skip the
+        // persons join, this test fails.
+        let mut dao = fresh_dao();
+        let p = dao
+            .create_person(
+                &ctx(),
+                &CreatePersonReq {
+                    name: "Alice".into(),
+                    notes: None,
+                    entity_id: None,
+                    is_ignored: false,
+                },
+                false,
+            )
+            .unwrap();
+        let row = seed_library_and_face(&mut dao, Some(p.id));
+        let joined =
+            hydrate_face_with_person(&mut dao, &ctx(), row).expect("hydrate assigned");
+        assert_eq!(joined.person_id, Some(p.id));
+        assert_eq!(joined.person_name.as_deref(), Some("Alice"));
+        // Bbox + confidence + source must round-trip — these are what
+        // the optimistic-replace also keys on.
+        assert!((joined.bbox_x - 0.1).abs() < 1e-6);
+        assert!((joined.bbox_y - 0.2).abs() < 1e-6);
+        assert!((joined.bbox_w - 0.3).abs() < 1e-6);
+        assert!((joined.bbox_h - 0.4).abs() < 1e-6);
+        assert_eq!(joined.source, "manual");
+    }
+
+    #[test]
+    fn hydrate_face_leaves_person_name_null_when_unassigned() {
+        // Mirror branch: an unassigned face must hydrate cleanly with
+        // person_name = None, not a stale value left over from a
+        // previously-assigned row's serialization.
+        let mut dao = fresh_dao();
+        let row = seed_library_and_face(&mut dao, None);
+        let joined =
+            hydrate_face_with_person(&mut dao, &ctx(), row).expect("hydrate unassigned");
+        assert!(joined.person_id.is_none());
+        assert!(joined.person_name.is_none());
+    }
+
 }