From 418c4ee40cc4e1a55059fb0a652e731865eef4eb Mon Sep 17 00:00:00 2001 From: Nico Hinderling Date: Fri, 10 Apr 2026 12:41:31 -0700 Subject: [PATCH 1/2] ref(snapshots): Add configurable diff_threshold for snapshot comparison --- .../endpoints/preprod_artifact_snapshot.py | 4 ++- src/sentry/preprod/snapshots/manifest.py | 1 + src/sentry/preprod/snapshots/tasks.py | 29 ++++++++++++++++++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py b/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py index 5bf1b1c86ac9e4..1b1dc10d0bd922 100644 --- a/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py +++ b/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py @@ -78,6 +78,7 @@ "additionalProperties": ImageMetadata.schema(), "maxProperties": 50000, }, + "diff_threshold": {"type": "number", "minimum": 0.0, "maximum": 1.0}, **VCS_SCHEMA_PROPERTIES, }, "required": ["app_id", "images"], @@ -466,6 +467,7 @@ def post(self, request: Request, project: Project) -> Response: app_id = data.get("app_id") images = data.get("images", {}) + diff_threshold = data.get("diff_threshold") # VCS info head_sha = data.get("head_sha") @@ -524,7 +526,7 @@ def post(self, request: Request, project: Project) -> Response: # Write manifest inside the transaction so that a failed objectstore # write rolls back the DB records, ensuring both succeed or neither does. session = get_preprod_session(project.organization_id, project.id) - manifest = SnapshotManifest(images=images) + manifest = SnapshotManifest(images=images, diff_threshold=diff_threshold) manifest_json = manifest.json(exclude_none=True) session.put(manifest_json.encode(), key=manifest_key) diff --git a/src/sentry/preprod/snapshots/manifest.py b/src/sentry/preprod/snapshots/manifest.py index 894a2a8c161618..f601a6ea0890ec 100644 --- a/src/sentry/preprod/snapshots/manifest.py +++ b/src/sentry/preprod/snapshots/manifest.py @@ -18,6 +18,7 @@ class Config: class SnapshotManifest(BaseModel): images: dict[str, ImageMetadata] + diff_threshold: float | None = None class ComparisonImageResult(BaseModel): diff --git a/src/sentry/preprod/snapshots/tasks.py b/src/sentry/preprod/snapshots/tasks.py index d6968ebb46c5e7..027d210c25ad8b 100644 --- a/src/sentry/preprod/snapshots/tasks.py +++ b/src/sentry/preprod/snapshots/tasks.py @@ -381,6 +381,8 @@ def compare_snapshots( comparison.save(update_fields=["state", "error_code", "date_updated"]) return + diff_threshold = head_manifest.diff_threshold + head_images = head_manifest.images base_images = base_manifest.images @@ -571,12 +573,37 @@ def _fetch_hash(h: str) -> None: ) session.put(diff_mask_bytes, key=diff_mask_key, content_type="image/png") - is_changed = diff_result.changed_pixels > 0 + diff_pct = ( + diff_result.changed_pixels / diff_result.total_pixels + if diff_result.total_pixels > 0 + else 0 + ) + hashes_differ = head_hash != base_hash + effective_threshold = diff_threshold if diff_threshold is not None else 0.0 + is_changed = diff_pct > effective_threshold + if not is_changed and hashes_differ and diff_result.changed_pixels == 0: + logger.warning( + "compare_snapshots: odiff reported 0 diff for %s but content hashes differ, forcing changed", + name, + extra={"head_artifact_id": head_artifact_id}, + ) + is_changed = True if is_changed: changed_count += 1 else: unchanged_count += 1 + logger.debug( + "compare_snapshots: %s diff_pct=%.6f threshold=%s is_changed=%s pixels=%d/%d", + name, + diff_pct, + diff_threshold, + is_changed, + diff_result.changed_pixels, + diff_result.total_pixels, + extra={"head_artifact_id": head_artifact_id}, + ) + diff_mask_image_id = f"{head_artifact_id}/{base_artifact_id}/diff/{stem}.png" image_results[name] = { From 2fdde0565f8222debb896a11b38636aa33e8572d Mon Sep 17 00:00:00 2001 From: Nico Hinderling Date: Fri, 10 Apr 2026 14:40:48 -0700 Subject: [PATCH 2/2] ref(snapshots): Validate diff_threshold range and remove odiff workaround MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make diff_threshold exclusive of 1.0 in both the API schema and Pydantic model since a threshold of 1.0 with strict > comparison would silently prevent any image from being marked changed. Also remove the hash-mismatch fallback that forced changed status when odiff reported 0 diff — the upstream odiff bug is being fixed. Co-Authored-By: Claude Opus 4.6 --- .../preprod/api/endpoints/preprod_artifact_snapshot.py | 2 +- src/sentry/preprod/snapshots/manifest.py | 2 +- src/sentry/preprod/snapshots/tasks.py | 8 -------- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py b/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py index 1b1dc10d0bd922..ddf3f864fb1805 100644 --- a/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py +++ b/src/sentry/preprod/api/endpoints/preprod_artifact_snapshot.py @@ -78,7 +78,7 @@ "additionalProperties": ImageMetadata.schema(), "maxProperties": 50000, }, - "diff_threshold": {"type": "number", "minimum": 0.0, "maximum": 1.0}, + "diff_threshold": {"type": "number", "minimum": 0.0, "exclusiveMaximum": 1.0}, **VCS_SCHEMA_PROPERTIES, }, "required": ["app_id", "images"], diff --git a/src/sentry/preprod/snapshots/manifest.py b/src/sentry/preprod/snapshots/manifest.py index f601a6ea0890ec..1af789528ace98 100644 --- a/src/sentry/preprod/snapshots/manifest.py +++ b/src/sentry/preprod/snapshots/manifest.py @@ -18,7 +18,7 @@ class Config: class SnapshotManifest(BaseModel): images: dict[str, ImageMetadata] - diff_threshold: float | None = None + diff_threshold: float | None = Field(default=None, ge=0.0, lt=1.0) class ComparisonImageResult(BaseModel): diff --git a/src/sentry/preprod/snapshots/tasks.py b/src/sentry/preprod/snapshots/tasks.py index 027d210c25ad8b..71a74fe326d995 100644 --- a/src/sentry/preprod/snapshots/tasks.py +++ b/src/sentry/preprod/snapshots/tasks.py @@ -578,16 +578,8 @@ def _fetch_hash(h: str) -> None: if diff_result.total_pixels > 0 else 0 ) - hashes_differ = head_hash != base_hash effective_threshold = diff_threshold if diff_threshold is not None else 0.0 is_changed = diff_pct > effective_threshold - if not is_changed and hashes_differ and diff_result.changed_pixels == 0: - logger.warning( - "compare_snapshots: odiff reported 0 diff for %s but content hashes differ, forcing changed", - name, - extra={"head_artifact_id": head_artifact_id}, - ) - is_changed = True if is_changed: changed_count += 1 else: