Skip to content

Commit 9ec1977

Browse files
committed
feat: auto-collapse old roadmap comments when posting new ones
Implements GitHub issue #5. When using --post, previous roadmap comments are automatically minimized (collapsed) using GitHub's GraphQL API before posting the new roadmap. This keeps PR conversations clean. Changes: - Add minimize_old_roadmap_comments() to GitHubClient - Call minimize before posting in main.py - Extract ROADMAP_HEADER_PREFIX constant for comment identification - Add 6 tests for the new functionality Behavior: - All comments starting with the roadmap header are collapsed - Comments from any user are collapsed (not just bots) - Minimize failures are non-blocking (warnings shown, posting continues) - Uses OUTDATED classifier for collapsed comments
1 parent 8ccc0a1 commit 9ec1977

File tree

3 files changed

+278
-2
lines changed

3 files changed

+278
-2
lines changed

review_roadmap/github/client.py

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
generate review roadmaps.
66
"""
77

8-
from typing import Any, Dict, List, Optional
8+
from typing import Any, Dict, List, Optional, Tuple
99

1010
import httpx
1111

@@ -355,6 +355,78 @@ def check_write_access(
355355
)
356356
)
357357

358+
def minimize_old_roadmap_comments(
359+
self, owner: str, repo: str, pr_number: int, header_prefix: str
360+
) -> Tuple[int, int]:
361+
"""Minimize (collapse) old roadmap comments on a PR.
362+
363+
Finds all issue comments that start with the given header prefix
364+
and minimizes them using GitHub's GraphQL API. This keeps the PR
365+
conversation clean by collapsing outdated roadmaps.
366+
367+
Args:
368+
owner: Repository owner.
369+
repo: Repository name.
370+
pr_number: Pull request number.
371+
header_prefix: The prefix to match against comment bodies
372+
(e.g., "🗺️ **Auto-Generated Review Roadmap**").
373+
374+
Returns:
375+
Tuple of (minimized_count, error_count).
376+
"""
377+
# Fetch all issue comments (includes node_id needed for GraphQL)
378+
resp = self.client.get(f"/repos/{owner}/{repo}/issues/{pr_number}/comments")
379+
if resp.status_code != 200:
380+
return (0, 0)
381+
382+
comments = resp.json()
383+
384+
# Filter for roadmap comments by body prefix
385+
roadmap_node_ids = [
386+
c["node_id"]
387+
for c in comments
388+
if c.get("body", "").startswith(header_prefix)
389+
]
390+
391+
if not roadmap_node_ids:
392+
return (0, 0)
393+
394+
# Minimize each comment using GraphQL
395+
minimized = 0
396+
errors = 0
397+
mutation = """
398+
mutation($id: ID!) {
399+
minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) {
400+
minimizedComment { isMinimized }
401+
}
402+
}
403+
"""
404+
405+
for node_id in roadmap_node_ids:
406+
try:
407+
gql_resp = self.client.post(
408+
"https://api.github.com/graphql",
409+
json={"query": mutation, "variables": {"id": node_id}},
410+
)
411+
if gql_resp.status_code == 200:
412+
data = gql_resp.json()
413+
if data.get("data", {}).get("minimizeComment", {}).get(
414+
"minimizedComment", {}
415+
).get("isMinimized"):
416+
minimized += 1
417+
elif "errors" in data:
418+
errors += 1
419+
else:
420+
# Response OK but isMinimized not true - count as success
421+
# (might already be minimized)
422+
minimized += 1
423+
else:
424+
errors += 1
425+
except Exception:
426+
errors += 1
427+
428+
return (minimized, errors)
429+
358430
def post_pr_comment(self, owner: str, repo: str, pr_number: int, body: str) -> Dict[str, Any]:
359431
"""Post a comment on a pull request.
360432

review_roadmap/main.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
app = typer.Typer(add_completion=False)
1111
console = Console()
1212

13+
# Header prefix used to identify roadmap comments for collapse feature
14+
ROADMAP_HEADER_PREFIX = "🗺️ **Auto-Generated Review Roadmap**"
15+
1316
# Initialize structured logging
1417
configure_logging(
1518
log_level=settings.REVIEW_ROADMAP_LOG_LEVEL,
@@ -22,7 +25,7 @@ def format_pr_comment(roadmap_content: str) -> str:
2225
provider = settings.REVIEW_ROADMAP_LLM_PROVIDER
2326
model = settings.REVIEW_ROADMAP_MODEL_NAME
2427

25-
header = f"""🗺️ **Auto-Generated Review Roadmap**
28+
header = f"""{ROADMAP_HEADER_PREFIX}
2629
2730
> This roadmap was automatically generated by [review-roadmap](https://github.com/ambient-code/review-roadmap).
2831
> Model: `{provider}/{model}`
@@ -114,6 +117,20 @@ def generate(
114117

115118
if post:
116119
console.print("[bold blue]Posting roadmap to PR...[/bold blue]")
120+
121+
# First, minimize any old roadmap comments (non-fatal if this fails)
122+
try:
123+
minimized, errors = gh_client.minimize_old_roadmap_comments(
124+
owner, repo, pr_number, ROADMAP_HEADER_PREFIX
125+
)
126+
if minimized > 0:
127+
console.print(f"[dim]Collapsed {minimized} previous roadmap(s)[/dim]")
128+
if errors > 0:
129+
console.print(f"[yellow]Warning: Failed to collapse {errors} comment(s)[/yellow]")
130+
except Exception as e:
131+
console.print(f"[yellow]Warning: Could not collapse old comments: {e}[/yellow]")
132+
133+
# Then post the new comment
117134
try:
118135
comment_body = format_pr_comment(roadmap_content)
119136
gh_client.post_pr_comment(owner, repo, pr_number, comment_body)

tests/test_github_client.py

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,3 +436,190 @@ def test_post_pr_comment_success():
436436

437437
assert result["id"] == 123456
438438
assert result["body"] == comment_body
439+
440+
441+
# --- Tests for minimize_old_roadmap_comments ---
442+
443+
ROADMAP_PREFIX = "🗺️ **Auto-Generated Review Roadmap**"
444+
445+
446+
@respx.mock
447+
def test_minimize_old_roadmap_comments_no_comments():
448+
"""Returns (0, 0) when there are no comments on the PR."""
449+
owner = "owner"
450+
repo = "repo"
451+
pr_number = 42
452+
453+
url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments"
454+
respx.get(url).mock(return_value=Response(200, json=[]))
455+
456+
client = GitHubClient(token="fake-token")
457+
minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX)
458+
459+
assert minimized == 0
460+
assert errors == 0
461+
462+
463+
@respx.mock
464+
def test_minimize_old_roadmap_comments_no_matching_comments():
465+
"""Returns (0, 0) when no comments match the roadmap prefix."""
466+
owner = "owner"
467+
repo = "repo"
468+
pr_number = 42
469+
470+
url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments"
471+
respx.get(url).mock(return_value=Response(200, json=[
472+
{
473+
"id": 1,
474+
"node_id": "IC_node1",
475+
"body": "This is a regular comment",
476+
"user": {"login": "user1"},
477+
"created_at": "2024-01-01T00:00:00Z"
478+
},
479+
{
480+
"id": 2,
481+
"node_id": "IC_node2",
482+
"body": "Another comment mentioning roadmap but not starting with prefix",
483+
"user": {"login": "user2"},
484+
"created_at": "2024-01-02T00:00:00Z"
485+
}
486+
]))
487+
488+
client = GitHubClient(token="fake-token")
489+
minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX)
490+
491+
assert minimized == 0
492+
assert errors == 0
493+
494+
495+
@respx.mock
496+
def test_minimize_old_roadmap_comments_success():
497+
"""Successfully minimizes roadmap comments and ignores non-roadmap comments."""
498+
owner = "owner"
499+
repo = "repo"
500+
pr_number = 42
501+
502+
comments_url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments"
503+
graphql_url = "https://api.github.com/graphql"
504+
505+
# Mix of roadmap and non-roadmap comments
506+
respx.get(comments_url).mock(return_value=Response(200, json=[
507+
{
508+
"id": 1,
509+
"node_id": "IC_roadmap1",
510+
"body": f"{ROADMAP_PREFIX}\n\nOld roadmap content here",
511+
"user": {"login": "github-actions[bot]"},
512+
"created_at": "2024-01-01T00:00:00Z"
513+
},
514+
{
515+
"id": 2,
516+
"node_id": "IC_regular",
517+
"body": "Regular comment - should not be minimized",
518+
"user": {"login": "reviewer"},
519+
"created_at": "2024-01-02T00:00:00Z"
520+
},
521+
{
522+
"id": 3,
523+
"node_id": "IC_roadmap2",
524+
"body": f"{ROADMAP_PREFIX}\n\nAnother old roadmap",
525+
"user": {"login": "user1"},
526+
"created_at": "2024-01-03T00:00:00Z"
527+
}
528+
]))
529+
530+
# Mock GraphQL responses for both roadmap comments
531+
respx.post(graphql_url).mock(return_value=Response(200, json={
532+
"data": {
533+
"minimizeComment": {
534+
"minimizedComment": {"isMinimized": True}
535+
}
536+
}
537+
}))
538+
539+
client = GitHubClient(token="fake-token")
540+
minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX)
541+
542+
# Should minimize 2 roadmap comments, not the regular one
543+
assert minimized == 2
544+
assert errors == 0
545+
546+
547+
@respx.mock
548+
def test_minimize_old_roadmap_comments_graphql_error():
549+
"""Handles GraphQL errors gracefully and counts them."""
550+
owner = "owner"
551+
repo = "repo"
552+
pr_number = 42
553+
554+
comments_url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments"
555+
graphql_url = "https://api.github.com/graphql"
556+
557+
respx.get(comments_url).mock(return_value=Response(200, json=[
558+
{
559+
"id": 1,
560+
"node_id": "IC_roadmap1",
561+
"body": f"{ROADMAP_PREFIX}\n\nOld roadmap",
562+
"user": {"login": "user1"},
563+
"created_at": "2024-01-01T00:00:00Z"
564+
}
565+
]))
566+
567+
# Mock GraphQL error response
568+
respx.post(graphql_url).mock(return_value=Response(200, json={
569+
"errors": [{"message": "Could not resolve to a node with the global id"}]
570+
}))
571+
572+
client = GitHubClient(token="fake-token")
573+
minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX)
574+
575+
assert minimized == 0
576+
assert errors == 1
577+
578+
579+
@respx.mock
580+
def test_minimize_old_roadmap_comments_http_error():
581+
"""Handles HTTP errors on GraphQL endpoint gracefully."""
582+
owner = "owner"
583+
repo = "repo"
584+
pr_number = 42
585+
586+
comments_url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments"
587+
graphql_url = "https://api.github.com/graphql"
588+
589+
respx.get(comments_url).mock(return_value=Response(200, json=[
590+
{
591+
"id": 1,
592+
"node_id": "IC_roadmap1",
593+
"body": f"{ROADMAP_PREFIX}\n\nOld roadmap",
594+
"user": {"login": "user1"},
595+
"created_at": "2024-01-01T00:00:00Z"
596+
}
597+
]))
598+
599+
# Mock HTTP 403 error on GraphQL
600+
respx.post(graphql_url).mock(return_value=Response(403, json={
601+
"message": "Forbidden"
602+
}))
603+
604+
client = GitHubClient(token="fake-token")
605+
minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX)
606+
607+
assert minimized == 0
608+
assert errors == 1
609+
610+
611+
@respx.mock
612+
def test_minimize_old_roadmap_comments_fetch_error():
613+
"""Returns (0, 0) when fetching comments fails."""
614+
owner = "owner"
615+
repo = "repo"
616+
pr_number = 42
617+
618+
comments_url = f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/comments"
619+
respx.get(comments_url).mock(return_value=Response(404, json={"message": "Not Found"}))
620+
621+
client = GitHubClient(token="fake-token")
622+
minimized, errors = client.minimize_old_roadmap_comments(owner, repo, pr_number, ROADMAP_PREFIX)
623+
624+
assert minimized == 0
625+
assert errors == 0

0 commit comments

Comments
 (0)