diff --git a/review_roadmap/github/client.py b/review_roadmap/github/client.py index 651602d..e875252 100644 --- a/review_roadmap/github/client.py +++ b/review_roadmap/github/client.py @@ -5,7 +5,7 @@ generate review roadmaps. """ -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple import httpx @@ -355,6 +355,78 @@ def check_write_access( ) ) + def minimize_old_roadmap_comments( + self, owner: str, repo: str, pr_number: int, header_prefix: str + ) -> Tuple[int, int]: + """Minimize (collapse) old roadmap comments on a PR. + + Finds all issue comments that start with the given header prefix + and minimizes them using GitHub's GraphQL API. This keeps the PR + conversation clean by collapsing outdated roadmaps. + + Args: + owner: Repository owner. + repo: Repository name. + pr_number: Pull request number. + header_prefix: The prefix to match against comment bodies + (e.g., "🗺️ **Auto-Generated Review Roadmap**"). + + Returns: + Tuple of (minimized_count, error_count). + """ + # Fetch all issue comments (includes node_id needed for GraphQL) + resp = self.client.get(f"/repos/{owner}/{repo}/issues/{pr_number}/comments") + if resp.status_code != 200: + return (0, 0) + + comments = resp.json() + + # Filter for roadmap comments by body prefix + roadmap_node_ids = [ + c["node_id"] + for c in comments + if c.get("body", "").startswith(header_prefix) + ] + + if not roadmap_node_ids: + return (0, 0) + + # Minimize each comment using GraphQL + minimized = 0 + errors = 0 + mutation = """ + mutation($id: ID!) { + minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) { + minimizedComment { isMinimized } + } + } + """ + + for node_id in roadmap_node_ids: + try: + gql_resp = self.client.post( + "https://api.github.com/graphql", + json={"query": mutation, "variables": {"id": node_id}}, + ) + if gql_resp.status_code == 200: + data = gql_resp.json() + if data.get("data", {}).get("minimizeComment", {}).get( + "minimizedComment", {} + ).get("isMinimized"): + minimized += 1 + elif "errors" in data: + errors += 1 + else: + # Response OK but isMinimized not true - count as success + # (might already be minimized) + minimized += 1 + else: + errors += 1 + except Exception: + errors += 1 + + return (minimized, errors) + def post_pr_comment(self, owner: str, repo: str, pr_number: int, body: str) -> Dict[str, Any]: """Post a comment on a pull request. diff --git a/review_roadmap/main.py b/review_roadmap/main.py index 4b97be8..373420c 100644 --- a/review_roadmap/main.py +++ b/review_roadmap/main.py @@ -10,6 +10,9 @@ app = typer.Typer(add_completion=False) console = Console() +# Header prefix used to identify roadmap comments for collapse feature +ROADMAP_HEADER_PREFIX = "🗺️ **Auto-Generated Review Roadmap**" + # Initialize structured logging configure_logging( log_level=settings.REVIEW_ROADMAP_LOG_LEVEL, @@ -22,7 +25,7 @@ def format_pr_comment(roadmap_content: str) -> str: provider = settings.REVIEW_ROADMAP_LLM_PROVIDER model = settings.REVIEW_ROADMAP_MODEL_NAME - header = f"""🗺️ **Auto-Generated Review Roadmap** + header = f"""{ROADMAP_HEADER_PREFIX} > This roadmap was automatically generated by [review-roadmap](https://github.com/ambient-code/review-roadmap). > Model: `{provider}/{model}` @@ -114,6 +117,20 @@ def generate( if post: console.print("[bold blue]Posting roadmap to PR...[/bold blue]") + + # First, minimize any old roadmap comments (non-fatal if this fails) + try: + minimized, errors = gh_client.minimize_old_roadmap_comments( + owner, repo, pr_number, ROADMAP_HEADER_PREFIX + ) + if minimized > 0: + console.print(f"[dim]Collapsed {minimized} previous roadmap(s)[/dim]") + if errors > 0: + console.print(f"[yellow]Warning: Failed to collapse {errors} comment(s)[/yellow]") + except Exception as e: + console.print(f"[yellow]Warning: Could not collapse old comments: {e}[/yellow]") + + # Then post the new comment try: comment_body = format_pr_comment(roadmap_content) gh_client.post_pr_comment(owner, repo, pr_number, comment_body) diff --git a/tests/test_github_client.py b/tests/test_github_client.py index cd76fee..b6c1c78 100644 --- a/tests/test_github_client.py +++ b/tests/test_github_client.py @@ -436,3 +436,190 @@ def test_post_pr_comment_success(): assert result["id"] == 123456 assert result["body"] == comment_body + + +# --- Tests for minimize_old_roadmap_comments --- + +ROADMAP_PREFIX = "🗺️ **Auto-Generated Review Roadmap**" + + +@respx.mock +def test_minimize_old_roadmap_comments_no_comments(): + """Returns (0, 0) when there are no comments on the PR.""" + owner = "owner" + repo = "repo" + pr_number = 42 + + url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments" + respx.get(url).mock(return_value=Response(200, json=[])) + + client = GitHubClient(token="fake-token") + minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX) + + assert minimized == 0 + assert errors == 0 + + +@respx.mock +def test_minimize_old_roadmap_comments_no_matching_comments(): + """Returns (0, 0) when no comments match the roadmap prefix.""" + owner = "owner" + repo = "repo" + pr_number = 42 + + url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments" + respx.get(url).mock(return_value=Response(200, json=[ + { + "id": 1, + "node_id": "IC_node1", + "body": "This is a regular comment", + "user": {"login": "user1"}, + "created_at": "2024-01-01T00:00:00Z" + }, + { + "id": 2, + "node_id": "IC_node2", + "body": "Another comment mentioning roadmap but not starting with prefix", + "user": {"login": "user2"}, + "created_at": "2024-01-02T00:00:00Z" + } + ])) + + client = GitHubClient(token="fake-token") + minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX) + + assert minimized == 0 + assert errors == 0 + + +@respx.mock +def test_minimize_old_roadmap_comments_success(): + """Successfully minimizes roadmap comments and ignores non-roadmap comments.""" + owner = "owner" + repo = "repo" + pr_number = 42 + + comments_url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments" + graphql_url = "https://api.github.com/graphql" + + # Mix of roadmap and non-roadmap comments + respx.get(comments_url).mock(return_value=Response(200, json=[ + { + "id": 1, + "node_id": "IC_roadmap1", + "body": f"{ROADMAP_PREFIX}\n\nOld roadmap content here", + "user": {"login": "github-actions[bot]"}, + "created_at": "2024-01-01T00:00:00Z" + }, + { + "id": 2, + "node_id": "IC_regular", + "body": "Regular comment - should not be minimized", + "user": {"login": "reviewer"}, + "created_at": "2024-01-02T00:00:00Z" + }, + { + "id": 3, + "node_id": "IC_roadmap2", + "body": f"{ROADMAP_PREFIX}\n\nAnother old roadmap", + "user": {"login": "user1"}, + "created_at": "2024-01-03T00:00:00Z" + } + ])) + + # Mock GraphQL responses for both roadmap comments + respx.post(graphql_url).mock(return_value=Response(200, json={ + "data": { + "minimizeComment": { + "minimizedComment": {"isMinimized": True} + } + } + })) + + client = GitHubClient(token="fake-token") + minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX) + + # Should minimize 2 roadmap comments, not the regular one + assert minimized == 2 + assert errors == 0 + + +@respx.mock +def test_minimize_old_roadmap_comments_graphql_error(): + """Handles GraphQL errors gracefully and counts them.""" + owner = "owner" + repo = "repo" + pr_number = 42 + + comments_url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments" + graphql_url = "https://api.github.com/graphql" + + respx.get(comments_url).mock(return_value=Response(200, json=[ + { + "id": 1, + "node_id": "IC_roadmap1", + "body": f"{ROADMAP_PREFIX}\n\nOld roadmap", + "user": {"login": "user1"}, + "created_at": "2024-01-01T00:00:00Z" + } + ])) + + # Mock GraphQL error response + respx.post(graphql_url).mock(return_value=Response(200, json={ + "errors": [{"message": "Could not resolve to a node with the global id"}] + })) + + client = GitHubClient(token="fake-token") + minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX) + + assert minimized == 0 + assert errors == 1 + + +@respx.mock +def test_minimize_old_roadmap_comments_http_error(): + """Handles HTTP errors on GraphQL endpoint gracefully.""" + owner = "owner" + repo = "repo" + pr_number = 42 + + comments_url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments" + graphql_url = "https://api.github.com/graphql" + + respx.get(comments_url).mock(return_value=Response(200, json=[ + { + "id": 1, + "node_id": "IC_roadmap1", + "body": f"{ROADMAP_PREFIX}\n\nOld roadmap", + "user": {"login": "user1"}, + "created_at": "2024-01-01T00:00:00Z" + } + ])) + + # Mock HTTP 403 error on GraphQL + respx.post(graphql_url).mock(return_value=Response(403, json={ + "message": "Forbidden" + })) + + client = GitHubClient(token="fake-token") + minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX) + + assert minimized == 0 + assert errors == 1 + + +@respx.mock +def test_minimize_old_roadmap_comments_fetch_error(): + """Returns (0, 0) when fetching comments fails.""" + owner = "owner" + repo = "repo" + pr_number = 42 + + comments_url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments" + respx.get(comments_url).mock(return_value=Response(404, json={"message": "Not Found"})) + + client = GitHubClient(token="fake-token") + minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX) + + assert minimized == 0 + assert errors == 0