Skip to content

Commit 418c4ee

Browse files
ref(snapshots): Add configurable diff_threshold for snapshot comparison
1 parent f6e153a commit 418c4ee

File tree

3 files changed

+32
-2
lines changed

3 files changed

+32
-2
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
"additionalProperties": ImageMetadata.schema(),
7979
"maxProperties": 50000,
8080
},
81+
"diff_threshold": {"type": "number", "minimum": 0.0, "maximum": 1.0},
8182
**VCS_SCHEMA_PROPERTIES,
8283
},
8384
"required": ["app_id", "images"],
@@ -466,6 +467,7 @@ def post(self, request: Request, project: Project) -> Response:
466467

467468
app_id = data.get("app_id")
468469
images = data.get("images", {})
470+
diff_threshold = data.get("diff_threshold")
469471

470472
# VCS info
471473
head_sha = data.get("head_sha")
@@ -524,7 +526,7 @@ def post(self, request: Request, project: Project) -> Response:
524526
# Write manifest inside the transaction so that a failed objectstore
525527
# write rolls back the DB records, ensuring both succeed or neither does.
526528
session = get_preprod_session(project.organization_id, project.id)
527-
manifest = SnapshotManifest(images=images)
529+
manifest = SnapshotManifest(images=images, diff_threshold=diff_threshold)
528530
manifest_json = manifest.json(exclude_none=True)
529531
session.put(manifest_json.encode(), key=manifest_key)
530532

src/sentry/preprod/snapshots/manifest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ class Config:
1818

1919
class SnapshotManifest(BaseModel):
2020
images: dict[str, ImageMetadata]
21+
diff_threshold: float | None = None
2122

2223

2324
class ComparisonImageResult(BaseModel):

src/sentry/preprod/snapshots/tasks.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,8 @@ def compare_snapshots(
381381
comparison.save(update_fields=["state", "error_code", "date_updated"])
382382
return
383383

384+
diff_threshold = head_manifest.diff_threshold
385+
384386
head_images = head_manifest.images
385387
base_images = base_manifest.images
386388

@@ -571,12 +573,37 @@ def _fetch_hash(h: str) -> None:
571573
)
572574
session.put(diff_mask_bytes, key=diff_mask_key, content_type="image/png")
573575

574-
is_changed = diff_result.changed_pixels > 0
576+
diff_pct = (
577+
diff_result.changed_pixels / diff_result.total_pixels
578+
if diff_result.total_pixels > 0
579+
else 0
580+
)
581+
hashes_differ = head_hash != base_hash
582+
effective_threshold = diff_threshold if diff_threshold is not None else 0.0
583+
is_changed = diff_pct > effective_threshold
584+
if not is_changed and hashes_differ and diff_result.changed_pixels == 0:
585+
logger.warning(
586+
"compare_snapshots: odiff reported 0 diff for %s but content hashes differ, forcing changed",
587+
name,
588+
extra={"head_artifact_id": head_artifact_id},
589+
)
590+
is_changed = True
575591
if is_changed:
576592
changed_count += 1
577593
else:
578594
unchanged_count += 1
579595

596+
logger.debug(
597+
"compare_snapshots: %s diff_pct=%.6f threshold=%s is_changed=%s pixels=%d/%d",
598+
name,
599+
diff_pct,
600+
diff_threshold,
601+
is_changed,
602+
diff_result.changed_pixels,
603+
diff_result.total_pixels,
604+
extra={"head_artifact_id": head_artifact_id},
605+
)
606+
580607
diff_mask_image_id = f"{head_artifact_id}/{base_artifact_id}/diff/{stem}.png"
581608

582609
image_results[name] = {

0 commit comments

Comments
 (0)