Merge branch 'master' into chore/test-upgrade-ci-postgres-to-v18

Author: sentry-junior[bot]

src/sentry/preprod/api/endpoints/snapshots/preprod_artifact_snapshot.py

@@ -43,13 +43,14 @@
     BuildDetailsVcsInfo,
 )
 from sentry.preprod.api.models.public.snapshots import (
+    SnapshotApproverResponseDict,
     SnapshotCreateResponseDict,
     SnapshotDetailsResponseDict,
+    SnapshotImageResponseDict,
+    VcsInfoResponseDict,
 )
 from sentry.preprod.api.models.snapshots.project_preprod_snapshot_models import (
     SnapshotApprover,
-    SnapshotDetailsApiResponse,
-    SnapshotImageResponse,
 )
 from sentry.preprod.api.models.snapshots.snapshot_status import (
     SnapshotStatusInput,
@@ -66,12 +67,8 @@
     MISSING_BASE_GRACE_PERIOD_SECONDS,
     SNAPSHOT_ARCHIVE_MANIFEST_FILENAME,
 )
-from sentry.preprod.snapshots.manifest import (
-    ComparisonManifest,
-    ImageMetadata,
-    SnapshotManifest,
-    image_metadata_extras,
-)
+from sentry.preprod.snapshots.image_serialization import build_head_image_dict
+from sentry.preprod.snapshots.manifest import SnapshotManifest
 from sentry.preprod.snapshots.models import (
     PreprodSnapshotComparison,
     PreprodSnapshotMetrics,
@@ -130,28 +127,6 @@ def _strip_to_compact(img: dict[str, Any]) -> dict[str, Any]:
     return {k: img[k] for k in _COMPACT_FIELDS if k in img}
 
 
-def build_snapshot_image_response(
-    image_file_name: str,
-    metadata: ImageMetadata,
-    global_diff_threshold: float | None,
-) -> SnapshotImageResponse:
-    return SnapshotImageResponse(
-        **image_metadata_extras(metadata, exclude={"key", "image_file_name"}),
-        key=metadata.content_hash,
-        display_name=metadata.display_name,
-        image_file_name=image_file_name,
-        group=metadata.group,
-        width=metadata.width,
-        height=metadata.height,
-        diff_threshold=metadata.diff_threshold
-        if metadata.diff_threshold is not None
-        else global_diff_threshold,
-        description=metadata.description,
-        tags=metadata.tags,
-        canvas_theme=metadata.canvas_theme,
-    )
-
-
 MAX_SNAPSHOT_REQUEST_BODY_SIZE = 256 * 1024 * 1024
 
 
@@ -387,8 +362,10 @@ def get(
             with start_span(
                 op="preprod.snapshot.parse_manifest", name="parse_head_manifest"
             ) as span:
-                manifest = SnapshotManifest(**orjson.loads(raw_manifest))
-                set_span_data(span, "image_count", len(manifest.images))
+                head_manifest = orjson.loads(raw_manifest)
+                head_images: dict[str, Any] = head_manifest.get("images", {})
+                head_diff_threshold = head_manifest.get("diff_threshold")
+                set_span_data(span, "image_count", len(head_images))
         except Exception:
             logger.exception(
                 "Failed to retrieve snapshot manifest",
@@ -415,8 +392,8 @@ def get(
         else:
             vcs_info = BuildDetailsVcsInfo()
 
-        comparison_manifest: ComparisonManifest | None = None
-        base_manifest: SnapshotManifest | None = None
+        comparison_manifest: dict[str, Any] | None = None
+        base_manifest: dict[str, Any] | None = None
         all_comparisons = list(
             PreprodSnapshotComparison.objects.select_related("base_snapshot_metrics")
             .filter(head_snapshot_metrics=snapshot_metrics)
@@ -441,10 +418,12 @@ def get(
                     with start_span(
                         op="preprod.snapshot.parse_manifest", name="parse_comparison_manifest"
                     ) as span:
-                        comparison_manifest = ComparisonManifest(
-                            **orjson.loads(raw_comparison_manifest)
+                        comparison_manifest = orjson.loads(raw_comparison_manifest)
+                        if "base_artifact_id" not in comparison_manifest:
+                            raise ValueError("comparison manifest missing base_artifact_id")
+                        set_span_data(
+                            span, "image_count", len(comparison_manifest.get("images", {}))
                         )
-                        set_span_data(span, "image_count", len(comparison_manifest.images))
                 except Exception:
                     comparison_manifest = None
                     logger.exception(
@@ -466,8 +445,8 @@ def get(
                     with start_span(
                         op="preprod.snapshot.parse_manifest", name="parse_base_manifest"
                     ) as span:
-                        base_manifest = SnapshotManifest(**orjson.loads(raw_base_manifest))
-                        set_span_data(span, "image_count", len(base_manifest.images))
+                        base_manifest = orjson.loads(raw_base_manifest)
+                        set_span_data(span, "image_count", len(base_manifest.get("images", {})))
                 except Exception:
                     logger.exception(
                         "Failed to fetch base manifest",
@@ -509,26 +488,29 @@ def get(
         with start_span(
             op="preprod.snapshot.serialize_images", name="serialize_head_images"
         ) as span:
-            set_span_data(span, "image_count", len(manifest.images))
-            image_list = [
-                build_snapshot_image_response(key, metadata, manifest.diff_threshold)
-                for key, metadata in sorted(manifest.images.items())
+            set_span_data(span, "image_count", len(head_images))
+            image_list: list[SnapshotImageResponseDict] = [
+                build_head_image_dict(key, metadata, head_diff_threshold)
+                for key, metadata in sorted(head_images.items())
             ]
 
-        images_by_file_name: dict[str, SnapshotImageResponse] = {
-            img.image_file_name: img for img in image_list
+        images_by_file_name: dict[str, SnapshotImageResponseDict] = {
+            img["image_file_name"]: img for img in image_list
         }
 
         base_artifact_id: str | None = None
 
         if comparison_manifest is not None:
-            base_artifact_id = str(comparison_manifest.base_artifact_id)
+            base_artifact_id = str(comparison_manifest["base_artifact_id"])
+            comparison_images = comparison_manifest.get("images", {})
             with start_span(
                 op="preprod.snapshot.categorize_comparison", name="categorize_comparison_images"
             ) as span:
-                set_span_data(span, "image_count", len(comparison_manifest.images))
+                set_span_data(span, "image_count", len(comparison_images))
                 categorized = categorize_comparison_images(
-                    comparison_manifest, images_by_file_name, base_manifest
+                    comparison_images,
+                    images_by_file_name,
+                    base_manifest.get("images", {}) if base_manifest else None,
                 )
         else:
             if comparison is not None:
@@ -628,51 +610,55 @@ def get(
             op="preprod.snapshot.serialize_response", name="serialize_response_body"
         ) as span:
             set_span_data(span, "image_count", len(image_list))
-            response_data = SnapshotDetailsApiResponse(
-                head_artifact_id=str(artifact.id),
-                base_artifact_id=base_artifact_id,
-                project_id=str(artifact.project_id),
-                comparison_type=comparison_type,
-                state=artifact.state,
-                vcs_info=vcs_info,
-                app_id=artifact.app_id,
-                is_selective=snapshot_metrics.is_selective,
-                images=image_list if comparison_type != "diff" else [],
-                image_count=snapshot_metrics.image_count,
-                changed=categorized.changed,
-                changed_count=len(categorized.changed),
-                added=categorized.added,
-                added_count=len(categorized.added),
-                removed=categorized.removed,
-                removed_count=len(categorized.removed),
-                renamed=categorized.renamed,
-                renamed_count=len(categorized.renamed),
-                unchanged=categorized.unchanged,
-                unchanged_count=len(categorized.unchanged),
-                errored=categorized.errored,
-                errored_count=len(categorized.errored),
-                skipped=categorized.skipped,
-                skipped_count=len(categorized.skipped),
-                diff_threshold=manifest.diff_threshold,
-                comparison_state=derived_status.comparison_state,
-                approval_status=derived_status.approval_status,
-                comparison_error_message=derived_status.comparison_error_message,
-                approvers=approver_list if approved else [],
-            ).dict()
+            response_data: SnapshotDetailsResponseDict = {
+                "head_artifact_id": str(artifact.id),
+                "base_artifact_id": base_artifact_id,
+                "project_id": str(artifact.project_id),
+                "comparison_type": comparison_type,
+                "state": cast(str, artifact.state),
+                "vcs_info": cast(VcsInfoResponseDict, vcs_info.dict()),
+                "app_id": artifact.app_id,
+                "is_selective": snapshot_metrics.is_selective,
+                "images": image_list if comparison_type != "diff" else [],
+                "image_count": snapshot_metrics.image_count,
+                "added": categorized.added,
+                "added_count": len(categorized.added),
+                "removed": categorized.removed,
+                "removed_count": len(categorized.removed),
+                "renamed": categorized.renamed,
+                "renamed_count": len(categorized.renamed),
+                "changed": categorized.changed,
+                "changed_count": len(categorized.changed),
+                "unchanged": categorized.unchanged,
+                "unchanged_count": len(categorized.unchanged),
+                "errored": categorized.errored,
+                "errored_count": len(categorized.errored),
+                "skipped": categorized.skipped,
+                "skipped_count": len(categorized.skipped),
+                "diff_threshold": head_diff_threshold,
+                "comparison_state": derived_status.comparison_state,
+                "approval_status": derived_status.approval_status,
+                "comparison_error_message": derived_status.comparison_error_message,
+                "approvers": (
+                    [cast(SnapshotApproverResponseDict, a.dict()) for a in approver_list]
+                    if approved
+                    else []
+                ),
+            }
 
             if compact:
+                # Compact mode strips images to a subset of keys, producing a shape that
+                # is intentionally looser than the response TypedDict; mutate via a plain
+                # dict view.
+                compact_data = cast(dict[str, Any], response_data)
                 for key in _COMPACT_IMAGE_LIST_KEYS:
-                    response_data[key] = [_strip_to_compact(img) for img in response_data[key]]
+                    compact_data[key] = [_strip_to_compact(img) for img in compact_data[key]]
                 for key in _COMPACT_PAIR_LIST_KEYS:
-                    for pair in response_data[key]:
+                    for pair in compact_data[key]:
                         pair["base_image"] = _strip_to_compact(pair["base_image"])
                         pair["head_image"] = _strip_to_compact(pair["head_image"])
 
-        # cast() sanctioned here: pydantic .dict() returns dict[str, Any] with no
-        # static link back to SnapshotDetailsResponseDict. The TypedDict and the
-        # Pydantic model are kept in sync by hand at the source of truth.
-        body = cast(SnapshotDetailsResponseDict, response_data)
-        return Response(body)
+        return Response(response_data)
 
 
 @extend_schema(tags=["Snapshots"])

src/sentry/preprod/api/endpoints/snapshots/preprod_artifact_snapshot_image_detail.py

@@ -122,7 +122,7 @@ def _to_response_dict(resp: SnapshotImageDetailResponse) -> SnapshotImageDetailR
 
 
 # Intentionally uses a flat response format (nullable fields, no conditional shapes)
-# rather than matching the details endpoint's SnapshotDiffPair/SnapshotImageResponse split.
+# rather than the details endpoint's categorized diff-pair/image split.
 # This endpoint is designed for LLM/MCP consumers that benefit from a single uniform shape.
 @extend_schema(tags=["Snapshots"])
 @cell_silo_endpoint

src/sentry/preprod/api/endpoints/snapshots/preprod_artifact_snapshot_latest_base.py

@@ -31,11 +31,13 @@
 from sentry.preprod.analytics import PreprodArtifactApiGetLatestBaseSnapshotEvent
 from sentry.preprod.api.endpoints.snapshots.preprod_artifact_snapshot import (
     _strip_to_compact,
-    build_snapshot_image_response,
 )
-from sentry.preprod.api.models.public.snapshots import LatestBaseSnapshotResponseDict
+from sentry.preprod.api.models.public.snapshots import (
+    LatestBaseSnapshotImageResponseDict,
+    LatestBaseSnapshotResponseDict,
+)
 from sentry.preprod.models import PreprodArtifact
-from sentry.preprod.snapshots.manifest import SnapshotManifest
+from sentry.preprod.snapshots.image_serialization import build_head_image_dict
 
 logger = logging.getLogger(__name__)
 
@@ -204,7 +206,8 @@ def get(
             if response is None:
                 raise FileNotFoundError("Manifest does not exist in objectstore")
             manifest_data = orjson.loads(response.payload.read())
-            manifest = SnapshotManifest(**manifest_data)
+            manifest_images = manifest_data.get("images", {})
+            manifest_diff_threshold = manifest_data.get("diff_threshold")
         except Exception:
             logger.exception(
                 "Failed to retrieve snapshot manifest",
@@ -217,20 +220,23 @@ def get(
 
         image_base_url = f"/api/0/projects/{organization.slug}/{artifact.project.slug}/files/images"
 
-        images = []
-        for key, metadata in sorted(manifest.images.items()):
-            img = build_snapshot_image_response(key, metadata, manifest.diff_threshold).dict()
-            img["image_url"] = f"{image_base_url}/{metadata.content_hash}/"
+        images: list[LatestBaseSnapshotImageResponseDict] = []
+        for key, metadata in sorted(manifest_images.items()):
+            img = cast(
+                LatestBaseSnapshotImageResponseDict,
+                build_head_image_dict(key, metadata, manifest_diff_threshold),
+            )
+            img["image_url"] = f"{image_base_url}/{metadata['content_hash']}/"
             images.append(img)
 
-        response_data: dict[str, Any] = {
+        response_data: LatestBaseSnapshotResponseDict = {
             "head_artifact_id": str(artifact.id),
             "project_id": str(artifact.project_id),
             "project_slug": artifact.project.slug,
             "app_id": artifact.app_id,
             "image_count": snapshot_metrics.image_count,
             "images": images,
-            "diff_threshold": manifest.diff_threshold,
+            "diff_threshold": manifest_diff_threshold,
             "date_added": artifact.date_added.isoformat(),
         }
 
@@ -246,13 +252,10 @@ def get(
             }
 
         if compact:
-            response_data["images"] = [
+            compact_data = cast(dict[str, Any], response_data)
+            compact_data["images"] = [
                 {**_strip_to_compact(img), "image_url": img["image_url"]}
-                for img in response_data["images"]
+                for img in compact_data["images"]
             ]
 
-        # cast() sanctioned: response_data is a hand-built dict[str, Any] whose
-        # shape mirrors LatestBaseSnapshotResponseDict. The TypedDict and the
-        # builder are kept in sync by hand at the source of truth.
-        body = cast(LatestBaseSnapshotResponseDict, response_data)
-        return Response(body)
+        return Response(response_data)