diff --git a/pyproject.toml b/pyproject.toml index c3a059b35..906341971 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "GitAuto" -version = "1.6.11" +version = "1.12.0" requires-python = ">=3.14" dependencies = [ "annotated-doc==0.0.4", diff --git a/services/github/comments/delete_comments_by_identifiers.py b/services/github/comments/delete_comments_by_identifiers.py index 32daf894b..a4c867fe7 100644 --- a/services/github/comments/delete_comments_by_identifiers.py +++ b/services/github/comments/delete_comments_by_identifiers.py @@ -2,7 +2,7 @@ from services.github.comments.filter_comments_by_identifiers import ( filter_comments_by_identifiers, ) -from services.github.comments.get_all_comments import get_all_comments +from services.github.comments.get_pr_comments import get_pr_comments from utils.error.handle_exceptions import handle_exceptions @@ -16,8 +16,12 @@ def delete_comments_by_identifiers( identifiers: list[str], ): """Delete all comments containing the identifiers made by GitAuto""" - comments = get_all_comments( - owner=owner, repo=repo, pr_number=pr_number, token=token + comments = get_pr_comments( + owner=owner, + repo=repo, + pr_number=pr_number, + token=token, + exclude_self=False, ) matching_comments = filter_comments_by_identifiers(comments, identifiers) for comment in matching_comments: diff --git a/services/github/comments/filter_comments_by_identifiers.py b/services/github/comments/filter_comments_by_identifiers.py index d0e38f885..3a12bd812 100644 --- a/services/github/comments/filter_comments_by_identifiers.py +++ b/services/github/comments/filter_comments_by_identifiers.py @@ -1,9 +1,12 @@ +from typing import Sequence + from config import GITHUB_APP_USER_NAME +from services.github.types.webhook.pr_comment import Comment from utils.error.handle_exceptions import handle_exceptions @handle_exceptions(default_return_value=[], raise_on_error=False) -def filter_comments_by_identifiers(comments: list[dict], identifiers: list[str]): +def filter_comments_by_identifiers(comments: Sequence[Comment], identifiers: list[str]): """Filter comments by identifier text(s) and made by GitAuto""" comments = comments or [] diff --git a/services/github/comments/get_all_comments.py b/services/github/comments/get_all_comments.py deleted file mode 100644 index 484dac5b3..000000000 --- a/services/github/comments/get_all_comments.py +++ /dev/null @@ -1,15 +0,0 @@ -import requests -from config import GITHUB_API_URL, TIMEOUT -from services.github.utils.create_headers import create_headers -from utils.error.handle_exceptions import handle_exceptions - - -@handle_exceptions(default_return_value=[], raise_on_error=False) -def get_all_comments(*, owner: str, repo: str, pr_number: int, token: str): - """https://docs.github.com/en/rest/issues/comments?apiVersion=2022-11-28#list-issue-comments""" - url = f"{GITHUB_API_URL}/repos/{owner}/{repo}/issues/{pr_number}/comments" - headers: dict[str, str] = create_headers(token=token) - response = requests.get(url=url, headers=headers, timeout=TIMEOUT) - response.raise_for_status() - comments: list[dict] = response.json() - return comments diff --git a/services/github/comments/get_comments.py b/services/github/comments/get_comments.py deleted file mode 100644 index de7edad64..000000000 --- a/services/github/comments/get_comments.py +++ /dev/null @@ -1,35 +0,0 @@ -# Third party imports -import requests - -# Local imports -from config import GITHUB_API_URL, GITHUB_APP_IDS, TIMEOUT -from services.github.utils.create_headers import create_headers -from services.types.base_args import BaseArgs -from utils.error.handle_exceptions import handle_exceptions - - -@handle_exceptions(default_return_value=[], raise_on_error=False) -def get_comments(pr_number: int, base_args: BaseArgs, includes_me: bool = False): - """https://docs.github.com/en/rest/issues/comments#list-issue-comments""" - owner = base_args["owner"] - repo = base_args["repo"] - token = base_args["token"] - response = requests.get( - url=f"{GITHUB_API_URL}/repos/{owner}/{repo}/issues/{pr_number}/comments", - headers=create_headers(token=token), - timeout=TIMEOUT, - ) - response.raise_for_status() - comments: list[dict[str, object]] = response.json() - if not includes_me: - filtered_comments: list[dict[str, object]] = [] - for comment in comments: - app = comment.get("performed_via_github_app") - if app is None: - filtered_comments.append(comment) - elif isinstance(app, dict) and app.get("id") not in GITHUB_APP_IDS: - filtered_comments.append(comment) - else: - filtered_comments = comments - comment_texts: list[str] = [str(comment["body"]) for comment in filtered_comments] - return comment_texts diff --git a/services/github/comments/get_pr_comments.py b/services/github/comments/get_pr_comments.py new file mode 100644 index 000000000..ec7698db7 --- /dev/null +++ b/services/github/comments/get_pr_comments.py @@ -0,0 +1,32 @@ +import requests + +from config import GITHUB_API_URL, GITHUB_APP_IDS, TIMEOUT +from services.github.types.webhook.pr_comment import Comment +from services.github.utils.create_headers import create_headers +from utils.error.handle_exceptions import handle_exceptions +from utils.logging.logging_config import logger + + +@handle_exceptions(default_return_value=[], raise_on_error=False) +def get_pr_comments( + *, owner: str, repo: str, pr_number: int, token: str, exclude_self: bool +): + """https://docs.github.com/en/rest/issues/comments#list-issue-comments""" + url = f"{GITHUB_API_URL}/repos/{owner}/{repo}/issues/{pr_number}/comments" + headers = create_headers(token=token) + response = requests.get(url=url, headers=headers, timeout=TIMEOUT) + response.raise_for_status() + comments: list[Comment] = response.json() + if exclude_self: + filtered: list[Comment] = [] + for comment in comments: + app = comment.get("performed_via_github_app") + if app is None or app.get("id") not in GITHUB_APP_IDS: + filtered.append(comment) + logger.info( + "Filtered %d → %d PR comments (excluded self)", len(comments), len(filtered) + ) + return filtered + + logger.info("Returning all %d PR comments", len(comments)) + return comments diff --git a/services/github/comments/test_delete_comments_by_identifiers.py b/services/github/comments/test_delete_comments_by_identifiers.py index 3fab86777..a461614e5 100644 --- a/services/github/comments/test_delete_comments_by_identifiers.py +++ b/services/github/comments/test_delete_comments_by_identifiers.py @@ -13,9 +13,9 @@ @pytest.fixture -def mock_get_all_comments(): +def mock_get_pr_comments(): with patch( - "services.github.comments.delete_comments_by_identifiers.get_all_comments" + "services.github.comments.delete_comments_by_identifiers.get_pr_comments" ) as mock: yield mock @@ -37,7 +37,7 @@ def mock_delete_comment(): def test_delete_comments_by_identifiers_successful_deletion( - mock_get_all_comments, + mock_get_pr_comments, mock_filter_comments_by_identifiers, mock_delete_comment, ): @@ -50,7 +50,7 @@ def test_delete_comments_by_identifiers_successful_deletion( {"id": 1, "body": "Comment with id-1", "user": {"login": "gitauto-ai[bot]"}}, {"id": 2, "body": "Comment with id-2", "user": {"login": "gitauto-ai[bot]"}}, ] - mock_get_all_comments.return_value = sample_comments + mock_get_pr_comments.return_value = sample_comments mock_filter_comments_by_identifiers.return_value = sample_comments mock_delete_comment.return_value = None @@ -65,14 +65,18 @@ def test_delete_comments_by_identifiers_successful_deletion( ) assert result is None - mock_get_all_comments.assert_called_once_with( - owner=owner, repo=repo, pr_number=pr_number, token=token + mock_get_pr_comments.assert_called_once_with( + owner=owner, + repo=repo, + pr_number=pr_number, + token=token, + exclude_self=False, ) assert mock_delete_comment.call_count == 2 def test_delete_comments_by_identifiers_no_comments_found( - mock_get_all_comments, + mock_get_pr_comments, mock_filter_comments_by_identifiers, mock_delete_comment, ): @@ -81,7 +85,7 @@ def test_delete_comments_by_identifiers_no_comments_found( pr_number = fake.random_int(min=1, max=999) token = fake.sha256() - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_filter_comments_by_identifiers.return_value = [] result = delete_comments_by_identifiers( @@ -97,7 +101,7 @@ def test_delete_comments_by_identifiers_no_comments_found( def test_delete_comments_by_identifiers_no_matching_comments( - mock_get_all_comments, + mock_get_pr_comments, mock_filter_comments_by_identifiers, mock_delete_comment, ): @@ -109,7 +113,7 @@ def test_delete_comments_by_identifiers_no_matching_comments( sample_comments = [ {"id": 1, "body": "Some comment", "user": {"login": "gitauto-ai[bot]"}} ] - mock_get_all_comments.return_value = sample_comments + mock_get_pr_comments.return_value = sample_comments mock_filter_comments_by_identifiers.return_value = [] result = delete_comments_by_identifiers( @@ -125,7 +129,7 @@ def test_delete_comments_by_identifiers_no_matching_comments( def test_delete_comments_by_identifiers_exception_handling( - mock_get_all_comments, + mock_get_pr_comments, mock_filter_comments_by_identifiers, mock_delete_comment, ): @@ -134,7 +138,7 @@ def test_delete_comments_by_identifiers_exception_handling( pr_number = fake.random_int(min=1, max=999) token = fake.sha256() - mock_get_all_comments.side_effect = Exception("API error") + mock_get_pr_comments.side_effect = Exception("API error") result = delete_comments_by_identifiers( owner=owner, @@ -150,7 +154,7 @@ def test_delete_comments_by_identifiers_exception_handling( def test_delete_comments_by_identifiers_delete_comment_exception( - mock_get_all_comments, + mock_get_pr_comments, mock_filter_comments_by_identifiers, mock_delete_comment, ): @@ -162,7 +166,7 @@ def test_delete_comments_by_identifiers_delete_comment_exception( matching_comments = [ {"id": 1, "body": "test", "user": {"login": "gitauto-ai[bot]"}} ] - mock_get_all_comments.return_value = matching_comments + mock_get_pr_comments.return_value = matching_comments mock_filter_comments_by_identifiers.return_value = matching_comments mock_delete_comment.side_effect = Exception("Delete failed") diff --git a/services/github/comments/test_filter_comments_by_identifiers.py b/services/github/comments/test_filter_comments_by_identifiers.py index 81268eb0f..1f54953fe 100644 --- a/services/github/comments/test_filter_comments_by_identifiers.py +++ b/services/github/comments/test_filter_comments_by_identifiers.py @@ -1,8 +1,11 @@ +# pyright: reportArgumentType=false +import inspect from unittest.mock import patch from services.github.comments.filter_comments_by_identifiers import ( filter_comments_by_identifiers, ) +from services.github.types.webhook.pr_comment import Comment def test_filter_comments_by_identifiers_empty_comments(): @@ -224,3 +227,12 @@ def test_filter_comments_by_identifiers_multiple_identifiers_single_comment(): result = filter_comments_by_identifiers(comments, ["first-id", "second-id"]) assert len(result) == 1 assert result[0]["id"] == 1 + + +def test_filter_comments_by_identifiers_accepts_comment_type(): + """Verify function signature accepts list[Comment] type.""" + sig = inspect.signature(filter_comments_by_identifiers) + params = sig.parameters + assert "comments" in params + assert "identifiers" in params + assert Comment is not None diff --git a/services/github/comments/test_get_comments.py b/services/github/comments/test_get_comments.py deleted file mode 100644 index e34daab06..000000000 --- a/services/github/comments/test_get_comments.py +++ /dev/null @@ -1,112 +0,0 @@ -from unittest.mock import MagicMock, patch - -from config import GITHUB_APP_IDS -from services.github.comments.get_comments import get_comments - - -def test_get_comments_success(test_owner, test_repo, test_token, create_test_base_args): - # Arrange - mock_response = MagicMock() - mock_response.json.return_value = [ - {"body": "Comment 1", "performed_via_github_app": None}, - {"body": "Comment 2", "performed_via_github_app": None}, - ] - - base_args = create_test_base_args( - owner=test_owner, - repo=test_repo, - token=test_token, - ) - - # Act - with patch("services.github.comments.get_comments.requests.get") as mock_get, patch( - "services.github.comments.get_comments.create_headers" - ) as mock_create_headers: - mock_create_headers.return_value = {"Authorization": f"Bearer {test_token}"} - mock_get.return_value = mock_response - result = get_comments(123, base_args) - - # Assert - mock_get.assert_called_once() - assert result == ["Comment 1", "Comment 2"] - - -def test_get_comments_with_app_comments_excluded( - test_owner, test_repo, test_token, create_test_base_args -): - # Arrange - mock_response = MagicMock() - mock_response.json.return_value = [ - {"body": "Comment 1", "performed_via_github_app": None}, - {"body": "Comment 2", "performed_via_github_app": {"id": GITHUB_APP_IDS[0]}}, - { - "body": "Comment 3", - "performed_via_github_app": {"id": 999999}, - }, # Not in GITHUB_APP_IDS - ] - - base_args = create_test_base_args( - owner=test_owner, - repo=test_repo, - token=test_token, - ) - - # Act - with patch("services.github.comments.get_comments.requests.get") as mock_get: - mock_get.return_value = mock_response - result = get_comments(123, base_args, includes_me=False) - - # Assert - mock_get.assert_called_once() - # Should exclude comments from our app (GITHUB_APP_IDS) - assert result == ["Comment 1", "Comment 3"] - - -def test_get_comments_with_app_comments_included( - test_owner, test_repo, test_token, create_test_base_args -): - # Arrange - mock_response = MagicMock() - mock_response.json.return_value = [ - {"body": "Comment 1", "performed_via_github_app": None}, - {"body": "Comment 2", "performed_via_github_app": {"id": GITHUB_APP_IDS[0]}}, - {"body": "Comment 3", "performed_via_github_app": {"id": 999999}}, - ] - - base_args = create_test_base_args( - owner=test_owner, - repo=test_repo, - token=test_token, - ) - - # Act - with patch("services.github.comments.get_comments.requests.get") as mock_get: - mock_get.return_value = mock_response - result = get_comments(123, base_args, includes_me=True) - - # Assert - mock_get.assert_called_once() - # Should include all comments when includes_me=True - assert result == ["Comment 1", "Comment 2", "Comment 3"] - - -def test_get_comments_request_error( - test_owner, test_repo, test_token, create_test_base_args -): - # Arrange - mock_response = MagicMock() - mock_response.raise_for_status.side_effect = Exception("API error") - - base_args = create_test_base_args( - owner=test_owner, - repo=test_repo, - token=test_token, - ) - - # Act - with patch("services.github.comments.get_comments.requests.get") as mock_get: - mock_get.return_value = mock_response - result = get_comments(123, base_args) - - # Assert - assert not result # The handle_exceptions decorator should return [] on error diff --git a/services/github/comments/test_get_all_comments.py b/services/github/comments/test_get_pr_comments.py similarity index 62% rename from services/github/comments/test_get_all_comments.py rename to services/github/comments/test_get_pr_comments.py index 2624975ec..56ed7c6e7 100644 --- a/services/github/comments/test_get_all_comments.py +++ b/services/github/comments/test_get_pr_comments.py @@ -7,7 +7,7 @@ from faker import Faker from config import TIMEOUT -from services.github.comments.get_all_comments import get_all_comments +from services.github.comments.get_pr_comments import get_pr_comments fake = Faker() @@ -29,18 +29,18 @@ def token(): @pytest.fixture def mock_requests_get(): - with patch("services.github.comments.get_all_comments.requests.get") as mock: + with patch("services.github.comments.get_pr_comments.requests.get") as mock: yield mock @pytest.fixture def mock_create_headers(token): - with patch("services.github.comments.get_all_comments.create_headers") as mock: + with patch("services.github.comments.get_pr_comments.create_headers") as mock: mock.return_value = {"Authorization": f"Bearer {token}"} yield mock -def test_get_all_comments_success( +def test_get_pr_comments_success( owner, repo, token, mock_requests_get, mock_create_headers ): expected_comments = [ @@ -51,7 +51,9 @@ def test_get_all_comments_success( mock_response.json.return_value = expected_comments mock_requests_get.return_value = mock_response - result = get_all_comments(owner=owner, repo=repo, pr_number=123, token=token) + result = get_pr_comments( + owner=owner, repo=repo, pr_number=123, token=token, exclude_self=False + ) assert result == expected_comments mock_create_headers.assert_called_once_with(token=token) @@ -59,44 +61,50 @@ def test_get_all_comments_success( mock_response.raise_for_status.assert_called_once() -def test_get_all_comments_empty_response(owner, repo, token, mock_requests_get): +def test_get_pr_comments_empty_response(owner, repo, token, mock_requests_get): mock_response = MagicMock() mock_response.json.return_value = [] mock_requests_get.return_value = mock_response - result = get_all_comments(owner=owner, repo=repo, pr_number=123, token=token) + result = get_pr_comments( + owner=owner, repo=repo, pr_number=123, token=token, exclude_self=False + ) assert not result mock_requests_get.assert_called_once() -def test_get_all_comments_correct_url_construction( +def test_get_pr_comments_correct_url_construction( owner, repo, token, mock_requests_get ): mock_response = MagicMock() mock_response.json.return_value = [] mock_requests_get.return_value = mock_response - get_all_comments(owner=owner, repo=repo, pr_number=123, token=token) + get_pr_comments( + owner=owner, repo=repo, pr_number=123, token=token, exclude_self=False + ) expected_url = f"https://api.github.com/repos/{owner}/{repo}/issues/123/comments" call_args = mock_requests_get.call_args assert call_args[1]["url"] == expected_url -def test_get_all_comments_timeout_parameter(owner, repo, token, mock_requests_get): +def test_get_pr_comments_timeout_parameter(owner, repo, token, mock_requests_get): mock_response = MagicMock() mock_response.json.return_value = [] mock_requests_get.return_value = mock_response - get_all_comments(owner=owner, repo=repo, pr_number=123, token=token) + get_pr_comments( + owner=owner, repo=repo, pr_number=123, token=token, exclude_self=False + ) call_args = mock_requests_get.call_args assert "timeout" in call_args[1] assert call_args[1]["timeout"] == TIMEOUT -def test_get_all_comments_http_error(owner, repo, token, mock_requests_get): +def test_get_pr_comments_http_error(owner, repo, token, mock_requests_get): mock_response = MagicMock() http_error = requests.exceptions.HTTPError("404 Not Found") mock_error_response = MagicMock() @@ -107,20 +115,24 @@ def test_get_all_comments_http_error(owner, repo, token, mock_requests_get): mock_response.raise_for_status.side_effect = http_error mock_requests_get.return_value = mock_response - result = get_all_comments(owner=owner, repo=repo, pr_number=123, token=token) + result = get_pr_comments( + owner=owner, repo=repo, pr_number=123, token=token, exclude_self=False + ) assert not result -def test_get_all_comments_network_error(owner, repo, token, mock_requests_get): +def test_get_pr_comments_network_error(owner, repo, token, mock_requests_get): mock_requests_get.side_effect = requests.exceptions.ConnectionError("Network error") - result = get_all_comments(owner=owner, repo=repo, pr_number=123, token=token) + result = get_pr_comments( + owner=owner, repo=repo, pr_number=123, token=token, exclude_self=False + ) assert not result -def test_get_all_comments_uses_github_api_url_constant( +def test_get_pr_comments_uses_github_api_url_constant( owner, repo, token, mock_requests_get ): mock_response = MagicMock() @@ -128,10 +140,12 @@ def test_get_all_comments_uses_github_api_url_constant( mock_requests_get.return_value = mock_response with patch( - "services.github.comments.get_all_comments.GITHUB_API_URL", + "services.github.comments.get_pr_comments.GITHUB_API_URL", "https://custom.api.github.com", ): - get_all_comments(owner=owner, repo=repo, pr_number=123, token=token) + get_pr_comments( + owner=owner, repo=repo, pr_number=123, token=token, exclude_self=False + ) expected_url = ( f"https://custom.api.github.com/repos/{owner}/{repo}/issues/123/comments" @@ -140,12 +154,12 @@ def test_get_all_comments_uses_github_api_url_constant( assert call_args[1]["url"] == expected_url -def test_get_all_comments_decorator_configuration(): - assert hasattr(get_all_comments, "__wrapped__") +def test_get_pr_comments_decorator_configuration(): + assert hasattr(get_pr_comments, "__wrapped__") -def test_get_all_comments_function_signature_compliance(): - sig = inspect.signature(get_all_comments) +def test_get_pr_comments_function_signature_compliance(): + sig = inspect.signature(get_pr_comments) params = list(sig.parameters.keys()) - expected_params = ["owner", "repo", "pr_number", "token"] + expected_params = ["owner", "repo", "pr_number", "token", "exclude_self"] assert params == expected_params diff --git a/services/github/comments/test_update_progress.py b/services/github/comments/test_update_progress.py new file mode 100644 index 000000000..e2fb3b5a8 --- /dev/null +++ b/services/github/comments/test_update_progress.py @@ -0,0 +1,49 @@ +# pyright: reportUnusedVariable=false +import inspect +from unittest.mock import MagicMock, patch + +from services.github.comments.update_progress import update_progress + + +@patch("services.github.comments.update_progress.update_comment") +@patch("services.github.comments.update_progress.create_progress_bar") +@patch("services.github.comments.update_progress.add_log_message") +def test_update_progress_returns_incremented_p( + mock_add_log, mock_progress_bar, mock_update_comment +): + mock_progress_bar.return_value = "progress bar" + base_args = MagicMock() + log_messages: list[str] = [] + + result = update_progress( + msg="Test", p=10, log_messages=log_messages, base_args=base_args + ) + + assert result == 15 + mock_add_log.assert_called_once_with("Test", log_messages) + mock_progress_bar.assert_called_once() + mock_update_comment.assert_called_once() + + +@patch("services.github.comments.update_progress.update_comment") +@patch("services.github.comments.update_progress.create_progress_bar") +@patch("services.github.comments.update_progress.add_log_message") +def test_update_progress_exception_returns_default( + _mock_add_log, _mock_progress_bar, mock_update_comment +): + mock_update_comment.side_effect = Exception("API error") + base_args = MagicMock() + + result = update_progress(msg="Test", p=10, log_messages=[], base_args=base_args) + + assert result == 0 + + +def test_update_progress_function_signature(): + sig = inspect.signature(update_progress) + params = list(sig.parameters.keys()) + assert params == ["msg", "p", "log_messages", "base_args"] + + +def test_update_progress_decorator_applied(): + assert hasattr(update_progress, "__wrapped__") diff --git a/services/github/comments/update_progress.py b/services/github/comments/update_progress.py new file mode 100644 index 000000000..3cee0d6d9 --- /dev/null +++ b/services/github/comments/update_progress.py @@ -0,0 +1,20 @@ +from services.github.comments.update_comment import update_comment +from services.types.base_args import BaseArgs +from utils.error.handle_exceptions import handle_exceptions +from utils.logging.add_log_message import add_log_message +from utils.progress_bar.progress_bar import create_progress_bar + + +@handle_exceptions(default_return_value=0, raise_on_error=False) +def update_progress( + msg: str, + p: int, + log_messages: list[str], + base_args: BaseArgs, +): + """Increment progress by 5, log a message, and update the GitHub comment.""" + p += 5 + add_log_message(msg, log_messages) + comment_body = create_progress_bar(p=p, msg="\n".join(log_messages)) + update_comment(body=comment_body, base_args=base_args) + return p diff --git a/services/github/pulls/get_review_inline_comments.py b/services/github/pulls/get_review_inline_comments.py index 4cbaaa504..88d257c40 100644 --- a/services/github/pulls/get_review_inline_comments.py +++ b/services/github/pulls/get_review_inline_comments.py @@ -2,7 +2,8 @@ import requests # Local imports -from config import GITHUB_API_URL, TIMEOUT +from config import GITHUB_API_URL, PER_PAGE, TIMEOUT +from payloads.github.pull_request_review_comment.types import ReviewComment from services.github.utils.create_headers import create_headers from utils.error.handle_exceptions import handle_exceptions @@ -14,7 +15,19 @@ def get_review_inline_comments( """https://docs.github.com/en/rest/pulls/reviews#list-comments-for-a-pull-request-review""" url = f"{GITHUB_API_URL}/repos/{owner}/{repo}/pulls/{pr_number}/reviews/{review_id}/comments" headers = create_headers(token=token) - response = requests.get(url=url, headers=headers, timeout=TIMEOUT) - response.raise_for_status() - comments: list[dict[str, str | int]] = response.json() - return comments + all_comments: list[ReviewComment] = [] + page = 1 + while True: + response = requests.get( + url=url, + headers=headers, + timeout=TIMEOUT, + params={"per_page": PER_PAGE, "page": page}, + ) + response.raise_for_status() + batch: list[ReviewComment] = response.json() + all_comments.extend(batch) + if len(batch) < PER_PAGE: + break + page += 1 + return all_comments diff --git a/services/github/pulls/get_review_summary.py b/services/github/pulls/get_review_summary_comment.py similarity index 96% rename from services/github/pulls/get_review_summary.py rename to services/github/pulls/get_review_summary_comment.py index baa72b03e..c89da3667 100644 --- a/services/github/pulls/get_review_summary.py +++ b/services/github/pulls/get_review_summary_comment.py @@ -8,7 +8,7 @@ @handle_exceptions(default_return_value=None, raise_on_error=False) -def get_review_summary( +def get_review_summary_comment( owner: str, repo: str, pr_number: int, review_id: int, token: str ): """https://docs.github.com/en/rest/pulls/reviews#get-a-review-for-a-pull-request""" diff --git a/services/github/pulls/test_get_review_inline_comments.py b/services/github/pulls/test_get_review_inline_comments.py index 4f29835ae..4fe4bb627 100644 --- a/services/github/pulls/test_get_review_inline_comments.py +++ b/services/github/pulls/test_get_review_inline_comments.py @@ -29,6 +29,7 @@ def test_get_review_inline_comments_success(): url="https://api.github.com/repos/owner/repo/pulls/1/reviews/100/comments", headers={"Authorization": "Bearer token"}, timeout=120, + params={"per_page": 100, "page": 1}, ) mock_response.raise_for_status.assert_called_once() assert result == mock_data @@ -46,7 +47,7 @@ def test_get_review_inline_comments_empty(): result = get_review_inline_comments("owner", "repo", 1, 100, "token") - assert result == [] + assert not result def test_get_review_inline_comments_http_error(): @@ -75,3 +76,34 @@ def test_get_review_inline_comments_network_error(): result = get_review_inline_comments("owner", "repo", 1, 100, "token") assert result is None + + +def test_get_review_inline_comments_paginates(): + """Verify pagination fetches all pages until a partial page is returned.""" + page1 = [{"id": i, "body": f"c{i}", "path": "a.py", "line": i} for i in range(100)] + page2 = [{"id": 100, "body": "c100", "path": "b.py", "line": 1}] + + with patch(f"{MODULE}.requests.get") as mock_get, patch( + f"{MODULE}.create_headers" + ) as mock_headers: + resp1 = MagicMock() + resp1.json.return_value = page1 + resp2 = MagicMock() + resp2.json.return_value = page2 + mock_get.side_effect = [resp1, resp2] + mock_headers.return_value = {"Authorization": "Bearer token"} + + result = get_review_inline_comments("owner", "repo", 1, 100, "token") + + assert len(result) == 101 + assert result == page1 + page2 + assert mock_get.call_count == 2 + # Verify page params + assert mock_get.call_args_list[0].kwargs["params"] == { + "per_page": 100, + "page": 1, + } + assert mock_get.call_args_list[1].kwargs["params"] == { + "per_page": 100, + "page": 2, + } diff --git a/services/github/pulls/test_get_review_summary.py b/services/github/pulls/test_get_review_summary_comment.py similarity index 74% rename from services/github/pulls/test_get_review_summary.py rename to services/github/pulls/test_get_review_summary_comment.py index e3b3e62d8..655fb50d0 100644 --- a/services/github/pulls/test_get_review_summary.py +++ b/services/github/pulls/test_get_review_summary_comment.py @@ -5,12 +5,12 @@ import requests # Local imports -from services.github.pulls.get_review_summary import get_review_summary +from services.github.pulls.get_review_summary_comment import get_review_summary_comment -MODULE = "services.github.pulls.get_review_summary" +MODULE = "services.github.pulls.get_review_summary_comment" -def test_get_review_summary_success(): +def test_get_review_summary_comment_success(): mock_data = {"id": 100, "body": "Looks good overall", "state": "commented"} with patch(f"{MODULE}.requests.get") as mock_get, patch( f"{MODULE}.create_headers" @@ -20,7 +20,7 @@ def test_get_review_summary_success(): mock_get.return_value = mock_response mock_headers.return_value = {"Authorization": "Bearer token"} - result = get_review_summary("owner", "repo", 1, 100, "token") + result = get_review_summary_comment("owner", "repo", 1, 100, "token") mock_get.assert_called_once_with( url="https://api.github.com/repos/owner/repo/pulls/1/reviews/100", @@ -31,7 +31,7 @@ def test_get_review_summary_success(): assert result == "Looks good overall" -def test_get_review_summary_http_error(): +def test_get_review_summary_comment_http_error(): with patch(f"{MODULE}.requests.get") as mock_get, patch( f"{MODULE}.create_headers" ) as mock_headers: @@ -42,18 +42,18 @@ def test_get_review_summary_http_error(): mock_get.return_value = mock_response mock_headers.return_value = {"Authorization": "Bearer token"} - result = get_review_summary("owner", "repo", 1, 999, "token") + result = get_review_summary_comment("owner", "repo", 1, 999, "token") assert result is None -def test_get_review_summary_network_error(): +def test_get_review_summary_comment_network_error(): with patch(f"{MODULE}.requests.get") as mock_get, patch( f"{MODULE}.create_headers" ) as mock_headers: mock_get.side_effect = requests.exceptions.ConnectionError("Network error") mock_headers.return_value = {"Authorization": "Bearer token"} - result = get_review_summary("owner", "repo", 1, 100, "token") + result = get_review_summary_comment("owner", "repo", 1, 100, "token") assert result is None diff --git a/services/github/types/webhook/issue_comment.py b/services/github/types/webhook/pr_comment.py similarity index 84% rename from services/github/types/webhook/issue_comment.py rename to services/github/types/webhook/pr_comment.py index 7d284588b..ebf22ba1b 100644 --- a/services/github/types/webhook/issue_comment.py +++ b/services/github/types/webhook/pr_comment.py @@ -1,5 +1,6 @@ -from typing import Literal, NotRequired, TypedDict +from typing import Literal, NotRequired, Optional, TypedDict +from services.github.types.app import App from services.github.types.installation import Installation from services.github.types.issue import Issue, PrIssue from services.github.types.organization import Organization @@ -9,12 +10,15 @@ class Comment(TypedDict): + """PR issue comment. performed_via_github_app is App when made via GitHub App (e.g. GitAuto), None when made by a human.""" + id: int node_id: str user: User body: str created_at: str updated_at: str + performed_via_github_app: Optional[App] BodyChange = TypedDict("BodyChange", {"from": str}) diff --git a/services/webhook/check_suite_handler.py b/services/webhook/check_suite_handler.py index d54dc8e11..530a5a7c2 100644 --- a/services/webhook/check_suite_handler.py +++ b/services/webhook/check_suite_handler.py @@ -35,7 +35,7 @@ get_failed_check_runs_from_check_suite, ) from services.github.comments.create_comment import create_comment -from services.github.comments.get_all_comments import get_all_comments +from services.github.comments.get_pr_comments import get_pr_comments from services.github.comments.update_comment import update_comment from services.github.installations.get_installation_permissions import ( get_installation_permissions, @@ -246,7 +246,16 @@ async def handle_check_suite( "pr_number": pr_number, "pr_title": pr_title, "pr_body": full_pr["body"] or "", - "pr_comments": [], # Not needed - check_suite agent uses CI error logs, not PR discussion + "pr_comments": [ + f"@{c['user']['login']} ({c['created_at']}): {c['body']}" + for c in get_pr_comments( + owner=owner_name, + repo=repo_name, + pr_number=pr_number, + token=token, + exclude_self=True, + ) + ], "latest_commit_sha": check_run["head_sha"], "pr_creator": sender_name, "base_branch": base_branch, @@ -324,8 +333,12 @@ async def handle_check_suite( # Check if CI-failed comment already exists (skip if GitAuto is already handling this PR). # Exception: if the LATEST CI-failed comment is from an infra retry (contains "Re-triggering CI"), proceed because CI was re-triggered and failed with real errors. - comments = get_all_comments( - owner=owner_name, repo=repo_name, pr_number=pr_number, token=token + comments = get_pr_comments( + owner=owner_name, + repo=repo_name, + pr_number=pr_number, + token=token, + exclude_self=False, ) gitauto_failed_comments = [ c diff --git a/services/webhook/new_pr_handler.py b/services/webhook/new_pr_handler.py index d8a64beda..15461d285 100644 --- a/services/webhook/new_pr_handler.py +++ b/services/webhook/new_pr_handler.py @@ -24,7 +24,7 @@ ) from services.git.write_and_commit_file import write_and_commit_file from services.github.comments.create_comment import create_comment -from services.github.comments.get_comments import get_comments +from services.github.comments.get_pr_comments import get_pr_comments from services.github.comments.update_comment import update_comment from services.github.files.get_remote_file_content_by_url import ( get_remote_file_content_by_url, @@ -238,7 +238,17 @@ async def handle_new_pr( ) # Check out the PR comments - pr_comments = get_comments(pr_number=pr_number, base_args=base_args) + pr_comment_objs = get_pr_comments( + owner=owner_name, + repo=repo_name, + pr_number=pr_number, + token=token, + exclude_self=True, + ) + pr_comments = [ + f"@{c['user']['login']} ({c['created_at']}): {c['body']}" + for c in pr_comment_objs + ] comment_body = f"Found {len(pr_comments)} PR comments." p += 5 add_log_message(comment_body, log_messages) @@ -663,10 +673,18 @@ async def handle_new_pr( ) update_comment(body=body_after_pr, base_args=base_args) - # Update PR body with Claude-generated summary of what GA did - agent_comments = get_comments( - pr_number=pr_number, base_args=base_args, includes_me=True + # Re-fetch because GitAuto posted new comments during the agent run + agent_comment_objs = get_pr_comments( + owner=owner_name, + repo=repo_name, + pr_number=pr_number, + token=token, + exclude_self=False, ) + agent_comments = [ + f"@{c['user']['login']} ({c['created_at']}): {c['body']}" + for c in agent_comment_objs + ] pr_body_summary = generate_and_upsert_pr_body_section( owner_name=owner_name, repo_name=repo_name, diff --git a/services/webhook/review_run_handler.py b/services/webhook/review_run_handler.py index ccb243ef5..c0cbbcd08 100644 --- a/services/webhook/review_run_handler.py +++ b/services/webhook/review_run_handler.py @@ -11,6 +11,7 @@ from config import GITHUB_APP_USER_NAME, PRODUCT_ID from constants.agent import COST_CAP_RATIO, MAX_ITERATIONS from constants.triggers import ReviewTrigger +from payloads.github.pull_request_review_comment.types import ReviewComment from services.github.types.webhook.review_run_payload import ReviewRunPayload from services.agents.verify_task_is_complete import verify_task_is_complete from services.agents.verify_task_is_ready import verify_task_is_ready @@ -26,6 +27,7 @@ ) from services.git.git_merge_base_into_pr import git_merge_base_into_pr from services.github.comments.create_comment import create_comment +from services.github.comments.get_pr_comments import get_pr_comments from services.github.commits.get_head_commit_count_behind_base import ( get_head_commit_count_behind_base, ) @@ -33,17 +35,22 @@ from services.github.comments.update_comment import update_comment from services.slack.slack_notify import slack_notify from services.supabase.credits.check_purchase_exists import check_purchase_exists +from services.supabase.webhook_deliveries.insert_webhook_delivery import ( + insert_webhook_delivery, +) from services.supabase.credits.get_credit_price import get_credit_price from services.git.create_empty_commit import create_empty_commit from services.git.get_reference import get_reference from services.github.pulls.get_pull_request import get_pull_request from services.github.pulls.get_pull_request_files import get_pull_request_files -from services.github.pulls.get_review_summary import get_review_summary +from services.github.pulls.get_review_inline_comments import get_review_inline_comments +from services.github.pulls.get_review_summary_comment import get_review_summary_comment from services.github.pulls.get_review_thread_comments import get_review_thread_comments from services.github.token.get_installation_token import get_installation_access_token from services.github.users.get_email_from_commits import get_email_from_commits from services.github.users.get_user_public_email import get_user_public_info from services.claude.tools.tools import TOOLS_FOR_REVIEW_COMMENTS +from services.github.comments.update_progress import update_progress from services.supabase.create_user_request import create_user_request from services.supabase.repositories.get_repository import get_repository from services.supabase.usage.update_usage import update_usage @@ -51,7 +58,8 @@ from services.webhook.utils.create_system_message import create_system_message from services.webhook.utils.get_preferred_model import get_preferred_model from services.webhook.utils.should_bail import should_bail -from utils.files.get_local_file_content import get_local_file_content +from utils.files.read_local_file import read_local_file +from utils.formatting.format_with_line_numbers import format_content_with_line_numbers from utils.files.get_local_file_tree import get_local_file_tree from utils.logging.add_log_message import add_log_message from utils.logging.logging_config import logger @@ -140,7 +148,7 @@ async def handle_review_run( review_summary = "" pull_request_review_id = review.get("pull_request_review_id") if pull_request_review_id: - summary_body = get_review_summary( + summary_body = get_review_summary_comment( owner=owner_name, repo=repo_name, pr_number=pr_number, @@ -150,13 +158,61 @@ async def handle_review_run( if summary_body and summary_body.strip(): review_summary = summary_body + # Batch: when a reviewer (e.g. Devin) submits a review with N inline comments, GitHub fires N webhooks. Without dedup, each triggers a separate Lambda that races to push, wasting tokens. Only one invocation wins the lock; it sleeps, fetches all comments, and handles them in one session. + review_inline_comments: list[ReviewComment] = [] + if pull_request_review_id and trigger == "pr_file_review": + logger.info("Attempting batch dedup for review_id=%d", pull_request_review_id) + dedup_key = f"review-dedup-{repo_id}-{pr_number}-{pull_request_review_id}" + inserted = insert_webhook_delivery( + delivery_id=dedup_key, event_name="pr_file_review_dedup" + ) + if inserted is False: + logger.info( + "Another invocation handling review_id=%d for PR #%d, skipping", + pull_request_review_id, + pr_number, + ) + return + + # Real data: Devin review_id 4117870568 on SpiderPlus PR #13871 (2026-04-16) had 4 inline comments created 01:57:24–01:57:32 UTC but submitted together at 01:57:33 — GitHub webhooks fire at submission (~1s apart). 10s gives ample headroom including Lambda cold starts. + logger.info("Won batch lock, sleeping 10s for sibling webhooks") + time.sleep(10) + review_inline_comments = ( + get_review_inline_comments( + owner=owner_name, + repo=repo_name, + pr_number=pr_number, + review_id=pull_request_review_id, + token=token, + ) + or [] + ) + logger.info( + "Fetched %d inline comments from review %d", + len(review_inline_comments), + pull_request_review_id, + ) + # Get list of changed files in the PR (used for bot relevance check and later for file processing) pr_files = get_pull_request_files( owner=owner_name, repo=repo_name, pr_number=pr_number, token=token ) - # For PR-level comments (no file path), check bot relevance and loop prevention - if not review_path: + # Build review_comment based on multiple review inline comments, PR-level, or single inline comment + if review_inline_comments: + logger.info( + "Building combined review_comment from %d review inline comments", + len(review_inline_comments), + ) + parts = [f"## Review with {len(review_inline_comments)} inline comments\n"] + for i, rc in enumerate(review_inline_comments, 1): + rc_path = rc.get("path", "") + rc_line = rc.get("line", "?") + rc_body = rc.get("body", "") + parts.append(f"### Comment {i}: `{rc_path}` Line {rc_line}\n{rc_body}\n") + review_comment = "\n".join(parts) + elif not review_path: + # For PR-level comments (no file path), check bot relevance and loop prevention if review_author_is_bot: # Check if bot comment mentions any PR file paths (skip irrelevant bot comments like Security Hub scans) pr_file_paths = [f["filename"] for f in pr_files] @@ -227,7 +283,16 @@ async def handle_review_run( "clone_url": get_clone_url(owner_name, repo_name, token), "is_fork": is_fork, "pr_number": pr_number, - "pr_comments": [], + "pr_comments": [ + f"@{c['user']['login']} ({c['created_at']}): {c['body']}" + for c in get_pr_comments( + owner=owner_name, + repo=repo_name, + pr_number=pr_number, + token=token, + exclude_self=True, + ) + ], "latest_commit_sha": pull_request["head"]["sha"], "base_branch": base_branch, "new_branch": head_branch, @@ -340,35 +405,83 @@ async def handle_review_run( p = 0 log_messages: list[str] = [] if not review_author_is_bot: + logger.info("Posting greeting comment for human reviewer") msg = "Thanks for the feedback. I'm on it." add_log_message(msg, log_messages) comment_body = create_progress_bar(p=0, msg="\n".join(log_messages)) if review_path: + logger.info("Replying inline to review comment on %s", review_path) comment_url = reply_to_comment(base_args=base_args, body=comment_body) else: + logger.info("Creating top-level PR comment for review_id=%s", review_id) original_url = f"https://github.com/{owner_name}/{repo_name}/pull/{pr_number}#issuecomment-{review_id}" mention = f"@{sender_name}'s " if not review_author_is_bot else "" linked_body = f"Re: {mention}[comment]({original_url})\n\n{comment_body}" comment_url = create_comment(base_args=base_args, body=linked_body) base_args["comment_url"] = comment_url + else: + logger.info("Skipping greeting comment for bot reviewer %s", sender_name) - # Get a review commented file (skip when no specific file path) - review_file = "" - if review_path: - review_file = get_local_file_content(file_path=review_path, base_args=base_args) + # Get review commented file(s) — stored as separate messages (not inside JSON) so replace_old_file_content() can find and replace them when agent re-reads. + review_file_messages: list[MessageParam] = [] + if review_inline_comments: + logger.info( + "Reading files for %d review inline comments", len(review_inline_comments) + ) + unique_paths: list[str] = list( + dict.fromkeys( + str(rc.get("path")) for rc in review_inline_comments if rc.get("path") + ) + ) + for fp in unique_paths: + raw = read_local_file(file_path=fp, base_dir=clone_dir) + if raw: + logger.info("Read review inline comment file %s", fp) + formatted = format_content_with_line_numbers(file_path=fp, content=raw) + review_file_messages.append({"role": "user", "content": formatted}) + else: + logger.info("Batched file %s not found or empty", fp) + if not review_author_is_bot: + p = update_progress( + msg=f"Read {len(unique_paths)} files from review comments.", + p=p, + log_messages=log_messages, + base_args=base_args, + ) + else: + logger.info( + "Skipping progress update for bot reviewer (review inline comments)" + ) + elif review_path: + logger.info("Reading single review file %s", review_path) + raw = read_local_file(file_path=review_path, base_dir=clone_dir) + if raw: + logger.info("Read review file %s", review_path) + formatted = format_content_with_line_numbers( + file_path=review_path, content=raw + ) + review_file_messages.append({"role": "user", "content": formatted}) + else: + logger.info("Review file %s not found or empty", review_path) if not review_author_is_bot: - p += 5 - add_log_message( - f"Read the file `{review_path}` you commented on.", log_messages + p = update_progress( + msg=f"Read the file `{review_path}` you commented on.", + p=p, + log_messages=log_messages, + base_args=base_args, ) - comment_body = create_progress_bar(p=p, msg="\n".join(log_messages)) - update_comment(body=comment_body, base_args=base_args) + else: + logger.info("Skipping progress update for bot reviewer (single file)") + else: + logger.info("No review file path — PR-level comment, skipping file read") if not review_author_is_bot: - p += 5 - add_log_message(f"Found {len(pr_files)} changed files in the PR.", log_messages) - comment_body = create_progress_bar(p=p, msg="\n".join(log_messages)) - update_comment(body=comment_body, base_args=base_args) + p = update_progress( + msg=f"Found {len(pr_files)} changed files in the PR.", + p=p, + log_messages=log_messages, + base_args=base_args, + ) # Validate files for syntax issues before editing files_to_validate = [f["filename"] for f in pr_files if f["status"] != "removed"] @@ -403,7 +516,6 @@ async def handle_review_run( "step_1_review_comment": review_comment, "pull_request_title": pr_title, "pull_request_body": pr_body, - "review_file": review_file, "pr_files": pr_files, "today": today, "root_files": root_files, @@ -423,8 +535,9 @@ async def handle_review_run( input_message["step_2b_remaining_unfixable_errors"] = pre_existing_errors user_input = json.dumps(obj=input_message) - # Create messages + # Create messages — file content in separate messages for replace_old_file_content() messages: list[MessageParam] = [{"role": "user", "content": user_input}] + messages.extend(review_file_messages) # Loop a process explore repo and commit changes until the ticket is resolved total_token_input = 0 @@ -458,7 +571,7 @@ async def handle_review_run( logger.info(completion_reason) break - # Check if the review thread was resolved while we were working (no thread for PR comments) + # Check if the review thread was resolved while we were working if review_path: thread_check = get_review_thread_comments( owner=owner_name, diff --git a/services/webhook/test_check_suite_handler.py b/services/webhook/test_check_suite_handler.py index 00862c20a..665e2b2b0 100644 --- a/services/webhook/test_check_suite_handler.py +++ b/services/webhook/test_check_suite_handler.py @@ -169,7 +169,7 @@ async def test_handle_check_suite_skips_when_trigger_disabled( @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.get_pull_request") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.ensure_node_packages") @patch("services.webhook.check_suite_handler.ensure_php_packages") @@ -186,7 +186,7 @@ async def test_handle_check_suite_skips_when_comment_exists( _mock_ensure_php, _mock_start_async, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, mock_slack_notify, mock_get_pr, mock_get_repo, @@ -214,8 +214,12 @@ async def test_handle_check_suite_skips_when_comment_exists( "base": {"ref": "main"}, } mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [ - {"user": {"login": GITHUB_APP_USER_NAME}, "body": CHECK_RUN_FAILED_MESSAGE} + mock_get_pr_comments.return_value = [ + { + "user": {"login": GITHUB_APP_USER_NAME}, + "body": CHECK_RUN_FAILED_MESSAGE, + "created_at": "2025-01-01T00:00:00Z", + } ] await handle_check_suite(payload) @@ -224,7 +228,7 @@ async def test_handle_check_suite_skips_when_comment_exists( mock_get_failed_runs.assert_called_once() mock_get_pr.assert_called_once() mock_get_repo.assert_called() - mock_get_all_comments.assert_called_once() + assert mock_get_pr_comments.call_count == 2 mock_create_comment.assert_not_called() mock_slack_notify.assert_called() @@ -234,7 +238,7 @@ async def test_handle_check_suite_skips_when_comment_exists( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -279,7 +283,7 @@ async def test_handle_check_suite_race_condition_prevention( mock_cancel_workflows, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, mock_slack_notify, mock_get_repo, mock_get_token, @@ -301,7 +305,7 @@ async def test_handle_check_suite_race_condition_prevention( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = ( "https://github.com/test/test/issues/1#issuecomment-123" ) @@ -360,7 +364,7 @@ async def test_handle_check_suite_race_condition_prevention( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -401,7 +405,7 @@ async def test_handle_check_suite_full_workflow( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -423,7 +427,7 @@ async def test_handle_check_suite_full_workflow( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = "usage-id-123" mock_get_pr.return_value = { @@ -494,7 +498,7 @@ async def test_handle_check_suite_full_workflow( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -527,7 +531,7 @@ async def test_handle_check_suite_with_404_logs( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -549,7 +553,7 @@ async def test_handle_check_suite_with_404_logs( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = "usage-id-123" mock_get_pr.return_value = { @@ -593,7 +597,7 @@ async def test_handle_check_suite_with_404_logs( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -622,7 +626,7 @@ async def test_handle_check_suite_with_none_logs( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -644,7 +648,7 @@ async def test_handle_check_suite_with_none_logs( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = "usage-id-123" mock_get_pr.return_value = { @@ -684,7 +688,7 @@ async def test_handle_check_suite_with_none_logs( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -721,7 +725,7 @@ async def test_handle_check_suite_with_existing_retry_pair( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -743,7 +747,7 @@ async def test_handle_check_suite_with_existing_retry_pair( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = "usage-id-123" mock_get_pr.return_value = { @@ -796,7 +800,7 @@ async def test_handle_check_suite_with_existing_retry_pair( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -835,7 +839,7 @@ async def test_handle_check_suite_with_closed_pr( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -857,7 +861,7 @@ async def test_handle_check_suite_with_closed_pr( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = "usage-id-123" mock_get_pr.return_value = { @@ -899,7 +903,7 @@ async def test_handle_check_suite_with_closed_pr( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -938,7 +942,7 @@ async def test_handle_check_suite_with_deleted_branch( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -960,7 +964,7 @@ async def test_handle_check_suite_with_deleted_branch( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = "usage-id-123" mock_get_pr.return_value = { @@ -1002,7 +1006,7 @@ async def test_handle_check_suite_with_deleted_branch( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -1043,7 +1047,7 @@ async def test_check_run_handler_token_accumulation( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -1065,7 +1069,7 @@ async def test_check_run_handler_token_accumulation( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = 888 mock_get_pr.return_value = { @@ -1127,7 +1131,7 @@ async def test_check_run_handler_token_accumulation( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -1170,7 +1174,7 @@ async def test_handle_check_suite_skips_duplicate_older_request( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, mock_slack_notify, mock_get_repo, mock_get_token, @@ -1192,7 +1196,7 @@ async def test_handle_check_suite_skips_duplicate_older_request( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_slack_notify.return_value = "thread-123" mock_create_user_request.return_value = 999 @@ -1251,7 +1255,7 @@ async def test_handle_check_suite_skips_duplicate_older_request( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -1295,7 +1299,7 @@ async def test_handle_check_suite_codecov_failure( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -1321,7 +1325,7 @@ async def test_handle_check_suite_codecov_failure( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = "usage-id-123" mock_get_pr.return_value = { @@ -1388,7 +1392,7 @@ async def test_handle_check_suite_codecov_failure( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -1430,7 +1434,7 @@ async def test_handle_check_suite_codecov_no_token( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -1456,7 +1460,7 @@ async def test_handle_check_suite_codecov_no_token( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = "usage-id-123" mock_get_pr.return_value = { @@ -1510,7 +1514,7 @@ async def test_handle_check_suite_codecov_no_token( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -1552,7 +1556,7 @@ async def test_handle_check_suite_max_iterations_forces_verification( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, _mock_slack_notify, mock_get_repo, mock_get_token, @@ -1574,7 +1578,7 @@ async def test_handle_check_suite_max_iterations_forces_verification( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_create_user_request.return_value = "usage-id-123" mock_get_pr.return_value = { @@ -1632,7 +1636,7 @@ async def test_handle_check_suite_max_iterations_forces_verification( @patch("services.webhook.check_suite_handler.get_installation_access_token") @patch("services.webhook.check_suite_handler.get_repository") @patch("services.webhook.check_suite_handler.slack_notify") -@patch("services.webhook.check_suite_handler.get_all_comments") +@patch("services.webhook.check_suite_handler.get_pr_comments") @patch("services.webhook.check_suite_handler.create_comment") @patch("services.webhook.check_suite_handler.create_user_request") @patch("services.webhook.check_suite_handler.cancel_workflow_runs") @@ -1667,7 +1671,7 @@ async def test_handle_check_suite_skips_same_error_hash_across_workflow_ids( _mock_cancel_workflow_runs, mock_create_user_request, mock_create_comment, - mock_get_all_comments, + mock_get_pr_comments, mock_slack_notify, mock_get_repo, mock_get_token, @@ -1688,7 +1692,7 @@ async def test_handle_check_suite_skips_same_error_hash_across_workflow_ids( } ] mock_get_repo.return_value = {"trigger_on_test_failure": True} - mock_get_all_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_create_comment.return_value = "http://comment-url" mock_slack_notify.return_value = "thread-123" mock_create_user_request.return_value = 777 diff --git a/services/webhook/test_new_pr_handler.py b/services/webhook/test_new_pr_handler.py index 39228ba18..6a400525c 100644 --- a/services/webhook/test_new_pr_handler.py +++ b/services/webhook/test_new_pr_handler.py @@ -189,13 +189,13 @@ async def test_stripe_customer_id_update( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") async def test_image_urls_processing( mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -229,7 +229,15 @@ async def test_image_urls_processing( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = ["Comment text"] + mock_get_pr_comments.return_value = [ + { + "id": 1, + "body": "Comment text", + "user": {"login": "testuser"}, + "created_at": "2025-01-01T00:00:00Z", + "performed_via_github_app": None, + } + ] mock_extract_image_urls.side_effect = [ [{"url": "http://example.com/image.png", "alt": "image"}], [{"url": "http://example.com/comment.png", "alt": "comment img"}], @@ -277,13 +285,13 @@ async def test_image_urls_processing( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") async def test_image_unsupported_format_skipped( mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -316,7 +324,7 @@ async def test_image_unsupported_format_skipped( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [ {"url": "http://example.com/image.svg", "alt": "svg image"} ] @@ -357,13 +365,13 @@ async def test_image_unsupported_format_skipped( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") async def test_image_base64_fetch_failed( mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -397,7 +405,7 @@ async def test_image_base64_fetch_failed( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [ {"url": "http://example.com/image.png", "alt": "image"} ] @@ -445,13 +453,13 @@ async def test_image_base64_fetch_failed( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") async def test_timeout_approaching_breaks_loop( mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -485,7 +493,7 @@ async def test_timeout_approaching_breaks_loop( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_get_pr_files.return_value = [] @@ -520,13 +528,13 @@ async def test_timeout_approaching_breaks_loop( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") async def test_branch_deleted_breaks_loop( mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -560,7 +568,7 @@ async def test_branch_deleted_breaks_loop( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_get_pr_files.return_value = [] @@ -597,13 +605,13 @@ async def test_branch_deleted_breaks_loop( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") async def test_retry_loop_exhausted_not_explored_but_committed( mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -638,7 +646,7 @@ async def test_retry_loop_exhausted_not_explored_but_committed( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_get_pr_files.return_value = [] @@ -781,13 +789,13 @@ async def test_retry_loop_exhausted_not_explored_but_committed( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") async def test_retry_loop_exhausted_explored_but_not_committed( mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -822,7 +830,7 @@ async def test_retry_loop_exhausted_explored_but_not_committed( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_get_pr_files.return_value = [] @@ -953,13 +961,13 @@ async def test_retry_loop_exhausted_explored_but_not_committed( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") async def test_retry_counter_reset_on_successful_loop( mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -993,7 +1001,7 @@ async def test_retry_counter_reset_on_successful_loop( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_get_pr_files.return_value = [] @@ -1060,7 +1068,7 @@ async def test_retry_counter_reset_on_successful_loop( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") @patch("services.webhook.new_pr_handler.generate_and_upsert_pr_body_section") @@ -1068,7 +1076,7 @@ async def test_non_test_file_skipped_in_header_merge( _mock_generate_pr_body, mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -1103,7 +1111,7 @@ async def test_non_test_file_skipped_in_header_merge( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_chat_with_agent.return_value = AgentResult( @@ -1151,7 +1159,7 @@ async def test_non_test_file_skipped_in_header_merge( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") @patch("services.webhook.new_pr_handler.generate_and_upsert_pr_body_section") @@ -1159,7 +1167,7 @@ async def test_test_file_header_merge( _mock_generate_pr_body, mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -1196,7 +1204,7 @@ async def test_test_file_header_merge( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_chat_with_agent.return_value = AgentResult( @@ -1247,7 +1255,7 @@ async def test_test_file_header_merge( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") @patch("services.webhook.new_pr_handler.generate_and_upsert_pr_body_section") @@ -1255,7 +1263,7 @@ async def test_test_file_header_merge_no_content( _mock_generate_pr_body, mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -1292,7 +1300,7 @@ async def test_test_file_header_merge_no_content( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_chat_with_agent.return_value = AgentResult( @@ -1350,7 +1358,7 @@ def read_side_effect(file_path, base_dir=None): @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") @patch("services.webhook.new_pr_handler.generate_and_upsert_pr_body_section") @@ -1358,7 +1366,7 @@ async def test_test_file_header_merge_no_change( _mock_generate_pr_body, mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -1395,7 +1403,7 @@ async def test_test_file_header_merge_no_change( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_chat_with_agent.return_value = AgentResult( @@ -1451,13 +1459,13 @@ async def test_test_file_header_merge_no_change( @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.create_comment") @patch("services.webhook.new_pr_handler.render_text") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.check_availability") @patch("services.webhook.new_pr_handler.deconstruct_github_payload") async def test_credits_depleted_email_sent( mock_deconstruct, mock_check_availability, - mock_get_comments, + mock_get_pr_comments, mock_render_text, mock_create_comment, mock_create_progress_bar, @@ -1498,7 +1506,7 @@ async def test_credits_depleted_email_sent( "user_message": "", "log_message": "Proceeding", } - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_extract_image_urls.return_value = [] mock_get_remote_file.return_value = ("", "") mock_chat_with_agent.return_value = AgentResult( @@ -1539,7 +1547,7 @@ async def test_credits_depleted_email_sent( @patch("services.webhook.new_pr_handler.should_bail", return_value=False) @patch("services.webhook.new_pr_handler.create_empty_commit") @patch("services.webhook.new_pr_handler.get_remote_file_content_by_url") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.slack_notify") @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.update_usage") @@ -1567,7 +1575,7 @@ async def test_new_pr_handler_token_accumulation( mock_update_usage, mock_create_progress_bar, mock_slack_notify, - mock_get_comments, + mock_get_pr_comments, mock_get_remote_file_content_by_url, mock_create_empty_commit, mock_should_bail, @@ -1610,7 +1618,7 @@ async def test_new_pr_handler_token_accumulation( mock_render_text.return_value = "Rendered PR body" mock_slack_notify.return_value = "thread_123" mock_create_progress_bar.return_value = "Progress bar content" - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_get_remote_file_content_by_url.return_value = ("", "") mock_create_empty_commit.return_value = None mock_insert_credit.return_value = None @@ -1714,7 +1722,7 @@ async def test_new_pr_handler_token_accumulation( @patch("services.webhook.new_pr_handler.should_bail", return_value=False) @patch("services.webhook.new_pr_handler.create_empty_commit") @patch("services.webhook.new_pr_handler.get_remote_file_content_by_url") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.slack_notify") @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.update_usage") @@ -1746,7 +1754,7 @@ async def test_few_test_files_include_contents_in_prompt( mock_update_usage, mock_create_progress_bar, mock_slack_notify, - mock_get_comments, + mock_get_pr_comments, mock_get_remote_file_content_by_url, mock_create_empty_commit, mock_should_bail, @@ -1814,7 +1822,7 @@ async def test_few_test_files_include_contents_in_prompt( mock_update_usage.return_value = None mock_create_progress_bar.return_value = "Progress: 0%" mock_slack_notify.return_value = "thread_1" - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_get_remote_file_content_by_url.return_value = ("", "") mock_create_empty_commit.return_value = None mock_insert_credit.return_value = None @@ -1865,7 +1873,7 @@ async def test_few_test_files_include_contents_in_prompt( @patch("services.webhook.new_pr_handler.should_bail", return_value=False) @patch("services.webhook.new_pr_handler.create_empty_commit") @patch("services.webhook.new_pr_handler.get_remote_file_content_by_url") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.slack_notify") @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.update_usage") @@ -1897,7 +1905,7 @@ async def test_many_test_files_include_paths_only_in_prompt( mock_update_usage, mock_create_progress_bar, mock_slack_notify, - mock_get_comments, + mock_get_pr_comments, mock_get_remote_file_content_by_url, mock_create_empty_commit, mock_should_bail, @@ -1961,7 +1969,7 @@ async def test_many_test_files_include_paths_only_in_prompt( mock_update_usage.return_value = None mock_create_progress_bar.return_value = "Progress: 0%" mock_slack_notify.return_value = "thread_1" - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_get_remote_file_content_by_url.return_value = ("", "") mock_create_empty_commit.return_value = None mock_insert_credit.return_value = None @@ -2015,7 +2023,7 @@ async def test_many_test_files_include_paths_only_in_prompt( @patch("services.webhook.new_pr_handler.should_bail", return_value=False) @patch("services.webhook.new_pr_handler.create_empty_commit") @patch("services.webhook.new_pr_handler.get_remote_file_content_by_url") -@patch("services.webhook.new_pr_handler.get_comments") +@patch("services.webhook.new_pr_handler.get_pr_comments") @patch("services.webhook.new_pr_handler.slack_notify") @patch("services.webhook.new_pr_handler.create_progress_bar") @patch("services.webhook.new_pr_handler.update_usage") @@ -2049,7 +2057,7 @@ async def test_auto_detect_location_ignores_dashboard_setting( mock_update_usage, mock_create_progress_bar, mock_slack_notify, - mock_get_comments, + mock_get_pr_comments, mock_get_remote_file_content_by_url, mock_create_empty_commit, mock_should_bail, @@ -2101,7 +2109,7 @@ async def test_auto_detect_location_ignores_dashboard_setting( mock_update_usage.return_value = None mock_create_progress_bar.return_value = "Progress: 0%" mock_slack_notify.return_value = "thread_1" - mock_get_comments.return_value = [] + mock_get_pr_comments.return_value = [] mock_get_remote_file_content_by_url.return_value = ("", "") mock_create_empty_commit.return_value = None mock_insert_credit.return_value = None diff --git a/services/webhook/test_review_run_handler.py b/services/webhook/test_review_run_handler.py index ff78afe10..371a28033 100644 --- a/services/webhook/test_review_run_handler.py +++ b/services/webhook/test_review_run_handler.py @@ -79,7 +79,11 @@ def mock_review_comment_payload(): @patch("services.webhook.review_run_handler.create_user_request") @patch("services.webhook.review_run_handler.get_review_thread_comments") @patch("services.webhook.review_run_handler.reply_to_comment") -@patch("services.webhook.review_run_handler.get_local_file_content") +@patch( + "services.webhook.review_run_handler.format_content_with_line_numbers", + side_effect=lambda file_path, content: content, +) +@patch("services.webhook.review_run_handler.read_local_file") @patch("services.webhook.review_run_handler.get_pull_request_files") @patch("services.webhook.review_run_handler.update_comment") @patch("services.webhook.review_run_handler.should_bail", return_value=False) @@ -115,6 +119,7 @@ async def test_review_run_handler_accumulates_tokens_correctly( mock_update_comment, mock_get_pr_files, mock_get_file_content, + _mock_format, mock_reply_to_comment, mock_get_thread_comments, mock_create_user_request, @@ -215,7 +220,11 @@ async def test_review_run_handler_accumulates_tokens_correctly( @patch("services.webhook.review_run_handler.create_user_request") @patch("services.webhook.review_run_handler.get_review_thread_comments") @patch("services.webhook.review_run_handler.reply_to_comment") -@patch("services.webhook.review_run_handler.get_local_file_content") +@patch( + "services.webhook.review_run_handler.format_content_with_line_numbers", + side_effect=lambda file_path, content: content, +) +@patch("services.webhook.review_run_handler.read_local_file") @patch("services.webhook.review_run_handler.get_pull_request_files") @patch("services.webhook.review_run_handler.update_comment") @patch("services.webhook.review_run_handler.should_bail", return_value=False) @@ -252,6 +261,7 @@ async def test_review_run_handler_max_iterations_forces_verification( mock_update_comment, mock_get_pr_files, mock_get_file_content, + _mock_format, mock_reply_to_comment, mock_get_thread_comments, mock_create_user_request, @@ -382,7 +392,11 @@ def mock_bot_review_comment_payload(): @patch("services.webhook.review_run_handler.create_user_request") @patch("services.webhook.review_run_handler.get_review_thread_comments") @patch("services.webhook.review_run_handler.reply_to_comment") -@patch("services.webhook.review_run_handler.get_local_file_content") +@patch( + "services.webhook.review_run_handler.format_content_with_line_numbers", + side_effect=lambda file_path, content: content, +) +@patch("services.webhook.review_run_handler.read_local_file") @patch("services.webhook.review_run_handler.get_pull_request_files") @patch("services.webhook.review_run_handler.update_comment") @patch("services.webhook.review_run_handler.should_bail", return_value=False) @@ -418,6 +432,7 @@ async def test_thread_resolved_during_loop_stops_agent( _mock_update_comment, mock_get_pr_files, mock_get_file_content, + _mock_format, mock_reply_to_comment, mock_get_thread_comments, mock_create_user_request, @@ -473,7 +488,11 @@ async def test_thread_resolved_during_loop_stops_agent( @patch("services.webhook.review_run_handler.create_user_request") @patch("services.webhook.review_run_handler.get_review_thread_comments") @patch("services.webhook.review_run_handler.reply_to_comment") -@patch("services.webhook.review_run_handler.get_local_file_content") +@patch( + "services.webhook.review_run_handler.format_content_with_line_numbers", + side_effect=lambda file_path, content: content, +) +@patch("services.webhook.review_run_handler.read_local_file") @patch("services.webhook.review_run_handler.get_pull_request_files") @patch("services.webhook.review_run_handler.update_comment") @patch("services.webhook.review_run_handler.should_bail", return_value=False) @@ -509,6 +528,7 @@ async def test_bot_first_review_comment_is_processed( _mock_update_comment, mock_get_pr_files, mock_get_file_content, + _mock_format, mock_reply_to_comment, mock_get_thread_comments, mock_create_user_request, @@ -658,7 +678,11 @@ async def test_bot_reply_after_gitauto_replied_is_skipped( @patch("services.webhook.review_run_handler.create_user_request") @patch("services.webhook.review_run_handler.get_review_thread_comments") @patch("services.webhook.review_run_handler.reply_to_comment") -@patch("services.webhook.review_run_handler.get_local_file_content") +@patch( + "services.webhook.review_run_handler.format_content_with_line_numbers", + side_effect=lambda file_path, content: content, +) +@patch("services.webhook.review_run_handler.read_local_file") @patch("services.webhook.review_run_handler.get_pull_request_files") @patch("services.webhook.review_run_handler.update_comment") @patch("services.webhook.review_run_handler.should_bail", return_value=False) @@ -694,6 +718,7 @@ async def test_human_review_comment_always_processed( _mock_update_comment, mock_get_pr_files, mock_get_file_content, + _mock_format, mock_reply_to_comment, mock_get_thread_comments, mock_create_user_request, @@ -817,7 +842,11 @@ def mock_pr_comment_payload(): @patch("services.webhook.review_run_handler.get_review_thread_comments") @patch("services.webhook.review_run_handler.reply_to_comment") @patch("services.webhook.review_run_handler.create_comment") -@patch("services.webhook.review_run_handler.get_local_file_content") +@patch( + "services.webhook.review_run_handler.format_content_with_line_numbers", + side_effect=lambda file_path, content: content, +) +@patch("services.webhook.review_run_handler.read_local_file") @patch("services.webhook.review_run_handler.get_pull_request_files") @patch("services.webhook.review_run_handler.update_comment") @patch("services.webhook.review_run_handler.should_bail", return_value=False) @@ -853,6 +882,7 @@ async def test_pr_comment_uses_create_comment_not_reply( _mock_update_comment, mock_get_pr_files, _mock_get_file_content, + _mock_format, mock_create_comment, mock_reply_to_comment, mock_get_thread_comments, @@ -902,7 +932,7 @@ async def test_pr_comment_uses_create_comment_not_reply( # get_review_thread_comments NOT called (no thread for PR comments) mock_get_thread_comments.assert_not_called() - # get_local_file_content NOT called (no review_path) + # read_local_file NOT called (no review_path) _mock_get_file_content.assert_not_called() # Agent was called @@ -923,7 +953,11 @@ async def test_pr_comment_uses_create_comment_not_reply( @patch("services.webhook.review_run_handler.get_review_thread_comments") @patch("services.webhook.review_run_handler.reply_to_comment") @patch("services.webhook.review_run_handler.create_comment") -@patch("services.webhook.review_run_handler.get_local_file_content") +@patch( + "services.webhook.review_run_handler.format_content_with_line_numbers", + side_effect=lambda file_path, content: content, +) +@patch("services.webhook.review_run_handler.read_local_file") @patch("services.webhook.review_run_handler.get_pull_request_files") @patch("services.webhook.review_run_handler.update_comment") @patch("services.webhook.review_run_handler.should_bail", return_value=False) @@ -959,6 +993,7 @@ async def test_question_comment_agent_replies_without_code_changes( _mock_update_comment, mock_get_pr_files, _mock_get_file_content, + _mock_format, mock_create_comment, mock_reply_to_comment, mock_get_thread_comments, @@ -1103,7 +1138,11 @@ async def test_bot_pr_comment_irrelevant_to_pr_files_is_skipped( @patch("services.webhook.review_run_handler.get_review_thread_comments") @patch("services.webhook.review_run_handler.reply_to_comment") @patch("services.webhook.review_run_handler.create_comment") -@patch("services.webhook.review_run_handler.get_local_file_content") +@patch( + "services.webhook.review_run_handler.format_content_with_line_numbers", + side_effect=lambda file_path, content: content, +) +@patch("services.webhook.review_run_handler.read_local_file") @patch("services.webhook.review_run_handler.get_pull_request_files") @patch("services.webhook.review_run_handler.update_comment") @patch("services.webhook.review_run_handler.should_bail", return_value=False) @@ -1139,6 +1178,7 @@ async def test_bot_pr_comment_mentioning_pr_file_is_processed( _mock_update_comment, mock_get_pr_files, _mock_get_file_content, + _mock_format, mock_create_comment, mock_reply_to_comment, mock_get_thread_comments, @@ -1182,3 +1222,221 @@ async def test_bot_pr_comment_mentioning_pr_file_is_processed( await handle_review_run(mock_bot_pr_comment_payload, trigger="pr_comment") mock_chat_with_agent.assert_called_once() + + +@pytest.fixture +def mock_batched_review_payload(): + """Payload for a review comment that has a pull_request_review_id (part of a multi-comment review).""" + return { + "action": "created", + "comment": { + "id": 12345, + "node_id": "PRRC_kwDOJTestNodeId", + "path": "src/main.py", + "subject_type": "line", + "line": 42, + "side": "RIGHT", + "body": "Fix this function.", + "user": {"login": "test-reviewer", "type": "User"}, + "pull_request_review_id": 77777, + }, + "pull_request": { + "number": 123, + "title": "Add new feature", + "body": "This PR adds a new feature", + "url": "https://api.github.com/repos/test-owner/test-repo/pulls/123", + "user": {"login": "gitauto-ai[bot]"}, + "head": { + "ref": f"{PRODUCT_ID}/dashboard-20250101-155924-Ab1C", + "sha": "abc123def456", + }, + "base": {"ref": "main"}, + }, + "repository": { + "id": 98765, + "name": "test-repo", + "owner": { + "id": 11111, + "login": "test-owner", + "type": "Organization", + }, + "clone_url": "https://github.com/test-owner/test-repo.git", + "fork": False, + }, + "sender": {"id": 22222, "login": "test-reviewer"}, + "installation": {"id": 33333}, + } + + +@patch( + "services.webhook.review_run_handler.insert_webhook_delivery", return_value=False +) +@patch("services.webhook.review_run_handler.get_review_summary_comment") +@patch("services.webhook.review_run_handler.set_npm_token_env") +@patch("services.webhook.review_run_handler.get_installation_access_token") +@patch("services.webhook.review_run_handler.get_user_public_info") +@patch("services.webhook.review_run_handler.get_repository") +@patch("services.webhook.review_run_handler.check_purchase_exists") +@patch("services.webhook.review_run_handler.get_email_from_commits") +@patch("services.webhook.review_run_handler.GITHUB_APP_USER_NAME", "gitauto-ai[bot]") +@pytest.mark.asyncio +async def test_batch_dedup_second_invocation_returns_early( + _mock_get_email, + _mock_check_purchase, + mock_get_repo, + mock_get_user_public_info, + mock_get_token, + _mock_set_npm_token_env, + _mock_get_review_summary_comment, + mock_insert_webhook_delivery, + mock_batched_review_payload, +): + """Second invocation for same review_id should return early (insert_webhook_delivery returns False).""" + mock_get_token.return_value = "ghs_test_token" + mock_get_user_public_info.return_value = type( + "UserPublicInfo", (), {"email": "test@test.com", "display_name": "Test"} + )() + mock_get_repo.return_value = {"id": 98765, "trigger_on_review_comment": True} + + result = await handle_review_run( + mock_batched_review_payload, trigger="pr_file_review" + ) + + assert result is None + mock_insert_webhook_delivery.assert_called_once_with( + delivery_id="review-dedup-98765-123-77777", + event_name="pr_file_review_dedup", + ) + + +@patch("services.webhook.review_run_handler.get_pull_request") +@patch("services.webhook.review_run_handler.slack_notify") +@patch("services.webhook.review_run_handler.get_local_file_tree", return_value=[]) +@patch("services.webhook.review_run_handler.set_npm_token_env") +@patch("services.webhook.review_run_handler.get_installation_access_token") +@patch("services.webhook.review_run_handler.get_user_public_info") +@patch("services.webhook.review_run_handler.get_repository") +@patch("services.webhook.review_run_handler.create_user_request") +@patch("services.webhook.review_run_handler.get_review_thread_comments") +@patch("services.webhook.review_run_handler.reply_to_comment") +@patch( + "services.webhook.review_run_handler.format_content_with_line_numbers", + side_effect=lambda file_path, content: content, +) +@patch("services.webhook.review_run_handler.read_local_file") +@patch("services.webhook.review_run_handler.get_pull_request_files") +@patch("services.webhook.review_run_handler.update_comment") +@patch("services.webhook.review_run_handler.should_bail", return_value=False) +@patch("services.webhook.review_run_handler.chat_with_agent") +@patch("services.webhook.review_run_handler.create_empty_commit") +@patch("services.webhook.review_run_handler.get_reference", return_value="changed_sha") +@patch("services.webhook.review_run_handler.update_usage") +@patch("services.webhook.review_run_handler.ensure_node_packages") +@patch("services.webhook.review_run_handler.clone_repo_and_install_dependencies") +@patch( + "services.webhook.review_run_handler.get_head_commit_count_behind_base", + return_value=0, +) +@patch("services.webhook.review_run_handler.git_merge_base_into_pr") +@patch("services.webhook.review_run_handler.ensure_php_packages") +@patch( + "services.webhook.review_run_handler.verify_task_is_ready", new_callable=AsyncMock +) +@patch("services.webhook.review_run_handler.insert_webhook_delivery", return_value=True) +@patch("services.webhook.review_run_handler.get_review_inline_comments") +@patch( + "services.webhook.review_run_handler.get_review_summary_comment", return_value="" +) +@patch("services.webhook.review_run_handler.check_purchase_exists") +@patch("services.webhook.review_run_handler.get_email_from_commits") +@patch("services.webhook.review_run_handler.time") +@patch("services.webhook.review_run_handler.get_pr_comments", return_value=[]) +@patch("services.webhook.review_run_handler.GITHUB_APP_USER_NAME", "gitauto-ai[bot]") +@pytest.mark.asyncio +async def test_batch_builds_combined_review_comment( + _mock_get_pr_comments, + mock_time, + _mock_get_email, + _mock_check_purchase, + _mock_get_review_summary_comment, + mock_get_review_inline_comments, + _mock_insert_webhook_delivery, + _mock_verify_task_is_ready, + _mock_ensure_php, + _mock_merge_base, + _mock_get_behind, + _mock_prepare_repo, + _mock_ensure_node, + _mock_update_usage, + _mock_get_reference, + _mock_create_empty_commit, + mock_chat_with_agent, + _mock_should_bail, + _mock_update_comment, + mock_get_pr_files, + mock_get_file_content, + _mock_format, + mock_reply_to_comment, + _mock_get_thread_comments, + mock_create_user_request, + mock_get_repo, + mock_get_user_public_info, + mock_get_token, + _mock_set_npm_token_env, + _mock_get_local_file_tree, + _mock_slack_notify, + _mock_get_pull_request, + mock_batched_review_payload, +): + """When batched, review_comment should combine all inline comments from the review.""" + mock_time.time.return_value = 1000.0 + mock_time.sleep = lambda x: None + mock_get_token.return_value = "ghs_test_token" + mock_get_user_public_info.return_value = type( + "UserPublicInfo", (), {"email": "test@test.com", "display_name": "Test"} + )() + mock_get_repo.return_value = {"id": 98765, "trigger_on_review_comment": True} + mock_create_user_request.return_value = 777 + _mock_verify_task_is_ready.return_value = VerifyTaskIsReadyResult() + mock_reply_to_comment.return_value = "http://comment-url" + mock_get_file_content.return_value = "def main():\n pass" + mock_get_pr_files.return_value = [{"filename": "src/main.py", "status": "modified"}] + + # Return 3 inline comments for the review + mock_get_review_inline_comments.return_value = [ + {"id": 1, "body": "Fix this", "path": "src/main.py", "line": 10}, + {"id": 2, "body": "Rename var", "path": "src/main.py", "line": 20}, + {"id": 3, "body": "Add test", "path": "src/utils.py", "line": 5}, + ] + + _mock_get_thread_comments.return_value = type( + "ThreadCheck", (), {"is_resolved": False} + )() + + mock_chat_with_agent.return_value = AgentResult( + messages=[{"role": "user", "content": "review"}], + token_input=100, + token_output=50, + is_completed=True, + completion_reason="Done.", + p=40, + is_planned=False, + cost_usd=0.0, + ) + + await handle_review_run(mock_batched_review_payload, trigger="pr_file_review") + + mock_chat_with_agent.assert_called_once() + call_kwargs = mock_chat_with_agent.call_args.kwargs + base_args = call_kwargs["base_args"] + review_comment = base_args["review_comment"] + # Verify combined comment structure + assert "3 inline comments" in review_comment + assert "Comment 1:" in review_comment + assert "Comment 2:" in review_comment + assert "Comment 3:" in review_comment + assert "Fix this" in review_comment + assert "Rename var" in review_comment + assert "Add test" in review_comment + # Verify file content was read for both unique paths + assert mock_get_file_content.call_count == 2 diff --git a/services/webhook/test_webhook_handler.py b/services/webhook/test_webhook_handler.py index f3aa249a8..e1ec19052 100644 --- a/services/webhook/test_webhook_handler.py +++ b/services/webhook/test_webhook_handler.py @@ -17,6 +17,7 @@ PrLabeledPayload, ) from services.github.types.pull_request_webhook_payload import PullRequestWebhookPayload +from services.github.types.webhook.pr_comment import IssueCommentWebhookPayload from services.webhook.webhook_handler import handle_webhook_event @@ -1401,3 +1402,8 @@ async def test_pr_review_null_body_sends_slack_only( mock_slack_notify.assert_not_called() mock_handle_review_run.assert_not_called() + + +def test_issue_comment_webhook_payload_importable(): + """Verify IssueCommentWebhookPayload is importable from pr_comment module (renamed from issue_comment).""" + assert IssueCommentWebhookPayload is not None diff --git a/services/webhook/utils/adapt_pr_comment_to_review_payload.py b/services/webhook/utils/adapt_pr_comment_to_review_payload.py index be8caf588..1b217ea7c 100644 --- a/services/webhook/utils/adapt_pr_comment_to_review_payload.py +++ b/services/webhook/utils/adapt_pr_comment_to_review_payload.py @@ -1,5 +1,5 @@ from services.github.types.pull_request import PullRequest -from services.github.types.webhook.issue_comment import IssueCommentWebhookPayload +from services.github.types.webhook.pr_comment import IssueCommentWebhookPayload from services.github.types.webhook.review_run_payload import ReviewRunPayload from utils.error.handle_exceptions import handle_exceptions diff --git a/services/webhook/utils/test_adapt_pr_comment_to_review_payload.py b/services/webhook/utils/test_adapt_pr_comment_to_review_payload.py index 8e480ce15..3b818c60c 100644 --- a/services/webhook/utils/test_adapt_pr_comment_to_review_payload.py +++ b/services/webhook/utils/test_adapt_pr_comment_to_review_payload.py @@ -3,7 +3,7 @@ from typing import cast from services.github.types.pull_request import PullRequest -from services.github.types.webhook.issue_comment import IssueCommentWebhookPayload +from services.github.types.webhook.pr_comment import IssueCommentWebhookPayload from services.webhook.utils.adapt_pr_comment_to_review_payload import ( adapt_pr_comment_to_review_payload, ) diff --git a/services/webhook/webhook_handler.py b/services/webhook/webhook_handler.py index cde1341a5..8b3ed960a 100644 --- a/services/webhook/webhook_handler.py +++ b/services/webhook/webhook_handler.py @@ -17,7 +17,7 @@ PrClosedPayload, PrLabeledPayload, ) -from services.github.types.webhook.issue_comment import IssueCommentWebhookPayload +from services.github.types.webhook.pr_comment import IssueCommentWebhookPayload from services.github.types.webhook.pull_request_review import PullRequestReviewPayload from services.github.types.webhook.push import PushWebhookPayload from services.slack.slack_notify import slack_notify diff --git a/uv.lock b/uv.lock index eef20a740..88c8fb8bc 100644 --- a/uv.lock +++ b/uv.lock @@ -596,7 +596,7 @@ wheels = [ [[package]] name = "gitauto" -version = "1.6.11" +version = "1.12.0" source = { virtual = "." } dependencies = [ { name = "annotated-doc" },