diff --git a/src/sentry/issues/derived/aggregators.py b/src/sentry/issues/derived/aggregators.py index f4779331cb30..30095eaeb84c 100644 --- a/src/sentry/issues/derived/aggregators.py +++ b/src/sentry/issues/derived/aggregators.py @@ -97,11 +97,12 @@ def track_status(state: StateView, entry: GroupActionLogEntry) -> AggregatorResu # Progress for open issues (None when closed). # -# Progress is dervived from a few features, which are tracked independently: +# Progress is derived from a few features, which are tracked independently: # # * IS_ASSIGNED — issue has an assignee. Survives close/reopen. # * HAS_ROOT_CAUSE — a root cause has been identified (diagnosed). Cleared on -# regression (SET_REGRESSED), but preserved when manually reopened (UNRESOLVE). +# regression (SET_REGRESSED) and when a Seer-proposed fix is rejected, but +# preserved when manually reopened (UNRESOLVE). # * HAS_OPEN_FIX_PR — at least one PR which resolves the issue is still open. # Cleared when the last linked PR closes without being merged. # @@ -133,10 +134,12 @@ def track_assignment(state: StateView, entry: GroupActionLogEntry) -> Aggregator @aggregator( (HAS_ROOT_CAUSE,), + deps=(LAST_COMPLETED_AUTOFIX_STEP,), scope=( RootCauseIdentifiedAction, SeerRCACompletedAction, SetRegressedAction, + PullRequestClosedAction, ), ) def track_root_cause(state: StateView, entry: GroupActionLogEntry) -> AggregatorResult: @@ -144,11 +147,26 @@ def track_root_cause(state: StateView, entry: GroupActionLogEntry) -> Aggregator Set by ROOT_CAUSE_IDENTIFIED or SEER_RCA_COMPLETED and cleared on regression (SET_REGRESSED), since a regressed issue is a fresh occurrence that needs - re-diagnosing. A manual reopen (UNRESOLVE) preserves the diagnosis. + re-diagnosing. Also cleared when a Seer fix PR closes unmerged: the rejected + fix takes its diagnosis with it. A manual reopen (UNRESOLVE) preserves the + diagnosis. """ - has_root_cause = isinstance(entry.action, (RootCauseIdentifiedAction, SeerRCACompletedAction)) - if has_root_cause != state[HAS_ROOT_CAUSE]: - return emit(HAS_ROOT_CAUSE.value(has_root_cause)) + current = state[HAS_ROOT_CAUSE] + # Per-issue, not per-PR: SeerPRCreatedAction carries raw PR payloads rather + # than PullRequest ids, so a closed PR can't be traced back to a Seer run. + seer_opened_a_pr = state[LAST_COMPLETED_AUTOFIX_STEP] in ( + IssueAutofixStep.PR_CREATED, + IssueAutofixStep.PR_ITERATION, + ) + + match entry.action: + case RootCauseIdentifiedAction() | SeerRCACompletedAction() if not current: + return emit(HAS_ROOT_CAUSE.value(True)) + case SetRegressedAction() if current: + return emit(HAS_ROOT_CAUSE.value(False)) + case PullRequestClosedAction(has_other_open_prs=False) if current and seer_opened_a_pr: + return emit(HAS_ROOT_CAUSE.value(False)) + return None diff --git a/src/sentry/issues/derived/features.py b/src/sentry/issues/derived/features.py index 19c88874c668..b1451efc8ff4 100644 --- a/src/sentry/issues/derived/features.py +++ b/src/sentry/issues/derived/features.py @@ -38,7 +38,7 @@ class IssueStatus(StrEnum): IS_ASSIGNED = Feature[bool]("is_assigned", default=False) # Whether the issue has a root cause identified. -HAS_ROOT_CAUSE = Feature[bool]("has_root_cause", default=False) +HAS_ROOT_CAUSE = Feature[bool]("has_root_cause", default=False, version=1) # The furthest autofix step the issue has reached, from the latest completed LAST_COMPLETED_AUTOFIX_STEP = Feature[IssueAutofixStep]( diff --git a/tests/sentry/issues/derived/test_aggregators.py b/tests/sentry/issues/derived/test_aggregators.py index eaa2f4070a06..f06bb431faf8 100644 --- a/tests/sentry/issues/derived/test_aggregators.py +++ b/tests/sentry/issues/derived/test_aggregators.py @@ -22,6 +22,7 @@ from sentry.issues.derived.aggregators import AGGREGATORS from sentry.issues.derived.features import ( BLOCKER, + HAS_ROOT_CAUSE, LAST_COMPLETED_AUTOFIX_STEP, LAST_PROGRESSED_AT, PROGRESS, @@ -100,6 +101,15 @@ def _pr_closed(has_other: bool | None = None, *, pr_id: int = 101, hour: int = 0 return FakeEntry(type=GroupActionType.PULL_REQUEST_CLOSED, date_added=_ts(hour=hour), data=data) +def _seer_fix_proposed() -> list[FakeEntry]: + """Seer's route to fix_proposed. SEER_PR_CREATED is what marks the PR as Seer's.""" + return [ + FakeEntry(type=GroupActionType.SEER_RCA_COMPLETED), + FakeEntry(type=GroupActionType.SEER_PR_CREATED), + FakeEntry(type=GroupActionType.RESOLVED_IN_PULL_REQUEST, data=_resolved_pr_data(101)), + ] + + def _pr_terminal(action: GroupActionType, has_other: bool | None) -> FakeEntry: data: dict[str, object] = {"pull_request": 101} if has_other is not None: @@ -952,6 +962,92 @@ def test_repropose_after_demotion_returns_to_fix_proposed() -> None: ) +# --------------------------------------------------------------------------- +# A rejected Seer fix invalidates the diagnosis +# --------------------------------------------------------------------------- + + +def test_seer_pr_close_demotes_to_assigned_when_assigned() -> None: + assert ( + _run_for_feature( + PROGRESS, + [ + FakeEntry(type=GroupActionType.ASSIGN), + *_seer_fix_proposed(), + _pr_closed(has_other=False), + ], + ) + == IssueProgressState.ASSIGNED + ) + + +@pytest.mark.parametrize("has_other", [True, None]) +def test_seer_pr_close_with_remaining_or_unknown_prs_preserves_root_cause( + has_other: bool | None, +) -> None: + entries = [*_seer_fix_proposed(), _pr_closed(has_other=has_other)] + assert _run_for_feature(HAS_ROOT_CAUSE, entries) is True + + +@pytest.mark.parametrize( + "action_type", + [ + GroupActionType.PULL_REQUEST_MERGED, + GroupActionType.PULL_REQUEST_UNLINKED, + ], +) +def test_seer_pr_merged_or_unlinked_preserves_root_cause(action_type: int) -> None: + # Merging is progress and unlinking is bookkeeping; neither rejects the fix. + entries = [ + *_seer_fix_proposed(), + FakeEntry(type=action_type, data={"pull_request": 101, "has_other_open_prs": False}), + ] + assert _run_for_feature(HAS_ROOT_CAUSE, entries) is True + + +def test_human_pr_close_preserves_root_cause() -> None: + # Seer never opened a PR here, so a closed PR says nothing about the diagnosis. + entries = [ + FakeEntry(type=GroupActionType.ROOT_CAUSE_IDENTIFIED), + FakeEntry(type=GroupActionType.RESOLVED_IN_PULL_REQUEST, data=_resolved_pr_data(101)), + _pr_closed(has_other=False), + ] + assert _run_for_feature(HAS_ROOT_CAUSE, entries) is True + + +def test_seer_rerun_after_rejected_fix_restores_diagnosed() -> None: + assert ( + _run_for_feature( + PROGRESS, + [ + *_seer_fix_proposed(), + _pr_closed(has_other=False), + FakeEntry(type=GroupActionType.SEER_RCA_COMPLETED), + ], + ) + == IssueProgressState.DIAGNOSED + ) + + +def test_seer_pr_close_demotes_and_reopen_keeps_diagnosis_cleared() -> None: + p = _pipeline(targets=(PROGRESS,)) + state = p.run(_seer_fix_proposed()) + assert state[PROGRESS] == IssueProgressState.FIX_PROPOSED + + state = p.step(state, _pr_closed(has_other=False)) + assert state[PROGRESS] == IssueProgressState.IDENTIFIED + + # Reopening restores the open-PR flag, but the diagnosis stays cleared. + state = p.step( + state, + FakeEntry(type=GroupActionType.PULL_REQUEST_REOPENED, data={"pull_request": 101}), + ) + assert state[PROGRESS] == IssueProgressState.FIX_PROPOSED + + state = p.step(state, _pr_closed(has_other=False)) + assert state[PROGRESS] == IssueProgressState.IDENTIFIED + + def test_last_progressed_at_updated_on_demotion() -> None: p = _pipeline(targets=(PROGRESS,)) state = p.initial_state()