Thoughts on replacing pr-management-triage reviewer pings with author confirmation #171
Closed
choo121600
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Summary
Row 14 of
pr-management-triage's decision table (actionmark-ready-with-ping) promotes a PR toready for maintainer reviewand posts a comment that@-mentions the original reviewer(s), based onthe
unresolved_threads_only_likely_addressedheuristic. Although the triaging maintainer confirmsevery action, the resulting reviewer mention pushes a notification to other maintainers whose review
threads are guessed — not verified — to be addressed. I think this is doing more push-notification
work than the natural label-driven queue requires, and shifts the cost of heuristic false-positives
onto the wrong people.
I'd like to propose splitting Row 14 into a two-step, author-confirmation-gated flow.
Current behaviour (for reference)
.claude/skills/pr-management-triage/classify-and-act.md, Row 14mark-ready-with-ping(
actions.md)comment-templates.md § Mark ready with pingunresolved_threads_onlyANDunresolved_threads_only_likely_addressed(every unresolvedthread has an in-thread author reply or a post-review commit, and the latest commit post-dates the
most recent thread).
@-mentions the author and every reviewer with an unresolved thread,asserting the threads "appear to have been addressed".
ready for maintainer reviewlabel.SKILL.mdGolden rule 1 means the triaging maintainer confirms before either mutation runs.Problem
Heuristic is an engagement signal, not a resolution signal. A post-review commit does not
guarantee the commit addresses the specific thread; an in-thread reply does not guarantee the reply
resolves it (it could be a question or pushback). The triaging maintainer confirming the proposal is
endorsing "the heuristic looks plausible", not "the threads are definitively resolved". The comment
template, however, states "appear to have been addressed" with the original reviewer
@-mentioned —the reviewer receives a push notification framed as a stronger claim than the underlying evidence
supports.
The reviewer mention duplicates the label. Maintainers pull review-ready PRs from the
ready for maintainer reviewqueue regardless. Whichever maintainer picks the PR up (often the originalreviewer, sometimes another) can then re-engage with the reviewer in a more informed way than the
bot's heuristic ping. The explicit
@-mention is an early push notification on top of a pull-modequeue signal that already exists — useful in the true-positive case (saves a day), wasted in the
false-positive case (pings a reviewer who would have noticed the unresolved state themselves once the
PR came up in queue).
False-positive cost lands on other maintainers. The triaging maintainer pays one quick "is the
heuristic plausible?" decision; every reviewer mentioned pays a push notification and a context-switch
into a PR that may not actually be ready. The asymmetry is the part I'd like to fix.
rationale.mdalready names false-positive promotion as the worse failure mode and relies on thereviewer pushing back to self-correct on the next sweep. The proposal below shifts that correction
step from the reviewer to the author, where the information lives.
Proposed flow
Replace Row 14's single-shot
mark-ready-with-pingwith a two-sweep gated path. The triagingmaintainer still confirms every action; only the content of each step changes.
Sweep N (heuristic match):
request-author-confirmation(new).@-mentions the author only, asking whether they believe all review feedbackhas been addressed and the PR is ready for maintainer review.
Sweep N+M (author replied):
mark-ready-no-ping(new): addready for maintainer reviewlabel, no comment, no reviewer mention.ping(Row 15) or skip; heuristic is re-evaluated normally on the next sweep.Sweep N+K (author silent):
stale_ready_label: if the confirm-request is older than the project'sgrace window with no author reply, surface to the maintainer as a stale-confirm-request group (typical
resolution: plain
pingor close).Cost / benefit
Trade is one sweep of latency in exchange for removing the reviewer mention path entirely and routing
label application through an explicit author confirmation. The "reviewer mention saves a day in
true-positive cases" benefit is real but small — the PR still reaches the same reviewer through the
label queue, just one sweep later.
I may be overweighting the notification-cost side of this trade-off, so I'd be interested in how other
maintainers think about the current reviewer-ping behaviour.
Beta Was this translation helpful? Give feedback.
All reactions