From ba94a7fbd7c5ed0c778d7a7ac1f49b6bae28826a Mon Sep 17 00:00:00 2001 From: Karibhou <157208370+Karibhou@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:59:41 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix:=20changes-requested=20PRs?= =?UTF-8?q?=20no=20longer=20count=20as=20waiting=20on=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hive Chat's `/prs waiting on review` filter was `!Draft && !approved`, so a PR whose review decision is CHANGES_REQUESTED was listed as waiting on a reviewer even though the next move is the author's. Add a chatPRChangesRequested helper, exclude those PRs from the predicate, reuse the helper for the "changes requested" row tag, and document the rule. Closes #14437 Hive-Run: hivecommons/hive#14437 Hive-Plan: review-backlog/correctness Hive-Spec: hive-chat-prs#waiting-on-review Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XhLFfSq5K9MhMey7RkaK7Q Signed-off-by: Karibhou <157208370+Karibhou@users.noreply.github.com> --- .../fixed-14437-prs-changes-requested.md | 1 + src/docs/dashboard.md | 2 +- src/pkg/dashboard/api_chat_prs.go | 15 +++++++--- src/pkg/dashboard/api_chat_prs_test.go | 28 +++++++++++++++++-- 4 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 changelog.d/fixed-14437-prs-changes-requested.md diff --git a/changelog.d/fixed-14437-prs-changes-requested.md b/changelog.d/fixed-14437-prs-changes-requested.md new file mode 100644 index 0000000000..848937536e --- /dev/null +++ b/changelog.d/fixed-14437-prs-changes-requested.md @@ -0,0 +1 @@ +- Hive Chat `/prs waiting on review` no longer lists pull requests whose review decision is changes requested; those wait on their author, not on a reviewer ([#14437](https://github.com/hivecommons/hive/issues/14437)). diff --git a/src/docs/dashboard.md b/src/docs/dashboard.md index 616dfc491a..a1304230be 100644 --- a/src/docs/dashboard.md +++ b/src/docs/dashboard.md @@ -202,7 +202,7 @@ by repo, one row each — `owner/repo#N — title (@author · stage labels · ag — under the aggregate count, capped at 25 rows with an "… and N more" footer. The remainder of the command narrows the list and every filter is a conjunct: `waiting on review` / `needs review` (not a draft, no approving -review), `needs decision` (`stage/085-needs-decision` or +review, no outstanding change request), `needs decision` (`stage/085-needs-decision` or `stage/045-needs-decision`), `hold` (held set or a hold label), `failing ci`, `mine` (the signed-in viewer) / `by ` / `@login`, `repo ` or a bare `owner/repo` token (the short repo name also works), and `stale` diff --git a/src/pkg/dashboard/api_chat_prs.go b/src/pkg/dashboard/api_chat_prs.go index bfc0c946a9..4c2973d017 100644 --- a/src/pkg/dashboard/api_chat_prs.go +++ b/src/pkg/dashboard/api_chat_prs.go @@ -297,10 +297,17 @@ func chatPRApproved(pr github.PullRequest) bool { return pr.Protection.ReviewDecision == github.ReviewDecisionApproved || pr.Protection.ApprovalsGiven > 0 } -// chatPRWaitingOnReview is the `waiting on review` predicate: not a draft and -// no approving review yet. +// chatPRChangesRequested is true when GitHub's review decision asks the author +// for changes. +func chatPRChangesRequested(pr github.PullRequest) bool { + return pr.Protection != nil && pr.Protection.ReviewDecision == github.ReviewDecisionChangesRequested +} + +// chatPRWaitingOnReview is the `waiting on review` predicate: not a draft, no +// approving review yet, and no outstanding change request — a PR with changes +// requested is waiting on its author, not on a reviewer. func chatPRWaitingOnReview(pr github.PullRequest) bool { - return !pr.Draft && !chatPRApproved(pr) + return !pr.Draft && !chatPRApproved(pr) && !chatPRChangesRequested(pr) } func (f chatPRFilter) matches(row chatPRRow, now time.Time) bool { @@ -379,7 +386,7 @@ func chatPRLine(row chatPRRow, now time.Time) string { meta = append(meta, chatPRCI(pr)) if chatPRApproved(pr) { meta = append(meta, "approved") - } else if pr.Protection != nil && pr.Protection.ReviewDecision == github.ReviewDecisionChangesRequested { + } else if chatPRChangesRequested(pr) { meta = append(meta, "changes requested") } if chatPRNeedsDecision(pr.Labels) { diff --git a/src/pkg/dashboard/api_chat_prs_test.go b/src/pkg/dashboard/api_chat_prs_test.go index c41064ee08..ce85ade2e7 100644 --- a/src/pkg/dashboard/api_chat_prs_test.go +++ b/src/pkg/dashboard/api_chat_prs_test.go @@ -110,10 +110,10 @@ func TestRenderChatPRList(t *testing.T) { absent: []string{"github.com/hivecommons/hive/pull/101", "Ignored", "… and"}, }, { - name: "waiting on review drops approved and drafts", + name: "waiting on review drops approved, drafts and changes requested", query: "prs: waiting on review", - want: []string{"4 of 6 open pull request(s) match waiting on review:", "#101", "#104", "#105", "docs#7"}, - absent: []string{"#102", "#103"}, + want: []string{"3 of 6 open pull request(s) match waiting on review:", "#101", "#105", "docs#7"}, + absent: []string{"#102", "#103", "#104"}, }, { name: "needs decision", @@ -195,6 +195,28 @@ func TestRenderChatPRList(t *testing.T) { } } +func TestChatPRWaitingOnReview(t *testing.T) { + cases := []struct { + name string + pr github.PullRequest + want bool + }{ + {name: "no review yet", pr: github.PullRequest{}, want: true}, + {name: "review required", pr: github.PullRequest{Protection: &github.ProtectionFacts{ReviewDecision: github.ReviewDecisionReviewRequired}}, want: true}, + {name: "draft", pr: github.PullRequest{Draft: true}, want: false}, + {name: "approved", pr: github.PullRequest{Protection: &github.ProtectionFacts{ReviewDecision: github.ReviewDecisionApproved}}, want: false}, + {name: "changes requested waits on the author", pr: github.PullRequest{Protection: &github.ProtectionFacts{ReviewDecision: github.ReviewDecisionChangesRequested}}, want: false}, + {name: "changes requested with an approval", pr: github.PullRequest{Protection: &github.ProtectionFacts{ReviewDecision: github.ReviewDecisionChangesRequested, ApprovalsGiven: 1}}, want: false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := chatPRWaitingOnReview(tc.pr); got != tc.want { + t.Fatalf("chatPRWaitingOnReview = %v, want %v", got, tc.want) + } + }) + } +} + func TestRenderChatPRListCapsAndFooter(t *testing.T) { now := time.Date(2026, 10, 26, 12, 0, 0, 0, time.UTC) repo := FrontendRepo{Name: "hive", Full: "hivecommons/hive"}