Skip to content

Commit 4154cc8

Browse files
feat(snapshots): Add selective flag and rename all_image_names to all_image_file_names (#113006)
## Summary Adds backend support for the new sentry-cli `--selective` flag ([sentry-cli PR #3268](getsentry/sentry-cli#3268)). - **`selective` boolean** added to upload API and manifest - **`all_image_names` renamed to `all_image_file_names`** to match CLI naming - **Three categorization modes:** - `selective` + `all_image_file_names`: can distinguish removed vs skipped (like Happo/Chromatic) - `selective` only: all missing base images treated as skipped, no removals (like Argos `--subset`) - Full build: missing = removed (existing behavior) - **Validation:** `all_image_file_names` requires `selective`, `selective` requires `base_sha` - **Baseline guard:** selective builds excluded from `find_base_snapshot_artifact` via `is_selective` DB field and don't trigger waiting-heads comparisons ## Test plan - [x] Unit tests for 3-branch categorization logic (selective+names, selective-only, full) - [x] API validation tests (selective requires base_sha, names requires selective, names rejects empty) - [x] Selective build without names: all missing = skipped - [x] Selective build with names: removed vs skipped distinguished - [x] Fingerprinting excludes skipped images for auto-approval --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0762b16 commit 4154cc8

10 files changed

Lines changed: 324 additions & 8 deletions

File tree

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

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@
8080
"maxProperties": 50000,
8181
},
8282
"diff_threshold": {"type": "number", "minimum": 0.0, "exclusiveMaximum": 1.0},
83+
"selective": {"type": "boolean"},
84+
"all_image_file_names": {
85+
"type": "array",
86+
"items": {"type": "string"},
87+
"maxItems": 50000,
88+
},
8389
**VCS_SCHEMA_PROPERTIES,
8490
},
8591
"required": ["app_id", "images"],
@@ -89,6 +95,8 @@
8995
SNAPSHOT_POST_REQUEST_ERROR_MESSAGES: dict[str, str] = {
9096
"app_id": "The app_id field is required and must be a string with maximum length of 255 characters.",
9197
"images": "The images field is required and must be an object mapping image names to image metadata.",
98+
"selective": "The selective field must be a boolean.",
99+
"all_image_file_names": "The all_image_file_names field must be an array of strings with at most 50000 entries.",
92100
**VCS_ERROR_MESSAGES,
93101
}
94102

@@ -434,6 +442,8 @@ def get(self, request: Request, organization: Organization, snapshot_id: str) ->
434442
unchanged_count=len(categorized.unchanged),
435443
errored=categorized.errored,
436444
errored_count=len(categorized.errored),
445+
skipped=categorized.skipped,
446+
skipped_count=len(categorized.skipped),
437447
comparison_run_info=run_info,
438448
approval_info=approval_info,
439449
diff_threshold=manifest.diff_threshold,
@@ -481,6 +491,35 @@ def post(self, request: Request, project: Project) -> Response:
481491
base_ref = data.get("base_ref")
482492
pr_number = data.get("pr_number")
483493

494+
selective = data.get("selective", False)
495+
all_image_file_names = data.get("all_image_file_names")
496+
497+
if all_image_file_names is not None and not selective:
498+
return Response(
499+
{"detail": "all_image_file_names requires selective to be true."},
500+
status=400,
501+
)
502+
503+
if selective and not base_sha:
504+
return Response(
505+
{"detail": "selective requires base_sha to be provided."},
506+
status=400,
507+
)
508+
509+
if all_image_file_names is not None:
510+
if not all_image_file_names:
511+
return Response(
512+
{"detail": "all_image_file_names must not be empty."},
513+
status=400,
514+
)
515+
all_image_file_names_set = set(all_image_file_names)
516+
missing = set(images.keys()) - all_image_file_names_set
517+
if missing:
518+
return Response(
519+
{"detail": "Every image name must appear in all_image_file_names."},
520+
status=400,
521+
)
522+
484523
# has_vcs tag differentiates transactions that include a CommitComparison
485524
# lookup from those that skip it, so we can isolate their latency on dashboards.
486525
with (
@@ -522,13 +561,19 @@ def post(self, request: Request, project: Project) -> Response:
522561
snapshot_metrics = PreprodSnapshotMetrics.objects.create(
523562
preprod_artifact=artifact,
524563
image_count=len(images),
564+
is_selective=selective,
525565
extras={"manifest_key": manifest_key},
526566
)
527567

528568
# Write manifest inside the transaction so that a failed objectstore
529569
# write rolls back the DB records, ensuring both succeed or neither does.
530570
session = get_preprod_session(project.organization_id, project.id)
531-
manifest = SnapshotManifest(images=images, diff_threshold=diff_threshold)
571+
manifest = SnapshotManifest(
572+
images=images,
573+
diff_threshold=diff_threshold,
574+
selective=selective,
575+
all_image_file_names=all_image_file_names,
576+
)
532577
manifest_json = manifest.json(exclude_none=True)
533578
session.put(manifest_json.encode(), key=manifest_key)
534579

@@ -654,7 +699,7 @@ def post(self, request: Request, project: Project) -> Response:
654699

655700
# Trigger comparisons for any head artifacts that were uploaded before this base.
656701
# Handles possible out-of-order uploads where heads arrive before their base build.
657-
if commit_comparison is not None:
702+
if commit_comparison is not None and not selective:
658703
try:
659704
waiting_heads = find_head_snapshot_artifacts_awaiting_base(
660705
organization_id=project.organization_id,

src/sentry/preprod/api/models/snapshots/project_preprod_snapshot_models.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ class SnapshotDiffSection(StrEnum):
1616
CHANGED = "changed"
1717
UNCHANGED = "unchanged"
1818
ERRORED = "errored"
19+
SKIPPED = "skipped"
1920

2021

2122
# GET response
@@ -93,6 +94,9 @@ class SnapshotDetailsApiResponse(BaseModel):
9394
errored: list[SnapshotDiffPair] = []
9495
errored_count: int = 0
9596

97+
skipped: list[SnapshotImageResponse] = []
98+
skipped_count: int = 0
99+
96100
comparison_run_info: SnapshotComparisonRunInfo | None = None
97101

98102
approval_info: SnapshotApprovalInfo | None = None

src/sentry/preprod/snapshots/comparison_categorizer.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class CategorizedComparison(BaseModel):
2121
unchanged: list[SnapshotImageResponse] = []
2222
renamed: list[SnapshotDiffPair] = []
2323
errored: list[SnapshotDiffPair] = []
24+
skipped: list[SnapshotImageResponse] = []
2425

2526

2627
def _base_image_from_comparison(name: str, img: ComparisonImageResult) -> SnapshotImageResponse:
@@ -109,6 +110,8 @@ def categorize_comparison_images(
109110
head_image=head,
110111
)
111112
)
113+
elif img.status == "skipped":
114+
result.skipped.append(base_img or _base_image_from_comparison(name, img))
112115

113116
result.changed.sort(key=lambda p: p.diff or 0, reverse=True)
114117
return result

src/sentry/preprod/snapshots/manifest.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@ class Config:
1919
class SnapshotManifest(BaseModel):
2020
images: dict[str, ImageMetadata]
2121
diff_threshold: float | None = Field(default=None, ge=0.0, lt=1.0)
22+
selective: bool = False
23+
all_image_file_names: list[str] | None = None
2224

2325

2426
class ComparisonImageResult(BaseModel):
25-
status: Literal["added", "removed", "changed", "unchanged", "errored", "renamed"]
27+
status: Literal["added", "removed", "changed", "unchanged", "errored", "renamed", "skipped"]
2628
head_hash: str | None = None
2729
base_hash: str | None = None
2830
changed_pixels: int | None = None
@@ -46,6 +48,7 @@ class ComparisonSummary(BaseModel):
4648
removed: int
4749
errored: int
4850
renamed: int
51+
skipped: int = 0
4952

5053

5154
class ComparisonManifest(BaseModel):

src/sentry/preprod/snapshots/tasks.py

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ class _ImageDiffResult(NamedTuple):
5151
matched: set[str]
5252
head_by_name: dict[str, str]
5353
base_by_name: dict[str, str]
54+
skipped: set[str]
5455

5556

5657
def categorize_image_diff(
@@ -60,9 +61,21 @@ def categorize_image_diff(
6061
head_by_name = {key: (meta.content_hash or key) for key, meta in head_manifest.images.items()}
6162
base_by_name = {key: (meta.content_hash or key) for key, meta in base_manifest.images.items()}
6263

64+
all_image_file_names = head_manifest.all_image_file_names
65+
6366
matched = head_by_name.keys() & base_by_name.keys()
6467
added = head_by_name.keys() - base_by_name.keys()
65-
removed = base_by_name.keys() - head_by_name.keys()
68+
69+
if all_image_file_names is not None:
70+
all_names_set = set(all_image_file_names)
71+
removed = base_by_name.keys() - all_names_set
72+
skipped = (all_names_set - head_by_name.keys()) & base_by_name.keys()
73+
elif head_manifest.selective:
74+
removed = set()
75+
skipped = base_by_name.keys() - head_by_name.keys()
76+
else:
77+
removed = base_by_name.keys() - head_by_name.keys()
78+
skipped = set()
6679

6780
added_hash_to_names: dict[str, list[str]] = {}
6881
for name in added:
@@ -84,8 +97,25 @@ def categorize_image_diff(
8497
for new_name, old_name in renamed_pairs:
8598
added.discard(new_name)
8699
removed.discard(old_name)
87-
88-
return _ImageDiffResult(renamed_pairs, added, removed, matched, head_by_name, base_by_name)
100+
added_hash_to_names.pop(head_by_name[new_name], None)
101+
102+
if skipped:
103+
skipped_hash_to_names: dict[str, list[str]] = {}
104+
for name in skipped:
105+
h = base_by_name[name]
106+
skipped_hash_to_names.setdefault(h, []).append(name)
107+
108+
for h in added_hash_to_names.keys() & skipped_hash_to_names.keys():
109+
a_names = added_hash_to_names[h]
110+
s_names = skipped_hash_to_names[h]
111+
if len(a_names) == 1 and len(s_names) == 1:
112+
renamed_pairs.append((a_names[0], s_names[0]))
113+
added.discard(a_names[0])
114+
skipped.discard(s_names[0])
115+
116+
return _ImageDiffResult(
117+
renamed_pairs, added, removed, matched, head_by_name, base_by_name, skipped
118+
)
89119

90120

91121
def _image_name_to_path_stem(name: str) -> str:
@@ -124,7 +154,7 @@ class ImageFingerprint(NamedTuple):
124154
def _build_comparison_fingerprints(manifest: ComparisonManifest) -> set[ImageFingerprint]:
125155
fingerprints: set[ImageFingerprint] = set()
126156
for name, image in manifest.images.items():
127-
if image.status == "unchanged":
157+
if image.status in ("unchanged", "skipped"):
128158
continue
129159
if image.status in ("changed", "added"):
130160
if not image.head_hash:
@@ -629,6 +659,17 @@ def _fetch_hash(h: str) -> None:
629659
"before_height": base_meta.height,
630660
}
631661

662+
skipped = categories.skipped
663+
for name in sorted(skipped):
664+
base_hash = base_by_name[name]
665+
base_meta = base_meta_by_hash[base_hash]
666+
image_results[name] = {
667+
"status": "skipped",
668+
"base_hash": base_hash,
669+
"before_width": base_meta.width,
670+
"before_height": base_meta.height,
671+
}
672+
632673
for new_name, old_name in sorted(renamed_pairs):
633674
content_hash = head_by_name[new_name]
634675
image_results[new_name] = {
@@ -641,13 +682,14 @@ def _fetch_hash(h: str) -> None:
641682
head_artifact_id=head_artifact_id,
642683
base_artifact_id=base_artifact_id,
643684
summary=ComparisonSummary(
644-
total=len(matched) + len(added) + len(removed) + len(renamed_pairs),
685+
total=len(matched) + len(added) + len(removed) + len(renamed_pairs) + len(skipped),
645686
changed=changed_count,
646687
unchanged=unchanged_count,
647688
added=len(added),
648689
removed=len(removed),
649690
errored=error_count,
650691
renamed=len(renamed_pairs),
692+
skipped=len(skipped),
651693
),
652694
images=image_results,
653695
)
@@ -668,6 +710,7 @@ def _fetch_hash(h: str) -> None:
668710
comparison.images_added = len(added)
669711
comparison.images_removed = len(removed)
670712
comparison.images_renamed = len(renamed_pairs)
713+
comparison.images_skipped = len(skipped)
671714
extras = comparison.extras or {}
672715
# EME-896: Could become a proper column on PreprodSnapshotComparison
673716
extras["comparison_key"] = comparison_key
@@ -682,6 +725,7 @@ def _fetch_hash(h: str) -> None:
682725
"images_added",
683726
"images_removed",
684727
"images_renamed",
728+
"images_skipped",
685729
"extras",
686730
"date_updated",
687731
]

src/sentry/preprod/snapshots/utils.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ def find_base_snapshot_artifact(
2323
commit_comparison__head_repo_name=base_repo_name,
2424
project_id=project_id,
2525
preprodsnapshotmetrics__isnull=False,
26+
preprodsnapshotmetrics__is_selective=False,
2627
app_id=app_id,
2728
artifact_type=artifact_type,
2829
build_configuration=build_configuration,

tests/sentry/preprod/api/endpoints/test_preprod_artifact_snapshot.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,58 @@ def test_snapshot_invalid_sha_format(self) -> None:
250250

251251
assert response.status_code == 400
252252

253+
def _selective_data(self, **overrides):
254+
data = {
255+
"app_id": "com.test.app",
256+
"images": {"screen.png": {"width": 100, "height": 200}},
257+
"selective": True,
258+
"all_image_file_names": ["screen.png", "skipped.png"],
259+
"head_sha": "a" * 40,
260+
"base_sha": "b" * 40,
261+
"provider": "github.com",
262+
"head_repo_name": "org/repo",
263+
"head_ref": "feature",
264+
}
265+
data.update(overrides)
266+
return data
267+
268+
def _post_selective(self, **overrides):
269+
with self.feature("organizations:preprod-snapshots"):
270+
return self.client.post(
271+
self._get_create_url(), self._selective_data(**overrides), format="json"
272+
)
273+
274+
def test_all_image_file_names_rejects_empty_list(self):
275+
response = self._post_selective(images={}, all_image_file_names=[])
276+
assert response.status_code == 400
277+
assert "empty" in response.data["detail"]
278+
279+
def test_selective_requires_base_sha(self):
280+
response = self._post_selective(base_sha=None)
281+
assert response.status_code == 400
282+
assert "base_sha" in response.data["detail"]
283+
284+
def test_all_image_file_names_must_contain_all_images(self):
285+
response = self._post_selective(all_image_file_names=["other.png"])
286+
assert response.status_code == 400
287+
assert "all_image_file_names" in response.data["detail"]
288+
289+
def test_all_image_file_names_requires_selective(self):
290+
response = self._post_selective(selective=False)
291+
assert response.status_code == 400
292+
assert "selective" in response.data["detail"]
293+
294+
def test_selective_without_all_image_file_names_accepted(self):
295+
data = self._selective_data()
296+
del data["all_image_file_names"]
297+
with self.feature("organizations:preprod-snapshots"):
298+
response = self.client.post(self._get_create_url(), data, format="json")
299+
assert response.status_code == 200
300+
301+
def test_selective_with_all_image_file_names_accepted(self):
302+
response = self._post_selective()
303+
assert response.status_code == 200
304+
253305
@patch("sentry.preprod.api.endpoints.preprod_artifact_snapshot.get_preprod_session")
254306
@patch("sentry.preprod.api.endpoints.preprod_artifact_snapshot.compare_snapshots")
255307
def test_base_upload_triggers_comparison_for_waiting_head(

tests/sentry/preprod/snapshots/test_auto_approve.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,18 @@ def test_skips_changed_with_missing_head_hash(self):
8686
fps = _build_comparison_fingerprints(manifest)
8787
assert fps == {ImageFingerprint("has_hash.png", "changed", "def")}
8888

89+
def test_skipped_images_excluded_from_fingerprints(self):
90+
manifest = self._make_manifest(
91+
{
92+
"changed.png": ComparisonImageResult(
93+
status="changed", head_hash="b", base_hash="c"
94+
),
95+
"skipped.png": ComparisonImageResult(status="skipped", base_hash="e"),
96+
}
97+
)
98+
fps = _build_comparison_fingerprints(manifest)
99+
assert fps == {ImageFingerprint("changed.png", "changed", "b")}
100+
89101
def test_skips_renamed_with_missing_hash_or_previous_name(self):
90102
manifest = self._make_manifest(
91103
{

0 commit comments

Comments
 (0)