Skip to content

Suppress status comments for review runs superseded by a newer push #7050

Description

@fullsend-ai-retro

What happened

When the human pushed 5 intermediate commits to PR #7044 between 14:50 and 15:50 UTC, each push triggered a review dispatch via fullsend.yaml. The concurrency group correctly cancelled each previous run when the next push arrived, but each cancelled run's cleanup step called ReconcileOrphaned in internal/statuscomment/statuscomment.go, which updated its "Started" status comment to show "Review cancelled on commit X". This produced 5 separate cancellation comments (timestamps: 14:52, 15:10, 15:29, 15:42, 15:50), comprising ~38% of the PR's comment thread and pushing the actual review findings and human discussion below the fold.

What could go better

When a review run is cancelled because a newer push to the same PR superseded it (concurrency-group cancellation), the "Review cancelled on commit X" status comment provides no actionable information — the developer already knows they pushed a new commit, and the newer run's status comment will report the current state. Five consecutive cancellation comments create significant noise that makes it harder to find the actual review findings and human discussion. Confidence: high — this is a mechanical observation about comment volume, not a judgment about review quality. The concurrency-group cancellation mechanism itself works correctly; the issue is purely about the resulting comment noise.

Proposed change

In internal/statuscomment/statuscomment.go, differentiate between supersession cancellations and hard kills in ReconcileOrphaned. When the cancellation reason is supersession (detectable by checking whether a newer status comment for the same agent already exists on the same PR, or by passing a distinct TerminationReason like ReasonSuperseded from the cleanup step), either (a) delete the orphaned status comment entirely instead of updating it to "Cancelled", or (b) collapse it into a minimal single-line format. The current multi-line "Cancelled" format should be preserved for hard kills (ReasonCancelled) where the user may not know what happened. The cleanup step in the workflow would need to detect whether the cancellation was due to concurrency (GitHub sets github.event.action and the job's cancellation context) and pass the appropriate reason.

Validation criteria

On a PR with 3+ rapid pushes that each trigger a review dispatch, the comment thread should contain at most 1 status comment per superseded batch (e.g., a consolidated note or no comment at all) instead of N separate "Review cancelled" comments. Measure on the next 5 PRs that experience 2+ consecutive review cancellations: the ratio of cancellation comments to total comments should drop below 10%, compared to ~38% on this PR.


Generated by retro agent from #7044

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent/reviewReview agentcomponent/runnerAgent runner behavior and lifecyclefeatureFeature-category issue awaiting human prioritizationpriority/lowNice to have, address when convenientready-for-triageTriggers triage agent dispatchtriagedTriaged but awaiting human prioritizationtype/enhancement

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions