This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow-steward.git
The following commit(s) were added to refs/heads/main by this push:
new 1a0d31f feat(pr-management-triage): F5a/F5b/follow_up_ping consider
review-thread comments + row-18 author-push cooldown (#115)
1a0d31f is described below
commit 1a0d31fd24560a7192d452ded0d2325b00cd5bcf
Author: Jarek Potiuk <[email protected]>
AuthorDate: Mon May 11 12:30:32 2026 +0200
feat(pr-management-triage): F5a/F5b/follow_up_ping consider review-thread
comments + row-18 author-push cooldown (#115)
Three related improvements to the "respect active conversations"
logic, all prompted by misclassifications hit during a triage
sweep on apache/airflow:
1. F5a (author cooldown) and F5b (m2m ping unanswered) extend
the "most recent comment" union to include review-thread
comments, not just general issue comments. A maintainer
asking a clarifying question in a review thread is just as
active as a top-level comment, but the prior text only
considered comments(last:10) and let the active conversation
be auto-pinged.
2. follow_up_ping adds a third resolving signal: author's most
recent commit < 24h old. The push itself is the follow-up;
the author is still actively working through the reviewer's
feedback. 24h is the shortest gap that lets a fixup-and-push
cycle finish without an interruption.
3. Required GraphQL fields table updated to reflect that F5a,
F5b, F6, and Row 18 now consume reviewThreads.nodes.comments.
The field is already populated by the existing per-page
batch query (used for unresolved-thread classification), so
no new query needed — only the documentation contract.
---
.../pr-management-triage/classify-and-act.md | 32 ++++++++++++++--------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/.claude/skills/pr-management-triage/classify-and-act.md
b/.claude/skills/pr-management-triage/classify-and-act.md
index c425271..3fa7040 100644
--- a/.claude/skills/pr-management-triage/classify-and-act.md
+++ b/.claude/skills/pr-management-triage/classify-and-act.md
@@ -45,8 +45,8 @@ filter is skipped silently from the main triage flow.
| F2 | Author is a known bot | login is `dependabot`, `dependabot[bot]`,
`renovate[bot]`, `github-actions`, `github-actions[bot]`, or matches `*[bot]` |
| F3 | Draft and not stale | `isDraft == true` and any activity within the
last 14 days. Stale-sweep classifications in
[`stale-sweeps.md`](stale-sweeps.md) may still pull the PR back in. |
| F4 | Already marked ready, no regression | `labels` contains `ready for
maintainer review` AND CI green AND `mergeable != CONFLICTING` AND no
unresolved threads. **Regression bypasses this filter** — any of: CI red, new
conflict, or new unresolved thread whose triggering event (failing check
`startedAt`, conflict detection, thread `createdAt`) is *after* the label-add
timestamp. The typical case is a contributor pushing a rebase or fixup commit
to a ready-for-review PR that re-introduc [...]
-| F5a | Recent collaborator comment (author cooldown) | Most recent comment is
by a `COLLABORATOR`/`MEMBER`/`OWNER`, `createdAt < 72h` ago, AND posted after
`commits(last:1).committedDate`. |
-| F5b | Maintainer-to-maintainer ping unanswered | Most recent collaborator
comment `@`-mentions one or more logins other than the PR author AND none of
those mentioned logins have posted on the PR or in `latestReviews` after that
comment. Team mentions (e.g. `@<upstream>-committers`) are conservatively
treated as F5b matches. |
+| F5a | Recent collaborator comment (author cooldown) | Most recent comment
from the **union of** general-issue comments (`comments(last:10)`) and
**review-thread comments** (`reviewThreads.nodes.comments`) is by a
`COLLABORATOR`/`MEMBER`/`OWNER`, `createdAt < 72h` ago, AND posted after
`commits(last:1).committedDate`. The review-thread leg is essential — a
maintainer asking a clarifying question in-thread is just as much an active
conversation as a top-level comment, and treating only t [...]
+| F5b | Maintainer-to-maintainer ping unanswered | Most recent collaborator
comment from the **union** described in F5a above `@`-mentions one or more
logins other than the PR author AND none of those mentioned logins have posted
on the PR (general comments **or** review threads) or in `latestReviews` after
that comment. Team mentions (e.g. `@<upstream>-committers`) are conservatively
treated as F5b matches. |
| F6 | Maintainer co-drafted | `isDraft == true` AND any of: (a)
`latestReviews` has a node with `authorAssociation ∈ {OWNER, MEMBER,
COLLABORATOR}` AND `author.login ≠ <viewer>` AND `state ∈ {COMMENTED,
CHANGES_REQUESTED, APPROVED}` AND `submittedAt > commits(last:1).committedDate`
AND review body is non-empty (avoids the "review with only inline thread
comments and an empty top-level body" false positive — those are already
counted by row 14/15 unresolved-thread logic); (b) `comments(l [...]
F5a, F5b, and F6 override every signal in the decision table —
@@ -288,13 +288,23 @@ True when at least one of the following resolves the
apparent
`stale_review` (Row 18):
- A comment by the PR author after the most recent
- `CHANGES_REQUESTED` review (`comments(last:10)`) whose body
- `@`-mentions the reviewer login.
-- A comment by the reviewer after the author's most recent
- commit (`commits(last:1).committedDate`).
-
-Either signal indicates the conversation is alive and a fresh
-ping would talk over an existing nudge. False otherwise.
+ `CHANGES_REQUESTED` review (`comments(last:10)` or
+ `reviewThreads.nodes.comments`) whose body `@`-mentions the
+ reviewer login.
+- A comment by the reviewer (general or review-thread) after
+ the author's most recent commit
+ (`commits(last:1).committedDate`).
+- The PR author's most recent commit is **less than 24 hours
+ old**. The push itself is the follow-up — they're still
+ actively working through the reviewer's feedback. Pinging
+ immediately reads as the bot rushing them. (24 h is the
+ shortest gap that lets the author finish a fixup-and-push
+ cycle without an interruption; the broader F5a 72h cooldown
+ applies on the maintainer side.)
+
+Any of these signals indicates the conversation is alive and a
+fresh ping would talk over an existing exchange. False
+otherwise.
---
@@ -419,11 +429,11 @@ applies — rows do not get to reach back for more data.
| Decision rows / preconditions | Required fields |
|---|---|
-| F5a, F5b, F6, grace periods |
`comments(last:10).nodes.{author.login,authorAssociation,bodyText,createdAt}`,
`latestReviews.nodes.{state,author.login,authorAssociation,submittedAt}`,
`commits(last:1).nodes.commit.committedDate`, viewer login |
+| F5a, F5b, F6, grace periods |
`comments(last:10).nodes.{author.login,authorAssociation,bodyText,createdAt}`,
`reviewThreads.nodes.comments(first:5).nodes.{author.login,authorAssociation,bodyText,createdAt}`,
`latestReviews.nodes.{state,author.login,authorAssociation,submittedAt}`,
`commits(last:1).nodes.commit.committedDate`, viewer login |
| Row 1 + Real-CI guard | `statusCheckRollup.state`,
`statusCheckRollup.contexts`, `authorAssociation`, `head_sha` (REST
`action_required` index keyed by `head_sha`) |
| `copilot_review_stale` (row 2) |
`reviewThreads.nodes.{isResolved,comments.nodes.{author.login,createdAt,url}}`,
`comments(last:10).nodes.{author.login,createdAt}` |
| `has_deterministic_signal`, `ci_failures_only`, `unresolved_threads_only`,
`unresolved_threads_only_likely_addressed` (rows 8–17) | `mergeable`,
`statusCheckRollup.{state,contexts}`,
`reviewThreads.nodes.{isResolved,comments(first:5).nodes.{author.login,authorAssociation,createdAt}}`,
`updatedAt`,
`comments(last:10).nodes.{author.login,authorAssociation,createdAt}`,
`commits(last:1).nodes.commit.committedDate`, `author.login` |
-| Row 18 (`stale_review`) |
`latestReviews.nodes.{state,author.login,submittedAt}`,
`commits(last:1).nodes.commit.committedDate`, `comments(last:10)` |
+| Row 18 (`stale_review`) |
`latestReviews.nodes.{state,author.login,submittedAt}`,
`commits(last:1).nodes.commit.committedDate`, `comments(last:10)`,
`reviewThreads.nodes.comments(first:5).nodes.{author.login,createdAt}` |
| Rows 3–5 (`already_triaged` / `stale_draft` from triage marker) |
`comments(last:10).nodes.{author.login,bodyText,createdAt}`, viewer login,
`commits(last:1).nodes.commit.committedDate` |
| Rows 19, 20 (`passing`) | `statusCheckRollup.state`,
`statusCheckRollup.contexts`, `mergeable`, `reviewThreads.totalCount`, `labels`
|
