Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 16 additions & 32 deletions src/sentry/seer/autofix/issue_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,13 +314,18 @@ def _run_automation(
}
)

fixability_score = group.seer_fixability_score
if fixability_score is None:
logger.error("Fixability score is not available for group %s", group.id)
return
with sentry_sdk.start_span(op="ai_summary.generate_fixability_score"):
issue_summary = _generate_fixability_score(group)

if not issue_summary.scores:
raise ValueError("Issue summary scores is None or empty.")
if issue_summary.scores.fixability_score is None:
raise ValueError("Issue summary fixability score is None.")

group.update(seer_fixability_score=issue_summary.scores.fixability_score)
Comment on lines +317 to +325
Copy link

Choose a reason for hiding this comment

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

Bug: The _generate_fixability_score function is unconditionally called in _run_automation, causing redundant Seer API requests.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The _generate_fixability_score function, which makes an expensive signed HTTP request to the Seer API, is now unconditionally called within _run_automation (lines 317-325). This occurs on every automation trigger, even if group.seer_fixability_score is already populated, leading to increased load on the Seer API, unnecessary latency, and reduced reliability. This regression likely caused canary deploy failures due to timeouts or rate limiting.

💡 Suggested Fix

Reintroduce a check within _run_automation to only call _generate_fixability_score if group.seer_fixability_score is None or if the source explicitly requires a fresh score, similar to the previous guarded logic.

🤖 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/issue_summary.py#L317-L325

Potential issue: The `_generate_fixability_score` function, which makes an expensive
signed HTTP request to the Seer API, is now unconditionally called within
`_run_automation` (lines 317-325). This occurs on every automation trigger, even if
`group.seer_fixability_score` is already populated, leading to increased load on the
Seer API, unnecessary latency, and reduced reliability. This regression likely caused
canary deploy failures due to timeouts or rate limiting.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2817270


if (
not _is_issue_fixable(group, fixability_score)
not _is_issue_fixable(group, issue_summary.scores.fixability_score)
and not group.issue_type.always_trigger_seer_automation
):
return
Expand All @@ -342,7 +347,9 @@ def _run_automation(

stopping_point = None
if features.has("projects:triage-signals-v0", group.project):
fixability_stopping_point = _get_stopping_point_from_fixability(fixability_score)
fixability_stopping_point = _get_stopping_point_from_fixability(
issue_summary.scores.fixability_score
)
logger.info("Fixability-based stopping point: %s", fixability_stopping_point)

# Fetch user preference and apply as upper bound
Expand Down Expand Up @@ -394,35 +401,12 @@ def _generate_summary(
trace_tree,
)

if source != SeerAutomationSource.ISSUE_DETAILS and group.seer_fixability_score is None:
if should_run_automation:
try:
with sentry_sdk.start_span(op="ai_summary.generate_fixability_score"):
fixability_response = _generate_fixability_score(group)

if not fixability_response.scores:
raise ValueError("Issue summary scores is None or empty.")
if fixability_response.scores.fixability_score is None:
raise ValueError("Issue summary fixability score is None.")

group.update(seer_fixability_score=fixability_response.scores.fixability_score)
_run_automation(group, user, event, source)
except Exception:
logger.exception(
"Error generating fixability score in summary", extra={"group_id": group.id}
)

if should_run_automation:
if group.seer_fixability_score is not None:
try:
_run_automation(group, user, event, source)
except Exception:
logger.exception(
"Error auto-triggering autofix from issue summary", extra={"group_id": group.id}
)
else:
logger.error(
"Skipping automation: fixability score unavailable for group %s",
group.id,
extra={"group_id": group.id},
"Error auto-triggering autofix from issue summary", extra={"group_id": group.id}
)

summary_dict = issue_summary.dict()
Expand Down
193 changes: 67 additions & 126 deletions tests/sentry/seer/autofix/test_issue_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -641,16 +641,11 @@ def test_get_issue_summary_continues_when_automation_fails(
)
mock_call_seer.return_value = mock_summary

# Set fixability score so _run_automation will be called
self.group.update(seer_fixability_score=0.75)

# Make _run_automation raise an exception
mock_run_automation.side_effect = Exception("Automation failed")

# Call get_issue_summary with a source that triggers automation
summary_data, status_code = get_issue_summary(
self.group, self.user, source=SeerAutomationSource.POST_PROCESS
)
# Call get_issue_summary and verify it still returns successfully
summary_data, status_code = get_issue_summary(self.group, self.user)

assert status_code == 200
expected_response = mock_summary.dict()
Expand Down Expand Up @@ -755,105 +750,6 @@ def test_get_issue_summary_with_should_run_automation_false(
cached_summary = cache.get(f"ai-group-summary-v2:{self.group.id}")
assert cached_summary == expected_response_summary

@patch("sentry.seer.autofix.issue_summary.get_seer_org_acknowledgement")
@patch("sentry.seer.autofix.issue_summary._generate_fixability_score")
@patch("sentry.seer.autofix.issue_summary._get_trace_tree_for_event")
@patch("sentry.seer.autofix.issue_summary._call_seer")
@patch("sentry.seer.autofix.issue_summary._get_event")
def test_generate_summary_fixability_generation(
self,
mock_get_event,
mock_call_seer,
mock_get_trace_tree,
mock_generate_fixability,
mock_get_acknowledgement,
):
"""Test fixability generation: creates when missing, skips when exists."""
mock_get_acknowledgement.return_value = True
event = Mock(event_id="test_event_id", datetime=datetime.datetime.now())
serialized_event = {"event_id": "test_event_id", "data": "test_event_data"}
mock_get_event.return_value = [serialized_event, event]
mock_summary = SummarizeIssueResponse(
group_id=str(self.group.id),
headline="Test headline",
whats_wrong="Test whats wrong",
trace="Test trace",
possible_cause="Test possible cause",
)
mock_call_seer.return_value = mock_summary
mock_get_trace_tree.return_value = None
mock_generate_fixability.return_value = SummarizeIssueResponse(
group_id=str(self.group.id),
headline="h",
whats_wrong="w",
trace="t",
possible_cause="c",
scores=SummarizeIssueScores(fixability_score=0.75),
)

# Test 1: Generates fixability when missing
assert self.group.seer_fixability_score is None
get_issue_summary(
self.group,
self.user,
source=SeerAutomationSource.POST_PROCESS,
should_run_automation=False,
)
mock_generate_fixability.assert_called_once_with(self.group)
self.group.refresh_from_db()
assert self.group.seer_fixability_score == 0.75

# Test 2: Skips fixability when already exists
mock_generate_fixability.reset_mock()
cache.delete(f"ai-group-summary-v2:{self.group.id}")
get_issue_summary(
self.group,
self.user,
source=SeerAutomationSource.POST_PROCESS,
should_run_automation=False,
)
mock_generate_fixability.assert_not_called()

@patch("sentry.seer.autofix.issue_summary.get_seer_org_acknowledgement")
@patch("sentry.seer.autofix.issue_summary._generate_fixability_score")
@patch("sentry.seer.autofix.issue_summary._get_trace_tree_for_event")
@patch("sentry.seer.autofix.issue_summary._call_seer")
@patch("sentry.seer.autofix.issue_summary._get_event")
def test_generate_summary_continues_when_fixability_fails(
self,
mock_get_event,
mock_call_seer,
mock_get_trace_tree,
mock_generate_fixability,
mock_get_acknowledgement,
):
"""Test that summary is still cached when fixability generation fails."""
mock_get_acknowledgement.return_value = True
event = Mock(event_id="test_event_id", datetime=datetime.datetime.now())
serialized_event = {"event_id": "test_event_id", "data": "test_event_data"}
mock_get_event.return_value = [serialized_event, event]
mock_summary = SummarizeIssueResponse(
group_id=str(self.group.id),
headline="Test headline",
whats_wrong="Test whats wrong",
trace="Test trace",
possible_cause="Test possible cause",
)
mock_call_seer.return_value = mock_summary
mock_get_trace_tree.return_value = None
mock_generate_fixability.side_effect = Exception("Fixability service down")

summary_data, status_code = get_issue_summary(
self.group,
self.user,
source=SeerAutomationSource.POST_PROCESS,
should_run_automation=False,
)

assert status_code == 200
assert summary_data["headline"] == "Test headline"
mock_generate_fixability.assert_called_once()


class TestGetStoppingPointFromFixability:
@pytest.mark.parametrize(
Expand Down Expand Up @@ -889,12 +785,22 @@ def setUp(self) -> None:
)
@patch("sentry.seer.autofix.issue_summary.get_autofix_state", return_value=None)
@patch("sentry.quotas.backend.has_available_reserved_budget", return_value=True)
def test_high_fixability_code_changes(self, mock_budget, mock_state, mock_rate, mock_trigger):
@patch("sentry.seer.autofix.issue_summary._generate_fixability_score")
def test_high_fixability_code_changes(
self, mock_gen, mock_budget, mock_state, mock_rate, mock_trigger
):
self.project.update_option("sentry:autofix_automation_tuning", "always")
self.group.update(seer_fixability_score=0.80)
mock_gen.return_value = SummarizeIssueResponse(
group_id=str(self.group.id),
headline="h",
whats_wrong="w",
trace="t",
possible_cause="c",
scores=SummarizeIssueScores(fixability_score=0.70),
)
_run_automation(self.group, self.user, self.event, SeerAutomationSource.ALERT)
mock_trigger.assert_called_once()
assert mock_trigger.call_args[1]["stopping_point"] == AutofixStoppingPoint.OPEN_PR
assert mock_trigger.call_args[1]["stopping_point"] == AutofixStoppingPoint.CODE_CHANGES

@patch("sentry.seer.autofix.issue_summary._trigger_autofix_task.delay")
@patch(
Expand All @@ -903,9 +809,19 @@ def test_high_fixability_code_changes(self, mock_budget, mock_state, mock_rate,
)
@patch("sentry.seer.autofix.issue_summary.get_autofix_state", return_value=None)
@patch("sentry.quotas.backend.has_available_reserved_budget", return_value=True)
def test_medium_fixability_solution(self, mock_budget, mock_state, mock_rate, mock_trigger):
@patch("sentry.seer.autofix.issue_summary._generate_fixability_score")
def test_medium_fixability_solution(
self, mock_gen, mock_budget, mock_state, mock_rate, mock_trigger
):
self.project.update_option("sentry:autofix_automation_tuning", "always")
self.group.update(seer_fixability_score=0.50)
mock_gen.return_value = SummarizeIssueResponse(
group_id=str(self.group.id),
headline="h",
whats_wrong="w",
trace="t",
possible_cause="c",
scores=SummarizeIssueScores(fixability_score=0.50),
)
_run_automation(self.group, self.user, self.event, SeerAutomationSource.ALERT)
mock_trigger.assert_called_once()
assert mock_trigger.call_args[1]["stopping_point"] == AutofixStoppingPoint.SOLUTION
Expand All @@ -917,9 +833,17 @@ def test_medium_fixability_solution(self, mock_budget, mock_state, mock_rate, mo
)
@patch("sentry.seer.autofix.issue_summary.get_autofix_state", return_value=None)
@patch("sentry.quotas.backend.has_available_reserved_budget", return_value=True)
def test_without_feature_flag(self, mock_budget, mock_state, mock_rate, mock_trigger):
@patch("sentry.seer.autofix.issue_summary._generate_fixability_score")
def test_without_feature_flag(self, mock_gen, mock_budget, mock_state, mock_rate, mock_trigger):
self.project.update_option("sentry:autofix_automation_tuning", "always")
self.group.update(seer_fixability_score=0.80)
mock_gen.return_value = SummarizeIssueResponse(
group_id=str(self.group.id),
headline="h",
whats_wrong="w",
trace="t",
possible_cause="c",
scores=SummarizeIssueScores(fixability_score=0.80),
)

with self.feature(
{"organizations:gen-ai-features": True, "projects:triage-signals-v0": False}
Expand All @@ -929,13 +853,6 @@ def test_without_feature_flag(self, mock_budget, mock_state, mock_rate, mock_tri
mock_trigger.assert_called_once()
assert mock_trigger.call_args[1]["stopping_point"] is None

@patch("sentry.seer.autofix.issue_summary._trigger_autofix_task.delay")
def test_missing_fixability_score_returns_early(self, mock_trigger):
"""Test that _run_automation returns early when fixability score is None."""
assert self.group.seer_fixability_score is None
_run_automation(self.group, self.user, self.event, SeerAutomationSource.ALERT)
mock_trigger.assert_not_called()


class TestFetchUserPreference:
@patch("sentry.seer.autofix.issue_summary.sign_with_seer_secret", return_value={})
Expand Down Expand Up @@ -1068,12 +985,20 @@ def setUp(self) -> None:
)
@patch("sentry.seer.autofix.issue_summary.get_autofix_state", return_value=None)
@patch("sentry.quotas.backend.has_available_reserved_budget", return_value=True)
@patch("sentry.seer.autofix.issue_summary._generate_fixability_score")
def test_user_preference_limits_high_fixability(
self, mock_budget, mock_state, mock_rate, mock_fetch, mock_trigger
self, mock_gen, mock_budget, mock_state, mock_rate, mock_fetch, mock_trigger
):
"""High fixability (OPEN_PR) limited by user preference (SOLUTION)"""
self.project.update_option("sentry:autofix_automation_tuning", "always")
self.group.update(seer_fixability_score=0.80)
mock_gen.return_value = SummarizeIssueResponse(
group_id=str(self.group.id),
headline="h",
whats_wrong="w",
trace="t",
possible_cause="c",
scores=SummarizeIssueScores(fixability_score=0.80), # High = OPEN_PR
)
mock_fetch.return_value = "solution"

_run_automation(self.group, self.user, self.event, SeerAutomationSource.ALERT)
Expand All @@ -1090,12 +1015,20 @@ def test_user_preference_limits_high_fixability(
)
@patch("sentry.seer.autofix.issue_summary.get_autofix_state", return_value=None)
@patch("sentry.quotas.backend.has_available_reserved_budget", return_value=True)
@patch("sentry.seer.autofix.issue_summary._generate_fixability_score")
def test_fixability_limits_permissive_user_preference(
self, mock_budget, mock_state, mock_rate, mock_fetch, mock_trigger
self, mock_gen, mock_budget, mock_state, mock_rate, mock_fetch, mock_trigger
):
"""Medium fixability (SOLUTION) used despite user allowing OPEN_PR"""
self.project.update_option("sentry:autofix_automation_tuning", "always")
self.group.update(seer_fixability_score=0.50)
mock_gen.return_value = SummarizeIssueResponse(
group_id=str(self.group.id),
headline="h",
whats_wrong="w",
trace="t",
possible_cause="c",
scores=SummarizeIssueScores(fixability_score=0.50), # Medium = SOLUTION
)
mock_fetch.return_value = "open_pr"

_run_automation(self.group, self.user, self.event, SeerAutomationSource.ALERT)
Expand All @@ -1112,12 +1045,20 @@ def test_fixability_limits_permissive_user_preference(
)
@patch("sentry.seer.autofix.issue_summary.get_autofix_state", return_value=None)
@patch("sentry.quotas.backend.has_available_reserved_budget", return_value=True)
@patch("sentry.seer.autofix.issue_summary._generate_fixability_score")
def test_no_user_preference_uses_fixability_only(
self, mock_budget, mock_state, mock_rate, mock_fetch, mock_trigger
self, mock_gen, mock_budget, mock_state, mock_rate, mock_fetch, mock_trigger
):
"""When user has no preference, use fixability score alone"""
self.project.update_option("sentry:autofix_automation_tuning", "always")
self.group.update(seer_fixability_score=0.80)
mock_gen.return_value = SummarizeIssueResponse(
group_id=str(self.group.id),
headline="h",
whats_wrong="w",
trace="t",
possible_cause="c",
scores=SummarizeIssueScores(fixability_score=0.80), # High = OPEN_PR
)
mock_fetch.return_value = None

_run_automation(self.group, self.user, self.event, SeerAutomationSource.ALERT)
Expand Down
Loading