diff --git a/src/sentry/seer/autofix/autofix_agent.py b/src/sentry/seer/autofix/autofix_agent.py index f4a74464539c55..317f0e266c78cd 100644 --- a/src/sentry/seer/autofix/autofix_agent.py +++ b/src/sentry/seer/autofix/autofix_agent.py @@ -231,7 +231,14 @@ def get_latest_iteration_index(state: SeerRunState) -> int: for block in reversed(state.blocks): metadata = block.message.metadata or {} if metadata.get("step") == AutofixStep.PR_ITERATION.value: - return int(metadata["iteration_index"]) + iteration_index = metadata.get("iteration_index") + if iteration_index is None: + logger.error( + "autofix.get_latest_iteration_index.missing_iteration_index", + extra={"run_id": state.run_id}, + ) + return 0 + return int(iteration_index) return 0 @@ -241,27 +248,6 @@ def get_iteration_for_insert_index(state: SeerRunState, insert_index: int) -> in return int(metadata["iteration_index"]) -def recover_iteration_feedback(state: SeerRunState, insert_index: int) -> str | None: - """ - Recover the user feedback that originally triggered a PR iteration. - - When a PR iteration is retried, the frontend truncates the run at the - iteration's first block (``insert_index``). That block carries the original - feedback in its metadata, so we reuse it to retry with the same feedback - rather than dropping it. - """ - if insert_index < 0 or insert_index >= len(state.blocks): - return None - metadata = state.blocks[insert_index].message.metadata - if metadata is None: - return None - raw = metadata.get("feedback") - if raw is None: - return None - # Feedback is stored as a JSON object (``{"text", "username", "timestamp"}``). - return json.loads(raw).get("text") - - def get_autofix_agent_client( group: Group, intelligence_level: Literal["low", "medium", "high"] = "medium", @@ -414,8 +400,7 @@ def trigger_autofix_agent( } if step == AutofixStep.PR_ITERATION and feedback is not None: # Stored as a JSON object so the UI can attribute the feedback to its - # source and show when it was submitted. See ``recover_iteration_feedback`` - # for the read side. + # source and show when it was submitted. prompt_metadata["feedback"] = json.dumps( { "text": feedback.message, diff --git a/src/sentry/seer/autofix/introspection.py b/src/sentry/seer/autofix/introspection.py index 685638d61d0172..7dbadc5c08dd77 100644 --- a/src/sentry/seer/autofix/introspection.py +++ b/src/sentry/seer/autofix/introspection.py @@ -8,7 +8,7 @@ from sentry.models.organization import Organization from sentry.seer.agent.client_models import SeerRunState from sentry.seer.autofix.artifact_schemas import RootCauseArtifact, SolutionArtifact -from sentry.seer.autofix.autofix_agent import AutofixStep +from sentry.seer.autofix.autofix_agent import AutofixStep, get_latest_iteration_index from sentry.seer.models.seer_api_models import SeerApiError from sentry.seer.signed_seer_api import LlmGenerateRequest, make_llm_generate_request from sentry.utils import json, metrics @@ -442,3 +442,86 @@ def introspect_code_changes( ) return None + + +def _iteration_introspection_prompt( + *, + short_id: str, + title: str, + culprit: str, + diffs_by_repo: dict[str, str], +) -> str: + return dedent(f"""\ + You are evaluating whether pull request iteration changes are ready for issue {short_id}: "{title}" (culprit: {culprit}). + + ## Revised Code Changes to Evaluate + {_format_code_changes_section(diffs_by_repo)} + ## Your Task + + Evaluate whether the revised changes are suitable to update the existing pull request. + + Choose one action: + + - **continue**: The revised changes are focused, coherent, and suitable to add to the existing pull request. + + - **needs_more_context**: The revised changes are incomplete or there is not enough evidence to tell whether they address the requested iteration. + + - **redo**: The revised changes introduce obvious bugs, contradict the prior fix, or make unrelated modifications. + + - **not_actionable**: The pull request cannot be iterated through code changes. For example: no revised code changes were produced. + + Include a brief reason (1-2 sentences) explaining your decision.\ + """) + + +def introspect_iteration( + organization: Organization, + run_id: int, + state: SeerRunState, + group: Group, +) -> IntrospectionDecision | None: + iteration_index = get_latest_iteration_index(state) + try: + diffs_by_repo = state.get_diffs_by_repo() + if not diffs_by_repo: + logger.warning( + "autofix.introspection.no_artifact", + extra={ + "run_id": run_id, + "organization_id": organization.id, + "step": "pr_iteration", + "type": "code_changes", + "iteration_index": iteration_index, + }, + ) + return None + + diffs_by_repo_str = { + repo: "\n".join(fp.diff for fp in patches if fp.diff) + for repo, patches in diffs_by_repo.items() + } + + prompt = _iteration_introspection_prompt( + short_id=group.qualified_short_id or str(group.id), + title=group.title or "", + culprit=group.culprit or "", + diffs_by_repo=diffs_by_repo_str, + ) + + return _run_introspection( + run_id, + AutofixStep.PR_ITERATION, + prompt, + ) + except Exception: + logger.exception( + "autofix.introspection.failed", + extra={ + "run_id": run_id, + "organization_id": organization.id, + "step": "pr_iteration", + "iteration_index": iteration_index, + }, + ) + + return None diff --git a/src/sentry/seer/autofix/on_completion_hook.py b/src/sentry/seer/autofix/on_completion_hook.py index 1f3762936411ec..bdc57f4e4798c6 100644 --- a/src/sentry/seer/autofix/on_completion_hook.py +++ b/src/sentry/seer/autofix/on_completion_hook.py @@ -20,6 +20,7 @@ from sentry.seer.autofix.autofix_agent import ( STEP_CONFIGS, AutofixStep, + get_latest_iteration_index, trigger_autofix_agent, trigger_coding_agent_handoff, trigger_push_changes, @@ -29,6 +30,7 @@ from sentry.seer.autofix.introspection import ( IntrospectionDecision, introspect_code_changes, + introspect_iteration, introspect_root_cause, introspect_solution, ) @@ -167,18 +169,7 @@ def _send_step_webhook( # handled but the expectation is that we only create PRs once # per seer run. webhook_action_type = SeerActionType.PR_CREATED - webhook_payload["pull_requests"] = [ - { - "provider": "unknown", # TODO: we don't have the repo object readily accessible here - "repo_name": pull_request.repo_name, - "pull_request": { - "pr_id": pull_request.pr_id, - "pr_number": pull_request.pr_number, - "pr_url": pull_request.pr_url, - }, - } - for pull_request in state.repo_pr_states.values() - ] + webhook_payload["pull_requests"] = cls._format_pull_requests_payload(state) is_pr_created = True analytics.record( AiAutofixPrCreatedCompletedEvent( @@ -190,20 +181,19 @@ def _send_step_webhook( ) else: webhook_action_type = SeerActionType.CODING_COMPLETED - diffs_by_repo = state.get_diffs_by_repo() - webhook_payload["code_changes"] = { - repo: [ - { - "diff": p.diff, - "path": p.patch.path, - "type": p.patch.type, - "added": p.patch.added, - "removed": p.patch.removed, - } - for p in patches - ] - for repo, patches in diffs_by_repo.items() - } + webhook_payload["code_changes"] = cls._format_code_changes_payload(state) + elif current_step == AutofixStep.PR_ITERATION: + # PR iteration only runs against an existing PR, so there should be pr states. + if not state.repo_pr_states: + logger.error( + "autofix.on_completion_hook.pr_iteration_missing_repo_pr_states", + extra={"run_id": run_id, "organization_id": organization.id}, + ) + webhook_action_type = SeerActionType.ITERATION_COMPLETED + iteration_index = get_latest_iteration_index(state) + webhook_payload["pull_requests"] = cls._format_pull_requests_payload(state) + webhook_payload["code_changes"] = cls._format_code_changes_payload(state) + webhook_payload["iteration_index"] = iteration_index if not webhook_action_type: return @@ -264,6 +254,38 @@ def _send_step_webhook( ) ) + @classmethod + def _format_code_changes_payload(cls, state: SeerRunState) -> dict: + diffs_by_repo = state.get_diffs_by_repo() + return { + repo: [ + { + "diff": p.diff, + "path": p.patch.path, + "type": p.patch.type, + "added": p.patch.added, + "removed": p.patch.removed, + } + for p in patches + ] + for repo, patches in diffs_by_repo.items() + } + + @classmethod + def _format_pull_requests_payload(cls, state: SeerRunState) -> list[dict]: + return [ + { + "provider": "unknown", + "repo_name": pull_request.repo_name, + "pull_request": { + "pr_id": pull_request.pr_id, + "pr_number": pull_request.pr_number, + "pr_url": pull_request.pr_url, + }, + } + for pull_request in state.repo_pr_states.values() + ] + @classmethod def _get_current_step( cls, state: SeerRunState @@ -350,6 +372,7 @@ def _maybe_continue_pipeline( group, ) if decision is not None: + iteration_index = get_latest_iteration_index(state) analytics.record( AiAutofixIntrospectionEvent( organization_id=organization.id, @@ -359,6 +382,7 @@ def _maybe_continue_pipeline( step=current_step.value, action=decision.action.value, reached_stopping_point=reached_stopping_point, + iteration_index=iteration_index, ) ) logger.info( @@ -452,7 +476,8 @@ def run_introspection( return introspect_solution(organization, run_id, state, group) elif step == AutofixStep.CODE_CHANGES: return introspect_code_changes(organization, run_id, state, group) - return None + elif step == AutofixStep.PR_ITERATION: + return introspect_iteration(organization, run_id, state, group) @classmethod def _push_changes(cls, group: Group, run_id: int, state: SeerRunState) -> None: diff --git a/tests/sentry/seer/autofix/test_autofix_agent.py b/tests/sentry/seer/autofix/test_autofix_agent.py index 7631d7862d3485..24d05bc6fa4562 100644 --- a/tests/sentry/seer/autofix/test_autofix_agent.py +++ b/tests/sentry/seer/autofix/test_autofix_agent.py @@ -19,7 +19,6 @@ generate_autofix_handoff_prompt, get_iteration_for_insert_index, get_latest_iteration_index, - recover_iteration_feedback, trigger_autofix_agent, trigger_coding_agent_handoff, trigger_push_changes, @@ -28,7 +27,6 @@ from sentry.seer.models import SeerPermissionError from sentry.sentry_apps.utils.webhooks import SeerActionType from sentry.testutils.cases import TestCase -from sentry.utils import json class TestGenerateAutofixHandoffPrompt(TestCase): @@ -247,20 +245,10 @@ def test_all_prompts_are_dedented(self) -> None: assert not prompt.startswith("\t"), f"{step} prompt starts with tab" -def _iteration_block( - iteration_index: int | None = None, feedback: str | None = None -) -> MemoryBlock: +def _iteration_block(iteration_index: int | None = None) -> MemoryBlock: metadata: dict[str, str] = {"step": AutofixStep.PR_ITERATION.value} if iteration_index is not None: metadata["iteration_index"] = str(iteration_index) - if feedback is not None: - metadata["feedback"] = json.dumps( - { - "text": feedback, - "source": "user", - "timestamp": "2024-01-01T00:00:00Z", - } - ) return MemoryBlock( id=f"block-{iteration_index}", message=Message(role="assistant", content="iteration", metadata=metadata), @@ -291,24 +279,6 @@ def test_get_iteration_for_insert_index(self) -> None: state = _state_with_blocks([_iteration_block(1), _iteration_block(2)]) assert get_iteration_for_insert_index(state, 1) == 2 - def test_recover_iteration_feedback_returns_feedback(self) -> None: - state = _state_with_blocks([_iteration_block(1, feedback="please fix the tests")]) - assert recover_iteration_feedback(state, 0) == "please fix the tests" - - def test_recover_iteration_feedback_out_of_range(self) -> None: - state = _state_with_blocks([]) - assert recover_iteration_feedback(state, 0) is None - assert recover_iteration_feedback(state, -1) is None - - def test_recover_iteration_feedback_no_metadata(self) -> None: - block = MemoryBlock( - id="block-none", - message=Message(role="assistant", content="x", metadata=None), - timestamp="2024-01-01T00:00:00Z", - ) - state = _state_with_blocks([block]) - assert recover_iteration_feedback(state, 0) is None - class TestPrIterationPrompt(TestCase): def setUp(self) -> None: diff --git a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py index f64ede07004a36..0b0d630785acc0 100644 --- a/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py +++ b/tests/sentry/seer/autofix/test_autofix_on_completion_hook.py @@ -99,6 +99,31 @@ def code_changes_memory_block(referrer: str | None = None) -> MemoryBlock: ) +def pr_iteration_memory_block(referrer: str | None = None, iteration_index: int = 1) -> MemoryBlock: + metadata: dict[str, str] = { + "step": "pr_iteration", + "iteration_index": str(iteration_index), + } + if referrer is not None: + metadata["referrer"] = referrer + return MemoryBlock( + id="block-pr-iteration", + message=Message( + role="assistant", + content="message pr iteration", + metadata=metadata, + ), + timestamp="2026-02-10T00:00:00Z", + merged_file_patches=[ + AgentFilePatch( + repo_name="test-repo", + diff="diff --git a/test.py b/test.py", + patch=FilePatch(path="test.py", type="M", added=2, removed=1), + ) + ], + ) + + class TestAutofixOnCompletionHookHelpers(TestCase): """Tests for helper methods in AutofixOnCompletionHook.""" @@ -179,6 +204,27 @@ def test_get_next_step_code_changes_is_last(self) -> None: result = AutofixOnCompletionHook._get_next_step(AutofixStep.CODE_CHANGES) assert result is None + def test_get_next_step_pr_iteration_is_terminal(self) -> None: + result = AutofixOnCompletionHook._get_next_step(AutofixStep.PR_ITERATION) + assert result is None + + @patch("sentry.seer.autofix.on_completion_hook.introspect_iteration") + def test_run_introspection_pr_iteration(self, mock_introspect_iteration) -> None: + organization = self.create_organization() + project = self.create_project(organization=organization) + group = self.create_group(project=project) + state = run_state(blocks=[pr_iteration_memory_block()]) + + AutofixOnCompletionHook.run_introspection( + organization, + 123, + state, + AutofixStep.PR_ITERATION, + group, + ) + + mock_introspect_iteration.assert_called_once_with(organization, 123, state, group) + class TestAutofixOnCompletionHookPipeline(TestCase): """Tests for pipeline continuation logic.""" @@ -406,6 +452,75 @@ def test_send_step_webhook_coding(self, mock_broadcast): assert call_kwargs["payload"]["code_changes"]["test-repo"][0]["added"] == 5 assert call_kwargs["payload"]["code_changes"]["test-repo"][0]["removed"] == 2 + @patch("sentry.seer.autofix.on_completion_hook.analytics.record") + @patch("sentry.seer.autofix.on_completion_hook.process_autofix_updates.apply_async") + @patch("sentry.seer.autofix.on_completion_hook.SeerAutofixOperator.has_access") + @patch("sentry.seer.autofix.on_completion_hook.broadcast_webhooks_for_organization.delay") + def test_send_step_webhook_pr_iteration( + self, mock_broadcast, mock_has_access, mock_process_autofix_updates, mock_analytics + ): + mock_has_access.return_value = True + state = run_state( + blocks=[ + root_cause_memory_block(), + solution_memory_block(), + code_changes_memory_block(), + pr_iteration_memory_block(referrer=AutofixReferrer.GROUP_AUTOFIX_ENDPOINT.value), + ] + ) + state.repo_pr_states = { + "test-repo": RepoPRState( + repo_name="test-repo", + pr_id=77, + pr_number=7, + pr_url="https://example.com/pull/7", + pr_creation_status="completed", + ) + } + + AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state, self.group) + + mock_broadcast.assert_called_once() + call_kwargs = mock_broadcast.call_args.kwargs + assert call_kwargs["event_name"] == SeerActionType.ITERATION_COMPLETED.value + assert call_kwargs["payload"]["code_changes"]["test-repo"][0]["path"] == "test.py" + assert call_kwargs["payload"]["pull_requests"][0]["pull_request"]["pr_number"] == 7 + mock_process_autofix_updates.assert_called_once() + assert ( + mock_analytics.call_args.args[0].referrer + == AutofixReferrer.GROUP_AUTOFIX_ENDPOINT.value + ) + + @patch("sentry.seer.autofix.on_completion_hook.analytics.record") + @patch("sentry.seer.autofix.on_completion_hook.broadcast_webhooks_for_organization.delay") + def test_send_step_webhook_pr_iteration_does_not_emit_pr_created( + self, mock_broadcast, mock_analytics + ): + state = run_state( + blocks=[ + code_changes_memory_block(), + pr_iteration_memory_block(), + ] + ) + state.repo_pr_states = { + "test-repo": RepoPRState( + repo_name="test-repo", + pr_id=77, + pr_number=7, + pr_url="https://example.com/pull/7", + pr_creation_status="completed", + ) + } + + AutofixOnCompletionHook._send_step_webhook(self.organization, 123, state, self.group) + + assert ( + mock_broadcast.call_args.kwargs["event_name"] + == SeerActionType.ITERATION_COMPLETED.value + ) + event_names = [call.args[0].type for call in mock_analytics.call_args_list] + assert "ai.autofix.pr_created.completed" not in event_names + @patch("sentry.seer.autofix.on_completion_hook.broadcast_webhooks_for_organization.delay") def test_send_step_webhook_no_artifacts_no_webhook(self, mock_broadcast): """Does not send webhook when no artifacts or file patches exist."""