Skip to content

feat(preprod): skip comparisons for different analysis versions #103195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 12, 2025
+386 −58
Merged

feat(preprod): skip comparisons for different analysis versions #103195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9e21f15
fix
trevor-e Nov 11, 2025
c8fd428
fix
trevor-e Nov 11, 2025
9bb7da9
tests
trevor-e Nov 11, 2025
a645506
fix
trevor-e Nov 11, 2025
179fd7b
fix
trevor-e Nov 12, 2025
96b913c
ugh
trevor-e Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 115 additions & 57 deletions src/sentry/preprod/size_analysis/compare.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

import logging

from packaging.version import InvalidVersion
from packaging.version import parse as parse_version

from sentry.preprod.models import PreprodArtifactSizeMetrics
from sentry.preprod.size_analysis.models import (
ComparisonResults,
Expand All @@ -21,7 +24,9 @@ def compare_size_analysis(
base_size_analysis: PreprodArtifactSizeMetrics,
base_size_analysis_results: SizeAnalysisResults,
) -> ComparisonResults:
diff_items = []
skip_diff_item_comparison = _should_skip_diff_item_comparison(
head_size_analysis_results, base_size_analysis_results
)

head_treemap = (
head_size_analysis_results.treemap.root if head_size_analysis_results.treemap else None
Expand All @@ -35,70 +40,82 @@ def compare_size_analysis(

all_paths = set(head_files.keys()) | set(base_files.keys())

for path in sorted(all_paths):
head_elements = head_files.get(path, [])
base_elements = base_files.get(path, [])
diff_items = []

matched_pairs, unmatched_head, unmatched_base = _match_elements(
head_elements, base_elements
)
if not skip_diff_item_comparison:
Copy link
Member

@rbro112 rbro112 Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should log the fact we're skipping if so

for path in sorted(all_paths):
head_elements = head_files.get(path, [])
base_elements = base_files.get(path, [])

# Process matched pairs (modified or unchanged)
for head_element, base_element in matched_pairs:
head_size = head_element.size
base_size = base_element.size
size_diff = head_size - base_size

if size_diff == 0:
continue # Skip items with no size change

item_type = head_element.type
diff_type = DiffType.INCREASED if size_diff > 0 else DiffType.DECREASED

diff_items.append(
DiffItem(
size_diff=size_diff,
head_size=head_size,
base_size=base_size,
path=path,
item_type=item_type,
type=diff_type,
)
matched_pairs, unmatched_head, unmatched_base = _match_elements(
head_elements, base_elements
)

# Process unmatched head elements (added)
for head_element in unmatched_head:
head_size = head_element.size
if head_size == 0:
continue

diff_items.append(
DiffItem(
size_diff=head_size,
head_size=head_size,
base_size=None,
path=path,
item_type=head_element.type,
type=DiffType.ADDED,
# Process matched pairs (modified or unchanged)
for head_element, base_element in matched_pairs:
head_size = head_element.size
base_size = base_element.size
size_diff = head_size - base_size

if size_diff == 0:
continue # Skip items with no size change

item_type = head_element.type
diff_type = DiffType.INCREASED if size_diff > 0 else DiffType.DECREASED

diff_items.append(
DiffItem(
size_diff=size_diff,
head_size=head_size,
base_size=base_size,
path=path,
item_type=item_type,
type=diff_type,
)
)
)

# Process unmatched base elements (removed)
for base_element in unmatched_base:
base_size = base_element.size
if base_size == 0:
continue
# Process unmatched head elements (added)
for head_element in unmatched_head:
head_size = head_element.size
if head_size == 0:
continue

diff_items.append(
DiffItem(
size_diff=head_size,
head_size=head_size,
base_size=None,
path=path,
item_type=head_element.type,
type=DiffType.ADDED,
)
)

diff_items.append(
DiffItem(
size_diff=-base_size,
head_size=None,
base_size=base_size,
path=path,
item_type=base_element.type,
type=DiffType.REMOVED,
# Process unmatched base elements (removed)
for base_element in unmatched_base:
base_size = base_element.size
if base_size == 0:
continue

diff_items.append(
DiffItem(
size_diff=-base_size,
head_size=None,
base_size=base_size,
path=path,
item_type=base_element.type,
type=DiffType.REMOVED,
)
)
)
else:
logger.info(
"preprod.size_analysis.compare.skipped_diff_item_comparison",
extra={
"head_analysis_version": head_size_analysis_results.analysis_version,
"base_analysis_version": base_size_analysis_results.analysis_version,
"preprod_artifact_id": head_size_analysis.preprod_artifact_id,
},
)

size_metric_diff_item = SizeMetricDiffItem(
metrics_artifact_type=head_size_analysis.metrics_artifact_type,
Expand All @@ -112,9 +129,50 @@ def compare_size_analysis(
return ComparisonResults(
diff_items=diff_items,
size_metric_diff_item=size_metric_diff_item,
skipped_diff_item_comparison=skip_diff_item_comparison,
head_analysis_version=head_size_analysis_results.analysis_version,
base_analysis_version=base_size_analysis_results.analysis_version,
)


def _should_skip_diff_item_comparison(
head_size_analysis_results: SizeAnalysisResults,
base_size_analysis_results: SizeAnalysisResults,
) -> bool:
head_version = None
base_version = None

if head_size_analysis_results.analysis_version:
try:
head_version = parse_version(head_size_analysis_results.analysis_version)
except InvalidVersion:
logger.warning(
"preprod.size_analysis.compare.invalid_version_format",
extra={
"analysis_version": head_size_analysis_results.analysis_version,
},
)

if base_size_analysis_results.analysis_version:
try:
base_version = parse_version(base_size_analysis_results.analysis_version)
except InvalidVersion:
logger.warning(
"preprod.size_analysis.compare.invalid_version_format",
extra={
"analysis_version": base_size_analysis_results.analysis_version,
},
)

if not head_version or not base_version:
return False

has_mismatched_major = head_version.major != base_version.major
has_mismatched_minor = head_version.minor != base_version.minor

return has_mismatched_major or has_mismatched_minor


def _match_elements(
head_elements: list[TreemapElement], base_elements: list[TreemapElement]
) -> tuple[list[tuple[TreemapElement, TreemapElement]], list[TreemapElement], list[TreemapElement]]:
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/preprod/size_analysis/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,6 @@ class SizeMetricDiffItem(BaseModel):
class ComparisonResults(BaseModel):
diff_items: list[DiffItem]
size_metric_diff_item: SizeMetricDiffItem
skipped_diff_item_comparison: bool
Copy link
Member Author

@trevor-e trevor-e Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can look for this field in the frontend to show a nice UI around "You should rebase..."

head_analysis_version: str | None
Copy link
Member Author

@trevor-e trevor-e Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't sure if the analysis versions should go in SizeMetricDiffItem or not.

base_analysis_version: str | None
Loading
Loading