Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion migrations_lockfile.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ replays: 0007_organizationmember_replay_access

seer: 0002_add_default_coding_agent

sentry: 1041_projectkeymapping
sentry: 1042_create_code_review_event

social_auth: 0003_social_auth_json_field

Expand Down
3 changes: 3 additions & 0 deletions src/sentry/backup/comparators.py
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,9 @@ def get_default_comparators() -> dict[str, list[JSONScrubbingComparator]]:
"sentry.alertrule": [
DateUpdatedComparator("date_modified"),
],
"sentry.codereviewevent": [
DateUpdatedComparator("date_added", "date_updated"),
],
"sentry.dashboardfavoriteuser": [
DateUpdatedComparator("date_added", "date_updated"),
],
Expand Down
119 changes: 119 additions & 0 deletions src/sentry/migrations/1042_create_code_review_event.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# Generated by Django 5.2.11 on 2026-02-26 00:51

import django.db.models.deletion
import django.utils.timezone
import sentry.db.models.fields.bounded
import sentry.db.models.fields.foreignkey
import sentry.models.code_review_event
from django.db import migrations, models

from sentry.new_migrations.migrations import CheckedMigration


class Migration(CheckedMigration):
# This flag is used to mark that a migration shouldn't be automatically run in production.
# This should only be used for operations where it's safe to run the migration after your
# code has deployed. So this should not be used for most operations that alter the schema
# of a table.
# Here are some things that make sense to mark as post deployment:
# - Large data migrations. Typically we want these to be run manually so that they can be
# monitored and not block the deploy for a long period of time while they run.
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
# run this outside deployments so that we don't block them. Note that while adding an index
# is a schema change, it's completely safe to run the operation after the code has deployed.
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment

is_post_deployment = False

dependencies = [
("sentry", "1041_projectkeymapping"),
]

operations = [
migrations.CreateModel(
name="CodeReviewEvent",
fields=[
(
"id",
sentry.db.models.fields.bounded.BoundedBigAutoField(
primary_key=True, serialize=False
),
),
("date_updated", models.DateTimeField(auto_now=True)),
("date_added", models.DateTimeField(auto_now_add=True)),
("pr_number", models.IntegerField(null=True)),
("pr_title", models.TextField(null=True)),
("pr_author", models.TextField(null=True)),
("pr_url", models.TextField(null=True)),
("pr_state", models.CharField(max_length=16, null=True)),
("raw_event_type", models.CharField(max_length=64)),
("raw_event_action", models.CharField(max_length=64)),
("trigger_id", models.CharField(max_length=64, null=True)),
("trigger", models.CharField(max_length=64, null=True)),
("trigger_user", models.TextField(null=True)),
("trigger_at", models.DateTimeField(default=django.utils.timezone.now)),
("target_commit_sha", models.CharField(max_length=64, null=True)),
(
"status",
models.CharField(
default=sentry.models.code_review_event.CodeReviewEventStatus[
"WEBHOOK_RECEIVED"
],
max_length=32,
),
),
("denial_reason", models.TextField(null=True)),
("webhook_received_at", models.DateTimeField(null=True)),
("preflight_completed_at", models.DateTimeField(null=True)),
("task_enqueued_at", models.DateTimeField(null=True)),
("sent_to_seer_at", models.DateTimeField(null=True)),
("review_started_at", models.DateTimeField(null=True)),
("review_completed_at", models.DateTimeField(null=True)),
("seer_run_id", models.CharField(max_length=64, null=True)),
(
"comments_posted",
sentry.db.models.fields.bounded.BoundedPositiveIntegerField(null=True),
),
("review_result", models.JSONField(null=True)),
(
"organization",
sentry.db.models.fields.foreignkey.FlexibleForeignKey(
on_delete=django.db.models.deletion.CASCADE,
to="sentry.organization",
),
),
(
"repository",
sentry.db.models.fields.foreignkey.FlexibleForeignKey(
on_delete=django.db.models.deletion.CASCADE,
to="sentry.repository",
),
),
],
options={
"db_table": "sentry_code_review_event",
"indexes": [
models.Index(fields=["date_added"], name="sentry_code_date_ad_a2451c_idx"),
models.Index(
fields=["organization", "trigger_at"],
name="sentry_code_organiz_4f4b09_idx",
),
models.Index(
fields=["organization", "repository", "trigger_at"],
name="sentry_code_organiz_7ba32c_idx",
),
models.Index(
fields=["organization", "repository", "pr_number"],
name="sentry_code_organiz_76bbd1_idx",
),
],
"constraints": [
models.UniqueConstraint(
condition=models.Q(("trigger_id__isnull", False)),
fields=("organization", "repository", "trigger_id"),
name="unique_org_repo_trigger_id",
)
],
Comment on lines +106 to +116
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Migration 1042_create_code_review_event is missing a dependency on 1035_delete_code_review_event, which can cause migration failures due to pre-existing table indexes and constraints.
Severity: CRITICAL

Suggested Fix

Update the dependencies in 1042_create_code_review_event.py to depend on ("sentry", "1035_delete_code_review_event") instead of ("sentry", "1041_projectkeymapping"). This ensures the old table is dropped before the new one is created.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/sentry/migrations/1042_create_code_review_event.py#L96-L116

Potential issue: The migration `1042_create_code_review_event` recreates the
`CodeReviewEvent` table, which was deleted by a previous migration,
`1035_delete_code_review_event`. However, migration `1042` incorrectly depends on
`1041_projectkeymapping` instead of `1035`. This lack of an explicit dependency means
the Django migration framework doesn't guarantee the deletion migration runs before the
creation migration. If the ordering is incorrect during deployment, the migration will
fail when attempting to create a table and indexes with names that already exist,
blocking the deployment process.

Did we get this right? 👍 / 👎 to inform future reviews.

},
),
]
1 change: 1 addition & 0 deletions src/sentry/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from .authproviderreplica import * # NOQA
from .avatars import * # NOQA
from .broadcast import * # NOQA
from .code_review_event import * # NOQA
from .commit import * # NOQA
from .commitauthor import * # NOQA
from .commitcomparison import * # NOQA
Expand Down
85 changes: 85 additions & 0 deletions src/sentry/models/code_review_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@

from enum import StrEnum

from django.db import models
from django.utils import timezone

from sentry.backup.scopes import RelocationScope
from sentry.db.models import (
BoundedPositiveIntegerField,
DefaultFieldsModel,
FlexibleForeignKey,
region_silo_model,
sane_repr,
)


class CodeReviewEventStatus(StrEnum):
WEBHOOK_RECEIVED = "webhook_received"
Expand All @@ -16,3 +28,76 @@ class CodeReviewEventStatus(StrEnum):
@classmethod
def as_choices(cls) -> tuple[tuple[str, str], ...]:
return tuple((status.value, status.value) for status in cls)


@region_silo_model
class CodeReviewEvent(DefaultFieldsModel):
"""
Records every SCM webhook event entering the Seer code review pipeline.
Tracks the full lifecycle from webhook receipt to review completion.
"""

__relocation_scope__ = RelocationScope.Global

organization = FlexibleForeignKey("sentry.Organization")
repository = FlexibleForeignKey("sentry.Repository")

# PR identification
pr_number = models.IntegerField(null=True)
pr_title = models.TextField(null=True)
pr_author = models.TextField(null=True)
pr_url = models.TextField(null=True)
pr_state = models.CharField(max_length=16, null=True) # open, closed, merged

# Raw webhook event metadata (provider-specific values)
raw_event_type = models.CharField(max_length=64)
raw_event_action = models.CharField(max_length=64)
trigger_id = models.CharField(max_length=64, null=True)

# Provider-agnostic fields (aligns with SeerCodeReviewConfig)
trigger = models.CharField(max_length=64, null=True)
trigger_user = models.TextField(null=True)
trigger_at = models.DateTimeField(default=timezone.now)

target_commit_sha = models.CharField(max_length=64, null=True)

# Explicit status column because multiple statuses share the same timestamp
# field (e.g. PREFLIGHT_DENIED/WEBHOOK_FILTERED both set preflight_completed_at).
status = models.CharField(
max_length=32,
choices=CodeReviewEventStatus.as_choices(),
default=CodeReviewEventStatus.WEBHOOK_RECEIVED,
)
Comment on lines +66 to +70
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if the status can just be derived by looking at the dates, but maybe it's easier to just keep it as a column

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't really distinguish based on timestamp fields alone as these are stored for success or failure states both. Plus the stats endpoint (in subsequent PR) filters based on status directly.

denial_reason = models.TextField(null=True)

# Timestamps for pipeline stages (region-silo wall clock)
webhook_received_at = models.DateTimeField(null=True)
Copy link
Member

Choose a reason for hiding this comment

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

Is this the control timestamp? or when the region receives it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

region, i'll add a note

preflight_completed_at = models.DateTimeField(null=True)
task_enqueued_at = models.DateTimeField(null=True)
sent_to_seer_at = models.DateTimeField(null=True)
review_started_at = models.DateTimeField(null=True)
review_completed_at = models.DateTimeField(null=True)

# Seer callback data
seer_run_id = models.CharField(max_length=64, null=True)
comments_posted = BoundedPositiveIntegerField(null=True)
review_result = models.JSONField(null=True) # raw Seer response payload

class Meta:
app_label = "sentry"
db_table = "sentry_code_review_event"
indexes = (
models.Index(fields=("date_added",)), # cleanup task
models.Index(fields=("organization", "trigger_at")), # stats endpoint
models.Index(fields=("organization", "repository", "trigger_at")), # events list
models.Index(fields=("organization", "repository", "pr_number")), # PR lookup
)
constraints = [
models.UniqueConstraint(
fields=["organization", "repository", "trigger_id"],
Copy link
Member

Choose a reason for hiding this comment

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

What will happen when two rows have a trigger_id of null? (e.g. two events for the same PR come in before we have a trigger_id)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a partial unique index with condition=models.Q(trigger_id__isnull=False), meaning it only enforces uniqueness when trigger_id is non-null

name="unique_org_repo_trigger_id",
condition=models.Q(trigger_id__isnull=False),
),
]

__repr__ = sane_repr("organization_id", "repository_id", "pr_number", "status")
20 changes: 20 additions & 0 deletions src/sentry/testutils/helpers/backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
from sentry.models.apitoken import ApiToken
from sentry.models.authidentity import AuthIdentity
from sentry.models.authprovider import AuthProvider
from sentry.models.code_review_event import CodeReviewEvent, CodeReviewEventStatus
from sentry.models.counter import Counter
from sentry.models.dashboard import (
Dashboard,
Expand Down Expand Up @@ -656,6 +657,25 @@ def create_exhaustive_organization(
],
)

CodeReviewEvent.objects.create(
organization=org,
repository=repo,
raw_event_type="pull_request",
raw_event_action="opened",
trigger_id=f"trigger-{slug}",
pr_number=1,
pr_title=f"Test PR for {slug}",
pr_author="test-author",
pr_url="https://github.com/getsentry/getsentry/pull/1",
pr_state="open",
trigger="on_new_commit",
trigger_user="test-user",
target_commit_sha="abc123",
status=CodeReviewEventStatus.REVIEW_COMPLETED,
seer_run_id=f"seer-run-{slug}",
comments_posted=2,
)

# Group*
group = self.create_group(project=project)
group_search_view = GroupSearchView.objects.create(
Expand Down
Loading