From 5a2f406429e2fe6c471e16394ec53ba20393c95b Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 30 Apr 2026 01:01:07 +0000 Subject: [PATCH] faces: bbox edits survive when re-detection finds no face MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving a tagged bbox off-center (to fine-tune position, or onto a back-of-head the operator already manually tagged) made update_face_handler 422 because the re-embed step ran detection on the new crop and found nothing. Frontend's catch then reverted the optimistic update — visible as the bbox snapping back the moment the user released their drag. The re-embed is a soft contract: a fresh ArcFace vector is preferable, but the operator's bbox edit is sacred. Now: - empty faces[] → keep old embedding, apply the bbox, log info - permanent embed error → keep old embedding, apply the bbox, log info - bad-bytes embedding → keep old embedding, apply the bbox, log warn - transient failure (cuda_oom, engine unavailable) still 503s so the operator can retry — those are recoverable and we don't want to silently drift cluster math on retries that succeed later Cost: a slightly stale embedding for the row, which marginally affects clustering / auto-bind cosine for files re-detected against this person. Accepted because dropping the user's manual drag every time the new crop happens to lose detection is a much worse UX — especially for the force-create rows (back of head, profile) where re-detection will *always* fail. --- src/faces.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/faces.rs b/src/faces.rs index 5cacf14..6bcce3a 100644 --- a/src/faces.rs +++ b/src/faces.rs @@ -2234,6 +2234,17 @@ async fn update_face_handler( orientation: None, model_version: Some(current.model_version.clone()), }; + // Soft contract on the re-embed: we'd LIKE a fresh ArcFace + // vector for the new crop, but the operator's bbox edit is + // sacred. If detection finds no face in the new region (they + // dragged the box slightly off-center, or moved it to a back- + // of-head shot they've already manually tagged), or returns a + // bad embedding, we keep the old embedding and apply the bbox + // anyway. Cost: stale embedding for that row, which slightly + // pollutes clustering for files re-detected against this + // person — accepted because dropping the user's drag is a + // worse UX. Transient failures (cuda_oom, engine unavailable) + // still 503 so the operator can retry once Apollo recovers. match face_client.embed(crop_bytes, meta).await { Ok(resp) => { if let Some(face) = resp.faces.first() { @@ -2241,20 +2252,23 @@ async fn update_face_handler( Ok(b) => new_embedding = Some(b), Err(e) => { warn!( - "PATCH /image/faces/{}: bad embedding from face service: {:?}", + "PATCH /image/faces/{}: bad embedding from face service ({:?}); keeping old embedding, bbox still applied", id, e ); - return HttpResponse::BadGateway() - .body("invalid embedding from face service"); } } } else { - return HttpResponse::UnprocessableEntity() - .body("no face in new bbox"); + info!( + "PATCH /image/faces/{}: no face detected in new bbox — keeping old embedding, bbox still applied", + id + ); } } Err(FaceDetectError::Permanent(e)) => { - return HttpResponse::UnprocessableEntity().body(format!("{}", e)); + info!( + "PATCH /image/faces/{}: embed permanent error ({}); keeping old embedding, bbox still applied", + id, e + ); } Err(FaceDetectError::Transient(e)) => { return HttpResponse::ServiceUnavailable().body(format!("{}", e));