feat(code-review): Handle issue comment incl eyes emoji #105527

suejung-sentry · 2025-12-30T04:47:05Z

Hook up so we handle issue comment including the posting of eyes emoji from within sentry. This is behind an option for github.webhook.issue-comment which decides whether we route flow through this path or the path that forwards to overwatch.

Note that though in theory overwatch at some point also supported review (including posting 👀 ) for pull_request_review and pull_request_review_comment, I tried out these flows and they don't need to be working right now. I think it's fine to maintain parity and just port it over as just the issue comment is how you can trigger the on_command_phrase review.

Closes CW-10

linear · 2025-12-30T04:49:55Z

CW-10 Port emoji 👀 🎉 reactions to Sentry

armenzg

I'm sending some feedback for now. I need to implement something and get back to you.

src/sentry/seer/code_review/webhooks/handlers.py

armenzg · 2025-12-30T12:58:06Z

src/sentry/seer/code_review/webhooks/issue_comment.py

+    comment_id: str,
+) -> None:
+    if integration is None:
+        metrics.incr("seer.code_review.webhook.issue_comment.reaction_missing_integration")


I tend to use a prefix so I can easily create error widgets in Datadog.

sentry/src/sentry/seer/code_review/webhooks/check_run.py

Lines 44 to 45 in 40560ae

class Metrics(enum.StrEnum):

ERROR = "seer.code_review.error"

This is really nice. Thanks for the best practice.
Should it be scoped this widely as all of seer.code_review.error, or worth narrowing down the ones I added to seer.code_review_.webhook.issue_comment.error?

src/sentry/seer/code_review/webhooks/issue_comment.py

armenzg · 2025-12-30T13:03:50Z

src/sentry/seer/code_review/webhooks/issue_comment.py

+        client = integration.get_installation(organization_id=organization.id).get_client()
+        client.create_comment_reaction(repo.name, comment_id, GitHubReaction.EYES)
+        metrics.incr("seer.code_review.webhook.issue_comment.reaction_added")
+    except Exception:


You can either let the exception raise and handle it in the caller or you need to return the outcome to the caller and exit.

On this one, I was purposely having the helper _add_eyes_reaction_to_comment log and swallow the exception so every caller of the helper wouldn't need to wrap in a try/catch. I would like us to still forward to seer even if for some reason the 👀 posting failed. For example if we hit a rate limit here, but still want the PR review itself to proceed. What do you think?

tests/sentry/seer/code_review/test_webhooks.py

armenzg · 2025-12-30T13:10:33Z

tests/sentry/seer/code_review/test_webhooks.py

+        }
+
+    def _enable_code_review(self) -> None:
+        self.organization.update_option("sentry:enable_pr_review_test_generation", True)


Since we're adding new code, would it make sense to open a PR to add a more meaningful option? That way we can remediate the problem as we're making progress in the Overwatch port. I now see why you asked me about that other ticket.

It's possible this is way more complicated than I assume 😆 .

Yeah.. I'll clean this up with https://linear.app/getsentry/issue/CW-88/list-features-and-related-features-for-code-review

armenzg · 2025-12-30T13:17:29Z

tests/sentry/seer/code_review/test_webhooks.py

+        mock_forward.assert_not_called()
+
+    @patch("sentry.seer.code_review.webhooks.issue_comment._add_eyes_reaction_to_comment")
+    @patch("sentry.seer.code_review.webhooks.handlers._forward_to_seer")


For this case, instead of mocking the task scheduling remove this and use with self.tasks():

You will need to mock the function that would attempt to do the reaction.

I like to see closer to full integration testing when dealing with the success case.

src/sentry/features/temporary.py

armenzg

This is very close. A few more changes and we can merge it.

src/sentry/seer/code_review/webhooks/issue_comment.py

tests/sentry/seer/code_review/test_webhooks.py

armenzg · 2025-12-30T23:34:29Z

tests/sentry/seer/code_review/test_webhooks.py

-            mock_integration, self.organization, self.repo, "123456789"
-        )
+        mock_create_reaction.assert_called_once()
        mock_seer.assert_called_once()


@suejung-sentry Calling Seer should not happen since we have not set the option to False (Switching from forwarding to Overwatch to Seer).

There's a bug in this code which I was going to handle in one of my PRs:

sentry/src/sentry/seer/code_review/webhooks/task.py

Line 36 in 3c0966a

make_seer_request(path=SeerEndpoint.SENTRY_REQUEST.value, payload=event_payload)

I will be adding a way to prevent the call to Seer if the options for the specific Github event type is not set to False.

Thanks, I totally thought we were flipping the other way around, my bad. I'll await your other PR that makes the fix and you can make any changes to this PR as we discussed

src/sentry/seer/code_review/webhooks/issue_comment.py

tests/sentry/seer/code_review/test_webhooks.py

sentry · 2025-12-31T03:10:12Z

src/sentry/seer/code_review/webhooks/issue_comment.py

+    if comment_id:
+        if not options.get("github.webhook.issue-comment"):
+            _add_eyes_reaction_to_comment(integration, organization, repo, str(comment_id))
+
+    # Import here to avoid circular dependency with handlers
+    from .handlers import _schedule_task
+
+    _schedule_task(event_type, event, organization, repo)


Bug: The _schedule_task function is called unconditionally, causing duplicate processing when the github.webhook.issue-comment option is True.
_{Severity: HIGH | Confidence: High}

🔍 Detailed Analysis

In issue_comment.py, the code checks the github.webhook.issue-comment option. If this option is False, it adds a reaction. However, the call to _schedule_task on line 114 occurs outside of this conditional check, meaning it is executed unconditionally. This causes a task to be scheduled in Sentry even when the option is True, a state intended to forward the event to another service (Overwatch). This results in the event being processed by both Sentry and the other service, leading to conflicting behavior.

💡 Suggested Fix

Move the _schedule_task(...) call inside the if not options.get("github.webhook.issue-comment"): block to ensure it is only executed when this option is False.

🤖 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/seer/code_review/webhooks/issue_comment.py#L107-L114 Potential issue: In `issue_comment.py`, the code checks the `github.webhook.issue-comment` option. If this option is `False`, it adds a reaction. However, the call to `_schedule_task` on line 114 occurs outside of this conditional check, meaning it is executed unconditionally. This causes a task to be scheduled in Sentry even when the option is `True`, a state intended to forward the event to another service (Overwatch). This results in the event being processed by both Sentry and the other service, leading to conflicting behavior.

_{Did we get this right? 👍 / 👎 to inform future reviews.}
_{Reference ID: 8054017}

feat(code-review): Add post eyes from sentry instead overwatch

92be11c

suejung-sentry requested review from a team as code owners December 30, 2025 04:47

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Dec 30, 2025

suejung-sentry marked this pull request as draft December 30, 2025 04:48

fix test and lint

40560ae

vercel bot deployed to Preview December 30, 2025 05:20 View deployment

armenzg assigned suejung-sentry Dec 30, 2025

armenzg reviewed Dec 30, 2025

View reviewed changes

incorporate pr comments

3683d7c

vercel bot deployed to Preview December 30, 2025 19:02 View deployment

fix test

0b0ea04

vercel bot deployed to Preview December 30, 2025 19:17 View deployment

suejung-sentry requested a review from armenzg December 30, 2025 19:20

armenzg approved these changes Dec 30, 2025

View reviewed changes

src/sentry/seer/code_review/webhooks/issue_comment.py Outdated Show resolved Hide resolved

src/sentry/seer/code_review/webhooks/issue_comment.py Outdated Show resolved Hide resolved

tests/sentry/seer/code_review/test_webhooks.py Outdated Show resolved Hide resolved

incorporate pr comments

b78e5bd

vercel bot deployed to Preview December 30, 2025 20:34 View deployment

use enum

4f52e81

suejung-sentry marked this pull request as ready for review December 30, 2025 20:42

vercel bot deployed to Preview December 30, 2025 20:44 View deployment

armenzg requested changes Dec 30, 2025

View reviewed changes

flip the feature flag

740b846

sentry bot reviewed Dec 31, 2025

View reviewed changes

vercel bot deployed to Preview December 31, 2025 03:11 View deployment

	class Metrics(enum.StrEnum):
	ERROR = "seer.code_review.error"

Uh oh!

feat(code-review): Handle issue comment incl eyes emoji #105527

Are you sure you want to change the base?

feat(code-review): Handle issue comment incl eyes emoji #105527

Conversation

suejung-sentry commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linear bot commented Dec 30, 2025

Uh oh!

armenzg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

armenzg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sentry bot Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

suejung-sentry commented Dec 30, 2025 •

edited

Loading