cpp_linter/clang_tools/patcher.py (1)

132-143: Simplify string concatenation for readability

Consider using a single f-string to construct the summary message instead of concatenating multiple strings. This enhances readability and maintains consistency.

Apply this diff to improve the code:

-                summary += (
-                    f"Only {posted_tool_advice[tool_name]} out of "
-                    + f"{self.tool_total[tool_name]} {tool_name}"
-                    + " new concerns fit within this pull request's diff."
-                )
+                summary += f"Only {posted_tool_advice[tool_name]} out of {self.tool_total[tool_name]} {tool_name} new concerns fit within this pull request's diff."

cpp_linter/rest_api/github_api.py (1)

507-574: Improve logging for clarity in _close_review_comment

When deleting a comment, the logging message references thread_id, which may not correspond to the operation being performed. Update the logging to reference the correct ID based on the operation.

Apply this diff to enhance the logging:

             response = self.api_request(
                 url=f"{self.api_url}/graphql",
                 method="POST",
                 data=json.dumps({"query": mutation, "variables": variables}),
                 strict=False,
             )
-            logger.debug(
-                "%s review comment thread %s (thread_id: %s)",
-                operation.title(),
-                "failed" if response.status_code != 200 else "succeeded",
-                thread_id,
-            )
+            logger.debug(
+                "%s review comment %s (%s: %s)",
+                operation.title(),
+                "failed" if response.status_code != 200 else "succeeded",
+                "comment_id" if delete else "thread_id",
+                comment_id if delete else thread_id,
+            )

This ensures that the log message accurately reflects the operation and references the correct identifier.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cpp_linter/rest_api/github_api.py (2)

474-475: Update method docstring to include new parameters

The method has new parameters but the docstring hasn't been updated to reflect these changes. Consider adding parameter descriptions for delete_review_comments and reuse_review_comments.

     def post_review(
         self,
         files: List[FileObj],
         tidy_review: bool,
         format_review: bool,
         no_lgtm: bool,
         passive_reviews: bool,
         clang_versions: ClangVersions,
         delete_review_comments: bool = True,
         reuse_review_comments: bool = True,
+    ):
+        """Post a review for the pull request.
+
+        :param files: List of files to review
+        :param tidy_review: Flag to enable clang-tidy review
+        :param format_review: Flag to enable clang-format review
+        :param no_lgtm: Flag to control LGTM comments
+        :param passive_reviews: Flag for passive review mode
+        :param clang_versions: Clang tool versions
+        :param delete_review_comments: Flag to delete rather than resolve outdated comments
+        :param reuse_review_comments: Flag to reuse existing comments if they match
+        """
     ):

---

713-778: Consider breaking down the complex review comment checking logic

The _check_reused_comments method is quite long and handles multiple responsibilities. Consider breaking it down into smaller, more focused methods for better maintainability.

Suggested structure:

def _check_reused_comments(self, delete_review_comments: bool, reuse_review_comments: bool, review_comments: ReviewComments) -> List[str]:
    """Main orchestrator method for handling review comments."""
    ignored_reviews = []
    found_threads = self._get_existing_review_comments(
        no_dismissed=reuse_review_comments and not delete_review_comments
    )
    if not found_threads:
        return ignored_reviews
    
    if reuse_review_comments:
        return self._handle_reusable_comments(found_threads, review_comments)
    else:
        self._close_all_comments(found_threads, delete_review_comments)
        return ignored_reviews

def _handle_reusable_comments(self, found_threads: List[Dict], review_comments: ReviewComments) -> List[str]:
    """Handle reusable comments logic."""
    ignored_reviews = []
    existing_review_comments = []
    for thread in found_threads:
        self._process_thread_comments(thread, review_comments, ignored_reviews, existing_review_comments)
    review_comments.remove_reused_suggestions(existing_review_comments=existing_review_comments)
    return ignored_reviews

def _process_thread_comments(self, thread: Dict, review_comments: ReviewComments, ignored_reviews: List[str], existing_review_comments: List):
    """Process individual thread comments."""
    for comment in thread["comments"]["nodes"]:
        if self._is_comment_reusable(comment, thread):
            self._try_reuse_comment(comment, thread, review_comments, ignored_reviews, existing_review_comments)
        else:
            self._close_review_comment(thread["id"], comment["id"], delete_review_comments)

cpp_linter/clang_tools/patcher.py (2)

132-134: Consider improving readability of the condition

The condition could be more readable by breaking it down into smaller parts or using intermediate variables.

Consider this refactor:

-            if len(comments) and (
-                posted_tool_advice[tool_name] != self.tool_total[tool_name]
-                or self.tool_reused[tool_name] != 0
-            ):
+            has_comments = len(comments) > 0
+            has_unposted_concerns = posted_tool_advice[tool_name] != self.tool_total[tool_name]
+            has_reused_concerns = self.tool_reused[tool_name] != 0
+            if has_comments and (has_unposted_concerns or has_reused_concerns):

---

154-178: Consider adding input validation for existing review comments

The remove_reused_suggestions method processes existing review comments without explicit validation. Consider adding validation to ensure the safety of processing user-provided content.

Consider:

1. Validating the structure and content of existing review comments
2. Adding length limits for comments
3. Sanitizing or escaping special characters in comments

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: maoliver-amd
PR: cpp-linter/cpp-linter#131
File: cpp_linter/rest_api/github_api.py:0-0
Timestamp: 2024-12-09T09:50:38.136Z
Learning: In `cpp_linter/rest_api/github_api.py`, when using the GitHub GraphQL API, the maximum allowable pagination limit for `first` and `last` parameters is 100.

cpp_linter/rest_api/github_api.py (1)

748-754: Simplify line number handling logic

The current implementation for handling line numbers is a bit complex. Consider using the null coalescing pattern for better readability.

Apply this diff:

-                        line_start = (
-                            comment.get("startLine", None)
-                            or comment.get("originalStartLine", None)
-                            or -1
-                        )
-                        line_end = comment.get("line", None) or comment["originalLine"]
+                        line_start = next((val for val in [
+                            comment.get("startLine"),
+                            comment.get("originalStartLine"),
+                            -1
+                        ] if val is not None), -1)
+                        line_end = comment.get("line") or comment["originalLine"]

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: maoliver-amd
PR: cpp-linter/cpp-linter#131
File: cpp_linter/rest_api/github_api.py:0-0
Timestamp: 2024-12-09T09:50:38.136Z
Learning: In `cpp_linter/rest_api/github_api.py`, when using the GitHub GraphQL API, the maximum allowable pagination limit for `first` and `last` parameters is 100.

-            data=json.dumps({"query": query, "variables": variables}),
+            data=json.dumps({
+                "query": query,
+                "variables": variables,
+                "operationName": "GetReviewComments"
+            }),

cpp_linter/cli.py (1)

368-369: Fix grammatical error in help text.

The word "effects" should be "affects" as it's being used as a verb.

Apply this change:

-This only effects review comments made when either
+This only affects review comments made when either

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cpp_linter/rest_api/github_api.py (1)

725-781: Consider breaking down the complex logic into smaller functions

While the implementation is correct, the method's complexity could be reduced by extracting some logic into helper functions:

• Comment matching logic (lines 750-761)
• Line number handling (lines 744-749)

Example refactor:

    def _check_reused_comments(
        self,
        delete_review_comments: bool,
        review_comments: ReviewComments,
    ) -> List[str]:
+       def _get_line_numbers(comment: Dict[str, Any]) -> Tuple[int, int]:
+           line_start = (
+               comment.get("startLine", None)
+               or comment.get("originalStartLine", None)
+               or -1
+           )
+           line_end = comment.get("line", None) or comment["originalLine"]
+           return line_start, line_end
+
+       def _is_matching_comment(
+           suggestion: Any,
+           comment: Dict[str, Any],
+           thread: Dict[str, Any],
+           line_start: int,
+           line_end: int,
+           existing_review_comments: List[Any]
+       ) -> bool:
+           return (
+               suggestion.file_name == comment["path"]
+               and suggestion.line_start == line_start
+               and suggestion.line_end == line_end
+               and f"{COMMENT_MARKER}{suggestion.comment}" == comment["body"]
+               and suggestion not in existing_review_comments
+               and thread["isResolved"] is False
+               and thread["isCollapsed"] is False
+               and comment["pullRequestReview"]["isMinimized"] is False
+           )

        ignored_reviews = []
        found_threads = self._get_existing_review_comments(
            no_dismissed=not delete_review_comments
        )
        if found_threads:
            existing_review_comments = []
            for thread in found_threads:
                for comment in thread["comments"]["nodes"]:
                    found = False
                    if "originalLine" not in comment:
                        raise ValueError(
                            "GraphQL response malformed: 'originalLine' missing in comment"
                        )
-                   line_start = (
-                       comment.get("startLine", None)
-                       or comment.get("originalStartLine", None)
-                       or -1
-                   )
-                   line_end = comment.get("line", None) or comment["originalLine"]
+                   line_start, line_end = _get_line_numbers(comment)
                    for suggestion in review_comments.suggestions:
-                       if (
-                           suggestion.file_name == comment["path"]
-                           and suggestion.line_start == line_start
-                           and suggestion.line_end == line_end
-                           and f"{COMMENT_MARKER}{suggestion.comment}"
-                           == comment["body"]
-                           and suggestion not in existing_review_comments
-                           and thread["isResolved"] is False
-                           and thread["isCollapsed"] is False
-                           and comment["pullRequestReview"]["isMinimized"] is False
-                       ):
+                       if _is_matching_comment(
+                           suggestion,
+                           comment,
+                           thread,
+                           line_start,
+                           line_end,
+                           existing_review_comments
+                       ):
                            found = True
                            logger.info(
                                "Using existing review comment: path='%s', line_start='%s', line_end='%s'",

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: maoliver-amd
PR: cpp-linter/cpp-linter#131
File: cpp_linter/rest_api/github_api.py:0-0
Timestamp: 2024-12-09T09:50:38.136Z
Learning: In `cpp_linter/rest_api/github_api.py`, when using the GitHub GraphQL API, the maximum allowable pagination limit for `first` and `last` parameters is 100.