Skip to content

feat(pr-iteration): react with 🎉 on comment after iteration completes#119441

Open
alexsohn1126 wants to merge 6 commits into
masterfrom
alexsohn/react-with-tada-on-comment-after-iteration
Open

feat(pr-iteration): react with 🎉 on comment after iteration completes#119441
alexsohn1126 wants to merge 6 commits into
masterfrom
alexsohn/react-with-tada-on-comment-after-iteration

Conversation

@alexsohn1126

@alexsohn1126 alexsohn1126 commented Jul 10, 2026

Copy link
Copy Markdown
Member

previously we weren't updating the reactions that we left on GitHub messages on Autofix PR iteration comments, but now we will do so after this PR.

There were a couple of key decisions made here:

repo_name is now attached to every GH feedback. This is because there may be a case where multiple PRs exist for one autofix run, and we need to know which repository the comment is from in order to react on the correct comment. This is because when we are reacting 👀 while processing the feedback, we know exactly what feedback to react to since we are handling a webhook. However, we are using OnCompletionHook for post-seer workflows, which does not include any info on what comment we are looking at.

fixes CW-1398

@alexsohn1126 alexsohn1126 force-pushed the alexsohn/react-with-tada-on-comment-after-iteration branch from c7f6260 to 587b5df Compare July 10, 2026 21:51
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 10, 2026
When a PR-comment-triggered Autofix iteration finishes and its changes are
synced to the PR, swap the 👀 reaction we add at trigger time for a 🎉 on the
originating comment — for both inline review comments and top-level PR
comments — so the commenter can see from the comment that their feedback was
applied.

The repo the comment lives on is captured on the feedback source at trigger
time (round-tripping through the run state), so completion handling rebuilds
the SCM client without re-deriving it from the comment URL. Rate-limit-
sensitive orgs skip the reaction reads the 👀 deletion needs and only add 🎉.

Co-Authored-By: Claude <noreply@anthropic.com>
@alexsohn1126 alexsohn1126 force-pushed the alexsohn/react-with-tada-on-comment-after-iteration branch from 587b5df to dd123c6 Compare July 13, 2026 15:29
…-tada-on-comment-after-iteration

# Conflicts:
#	src/sentry/seer/autofix/pr_iteration/types.py
#	src/sentry/tasks/seer/pr_iteration.py
Replace the next()/generator expression for the feedback block and the
list comprehension for the sources with straightforward loops; same
behavior, easier to read.

Co-Authored-By: Claude <noreply@anthropic.com>
@alexsohn1126 alexsohn1126 marked this pull request as ready for review July 13, 2026 17:32
@alexsohn1126 alexsohn1126 requested a review from a team as a code owner July 13, 2026 17:32
Comment thread src/sentry/seer/autofix/on_completion_hook.py Outdated
Comment thread src/sentry/seer/autofix/on_completion_hook.py
A fixed ("github", repo_name) provider tuple can't resolve GitHub
Enterprise repos (provider integrations:github_enterprise), so
completion reactions were silently skipped on GHE. Resolve the
repository by its DB id instead, matching the comment-trigger path.
Comment thread src/sentry/seer/autofix/on_completion_hook.py
for reaction in scm_actions.get_review_comment_reactions(scm, pr, cid)["data"]:
author = reaction["author"]
if reaction["content"] == "eyes" and author and author["id"] == actor_id:
scm_actions.delete_review_comment_reaction(scm, pr, cid, reaction["id"])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The code passes a GitHub reaction ID, which is an integer, to an SCM function that may expect a string. This could cause silent failure when deleting reactions.
Severity: LOW

Suggested Fix

To ensure type consistency, explicitly cast the reaction ID to a string before passing it to the SCM deletion functions. For example, use str(reaction["id"]) when calling scm_actions.delete_review_comment_reaction and scm_actions.delete_pull_request_comment_reaction. This aligns with the pattern used in the GitLab integration path.

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/tasks/seer/pr_iteration.py#L302

Potential issue: The GitHub API returns integer IDs for reactions. The code passes the
`reaction["id"]` directly to `scm_actions.delete_review_comment_reaction` and
`scm_actions.delete_pull_request_comment_reaction` without type conversion. A similar
code path for GitLab explicitly converts the ID to a string, suggesting the SCM
functions may expect string arguments. This potential type mismatch (passing an integer
where a string is expected) would likely cause the reaction deletion to fail silently
within a `try...except` block, leaving the bot's `:eyes:` reaction on the comment
indefinitely.

Also affects:

  • src/sentry/tasks/seer/pr_iteration.py:331~331

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7ce2525. Configure here.

# if we don't check this, we would react tada before the commit lands
_, is_synced = state.has_code_changes()
if not is_synced:
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Terminal push skips comment reaction

Medium Severity

_maybe_react_to_completed_iteration returns whenever has_code_changes() reports unsynced repos, but _send_step_webhook for the same PR iteration still emits ITERATION_COMPLETED when _iteration_terminal_errored_repos is non-empty. After a terminal push failure, the webhook fires while the originating GitHub comment keeps the :eyes: reaction and never gets :hooray:.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7ce2525. Configure here.

Comment on lines +309 to +310
pr_state = state.repo_pr_states.get(repo_name)
pr_number = pr_state.pr_number if pr_state and pr_state.pr_number else 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The code uses the Autofix PR number instead of the original user PR number when swapping comment reactions, causing API calls to fail silently.
Severity: MEDIUM

Suggested Fix

Store the original PR number from the user's pull request when the iteration is triggered. This stored original PR number should then be passed to and used by the _swap_comment_reaction_to_done function to ensure the API call targets the correct pull request where the comment exists.

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/autofix/on_completion_hook.py#L309-L310

Potential issue: When an Autofix iteration completes, the system attempts to swap the
'eyes' reaction on the triggering comment to a 'tada' reaction. However, it incorrectly
uses the PR number of the Sentry-created Autofix PR instead of the user's original PR
where the comment was made. This mismatch in PR numbers causes the GitHub API call to
update the reaction to fail with a 404 error, as the comment ID does not exist on the
Autofix PR. The failure is logged, but the user-facing consequence is that the reaction
is never updated, making it seem as if the iteration is stuck or incomplete.

@linear-code

linear-code Bot commented Jul 13, 2026

Copy link
Copy Markdown

CW-1398

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant