Repository navigation
π fix: changes-requested PRs no longer count as waiting on review - #14440
hivecommons-hive[bot] merged 2 commits into
Conversation
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 hivecommons#14437 Hive-Run: hivecommons#14437 Hive-Plan: review-backlog/correctness Hive-Spec: hive-chat-prs#waiting-on-review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XhLFfSq5K9MhMey7RkaK7Q Signed-off-by: Karibhou <157208370+Karibhou@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of No conflicting open PRs or same-file overlaps were found. |
There was a problem hiding this comment.
Read the predicate and its two callers at the PR head; this looks correct to me.
src/pkg/dashboard/api_chat_prs.go:302-310βchatPRChangesRequestednil-guardsProtection, andchatPRWaitingOnReviewnow excludes it alongsidechatPRApproved(which already guards nil at:293). Thematchescall at:315and the row-tag branch at:389are the only callers; the tag branch is a pure refactor of the same condition.api_chat_prs_test.go:64,73β fixture#104isReviewDecisionChangesRequestedwith noApprovalsGiven, so the4 β 3expectation change is the behaviour #14437 asked for, not a loosened test. The newTestChatPRWaitingOnReviewcovers thechanges requested + ApprovalsGivencase, wherechatPRApprovedwins first β same result either way.src/docs/dashboard.md:205and the changelog fragment describe the new predicate accurately.
CI note for the maintainer: the red test (agent 2/5) shard is TestPollTmuxOutputForAgent_WiresThrashBreaker in src/pkg/agent (job 121845418040), which this PR does not touch β it is the base-branch failure #14415 is fixing. coverage fails only because that shard failed (agent=failure, rest=success). The dashboard package shard passed.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
@Karibhou this PR is back in your court ( Failing checks:
Reply here or push a new head to this branch (same sign-off); CI and the review swarm re-run automatically and the PR returns to the normal path. Your relay session received this as a |
|
Check report: the red agent shard here is π Hive Agent: β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
|
@Karibhou the hive has adopted this PR ( |
|
The hive adopted this PR ( Blocker: lane reason Moving to |
|
The hive adopted this PR ( Blocker: lane reason Moving to |
Base-sync: pick up hivecommons#14415 and hivecommons#14469 test fixes so CI runs against the current v5. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
hive/awaiting-ci: base-synced this adopted branch with π Hive Agent: β hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Looks correct to me, with one small follow-up worth considering.
Verified on v5 with the diff applied:
chatPRChangesRequested(api_chat_prs.go:302) nil-guardsProtectionitself, andchatPRApprovedalready does at:293, so the composed predicate is nil-safe for thegithub.PullRequest{}fixture.chatPRWaitingOnReviewhas one production caller (api_chat_prs.go:308, theNeedsReviewfilter), so the change is scoped to that filter; the renderer swap at:382is behaviour-preserving.- Fixture
#104(api_chat_prs_test.go:73-74) carriesProtection: changes, so the 4β3 expectation and the#104βabsentmove match the fixture, not just the narrative. go build ./pkg/dashboard/andgofmt -lare clean. (I can't rungo testfrom this pod; CI'sbuild-and-testwas still in progress when I looked.)- Docs and changelog fragment match the change;
Closes #14437is correct since its blocker #14410 has landed.
Low β a PR whose author already pushed after the change request is also dropped. GitHub's reviewDecision stays CHANGES_REQUESTED until the reviewer re-reviews or dismisses, even after the author pushes. The hive already distinguishes that case elsewhere: pkg/prfollowup/prfollowup.go:827 pairs ReviewDecisionChangesRequested with !github.HumanReviewAddressed(*pr) (pkg/github/review_addressing.go:33). With this PR, a change-requested PR that the author has since addressed and is now genuinely waiting on the reviewer disappears from waiting on review. Not a defect against #14437's wording, and arguably the right default for a first cut β but if that's not intended, !chatPRChangesRequested(pr) || github.HumanReviewAddressed(pr) would keep those rows. Fine to defer to a follow-up.
β hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read this against the v5 tree; looks correct to me.
src/pkg/dashboard/api_chat_prs.go:chatPRChangesRequestednil-guardsProtectionand compares againstgithub.ReviewDecisionChangesRequested(src/pkg/github/merge_block_reason.go:26), so the new conjunct inchatPRWaitingOnReviewis safe for PRs with no protection facts. The renderer'schanges requestedtag reuses the same helper with identical semantics β no output change there.- Fixture
#104inapi_chat_prs_test.go:73carriesCHANGES_REQUESTED, so the4 of 6β3 of 6expectation change is the right one, andTestChatPRWaitingOnReviewcovers the approved-plus-changes-requested corner. src/docs/dashboard.mdfilter text and thefixed-14437-β¦fragment match the behaviour.
Scope matches #14437 exactly. Nothing blocking from my side; a maintainer approval is what this needs.
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
stage 095-pre-merge-soak β 074-adopted-by-hive: untrusted-author-role |
|
Merged by hive contributor-lane auto-merge: CI green for β₯30m, reviewer verdict approve, head |
What
Hive Chat's
/prs waiting on review(needs review,unreviewed) filter used!Draft && !approved, so a PR whose GitHub review decision isCHANGES_REQUESTEDwas listed as waiting on a reviewer even though the next move belongs to its author (fixture#104, flagged in the review of #14410).src/pkg/dashboard/api_chat_prs.go: newchatPRChangesRequestedhelper;chatPRWaitingOnReviewnow also excludes PRs with an outstanding change request. The row renderer reuses the helper for itschanges requestedtag (no output change).src/docs/dashboard.md: the filter description now says "no outstanding change request".changelog.d/fixed-14437-prs-changes-requested.md: fragment.Tests
TestRenderChatPRList/ "waiting on review": expectation goes from 4 to 3 matches, with#104(changes requested) now absent. This fails on the parent commit.TestChatPRWaitingOnReviewtable test: no review, review required, draft, approved, changes requested, and changes requested with an approval.gofmt -lis clean on the changed files.Closes #14437
π€ Generated with Claude Code
https://claude.ai/code/session_01XhLFfSq5K9MhMey7RkaK7Q
β hive: backend=claude model=claude-opus-5-5