feat(#97): add code suggestions to GitHub comments (#113)

Author: scastlara

src/lgtm_ai/__main__.py

@@ -14,7 +14,7 @@
 )
 from lgtm_ai.ai.schemas import AgentSettings, CommentCategory, SupportedAIModels, SupportedAIModelsList
 from lgtm_ai.base.schemas import IntOrNoLimit, IssuesSource, OutputFormat, PRUrl
-from lgtm_ai.base.utils import git_source_supports_suggestions
+from lgtm_ai.base.utils import git_source_supports_multiline_suggestions
 from lgtm_ai.config.constants import DEFAULT_INPUT_TOKEN_LIMIT
 from lgtm_ai.config.handler import ConfigHandler, PartialConfig, ResolvedConfig
 from lgtm_ai.formatters.base import Formatter
@@ -177,7 +177,9 @@ def review(
         config_file=config,
     ).resolve_config()
     agent_extra_settings = AgentSettings(retries=resolved_config.ai_retries)
-    formatter: Formatter[Any] = MarkDownFormatter(use_suggestions=git_source_supports_suggestions(pr_url.source))
+    formatter: Formatter[Any] = MarkDownFormatter(
+        add_ranges_to_suggestions=git_source_supports_multiline_suggestions(pr_url.source)
+    )
     git_client = get_git_client(source=pr_url.source, token=resolved_config.git_api_key, formatter=formatter)
     issues_client = _get_issues_client(resolved_config, git_client, formatter)
 

src/lgtm_ai/base/utils.py

@@ -14,9 +14,10 @@ def file_matches_any_pattern(file_name: str, patterns: tuple[str, ...]) -> bool:
     return False
 
 
-def git_source_supports_suggestions(source: PRSource) -> bool:
-    """For now, we only support suggestions in GitLab.
+def git_source_supports_multiline_suggestions(source: PRSource) -> bool:
+    """Check if the given git source supports multiline suggestions.
 
-    TODO: https://github.com/elementsinteractive/lgtm-ai/issues/96
+    GitLab does support specifying suggestions that span multiple lines in single-line comments.
+    GitHub requires the comment to be multi-line, and the suggestion does not need special markup with ranges.
     """
     return source == PRSource.gitlab

src/lgtm_ai/formatters/markdown.py

@@ -15,8 +15,8 @@ class MarkDownFormatter(Formatter[str]):
     SNIPPET_TEMPLATE: ClassVar[str] = "snippet.md.j2"
     METADATA_TEMPLATE: ClassVar[str] = "metadata.md.j2"
 
-    def __init__(self, use_suggestions: bool = False) -> None:
-        self.use_suggestions = use_suggestions
+    def __init__(self, add_ranges_to_suggestions: bool = False) -> None:
+        self.add_ranges_to_suggestions = add_ranges_to_suggestions
         template_dir = pathlib.Path(__file__).parent / "templates"
         self._template_env = Environment(loader=FileSystemLoader(template_dir), autoescape=True)
 
@@ -52,7 +52,7 @@ def format_review_comment(self, comment: ReviewComment, *, with_footer: bool = T
             snippet=snippet,
             comment=comment.comment,
             suggestion=comment.suggestion,
-            use_suggestions=self.use_suggestions,
+            add_ranges_to_suggestions=self.add_ranges_to_suggestions,
             with_footer=with_footer,
             new_path=comment.new_path,
             line_number=comment.line_number,

src/lgtm_ai/formatters/templates/review_comment.md.j2

@@ -9,8 +9,8 @@
 {{ comment | safe }}
 
 {% if suggestion %}
-{% if use_suggestions and suggestion.ready_for_replacement %}
-`````suggestion:{{ suggestion.start_offset.direction}}{{ suggestion.start_offset.offset }}{{ suggestion.end_offset.direction }}{{ suggestion.end_offset.offset }}
+{% if suggestion.ready_for_replacement %}
+`````suggestion{% if add_ranges_to_suggestions %}:{{ suggestion.start_offset.direction}}{{ suggestion.start_offset.offset }}{{ suggestion.end_offset.direction }}{{ suggestion.end_offset.offset }}{% endif %}
 {{ suggestion.snippet | safe }}
 `````
 {% else %}

src/lgtm_ai/git_client/github.py

@@ -1,6 +1,7 @@
 import binascii
 import logging
 from functools import lru_cache
+from typing import Any, Literal, cast
 from urllib.parse import urlparse
 
 import github
@@ -10,7 +11,7 @@
 import github.PullRequest
 import github.PullRequestReview
 import github.Repository
-from lgtm_ai.ai.schemas import Review, ReviewGuide
+from lgtm_ai.ai.schemas import CodeSuggestionOffset, Review, ReviewComment, ReviewGuide
 from lgtm_ai.base.schemas import PRUrl
 from lgtm_ai.formatters.base import Formatter
 from lgtm_ai.git_client.base import GitClient
@@ -74,16 +75,8 @@ def publish_review(self, pr_url: PRUrl, review: Review) -> None:
         Publish a main summary comment and then specific line comments.
         """
         pr = _get_pr(self.client, pr_url)
-        # Prepare the list of inline comments
-        comments: list[github.PullRequest.ReviewComment] = []
-        for c in review.review_response.comments:
-            comments.append(
-                {
-                    "path": c.new_path,
-                    "position": c.relative_line_number,
-                    "body": self.formatter.format_review_comment(c),
-                }
-            )
+        comment_builder = CommentBuilder(self.formatter)
+        comments = [comment_builder.generate_comment_payload(c) for c in review.review_response.comments]
         try:
             commit = pr.base.repo.get_commit(pr.head.sha)
             pr.create_review(
@@ -92,8 +85,24 @@ def publish_review(self, pr_url: PRUrl, review: Review) -> None:
                 comments=comments,
                 commit=commit,
             )
-        except github.GithubException as err:
-            raise PublishReviewError from err
+        except github.GithubException:
+            try:
+                # Fallback to single-line comments if multi-line comments fail
+                logger.warning(
+                    "Failed to publish review with multi-line comments, falling back to single-line comments"
+                )
+                comments = [
+                    comment_builder.generate_comment_payload(c, force_single_line=True)
+                    for c in review.review_response.comments
+                ]
+                pr.create_review(
+                    body=self.formatter.format_review_summary_section(review),
+                    event="COMMENT",
+                    comments=comments,
+                    commit=commit,
+                )
+            except github.GithubException as err:
+                raise PublishReviewError from err
 
     def get_pr_metadata(self, pr_url: PRUrl) -> PRMetadata:
         """Return a PRMetadata object containing the metadata of the given pull request URL."""
@@ -201,3 +210,82 @@ def _get_repo_from_issues_url(client: github.Github, issues_url: HttpUrl) -> git
         raise ValueError("Invalid GitHub issues URL")
     repo_path = f"{parts[0]}/{parts[1]}"
     return _get_repo(client, repo_path)
+
+
+class CommentBuilder:
+    def __init__(self, formatter: Formatter[str]) -> None:
+        self.formatter = formatter
+
+    def generate_comment_payload(
+        self, comment: ReviewComment, *, force_single_line: bool = False
+    ) -> github.PullRequest.ReviewComment:
+        """Prepare comment data for GitHub API, handling both single-line and multi-line comments."""
+        comment_data: dict[str, Any] = {
+            "path": comment.new_path,
+            "body": self.formatter.format_review_comment(comment),
+        }
+
+        if not force_single_line and comment.suggestion and self._should_create_multiline_comment(comment):
+            # Use the new GitHub API parameters for multi-line comments
+            start_line, end_line = self._calculate_multiline_range(comment)
+            side = self._determine_comment_side(comment)
+            comment_data.update(
+                {
+                    "line": end_line,
+                    "side": side,
+                    "start_line": start_line,
+                    "start_side": side,
+                }
+            )
+        else:
+            # Single-line comment using position (legacy parameter)
+            comment_data["position"] = comment.relative_line_number
+
+        return cast(github.PullRequest.ReviewComment, comment_data)
+
+    def _should_create_multiline_comment(self, comment: ReviewComment) -> bool:
+        """Determine if a comment should be created as multi-line based on suggestion offsets."""
+        if not comment.suggestion or not comment.suggestion.ready_for_replacement:
+            return False
+
+        start_offset = comment.suggestion.start_offset
+        end_offset = comment.suggestion.end_offset
+
+        # Check if the range spans more than one line
+        start_line_offset = self._calculate_line_offset(start_offset)
+        end_line_offset = self._calculate_line_offset(end_offset)
+
+        return start_line_offset != end_line_offset
+
+    def _calculate_multiline_range(self, comment: ReviewComment) -> tuple[int, int]:
+        """Calculate the start and end line numbers for a multi-line comment."""
+        if not comment.suggestion:
+            return comment.line_number, comment.line_number
+
+        base_line = comment.line_number
+        start_line_offset = self._calculate_line_offset(comment.suggestion.start_offset)
+        end_line_offset = self._calculate_line_offset(comment.suggestion.end_offset)
+
+        start_line = base_line + start_line_offset
+        end_line = base_line + end_line_offset
+
+        # Ensure valid line numbers (must be positive)
+        start_line = max(1, start_line)
+        end_line = max(1, end_line)
+
+        # Ensure start_line <= end_line
+        if start_line > end_line:
+            start_line, end_line = end_line, start_line
+
+        return start_line, end_line
+
+    def _calculate_line_offset(self, suggestion_offset: CodeSuggestionOffset) -> int:
+        """Calculate the line offset from a CodeSuggestionOffset."""
+        if suggestion_offset.direction == "-":
+            return -suggestion_offset.offset
+        else:
+            return suggestion_offset.offset
+
+    def _determine_comment_side(self, comment: ReviewComment) -> Literal["LEFT", "RIGHT"]:
+        """Determine the correct side for GitHub API based on line type."""
+        return "RIGHT" if comment.is_comment_on_new_path else "LEFT"

tests/formatters/test_markdown.py

@@ -168,7 +168,7 @@ def test_format_comments_section_several_comments(self) -> None:
         assert self.formatter.format_review_comments_section(review.review_response.comments).split("\n") == expected
 
     def test_format_comments_with_suggestions(self) -> None:
-        formatter = MarkDownFormatter(use_suggestions=True)
+        formatter = MarkDownFormatter(add_ranges_to_suggestions=True)
         review = Review(
             metadata=PublishMetadata(model_name="whatever", usage=MOCK_USAGE),
             review_response=ReviewResponse(
@@ -223,16 +223,20 @@ def test_format_comments_with_suggestions(self) -> None:
         assert formatter.format_review_comments_section(review.review_response.comments).split("\n") == expected
 
     @pytest.mark.parametrize(
-        ("use_suggestions", "ready_for_replacement"),
+        ("add_ranges_to_suggestions", "ready_for_replacement", "header"),
         [
-            (True, False),  # should not format as suggestion block if not ready
-            (False, True),  # even if ready, should not format as suggestion block if use_suggestions is False
+            (True, False, "python"),  # should not format as suggestion block if not ready
+            (
+                False,
+                True,
+                "suggestion",
+            ),  # even if ready, should not add ranges if the flag is off
         ],
     )
     def test_format_comments_with_suggestions_but_formatted_normally(
-        self, use_suggestions: bool, ready_for_replacement: bool
+        self, add_ranges_to_suggestions: bool, ready_for_replacement: bool, header: str
     ) -> None:
-        formatter = MarkDownFormatter(use_suggestions=use_suggestions)
+        formatter = MarkDownFormatter(add_ranges_to_suggestions=add_ranges_to_suggestions)
         review = Review(
             metadata=PublishMetadata(model_name="whatever", usage=MOCK_USAGE),
             review_response=ReviewResponse(
@@ -274,7 +278,7 @@ def test_format_comments_with_suggestions_but_formatted_normally(
             "",
             "",
             "",
-            "`````python",  # not a suggestion block
+            f"`````{header}",
             "print('Hello World')",
             "`````",
             "",