GitHub user choo121600 closed a discussion: Thoughts on replacing pr-management-triage reviewer pings with author confirmation
## Summary Row 14 of `pr-management-triage`'s decision table (action `mark-ready-with-ping`) promotes a PR to `ready for maintainer review` and posts a comment that `@`-mentions the original reviewer(s), based on the `unresolved_threads_only_likely_addressed` heuristic. Although the triaging maintainer confirms every 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) <img width="1021" height="657" alt="image" src="https://github.com/user-attachments/assets/70f8642c-441f-483e-ba00-05287f7eb626"; /> - File: `.claude/skills/pr-management-triage/classify-and-act.md`, Row 14 - Action: `mark-ready-with-ping` ([`actions.md`](../blob/main/.claude/skills/pr-management-triage/actions.md)) - Template: `comment-templates.md § Mark ready with ping` - Trigger: `unresolved_threads_only` AND `unresolved_threads_only_likely_addressed` (every unresolved thread has an in-thread author reply or a post-review commit, and the latest commit post-dates the most recent thread). - Effect on confirm: 1. Post a comment that `@`-mentions the author and every reviewer with an unresolved thread, asserting the threads "appear to have been addressed". 2. Add the `ready for maintainer review` label. `SKILL.md` Golden rule 1 means the triaging maintainer confirms before either mutation runs. ## Problem 1. **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. 2. **The reviewer mention duplicates the label.** Maintainers pull review-ready PRs from the `ready for maintainer review` queue regardless. Whichever maintainer picks the PR up (often the original reviewer, 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-mode queue 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). 3. **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.md` already names false-positive promotion as the worse failure mode and relies on the reviewer 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-ping` with a two-sweep gated path. The triaging maintainer still confirms every action; only the *content* of each step changes. **Sweep N (heuristic match):** - Action: `request-author-confirmation` (new). - Post a comment that `@`-mentions the author only, asking whether they believe all review feedback has been addressed and the PR is ready for maintainer review. - No label, no reviewer mention. **Sweep N+M (author replied):** - New decision-table row matches when the author has commented after our confirm-request. - Bot surfaces the author's reply to the triaging maintainer alongside the proposal. - If the maintainer reads the reply as affirmative → action `mark-ready-no-ping` (new): add `ready for maintainer review` label, no comment, no reviewer mention. - If the maintainer reads it as "still working on X" or asks for clarification → re-route to plain `ping` (Row 15) or skip; heuristic is re-evaluated normally on the next sweep. **Sweep N+K (author silent):** - A new stale rule mirrors `stale_ready_label`: if the confirm-request is older than the project's grace window with no author reply, surface to the maintainer as a stale-confirm-request group (typical resolution: plain `ping` or close). ## Cost / benefit | | Current Row 14 | Proposed | |---|---|---| | Reviewer push notification (true-positive) | 1 | 0 | | Reviewer push notification (false-positive) | 1 (wasted) | 0 | | Author push notification | 1 | 1 | | Latency from heuristic match → label | immediate | author reply + 1 sweep | | Risk of label on a not-actually-ready PR | heuristic-gated | author-confirmation-gated | | Triaging-maintainer cognitive load per PR | "heuristic plausible?" | "author reply affirmative?" | 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. GitHub link: https://github.com/apache/airflow-steward/discussions/171 ---- This is an automatically sent email for [email protected]. To unsubscribe, please send an email to: [email protected]
