Skip to content

feat(seer): Route reviewer-less Seer PRs via fallback candidate sources - #120202

Open
vaind wants to merge 5 commits into
masterfrom
feat/seer-pr-reviewer-candidates
Open

feat(seer): Route reviewer-less Seer PRs via fallback candidate sources#120202
vaind wants to merge 5 commits into
masterfrom
feat/seer-pr-reviewer-candidates

Conversation

@vaind

@vaind vaind commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Final PR of the stack — the candidate-list plumbing (module, marker storage, request wiring, provenance metrics) already landed in #120185, and cap-exhausted assignment in #120194. This PR only adds the fallback sources.

Problem

A run whose triggering user can't be resolved to a GitHub login — Night Shift and other system runs, or an unlinked identity — never gets a review request at all: the candidate list from #120185 comes back empty and the green event is skipped as no_candidates.

What this does

collect_reviewer_candidates now falls back to people connected to the change when no triggering user resolves, in rank order:

  1. suspect_commit_author — latest suspect-commit GroupOwner for the run's issue → Commit.author.get_username_from_external_id(), falling back to the email-matched Sentry user. They wrote the code being fixed.
  2. code_owner — the PR's changed files (provider get_pull_request_files) matched against the repo's synced ProjectCodeOwners.raw with last-rule-wins CODEOWNERS semantics, ranked by files owned. Only individual @login owners with a linked ExternalActor identity are kept — teams are skipped on purpose (team-level requests are the diffuse thing this project avoids), and the raw file is matched directly so no stack-root mapping is involved.
  3. recent_committer — most frequent recent (90d) committers of the most heavily changed files via the provider's per-path commit listing (get_commits_by_path); Seer checkouts are tarball extracts with no git history, so the provider API is the only source. Logins come from the raw payload, which also lets us drop type: Bot accounts.

A resolvable triggering user still short-circuits as the sole candidate with zero provider calls — the common case behaves exactly as in #120185. The only new ping-a-third-party behavior is confined to runs that were previously unroutable, which is why this ships without its own feature flag, riding the existing review-request flag: turning the expansion off again means reverting one additive commit, and the blast radius of a ranking mistake is limited to PRs nobody would have been asked to review at all.

Provider work is bounded (≤20 changed files fetched, ≤5 per-path history calls, one page each); a failing source is skipped (metric-counted) rather than emptying the list; duplicates keep their highest-ranked provenance.

Everything downstream is unchanged from #120185: lazy decision-time computation, the persisted candidates marker with provenance (fallbacks for re-request, and the data to measure which source's reviewers actually respond before tuning the ranking), the preexisting-reviewer check, the bounded request retry, and the source-tagged metrics — the fallback sources just appear as new source tag values.

This is the chunk that makes night-shift PRs routable at all.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 21, 2026
@sentry

sentry Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Sentry Snapshot Testing

Name Added Removed Changed Renamed Unchanged Skipped Status
sentry-frontend
sentry-frontend
0 0 0 0 451 0 ✅ Unchanged

⚙️ sentry-frontend Snapshot Settings

@vaind vaind changed the title feat(seer): Select review-request reviewer from ranked candidate list feat(seer): Route reviewer-less Seer PRs via fallback candidate sources Jul 21, 2026
@vaind
vaind force-pushed the feat/seer-pr-cap-exhausted-assign branch from 446a29d to aa2640c Compare July 21, 2026 15:52
@vaind
vaind force-pushed the feat/seer-pr-reviewer-candidates branch 2 times, most recently from 20819f5 to 7add9a0 Compare July 21, 2026 16:02
@vaind
vaind force-pushed the feat/seer-pr-cap-exhausted-assign branch 2 times, most recently from 26ce262 to 0d6f6bb Compare July 21, 2026 18:30
Base automatically changed from feat/seer-pr-cap-exhausted-assign to master July 22, 2026 16:10
@vaind
vaind force-pushed the feat/seer-pr-reviewer-candidates branch from 7add9a0 to c3290b2 Compare July 22, 2026 18:27
@linear-code

linear-code Bot commented Jul 22, 2026

Copy link
Copy Markdown

CW-1703

@vaind
vaind marked this pull request as ready for review July 22, 2026 18:27
@vaind
vaind requested a review from a team as a code owner July 22, 2026 18:27
Comment thread src/sentry/seer/autofix/pr_iteration/reviewer_candidates.py
@vaind
vaind requested a review from a team July 23, 2026 08:07
vaind added 2 commits July 23, 2026 10:08
A run whose triggering user can't be resolved to a GitHub login (Night Shift, unlinked identities) previously never got a review request at all. Candidate collection now falls back to people connected to the change — the issue's suspect-commit author, linked individual code owners of the PR's changed files, then their most frequent recent committers via the provider's per-path commit listing — ranked with provenance and bounded in provider cost.

A resolvable triggering user still short-circuits as the sole candidate, so the only new ping-a-third-party behavior is confined to runs that were previously unroutable; that confinement is why this ships without its own feature flag, riding the existing review-request flag.
GitHub ignores character-range, negation, and unescaped-whitespace patterns as documented syntax exceptions (GitLab-style [Section] headers parse to one), so drop those lines like convert_codeowners_syntax does instead of keeping them as inert rules.
@vaind
vaind force-pushed the feat/seer-pr-reviewer-candidates branch from 96ce5db to f06292d Compare July 23, 2026 08:08
Comment thread src/sentry/seer/autofix/pr_iteration/reviewer_candidates.py
build_codeowners_associations validates identities against a single
ProjectCodeOwners.project, so in repos shared by several projects a
linked owner without team access on the arbitrarily-picked project was
dropped. Reviewing happens on GitHub where repo access is what matters,
so resolve the identity link with an org-scoped ExternalActor lookup,
the same way the triggering-user source does.
Comment thread src/sentry/seer/autofix/pr_iteration/reviewer_candidates.py Outdated
asked (e.g. Night Shift, or an identity we can't map to a GitHub login)
does selection fan out to people who didn't opt in, in rank order:

1. ``suspect_commit_author`` — the author of the issue's suspect commit; they

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.

I thought this was considered an unreliable source of info?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmm, I didn't know about this - do you have some more insights/doc reference?

…r source

The raw-payload access sat outside the per-path try/except, so a
malformed provider response for one path aborted the whole source
rather than skipping just that path, unlike the equivalent guard in
_changed_files.

@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 d496359. Configure here.

Comment thread src/sentry/seer/autofix/pr_iteration/reviewer_candidates.py
Schema-only rebuilds (e.g. on team changes) save the row and the
pre_save signal bumps date_updated without refreshing raw, so in
multi-project repos a stale copy could outrank a freshly synced one.
date_synced is the actual fetch timestamp; date_updated remains the
tiebreak for manually uploaded copies where it is NULL.
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