[CLEAN] Synthetic Benchmark PR #103469 - chore(preprod): abstract metric cards for build comparison

[tomerqodo]

User description

Benchmark PR getsentry#103469

Type: Clean (correct implementation)

Original PR Title: chore(preprod): abstract metric cards for build comparison
Original PR Description: - small style changes

• abstract card into component to reuse in follow up pr

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Original PR URL: getsentry#103469

---

PR Type

Enhancement

---

Description

• Extract metric cards rendering into reusable component

• Abstract build comparison metrics display logic

• Remove inline styled components and consolidate styling

• Simplify main content component by delegating card rendering

---

Diagram Walkthrough

flowchart LR
  A["sizeCompareMainContent.tsx"] -- "delegates metric rendering" --> B["BuildComparisonMetricCards.tsx"]
  B -- "uses" --> C["MetricCard.tsx"]
  A -- "removes inline logic" --> D["Cleaner main component"]

Loading

File Walkthrough

Relevant files

Enhancement

BuildComparisonMetricCards.tsx New metric cards component for build comparison                    static/app/views/preprod/buildComparison/main/BuildComparisonMetricCards.tsx New component that encapsulates metric card rendering logic Processes comparison results to calculate install and download size metrics Handles trend calculation and percentage change formatting Renders metric cards with size differences and base build information +168/-0  metricCard.tsx New reusable metric card component                                              static/app/views/preprod/components/metricCard.tsx New reusable metric card component with configurable icon and label Supports optional tooltip for label and action button Provides consistent styling with background, radius, padding and border Accepts children for flexible content rendering +68/-0    sizeCompareMainContent.tsx Refactor to use extracted metric cards component                  static/app/views/preprod/buildComparison/main/sizeCompareMainContent.tsx Remove inline metric card rendering logic and styled components Remove unused imports for PercentChange, IconArrow, IconCode, IconDownload Remove styled DiffText component definition Replace with BuildComparisonMetricCards component import and usage Simplify component by delegating metric processing to new component +6/-142

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logs: The new UI components compute and display build size metrics without emitting any audit or analytics logs for critical user actions, but it is unclear if such logging is handled elsewhere. Referred Code export function BuildComparisonMetricCards(props: BuildComparisonMetricCardsProps) { const {comparisonResults, comparisonResponse} = props; const metrics = useMemo<ComparisonMetric[]>(() => { if (!comparisonResults) { return []; } const labels = getLabels( comparisonResponse?.head_build_details.app_info?.platform ?? undefined ); const {size_metric_diff_item} = comparisonResults; return [ { key: 'install', title: labels.installSizeLabel, icon: <IconCode size="sm" />, head: size_metric_diff_item.head_install_size, base: size_metric_diff_item.base_install_size, diff: ... (clipped 95 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Edge case handling: Percentage change divides by base and falls back to 0 when base is 0, which may mask infinite change; confirm this fallback is acceptable and surfaced in UI. Referred Code size_metric_diff_item.base_install_size === 0 ? 0 : (size_metric_diff_item.head_install_size - size_metric_diff_item.base_install_size) / size_metric_diff_item.base_install_size, }, { key: 'download', title: labels.downloadSizeLabel, icon: <IconDownload size="sm" />, head: size_metric_diff_item.head_download_size, base: size_metric_diff_item.base_download_size, diff: size_metric_diff_item.head_download_size - size_metric_diff_item.base_download_size, percentageChange: size_metric_diff_item.base_download_size === 0 ? 0 : (size_metric_diff_item.head_download_size - size_metric_diff_item.base_download_size) / size_metric_diff_item.base_download_size, Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: External data usage: The component renders values from API responses without explicit validation here, though sanitization may occur upstream; verify types and nullability are enforced at API boundaries. Referred Code export function BuildComparisonMetricCards(props: BuildComparisonMetricCardsProps) { const {comparisonResults, comparisonResponse} = props; const metrics = useMemo<ComparisonMetric[]>(() => { if (!comparisonResults) { return []; } const labels = getLabels( comparisonResponse?.head_build_details.app_info?.platform ?? undefined ); const {size_metric_diff_item} = comparisonResults; return [ { key: 'install', title: labels.installSizeLabel, icon: <IconCode size="sm" />, head: size_metric_diff_item.head_install_size, base: size_metric_diff_item.base_install_size, diff: ... (clipped 95 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Create a shared trend utility The new getTrend function should be extracted into a shared utility. This promotes code reuse and consistency, as similar trend-related logic exists elsewhere in the codebase. Examples: static/app/views/preprod/buildComparison/main/BuildComparisonMetricCards.tsx [149-168] function getTrend(diff: number): { variant: 'danger' | 'success' | 'muted'; icon?: React.ReactNode; } { if (diff > 0) { return { variant: 'danger', icon: <IconArrow direction="up" size="xs" />, }; } ... (clipped 10 lines) Solution Walkthrough: Before: // In '.../BuildComparisonMetricCards.tsx' export function BuildComparisonMetricCards(props) { // ... return ( <Flex> {metrics.map(metric => { const {variant, icon} = getTrend(metric.diff); // ... render card using variant and icon })} </Flex> ); } function getTrend(diff: number) { if (diff > 0) { return { variant: 'danger', icon: <IconArrow direction="up" /> }; } if (diff < 0) { return { variant: 'success', icon: <IconArrow direction="down" /> }; } return { variant: 'muted' }; } After: // In 'sentry/utils/trends.ts' (new file) export function getTrendVisuals(diff: number) { if (diff > 0) { return { variant: 'danger', icon: <IconArrow direction="up" /> }; } if (diff < 0) { return { variant: 'success', icon: <IconArrow direction="down" /> }; } return { variant: 'muted' }; } // In '.../BuildComparisonMetricCards.tsx' import {getTrendVisuals} from 'sentry/utils/trends'; export function BuildComparisonMetricCards(props) { // ... return ( <Flex> {metrics.map(metric => { const {variant, icon} = getTrendVisuals(metric.diff); // ... render card using variant and icon })} </Flex> ); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies the new getTrend function and a recurring pattern of trend visualization, proposing a valid abstraction for better code reuse and consistency.	Medium
Possible issue	Fix incorrect percentage change calculation Update the percentageChange calculation to return Infinity when base_install_size is zero and head_install_size is positive, correctly representing newly introduced assets. static/app/views/preprod/buildComparison/main/BuildComparisonMetricCards.tsx [55-60] percentageChange: size_metric_diff_item.base_install_size === 0 - ? 0 + ? size_metric_diff_item.head_install_size > 0 + ? Infinity + : 0 : (size_metric_diff_item.head_install_size - size_metric_diff_item.base_install_size) / size_metric_diff_item.base_install_size, Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a logical bug in the new component where a new asset (base size 0) would misleadingly show a 0% change, fixing how data is calculated.	Medium
	Avoid rendering infinite percentage change Add a check for isFinite(metric.percentageChange) before rendering the PercentChange component to handle cases where the percentage change is Infinity. static/app/views/preprod/buildComparison/main/BuildComparisonMetricCards.tsx [110-129] -{metric.percentageChange !== 0 && ( +{metric.percentageChange !== 0 && isFinite(metric.percentageChange) && ( <Text as="span" variant={variant} style={{ display: 'inline-flex', alignItems: 'center', whiteSpace: 'nowrap', }} > {' ('} <PercentChange value={metric.percentageChange} minimumValue={0.001} preferredPolarity="-" colorize /> {')'} </Text> )} Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion is a necessary follow-up to the first one, preventing the UI from attempting to render an infinite value, which could cause rendering errors or display NaN.	Medium
More