fix: add metadata JSON parsing with error handling in get_cluster_images

backend/app/database/face_clusters.py

@@ -328,7 +328,13 @@ def db_get_images_by_cluster_id(
 
             import json
 
-            metadata_dict = json.loads(metadata) if metadata else None
+            if metadata:
+                try:
+                    metadata_dict = json.loads(metadata)
+                except json.JSONDecodeError:
+                    metadata_dict = None
+            else:
+                metadata_dict = None
             # Parse bbox JSON if it exists
             bbox = None
             if bbox_json:

backend/app/routes/face_clusters.py

@@ -174,12 +174,25 @@ def get_cluster_images(cluster_id: str):
         images_data = db_get_images_by_cluster_id(cluster_id)
 
         # Step 3: Convert to response models
+
+        # Step 3: Convert to response models
+        import json
+
+        def parse_metadata(metadata):
+            """Parse metadata if stored as JSON string."""
+            if isinstance(metadata, str):
+                try:
+                    return json.loads(metadata)
+                except json.JSONDecodeError:
+                    return None
+            return metadata
+
         images = [
             ImageInCluster(
                 id=img["image_id"],
                 path=img["image_path"],
                 thumbnailPath=img["thumbnail_path"],
-                metadata=img["metadata"],
+                metadata=parse_metadata(img["metadata"]),
                 face_id=img["face_id"],
                 confidence=img["confidence"],
                 bbox=img["bbox"],

backend/tests/test_face_clusters.py

@@ -370,18 +370,168 @@ def test_get_cluster_images_empty(self, mock_get_cluster, mock_get_images):
         assert data["data"]["total_images"] == 0
         assert data["data"]["images"] == []
 
+    # ============================================================================
+    # Metadata JSON Parsing Tests (Fix for Issue #705)
+    # ============================================================================
+
+    @patch("app.routes.face_clusters.db_get_images_by_cluster_id")
     @patch("app.routes.face_clusters.db_get_cluster_by_id")
-    def test_get_cluster_images_database_error(self, mock_get_cluster):
-        """Test handling database errors during image retrieval."""
-        cluster_id = "cluster_123"
-        mock_get_cluster.side_effect = Exception("Database connection failed")
+    def test_metadata_string_is_parsed(self, mock_get_cluster, mock_get_images):
+        """Metadata stored as JSON string should be parsed into a dict."""
+        mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"}
+        mock_get_images.return_value = [{
+            "image_id": "img_1",
+            "image_path": "/path/to/image1.jpg",
+            "thumbnail_path": "/path/to/thumb1.jpg",
+            "metadata": '{"camera": "Canon EOS", "location": "NYC"}',
+            "face_id": 101,
+            "confidence": 0.95,
+            "bbox": {"x": 100, "y": 200, "width": 150, "height": 200},
+        }]
 
-        response = client.get(f"/face_clusters/{cluster_id}/images")
+        response = client.get("/face_clusters/cluster_123/images")
+        data = response.json()
+        image = data["data"]["images"][0]
 
-        assert response.status_code == 500
+        assert isinstance(image["metadata"], dict)
+        assert image["metadata"]["camera"] == "Canon EOS"
+
+    @patch("app.routes.face_clusters.db_get_images_by_cluster_id")
+    @patch("app.routes.face_clusters.db_get_cluster_by_id")
+    def test_metadata_dict_passes_through(self, mock_get_cluster, mock_get_images):
+        """Metadata already as dict should stay unchanged."""
+        mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"}
+        mock_get_images.return_value = [{
+            "image_id": "img_1",
+            "image_path": "/path/to/image1.jpg",
+            "thumbnail_path": "/path/to/thumb1.jpg",
+            "metadata": {"camera": "Canon EOS"},
+            "face_id": 101,
+            "confidence": 0.95,
+            "bbox": {"x": 100, "y": 200, "width": 150, "height": 200},
+        }]
+
+        response = client.get("/face_clusters/cluster_123/images")
         data = response.json()
-        assert data["detail"]["success"] is False
-        assert data["detail"]["error"] == "Internal server error"
+        image = data["data"]["images"][0]
+
+        assert isinstance(image["metadata"], dict)
+        assert image["metadata"]["camera"] == "Canon EOS"
+
+    @patch("app.routes.face_clusters.db_get_images_by_cluster_id")
+    @patch("app.routes.face_clusters.db_get_cluster_by_id")
+    def test_invalid_metadata_returns_none(self, mock_get_cluster, mock_get_images):
+        """Broken JSON metadata should return None without crashing."""
+        mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"}
+        mock_get_images.return_value = [{
+            "image_id": "img_1",
+            "image_path": "/path/to/image1.jpg",
+            "thumbnail_path": "/path/to/thumb1.jpg",
+            "metadata": "{bad json!!!}",
+            "face_id": 101,
+            "confidence": 0.95,
+            "bbox": {"x": 100, "y": 200, "width": 150, "height": 200},
+        }]
+
+        response = client.get("/face_clusters/cluster_123/images")
+
+        assert response.status_code == 200
+        data = response.json()
+        assert data["data"]["images"][0]["metadata"] is None
+
+    # ============================================================================
+    # Metadata JSON Parsing Tests (Fix for Issue #705)
+    # ============================================================================
+
+    @patch("app.routes.face_clusters.db_get_images_by_cluster_id")
+    @patch("app.routes.face_clusters.db_get_cluster_by_id")
+    def test_metadata_string_is_parsed(self, mock_get_cluster, mock_get_images):
+        """Metadata stored as JSON string should be parsed into a dict."""
+        mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"}
+        mock_get_images.return_value = [{
+            "image_id": "img_1",
+            "image_path": "/path/to/image1.jpg",
+            "thumbnail_path": "/path/to/thumb1.jpg",
+            "metadata": '{"camera": "Canon EOS", "location": "NYC"}',
+            "face_id": 101,
+            "confidence": 0.95,
+            "bbox": {"x": 100, "y": 200, "width": 150, "height": 200},
+        }]
+
+        response = client.get("/face_clusters/cluster_123/images")
+        data = response.json()
+        image = data["data"]["images"][0]
+
+        assert isinstance(image["metadata"], dict)
+        assert image["metadata"]["camera"] == "Canon EOS"
+
+    @patch("app.routes.face_clusters.db_get_images_by_cluster_id")
+    @patch("app.routes.face_clusters.db_get_cluster_by_id")
+    def test_metadata_dict_passes_through(self, mock_get_cluster, mock_get_images):
+        """Metadata already as dict should stay unchanged."""
+        mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"}
+        mock_get_images.return_value = [{
+            "image_id": "img_1",
+            "image_path": "/path/to/image1.jpg",
+            "thumbnail_path": "/path/to/thumb1.jpg",
+            "metadata": {"camera": "Canon EOS"},
+            "face_id": 101,
+            "confidence": 0.95,
+            "bbox": {"x": 100, "y": 200, "width": 150, "height": 200},
+        }]
+
+        response = client.get("/face_clusters/cluster_123/images")
+        data = response.json()
+        image = data["data"]["images"][0]
+
+        assert isinstance(image["metadata"], dict)
+        assert image["metadata"]["camera"] == "Canon EOS"
+
+    @patch("app.routes.face_clusters.db_get_images_by_cluster_id")
+    @patch("app.routes.face_clusters.db_get_cluster_by_id")
+    def test_invalid_metadata_returns_none(self, mock_get_cluster, mock_get_images):
+        """Broken JSON metadata should return None without crashing."""
+        mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"}
+        mock_get_images.return_value = [{
+            "image_id": "img_1",
+            "image_path": "/path/to/image1.jpg",
+            "thumbnail_path": "/path/to/thumb1.jpg",
+            "metadata": "{bad json!!!}",
+            "face_id": 101,
+            "confidence": 0.95,
+            "bbox": {"x": 100, "y": 200, "width": 150, "height": 200},
+        }]
+
+        response = client.get("/face_clusters/cluster_123/images")
+
+        assert response.status_code == 200
+        data = response.json()
+        assert data["data"]["images"][0]["metadata"] is None
+
+    @patch("app.routes.face_clusters.db_get_images_by_cluster_id")
+    @patch("app.routes.face_clusters.db_get_cluster_by_id")
+    def test_null_metadata_handled(self, mock_get_cluster, mock_get_images):
+        """None metadata from database should be handled safely."""
+        mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"}
+        mock_get_images.return_value = [{
+            "image_id": "img_1",
+            "image_path": "/path/to/image1.jpg",
+            "thumbnail_path": "/path/to/thumb1.jpg",
+            "metadata": None,
+            "face_id": 101,
+            "confidence": 0.95,
+            "bbox": {"x": 100, "y": 200, "width": 150, "height": 200},
+        }]
+
+        response = client.get("/face_clusters/cluster_123/images")
+
+        assert response.status_code == 200
+        assert response.json()["data"]["images"][0]["metadata"] is None
+
+        response = client.get("/face_clusters/cluster_123/images")
+
+        assert response.status_code == 200
+        assert response.json()["data"]["images"][0]["metadata"] is None
 
     # ============================================================================
     # Additional Edge Case Tests