fix: UI and status report diff patch numbers

helpers/comparison.py

@@ -0,0 +1,21 @@
+from shared.reports.types import ReportTotals
+
+
+def minimal_totals(totals: ReportTotals | None) -> dict:
+    if totals is None:
+        return {
+            "hits": 0,
+            "misses": 0,
+            "partials": 0,
+            "coverage": None,
+        }
+    return {
+        "hits": totals.hits,
+        "misses": totals.misses,
+        "partials": totals.partials,
+        # ReportTotals has coverage as a string, we want float in the DB
+        # Also the coverage from ReportTotals is 0-100, while in the DB it's 0-1
+        "coverage": (
+            (float(totals.coverage) / 100) if totals.coverage is not None else None
+        ),
+    }

services/comparison/__init__.py

@@ -59,6 +59,7 @@
         self._repository_service = None
         self._adjusted_base_diff = None
         self._original_base_diff = None
+        self._patch_totals = None
         self._changes = None
         self._existing_statuses = None
         self._behind_by = None
@@ -209,9 +210,11 @@
 
         Patch coverage refers to looking at the coverage in HEAD report filtered by the git diff HEAD..BASE.
         """
+        if self._patch_totals:
+            return self._patch_totals
         diff = await self.get_diff(use_original_base=True)
-        totals = self.head.report.apply_diff(diff)
-        return totals
+        self._patch_totals = self.head.report.apply_diff(diff)
+        return self._patch_totals
 
     async def get_behind_by(self):
         async with self._behind_by_lock:
@@ -371,6 +374,7 @@
         self.flags = flags
         self.path_patterns = path_patterns
         self.real_comparison = real_comparison
+        self._patch_totals = None
         self._changes = None
         self.project_coverage_base = FullCommit(
             commit=real_comparison.project_coverage_base.commit,
@@ -394,6 +398,18 @@
     async def get_diff(self, use_original_base=False):
         return await self.real_comparison.get_diff(use_original_base=use_original_base)
 
+    @sentry_sdk.trace
+    async def get_patch_totals(self) -> ReportTotals | None:
+        """Returns the patch coverage for the comparison.
+
+        Patch coverage refers to looking at the coverage in HEAD report filtered by the git diff HEAD..BASE.
+        """
+        if self._patch_totals:
+            return self._patch_totals
+        diff = await self.get_diff(use_original_base=True)
+        self._patch_totals = self.head.report.apply_diff(diff)
+        return self._patch_totals
+
     async def get_existing_statuses(self):
         return await self.real_comparison.get_existing_statuses()
 

services/notification/notifiers/mixins/status.py

@@ -9,7 +9,9 @@
 
 
 class StatusPatchMixin(object):
-    async def get_patch_status(self, comparison) -> Tuple[str, str]:
+    async def get_patch_status(
+        self, comparison: ComparisonProxy | FilteredComparison
+    ) -> Tuple[str, str]:
         threshold = self.notifier_yaml_settings.get("threshold", "0.0")
 
         # check if user has erroneously added a % to this input and fix
@@ -21,8 +23,7 @@ async def get_patch_status(self, comparison) -> Tuple[str, str]:
         except (InvalidOperation, TypeError):
             threshold = Decimal("0.0")
 
-        diff = await comparison.get_diff(use_original_base=True)
-        totals = comparison.head.report.apply_diff(diff)
+        totals = await comparison.get_patch_totals()
         if self.notifier_yaml_settings.get("target") not in ("auto", None):
             target_coverage = Decimal(
                 str(self.notifier_yaml_settings.get("target")).replace("%", "")

services/repository.py

@@ -471,6 +471,7 @@ class EnrichedPull(object):
     provider_pull: Optional[Mapping[str, Any]]
 
 
+@sentry_sdk.trace
 async def fetch_and_update_pull_request_information_from_commit(
     repository_service, commit, current_yaml
 ) -> Optional[EnrichedPull]:

tasks/compute_comparison.py

@@ -7,14 +7,14 @@
 from shared.components import Component
 from shared.helpers.flag import Flag
 from shared.reports.readonly import ReadOnlyReport
-from shared.reports.types import ReportTotals
 from shared.torngit.exceptions import TorngitRateLimitError
 from shared.yaml import UserYaml
 
 from app import celery_app
 from database.enums import CompareCommitError, CompareCommitState
 from database.models import CompareCommit, CompareComponent, CompareFlag
 from database.models.reports import ReportLevelTotals, RepositoryFlag
+from helpers.comparison import minimal_totals
 from helpers.github_installation import get_installation_name_for_owner_for_task
 from services.archive import ArchiveService
 from services.comparison import ComparisonContext, ComparisonProxy, FilteredComparison
@@ -66,8 +66,9 @@ def run_impl(
 
         # At this point we can calculate the patch coverage
         # Because we have a HEAD report and a base commit to get the diff from
-        patch_totals = async_to_sync(comparison_proxy.get_patch_totals)()
-        comparison.patch_totals = self.minimal_totals(patch_totals)
+        if comparison.patch_totals is None:
+            patch_totals = async_to_sync(comparison_proxy.get_patch_totals)()
+            comparison.patch_totals = minimal_totals(patch_totals)
 
         if not comparison_proxy.has_project_coverage_base_report():
             comparison.error = CompareCommitError.missing_base_report.value
@@ -96,18 +97,6 @@ def run_impl(
         path = self.store_results(comparison, impacted_files)
 
         comparison.report_storage_path = path
-        calculated_patch_totals = impacted_files.get("changes_summary").get(
-            "patch_totals"
-        )
-        if calculated_patch_totals != comparison.patch_totals:
-            log.warning(
-                "Calculated patch totals differ!",
-                extra={
-                    **log_extra,
-                    "calculated_patch_totals": calculated_patch_totals,
-                    "patch_totals": comparison.patch_totals,
-                },
-            )
 
         comparison.state = CompareCommitState.processed.value
         log.info("Computing comparison successful", extra=log_extra)
@@ -119,25 +108,6 @@ def run_impl(
         db_session.commit()
         return {"successful": True}
 
-    def minimal_totals(self, totals: ReportTotals | None) -> dict:
-        if totals is None:
-            return {
-                "hits": 0,
-                "misses": 0,
-                "partials": 0,
-                "coverage": None,
-            }
-        return {
-            "hits": totals.hits,
-            "misses": totals.misses,
-            "partials": totals.partials,
-            # ReportTotals has coverage as a string, we want float in the DB
-            # Also the coverage from ReportTotals is 0-100, while in the DB it's 0-1
-            "coverage": (
-                (float(totals.coverage) / 100) if totals.coverage is not None else None
-            ),
-        }
-
     def compute_flag_comparison(self, db_session, comparison, comparison_proxy):
         log_extra = dict(comparison_id=comparison.id)
         log.info("Computing flag comparisons", extra=log_extra)

tasks/notify.py

@@ -1,6 +1,7 @@
 import logging
 from typing import Optional
 
+import sentry_sdk
 from asgiref.sync import async_to_sync
 from celery.exceptions import MaxRetriesExceededError, SoftTimeLimitExceeded
 from shared.celery_config import (
@@ -14,15 +15,17 @@
 from shared.torngit.base import TokenType, TorngitBaseAdapter
 from shared.torngit.exceptions import TorngitClientError, TorngitServerFailureError
 from shared.yaml import UserYaml
+from sqlalchemy import and_
 from sqlalchemy.orm.session import Session
 
 from app import celery_app
 from database.enums import CommitErrorTypes, Decoration, NotificationState, ReportType
 from database.models import Commit, Pull
-from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME
+from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME, CompareCommit
 from helpers.checkpoint_logger import from_kwargs as checkpoints_from_kwargs
 from helpers.checkpoint_logger.flows import UploadFlow
 from helpers.clock import get_seconds_to_next_hour
+from helpers.comparison import minimal_totals
 from helpers.exceptions import NoConfiguredAppsAvailable, RepositoryWithoutValidBotError
 from helpers.github_installation import get_installation_name_for_owner_for_task
 from helpers.save_commit_error import save_commit_error
@@ -511,6 +514,33 @@ def has_upcoming_notifies_according_to_redis(
             return True
         return False
 
+    @sentry_sdk.trace
+    def save_patch_totals(self, comparison: ComparisonProxy) -> None:
+        """Saves patch coverage to the CompareCommit, if it exists.
+        This is done to make sure the patch coverage reported by notifications and UI is the same
+        (because they come from the same source)
+        """
+        if comparison.project_coverage_base.commit is None:
+            # This is the base that will be saved in the CommitComparison
+            # Even if the patch coverage could come from a different commit
+            return
+        head_commit = comparison.head.commit
+        db_session = head_commit.get_db_session()
+        patch_coverage = async_to_sync(comparison.get_patch_totals)()
+        statement = (
+            CompareCommit.__table__.update()
+            .where(
+                and_(
+                    CompareCommit.compare_commit_id == head_commit.id,
+                    CompareCommit.base_commit_id
+                    == comparison.project_coverage_base.commit.id,
+                )
+            )
+            .values(patch_totals=minimal_totals(patch_coverage))
+        )
+        db_session.execute(statement)
+
+    @sentry_sdk.trace
     def submit_third_party_notifications(
         self,
         current_yaml: UserYaml,
@@ -562,6 +592,8 @@ def submit_third_party_notifications(
             ),
         )
 
+        self.save_patch_totals(comparison)
+
         decoration_type = self.determine_decoration_type_from_pull(
             enriched_pull, empty_upload
         )
@@ -687,6 +719,7 @@ def schedule_new_user_activated_task(self, org_ownerid, user_ownerid):
             ),
         )
 
+    @sentry_sdk.trace
     def fetch_and_update_whether_ci_passed(
         self, repository_service, commit, current_yaml
     ):