faces: re-embed on bbox edit instead of leaving the embedding stale

Phase 2 stored the new bbox on PATCH /image/faces/{id} but logged
"embedding now stale (Phase 3 will re-embed)" and moved on. That left
the embedding column pointing at the *old* face area while the bbox
described a new one — auto-bind cosine similarity and the cluster
suggester would silently rank the row as "the same face it was before
the edit" forever after, even though the geometry no longer matched.

Now: when the PATCH includes a bbox, the handler:
  1. Looks up the row to find its photo (library_id + rel_path).
  2. Crops the new bbox region with the same crop_image_to_bbox helper
     manual-create uses (10% pad on each side so the detector has
     ear/jaw context).
  3. POSTs the crop to face_client.embed for a fresh ArcFace vector.
  4. Stores both the new bbox AND the new embedding in one
     update_face transaction.

Errors map cleanly:
  - face_client disabled → 503 (bbox edit needs Apollo).
  - decode failure / no face in crop → 422.
  - Apollo CUDA OOM / unavailable → 503 transient.
  - Underlying row missing → 404.

About 100-500ms per edit on CPU, dominated by Apollo's inference call.
Acceptable for a manual operator action; the alternative (stale
embedding) silently broke the rest of the face stack.

Prerequisite for the upcoming carousel-side draw/resize bbox UI —
without re-embed, every operator-driven bbox tweak would corrode the
clustering/auto-bind quality. ApiPatchFaceBody on Apollo's side
already passes bbox through verbatim, so no Apollo change needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cameron Cordes
2026-04-29 23:10:25 +00:00
parent 7303fb8aa3
commit 43cb60d3ad

View File

@@ -2107,6 +2107,8 @@ async fn update_face_handler<D: FaceDao>(
request: HttpRequest,
path: web::Path<i32>,
body: web::Json<UpdateFaceReq>,
app_state: web::Data<AppState>,
face_client: web::Data<FaceClient>,
face_dao: web::Data<Mutex<D>>,
) -> impl Responder {
let context = extract_context_from_request(&request);
@@ -2121,21 +2123,92 @@ async fn update_face_handler<D: FaceDao>(
};
let bbox_patch = body.bbox.as_ref().map(|b| (b.x, b.y, b.w, b.h));
// bbox change → embedding becomes stale. Phase 2 only stores the new
// bbox; re-embed is a Phase 3 concern (it requires reading the image
// off disk and going back through face_client.embed). For now log a
// warning so we can spot orphan-embedding rows.
if bbox_patch.is_some() {
warn!(
"PATCH /image/faces/{}: bbox updated; embedding now stale (Phase 3 will re-embed)",
id
);
// Bbox change → re-embed. The embedding is what auto-bind and the
// cluster suggester key on, so leaving it stale would silently
// corrupt every downstream similarity match. We crop the new bbox,
// pass it through face_client.embed, and store the fresh vector.
// Net cost: one Apollo round-trip per bbox edit (~100-500ms on
// CPU); acceptable for a manual operator action.
let mut new_embedding: Option<Vec<u8>> = None;
if let Some((bx, by, bw, bh)) = bbox_patch {
if !face_client.is_enabled() {
return HttpResponse::ServiceUnavailable()
.body("face client disabled — bbox edit requires Apollo");
}
// Look up the current row so we know which photo to crop.
let current = {
let mut dao = face_dao.lock().expect("face dao lock");
match dao.get_face(&span_context, id) {
Ok(Some(r)) => r,
Ok(None) => return HttpResponse::NotFound().finish(),
Err(e) => return HttpResponse::InternalServerError().body(e.to_string()),
}
};
let library = match app_state.library_by_id(current.library_id) {
Some(l) => l.clone(),
None => {
return HttpResponse::InternalServerError().body(format!(
"face row references unknown library_id {}",
current.library_id
));
}
};
let abs_path = library.resolve(&current.rel_path);
let crop_bytes = match crop_image_to_bbox(&abs_path, bx, by, bw, bh) {
Ok(b) => b,
Err(e) => {
warn!(
"PATCH /image/faces/{}: crop failed for {:?}: {:?}",
id, abs_path, e
);
return HttpResponse::BadRequest()
.body(format!("cannot crop new bbox: {}", e));
}
};
let meta = DetectMeta {
content_hash: current.content_hash.clone(),
library_id: current.library_id,
rel_path: current.rel_path.clone(),
orientation: None,
model_version: Some(current.model_version.clone()),
};
match face_client.embed(crop_bytes, meta).await {
Ok(resp) => {
if let Some(face) = resp.faces.first() {
match face.decode_embedding() {
Ok(b) => new_embedding = Some(b),
Err(e) => {
warn!(
"PATCH /image/faces/{}: bad embedding from face service: {:?}",
id, e
);
return HttpResponse::BadGateway()
.body("invalid embedding from face service");
}
}
} else {
return HttpResponse::UnprocessableEntity()
.body("no face in new bbox");
}
}
Err(FaceDetectError::Permanent(e)) => {
return HttpResponse::UnprocessableEntity().body(format!("{}", e));
}
Err(FaceDetectError::Transient(e)) => {
return HttpResponse::ServiceUnavailable().body(format!("{}", e));
}
Err(FaceDetectError::Disabled) => {
return HttpResponse::ServiceUnavailable()
.body("face client disabled mid-flight");
}
}
}
let mut dao = face_dao.lock().expect("face dao lock");
dao.update_face(&span_context, id, person_patch, bbox_patch, None)
.map(|row| HttpResponse::Ok().json(row))
.into_http_internal_err()
match dao.update_face(&span_context, id, person_patch, bbox_patch, new_embedding) {
Ok(row) => HttpResponse::Ok().json(row),
Err(e) => HttpResponse::InternalServerError().body(e.to_string()),
}
}
async fn delete_face_handler<D: FaceDao>(