Skip to content

Commit b089807

Browse files
vaindclaude
andcommitted
fix(pr-metrics): Send the canonical Seer RepoDefinition in the judge forward
The judge forward sent a flat repo dict with a combined "owner/repo" name, which won't validate into Seer's RepoDefinition — it requires owner and name split, a bare provider slug, and the base/org/privacy fields. Reuse code review's repo definition builder so the forward emits the exact shape Seer already parses, with no normalization on the Seer side. Promote build_repo_definition to public and make its event_payload optional: the forward runs in a task with no webhook payload, so it leaves is_private unknown. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5fefe72 commit b089807

4 files changed

Lines changed: 29 additions & 22 deletions

File tree

src/sentry/pr_metrics/judge.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
from sentry.net.http import connection_from_url
3333
from sentry.pr_metrics.attribution import record_attribution_signal
3434
from sentry.pr_metrics.emit import active_attributions, emit_pr_metrics_row, resolved_group_ids
35+
from sentry.seer.code_review.utils import build_repo_definition
3536
from sentry.seer.signed_seer_api import SeerViewerContext, make_signed_seer_api_request
3637
from sentry.utils import metrics
3738

@@ -81,14 +82,10 @@ def _build_judge_request(pull_request: PullRequest, repository: Repository) -> d
8182
"organization_id": pull_request.organization_id,
8283
"repository_id": pull_request.repository_id,
8384
"pull_request_id": pull_request.id,
84-
"repo": {
85-
"provider": repository.provider,
86-
"external_id": repository.external_id,
87-
"name": repository.name,
88-
"integration_id": (
89-
str(repository.integration_id) if repository.integration_id is not None else None
90-
),
91-
},
85+
# The shared Seer RepoDefinition shape (split owner/name, bare provider
86+
# slug) so Seer parses it directly; head_commit_sha is the PR tip Seer
87+
# resolves the repo at, with the merge/head SHAs also sent below.
88+
"repo": build_repo_definition(repository, head_commit_sha),
9289
"pr_number": pull_request.key,
9390
"close_action": close_action,
9491
"head_commit_sha": head_commit_sha,

src/sentry/seer/code_review/utils.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ def _common_codegen_request_payload(
267267
event_payload: Mapping[str, Any],
268268
) -> dict[str, Any]:
269269
data: dict[str, Any] = {
270-
"repo": _build_repo_definition(repo, target_commit_sha, event_payload),
270+
"repo": build_repo_definition(repo, target_commit_sha, event_payload),
271271
"bug_prediction_specific_information": {
272272
"organization_id": organization.id,
273273
"organization_slug": organization.slug,
@@ -356,17 +356,22 @@ def transform_pull_request_to_codegen_request(
356356
return payload
357357

358358

359-
def _build_repo_definition(
359+
def build_repo_definition(
360360
repo: Repository,
361361
target_commit_sha: str,
362-
event_payload: Mapping[str, Any],
362+
event_payload: Mapping[str, Any] | None = None,
363363
) -> dict[str, Any]:
364364
"""
365-
Build the repository definition for code review requests.
365+
Build the Seer RepoDefinition for a repository.
366366
367367
GitHub and GitLab expose repo identity and visibility differently, so each
368368
provider has its own builder. Anything unrecognized falls back to GitHub.
369+
370+
``event_payload`` supplies only the repo's privacy flag; a payload-less caller
371+
(e.g. the PR metrics judge forward, which runs in a task off the webhook) may
372+
omit it, leaving ``is_private`` unknown.
369373
"""
374+
event_payload = event_payload or {}
370375
# repo.provider uses the "integrations:<slug>" format; Seer expects the bare slug
371376
provider = repo.provider.removeprefix("integrations:") if repo.provider else "github"
372377

tests/sentry/pr_metrics/test_judge.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,10 +326,15 @@ def test_forwards_terminal_facts_and_repo_identity(self, mock_request: Any) -> N
326326
assert body["close_action"] == "merged"
327327
assert body["head_commit_sha"] == HEAD_SHA
328328
assert body["merge_commit_sha"] == MERGE_SHA
329+
# The shared Seer RepoDefinition shape: split owner/name and bare provider slug.
329330
assert body["repo"] == {
330-
"provider": "integrations:github",
331+
"provider": "github",
332+
"owner": "getsentry",
333+
"name": "sentry",
331334
"external_id": "10270250",
332-
"name": "getsentry/sentry",
335+
"base_commit_sha": HEAD_SHA,
336+
"organization_id": self.organization.id,
337+
"is_private": None,
333338
"integration_id": "99",
334339
}
335340
assert body["additions"] == 12

tests/sentry/seer/code_review/test_utils.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -866,34 +866,34 @@ def _make_repo(self, provider: str = "integrations:github") -> MagicMock:
866866
return repo
867867

868868
def test_provider_is_github_for_cloud_integration(self) -> None:
869-
from sentry.seer.code_review.utils import _build_repo_definition
869+
from sentry.seer.code_review.utils import build_repo_definition
870870

871-
result = _build_repo_definition(
871+
result = build_repo_definition(
872872
repo=self._make_repo("integrations:github"),
873873
target_commit_sha="abc123",
874874
event_payload={"repository": {"private": False}},
875875
)
876876
assert result["provider"] == "github"
877877

878878
def test_provider_is_github_enterprise_for_ghe_integration(self) -> None:
879-
from sentry.seer.code_review.utils import _build_repo_definition
879+
from sentry.seer.code_review.utils import build_repo_definition
880880

881-
result = _build_repo_definition(
881+
result = build_repo_definition(
882882
repo=self._make_repo("integrations:github_enterprise"),
883883
target_commit_sha="abc123",
884884
event_payload={"repository": {"private": False}},
885885
)
886886
assert result["provider"] == "github_enterprise"
887887

888888
def test_gitlab_uses_config_path_not_display_name(self) -> None:
889-
from sentry.seer.code_review.utils import _build_repo_definition
889+
from sentry.seer.code_review.utils import build_repo_definition
890890

891891
repo = self._make_repo("integrations:gitlab")
892892
# Repository.name is the display "name_with_namespace"; it must not be used.
893893
repo.name = "Cool Group / Sentry"
894894
repo.config = {"path": "cool-group/sentry"}
895895

896-
result = _build_repo_definition(
896+
result = build_repo_definition(
897897
repo=repo,
898898
target_commit_sha="abc123",
899899
event_payload={"project": {"visibility_level": 0}},
@@ -903,14 +903,14 @@ def test_gitlab_uses_config_path_not_display_name(self) -> None:
903903
assert result["name"] == "sentry"
904904

905905
def test_gitlab_missing_config_path_raises_rather_than_using_display_name(self) -> None:
906-
from sentry.seer.code_review.utils import _build_repo_definition
906+
from sentry.seer.code_review.utils import build_repo_definition
907907

908908
repo = self._make_repo("integrations:gitlab")
909909
repo.name = "Cool Group / Sentry"
910910
repo.config = {}
911911

912912
with pytest.raises(ValueError):
913-
_build_repo_definition(
913+
build_repo_definition(
914914
repo=repo,
915915
target_commit_sha="abc123",
916916
event_payload={"project": {}},

0 commit comments

Comments
 (0)