Skip to content

Commit 0678049

Browse files
feat: Enhance PR comment script with context and filters
This commit significantly enhances the `scripts/gha/get_pr_review_comments.py` script and its underlying library function in `scripts/gha/firebase_github.py`. Key improvements include: - Copyright year updated to 2025. - Output now includes comment ID, in_reply_to_id (if applicable), and the comment creation timestamp. - Comments are marked as [OUTDATED] if their diff position is no longer current (i.e., API 'position' field is null). - Added a `--context-lines <N>` argument (default 10) to control the amount of diff hunk context displayed. N=0 shows the full hunk. - Introduced a `--since <ISO_8601_timestamp>` argument to filter comments, showing only those created at or after the specified time. The `get_pull_request_review_comments` function in the library was updated to support this `since` parameter in the API call. These changes provide more comprehensive comment information and allow for better control over the data fetched, making it more useful for reviewing and addressing PR feedback, especially in complex PRs with multiple review rounds.
1 parent da2efa9 commit 0678049

File tree

2 files changed

+116
-50
lines changed

2 files changed

+116
-50
lines changed

scripts/gha/firebase_github.py

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -225,33 +225,45 @@ def get_reviews(token, pull_number):
225225
return results
226226

227227

228-
def get_pull_request_review_comments(token, pull_number):
228+
def get_pull_request_review_comments(token, pull_number, since=None): # Added since=None
229229
"""https://docs.github.com/en/rest/pulls/comments#list-review-comments-on-a-pull-request"""
230230
url = f'{GITHUB_API_URL}/pulls/{pull_number}/comments'
231231
headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
232+
232233
page = 1
233234
per_page = 100
234235
results = []
235-
keep_going = True
236-
while keep_going:
237-
params = {'per_page': per_page, 'page': page}
238-
page = page + 1
239-
keep_going = False
240-
# Use a try-except block to catch potential errors during the API request
236+
237+
# Base parameters for the API request
238+
base_params = {'per_page': per_page}
239+
if since:
240+
base_params['since'] = since
241+
242+
while True: # Loop indefinitely until explicitly broken
243+
current_page_params = base_params.copy()
244+
current_page_params['page'] = page
245+
241246
try:
242-
with requests_retry_session().get(url, headers=headers, params=params,
247+
with requests_retry_session().get(url, headers=headers, params=current_page_params,
243248
stream=True, timeout=TIMEOUT) as response:
244-
response.raise_for_status() # Raise an exception for bad status codes (4xx or 5xx)
245-
logging.info("get_pull_request_review_comments: %s page %s response: %s", url, params.get('page'), response)
249+
response.raise_for_status()
250+
# Log which page and if 'since' was used for clarity
251+
logging.info("get_pull_request_review_comments: %s params %s response: %s", url, current_page_params, response)
252+
246253
current_page_results = response.json()
247-
if not current_page_results: # No more results
248-
break
254+
if not current_page_results: # No more results on this page
255+
break # Exit loop, no more comments to fetch
256+
249257
results.extend(current_page_results)
250-
# If exactly per_page results were retrieved, there might be more.
251-
keep_going = (len(current_page_results) == per_page)
258+
259+
# If fewer results than per_page were returned, it's the last page
260+
if len(current_page_results) < per_page:
261+
break # Exit loop, this was the last page
262+
263+
page += 1 # Increment page for the next iteration
264+
252265
except requests.exceptions.RequestException as e:
253-
logging.error(f"Error fetching review comments page {params.get('page')-1} for PR {pull_number}: {e}")
254-
# Optionally, re-raise the exception or handle it by returning partial results or an empty list
266+
logging.error(f"Error fetching review comments (page {page}, params: {current_page_params}) for PR {pull_number}: {e}")
255267
break # Stop trying if there's an error
256268
return results
257269

scripts/gha/get_pr_review_comments.py

Lines changed: 88 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/usr/bin/env python3
2-
# Copyright 2024 Google LLC
2+
# Copyright 2025 Google LLC
33
#
44
# Licensed under the Apache License, Version 2.0 (the "License");
55
# you may not use this file except in compliance with the License.
@@ -26,20 +26,47 @@
2626
# Set verbosity for absl logging if you want to see logs from firebase_github
2727
# absl_logging.set_verbosity(absl_logging.INFO)
2828
except ImportError:
29-
# If absl is not used, standard logging can be configured if needed
30-
# import logging as std_logging
31-
# std_logging.basicConfig(level=std_logging.INFO)
3229
pass # firebase_github.py uses absl.logging.info, so this won't redirect.
3330

3431

32+
def print_contextual_diff_hunk(diff_hunk, comment_position, context_lines_count):
33+
if not diff_hunk or not diff_hunk.strip(): # Handle empty or whitespace-only diff_hunk
34+
print("(No diff hunk available or content is empty)")
35+
return
36+
37+
hunk_lines = diff_hunk.split('\n')
38+
39+
# comment_position is 1-indexed from GitHub API. If None, or context is 0, print full hunk.
40+
if context_lines_count == 0 or comment_position is None or comment_position < 1 or comment_position > len(hunk_lines):
41+
print(diff_hunk)
42+
return
43+
44+
comment_line_index = comment_position - 1 # Convert to 0-indexed for list access
45+
46+
start_index = max(0, comment_line_index - context_lines_count)
47+
end_index = min(len(hunk_lines), comment_line_index + context_lines_count + 1)
48+
49+
# Ensure start_index is not greater than comment_line_index, in case of small hunks
50+
# This also means that if comment_line_index is valid, start_index will be <= comment_line_index
51+
start_index = min(start_index, comment_line_index if comment_line_index >=0 else 0)
52+
53+
54+
for i in range(start_index, end_index):
55+
# Basic safety for i, though start/end logic should make this robust
56+
if i >= 0 and i < len(hunk_lines):
57+
prefix = "> " if i == comment_line_index else " "
58+
print(f"{prefix}{hunk_lines[i]}")
59+
# else: # This case should ideally not be reached with correct boundary conditions
60+
# print(f" Error: Skipped line index {i} in hunk processing due to boundary issue.")
61+
62+
3563
def main():
36-
# Default owner and repo from firebase_github, ensuring it's loaded.
3764
default_owner = firebase_github.OWNER
3865
default_repo = firebase_github.REPO
3966

4067
parser = argparse.ArgumentParser(
4168
description="Fetch review comments from a GitHub PR and format for use with Jules.",
42-
formatter_class=argparse.RawTextHelpFormatter # To preserve formatting in help text
69+
formatter_class=argparse.RawTextHelpFormatter
4370
)
4471
parser.add_argument(
4572
"--pull_number",
@@ -51,73 +78,100 @@ def main():
5178
"--owner",
5279
type=str,
5380
default=default_owner,
54-
help=f"Repository owner. Defaults to '{default_owner}' (from firebase_github.py)."
81+
help=f"Repository owner. Defaults to '{default_owner}'."
5582
)
5683
parser.add_argument(
5784
"--repo",
5885
type=str,
5986
default=default_repo,
60-
help=f"Repository name. Defaults to '{default_repo}' (from firebase_github.py)."
87+
help=f"Repository name. Defaults to '{default_repo}'."
6188
)
6289
parser.add_argument(
6390
"--token",
6491
type=str,
6592
default=os.environ.get("GITHUB_TOKEN"),
66-
help="GitHub token. Can also be set via GITHUB_TOKEN environment variable."
93+
help="GitHub token. Can also be set via GITHUB_TOKEN env var."
94+
)
95+
parser.add_argument(
96+
"--context-lines",
97+
type=int,
98+
default=10,
99+
help="Number of context lines around the commented line from the diff hunk. Use 0 for the full hunk. Default: 10."
100+
)
101+
parser.add_argument(
102+
"--since",
103+
type=str,
104+
default=None,
105+
help="Only show comments created at or after this ISO 8601 timestamp (e.g., YYYY-MM-DDTHH:MM:SSZ)."
67106
)
68107

69108
args = parser.parse_args()
70109

71110
if not args.token:
72-
sys.stderr.write("Error: GitHub token not provided. Set GITHUB_TOKEN environment variable or use --token argument.\n")
111+
sys.stderr.write("Error: GitHub token not provided. Set GITHUB_TOKEN or use --token.\n")
73112
sys.exit(1)
74113

75-
# Update the repository details in firebase_github module if different from default
76114
if args.owner != firebase_github.OWNER or args.repo != firebase_github.REPO:
77115
repo_url = f"https://github.com/{args.owner}/{args.repo}"
78116
if not firebase_github.set_repo_url(repo_url):
79-
sys.stderr.write(f"Error: Invalid repository URL format for {args.owner}/{args.repo}. Expected format: https://github.com/owner/repo\n")
117+
sys.stderr.write(f"Error: Invalid repo URL: {args.owner}/{args.repo}. Expected https://github.com/owner/repo\n")
80118
sys.exit(1)
81-
# Using print to stderr for info, as absl logging might not be configured here for this script's own messages.
82119
print(f"Targeting repository: {firebase_github.OWNER}/{firebase_github.REPO}", file=sys.stderr)
83120

121+
print(f"Fetching comments for PR #{args.pull_number} from {firebase_github.OWNER}/{firebase_github.REPO}...", file=sys.stderr)
122+
if args.since:
123+
print(f"Filtering comments created since: {args.since}", file=sys.stderr)
84124

85-
print(f"Fetching review comments for PR #{args.pull_number} from {firebase_github.OWNER}/{firebase_github.REPO}...", file=sys.stderr)
86-
87-
comments = firebase_github.get_pull_request_review_comments(args.token, args.pull_number)
125+
comments = firebase_github.get_pull_request_review_comments(
126+
args.token,
127+
args.pull_number,
128+
since=args.since # Pass the 'since' argument
129+
)
88130

89-
if not comments: # This will be true if list is empty (no comments or error in fetching first page)
90-
print(f"No review comments found for PR #{args.pull_number}, or an error occurred during fetching.", file=sys.stderr)
91-
# If firebase_github.py's get_pull_request_review_comments logs errors, those might provide more details.
92-
return # Exit gracefully if no comments
131+
if not comments:
132+
print(f"No review comments found for PR #{args.pull_number} (or matching filters), or an error occurred.", file=sys.stderr)
133+
return
93134

94-
# Output actual data to stdout
95135
print("\n--- Review Comments ---")
96136
for comment in comments:
97137
user = comment.get("user", {}).get("login", "Unknown user")
98138
path = comment.get("path", "N/A")
99-
line = comment.get("line", "N/A")
100-
body = comment.get("body", "").strip() # Strip whitespace from comment body
101-
diff_hunk = comment.get("diff_hunk", "N/A")
139+
file_line = comment.get("line", "N/A")
140+
hunk_position = comment.get("position") # This is the 1-indexed position in the hunk
141+
142+
body = comment.get("body", "").strip()
143+
diff_hunk = comment.get("diff_hunk") # Can be None or empty
102144
html_url = comment.get("html_url", "N/A")
103145

104-
# Only print comments that have a body
146+
comment_id = comment.get("id")
147+
in_reply_to_id = comment.get("in_reply_to_id")
148+
created_at = comment.get("created_at")
149+
105150
if not body:
106151
continue
107152

108-
print(f"Comment by: {user}")
153+
print(f"Comment by: {user} (ID: {comment_id}){f' (In Reply To: {in_reply_to_id})' if in_reply_to_id else ''}")
154+
if created_at:
155+
print(f"Timestamp: {created_at}")
156+
157+
if diff_hunk: # Only show status if it's a diff-related comment
158+
status_text = "[OUTDATED]" if hunk_position is None else "[CURRENT]"
159+
print(f"Status: {status_text}")
160+
109161
print(f"File: {path}")
110-
# The 'line' field in GitHub's API for PR review comments refers to the line number in the diff.
111-
# 'original_line' refers to the line number in the file at the time the comment was made.
112-
# 'start_line' and 'original_start_line' for multi-line comments.
113-
# For simplicity, we use 'line'.
114-
print(f"Line in diff: {line}")
162+
print(f"Line in File Diff: {file_line}")
115163
print(f"URL: {html_url}")
116-
print("--- Diff Hunk ---")
117-
print(diff_hunk)
164+
165+
print("--- Diff Hunk Context ---")
166+
if diff_hunk:
167+
print_contextual_diff_hunk(diff_hunk, hunk_position, args.context_lines)
168+
else:
169+
print("(Comment not associated with a specific diff hunk)")
170+
118171
print("--- Comment ---")
119172
print(body)
120-
print("----------------------------------------\n")
173+
print("----------------------------------------\n") # Use
174+
for newline
121175

122176
if __name__ == "__main__":
123177
main()

0 commit comments

Comments
 (0)