insight-chat: code-review polish on get_faces_in_photo

- Drop redundant `use anyhow::Context` inside has_any_faces (already
  imported at the module level).
- Drop dead `.unwrap_or("?")` on bound faces — the vec is filtered to
  is_some() so the fallback can never fire.
- Reorder the face_dao constructor param + initializer to match the
  struct declaration (between tag_dao and knowledge_dao). Update both
  state.rs call sites and populate_knowledge.rs to match.
- Hold face_dao lock once across the library-resolver loop instead of
  reacquiring per iteration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/ai/insight_generator.rs

@@ -119,8 +119,8 @@ impl InsightGenerator {
         location_dao: Arc<Mutex<Box<dyn LocationHistoryDao>>>,
         search_dao: Arc<Mutex<Box<dyn SearchHistoryDao>>>,
         tag_dao: Arc<Mutex<Box<dyn TagDao>>>,
-        knowledge_dao: Arc<Mutex<Box<dyn KnowledgeDao>>>,
         face_dao: Arc<Mutex<Box<dyn crate::faces::FaceDao>>>,
+        knowledge_dao: Arc<Mutex<Box<dyn KnowledgeDao>>>,
         libraries: Vec<Library>,
     ) -> Self {
         Self {
@@ -135,8 +135,8 @@ impl InsightGenerator {
             location_dao,
             search_dao,
             tag_dao,
-            knowledge_dao,
             face_dao,
+            knowledge_dao,
             libraries,
         }
     }
@@ -2180,15 +2180,16 @@ Return ONLY the summary, nothing else."#,
         log::info!("tool_get_faces_in_photo: file_path='{}'", file_path);
 
         // Resolve content_hash from any library that has this rel_path.
-        // Walk libraries in their declared order and take the first hit.
+        // Hold the FaceDao lock once across all libraries — resolve_content_hash
-        let mut content_hash: Option<String> = None;
+        // is synchronous and there's no await in the loop body.
-        for lib in &self.libraries {
+        let content_hash: Option<String> = {
             let mut dao = self.face_dao.lock().expect("Unable to lock FaceDao");
-            if let Ok(Some(h)) = dao.resolve_content_hash(cx, lib.id, &file_path) {
+            self.libraries.iter().find_map(|lib| {
-                content_hash = Some(h);
+                dao.resolve_content_hash(cx, lib.id, &file_path)
-                break;
+                    .ok()
-            }
+                    .flatten()
-        }
+            })
+        };
         let Some(content_hash) = content_hash else {
             return "No content_hash found for that file path (the photo may not be indexed yet, \
                     or the path doesn't match any library)."
@@ -2215,9 +2216,11 @@ Return ONLY the summary, nothing else."#,
 
         let mut out = format!("Found {} face(s) in this photo:\n", faces.len());
         for f in &bound {
+            // Invariant: `bound` is filtered on `person_name.is_some()` above.
+            let name = f.person_name.as_deref().expect("bound face must have a name");
             out.push_str(&format!(
                 "- {} (confidence {:.2}, bbox x={:.2} y={:.2} w={:.2} h={:.2}, source: {})\n",
-                f.person_name.as_deref().unwrap_or("?"),
+                name,
                 f.confidence,
                 f.bbox_x,
                 f.bbox_y,

src/bin/populate_knowledge.rs

@@ -201,8 +201,8 @@ async fn main() -> anyhow::Result<()> {
         location_dao,
         search_dao,
         tag_dao,
-        knowledge_dao,
         face_dao,
+        knowledge_dao,
         all_libs.clone(),
     );
 

src/faces.rs

@@ -1437,7 +1437,6 @@ impl FaceDao for SqliteFaceDao {
     }
 
     fn has_any_faces(&mut self, ctx: &opentelemetry::Context) -> anyhow::Result<bool> {
-        use anyhow::Context;
         let mut conn = self.connection.lock().expect("face dao lock");
         trace_db_call(ctx, "query", "has_any_faces", |_span| {
             face_detections::table

src/state.rs

@@ -234,8 +234,8 @@ impl Default for AppState {
             location_dao.clone(),
             search_dao.clone(),
             tag_dao.clone(),
-            knowledge_dao,
             face_dao.clone(),
+            knowledge_dao,
             libraries_vec.clone(),
         );
 
@@ -376,8 +376,8 @@ impl AppState {
             location_dao.clone(),
             search_dao.clone(),
             tag_dao.clone(),
-            knowledge_dao,
             face_dao.clone(),
+            knowledge_dao,
             vec![test_lib],
         );