Skip to content

Commit 4ab83b7

Browse files
authored
feat(autofix): Update seer explorer autofix last triggered on completion (#111663)
Instead of updating the timestamp when the job is triggered, wait until the completion webhook fires. This ensured that it only updates when we know there is at least a root cause.
1 parent 65969cb commit 4ab83b7

File tree

6 files changed

+53
-93
lines changed

6 files changed

+53
-93
lines changed

src/sentry/models/group.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,7 @@ class Group(Model):
685685
priority_locked_at = models.DateTimeField(null=True)
686686
seer_fixability_score = models.FloatField(null=True)
687687
seer_autofix_last_triggered = models.DateTimeField(null=True)
688+
# This actually represents the last timestamp when the explorer agent completes a step
688689
seer_explorer_autofix_last_triggered = models.DateTimeField(null=True)
689690

690691
objects: ClassVar[GroupManager] = GroupManager(cache_fields=("id",))

src/sentry/seer/autofix/autofix_agent.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from enum import StrEnum
66
from typing import TYPE_CHECKING, Literal
77

8-
from django.utils import timezone
98
from pydantic import BaseModel
109

1110
from sentry.seer.autofix.artifact_schemas import (
@@ -248,8 +247,6 @@ def trigger_autofix_explorer(
248247
artifact_schema=artifact_schema,
249248
)
250249

251-
group.update(seer_explorer_autofix_last_triggered=timezone.now())
252-
253250
payload = {
254251
"run_id": run_id,
255252
"group_id": group.id,

src/sentry/seer/autofix/on_completion_hook.py

Lines changed: 27 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import logging
44
from typing import TYPE_CHECKING
55

6+
from django.utils import timezone
7+
68
from sentry import features
79
from sentry.models.group import Group
810
from sentry.models.organization import Organization
@@ -76,13 +78,20 @@ def execute(cls, organization: Organization, run_id: int) -> None:
7678
)
7779
return
7880

81+
group_id = state.metadata.get("group_id") if state.metadata else None
82+
if group_id is None:
83+
return
84+
85+
group = Group.objects.get(id=group_id, project__organization_id=organization.id)
86+
group.update(seer_explorer_autofix_last_triggered=timezone.now())
87+
7988
# Send webhook for the completed step
80-
cls._send_step_webhook(organization, run_id, state)
89+
cls._send_step_webhook(organization, run_id, state, group)
8190

82-
cls._maybe_trigger_supergroups_embedding(organization, run_id, state)
91+
cls._maybe_trigger_supergroups_embedding(organization, run_id, state, group)
8392

8493
# Continue the automated pipeline if stopping_point hasn't been reached
85-
cls._maybe_continue_pipeline(organization, run_id, state)
94+
cls._maybe_continue_pipeline(organization, run_id, state, group)
8695

8796
@classmethod
8897
def find_latest_artifact_for_step(cls, state: SeerRunState, key: str) -> Artifact | None:
@@ -95,19 +104,24 @@ def find_latest_artifact_for_step(cls, state: SeerRunState, key: str) -> Artifac
95104
return None
96105

97106
@classmethod
98-
def _send_step_webhook(cls, organization, run_id, state: SeerRunState):
107+
def _send_step_webhook(
108+
cls,
109+
organization: Organization,
110+
run_id: int,
111+
state: SeerRunState,
112+
group: Group,
113+
):
99114
"""
100115
Send webhook for the completed step.
101116
102117
Determines which step just completed and sends the appropriate webhook event.
103118
"""
104119
current_step = cls._get_current_step(state)
105120

106-
webhook_payload = {"run_id": run_id}
107-
108-
group_id = state.metadata.get("group_id") if state.metadata else None
109-
if group_id is not None:
110-
webhook_payload["group_id"] = group_id
121+
webhook_payload = {
122+
"run_id": run_id,
123+
"group_id": group.id,
124+
}
111125

112126
# Iterate through blocks in reverse order (most recent first)
113127
# to find which step just completed
@@ -213,25 +227,13 @@ def _maybe_trigger_supergroups_embedding(
213227
organization: Organization,
214228
run_id: int,
215229
state: SeerRunState,
230+
group: Group,
216231
) -> None:
217232
"""Trigger supergroups embedding if feature flag is enabled."""
218233
current_step = cls._get_current_step(state)
219234
if current_step != AutofixStep.ROOT_CAUSE:
220235
return
221236

222-
group_id = state.metadata.get("group_id") if state.metadata else None
223-
if group_id is None:
224-
return
225-
226-
try:
227-
group = Group.objects.get(id=group_id)
228-
except Group.DoesNotExist:
229-
logger.warning(
230-
"autofix.supergroup_embedding.group_not_found",
231-
extra={"group_id": group_id},
232-
)
233-
return
234-
235237
if not features.has("projects:supergroup-embeddings-explorer", group.project):
236238
return
237239

@@ -242,7 +244,7 @@ def _maybe_trigger_supergroups_embedding(
242244
try:
243245
trigger_supergroups_embedding(
244246
organization_id=organization.id,
245-
group_id=group_id,
247+
group_id=group.id,
246248
project_id=group.project_id,
247249
artifact_data=root_cause_artifact.data,
248250
)
@@ -252,7 +254,7 @@ def _maybe_trigger_supergroups_embedding(
252254
extra={
253255
"run_id": run_id,
254256
"organization_id": organization.id,
255-
"group_id": group_id,
257+
"group_id": group.id,
256258
},
257259
)
258260

@@ -289,6 +291,7 @@ def _maybe_continue_pipeline(
289291
organization: Organization,
290292
run_id: int,
291293
state: SeerRunState,
294+
group: Group,
292295
) -> None:
293296
"""
294297
Continue to the next step if stopping_point hasn't been reached.
@@ -307,28 +310,6 @@ def _maybe_continue_pipeline(
307310
return
308311

309312
stopping_point = AutofixStoppingPoint(metadata["stopping_point"])
310-
group_id = metadata.get("group_id")
311-
312-
if not group_id:
313-
logger.warning(
314-
"autofix.on_completion_hook.no_group_id_in_metadata",
315-
extra={"run_id": run_id, "organization_id": organization.id},
316-
)
317-
return
318-
319-
# Get the group
320-
try:
321-
group = Group.objects.get(id=group_id, project__organization=organization)
322-
except Group.DoesNotExist:
323-
logger.warning(
324-
"autofix.on_completion_hook.group_not_found",
325-
extra={
326-
"run_id": run_id,
327-
"organization_id": organization.id,
328-
"group_id": group_id,
329-
},
330-
)
331-
return
332313

333314
if current_step is None:
334315
logger.warning(

src/sentry/seer/autofix/trigger.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@
1111
)
1212
from sentry.utils.cache import cache
1313

14+
# This timeout should be longer than what we expect the seer agents to take.
15+
# This is because we do not want to trigger another run while the previous
16+
# run is still running.
17+
#
18+
# NOTE: The timeout on the seer job is 5 minutes.
19+
SEER_AUTOMATION_TIMEOUT_SECONDS = 30 * 60
20+
1421
if TYPE_CHECKING:
1522
from sentry.models.group import Group
1623
from sentry.utils.locking.manager import LockManager
@@ -171,7 +178,7 @@ def get_seat_based_seer_automation_skip_reason(
171178

172179
# Atomically set cache to prevent duplicate dispatches (returns False if key exists)
173180
automation_dispatch_cache_key = f"seer-automation-dispatched:{group.id}"
174-
if not cache.add(automation_dispatch_cache_key, True, timeout=300):
181+
if not cache.add(automation_dispatch_cache_key, True, timeout=SEER_AUTOMATION_TIMEOUT_SECONDS):
175182
return "automation_already_dispatched" # Another process already dispatched automation
176183

177184
# Check if project has connected repositories - requirement for new pricing

tests/sentry/seer/autofix/test_autofix_agent.py

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -354,30 +354,6 @@ def test_trigger_autofix_explorer_passes_group_id_in_metadata(
354354
call_kwargs = mock_client.start_run.call_args.kwargs
355355
assert call_kwargs["metadata"] == {"group_id": self.group.id, "referrer": "unknown"}
356356

357-
@patch("sentry.seer.autofix.autofix_agent.broadcast_webhooks_for_organization.delay")
358-
@patch("sentry.seer.autofix.autofix_agent.SeerExplorerClient")
359-
def test_trigger_autofix_explorer_updates_explorer_last_triggered(
360-
self, mock_client_class, mock_broadcast
361-
):
362-
"""trigger_autofix_explorer sets seer_explorer_autofix_last_triggered on the group."""
363-
mock_client = MagicMock()
364-
mock_client_class.return_value = mock_client
365-
mock_client.start_run.return_value = 123
366-
367-
assert self.group.seer_explorer_autofix_last_triggered is None
368-
369-
trigger_autofix_explorer(
370-
group=self.group,
371-
step=AutofixStep.ROOT_CAUSE,
372-
referrer=AutofixReferrer.UNKNOWN,
373-
run_id=None,
374-
)
375-
376-
self.group.refresh_from_db()
377-
378-
assert self.group.seer_autofix_last_triggered is None
379-
assert self.group.seer_explorer_autofix_last_triggered is not None
380-
381357

382358
class TestTriggerCodingAgentHandoff(TestCase):
383359
"""Tests for trigger_coding_agent_handoff function."""

tests/sentry/seer/autofix/test_autofix_on_completion_hook.py

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,14 @@ def setUp(self):
200200
def test_maybe_continue_pipeline_no_metadata(self, mock_trigger):
201201
"""Does not continue when metadata is missing."""
202202
state = run_state(blocks=[root_cause_memory_block()])
203-
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
203+
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
204204
mock_trigger.assert_not_called()
205205

206206
@patch("sentry.seer.autofix.on_completion_hook.trigger_autofix_explorer")
207207
def test_maybe_continue_pipeline_no_stopping_point_in_metadata(self, mock_trigger):
208208
"""Does not continue when stopping_point is missing from metadata."""
209209
state = run_state(blocks=[root_cause_memory_block()], metadata={"group_id": self.group.id})
210-
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
210+
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
211211
mock_trigger.assert_not_called()
212212

213213
@patch("sentry.seer.autofix.on_completion_hook.trigger_autofix_explorer")
@@ -220,7 +220,7 @@ def test_maybe_continue_pipeline_at_stopping_point(self, mock_trigger):
220220
"stopping_point": AutofixStoppingPoint.ROOT_CAUSE.value,
221221
},
222222
)
223-
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
223+
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
224224
mock_trigger.assert_not_called()
225225

226226
@patch("sentry.seer.autofix.on_completion_hook.get_project_seer_preferences")
@@ -237,7 +237,7 @@ def test_maybe_continue_pipeline_continues_to_next_step(self, mock_trigger, mock
237237
"stopping_point": AutofixStoppingPoint.CODE_CHANGES.value,
238238
},
239239
)
240-
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
240+
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
241241
mock_trigger.assert_called_once()
242242
call_kwargs = mock_trigger.call_args.kwargs
243243
assert call_kwargs["group"].id == self.group.id
@@ -258,7 +258,7 @@ def test_maybe_continue_pipeline_pushes_changes_for_open_pr(self, mock_push_chan
258258
"stopping_point": AutofixStoppingPoint.OPEN_PR.value,
259259
},
260260
)
261-
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
261+
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
262262
mock_push_changes.assert_called_once_with(
263263
self.group,
264264
123,
@@ -290,6 +290,8 @@ class TestAutofixOnCompletionHookWebhooks(TestCase):
290290
def setUp(self):
291291
super().setUp()
292292
self.organization = self.create_organization()
293+
self.project = self.create_project(organization=self.organization)
294+
self.group = self.create_group(project=self.project)
293295

294296
@patch("sentry.seer.autofix.on_completion_hook.broadcast_webhooks_for_organization.delay")
295297
def test_send_step_webhook_artifact_types(self, mock_broadcast):
@@ -328,7 +330,7 @@ class TestCaseDict(TypedDict):
328330
for i, test_case in enumerate(test_cases):
329331
mock_broadcast.reset_mock()
330332
state = run_state(blocks=[test_case["block"]])
331-
AutofixOnCompletionHook._send_step_webhook(self.organization, run_id, state)
333+
AutofixOnCompletionHook._send_step_webhook(self.organization, run_id, state, self.group)
332334

333335
mock_broadcast.assert_called_once()
334336
call_kwargs = mock_broadcast.call_args.kwargs
@@ -352,7 +354,7 @@ def test_send_step_webhook_coding(self, mock_broadcast):
352354
code_changes_memory_block(),
353355
]
354356
)
355-
AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state)
357+
AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state, self.group)
356358

357359
mock_broadcast.assert_called_once()
358360
call_kwargs = mock_broadcast.call_args.kwargs
@@ -371,7 +373,7 @@ def test_send_step_webhook_no_artifacts_no_webhook(self, mock_broadcast):
371373
artifacts=[],
372374
)
373375
state = run_state(blocks=[block])
374-
AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state)
376+
AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state, self.group)
375377

376378
mock_broadcast.assert_not_called()
377379

@@ -391,7 +393,9 @@ def test_triggers_embedding_on_root_cause(self, mock_trigger_sg):
391393
"""Triggers supergroups embedding when root cause completes with feature flag enabled."""
392394
block = root_cause_memory_block()
393395
state = run_state(blocks=[block], metadata={"group_id": self.group.id})
394-
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(self.organization, 123, state)
396+
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(
397+
self.organization, 123, state, self.group
398+
)
395399

396400
mock_trigger_sg.assert_called_once_with(
397401
organization_id=self.organization.id,
@@ -407,15 +411,9 @@ def test_skips_embedding_when_flag_disabled(self, mock_trigger_sg):
407411
blocks=[root_cause_memory_block()],
408412
metadata={"group_id": self.group.id},
409413
)
410-
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(self.organization, 123, state)
411-
412-
mock_trigger_sg.assert_not_called()
413-
414-
@patch("sentry.seer.autofix.on_completion_hook.trigger_supergroups_embedding")
415-
def test_skips_embedding_when_no_group_id(self, mock_trigger_sg):
416-
"""Does not trigger supergroups embedding when group_id is missing from metadata."""
417-
state = run_state(blocks=[root_cause_memory_block()])
418-
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(self.organization, 123, state)
414+
AutofixOnCompletionHook._maybe_trigger_supergroups_embedding(
415+
self.organization, 123, state, self.group
416+
)
419417

420418
mock_trigger_sg.assert_not_called()
421419

@@ -559,7 +557,7 @@ def test_maybe_continue_pipeline_triggers_handoff_when_configured(
559557
},
560558
)
561559

562-
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state)
560+
AutofixOnCompletionHook._maybe_continue_pipeline(self.organization, 123, state, self.group)
563561

564562
mock_trigger_handoff.assert_called_once()
565563

0 commit comments

Comments
 (0)