Skip to content

feat(preprod): Add only-if-diff toggle for snapshot PR comments (EME-1046)#114035

Merged
runningcode merged 2 commits intomasterfrom
no/eme-1046-snapshot-pr-comments-only-if-diff
Apr 28, 2026
Merged

feat(preprod): Add only-if-diff toggle for snapshot PR comments (EME-1046)#114035
runningcode merged 2 commits intomasterfrom
no/eme-1046-snapshot-pr-comments-only-if-diff

Conversation

@runningcode
Copy link
Copy Markdown
Contributor

@runningcode runningcode commented Apr 27, 2026

What

Adds a second per-project option, sentry:preprod_snapshot_pr_comments_only_if_diff (default False), surfaced as preprodSnapshotPrCommentsOnlyIfDiff on the project details API. When enabled, create_preprod_snapshot_pr_comment_task short-circuits before posting if the comparison reports no changes across any sibling artifact for the PR.

Why

EME-1046. Today the only knob is "PR comments on/off". Some teams want the comment to appear only when there is something interesting to look at, so they can keep PR comments enabled by default without flooding no-op PRs.

…1046)

Adds a second per-project option, sentry:preprod_snapshot_pr_comments_only_if_diff
(default False). When enabled, the snapshot PR comment task short-circuits
before posting if the comparison reports no changes for any sibling artifact
on the PR. Existing comments from prior runs are left in place.

Backend slice; frontend toggle ships in a follow-up PR.
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Apr 27, 2026

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 27, 2026
@runningcode runningcode marked this pull request as ready for review April 27, 2026 14:27
@runningcode runningcode requested review from a team as code owners April 27, 2026 14:27
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

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 7c52316. Configure here.

Comment thread src/sentry/preprod/vcs/pr_comments/snapshot_tasks.py Outdated
Comment thread src/sentry/preprod/vcs/pr_comments/snapshot_tasks.py Outdated
When the only-if-diff toggle is enabled, the early-return checked
`not any(changes_map.values())`. `build_changes_map` only populates
entries for SUCCESS comparisons, so when comparisons fail the map is
empty and the failure comment is silently dropped — even though
`compare_snapshots` enqueues this task on its failure path and the
template renders a meaningful row for FAILED state.

Also check `comparisons_map` for FAILED entries so the comment is still
posted to surface the failure.
Copy link
Copy Markdown
Contributor

@cameroncooke cameroncooke left a comment

Choose a reason for hiding this comment

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

LGTM!

logger = logging.getLogger(__name__)

ENABLED_OPTION_KEY = "sentry:preprod_snapshot_pr_comments_enabled"
ONLY_IF_DIFF_OPTION_KEY = "sentry:preprod_snapshot_pr_comments_only_if_diff"
Copy link
Copy Markdown
Contributor

@cameroncooke cameroncooke Apr 28, 2026

Choose a reason for hiding this comment

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

I see we have this constant but we also have multiple places that use the key inline, though I can see we're just following the existing pattern so don't think we need to do anything. Maybe a future improvement would be to use a global constant as a single source of truth for these values.

@runningcode runningcode merged commit e1d6d4f into master Apr 28, 2026
77 checks passed
@runningcode runningcode deleted the no/eme-1046-snapshot-pr-comments-only-if-diff branch April 28, 2026 08:29
cleptric pushed a commit that referenced this pull request May 5, 2026
…1046) (#114035)

## What

Adds a second per-project option,
`sentry:preprod_snapshot_pr_comments_only_if_diff` (default `False`),
surfaced as `preprodSnapshotPrCommentsOnlyIfDiff` on the project details
API. When enabled, `create_preprod_snapshot_pr_comment_task`
short-circuits before posting if the comparison reports no changes
across any sibling artifact for the PR.

## Why

EME-1046. Today the only knob is "PR comments on/off". Some teams want
the comment to appear only when there is something interesting to look
at, so they can keep PR comments enabled by default without flooding
no-op PRs.
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.

2 participants