Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 73 additions & 1 deletion review_roadmap/github/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
generate review roadmaps.
"""

from typing import Any, Dict, List, Optional
from typing import Any, Dict, List, Optional, Tuple

import httpx

Expand Down Expand Up @@ -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.

Expand Down
19 changes: 18 additions & 1 deletion review_roadmap/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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}`
Expand Down Expand Up @@ -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)
Expand Down
187 changes: 187 additions & 0 deletions tests/test_github_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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