Skip to content

fix(pr-review): refresh stale review loop state#5833

Merged
matthewevans merged 1 commit into
mainfrom
ship/fixpr-review-refresh-stale-review-loop-state
Jul 14, 2026
Merged

fix(pr-review): refresh stale review loop state#5833
matthewevans merged 1 commit into
mainfrom
ship/fixpr-review-refresh-stale-review-loop-state

Conversation

@matthewevans

Copy link
Copy Markdown
Member

No description provided.

@matthewevans
matthewevans enabled auto-merge July 14, 2026 18:50

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the PR review automation tool and its documentation. It introduces a 'freshness invariant' to track head changes, incomplete comment histories, and author follow-ups after maintainer activity, ensuring stale states are not incorrectly preserved. It also refines CI status checks by fetching branch-protection rules to distinguish between required and advisory checks. Additionally, the frontend component skill guide is updated with a new section on component testing. The review feedback highlights a critical issue across multiple functions (author_activity_after, has_author_activity, and latest_maintainer_activity_timestamp) where case-sensitive comparisons of GitHub logins are used; since GitHub logins are case-insensitive, these should be normalized using fold_login to prevent missing relevant activities.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread scripts/pr_review.py
Comment on lines 2408 to 2418
def author_activity_after(pr: dict[str, Any], timestamp: str | None) -> bool:
author_login = pr.get("author_login")
return any(
comment_login(comment) == author_login
and timestamp_after(comment.get("createdAt"), timestamp)
and timestamp_after(activity_timestamp(comment, "createdAt", "updatedAt"), timestamp)
for comment in pr.get("comments", [])
) or any(
comment_login(review) == author_login
and timestamp_after(review.get("submittedAt"), timestamp)
and timestamp_after(activity_timestamp(review, "submittedAt"), timestamp)
for review in pr.get("reviews", [])
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

[HIGH] Case-sensitive GitHub login comparison in author_activity_after. Evidence: scripts/pr_review.py:2408. Why it matters: GitHub logins are case-insensitive, and case-sensitive comparisons can cause the review loop to miss author follow-ups if the casing differs between API endpoints. Suggested fix: Use fold_login to normalize logins before comparison.

Suggested change
def author_activity_after(pr: dict[str, Any], timestamp: str | None) -> bool:
author_login = pr.get("author_login")
return any(
comment_login(comment) == author_login
and timestamp_after(comment.get("createdAt"), timestamp)
and timestamp_after(activity_timestamp(comment, "createdAt", "updatedAt"), timestamp)
for comment in pr.get("comments", [])
) or any(
comment_login(review) == author_login
and timestamp_after(review.get("submittedAt"), timestamp)
and timestamp_after(activity_timestamp(review, "submittedAt"), timestamp)
for review in pr.get("reviews", [])
)
def author_activity_after(pr: dict[str, Any], timestamp: str | None) -> bool:
author_login = pr.get('author_login')
if not author_login:
return False
author_login_folded = fold_login(author_login)
return any(
comment_login(comment) is not None
and fold_login(comment_login(comment)) == author_login_folded
and timestamp_after(activity_timestamp(comment, 'createdAt', 'updatedAt'), timestamp)
for comment in pr.get('comments', [])
) or any(
comment_login(review) is not None
and fold_login(comment_login(review)) == author_login_folded
and timestamp_after(activity_timestamp(review, 'submittedAt'), timestamp)
for review in pr.get('reviews', [])
)

Comment thread scripts/pr_review.py
Comment on lines +2421 to +2428
def has_author_activity(pr: dict[str, Any]) -> bool:
"""Whether the contributor has left a comment or review on this PR."""
author_login = pr.get("author_login")
return any(
comment_login(comment) == author_login for comment in pr.get("comments", [])
) or any(
comment_login(review) == author_login for review in pr.get("reviews", [])
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

[HIGH] Case-sensitive GitHub login comparison in has_author_activity. Evidence: scripts/pr_review.py:2421. Why it matters: GitHub logins are case-insensitive, and case-sensitive comparisons can cause the review loop to miss author activity if the casing differs between API endpoints. Suggested fix: Use fold_login to normalize logins before comparison.

def has_author_activity(pr: dict[str, Any]) -> bool:
    '''Whether the contributor has left a comment or review on this PR.'''
    author_login = pr.get('author_login')
    if not author_login:
        return False
    author_login_folded = fold_login(author_login)
    return any(
        comment_login(comment) is not None
        and fold_login(comment_login(comment)) == author_login_folded for comment in pr.get('comments', [])
    ) or any(
        comment_login(review) is not None
        and fold_login(comment_login(review)) == author_login_folded for review in pr.get('reviews', [])
    )

Comment thread scripts/pr_review.py
Comment on lines +2431 to +2454
def latest_maintainer_activity_timestamp(pr: dict[str, Any], acting_login: str | None) -> str | None:
"""Find the latest GitHub-visible response from the acting maintainer.

Local event-log rows record what the loop observed or decided; they are not a
response to a contributor. Follow-up freshness must therefore anchor only on
a real GitHub review/comment from the maintainer.
"""
if not acting_login:
return None
timestamps = [
activity_timestamp(comment, "createdAt", "updatedAt")
for comment in pr.get("comments", [])
if comment_login(comment) == acting_login
] + [
activity_timestamp(review, "submittedAt")
for review in pr.get("reviews", [])
if comment_login(review) == acting_login
]
timestamps = [timestamp for timestamp in timestamps if timestamp]
return max(
timestamps,
key=lambda timestamp: parse_event_datetime(timestamp) or datetime.min.replace(tzinfo=UTC),
default=None,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

[HIGH] Case-sensitive GitHub login comparison in latest_maintainer_activity_timestamp. Evidence: scripts/pr_review.py:2431. Why it matters: GitHub logins are case-insensitive, and case-sensitive comparisons can cause the review loop to miss maintainer activity if the casing differs between API endpoints. Suggested fix: Use fold_login to normalize logins before comparison.

def latest_maintainer_activity_timestamp(pr: dict[str, Any], acting_login: str | None) -> str | None:
    '''Find the latest GitHub-visible response from the acting maintainer.

    Local event-log rows record what the loop observed or decided; they are not a
    response to a contributor. Follow-up freshness must therefore anchor only on
    a real GitHub review/comment from the maintainer.
    '''
    if not acting_login:
        return None
    acting_login_folded = fold_login(acting_login)
    timestamps = [
        activity_timestamp(comment, 'createdAt', 'updatedAt')
        for comment in pr.get('comments', [])
        if comment_login(comment) is not None
        and fold_login(comment_login(comment)) == acting_login_folded
    ] + [
        activity_timestamp(review, 'submittedAt')
        for review in pr.get('reviews', [])
        if comment_login(review) is not None
        and fold_login(comment_login(review)) == acting_login_folded
    ]
    timestamps = [timestamp for timestamp in timestamps if timestamp]
    return max(
        timestamps,
        key=lambda timestamp: parse_event_datetime(timestamp) or datetime.min.replace(tzinfo=UTC),
        default=None,
    )

@matthewevans
matthewevans added this pull request to the merge queue Jul 14, 2026
Merged via the queue into main with commit f68a5ec Jul 14, 2026
13 checks passed
@matthewevans
matthewevans deleted the ship/fixpr-review-refresh-stale-review-loop-state branch July 14, 2026 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant