docs(triage): waiting-on-author bucket, read reviews not only comments, deliberate-behavior premise check
This commit is contained in:
@@ -11,7 +11,7 @@ The maintainer directs the project at the product level; they plan and understan
|
||||
|
||||
Choose exactly one. When torn between two, the deciding question is always: **what does accepting this cost the maintainer over the next year?** Between PUSH-BACK and MERGE-THEN-FIX specifically, size of the residue never decides — its owner does: *does the fix close the symptom?* then *whose knowledge finishes it?* then *what does a round-trip cost?*
|
||||
|
||||
**Product fit is the maintainer's call, not yours.** For a PR that adds or changes user-facing functionality, judge the code but never silently decide the feature is wanted: state the product question explicitly (who asks for this, what it costs the product) and make the verdict conditional on the maintainer's answer when desirability is genuinely open — "PUSH-BACK if you want this feature; DECLINE if you don't". A bug fix has no product question; a new surface always does.
|
||||
**Product fit is the maintainer's call, not yours.** For a PR that adds or changes user-facing functionality, judge the code but never silently decide the feature is wanted: state the product question explicitly (who asks for this, what it costs the product) and make the verdict conditional on the maintainer's answer when desirability is genuinely open — "PUSH-BACK if you want this feature; DECLINE if you don't". A bug fix has no product question; a new surface always does. So does a PR that **removes or bypasses behavior the code marks as deliberate** — a `skip`/`intentionally`/`on purpose` comment, a guard with a reason next to it, a suppression with its own setting: the PR's premise ("this is a bug") is then the first thing to question, before any of its implementation. Put the product question to the maintainer up front — "the code suppresses X on purpose; the PR treats that as a defect — is it?" — and hold the implementation findings until it is answered; a push-back list on a change whose premise the maintainer rejects is wasted work for both sides.
|
||||
|
||||
1. **DECLINE** — the project must not take this change. Grounds:
|
||||
- *Whim*: functionality that suits the author's personal workflow, not the product's direction.
|
||||
|
||||
@@ -7,7 +7,7 @@ Turn an unbounded PR queue into a short list of maintainer decisions. The pipeli
|
||||
|
||||
Companion: each substantive review inside phase 3 applies the `pr-review` skill; this skill owns only the batch mechanics around it.
|
||||
|
||||
**The timeline outranks the snapshot.** Before any verdict or comment on a PR, read its existing comments: a prior maintainer verdict (a push-back list, a recorded product decision like a placement or scope call) is BINDING — a new sweep verifies whether it was addressed at the current HEAD and says so explicitly ("all three prior items resolved" / "item 2 still open"), never re-decides it or asks the maintainer the same product question again. And never post the generic rebase-request on a PR that already carries a substantive review comment — the author already has their instructions; a bare "please rebase" on top reads as the left hand not knowing the right.
|
||||
**The timeline outranks the snapshot.** Before any verdict or comment on a PR, read its full timeline — issue comments AND reviews (`gh pr view --json comments,reviews` or `gh api repos/{owner}/{repo}/pulls/N/reviews`; a maintainer's *Changes requested* is a review and never appears in the comments list) AND commits since the last human event: a prior maintainer verdict (a push-back list, a recorded product decision like a placement or scope call) is BINDING — a new sweep verifies whether it was addressed at the current HEAD and says so explicitly ("all three prior items resolved" / "item 2 still open"), never re-decides it or asks the maintainer the same product question again. And never post the generic rebase-request on a PR that already carries a substantive review comment — the author already has their instructions; a bare "please rebase" on top reads as the left hand not knowing the right.
|
||||
|
||||
**Pickup mode.** A PR with human activity beyond the bot — a maintainer comment, an author reply, a trusted-reviewer thread — is a conversation in progress, not a fresh review target. Such PRs go into their own report bucket ("Розмова триває"), and each entry opens with the thread state: what the maintainer asked, what the author answered, which points are resolved at the current HEAD and which remain. The ready action *continues* the thread (a reply, a verdict on the author's answer, a merge if everything asked for was delivered) — it never restarts review from scratch. The maintainer may not remember their own comment from days ago; the sweep remembers for them.
|
||||
|
||||
@@ -24,7 +24,8 @@ Bucket every non-draft PR:
|
||||
|---|---|---|
|
||||
| Dead | merge conflict AND no author commit in >30 days | close with **stale-close** |
|
||||
| Conflicted-active | merge conflict, author committed within 30 days | comment **rebase-request**, leave open |
|
||||
| Clean | mergeable | phase 3 review pool |
|
||||
| Waiting on author | the last substantive event is a request for changes — a maintainer review with `CHANGES_REQUESTED`, a maintainer push-back comment, or a bot `review:blocked` / `review:needs-evidence` — and the author has neither pushed nor replied since | one line in the report ("чекає автора: <what was asked>"); no re-review, no new comment — the ball is theirs |
|
||||
| Clean | mergeable and not waiting on the author | phase 3 review pool |
|
||||
| Draft | `isDraft` | untouched until marked ready |
|
||||
|
||||
Then detect **duplicate clusters** across the survivors: pairs with high title-token overlap or high changed-file overlap. For each cluster recommend one keeper (prefer: mergeable over conflicted, references an issue, smaller diff, earlier author — a later near-identical body is likely a regenerated copy of the earlier PR, and the earlier author keeps the credit); the rest close with **duplicate-close**.
|
||||
@@ -39,7 +40,7 @@ Execute the approved closes/comments with retries and ~1–2s spacing between ca
|
||||
|
||||
**Trusted community reviewers.** `yulia-ivashko` is a core maintainer with merge rights — her review decisions carry maintainer weight (a PR she approved or merged needs no re-verdict; her open questions are the maintainer's questions). Comments and reviews from `patrick-motard` and `mattv8` are strong human signals: during any sweep, collect the PRs/issues they weighed in on, read their assessment, and carry it into the verdict — an approval from them upgrades confidence like a passing verifier; a concern from them is a finding to verify, never to ignore. They write free-form; map their conclusion onto the verdict ladder rather than expecting the format.
|
||||
|
||||
The review bot's `review:*` labels are a pre-sort, not a verdict: `review:ready` PRs go first (the bot found no code defects — likely MERGE/MERGE-THEN-FIX), `review:blocked` ones carry a bot comment whose findings the verdict review verifies rather than rediscovers. Bot labels never replace the pr-review pass — the bot cannot judge product fit or maintainability scope.
|
||||
The review bot's `review:*` labels are a pre-sort, not a verdict: `review:ready` PRs go first (the bot found no code defects — likely MERGE/MERGE-THEN-FIX), `review:blocked` ones carry a bot comment whose findings the verdict review verifies rather than rediscovers. Bot labels never replace the pr-review pass — the bot cannot judge product fit or maintainability scope. The reverse holds too: when the bot's BLOCKED findings are the whole story and the author has not answered, the maintainer never re-posts them in their own voice — the PR is *waiting on author* and the report says so in one line.
|
||||
|
||||
Split the clean pool smallest-first (tiny diffs are fast wins and most likely mergeable). Fan out the `pr-reviewer` subagent (`.opencode/agent/pr-reviewer.md`, which loads the `pr-review` skill and carries the hard rules) — one PR per call, or ~10 PRs per general subagent that receives the full `pr-review` skill text when `pr-reviewer` is unavailable. The subagent inherits the chat's model; never hand verdicts to a smaller model to save quota — a verdict from a small model is a pre-sort, not a decision. Each returns per-PR verdict blocks in the skill's output format.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user