Skip to content

feat(pr-management-triage): keep ready label during merit discussion#232

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:pr-triage-merit-discussion-keep-ready-label
May 19, 2026
Merged

feat(pr-management-triage): keep ready label during merit discussion#232
potiuk merged 1 commit into
apache:mainfrom
potiuk:pr-triage-merit-discussion-keep-ready-label

Conversation

@potiuk

@potiuk potiuk commented May 19, 2026

Copy link
Copy Markdown
Member

Summary

Adds a merit-discussion-in-flight exception to the
strip-ready-on-downgrade hard rule in
pr-management-triage. When the PR has any unresolved review
thread whose first comment is from a maintainer
(COLLABORATOR / MEMBER / OWNER), the ready for maintainer review label is preserved on regression:

  • draft actions skip the draft conversion (PR stays open);
    the violations comment is still posted.
  • close actions skip the close (PR stays open); the comment
    and the quality-violations label still apply.
  • comment actions just post the violations comment without
    stripping the label.

rerun / rebase / ping were already non-stripping and
are unchanged.

Motivation

The ready for maintainer review label exists to attract
senior eyes. An unresolved maintainer-opened review thread
is exactly the moment those eyes are most valuable —
stripping the label or pushing the PR back to draft
mid-discussion makes it disappear from the maintainer queue
at the worst possible time. CI red / lint failures / merge
conflicts and a live design debate are orthogonal axes: a
maintainer can usefully weigh in on design even when CI is
red.

Originating user-scope feedback memory:
feedback-ready-for-maintainer-review-label.

Design choices

  • Precondition is deliberately broad. Any
    maintainer-opened unresolved review thread satisfies
    merit_discussion_thread_present, regardless of body
    length or when it was opened relative to the label-add.
    A narrower "substantive content" heuristic risked
    mis-classifying short-but-substantive prompts ("is this
    really the right layer?") as trivial. Erring toward
    keeping the label is the safer asymmetry: a stale-but-kept
    label costs a maintainer a glance; a stripped label
    mid-discussion costs the discussion its audience.
  • Contributor-author threads do not satisfy the
    precondition.
    Contributor-to-contributor side chatter
    is not the merit signal the label defers to. Mirrors the
    unresolved_threads_only collaborator qualifier already
    used by rows 14c/15.
  • Preview must quote the maintainer-thread URL(s) that
    triggered the exception
    so the maintainer can
    sanity-check before confirming.

Out of scope

stale-sweeps.md Sweep 4 (4a strip / 4b close) also touches
the ready label, but on a different signal — author
silence ≥ 7 days after a maintainer comment
. That flow is
about a stalled discussion, not an active one; applying the
same exception there would block stripping a label whose
discussion has been stuck for weeks. Flagged for follow-up
if desired.

Files

  • .claude/skills/pr-management-triage/classify-and-act.md
    — new merit_discussion_thread_present precondition;
    exception block appended to the strip-ready-on-downgrade
    hard rule.
  • .claude/skills/pr-management-triage/actions.md — Case A
    (default strip-and-mutate) vs Case B (exception)
    branches in the draft, comment, and close recipes.
  • .claude/skills/pr-management-triage/rationale.md — new
    "Merit-discussion exception to strip-ready-on-downgrade"
    section.

Test plan

  • uv run --project tools/skill-validator skill-validate
    passes locally.
  • Manual: ready-labelled PR + CI red + maintainer-opened
    unresolved thread → proposed action keeps the label and
    skips the draft conversion.
  • Manual: ready-labelled PR + CI red + only
    contributor-author threads → strip-on-downgrade still
    fires (no false-positive exception).
  • Manual: ready-labelled PR + merge conflict +
    maintainer thread → keep label + skip close, still post
    comment.

Add a merit-discussion-in-flight exception to the
strip-ready-on-downgrade hard rule. When the PR has an
unresolved review thread whose first comment is from a
COLLABORATOR/MEMBER/OWNER, the `ready for maintainer
review` label is preserved on regression — `draft` actions
additionally skip the draft conversion, `close` actions
skip the close (comment + quality-violations label still
apply), `comment` actions just post without stripping.

The precondition is deliberately broad — any
maintainer-opened unresolved thread satisfies it,
regardless of body length or whether it was opened before
or after the label-add. Contributor-author threads alone
do not satisfy it.

Source: user-scope feedback memory
`feedback-ready-for-maintainer-review-label`. CI red and
a live design debate are orthogonal; a maintainer can
weigh in on design even when CI is red.

Files touched (skill `pr-management-triage`):
- classify-and-act.md — new `merit_discussion_thread_present`
  precondition + exception block in the
  `strip-ready-on-downgrade` hard rule.
- actions.md — Case A / Case B branches in `draft`,
  `comment`, and `close` recipes; preview must quote the
  maintainer-thread URLs that triggered the exception.
- rationale.md — new "Merit-discussion exception" section
  explaining why the precondition is broad.

Out of scope: \`stale-sweeps.md\` Sweep 4 also strips the
ready label, but on a different signal (author silence
>= 7 days) — flagged for follow-up if wanted.

Generated-by: Claude Code (Opus 4.7)
@potiuk potiuk merged commit 01d71dd into apache:main May 19, 2026
13 checks passed
potiuk added a commit that referenced this pull request May 31, 2026
…idle trackers in bulk mode (#414)

Bulk sync (sync all, sync announced, etc.) currently dispatches
one full subagent per resolved tracker. Each subagent loads the
skill + does a `gh issue view` + reads comments + reads mail +
returns a structured report — ~50 KB per subagent transcript.
On bulk sweeps where 30–50% of trackers are in steady state
(closed > 30d with `announced`, or open with the full
cve-allocated + pr-merged + announced label set and no recent
activity), the subagent's full work is a no-op that produces an
empty proposal — pure waste.

This change inserts a Step 1b pre-flight classifier between
selector-resolution and subagent dispatch. One batched
`gh api graphql` round-trip fetches `state`, `closedAt`,
`updatedAt`, `labels`, and the last comment's author+timestamp
for every resolved issue at once (aliased multi-field query,
~3 KB request, ~6 KB response for 30 issues). A conservative
rule table classifies each as `dispatch` / `dispatch-urgent` /
`skip-noop`; only the non-skipped ones get subagents.

Safety:

* Conservative — `skip-noop` fires only when multiple signals
  align (closed AND age AND label set AND inactive last comment
  AND bot last commenter).
* `updatedAt` within last 7 days is an absolute override; never
  skip a tracker with recent activity regardless of other
  signals.
* Pre-flight only applies to set-resolving selectors
  (`sync all`, `sync announced`, label/title selectors). An
  explicit number selector like `sync #232, #233` never skips.
* Every skip appears in the proposal's "Pre-flight skipped"
  group with the rule that fired — never silent. The user can
  `force-sync <N>` any of them at confirmation.
* `--no-preflight` opts out entirely.

This is a skill-instruction change; no Python tool added. The
orchestrator builds the GraphQL query directly. Rules can be
iterated quickly by editing the table; if real-world results
show the classifier is too aggressive or too timid, the patches
are one-line edits to the rule table.
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