From 1ce97b21636aed8547cbfaa3a66ae39260fac46b Mon Sep 17 00:00:00 2001 From: Trevor Elkins Date: Wed, 8 Jul 2026 14:52:11 -0400 Subject: [PATCH 1/4] fix(preprod): Parse dotted Apple build numbers instead of dropping them app_info.build (CFBundleVersion) is only guaranteed to be a plain integer for Android's versionCode. Apple allows up to three dot-separated non-negative integers (e.g. "1.2.3"), which the previous isdigit() check silently turned into None. That broke Sentry's build_number tiebreaker for sorting/finding the latest build within a version, and also made those artifacts non-installable on the Sentry side. Pack multi-component builds into a single sortable int by zero-padding each component to a fixed width, while leaving the existing plain-integer path unchanged. Also send the raw, unparsed build string alongside it as build_number_raw so Sentry can store/display the exact value later without another launchpad deploy; Sentry currently ignores unrecognized fields, so this is safe to ship ahead of that work. Co-Authored-By: Claude Sonnet 5 --- src/launchpad/api/update_api_models.py | 3 +++ src/launchpad/artifact_processor.py | 24 +++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/launchpad/api/update_api_models.py b/src/launchpad/api/update_api_models.py index facc378f..205520c6 100644 --- a/src/launchpad/api/update_api_models.py +++ b/src/launchpad/api/update_api_models.py @@ -65,6 +65,9 @@ class UpdateData(BaseModel): app_id: str build_version: str build_number: Optional[int] + build_number_raw: Optional[str] = Field( + None, description="Unparsed build identifier (e.g. raw CFBundleVersion) for display purposes" + ) artifact_type: int apple_app_info: Optional[AppleAppInfo] = None android_app_info: Optional[AndroidAppInfo] = None diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index aa301441..6a5b0d13 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -570,7 +570,7 @@ def _prepare_update_data( dequeued_at: datetime, app_icon_id: str | None, ) -> dict[str, Any]: - build_number = int(app_info.build) if app_info.build.isdigit() else None + build_number = _parse_build_number(app_info.build) apple_app_info = None if isinstance(app_info, AppleAppInfo): @@ -602,6 +602,7 @@ def _prepare_update_data( app_id=app_info.app_id, build_version=app_info.version, build_number=build_number, + build_number_raw=app_info.build, artifact_type=_get_artifact_type(artifact).value, apple_app_info=apple_app_info, android_app_info=android_app_info, @@ -662,3 +663,24 @@ def _guess_message(code: ProcessingErrorCode, e: Exception) -> ProcessingErrorMe elif code == ProcessingErrorCode.ARTIFACT_PROCESSING_TIMEOUT: return ProcessingErrorMessage.PROCESSING_TIMEOUT return ProcessingErrorMessage.UNKNOWN_ERROR + + +def _parse_build_number(build: str, component_width: int = 6) -> int | None: + """Parse a raw build identifier (e.g. CFBundleVersion) into a sortable int. + + Plain integer builds (the common case, e.g. Android versionCode) pass through + unchanged. Apple's CFBundleVersion also allows up to three dot-separated + non-negative integers (e.g. "1.2.3"); those are packed into a single int by + zero-padding each component to `component_width` digits, which preserves + correct ordering as long as no component reaches 10**component_width. + Anything else (non-numeric, malformed) returns None, same as today. + """ + if build.isdigit(): + return int(build) + + parts = build.split(".") + if 2 <= len(parts) <= 3 and all(p.isdigit() and len(p) <= component_width for p in parts): + parts += ["0"] * (3 - len(parts)) + return sum(int(part) * 10 ** (component_width * (2 - i)) for i, part in enumerate(parts)) + + return None From 2f223dd57aa2a34e1e6bde0374597ca1a3522b00 Mon Sep 17 00:00:00 2001 From: Trevor Elkins Date: Wed, 8 Jul 2026 15:05:31 -0400 Subject: [PATCH 2/4] test(preprod): Add unit tests for build number parsing Covers _parse_build_number directly (plain ints unchanged, dotted Apple CFBundleVersion values packed and correctly ordered, malformed input still falls back to None) and _prepare_update_data's wiring of build_number/build_number_raw into the update payload. Co-Authored-By: Claude Sonnet 5 --- .../unit/artifacts/test_artifact_processor.py | 89 ++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-) diff --git a/tests/unit/artifacts/test_artifact_processor.py b/tests/unit/artifacts/test_artifact_processor.py index 4276d371..8e7aa42d 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -1,8 +1,12 @@ +from datetime import datetime from unittest.mock import Mock, patch +import pytest + from objectstore_client import Client as ObjectstoreClient -from launchpad.artifact_processor import ArtifactProcessor, ServiceConfig +from launchpad.artifact_processor import ArtifactProcessor, ServiceConfig, _parse_build_number +from launchpad.artifacts.android.aab import AAB from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive from launchpad.artifacts.artifact import Artifact from launchpad.constants import ( @@ -11,6 +15,7 @@ ProcessingErrorMessage, ) from launchpad.sentry_client import SentryClient, SentryClientError +from launchpad.size.models.android import AndroidAppInfo from launchpad.utils.objectstore import ObjectstoreConfig from launchpad.utils.statsd import FakeStatsd @@ -398,3 +403,85 @@ def test_process_message_project_not_skipped(self, mock_process, mock_sentry_cli "increment", {"metric": "artifact.processing.completed", "value": 1, "tags": None}, ) in calls + + +class TestParseBuildNumber: + @pytest.mark.parametrize( + "build,expected", + [ + # Plain integers (e.g. Android versionCode) pass through unchanged + ("9999", 9999), + ("0", 0), + ("1", 1), + # Apple CFBundleVersion: up to three dot-separated non-negative integers + ("1.2.3", 1_000_002_000_003), + ("1.2", 1_000_002_000_000), + # Malformed or unsupported shapes fall back to None, same as before + ("1.2.a", None), + ("abc", None), + ("1.2.3.4", None), + ("", None), + # A component too wide for the padding width is refused rather than + # silently corrupting the ordering of adjacent components + ("1234567.2.3", None), + ], + ) + def test_parse_build_number(self, build, expected): + assert _parse_build_number(build) == expected + + def test_dotted_builds_sort_correctly_within_a_version(self): + # Naive "strip the dots and parse as one number" concatenation would rank + # 1.99 ("199") above 2.0 ("20"), even though 1.99 is the earlier build. + assert _parse_build_number("1.99") < _parse_build_number("2.0") + + def test_distinguishes_builds_that_naive_concatenation_would_collide(self): + # "1.2.3", "12.3", and "1.23" would all naively concatenate to "123". + assert _parse_build_number("1.2.3") != _parse_build_number("12.3") + assert _parse_build_number("1.2.3") != _parse_build_number("1.23") + + +class TestPrepareUpdateData: + def setup_method(self): + """Set up test fixtures.""" + mock_sentry_client = Mock(spec=SentryClient) + mock_statsd = Mock() + mock_objectstore_client = Mock(spec=ObjectstoreClient) + self.processor = ArtifactProcessor(mock_sentry_client, mock_statsd, mock_objectstore_client) + + def test_dotted_build_number_is_parsed_and_raw_value_preserved(self): + app_info = AndroidAppInfo( + name="My App", + version="1.2.300", + build="1.2.3", + app_id="com.example.app", + has_proguard_mapping=False, + ) + + update_data = self.processor._prepare_update_data( + app_info=app_info, + artifact=Mock(spec=AAB), + dequeued_at=datetime(2026, 1, 1), + app_icon_id=None, + ) + + assert update_data["build_number"] == 1_000_002_000_003 + assert update_data["build_number_raw"] == "1.2.3" + + def test_plain_build_number_still_parses_as_int(self): + app_info = AndroidAppInfo( + name="My App", + version="1.2.300", + build="9999", + app_id="com.example.app", + has_proguard_mapping=False, + ) + + update_data = self.processor._prepare_update_data( + app_info=app_info, + artifact=Mock(spec=AAB), + dequeued_at=datetime(2026, 1, 1), + app_icon_id=None, + ) + + assert update_data["build_number"] == 9999 + assert update_data["build_number_raw"] == "9999" From a08c7e023ba4270c0e5d149a31613b462131c2b5 Mon Sep 17 00:00:00 2001 From: Trevor Elkins Date: Wed, 8 Jul 2026 15:09:40 -0400 Subject: [PATCH 3/4] ref(preprod): Make build number component width a module constant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit component_width was a function parameter but nothing ever called _parse_build_number with a non-default value — it's an implementation detail of the packing scheme, not something callers should tune. Co-Authored-By: Claude Sonnet 5 --- src/launchpad/artifact_processor.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index 6a5b0d13..b1fbdc5b 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -665,22 +665,28 @@ def _guess_message(code: ProcessingErrorCode, e: Exception) -> ProcessingErrorMe return ProcessingErrorMessage.UNKNOWN_ERROR -def _parse_build_number(build: str, component_width: int = 6) -> int | None: +# Zero-padding width per CFBundleVersion component when packing it into a single +# sortable int. 6 digits comfortably covers any realistic CI build counter while +# leaving the packed value well under BoundedBigIntegerField's max (bigint). +_BUILD_NUMBER_COMPONENT_WIDTH = 6 + + +def _parse_build_number(build: str) -> int | None: """Parse a raw build identifier (e.g. CFBundleVersion) into a sortable int. Plain integer builds (the common case, e.g. Android versionCode) pass through unchanged. Apple's CFBundleVersion also allows up to three dot-separated non-negative integers (e.g. "1.2.3"); those are packed into a single int by - zero-padding each component to `component_width` digits, which preserves - correct ordering as long as no component reaches 10**component_width. - Anything else (non-numeric, malformed) returns None, same as today. + zero-padding each component, which preserves correct ordering as long as no + component reaches 10**_BUILD_NUMBER_COMPONENT_WIDTH. Anything else + (non-numeric, malformed) returns None, same as today. """ if build.isdigit(): return int(build) parts = build.split(".") - if 2 <= len(parts) <= 3 and all(p.isdigit() and len(p) <= component_width for p in parts): + if 2 <= len(parts) <= 3 and all(p.isdigit() and len(p) <= _BUILD_NUMBER_COMPONENT_WIDTH for p in parts): parts += ["0"] * (3 - len(parts)) - return sum(int(part) * 10 ** (component_width * (2 - i)) for i, part in enumerate(parts)) + return sum(int(part) * 10 ** (_BUILD_NUMBER_COMPONENT_WIDTH * (2 - i)) for i, part in enumerate(parts)) return None From ddd89c0ba9a478a8b38ff4cdb3de347b8dee13a1 Mon Sep 17 00:00:00 2001 From: Trevor Elkins Date: Wed, 8 Jul 2026 15:29:20 -0400 Subject: [PATCH 4/4] docs(preprod): Document mixed build-format ordering limitation A reviewer correctly flagged that plain-integer build numbers stay small while packed dotted values are much larger, so mixing both conventions within the same app_id/build_version isn't reliably ordered by this int alone. This can't be fixed here: a single artifact update has no visibility into sibling artifacts' formats, and unifying the magnitude would break backward compatibility with every already-stored plain-integer build_number. The authoritative fix belongs in Sentry's tiebreak query, comparing the raw build string once build_number_raw is available there, with this int remaining a fast common-case sort key. Co-Authored-By: Claude Sonnet 5 --- src/launchpad/artifact_processor.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index b1fbdc5b..c54b726d 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -680,6 +680,16 @@ def _parse_build_number(build: str) -> int | None: zero-padding each component, which preserves correct ordering as long as no component reaches 10**_BUILD_NUMBER_COMPONENT_WIDTH. Anything else (non-numeric, malformed) returns None, same as today. + + Known limitation: plain-integer values are left small while packed dotted + values are much larger, so if the same app_id/build_version ever has builds + in both conventions, this int alone is not a reliable ordering between them. + We can't fix that here — a single artifact update has no visibility into + sibling artifacts' formats, and unifying the magnitude would break backward + compatibility with every already-stored plain-integer build_number. Sentry's + tiebreak query should treat this as a fast, common-case sort key and fall + back to comparing the raw build string (build_number_raw) directly when it + needs to be authoritative across mixed formats. """ if build.isdigit(): return int(build)